Re: [patch 2/8] PCI/MSI: Mask all unused MSI-X entries

Marc Zyngier
2021-07-22T13:46:30Z

On Wed, 21 Jul 2021 23:57:55 +0100,
Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Ashok,
>
> On Wed, Jul 21 2021 at 15:23, Ashok Raj wrote:
> > On Wed, Jul 21, 2021 at 09:11:28PM +0200, Thomas Gleixner wrote:
> >>
> >> + addr = pci_msix_desc_addr(entry);
> >> + if (addr)
> >> + entry->masked = readl(addr + PCI_MSIX_ENTRY_VECTOR_CTRL);
> >
> > Silly question:
> > Do we have to read what the HW has to set this entry->masked? Shouldn't
> > this be all masked before we start the setup?
>
> msix_mask_all() is invoked before the msi descriptors are
> allocated. msi_desc::masked is actually a misnomer because it's not like
> the name suggests a boolean representing the masked state. It's caching
> the content of the PCI_MSIX_ENTRY_VECTOR_CTRL part of the corresponding
> table entry. Right now this is just using bit 0 (the mask bit), but is
> that true forever? So we actually should rename that member to
> vector_ctrl or such.

To follow-up with this forward looking statement, should we only keep
bit 0 when reading PCI_MSIX_ENTRY_VECTOR_CTRL? I.e.:

entry->masked = (readl(addr + PCI_MSIX_ENTRY_VECTOR_CTRL) &
PCI_MSIX_ENTRY_CTRL_MASKBIT);

Or do we want to cache the whole register? In which case I'm all for
the suggesting renaming (though 'masked' is shared with the old-school
multi-MSI).

Otherwise:

Reviewed-by: Marc Zyngier <maz@kernel.org>

Thanks,

M.

--
Without deviation from the norm, progress is not possible.