qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] vfio: Fix handling VFIO_IOMMU_GET_INFO results


From: Alex Williamson
Subject: Re: [Qemu-devel] [PATCH] vfio: Fix handling VFIO_IOMMU_GET_INFO results
Date: Thu, 12 Nov 2015 08:33:28 -0700

On Thu, 2015-11-12 at 17:35 +0300, Pavel Fedin wrote:
>  Hello!
> 
> > > Kernel headers define VFIO_IOMMU_INFO_PGSIZES flag, however it has
> > > actually been never used, probably by mistake which now became a part
> > > of the ABI. The kernel always sets info.flags to 0:
> > 
> > I don't see how this implies that it becomes part of the ABI.  In fact,
> > as the defacto userspace driver for vfio, QEMU honoring the flag and not
> > using the value the kernel provides implies the ABI is still valid.  We
> > should fix the kernel instead.
> 
>  Well... I intentionally put two links to LXR. From the very
> beginning, this ioctl returned valid page sizes. And it never set this
> flag. We simply cannot have a kernel which does not report page sizes.

Isn't that the state we've been in since the very beginning?  QEMU gets
along ok assuming a minimum page size.  If that assumption were to be
invalid, DMA map would fail.  The fact that this bug has been there
since the very beginning and that QEMU has still been honoring the flag
suggests to me that it's not sufficiently critical to change the ABI.

>  If we fix qemu, it will automatically start working with all
> available kernels which are there in the wild. If we fix kernel, older
> versions will still not work, however they can.
>  That's why i think that we should adapt qemu to what already exists.
> But, well, you are The Boss, so you can just say "i don't care". So,
> just let me now if you strongly disagree with this.

I do care, in fact I care enough about the ABI that I'm suggesting what
I think is the correct fix rather than taking the quick and dirty
solution.  It's an unfortunate bug, but it's not worth changing the ABI
and removing the kernel's ability to indicate whether the pgsize bitmap
field is valid IMO.  Thanks,

Alex




reply via email to

[Prev in Thread] Current Thread [Next in Thread]