Re: [Xen-devel] [PATCH] firmware: Fix vcpu hotplug race with qemu-xen

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Dec 10, 2013 at 08:43:20AM +0000, Jan Beulich wrote:
> >>> On 09.12.13 at 19:30, Anthony PERARD <anthony.perard@xxxxxxxxxx> wrote:
> > When hotplugging more than one vcpu, some of those vcpus might not be
> > seen as plugged by the guest.
> > 
> > By using edged-triggered General-Purpose Event instead of a
> > level-triggered GPE, the race is avoided. This is in sync with how a
> > different QEMU guest is handeling CPU hotplug event.
> 
> What "different QEMU guest" are you talking about here? That
> second sentence is rather cryptic to me.

Well, I've tested only KVM, but the ACPI table are provided by the
SeaBIOS project. So the "different QEMU guest" are any guest that use
the default machine (Standard PC (i440FX + PIIX, 1996)) with SeaBIOS and
default ACPI tables.

> > There is more information about why in SeaBIOS's commit
> > 9c6635bd48d39a1d17d0a73df6e577ef6bd0037c.
> 
> That commit's description talks of reducing the chance of a race;
> it doesn't say it's being avoided completely. Can you clarify the
> discrepancy?

Yes, I did not really understand the change, but it is now clear, the
change here does not fix the race. So there is no more discrepancy. I
will rewrite the patch comment.

> Also, I personally think referring to a foreign tree's commit in a
> commit message like this is putting too high expectations on the
> reader. Naming commit ID _and title_ would be nice, but you
> should really have quoted the core part of the explanation from
> there.

Yes, that make sense, I will fix that.

> > --- a/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > +++ b/tools/firmware/hvmloader/acpi/mk_dsdt.c
> > @@ -220,9 +220,13 @@ int main(int argc, char **argv)
> >  
> >      pop_block();
> >  
> > -    /* Define GPE control method '_L02'. */
> > +    /* Define GPE control method. */
> >      push_block("Scope", "\\_GPE");
> > -    push_block("Method", "_L02");
> > +    if (dm_version == QEMU_XEN) {
> > +        push_block("Method", "_E02");
> > +    } else {
> > +        push_block("Method", "_L02");
> > +    }
> 
> The commit description says nothing at all about why this needs to
> be done differently depending on dm_version.

I though it did, the title have "with qemu-xen". I want to avoid
changing the table for qemu-traditional where the race have been fix in
qemu.


Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel