On Thu, Dec 01, 2016 at 09:42:58PM +0100, Laszlo Ersek 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" <m...@redhat.com> > >> Cc: Eduardo Habkost <ehabk...@redhat.com> > >> Cc: Gerd Hoffmann <kra...@redhat.com> > >> Cc: Igor Mammedov <imamm...@redhat.com> > >> Cc: Paolo Bonzini <pbonz...@redhat.com> > >> Signed-off-by: Laszlo Ersek <ler...@redhat.com> > >> --- > >> 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". > > 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().
It's more complex than I expected, but I would still prefer that than having a growing collection of get_smi_host_features() functions. However, I am not strongly against the function pointer if others want to avoid the complexity of two extra bitmaps. -- Eduardo