Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jamie Nguyen <jamien@nvidia.com>
To: Sudeep Holla <sudeep.holla@kernel.org>
Cc: "linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS
Date: Thu, 14 May 2026 17:37:41 +0000	[thread overview]
Message-ID: <27E42CCD-8518-4802-B549-A79B94B5658A@nvidia.com> (raw)
In-Reply-To: <20260514-generous-undetectable-mastiff-cc43d3@sudeepholla>



> On May 14, 2026, at 2:31 AM, Sudeep Holla <sudeep.holla@kernel.org> wrote:
> 
> On Tue, May 12, 2026 at 08:28:00PM -0700, Jamie Nguyen wrote:
>> __ffa_partition_info_get_regs() walks the response with a hardcoded
>> 24-byte stride (regs += 3) even though the SPMC tells us the actual
>> per-descriptor size via PARTITION_INFO_SZ in x2[63:48]. The size is
>> read into buf_sz and then thrown away.
>> 
>> That works while every SPMC returns the FF-A v1.1 layout, but it falls
>> apart against a v1.3 SPMC returning the 48-byte descriptor. The loop
>> strides over half a descriptor at a time and ends up parsing every
>> other entry from a slice of two adjacent ones.
>> 
>> The FF-A spec (v1.2, section 18.5) says that the producer should
>> report the descriptor size, and the consumer is supposed to stride by
>> that size and ignore any trailing fields it doesn't understand. The
>> non-REGS path (__ffa_partition_info_get) does this already, and the
>> REGS path should match.
>> 
>> Use buf_sz for the stride, and bail out with -EPROTO if the SPMC
>> reports something we can't safely walk.
>> 
>> Fixes: 7bc0f589c81d ("firmware: arm_ffa: Fix big-endian support in __ffa_partition_info_regs_get()")
>> Signed-off-by: Jamie Nguyen <jamien@nvidia.com>
>> ---
>> drivers/firmware/arm_ffa/driver.c | 35 ++++++++++++++++++++++++++++---
>> 1 file changed, 32 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
>> index c72ee4756585..b712e8a03dab 100644
>> --- a/drivers/firmware/arm_ffa/driver.c
>> +++ b/drivers/firmware/arm_ffa/driver.c
>> @@ -321,6 +321,22 @@ __ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
>> #define PART_INFO_ID(x)		((u16)(FIELD_GET(PART_INFO_ID_MASK, (x))))
>> #define PART_INFO_EXEC_CXT(x)	((u16)(FIELD_GET(PART_INFO_EXEC_CXT_MASK, (x))))
>> #define PART_INFO_PROPERTIES(x)	((u32)(FIELD_GET(PART_INFO_PROPS_MASK, (x))))
>> +
>> +/*
>> + * FF-A v1.2 section 13.9 Table 13.40: registers x3..x17 carry the partition
>> + * descriptors, i.e. 15 u64 of payload per FFA_PARTITION_INFO_GET_REGS call.
>> + */
>> +#define FFA_PART_INFO_REGS_PAYLOAD_U64	15
>> +
>> +/*
>> + * FF-A v1.1 partition information descriptor (FF-A v1.2 section 6.2.1
>> + * Table 6.1): id (2) + exec_ctxt (2) + properties (4) + UUID (16) = 24
>> + * bytes. This is the minimum size the SPMC must report; the kernel reads
>> + * exactly these fields and ignores any trailing ones per the forward-
>> + * compatibility rules in FF-A v1.2 section 18.5.
>> + */
> 
> I can't see any such details is the above mention version and section.
> Can you confirm you are looking at [1] ?

Yes, I am looking at [1].  The size field and the rule are in two
different places:

      - Section 13.9 Table 13.40, page 188:
          x2 Bits[63:48] = "Size in bytes of each partition information
          entry descriptor."
        (read into buf_sz today and discarded)

      - Section 18.5 page 264 rule 4:
          "A consumer of this data structure uses the size corresponding
           to the Framework version it implements to consume only fields
           defined in its version. Additional fields in the producer's
           version of this data structure are safely ignored enabling
           forward compatibility."

  If you agree, I'll rebase against linux-next and send a v2.

  [1]: https://developer.arm.com/documentation/den0077/j

> -- 
> Regards,
> Sudeep
> 
> [1] https://developer.arm.com/documentation/den0077/j


      reply	other threads:[~2026-05-14 17:38 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13  3:28 [PATCH] firmware: arm_ffa: honor descriptor size in PARTITION_INFO_GET_REGS Jamie Nguyen
2026-05-13 17:15 ` Sudeep Holla
2026-05-13 19:48   ` Jamie Nguyen
2026-05-14  9:16     ` Sudeep Holla
2026-05-14 17:37       ` Jamie Nguyen
2026-05-14  9:31 ` Sudeep Holla
2026-05-14 17:37   ` Jamie Nguyen [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27E42CCD-8518-4802-B549-A79B94B5658A@nvidia.com \
    --to=jamien@nvidia.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sudeep.holla@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox