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

[Jason Gunthorpe]

On Sat, Aug 29, 2026 at 11:42:20AM +0530, Aneesh Kumar K.V wrote:
> Jason Gunthorpe <jgg@xxxxxxxxxx> writes:
> 
> >> [ ... 43 lines skipped ... ]
> >> @@ -94,6 +95,12 @@ static const struct smccc_device_info smccc_devices[] __initconst = {
> >>  		.requires_smc   = false,
> >>  		.device_name    = "arm-smccc-trng",
> >>  	},
> >> +
> >> +	{
> >> +		.func_id        = SMC_RSI_ABI_VERSION,
> >  [Severity: High]
> >  Does this new entry cause uninitialized registers to be passed to the
> >  firmware?
> >
> >  Looking at smccc_probe_smccc_device():
> >
> >  	arm_smccc_1_1_invoke(smccc_dev->func_id, &res);
> >
> >  The invocation only passes the func_id, leaving r1 uninitialized. Since the
> >  SMC_RSI_ABI_VERSION ABI requires r1 to hold the requested version parameter,
> >  does this leak uninitialized kernel register state to the firmware and pass
> >  a garbage ABI version?
> >
> 
> Yes. This even can result in error return from firmware like
> 
> [   rmm ] SMC_RMI_VERSION                   6 > RMI_RMI_ERROR_INPUT

Yeah, it strikes me that the RSI handshake is quite a different
discovery protocol than the rng. That's unfortunate that it isn't
standardized for this bus design. Still it shouldn't be a big deal for
this code to deal with it, but unfortunate.
 
> >> -	return ret;
> >> +	ret = devm_add_action_or_reset(&sdev->dev, unregister_cca_tsm_report,
> >> +				       NULL);
> >> +	if (ret < 0) {
> >
> > Can just make unregister the remove function. Don't need to use devm
> > for everything.
> >
> 
> IIUC, you are suggesting to do the below?
> 
> static void cca_tsm_remove(struct arm_smccc_device *sdev)
> {
>         tsm_report_unregister(&arm_cca_tsm_report_ops);
> }
> 
> static struct arm_smccc_driver cca_tsm_driver = {
>         .driver_name = "arm_cca_tsm",
>         .probe = cca_tsm_probe,
>         .remove = cca_tsm_remove,
>         ...

Yeah

Jason