From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH-4.5 v3 10/12] xen/arm: don't protect GICH and lr_queue accesses with gic.lock Date: Tue, 18 Mar 2014 20:26:42 +0000 Message-ID: <5328AC02.807@linaro.org> References: <1393439997-26936-10-git-send-email-stefano.stabellini@eu.citrix.com> <530E4BBD.4010703@linaro.org> <53286424.8080000@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: jtd@galois.com, xen-devel@lists.xensource.com, Ian.Campbell@citrix.com List-Id: xen-devel@lists.xenproject.org Hi Stefano, On 03/18/2014 06:52 PM, Stefano Stabellini wrote: > On Tue, 18 Mar 2014, Julien Grall wrote: >> Hi Stefano, >> >> On 03/18/2014 02:59 PM, Stefano Stabellini wrote: >>> On Wed, 26 Feb 2014, Julien Grall wrote: >>>> On 26/02/14 18:39, Stefano Stabellini wrote: >>>>> GICH is banked, protect accesses by disabling interrupts. >>>>> Protect lr_queue accesses with the vgic.lock only. >>>> >>>> When the interrupt is an SPI, the lr_queue is shared between every VCPU. Using >>>> only vgic.lock seems wrong to me. >>> >>> Even though lr_queue is a field in a struct that can be per domain for >>> irq > 32, the lr_pending queue is always per vcpu, in fact it keeps >>> track of the irqs waiting to go into lr registers, that are banked. >>> >> >> It might have a race between adding and removing lr_queue. For now it's >> safe because the SPIs are always routed to VCPU0 (even if the guest ask >> to route on VCPU1). >> >> Just by reading the code, nothing protect correctly SPIs between VCPUs. >> We are only take vgic.lock, which is wrong. > > Like you said, today SPIs are always routed to the same cpu. > When we'll implement IRQ migration we'll have to make sure that we take > the appropriate locks during the operation but I don't expect that this > patch is going to make things more difficult. I'm fine with that. Can you create a TODO somewhere to not forget this possible locking issue later? Regards, -- Julien Grall