Re: [Qemu-devel] [PATCH 1/1] Add support for PCI Enhanced Allocation "BARs"

[Michael S. Tsirkin]

On Mon, May 11, 2015 at 11:56:44AM -0700, Sean O. Stalley wrote:
> PCI Enhanced Allocation is a new method of allocating MMIO & IO
> resources for PCI devices & bridges. It can be used instead
> of the traditional PCI method of using BARs.
> 
> EA entries are hardware-initialized to a fixed address.
> Unlike BARs, regions described by EA are cannot be moved.
> Because of this, only devices which are perminately connected to
> the PCI bus can use EA. A removeable PCI card must not use EA.
> 
> This patchset enables any existing QEMU PCI model to use EA in leiu of
> BARs. It adds EA options to the PCI device paramaters.
> 
> The Enhanced Allocation ECN is publicly available here:
> https://www.pcisig.com/specifications/conventional/ECN_Enhanced_Allocation_23_Oct_2014_Final.pdf
> 
> Signed-off-by: Sean O. Stalley <address@hidden>


I will review this, thanks.  Will you also be sending a seabios patch to
support these registers?  When you do, please Cc me.


> ---
>  hw/pci/Makefile.objs      |   2 +-
>  hw/pci/pci.c              |  96 ++++++++++++++++------
>  hw/pci/pci_ea.c           | 203 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/pci/pci.h      |   7 ++
>  include/hw/pci/pci_ea.h   |  39 +++++++++
>  include/hw/pci/pci_regs.h |   4 +
>  include/qemu/typedefs.h   |   1 +
>  7 files changed, 328 insertions(+), 24 deletions(-)
>  create mode 100644 hw/pci/pci_ea.c
>  create mode 100644 include/hw/pci/pci_ea.h
> 
> diff --git a/hw/pci/Makefile.objs b/hw/pci/Makefile.objs
> index 9f905e6..e5d80cf 100644
> --- a/hw/pci/Makefile.objs
> +++ b/hw/pci/Makefile.objs
> @@ -3,7 +3,7 @@ common-obj-$(CONFIG_PCI) += msix.o msi.o
>  common-obj-$(CONFIG_PCI) += shpc.o
>  common-obj-$(CONFIG_PCI) += slotid_cap.o
>  common-obj-$(CONFIG_PCI) += pci_host.o pcie_host.o
> -common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o
> +common-obj-$(CONFIG_PCI) += pcie.o pcie_aer.o pcie_port.o pci_ea.o
>  
>  common-obj-$(call lnot,$(CONFIG_PCI)) += pci-stub.o
>  common-obj-$(CONFIG_ALL) += pci-stub.o
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b3d5100..c7552ca 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -23,6 +23,7 @@
>   */
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
> +#include "hw/pci/pci_ea.h"
>  #include "hw/pci/pci_bridge.h"
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci/pci_host.h"
> @@ -58,6 +59,10 @@ static Property pci_props[] = {
>                      QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
>      DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,
>                      QEMU_PCI_CAP_SERR_BITNR, true),
> +    DEFINE_PROP_BOOL("enhanced_allocation", PCIDevice, enhanced, false),
> +    DEFINE_PROP_ARRAY("ea_addr", PCIDevice, ea_addr_len, ea_addr,
> +                      qdev_prop_uint64, hwaddr),
> +
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> @@ -882,6 +887,7 @@ static PCIDevice *do_pci_register_device(PCIDevice 
> *pci_dev, PCIBus *bus,
>          config_read = pci_default_read_config;
>      if (!config_write)
>          config_write = pci_default_write_config;
> +
>      pci_dev->config_read = config_read;
>      pci_dev->config_write = config_write;
>      bus->devices[devfn] = pci_dev;
> @@ -929,40 +935,72 @@ void pci_register_bar(PCIDevice *pci_dev, int 
> region_num,
>  
>      assert(region_num >= 0);
>      assert(region_num < PCI_NUM_REGIONS);
> -    if (size & (size-1)) {
> +
> +
> +    /* TODO: these checks should be done earlier */
> +    if (!pci_dev->enhanced && (size & (size-1))) {
>          fprintf(stderr, "ERROR: PCI region size must be pow2 "
>                      "type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);
>          exit(1);
>      }
>  
> +    if (pci_dev->enhanced) {
> +
> +        if (pci_dev->ea_addr_len <= region_num) {
> +            fprintf(stderr, "ERROR: Address for EA entry %i not given\n",
> +                    region_num);
> +            exit(1);
> +        }
> +
> +        if (pci_dev->ea_addr[region_num] == 0x0) {
> +            fprintf(stderr, "ERROR: Address for EA entry %i not set\n",
> +                    region_num);
> +            exit(1);
> +        }
> +
> +        if (pci_ea_invalid_addr(pci_dev->ea_addr[region_num])) {
> +            fprintf(stderr, "ERROR: Address for EA entry %i not valid\n",
> +                    region_num);
> +            exit(1);
> +        }
> +    }
> +
>      r = &pci_dev->io_regions[region_num];
>      r->addr = PCI_BAR_UNMAPPED;
>      r->size = size;
>      r->type = type;
> -    r->memory = NULL;
> -
> -    wmask = ~(size - 1);
> -    addr = pci_bar(pci_dev, region_num);
> -    if (region_num == PCI_ROM_SLOT) {
> -        /* ROM enable bit is writable */
> -        wmask |= PCI_ROM_ADDRESS_ENABLE;
> -    }
> -    pci_set_long(pci_dev->config + addr, type);
> -    if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> -        r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> -        pci_set_quad(pci_dev->wmask + addr, wmask);
> -        pci_set_quad(pci_dev->cmask + addr, ~0ULL);
> -    } else {
> -        pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> -        pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> +    r->enhanced = pci_dev->enhanced;
> +    r->memory = memory;
> +    r->address_space = type & PCI_BASE_ADDRESS_SPACE_IO ?
> +        pci_dev->bus->address_space_io : pci_dev->bus->address_space_mem;
> +
> +    if (!pci_dev->enhanced) {
> +
> +        wmask = ~(size - 1);
> +        addr = pci_bar(pci_dev, region_num);
> +        if (region_num == PCI_ROM_SLOT) {
> +            /* ROM enable bit is writable */
> +            wmask |= PCI_ROM_ADDRESS_ENABLE;
> +        }
> +        pci_set_long(pci_dev->config + addr, type);
> +        if (!(r->type & PCI_BASE_ADDRESS_SPACE_IO) &&
> +            r->type & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> +            pci_set_quad(pci_dev->wmask + addr, wmask);
> +            pci_set_quad(pci_dev->cmask + addr, ~0ULL);
> +        } else {
> +            pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> +            pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> +        }
> +    } else { /* enhanced */
> +
> +        /* add the memory space now */
> +        r->addr = pci_dev->ea_addr[region_num];
> +        memory_region_add_subregion(r->address_space, r->addr, r->memory);
>      }
> -    pci_dev->io_regions[region_num].memory = memory;
> -    pci_dev->io_regions[region_num].address_space
> -        = type & PCI_BASE_ADDRESS_SPACE_IO
> -        ? pci_dev->bus->address_space_io
> -        : pci_dev->bus->address_space_mem;
>  }
>  
> +
> +
>  static void pci_update_vga(PCIDevice *pci_dev)
>  {
>      uint16_t cmd;
> @@ -1107,6 +1145,11 @@ static void pci_update_mappings(PCIDevice *d)
>  
>          new_addr = pci_bar_address(d, i, r->type, r->size);
>  
> +        /* EA BARs cannot be moved */
> +        if (r->enhanced) {
> +            continue;
> +        }
> +
>          /* This bar isn't changed */
>          if (new_addr == r->addr)
>              continue;
> @@ -1785,8 +1828,9 @@ static void pci_qdev_realize(DeviceState *qdev, Error 
> **errp)
>      pci_dev = do_pci_register_device(pci_dev, bus,
>                                       object_get_typename(OBJECT(qdev)),
>                                       pci_dev->devfn, errp);
> -    if (pci_dev == NULL)
> +    if (pci_dev == NULL) {
>          return;
> +    }
>  
>      if (pc->realize) {
>          pc->realize(pci_dev, &local_err);
> @@ -1797,6 +1841,12 @@ static void pci_qdev_realize(DeviceState *qdev, Error 
> **errp)
>          }
>      }
>  
> +    /* write the EA entry */
> +    if (pci_dev->enhanced) {
> +        pci_ea_cap_init(pci_dev);
> +    }
> +
> +
>      /* rom loading */
>      is_default_rom = false;
>      if (pci_dev->romfile == NULL && pc->romfile != NULL) {
> diff --git a/hw/pci/pci_ea.c b/hw/pci/pci_ea.c
> new file mode 100644
> index 0000000..a052519
> --- /dev/null
> +++ b/hw/pci/pci_ea.c
> @@ -0,0 +1,203 @@
> +/*
> + * PCI Enhanced Allocation (EA) Helper functions
> + * Copyright (c) 2015, Intel Corporation.
> + *
> + * Written by Sean O. Stalley <address@hidden>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +/*
> + * Contians a lot of code taken from pcie.c
> + */
> +
> +#include "qemu-common.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_ea.h"
> +
> +bool pci_ea_invalid_addr(uint64_t addr)
> +{
> +    return (addr != (addr & PCI_EA_ADDR_MASK_64));
> +}
> +
> +/* determines if a 64 bit address is necessary for this address */
> +static bool pci_ea_addr_is_64(uint64_t addr)
> +{
> +    return (addr != (uint32_t)addr);
> +}
> +
> +/* Sets the first 32 bits of the Base or MaxOffset field
> + * Returns the length of the address set
> + */
> +static uint8_t pci_ea_set_addr(PCIDevice *dev, uint8_t pos, uint64_t val)
> +{
> +    uint8_t *current_reg = dev->config + pos;
> +
> +    val = val & PCI_EA_ADDR_MASK_64;
> +
> +    if (pci_ea_addr_is_64(val)) {
> +        val |= PCI_EA_FIELD_SIZE_FLAG;
> +    }
> +
> +    pci_set_long(current_reg, val);
> +    return sizeof(int32_t);
> +}
> +
> +/* Determines EA entry length for this IORegion in bytes
> + * (including the first DWORD) */
> +static int pci_ea_entry_length(PCIIORegion *r)
> +{
> +
> +    int len = 3 * sizeof(int32_t); /* First 32 bits of Base & Offset */
> +
> +    if (pci_ea_addr_is_64(r->addr)) {
> +        len += sizeof(int32_t); /* Base (if 64 bit) */
> +    }
> +
> +    if (pci_ea_addr_is_64(r->size - 1)) {
> +        len += sizeof(int32_t); /* Offset (if 64 bit) */
> +    }
> +
> +    return len;
> +
> +}
> +
> +/* determines the value to be set in the Entry Size field */
> +static int pci_ea_get_es(PCIIORegion *r)
> +{
> +
> +    int es = (pci_ea_entry_length(r) / sizeof(int32_t)) - 1;
> +
> +    assert(PCI_EA_MAX_ES >= es);
> +
> +    return es;
> +}
> +
> +/* Initialize an EA entry for the given PCIIORegion
> + *
> + * @dev: The PCI Device
> + * @bei: The BAR Equivalent Indicator
> + * @pos: The offset of this entry in PCI Configspace;
> + */
> +
> +static int pci_ea_fill_entry(PCIDevice *dev, int bei, int pos)
> +{
> +    PCIIORegion *r = &dev->io_regions[bei];
> +    uint32_t dw0 = 0; /* used for setting first DWORD */
> +    int len = 0;
> +
> +    assert(bei <= PCI_EA_MAX_BEI);
> +
> +    /* Check that the base and size are DWORD aligned */
> +    assert(!pci_ea_invalid_addr(r->addr));
> +    assert(!pci_ea_invalid_addr(r->size));
> +
> +    dw0 |= pci_ea_get_es(r);
> +    dw0 |= (bei << PCI_EA_BEI_OFFSET);
> +    dw0 |= (PCI_EA_ENABLE);
> +
> +    /* base Primary Properties off of type field */
> +    if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> +        dw0 |= (PCI_EA_IO << PCI_EA_PP_OFFSET);
> +
> +    } else if (r->type & PCI_BASE_ADDRESS_MEM_PREFETCH) {
> +        dw0 |= (PCI_EA_MEM_PREFETCH << PCI_EA_PP_OFFSET);
> +
> +    } else {
> +        dw0 |= (PCI_EA_MEM << PCI_EA_PP_OFFSET);
> +    }
> +
> +    /* Secondary Properties should be ignored */
> +    dw0 |= (PCI_EA_UNAVAL << PCI_EA_SP_OFFSET);
> +
> +    pci_set_long(dev->config + pos, dw0);
> +
> +    len += sizeof(int32_t);
> +
> +    len += pci_ea_set_addr(dev, pos + len, r->addr);
> +    len += pci_ea_set_addr(dev, pos + len, r->size - 1);
> +
> +    /* Base (if 64 bit) */
> +    if (pci_ea_addr_is_64(r->addr)) {
> +        pci_set_long(dev->config + pos + len, (r->addr >> 32));
> +        len += sizeof(int32_t);
> +    }
> +
> +    /* Offset (if 64 bit) */
> +    if (pci_ea_addr_is_64(r->size - 4)) {
> +        pci_set_long(dev->config + pos + len, (r->size >> 32));
> +        len += sizeof(int32_t);
> +    }
> +
> +    assert(len == pci_ea_entry_length(r));
> +
> +    return len;
> +
> +}
> +
> +/*
> + * Initialize the EA capability descriptor
> + * must be done after the EA memory regions are initialized
> + * (or else the EA entries won't get written)
> + */
> +int pci_ea_cap_init(PCIDevice *dev)
> +{
> +    int pos;
> +    int ret;
> +    int i;
> +    int num_entries = 0;
> +    uint8_t length; /* length of this cap entry */
> +    PCIIORegion *r;
> +
> +    /* Determine the length of the capability entry */
> +
> +    length = sizeof(int32_t); /* DWORD 0: Cap ID, Next Pointer, Num Entries 
> */
> +    /* TODO: DWORD 1 for Type 1 Functions */
> +
> +    /* add the length of every entry */
> +    for (i = 0; i < PCI_NUM_REGIONS; ++i) {
> +        r = &dev->io_regions[i];
> +
> +        if (r->enhanced) {
> +            num_entries++;
> +            length += pci_ea_entry_length(r);
> +        }
> +    }
> +
> +    /* add the EA CAP (sets DWORD 0) */
> +    pos = pci_add_capability(dev, PCI_CAP_ID_EA, 0, length);
> +    if (0 > pos) {
> +        return pos;
> +    }
> +
> +    /* DWORD 0: Cap ID, Next Pointer, Num Entries */
> +    assert(num_entries <= PCI_EA_MAX_NUM_ENTRIES); /* overflow */
> +    pci_set_byte(dev->config + pos + PCI_EA_NUM_ENTRIES_OFFSET, num_entries);
> +    pos += sizeof(int32_t);
> +
> +    for (i = 0; i < PCI_NUM_REGIONS; ++i) {
> +        r = &dev->io_regions[i];
> +
> +        if (r->enhanced) {
> +            ret = pci_ea_fill_entry(dev, i, pos);
> +
> +            if (0 > ret) {
> +                return ret;
> +            }
> +            pos += ret;
> +        }
> +
> +    }
> +
> +    assert(pos == pci_find_capability(dev, PCI_CAP_ID_EA) + length);
> +
> +    return pos;
> +
> +}
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index d4ffead..da64435 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -12,6 +12,7 @@
>  #include "hw/isa/isa.h"
>  
>  #include "hw/pci/pcie.h"
> +#include "hw/pci/pci_ea.h"
>  
>  /* PCI bus */
>  
> @@ -109,6 +110,7 @@ typedef struct PCIIORegion {
>      uint8_t type;
>      MemoryRegion *memory;
>      MemoryRegion *address_space;
> +    bool          enhanced;
>  } PCIIORegion;
>  
>  #define PCI_ROM_SLOT 6
> @@ -288,6 +290,11 @@ struct PCIDevice {
>      /* PCI Express */
>      PCIExpressDevice exp;
>  
> +    /* Enhanced Allocation */
> +    bool enhanced;
> +    uint32_t ea_addr_len;
> +    hwaddr *ea_addr;
> +
>      /* SHPC */
>      SHPCDevice *shpc;
>  
> diff --git a/include/hw/pci/pci_ea.h b/include/hw/pci/pci_ea.h
> new file mode 100644
> index 0000000..2b63012
> --- /dev/null
> +++ b/include/hw/pci/pci_ea.h
> @@ -0,0 +1,39 @@
> +/* Constants for pci enhanced allocation (EA) capability descriptor */
> +
> +#ifndef QEMU_PCI_EA_H
> +#define QEMU_PCI_EA_H
> +
> +#define PCI_EA_FIELD_SIZE_FLAG 0x00000002
> +
> +/* the address' must be DWORD aligned */
> +#define PCI_EA_ADDR_MASK_32 0xfffffffc
> +#define PCI_EA_ADDR_MASK_64 0xfffffffffffffffc
> +
> +#define PCI_EA_MAX_NUM_ENTRIES 0x3f
> +
> +#define PCI_EA_BEI_OFFSET 4 /* In bits from beginning of EA entry */
> +#define PCI_EA_ENABLE 0x80000000
> +#define PCI_EA_MAX_BEI 0x8
> +#define PCI_EA_MAX_ES  0x4
> +
> +/* Primary & Secondary Properties, per table 6-1 */
> +#define PCI_EA_PP_OFFSET 8
> +#define PCI_EA_SP_OFFSET 16
> +
> +/* Primary & Secondary Properties Fields, per table 6-1 */
> +#define PCI_EA_MEM               0x00 /* Non-Prefetchable */
> +#define PCI_EA_MEM_PREFETCH      0x01
> +#define PCI_EA_IO                0x02
> +#define PCI_EA_VIRT_MEM_PREFETCH 0x03
> +#define PCI_EA_VIRT_MEM          0x04 /* Non-Prefetchable */
> +#define PCI_EA_UNAVAL_MEM        0xFD
> +#define PCI_EA_UNAVAL_IO         0xFE
> +#define PCI_EA_UNAVAL            0xFF
> +
> +/* checks to see if ea_address is invalid */
> +bool pci_ea_invalid_addr(uint64_t addr);
> +
> +/* Initialize the ea capability descriptor */
> +int pci_ea_cap_init(PCIDevice *dev);
> +
> +#endif /* QEMU_PCI_EA_REG_H */
> diff --git a/include/hw/pci/pci_regs.h b/include/hw/pci/pci_regs.h
> index 56a404b..746ba1a 100644
> --- a/include/hw/pci/pci_regs.h
> +++ b/include/hw/pci/pci_regs.h
> @@ -213,6 +213,7 @@
>  #define  PCI_CAP_ID_MSIX     0x11    /* MSI-X */
>  #define  PCI_CAP_ID_SATA     0x12    /* Serial ATA */
>  #define  PCI_CAP_ID_AF               0x13    /* PCI Advanced Features */
> +#define  PCI_CAP_ID_EA               0x14    /* PCI Enhanced Allocation */
>  #define PCI_CAP_LIST_NEXT    1       /* Next capability in the list */
>  #define PCI_CAP_FLAGS                2       /* Capability defined flags (16 
> bits) */
>  #define PCI_CAP_SIZEOF               4
> @@ -340,6 +341,9 @@
>  #define PCI_AF_STATUS                5
>  #define  PCI_AF_STATUS_TP    0x01
>  
> +/* PCI Enhanced Allocation registers */
> +#define PCI_EA_NUM_ENTRIES_OFFSET 2
> +
>  /* PCI-X registers */
>  
>  #define PCI_X_CMD            2       /* Modes & Features */
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index cde3314..70a9f3f 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -48,6 +48,7 @@ typedef struct PcGuestInfo PcGuestInfo;
>  typedef struct PCIBridge PCIBridge;
>  typedef struct PCIBus PCIBus;
>  typedef struct PCIDevice PCIDevice;
> +typedef struct PCIEADevice PCIEADevice;
>  typedef struct PCIEAERErr PCIEAERErr;
>  typedef struct PCIEAERLog PCIEAERLog;
>  typedef struct PCIEAERMsg PCIEAERMsg;
> -- 
> 1.9.1