Re: [PATCH] cpu: Add starts_halted() method

2020-07-11 Thread Alex Bennée
Thiago Jung Bauermann writes: > Alex Bennée writes: > >> Thiago Jung Bauermann writes: >> >>> Eduardo Habkost writes: >>> On Wed, Jul 08, 2020 at 09:11:55PM +0100, Peter Maydell wrote: > On Wed, 8 Jul 2020 at 18:36, Eduardo Habkost wrote: > > > > On Wed, Jul 08, 2020 at 06:

Re: [PATCH] cpu: Add starts_halted() method

2020-07-10 Thread Eduardo Habkost
On Fri, Jul 10, 2020 at 05:02:38PM -0300, Thiago Jung Bauermann wrote: > > Philippe Mathieu-Daudé writes: > > > On 7/9/20 5:26 AM, Thiago Jung Bauermann wrote: > >> > >> Thiago Jung Bauermann writes: > >> > >>> I'm seeing the vcpu being KVM_RUN'd too early twice during hotplug. > >>> Both of t

Re: [PATCH] cpu: Add starts_halted() method

2020-07-10 Thread Thiago Jung Bauermann
Alex Bennée writes: > Thiago Jung Bauermann writes: > >> Eduardo Habkost writes: >> >>> On Wed, Jul 08, 2020 at 09:11:55PM +0100, Peter Maydell wrote: On Wed, 8 Jul 2020 at 18:36, Eduardo Habkost wrote: > > On Wed, Jul 08, 2020 at 06:09:49PM +0100, Peter Maydell wrote: >

Re: [PATCH] cpu: Add starts_halted() method

2020-07-10 Thread Thiago Jung Bauermann
Philippe Mathieu-Daudé writes: > On 7/9/20 5:26 AM, Thiago Jung Bauermann wrote: >> >> Thiago Jung Bauermann writes: >> >>> I'm seeing the vcpu being KVM_RUN'd too early twice during hotplug. >>> Both of them are before cpu_reset() and ppc_cpu_reset(). >> >> Hm, rereading the message obviously

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Peter Maydell
On Thu, 9 Jul 2020 at 14:13, Greg Kurz wrote: > On Thu, 9 Jul 2020 14:21:04 +0200 > Philippe Mathieu-Daudé wrote: > > The machine simply has to set the 'start-powered-off' flag on > > all vCPUS except the 1st one. > > > > We only want the first vCPU to start when the platform is > fully configure

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Philippe Mathieu-Daudé
On 7/9/20 3:13 PM, Greg Kurz wrote: > On Thu, 9 Jul 2020 14:21:04 +0200 > Philippe Mathieu-Daudé wrote: > >> On 7/9/20 12:55 PM, Greg Kurz wrote: >>> On Thu, 9 Jul 2020 12:18:06 +0200 >>> Philippe Mathieu-Daudé wrote: >>> > > [...] > > > FYI, PAPR requires all vCPUs to be "stopped" by

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Greg Kurz
On Thu, 9 Jul 2020 14:21:04 +0200 Philippe Mathieu-Daudé wrote: > On 7/9/20 12:55 PM, Greg Kurz wrote: > > On Thu, 9 Jul 2020 12:18:06 +0200 > > Philippe Mathieu-Daudé wrote: > > [...] > >>> > >>> FYI, PAPR requires all vCPUs to be "stopped" by default. It is up to the > >>> guest to start the

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Philippe Mathieu-Daudé
On 7/9/20 12:55 PM, Greg Kurz wrote: > On Thu, 9 Jul 2020 12:18:06 +0200 > Philippe Mathieu-Daudé wrote: > >> On 7/9/20 11:54 AM, Greg Kurz wrote: >>> On Thu, 9 Jul 2020 07:11:09 +0200 >>> Philippe Mathieu-Daudé wrote: On 7/8/20 11:39 PM, Eduardo Habkost wrote: > On Wed, Jul 08, 2020 at

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Greg Kurz
On Thu, 9 Jul 2020 12:18:06 +0200 Philippe Mathieu-Daudé wrote: > On 7/9/20 11:54 AM, Greg Kurz wrote: > > On Thu, 9 Jul 2020 07:11:09 +0200 > > Philippe Mathieu-Daudé wrote: > >> On 7/8/20 11:39 PM, Eduardo Habkost wrote: > >>> On Wed, Jul 08, 2020 at 06:45:57PM +0200, Philippe Mathieu-Daudé w

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Philippe Mathieu-Daudé
On 7/9/20 5:26 AM, Thiago Jung Bauermann wrote: > > Thiago Jung Bauermann writes: > >> I'm seeing the vcpu being KVM_RUN'd too early twice during hotplug. >> Both of them are before cpu_reset() and ppc_cpu_reset(). > > Hm, rereading the message obviously the above is partially wrong. The > seco

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Philippe Mathieu-Daudé
On 7/9/20 11:54 AM, Greg Kurz wrote: > On Thu, 9 Jul 2020 07:11:09 +0200 > Philippe Mathieu-Daudé wrote: >> On 7/8/20 11:39 PM, Eduardo Habkost wrote: >>> On Wed, Jul 08, 2020 at 06:45:57PM +0200, Philippe Mathieu-Daudé wrote: On 7/8/20 5:25 PM, Eduardo Habkost wrote: > On Wed, Jul 08, 2

