[v1] xen/arm: XSA-201 and XSA-263 fixes

[Xen-devel] [PATCH RFC for-4.13 09/10] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

Posted by Julien Grall 6 years, 1 month ago

Using alternative_if makes the code a bit more streamlined.

Take the opportunity to use the new auto-nop infrastructure to avoid
counting the number of nop in the else part for arch/arm/arm64/entry.S

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    This is pretty much a matter of taste, but at least for arm64 this
    allows us to use the auto-nop infrastructure. So the arm32 is more
    to keep inline with arm64.
---
 xen/arch/arm/arm32/entry.S | 9 ++++++---
 xen/arch/arm/arm64/entry.S | 8 +++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 0b4cd19abd..1428cd3583 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -65,9 +65,12 @@ save_guest_regs:
          * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
          * feature, the checking of pending SErrors will be skipped.
          */
-        ALTERNATIVE("nop",
-                    "b skip_check",
-                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+        nop
+        alternative_else
+        b   skip_check
+        alternative_endif
+
         /*
          * Start to check pending virtual abort in the gap of Guest -> HYP
          * world switch.
diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
index 458d12f188..91cf6ee6f4 100644
--- a/xen/arch/arm/arm64/entry.S
+++ b/xen/arch/arm/arm64/entry.S
@@ -170,9 +170,11 @@
          * is not set. If a vSError took place, the initial exception will be
          * skipped. Exit ASAP
          */
-        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
-                    "nop; nop",
-                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
+        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
+        bl      check_pending_vserror
+        cbnz    x0, 1f
+        alternative_else_nop_endif
+
         mov     x0, sp
         bl      enter_hypervisor_from_guest_noirq
         msr     daifclr, \iflags
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC for-4.13 09/10] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

Posted by Stefano Stabellini 6 years, 1 month ago

On Thu, 26 Sep 2019, Julien Grall wrote:
> Using alternative_if makes the code a bit more streamlined.
> 
> Take the opportunity to use the new auto-nop infrastructure to avoid
> counting the number of nop in the else part for arch/arm/arm64/entry.S
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> ---
>     This is pretty much a matter of taste, but at least for arm64 this
>     allows us to use the auto-nop infrastructure. So the arm32 is more
>     to keep inline with arm64.
> ---
>  xen/arch/arm/arm32/entry.S | 9 ++++++---
>  xen/arch/arm/arm64/entry.S | 8 +++++---
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> index 0b4cd19abd..1428cd3583 100644
> --- a/xen/arch/arm/arm32/entry.S
> +++ b/xen/arch/arm/arm32/entry.S
> @@ -65,9 +65,12 @@ save_guest_regs:
>           * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
>           * feature, the checking of pending SErrors will be skipped.
>           */
> -        ALTERNATIVE("nop",
> -                    "b skip_check",
> -                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> +        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> +        nop
> +        alternative_else
> +        b   skip_check
> +        alternative_endif

This could be written as:

alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
b   skip_check
alternative_else_nop_endif


>          /*
>           * Start to check pending virtual abort in the gap of Guest -> HYP
>           * world switch.
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 458d12f188..91cf6ee6f4 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -170,9 +170,11 @@
>           * is not set. If a vSError took place, the initial exception will be
>           * skipped. Exit ASAP
>           */
> -        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
> -                    "nop; nop",
> -                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> +        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> +        bl      check_pending_vserror
> +        cbnz    x0, 1f
> +        alternative_else_nop_endif
> +
>          mov     x0, sp
>          bl      enter_hypervisor_from_guest_noirq
>          msr     daifclr, \iflags
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC for-4.13 09/10] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

Posted by Julien Grall 6 years, 1 month ago

Hi Stefano,

