diff mbox

[v2,11/19] xen/arm32: Use alternative to skip the check of pending serrors

Message ID 1490865209-18283-12-git-send-email-Wei.Chen@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Chen March 30, 2017, 9:13 a.m. UTC
We have provided an option to administrator to determine how to
handle the SErrors. In order to skip the check of pending SError,
in conventional way, we have to read the option every time before
we try to check the pending SError. This will add overhead to check
the option at every trap.

The ARM32 supports the alternative patching feature. We can use an
ALTERNATIVE to avoid checking option at every trap. We added a new
cpufeature named "SKIP_CHECK_PENDING_VSERROR". This feature will be
enabled when the option is not diverse.

Signed-off-by: Wei Chen <Wei.Chen@arm.com>
---
 xen/arch/arm/arm32/entry.S | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Julien Grall March 30, 2017, 6:06 p.m. UTC | #1
Hi Wei,

On 30/03/17 10:13, Wei Chen wrote:
> We have provided an option to administrator to determine how to
> handle the SErrors. In order to skip the check of pending SError,
> in conventional way, we have to read the option every time before
> we try to check the pending SError. This will add overhead to check
> the option at every trap.
>
> The ARM32 supports the alternative patching feature. We can use an
> ALTERNATIVE to avoid checking option at every trap. We added a new
> cpufeature named "SKIP_CHECK_PENDING_VSERROR". This feature will be
> enabled when the option is not diverse.
>
> Signed-off-by: Wei Chen <Wei.Chen@arm.com>

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

> ---

Stefano, this is requiring the alternative patch (see [1]) which is 
still in review.

Wei, a general rule is to mention the dependencies between series. So we 
don't apply by mistake in the wrong order.

Cheers,

[1] 
https://lists.xenproject.org/archives/html/xen-devel/2017-03/msg04132.html

>  xen/arch/arm/arm32/entry.S | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> index 2187226..105cae8 100644
> --- a/xen/arch/arm/arm32/entry.S
> +++ b/xen/arch/arm/arm32/entry.S
> @@ -1,5 +1,6 @@
>  #include <asm/asm_defns.h>
>  #include <asm/regs.h>
> +#include <asm/alternative.h>
>  #include <public/xen.h>
>
>  #define SAVE_ONE_BANKED(reg)    mrs r11, reg; str r11, [sp, #UREGS_##reg]
> @@ -44,6 +45,14 @@ save_guest_regs:
>          SAVE_BANKED(fiq)
>          SAVE_ONE_BANKED(R8_fiq); SAVE_ONE_BANKED(R9_fiq); SAVE_ONE_BANKED(R10_fiq)
>          SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
> +
> +        /*
> +         * If the SKIP_CHECK_PENDING_VSERROR has been set in the cpu feature,
> +         * the checking of pending SErrors will be skipped.
> +         */
> +        ALTERNATIVE("nop",
> +                    "b skip_check",
> +                    SKIP_CHECK_PENDING_VSERROR)
>          /*
>           * Start to check pending virtual abort in the gap of Guest -> HYP
>           * world switch.
> @@ -99,6 +108,7 @@ abort_guest_exit_end:
>           */
>          bne return_from_trap
>
> +skip_check:
>          mov pc, lr
>
>  #define DEFINE_TRAP_ENTRY(trap)                                         \
>
Stefano Stabellini March 30, 2017, 9:29 p.m. UTC | #2
On Thu, 30 Mar 2017, Julien Grall wrote:
> Hi Wei,
> 
> On 30/03/17 10:13, Wei Chen wrote:
> > We have provided an option to administrator to determine how to
> > handle the SErrors. In order to skip the check of pending SError,
> > in conventional way, we have to read the option every time before
> > we try to check the pending SError. This will add overhead to check
> > the option at every trap.
> > 
> > The ARM32 supports the alternative patching feature. We can use an
> > ALTERNATIVE to avoid checking option at every trap. We added a new
> > cpufeature named "SKIP_CHECK_PENDING_VSERROR". This feature will be
> > enabled when the option is not diverse.
> > 
> > Signed-off-by: Wei Chen <Wei.Chen@arm.com>
> 
> Reviewed-by: Julien Grall <julien.grall@arm.com>
> 
> > ---
> 
> Stefano, this is requiring the alternative patch (see [1]) which is still in
> review.
> 
> Wei, a general rule is to mention the dependencies between series. So we don't
> apply by mistake in the wrong order.

Right. Thanks Wei for sending both the other patch and this series
together, that helps me avoiding mistakes.

But please add a note about the dependencies, just in case I forget.


