Re: [QEMU-traditional] ACPI AML code races with QEMU updating the vCPU count when hotplugging more than ~14 VCPUs.

[Igor Mammedov <imammedo@redhat.com>]

On Thu, 9 May 2013 16:47:24 -0400
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> This is a race so the amount varies but on a 4PCPU box
> I seem to get only ~14 out of 16 vCPUs I want to online.
> 
> The issue at hand is that QEMU xenstore.c hotplug code
> does to the command: xl vcpu-set latest 16
> (guest config has vcpus=1, maxvcpus=32) this:
> 
> 
> QEMU:						Guest OS:
> -xenstore_process_vcpu_set_event
>  -> Gets an XenBus notification for CPU1
>  -> Updates the gpe_state.cpus_state bitfield.       
> 	-> Pulses the ACPI SCI 
> 						- ACPI SCI kicks in
> 
>  -> Gets an XenBus notification for CPU2
>  -> Updates the gpe_state.cpus_state bitfield.
> 	-> Pulses the ACPI SCI 
> 
>  -> Gets an XenBus notification for CPU3
>  -> Updates the gpe_state.cpus_state bitfield.
> 	-> Pulses the ACPI SCI 
>    ...
> 						 - Method(PRST) invoked
> 							  
>  -> Gets an XenBus notification for CPU12
>  -> Updates the gpe_state.cpus_state bitfield.
> 	-> Pulses the ACPI SCI 
> 						  - reads AF00 for CPU state
> 						    [gets 0xff]
> 						  - reads AF02 [gets 0x7f]
> 
> 							
>  -> Gets an XenBus notification for CPU13
>  -> Updates the gpe_state.cpus_state bitfield.
> 	-> Pulses the ACPI SCI 
> 
> 	.. until VCPU 16
> 						 - Method PRST updates
> 						   PR01 through 13 FLG
> 						   entry.
> 						 - PR01->PR13 _MAD
> 						   invoked.
> 
> 						 - Brings up 13 CPUs.
> 
> While QEMU updates the rest of the cpus_state bitfields the ACPI AML
> only does the CPU hotplug on those it had read. For reference
> please see the debug patch and also the QEMU log. Look for
> 'gpe_cpus_readb'.
> 
> My thinking of how to fix this is just to add in
> xenstore_process_vcpu_set_event
>  - a scan for all of the other availability/cpu states.
>  - for each of the cpu availability states query the
>    gpe_state.cpus_state and if different modify them (and set
>    a bool that one of them was modified).
>  - When done scanning and if the bool was set, then kick of the
>    qemu_irq_pulse.
> 
> Then if the other events are triggered we can just check the
> gpe_state.cpus_state against what XenBus thinks and if they
> are the same just return without doing the qemu_irq_pulse.
> 
> Thoughts?
Could you check if switching from level to edge handling in AML helps?
http://git.qemu.org/?p=seabios.git;a=commit;h=9c6635bd48d39a1d17d0a73df6e577ef6bd0037c


-- 
Regards,
  Igor