Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254

[Igor Mammedov]

On Tue, 18 Oct 2016 14:37:39 -0200
Eduardo Habkost  wrote:

> On Tue, Oct 18, 2016 at 05:23:04PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 13:05:51 -0200
> > Eduardo Habkost  wrote:
> >   
> > > On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:  
> > > > On Tue, 18 Oct 2016 11:38:31 -0200
> > > > Eduardo Habkost  wrote:
> > > > 
> > > > > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > > > > > Switch to modern cpu hotplug at machine startup time if
> > > > > > a cpu present at boot has apic-id in range unsupported
> > > > > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > > > > killing QEMU from legacy cpu hotplug code with error:
> > > > > >"acpi: invalid cpu id: #apic-id#"
> > > > > > 
> > > > > > Signed-off-by: Igor Mammedov 
> > > > > > ---
> > > > > >  hw/acpi/cpu_hotplug.c | 10 ++
> > > > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > > > > index e19d902..c2ab9b8 100644
> > > > > > --- a/hw/acpi/cpu_hotplug.c
> > > > > > +++ b/hw/acpi/cpu_hotplug.c
> > > > > > @@ -63,7 +63,8 @@ static void 
> > > > > > acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
> > > > > >  
> > > > > >  cpu_id = k->get_arch_id(cpu);
> > > > > >  if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > > > > -error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > > > > +object_property_set_bool(g->device, false, 
> > > > > > "cpu-hotplug-legacy",
> > > > > > + _abort);  
> > > > > 
> > > > > What happens we are in legacy mode and this is triggered during
> > > > > hotplug instead of machine init? Would it break something, or is
> > > > > it safe?
> > > >  case 1: guest with legacy hotplug AML (migrated for example) would use
> > > >  legacy interface and it won't be possible to trigger this path
> > > >  as target should be started with the same CLI as source
> > > >  (hence < 255 cpus)
> > > >  case 2: guest started on new QEMU will have new hotplug AML which 
> > > > switches
> > > >  QEMU to modern cpu hotplug interface at ACPI tables _INI time
> > > >  so this path is unreachable.
> > > 
> > > I see. Thanks for the explanation!
> > >   
> > > > 
> > > > Originally it's been static rule:
> > > > since 2.7 use new hotplug interface and old one for older machine 
> > > > types
> > > > Well it's complex but Michael insisted on keeping legacy hotplug
> > > > by default and do dynamic switching, so here we are.  
> 
> Do you rememer the reasoning for that?
Ancient SeaBIOS that doesn't get ACPI tables from QEMU supports only
legacy interface. So it's been argued that new machines should start
in legacy mode so that CPU hotplug would be operational with old SeaBIOS.
That's not applicable in practice as QEMU with new hotplug support ships
with SeaBIOS supporting ACPI tables from QEMU.
So the only case, when it could happen, is user overrides default bios
image with old one for purposes of debugging old bios with new machine
type. (that user could have used old machine type for that purpose and
we could have kept code simpler, but train already left and we are
stuck with supporting dynamic switching).

> > > > 
> > > 
> > > This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
> > > hotplug _only_", because legacy CPU hotplug is always available
> > > on startup, right?  
> > Sorry, I can't parse question, could you rephrase?  
> 
> I was just trying to clarify the meaning of
> PCMachineClass::legacy_cpu_hotplug.
> 
> I thought legacy CPU hotplug was available only if
> PCMC::legacy_cpu_hotplug was set. But legacy hotplug is still
> available even on pc-2.7 (and then switched dynamically).
both hoptlug methods available even on pre pc-2.7 machines,
and legacy hotplug is used by default. User can one way switch
mode to modern hotplug by setting cpu-hotplug-legacy prop to false
which is done by writing 0 at the beginning of legacy CPU bitmap
from AML by guest.

> 
> So the difference is that PCMC::legacy_cpu_hotplug only disables
> the ability to dynamically switch to modern hotplug, but doesn't
> disable legacy hotplug completely. Correct?
yep,
more exactly PCMC::legacy_cpu_hotplug picks whether legacy or modern
AML code should be generated by QEMU.

After guest is switched to modern mode, it can't switch back
'abort(!value)' takes care about that.

> 
> >   
> > >   
> > > > this behavior is since 2.7:
> > > > commit 679dd1a957df418453efdd3ed2914dba5cd73773
> > > > pc: use new CPU hotplug interface since 2.7 machine type
> > > > 
> > > >  
> > > > > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > > > > assert(!value). I assume this means we must replace the QOM
> > > > > property with something that the user can't 

Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254

[Eduardo Habkost]

On Tue, Oct 18, 2016 at 05:23:04PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 13:05:51 -0200
> Eduardo Habkost  wrote:
> 
> > On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:
> > > On Tue, 18 Oct 2016 11:38:31 -0200
> > > Eduardo Habkost  wrote:
> > >   
> > > > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:  
> > > > > Switch to modern cpu hotplug at machine startup time if
> > > > > a cpu present at boot has apic-id in range unsupported
> > > > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > > > killing QEMU from legacy cpu hotplug code with error:
> > > > >"acpi: invalid cpu id: #apic-id#"
> > > > > 
> > > > > Signed-off-by: Igor Mammedov 
> > > > > ---
> > > > >  hw/acpi/cpu_hotplug.c | 10 ++
> > > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > > > index e19d902..c2ab9b8 100644
> > > > > --- a/hw/acpi/cpu_hotplug.c
> > > > > +++ b/hw/acpi/cpu_hotplug.c
> > > > > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug 
> > > > > *g, CPUState *cpu,
> > > > >  
> > > > >  cpu_id = k->get_arch_id(cpu);
> > > > >  if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > > > -error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > > > +object_property_set_bool(g->device, false, 
> > > > > "cpu-hotplug-legacy",
> > > > > + _abort);
> > > > 
> > > > What happens we are in legacy mode and this is triggered during
> > > > hotplug instead of machine init? Would it break something, or is
> > > > it safe?  
> > >  case 1: guest with legacy hotplug AML (migrated for example) would use
> > >  legacy interface and it won't be possible to trigger this path
> > >  as target should be started with the same CLI as source
> > >  (hence < 255 cpus)
> > >  case 2: guest started on new QEMU will have new hotplug AML which 
> > > switches
> > >  QEMU to modern cpu hotplug interface at ACPI tables _INI time
> > >  so this path is unreachable.  
> > 
> > I see. Thanks for the explanation!
> > 
> > > 
> > > Originally it's been static rule:
> > > since 2.7 use new hotplug interface and old one for older machine 
> > > types
> > > Well it's complex but Michael insisted on keeping legacy hotplug
> > > by default and do dynamic switching, so here we are.

Do you rememer the reasoning for that?

> > >   
> > 
> > This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
> > hotplug _only_", because legacy CPU hotplug is always available
> > on startup, right?
> Sorry, I can't parse question, could you rephrase?

I was just trying to clarify the meaning of
PCMachineClass::legacy_cpu_hotplug.

I thought legacy CPU hotplug was available only if
PCMC::legacy_cpu_hotplug was set. But legacy hotplug is still
available even on pc-2.7 (and then switched dynamically).

So the difference is that PCMC::legacy_cpu_hotplug only disables
the ability to dynamically switch to modern hotplug, but doesn't
disable legacy hotplug completely. Correct?

> 
> > 
> > > this behavior is since 2.7:
> > > commit 679dd1a957df418453efdd3ed2914dba5cd73773
> > > pc: use new CPU hotplug interface since 2.7 machine type
> > > 
> > >
> > > > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > > > assert(!value). I assume this means we must replace the QOM
> > > > property with something that the user can't fiddle with, right?  
> > > it's readonly to user after machine starts, but allows user
> > > to play modern hotplug interface on old machine types if needed.
> > > assert is there to trap mistake of switching to legacy mode
> > > (which is default) from compat_properties.  
> > 
> > What exactly makes the property read-only to the user? Maybe we
> > should make the setter return an error instead, as all
> > object_property_set_bool("cpu-hotplug-legacy") calls already use
> > _abort?
> My mistake,
> it's dynamic property with custom setters in piix4/ich9_pm and user
> potentially can write there.
> 
> I was under impression that errors are ignored if property comes from
> compat_props, if returning error would prevent machine from starting
> when property comes from compat_props I can fix cpu-hotplug-legacy to
> return error.

compat_props set errp to _abort, see
machine_register_compat_props().

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254

[Igor Mammedov]

On Tue, 18 Oct 2016 13:05:51 -0200
Eduardo Habkost  wrote:

> On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:
> > On Tue, 18 Oct 2016 11:38:31 -0200
> > Eduardo Habkost  wrote:
> >   
> > > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:  
> > > > Switch to modern cpu hotplug at machine startup time if
> > > > a cpu present at boot has apic-id in range unsupported
> > > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > > killing QEMU from legacy cpu hotplug code with error:
> > > >"acpi: invalid cpu id: #apic-id#"
> > > > 
> > > > Signed-off-by: Igor Mammedov 
> > > > ---
> > > >  hw/acpi/cpu_hotplug.c | 10 ++
> > > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > > index e19d902..c2ab9b8 100644
> > > > --- a/hw/acpi/cpu_hotplug.c
> > > > +++ b/hw/acpi/cpu_hotplug.c
> > > > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug 
> > > > *g, CPUState *cpu,
> > > >  
> > > >  cpu_id = k->get_arch_id(cpu);
> > > >  if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > > -error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > > +object_property_set_bool(g->device, false, 
> > > > "cpu-hotplug-legacy",
> > > > + _abort);
> > > 
> > > What happens we are in legacy mode and this is triggered during
> > > hotplug instead of machine init? Would it break something, or is
> > > it safe?  
> >  case 1: guest with legacy hotplug AML (migrated for example) would use
> >  legacy interface and it won't be possible to trigger this path
> >  as target should be started with the same CLI as source
> >  (hence < 255 cpus)
> >  case 2: guest started on new QEMU will have new hotplug AML which switches
> >  QEMU to modern cpu hotplug interface at ACPI tables _INI time
> >  so this path is unreachable.  
> 
> I see. Thanks for the explanation!
> 
> > 
> > Originally it's been static rule:
> > since 2.7 use new hotplug interface and old one for older machine types
> > Well it's complex but Michael insisted on keeping legacy hotplug
> > by default and do dynamic switching, so here we are.
> >   
> 
> This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
> hotplug _only_", because legacy CPU hotplug is always available
> on startup, right?
Sorry, I can't parse question, could you rephrase?

> 
> > this behavior is since 2.7:
> > commit 679dd1a957df418453efdd3ed2914dba5cd73773
> > pc: use new CPU hotplug interface since 2.7 machine type
> > 
> >
> > > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > > assert(!value). I assume this means we must replace the QOM
> > > property with something that the user can't fiddle with, right?  
> > it's readonly to user after machine starts, but allows user
> > to play modern hotplug interface on old machine types if needed.
> > assert is there to trap mistake of switching to legacy mode
> > (which is default) from compat_properties.  
> 
> What exactly makes the property read-only to the user? Maybe we
> should make the setter return an error instead, as all
> object_property_set_bool("cpu-hotplug-legacy") calls already use
> _abort?
My mistake,
it's dynamic property with custom setters in piix4/ich9_pm and user
potentially can write there.

I was under impression that errors are ignored if property comes from
compat_props, if returning error would prevent machine from starting
when property comes from compat_props I can fix cpu-hotplug-legacy to
return error.

> 
> >   
> > >   
> > > >  return;
> > > >  }
> > > >  
> > > > @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion 
> > > > *parent, Object *owner,
> > > >  {
> > > >  CPUState *cpu;
> > > >  
> > > > -CPU_FOREACH(cpu) {
> > > > -acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> > > > -}
> > > >  memory_region_init_io(_cpu->io, owner, _ops,
> > > >gpe_cpu, "acpi-cpu-hotplug", 
> > > > ACPI_GPE_PROC_LEN);
> > > >  memory_region_add_subregion(parent, base, _cpu->io);
> > > >  gpe_cpu->device = owner;
> > > > +
> > > > +CPU_FOREACH(cpu) {
> > > > +acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> > > > +}
> > > >  }
> > > >  
> > > >  void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> > > > -- 
> > > > 2.7.4
> > > > 
> > >   
> >   
> 

Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254

[Eduardo Habkost]

On Tue, Oct 18, 2016 at 04:34:53PM +0200, Igor Mammedov wrote:
> On Tue, 18 Oct 2016 11:38:31 -0200
> Eduardo Habkost  wrote:
> 
> > On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > > Switch to modern cpu hotplug at machine startup time if
> > > a cpu present at boot has apic-id in range unsupported
> > > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > > killing QEMU from legacy cpu hotplug code with error:
> > >"acpi: invalid cpu id: #apic-id#"
> > > 
> > > Signed-off-by: Igor Mammedov 
> > > ---
> > >  hw/acpi/cpu_hotplug.c | 10 ++
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > > index e19d902..c2ab9b8 100644
> > > --- a/hw/acpi/cpu_hotplug.c
> > > +++ b/hw/acpi/cpu_hotplug.c
> > > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, 
> > > CPUState *cpu,
> > >  
> > >  cpu_id = k->get_arch_id(cpu);
> > >  if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > > -error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > > +object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> > > + _abort);  
> > 
> > What happens we are in legacy mode and this is triggered during
> > hotplug instead of machine init? Would it break something, or is
> > it safe?
>  case 1: guest with legacy hotplug AML (migrated for example) would use
>  legacy interface and it won't be possible to trigger this path
>  as target should be started with the same CLI as source
>  (hence < 255 cpus)
>  case 2: guest started on new QEMU will have new hotplug AML which switches
>  QEMU to modern cpu hotplug interface at ACPI tables _INI time
>  so this path is unreachable.

I see. Thanks for the explanation!

> 
> Originally it's been static rule:
> since 2.7 use new hotplug interface and old one for older machine types
> Well it's complex but Michael insisted on keeping legacy hotplug
> by default and do dynamic switching, so here we are.
> 

This means PCMachineClass::legacy_cpu_hotplug means "legacy CPU
hotplug _only_", because legacy CPU hotplug is always available
on startup, right?

> this behavior is since 2.7:
> commit 679dd1a957df418453efdd3ed2914dba5cd73773
> pc: use new CPU hotplug interface since 2.7 machine type
> 
>  
> > Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> > assert(!value). I assume this means we must replace the QOM
> > property with something that the user can't fiddle with, right?
> it's readonly to user after machine starts, but allows user
> to play modern hotplug interface on old machine types if needed.
> assert is there to trap mistake of switching to legacy mode
> (which is default) from compat_properties.

What exactly makes the property read-only to the user? Maybe we
should make the setter return an error instead, as all
object_property_set_bool("cpu-hotplug-legacy") calls already use
_abort?

> 
> > 
> > >  return;
> > >  }
> > >  
> > > @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion 
> > > *parent, Object *owner,
> > >  {
> > >  CPUState *cpu;
> > >  
> > > -CPU_FOREACH(cpu) {
> > > -acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> > > -}
> > >  memory_region_init_io(_cpu->io, owner, _ops,
> > >gpe_cpu, "acpi-cpu-hotplug", 
> > > ACPI_GPE_PROC_LEN);
> > >  memory_region_add_subregion(parent, base, _cpu->io);
> > >  gpe_cpu->device = owner;
> > > +
> > > +CPU_FOREACH(cpu) {
> > > +acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> > > +}
> > >  }
> > >  
> > >  void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> > > -- 
> > > 2.7.4
> > >   
> > 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254

[Igor Mammedov]

On Tue, 18 Oct 2016 11:38:31 -0200
Eduardo Habkost  wrote:

