Re: [Xen-devel] [PATCH 1/4] xen/arm: gic: Ensure we have an ISB between ack and do_IRQ()

[Andrii Anisov <andrii.anisov@xxxxxxxxx>]

Julien,


It's me again about your patch:)

I've found this patch useful and even can give a motivation to have it 
in the mainline. The patch ensures that vgic_sync_from_lrs is performed 
on guest to hyp switch prior to any IRQ processing.
So, do you plan to push it for review? Could I do that on behalf of you?


On 09.11.18 16:42, Andrii Anisov wrote:
> Hello Julien,
>
> I just wonder, do you plan to upstream the patch below?
>
> Andrii Anisov
>
>
>
> commit 11e360b93be81a58a41832d714f33f797ad312a9
> Author: Julien Grall <julien.grall@xxxxxxx>
> Date:   Mon Oct 29 13:32:56 2018 +0000
>
>       xen/arm: Re-enable interrupt later in the trap path
>
>       Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
>
> diff --git a/xen/arch/arm/arm64/entry.S b/xen/arch/arm/arm64/entry.S
> index 97b05f53ea..8f287891b6 100644
> --- a/xen/arch/arm/arm64/entry.S
> +++ b/xen/arch/arm/arm64/entry.S
> @@ -195,7 +195,6 @@ hyp_error_invalid:
>
>    hyp_error:
>            entry   hyp=1
> -        msr     daifclr, #2
>            mov     x0, sp
>            bl      do_trap_hyp_serror
>            exit    hyp=1
> @@ -203,7 +202,7 @@ hyp_error:
>    /* Traps taken in Current EL with SP_ELx */
>    hyp_sync:
>            entry   hyp=1
> -        msr     daifclr, #6
> +        msr     daifclr, #4
>            mov     x0, sp
>            bl      do_trap_hyp_sync
>            exit    hyp=1
> @@ -304,7 +303,7 @@ guest_sync_slowpath:
>            ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>                        "nop; nop",
>                        SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> -        msr     daifclr, #6
> +        msr     daifclr, #4
>            mov     x0, sp
>            bl      do_trap_guest_sync
>    1:
> @@ -332,7 +331,7 @@ guest_fiq_invalid:
>
>    guest_error:
>            entry   hyp=0, compat=0
> -        msr     daifclr, #6
> +        msr     daifclr, #4
>            mov     x0, sp
>            bl      do_trap_guest_serror
>            exit    hyp=0, compat=0
> @@ -347,7 +346,7 @@ guest_sync_compat:
>            ALTERNATIVE("bl check_pending_vserror; cbnz x0, 1f",
>                        "nop; nop",
>                        SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT)
> -        msr     daifclr, #6
> +        msr     daifclr, #4
>            mov     x0, sp
>            bl      do_trap_guest_sync
>    1:
> @@ -375,7 +374,7 @@ guest_fiq_invalid_compat:
>
>    guest_error_compat:
>            entry   hyp=0, compat=1
> -        msr     daifclr, #6
> +        msr     daifclr, #4
>            mov     x0, sp
>            bl      do_trap_guest_serror
>            exit    hyp=0, compat=1
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 51d2e42c77..c18f89b41f 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2039,6 +2039,8 @@ static void enter_hypervisor_head(struct cpu_user_regs 
> *regs)
>        {
>            struct vcpu *v = current;
>
> +        ASSERT(!local_irq_is_enabled());
> +
>            /* If the guest has disabled the workaround, bring it back on. */
>            if ( needs_ssbd_flip(v) )
>                arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL);
> @@ -2073,6 +2075,7 @@ void do_trap_guest_sync(struct cpu_user_regs *regs)
>        const union hsr hsr = { .bits = regs->hsr };
>
>        enter_hypervisor_head(regs);
> +    local_irq_enable();
>
>        switch ( hsr.ec )
>        {
> @@ -2208,6 +2211,7 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
>        const union hsr hsr = { .bits = regs->hsr };
>
>        enter_hypervisor_head(regs);
> +    local_irq_enable();
>
>        switch ( hsr.ec )
>        {
> @@ -2246,6 +2250,7 @@ void do_trap_hyp_sync(struct cpu_user_regs *regs)
>    void do_trap_hyp_serror(struct cpu_user_regs *regs)
>    {
>        enter_hypervisor_head(regs);
> +    local_irq_enable();
>
>        __do_trap_serror(regs, VABORT_GEN_BY_GUEST(regs));
>    }
> @@ -2253,6 +2258,7 @@ void do_trap_hyp_serror(struct cpu_user_regs *regs)
>    void do_trap_guest_serror(struct cpu_user_regs *regs)
>    {
>        enter_hypervisor_head(regs);
> +    local_irq_enable();
>
>        __do_trap_serror(regs, true);
>    }
--
Sincerely,
Andrii Anisov.


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