Message-ID: <1BE3A8DD-A276-4FC5-A2E3-9B2BB5115704@nutanix.com>
Date: Tue, 23 Dec 2025 04:15:48 +0000
From: Jon Kohler <jon@...anix.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "tglx@...utronix.de"
	<tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de"
	<bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Sergey Dyasli <sergey.dyasli@...anix.com>
Subject: Re: [RFC PATCH 12/18] KVM: x86/mmu: Introduce shadow_ux_mask



> On May 12, 2025, at 3:13 PM, Sean Christopherson <seanjc@...gle.com> wrote:
> 
> On Thu, Mar 13, 2025, Jon Kohler wrote:
>> @@ -28,6 +28,7 @@ u64 __read_mostly shadow_host_writable_mask;
>> u64 __read_mostly shadow_mmu_writable_mask;
>> u64 __read_mostly shadow_nx_mask;
>> u64 __read_mostly shadow_x_mask; /* mutual exclusive with nx_mask */
>> +u64 __read_mostly shadow_ux_mask;
>> u64 __read_mostly shadow_user_mask;
>> u64 __read_mostly shadow_accessed_mask;
>> u64 __read_mostly shadow_dirty_mask;
>> @@ -313,8 +314,14 @@ u64 make_huge_page_split_spte(struct kvm *kvm, u64 huge_spte,
>> * the page executable as the NX hugepage mitigation no longer
>> * applies.
>> */
>> - if ((role.access & ACC_EXEC_MASK) && is_nx_huge_page_enabled(kvm))
>> + if ((role.access & ACC_EXEC_MASK) && is_nx_huge_page_enabled(kvm)) {
> 
> This is wrong, and probably so is every other chunk of KVM that looks at
> ACC_EXEC_MASK.  E.g. if a guest hugepage is executable for user but not supervisor,
> KVM will fail to make the small child user-executable.
> 
> The bug in make_spte() is even worse, because KVM would let an MBEC-aware guest
> trigger the iTLB multi-hit #MC.

Ack/done - I’ve tuned this up in the v1 series, thank you for the call out. 

>> child_spte = make_spte_executable(child_spte);
>> + // TODO: For LKML: switch to vcpu->arch.pt_guest_exec_control? up
>> + // for suggestions on how best to toggle this.
> 
> No, it belongs in the role.

Sold! Fixed in v1

>> + if (enable_pt_guest_exec_control &&
>> +    role.access & ACC_USER_EXEC_MASK)
>> + child_spte |= shadow_ux_mask;
>> + }
>> }
>> 
>> return child_spte;
>> @@ -326,7 +333,7 @@ u64 make_nonleaf_spte(u64 *child_pt, bool ad_disabled)
>> u64 spte = SPTE_MMU_PRESENT_MASK;
>> 
>> spte |= __pa(child_pt) | shadow_present_mask | PT_WRITABLE_MASK |
>> - shadow_user_mask | shadow_x_mask | shadow_me_value;
>> + shadow_user_mask | shadow_x_mask | shadow_ux_mask | shadow_me_value;
>> 
>> if (ad_disabled)
>> spte |= SPTE_TDP_AD_DISABLED;
>> @@ -420,7 +427,8 @@ void kvm_mmu_set_me_spte_mask(u64 me_value, u64 me_mask)
>> }
>> EXPORT_SYMBOL_GPL(kvm_mmu_set_me_spte_mask);
>> 
>> -void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only)
>> +void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only,
>> +   bool has_guest_exec_ctrl)
>> {
>> shadow_user_mask = VMX_EPT_READABLE_MASK;
>> shadow_accessed_mask = has_ad_bits ? VMX_EPT_ACCESS_BIT : 0ull;
>> @@ -428,8 +436,14 @@ void kvm_mmu_set_ept_masks(bool has_ad_bits, bool has_exec_only)
>> shadow_nx_mask = 0ull;
>> shadow_x_mask = VMX_EPT_EXECUTABLE_MASK;
>> /* VMX_EPT_SUPPRESS_VE_BIT is needed for W or X violation. */
>> + // For LKML Review:
>> + // Do we need to modify shadow_present_mask in the MBEC case?
> 
> No, because MBEC bifurcates X, it doesn't change whether or not an EPTE can be
> X without being R.  From the SDM:
> 
>  1. If the “mode-based execute control for EPT” VM-execution control is 1,
>     setting bit 0 indicates also that software may also configure EPT
>     paging-structure entries in which bits 1:0 are both clear and in which bit 10
>     is set (indicating a translation that can be used to fetch instructions from a
>     supervisor-mode linear address or a user-mode linear address).

Gotcha, integrated in v1

>> shadow_present_mask =
>> (has_exec_only ? 0ull : VMX_EPT_READABLE_MASK) | VMX_EPT_SUPPRESS_VE_BIT;
>> +
>> + shadow_ux_mask =
>> + has_guest_exec_ctrl ? VMX_EPT_USER_EXECUTABLE_MASK : 0ull;
> 
> This is EPT specific code, just call this what it is:
> 
> shadow_ux_mask = has_mbec ? VMX_EPT_USER_EXECUTABLE_MASK : 0ull;

Thanks for the code suggestion, I’ve used this in v1.

>> +
>> /*
>> * EPT overrides the host MTRRs, and so KVM must program the desired
>> * memtype directly into the SPTEs.  Note, this mask is just the mask
>> @@ -484,6 +498,7 @@ void kvm_mmu_reset_all_pte_masks(void)
>> shadow_dirty_mask = PT_DIRTY_MASK;
>> shadow_nx_mask = PT64_NX_MASK;
>> shadow_x_mask = 0;
>> + shadow_ux_mask = 0;
>> shadow_present_mask = PT_PRESENT_MASK;
>> 
>> /*
>> diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
>> index d9e22133b6d0..dc2f0dc9c46e 100644
>> --- a/arch/x86/kvm/mmu/spte.h
>> +++ b/arch/x86/kvm/mmu/spte.h
>> @@ -171,6 +171,7 @@ extern u64 __read_mostly shadow_mmu_writable_mask;
>> extern u64 __read_mostly shadow_nx_mask;
>> extern u64 __read_mostly shadow_x_mask; /* mutual exclusive with nx_mask */
>> extern u64 __read_mostly shadow_user_mask;
>> +extern u64 __read_mostly shadow_ux_mask;
>> extern u64 __read_mostly shadow_accessed_mask;
>> extern u64 __read_mostly shadow_dirty_mask;
>> extern u64 __read_mostly shadow_mmio_value;
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index 0aadfa924045..d16e3f170258 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -8544,7 +8544,8 @@ __init int vmx_hardware_setup(void)
>> 
>> if (enable_ept)
>> kvm_mmu_set_ept_masks(enable_ept_ad_bits,
>> -      cpu_has_vmx_ept_execute_only());
>> +      cpu_has_vmx_ept_execute_only(),
>> +      enable_pt_guest_exec_control);
> 
> Without the module param, just cpu_has_vmx_mbec().

Sold! Though, I’ve changed this to cpu_has_vmx_mode_based_ept_exec() in v1.

Powered by blists - more mailing lists