[PATCH v2] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0

Jason Andryuk

author

2023-09-06 18:21 UTC #0 lore raw

PATCH v2 1/1 acpi/processor: sanitize _PDC buffer bits when running as Xen dom0

From: Roger Pau Monne <[email protected]>

The Processor _PDC buffer bits notify ACPI of the OS capabilities, and so ACPI can adjust the return of other Processor methods taking the OS capabilities into account.

When Linux is running as a Xen dom0, it's the hypervisor the entity in charge of processor power management, and hence Xen needs to make sure the capabilities reported in the _PDC buffer match the capabilities of the driver in Xen.

Introduce a small helper to sanitize the buffer when running as Xen dom0.

When Xen supports HWP, this serves as the equivalent of commit a21211672c9a ("ACPI / processor: Request native thermal interrupt handling via _OSC") to avoid SMM crashes. Xen will set bit 12 in the _PDC bits and the _PDC call will apply it.

[ jandryuk: Mention Xen HWP's need. Move local variables ]

Signed-off-by Roger Pau Monné <[email protected]>

Cc [email protected]

Signed-off-by Jason Andryuk <[email protected]>

---
v2:
Move local variables in acpi_processor_eval_pdc() to reuse in both conditions.

---
 arch/x86/include/asm/xen/hypervisor.h |  6 ++++++
 arch/x86/xen/enlighten.c              | 19 +++++++++++++++++++
 drivers/acpi/processor_pdc.c          | 22 ++++++++++++++++------
 3 files changed, 41 insertions(+), 6 deletions(-)

arch/x86/include/asm/xen/hypervisor.h +6 -0

index 5fc35f889cd1..0f88a7e450d3 100644
@@ -63,4 +63,10 @@ void __init xen_pvh_init(struct boot_params *boot_params);
 void __init mem_map_via_hcall(struct boot_params *boot_params_p);
 #endif
 
+#ifdef CONFIG_XEN_DOM0
+void xen_sanitize_pdc(uint32_t *buf);
+#else
+static inline void xen_sanitize_pdc(uint32_t *buf) { BUG(); }
+#endif
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */

arch/x86/xen/enlighten.c +19 -0

index b8db2148c07d..9f7fc11330a3 100644
@@ -346,3 +346,22 @@ void xen_arch_unregister_cpu(int num)
 }
 EXPORT_SYMBOL(xen_arch_unregister_cpu);
 #endif
+
+#ifdef CONFIG_XEN_DOM0
+void xen_sanitize_pdc(uint32_t *buf)
+{
+	struct xen_platform_op op = {
+		.cmd			= XENPF_set_processor_pminfo,
+		.interface_version	= XENPF_INTERFACE_VERSION,
+		.u.set_pminfo.id	= -1,
+		.u.set_pminfo.type	= XEN_PM_PDC,
+	};
+	int ret;
+
+	set_xen_guest_handle(op.u.set_pminfo.pdc, buf);
+	ret = HYPERVISOR_platform_op(&op);
+	if (ret)
+		pr_info("sanitize of _PDC buffer bits from Xen failed: %d\n",
+		        ret);
+}
+#endif

drivers/acpi/processor_pdc.c +16 -6

