[Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device

Eddie James posted 1 patch 4 years, 10 months ago
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Test s390x passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1559599768-9176-1-git-send-email-eajames@linux.ibm.com
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
hw/arm/aspeed_soc.c           |  14 ++++
hw/misc/Makefile.objs         |   2 +-
hw/misc/aspeed_xdma.c         | 156 ++++++++++++++++++++++++++++++++++++++++++
include/hw/arm/aspeed_soc.h   |   2 +
include/hw/misc/aspeed_xdma.h |  31 +++++++++
5 files changed, 204 insertions(+), 1 deletion(-)
create mode 100644 hw/misc/aspeed_xdma.c
create mode 100644 include/hw/misc/aspeed_xdma.h
[Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
Posted by Eddie James 4 years, 10 months ago
The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
between the SOC (acting as a BMC) and a host processor in a server.

The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
enable it for all of those.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 hw/arm/aspeed_soc.c           |  14 ++++
 hw/misc/Makefile.objs         |   2 +-
 hw/misc/aspeed_xdma.c         | 156 ++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/aspeed_soc.h   |   2 +
 include/hw/misc/aspeed_xdma.h |  31 +++++++++
 5 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 hw/misc/aspeed_xdma.c
 create mode 100644 include/hw/misc/aspeed_xdma.h

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index faff42b..b25bb18 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -31,6 +31,7 @@
 #define ASPEED_SOC_VIC_BASE         0x1E6C0000
 #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
 #define ASPEED_SOC_SCU_BASE         0x1E6E2000
+#define ASPEED_SOC_XDMA_BASE        0x1E6E7000
 #define ASPEED_SOC_SRAM_BASE        0x1E720000
 #define ASPEED_SOC_TIMER_BASE       0x1E782000
 #define ASPEED_SOC_WDT_BASE         0x1E785000
@@ -159,6 +160,9 @@ static void aspeed_soc_init(Object *obj)
 
     sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
                           sizeof(s->ftgmac100), TYPE_FTGMAC100);
+
+    sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
+                          TYPE_ASPEED_XDMA);
 }
 
 static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -298,6 +302,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
                        qdev_get_gpio_in(DEVICE(&s->vic), 2));
+
+    /* XDMA */
+    object_property_set_bool(OBJECT(&s->xdma), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0, ASPEED_SOC_XDMA_BASE);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
+                       qdev_get_gpio_in(DEVICE(&s->vic), 6));
 }
 
 static void aspeed_soc_class_init(ObjectClass *oc, void *data)
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 77b9df9..a4483af 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -74,7 +74,7 @@ obj-$(CONFIG_ARMSSE_MHU) += armsse-mhu.o
 
 obj-$(CONFIG_PVPANIC) += pvpanic.o
 obj-$(CONFIG_AUX) += auxbus.o
-obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o aspeed_xdma.o
 obj-$(CONFIG_MSF2) += msf2-sysreg.o
 obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
 
diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c
new file mode 100644
index 0000000..fe3a32e
--- /dev/null
+++ b/hw/misc/aspeed_xdma.c
@@ -0,0 +1,156 @@
+/*
+ * ASPEED XDMA Controller
+ * Eddie James <eajames@linux.ibm.com>
+ *
+ * Copyright (C) 2019 IBM Corp
+ * SPDX-License-Identifer: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/error-report.h"
+#include "hw/misc/aspeed_xdma.h"
+#include "qapi/error.h"
+
+#define XDMA_BMC_CMDQ_ADDR         0x10
+#define XDMA_BMC_CMDQ_ENDP         0x14
+#define XDMA_BMC_CMDQ_WRP          0x18
+#define  XDMA_BMC_CMDQ_W_MASK      0x0003FFFF
+#define XDMA_BMC_CMDQ_RDP          0x1C
+#define  XDMA_BMC_CMDQ_RDP_MAGIC   0xEE882266
+#define XDMA_IRQ_ENG_CTRL          0x20
+#define  XDMA_IRQ_ENG_CTRL_US_COMP BIT(4)
+#define  XDMA_IRQ_ENG_CTRL_DS_COMP BIT(5)
+#define  XDMA_IRQ_ENG_CTRL_W_MASK  0xBFEFF07F
+#define XDMA_IRQ_ENG_STAT          0x24
+#define  XDMA_IRQ_ENG_STAT_US_COMP BIT(4)
+#define  XDMA_IRQ_ENG_STAT_DS_COMP BIT(5)
+#define  XDMA_IRQ_ENG_STAT_RESET   0xF8000000
+
+#define TO_REG(addr) ((addr) / sizeof(uint32_t))
+
+static uint64_t aspeed_xdma_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    uint32_t val = 0;
+    AspeedXDMAState *xdma = opaque;
+
+    if (addr < ASPEED_XDMA_REG_SIZE) {
+        val = xdma->regs[TO_REG(addr)];
+    }
+
+    return (uint64_t)val;
+}
+
+static void aspeed_xdma_write(void *opaque, hwaddr addr, uint64_t val,
+                              unsigned int size)
+{
+    unsigned int idx;
+    uint32_t val32 = (uint32_t)val;
+    AspeedXDMAState *xdma = opaque;
+
+    if (addr >= ASPEED_XDMA_REG_SIZE) {
+        return;
+    }
+
+    switch (addr) {
+    case XDMA_BMC_CMDQ_ENDP:
+        xdma->regs[TO_REG(addr)] = val32 & XDMA_BMC_CMDQ_W_MASK;
+        break;
+    case XDMA_BMC_CMDQ_WRP:
+        idx = TO_REG(addr);
+        xdma->regs[idx] = val32 & XDMA_BMC_CMDQ_W_MASK;
+        xdma->regs[TO_REG(XDMA_BMC_CMDQ_RDP)] = xdma->regs[idx];
+
+        if (xdma->bmc_cmdq_readp_set) {
+            xdma->bmc_cmdq_readp_set = 0;
+        } else {
+            xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] |=
+                XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP;
+
+            if (xdma->regs[TO_REG(XDMA_IRQ_ENG_CTRL)] &
+                (XDMA_IRQ_ENG_CTRL_US_COMP | XDMA_IRQ_ENG_CTRL_DS_COMP))
+                qemu_irq_raise(xdma->irq);
+        }
+        break;
+    case XDMA_BMC_CMDQ_RDP:
+        if (val32 == XDMA_BMC_CMDQ_RDP_MAGIC) {
+            xdma->bmc_cmdq_readp_set = 1;
+        }
+        break;
+    case XDMA_IRQ_ENG_CTRL:
+        xdma->regs[TO_REG(addr)] = val32 & XDMA_IRQ_ENG_CTRL_W_MASK;
+        break;
+    case XDMA_IRQ_ENG_STAT:
+        idx = TO_REG(addr);
+        if (val32 & (XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP)) {
+            xdma->regs[TO_REG(addr)] &=
+                ~(XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP);
+            qemu_irq_lower(xdma->irq);
+        }
+        break;
+    default:
+        xdma->regs[TO_REG(addr)] = val32;
+        break;
+    }
+}
+
+static const MemoryRegionOps aspeed_xdma_ops = {
+    .read = aspeed_xdma_read,
+    .write = aspeed_xdma_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid.min_access_size = 4,
+    .valid.max_access_size = 4,
+};
+
+static void aspeed_xdma_realize(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
+
+    sysbus_init_irq(sbd, &xdma->irq);
+    memory_region_init_io(&xdma->iomem, OBJECT(xdma), &aspeed_xdma_ops, xdma,
+                          TYPE_ASPEED_XDMA, ASPEED_XDMA_MEM_SIZE);
+    sysbus_init_mmio(sbd, &xdma->iomem);
+}
+
+static void aspeed_xdma_reset(DeviceState *dev)
+{
+    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
+
+    xdma->bmc_cmdq_readp_set = 0;
+    memset(xdma->regs, 0, ASPEED_XDMA_REG_SIZE);
+    xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] = XDMA_IRQ_ENG_STAT_RESET;
+
+    qemu_irq_lower(xdma->irq);
+}
+
+static const VMStateDescription aspeed_xdma_vmstate = {
+    .name = TYPE_ASPEED_XDMA,
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, AspeedXDMAState, ASPEED_XDMA_NUM_REGS),
+        VMSTATE_END_OF_LIST(),
+    },
+};
+
+static void aspeed_xdma_class_init(ObjectClass *classp, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(classp);
+
+    dc->realize = aspeed_xdma_realize;
+    dc->reset = aspeed_xdma_reset;
+    dc->vmsd = &aspeed_xdma_vmstate;
+}
+
+static const TypeInfo aspeed_xdma_info = {
+    .name          = TYPE_ASPEED_XDMA,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(AspeedXDMAState),
+    .class_init    = aspeed_xdma_class_init,
+};
+
+static void aspeed_xdma_register_type(void)
+{
+    type_register_static(&aspeed_xdma_info);
+}
+type_init(aspeed_xdma_register_type);
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 836b2ba..0329247 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -20,6 +20,7 @@
 #include "hw/ssi/aspeed_smc.h"
 #include "hw/watchdog/wdt_aspeed.h"
 #include "hw/net/ftgmac100.h"
+#include "hw/misc/aspeed_xdma.h"
 
 #define ASPEED_SPIS_NUM  2
 #define ASPEED_WDTS_NUM  3
@@ -40,6 +41,7 @@ typedef struct AspeedSoCState {
     AspeedSDMCState sdmc;
     AspeedWDTState wdt[ASPEED_WDTS_NUM];
     FTGMAC100State ftgmac100;
+    AspeedXDMAState xdma;
 } AspeedSoCState;
 
 #define TYPE_ASPEED_SOC "aspeed-soc"
diff --git a/include/hw/misc/aspeed_xdma.h b/include/hw/misc/aspeed_xdma.h
new file mode 100644
index 0000000..d19e9a7
--- /dev/null
+++ b/include/hw/misc/aspeed_xdma.h
@@ -0,0 +1,31 @@
+/*
+ * ASPEED XDMA Controller
+ * Eddie James <eajames@linux.ibm.com>
+ *
+ * Copyright (C) 2019 IBM Corp.
+ * SPDX-License-Identifer: GPL-2.0-or-later
+ */
+
+#ifndef ASPEED_XDMA_H
+#define ASPEED_XDMA_H
+
+#include "hw/sysbus.h"
+
+#define TYPE_ASPEED_XDMA "aspeed.xdma"
+#define ASPEED_XDMA(obj) OBJECT_CHECK(AspeedXDMAState, (obj), TYPE_ASPEED_XDMA)
+
+#define ASPEED_XDMA_MEM_SIZE 0x1000
+#define ASPEED_XDMA_NUM_REGS (ASPEED_XDMA_REG_SIZE / sizeof(uint32_t))
+#define ASPEED_XDMA_REG_SIZE 0x7C
+
+typedef struct AspeedXDMAState {
+    SysBusDevice parent;
+
+    MemoryRegion iomem;
+    qemu_irq irq;
+
+    char bmc_cmdq_readp_set;
+    uint32_t regs[ASPEED_XDMA_NUM_REGS];
+} AspeedXDMAState;
+
+#endif /* ASPEED_XDMA_H */
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
Posted by Cédric Le Goater 4 years, 10 months ago
Hello Eddie,

On 04/06/2019 00:09, Eddie James wrote:
> The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
> between the SOC (acting as a BMC) and a host processor in a server.
> 
> The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
> enable it for all of those.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

This looks correct to me. It's sufficient to exercise the BMC driver.

However, we will need to rebase on an Aspeed patchset I sent earlier :

   http://patchwork.ozlabs.org/cover/1105343/

I can do that and include the patch in my tree for the moment. 


For my understanding, how can we interact with the model and pretend
there is a host side ? 

Thanks,

C. 

> ---
>  hw/arm/aspeed_soc.c           |  14 ++++
>  hw/misc/Makefile.objs         |   2 +-
>  hw/misc/aspeed_xdma.c         | 156 ++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/aspeed_soc.h   |   2 +
>  include/hw/misc/aspeed_xdma.h |  31 +++++++++
>  5 files changed, 204 insertions(+), 1 deletion(-)
>  create mode 100644 hw/misc/aspeed_xdma.c
>  create mode 100644 include/hw/misc/aspeed_xdma.h
> 
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index faff42b..b25bb18 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -31,6 +31,7 @@
>  #define ASPEED_SOC_VIC_BASE         0x1E6C0000
>  #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
>  #define ASPEED_SOC_SCU_BASE         0x1E6E2000
> +#define ASPEED_SOC_XDMA_BASE        0x1E6E7000
>  #define ASPEED_SOC_SRAM_BASE        0x1E720000
>  #define ASPEED_SOC_TIMER_BASE       0x1E782000
>  #define ASPEED_SOC_WDT_BASE         0x1E785000
> @@ -159,6 +160,9 @@ static void aspeed_soc_init(Object *obj)
>  
>      sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
>                            sizeof(s->ftgmac100), TYPE_FTGMAC100);
> +
> +    sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
> +                          TYPE_ASPEED_XDMA);
>  }
>  
>  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -298,6 +302,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
>                         qdev_get_gpio_in(DEVICE(&s->vic), 2));
> +
> +    /* XDMA */
> +    object_property_set_bool(OBJECT(&s->xdma), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0, ASPEED_SOC_XDMA_BASE);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->vic), 6));
>  }
>  
>  static void aspeed_soc_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 77b9df9..a4483af 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -74,7 +74,7 @@ obj-$(CONFIG_ARMSSE_MHU) += armsse-mhu.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
>  obj-$(CONFIG_AUX) += auxbus.o
> -obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
> +obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o aspeed_xdma.o
>  obj-$(CONFIG_MSF2) += msf2-sysreg.o
>  obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>  
> diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c
> new file mode 100644
> index 0000000..fe3a32e
> --- /dev/null
> +++ b/hw/misc/aspeed_xdma.c
> @@ -0,0 +1,156 @@
> +/*
> + * ASPEED XDMA Controller
> + * Eddie James <eajames@linux.ibm.com>
> + *
> + * Copyright (C) 2019 IBM Corp
> + * SPDX-License-Identifer: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/error-report.h"
> +#include "hw/misc/aspeed_xdma.h"
> +#include "qapi/error.h"
> +
> +#define XDMA_BMC_CMDQ_ADDR         0x10
> +#define XDMA_BMC_CMDQ_ENDP         0x14
> +#define XDMA_BMC_CMDQ_WRP          0x18
> +#define  XDMA_BMC_CMDQ_W_MASK      0x0003FFFF
> +#define XDMA_BMC_CMDQ_RDP          0x1C
> +#define  XDMA_BMC_CMDQ_RDP_MAGIC   0xEE882266
> +#define XDMA_IRQ_ENG_CTRL          0x20
> +#define  XDMA_IRQ_ENG_CTRL_US_COMP BIT(4)
> +#define  XDMA_IRQ_ENG_CTRL_DS_COMP BIT(5)
> +#define  XDMA_IRQ_ENG_CTRL_W_MASK  0xBFEFF07F
> +#define XDMA_IRQ_ENG_STAT          0x24
> +#define  XDMA_IRQ_ENG_STAT_US_COMP BIT(4)
> +#define  XDMA_IRQ_ENG_STAT_DS_COMP BIT(5)
> +#define  XDMA_IRQ_ENG_STAT_RESET   0xF8000000
> +
> +#define TO_REG(addr) ((addr) / sizeof(uint32_t))
> +
> +static uint64_t aspeed_xdma_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    uint32_t val = 0;
> +    AspeedXDMAState *xdma = opaque;
> +
> +    if (addr < ASPEED_XDMA_REG_SIZE) {
> +        val = xdma->regs[TO_REG(addr)];
> +    }
> +
> +    return (uint64_t)val;
> +}
> +
> +static void aspeed_xdma_write(void *opaque, hwaddr addr, uint64_t val,
> +                              unsigned int size)
> +{
> +    unsigned int idx;
> +    uint32_t val32 = (uint32_t)val;
> +    AspeedXDMAState *xdma = opaque;
> +
> +    if (addr >= ASPEED_XDMA_REG_SIZE) {
> +        return;
> +    }
> +
> +    switch (addr) {
> +    case XDMA_BMC_CMDQ_ENDP:
> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_BMC_CMDQ_W_MASK;
> +        break;
> +    case XDMA_BMC_CMDQ_WRP:
> +        idx = TO_REG(addr);
> +        xdma->regs[idx] = val32 & XDMA_BMC_CMDQ_W_MASK;
> +        xdma->regs[TO_REG(XDMA_BMC_CMDQ_RDP)] = xdma->regs[idx];
> +
> +        if (xdma->bmc_cmdq_readp_set) {
> +            xdma->bmc_cmdq_readp_set = 0;
> +        } else {
> +            xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] |=
> +                XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP;
> +
> +            if (xdma->regs[TO_REG(XDMA_IRQ_ENG_CTRL)] &
> +                (XDMA_IRQ_ENG_CTRL_US_COMP | XDMA_IRQ_ENG_CTRL_DS_COMP))
> +                qemu_irq_raise(xdma->irq);
> +        }
> +        break;
> +    case XDMA_BMC_CMDQ_RDP:
> +        if (val32 == XDMA_BMC_CMDQ_RDP_MAGIC) {
> +            xdma->bmc_cmdq_readp_set = 1;
> +        }
> +        break;
> +    case XDMA_IRQ_ENG_CTRL:
> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_IRQ_ENG_CTRL_W_MASK;
> +        break;
> +    case XDMA_IRQ_ENG_STAT:
> +        idx = TO_REG(addr);
> +        if (val32 & (XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP)) {
> +            xdma->regs[TO_REG(addr)] &=
> +                ~(XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP);
> +            qemu_irq_lower(xdma->irq);
> +        }
> +        break;
> +    default:
> +        xdma->regs[TO_REG(addr)] = val32;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps aspeed_xdma_ops = {
> +    .read = aspeed_xdma_read,
> +    .write = aspeed_xdma_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +};
> +
> +static void aspeed_xdma_realize(DeviceState *dev, Error **errp)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
> +
> +    sysbus_init_irq(sbd, &xdma->irq);
> +    memory_region_init_io(&xdma->iomem, OBJECT(xdma), &aspeed_xdma_ops, xdma,
> +                          TYPE_ASPEED_XDMA, ASPEED_XDMA_MEM_SIZE);
> +    sysbus_init_mmio(sbd, &xdma->iomem);
> +}
> +
> +static void aspeed_xdma_reset(DeviceState *dev)
> +{
> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
> +
> +    xdma->bmc_cmdq_readp_set = 0;
> +    memset(xdma->regs, 0, ASPEED_XDMA_REG_SIZE);
> +    xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] = XDMA_IRQ_ENG_STAT_RESET;
> +
> +    qemu_irq_lower(xdma->irq);
> +}
> +
> +static const VMStateDescription aspeed_xdma_vmstate = {
> +    .name = TYPE_ASPEED_XDMA,
> +    .version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32_ARRAY(regs, AspeedXDMAState, ASPEED_XDMA_NUM_REGS),
> +        VMSTATE_END_OF_LIST(),
> +    },
> +};
> +
> +static void aspeed_xdma_class_init(ObjectClass *classp, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(classp);
> +
> +    dc->realize = aspeed_xdma_realize;
> +    dc->reset = aspeed_xdma_reset;
> +    dc->vmsd = &aspeed_xdma_vmstate;
> +}
> +
> +static const TypeInfo aspeed_xdma_info = {
> +    .name          = TYPE_ASPEED_XDMA,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(AspeedXDMAState),
> +    .class_init    = aspeed_xdma_class_init,
> +};
> +
> +static void aspeed_xdma_register_type(void)
> +{
> +    type_register_static(&aspeed_xdma_info);
> +}
> +type_init(aspeed_xdma_register_type);
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 836b2ba..0329247 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -20,6 +20,7 @@
>  #include "hw/ssi/aspeed_smc.h"
>  #include "hw/watchdog/wdt_aspeed.h"
>  #include "hw/net/ftgmac100.h"
> +#include "hw/misc/aspeed_xdma.h"
>  
>  #define ASPEED_SPIS_NUM  2
>  #define ASPEED_WDTS_NUM  3
> @@ -40,6 +41,7 @@ typedef struct AspeedSoCState {
>      AspeedSDMCState sdmc;
>      AspeedWDTState wdt[ASPEED_WDTS_NUM];
>      FTGMAC100State ftgmac100;
> +    AspeedXDMAState xdma;
>  } AspeedSoCState;
>  
>  #define TYPE_ASPEED_SOC "aspeed-soc"
> diff --git a/include/hw/misc/aspeed_xdma.h b/include/hw/misc/aspeed_xdma.h
> new file mode 100644
> index 0000000..d19e9a7
> --- /dev/null
> +++ b/include/hw/misc/aspeed_xdma.h
> @@ -0,0 +1,31 @@
> +/*
> + * ASPEED XDMA Controller
> + * Eddie James <eajames@linux.ibm.com>
> + *
> + * Copyright (C) 2019 IBM Corp.
> + * SPDX-License-Identifer: GPL-2.0-or-later
> + */
> +
> +#ifndef ASPEED_XDMA_H
> +#define ASPEED_XDMA_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_ASPEED_XDMA "aspeed.xdma"
> +#define ASPEED_XDMA(obj) OBJECT_CHECK(AspeedXDMAState, (obj), TYPE_ASPEED_XDMA)
> +
> +#define ASPEED_XDMA_MEM_SIZE 0x1000
> +#define ASPEED_XDMA_NUM_REGS (ASPEED_XDMA_REG_SIZE / sizeof(uint32_t))
> +#define ASPEED_XDMA_REG_SIZE 0x7C
> +
> +typedef struct AspeedXDMAState {
> +    SysBusDevice parent;
> +
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    char bmc_cmdq_readp_set;
> +    uint32_t regs[ASPEED_XDMA_NUM_REGS];
> +} AspeedXDMAState;
> +
> +#endif /* ASPEED_XDMA_H */
> 


Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
Posted by Eddie James 4 years, 10 months ago
On 6/6/19 1:16 AM, Cédric Le Goater wrote:
> Hello Eddie,
>
> On 04/06/2019 00:09, Eddie James wrote:
>> The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
>> between the SOC (acting as a BMC) and a host processor in a server.
>>
>> The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
>> enable it for all of those.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> This looks correct to me. It's sufficient to exercise the BMC driver.
>
> However, we will need to rebase on an Aspeed patchset I sent earlier :
>
>     http://patchwork.ozlabs.org/cover/1105343/
>
> I can do that and include the patch in my tree for the moment.


