This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH] make HVM PIC emulation code SMP-safe

To: "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] make HVM PIC emulation code SMP-safe
From: "Dave Lively" <dave.lively@xxxxxxxxx>
Date: Tue, 16 May 2006 11:05:48 -0400
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, "Dong, Eddie" <eddie.dong@xxxxxxxxx>
Delivery-date: Tue, 16 May 2006 08:08:21 -0700
Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:references; b=m/3qvquKazvDA9S0e4/OPK8k1a34gF/6EHm7eVVwPy3ex7kkICDMrNzDnFcCk+2Z4Uxxes+QQ5Z8WrTMThxsDwMZuP0aRPcfCDq5sV0vmZ+Kurqt3IAls/TnoXhiaaC61G+PL9PWD0l489qzP8lFDxXBl3ydr/YhkOnigG+LBxk=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <5feefd15d4e12039b0208dd6de809a93@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <255D0D519B44514CA2384FC76944C1AA467DAE@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <5feefd15d4e12039b0208dd6de809a93@xxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Hi Keir / Eddie -

  I was also thinking cpu_get_pic_interrupt() should be called only on
VCPU 0 (and in fact, originally tested with restriction).  But since I
wasn't sure the restriction was necessary (and things worked fine
without it), I removed it.  I'll put it back in now, in light of these
comments.  (Revised patch attached.)

  However, that doesn't remove the need for locking (or some sort
of PIC concurrency control).  First (and most importantly), the guest
can access the PIC from *any* VCPU, though it must be careful to
access it from at most one VCPU at once.  This alone means it can
conflict with hypervisor PIC access from VCPU 0.  (Note that the
"APIC kludge" causes PIC acccess from arbitrary VCPUs.  So this
patch fixes that as well.  But the patch would be necessary even without
this kludge.)

  I'm not wild about having locks on this path either.  A safe lockless
protocol may be possible, but I don't have the time to try to work one
out now.  In our experience, this patch greatly improves the stability
of SMP guests.  (But note we currently tell SMP guest kernels to
ignore the I/O APIC, otherwise we lose too many hda interrupts.)


P.S. The I/O APIC code has similar issues, for mostly the same
reasons (guests can access from any VCPU).  I have a similar patch
for it, but haven't been able to test it yet (due to other I/O APIC

On 5/16/06, Keir Fraser <Keir.Fraser@xxxxxxxxxxxx> wrote:

Well, that would make it behave like a *real* PIC, wired through to
CPU0's INT pin, right? So it sounds good to me, especially if we are
currently spraying PIC interrupts across all VCPUs. And it's even
better if it avoids requiring locking on the vmentry path.

  -- Keir

On 16 May 2006, at 06:20, Dong, Eddie wrote:

> For the PIC I/O access, do weneed lock protect? But for IRQ
> generation, i.e. cpu_get_interrupt, how about following for simplicity
> (not tested)?
> diff -r 1e3977e029fd xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c Mon May 08 19:21:41 2006 +0100
> +++ b/xen/arch/x86/hvm/hvm.c Tue May 16 13:18:04 2006 +0800
> @@ -248,7 +248,7 @@ int cpu_get_interrupt(struct vcpu *v, in
> return intno;
> }
> /* read the irq from the PIC */
> - if ( (intno = cpu_get_pic_interrupt(v, type)) != -1 )
> + if ( v->vcpu_id == 0 && (intno = cpu_get_pic_interrupt(v, type))
> != -1 )
> return intno;
> return -1;

Attachment: vpic-smp-safety.patch
Description: Text Data

Xen-devel mailing list