xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Julien Grall <julien.grall@arm.com>
To: Shanker Donthineni <shankerd@codeaurora.org>,
	xen-devel <xen-devel@lists.xensource.com>
Cc: Philip Elcan <pelcan@codeaurora.org>,
	Vikram Sethi <vikrams@codeaurora.org>,
	Wei Chen <Wei.Chen@arm.com>, Steve Capper <Steve.Capper@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Shannon Zhao <shannon.zhao@linaro.org>
Subject: Re: [PATCH] arm/acpi: Fix the deadlock in function vgic_lock_rank()
Date: Fri, 27 May 2016 14:56:19 +0100	[thread overview]
Message-ID: <57485203.3030407@arm.com> (raw)
In-Reply-To: <1464309582-22637-1-git-send-email-shankerd@codeaurora.org>

Hello Shanker,

On 27/05/16 01:39, Shanker Donthineni wrote:
> Commit 9d77b3c01d1261c (Configure SPI interrupt type and route to
> Dom0 dynamically) causing dead loop inside the spinlock function.
> Note that spinlocks in XEN are not recursive. Re-acquiring a spinlock
> that has already held by calling CPU leads to deadlock. This happens
> whenever dom0 does writes to GICD regs ISENABLER/ICENABLER.

Thank you for spotting it, I have not noticed it while I was  reviewing, 
only tested on a model without any SPIs.

> The following call trace explains the problem.
>
> DOM0 writes GICD_ISENABLER/GICD_ICENABLER
>    vgic_v3_distr_common_mmio_write()
>      vgic_lock_rank()  -->  acquiring first time
>        vgic_enable_irqs()
>          route_irq_to_guest()
>            gic_route_irq_to_guest()
>              vgic_get_target_vcpu()
>                vgic_lock_rank()  -->  attemping acquired lock
>
> The simple fix release spinlock before calling vgic_enable_irqs()
> and vgic_disable_irqs().

You should explain why you think it is valid to release the lock earlier.

In this case, I think the fix is not correct because the lock is 
protecting both the register value and the internal state in Xen 
(modified by vgic_enable_irqs). By releasing the lock earlier, they may 
become inconsistent if another vCPU is disabling the IRQs at the same time.

I cannot find an easy fix which does not involve release the lock. When 
I was reviewing this patch, I suggested to split the IRQ configuration 
from the routing.

The routing (call to route_irq_to_guest) will be done before DOM0 is 
booting. The IRQ configuration will be done in the ICFGR register.

This will also help for PCI-passthrough as the guest will have to 
configure the SPIs (we can't expect DOM0 doing it for it). But the 
routing will be done ahead.

This would resolve the locking issue, however it is a big task. Feel 
free to suggest a simpler one.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

  reply	other threads:[~2016-05-27 13:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-27  0:39 [PATCH] arm/acpi: Fix the deadlock in function vgic_lock_rank() Shanker Donthineni
2016-05-27 13:56 ` Julien Grall [this message]
2016-05-30 13:16   ` Stefano Stabellini
2016-05-30 19:45     ` Julien Grall
2016-05-31  0:55       ` Shannon Zhao
2016-05-31  9:40       ` Stefano Stabellini
2016-05-31 10:11         ` Julien Grall
2016-06-01  9:54           ` Stefano Stabellini
2016-06-01 10:49             ` Julien Grall
2016-06-01 13:55               ` Shannon Zhao
2016-05-31 11:37         ` Wei Liu

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=57485203.3030407@arm.com \
    --to=julien.grall@arm.com \
    --cc=Steve.Capper@arm.com \
    --cc=Wei.Chen@arm.com \
    --cc=pelcan@codeaurora.org \
    --cc=shankerd@codeaurora.org \
    --cc=shannon.zhao@linaro.org \
    --cc=sstabellini@kernel.org \
    --cc=vikrams@codeaurora.org \
    --cc=xen-devel@lists.xensource.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 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).