I built and tested the model on your tree, so let me know if you want me 
to send that patch instead?


>
>
> For my understanding, how can we interact with the model and pretend
> there is a host side ?


I have an application that can test the driver here: 
https://github.com/eddiejames/xdma-test

But as you say there is no host side; the operations don't copy any 
memory anywhere. Joel suggested adding some way to copy and check some 
dummy memory contents, but I haven't looked into that yet.


Thanks,

Eddie


>
> Thanks,
>
> C.
>
>> ---
>>   hw/arm/aspeed_soc.c           |  14 ++++
>>   hw/misc/Makefile.objs         |   2 +-
>>   hw/misc/aspeed_xdma.c         | 156 ++++++++++++++++++++++++++++++++++++++++++
>>   include/hw/arm/aspeed_soc.h   |   2 +
>>   include/hw/misc/aspeed_xdma.h |  31 +++++++++
>>   5 files changed, 204 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/misc/aspeed_xdma.c
>>   create mode 100644 include/hw/misc/aspeed_xdma.h
>>
>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>> index faff42b..b25bb18 100644
>> --- a/hw/arm/aspeed_soc.c
>> +++ b/hw/arm/aspeed_soc.c
>> @@ -31,6 +31,7 @@
>>   #define ASPEED_SOC_VIC_BASE         0x1E6C0000
>>   #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
>>   #define ASPEED_SOC_SCU_BASE         0x1E6E2000
>> +#define ASPEED_SOC_XDMA_BASE        0x1E6E7000
>>   #define ASPEED_SOC_SRAM_BASE        0x1E720000
>>   #define ASPEED_SOC_TIMER_BASE       0x1E782000
>>   #define ASPEED_SOC_WDT_BASE         0x1E785000
>> @@ -159,6 +160,9 @@ static void aspeed_soc_init(Object *obj)
>>   
>>       sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
>>                             sizeof(s->ftgmac100), TYPE_FTGMAC100);
>> +
>> +    sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
>> +                          TYPE_ASPEED_XDMA);
>>   }
>>   
>>   static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> @@ -298,6 +302,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>>       sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
>>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
>>                          qdev_get_gpio_in(DEVICE(&s->vic), 2));
>> +
>> +    /* XDMA */
>> +    object_property_set_bool(OBJECT(&s->xdma), true, "realized", &err);
>> +    if (err) {
>> +        error_propagate(errp, err);
>> +        return;
>> +    }
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0, ASPEED_SOC_XDMA_BASE);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
>> +                       qdev_get_gpio_in(DEVICE(&s->vic), 6));
>>   }
>>   
>>   static void aspeed_soc_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 77b9df9..a4483af 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -74,7 +74,7 @@ obj-$(CONFIG_ARMSSE_MHU) += armsse-mhu.o
>>   
>>   obj-$(CONFIG_PVPANIC) += pvpanic.o
>>   obj-$(CONFIG_AUX) += auxbus.o
>> -obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>> +obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o aspeed_xdma.o
>>   obj-$(CONFIG_MSF2) += msf2-sysreg.o
>>   obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>>   
>> diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c
>> new file mode 100644
>> index 0000000..fe3a32e
>> --- /dev/null
>> +++ b/hw/misc/aspeed_xdma.c
>> @@ -0,0 +1,156 @@
>> +/*
>> + * ASPEED XDMA Controller
>> + * Eddie James <eajames@linux.ibm.com>
>> + *
>> + * Copyright (C) 2019 IBM Corp
>> + * SPDX-License-Identifer: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "qemu/error-report.h"
>> +#include "hw/misc/aspeed_xdma.h"
>> +#include "qapi/error.h"
>> +
>> +#define XDMA_BMC_CMDQ_ADDR         0x10
>> +#define XDMA_BMC_CMDQ_ENDP         0x14
>> +#define XDMA_BMC_CMDQ_WRP          0x18
>> +#define  XDMA_BMC_CMDQ_W_MASK      0x0003FFFF
>> +#define XDMA_BMC_CMDQ_RDP          0x1C
>> +#define  XDMA_BMC_CMDQ_RDP_MAGIC   0xEE882266
>> +#define XDMA_IRQ_ENG_CTRL          0x20
>> +#define  XDMA_IRQ_ENG_CTRL_US_COMP BIT(4)
>> +#define  XDMA_IRQ_ENG_CTRL_DS_COMP BIT(5)
>> +#define  XDMA_IRQ_ENG_CTRL_W_MASK  0xBFEFF07F
>> +#define XDMA_IRQ_ENG_STAT          0x24
>> +#define  XDMA_IRQ_ENG_STAT_US_COMP BIT(4)
>> +#define  XDMA_IRQ_ENG_STAT_DS_COMP BIT(5)
>> +#define  XDMA_IRQ_ENG_STAT_RESET   0xF8000000
>> +
>> +#define TO_REG(addr) ((addr) / sizeof(uint32_t))
>> +
>> +static uint64_t aspeed_xdma_read(void *opaque, hwaddr addr, unsigned int size)
>> +{
>> +    uint32_t val = 0;
>> +    AspeedXDMAState *xdma = opaque;
>> +
>> +    if (addr < ASPEED_XDMA_REG_SIZE) {
>> +        val = xdma->regs[TO_REG(addr)];
>> +    }
>> +
>> +    return (uint64_t)val;
>> +}
>> +
>> +static void aspeed_xdma_write(void *opaque, hwaddr addr, uint64_t val,
>> +                              unsigned int size)
>> +{
>> +    unsigned int idx;
>> +    uint32_t val32 = (uint32_t)val;
>> +    AspeedXDMAState *xdma = opaque;
>> +
>> +    if (addr >= ASPEED_XDMA_REG_SIZE) {
>> +        return;
>> +    }
>> +
>> +    switch (addr) {
>> +    case XDMA_BMC_CMDQ_ENDP:
>> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_BMC_CMDQ_W_MASK;
>> +        break;
>> +    case XDMA_BMC_CMDQ_WRP:
>> +        idx = TO_REG(addr);
>> +        xdma->regs[idx] = val32 & XDMA_BMC_CMDQ_W_MASK;
>> +        xdma->regs[TO_REG(XDMA_BMC_CMDQ_RDP)] = xdma->regs[idx];
>> +
>> +        if (xdma->bmc_cmdq_readp_set) {
>> +            xdma->bmc_cmdq_readp_set = 0;
>> +        } else {
>> +            xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] |=
>> +                XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP;
>> +
>> +            if (xdma->regs[TO_REG(XDMA_IRQ_ENG_CTRL)] &
>> +                (XDMA_IRQ_ENG_CTRL_US_COMP | XDMA_IRQ_ENG_CTRL_DS_COMP))
>> +                qemu_irq_raise(xdma->irq);
>> +        }
>> +        break;
>> +    case XDMA_BMC_CMDQ_RDP:
>> +        if (val32 == XDMA_BMC_CMDQ_RDP_MAGIC) {
>> +            xdma->bmc_cmdq_readp_set = 1;
>> +        }
>> +        break;
>> +    case XDMA_IRQ_ENG_CTRL:
>> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_IRQ_ENG_CTRL_W_MASK;
>> +        break;
>> +    case XDMA_IRQ_ENG_STAT:
>> +        idx = TO_REG(addr);
>> +        if (val32 & (XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP)) {
>> +            xdma->regs[TO_REG(addr)] &=
>> +                ~(XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP);
>> +            qemu_irq_lower(xdma->irq);
>> +        }
>> +        break;
>> +    default:
>> +        xdma->regs[TO_REG(addr)] = val32;
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps aspeed_xdma_ops = {
>> +    .read = aspeed_xdma_read,
>> +    .write = aspeed_xdma_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .valid.min_access_size = 4,
>> +    .valid.max_access_size = 4,
>> +};
>> +
>> +static void aspeed_xdma_realize(DeviceState *dev, Error **errp)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
>> +
>> +    sysbus_init_irq(sbd, &xdma->irq);
>> +    memory_region_init_io(&xdma->iomem, OBJECT(xdma), &aspeed_xdma_ops, xdma,
>> +                          TYPE_ASPEED_XDMA, ASPEED_XDMA_MEM_SIZE);
>> +    sysbus_init_mmio(sbd, &xdma->iomem);
>> +}
>> +
>> +static void aspeed_xdma_reset(DeviceState *dev)
>> +{
>> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
>> +
>> +    xdma->bmc_cmdq_readp_set = 0;
>> +    memset(xdma->regs, 0, ASPEED_XDMA_REG_SIZE);
>> +    xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] = XDMA_IRQ_ENG_STAT_RESET;
>> +
>> +    qemu_irq_lower(xdma->irq);
>> +}
>> +
>> +static const VMStateDescription aspeed_xdma_vmstate = {
>> +    .name = TYPE_ASPEED_XDMA,
>> +    .version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32_ARRAY(regs, AspeedXDMAState, ASPEED_XDMA_NUM_REGS),
>> +        VMSTATE_END_OF_LIST(),
>> +    },
>> +};
>> +
>> +static void aspeed_xdma_class_init(ObjectClass *classp, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(classp);
>> +
>> +    dc->realize = aspeed_xdma_realize;
>> +    dc->reset = aspeed_xdma_reset;
>> +    dc->vmsd = &aspeed_xdma_vmstate;
>> +}
>> +
>> +static const TypeInfo aspeed_xdma_info = {
>> +    .name          = TYPE_ASPEED_XDMA,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(AspeedXDMAState),
>> +    .class_init    = aspeed_xdma_class_init,
>> +};
>> +
>> +static void aspeed_xdma_register_type(void)
>> +{
>> +    type_register_static(&aspeed_xdma_info);
>> +}
>> +type_init(aspeed_xdma_register_type);
>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>> index 836b2ba..0329247 100644
>> --- a/include/hw/arm/aspeed_soc.h
>> +++ b/include/hw/arm/aspeed_soc.h
>> @@ -20,6 +20,7 @@
>>   #include "hw/ssi/aspeed_smc.h"
>>   #include "hw/watchdog/wdt_aspeed.h"
>>   #include "hw/net/ftgmac100.h"
>> +#include "hw/misc/aspeed_xdma.h"
>>   
>>   #define ASPEED_SPIS_NUM  2
>>   #define ASPEED_WDTS_NUM  3
>> @@ -40,6 +41,7 @@ typedef struct AspeedSoCState {
>>       AspeedSDMCState sdmc;
>>       AspeedWDTState wdt[ASPEED_WDTS_NUM];
>>       FTGMAC100State ftgmac100;
>> +    AspeedXDMAState xdma;
>>   } AspeedSoCState;
>>   
>>   #define TYPE_ASPEED_SOC "aspeed-soc"
>> diff --git a/include/hw/misc/aspeed_xdma.h b/include/hw/misc/aspeed_xdma.h
>> new file mode 100644
>> index 0000000..d19e9a7
>> --- /dev/null
>> +++ b/include/hw/misc/aspeed_xdma.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * ASPEED XDMA Controller
>> + * Eddie James <eajames@linux.ibm.com>
>> + *
>> + * Copyright (C) 2019 IBM Corp.
>> + * SPDX-License-Identifer: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef ASPEED_XDMA_H
>> +#define ASPEED_XDMA_H
>> +
>> +#include "hw/sysbus.h"
>> +
>> +#define TYPE_ASPEED_XDMA "aspeed.xdma"
>> +#define ASPEED_XDMA(obj) OBJECT_CHECK(AspeedXDMAState, (obj), TYPE_ASPEED_XDMA)
>> +
>> +#define ASPEED_XDMA_MEM_SIZE 0x1000
>> +#define ASPEED_XDMA_NUM_REGS (ASPEED_XDMA_REG_SIZE / sizeof(uint32_t))
>> +#define ASPEED_XDMA_REG_SIZE 0x7C
>> +
>> +typedef struct AspeedXDMAState {
>> +    SysBusDevice parent;
>> +
>> +    MemoryRegion iomem;
>> +    qemu_irq irq;
>> +
>> +    char bmc_cmdq_readp_set;
>> +    uint32_t regs[ASPEED_XDMA_NUM_REGS];
>> +} AspeedXDMAState;
>> +
>> +#endif /* ASPEED_XDMA_H */
>>


Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
Posted by Cédric Le Goater 4 years, 10 months ago
On 06/06/2019 23:42, Eddie James wrote:
> 
> On 6/6/19 1:16 AM, Cédric Le Goater wrote:
>> Hello Eddie,
>>
>> On 04/06/2019 00:09, Eddie James wrote:
>>> The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
>>> between the SOC (acting as a BMC) and a host processor in a server.
>>>
>>> The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
>>> enable it for all of those.
>>>
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> This looks correct to me. It's sufficient to exercise the BMC driver.
>>
>> However, we will need to rebase on an Aspeed patchset I sent earlier :
>>
>>     http://patchwork.ozlabs.org/cover/1105343/
>>
>> I can do that and include the patch in my tree for the moment.
> 
> 
> I built and tested the model on your tree, so let me know if you want me to send that patch instead?

