RE: [PATCH 3/3] hw/vfio/region: Create dmabuf for PCI BAR per region

[Duan, Zhenzhong]

>> >> > @@ -305,6 +345,21 @@ int vfio_region_mmap(VFIORegion *region)
>> >> >                                  region->mmaps[i].size - 1);
>> >> >       }
>> >> >
>> >> > +    ret = vfio_region_create_dma_buf(region);
>> >> > +    if (ret < 0) {
>> >> > +        if (ret == -ENOTTY) {
>> >> > +            warn_report_once("VFIO dmabuf not supported in
>> >kernel");
>> >> > +        } else {
>> >> > +            error_report("%s: failed to create dmabuf: %s",
>> >> > +                         memory_region_name(region->mem),
>> >strerror(errno));
>> >> > +        }
>> >> > +    } else {
>> >> > +        MemoryRegion *mr = &region->mmaps[0].mem;
>> >>
>> >> Do we need to support region->mmaps[1]?
>> >
>> >My understanding is all region->mmaps[] entries for a VFIO region share
>> >the same RAMBlock. And the kernel returns a single dmabuf fd per
>> >region, not per subrange.
>>
>> Not get, can RAMBlock have holes?
>
>Yes, a RAMBlock can effectively have holes, but in this context
>that is not what is happening.
>
>IIUC, for a VFIO PCI BAR region, all region->mmaps[] entries
>correspond to subranges of the same BAR and are backed by the
>same MemoryRegion and therefore the same RAMBlock. The sparse
>mmap layout (nr_mmaps > 1) exists to describe which parts of the
>BAR are mappable, not to represent distinct backing memory objects.
>
>So while sparse regions may look like "holes" at the mmap level, there
>are no holes in the RAMBlock abstraction itself. All region->mmaps[]
>entries share the same RAMBlock, which is why attaching the returned
>dmabuf fd to region->mmaps[0].mem.ram_block is sufficient, I think.
>
>However, possible I may be missing the case you are concerned about here.
>Please let me know.

I see memory_region_init_ram_device_ptr() is called for each 
region->mmaps[x].mem,
and RAMBlock is allocated in each call.
IIUC, we should set fd and fd_offset in each RAMBlock.

Thanks
Zhenzhong