kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexandru Elisei <alexandru.elisei@arm.com>
To: Marc Zyngier <maz@kernel.org>,
	linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	kvmarm@lists.cs.columbia.edu
Cc: James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Eric Auger <eric.auger@redhat.com>,
	Hector Martin <marcan@marcan.st>,
	Mark Rutland <mark.rutland@arm.com>,
	kernel-team@android.com
Subject: Re: [PATCH v3 4/9] KVM: arm64: vgic: Let an interrupt controller advertise lack of HW deactivation
Date: Fri, 21 May 2021 18:01:05 +0100	[thread overview]
Message-ID: <417846b3-ff5e-1832-82b2-3e0064275944@arm.com> (raw)
In-Reply-To: <20210510134824.1910399-5-maz@kernel.org>

Hi Marc,

On 5/10/21 2:48 PM, Marc Zyngier wrote:
> The vGIC, as architected by ARM, allows a virtual interrupt to
> trigger the deactivation of a physical interrupt. This allows
> the following interrupt to be delivered without requiring an exit.

If I got this right, the AIC doesn't implement/ignores the ICH_LR_EL2.HW bit. Does
it mean that the CPU IF behaves as if HW = 0b0, meaning it asserts a maintenance
interrupt on virtual interrupt deactivation when ICH_LR_EL2.EOI = 0b1? I assume
that's the case, just double checking.

I am wondering what would happen if we come across an interrupt controller where
the CPU IF cannot assert a maintenance interrupt at all and we rely on the EOI bit
to take us out of the guest to deactivate the HW interrupt. I have to say that it
looks a bit strange to start relying on the maintenance interrupt to emulate
interrupt deactivate for hardware interrupts, but at the same timer allowing an
interrupt controller without a maintenance interrupt.

Other than that, this idea sounds like the best thing to do considering the
circumstances, I certainly can't think of anything better.

>
> However, some implementations have choosen not to implement this,
> meaning that we will need some unsavoury workarounds to deal with this.
>
> On detecting such a case, taint the kernel and spit a nastygram.
> We'll deal with this in later patches.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/vgic/vgic-init.c       | 10 ++++++++++
>  include/kvm/arm_vgic.h                |  3 +++
>  include/linux/irqchip/arm-vgic-info.h |  2 ++
>  3 files changed, 15 insertions(+)
>
> diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
> index 9fd23f32aa54..5b06a9970a57 100644
> --- a/arch/arm64/kvm/vgic/vgic-init.c
> +++ b/arch/arm64/kvm/vgic/vgic-init.c
> @@ -524,6 +524,16 @@ int kvm_vgic_hyp_init(void)
>  	if (!gic_kvm_info)
>  		return -ENODEV;
>  
> +	/*
> +	 * If we get one of these oddball non-GICs, taint the kernel,
> +	 * as we have no idea of how they *really* behave.
> +	 */
> +	if (gic_kvm_info->no_hw_deactivation) {
> +		kvm_info("Non-architectural vgic, tainting kernel\n");
> +		add_taint(TAINT_CPU_OUT_OF_SPEC, LOCKDEP_STILL_OK);
> +		kvm_vgic_global_state.no_hw_deactivation = true;
> +	}

IMO, since this means we're going to rely even more on the maintenance interrupt
(not just for software emulation of level sensitive interrupts), I think there
should be some sort of dependency on having something that resembles a working
maintenance interrupt.

Thanks,

Alex

> +
>  	switch (gic_kvm_info->type) {
>  	case GIC_V2:
>  		ret = vgic_v2_probe(gic_kvm_info);
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index ec621180ef09..e45b26e8d479 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -72,6 +72,9 @@ struct vgic_global {
>  	bool			has_gicv4;
>  	bool			has_gicv4_1;
>  
> +	/* Pseudo GICv3 from outer space */
> +	bool			no_hw_deactivation;
> +
>  	/* GIC system register CPU interface */
>  	struct static_key_false gicv3_cpuif;
>  
> diff --git a/include/linux/irqchip/arm-vgic-info.h b/include/linux/irqchip/arm-vgic-info.h
> index 0319636be928..d39d0b591c5a 100644
> --- a/include/linux/irqchip/arm-vgic-info.h
> +++ b/include/linux/irqchip/arm-vgic-info.h
> @@ -30,6 +30,8 @@ struct gic_kvm_info {
>  	bool		has_v4;
>  	/* rvpeid support */
>  	bool		has_v4_1;
> +	/* Deactivation impared, subpar stuff */
> +	bool		no_hw_deactivation;
>  };
>  
>  #ifdef CONFIG_KVM

  reply	other threads:[~2021-05-21 17:00 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-10 13:48 [PATCH v3 0/9] KVM: arm64: Initial host support for the Apple M1 Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 1/9] irqchip/gic: Split vGIC probing information from the GIC code Marc Zyngier
2021-05-18 16:51   ` Alexandru Elisei
2021-05-10 13:48 ` [PATCH v3 2/9] KVM: arm64: Handle physical FIQ as an IRQ while running a guest Marc Zyngier
2021-05-20 17:46   ` Alexandru Elisei
2021-05-10 13:48 ` [PATCH v3 3/9] KVM: arm64: vgic: Be tolerant to the lack of maintenance interrupt Marc Zyngier
2021-05-10 16:19   ` Mark Rutland
2021-05-10 17:44     ` Marc Zyngier
2021-05-11 11:13       ` Mark Rutland
2021-05-10 13:48 ` [PATCH v3 4/9] KVM: arm64: vgic: Let an interrupt controller advertise lack of HW deactivation Marc Zyngier
2021-05-21 17:01   ` Alexandru Elisei [this message]
2021-05-24 17:17     ` Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 5/9] KVM: arm64: vgic: move irq->get_input_level into an ops structure Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 6/9] KVM: arm64: vgic: Implement SW-driven deactivation Marc Zyngier
2021-05-24 16:53   ` Alexandru Elisei
2021-05-24 17:43     ` Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 7/9] KVM: arm64: timer: Refactor IRQ configuration Marc Zyngier
2021-05-14 12:46   ` Zenghui Yu
2021-05-24 17:48     ` Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 8/9] KVM: arm64: timer: Add support for SW-based deactivation Marc Zyngier
2021-05-10 13:48 ` [PATCH v3 9/9] irqchip/apple-aic: Advertise some level of vGICv3 compatibility Marc Zyngier
2021-05-12 16:22 ` [PATCH v3 0/9] KVM: arm64: Initial host support for the Apple M1 Alexandru Elisei
2021-05-12 16:33   ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=417846b3-ff5e-1832-82b2-3e0064275944@arm.com \
    --to=alexandru.elisei@arm.com \
    --cc=eric.auger@redhat.com \
    --cc=james.morse@arm.com \
    --cc=kernel-team@android.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marcan@marcan.st \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=suzuki.poulose@arm.com \
    --subject='Re: [PATCH v3 4/9] KVM: arm64: vgic: Let an interrupt controller advertise lack of HW deactivation' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).