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

["Dave Lively" <dave.lively@xxxxxxxxx>]

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;

vpic-smp-safety.patch
Description: Text Data

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