Re: [PATCH v9 10/26] arm64: kvm: Unmask PMR before entering guest

[Marc Zyngier]

On Mon, 21 Jan 2019 15:33:29 +0000,
Julien Thierry <julien.thierry@xxxxxxx> wrote:
> 
> Interrupts masked by ICC_PMR_EL1 will not be signaled to the CPU. This
> means that hypervisor will not receive masked interrupts while running a
> guest.
> 
> Avoid this by making sure ICC_PMR_EL1 is unmasked when we enter a guest.
> 
> Signed-off-by: Julien Thierry <julien.thierry@xxxxxxx>
> Acked-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Christoffer Dall <christoffer.dall@xxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Catalin Marinas <catalin.marinas@xxxxxxx>
> Cc: Will Deacon <will.deacon@xxxxxxx>
> Cc: kvmarm@xxxxxxxxxxxxxxxxxxxxx
> ---
>  arch/arm64/include/asm/kvm_host.h | 12 ++++++++++++
>  arch/arm64/kvm/hyp/switch.c       | 16 ++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 7732d0b..a1f9f55 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -24,6 +24,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/kvm_types.h>
> +#include <asm/arch_gicv3.h>
>  #include <asm/cpufeature.h>
>  #include <asm/daifflags.h>
>  #include <asm/fpsimd.h>
> @@ -474,6 +475,17 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
>  static inline void kvm_arm_vhe_guest_enter(void)
>  {
>  	local_daif_mask();
> +
> +	/*
> +	 * Having IRQs masked via PMR when entering the guest means the GIC
> +	 * will not signal the CPU of interrupts of lower priority, and the
> +	 * only way to get out will be via guest exceptions.
> +	 * Naturally, we want to avoid this.
> +	 */
> +	if (system_uses_irq_prio_masking()) {
> +		gic_write_pmr(GIC_PRIO_IRQON);
> +		dsb(sy);
> +	}
>  }
>  
>  static inline void kvm_arm_vhe_guest_exit(void)
> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
> index b0b1478..6a4c2d6 100644
> --- a/arch/arm64/kvm/hyp/switch.c
> +++ b/arch/arm64/kvm/hyp/switch.c
> @@ -22,6 +22,7 @@
>  
>  #include <kvm/arm_psci.h>
>  
> +#include <asm/arch_gicv3.h>
>  #include <asm/cpufeature.h>
>  #include <asm/kvm_asm.h>
>  #include <asm/kvm_emulate.h>
> @@ -521,6 +522,17 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  	struct kvm_cpu_context *guest_ctxt;
>  	u64 exit_code;
>  
> +	/*
> +	 * Having IRQs masked via PMR when entering the guest means the GIC
> +	 * will not signal the CPU of interrupts of lower priority, and the
> +	 * only way to get out will be via guest exceptions.
> +	 * Naturally, we want to avoid this.
> +	 */
> +	if (system_uses_irq_prio_masking()) {
> +		gic_write_pmr(GIC_PRIO_IRQON);
> +		dsb(sy);
> +	}
> +
>  	vcpu = kern_hyp_va(vcpu);
>  
>  	host_ctxt = kern_hyp_va(vcpu->arch.host_cpu_context);
> @@ -573,6 +585,10 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
>  	 */
>  	__debug_switch_to_host(vcpu);
>  
> +	/* Returning to host will clear PSR.I, remask PMR if needed */
> +	if (system_uses_irq_prio_masking())
> +		gic_write_pmr(GIC_PRIO_IRQOFF);
> +
>  	return exit_code;
>  }

It'd be good to have helpers for these PMR manipulations, which I
suspect you could use in other parts of the code such as patch #9.

The logic itself seems good to me, so irrespective of the above:

Reviewed-by: Marc Zyngier <marc.zyngier@xxxxxxx>

	M.

-- 
Jazz is not dead, it just smell funny.