[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 5/7] hw/isa/lpc_ich9: add SMI feature negotia
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH v4 5/7] hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg |
Date: |
Fri, 2 Dec 2016 12:54:12 +0100 |
On Thu, 1 Dec 2016 18:06:22 +0100
Laszlo Ersek <address@hidden> 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" <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/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[] = {
- Re: [Qemu-devel] [PATCH v4 2/7] fw-cfg: turn FW_CFG_FILE_SLOTS into a device property, (continued)
[Qemu-devel] [PATCH v4 4/7] hw/i386/pc: introduce 2.9 machine types with 0x20 fw_cfg file slots, Laszlo Ersek, 2016/12/01
[Qemu-devel] [PATCH v4 6/7] hw/isa/lpc_ich9: add broadcast SMI feature, Laszlo Ersek, 2016/12/01
[Qemu-devel] [PATCH v4 1/7] fw-cfg: support writeable blobs, Laszlo Ersek, 2016/12/01
[Qemu-devel] [PATCH v4 5/7] hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg, Laszlo Ersek, 2016/12/01
- Re: [Qemu-devel] [PATCH v4 5/7] hw/isa/lpc_ich9: add SMI feature negotiation via fw_cfg,
Igor Mammedov <=
[Qemu-devel] [PATCH v4 7/7] hw/i386/pc_q35: advertise broadcast SMI if VCPU hotplug is turned off, Laszlo Ersek, 2016/12/01
Re: [Qemu-devel] [PATCH v4 0/7] q35: add negotiable broadcast SMI, no-reply, 2016/12/20