Re: [PATCH v2 3/9] acpi/core: always set SCI_EN when SMM isn't supported

[Igor Mammedov]

On Tue, 9 Feb 2021 11:23:05 -0800
Isaku Yamahata <yamahata.qemudev@gmail.com> wrote:

> On Tue, Feb 09, 2021 at 04:05:14PM +0100,
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
> > On Mon,  8 Feb 2021 13:57:22 -0800
> > isaku.yamahata@gmail.com wrote:
> >   
> > > From: Isaku Yamahata <isaku.yamahata@intel.com>
> > > 
> > > If SMM is not supported, ACPI fixed hardware doesn't support
> > > legacy-mode. ACPI-only platform. Where SCI_EN in PM1_CNT register is
> > > always set.
> > > The bit tells OS legacy mode(SCI_EN cleared) or ACPI mode(SCI_EN set).  
> > 
> > does it break some specific software?  
> 
> With the next patch (setting fadt.smi_cmd = 0 when smm isn't supported),
> guest Linux tries to switch to ACPI mode, finds smi_cmd = 0, and then
> fails to initialize acpi subsystem.
> will update the commit message in next respin.
> 
> 
> > > ACPI spec 4.8.10.1 PM1 Event Grouping
> > > PM1 Eanble Registers  
> > > > For ACPI-only platforms (where SCI_EN is always set)    
> > > 
> > > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>  
> > it changes guest ABI for old machine types but it seems to me that
> > it's harmless (in typical use-cases backward and forward migrated
> > guest should work fine).
> > 
> > The only thing that is broken is transitioning to legacy mode
> > when guest was started on old QEMU and then migrated to the new one
> > where disable op will be NOP and qemu always stays in ACPI mode
> > (so guest will hang while it waits for transition to happen).  
> 
> The patch affects guests only when SMM isn't supported.
> Concretely
> - user explicitly specified to disable smm by -machine smm=off
> or
> - underlying kvm doesn't have KVM_CAP_X86_SMM (smm=auto: default)
> Please refer to x86_machine_is_smm_enabled().
> Also Libvirt checks if guest bios requires SMI and enables smm even
> when user disabling SMM. qemuFirmwareEnableFeatures()


> If smm is disabled and legacy-mode is enabled without this patch,
> SMI won't be injected to guest anyway. Thus guest breaks already.
can you point to code in QEMU that prevents SMI being triggered?
(what see is QEMU faking SMM being configured when smm_enable=false,
and seabios skipping its smm code based on that, other guest code
may behave differently though).

But that's beside point, guest started on old QEMU may believe that
it runs on Legacy/ACPI platform even with (smm=off), and hence can try
to enable/disable ACPI mode explicitly. Combined with migration to QEMU
with this patches it might turn into problem (which is hard to
troubleshoot in the wild).


> > Can you test this scenario with various guest OSes (old/new/MS Windows)
> > to check if it won't break them.  
> 
> Unless -machine smm=off is explicitly passed, this patch won't break
> guests. And such case is rare as I described above.

yes, it's a corner case but it doesn't guarantee that someone isn't using
it it in the wild.

> My motivation for this patch series is preparation for TDX which disallows
> SMM mode and SMI injection.
> 
> 
> > if we are to be conservative, we need to disable this compliance fix
> > on old machine types.  
> 
> I'm fine with adding one more knob to be on safe side.
> -machine smm=off is such knob, though.

you can use 2ebc21216f as an example of compat knob that changes behavior
depending on machine version. (i.e. to keep old behavior in piix4/ich9 on
exiting machine versions)

> Thanks,