2022-06-27 21:58:33

by Isaku Yamahata

[permalink] [raw]
Subject: [PATCH v7 033/102] KVM: x86/mmu: Add address conversion functions for TDX shared bits

From: Rick Edgecombe <[email protected]>

TDX repurposes one GPA bits (51 bit or 47 bit based on configuration) to
indicate the GPA is private(if cleared) or shared (if set) with VMM. If
GPA.shared is set, GPA is converted existing conventional EPT pointed by
EPTP. If GPA.shared bit is cleared, GPA is converted by Secure-EPT(S-EPT)
TDX module manages. VMM has to issue SEAM call to TDX module to operate on
S-EPT. e.g. populating/zapping guest page or shadow page by TDH.PAGE.{ADD,
REMOVE} for guest page, TDH.PAGE.SEPT.{ADD, REMOVE} S-EPT etc.

Several hooks needs to be added to KVM MMU to support TDX. Add a function
to check if KVM MMU is running for TDX and several functions for address
conversation between private-GPA and shared-GPA.

Signed-off-by: Isaku Yamahata <[email protected]>
---
arch/x86/include/asm/kvm_host.h | 2 ++
arch/x86/kvm/mmu.h | 32 ++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index e5d4e5b60fdc..2c47aab72a1b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1339,7 +1339,9 @@ struct kvm_arch {
*/
u32 max_vcpu_ids;

+#ifdef CONFIG_KVM_MMU_PRIVATE
gfn_t gfn_shared_mask;
+#endif
};

