Le 26/01/2017 à 06:50, Thomas Huth a écrit :
> On 26.01.2017 00:26, Alistair Francis wrote:
>> On Wed, Jan 25, 2017 at 12:52 PM, Laurent Vivier <laur...@vivier.eu> wrote:
>>> Le 25/01/2017 à 21:45, Thomas Huth a écrit :
>>>> When running QEMU with "-M none -device loader,file=kernel.elf", it
>>>> currently crashes with a segmentation fault, because the "none"-machine
>>>> does not have any CPU by default and the generic loader code tries
>>>> to dereference s->cpu. Fix it by adding an appropriate check for a
>>>> NULL pointer.
>>>>
>>>> Reported-by: Laurent Vivier <laur...@vivier.eu>
>>>> Signed-off-by: Thomas Huth <th...@redhat.com>
>>>> ---
>>>> hw/core/generic-loader.c | 9 +++++----
>>>> 1 file changed, 5 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/hw/core/generic-loader.c b/hw/core/generic-loader.c
>>>> index 58f1f02..4601267 100644
>>>> --- a/hw/core/generic-loader.c
>>>> +++ b/hw/core/generic-loader.c
>>>> @@ -137,20 +137,21 @@ static void generic_loader_realize(DeviceState *dev,
>>>> Error **errp)
>>>> #endif
>>>>
>>>> if (s->file) {
>>>> + AddressSpace *as = s->cpu ? s->cpu->as : NULL;
>>>
>>> Should we just abort if s->cpu is NULL?
>>
>> I agree, what is the use case where you are loading images without a CPU?
>>
>> If there is a use case (maybe some KVM thing?) then this patch looks fine to
>> me.
>
> I think there is no real use case yet. But this fix is 1) simpler than
> doing an error_report() + exit() here, and 2) maybe the vision of
> constructing machines on the fly with QEMU will eventually come true one
> day in the distant future, so with that patch here, the code would
> already be prepared for the case when QEMU gets started without CPUs and
> the CPUs are then later added via QOM...
> Well, I don't mind ... if you prefer an error message instead, feel free
> to suggest another patch. I'm fine as long as we do not simply crash
> with a segmentation fault here.
OK, the use of NULL as "as" seems reasonable (this is what uses
load_elf()), so:
Reviewed-by: Laurent Vivier <laur...@vivier.eu>
