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

[Matthew Rosato]

On 10/9/20 11:48 AM, Cornelia Huck wrote:
> 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?

Sure, I was just continuing what -inst.c and -bus.c do today.  I'll 
remove DPRINTF here and look at what trace-events make sense, with a 
note to convert the rest of s390-pci* to use trace events at some later 
time.

> Other than that, looks good to me.

Thanks!  Assuming nobody has further comments, I'll plan to send both 
this set (with the change above) and the 's390x/pci: Accomodate vfio DMA 
limiting' set merged together with a single linux header sync once 
5.10-rc1 is available, sound OK?