> On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> > Switch to modern cpu hotplug at machine startup time if
> > a cpu present at boot has apic-id in range unsupported
> > by legacy cpu hotplug interface (i.e. > 254), to avoid
> > killing QEMU from legacy cpu hotplug code with error:
> >"acpi: invalid cpu id: #apic-id#"
> > 
> > Signed-off-by: Igor Mammedov 
> > ---
> >  hw/acpi/cpu_hotplug.c | 10 ++
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> > index e19d902..c2ab9b8 100644
> > --- a/hw/acpi/cpu_hotplug.c
> > +++ b/hw/acpi/cpu_hotplug.c
> > @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, 
> > CPUState *cpu,
> >  
> >  cpu_id = k->get_arch_id(cpu);
> >  if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> > -error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> > +object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> > + _abort);  
> 
> What happens we are in legacy mode and this is triggered during
> hotplug instead of machine init? Would it break something, or is
> it safe?
 case 1: guest with legacy hotplug AML (migrated for example) would use
 legacy interface and it won't be possible to trigger this path
 as target should be started with the same CLI as source
 (hence < 255 cpus)
 case 2: guest started on new QEMU will have new hotplug AML which switches
 QEMU to modern cpu hotplug interface at ACPI tables _INI time
 so this path is unreachable.

Originally it's been static rule:
since 2.7 use new hotplug interface and old one for older machine types
Well it's complex but Michael insisted on keeping legacy hotplug
by default and do dynamic switching, so here we are.

this behavior is since 2.7:
commit 679dd1a957df418453efdd3ed2914dba5cd73773
pc: use new CPU hotplug interface since 2.7 machine type

 
> Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
> assert(!value). I assume this means we must replace the QOM
> property with something that the user can't fiddle with, right?
it's readonly to user after machine starts, but allows user
to play modern hotplug interface on old machine types if needed.

assert is there to trap mistake of switching to legacy mode
(which is default) from compat_properties.

> 
> >  return;
> >  }
> >  
> > @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, 
> > Object *owner,
> >  {
> >  CPUState *cpu;
> >  
> > -CPU_FOREACH(cpu) {
> > -acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> > -}
> >  memory_region_init_io(_cpu->io, owner, _ops,
> >gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
> >  memory_region_add_subregion(parent, base, _cpu->io);
> >  gpe_cpu->device = owner;
> > +
> > +CPU_FOREACH(cpu) {
> > +acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> > +}
> >  }
> >  
> >  void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> > -- 
> > 2.7.4
> >   
> 

Re: [Qemu-devel] [PATCH v3 04/13] acpi: cphp: force switch to modern cpu hotplug if APIC ID > 254

[Eduardo Habkost]

On Thu, Oct 13, 2016 at 11:52:38AM +0200, Igor Mammedov wrote:
> Switch to modern cpu hotplug at machine startup time if
> a cpu present at boot has apic-id in range unsupported
> by legacy cpu hotplug interface (i.e. > 254), to avoid
> killing QEMU from legacy cpu hotplug code with error:
>"acpi: invalid cpu id: #apic-id#"
> 
> Signed-off-by: Igor Mammedov 
> ---
>  hw/acpi/cpu_hotplug.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index e19d902..c2ab9b8 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -63,7 +63,8 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, 
> CPUState *cpu,
>  
>  cpu_id = k->get_arch_id(cpu);
>  if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> -error_setg(errp, "acpi: invalid cpu id: %" PRIi64, cpu_id);
> +object_property_set_bool(g->device, false, "cpu-hotplug-legacy",
> + _abort);

What happens we are in legacy mode and this is triggered during
hotplug instead of machine init? Would it break something, or is
it safe?

Unrelated to this patch: piix4_set_cpu_hotplug_legacy() has an
assert(!value). I assume this means we must replace the QOM
property with something that the user can't fiddle with, right?

>  return;
>  }
>  
> @@ -85,13 +86,14 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, 
> Object *owner,
>  {
>  CPUState *cpu;
>  
> -CPU_FOREACH(cpu) {
> -acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> -}
>  memory_region_init_io(_cpu->io, owner, _ops,
>gpe_cpu, "acpi-cpu-hotplug", ACPI_GPE_PROC_LEN);
>  memory_region_add_subregion(parent, base, _cpu->io);
>  gpe_cpu->device = owner;
> +
> +CPU_FOREACH(cpu) {
> +acpi_set_cpu_present_bit(gpe_cpu, cpu, _abort);
> +}
>  }
>  
>  void acpi_switch_to_modern_cphp(AcpiCpuHotplug *gpe_cpu,
> -- 
> 2.7.4
> 

-- 
Eduardo