Re: [PATCH v10 6/7] firmware: smccc: arm-cca-guest: Bind the TSM provider to an SMCCC device

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Jason Gunthorpe <jgg@nvidia.com> writes:

>> [ ... 42 lines skipped ... ]
>> @@ -94,6 +95,11 @@ static const struct smccc_device_info smccc_devices[] __initconst = {
>>  		.requires_smc   = false,
>>  		.device_name    = "arm-smccc-trng",
>>  	},
>> +	{
>> +		.func_id        = SMC_RSI_ABI_VERSION,
>> +		.requires_smc   = true,
>> +		.device_name    = "arm-rsi",
>> +	},
>>  };
>>  
>>  static bool __init smccc_probe_smccc_device(const struct smccc_device_info *smccc_dev)
>
>    [Severity: High]
>    When this new entry is probed by smccc_probe_smccc_device(), does
>    arm_smccc_1_1_invoke() pass uninitialized registers to the firmware?
>
>    Looking at smccc_probe_smccc_device() during system initialization:
>
>    drivers/firmware/smccc/smccc.c:smccc_probe_smccc_device() {
>        ...
> 	   arm_smccc_1_1_invoke(smccc_dev->func_id, &res);
>        ...
>    }
>
>    Since arm_smccc_1_1_invoke() is a variadic macro, providing only the func_id
>    leaves x1/r1 uninitialized in the SMC inline assembly.
>
>    For SMC_RSI_ABI_VERSION, the RMM uses x1 as the requested interface revision
>    to determine supported revisions.
>
>    Could this pass a garbage requested interface revision to the RMM,
>    potentially causing probe failures or returning an unsupported boundary?
>
>    Could this also leak kernel register state to EL2?
>
> Sashiko still has this to say, I think it should pass 0 as the x1
> 'requested version' instead of unpredictable garbage.
>

To handle 

> Could this also leak kernel register state to EL2?
>

Should we instead do

	arm_smccc_1_1_invoke(smccc_dev->func_id,
			     0, 0, 0, 0, 0, 0, 0, &res);
	ret = res.a0;

There is no standard defining how the various SMCCC VERSION calls are
expected to work. For example, ARM_SMCCC_TRNG_VERSION does not use x1,
while SMC_RSI_ABI_VERSION does. Another VERSION call could use both x1
and x2. The only consistent behavior is to return
SMCCC_RET_NOT_SUPPORTED when the SMCCC function ID is not supported.

-aneesh