The problem is that, address_space_get_iotlb_entry() shares a lot with address_space_translate(). This patch tries to abstract the shared elements. Originally, this work is derived from discussion from VT-d passthrough series discussions [1]. But for sure we can just see this series as a standalone cleanup. So I posted it separately here. Smoke tests are done with general VM boots, IOs, especially with vhost dmar configurations. I believe with current series I can throw away the old patch [1], which may be good. But before that, please kindly review. Thanks. References: [1] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg02335.html Peter Xu (4): exec: simplify phys_page_find() params exec: rename resolve_subpage exec: further use is_mmio exec: abstract address_space_do_translate() exec.c | 122 ++++++++++++++++++++++++++++++++++++++++------------------------- 1 file changed, 76 insertions(+), 46 deletions(-) -- 2.7.4
On 05/15/2017 10:50 AM, Peter Xu wrote: > The problem is that, address_space_get_iotlb_entry() shares a lot with > address_space_translate(). This patch tries to abstract the > shared elements. > > Originally, this work is derived from discussion from VT-d passthrough > series discussions [1]. But for sure we can just see this series as a > standalone cleanup. So I posted it separately here. > > Smoke tests are done with general VM boots, IOs, especially with vhost > dmar configurations. > > I believe with current series I can throw away the old patch [1], > which may be good. But before that, please kindly review. Thanks. I faced the problem the old patch fixes when declaring and attaching an IOMMU device, but booting the kernel with intel_iommu=off. I tested again with patches 1 & 4 of your series, and I confirm it fixes the issue: Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> Thanks! Maxime > > References: > > [1] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg02335.html > > Peter Xu (4): > exec: simplify phys_page_find() params > exec: rename resolve_subpage > exec: further use is_mmio > exec: abstract address_space_do_translate() > > exec.c | 122 ++++++++++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 76 insertions(+), 46 deletions(-) >
Hi Peter, On 05/16/2017 03:24 PM, Maxime Coquelin wrote: > > > On 05/15/2017 10:50 AM, Peter Xu wrote: >> The problem is that, address_space_get_iotlb_entry() shares a lot with >> address_space_translate(). This patch tries to abstract the >> shared elements. >> >> Originally, this work is derived from discussion from VT-d passthrough >> series discussions [1]. But for sure we can just see this series as a >> standalone cleanup. So I posted it separately here. >> >> Smoke tests are done with general VM boots, IOs, especially with vhost >> dmar configurations. >> >> I believe with current series I can throw away the old patch [1], >> which may be good. But before that, please kindly review. Thanks. > > I faced the problem the old patch fixes when declaring and attaching an > IOMMU device, but booting the kernel with intel_iommu=off. > > I tested again with patches 1 & 4 of your series, and I confirm it fixes > the issue: > Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> I did some more testing with my "vhost-user IOMMU" setup, and the series actually breaks with IOMMU device attached, and intel_iommu=on. The main difference with the previous passing test is the guest RAM size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M hugepages in the failing one. Note that I also reproduce with vhost-kernel backend. The error happens in the first vhost_device_iotlb_miss() call: qemu-system-x86_64: Fail to lookup the translated address b5d7c000 I don't have the root cause yet, I'll keep you updated. Maxime > Thanks! > Maxime > >> >> References: >> >> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg02335.html >> >> Peter Xu (4): >> exec: simplify phys_page_find() params >> exec: rename resolve_subpage >> exec: further use is_mmio >> exec: abstract address_space_do_translate() >> >> exec.c | 122 >> ++++++++++++++++++++++++++++++++++++++++------------------------- >> 1 file changed, 76 insertions(+), 46 deletions(-) >> >
On 16/05/2017 18:51, Maxime Coquelin wrote: >> >> I faced the problem the old patch fixes when declaring and attaching >> an IOMMU device, but booting the kernel with intel_iommu=off. >> >> I tested again with patches 1 & 4 of your series, and I confirm it >> fixes the issue: >> Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > I did some more testing with my "vhost-user IOMMU" setup, and the series > actually breaks with IOMMU device attached, and intel_iommu=on. > > The main difference with the previous passing test is the guest RAM > size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M > hugepages in the failing one. Note that I also reproduce with > vhost-kernel backend. > > The error happens in the first vhost_device_iotlb_miss() call: > qemu-system-x86_64: Fail to lookup the translated address b5d7c000 > > I don't have the root cause yet, I'll keep you updated. I suppose it is patch 4? Paolo
On 05/16/2017 06:54 PM, Paolo Bonzini wrote: > > > On 16/05/2017 18:51, Maxime Coquelin wrote: >>> >>> I faced the problem the old patch fixes when declaring and attaching >>> an IOMMU device, but booting the kernel with intel_iommu=off. >>> >>> I tested again with patches 1 & 4 of your series, and I confirm it >>> fixes the issue: >>> Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> >> I did some more testing with my "vhost-user IOMMU" setup, and the series >> actually breaks with IOMMU device attached, and intel_iommu=on. >> >> The main difference with the previous passing test is the guest RAM >> size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M >> hugepages in the failing one. Note that I also reproduce with >> vhost-kernel backend. >> >> The error happens in the first vhost_device_iotlb_miss() call: >> qemu-system-x86_64: Fail to lookup the translated address b5d7c000 >> >> I don't have the root cause yet, I'll keep you updated. > > I suppose it is patch 4? Yes it is. Maxime
On Tue, May 16, 2017 at 06:51:03PM +0200, Maxime Coquelin wrote: > Hi Peter, > > On 05/16/2017 03:24 PM, Maxime Coquelin wrote: > > > > > >On 05/15/2017 10:50 AM, Peter Xu wrote: > >>The problem is that, address_space_get_iotlb_entry() shares a lot with > >>address_space_translate(). This patch tries to abstract the > >>shared elements. > >> > >>Originally, this work is derived from discussion from VT-d passthrough > >>series discussions [1]. But for sure we can just see this series as a > >>standalone cleanup. So I posted it separately here. > >> > >>Smoke tests are done with general VM boots, IOs, especially with vhost > >>dmar configurations. > >> > >>I believe with current series I can throw away the old patch [1], > >>which may be good. But before that, please kindly review. Thanks. > > > >I faced the problem the old patch fixes when declaring and attaching an > >IOMMU device, but booting the kernel with intel_iommu=off. > > > >I tested again with patches 1 & 4 of your series, and I confirm it fixes > >the issue: > >Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > I did some more testing with my "vhost-user IOMMU" setup, and the series > actually breaks with IOMMU device attached, and intel_iommu=on. > > The main difference with the previous passing test is the guest RAM > size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M > hugepages in the failing one. Note that I also reproduce with vhost-kernel > backend. > > The error happens in the first vhost_device_iotlb_miss() call: > qemu-system-x86_64: Fail to lookup the translated address b5d7c000 > > I don't have the root cause yet, I'll keep you updated. Maxime, Thanks a lot for help testing this series! I reproduced this problem, and this is not a problem obvious enough for me. Let me investigate as well. -- Peter Xu
On Wed, May 17, 2017 at 12:23:42PM +0800, Peter Xu wrote: > On Tue, May 16, 2017 at 06:51:03PM +0200, Maxime Coquelin wrote: > > Hi Peter, > > > > On 05/16/2017 03:24 PM, Maxime Coquelin wrote: > > > > > > > > >On 05/15/2017 10:50 AM, Peter Xu wrote: > > >>The problem is that, address_space_get_iotlb_entry() shares a lot with > > >>address_space_translate(). This patch tries to abstract the > > >>shared elements. > > >> > > >>Originally, this work is derived from discussion from VT-d passthrough > > >>series discussions [1]. But for sure we can just see this series as a > > >>standalone cleanup. So I posted it separately here. > > >> > > >>Smoke tests are done with general VM boots, IOs, especially with vhost > > >>dmar configurations. > > >> > > >>I believe with current series I can throw away the old patch [1], > > >>which may be good. But before that, please kindly review. Thanks. > > > > > >I faced the problem the old patch fixes when declaring and attaching an > > >IOMMU device, but booting the kernel with intel_iommu=off. > > > > > >I tested again with patches 1 & 4 of your series, and I confirm it fixes > > >the issue: > > >Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > > > > I did some more testing with my "vhost-user IOMMU" setup, and the series > > actually breaks with IOMMU device attached, and intel_iommu=on. > > > > The main difference with the previous passing test is the guest RAM > > size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M > > hugepages in the failing one. Note that I also reproduce with vhost-kernel > > backend. > > > > The error happens in the first vhost_device_iotlb_miss() call: > > qemu-system-x86_64: Fail to lookup the translated address b5d7c000 > > > > I don't have the root cause yet, I'll keep you updated. > > Maxime, > > Thanks a lot for help testing this series! > > I reproduced this problem, and this is not a problem obvious enough > for me. Let me investigate as well. > > -- > Peter Xu Maxime, Could you help try adding this change upon current to see whether problem solved? diff --git a/exec.c b/exec.c index 697d902..68576a2 100644 --- a/exec.c +++ b/exec.c @@ -521,6 +521,10 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, goto iotlb_fail; } + /* Convert memory region offset into address space offset */ + xlat += section.offset_within_address_space - + section.offset_within_region; + if (plen == (hwaddr)-1) { /* * We use default page size here. Logically it only happens Thanks in advance, -- Peter Xu
On 05/17/2017 07:57 AM, Peter Xu wrote: > On Wed, May 17, 2017 at 12:23:42PM +0800, Peter Xu wrote: >> On Tue, May 16, 2017 at 06:51:03PM +0200, Maxime Coquelin wrote: >>> Hi Peter, >>> >>> On 05/16/2017 03:24 PM, Maxime Coquelin wrote: >>>> >>>> >>>> On 05/15/2017 10:50 AM, Peter Xu wrote: >>>>> The problem is that, address_space_get_iotlb_entry() shares a lot with >>>>> address_space_translate(). This patch tries to abstract the >>>>> shared elements. >>>>> >>>>> Originally, this work is derived from discussion from VT-d passthrough >>>>> series discussions [1]. But for sure we can just see this series as a >>>>> standalone cleanup. So I posted it separately here. >>>>> >>>>> Smoke tests are done with general VM boots, IOs, especially with vhost >>>>> dmar configurations. >>>>> >>>>> I believe with current series I can throw away the old patch [1], >>>>> which may be good. But before that, please kindly review. Thanks. >>>> >>>> I faced the problem the old patch fixes when declaring and attaching an >>>> IOMMU device, but booting the kernel with intel_iommu=off. >>>> >>>> I tested again with patches 1 & 4 of your series, and I confirm it fixes >>>> the issue: >>>> Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> >>> >>> I did some more testing with my "vhost-user IOMMU" setup, and the series >>> actually breaks with IOMMU device attached, and intel_iommu=on. >>> >>> The main difference with the previous passing test is the guest RAM >>> size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M >>> hugepages in the failing one. Note that I also reproduce with vhost-kernel >>> backend. >>> >>> The error happens in the first vhost_device_iotlb_miss() call: >>> qemu-system-x86_64: Fail to lookup the translated address b5d7c000 >>> >>> I don't have the root cause yet, I'll keep you updated. >> >> Maxime, >> >> Thanks a lot for help testing this series! >> >> I reproduced this problem, and this is not a problem obvious enough >> for me. Let me investigate as well. >> >> -- >> Peter Xu > > Maxime, > > Could you help try adding this change upon current to see whether > problem solved? Hi Peter, Yes, problem is solved with below change. Thanks! Maxime > diff --git a/exec.c b/exec.c > index 697d902..68576a2 100644 > --- a/exec.c > +++ b/exec.c > @@ -521,6 +521,10 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr, > goto iotlb_fail; > } > > + /* Convert memory region offset into address space offset */ > + xlat += section.offset_within_address_space - > + section.offset_within_region; > + > if (plen == (hwaddr)-1) { > /* > * We use default page size here. Logically it only happens > > Thanks in advance, >
On 17/05/2017 09:17, Maxime Coquelin wrote: > > > On 05/17/2017 07:57 AM, Peter Xu wrote: >> On Wed, May 17, 2017 at 12:23:42PM +0800, Peter Xu wrote: >>> On Tue, May 16, 2017 at 06:51:03PM +0200, Maxime Coquelin wrote: >>>> Hi Peter, >>>> >>>> On 05/16/2017 03:24 PM, Maxime Coquelin wrote: >>>>> >>>>> >>>>> On 05/15/2017 10:50 AM, Peter Xu wrote: >>>>>> The problem is that, address_space_get_iotlb_entry() shares a lot >>>>>> with >>>>>> address_space_translate(). This patch tries to abstract the >>>>>> shared elements. >>>>>> >>>>>> Originally, this work is derived from discussion from VT-d >>>>>> passthrough >>>>>> series discussions [1]. But for sure we can just see this series as a >>>>>> standalone cleanup. So I posted it separately here. >>>>>> >>>>>> Smoke tests are done with general VM boots, IOs, especially with >>>>>> vhost >>>>>> dmar configurations. >>>>>> >>>>>> I believe with current series I can throw away the old patch [1], >>>>>> which may be good. But before that, please kindly review. Thanks. >>>>> >>>>> I faced the problem the old patch fixes when declaring and >>>>> attaching an >>>>> IOMMU device, but booting the kernel with intel_iommu=off. >>>>> >>>>> I tested again with patches 1 & 4 of your series, and I confirm it >>>>> fixes >>>>> the issue: >>>>> Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> >>>> >>>> I did some more testing with my "vhost-user IOMMU" setup, and the >>>> series >>>> actually breaks with IOMMU device attached, and intel_iommu=on. >>>> >>>> The main difference with the previous passing test is the guest RAM >>>> size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M >>>> hugepages in the failing one. Note that I also reproduce with >>>> vhost-kernel >>>> backend. >>>> >>>> The error happens in the first vhost_device_iotlb_miss() call: >>>> qemu-system-x86_64: Fail to lookup the translated address b5d7c000 >>>> >>>> I don't have the root cause yet, I'll keep you updated. >>> >>> Maxime, >>> >>> Thanks a lot for help testing this series! >>> >>> I reproduced this problem, and this is not a problem obvious enough >>> for me. Let me investigate as well. >>> >>> -- >>> Peter Xu >> >> Maxime, >> >> Could you help try adding this change upon current to see whether >> problem solved? > > Hi Peter, > > Yes, problem is solved with below change. Cool---Peter, please send the new patch 4 as toplevel message! Thanks, Paolo
On Wed, May 17, 2017 at 09:26:25AM +0200, Paolo Bonzini wrote: > > > On 17/05/2017 09:17, Maxime Coquelin wrote: > > > > > > On 05/17/2017 07:57 AM, Peter Xu wrote: > >> On Wed, May 17, 2017 at 12:23:42PM +0800, Peter Xu wrote: > >>> On Tue, May 16, 2017 at 06:51:03PM +0200, Maxime Coquelin wrote: > >>>> Hi Peter, > >>>> > >>>> On 05/16/2017 03:24 PM, Maxime Coquelin wrote: > >>>>> > >>>>> > >>>>> On 05/15/2017 10:50 AM, Peter Xu wrote: > >>>>>> The problem is that, address_space_get_iotlb_entry() shares a lot > >>>>>> with > >>>>>> address_space_translate(). This patch tries to abstract the > >>>>>> shared elements. > >>>>>> > >>>>>> Originally, this work is derived from discussion from VT-d > >>>>>> passthrough > >>>>>> series discussions [1]. But for sure we can just see this series as a > >>>>>> standalone cleanup. So I posted it separately here. > >>>>>> > >>>>>> Smoke tests are done with general VM boots, IOs, especially with > >>>>>> vhost > >>>>>> dmar configurations. > >>>>>> > >>>>>> I believe with current series I can throw away the old patch [1], > >>>>>> which may be good. But before that, please kindly review. Thanks. > >>>>> > >>>>> I faced the problem the old patch fixes when declaring and > >>>>> attaching an > >>>>> IOMMU device, but booting the kernel with intel_iommu=off. > >>>>> > >>>>> I tested again with patches 1 & 4 of your series, and I confirm it > >>>>> fixes > >>>>> the issue: > >>>>> Tested-by: Maxime Coquelin <maxime.coquelin@redhat.com> > >>>> > >>>> I did some more testing with my "vhost-user IOMMU" setup, and the > >>>> series > >>>> actually breaks with IOMMU device attached, and intel_iommu=on. > >>>> > >>>> The main difference with the previous passing test is the guest RAM > >>>> size. In the working setup, it is 2G of 2M hugepages, vs. 4G of 2M > >>>> hugepages in the failing one. Note that I also reproduce with > >>>> vhost-kernel > >>>> backend. > >>>> > >>>> The error happens in the first vhost_device_iotlb_miss() call: > >>>> qemu-system-x86_64: Fail to lookup the translated address b5d7c000 > >>>> > >>>> I don't have the root cause yet, I'll keep you updated. > >>> > >>> Maxime, > >>> > >>> Thanks a lot for help testing this series! > >>> > >>> I reproduced this problem, and this is not a problem obvious enough > >>> for me. Let me investigate as well. > >>> > >>> -- > >>> Peter Xu > >> > >> Maxime, > >> > >> Could you help try adding this change upon current to see whether > >> problem solved? > > > > Hi Peter, > > > > Yes, problem is solved with below change. > > Cool---Peter, please send the new patch 4 as toplevel message! Will do. Thanks! -- Peter Xu
© 2016 - 2024 Red Hat, Inc.