From: Laurent Vivier <lvivier@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>,
pbonzini@redhat.com, qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org, agraf@suse.de, stefanha@redhat.com,
mst@redhat.com, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com,
groug@kaod.org, thuth@redhat.com
Subject: Re: [Qemu-devel] [PATCH 6/8] libqos: Implement mmio accessors in terms of mem{read, write}
Date: Tue, 18 Oct 2016 18:04:01 +0200 [thread overview]
Message-ID: <91e82aff-3948-3502-109d-3c16a7930b07@redhat.com> (raw)
In-Reply-To: <1476787933-7180-7-git-send-email-david@gibson.dropbear.id.au>
On 18/10/2016 12:52, David Gibson wrote:
> In the libqos PCI code we now have accessors both for registers (byte
> significance preserving) and for streaming data (byte address order
> preserving). These exist in both the interface for qtest drivers and in
> the machine specific backends.
>
> However, the register-style accessors aren't actually necessary in the
> backend. They can be implemented in terms of the byte address order
> preserving accessors by the libqos wrappers. This works because PCI is
> always little endian.
>
> This does assume that the back end byte address order preserving accessors
> will perform the equivalent of a single bus transaction for short lengths.
> This is the case, and in fact they currently end up using the same
> cpu_physical_memory_rw() implementation within the qtest accelerator.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> ---
> tests/libqos/pci-pc.c | 38 --------------------------------------
> tests/libqos/pci-spapr.c | 44 --------------------------------------------
> tests/libqos/pci.c | 20 ++++++++++++++------
> tests/libqos/pci.h | 8 --------
> 4 files changed, 14 insertions(+), 96 deletions(-)
>
> diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
> index 0e95772..e8b885a 100644
> --- a/tests/libqos/pci-pc.c
> +++ b/tests/libqos/pci-pc.c
> @@ -40,61 +40,31 @@ static uint8_t qpci_pc_pio_readb(QPCIBus *bus, uint32_t addr)
> return inb(addr);
> }
>
> -static uint8_t qpci_pc_mmio_readb(QPCIBus *bus, uint32_t addr)
> -{
> - return readb(addr);
> -}
> -
> static void qpci_pc_pio_writeb(QPCIBus *bus, uint32_t addr, uint8_t val)
> {
> outb(addr, val);
> }
>
> -static void qpci_pc_mmio_writeb(QPCIBus *bus, uint32_t addr, uint8_t val)
> -{
> - writeb(addr, val);
> -}
> -
> static uint16_t qpci_pc_pio_readw(QPCIBus *bus, uint32_t addr)
> {
> return inw(addr);
> }
>
> -static uint16_t qpci_pc_mmio_readw(QPCIBus *bus, uint32_t addr)
> -{
> - return readw(addr);
> -}
> -
> static void qpci_pc_pio_writew(QPCIBus *bus, uint32_t addr, uint16_t val)
> {
> outw(addr, val);
> }
>
> -static void qpci_pc_mmio_writew(QPCIBus *bus, uint32_t addr, uint16_t val)
> -{
> - writew(addr, val);
> -}
> -
> static uint32_t qpci_pc_pio_readl(QPCIBus *bus, uint32_t addr)
> {
> return inl(addr);
> }
>
> -static uint32_t qpci_pc_mmio_readl(QPCIBus *bus, uint32_t addr)
> -{
> - return readl(addr);
> -}
> -
> static void qpci_pc_pio_writel(QPCIBus *bus, uint32_t addr, uint32_t val)
> {
> outl(addr, val);
> }
>
> -static void qpci_pc_mmio_writel(QPCIBus *bus, uint32_t addr, uint32_t val)
> -{
> - writel(addr, val);
> -}
> -
> static void qpci_pc_memread(QPCIBus *bus, uint32_t addr, void *buf, size_t len)
> {
> memread(addr, buf, len);
> @@ -156,14 +126,6 @@ QPCIBus *qpci_init_pc(QGuestAllocator *alloc)
> ret->bus.pio_writew = qpci_pc_pio_writew;
> ret->bus.pio_writel = qpci_pc_pio_writel;
>
> - ret->bus.mmio_readb = qpci_pc_mmio_readb;
> - ret->bus.mmio_readw = qpci_pc_mmio_readw;
> - ret->bus.mmio_readl = qpci_pc_mmio_readl;
> -
> - ret->bus.mmio_writeb = qpci_pc_mmio_writeb;
> - ret->bus.mmio_writew = qpci_pc_mmio_writew;
> - ret->bus.mmio_writel = qpci_pc_mmio_writel;
> -
> ret->bus.memread = qpci_pc_memread;
> ret->bus.memwrite = qpci_pc_memwrite;
>
> diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
> index b4c4b81..dc89b25 100644
> --- a/tests/libqos/pci-spapr.c
> +++ b/tests/libqos/pci-spapr.c
> @@ -56,72 +56,36 @@ static uint8_t qpci_spapr_pio_readb(QPCIBus *bus, uint32_t addr)
> return readb(s->pio_cpu_base + addr);
> }
>
> -static uint8_t qpci_spapr_mmio32_readb(QPCIBus *bus, uint32_t addr)
> -{
> - QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> - return readb(s->mmio32_cpu_base + addr);
> -}
> -
> static void qpci_spapr_pio_writeb(QPCIBus *bus, uint32_t addr, uint8_t val)
> {
> QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> writeb(s->pio_cpu_base + addr, val);
> }
>
> -static void qpci_spapr_mmio32_writeb(QPCIBus *bus, uint32_t addr, uint8_t val)
> -{
> - QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> - writeb(s->mmio32_cpu_base + addr, val);
> -}
> -
> static uint16_t qpci_spapr_pio_readw(QPCIBus *bus, uint32_t addr)
> {
> QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> return bswap16(readw(s->pio_cpu_base + addr));
> }
>
> -static uint16_t qpci_spapr_mmio32_readw(QPCIBus *bus, uint32_t addr)
> -{
> - QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> - return bswap16(readw(s->mmio32_cpu_base + addr));
> -}
> -
> static void qpci_spapr_pio_writew(QPCIBus *bus, uint32_t addr, uint16_t val)
> {
> QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> writew(s->pio_cpu_base + addr, bswap16(val));
> }
>
> -static void qpci_spapr_mmio32_writew(QPCIBus *bus, uint32_t addr, uint16_t val)
> -{
> - QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> - writew(s->mmio32_cpu_base + addr, bswap16(val));
> -}
> -
> static uint32_t qpci_spapr_pio_readl(QPCIBus *bus, uint32_t addr)
> {
> QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> return bswap32(readl(s->pio_cpu_base + addr));
> }
>
> -static uint32_t qpci_spapr_mmio32_readl(QPCIBus *bus, uint32_t addr)
> -{
> - QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> - return bswap32(readl(s->mmio32_cpu_base + addr));
> -}
> -
> static void qpci_spapr_pio_writel(QPCIBus *bus, uint32_t addr, uint32_t val)
> {
> QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> writel(s->pio_cpu_base + addr, bswap32(val));
> }
>
> -static void qpci_spapr_mmio32_writel(QPCIBus *bus, uint32_t addr, uint32_t val)
> -{
> - QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> - writel(s->mmio32_cpu_base + addr, bswap32(val));
> -}
> -
> static void qpci_spapr_memread(QPCIBus *bus, uint32_t addr,
> void *buf, size_t len)
> {
> @@ -202,14 +166,6 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
> ret->bus.pio_writew = qpci_spapr_pio_writew;
> ret->bus.pio_writel = qpci_spapr_pio_writel;
>
> - ret->bus.mmio_readb = qpci_spapr_mmio32_readb;
> - ret->bus.mmio_readw = qpci_spapr_mmio32_readw;
> - ret->bus.mmio_readl = qpci_spapr_mmio32_readl;
> -
> - ret->bus.mmio_writeb = qpci_spapr_mmio32_writeb;
> - ret->bus.mmio_writew = qpci_spapr_mmio32_writew;
> - ret->bus.mmio_writel = qpci_spapr_mmio32_writel;
> -
> ret->bus.memread = qpci_spapr_memread;
> ret->bus.memwrite = qpci_spapr_memwrite;
>
> diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
> index 1e242f9..bbacbcf 100644
> --- a/tests/libqos/pci.c
> +++ b/tests/libqos/pci.c
> @@ -230,7 +230,9 @@ uint8_t qpci_io_readb(QPCIDevice *dev, void *data)
> if (addr < QPCI_PIO_LIMIT) {
> return dev->bus->pio_readb(dev->bus, addr);
> } else {
> - return dev->bus->mmio_readb(dev->bus, addr);
> + uint8_t val;
> + dev->bus->memread(dev->bus, addr, &val, sizeof(val));
> + return val;
> }
> }
>
> @@ -241,7 +243,9 @@ uint16_t qpci_io_readw(QPCIDevice *dev, void *data)
> if (addr < QPCI_PIO_LIMIT) {
> return dev->bus->pio_readw(dev->bus, addr);
> } else {
> - return dev->bus->mmio_readw(dev->bus, addr);
> + uint16_t val;
> + dev->bus->memread(dev->bus, addr, &val, sizeof(val));
> + return le16_to_cpu(val);
> }
> }
>
> @@ -252,7 +256,9 @@ uint32_t qpci_io_readl(QPCIDevice *dev, void *data)
> if (addr < QPCI_PIO_LIMIT) {
> return dev->bus->pio_readl(dev->bus, addr);
> } else {
> - return dev->bus->mmio_readl(dev->bus, addr);
> + uint32_t val;
> + dev->bus->memread(dev->bus, addr, &val, sizeof(val));
> + return le32_to_cpu(val);
> }
> }
>
> @@ -263,7 +269,7 @@ void qpci_io_writeb(QPCIDevice *dev, void *data, uint8_t value)
> if (addr < QPCI_PIO_LIMIT) {
> dev->bus->pio_writeb(dev->bus, addr, value);
> } else {
> - dev->bus->mmio_writeb(dev->bus, addr, value);
> + dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
> }
> }
>
> @@ -274,7 +280,8 @@ void qpci_io_writew(QPCIDevice *dev, void *data, uint16_t value)
> if (addr < QPCI_PIO_LIMIT) {
> dev->bus->pio_writew(dev->bus, addr, value);
> } else {
> - dev->bus->mmio_writew(dev->bus, addr, value);
> + value = cpu_to_le16(value);
> + dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
> }
> }
>
> @@ -285,7 +292,8 @@ void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value)
> if (addr < QPCI_PIO_LIMIT) {
> dev->bus->pio_writel(dev->bus, addr, value);
> } else {
> - dev->bus->mmio_writel(dev->bus, addr, value);
> + value = cpu_to_le32(value);
> + dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
> }
> }
>
> diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
> index 2fa5b4f..59fa3da 100644
> --- a/tests/libqos/pci.h
> +++ b/tests/libqos/pci.h
> @@ -27,18 +27,10 @@ struct QPCIBus {
> uint16_t (*pio_readw)(QPCIBus *bus, uint32_t addr);
> uint32_t (*pio_readl)(QPCIBus *bus, uint32_t addr);
>
> - uint8_t (*mmio_readb)(QPCIBus *bus, uint32_t addr);
> - uint16_t (*mmio_readw)(QPCIBus *bus, uint32_t addr);
> - uint32_t (*mmio_readl)(QPCIBus *bus, uint32_t addr);
> -
> void (*pio_writeb)(QPCIBus *bus, uint32_t addr, uint8_t value);
> void (*pio_writew)(QPCIBus *bus, uint32_t addr, uint16_t value);
> void (*pio_writel)(QPCIBus *bus, uint32_t addr, uint32_t value);
>
> - void (*mmio_writeb)(QPCIBus *bus, uint32_t addr, uint8_t value);
> - void (*mmio_writew)(QPCIBus *bus, uint32_t addr, uint16_t value);
> - void (*mmio_writel)(QPCIBus *bus, uint32_t addr, uint32_t value);
> -
> void (*memread)(QPCIBus *bus, uint32_t addr, void *buf, size_t len);
> void (*memwrite)(QPCIBus *bus, uint32_t addr, const void *buf, size_t len);
>
>
next prev parent reply other threads:[~2016-10-18 16:04 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-18 10:52 [Qemu-devel] [PATCH 0/8] Cleanups to qtest PCI handling David Gibson
2016-10-18 10:52 ` [Qemu-devel] [PATCH 1/8] libqos: Give qvirtio_config_read*() consistent semantics David Gibson
2016-10-18 12:41 ` Laurent Vivier
2016-10-18 13:27 ` Greg Kurz
2016-10-19 2:41 ` David Gibson
2016-10-18 10:52 ` [Qemu-devel] [PATCH 2/8] libqos: Handle PCI IO de-multiplexing in common code David Gibson
2016-10-18 13:28 ` Laurent Vivier
2016-10-19 2:59 ` David Gibson
2016-10-18 10:52 ` [Qemu-devel] [PATCH 3/8] libqos: Move BAR assignment to " David Gibson
2016-10-18 15:00 ` Laurent Vivier
2016-10-19 3:07 ` David Gibson
2016-10-18 10:52 ` [Qemu-devel] [PATCH 4/8] tests: Better handle legacy IO addresses in tco-test David Gibson
2016-10-18 15:14 ` Laurent Vivier
2016-10-18 16:28 ` Laurent Vivier
2016-10-19 3:19 ` David Gibson
2016-10-19 3:09 ` David Gibson
2016-10-18 10:52 ` [Qemu-devel] [PATCH 5/8] libqos: Add streaming accessors for PCI MMIO David Gibson
2016-10-18 15:17 ` Laurent Vivier
2016-10-18 10:52 ` [Qemu-devel] [PATCH 6/8] libqos: Implement mmio accessors in terms of mem{read, write} David Gibson
2016-10-18 16:04 ` Laurent Vivier [this message]
2016-10-18 10:52 ` [Qemu-devel] [PATCH 7/8] tests: Use qpci_mem{read, write} in ivshmem-test David Gibson
2016-10-18 16:14 ` Laurent Vivier
2016-10-19 4:13 ` David Gibson
2016-10-18 10:52 ` [Qemu-devel] [PATCH 8/8] libqos: Change PCI accessors to take opaque BAR handle David Gibson
2016-10-18 16:48 ` Laurent Vivier
2016-10-19 4:06 ` David Gibson
2016-10-18 11:56 ` [Qemu-devel] [PATCH 0/8] Cleanups to qtest PCI handling Laurent Vivier
2016-10-19 1:11 ` David Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=91e82aff-3948-3502-109d-3c16a7930b07@redhat.com \
--to=lvivier@redhat.com \
--cc=agraf@suse.de \
--cc=aik@ozlabs.ru \
--cc=david@gibson.dropbear.id.au \
--cc=groug@kaod.org \
--cc=mdroth@linux.vnet.ibm.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=stefanha@redhat.com \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).