Re: [Qemu-stable] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q'

[Michael S. Tsirkin]

On Wed, Nov 16, 2016 at 01:04:00PM -0500, 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.
> 
> > 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? :)  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.

Indeed. Is an unconditional thing so bad?
What would be the observed behaviour with new OVMF on old QEMU?
Note you need to migrate during boot to notice this.

> Though we just got
> rid of one file for the number of CPUs, so I guess we might not care.
> 
> > 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.
> 
> Paolo