Re: [Xen-devel] [PATCH v2 2/2] ns16550: enable use of PCI MSI

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Dec 03, 2018 at 01:40:06AM -0700, Jan Beulich wrote:
> >>> On 30.11.18 at 17:33, <roger.pau@xxxxxxxxxx> wrote:
> > On Mon, Oct 01, 2018 at 10:26:05AM -0600, Jan Beulich wrote:
> >> --- a/xen/arch/x86/msi.c
> >> +++ b/xen/arch/x86/msi.c
> >> @@ -742,6 +742,16 @@ static int msi_capability_init(struct pc
> >>  
> >>      *desc = entry;
> >>      /* Restore the original MSI enabled bits  */
> >> +    if ( !hardware_domain )
> >> +    {
> >> +        /*
> >> +         * ..., except for internal requests (before Dom0 starts), in 
> >> which
> >> +         * case we rather need to behave "normally", i.e. not follow the 
> >> split
> >> +         * brain model where Dom0 actually enables MSI (and disables 
> >> INTx).
> >> +         */
> >> +        pci_intx(dev, 0);
> >> +        control |= PCI_MSI_FLAGS_ENABLE;
> > 
> > Sorry for the split reply, I've been wondering about the MSI enabling
> > and INTX disabling done here. Xen already owns other PCI devices (AMD
> > IOMMU for example, see set_iommu_interrupt_handler) that use MSI, yet
> > they seem to manage to work without this by doing a manual MSI enable
> > (and I cannot figure out where the INTX disable is done).
> 
> That's because IOMMUs don't normally have a means to signal
> interrupts via a pin. Hence there's nothing to disable.
> 
> > Shouldn't Xen have a more uniform way of dealing with MSI interrupt
> > setup for such devices?
> 
> Perhaps, but the crude way of setting up IOMMU interrupts was
> invented by the CPU vendor engineers; on the AMD side later I
> then re-worked this to re-use at least some of the generic MSI
> code we have.
> 
> > And doesn't your change here imply that some code from the current
> > internal MSI users should be dropped? There's a call to
> > __msi_set_enable in the AMD IOMMU code (amd_iommu_msi_enable) that I
> > guess can be dropped?
> 
> Quite possible (I admit I didn't check in detail), but here I didn't
> want to fiddle with unrelated code.

Ack, I guess it's something to add to the list of nice-to-have.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel