All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Laszlo Ersek <lersek@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	qemu devel list <qemu-devel@nongnu.org>,
	Michael Roth <mdroth@linux.vnet.ibm.com>,
	qemu-stable@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q'
Date: Wed, 16 Nov 2016 13:04:00 -0500 (EST)	[thread overview]
Message-ID: <771517296.13175228.1479319440822.JavaMail.zimbra@redhat.com> (raw)
In-Reply-To: <8f3f29f5-ddb3-5864-84e9-90af5aa72100@redhat.com>

> 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.  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

  reply	other threads:[~2016-11-16 18:05 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-15  1:50 [Qemu-devel] [PATCH v2] hw/isa/lpc_ich9: inject SMI on all VCPUs if APM_STS == 'Q' Laszlo Ersek
2016-11-15 13:59 ` Paolo Bonzini
2016-11-15 15:39   ` Laszlo Ersek
2016-11-15 15:45     ` Michael S. Tsirkin
2016-11-15 16:40       ` Laszlo Ersek
2016-11-16 12:47         ` Paolo Bonzini
2016-11-16 13:18           ` Michael S. Tsirkin
2016-11-16 14:05             ` Paolo Bonzini
2016-11-16 18:03               ` Laszlo Ersek
2016-11-16 20:27                 ` Michael S. Tsirkin
2016-11-17 13:16                   ` Laszlo Ersek
2016-11-17 17:46                     ` Michael S. Tsirkin
2016-11-17 18:45                       ` Laszlo Ersek
2016-11-16 17:56             ` Laszlo Ersek
2016-11-16 17:37           ` Laszlo Ersek
2016-11-16 18:04             ` Paolo Bonzini [this message]
2016-11-16 18:50               ` Laszlo Ersek
2016-11-16 20:38               ` Michael S. Tsirkin
2016-11-17  9:26                 ` Laszlo Ersek
2016-11-16 20:32             ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=771517296.13175228.1479319440822.JavaMail.zimbra@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=lersek@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.