linux-kernel - Re: [PATCH] KVM: arm/arm64: Set ICH_HCR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <3993685d-a78f-2e73-e22a-8ade3b6a6279@arm.com>
Date:   Tue, 12 Mar 2019 15:48:35 +0000
From:   Marc Zyngier <marc.zyngier@....com>
To:     "Tangnianyao (ICT)" <tangnianyao@...wei.com>,
        Christoffer Dall <christoffer.dall@....com>,
        "james.morse@....com" <james.morse@....com>,
        "julien.thierry@....com" <julien.thierry@....com>,
        "suzuki.poulose@....com" <suzuki.poulose@....com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "will.deacon@....com" <will.deacon@....com>,
        "alex.bennee@...aro.org" <alex.bennee@...aro.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "andre.przywara@....com" <andre.przywara@....com>,
        Zhangshaokun <zhangshaokun@...ilicon.com>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] KVM: arm/arm64: Set ICH_HCR_EN in guest anyway when using
 gicv4

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@...wei.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.
-- 
Jazz is not dead. It just smells funny...