On 5/27/2026 3:44 AM, Cédric Le Goater wrote:
> Hello Chenyi,
>
>
> On 5/19/26 07:57, Chenyi Qiang wrote:
>> vfio_container_dma_map() uses dma_map_file() whenever a RAMBlock has an
>> fd and the VFIO IOMMU backend supports file-based DMA mapping. That is
>> not correct for private file-backed RAM.
>>
>> dma_map_file() resolves PFNs from the backing file, but private
>> mappings can run on different PFNs than the file itself. As a result,
>> using dma_map_file() on a private RAMBlock can program DMA against pages
>> that do not back QEMU's actual guest memory.
>>
>> This was observed with hugetlbfs-backed guest memory and iommufd/VFIO:
>> share=on works, while share=off can fault because the file-backed PFNs
>> can diverge from the PFNs backing QEMU's private mapping.
>>
>> Fix this by using dma_map_file() only for shared RAMBlocks.
>>
>> Fixes: fb32965b6dd8 ("vfio/iommufd: use IOMMU_IOAS_MAP_FILE")
>> Reported-by: Farrah Chen <[email protected]>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220776
>> Signed-off-by: Chenyi Qiang <[email protected]>
>> ---
>> hw/vfio/container.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 4c2816b574..c5a3c60a27 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -82,7 +82,7 @@ int vfio_container_dma_map(VFIOContainer *bcontainer,
>> RAMBlock *rb = mr->ram_block;
>> int mfd = rb ? qemu_ram_get_fd(rb) : -1;
>> - if (mfd >= 0 && vioc->dma_map_file) {
>> + if (mfd >= 0 && vioc->dma_map_file && qemu_ram_is_shared(rb)) {
>
> I think we should introduce an helper to check that the conditions
> for using the .dma_map_file() handler are met. Something like :
>
> if (vfio_container_can_dma_map_file(bcontainer, ...)) {
> RAMBlock *rb = mr->ram_block;
>
> and
>
> /*
> * We can use IOMMU DMA mapping (IOMMU_IOAS_MAP_FILE) for :
> *
> * 1) Guest RAM blocks explicitly configured as shared (MAP_SHARED)
> * 2) RAM device sub-regions (MMIO BARs)
> *
> * Private RAM mappings (MAP_PRIVATE) are strictly excluded. Because
> * they are subject to copy-on-write (COW) anomalies, their
> * underlying PFNs can permanently diverge from the backing file
> */
> bool vfio_container_can_dma_map_file(VFIOContainer *bcontainer, ...)
> {
> return ...
> }
>
> How's that ?
Thanks for the review. I'm fine with introducing the helper.
But I'm not sure if it's cleaner to include duplicated calculations in both the
caller and helper.
+static bool vfio_container_can_dma_map_file(VFIOContainer *bcontainer,
RAMBlock *rb)
+{
+ VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+
+ return rb && qemu_ram_get_fd(rb) >= 0 && vioc->dma_map_file &&
+ qemu_ram_is_shared(rb);
+}
+
int vfio_container_dma_map(VFIOContainer *bcontainer, ...)
{
...
RAMBlock *rb = mr->ram_block;
int mfd = rb ? qemu_ram_get_fd(rb) : -1;
- if (mfd >= 0 && vioc->dma_map_file && qemu_ram_is_shared(rb)) {
+ if (vfio_container_can_dma_map_file(bcontainer, rb)) {
unsigned long start = vaddr - qemu_ram_get_host_addr(rb);
unsigned long offset = qemu_ram_get_fd_offset(rb);
Perhaps it can be simplified slightly.