> Cheers,
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2017-03/msg04132.html
> 
> >  xen/arch/arm/arm32/entry.S | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
> > index 2187226..105cae8 100644
> > --- a/xen/arch/arm/arm32/entry.S
> > +++ b/xen/arch/arm/arm32/entry.S
> > @@ -1,5 +1,6 @@
> >  #include <asm/asm_defns.h>
> >  #include <asm/regs.h>
> > +#include <asm/alternative.h>
> >  #include <public/xen.h>
> > 
> >  #define SAVE_ONE_BANKED(reg)    mrs r11, reg; str r11, [sp, #UREGS_##reg]
> > @@ -44,6 +45,14 @@ save_guest_regs:
> >          SAVE_BANKED(fiq)
> >          SAVE_ONE_BANKED(R8_fiq); SAVE_ONE_BANKED(R9_fiq);
> > SAVE_ONE_BANKED(R10_fiq)
> >          SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
> > +
> > +        /*
> > +         * If the SKIP_CHECK_PENDING_VSERROR has been set in the cpu
> > feature,
> > +         * the checking of pending SErrors will be skipped.
> > +         */
> > +        ALTERNATIVE("nop",
> > +                    "b skip_check",
> > +                    SKIP_CHECK_PENDING_VSERROR)
> >          /*
> >           * Start to check pending virtual abort in the gap of Guest -> HYP
> >           * world switch.
> > @@ -99,6 +108,7 @@ abort_guest_exit_end:
> >           */
> >          bne return_from_trap
> > 
> > +skip_check:
> >          mov pc, lr
> > 
> >  #define DEFINE_TRAP_ENTRY(trap)                                         \
> > 
> 
> -- 
> Julien Grall
>
Wei Chen March 31, 2017, 5:33 a.m. UTC | #3
On 2017/3/31 5:29, Stefano Stabellini wrote:
> On Thu, 30 Mar 2017, Julien Grall wrote:
>> Hi Wei,
>>
>> On 30/03/17 10:13, Wei Chen wrote:
>>> We have provided an option to administrator to determine how to
>>> handle the SErrors. In order to skip the check of pending SError,
>>> in conventional way, we have to read the option every time before
>>> we try to check the pending SError. This will add overhead to check
>>> the option at every trap.
>>>
>>> The ARM32 supports the alternative patching feature. We can use an
>>> ALTERNATIVE to avoid checking option at every trap. We added a new
>>> cpufeature named "SKIP_CHECK_PENDING_VSERROR". This feature will be
>>> enabled when the option is not diverse.
>>>
>>> Signed-off-by: Wei Chen <Wei.Chen@arm.com>
>>
>> Reviewed-by: Julien Grall <julien.grall@arm.com>
>>
>>> ---
>>
>> Stefano, this is requiring the alternative patch (see [1]) which is still in
>> review.
>>
>> Wei, a general rule is to mention the dependencies between series. So we don't
>> apply by mistake in the wrong order.
>
> Right. Thanks Wei for sending both the other patch and this series
> together, that helps me avoiding mistakes.
>
> But please add a note about the dependencies, just in case I forget.
>

Stefano and Julien, thank you. I would place a note about the
dependencies in next version.

>
>> Cheers,
>>
>> [1] https://lists.xenproject.org/archives/html/xen-devel/2017-03/msg04132.html
>>
>>>  xen/arch/arm/arm32/entry.S | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
>>> index 2187226..105cae8 100644
>>> --- a/xen/arch/arm/arm32/entry.S
>>> +++ b/xen/arch/arm/arm32/entry.S
>>> @@ -1,5 +1,6 @@
>>>  #include <asm/asm_defns.h>
>>>  #include <asm/regs.h>
>>> +#include <asm/alternative.h>
>>>  #include <public/xen.h>
>>>
>>>  #define SAVE_ONE_BANKED(reg)    mrs r11, reg; str r11, [sp, #UREGS_##reg]
>>> @@ -44,6 +45,14 @@ save_guest_regs:
>>>          SAVE_BANKED(fiq)
>>>          SAVE_ONE_BANKED(R8_fiq); SAVE_ONE_BANKED(R9_fiq);
>>> SAVE_ONE_BANKED(R10_fiq)
>>>          SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
>>> +
>>> +        /*
>>> +         * If the SKIP_CHECK_PENDING_VSERROR has been set in the cpu
>>> feature,
>>> +         * the checking of pending SErrors will be skipped.
>>> +         */
>>> +        ALTERNATIVE("nop",
>>> +                    "b skip_check",
>>> +                    SKIP_CHECK_PENDING_VSERROR)
>>>          /*
>>>           * Start to check pending virtual abort in the gap of Guest -> HYP
>>>           * world switch.
>>> @@ -99,6 +108,7 @@ abort_guest_exit_end:
>>>           */
>>>          bne return_from_trap
>>>
>>> +skip_check:
>>>          mov pc, lr
>>>
>>>  #define DEFINE_TRAP_ENTRY(trap)                                         \
>>>
>>
>> --
>> Julien Grall
>>
>
diff mbox

Patch

diff --git a/xen/arch/arm/arm32/entry.S b/xen/arch/arm/arm32/entry.S
index 2187226..105cae8 100644
--- a/xen/arch/arm/arm32/entry.S
+++ b/xen/arch/arm/arm32/entry.S
@@ -1,5 +1,6 @@ 
 #include <asm/asm_defns.h>
 #include <asm/regs.h>
+#include <asm/alternative.h>
 #include <public/xen.h>
 
 #define SAVE_ONE_BANKED(reg)    mrs r11, reg; str r11, [sp, #UREGS_##reg]
@@ -44,6 +45,14 @@  save_guest_regs:
         SAVE_BANKED(fiq)
         SAVE_ONE_BANKED(R8_fiq); SAVE_ONE_BANKED(R9_fiq); SAVE_ONE_BANKED(R10_fiq)
         SAVE_ONE_BANKED(R11_fiq); SAVE_ONE_BANKED(R12_fiq);
+
+        /*
+         * If the SKIP_CHECK_PENDING_VSERROR has been set in the cpu feature,
+         * the checking of pending SErrors will be skipped.
+         */
+        ALTERNATIVE("nop",
+                    "b skip_check",
+                    SKIP_CHECK_PENDING_VSERROR)
         /*
          * Start to check pending virtual abort in the gap of Guest -> HYP
          * world switch.
@@ -99,6 +108,7 @@  abort_guest_exit_end:
          */
         bne return_from_trap
 
+skip_check:
         mov pc, lr
 
 #define DEFINE_TRAP_ENTRY(trap)                                         \