Re: [PATCH v4 2/5] vfio/spapr_tce: Normalize EEH IOA error injection addresses
From: Narayana Murty N
Date: Mon Oct 19 2026 - 05:45:51 EST
Hi Sourabh,
On 23/09/26 5:10 PM, Sourabh Jain wrote:
Good point.
On 31/08/26 12:24, Narayana Murty N wrote:
VFIO_EEH_PE_INJECT_ERR receives a target address from userspace. For
userspace such as QEMU, this address may be derived from the host Linux
BAR resource. The platform EEH backends, however, expect the PCI/IOA
bus address used by firmware error-injection interfaces.
Normalize IOA/MMIO error-injection addresses in the sPAPR VFIO EEH
ioctl path before dispatching to eeh_pe_inject_err(). If the supplied
address is already a PCI bus BAR address it is left unchanged. If it is
a Linux resource address, translate it to the corresponding PCI bus
address using the BAR-relative offset.
Keep the helper local to VFIO so the address semantics of other
in-kernel EEH callers are unchanged. This provides common handling for
both pseries and PowerNV backends.
Signed-off-by: Narayana Murty N <nnmlinux@xxxxxxxxxxxxx>
---
drivers/vfio/vfio_iommu_spapr_tce.c | 90 +++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/ vfio_iommu_spapr_tce.c
index 1c0eec228cc6..8a6cc856d7da 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -774,6 +774,85 @@ static long tce_iommu_create_default_window(struct tce_container *container)
return ret;
}
+static bool vfio_spapr_eeh_err_needs_addr_normalize(unsigned int type)
+{
+ if (type != EEH_ERR_TYPE_32 && type != EEH_ERR_TYPE_64)
+ return false;
+
+ return true;
+}
Do we really need this function? It is only used once.
+
+static int vfio_spapr_eeh_normalize_addr(struct eeh_pe *pe,
+ unsigned long addr,
+ unsigned long *normalized)
+{
+ struct pci_bus_region region;
+ struct eeh_dev *edev, *tmp;
+ struct pci_dev *pdev;
+ struct resource *res;
+ resource_size_t pci_start, pci_len;
+ resource_size_t res_start, res_len;
+ resource_size_t offset;
+ int bar;
+
+ if (!pe || !normalized)
+ return -EINVAL;
+
+ if (!addr) {
+ *normalized = addr;
+ return 0;
+ }
This silently bypasses normalization for addr == 0. A comment on
why zero is exempted would be helpful.
In v5 I made the no-address case explicit as addr == 0 && mask == 0, rather than bypassing normalization based on the address alone.
I also added a comment explaining that this represents an injection with no specific IOA target and therefore does not require BAR translation.
pdev->resource[] is not limited to the standard BARs. The lookup intentionally searches only BAR0..BAR5 using PCI_STD_NUM_BARS; ROM and bridge resources are not considered IOA bus-error targets.
+
+ eeh_pe_for_each_dev(pe, edev, tmp) {
+ pdev = eeh_dev_to_pci_dev(edev);
+ if (!pdev)
+ continue;
+
+ for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
+ res = &pdev->resource[bar];
I'm not very familiar with this PCI resource regions, so quick question.
is pdev->resource[] limited to PCI_STD_NUM_BARS?
Also doesn't this need locking around the traversal? This dereferences
pdev->resource with no lock held, what stops hot-removal or BAR
reassignment racing this?
For the lifetime issue, agreed. In v5 I hold pci_rescan_remove_lock while walking the PE devices and accessing their BAR resources so device removal cannot race the traversal.
Agreed.
+
+ if (!resource_size(res))
+ continue;
+
+ if (!(res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
+ continue;
+
+ pcibios_resource_to_bus(pdev->bus, ®ion, res);
+
+ pci_start = region.start;
+ pci_len = resource_size(res);
+
+ /*
+ * Case 1: userspace already supplied PCI/IOA
+ * bus address.
+ */
+ if ((resource_size_t)addr >= pci_start &&
+ ((resource_size_t)addr - pci_start) < pci_len) {
+ *normalized = addr;
+ return 0;
+ }
+
+ /*
+ * Case 2: userspace supplied Linux resource/CPU
+ * address Convert it back to PCI/IOA bus address
+ * before calling the platform EEH backend.
+ */
+ res_start = res->start;
+ res_len = resource_size(res);
+
+ if ((resource_size_t)addr >= res_start &&
+ ((resource_size_t)addr - res_start) < res_len) {
+ offset = (resource_size_t)addr - res_start;
+ *normalized = region.start + offset;
+
+ return 0;
+ }
+ }
+ }
+
+ return -EINVAL;
+}
+
static long vfio_spapr_ioctl_eeh_pe_op(struct iommu_group *group,
unsigned long arg)
{
@@ -818,6 +897,17 @@ static long vfio_spapr_ioctl_eeh_pe_op(struct iommu_group *group,
if (copy_from_user(&op, (void __user *)arg, minsz))
return -EFAULT;
+ if (vfio_spapr_eeh_err_needs_addr_normalize(op.err.type)) {
At this point eeh_pe_inject_err is still using eeh_pe_inject_mmio_error, so why to normalize the address in this patch itself? Isn't this is a good candidate for 4/5 patch?
I reordered the series in v5 so that the pSeries RTAS error-injection implementation is added before the VFIO address-normalization patch.
The normalization patch is now 4/5, after the pSeries backend has switched from the legacy MMIO injection path to RTAS.
Agreed. I added a pr_debug() in v5 when the supplied address cannot be matched against any PCI bus BAR of the PE.+ unsigned long normalized;
+ long ret;
+
+ ret = vfio_spapr_eeh_normalize_addr(pe, op.err.addr, &normalized);
+ if (ret)
+ return ret;
We should have error/debug log to tell eeh is not going through.
Thanks,
Narayana
+
+ op.err.addr = normalized;
+ }
+
return eeh_pe_inject_err(pe, op.err.type, op.err.func,
op.err.addr, op.err.mask);
default: