On Mon, Nov 20, 2023 at 07:24:51PM +0800, Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx> wrote: > > > On 11/7/2023 11:00 PM, isaku.yamahata@xxxxxxxxx wrote: > > From: Xiaoyao Li <xiaoyao.li@xxxxxxxxx> > > > > 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. -- Isaku Yamahata <isaku.yamahata@xxxxxxxxxxxxxxx>