[Qemu-devel] [PATCH 0/3] Vhost: no more leak QEMU virtual addresses to user backend

Maxime Coquelin posted 3 patches 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171219181129.24189-1-maxime.coquelin@redhat.com
Test checkpatch passed
Test docker passed
Test ppc passed
Test s390x passed
hw/virtio/vhost-backend.c         |  1 +
hw/virtio/vhost-user.c            |  6 ++++--
hw/virtio/vhost.c                 | 16 ++++++++++++----
include/hw/virtio/vhost-backend.h |  6 ++++++
4 files changed, 23 insertions(+), 6 deletions(-)
[Qemu-devel] [PATCH 0/3] Vhost: no more leak QEMU virtual addresses to user backend
Posted by Maxime Coquelin 6 years, 4 months ago
Before this series, QEMU process virtual addresses are sent to the
user backend as user addresses.

Passing these virtual addresses aren't useful, as the backend doesn't
direct access to QEMU address space.

It does make sense however for kernel backend, which can access QEMU
address space.

This series introduce a new enum set by the backend stating whether it
prefers using QEMU Virtual addresses or Guest physical addresses as
User address, and make vhost-user backend to use Guest physical
addresses.

Maxime Coquelin (3):
  vhost-user: rename VhostUserMemory userspace_addr field to user_addr
  vhost: introduce backend's user address type
  vhost-user: no more leak QEMU virtual addresses to user backend

 hw/virtio/vhost-backend.c         |  1 +
 hw/virtio/vhost-user.c            |  6 ++++--
 hw/virtio/vhost.c                 | 16 ++++++++++++----
 include/hw/virtio/vhost-backend.h |  6 ++++++
 4 files changed, 23 insertions(+), 6 deletions(-)

-- 
2.14.3


Re: [Qemu-devel] [PATCH 0/3] Vhost: no more leak QEMU virtual addresses to user backend
Posted by Stefan Hajnoczi 6 years, 4 months ago
On Tue, Dec 19, 2017 at 07:11:26PM +0100, Maxime Coquelin wrote:
> Before this series, QEMU process virtual addresses are sent to the
> user backend as user addresses.
> 
> Passing these virtual addresses aren't useful, as the backend doesn't
> direct access to QEMU address space.
> 
> It does make sense however for kernel backend, which can access QEMU
> address space.
> 
> This series introduce a new enum set by the backend stating whether it
> prefers using QEMU Virtual addresses or Guest physical addresses as
> User address, and make vhost-user backend to use Guest physical
> addresses.
> 
> Maxime Coquelin (3):
>   vhost-user: rename VhostUserMemory userspace_addr field to user_addr
>   vhost: introduce backend's user address type
>   vhost-user: no more leak QEMU virtual addresses to user backend
> 
>  hw/virtio/vhost-backend.c         |  1 +
>  hw/virtio/vhost-user.c            |  6 ++++--
>  hw/virtio/vhost.c                 | 16 ++++++++++++----
>  include/hw/virtio/vhost-backend.h |  6 ++++++
>  4 files changed, 23 insertions(+), 6 deletions(-)
> 
> -- 
> 2.14.3
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [Qemu-devel] [PATCH 0/3] Vhost: no more leak QEMU virtual addresses to user backend
Posted by Michael S. Tsirkin 6 years, 4 months ago
On Wed, Dec 20, 2017 at 04:07:41PM +0000, Stefan Hajnoczi wrote:
> On Tue, Dec 19, 2017 at 07:11:26PM +0100, Maxime Coquelin wrote:
> > Before this series, QEMU process virtual addresses are sent to the
> > user backend as user addresses.
> > 
> > Passing these virtual addresses aren't useful, as the backend doesn't
> > direct access to QEMU address space.
> > 
> > It does make sense however for kernel backend, which can access QEMU
> > address space.
> > 
> > This series introduce a new enum set by the backend stating whether it
> > prefers using QEMU Virtual addresses or Guest physical addresses as
> > User address, and make vhost-user backend to use Guest physical
> > addresses.
> > 
> > Maxime Coquelin (3):
> >   vhost-user: rename VhostUserMemory userspace_addr field to user_addr
> >   vhost: introduce backend's user address type
> >   vhost-user: no more leak QEMU virtual addresses to user backend
> > 
> >  hw/virtio/vhost-backend.c         |  1 +
> >  hw/virtio/vhost-user.c            |  6 ++++--
> >  hw/virtio/vhost.c                 | 16 ++++++++++++----
> >  include/hw/virtio/vhost-backend.h |  6 ++++++
> >  4 files changed, 23 insertions(+), 6 deletions(-)
> > 
> > -- 
> > 2.14.3
> > 
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

This makes make check fail. Any idea why?

Re: [Qemu-devel] [PATCH 0/3] Vhost: no more leak QEMU virtual addresses to user backend
Posted by Maxime Coquelin 6 years, 4 months ago

