linux-kernel - Re: [PATCH] KVM: x86: async_pf: check earlier if can deliver async pf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <522cb8d6-63fa-4450-a786-86da64f8ecc3@amazon.com>
Date: Wed, 27 Nov 2024 10:35:17 +0000
From: Nikita Kalyazin <kalyazin@...zon.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: <pbonzini@...hat.com>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<bp@...en8.de>, <dave.hansen@...ux.intel.com>, <hpa@...or.com>,
	<kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <david@...hat.com>,
	<peterx@...hat.com>, <oleg@...hat.com>, <vkuznets@...hat.com>,
	<gshan@...hat.com>, <graf@...zon.de>, <jgowans@...zon.com>,
	<roypat@...zon.co.uk>, <derekmn@...zon.com>, <nsaenz@...zon.es>,
	<xmarcalx@...zon.com>
Subject: Re: [PATCH] KVM: x86: async_pf: check earlier if can deliver async pf



On 26/11/2024 22:10, Sean Christopherson wrote:
> On Tue, Nov 26, 2024, Nikita Kalyazin wrote:
>> On 26/11/2024 00:06, Sean Christopherson wrote:
>>> On Mon, Nov 25, 2024, Nikita Kalyazin wrote:
>>>> In both cases the fault handling code is blocked and the pCPU is free for
>>>> other tasks.  I can't see the vCPU spinning on the IO to get completed if
>>>> the async task isn't created.  I tried that with and without async PF
>>>> enabled by the guest (MSR_KVM_ASYNC_PF_EN).
>>>>
>>>> What am I missing?
>>>
>>> Ah, I was wrong about the vCPU spinning.
>>>
>>> The goal is specifically to schedule() from KVM context, i.e. from kvm_vcpu_block(),
>>> so that if a virtual interrupt arrives for the guest, KVM can wake the vCPU and
>>> deliver the IRQ, e.g. to reduce latency for interrupt delivery, and possible even
>>> to let the guest schedule in a different task if the IRQ is the guest's tick.
>>>
>>> Letting mm/ or fs/ do schedule() means the only wake event even for the vCPU task
>>> is the completion of the I/O (or whatever the fault is waiting on).
>>
>> Ok, great, then that's how I understood it last time.  The only thing that
>> is not entirely clear to me is like Vitaly says, KVM_ASYNC_PF_SEND_ALWAYS is
>> no longer set, because we don't want to inject IRQs into the guest when it's
>> in kernel mode, but the "host async PF" case would still allow IRQs (eg
>> ticks like you said).  Why is it safe to deliver them?
> 
> IRQs are fine, the problem with PV async #PF is that it directly injects a #PF,
> which the kernel may not be prepared to handle.

You're right indeed, I was overfocused on IRQs for some reason.

>>>>>>> I have no objection to disabling host async page faults,
>>>>>>> e.g. it's probably a net>>>>> negative for 1:1 vCPU:pCPU pinned setups, but such disabling
>>>>>>> needs an opt-in from>>>>> userspace.
>> Back to this, I couldn't see a significant effect of this optimisation with
>> the original async PF so happy to give it up, but it does make a difference
>> when applied to async PF user [2] in my setup.  Would a new cap be a good
>> way for users to express their opt-in for it?
> 
> This probably needs to be handled in the context of the async #PF user series.
> If that series never lands, adding a new cap is likely a waste.  And I suspect
> that even then, a capability may not be warranted (truly don't know, haven't
> looked at your other series).

Yes, I meant that to be included in the async #PF user series (if 
required), not this one.  Just wanted to bring it up here, because the 
thread already had the relevant context.  Thanks.