* [PATCH v9] mac_oldworld: Allow loading binary ROM image
@ 2020-10-17 15:47 BALATON Zoltan via
2020-10-17 16:05 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: BALATON Zoltan via @ 2020-10-17 15:47 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Mark Cave-Ayland
The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
the rom region and fall back to loading a binary image with -bios if
loading ELF image failed. This allows testing emulation with a ROM
image from real hardware as well as using an ELF OpenBIOS image.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
v9: Revert change from v8, back to the same as v7 rebased on latest
hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 05e46ee6fe..0117ae17f5 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -59,6 +59,8 @@
#define NDRV_VGA_FILENAME "qemu_vga.ndrv"
#define GRACKLE_BASE 0xfec00000
+#define PROM_BASE 0xffc00000
+#define PROM_SIZE (4 * MiB)
static void fw_cfg_boot_set(void *opaque, const char *boot_device,
Error **errp)
@@ -100,6 +102,7 @@ static void ppc_heathrow_init(MachineState *machine)
SysBusDevice *s;
DeviceState *dev, *pic_dev;
BusState *adb_bus;
+ uint64_t bios_addr;
int bios_size;
unsigned int smp_cpus = machine->smp.cpus;
uint16_t ppc_boot_device;
@@ -128,24 +131,32 @@ static void ppc_heathrow_init(MachineState *machine)
memory_region_add_subregion(sysmem, 0, machine->ram);
- /* allocate and load BIOS */
- memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
+ /* allocate and load firmware ROM */
+ memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
&error_fatal);
+ memory_region_add_subregion(sysmem, PROM_BASE, bios);
- if (bios_name == NULL)
+ if (!bios_name) {
bios_name = PROM_FILENAME;
+ }
filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
- memory_region_add_subregion(sysmem, PROM_ADDR, bios);
-
- /* Load OpenBIOS (ELF) */
if (filename) {
- bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL, NULL,
- 1, PPC_ELF_MACHINE, 0, 0);
+ /* Load OpenBIOS (ELF) */
+ bios_size = load_elf(filename, NULL, NULL, NULL, NULL, &bios_addr,
+ NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
+ /* Unfortunately, load_elf sign-extends reading elf32 */
+ bios_addr = (uint32_t)bios_addr;
+
+ if (bios_size <= 0) {
+ /* or load binary ROM image */
+ bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
+ bios_addr = PROM_BASE;
+ }
g_free(filename);
} else {
bios_size = -1;
}
- if (bios_size < 0 || bios_size > BIOS_SIZE) {
+ if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
error_report("could not load PowerPC bios '%s'", bios_name);
exit(1);
}
--
2.21.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v9] mac_oldworld: Allow loading binary ROM image
2020-10-17 15:47 [PATCH v9] mac_oldworld: Allow loading binary ROM image BALATON Zoltan via
@ 2020-10-17 16:05 ` Philippe Mathieu-Daudé
2020-10-17 16:31 ` BALATON Zoltan via
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-17 16:05 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc, Alistair Francis; +Cc: Mark Cave-Ayland
+Alistair for loader
On 10/17/20 5:47 PM, BALATON Zoltan via wrote:
> The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
> the rom region and fall back to loading a binary image with -bios if
> loading ELF image failed. This allows testing emulation with a ROM
> image from real hardware as well as using an ELF OpenBIOS image.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> v9: Revert change from v8, back to the same as v7 rebased on latest
>
> hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 05e46ee6fe..0117ae17f5 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -59,6 +59,8 @@
> #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
>
> #define GRACKLE_BASE 0xfec00000
> +#define PROM_BASE 0xffc00000
> +#define PROM_SIZE (4 * MiB)
>
> static void fw_cfg_boot_set(void *opaque, const char *boot_device,
> Error **errp)
> @@ -100,6 +102,7 @@ static void ppc_heathrow_init(MachineState *machine)
> SysBusDevice *s;
> DeviceState *dev, *pic_dev;
> BusState *adb_bus;
> + uint64_t bios_addr;
> int bios_size;
> unsigned int smp_cpus = machine->smp.cpus;
> uint16_t ppc_boot_device;
> @@ -128,24 +131,32 @@ static void ppc_heathrow_init(MachineState *machine)
>
> memory_region_add_subregion(sysmem, 0, machine->ram);
>
> - /* allocate and load BIOS */
> - memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
> + /* allocate and load firmware ROM */
> + memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
> &error_fatal);
> + memory_region_add_subregion(sysmem, PROM_BASE, bios);
>
> - if (bios_name == NULL)
> + if (!bios_name) {
> bios_name = PROM_FILENAME;
> + }
> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> - memory_region_add_subregion(sysmem, PROM_ADDR, bios);
> -
> - /* Load OpenBIOS (ELF) */
> if (filename) {
> - bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL, NULL,
> - 1, PPC_ELF_MACHINE, 0, 0);
> + /* Load OpenBIOS (ELF) */
> + bios_size = load_elf(filename, NULL, NULL, NULL, NULL, &bios_addr,
> + NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
> + /* Unfortunately, load_elf sign-extends reading elf32 */
Maybe this is what translate_fn() is for?
uint64_t oldworld_phys(void *opaque, uint64_t addr)
{
return addr & UINT32_MAX;
}
Using as (untested):
bios_size = load_elf(filename, NULL, oldworld_phys, NULL,
NULL, &bios_addr, NULL,
NULL, 1, PPC_ELF_MACHINE, 0, 0);
> + bios_addr = (uint32_t)bios_addr;
> +
> + if (bios_size <= 0) {
> + /* or load binary ROM image */
> + bios_size = load_image_targphys(filename, PROM_BASE, PROM_SIZE);
> + bios_addr = PROM_BASE;
> + }
> g_free(filename);
> } else {
> bios_size = -1;
> }
> - if (bios_size < 0 || bios_size > BIOS_SIZE) {
> + if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
> error_report("could not load PowerPC bios '%s'", bios_name);
> exit(1);
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v9] mac_oldworld: Allow loading binary ROM image
2020-10-17 16:05 ` Philippe Mathieu-Daudé
@ 2020-10-17 16:31 ` BALATON Zoltan via
2020-10-17 17:38 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: BALATON Zoltan via @ 2020-10-17 16:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Alistair Francis, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4457 bytes --]
On Sat, 17 Oct 2020, Philippe Mathieu-Daudé wrote:
> +Alistair for loader
>
> On 10/17/20 5:47 PM, BALATON Zoltan via wrote:
>> The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
>> the rom region and fall back to loading a binary image with -bios if
>> loading ELF image failed. This allows testing emulation with a ROM
>> image from real hardware as well as using an ELF OpenBIOS image.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> v9: Revert change from v8, back to the same as v7 rebased on latest
>>
>> hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
>> 1 file changed, 20 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>> index 05e46ee6fe..0117ae17f5 100644
>> --- a/hw/ppc/mac_oldworld.c
>> +++ b/hw/ppc/mac_oldworld.c
>> @@ -59,6 +59,8 @@
>> #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
>> #define GRACKLE_BASE 0xfec00000
>> +#define PROM_BASE 0xffc00000
>> +#define PROM_SIZE (4 * MiB)
>> static void fw_cfg_boot_set(void *opaque, const char *boot_device,
>> Error **errp)
>> @@ -100,6 +102,7 @@ static void ppc_heathrow_init(MachineState *machine)
>> SysBusDevice *s;
>> DeviceState *dev, *pic_dev;
>> BusState *adb_bus;
>> + uint64_t bios_addr;
>> int bios_size;
>> unsigned int smp_cpus = machine->smp.cpus;
>> uint16_t ppc_boot_device;
>> @@ -128,24 +131,32 @@ static void ppc_heathrow_init(MachineState *machine)
>> memory_region_add_subregion(sysmem, 0, machine->ram);
>> - /* allocate and load BIOS */
>> - memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
>> + /* allocate and load firmware ROM */
>> + memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
>> &error_fatal);
>> + memory_region_add_subregion(sysmem, PROM_BASE, bios);
>> - if (bios_name == NULL)
>> + if (!bios_name) {
>> bios_name = PROM_FILENAME;
>> + }
>> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>> - memory_region_add_subregion(sysmem, PROM_ADDR, bios);
>> -
>> - /* Load OpenBIOS (ELF) */
>> if (filename) {
>> - bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL,
>> NULL,
>> - 1, PPC_ELF_MACHINE, 0, 0);
>> + /* Load OpenBIOS (ELF) */
>> + bios_size = load_elf(filename, NULL, NULL, NULL, NULL, &bios_addr,
>> + NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
>> + /* Unfortunately, load_elf sign-extends reading elf32 */
>
> Maybe this is what translate_fn() is for?
>
> uint64_t oldworld_phys(void *opaque, uint64_t addr)
> {
> return addr & UINT32_MAX;
> }
>
> Using as (untested):
>
> bios_size = load_elf(filename, NULL, oldworld_phys, NULL,
> NULL, &bios_addr, NULL,
> NULL, 1, PPC_ELF_MACHINE, 0, 0);
Please don't come up with any more great ideas for this patch unless you
also propose a replacement and test it. This one works and we could just
get this in as it is until the real problem with load_elf is fixed at
which point all this can be removed so no need to be more sophisticated as
the simple cast I have. As you can see in the original discussion:
http://patchwork.ozlabs.org/project/qemu-devel/patch/c69a791c7cad1246f3f34b3993dee4f549b75aa2.1593456926.git.balaton@eik.bme.hu/
problem is really in include/hw/elf_ops.h this is just a work around for
that as I did not want to break anything I can't test so I'd rather fix it
up here and let you fix load_elf then drop this cast. But unless you can
do that before the freeze please don't hold up this patch any more.
Regards,
BALATON Zoltan
>> + bios_addr = (uint32_t)bios_addr;
>> +
>> + if (bios_size <= 0) {
>> + /* or load binary ROM image */
>> + bios_size = load_image_targphys(filename, PROM_BASE,
>> PROM_SIZE);
>> + bios_addr = PROM_BASE;
>> + }
>> g_free(filename);
>> } else {
>> bios_size = -1;
>> }
>> - if (bios_size < 0 || bios_size > BIOS_SIZE) {
>> + if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE) {
>> error_report("could not load PowerPC bios '%s'", bios_name);
>> exit(1);
>> }
>>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v9] mac_oldworld: Allow loading binary ROM image
2020-10-17 16:31 ` BALATON Zoltan via
@ 2020-10-17 17:38 ` Philippe Mathieu-Daudé
2020-10-17 21:57 ` BALATON Zoltan via
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-17 17:38 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: Alistair Francis, qemu-ppc, qemu-devel
On 10/17/20 6:31 PM, BALATON Zoltan via wrote:
> On Sat, 17 Oct 2020, Philippe Mathieu-Daudé wrote:
>> +Alistair for loader
>>
>> On 10/17/20 5:47 PM, BALATON Zoltan via wrote:
>>> The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
>>> the rom region and fall back to loading a binary image with -bios if
>>> loading ELF image failed. This allows testing emulation with a ROM
>>> image from real hardware as well as using an ELF OpenBIOS image.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>> v9: Revert change from v8, back to the same as v7 rebased on latest
>>>
>>> hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
>>> 1 file changed, 20 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>>> index 05e46ee6fe..0117ae17f5 100644
>>> --- a/hw/ppc/mac_oldworld.c
>>> +++ b/hw/ppc/mac_oldworld.c
>>> @@ -59,6 +59,8 @@
>>> #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
>>> #define GRACKLE_BASE 0xfec00000
>>> +#define PROM_BASE 0xffc00000
>>> +#define PROM_SIZE (4 * MiB)
>>> static void fw_cfg_boot_set(void *opaque, const char *boot_device,
>>> Error **errp)
>>> @@ -100,6 +102,7 @@ static void ppc_heathrow_init(MachineState *machine)
>>> SysBusDevice *s;
>>> DeviceState *dev, *pic_dev;
>>> BusState *adb_bus;
>>> + uint64_t bios_addr;
>>> int bios_size;
>>> unsigned int smp_cpus = machine->smp.cpus;
>>> uint16_t ppc_boot_device;
>>> @@ -128,24 +131,32 @@ static void ppc_heathrow_init(MachineState
>>> *machine)
>>> memory_region_add_subregion(sysmem, 0, machine->ram);
>>> - /* allocate and load BIOS */
>>> - memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
>>> + /* allocate and load firmware ROM */
>>> + memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
>>> &error_fatal);
>>> + memory_region_add_subregion(sysmem, PROM_BASE, bios);
>>> - if (bios_name == NULL)
>>> + if (!bios_name) {
>>> bios_name = PROM_FILENAME;
>>> + }
>>> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>> - memory_region_add_subregion(sysmem, PROM_ADDR, bios);
>>> -
>>> - /* Load OpenBIOS (ELF) */
>>> if (filename) {
>>> - bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL,
>>> NULL, NULL,
>>> - 1, PPC_ELF_MACHINE, 0, 0);
>>> + /* Load OpenBIOS (ELF) */
>>> + bios_size = load_elf(filename, NULL, NULL, NULL, NULL,
>>> &bios_addr,
>>> + NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
>>> + /* Unfortunately, load_elf sign-extends reading elf32 */
>>
>> Maybe this is what translate_fn() is for?
>>
>> uint64_t oldworld_phys(void *opaque, uint64_t addr)
>> {
>> return addr & UINT32_MAX;
>> }
>>
>> Using as (untested):
>>
>> bios_size = load_elf(filename, NULL, oldworld_phys, NULL,
>> NULL, &bios_addr, NULL,
>> NULL, 1, PPC_ELF_MACHINE, 0, 0);
>
> Please don't come up with any more great ideas for this patch unless you
> also propose a replacement and test it. This one works and we could just
> get this in as it is until the real problem with load_elf is fixed at
> which point all this can be removed so no need to be more sophisticated
> as the simple cast I have.
Zoltan, I'm not trying to block your patch to get merged,
I'm asking because I'm trying to understand how this API
is expected to be used.
> As you can see in the original discussion:
>
> http://patchwork.ozlabs.org/project/qemu-devel/patch/c69a791c7cad1246f3f34b3993dee4f549b75aa2.1593456926.git.balaton@eik.bme.hu/
>
>
> problem is really in include/hw/elf_ops.h this is just a work around for
> that as I did not want to break anything I can't test so I'd rather fix
> it up here and let you fix load_elf then drop this cast. But unless you
> can do that before the freeze please don't hold up this patch any more.
>
> Regards,
> BALATON Zoltan
>
>>> + bios_addr = (uint32_t)bios_addr;
>>> +
>>> + if (bios_size <= 0) {
>>> + /* or load binary ROM image */
>>> + bios_size = load_image_targphys(filename, PROM_BASE,
>>> PROM_SIZE);
>>> + bios_addr = PROM_BASE;
>>> + }
>>> g_free(filename);
>>> } else {
>>> bios_size = -1;
>>> }
>>> - if (bios_size < 0 || bios_size > BIOS_SIZE) {
>>> + if (bios_size < 0 || bios_addr - PROM_BASE + bios_size >
>>> PROM_SIZE) {
>>> error_report("could not load PowerPC bios '%s'", bios_name);
>>> exit(1);
>>> }
>>>
>>
>>
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v9] mac_oldworld: Allow loading binary ROM image
2020-10-17 17:38 ` Philippe Mathieu-Daudé
@ 2020-10-17 21:57 ` BALATON Zoltan via
0 siblings, 0 replies; 5+ messages in thread
From: BALATON Zoltan via @ 2020-10-17 21:57 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Alistair Francis, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5870 bytes --]
On Sat, 17 Oct 2020, Philippe Mathieu-Daudé wrote:
> On 10/17/20 6:31 PM, BALATON Zoltan via wrote:
>> On Sat, 17 Oct 2020, Philippe Mathieu-Daudé wrote:
>>> +Alistair for loader
>>>
>>> On 10/17/20 5:47 PM, BALATON Zoltan via wrote:
>>>> The beige G3 Power Macintosh has a 4MB firmware ROM. Fix the size of
>>>> the rom region and fall back to loading a binary image with -bios if
>>>> loading ELF image failed. This allows testing emulation with a ROM
>>>> image from real hardware as well as using an ELF OpenBIOS image.
>>>>
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>>> ---
>>>> v9: Revert change from v8, back to the same as v7 rebased on latest
>>>>
>>>> hw/ppc/mac_oldworld.c | 29 ++++++++++++++++++++---------
>>>> 1 file changed, 20 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
>>>> index 05e46ee6fe..0117ae17f5 100644
>>>> --- a/hw/ppc/mac_oldworld.c
>>>> +++ b/hw/ppc/mac_oldworld.c
>>>> @@ -59,6 +59,8 @@
>>>> #define NDRV_VGA_FILENAME "qemu_vga.ndrv"
>>>> #define GRACKLE_BASE 0xfec00000
>>>> +#define PROM_BASE 0xffc00000
>>>> +#define PROM_SIZE (4 * MiB)
>>>> static void fw_cfg_boot_set(void *opaque, const char *boot_device,
>>>> Error **errp)
>>>> @@ -100,6 +102,7 @@ static void ppc_heathrow_init(MachineState *machine)
>>>> SysBusDevice *s;
>>>> DeviceState *dev, *pic_dev;
>>>> BusState *adb_bus;
>>>> + uint64_t bios_addr;
>>>> int bios_size;
>>>> unsigned int smp_cpus = machine->smp.cpus;
>>>> uint16_t ppc_boot_device;
>>>> @@ -128,24 +131,32 @@ static void ppc_heathrow_init(MachineState
>>>> *machine)
>>>> memory_region_add_subregion(sysmem, 0, machine->ram);
>>>> - /* allocate and load BIOS */
>>>> - memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", BIOS_SIZE,
>>>> + /* allocate and load firmware ROM */
>>>> + memory_region_init_rom(bios, NULL, "ppc_heathrow.bios", PROM_SIZE,
>>>> &error_fatal);
>>>> + memory_region_add_subregion(sysmem, PROM_BASE, bios);
>>>> - if (bios_name == NULL)
>>>> + if (!bios_name) {
>>>> bios_name = PROM_FILENAME;
>>>> + }
>>>> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>>>> - memory_region_add_subregion(sysmem, PROM_ADDR, bios);
>>>> -
>>>> - /* Load OpenBIOS (ELF) */
>>>> if (filename) {
>>>> - bios_size = load_elf(filename, NULL, 0, NULL, NULL, NULL, NULL,
>>>> NULL,
>>>> - 1, PPC_ELF_MACHINE, 0, 0);
>>>> + /* Load OpenBIOS (ELF) */
>>>> + bios_size = load_elf(filename, NULL, NULL, NULL, NULL,
>>>> &bios_addr,
>>>> + NULL, NULL, 1, PPC_ELF_MACHINE, 0, 0);
>>>> + /* Unfortunately, load_elf sign-extends reading elf32 */
>>>
>>> Maybe this is what translate_fn() is for?
>>>
>>> uint64_t oldworld_phys(void *opaque, uint64_t addr)
>>> {
>>> return addr & UINT32_MAX;
>>> }
>>>
>>> Using as (untested):
>>>
>>> bios_size = load_elf(filename, NULL, oldworld_phys, NULL,
>>> NULL, &bios_addr, NULL,
>>> NULL, 1, PPC_ELF_MACHINE, 0, 0);
>>
>> Please don't come up with any more great ideas for this patch unless you
>> also propose a replacement and test it. This one works and we could just
>> get this in as it is until the real problem with load_elf is fixed at which
>> point all this can be removed so no need to be more sophisticated as the
>> simple cast I have.
>
> Zoltan, I'm not trying to block your patch to get merged,
I didn't say you're trying to do that but based on my past experience any
slightest doubt could result in that so I did not want it to miss another
freeze now that it's almost got in.
> I'm asking because I'm trying to understand how this API
> is expected to be used.
Likely nobody knows, this seems to have been evolved into this mess which
could be cleaned up but unrelated to this series. Probably nobody dared to
touch it so far as it's used by almost every board so breling something is
easy and testing it is difficult.
Regards,
BALATON Zoltan
>> As you can see in the original discussion:
>>
>> http://patchwork.ozlabs.org/project/qemu-devel/patch/c69a791c7cad1246f3f34b3993dee4f549b75aa2.1593456926.git.balaton@eik.bme.hu/
>>
>> problem is really in include/hw/elf_ops.h this is just a work around for
>> that as I did not want to break anything I can't test so I'd rather fix it
>> up here and let you fix load_elf then drop this cast. But unless you can do
>> that before the freeze please don't hold up this patch any more.
>>
>> Regards,
>> BALATON Zoltan
>>
>>>> + bios_addr = (uint32_t)bios_addr;
>>>> +
>>>> + if (bios_size <= 0) {
>>>> + /* or load binary ROM image */
>>>> + bios_size = load_image_targphys(filename, PROM_BASE,
>>>> PROM_SIZE);
>>>> + bios_addr = PROM_BASE;
>>>> + }
>>>> g_free(filename);
>>>> } else {
>>>> bios_size = -1;
>>>> }
>>>> - if (bios_size < 0 || bios_size > BIOS_SIZE) {
>>>> + if (bios_size < 0 || bios_addr - PROM_BASE + bios_size > PROM_SIZE)
>>>> {
>>>> error_report("could not load PowerPC bios '%s'", bios_name);
>>>> exit(1);
>>>> }
>>>>
>>>
>>>
>>>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-10-17 21:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-17 15:47 [PATCH v9] mac_oldworld: Allow loading binary ROM image BALATON Zoltan via
2020-10-17 16:05 ` Philippe Mathieu-Daudé
2020-10-17 16:31 ` BALATON Zoltan via
2020-10-17 17:38 ` Philippe Mathieu-Daudé
2020-10-17 21:57 ` BALATON Zoltan via
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).