Re: [PATCH RFC] uaccess: user_access_begin_after_access_ok()

From: Jason Wang
Date: Wed Jun 03 2020 - 02:23:23 EST



On 2020/6/3 äå1:46, Michael S. Tsirkin wrote:
On Wed, Jun 03, 2020 at 01:18:54PM +0800, Jason Wang wrote:
On 2020/6/3 äå12:18, Al Viro wrote:
On Wed, Jun 03, 2020 at 11:57:11AM +0800, Jason Wang wrote:

How widely do you hope to stretch the user_access areas, anyway?
To have best performance for small packets like 64B, if possible, we want to
disable STAC not only for the metadata access done by vhost accessors but
also the data access via iov iterator.
If you want to try and convince Linus to go for that, make sure to Cc
me on that thread. Always liked quality flame...

The same goes for interval tree lookups with uaccess allowed. IOW, I _really_
doubt that it's a good idea.

I see. We are just seeking an approach to perform better in order to compete
with userspace dpdk backends.

I tried another approach of using direct mapping + mmu notifier [1] but the
synchronization with MMU notifier is not easy to perform well.

[1] https://patchwork.kernel.org/patch/11133009/


Incidentally, who had come up with the name __vhost_get_user?
Makes for lovey WTF moment for readers - esp. in vhost_put_user()...
I think the confusion comes since it does not accept userspace pointer (when
IOTLB is enabled).

How about renaming it as vhost_read()/vhost_write() ?
Huh?

__vhost_get_user() is IOTLB remapping of userland pointer. It does not access
userland memory. Neither for read, nor for write. It is used by vhost_get_user()
and vhost_put_user().

Why would you want to rename it into vhost_read _or_ vhost_write, and in any case,
how do you give one function two names? IDGI...

I get you know, I thought you're concerning the names of
vhost_get_user()/vhost_put_user() but actually __vhost_get_user().

Maybe something like __vhost_fetch_uaddr() is better.

Thanks

It's basically vhost_translate_uaddr isn't it?


Yes.



BTW now I re-read it I don't understand __vhost_get_user_slow:


static void __user *__vhost_get_user_slow(struct vhost_virtqueue *vq,
void __user *addr, unsigned int size,
int type)
{
int ret;

ret = translate_desc(vq, (u64)(uintptr_t)addr, size, vq->iotlb_iov,
ARRAY_SIZE(vq->iotlb_iov),
VHOST_ACCESS_RO);

..
}

how does this work? how can we cast a pointer to guest address without
adding any offsets?


I'm not sure I get you here. What kind of offset did you mean?

Thanks