Message ID | 1579100861-73692-42-git-send-email-imammedo@redhat.com |
---|---|
State | New |
Headers | show |
Series | refactor main RAM allocation to use hostmem backend | expand |
On Wed, 15 Jan 2020, Igor Mammedov wrote: > From: Philippe Mathieu-Daudé <f4bug@amsat.org> > > The firmware has to reside in the PDC range. If the Elf file > expects to load it below FIRMWARE_START, it is incorrect, > regardless the RAM size. > > Acked-by: Helge Deller <deller@gmx.de> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > hw/hppa/machine.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c > index 5d0de26..6775d87 100644 > --- a/hw/hppa/machine.c > +++ b/hw/hppa/machine.c > @@ -155,7 +155,7 @@ static void machine_hppa_init(MachineState *machine) > qemu_log_mask(CPU_LOG_PAGE, "Firmware loaded at 0x%08" PRIx64 > "-0x%08" PRIx64 ", entry at 0x%08" PRIx64 ".\n", > firmware_low, firmware_high, firmware_entry); > - if (firmware_low < ram_size || firmware_high >= FIRMWARE_END) { > + if (firmware_low < FIRMWARE_START || firmware_high >= FIRMWARE_END) { > error_report("Firmware overlaps with memory or IO space"); > exit(1); Should this also be EXIT_FAILURE like in other places when you're changing the line nearby? Regards, BALATON Zoltan
On 1/15/20 7:15 PM, BALATON Zoltan wrote: > On Wed, 15 Jan 2020, Igor Mammedov wrote: >> From: Philippe Mathieu-Daudé <f4bug@amsat.org> >> >> The firmware has to reside in the PDC range. If the Elf file >> expects to load it below FIRMWARE_START, it is incorrect, >> regardless the RAM size. >> >> Acked-by: Helge Deller <deller@gmx.de> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >> --- >> hw/hppa/machine.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >> index 5d0de26..6775d87 100644 >> --- a/hw/hppa/machine.c >> +++ b/hw/hppa/machine.c >> @@ -155,7 +155,7 @@ static void machine_hppa_init(MachineState *machine) >> qemu_log_mask(CPU_LOG_PAGE, "Firmware loaded at 0x%08" PRIx64 >> "-0x%08" PRIx64 ", entry at 0x%08" PRIx64 ".\n", >> firmware_low, firmware_high, firmware_entry); >> - if (firmware_low < ram_size || firmware_high >= FIRMWARE_END) { >> + if (firmware_low < FIRMWARE_START || firmware_high >= >> FIRMWARE_END) { >> error_report("Firmware overlaps with memory or IO space"); >> exit(1); > > Should this also be EXIT_FAILURE like in other places when you're > changing the line nearby? I didn't changed this line, this seems unrelated to the patch purpose.
On Wed, 15 Jan 2020, Philippe Mathieu-Daudé wrote: > On 1/15/20 7:15 PM, BALATON Zoltan wrote: >> On Wed, 15 Jan 2020, Igor Mammedov wrote: >>> From: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> >>> The firmware has to reside in the PDC range. If the Elf file >>> expects to load it below FIRMWARE_START, it is incorrect, >>> regardless the RAM size. >>> >>> Acked-by: Helge Deller <deller@gmx.de> >>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >>> --- >>> hw/hppa/machine.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >>> index 5d0de26..6775d87 100644 >>> --- a/hw/hppa/machine.c >>> +++ b/hw/hppa/machine.c >>> @@ -155,7 +155,7 @@ static void machine_hppa_init(MachineState *machine) >>> qemu_log_mask(CPU_LOG_PAGE, "Firmware loaded at 0x%08" PRIx64 >>> "-0x%08" PRIx64 ", entry at 0x%08" PRIx64 ".\n", >>> firmware_low, firmware_high, firmware_entry); >>> - if (firmware_low < ram_size || firmware_high >= FIRMWARE_END) { >>> + if (firmware_low < FIRMWARE_START || firmware_high >= FIRMWARE_END) { >>> error_report("Firmware overlaps with memory or IO space"); >>> exit(1); >> >> Should this also be EXIT_FAILURE like in other places when you're changing >> the line nearby? > > I didn't changed this line, this seems unrelated to the patch purpose. Fair enough. Just thought because there was patch 85/86 making that change to keep consistency. Maybe you can change this in that patch but I don't really mind just spotted it. Regards, BALATON Zoltan
On 1/15/20 10:59 PM, BALATON Zoltan wrote: > On Wed, 15 Jan 2020, Philippe Mathieu-Daudé wrote: >> On 1/15/20 7:15 PM, BALATON Zoltan wrote: >>> On Wed, 15 Jan 2020, Igor Mammedov wrote: >>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org> >>>> >>>> The firmware has to reside in the PDC range. If the Elf file >>>> expects to load it below FIRMWARE_START, it is incorrect, >>>> regardless the RAM size. >>>> >>>> Acked-by: Helge Deller <deller@gmx.de> >>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >>>> --- >>>> hw/hppa/machine.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c >>>> index 5d0de26..6775d87 100644 >>>> --- a/hw/hppa/machine.c >>>> +++ b/hw/hppa/machine.c >>>> @@ -155,7 +155,7 @@ static void machine_hppa_init(MachineState >>>> *machine) >>>> qemu_log_mask(CPU_LOG_PAGE, "Firmware loaded at 0x%08" PRIx64 >>>> "-0x%08" PRIx64 ", entry at 0x%08" PRIx64 ".\n", >>>> firmware_low, firmware_high, firmware_entry); >>>> - if (firmware_low < ram_size || firmware_high >= FIRMWARE_END) { >>>> + if (firmware_low < FIRMWARE_START || firmware_high >= >>>> FIRMWARE_END) { >>>> error_report("Firmware overlaps with memory or IO space"); >>>> exit(1); >>> >>> Should this also be EXIT_FAILURE like in other places when you're >>> changing the line nearby? >> >> I didn't changed this line, this seems unrelated to the patch purpose. > > Fair enough. Just thought because there was patch 85/86 making that > change to keep consistency. Maybe you can change this in that patch but > I don't really mind just spotted it. Ah this is because it is a patch of mine included in Igor series, and Igor uses EXIT_FAILURE in his other patches, OK now it makes sense. Maybe the EXIT_FAILURE can be done in a new series, calling 'sed' to update the full repository.
On Thu, 16 Jan 2020 16:14:45 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 1/15/20 10:59 PM, BALATON Zoltan wrote: > > On Wed, 15 Jan 2020, Philippe Mathieu-Daudé wrote: > >> On 1/15/20 7:15 PM, BALATON Zoltan wrote: > >>> On Wed, 15 Jan 2020, Igor Mammedov wrote: > >>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org> > >>>> > >>>> The firmware has to reside in the PDC range. If the Elf file > >>>> expects to load it below FIRMWARE_START, it is incorrect, > >>>> regardless the RAM size. > >>>> > >>>> Acked-by: Helge Deller <deller@gmx.de> > >>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > >>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > >>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> > >>>> --- > >>>> hw/hppa/machine.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c > >>>> index 5d0de26..6775d87 100644 > >>>> --- a/hw/hppa/machine.c > >>>> +++ b/hw/hppa/machine.c > >>>> @@ -155,7 +155,7 @@ static void machine_hppa_init(MachineState > >>>> *machine) > >>>> qemu_log_mask(CPU_LOG_PAGE, "Firmware loaded at 0x%08" PRIx64 > >>>> "-0x%08" PRIx64 ", entry at 0x%08" PRIx64 ".\n", > >>>> firmware_low, firmware_high, firmware_entry); > >>>> - if (firmware_low < ram_size || firmware_high >= FIRMWARE_END) { > >>>> + if (firmware_low < FIRMWARE_START || firmware_high >= > >>>> FIRMWARE_END) { > >>>> error_report("Firmware overlaps with memory or IO space"); > >>>> exit(1); > >>> > >>> Should this also be EXIT_FAILURE like in other places when you're > >>> changing the line nearby? > >> > >> I didn't changed this line, this seems unrelated to the patch purpose. > > > > Fair enough. Just thought because there was patch 85/86 making that > > change to keep consistency. Maybe you can change this in that patch but > > I don't really mind just spotted it. > > Ah this is because it is a patch of mine included in Igor series, and > Igor uses EXIT_FAILURE in his other patches, OK now it makes sense. > > Maybe the EXIT_FAILURE can be done in a new series, calling 'sed' to > update the full repository. I'll fix it up on respin >
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c index 5d0de26..6775d87 100644 --- a/hw/hppa/machine.c +++ b/hw/hppa/machine.c @@ -155,7 +155,7 @@ static void machine_hppa_init(MachineState *machine) qemu_log_mask(CPU_LOG_PAGE, "Firmware loaded at 0x%08" PRIx64 "-0x%08" PRIx64 ", entry at 0x%08" PRIx64 ".\n", firmware_low, firmware_high, firmware_entry); - if (firmware_low < ram_size || firmware_high >= FIRMWARE_END) { + if (firmware_low < FIRMWARE_START || firmware_high >= FIRMWARE_END) { error_report("Firmware overlaps with memory or IO space"); exit(1); }