Re: [PATCH v3 10/10] s390x/pci: get zPCI function info from host

[Cornelia Huck <cohuck@redhat.com>]

On Wed,  7 Oct 2020 15:04:15 -0400
Matthew Rosato <mjrosato@linux.ibm.com> wrote:

> We use the capability chains of the VFIO_DEVICE_GET_INFO ioctl to retrieve
> the CLP information that the kernel exports.
> 
> To be compatible with previous kernel versions we fall back on previous
> predefined values, same as the emulation values, when the ioctl is found
> to not support capability chains. If individual CLP capabilities are not
> found, we fall back on default values for only those capabilities missing
> from the chain.
> 
> This patch is based on work previously done by Pierre Morel.
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
>  hw/s390x/meson.build             |   1 +
>  hw/s390x/s390-pci-bus.c          |  10 +-
>  hw/s390x/s390-pci-vfio.c         | 197 +++++++++++++++++++++++++++++++++++++++
>  include/hw/s390x/s390-pci-bus.h  |   1 +
>  include/hw/s390x/s390-pci-clp.h  |  12 ++-
>  include/hw/s390x/s390-pci-vfio.h |  19 ++++
>  6 files changed, 233 insertions(+), 7 deletions(-)
>  create mode 100644 hw/s390x/s390-pci-vfio.c
>  create mode 100644 include/hw/s390x/s390-pci-vfio.h

(...)

> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
> new file mode 100644
> index 0000000..43684c6
> --- /dev/null
> +++ b/hw/s390x/s390-pci-vfio.c
> @@ -0,0 +1,197 @@
> +/*
> + * s390 vfio-pci interfaces
> + *
> + * Copyright 2020 IBM Corp.
> + * Author(s): Matthew Rosato <mjrosato@linux.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include <sys/ioctl.h>
> +#include <linux/vfio.h>
> +#include <linux/vfio_zdev.h>
> +
> +#include "qemu/osdep.h"
> +#include "hw/s390x/s390-pci-bus.h"
> +#include "hw/s390x/s390-pci-clp.h"
> +#include "hw/s390x/s390-pci-vfio.h"
> +#include "hw/vfio/pci.h"
> +
> +#ifndef DEBUG_S390PCI_VFIO
> +#define DEBUG_S390PCI_VFIO  0
> +#endif
> +
> +#define DPRINTF(fmt, ...)                                          \
> +    do {                                                           \
> +        if (DEBUG_S390PCI_VFIO) {                                  \
> +            fprintf(stderr, "S390pci-vfio: " fmt, ## __VA_ARGS__); \
> +        }                                                          \
> +    } while (0)

Not really a fan of DPRINTF. Can you maybe use trace events instead?

Other than that, looks good to me.