struct kvm_vm_stat {
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index f8192864b496..ccf0ba7a6387 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -286,4 +286,36 @@ static inline gpa_t kvm_translate_gpa(struct kvm_vcpu *vcpu,
return gpa;
return translate_nested_gpa(vcpu, gpa, access, exception);
}
+
+static inline gfn_t kvm_gfn_shared_mask(const struct kvm *kvm)
+{
+#ifdef CONFIG_KVM_MMU_PRIVATE
+ return kvm->arch.gfn_shared_mask;
+#else
+ return 0;
+#endif
+}
+
+static inline gfn_t kvm_gfn_shared(const struct kvm *kvm, gfn_t gfn)
+{
+ return gfn | kvm_gfn_shared_mask(kvm);
+}
+
+static inline gfn_t kvm_gfn_private(const struct kvm *kvm, gfn_t gfn)
+{
+ return gfn & ~kvm_gfn_shared_mask(kvm);
+}
+
+static inline gpa_t kvm_gpa_private(const struct kvm *kvm, gpa_t gpa)
+{
+ return gpa & ~gfn_to_gpa(kvm_gfn_shared_mask(kvm));
+}
+
+static inline bool kvm_is_private_gpa(const struct kvm *kvm, gpa_t gpa)
+{
+ gfn_t mask = kvm_gfn_shared_mask(kvm);
+
+ return mask && !(gpa_to_gfn(gpa) & mask);
+}
+
#endif
--
2.25.1


2022-07-08 02:19:21

by Kai Huang

[permalink] [raw]
Subject: Re: [PATCH v7 033/102] KVM: x86/mmu: Add address conversion functions for TDX shared bits

On Mon, 2022-06-27 at 14:53 -0700, [email protected] wrote:
> From: Rick Edgecombe <[email protected]>

I don't think this is appropriate any more. You can add Co-developed-by I
guess.

>
> TDX repurposes one GPA bits (51 bit or 47 bit based on configuration) to
> indicate the GPA is private(if cleared) or shared (if set) with VMM. If
> GPA.shared is set, GPA is converted existing conventional EPT pointed by
> EPTP. If GPA.shared bit is cleared, GPA is converted by Secure-EPT(S-EPT)

Not sure whether Secure EPT has even been mentioned before in this series. If
not, perhaps better to explain it here. Or not sure whether you need to mention
S-EPT at all.

> TDX module manages. VMM has to issue SEAM call to TDX module to operate on

SEAM call -> SEAMCALL

> S-EPT. e.g. populating/zapping guest page or shadow page by TDH.PAGE.{ADD,
> REMOVE} for guest page, TDH.PAGE.SEPT.{ADD, REMOVE} S-EPT etc.

Not sure why you want to mention those particular SEAMCALLs.

>
> Several hooks needs to be added to KVM MMU to support TDX. Add a function

needs -> need.

Not sure why you need first sentence at all.

But I do think you should mention adding per-VM scope 'gfn_shared_mask' thing.

> to check if KVM MMU is running for TDX and several functions for address
> conversation between private-GPA and shared-GPA.
>
> Signed-off-by: Isaku Yamahata <[email protected]>
> ---
> arch/x86/include/asm/kvm_host.h | 2 ++
> arch/x86/kvm/mmu.h | 32 ++++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index e5d4e5b60fdc..2c47aab72a1b 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1339,7 +1339,9 @@ struct kvm_arch {
> */
> u32 max_vcpu_ids;
>
> +#ifdef CONFIG_KVM_MMU_PRIVATE
> gfn_t gfn_shared_mask;
> +#endif

As Xiaoyao said, please introduce gfn_shared_mask in this patch.

And by applying this patch, nothing will prevent you to turn on INTEL_TDX_HOST
and KVM_INTEL, which also turns on KVM_MMU_PRIVATE.

So 'kvm_arch::gfn_shared_mask' is guaranteed to be 0? If not, can legal
(shared) GFN for normal VM be potentially treated as private?

If yes, perhaps explicitly call out in changelog so people don't need to worry
about?

> };
>
> struct kvm_vm_stat {
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index f8192864b496..ccf0ba7a6387 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -286,4 +286,36 @@ static inline gpa_t kvm_translate_gpa(struct kvm_vcpu *vcpu,
> return gpa;
> return translate_nested_gpa(vcpu, gpa, access, exception);
> }
> +
> +static inline gfn_t kvm_gfn_shared_mask(const struct kvm *kvm)
> +{
> +#ifdef CONFIG_KVM_MMU_PRIVATE
> + return kvm->arch.gfn_shared_mask;
> +#else
> + return 0;
> +#endif
> +}
> +
> +static inline gfn_t kvm_gfn_shared(const struct kvm *kvm, gfn_t gfn)
> +{
> + return gfn | kvm_gfn_shared_mask(kvm);
> +}
> +
> +static inline gfn_t kvm_gfn_private(const struct kvm *kvm, gfn_t gfn)
> +{
> + return gfn & ~kvm_gfn_shared_mask(kvm);
> +}
> +
> +static inline gpa_t kvm_gpa_private(const struct kvm *kvm, gpa_t gpa)
> +{
> + return gpa & ~gfn_to_gpa(kvm_gfn_shared_mask(kvm));
> +}
> +
> +static inline bool kvm_is_private_gpa(const struct kvm *kvm, gpa_t gpa)
> +{
> + gfn_t mask = kvm_gfn_shared_mask(kvm);
> +
> + return mask && !(gpa_to_gfn(gpa) & mask);
> +}
> +
> #endif

2022-07-13 04:59:46

by Isaku Yamahata

[permalink] [raw]
Subject: Re: [PATCH v7 033/102] KVM: x86/mmu: Add address conversion functions for TDX shared bits

On Fri, Jul 08, 2022 at 02:15:20PM +1200,
Kai Huang <[email protected]> wrote:

> On Mon, 2022-06-27 at 14:53 -0700, [email protected] wrote:
> > From: Rick Edgecombe <[email protected]>
>
> I don't think this is appropriate any more. You can add Co-developed-by I
> guess.

Makes sense.


> >
> > TDX repurposes one GPA bits (51 bit or 47 bit based on configuration) to
> > indicate the GPA is private(if cleared) or shared (if set) with VMM. If
> > GPA.shared is set, GPA is converted existing conventional EPT pointed by
> > EPTP. If GPA.shared bit is cleared, GPA is converted by Secure-EPT(S-EPT)
>
> Not sure whether Secure EPT has even been mentioned before in this series. If
> not, perhaps better to explain it here. Or not sure whether you need to mention
> S-EPT at all.
>
> > TDX module manages. VMM has to issue SEAM call to TDX module to operate on
>
> SEAM call -> SEAMCALL
>
> > S-EPT. e.g. populating/zapping guest page or shadow page by TDH.PAGE.{ADD,
> > REMOVE} for guest page, TDH.PAGE.SEPT.{ADD, REMOVE} S-EPT etc.
>
> Not sure why you want to mention those particular SEAMCALLs.
>
> >
> > Several hooks needs to be added to KVM MMU to support TDX. Add a function
>
> needs -> need.
>
> Not sure why you need first sentence at all.
>
> But I do think you should mention adding per-VM scope 'gfn_shared_mask' thing.
>
> > to check if KVM MMU is running for TDX and several functions for address
> > conversation between private-GPA and shared-GPA.
> >
> > Signed-off-by: Isaku Yamahata <[email protected]>
> > ---
> > arch/x86/include/asm/kvm_host.h | 2 ++
> > arch/x86/kvm/mmu.h | 32 ++++++++++++++++++++++++++++++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index e5d4e5b60fdc..2c47aab72a1b 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1339,7 +1339,9 @@ struct kvm_arch {
> > */
> > u32 max_vcpu_ids;
> >
> > +#ifdef CONFIG_KVM_MMU_PRIVATE
> > gfn_t gfn_shared_mask;
> > +#endif
>
> As Xiaoyao said, please introduce gfn_shared_mask in this patch.
>
> And by applying this patch, nothing will prevent you to turn on INTEL_TDX_HOST
> and KVM_INTEL, which also turns on KVM_MMU_PRIVATE.
>
> So 'kvm_arch::gfn_shared_mask' is guaranteed to be 0? If not, can legal
> (shared) GFN for normal VM be potentially treated as private?
>
> If yes, perhaps explicitly call out in changelog so people don't need to worry
> about?

struct kvm that includes struct kvm_arch is guaranteed to be zero.

Here is the updated commit message.

Author: Isaku Yamahata <[email protected]>
Date: Tue Jul 12 00:10:13 2022 -0700

KVM: x86/mmu: Add address conversion functions for TDX shared bit of GPA

TDX repurposes one GPA bit (51 bit or 47 bit based on configuration) to
indicate the GPA is private(if cleared) or shared (if set) with VMM. If
GPA.shared is set, GPA is converted existing conventional EPT pointed by
EPTP. If GPA.shared bit is cleared, GPA is converted by TDX module.
VMM has to issue SEAMCALLs to operate.

Add a member to remember GPA shared bit for each guest TDs, add address
conversion functions between private GPA and shared GPA and test if GPA
is private.

Because struct kvm_arch (or struct kvm which includes struct kvm_arch. See
kvm_arch_alloc_vm() that passes __GPF_ZERO) is zero-cleared when allocated,
the new member to remember GPA shared bit is guaranteed to be zero with
this patch unless it's initialized explicitly.

Co-developed-by: Rick Edgecombe <[email protected]>
Signed-off-by: Rick Edgecombe <[email protected]>
Signed-off-by: Isaku Yamahata <[email protected]>


--
Isaku Yamahata <[email protected]>

2022-07-13 11:04:14

by Kai Huang

[permalink] [raw]
Subject: Re: [PATCH v7 033/102] KVM: x86/mmu: Add address conversion functions for TDX shared bits


> >
> > And by applying this patch, nothing will prevent you to turn on INTEL_TDX_HOST
> > and KVM_INTEL, which also turns on KVM_MMU_PRIVATE.
> >
> > So 'kvm_arch::gfn_shared_mask' is guaranteed to be 0? If not, can legal
> > (shared) GFN for normal VM be potentially treated as private?
> >
> > If yes, perhaps explicitly call out in changelog so people don't need to worry
> > about?
>
> struct kvm that includes struct kvm_arch is guaranteed to be zero.
>
> Here is the updated commit message.
>
> Author: Isaku Yamahata <[email protected]>
> Date: Tue Jul 12 00:10:13 2022 -0700
>
> KVM: x86/mmu: Add address conversion functions for TDX shared bit of GPA
>
> TDX repurposes one GPA bit (51 bit or 47 bit based on configuration) to
> indicate the GPA is private(if cleared) or shared (if set) with VMM. If
> GPA.shared is set, GPA is converted existing conventional EPT pointed by
> EPTP. If GPA.shared bit is cleared, GPA is converted by TDX module.
> VMM has to issue SEAMCALLs to operate.

Sorry what does "GPA is converted ..." mean?


--
Thanks,
-Kai


2022-07-14 00:36:17

by Isaku Yamahata

[permalink] [raw]
Subject: Re: [PATCH v7 033/102] KVM: x86/mmu: Add address conversion functions for TDX shared bits

On Wed, Jul 13, 2022 at 10:41:33PM +1200,
Kai Huang <[email protected]> wrote:

>
> > >
> > > And by applying this patch, nothing will prevent you to turn on INTEL_TDX_HOST
> > > and KVM_INTEL, which also turns on KVM_MMU_PRIVATE.
> > >
> > > So 'kvm_arch::gfn_shared_mask' is guaranteed to be 0? If not, can legal
> > > (shared) GFN for normal VM be potentially treated as private?
> > >
> > > If yes, perhaps explicitly call out in changelog so people don't need to worry
> > > about?
> >
> > struct kvm that includes struct kvm_arch is guaranteed to be zero.
> >
> > Here is the updated commit message.
> >
> > Author: Isaku Yamahata <[email protected]>
> > Date: Tue Jul 12 00:10:13 2022 -0700
> >
> > KVM: x86/mmu: Add address conversion functions for TDX shared bit of GPA
> >
> > TDX repurposes one GPA bit (51 bit or 47 bit based on configuration) to
> > indicate the GPA is private(if cleared) or shared (if set) with VMM. If
> > GPA.shared is set, GPA is converted existing conventional EPT pointed by
> > EPTP. If GPA.shared bit is cleared, GPA is converted by TDX module.
> > VMM has to issue SEAMCALLs to operate.
>
> Sorry what does "GPA is converted ..." mean?

Oops. typo. I meant GPA is covered by ...

--
Isaku Yamahata <[email protected]>