On Thu, Jan 26, 2017 at 12:38 AM, Laurent Vivier <laur...@vivier.eu> wrote: > 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...
Ok, that is a good enough reason for me. Reviewed-by: Alistair Francis <alistair.fran...@xilinx.com> Thanks, Alistair >> 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> > > >