| Message ID | 20161201170624.26496-6-lersek@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 1 Dec 2016 18:06:22 +0100 Laszlo Ersek <lersek@redhat.com> wrote: > Introduce the following fw_cfg files: > > - "etc/smi/host-features": a little endian uint64_t feature bitmap, > presenting the features known by the host to the guest. Read-only for > the guest. 'host' here is a little bit confusing, I'd suggest 'supported-features' instead. > The content of this file is calculated by QEMU at startup (the > calculation will be added later). The same machine type is supposed to > expose the same SMI features regardless of QEMU version, hence this file > is not migrated. > > - "etc/smi/guest-features": a little endian uint64_t feature bitmap, > representing the features the guest would like to request. Read-write > for the guest. and to match above 'requested-features' > The guest can freely (re)write this file, it has no direct consequence. > Initial value is zero. A nonzero value causes the SMI-related fw_cfg > files and fields that are under guest influence to be migrated. > > - "etc/smi/features-ok": contains a uint8_t value, and it is read-only for > the guest. When the guest selects the associated fw_cfg key, the guest > features are validated against the host features. In case of error, the > negotiation doesn't proceed, and the features-ok file remains zero. In > case of success, the features-ok file becomes (uint8_t)1, and the > negotiated features are locked down internally (to which no further > changes are possible until reset). > > The initial value is zero. A nonzero value causes the SMI-related > fw_cfg files and fields that are under guest influence to be migrated. > > The C-language fields backing the host-features and guest-features files > are uint8_t arrays. This is because they carry guest-side representation > (our choice is little endian), while VMSTATE_UINT64() assumes / implies > host-side endianness for any uint64_t fields. If we migrate a guest > between hosts with different endiannesses (which is possible with TCG), > then the host-side value is preserved, and the host-side representation is > translated. This would be visible to the guest through fw_cfg, unless we > used plain byte arrays. So we do. > > Cc: "Michael S. Tsirkin" <mst@redhat.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: Igor Mammedov <imammedo@redhat.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > include/hw/i386/ich9.h | 12 +++++++- > hw/i386/pc_q35.c | 2 +- > hw/isa/lpc_ich9.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 94 insertions(+), 3 deletions(-) > > diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h > index 5fd7e97d2347..33142eb37252 100644 > --- a/include/hw/i386/ich9.h > +++ b/include/hw/i386/ich9.h > @@ -15,11 +15,12 @@ > #include "hw/pci/pci_bus.h" > > void ich9_lpc_set_irq(void *opaque, int irq_num, int level); > int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx); > PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin); > -void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled); > +void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled, > + uint64_t smi_host_features); > I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base); > > void ich9_generate_smi(void); > void ich9_generate_nmi(void); > > @@ -62,10 +63,19 @@ typedef struct ICH9LPCState { > * register contents and IO memory region > */ > uint8_t rst_cnt; > MemoryRegion rst_cnt_mem; > > + /* SMI feature negotiation via fw_cfg */ > + uint8_t smi_host_features[8]; /* guest-visible, read-only, little > + * endian uint64_t */ > + uint8_t smi_guest_features[8]; /* guest-visible, read-write, little > + * endian uint64_t */ how about adding _le suffix to 2 above fields? that way it would be easier to read usage places without chance to misinterpret content > + uint8_t smi_features_ok; /* guest-visible, read-only; selecting it > + * triggers feature lockdown */ > + uint64_t smi_negotiated_features; /* guest-invisible, host endian */ > + > /* isa bus */ > ISABus *isa_bus; > MemoryRegion rcrb_mem; /* root complex register block */ > Notifier machine_ready; > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 7fa40e7cbe0e..eb0953bb6b16 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -228,11 +228,11 @@ static void pc_q35_init(MachineState *machine) > /* init basic PC hardware */ > pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, !mc->no_floppy, > (pcms->vmport != ON_OFF_AUTO_ON), 0xff0104); > > /* connect pm stuff to lpc */ > - ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms)); > + ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms), 0); > > /* ahci and SATA device, for q35 1 ahci controller is built-in */ > ahci = pci_create_simple_multifunction(host_bus, > PCI_DEVFN(ICH9_SATA1_DEV, > ICH9_SATA1_FUNC), > diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c > index 10d1ee8b9310..ee1fa553bfee 100644 > --- a/hw/isa/lpc_ich9.c > +++ b/hw/isa/lpc_ich9.c > @@ -46,10 +46,12 @@ > #include "hw/acpi/ich9.h" > #include "hw/pci/pci_bus.h" > #include "exec/address-spaces.h" > #include "sysemu/sysemu.h" > #include "qom/cpu.h" > +#include "hw/nvram/fw_cfg.h" > +#include "qemu/cutils.h" > > /*****************************************************************************/ > /* ICH9 LPC PCI to ISA bridge */ > > static void ich9_lpc_reset(DeviceState *qdev); > @@ -358,17 +360,68 @@ static void ich9_set_sci(void *opaque, int irq_num, int level) > } else { > ich9_lpc_update_pic(lpc, irq); > } > } > > -void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled) > +static void smi_features_ok_callback(void *opaque) > +{ > + ICH9LPCState *lpc = opaque; > + uint64_t host_features, guest_features; > + > + if (lpc->smi_features_ok) { > + /* negotiation already complete, features locked */ > + return; > + } > + > + memcpy(&host_features, lpc->smi_host_features, sizeof host_features); > + memcpy(&guest_features, lpc->smi_guest_features, sizeof guest_features); > + le64_to_cpus(&host_features); > + le64_to_cpus(&guest_features); > + > + if (guest_features & ~host_features) { > + /* guest requests invalid features, leave @features_ok at zero */ > + return; > + } > + > + /* valid feature subset requested, lock it down, report success */ > + lpc->smi_negotiated_features = guest_features; > + lpc->smi_features_ok = 1; > +} > + > +void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled, > + uint64_t smi_host_features) > { > ICH9LPCState *lpc = ICH9_LPC_DEVICE(lpc_pci); > qemu_irq sci_irq; > + FWCfgState *fw_cfg = fw_cfg_find(); > > sci_irq = qemu_allocate_irq(ich9_set_sci, lpc, 0); > ich9_pm_init(lpc_pci, &lpc->pm, smm_enabled, sci_irq); > + > + if (smi_host_features && fw_cfg) { > + cpu_to_le64s(&smi_host_features); > + memcpy(lpc->smi_host_features, &smi_host_features, > + sizeof smi_host_features); > + fw_cfg_add_file(fw_cfg, "etc/smi/host-features", > + lpc->smi_host_features, > + sizeof lpc->smi_host_features); > + > + /* The other two guest-visible fields are cleared on device reset, we > + * just link them into fw_cfg here. > + */ > + fw_cfg_add_file_callback(fw_cfg, "etc/smi/guest-features", > + NULL, NULL, > + lpc->smi_guest_features, > + sizeof lpc->smi_guest_features, > + false); > + fw_cfg_add_file_callback(fw_cfg, "etc/smi/features-ok", > + smi_features_ok_callback, lpc, > + &lpc->smi_features_ok, > + sizeof lpc->smi_features_ok, > + true); > + } > + > ich9_lpc_reset(&lpc->d.qdev); > } > > /* APM */ > > @@ -505,10 +558,14 @@ static void ich9_lpc_reset(DeviceState *qdev) > ich9_lpc_pmbase_sci_update(lpc); > ich9_lpc_rcba_update(lpc, rcba_old); > > lpc->sci_level = 0; > lpc->rst_cnt = 0; > + > + memset(lpc->smi_guest_features, 0, sizeof lpc->smi_guest_features); > + lpc->smi_features_ok = 0; > + lpc->smi_negotiated_features = 0; > } > > /* root complex register block is mapped into memory space */ > static const MemoryRegionOps rcrb_mmio_ops = { > .read = ich9_cc_read, > @@ -666,10 +723,33 @@ static const VMStateDescription vmstate_ich9_rst_cnt = { > VMSTATE_UINT8(rst_cnt, ICH9LPCState), > VMSTATE_END_OF_LIST() > } > }; > > +static bool ich9_smi_feat_needed(void *opaque) > +{ > + ICH9LPCState *lpc = opaque; > + > + return !buffer_is_zero(lpc->smi_guest_features, > + sizeof lpc->smi_guest_features) || > + lpc->smi_features_ok; > +} > + > +static const VMStateDescription vmstate_ich9_smi_feat = { > + .name = "ICH9LPC/smi_feat", > + .version_id = 1, > + .minimum_version_id = 1, > + .needed = ich9_smi_feat_needed, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8_ARRAY(smi_guest_features, ICH9LPCState, > + sizeof(uint64_t)), > + VMSTATE_UINT8(smi_features_ok, ICH9LPCState), > + VMSTATE_UINT64(smi_negotiated_features, ICH9LPCState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static const VMStateDescription vmstate_ich9_lpc = { > .name = "ICH9LPC", > .version_id = 1, > .minimum_version_id = 1, > .post_load = ich9_lpc_post_load, > @@ -681,10 +761,11 @@ static const VMStateDescription vmstate_ich9_lpc = { > VMSTATE_UINT32(sci_level, ICH9LPCState), > VMSTATE_END_OF_LIST() > }, > .subsections = (const VMStateDescription*[]) { > &vmstate_ich9_rst_cnt, > + &vmstate_ich9_smi_feat, > NULL > } > }; > > static Property ich9_lpc_properties[] = {
On 12/02/16 12:54, Igor Mammedov wrote: > On Thu, 1 Dec 2016 18:06:22 +0100 > Laszlo Ersek <lersek@redhat.com> wrote: > >> Introduce the following fw_cfg files: >> >> - "etc/smi/host-features": a little endian uint64_t feature bitmap, >> presenting the features known by the host to the guest. Read-only for >> the guest. > 'host' here is a little bit confusing, I'd suggest 'supported-features' > instead. > > >> The content of this file is calculated by QEMU at startup (the >> calculation will be added later). The same machine type is supposed to >> expose the same SMI features regardless of QEMU version, hence this file >> is not migrated. >> >> - "etc/smi/guest-features": a little endian uint64_t feature bitmap, >> representing the features the guest would like to request. Read-write >> for the guest. > and to match above 'requested-features' The names were supposed to follow virtio 1.0, which calls these things "device features" and "driver features". Not a big deal anyway, I can update the file names as you suggest. >> The guest can freely (re)write this file, it has no direct consequence. >> Initial value is zero. A nonzero value causes the SMI-related fw_cfg >> files and fields that are under guest influence to be migrated. >> >> - "etc/smi/features-ok": contains a uint8_t value, and it is read-only for >> the guest. When the guest selects the associated fw_cfg key, the guest >> features are validated against the host features. In case of error, the >> negotiation doesn't proceed, and the features-ok file remains zero. In >> case of success, the features-ok file becomes (uint8_t)1, and the >> negotiated features are locked down internally (to which no further >> changes are possible until reset). >> >> The initial value is zero. A nonzero value causes the SMI-related >> fw_cfg files and fields that are under guest influence to be migrated. >> >> The C-language fields backing the host-features and guest-features files >> are uint8_t arrays. This is because they carry guest-side representation >> (our choice is little endian), while VMSTATE_UINT64() assumes / implies >> host-side endianness for any uint64_t fields. If we migrate a guest >> between hosts with different endiannesses (which is possible with TCG), >> then the host-side value is preserved, and the host-side representation is >> translated. This would be visible to the guest through fw_cfg, unless we >> used plain byte arrays. So we do. >> >> Cc: "Michael S. Tsirkin" <mst@redhat.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Cc: Igor Mammedov <imammedo@redhat.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> >> --- >> include/hw/i386/ich9.h | 12 +++++++- >> hw/i386/pc_q35.c | 2 +- >> hw/isa/lpc_ich9.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++- >> 3 files changed, 94 insertions(+), 3 deletions(-) >> >> diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h >> index 5fd7e97d2347..33142eb37252 100644 >> --- a/include/hw/i386/ich9.h >> +++ b/include/hw/i386/ich9.h >> @@ -15,11 +15,12 @@ >> #include "hw/pci/pci_bus.h" >> >> void ich9_lpc_set_irq(void *opaque, int irq_num, int level); >> int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx); >> PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin); >> -void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled); >> +void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled, >> + uint64_t smi_host_features); >> I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base); >> >> void ich9_generate_smi(void); >> void ich9_generate_nmi(void); >> >> @@ -62,10 +63,19 @@ typedef struct ICH9LPCState { >> * register contents and IO memory region >> */ >> uint8_t rst_cnt; >> MemoryRegion rst_cnt_mem; >> >> + /* SMI feature negotiation via fw_cfg */ >> + uint8_t smi_host_features[8]; /* guest-visible, read-only, little >> + * endian uint64_t */ >> + uint8_t smi_guest_features[8]; /* guest-visible, read-write, little >> + * endian uint64_t */ > how about adding _le suffix to 2 above fields? > that way it would be easier to read usage places without chance to misinterpret content Good idea, I will do that. Thanks! Laszlo > >> + uint8_t smi_features_ok; /* guest-visible, read-only; selecting it >> + * triggers feature lockdown */ >> + uint64_t smi_negotiated_features; /* guest-invisible, host endian */ >> + >> /* isa bus */ >> ISABus *isa_bus; >> MemoryRegion rcrb_mem; /* root complex register block */ >> Notifier machine_ready; >> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index 7fa40e7cbe0e..eb0953bb6b16 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -228,11 +228,11 @@ static void pc_q35_init(MachineState *machine) >> /* init basic PC hardware */ >> pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, !mc->no_floppy, >> (pcms->vmport != ON_OFF_AUTO_ON), 0xff0104); >> >> /* connect pm stuff to lpc */ >> - ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms)); >> + ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms), 0); >> >> /* ahci and SATA device, for q35 1 ahci controller is built-in */ >> ahci = pci_create_simple_multifunction(host_bus, >> PCI_DEVFN(ICH9_SATA1_DEV, >> ICH9_SATA1_FUNC), >> diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c >> index 10d1ee8b9310..ee1fa553bfee 100644 >> --- a/hw/isa/lpc_ich9.c >> +++ b/hw/isa/lpc_ich9.c >> @@ -46,10 +46,12 @@ >> #include "hw/acpi/ich9.h" >> #include "hw/pci/pci_bus.h" >> #include "exec/address-spaces.h" >> #include "sysemu/sysemu.h" >> #include "qom/cpu.h" >> +#include "hw/nvram/fw_cfg.h" >> +#include "qemu/cutils.h" >> >> /*****************************************************************************/ >> /* ICH9 LPC PCI to ISA bridge */ >> >> static void ich9_lpc_reset(DeviceState *qdev); >> @@ -358,17 +360,68 @@ static void ich9_set_sci(void *opaque, int irq_num, int level) >> } else { >> ich9_lpc_update_pic(lpc, irq); >> } >> } >> >> -void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled) >> +static void smi_features_ok_callback(void *opaque) >> +{ >> + ICH9LPCState *lpc = opaque; >> + uint64_t host_features, guest_features; >> + >> + if (lpc->smi_features_ok) { >> + /* negotiation already complete, features locked */ >> + return; >> + } >> + >> + memcpy(&host_features, lpc->smi_host_features, sizeof host_features); >> + memcpy(&guest_features, lpc->smi_guest_features, sizeof guest_features); >> + le64_to_cpus(&host_features); >> + le64_to_cpus(&guest_features); >> + >> + if (guest_features & ~host_features) { >> + /* guest requests invalid features, leave @features_ok at zero */ >> + return; >> + } >> + >> + /* valid feature subset requested, lock it down, report success */ >> + lpc->smi_negotiated_features = guest_features; >> + lpc->smi_features_ok = 1; >> +} >> + >> +void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled, >> + uint64_t smi_host_features) >> { >> ICH9LPCState *lpc = ICH9_LPC_DEVICE(lpc_pci); >> qemu_irq sci_irq; >> + FWCfgState *fw_cfg = fw_cfg_find(); >> >> sci_irq = qemu_allocate_irq(ich9_set_sci, lpc, 0); >> ich9_pm_init(lpc_pci, &lpc->pm, smm_enabled, sci_irq); >> + >> + if (smi_host_features && fw_cfg) { >> + cpu_to_le64s(&smi_host_features); >> + memcpy(lpc->smi_host_features, &smi_host_features, >> + sizeof smi_host_features); >> + fw_cfg_add_file(fw_cfg, "etc/smi/host-features", >> + lpc->smi_host_features, >> + sizeof lpc->smi_host_features); >> + >> + /* The other two guest-visible fields are cleared on device reset, we >> + * just link them into fw_cfg here. >> + */ >> + fw_cfg_add_file_callback(fw_cfg, "etc/smi/guest-features", >> + NULL, NULL, >> + lpc->smi_guest_features, >> + sizeof lpc->smi_guest_features, >> + false); >> + fw_cfg_add_file_callback(fw_cfg, "etc/smi/features-ok", >> + smi_features_ok_callback, lpc, >> + &lpc->smi_features_ok, >> + sizeof lpc->smi_features_ok, >> + true); >> + } >> + >> ich9_lpc_reset(&lpc->d.qdev); >> } >> >> /* APM */ >> >> @@ -505,10 +558,14 @@ static void ich9_lpc_reset(DeviceState *qdev) >> ich9_lpc_pmbase_sci_update(lpc); >> ich9_lpc_rcba_update(lpc, rcba_old); >> >> lpc->sci_level = 0; >> lpc->rst_cnt = 0; >> + >> + memset(lpc->smi_guest_features, 0, sizeof lpc->smi_guest_features); >> + lpc->smi_features_ok = 0; >> + lpc->smi_negotiated_features = 0; >> } >> >> /* root complex register block is mapped into memory space */ >> static const MemoryRegionOps rcrb_mmio_ops = { >> .read = ich9_cc_read, >> @@ -666,10 +723,33 @@ static const VMStateDescription vmstate_ich9_rst_cnt = { >> VMSTATE_UINT8(rst_cnt, ICH9LPCState), >> VMSTATE_END_OF_LIST() >> } >> }; >> >> +static bool ich9_smi_feat_needed(void *opaque) >> +{ >> + ICH9LPCState *lpc = opaque; >> + >> + return !buffer_is_zero(lpc->smi_guest_features, >> + sizeof lpc->smi_guest_features) || >> + lpc->smi_features_ok; >> +} >> + >> +static const VMStateDescription vmstate_ich9_smi_feat = { >> + .name = "ICH9LPC/smi_feat", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .needed = ich9_smi_feat_needed, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT8_ARRAY(smi_guest_features, ICH9LPCState, >> + sizeof(uint64_t)), >> + VMSTATE_UINT8(smi_features_ok, ICH9LPCState), >> + VMSTATE_UINT64(smi_negotiated_features, ICH9LPCState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> static const VMStateDescription vmstate_ich9_lpc = { >> .name = "ICH9LPC", >> .version_id = 1, >> .minimum_version_id = 1, >> .post_load = ich9_lpc_post_load, >> @@ -681,10 +761,11 @@ static const VMStateDescription vmstate_ich9_lpc = { >> VMSTATE_UINT32(sci_level, ICH9LPCState), >> VMSTATE_END_OF_LIST() >> }, >> .subsections = (const VMStateDescription*[]) { >> &vmstate_ich9_rst_cnt, >> + &vmstate_ich9_smi_feat, >> NULL >> } >> }; >> >> static Property ich9_lpc_properties[] = { >
diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h index 5fd7e97d2347..33142eb37252 100644 --- a/include/hw/i386/ich9.h +++ b/include/hw/i386/ich9.h @@ -15,11 +15,12 @@ #include "hw/pci/pci_bus.h" void ich9_lpc_set_irq(void *opaque, int irq_num, int level); int ich9_lpc_map_irq(PCIDevice *pci_dev, int intx); PCIINTxRoute ich9_route_intx_pin_to_irq(void *opaque, int pirq_pin); -void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled); +void ich9_lpc_pm_init(PCIDevice *pci_lpc, bool smm_enabled, + uint64_t smi_host_features); I2CBus *ich9_smb_init(PCIBus *bus, int devfn, uint32_t smb_io_base); void ich9_generate_smi(void); void ich9_generate_nmi(void); @@ -62,10 +63,19 @@ typedef struct ICH9LPCState { * register contents and IO memory region */ uint8_t rst_cnt; MemoryRegion rst_cnt_mem; + /* SMI feature negotiation via fw_cfg */ + uint8_t smi_host_features[8]; /* guest-visible, read-only, little + * endian uint64_t */ + uint8_t smi_guest_features[8]; /* guest-visible, read-write, little + * endian uint64_t */ + uint8_t smi_features_ok; /* guest-visible, read-only; selecting it + * triggers feature lockdown */ + uint64_t smi_negotiated_features; /* guest-invisible, host endian */ + /* isa bus */ ISABus *isa_bus; MemoryRegion rcrb_mem; /* root complex register block */ Notifier machine_ready; diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 7fa40e7cbe0e..eb0953bb6b16 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -228,11 +228,11 @@ static void pc_q35_init(MachineState *machine) /* init basic PC hardware */ pc_basic_device_init(isa_bus, pcms->gsi, &rtc_state, !mc->no_floppy, (pcms->vmport != ON_OFF_AUTO_ON), 0xff0104); /* connect pm stuff to lpc */ - ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms)); + ich9_lpc_pm_init(lpc, pc_machine_is_smm_enabled(pcms), 0); /* ahci and SATA device, for q35 1 ahci controller is built-in */ ahci = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_SATA1_DEV, ICH9_SATA1_FUNC), diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index 10d1ee8b9310..ee1fa553bfee 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -46,10 +46,12 @@ #include "hw/acpi/ich9.h" #include "hw/pci/pci_bus.h" #include "exec/address-spaces.h" #include "sysemu/sysemu.h" #include "qom/cpu.h" +#include "hw/nvram/fw_cfg.h" +#include "qemu/cutils.h" /*****************************************************************************/ /* ICH9 LPC PCI to ISA bridge */ static void ich9_lpc_reset(DeviceState *qdev); @@ -358,17 +360,68 @@ static void ich9_set_sci(void *opaque, int irq_num, int level) } else { ich9_lpc_update_pic(lpc, irq); } } -void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled) +static void smi_features_ok_callback(void *opaque) +{ + ICH9LPCState *lpc = opaque; + uint64_t host_features, guest_features; + + if (lpc->smi_features_ok) { + /* negotiation already complete, features locked */ + return; + } + + memcpy(&host_features, lpc->smi_host_features, sizeof host_features); + memcpy(&guest_features, lpc->smi_guest_features, sizeof guest_features); + le64_to_cpus(&host_features); + le64_to_cpus(&guest_features); + + if (guest_features & ~host_features) { + /* guest requests invalid features, leave @features_ok at zero */ + return; + } + + /* valid feature subset requested, lock it down, report success */ + lpc->smi_negotiated_features = guest_features; + lpc->smi_features_ok = 1; +} + +void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled, + uint64_t smi_host_features) { ICH9LPCState *lpc = ICH9_LPC_DEVICE(lpc_pci); qemu_irq sci_irq; + FWCfgState *fw_cfg = fw_cfg_find(); sci_irq = qemu_allocate_irq(ich9_set_sci, lpc, 0); ich9_pm_init(lpc_pci, &lpc->pm, smm_enabled, sci_irq); + + if (smi_host_features && fw_cfg) { + cpu_to_le64s(&smi_host_features); + memcpy(lpc->smi_host_features, &smi_host_features, + sizeof smi_host_features); + fw_cfg_add_file(fw_cfg, "etc/smi/host-features", + lpc->smi_host_features, + sizeof lpc->smi_host_features); + + /* The other two guest-visible fields are cleared on device reset, we + * just link them into fw_cfg here. + */ + fw_cfg_add_file_callback(fw_cfg, "etc/smi/guest-features", + NULL, NULL, + lpc->smi_guest_features, + sizeof lpc->smi_guest_features, + false); + fw_cfg_add_file_callback(fw_cfg, "etc/smi/features-ok", + smi_features_ok_callback, lpc, + &lpc->smi_features_ok, + sizeof lpc->smi_features_ok, + true); + } + ich9_lpc_reset(&lpc->d.qdev); } /* APM */ @@ -505,10 +558,14 @@ static void ich9_lpc_reset(DeviceState *qdev) ich9_lpc_pmbase_sci_update(lpc); ich9_lpc_rcba_update(lpc, rcba_old); lpc->sci_level = 0; lpc->rst_cnt = 0; + + memset(lpc->smi_guest_features, 0, sizeof lpc->smi_guest_features); + lpc->smi_features_ok = 0; + lpc->smi_negotiated_features = 0; } /* root complex register block is mapped into memory space */ static const MemoryRegionOps rcrb_mmio_ops = { .read = ich9_cc_read, @@ -666,10 +723,33 @@ static const VMStateDescription vmstate_ich9_rst_cnt = { VMSTATE_UINT8(rst_cnt, ICH9LPCState), VMSTATE_END_OF_LIST() } }; +static bool ich9_smi_feat_needed(void *opaque) +{ + ICH9LPCState *lpc = opaque; + + return !buffer_is_zero(lpc->smi_guest_features, + sizeof lpc->smi_guest_features) || + lpc->smi_features_ok; +} + +static const VMStateDescription vmstate_ich9_smi_feat = { + .name = "ICH9LPC/smi_feat", + .version_id = 1, + .minimum_version_id = 1, + .needed = ich9_smi_feat_needed, + .fields = (VMStateField[]) { + VMSTATE_UINT8_ARRAY(smi_guest_features, ICH9LPCState, + sizeof(uint64_t)), + VMSTATE_UINT8(smi_features_ok, ICH9LPCState), + VMSTATE_UINT64(smi_negotiated_features, ICH9LPCState), + VMSTATE_END_OF_LIST() + } +}; + static const VMStateDescription vmstate_ich9_lpc = { .name = "ICH9LPC", .version_id = 1, .minimum_version_id = 1, .post_load = ich9_lpc_post_load, @@ -681,10 +761,11 @@ static const VMStateDescription vmstate_ich9_lpc = { VMSTATE_UINT32(sci_level, ICH9LPCState), VMSTATE_END_OF_LIST() }, .subsections = (const VMStateDescription*[]) { &vmstate_ich9_rst_cnt, + &vmstate_ich9_smi_feat, NULL } }; static Property ich9_lpc_properties[] = {
Introduce the following fw_cfg files: - "etc/smi/host-features": a little endian uint64_t feature bitmap, presenting the features known by the host to the guest. Read-only for the guest. The content of this file is calculated by QEMU at startup (the calculation will be added later). The same machine type is supposed to expose the same SMI features regardless of QEMU version, hence this file is not migrated. - "etc/smi/guest-features": a little endian uint64_t feature bitmap, representing the features the guest would like to request. Read-write for the guest. The guest can freely (re)write this file, it has no direct consequence. Initial value is zero. A nonzero value causes the SMI-related fw_cfg files and fields that are under guest influence to be migrated. - "etc/smi/features-ok": contains a uint8_t value, and it is read-only for the guest. When the guest selects the associated fw_cfg key, the guest features are validated against the host features. In case of error, the negotiation doesn't proceed, and the features-ok file remains zero. In case of success, the features-ok file becomes (uint8_t)1, and the negotiated features are locked down internally (to which no further changes are possible until reset). The initial value is zero. A nonzero value causes the SMI-related fw_cfg files and fields that are under guest influence to be migrated. The C-language fields backing the host-features and guest-features files are uint8_t arrays. This is because they carry guest-side representation (our choice is little endian), while VMSTATE_UINT64() assumes / implies host-side endianness for any uint64_t fields. If we migrate a guest between hosts with different endiannesses (which is possible with TCG), then the host-side value is preserved, and the host-side representation is translated. This would be visible to the guest through fw_cfg, unless we used plain byte arrays. So we do. Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- include/hw/i386/ich9.h | 12 +++++++- hw/i386/pc_q35.c | 2 +- hw/isa/lpc_ich9.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 94 insertions(+), 3 deletions(-)