[PATCH 2/2] Add support for enhanced allocation regions

[PATCH 2/2] Add support for enhanced allocation regions

[Sean O. Stalley]

Append [enhanced] to Regions that contain the BEI flag in sysfs.
Don't tuncate least significant bits of the region size.
ex: a 2000 byte region should display [size=2000] instead of [size=1K]

Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
---
 lib/header.h |  4 ++++
 lib/pci.h    |  2 ++
 lib/sysfs.c  |  5 ++++-
 lspci.c      | 17 ++++++++++++++---
 4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/lib/header.h b/lib/header.h
index b8f7dc1..7b9a803 100644
--- a/lib/header.h
+++ b/lib/header.h
@@ -1235,3 +1235,7 @@
 
 #define PCI_VENDOR_ID_INTEL		0x8086
 #define PCI_VENDOR_ID_COMPAQ		0x0e11
+
+/* taken from <include/linux/ioport.h> */
+
+#define IORESOURCE_PCI_EA_BEI          (1<<5)
diff --git a/lib/pci.h b/lib/pci.h
index 9c1e281..a88e156 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -129,8 +129,10 @@ struct pci_dev {
   int irq;				/* IRQ number */
   pciaddr_t base_addr[6];		/* Base addresses including flags in lower bits */
   pciaddr_t size[6];			/* Region sizes */
+  pciaddr_t flags[6];			/* Region flags */
   pciaddr_t rom_base_addr;		/* Expansion ROM base address */
   pciaddr_t rom_size;			/* Expansion ROM size */
+  pciaddr_t rom_flags;			/* Expansion ROM flags */
   struct pci_cap *first_cap;		/* List of capabilities */
   char *phy_slot;			/* Physical slot */
   char *module_alias;			/* Linux kernel module alias */
diff --git a/lib/sysfs.c b/lib/sysfs.c
index 986ecc9..e900e3a 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -153,14 +153,17 @@ sysfs_get_resources(struct pci_dev *d)
 	size = end - start + 1;
       else
 	size = 0;
-      flags &= PCI_ADDR_FLAG_MASK;
       if (i < 6)
 	{
+	  d->flags[i] = flags;
+	  flags &= PCI_ADDR_FLAG_MASK;
 	  d->base_addr[i] = start | flags;
 	  d->size[i] = size;
 	}
       else
 	{
+	  d->rom_flags = flags;
+	  flags &= PCI_ADDR_FLAG_MASK;
 	  d->rom_base_addr = start | flags;
 	  d->rom_size = size;
 	}
diff --git a/lspci.c b/lspci.c
index fe7b7fe..358bb34 100644
--- a/lspci.c
+++ b/lspci.c
@@ -336,7 +336,7 @@ show_size(pciaddr_t x)
   if (!x)
     return;
   for (i = 0; i < (sizeof(suffix) / sizeof(*suffix) - 1); i++) {
-    if (x < 1024)
+    if (x % 1024)
       break;
     x /= 1024;
   }
@@ -364,7 +364,12 @@ show_bases(struct device *d, int cnt)
 	printf("\tRegion %d: ", i);
       else
 	putchar('\t');
-      if (pos && !flg)			/* Reported by the OS, but not by the device */
+
+      if (p->flags[i] & IORESOURCE_PCI_EA_BEI)
+	{
+	  printf("[enhanced] ");
+	}
+      else if (pos && !flg)	/* Reported by the OS, but not by the device */
 	{
 	  printf("[virtual] ");
 	  flg = pos;
@@ -437,7 +442,13 @@ show_rom(struct device *d, int reg)
   if (!rom && !flg && !len)
     return;
   putchar('\t');
-  if ((rom & PCI_ROM_ADDRESS_MASK) && !(flg & PCI_ROM_ADDRESS_MASK))
+
+
+  if (p->rom_flags & IORESOURCE_PCI_EA_BEI)
+    {
+      printf("[enhanced] ");
+    }
+  else if ((rom & PCI_ROM_ADDRESS_MASK) && !(flg & PCI_ROM_ADDRESS_MASK))
     {
       printf("[virtual] ");
       flg = rom;
-- 
1.9.1

[PATCH 0/2] pci/lspci: Identify Enhanced Allocation (EA) Resources

[Sean O. Stalley]

Identify BAR-equivalent resources that are described by EA entries
with the IORESOURCE_PCI_EA_BEI flag.

lspci cannot distinguish between resources from VF BARs and resources from EA.
This results in EA Resources being incorrectly identified as [virtual].
Adding this flag allows EA resources to be marked more accurately as [enhanced].

Although this patchset only add support for this flag to lspci,
there are other use cases (such as vfio) where knowing a resource
comes from EA would be useful.

[PATCH 1/2] is for the kernel, [PATCH 2/2] is for lspci.


Alex Williamson (1):
  pci: Identify Enhanced Allocation (EA) BAR Equivalent resources

Sean O. Stalley (1):
  Add support for enhanced allocation regions

linux changes:
 drivers/pci/pci.c      | 2 +-
 include/linux/ioport.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

lspci changes:
 lib/header.h |  4 ++++
 lib/pci.h    |  2 ++
 lib/sysfs.c  |  5 ++++-
 lspci.c      | 17 ++++++++++++++---
 4 files changed, 24 insertions(+), 4 deletions(-)


-- 
1.9.1

[PATCH 1/2] pci: Identify Enhanced Allocation (EA) BAR Equivalent resources

[Sean O. Stalley]

From: Alex Williamson <alex.williamson@redhat.com>

We've done a pretty good job of abstracting EA from drivers,
but there are some cases where flaging resources as EA would be
useful. Specifically, EA is flexabable when it comes to resource
alignment & sizing. This can create issues when existing software
makes assumptions about the PCI resources.

For example, lspci assumes any PCI resource exposed by the kernel
that isn't visible in standard configspace comes from a VF BAR.
vfio makes assumptions about alignment and sizing, and runs into
problems when attempting to emulate a BAR Equivalent EA resource.

To facilitate that, a flag indicating whether a PCI resource is a
traditional BAR or BAR equivalent seems much nicer than attempting
to deduce where the resource came from from the size & address fields
(which is impossible if a EA resource happens naturally aligned).

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
---
 drivers/pci/pci.c      | 2 +-
 include/linux/ioport.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d1a7105..8ff678c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2229,7 +2229,7 @@ void pci_pm_init(struct pci_dev *dev)
 
 static unsigned long pci_ea_flags(struct pci_dev *dev, u8 prop)
 {
-	unsigned long flags = IORESOURCE_PCI_FIXED;
+	unsigned long flags = IORESOURCE_PCI_FIXED | IORESOURCE_PCI_EA_BEI;
 
 	switch (prop) {
 	case PCI_EA_P_MEM:
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 24bea08..5acc194 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -105,6 +105,8 @@ struct resource {
 /* PCI control bits.  Shares IORESOURCE_BITS with above PCI ROM.  */
 #define IORESOURCE_PCI_FIXED		(1<<4)	/* Do not move resource */
 
+/* PCI Enhanced Allocation defined BAR equivalent resource */
+#define IORESOURCE_PCI_EA_BEI		(1<<5)
 
 /* helpers to define resources */
 #define DEFINE_RES_NAMED(_start, _size, _name, _flags)			\
-- 
1.9.1

[PATCH 2/2] Add support for enhanced allocation regions

[Sean O. Stalley]

Append [enhanced] to Regions that contain the BEI flag in sysfs.
Don't tuncate least significant bits of the region size.
ex: a 2000 byte region should display [size=2000] instead of [size=1K]

Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
---
 lib/header.h |  4 ++++
 lib/pci.h    |  2 ++
 lib/sysfs.c  |  5 ++++-
 lspci.c      | 17 ++++++++++++++---
 4 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/lib/header.h b/lib/header.h
index b8f7dc1..7b9a803 100644
--- a/lib/header.h
+++ b/lib/header.h
@@ -1235,3 +1235,7 @@
 
 #define PCI_VENDOR_ID_INTEL		0x8086
 #define PCI_VENDOR_ID_COMPAQ		0x0e11
+
+/* taken from <include/linux/ioport.h> */
+
+#define IORESOURCE_PCI_EA_BEI          (1<<5)
diff --git a/lib/pci.h b/lib/pci.h
index 9c1e281..a88e156 100644
--- a/lib/pci.h
+++ b/lib/pci.h
@@ -129,8 +129,10 @@ struct pci_dev {
   int irq;				/* IRQ number */
   pciaddr_t base_addr[6];		/* Base addresses including flags in lower bits */
   pciaddr_t size[6];			/* Region sizes */
+  pciaddr_t flags[6];			/* Region flags */
   pciaddr_t rom_base_addr;		/* Expansion ROM base address */
   pciaddr_t rom_size;			/* Expansion ROM size */
+  pciaddr_t rom_flags;			/* Expansion ROM flags */
   struct pci_cap *first_cap;		/* List of capabilities */
   char *phy_slot;			/* Physical slot */
   char *module_alias;			/* Linux kernel module alias */
diff --git a/lib/sysfs.c b/lib/sysfs.c
index 986ecc9..e900e3a 100644
--- a/lib/sysfs.c
+++ b/lib/sysfs.c
@@ -153,14 +153,17 @@ sysfs_get_resources(struct pci_dev *d)
 	size = end - start + 1;
       else
 	size = 0;
-      flags &= PCI_ADDR_FLAG_MASK;
       if (i < 6)
 	{
+	  d->flags[i] = flags;
+	  flags &= PCI_ADDR_FLAG_MASK;
 	  d->base_addr[i] = start | flags;
 	  d->size[i] = size;
 	}
       else
 	{
+	  d->rom_flags = flags;
+	  flags &= PCI_ADDR_FLAG_MASK;
 	  d->rom_base_addr = start | flags;
 	  d->rom_size = size;
 	}
diff --git a/lspci.c b/lspci.c
index fe7b7fe..358bb34 100644
--- a/lspci.c
+++ b/lspci.c
@@ -336,7 +336,7 @@ show_size(pciaddr_t x)
   if (!x)
     return;
   for (i = 0; i < (sizeof(suffix) / sizeof(*suffix) - 1); i++) {
-    if (x < 1024)
+    if (x % 1024)
       break;
     x /= 1024;
   }
@@ -364,7 +364,12 @@ show_bases(struct device *d, int cnt)
 	printf("\tRegion %d: ", i);
       else
 	putchar('\t');
-      if (pos && !flg)			/* Reported by the OS, but not by the device */
+
+      if (p->flags[i] & IORESOURCE_PCI_EA_BEI)
+	{
+	  printf("[enhanced] ");
+	}
+      else if (pos && !flg)	/* Reported by the OS, but not by the device */
 	{
 	  printf("[virtual] ");
 	  flg = pos;
@@ -437,7 +442,13 @@ show_rom(struct device *d, int reg)
   if (!rom && !flg && !len)
     return;
   putchar('\t');
-  if ((rom & PCI_ROM_ADDRESS_MASK) && !(flg & PCI_ROM_ADDRESS_MASK))
+
+
+  if (p->rom_flags & IORESOURCE_PCI_EA_BEI)
+    {
+      printf("[enhanced] ");
+    }
+  else if ((rom & PCI_ROM_ADDRESS_MASK) && !(flg & PCI_ROM_ADDRESS_MASK))
     {
       printf("[virtual] ");
       flg = rom;
-- 
1.9.1

Re: [PATCH 0/2] pci/lspci: Identify Enhanced Allocation (EA) Resources

[Sean O. Stalley]

Sorry, I forgot to increment the patch version.

This is V2. The only difference from V1 is the commit message on 1/2.
No functional changes.

-Sean

On Fri, Jan 22, 2016 at 02:04:52PM -0800, Sean O. Stalley wrote:
> Identify BAR-equivalent resources that are described by EA entries
> with the IORESOURCE_PCI_EA_BEI flag.
> 
> lspci cannot distinguish between resources from VF BARs and resources from EA.
> This results in EA Resources being incorrectly identified as [virtual].
> Adding this flag allows EA resources to be marked more accurately as [enhanced].
> 
> Although this patchset only add support for this flag to lspci,
> there are other use cases (such as vfio) where knowing a resource
> comes from EA would be useful.
> 
> [PATCH 1/2] is for the kernel, [PATCH 2/2] is for lspci.
> 
> 
> Alex Williamson (1):
>   pci: Identify Enhanced Allocation (EA) BAR Equivalent resources
> 
> Sean O. Stalley (1):
>   Add support for enhanced allocation regions
> 
> linux changes:
>  drivers/pci/pci.c      | 2 +-
>  include/linux/ioport.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> lspci changes:
>  lib/header.h |  4 ++++
>  lib/pci.h    |  2 ++
>  lib/sysfs.c  |  5 ++++-
>  lspci.c      | 17 ++++++++++++++---
>  4 files changed, 24 insertions(+), 4 deletions(-)
> 
> 
> -- 
> 1.9.1
> 

Re: [PATCH 1/2] pci: Identify Enhanced Allocation (EA) BAR Equivalent resources

[Bjorn Helgaas]

On Fri, Jan 22, 2016 at 02:04:53PM -0800, Sean O. Stalley wrote:
> From: Alex Williamson <alex.williamson@redhat.com>
> 
> We've done a pretty good job of abstracting EA from drivers,
> but there are some cases where flaging resources as EA would be
> useful. Specifically, EA is flexabable when it comes to resource

Heh, "flexabable" :)

> alignment & sizing. This can create issues when existing software
> makes assumptions about the PCI resources.
> 
> For example, lspci assumes any PCI resource exposed by the kernel
> that isn't visible in standard configspace comes from a VF BAR.
> vfio makes assumptions about alignment and sizing, and runs into
> problems when attempting to emulate a BAR Equivalent EA resource.
> 
> To facilitate that, a flag indicating whether a PCI resource is a
> traditional BAR or BAR equivalent seems much nicer than attempting
> to deduce where the resource came from from the size & address fields
> (which is impossible if a EA resource happens naturally aligned).
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Sean O. Stalley <sean.stalley@intel.com>
> ---
>  drivers/pci/pci.c      | 2 +-
>  include/linux/ioport.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index d1a7105..8ff678c 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2229,7 +2229,7 @@ void pci_pm_init(struct pci_dev *dev)
>  
>  static unsigned long pci_ea_flags(struct pci_dev *dev, u8 prop)
>  {
> -	unsigned long flags = IORESOURCE_PCI_FIXED;
> +	unsigned long flags = IORESOURCE_PCI_FIXED | IORESOURCE_PCI_EA_BEI;
>  
>  	switch (prop) {
>  	case PCI_EA_P_MEM:
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 24bea08..5acc194 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -105,6 +105,8 @@ struct resource {
>  /* PCI control bits.  Shares IORESOURCE_BITS with above PCI ROM.  */
>  #define IORESOURCE_PCI_FIXED		(1<<4)	/* Do not move resource */
>  
> +/* PCI Enhanced Allocation defined BAR equivalent resource */
> +#define IORESOURCE_PCI_EA_BEI		(1<<5)

Hmmm, I didn't realize until I read the lspci patch that the resource
flags are exposed via sysfs (I think in resource_show()) and are thus
part of the sysfs ABI.  It would be nice to have a comment to that
effect here in ioport.h so we don't move bits and inadvertently break
something.

The changelog should also connect the dots a little bit more, e.g.,
mention the sysfs file affected by this patch, and that this currently
only makes lspci print slightly more correct output.

>  /* helpers to define resources */
>  #define DEFINE_RES_NAMED(_start, _size, _name, _flags)			\
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html