[Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes

Emanuele Giuseppe Esposito posted 34 patches 7 years, 3 months ago
[Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes
Posted by Emanuele Giuseppe Esposito 7 years, 3 months ago
Add qgraph nodes for sdhci-pci and generic-sdhci (memory mapped) drivers.
Both drivers implement (produce) the same interface sdhci, that provides the
readw - readq - writeq functions.

Signed-off-by: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
---
 tests/Makefile.include |   1 +
 tests/libqos/sdhci.c   | 163 +++++++++++++++++++++++++++++++++++++++++
 tests/libqos/sdhci.h   |  69 +++++++++++++++++
 3 files changed, 233 insertions(+)
 create mode 100644 tests/libqos/sdhci.c
 create mode 100644 tests/libqos/sdhci.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 4e7b4bb614..be00fb71ec 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -775,6 +775,7 @@ libqgraph-machines-obj-y = tests/libqos/x86_64_pc-machine.o
 
 libqgraph-pci-obj-y = $(libqos-pc-obj-y)
 libqgraph-pci-obj-y += $(libqgraph-machines-obj-y)
+libqgraph-pci-obj-y += tests/libqos/sdhci.o
 
 check-unit-y += tests/test-qgraph$(EXESUF)
 tests/test-qgraph$(EXESUF): tests/test-qgraph.o $(libqgraph-obj-y)
diff --git a/tests/libqos/sdhci.c b/tests/libqos/sdhci.c
new file mode 100644
index 0000000000..f5eb2c5459
--- /dev/null
+++ b/tests/libqos/sdhci.c
@@ -0,0 +1,163 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "libqos/qgraph.h"
+#include "pci.h"
+#include "sdhci.h"
+#include "hw/pci/pci.h"
+
+static void set_qsdhci_fields(QSDHCI *s, uint8_t version, uint8_t baseclock,
+                              bool sdma, uint64_t reg)
+{
+    s->props.version = version;
+    s->props.baseclock = baseclock;
+    s->props.capab.sdma = sdma;
+    s->props.capab.reg = reg;
+}
+
+/* Memory mapped implementation of QSDHCI */
+
+static uint16_t sdhci_mm_readw(QSDHCI *s, uint32_t reg)
+{
+    QSDHCI_MemoryMapped *smm = container_of(s, QSDHCI_MemoryMapped, sdhci);
+    return qtest_readw(global_qtest, smm->addr + reg);
+}
+
+static uint64_t sdhci_mm_readq(QSDHCI *s, uint32_t reg)
+{
+    QSDHCI_MemoryMapped *smm = container_of(s, QSDHCI_MemoryMapped, sdhci);
+    return qtest_readq(global_qtest, smm->addr + reg);
+}
+
+static void sdhci_mm_writeq(QSDHCI *s, uint32_t reg, uint64_t val)
+{
+    QSDHCI_MemoryMapped *smm = container_of(s, QSDHCI_MemoryMapped, sdhci);
+    qtest_writeq(global_qtest, smm->addr + reg, val);
+}
+
+static void *sdhci_mm_get_driver(void *obj, const char *interface)
+{
+    QSDHCI_MemoryMapped *spci = obj;
+    if (!g_strcmp0(interface, "sdhci")) {
+        return &spci->sdhci;
+    }
+    printf("%s not present in generic-sdhci\n", interface);
+    abort();
+}
+
+void qos_init_sdhci_mm(QSDHCI_MemoryMapped *sdhci, uint32_t addr,
+                       QSDHCIProperties *common)
+{
+    sdhci->obj.get_driver = sdhci_mm_get_driver;
+    sdhci->sdhci.readw = sdhci_mm_readw;
+    sdhci->sdhci.readq = sdhci_mm_readq;
+    sdhci->sdhci.writeq = sdhci_mm_writeq;
+    memcpy(&sdhci->sdhci.props, common, sizeof(QSDHCIProperties));
+    sdhci->addr = addr;
+}
+
+/* PCI implementation of QSDHCI */
+
+static uint16_t sdhci_pci_readw(QSDHCI *s, uint32_t reg)
+{
+    QSDHCI_PCI *spci = container_of(s, QSDHCI_PCI, sdhci);
+    return qpci_io_readw(&spci->dev, spci->mem_bar, reg);
+}
+
+static uint64_t sdhci_readq(QSDHCI *s, uint32_t reg)
+{
+    QSDHCI_PCI *spci = container_of(s, QSDHCI_PCI, sdhci);
+    return qpci_io_readq(&spci->dev, spci->mem_bar, reg);
+}
+
+static void sdhci_writeq(QSDHCI *s, uint32_t reg, uint64_t val)
+{
+    QSDHCI_PCI *spci = container_of(s, QSDHCI_PCI, sdhci);
+    return qpci_io_writeq(&spci->dev, spci->mem_bar, reg, val);
+}
+
+static void *sdhci_pci_get_driver(void *object, const char *interface)
+{
+    QSDHCI_PCI *spci = object;
+    if (!g_strcmp0(interface, "sdhci")) {
+        return &spci->sdhci;
+    }
+
+    printf("%s not present in sdhci-pci\n", interface);
+    abort();
+}
+
+static void sdhci_start_hw(QOSGraphObject *obj)
+{
+    QSDHCI_PCI *spci = (QSDHCI_PCI *)obj;
+    qpci_device_enable(&spci->dev);
+}
+
+static void sdhci_destroy(QOSGraphObject *obj)
+{
+    QSDHCI_PCI *spci = (QSDHCI_PCI *)obj;
+    qpci_iounmap(&spci->dev, spci->mem_bar);
+    g_free(spci);
+}
+
+static void *sdhci_pci_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
+{
+    QSDHCI_PCI *spci = g_new0(QSDHCI_PCI, 1);
+    QPCIBus *bus = pci_bus;
+    uint64_t barsize;
+
+    qpci_device_init(&spci->dev, bus, addr);
+    spci->mem_bar = qpci_iomap(&spci->dev, 0, &barsize);
+    spci->sdhci.readw = sdhci_pci_readw;
+    spci->sdhci.readq = sdhci_readq;
+    spci->sdhci.writeq = sdhci_writeq;
+    set_qsdhci_fields(&spci->sdhci, 2, 0, 1, 0x057834b4);
+
+    spci->obj.get_driver = sdhci_pci_get_driver;
+    spci->obj.start_hw = sdhci_start_hw;
+    spci->obj.destructor = sdhci_destroy;
+    return &spci->obj;
+}
+
+static void qsdhci(void)
+{
+    QPCIAddress addr = {
+        .devfn = QPCI_DEVFN(4, 0),
+        .vendor_id = PCI_VENDOR_ID_REDHAT,
+        .device_id = PCI_DEVICE_ID_REDHAT_SDHCI,
+    };
+
+    QOSGraphEdgeOptions opts = {
+        .extra_device_opts = "addr=04.0",
+    };
+
+    /* generic-sdhci */
+    qos_node_create_driver("generic-sdhci", NULL);
+    qos_node_produces("generic-sdhci", "sdhci");
+
+    /* sdhci-pci */
+    add_qpci_address(&opts, &addr);
+    qos_node_create_driver("sdhci-pci", sdhci_pci_create);
+    qos_node_produces("sdhci-pci", "sdhci");
+    qos_node_consumes("sdhci-pci", "pci-bus", &opts);
+
+}
+
+libqos_init(qsdhci);
diff --git a/tests/libqos/sdhci.h b/tests/libqos/sdhci.h
new file mode 100644
index 0000000000..57e8134dee
--- /dev/null
+++ b/tests/libqos/sdhci.h
@@ -0,0 +1,69 @@
+/*
+ * libqos driver framework
+ *
+ * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#ifndef QGRAPH_QSDHCI
+#define QGRAPH_QSDHCI
+
+#include "libqos/qgraph.h"
+#include "pci.h"
+
+typedef struct QSDHCI QSDHCI;
+typedef struct QSDHCI_MemoryMapped QSDHCI_MemoryMapped;
+typedef struct QSDHCI_PCI  QSDHCI_PCI;
+typedef struct QSDHCIProperties QSDHCIProperties;
+
+/* Properties common to all QSDHCI devices */
+struct QSDHCIProperties {
+    uint8_t version;
+    uint8_t baseclock;
+    struct {
+        bool sdma;
+        uint64_t reg;
+    } capab;
+};
+
+struct QSDHCI {
+    uint16_t (*readw)(QSDHCI *s, uint32_t reg);
+    uint64_t (*readq)(QSDHCI *s, uint32_t reg);
+    void (*writeq)(QSDHCI *s, uint32_t reg, uint64_t val);
+    QSDHCIProperties props;
+};
+
+/* Memory Mapped implementation of QSDHCI */
+struct QSDHCI_MemoryMapped {
+    QOSGraphObject obj;
+    QSDHCI sdhci;
+    uint64_t addr;
+};
+
+/* PCI implementation of QSDHCI */
+struct QSDHCI_PCI {
+    QOSGraphObject obj;
+    QPCIDevice dev;
+    QSDHCI sdhci;
+    QPCIBar mem_bar;
+};
+
+/**
+ * qos_init_sdhci_mm(): external constructor used by all drivers/machines
+ * that "contain" a #QSDHCI_MemoryMapped driver
+ */
+void qos_init_sdhci_mm(QSDHCI_MemoryMapped *sdhci, uint32_t addr,
+                       QSDHCIProperties *common);
+
+#endif
-- 
2.17.1


Re: [Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes
Posted by Laurent Vivier 7 years, 2 months ago
On 06/08/2018 16:33, Emanuele Giuseppe Esposito wrote:
> Add qgraph nodes for sdhci-pci and generic-sdhci (memory mapped) drivers.
> Both drivers implement (produce) the same interface sdhci, that provides the
> readw - readq - writeq functions.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
> ---
>  tests/Makefile.include |   1 +
>  tests/libqos/sdhci.c   | 163 +++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/sdhci.h   |  69 +++++++++++++++++
>  3 files changed, 233 insertions(+)
>  create mode 100644 tests/libqos/sdhci.c
>  create mode 100644 tests/libqos/sdhci.h
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 4e7b4bb614..be00fb71ec 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -775,6 +775,7 @@ libqgraph-machines-obj-y = tests/libqos/x86_64_pc-machine.o
>  
>  libqgraph-pci-obj-y = $(libqos-pc-obj-y)
>  libqgraph-pci-obj-y += $(libqgraph-machines-obj-y)
> +libqgraph-pci-obj-y += tests/libqos/sdhci.o

The file also implements memory mapped SDHCI, should we split the file
in two files, one added to libqgraph-pci-obj-y and the other to
check-qtest-arm-y and check-qtest-aarch64-y?

>  check-unit-y += tests/test-qgraph$(EXESUF)
>  tests/test-qgraph$(EXESUF): tests/test-qgraph.o $(libqgraph-obj-y)
> diff --git a/tests/libqos/sdhci.c b/tests/libqos/sdhci.c
> new file mode 100644
> index 0000000000..f5eb2c5459
> --- /dev/null
> +++ b/tests/libqos/sdhci.c
> @@ -0,0 +1,163 @@
> +/*
> + * libqos driver framework
> + *
> + * Copyright (c) 2018 Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License version 2 as published by the Free Software Foundation.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "libqos/qgraph.h"
> +#include "pci.h"
> +#include "sdhci.h"
> +#include "hw/pci/pci.h"
> +
> +static void set_qsdhci_fields(QSDHCI *s, uint8_t version, uint8_t baseclock,
> +                              bool sdma, uint64_t reg)
> +{
> +    s->props.version = version;
> +    s->props.baseclock = baseclock;
> +    s->props.capab.sdma = sdma;
> +    s->props.capab.reg = reg;
> +}
> +
...
> +static void sdhci_mm_writeq(QSDHCI *s, uint32_t reg, uint64_t val)
> +{
> +    QSDHCI_MemoryMapped *smm = container_of(s, QSDHCI_MemoryMapped, sdhci);
> +    qtest_writeq(global_qtest, smm->addr + reg, val);
> +}
> +
> +static void *sdhci_mm_get_driver(void *obj, const char *interface)
> +{
> +    QSDHCI_MemoryMapped *spci = obj;

You should call it "smm" rather than "spci"

> +    if (!g_strcmp0(interface, "sdhci")) {
> +        return &spci->sdhci;
> +    }
> +    printf("%s not present in generic-sdhci\n", interface);

fprintf(stderr, ....

> +    abort();

g_assert_not_reached()

> +}
> +
> +void qos_init_sdhci_mm(QSDHCI_MemoryMapped *sdhci, uint32_t addr,

To be consistent with previous functions, you should it "smm" rather
than "sdhci".

...
> +/* PCI implementation of QSDHCI */
...
> +
> +static uint64_t sdhci_readq(QSDHCI *s, uint32_t reg)

"sdhci_pci_readq()"

> +{
> +    QSDHCI_PCI *spci = container_of(s, QSDHCI_PCI, sdhci);
> +    return qpci_io_readq(&spci->dev, spci->mem_bar, reg);
> +}
> +
> +static void sdhci_writeq(QSDHCI *s, uint32_t reg, uint64_t val)

"sdhci_pci_writeq()"

> +{
> +    QSDHCI_PCI *spci = container_of(s, QSDHCI_PCI, sdhci);
> +    return qpci_io_writeq(&spci->dev, spci->mem_bar, reg, val);
> +}
> +
> +static void *sdhci_pci_get_driver(void *object, const char *interface)
> +{
> +    QSDHCI_PCI *spci = object;
> +    if (!g_strcmp0(interface, "sdhci")) {
> +        return &spci->sdhci;
> +    }
> +
> +    printf("%s not present in sdhci-pci\n", interface);

fprintf(stderr, ...

> +    abort();

g_assert_not_reached()

> +}
> +
> +static void sdhci_start_hw(QOSGraphObject *obj)

perhaps you can call this "sdhci_pci_start_hw()"?

> +{
> +    QSDHCI_PCI *spci = (QSDHCI_PCI *)obj;
> +    qpci_device_enable(&spci->dev);
> +}
> +
> +static void sdhci_destroy(QOSGraphObject *obj)

"sdhci_pci_destructor()"?

> +{
> +    QSDHCI_PCI *spci = (QSDHCI_PCI *)obj;
> +    qpci_iounmap(&spci->dev, spci->mem_bar);
> +    g_free(spci);
> +}
> +
> +static void *sdhci_pci_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
> +{
> +    QSDHCI_PCI *spci = g_new0(QSDHCI_PCI, 1);
> +    QPCIBus *bus = pci_bus;
> +    uint64_t barsize;
> +
> +    qpci_device_init(&spci->dev, bus, addr);
> +    spci->mem_bar = qpci_iomap(&spci->dev, 0, &barsize);
> +    spci->sdhci.readw = sdhci_pci_readw;
> +    spci->sdhci.readq = sdhci_readq;

 ... = sdhci_pci_readq;

> +    spci->sdhci.writeq = sdhci_writeq;

 ... = sdhci_pci_writeq;

> +    set_qsdhci_fields(&spci->sdhci, 2, 0, 1, 0x057834b4);
> +
> +    spci->obj.get_driver = sdhci_pci_get_driver;
> +    spci->obj.start_hw = sdhci_start_hw;
> +    spci->obj.destructor = sdhci_destroy;
> +    return &spci->obj;
> +}
> +
> +static void qsdhci(void)
> +{
> +    QPCIAddress addr = {
> +        .devfn = QPCI_DEVFN(4, 0),
> +        .vendor_id = PCI_VENDOR_ID_REDHAT,
> +        .device_id = PCI_DEVICE_ID_REDHAT_SDHCI,
> +    };
> +
> +    QOSGraphEdgeOptions opts = {
> +        .extra_device_opts = "addr=04.0",
> +    };
> +
> +    /* generic-sdhci */
> +    qos_node_create_driver("generic-sdhci", NULL);
> +    qos_node_produces("generic-sdhci", "sdhci");
> +
> +    /* sdhci-pci */
> +    add_qpci_address(&opts, &addr);
> +    qos_node_create_driver("sdhci-pci", sdhci_pci_create);
> +    qos_node_produces("sdhci-pci", "sdhci");
> +    qos_node_consumes("sdhci-pci", "pci-bus", &opts);
> +
> +}
> +
> +libqos_init(qsdhci);

Perhaps we can have two functions qsdhci_mm_register_nodes() and
qsdhci_pci_register_nodes(), with two libqos_init() and split the file
in two files sdhci_pci.c and sdhci_mm.c?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes
Posted by Emanuele 7 years, 2 months ago
>> +{
>> +    QSDHCI_PCI *spci = (QSDHCI_PCI *)obj;
>> +    qpci_device_enable(&spci->dev);
>> +}
>> +
>> +static void sdhci_destroy(QOSGraphObject *obj)
> "sdhci_pci_destructor()"?
In general, I called the constructor name  *_create, so in theory to 
keep it consistent the destructor should be *_destroy, no?

Thank you,
Emanuele

Re: [Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes
Posted by Laurent Vivier 7 years, 2 months ago
On 09/08/2018 15:27, Emanuele wrote:
> 
>>> +{
>>> +    QSDHCI_PCI *spci = (QSDHCI_PCI *)obj;
>>> +    qpci_device_enable(&spci->dev);
>>> +}
>>> +
>>> +static void sdhci_destroy(QOSGraphObject *obj)
>> "sdhci_pci_destructor()"?
> In general, I called the constructor name  *_create, so in theory to
> keep it consistent the destructor should be *_destroy, no?

Yes, but the field name in QOSGraphObject is "destructor".

You have:

    obj.get_device = XXX_get_device;
    obj.get_driver = XXX_get_driver;
    obj.start_hw   = XXX_start_hw;

So we can expect:

   obj.destructor = XXX_destructor;

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes
Posted by Paolo Bonzini 7 years, 2 months ago
On 09/08/2018 12:10, Laurent Vivier wrote:
>>  libqgraph-pci-obj-y = $(libqos-pc-obj-y)
>>  libqgraph-pci-obj-y += $(libqgraph-machines-obj-y)
>> +libqgraph-pci-obj-y += tests/libqos/sdhci.o
> The file also implements memory mapped SDHCI, should we split the file
> in two files, one added to libqgraph-pci-obj-y and the other to
> check-qtest-arm-y and check-qtest-aarch64-y?
> 

No, there is only one libqgraph.  The same test (qos-test) can run for
both PCI and non-PCI machines, and will cull the unnecessary part of the
graph automatically simply because there is no path from a machine to a
PCI device.

Paolo

Re: [Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes
Posted by Laurent Vivier 7 years, 2 months ago
On 09/08/2018 12:15, Paolo Bonzini wrote:
> On 09/08/2018 12:10, Laurent Vivier wrote:
>>>  libqgraph-pci-obj-y = $(libqos-pc-obj-y)
>>>  libqgraph-pci-obj-y += $(libqgraph-machines-obj-y)
>>> +libqgraph-pci-obj-y += tests/libqos/sdhci.o
>> The file also implements memory mapped SDHCI, should we split the file
>> in two files, one added to libqgraph-pci-obj-y and the other to
>> check-qtest-arm-y and check-qtest-aarch64-y?
>>
> 
> No, there is only one libqgraph.  The same test (qos-test) can run for
> both PCI and non-PCI machines, and will cull the unnecessary part of the
> graph automatically simply because there is no path from a machine to a
> PCI device.

So, all libqgraph files should be added to check-qtest-generic-y?

Thanks,
Laurent

Re: [Qemu-devel] [PATCH v2 05/34] tests/qgraph: sdhci driver and interface nodes
Posted by Paolo Bonzini 7 years, 2 months ago
On 09/08/2018 13:01, Laurent Vivier wrote:
>> No, there is only one libqgraph.  The same test (qos-test) can run for
>> both PCI and non-PCI machines, and will cull the unnecessary part of the
>> graph automatically simply because there is no path from a machine to a
>> PCI device.
> So, all libqgraph files should be added to check-qtest-generic-y?

Yes.

Paolo