[PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[BALATON Zoltan]

The machine uses a modified U-Boot under GPL license but the sources
of it are lost with only a binary available so it cannot be included
in QEMU. Allow running without the firmware image with -bios none
which can be used when calling a boot loader directly and thus
simplifying booting guests. We need a small routine that AmigaOS calls
from ROM which is added in this case to allow booting AmigaOS without
external firmware image.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
v2: Unfortunately AmigaOS needs some additional ROM part which is added
Please merge for 8.2 as it allows booting AmigaOS simpler without
having to download separate firmware.

 hw/ppc/amigaone.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 992a55e632..a11d2d5556 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -40,6 +40,16 @@
 #define PROM_ADDR 0xfff00000
 #define PROM_SIZE (512 * KiB)
 
+/* AmigaOS calls this routine from ROM, use this if -bios none */
+static const char dummy_fw[] = {
+    0x38, 0x00, 0x00, 0x08, /* li      r0,8 */
+    0x7c, 0x09, 0x03, 0xa6, /* mtctr   r0 */
+    0x54, 0x63, 0xf8, 0x7e, /* srwi    r3,r3,1 */
+    0x42, 0x00, 0xff, 0xfc, /* bdnz    0x8 */
+    0x7c, 0x63, 0x18, 0xf8, /* not     r3,r3 */
+    0x4e, 0x80, 0x00, 0x20, /* blr */
+};
+
 static void amigaone_cpu_reset(void *opaque)
 {
     PowerPCCPU *cpu = opaque;
@@ -94,17 +104,21 @@ static void amigaone_init(MachineState *machine)
     }
 
     /* allocate and load firmware */
+    rom = g_new(MemoryRegion, 1);
+    memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
+    memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, fwname);
     if (filename) {
-        rom = g_new(MemoryRegion, 1);
-        memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
-        memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
         sz = load_image_targphys(filename, PROM_ADDR, PROM_SIZE);
         if (sz <= 0 || sz > PROM_SIZE) {
             error_report("Could not load firmware '%s'", filename);
             exit(1);
         }
         g_free(filename);
+    } else if (!strcmp(fwname, "none")) {
+        address_space_write_rom(&address_space_memory, 0xfff7ff80,
+                                MEMTXATTRS_UNSPECIFIED, dummy_fw,
+                                ARRAY_SIZE(dummy_fw));
     } else if (!qtest_enabled()) {
         error_report("Could not find firmware '%s'", fwname);
         exit(1);
-- 
2.30.9

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[Nicholas Piggin]

On Sun Nov 26, 2023 at 2:34 AM AEST, BALATON Zoltan wrote:
> The machine uses a modified U-Boot under GPL license but the sources
> of it are lost with only a binary available so it cannot be included
> in QEMU. Allow running without the firmware image with -bios none
> which can be used when calling a boot loader directly and thus
> simplifying booting guests. We need a small routine that AmigaOS calls
> from ROM which is added in this case to allow booting AmigaOS without
> external firmware image.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> v2: Unfortunately AmigaOS needs some additional ROM part which is added
> Please merge for 8.2 as it allows booting AmigaOS simpler without
> having to download separate firmware.

How to test this?

>
>  hw/ppc/amigaone.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
> index 992a55e632..a11d2d5556 100644
> --- a/hw/ppc/amigaone.c
> +++ b/hw/ppc/amigaone.c
> @@ -40,6 +40,16 @@
>  #define PROM_ADDR 0xfff00000
>  #define PROM_SIZE (512 * KiB)
>  
> +/* AmigaOS calls this routine from ROM, use this if -bios none */
> +static const char dummy_fw[] = {
> +    0x38, 0x00, 0x00, 0x08, /* li      r0,8 */
> +    0x7c, 0x09, 0x03, 0xa6, /* mtctr   r0 */
> +    0x54, 0x63, 0xf8, 0x7e, /* srwi    r3,r3,1 */
> +    0x42, 0x00, 0xff, 0xfc, /* bdnz    0x8 */
> +    0x7c, 0x63, 0x18, 0xf8, /* not     r3,r3 */
> +    0x4e, 0x80, 0x00, 0x20, /* blr */
> +};

This is clever, but does anything else create blobs like this?
It could be put into a .S in pc-bios, which might be a bit more
consistent.

We might make a ppc/ subdirectory under there, but that's for
another time.

Thanks,
Nick

> +
>  static void amigaone_cpu_reset(void *opaque)
>  {
>      PowerPCCPU *cpu = opaque;
> @@ -94,17 +104,21 @@ static void amigaone_init(MachineState *machine)
>      }
>  
>      /* allocate and load firmware */
> +    rom = g_new(MemoryRegion, 1);
> +    memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
> +    memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, fwname);
>      if (filename) {
> -        rom = g_new(MemoryRegion, 1);
> -        memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
> -        memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
>          sz = load_image_targphys(filename, PROM_ADDR, PROM_SIZE);
>          if (sz <= 0 || sz > PROM_SIZE) {
>              error_report("Could not load firmware '%s'", filename);
>              exit(1);
>          }
>          g_free(filename);
> +    } else if (!strcmp(fwname, "none")) {
> +        address_space_write_rom(&address_space_memory, 0xfff7ff80,
> +                                MEMTXATTRS_UNSPECIFIED, dummy_fw,
> +                                ARRAY_SIZE(dummy_fw));
>      } else if (!qtest_enabled()) {
>          error_report("Could not find firmware '%s'", fwname);
>          exit(1);

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[BALATON Zoltan]

On Mon, 27 Nov 2023, Nicholas Piggin wrote:
> On Sun Nov 26, 2023 at 2:34 AM AEST, BALATON Zoltan wrote:
>> The machine uses a modified U-Boot under GPL license but the sources
>> of it are lost with only a binary available so it cannot be included
>> in QEMU. Allow running without the firmware image with -bios none
>> which can be used when calling a boot loader directly and thus
>> simplifying booting guests. We need a small routine that AmigaOS calls
>> from ROM which is added in this case to allow booting AmigaOS without
>> external firmware image.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> v2: Unfortunately AmigaOS needs some additional ROM part which is added
>> Please merge for 8.2 as it allows booting AmigaOS simpler without
>> having to download separate firmware.
>
> How to test this?

You can check with -M amigaone -bios none then from QEMU monitor
(qemu) xp/10i 0xfff7ff80

>>  hw/ppc/amigaone.c | 20 +++++++++++++++++---
>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>> index 992a55e632..a11d2d5556 100644
>> --- a/hw/ppc/amigaone.c
>> +++ b/hw/ppc/amigaone.c
>> @@ -40,6 +40,16 @@
>>  #define PROM_ADDR 0xfff00000
>>  #define PROM_SIZE (512 * KiB)
>>
>> +/* AmigaOS calls this routine from ROM, use this if -bios none */
>> +static const char dummy_fw[] = {
>> +    0x38, 0x00, 0x00, 0x08, /* li      r0,8 */
>> +    0x7c, 0x09, 0x03, 0xa6, /* mtctr   r0 */
>> +    0x54, 0x63, 0xf8, 0x7e, /* srwi    r3,r3,1 */
>> +    0x42, 0x00, 0xff, 0xfc, /* bdnz    0x8 */
>> +    0x7c, 0x63, 0x18, 0xf8, /* not     r3,r3 */
>> +    0x4e, 0x80, 0x00, 0x20, /* blr */
>> +};
>
> This is clever, but does anything else create blobs like this?

There are some examples in hw/mips/{loongson3_virt.c, malta.c} at least 
and maybe others that put code in guest memory. If this was longer than 
this few instructions I'd consider putting it in a binary but this seems 
simpler for such small code.

> It could be put into a .S in pc-bios, which might be a bit more
> consistent.
>
> We might make a ppc/ subdirectory under there, but that's for
> another time.

Maybe later we could reorganise these unless it's really necessary to 
change this for 8.2 now. (The mips boards and some arm and riscv machines 
seem to use rom_add_blob_fixed() which sould show up in info roms under 
monitor so maybe I could look at changing to use that now if you think it 
would be better that way.)

Regards,
BALATON Zoltan

> Thanks,
> Nick
>
>> +
>>  static void amigaone_cpu_reset(void *opaque)
>>  {
>>      PowerPCCPU *cpu = opaque;
>> @@ -94,17 +104,21 @@ static void amigaone_init(MachineState *machine)
>>      }
>>
>>      /* allocate and load firmware */
>> +    rom = g_new(MemoryRegion, 1);
>> +    memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
>> +    memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
>>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, fwname);
>>      if (filename) {
>> -        rom = g_new(MemoryRegion, 1);
>> -        memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
>> -        memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
>>          sz = load_image_targphys(filename, PROM_ADDR, PROM_SIZE);
>>          if (sz <= 0 || sz > PROM_SIZE) {
>>              error_report("Could not load firmware '%s'", filename);
>>              exit(1);
>>          }
>>          g_free(filename);
>> +    } else if (!strcmp(fwname, "none")) {
>> +        address_space_write_rom(&address_space_memory, 0xfff7ff80,
>> +                                MEMTXATTRS_UNSPECIFIED, dummy_fw,
>> +                                ARRAY_SIZE(dummy_fw));
>>      } else if (!qtest_enabled()) {
>>          error_report("Could not find firmware '%s'", fwname);
>>          exit(1);
>
>
>

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[Nicholas Piggin]

On Mon Nov 27, 2023 at 9:43 PM AEST, BALATON Zoltan wrote:
> On Mon, 27 Nov 2023, Nicholas Piggin wrote:
> > On Sun Nov 26, 2023 at 2:34 AM AEST, BALATON Zoltan wrote:
> >> The machine uses a modified U-Boot under GPL license but the sources
> >> of it are lost with only a binary available so it cannot be included
> >> in QEMU. Allow running without the firmware image with -bios none
> >> which can be used when calling a boot loader directly and thus
> >> simplifying booting guests. We need a small routine that AmigaOS calls
> >> from ROM which is added in this case to allow booting AmigaOS without
> >> external firmware image.
> >>
> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >> ---
> >> v2: Unfortunately AmigaOS needs some additional ROM part which is added
> >> Please merge for 8.2 as it allows booting AmigaOS simpler without
> >> having to download separate firmware.
> >
> > How to test this?
>
> You can check with -M amigaone -bios none then from QEMU monitor
> (qemu) xp/10i 0xfff7ff80

Okay, so to fully test it you really need a non-free AmigaOS image?

And, how does a user know how or when to use this? Should it just be
default if there is no bios binary found?

>
> >>  hw/ppc/amigaone.c | 20 +++++++++++++++++---
> >>  1 file changed, 17 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
> >> index 992a55e632..a11d2d5556 100644
> >> --- a/hw/ppc/amigaone.c
> >> +++ b/hw/ppc/amigaone.c
> >> @@ -40,6 +40,16 @@
> >>  #define PROM_ADDR 0xfff00000
> >>  #define PROM_SIZE (512 * KiB)
> >>
> >> +/* AmigaOS calls this routine from ROM, use this if -bios none */
> >> +static const char dummy_fw[] = {
> >> +    0x38, 0x00, 0x00, 0x08, /* li      r0,8 */
> >> +    0x7c, 0x09, 0x03, 0xa6, /* mtctr   r0 */
> >> +    0x54, 0x63, 0xf8, 0x7e, /* srwi    r3,r3,1 */
> >> +    0x42, 0x00, 0xff, 0xfc, /* bdnz    0x8 */
> >> +    0x7c, 0x63, 0x18, 0xf8, /* not     r3,r3 */
> >> +    0x4e, 0x80, 0x00, 0x20, /* blr */
> >> +};
> >
> > This is clever, but does anything else create blobs like this?
>
> There are some examples in hw/mips/{loongson3_virt.c, malta.c} at least 
> and maybe others that put code in guest memory. If this was longer than 
> this few instructions I'd consider putting it in a binary but this seems 
> simpler for such small code.

Thanks, compiling blob inline looks fine then.

It's 0x80 bytes from the end of prom AFAIKS. Should you make that
PROM_ADDR + PROM_SIZE - 0x80 instead of hard coding it?

>
> > It could be put into a .S in pc-bios, which might be a bit more
> > consistent.
> >
> > We might make a ppc/ subdirectory under there, but that's for
> > another time.
>
> Maybe later we could reorganise these unless it's really necessary to 
> change this for 8.2 now.

Nah that's fine.

> (The mips boards and some arm and riscv machines 
> seem to use rom_add_blob_fixed() which sould show up in info roms under 
> monitor so maybe I could look at changing to use that now if you think it 
> would be better that way.)

I'm not sure, I don't think it's necessary if your minimal patch works.

I'll do a PR for 8.2 for SLOF and Skiboot updates, so happy to include
this as well.

Thanks,
Nick

>
> Regards,
> BALATON Zoltan
>
> > Thanks,
> > Nick
> >
> >> +
> >>  static void amigaone_cpu_reset(void *opaque)
> >>  {
> >>      PowerPCCPU *cpu = opaque;
> >> @@ -94,17 +104,21 @@ static void amigaone_init(MachineState *machine)
> >>      }
> >>
> >>      /* allocate and load firmware */
> >> +    rom = g_new(MemoryRegion, 1);
> >> +    memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
> >> +    memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
> >>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, fwname);
> >>      if (filename) {
> >> -        rom = g_new(MemoryRegion, 1);
> >> -        memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
> >> -        memory_region_add_subregion(get_system_memory(), PROM_ADDR, rom);
> >>          sz = load_image_targphys(filename, PROM_ADDR, PROM_SIZE);
> >>          if (sz <= 0 || sz > PROM_SIZE) {
> >>              error_report("Could not load firmware '%s'", filename);
> >>              exit(1);
> >>          }
> >>          g_free(filename);
> >> +    } else if (!strcmp(fwname, "none")) {
> >> +        address_space_write_rom(&address_space_memory, 0xfff7ff80,
> >> +                                MEMTXATTRS_UNSPECIFIED, dummy_fw,
> >> +                                ARRAY_SIZE(dummy_fw));
> >>      } else if (!qtest_enabled()) {
> >>          error_report("Could not find firmware '%s'", fwname);
> >>          exit(1);
> >
> >
> >

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[BALATON Zoltan]

On Mon, 27 Nov 2023, Nicholas Piggin wrote:
> On Mon Nov 27, 2023 at 9:43 PM AEST, BALATON Zoltan wrote:
>> On Mon, 27 Nov 2023, Nicholas Piggin wrote:
>>> On Sun Nov 26, 2023 at 2:34 AM AEST, BALATON Zoltan wrote:
>>>> The machine uses a modified U-Boot under GPL license but the sources
>>>> of it are lost with only a binary available so it cannot be included
>>>> in QEMU. Allow running without the firmware image with -bios none
>>>> which can be used when calling a boot loader directly and thus
>>>> simplifying booting guests. We need a small routine that AmigaOS calls
>>>> from ROM which is added in this case to allow booting AmigaOS without
>>>> external firmware image.
>>>>
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> ---
>>>> v2: Unfortunately AmigaOS needs some additional ROM part which is added
>>>> Please merge for 8.2 as it allows booting AmigaOS simpler without
>>>> having to download separate firmware.
>>>
>>> How to test this?
>>
>> You can check with -M amigaone -bios none then from QEMU monitor
>> (qemu) xp/10i 0xfff7ff80
>
> Okay, so to fully test it you really need a non-free AmigaOS image?

I'm afraid yes, it's hard to test without AmigaOS otherwise as that's the 
only thing that seems to need this.

> And, how does a user know how or when to use this? Should it just be
> default if there is no bios binary found?

It could be the default without -bios, that could also allow removing the 
qtest special case than. Maybe it's easier for users so I'll change that 
in v2.

>>>>  hw/ppc/amigaone.c | 20 +++++++++++++++++---
>>>>  1 file changed, 17 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>>>> index 992a55e632..a11d2d5556 100644
>>>> --- a/hw/ppc/amigaone.c
>>>> +++ b/hw/ppc/amigaone.c
>>>> @@ -40,6 +40,16 @@
>>>>  #define PROM_ADDR 0xfff00000
>>>>  #define PROM_SIZE (512 * KiB)
>>>>
>>>> +/* AmigaOS calls this routine from ROM, use this if -bios none */
>>>> +static const char dummy_fw[] = {
>>>> +    0x38, 0x00, 0x00, 0x08, /* li      r0,8 */
>>>> +    0x7c, 0x09, 0x03, 0xa6, /* mtctr   r0 */
>>>> +    0x54, 0x63, 0xf8, 0x7e, /* srwi    r3,r3,1 */
>>>> +    0x42, 0x00, 0xff, 0xfc, /* bdnz    0x8 */
>>>> +    0x7c, 0x63, 0x18, 0xf8, /* not     r3,r3 */
>>>> +    0x4e, 0x80, 0x00, 0x20, /* blr */
>>>> +};
>>>
>>> This is clever, but does anything else create blobs like this?
>>
>> There are some examples in hw/mips/{loongson3_virt.c, malta.c} at least
>> and maybe others that put code in guest memory. If this was longer than
>> this few instructions I'd consider putting it in a binary but this seems
>> simpler for such small code.
>
> Thanks, compiling blob inline looks fine then.
>
> It's 0x80 bytes from the end of prom AFAIKS. Should you make that
> PROM_ADDR + PROM_SIZE - 0x80 instead of hard coding it?

I thought about that after sending the patch, I'll change it too.

>>> It could be put into a .S in pc-bios, which might be a bit more
>>> consistent.
>>>
>>> We might make a ppc/ subdirectory under there, but that's for
>>> another time.
>>
>> Maybe later we could reorganise these unless it's really necessary to
>> change this for 8.2 now.
>
> Nah that's fine.
>
>> (The mips boards and some arm and riscv machines
>> seem to use rom_add_blob_fixed() which sould show up in info roms under
>> monitor so maybe I could look at changing to use that now if you think it
>> would be better that way.)
>
> I'm not sure, I don't think it's necessary if your minimal patch works.

It works but just for consistency with other similar machines I'll 
consider trying rom_add_blob_fixed() now that there will be another 
iteration.

> I'll do a PR for 8.2 for SLOF and Skiboot updates, so happy to include
> this as well.

Thanks, I'll only have time to do a v2 later but still before the next rc 
due tomorrow.

There are some more data around this function in ROM that I'm not sure 
would be needed but don't know how to verify what AmigaOS accesses. I've 
tried enabling memory_region* traces but those only seem to log io 
regions, ram and rom region accesses are probably just memory reads so not 
traced. Then I've tried adding watch points in gdb like this:

(gdb) watch *(char [0x80000])*0xfff00000
value requires 524288 bytes, which is more than max-value-size
(gdb) show max-value-size
Maximum value size is 65536 bytes.
(gdb) watch *(char [0x10000])*0xfff00000
Hardware watchpoint 1: *(char [0x10000])*0xfff00000
(gdb) watch *(char [0x10000])*0xfff10000
Hardware watchpoint 2: *(char [0x10000])*0xfff10000
(gdb) watch *(char [0x10000])*0xfff20000
Hardware watchpoint 3: *(char [0x10000])*0xfff20000
(gdb) watch *(char [0x10000])*0xfff30000
Hardware watchpoint 4: *(char [0x10000])*0xfff30000
(gdb) watch *(char [0x10000])*0xfff40000
Hardware watchpoint 5: *(char [0x10000])*0xfff40000
(gdb) watch *(char [0x10000])*0xfff50000
Hardware watchpoint 6: *(char [0x10000])*0xfff50000
(gdb) watch *(char [0x10000])*0xfff60000
Hardware watchpoint 7: *(char [0x10000])*0xfff60000
(gdb) watch *(char [0x10000])*0xfff70000
Hardware watchpoint 8: *(char [0x10000])*0xfff70000

but they are not firing even for the code known to be accessed so not sure 
how to trace rom access. Maybe I'll need to temporarily make it an io 
region to be able to trace access to it. Any better idea for that? That 
way I could check that no other parts of the rom are needed. With just 
this routine it boots but wanted to make sure nothing else is needed 
later.

Regards,
BALATON Zoltan

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[Cédric Le Goater]

> I'm not sure, I don't think it's necessary if your minimal patch works.
> 
> I'll do a PR for 8.2 for SLOF and Skiboot updates, so happy to include
> this as well.

I think this is a bit late for 8.2 to change FW images, well, at least
SLOF and skiboot. Are the new versions fixing something critical ?

Thanks,

C.

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[Nicholas Piggin]

On Tue Nov 28, 2023 at 2:37 AM AEST, Cédric Le Goater wrote:
>
> > I'm not sure, I don't think it's necessary if your minimal patch works.
> > 
> > I'll do a PR for 8.2 for SLOF and Skiboot updates, so happy to include
> > this as well.
>
> I think this is a bit late for 8.2 to change FW images, well, at least
> SLOF and skiboot. Are the new versions fixing something critical ?

Ah okay. Well then I can put them in next instead.

SLOF has a fix for virtio console over reboots, pretty minimal.
skiboot has some bug fixes but it's a bigger change and maybe not
so important for QEMU.

Could they be merged in next release and SLOF tagged with stable?

I think this amigaone patch could still be merged since it's only
touching a new machine and it's fixing an issue of missing firmware.

Thanks,
Nick

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[Cédric Le Goater]

On 11/28/23 02:47, Nicholas Piggin wrote:
> On Tue Nov 28, 2023 at 2:37 AM AEST, Cédric Le Goater wrote:
>>
>>> I'm not sure, I don't think it's necessary if your minimal patch works.
>>>
>>> I'll do a PR for 8.2 for SLOF and Skiboot updates, so happy to include
>>> this as well.
>>
>> I think this is a bit late for 8.2 to change FW images, well, at least
>> SLOF and skiboot. Are the new versions fixing something critical ?
> 
> Ah okay. Well then I can put them in next instead.
> 
> SLOF has a fix for virtio console over reboots, pretty minimal.

I see that commit dd4d4ea0add9 has :

   Fixes: cf28264 ("virtio-serial: Rework shutdown sequence")

Looks good for 8.2

> skiboot has some bug fixes but it's a bigger change and maybe not
> so important for QEMU.> Could they be merged in next release 

yes. it seems skiboot should be merged with chiptod support in 9.0.

> and SLOF tagged with stable?
>
> I think this amigaone patch could still be merged since it's only
> touching a new machine and it's fixing an issue of missing firmware.

ARM does something similar with roms. See hw/arm/boot.c file.

It will need a "Fixes" tag.

Thanks,

C.

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[Philippe Mathieu-Daudé]

On 28/11/23 08:07, Cédric Le Goater wrote:
> On 11/28/23 02:47, Nicholas Piggin wrote:
>> On Tue Nov 28, 2023 at 2:37 AM AEST, Cédric Le Goater wrote:
>>>
>>>> I'm not sure, I don't think it's necessary if your minimal patch works.
>>>>
>>>> I'll do a PR for 8.2 for SLOF and Skiboot updates, so happy to include
>>>> this as well.


>> I think this amigaone patch could still be merged since it's only
>> touching a new machine and it's fixing an issue of missing firmware.
> 
> ARM does something similar with roms. See hw/arm/boot.c file.
> 
> It will need a "Fixes" tag.

Fixes: d9656f860a ("hw/ppc: Add emulation of AmigaOne XE board")

Re: [PATCH v2 for-8.2] ppc/amigaone: Allow running AmigaOS without firmware image

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On Tue, 28 Nov 2023, Cédric Le Goater wrote:
> On 11/28/23 02:47, Nicholas Piggin wrote:
>> On Tue Nov 28, 2023 at 2:37 AM AEST, Cédric Le Goater wrote:
>>> 
>>>> I'm not sure, I don't think it's necessary if your minimal patch works.
>>>> 
>>>> I'll do a PR for 8.2 for SLOF and Skiboot updates, so happy to include
>>>> this as well.
>>> 
>>> I think this is a bit late for 8.2 to change FW images, well, at least
>>> SLOF and skiboot. Are the new versions fixing something critical ?
>> 
>> Ah okay. Well then I can put them in next instead.
>> 
>> SLOF has a fix for virtio console over reboots, pretty minimal.
>
> I see that commit dd4d4ea0add9 has :
>
>  Fixes: cf28264 ("virtio-serial: Rework shutdown sequence")
>
> Looks good for 8.2
>
>> skiboot has some bug fixes but it's a bigger change and maybe not
>> so important for QEMU.> Could they be merged in next release 
>
> yes. it seems skiboot should be merged with chiptod support in 9.0.
>
>> and SLOF tagged with stable?
>> 
>> I think this amigaone patch could still be merged since it's only
>> touching a new machine and it's fixing an issue of missing firmware.
>
> ARM does something similar with roms. See hw/arm/boot.c file.
>
> It will need a "Fixes" tag.

That would be

Fixes: d9656f860a38f83efc9710c515eab6a5b015134c

as the only commit for this machine so far was the one that added it.

Regards,
BALATON Zoltan