Drop C extension from RISC-V calling convention requirements?
24 views
Skip to first unread message
Warkentin, Andrei
Sep 6, 2023, 1:10:59 PM9/6/23
to RISC-V Firmware Exchange
Dear all,
There's ongoing discussions/proposals from some companies on dropping the C (compressed) extension for 'higher end' profiles.
The C extension today is part of the "baseline ISA" defined by the RISC-V calling convention (https://mantis.uefi.org/mantis/view.php?id=2376), but depending on how those RVI discussions go, it might disappear from some chips. The implication here is that, while platform firmware is free to use the C extension (e.g. compile edk2 or uboot against rv64gc), OpRoms for sure would need avoid targeting C (esp since discrete devices would be likely used in systems without C).
Any concerns around dropping C from the UEFI calling convention requirements? Again this doesn't mean a UEFI firmware implementation can't use C. The implication is more on 3rd party binary drivers (OpRoms) and OS loaders.
A
There's ongoing discussions/proposals from some companies on dropping the C (compressed) extension for 'higher end' profiles.
The C extension today is part of the "baseline ISA" defined by the RISC-V calling convention (https://mantis.uefi.org/mantis/view.php?id=2376), but depending on how those RVI discussions go, it might disappear from some chips. The implication here is that, while platform firmware is free to use the C extension (e.g. compile edk2 or uboot against rv64gc), OpRoms for sure would need avoid targeting C (esp since discrete devices would be likely used in systems without C).
Any concerns around dropping C from the UEFI calling convention requirements? Again this doesn't mean a UEFI firmware implementation can't use C. The implication is more on 3rd party binary drivers (OpRoms) and OS loaders.
A
Heinrich Schuchardt
Sep 6, 2023, 1:38:37 PM9/6/23
to Warkentin, Andrei, RISC-V Firmware Exchange
could you, please, provide a link to the proposal that can be viewed by
non-members of the UEFI working group?
It is unclear why the existence of the compressed code extension should
have any impact on a calling convention which is about register and
stack use.
Do you want to require that all code pointers (e.g. in the system table)
should point to an address that is a multiple of four?
Best regards
Heinrich
Warkentin, Andrei
Sep 6, 2023, 3:53:28 PM9/6/23
to Heinrich Schuchardt, RISC-V Firmware Exchange
Hi Heinrich,
>
> could you, please, provide a link to the proposal that can be viewed by non-
> members of the UEFI working group?
>
I don't have a proposal for removing the C extension for you as one has not been shared with me or publicly (within RVI or USWG). I'm just being upfront about something that could be a pretty impactful change to the UEFI spec short term *if* approved once it is discussed. Seeing that the Mantis ticket for cleanup up the UEFI spec has not been merged yet, not is as good as time as any to "lay some straw on the ground" to avoid another UEFI spec revision down the line.
Or were you asking about the existing https://mantis.uefi.org/mantis/view.php?id=2376 cleanup (which *does* include the C extension as a requirement?)
> It is unclear why the existence of the compressed code extension should have
> any impact on a calling convention which is about register and stack use.
If the UEFI spec allows the C extension, then OpRom code is going to be built with it. This will mean that the affected EFI drivers will include a mix of 16- and 32-bit instructions, which will result in an illegal instruction exception when executed on chips without support for C. This is why the UEFI spec is upfront about the RISC-V extensions targeted - it's the baseline of compatibility to be used by code that has to run everywhere.
> Do you want to require that all code pointers (e.g. in the system table) should
> point to an address that is a multiple of four?
IMHO that's not necessary but I haven't thought too long about it. Did you have specific reasons in mind? A firmware implementation (the actual platform firmware) should be free to use C or any other extension it needs to meet the product requirements. The restriction applies only on binaries that come from outside (like OpRoms).
Plus adding a restriction on alignment now will cause issues later if RISC-V goes to 48-bit or 64-bit instructions (and one of the reasons why the C extension is being proposed to be made optional are ongoing concerns about running out of the existing 32-bit instruction space).
A
>
> could you, please, provide a link to the proposal that can be viewed by non-
> members of the UEFI working group?
>
Or were you asking about the existing https://mantis.uefi.org/mantis/view.php?id=2376 cleanup (which *does* include the C extension as a requirement?)
> It is unclear why the existence of the compressed code extension should have
> any impact on a calling convention which is about register and stack use.
> Do you want to require that all code pointers (e.g. in the system table) should
> point to an address that is a multiple of four?
Plus adding a restriction on alignment now will cause issues later if RISC-V goes to 48-bit or 64-bit instructions (and one of the reasons why the C extension is being proposed to be made optional are ongoing concerns about running out of the existing 32-bit instruction space).
A
Heinrich Schuchardt
Sep 6, 2023, 4:07:52 PM9/6/23
to Warkentin, Andrei, RISC-V Firmware Exchange
Hello Andrew,
I cannot access the Mantis link. Is there a public document?
Regards
Heinrich
Warkentin, Andrei
Sep 6, 2023, 5:17:39 PM9/6/23
to Heinrich Schuchardt, RISC-V Firmware Exchange
See attached.
The only difference from the word doc is clarifying the text around behavior when hypervisor extension is present (i.e. clarify that the requirement to run in hyp mode doesn't apply if you're running in a VM... kind of obvious, but still). Also, instead of not talking about address translation, allow translation like ia32 (where it is optional, unlike x64 or aarch64), mandating that all memory regions must be identity mapped.
Of course this still includes RVC as a requirement.
Heinrich Schuchardt
Sep 6, 2023, 7:23:41 PM9/6/23
to Warkentin, Andrei, RISC-V Firmware Exchange
Hello Andrej,
thanks for sharing the document.
A major problem I see is that it is unclear who is responsible for the
thread-safety of the runtime:
* Must the operating system guarantee that only one EFI runtime call is
executed at a time?
* Does the EFI runtime have to be thread-safe?
In the Linux code we find down_interruptible(&efi_runtime_lock) which
ensures thread-safety.
For the boot time we have this sentence: "During boot services only a
single processor is used for execution." But I could not find anything
similar for the runtime.
It would be preferable to let the UEFI specification provide clear guidance.
Concerning the C extension:
When compiling U-Boot for minimum size (-Os) I found these file sizes:
u-boot.bin:
* with C extension: 599916
* without C extension: 755084
+ 26 %
u-boot-spl.bin:
* with C extension: 39120
* without C extensions: 51904
+ 33 %
The same order of code size uplift can be seen when compiling for speed
(-O2). I don't assume that code size will be the main driver for a decision.
When we exclude the C extension IALIGN will be 32bit. Even if we only
exclude the C extensions in option ROMS, all function pointers exposed
via the UEFI API still should follow this value of IALIGN to avoid
interoperability issues caused by possible assumptions of the code
compiled without C extension.
It would be easier to follow a specification which either allows the C
extension everywhere or nowhere in the firmware.
The RVS20 and RVS22 profiles require the C-extension. Has work on a
profile without C-extension started or is it imminent?
Best regards
Heinrich
thanks for sharing the document.
A major problem I see is that it is unclear who is responsible for the
thread-safety of the runtime:
* Must the operating system guarantee that only one EFI runtime call is
executed at a time?
* Does the EFI runtime have to be thread-safe?
In the Linux code we find down_interruptible(&efi_runtime_lock) which
ensures thread-safety.
For the boot time we have this sentence: "During boot services only a
single processor is used for execution." But I could not find anything
similar for the runtime.
It would be preferable to let the UEFI specification provide clear guidance.
Concerning the C extension:
When compiling U-Boot for minimum size (-Os) I found these file sizes:
u-boot.bin:
* with C extension: 599916
* without C extension: 755084
+ 26 %
u-boot-spl.bin:
* with C extension: 39120
* without C extensions: 51904
+ 33 %
The same order of code size uplift can be seen when compiling for speed
(-O2). I don't assume that code size will be the main driver for a decision.
When we exclude the C extension IALIGN will be 32bit. Even if we only
exclude the C extensions in option ROMS, all function pointers exposed
via the UEFI API still should follow this value of IALIGN to avoid
interoperability issues caused by possible assumptions of the code
compiled without C extension.
It would be easier to follow a specification which either allows the C
extension everywhere or nowhere in the firmware.
The RVS20 and RVS22 profiles require the C-extension. Has work on a
profile without C-extension started or is it imminent?
Best regards
Heinrich
Warkentin, Andrei
Sep 7, 2023, 12:07:13 AM9/7/23
to Heinrich Schuchardt, RISC-V Firmware Exchange
Hi Heinrich,
With respect to invoking RT services, there's fairly involved logic under 8.1. Runtime Services Rules and Restrictions (https://uefi.org/specs/UEFI/2.10/08_Services_Runtime_Services.html?s#runtime-services-rules-and-restrictions). I'm not necessarily surprised than an OS may choose to simplify those rules to simply avoid any concurrent calls, esp. in the face of implementation bugs (which RT in general is susceptible to, leading to other implementation choices, say around address space or preserving architecture state). So I don't think there's anything else here to do, esp. for RISC-V.
I see your reasoning about IALIGN and I do agree with it. Thanks for taking the time to explain. Figuring out how to enforce specific IALIGN for function pointers for code build with rv64gc...that's an interesting aspect to think about. I need to look into interop between rv64g and rv64gc code...
As far as allowing/disallowing... UEFI currently (with or without the ongoing cleanup) mandates C. If we just stop mandating C, this allows C (or any other extension to be used) in implementations where it makes sense - this would, in theory, allow actual platform firmware to use it (flash is after all a premium). But because C won't be part of the extensions required to exist, OpRoms and OS loaders wouldn't be built against it.
A
> -----Original Message-----
> From: Heinrich Schuchardt <heinrich....@canonical.com>
> Sent: Wednesday, September 6, 2023 6:25 PM
> To: Warkentin, Andrei <andrei.w...@intel.com>
> Cc: RISC-V Firmware Exchange <fw-ex...@riscv.org>
> Subject: Re: Drop C extension from RISC-V calling convention requirements?
>
> --
> You received this message because you are subscribed to the Google Groups
> "RISC-V Firmware Exchange" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to fw-exchange...@riscv.org.
> To view this discussion on the web visit
> https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/5b3da515-dbc9-
> 4ffe-b8b5-2aaa57a0c8d2%40canonical.com.
With respect to invoking RT services, there's fairly involved logic under 8.1. Runtime Services Rules and Restrictions (https://uefi.org/specs/UEFI/2.10/08_Services_Runtime_Services.html?s#runtime-services-rules-and-restrictions). I'm not necessarily surprised than an OS may choose to simplify those rules to simply avoid any concurrent calls, esp. in the face of implementation bugs (which RT in general is susceptible to, leading to other implementation choices, say around address space or preserving architecture state). So I don't think there's anything else here to do, esp. for RISC-V.
I see your reasoning about IALIGN and I do agree with it. Thanks for taking the time to explain. Figuring out how to enforce specific IALIGN for function pointers for code build with rv64gc...that's an interesting aspect to think about. I need to look into interop between rv64g and rv64gc code...
As far as allowing/disallowing... UEFI currently (with or without the ongoing cleanup) mandates C. If we just stop mandating C, this allows C (or any other extension to be used) in implementations where it makes sense - this would, in theory, allow actual platform firmware to use it (flash is after all a premium). But because C won't be part of the extensions required to exist, OpRoms and OS loaders wouldn't be built against it.
A
> -----Original Message-----
> From: Heinrich Schuchardt <heinrich....@canonical.com>
> Sent: Wednesday, September 6, 2023 6:25 PM
> To: Warkentin, Andrei <andrei.w...@intel.com>
> Cc: RISC-V Firmware Exchange <fw-ex...@riscv.org>
> Subject: Re: Drop C extension from RISC-V calling convention requirements?
>
> You received this message because you are subscribed to the Google Groups
> "RISC-V Firmware Exchange" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to fw-exchange...@riscv.org.
> To view this discussion on the web visit
> https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/5b3da515-dbc9-
> 4ffe-b8b5-2aaa57a0c8d2%40canonical.com.
Warkentin, Andrei
Sep 11, 2023, 7:32:30 PM9/11/23
to Warkentin, Andrei, Heinrich Schuchardt, RISC-V Firmware Exchange
So I looked/asked and it seems both GCC and Clang have ways of ensuring alignment - either via #pragma or via a compiler flag (-falign-functions=4). I've yet to test this (particularly in the Clang case, which might have its own funky way via (-mllvm -align-all-function=2).
So we could certainly mandate the alignment to be 32-bit for EFIAPI. Further down the line when/if 48-bit instructions happen this will ensure further sanity...
Anyone think this is a terrible idea? Can someone from the U-boot side give their take - or maybe you know who to ask? Again the intention is to allow platform firmware to be build with C extension, the worry is just around EFIAPI (it should not be visible that the implementation uses C extension).
Can we get a show of hands for the following two options:
-----------------------------------------------------------------------------
Option 1) Sure, let's preemptively remove C as a requirement from UEFI spec, and add a 32-bit alignment requirement to EFIAPI. Even though deprecating C extension is just a possibility, we'd rather not keep perpetually introducing breaking changes to the UEFI spec.
Option 2) We can wait it out and deal with it when/if the C extension becomes optional.
A
> exchange/PH8PR11MB6856A3D237164090925431F783EEA%40PH8PR11MB68
> 56.namprd11.prod.outlook.com.
So we could certainly mandate the alignment to be 32-bit for EFIAPI. Further down the line when/if 48-bit instructions happen this will ensure further sanity...
Anyone think this is a terrible idea? Can someone from the U-boot side give their take - or maybe you know who to ask? Again the intention is to allow platform firmware to be build with C extension, the worry is just around EFIAPI (it should not be visible that the implementation uses C extension).
Can we get a show of hands for the following two options:
-----------------------------------------------------------------------------
Option 1) Sure, let's preemptively remove C as a requirement from UEFI spec, and add a 32-bit alignment requirement to EFIAPI. Even though deprecating C extension is just a possibility, we'd rather not keep perpetually introducing breaking changes to the UEFI spec.
Option 2) We can wait it out and deal with it when/if the C extension becomes optional.
A
> 56.namprd11.prod.outlook.com.
Heinrich Schuchardt
Sep 11, 2023, 11:32:39 PM9/11/23
to Warkentin, Andrei, RISC-V Firmware Exchange
On 9/12/23 01:32, Warkentin, Andrei wrote:
> So I looked/asked and it seems both GCC and Clang have ways of ensuring
> alignment - either via #pragma or via a compiler flag
> (-falign-functions=4). I've yet to test this (particularly in the Clang
> case, which might have its own funky way via (-mllvm
> -align-all-function=2). So we could certainly mandate the alignment to
> be 32-bit for EFIAPI. Further down the line when/if 48-bit instructions
> happen this will ensure further sanity... Anyone think this is a
> terrible idea? Can someone from the U-boot side give their take - or
> maybe you know who to ask? Again the intention is to allow platform
> firmware to be build with C extension, the worry is just around EFIAPI
> (it should not be visible that the implementation uses C extension). Can
> we get a show of hands for the following two options:
> ---------------------------------------------------------------------
> So I looked/asked and it seems both GCC and Clang have ways of ensuring
> alignment - either via #pragma or via a compiler flag
> (-falign-functions=4). I've yet to test this (particularly in the Clang
> case, which might have its own funky way via (-mllvm
> -align-all-function=2). So we could certainly mandate the alignment to
> be 32-bit for EFIAPI. Further down the line when/if 48-bit instructions
> happen this will ensure further sanity... Anyone think this is a
> terrible idea? Can someone from the U-boot side give their take - or
> maybe you know who to ask? Again the intention is to allow platform
> firmware to be build with C extension, the worry is just around EFIAPI
> (it should not be visible that the implementation uses C extension). Can
> we get a show of hands for the following two options:
> ---------------------------------------------------------------------
> Option 1) Sure, let's preemptively remove C as a requirement from UEFI
> spec, and add a 32-bit alignment requirement to EFIAPI. Even though
> deprecating C extension is just a possibility, we'd rather not keep
> perpetually introducing breaking changes to the UEFI spec.
> Option 2) We can wait it out and deal with it when/if the C extension
> becomes optional.
gcc generates 2 aligned function addresses by default. E.g. I found in
> spec, and add a 32-bit alignment requirement to EFIAPI. Even though
> deprecating C extension is just a possibility, we'd rather not keep
> perpetually introducing breaking changes to the UEFI spec.
> Option 2) We can wait it out and deal with it when/if the C extension
> becomes optional.
u-boot.map:
.text.efi_setup_loaded_image
0x000000008023ce7e 0xe2 lib/efi_loader/efi_boottime.o
0x000000008023ce7e efi_setup_loaded_image
-falign-functions changes the alignment of all functions.
Instead we could change the definition of EFIAPI in U-Boot and EDK II
-#define EFIAPI
+#define EFIAPI __attribute__((aligned(4)))
as described in
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-aligned-function-attribute
From the U-Boot side requiring EFIAPI function pointers to be 4 aligned
can easily be followed.
In the EDK II SCT many functions which should be EFIAPI lack that
attribute (https://bugzilla.tianocore.org/show_bug.cgi?id=2126). I don't
know if the same holds true for EDK II itself.
U-Boot provides configuration symbol CONFIG_RISCV_ISA_C which would
allow to disable emission of compressed instructions if not supported by
the CPU.
In my role as U-Boot EFI maintainer both option 1 and option 2 look good
to me. Requiring 4 byte alignment for EFIAPI would be a prudent choice
in both cases.
Best regards
Heinrich
Warkentin, Andrei
Sep 12, 2023, 4:49:11 PM9/12/23
to Heinrich Schuchardt, RISC-V Firmware Exchange
Thanks Heinrich.
Agree with you on the prudence of alignment guarantees by changing the EFIAPI signature. Does anyone have an objection to that? I think I'll raise a separate ECR since this isn't a "clarification" per se but an impactful change (in the sense that anyone building bootloader code will have to deal with it, or risk running into potential issues later).
If we have agreement on that front, making C optional is very easy - practically free.
A
> -----Original Message-----
> From: Heinrich Schuchardt <heinrich....@canonical.com>
> Sent: Monday, September 11, 2023 10:34 PM
> To: Warkentin, Andrei <andrei.w...@intel.com>
> Cc: RISC-V Firmware Exchange <fw-ex...@riscv.org>
> Subject: Re: Drop C extension from RISC-V calling convention requirements?
>
> 4715-afae-75b1fdd9be1e%40canonical.com.
Agree with you on the prudence of alignment guarantees by changing the EFIAPI signature. Does anyone have an objection to that? I think I'll raise a separate ECR since this isn't a "clarification" per se but an impactful change (in the sense that anyone building bootloader code will have to deal with it, or risk running into potential issues later).
If we have agreement on that front, making C optional is very easy - practically free.
A
> -----Original Message-----
> From: Heinrich Schuchardt <heinrich....@canonical.com>
> Sent: Monday, September 11, 2023 10:34 PM
> To: Warkentin, Andrei <andrei.w...@intel.com>
> Cc: RISC-V Firmware Exchange <fw-ex...@riscv.org>
> Subject: Re: Drop C extension from RISC-V calling convention requirements?
>
> --
> You received this message because you are subscribed to the Google Groups
> "RISC-V Firmware Exchange" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to fw-exchange...@riscv.org.
> To view this discussion on the web visit
> https://groups.google.com/a/riscv.org/d/msgid/fw-exchange/f38c5d15-10a0-
> You received this message because you are subscribed to the Google Groups
> "RISC-V Firmware Exchange" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to fw-exchange...@riscv.org.
> To view this discussion on the web visit
> 4715-afae-75b1fdd9be1e%40canonical.com.
Reply all
Reply to author
Forward
0 new messages