qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-ppc] [PATCH] PPC: MMU compatibility check.


From: Thomas Huth
Subject: Re: [Qemu-trivial] [Qemu-ppc] [PATCH] PPC: MMU compatibility check.
Date: Wed, 25 Jan 2017 09:12:07 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0

On 24.01.2017 22:41, Valentin Plotkin wrote:
> On Tue, 24 Jan 2017, Thomas Huth wrote:
> 
>> Date: Tue, 24 Jan 2017 21:32:44 +0100
>> From: Thomas Huth <address@hidden>
>> To: Valentin Plotkin <address@hidden>, address@hidden
>> Cc: address@hidden, address@hidden
>> Subject: Re: [Qemu-ppc] [PATCH] PPC: MMU compatibility check.
>>
>> On 24.01.2017 19:56, Valentin Plotkin wrote:
>>>
>>> Hi everyone,
>>>
>>> I looked at the "qemu-system-ppc -nographic -cpu G2leGP3 -M ppce500" on
>>> the BiteSizedTasks page. The segfault was caused by machine
>>> initialization code which expected a certain MMU model, checked, so
>>> unused SPR were read, returning zeros. bamboo and virtex machines are
>>> affected as well, but it doesn't always cause segfault, usually running
>>> into unmapped memory and failing somewhat more nicely.
>>>
>>> I added the checks. It would be possible to add support for other MMU
>>> models, but I'm not sure if there is any point (would any guest OS
>>> support mutually exclusive CPU and machine)?
>>
>> Hi,
>>
>> great to have a fix for this crash! I don't think it make much sense to
>> add support for other MMU models here, so the simple checks should be
>> good enough.
>> However, your new code obviously does not follow the QEMU coding style.
>> Could you please run your patch through scripts/checkpatch.pl and fix
>> all issues that it reports? And when you resubmit, please make sure to
>> copy the maintainers on CC: as well (scripts/get_maintainers.pl is your
>> friend here).
> 
> Here is fengshuised version (at least I hope so). Thanks for guiding me.

Thanks, code looks better now!
Now to get your patch included, you've got to send it in a separate
mail, with only the patch description in it, i.e. not as a reply to
another mail with the history of the mail thread. Reason for this is
simply that the maintainers can pick up your patch with "git am" without
having to edit things manually. Please see also
http://qemu-project.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message
if you haven't read that page yet.

Ah, and two more things that came to my mind:

1) Please use "exit(1)" (or "exit(EXIT_FAILURE)") instead of "exit(-1)",
since that's the more common way to do that (since AFAIK the -1 will be
cast to 255 at the shell level, which is just ugly).

2) Maybe it would also be better to do the check right after the
cpu_ppc_init() in the machine's init function already, instead of doing
it in the mmubooke_create_initial_mapping() function? That way we could
be sure that wrong CPUs are blocked right from the start.

 Thanks,
  Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]