On Tue, 4 Oct 2022 at 11:51, Nikunj A. Dadhania <nikunj@xxxxxxx> wrote: > > On 04/10/22 13:58, Ard Biesheuvel wrote: > > On Tue, 4 Oct 2022 at 10:24, Ard Biesheuvel <ardb@xxxxxxxxxx> wrote: > >> > >> On Tue, 4 Oct 2022 at 06:41, Nikunj A. Dadhania <nikunj@xxxxxxx> wrote: > >>> > >>> Hi! > >>> > >>> We are trying to implement Secure TSC feature for AMD SNP guests [1]. During the boot-up of the > >>> secondary cpus, SecureTSC enabled guests need to query TSC info from Security processor (PSP). > >>> This communication channel is encrypted between the security processor and the guest, > >>> hypervisor is just the conduit to deliver the guest messages to the security processor. > >>> Each message is protected with an AEAD (AES-256 GCM). > >>> > >>> As the TSC info is needed during the smpboot phase, few crypto modules need to be loaded early > >>> to use the crypto api for encryption/decryption of SNP Guest messages. > >>> > >>> I was able to get the SNP Guest messages working with initializing few crypto modules using > >>> early_initcall() instead of subsys_initcall(). > >>> > >>> Require suggestion/inputs if this is acceptable. List of modules that was changed > >>> to early_initcall: > >>> > >>> early_initcall(aes_init); > >>> early_initcall(cryptomgr_init); > >>> early_initcall(crypto_ctr_module_init); > >>> early_initcall(crypto_gcm_module_init); > >>> early_initcall(ghash_mod_init); > >>> > >> > >> I understand the need for this, but I think it is a bad idea. These > >> will run even before SMP bringup, and before pure initcalls, which are > >> documented as > > > > /* > > * A "pure" initcall has no dependencies on anything else, and purely > > * initializes variables that couldn't be statically initialized. > > */> > > So basically, you should not be relying on any global infrastructure > > to have been initialized. This is also something that may cause > > different problems on different architectures, and doing this only for > > x86 seems like a problem as well. > > > > Can you elaborate a bit on the use case? > > Parameters used in TSC value calculation is controlled by > the hypervisor and a malicious hypervisor can prevent guest from > moving forward. Secure TSC allows guest to securely use rdtsc/rdtscp > as the parameters being used now cannot be changed by hypervisor once > the guest is launched. > > For the boot-cpu, TSC_SCALE/OFFSET is initialized as part of the guest > launch process. During the secure guest boot, boot cpu will start bringing > up the secondary CPUs. While creation of secondary CPU, TSC_SCALE/OFFSET > field needs to be initialized appropriately. SNP Guest messages are the > mechanism to communicate with the PSP to prevent risks from a malicious > hypervisor snooping. > > The PSP firmware provides each guests with four Virtual Machine Platform > Communication key(VMPCK) and is passed to the guest using a special secrets page > as part of the guest launch process. The key is either with the guest or the > PSP firmware. > > The messages exchanged between the guest and the PSP firmware is > encrypted/decrypted using this key. > > > AES in GCM mode seems like a > > thing that we might be able to add to the crypto library API without > > much hassle (which already has a minimal implementation of AES) > > That will be great ! > Try this branch and see if it works for you https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=libgcm
