Re: [PVH]: Help: msi.c

[Konrad Rzeszutek Wilk <konrad@kernel.org>]

On Fri, Jan 04, 2013 at 04:57:13PM +0000, Stefano Stabellini wrote:
> On Fri, 21 Dec 2012, Konrad Rzeszutek Wilk wrote:
> > > > > > > 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.
> 
> The concept of IOAPIC domain is a bit blurry to me when it comes to MSI-X.
> But I see what you mean.
> 
> 
> > 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).
> 
> Ah yes, that would work. Even though more code to work around generic
> Linux infrastructure is not ideal.

I concur. I am just thinking of a fail-safe mechanism. The abstraction
of how to do the MSI-X read/write is not yet completly clear in my mind.

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