Re: [PATCH] PCI/MSI: Skip address mask check for MSI-X entries

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] PCI/MSI: skipping MSI-X in msi_verify_entries() breaks mask
  verification
- [Low] PCI/MSI: wrong commit cited in Fixes trailer for
  msi_verify_entries()
--

commit ad679f69e8ee163ac9de4d91832ae0f1a10a2a14
Author: Yu Yang <yuyang@kylinos.cn>

> PCI/MSI: Skip address mask check for MSI-X entries
>
> The AHCI controller on Zhaoxin ZX-200 platforms supports only 32-bit
> MSI addresses, but its MSI-X capability uses 64-bit message addresses.
> pci_msi_init() therefore sets dev->msi_addr_mask to DMA_BIT_MASK(32)
> based on the MSI capability.
>
> msi_verify_entries() is shared between the MSI and MSI-X setup paths.
> When the irq domain assigns an MSI-X message address above 4GB, the
> 32-bit mask inherited from the MSI capability causes MSI-X setup to
> fail with -EIO:
>
>   ahci 0000:09:0f.0: arch assigned 64-bit MSI address 0x8002a030040
>   above device MSI address mask 0xffffffff.
>   ...
>   ahci: probe of 0000:09:0f.0 failed with error -22.
>
> MSI-X table entries have a 64-bit message address field, so the 32-bit
> limit derived from the MSI capability does not apply to them.
> Platforms that use msi_addr_mask to constrain MSI-X allocation, e.g.
> powerpc pseries/powernv and ionic on PPC64, already allocate addresses
> that satisfy the mask, so the verification is redundant for MSI-X.
>
> Skip the address mask check for MSI-X descriptors.
>
> Fixes: f144d1496b47 ("PCI/MSI: Add device flag indicating that 64-bit MSIs don't work")
>
> Signed-off-by: Yu Yang <yuyang@kylinos.cn>

[Severity: Low]
Should the Fixes tag reference 2053230af11d ("PCI/MSI: Set device flag
indicating only 32-bit MSI support") instead?  Commit f144d1496b47
("PCI/MSI: Add device flag indicating that 64-bit MSIs don't work") only
added the opt-in flag for quirks and drivers; 2053230af11d ("PCI/MSI: Set
device flag indicating only 32-bit MSI support") is what made
pci_msi_init() restrict the device based on PCI_MSI_FLAGS_64BIT in the MSI
capability.

> diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
> index 8a0133a6a2863..7f34bccc8248b 100644
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -338,6 +338,13 @@ static int msi_verify_entries(struct pci_dev *dev)
>  		return 0;
>  
>  	msi_for_each_desc(entry, &dev->dev, MSI_DESC_ALL) {
> +		/*
> +		 * MSI-X entries always use a 64-bit address, so the address
> +		 * limit for MSI capability does not apply for MSI-X.
> +		 */
> +		if (entry->pci.msi_attrib.is_msix)
> +			continue;
> +
>  		address = (u64)entry->msg.address_hi << 32 | entry->msg.address_lo;
>  		if (address & ~dev->msi_addr_mask) {
>  			pci_err(dev, "arch assigned 64-bit MSI address %#llx above device MSI address mask %#llx\n",

[Severity: Medium]
Does skipping MSI-X descriptors in msi_verify_entries() bypass
dev->msi_addr_mask verification when a driver or quirk explicitly restricts
the device's addressable bits?

Although MSI-X table entries have 64-bit address fields, MSI-X is delivered
as a PCIe Memory Write TLP and is still constrained by the device's DMA
address width.  Drivers such as ionic (which only uses MSI-X) and radeon
set dev->msi_addr_mask to their hardware address limit, and on
architectures that do not consult dev->msi_addr_mask during MSI-X doorbell
allocation (such as x86 and arm64), msi_verify_entries() is the only check
preventing an unreachable doorbell address from being programmed.

Because msi_verify_entries() is only called from __msi_capability_init()
(all entries have is_msix == 0) and __msix_setup_interrupts() (all entries
have is_msix == 1), this check also turns the call in
__msix_setup_interrupts() into a no-op loop.  Meanwhile, pci_msi_init()
still lowers dev->msi_addr_mask to DMA_BIT_MASK(32) based on the MSI
capability, so powerpc's rtas_prepare_msi_irqs() and
__pnv_pci_ioda_msi_setup() will still force 32-bit MSI-X allocation on
devices with 32-bit MSI and 64-bit MSI-X capabilities.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-pci-fix-msix-alloc-failure-v1-1-6370c0b84354@kylinos.cn?part=1