Re: [PATCH 1/2] powerpc/eeh: Introduce eeh_pe_inject_err()

Gavin Shan
2015-03-15T22:41:02Z

On Fri, Mar 13, 2015 at 02:28:33PM -0600, Alex Williamson wrote:
>On Wed, 2015-03-11 at 17:34 +1100, Gavin Shan wrote:
>> The patch defines PCI error types and functions in eeh.h and
>> exports function eeh_pe_inject_err(), which will be called by
>> VFIO driver to inject the specified PCI error to the indicated
>> PE for testing purpose.
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/include/asm/eeh.h | 24 ++++++++++++++++
>> arch/powerpc/kernel/eeh.c | 63 ++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 87 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
>> index 9de87ce..eb20c62 100644
>> --- a/arch/powerpc/include/asm/eeh.h
>> +++ b/arch/powerpc/include/asm/eeh.h
>> @@ -196,6 +196,28 @@ enum {
>> #define EEH_RESET_COMPLETE 4 /* PHB complete reset */
>> #define EEH_LOG_TEMP 1 /* EEH temporary error log */
>> #define EEH_LOG_PERM 2 /* EEH permanent error log */
>> +#define EEH_ERR_TYPE_32 0 /* 32-bits PCI error */
>> +#define EEH_ERR_TYPE_64 1 /* 64-bits PCI error */
>> +#define EEH_ERR_FUNC_LD_MEM_ADDR 0 /* Memory load */
>> +#define EEH_ERR_FUNC_LD_MEM_DATA 1
>> +#define EEH_ERR_FUNC_LD_IO_ADDR 2 /* IO load */
>> +#define EEH_ERR_FUNC_LD_IO_DATA 3
>> +#define EEH_ERR_FUNC_LD_CFG_ADDR 4 /* Config load */
>> +#define EEH_ERR_FUNC_LD_CFG_DATA 5
>> +#define EEH_ERR_FUNC_ST_MEM_ADDR 6 /* Memory store */
>> +#define EEH_ERR_FUNC_ST_MEM_DATA 7
>> +#define EEH_ERR_FUNC_ST_IO_ADDR 8 /* IO store */
>> +#define EEH_ERR_FUNC_ST_IO_DATA 9
>> +#define EEH_ERR_FUNC_ST_CFG_ADDR 10 /* Config store */
>> +#define EEH_ERR_FUNC_ST_CFG_DATA 11
>> +#define EEH_ERR_FUNC_DMA_RD_ADDR 12 /* DMA read */
>> +#define EEH_ERR_FUNC_DMA_RD_DATA 13
>> +#define EEH_ERR_FUNC_DMA_RD_MASTER 14
>> +#define EEH_ERR_FUNC_DMA_RD_TARGET 15
>> +#define EEH_ERR_FUNC_DMA_WR_ADDR 16 /* DMA write */
>> +#define EEH_ERR_FUNC_DMA_WR_DATA 17
>> +#define EEH_ERR_FUNC_DMA_WR_MASTER 18
>> +#define EEH_ERR_FUNC_DMA_WR_TARGET 19
>>
>> struct eeh_ops {
>> char *name;
>> @@ -296,6 +318,8 @@ int eeh_pe_set_option(struct eeh_pe *pe, int option);
>> int eeh_pe_get_state(struct eeh_pe *pe);
>> int eeh_pe_reset(struct eeh_pe *pe, int option);
>> 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);
>>
>> /**
>> * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>> index 60a0f15ce..dbab1a4 100644
>> --- a/arch/powerpc/kernel/eeh.c
>> +++ b/arch/powerpc/kernel/eeh.c
>> @@ -1657,6 +1657,69 @@ int eeh_pe_configure(struct eeh_pe *pe)
>> }
>> EXPORT_SYMBOL_GPL(eeh_pe_configure);
>>
>> +/**
>> + * eeh_pe_inject_err - Injecting the specified PCI error to the indicated PE
>> + * @pe: the indicated PE
>> + * @type: error type
>> + * @function: error function
>> + * @addr: address
>> + * @mask: address mask
>> + *
>> + * The routine is called to inject the specified PCI error, which
>> + * is determined by @type and @function, to the indicated PE for
>> + * testing purpose.
>> + */
>> +int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
>> + unsigned long addr, unsigned long mask)
>> +{
>> + /* Invalid PE ? */
>> + if (!pe)
>> + return -ENODEV;
>> +
>> + /* Unsupported operation ? */
>> + if (!eeh_ops || !eeh_ops->err_inject)
>> + return -ENOENT;
>> +
>> + /* Check on PCI error type */
>> + switch (type) {
>> + case EEH_ERR_TYPE_32:
>> + case EEH_ERR_TYPE_64:
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + /* Check on PCI error function */
>> + switch (func) {
>> + case EEH_ERR_FUNC_LD_MEM_ADDR:
>> + case EEH_ERR_FUNC_LD_MEM_DATA:
>> + case EEH_ERR_FUNC_LD_IO_ADDR:
>> + case EEH_ERR_FUNC_LD_IO_DATA:
>> + case EEH_ERR_FUNC_LD_CFG_ADDR:
>> + case EEH_ERR_FUNC_LD_CFG_DATA:
>> + case EEH_ERR_FUNC_ST_MEM_ADDR:
>> + case EEH_ERR_FUNC_ST_MEM_DATA:
>> + case EEH_ERR_FUNC_ST_IO_ADDR:
>> + case EEH_ERR_FUNC_ST_IO_DATA:
>> + case EEH_ERR_FUNC_ST_CFG_ADDR:
>> + case EEH_ERR_FUNC_ST_CFG_DATA:
>> + case EEH_ERR_FUNC_DMA_RD_ADDR:
>> + case EEH_ERR_FUNC_DMA_RD_DATA:
>> + case EEH_ERR_FUNC_DMA_RD_MASTER:
>> + case EEH_ERR_FUNC_DMA_RD_TARGET:
>> + case EEH_ERR_FUNC_DMA_WR_ADDR:
>> + case EEH_ERR_FUNC_DMA_WR_DATA:
>> + case EEH_ERR_FUNC_DMA_WR_MASTER:
>> + case EEH_ERR_FUNC_DMA_WR_TARGET:
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>
>Potentially a good use case for an enum for both type and func. Then
>you could close each enum with a "MAX" entry and simply test type/func <
>MAX. Thanks,
>

Yes, It's reasonable to have following macro as all constants in eeh.h
are represented by macros.

#define EEH_ERR_FUNC_MAX 19

if (func > 19)
return -EINVAL;

Thanks,
Gavin

>Alex
>
>> +
>> + return eeh_ops->err_inject(pe, type, func, addr, mask);
>> +}
>> +EXPORT_SYMBOL_GPL(eeh_pe_inject_err);
>> +
>> static int proc_eeh_show(struct seq_file *m, void *v)
>> {
>> if (!eeh_enabled()) {
>
>
>
>--
>To unsubscribe from this list: send the line "unsubscribe kvm" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>