linux-kernel - Re: [PATCH v2 3/6] KVM: VMX: Handle vectoring error in check_emulate

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <8420f030-c949-4735-8680-e82a57692142@gmail.com>
Date: Wed, 11 Dec 2024 22:05:38 +0000
From: Ivan Orlov <ivan.orlov0322@...il.com>
To: Sean Christopherson <seanjc@...gle.com>, Ivan Orlov <iorlov@...zon.com>
Cc: bp@...en8.de, dave.hansen@...ux.intel.com, mingo@...hat.com,
 pbonzini@...hat.com, shuah@...nel.org, tglx@...utronix.de, hpa@...or.com,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-kselftest@...r.kernel.org, x86@...nel.org, pdurrant@...zon.co.uk,
 dwmw@...zon.co.uk
Subject: Re: [PATCH v2 3/6] KVM: VMX: Handle vectoring error in
 check_emulate_instruction

On 12/11/24 18:15, Sean Christopherson wrote:
> On Mon, Nov 11, 2024, Ivan Orlov wrote:
>> Move unhandleable vmexit due to MMIO during vectoring error detection
>> into check_emulate_instruction. Implement a function which checks if
>> emul_type indicates MMIO so it can be used for both VMX and SVM.
>>
>> Fix the comment about EMULTYPE_PF as this flag doesn't necessarily
>> mean MMIO anymore: it can also be set due to the write protection
>> violation.
>>
>> Signed-off-by: Ivan Orlov <iorlov@...zon.com>
>> ---
>> V1 -> V2:
>> - Detect the unhandleable vectoring error in vmx_check_emulate_instruction
>> instead of handling it in the common MMU code (which is specific for
>> cached MMIO)
>>
>>   arch/x86/include/asm/kvm_host.h | 10 ++++++++--
>>   arch/x86/kvm/vmx/vmx.c          | 25 ++++++++++++-------------
>>   2 files changed, 20 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
>> index eb413079b7c6..3de9702a9135 100644
>> --- a/arch/x86/include/asm/kvm_host.h
>> +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -2017,8 +2017,8 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
>>    *			VMware backdoor emulation handles select instructions
>>    *			and reinjects the #GP for all other cases.
>>    *
>> - * EMULTYPE_PF - Set when emulating MMIO by way of an intercepted #PF, in which
>> - *		 case the CR2/GPA value pass on the stack is valid.
>> + * EMULTYPE_PF - Set when an intercepted #PF triggers the emulation, in which case
>> + *		 the CR2/GPA value pass on the stack is valid.
>>    *
>>    * EMULTYPE_COMPLETE_USER_EXIT - Set when the emulator should update interruptibility
>>    *				 state and inject single-step #DBs after skipping
>> @@ -2053,6 +2053,12 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
>>   #define EMULTYPE_COMPLETE_USER_EXIT (1 << 7)
>>   #define EMULTYPE_WRITE_PF_TO_SP	    (1 << 8)
>>   
>> +static inline bool kvm_is_emul_type_mmio(int emul_type)
> 
> Hmm, this should probably be "pf_mmio", not just "mmio".  E.g. if KVM is emulating
> large swaths of guest code because unrestricted guest is disabled, then can end up
> emulating an MMIO access for "normal" emulation.
> 
> Hmm, actually, what if we go with this?
> 
>    static inline bool kvm_can_emulate_event_vectoring(int emul_type)
>    {
> 	return !(emul_type & EMULTYPE_PF) ||
> 	       (emul_type & EMULTYPE_WRITE_PF_TO_SP);
>    }
> 

I don't mind using either option here, in fact both of them would 
require an update if there is a new emulated page fault type; Let's use 
more generic one (which is kvm_can_emulate_event_vectoring) :)

I'm thinking about a static assert we could add to it, to be sure the 
condition gets updated if such an EMUL_TYPE is introduced, but I can't 
think of something neat... Anyway, it can be done in a separate patch I 
guess (if we really need it).

--
Kind regards,
Ivan Orlov