| Message ID | 160374054442.22414.10832953989449611268.stgit@gimli.home |
|---|---|
| State | New |
| Headers | show |
On Mon, 26 Oct 2020 at 19:39, Alex Williamson <alex.williamson@redhat.com> wrote: > ---------------------------------------------------------------- > VFIO update 2020-10-26 > > * Migration support (Kirti Wankhede) > * s390 DMA limiting (Matthew Rosato) > * zPCI hardware info (Matthew Rosato) > * Lock guard (Amey Narkhede) > * Print fixes (Zhengui li) I get a conflict here in include/standard-headers/linux/fuse.h: ++<<<<<<< HEAD +#define FUSE_ATTR_FLAGS (1 << 27) ++======= + #define FUSE_SUBMOUNTS (1 << 27) ++>>>>>>> remotes/awilliam/tags/vfio-update-20201026.0 I assume these should not both be trying to use the same value, so something has gone wrong somewhere. The conflicting commit now in master is Max's 97d741cc96dd08 ("linux/fuse.h: Pull in from Linux"). Can you sort out the correct resolution between you, please? (My guess is that Max's commit is the erroneous one because it doesn't look like it was created via a standard update from the kernel headers.) thanks -- PMM
On Tue, 27 Oct 2020 23:42:57 +0000 Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 26 Oct 2020 at 19:39, Alex Williamson > <alex.williamson@redhat.com> wrote: > > ---------------------------------------------------------------- > > VFIO update 2020-10-26 > > > > * Migration support (Kirti Wankhede) > > * s390 DMA limiting (Matthew Rosato) > > * zPCI hardware info (Matthew Rosato) > > * Lock guard (Amey Narkhede) > > * Print fixes (Zhengui li) > > I get a conflict here in > include/standard-headers/linux/fuse.h: > > ++<<<<<<< HEAD > +#define FUSE_ATTR_FLAGS (1 << 27) > ++======= > + #define FUSE_SUBMOUNTS (1 << 27) > ++>>>>>>> remotes/awilliam/tags/vfio-update-20201026.0 > > I assume these should not both be trying to use the same value, > so something has gone wrong somewhere. The conflicting commit > now in master is Max's 97d741cc96dd08 ("linux/fuse.h: Pull in from Linux"). > > Can you sort out the correct resolution between you, please? > (My guess is that Max's commit is the erroneous one because > it doesn't look like it was created via a standard update > from the kernel headers.) So as near as I can tell, QEMU commit 97d741cc96dd ("linux/fuse.h: Pull in from Linux") is fantasy land. The only thing I can find of this FUSE_ATTR_FLAGS outside Max's QEMU series is this[1] posting where the fuse maintainer announces that he's replaced FUSE_ATTR_FLAGS with FUSE_SUBMOUNTS, but the usage is "slightly different". Reading that thread, it seems that virtiofsd probably needed an update but I can't see that it ever happened. I'm not comfortable trying to update Max's series to try to determine if FUSE_SUBMOUNTS can be interchanged with FUSE_ATTR_FLAGS, where the latter appears to be used to express the new field in struct fuse_attr exists, but the former appears to be a feature. My guess would be that maybe FUSE_KERNEL_MINOR_VERSION needs to be tested instead for this new field?? Anyway, I hate to pull the big hammer, but I think Max's series is bogus. The only thing I can propose is to revert it in its entirety, after which this series applies cleanly. I'll post a patch to do that as I think the code currently in master is on pretty shaky ground with respect to interpreting flag bits differently from those the kernel defines. Thanks, Alex [1] https://marc.info/?l=fuse-devel&m=160069539811247
On Tue, 27 Oct 2020 23:42:57 +0000 Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 26 Oct 2020 at 19:39, Alex Williamson > <alex.williamson@redhat.com> wrote: > > ---------------------------------------------------------------- > > VFIO update 2020-10-26 > > > > * Migration support (Kirti Wankhede) > > * s390 DMA limiting (Matthew Rosato) > > * zPCI hardware info (Matthew Rosato) > > * Lock guard (Amey Narkhede) > > * Print fixes (Zhengui li) > > I get a conflict here in > include/standard-headers/linux/fuse.h: > > ++<<<<<<< HEAD > +#define FUSE_ATTR_FLAGS (1 << 27) > ++======= > + #define FUSE_SUBMOUNTS (1 << 27) > ++>>>>>>> remotes/awilliam/tags/vfio-update-20201026.0 > > I assume these should not both be trying to use the same value, > so something has gone wrong somewhere. The conflicting commit > now in master is Max's 97d741cc96dd08 ("linux/fuse.h: Pull in from Linux"). > > Can you sort out the correct resolution between you, please? > (My guess is that Max's commit is the erroneous one because > it doesn't look like it was created via a standard update > from the kernel headers.) We should never change things in the synced headers other than via a headers update (excluding fixups of prior messes.) I'm pointing it out whenever I see something like that happening, but nobody is going to catch all of those. Is there any place where we can have some kind of automatic check on a pull request for that kind of stuff? We'd need to formalize an "update headers" commit message, or maybe have the update script write some kind of "last updated" file?
* Alex Williamson (alex.williamson@redhat.com) wrote: > On Tue, 27 Oct 2020 23:42:57 +0000 > Peter Maydell <peter.maydell@linaro.org> wrote: > > > On Mon, 26 Oct 2020 at 19:39, Alex Williamson > > <alex.williamson@redhat.com> wrote: > > > ---------------------------------------------------------------- > > > VFIO update 2020-10-26 > > > > > > * Migration support (Kirti Wankhede) > > > * s390 DMA limiting (Matthew Rosato) > > > * zPCI hardware info (Matthew Rosato) > > > * Lock guard (Amey Narkhede) > > > * Print fixes (Zhengui li) > > > > I get a conflict here in > > include/standard-headers/linux/fuse.h: > > > > ++<<<<<<< HEAD > > +#define FUSE_ATTR_FLAGS (1 << 27) > > ++======= > > + #define FUSE_SUBMOUNTS (1 << 27) > > ++>>>>>>> remotes/awilliam/tags/vfio-update-20201026.0 > > > > I assume these should not both be trying to use the same value, > > so something has gone wrong somewhere. The conflicting commit > > now in master is Max's 97d741cc96dd08 ("linux/fuse.h: Pull in from Linux"). > > > > Can you sort out the correct resolution between you, please? > > (My guess is that Max's commit is the erroneous one because > > it doesn't look like it was created via a standard update > > from the kernel headers.) Eww that's messy; copying in Miklos to see what's going on. > So as near as I can tell, QEMU commit 97d741cc96dd ("linux/fuse.h: Pull > in from Linux") is fantasy land. The only thing I can find of this > FUSE_ATTR_FLAGS outside Max's QEMU series is this[1] posting where the > fuse maintainer announces that he's replaced FUSE_ATTR_FLAGS with > FUSE_SUBMOUNTS, but the usage is "slightly different". Reading that > thread, it seems that virtiofsd probably needed an update but I can't > see that it ever happened. > > I'm not comfortable trying to update Max's series to try to determine > if FUSE_SUBMOUNTS can be interchanged with FUSE_ATTR_FLAGS, where the > latter appears to be used to express the new field in struct fuse_attr > exists, but the former appears to be a feature. My guess would be that > maybe FUSE_KERNEL_MINOR_VERSION needs to be tested instead for this new > field?? They're part of the init flags sent in the negotiation; so they should be fine. > Anyway, I hate to pull the big hammer, but I think Max's series is > bogus. The only thing I can propose is to revert it in its entirety, > after which this series applies cleanly. I'll post a patch to do that > as I think the code currently in master is on pretty shaky ground with > respect to interpreting flag bits differently from those the kernel > defines. Thanks, Yeh the kernel header is king in the definition of those flags. It maybe bet, but I'd like to see what Miklos says. Dave > Alex > > [1] https://marc.info/?l=fuse-devel&m=160069539811247
On 28.10.20 03:00, Alex Williamson wrote: > On Tue, 27 Oct 2020 23:42:57 +0000 > Peter Maydell <peter.maydell@linaro.org> wrote: > >> On Mon, 26 Oct 2020 at 19:39, Alex Williamson >> <alex.williamson@redhat.com> wrote: >>> ---------------------------------------------------------------- >>> VFIO update 2020-10-26 >>> >>> * Migration support (Kirti Wankhede) >>> * s390 DMA limiting (Matthew Rosato) >>> * zPCI hardware info (Matthew Rosato) >>> * Lock guard (Amey Narkhede) >>> * Print fixes (Zhengui li) >> >> I get a conflict here in >> include/standard-headers/linux/fuse.h: >> >> ++<<<<<<< HEAD >> +#define FUSE_ATTR_FLAGS (1 << 27) >> ++======= >> + #define FUSE_SUBMOUNTS (1 << 27) >> ++>>>>>>> remotes/awilliam/tags/vfio-update-20201026.0 Oh no. >> I assume these should not both be trying to use the same value, >> so something has gone wrong somewhere. The conflicting commit >> now in master is Max's 97d741cc96dd08 ("linux/fuse.h: Pull in from Linux"). >> >> Can you sort out the correct resolution between you, please? >> (My guess is that Max's commit is the erroneous one because >> it doesn't look like it was created via a standard update >> from the kernel headers.) It is the erroneous one, because it was based on an earlier version of the kernel series. > So as near as I can tell, QEMU commit 97d741cc96dd ("linux/fuse.h: Pull > in from Linux") is fantasy land. The only thing I can find of this > FUSE_ATTR_FLAGS outside Max's QEMU series is this[1] posting where the > fuse maintainer announces that he's replaced FUSE_ATTR_FLAGS with > FUSE_SUBMOUNTS, but the usage is "slightly different". Reading that > thread, it seems that virtiofsd probably needed an update but I can't > see that it ever happened. No, it didn't happen yet. The series should have got a v2. As an alternative to reverting, I could try to fix it up on top, but I don't think that's really preferable. So I would vote for reverting. > I'm not comfortable trying to update Max's series to try to determine > if FUSE_SUBMOUNTS can be interchanged with FUSE_ATTR_FLAGS, where the > latter appears to be used to express the new field in struct fuse_attr > exists, but the former appears to be a feature. My guess would be that > maybe FUSE_KERNEL_MINOR_VERSION needs to be tested instead for this new > field?? It can't be interchanged 1:1. The series should be updated, but not with such a hack as using some other indicator to see whether the flag is there, but with properly using FUSE_SUBMOUNTS. (I suppose technically it's OK for the virtiofsd side to interpret FUSE_SUBMOUNTS as meaning the field to be present, because FUSE_SUBMOUNTS does imply that. But I wouldn't want to test that hypothesis, and instead just write a clean v2.) > Anyway, I hate to pull the big hammer, but I think Max's series is > bogus. The only thing I can propose is to revert it in its entirety, > after which this series applies cleanly. I'll post a patch to do that > as I think the code currently in master is on pretty shaky ground with > respect to interpreting flag bits differently from those the kernel > defines. Sounds, well, not good, but definitely reasonable. Thanks! Max
On 28.10.20 09:11, Cornelia Huck wrote: > On Tue, 27 Oct 2020 23:42:57 +0000 > Peter Maydell <peter.maydell@linaro.org> wrote: > >> On Mon, 26 Oct 2020 at 19:39, Alex Williamson >> <alex.williamson@redhat.com> wrote: >>> ---------------------------------------------------------------- >>> VFIO update 2020-10-26 >>> >>> * Migration support (Kirti Wankhede) >>> * s390 DMA limiting (Matthew Rosato) >>> * zPCI hardware info (Matthew Rosato) >>> * Lock guard (Amey Narkhede) >>> * Print fixes (Zhengui li) >> >> I get a conflict here in >> include/standard-headers/linux/fuse.h: >> >> ++<<<<<<< HEAD >> +#define FUSE_ATTR_FLAGS (1 << 27) >> ++======= >> + #define FUSE_SUBMOUNTS (1 << 27) >> ++>>>>>>> remotes/awilliam/tags/vfio-update-20201026.0 >> >> I assume these should not both be trying to use the same value, >> so something has gone wrong somewhere. The conflicting commit >> now in master is Max's 97d741cc96dd08 ("linux/fuse.h: Pull in from Linux"). >> >> Can you sort out the correct resolution between you, please? >> (My guess is that Max's commit is the erroneous one because >> it doesn't look like it was created via a standard update >> from the kernel headers.) > > We should never change things in the synced headers other than via a > headers update (excluding fixups of prior messes.) I'm pointing it out > whenever I see something like that happening, but nobody is going to > catch all of those. Well, it was a kernel update. Just based on a preliminary version of the kernel part of the FUSE submount feature. It was clear that the kernel part would have to be merged before the qemu/virtiofsd series, and that did happen, but Miklos (the FUSE maintainer) fixed some things on top while doing so, an that included changing the flag in question. As Adam wrote, I noted that I would thus have to write a v2 of the virtiofsd series. Unfortunately, that all was a bit buried in the thread, so I suppose for Dave it looked like the kernel series was applied, so the virtiofsd series could go in, too. And I in turn didn't catch that. :/ > Is there any place where we can have some kind of automatic check on a > pull request for that kind of stuff? We'd need to formalize an "update > headers" commit message, or maybe have the update script write some > kind of "last updated" file? It would also need to actually check against the kernel tree, because, well, I did use the script. Just against a kernel tree that never came to master. Max
> changing the flag in question. As Adam wrote, I noted that I would thus
*Alex, sorry :/
On Wed, 28 Oct 2020 10:28:39 +0100 Max Reitz <mreitz@redhat.com> wrote: > On 28.10.20 09:11, Cornelia Huck wrote: > > On Tue, 27 Oct 2020 23:42:57 +0000 > > Peter Maydell <peter.maydell@linaro.org> wrote: > > > >> On Mon, 26 Oct 2020 at 19:39, Alex Williamson > >> <alex.williamson@redhat.com> wrote: > >>> ---------------------------------------------------------------- > >>> VFIO update 2020-10-26 > >>> > >>> * Migration support (Kirti Wankhede) > >>> * s390 DMA limiting (Matthew Rosato) > >>> * zPCI hardware info (Matthew Rosato) > >>> * Lock guard (Amey Narkhede) > >>> * Print fixes (Zhengui li) > >> > >> I get a conflict here in > >> include/standard-headers/linux/fuse.h: > >> > >> ++<<<<<<< HEAD > >> +#define FUSE_ATTR_FLAGS (1 << 27) > >> ++======= > >> + #define FUSE_SUBMOUNTS (1 << 27) > >> ++>>>>>>> remotes/awilliam/tags/vfio-update-20201026.0 > >> > >> I assume these should not both be trying to use the same value, > >> so something has gone wrong somewhere. The conflicting commit > >> now in master is Max's 97d741cc96dd08 ("linux/fuse.h: Pull in from Linux"). > >> > >> Can you sort out the correct resolution between you, please? > >> (My guess is that Max's commit is the erroneous one because > >> it doesn't look like it was created via a standard update > >> from the kernel headers.) > > > > We should never change things in the synced headers other than via a > > headers update (excluding fixups of prior messes.) I'm pointing it out > > whenever I see something like that happening, but nobody is going to > > catch all of those. > > Well, it was a kernel update. Just based on a preliminary version of > the kernel part of the FUSE submount feature. > > It was clear that the kernel part would have to be merged before the > qemu/virtiofsd series, and that did happen, but Miklos (the FUSE > maintainer) fixed some things on top while doing so, an that included > changing the flag in question. As Adam wrote, I noted that I would thus > have to write a v2 of the virtiofsd series. > > Unfortunately, that all was a bit buried in the thread, so I suppose for > Dave it looked like the kernel series was applied, so the virtiofsd > series could go in, too. And I in turn didn't catch that. :/ Yeah, things like that happen, especially if explanations are buried deeply somewhere :/ > > > Is there any place where we can have some kind of automatic check on a > > pull request for that kind of stuff? We'd need to formalize an "update > > headers" commit message, or maybe have the update script write some > > kind of "last updated" file? > It would also need to actually check against the kernel tree, because, > well, I did use the script. Just against a kernel tree that never came > to master. Hm. That's probably hard to get right, unless we require all updates to be against a tagged kernel (-rc) version. Not sure if that's too strict.
On Mon, 26 Oct 2020 at 19:39, Alex Williamson <alex.williamson@redhat.com> wrote: > > The following changes since commit a5fac424c76d6401ecde4ecb7d846e656d0d6e89: > > Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2020-10-26 10:33:59 +0000) > > are available in the Git repository at: > > git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20201026.0 > > for you to fetch changes up to 5219bf8e0fa86573427aa8812bbfe93d83c3d664: > > vfio: fix incorrect print type (2020-10-26 12:07:46 -0600) > > ---------------------------------------------------------------- > VFIO update 2020-10-26 > > * Migration support (Kirti Wankhede) > * s390 DMA limiting (Matthew Rosato) > * zPCI hardware info (Matthew Rosato) > * Lock guard (Amey Narkhede) > * Print fixes (Zhengui li) > I retried the merge of this after the revert from Max, and it no longer gives merge conflicts, but it has compile errors: On FreeBSD, OpenBSD, NetBSD, OSX and Windows: In file included from ../src/hw/arm/sysbus-fdt.c:35: In file included from /usr/home/qemu/qemu-test.ffr5Sp/src/include/hw/vfio/vfio-platform.h:20: /usr/home/qemu/qemu-test.ffr5Sp/src/include/hw/vfio/vfio-common.h:201:37: warning: declaration of 'struct vfio_iommu_type1_info' will not be visible outside of this function [-Wvisibility] bool vfio_get_info_dma_avail(struct vfio_iommu_type1_info *info, ^ On clang x86-64 Linux: ../../hw/vfio/migration.c:737:42: error: equality comparison with extraneous parentheses [-Werror,-Wparentheses-equality] if ((vbasedev->migration->vm_running == running)) { ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~ ../../hw/vfio/migration.c:737:42: note: remove extraneous parentheses around the comparison to silence this warning if ((vbasedev->migration->vm_running == running)) { ~ ^ ~ ../../hw/vfio/migration.c:737:42: note: use '=' to turn this equality comparison into an assignment if ((vbasedev->migration->vm_running == running)) { ^~ = On AArch32: ../../hw/vfio/migration.c: In function 'vfio_mig_access': ../../hw/vfio/migration.c:58:68: error: format '%lx' expects argument of type 'long unsigned int', but argument 5 has type 'off_t {aka long long int}' [-Werror=format=] error_report("vfio_mig_%s %d byte %s: failed at offset 0x%lx, err: %s", ~~^ %llx cc1: all warnings being treated as errors On PPC64: ../../hw/vfio/common.c: In function ‘vfio_dma_unmap_bitmap’: ../../hw/vfio/common.c:400:9: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ [-Werror=format=] error_report("UNMAP: Size of bitmap too big 0x%llx", bitmap->size); ^ ../../hw/vfio/common.c: In function ‘vfio_get_dirty_bitmap’: ../../hw/vfio/common.c:1003:17: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ [-Werror=format=] range->iova, range->size, errno); ^ ../../hw/vfio/common.c:1003:17: error: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘__u64’ [-Werror=format=] thanks -- PMM
On 10/28/20 11:07 AM, Peter Maydell wrote: > On Mon, 26 Oct 2020 at 19:39, Alex Williamson > <alex.williamson@redhat.com> wrote: >> >> The following changes since commit a5fac424c76d6401ecde4ecb7d846e656d0d6e89: >> >> Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2020-10-26 10:33:59 +0000) >> >> are available in the Git repository at: >> >> git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20201026.0 >> >> for you to fetch changes up to 5219bf8e0fa86573427aa8812bbfe93d83c3d664: >> >> vfio: fix incorrect print type (2020-10-26 12:07:46 -0600) >> >> ---------------------------------------------------------------- >> VFIO update 2020-10-26 >> >> * Migration support (Kirti Wankhede) >> * s390 DMA limiting (Matthew Rosato) >> * zPCI hardware info (Matthew Rosato) >> * Lock guard (Amey Narkhede) >> * Print fixes (Zhengui li) >> > > I retried the merge of this after the revert from Max, and it > no longer gives merge conflicts, but it has compile errors: > > On FreeBSD, OpenBSD, NetBSD, OSX and Windows: > > In file included from ../src/hw/arm/sysbus-fdt.c:35: > In file included from > /usr/home/qemu/qemu-test.ffr5Sp/src/include/hw/vfio/vfio-platform.h:20: > /usr/home/qemu/qemu-test.ffr5Sp/src/include/hw/vfio/vfio-common.h:201:37: > warning: declaration of 'struct vfio_iommu_type1_info' will not be > visible outside of this function [-Wvisibility] > bool vfio_get_info_dma_avail(struct vfio_iommu_type1_info *info, > ^ Alex, for this one I think the definition of bool vfio_get_info_dma_avail(struct vfio_iommu_type1_info *info, unsigned int *avail); in vfio-common.h needs to be behind the #ifdef CONFIG_LINUX as that's the only case where we include vfio.h where vfio_iommu_type1_info is defined. > > On clang x86-64 Linux: > > ../../hw/vfio/migration.c:737:42: error: equality comparison with > extraneous parentheses [-Werror,-Wparentheses-equality] > if ((vbasedev->migration->vm_running == running)) { > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~ > ../../hw/vfio/migration.c:737:42: note: remove extraneous parentheses > around the comparison to silence this warning > if ((vbasedev->migration->vm_running == running)) { > ~ ^ ~ > ../../hw/vfio/migration.c:737:42: note: use '=' to turn this equality > comparison into an assignment > if ((vbasedev->migration->vm_running == running)) { > ^~ > = > > On AArch32: > > ../../hw/vfio/migration.c: In function 'vfio_mig_access': > ../../hw/vfio/migration.c:58:68: error: format '%lx' expects argument > of type 'long unsigned int', but argument 5 has type 'off_t {aka long > long int}' [-Werror=format=] > error_report("vfio_mig_%s %d byte %s: failed at offset 0x%lx, err: %s", > ~~^ > %llx > cc1: all warnings being treated as errors > > > On PPC64: > > ../../hw/vfio/common.c: In function ‘vfio_dma_unmap_bitmap’: > ../../hw/vfio/common.c:400:9: error: format ‘%llx’ expects argument of > type ‘long long unsigned int’, but argument 2 has type ‘__u64’ > [-Werror=format=] > error_report("UNMAP: Size of bitmap too big 0x%llx", bitmap->size); > ^ > ../../hw/vfio/common.c: In function ‘vfio_get_dirty_bitmap’: > ../../hw/vfio/common.c:1003:17: error: format ‘%llx’ expects argument > of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ > [-Werror=format=] > range->iova, range->size, errno); > ^ > ../../hw/vfio/common.c:1003:17: error: format ‘%llx’ expects argument > of type ‘long long unsigned int’, but argument 3 has type ‘__u64’ > [-Werror=format=] > > thanks > -- PMM >
On Wed, Oct 28, 2020 at 10:19 AM Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: > > > I'm not comfortable trying to update Max's series to try to determine > > if FUSE_SUBMOUNTS can be interchanged with FUSE_ATTR_FLAGS, where the FUSE_SUBMOUNTS is the correct one, FUSE_ATTR_FLAGS was never merged into mainline linux. The only difference is that FUSE_ATTR_FLAGS was meant to be negotiated (AFAIR), while FUSE_SUBMOUNTS is just announcing that the client supports submounts and will honour the FUSE_ATTR_SUBMOUNT flag from the server. Thanks, Miklos
On Wed, 28 Oct 2020 11:20:36 -0400 Matthew Rosato <mjrosato@linux.ibm.com> wrote: > On 10/28/20 11:07 AM, Peter Maydell wrote: > > On Mon, 26 Oct 2020 at 19:39, Alex Williamson > > <alex.williamson@redhat.com> wrote: > >> > >> The following changes since commit a5fac424c76d6401ecde4ecb7d846e656d0d6e89: > >> > >> Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2020-10-26 10:33:59 +0000) > >> > >> are available in the Git repository at: > >> > >> git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20201026.0 > >> > >> for you to fetch changes up to 5219bf8e0fa86573427aa8812bbfe93d83c3d664: > >> > >> vfio: fix incorrect print type (2020-10-26 12:07:46 -0600) > >> > >> ---------------------------------------------------------------- > >> VFIO update 2020-10-26 > >> > >> * Migration support (Kirti Wankhede) > >> * s390 DMA limiting (Matthew Rosato) > >> * zPCI hardware info (Matthew Rosato) > >> * Lock guard (Amey Narkhede) > >> * Print fixes (Zhengui li) > >> > > > > I retried the merge of this after the revert from Max, and it > > no longer gives merge conflicts, but it has compile errors: > > > > On FreeBSD, OpenBSD, NetBSD, OSX and Windows: > > > > In file included from ../src/hw/arm/sysbus-fdt.c:35: > > In file included from > > /usr/home/qemu/qemu-test.ffr5Sp/src/include/hw/vfio/vfio-platform.h:20: > > /usr/home/qemu/qemu-test.ffr5Sp/src/include/hw/vfio/vfio-common.h:201:37: > > warning: declaration of 'struct vfio_iommu_type1_info' will not be > > visible outside of this function [-Wvisibility] > > bool vfio_get_info_dma_avail(struct vfio_iommu_type1_info *info, > > ^ > > Alex, for this one I think the definition of > bool vfio_get_info_dma_avail(struct vfio_iommu_type1_info *info, > unsigned int *avail); > > in vfio-common.h needs to be behind the #ifdef CONFIG_LINUX as that's > the only case where we include vfio.h where vfio_iommu_type1_info is > defined. Thank you. I'll update and repost, I think the others below are resolved using HWADDR_PRIx and PRIx64, plus removing the redundant parens. Thanks, Alex > > On clang x86-64 Linux: > > > > ../../hw/vfio/migration.c:737:42: error: equality comparison with > > extraneous parentheses [-Werror,-Wparentheses-equality] > > if ((vbasedev->migration->vm_running == running)) { > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~ > > ../../hw/vfio/migration.c:737:42: note: remove extraneous parentheses > > around the comparison to silence this warning > > if ((vbasedev->migration->vm_running == running)) { > > ~ ^ ~ > > ../../hw/vfio/migration.c:737:42: note: use '=' to turn this equality > > comparison into an assignment > > if ((vbasedev->migration->vm_running == running)) { > > ^~ > > = > > > > On AArch32: > > > > ../../hw/vfio/migration.c: In function 'vfio_mig_access': > > ../../hw/vfio/migration.c:58:68: error: format '%lx' expects argument > > of type 'long unsigned int', but argument 5 has type 'off_t {aka long > > long int}' [-Werror=format=] > > error_report("vfio_mig_%s %d byte %s: failed at offset 0x%lx, err: %s", > > ~~^ > > %llx > > cc1: all warnings being treated as errors > > > > > > On PPC64: > > > > ../../hw/vfio/common.c: In function ‘vfio_dma_unmap_bitmap’: > > ../../hw/vfio/common.c:400:9: error: format ‘%llx’ expects argument of > > type ‘long long unsigned int’, but argument 2 has type ‘__u64’ > > [-Werror=format=] > > error_report("UNMAP: Size of bitmap too big 0x%llx", bitmap->size); > > ^ > > ../../hw/vfio/common.c: In function ‘vfio_get_dirty_bitmap’: > > ../../hw/vfio/common.c:1003:17: error: format ‘%llx’ expects argument > > of type ‘long long unsigned int’, but argument 2 has type ‘__u64’ > > [-Werror=format=] > > range->iova, range->size, errno); > > ^ > > ../../hw/vfio/common.c:1003:17: error: format ‘%llx’ expects argument > > of type ‘long long unsigned int’, but argument 3 has type ‘__u64’ > > [-Werror=format=] > > > > thanks > > -- PMM > > >
The following changes since commit a5fac424c76d6401ecde4ecb7d846e656d0d6e89: Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/block-pull-request' into staging (2020-10-26 10:33:59 +0000) are available in the Git repository at: git://github.com/awilliam/qemu-vfio.git tags/vfio-update-20201026.0 for you to fetch changes up to 5219bf8e0fa86573427aa8812bbfe93d83c3d664: vfio: fix incorrect print type (2020-10-26 12:07:46 -0600) ---------------------------------------------------------------- VFIO update 2020-10-26 * Migration support (Kirti Wankhede) * s390 DMA limiting (Matthew Rosato) * zPCI hardware info (Matthew Rosato) * Lock guard (Amey Narkhede) * Print fixes (Zhengui li) ---------------------------------------------------------------- Amey Narkhede (1): hw/vfio: Use lock guard macros Kirti Wankhede (17): vfio: Add function to unmap VFIO region vfio: Add vfio_get_object callback to VFIODeviceOps vfio: Add save and load functions for VFIO PCI devices vfio: Add migration region initialization and finalize function vfio: Add VM state change handler to know state of VM vfio: Add migration state change notifier vfio: Register SaveVMHandlers for VFIO device vfio: Add save state functions to SaveVMHandlers vfio: Add load state functions to SaveVMHandlers memory: Set DIRTY_MEMORY_MIGRATION when IOMMU is enabled vfio: Get migration capability flags for container vfio: Add function to start and stop dirty pages tracking vfio: Add vfio_listener_log_sync to mark dirty pages vfio: Dirty page tracking when vIOMMU is enabled vfio: Add ioctl to get dirty pages bitmap during dma unmap vfio: Make vfio-pci device migration capable qapi: Add VFIO devices migration stats in Migration stats Matthew Rosato (10): update-linux-headers: Add vfio_zdev.h linux-headers: update against 5.10-rc1 s390x/pci: Move header files to include/hw/s390x vfio: Create shared routine for scanning info capabilities vfio: Find DMA available capability s390x/pci: Add routine to get the vfio dma available count s390x/pci: Honor DMA limits set by vfio s390x/pci: clean up s390 PCI groups vfio: Add routine for finding VFIO_DEVICE_GET_INFO capabilities s390x/pci: get zPCI function info from host Pierre Morel (3): s390x/pci: create a header dedicated to PCI CLP s390x/pci: use a PCI Group structure s390x/pci: use a PCI Function structure Zhengui Li (1): vfio: fix incorrect print type MAINTAINERS | 1 + hw/s390x/meson.build | 1 + hw/s390x/s390-pci-bus.c | 91 +- hw/s390x/s390-pci-inst.c | 78 +- hw/s390x/s390-pci-vfio.c | 276 ++++++ hw/s390x/s390-virtio-ccw.c | 2 +- hw/s390x/trace-events | 6 + hw/vfio/common.c | 507 ++++++++++- hw/vfio/meson.build | 1 + hw/vfio/migration.c | 933 +++++++++++++++++++++ hw/vfio/pci.c | 87 +- hw/vfio/pci.h | 1 - hw/vfio/platform.c | 7 +- hw/vfio/trace-events | 21 + {hw => include/hw}/s390x/s390-pci-bus.h | 22 + .../hw/s390x/s390-pci-clp.h | 123 +-- include/hw/s390x/s390-pci-inst.h | 119 +++ include/hw/s390x/s390-pci-vfio.h | 23 + include/hw/vfio/vfio-common.h | 30 + .../infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 +- include/standard-headers/linux/ethtool.h | 2 + include/standard-headers/linux/fuse.h | 50 +- include/standard-headers/linux/input-event-codes.h | 4 + include/standard-headers/linux/pci_regs.h | 6 +- include/standard-headers/linux/virtio_fs.h | 3 + include/standard-headers/linux/virtio_gpu.h | 19 + include/standard-headers/linux/virtio_mmio.h | 11 + include/standard-headers/linux/virtio_pci.h | 11 +- linux-headers/asm-arm64/kvm.h | 25 + linux-headers/asm-arm64/mman.h | 1 + linux-headers/asm-generic/hugetlb_encode.h | 1 + linux-headers/asm-generic/unistd.h | 18 +- linux-headers/asm-mips/unistd_n32.h | 1 + linux-headers/asm-mips/unistd_n64.h | 1 + linux-headers/asm-mips/unistd_o32.h | 1 + linux-headers/asm-powerpc/unistd_32.h | 1 + linux-headers/asm-powerpc/unistd_64.h | 1 + linux-headers/asm-s390/unistd_32.h | 1 + linux-headers/asm-s390/unistd_64.h | 1 + linux-headers/asm-x86/kvm.h | 20 + linux-headers/asm-x86/unistd_32.h | 1 + linux-headers/asm-x86/unistd_64.h | 1 + linux-headers/asm-x86/unistd_x32.h | 1 + linux-headers/linux/kvm.h | 19 + linux-headers/linux/mman.h | 1 + linux-headers/linux/vfio.h | 29 +- linux-headers/linux/vfio_zdev.h | 78 ++ migration/migration.c | 17 + monitor/hmp-cmds.c | 6 + qapi/migration.json | 17 + scripts/update-linux-headers.sh | 2 +- softmmu/memory.c | 2 +- 52 files changed, 2466 insertions(+), 217 deletions(-) create mode 100644 hw/s390x/s390-pci-vfio.c create mode 100644 hw/vfio/migration.c rename {hw => include/hw}/s390x/s390-pci-bus.h (94%) rename hw/s390x/s390-pci-inst.h => include/hw/s390x/s390-pci-clp.h (59%) create mode 100644 include/hw/s390x/s390-pci-inst.h create mode 100644 include/hw/s390x/s390-pci-vfio.h create mode 100644 linux-headers/linux/vfio_zdev.h