Re: [kvm-unit-tests PATCH v3 15/17] x86/cet: Run SHSTK and IBT tests as appropriate if either feature is supported

[Mathias Krause <minipli@grsecurity.net>]

On 14.11.25 01:12, Sean Christopherson wrote:
> Run the SHSTK and IBT tests if their respective feature is supported, as
> nothing in the architecture requires both features to be supported.
> Decoupling the two features allows running the SHSTK test on AMD CPUs,
> which support SHSTK but not IBT.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  x86/cet.c | 50 +++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/x86/cet.c b/x86/cet.c
> index eeab5901..26cd1c9b 100644
> --- a/x86/cet.c
> +++ b/x86/cet.c
> @@ -85,7 +85,7 @@ static uint64_t cet_ibt_func(void)
>  #define ENABLE_SHSTK_BIT 0x1
>  #define ENABLE_IBT_BIT   0x4
>  
> -int main(int ac, char **av)
> +static void test_shstk(void)
>  {
>  	char *shstk_virt;
>  	unsigned long shstk_phys;
> @@ -94,17 +94,10 @@ int main(int ac, char **av)
>  	bool rvc;
>  
>  	if (!this_cpu_has(X86_FEATURE_SHSTK)) {
> -		report_skip("SHSTK not enabled");
> -		return report_summary();
> +		report_skip("SHSTK not supported");
> +		return;
>  	}
>  
> -	if (!this_cpu_has(X86_FEATURE_IBT)) {
> -		report_skip("IBT not enabled");
> -		return report_summary();
> -	}
> -
> -	setup_vm();
> -
>  	/* Allocate one page for shadow-stack. */
>  	shstk_virt = alloc_vpage();
>  	shstk_phys = (unsigned long)virt_to_phys(alloc_page());
> @@ -124,9 +117,6 @@ int main(int ac, char **av)
>  	/* Store shadow-stack pointer. */
>  	wrmsr(MSR_IA32_PL3_SSP, (u64)(shstk_virt + 0x1000));
>  
> -	/* Enable CET master control bit in CR4. */
> -	write_cr4(read_cr4() | X86_CR4_CET);
> -
>  	printf("Unit tests for CET user mode...\n");
>  	run_in_user(cet_shstk_func, CP_VECTOR, 0, 0, 0, 0, &rvc);
>  	report(rvc && exception_error_code() == CP_ERR_NEAR_RET,
> @@ -136,19 +126,45 @@ int main(int ac, char **av)
>  	report(rvc && exception_error_code() == CP_ERR_FAR_RET,
>  	       "FAR RET shadow-stack protection test");
>  
> +	/* SSP should be 4-Byte aligned */
> +	vector = wrmsr_safe(MSR_IA32_PL3_SSP, 0x1);
> +	report(vector == GP_VECTOR, "MSR_IA32_PL3_SSP alignment test.");
> +}
> +
> +static void test_ibt(void)
> +{
> +	bool rvc;
> +
> +	if (!this_cpu_has(X86_FEATURE_IBT)) {
> +		report_skip("IBT not supported");
> +		return;
> +	}
> +
>  	/* Enable indirect-branch tracking */
>  	wrmsr(MSR_IA32_U_CET, ENABLE_IBT_BIT);
>  
>  	run_in_user(cet_ibt_func, CP_VECTOR, 0, 0, 0, 0, &rvc);
>  	report(rvc && exception_error_code() == CP_ERR_ENDBR,
>  	       "Indirect-branch tracking test");
> +}
> +
> +int main(int ac, char **av)
> +{
> +	if (!this_cpu_has(X86_FEATURE_SHSTK) && !this_cpu_has(X86_FEATURE_IBT)) {
> +		report_skip("No CET features supported");
> +		return report_summary();
> +	}
> +
> +	setup_vm();
> +
> +	/* Enable CET global control bit in CR4. */
> +	write_cr4(read_cr4() | X86_CR4_CET);
> +
> +	test_shstk();
> +	test_ibt();
>  
>  	write_cr4(read_cr4() & ~X86_CR4_CET);
>  	wrmsr(MSR_IA32_U_CET, 0);
>  
> -	/* SSP should be 4-Byte aligned */
> -	vector = wrmsr_safe(MSR_IA32_PL3_SSP, 0x1);
> -	report(vector == GP_VECTOR, "MSR_IA32_PL3_SSP alignment test.");
> -
>  	return report_summary();
>  }

Looks good to me!

Successfully tested on Intel ADL, selectively disabling IBT (-cpu
host,-ibt), shadow stacks (-cpu host,-shstk) or both (-cpu
host,-ibt,-shstk), each doing "The Right Thing," therefore:

Reviewed-by: Mathias Krause <minipli@grsecurity.net>
Tested-by: Mathias Krause <minipli@grsecurity.net>

Thanks,
Mathias