| Message ID | 20231026103104.1686921-2-zhenzhong.duan@intel.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v3,01/37] vfio/container: Move IBM EEH related functions into spapr_pci_vfio.c | expand |
On Thu, 2023-10-26 at 18:30 +0800, Zhenzhong Duan wrote: > With vfio_eeh_as_ok/vfio_eeh_as_op moved and made static, > vfio.h becomes empty and is deleted. > > No functional changes intended. > > Suggested-by: Cédric Le Goater <clg@redhat.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > include/hw/vfio/vfio.h | 7 --- > hw/ppc/spapr_pci_vfio.c | 100 > +++++++++++++++++++++++++++++++++++++++- > hw/vfio/ap.c | 1 - > hw/vfio/ccw.c | 1 - > hw/vfio/common.c | 1 - > hw/vfio/container.c | 98 -------------------------------------- > - > hw/vfio/helpers.c | 1 - > 7 files changed, 99 insertions(+), 110 deletions(-) > delete mode 100644 include/hw/vfio/vfio.h Can't speak to the code movement, but removing the header file from those places where it's not used is fine. Acked-by: Eric Farman <farman@linux.ibm.com>
On 10/26/23 12:30, Zhenzhong Duan wrote: > With vfio_eeh_as_ok/vfio_eeh_as_op moved and made static, > vfio.h becomes empty and is deleted. > > No functional changes intended. > > Suggested-by: Cédric Le Goater <clg@redhat.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > include/hw/vfio/vfio.h | 7 --- > hw/ppc/spapr_pci_vfio.c | 100 +++++++++++++++++++++++++++++++++++++++- > hw/vfio/ap.c | 1 - > hw/vfio/ccw.c | 1 - > hw/vfio/common.c | 1 - > hw/vfio/container.c | 98 --------------------------------------- > hw/vfio/helpers.c | 1 - > 7 files changed, 99 insertions(+), 110 deletions(-) > delete mode 100644 include/hw/vfio/vfio.h > > diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h > deleted file mode 100644 > index 86248f5436..0000000000 > --- a/include/hw/vfio/vfio.h > +++ /dev/null > @@ -1,7 +0,0 @@ > -#ifndef HW_VFIO_H > -#define HW_VFIO_H > - > -bool vfio_eeh_as_ok(AddressSpace *as); > -int vfio_eeh_as_op(AddressSpace *as, uint32_t op); > - > -#endif > diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c > index 9016720547..f283f7e38d 100644 > --- a/hw/ppc/spapr_pci_vfio.c > +++ b/hw/ppc/spapr_pci_vfio.c > @@ -18,14 +18,112 @@ > */ > > #include "qemu/osdep.h" > +#include <sys/ioctl.h> > #include <linux/vfio.h> > #include "hw/ppc/spapr.h" > #include "hw/pci-host/spapr.h" > #include "hw/pci/msix.h" > #include "hw/pci/pci_device.h" > -#include "hw/vfio/vfio.h" > +#include "hw/vfio/vfio-common.h" > #include "qemu/error-report.h" > > +/* > + * Interfaces for IBM EEH (Enhanced Error Handling) > + */ > +static bool vfio_eeh_container_ok(VFIOContainer *container) > +{ > + /* > + * As of 2016-03-04 (linux-4.5) the host kernel EEH/VFIO > + * implementation is broken if there are multiple groups in a > + * container. The hardware works in units of Partitionable > + * Endpoints (== IOMMU groups) and the EEH operations naively > + * iterate across all groups in the container, without any logic > + * to make sure the groups have their state synchronized. For > + * certain operations (ENABLE) that might be ok, until an error > + * occurs, but for others (GET_STATE) it's clearly broken. > + */ > + > + /* > + * XXX Once fixed kernels exist, test for them here > + */ > + > + if (QLIST_EMPTY(&container->group_list)) { > + return false; > + } > + > + if (QLIST_NEXT(QLIST_FIRST(&container->group_list), container_next)) { > + return false; > + } > + > + return true; > +} > + > +static int vfio_eeh_container_op(VFIOContainer *container, uint32_t op) > +{ > + struct vfio_eeh_pe_op pe_op = { > + .argsz = sizeof(pe_op), > + .op = op, > + }; > + int ret; > + > + if (!vfio_eeh_container_ok(container)) { > + error_report("vfio/eeh: EEH_PE_OP 0x%x: " > + "kernel requires a container with exactly one group", op); > + return -EPERM; > + } > + > + ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op); > + if (ret < 0) { > + error_report("vfio/eeh: EEH_PE_OP 0x%x failed: %m", op); > + return -errno; > + } > + > + return ret; > +} > + > +static VFIOContainer *vfio_eeh_as_container(AddressSpace *as) > +{ > + VFIOAddressSpace *space = vfio_get_address_space(as); > + VFIOContainer *container = NULL; > + > + if (QLIST_EMPTY(&space->containers)) { > + /* No containers to act on */ > + goto out; > + } > + > + container = QLIST_FIRST(&space->containers); > + > + if (QLIST_NEXT(container, next)) { > + /* > + * We don't yet have logic to synchronize EEH state across > + * multiple containers > + */ > + container = NULL; > + goto out; > + } > + > +out: > + vfio_put_address_space(space); > + return container; > +} > + > +static bool vfio_eeh_as_ok(AddressSpace *as) > +{ > + VFIOContainer *container = vfio_eeh_as_container(as); > + > + return (container != NULL) && vfio_eeh_container_ok(container); > +} > + > +static int vfio_eeh_as_op(AddressSpace *as, uint32_t op) > +{ > + VFIOContainer *container = vfio_eeh_as_container(as); > + > + if (!container) { > + return -ENODEV; > + } > + return vfio_eeh_container_op(container, op); > +} > + > bool spapr_phb_eeh_available(SpaprPhbState *sphb) > { > return vfio_eeh_as_ok(&sphb->iommu_as); > diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c > index 5f257bffb9..bbf69ff55a 100644 > --- a/hw/vfio/ap.c > +++ b/hw/vfio/ap.c > @@ -14,7 +14,6 @@ > #include <linux/vfio.h> > #include <sys/ioctl.h> > #include "qapi/error.h" > -#include "hw/vfio/vfio.h" > #include "hw/vfio/vfio-common.h" > #include "hw/s390x/ap-device.h" > #include "qemu/error-report.h" > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index 6623ae237b..d857bb8d0f 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -20,7 +20,6 @@ > #include <sys/ioctl.h> > > #include "qapi/error.h" > -#include "hw/vfio/vfio.h" > #include "hw/vfio/vfio-common.h" > #include "hw/s390x/s390-ccw.h" > #include "hw/s390x/vfio-ccw.h" > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index 9c5c6433f2..e72055e752 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -26,7 +26,6 @@ > #include <linux/vfio.h> > > #include "hw/vfio/vfio-common.h" > -#include "hw/vfio/vfio.h" > #include "hw/vfio/pci.h" > #include "exec/address-spaces.h" > #include "exec/memory.h" > diff --git a/hw/vfio/container.c b/hw/vfio/container.c > index fc88222377..83c0f05bba 100644 > --- a/hw/vfio/container.c > +++ b/hw/vfio/container.c > @@ -26,7 +26,6 @@ > #include <linux/vfio.h> > > #include "hw/vfio/vfio-common.h" > -#include "hw/vfio/vfio.h" > #include "exec/address-spaces.h" > #include "exec/memory.h" > #include "exec/ram_addr.h" > @@ -1011,103 +1010,6 @@ static void vfio_put_base_device(VFIODevice *vbasedev) > close(vbasedev->fd); > } > > -/* > - * Interfaces for IBM EEH (Enhanced Error Handling) > - */ > -static bool vfio_eeh_container_ok(VFIOContainer *container) > -{ > - /* > - * As of 2016-03-04 (linux-4.5) the host kernel EEH/VFIO > - * implementation is broken if there are multiple groups in a > - * container. The hardware works in units of Partitionable > - * Endpoints (== IOMMU groups) and the EEH operations naively > - * iterate across all groups in the container, without any logic > - * to make sure the groups have their state synchronized. For > - * certain operations (ENABLE) that might be ok, until an error > - * occurs, but for others (GET_STATE) it's clearly broken. > - */ > - > - /* > - * XXX Once fixed kernels exist, test for them here > - */ > - > - if (QLIST_EMPTY(&container->group_list)) { > - return false; > - } > - > - if (QLIST_NEXT(QLIST_FIRST(&container->group_list), container_next)) { > - return false; > - } > - > - return true; > -} > - > -static int vfio_eeh_container_op(VFIOContainer *container, uint32_t op) > -{ > - struct vfio_eeh_pe_op pe_op = { > - .argsz = sizeof(pe_op), > - .op = op, > - }; > - int ret; > - > - if (!vfio_eeh_container_ok(container)) { > - error_report("vfio/eeh: EEH_PE_OP 0x%x: " > - "kernel requires a container with exactly one group", op); > - return -EPERM; > - } > - > - ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op); > - if (ret < 0) { > - error_report("vfio/eeh: EEH_PE_OP 0x%x failed: %m", op); > - return -errno; > - } > - > - return ret; > -} > - > -static VFIOContainer *vfio_eeh_as_container(AddressSpace *as) > -{ > - VFIOAddressSpace *space = vfio_get_address_space(as); > - VFIOContainer *container = NULL; > - > - if (QLIST_EMPTY(&space->containers)) { > - /* No containers to act on */ > - goto out; > - } > - > - container = QLIST_FIRST(&space->containers); > - > - if (QLIST_NEXT(container, next)) { > - /* > - * We don't yet have logic to synchronize EEH state across > - * multiple containers > - */ > - container = NULL; > - goto out; > - } > - > -out: > - vfio_put_address_space(space); > - return container; > -} > - > -bool vfio_eeh_as_ok(AddressSpace *as) > -{ > - VFIOContainer *container = vfio_eeh_as_container(as); > - > - return (container != NULL) && vfio_eeh_container_ok(container); > -} > - > -int vfio_eeh_as_op(AddressSpace *as, uint32_t op) > -{ > - VFIOContainer *container = vfio_eeh_as_container(as); > - > - if (!container) { > - return -ENODEV; > - } > - return vfio_eeh_container_op(container, op); > -} > - > static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp) > { > char *tmp, group_path[PATH_MAX], *group_name; > diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c > index 7e5da21b31..168847e7c5 100644 > --- a/hw/vfio/helpers.c > +++ b/hw/vfio/helpers.c > @@ -23,7 +23,6 @@ > #include <sys/ioctl.h> > > #include "hw/vfio/vfio-common.h" > -#include "hw/vfio/vfio.h" > #include "hw/hw.h" > #include "trace.h" > #include "qapi/error.h"
diff --git a/include/hw/vfio/vfio.h b/include/hw/vfio/vfio.h deleted file mode 100644 index 86248f5436..0000000000 --- a/include/hw/vfio/vfio.h +++ /dev/null @@ -1,7 +0,0 @@ -#ifndef HW_VFIO_H -#define HW_VFIO_H - -bool vfio_eeh_as_ok(AddressSpace *as); -int vfio_eeh_as_op(AddressSpace *as, uint32_t op); - -#endif diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c index 9016720547..f283f7e38d 100644 --- a/hw/ppc/spapr_pci_vfio.c +++ b/hw/ppc/spapr_pci_vfio.c @@ -18,14 +18,112 @@ */ #include "qemu/osdep.h" +#include <sys/ioctl.h> #include <linux/vfio.h> #include "hw/ppc/spapr.h" #include "hw/pci-host/spapr.h" #include "hw/pci/msix.h" #include "hw/pci/pci_device.h" -#include "hw/vfio/vfio.h" +#include "hw/vfio/vfio-common.h" #include "qemu/error-report.h" +/* + * Interfaces for IBM EEH (Enhanced Error Handling) + */ +static bool vfio_eeh_container_ok(VFIOContainer *container) +{ + /* + * As of 2016-03-04 (linux-4.5) the host kernel EEH/VFIO + * implementation is broken if there are multiple groups in a + * container. The hardware works in units of Partitionable + * Endpoints (== IOMMU groups) and the EEH operations naively + * iterate across all groups in the container, without any logic + * to make sure the groups have their state synchronized. For + * certain operations (ENABLE) that might be ok, until an error + * occurs, but for others (GET_STATE) it's clearly broken. + */ + + /* + * XXX Once fixed kernels exist, test for them here + */ + + if (QLIST_EMPTY(&container->group_list)) { + return false; + } + + if (QLIST_NEXT(QLIST_FIRST(&container->group_list), container_next)) { + return false; + } + + return true; +} + +static int vfio_eeh_container_op(VFIOContainer *container, uint32_t op) +{ + struct vfio_eeh_pe_op pe_op = { + .argsz = sizeof(pe_op), + .op = op, + }; + int ret; + + if (!vfio_eeh_container_ok(container)) { + error_report("vfio/eeh: EEH_PE_OP 0x%x: " + "kernel requires a container with exactly one group", op); + return -EPERM; + } + + ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op); + if (ret < 0) { + error_report("vfio/eeh: EEH_PE_OP 0x%x failed: %m", op); + return -errno; + } + + return ret; +} + +static VFIOContainer *vfio_eeh_as_container(AddressSpace *as) +{ + VFIOAddressSpace *space = vfio_get_address_space(as); + VFIOContainer *container = NULL; + + if (QLIST_EMPTY(&space->containers)) { + /* No containers to act on */ + goto out; + } + + container = QLIST_FIRST(&space->containers); + + if (QLIST_NEXT(container, next)) { + /* + * We don't yet have logic to synchronize EEH state across + * multiple containers + */ + container = NULL; + goto out; + } + +out: + vfio_put_address_space(space); + return container; +} + +static bool vfio_eeh_as_ok(AddressSpace *as) +{ + VFIOContainer *container = vfio_eeh_as_container(as); + + return (container != NULL) && vfio_eeh_container_ok(container); +} + +static int vfio_eeh_as_op(AddressSpace *as, uint32_t op) +{ + VFIOContainer *container = vfio_eeh_as_container(as); + + if (!container) { + return -ENODEV; + } + return vfio_eeh_container_op(container, op); +} + bool spapr_phb_eeh_available(SpaprPhbState *sphb) { return vfio_eeh_as_ok(&sphb->iommu_as); diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c index 5f257bffb9..bbf69ff55a 100644 --- a/hw/vfio/ap.c +++ b/hw/vfio/ap.c @@ -14,7 +14,6 @@ #include <linux/vfio.h> #include <sys/ioctl.h> #include "qapi/error.h" -#include "hw/vfio/vfio.h" #include "hw/vfio/vfio-common.h" #include "hw/s390x/ap-device.h" #include "qemu/error-report.h" diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index 6623ae237b..d857bb8d0f 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -20,7 +20,6 @@ #include <sys/ioctl.h> #include "qapi/error.h" -#include "hw/vfio/vfio.h" #include "hw/vfio/vfio-common.h" #include "hw/s390x/s390-ccw.h" #include "hw/s390x/vfio-ccw.h" diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 9c5c6433f2..e72055e752 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -26,7 +26,6 @@ #include <linux/vfio.h> #include "hw/vfio/vfio-common.h" -#include "hw/vfio/vfio.h" #include "hw/vfio/pci.h" #include "exec/address-spaces.h" #include "exec/memory.h" diff --git a/hw/vfio/container.c b/hw/vfio/container.c index fc88222377..83c0f05bba 100644 --- a/hw/vfio/container.c +++ b/hw/vfio/container.c @@ -26,7 +26,6 @@ #include <linux/vfio.h> #include "hw/vfio/vfio-common.h" -#include "hw/vfio/vfio.h" #include "exec/address-spaces.h" #include "exec/memory.h" #include "exec/ram_addr.h" @@ -1011,103 +1010,6 @@ static void vfio_put_base_device(VFIODevice *vbasedev) close(vbasedev->fd); } -/* - * Interfaces for IBM EEH (Enhanced Error Handling) - */ -static bool vfio_eeh_container_ok(VFIOContainer *container) -{ - /* - * As of 2016-03-04 (linux-4.5) the host kernel EEH/VFIO - * implementation is broken if there are multiple groups in a - * container. The hardware works in units of Partitionable - * Endpoints (== IOMMU groups) and the EEH operations naively - * iterate across all groups in the container, without any logic - * to make sure the groups have their state synchronized. For - * certain operations (ENABLE) that might be ok, until an error - * occurs, but for others (GET_STATE) it's clearly broken. - */ - - /* - * XXX Once fixed kernels exist, test for them here - */ - - if (QLIST_EMPTY(&container->group_list)) { - return false; - } - - if (QLIST_NEXT(QLIST_FIRST(&container->group_list), container_next)) { - return false; - } - - return true; -} - -static int vfio_eeh_container_op(VFIOContainer *container, uint32_t op) -{ - struct vfio_eeh_pe_op pe_op = { - .argsz = sizeof(pe_op), - .op = op, - }; - int ret; - - if (!vfio_eeh_container_ok(container)) { - error_report("vfio/eeh: EEH_PE_OP 0x%x: " - "kernel requires a container with exactly one group", op); - return -EPERM; - } - - ret = ioctl(container->fd, VFIO_EEH_PE_OP, &pe_op); - if (ret < 0) { - error_report("vfio/eeh: EEH_PE_OP 0x%x failed: %m", op); - return -errno; - } - - return ret; -} - -static VFIOContainer *vfio_eeh_as_container(AddressSpace *as) -{ - VFIOAddressSpace *space = vfio_get_address_space(as); - VFIOContainer *container = NULL; - - if (QLIST_EMPTY(&space->containers)) { - /* No containers to act on */ - goto out; - } - - container = QLIST_FIRST(&space->containers); - - if (QLIST_NEXT(container, next)) { - /* - * We don't yet have logic to synchronize EEH state across - * multiple containers - */ - container = NULL; - goto out; - } - -out: - vfio_put_address_space(space); - return container; -} - -bool vfio_eeh_as_ok(AddressSpace *as) -{ - VFIOContainer *container = vfio_eeh_as_container(as); - - return (container != NULL) && vfio_eeh_container_ok(container); -} - -int vfio_eeh_as_op(AddressSpace *as, uint32_t op) -{ - VFIOContainer *container = vfio_eeh_as_container(as); - - if (!container) { - return -ENODEV; - } - return vfio_eeh_container_op(container, op); -} - static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp) { char *tmp, group_path[PATH_MAX], *group_name; diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c index 7e5da21b31..168847e7c5 100644 --- a/hw/vfio/helpers.c +++ b/hw/vfio/helpers.c @@ -23,7 +23,6 @@ #include <sys/ioctl.h> #include "hw/vfio/vfio-common.h" -#include "hw/vfio/vfio.h" #include "hw/hw.h" #include "trace.h" #include "qapi/error.h"
With vfio_eeh_as_ok/vfio_eeh_as_op moved and made static, vfio.h becomes empty and is deleted. No functional changes intended. Suggested-by: Cédric Le Goater <clg@redhat.com> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- include/hw/vfio/vfio.h | 7 --- hw/ppc/spapr_pci_vfio.c | 100 +++++++++++++++++++++++++++++++++++++++- hw/vfio/ap.c | 1 - hw/vfio/ccw.c | 1 - hw/vfio/common.c | 1 - hw/vfio/container.c | 98 --------------------------------------- hw/vfio/helpers.c | 1 - 7 files changed, 99 insertions(+), 110 deletions(-) delete mode 100644 include/hw/vfio/vfio.h