All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Anthony PERARD <anthony.perard@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Stefano Stabellini <stefano.stabellini@citrix.com>,
	Ian Campbell <ian.campbell@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: [PATCH] firmware: Fix vcpu hotplug race with qemu-xen
Date: Tue, 10 Dec 2013 08:43:20 +0000	[thread overview]
Message-ID: <52A6E238020000780010BB22@nat28.tlf.novell.com> (raw)
In-Reply-To: <1386613837-27399-1-git-send-email-anthony.perard@citrix.com>

>>> On 09.12.13 at 19:30, Anthony PERARD <anthony.perard@citrix.com> 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

  reply	other threads:[~2013-12-10  8:43 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 18:30 [PATCH] firmware: Fix vcpu hotplug race with qemu-xen Anthony PERARD
2013-12-10  8:43 ` Jan Beulich [this message]
2013-12-11 16:12   ` Anthony PERARD
2013-12-12 15:30 ` [PATCH V2] firmware: Change level-triggered GPE event to a edge one for qemu-xen Anthony PERARD
2013-12-13 11:51   ` Jan Beulich
2013-12-13 17:11     ` Anthony PERARD
2013-12-13 17:32       ` Ian Jackson
2013-12-13 17:44       ` Konrad Rzeszutek Wilk
2013-12-13 18:17         ` Anthony PERARD
2013-12-16  7:51       ` Jan Beulich
2013-12-13 19:17   ` [PATCH V3] " Anthony PERARD

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A6E238020000780010BB22@nat28.tlf.novell.com \
    --to=jbeulich@suse.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=stefano.stabellini@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.