Re: [PATCH 2/2] drivers/vfio: Support EEH error injection

Gavin Shan
2015-03-12T03:17:46Z

On Thu, Mar 12, 2015 at 11:57:21AM +1100, David Gibson wrote:
>On Wed, Mar 11, 2015 at 05:34:11PM +1100, Gavin Shan wrote:
>> The patch adds one more EEH sub-command (VFIO_EEH_PE_INJECT_ERR)
>> to inject the specified EEH error, which is represented by
>> (struct vfio_eeh_pe_err), to the indicated PE for testing purpose.
>>
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>> Documentation/vfio.txt | 47 ++++++++++++++++++++++++++++++-------------
>> drivers/vfio/vfio_spapr_eeh.c | 14 +++++++++++++
>> include/uapi/linux/vfio.h | 34 ++++++++++++++++++++++++++++++-
>> 3 files changed, 80 insertions(+), 15 deletions(-)
>>
>> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
>> index 96978ec..2e7f736 100644
>> --- a/Documentation/vfio.txt
>> +++ b/Documentation/vfio.txt
>> @@ -328,7 +328,13 @@ So 4 additional ioctls have been added:
>>
>> The code flow from the example above should be slightly changed:
>>
>> - struct vfio_eeh_pe_op pe_op = { .argsz = sizeof(pe_op), .flags = 0 };
>> + struct vfio_eeh_pe_op *pe_op;
>> + struct vfio_eeh_pe_err *pe_err;
>> +
>> + pe_op = malloc(sizeof(*pe_op) + sizeof(*pe_err));
>> + pe_err = (void *)pe_op + sizeof(*pe_op);
>> + pe_op->argsz = sizeof(*pe_op) + sizeof(*pe_err);
>
>Surely that argsz can't be correct for most of the operations. The
>extended structure should only be there for the error inject ioctl,
>yes?
>

argsz isn't appropriate for most cases because kernel has the check
"expected_argsz < passed_argsz", not "expected_argsz == passed_argsz".
However, I'll fix it as follows to avoid confusion after collecting
more comments:

struct vfio_eeh_pe_op *pe_op;
struct vfio_eeh_pe_err *pe_err;

/* For all cases except error injection */
pe_op = malloc(sizeof(*pe_op));
pe_op->argsz = sizeof(*pe_op);

/* For error injection case here */
pe_op = realloc(sizeof(*pe_op) + sizeof(*pe_err));
pe_op->argsz = sizeof(*pe_op) + sizeof(*pe_err);
pe_err = (void *)pe_op + sizeof(*pe_op);

Thanks,
Gavin

>--
>David Gibson | I'll have my music baroque, and my code
>david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
>http://www.ozlabs.org/~dgibson