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.
>> +static void msix_mask_all(void __iomem *base, int tsize)
>> +{
>> + u32 ctrl = PCI_MSIX_ENTRY_CTRL_MASKBIT;
>> + int i;
>>
>> - msix_mask_irq(entry, 1);
>> - }
>> + for (i = 0; i < tsize; i++, base += PCI_MSIX_ENTRY_SIZE)
>> + writel(ctrl, base + PCI_MSIX_ENTRY_VECTOR_CTRL);
>
> shouldn't we initialize entry->masked here?
How so?
>> + /* Ensure that all table entries are masked. */
>> + msix_mask_all(base, tsize);
>> +
>> ret = msix_setup_entries(dev, base, entries, nvec, affd);
It's invoked before the msi descriptors are set up. We can of course
setup the descriptors first and then do the masking, but notice that
@nvec is not necessarily the same as the table size.
Thanks,
tglx