On 12/21/2017 04:53 AM, Michael S. Tsirkin wrote:
> On Wed, Dec 20, 2017 at 04:07:41PM +0000, Stefan Hajnoczi wrote:
>> On Tue, Dec 19, 2017 at 07:11:26PM +0100, Maxime Coquelin wrote:
>>> Before this series, QEMU process virtual addresses are sent to the
>>> user backend as user addresses.
>>>
>>> Passing these virtual addresses aren't useful, as the backend doesn't
>>> direct access to QEMU address space.
>>>
>>> It does make sense however for kernel backend, which can access QEMU
>>> address space.
>>>
>>> This series introduce a new enum set by the backend stating whether it
>>> prefers using QEMU Virtual addresses or Guest physical addresses as
>>> User address, and make vhost-user backend to use Guest physical
>>> addresses.
>>>
>>> Maxime Coquelin (3):
>>>    vhost-user: rename VhostUserMemory userspace_addr field to user_addr
>>>    vhost: introduce backend's user address type
>>>    vhost-user: no more leak QEMU virtual addresses to user backend
>>>
>>>   hw/virtio/vhost-backend.c         |  1 +
>>>   hw/virtio/vhost-user.c            |  6 ++++--
>>>   hw/virtio/vhost.c                 | 16 ++++++++++++----
>>>   include/hw/virtio/vhost-backend.h |  6 ++++++
>>>   4 files changed, 23 insertions(+), 6 deletions(-)
>>>
>>> -- 
>>> 2.14.3
>>>
>>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> This makes make check fail. Any idea why?
> 

That's interesting, it fails because the rings aren't initialized with
vhost-user-test (even without my series, but we don't notice).

It fails in vhost_virtqueue_start(), because vq->desc is NULL.

Adding some debug prints shows that vq->desc_phys, vq->avail_phys and 
vq->used_phys are all 0 for both queues.

So when using QEMU VAs as user addresses, vq->desc, vq->used and
vq->avail are translated to a non-NULL address, and it doesn't fail.

Only the multiqueue test returns non-zero and different addresses
between the rings.

I'm looking into it.

Regards,
Maxime

Re: [Qemu-devel] [PATCH 0/3] Vhost: no more leak QEMU virtual addresses to user backend
Posted by Maxime Coquelin 6 years, 4 months ago

On 12/21/2017 03:01 PM, Maxime Coquelin wrote:
> 
> 
> On 12/21/2017 04:53 AM, Michael S. Tsirkin wrote:
>> On Wed, Dec 20, 2017 at 04:07:41PM +0000, Stefan Hajnoczi wrote:
>>> On Tue, Dec 19, 2017 at 07:11:26PM +0100, Maxime Coquelin wrote:
>>>> Before this series, QEMU process virtual addresses are sent to the
>>>> user backend as user addresses.
>>>>
>>>> Passing these virtual addresses aren't useful, as the backend doesn't
>>>> direct access to QEMU address space.
>>>>
>>>> It does make sense however for kernel backend, which can access QEMU
>>>> address space.
>>>>
>>>> This series introduce a new enum set by the backend stating whether it
>>>> prefers using QEMU Virtual addresses or Guest physical addresses as
>>>> User address, and make vhost-user backend to use Guest physical
>>>> addresses.
>>>>
>>>> Maxime Coquelin (3):
>>>>    vhost-user: rename VhostUserMemory userspace_addr field to user_addr
>>>>    vhost: introduce backend's user address type
>>>>    vhost-user: no more leak QEMU virtual addresses to user backend
>>>>
>>>>   hw/virtio/vhost-backend.c         |  1 +
>>>>   hw/virtio/vhost-user.c            |  6 ++++--
>>>>   hw/virtio/vhost.c                 | 16 ++++++++++++----
>>>>   include/hw/virtio/vhost-backend.h |  6 ++++++
>>>>   4 files changed, 23 insertions(+), 6 deletions(-)
>>>>
>>>> -- 
>>>> 2.14.3
>>>>
>>>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>>
>> This makes make check fail. Any idea why?
>>
> 
> That's interesting, it fails because the rings aren't initialized with
> vhost-user-test (even without my series, but we don't notice).
> 
> It fails in vhost_virtqueue_start(), because vq->desc is NULL.
> 
> Adding some debug prints shows that vq->desc_phys, vq->avail_phys and 
> vq->used_phys are all 0 for both queues.
> 
> So when using QEMU VAs as user addresses, vq->desc, vq->used and
> vq->avail are translated to a non-NULL address, and it doesn't fail.
> 
> Only the multiqueue test returns non-zero and different addresses
> between the rings.
> 
> I'm looking into it.

I get it now, qvirtqueue_setup() is only called in the multiqueue test.

> Regards,
> Maxime