xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Jan Beulich <JBeulich@suse.com>, xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PVH]: Help: msi.c
Date: Fri, 21 Dec 2012 16:01:53 -0500	[thread overview]
Message-ID: <20121221210153.GA32115@phenom.dumpdata.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1212201145030.17523@kaball.uk.xensource.com>

> > > > > pci_enable_msix -> msix_capability_init -> msix_program_entries
> > > > > 
> > > > > Unfortunately msix_program_entries is called few lines after
> > > > > arch_setup_msi_irqs, where we call PHYSDEVOP_map_pirq to map the MSI as
> > > > > a pirq.
> > > > > However after that is done, all the masking/unmask is done via irq_mask
> > > > > that we handle properly masking/unmasking the corresponding event
> > > > > channels.
> > > > > 
> > > > > 
> > > > > Possible solutions on top of my head:
> > > > 
> > > > There is also the potential to piggyback on Joerg's patches
> > > > that introduce a new x86_msi_ops: compose_msi_msg.
> > > > 
> > > > See here: https://lkml.org/lkml/2012/8/20/432
> > > > (I think there was also a more recent one posted at some point).
> > > 
> > > Given that dom0 should never write to the MSI-X table, introducing a new
> > 
> > How does this work with QEMU setting up MSI and MSI-X on behalf of
> > guests? Or is that actually handled by Xen hypervisor?
> 
> In the case of HVM guests, QEMU emulates the PCI config space and the
> table, so it is OK for the guest to write to it.
> 
> 
> > > msi_ops that replaces msix_program_entries (or at least the part of
> > > msix_program_entries that masks all the entried) is the only solution
> > > left.
> > 
> > so this one (__msix_mask_irq):
> > 
> >          mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> >  198         if (flag)
> >  199                 mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
> >  200         writel(mask_bits, desc->mask_base + offset);
> > 
> 
> Yes, that's the one. Once could argue that __msix_mask_irq should call
> mask_irq rather than writing to the table directly.

You mean 'irq_mask ' ? Not really - that is within the IOAPIC domain.

To be more generic it should encompass then also the other usages -
that is the 'readl' and 'writel' users.

My understading of the reason we have been fortunate enough to have this
working right now is b/c the hypercall we do beforehand writes the
'pirq' in the MSI-X BAR and that is later what the Linux kernel
does (by doing readl) -  and we end up re-writing that value
by the Linux kernel.

The other thing we can do and entirely bypass the msi.c writes is
xen_initdom_setup_msi_irqs make the desc->mask_base point to
somewhere safe. Meaning point to an page we allocate when
we setup the IRQs and we fill it with whatever we want
(which I guess would be the pirq values we just got).

  reply	other threads:[~2012-12-21 21:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-12-08  1:46 [PVH]: Help: msi.c Mukesh Rathor
2012-12-10  9:43 ` Jan Beulich
2012-12-11  2:43   ` Mukesh Rathor
2012-12-11 12:10     ` Stefano Stabellini
2012-12-13  1:15       ` Mukesh Rathor
2012-12-13  1:43         ` Mukesh Rathor
2012-12-13 10:42           ` Jan Beulich
2012-12-13 12:19             ` Stefano Stabellini
2012-12-13 13:39               ` Jan Beulich
2012-12-13 14:25                 ` Stefano Stabellini
2012-12-14 20:08                   ` Mukesh Rathor
2012-12-17 12:42                     ` Stefano Stabellini
2012-12-17 12:57                       ` Jan Beulich
2012-12-17 14:16                         ` Stefano Stabellini
2012-12-17 15:28                           ` Jan Beulich
2012-12-18 21:16                   ` Konrad Rzeszutek Wilk
2012-12-19 11:19                     ` Stefano Stabellini
2012-12-19 15:37                       ` Konrad Rzeszutek Wilk
2012-12-20 11:49                         ` Stefano Stabellini
2012-12-21 21:01                           ` Konrad Rzeszutek Wilk [this message]
2013-01-04 16:57                             ` Stefano Stabellini
2013-01-08 19:49                               ` Konrad Rzeszutek Wilk

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=20121221210153.GA32115@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xen.org \
    /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 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).