On 1/6/26 02:05, Creeley, Brett wrote:
> On 12/23/2025 7:10 PM, Vivian Wang wrote:
>> Caution: This message originated from an External Source. Use proper caution
>> when opening attachments, clicking links, or responding.
>>
>>
>> Some PCI devices have PCI_MSI_FLAGS_64BIT in the MSI capability, but
>> implement less than 64 address bits. This breaks on platforms where such
>> a device is assigned an MSI address higher than what's reachable.
>>
>> Currently, we deal with this with a single no_64bit_msi flag, and
>> (notably on powerpc) use 32-bit MSI address for these devices. However,
>> on some platforms the MSI doorbell address is above 32-bit but within
>> device ability.
>>
>> As a first step, conservatively generalize the single-bit flag
>> no_64bit_msi into msi_addr_mask. (The name msi_addr_mask is chosen to
>> avoid confusion with msi_mask.)
>>
>> The translation is essentially:
>>
>> - no_64bit_msi = 1 -> msi_addr_mask = DMA_BIT_MASK(32)
>> - no_64bit_msi = 0 -> msi_addr_mask = DMA_BIT_MASK(64)
>> - if (no_64bit_msi) -> if (msi_addr_mask < DMA_BIT_MASK(64))
>>
>> Since no values other than DMA_BIT_MASK(32) and DMA_BIT_MASK(64) is
>> used, no functional change is intended. Future patches that make use of
>> intermediate values of msi_addr_mask will follow, allowing devices that
>> cannot use full 64-bit addresses for MSI to work on platforms with MSI
>> doorbell above 32-bit address space.
>>
>> Signed-off-by: Vivian Wang <[email protected]>
>>
>> ---
>>
>> checkpatch complains about the comment include/linux/pci.h, which I have
>> formatted similarly with other comments in the vicinity.
>> ---
>> arch/powerpc/platforms/powernv/pci-ioda.c | 2 +-
>> arch/powerpc/platforms/pseries/msi.c | 4 ++--
>> drivers/gpu/drm/radeon/radeon_irq_kms.c | 2 +-
>> drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 2 +-
>> drivers/pci/msi/msi.c | 2 +-
>> drivers/pci/msi/pcidev_msi.c | 2 +-
>> drivers/pci/probe.c | 7 +++++++
>> include/linux/pci.h | 8 +++++++-
>> sound/hda/controllers/intel.c | 2 +-
>> 9 files changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
>> b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index b0c1d9d16fb5..1c78fdfb7b03 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -1666,7 +1666,7 @@ static int __pnv_pci_ioda_msi_setup(struct pnv_phb
>> *phb, struct pci_dev *dev,
>> return -ENXIO;
>>
>> /* Force 32-bit MSI on some broken devices */
>> - if (dev->no_64bit_msi)
>> + if (dev->msi_addr_mask < DMA_BIT_MASK(64))
>> is_64 = 0;
>>
>> /* Assign XIVE to PE */
>> diff --git a/arch/powerpc/platforms/pseries/msi.c
>> b/arch/powerpc/platforms/pseries/msi.c
>> index a82aaa786e9e..7473c7ca1db0 100644
>> --- a/arch/powerpc/platforms/pseries/msi.c
>> +++ b/arch/powerpc/platforms/pseries/msi.c
>> @@ -383,7 +383,7 @@ static int rtas_prepare_msi_irqs(struct pci_dev *pdev,
>> int nvec_in, int type,
>> */
>> again:
>> if (type == PCI_CAP_ID_MSI) {
>> - if (pdev->no_64bit_msi) {
>> + if (pdev->msi_addr_mask < DMA_BIT_MASK(64)) {
>> rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSI_FN,
>> nvec);
>> if (rc < 0) {
>> /*
>> @@ -409,7 +409,7 @@ static int rtas_prepare_msi_irqs(struct pci_dev *pdev,
>> int nvec_in, int type,
>> if (use_32bit_msi_hack && rc > 0)
>> rtas_hack_32bit_msi_gen2(pdev);
>> } else {
>> - if (pdev->no_64bit_msi)
>> + if (pdev->msi_addr_mask < DMA_BIT_MASK(64))
>> rc = rtas_change_msi(pdn, RTAS_CHANGE_32MSIX_FN,
>> nvec);
>> else
>> rc = rtas_change_msi(pdn, RTAS_CHANGE_MSIX_FN,
>> nvec);
>> diff --git a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> index 9961251b44ba..d550554a6f3f 100644
>> --- a/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> +++ b/drivers/gpu/drm/radeon/radeon_irq_kms.c
>> @@ -252,7 +252,7 @@ static bool radeon_msi_ok(struct radeon_device *rdev)
>> */
>> if (rdev->family < CHIP_BONAIRE) {
>> dev_info(rdev->dev, "radeon: MSI limited to 32-bit\n");
>> - rdev->pdev->no_64bit_msi = 1;
>> + rdev->pdev->msi_addr_mask = DMA_BIT_MASK(32);
>> }
>>
>> /* force MSI on */
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> index 70d86c5f52fb..0671deae9a28 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> @@ -331,7 +331,7 @@ static int ionic_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>>
>> #ifdef CONFIG_PPC64
>> /* Ensure MSI/MSI-X interrupts lie within addressable physical
>> memory */
>> - pdev->no_64bit_msi = 1;
>> + pdev->msi_addr_mask = DMA_BIT_MASK(32);
> I know this is just an intermediate commit in the series, but does this
> retain the original intent?
I do believe so, yes. The no_64bit_msi bit's meaning is the negation of
this bit found in the MSI capability:
#define PCI_MSI_FLAGS_64BIT 0x0080 /* 64-bit addresses allowed */
PCI_MSI_FLAGS_64BIT is set if this function handles PCI_MSI_ADDRESS_HI
and cleared if doesn't handle PCI_MSI_ADDRESS_HI. So with "no 64bit",
only PCI_MSI_ADDRESS_LO is usable, and MSI is limited to 32 bits. See
also old handling here:
>> diff --git a/drivers/pci/msi/pcidev_msi.c b/drivers/pci/msi/pcidev_msi.c
>> index 5520aff53b56..0b0346813092 100644
>> --- a/drivers/pci/msi/pcidev_msi.c
>> +++ b/drivers/pci/msi/pcidev_msi.c
>> @@ -24,7 +24,7 @@ void pci_msi_init(struct pci_dev *dev)
>> }
>>
>> if (!(ctrl & PCI_MSI_FLAGS_64BIT))
>> - dev->no_64bit_msi = 1;
>> + dev->msi_addr_mask = DMA_BIT_MASK(32);
>> }
>>
>> void pci_msix_init(struct pci_dev *dev)
... and the old definition of the flag here, where the comment
explicitly says no_64bit_msi means 32-bit:
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 41183aed8f5d..a2bff57176a3 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>>
>> [...]
>>
>> @@ -441,7 +448,6 @@ struct pci_dev {
>>
>> unsigned int is_busmaster:1; /* Is busmaster */
>> unsigned int no_msi:1; /* May not use MSI */
>> - unsigned int no_64bit_msi:1; /* May only use 32-bit MSIs
>> */
>> unsigned int block_cfg_access:1; /* Config space access
>> blocked */
>> unsigned int broken_parity_status:1; /* Generates false positive
>> parity */
>> unsigned int irq_reroute_variant:2; /* Needs IRQ rerouting
>> variant */
Vivian "dramforever" Wang
