Re: [Qemu-devel] [PATCH v6 2/4] vfio: VFIO driver for mediated PCI device

[Kirti Wankhede]

On 8/13/2016 2:55 AM, Alex Williamson wrote:
> On Fri, 12 Aug 2016 23:27:01 +0530
> Kirti Wankhede <address@hidden> wrote:
> 
>> On 8/12/2016 12:13 AM, Alex Williamson wrote:
>>
>>>
>>> TBH, I don't see how providing a default implementation of
>>> validate_map_request() is useful.  How many mediated devices are going
>>> to want to identity map resources from the parent?  Even if they do, it
>>> seems we can only support a single mediated device per parent device
>>> since each will map the same parent resource offset. Let's not even try
>>> to define a default.  If we get a fault and the vendor driver hasn't
>>> provided a handler, send a SIGBUS.  I expect we should also allow
>>> vendor drivers to fill the mapping at mmap() time rather than expecting
>>> this map on fault scheme.  Maybe the mid-level driver should not even be
>>> interacting with mmap() and should let the vendor driver entirely
>>> determine the handling.
>>>  
>>
>> Should we go ahead with pass through mmap() call to vendor driver and
>> let vendor driver decide what to do in mmap() call, either
>> remap_pfn_range in mmap() or do fault on access and handle the fault in
>> their driver. In that case we don't need to track mappings in mdev core.
>> Let vendor driver do that on their own, right?
> 
> This sounds right to me, I don't think we want to impose either model
> on the vendor driver.  The vendor driver owns the vfio device file
> descriptor and is responsible for managing it should they expose mmap
> support for regions on the file descriptor.  They either need to insert
> mappings at the point where mmap() is called or setup fault handlers to
> insert them on demand.  If we can provide helper functions so that each
> vendor driver doesn't need to re-invent either of those, that would be
> a bonus.  Thanks,
> 

Since mmap() is going to be handled in vendor driver, let vendor driver
do their own tracking logic of mappings based on which way they decide
to go. No need to keep it in mdev coer module and try to handle all the
cases in one function.

Thanks,
Kirti


> Alex
>