[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 2/2] ns16550: enable use of PCI MSI
>>> 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. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |