xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 1/4] x86/vMSI-X: defer intercept handler registration
Date: Fri, 17 Jun 2016 12:13:58 -0400	[thread overview]
Message-ID: <20160617161358.GM1340@char.us.oracle.com> (raw)
In-Reply-To: <5758314A02000078000F30A2@prv-mh.provo.novell.com>

On Wed, Jun 08, 2016 at 06:52:58AM -0600, Jan Beulich wrote:
> There's no point in registering the internal MSI-X table intercept
> functions on all domains - it is sufficient to do so once a domain gets
> an MSI-X capable device assigned.

I think this will break on SR-IOV devices that are created and we
had not setup MCFG.

There is this Intel board (which I have) where the MCFG is setup
only via ACPI and in the past we had issues - where we never
detect that the VF had MSI-X - b/c we could not access the
configuration registers past 0xff.

I am pretty sure that Linux now uploads the MCFG data during
bootup from the ACPI AML code, but earlier kernels may not.

And that would mean the device would try to use MSI-X while
code would be !pdev->msix.

Hmm, thought I wonder - with all those improvements of capturing
the MSI-X and validating it in the hypervisor - would this
even matter?

Let me stash an 82576 card in that box and see what happens with
Xen 4.7.

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -644,8 +644,6 @@ int hvm_domain_initialise(struct domain
>  
>      rtc_init(d);
>  
> -    msixtbl_init(d);
> -
>      register_portio_handler(d, 0xe9, 1, hvm_print_line);
>  
>      if ( hvm_tsc_scaling_supported )
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -553,7 +553,8 @@ found:
>  
>  void msixtbl_init(struct domain *d)
>  {
> -    if ( !has_vlapic(d) )
> +    if ( !has_hvm_container_domain(d) || !has_vlapic(d) ||
> +         d->arch.hvm_domain.msixtbl_list.next )
>          return;
>  
>      INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list);
> @@ -567,7 +568,7 @@ void msixtbl_pt_cleanup(struct domain *d
>      struct msixtbl_entry *entry, *temp;
>      unsigned long flags;
>  
> -    if ( !has_vlapic(d) )
> +    if ( !d->arch.hvm_domain.msixtbl_list.next )
>          return;
>  
>      /* msixtbl_list_lock must be acquired with irq_disabled for check_lock() */
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1380,6 +1380,9 @@ static int assign_device(struct domain *
>          goto done;
>      }
>  
> +    if ( pdev->msix )
> +        msixtbl_init(d);
> +
>      pdev->fault.count = 0;
>  
>      if ( (rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), flag)) )
> 
> 
> 

> x86/vMSI-X: defer intercept handler registration
> 
> There's no point in registering the internal MSI-X table intercept
> functions on all domains - it is sufficient to do so once a domain gets
> an MSI-X capable device assigned.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -644,8 +644,6 @@ int hvm_domain_initialise(struct domain
>  
>      rtc_init(d);
>  
> -    msixtbl_init(d);
> -
>      register_portio_handler(d, 0xe9, 1, hvm_print_line);
>  
>      if ( hvm_tsc_scaling_supported )
> --- a/xen/arch/x86/hvm/vmsi.c
> +++ b/xen/arch/x86/hvm/vmsi.c
> @@ -553,7 +553,8 @@ found:
>  
>  void msixtbl_init(struct domain *d)
>  {
> -    if ( !has_vlapic(d) )
> +    if ( !has_hvm_container_domain(d) || !has_vlapic(d) ||
> +         d->arch.hvm_domain.msixtbl_list.next )
>          return;
>  
>      INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list);
> @@ -567,7 +568,7 @@ void msixtbl_pt_cleanup(struct domain *d
>      struct msixtbl_entry *entry, *temp;
>      unsigned long flags;
>  
> -    if ( !has_vlapic(d) )
> +    if ( !d->arch.hvm_domain.msixtbl_list.next )
>          return;
>  
>      /* msixtbl_list_lock must be acquired with irq_disabled for check_lock() */
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1380,6 +1380,9 @@ static int assign_device(struct domain *
>          goto done;
>      }
>  
> +    if ( pdev->msix )
> +        msixtbl_init(d);
> +
>      pdev->fault.count = 0;
>  
>      if ( (rc = hd->platform_ops->assign_device(d, devfn, pci_to_dev(pdev), flag)) )

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-06-17 16:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-08 12:48 [PATCH 0/4] x86/vMSI-X: misc improvements Jan Beulich
2016-06-08 12:52 ` [PATCH 1/4] x86/vMSI-X: defer intercept handler registration Jan Beulich
2016-06-17 16:13   ` Konrad Rzeszutek Wilk [this message]
2016-06-17 16:38     ` Jan Beulich
2016-06-21 17:11   ` Andrew Cooper
2016-06-08 12:53 ` [PATCH 2/4] x86/vMSI-X: drop list lock Jan Beulich
2016-06-21 17:26   ` Andrew Cooper
2016-06-08 12:54 ` [PATCH 3/4] x86/vMSI-X: drop pci_msix_get_table_len() Jan Beulich
2016-06-21 17:27   ` Andrew Cooper
2016-06-08 12:54 ` [PATCH 4/4] x86/vMSI-X: use generic intercept handler in place of MMIO one Jan Beulich
2016-06-13  8:36   ` Paul Durrant
2016-06-21 17:33   ` Andrew Cooper
2016-06-17  8:20 ` Ping: [PATCH 0/4] x86/vMSI-X: misc improvements Jan Beulich

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=20160617161358.GM1340@char.us.oracle.com \
    --to=konrad.wilk@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    --subject='Re: [PATCH 1/4] x86/vMSI-X: defer intercept handler registration' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).