Re: [PATCH v4 3/9] KVM: arm64: vgic: Be tolerant to the lack of maintenance interrupt masking

Alexandru Elisei
2021-06-11T16:37:47Z

Hi Marc,

On 6/1/21 11:39 AM, Marc Zyngier wrote:
> As it turns out, not all the interrupt controllers are able to
> expose a vGIC maintenance interrupt that can be independently
> enabled/disabled.
>
> And to be fair, it doesn't really matter as all we require is
> for the interrupt to kick us out of guest mode out way or another.
>
> To that effect, add gic_kvm_info.no_maint_irq_mask for an interrupt
> controller to advertise the lack of masking.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> arch/arm64/kvm/vgic/vgic-init.c | 8 +++++++-
> include/linux/irqchip/arm-vgic-info.h | 2 ++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 2fdb65529594..6752d084934d 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -519,12 +519,15 @@ void kvm_vgic_init_cpu_hardware(void)
> */
> int kvm_vgic_hyp_init(void)
> {
> + bool has_mask;
> int ret;
>
> if (!gic_kvm_info)
> return -ENODEV;
>
> - if (!gic_kvm_info->maint_irq) {
> + has_mask = !gic_kvm_info->no_maint_irq_mask;

This double negative is pretty awkward, I suppose this was done to avoid changes
to the gic drivers, because the default value is 0 (false). Just an idea, maybe
renaming it to maint_irq_unmaskable would be more readable?

Other than that, the patch looks good:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,

Alex

> +
> + if (has_mask && !gic_kvm_info->maint_irq) {
> kvm_err("No vgic maintenance irq\n");
> return -ENXIO;
> }
> @@ -552,6 +555,9 @@ int kvm_vgic_hyp_init(void)
> if (ret)
> return ret;
>
> + if (!has_mask)
> + return 0;
> +
> ret = request_percpu_irq(kvm_vgic_global_state.maint_irq,
> vgic_maintenance_handler,
> "vgic", kvm_get_running_vcpus());
> diff --git a/include/linux/irqchip/arm-vgic-info.h b/include/linux/irqchip/arm-vgic-info.h
> index a25d4da5697d..7c0d08ebb82c 100644
> --- a/include/linux/irqchip/arm-vgic-info.h
> +++ b/include/linux/irqchip/arm-vgic-info.h
> @@ -24,6 +24,8 @@ struct gic_kvm_info {
> struct resource vcpu;
> /* Interrupt number */
> unsigned int maint_irq;
> + /* No interrupt mask, no need to use the above field */
> + bool no_maint_irq_mask;
> /* Virtual control interface */
> struct resource vctrl;
> /* vlpi support */