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

Jason Gunthorpe [off-list ref] writes:

quoted

[ ... 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