Re: [PATCH v14 12/19] kexec: Secure Launch kexec SEXIT support

[ross.philipson@oracle.com]

On 4/23/25 12:58 PM, ALOK TIWARI wrote:
> 
> 
> On 21-04-2025 21:57, Ross Philipson wrote:
>> Prior to running the next kernel via kexec, the Secure Launch code
>> closes down private SMX resources and does an SEXIT. This allows the
>> next kernel to start normally without any issues starting the APs etc.
>>
>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
>> ---
> [clip]
>> +static inline void smx_getsec_sexit(void)
>> +{
>> +    asm volatile ("getsec\n"
>> +              : : "a" (SMX_X86_GETSEC_SEXIT));
>> +}
>> +
>> +/*
>> + * Used during kexec and on reboot paths to finalize the TXT state
>> + * and do an SEXIT exiting the DRTM and disabling SMX mode.
> 
> 'do an SEXIT exiting', sounds awkward. Changed to 'perform an SEXIT to 
> exit' for clarity.
> 
>> + */
>> +void slaunch_finalize(int do_sexit)
>> +{
>> +    u64 one = TXT_REGVALUE_ONE, val;
>> +    void __iomem *config;
>> +
>> +    if (!slaunch_is_txt_launch())
>> +        return;
>> +
>> +    config = ioremap(TXT_PRIV_CONFIG_REGS_BASE, TXT_NR_CONFIG_PAGES *
>> +             PAGE_SIZE);
>> +    if (!config) {
>> +        pr_emerg("Error SEXIT failed to ioremap TXT private reqs\n");
>> +        return;
>> +    }
>> +
>> +    /* Clear secrets bit for SEXIT */
>> +    memcpy_toio(config + TXT_CR_CMD_NO_SECRETS, &one, sizeof(one));
>> +    memcpy_fromio(&val, config + TXT_CR_E2STS, sizeof(val));
>> +
>> +    /* Unlock memory configurations */
>> +    memcpy_toio(config + TXT_CR_CMD_UNLOCK_MEM_CONFIG, &one, 
>> sizeof(one));
>> +    memcpy_fromio(&val, config + TXT_CR_E2STS, sizeof(val));
>> +
>> +    /* Close the TXT private register space */
>> +    memcpy_toio(config + TXT_CR_CMD_CLOSE_PRIVATE, &one, sizeof(one));
>> +    memcpy_fromio(&val, config + TXT_CR_E2STS, sizeof(val));
>> +
>> +    /*
>> +     * Calls to iounmap are not being done because of the state of the
>> +     * system this late in the kexec process. Local IRQs are disabled 
>> and
>> +     * iounmap causes a TLB flush which in turn causes a warning. 
>> Leaving
>> +     * thse mappings is not an issue since the next kernel is going to
> 
> typo thse -> these
> "are not being done because of the state of the system" can be 
> simplified to "are skipped due to the system state."
> "Calls to iounmap are skipped due to the system state this late in the 
> kexec process"
> 
>> +     * completely re-setup memory management.
>> +     */
>> +
>> +    /* Map public registers and do a final read fence */
>> +    config = ioremap(TXT_PUB_CONFIG_REGS_BASE, TXT_NR_CONFIG_PAGES *
>> +             PAGE_SIZE);
>> +    if (!config) {
>> +        pr_emerg("Error SEXIT failed to ioremap TXT public reqs\n");
> 
> reqs or regs ?
> Assuming you meant registers (regs), not requests (reqs)
> 
>> +        return;
>> +    }
>> +
>> +    memcpy_fromio(&val, config + TXT_CR_E2STS, sizeof(val));
>> +
>> +    pr_emerg("TXT clear secrets bit and unlock memory complete.\n");
>> +
>> +    if (!do_sexit)
>> +        return;
>> +
>> +    if (smp_processor_id() != 0)
>> +        panic("Error TXT SEXIT must be called on CPU 0\n");
> 
> Prefixing with "TXT:"
> 'Error' is redundant — panic() itself implies a fatal error.
> we can use panic("TXT: SEXIT must be called on CPU 0\n");

All good points, we will address these.

Thanks
Ross

> 
>> +
>> +    /* In case SMX mode was disabled, enable it for SEXIT */
>> +    cr4_set_bits(X86_CR4_SMXE);
>> +
>> +    /* Do the SEXIT SMX operation */
>> +    smx_getsec_sexit();
>> +
> 
> Thanks,
> Alok