Re: [PATCH v2 20/20] s390: Report AEF features to sysfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] aef_init_save_area() unconditionally executes the 'qaaf' hardware instruction without checking for hardware support, causing a boot failure or kernel panic on unsupported hosts.
- [Medium] Missing error code assignment when kset_create_and_add() fails, resulting in a false success return.

Pre-existing issues:
- [High] The EX_TABLE entry in the qaaf() inline assembly incorrectly protects the non-faulting lhi instruction instead of the potentially faulting .insn instruction.
--

commit 90a00cf465554d5bd4e3bc2a83bb5b6d1a27209d
Author: Steffen Eiden <seiden@linux.ibm.com>

s390: Report AEF features to sysfs

Display various information about the Arm execution Facility (AEF) to
userspace. The files are located in /sys/firmware/aef/. Show if the
feature is available and usable for the host, which formats are
available, and how many arm vCPUS per guest are supported by the
machine. Additionally, provide the raw binary output of QAAF fc1 (Query
Model Capabilities) in the file qmc and fc2 (Initial
System-Registers-Save Area) in the file save_area.

> diff --git a/arch/s390/kernel/aef.c b/arch/s390/kernel/aef.c
> index 825e2643a8921..2691f1ac63dca 100644
> --- a/arch/s390/kernel/aef.c
> +++ b/arch/s390/kernel/aef.c
[ ... ]
> @@ -40,6 +40,66 @@ const struct aef_info *aef_info(void)
>  }
>  EXPORT_SYMBOL(aef_info);
>  
[ ... ]
> +static int __init aef_init_save_area(void)
> +{
> +	int ret;
> +
> +	union qaaf_gr0_gisrsa gr0 = {
> +		.fc = QAAF_FC_GISRSA,
> +		.saf = SAE_SAVE_AREA_FORMAT_0,
> +	};
> +
> +	ret = qaaf(gr0.val, (union qaaf_block *)&save_area);

[Severity: High]
Does aef_init_save_area() need to check if info.sae_avail is set before
executing qaaf()?

If aef_query_info() early returns success when the AEF feature is unsupported,
aef_init_save_area() executes the qaaf instruction unconditionally. This
will cause a boot failure or kernel panic on unsupported hosts.

[Severity: High]
This is a pre-existing issue, but this call unconditionally triggers a kernel
panic on older hardware due to an issue with the qaaf() inline assembly.

arch/s390/include/asm/sae.h:qaaf() {
...
	asm volatile(
		"	lgr	0,%[r0]\n"
		"	.insn	rre,0xb9ad0000,%[r1],0\n"
		"0:	lhi	%[exc],0\n"
		"1:\n"
		EX_TABLE(0b, 1b)
...
}

The EX_TABLE entry incorrectly protects the non-faulting lhi instruction (label
0:) instead of the preceding, potentially faulting .insn instruction. When qaaf
faults, the exception table misses it, resulting in an unhandled hardware
exception.

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
[ ... ]
> @@ -95,7 +155,33 @@ static int __init aef_sysfs_init(void)
>  	if (rc)
>  		return rc;
>  
> +	aef_kobj = kobject_create_and_add("aef", firmware_kobj);
> +	if (!aef_kobj)
> +		return -ENOMEM;
> +
> +	rc = sysfs_create_file(aef_kobj, aef_sysfs_qval_attr(arm_guest_supp));
> +	if (rc)
> +		goto out_kobj;
> +
> +	aef_query_kset = kset_create_and_add("query", NULL, aef_kobj);
> +	if (!aef_query_kset)
> +		goto out_arm_guest_supp;

[Severity: Medium]
Will this error path in aef_sysfs_init() return a false success?

If kset_create_and_add() fails due to memory exhaustion, the local variable
'rc' remains 0 from the successful sysfs_create_file() call above. The initcall
will report success despite failing to create the sysfs structure.

> +
> +	rc = sysfs_create_group(&aef_query_kset->kobj, &aef_query_attr_group);
> +	if (rc)
> +		goto out_kset;
> +
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831145536.913567-1-seiden@linux.ibm.com?part=20