Re: [Xen-devel] [PATCH V2] firmware: Change level-triggered GPE event to a edge one for qemu-xen

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Dec 13, 2013 at 11:51:19AM +0000, Jan Beulich wrote:
> >>> On 12.12.13 at 16:30, Anthony PERARD <anthony.perard@xxxxxxxxxx> wrote:
> > Change in V2:
> >   - better patch comment:
> >     patch those not fix race, but reduce the window
> >     include patch description of the quoted commit
> 
> Thanks - quite a bit better to understand.
> 
> >   - change also apply to pci hotplug.
> 
> The one thing I'm still missing for both changes is a brief word on
> why qemu-xen-traditional doesn't want/need this and - as iirc
> you said - if that one manages to get this implemented without a
> similar race, why upstream qemu can't do things in a similar way.

In qemu-trad, instead of sending an SCI interrupt for every new vcpu, we
loop through every xenstore key that represent vcpus availability and
only send an SCI only when the loop is over. And it looks like one `xl
vcpu-set` provoc only one loop.

But adding a vcpu in qemu-xen is done via a QMP command, "cpu-add id=X".
qemu-xen have no way to know if there will be a next cpu-add command, so
we can not apply the same thing.

For why qemu-xen-traditional doesn't not need this:
  - a single `xl vcpu-set` can not trigger the race

For why qemu-xen-traditional doesn't not want this:
  - avoid unnecessary change, especially in the ACPI table

About the PCI hotplug change and qemu-xen-traditional, I actually don't
know what would be the effect.

Why qemu-xen-traditional would like this change for vcpu only:
  - we might hit the race window via several consecutive `xl vcpu-set`
    calls, or via direct change of xenstore entries.
but after few try, I could not manage to have a missing cpu hotplug
event.


So should I just add in the patch comment something like:
"Since some measure have been put in place in qemu-xen-traditional
against this race, we don't change the ACPI table for it."
?

> Also iirc someone responded to v1 with a comment saying a race
> like this is an OS bug - I don#t think I've seen a response to this.

I can not find this comment. But the fact that a race can happen does
not look like an OS bug. Also, I've discrabed in the patch comment the
behavior of QEMU (hardware) and what an OS is suppose to do with the
AML as specified in the ACPI document.

-- 
Anthony PERARD

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