Re: [PATCH 2/2] ACPI: FFH: Support FF-A direct request 2 calls

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Tue,  3 Nov 2026 17:39:07 +0000
Sudeep Holla <sudeep.holla@kernel.org> wrote:

> Arm Functional Fixed Hardware specification v1.3 defines FFH
> OperationRegion offset 2 for issuing FFA_MSG_SEND_DIRECT_REQ2 calls.
> The arm64 FFH handler currently supports only generic 32-bit and 64-bit
> SMCCC calls, so it cannot implement this standardized interface.
> 
> Add a standalone FF-A path using the existing SMCCC conduit. Negotiate
> FF-A v1.2, discover the sender endpoint with FFA_ID_GET, and use either
> the receiver supplied by firmware or resolve a unique receiver from the
> service UUID with FFA_PARTITION_INFO_GET_REGS.
> 
> Convert the ACPI ToUUID representation to the FF-A register encoding,
> construct the direct request, resume interrupted or yielded calls with
> FFA_RUN, and return the defined FFH status codes to AML.
> 
> Register-based partition discovery avoids RX/TX buffer ownership and
> keeps the FFH implementation independent of the Arm FF-A transport
> driver.
> 
> Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
Hi Sudeep,

I was curious so took a quick look. Some drive by comments inline.

thanks,

Jonathan

> +static int acpi_ffh_ffa_features(struct acpi_ffh_data *ffh_ctxt, u32 fid)
> +{
> +	struct arm_smccc_1_2_regs args = {
> +		.a0 = FFA_FEATURES,
> +		.a1 = fid,
> +	};
> +	struct arm_smccc_1_2_regs res;
> +
> +	acpi_ffh_ffa_invoke(ffh_ctxt, &args, &res);
> +	if (res.a0 == FFA_ERROR) {
> +		if ((s32)res.a2 == FFA_RET_NOT_SUPPORTED)
> +			return -EOPNOTSUPP;
> +		return -EINVAL;
> +	}
> +
> +	return res.a0 == FFA_SUCCESS ? 0 : -EIO;
> +}
> +
> +static int acpi_ffh_ffa_init(struct acpi_ffh_data *ffh_ctxt)
> +{
> +	struct arm_smccc_1_2_regs args = { }, res;
> +	u32 version;
> +	int ret = 0;
> +
> +	mutex_lock(&acpi_ffh_ffa_lock);

Use a guard() here given it will allow direct returns
in all the error paths. Seems like a nice easy code
flow simplification particularly as the good vs bad
paths will be more obvious.


> +	if (acpi_ffh_ffa_initialized)
> +		goto out;

With guard this becomes
		return 0;
and is obviously a good path.

> +
> +	if (!acpi_ffh_ffa_version_negotiated) {
Maybe combine this flag and acpi_ffh_ffa_initialized into
states in a state machine rather than separate bool.

Passing this one just becomes moving to the partly configured
state.

> +		args.a0 = FFA_VERSION;
> +		args.a1 = FFA_VERSION_1_2;

I'd do similar to have below and initialize the whole thing here.
That way no need to look back to see if the rest is correctly
zeroed.

> +		acpi_ffh_ffa_invoke(ffh_ctxt, &args, &res);
> +		version = res.a0;
> +		if ((s32)version == SMCCC_RET_NOT_SUPPORTED ||
> +		    FFA_MAJOR_VERSION(version) != 1 ||
> +		    version < FFA_VERSION_1_2) {
> +			ret = -EOPNOTSUPP;
> +			goto out;
> +		}
> +		acpi_ffh_ffa_version_negotiated = true;
> +	}
> +
> +	ret = acpi_ffh_ffa_features(ffh_ctxt, FFA_MSG_SEND_DIRECT_REQ2);
> +	if (ret)
> +		goto out;
> +
> +	ret = acpi_ffh_ffa_features(ffh_ctxt, FFA_MSG_SEND_DIRECT_RESP2);
> +	if (ret)
> +		goto out;
> +
> +	ret = acpi_ffh_ffa_features(ffh_ctxt, FFA_PARTITION_INFO_GET_REGS);
> +	if (ret)
> +		goto out;
> +
> +	args = (struct arm_smccc_1_2_regs) { .a0 = FFA_ID_GET };
> +	acpi_ffh_ffa_invoke(ffh_ctxt, &args, &res);
> +	if (res.a0 == FFA_ERROR) {
> +		ret = (s32)res.a2 == FFA_RET_NOT_SUPPORTED ?
> +		      -EOPNOTSUPP : -EIO;
> +		goto out;
> +	}
> +	if (res.a0 != FFA_SUCCESS) {
> +		ret = -EIO;
> +		goto out;
> +	}
> +
> +	acpi_ffh_ffa_sender_id = (u16)res.a2;
> +	acpi_ffh_ffa_initialized = true;
> +out:
> +	mutex_unlock(&acpi_ffh_ffa_lock);
> +	return ret;
> +}