* [PATCH v2 0/5] ppc/amigaone patches
@ 2025-02-27 16:39 BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 1/5] ppc/amigaone: Simplify replacement dummy_fw BALATON Zoltan
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: BALATON Zoltan @ 2025-02-27 16:39 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
Hello,
v2:
- change unused read function to g_assert_not_reached()
- new patch to add defines to constants
- added R-b tags
This series adds NVRAM and support for -kernel, -initrd and -append
options to the amigaone machine. This makes it easier to boot AmigaOS
and avoids a crash in the guest when it tries to access NVRAM.
While the -kernel option emulates what U-Boot passes to the kernel,
old Linux kernels for amigaone may not work with it because of two
reasons: these come in legacy U-Boot Multi-File image format that QEMU
cannot read and even after unpacking that and creating a kernel uimage
it won't find PCI devices because it does not initialise them
correctly. This works when booted from U-Boot because U-Boot inits PCI
devices. So does my BBoot loader which can be used to load AmigaOS so
I don't intend to emulate that part of U-Boot.
I'd like this to be merged for the next release please. When merging
please update https://wiki.qemu.org/ChangeLog/10.0 with the following:
amigaone
Added support for NVRAM and -kernel, -initrd, -append command line
options. By default the NVRAM contents are not preserved between
sessions. To make it persistent create a backing file with 'qemu-image
create -f raw nvram.bin 4k' and add -drive
if=mtd,format=raw,file=nvram.bin to keep NVRAM contents in the backing
file so settings stored in it will be preserved between sessions.
To run AmigaOS with BBoot using the -kernel option at least BBoot
version 0.8 is needed. Older BBoot versions only work with -device
loader and cannot be used with -kernel on amigaone.
Regards,
BALATON Zoltan (5):
ppc/amigaone: Simplify replacement dummy_fw
ppc/amigaone: Implement NVRAM emulation
ppc/amigaone: Add default environment
ppc/amigaone: Add kernel and initrd support
ppc/amigaone: Add #defines for memory map constants
hw/ppc/amigaone.c | 284 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 271 insertions(+), 13 deletions(-)
--
2.30.9
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/5] ppc/amigaone: Simplify replacement dummy_fw
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
@ 2025-02-27 16:39 ` BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 2/5] ppc/amigaone: Implement NVRAM emulation BALATON Zoltan
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: BALATON Zoltan @ 2025-02-27 16:39 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
There's no need to do shift in a loop, doing it in one instruction
works just as well, only the result is used.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ppc/amigaone.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index b02792221c..4290d58613 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -41,10 +41,7 @@
/* AmigaOS calls this routine from ROM, use this if no firmware loaded */
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 */
+ 0x54, 0x63, 0xc2, 0x3e, /* srwi r3,r3,8 */
0x7c, 0x63, 0x18, 0xf8, /* not r3,r3 */
0x4e, 0x80, 0x00, 0x20, /* blr */
};
--
2.30.9
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/5] ppc/amigaone: Implement NVRAM emulation
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 1/5] ppc/amigaone: Simplify replacement dummy_fw BALATON Zoltan
@ 2025-02-27 16:39 ` BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 3/5] ppc/amigaone: Add default environment BALATON Zoltan
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: BALATON Zoltan @ 2025-02-27 16:39 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
The board has a battery backed NVRAM where U-Boot environment is
stored which is also accessed by AmigaOS and e.g. C:NVGetVar command
crashes without it having at least a valid checksum.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/amigaone.c | 113 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 110 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 4290d58613..849c9fc6e0 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -21,10 +21,13 @@
#include "hw/ide/pci.h"
#include "hw/i2c/smbus_eeprom.h"
#include "hw/ppc/ppc.h"
+#include "system/block-backend.h"
#include "system/qtest.h"
#include "system/reset.h"
#include "kvm_ppc.h"
+#include <zlib.h> /* for crc32 */
+
#define BUS_FREQ_HZ 100000000
/*
@@ -46,6 +49,100 @@ static const char dummy_fw[] = {
0x4e, 0x80, 0x00, 0x20, /* blr */
};
+#define NVRAM_ADDR 0xfd0e0000
+#define NVRAM_SIZE (4 * KiB)
+
+#define TYPE_A1_NVRAM "a1-nvram"
+OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
+
+struct A1NVRAMState {
+ SysBusDevice parent_obj;
+
+ MemoryRegion mr;
+ BlockBackend *blk;
+};
+
+static uint64_t nvram_read(void *opaque, hwaddr addr, unsigned int size)
+{
+ /* read callback not used because of romd mode */
+ g_assert_not_reached();
+}
+
+static void nvram_write(void *opaque, hwaddr addr, uint64_t val,
+ unsigned int size)
+{
+ A1NVRAMState *s = opaque;
+ uint8_t *p = memory_region_get_ram_ptr(&s->mr);
+
+ p[addr] = val;
+ if (s->blk) {
+ blk_pwrite(s->blk, addr, 1, &val, 0);
+ }
+}
+
+static const MemoryRegionOps nvram_ops = {
+ .read = nvram_read,
+ .write = nvram_write,
+ .endianness = DEVICE_BIG_ENDIAN,
+ .impl = {
+ .min_access_size = 1,
+ .max_access_size = 1,
+ },
+};
+
+static void nvram_realize(DeviceState *dev, Error **errp)
+{
+ A1NVRAMState *s = A1_NVRAM(dev);
+ void *p;
+ uint32_t *c;
+
+ memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, "nvram",
+ NVRAM_SIZE, &error_fatal);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mr);
+ c = p = memory_region_get_ram_ptr(&s->mr);
+ if (s->blk) {
+ if (blk_getlength(s->blk) != NVRAM_SIZE) {
+ error_setg(errp, "NVRAM backing file size must be %ld bytes",
+ NVRAM_SIZE);
+ return;
+ }
+ blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
+ BLK_PERM_ALL, &error_fatal);
+ if (blk_pread(s->blk, 0, NVRAM_SIZE, p, 0) < 0) {
+ error_setg(errp, "Cannot read NVRAM contents from backing file");
+ return;
+ }
+ }
+ if (*c == 0) {
+ *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
+ if (s->blk) {
+ blk_pwrite(s->blk, 0, 4, p, 0);
+ }
+ }
+}
+
+static const Property nvram_properties[] = {
+ DEFINE_PROP_DRIVE("drive", A1NVRAMState, blk),
+};
+
+static void nvram_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ dc->realize = nvram_realize;
+ device_class_set_props(dc, nvram_properties);
+}
+
+static const TypeInfo nvram_types[] = {
+ {
+ .name = TYPE_A1_NVRAM,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(A1NVRAMState),
+ .class_init = nvram_class_init,
+ },
+};
+DEFINE_TYPES(nvram_types)
+
static void amigaone_cpu_reset(void *opaque)
{
PowerPCCPU *cpu = opaque;
@@ -72,7 +169,7 @@ static void amigaone_init(MachineState *machine)
DeviceState *dev;
I2CBus *i2c_bus;
uint8_t *spd_data;
- int i;
+ DriveInfo *di;
/* init CPU */
cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
@@ -97,6 +194,16 @@ static void amigaone_init(MachineState *machine)
memory_region_add_subregion(get_system_memory(), 0x40000000, mr);
}
+ /* nvram */
+ dev = qdev_new(TYPE_A1_NVRAM);
+ di = drive_get(IF_MTD, 0, 0);
+ if (di) {
+ qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(di));
+ }
+ sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+ memory_region_add_subregion(get_system_memory(), NVRAM_ADDR,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0));
+
/* allocate and load firmware */
rom = g_new(MemoryRegion, 1);
memory_region_init_rom(rom, NULL, "rom", PROM_SIZE, &error_fatal);
@@ -136,7 +243,7 @@ static void amigaone_init(MachineState *machine)
pci_mem = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
mr = g_new(MemoryRegion, 1);
memory_region_init_alias(mr, OBJECT(dev), "pci-mem-low", pci_mem,
- 0, 0x1000000);
+ 0, 0xe0000);
memory_region_add_subregion(get_system_memory(), 0xfd000000, mr);
mr = g_new(MemoryRegion, 1);
memory_region_init_alias(mr, OBJECT(dev), "pci-mem-high", pci_mem,
@@ -153,7 +260,7 @@ static void amigaone_init(MachineState *machine)
qdev_connect_gpio_out_named(DEVICE(via), "intr", 0,
qdev_get_gpio_in(DEVICE(cpu),
PPC6xx_INPUT_INT));
- for (i = 0; i < PCI_NUM_PINS; i++) {
+ for (int i = 0; i < PCI_NUM_PINS; i++) {
qdev_connect_gpio_out(dev, i, qdev_get_gpio_in_named(DEVICE(via),
"pirq", i));
}
--
2.30.9
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/5] ppc/amigaone: Add default environment
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 1/5] ppc/amigaone: Simplify replacement dummy_fw BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 2/5] ppc/amigaone: Implement NVRAM emulation BALATON Zoltan
@ 2025-02-27 16:39 ` BALATON Zoltan
2025-03-07 14:10 ` Philippe Mathieu-Daudé
2025-02-27 16:39 ` [PATCH v2 4/5] ppc/amigaone: Add kernel and initrd support BALATON Zoltan
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: BALATON Zoltan @ 2025-02-27 16:39 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
Initialise empty NVRAM with default values. This also enables IDE UDMA
mode in AmigaOS that is faster but has to be enabled in environment
due to problems with real hardware but that does not affect emulation
so we can use faster defaults here.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
1 file changed, 36 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 849c9fc6e0..5c5585d39a 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -52,6 +52,28 @@ static const char dummy_fw[] = {
#define NVRAM_ADDR 0xfd0e0000
#define NVRAM_SIZE (4 * KiB)
+static char default_env[] =
+ "baudrate=115200\0"
+ "stdout=vga\0"
+ "stdin=ps2kbd\0"
+ "bootcmd=boota; menu; run menuboot_cmd\0"
+ "boot1=ide\0"
+ "boot2=cdrom\0"
+ "boota_timeout=3\0"
+ "ide_doreset=on\0"
+ "pci_irqa=9\0"
+ "pci_irqa_select=level\0"
+ "pci_irqb=10\0"
+ "pci_irqb_select=level\0"
+ "pci_irqc=11\0"
+ "pci_irqc_select=level\0"
+ "pci_irqd=7\0"
+ "pci_irqd_select=level\0"
+ "a1ide_irq=1111\0"
+ "a1ide_xfer=FFFF\0";
+#define CRC32_DEFAULT_ENV 0xb5548481
+#define CRC32_ALL_ZEROS 0x603b0489
+
#define TYPE_A1_NVRAM "a1-nvram"
OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
@@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error **errp)
{
A1NVRAMState *s = A1_NVRAM(dev);
void *p;
- uint32_t *c;
+ uint32_t crc, *c;
memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, "nvram",
NVRAM_SIZE, &error_fatal);
@@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error **errp)
return;
}
}
+ crc = crc32(0, p + 4, NVRAM_SIZE - 4);
+ if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default */
+ *c = cpu_to_be32(CRC32_DEFAULT_ENV);
+ /* Also copies terminating \0 as env is terminated by \0\0 */
+ memcpy(p + 4, default_env, sizeof(default_env));
+ if (s->blk) {
+ blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p, 0);
+ }
+ return;
+ }
if (*c == 0) {
*c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
if (s->blk) {
blk_pwrite(s->blk, 0, 4, p, 0);
}
}
+ if (be32_to_cpu(*c) != crc) {
+ warn_report("NVRAM checksum mismatch");
+ }
}
static const Property nvram_properties[] = {
--
2.30.9
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/5] ppc/amigaone: Add kernel and initrd support
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
` (2 preceding siblings ...)
2025-02-27 16:39 ` [PATCH v2 3/5] ppc/amigaone: Add default environment BALATON Zoltan
@ 2025-02-27 16:39 ` BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 5/5] ppc/amigaone: Add #defines for memory map constants BALATON Zoltan
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: BALATON Zoltan @ 2025-02-27 16:39 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
Add support for -kernel, -initrd and -append command line options.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ppc/amigaone.c | 113 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 112 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 5c5585d39a..04ff415a6b 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -25,11 +25,14 @@
#include "system/qtest.h"
#include "system/reset.h"
#include "kvm_ppc.h"
+#include "elf.h"
#include <zlib.h> /* for crc32 */
#define BUS_FREQ_HZ 100000000
+#define INITRD_MIN_ADDR 0x600000
+
/*
* Firmware binary available at
* https://www.hyperion-entertainment.com/index.php/downloads?view=files&parent=28
@@ -178,12 +181,68 @@ static const TypeInfo nvram_types[] = {
};
DEFINE_TYPES(nvram_types)
+struct boot_info {
+ hwaddr entry;
+ hwaddr stack;
+ hwaddr bd_info;
+ hwaddr initrd_start;
+ hwaddr initrd_end;
+ hwaddr cmdline_start;
+ hwaddr cmdline_end;
+};
+
+/* Board info struct from U-Boot */
+struct bd_info {
+ uint32_t bi_memstart;
+ uint32_t bi_memsize;
+ uint32_t bi_flashstart;
+ uint32_t bi_flashsize;
+ uint32_t bi_flashoffset;
+ uint32_t bi_sramstart;
+ uint32_t bi_sramsize;
+ uint32_t bi_bootflags;
+ uint32_t bi_ip_addr;
+ uint8_t bi_enetaddr[6];
+ uint16_t bi_ethspeed;
+ uint32_t bi_intfreq;
+ uint32_t bi_busfreq;
+ uint32_t bi_baudrate;
+} QEMU_PACKED;
+
+static void create_bd_info(hwaddr addr, ram_addr_t ram_size)
+{
+ struct bd_info *bd = g_new0(struct bd_info, 1);
+
+ bd->bi_memsize = cpu_to_be32(ram_size);
+ bd->bi_flashstart = cpu_to_be32(PROM_ADDR);
+ bd->bi_flashsize = cpu_to_be32(1); /* match what U-Boot detects */
+ bd->bi_bootflags = cpu_to_be32(1);
+ bd->bi_intfreq = cpu_to_be32(11.5 * BUS_FREQ_HZ);
+ bd->bi_busfreq = cpu_to_be32(BUS_FREQ_HZ);
+ bd->bi_baudrate = cpu_to_be32(115200);
+
+ cpu_physical_memory_write(addr, bd, sizeof(*bd));
+}
+
static void amigaone_cpu_reset(void *opaque)
{
PowerPCCPU *cpu = opaque;
+ CPUPPCState *env = &cpu->env;
cpu_reset(CPU(cpu));
- cpu_ppc_tb_reset(&cpu->env);
+ if (env->load_info) {
+ struct boot_info *bi = env->load_info;
+
+ env->gpr[1] = bi->stack;
+ env->gpr[2] = 1024;
+ env->gpr[3] = bi->bd_info;
+ env->gpr[4] = bi->initrd_start;
+ env->gpr[5] = bi->initrd_end;
+ env->gpr[6] = bi->cmdline_start;
+ env->gpr[7] = bi->cmdline_end;
+ env->nip = bi->entry;
+ }
+ cpu_ppc_tb_reset(env);
}
static void fix_spd_data(uint8_t *spd)
@@ -205,6 +264,8 @@ static void amigaone_init(MachineState *machine)
I2CBus *i2c_bus;
uint8_t *spd_data;
DriveInfo *di;
+ hwaddr loadaddr;
+ struct boot_info *bi = NULL;
/* init CPU */
cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
@@ -301,6 +362,56 @@ static void amigaone_init(MachineState *machine)
}
pci_ide_create_devs(PCI_DEVICE(object_resolve_path_component(via, "ide")));
pci_vga_init(pci_bus);
+
+ if (!machine->kernel_filename) {
+ return;
+ }
+
+ /* handle -kernel, -initrd, -append options and emulate U-Boot */
+ bi = g_new0(struct boot_info, 1);
+ cpu->env.load_info = bi;
+
+ loadaddr = MIN(machine->ram_size, 256 * MiB);
+ bi->bd_info = loadaddr - 8 * MiB;
+ create_bd_info(bi->bd_info, machine->ram_size);
+ bi->stack = bi->bd_info - 64 * KiB - 8;
+
+ if (machine->kernel_cmdline && machine->kernel_cmdline[0]) {
+ size_t len = strlen(machine->kernel_cmdline);
+
+ loadaddr = bi->bd_info + 1 * MiB;
+ cpu_physical_memory_write(loadaddr, machine->kernel_cmdline, len + 1);
+ bi->cmdline_start = loadaddr;
+ bi->cmdline_end = loadaddr + len + 1; /* including terminating '\0' */
+ }
+
+ sz = load_elf(machine->kernel_filename, NULL, NULL, NULL,
+ &bi->entry, &loadaddr, NULL, NULL,
+ ELFDATA2MSB, PPC_ELF_MACHINE, 0, 0);
+ if (sz <= 0) {
+ sz = load_uimage(machine->kernel_filename, &bi->entry, &loadaddr,
+ NULL, NULL, NULL);
+ }
+ if (sz <= 0) {
+ error_report("Could not load kernel '%s'",
+ machine->kernel_filename);
+ exit(1);
+ }
+ loadaddr += sz;
+
+ if (machine->initrd_filename) {
+ loadaddr = ROUND_UP(loadaddr + 4 * MiB, 4 * KiB);
+ loadaddr = MAX(loadaddr, INITRD_MIN_ADDR);
+ sz = load_image_targphys(machine->initrd_filename, loadaddr,
+ bi->bd_info - loadaddr);
+ if (sz <= 0) {
+ error_report("Could not load initrd '%s'",
+ machine->initrd_filename);
+ exit(1);
+ }
+ bi->initrd_start = loadaddr;
+ bi->initrd_end = loadaddr + sz;
+ }
}
static void amigaone_machine_init(MachineClass *mc)
--
2.30.9
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/5] ppc/amigaone: Add #defines for memory map constants
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
` (3 preceding siblings ...)
2025-02-27 16:39 ` [PATCH v2 4/5] ppc/amigaone: Add kernel and initrd support BALATON Zoltan
@ 2025-02-27 16:39 ` BALATON Zoltan
2025-03-07 14:11 ` Philippe Mathieu-Daudé
2025-03-07 13:09 ` [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
2025-03-11 7:36 ` Nicholas Piggin
6 siblings, 1 reply; 13+ messages in thread
From: BALATON Zoltan @ 2025-02-27 16:39 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
hw/ppc/amigaone.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 04ff415a6b..f6c9eb69ab 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -32,6 +32,14 @@
#define BUS_FREQ_HZ 100000000
#define INITRD_MIN_ADDR 0x600000
+#define INIT_RAM_ADDR 0x40000000
+
+#define PCI_HIGH_ADDR 0x80000000
+#define PCI_HIGH_SIZE 0x7d000000
+#define PCI_LOW_ADDR 0xfd000000
+#define PCI_LOW_SIZE 0xe0000
+
+#define ARTICIA_ADDR 0xfe000000
/*
* Firmware binary available at
@@ -287,7 +295,7 @@ static void amigaone_init(MachineState *machine)
/* Firmware uses this area for startup */
mr = g_new(MemoryRegion, 1);
memory_region_init_ram(mr, NULL, "init-cache", 32 * KiB, &error_fatal);
- memory_region_add_subregion(get_system_memory(), 0x40000000, mr);
+ memory_region_add_subregion(get_system_memory(), INIT_RAM_ADDR, mr);
}
/* nvram */
@@ -322,7 +330,7 @@ static void amigaone_init(MachineState *machine)
}
/* Articia S */
- dev = sysbus_create_simple(TYPE_ARTICIA, 0xfe000000, NULL);
+ dev = sysbus_create_simple(TYPE_ARTICIA, ARTICIA_ADDR, NULL);
i2c_bus = I2C_BUS(qdev_get_child_bus(dev, "smbus"));
if (machine->ram_size > 512 * MiB) {
@@ -339,12 +347,12 @@ static void amigaone_init(MachineState *machine)
pci_mem = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
mr = g_new(MemoryRegion, 1);
memory_region_init_alias(mr, OBJECT(dev), "pci-mem-low", pci_mem,
- 0, 0xe0000);
- memory_region_add_subregion(get_system_memory(), 0xfd000000, mr);
+ 0, PCI_LOW_SIZE);
+ memory_region_add_subregion(get_system_memory(), PCI_LOW_ADDR, mr);
mr = g_new(MemoryRegion, 1);
memory_region_init_alias(mr, OBJECT(dev), "pci-mem-high", pci_mem,
- 0x80000000, 0x7d000000);
- memory_region_add_subregion(get_system_memory(), 0x80000000, mr);
+ PCI_HIGH_ADDR, PCI_HIGH_SIZE);
+ memory_region_add_subregion(get_system_memory(), PCI_HIGH_ADDR, mr);
pci_bus = PCI_BUS(qdev_get_child_bus(dev, "pci.0"));
/* VIA VT82c686B South Bridge (multifunction PCI device) */
--
2.30.9
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] ppc/amigaone patches
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
` (4 preceding siblings ...)
2025-02-27 16:39 ` [PATCH v2 5/5] ppc/amigaone: Add #defines for memory map constants BALATON Zoltan
@ 2025-03-07 13:09 ` BALATON Zoltan
2025-03-11 7:36 ` Nicholas Piggin
6 siblings, 0 replies; 13+ messages in thread
From: BALATON Zoltan @ 2025-03-07 13:09 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
On Thu, 27 Feb 2025, BALATON Zoltan wrote:
> Hello,
>
> v2:
> - change unused read function to g_assert_not_reached()
> - new patch to add defines to constants
> - added R-b tags
Ping? Will there be a pull request for soft freeze with this series
included or do I need to do anything with this?
Regards,
BALATON Zoltan
> This series adds NVRAM and support for -kernel, -initrd and -append
> options to the amigaone machine. This makes it easier to boot AmigaOS
> and avoids a crash in the guest when it tries to access NVRAM.
>
> While the -kernel option emulates what U-Boot passes to the kernel,
> old Linux kernels for amigaone may not work with it because of two
> reasons: these come in legacy U-Boot Multi-File image format that QEMU
> cannot read and even after unpacking that and creating a kernel uimage
> it won't find PCI devices because it does not initialise them
> correctly. This works when booted from U-Boot because U-Boot inits PCI
> devices. So does my BBoot loader which can be used to load AmigaOS so
> I don't intend to emulate that part of U-Boot.
>
> I'd like this to be merged for the next release please. When merging
> please update https://wiki.qemu.org/ChangeLog/10.0 with the following:
>
> amigaone
>
> Added support for NVRAM and -kernel, -initrd, -append command line
> options. By default the NVRAM contents are not preserved between
> sessions. To make it persistent create a backing file with 'qemu-image
> create -f raw nvram.bin 4k' and add -drive
> if=mtd,format=raw,file=nvram.bin to keep NVRAM contents in the backing
> file so settings stored in it will be preserved between sessions.
>
> To run AmigaOS with BBoot using the -kernel option at least BBoot
> version 0.8 is needed. Older BBoot versions only work with -device
> loader and cannot be used with -kernel on amigaone.
>
> Regards,
>
> BALATON Zoltan (5):
> ppc/amigaone: Simplify replacement dummy_fw
> ppc/amigaone: Implement NVRAM emulation
> ppc/amigaone: Add default environment
> ppc/amigaone: Add kernel and initrd support
> ppc/amigaone: Add #defines for memory map constants
>
> hw/ppc/amigaone.c | 284 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 271 insertions(+), 13 deletions(-)
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/5] ppc/amigaone: Add default environment
2025-02-27 16:39 ` [PATCH v2 3/5] ppc/amigaone: Add default environment BALATON Zoltan
@ 2025-03-07 14:10 ` Philippe Mathieu-Daudé
2025-03-07 14:46 ` BALATON Zoltan
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 14:10 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
Hi Zoltan,
Minor review comments in case you respin (not blocking).
On 27/2/25 17:39, BALATON Zoltan wrote:
> Initialise empty NVRAM with default values. This also enables IDE UDMA
> mode in AmigaOS that is faster but has to be enabled in environment
> due to problems with real hardware but that does not affect emulation
> so we can use faster defaults here.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
> index 849c9fc6e0..5c5585d39a 100644
> --- a/hw/ppc/amigaone.c
> +++ b/hw/ppc/amigaone.c
> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
> #define NVRAM_ADDR 0xfd0e0000
> #define NVRAM_SIZE (4 * KiB)
>
> +static char default_env[] =
'const'
> + "baudrate=115200\0"
> + "stdout=vga\0"
> + "stdin=ps2kbd\0"
> + "bootcmd=boota; menu; run menuboot_cmd\0"
> + "boot1=ide\0"
> + "boot2=cdrom\0"
> + "boota_timeout=3\0"
> + "ide_doreset=on\0"
> + "pci_irqa=9\0"
> + "pci_irqa_select=level\0"
> + "pci_irqb=10\0"
> + "pci_irqb_select=level\0"
> + "pci_irqc=11\0"
> + "pci_irqc_select=level\0"
> + "pci_irqd=7\0"
> + "pci_irqd_select=level\0"
> + "a1ide_irq=1111\0"
> + "a1ide_xfer=FFFF\0";
> +#define CRC32_DEFAULT_ENV 0xb5548481
> +#define CRC32_ALL_ZEROS 0x603b0489
> +
> #define TYPE_A1_NVRAM "a1-nvram"
> OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>
> @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error **errp)
> {
> A1NVRAMState *s = A1_NVRAM(dev);
> void *p;
> - uint32_t *c;
> + uint32_t crc, *c;
>
> memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, "nvram",
> NVRAM_SIZE, &error_fatal);
> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error **errp)
> return;
> }
> }
> + crc = crc32(0, p + 4, NVRAM_SIZE - 4);
> + if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default */
> + *c = cpu_to_be32(CRC32_DEFAULT_ENV);
Prefer the ld/st API over cpu_to/from:
stl_be_p(c, CRC32_DEFAULT_ENV);
> + /* Also copies terminating \0 as env is terminated by \0\0 */
> + memcpy(p + 4, default_env, sizeof(default_env));
> + if (s->blk) {
> + blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p, 0);
> + }
> + return;
> + }
> if (*c == 0) {
> *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
> if (s->blk) {
> blk_pwrite(s->blk, 0, 4, p, 0);
> }
> }
> + if (be32_to_cpu(*c) != crc) {
if (ldl_be_p(c) != crc) {
> + warn_report("NVRAM checksum mismatch");
> + }
> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 5/5] ppc/amigaone: Add #defines for memory map constants
2025-02-27 16:39 ` [PATCH v2 5/5] ppc/amigaone: Add #defines for memory map constants BALATON Zoltan
@ 2025-03-07 14:11 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 14:11 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: Nicholas Piggin
On 27/2/25 17:39, BALATON Zoltan wrote:
> Suggested-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> hw/ppc/amigaone.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/5] ppc/amigaone: Add default environment
2025-03-07 14:10 ` Philippe Mathieu-Daudé
@ 2025-03-07 14:46 ` BALATON Zoltan
2025-03-07 15:22 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 13+ messages in thread
From: BALATON Zoltan @ 2025-03-07 14:46 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-ppc, Nicholas Piggin
[-- Attachment #1: Type: text/plain, Size: 4016 bytes --]
On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
> Hi Zoltan,
>
> Minor review comments in case you respin (not blocking).
>
> On 27/2/25 17:39, BALATON Zoltan wrote:
>> Initialise empty NVRAM with default values. This also enables IDE UDMA
>> mode in AmigaOS that is faster but has to be enabled in environment
>> due to problems with real hardware but that does not affect emulation
>> so we can use faster defaults here.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>> index 849c9fc6e0..5c5585d39a 100644
>> --- a/hw/ppc/amigaone.c
>> +++ b/hw/ppc/amigaone.c
>> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
>> #define NVRAM_ADDR 0xfd0e0000
>> #define NVRAM_SIZE (4 * KiB)
>> +static char default_env[] =
>
> 'const'
OK. Could be fixed up on merge by Nick or I can send a new version if
needed.
>> + "baudrate=115200\0"
>> + "stdout=vga\0"
>> + "stdin=ps2kbd\0"
>> + "bootcmd=boota; menu; run menuboot_cmd\0"
>> + "boot1=ide\0"
>> + "boot2=cdrom\0"
>> + "boota_timeout=3\0"
>> + "ide_doreset=on\0"
>> + "pci_irqa=9\0"
>> + "pci_irqa_select=level\0"
>> + "pci_irqb=10\0"
>> + "pci_irqb_select=level\0"
>> + "pci_irqc=11\0"
>> + "pci_irqc_select=level\0"
>> + "pci_irqd=7\0"
>> + "pci_irqd_select=level\0"
>> + "a1ide_irq=1111\0"
>> + "a1ide_xfer=FFFF\0";
>> +#define CRC32_DEFAULT_ENV 0xb5548481
>> +#define CRC32_ALL_ZEROS 0x603b0489
>
>> +
>> #define TYPE_A1_NVRAM "a1-nvram"
>> OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>> @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error
>> **errp)
>> {
>> A1NVRAMState *s = A1_NVRAM(dev);
>> void *p;
>> - uint32_t *c;
>> + uint32_t crc, *c;
>> memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, "nvram",
>> NVRAM_SIZE, &error_fatal);
>> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error
>> **errp)
>> return;
>> }
>> }
>> + crc = crc32(0, p + 4, NVRAM_SIZE - 4);
>> + if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default
>> */
>> + *c = cpu_to_be32(CRC32_DEFAULT_ENV);
>
> Prefer the ld/st API over cpu_to/from:
>
> stl_be_p(c, CRC32_DEFAULT_ENV);
>
>> + /* Also copies terminating \0 as env is terminated by \0\0 */
>> + memcpy(p + 4, default_env, sizeof(default_env));
>> + if (s->blk) {
>> + blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p,
>> 0);
>> + }
>> + return;
>> + }
>> if (*c == 0) {
>> *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
>> if (s->blk) {
>> blk_pwrite(s->blk, 0, 4, p, 0);
>> }
>> }
>> + if (be32_to_cpu(*c) != crc) {
>
> if (ldl_be_p(c) != crc) {
Why? Here we want to convert a value from host CPU endianness to a
specific endianness and vice versa in code running on the host. (We are
not accessing guest memory, we operate on the memory region pointer. The
guest is not even running yet.)
Also:
static inline int ldl_be_p(const void *ptr)
{
return be_bswap(ldl_he_p(ptr), 32);
}
static inline int ldl_he_p(const void *ptr)
{
int32_t r;
__builtin_memcpy(&r, ptr, sizeof(r));
return r;
}
#define be_bswap(v, size) glue(__builtin_bswap, size)(v)
so this is
int32_t r;
__builtin_memcpy(&r, c, sizeof(r));
__builtin_bswap32(r);
versus
static inline type endian ## size ## _to_cpu(type v)
{
return glue(endian, _bswap)(v, size);
}
which is just
__builtin_bswap32(*c);
The second one makes more sense to me and don't see why I'd want to do it
in a more cumbersome way when we end up with the same result but simpler.
Regards,
BALATON Zoltan
>> + warn_report("NVRAM checksum mismatch");
>> + }
>> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/5] ppc/amigaone: Add default environment
2025-03-07 14:46 ` BALATON Zoltan
@ 2025-03-07 15:22 ` Philippe Mathieu-Daudé
2025-03-07 15:36 ` BALATON Zoltan
0 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-03-07 15:22 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc, Nicholas Piggin
On 7/3/25 15:46, BALATON Zoltan wrote:
> On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
>> Hi Zoltan,
>>
>> Minor review comments in case you respin (not blocking).
>>
>> On 27/2/25 17:39, BALATON Zoltan wrote:
>>> Initialise empty NVRAM with default values. This also enables IDE UDMA
>>> mode in AmigaOS that is faster but has to be enabled in environment
>>> due to problems with real hardware but that does not affect emulation
>>> so we can use faster defaults here.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>> hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>>> index 849c9fc6e0..5c5585d39a 100644
>>> --- a/hw/ppc/amigaone.c
>>> +++ b/hw/ppc/amigaone.c
>>> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
>>> #define NVRAM_ADDR 0xfd0e0000
>>> #define NVRAM_SIZE (4 * KiB)
>>> +static char default_env[] =
>>
>> 'const'
>
> OK. Could be fixed up on merge by Nick or I can send a new version if
> needed.
>
>>> + "baudrate=115200\0"
>>> + "stdout=vga\0"
>>> + "stdin=ps2kbd\0"
>>> + "bootcmd=boota; menu; run menuboot_cmd\0"
>>> + "boot1=ide\0"
>>> + "boot2=cdrom\0"
>>> + "boota_timeout=3\0"
>>> + "ide_doreset=on\0"
>>> + "pci_irqa=9\0"
>>> + "pci_irqa_select=level\0"
>>> + "pci_irqb=10\0"
>>> + "pci_irqb_select=level\0"
>>> + "pci_irqc=11\0"
>>> + "pci_irqc_select=level\0"
>>> + "pci_irqd=7\0"
>>> + "pci_irqd_select=level\0"
>>> + "a1ide_irq=1111\0"
>>> + "a1ide_xfer=FFFF\0";
>>> +#define CRC32_DEFAULT_ENV 0xb5548481
>>> +#define CRC32_ALL_ZEROS 0x603b0489
>>
>>> +
>>> #define TYPE_A1_NVRAM "a1-nvram"
>>> OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>>> @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev,
>>> Error **errp)
>>> {
>>> A1NVRAMState *s = A1_NVRAM(dev);
>>> void *p;
>>> - uint32_t *c;
>>> + uint32_t crc, *c;
>>> memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s,
>>> "nvram",
>>> NVRAM_SIZE, &error_fatal);
>>> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev,
>>> Error **errp)
>>> return;
>>> }
>>> }
>>> + crc = crc32(0, p + 4, NVRAM_SIZE - 4);
>>> + if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set
>>> default */
>>> + *c = cpu_to_be32(CRC32_DEFAULT_ENV);
>>
>> Prefer the ld/st API over cpu_to/from:
>>
>> stl_be_p(c, CRC32_DEFAULT_ENV);
>>
>>> + /* Also copies terminating \0 as env is terminated by \0\0 */
>>> + memcpy(p + 4, default_env, sizeof(default_env));
>>> + if (s->blk) {
>>> + blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env),
>>> p, 0);
>>> + }
>>> + return;
>>> + }
>>> if (*c == 0) {
>>> *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
>>> if (s->blk) {
>>> blk_pwrite(s->blk, 0, 4, p, 0);
>>> }
>>> }
>>> + if (be32_to_cpu(*c) != crc) {
>>
>> if (ldl_be_p(c) != crc) {
>
> Why? Here we want to convert a value from host CPU endianness to a
> specific endianness and vice versa in code running on the host. (We are
> not accessing guest memory, we operate on the memory region pointer. The
> guest is not even running yet.)
>
> Also:
>
> static inline int ldl_be_p(const void *ptr)
> {
> return be_bswap(ldl_he_p(ptr), 32);
> }
>
> static inline int ldl_he_p(const void *ptr)
> {
> int32_t r;
> __builtin_memcpy(&r, ptr, sizeof(r));
> return r;
> }
>
> #define be_bswap(v, size) glue(__builtin_bswap, size)(v)
>
> so this is
>
> int32_t r;
> __builtin_memcpy(&r, c, sizeof(r));
This call makes the address alignment access safe.
Sometimes we use similar API doing unaligned access and static
analyzers complain [*]. Rather than maintaining 2 differents APIs
with some corner cases in one, we could always use the reliable
one.
[*] see for example commit 5814c084679 ("hw/net/virtio-net.c: Don't
assume IP length field is aligned")
> __builtin_bswap32(r);
>
> versus
>
> static inline type endian ## size ## _to_cpu(type v)
> {
> return glue(endian, _bswap)(v, size);
> }
>
> which is just
>
> __builtin_bswap32(*c);
>
> The second one makes more sense to me and don't see why I'd want to do
> it in a more cumbersome way when we end up with the same result but
> simpler.
>
> Regards,
> BALATON Zoltan
>
>>> + warn_report("NVRAM checksum mismatch");
>>> + }
>>> }
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/5] ppc/amigaone: Add default environment
2025-03-07 15:22 ` Philippe Mathieu-Daudé
@ 2025-03-07 15:36 ` BALATON Zoltan
0 siblings, 0 replies; 13+ messages in thread
From: BALATON Zoltan @ 2025-03-07 15:36 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, qemu-ppc, Nicholas Piggin
[-- Attachment #1: Type: text/plain, Size: 5504 bytes --]
On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
> On 7/3/25 15:46, BALATON Zoltan wrote:
>> On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
>>> Hi Zoltan,
>>>
>>> Minor review comments in case you respin (not blocking).
>>>
>>> On 27/2/25 17:39, BALATON Zoltan wrote:
>>>> Initialise empty NVRAM with default values. This also enables IDE UDMA
>>>> mode in AmigaOS that is faster but has to be enabled in environment
>>>> due to problems with real hardware but that does not affect emulation
>>>> so we can use faster defaults here.
>>>>
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> ---
>>>> hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
>>>> 1 file changed, 36 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>>>> index 849c9fc6e0..5c5585d39a 100644
>>>> --- a/hw/ppc/amigaone.c
>>>> +++ b/hw/ppc/amigaone.c
>>>> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
>>>> #define NVRAM_ADDR 0xfd0e0000
>>>> #define NVRAM_SIZE (4 * KiB)
>>>> +static char default_env[] =
>>>
>>> 'const'
>>
>> OK. Could be fixed up on merge by Nick or I can send a new version if
>> needed.
>>
>>>> + "baudrate=115200\0"
>>>> + "stdout=vga\0"
>>>> + "stdin=ps2kbd\0"
>>>> + "bootcmd=boota; menu; run menuboot_cmd\0"
>>>> + "boot1=ide\0"
>>>> + "boot2=cdrom\0"
>>>> + "boota_timeout=3\0"
>>>> + "ide_doreset=on\0"
>>>> + "pci_irqa=9\0"
>>>> + "pci_irqa_select=level\0"
>>>> + "pci_irqb=10\0"
>>>> + "pci_irqb_select=level\0"
>>>> + "pci_irqc=11\0"
>>>> + "pci_irqc_select=level\0"
>>>> + "pci_irqd=7\0"
>>>> + "pci_irqd_select=level\0"
>>>> + "a1ide_irq=1111\0"
>>>> + "a1ide_xfer=FFFF\0";
>>>> +#define CRC32_DEFAULT_ENV 0xb5548481
>>>> +#define CRC32_ALL_ZEROS 0x603b0489
>>>
>>>> +
>>>> #define TYPE_A1_NVRAM "a1-nvram"
>>>> OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>>>> @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error
>>>> **errp)
>>>> {
>>>> A1NVRAMState *s = A1_NVRAM(dev);
>>>> void *p;
>>>> - uint32_t *c;
>>>> + uint32_t crc, *c;
>>>> memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s,
>>>> "nvram",
>>>> NVRAM_SIZE, &error_fatal);
>>>> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error
>>>> **errp)
>>>> return;
>>>> }
>>>> }
>>>> + crc = crc32(0, p + 4, NVRAM_SIZE - 4);
>>>> + if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default
>>>> */
>>>> + *c = cpu_to_be32(CRC32_DEFAULT_ENV);
>>>
>>> Prefer the ld/st API over cpu_to/from:
>>>
>>> stl_be_p(c, CRC32_DEFAULT_ENV);
>>>
>>>> + /* Also copies terminating \0 as env is terminated by \0\0 */
>>>> + memcpy(p + 4, default_env, sizeof(default_env));
>>>> + if (s->blk) {
>>>> + blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p,
>>>> 0);
>>>> + }
>>>> + return;
>>>> + }
>>>> if (*c == 0) {
>>>> *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
>>>> if (s->blk) {
>>>> blk_pwrite(s->blk, 0, 4, p, 0);
>>>> }
>>>> }
>>>> + if (be32_to_cpu(*c) != crc) {
>>>
>>> if (ldl_be_p(c) != crc) {
>>
>> Why? Here we want to convert a value from host CPU endianness to a specific
>> endianness and vice versa in code running on the host. (We are not
>> accessing guest memory, we operate on the memory region pointer. The guest
>> is not even running yet.)
>>
>> Also:
>>
>> static inline int ldl_be_p(const void *ptr)
>> {
>> return be_bswap(ldl_he_p(ptr), 32);
>> }
>>
>> static inline int ldl_he_p(const void *ptr)
>> {
>> int32_t r;
>> __builtin_memcpy(&r, ptr, sizeof(r));
>> return r;
>> }
>>
>> #define be_bswap(v, size) glue(__builtin_bswap, size)(v)
>>
>> so this is
>>
>> int32_t r;
>> __builtin_memcpy(&r, c, sizeof(r));
>
> This call makes the address alignment access safe.
>
> Sometimes we use similar API doing unaligned access and static
> analyzers complain [*]. Rather than maintaining 2 differents APIs
> with some corner cases in one, we could always use the reliable
> one.
>
> [*] see for example commit 5814c084679 ("hw/net/virtio-net.c: Don't assume IP
> length field is aligned")
That concern does not apply here as it's unlikely to have a memory region
allocated unaligned so I'd keep this until it's removed from everywhere.
We have a lot of uses of cpu_to_, _to_cpu now so it's not likely it would
go away soon. I see no advantage in introducing unneeded complexity here
to prevent something that cannot happen.
Regards,
BALATON Zoltan
>> __builtin_bswap32(r);
>>
>> versus
>>
>> static inline type endian ## size ## _to_cpu(type v)
>> {
>> return glue(endian, _bswap)(v, size);
>> }
>>
>> which is just
>>
>> __builtin_bswap32(*c);
>>
>> The second one makes more sense to me and don't see why I'd want to do it
>> in a more cumbersome way when we end up with the same result but simpler.
>>
>> Regards,
>> BALATON Zoltan
>>
>>>> + warn_report("NVRAM checksum mismatch");
>>>> + }
>>>> }
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] ppc/amigaone patches
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
` (5 preceding siblings ...)
2025-03-07 13:09 ` [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
@ 2025-03-11 7:36 ` Nicholas Piggin
6 siblings, 0 replies; 13+ messages in thread
From: Nicholas Piggin @ 2025-03-11 7:36 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
On Fri Feb 28, 2025 at 2:39 AM AEST, BALATON Zoltan wrote:
> Hello,
>
> v2:
> - change unused read function to g_assert_not_reached()
> - new patch to add defines to constants
> - added R-b tags
>
> This series adds NVRAM and support for -kernel, -initrd and -append
> options to the amigaone machine. This makes it easier to boot AmigaOS
> and avoids a crash in the guest when it tries to access NVRAM.
>
> While the -kernel option emulates what U-Boot passes to the kernel,
> old Linux kernels for amigaone may not work with it because of two
> reasons: these come in legacy U-Boot Multi-File image format that QEMU
> cannot read and even after unpacking that and creating a kernel uimage
> it won't find PCI devices because it does not initialise them
> correctly. This works when booted from U-Boot because U-Boot inits PCI
> devices. So does my BBoot loader which can be used to load AmigaOS so
> I don't intend to emulate that part of U-Boot.
>
> I'd like this to be merged for the next release please. When merging
> please update https://wiki.qemu.org/ChangeLog/10.0 with the following:
>
> amigaone
>
> Added support for NVRAM and -kernel, -initrd, -append command line
> options. By default the NVRAM contents are not preserved between
> sessions. To make it persistent create a backing file with 'qemu-image
> create -f raw nvram.bin 4k' and add -drive
> if=mtd,format=raw,file=nvram.bin to keep NVRAM contents in the backing
> file so settings stored in it will be preserved between sessions.
>
> To run AmigaOS with BBoot using the -kernel option at least BBoot
> version 0.8 is needed. Older BBoot versions only work with -device
> loader and cannot be used with -kernel on amigaone.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Regards,
>
> BALATON Zoltan (5):
> ppc/amigaone: Simplify replacement dummy_fw
> ppc/amigaone: Implement NVRAM emulation
> ppc/amigaone: Add default environment
> ppc/amigaone: Add kernel and initrd support
> ppc/amigaone: Add #defines for memory map constants
>
> hw/ppc/amigaone.c | 284 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 271 insertions(+), 13 deletions(-)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-03-11 7:37 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-27 16:39 [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 1/5] ppc/amigaone: Simplify replacement dummy_fw BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 2/5] ppc/amigaone: Implement NVRAM emulation BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 3/5] ppc/amigaone: Add default environment BALATON Zoltan
2025-03-07 14:10 ` Philippe Mathieu-Daudé
2025-03-07 14:46 ` BALATON Zoltan
2025-03-07 15:22 ` Philippe Mathieu-Daudé
2025-03-07 15:36 ` BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 4/5] ppc/amigaone: Add kernel and initrd support BALATON Zoltan
2025-02-27 16:39 ` [PATCH v2 5/5] ppc/amigaone: Add #defines for memory map constants BALATON Zoltan
2025-03-07 14:11 ` Philippe Mathieu-Daudé
2025-03-07 13:09 ` [PATCH v2 0/5] ppc/amigaone patches BALATON Zoltan
2025-03-11 7:36 ` Nicholas Piggin
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).