All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoffer Dall <christoffer.dall@linaro.org>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH] KVM: arm/arm64: vgic-new: Removel harmful BUG_ON
Date: Thu, 2 Jun 2016 11:52:01 +0200	[thread overview]
Message-ID: <20160602095201.GA20310@cbox> (raw)
In-Reply-To: <1464855846-1625-1-git-send-email-marc.zyngier@arm.com>

On Thu, Jun 02, 2016 at 09:24:06AM +0100, Marc Zyngier wrote:
> When changing the active bit from an MMIO trap, we decide to
> explode if the intid is that of a private interrupt.
> 
> This really looks like a leftover from a previous version, as
> the callers explicitly check for private interrupts and handle
> them correctly. I've managed to reproduce it using a custom
> guest.

Actually this was me mistakingly thinking that kvm_vcpu_kick() would be
synchronous and we therefore would have an assurance that we're back
from running the VCPU, but this is clearly not the case, so dropping the
BUG_ON is indeed the right thing to do.

> 
> Dropping the BUG_ON seems like the right thing to do.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Applied, thanks.

-Christoffer

> ---
>  virt/kvm/arm/vgic/vgic-mmio.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
> index 059595e..9f6fab7 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
> @@ -191,10 +191,8 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
>  	 * other thread sync back the IRQ.
>  	 */
>  	while (irq->vcpu && /* IRQ may have state in an LR somewhere */
> -	       irq->vcpu->cpu != -1) { /* VCPU thread is running */
> -		BUG_ON(irq->intid < VGIC_NR_PRIVATE_IRQS);
> +	       irq->vcpu->cpu != -1) /* VCPU thread is running */
>  		cond_resched_lock(&irq->irq_lock);
> -	}
>  
>  	irq->active = new_active_state;
>  	if (new_active_state)
> -- 
> 2.1.4
> 

WARNING: multiple messages have this Message-ID (diff)
From: christoffer.dall@linaro.org (Christoffer Dall)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] KVM: arm/arm64: vgic-new: Removel harmful BUG_ON
Date: Thu, 2 Jun 2016 11:52:01 +0200	[thread overview]
Message-ID: <20160602095201.GA20310@cbox> (raw)
In-Reply-To: <1464855846-1625-1-git-send-email-marc.zyngier@arm.com>

On Thu, Jun 02, 2016 at 09:24:06AM +0100, Marc Zyngier wrote:
> When changing the active bit from an MMIO trap, we decide to
> explode if the intid is that of a private interrupt.
> 
> This really looks like a leftover from a previous version, as
> the callers explicitly check for private interrupts and handle
> them correctly. I've managed to reproduce it using a custom
> guest.

Actually this was me mistakingly thinking that kvm_vcpu_kick() would be
synchronous and we therefore would have an assurance that we're back
from running the VCPU, but this is clearly not the case, so dropping the
BUG_ON is indeed the right thing to do.

> 
> Dropping the BUG_ON seems like the right thing to do.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Applied, thanks.

-Christoffer

> ---
>  virt/kvm/arm/vgic/vgic-mmio.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
> index 059595e..9f6fab7 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
> @@ -191,10 +191,8 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
>  	 * other thread sync back the IRQ.
>  	 */
>  	while (irq->vcpu && /* IRQ may have state in an LR somewhere */
> -	       irq->vcpu->cpu != -1) { /* VCPU thread is running */
> -		BUG_ON(irq->intid < VGIC_NR_PRIVATE_IRQS);
> +	       irq->vcpu->cpu != -1) /* VCPU thread is running */
>  		cond_resched_lock(&irq->irq_lock);
> -	}
>  
>  	irq->active = new_active_state;
>  	if (new_active_state)
> -- 
> 2.1.4
> 

  reply	other threads:[~2016-06-02  9:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-02  8:24 [PATCH] KVM: arm/arm64: vgic-new: Removel harmful BUG_ON Marc Zyngier
2016-06-02  8:24 ` Marc Zyngier
2016-06-02  9:52 ` Christoffer Dall [this message]
2016-06-02  9:52   ` Christoffer Dall

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=20160602095201.GA20310@cbox \
    --to=christoffer.dall@linaro.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.