yes. I will take it into my tree and reshuffle the code if needed.

The Aspeed patchset above needs more reviews and it is blocking 
other changes.

Thanks,

C. 

 
> 
> 
>>
>>
>> For my understanding, how can we interact with the model and pretend
>> there is a host side ?
> 
> 
> I have an application that can test the driver here: https://github.com/eddiejames/xdma-test
> 
> But as you say there is no host side; the operations don't copy any memory anywhere. Joel suggested adding some way to copy and check some dummy memory contents, but I haven't looked into that yet.
> 
> 
> Thanks,
> 
> Eddie
> 
> 
>>
>> Thanks,
>>
>> C.
>>
>>> ---
>>>   hw/arm/aspeed_soc.c           |  14 ++++
>>>   hw/misc/Makefile.objs         |   2 +-
>>>   hw/misc/aspeed_xdma.c         | 156 ++++++++++++++++++++++++++++++++++++++++++
>>>   include/hw/arm/aspeed_soc.h   |   2 +
>>>   include/hw/misc/aspeed_xdma.h |  31 +++++++++
>>>   5 files changed, 204 insertions(+), 1 deletion(-)
>>>   create mode 100644 hw/misc/aspeed_xdma.c
>>>   create mode 100644 include/hw/misc/aspeed_xdma.h
>>>
>>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>>> index faff42b..b25bb18 100644
>>> --- a/hw/arm/aspeed_soc.c
>>> +++ b/hw/arm/aspeed_soc.c
>>> @@ -31,6 +31,7 @@
>>>   #define ASPEED_SOC_VIC_BASE         0x1E6C0000
>>>   #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
>>>   #define ASPEED_SOC_SCU_BASE         0x1E6E2000
>>> +#define ASPEED_SOC_XDMA_BASE        0x1E6E7000
>>>   #define ASPEED_SOC_SRAM_BASE        0x1E720000
>>>   #define ASPEED_SOC_TIMER_BASE       0x1E782000
>>>   #define ASPEED_SOC_WDT_BASE         0x1E785000
>>> @@ -159,6 +160,9 @@ static void aspeed_soc_init(Object *obj)
>>>         sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
>>>                             sizeof(s->ftgmac100), TYPE_FTGMAC100);
>>> +
>>> +    sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
>>> +                          TYPE_ASPEED_XDMA);
>>>   }
>>>     static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>>> @@ -298,6 +302,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>>>       sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
>>>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
>>>                          qdev_get_gpio_in(DEVICE(&s->vic), 2));
>>> +
>>> +    /* XDMA */
>>> +    object_property_set_bool(OBJECT(&s->xdma), true, "realized", &err);
>>> +    if (err) {
>>> +        error_propagate(errp, err);
>>> +        return;
>>> +    }
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0, ASPEED_SOC_XDMA_BASE);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
>>> +                       qdev_get_gpio_in(DEVICE(&s->vic), 6));
>>>   }
>>>     static void aspeed_soc_class_init(ObjectClass *oc, void *data)
>>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>>> index 77b9df9..a4483af 100644
>>> --- a/hw/misc/Makefile.objs
>>> +++ b/hw/misc/Makefile.objs
>>> @@ -74,7 +74,7 @@ obj-$(CONFIG_ARMSSE_MHU) += armsse-mhu.o
>>>     obj-$(CONFIG_PVPANIC) += pvpanic.o
>>>   obj-$(CONFIG_AUX) += auxbus.o
>>> -obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>>> +obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o aspeed_xdma.o
>>>   obj-$(CONFIG_MSF2) += msf2-sysreg.o
>>>   obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>>>   diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c
>>> new file mode 100644
>>> index 0000000..fe3a32e
>>> --- /dev/null
>>> +++ b/hw/misc/aspeed_xdma.c
>>> @@ -0,0 +1,156 @@
>>> +/*
>>> + * ASPEED XDMA Controller
>>> + * Eddie James <eajames@linux.ibm.com>
>>> + *
>>> + * Copyright (C) 2019 IBM Corp
>>> + * SPDX-License-Identifer: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qemu/log.h"
>>> +#include "qemu/error-report.h"
>>> +#include "hw/misc/aspeed_xdma.h"
>>> +#include "qapi/error.h"
>>> +
>>> +#define XDMA_BMC_CMDQ_ADDR         0x10
>>> +#define XDMA_BMC_CMDQ_ENDP         0x14
>>> +#define XDMA_BMC_CMDQ_WRP          0x18
>>> +#define  XDMA_BMC_CMDQ_W_MASK      0x0003FFFF
>>> +#define XDMA_BMC_CMDQ_RDP          0x1C
>>> +#define  XDMA_BMC_CMDQ_RDP_MAGIC   0xEE882266
>>> +#define XDMA_IRQ_ENG_CTRL          0x20
>>> +#define  XDMA_IRQ_ENG_CTRL_US_COMP BIT(4)
>>> +#define  XDMA_IRQ_ENG_CTRL_DS_COMP BIT(5)
>>> +#define  XDMA_IRQ_ENG_CTRL_W_MASK  0xBFEFF07F
>>> +#define XDMA_IRQ_ENG_STAT          0x24
>>> +#define  XDMA_IRQ_ENG_STAT_US_COMP BIT(4)
>>> +#define  XDMA_IRQ_ENG_STAT_DS_COMP BIT(5)
>>> +#define  XDMA_IRQ_ENG_STAT_RESET   0xF8000000
>>> +
>>> +#define TO_REG(addr) ((addr) / sizeof(uint32_t))
>>> +
>>> +static uint64_t aspeed_xdma_read(void *opaque, hwaddr addr, unsigned int size)
>>> +{
>>> +    uint32_t val = 0;
>>> +    AspeedXDMAState *xdma = opaque;
>>> +
>>> +    if (addr < ASPEED_XDMA_REG_SIZE) {
>>> +        val = xdma->regs[TO_REG(addr)];
>>> +    }
>>> +
>>> +    return (uint64_t)val;
>>> +}
>>> +
>>> +static void aspeed_xdma_write(void *opaque, hwaddr addr, uint64_t val,
>>> +                              unsigned int size)
>>> +{
>>> +    unsigned int idx;
>>> +    uint32_t val32 = (uint32_t)val;
>>> +    AspeedXDMAState *xdma = opaque;
>>> +
>>> +    if (addr >= ASPEED_XDMA_REG_SIZE) {
>>> +        return;
>>> +    }
>>> +
>>> +    switch (addr) {
>>> +    case XDMA_BMC_CMDQ_ENDP:
>>> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_BMC_CMDQ_W_MASK;
>>> +        break;
>>> +    case XDMA_BMC_CMDQ_WRP:
>>> +        idx = TO_REG(addr);
>>> +        xdma->regs[idx] = val32 & XDMA_BMC_CMDQ_W_MASK;
>>> +        xdma->regs[TO_REG(XDMA_BMC_CMDQ_RDP)] = xdma->regs[idx];
>>> +
>>> +        if (xdma->bmc_cmdq_readp_set) {
>>> +            xdma->bmc_cmdq_readp_set = 0;
>>> +        } else {
>>> +            xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] |=
>>> +                XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP;
>>> +
>>> +            if (xdma->regs[TO_REG(XDMA_IRQ_ENG_CTRL)] &
>>> +                (XDMA_IRQ_ENG_CTRL_US_COMP | XDMA_IRQ_ENG_CTRL_DS_COMP))
>>> +                qemu_irq_raise(xdma->irq);
>>> +        }
>>> +        break;
>>> +    case XDMA_BMC_CMDQ_RDP:
>>> +        if (val32 == XDMA_BMC_CMDQ_RDP_MAGIC) {
>>> +            xdma->bmc_cmdq_readp_set = 1;
>>> +        }
>>> +        break;
>>> +    case XDMA_IRQ_ENG_CTRL:
>>> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_IRQ_ENG_CTRL_W_MASK;
>>> +        break;
>>> +    case XDMA_IRQ_ENG_STAT:
>>> +        idx = TO_REG(addr);
>>> +        if (val32 & (XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP)) {
>>> +            xdma->regs[TO_REG(addr)] &=
>>> +                ~(XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP);
>>> +            qemu_irq_lower(xdma->irq);
>>> +        }
>>> +        break;
>>> +    default:
>>> +        xdma->regs[TO_REG(addr)] = val32;
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static const MemoryRegionOps aspeed_xdma_ops = {
>>> +    .read = aspeed_xdma_read,
>>> +    .write = aspeed_xdma_write,
>>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>>> +    .valid.min_access_size = 4,
>>> +    .valid.max_access_size = 4,
>>> +};
>>> +
>>> +static void aspeed_xdma_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
>>> +
>>> +    sysbus_init_irq(sbd, &xdma->irq);
>>> +    memory_region_init_io(&xdma->iomem, OBJECT(xdma), &aspeed_xdma_ops, xdma,
>>> +                          TYPE_ASPEED_XDMA, ASPEED_XDMA_MEM_SIZE);
>>> +    sysbus_init_mmio(sbd, &xdma->iomem);
>>> +}
>>> +
>>> +static void aspeed_xdma_reset(DeviceState *dev)
>>> +{
>>> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
>>> +
>>> +    xdma->bmc_cmdq_readp_set = 0;
>>> +    memset(xdma->regs, 0, ASPEED_XDMA_REG_SIZE);
>>> +    xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] = XDMA_IRQ_ENG_STAT_RESET;
>>> +
>>> +    qemu_irq_lower(xdma->irq);
>>> +}
>>> +
>>> +static const VMStateDescription aspeed_xdma_vmstate = {
>>> +    .name = TYPE_ASPEED_XDMA,
>>> +    .version_id = 1,
>>> +    .fields = (VMStateField[]) {
>>> +        VMSTATE_UINT32_ARRAY(regs, AspeedXDMAState, ASPEED_XDMA_NUM_REGS),
>>> +        VMSTATE_END_OF_LIST(),
>>> +    },
>>> +};
>>> +
>>> +static void aspeed_xdma_class_init(ObjectClass *classp, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(classp);
>>> +
>>> +    dc->realize = aspeed_xdma_realize;
>>> +    dc->reset = aspeed_xdma_reset;
>>> +    dc->vmsd = &aspeed_xdma_vmstate;
>>> +}
>>> +
>>> +static const TypeInfo aspeed_xdma_info = {
>>> +    .name          = TYPE_ASPEED_XDMA,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(AspeedXDMAState),
>>> +    .class_init    = aspeed_xdma_class_init,
>>> +};
>>> +
>>> +static void aspeed_xdma_register_type(void)
>>> +{
>>> +    type_register_static(&aspeed_xdma_info);
>>> +}
>>> +type_init(aspeed_xdma_register_type);
>>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>>> index 836b2ba..0329247 100644
>>> --- a/include/hw/arm/aspeed_soc.h
>>> +++ b/include/hw/arm/aspeed_soc.h
>>> @@ -20,6 +20,7 @@
>>>   #include "hw/ssi/aspeed_smc.h"
>>>   #include "hw/watchdog/wdt_aspeed.h"
>>>   #include "hw/net/ftgmac100.h"
>>> +#include "hw/misc/aspeed_xdma.h"
>>>     #define ASPEED_SPIS_NUM  2
>>>   #define ASPEED_WDTS_NUM  3
>>> @@ -40,6 +41,7 @@ typedef struct AspeedSoCState {
>>>       AspeedSDMCState sdmc;
>>>       AspeedWDTState wdt[ASPEED_WDTS_NUM];
>>>       FTGMAC100State ftgmac100;
>>> +    AspeedXDMAState xdma;
>>>   } AspeedSoCState;
>>>     #define TYPE_ASPEED_SOC "aspeed-soc"
>>> diff --git a/include/hw/misc/aspeed_xdma.h b/include/hw/misc/aspeed_xdma.h
>>> new file mode 100644
>>> index 0000000..d19e9a7
>>> --- /dev/null
>>> +++ b/include/hw/misc/aspeed_xdma.h
>>> @@ -0,0 +1,31 @@
>>> +/*
>>> + * ASPEED XDMA Controller
>>> + * Eddie James <eajames@linux.ibm.com>
>>> + *
>>> + * Copyright (C) 2019 IBM Corp.
>>> + * SPDX-License-Identifer: GPL-2.0-or-later
>>> + */
>>> +
>>> +#ifndef ASPEED_XDMA_H
>>> +#define ASPEED_XDMA_H
>>> +
>>> +#include "hw/sysbus.h"
>>> +
>>> +#define TYPE_ASPEED_XDMA "aspeed.xdma"
>>> +#define ASPEED_XDMA(obj) OBJECT_CHECK(AspeedXDMAState, (obj), TYPE_ASPEED_XDMA)
>>> +
>>> +#define ASPEED_XDMA_MEM_SIZE 0x1000
>>> +#define ASPEED_XDMA_NUM_REGS (ASPEED_XDMA_REG_SIZE / sizeof(uint32_t))
>>> +#define ASPEED_XDMA_REG_SIZE 0x7C
>>> +
>>> +typedef struct AspeedXDMAState {
>>> +    SysBusDevice parent;
>>> +
>>> +    MemoryRegion iomem;
>>> +    qemu_irq irq;
>>> +
>>> +    char bmc_cmdq_readp_set;
>>> +    uint32_t regs[ASPEED_XDMA_NUM_REGS];
>>> +} AspeedXDMAState;
>>> +
>>> +#endif /* ASPEED_XDMA_H */
>>>
> 


Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
Posted by Philippe Mathieu-Daudé 4 years, 10 months ago
Hi Eddie,

