On 10/04/2024 21:16, Beata Michalska wrote:
On Fri, Mar 01, 2024 at 11:22:51AM +0100, Kevin Brodsky wrote:
On 26/02/2024 09:20, Beata Michalska wrote:
Provide capability-aware set of helper macros for system register read/writes. For the time being, if the transfer register is not a capability one, a prep step is being carried out to provide it through the use of CVTP instruction. In a long run this should be replace by proper setup where a source capability is derived from an appropriate authorizing capability.
Signed-off-by: Beata Michalska beata.michalska@arm.com
arch/arm64/include/asm/sysreg.h | 55 +++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index e2109e2c0bef..c55c4c4fa945 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -1141,6 +1141,42 @@ : : "rZ" (__val)); \ } while (0) +#ifdef CONFIG_ARM64_MORELLO +#define __convert_to_cap(v) \ +({ \
uintcap_t __cval; \asm volatile("cvtp %0, %1" : "=r" (__cval) : "r" (v)); \__cval; \+})
+#define write_cap_sysreg(v, r) \
- do { \
uintcap_t __cval; \__cval = __builtin_choose_expr( \__builtin_types_compatible_p(typeof(v), \uintcap_t), \(uintcap_t)(v), \__convert_to_cap(v) \); \asm volatile("msr " __stringify(r) ", %x0" : : "rZ" (__cval)); \- } while (0)
+/*
- Select appropriate variant of write_sysreg based on whether
- CONFIG_ARM64_MORELLO is enabled or not.
- For the capability system registers, unless capability is being provided,
- one is created through CVTP instruction. This immplies that this macro
- is not suitable for all use-cases. Additionally, this macro is based on
- the assumption that the capability system register name can be derived
- from corresponding aarch64 one.
- */
+#define write_sysreg_variant(v, r) \
- IS_BUILTIN(CONFIG_ARM64_MORELLO) \
? ({ write_cap_sysreg(v, c##r); }) \: ({ write_sysreg(v, r); })At first I wasn't convinced about having such a generic macro, but I do see the point in subsequent patches - it avoids quite a bit of #ifdef'ing. That said, I would prefer things to be more explicit. I don't see a need for write_cap_sysreg() accepting both 64-bit and capability values. I think we have two situations:
- We've got a 64-bit code address and we want to create a valid
capability to write into a capability register on Morello (and just the 64-bit address otherwise). That seems to be relevant to VBAR/ELR. 2. We're explicitly manipulating a capability and writing it into a capability register.
For the first situation, something like this macro is useful, but I would always have it use CVTP to create a valid code capability (on Morello). Additionally, I would name it accordingly:
Did I get it right that you would prefer to drop checking the type of the argument and make it always u64 being converted to cap even if a valid one has been provided?
Yes. AFAICT we never provide a capability to it at the moment. If we also have such situation, we could have a separate helper (see paragraph below).
Kevin
write_sysreg_code_addr() or similar. This is because using CVTP is not
Fair point.
appropriate for creating data capability, and we shouldn't suggest that this macro is more generic than it really is. Overall my thinking is: if a capability is going to be created, that needs to be reflected in the helper's name, and we don't want to have that happen by accident (e.g. by passing the wrong type to the helper).
If in the future we move to saving/restoring the full capabilities for VBAR/ELR, then we can introduce a new macro, maybe write_sysreg_ptr(), that would always take a capability on Morello and a 64-bit address otherwise.
+#else +#define write_sysreg_variant write_sysreg +#endif /*
- For registers without architectural names, or simply unsupported by
- GAS.
@@ -1162,6 +1198,25 @@ asm volatile(__msr_s(r, "%x0") : : "rZ" (__val)); \ } while (0) +#define read_cap_sysreg_s(r) ({ \
- uintcap_t __val; \
- u32 __maybe_unused __check_r = (u32)(r); \
- asm volatile(__mrs_s_c("%0", r) : "=r" (__val)); \
- __val; \
+})
+#define write_cap_sysreg_s(v, r) do { \
- uintcap_t __cval; \
- u32 __maybe_unused __check_r = (u32)(r); \
- __cval = __builtin_choose_expr( \
__builtin_types_compatible_p(typeof(v), \uintcap_t), \(uintcap_t)(v), \__convert_to_cap(v) \); \- asm volatile(__msr_s_c(r, "%x0") : : "rZ" (__cval)); \
+} while (0)
Same idea here: write_cap_sysreg_s() should do exactly what it says (write a capability). We can have a separate helper that (always) creates a capability with CVTP.
Noted.
BR Beata
Kevin
/*
- Modify bits in a sysreg. Bits in the clear mask are zeroed, then bits in the
- set mask are set. Other bits are left as-is.