| Message ID | 160426454077.24886.1574050205264167707.stgit@gimli.home |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,v3,01/32] vfio: Add function to unmap VFIO region | expand |
Hi Matthew, On 11/1/20 10:02 PM, Alex Williamson wrote: > From: Matthew Rosato <mjrosato@linux.ibm.com> > > Create new files for separating out vfio-specific work for s390 > pci. Add the first such routine, which issues VFIO_IOMMU_GET_INFO > ioctl to collect the current dma available count. > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > [aw: Fix non-Linux build with CONFIG_LINUX] > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > --- > hw/s390x/meson.build | 1 + > hw/s390x/s390-pci-vfio.c | 54 ++++++++++++++++++++++++++++++++++++++ > include/hw/s390x/s390-pci-vfio.h | 24 +++++++++++++++++ > 3 files changed, 79 insertions(+) > create mode 100644 hw/s390x/s390-pci-vfio.c > create mode 100644 include/hw/s390x/s390-pci-vfio.h > > diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build > index 948ceae7a7b3..f4663a835514 100644 > --- a/hw/s390x/meson.build > +++ b/hw/s390x/meson.build > @@ -27,6 +27,7 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( > )) > s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c')) > s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c')) > +s390x_ss.add(when: 'CONFIG_LINUX', if_true: files('s390-pci-vfio.c')) > > virtio_ss = ss.source_set() > virtio_ss.add(files('virtio-ccw.c')) > diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c > new file mode 100644 > index 000000000000..cb3f4d98adf8 > --- /dev/null > +++ b/hw/s390x/s390-pci-vfio.c > @@ -0,0 +1,54 @@ > +/* > + * 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 "qemu/osdep.h" > +#include "hw/s390x/s390-pci-vfio.h" > +#include "hw/vfio/vfio-common.h" > + > +/* > + * Get the current DMA available count from vfio. Returns true if vfio is > + * limiting DMA requests, false otherwise. The current available count read > + * from vfio is returned in avail. > + */ > +bool s390_pci_update_dma_avail(int fd, unsigned int *avail) > +{ > + g_autofree struct vfio_iommu_type1_info *info; > + uint32_t argsz; > + > + assert(avail); > + > + argsz = sizeof(struct vfio_iommu_type1_info); > + info = g_malloc0(argsz); > + > + /* > + * If the specified argsz is not large enough to contain all capabilities > + * it will be updated upon return from the ioctl. Retry until we have > + * a big enough buffer to hold the entire capability chain. > + */ > +retry: > + info->argsz = argsz; > + > + if (ioctl(fd, VFIO_IOMMU_GET_INFO, info)) { > + return false; > + } > + > + if (info->argsz > argsz) { > + argsz = info->argsz; > + info = g_realloc(info, argsz); > + goto retry; > + } > + > + /* If the capability exists, update with the current value */ > + return vfio_get_info_dma_avail(info, avail); > +} --without-default-devices configuration is broken, bisected to this commit: hw/s390x/s390-pci-vfio.c:52: undefined reference to `vfio_get_info_dma_avail' Can you have a look please? Thanks.
On Tue, 3 Nov 2020 06:49:13 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Matthew, > > On 11/1/20 10:02 PM, Alex Williamson wrote: > > From: Matthew Rosato <mjrosato@linux.ibm.com> > > > > Create new files for separating out vfio-specific work for s390 > > pci. Add the first such routine, which issues VFIO_IOMMU_GET_INFO > > ioctl to collect the current dma available count. > > > > Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > [aw: Fix non-Linux build with CONFIG_LINUX] > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > > --- > > hw/s390x/meson.build | 1 + > > hw/s390x/s390-pci-vfio.c | 54 ++++++++++++++++++++++++++++++++++++++ > > include/hw/s390x/s390-pci-vfio.h | 24 +++++++++++++++++ > > 3 files changed, 79 insertions(+) > > create mode 100644 hw/s390x/s390-pci-vfio.c > > create mode 100644 include/hw/s390x/s390-pci-vfio.h > > > > diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build > > index 948ceae7a7b3..f4663a835514 100644 > > --- a/hw/s390x/meson.build > > +++ b/hw/s390x/meson.build > > @@ -27,6 +27,7 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( > > )) > > s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c')) > > s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c')) > > +s390x_ss.add(when: 'CONFIG_LINUX', if_true: files('s390-pci-vfio.c')) I think we need s/CONFIG_LINUX/CONFIG_VFIO/ here. > > > > virtio_ss = ss.source_set() > > virtio_ss.add(files('virtio-ccw.c')) > > diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c > > new file mode 100644 > > index 000000000000..cb3f4d98adf8 > > --- /dev/null > > +++ b/hw/s390x/s390-pci-vfio.c > > @@ -0,0 +1,54 @@ > > +/* > > + * 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 "qemu/osdep.h" > > +#include "hw/s390x/s390-pci-vfio.h" > > +#include "hw/vfio/vfio-common.h" > > + > > +/* > > + * Get the current DMA available count from vfio. Returns true if vfio is > > + * limiting DMA requests, false otherwise. The current available count read > > + * from vfio is returned in avail. > > + */ > > +bool s390_pci_update_dma_avail(int fd, unsigned int *avail) > > +{ > > + g_autofree struct vfio_iommu_type1_info *info; > > + uint32_t argsz; > > + > > + assert(avail); > > + > > + argsz = sizeof(struct vfio_iommu_type1_info); > > + info = g_malloc0(argsz); > > + > > + /* > > + * If the specified argsz is not large enough to contain all capabilities > > + * it will be updated upon return from the ioctl. Retry until we have > > + * a big enough buffer to hold the entire capability chain. > > + */ > > +retry: > > + info->argsz = argsz; > > + > > + if (ioctl(fd, VFIO_IOMMU_GET_INFO, info)) { > > + return false; > > + } > > + > > + if (info->argsz > argsz) { > > + argsz = info->argsz; > > + info = g_realloc(info, argsz); > > + goto retry; > > + } > > + > > + /* If the capability exists, update with the current value */ > > + return vfio_get_info_dma_avail(info, avail); > > +} > > --without-default-devices configuration is broken, bisected > to this commit: Hm. Is this run in any of the CI tests? I did not even realize it existed :) > > hw/s390x/s390-pci-vfio.c:52: undefined reference to > `vfio_get_info_dma_avail' > > Can you have a look please? > > Thanks. >
On 11/3/20 11:45 AM, Cornelia Huck wrote: > On Tue, 3 Nov 2020 06:49:13 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> Hi Matthew, >> >> On 11/1/20 10:02 PM, Alex Williamson wrote: >>> From: Matthew Rosato <mjrosato@linux.ibm.com> >>> >>> Create new files for separating out vfio-specific work for s390 >>> pci. Add the first such routine, which issues VFIO_IOMMU_GET_INFO >>> ioctl to collect the current dma available count. >>> >>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>> [aw: Fix non-Linux build with CONFIG_LINUX] >>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> >>> --- >>> hw/s390x/meson.build | 1 + >>> hw/s390x/s390-pci-vfio.c | 54 ++++++++++++++++++++++++++++++++++++++ >>> include/hw/s390x/s390-pci-vfio.h | 24 +++++++++++++++++ >>> 3 files changed, 79 insertions(+) >>> create mode 100644 hw/s390x/s390-pci-vfio.c >>> create mode 100644 include/hw/s390x/s390-pci-vfio.h >>> >>> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build >>> index 948ceae7a7b3..f4663a835514 100644 >>> --- a/hw/s390x/meson.build >>> +++ b/hw/s390x/meson.build >>> @@ -27,6 +27,7 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( >>> )) >>> s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c')) >>> s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c')) >>> +s390x_ss.add(when: 'CONFIG_LINUX', if_true: files('s390-pci-vfio.c')) > > I think we need s/CONFIG_LINUX/CONFIG_VFIO/ here. > >>> >>> virtio_ss = ss.source_set() >>> virtio_ss.add(files('virtio-ccw.c')) >>> diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c >>> new file mode 100644 >>> index 000000000000..cb3f4d98adf8 >>> --- /dev/null >>> +++ b/hw/s390x/s390-pci-vfio.c >>> @@ -0,0 +1,54 @@ >>> +/* >>> + * 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 "qemu/osdep.h" >>> +#include "hw/s390x/s390-pci-vfio.h" >>> +#include "hw/vfio/vfio-common.h" >>> + >>> +/* >>> + * Get the current DMA available count from vfio. Returns true if vfio is >>> + * limiting DMA requests, false otherwise. The current available count read >>> + * from vfio is returned in avail. >>> + */ >>> +bool s390_pci_update_dma_avail(int fd, unsigned int *avail) >>> +{ >>> + g_autofree struct vfio_iommu_type1_info *info; >>> + uint32_t argsz; >>> + >>> + assert(avail); >>> + >>> + argsz = sizeof(struct vfio_iommu_type1_info); >>> + info = g_malloc0(argsz); >>> + >>> + /* >>> + * If the specified argsz is not large enough to contain all capabilities >>> + * it will be updated upon return from the ioctl. Retry until we have >>> + * a big enough buffer to hold the entire capability chain. >>> + */ >>> +retry: >>> + info->argsz = argsz; >>> + >>> + if (ioctl(fd, VFIO_IOMMU_GET_INFO, info)) { >>> + return false; >>> + } >>> + >>> + if (info->argsz > argsz) { >>> + argsz = info->argsz; >>> + info = g_realloc(info, argsz); >>> + goto retry; >>> + } >>> + >>> + /* If the capability exists, update with the current value */ >>> + return vfio_get_info_dma_avail(info, avail); >>> +} >> >> --without-default-devices configuration is broken, bisected >> to this commit: > > Hm. Is this run in any of the CI tests? I did not even realize it > existed :) Paolo added it to Travis-CI 22 months ago: commit 20885b5b1697247038018f3a7dfe8a84adf1b8e9 Author: Paolo Bonzini <pbonzini@redhat.com> Date: Thu Jan 31 15:57:53 2019 +0100 .travis.yml: test that no-default-device builds do not regress Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> diff --git a/.travis.yml b/.travis.yml index cca57f43148..e942175dd30 100644 --- a/.travis.yml +++ b/.travis.yml @@ -149,6 +149,12 @@ matrix: - ${SRC_DIR}/scripts/travis/coverage-summary.sh + # We manually include builds which we disable "make check" for + - env: + - CONFIG="--without-default-devices" + - TEST_CMD="" + + # We manually include builds which we disable "make check" for - env: - CONFIG="--enable-debug --enable-tcg-interpreter" --- > >> >> hw/s390x/s390-pci-vfio.c:52: undefined reference to >> `vfio_get_info_dma_avail' >> >> Can you have a look please? >> >> Thanks. >> >
On 11/3/20 11:45 AM, Cornelia Huck wrote: > On Tue, 3 Nov 2020 06:49:13 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > >> Hi Matthew, >> >> On 11/1/20 10:02 PM, Alex Williamson wrote: >>> From: Matthew Rosato <mjrosato@linux.ibm.com> >>> >>> Create new files for separating out vfio-specific work for s390 >>> pci. Add the first such routine, which issues VFIO_IOMMU_GET_INFO >>> ioctl to collect the current dma available count. >>> >>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> >>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> >>> [aw: Fix non-Linux build with CONFIG_LINUX] >>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> >>> --- >>> hw/s390x/meson.build | 1 + >>> hw/s390x/s390-pci-vfio.c | 54 ++++++++++++++++++++++++++++++++++++++ >>> include/hw/s390x/s390-pci-vfio.h | 24 +++++++++++++++++ >>> 3 files changed, 79 insertions(+) >>> create mode 100644 hw/s390x/s390-pci-vfio.c >>> create mode 100644 include/hw/s390x/s390-pci-vfio.h >>> >>> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build >>> index 948ceae7a7b3..f4663a835514 100644 >>> --- a/hw/s390x/meson.build >>> +++ b/hw/s390x/meson.build >>> @@ -27,6 +27,7 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( >>> )) >>> s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c')) >>> s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c')) >>> +s390x_ss.add(when: 'CONFIG_LINUX', if_true: files('s390-pci-vfio.c')) > > I think we need s/CONFIG_LINUX/CONFIG_VFIO/ here. With your change: hw/s390x/s390-pci-bus.c:1079: undefined reference to `s390_pci_end_dma_count' hw/s390x/s390-pci-bus.c:1019: undefined reference to `s390_pci_start_dma_count' hw/s390x/s390-pci-bus.c:1021: undefined reference to `s390_pci_get_clp_info' Do we need stubs instead?
On Tue, 3 Nov 2020 12:08:20 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > On 11/3/20 11:45 AM, Cornelia Huck wrote: > > On Tue, 3 Nov 2020 06:49:13 +0100 > > Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > > >> Hi Matthew, > >> > >> On 11/1/20 10:02 PM, Alex Williamson wrote: > >>> From: Matthew Rosato <mjrosato@linux.ibm.com> > >>> > >>> Create new files for separating out vfio-specific work for s390 > >>> pci. Add the first such routine, which issues VFIO_IOMMU_GET_INFO > >>> ioctl to collect the current dma available count. > >>> > >>> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com> > >>> Reviewed-by: Cornelia Huck <cohuck@redhat.com> > >>> [aw: Fix non-Linux build with CONFIG_LINUX] > >>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > >>> --- > >>> hw/s390x/meson.build | 1 + > >>> hw/s390x/s390-pci-vfio.c | 54 ++++++++++++++++++++++++++++++++++++++ > >>> include/hw/s390x/s390-pci-vfio.h | 24 +++++++++++++++++ > >>> 3 files changed, 79 insertions(+) > >>> create mode 100644 hw/s390x/s390-pci-vfio.c > >>> create mode 100644 include/hw/s390x/s390-pci-vfio.h > >>> > >>> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build > >>> index 948ceae7a7b3..f4663a835514 100644 > >>> --- a/hw/s390x/meson.build > >>> +++ b/hw/s390x/meson.build > >>> @@ -27,6 +27,7 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( > >>> )) > >>> s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c')) > >>> s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c')) > >>> +s390x_ss.add(when: 'CONFIG_LINUX', if_true: files('s390-pci-vfio.c')) > > > > I think we need s/CONFIG_LINUX/CONFIG_VFIO/ here. > > With your change: > > hw/s390x/s390-pci-bus.c:1079: undefined reference to > `s390_pci_end_dma_count' > hw/s390x/s390-pci-bus.c:1019: undefined reference to > `s390_pci_start_dma_count' > hw/s390x/s390-pci-bus.c:1021: undefined reference to `s390_pci_get_clp_info' > > Do we need stubs instead? > We do have stubs; we just need them to be ifdeffed with CONFIG_VFIO instead of CONFIG_LINUX. Currently preparing a proper patch.
diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build index 948ceae7a7b3..f4663a835514 100644 --- a/hw/s390x/meson.build +++ b/hw/s390x/meson.build @@ -27,6 +27,7 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files( )) s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c')) s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c')) +s390x_ss.add(when: 'CONFIG_LINUX', if_true: files('s390-pci-vfio.c')) virtio_ss = ss.source_set() virtio_ss.add(files('virtio-ccw.c')) diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c new file mode 100644 index 000000000000..cb3f4d98adf8 --- /dev/null +++ b/hw/s390x/s390-pci-vfio.c @@ -0,0 +1,54 @@ +/* + * 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 "qemu/osdep.h" +#include "hw/s390x/s390-pci-vfio.h" +#include "hw/vfio/vfio-common.h" + +/* + * Get the current DMA available count from vfio. Returns true if vfio is + * limiting DMA requests, false otherwise. The current available count read + * from vfio is returned in avail. + */ +bool s390_pci_update_dma_avail(int fd, unsigned int *avail) +{ + g_autofree struct vfio_iommu_type1_info *info; + uint32_t argsz; + + assert(avail); + + argsz = sizeof(struct vfio_iommu_type1_info); + info = g_malloc0(argsz); + + /* + * If the specified argsz is not large enough to contain all capabilities + * it will be updated upon return from the ioctl. Retry until we have + * a big enough buffer to hold the entire capability chain. + */ +retry: + info->argsz = argsz; + + if (ioctl(fd, VFIO_IOMMU_GET_INFO, info)) { + return false; + } + + if (info->argsz > argsz) { + argsz = info->argsz; + info = g_realloc(info, argsz); + goto retry; + } + + /* If the capability exists, update with the current value */ + return vfio_get_info_dma_avail(info, avail); +} + diff --git a/include/hw/s390x/s390-pci-vfio.h b/include/hw/s390x/s390-pci-vfio.h new file mode 100644 index 000000000000..1727292e9b5d --- /dev/null +++ b/include/hw/s390x/s390-pci-vfio.h @@ -0,0 +1,24 @@ +/* + * 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. + */ + +#ifndef HW_S390_PCI_VFIO_H +#define HW_S390_PCI_VFIO_H + +#ifdef CONFIG_LINUX +bool s390_pci_update_dma_avail(int fd, unsigned int *avail); +#else +static inline bool s390_pci_update_dma_avail(int fd, unsigned int *avail) +{ + return false; +} +#endif + +#endif