[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Dave:
    Looks like this patch is just using "BUG_ON" for APIs like pic_ioport_read.
    Also let me share something about 1) HVM virtual Interrupt Controller movement in consideration. When APIC is enabled for SMP, guest software need to disable PIC for correct interrupt generation mechanism. If guest bios is presenting MPS, then we can know this through IMCR base on MPS spec when guest switch from PIC mode to Symmetric I/O Mode. If guest bios is presenting ACPI, the guest software will disable PIC by writing 0xff to master PIC IMR. Current VMX guest implements ACPI.
    In either way, when Xen detects the guest has disabled PIC, the PIC IRQ generation logic can be removed for both performance and simplicity. So even VP0 doesn't need to get/queue interrupt from PIC. The ideal solution may be that we need to define some registable APIs in HVM virtual weired interrupt controller logic. At startup time, we regieter PIC APIs, and later on replace it with APIC's (the transition time need special concern while it is non performance critical path).
    Anyway, 2) PIC I/O access may need to be SMP safe (maybe a big lock for simplicity), 3) IOAPIC needs to be SMP safe. Then we try to keep minimal changes to device models with that in Qemu (yes we already have some) for future maintain effort. You are very welcome here.
    thx,eddie

From: Dave Lively [mailto:dave.lively@xxxxxxxxx]
Sent: 2006年5月16日 23:06
To: Keir Fraser
Cc: Dong, Eddie; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH] make HVM PIC emulation code SMP-safe

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.)

Dave

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
problems).

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;


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.