> On 21 Sep 2020, at 10:23, Jan Kiszka <jan.kis...@siemens.com> wrote:
>
> On 17.09.20 20:14, Oliver Schwartz wrote:
>>> On 17 Sep 2020, at 18:21, Ralf Ramsauer <ralf.ramsa...@oth-regensburg.de
>>> <mailto:ralf.ramsa...@oth-regensburg.de>> wrote:
>>>
>>>
>>> On 9/17/20 8:26 AM, Jan Kiszka wrote:
>>>> Subject tag should be "arm64". And the patch should go over next first.
>>>> I can fix up both.
>>>>
>>>> On 16.09.20 15:07, Oliver Schwartz wrote:
>>>>> SMC/HVC 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
>>>>> <mailto:oliver.schwa...@gmx.de>>
>>>>> ---
>>>>> hypervisor/arch/arm64/include/asm/smc.h | 13 ++++++-------
>>>>> include/arch/arm64/asm/jailhouse_hypercall.h | 20 +++++++++++---------
>>>>> 2 files changed, 17 insertions(+), 16 deletions(-)
>>>>>
>>>>> diff --git a/hypervisor/arch/arm64/include/asm/smc.h
>>>>> b/hypervisor/arch/arm64/include/asm/smc.h
>>>>> index 1a5d5c8..c80fe15 100644
>>>>> --- a/hypervisor/arch/arm64/include/asm/smc.h
>>>>> +++ b/hypervisor/arch/arm64/include/asm/smc.h
>>>>> @@ -28,8 +28,8 @@ 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");
>>>>>
>>>>
>>>> For SMCCC, I'm considering to align fully with Linux, i.e. convert the
>>>> remaining register clobberings into early ones, but I also have no
>>>> strong argument for it.
>>>>
>>>> Ralf, thoughts?
>>>
>>> Just had a look at Linux's implementation. And now I recall why I didn't
>>> c&p it from Linux: it's an unreadable macro hell [1] that would benefit
>>> from being open-coded. But trying to follow Linux's __constraint macro,
>>> shouldn't we protect r2 and r3 as well?
>>>
>>> Linux would unroll __constraint_read_1 and __constraint_write_1, and
>>> __constraint_read_1 would protect r2 and r3 as well. We protect x2 and
>>> x3 via the clobber list -- is that enough?
>> I’m by no means an expert on arm assembler. However, I’m quite positive that
>> there are no physical registers r2 and r3 that can be protected in arm64.
>> These are just placeholders that are translated by the compiler to x2 and
>> x3. So protecting r2/r3 as dummy arguments or x2/x3 via clobber list is in
>> effect the same, just more explicit.
>
> I've merged both patches to next now using your pattern. I applied some
> massaging though. Please have a look at the result.
Definitely looks good to me.
Oliver
> Some consolidation patch for the hypercalls will also follow.
--
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/525A4B24-2A65-4671-B45F-35F146C7C520%40gmx.de.
