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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxx>
> >>
> >> ---
> >>      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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel