Message-ID: <6AD6B7D8-6368-45ED-B7EB-484F25D13BE6@nutanix.com>
Date: Tue, 2 Dec 2025 16:54:44 +0000
From: Jon Kohler <jon@...anix.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
CC: Jason Wang <jasowang@...hat.com>, Arnd Bergmann <arnd@...db.de>,
        Eugenio Pérez <eperezma@...hat.com>,
        "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>,
        "virtualization@...ts.linux.dev"
	<virtualization@...ts.linux.dev>,
        Netdev <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Torvalds
	<torvalds@...ux-foundation.org>,
        Borislav Petkov <bp@...en8.de>,
        Sean
 Christopherson <seanjc@...gle.com>,
        "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>,
        Russell King <linux@...linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
        Alexandre Belloni
	<alexandre.belloni@...tlin.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Drew Fustini <fustini@...nel.org>
Subject: Re: [PATCH net-next] vhost: use "checked" versions of get_user() and
 put_user()



> On Nov 27, 2025, at 1:32 AM, Michael S. Tsirkin <mst@...hat.com> wrote:
> 
> On Thu, Nov 27, 2025 at 03:11:57AM +0000, Jon Kohler wrote:
>> 
>> 
>>> On Nov 26, 2025, at 8:08 PM, Jason Wang <jasowang@...hat.com> wrote:
>>> 
>>> On Thu, Nov 27, 2025 at 3:48 AM Jon Kohler <jon@...anix.com> wrote:
>>>> 
>>>> 
>>>>> On Nov 26, 2025, at 5:25 AM, Arnd Bergmann <arnd@...db.de> wrote:
>>>>> 
>>>>> On Wed, Nov 26, 2025, at 07:04, Jason Wang wrote:
>>>>>> On Wed, Nov 26, 2025 at 3:45 AM Jon Kohler <jon@...anix.com> wrote:
>>>>>>>> On Nov 19, 2025, at 8:57 PM, Jason Wang <jasowang@...hat.com> wrote:
>>>>>>>> On Tue, Nov 18, 2025 at 1:35 AM Jon Kohler <jon@...anix.com> wrote:
>>>>>>> Same deal goes for __put_user() vs put_user by way of commit
>>>>>>> e3aa6243434f ("ARM: 8795/1: spectre-v1.1: use put_user() for __put_user()”)
>>>>>>> 
>>>>>>> Looking at arch/arm/mm/Kconfig, there are a variety of scenarios
>>>>>>> where CONFIG_CPU_SPECTRE will be enabled automagically. Looking at
>>>>>>> commit 252309adc81f ("ARM: Make CONFIG_CPU_V7 valid for 32bit ARMv8 implementations")
>>>>>>> it says that "ARMv8 is a superset of ARMv7", so I’d guess that just
>>>>>>> about everything ARM would include this by default?
>>>>> 
>>>>> I think the more relevant commit is for 64-bit Arm here, but this does
>>>>> the same thing, see 84624087dd7e ("arm64: uaccess: Don't bother
>>>>> eliding access_ok checks in __{get, put}_user").
>>>> 
>>>> Ah! Right, this is definitely the important bit, as it makes it
>>>> crystal clear that these are exactly the same thing. The current
>>>> code is:
>>>> #define get_user        __get_user
>>>> #define put_user        __put_user
>>>> 
>>>> So, this patch changing from __* to regular versions is a no-op
>>>> on arm side of the house, yea?
>>>> 
>>>>> I would think that if we change the __get_user() to get_user()
>>>>> in this driver, the same should be done for the
>>>>> __copy_{from,to}_user(), which similarly skips the access_ok()
>>>>> check but not the PAN/SMAP handling.
>>>> 
>>>> Perhaps, thats a good call out. I’d file that under one battle
>>>> at a time. Let’s get get/put user dusted first, then go down
>>>> that road?
>>>> 
>>>>> In general, the access_ok()/__get_user()/__copy_from_user()
>>>>> pattern isn't really helpful any more, as Linus already
>>>>> explained. I can't tell from the vhost driver code whether
>>>>> we can just drop the access_ok() here and use the plain
>>>>> get_user()/copy_from_user(), or if it makes sense to move
>>>>> to the newer user_access_begin()/unsafe_get_user()/
>>>>> unsafe_copy_from_user()/user_access_end() and try optimize
>>>>> out a few PAN/SMAP flips in the process.
>>> 
>>> Right, according to my testing in the past, PAN/SMAP is a killer for
>>> small packet performance (PPS).
>> 
>> For sure, every little bit helps along that path
>> 
>>> 
>>>> 
>>>> In general, I think there are a few spots where we might be
>>>> able to optimize (vhost_get_vq_desc perhaps?) as that gets
>>>> called quite a bit and IIRC there are at least two flips
>>>> in there that perhaps we could elide to one? An investigation
>>>> for another day I think.
>>> 
>>> Did you mean trying to read descriptors in a batch, that would be
>>> better and with IN_ORDER it would be even faster as a single (at most
>>> two) copy_from_user() might work (without the need to use
>>> user_access_begin()/user_access_end().
>> 
>> Yep. I haven’t fully thought through it, just a drive-by idea
>> from looking at code for the recent work I’ve been doing, just
>> scratching my head thinking there *must* be something we can do
>> better there.
>> 
>> Basically on the get rx/tx bufs path as well as the
>> vhost_add_used_and_signal_n path, I think we could cluster together
>> some of the get/put users and copy to/from’s. Would take some
>> massaging, but I think there is something there.
>> 
>>>> 
>>>> Anyhow, with this info - Jason - is there anything else you
>>>> can think of that we want to double click on?
>>> 
>>> Nope.
>>> 
>>> Thanks
>> 
>> Ok thanks. Perhaps we can land this in next and let it soak in,
>> though, knock on wood, I don’t think there will be fallout
>> (famous last words!) ?
>> 
> 
> 
> To clairify, I think vhost tree is better to put this
> in next than net-next, both because it's purely core vhost
> and because unlike net-next vhost rebases so it is easy to
> just drop the patch if there are issues.
> I'll put it there.
> 
> -- 
> MST
> 

Ok cool, thank you!

Powered by blists - more mailing lists