[PATCH 00/15] hw/alpha: QOMify Clipper and Typhoon

[PATCH 00/15] hw/alpha: QOMify Clipper and Typhoon

[Yodel Eldar]

Hi,

This series expands Alpha's use of the QEMU Object Model.
Currently, Typhoon's typhoon_init() serves as both
instance_init and realize for the chipset. The helper takes
several arguments that are passed to it by the machine (in
clipper_init()); to QOMify Typhoon and give them proper
hooks, this series incrementally eliminates all of the
parameters of typhoon_init().

But first, some small cleanups of checkpatch.pl style problems
and input validation in Patches 1-3.

Part of the motivation behind the series was to get Alpha
ready for the Resettable Interface, and Patch 4 helps with that
by using rom_add_blob_fixed() so that the initrd parameters are
in-place upon reset (once that is possible).

Patches 5 and on begin the QOMification process in earnest:
To begin, we define an instance_init skeleton that adds property
links for the CPUs to point to Typhoon's cchip.cpu elements and
some plumbing. Next, to parent the CPUs to the machine in
clipper_init() and remove the `cpus` array from the typhoon_init()
signature, we replace cpu_create invocations with object_new(),
add them as children, and set their links. Since we iterate over
the CPUs, we took this route instead of multiple DEFINE_PROP_LINKs
in a Property array.

Note: Patch 8 (the final CPU patch) eliminates `cpus` from the
init helper's (typhoon_init) prototype but also add a TyphoonState
pointer to it, because we'll need it until the end of the series
when all the remaining parameters have been eliminated.

Patch 9 replaces the two qemu_irq pointers in typhoon_init() with
qdev_{get,init}_gpio_in_named() invocations.

Ten deals with RAM; here, we use the DEFINE_PROP_LINK macro in a
Property array since we only need to add and set the link property
to the existing machine->ram.

Patches 11-13: Since the Typhoon expects a function pointer and a
static configuration value to be provided to it (via typhoon_init)
by the board, we extend the TyphoonClass definition to hold these
as members: the PCI IRQ mapper and minimum PCI device slot,
respectively; in the Clipper, we define the Typhoon subclass
(TYPHOON_PCIHOST_CLIPPER) such that it assigns both of these
hardware constants in the (sub)class_init.
After this leg of the series, typhoon_init() only has the
TyphoonState pointer remaining as a parameter; it needs
this, because it still realizes the bus among other things.

Patch 14: So that we could move the realize out of typhoon_init(),
first we need to be able to extract the PCI bus that it returns.
We do this in clipper_init() using QOM path resolution via
object_resolve_path_component().

Patch 15: Finally, with the return value and other parameters
eliminated, we can split the logic of typhoon_init() into its
instance_init and realize hooks, and replace it with
sysbus_realize_and_unref() in clipper_init(). Huzzah!

Thanks,
Yodel

---
Yodel Eldar (15):
      hw/alpha/typhoon: Fix whitespace and block comment style problems
      hw/alpha/dp264: Fix block comment style problems
      hw/alpha/dp264: Validate kernel and initrd sizes
      hw/alpha/dp264: Use rom_add_blob_fixed() for initrd params
      hw/alpha/typhoon: Create instance_init and class_init
      hw/alpha/dp264: Add CPUs as children of the machine
      hw/alpha: Use QOM composition for the Typhoon chipset
      hw/alpha: Set CPU link properties and use them
      hw/alpha: Convert IRQ pointers to named GPIOs
      hw/alpha: Expose RAM to typhoon via QOM property link
      hw/alpha: Explicitly define TyphoonClass in alpha_sys.h
      hw/alpha: Move PCI IRQ mapping to TyphoonClass subclass
      hw/alpha: Move minimum PCI device slot to TyphoonClass
      hw/alpha: Use QOM path resolution to get Typhoon PCI bus
      hw/alpha: Split typhoon_init() into instance_init and realize

 hw/alpha/alpha_sys.h |  18 +++-
 hw/alpha/dp264.c     | 121 ++++++++++++++++++++-------
 hw/alpha/typhoon.c   | 226 +++++++++++++++++++++++++++++++--------------------
 3 files changed, 249 insertions(+), 116 deletions(-)
---
base-commit: 63ec1f769490d8371fd03220eaede91cc7dd7c80
change-id: 20260310-qomify-alpha-65135464492d

[PATCH 01/15] hw/alpha/typhoon: Fix whitespace and block comment style problems

[Yodel Eldar]

Eliminate trailing whitespaces.

Use leading "/*", internal "*", and trailing "*/" for block comments.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/typhoon.c | 84 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 52 insertions(+), 32 deletions(-)

diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 7722d4fd00..f392772500 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -34,7 +34,7 @@ typedef struct TyphoonWindow {
     uint64_t wsm;
     uint64_t tba;
 } TyphoonWindow;