Re: [PATCH] cpu: Add starts_halted() method

2020-07-09 Thread Greg Kurz
On Thu, 9 Jul 2020 07:11:09 +0200 Philippe Mathieu-Daudé wrote: > On 7/8/20 11:39 PM, Eduardo Habkost wrote: > > On Wed, Jul 08, 2020 at 06:45:57PM +0200, Philippe Mathieu-Daudé wrote: > >> On 7/8/20 5:25 PM, Eduardo Habkost wrote: > >>> On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wr

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Philippe Mathieu-Daudé
On 7/8/20 11:39 PM, Eduardo Habkost wrote: > On Wed, Jul 08, 2020 at 06:45:57PM +0200, Philippe Mathieu-Daudé wrote: >> On 7/8/20 5:25 PM, Eduardo Habkost wrote: >>> On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: On Wed, 8 Jul 2020 at 12:12, David Gibson wrote: > On

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Thiago Jung Bauermann
Thiago Jung Bauermann writes: > I'm seeing the vcpu being KVM_RUN'd too early twice during hotplug. > Both of them are before cpu_reset() and ppc_cpu_reset(). Hm, rereading the message obviously the above is partially wrong. The second case happens during ppc_cpu_reset(). > Here's the second:

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Thiago Jung Bauermann
Eduardo Habkost writes: > On Wed, Jul 08, 2020 at 09:11:55PM +0100, Peter Maydell wrote: >> On Wed, 8 Jul 2020 at 18:36, Eduardo Habkost wrote: >> > >> > On Wed, Jul 08, 2020 at 06:09:49PM +0100, Peter Maydell wrote: >> > > Exactly. It appears that there's a bug in our mechanisms, >> > > which

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Philippe Mathieu-Daudé
Hi Thiago, On 7/8/20 1:28 AM, Thiago Jung Bauermann wrote: > > Hello Eduardo, > > Eduardo Habkost writes: > >> On Tue, Jul 07, 2020 at 05:43:33PM -0300, Thiago Jung Bauermann wrote: >>> PowerPC sPAPRs CPUs start in the halted state, but generic QEMU code >>> assumes that CPUs start in the non-

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Eduardo Habkost
On Wed, Jul 08, 2020 at 04:32:51PM +0100, Peter Maydell wrote: > On Wed, 8 Jul 2020 at 16:25, Eduardo Habkost wrote: > > On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: > > > The original bug as described in the commit message sounds > > > to me like something we should look to fix

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Eduardo Habkost
On Tue, Jul 07, 2020 at 05:43:33PM -0300, Thiago Jung Bauermann wrote: > PowerPC sPAPRs CPUs start in the halted state, but generic QEMU code > assumes that CPUs start in the non-halted state. spapr_reset_vcpu() > attempts to rectify this by setting CPUState::halted to 1. But that's too > late for

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Philippe Mathieu-Daudé
On 7/8/20 5:25 PM, Eduardo Habkost wrote: > On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: >> On Wed, 8 Jul 2020 at 12:12, David Gibson >> wrote: >>> On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philippe Mathieu-Daudé wrote: Class boolean field certainly sounds better, but I am

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Eduardo Habkost
On Wed, Jul 08, 2020 at 06:45:57PM +0200, Philippe Mathieu-Daudé wrote: > On 7/8/20 5:25 PM, Eduardo Habkost wrote: > > On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: > >> On Wed, 8 Jul 2020 at 12:12, David Gibson > >> wrote: > >>> On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philip

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Eduardo Habkost
On Wed, Jul 08, 2020 at 06:09:49PM +0100, Peter Maydell wrote: > On Wed, 8 Jul 2020 at 17:03, Eduardo Habkost wrote: > > > > On Wed, Jul 08, 2020 at 04:32:51PM +0100, Peter Maydell wrote: > > > On Wed, 8 Jul 2020 at 16:25, Eduardo Habkost wrote: > > > > On Wed, Jul 08, 2020 at 02:14:03PM +0100, P

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Eduardo Habkost
On Wed, Jul 08, 2020 at 09:11:55PM +0100, Peter Maydell wrote: > On Wed, 8 Jul 2020 at 18:36, Eduardo Habkost wrote: > > > > On Wed, Jul 08, 2020 at 06:09:49PM +0100, Peter Maydell wrote: > > > Exactly. It appears that there's a bug in our mechanisms, > > > which is why I'm suggesting that the rig

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Eduardo Habkost
On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: > On Wed, 8 Jul 2020 at 12:12, David Gibson wrote: > > On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philippe Mathieu-Daudé wrote: > > > Class boolean field certainly sounds better, but I am not sure this > > > is a property of the machin

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Peter Maydell
On Wed, 8 Jul 2020 at 18:36, Eduardo Habkost wrote: > > On Wed, Jul 08, 2020 at 06:09:49PM +0100, Peter Maydell wrote: > > Exactly. It appears that there's a bug in our mechanisms, > > which is why I'm suggesting that the right thing is > > to fix that bug rather than marking the CPU as halted > >

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Peter Maydell
On Wed, 8 Jul 2020 at 17:03, Eduardo Habkost wrote: > > On Wed, Jul 08, 2020 at 04:32:51PM +0100, Peter Maydell wrote: > > On Wed, 8 Jul 2020 at 16:25, Eduardo Habkost wrote: > > > On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: > > > > The original bug as described in the commit m

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Peter Maydell
On Wed, 8 Jul 2020 at 16:25, Eduardo Habkost wrote: > On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: > > The original bug as described in the commit message sounds > > to me like something we should look to fix in the implementation > > of async_run_on_cpu() -- it shouldn't cause a

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread Peter Maydell
On Wed, 8 Jul 2020 at 12:12, David Gibson wrote: > On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philippe Mathieu-Daudé wrote: > > Class boolean field certainly sounds better, but I am not sure this > > is a property of the machine. Rather the arch? So move the field > > to CPUClass? Maybe not, let's

