Re: [PATCH 1/2] x86: notify hypervisor about guest entering s2idle state

Sean Christopherson
2022-06-09T14:55:53Z

On Thu, Jun 09, 2022, Grzegorz Jaszczyk wrote:
> +9. KVM_HC_SYSTEM_S2IDLE
> +------------------------
> +
> +:Architecture: x86
> +:Status: active
> +:Purpose: Notify the hypervisor that the guest is entering s2idle state.

What about exiting s2idle? E.g.

1. VM0 enters s2idle
2. host notes that VM0 is in s2idle
3. VM0 exits s2idle
4. host still thinks VM0 is in s2idle
5. VM1 enters s2idle
6. host thinks all VMs are in s2idle, suspends the system

> +static void s2idle_hypervisor_notify(void)
> +{
> + if (static_cpu_has(X86_FEATURE_HYPERVISOR))
> + kvm_hypercall0(KVM_HC_SYSTEM_S2IDLE);

Checking the HYPERVISOR flag is not remotely sufficient. The hypervisor may not
be KVM, and if it is KVM, it may be an older version of KVM that doesn't support
the hypercall. The latter scenario won't be fatal unless KVM has been modified,
but blindly doing a hypercall for a different hypervisor could have disastrous
results, e.g. the registers ABIs are different, so the above will make a random
request depending on what is in other GPRs.

The bigger question is, why is KVM involved at all? KVM is just a dumb pipe out
to userspace, and not a very good one at that. There are multiple well established
ways to communicate with the VMM without custom hypercalls.

I bet if you're clever this can even be done without any guest changes, e.g. I
gotta imagine acpi_sleep_run_lps0_dsm() triggers MMIO/PIO with the right ACPI
configuration.