Messages in this thread | | | Date | Thu, 22 Jul 2021 14:46:25 +0100 | From | Marc Zyngier <> | Subject | Re: [patch 2/8] PCI/MSI: Mask all unused MSI-X entries |
| |
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.
| |