Re: [SPAM] [PATCH v4 04/14] hw/pci-host/aspeed: Add AST2600 PCIe Root Device support

["Cédric Le Goater" <clg@redhat.com>]

On 9/19/25 05:24, Jamin Lin wrote:
> Introduce a PCIe Root Device for AST2600 platform.
> 
> The AST2600 root complex exposes a PCIe root device at bus 80, devfn 0.
> This root device is implemented as a child of the PCIe RC and modeled
> as a host bridge PCI function (class_id = PCI_CLASS_BRIDGE_HOST).
> 
> Key changes:
> - Add a new device type "aspeed.pcie-root-device".
> - Instantiate the root device as part of AspeedPCIERcState.
> - Initialize it during RC realize() and attach it to the root bus.
> - Mark the root device as non-user-creatable.
> - Add RC boolean property "has-rd" to control whether the Root Device is
>    created (platforms can enable/disable it as needed).
> 
> Note: Only AST2600 implements this PCIe root device. AST2700 does not
> provide one.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/pci-host/aspeed_pcie.h | 11 ++++++
>   hw/pci-host/aspeed_pcie.c         | 56 +++++++++++++++++++++++++++++++
>   2 files changed, 67 insertions(+)
> 
> diff --git a/include/hw/pci-host/aspeed_pcie.h b/include/hw/pci-host/aspeed_pcie.h
> index 850d579189..fe30ac02ae 100644
> --- a/include/hw/pci-host/aspeed_pcie.h
> +++ b/include/hw/pci-host/aspeed_pcie.h
> @@ -40,6 +40,13 @@ typedef struct AspeedPCIERegMap {
>       AspeedPCIERcRegs rc;
>   } AspeedPCIERegMap;
>   
> +#define TYPE_ASPEED_PCIE_ROOT_DEVICE "aspeed.pcie-root-device"
> +OBJECT_DECLARE_SIMPLE_TYPE(AspeedPCIERootDeviceState, ASPEED_PCIE_ROOT_DEVICE);
> +
> +struct AspeedPCIERootDeviceState {
> +    PCIBridge parent_obj;
> +};
> +
>   #define TYPE_ASPEED_PCIE_RC "aspeed.pcie-rc"
>   OBJECT_DECLARE_SIMPLE_TYPE(AspeedPCIERcState, ASPEED_PCIE_RC);
>   
> @@ -53,7 +60,10 @@ struct AspeedPCIERcState {
>   
>       uint32_t bus_nr;
>       char name[16];
> +    bool has_rd;
>       qemu_irq irq;
> +
> +    AspeedPCIERootDeviceState root_device;
>   };
>   
>   /* Bridge between AHB bus and PCIe RC. */
> @@ -79,6 +89,7 @@ struct AspeedPCIECfgClass {
>   
>       uint64_t rc_bus_nr;
>       uint64_t nr_regs;
> +    bool rc_has_rd;
>   };
>   
>   #define TYPE_ASPEED_PCIE_PHY "aspeed.pcie-phy"
> diff --git a/hw/pci-host/aspeed_pcie.c b/hw/pci-host/aspeed_pcie.c
> index c3e92ee449..6e563a07a3 100644
> --- a/hw/pci-host/aspeed_pcie.c
> +++ b/hw/pci-host/aspeed_pcie.c
> @@ -25,6 +25,44 @@
>   #include "hw/pci/msi.h"
>   #include "trace.h"
>   
> +/*
> + * PCIe Root Device
> + * This device exists only on AST2600.
> + */
> +
> +static void aspeed_pcie_root_device_class_init(ObjectClass *klass,
> +                                               const void *data)
> +{
> +    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> +    dc->desc = "ASPEED PCIe Root Device";
> +    k->vendor_id = PCI_VENDOR_ID_ASPEED;
> +    k->device_id = 0x2600;
> +    k->class_id = PCI_CLASS_BRIDGE_HOST;
> +    k->subsystem_vendor_id = k->vendor_id;
> +    k->subsystem_id = k->device_id;
> +    k->revision = 0;
> +
> +    /*
> +     * PCI-facing part of the host bridge,
> +     * not usable without the host-facing part
> +     */
> +    dc->user_creatable = false;
> +}
> +
> +static const TypeInfo aspeed_pcie_root_device_info = {
> +    .name = TYPE_ASPEED_PCIE_ROOT_DEVICE,
> +    .parent = TYPE_PCI_DEVICE,
> +    .instance_size = sizeof(AspeedPCIERootDeviceState),
> +    .class_init = aspeed_pcie_root_device_class_init,
> +    .interfaces = (const InterfaceInfo[]) {
> +        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
> +        { },
> +    },
> +};
> +
>   /*
>    * PCIe Root Complex (RC)
>    */
> @@ -94,6 +132,18 @@ static void aspeed_pcie_rc_realize(DeviceState *dev, Error **errp)
>                                        aspeed_pcie_rc_map_irq, rc, &rc->mmio,
>                                        &rc->io, 0, 4, TYPE_PCIE_BUS);
>       pci->bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
> +
> +    /* setup root device */
> +    if (rc->has_rd) {
> +        object_initialize_child(OBJECT(rc), "root_device", &rc->root_device,
> +                                TYPE_ASPEED_PCIE_ROOT_DEVICE);
> +        qdev_prop_set_int32(DEVICE(&rc->root_device), "addr",
> +                            PCI_DEVFN(0, 0));
> +        qdev_prop_set_bit(DEVICE(&rc->root_device), "multifunction", false);
> +        if (!qdev_realize(DEVICE(&rc->root_device), BUS(pci->bus), errp)) {
> +            return;
> +        }
> +    }
>   }
>   
>   static const char *aspeed_pcie_rc_root_bus_path(PCIHostState *host_bridge,
> @@ -110,6 +160,7 @@ static const char *aspeed_pcie_rc_root_bus_path(PCIHostState *host_bridge,
>   
>   static const Property aspeed_pcie_rc_props[] = {
>       DEFINE_PROP_UINT32("bus-nr", AspeedPCIERcState, bus_nr, 0),
> +    DEFINE_PROP_BOOL("has-rd", AspeedPCIERcState, has_rd, 0),
>   };
>   
>   static void aspeed_pcie_rc_class_init(ObjectClass *klass, const void *data)
> @@ -401,6 +452,9 @@ static void aspeed_pcie_cfg_realize(DeviceState *dev, Error **errp)
>       object_property_set_int(OBJECT(&s->rc), "bus-nr",
>                               apc->rc_bus_nr,
>                               &error_abort);
> +    object_property_set_bool(OBJECT(&s->rc), "has-rd",
> +                            apc->rc_has_rd,
> +                            &error_abort);
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->rc), errp)) {
>           return;
>       }
> @@ -433,6 +487,7 @@ static void aspeed_pcie_cfg_class_init(ObjectClass *klass, const void *data)
>       apc->reg_map = &aspeed_regmap;
>       apc->nr_regs = 0x100 >> 2;
>       apc->rc_bus_nr = 0x80;
> +    apc->rc_has_rd = true;
>   }
>   
>   static const TypeInfo aspeed_pcie_cfg_info = {
> @@ -570,6 +625,7 @@ static const TypeInfo aspeed_pcie_phy_info = {
>   static void aspeed_pcie_register_types(void)
>   {
>       type_register_static(&aspeed_pcie_rc_info);
> +    type_register_static(&aspeed_pcie_root_device_info);
>       type_register_static(&aspeed_pcie_cfg_info);
>       type_register_static(&aspeed_pcie_phy_info);
>   }