lkml.org 
[lkml]   [2022]   [Feb]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCHv3.1 2.1/2] x86/coco: Add API to handle encryption mask
On Sat, Feb 19, 2022 at 03:13:05AM +0300, Kirill A. Shutemov wrote:
> AMD SME/SEV uses a bit in the page table entries to indicate that the
> page is encrypted and not accessible to the VMM.
>
> TDX uses a similar approach, but the polarity of the mask is opposite to
> AMD: if the bit is set the page is accessible to VMM.
>
> Provide vendor-neutral API to deal with the mask:
>
> - cc_mkenc() and cc_mkdec() modify given address to make it
> encrypted/decrypted. It can be applied to phys_addr_t, pgprotval_t
> or page table entry value.
>
> - cc_get_mask() returns encryption or decrypthion mask. It is useful

Unknown word [encrypthion] in commit message.
Suggestions: ['encryption', 'decryption']

Unknown word [decrypthion] in commit message.
Suggestions: ['decryption', 'encryption']

> for set_memory_encrypted() and set_memory_decrypted()
> implementation.
>
> The implementation will be extended to cover TDX.
>
> pgprot_decrypted() is used by drivers (i915, virtio_gpu, vfio).
> cc_mkdec() called by pgprot_decrypted(). Export cc_mkdec().
>
> HyperV doesn't use bits in page table entries, so the mask is 0 for both
> encrypthion and decrypthion.

ditto.

> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index d77cf3a31f07..9af6be143998 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -346,7 +346,7 @@ static void __init ms_hyperv_init_platform(void)
> swiotlb_force = SWIOTLB_FORCE;
> #endif
> if (hv_get_isolation_type() != HV_ISOLATION_TYPE_NONE)
> - cc_init(CC_VENDOR_HYPERV);
> + cc_init(CC_VENDOR_HYPERV, 0);
> }
>
> if (hv_max_functions_eax >= HYPERV_CPUID_NESTED_FEATURES) {
> diff --git a/arch/x86/mm/mem_encrypt_identity.c b/arch/x86/mm/mem_encrypt_identity.c
> index eb7fbd85b77e..fa758247ab57 100644
> --- a/arch/x86/mm/mem_encrypt_identity.c
> +++ b/arch/x86/mm/mem_encrypt_identity.c
> @@ -603,5 +603,5 @@ void __init sme_enable(struct boot_params *bp)
> out:
> physical_mask &= ~sme_me_mask;
> if (sme_me_mask)
> - cc_init(CC_VENDOR_AMD);
> + cc_init(CC_VENDOR_AMD, sme_me_mask);

I'm wondering why this is looking weird... an init function with two
arguments which get assigned. I think it would be better if you do:

cc_set_vendor(CC_VENDOR_XXX);
cc_set_mask(mask);

and those helpers simply assign to __ro_after_init, local variables.

IOW, simple setter functions.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

\
 
 \ /
  Last update: 2022-02-21 22:05    [W:0.380 / U:0.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site