diff mbox series

[RFC,V1,02/12] iommufd: no DMA to BARs

Message ID 1721502937-87102-3-git-send-email-steven.sistare@oracle.com
State New
Headers show
Series Live update: iommufd | expand

Commit Message

Steven Sistare July 20, 2024, 7:15 p.m. UTC 
Do not map VFIO PCI BARs for DMA.  This stops a raft of warnings of the
following form at QEMU start time when using -object iommufd:

qemu-kvm: warning: IOMMU_IOAS_MAP failed: Bad address, PCI BAR?
qemu-kvm: vfio_container_dma_map(0x555558282db0, 0x8800010000, 0x4000, 0x7ffff7ff0000) = -14 (Bad address)

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 hw/vfio/common.c      | 3 ++-
 hw/vfio/helpers.c     | 1 +
 include/exec/memory.h | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

Comments

Alex Williamson Aug. 12, 2024, 10:05 p.m. UTC | #1 
On Sat, 20 Jul 2024 12:15:27 -0700
Steve Sistare <steven.sistare@oracle.com> wrote:

> Do not map VFIO PCI BARs for DMA.  This stops a raft of warnings of the
> following form at QEMU start time when using -object iommufd:
> 
> qemu-kvm: warning: IOMMU_IOAS_MAP failed: Bad address, PCI BAR?
> qemu-kvm: vfio_container_dma_map(0x555558282db0, 0x8800010000, 0x4000, 0x7ffff7ff0000) = -14 (Bad address)

NAK.  These mappings are required for P2P DMA between devices.  This is
currently a gap in IOMMUFD support that it doesn't have parity to legacy
vfio containers for these mappings.  Thanks,

