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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.