Re: [Qemu-ppc] [PATCHv2 09/11] libqos: Add 64-bit PCI IO accessors

[Laurent Vivier]

On 19/10/2016 14:25, David Gibson wrote:
> Currently the libqos PCI layer includes accessor helpers for 8, 16 and 32
> bit reads and writes.  It's likely that we'll want 64-bit accesses in the
> future (plenty of modern peripherals will have 64-bit reigsters).  This
> adds them.
> 
> For PIO (not MMIO) accesses on the PC backend, this is implemented as two
> 32-bit ins or outs.  That's not ideal but AFAICT x86 doesn't have 64-bit
> versions of in and out.
> 
> This patch also converts the single current user of 64-bit accesses -
> virtio-pci.c to use the new mechanism, rather than a sequence of 8 byte
> reads.
> 
> Signed-off-by: David Gibson <address@hidden>

Reviewed-by: Laurent Vivier <address@hidden>

> ---
>  tests/libqos/pci-pc.c     | 13 +++++++++++++
>  tests/libqos/pci-spapr.c  | 14 ++++++++++++++
>  tests/libqos/pci.c        | 25 +++++++++++++++++++++++++
>  tests/libqos/pci.h        |  4 ++++
>  tests/libqos/virtio-pci.c | 16 ++++------------
>  5 files changed, 60 insertions(+), 12 deletions(-)
> 
> diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c
> index 849ea56..ded1c54 100644
> --- a/tests/libqos/pci-pc.c
> +++ b/tests/libqos/pci-pc.c
> @@ -57,6 +57,17 @@ static void qpci_pc_pio_writel(QPCIBus *bus, uint32_t 
> addr, uint32_t val)
>      outl(addr, val);
>  }
>  
> +static uint64_t qpci_pc_pio_readq(QPCIBus *bus, uint32_t addr)
> +{
> +    return (uint64_t)inl(addr) + ((uint64_t)inl(addr + 4) << 32);
> +}
> +
> +static void qpci_pc_pio_writeq(QPCIBus *bus, uint32_t addr, uint64_t val)
> +{
> +    outl(addr, val & 0xffffffff);
> +    outl(addr + 4, val >> 32);
> +}
> +
>  static void qpci_pc_memread(QPCIBus *bus, uint32_t addr, void *buf, size_t 
> len)
>  {
>      memread(addr, buf, len);
> @@ -113,10 +124,12 @@ QPCIBus *qpci_init_pc(QGuestAllocator *alloc)
>      ret->bus.pio_readb = qpci_pc_pio_readb;
>      ret->bus.pio_readw = qpci_pc_pio_readw;
>      ret->bus.pio_readl = qpci_pc_pio_readl;
> +    ret->bus.pio_readq = qpci_pc_pio_readq;
>  
>      ret->bus.pio_writeb = qpci_pc_pio_writeb;
>      ret->bus.pio_writew = qpci_pc_pio_writew;
>      ret->bus.pio_writel = qpci_pc_pio_writel;
> +    ret->bus.pio_writeq = qpci_pc_pio_writeq;
>  
>      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 f26488a..1e5d015 100644
> --- a/tests/libqos/pci-spapr.c
> +++ b/tests/libqos/pci-spapr.c
> @@ -78,6 +78,18 @@ static void qpci_spapr_pio_writel(QPCIBus *bus, uint32_t 
> addr, uint32_t val)
>      writel(s->pio_cpu_base + addr, bswap32(val));
>  }
>  
> +static uint64_t qpci_spapr_pio_readq(QPCIBus *bus, uint32_t addr)
> +{
> +    QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> +    return bswap64(readq(s->pio_cpu_base + addr));
> +}
> +
> +static void qpci_spapr_pio_writeq(QPCIBus *bus, uint32_t addr, uint64_t val)
> +{
> +    QPCIBusSPAPR *s = container_of(bus, QPCIBusSPAPR, bus);
> +    writeq(s->pio_cpu_base + addr, bswap64(val));
> +}
> +
>  static void qpci_spapr_memread(QPCIBus *bus, uint32_t addr,
>                                 void *buf, size_t len)
>  {
> @@ -153,10 +165,12 @@ QPCIBus *qpci_init_spapr(QGuestAllocator *alloc)
>      ret->bus.pio_readb = qpci_spapr_pio_readb;
>      ret->bus.pio_readw = qpci_spapr_pio_readw;
>      ret->bus.pio_readl = qpci_spapr_pio_readl;
> +    ret->bus.pio_readq = qpci_spapr_pio_readq;
>  
>      ret->bus.pio_writeb = qpci_spapr_pio_writeb;
>      ret->bus.pio_writew = qpci_spapr_pio_writew;
>      ret->bus.pio_writel = qpci_spapr_pio_writel;
> +    ret->bus.pio_writeq = qpci_spapr_pio_writeq;
>  
>      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 bdffeb3..3021651 100644
> --- a/tests/libqos/pci.c
> +++ b/tests/libqos/pci.c
> @@ -262,6 +262,19 @@ uint32_t qpci_io_readl(QPCIDevice *dev, void *data)
>      }
>  }
>  
> +uint64_t qpci_io_readq(QPCIDevice *dev, void *data)
> +{
> +    uintptr_t addr = (uintptr_t)data;
> +
> +    if (addr < QPCI_PIO_LIMIT) {
> +        return dev->bus->pio_readq(dev->bus, addr);
> +    } else {
> +        uint64_t val;
> +        dev->bus->memread(dev->bus, addr, &val, sizeof(val));
> +        return le64_to_cpu(val);
> +    }
> +}
> +
>  void qpci_io_writeb(QPCIDevice *dev, void *data, uint8_t value)
>  {
>      uintptr_t addr = (uintptr_t)data;
> @@ -297,6 +310,18 @@ void qpci_io_writel(QPCIDevice *dev, void *data, 
> uint32_t value)
>      }
>  }
>  
> +void qpci_io_writeq(QPCIDevice *dev, void *data, uint64_t value)
> +{
> +    uintptr_t addr = (uintptr_t)data;
> +
> +    if (addr < QPCI_PIO_LIMIT) {
> +        dev->bus->pio_writeq(dev->bus, addr, value);
> +    } else {
> +        value = cpu_to_le64(value);
> +        dev->bus->memwrite(dev->bus, addr, &value, sizeof(value));
> +    }
> +}
> +
>  void qpci_memread(QPCIDevice *dev, void *data, void *buf, size_t len)
>  {
>      uintptr_t addr = (uintptr_t)data;
> diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h
> index ce6ed08..531e3f7 100644
> --- a/tests/libqos/pci.h
> +++ b/tests/libqos/pci.h
> @@ -26,10 +26,12 @@ 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);
> +    uint64_t (*pio_readq)(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 (*pio_writeq)(QPCIBus *bus, uint32_t addr, uint64_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);
> @@ -82,10 +84,12 @@ void qpci_config_writel(QPCIDevice *dev, uint8_t offset, 
> uint32_t value);
>  uint8_t qpci_io_readb(QPCIDevice *dev, void *data);
>  uint16_t qpci_io_readw(QPCIDevice *dev, void *data);
>  uint32_t qpci_io_readl(QPCIDevice *dev, void *data);
> +uint64_t qpci_io_readq(QPCIDevice *dev, void *data);
>  
>  void qpci_io_writeb(QPCIDevice *dev, void *data, uint8_t value);
>  void qpci_io_writew(QPCIDevice *dev, void *data, uint16_t value);
>  void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value);
> +void qpci_io_writeq(QPCIDevice *dev, void *data, uint64_t value);
>  
>  void qpci_memread(QPCIDevice *bus, void *data, void *buf, size_t len);
>  void qpci_memwrite(QPCIDevice *bus, void *data, const void *buf, size_t len);
> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> index fa82132..c69d09d 100644
> --- a/tests/libqos/virtio-pci.c
> +++ b/tests/libqos/virtio-pci.c
> @@ -106,22 +106,14 @@ static uint32_t qvirtio_pci_config_readl(QVirtioDevice 
> *d, uint64_t off)
>  static uint64_t qvirtio_pci_config_readq(QVirtioDevice *d, uint64_t off)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    int i;
> -    uint64_t u64 = 0;
> +    uint64_t val;
>  
> +    val = qpci_io_readq(dev->pdev, CONFIG_BASE(dev) + off);
>      if (qvirtio_is_big_endian(d)) {
> -        for (i = 0; i < 8; ++i) {
> -            u64 |= (uint64_t)qpci_io_readb(dev->pdev, CONFIG_BASE(dev)
> -                                           + off + i) << (7 - i) * 8;
> -        }
> -    } else {
> -        for (i = 0; i < 8; ++i) {
> -            u64 |= (uint64_t)qpci_io_readb(dev->pdev, CONFIG_BASE(dev)
> -                                           + off + i) << i * 8;
> -        }
> +        val = bswap64(val);
>      }
>  
> -    return u64;
> +    return val;
>  }
>  
>  static uint32_t qvirtio_pci_get_features(QVirtioDevice *d)
>