index 18fb04523f93..9393dd4a3158 100644
@@ -122,6 +122,11 @@ static acpi_status
 acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 {
 	acpi_status status = AE_OK;
+	union acpi_object *obj;
+	u32 *buffer = NULL;
+
+	obj = pdc_in->pointer;
+	buffer = (u32 *)(obj->buffer.pointer);
 
 	if (boot_option_idle_override == IDLE_NOMWAIT) {
 		/*

@@ -129,14 +134,19 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 		 * mode will be disabled in the parameter of _PDC object.
 		 * Of course C1_FFH access mode will also be disabled.
 		 */
-		union acpi_object *obj;
-		u32 *buffer = NULL;
-
-		obj = pdc_in->pointer;
-		buffer = (u32 *)(obj->buffer.pointer);
 		buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
-
 	}
+
+	if (xen_initial_domain()) {
+		/*
+		 * When Linux is running as Xen dom0, the hypervisor is the
+		 * entity in charge of the processor power management, and so
+		 * Xen needs to check the OS capabilities reported in the _PDC
+		 * buffer matches what the hypervisor driver supports.
+		 */
+		xen_sanitize_pdc(buffer);
+	}
+
 	status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
 
 	if (ACPI_FAILURE(status))

Signature · 1 line

2.41.0

kernel test robot

build bot

2023-09-06 23:10 UTC #1 lore raw

kernel test robot noticed the following build errors:

Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v6.5]
[cannot apply to rafael-pm/linux-next linus/master next-20230906] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Andryuk/acpi-processor-sanitize-_PDC-buffer-bits-when-running-as-Xen-dom0/20230907-022235
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20230906182125.48642-1-jandryuk%40gmail.com
patch subject: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0
config: x86_64-randconfig-r011-20230907 (https://download.01.org/0day-ci/archive/20230907/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230907/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/acpi/processor_pdc.c: In function 'acpi_processor_eval_pdc':

>> drivers/acpi/processor_pdc.c:147:17: error: implicit declaration of function 'xen_sanitize_pdc' [-Werror=implicit-function-declaration]
     147 |                 xen_sanitize_pdc(buffer);
         |                 ^~~~~~~~~~~~~~~~

cc1: some warnings being treated as errors

vim +/xen_sanitize_pdc +147 drivers/acpi/processor_pdc.c

116
117 /*
118 * _PDC is required for a BIOS-OS handshake for most of the newer
119 * ACPI processor features.
120 */
121 static acpi_status
122 acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
123 {
124 acpi_status status = AE_OK;
125 union acpi_object *obj;
126 u32 *buffer = NULL;
127
128 obj = pdc_in->pointer;
129 buffer = (u32 *)(obj->buffer.pointer);
130
131 if (boot_option_idle_override == IDLE_NOMWAIT) {
132 /*
133 * If mwait is disabled for CPU C-states, the C2C3_FFH access
134 * mode will be disabled in the parameter of _PDC object.
135 * Of course C1_FFH access mode will also be disabled.
136 */
137 buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
138 }
139
140 if (xen_initial_domain()) {
141 /*
142 * When Linux is running as Xen dom0, the hypervisor is the
143 * entity in charge of the processor power management, and so
144 * Xen needs to check the OS capabilities reported in the _PDC
145 * buffer matches what the hypervisor driver supports.
146 */

 > 147			xen_sanitize_pdc(buffer);

148 }
149
150 status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
151
152 if (ACPI_FAILURE(status))
153 acpi_handle_debug(handle,
154 "Could not evaluate _PDC, using legacy perf control\n");
155
156 return status;
157 }
158

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

kernel test robot

build bot

2023-09-07 00:13 UTC #2 lore raw

In reply to Jason Andryuk

kernel test robot noticed the following build errors:

Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v6.5]
[cannot apply to rafael-pm/linux-next linus/master next-20230906] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Andryuk/acpi-processor-sanitize-_PDC-buffer-bits-when-running-as-Xen-dom0/20230907-022235
base:   tip/x86/core
patch link:    https://lore.kernel.org/r/20230906182125.48642-1-jandryuk%40gmail.com
patch subject: [PATCH v2] acpi/processor: sanitize _PDC buffer bits when running as Xen dom0
config: i386-randconfig-003-20230907 (https://download.01.org/0day-ci/archive/20230907/[email protected]/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230907/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/acpi/processor_pdc.c:147:3: error: call to undeclared function 'xen_sanitize_pdc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                   xen_sanitize_pdc(buffer);
                   ^

1 error generated.

vim +/xen_sanitize_pdc +147 drivers/acpi/processor_pdc.c

116
117 /*
118 * _PDC is required for a BIOS-OS handshake for most of the newer
119 * ACPI processor features.
120 */
121 static acpi_status
122 acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
123 {
124 acpi_status status = AE_OK;
125 union acpi_object *obj;
126 u32 *buffer = NULL;
127
128 obj = pdc_in->pointer;
129 buffer = (u32 *)(obj->buffer.pointer);
130
131 if (boot_option_idle_override == IDLE_NOMWAIT) {
132 /*
133 * If mwait is disabled for CPU C-states, the C2C3_FFH access
134 * mode will be disabled in the parameter of _PDC object.
135 * Of course C1_FFH access mode will also be disabled.
136 */
137 buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
138 }
139
140 if (xen_initial_domain()) {
141 /*
142 * When Linux is running as Xen dom0, the hypervisor is the
143 * entity in charge of the processor power management, and so
144 * Xen needs to check the OS capabilities reported in the _PDC
145 * buffer matches what the hypervisor driver supports.
146 */

 > 147			xen_sanitize_pdc(buffer);

148 }
149
150 status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
151
152 if (ACPI_FAILURE(status))
153 acpi_handle_debug(handle,
154 "Could not evaluate _PDC, using legacy perf control\n");
155
156 return status;
157 }
158

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Wilczynski, Michal

2023-09-07 13:54 UTC #3 lore raw

In reply to Jason Andryuk

Hi,

Quoted 5 lines · Jason Andryuk

From: Roger Pau Monne <[email protected]>
The Processor _PDC buffer bits notify ACPI of the OS capabilities, and so ACPI can adjust the return of other Processor methods taking the OS capabilities into account.

_PDC method is deprecated for this purpose, since 2018, and is dropped from spec since 6.5

We made the switch in linux since 6.6:
95272641338a ("ACPI: processor: Use _OSC to convey OSPM processor support information")

Quoted 5 lines

When Linux is running as a Xen dom0, it's the hypervisor the entity in charge of processor power management, and hence Xen needs to make sure the capabilities reported in the _PDC buffer match the capabilities of the driver in Xen.

So I guess you would need to sanitize buffer passed to _OSC method instead ?

Quoted 106 lines

Introduce a small helper to sanitize the buffer when running as Xen dom0.

When Xen supports HWP, this serves as the equivalent of commit a21211672c9a ("ACPI / processor: Request native thermal interrupt handling via _OSC") to avoid SMM crashes. Xen will set bit 12 in the _PDC bits and the _PDC call will apply it.

[ jandryuk: Mention Xen HWP's need. Move local variables ]

Signed-off-by Roger Pau Monné <[email protected]>

Cc [email protected]

Signed-off-by Jason Andryuk <[email protected]>

---
v2:
Move local variables in acpi_processor_eval_pdc() to reuse in both conditions.

---
 arch/x86/include/asm/xen/hypervisor.h |  6 ++++++
 arch/x86/xen/enlighten.c              | 19 +++++++++++++++++++
 drivers/acpi/processor_pdc.c          | 22 ++++++++++++++++------
 3 files changed, 41 insertions(+), 6 deletions(-)

arch/x86/include/asm/xen/hypervisor.h +6 -0

index 5fc35f889cd1..0f88a7e450d3 100644
@@ -63,4 +63,10 @@ void __init xen_pvh_init(struct boot_params *boot_params);
 void __init mem_map_via_hcall(struct boot_params *boot_params_p);
 #endif
 
+#ifdef CONFIG_XEN_DOM0
+void xen_sanitize_pdc(uint32_t *buf);
+#else
+static inline void xen_sanitize_pdc(uint32_t *buf) { BUG(); }
+#endif
+
 #endif /* _ASM_X86_XEN_HYPERVISOR_H */

arch/x86/xen/enlighten.c +19 -0

index b8db2148c07d..9f7fc11330a3 100644
@@ -346,3 +346,22 @@ void xen_arch_unregister_cpu(int num)
 }
 EXPORT_SYMBOL(xen_arch_unregister_cpu);
 #endif
+
+#ifdef CONFIG_XEN_DOM0
+void xen_sanitize_pdc(uint32_t *buf)
+{
+	struct xen_platform_op op = {
+		.cmd			= XENPF_set_processor_pminfo,
+		.interface_version	= XENPF_INTERFACE_VERSION,
+		.u.set_pminfo.id	= -1,
+		.u.set_pminfo.type	= XEN_PM_PDC,
+	};
+	int ret;
+
+	set_xen_guest_handle(op.u.set_pminfo.pdc, buf);
+	ret = HYPERVISOR_platform_op(&op);
+	if (ret)
+		pr_info("sanitize of _PDC buffer bits from Xen failed: %d\n",
+		        ret);
+}
+#endif

drivers/acpi/processor_pdc.c +16 -6

index 18fb04523f93..9393dd4a3158 100644
@@ -122,6 +122,11 @@ static acpi_status
 acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 {
 	acpi_status status = AE_OK;
+	union acpi_object *obj;
+	u32 *buffer = NULL;
+
+	obj = pdc_in->pointer;
+	buffer = (u32 *)(obj->buffer.pointer);
 
 	if (boot_option_idle_override == IDLE_NOMWAIT) {
 		/*

@@ -129,14 +134,19 @@ acpi_processor_eval_pdc(acpi_handle handle, struct acpi_object_list *pdc_in)
 		 * mode will be disabled in the parameter of _PDC object.
 		 * Of course C1_FFH access mode will also be disabled.
 		 */
-		union acpi_object *obj;
-		u32 *buffer = NULL;
-
-		obj = pdc_in->pointer;
-		buffer = (u32 *)(obj->buffer.pointer);
 		buffer[2] &= ~(ACPI_PDC_C_C2C3_FFH | ACPI_PDC_C_C1_FFH);
-
 	}
+
+	if (xen_initial_domain()) {
+		/*
+		 * When Linux is running as Xen dom0, the hypervisor is the
+		 * entity in charge of the processor power management, and so
+		 * Xen needs to check the OS capabilities reported in the _PDC
+		 * buffer matches what the hypervisor driver supports.
+		 */
+		xen_sanitize_pdc(buffer);
+	}
+
 	status = acpi_evaluate_object(handle, "_PDC", pdc_in, NULL);
 
 	if (ACPI_FAILURE(status))

Jason Andryuk

author

2023-09-08 17:11 UTC #4 lore raw

Quoted 11 lines · Michal

Hi,
Quoted 5 lines · Jason Andryuk
From: Roger Pau Monne <[email protected]>
The Processor _PDC buffer bits notify ACPI of the OS capabilities, and so ACPI can adjust the return of other Processor methods taking the OS capabilities into account.

Quoted 5 lines

_PDC method is deprecated for this purpose, since 2018, and is dropped from spec since 6.5
We made the switch in linux since 6.6:
95272641338a ("ACPI: processor: Use _OSC to convey OSPM processor support information")

Thanks for the heads up, Michal. The patch pre-dated 6.6 and I based this one off of 6.5.

Quoted 7 lines

Quoted 5 lines
When Linux is running as a Xen dom0, it's the hypervisor the entity in charge of processor power management, and hence Xen needs to make sure the capabilities reported in the _PDC buffer match the capabilities of the driver in Xen.
So I guess you would need to sanitize buffer passed to _OSC method instead ?

I think I'll modify the capabilities in arch_acpi_set_proc_cap_bits() and that will handle both _OSC and the _PDC fallback.

Regards,
Jason