Re: [Qemu-ppc] [Qemu-devel] [RFC PATCH 01/14] vfio: Start adding VFIO/EEH interface

[Alex Williamson]

On Sat, 2015-09-19 at 17:18 +1000, David Gibson wrote:
> At present the code handling IBM's Enhanced Error Handling (EEH) interface
> on VFIO devices operates by bypassing the usual VFIO logic with
> vfio_container_ioctl().  That's a poorly designed interface with unclear
> semantics about exactly what can be operated on.
> 
> As a first step to cleaning that up, start creating a new VFIO interface
> for EEH operations.  Because EEH operates in units of a "Partitionable
> Endpoint" (PE) - a group of devices that can't be mutually isolated), it
> needs to expose host PEs (that is, IOMMU groups) to the guest.  This means
> EEH needs VFIO concepts exposed that other VFIO users don't.
> 
> At present all EEH ioctl()s in use operate conceptually on a single PE and
> take no parameters except the opcode itself.  So, expose a vfio_eeh_op()
> function to apply a single no-argument operation to a VFIO group.
> 
> At present the kernel VFIO/EEH interface is broken, because it assumes
> there is only one VFIO group per container, which is no longer always the
> case.  So, add logic to detect this case and warn.
> 
> Signed-off-by: David Gibson <address@hidden>
> ---
>  hw/vfio/Makefile.objs      |  1 +
>  hw/vfio/eeh.c              | 64 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/vfio/vfio-eeh.h | 42 ++++++++++++++++++++++++++++++
>  3 files changed, 107 insertions(+)
>  create mode 100644 hw/vfio/eeh.c
>  create mode 100644 include/hw/vfio/vfio-eeh.h
> 
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index d540c9d..384c702 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -3,4 +3,5 @@ obj-$(CONFIG_SOFTMMU) += common.o
>  obj-$(CONFIG_PCI) += pci.o
>  obj-$(CONFIG_SOFTMMU) += platform.o
>  obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
> +obj-$(CONFIG_PSERIES) += eeh.o
>  endif
> diff --git a/hw/vfio/eeh.c b/hw/vfio/eeh.c
> new file mode 100644
> index 0000000..35bd06c
> --- /dev/null
> +++ b/hw/vfio/eeh.c
> @@ -0,0 +1,64 @@
> +/*
> + * EEH (IBM Enhanced Error Handling) functions for VFIO devices
> + *
> + * Copyright Red Hat, Inc. 2015
> + *
> + * Authors:
> + *  David Gibson <address@hidden>
> + *
> + * This program is free software: you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Based on earlier EEH implementations:
> + * Copyright (c) 2011-2014 Alexey Kardashevskiy, IBM Corporation.
> + */
> +#include <sys/ioctl.h>
> +#include <linux/vfio.h>
> +
> +#include "qemu/error-report.h"
> +
> +#include "hw/vfio/vfio-common.h"
> +#include "hw/vfio/vfio-eeh.h"
> +
> +int vfio_eeh_op(VFIOGroup *group, uint32_t op)
> +{
> +    VFIOContainer *container = group->container;
> +    struct vfio_eeh_pe_op pe_op = {
> +        .argsz = sizeof(pe_op),
> +        .op = op,
> +    };
> +    int ret;
> +
> +    if (!container) {
> +        error_report("vfio/eeh: EEH_PE_OP 0x%x called on unattached group 
> %d",
> +                     op, group->groupid);
> +        return -ENODEV;
> +    }
> +
> +    /* A broken kernel interface means EEH operations can't work
> +     * correctly if there are multiple groups in a container */
> +    if ((QLIST_FIRST(&container->group_list) != group)
> +        || QLIST_NEXT(group, container_next)) {
> +        error_report("vfio/eeh: EEH_PE_OP 0x%x called on container"
> +                     " with multiple groups", op);
> +        return -ENOSPC;

-EINVAL really seems more appropriate

> +    }
> +
> +    ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op);
> +    if (ret < 0) {
> +        error_report("vfio/eeh: EEH_PE_OP 0x%x failed on group %d: %m",
> +                     op, group->groupid);
> +    }
> +
> +    return ret;

Would -errno be more interesting in the failure case?

> +}
> diff --git a/include/hw/vfio/vfio-eeh.h b/include/hw/vfio/vfio-eeh.h
> new file mode 100644
> index 0000000..d7356f2
> --- /dev/null
> +++ b/include/hw/vfio/vfio-eeh.h
> @@ -0,0 +1,42 @@
> +/*
> + * EEH (IBM Enhanced Error Handling) functions for VFIO devices
> + *
> + * Copyright Red Hat, Inc. 2015
> + *
> + * Authors:
> + *  David Gibson <address@hidden>
> + *
> + * This program is free software: you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Based on earlier EEH implementations:
> + * Copyright (c) 2011-2014 Alexey Kardashevskiy, IBM Corporation.
> + */
> +#ifndef VFIO_EEH_H
> +#define VFIO_EEH_H
> +
> +#include <linux/vfio.h>
> +
> +/*
> + * EEH (Enhanced Error Handling) used in IBM Power guests, needs extra
> + * hooks in VFIO to correctly pass error handling operations between
> + * guest and host.
> + */
> +
> +/* EEH needs to be aware of VFIOGroups as a concept, though not the
> + * internals */
> +typedef struct VFIOGroup VFIOGroup;
> +
> +int vfio_eeh_op(VFIOGroup *group, uint32_t op);
> +
> +#endif /* VFIO_EEH_H */