Re: [PATCH v3 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions

From: Achin Gupta <achin.gupta@arm.com>
To: Jens Wiklander <jens.wiklander@linaro.org>
Cc: Trilok Soni <tsoni@codeaurora.org>,
	Devicetree List <devicetree@vger.kernel.org>,
	David Hartley <dhh@qti.qualcomm.com>,
	Andrew Walbran <qwandor@google.com>,
	arve@android.com, Sudeep Holla <sudeep.holla@arm.com>,
	Arunachalam Ganapathy <arunachalam.ganapathy@arm.com>,
	nd@arm.com, Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions
Date: Wed, 13 Jan 2021 13:58:56 +0000	[thread overview]
Message-ID: <X/78oOu6+YzvYnJ7@C02ZJ1BRLVDN> (raw)
In-Reply-To: <CAHUa44EcxCK0ssNHL_GR7uO3airNUdS9+xmVPvXgQfL9DhZCSw@mail.gmail.com>

On Wed, Jan 13, 2021 at 01:30:56PM +0100, Jens Wiklander wrote:
> On Wed, Jan 13, 2021 at 10:44 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> [...]
> > > > > +static int ffa_partition_probe(const char *uuid_str,
> > > > > +                              struct ffa_partition_info *buffer)
> > > > > +{
> > > > > +       int count;
> > > > > +       uuid_t uuid;
> > > > > +       u32 uuid0_4[4] = { 0 };
> > > > > +
> > > > > +       if (uuid_parse(uuid_str, &uuid)) {
> > > > > +               pr_err("invalid uuid (%s)\n", uuid_str);
> > > > > +               return -ENODEV;
> > > > > +       }
> > > > > +
> > > > > +       export_uuid((u8 *)uuid0_4, &uuid);
> > > > > +       count = __ffa_partition_info_get(uuid0_4[0], uuid0_4[1], uuid0_4[2],
> > > > > +                                        uuid0_4[3], &buffer);
> > > Wrong byte order?
> > > According to section 5.3 of the SMCCC, UUIDs are returned as a single
> > > 128-bit value using the SMC32 calling convention. This value is mapped
> > > to argument registers x0-x3 on AArch64 (resp. r0-r3 on AArch32). x0
> > > for example shall hold bytes 0 to 3, with byte 0 in the low-order
> > > bits.
> > >
> >
> > I need to spend some time to understand the concern here. Initially I agreed
> > with your analysis and then a quick review make be realise it is all OK.
> > I need to check if my understanding is correct again. I thought I will
> > take example and check here itself.
> >
> > UUID: "fd02c9da-306c-48c7-a49c-bbd827ae86ee"

IIUC this maps to (as per RFC4122).

fd02c9da = time_low (bytes 0-3)
306c48c7 = time_mid & time_hi_and_version (bytes 4-7)
a49cbbd8 = clock_seq_hi_and_reserved, clock_seq_low and bytes/octets 0-1 of node (bytes 8-11)
27ae86ee = bytes 2-5 of node (bytes 12-15)

SMCCC says:

w0 : bytes 0-3 with byte 0 in the lower order bits.
w1 : bytes 4-7 with byte 4 in the lower order bits.
w2 : bytes 8-11 with byte 8 in the lower order bits.
w3 : bytes 12-15 with byte 12 in the lower order bits.

This should amount to:

w0 = dac902fd
w1 = c7486c30
w2 = d8bb9ca4
w3 = ee86ae27

So, even though RFC4122 uses big-endian i.e network byte order. The UUID is
encoded as little-endian as per the SMCCC.

What do you reckon?

cheers,
Achin

> >
> > UUID[0]   UUID[1]  UUID[2]  UUID[3] (referring uuid0_4 above)
> > dac902fd c7486c30 d8bb9ca4 ee86ae27
> >
> > It seems correct as per SMCCC convention to me, or am I missing something
> > obvious ?
>
> In this example I'd expect the first register to hold 0xfd02c9da
> regardless of the byte order of the machine. If there is a different
> byte order in the receiver it will still be received as 0xfd02c9da.
> That's how I've understood the specification.

>
> Cheers,
> Jens

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel