Hi Bharat,
On 10/10/2017 08:42, Bharat Bhushan wrote:
> Hi Alex, Eric,
>
>> -----Original Message-----
>> From: Qemu-devel [mailto:qemu-devel-
>> bounces+bharat.bhushan=nxp....@nongnu.org] On Behalf Of Bharat
>> Bhushan
>> Sent: Friday, October 06, 2017 9:16 AM
>> To: Auger Eric <eric.au...@redhat.com>; Linu Cherian
>> <linuc.dec...@gmail.com>
>> Cc: peter.mayd...@linaro.org; kevin.t...@intel.com; m...@redhat.com;
>> marc.zyng...@arm.com; t...@semihalf.com; will.dea...@arm.com;
>> drjo...@redhat.com; qemu-devel@nongnu.org;
>> alex.william...@redhat.com; qemu-...@nongnu.org;
>> linu.cher...@cavium.com; eric.auger....@gmail.com;
>> robin.mur...@arm.com; christoffer.d...@linaro.org;
>> bharatb.ya...@gmail.com
>> Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH v4 0/5] virtio-iommu: VFIO
>> integration
>>
>>
>>
>>>>> Thanks
>>>>>
>>>>> Eric
>>>>>>
>>>>>> However you should be allowed to map 1 sg element of 5 pages and
>>>>>> then notify the host about this event I think. Still looking at the
>>>>>> code...
>>>>>>
>>>>>> I still can't reproduce the issue at the moment. What kind of
>>>>>> device are you assigning?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Eric
>>>>>>>
>>>>>>> Atleast vfio_get_vaddr called from vfio_iommu_map_notify in Qemu
>>>>>>> expects the map size to be a power of 2.
>>>>
>>>> Actually I missed the most important here ;-)
>>>>>>>
>>>>>>> if (len & iotlb->addr_mask) {
>>>> This check looks suspiscious to me. In our case the len is not
>>>> modified by the previous translation and it fails, I don't see why.
>>>> It should be valid to be able to notify 5 granules.
>>>
>>> So after discussion with Alex, looks the way we notify the host
>>> currently is wrong. we set the addr_mask to the mapping/unmapping size
>>> -1 whereas this should be a page mask instead (granule size or block size?).
>>> So if the guest maps 5 x 4kB pages we should send 5 notifications for
>>> each page and not a single one. It is unclear to me if we can notify
>>> with hugepage/block page size mask. Peter may confirm/infirm this. in
>>> vsmmuv3 code I notify by granule or block size.
>
> My understanding is that host provides supported page sizes (page_size_mask),
> and Size of each notification to host should be exactly best fit of supported
> page-size and/or multiples of supported page-sizes.
> So if guest maps 20K size (single request), and supported page size is 4K, so
> we can still send one 20K size request.
> Not sure I get it, why multiples of supported page-size cannot be provided in
> one notification to host.
I think the IOTLB API was originally devised to manage only granule or
block ^2 sizes. We might change this in the future but for the moment,
with respect to this series, I would simply recommend to stick to the
existing API limitation and concurrently we can work on relaxing this
constraint on another series.
Thanks
Eric
>
> Thanks
> -Bharat
>
>>>
>>> Bharat, please can you add this to your TODO list?
>>>
>>> Linu, thanks a lot for the time you spent debugging this issue.
>>> Curiously on my side, it is really seldom hit but it is ...
>>
>> Thanks Linu and Eric, I added this to my todo list.
>> While I am still not able to reproduce the issue. I tried with e1000 and now
>> try with ixgbe device. May I know which device can be used to reproduce this
>> issue?
>>
>> Thanks
>> -Bharat
>>
>>>
>>> Thanks!
>>>
>>> Eric
>>>>
>>>> Thanks
>>>>
>>>> Eric
>>>>>>> error_report("iommu has granularity incompatible with target
>> AS");
>>>>>>> return false;
>>>>>>> }
>>>>>>>
>>>>>>> Just trying to understand how this is not hitting in your case.
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>
