> On 16 Sep 2020, at 11:54, Jan Kiszka <[email protected]> wrote:
>
> On 16.09.20 11:19, Oliver Schwartz wrote:
>>> On 16 Sep 2020, at 10:28, Jan Kiszka <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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.
Linux uses the “=&r” values for registers r0 to r3 if they are _not_ inputs but
might be clobbered.
>> 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.
Yes, ok. What I don’t understand is why for the SMC calls the “=&r” syntax
should have any advantage over mentioning the register in the clobber list.
Doesn’t this have the same effect? Or is this just used in Linux to make the
macro expansion easier?
> 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.
This is described in chapter "Specifying Registers for Local Variables” and as
far as I understand it takes precedence for assigning arguments to registers.
> 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.
I can prepare a patch for the 32 bit files as well, but I have no means to test
them.
Oliver
--
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 [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/EF89FA0E-53D2-41E3-977A-43F502410816%40gmx.de.
