| Message ID | 20230209152123.3186930-2-jaz@semihalf.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | x86: allow to notify host about guest entering s2idle | expand |
On Thu, Feb 09 2023 at 15:21, Grzegorz Jaszczyk wrote: > + > +static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { What is AMD specific about this? > + .check = virt_pmc_s2idle_notify, > +}; ... > + > +MODULE_DESCRIPTION("Virtual PMC Driver"); Lacks MODULE_LICENSE("GPL") Thanks, tglx
[AMD Official Use Only - General] > -----Original Message----- > From: Grzegorz Jaszczyk <jaz@semihalf.com> > Sent: Thursday, February 9, 2023 09:21 > To: linux-kernel@vger.kernel.org; rafael@kernel.org > Cc: dmy@semihalf.com; tn@semihalf.com; dbehr@google.com; > zide.chen@intel.corp-partner.google.com; seanjc@google.com; > upstream@semihalf.com; hdegoede@redhat.com; markgross@kernel.org; > dtor@google.com; Limonciello, Mario <Mario.Limonciello@amd.com>; linux- > pm@vger.kernel.org; x86@kernel.org; platform-driver-x86@vger.kernel.org; > Grzegorz Jaszczyk <jaz@semihalf.com> > Subject: [RESEND RFCv2 1/1] platform/x86: Add virtual PMC driver used for > S2Idle > > Virtual PMC driver is meant for the guest VMs for the S2Idle > notification. Its purpose is to register S2Idle dev ops check handler, > which will evaluate ACPI _DSM just before the guest enters S2Idle power > state. > > This allows to trap on MMIO access done as a consequence of _DSM > evaluation and therefore notify the VMM about the guest entering S2Idle > state. > > Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com> > --- > Changelog v1..v2: > - Take advantage of acpi_s2idle_dev_ops's check() instead of notify() > --- > drivers/platform/x86/Kconfig | 7 ++++ > drivers/platform/x86/Makefile | 3 ++ > drivers/platform/x86/virt_pmc.c | 73 > +++++++++++++++++++++++++++++++++ > 3 files changed, 83 insertions(+) > create mode 100644 drivers/platform/x86/virt_pmc.c > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 5692385e2d26..b7c3f98031d7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1099,6 +1099,13 @@ config WINMATE_FM07_KEYS > buttons below the display. This module adds an input device > that delivers key events when these buttons are pressed. > > +config VIRT_PMC > + tristate "Virt PMC" > + depends on ACPI && SUSPEND > + help > + The Virtual PMC driver is meant for the guest VMs and its main > + purpose is to notify about guest entering s2idle state. > + > endif # X86_PLATFORM_DEVICES > > config P2SB > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index 1d3d1b02541b..c4d3056cf4ea 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -129,6 +129,9 @@ obj-$(CONFIG_INTEL_SCU_WDT) += > intel_scu_wdt.o > obj-$(CONFIG_INTEL_SCU_IPC_UTIL) += intel_scu_ipcutil.o > obj-$(CONFIG_X86_INTEL_LPSS) += pmc_atom.o > > +# Virtual PMC > +obj-$(CONFIG_VIRT_PMC) += virt_pmc.o > + > # Siemens Simatic Industrial PCs > obj-$(CONFIG_SIEMENS_SIMATIC_IPC) += simatic-ipc.o > > diff --git a/drivers/platform/x86/virt_pmc.c > b/drivers/platform/x86/virt_pmc.c > new file mode 100644 > index 000000000000..daf9c9ed86e5 > --- /dev/null > +++ b/drivers/platform/x86/virt_pmc.c > @@ -0,0 +1,73 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Virtual Power Management Controller Driver > + * > + * Author: Grzegorz Jaszczyk <jaz@semihalf.com> > + */ > + > +#include <linux/acpi.h> > +#include <linux/platform_device.h> > + > +#define ACPI_VIRT_PMC_DSM_UUID "9ea49ba3-434a-49a6-be30- > 37cc55c4d397" > +#define ACPI_VIRT_PMC_NOTIFY 1 > + > +static acpi_handle virt_pmc_handle; > + > +static void virt_pmc_s2idle_notify(void) > +{ > + union acpi_object *out_obj; > + static guid_t dsm_guid; > + > + guid_parse(ACPI_VIRT_PMC_DSM_UUID, &dsm_guid); > + > + out_obj = acpi_evaluate_dsm(virt_pmc_handle, &dsm_guid, > + 0, ACPI_VIRT_PMC_NOTIFY, NULL); > + > + acpi_handle_debug(virt_pmc_handle, "_DSM function %u > evaluation %s\n", > + ACPI_VIRT_PMC_NOTIFY, out_obj ? "successful" : > "failed"); > + > + ACPI_FREE(out_obj); > +} > + > +static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { > + .check = virt_pmc_s2idle_notify, > +}; > + > +static int virt_pmc_probe(struct platform_device *pdev) > +{ > + int err = 0; > + > + virt_pmc_handle = ACPI_HANDLE(&pdev->dev); > + > + err = acpi_register_lps0_dev(&amd_pmc_s2idle_dev_ops); > + if (err) > + dev_warn(&pdev->dev, "failed to register LPS0 sleep > handler\n"); > + Besides registering, I would think you also want to have a query to the _DSM to determine what functions are available. It could allow this to scale better if you're going to need to introduce a new function again later for a different reason. > + return err; > +} > + > +static int virt_pmc_remove(struct platform_device *pdev) > +{ > + acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); > + > + return 0; > +} > + > +static const struct acpi_device_id virt_pmc_acpi_ids[] = { > + {"HYPE0001", 0}, /* _HID for XXX Power Engine, _CID PNP0D80*/ > + { } > +}; > +MODULE_DEVICE_TABLE(acpi, virt_pmc_acpi_ids); > + > +static struct platform_driver virt_pmc_driver = { > + .driver = { > + .name = "virtual_pmc", > + .acpi_match_table = ACPI_PTR(virt_pmc_acpi_ids), > + }, > + .probe = virt_pmc_probe, > + .remove = virt_pmc_remove, > +}; > + > +module_platform_driver(virt_pmc_driver); > + > +MODULE_DESCRIPTION("Virtual PMC Driver"); > -- > 2.39.1.519.gcb327c4b5f-goog
On Thu, Feb 09, 2023, Grzegorz Jaszczyk wrote: > Virtual PMC driver is meant for the guest VMs for the S2Idle > notification. Its purpose is to register S2Idle dev ops check handler, > which will evaluate ACPI _DSM just before the guest enters S2Idle power > state. > > This allows to trap on MMIO access done as a consequence of _DSM > evaluation and therefore notify the VMM about the guest entering S2Idle > state. > > Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com> > --- > Changelog v1..v2: > - Take advantage of acpi_s2idle_dev_ops's check() instead of notify() > --- > drivers/platform/x86/Kconfig | 7 ++++ > drivers/platform/x86/Makefile | 3 ++ > drivers/platform/x86/virt_pmc.c | 73 +++++++++++++++++++++++++++++++++ > 3 files changed, 83 insertions(+) > create mode 100644 drivers/platform/x86/virt_pmc.c > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 5692385e2d26..b7c3f98031d7 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -1099,6 +1099,13 @@ config WINMATE_FM07_KEYS > buttons below the display. This module adds an input device > that delivers key events when these buttons are pressed. > > +config VIRT_PMC > + tristate "Virt PMC" Maybe spell out "Virtual Power Management Controller"? See below. > + depends on ACPI && SUSPEND I think it makes sense to take a dependency on HYPERVISOR_GUEST. It's not strictly required, but taking that dependency helps clarify that this is a guest-side thing, e.g. "virtual PMC" in KVM-land means "virtual performance monitoring counter". And IMO, disabling HYPERVISOR_GUEST should disable these type of guest-specific features.
czw., 9 lut 2023 o 19:05 Thomas Gleixner <tglx@linutronix.de> napisał(a): > > On Thu, Feb 09 2023 at 15:21, Grzegorz Jaszczyk wrote: > > + > > +static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { > > What is AMD specific about this? Nothing, my bad - I will remove the 'amd' prefix. > > > + .check = virt_pmc_s2idle_notify, > > +}; > > ... > > > + > > +MODULE_DESCRIPTION("Virtual PMC Driver"); > > Lacks MODULE_LICENSE("GPL") Sure, Thank you > > Thanks, > > tglx
czw., 9 lut 2023 o 19:25 Limonciello, Mario <Mario.Limonciello@amd.com> napisał(a): > > [AMD Official Use Only - General] > > > > > -----Original Message----- > > From: Grzegorz Jaszczyk <jaz@semihalf.com> > > Sent: Thursday, February 9, 2023 09:21 > > To: linux-kernel@vger.kernel.org; rafael@kernel.org > > Cc: dmy@semihalf.com; tn@semihalf.com; dbehr@google.com; > > zide.chen@intel.corp-partner.google.com; seanjc@google.com; > > upstream@semihalf.com; hdegoede@redhat.com; markgross@kernel.org; > > dtor@google.com; Limonciello, Mario <Mario.Limonciello@amd.com>; linux- > > pm@vger.kernel.org; x86@kernel.org; platform-driver-x86@vger.kernel.org; > > Grzegorz Jaszczyk <jaz@semihalf.com> > > Subject: [RESEND RFCv2 1/1] platform/x86: Add virtual PMC driver used for > > S2Idle > > > > Virtual PMC driver is meant for the guest VMs for the S2Idle > > notification. Its purpose is to register S2Idle dev ops check handler, > > which will evaluate ACPI _DSM just before the guest enters S2Idle power > > state. > > > > This allows to trap on MMIO access done as a consequence of _DSM > > evaluation and therefore notify the VMM about the guest entering S2Idle > > state. > > > > Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com> > > --- > > Changelog v1..v2: > > - Take advantage of acpi_s2idle_dev_ops's check() instead of notify() > > --- > > drivers/platform/x86/Kconfig | 7 ++++ > > drivers/platform/x86/Makefile | 3 ++ > > drivers/platform/x86/virt_pmc.c | 73 > > +++++++++++++++++++++++++++++++++ > > 3 files changed, 83 insertions(+) > > create mode 100644 drivers/platform/x86/virt_pmc.c > > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index 5692385e2d26..b7c3f98031d7 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -1099,6 +1099,13 @@ config WINMATE_FM07_KEYS > > buttons below the display. This module adds an input device > > that delivers key events when these buttons are pressed. > > > > +config VIRT_PMC > > + tristate "Virt PMC" > > + depends on ACPI && SUSPEND > > + help > > + The Virtual PMC driver is meant for the guest VMs and its main > > + purpose is to notify about guest entering s2idle state. > > + > > endif # X86_PLATFORM_DEVICES > > > > config P2SB > > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > > index 1d3d1b02541b..c4d3056cf4ea 100644 > > --- a/drivers/platform/x86/Makefile > > +++ b/drivers/platform/x86/Makefile > > @@ -129,6 +129,9 @@ obj-$(CONFIG_INTEL_SCU_WDT) += > > intel_scu_wdt.o > > obj-$(CONFIG_INTEL_SCU_IPC_UTIL) += intel_scu_ipcutil.o > > obj-$(CONFIG_X86_INTEL_LPSS) += pmc_atom.o > > > > +# Virtual PMC > > +obj-$(CONFIG_VIRT_PMC) += virt_pmc.o > > + > > # Siemens Simatic Industrial PCs > > obj-$(CONFIG_SIEMENS_SIMATIC_IPC) += simatic-ipc.o > > > > diff --git a/drivers/platform/x86/virt_pmc.c > > b/drivers/platform/x86/virt_pmc.c > > new file mode 100644 > > index 000000000000..daf9c9ed86e5 > > --- /dev/null > > +++ b/drivers/platform/x86/virt_pmc.c > > @@ -0,0 +1,73 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Virtual Power Management Controller Driver > > + * > > + * Author: Grzegorz Jaszczyk <jaz@semihalf.com> > > + */ > > + > > +#include <linux/acpi.h> > > +#include <linux/platform_device.h> > > + > > +#define ACPI_VIRT_PMC_DSM_UUID "9ea49ba3-434a-49a6-be30- > > 37cc55c4d397" > > +#define ACPI_VIRT_PMC_NOTIFY 1 > > + > > +static acpi_handle virt_pmc_handle; > > + > > +static void virt_pmc_s2idle_notify(void) > > +{ > > + union acpi_object *out_obj; > > + static guid_t dsm_guid; > > + > > + guid_parse(ACPI_VIRT_PMC_DSM_UUID, &dsm_guid); > > + > > + out_obj = acpi_evaluate_dsm(virt_pmc_handle, &dsm_guid, > > + 0, ACPI_VIRT_PMC_NOTIFY, NULL); > > + > > + acpi_handle_debug(virt_pmc_handle, "_DSM function %u > > evaluation %s\n", > > + ACPI_VIRT_PMC_NOTIFY, out_obj ? "successful" : > > "failed"); > > + > > + ACPI_FREE(out_obj); > > +} > > + > > +static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { > > + .check = virt_pmc_s2idle_notify, > > +}; > > + > > +static int virt_pmc_probe(struct platform_device *pdev) > > +{ > > + int err = 0; > > + > > + virt_pmc_handle = ACPI_HANDLE(&pdev->dev); > > + > > + err = acpi_register_lps0_dev(&amd_pmc_s2idle_dev_ops); > > + if (err) > > + dev_warn(&pdev->dev, "failed to register LPS0 sleep > > handler\n"); > > + > > Besides registering, I would think you also want to have a query to the > _DSM to determine what functions are available. It could allow this to > scale better if you're going to need to introduce a new function again > later for a different reason. > Ok, I will add such a query. Thanks
czw., 9 lut 2023 o 22:26 Sean Christopherson <seanjc@google.com> napisał(a): > > On Thu, Feb 09, 2023, Grzegorz Jaszczyk wrote: > > Virtual PMC driver is meant for the guest VMs for the S2Idle > > notification. Its purpose is to register S2Idle dev ops check handler, > > which will evaluate ACPI _DSM just before the guest enters S2Idle power > > state. > > > > This allows to trap on MMIO access done as a consequence of _DSM > > evaluation and therefore notify the VMM about the guest entering S2Idle > > state. > > > > Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com> > > --- > > Changelog v1..v2: > > - Take advantage of acpi_s2idle_dev_ops's check() instead of notify() > > --- > > drivers/platform/x86/Kconfig | 7 ++++ > > drivers/platform/x86/Makefile | 3 ++ > > drivers/platform/x86/virt_pmc.c | 73 +++++++++++++++++++++++++++++++++ > > 3 files changed, 83 insertions(+) > > create mode 100644 drivers/platform/x86/virt_pmc.c > > > > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > > index 5692385e2d26..b7c3f98031d7 100644 > > --- a/drivers/platform/x86/Kconfig > > +++ b/drivers/platform/x86/Kconfig > > @@ -1099,6 +1099,13 @@ config WINMATE_FM07_KEYS > > buttons below the display. This module adds an input device > > that delivers key events when these buttons are pressed. > > > > +config VIRT_PMC > > + tristate "Virt PMC" > > Maybe spell out "Virtual Power Management Controller"? See below. Sure, it makes sense to be more verbose here. > > > + depends on ACPI && SUSPEND > > I think it makes sense to take a dependency on HYPERVISOR_GUEST. It's not strictly > required, but taking that dependency helps clarify that this is a guest-side thing, > e.g. "virtual PMC" in KVM-land means "virtual performance monitoring counter". > > And IMO, disabling HYPERVISOR_GUEST should disable these type of guest-specific > features. Ok, thank you
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 5692385e2d26..b7c3f98031d7 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -1099,6 +1099,13 @@ config WINMATE_FM07_KEYS buttons below the display. This module adds an input device that delivers key events when these buttons are pressed. +config VIRT_PMC + tristate "Virt PMC" + depends on ACPI && SUSPEND + help + The Virtual PMC driver is meant for the guest VMs and its main + purpose is to notify about guest entering s2idle state. + endif # X86_PLATFORM_DEVICES config P2SB diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index 1d3d1b02541b..c4d3056cf4ea 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -129,6 +129,9 @@ obj-$(CONFIG_INTEL_SCU_WDT) += intel_scu_wdt.o obj-$(CONFIG_INTEL_SCU_IPC_UTIL) += intel_scu_ipcutil.o obj-$(CONFIG_X86_INTEL_LPSS) += pmc_atom.o +# Virtual PMC +obj-$(CONFIG_VIRT_PMC) += virt_pmc.o + # Siemens Simatic Industrial PCs obj-$(CONFIG_SIEMENS_SIMATIC_IPC) += simatic-ipc.o diff --git a/drivers/platform/x86/virt_pmc.c b/drivers/platform/x86/virt_pmc.c new file mode 100644 index 000000000000..daf9c9ed86e5 --- /dev/null +++ b/drivers/platform/x86/virt_pmc.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Virtual Power Management Controller Driver + * + * Author: Grzegorz Jaszczyk <jaz@semihalf.com> + */ + +#include <linux/acpi.h> +#include <linux/platform_device.h> + +#define ACPI_VIRT_PMC_DSM_UUID "9ea49ba3-434a-49a6-be30-37cc55c4d397" +#define ACPI_VIRT_PMC_NOTIFY 1 + +static acpi_handle virt_pmc_handle; + +static void virt_pmc_s2idle_notify(void) +{ + union acpi_object *out_obj; + static guid_t dsm_guid; + + guid_parse(ACPI_VIRT_PMC_DSM_UUID, &dsm_guid); + + out_obj = acpi_evaluate_dsm(virt_pmc_handle, &dsm_guid, + 0, ACPI_VIRT_PMC_NOTIFY, NULL); + + acpi_handle_debug(virt_pmc_handle, "_DSM function %u evaluation %s\n", + ACPI_VIRT_PMC_NOTIFY, out_obj ? "successful" : "failed"); + + ACPI_FREE(out_obj); +} + +static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { + .check = virt_pmc_s2idle_notify, +}; + +static int virt_pmc_probe(struct platform_device *pdev) +{ + int err = 0; + + virt_pmc_handle = ACPI_HANDLE(&pdev->dev); + + err = acpi_register_lps0_dev(&amd_pmc_s2idle_dev_ops); + if (err) + dev_warn(&pdev->dev, "failed to register LPS0 sleep handler\n"); + + return err; +} + +static int virt_pmc_remove(struct platform_device *pdev) +{ + acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); + + return 0; +} + +static const struct acpi_device_id virt_pmc_acpi_ids[] = { + {"HYPE0001", 0}, /* _HID for XXX Power Engine, _CID PNP0D80*/ + { } +}; +MODULE_DEVICE_TABLE(acpi, virt_pmc_acpi_ids); + +static struct platform_driver virt_pmc_driver = { + .driver = { + .name = "virtual_pmc", + .acpi_match_table = ACPI_PTR(virt_pmc_acpi_ids), + }, + .probe = virt_pmc_probe, + .remove = virt_pmc_remove, +}; + +module_platform_driver(virt_pmc_driver); + +MODULE_DESCRIPTION("Virtual PMC Driver");
Virtual PMC driver is meant for the guest VMs for the S2Idle notification. Its purpose is to register S2Idle dev ops check handler, which will evaluate ACPI _DSM just before the guest enters S2Idle power state. This allows to trap on MMIO access done as a consequence of _DSM evaluation and therefore notify the VMM about the guest entering S2Idle state. Signed-off-by: Grzegorz Jaszczyk <jaz@semihalf.com> --- Changelog v1..v2: - Take advantage of acpi_s2idle_dev_ops's check() instead of notify() --- drivers/platform/x86/Kconfig | 7 ++++ drivers/platform/x86/Makefile | 3 ++ drivers/platform/x86/virt_pmc.c | 73 +++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+) create mode 100644 drivers/platform/x86/virt_pmc.c