| Message ID | 1399475403-5408-1-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | Superseded |
| Headers | show |
On 7 May 2014 16:09, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > From: Alexey Kardashevskiy <aik@ozlabs.ru> > > The address_space_translate() function cuts the returned plen (page size) > to hardcoded TARGET_PAGE_SIZE. This function can be used on pages bigger > than that so this limiting should not be used on such pages. > > Since originally the limiting was introduced for XEN, we can safely > limit this piece of code to XEN. So does the patch. > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > exec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/exec.c b/exec.c > index 91513c6..cf12049 100644 > --- a/exec.c > +++ b/exec.c > @@ -380,7 +380,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, > as = iotlb.target_as; > } > > - if (memory_access_is_direct(mr, is_write)) { > + if (xen_enabled() && memory_access_is_direct(mr, is_write)) { > hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr; > len = MIN(page, len); > } We should put this patch in (both as an efficiency thing and an expedient fix) but we really need to either track down which callers of this API are relying on the returned plen not being truncated, or we need to fix Xen to not truncate either. This is just a bandaid IMHO. thanks -- PMM
Il 07/05/2014 17:12, Peter Maydell ha scritto: >> > - if (memory_access_is_direct(mr, is_write)) { >> > + if (xen_enabled() && memory_access_is_direct(mr, is_write)) { >> > hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr; >> > len = MIN(page, len); >> > } > We should put this patch in (both as an efficiency thing and an > expedient fix) but we really need to either track down which > callers of this API are relying on the returned plen not being > truncated, or we need to fix Xen to not truncate either. This > is just a bandaid IMHO. Fixing Xen to not truncate is not possible because of the Xen mapcache, unless of course QEMU is changed to avoid the mapcache completely on 64-bit hosts. I'm not sure if that makes sense from the Xen point of view. Regarding fixing callers, a known one is virtio-scsi which is a bug and on my todo list. But another is VFIO, which cannot accept truncation if the IOMMU page size is greater than TARGET_PAGE_SIZE. Paolo
On Wed, 7 May 2014, Paolo Bonzini wrote: > Il 07/05/2014 17:12, Peter Maydell ha scritto: > > > > - if (memory_access_is_direct(mr, is_write)) { > > > > + if (xen_enabled() && memory_access_is_direct(mr, is_write)) { > > > > hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - > > > addr; > > > > len = MIN(page, len); > > > > } > > We should put this patch in (both as an efficiency thing and an > > expedient fix) but we really need to either track down which > > callers of this API are relying on the returned plen not being > > truncated, or we need to fix Xen to not truncate either. This > > is just a bandaid IMHO. > > Fixing Xen to not truncate is not possible because of the Xen mapcache, unless > of course QEMU is changed to avoid the mapcache completely on 64-bit hosts. > I'm not sure if that makes sense from the Xen point of view. Right, it makes sense, however we would still need to keep the 32-bit mapcache code path working. > Regarding fixing callers, a known one is virtio-scsi which is a bug and on my > todo list. But another is VFIO, which cannot accept truncation if the IOMMU > page size is greater than TARGET_PAGE_SIZE.
On 7 May 2014 16:22, Paolo Bonzini <pbonzini@redhat.com> wrote: > Fixing Xen to not truncate is not possible because of the Xen mapcache, > unless of course QEMU is changed to avoid the mapcache completely on 64-bit > hosts. I'm not sure if that makes sense from the Xen point of view. > > Regarding fixing callers, a known one is virtio-scsi which is a bug and on > my todo list. But another is VFIO, which cannot accept truncation if the > IOMMU page size is greater than TARGET_PAGE_SIZE. The API can't simultaneously allow the implementation to truncate and guarantee to the caller that we don't truncate, so one of these has to change, surely? Otherwise we would need to provide some sort of flag for truncation-unacceptable so that incompatible combinations fail nicely rather than silently doing weird stuff, I guess. thanks -- PMM
Il 07/05/2014 17:32, Peter Maydell ha scritto: > > Regarding fixing callers, a known one is virtio-scsi which is a bug and on > > my todo list. But another is VFIO, which cannot accept truncation if the > > IOMMU page size is greater than TARGET_PAGE_SIZE. > > The API can't simultaneously allow the implementation to truncate > and guarantee to the caller that we don't truncate, so one of these > has to change, surely? We can provide a function to return the truncation granularity, and add to VFIO a check that will prevent using VFIO together with Xen. Paolo
diff --git a/exec.c b/exec.c index 91513c6..cf12049 100644 --- a/exec.c +++ b/exec.c @@ -380,7 +380,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr, as = iotlb.target_as; } - if (memory_access_is_direct(mr, is_write)) { + if (xen_enabled() && memory_access_is_direct(mr, is_write)) { hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr; len = MIN(page, len); }