qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
@ 2019-06-03 22:09 Eddie James
  2019-06-06  6:16 ` Cédric Le Goater
  2019-06-06 23:34 ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 6+ messages in thread
From: Eddie James @ 2019-06-03 22:09 UTC (permalink / raw)
  To: qemu-arm; +Cc: peter.maydell, andrew, Eddie James, qemu-devel, clg, joel

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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
  2019-06-03 22:09 [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device Eddie James
@ 2019-06-06  6:16 ` Cédric Le Goater
  2019-06-06 21:42   ` Eddie James
  2019-06-06 23:34 ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 6+ messages in thread
From: Cédric Le Goater @ 2019-06-06  6:16 UTC (permalink / raw)
  To: Eddie James, qemu-arm; +Cc: andrew, peter.maydell, qemu-devel, joel

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 */
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
  2019-06-06  6:16 ` Cédric Le Goater
@ 2019-06-06 21:42   ` Eddie James
  2019-06-11  6:53     ` Cédric Le Goater
  0 siblings, 1 reply; 6+ messages in thread
From: Eddie James @ 2019-06-06 21:42 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-arm; +Cc: andrew, peter.maydell, qemu-devel, joel


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 */
>>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
  2019-06-03 22:09 [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device Eddie James
  2019-06-06  6:16 ` Cédric Le Goater
@ 2019-06-06 23:34 ` Philippe Mathieu-Daudé
  2019-06-10 18:00   ` Eddie James
  1 sibling, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-06-06 23:34 UTC (permalink / raw)
  To: Eddie James, qemu-arm; +Cc: andrew, peter.maydell, joel, qemu-devel, clg

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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
  2019-06-06 23:34 ` Philippe Mathieu-Daudé
@ 2019-06-10 18:00   ` Eddie James
  0 siblings, 0 replies; 6+ messages in thread
From: Eddie James @ 2019-06-10 18:00 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Eddie James, qemu-arm
  Cc: andrew, peter.maydell, joel, qemu-devel, clg


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.
>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device
  2019-06-06 21:42   ` Eddie James
@ 2019-06-11  6:53     ` Cédric Le Goater
  0 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2019-06-11  6:53 UTC (permalink / raw)
  To: Eddie James, qemu-arm; +Cc: andrew, peter.maydell, qemu-devel, joel

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 */
>>>
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-06-11  6:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-03 22:09 [Qemu-devel] [PATCH] hw: misc: Add Aspeed XDMA device Eddie James
2019-06-06  6:16 ` Cédric Le Goater
2019-06-06 21:42   ` Eddie James
2019-06-11  6:53     ` Cédric Le Goater
2019-06-06 23:34 ` Philippe Mathieu-Daudé
2019-06-10 18:00   ` Eddie James

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).