Thread (13 messages) 13 messages, 3 authors, 2026-01-08

Re: [PATCH 1/5] PCI/MSI: Conservatively generalize no_64bit_msi into msi_addr_mask

From: Vivian Wang <hidden>
Date: 2026-01-06 07:42:54
Also in: amd-gfx, dri-devel, linux-pci, linux-sound, linuxppc-dev, lkml

On 1/6/26 02:05, Creeley, Brett wrote:
On 12/23/2025 7:10 PM, Vivian Wang wrote:
quoted
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 <redacted>

---

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:
quoted
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:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help