Message ID | 20240215113128.275608-9-nikunj@amd.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add Secure TSC support for SNP guests | expand |
On Thu, Feb 15, 2024 at 05:01:20PM +0530, Nikunj A Dadhania wrote: > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c > index d035bce3a2b0..68aa06852466 100644 > --- a/arch/x86/mm/mem_encrypt.c > +++ b/arch/x86/mm/mem_encrypt.c > @@ -89,6 +89,8 @@ void __init mem_encrypt_init(void) > /* Call into SWIOTLB to update the SWIOTLB DMA buffers */ > swiotlb_update_mem_attributes(); > > + x86_platform.guest.enc_init(); > + > print_mem_encrypt_feature_info(); Why all this hoopla if all you need is to call it once in mem_encrypt.c? IOW, you can simply call snp_secure_tsc_prepare() there, no? Those function pointers are to be used in generic code in order to hide all the platform-specific hackery but mem_encrypt.c is not really generic code, I'd say...
On 4/22/2024 6:50 PM, Borislav Petkov wrote: > On Thu, Feb 15, 2024 at 05:01:20PM +0530, Nikunj A Dadhania wrote: >> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c >> index d035bce3a2b0..68aa06852466 100644 >> --- a/arch/x86/mm/mem_encrypt.c >> +++ b/arch/x86/mm/mem_encrypt.c >> @@ -89,6 +89,8 @@ void __init mem_encrypt_init(void) >> /* Call into SWIOTLB to update the SWIOTLB DMA buffers */ >> swiotlb_update_mem_attributes(); >> >> + x86_platform.guest.enc_init(); >> + >> print_mem_encrypt_feature_info(); > > Why all this hoopla if all you need is to call it once in mem_encrypt.c? This was added thinking in mind that any SNP/TDX init can happen in this hook. > IOW, you can simply call snp_secure_tsc_prepare() there, no? Yes, that is very simple, will drop this change. > Those function pointers are to be used in generic code in order to hide > all the platform-specific hackery but mem_encrypt.c is not really > generic code, I'd say... > Sure. Regards Nikunj
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h index c878616a18b8..8095553e14a7 100644 --- a/arch/x86/include/asm/x86_init.h +++ b/arch/x86/include/asm/x86_init.h @@ -148,12 +148,14 @@ struct x86_init_acpi { * @enc_status_change_finish Notify HV after the encryption status of a range is changed * @enc_tlb_flush_required Returns true if a TLB flush is needed before changing page encryption status * @enc_cache_flush_required Returns true if a cache flush is needed before changing page encryption status + * @enc_init Prepare and initialize encryption features */ struct x86_guest { bool (*enc_status_change_prepare)(unsigned long vaddr, int npages, bool enc); bool (*enc_status_change_finish)(unsigned long vaddr, int npages, bool enc); bool (*enc_tlb_flush_required)(bool enc); bool (*enc_cache_flush_required)(void); + void (*enc_init)(void); }; /** diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c index a37ebd3b4773..a07985a96ca5 100644 --- a/arch/x86/kernel/x86_init.c +++ b/arch/x86/kernel/x86_init.c @@ -136,6 +136,7 @@ static bool enc_status_change_finish_noop(unsigned long vaddr, int npages, bool static bool enc_tlb_flush_required_noop(bool enc) { return false; } static bool enc_cache_flush_required_noop(void) { return false; } static bool is_private_mmio_noop(u64 addr) {return false; } +static void enc_init_noop(void) { } struct x86_platform_ops x86_platform __ro_after_init = { .calibrate_cpu = native_calibrate_cpu_early, @@ -158,6 +159,7 @@ struct x86_platform_ops x86_platform __ro_after_init = { .enc_status_change_finish = enc_status_change_finish_noop, .enc_tlb_flush_required = enc_tlb_flush_required_noop, .enc_cache_flush_required = enc_cache_flush_required_noop, + .enc_init = enc_init_noop, }, }; diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c index d035bce3a2b0..68aa06852466 100644 --- a/arch/x86/mm/mem_encrypt.c +++ b/arch/x86/mm/mem_encrypt.c @@ -89,6 +89,8 @@ void __init mem_encrypt_init(void) /* Call into SWIOTLB to update the SWIOTLB DMA buffers */ swiotlb_update_mem_attributes(); + x86_platform.guest.enc_init(); + print_mem_encrypt_feature_info(); }