From: Isaku Yamahata <isaku.yamahata@intel.com>

TDX requires TDX SEAMCALL to operate Secure EPT instead of direct memory
access and TDX SEAMCALL is heavy operation.  Fast page fault on private GPA
doesn't make sense.  Disallow fast page fault on private GPA.

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index b2924bd9b668..54d4c8f1ba68 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3339,8 +3339,16 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
 	return RET_PF_CONTINUE;
 }
 
-static bool page_fault_can_be_fast(struct kvm_page_fault *fault)
+static bool page_fault_can_be_fast(struct kvm *kvm, struct kvm_page_fault *fault)
 {
+	/*
+	 * TDX private mapping doesn't support fast page fault because the EPT
+	 * entry is read/written with TDX SEAMCALLs instead of direct memory
+	 * access.
+	 */
+	if (kvm_is_private_gpa(kvm, fault->addr))
+		return false;
+
 	/*
 	 * Page faults with reserved bits set, i.e. faults on MMIO SPTEs, only
 	 * reach the common page fault handler if the SPTE has an invalid MMIO
@@ -3450,7 +3458,7 @@ static int fast_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
 	u64 *sptep;
 	uint retry_count = 0;
 
-	if (!page_fault_can_be_fast(fault))
+	if (!page_fault_can_be_fast(vcpu->kvm, fault))
 		return ret;
 
 	walk_shadow_page_lockless_begin(vcpu);
-- 
2.25.1

On Tue, Jan 23, 2024 at 12:55 AM <isaku.yamahata@intel.com> wrote:
>
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> TDX requires TDX SEAMCALL to operate Secure EPT instead of direct memory
> access and TDX SEAMCALL is heavy operation.  Fast page fault on private GPA
> doesn't make sense.  Disallow fast page fault on private GPA.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/mmu/mmu.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index b2924bd9b668..54d4c8f1ba68 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3339,8 +3339,16 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
>         return RET_PF_CONTINUE;
>  }
>
> -static bool page_fault_can_be_fast(struct kvm_page_fault *fault)
> +static bool page_fault_can_be_fast(struct kvm *kvm, struct kvm_page_fault *fault)
>  {
> +       /*
> +        * TDX private mapping doesn't support fast page fault because the EPT
> +        * entry is read/written with TDX SEAMCALLs instead of direct memory
> +        * access.
> +        */
> +       if (kvm_is_private_gpa(kvm, fault->addr))
> +               return false;

I think this does not apply to SNP? If so, it would be better to check
the SPTE against the shared-page mask inside the do...while loop.

Paolo

>         /*
>          * Page faults with reserved bits set, i.e. faults on MMIO SPTEs, only
>          * reach the common page fault handler if the SPTE has an invalid MMIO
> @@ -3450,7 +3458,7 @@ static int fast_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>         u64 *sptep;
>         uint retry_count = 0;
>
> -       if (!page_fault_can_be_fast(fault))
> +       if (!page_fault_can_be_fast(vcpu->kvm, fault))
>                 return ret;
>
>         walk_shadow_page_lockless_begin(vcpu);
> --
> 2.25.1
>

On Mon, Feb 12, 2024 at 06:01:54PM +0100,
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On Tue, Jan 23, 2024 at 12:55 AM <isaku.yamahata@intel.com> wrote:
> >
> > From: Isaku Yamahata <isaku.yamahata@intel.com>
> >
> > TDX requires TDX SEAMCALL to operate Secure EPT instead of direct memory
> > access and TDX SEAMCALL is heavy operation.  Fast page fault on private GPA
> > doesn't make sense.  Disallow fast page fault on private GPA.
> >
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  arch/x86/kvm/mmu/mmu.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index b2924bd9b668..54d4c8f1ba68 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -3339,8 +3339,16 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
> >         return RET_PF_CONTINUE;
> >  }
> >
> > -static bool page_fault_can_be_fast(struct kvm_page_fault *fault)
> > +static bool page_fault_can_be_fast(struct kvm *kvm, struct kvm_page_fault *fault)
> >  {
> > +       /*
> > +        * TDX private mapping doesn't support fast page fault because the EPT
> > +        * entry is read/written with TDX SEAMCALLs instead of direct memory
> > +        * access.
> > +        */
> > +       if (kvm_is_private_gpa(kvm, fault->addr))
> > +               return false;
> 
> I think this does not apply to SNP? If so, it would be better to check
> the SPTE against the shared-page mask inside the do...while loop.

No, this won't apply to SNP. Let me update the patch corresponding in v19.
-- 
Isaku Yamahata <isaku.yamahata@linux.intel.com>

On Mon, Feb 26, 2024 at 09:55:10AM -0800,
Isaku Yamahata <isaku.yamahata@linux.intel.com> wrote:

> On Mon, Feb 12, 2024 at 06:01:54PM +0100,
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> > On Tue, Jan 23, 2024 at 12:55 AM <isaku.yamahata@intel.com> wrote:
> > >
> > > From: Isaku Yamahata <isaku.yamahata@intel.com>
> > >
> > > TDX requires TDX SEAMCALL to operate Secure EPT instead of direct memory
> > > access and TDX SEAMCALL is heavy operation.  Fast page fault on private GPA
> > > doesn't make sense.  Disallow fast page fault on private GPA.
> > >
> > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > > ---
> > >  arch/x86/kvm/mmu/mmu.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > index b2924bd9b668..54d4c8f1ba68 100644
> > > --- a/arch/x86/kvm/mmu/mmu.c
> > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > @@ -3339,8 +3339,16 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
> > >         return RET_PF_CONTINUE;
> > >  }
> > >
> > > -static bool page_fault_can_be_fast(struct kvm_page_fault *fault)
> > > +static bool page_fault_can_be_fast(struct kvm *kvm, struct kvm_page_fault *fault)
> > >  {
> > > +       /*
> > > +        * TDX private mapping doesn't support fast page fault because the EPT
> > > +        * entry is read/written with TDX SEAMCALLs instead of direct memory
> > > +        * access.
> > > +        */
> > > +       if (kvm_is_private_gpa(kvm, fault->addr))
> > > +               return false;
> > 
> > I think this does not apply to SNP? If so, it would be better to check
> > the SPTE against the shared-page mask inside the do...while loop.
> 
> No, this won't apply to SNP. Let me update the patch corresponding in v19.

shared-page mask is against GPA or faulting address. Not SPTE unlike SNP.
So it doesn't make sense to check inside the do..while loop.
-- 
Isaku Yamahata <isaku.yamahata@linux.intel.com>

On 1/23/2024 7:53 AM, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> TDX requires TDX SEAMCALL to operate Secure EPT instead of direct memory
> access and TDX SEAMCALL is heavy operation.  Fast page fault on private GPA
> doesn't make sense.  Disallow fast page fault on private GPA.
>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>

> ---
>   arch/x86/kvm/mmu/mmu.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index b2924bd9b668..54d4c8f1ba68 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3339,8 +3339,16 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
>   	return RET_PF_CONTINUE;
>   }
>   
> -static bool page_fault_can_be_fast(struct kvm_page_fault *fault)
> +static bool page_fault_can_be_fast(struct kvm *kvm, struct kvm_page_fault *fault)
>   {
> +	/*
> +	 * TDX private mapping doesn't support fast page fault because the EPT
> +	 * entry is read/written with TDX SEAMCALLs instead of direct memory
> +	 * access.
> +	 */
> +	if (kvm_is_private_gpa(kvm, fault->addr))
> +		return false;
> +
>   	/*
>   	 * Page faults with reserved bits set, i.e. faults on MMIO SPTEs, only
>   	 * reach the common page fault handler if the SPTE has an invalid MMIO
> @@ -3450,7 +3458,7 @@ static int fast_page_fault(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault)
>   	u64 *sptep;
>   	uint retry_count = 0;
>   
> -	if (!page_fault_can_be_fast(fault))
> +	if (!page_fault_can_be_fast(vcpu->kvm, fault))
>   		return ret;
>   
>   	walk_shadow_page_lockless_begin(vcpu);