public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kun Qin via groups.io" <kuqin12=gmail.com@groups.io>
To: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: Leif Lindholm <leif.lindholm@oss.qualcomm.com>,
	 "devel@edk2.groups.io" <devel@edk2.groups.io>,
	Olivier Deprez <Olivier.Deprez@arm.com>,
	 Yeo Reum Yun <YeoReum.Yun@arm.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	 Manish Pandey2 <Manish.Pandey2@arm.com>
Subject: Re: [edk2-devel] 18 register support for SMC/SVC on AARCH64
Date: Tue, 11 Mar 2025 23:32:30 -0700	[thread overview]
Message-ID: <CABhrWrQV9XSNxcff8hfdM4u659=FxVqhbjO_HNAmvPTk5ekvnA@mail.gmail.com> (raw)
In-Reply-To: <AS8PR08MB6806E12F5C5C22C9EFDB49CC84D62@AS8PR08MB6806.eurprd08.prod.outlook.com>

[-- Attachment #1: Type: text/plain, Size: 11643 bytes --]

Thank you for the clarification, Sami. I will update the PR according to
the discussion shortly.

Regards,
Kun

On Mon, Mar 10, 2025 at 11:25 AM Sami Mujawar <Sami.Mujawar@arm.com> wrote:

> Hi All,
>
> I think we can only support FF-A 1.2 in StMM. The main reason being we
> need Direct Req2, without which it would not be possible to target services
> hosted in StMM. e.g. when StMM hosts variable service and another service
> say fTPM. Since both these services are within the same StMM partition we
> need Direct Req2 to target messages to the fTPM service.
>
> Since Direct Req2 is only available since FF-A 1.2, it is a minimum
> requirement for supporting in StMM.
>
> I think we just move to 18 registers.
>
> We can add a function to get the SMCCC version, but invoking that API
> should be the caller’s responsibility and we should not make that call
> implicit in the library (i.e. calling in the SMC/HVC library constructor).
>
> As for the FF-A version check, I think we can bailout if the version is <
> 1.2.
>
> Regards,
>
> Sami Mujawar
>
>
> ------------------------------
> *From:* Kun Qin <kuqin12@gmail.com>
> *Sent:* 10 March 2025 17:13
> *To:* Leif Lindholm <leif.lindholm@oss.qualcomm.com>
> *Cc:* devel@edk2.groups.io <devel@edk2.groups.io>; Sami Mujawar <
> Sami.Mujawar@arm.com>; Olivier Deprez <Olivier.Deprez@arm.com>; Yeo Reum
> Yun <YeoReum.Yun@arm.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Manish Pandey2 <Manish.Pandey2@arm.com>
> *Subject:* Re: [edk2-devel] 18 register support for SMC/SVC on AARCH64
>
> Hi Leif & Sami,
>
> Thanks for providing the information and feedback. It sounds like the
> SPMC@EL3 will get resolved with the TF-A PR, which will cover the
> complete usage of FF-A over SMC/SVC conduit with 18 registers.
>
> But do we want to move to 18-register all along? Or add a version check in
> the SMC/SVC library to figure out the number of registers supported (then
> FF-A library will also check this and bail if <= 1.1)? If we care to
> entertain that front, we can add it as well.
>
> Thanks,
> Kun
>
> On Mon, Mar 10, 2025 at 5:23 AM Leif Lindholm <
> leif.lindholm@oss.qualcomm.com> wrote:
>
> Hi Sami,
>
> Thanks for the detailed summary.
>
> FF-A 1.1 is extremely unfortunate in how it fails to specify which
> revision of SMCCC
> it builds on, but thankfully 1.0 *did* explicitly call out 1.2.
>
> Right. So, yeah, since the conduit cannot care about how or why it's used,
> it feels to me like either:
> - FF-A 1.0/1,1 are violating the abstraction layers
> or
> - The existing TF-A implementation violates the specification.
>
> But I don't see anything in FF-A 1.0/1,1 implying parts of the SMC
> calling convention
> can be sidestepped, so personal hot take would be that the current
> TF-A behaviour is
> buggy, and that patch is a bugfix.
>
> Best Regards,
>
> Leif
>
> On Fri, 7 Mar 2025 at 14:20, Sami Mujawar via groups.io
> <sami.mujawar=arm.com@groups.io> wrote:
> >
> > Hi All,
> >
> >
> >
> > Following is a summary of the situation.
> >
> >
> >
> > FF-A 1.1 does not use more than 8 registers for parameter/return values.
> >
> >
> >
> > FF-A 1.2 introduced Direct Req/Resp 2 which utilises up to 18 registers
> for parameter/return values.
> >
> >
> >
> > SMCCC 1.2 permitted the use of X4-X17 as return registers and X8-X17 as
> argument registers.
> >
> > See ‘Appendix F: Changelog’, in the SMCCC specification, DEN0028,
> version 1.6G
> >
> > (https://developer.arm.com/documentation/den0028/latest/)
> >
> >
> >
> > The section ‘2.4 Conduits’, in the FF-A 1.0 specification specifies that
> the SMC conduit as described in the SMCCC 1.2 specification should be used,
> see
> https://documentation-service.arm.com/static/5fb7e8a6ca04df4095c1d65e?token=
> >
> >
> >
> > Considering the above one can infer that FF-A 1.0+ requires SMCCC 1.2
> which allows usage of X1-X17 as parameter and X0-X17 as return values when
> using SMC64/HVC64 argument passing. See SMCCC 1.2 specification, Section
> 2.7 SMC64/HVC64 argument passing.
> >
> >
> >
> > To support StandaloneMM (StMM), TF-A provides a SPMC+SPMD@EL3 which
> currently supports FF-A 1.1 with partial support for Direct Req/Resp2 (i.e.
> only 8 registers supported as parameter/return values).  This was done to
> have a minimal software stack for supporting FF-A with StMM in the
> upstream, i.e. the FVP software stack with FF-A support has UEFI + TF-A
> (with SPMC+SPMD@EL3) + StMM.
> >
> >
> >
> > Apparently, the SPMC+SPMD@EL3 support in TF-A is deprecated and no
> further updates are planned. Therefore, to support a minimal software stack
> we were looking at a build option in edk2 that supports configuring the use
> of 8 registers.
> >
> >
> >
> > However, considering the fact that FF-A 1.0+ requires compliance with
> SMCCC 1.2, I think we should just extend the SPMC+SPMD@EL3 support in
> TF-A to support 18 registers. That way even though the SPMC+SPMD@EL3
> support in TF-A is deprecated, it is at least fully compliant with the
> SMCCC 1.2 requirements.
> >
> > Levi has submitted a change to TF-A at
> https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/36018. If
> this TF-A change is approved and merged, the ArmCallSmc() implementation in
> edk2 can just support 18 registers by default.
> >
> >
> >
> > Regards,
> >
> >
> >
> > Sami Mujawar
> >
> >
> >
> > From: Leif Lindholm <leif.lindholm@oss.qualcomm.com>
> > Date: Thursday, 6 March 2025 at 22:58
> > To: Kun Qin <kuqin12@gmail.com>
> > Cc: Sami Mujawar <Sami.Mujawar@arm.com>, Olivier Deprez <
> Olivier.Deprez@arm.com>, Yeo Reum Yun <YeoReum.Yun@arm.com>,
> edk2-devel-groups-io <devel@edk2.groups.io>, Ard Biesheuvel <
> ardb+tianocore@kernel.org>
> > Subject: Re: [edk2-devel] 18 register support for SMC/SVC on AARCH64
> >
> > Hi Kun,
> >
> > My point is this has nothing to do with FF-A or likeliness. It is
> > architecturally broken just from an SMCCC standpoint.
> > But yes, I would like to hear more from Arm about the specific concern.
> >
> > Regards,
> >
> > Leif
> >
> > On Thu, 6 Mar 2025 at 20:42, Kun Qin <kuqin12@gmail.com> wrote:
> > >
> > > Hi Leif,
> > >
> > > Thanks for the input. I agree that platforms supporting FF-A v1.2+
> will rely on SMCCC v1.1+, and thus platforms not supporting whole 18
> register usage are not complying with the spec.
> > >
> > > I think Sami, Levi, or Olivier could chime in for better insights on
> their concerns about SPMC at EL3. As far as the setup we are using (Hafnium
> as SPMC), the 18-register usage is good across all firmware entities.
> > >
> > > Regards,
> > > Kun
> > >
> > > On Thu, Mar 6, 2025 at 1:42 AM Leif Lindholm <
> leif.lindholm@oss.qualcomm.com> wrote:
> > >>
> > >> Hi Kun,
> > >>
> > >> On Thu, 6 Mar 2025 at 06:13, Kun Qin <kuqin12@gmail.com> wrote:
> > >> >
> > >> > Hi ARM enthusiasts,
> > >> >
> > >> > I recently filed a PR to allow 18 register support for SMC/SVC
> calls between UEFI and secure partition components:
> https://github.com/tianocore/edk2/pull/10685/files.
> > >> >
> > >> > The main purpose of this change is to allow more registers to hold
> values while doing FF-A transactions. In FF-A spec v1.2 and onward, the
> section "FFA_MSG_SEND_DIRECT_REQ2" mentions that up to 18 general-purpose
> registers can be used for such calls. However, the current SMC/SVC
> implementation in EDK2 only supports up to 8 registers.
> > >> >
> > >> > There were some differing opinions on how to support this more
> properly. Could you please review the PR and chime in on the email thread
> about how to proceed with it?
> > >> >
> > >> > TL;DR:
> > >> >
> > >> > In conversations with ARM stakeholders, they revealed concerns
> about using 18 registers all along because some older firmware components
> on the secure side do not support full 18 register usage, and the returned
> values may not be sane. Therefore, there is a need for a build flag that
> controls how many registers are used during SMC calls to be backwards
> compatible, which is the PcdSxcUse18Registers approach I went with in the
> PR.
> > >>
> > >> I'm not sure I follow this one (and this is very much the reason I
> > >> asked for email thread breakout - thank you).
> > >> Code that relies on the 18 registers is relying on SMCCC >= 1.1.
> > >> If code is relying on SMCCC >= 1.1, then it must verify that the
> > >> secure side supports that
> > >> by making an SMCCC_VERSION call.
> > >> If that returns NOT_SUPPORTED, or that the version is 1.0, then the
> > >> fewer-registers calling
> > >> conventions MUST be used. Otherwise, the 18-register variant is safe.
> > >> Am I missing something?
> > >>
> > >> If we're talking about supporting secure sides that don't comply with
> > >> the spec, then I think
> > >> that should be very much a "deal with broken secure firmware quirk"
> > >> and not a different
> > >> library.
> > >> And in that case, it seems to me platform ports that felt the need to
> > >> deal with broken
> > >> secure sides should opt into that, with special handling in the
> single library.
> > >>
> > >> If we're talking about supporting edk2 code that doesn't sanity check
> > >> the version, then
> > >> I'd suggest we fix the buggy edk2 code instead.
> > >>
> > >> Best Regards,
> > >>
> > >> Leif
> > >>
> > >> > The original approach of using the PCD was to make it a feature
> flag so that all header files, assembly files, and C files will not even
> compile the code that supports more than 8 registers if not needed. But
> that would involve the PCDs getting pre-processed by the build framework,
> and all components using the ArmSmcLib would thus have to add the PCD in
> their inf files. So instead, we went with the runtime code evaluation.
> > >> >
> > >> > On the PR, Sami suggested creating a new interface that supports
> SMC with 18 registers and making the PCD control which function to call.
> For FF-A functions that only involve 8 registers or under, the caller
> should just use the legacy interfaces. But the issue is, once Standalone MM
> hands off the control using an 8 register SMC call, it will only be able to
> process 8 register incoming requests, which will not work if it is woken up
> by an  FFA_MSG_SEND_DIRECT_REQ2 call using 18 registers.
> > >> >
> > >> > Any input is appreciated.
> > >> >
> > >> > Regards,
> > >> > Kun
> >
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
> > 
>
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#121188): https://edk2.groups.io/g/devel/message/121188
Mute This Topic: https://groups.io/mt/111543575/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 17638 bytes --]

      reply	other threads:[~2025-03-12  6:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-06  6:13 [edk2-devel] 18 register support for SMC/SVC on AARCH64 Kun Qin via groups.io
2025-03-06  9:42 ` Leif Lindholm via groups.io
2025-03-06 20:42   ` Kun Qin via groups.io
2025-03-06 22:57     ` Leif Lindholm via groups.io
2025-03-07 14:19       ` Sami Mujawar via groups.io
2025-03-10 12:23         ` Leif Lindholm via groups.io
2025-03-10 17:13           ` Kun Qin via groups.io
2025-03-10 18:25             ` Sami Mujawar via groups.io
2025-03-12  6:32               ` Kun Qin via groups.io [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-list 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='CABhrWrQV9XSNxcff8hfdM4u659=FxVqhbjO_HNAmvPTk5ekvnA@mail.gmail.com' \
    --to=devel@edk2.groups.io \
    /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