linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities
@ 2022-03-09 16:37 Mario Limonciello
  2022-03-10  9:43 ` Mika Westerberg
  2022-03-10 19:05 ` Rafael J. Wysocki
  0 siblings, 2 replies; 5+ messages in thread
From: Mario Limonciello @ 2022-03-09 16:37 UTC (permalink / raw)
  To: Rafael J . Wysocki, linux-acpi
  Cc: Mika Westerberg, Xiaomeng.Hou, Aaron.Liu, Ray.Huang, hdegoede,
	Mario Limonciello

According to the ACPI 6.4 spec:
It is strongly recommended that the OS evaluate _OSC with the Query
Support Flag set until _OSC returns the Capabilities Masked bit clear,
to negotiate the set of features to be granted to the OS for native
support; a platform may require a specific combination of features
to be supported natively by an OS before granting native control
of a given feature. After negotiation with the query flag set,
the OS should evaluate without it so that any negotiated values
can be made effective to hardware.

Currently the code sends the exact same values in both executions of the
_OSC and this leads to some problems on some AMD platforms in certain
configurations.

The following notable capabilities are set by OSPM when query is enabled:
* OSC_SB_PR3_SUPPORT
* OSC_SB_PCLPI_SUPPORT
* OSC_SB_NATIVE_USB4_SUPPORT

The first call to the platform OSC returns back a masked capabilities
error because the firmware did not acknowledge OSC_SB_PCLPI_SUPPORT but
it acknolwedged the others.

The second call to the platform _OSC without the query flag set then
fails because the OSPM still sent the exact same values.  This leads
to not acknowledging OSC_SB_NATIVE_USB4_SUPPORT and later USB4 PCIe
tunnels can't be authorized.

This problem was first introduced by commit 159d8c274fd9 ("ACPI: Pass the
same capabilities to the _OSC regardless of the query flag") which subtly
adjusted the behavior from 719e1f5 ("ACPI: Execute platform _OSC also
with query bit clear").

The _OSC was called exactly 2 times:
 * Once to query and request from firmware
 * Once to commit to firmware without query

To fix this problem, continue to call the _OSC until the firmware has
indicated that capabilities are no longer masked or after an arbitrary
number of negotiation attempts.

Furthermore, to avoid the problem that commit 159d8c274fd9 ("ACPI: Pass
the same capabilities to the _OSC regardless of the query flag")
introduced, explicitly mark support for CPC and CPPCv2 even if they
were masked by the series of query calls due to table loading order on
some systems.

Fixes: 159d8c274fd9 ("ACPI: Pass the same capabilities to the _OSC regardless of the query flag")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
This series was accepted but showed a regression in another use of acpi_run_osc
so the series was dropped.

Changes from v4->v5:
 * Move negotiation entirely into acpi_bus_osc_negotiate_platform_control
 drivers/acpi/bus.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index b96c54813886..86d88bd72c07 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -294,6 +294,7 @@ static void acpi_bus_osc_negotiate_platform_control(void)
 		.cap.pointer = capbuf,
 	};
 	acpi_handle handle;
+	int i;
 
 	capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
 	capbuf[OSC_SUPPORT_DWORD] = OSC_SB_PR3_SUPPORT; /* _PR3 is in use */
@@ -329,10 +330,34 @@ static void acpi_bus_osc_negotiate_platform_control(void)
 	if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
 		return;
 
-	if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
-		return;
+	/*
+	 * Check if bits were masked, we need to negotiate
+	 * prevent potential endless loop by limited number of
+	 * negotiation cycles.
+	 */
+	for (i = 0; i < 5; i++) {
+		bool retry = false;
+
+		if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
+			return;
+		capbuf_ret = context.ret.pointer;
+		retry = capbuf_ret[OSC_SUPPORT_DWORD] != capbuf[OSC_SUPPORT_DWORD];
+		capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD];
+		kfree(context.ret.pointer);
+		if (!retry)
+			break;
+	}
 
-	kfree(context.ret.pointer);
+	/*
+	 * Avoid problems with BIOS dynamically loading tables by indicating
+	 * support for CPPC even if it was masked.
+	 */
+#ifdef CONFIG_X86
+	if (boot_cpu_has(X86_FEATURE_HWP)) {
+		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
+		capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
+	}
+#endif
 
 	/* Now run _OSC again with query flag clear */
 	capbuf[OSC_QUERY_DWORD] = 0;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities
  2022-03-09 16:37 [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities Mario Limonciello
@ 2022-03-10  9:43 ` Mika Westerberg
  2022-03-10 19:05 ` Rafael J. Wysocki
  1 sibling, 0 replies; 5+ messages in thread
From: Mika Westerberg @ 2022-03-10  9:43 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Rafael J . Wysocki, linux-acpi, Xiaomeng.Hou, Aaron.Liu,
	Ray.Huang, hdegoede

On Wed, Mar 09, 2022 at 10:37:49AM -0600, Mario Limonciello wrote:
> According to the ACPI 6.4 spec:
> It is strongly recommended that the OS evaluate _OSC with the Query
> Support Flag set until _OSC returns the Capabilities Masked bit clear,
> to negotiate the set of features to be granted to the OS for native
> support; a platform may require a specific combination of features
> to be supported natively by an OS before granting native control
> of a given feature. After negotiation with the query flag set,
> the OS should evaluate without it so that any negotiated values
> can be made effective to hardware.
> 
> Currently the code sends the exact same values in both executions of the
> _OSC and this leads to some problems on some AMD platforms in certain
> configurations.
> 
> The following notable capabilities are set by OSPM when query is enabled:
> * OSC_SB_PR3_SUPPORT
> * OSC_SB_PCLPI_SUPPORT
> * OSC_SB_NATIVE_USB4_SUPPORT
> 
> The first call to the platform OSC returns back a masked capabilities
> error because the firmware did not acknowledge OSC_SB_PCLPI_SUPPORT but
> it acknolwedged the others.
> 
> The second call to the platform _OSC without the query flag set then
> fails because the OSPM still sent the exact same values.  This leads
> to not acknowledging OSC_SB_NATIVE_USB4_SUPPORT and later USB4 PCIe
> tunnels can't be authorized.
> 
> This problem was first introduced by commit 159d8c274fd9 ("ACPI: Pass the
> same capabilities to the _OSC regardless of the query flag") which subtly
> adjusted the behavior from 719e1f5 ("ACPI: Execute platform _OSC also
> with query bit clear").
> 
> The _OSC was called exactly 2 times:
>  * Once to query and request from firmware
>  * Once to commit to firmware without query
> 
> To fix this problem, continue to call the _OSC until the firmware has
> indicated that capabilities are no longer masked or after an arbitrary
> number of negotiation attempts.
> 
> Furthermore, to avoid the problem that commit 159d8c274fd9 ("ACPI: Pass
> the same capabilities to the _OSC regardless of the query flag")
> introduced, explicitly mark support for CPC and CPPCv2 even if they
> were masked by the series of query calls due to table loading order on
> some systems.
> 
> Fixes: 159d8c274fd9 ("ACPI: Pass the same capabilities to the _OSC regardless of the query flag")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> This series was accepted but showed a regression in another use of acpi_run_osc
> so the series was dropped.
> 
> Changes from v4->v5:
>  * Move negotiation entirely into acpi_bus_osc_negotiate_platform_control

Probably worth mentioning that the v5 fixes memory leak that was
reported.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities
  2022-03-09 16:37 [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities Mario Limonciello
  2022-03-10  9:43 ` Mika Westerberg
@ 2022-03-10 19:05 ` Rafael J. Wysocki
  2022-03-10 19:08   ` Limonciello, Mario
  1 sibling, 1 reply; 5+ messages in thread
From: Rafael J. Wysocki @ 2022-03-10 19:05 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Rafael J . Wysocki, ACPI Devel Maling List, Mika Westerberg,
	Xiaomeng.Hou, Aaron.Liu, Huang Rui, Hans de Goede

On Wed, Mar 9, 2022 at 5:46 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> According to the ACPI 6.4 spec:
> It is strongly recommended that the OS evaluate _OSC with the Query
> Support Flag set until _OSC returns the Capabilities Masked bit clear,
> to negotiate the set of features to be granted to the OS for native
> support; a platform may require a specific combination of features
> to be supported natively by an OS before granting native control
> of a given feature. After negotiation with the query flag set,
> the OS should evaluate without it so that any negotiated values
> can be made effective to hardware.
>
> Currently the code sends the exact same values in both executions of the
> _OSC and this leads to some problems on some AMD platforms in certain
> configurations.
>
> The following notable capabilities are set by OSPM when query is enabled:
> * OSC_SB_PR3_SUPPORT
> * OSC_SB_PCLPI_SUPPORT
> * OSC_SB_NATIVE_USB4_SUPPORT
>
> The first call to the platform OSC returns back a masked capabilities
> error because the firmware did not acknowledge OSC_SB_PCLPI_SUPPORT but
> it acknolwedged the others.
>
> The second call to the platform _OSC without the query flag set then
> fails because the OSPM still sent the exact same values.  This leads
> to not acknowledging OSC_SB_NATIVE_USB4_SUPPORT and later USB4 PCIe
> tunnels can't be authorized.
>
> This problem was first introduced by commit 159d8c274fd9 ("ACPI: Pass the
> same capabilities to the _OSC regardless of the query flag") which subtly
> adjusted the behavior from 719e1f5 ("ACPI: Execute platform _OSC also
> with query bit clear").
>
> The _OSC was called exactly 2 times:
>  * Once to query and request from firmware
>  * Once to commit to firmware without query
>
> To fix this problem, continue to call the _OSC until the firmware has
> indicated that capabilities are no longer masked or after an arbitrary
> number of negotiation attempts.
>
> Furthermore, to avoid the problem that commit 159d8c274fd9 ("ACPI: Pass
> the same capabilities to the _OSC regardless of the query flag")
> introduced, explicitly mark support for CPC and CPPCv2 even if they
> were masked by the series of query calls due to table loading order on
> some systems.
>
> Fixes: 159d8c274fd9 ("ACPI: Pass the same capabilities to the _OSC regardless of the query flag")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> This series was accepted but showed a regression in another use of acpi_run_osc
> so the series was dropped.
>
> Changes from v4->v5:
>  * Move negotiation entirely into acpi_bus_osc_negotiate_platform_control
>  drivers/acpi/bus.c | 31 ++++++++++++++++++++++++++++---
>  1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index b96c54813886..86d88bd72c07 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -294,6 +294,7 @@ static void acpi_bus_osc_negotiate_platform_control(void)
>                 .cap.pointer = capbuf,
>         };
>         acpi_handle handle;
> +       int i;
>
>         capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
>         capbuf[OSC_SUPPORT_DWORD] = OSC_SB_PR3_SUPPORT; /* _PR3 is in use */
> @@ -329,10 +330,34 @@ static void acpi_bus_osc_negotiate_platform_control(void)
>         if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
>                 return;
>
> -       if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> -               return;
> +       /*
> +        * Check if bits were masked, we need to negotiate
> +        * prevent potential endless loop by limited number of
> +        * negotiation cycles.
> +        */
> +       for (i = 0; i < 5; i++) {

Why 5 iterations?

Why cannot it work in analogy with the loop in acpi_pci_osc_control_set()?

> +               bool retry = false;
> +
> +               if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> +                       return;
> +               capbuf_ret = context.ret.pointer;
> +               retry = capbuf_ret[OSC_SUPPORT_DWORD] != capbuf[OSC_SUPPORT_DWORD];
> +               capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD];
> +               kfree(context.ret.pointer);
> +               if (!retry)
> +                       break;
> +       }
>
> -       kfree(context.ret.pointer);
> +       /*
> +        * Avoid problems with BIOS dynamically loading tables by indicating
> +        * support for CPPC even if it was masked.
> +        */
> +#ifdef CONFIG_X86
> +       if (boot_cpu_has(X86_FEATURE_HWP)) {
> +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> +       }
> +#endif
>
>         /* Now run _OSC again with query flag clear */
>         capbuf[OSC_QUERY_DWORD] = 0;
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities
  2022-03-10 19:05 ` Rafael J. Wysocki
@ 2022-03-10 19:08   ` Limonciello, Mario
  2022-03-10 19:13     ` Rafael J. Wysocki
  0 siblings, 1 reply; 5+ messages in thread
From: Limonciello, Mario @ 2022-03-10 19:08 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Rafael J . Wysocki, ACPI Devel Maling List, Mika Westerberg, Hou,
	Xiaomeng (Matthew),
	Liu, Aaron, Huang, Ray, Hans de Goede

[Public]

> >
> > According to the ACPI 6.4 spec:
> > It is strongly recommended that the OS evaluate _OSC with the Query
> > Support Flag set until _OSC returns the Capabilities Masked bit clear,
> > to negotiate the set of features to be granted to the OS for native
> > support; a platform may require a specific combination of features
> > to be supported natively by an OS before granting native control
> > of a given feature. After negotiation with the query flag set,
> > the OS should evaluate without it so that any negotiated values
> > can be made effective to hardware.
> >
> > Currently the code sends the exact same values in both executions of the
> > _OSC and this leads to some problems on some AMD platforms in certain
> > configurations.
> >
> > The following notable capabilities are set by OSPM when query is enabled:
> > * OSC_SB_PR3_SUPPORT
> > * OSC_SB_PCLPI_SUPPORT
> > * OSC_SB_NATIVE_USB4_SUPPORT
> >
> > The first call to the platform OSC returns back a masked capabilities
> > error because the firmware did not acknowledge OSC_SB_PCLPI_SUPPORT
> but
> > it acknolwedged the others.
> >
> > The second call to the platform _OSC without the query flag set then
> > fails because the OSPM still sent the exact same values.  This leads
> > to not acknowledging OSC_SB_NATIVE_USB4_SUPPORT and later USB4
> PCIe
> > tunnels can't be authorized.
> >
> > This problem was first introduced by commit 159d8c274fd9 ("ACPI: Pass the
> > same capabilities to the _OSC regardless of the query flag") which subtly
> > adjusted the behavior from 719e1f5 ("ACPI: Execute platform _OSC also
> > with query bit clear").
> >
> > The _OSC was called exactly 2 times:
> >  * Once to query and request from firmware
> >  * Once to commit to firmware without query
> >
> > To fix this problem, continue to call the _OSC until the firmware has
> > indicated that capabilities are no longer masked or after an arbitrary
> > number of negotiation attempts.
> >
> > Furthermore, to avoid the problem that commit 159d8c274fd9 ("ACPI: Pass
> > the same capabilities to the _OSC regardless of the query flag")
> > introduced, explicitly mark support for CPC and CPPCv2 even if they
> > were masked by the series of query calls due to table loading order on
> > some systems.
> >
> > Fixes: 159d8c274fd9 ("ACPI: Pass the same capabilities to the _OSC
> regardless of the query flag")
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> > This series was accepted but showed a regression in another use of
> acpi_run_osc
> > so the series was dropped.
> >
> > Changes from v4->v5:
> >  * Move negotiation entirely into
> acpi_bus_osc_negotiate_platform_control
> >  drivers/acpi/bus.c | 31 ++++++++++++++++++++++++++++---
> >  1 file changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index b96c54813886..86d88bd72c07 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -294,6 +294,7 @@ static void
> acpi_bus_osc_negotiate_platform_control(void)
> >                 .cap.pointer = capbuf,
> >         };
> >         acpi_handle handle;
> > +       int i;
> >
> >         capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
> >         capbuf[OSC_SUPPORT_DWORD] = OSC_SB_PR3_SUPPORT; /* _PR3 is
> in use */
> > @@ -329,10 +330,34 @@ static void
> acpi_bus_osc_negotiate_platform_control(void)
> >         if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> >                 return;
> >
> > -       if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > -               return;
> > +       /*
> > +        * Check if bits were masked, we need to negotiate
> > +        * prevent potential endless loop by limited number of
> > +        * negotiation cycles.
> > +        */
> > +       for (i = 0; i < 5; i++) {
> 
> Why 5 iterations?
> 
> Why cannot it work in analogy with the loop in acpi_pci_osc_control_set()?

5 was an arbitrary number selected just to guarantee that bad firmware couldn't
deadlock the negotiation.  It's admittedly unlikely, and if you would prefer I'll swap
over to an endless loop design like acpi_pci_osc_control_set.

> 
> > +               bool retry = false;
> > +
> > +               if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > +                       return;
> > +               capbuf_ret = context.ret.pointer;
> > +               retry = capbuf_ret[OSC_SUPPORT_DWORD] !=
> capbuf[OSC_SUPPORT_DWORD];
> > +               capbuf[OSC_SUPPORT_DWORD] =
> capbuf_ret[OSC_SUPPORT_DWORD];
> > +               kfree(context.ret.pointer);
> > +               if (!retry)
> > +                       break;
> > +       }
> >
> > -       kfree(context.ret.pointer);
> > +       /*
> > +        * Avoid problems with BIOS dynamically loading tables by indicating
> > +        * support for CPPC even if it was masked.
> > +        */
> > +#ifdef CONFIG_X86
> > +       if (boot_cpu_has(X86_FEATURE_HWP)) {
> > +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> > +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> > +       }
> > +#endif
> >
> >         /* Now run _OSC again with query flag clear */
> >         capbuf[OSC_QUERY_DWORD] = 0;
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities
  2022-03-10 19:08   ` Limonciello, Mario
@ 2022-03-10 19:13     ` Rafael J. Wysocki
  0 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2022-03-10 19:13 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Rafael J. Wysocki, Rafael J . Wysocki, ACPI Devel Maling List,
	Mika Westerberg, Hou, Xiaomeng (Matthew),
	Liu, Aaron, Huang, Ray, Hans de Goede

On Thu, Mar 10, 2022 at 8:09 PM Limonciello, Mario
<Mario.Limonciello@amd.com> wrote:
>
> [Public]
>
> > >
> > > According to the ACPI 6.4 spec:
> > > It is strongly recommended that the OS evaluate _OSC with the Query
> > > Support Flag set until _OSC returns the Capabilities Masked bit clear,
> > > to negotiate the set of features to be granted to the OS for native
> > > support; a platform may require a specific combination of features
> > > to be supported natively by an OS before granting native control
> > > of a given feature. After negotiation with the query flag set,
> > > the OS should evaluate without it so that any negotiated values
> > > can be made effective to hardware.
> > >
> > > Currently the code sends the exact same values in both executions of the
> > > _OSC and this leads to some problems on some AMD platforms in certain
> > > configurations.
> > >
> > > The following notable capabilities are set by OSPM when query is enabled:
> > > * OSC_SB_PR3_SUPPORT
> > > * OSC_SB_PCLPI_SUPPORT
> > > * OSC_SB_NATIVE_USB4_SUPPORT
> > >
> > > The first call to the platform OSC returns back a masked capabilities
> > > error because the firmware did not acknowledge OSC_SB_PCLPI_SUPPORT
> > but
> > > it acknolwedged the others.
> > >
> > > The second call to the platform _OSC without the query flag set then
> > > fails because the OSPM still sent the exact same values.  This leads
> > > to not acknowledging OSC_SB_NATIVE_USB4_SUPPORT and later USB4
> > PCIe
> > > tunnels can't be authorized.
> > >
> > > This problem was first introduced by commit 159d8c274fd9 ("ACPI: Pass the
> > > same capabilities to the _OSC regardless of the query flag") which subtly
> > > adjusted the behavior from 719e1f5 ("ACPI: Execute platform _OSC also
> > > with query bit clear").
> > >
> > > The _OSC was called exactly 2 times:
> > >  * Once to query and request from firmware
> > >  * Once to commit to firmware without query
> > >
> > > To fix this problem, continue to call the _OSC until the firmware has
> > > indicated that capabilities are no longer masked or after an arbitrary
> > > number of negotiation attempts.
> > >
> > > Furthermore, to avoid the problem that commit 159d8c274fd9 ("ACPI: Pass
> > > the same capabilities to the _OSC regardless of the query flag")
> > > introduced, explicitly mark support for CPC and CPPCv2 even if they
> > > were masked by the series of query calls due to table loading order on
> > > some systems.
> > >
> > > Fixes: 159d8c274fd9 ("ACPI: Pass the same capabilities to the _OSC
> > regardless of the query flag")
> > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > > ---
> > > This series was accepted but showed a regression in another use of
> > acpi_run_osc
> > > so the series was dropped.
> > >
> > > Changes from v4->v5:
> > >  * Move negotiation entirely into
> > acpi_bus_osc_negotiate_platform_control
> > >  drivers/acpi/bus.c | 31 ++++++++++++++++++++++++++++---
> > >  1 file changed, 28 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > index b96c54813886..86d88bd72c07 100644
> > > --- a/drivers/acpi/bus.c
> > > +++ b/drivers/acpi/bus.c
> > > @@ -294,6 +294,7 @@ static void
> > acpi_bus_osc_negotiate_platform_control(void)
> > >                 .cap.pointer = capbuf,
> > >         };
> > >         acpi_handle handle;
> > > +       int i;
> > >
> > >         capbuf[OSC_QUERY_DWORD] = OSC_QUERY_ENABLE;
> > >         capbuf[OSC_SUPPORT_DWORD] = OSC_SB_PR3_SUPPORT; /* _PR3 is
> > in use */
> > > @@ -329,10 +330,34 @@ static void
> > acpi_bus_osc_negotiate_platform_control(void)
> > >         if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> > >                 return;
> > >
> > > -       if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > > -               return;
> > > +       /*
> > > +        * Check if bits were masked, we need to negotiate
> > > +        * prevent potential endless loop by limited number of
> > > +        * negotiation cycles.
> > > +        */
> > > +       for (i = 0; i < 5; i++) {
> >
> > Why 5 iterations?
> >
> > Why cannot it work in analogy with the loop in acpi_pci_osc_control_set()?
>
> 5 was an arbitrary number selected just to guarantee that bad firmware couldn't
> deadlock the negotiation.  It's admittedly unlikely, and if you would prefer I'll swap
> over to an endless loop design like acpi_pci_osc_control_set.

It need not be endless.  Bits that are returned as clear can be
removed from the mask in the next iteration I think.

>
> >
> > > +               bool retry = false;
> > > +
> > > +               if (ACPI_FAILURE(acpi_run_osc(handle, &context)))
> > > +                       return;
> > > +               capbuf_ret = context.ret.pointer;
> > > +               retry = capbuf_ret[OSC_SUPPORT_DWORD] !=
> > capbuf[OSC_SUPPORT_DWORD];
> > > +               capbuf[OSC_SUPPORT_DWORD] =
> > capbuf_ret[OSC_SUPPORT_DWORD];
> > > +               kfree(context.ret.pointer);
> > > +               if (!retry)
> > > +                       break;
> > > +       }
> > >
> > > -       kfree(context.ret.pointer);
> > > +       /*
> > > +        * Avoid problems with BIOS dynamically loading tables by indicating
> > > +        * support for CPPC even if it was masked.
> > > +        */
> > > +#ifdef CONFIG_X86
> > > +       if (boot_cpu_has(X86_FEATURE_HWP)) {
> > > +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT;
> > > +               capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT;
> > > +       }
> > > +#endif
> > >
> > >         /* Now run _OSC again with query flag clear */
> > >         capbuf[OSC_QUERY_DWORD] = 0;
> > > --
> > > 2.34.1
> > >

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-10 19:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 16:37 [PATCH v5] ACPI: bus: For platform OSC negotiate capabilities Mario Limonciello
2022-03-10  9:43 ` Mika Westerberg
2022-03-10 19:05 ` Rafael J. Wysocki
2022-03-10 19:08   ` Limonciello, Mario
2022-03-10 19:13     ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).