Re: [PATCH v2] PCI/MSI: Re-add checks for skip masking MSI-X on Xen PV

[Josef Johansson <josef@xxxxxxxxxxx>]

On 10/27/21 08:24, David Woodhouse wrote:
> On Mon, 2021-10-25 at 21:21 +0200, Josef Johansson wrote:
>> +       if (!(pci_msi_ignore_mask || entry->msi_attrib.is_virtual))
> Is it just me, or is that a lot easier to read if you write it as the
> tautologically-identical (!pci_msi_ignore_mask && !entry->…is_virtual)?
Sure, the less parentheses the better.
>
>> @@ -546,7 +548,8 @@ static int msi_capability_init(struct pci_dev *dev, int 
>> nvec,
>>                 return -ENOMEM;
>>         /* All MSIs are unmasked by default; mask them all *
>> -       pci_msi_mask(entry, msi_multi_mask(entry))
>> +       if (!pci_msi_ignore_mask)
>> +               pci_msi_mask(entry, msi_multi_mask(entry));
>>
>>         list_add_tail(&entry->list, dev_to_msi_list(&dev->dev));
>
> Hm, I thought that older kernels *did* do this part (or at least the
> later ones in pci_msi*_hutdown). I was watching it when I did the Xen
> hosting implementation I mentioned before; even a hack to unmask them
> all when the VM was started, wasn't working because the guest would
> *mask* all MSI-X, just never unmask them again.
When you're saying *did* here, do you mean that they mask even though
pci_msi_ignore_mask = 0?

As I was looking through pre Thomas' changes and post, it seems that we
did indeed
check for pci_msi_ignore_mask in msi_capability_init.
>
> I wonder if we should rename 'pci_msi_ignore_mask' to something with
> Xen in its name because Xen is the only user of this abomination (which
> fundamentally seems to require that the virtual hardware use MSI
> entries even while they're masked, so hopefully nobody else would
> *ever* do such a thing), and the required behaviour is very Xen-
> specific.
Second that, i.e. pci_msi_masked_by_xen.