Re: [PATCH v4 3/5] powerpc/pseries/eeh: Add RTAS error validation helpers

[Narayana Murty N <nnmlinux@linux.ibm.com>]

Hi Sourabh,

On 23/09/26 5:46 PM, Sourabh Jain wrote:
> 
> On 31/08/26 12:24, Narayana Murty N wrote:
>> Add pseries-specific infrastructure for RTAS-based EEH error injection.
>>
>> Define pr_fmt unconditionally at the top of eeh_pseries.c so all
>> pr_*() calls carry the "EEH: " prefix:
>>
>>    #define pr_fmt(fmt) "EEH: " fmt
> 
> Could you please describe the overall goal of the helpers introduced by
> this patch first, before discussing pr_fmt and the other macros?
> 
Agreed. I reworked the commit message in v5 to start with the purpose of 
the helpers: mapping the generic EEH types, validating the injection 
arguments and target address, and constructing the PAPR ibm,errinjct 
parameter buffer.

The implementation details such as pr_fmt and the RTAS constants follow 
that description.

>>
>> Define RTAS firmware error-type encodings as file-local constants.
>> Only the two IOA bus-error types are supported; other PAPR encodings
>> are not reachable through the generic EEH_ERR_TYPE_* UAPI:
>>
>>    RTAS_ERR_TYPE_IOA_BUS_ERROR     0x07
>>    RTAS_ERR_TYPE_IOA_BUS_ERROR_64  0x0f
>>
>> Add RTAS_ERRINJCT_BUF_SIZE for the ibm,errinjct work buffer size.
>>
>> Add pseries_eeh_type_to_rtas() to translate the two generic EEH error
>> types (EEH_ERR_TYPE_32, EEH_ERR_TYPE_64) into the corresponding RTAS
>> ibm,errinjct encodings.
>>
>> Add validate_addr_mask_in_pe() to verify that a caller-provided address
>> falls within a BAR of some device in the PE.  RTAS IOA bus-error
>> injection requires a PCI/IOA bus address; validate_addr_mask_in_pe()
>> accepts only PCI bus addresses.  VFIO userspace resource addresses are
>> normalized to PCI bus addresses in vfio_iommu_spapr_tce.c before
>> reaching this backend; no resource-address fallback is added here.
>> Use pcibios_resource_to_bus() to convert each BAR resource to PCI bus
>> address space before validating the address.  Returns -EINVAL (Linux
>> errno) rather than a raw RTAS status.
>>
>> Add validate_errinjct_args() as a top-level validation wrapper that
>> checks the PE pointer, maps the generic type, and validates the function
>> range.  Address/mask validation against BARs is done in the buffer
>> preparation step.
>>
>> Add prepare_errinjct_buffer() which:
>>    - guards against NULL buf/pe/pe->phb
>>    - zeroes the buffer unconditionally with memset()
>>    - checks 32-bit truncation for the IOA_BUS_ERROR case
>>    - calls validate_addr_mask_in_pe() for address validation
>>    - supports only the IOA bus-error types (unreachable non-IOA
>>      RTAS cases removed)
>> The caller provides an exclusive per-call RTAS work-area buffer.
>> Firmware session serialization is left to the caller.
>>
>> pseries_eeh_err_inject() retains the existing MMIO injection body
>> unchanged in this patch.  It is replaced by the full RTAS session
>> implementation in the next patch.  This keeps the patch bisectable:
>> each commit builds and functions correctly independently.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202512101130.EYUo0oZx- 
>> lkp@intel.com/
>> Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
>> ---
>>   arch/powerpc/platforms/pseries/eeh_pseries.c | 215 +++++++++++++++++++
>>   1 file changed, 215 insertions(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/ 
>> powerpc/platforms/pseries/eeh_pseries.c
>> index b12ef382fec7..fafe0004e738 100644
>> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
>> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
>> @@ -12,6 +12,8 @@
>>    * Copyright Linas Vepstas 2005, 2006
>>    */
>> +#define pr_fmt(fmt) "EEH: " fmt
>> +
>>   #include <linux/atomic.h>
>>   #include <linux/delay.h>
>>   #include <linux/export.h>
>> @@ -786,6 +788,219 @@ static int pseries_notify_resume(struct eeh_dev 
>> *edev)
>>   }
>>   #endif
>> +/*
>> + * RTAS firmware error-type encodings (PAPR).  These are pseries- 
>> private;
>> + * user-space sees only the generic EEH_ERR_TYPE_* values from eeh.h.
>> + * Only IOA bus-error types are supported; other PAPR encodings are not
>> + * reachable through the generic EEH_ERR_TYPE_* UAPI.
>> + */
>> +#define RTAS_ERR_TYPE_IOA_BUS_ERROR        0x07
>> +#define RTAS_ERR_TYPE_IOA_BUS_ERROR_64        0x0f
>> +
>> +/* Size of the ibm,errinjct work buffer as defined by PAPR */
>> +#define RTAS_ERRINJCT_BUF_SIZE            SZ_1K
>> +
>> +/**
>> + * pseries_eeh_type_to_rtas - Map generic EEH error type to RTAS 
>> encoding
>> + * @type: generic EEH error type (EEH_ERR_TYPE_32, EEH_ERR_TYPE_64)
>> + *
>> + * Translates the generic VFIO/EEH error type passed from userspace into
>> + * the RTAS-specific ibm,errinjct error type encoding defined by PAPR.
>> + *
>> + * Return: RTAS error type on success, -EINVAL for unknown types.
>> + */
>> +static int pseries_eeh_type_to_rtas(int type)
>> +{
>> +    switch (type) {
>> +    case EEH_ERR_TYPE_32:
>> +        return RTAS_ERR_TYPE_IOA_BUS_ERROR;
>> +    case EEH_ERR_TYPE_64:
>> +        return RTAS_ERR_TYPE_IOA_BUS_ERROR_64;
>> +    default:
>> +        return -EINVAL;
>> +    }
>> +}
>> +
>> +/**
>> + * validate_addr_mask_in_pe - Validate addr against PCI bus BAR 
>> addresses in PE
>> + * @pe:   EEH PE containing one or more PCI devices
>> + * @addr: PCI bus address to validate
>> + * @mask: address mask (passed to firmware unchanged)
>> + *
>> + * RTAS IOA bus-error injection uses PCI bus addresses.  Linux PCI 
>> resources
>> + * are CPU/resource addresses and may differ on pseries due to PHB 
>> window
>> + * translation.  Convert each BAR resource to PCI bus address space 
>> before
>> + * validating @addr.
>> + *
>> + * Zero addr and mask are accepted without BAR lookup (no-address 
>> injection).
>> + *
>> + * Return: 0 if valid, -EINVAL on invalid input.
>> + */
>> +static int validate_addr_mask_in_pe(struct eeh_pe *pe, unsigned long 
>> addr,
>> +                    unsigned long mask)
>> +{
>> +    struct pci_bus_region region;
>> +    struct eeh_dev *edev, *tmp;
>> +    struct pci_dev *pdev;
>> +    struct resource *res;
>> +    resource_size_t bus_start;
>> +    resource_size_t bus_len;
>> +    int bar;
>> +
>> +    if (!addr && !mask)
>> +        return 0;
>> +
>> +    if (!pe)
>> +        return -EINVAL;
>> +
>> +    /*
>> +     * RTAS IOA bus-error injection uses PCI bus addresses. Linux PCI
>> +     * resources are CPU/resource addresses and may differ on pseries 
>> due
>> +     * to PHB window translation. Convert each BAR resource to PCI bus
>> +     * address space before validating @addr.
>> +     */
>> +    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];
>> +
>> +            if (!resource_size(res))
>> +                continue;
>> +
>> +            if (!(res->flags & (IORESOURCE_MEM | IORESOURCE_IO)))
>> +                continue;
>> +
>> +            pcibios_resource_to_bus(pdev->bus, &region, res);
>> +
>> +            bus_start = region.start;
>> +            bus_len = resource_size(res);
>> +
>> +            if ((resource_size_t)addr >= bus_start &&
>> +                ((resource_size_t)addr - bus_start) < bus_len) {
>> +                pr_debug("addr=0x%lx mask=0x%lx validated in PCI bus 
>> BAR[%d] of %s: bus range 0x%llx-0x%llx\n",
>> +                     addr, mask, bar, pci_name(pdev),
>> +                     (unsigned long long)region.start,
>> +                     (unsigned long long)region.end);
>> +                return 0;
>> +            }
>> +        }
>> +    }
>> +
>> +    pr_err("addr=0x%lx mask=0x%lx not within any PCI bus BAR of any 
>> device in PE\n",
>> +           addr, mask);
>> +    return -EINVAL;
>> +}
>> +
>> +/**
>> + * validate_errinjct_args - Top-level validation for RTAS error 
>> injection arguments
>> + * @pe:   EEH PE for the target device
>> + * @type: generic EEH error type
>> + * @func: error function selector
>> + * @addr: address argument (type-dependent, may be zero)
>> + * @mask: mask argument (type-dependent, may be zero)
>> + *
>> + * Validates all parameters before opening an RTAS injection session.
>> + * Returns Linux errno values; does not return raw RTAS status codes.
>> + *
>> + * Return: 0 if all parameters are valid, negative errno otherwise.
>> + */
>> +static int validate_errinjct_args(struct eeh_pe *pe, int type, int func,
>> +                  unsigned long addr, unsigned long mask)
>> +{
>> +    if (!pe || !pe->phb)
>> +        return -EINVAL;
>> +
>> +    if (pseries_eeh_type_to_rtas(type) < 0)
>> +        return -EINVAL;
>> +
>> +    if (func < EEH_ERR_FUNC_MIN || func > EEH_ERR_FUNC_MAX)
>> +        return -EINVAL;
>> +
>> +    return 0;
>> +}
>> +
>> +/**
>> + * prepare_errinjct_buffer() - Build ibm,errinjct work buffer
>> + * @buf: RTAS error-injection work buffer
>> + * @pe: EEH PE associated with the injection target
>> + * @rtas_type: RTAS firmware error-injection type
> 
> rtas_type is confusing to me. How about err_type?
Agreed on making the distinction clearer. In v5 I renamed it to 
rtas_err_type, since this value is specifically the RTAS/PAPR 
error-injection type after mapping from the generic EEH type.
> 
>> + * @func: Error function selector
>> + * @addr: Target PCI bus address
>> + * @mask: Address mask passed to firmware
>> + *
>> + * Zeroes @buf and populates it according to the PAPR layout for the
>> + * selected IOA bus-error injection type.
>> + *
>> + * The caller provides an exclusive per-call RTAS work-area buffer.
>> + * Firmware session serialization is handled by 
>> pseries_eeh_err_inject().
>> + *
>> + * Return: 0 on success or a negative errno on invalid input.
>> + */
>> +static int prepare_errinjct_buffer(void *buf, struct eeh_pe *pe,
>> +                   int rtas_type, int func,
>> +                   unsigned long addr, unsigned long mask)
>> +{
>> +    __be64 *buf64 = (__be64 *)buf;
>> +    __be32 *buf32 = (__be32 *)buf;
>> +    int rc;
>> +
>> +    if (!buf || !pe || !pe->phb)
>> +        return -EINVAL;
>> +
>> +    memset(buf, 0, RTAS_ERRINJCT_BUF_SIZE);
>> +
>> +    switch (rtas_type) {
>> +    case RTAS_ERR_TYPE_IOA_BUS_ERROR:
>> +        if (func < EEH_ERR_FUNC_MIN || func > EEH_ERR_FUNC_MAX)
>> +            return -EINVAL;
> 
> This condition is common for both cases, so let take it out of switch.
> And lets move the func check before memset.
Agreed.

In v5 the function-range check is moved out of the type-specific switch 
and is owned by validate_errinjct_args(), which runs before 
prepare_errinjct_buffer().

prepare_errinjct_buffer() therefore no longer duplicates the function 
validation and can zero/build the buffer after the common arguments have 
already been validated.

Thanks,
Narayana
> 
>> +
>> +        if (upper_32_bits(addr) || upper_32_bits(mask)) {
>> +            pr_err("32-bit IOA injection cannot encode addr=%#lx 
>> mask=%#lx\n",
>> +                   addr, mask);
>> +            return -EINVAL;
>> +        }
>> +
>> +        rc = validate_addr_mask_in_pe(pe, addr, mask);
>> +        if (rc)
>> +            return rc;
>> +
>> +        buf32[0] = cpu_to_be32((u32)addr);
>> +        buf32[1] = cpu_to_be32((u32)mask);
>> +        buf32[2] = cpu_to_be32(pe->addr);
>> +        buf32[3] = cpu_to_be32(BUID_HI(pe->phb->buid));
>> +        buf32[4] = cpu_to_be32(BUID_LO(pe->phb->buid));
>> +        buf32[5] = cpu_to_be32(func);
>> +        break;
>> +
>> +    case RTAS_ERR_TYPE_IOA_BUS_ERROR_64:
>> +        if (func < EEH_ERR_FUNC_MIN || func > EEH_ERR_FUNC_MAX)
>> +            return -EINVAL;
>> +
>> +        rc = validate_addr_mask_in_pe(pe, addr, mask);
>> +        if (rc)
>> +            return rc;
>> +
>> +        buf64[0] = cpu_to_be64(addr);
>> +        buf64[1] = cpu_to_be64(mask);
>> +        buf32[4] = cpu_to_be32(pe->addr);
>> +        buf32[5] = cpu_to_be32(BUID_HI(pe->phb->buid));
>> +        buf32[6] = cpu_to_be32(BUID_LO(pe->phb->buid));
>> +        buf32[7] = cpu_to_be32(func);
>> +        break;
>> +
>> +    default:
>> +        pr_err("unsupported RTAS error injection type 0x%x\n", 
>> rtas_type);
>> +        return -EINVAL;
>> +    }
>> +
>> +    pr_debug("errinjct buffer ready: rtas_type=0x%x func=%d 
>> addr=0x%lx mask=0x%lx\n",
>> +         rtas_type, func, addr, mask);
>> +    return 0;
>> +}
>> +
>>   /**
>>    * pseries_eeh_err_inject - Inject specified error to the indicated PE
>>    * @pe: the indicated PE
>