> On Feb 27, 2025, at 2:34 PM, Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Thu, Feb 27, 2025, Jon Kohler wrote:
>>> On Feb 27, 2025, at 1:52 AM, Nikolay Borisov <nik.borisov@xxxxxxxx> wrote:
>>>
>>> !-------------------------------------------------------------------|
>>> CAUTION: External Email
>
> Noted. :-D
Silly IT !
>
>>> |-------------------------------------------------------------------!
>>>
>>> On 27.02.25 г. 2:07 ч., Sean Christopherson wrote:
>>>> Define independent macros for the RWX protection bits that are enumerated
>>>> via EXIT_QUALIFICATION for EPT Violations, and tie them to the RWX bits in
>>>> EPT entries via compile-time asserts. Piggybacking the EPTE defines works
>>>> for now, but it creates holes in the EPT_VIOLATION_xxx macros and will
>>>> cause headaches if/when KVM emulates Mode-Based Execution (MBEC), or any
>>>> other features that introduces additional protection information.
>>>> Opportunistically rename EPT_VIOLATION_RWX_MASK to EPT_VIOLATION_PROT_MASK
>>>> so that it doesn't become stale if/when MBEC support is added.
>>>> No functional change intended.
>>>> Cc: Jon Kohler <jon@xxxxxxxxxxx>
>>>> Cc: Nikolay Borisov <nik.borisov@xxxxxxxx>
>>>> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
>>>
>>> Reviewed-by: Nikolay Borisov <nik.borisov@xxxxxxxx>
>>
>> LGTM, but any chance we could hold this until I get the MBEC RFC out?
>
> No? It's definitely landing before the MBEC support, and IOM it works quite nicely
> with the MBEC support (my diff at the bottom). I don't see any reason to delay
> or change this cleanup.
Ok no problem at all, happy to rebase on top of this when it lands.
Thanks for the suggestions on the diff, will give it a poke
>
>> My apologies on the delay, I caught a terrible chest cold after we met about
>> it, followed by a secondary case of strep!
>
> Ow. Don't rush on behalf of upstream, KVM has lived without MBEC for a long time,
> it's not going anywhere.o
>
> ---
> arch/x86/include/asm/vmx.h | 4 +++-
> arch/x86/kvm/mmu/paging_tmpl.h | 9 +++++++--
> arch/x86/kvm/vmx/vmx.c | 7 +++++++
> 3 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index d7ab0ad63be6..61e31e915e46 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -587,9 +587,11 @@ enum vm_entry_failure_code {
> #define EPT_VIOLATION_PROT_READ BIT(3)
> #define EPT_VIOLATION_PROT_WRITE BIT(4)
> #define EPT_VIOLATION_PROT_EXEC BIT(5)
> +#define EPT_VIOLATION_PROT_USER_EXEC BIT(6)
> #define EPT_VIOLATION_PROT_MASK (EPT_VIOLATION_PROT_READ | \
> EPT_VIOLATION_PROT_WRITE | \
> - EPT_VIOLATION_PROT_EXEC)
> + EPT_VIOLATION_PROT_EXEC | \
> + EPT_VIOLATION_PROT_USER_EXEC)
> #define EPT_VIOLATION_GVA_IS_VALID BIT(7)
> #define EPT_VIOLATION_GVA_TRANSLATED BIT(8)
>
> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index 68e323568e95..ede8207bf4d7 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> @@ -181,8 +181,9 @@ static inline unsigned FNAME(gpte_access)(u64 gpte)
> unsigned access;
> #if PTTYPE == PTTYPE_EPT
> access = ((gpte & VMX_EPT_WRITABLE_MASK) ? ACC_WRITE_MASK : 0) |
> - ((gpte & VMX_EPT_EXECUTABLE_MASK) ? ACC_EXEC_MASK : 0) |
> - ((gpte & VMX_EPT_READABLE_MASK) ? ACC_USER_MASK : 0);
> + ((gpte & VMX_EPT_EXECUTABLE_MASK) ? ACC_EXEC_MASK : 0) |
> + ((gpte & VMX_EPT_USER_EXECUTABLE_MASK) ? ACC_USER_EXEC_MASK : 0) |
> + ((gpte & VMX_EPT_READABLE_MASK) ? ACC_USER_MASK : 0);
> #else
> BUILD_BUG_ON(ACC_EXEC_MASK != PT_PRESENT_MASK);
> BUILD_BUG_ON(ACC_EXEC_MASK != 1);
> @@ -511,6 +512,10 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
> * ACC_*_MASK flags!
> */
> walker->fault.exit_qualification |= EPT_VIOLATION_RWX_TO_PROT(pte_access);
> + /* This is also wrong.*/
> + if (vcpu->arch.pt_guest_exec_control &&
> + (pte_access & VMX_EPT_USER_EXECUTABLE_MASK))
> + walker->fault.exit_qualification |= EPT_VIOLATION_PROT_USER_EXEC;
> }
> #endif
> walker->fault.address = addr;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 0db64f4adf2a..4684647ef063 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5806,6 +5806,13 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu)
>
> exit_qualification = vmx_get_exit_qual(vcpu);
>
> + /*
> + * The USER_EXEC flag is undefined if MBEC is disabled.
> + * Note, this is wrong, MBEC should be a property of the MMU.
> + */
> + if (!vcpu->arch.pt_guest_exec_control)
> + exit_qualification &= ~EPT_VIOLATION_PROT_USER_EXEC;
> +
> /*
> * EPT violation happened while executing iret from NMI,
> * "blocked by NMI" bit has to be set before next VM entry.
>
> base-commit: 67983df09fc3f96d0d6107fe1a99d29460bab481
> --
>