Alex

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  hw/vfio/common.c      | 3 ++-
>  hw/vfio/helpers.c     | 1 +
>  include/exec/memory.h | 1 +
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index da2e0ec..403d45a 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -248,7 +248,8 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>              * are never accessed by the CPU and beyond the address width of
>              * some IOMMU hardware.  TODO: VFIO should tell us the IOMMU width.
>              */
> -           section->offset_within_address_space & (1ULL << 63);
> +           section->offset_within_address_space & (1ULL << 63) ||
> +           section->mr->no_dma;
>  }
>  
>  /* Called with rcu_read_lock held.  */
> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
> index b14edd4..e4cfdd2 100644
> --- a/hw/vfio/helpers.c
> +++ b/hw/vfio/helpers.c
> @@ -435,6 +435,7 @@ int vfio_region_mmap(VFIORegion *region)
>                                            memory_region_owner(region->mem),
>                                            name, region->mmaps[i].size,
>                                            region->mmaps[i].mmap);
> +        region->mmaps[i].mem.no_dma = true;
>          g_free(name);
>          memory_region_add_subregion(region->mem, region->mmaps[i].offset,
>                                      &region->mmaps[i].mem);
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index ea03ef2..850cc8c 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -794,6 +794,7 @@ struct MemoryRegion {
>      bool unmergeable;
>      uint8_t dirty_log_mask;
>      bool is_iommu;
> +    bool no_dma;
>      RAMBlock *ram_block;
>      Object *owner;
>      /* owner as TYPE_DEVICE. Used for re-entrancy checks in MR access hotpath */
Yi Liu Aug. 13, 2024, 1:39 a.m. UTC | #2 
On 2024/7/21 03:15, Steve Sistare wrote:
> Do not map VFIO PCI BARs for DMA.  This stops a raft of warnings of the
> following form at QEMU start time when using -object iommufd:
> 
> qemu-kvm: warning: IOMMU_IOAS_MAP failed: Bad address, PCI BAR?
> qemu-kvm: vfio_container_dma_map(0x555558282db0, 0x8800010000, 0x4000, 0x7ffff7ff0000) = -14 (Bad address)

It is required as Alex pointed, so no need to pay further attempt to hide
this message. And there were efforts to make it. But not done yet. Below
links may be helpful if you are interested about the history.

[1] https://lore.kernel.org/kvm/14-v4-0de2f6c78ed0+9d1-iommufd_jgg@nvidia.com/
[2] https://lore.kernel.org/kvm/20240624141139.GH29266@unreal/
[3] 
https://lore.kernel.org/kvm/0-v1-9e6e1739ed95+5fa-vfio_dma_buf_jgg@nvidia.com/

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   hw/vfio/common.c      | 3 ++-
>   hw/vfio/helpers.c     | 1 +
>   include/exec/memory.h | 1 +
>   3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index da2e0ec..403d45a 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -248,7 +248,8 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>               * are never accessed by the CPU and beyond the address width of
>               * some IOMMU hardware.  TODO: VFIO should tell us the IOMMU width.
>               */
> -           section->offset_within_address_space & (1ULL << 63);
> +           section->offset_within_address_space & (1ULL << 63) ||
> +           section->mr->no_dma;
>   }
>   
>   /* Called with rcu_read_lock held.  */
> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
> index b14edd4..e4cfdd2 100644
> --- a/hw/vfio/helpers.c
> +++ b/hw/vfio/helpers.c
> @@ -435,6 +435,7 @@ int vfio_region_mmap(VFIORegion *region)
>                                             memory_region_owner(region->mem),
>                                             name, region->mmaps[i].size,
>                                             region->mmaps[i].mmap);
> +        region->mmaps[i].mem.no_dma = true;
>           g_free(name);
>           memory_region_add_subregion(region->mem, region->mmaps[i].offset,
>                                       &region->mmaps[i].mem);
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index ea03ef2..850cc8c 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -794,6 +794,7 @@ struct MemoryRegion {
>       bool unmergeable;
>       uint8_t dirty_log_mask;
>       bool is_iommu;
> +    bool no_dma;
>       RAMBlock *ram_block;
>       Object *owner;
>       /* owner as TYPE_DEVICE. Used for re-entrancy checks in MR access hotpath */
Steven Sistare Aug. 13, 2024, 2:53 p.m. UTC | #3 
On 8/12/2024 9:39 PM, Yi Liu wrote:
> On 2024/7/21 03:15, Steve Sistare wrote:
>> Do not map VFIO PCI BARs for DMA.  This stops a raft of warnings of the
>> following form at QEMU start time when using -object iommufd:
>>
>> qemu-kvm: warning: IOMMU_IOAS_MAP failed: Bad address, PCI BAR?
>> qemu-kvm: vfio_container_dma_map(0x555558282db0, 0x8800010000, 0x4000, 0x7ffff7ff0000) = -14 (Bad address)
> 
> It is required as Alex pointed, so no need to pay further attempt to hide
> this message. And there were efforts to make it. But not done yet. Below
> links may be helpful if you are interested about the history.
> 
> [1] https://lore.kernel.org/kvm/14-v4-0de2f6c78ed0+9d1-iommufd_jgg@nvidia.com/
> [2] https://lore.kernel.org/kvm/20240624141139.GH29266@unreal/
> [3] https://lore.kernel.org/kvm/0-v1-9e6e1739ed95+5fa-vfio_dma_buf_jgg@nvidia.com/

Thanks for the pointers.  Good to here the problem is being worked.

- Steve

>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>   hw/vfio/common.c      | 3 ++-
>>   hw/vfio/helpers.c     | 1 +
>>   include/exec/memory.h | 1 +
>>   3 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index da2e0ec..403d45a 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -248,7 +248,8 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>>               * are never accessed by the CPU and beyond the address width of
>>               * some IOMMU hardware.  TODO: VFIO should tell us the IOMMU width.
>>               */
>> -           section->offset_within_address_space & (1ULL << 63);
>> +           section->offset_within_address_space & (1ULL << 63) ||
>> +           section->mr->no_dma;
>>   }
>>   /* Called with rcu_read_lock held.  */
>> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
>> index b14edd4..e4cfdd2 100644
>> --- a/hw/vfio/helpers.c
>> +++ b/hw/vfio/helpers.c
>> @@ -435,6 +435,7 @@ int vfio_region_mmap(VFIORegion *region)
>>                                             memory_region_owner(region->mem),
>>                                             name, region->mmaps[i].size,
>>                                             region->mmaps[i].mmap);
>> +        region->mmaps[i].mem.no_dma = true;
>>           g_free(name);
>>           memory_region_add_subregion(region->mem, region->mmaps[i].offset,
>>                                       &region->mmaps[i].mem);
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index ea03ef2..850cc8c 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -794,6 +794,7 @@ struct MemoryRegion {
>>       bool unmergeable;
>>       uint8_t dirty_log_mask;
>>       bool is_iommu;
>> +    bool no_dma;
>>       RAMBlock *ram_block;
>>       Object *owner;
>>       /* owner as TYPE_DEVICE. Used for re-entrancy checks in MR access hotpath */
>
diff mbox series

Patch

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index da2e0ec..403d45a 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -248,7 +248,8 @@  static bool vfio_listener_skipped_section(MemoryRegionSection *section)
             * are never accessed by the CPU and beyond the address width of
             * some IOMMU hardware.  TODO: VFIO should tell us the IOMMU width.
             */
-           section->offset_within_address_space & (1ULL << 63);
+           section->offset_within_address_space & (1ULL << 63) ||
+           section->mr->no_dma;
 }
 
 /* Called with rcu_read_lock held.  */
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
index b14edd4..e4cfdd2 100644
--- a/hw/vfio/helpers.c
+++ b/hw/vfio/helpers.c
@@ -435,6 +435,7 @@  int vfio_region_mmap(VFIORegion *region)
                                           memory_region_owner(region->mem),
                                           name, region->mmaps[i].size,
                                           region->mmaps[i].mmap);
+        region->mmaps[i].mem.no_dma = true;
         g_free(name);
         memory_region_add_subregion(region->mem, region->mmaps[i].offset,
                                     &region->mmaps[i].mem);
diff --git a/include/exec/memory.h b/include/exec/memory.h
index ea03ef2..850cc8c 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -794,6 +794,7 @@  struct MemoryRegion {
     bool unmergeable;
     uint8_t dirty_log_mask;
     bool is_iommu;
+    bool no_dma;
     RAMBlock *ram_block;
     Object *owner;
     /* owner as TYPE_DEVICE. Used for re-entrancy checks in MR access hotpath */