From: Greg Kurz <groug@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, qemu-ppc@nongnu.org,
lvivier@redhat.com, agraf@suse.de, stefanha@redhat.com,
mst@redhat.com, aik@ozlabs.ru, mdroth@linux.vnet.ibm.com,
thuth@redhat.com
Subject: Re: [Qemu-devel] [PATCHv3 03/11] libqos: Move BAR assignment to common code
Date: Thu, 20 Oct 2016 21:32:24 +0200 [thread overview]
Message-ID: <20161020213224.5f7a659d@bahia> (raw)
In-Reply-To: <1476934994-5515-4-git-send-email-david@gibson.dropbear.id.au>
On Thu, 20 Oct 2016 14:43:06 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:
> The PCI backends in libqos each supply an iomap() and iounmap() function
> which is used to set up a specified PCI BAR. But PCI BAR allocation takes
> place entirely within PCI space, so doesn't really need per-backend
> versions. For example, Linux includes generic BAR allocation code used on
> platforms where that isn't done by firmware.
>
> This patch merges the BAR allocation from the two existing backends into a
> single simplified copy. The back ends just need to set up some parameters
> describing the window of PCI IO and PCI memory addresses which are
> available for allocation. Like both the existing versions the new one uses
> a simple bump allocator.
>
> Note that (again like the existing versions) this doesn't really handle
> 64-bit memory BARs properly. It is actually used for such a BAR by the
> ivshmem test, and apparently the 32-bit MMIO BAR logic is close enough to
> work, as long as the BAR isn't too big. Fixing that to properly handle
> 64-bit BAR allocation is a problem for another time.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> tests/libqos/pci-pc.c | 87 ++--------------------------------------------
> tests/libqos/pci-spapr.c | 89 ++----------------------------------------------
> tests/libqos/pci.c | 56 ++++++++++++++++++++++++++++--
> tests/libqos/pci.h | 7 ++--
> 4 files changed, 63 insertions(+), 176 deletions(-)
>
> diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
> index 51dff8a..d0eb84a 100644
> --- a/tests/libqos/pci-pc.c
> +++ b/tests/libqos/pci-pc.c
> @@ -17,7 +17,6 @@
> #include "hw/pci/pci_regs.h"
>
> #include "qemu-common.h"
> -#include "qemu/host-utils.h"
>
>
> #define ACPI_PCIHP_ADDR 0xae00
> @@ -26,14 +25,6 @@
> typedef struct QPCIBusPC
> {
> QPCIBus bus;
> -
> - uint32_t pci_hole_start;
> - uint32_t pci_hole_size;
> - uint32_t pci_hole_alloc;
> -
> - uint16_t pci_iohole_start;
> - uint16_t pci_iohole_size;
> - uint16_t pci_iohole_alloc;
> } QPCIBusPC;
>
> static uint8_t qpci_pc_pio_readb(QPCIBus *bus, uint32_t addr)
> @@ -132,71 +123,6 @@ static void qpci_pc_config_writel(QPCIBus *bus, int devfn, uint8_t offset, uint3
> outl(0xcfc, value);
> }
>
> -static void *qpci_pc_iomap(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t *sizeptr)
> -{
> - QPCIBusPC *s = container_of(bus, QPCIBusPC, bus);
> - static const int bar_reg_map[] = {
> - PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_1, PCI_BASE_ADDRESS_2,
> - PCI_BASE_ADDRESS_3, PCI_BASE_ADDRESS_4, PCI_BASE_ADDRESS_5,
> - };
> - int bar_reg;
> - uint32_t addr;
> - uint64_t size;
> - uint32_t io_type;
> -
> - g_assert(barno >= 0 && barno <= 5);
> - bar_reg = bar_reg_map[barno];
> -
> - qpci_config_writel(dev, bar_reg, 0xFFFFFFFF);
> - addr = qpci_config_readl(dev, bar_reg);
> -
> - io_type = addr & PCI_BASE_ADDRESS_SPACE;
> - if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
> - addr &= PCI_BASE_ADDRESS_IO_MASK;
> - } else {
> - addr &= PCI_BASE_ADDRESS_MEM_MASK;
> - }
> -
> - size = (1ULL << ctzl(addr));
> - if (size == 0) {
> - return NULL;
> - }
> - if (sizeptr) {
> - *sizeptr = size;
> - }
> -
> - if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
> - uint16_t loc;
> -
> - g_assert(QEMU_ALIGN_UP(s->pci_iohole_alloc, size) + size
> - <= s->pci_iohole_size);
> - s->pci_iohole_alloc = QEMU_ALIGN_UP(s->pci_iohole_alloc, size);
> - loc = s->pci_iohole_start + s->pci_iohole_alloc;
> - s->pci_iohole_alloc += size;
> -
> - qpci_config_writel(dev, bar_reg, loc | PCI_BASE_ADDRESS_SPACE_IO);
> -
> - return (void *)(intptr_t)loc;
> - } else {
> - uint64_t loc;
> -
> - g_assert(QEMU_ALIGN_UP(s->pci_hole_alloc, size) + size
> - <= s->pci_hole_size);
> - s->pci_hole_alloc = QEMU_ALIGN_UP(s->pci_hole_alloc, size);
> - loc = s->pci_hole_start + s->pci_hole_alloc;
> - s->pci_hole_alloc += size;
> -
> - qpci_config_writel(dev, bar_reg, loc);
> -
> - return (void *)(intptr_t)loc;
> - }
> -}
> -
> -static void qpci_pc_iounmap(QPCIBus *bus, void *data)
> -{
> - /* FIXME */
> -}
> -
> QPCIBus *qpci_init_pc(QGuestAllocator *alloc)
> {
> QPCIBusPC *ret;
> @@ -227,16 +153,9 @@ QPCIBus *qpci_init_pc(QGuestAllocator *alloc)
> ret->bus.config_writew = qpci_pc_config_writew;
> ret->bus.config_writel = qpci_pc_config_writel;
>
> - ret->bus.iomap = qpci_pc_iomap;
> - ret->bus.iounmap = qpci_pc_iounmap;
> -
> - ret->pci_hole_start = 0xE0000000;
> - ret->pci_hole_size = 0x20000000;
> - ret->pci_hole_alloc = 0;
> -
> - ret->pci_iohole_start = 0xc000;
> - ret->pci_iohole_size = 0x4000;
> - ret->pci_iohole_alloc = 0;
> + ret->bus.pio_alloc_ptr = 0xc000;
> + ret->bus.mmio_alloc_ptr = 0xE0000000;
> + ret->bus.mmio_limit = 0x100000000ULL;
>
> return &ret->bus;
> }
> diff --git a/tests/libqos/pci-spapr.c b/tests/libqos/pci-spapr.c
> index 2d26a94..70a24b5 100644
> --- a/tests/libqos/pci-spapr.c
> +++ b/tests/libqos/pci-spapr.c
> @@ -34,14 +34,6 @@ typedef struct QPCIBusSPAPR {
>
> uint64_t mmio32_cpu_base;
> QPCIWindow mmio32;
> -
> - uint64_t pci_hole_start;
> - uint64_t pci_hole_size;
> - uint64_t pci_hole_alloc;
> -
> - uint32_t pci_iohole_start;
> - uint32_t pci_iohole_size;
> - uint32_t pci_iohole_alloc;
> } QPCIBusSPAPR;
>
> /*
> @@ -167,72 +159,6 @@ static void qpci_spapr_config_writel(QPCIBus *bus, int devfn, uint8_t offset,
> qrtas_ibm_write_pci_config(s->alloc, s->buid, config_addr, 4, value);
> }
>
> -static void *qpci_spapr_iomap(QPCIBus *bus, QPCIDevice *dev, int barno,
> - uint64_t *sizeptr)
> -{
> - QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> - static const int bar_reg_map[] = {
> - PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_1, PCI_BASE_ADDRESS_2,
> - PCI_BASE_ADDRESS_3, PCI_BASE_ADDRESS_4, PCI_BASE_ADDRESS_5,
> - };
> - int bar_reg;
> - uint32_t addr;
> - uint64_t size;
> - uint32_t io_type;
> -
> - g_assert(barno >= 0 && barno <= 5);
> - bar_reg = bar_reg_map[barno];
> -
> - qpci_config_writel(dev, bar_reg, 0xFFFFFFFF);
> - addr = qpci_config_readl(dev, bar_reg);
> -
> - io_type = addr & PCI_BASE_ADDRESS_SPACE;
> - if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
> - addr &= PCI_BASE_ADDRESS_IO_MASK;
> - } else {
> - addr &= PCI_BASE_ADDRESS_MEM_MASK;
> - }
> -
> - size = (1ULL << ctzl(addr));
> - if (size == 0) {
> - return NULL;
> - }
> - if (sizeptr) {
> - *sizeptr = size;
> - }
> -
> - if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
> - uint16_t loc;
> -
> - g_assert(QEMU_ALIGN_UP(s->pci_iohole_alloc, size) + size
> - <= s->pci_iohole_size);
> - s->pci_iohole_alloc = QEMU_ALIGN_UP(s->pci_iohole_alloc, size);
> - loc = s->pci_iohole_start + s->pci_iohole_alloc;
> - s->pci_iohole_alloc += size;
> -
> - qpci_config_writel(dev, bar_reg, loc | PCI_BASE_ADDRESS_SPACE_IO);
> -
> - return (void *)(unsigned long)loc;
> - } else {
> - uint64_t loc;
> -
> - g_assert(QEMU_ALIGN_UP(s->pci_hole_alloc, size) + size
> - <= s->pci_hole_size);
> - s->pci_hole_alloc = QEMU_ALIGN_UP(s->pci_hole_alloc, size);
> - loc = s->pci_hole_start + s->pci_hole_alloc;
> - s->pci_hole_alloc += size;
> -
> - qpci_config_writel(dev, bar_reg, loc);
> -
> - return (void *)(unsigned long)loc;
> - }
> -}
> -
> -static void qpci_spapr_iounmap(QPCIBus *bus, void *data)
> -{
> - /* FIXME */
> -}
> -
> #define SPAPR_PCI_BASE (1ULL << 45)
>
> #define SPAPR_PCI_MMIO32_WIN_SIZE 0x80000000 /* 2 GiB */
> @@ -270,9 +196,6 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
> ret->bus.config_writew = qpci_spapr_config_writew;
> ret->bus.config_writel = qpci_spapr_config_writel;
>
> - ret->bus.iomap = qpci_spapr_iomap;
> - ret->bus.iounmap = qpci_spapr_iounmap;
> -
> /* FIXME: We assume the default location of the PHB for now.
> * Ideally we'd parse the device tree deposited in the guest to
> * get the window locations */
> @@ -287,15 +210,9 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
> ret->mmio32.pci_base = 0x80000000; /* 2 GiB */
> ret->mmio32.size = SPAPR_PCI_MMIO32_WIN_SIZE;
>
> - ret->pci_hole_start = 0xC0000000;
> - ret->pci_hole_size =
> - ret->mmio32.pci_base + ret->mmio32.size - ret->pci_hole_start;
> - ret->pci_hole_alloc = 0;
> -
> - ret->pci_iohole_start = 0xc000;
> - ret->pci_iohole_size =
> - ret->pio.pci_base + ret->pio.size - ret->pci_iohole_start;
> - ret->pci_iohole_alloc = 0;
> + ret->bus.pio_alloc_ptr = 0xc000;
> + ret->bus.mmio_alloc_ptr = ret->mmio32.pci_base;
> + ret->bus.mmio_limit = ret->mmio32.pci_base + ret->mmio32.size;
>
> return &ret->bus;
> }
> diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c
> index 55b01df..bf1c532 100644
> --- a/tests/libqos/pci.c
> +++ b/tests/libqos/pci.c
> @@ -14,6 +14,7 @@
> #include "libqos/pci.h"
>
> #include "hw/pci/pci_regs.h"
> +#include "qemu/host-utils.h"
>
> void qpci_device_foreach(QPCIBus *bus, int vendor_id, int device_id,
> void (*func)(QPCIDevice *dev, int devfn, void *data),
> @@ -290,12 +291,63 @@ void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value)
>
> void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr)
> {
> - return dev->bus->iomap(dev->bus, dev, barno, sizeptr);
> + QPCIBus *bus = dev->bus;
> + static const int bar_reg_map[] = {
> + PCI_BASE_ADDRESS_0, PCI_BASE_ADDRESS_1, PCI_BASE_ADDRESS_2,
> + PCI_BASE_ADDRESS_3, PCI_BASE_ADDRESS_4, PCI_BASE_ADDRESS_5,
> + };
> + int bar_reg;
> + uint32_t addr, size;
> + uint32_t io_type;
> + uint64_t loc;
> +
> + g_assert(barno >= 0 && barno <= 5);
> + bar_reg = bar_reg_map[barno];
> +
> + qpci_config_writel(dev, bar_reg, 0xFFFFFFFF);
> + addr = qpci_config_readl(dev, bar_reg);
> +
> + io_type = addr & PCI_BASE_ADDRESS_SPACE;
> + if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
> + addr &= PCI_BASE_ADDRESS_IO_MASK;
> + } else {
> + addr &= PCI_BASE_ADDRESS_MEM_MASK;
> + }
> +
> + g_assert(addr); /* Must have *some* size bits */
> +
> + size = 1U << ctz32(addr);
> + if (sizeptr) {
> + *sizeptr = size;
> + }
> +
> + if (io_type == PCI_BASE_ADDRESS_SPACE_IO) {
> + loc = QEMU_ALIGN_UP(bus->pio_alloc_ptr, size);
> +
> + g_assert(loc >= bus->pio_alloc_ptr);
> + g_assert(loc + size <= QPCI_PIO_LIMIT); /* Keep PIO below 64kiB */
> +
> + bus->pio_alloc_ptr = loc + size;
> +
> + qpci_config_writel(dev, bar_reg, loc | PCI_BASE_ADDRESS_SPACE_IO);
> + } else {
> + loc = QEMU_ALIGN_UP(bus->mmio_alloc_ptr, size);
> +
> + /* Check for space */
> + g_assert(loc >= bus->mmio_alloc_ptr);
> + g_assert(loc + size <= bus->mmio_limit);
> +
> + bus->mmio_alloc_ptr = loc + size;
> +
> + qpci_config_writel(dev, bar_reg, loc);
> + }
> +
> + return (void *)(uintptr_t)loc;
> }
>
> void qpci_iounmap(QPCIDevice *dev, void *data)
> {
> - dev->bus->iounmap(dev->bus, data);
> + /* FIXME */
> }
>
> void qpci_plug_device_test(const char *driver, const char *id,
> diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
> index 72a2245..f6f916d 100644
> --- a/tests/libqos/pci.h
> +++ b/tests/libqos/pci.h
> @@ -22,8 +22,7 @@
> typedef struct QPCIDevice QPCIDevice;
> typedef struct QPCIBus QPCIBus;
>
> -struct QPCIBus
> -{
> +struct QPCIBus {
> uint8_t (*pio_readb)(QPCIBus *bus, uint32_t addr);
> uint16_t (*pio_readw)(QPCIBus *bus, uint32_t addr);
> uint32_t (*pio_readl)(QPCIBus *bus, uint32_t addr);
> @@ -51,8 +50,8 @@ struct QPCIBus
> void (*config_writel)(QPCIBus *bus, int devfn,
> uint8_t offset, uint32_t value);
>
> - void *(*iomap)(QPCIBus *bus, QPCIDevice *dev, int barno, uint64_t *sizeptr);
> - void (*iounmap)(QPCIBus *bus, void *data);
> + uint16_t pio_alloc_ptr;
> + uint64_t mmio_alloc_ptr, mmio_limit;
> };
>
> struct QPCIDevice
next prev parent reply other threads:[~2016-10-20 19:32 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-20 3:43 [Qemu-devel] [PATCHv3 00/11] Cleanups to qtest PCI handling David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 01/11] libqos: Give qvirtio_config_read*() consistent semantics David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 02/11] libqos: Handle PCI IO de-multiplexing in common code David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 03/11] libqos: Move BAR assignment to " David Gibson
2016-10-20 19:32 ` Greg Kurz [this message]
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 04/11] libqos: Better handling of PCI legacy IO David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 05/11] tests: Adjust tco-test to use qpci_legacy_iomap() David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 06/11] libqos: Add streaming accessors for PCI MMIO David Gibson
2016-10-20 19:37 ` Greg Kurz
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 07/11] libqos: Implement mmio accessors in terms of mem{read, write} David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 08/11] tests: Clean up IO handling in ide-test David Gibson
2016-10-20 9:54 ` Laurent Vivier
2016-10-20 23:49 ` David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 09/11] libqos: Add 64-bit PCI IO accessors David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 10/11] tests: Use qpci_mem{read, write} in ivshmem-test David Gibson
2016-10-20 3:43 ` [Qemu-devel] [PATCHv3 11/11] libqos: Change PCI accessors to take opaque BAR handle 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=20161020213224.5f7a659d@bahia \
--to=groug@kaod.org \
--cc=agraf@suse.de \
--cc=aik@ozlabs.ru \
--cc=david@gibson.dropbear.id.au \
--cc=lvivier@redhat.com \
--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).