Re: [PATCH] cpu: Add starts_halted() method

2020-07-08 Thread David Gibson
On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philippe Mathieu-Daudé wrote: > Hi Thiago, > > On 7/8/20 1:28 AM, Thiago Jung Bauermann wrote: > > > > Hello Eduardo, > > > > Eduardo Habkost writes: > > > >> On Tue, Jul 07, 2020 at 05:43:33PM -0300, Thiago Jung Bauermann wrote: > >>> PowerPC sPAPRs C

Re: [PATCH] cpu: Add starts_halted() method

2020-07-07 Thread Thiago Jung Bauermann
Hello Eduardo, Eduardo Habkost writes: > On Tue, Jul 07, 2020 at 05:43:33PM -0300, Thiago Jung Bauermann wrote: >> PowerPC sPAPRs CPUs start in the halted state, but generic QEMU code >> assumes that CPUs start in the non-halted state. spapr_reset_vcpu() >> attempts to rectify this by setting

[PATCH] cpu: Add starts_halted() method

2020-07-07 Thread Thiago Jung Bauermann
PowerPC sPAPRs CPUs start in the halted state, but generic QEMU code assumes that CPUs start in the non-halted state. spapr_reset_vcpu() attempts to rectify this by setting CPUState::halted to 1. But that's too late for hotplugged CPUs in a machine configured with 2 or mor threads per core. By the