On 05/09/24 6:33 PM, Michael Ellerman wrote:
Narayana Murty N <nnmli...@linux.ibm.com> writes:
VFIO_EEH_PE_INJECT_ERR ioctl is currently failing on pseries
due to missing implementation of err_inject eeh_ops for pseries.
This patch implements pseries_eeh_err_inject in eeh_ops/pseries
eeh_ops. Implements support for injecting MMIO load/store error
for testing from user space.

The check on PCI error type code is moved to platform code, since
the eeh_pe_inject_err can be allowed to more error types depending
on platform requirement.

Signed-off-by: Narayana Murty N <nnmli...@linux.ibm.com>
---

Testing:
========
vfio-test [1] by Alex Willamson, was forked and updated to add
support inject error on pSeries guest and used to test this
patch[2].

References:
===========
[1] https://github.com/awilliam/tests
[2] https://github.com/nnmwebmin/vfio-ppc-tests/tree/vfio-ppc-ex

================
Changelog:
V1:https://lore.kernel.org/all/20240822082713.529982-1-nnmli...@linux.ibm.com/
- Resolved build issues for ppc64|le_defconfig by moving the
pseries_eeh_err_inject() definition outside of the CONFIG_PCI_IOV
code block.
- New eeh_pe_inject_mmio_error wrapper function added to avoid
CONFIG_EEH is not set.
I don't see why that's necessary?

It's only called from eeh_pseries.c, which is only built for
PPC_PSERIES, and when PPC_PSERIES=y, EEH is always enabled.

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 91a9fd53254f..8da6b047a4fe 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -308,7 +308,7 @@ int eeh_pe_reset(struct eeh_pe *pe, int option, bool 
include_passed);
  int eeh_pe_configure(struct eeh_pe *pe);
  int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
                      unsigned long addr, unsigned long mask);
-
+int eeh_pe_inject_mmio_error(struct pci_dev *pdev);
  /**
   * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
   *
@@ -338,6 +338,10 @@ static inline int eeh_check_failure(const volatile void 
__iomem *token)
        return 0;
  }
+static inline int eeh_pe_inject_mmio_error(struct pci_dev *pdev)
+{
+       return -ENXIO;
+}
  #define eeh_dev_check_failure(x) (0)
static inline void eeh_addr_cache_init(void) { }
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index d03f17987fca..49ab11a287a3 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1537,10 +1537,6 @@ int eeh_pe_inject_err(struct eeh_pe *pe, int type, int 
func,
        if (!eeh_ops || !eeh_ops->err_inject)
                return -ENOENT;
- /* Check on PCI error type */
-       if (type != EEH_ERR_TYPE_32 && type != EEH_ERR_TYPE_64)
-               return -EINVAL;
-
The change log should mention why it's OK to remove these checks. You
add the same checks in pseries_eeh_err_inject(), but what about
pnv_eeh_err_inject() ?

It is OK AFAICS, because pnv_eeh_err_inject() already contains
equivalent checks, but you should spell that out.

cheers

yes mpe. I do agree, your comments are addressed in V3 posted

here https://lore.kernel.org/all/20240909140220.529333-1-nnmli...@linux.ibm.com/

regards,

Narayana Murty.


Reply via email to