qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3] mac_oldworld: Add machine ID register


From: BALATON Zoltan
Subject: Re: [PATCH v3] mac_oldworld: Add machine ID register
Date: Sun, 14 Jun 2020 16:11:18 +0200 (CEST)
User-agent: Alpine 2.22 (BSF 395 2020-01-19)

On Sun, 14 Jun 2020, Mark Cave-Ayland wrote:
On 13/06/2020 18:57, BALATON Zoltan wrote:

The G3 beige machine has a machine ID register that is accessed by the
firmware to deternine the board config. Add basic emulation of it.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
v3: add empty write function in case anything tries to write reg

hw/ppc/mac_oldworld.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 3812adc441..acaf468458 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -80,6 +80,22 @@ static void ppc_heathrow_reset(void *opaque)
     cpu_reset(CPU(cpu));
 }

+static uint64_t machine_id_read(void *opaque, hwaddr addr, unsigned size)
+{
+    return (addr == 0 && size == 2 ? 0x3d8c : 0);
+}
+
+static void machine_id_write(void *opaque, hwaddr addr,
+                             uint64_t val, unsigned size)
+{
+    return;
+}
+
+const MemoryRegionOps machine_id_reg_ops = {
+    .read = machine_id_read,
+    .write = machine_id_write,
+};
+
 static void ppc_heathrow_init(MachineState *machine)
 {
     ram_addr_t ram_size = machine->ram_size;
@@ -93,6 +109,7 @@ static void ppc_heathrow_init(MachineState *machine)
     char *filename;
     int linux_boot, i;
     MemoryRegion *bios = g_new(MemoryRegion, 1);
+    MemoryRegion *machine_id = g_new(MemoryRegion, 1);
     uint32_t kernel_base, initrd_base, cmdline_base = 0;
     int32_t kernel_size, initrd_size;
     PCIBus *pci_bus;
@@ -227,6 +244,10 @@ static void ppc_heathrow_init(MachineState *machine)
         }
     }

+    memory_region_init_io(machine_id, OBJECT(machine), &machine_id_reg_ops,
+                          NULL, "machine_id", 2);
+    memory_region_add_subregion(get_system_memory(), 0xff000004, machine_id);
+
     /* XXX: we register only 1 output pin for heathrow PIC */
     pic_dev = qdev_create(NULL, TYPE_HEATHROW);
     qdev_init_nofail(pic_dev);

I'm guessing this supersedes the version from the v2 patchset? Given that you're

Yes, I've realized that a write method is needed otherwise it will crash if something writes the reg. I've assumed it would check for NULL but seems it doesn't. I've found that out when adding similar code for a (yet unsubmitted) dummy screamer reg which was written so I've corrected this and resent only this patch. I've added the screamer reg to see if it would go farther but it seems it's missing i2c so cannot find RAM. I have a sketch to implement i2c in cuda but it does not work yet.

introducing a HeathrowMachineState in patch 5, I'd be inclined to store the
MemoryRegions there to prevent these references leaking after init(). A 
constant for
the address of the register would also be good here too.

Yes, this could be included in MachineState just I've done this before that and haven't thought about it. I might move it there. I'm not sure about the constant as this value is only used at this one place and for those I prefer writing the value directly as it's easier to read (no need to look up constant value) so I'd only use a constant for values needed more than once but could add one for this.

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]