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

[ALOK TIWARI <alok.a.tiwari@oracle.com>]

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");

> +
> +	/* 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