lkml.org 
[lkml]   [2021]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC v2 27/32] x86/tdx: Exclude Shared bit from __PHYSICAL_MASK
    From
    Date
    On 4/26/21 11:01 AM, Kuppuswamy Sathyanarayanan wrote:
    > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
    >
    > tdx_shared_mask() returns the mask that has to be set in a page
    > table entry to make page shared with VMM.

    Here's a rewrite:

    Just like MKTME, TDX reassigns bits of the physical address for
    metadata. MKTME used several bits for an encryption KeyID. TDX uses a
    single bit in guests to communicate whether a physical page should be
    protected by TDX as private memory (bit set to 0) or unprotected and
    shared with the VMM (bit set to 1).

    Add a helper, tdg_shared_mask() (bad name please fix it) to generate the
    mask. The processor enumerates its physical address width to include
    the shared bit, which means it gets included in __PHYSICAL_MASK by default.

    Remove the shared mask from 'physical_mask' since any bits in
    tdg_shared_mask() are not used for physical addresses in page table entries.

    --

    BTW, do you find it confusing that the subject says: '__PHYSICAL_MASK'
    and yet the code only modifies 'physical_mask'?

    > Also, note that we cannot club shared mapping configuration between
    > AMD SME and Intel TDX Guest platforms in common function. SME has
    > to do it very early in __startup_64() as it sets the bit on all
    > memory, except what is used for communication. TDX can postpone as
    > we don't need any shared mapping in very early boot.
    >
    > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
    > Reviewed-by: Andi Kleen <ak@linux.intel.com>
    > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
    > ---
    > arch/x86/Kconfig | 1 +
    > arch/x86/include/asm/tdx.h | 6 ++++++
    > arch/x86/kernel/tdx.c | 9 +++++++++
    > 3 files changed, 16 insertions(+)
    >
    > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
    > index 67f99bf27729..5f92e8205de2 100644
    > --- a/arch/x86/Kconfig
    > +++ b/arch/x86/Kconfig
    > @@ -882,6 +882,7 @@ config INTEL_TDX_GUEST
    > select PARAVIRT_XL
    > select X86_X2APIC
    > select SECURITY_LOCKDOWN_LSM
    > + select X86_MEM_ENCRYPT_COMMON
    > help
    > Provide support for running in a trusted domain on Intel processors
    > equipped with Trusted Domain eXtenstions. TDX is an new Intel
    > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
    > index b972c6531a53..dc80cf7f7d08 100644
    > --- a/arch/x86/include/asm/tdx.h
    > +++ b/arch/x86/include/asm/tdx.h
    > @@ -111,6 +111,8 @@ unsigned char tdg_inb(unsigned short port);
    > unsigned short tdg_inw(unsigned short port);
    > unsigned int tdg_inl(unsigned short port);
    >
    > +extern phys_addr_t tdg_shared_mask(void);
    > +
    > #else // !CONFIG_INTEL_TDX_GUEST
    >
    > static inline bool is_tdx_guest(void)
    > @@ -149,6 +151,10 @@ static inline long tdx_kvm_hypercall4(unsigned int nr, unsigned long p1,
    > return -ENODEV;
    > }
    >
    > +static inline phys_addr_t tdg_shared_mask(void)
    > +{
    > + return 0;
    > +}
    > #endif /* CONFIG_INTEL_TDX_GUEST */
    > #endif /* __ASSEMBLY__ */
    > #endif /* _ASM_X86_TDX_H */
    > diff --git a/arch/x86/kernel/tdx.c b/arch/x86/kernel/tdx.c
    > index 1f1bb98e1d38..7e391cd7aa2b 100644
    > --- a/arch/x86/kernel/tdx.c
    > +++ b/arch/x86/kernel/tdx.c
    > @@ -76,6 +76,12 @@ bool is_tdx_guest(void)
    > }
    > EXPORT_SYMBOL_GPL(is_tdx_guest);
    >
    > +/* The highest bit of a guest physical address is the "sharing" bit */
    > +phys_addr_t tdg_shared_mask(void)
    > +{
    > + return 1ULL << (td_info.gpa_width - 1);
    > +}

    Why not just inline this thing? Functions don't get any smaller than
    that. Or does it not get used anywhere else? Or are you concerned
    about exporting td_info?

    > static void tdg_get_info(void)
    > {
    > u64 ret;
    > @@ -87,6 +93,9 @@ static void tdg_get_info(void)
    >
    > td_info.gpa_width = out.rcx & GENMASK(5, 0);
    > td_info.attributes = out.rdx;
    > +
    > + /* Exclude Shared bit from the __PHYSICAL_MASK */
    > + physical_mask &= ~tdg_shared_mask();
    > }
    >
    > static __cpuidle void tdg_halt(void)
    >

    \
     
     \ /
      Last update: 2021-05-19 18:19    [W:5.360 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site