All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: bhelgaas@google.com, linux-pci@vger.kernel.org
Cc: sean.stalley@intel.com, david.daney@cavium.com
Subject: Re: [RFC PATCH] pci: Identify Enhanced Allocation (EA) BAR Equivalent resources
Date: Wed, 20 Jan 2016 13:20:27 -0700	[thread overview]
Message-ID: <1453321227.32741.332.camel@redhat.com> (raw)
In-Reply-To: <20160114172645.23429.9938.stgit@gimli.home>

On Thu, 2016-01-14 at 10:26 -0700, Alex Williamson wrote:
> We've done a pretty good job of abstracting EA from drivers, but there
> are some properties of BAR Equivalent resources that don't really jive
> with traditional PCI BARs.  In particular, natural alignment is only
> encouraged, not required.
> 
> Why does this matter?  There are drivers like vfio-pci that will
> happily gobble up the EA abstraction that's been implemented and
> expose a device using EA to userspace as if those resources are
> traditional BARs.  Pretty cool.  The vfio API is bus agnostic, so it
> doesn't care about alignment.  The problem comes with PCI config space
> emulation where we don't let userspace manipulate the BAR value, but
> we do emulate BAR sizing.  The abstraction kind of falls apart if
> userspace gets garbage when they try to size what appears to be a
> traditional BAR, but is actually a BAR equivalent.
> 
> We could simply round up the size in vfio to make it naturally
> aligned, but then we're imposing artificial sizes to the user and we
> have the discontinuity that BAR size emulation and vfio region size
> reporting don't agree on the size.  I think what we want to do is
> expose EA to the user, reporting traditional BARs with BEIs as
> zero-sized and providing additional regions for the user to access
> each EA region, whether it has a BEI or not.
> 
> To facilitate that, a flag indicating whether a PCI resource is a
> traditional BAR or BAR equivalent seems much nicer than attempting
> to size the BAR ourselves or deducing it through the EA capability.
> 
> Thoughts?
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---


Just to loop back on this, it seems like we do have some support and
use cases beyond what I proposed.  Thanks for the discussion of that.
However, I'm reluctant to post this formally because the change is user
visible, it consumes a limited resource, and I don't know how quickly
vfio-pci is going to be able to make use of this flag.  The vfio-pci
work may not happen until a device appears with poorly sized resources
that has some use case with vfio-pci.  Even then, we may be able to
infer the BEI association without this flag.  So, while I'm not opposed
to this flag, I don't see a need to drive it right now and those that
do have a more immediate need are welcome to take over.  Thanks,

Alex


>  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 314db8c..174c734 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)			\
> 


  parent reply	other threads:[~2016-01-20 20:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 17:26 [RFC PATCH] pci: Identify Enhanced Allocation (EA) BAR Equivalent resources Alex Williamson
2016-01-14 18:34 ` Sean O. Stalley
2016-01-14 19:16   ` Alex Williamson
2016-01-14 20:23     ` Sean O. Stalley
2016-01-14 21:14       ` Alex Williamson
2016-01-14 23:02         ` Sean O. Stalley
2016-01-14 18:54 ` David Daney
2016-01-14 19:20   ` Alex Williamson
2016-01-14 19:27   ` Sean O. Stalley
2016-01-20 20:20 ` Alex Williamson [this message]
2016-01-21 17:48   ` Sean O. Stalley

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=1453321227.32741.332.camel@redhat.com \
    --to=alex.williamson@redhat.com \
    --cc=bhelgaas@google.com \
    --cc=david.daney@cavium.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=sean.stalley@intel.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.