On 6/4/19 12:09 AM, Eddie James wrote:
> The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
> between the SOC (acting as a BMC) and a host processor in a server.

If I got your model correctly, it does no DMA operation but simply
answer correctly to the BMC, and set 'operation done' IRQ with no delay.
So this is a dummy device. Then it would be more useful having ignored
DMA ops traced with trace-events.

> 
> The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
> enable it for all of those.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  hw/arm/aspeed_soc.c           |  14 ++++
>  hw/misc/Makefile.objs         |   2 +-
>  hw/misc/aspeed_xdma.c         | 156 ++++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/aspeed_soc.h   |   2 +
>  include/hw/misc/aspeed_xdma.h |  31 +++++++++
>  5 files changed, 204 insertions(+), 1 deletion(-)
>  create mode 100644 hw/misc/aspeed_xdma.c
>  create mode 100644 include/hw/misc/aspeed_xdma.h
> 
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index faff42b..b25bb18 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -31,6 +31,7 @@
>  #define ASPEED_SOC_VIC_BASE         0x1E6C0000
>  #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
>  #define ASPEED_SOC_SCU_BASE         0x1E6E2000
> +#define ASPEED_SOC_XDMA_BASE        0x1E6E7000
>  #define ASPEED_SOC_SRAM_BASE        0x1E720000
>  #define ASPEED_SOC_TIMER_BASE       0x1E782000
>  #define ASPEED_SOC_WDT_BASE         0x1E785000
> @@ -159,6 +160,9 @@ static void aspeed_soc_init(Object *obj)
>  
>      sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
>                            sizeof(s->ftgmac100), TYPE_FTGMAC100);
> +
> +    sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
> +                          TYPE_ASPEED_XDMA);
>  }
>  
>  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -298,6 +302,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>      sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
>                         qdev_get_gpio_in(DEVICE(&s->vic), 2));
> +
> +    /* XDMA */
> +    object_property_set_bool(OBJECT(&s->xdma), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0, ASPEED_SOC_XDMA_BASE);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->vic), 6));
>  }
>  
>  static void aspeed_soc_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index 77b9df9..a4483af 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -74,7 +74,7 @@ obj-$(CONFIG_ARMSSE_MHU) += armsse-mhu.o
>  
>  obj-$(CONFIG_PVPANIC) += pvpanic.o
>  obj-$(CONFIG_AUX) += auxbus.o
> -obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
> +obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o aspeed_xdma.o
>  obj-$(CONFIG_MSF2) += msf2-sysreg.o
>  obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>  
> diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c
> new file mode 100644
> index 0000000..fe3a32e
> --- /dev/null
> +++ b/hw/misc/aspeed_xdma.c
> @@ -0,0 +1,156 @@
> +/*
> + * ASPEED XDMA Controller
> + * Eddie James <eajames@linux.ibm.com>
> + *
> + * Copyright (C) 2019 IBM Corp
> + * SPDX-License-Identifer: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/error-report.h"
> +#include "hw/misc/aspeed_xdma.h"
> +#include "qapi/error.h"
> +
> +#define XDMA_BMC_CMDQ_ADDR         0x10
> +#define XDMA_BMC_CMDQ_ENDP         0x14
> +#define XDMA_BMC_CMDQ_WRP          0x18
> +#define  XDMA_BMC_CMDQ_W_MASK      0x0003FFFF
> +#define XDMA_BMC_CMDQ_RDP          0x1C
> +#define  XDMA_BMC_CMDQ_RDP_MAGIC   0xEE882266
> +#define XDMA_IRQ_ENG_CTRL          0x20
> +#define  XDMA_IRQ_ENG_CTRL_US_COMP BIT(4)
> +#define  XDMA_IRQ_ENG_CTRL_DS_COMP BIT(5)
> +#define  XDMA_IRQ_ENG_CTRL_W_MASK  0xBFEFF07F
> +#define XDMA_IRQ_ENG_STAT          0x24
> +#define  XDMA_IRQ_ENG_STAT_US_COMP BIT(4)
> +#define  XDMA_IRQ_ENG_STAT_DS_COMP BIT(5)
> +#define  XDMA_IRQ_ENG_STAT_RESET   0xF8000000
> +
> +#define TO_REG(addr) ((addr) / sizeof(uint32_t))
> +
> +static uint64_t aspeed_xdma_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    uint32_t val = 0;
> +    AspeedXDMAState *xdma = opaque;
> +
> +    if (addr < ASPEED_XDMA_REG_SIZE) {
> +        val = xdma->regs[TO_REG(addr)];
> +    }
> +
> +    return (uint64_t)val;
> +}
> +
> +static void aspeed_xdma_write(void *opaque, hwaddr addr, uint64_t val,
> +                              unsigned int size)
> +{
> +    unsigned int idx;
> +    uint32_t val32 = (uint32_t)val;
> +    AspeedXDMAState *xdma = opaque;
> +
> +    if (addr >= ASPEED_XDMA_REG_SIZE) {
> +        return;
> +    }
> +
> +    switch (addr) {
> +    case XDMA_BMC_CMDQ_ENDP:
> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_BMC_CMDQ_W_MASK;
> +        break;
> +    case XDMA_BMC_CMDQ_WRP:
> +        idx = TO_REG(addr);
> +        xdma->regs[idx] = val32 & XDMA_BMC_CMDQ_W_MASK;
> +        xdma->regs[TO_REG(XDMA_BMC_CMDQ_RDP)] = xdma->regs[idx];

The two previous lines are odd. Are they inverted?

I guess I'd trace here:

           trace_aspeed_xdma_...(val, ...);

> +
> +        if (xdma->bmc_cmdq_readp_set) {
> +            xdma->bmc_cmdq_readp_set = 0;
> +        } else {
> +            xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] |=
> +                XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP;
> +
> +            if (xdma->regs[TO_REG(XDMA_IRQ_ENG_CTRL)] &
> +                (XDMA_IRQ_ENG_CTRL_US_COMP | XDMA_IRQ_ENG_CTRL_DS_COMP))
> +                qemu_irq_raise(xdma->irq);
> +        }
> +        break;
> +    case XDMA_BMC_CMDQ_RDP:

Trace here too.

> +        if (val32 == XDMA_BMC_CMDQ_RDP_MAGIC) {
> +            xdma->bmc_cmdq_readp_set = 1;
> +        }
> +        break;
> +    case XDMA_IRQ_ENG_CTRL:
> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_IRQ_ENG_CTRL_W_MASK;
> +        break;
> +    case XDMA_IRQ_ENG_STAT:
> +        idx = TO_REG(addr);
> +        if (val32 & (XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP)) {
> +            xdma->regs[TO_REG(addr)] &=

                          ^ idx

> +                ~(XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP);
> +            qemu_irq_lower(xdma->irq);
> +        }
> +        break;
> +    default:
> +        xdma->regs[TO_REG(addr)] = val32;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps aspeed_xdma_ops = {
> +    .read = aspeed_xdma_read,
> +    .write = aspeed_xdma_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid.min_access_size = 4,
> +    .valid.max_access_size = 4,
> +};
> +
> +static void aspeed_xdma_realize(DeviceState *dev, Error **errp)
> +{
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
> +
> +    sysbus_init_irq(sbd, &xdma->irq);
> +    memory_region_init_io(&xdma->iomem, OBJECT(xdma), &aspeed_xdma_ops, xdma,
> +                          TYPE_ASPEED_XDMA, ASPEED_XDMA_MEM_SIZE);
> +    sysbus_init_mmio(sbd, &xdma->iomem);
> +}
> +
> +static void aspeed_xdma_reset(DeviceState *dev)
> +{
> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
> +
> +    xdma->bmc_cmdq_readp_set = 0;
> +    memset(xdma->regs, 0, ASPEED_XDMA_REG_SIZE);
> +    xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] = XDMA_IRQ_ENG_STAT_RESET;
> +
> +    qemu_irq_lower(xdma->irq);
> +}
> +
> +static const VMStateDescription aspeed_xdma_vmstate = {
> +    .name = TYPE_ASPEED_XDMA,
> +    .version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32_ARRAY(regs, AspeedXDMAState, ASPEED_XDMA_NUM_REGS),
> +        VMSTATE_END_OF_LIST(),
> +    },
> +};
> +
> +static void aspeed_xdma_class_init(ObjectClass *classp, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(classp);
> +
> +    dc->realize = aspeed_xdma_realize;
> +    dc->reset = aspeed_xdma_reset;
> +    dc->vmsd = &aspeed_xdma_vmstate;
> +}
> +
> +static const TypeInfo aspeed_xdma_info = {
> +    .name          = TYPE_ASPEED_XDMA,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(AspeedXDMAState),
> +    .class_init    = aspeed_xdma_class_init,
> +};
> +
> +static void aspeed_xdma_register_type(void)
> +{
> +    type_register_static(&aspeed_xdma_info);
> +}
> +type_init(aspeed_xdma_register_type);
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 836b2ba..0329247 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -20,6 +20,7 @@
>  #include "hw/ssi/aspeed_smc.h"
>  #include "hw/watchdog/wdt_aspeed.h"
>  #include "hw/net/ftgmac100.h"
> +#include "hw/misc/aspeed_xdma.h"
>  
>  #define ASPEED_SPIS_NUM  2
>  #define ASPEED_WDTS_NUM  3
> @@ -40,6 +41,7 @@ typedef struct AspeedSoCState {
>      AspeedSDMCState sdmc;
>      AspeedWDTState wdt[ASPEED_WDTS_NUM];
>      FTGMAC100State ftgmac100;
> +    AspeedXDMAState xdma;
>  } AspeedSoCState;
>  
>  #define TYPE_ASPEED_SOC "aspeed-soc"
> diff --git a/include/hw/misc/aspeed_xdma.h b/include/hw/misc/aspeed_xdma.h
> new file mode 100644
> index 0000000..d19e9a7
> --- /dev/null
> +++ b/include/hw/misc/aspeed_xdma.h
> @@ -0,0 +1,31 @@
> +/*
> + * ASPEED XDMA Controller
> + * Eddie James <eajames@linux.ibm.com>
> + *
> + * Copyright (C) 2019 IBM Corp.
> + * SPDX-License-Identifer: GPL-2.0-or-later
> + */
> +
> +#ifndef ASPEED_XDMA_H
> +#define ASPEED_XDMA_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_ASPEED_XDMA "aspeed.xdma"
> +#define ASPEED_XDMA(obj) OBJECT_CHECK(AspeedXDMAState, (obj), TYPE_ASPEED_XDMA)
> +
> +#define ASPEED_XDMA_MEM_SIZE 0x1000

