[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q' |
Date: |
Wed, 16 Nov 2016 19:50:01 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 11/16/16 19:04, Paolo Bonzini wrote:
>> I guess that's what the next paragraph is about:
>>
>>> - we could have another magic 0xB2 value, which is implemented directly
>>> in QEMU and sets 0xB3 to a magic value. Then OVMF can invoke it
>>> after SMBASE relocation and SMM IPL (so as not to crash on old QEMUs)
>>> to detect the new feature. It can fail to start if using traditional
>>> AP and the new feature is not there.
>>
>> Please explain in more detail. If I write to 0xB2 (by invoking the
>> Trigger() method or somehow else), then on old QEMU's that will raise a
>> sync / unicast SMI. The SMI handler in edk2 will run, but no request
>> parameters will have been set up by OVMF, so the SMI handler will do...
>> no clue what.
>
> It should hopefully do nothing. A spurious SMI (such as the one caused
> by the write to 0xB2) should not crash OVMF.
>
> SMBASE relocation uses IPIs, so my hope was to use the
> SmmCpuFeaturesSmmRelocationComplete hook.
>From a cursory look, SmmCpuFeaturesSmmRelocationComplete() seems to be
called early enough from PiSmmCpuDxeSmm that we might be able to call
PcdSet() from it, for updating PcdCpuSmmApSyncTimeout and
PcdCpuSmmSyncMode. I perceive it a bit too close to the edge :)
>> My preference is fw_cfg ATM. It provides a prove, flexible and
>> extensible interface (it's easy to add new files for future features).
>> If we expect more knobs in the area, I can modify my proposal to use
>> "etc/smi/broadcast", so we can add "etc/smi/XXXX" later.
>
> Did you know there are 16 entries only for fw_cfg files? :)
Yes, I've known that, but it can be changed by redefining
FW_CFG_FILE_SLOTS, can't it? The key type for fw_cfg is uint16_t, so we
should have some reserves.
> And we're
> using already 20 in the worst case:
>
> genroms/linuxboot.bin
> genroms/kvmvapic.bin
> NVDIMM_DSM_MEM_FILE
> "etc/smbios/smbios-tables"
> "etc/smbios/smbios-anchor"
> "etc/acpi/tables"
> "etc/table-loader"
> ACPI_BUILD_TPMLOG_FILE
> ACPI_BUILD_RSDP_FILE
> "etc/e820"
> "etc/msr_feature_control"
> "etc/reserved-memory-end"
> "etc/pvpanic-port"
> "etc/boot-menu-wait"
> "bootsplash.jpg"
> "etc/boot-fail-wait"
> "etc/igd-opregion"
> "etc/igd-bdsm-size"
> "etc/extra-pci-roots"
> "bootorder"
>
> Therefore, so close to the release I'm a bit worried about doing
> changes to fw_cfg or adding more fw_cfg files. Though we just got
> rid of one file for the number of CPUs, so I guess we might not care.
I agree with your caution about this. I'm also perfectly fine if this
update misses 2.8. :)
>
>> Do you have any specific arguments against fw_cfg? As I suggested in my
>> previous email, with fw_cfg I can implement the change in OVMF such that
>> the default behavior wouldn't change -- the default delivery would
>> remain relaxed, and the broadcast wouldn't be requested, unless the
>> fw_cfg file told OVMF otherwise.
>>
>>> By the way, in case OVMF needs to use SmmSwDispatch in the future, I
>>> would make QEMU use broadcast behavior for all values in the 0x10-0xff
>>> range, or something like that.
>>
>> Are we talking control/command (0xB2) or scratch/data (0xB3) register
>> values? My patches currently use the scratch/data register to provide
>> the hint to QEMU; that register is less likely to interfere with
>> anything the SMM core in edk2 does.
>
> Sorry I confused the two registers. 0xb3 is more or less unused as far
> as I can see indeed.
Thanks
Laszlo
- Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', (continued)
- Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Paolo Bonzini, 2016/11/16
- Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Laszlo Ersek, 2016/11/16
- Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-stable] [Qemu-devel] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Laszlo Ersek, 2016/11/17
- Re: [Qemu-stable] [Qemu-devel] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Michael S. Tsirkin, 2016/11/17
- Re: [Qemu-stable] [Qemu-devel] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Laszlo Ersek, 2016/11/17
- Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Laszlo Ersek, 2016/11/16
Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Laszlo Ersek, 2016/11/16
Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q', Michael S. Tsirkin, 2016/11/16