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

["Jan Beulich" <JBeulich@xxxxxxxx>]

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

> 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?

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.

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

Furthermore, to more clearly reflect what the real difference is, I'd
suggest using a conditional operator for passing the one argument
that varies rather than using an if/else construct.

Jan


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