[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 7/7] hw/i386/pc_q35: advertise broadcast SMI
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH v4 7/7] hw/i386/pc_q35: advertise broadcast SMI if VCPU hotplug is turned off |
Date: |
Fri, 2 Dec 2016 13:18:15 +0100 |
On Thu, 1 Dec 2016 21:42:58 +0100
Laszlo Ersek <address@hidden> wrote:
> On 12/01/16 20:13, Eduardo Habkost wrote:
> > On Thu, Dec 01, 2016 at 06:06:24PM +0100, Laszlo Ersek wrote:
> >> For the time being, we cannot handle SMIs in OVMF if VCPUs can show up
> >> after boot. Otherwise, advertise ICH9_LPC_SMI_F_BROADCAST.
> >>
> >> Implement this generally, by introducing a new PCMachineClass method,
> >> namely get_smi_host_features(), and implement the above logic for
> >> pc-q35-2.9 and later. The idea is that future machine types might want to
> >> calculate (the same or different) SMI host features from different
> >> information, and that shouldn't affect earlier machine types.
> >>
> >> In turn, validating guest feature requests (inter-feature dependencies)
> >> should be possible purely based on the available host feature set. For
> >> example, in the future we might enforce that the guest select
> >> ICH9_LPC_SMI_F_VCPU_PARKING as a prerequisite for
> >> ICH9_LPC_SMI_F_BROADCAST, but only if the machine type itself advertises
> >> ICH9_LPC_SMI_F_VCPU_PARKING.
> >>
> >> Cc: "Michael S. Tsirkin" <address@hidden>
> >> Cc: Eduardo Habkost <address@hidden>
> >> Cc: Gerd Hoffmann <address@hidden>
> >> Cc: Igor Mammedov <address@hidden>
> >> Cc: Paolo Bonzini <address@hidden>
> >> Signed-off-by: Laszlo Ersek <address@hidden>
> >> ---
> >> include/hw/i386/pc.h | 1 +
> >> hw/i386/pc_q35.c | 24 +++++++++++++++++++++++-
> >> 2 files changed, 24 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >> index 430735e501dd..e164947116b6 100644
> >> --- a/include/hw/i386/pc.h
> >> +++ b/include/hw/i386/pc.h
> >> @@ -116,10 +116,11 @@ struct PCMachineClass {
> >> /*< public >*/
> >>
> >> /* Methods: */
> >> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> >> DeviceState *dev);
> >> + uint64_t (*get_smi_host_features)(void);
> >
> > I'd prefer to encode the differences between machine-types as
> > data, instead of code, to make introspection easier in the
> > future. Is it possible to encode this in a simple PCMachineClass
> > struct data field or (even bettter) a QOM property?
> >
>
> I don't know how else to capture the (smp_cpus == max_cpus) question,
> for saying that "this machine type supports SMI broadcast, as long as
> VCPU hotplug is disabled in the configuration".
(smp_cpus == max_cpus) doesn't mean that CPU hotplug is disabled
(it actually can't be disabled at all).
In addition, it's possible to start machine with
-smp 1,sockets=2,max_cpus=2 -device mycputype,socket=2
where all cpus will be coldpugged
It would be better to use PCMachineState.boot_cpus which contains
present cpus count.
I'd drop hotplug check (as usecase is broken in many places anyway)
and even won't touch PCMachineState, instead add a property like
ICH9_LPC.enable_smi_broadcast(on by default) and disable it for
old machine types using compat props.
We can work on making CPU hotplug and OVMF work in follow up patches.
> Technically we could give PCMC two new (data) fields, a host features
> bitmap for when VCPU hotplug is enabled, and another for when VCPU
> hotplug is disabled. Then board code would take the right field and pass
> it on to ich9_lpc_pm_init().
>
> Thanks
> Laszlo
- [Qemu-devel] [PATCH v4 6/7] hw/isa/lpc_ich9: add broadcast SMI feature, (continued)
Re: [Qemu-devel] [PATCH v4 0/7] q35: add negotiable broadcast SMI, no-reply, 2016/12/20