[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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.