| Message ID | 20231101134154.18336-1-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | ebbf7f07007803c3095de2e632f3e1e3d5794712 |
| Headers | show |
| Series | [v4] acpi/processor: sanitize _OSC/_PDC capabilities for Xen dom0 | expand |
On 11/1/2023 2:41 PM, Jason Andryuk wrote: > From: Roger Pau Monne <roger.pau@citrix.com> > > The Processor capability 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, the hypervisor is the entity > in charge of processor power management, and hence Xen needs to make > sure the capabilities reported by _OSC/_PDC 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 > ACPI_PROC_CAP_COLLAB_PROC_PERF (bit 12) in the capability bits and the > _OSC/_PDC call will apply it. > > [ jandryuk: Mention Xen HWP's need. Support _OSC & _PDC ] > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: stable@vger.kernel.org > Signed-off-by: Jason Andryuk <jandryuk@gmail.com> > --- > v4: > Use xen_santize_proc_cap_bits() name - Michal > Rephrase comment - Michal > > v3: > Move xen_sanitize_pdc() call to arch_acpi_set_proc_cap_bits() to cover > _OSC and _PDC. > drivers/xen/pcpu.c is CONFIG_DOM0 && CONFIG_X86 > > v2: > Move local variables in acpi_processor_eval_pdc() to reuse in both conditions. > --- > arch/x86/include/asm/acpi.h | 14 ++++++++++++++ > arch/x86/include/asm/xen/hypervisor.h | 9 +++++++++ > drivers/xen/pcpu.c | 21 +++++++++++++++++++++ > 3 files changed, 44 insertions(+) > > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h > index c8a7fc23f63c..f896eed4516c 100644 > --- a/arch/x86/include/asm/acpi.h > +++ b/arch/x86/include/asm/acpi.h > @@ -16,6 +16,9 @@ > #include <asm/x86_init.h> > #include <asm/cpufeature.h> > #include <asm/irq_vectors.h> > +#include <asm/xen/hypervisor.h> > + > +#include <xen/xen.h> > > #ifdef CONFIG_ACPI_APEI > # include <asm/pgtable_types.h> > @@ -127,6 +130,17 @@ static inline void arch_acpi_set_proc_cap_bits(u32 *cap) > if (!cpu_has(c, X86_FEATURE_MWAIT) || > boot_option_idle_override == IDLE_NOMWAIT) > *cap &= ~(ACPI_PROC_CAP_C_C1_FFH | ACPI_PROC_CAP_C_C2C3_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 > + * processor capabilities buffer matches what the hypervisor > + * driver supports. > + */ > + xen_sanitize_proc_cap_bits(cap); > + } > } > > static inline bool acpi_has_cpu_in_madt(void) > diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h > index 7048dfacc04b..a9088250770f 100644 > --- a/arch/x86/include/asm/xen/hypervisor.h > +++ b/arch/x86/include/asm/xen/hypervisor.h > @@ -100,4 +100,13 @@ static inline void leave_lazy(enum xen_lazy_mode mode) > > enum xen_lazy_mode xen_get_lazy_mode(void); > > +#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) > +void xen_sanitize_proc_cap_bits(uint32_t *buf); > +#else > +static inline void xen_sanitize_proc_cap_bits(uint32_t *buf) > +{ > + BUG(); > +} > +#endif > + > #endif /* _ASM_X86_XEN_HYPERVISOR_H */ > diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c > index b3e3d1bb37f3..7000701dff8f 100644 > --- a/drivers/xen/pcpu.c > +++ b/drivers/xen/pcpu.c > @@ -47,6 +47,9 @@ > #include <asm/xen/hypervisor.h> > #include <asm/xen/hypercall.h> > > +#ifdef CONFIG_ACPI > +#include <acpi/processor.h> > +#endif > > /* > * @cpu_id: Xen physical cpu logic number > @@ -400,4 +403,22 @@ bool __init xen_processor_present(uint32_t acpi_id) > > return online; > } > + > +void xen_sanitize_proc_cap_bits(uint32_t *cap) > +{ > + struct xen_platform_op op = { > + .cmd = XENPF_set_processor_pminfo, > + .u.set_pminfo.id = -1, > + .u.set_pminfo.type = XEN_PM_PDC, > + }; > + u32 buf[3] = { ACPI_PDC_REVISION_ID, 1, *cap }; > + int ret; > + > + set_xen_guest_handle(op.u.set_pminfo.pdc, buf); > + ret = HYPERVISOR_platform_op(&op); > + if (ret) > + pr_err("sanitize of _PDC buffer bits from Xen failed: %d\n", > + ret); > + *cap = buf[2]; > +} > #endif Looks good to me. For what it's worth - Reviewed-by: Michal Wilczynski <michal.wilczynski@intel.com>
On 01.11.23 14:41, Jason Andryuk wrote: > From: Roger Pau Monne <roger.pau@citrix.com> > > The Processor capability 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, the hypervisor is the entity > in charge of processor power management, and hence Xen needs to make > sure the capabilities reported by _OSC/_PDC 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 > ACPI_PROC_CAP_COLLAB_PROC_PERF (bit 12) in the capability bits and the > _OSC/_PDC call will apply it. > > [ jandryuk: Mention Xen HWP's need. Support _OSC & _PDC ] > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: stable@vger.kernel.org > Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On Wed, Nov 01, 2023 at 09:41:52AM -0400, Jason Andryuk wrote: > From: Roger Pau Monne <roger.pau@citrix.com> > > The Processor capability 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, the hypervisor is the entity > in charge of processor power management, and hence Xen needs to make > sure the capabilities reported by _OSC/_PDC 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 > ACPI_PROC_CAP_COLLAB_PROC_PERF (bit 12) in the capability bits and the > _OSC/_PDC call will apply it. > > [ jandryuk: Mention Xen HWP's need. Support _OSC & _PDC ] > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: stable@vger.kernel.org > Signed-off-by: Jason Andryuk <jandryuk@gmail.com> > --- > v4: > Use xen_santize_proc_cap_bits() name - Michal > Rephrase comment - Michal > > v3: > Move xen_sanitize_pdc() call to arch_acpi_set_proc_cap_bits() to cover > _OSC and _PDC. > drivers/xen/pcpu.c is CONFIG_DOM0 && CONFIG_X86 > > v2: > Move local variables in acpi_processor_eval_pdc() to reuse in both conditions. > --- > arch/x86/include/asm/acpi.h | 14 ++++++++++++++ > arch/x86/include/asm/xen/hypervisor.h | 9 +++++++++ > drivers/xen/pcpu.c | 21 +++++++++++++++++++++ > 3 files changed, 44 insertions(+) > > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h > index c8a7fc23f63c..f896eed4516c 100644 > --- a/arch/x86/include/asm/acpi.h > +++ b/arch/x86/include/asm/acpi.h > @@ -16,6 +16,9 @@ > #include <asm/x86_init.h> > #include <asm/cpufeature.h> > #include <asm/irq_vectors.h> > +#include <asm/xen/hypervisor.h> > + > +#include <xen/xen.h> > > #ifdef CONFIG_ACPI_APEI > # include <asm/pgtable_types.h> > @@ -127,6 +130,17 @@ static inline void arch_acpi_set_proc_cap_bits(u32 *cap) > if (!cpu_has(c, X86_FEATURE_MWAIT) || > boot_option_idle_override == IDLE_NOMWAIT) > *cap &= ~(ACPI_PROC_CAP_C_C1_FFH | ACPI_PROC_CAP_C_C2C3_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 > + * processor capabilities buffer matches what the hypervisor > + * driver supports. > + */ > + xen_sanitize_proc_cap_bits(cap); > + } > } > > static inline bool acpi_has_cpu_in_madt(void) > diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h > index 7048dfacc04b..a9088250770f 100644 > --- a/arch/x86/include/asm/xen/hypervisor.h > +++ b/arch/x86/include/asm/xen/hypervisor.h > @@ -100,4 +100,13 @@ static inline void leave_lazy(enum xen_lazy_mode mode) > > enum xen_lazy_mode xen_get_lazy_mode(void); > > +#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) > +void xen_sanitize_proc_cap_bits(uint32_t *buf); > +#else > +static inline void xen_sanitize_proc_cap_bits(uint32_t *buf) > +{ > + BUG(); > +} > +#endif > + > #endif /* _ASM_X86_XEN_HYPERVISOR_H */ > diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c > index b3e3d1bb37f3..7000701dff8f 100644 > --- a/drivers/xen/pcpu.c > +++ b/drivers/xen/pcpu.c > @@ -47,6 +47,9 @@ > #include <asm/xen/hypervisor.h> > #include <asm/xen/hypercall.h> > > +#ifdef CONFIG_ACPI > +#include <acpi/processor.h> > +#endif > > /* > * @cpu_id: Xen physical cpu logic number > @@ -400,4 +403,22 @@ bool __init xen_processor_present(uint32_t acpi_id) > > return online; > } > + > +void xen_sanitize_proc_cap_bits(uint32_t *cap) > +{ > + struct xen_platform_op op = { > + .cmd = XENPF_set_processor_pminfo, > + .u.set_pminfo.id = -1, > + .u.set_pminfo.type = XEN_PM_PDC, > + }; > + u32 buf[3] = { ACPI_PDC_REVISION_ID, 1, *cap }; > + int ret; > + > + set_xen_guest_handle(op.u.set_pminfo.pdc, buf); > + ret = HYPERVISOR_platform_op(&op); > + if (ret) > + pr_err("sanitize of _PDC buffer bits from Xen failed: %d\n", > + ret); > + *cap = buf[2]; FWIW, we might want to only update cap if the hypercall has been successful, otherwise there's no guarantee of what's in the buffer, so I would recommend to put the updating of cap in an else branch. Anyway, not a strong opinion, as I think in practice even if the hypercall fails it wouldn't corrupt the data in the buffer, but seems more robust. Thanks, Roger.
diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h index c8a7fc23f63c..f896eed4516c 100644 --- a/arch/x86/include/asm/acpi.h +++ b/arch/x86/include/asm/acpi.h @@ -16,6 +16,9 @@ #include <asm/x86_init.h> #include <asm/cpufeature.h> #include <asm/irq_vectors.h> +#include <asm/xen/hypervisor.h> + +#include <xen/xen.h> #ifdef CONFIG_ACPI_APEI # include <asm/pgtable_types.h> @@ -127,6 +130,17 @@ static inline void arch_acpi_set_proc_cap_bits(u32 *cap) if (!cpu_has(c, X86_FEATURE_MWAIT) || boot_option_idle_override == IDLE_NOMWAIT) *cap &= ~(ACPI_PROC_CAP_C_C1_FFH | ACPI_PROC_CAP_C_C2C3_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 + * processor capabilities buffer matches what the hypervisor + * driver supports. + */ + xen_sanitize_proc_cap_bits(cap); + } } static inline bool acpi_has_cpu_in_madt(void) diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h index 7048dfacc04b..a9088250770f 100644 --- a/arch/x86/include/asm/xen/hypervisor.h +++ b/arch/x86/include/asm/xen/hypervisor.h @@ -100,4 +100,13 @@ static inline void leave_lazy(enum xen_lazy_mode mode) enum xen_lazy_mode xen_get_lazy_mode(void); +#if defined(CONFIG_XEN_DOM0) && defined(CONFIG_ACPI) +void xen_sanitize_proc_cap_bits(uint32_t *buf); +#else +static inline void xen_sanitize_proc_cap_bits(uint32_t *buf) +{ + BUG(); +} +#endif + #endif /* _ASM_X86_XEN_HYPERVISOR_H */ diff --git a/drivers/xen/pcpu.c b/drivers/xen/pcpu.c index b3e3d1bb37f3..7000701dff8f 100644 --- a/drivers/xen/pcpu.c +++ b/drivers/xen/pcpu.c @@ -47,6 +47,9 @@ #include <asm/xen/hypervisor.h> #include <asm/xen/hypercall.h> +#ifdef CONFIG_ACPI +#include <acpi/processor.h> +#endif /* * @cpu_id: Xen physical cpu logic number @@ -400,4 +403,22 @@ bool __init xen_processor_present(uint32_t acpi_id) return online; } + +void xen_sanitize_proc_cap_bits(uint32_t *cap) +{ + struct xen_platform_op op = { + .cmd = XENPF_set_processor_pminfo, + .u.set_pminfo.id = -1, + .u.set_pminfo.type = XEN_PM_PDC, + }; + u32 buf[3] = { ACPI_PDC_REVISION_ID, 1, *cap }; + int ret; + + set_xen_guest_handle(op.u.set_pminfo.pdc, buf); + ret = HYPERVISOR_platform_op(&op); + if (ret) + pr_err("sanitize of _PDC buffer bits from Xen failed: %d\n", + ret); + *cap = buf[2]; +} #endif