Patchwork KVM: arm/arm64: Set ICH_HCR_EN in guest anyway when using gicv4

login
register
mail settings
Submitter Tangnianyao (ICT)
Date March 12, 2019, 12:22 p.m.
Message ID <F348A9E21A573744858FF7E5D32E29D81FEFA462@DGGEMI529-MBX.china.huawei.com>
Download mbox | patch
Permalink /patch/747111/
State New
Headers show

Comments

Tangnianyao (ICT) - March 12, 2019, 12:22 p.m.
In gicv4, direct vlpi may be forward to PE without using LR or ap_list. If
ICH_HCR_EL2.En is 0 in guest, direct vlpi cannot be accepted by PE.
It will take long time for direct vlpi to be forwarded in some cases.
Direct vlpi has to wait for ICH_HCR_EL2.En=1 where some other interrupts
using LR need to be forwarded, because in kvm_vgic_flush_hwstate,
if ap_list is empty, it will return quickly not setting ICH_HCR_EL2.En.
To fix this, set ICH_HCR_EL2.En to 1 before returning to guest when
using GICv4.

Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
---
arch/arm64/include/asm/kvm_asm.h |  1 +
virt/kvm/arm/hyp/vgic-v3-sr.c    | 10 ++++++++++
virt/kvm/arm/vgic/vgic-v4.c      |  8 ++++++++
3 files changed, 19 insertions(+)

        /*
+       * Enable ICH_HCR_EL.En so that guest can accpet and handle direct
+       * vlpi.
+       */
+       __vgic_v3_write_hcr(vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr);
+
+       /*
        * Before making the VPE resident, make sure the redistributor
        * corresponding to our current CPU expects us here. See the
        * doc in drivers/irqchip/irq-gic-v4.c to understand how this
--
1.9.1
Marc Zyngier - March 12, 2019, 3:48 p.m.
Nianyao,

Please do not send patches as HTML. Or any email as HTML. Please make
sure that you only send plain text emails on any mailing list (see point
#6 in Documentation/process/submitting-patches.rst).

On 12/03/2019 12:22, Tangnianyao (ICT) wrote:
> In gicv4, direct vlpi may be forward to PE without using LR or ap_list. If
> 
> ICH_HCR_EL2.En is 0 in guest, direct vlpi cannot be accepted by PE.
> 
> It will take long time for direct vlpi to be forwarded in some cases.
> 
> Direct vlpi has to wait for ICH_HCR_EL2.En=1 where some other interrupts
> 
> using LR need to be forwarded, because in kvm_vgic_flush_hwstate,
> 
> if ap_list is empty, it will return quickly not setting ICH_HCR_EL2.En.
> 
> To fix this, set ICH_HCR_EL2.En to 1 before returning to guest when
> 
> using GICv4.
> 
>  
> 
> Signed-off-by: Nianyao Tang <tangnianyao@huawei.com>
> 
> ---
> 
> arch/arm64/include/asm/kvm_asm.h |  1 +
> 
> virt/kvm/arm/hyp/vgic-v3-sr.c    | 10 ++++++++++
> 
> virt/kvm/arm/vgic/vgic-v4.c      |  8 ++++++++
> 
> 3 files changed, 19 insertions(+)
> 
>  
> 
> diff --git a/arch/arm64/include/asm/kvm_asm.h
> b/arch/arm64/include/asm/kvm_asm.h
> 
> index f5b79e9..0581c4d 100644
> 
> --- a/arch/arm64/include/asm/kvm_asm.h
> 
> +++ b/arch/arm64/include/asm/kvm_asm.h
> 
> @@ -79,6 +79,7 @@
> 
> extern void __vgic_v3_init_lrs(void);
> 
>  extern u32 __kvm_get_mdcr_el2(void);
> 
> +extern void __vgic_v3_write_hcr(u32 vmcr);
> 
>  /* Home-grown __this_cpu_{ptr,read} variants that always work at HYP */
> 
> #define
> __hyp_this_cpu_ptr(sym)                                                       
> \
> 
> diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c
> 
> index 264d92d..12027af 100644
> 
> --- a/virt/kvm/arm/hyp/vgic-v3-sr.c
> 
> +++ b/virt/kvm/arm/hyp/vgic-v3-sr.c
> 
> @@ -434,6 +434,16 @@ void __hyp_text __vgic_v3_write_vmcr(u32 vmcr)
> 
>        write_gicreg(vmcr, ICH_VMCR_EL2);
> 
> }
> 
> +u64 __hyp_text __vgic_v3_read_hcr(void)
> 
> +{
> 
> +       return read_gicreg(ICH_HCR_EL2);
> 
> +}
> 
> +
> 
> +void __hyp_text __vgic_v3_write_hcr(u32 vmcr)
> 
> +{
> 
> +       write_gicreg(vmcr, ICH_HCR_EL2);
> 
> +}

This is HYP code...

> 
> +
> 
> #ifdef CONFIG_ARM64
> 
>  static int __hyp_text __vgic_v3_bpr_min(void)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
> 
> index 1ed5f22..515171a 100644
> 
> --- a/virt/kvm/arm/vgic/vgic-v4.c
> 
> +++ b/virt/kvm/arm/vgic/vgic-v4.c
> 
> @@ -208,6 +208,8 @@ int vgic_v4_sync_hwstate(struct kvm_vcpu *vcpu)
> 
>        if (!vgic_supports_direct_msis(vcpu->kvm))
> 
>                 return 0;
> 
> +       __vgic_v3_write_hcr(vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr &
> ~ICH_HCR_EN);

And you've now crashed your non-VHE system by branching directly to code
that cannot be executed at EL1.

> 
> +
> 
>        return its_schedule_vpe(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe, false);
> 
> }
> 
> @@ -220,6 +222,12 @@ int vgic_v4_flush_hwstate(struct kvm_vcpu *vcpu)
> 
>                 return 0;
> 
>         /*
> 
> +       * Enable ICH_HCR_EL.En so that guest can accpet and handle direct
> 
> +       * vlpi.
> 
> +       */
> 
> +       __vgic_v3_write_hcr(vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr);
> 
> +
> 
> +       /*
> 
>         * Before making the VPE resident, make sure the redistributor
> 
>         * corresponding to our current CPU expects us here. See the
> 
>         * doc in drivers/irqchip/irq-gic-v4.c to understand how this
> 
> -- 
> 
> 1.9.1
> 
>  
> 
>  
> 

Overall, this looks like the wrong approach. It is not the GICv4 code's
job to do this, but the low-level code (either the load/put code for VHE
or the save/restore code for non-VHE).

It would certainly help if you could describe which context you're in
(VHE, non-VHE). I suppose the former...

Thanks,

	M.

Patch

diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index f5b79e9..0581c4d 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -79,6 +79,7 @@ 
extern void __vgic_v3_init_lrs(void);
 extern u32 __kvm_get_mdcr_el2(void);
+extern void __vgic_v3_write_hcr(u32 vmcr);
 /* Home-grown __this_cpu_{ptr,read} variants that always work at HYP */
#define __hyp_this_cpu_ptr(sym)                                                        \
diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c
index 264d92d..12027af 100644
--- a/virt/kvm/arm/hyp/vgic-v3-sr.c
+++ b/virt/kvm/arm/hyp/vgic-v3-sr.c
@@ -434,6 +434,16 @@  void __hyp_text __vgic_v3_write_vmcr(u32 vmcr)
       write_gicreg(vmcr, ICH_VMCR_EL2);
}
+u64 __hyp_text __vgic_v3_read_hcr(void)
+{
+       return read_gicreg(ICH_HCR_EL2);
+}
+
+void __hyp_text __vgic_v3_write_hcr(u32 vmcr)
+{
+       write_gicreg(vmcr, ICH_HCR_EL2);
+}
+
#ifdef CONFIG_ARM64
 static int __hyp_text __vgic_v3_bpr_min(void)
diff --git a/virt/kvm/arm/vgic/vgic-v4.c b/virt/kvm/arm/vgic/vgic-v4.c
index 1ed5f22..515171a 100644
--- a/virt/kvm/arm/vgic/vgic-v4.c
+++ b/virt/kvm/arm/vgic/vgic-v4.c
@@ -208,6 +208,8 @@  int vgic_v4_sync_hwstate(struct kvm_vcpu *vcpu)
       if (!vgic_supports_direct_msis(vcpu->kvm))
                return 0;
+       __vgic_v3_write_hcr(vcpu->arch.vgic_cpu.vgic_v3.vgic_hcr & ~ICH_HCR_EN);
+
       return its_schedule_vpe(&vcpu->arch.vgic_cpu.vgic_v3.its_vpe, false);
}
@@ -220,6 +222,12 @@  int vgic_v4_flush_hwstate(struct kvm_vcpu *vcpu)
                return 0;