- 
+
 typedef struct TyphoonPchip {
     MemoryRegion region;
     MemoryRegion reg_iack;
@@ -84,8 +84,10 @@ static MemTxResult cchip_read(void *opaque, hwaddr addr,
     switch (addr) {
     case 0x0000:
         /* CSC: Cchip System Configuration Register.  */
-        /* All sorts of data here; probably the only thing relevant is
-           PIP<14> Pchip 1 Present = 0.  */
+        /*
+         * All sorts of data here; probably the only thing relevant is
+         * PIP<14> Pchip 1 Present = 0.
+         */
         break;
 
     case 0x0040:
@@ -188,7 +190,7 @@ static MemTxResult cchip_read(void *opaque, hwaddr addr,
     case 0x0780:
         /* PWR: Power Management Control.   */
         break;
-    
+
     case 0x0c00: /* CMONCTLA */
     case 0x0c40: /* CMONCTLB */
     case 0x0c80: /* CMONCNT01 */
@@ -440,7 +442,7 @@ static MemTxResult cchip_write(void *opaque, hwaddr addr,
     case 0x0780:
         /* PWR: Power Management Control.   */
         break;
-    
+
     case 0x0c00: /* CMONCTLA */
     case 0x0c40: /* CMONCTLB */
     case 0x0c80: /* CMONCNT01 */
@@ -605,8 +607,10 @@ static const MemoryRegionOps pchip_ops = {
     },
 };
 
-/* A subroutine of typhoon_translate_iommu that builds an IOMMUTLBEntry
-   using the given translated address and mask.  */
+/*
+ * A subroutine of typhoon_translate_iommu that builds an IOMMUTLBEntry
+ * using the given translated address and mask.
+ */
 static bool make_iommu_tlbe(hwaddr taddr, hwaddr mask, IOMMUTLBEntry *ret)
 {
     *ret = (IOMMUTLBEntry) {
@@ -618,8 +622,10 @@ static bool make_iommu_tlbe(hwaddr taddr, hwaddr mask, IOMMUTLBEntry *ret)
     return true;
 }
 
-/* A subroutine of typhoon_translate_iommu that handles scatter-gather
-   translation, given the address of the PTE.  */
+/*
+ * A subroutine of typhoon_translate_iommu that handles scatter-gather
+ * translation, given the address of the PTE.
+ */
 static bool pte_translate(hwaddr pte_addr, IOMMUTLBEntry *ret)
 {
     uint64_t pte = address_space_ldq_le(&address_space_memory, pte_addr,
@@ -633,8 +639,10 @@ static bool pte_translate(hwaddr pte_addr, IOMMUTLBEntry *ret)
     return make_iommu_tlbe((pte & 0x3ffffe) << 12, 0x1fff, ret);
 }
 
-/* A subroutine of typhoon_translate_iommu that handles one of the
-   four single-address-cycle translation windows.  */
+/*
+ * A subroutine of typhoon_translate_iommu that handles one of the
+ * four single-address-cycle translation windows.
+ */
 static bool window_translate(TyphoonWindow *win, hwaddr addr,
                              IOMMUTLBEntry *ret)
 {
@@ -668,8 +676,10 @@ static bool window_translate(TyphoonWindow *win, hwaddr addr,
 }
 
 /* Handle PCI-to-system address translation.  */
-/* TODO: A translation failure here ought to set PCI error codes on the
-   Pchip and generate a machine check interrupt.  */
+/*
+ * TODO: A translation failure here ought to set PCI error codes on the
+ * Pchip and generate a machine check interrupt.
+ */
 static IOMMUTLBEntry typhoon_translate_iommu(IOMMUMemoryRegion *iommu,
                                              hwaddr addr,
                                              IOMMUAccessFlags flag,
@@ -773,10 +783,12 @@ static void typhoon_set_timer_irq(void *opaque, int irq, int level)
     TyphoonState *s = opaque;
     int i;
 
-    /* Thankfully, the mc146818rtc code doesn't track the IRQ state,
-       and so we don't have to worry about missing interrupts just
-       because we never actually ACK the interrupt.  Just ignore any
-       case of the interrupt level going low.  */
+    /*
+     * Thankfully, the mc146818rtc code doesn't track the IRQ state,
+     * and so we don't have to worry about missing interrupts just
+     * because we never actually ACK the interrupt.  Just ignore any
+     * case of the interrupt level going low.
+     */
     if (level == 0) {
         return;
     }
@@ -787,14 +799,16 @@ static void typhoon_set_timer_irq(void *opaque, int irq, int level)
         if (cpu != NULL) {
             uint32_t iic = s->cchip.iic[i];
 
-            /* ??? The verbage in Section 10.2.2.10 isn't 100% clear.
-               Bit 24 is the OverFlow bit, RO, and set when the count
-               decrements past 0.  When is OF cleared?  My guess is that
-               OF is actually cleared when the IIC is written, and that
-               the ICNT field always decrements.  At least, that's an
-               interpretation that makes sense, and "allows the CPU to
-               determine exactly how mant interval timer ticks were
-               skipped".  At least within the next 4M ticks...  */
+            /*
+             * ??? The verbage in Section 10.2.2.10 isn't 100% clear.
+             * Bit 24 is the OverFlow bit, RO, and set when the count
+             * decrements past 0.  When is OF cleared?  My guess is that
+             * OF is actually cleared when the IIC is written, and that
+             * the ICNT field always decrements.  At least, that's an
+             * interpretation that makes sense, and "allows the CPU to
+             * determine exactly how mant interval timer ticks were
+             * skipped".  At least within the next 4M ticks...
+             */
 
             iic = ((iic - 1) & 0x1ffffff) | (iic & 0x1000000);
             s->cchip.iic[i] = iic;
@@ -852,13 +866,17 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
     *p_isa_irq = qemu_allocate_irq(typhoon_set_isa_irq, s, 0);
     *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
 
-    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
-       but the address space hole reserved at this point is 8TB.  */
+    /*
+     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
+     * but the address space hole reserved at this point is 8TB.
+     */
     memory_region_add_subregion(addr_space, 0, ram);
 
     /* TIGbus, 0x801.0000.0000, 1GB.  */
-    /* ??? The TIGbus is used for delivering interrupts, and access to
-       the flash ROM.  I'm not sure that we need to implement it at all.  */
+    /*
+     * ??? The TIGbus is used for delivering interrupts, and access to
+     * the flash ROM.  I'm not sure that we need to implement it at all.
+     */
 
     /* Pchip0 CSRs, 0x801.8000.0000, 256MB.  */
     memory_region_init_io(&s->pchip.region, OBJECT(s), &pchip_ops, s, "pchip0",
@@ -916,9 +934,11 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
     memory_region_add_subregion(addr_space, 0x801fe000000ULL,
                                 &s->pchip.reg_conf);
 
-    /* For the record, these are the mappings for the second PCI bus.
-       We can get away with not implementing them because we indicate
-       via the Cchip.CSC<PIP> bit that Pchip1 is not present.  */
+    /*
+     * For the record, these are the mappings for the second PCI bus.
+     * We can get away with not implementing them because we indicate
+     * via the Cchip.CSC<PIP> bit that Pchip1 is not present.
+     */
     /* Pchip1 PCI memory, 0x802.0000.0000, 4GB.  */
     /* Pchip1 CSRs, 0x802.8000.0000, 256MB.  */
     /* Pchip1 PCI special/interrupt acknowledge, 0x802.F800.0000, 64MB.  */

-- 
2.53.0

[PATCH 02/15] hw/alpha/dp264: Fix block comment style problems

[Yodel Eldar]

Use leading "/*", internal "*", and trailing "*/" for block comments.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/dp264.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 98219f0456..27fbcee637 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -26,12 +26,14 @@ static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
     return addr;
 }
 
-/* Note that there are at least 3 viewpoints of IRQ numbers on Alpha systems.
-    (0) The dev_irq_n lines into the cpu, which we totally ignore,
-    (1) The DRIR lines in the typhoon chipset,
-    (2) The "vector" aka mangled interrupt number reported by SRM PALcode,
-    (3) The interrupt number assigned by the kernel.
-   The following function is concerned with (1) only.  */
+/*
+ * Note that there are at least 3 viewpoints of IRQ numbers on Alpha systems.
+ *  (0) The dev_irq_n lines into the cpu, which we totally ignore,
+ *  (1) The DRIR lines in the typhoon chipset,
+ *  (2) The "vector" aka mangled interrupt number reported by SRM PALcode,
+ *  (3) The interrupt number assigned by the kernel.
+ * The following function is concerned with (1) only.
+ */
 
 static int clipper_pci_map_irq(PCIDevice *d, int irq_num)
 {
@@ -132,9 +134,11 @@ static void clipper_init(MachineState *machine)
     pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
     pci_ide_create_devs(pci_dev);
 
-    /* Load PALcode.  Given that this is not "real" cpu palcode,
-       but one explicitly written for the emulation, we might as
-       well load it directly from and ELF image.  */
+    /*
+     * Load PALcode.  Given that this is not "real" cpu palcode,
+     * but one explicitly written for the emulation, we might as
+     * well load it directly from and ELF image.
+     */
     palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS,
                                       machine->firmware ?: "palcode-clipper");
     if (palcode_filename == NULL) {

-- 
2.53.0

[PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes

[Yodel Eldar]

Add an underflow check when calculating the initrd base address.

Warn the user if initrd overlaps with kernel.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/dp264.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 27fbcee637..87af919895 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -61,7 +61,7 @@ static void clipper_init(MachineState *machine)
     long size, i;
     char *palcode_filename;
     uint64_t palcode_entry;
-    uint64_t kernel_entry, kernel_low;
+    uint64_t kernel_entry, kernel_low, kernel_high;
     unsigned int smp_cpus = machine->smp.cpus;
 
     /* Create up to 4 cpus.  */
@@ -165,7 +165,7 @@ static void clipper_init(MachineState *machine)
         uint64_t param_offset;
 
         size = load_elf(kernel_filename, NULL, cpu_alpha_superpage_to_phys,
-                        NULL, &kernel_entry, &kernel_low, NULL, NULL,
+                        NULL, &kernel_entry, &kernel_low, &kernel_high, NULL,
                         ELFDATA2LSB, EM_ALPHA, 0, 0);
         if (size < 0) {
             error_report("could not load kernel '%s'", kernel_filename);
@@ -181,7 +181,7 @@ static void clipper_init(MachineState *machine)
         }
 
         if (initrd_filename) {
-            long initrd_base;
+            hwaddr initrd_base;
             int64_t initrd_size;
 
             initrd_size = get_image_size(initrd_filename, NULL);
@@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
             }
 
             /* Put the initrd image as high in memory as possible.  */
-            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
+            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
+                error_report("initial ram disk exceeds allotted ram size");
+                exit(1);
+            }
+            initrd_base &= TARGET_PAGE_MASK;
+            if (initrd_base <= kernel_high) {
+                warn_report("initial ram disk overlaps with kernel");
+            }
+
             load_image_targphys(initrd_filename, initrd_base,
                                 ram_size - initrd_base, NULL);
 

-- 
2.53.0

[PATCH 04/15] hw/alpha/dp264: Use rom_add_blob_fixed() for initrd params

[Yodel Eldar]

Replace address_space_stq_le() invocations with rom_add_blob_fixed().
This will prove helpful when the Clipper becomes resettable, because
the params will be reloaded without the initrd file.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/dp264.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 87af919895..f036d72268 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -183,6 +183,7 @@ static void clipper_init(MachineState *machine)
         if (initrd_filename) {
             hwaddr initrd_base;
             int64_t initrd_size;
+            uint64_t initrd_info[2];
 
             initrd_size = get_image_size(initrd_filename, NULL);
             if (initrd_size < 0) {
@@ -204,11 +205,11 @@ static void clipper_init(MachineState *machine)
             load_image_targphys(initrd_filename, initrd_base,
                                 ram_size - initrd_base, NULL);
 
-            address_space_stq_le(&address_space_memory, param_offset + 0x100,
-                                 initrd_base + 0xfffffc0000000000ULL,
-                                 MEMTXATTRS_UNSPECIFIED, NULL);
-            address_space_stq_le(&address_space_memory, param_offset + 0x108,
-                                 initrd_size, MEMTXATTRS_UNSPECIFIED, NULL);
+            stq_le_p(&initrd_info[0], initrd_base + 0xfffffc0000000000ULL);
+            stq_le_p(&initrd_info[1], initrd_size);
+
+            rom_add_blob_fixed("initrd_info", initrd_info, sizeof(initrd_info),
+                               param_offset + 0x100);
         }
     }
 }

-- 
2.53.0

[PATCH 05/15] hw/alpha/typhoon: Create instance_init and class_init

[Yodel Eldar]

Define the Typhoon class and instance initializers.

Add "cpu[*]" link properties in the instance_init of Typhoon.
Setting these cpu link properties will occur in a later commit.

Note: This is one step towards splitting typhoon_init() into the
instance_init and realize hooks. typhoon_init() currently takes
a fixed cpus array as a parameter; that array will be removed soon.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/typhoon.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index f392772500..5789829818 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -15,6 +15,7 @@
 #include "cpu.h"
 #include "hw/core/irq.h"
 #include "alpha_sys.h"
+#include "hw/core/qdev-properties.h"
 
 
 #define TYPE_TYPHOON_PCI_HOST_BRIDGE "typhoon-pcihost"
@@ -948,10 +949,32 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
     return b;
 }
 
+static void typhoon_pcihost_init(Object *obj)
+{
+    TyphoonState *s = TYPHOON_PCI_HOST_BRIDGE(obj);
+
+    int i;
+    for (i = 0; i < 4; ++i) {
+        g_autofree char *name = g_strdup_printf("cpu[%d]", i);
+        object_property_add_link(obj, name,  TYPE_ALPHA_CPU,
+                                 (Object **)&s->cchip.cpu[i],
+                                 qdev_prop_allow_set_link_before_realize, 0);
+    }
+}
+
+static void typhoon_pcihost_class_init(ObjectClass *klass, const void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->user_creatable = false;
+}
+
 static const TypeInfo typhoon_pcihost_info = {
     .name          = TYPE_TYPHOON_PCI_HOST_BRIDGE,
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(TyphoonState),
+    .instance_init = typhoon_pcihost_init,
+    .class_init    = typhoon_pcihost_class_init,
 };
 
 static void typhoon_iommu_memory_region_class_init(ObjectClass *klass,

-- 
2.53.0

[PATCH 06/15] hw/alpha/dp264: Add CPUs as children of the machine

[Yodel Eldar]

Replace cpu_create() calls with object_new(). Then, add the
CPUs as children of the Clipper. Finally, realize and unref
them.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/dp264.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index f036d72268..b2ef423f93 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -67,7 +67,11 @@ static void clipper_init(MachineState *machine)
     /* Create up to 4 cpus.  */
     memset(cpus, 0, sizeof(cpus));
     for (i = 0; i < smp_cpus; ++i) {
-        cpus[i] = ALPHA_CPU(cpu_create(machine->cpu_type));
+        g_autofree char *name = g_strdup_printf("cpu[%ld]", i);
+        Object *cpu = object_new(machine->cpu_type);
+        object_property_add_child(OBJECT(machine), name, cpu);
+        qdev_realize_and_unref(DEVICE(cpu), NULL, &error_fatal);
+        cpus[i] = ALPHA_CPU(cpu);
     }
 
     /*

-- 
2.53.0

[PATCH 07/15] hw/alpha: Use QOM composition for the Typhoon chipset

[Yodel Eldar]

Export the QOM declaration of TyphoonState by moving it out of
hw/alpha/typhoon.c into hw/alpha/alpha_sys.h. Keep its definition
encapsulated.

Define ClipperMachineState with a TyphoonState member.

Move the instantiation of Typhoon out of typhoon_init() into the
Clipper machine and add it as a QOM child object of Clipper.

For now, add TyphoonState pointer as a parameter of typhoon_init().

Note: Eventually, typhoon_init() will be split into Typhoon's
instance_init and realize hooks; this is an intermediate commit
to that end.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h |  4 +++-
 hw/alpha/dp264.c     | 31 ++++++++++++++++++++++++++++---
 hw/alpha/typhoon.c   | 12 +++---------
 3 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 6e6691d116..247a52b776 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -8,9 +8,11 @@
 #include "hw/core/boards.h"
 #include "hw/intc/i8259.h"
 
+#define TYPE_TYPHOON_PCI_HOST_BRIDGE "typhoon-pcihost"
+OBJECT_DECLARE_SIMPLE_TYPE(TyphoonState, TYPHOON_PCI_HOST_BRIDGE)
 
 PCIBus *typhoon_init(MemoryRegion *, qemu_irq *, qemu_irq *, AlphaCPU *[4],
-                     pci_map_irq_fn, uint8_t devfn_min);
+                     pci_map_irq_fn, uint8_t devfn_min, TyphoonState *);
 
 /* alpha_pci.c.  */
 extern const MemoryRegionOps alpha_pci_ignore_ops;
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index b2ef423f93..c9d639eb7b 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -18,6 +18,15 @@
 #include "hw/isa/superio.h"
 #include "qemu/datadir.h"
 
+#define TYPE_CLIPPER_MACHINE MACHINE_TYPE_NAME("clipper")
+OBJECT_DECLARE_SIMPLE_TYPE(ClipperMachineState, CLIPPER_MACHINE)
+
+struct ClipperMachineState {
+    MachineState parent_obj;
+
+    TyphoonState *typhoon;
+};
+
 static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
 {
     if (((addr >> 41) & 3) == 2) {
@@ -46,6 +55,7 @@ static int clipper_pci_map_irq(PCIDevice *d, int irq_num)
 
 static void clipper_init(MachineState *machine)
 {
+    ClipperMachineState *cms = CLIPPER_MACHINE(machine);
     ram_addr_t ram_size = machine->ram_size;
     const char *kernel_filename = machine->kernel_filename;
     const char *kernel_cmdline = machine->kernel_cmdline;
@@ -64,6 +74,11 @@ static void clipper_init(MachineState *machine)
     uint64_t kernel_entry, kernel_low, kernel_high;
     unsigned int smp_cpus = machine->smp.cpus;
 
+    Object * const typhoon_obj = object_new(TYPE_TYPHOON_PCI_HOST_BRIDGE);
+    TyphoonState * const typhoon = TYPHOON_PCI_HOST_BRIDGE(typhoon_obj);
+    cms->typhoon = typhoon;
+    object_property_add_child(OBJECT(machine), "typhoon", typhoon_obj);
+
     /* Create up to 4 cpus.  */
     memset(cpus, 0, sizeof(cpus));
     for (i = 0; i < smp_cpus; ++i) {
@@ -93,7 +108,7 @@ static void clipper_init(MachineState *machine)
      * the minimum PCI device IdSel is 1.
      */
     pci_bus = typhoon_init(machine->ram, &isa_irq, &rtc_irq, cpus,
-                           clipper_pci_map_irq, PCI_DEVFN(1, 0));
+                           clipper_pci_map_irq, PCI_DEVFN(1, 0), typhoon);
 
     /*
      * Init the PCI -> ISA bridge.
@@ -218,8 +233,9 @@ static void clipper_init(MachineState *machine)
     }
 }
 
-static void clipper_machine_init(MachineClass *mc)
+static void clipper_machine_init(ObjectClass *oc, const void *data)
 {
+    MachineClass *mc = MACHINE_CLASS(oc);
     mc->desc = "Alpha DP264/CLIPPER";
     mc->init = clipper_init;
     mc->block_default_type = IF_IDE;
@@ -230,4 +246,13 @@ static void clipper_machine_init(MachineClass *mc)
     mc->default_nic = "e1000";
 }
 
-DEFINE_MACHINE("clipper", clipper_machine_init)
+static const TypeInfo clipper_types[] = {
+    {
+        .name = TYPE_CLIPPER_MACHINE,
+        .parent = TYPE_MACHINE,
+        .instance_size = sizeof(ClipperMachineState),
+        .class_init = clipper_machine_init,
+    },
+};
+
+DEFINE_TYPES(clipper_types);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 5789829818..b0eb4e415c 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -18,7 +18,6 @@
 #include "hw/core/qdev-properties.h"
 
 
-#define TYPE_TYPHOON_PCI_HOST_BRIDGE "typhoon-pcihost"
 #define TYPE_TYPHOON_IOMMU_MEMORY_REGION "typhoon-iommu-memory-region"
 
 typedef struct TyphoonCchip {
@@ -50,8 +49,6 @@ typedef struct TyphoonPchip {
     TyphoonWindow win[4];
 } TyphoonPchip;
 
-OBJECT_DECLARE_SIMPLE_TYPE(TyphoonState, TYPHOON_PCI_HOST_BRIDGE)
-
 struct TyphoonState {
     PCIHostState parent_obj;
 
@@ -836,18 +833,15 @@ static void typhoon_alarm_timer(void *opaque)
 
 PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
                      qemu_irq *p_rtc_irq, AlphaCPU *cpus[4],
-                     pci_map_irq_fn sys_map_irq, uint8_t devfn_min)
+                     pci_map_irq_fn sys_map_irq, uint8_t devfn_min,
+                     TyphoonState *s)
 {
     MemoryRegion *addr_space = get_system_memory();
-    DeviceState *dev;
-    TyphoonState *s;
+    DeviceState *dev = DEVICE(s);
     PCIHostState *phb;
     PCIBus *b;
     int i;
 
-    dev = qdev_new(TYPE_TYPHOON_PCI_HOST_BRIDGE);
-
-    s = TYPHOON_PCI_HOST_BRIDGE(dev);
     phb = PCI_HOST_BRIDGE(dev);
 
     s->cchip.misc = 0x800000000ull; /* Revision: Typhoon.  */

-- 
2.53.0

[PATCH 08/15] hw/alpha: Set CPU link properties and use them

[Yodel Eldar]

Set the "cpu[*]" links upon instantiation. Eliminate the cpus
array parameter of typhoon_init(), as it is no longer needed.

Note: This commit helps with the eventual splitting of
typhoon_init() into instance_init and realize hooks, by
eliminating one of its parameters.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h | 2 +-
 hw/alpha/dp264.c     | 3 ++-
 hw/alpha/typhoon.c   | 5 ++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 247a52b776..f6779520d9 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -11,7 +11,7 @@
 #define TYPE_TYPHOON_PCI_HOST_BRIDGE "typhoon-pcihost"
 OBJECT_DECLARE_SIMPLE_TYPE(TyphoonState, TYPHOON_PCI_HOST_BRIDGE)
 
-PCIBus *typhoon_init(MemoryRegion *, qemu_irq *, qemu_irq *, AlphaCPU *[4],
+PCIBus *typhoon_init(MemoryRegion *, qemu_irq *, qemu_irq *,
                      pci_map_irq_fn, uint8_t devfn_min, TyphoonState *);
 
 /* alpha_pci.c.  */
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index c9d639eb7b..c0996efffe 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -85,6 +85,7 @@ static void clipper_init(MachineState *machine)
         g_autofree char *name = g_strdup_printf("cpu[%ld]", i);
         Object *cpu = object_new(machine->cpu_type);
         object_property_add_child(OBJECT(machine), name, cpu);
+        object_property_set_link(typhoon_obj, name, cpu, &error_fatal);
         qdev_realize_and_unref(DEVICE(cpu), NULL, &error_fatal);
         cpus[i] = ALPHA_CPU(cpu);
     }
@@ -107,7 +108,7 @@ static void clipper_init(MachineState *machine)
      * Init the chipset.  Because we're using CLIPPER IRQ mappings,
      * the minimum PCI device IdSel is 1.
      */
-    pci_bus = typhoon_init(machine->ram, &isa_irq, &rtc_irq, cpus,
+    pci_bus = typhoon_init(machine->ram, &isa_irq, &rtc_irq,
                            clipper_pci_map_irq, PCI_DEVFN(1, 0), typhoon);
 
     /*
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index b0eb4e415c..5fac5eba1c 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -832,7 +832,7 @@ static void typhoon_alarm_timer(void *opaque)
 }
 
 PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
-                     qemu_irq *p_rtc_irq, AlphaCPU *cpus[4],
+                     qemu_irq *p_rtc_irq,
                      pci_map_irq_fn sys_map_irq, uint8_t devfn_min,
                      TyphoonState *s)
 {
@@ -849,8 +849,7 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
 
     /* Remember the CPUs so that we can deliver interrupts to them.  */
     for (i = 0; i < 4; i++) {
-        AlphaCPU *cpu = cpus[i];
-        s->cchip.cpu[i] = cpu;
+        AlphaCPU *cpu = s->cchip.cpu[i];
         if (cpu != NULL) {
             cpu->alarm_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
                                                  typhoon_alarm_timer,

-- 
2.53.0

[PATCH 09/15] hw/alpha: Convert IRQ pointers to named GPIOs

[Yodel Eldar]

Eliminate the qemu_irq parameters of typhoon_init() by using
qdev_init_gpio_in_named() for the IRQ lines.

Replace the qemu_irq variables in clipper_init() by calling
qdev_get_gpio_in_named() in their place.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h |  5 ++++-
 hw/alpha/dp264.c     | 12 +++++++-----
 hw/alpha/typhoon.c   | 11 ++++++-----
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index f6779520d9..39a96760d7 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -11,9 +11,12 @@
 #define TYPE_TYPHOON_PCI_HOST_BRIDGE "typhoon-pcihost"
 OBJECT_DECLARE_SIMPLE_TYPE(TyphoonState, TYPHOON_PCI_HOST_BRIDGE)
 
-PCIBus *typhoon_init(MemoryRegion *, qemu_irq *, qemu_irq *,
+PCIBus *typhoon_init(MemoryRegion *,
                      pci_map_irq_fn, uint8_t devfn_min, TyphoonState *);
 
+#define TYPHOON_GPIO_ISA_IRQ "isa-irq"
+#define TYPHOON_GPIO_RTC_IRQ "rtc-irq"
+
 /* alpha_pci.c.  */
 extern const MemoryRegionOps alpha_pci_ignore_ops;
 extern const MemoryRegionOps alpha_pci_conf1_ops;
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index c0996efffe..61eeaef6db 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -66,8 +66,6 @@ static void clipper_init(MachineState *machine)
     PCIDevice *pci_dev;
     DeviceState *i82378_dev;
     ISABus *isa_bus;
-    qemu_irq rtc_irq;
-    qemu_irq isa_irq;
     long size, i;
     char *palcode_filename;
     uint64_t palcode_entry;
@@ -108,7 +106,7 @@ static void clipper_init(MachineState *machine)
      * Init the chipset.  Because we're using CLIPPER IRQ mappings,
      * the minimum PCI device IdSel is 1.
      */
-    pci_bus = typhoon_init(machine->ram, &isa_irq, &rtc_irq,
+    pci_bus = typhoon_init(machine->ram,
                            clipper_pci_map_irq, PCI_DEVFN(1, 0), typhoon);
 
     /*
@@ -136,10 +134,14 @@ static void clipper_init(MachineState *machine)
     isa_bus = ISA_BUS(qdev_get_child_bus(i82378_dev, "isa.0"));
 
     /* Connect the ISA PIC to the Typhoon IRQ used for ISA interrupts. */
-    qdev_connect_gpio_out(i82378_dev, 0, isa_irq);
+    qdev_connect_gpio_out(i82378_dev, 0,
+                          qdev_get_gpio_in_named(DEVICE(typhoon),
+                                                 TYPHOON_GPIO_ISA_IRQ, 0));
 
     /* Since we have an SRM-compatible PALcode, use the SRM epoch.  */
-    mc146818_rtc_init(isa_bus, 1900, rtc_irq);
+    mc146818_rtc_init(isa_bus, 1900,
+                      qdev_get_gpio_in_named(DEVICE(typhoon),
+                                             TYPHOON_GPIO_RTC_IRQ, 0));
 
     /* VGA setup.  Don't bother loading the bios.  */
     pci_vga_init(pci_bus);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 5fac5eba1c..03bb4c8055 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -831,8 +831,7 @@ static void typhoon_alarm_timer(void *opaque)
     cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
 }
 
-PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
-                     qemu_irq *p_rtc_irq,
+PCIBus *typhoon_init(MemoryRegion *ram,
                      pci_map_irq_fn sys_map_irq, uint8_t devfn_min,
                      TyphoonState *s)
 {
@@ -857,9 +856,6 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
         }
     }
 
-    *p_isa_irq = qemu_allocate_irq(typhoon_set_isa_irq, s, 0);
-    *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
-
     /*
      * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
      * but the address space hole reserved at this point is 8TB.
@@ -945,6 +941,7 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
 static void typhoon_pcihost_init(Object *obj)
 {
     TyphoonState *s = TYPHOON_PCI_HOST_BRIDGE(obj);
+    DeviceState *dev = DEVICE(obj);
 
     int i;
     for (i = 0; i < 4; ++i) {
@@ -953,6 +950,10 @@ static void typhoon_pcihost_init(Object *obj)
                                  (Object **)&s->cchip.cpu[i],
                                  qdev_prop_allow_set_link_before_realize, 0);
     }
+
+    qdev_init_gpio_in_named(dev, typhoon_set_isa_irq, TYPHOON_GPIO_ISA_IRQ, 1);
+    qdev_init_gpio_in_named(dev, typhoon_set_timer_irq, TYPHOON_GPIO_RTC_IRQ,
+                            1);
 }
 
 static void typhoon_pcihost_class_init(ObjectClass *klass, const void *data)

-- 
2.53.0

[PATCH 10/15] hw/alpha: Expose RAM to typhoon via QOM property link

[Yodel Eldar]

Currently, the board (Clipper/DP264) passes machine->ram to the
chipset (Typhoon) as an arg to typhoon_init(), where it is
added as a subregion of system memory. This commit, instead adds
a ram pointer in TyphoonState, defines it as a Property, and sets
a link to it from the machine, thus removing the ram parameter.

Note: Moving the ram subregion mapping into machine code was
considered but decided against so as to preserve the Typhoon's
historical role in mapping the address space.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h |  5 +++--
 hw/alpha/dp264.c     |  8 +++++---
 hw/alpha/typhoon.c   | 14 +++++++++++---
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 39a96760d7..8611881b55 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -11,8 +11,9 @@
 #define TYPE_TYPHOON_PCI_HOST_BRIDGE "typhoon-pcihost"
 OBJECT_DECLARE_SIMPLE_TYPE(TyphoonState, TYPHOON_PCI_HOST_BRIDGE)
 
-PCIBus *typhoon_init(MemoryRegion *,
-                     pci_map_irq_fn, uint8_t devfn_min, TyphoonState *);
+PCIBus *typhoon_init(pci_map_irq_fn, uint8_t devfn_min, TyphoonState *);
+
+#define TYPHOON_PROP_RAM "ram"
 
 #define TYPHOON_GPIO_ISA_IRQ "isa-irq"
 #define TYPHOON_GPIO_RTC_IRQ "rtc-irq"
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 61eeaef6db..a7fc137e70 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -102,12 +102,14 @@ static void clipper_init(MachineState *machine)
     cpus[0]->env.trap_arg1 = 0;
     cpus[0]->env.trap_arg2 = smp_cpus | (!machine->enable_graphics << 6);
 
+    object_property_set_link(typhoon_obj, TYPHOON_PROP_RAM,
+                             OBJECT(machine->ram), &error_fatal);
+
     /*
      * Init the chipset.  Because we're using CLIPPER IRQ mappings,
      * the minimum PCI device IdSel is 1.
      */
-    pci_bus = typhoon_init(machine->ram,
-                           clipper_pci_map_irq, PCI_DEVFN(1, 0), typhoon);
+    pci_bus = typhoon_init(clipper_pci_map_irq, PCI_DEVFN(1, 0), typhoon);
 
     /*
      * Init the PCI -> ISA bridge.
@@ -245,7 +247,7 @@ static void clipper_machine_init(ObjectClass *oc, const void *data)
     mc->max_cpus = 4;
     mc->is_default = true;
     mc->default_cpu_type = ALPHA_CPU_TYPE_NAME("ev67");
-    mc->default_ram_id = "ram";
+    mc->default_ram_id = TYPHOON_PROP_RAM;
     mc->default_nic = "e1000";
 }
 
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 03bb4c8055..792ace5ce7 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -55,6 +55,8 @@ struct TyphoonState {
     TyphoonCchip cchip;
     TyphoonPchip pchip;
     MemoryRegion dchip_region;
+
+    MemoryRegion *ram;
 };
 
 /* Called when one of DRIR or DIM changes.  */
@@ -831,8 +833,7 @@ static void typhoon_alarm_timer(void *opaque)
     cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
 }
 
-PCIBus *typhoon_init(MemoryRegion *ram,
-                     pci_map_irq_fn sys_map_irq, uint8_t devfn_min,
+PCIBus *typhoon_init(pci_map_irq_fn sys_map_irq, uint8_t devfn_min,
                      TyphoonState *s)
 {
     MemoryRegion *addr_space = get_system_memory();
@@ -860,7 +861,7 @@ PCIBus *typhoon_init(MemoryRegion *ram,
      * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
      * but the address space hole reserved at this point is 8TB.
      */
-    memory_region_add_subregion(addr_space, 0, ram);
+    memory_region_add_subregion(addr_space, 0, s->ram);
 
     /* TIGbus, 0x801.0000.0000, 1GB.  */
     /*
@@ -956,11 +957,18 @@ static void typhoon_pcihost_init(Object *obj)
                             1);
 }
 
+static const Property typhoon_properties[] = {
+    DEFINE_PROP_LINK(TYPHOON_PROP_RAM, TyphoonState, ram,
+                     TYPE_MEMORY_REGION, MemoryRegion *),
+};
+
 static void typhoon_pcihost_class_init(ObjectClass *klass, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->user_creatable = false;
+
+    device_class_set_props(dc, typhoon_properties);
 }
 
 static const TypeInfo typhoon_pcihost_info = {

-- 
2.53.0

[PATCH 11/15] hw/alpha: Explicitly define TyphoonClass in alpha_sys.h

[Yodel Eldar]

Currently, the Typhoon has no additional class fields besides
its inheritance (PCIHostBridgeClass), so an explicit definition
of its class struct was unnecessary, and we simply used
OBJECT_DECLARE_SIMPLE_TYPE to declare its instance and class;
however, forthcoming changes will require additional Typhoon-specific
class fields.

This commit creates the Typhoon class definition, although it remains
a shell of its parent class for now.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h | 8 +++++++-
 hw/alpha/typhoon.c   | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 8611881b55..794a47127d 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -5,11 +5,17 @@
 
 #include "target/alpha/cpu-qom.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_host.h"
 #include "hw/core/boards.h"
 #include "hw/intc/i8259.h"
 
 #define TYPE_TYPHOON_PCI_HOST_BRIDGE "typhoon-pcihost"
-OBJECT_DECLARE_SIMPLE_TYPE(TyphoonState, TYPHOON_PCI_HOST_BRIDGE)
+
+typedef struct TyphoonClass {
+    PCIHostBridgeClass parent_class;
+} TyphoonClass;
+
+OBJECT_DECLARE_TYPE(TyphoonState, TyphoonClass, TYPHOON_PCI_HOST_BRIDGE)
 
 PCIBus *typhoon_init(pci_map_irq_fn, uint8_t devfn_min, TyphoonState *);
 
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 792ace5ce7..8745025319 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -976,6 +976,7 @@ static const TypeInfo typhoon_pcihost_info = {
     .parent        = TYPE_PCI_HOST_BRIDGE,
     .instance_size = sizeof(TyphoonState),
     .instance_init = typhoon_pcihost_init,
+    .class_size    = sizeof(TyphoonClass),
     .class_init    = typhoon_pcihost_class_init,
 };
 

-- 
2.53.0

[PATCH 12/15] hw/alpha: Move PCI IRQ mapping to TyphoonClass subclass

[Yodel Eldar]

Introduce a Clipper-specific subclass of TyphoonClass, and set
the pci_map_irq_fn class member in the subclass init.

Since the board must provide the PCI IRQ mapping to the Typhoon,
mark the base Typhoon type as abstract.

Eliminate the pci_map_irq_fn parameter of typhoon_init().

Note: This commit brings us closer to the splitting of typhoon_init()
into instance_init and realize hooks.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h |  4 +++-
 hw/alpha/dp264.c     | 19 +++++++++++++++++--
 hw/alpha/typhoon.c   |  9 ++++++---
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 794a47127d..fdfe96dcb4 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -13,11 +13,13 @@
 
 typedef struct TyphoonClass {
     PCIHostBridgeClass parent_class;
+
+    pci_map_irq_fn sys_map_irq;
 } TyphoonClass;
 
 OBJECT_DECLARE_TYPE(TyphoonState, TyphoonClass, TYPHOON_PCI_HOST_BRIDGE)
 
-PCIBus *typhoon_init(pci_map_irq_fn, uint8_t devfn_min, TyphoonState *);
+PCIBus *typhoon_init(uint8_t devfn_min, TyphoonState *);
 
 #define TYPHOON_PROP_RAM "ram"
 
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index a7fc137e70..8c3c8fec67 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -21,6 +21,8 @@
 #define TYPE_CLIPPER_MACHINE MACHINE_TYPE_NAME("clipper")
 OBJECT_DECLARE_SIMPLE_TYPE(ClipperMachineState, CLIPPER_MACHINE)
 
+#define TYPE_TYPHOON_PCIHOST_CLIPPER "typhoon-pcihost-clipper"
+
 struct ClipperMachineState {
     MachineState parent_obj;
 
@@ -72,7 +74,7 @@ static void clipper_init(MachineState *machine)
     uint64_t kernel_entry, kernel_low, kernel_high;
     unsigned int smp_cpus = machine->smp.cpus;
 
-    Object * const typhoon_obj = object_new(TYPE_TYPHOON_PCI_HOST_BRIDGE);
+    Object * const typhoon_obj = object_new(TYPE_TYPHOON_PCIHOST_CLIPPER);
     TyphoonState * const typhoon = TYPHOON_PCI_HOST_BRIDGE(typhoon_obj);
     cms->typhoon = typhoon;
     object_property_add_child(OBJECT(machine), "typhoon", typhoon_obj);
@@ -109,7 +111,7 @@ static void clipper_init(MachineState *machine)
      * Init the chipset.  Because we're using CLIPPER IRQ mappings,
      * the minimum PCI device IdSel is 1.
      */
-    pci_bus = typhoon_init(clipper_pci_map_irq, PCI_DEVFN(1, 0), typhoon);
+    pci_bus = typhoon_init(PCI_DEVFN(1, 0), typhoon);
 
     /*
      * Init the PCI -> ISA bridge.
@@ -251,6 +253,14 @@ static void clipper_machine_init(ObjectClass *oc, const void *data)
     mc->default_nic = "e1000";
 }
 
+static void typhoon_pcihost_clipper_class_init(ObjectClass *oc,
+                                               const void *data)
+{
+    TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_CLASS(oc);
+
+    tc->sys_map_irq = clipper_pci_map_irq;
+}
+
 static const TypeInfo clipper_types[] = {
     {
         .name = TYPE_CLIPPER_MACHINE,
@@ -258,6 +268,11 @@ static const TypeInfo clipper_types[] = {
         .instance_size = sizeof(ClipperMachineState),
         .class_init = clipper_machine_init,
     },
+    {
+        .name = TYPE_TYPHOON_PCIHOST_CLIPPER,
+        .parent = TYPE_TYPHOON_PCI_HOST_BRIDGE,
+        .class_init = typhoon_pcihost_clipper_class_init,
+    },
 };
 
 DEFINE_TYPES(clipper_types);
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 8745025319..b261dd7bc5 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -833,15 +833,17 @@ static void typhoon_alarm_timer(void *opaque)
     cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
 }
 
-PCIBus *typhoon_init(pci_map_irq_fn sys_map_irq, uint8_t devfn_min,
-                     TyphoonState *s)
+PCIBus *typhoon_init(uint8_t devfn_min, TyphoonState *s)
 {
+    TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_GET_CLASS(s);
     MemoryRegion *addr_space = get_system_memory();
     DeviceState *dev = DEVICE(s);
     PCIHostState *phb;
     PCIBus *b;
     int i;
 
+    assert(tc->sys_map_irq != NULL);
+
     phb = PCI_HOST_BRIDGE(dev);
 
     s->cchip.misc = 0x800000000ull; /* Revision: Typhoon.  */
@@ -899,7 +901,7 @@ PCIBus *typhoon_init(pci_map_irq_fn sys_map_irq, uint8_t devfn_min,
                                 &s->pchip.reg_io);
 
     b = pci_register_root_bus(dev, "pci",
-                              typhoon_set_irq, sys_map_irq, s,
+                              typhoon_set_irq, tc->sys_map_irq, s,
                               &s->pchip.reg_mem, &s->pchip.reg_io,
                               devfn_min, 64, TYPE_PCI_BUS);
     phb->bus = b;
@@ -978,6 +980,7 @@ static const TypeInfo typhoon_pcihost_info = {
     .instance_init = typhoon_pcihost_init,
     .class_size    = sizeof(TyphoonClass),
     .class_init    = typhoon_pcihost_class_init,
+    .abstract      = true,
 };
 
 static void typhoon_iommu_memory_region_class_init(ObjectClass *klass,

-- 
2.53.0

[PATCH 13/15] hw/alpha: Move minimum PCI device slot to TyphoonClass

[Yodel Eldar]

Move devfn_min to a member of TyphoonClass and set the
Clipper-specific value, PCI_DEVFN(1, 0), in its subclass init.

Eliminate devfn_min as a parameter of typhoon_init() to enable
its forthcoming split into instance_init and realize hooks.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h | 3 ++-
 hw/alpha/dp264.c     | 3 ++-
 hw/alpha/typhoon.c   | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index fdfe96dcb4..cf16e235fb 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -15,11 +15,12 @@ typedef struct TyphoonClass {
     PCIHostBridgeClass parent_class;
 
     pci_map_irq_fn sys_map_irq;
+    uint8_t devfn_min;
 } TyphoonClass;
 
 OBJECT_DECLARE_TYPE(TyphoonState, TyphoonClass, TYPHOON_PCI_HOST_BRIDGE)
 
-PCIBus *typhoon_init(uint8_t devfn_min, TyphoonState *);
+PCIBus *typhoon_init(TyphoonState *);
 
 #define TYPHOON_PROP_RAM "ram"
 
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 8c3c8fec67..de5829a004 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -111,7 +111,7 @@ static void clipper_init(MachineState *machine)
      * Init the chipset.  Because we're using CLIPPER IRQ mappings,
      * the minimum PCI device IdSel is 1.
      */
-    pci_bus = typhoon_init(PCI_DEVFN(1, 0), typhoon);
+    pci_bus = typhoon_init(typhoon);
 
     /*
      * Init the PCI -> ISA bridge.
@@ -259,6 +259,7 @@ static void typhoon_pcihost_clipper_class_init(ObjectClass *oc,
     TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_CLASS(oc);
 
     tc->sys_map_irq = clipper_pci_map_irq;
+    tc->devfn_min = PCI_DEVFN(1, 0);
 }
 
 static const TypeInfo clipper_types[] = {
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index b261dd7bc5..ad5010bea8 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -833,7 +833,7 @@ static void typhoon_alarm_timer(void *opaque)
     cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
 }
 
-PCIBus *typhoon_init(uint8_t devfn_min, TyphoonState *s)
+PCIBus *typhoon_init(TyphoonState *s)
 {
     TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_GET_CLASS(s);
     MemoryRegion *addr_space = get_system_memory();
@@ -903,7 +903,7 @@ PCIBus *typhoon_init(uint8_t devfn_min, TyphoonState *s)
     b = pci_register_root_bus(dev, "pci",
                               typhoon_set_irq, tc->sys_map_irq, s,
                               &s->pchip.reg_mem, &s->pchip.reg_io,
-                              devfn_min, 64, TYPE_PCI_BUS);
+                              tc->devfn_min, 64, TYPE_PCI_BUS);
     phb->bus = b;
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
 

-- 
2.53.0

[PATCH 14/15] hw/alpha: Use QOM path resolution to get Typhoon PCI bus

[Yodel Eldar]

Use object_resolve_path_component() in clipper_init() to retrieve
the PCI bus after it is realized in typhoon_init(). Modify
typhoon_init() such that it returns nothing instead of the PCI bus.

After this commit, we could cleanly split typhoon_init()
into the typhoon's instance_init and realize hooks.

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h | 3 ++-
 hw/alpha/dp264.c     | 4 +++-
 hw/alpha/typhoon.c   | 6 ++----
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index cf16e235fb..27c3a72b13 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -20,9 +20,10 @@ typedef struct TyphoonClass {
 
 OBJECT_DECLARE_TYPE(TyphoonState, TyphoonClass, TYPHOON_PCI_HOST_BRIDGE)
 
-PCIBus *typhoon_init(TyphoonState *);
+void typhoon_init(TyphoonState *);
 
 #define TYPHOON_PROP_RAM "ram"
+#define TYPHOON_PCI_BUS_NAME "pci"
 
 #define TYPHOON_GPIO_ISA_IRQ "isa-irq"
 #define TYPHOON_GPIO_RTC_IRQ "rtc-irq"
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index de5829a004..92a15b0239 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -111,7 +111,9 @@ static void clipper_init(MachineState *machine)
      * Init the chipset.  Because we're using CLIPPER IRQ mappings,
      * the minimum PCI device IdSel is 1.
      */
-    pci_bus = typhoon_init(typhoon);
+    typhoon_init(typhoon);
+    pci_bus = PCI_BUS(object_resolve_path_component(typhoon_obj,
+                                                    TYPHOON_PCI_BUS_NAME));
 
     /*
      * Init the PCI -> ISA bridge.
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index ad5010bea8..55f3aae584 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -833,7 +833,7 @@ static void typhoon_alarm_timer(void *opaque)
     cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
 }
 
-PCIBus *typhoon_init(TyphoonState *s)
+void typhoon_init(TyphoonState *s)
 {
     TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_GET_CLASS(s);
     MemoryRegion *addr_space = get_system_memory();
@@ -900,7 +900,7 @@ PCIBus *typhoon_init(TyphoonState *s)
     memory_region_add_subregion(addr_space, 0x801fc000000ULL,
                                 &s->pchip.reg_io);
 
-    b = pci_register_root_bus(dev, "pci",
+    b = pci_register_root_bus(dev, TYPHOON_PCI_BUS_NAME,
                               typhoon_set_irq, tc->sys_map_irq, s,
                               &s->pchip.reg_mem, &s->pchip.reg_io,
                               tc->devfn_min, 64, TYPE_PCI_BUS);
@@ -937,8 +937,6 @@ PCIBus *typhoon_init(TyphoonState *s)
     /* Pchip1 PCI special/interrupt acknowledge, 0x802.F800.0000, 64MB.  */
     /* Pchip1 PCI I/O, 0x802.FC00.0000, 32MB.  */
     /* Pchip1 PCI configuration, 0x802.FE00.0000, 16MB.  */
-
-    return b;
 }
 
 static void typhoon_pcihost_init(Object *obj)

-- 
2.53.0

[PATCH 15/15] hw/alpha: Split typhoon_init() into instance_init and realize

[Yodel Eldar]

Now that all of typhoon_init() inputs are either QOM link
properties or subclass members, split it between Typhoon's
instance_init and realize hooks.

Replace its invocation in clipper_init() with
sysbus_realize_and_unref().

Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
---
 hw/alpha/alpha_sys.h |   2 -
 hw/alpha/dp264.c     |   2 +-
 hw/alpha/typhoon.c   | 219 ++++++++++++++++++++++++++-------------------------
 3 files changed, 114 insertions(+), 109 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 27c3a72b13..fcc3abe95c 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -20,8 +20,6 @@ typedef struct TyphoonClass {
 
 OBJECT_DECLARE_TYPE(TyphoonState, TyphoonClass, TYPHOON_PCI_HOST_BRIDGE)
 
-void typhoon_init(TyphoonState *);
-
 #define TYPHOON_PROP_RAM "ram"
 #define TYPHOON_PCI_BUS_NAME "pci"
 
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 92a15b0239..c59528174f 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -111,7 +111,7 @@ static void clipper_init(MachineState *machine)
      * Init the chipset.  Because we're using CLIPPER IRQ mappings,
      * the minimum PCI device IdSel is 1.
      */
-    typhoon_init(typhoon);
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(typhoon), &error_fatal);
     pci_bus = PCI_BUS(object_resolve_path_component(typhoon_obj,
                                                     TYPHOON_PCI_BUS_NAME));
 
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 55f3aae584..e7a0bbfc1c 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -833,112 +833,6 @@ static void typhoon_alarm_timer(void *opaque)
     cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
 }
 
-void typhoon_init(TyphoonState *s)
-{
-    TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_GET_CLASS(s);
-    MemoryRegion *addr_space = get_system_memory();
-    DeviceState *dev = DEVICE(s);
-    PCIHostState *phb;
-    PCIBus *b;
-    int i;
-
-    assert(tc->sys_map_irq != NULL);
-
-    phb = PCI_HOST_BRIDGE(dev);
-
-    s->cchip.misc = 0x800000000ull; /* Revision: Typhoon.  */
-    s->pchip.win[3].wba = 2;        /* Window 3 SG always enabled. */
-
-    /* Remember the CPUs so that we can deliver interrupts to them.  */
-    for (i = 0; i < 4; i++) {
-        AlphaCPU *cpu = s->cchip.cpu[i];
-        if (cpu != NULL) {
-            cpu->alarm_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
-                                                 typhoon_alarm_timer,
-                                                 (void *)((uintptr_t)s + i));
-        }
-    }
-
-    /*
-     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
-     * but the address space hole reserved at this point is 8TB.
-     */
-    memory_region_add_subregion(addr_space, 0, s->ram);
-
-    /* TIGbus, 0x801.0000.0000, 1GB.  */
-    /*
-     * ??? The TIGbus is used for delivering interrupts, and access to
-     * the flash ROM.  I'm not sure that we need to implement it at all.
-     */
-
-    /* Pchip0 CSRs, 0x801.8000.0000, 256MB.  */
-    memory_region_init_io(&s->pchip.region, OBJECT(s), &pchip_ops, s, "pchip0",
-                          256 * MiB);
-    memory_region_add_subregion(addr_space, 0x80180000000ULL,
-                                &s->pchip.region);
-
-    /* Cchip CSRs, 0x801.A000.0000, 256MB.  */
-    memory_region_init_io(&s->cchip.region, OBJECT(s), &cchip_ops, s, "cchip0",
-                          256 * MiB);
-    memory_region_add_subregion(addr_space, 0x801a0000000ULL,
-                                &s->cchip.region);
-
-    /* Dchip CSRs, 0x801.B000.0000, 256MB.  */
-    memory_region_init_io(&s->dchip_region, OBJECT(s), &dchip_ops, s, "dchip0",
-                          256 * MiB);
-    memory_region_add_subregion(addr_space, 0x801b0000000ULL,
-                                &s->dchip_region);
-
-    /* Pchip0 PCI memory, 0x800.0000.0000, 4GB.  */
-    memory_region_init(&s->pchip.reg_mem, OBJECT(s), "pci0-mem", 4 * GiB);
-    memory_region_add_subregion(addr_space, 0x80000000000ULL,
-                                &s->pchip.reg_mem);
-
-    /* Pchip0 PCI I/O, 0x801.FC00.0000, 32MB.  */
-    memory_region_init_io(&s->pchip.reg_io, OBJECT(s), &alpha_pci_ignore_ops,
-                          NULL, "pci0-io", 32 * MiB);
-    memory_region_add_subregion(addr_space, 0x801fc000000ULL,
-                                &s->pchip.reg_io);
-
-    b = pci_register_root_bus(dev, TYPHOON_PCI_BUS_NAME,
-                              typhoon_set_irq, tc->sys_map_irq, s,
-                              &s->pchip.reg_mem, &s->pchip.reg_io,
-                              tc->devfn_min, 64, TYPE_PCI_BUS);
-    phb->bus = b;
-    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-
-    /* Host memory as seen from the PCI side, via the IOMMU.  */
-    memory_region_init_iommu(&s->pchip.iommu, sizeof(s->pchip.iommu),
-                             TYPE_TYPHOON_IOMMU_MEMORY_REGION, OBJECT(s),
-                             "iommu-typhoon", UINT64_MAX);
-    address_space_init(&s->pchip.iommu_as, MEMORY_REGION(&s->pchip.iommu),
-                       "pchip0-pci");
-    pci_setup_iommu(b, &typhoon_iommu_ops, s);
-
-    /* Pchip0 PCI special/interrupt acknowledge, 0x801.F800.0000, 64MB.  */
-    memory_region_init_io(&s->pchip.reg_iack, OBJECT(s), &alpha_pci_iack_ops,
-                          b, "pci0-iack", 64 * MiB);
-    memory_region_add_subregion(addr_space, 0x801f8000000ULL,
-                                &s->pchip.reg_iack);
-
-    /* Pchip0 PCI configuration, 0x801.FE00.0000, 16MB.  */
-    memory_region_init_io(&s->pchip.reg_conf, OBJECT(s), &alpha_pci_conf1_ops,
-                          b, "pci0-conf", 16 * MiB);
-    memory_region_add_subregion(addr_space, 0x801fe000000ULL,
-                                &s->pchip.reg_conf);
-
-    /*
-     * For the record, these are the mappings for the second PCI bus.
-     * We can get away with not implementing them because we indicate
-     * via the Cchip.CSC<PIP> bit that Pchip1 is not present.
-     */
-    /* Pchip1 PCI memory, 0x802.0000.0000, 4GB.  */
-    /* Pchip1 CSRs, 0x802.8000.0000, 256MB.  */
-    /* Pchip1 PCI special/interrupt acknowledge, 0x802.F800.0000, 64MB.  */
-    /* Pchip1 PCI I/O, 0x802.FC00.0000, 32MB.  */
-    /* Pchip1 PCI configuration, 0x802.FE00.0000, 16MB.  */
-}
-
 static void typhoon_pcihost_init(Object *obj)
 {
     TyphoonState *s = TYPHOON_PCI_HOST_BRIDGE(obj);
@@ -955,6 +849,118 @@ static void typhoon_pcihost_init(Object *obj)
     qdev_init_gpio_in_named(dev, typhoon_set_isa_irq, TYPHOON_GPIO_ISA_IRQ, 1);
     qdev_init_gpio_in_named(dev, typhoon_set_timer_irq, TYPHOON_GPIO_RTC_IRQ,
                             1);
+
+    s->cchip.misc = 0x800000000ull; /* Revision: Typhoon.  */
+    s->pchip.win[3].wba = 2;        /* Window 3 SG always enabled. */
+
+    /* TIGbus, 0x801.0000.0000, 1GB.  */
+    /*
+     * ??? The TIGbus is used for delivering interrupts, and access to
+     * the flash ROM.  I'm not sure that we need to implement it at all.
+     */
+
+    /* Pchip0 CSRs, 0x801.8000.0000, 256MB.  */
+    memory_region_init_io(&s->pchip.region, OBJECT(s), &pchip_ops, s, "pchip0",
+                          256 * MiB);
+
+    /* Cchip CSRs, 0x801.A000.0000, 256MB.  */
+    memory_region_init_io(&s->cchip.region, OBJECT(s), &cchip_ops, s, "cchip0",
+                          256 * MiB);
+
+    /* Dchip CSRs, 0x801.B000.0000, 256MB.  */
+    memory_region_init_io(&s->dchip_region, OBJECT(s), &dchip_ops, s, "dchip0",
+                          256 * MiB);
+
+    /* Pchip0 PCI memory, 0x800.0000.0000, 4GB.  */
+    memory_region_init(&s->pchip.reg_mem, OBJECT(s), "pci0-mem", 4 * GiB);
+
+    /* Pchip0 PCI I/O, 0x801.FC00.0000, 32MB.  */
+    memory_region_init_io(&s->pchip.reg_io, OBJECT(s), &alpha_pci_ignore_ops,
+                          NULL, "pci0-io", 32 * MiB);
+
+    /* Host memory as seen from the PCI side, via the IOMMU.  */
+    memory_region_init_iommu(&s->pchip.iommu, sizeof(s->pchip.iommu),
+                             TYPE_TYPHOON_IOMMU_MEMORY_REGION, OBJECT(s),
+                             "iommu-typhoon", UINT64_MAX);
+    address_space_init(&s->pchip.iommu_as, MEMORY_REGION(&s->pchip.iommu),
+                       "pchip0-pci");
+}
+
+static void typhoon_pcihost_realize(DeviceState *dev, Error **errp)
+{
+    TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_GET_CLASS(dev);
+    TyphoonState *s = TYPHOON_PCI_HOST_BRIDGE(dev);
+    MemoryRegion *addr_space = get_system_memory();
+    PCIHostState *phb = PCI_HOST_BRIDGE(dev);
+    PCIBus *b;
+    int i;
+    const char *typename = object_get_typename(OBJECT(s));
+
+    if (s->cchip.cpu[0] == NULL) {
+        error_setg(errp, "%s: \"cchip.cpu[0]\" link is not set", typename);
+        return;
+    }
+
+    if (s->ram == NULL) {
+        error_setg(errp, "%s: \"ram\" link is not set.", typename);
+        return;
+    }
+
+    if (tc->sys_map_irq == NULL) {
+        error_setg(errp, "%s: \"sys_map_irq\" setter has not been called.",
+                   typename);
+        return;
+    }
+
+    b = pci_register_root_bus(dev, TYPHOON_PCI_BUS_NAME,
+                              typhoon_set_irq, tc->sys_map_irq, s,
+                              &s->pchip.reg_mem, &s->pchip.reg_io,
+                              tc->devfn_min, 64, TYPE_PCI_BUS);
+    phb->bus = b;
+
+    /* Remember the CPUs so that we can deliver interrupts to them.  */
+    for (i = 0; i < 4; i++) {
+        AlphaCPU *cpu = s->cchip.cpu[i];
+        if (cpu != NULL) {
+            cpu->alarm_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+                                            typhoon_alarm_timer,
+                                            (void *)((uintptr_t)s + i));
+        }
+    }
+    /*
+     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
+     * but the address space hole reserved at this point is 8TB.
+     */
+    memory_region_add_subregion(addr_space, 0, s->ram);
+
+    memory_region_add_subregion(addr_space, 0x80180000000ULL,
+                                &s->pchip.region);
+
+    memory_region_add_subregion(addr_space, 0x801a0000000ULL,
+                                &s->cchip.region);
+
+    memory_region_add_subregion(addr_space, 0x801b0000000ULL,
+                                &s->dchip_region);
+
+    memory_region_add_subregion(addr_space, 0x80000000000ULL,
+                                &s->pchip.reg_mem);
+
+    memory_region_add_subregion(addr_space, 0x801fc000000ULL,
+                                &s->pchip.reg_io);
+
+    pci_setup_iommu(b, &typhoon_iommu_ops, s);
+
+    /* Pchip0 PCI special/interrupt acknowledge, 0x801.F800.0000, 64MB.  */
+    memory_region_init_io(&s->pchip.reg_iack, OBJECT(s), &alpha_pci_iack_ops,
+                          b, "pci0-iack", 64 * MiB);
+    memory_region_add_subregion(addr_space, 0x801f8000000ULL,
+                                &s->pchip.reg_iack);
+
+    /* Pchip0 PCI configuration, 0x801.FE00.0000, 16MB.  */
+    memory_region_init_io(&s->pchip.reg_conf, OBJECT(s), &alpha_pci_conf1_ops,
+                          b, "pci0-conf", 16 * MiB);
+    memory_region_add_subregion(addr_space, 0x801fe000000ULL,
+                                &s->pchip.reg_conf);
 }
 
 static const Property typhoon_properties[] = {
@@ -967,6 +973,7 @@ static void typhoon_pcihost_class_init(ObjectClass *klass, const void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->user_creatable = false;
+    dc->realize = typhoon_pcihost_realize;
 
     device_class_set_props(dc, typhoon_properties);
 }

-- 
2.53.0

Re: [PATCH 10/15] hw/alpha: Expose RAM to typhoon via QOM property link

[Philippe Mathieu-Daudé]

On 10/3/26 23:31, Yodel Eldar wrote:
> Currently, the board (Clipper/DP264) passes machine->ram to the
> chipset (Typhoon) as an arg to typhoon_init(), where it is
> added as a subregion of system memory. This commit, instead adds
> a ram pointer in TyphoonState, defines it as a Property, and sets
> a link to it from the machine, thus removing the ram parameter.
> 
> Note: Moving the ram subregion mapping into machine code was
> considered but decided against so as to preserve the Typhoon's
> historical role in mapping the address space.
> 
> Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
> ---
>   hw/alpha/alpha_sys.h |  5 +++--
>   hw/alpha/dp264.c     |  8 +++++---
>   hw/alpha/typhoon.c   | 14 +++++++++++---
>   3 files changed, 19 insertions(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 05/15] hw/alpha/typhoon: Create instance_init and class_init

[Philippe Mathieu-Daudé]

On 10/3/26 23:31, Yodel Eldar wrote:
> Define the Typhoon class and instance initializers.
> 
> Add "cpu[*]" link properties in the instance_init of Typhoon.
> Setting these cpu link properties will occur in a later commit.
> 
> Note: This is one step towards splitting typhoon_init() into the
> instance_init and realize hooks. typhoon_init() currently takes
> a fixed cpus array as a parameter; that array will be removed soon.
> 
> Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
> ---
>   hw/alpha/typhoon.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)


> +static void typhoon_pcihost_init(Object *obj)
> +{
> +    TyphoonState *s = TYPHOON_PCI_HOST_BRIDGE(obj);
> +
> +    int i;
> +    for (i = 0; i < 4; ++i) {
> +        g_autofree char *name = g_strdup_printf("cpu[%d]", i);
> +        object_property_add_link(obj, name,  TYPE_ALPHA_CPU,
> +                                 (Object **)&s->cchip.cpu[i],
> +                                 qdev_prop_allow_set_link_before_realize, 0);
> +    }
> +}

Hmm this machine is modelled using quite legacy style. The Typhoon
PCIHost object shouldn't have direct access to CPUs but only expose
IRQ lines, then the board layer wire these lines to CPUs.

Re: [PATCH 14/15] hw/alpha: Use QOM path resolution to get Typhoon PCI bus

[Yodel Eldar]

Hi,

On 10/03/2026 17:31, Yodel Eldar wrote:
> Use object_resolve_path_component() in clipper_init() to retrieve
> the PCI bus after it is realized in typhoon_init(). Modify
> typhoon_init() such that it returns nothing instead of the PCI bus.
> 
> After this commit, we could cleanly split typhoon_init()
> into the typhoon's instance_init and realize hooks.
> 
> Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
> ---
>   hw/alpha/alpha_sys.h | 3 ++-
>   hw/alpha/dp264.c     | 4 +++-
>   hw/alpha/typhoon.c   | 6 ++----
>   3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
> index cf16e235fb..27c3a72b13 100644
> --- a/hw/alpha/alpha_sys.h
> +++ b/hw/alpha/alpha_sys.h
> @@ -20,9 +20,10 @@ typedef struct TyphoonClass {
>   
>   OBJECT_DECLARE_TYPE(TyphoonState, TyphoonClass, TYPHOON_PCI_HOST_BRIDGE)
>   
> -PCIBus *typhoon_init(TyphoonState *);
> +void typhoon_init(TyphoonState *);
>   
>   #define TYPHOON_PROP_RAM "ram"
> +#define TYPHOON_PCI_BUS_NAME "pci"
>   
>   #define TYPHOON_GPIO_ISA_IRQ "isa-irq"
>   #define TYPHOON_GPIO_RTC_IRQ "rtc-irq"
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index de5829a004..92a15b0239 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -111,7 +111,9 @@ static void clipper_init(MachineState *machine)
>        * Init the chipset.  Because we're using CLIPPER IRQ mappings,
>        * the minimum PCI device IdSel is 1.
>        */
> -    pci_bus = typhoon_init(typhoon);
> +    typhoon_init(typhoon);
> +    pci_bus = PCI_BUS(object_resolve_path_component(typhoon_obj,
> +                                                    TYPHOON_PCI_BUS_NAME));

Scratch this; for v2:

      pci_bus = PCI_HOST_BRIDGE(typhoon_obj)->bus;

>   
>       /*
>        * Init the PCI -> ISA bridge.
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index ad5010bea8..55f3aae584 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -833,7 +833,7 @@ static void typhoon_alarm_timer(void *opaque)
>       cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
>   }
>   
> -PCIBus *typhoon_init(TyphoonState *s)
> +void typhoon_init(TyphoonState *s)
>   {
>       TyphoonClass *tc = TYPHOON_PCI_HOST_BRIDGE_GET_CLASS(s);
>       MemoryRegion *addr_space = get_system_memory();
> @@ -900,7 +900,7 @@ PCIBus *typhoon_init(TyphoonState *s)
>       memory_region_add_subregion(addr_space, 0x801fc000000ULL,
>                                   &s->pchip.reg_io);
>   
> -    b = pci_register_root_bus(dev, "pci",
> +    b = pci_register_root_bus(dev, TYPHOON_PCI_BUS_NAME,
>                                 typhoon_set_irq, tc->sys_map_irq, s,
>                                 &s->pchip.reg_mem, &s->pchip.reg_io,
>                                 tc->devfn_min, 64, TYPE_PCI_BUS);
> @@ -937,8 +937,6 @@ PCIBus *typhoon_init(TyphoonState *s)
>       /* Pchip1 PCI special/interrupt acknowledge, 0x802.F800.0000, 64MB.  */
>       /* Pchip1 PCI I/O, 0x802.FC00.0000, 32MB.  */
>       /* Pchip1 PCI configuration, 0x802.FE00.0000, 16MB.  */
> -
> -    return b;
>   }
>   
>   static void typhoon_pcihost_init(Object *obj)
> 

Re: [PATCH 05/15] hw/alpha/typhoon: Create instance_init and class_init

[Yodel Eldar]

Hi, Philippe! Thanks for commenting here and your review
of Patch 10.

On 11/03/2026 07:43, Philippe Mathieu-Daudé wrote:
> On 10/3/26 23:31, Yodel Eldar wrote:
>> Define the Typhoon class and instance initializers.
>>
>> Add "cpu[*]" link properties in the instance_init of Typhoon.
>> Setting these cpu link properties will occur in a later commit.
>>
>> Note: This is one step towards splitting typhoon_init() into the
>> instance_init and realize hooks. typhoon_init() currently takes
>> a fixed cpus array as a parameter; that array will be removed soon.
>>
>> Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
>> ---
>>   hw/alpha/typhoon.c | 23 +++++++++++++++++++++++
>>   1 file changed, 23 insertions(+)
> 
> 
>> +static void typhoon_pcihost_init(Object *obj)
>> +{
>> +    TyphoonState *s = TYPHOON_PCI_HOST_BRIDGE(obj);
>> +
>> +    int i;
>> +    for (i = 0; i < 4; ++i) {
>> +        g_autofree char *name = g_strdup_printf("cpu[%d]", i);
>> +        object_property_add_link(obj, name,  TYPE_ALPHA_CPU,
>> +                                 (Object **)&s->cchip.cpu[i],
>> +                                 
>> qdev_prop_allow_set_link_before_realize, 0);
>> +    }
>> +}
> 
> Hmm this machine is modelled using quite legacy style. The Typhoon
> PCIHost object shouldn't have direct access to CPUs but only expose
> IRQ lines, then the board layer wire these lines to CPUs.

Yeah, by inheritance the Typhoon is a PCI_HOST_BRIDGE, but
it's doing much more than that, so it's currently tightly
integrated with the CPUs.

I narrowly limited the scope of the series, so I hadn't even
considered decoupling the CPUs before your comment, but now
that I've been looking at it more closely: AFAICT there are
three nexuses we'd have to disentangle (please let me know
if I missed anything):
  (1) the CPU interrupts you mentioned
  (2) the timers that live in the CPUs (for now)
  (3) returning the cpu index when reading the MISC register

I think (1) and (2) are relatively straightforward: connect the
wires as you suggested with the IRQ handler in the cpu, and move
the timers into TyphoonState, respectively. (3)'s a bit trickier
IMHO: I suppose we could keep using current_cpu for (3) since
resolving (1) and (2) already lets us eliminate TyphoonState's
cchip.cpu, but it would be nice to remove that, too.
Open to suggestions. WDYT?

Thanks,
Yodel

Re: [PATCH 01/15] hw/alpha/typhoon: Fix whitespace and block comment style problems

[Richard Henderson]

On 3/11/26 08:31, Yodel Eldar wrote:
> Eliminate trailing whitespaces.
> 
> Use leading "/*", internal "*", and trailing "*/" for block comments.
> 
> Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
> ---
>   hw/alpha/typhoon.c | 84 +++++++++++++++++++++++++++++++++---------------------
>   1 file changed, 52 insertions(+), 32 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> 
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index 7722d4fd00..f392772500 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -34,7 +34,7 @@ typedef struct TyphoonWindow {
>       uint64_t wsm;
>       uint64_t tba;
>   } TyphoonWindow;
> -
> +
>   typedef struct TyphoonPchip {
>       MemoryRegion region;
>       MemoryRegion reg_iack;
> @@ -84,8 +84,10 @@ static MemTxResult cchip_read(void *opaque, hwaddr addr,
>       switch (addr) {
>       case 0x0000:
>           /* CSC: Cchip System Configuration Register.  */
> -        /* All sorts of data here; probably the only thing relevant is
> -           PIP<14> Pchip 1 Present = 0.  */
> +        /*
> +         * All sorts of data here; probably the only thing relevant is
> +         * PIP<14> Pchip 1 Present = 0.
> +         */
>           break;
>   
>       case 0x0040:
> @@ -188,7 +190,7 @@ static MemTxResult cchip_read(void *opaque, hwaddr addr,
>       case 0x0780:
>           /* PWR: Power Management Control.   */
>           break;
> -
> +
>       case 0x0c00: /* CMONCTLA */
>       case 0x0c40: /* CMONCTLB */
>       case 0x0c80: /* CMONCNT01 */
> @@ -440,7 +442,7 @@ static MemTxResult cchip_write(void *opaque, hwaddr addr,
>       case 0x0780:
>           /* PWR: Power Management Control.   */
>           break;
> -
> +
>       case 0x0c00: /* CMONCTLA */
>       case 0x0c40: /* CMONCTLB */
>       case 0x0c80: /* CMONCNT01 */
> @@ -605,8 +607,10 @@ static const MemoryRegionOps pchip_ops = {
>       },
>   };
>   
> -/* A subroutine of typhoon_translate_iommu that builds an IOMMUTLBEntry
> -   using the given translated address and mask.  */
> +/*
> + * A subroutine of typhoon_translate_iommu that builds an IOMMUTLBEntry
> + * using the given translated address and mask.
> + */
>   static bool make_iommu_tlbe(hwaddr taddr, hwaddr mask, IOMMUTLBEntry *ret)
>   {
>       *ret = (IOMMUTLBEntry) {
> @@ -618,8 +622,10 @@ static bool make_iommu_tlbe(hwaddr taddr, hwaddr mask, IOMMUTLBEntry *ret)
>       return true;
>   }
>   
> -/* A subroutine of typhoon_translate_iommu that handles scatter-gather
> -   translation, given the address of the PTE.  */
> +/*
> + * A subroutine of typhoon_translate_iommu that handles scatter-gather
> + * translation, given the address of the PTE.
> + */
>   static bool pte_translate(hwaddr pte_addr, IOMMUTLBEntry *ret)
>   {
>       uint64_t pte = address_space_ldq_le(&address_space_memory, pte_addr,
> @@ -633,8 +639,10 @@ static bool pte_translate(hwaddr pte_addr, IOMMUTLBEntry *ret)
>       return make_iommu_tlbe((pte & 0x3ffffe) << 12, 0x1fff, ret);
>   }
>   
> -/* A subroutine of typhoon_translate_iommu that handles one of the
> -   four single-address-cycle translation windows.  */
> +/*
> + * A subroutine of typhoon_translate_iommu that handles one of the
> + * four single-address-cycle translation windows.
> + */
>   static bool window_translate(TyphoonWindow *win, hwaddr addr,
>                                IOMMUTLBEntry *ret)
>   {
> @@ -668,8 +676,10 @@ static bool window_translate(TyphoonWindow *win, hwaddr addr,
>   }
>   
>   /* Handle PCI-to-system address translation.  */
> -/* TODO: A translation failure here ought to set PCI error codes on the
> -   Pchip and generate a machine check interrupt.  */
> +/*
> + * TODO: A translation failure here ought to set PCI error codes on the
> + * Pchip and generate a machine check interrupt.
> + */
>   static IOMMUTLBEntry typhoon_translate_iommu(IOMMUMemoryRegion *iommu,
>                                                hwaddr addr,
>                                                IOMMUAccessFlags flag,
> @@ -773,10 +783,12 @@ static void typhoon_set_timer_irq(void *opaque, int irq, int level)
>       TyphoonState *s = opaque;
>       int i;
>   
> -    /* Thankfully, the mc146818rtc code doesn't track the IRQ state,
> -       and so we don't have to worry about missing interrupts just
> -       because we never actually ACK the interrupt.  Just ignore any
> -       case of the interrupt level going low.  */
> +    /*
> +     * Thankfully, the mc146818rtc code doesn't track the IRQ state,
> +     * and so we don't have to worry about missing interrupts just
> +     * because we never actually ACK the interrupt.  Just ignore any
> +     * case of the interrupt level going low.
> +     */
>       if (level == 0) {
>           return;
>       }
> @@ -787,14 +799,16 @@ static void typhoon_set_timer_irq(void *opaque, int irq, int level)
>           if (cpu != NULL) {
>               uint32_t iic = s->cchip.iic[i];
>   
> -            /* ??? The verbage in Section 10.2.2.10 isn't 100% clear.
> -               Bit 24 is the OverFlow bit, RO, and set when the count
> -               decrements past 0.  When is OF cleared?  My guess is that
> -               OF is actually cleared when the IIC is written, and that
> -               the ICNT field always decrements.  At least, that's an
> -               interpretation that makes sense, and "allows the CPU to
> -               determine exactly how mant interval timer ticks were
> -               skipped".  At least within the next 4M ticks...  */
> +            /*
> +             * ??? The verbage in Section 10.2.2.10 isn't 100% clear.
> +             * Bit 24 is the OverFlow bit, RO, and set when the count
> +             * decrements past 0.  When is OF cleared?  My guess is that
> +             * OF is actually cleared when the IIC is written, and that
> +             * the ICNT field always decrements.  At least, that's an
> +             * interpretation that makes sense, and "allows the CPU to
> +             * determine exactly how mant interval timer ticks were
> +             * skipped".  At least within the next 4M ticks...
> +             */
>   
>               iic = ((iic - 1) & 0x1ffffff) | (iic & 0x1000000);
>               s->cchip.iic[i] = iic;
> @@ -852,13 +866,17 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
>       *p_isa_irq = qemu_allocate_irq(typhoon_set_isa_irq, s, 0);
>       *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
>   
> -    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> -       but the address space hole reserved at this point is 8TB.  */
> +    /*
> +     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> +     * but the address space hole reserved at this point is 8TB.
> +     */
>       memory_region_add_subregion(addr_space, 0, ram);
>   
>       /* TIGbus, 0x801.0000.0000, 1GB.  */
> -    /* ??? The TIGbus is used for delivering interrupts, and access to
> -       the flash ROM.  I'm not sure that we need to implement it at all.  */
> +    /*
> +     * ??? The TIGbus is used for delivering interrupts, and access to
> +     * the flash ROM.  I'm not sure that we need to implement it at all.
> +     */
>   
>       /* Pchip0 CSRs, 0x801.8000.0000, 256MB.  */
>       memory_region_init_io(&s->pchip.region, OBJECT(s), &pchip_ops, s, "pchip0",
> @@ -916,9 +934,11 @@ PCIBus *typhoon_init(MemoryRegion *ram, qemu_irq *p_isa_irq,
>       memory_region_add_subregion(addr_space, 0x801fe000000ULL,
>                                   &s->pchip.reg_conf);
>   
> -    /* For the record, these are the mappings for the second PCI bus.
> -       We can get away with not implementing them because we indicate
> -       via the Cchip.CSC<PIP> bit that Pchip1 is not present.  */
> +    /*
> +     * For the record, these are the mappings for the second PCI bus.
> +     * We can get away with not implementing them because we indicate
> +     * via the Cchip.CSC<PIP> bit that Pchip1 is not present.
> +     */
>       /* Pchip1 PCI memory, 0x802.0000.0000, 4GB.  */
>       /* Pchip1 CSRs, 0x802.8000.0000, 256MB.  */
>       /* Pchip1 PCI special/interrupt acknowledge, 0x802.F800.0000, 64MB.  */
> 

Re: [PATCH 02/15] hw/alpha/dp264: Fix block comment style problems

[Richard Henderson]

On 3/11/26 08:31, Yodel Eldar wrote:
> Use leading "/*", internal "*", and trailing "*/" for block comments.
> 
> Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
> ---
>   hw/alpha/dp264.c | 22 +++++++++++++---------
>   1 file changed, 13 insertions(+), 9 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> 
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 98219f0456..27fbcee637 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -26,12 +26,14 @@ static uint64_t cpu_alpha_superpage_to_phys(void *opaque, uint64_t addr)
>       return addr;
>   }
>   
> -/* Note that there are at least 3 viewpoints of IRQ numbers on Alpha systems.
> -    (0) The dev_irq_n lines into the cpu, which we totally ignore,
> -    (1) The DRIR lines in the typhoon chipset,
> -    (2) The "vector" aka mangled interrupt number reported by SRM PALcode,
> -    (3) The interrupt number assigned by the kernel.
> -   The following function is concerned with (1) only.  */
> +/*
> + * Note that there are at least 3 viewpoints of IRQ numbers on Alpha systems.
> + *  (0) The dev_irq_n lines into the cpu, which we totally ignore,
> + *  (1) The DRIR lines in the typhoon chipset,
> + *  (2) The "vector" aka mangled interrupt number reported by SRM PALcode,
> + *  (3) The interrupt number assigned by the kernel.
> + * The following function is concerned with (1) only.
> + */
>   
>   static int clipper_pci_map_irq(PCIDevice *d, int irq_num)
>   {
> @@ -132,9 +134,11 @@ static void clipper_init(MachineState *machine)
>       pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide");
>       pci_ide_create_devs(pci_dev);
>   
> -    /* Load PALcode.  Given that this is not "real" cpu palcode,
> -       but one explicitly written for the emulation, we might as
> -       well load it directly from and ELF image.  */
> +    /*
> +     * Load PALcode.  Given that this is not "real" cpu palcode,
> +     * but one explicitly written for the emulation, we might as
> +     * well load it directly from and ELF image.
> +     */
>       palcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS,
>                                         machine->firmware ?: "palcode-clipper");
>       if (palcode_filename == NULL) {
> 

Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes

[Richard Henderson]

On 3/11/26 08:31, Yodel Eldar wrote:
> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>               }
>   
>               /* Put the initrd image as high in memory as possible.  */
> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
> +            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
> +                error_report("initial ram disk exceeds allotted ram size");
> +                exit(1);
> +            }
> +            initrd_base &= TARGET_PAGE_MASK;
> +            if (initrd_base <= kernel_high) {
> +                warn_report("initial ram disk overlaps with kernel");
> +            }

Why is the first an error and the second a warning?


r~

Re: [PATCH 04/15] hw/alpha/dp264: Use rom_add_blob_fixed() for initrd params

[Richard Henderson]

On 3/11/26 08:31, Yodel Eldar wrote:
> Replace address_space_stq_le() invocations with rom_add_blob_fixed().
> This will prove helpful when the Clipper becomes resettable, because
> the params will be reloaded without the initrd file.
> 
> Signed-off-by: Yodel Eldar <yodel.eldar@yodel.dev>
> ---
>   hw/alpha/dp264.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 87af919895..f036d72268 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -183,6 +183,7 @@ static void clipper_init(MachineState *machine)
>           if (initrd_filename) {
>               hwaddr initrd_base;
>               int64_t initrd_size;
> +            uint64_t initrd_info[2];
>   
>               initrd_size = get_image_size(initrd_filename, NULL);
>               if (initrd_size < 0) {
> @@ -204,11 +205,11 @@ static void clipper_init(MachineState *machine)
>               load_image_targphys(initrd_filename, initrd_base,
>                                   ram_size - initrd_base, NULL);
>   
> -            address_space_stq_le(&address_space_memory, param_offset + 0x100,
> -                                 initrd_base + 0xfffffc0000000000ULL,
> -                                 MEMTXATTRS_UNSPECIFIED, NULL);
> -            address_space_stq_le(&address_space_memory, param_offset + 0x108,
> -                                 initrd_size, MEMTXATTRS_UNSPECIFIED, NULL);
> +            stq_le_p(&initrd_info[0], initrd_base + 0xfffffc0000000000ULL);
> +            stq_le_p(&initrd_info[1], initrd_size);

You don't need stq_*_p, as initrd_info will never be misaligned.
Use

     initrd_info[0] = cpu_to_le64(initrd_base + 0xfffffc0000000000ULL);

etc


r~

Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes

[Yodel Eldar]

Hi, Richard

On 25/03/2026 19:11, Richard Henderson wrote:
> On 3/11/26 08:31, Yodel Eldar wrote:
>> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>>               }
>>               /* Put the initrd image as high in memory as possible.  */
>> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
>> +            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
>> +                error_report("initial ram disk exceeds allotted ram 
>> size");
>> +                exit(1);
>> +            }
>> +            initrd_base &= TARGET_PAGE_MASK;
>> +            if (initrd_base <= kernel_high) {
>> +                warn_report("initial ram disk overlaps with kernel");
>> +            }
> 
> Why is the first an error and the second a warning?
> 

Initially, I had both as fatal, but then I figured a (mis)adventurous
user may actually want to experiment with kernel/initrd overlap, whereas
an underflow is something we can't allow (and initrd being larger than
RAM is physically impossible AFAIK). I could make the overlap fatal,
too, if that's better?

Thanks for the reviews in Patches 1-2, and for letting me know about
using cpu_to_le64 in Patch 4. v2 has seen some nontrivial changes in
the later patches, and I'll be sending it soon.

Thanks again,
Yodel

> 
> r~
> 

Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes

[Richard Henderson]

On 3/26/26 12:04, Yodel Eldar wrote:
> Hi, Richard
> 
> On 25/03/2026 19:11, Richard Henderson wrote:
>> On 3/11/26 08:31, Yodel Eldar wrote:
>>> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>>>               }
>>>               /* Put the initrd image as high in memory as possible.  */
>>> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
>>> +            if (usub64_overflow(ram_size, initrd_size, &initrd_base)) {
>>> +                error_report("initial ram disk exceeds allotted ram size");
>>> +                exit(1);
>>> +            }
>>> +            initrd_base &= TARGET_PAGE_MASK;
>>> +            if (initrd_base <= kernel_high) {
>>> +                warn_report("initial ram disk overlaps with kernel");
>>> +            }
>>
>> Why is the first an error and the second a warning?
>>
> 
> Initially, I had both as fatal, but then I figured a (mis)adventurous
> user may actually want to experiment with kernel/initrd overlap, whereas
> an underflow is something we can't allow (and initrd being larger than
> RAM is physically impossible AFAIK). I could make the overlap fatal,
> too, if that's better?

I can't see it being useful at all.


r~

Re: [PATCH 03/15] hw/alpha/dp264: Validate kernel and initrd sizes

[Yodel Eldar]

On 25/03/2026 22:07, Richard Henderson wrote:
> On 3/26/26 12:04, Yodel Eldar wrote:
>> Hi, Richard
>>
>> On 25/03/2026 19:11, Richard Henderson wrote:
>>> On 3/11/26 08:31, Yodel Eldar wrote:
>>>> @@ -192,7 +192,15 @@ static void clipper_init(MachineState *machine)
>>>>               }
>>>>               /* Put the initrd image as high in memory as 
>>>> possible.  */
>>>> -            initrd_base = (ram_size - initrd_size) & TARGET_PAGE_MASK;
>>>> +            if (usub64_overflow(ram_size, initrd_size, 
>>>> &initrd_base)) {
>>>> +                error_report("initial ram disk exceeds allotted ram 
>>>> size");
>>>> +                exit(1);
>>>> +            }
>>>> +            initrd_base &= TARGET_PAGE_MASK;
>>>> +            if (initrd_base <= kernel_high) {
>>>> +                warn_report("initial ram disk overlaps with kernel");
>>>> +            }
>>>
>>> Why is the first an error and the second a warning?
>>>
>>
>> Initially, I had both as fatal, but then I figured a (mis)adventurous
>> user may actually want to experiment with kernel/initrd overlap, whereas
>> an underflow is something we can't allow (and initrd being larger than
>> RAM is physically impossible AFAIK). I could make the overlap fatal,
>> too, if that's better?
> 
> I can't see it being useful at all.
> 

Heh, fair enough; fatal, it is, then.

Thanks,
Yodel

> 
> r~
>