Maybe you can keep ASPEED_XDMA_MEM_SIZE private in the source file.

> +#define ASPEED_XDMA_NUM_REGS (ASPEED_XDMA_REG_SIZE / sizeof(uint32_t))
> +#define ASPEED_XDMA_REG_SIZE 0x7C

0x80?

> +
> +typedef struct AspeedXDMAState {
> +    SysBusDevice parent;
> +
> +    MemoryRegion iomem;
> +    qemu_irq irq;
> +
> +    char bmc_cmdq_readp_set;
> +    uint32_t regs[ASPEED_XDMA_NUM_REGS];
> +} AspeedXDMAState;
> +
> +#endif /* ASPEED_XDMA_H */
> 

Few questions, but otherwise LGTM.

Regards,

Phil.

Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
Posted by Eddie James 4 years, 10 months ago
On 6/6/19 6:34 PM, Philippe Mathieu-Daudé wrote:
> Hi Eddie,
>
> On 6/4/19 12:09 AM, Eddie James wrote:
>> The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
>> between the SOC (acting as a BMC) and a host processor in a server.
> If I got your model correctly, it does no DMA operation but simply
> answer correctly to the BMC, and set 'operation done' IRQ with no delay.
> So this is a dummy device. Then it would be more useful having ignored
> DMA ops traced with trace-events.