On 01/10/2019 23:19, Stefano Stabellini wrote:
> On Thu, 26 Sep 2019, Julien Grall wrote:
>> Using alternative_if makes the code a bit more streamlined.
>>
>> Take the opportunity to use the new auto-nop infrastructure to avoid
>> counting the number of nop in the else part for arch/arm/arm64/entry.S
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> ---
>>      This is pretty much a matter of taste, but at least for arm64 this
>>      allows us to use the auto-nop infrastructure. So the arm32 is more
>>      to keep inline with arm64.
>> ---
>>   xen/arch/arm/arm32/entry.S | 9 ++++++---
>>   xen/arch/arm/arm64/entry.S | 8 +++++---
>>   2 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
>> index 0b4cd19abd..1428cd3583 100644
>> --- a/xen/arch/arm/arm32/entry.S
>> +++ b/xen/arch/arm/arm32/entry.S
>> @@ -65,9 +65,12 @@ save_guest_regs:
>>            * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
>>            * feature, the checking of pending SErrors will be skipped.
>>            */
>> -        ALTERNATIVE("nop",
>> -                    "b skip_check",
>> -                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>> +        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
>> +        nop
>> +        alternative_else
>> +        b   skip_check
>> +        alternative_endif
> 
> This could be written as:
> 
> alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> b   skip_check
> alternative_else_nop_endif

Actually my implementation is wrong :/. We want to skip the check if the 
cap is set. So this should be:

alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
b    skip_check
alternative_else_nop_endif

> 
> 
>>           /*
>>            * Start to check pending virtual abort in the gap of Guest -> HYP
>>            * world switch.
>> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
>> index 458d12f188..91cf6ee6f4 100644
>> --- a/xen/arch/arm/arm64/entry.S
>> +++ b/xen/arch/arm/arm64/entry.S
>> @@ -170,9 +170,11 @@
>>            * is not set. If a vSError took place, the initial exception will be
>>            * skipped. Exit ASAP
>>            */
>> -        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>> -                    "nop; nop",
>> -                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
>> +        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT

This would need to be alternative_if_not as want to call the function 
when the cap is not set.

>> +        bl      check_pending_vserror
>> +        cbnz    x0, 1f
>> +        alternative_else_nop_endif
>> +
>>           mov     x0, sp
>>           bl      enter_hypervisor_from_guest_noirq
>>           msr     daifclr, \iflags
>> -- 
>> 2.11.0
>>

Cheeers,

-- 
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH RFC for-4.13 09/10] xen/arm: asm: Replace use of ALTERNATIVE with alternative_if

Posted by Stefano Stabellini 6 years, 1 month ago

On Tue, 1 Oct 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 01/10/2019 23:19, Stefano Stabellini wrote:
> > On Thu, 26 Sep 2019, Julien Grall wrote:
> >> Using alternative_if makes the code a bit more streamlined.
> >>
> >> Take the opportunity to use the new auto-nop infrastructure to avoid
> >> counting the number of nop in the else part for arch/arm/arm64/entry.S
> >>
> >> Signed-off-by: Julien Grall <julien.grall@arm.com>
> >>
> >> ---
> >>      This is pretty much a matter of taste, but at least for arm64 this
> >>      allows us to use the auto-nop infrastructure. So the arm32 is more
> >>      to keep inline with arm64.
> >> ---
> >>   xen/arch/arm/arm32/entry.S | 9 ++++++---
> >>   xen/arch/arm/arm64/entry.S | 8 +++++---
> >>   2 files changed, 11 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> >> index 0b4cd19abd..1428cd3583 100644
> >> --- a/xen/arch/arm/arm32/entry.S
> >> +++ b/xen/arch/arm/arm32/entry.S
> >> @@ -65,9 +65,12 @@ save_guest_regs:
> >>            * If the SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT has been set in the cpu
> >>            * feature, the checking of pending SErrors will be skipped.
> >>            */
> >> -        ALTERNATIVE("nop",
> >> -                    "b skip_check",
> >> -                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> >> +        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> >> +        nop
> >> +        alternative_else
> >> +        b   skip_check
> >> +        alternative_endif
> > 
> > This could be written as:
> > 
> > alternative_if_not SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> > b   skip_check
> > alternative_else_nop_endif
> 
> Actually my implementation is wrong :/. We want to skip the check if the 
> cap is set. So this should be:
> 
> alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> b    skip_check
> alternative_else_nop_endif

Ah, yes of course, like the name suggests


> > 
> >>           /*
> >>            * Start to check pending virtual abort in the gap of Guest -> HYP
> >>            * world switch.
> >> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> >> index 458d12f188..91cf6ee6f4 100644
> >> --- a/xen/arch/arm/arm64/entry.S
> >> +++ b/xen/arch/arm/arm64/entry.S
> >> @@ -170,9 +170,11 @@
> >>            * is not set. If a vSError took place, the initial exception will be
> >>            * skipped. Exit ASAP
> >>            */
> >> -        ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
> >> -                    "nop; nop",
> >> -                    SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> >> +        alternative_if SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT
> 
> This would need to be alternative_if_not as want to call the function 
> when the cap is not set.

Yep


> >> +        bl      check_pending_vserror
> >> +        cbnz    x0, 1f
> >> +        alternative_else_nop_endif
> >> +
> >>           mov     x0, sp
> >>           bl      enter_hypervisor_from_guest_noirq
> >>           msr     daifclr, \iflags
> >> -- 
> >> 2.11.0
> >>
> 
> Cheeers,
> 
> -- 
> Julien Grall
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel