Re: [Qemu-devel] [RFC PATCH v2 3/3] VFIO: Type1 IOMMU mapping support for vGPU

[Jike Song]

On 03/08/2016 08:31 AM, Neo Jia wrote:
> On Mon, Mar 07, 2016 at 02:07:15PM +0800, Jike Song wrote:
>> Hi Neo,
>>
>> On Fri, Mar 4, 2016 at 3:00 PM, Neo Jia <address@hidden> wrote:
>>> On Wed, Mar 02, 2016 at 04:38:34PM +0800, Jike Song wrote:
>>>> On 02/24/2016 12:24 AM, Kirti Wankhede wrote:
>>>>> +   vgpu_dma->size = map->size;
>>>>> +
>>>>> +   vgpu_link_dma(vgpu_iommu, vgpu_dma);
>>>>
>>>> Hi Kirti & Neo,
>>>>
>>>> seems that no one actually setup mappings for IOMMU here?
>>>>
>>>
>>> Hi Jike,
>>>
>>> Yes.
>>>
>>> The actual mapping should be done by the host kernel driver after calling 
>>> the
>>> translation/pinning API vgpu_dma_do_translate.
>>
>> Thanks for the reply. I mis-deleted the mail in my intel account, so
>> reply with private mail account, sorry for that.
>>
>>
>> In vgpu_dma_do_translate():
>>
>> for (i = 0; i < count; i++) {
>>    {snip}
>>    dma_addr_t iova = gfn_buffer[i] << PAGE_SHIFT;
>>    vgpu_dma = vgpu_find_dma(vgpu_iommu, iova, 0 /*  size */);
>>
>>     remote_vaddr = vgpu_dma->vaddr + iova - vgpu_dma->iova;
>>     if (get_user_pages_unlocked(NULL, mm, remote_vaddr, 1, 1, 0, page) == 1) 
>> {
>>         pfn = page_to_pfn(page[0]);
>>     }
>>     gfn_buffer[i] = pfn;
>> }
>>
>> If I understand correctly, the purpose of above code, is given an
>> array of gfns, try to pin & return associated pfns. There is still no
>> IOMMU mappings here.  
> 
> Yes.
> 

Thanks for the conformation.

>> Is it supposed to be the caller who should set
>> up IOMMU by DMA api such as dma_map_page(), after calling
>> vgpu_dma_do_translate()?
>>
> 
> Don't think you need to call dma_map_page here. Once you have the pfn 
> available
> to your GPU kernel driver, you can just go ahead to setup the mapping as you
> normally do such as calling pci_map_sg and its friends.
> 

Technically it's definitely OK to call DMA API from the caller rather than here,
however personally I think it is a bit counter-intuitive: IOMMU page tables
should be constructed within the VFIO IOMMU driver.


> Thanks,
> Neo

--
Thanks,
Jike