Thats correct. Good idea, I will add some tracing.


>
>> The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
>> enable it for all of those.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   hw/arm/aspeed_soc.c           |  14 ++++
>>   hw/misc/Makefile.objs         |   2 +-
>>   hw/misc/aspeed_xdma.c         | 156 ++++++++++++++++++++++++++++++++++++++++++
>>   include/hw/arm/aspeed_soc.h   |   2 +
>>   include/hw/misc/aspeed_xdma.h |  31 +++++++++
>>   5 files changed, 204 insertions(+), 1 deletion(-)
>>   create mode 100644 hw/misc/aspeed_xdma.c
>>   create mode 100644 include/hw/misc/aspeed_xdma.h
>>
>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>> index faff42b..b25bb18 100644
>> --- a/hw/arm/aspeed_soc.c
>> +++ b/hw/arm/aspeed_soc.c
>> @@ -31,6 +31,7 @@
>>   #define ASPEED_SOC_VIC_BASE         0x1E6C0000
>>   #define ASPEED_SOC_SDMC_BASE        0x1E6E0000
>>   #define ASPEED_SOC_SCU_BASE         0x1E6E2000
>> +#define ASPEED_SOC_XDMA_BASE        0x1E6E7000
>>   #define ASPEED_SOC_SRAM_BASE        0x1E720000
>>   #define ASPEED_SOC_TIMER_BASE       0x1E782000
>>   #define ASPEED_SOC_WDT_BASE         0x1E785000
>> @@ -159,6 +160,9 @@ static void aspeed_soc_init(Object *obj)
>>   
>>       sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
>>                             sizeof(s->ftgmac100), TYPE_FTGMAC100);
>> +
>> +    sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
>> +                          TYPE_ASPEED_XDMA);
>>   }
>>   
>>   static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> @@ -298,6 +302,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>>       sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
>>       sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
>>                          qdev_get_gpio_in(DEVICE(&s->vic), 2));
>> +
>> +    /* XDMA */
>> +    object_property_set_bool(OBJECT(&s->xdma), true, "realized", &err);
>> +    if (err) {
>> +        error_propagate(errp, err);
>> +        return;
>> +    }
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0, ASPEED_SOC_XDMA_BASE);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
>> +                       qdev_get_gpio_in(DEVICE(&s->vic), 6));
>>   }
>>   
>>   static void aspeed_soc_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
>> index 77b9df9..a4483af 100644
>> --- a/hw/misc/Makefile.objs
>> +++ b/hw/misc/Makefile.objs
>> @@ -74,7 +74,7 @@ obj-$(CONFIG_ARMSSE_MHU) += armsse-mhu.o
>>   
>>   obj-$(CONFIG_PVPANIC) += pvpanic.o
>>   obj-$(CONFIG_AUX) += auxbus.o
>> -obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
>> +obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o aspeed_xdma.o
>>   obj-$(CONFIG_MSF2) += msf2-sysreg.o
>>   obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>>   
>> diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c
>> new file mode 100644
>> index 0000000..fe3a32e
>> --- /dev/null
>> +++ b/hw/misc/aspeed_xdma.c
>> @@ -0,0 +1,156 @@
>> +/*
>> + * ASPEED XDMA Controller
>> + * Eddie James <eajames@linux.ibm.com>
>> + *
>> + * Copyright (C) 2019 IBM Corp
>> + * SPDX-License-Identifer: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/log.h"
>> +#include "qemu/error-report.h"
>> +#include "hw/misc/aspeed_xdma.h"
>> +#include "qapi/error.h"
>> +
>> +#define XDMA_BMC_CMDQ_ADDR         0x10
>> +#define XDMA_BMC_CMDQ_ENDP         0x14
>> +#define XDMA_BMC_CMDQ_WRP          0x18
>> +#define  XDMA_BMC_CMDQ_W_MASK      0x0003FFFF
>> +#define XDMA_BMC_CMDQ_RDP          0x1C
>> +#define  XDMA_BMC_CMDQ_RDP_MAGIC   0xEE882266
>> +#define XDMA_IRQ_ENG_CTRL          0x20
>> +#define  XDMA_IRQ_ENG_CTRL_US_COMP BIT(4)
>> +#define  XDMA_IRQ_ENG_CTRL_DS_COMP BIT(5)
>> +#define  XDMA_IRQ_ENG_CTRL_W_MASK  0xBFEFF07F
>> +#define XDMA_IRQ_ENG_STAT          0x24
>> +#define  XDMA_IRQ_ENG_STAT_US_COMP BIT(4)
>> +#define  XDMA_IRQ_ENG_STAT_DS_COMP BIT(5)
>> +#define  XDMA_IRQ_ENG_STAT_RESET   0xF8000000
>> +
>> +#define TO_REG(addr) ((addr) / sizeof(uint32_t))
>> +
>> +static uint64_t aspeed_xdma_read(void *opaque, hwaddr addr, unsigned int size)
>> +{
>> +    uint32_t val = 0;
>> +    AspeedXDMAState *xdma = opaque;
>> +
>> +    if (addr < ASPEED_XDMA_REG_SIZE) {
>> +        val = xdma->regs[TO_REG(addr)];
>> +    }
>> +
>> +    return (uint64_t)val;
>> +}
>> +
>> +static void aspeed_xdma_write(void *opaque, hwaddr addr, uint64_t val,
>> +                              unsigned int size)
>> +{
>> +    unsigned int idx;
>> +    uint32_t val32 = (uint32_t)val;
>> +    AspeedXDMAState *xdma = opaque;
>> +
>> +    if (addr >= ASPEED_XDMA_REG_SIZE) {
>> +        return;
>> +    }
>> +
>> +    switch (addr) {
>> +    case XDMA_BMC_CMDQ_ENDP:
>> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_BMC_CMDQ_W_MASK;
>> +        break;
>> +    case XDMA_BMC_CMDQ_WRP:
>> +        idx = TO_REG(addr);
>> +        xdma->regs[idx] = val32 & XDMA_BMC_CMDQ_W_MASK;
>> +        xdma->regs[TO_REG(XDMA_BMC_CMDQ_RDP)] = xdma->regs[idx];
> The two previous lines are odd. Are they inverted?


No, this should be correct. Since the command completes instantly, the 
engine "read pointer" is updated to the new "write pointer," with the 
expected result that no more commands need to be completed, since write 
== read.


>
> I guess I'd trace here:
>
>             trace_aspeed_xdma_...(val, ...);
>
>> +
>> +        if (xdma->bmc_cmdq_readp_set) {
>> +            xdma->bmc_cmdq_readp_set = 0;
>> +        } else {
>> +            xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] |=
>> +                XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP;
>> +
>> +            if (xdma->regs[TO_REG(XDMA_IRQ_ENG_CTRL)] &
>> +                (XDMA_IRQ_ENG_CTRL_US_COMP | XDMA_IRQ_ENG_CTRL_DS_COMP))
>> +                qemu_irq_raise(xdma->irq);
>> +        }
>> +        break;
>> +    case XDMA_BMC_CMDQ_RDP:
> Trace here too.
>
>> +        if (val32 == XDMA_BMC_CMDQ_RDP_MAGIC) {
>> +            xdma->bmc_cmdq_readp_set = 1;
>> +        }
>> +        break;
>> +    case XDMA_IRQ_ENG_CTRL:
>> +        xdma->regs[TO_REG(addr)] = val32 & XDMA_IRQ_ENG_CTRL_W_MASK;
>> +        break;
>> +    case XDMA_IRQ_ENG_STAT:
>> +        idx = TO_REG(addr);
>> +        if (val32 & (XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP)) {
>> +            xdma->regs[TO_REG(addr)] &=
>                            ^ idx


Thanks, will update.


>
>> +                ~(XDMA_IRQ_ENG_STAT_US_COMP | XDMA_IRQ_ENG_STAT_DS_COMP);
>> +            qemu_irq_lower(xdma->irq);
>> +        }
>> +        break;
>> +    default:
>> +        xdma->regs[TO_REG(addr)] = val32;
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps aspeed_xdma_ops = {
>> +    .read = aspeed_xdma_read,
>> +    .write = aspeed_xdma_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .valid.min_access_size = 4,
>> +    .valid.max_access_size = 4,
>> +};
>> +
>> +static void aspeed_xdma_realize(DeviceState *dev, Error **errp)
>> +{
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
>> +
>> +    sysbus_init_irq(sbd, &xdma->irq);
>> +    memory_region_init_io(&xdma->iomem, OBJECT(xdma), &aspeed_xdma_ops, xdma,
>> +                          TYPE_ASPEED_XDMA, ASPEED_XDMA_MEM_SIZE);
>> +    sysbus_init_mmio(sbd, &xdma->iomem);
>> +}
>> +
>> +static void aspeed_xdma_reset(DeviceState *dev)
>> +{
>> +    AspeedXDMAState *xdma = ASPEED_XDMA(dev);
>> +
>> +    xdma->bmc_cmdq_readp_set = 0;
>> +    memset(xdma->regs, 0, ASPEED_XDMA_REG_SIZE);
>> +    xdma->regs[TO_REG(XDMA_IRQ_ENG_STAT)] = XDMA_IRQ_ENG_STAT_RESET;
>> +
>> +    qemu_irq_lower(xdma->irq);
>> +}
>> +
>> +static const VMStateDescription aspeed_xdma_vmstate = {
>> +    .name = TYPE_ASPEED_XDMA,
>> +    .version_id = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32_ARRAY(regs, AspeedXDMAState, ASPEED_XDMA_NUM_REGS),
>> +        VMSTATE_END_OF_LIST(),
>> +    },
>> +};
>> +
>> +static void aspeed_xdma_class_init(ObjectClass *classp, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(classp);
>> +
>> +    dc->realize = aspeed_xdma_realize;
>> +    dc->reset = aspeed_xdma_reset;
>> +    dc->vmsd = &aspeed_xdma_vmstate;
>> +}
>> +
>> +static const TypeInfo aspeed_xdma_info = {
>> +    .name          = TYPE_ASPEED_XDMA,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(AspeedXDMAState),
>> +    .class_init    = aspeed_xdma_class_init,
>> +};
>> +
>> +static void aspeed_xdma_register_type(void)
>> +{
>> +    type_register_static(&aspeed_xdma_info);
>> +}
>> +type_init(aspeed_xdma_register_type);
>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>> index 836b2ba..0329247 100644
>> --- a/include/hw/arm/aspeed_soc.h
>> +++ b/include/hw/arm/aspeed_soc.h
>> @@ -20,6 +20,7 @@
>>   #include "hw/ssi/aspeed_smc.h"
>>   #include "hw/watchdog/wdt_aspeed.h"
>>   #include "hw/net/ftgmac100.h"
>> +#include "hw/misc/aspeed_xdma.h"
>>   
>>   #define ASPEED_SPIS_NUM  2
>>   #define ASPEED_WDTS_NUM  3
>> @@ -40,6 +41,7 @@ typedef struct AspeedSoCState {
>>       AspeedSDMCState sdmc;
>>       AspeedWDTState wdt[ASPEED_WDTS_NUM];
>>       FTGMAC100State ftgmac100;
>> +    AspeedXDMAState xdma;
>>   } AspeedSoCState;
>>   
>>   #define TYPE_ASPEED_SOC "aspeed-soc"
>> diff --git a/include/hw/misc/aspeed_xdma.h b/include/hw/misc/aspeed_xdma.h
>> new file mode 100644
>> index 0000000..d19e9a7
>> --- /dev/null
>> +++ b/include/hw/misc/aspeed_xdma.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * ASPEED XDMA Controller
>> + * Eddie James <eajames@linux.ibm.com>
>> + *
>> + * Copyright (C) 2019 IBM Corp.
>> + * SPDX-License-Identifer: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef ASPEED_XDMA_H
>> +#define ASPEED_XDMA_H
>> +
>> +#include "hw/sysbus.h"
>> +
>> +#define TYPE_ASPEED_XDMA "aspeed.xdma"
>> +#define ASPEED_XDMA(obj) OBJECT_CHECK(AspeedXDMAState, (obj), TYPE_ASPEED_XDMA)
>> +
>> +#define ASPEED_XDMA_MEM_SIZE 0x1000
> Maybe you can keep ASPEED_XDMA_MEM_SIZE private in the source file.


Sure.


>
>> +#define ASPEED_XDMA_NUM_REGS (ASPEED_XDMA_REG_SIZE / sizeof(uint32_t))
>> +#define ASPEED_XDMA_REG_SIZE 0x7C
> 0x80?


Well 0x78 is the offset of the last register, so 0x7c would be the total 
register space. Of course the engine actually owns 4K bytes of register 
space but there's no reason to store all that.


>
>> +
>> +typedef struct AspeedXDMAState {
>> +    SysBusDevice parent;
>> +
>> +    MemoryRegion iomem;
>> +    qemu_irq irq;
>> +
>> +    char bmc_cmdq_readp_set;
>> +    uint32_t regs[ASPEED_XDMA_NUM_REGS];
>> +} AspeedXDMAState;
>> +
>> +#endif /* ASPEED_XDMA_H */
>>
> Few questions, but otherwise LGTM.
>
> Regards,
>
> Phil.
>