| Message ID | alpine.DEB.2.02.1405071606230.14596@kaball.uk.xensource.com |
|---|---|
| State | New |
| 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
On 7 May 2014 16:09, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > The following changes since commit 951916d02c59cd0eddd57c3d66f87f921931e394: > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-6' into staging (2014-05-06 13:06:32 +0100) > > are available in the git repository at: > > > git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-140507 > > for you to fetch changes up to 6a16be0810eccd3aede1ef592c3c1b64430b3280: > > xen_disk: add discard support (2014-05-07 14:56:54 +0000) Hi; I'm afraid I get merge conflicts when I pull this: From git://xenbits.xen.org/people/sstabellini/qemu-dm * [new branch] xen-140507 -> sstabellini/xen-140507 merging... Auto-merging xen-hvm.c CONFLICT (content): Merge conflict in xen-hvm.c Auto-merging xen-hvm-stub.c CONFLICT (content): Merge conflict in xen-hvm-stub.c Automatic merge failed; fix conflicts and then commit the result. It looks like they're pretty trivial, but since this pull involves file renames and I have no way to test Xen, I'd appreciate it if you could reroll the pull request. thanks -- PMM
On Wed, 7 May 2014, Peter Maydell wrote: > On 7 May 2014 16:09, Stefano Stabellini > <stefano.stabellini@eu.citrix.com> wrote: > > The following changes since commit 951916d02c59cd0eddd57c3d66f87f921931e394: > > > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-6' into staging (2014-05-06 13:06:32 +0100) > > > > are available in the git repository at: > > > > > > git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-140507 > > > > for you to fetch changes up to 6a16be0810eccd3aede1ef592c3c1b64430b3280: > > > > xen_disk: add discard support (2014-05-07 14:56:54 +0000) > > Hi; I'm afraid I get merge conflicts when I pull this: > >From git://xenbits.xen.org/people/sstabellini/qemu-dm > * [new branch] xen-140507 -> sstabellini/xen-140507 > merging... > Auto-merging xen-hvm.c > CONFLICT (content): Merge conflict in xen-hvm.c > Auto-merging xen-hvm-stub.c > CONFLICT (content): Merge conflict in xen-hvm-stub.c > Automatic merge failed; fix conflicts and then commit the result. > > It looks like they're pretty trivial, but since this pull involves > file renames and I have no way to test Xen, I'd appreciate > it if you could reroll the pull request. Sure
The following changes since commit 951916d02c59cd0eddd57c3d66f87f921931e394: Merge remote-tracking branch 'remotes/kraxel/tags/pull-usb-6' into staging (2014-05-06 13:06:32 +0100) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-140507 for you to fetch changes up to 6a16be0810eccd3aede1ef592c3c1b64430b3280: xen_disk: add discard support (2014-05-07 14:56:54 +0000) ---------------------------------------------------------------- Alexey Kardashevskiy (1): exec: Limit translation limiting in address_space_translate to xen Olaf Hering (1): xen_disk: add discard support Stefano Stabellini (1): pass an inclusive address range to xc_domain_pin_memory_cacheattr Wei Liu (3): xen: move Xen PV machine files to hw/xenpv xen: move Xen HVM files under hw/i386/xen xen: factor out common functions Zhenzhong Duan (1): qemu-xen: free all the pirqs for msi/msix when driver unload Makefile.target | 6 +- exec.c | 2 +- hw/block/xen_blkif.h | 12 ++++ hw/block/xen_disk.c | 33 +++++++++ hw/i386/Makefile.objs | 2 +- hw/i386/xen/Makefile.objs | 1 + hw/{ => i386}/xen/xen_apic.c | 0 hw/{ => i386}/xen/xen_platform.c | 0 hw/{ => i386}/xen/xen_pvdevice.c | 0 hw/xen/Makefile.objs | 1 - hw/xen/xen_pt_config_init.c | 6 +- hw/xen/xen_pt_msi.c | 6 +- hw/xenpv/Makefile.objs | 2 + hw/{i386 => xenpv}/xen_domainbuild.c | 0 hw/{i386 => xenpv}/xen_domainbuild.h | 0 hw/{i386 => xenpv}/xen_machine_pv.c | 0 include/hw/xen/xen_common.h | 7 ++ xen-common-stub.c | 19 ++++++ xen-common.c | 123 ++++++++++++++++++++++++++++++++++ xen-stub.c => xen-hvm-stub.c | 17 ++--- xen-all.c => xen-hvm.c | 123 ++++------------------------------ 21 files changed, 227 insertions(+), 133 deletions(-) create mode 100644 hw/i386/xen/Makefile.objs rename hw/{ => i386}/xen/xen_apic.c (100%) rename hw/{ => i386}/xen/xen_platform.c (100%) rename hw/{ => i386}/xen/xen_pvdevice.c (100%) create mode 100644 hw/xenpv/Makefile.objs rename hw/{i386 => xenpv}/xen_domainbuild.c (100%) rename hw/{i386 => xenpv}/xen_domainbuild.h (100%) rename hw/{i386 => xenpv}/xen_machine_pv.c (100%) create mode 100644 xen-common-stub.c create mode 100644 xen-common.c rename xen-stub.c => xen-hvm-stub.c (89%) rename xen-all.c => xen-hvm.c (92%)