Re: [Qemu-devel] Logging dirty pages from vhost-net in-kernel with vIOMMU

[Jason Wang <jasowang@redhat.com>]

On 2018/12/5 下午9:32, Michael S. Tsirkin wrote:
> On Wed, Dec 05, 2018 at 11:02:11AM +0800, Jason Wang wrote:
>> On 2018/12/5 上午9:59, Michael S. Tsirkin wrote:
>>> On Wed, Dec 05, 2018 at 09:30:19AM +0800, Jason Wang wrote:
>>>> On 2018/12/5 上午2:37, Jintack Lim wrote:
>>>>> Hi,
>>>>>
>>>>> I'm wondering how the current implementation works when logging dirty
>>>>> pages during migration from vhost-net (in kernel) when used vIOMMU.
>>>>>
>>>>> I understand how vhost-net logs GPAs when not using vIOMMU. But when
>>>>> we use vhost with vIOMMU, then shouldn't vhost-net need to log the
>>>>> translated address (GPA) instead of the address written in the
>>>>> descriptor (IOVA) ? The current implementation looks like vhost-net
>>>>> just logs IOVA without translation in vhost_get_vq_desc() in
>>>>> drivers/vhost/net.c. It seems like QEMU doesn't do any further
>>>>> translation of the dirty log when syncing.
>>>>>
>>>>> I might be missing something. Could somebody shed some light on this?
>>>> Good catch. It looks like a bug to me. Want to post a patch for this?
>>> This isn't going to be a quick fix: IOTLB UAPI is translating
>>> IOVA values directly to uaddr.
>>>
>>> So to fix it, we need to change IOVA messages to translate to GPA
>>> so GPA can be logged.
>>>
>>> for existing userspace We can try reverse translation uaddr->gpa as a
>>> hack for logging but that translation was never guaranteed to be unique.
>>
>> We have memory table in vhost as well, so looks like we can do this in
>> kernel as well without disturbing UAPI?
>>
>> Thanks
> Let me try to rephrase.
>
> Yes, as a temporary bugfix we can do the uaddr to gpa translations.
> It is probably good enough for what QEMU does now.
>
> However it can break some legal userspace, since it is possible to
> have multiple UADDR mappings for a single GPA.
> In that setup the vhost table would only have one of these
> and it's possible that IOTLB would use another one.


Consider we are logging GPA, so it doesn't matter which UADDR in this 
case since we finally get a same GPA. Maybe you mean multiple GPA 
mappings for a single UADDR? Then we may want to log all possible GPA in 
this case.


>
> And generally it's a better idea security-wise to make
> iotlb talk in GPA terms. This way whoever sets the static
> GPA-to-UADDR mappings controls security, and the dynamic
> and more fragile iova mappings can not break QEMU security.


AFAIK, this may only work if memory table and IOTLB entries were set by 
different process I believe. Consider it's all set by qemu, and qemu 
will go through GPA-UADDR mapping before setting device IOTLB. It's 
probably not a gain for us now.


>
> So we need a UAPI extension with a feature flag.
>

Yes.

Thanks


>>> Jason I think you'll have to work on it given the complexity.
>>>
>>>> Thanks
>>>>
>>>>
>>>>> Thanks,
>>>>> Jintack
>>>>>
>>>>>