lkml.org 
[lkml]   [2023]   [Apr]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2 08/11] x86/sev: Add Secure TSC support for SNP guests
    From
    On 4/4/2023 3:11 AM, Tom Lendacky wrote:
    > On 3/26/23 09:46, Nikunj A Dadhania wrote:
    >> Add support for Secure TSC in SNP enabled guests. Secure TSC
    >> allows guest to securely use RDTSC/RDTSCP instructions as the
    >> parameters being used cannot be changed by hypervisor once the
    >> guest is launched.
    >>
    >> During the boot-up of the secondary cpus, SecureTSC enabled
    >> guests need to query TSC info from Security processor (PSP).
    >
    > s/Security processor (PSP)/AMD Secure Processor/
    >
    >> This communication channel is encrypted between the security
    >
    > here as well.
    >
    >> processor and the guest, hypervisor is just the conduit to
    >
    > s/hypervisor/the hypervisor/
    >

    Sure, will change commit message

    >> deliver the guest messages to the security processor. Each
    >> message is protected with an AEAD (AES-256 GCM). Use minimal
    >> GCM library to encrypt/decrypt SNP Guest messages to communicate
    >> with the PSP.
    >>
    >> Moreover, the hypervisor should not be intercepting RDTSC/RDTSCP
    >> when Secure TSC is enabled. A #VC exception will be generated if
    >> the RDTSC/RDTSCP instructions are being intercepted. If this should
    >> occur and Secure TSC is enabled, terminate guest execution.
    >
    > This seems like a separate patch.

    Sure.

    >>
    >> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
    >> ---

    >> diff --git a/arch/x86/include/asm/sev-guest.h b/arch/x86/include/asm/sev-guest.h
    >> index 834cdae302ad..d5ed041ce06b 100644
    >> --- a/arch/x86/include/asm/sev-guest.h
    >> +++ b/arch/x86/include/asm/sev-guest.h

    >> +#define SNP_TSC_INFO_REQ_SZ 128
    >> +    /* Must be zero filled */
    >> +    u8 rsvd[SNP_TSC_INFO_REQ_SZ];
    >> +} __packed;
    >> +
    >> +struct snp_tsc_info_resp {
    >> +    /* Status of TSC_INFO message */
    >> +    u32 status;
    >> +    u32 rsvd1;
    >> +    u64 tsc_scale;
    >> +    u64 tsc_offset;
    >> +    u64 tsc_factor;
    >
    > This should be a u32 ...
    >

    Right, will correct.

    >> +    u8 rsvd2[96];
    >
    > Which then makes this 100.

    Yes.

    >> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
    >> index 3750e545d688..280aaa1e6aad 100644
    >> --- a/arch/x86/kernel/sev.c
    >> +++ b/arch/x86/kernel/sev.c

    >> +bool __init snp_secure_tsc_prepare(void)
    >> +{
    >> +    platform_data = kzalloc(sizeof(*platform_data), GFP_KERNEL);
    >> +    if (!platform_data)
    >> +        return false;
    >> +
    >> +    /* Initialize the PSP channel to send snp messages */
    >> +    if (snp_setup_psp_messaging(platform_data))
    >> +        sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
    >> +
    >> +    if (cc_platform_has(CC_ATTR_GUEST_SECURE_TSC)) {
    >
    > Should this be checked before allocating memory and calling snp_setup_psp_messaging()?

    No, as we need guest messaging to work without Secure TSC as well.


    > Also, I notice here you use the cc_platform_has() function but in previous
    > patches you check sev_status directly.

    For sev-shared.c, cc_platform_has() is not available when compiling boot/compressed.

    I will change the below call site to cc_platfrom_has() after testing.

    arch/x86/kernel/sev.c: if (regs->cx == MSR_IA32_TSC && (sev_status & MSR_AMD64_SNP_SECURE_TSC)) {

    > And you don't implement support for
    > CC_ATTR_GUEST_SECURE_TSC until the last patch instead of now.

    This is to make sure that SECURE_TSC is enabled only after the last patch.
    As cc_platform_has() is being used at multiple places to enable the feature.

    > You can't get here until SNP_FEATURES_PRESENT is updated.
    >
    >> +        if (snp_get_tsc_info())
    >> +            sev_es_terminate(SEV_TERM_SET_GEN, GHCB_SNP_UNSUPPORTED);
    >> +
    >> +        pr_info("SecureTSC enabled\n");
    >> +    }
    >
    > Blank line.

    Sure

    Regards
    Nikunj

    \
     
     \ /
      Last update: 2023-04-05 09:38    [W:3.106 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site