From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCH] xen/arm: introduce platform_need_explicit_eoi Date: Sat, 21 Jun 2014 15:43:08 +0100 Message-ID: References: <1403271316-21635-1-git-send-email-stefano.stabellini@eu.citrix.com> <53A440FD.6070303@linaro.org> <53A4496A.4080009@linaro.org> <53A46863.2040701@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: Julien Grall Cc: xen-devel@lists.xensource.com, apatel@apm.com, Ian Campbell , psawargaonkar@apm.com, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Fri, 20 Jun 2014, Julien Grall wrote: > On 20 Jun 2014 18:41, "Stefano Stabellini" wrote: > > > > On Fri, 20 Jun 2014, Julien Grall wrote: > > > On 20/06/14 17:48, Stefano Stabellini wrote: > > > > > > > > > Your patch "physical irq follow virtual irq" won't even solve this problem > > > > > if > > > > > the maintenance interrupt is request to EOI the IRQ. > > > > > > > > I don't understand this statement. > > > > The maintenance interrupt is requested to make sure that the vcpu is > > > > interrupted as soon as it EOIs the virtual irq, so that > > > > gic_update_one_lr can run. > > > > > > I agree with that but ... the following step can happen > > > > > > 1) Xen receives the interrupt > > > 2) Xen writes EOIR > > > 3) Xen inject the IRQ to the guest > > > 4) The guest will receive the IRQ (i.e read IAR) > > > 5) Xen migrates the VCPU to another physical CPU > > > 6) The guest writes into GIC_DIR and a maintenance IRQ is fired. > > > > > > In a such case, the IRQ will be EOIed to another physical CPU. If we assume > > > that we should always write to GICC_DIR of the physical CPU that receive the > > > interrupt, then even your patch "physcial irq follow virtual irq" won't solve > > > the problem. > > > > That's true. > > > > > > > > > > > The VGIC lock is per-vcpu. > > > > > > > > > > If this is happening while we migrate, nothing protect p->lr and the > > > > > different > > > > > bit anymore. > > > > > > > > The patch series > > > > alpine.DEB.2.02.1406111607450.13771@kaball.uk.xensource.com takes > > > > care of vcpu migration and locking. > > > > > > > > > > > > > Indeed, the vgic_inject_irq will use the new VCPU. This can happen as soon > > > > > as > > > > > we EOI the IRQ. > > > > > > > > Yes, but at that point we have also already migrated the corresponding > > > > physical irq to the new pcpu. > > > > > > Even tho we the migration has been done, the 2 clear bit are not atomic. Let's > > > imagine that the IRQ has fired between the two clear bit. In this case we may > > > clear the ACTIVE bit by mistake. > > > > > > I don't see anything in this code which prevents a such configuration. > > > > Which 2 clear bit? > > GUEST_VISIBLE and GUEST_ACTIVE see the code after eoi_irq. The code is protected by the vgic lock.