[Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration

Bharat Bhushan posted 5 patches 6 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/virtio/trace-events           |   5 ++
hw/virtio/virtio-iommu.c         | 181 ++++++++++++++++++++++++++++++++++++++-
include/hw/virtio/virtio-iommu.h |   6 ++
target/arm/kvm.c                 |  27 ++++++
target/arm/trace-events          |   3 +
5 files changed, 219 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Bharat Bhushan 6 years, 7 months ago
This patch series integrates VFIO/VHOST with virtio-iommu.

This version is mainly about rebasing on v4 version on
virtio-iommu device framework from Eric Augur and
addresing review comments.

This patch series allows PCI pass-through using virtio-iommu.
  
This series is based on:
 - virtio-iommu kernel driver by Jean-Philippe Brucker
    [1] [RFC] virtio-iommu version 0.4
    git://linux-arm.org/virtio-iommu.git branch viommu/v0.4

 - virtio-iommu device emulation by Eric Augur.
   [RFC v4 00/16] VIRTIO-IOMMU device
   https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4

Changes are available at : https://github.com/bharaty/qemu.git virtio-iommu-vfio-integration-v4

v3->v4:
 - Rebase to v4 version from Eric
 - Fixes from Eric with DPDK in VM
 - Logical division in multiple patches

v2->v3:
 - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
   Which is based on top of v2.10-rc0 that
 - Fixed issue with two PCI devices
 - Addressed review comments

v1->v2:
  - Added trace events
  - removed vSMMU3 link in patch description

Bharat Bhushan (5):
  target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route
  virtio-iommu: Add iommu notifier for map/unmap
  virtio-iommu: Call iommu notifier for attach/detach
  virtio-iommu: add iommu replay
  virtio-iommu: add iommu notifier memory-region

 hw/virtio/trace-events           |   5 ++
 hw/virtio/virtio-iommu.c         | 181 ++++++++++++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-iommu.h |   6 ++
 target/arm/kvm.c                 |  27 ++++++
 target/arm/trace-events          |   3 +
 5 files changed, 219 insertions(+), 3 deletions(-)

-- 
1.9.3


Re: [Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Bharat Bhushan 6 years, 7 months ago
Hi Peter,

While vfio with virtio-iommu I observed one issue,  When virtio-iommu device exists but guest kernel does not have virtio-iommu driver (not enabled in Config) then IOMMU faults are reported on host.

This is because no mapping is created in IOMMU, not even default guest-physical to real-physical. While looking at vfio_listener_region_add(), it does not create initial mapping in IOMMU and relies on guest to create mapping. Is this something known or I am missing something?

Thanks
-Bharat

> -----Original Message-----
> From: Bharat Bhushan [mailto:Bharat.Bhushan@nxp.com]
> Sent: Wednesday, September 27, 2017 12:03 PM
> To: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org
> Cc: wei@redhat.com; kevin.tian@intel.com; marc.zyngier@arm.com;
> tn@semihalf.com; will.deacon@arm.com; drjones@redhat.com;
> robin.murphy@arm.com; christoffer.dall@linaro.org;
> bharatb.yadav@gmail.com; Bharat Bhushan <bharat.bhushan@nxp.com>
> Subject: [PATCH v4 0/5] virtio-iommu: VFIO integration
> 
> This patch series integrates VFIO/VHOST with virtio-iommu.
> 
> This version is mainly about rebasing on v4 version on virtio-iommu device
> framework from Eric Augur and addresing review comments.
> 
> This patch series allows PCI pass-through using virtio-iommu.
> 
> This series is based on:
>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>     [1] [RFC] virtio-iommu version 0.4
>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
>  - virtio-iommu device emulation by Eric Augur.
>    [RFC v4 00/16] VIRTIO-IOMMU device
>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> 
> Changes are available at : https://github.com/bharaty/qemu.git virtio-
> iommu-vfio-integration-v4
> 
> v3->v4:
>  - Rebase to v4 version from Eric
>  - Fixes from Eric with DPDK in VM
>  - Logical division in multiple patches
> 
> v2->v3:
>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>    Which is based on top of v2.10-rc0 that
>  - Fixed issue with two PCI devices
>  - Addressed review comments
> 
> v1->v2:
>   - Added trace events
>   - removed vSMMU3 link in patch description
> 
> Bharat Bhushan (5):
>   target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route
>   virtio-iommu: Add iommu notifier for map/unmap
>   virtio-iommu: Call iommu notifier for attach/detach
>   virtio-iommu: add iommu replay
>   virtio-iommu: add iommu notifier memory-region
> 
>  hw/virtio/trace-events           |   5 ++
>  hw/virtio/virtio-iommu.c         | 181
> ++++++++++++++++++++++++++++++++++++++-
>  include/hw/virtio/virtio-iommu.h |   6 ++
>  target/arm/kvm.c                 |  27 ++++++
>  target/arm/trace-events          |   3 +
>  5 files changed, 219 insertions(+), 3 deletions(-)
> 
> --
> 1.9.3


Re: [Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Peter Xu 6 years, 7 months ago
On Wed, Sep 27, 2017 at 06:46:18AM +0000, Bharat Bhushan wrote:
> Hi Peter,

Hi, Bharat!

> 
> While vfio with virtio-iommu I observed one issue,  When virtio-iommu device exists but guest kernel does not have virtio-iommu driver (not enabled in Config) then IOMMU faults are reported on host.
> 
> This is because no mapping is created in IOMMU, not even default
guest-physical to real-physical. While looking at vfio_listener_region_add(), it does not create initial mapping in IOMMU and relies on guest to create mapping. Is this something known or I am missing something?

For VT-d, the trick is played using dynamic IOMMU memory region.
Please refer to commit 558e0024a428 ("intel_iommu: allow dynamic
switch of IOMMU region") for more information.

The whole idea is that, the IOMMU region will not be enabled only if
the guest enables that explicitly for the device.  Otherwise (for your
case, when guest driver is not loaded at all), the IOMMU region is by
default off, then the default GPA region will be used to build up the
mapping (just like when we don't have vIOMMU at all).  Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Bharat Bhushan 6 years, 7 months ago
Hi Peter,

> -----Original Message-----
> From: Peter Xu [mailto:peterx@redhat.com]
> Sent: Wednesday, September 27, 2017 12:32 PM
> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org; wei@redhat.com;
> kevin.tian@intel.com; marc.zyngier@arm.com; tn@semihalf.com;
> will.deacon@arm.com; drjones@redhat.com; robin.murphy@arm.com;
> christoffer.dall@linaro.org; bharatb.yadav@gmail.com
> Subject: Re: [PATCH v4 0/5] virtio-iommu: VFIO integration
> 
> On Wed, Sep 27, 2017 at 06:46:18AM +0000, Bharat Bhushan wrote:
> > Hi Peter,
> 
> Hi, Bharat!
> 
> >
> > While vfio with virtio-iommu I observed one issue,  When virtio-iommu
> device exists but guest kernel does not have virtio-iommu driver (not
> enabled in Config) then IOMMU faults are reported on host.
> >
> > This is because no mapping is created in IOMMU, not even default
> guest-physical to real-physical. While looking at vfio_listener_region_add(), it
> does not create initial mapping in IOMMU and relies on guest to create
> mapping. Is this something known or I am missing something?
> 
> For VT-d, the trick is played using dynamic IOMMU memory region.
> Please refer to commit 558e0024a428 ("intel_iommu: allow dynamic switch of
> IOMMU region") for more information.
> 
> The whole idea is that, the IOMMU region will not be enabled only if the
> guest enables that explicitly for the device.  Otherwise (for your case, when
> guest driver is not loaded at all), the IOMMU region is by default off, then
> the default GPA region will be used to build up the mapping (just like when
> we don't have vIOMMU at all).  Thanks,

Thanks, I will analyze and see how we can use for virtio-iommu.

Regards
-Bharat

> 
> --
> Peter Xu
Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Linu Cherian 6 years, 7 months ago
Hi,

On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> This patch series integrates VFIO/VHOST with virtio-iommu.
> 
> This version is mainly about rebasing on v4 version on
> virtio-iommu device framework from Eric Augur and
> addresing review comments.
> 
> This patch series allows PCI pass-through using virtio-iommu.
>   
> This series is based on:
>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>     [1] [RFC] virtio-iommu version 0.4
>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
>  - virtio-iommu device emulation by Eric Augur.
>    [RFC v4 00/16] VIRTIO-IOMMU device
>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> 
> Changes are available at : https://github.com/bharaty/qemu.git virtio-iommu-vfio-integration-v4
> 

# With the above sources, was trying to test the vfio-pci device assigned to 
  guest using Qemu.
# Both guest and host kernels are configured with 4k as page size.
# releavant qemu command snippet,
  -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
  -net none -device vfio-pci,host=xxx


On guest booting, observed mutliple messages as below,

qemu-system-aarch64: iommu has granularity incompatible with target AS

# On adding necessary prints, 0x5000 is len, 0x4fff is address mask
  and the code expects the address mask to be 0xfff.

if (len & iotlb->addr_mask) {
        error_report

# vfio_dma_map is failing due to this error.

Any pointers ?


> v3->v4:
>  - Rebase to v4 version from Eric
>  - Fixes from Eric with DPDK in VM
>  - Logical division in multiple patches
> 
> v2->v3:
>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>    Which is based on top of v2.10-rc0 that
>  - Fixed issue with two PCI devices
>  - Addressed review comments
> 
> v1->v2:
>   - Added trace events
>   - removed vSMMU3 link in patch description
> 
> Bharat Bhushan (5):
>   target/arm/kvm: Translate the MSI doorbell in kvm_arch_fixup_msi_route
>   virtio-iommu: Add iommu notifier for map/unmap
>   virtio-iommu: Call iommu notifier for attach/detach
>   virtio-iommu: add iommu replay
>   virtio-iommu: add iommu notifier memory-region
> 
>  hw/virtio/trace-events           |   5 ++
>  hw/virtio/virtio-iommu.c         | 181 ++++++++++++++++++++++++++++++++++++++-
>  include/hw/virtio/virtio-iommu.h |   6 ++
>  target/arm/kvm.c                 |  27 ++++++
>  target/arm/trace-events          |   3 +
>  5 files changed, 219 insertions(+), 3 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Bharat Bhushan 6 years, 7 months ago
Hi,

> -----Original Message-----
> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> Sent: Wednesday, September 27, 2017 1:11 PM
> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> bharatb.yadav@gmail.com
> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> 
> Hi,
> 
> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> > This patch series integrates VFIO/VHOST with virtio-iommu.
> >
> > This version is mainly about rebasing on v4 version on virtio-iommu
> > device framework from Eric Augur and addresing review comments.
> >
> > This patch series allows PCI pass-through using virtio-iommu.
> >
> > This series is based on:
> >  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >     [1] [RFC] virtio-iommu version 0.4
> >     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> >
> >  - virtio-iommu device emulation by Eric Augur.
> >    [RFC v4 00/16] VIRTIO-IOMMU device
> >    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> >
> > Changes are available at : https://github.com/bharaty/qemu.git
> > virtio-iommu-vfio-integration-v4
> >
> 
> # With the above sources, was trying to test the vfio-pci device assigned to
>   guest using Qemu.
> # Both guest and host kernels are configured with 4k as page size.
> # releavant qemu command snippet,
>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
>   -net none -device vfio-pci,host=xxx
> 
> 
> On guest booting, observed mutliple messages as below,
> 
> qemu-system-aarch64: iommu has granularity incompatible with target AS
> 
> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
>   and the code expects the address mask to be 0xfff.

I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 

Thanks
-Bharat

> 
> if (len & iotlb->addr_mask) {
>         error_report
> 
> # vfio_dma_map is failing due to this error.
> 
> Any pointers ?
> 
> 
> > v3->v4:
> >  - Rebase to v4 version from Eric
> >  - Fixes from Eric with DPDK in VM
> >  - Logical division in multiple patches
> >
> > v2->v3:
> >  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> >    Which is based on top of v2.10-rc0 that
> >  - Fixed issue with two PCI devices
> >  - Addressed review comments
> >
> > v1->v2:
> >   - Added trace events
> >   - removed vSMMU3 link in patch description
> >
> > Bharat Bhushan (5):
> >   target/arm/kvm: Translate the MSI doorbell in
> kvm_arch_fixup_msi_route
> >   virtio-iommu: Add iommu notifier for map/unmap
> >   virtio-iommu: Call iommu notifier for attach/detach
> >   virtio-iommu: add iommu replay
> >   virtio-iommu: add iommu notifier memory-region
> >
> >  hw/virtio/trace-events           |   5 ++
> >  hw/virtio/virtio-iommu.c         | 181
> ++++++++++++++++++++++++++++++++++++++-
> >  include/hw/virtio/virtio-iommu.h |   6 ++
> >  target/arm/kvm.c                 |  27 ++++++
> >  target/arm/trace-events          |   3 +
> >  5 files changed, 219 insertions(+), 3 deletions(-)
> >
> > --
> > 1.9.3
> >
> >
> 
> --
> Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 7 months ago
Hi Linu,

On 27/09/2017 10:30, Bharat Bhushan wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>> Sent: Wednesday, September 27, 2017 1:11 PM
>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>> bharatb.yadav@gmail.com
>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>
>> Hi,
>>
>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>
>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>> device framework from Eric Augur and addresing review comments.
>>>
>>> This patch series allows PCI pass-through using virtio-iommu.
>>>
>>> This series is based on:
>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>     [1] [RFC] virtio-iommu version 0.4
>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4

Just to make sure, do you use the v0.4 virtio-iommu driver from above
branch?

Thanks

Eric
>>>
>>>  - virtio-iommu device emulation by Eric Augur.
>>>    [RFC v4 00/16] VIRTIO-IOMMU device
>>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
>>>
>>> Changes are available at : https://github.com/bharaty/qemu.git
>>> virtio-iommu-vfio-integration-v4
>>>
>>
>> # With the above sources, was trying to test the vfio-pci device assigned to
>>   guest using Qemu.
>> # Both guest and host kernels are configured with 4k as page size.
>> # releavant qemu command snippet,
>>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
>>   -net none -device vfio-pci,host=xxx
>>
>>
>> On guest booting, observed mutliple messages as below,
>>
>> qemu-system-aarch64: iommu has granularity incompatible with target AS
>>
>> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
>>   and the code expects the address mask to be 0xfff.
> 
> I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
> 
> Thanks
> -Bharat
> 
>>
>> if (len & iotlb->addr_mask) {
>>         error_report
>>
>> # vfio_dma_map is failing due to this error.
>>
>> Any pointers ?
>>
>>
>>> v3->v4:
>>>  - Rebase to v4 version from Eric
>>>  - Fixes from Eric with DPDK in VM
>>>  - Logical division in multiple patches
>>>
>>> v2->v3:
>>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>>>    Which is based on top of v2.10-rc0 that
>>>  - Fixed issue with two PCI devices
>>>  - Addressed review comments
>>>
>>> v1->v2:
>>>   - Added trace events
>>>   - removed vSMMU3 link in patch description
>>>
>>> Bharat Bhushan (5):
>>>   target/arm/kvm: Translate the MSI doorbell in
>> kvm_arch_fixup_msi_route
>>>   virtio-iommu: Add iommu notifier for map/unmap
>>>   virtio-iommu: Call iommu notifier for attach/detach
>>>   virtio-iommu: add iommu replay
>>>   virtio-iommu: add iommu notifier memory-region
>>>
>>>  hw/virtio/trace-events           |   5 ++
>>>  hw/virtio/virtio-iommu.c         | 181
>> ++++++++++++++++++++++++++++++++++++++-
>>>  include/hw/virtio/virtio-iommu.h |   6 ++
>>>  target/arm/kvm.c                 |  27 ++++++
>>>  target/arm/trace-events          |   3 +
>>>  5 files changed, 219 insertions(+), 3 deletions(-)
>>>
>>> --
>>> 1.9.3
>>>
>>>
>>
>> --
>> Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Linu Cherian 6 years, 7 months ago
On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
> Hi Linu,
> 
> On 27/09/2017 10:30, Bharat Bhushan wrote:
> > Hi,
> > 
> >> -----Original Message-----
> >> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> >> Sent: Wednesday, September 27, 2017 1:11 PM
> >> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> >> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> >> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> >> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> >> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> >> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> >> bharatb.yadav@gmail.com
> >> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> >>
> >> Hi,
> >>
> >> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> >>> This patch series integrates VFIO/VHOST with virtio-iommu.
> >>>
> >>> This version is mainly about rebasing on v4 version on virtio-iommu
> >>> device framework from Eric Augur and addresing review comments.
> >>>
> >>> This patch series allows PCI pass-through using virtio-iommu.
> >>>
> >>> This series is based on:
> >>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >>>     [1] [RFC] virtio-iommu version 0.4
> >>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
> Just to make sure, do you use the v0.4 virtio-iommu driver from above
> branch?

Yes, Eric i have that.

From guest kernel,
  0.000000] Linux version 4.13.0-rc1-gd1949df
.
.
] virtio_iommu virtio0: aperture: 0x0-0xffffffffffffffff
[    0.927886] virtio_iommu virtio0: page mask: 0x40201000
[    0.931682] virtio_iommu virtio0: probe successful


Guest kernel source is on this commit,

commit d1949dfbf5c0d181b290625c28c5359284686e3a
Author: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Date:   Mon Jul 17 19:01:07 2017 +0100

    iommu/virtio-iommu: add MSI window probe
    
    Using the probe request, extract RESV_MEM information. When we encounter a
    MSI doorbell region, set it up as a IOMMU_RESV_MSI region. This will tell
    other subsystems that there is no need to map the MSI doorbell in the
    virtio-iommu, because MSIs bypass it.
    
    Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>

with CONFIG_VIRTIO_IOMMU=y in config.

> 
> Thanks
> 
> Eric
> >>>
> >>>  - virtio-iommu device emulation by Eric Augur.
> >>>    [RFC v4 00/16] VIRTIO-IOMMU device
> >>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> >>>
> >>> Changes are available at : https://github.com/bharaty/qemu.git
> >>> virtio-iommu-vfio-integration-v4
> >>>
> >>
> >> # With the above sources, was trying to test the vfio-pci device assigned to
> >>   guest using Qemu.
> >> # Both guest and host kernels are configured with 4k as page size.
> >> # releavant qemu command snippet,
> >>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
> >>   -net none -device vfio-pci,host=xxx
> >>
> >>
> >> On guest booting, observed mutliple messages as below,
> >>
> >> qemu-system-aarch64: iommu has granularity incompatible with target AS
> >>
> >> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
> >>   and the code expects the address mask to be 0xfff.
> > 
> > I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
> > 
> > Thanks
> > -Bharat
> > 
> >>
> >> if (len & iotlb->addr_mask) {
> >>         error_report
> >>
> >> # vfio_dma_map is failing due to this error.
> >>
> >> Any pointers ?
> >>
> >>
> >>> v3->v4:
> >>>  - Rebase to v4 version from Eric
> >>>  - Fixes from Eric with DPDK in VM
> >>>  - Logical division in multiple patches
> >>>
> >>> v2->v3:
> >>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> >>>    Which is based on top of v2.10-rc0 that
> >>>  - Fixed issue with two PCI devices
> >>>  - Addressed review comments
> >>>
> >>> v1->v2:
> >>>   - Added trace events
> >>>   - removed vSMMU3 link in patch description
> >>>
> >>> Bharat Bhushan (5):
> >>>   target/arm/kvm: Translate the MSI doorbell in
> >> kvm_arch_fixup_msi_route
> >>>   virtio-iommu: Add iommu notifier for map/unmap
> >>>   virtio-iommu: Call iommu notifier for attach/detach
> >>>   virtio-iommu: add iommu replay
> >>>   virtio-iommu: add iommu notifier memory-region
> >>>
> >>>  hw/virtio/trace-events           |   5 ++
> >>>  hw/virtio/virtio-iommu.c         | 181
> >> ++++++++++++++++++++++++++++++++++++++-
> >>>  include/hw/virtio/virtio-iommu.h |   6 ++
> >>>  target/arm/kvm.c                 |  27 ++++++
> >>>  target/arm/trace-events          |   3 +
> >>>  5 files changed, 219 insertions(+), 3 deletions(-)
> >>>
> >>> --
> >>> 1.9.3
> >>>
> >>>
> >>
> >> --
> >> Linu cherian

-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Linu Cherian 6 years, 7 months ago
On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
> Hi Linu,
> 
> On 27/09/2017 10:30, Bharat Bhushan wrote:
> > Hi,
> > 
> >> -----Original Message-----
> >> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> >> Sent: Wednesday, September 27, 2017 1:11 PM
> >> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> >> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> >> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> >> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> >> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> >> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> >> bharatb.yadav@gmail.com
> >> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> >>
> >> Hi,
> >>
> >> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> >>> This patch series integrates VFIO/VHOST with virtio-iommu.
> >>>
> >>> This version is mainly about rebasing on v4 version on virtio-iommu
> >>> device framework from Eric Augur and addresing review comments.
> >>>
> >>> This patch series allows PCI pass-through using virtio-iommu.
> >>>
> >>> This series is based on:
> >>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >>>     [1] [RFC] virtio-iommu version 0.4
> >>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> 
> Just to make sure, do you use the v0.4 virtio-iommu driver from above
> branch?
> 
> Thanks

I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
Hope you are referring to the same.


> 
> Eric
> >>>
> >>>  - virtio-iommu device emulation by Eric Augur.
> >>>    [RFC v4 00/16] VIRTIO-IOMMU device
> >>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> >>>
> >>> Changes are available at : https://github.com/bharaty/qemu.git
> >>> virtio-iommu-vfio-integration-v4
> >>>
> >>
> >> # With the above sources, was trying to test the vfio-pci device assigned to
> >>   guest using Qemu.
> >> # Both guest and host kernels are configured with 4k as page size.
> >> # releavant qemu command snippet,
> >>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
> >>   -net none -device vfio-pci,host=xxx
> >>
> >>
> >> On guest booting, observed mutliple messages as below,
> >>
> >> qemu-system-aarch64: iommu has granularity incompatible with target AS
> >>
> >> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
> >>   and the code expects the address mask to be 0xfff.
> > 
> > I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
> > 
> > Thanks
> > -Bharat
> > 
> >>
> >> if (len & iotlb->addr_mask) {
> >>         error_report
> >>
> >> # vfio_dma_map is failing due to this error.
> >>
> >> Any pointers ?
> >>
> >>
> >>> v3->v4:
> >>>  - Rebase to v4 version from Eric
> >>>  - Fixes from Eric with DPDK in VM
> >>>  - Logical division in multiple patches
> >>>
> >>> v2->v3:
> >>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> >>>    Which is based on top of v2.10-rc0 that
> >>>  - Fixed issue with two PCI devices
> >>>  - Addressed review comments
> >>>
> >>> v1->v2:
> >>>   - Added trace events
> >>>   - removed vSMMU3 link in patch description
> >>>
> >>> Bharat Bhushan (5):
> >>>   target/arm/kvm: Translate the MSI doorbell in
> >> kvm_arch_fixup_msi_route
> >>>   virtio-iommu: Add iommu notifier for map/unmap
> >>>   virtio-iommu: Call iommu notifier for attach/detach
> >>>   virtio-iommu: add iommu replay
> >>>   virtio-iommu: add iommu notifier memory-region
> >>>
> >>>  hw/virtio/trace-events           |   5 ++
> >>>  hw/virtio/virtio-iommu.c         | 181
> >> ++++++++++++++++++++++++++++++++++++++-
> >>>  include/hw/virtio/virtio-iommu.h |   6 ++
> >>>  target/arm/kvm.c                 |  27 ++++++
> >>>  target/arm/trace-events          |   3 +
> >>>  5 files changed, 219 insertions(+), 3 deletions(-)
> >>>
> >>> --
> >>> 1.9.3
> >>>
> >>>
> >>
> >> --
> >> Linu cherian

-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 7 months ago
Hi Linu,

On 27/09/2017 11:21, Linu Cherian wrote:
> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>> Hi Linu,
>>
>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>> Hi,
>>>
>>>> -----Original Message-----
>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>> bharatb.yadav@gmail.com
>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>
>>>> Hi,
>>>>
>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>
>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>> device framework from Eric Augur and addresing review comments.
>>>>>
>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>
>>>>> This series is based on:
>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>
>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>> branch?
>>
>> Thanks
> 
> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
> Hope you are referring to the same.

Yes that's the right one. I will also investigate on my side this afternoon.

Thanks

Eric
> 
> 
>>
>> Eric
>>>>>
>>>>>  - virtio-iommu device emulation by Eric Augur.
>>>>>    [RFC v4 00/16] VIRTIO-IOMMU device
>>>>>    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
>>>>>
>>>>> Changes are available at : https://github.com/bharaty/qemu.git
>>>>> virtio-iommu-vfio-integration-v4
>>>>>
>>>>
>>>> # With the above sources, was trying to test the vfio-pci device assigned to
>>>>   guest using Qemu.
>>>> # Both guest and host kernels are configured with 4k as page size.
>>>> # releavant qemu command snippet,
>>>>   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
>>>>   -net none -device vfio-pci,host=xxx
>>>>
>>>>
>>>> On guest booting, observed mutliple messages as below,
>>>>
>>>> qemu-system-aarch64: iommu has granularity incompatible with target AS
>>>>
>>>> # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
>>>>   and the code expects the address mask to be 0xfff.
>>>
>>> I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 
>>>
>>> Thanks
>>> -Bharat
>>>
>>>>
>>>> if (len & iotlb->addr_mask) {
>>>>         error_report
>>>>
>>>> # vfio_dma_map is failing due to this error.
>>>>
>>>> Any pointers ?
>>>>
>>>>
>>>>> v3->v4:
>>>>>  - Rebase to v4 version from Eric
>>>>>  - Fixes from Eric with DPDK in VM
>>>>>  - Logical division in multiple patches
>>>>>
>>>>> v2->v3:
>>>>>  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
>>>>>    Which is based on top of v2.10-rc0 that
>>>>>  - Fixed issue with two PCI devices
>>>>>  - Addressed review comments
>>>>>
>>>>> v1->v2:
>>>>>   - Added trace events
>>>>>   - removed vSMMU3 link in patch description
>>>>>
>>>>> Bharat Bhushan (5):
>>>>>   target/arm/kvm: Translate the MSI doorbell in
>>>> kvm_arch_fixup_msi_route
>>>>>   virtio-iommu: Add iommu notifier for map/unmap
>>>>>   virtio-iommu: Call iommu notifier for attach/detach
>>>>>   virtio-iommu: add iommu replay
>>>>>   virtio-iommu: add iommu notifier memory-region
>>>>>
>>>>>  hw/virtio/trace-events           |   5 ++
>>>>>  hw/virtio/virtio-iommu.c         | 181
>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>  include/hw/virtio/virtio-iommu.h |   6 ++
>>>>>  target/arm/kvm.c                 |  27 ++++++
>>>>>  target/arm/trace-events          |   3 +
>>>>>  5 files changed, 219 insertions(+), 3 deletions(-)
>>>>>
>>>>> --
>>>>> 1.9.3
>>>>>
>>>>>
>>>>
>>>> --
>>>> Linu cherian
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Linu Cherian 6 years, 6 months ago
Hi Eric,


On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
> Hi Linu,
> 
> On 27/09/2017 11:21, Linu Cherian wrote:
> > On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
> >> Hi Linu,
> >>
> >> On 27/09/2017 10:30, Bharat Bhushan wrote:
> >>> Hi,
> >>>
> >>>> -----Original Message-----
> >>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
> >>>> Sent: Wednesday, September 27, 2017 1:11 PM
> >>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
> >>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> >>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> >>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> >>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> >>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> >>>> bharatb.yadav@gmail.com
> >>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> >>>>
> >>>> Hi,
> >>>>
> >>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> >>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
> >>>>>
> >>>>> This version is mainly about rebasing on v4 version on virtio-iommu
> >>>>> device framework from Eric Augur and addresing review comments.
> >>>>>
> >>>>> This patch series allows PCI pass-through using virtio-iommu.
> >>>>>
> >>>>> This series is based on:
> >>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
> >>>>>     [1] [RFC] virtio-iommu version 0.4
> >>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> >>
> >> Just to make sure, do you use the v0.4 virtio-iommu driver from above
> >> branch?
> >>
> >> Thanks
> > 
> > I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
> > Hope you are referring to the same.
> 
> Yes that's the right one. I will also investigate on my side this afternoon.
> 
> Thanks
> 
> Eric

With the below workaround, atleast ping works for me.

diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 249964a..2904617 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
        .attach_dev             = viommu_attach_dev,
        .map                    = viommu_map,
        .unmap                  = viommu_unmap,
-       .map_sg                 = viommu_map_sg,
+       .map_sg                 = default_iommu_map_sg,
        .iova_to_phys           = viommu_iova_to_phys,
        .add_device             = viommu_add_device,
        .remove_device          = viommu_remove_device,


Looks like the qemu backend doesnt have support to handle the map requests from 
virtio_iommu_map_sg, since it  merges multiple map requests into one with 
mapsize larger than page size(for eg. 0x5000). 

Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
the map size to be a power of 2.

 if (len & iotlb->addr_mask) {
        error_report("iommu has granularity incompatible with target AS");
        return false;
    }

Just trying to understand how this is not hitting in your case. 
 
-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 6 months ago
Hi Linu,
On 04/10/2017 13:49, Linu Cherian wrote:
> Hi Eric,
> 
> 
> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>> Hi Linu,
>>
>> On 27/09/2017 11:21, Linu Cherian wrote:
>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>> Hi Linu,
>>>>
>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>> Hi,
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>> bharatb.yadav@gmail.com
>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>
>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>
>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>
>>>>>>> This series is based on:
>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>
>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>> branch?
>>>>
>>>> Thanks
>>>
>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>> Hope you are referring to the same.
>>
>> Yes that's the right one. I will also investigate on my side this afternoon.
>>
>> Thanks
>>
>> Eric
> 
> With the below workaround, atleast ping works for me.
> 
> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
> index 249964a..2904617 100644
> --- a/drivers/iommu/virtio-iommu.c
> +++ b/drivers/iommu/virtio-iommu.c
>         .attach_dev             = viommu_attach_dev,
>         .map                    = viommu_map,
>         .unmap                  = viommu_unmap,
> -       .map_sg                 = viommu_map_sg,
> +       .map_sg                 = default_iommu_map_sg,
>         .iova_to_phys           = viommu_iova_to_phys,
>         .add_device             = viommu_add_device,
>         .remove_device          = viommu_remove_device,
> 
> 
> Looks like the qemu backend doesnt have support to handle the map requests from 
> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
> mapsize larger than page size(for eg. 0x5000). 
On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
request for each sg element. The map size matches the sg element size.
Then each request is sent separately in _viommu_send_reqs_sync. I don't
see any concatenation. Looks Jean has a plan to check if it can
concatenate anything (/* TODO: merge physically-contiguous mappings if
any */) but this is not implemented yet.

However you should be allowed to map 1 sg element of 5 pages and then
notify the host about this event I think. Still looking at the code...

I still can't reproduce the issue at the moment. What kind of device are
you assigning?

Thanks

Eric
> 
> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
> the map size to be a power of 2.
> 
>  if (len & iotlb->addr_mask) {
>         error_report("iommu has granularity incompatible with target AS");
>         return false;
>     }
> 
> Just trying to understand how this is not hitting in your case. 
>  
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 6 months ago
Hi Linu,
On 05/10/2017 12:46, Auger Eric wrote:
> Hi Linu,
> On 04/10/2017 13:49, Linu Cherian wrote:
>> Hi Eric,
>>
>>
>> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>>> Hi Linu,
>>>
>>> On 27/09/2017 11:21, Linu Cherian wrote:
>>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>>> Hi Linu,
>>>>>
>>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>>> Hi,
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>>> bharatb.yadav@gmail.com
>>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>>
>>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>>
>>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>>
>>>>>>>> This series is based on:
>>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>>
>>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>>> branch?
>>>>>
>>>>> Thanks
>>>>
>>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>>> Hope you are referring to the same.
>>>
>>> Yes that's the right one. I will also investigate on my side this afternoon.
>>>
>>> Thanks
>>>
>>> Eric
>>
>> With the below workaround, atleast ping works for me.
>>
>> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
>> index 249964a..2904617 100644
>> --- a/drivers/iommu/virtio-iommu.c
>> +++ b/drivers/iommu/virtio-iommu.c
>>         .attach_dev             = viommu_attach_dev,
>>         .map                    = viommu_map,
>>         .unmap                  = viommu_unmap,
>> -       .map_sg                 = viommu_map_sg,
>> +       .map_sg                 = default_iommu_map_sg,
>>         .iova_to_phys           = viommu_iova_to_phys,
>>         .add_device             = viommu_add_device,
>>         .remove_device          = viommu_remove_device,
>>
>>
>> Looks like the qemu backend doesnt have support to handle the map requests from 
>> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
>> mapsize larger than page size(for eg. 0x5000). 
> On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
> request for each sg element. The map size matches the sg element size.
> Then each request is sent separately in _viommu_send_reqs_sync. I don't
> see any concatenation. Looks Jean has a plan to check if it can
> concatenate anything (/* TODO: merge physically-contiguous mappings if
> any */) but this is not implemented yet.

Hopefully I was just able to reproduce your issue with an igb device. I
keep on debugging...

vfio_get_vaddr 1 len=0x3000 iotlb->addr_mask=0x2fff
qemu-system-aarch64: iommu has granularity incompatible with target AS


Thanks

Eric
> 
> However you should be allowed to map 1 sg element of 5 pages and then
> notify the host about this event I think. Still looking at the code...
> 
> I still can't reproduce the issue at the moment. What kind of device are
> you assigning?
> 
> Thanks
> 
> Eric
>>
>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
>> the map size to be a power of 2.
>>
>>  if (len & iotlb->addr_mask) {
>>         error_report("iommu has granularity incompatible with target AS");
>>         return false;
>>     }
>>
>> Just trying to understand how this is not hitting in your case. 
>>  
>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 6 months ago
Hi Linu,

On 05/10/2017 13:54, Auger Eric wrote:
> Hi Linu,
> On 05/10/2017 12:46, Auger Eric wrote:
>> Hi Linu,
>> On 04/10/2017 13:49, Linu Cherian wrote:
>>> Hi Eric,
>>>
>>>
>>> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>>>> Hi Linu,
>>>>
>>>> On 27/09/2017 11:21, Linu Cherian wrote:
>>>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>>>> Hi Linu,
>>>>>>
>>>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>>>> bharatb.yadav@gmail.com
>>>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>>>
>>>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>>>
>>>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>>>
>>>>>>>>> This series is based on:
>>>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>>>
>>>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>>>> branch?
>>>>>>
>>>>>> Thanks
>>>>>
>>>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>>>> Hope you are referring to the same.
>>>>
>>>> Yes that's the right one. I will also investigate on my side this afternoon.
>>>>
>>>> Thanks
>>>>
>>>> Eric
>>>
>>> With the below workaround, atleast ping works for me.
>>>
>>> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
>>> index 249964a..2904617 100644
>>> --- a/drivers/iommu/virtio-iommu.c
>>> +++ b/drivers/iommu/virtio-iommu.c
>>>         .attach_dev             = viommu_attach_dev,
>>>         .map                    = viommu_map,
>>>         .unmap                  = viommu_unmap,
>>> -       .map_sg                 = viommu_map_sg,
>>> +       .map_sg                 = default_iommu_map_sg,
>>>         .iova_to_phys           = viommu_iova_to_phys,
>>>         .add_device             = viommu_add_device,
>>>         .remove_device          = viommu_remove_device,
>>>
>>>
>>> Looks like the qemu backend doesnt have support to handle the map requests from 
>>> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
>>> mapsize larger than page size(for eg. 0x5000). 
>> On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
>> request for each sg element. The map size matches the sg element size.
>> Then each request is sent separately in _viommu_send_reqs_sync. I don't
>> see any concatenation. Looks Jean has a plan to check if it can
>> concatenate anything (/* TODO: merge physically-contiguous mappings if
>> any */) but this is not implemented yet.
> 
> Hopefully I was just able to reproduce your issue with an igb device. I
> keep on debugging...
> 
> vfio_get_vaddr 1 len=0x3000 iotlb->addr_mask=0x2fff
> qemu-system-aarch64: iommu has granularity incompatible with target AS
> 
> 
> Thanks
> 
> Eric
>>
>> However you should be allowed to map 1 sg element of 5 pages and then
>> notify the host about this event I think. Still looking at the code...
>>
>> I still can't reproduce the issue at the moment. What kind of device are
>> you assigning?
>>
>> Thanks
>>
>> Eric
>>>
>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
>>> the map size to be a power of 2.

Actually I missed the most important here ;-)
>>>
>>>  if (len & iotlb->addr_mask) {
This check looks suspiscious to me. In our case the len is not modified
by the previous translation and it fails, I don't see why. It should be
valid to be able to notify 5 granules.

Thanks

Eric
>>>         error_report("iommu has granularity incompatible with target AS");
>>>         return false;
>>>     }
>>>
>>> Just trying to understand how this is not hitting in your case. 
>>>  
>>>
>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 6 months ago
Hi Linu,

On 05/10/2017 14:13, Auger Eric wrote:
> Hi Linu,
> 
> On 05/10/2017 13:54, Auger Eric wrote:
>> Hi Linu,
>> On 05/10/2017 12:46, Auger Eric wrote:
>>> Hi Linu,
>>> On 04/10/2017 13:49, Linu Cherian wrote:
>>>> Hi Eric,
>>>>
>>>>
>>>> On Wed Sep 27, 2017 at 11:24:01AM +0200, Auger Eric wrote:
>>>>> Hi Linu,
>>>>>
>>>>> On 27/09/2017 11:21, Linu Cherian wrote:
>>>>>> On Wed Sep 27, 2017 at 10:55:07AM +0200, Auger Eric wrote:
>>>>>>> Hi Linu,
>>>>>>>
>>>>>>> On 27/09/2017 10:30, Bharat Bhushan wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Linu Cherian [mailto:linuc.decode@gmail.com]
>>>>>>>>> Sent: Wednesday, September 27, 2017 1:11 PM
>>>>>>>>> To: Bharat Bhushan <bharat.bhushan@nxp.com>
>>>>>>>>> Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
>>>>>>>>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>>>>>>>>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
>>>>>>>>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>>>>>>>>> drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
>>>>>>>>> bharatb.yadav@gmail.com
>>>>>>>>> Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
>>>>>>>>>> This patch series integrates VFIO/VHOST with virtio-iommu.
>>>>>>>>>>
>>>>>>>>>> This version is mainly about rebasing on v4 version on virtio-iommu
>>>>>>>>>> device framework from Eric Augur and addresing review comments.
>>>>>>>>>>
>>>>>>>>>> This patch series allows PCI pass-through using virtio-iommu.
>>>>>>>>>>
>>>>>>>>>> This series is based on:
>>>>>>>>>>  - virtio-iommu kernel driver by Jean-Philippe Brucker
>>>>>>>>>>     [1] [RFC] virtio-iommu version 0.4
>>>>>>>>>>     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
>>>>>>>
>>>>>>> Just to make sure, do you use the v0.4 virtio-iommu driver from above
>>>>>>> branch?
>>>>>>>
>>>>>>> Thanks
>>>>>>
>>>>>> I am using git://linux-arm.org/linux-jpb.git branch virtio-iommu/v0.4.
>>>>>> Hope you are referring to the same.
>>>>>
>>>>> Yes that's the right one. I will also investigate on my side this afternoon.
>>>>>
>>>>> Thanks
>>>>>
>>>>> Eric
>>>>
>>>> With the below workaround, atleast ping works for me.
>>>>
>>>> diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
>>>> index 249964a..2904617 100644
>>>> --- a/drivers/iommu/virtio-iommu.c
>>>> +++ b/drivers/iommu/virtio-iommu.c
>>>>         .attach_dev             = viommu_attach_dev,
>>>>         .map                    = viommu_map,
>>>>         .unmap                  = viommu_unmap,
>>>> -       .map_sg                 = viommu_map_sg,
>>>> +       .map_sg                 = default_iommu_map_sg,
>>>>         .iova_to_phys           = viommu_iova_to_phys,
>>>>         .add_device             = viommu_add_device,
>>>>         .remove_device          = viommu_remove_device,
>>>>
>>>>
>>>> Looks like the qemu backend doesnt have support to handle the map requests from 
>>>> virtio_iommu_map_sg, since it  merges multiple map requests into one with 
>>>> mapsize larger than page size(for eg. 0x5000). 
>>> On my side I understand viommu_map_sg builds a VIRTIO_IOMMU_T_MAP
>>> request for each sg element. The map size matches the sg element size.
>>> Then each request is sent separately in _viommu_send_reqs_sync. I don't
>>> see any concatenation. Looks Jean has a plan to check if it can
>>> concatenate anything (/* TODO: merge physically-contiguous mappings if
>>> any */) but this is not implemented yet.
>>
>> Hopefully I was just able to reproduce your issue with an igb device. I
>> keep on debugging...
>>
>> vfio_get_vaddr 1 len=0x3000 iotlb->addr_mask=0x2fff
>> qemu-system-aarch64: iommu has granularity incompatible with target AS
>>
>>
>> Thanks
>>
>> Eric
>>>
>>> However you should be allowed to map 1 sg element of 5 pages and then
>>> notify the host about this event I think. Still looking at the code...
>>>
>>> I still can't reproduce the issue at the moment. What kind of device are
>>> you assigning?
>>>
>>> Thanks
>>>
>>> Eric
>>>>
>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu expects 
>>>> the map size to be a power of 2.
> 
> Actually I missed the most important here ;-)
>>>>
>>>>  if (len & iotlb->addr_mask) {
> This check looks suspiscious to me. In our case the len is not modified
> by the previous translation and it fails, I don't see why. It should be
> valid to be able to notify 5 granules.

So after discussion with Alex, looks the way we notify the host
currently is wrong. we set the addr_mask to the mapping/unmapping size
-1 whereas this should be a page mask instead (granule size or block
size?). So if the guest maps 5 x 4kB pages we should send 5
notifications for each page and not a single one. It is unclear to me if
we can notify with hugepage/block page size mask. Peter may
confirm/infirm this. in vsmmuv3 code I notify by granule or block size.

Bharat, please can you add this to your TODO list?

Linu, thanks a lot for the time you spent debugging this issue.
Curiously on my side, it is really seldom hit but it is ...

Thanks!

Eric
> 
> Thanks
> 
> Eric
>>>>         error_report("iommu has granularity incompatible with target AS");
>>>>         return false;
>>>>     }
>>>>
>>>> Just trying to understand how this is not hitting in your case. 
>>>>  
>>>>
>>>
>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Bharat Bhushan 6 years, 6 months ago

> >> Thanks
> >>
> >> Eric
> >>>
> >>> However you should be allowed to map 1 sg element of 5 pages and
> >>> then notify the host about this event I think. Still looking at the code...
> >>>
> >>> I still can't reproduce the issue at the moment. What kind of device
> >>> are you assigning?
> >>>
> >>> Thanks
> >>>
> >>> Eric
> >>>>
> >>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
> >>>> expects the map size to be a power of 2.
> >
> > Actually I missed the most important here ;-)
> >>>>
> >>>>  if (len & iotlb->addr_mask) {
> > This check looks suspiscious to me. In our case the len is not
> > modified by the previous translation and it fails, I don't see why. It
> > should be valid to be able to notify 5 granules.
> 
> So after discussion with Alex, looks the way we notify the host currently is
> wrong. we set the addr_mask to the mapping/unmapping size
> -1 whereas this should be a page mask instead (granule size or block size?).
> So if the guest maps 5 x 4kB pages we should send 5 notifications for each
> page and not a single one. It is unclear to me if we can notify with
> hugepage/block page size mask. Peter may confirm/infirm this. in vsmmuv3
> code I notify by granule or block size.
> 
> Bharat, please can you add this to your TODO list?
> 
> Linu, thanks a lot for the time you spent debugging this issue.
> Curiously on my side, it is really seldom hit but it is ...

Thanks Linu and Eric, I added this to my todo list.
While I am still not able to reproduce the issue.  I tried with e1000 and now try with ixgbe device. May I know which device can be used to reproduce this issue?

Thanks
-Bharat

> 
> Thanks!
> 
> Eric
> >
> > Thanks
> >
> > Eric
> >>>>         error_report("iommu has granularity incompatible with target AS");
> >>>>         return false;
> >>>>     }
> >>>>
> >>>> Just trying to understand how this is not hitting in your case.
> >>>>
> >>>>
> >>>
> >>
> >

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 6 months ago
Hi Bharat,

On 06/10/2017 05:46, Bharat Bhushan wrote:
> 
> 
>>>> Thanks
>>>>
>>>> Eric
>>>>>
>>>>> However you should be allowed to map 1 sg element of 5 pages and
>>>>> then notify the host about this event I think. Still looking at the code...
>>>>>
>>>>> I still can't reproduce the issue at the moment. What kind of device
>>>>> are you assigning?
>>>>>
>>>>> Thanks
>>>>>
>>>>> Eric
>>>>>>
>>>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
>>>>>> expects the map size to be a power of 2.
>>>
>>> Actually I missed the most important here ;-)
>>>>>>
>>>>>>  if (len & iotlb->addr_mask) {
>>> This check looks suspiscious to me. In our case the len is not
>>> modified by the previous translation and it fails, I don't see why. It
>>> should be valid to be able to notify 5 granules.
>>
>> So after discussion with Alex, looks the way we notify the host currently is
>> wrong. we set the addr_mask to the mapping/unmapping size
>> -1 whereas this should be a page mask instead (granule size or block size?).
>> So if the guest maps 5 x 4kB pages we should send 5 notifications for each
>> page and not a single one. It is unclear to me if we can notify with
>> hugepage/block page size mask. Peter may confirm/infirm this. in vsmmuv3
>> code I notify by granule or block size.
>>
>> Bharat, please can you add this to your TODO list?
>>
>> Linu, thanks a lot for the time you spent debugging this issue.
>> Curiously on my side, it is really seldom hit but it is ...
> 
> Thanks Linu and Eric, I added this to my todo list.
> While I am still not able to reproduce the issue.  I tried with e1000 and now try with ixgbe device. May I know which device can be used to reproduce this issue?

On my side I used an i350T2 device (igb) to reproduce the issue.

Thanks

Eric
> 
> Thanks
> -Bharat
> 
>>
>> Thanks!
>>
>> Eric
>>>
>>> Thanks
>>>
>>> Eric
>>>>>>         error_report("iommu has granularity incompatible with target AS");
>>>>>>         return false;
>>>>>>     }
>>>>>>
>>>>>> Just trying to understand how this is not hitting in your case.
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Linu Cherian 6 years, 6 months ago
On Fri Oct 06, 2017 at 09:24:20AM +0200, Auger Eric wrote:
> Hi Bharat,
> 
> On 06/10/2017 05:46, Bharat Bhushan wrote:
> > 
> > 
> >>>> Thanks
> >>>>
> >>>> Eric
> >>>>>
> >>>>> However you should be allowed to map 1 sg element of 5 pages and
> >>>>> then notify the host about this event I think. Still looking at the code...
> >>>>>
> >>>>> I still can't reproduce the issue at the moment. What kind of device
> >>>>> are you assigning?
> >>>>>
> >>>>> Thanks
> >>>>>
> >>>>> Eric
> >>>>>>
> >>>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
> >>>>>> expects the map size to be a power of 2.
> >>>
> >>> Actually I missed the most important here ;-)
> >>>>>>
> >>>>>>  if (len & iotlb->addr_mask) {
> >>> This check looks suspiscious to me. In our case the len is not
> >>> modified by the previous translation and it fails, I don't see why. It
> >>> should be valid to be able to notify 5 granules.
> >>
> >> So after discussion with Alex, looks the way we notify the host currently is
> >> wrong. we set the addr_mask to the mapping/unmapping size
> >> -1 whereas this should be a page mask instead (granule size or block size?).
> >> So if the guest maps 5 x 4kB pages we should send 5 notifications for each
> >> page and not a single one. It is unclear to me if we can notify with
> >> hugepage/block page size mask. Peter may confirm/infirm this. in vsmmuv3
> >> code I notify by granule or block size.
> >>
> >> Bharat, please can you add this to your TODO list?
> >>
> >> Linu, thanks a lot for the time you spent debugging this issue.
> >> Curiously on my side, it is really seldom hit but it is ...
> > 
> > Thanks Linu and Eric, I added this to my todo list.
> > While I am still not able to reproduce the issue.  I tried with e1000 and now try with ixgbe device. May I know which device can be used to reproduce this issue?
> 
> On my side I used an i350T2 device (igb) to reproduce the issue.

Myself used a Thunderx NIC which is an on chip PCI device.


-- 
Linu cherian

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Bharat Bhushan 6 years, 6 months ago
Hi Alex, Eric,

> -----Original Message-----
> From: Qemu-devel [mailto:qemu-devel-
> bounces+bharat.bhushan=nxp.com@nongnu.org] On Behalf Of Bharat
> Bhushan
> Sent: Friday, October 06, 2017 9:16 AM
> To: Auger Eric <eric.auger@redhat.com>; Linu Cherian
> <linuc.decode@gmail.com>
> Cc: peter.maydell@linaro.org; kevin.tian@intel.com; mst@redhat.com;
> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> drjones@redhat.com; qemu-devel@nongnu.org;
> alex.williamson@redhat.com; qemu-arm@nongnu.org;
> linu.cherian@cavium.com; eric.auger.pro@gmail.com;
> robin.murphy@arm.com; christoffer.dall@linaro.org;
> bharatb.yadav@gmail.com
> Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO
> integration
> 
> 
> 
> > >> Thanks
> > >>
> > >> Eric
> > >>>
> > >>> However you should be allowed to map 1 sg element of 5 pages and
> > >>> then notify the host about this event I think. Still looking at the code...
> > >>>
> > >>> I still can't reproduce the issue at the moment. What kind of
> > >>> device are you assigning?
> > >>>
> > >>> Thanks
> > >>>
> > >>> Eric
> > >>>>
> > >>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
> > >>>> expects the map size to be a power of 2.
> > >
> > > Actually I missed the most important here ;-)
> > >>>>
> > >>>>  if (len & iotlb->addr_mask) {
> > > This check looks suspiscious to me. In our case the len is not
> > > modified by the previous translation and it fails, I don't see why.
> > > It should be valid to be able to notify 5 granules.
> >
> > So after discussion with Alex, looks the way we notify the host
> > currently is wrong. we set the addr_mask to the mapping/unmapping size
> > -1 whereas this should be a page mask instead (granule size or block size?).
> > So if the guest maps 5 x 4kB pages we should send 5 notifications for
> > each page and not a single one. It is unclear to me if we can notify
> > with hugepage/block page size mask. Peter may confirm/infirm this. in
> > vsmmuv3 code I notify by granule or block size.

My understanding is that host provides supported page sizes (page_size_mask), and Size of each notification to host should be exactly best fit of supported page-size and/or multiples of supported page-sizes.
So if guest maps 20K size (single request), and supported page size is 4K, so we can still send one 20K size request.
Not sure I get it, why multiples of supported page-size cannot be provided in one notification to host.

Thanks
-Bharat

> >
> > Bharat, please can you add this to your TODO list?
> >
> > Linu, thanks a lot for the time you spent debugging this issue.
> > Curiously on my side, it is really seldom hit but it is ...
> 
> Thanks Linu and Eric, I added this to my todo list.
> While I am still not able to reproduce the issue.  I tried with e1000 and now
> try with ixgbe device. May I know which device can be used to reproduce this
> issue?
> 
> Thanks
> -Bharat
> 
> >
> > Thanks!
> >
> > Eric
> > >
> > > Thanks
> > >
> > > Eric
> > >>>>         error_report("iommu has granularity incompatible with target
> AS");
> > >>>>         return false;
> > >>>>     }
> > >>>>
> > >>>> Just trying to understand how this is not hitting in your case.
> > >>>>
> > >>>>
> > >>>
> > >>
> > >


Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Auger Eric 6 years, 6 months ago
Hi Bharat,

On 10/10/2017 08:42, Bharat Bhushan wrote:
> Hi Alex, Eric,
> 
>> -----Original Message-----
>> From: Qemu-devel [mailto:qemu-devel-
>> bounces+bharat.bhushan=nxp.com@nongnu.org] On Behalf Of Bharat
>> Bhushan
>> Sent: Friday, October 06, 2017 9:16 AM
>> To: Auger Eric <eric.auger@redhat.com>; Linu Cherian
>> <linuc.decode@gmail.com>
>> Cc: peter.maydell@linaro.org; kevin.tian@intel.com; mst@redhat.com;
>> marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
>> drjones@redhat.com; qemu-devel@nongnu.org;
>> alex.williamson@redhat.com; qemu-arm@nongnu.org;
>> linu.cherian@cavium.com; eric.auger.pro@gmail.com;
>> robin.murphy@arm.com; christoffer.dall@linaro.org;
>> bharatb.yadav@gmail.com
>> Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO
>> integration
>>
>>
>>
>>>>> Thanks
>>>>>
>>>>> Eric
>>>>>>
>>>>>> However you should be allowed to map 1 sg element of 5 pages and
>>>>>> then notify the host about this event I think. Still looking at the code...
>>>>>>
>>>>>> I still can't reproduce the issue at the moment. What kind of
>>>>>> device are you assigning?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Eric
>>>>>>>
>>>>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
>>>>>>> expects the map size to be a power of 2.
>>>>
>>>> Actually I missed the most important here ;-)
>>>>>>>
>>>>>>>  if (len & iotlb->addr_mask) {
>>>> This check looks suspiscious to me. In our case the len is not
>>>> modified by the previous translation and it fails, I don't see why.
>>>> It should be valid to be able to notify 5 granules.
>>>
>>> So after discussion with Alex, looks the way we notify the host
>>> currently is wrong. we set the addr_mask to the mapping/unmapping size
>>> -1 whereas this should be a page mask instead (granule size or block size?).
>>> So if the guest maps 5 x 4kB pages we should send 5 notifications for
>>> each page and not a single one. It is unclear to me if we can notify
>>> with hugepage/block page size mask. Peter may confirm/infirm this. in
>>> vsmmuv3 code I notify by granule or block size.
> 
> My understanding is that host provides supported page sizes (page_size_mask), and Size of each notification to host should be exactly best fit of supported page-size and/or multiples of supported page-sizes.
> So if guest maps 20K size (single request), and supported page size is 4K, so we can still send one 20K size request.
> Not sure I get it, why multiples of supported page-size cannot be provided in one notification to host.
I think the IOTLB API was originally devised to manage only granule or
block ^2 sizes. We might change this in the future but for the moment,
with respect to this series,  I would simply recommend to stick to the
existing API limitation and concurrently we can work on relaxing this
constraint on another series.

Thanks

Eric
> 
> Thanks
> -Bharat
> 
>>>
>>> Bharat, please can you add this to your TODO list?
>>>
>>> Linu, thanks a lot for the time you spent debugging this issue.
>>> Curiously on my side, it is really seldom hit but it is ...
>>
>> Thanks Linu and Eric, I added this to my todo list.
>> While I am still not able to reproduce the issue.  I tried with e1000 and now
>> try with ixgbe device. May I know which device can be used to reproduce this
>> issue?
>>
>> Thanks
>> -Bharat
>>
>>>
>>> Thanks!
>>>
>>> Eric
>>>>
>>>> Thanks
>>>>
>>>> Eric
>>>>>>>         error_report("iommu has granularity incompatible with target
>> AS");
>>>>>>>         return false;
>>>>>>>     }
>>>>>>>
>>>>>>> Just trying to understand how this is not hitting in your case.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
Posted by Linu Cherian 6 years, 7 months ago
On Wed Sep 27, 2017 at 08:30:51AM +0000, Bharat Bhushan wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Linu Cherian [mailto:linuc.decode@gmail.com]
> > Sent: Wednesday, September 27, 2017 1:11 PM
> > To: Bharat Bhushan <bharat.bhushan@nxp.com>
> > Cc: eric.auger@redhat.com; eric.auger.pro@gmail.com;
> > peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
> > qemu-arm@nongnu.org; qemu-devel@nongnu.org; kevin.tian@intel.com;
> > marc.zyngier@arm.com; tn@semihalf.com; will.deacon@arm.com;
> > drjones@redhat.com; robin.murphy@arm.com; christoffer.dall@linaro.org;
> > bharatb.yadav@gmail.com
> > Subject: Re: [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO integration
> > 
> > Hi,
> > 
> > On Wed Sep 27, 2017 at 12:03:15PM +0530, Bharat Bhushan wrote:
> > > This patch series integrates VFIO/VHOST with virtio-iommu.
> > >
> > > This version is mainly about rebasing on v4 version on virtio-iommu
> > > device framework from Eric Augur and addresing review comments.
> > >
> > > This patch series allows PCI pass-through using virtio-iommu.
> > >
> > > This series is based on:
> > >  - virtio-iommu kernel driver by Jean-Philippe Brucker
> > >     [1] [RFC] virtio-iommu version 0.4
> > >     git://linux-arm.org/virtio-iommu.git branch viommu/v0.4
> > >
> > >  - virtio-iommu device emulation by Eric Augur.
> > >    [RFC v4 00/16] VIRTIO-IOMMU device
> > >    https://github.com/eauger/qemu/tree/v2.10.0-virtio-iommu-v4
> > >
> > > Changes are available at : https://github.com/bharaty/qemu.git
> > > virtio-iommu-vfio-integration-v4
> > >
> > 
> > # With the above sources, was trying to test the vfio-pci device assigned to
> >   guest using Qemu.
> > # Both guest and host kernels are configured with 4k as page size.
> > # releavant qemu command snippet,
> >   -device virtio-iommu-device -device virtio-blk-device,drive=hd0 \
> >   -net none -device vfio-pci,host=xxx
> > 
> > 
> > On guest booting, observed mutliple messages as below,
> > 
> > qemu-system-aarch64: iommu has granularity incompatible with target AS
> > 
> > # On adding necessary prints, 0x5000 is len, 0x4fff is address mask
> >   and the code expects the address mask to be 0xfff.
> 
> I have not seen these errors, I am also using 4K page-size on both host and guest. Can you share compete qemu command and log. 

Qemu cmd:

qemu-system-aarch64 -machine virt,gic_version=3 -cpu host -enable-kvm -nographic
 -smp 8 -m 1024 -kernel /boot/Image --append "console=ttyAMA0 root=/dev/vda rw" 
-drive if=none,file=/home/ubuntu/fs_images/ubu1604.img,id=hd0,format=raw 
-device virtio-iommu-device -device virtio-blk-device,drive=hd0 -net none -device vfio-pci,host=0002:01:00.3


Qemu log:
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
qemu-system-aarch64: iommu has granularity incompatible with target AS
...





> 
> Thanks
> -Bharat
> 
> > 
> > if (len & iotlb->addr_mask) {
> >         error_report
> > 
> > # vfio_dma_map is failing due to this error.
> > 
> > Any pointers ?
> > 
> > 
> > > v3->v4:
> > >  - Rebase to v4 version from Eric
> > >  - Fixes from Eric with DPDK in VM
> > >  - Logical division in multiple patches
> > >
> > > v2->v3:
> > >  - This series is based on "[RFC v3 0/8] VIRTIO-IOMMU device"
> > >    Which is based on top of v2.10-rc0 that
> > >  - Fixed issue with two PCI devices
> > >  - Addressed review comments
> > >
> > > v1->v2:
> > >   - Added trace events
> > >   - removed vSMMU3 link in patch description
> > >
> > > Bharat Bhushan (5):
> > >   target/arm/kvm: Translate the MSI doorbell in
> > kvm_arch_fixup_msi_route
> > >   virtio-iommu: Add iommu notifier for map/unmap
> > >   virtio-iommu: Call iommu notifier for attach/detach
> > >   virtio-iommu: add iommu replay
> > >   virtio-iommu: add iommu notifier memory-region
> > >
> > >  hw/virtio/trace-events           |   5 ++
> > >  hw/virtio/virtio-iommu.c         | 181
> > ++++++++++++++++++++++++++++++++++++++-
> > >  include/hw/virtio/virtio-iommu.h |   6 ++
> > >  target/arm/kvm.c                 |  27 ++++++
> > >  target/arm/trace-events          |   3 +
> > >  5 files changed, 219 insertions(+), 3 deletions(-)
> > >
> > > --
> > > 1.9.3
> > >
> > >
> > 
> > --
> > Linu cherian

-- 
Linu cherian