On 16.09.20 11:19, Oliver Schwartz wrote:
On 16 Sep 2020, at 10:28, Jan Kiszka <jan.kis...@siemens.com> wrote:
On 16.09.20 09:12, Oliver Schwartz wrote:
SMC calls may modify registers x0 to x3. To make sure the compiler doesn't
assume input registers to be constant, also mark these registers as output
when used as input.
Signed-off-by: Oliver Schwartz <oliver.schwa...@gmx.de>
---
hypervisor/arch/arm64/include/asm/smc.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hypervisor/arch/arm64/include/asm/smc.h
b/hypervisor/arch/arm64/include/asm/smc.h
index 1a5d5c8..e7b6723 100644
--- a/hypervisor/arch/arm64/include/asm/smc.h
+++ b/hypervisor/arch/arm64/include/asm/smc.h
@@ -28,7 +28,7 @@ static inline long smc_arg1(unsigned long id, unsigned long
par1)
register unsigned long __par1 asm("r1") = par1;
asm volatile ("smc #0\n\t"
- : "=r" (__id)
+ : "=r" (__id), "=r"(__par1)
: "r"(__id), "r"(__par1)
: "memory", "x2", "x3");
@@ -43,7 +43,7 @@ static inline long smc_arg2(unsigned long id, unsigned long
par1,
register unsigned long __par2 asm("r2") = par2;
asm volatile ("smc #0\n\t"
- : "=r" (__id)
+ : "=r" (__id), "=r"(__par1), "=r"(__par2)
: "r"(__id), "r"(__par1), "r"(__par2)
: "memory", "x3");
@@ -62,7 +62,7 @@ static inline long smc_arg5(unsigned long id, unsigned long
par1,
register unsigned long __par5 asm("r5") = par5;
asm volatile ("smc #0\n\t"
- : "=r" (__id)
+ : "=r" (__id), "=r"(__par1), "=r"(__par2), "=r"(__par3)
: "r"(__id), "r"(__par1), "r"(__par2), "r"(__par3),
"r"(__par4), "r"(__par5)
: "memory");
Good catch! We likely have the same issue with our hypercall interface
(jailhouse_hypercall.h).
We should probably look carefully again at how Linux expresses these constraints:
linux/include/linux/arm-smccc.h. That appears to me like we need "+r" for input/output
registers and "=&r" for those that are just input but might be clobbered on return.
I must admit that I don’t fully understand the “Constraint Modifier Characters”
chapter in the gcc documentation. Please feel free to modify the patch as
needed.
I had to read it 3 times as well, but I think I'm starting to
understand. "+" vs. "=" is simple, I guess. The relevance of adding "&"
to "=" may be clearer from this scenario:
Consider you are passing "+"(p1) and "="(p2) to an assembly block. If p2
is only written after p1 was evaluated, the compiler can safely map both
to the same register. If p1 is only read after p2 was written (early
clobber...), that would obviously break - hence that "&" tagging.
What I do not understand yet is how our register assignment hints play
into this which map parameters to different registers. Maybe they just
create an internal conflict for the compiler and could in some cases
cause build errors. See also 2b9a200d6dba.
The 32 bit implementation for smc and hypercall also need a fix.
Right. Basically, we need to mark all callee-saved registers in our
interfaces as overwritten or, if not input as well, also early
clobbered. If you have the time to write such a patch, that would be great.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to jailhouse-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/9eef795b-8ecd-49de-3e97-1535003ba499%40siemens.com.
