Message ID | 71943490df987be8a3a3e131b12750e8c6d82afc.1699368363.git.isaku.yamahata@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM TDX: TDP MMU: large page support | expand |
On 11/7/2023 11:00 PM, isaku.yamahata@intel.com wrote: > From: Xiaoyao Li <xiaoyao.li@intel.com> > > For TDX, EPT violation can happen when TDG.MEM.PAGE.ACCEPT. > And TDG.MEM.PAGE.ACCEPT contains the desired accept page level of TD guest. > > 1. KVM can map it with 4KB page while TD guest wants to accept 2MB page. > > TD geust will get TDX_PAGE_SIZE_MISMATCH and it should try to accept > 4KB size. > > 2. KVM can map it with 2MB page while TD guest wants to accept 4KB page. > > KVM needs to honor it because > a) there is no way to tell guest KVM maps it as 2MB size. And > b) guest accepts it in 4KB size since guest knows some other 4KB page > in the same 2MB range will be used as shared page. > > For case 2, it need to pass desired page level to MMU's > page_fault_handler. Use bit 29:31 of kvm PF error code for this purpose. The shortlog is the same as patch 7/16..., I am a bit confused by the structure of this patch series... Can this patch be squashed into 7/16? > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > --- > arch/x86/include/asm/kvm_host.h | 2 ++ > arch/x86/kvm/vmx/common.h | 2 +- > arch/x86/kvm/vmx/tdx.c | 7 ++++++- > arch/x86/kvm/vmx/tdx.h | 19 ------------------- > arch/x86/kvm/vmx/tdx_arch.h | 19 +++++++++++++++++++ > arch/x86/kvm/vmx/vmx.c | 2 +- > 6 files changed, 29 insertions(+), 22 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index eed36c1eedb7..c16823f3326e 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -285,6 +285,8 @@ enum x86_intercept_stage; > PFERR_WRITE_MASK | \ > PFERR_PRESENT_MASK) > > +#define PFERR_LEVEL(err_code) (((err_code) & PFERR_LEVEL_MASK) >> PFERR_LEVEL_START_BIT) It's defined, but never used? > + > /* apic attention bits */ > #define KVM_APIC_CHECK_VAPIC 0 > /* > diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h > index bb00433932ee..787f59c44abc 100644 > --- a/arch/x86/kvm/vmx/common.h > +++ b/arch/x86/kvm/vmx/common.h > @@ -91,7 +91,7 @@ static inline int __vmx_handle_ept_violation(struct kvm_vcpu *vcpu, gpa_t gpa, > if (kvm_is_private_gpa(vcpu->kvm, gpa)) > error_code |= PFERR_GUEST_ENC_MASK; > > - if (err_page_level > 0) > + if (err_page_level > PG_LEVEL_NONE) > error_code |= (err_page_level << PFERR_LEVEL_START_BIT) & PFERR_LEVEL_MASK; > > return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0); > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c > index 7b81811eb404..c614ab20c191 100644 > --- a/arch/x86/kvm/vmx/tdx.c > +++ b/arch/x86/kvm/vmx/tdx.c > @@ -2713,6 +2713,7 @@ static int tdx_init_mem_region(struct kvm *kvm, struct kvm_tdx_cmd *cmd) > struct kvm_tdx_init_mem_region region; > struct kvm_vcpu *vcpu; > struct page *page; > + u64 error_code; > int idx, ret = 0; > bool added = false; > > @@ -2770,7 +2771,11 @@ static int tdx_init_mem_region(struct kvm *kvm, struct kvm_tdx_cmd *cmd) > kvm_tdx->source_pa = pfn_to_hpa(page_to_pfn(page)) | > (cmd->flags & KVM_TDX_MEASURE_MEMORY_REGION); > > - ret = kvm_mmu_map_tdp_page(vcpu, region.gpa, TDX_SEPT_PFERR, > + /* TODO: large page support. */ > + error_code = TDX_SEPT_PFERR; > + error_code |= (PG_LEVEL_4K << PFERR_LEVEL_START_BIT) & > + PFERR_LEVEL_MASK; > + ret = kvm_mmu_map_tdp_page(vcpu, region.gpa, error_code, > PG_LEVEL_4K); > put_page(page); > if (ret) > diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h > index 37ee944c36a1..54c3f6b83571 100644 > --- a/arch/x86/kvm/vmx/tdx.h > +++ b/arch/x86/kvm/vmx/tdx.h > @@ -72,25 +72,6 @@ union tdx_exit_reason { > u64 full; > }; > > -union tdx_ext_exit_qualification { > - struct { > - u64 type : 4; > - u64 reserved0 : 28; > - u64 req_sept_level : 3; > - u64 err_sept_level : 3; > - u64 err_sept_state : 8; > - u64 err_sept_is_leaf : 1; > - u64 reserved1 : 17; > - }; > - u64 full; > -}; > - > -enum tdx_ext_exit_qualification_type { > - EXT_EXIT_QUAL_NONE, > - EXT_EXIT_QUAL_ACCEPT, > - NUM_EXT_EXIT_QUAL, > -}; > - > struct vcpu_tdx { > struct kvm_vcpu vcpu; > > diff --git a/arch/x86/kvm/vmx/tdx_arch.h b/arch/x86/kvm/vmx/tdx_arch.h > index 9f93250d22b9..ba41fefa47ee 100644 > --- a/arch/x86/kvm/vmx/tdx_arch.h > +++ b/arch/x86/kvm/vmx/tdx_arch.h > @@ -218,4 +218,23 @@ union tdx_sept_level_state { > u64 raw; > }; > > +union tdx_ext_exit_qualification { > + struct { > + u64 type : 4; > + u64 reserved0 : 28; > + u64 req_sept_level : 3; > + u64 err_sept_level : 3; > + u64 err_sept_state : 8; > + u64 err_sept_is_leaf : 1; > + u64 reserved1 : 17; > + }; > + u64 full; > +}; > + > +enum tdx_ext_exit_qualification_type { > + EXT_EXIT_QUAL_NONE = 0, > + EXT_EXIT_QUAL_ACCEPT, Since this value should be fixed to 1, maybe better to initialize it to 1 for future proof? > + NUM_EXT_EXIT_QUAL, > +}; > + > #endif /* __KVM_X86_TDX_ARCH_H */ > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index ae9ba0731521..fb3913df6a5d 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -5753,7 +5753,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu) > if (unlikely(allow_smaller_maxphyaddr && kvm_vcpu_is_illegal_gpa(vcpu, gpa))) > return kvm_emulate_instruction(vcpu, 0); > > - return __vmx_handle_ept_violation(vcpu, gpa, exit_qualification, 0); > + return __vmx_handle_ept_violation(vcpu, gpa, exit_qualification, PG_LEVEL_NONE); > } > > static int handle_ept_misconfig(struct kvm_vcpu *vcpu)
On Mon, Nov 20, 2023 at 07:24:51PM +0800, Binbin Wu <binbin.wu@linux.intel.com> wrote: > > > On 11/7/2023 11:00 PM, isaku.yamahata@intel.com wrote: > > From: Xiaoyao Li <xiaoyao.li@intel.com> > > > > For TDX, EPT violation can happen when TDG.MEM.PAGE.ACCEPT. > > And TDG.MEM.PAGE.ACCEPT contains the desired accept page level of TD guest. > > > > 1. KVM can map it with 4KB page while TD guest wants to accept 2MB page. > > > > TD geust will get TDX_PAGE_SIZE_MISMATCH and it should try to accept > > 4KB size. > > > > 2. KVM can map it with 2MB page while TD guest wants to accept 4KB page. > > > > KVM needs to honor it because > > a) there is no way to tell guest KVM maps it as 2MB size. And > > b) guest accepts it in 4KB size since guest knows some other 4KB page > > in the same 2MB range will be used as shared page. > > > > For case 2, it need to pass desired page level to MMU's > > page_fault_handler. Use bit 29:31 of kvm PF error code for this purpose. > The shortlog is the same as patch 7/16..., I am a bit confused by the > structure of this patch series... > Can this patch be squashed into 7/16? Patch 7 should include the changes to arch/x86/include/asm/kvm_host.h and arch/x86/kvm/mmu/mmu.c. and use PFERR_LEVEL(). Patch 9 should include the changes arch/x86/kvm/vmx/*. I'll fix them. > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index eed36c1eedb7..c16823f3326e 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -285,6 +285,8 @@ enum x86_intercept_stage; > > PFERR_WRITE_MASK | \ > > PFERR_PRESENT_MASK) > > +#define PFERR_LEVEL(err_code) (((err_code) & PFERR_LEVEL_MASK) >> PFERR_LEVEL_START_BIT) > It's defined, but never used? I'll make kvm_tdp_page_fault() use it. > > diff --git a/arch/x86/kvm/vmx/tdx_arch.h b/arch/x86/kvm/vmx/tdx_arch.h > > index 9f93250d22b9..ba41fefa47ee 100644 > > --- a/arch/x86/kvm/vmx/tdx_arch.h > > +++ b/arch/x86/kvm/vmx/tdx_arch.h > > @@ -218,4 +218,23 @@ union tdx_sept_level_state { > > u64 raw; > > }; > > +union tdx_ext_exit_qualification { > > + struct { > > + u64 type : 4; > > + u64 reserved0 : 28; > > + u64 req_sept_level : 3; > > + u64 err_sept_level : 3; > > + u64 err_sept_state : 8; > > + u64 err_sept_is_leaf : 1; > > + u64 reserved1 : 17; > > + }; > > + u64 full; > > +}; > > + > > +enum tdx_ext_exit_qualification_type { > > + EXT_EXIT_QUAL_NONE = 0, > > + EXT_EXIT_QUAL_ACCEPT, > Since this value should be fixed to 1, maybe better to initialize it to 1 > for future proof? ok.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index eed36c1eedb7..c16823f3326e 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -285,6 +285,8 @@ enum x86_intercept_stage; PFERR_WRITE_MASK | \ PFERR_PRESENT_MASK) +#define PFERR_LEVEL(err_code) (((err_code) & PFERR_LEVEL_MASK) >> PFERR_LEVEL_START_BIT) + /* apic attention bits */ #define KVM_APIC_CHECK_VAPIC 0 /* diff --git a/arch/x86/kvm/vmx/common.h b/arch/x86/kvm/vmx/common.h index bb00433932ee..787f59c44abc 100644 --- a/arch/x86/kvm/vmx/common.h +++ b/arch/x86/kvm/vmx/common.h @@ -91,7 +91,7 @@ static inline int __vmx_handle_ept_violation(struct kvm_vcpu *vcpu, gpa_t gpa, if (kvm_is_private_gpa(vcpu->kvm, gpa)) error_code |= PFERR_GUEST_ENC_MASK; - if (err_page_level > 0) + if (err_page_level > PG_LEVEL_NONE) error_code |= (err_page_level << PFERR_LEVEL_START_BIT) & PFERR_LEVEL_MASK; return kvm_mmu_page_fault(vcpu, gpa, error_code, NULL, 0); diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c index 7b81811eb404..c614ab20c191 100644 --- a/arch/x86/kvm/vmx/tdx.c +++ b/arch/x86/kvm/vmx/tdx.c @@ -2713,6 +2713,7 @@ static int tdx_init_mem_region(struct kvm *kvm, struct kvm_tdx_cmd *cmd) struct kvm_tdx_init_mem_region region; struct kvm_vcpu *vcpu; struct page *page; + u64 error_code; int idx, ret = 0; bool added = false; @@ -2770,7 +2771,11 @@ static int tdx_init_mem_region(struct kvm *kvm, struct kvm_tdx_cmd *cmd) kvm_tdx->source_pa = pfn_to_hpa(page_to_pfn(page)) | (cmd->flags & KVM_TDX_MEASURE_MEMORY_REGION); - ret = kvm_mmu_map_tdp_page(vcpu, region.gpa, TDX_SEPT_PFERR, + /* TODO: large page support. */ + error_code = TDX_SEPT_PFERR; + error_code |= (PG_LEVEL_4K << PFERR_LEVEL_START_BIT) & + PFERR_LEVEL_MASK; + ret = kvm_mmu_map_tdp_page(vcpu, region.gpa, error_code, PG_LEVEL_4K); put_page(page); if (ret) diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h index 37ee944c36a1..54c3f6b83571 100644 --- a/arch/x86/kvm/vmx/tdx.h +++ b/arch/x86/kvm/vmx/tdx.h @@ -72,25 +72,6 @@ union tdx_exit_reason { u64 full; }; -union tdx_ext_exit_qualification { - struct { - u64 type : 4; - u64 reserved0 : 28; - u64 req_sept_level : 3; - u64 err_sept_level : 3; - u64 err_sept_state : 8; - u64 err_sept_is_leaf : 1; - u64 reserved1 : 17; - }; - u64 full; -}; - -enum tdx_ext_exit_qualification_type { - EXT_EXIT_QUAL_NONE, - EXT_EXIT_QUAL_ACCEPT, - NUM_EXT_EXIT_QUAL, -}; - struct vcpu_tdx { struct kvm_vcpu vcpu; diff --git a/arch/x86/kvm/vmx/tdx_arch.h b/arch/x86/kvm/vmx/tdx_arch.h index 9f93250d22b9..ba41fefa47ee 100644 --- a/arch/x86/kvm/vmx/tdx_arch.h +++ b/arch/x86/kvm/vmx/tdx_arch.h @@ -218,4 +218,23 @@ union tdx_sept_level_state { u64 raw; }; +union tdx_ext_exit_qualification { + struct { + u64 type : 4; + u64 reserved0 : 28; + u64 req_sept_level : 3; + u64 err_sept_level : 3; + u64 err_sept_state : 8; + u64 err_sept_is_leaf : 1; + u64 reserved1 : 17; + }; + u64 full; +}; + +enum tdx_ext_exit_qualification_type { + EXT_EXIT_QUAL_NONE = 0, + EXT_EXIT_QUAL_ACCEPT, + NUM_EXT_EXIT_QUAL, +}; + #endif /* __KVM_X86_TDX_ARCH_H */ diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index ae9ba0731521..fb3913df6a5d 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5753,7 +5753,7 @@ static int handle_ept_violation(struct kvm_vcpu *vcpu) if (unlikely(allow_smaller_maxphyaddr && kvm_vcpu_is_illegal_gpa(vcpu, gpa))) return kvm_emulate_instruction(vcpu, 0); - return __vmx_handle_ept_violation(vcpu, gpa, exit_qualification, 0); + return __vmx_handle_ept_violation(vcpu, gpa, exit_qualification, PG_LEVEL_NONE); } static int handle_ept_misconfig(struct kvm_vcpu *vcpu)