This patch introduces an initial ASPEED PCIe PHY/host controller model to
support the AST2600 SoC. It provides a simple register block with MMIO
read/write callbacks, integration into the build system, and trace events
for debugging.
Key changes:
1. PCIe PHY MMIO read/write callbacks
Implemented aspeed_pcie_phy_read() and aspeed_pcie_phy_write() to
handle 32-bit register accesses.
2. Build system and Kconfig integration
Added CONFIG_PCI_EXPRESS_ASPEED in hw/pci-host/Kconfig and meson
rules.
Updated ASPEED_SOC in hw/arm/Kconfig to imply PCI_DEVICES and select
PCI_EXPRESS_ASPEED.
3. Trace events for debug
New tracepoints aspeed_pcie_phy_read and aspeed_pcie_phy_write allow
monitoring MMIO accesses.
4. Register space and defaults (AST2600 reference)
Expose a 0x100 register space, as documented in the AST2600 datasheet.
On reset, set default values:
PEHR_ID: Vendor ID = ASPEED, Device ID = 0x1150
PEHR_CLASS_CODE = 0x06040006
PEHR_DATALINK = 0xD7040022
PEHR_LINK: bit[5] set to 1 to indicate link up.
This provides a skeleton device for the AST2600 platform. It enables
firmware to detect the PCIe link as up by default and allows future
extension.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
include/hw/pci-host/aspeed_pcie.h | 44 +++++++++
hw/pci-host/aspeed_pcie.c | 159 ++++++++++++++++++++++++++++++
hw/arm/Kconfig | 3 +
hw/pci-host/Kconfig | 4 +
hw/pci-host/meson.build | 1 +
hw/pci-host/trace-events | 4 +
6 files changed, 215 insertions(+)
create mode 100644 include/hw/pci-host/aspeed_pcie.h
create mode 100644 hw/pci-host/aspeed_pcie.c
diff --git a/include/hw/pci-host/aspeed_pcie.h b/include/hw/pci-host/aspeed_pcie.h
new file mode 100644
index 0000000000..faf87073ab
--- /dev/null
+++ b/include/hw/pci-host/aspeed_pcie.h
@@ -0,0 +1,44 @@
+/*
+ * ASPEED PCIe Host Controller
+ *
+ * Copyright (C) 2025 ASPEED Technology Inc.
+ * Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
+ *
+ * Jamin Lin <jamin_lin@aspeedtech.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This file is based on Cédric Le Goater's patch:
+ * "pci: Add Aspeed host bridge (WIP)"
+ * https://github.com/legoater/qemu/commit/d1b97b0c7844219d847122410dc189854f9d26df
+ *
+ * Modifications have been made to support the ASPEED AST2600 and AST2700
+ * platforms.
+ */
+
+#ifndef ASPEED_PCIE_H
+#define ASPEED_PCIE_H
+
+#include "hw/sysbus.h"
+#include "hw/pci/pci_bridge.h"
+#include "hw/pci/pcie_host.h"
+#include "qom/object.h"
+
+#define TYPE_ASPEED_PCIE_PHY "aspeed.pcie-phy"
+OBJECT_DECLARE_TYPE(AspeedPCIEPhyState, AspeedPCIEPhyClass, ASPEED_PCIE_PHY);
+
+struct AspeedPCIEPhyState {
+ SysBusDevice parent_obj;
+
+ MemoryRegion mmio;
+ uint32_t *regs;
+ uint32_t id;
+};
+
+struct AspeedPCIEPhyClass {
+ SysBusDeviceClass parent_class;
+
+ uint64_t nr_regs;
+};
+
+#endif /* ASPEED_PCIE_H */
diff --git a/hw/pci-host/aspeed_pcie.c b/hw/pci-host/aspeed_pcie.c
new file mode 100644
index 0000000000..5584449b17
--- /dev/null
+++ b/hw/pci-host/aspeed_pcie.c
@@ -0,0 +1,159 @@
+/*
+ * ASPEED PCIe Host Controller
+ *
+ * Copyright (C) 2025 ASPEED Technology Inc.
+ * Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
+ *
+ * Jamin Lin <jamin_lin@aspeedtech.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This file is based on Cédric Le Goater's patch:
+ * "pci: Add Aspeed host bridge (WIP)"
+ * https://github.com/legoater/qemu/commit/d1b97b0c7844219d847122410dc189854f9d26df
+ *
+ * Modifications have been made to support the ASPEED AST2600 and AST2700
+ * platforms.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/registerfields.h"
+#include "hw/irq.h"
+#include "hw/pci/pci_host.h"
+#include "hw/pci-host/aspeed_pcie.h"
+#include "hw/pci/msi.h"
+#include "trace.h"
+
+/*
+ * PCIe PHY
+ *
+ * PCIe Host Controller (PCIEH)
+ */
+
+/* AST2600 */
+REG32(PEHR_ID, 0x00)
+ FIELD(PEHR_ID, DEV, 16, 16)
+REG32(PEHR_CLASS_CODE, 0x04)
+REG32(PEHR_DATALINK, 0x10)
+REG32(PEHR_PROTECT, 0x7C)
+ FIELD(PEHR_PROTECT, LOCK, 0, 8)
+REG32(PEHR_LINK, 0xC0)
+ FIELD(PEHR_LINK, STS, 5, 1)
+
+#define ASPEED_PCIE_PHY_UNLOCK 0xA8
+
+static uint64_t aspeed_pcie_phy_read(void *opaque, hwaddr addr,
+ unsigned int size)
+{
+ AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(opaque);
+ uint32_t reg = addr >> 2;
+ uint32_t value = 0;
+
+ value = s->regs[reg];
+
+ trace_aspeed_pcie_phy_read(s->id, addr, value);
+
+ return value;
+}
+
+static void aspeed_pcie_phy_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned int size)
+{
+ AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(opaque);
+ uint32_t reg = addr >> 2;
+
+ trace_aspeed_pcie_phy_write(s->id, addr, data);
+
+ switch (reg) {
+ case R_PEHR_PROTECT:
+ data &= R_PEHR_PROTECT_LOCK_MASK;
+ s->regs[reg] = !!(data == ASPEED_PCIE_PHY_UNLOCK);
+ break;
+ default:
+ s->regs[reg] = data;
+ break;
+ }
+}
+
+static const MemoryRegionOps aspeed_pcie_phy_ops = {
+ .read = aspeed_pcie_phy_read,
+ .write = aspeed_pcie_phy_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 4,
+ },
+};
+
+static void aspeed_pcie_phy_reset(DeviceState *dev)
+{
+ AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
+ AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_GET_CLASS(s);
+
+ memset(s->regs, 0, apc->nr_regs << 2);
+
+ s->regs[R_PEHR_ID] =
+ (0x1150 << R_PEHR_ID_DEV_SHIFT) | PCI_VENDOR_ID_ASPEED;
+ s->regs[R_PEHR_CLASS_CODE] = 0x06040006;
+ s->regs[R_PEHR_DATALINK] = 0xD7040022;
+ s->regs[R_PEHR_LINK] = R_PEHR_LINK_STS_MASK;
+}
+
+static void aspeed_pcie_phy_realize(DeviceState *dev, Error **errp)
+{
+ AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
+ AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_GET_CLASS(s);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+ g_autofree char *name;
+
+ s->regs = g_new(uint32_t, apc->nr_regs);
+ name = g_strdup_printf(TYPE_ASPEED_PCIE_PHY ".regs.%d", s->id);
+ memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_pcie_phy_ops, s, name,
+ apc->nr_regs << 2);
+ sysbus_init_mmio(sbd, &s->mmio);
+}
+
+static void aspeed_pcie_phy_unrealize(DeviceState *dev)
+{
+ AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
+
+ g_free(s->regs);
+ s->regs = NULL;
+}
+
+static const Property aspeed_pcie_phy_props[] = {
+ DEFINE_PROP_UINT32("id", AspeedPCIEPhyState, id, 0),
+};
+
+static void aspeed_pcie_phy_class_init(ObjectClass *klass, const void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_CLASS(klass);
+
+ dc->desc = "ASPEED PCIe Phy";
+ dc->realize = aspeed_pcie_phy_realize;
+ dc->unrealize = aspeed_pcie_phy_unrealize;
+ device_class_set_legacy_reset(dc, aspeed_pcie_phy_reset);
+ device_class_set_props(dc, aspeed_pcie_phy_props);
+
+ apc->nr_regs = 0x100 >> 2;
+}
+
+static const TypeInfo aspeed_pcie_phy_info = {
+ .name = TYPE_ASPEED_PCIE_PHY,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(AspeedPCIEPhyState),
+ .class_init = aspeed_pcie_phy_class_init,
+ .class_size = sizeof(AspeedPCIEPhyClass),
+};
+
+static void aspeed_pcie_register_types(void)
+{
+ type_register_static(&aspeed_pcie_phy_info);
+}
+
+type_init(aspeed_pcie_register_types);
+
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 2aa4b5d778..83d2676f6c 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -544,6 +544,7 @@ config ASPEED_SOC
bool
default y
depends on TCG && ARM
+ imply PCI_DEVICES
select DS1338
select FTGMAC100
select I2C
@@ -564,6 +565,8 @@ config ASPEED_SOC
select MAX31785
select FSI_APB2OPB_ASPEED
select AT24C
+ select PCI_EXPRESS
+ select PCI_EXPRESS_ASPEED
config MPS2
bool
diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
index 9824fa188d..8cbb8304a3 100644
--- a/hw/pci-host/Kconfig
+++ b/hw/pci-host/Kconfig
@@ -46,6 +46,10 @@ config PCI_I440FX
select PCI
select PAM
+config PCI_EXPRESS_ASPEED
+ bool
+ select PCI_EXPRESS
+
config PCI_EXPRESS_Q35
bool
select PCI_EXPRESS
diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
index 937a0f72ac..86b754d0b0 100644
--- a/hw/pci-host/meson.build
+++ b/hw/pci-host/meson.build
@@ -2,6 +2,7 @@ pci_ss = ss.source_set()
pci_ss.add(when: 'CONFIG_PAM', if_true: files('pam.c'))
pci_ss.add(when: 'CONFIG_PCI_BONITO', if_true: files('bonito.c'))
pci_ss.add(when: 'CONFIG_GT64120', if_true: files('gt64120.c'))
+pci_ss.add(when: 'CONFIG_PCI_EXPRESS_ASPEED', if_true: files('aspeed_pcie.c'))
pci_ss.add(when: 'CONFIG_PCI_EXPRESS_DESIGNWARE', if_true: files('designware.c'))
pci_ss.add(when: 'CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', if_true: files('gpex.c'))
pci_ss.add(when: ['CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', 'CONFIG_ACPI'], if_true: files('gpex-acpi.c'))
diff --git a/hw/pci-host/trace-events b/hw/pci-host/trace-events
index 0a816b9aa1..3438516756 100644
--- a/hw/pci-host/trace-events
+++ b/hw/pci-host/trace-events
@@ -1,5 +1,9 @@
# See docs/devel/tracing.rst for syntax documentation.
+# aspeed_pcie.c
+aspeed_pcie_phy_read(uint32_t id, uint64_t addr, uint32_t value) "%d: addr 0x%" PRIx64 " value 0x%" PRIx32
+aspeed_pcie_phy_write(uint32_t id, uint64_t addr, uint32_t value) "%d: addr 0x%" PRIx64 " value 0x%" PRIx32
+
# bonito.c
bonito_spciconf_small_access(uint64_t addr, unsigned size) "PCI config address is smaller then 32-bit, addr: 0x%"PRIx64", size: %u"
--
2.43.0
Hello Jamin,
On 9/11/25 09:24, Jamin Lin wrote:
> This patch introduces an initial ASPEED PCIe PHY/host controller model to
> support the AST2600 SoC. It provides a simple register block with MMIO
> read/write callbacks, integration into the build system, and trace events
> for debugging.
>
> Key changes:
>
> 1. PCIe PHY MMIO read/write callbacks
> Implemented aspeed_pcie_phy_read() and aspeed_pcie_phy_write() to
> handle 32-bit register accesses.
>
> 2. Build system and Kconfig integration
> Added CONFIG_PCI_EXPRESS_ASPEED in hw/pci-host/Kconfig and meson
> rules.
> Updated ASPEED_SOC in hw/arm/Kconfig to imply PCI_DEVICES and select
> PCI_EXPRESS_ASPEED.
>
> 3. Trace events for debug
> New tracepoints aspeed_pcie_phy_read and aspeed_pcie_phy_write allow
> monitoring MMIO accesses.
>
> 4. Register space and defaults (AST2600 reference)
> Expose a 0x100 register space, as documented in the AST2600 datasheet.
> On reset, set default values:
> PEHR_ID: Vendor ID = ASPEED, Device ID = 0x1150
> PEHR_CLASS_CODE = 0x06040006
> PEHR_DATALINK = 0xD7040022
> PEHR_LINK: bit[5] set to 1 to indicate link up.
>
> This provides a skeleton device for the AST2600 platform. It enables
> firmware to detect the PCIe link as up by default and allows future
> extension.
There, you could say : based on previous work from me, plus add
a summary of the modifications that have been made.
>
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
> include/hw/pci-host/aspeed_pcie.h | 44 +++++++++
> hw/pci-host/aspeed_pcie.c | 159 ++++++++++++++++++++++++++++++
> hw/arm/Kconfig | 3 +
> hw/pci-host/Kconfig | 4 +
> hw/pci-host/meson.build | 1 +
> hw/pci-host/trace-events | 4 +
> 6 files changed, 215 insertions(+)
> create mode 100644 include/hw/pci-host/aspeed_pcie.h
> create mode 100644 hw/pci-host/aspeed_pcie.c
>
> diff --git a/include/hw/pci-host/aspeed_pcie.h b/include/hw/pci-host/aspeed_pcie.h
> new file mode 100644
> index 0000000000..faf87073ab
> --- /dev/null
> +++ b/include/hw/pci-host/aspeed_pcie.h
> @@ -0,0 +1,44 @@
> +/*
> + * ASPEED PCIe Host Controller
> + *
> + * Copyright (C) 2025 ASPEED Technology Inc.
> + * Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
> + *
> + * Jamin Lin <jamin_lin@aspeedtech.com>
I would add an "Authors" section and add our two names under it.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This file is based on Cédric Le Goater's patch:
> + * "pci: Add Aspeed host bridge (WIP)"
> + * https://github.com/legoater/qemu/commit/d1b97b0c7844219d847122410dc189854f9d26df
I don't think adding a reference on a github commit has much value
since I could remove the branch, which I do regularly. So, I would
remove it and simply keep the "Based on previous work from ... "
boiler plate.
The rest looks good.
> + * Modifications have been made to support the ASPEED AST2600 and AST2700
> + * platforms.
> + */
> +
> +#ifndef ASPEED_PCIE_H
> +#define ASPEED_PCIE_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/pci/pci_bridge.h"
> +#include "hw/pci/pcie_host.h"
> +#include "qom/object.h"
> +
> +#define TYPE_ASPEED_PCIE_PHY "aspeed.pcie-phy"
> +OBJECT_DECLARE_TYPE(AspeedPCIEPhyState, AspeedPCIEPhyClass, ASPEED_PCIE_PHY);
> +
> +struct AspeedPCIEPhyState {
> + SysBusDevice parent_obj;
> +
> + MemoryRegion mmio;
> + uint32_t *regs;
> + uint32_t id;
> +};
> +
> +struct AspeedPCIEPhyClass {
> + SysBusDeviceClass parent_class;
> +
> + uint64_t nr_regs;
> +};
> +
> +#endif /* ASPEED_PCIE_H */
> diff --git a/hw/pci-host/aspeed_pcie.c b/hw/pci-host/aspeed_pcie.c
> new file mode 100644
> index 0000000000..5584449b17
> --- /dev/null
> +++ b/hw/pci-host/aspeed_pcie.c
> @@ -0,0 +1,159 @@
> +/*
> + * ASPEED PCIe Host Controller
> + *
> + * Copyright (C) 2025 ASPEED Technology Inc.
> + * Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
> + *
> + * Jamin Lin <jamin_lin@aspeedtech.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * This file is based on Cédric Le Goater's patch:
> + * "pci: Add Aspeed host bridge (WIP)"
> + * https://github.com/legoater/qemu/commit/d1b97b0c7844219d847122410dc189854f9d26df
> + *
> + * Modifications have been made to support the ASPEED AST2600 and AST2700
> + * platforms.
Same comment
The rest looks good.
Thanks,
C.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "hw/qdev-properties.h"
> +#include "hw/registerfields.h"
> +#include "hw/irq.h"
> +#include "hw/pci/pci_host.h"
> +#include "hw/pci-host/aspeed_pcie.h"
> +#include "hw/pci/msi.h"
> +#include "trace.h"
> +
> +/*
> + * PCIe PHY
> + *
> + * PCIe Host Controller (PCIEH)
> + */
> +
> +/* AST2600 */
> +REG32(PEHR_ID, 0x00)
> + FIELD(PEHR_ID, DEV, 16, 16)
> +REG32(PEHR_CLASS_CODE, 0x04)
> +REG32(PEHR_DATALINK, 0x10)
> +REG32(PEHR_PROTECT, 0x7C)
> + FIELD(PEHR_PROTECT, LOCK, 0, 8)
> +REG32(PEHR_LINK, 0xC0)
> + FIELD(PEHR_LINK, STS, 5, 1)
> +
> +#define ASPEED_PCIE_PHY_UNLOCK 0xA8
> +
> +static uint64_t aspeed_pcie_phy_read(void *opaque, hwaddr addr,
> + unsigned int size)
> +{
> + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(opaque);
> + uint32_t reg = addr >> 2;
> + uint32_t value = 0;
> +
> + value = s->regs[reg];
> +
> + trace_aspeed_pcie_phy_read(s->id, addr, value);
> +
> + return value;
> +}
> +
> +static void aspeed_pcie_phy_write(void *opaque, hwaddr addr, uint64_t data,
> + unsigned int size)
> +{
> + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(opaque);
> + uint32_t reg = addr >> 2;
> +
> + trace_aspeed_pcie_phy_write(s->id, addr, data);
> +
> + switch (reg) {
> + case R_PEHR_PROTECT:
> + data &= R_PEHR_PROTECT_LOCK_MASK;
> + s->regs[reg] = !!(data == ASPEED_PCIE_PHY_UNLOCK);
> + break;
> + default:
> + s->regs[reg] = data;
> + break;
> + }
> +}
> +
> +static const MemoryRegionOps aspeed_pcie_phy_ops = {
> + .read = aspeed_pcie_phy_read,
> + .write = aspeed_pcie_phy_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> + .valid = {
> + .min_access_size = 1,
> + .max_access_size = 4,
> + },
> +};
> +
> +static void aspeed_pcie_phy_reset(DeviceState *dev)
> +{
> + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
> + AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_GET_CLASS(s);
> +
> + memset(s->regs, 0, apc->nr_regs << 2);
> +
> + s->regs[R_PEHR_ID] =
> + (0x1150 << R_PEHR_ID_DEV_SHIFT) | PCI_VENDOR_ID_ASPEED;
> + s->regs[R_PEHR_CLASS_CODE] = 0x06040006;
> + s->regs[R_PEHR_DATALINK] = 0xD7040022;
> + s->regs[R_PEHR_LINK] = R_PEHR_LINK_STS_MASK;
> +}
> +
> +static void aspeed_pcie_phy_realize(DeviceState *dev, Error **errp)
> +{
> + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
> + AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_GET_CLASS(s);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + g_autofree char *name;
> +
> + s->regs = g_new(uint32_t, apc->nr_regs);
> + name = g_strdup_printf(TYPE_ASPEED_PCIE_PHY ".regs.%d", s->id);
> + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_pcie_phy_ops, s, name,
> + apc->nr_regs << 2);
> + sysbus_init_mmio(sbd, &s->mmio);
> +}
> +
> +static void aspeed_pcie_phy_unrealize(DeviceState *dev)
> +{
> + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
> +
> + g_free(s->regs);
> + s->regs = NULL;
> +}
> +
> +static const Property aspeed_pcie_phy_props[] = {
> + DEFINE_PROP_UINT32("id", AspeedPCIEPhyState, id, 0),
> +};
> +
> +static void aspeed_pcie_phy_class_init(ObjectClass *klass, const void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> + AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_CLASS(klass);
> +
> + dc->desc = "ASPEED PCIe Phy";
> + dc->realize = aspeed_pcie_phy_realize;
> + dc->unrealize = aspeed_pcie_phy_unrealize;
> + device_class_set_legacy_reset(dc, aspeed_pcie_phy_reset);
> + device_class_set_props(dc, aspeed_pcie_phy_props);
> +
> + apc->nr_regs = 0x100 >> 2;
> +}
> +
> +static const TypeInfo aspeed_pcie_phy_info = {
> + .name = TYPE_ASPEED_PCIE_PHY,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(AspeedPCIEPhyState),
> + .class_init = aspeed_pcie_phy_class_init,
> + .class_size = sizeof(AspeedPCIEPhyClass),
> +};
> +
> +static void aspeed_pcie_register_types(void)
> +{
> + type_register_static(&aspeed_pcie_phy_info);
> +}
> +
> +type_init(aspeed_pcie_register_types);
> +
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 2aa4b5d778..83d2676f6c 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -544,6 +544,7 @@ config ASPEED_SOC
> bool
> default y
> depends on TCG && ARM
> + imply PCI_DEVICES
> select DS1338
> select FTGMAC100
> select I2C
> @@ -564,6 +565,8 @@ config ASPEED_SOC
> select MAX31785
> select FSI_APB2OPB_ASPEED
> select AT24C
> + select PCI_EXPRESS
> + select PCI_EXPRESS_ASPEED
>
> config MPS2
> bool
> diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig
> index 9824fa188d..8cbb8304a3 100644
> --- a/hw/pci-host/Kconfig
> +++ b/hw/pci-host/Kconfig
> @@ -46,6 +46,10 @@ config PCI_I440FX
> select PCI
> select PAM
>
> +config PCI_EXPRESS_ASPEED
> + bool
> + select PCI_EXPRESS
> +
> config PCI_EXPRESS_Q35
> bool
> select PCI_EXPRESS
> diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build
> index 937a0f72ac..86b754d0b0 100644
> --- a/hw/pci-host/meson.build
> +++ b/hw/pci-host/meson.build
> @@ -2,6 +2,7 @@ pci_ss = ss.source_set()
> pci_ss.add(when: 'CONFIG_PAM', if_true: files('pam.c'))
> pci_ss.add(when: 'CONFIG_PCI_BONITO', if_true: files('bonito.c'))
> pci_ss.add(when: 'CONFIG_GT64120', if_true: files('gt64120.c'))
> +pci_ss.add(when: 'CONFIG_PCI_EXPRESS_ASPEED', if_true: files('aspeed_pcie.c'))
> pci_ss.add(when: 'CONFIG_PCI_EXPRESS_DESIGNWARE', if_true: files('designware.c'))
> pci_ss.add(when: 'CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', if_true: files('gpex.c'))
> pci_ss.add(when: ['CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', 'CONFIG_ACPI'], if_true: files('gpex-acpi.c'))
> diff --git a/hw/pci-host/trace-events b/hw/pci-host/trace-events
> index 0a816b9aa1..3438516756 100644
> --- a/hw/pci-host/trace-events
> +++ b/hw/pci-host/trace-events
> @@ -1,5 +1,9 @@
> # See docs/devel/tracing.rst for syntax documentation.
>
> +# aspeed_pcie.c
> +aspeed_pcie_phy_read(uint32_t id, uint64_t addr, uint32_t value) "%d: addr 0x%" PRIx64 " value 0x%" PRIx32
> +aspeed_pcie_phy_write(uint32_t id, uint64_t addr, uint32_t value) "%d: addr 0x%" PRIx64 " value 0x%" PRIx32
> +
> # bonito.c
> bonito_spciconf_small_access(uint64_t addr, unsigned size) "PCI config address is smaller then 32-bit, addr: 0x%"PRIx64", size: %u"
>
Hi Cédric
> Subject: Re: [SPAM] [PATCH v2 02/14] hw/pci-host/aspeed: Add AST2600 PCIe
> PHY model
>
> Hello Jamin,
>
> On 9/11/25 09:24, Jamin Lin wrote:
> > This patch introduces an initial ASPEED PCIe PHY/host controller model
> > to support the AST2600 SoC. It provides a simple register block with
> > MMIO read/write callbacks, integration into the build system, and
> > trace events for debugging.
> >
> > Key changes:
> >
> > 1. PCIe PHY MMIO read/write callbacks
> > Implemented aspeed_pcie_phy_read() and aspeed_pcie_phy_write() to
> > handle 32-bit register accesses.
> >
> > 2. Build system and Kconfig integration
> > Added CONFIG_PCI_EXPRESS_ASPEED in hw/pci-host/Kconfig and
> meson
> > rules.
> > Updated ASPEED_SOC in hw/arm/Kconfig to imply PCI_DEVICES and
> select
> > PCI_EXPRESS_ASPEED.
> >
> > 3. Trace events for debug
> > New tracepoints aspeed_pcie_phy_read and aspeed_pcie_phy_write
> allow
> > monitoring MMIO accesses.
> >
> > 4. Register space and defaults (AST2600 reference)
> > Expose a 0x100 register space, as documented in the AST2600
> datasheet.
> > On reset, set default values:
> > PEHR_ID: Vendor ID = ASPEED, Device ID = 0x1150
> > PEHR_CLASS_CODE = 0x06040006
> > PEHR_DATALINK = 0xD7040022
> > PEHR_LINK: bit[5] set to 1 to indicate link up.
> >
> > This provides a skeleton device for the AST2600 platform. It enables
> > firmware to detect the PCIe link as up by default and allows future
> > extension.
>
> There, you could say : based on previous work from me, plus add a summary of
> the modifications that have been made.
>
Thanks for your review and suggestion.
I will add the following messages in this commit log
This commit is the starting point of the series to introduce ASPEED PCIe
Root Complex (RC) support. Based on previous work from Cédric Le Goater,
the following commits in this series extend and refine the implementation:
- Add a PCIe Root Port so that devices can be attached without requiring an
extra bridge.
- Restrict the Root Port device instantiation to the AST2600 platform.
- Integrate aspeed_cfg_translate_write() to support both AST2600 and AST2700.
- Add MSI support and a preliminary RC IOMMU address space.
- Fix issues with MSI interrupt clearing.
- Extend support to the AST2700 SoC.
- Drop the AST2600 RC_L support.
- Introduce PCIe RC functional tests covering both AST2600 and AST2700.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> > include/hw/pci-host/aspeed_pcie.h | 44 +++++++++
> > hw/pci-host/aspeed_pcie.c | 159
> ++++++++++++++++++++++++++++++
> > hw/arm/Kconfig | 3 +
> > hw/pci-host/Kconfig | 4 +
> > hw/pci-host/meson.build | 1 +
> > hw/pci-host/trace-events | 4 +
> > 6 files changed, 215 insertions(+)
> > create mode 100644 include/hw/pci-host/aspeed_pcie.h
> > create mode 100644 hw/pci-host/aspeed_pcie.c
> >
> > diff --git a/include/hw/pci-host/aspeed_pcie.h
> > b/include/hw/pci-host/aspeed_pcie.h
> > new file mode 100644
> > index 0000000000..faf87073ab
> > --- /dev/null
> > +++ b/include/hw/pci-host/aspeed_pcie.h
> > @@ -0,0 +1,44 @@
> > +/*
> > + * ASPEED PCIe Host Controller
> > + *
> > + * Copyright (C) 2025 ASPEED Technology Inc.
> > + * Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
> > + *
> > + * Jamin Lin <jamin_lin@aspeedtech.com>
>
> I would add an "Authors" section and add our two names under it.
>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * This file is based on Cédric Le Goater's patch:
> > + * "pci: Add Aspeed host bridge (WIP)"
> > + *
> > +
> https://github.com/legoater/qemu/commit/d1b97b0c7844219d847122410dc1
> > + 89854f9d26df
>
> I don't think adding a reference on a github commit has much value since I
> could remove the branch, which I do regularly. So, I would remove it and
> simply keep the "Based on previous work from ... "
> boiler plate.
>
Thanks for your review and suggestion.
LICENSE HEADER will be updated as following.
/*
* ASPEED PCIe Host Controller
*
* Copyright (C) 2025 ASPEED Technology Inc.
* Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
*
* Authors:
* Cédric Le Goater <clg@kaod.org>
* Jamin Lin <jamin_lin@aspeedtech.com>
*
* SPDX-License-Identifier: GPL-2.0-or-later
*
* Based on previous work from Cédric Le Goater.
* Modifications extend support for the ASPEED AST2600 and AST2700 platforms.
*/
> The rest looks good.
>
> > + * Modifications have been made to support the ASPEED AST2600 and
> > + AST2700
> > + * platforms.
> > + */
> > +
> > +#ifndef ASPEED_PCIE_H
> > +#define ASPEED_PCIE_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "hw/pci/pci_bridge.h"
> > +#include "hw/pci/pcie_host.h"
> > +#include "qom/object.h"
> > +
> > +#define TYPE_ASPEED_PCIE_PHY "aspeed.pcie-phy"
> > +OBJECT_DECLARE_TYPE(AspeedPCIEPhyState, AspeedPCIEPhyClass,
> > +ASPEED_PCIE_PHY);
> > +
> > +struct AspeedPCIEPhyState {
> > + SysBusDevice parent_obj;
> > +
> > + MemoryRegion mmio;
> > + uint32_t *regs;
> > + uint32_t id;
> > +};
> > +
> > +struct AspeedPCIEPhyClass {
> > + SysBusDeviceClass parent_class;
> > +
> > + uint64_t nr_regs;
> > +};
> > +
> > +#endif /* ASPEED_PCIE_H */
> > diff --git a/hw/pci-host/aspeed_pcie.c b/hw/pci-host/aspeed_pcie.c new
> > file mode 100644 index 0000000000..5584449b17
> > --- /dev/null
> > +++ b/hw/pci-host/aspeed_pcie.c
> > @@ -0,0 +1,159 @@
> > +/*
> > + * ASPEED PCIe Host Controller
> > + *
> > + * Copyright (C) 2025 ASPEED Technology Inc.
> > + * Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
> > + *
> > + * Jamin Lin <jamin_lin@aspeedtech.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * This file is based on Cédric Le Goater's patch:
> > + * "pci: Add Aspeed host bridge (WIP)"
> > + *
> >
> +https://github.com/legoater/qemu/commit/d1b97b0c7844219d847122410dc
> 18
> > +9854f9d26df
> > + *
> > + * Modifications have been made to support the ASPEED AST2600 and
> > +AST2700
> > + * platforms.
>
> Same comment
>
Thanks for your review and suggestion.
LICENSE HEADER will be updated as following
/*
* ASPEED PCIe Host Controller
*
* Copyright (C) 2025 ASPEED Technology Inc.
* Copyright (c) 2022 Cédric Le Goater <clg@kaod.org>
*
* Authors:
* Cédric Le Goater <clg@kaod.org>
* Jamin Lin <jamin_lin@aspeedtech.com>
*
* SPDX-License-Identifier: GPL-2.0-or-later
*
* Based on previous work from Cédric Le Goater.
* Modifications extend support for the ASPEED AST2600 and AST2700 platforms.
*/
Thanks-Jamin
> The rest looks good.
>
> Thanks,
>
> C.
>
>
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/registerfields.h"
> > +#include "hw/irq.h"
> > +#include "hw/pci/pci_host.h"
> > +#include "hw/pci-host/aspeed_pcie.h"
> > +#include "hw/pci/msi.h"
> > +#include "trace.h"
> > +
> > +/*
> > + * PCIe PHY
> > + *
> > + * PCIe Host Controller (PCIEH)
> > + */
> > +
> > +/* AST2600 */
> > +REG32(PEHR_ID, 0x00)
> > + FIELD(PEHR_ID, DEV, 16, 16)
> > +REG32(PEHR_CLASS_CODE, 0x04)
> > +REG32(PEHR_DATALINK, 0x10)
> > +REG32(PEHR_PROTECT, 0x7C)
> > + FIELD(PEHR_PROTECT, LOCK, 0, 8)
> > +REG32(PEHR_LINK, 0xC0)
> > + FIELD(PEHR_LINK, STS, 5, 1)
> > +
> > +#define ASPEED_PCIE_PHY_UNLOCK 0xA8
> > +
> > +static uint64_t aspeed_pcie_phy_read(void *opaque, hwaddr addr,
> > + unsigned int size) {
> > + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(opaque);
> > + uint32_t reg = addr >> 2;
> > + uint32_t value = 0;
> > +
> > + value = s->regs[reg];
> > +
> > + trace_aspeed_pcie_phy_read(s->id, addr, value);
> > +
> > + return value;
> > +}
> > +
> > +static void aspeed_pcie_phy_write(void *opaque, hwaddr addr, uint64_t
> data,
> > + unsigned int size) {
> > + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(opaque);
> > + uint32_t reg = addr >> 2;
> > +
> > + trace_aspeed_pcie_phy_write(s->id, addr, data);
> > +
> > + switch (reg) {
> > + case R_PEHR_PROTECT:
> > + data &= R_PEHR_PROTECT_LOCK_MASK;
> > + s->regs[reg] = !!(data == ASPEED_PCIE_PHY_UNLOCK);
> > + break;
> > + default:
> > + s->regs[reg] = data;
> > + break;
> > + }
> > +}
> > +
> > +static const MemoryRegionOps aspeed_pcie_phy_ops = {
> > + .read = aspeed_pcie_phy_read,
> > + .write = aspeed_pcie_phy_write,
> > + .endianness = DEVICE_LITTLE_ENDIAN,
> > + .valid = {
> > + .min_access_size = 1,
> > + .max_access_size = 4,
> > + },
> > +};
> > +
> > +static void aspeed_pcie_phy_reset(DeviceState *dev) {
> > + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
> > + AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_GET_CLASS(s);
> > +
> > + memset(s->regs, 0, apc->nr_regs << 2);
> > +
> > + s->regs[R_PEHR_ID] =
> > + (0x1150 << R_PEHR_ID_DEV_SHIFT) | PCI_VENDOR_ID_ASPEED;
> > + s->regs[R_PEHR_CLASS_CODE] = 0x06040006;
> > + s->regs[R_PEHR_DATALINK] = 0xD7040022;
> > + s->regs[R_PEHR_LINK] = R_PEHR_LINK_STS_MASK; }
> > +
> > +static void aspeed_pcie_phy_realize(DeviceState *dev, Error **errp) {
> > + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
> > + AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_GET_CLASS(s);
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> > + g_autofree char *name;
> > +
> > + s->regs = g_new(uint32_t, apc->nr_regs);
> > + name = g_strdup_printf(TYPE_ASPEED_PCIE_PHY ".regs.%d", s->id);
> > + memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_pcie_phy_ops,
> s, name,
> > + apc->nr_regs << 2);
> > + sysbus_init_mmio(sbd, &s->mmio);
> > +}
> > +
> > +static void aspeed_pcie_phy_unrealize(DeviceState *dev) {
> > + AspeedPCIEPhyState *s = ASPEED_PCIE_PHY(dev);
> > +
> > + g_free(s->regs);
> > + s->regs = NULL;
> > +}
> > +
> > +static const Property aspeed_pcie_phy_props[] = {
> > + DEFINE_PROP_UINT32("id", AspeedPCIEPhyState, id, 0), };
> > +
> > +static void aspeed_pcie_phy_class_init(ObjectClass *klass, const void
> > +*data) {
> > + DeviceClass *dc = DEVICE_CLASS(klass);
> > + AspeedPCIEPhyClass *apc = ASPEED_PCIE_PHY_CLASS(klass);
> > +
> > + dc->desc = "ASPEED PCIe Phy";
> > + dc->realize = aspeed_pcie_phy_realize;
> > + dc->unrealize = aspeed_pcie_phy_unrealize;
> > + device_class_set_legacy_reset(dc, aspeed_pcie_phy_reset);
> > + device_class_set_props(dc, aspeed_pcie_phy_props);
> > +
> > + apc->nr_regs = 0x100 >> 2;
> > +}
> > +
> > +static const TypeInfo aspeed_pcie_phy_info = {
> > + .name = TYPE_ASPEED_PCIE_PHY,
> > + .parent = TYPE_SYS_BUS_DEVICE,
> > + .instance_size = sizeof(AspeedPCIEPhyState),
> > + .class_init = aspeed_pcie_phy_class_init,
> > + .class_size = sizeof(AspeedPCIEPhyClass), };
> > +
> > +static void aspeed_pcie_register_types(void) {
> > + type_register_static(&aspeed_pcie_phy_info);
> > +}
> > +
> > +type_init(aspeed_pcie_register_types);
> > +
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
> > 2aa4b5d778..83d2676f6c 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -544,6 +544,7 @@ config ASPEED_SOC
> > bool
> > default y
> > depends on TCG && ARM
> > + imply PCI_DEVICES
> > select DS1338
> > select FTGMAC100
> > select I2C
> > @@ -564,6 +565,8 @@ config ASPEED_SOC
> > select MAX31785
> > select FSI_APB2OPB_ASPEED
> > select AT24C
> > + select PCI_EXPRESS
> > + select PCI_EXPRESS_ASPEED
> >
> > config MPS2
> > bool
> > diff --git a/hw/pci-host/Kconfig b/hw/pci-host/Kconfig index
> > 9824fa188d..8cbb8304a3 100644
> > --- a/hw/pci-host/Kconfig
> > +++ b/hw/pci-host/Kconfig
> > @@ -46,6 +46,10 @@ config PCI_I440FX
> > select PCI
> > select PAM
> >
> > +config PCI_EXPRESS_ASPEED
> > + bool
> > + select PCI_EXPRESS
> > +
> > config PCI_EXPRESS_Q35
> > bool
> > select PCI_EXPRESS
> > diff --git a/hw/pci-host/meson.build b/hw/pci-host/meson.build index
> > 937a0f72ac..86b754d0b0 100644
> > --- a/hw/pci-host/meson.build
> > +++ b/hw/pci-host/meson.build
> > @@ -2,6 +2,7 @@ pci_ss = ss.source_set()
> > pci_ss.add(when: 'CONFIG_PAM', if_true: files('pam.c'))
> > pci_ss.add(when: 'CONFIG_PCI_BONITO', if_true: files('bonito.c'))
> > pci_ss.add(when: 'CONFIG_GT64120', if_true: files('gt64120.c'))
> > +pci_ss.add(when: 'CONFIG_PCI_EXPRESS_ASPEED', if_true:
> > +files('aspeed_pcie.c'))
> > pci_ss.add(when: 'CONFIG_PCI_EXPRESS_DESIGNWARE', if_true:
> files('designware.c'))
> > pci_ss.add(when: 'CONFIG_PCI_EXPRESS_GENERIC_BRIDGE', if_true:
> files('gpex.c'))
> > pci_ss.add(when: ['CONFIG_PCI_EXPRESS_GENERIC_BRIDGE',
> > 'CONFIG_ACPI'], if_true: files('gpex-acpi.c')) diff --git
> > a/hw/pci-host/trace-events b/hw/pci-host/trace-events index
> > 0a816b9aa1..3438516756 100644
> > --- a/hw/pci-host/trace-events
> > +++ b/hw/pci-host/trace-events
> > @@ -1,5 +1,9 @@
> > # See docs/devel/tracing.rst for syntax documentation.
> >
> > +# aspeed_pcie.c
> > +aspeed_pcie_phy_read(uint32_t id, uint64_t addr, uint32_t value) "%d:
> > +addr 0x%" PRIx64 " value 0x%" PRIx32 aspeed_pcie_phy_write(uint32_t
> > +id, uint64_t addr, uint32_t value) "%d: addr 0x%" PRIx64 " value 0x%"
> > +PRIx32
> > +
> > # bonito.c
> > bonito_spciconf_small_access(uint64_t addr, unsigned size) "PCI config
> address is smaller then 32-bit, addr: 0x%"PRIx64", size: %u"
> >
© 2016 - 2025 Red Hat, Inc.