| Message ID | 20250328143040.9348-1-ggherdovich@suse.cz (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [1/2] ACPI: processor: idle: Return an error if both P_LVL{2,3} idle states are invalid | expand |
On Fri, 2025-03-28 at 15:30 +0100, Giovanni Gherdovich wrote: > Prior to commit 496121c02127e9c460b436244c38260b044cc45a ("ACPI: > processor: > idle: Allow probing on platforms with one ACPI C-state"), the > acpi_idle > driver wouldn't load on systems without a valid C-State at least as > deep > as C2. The behavior was desirable for guests on hypervisors such as > VMWare > ESXi, which by default don't have the _CST ACPI method, and set the > C2 and > C3 latencies to 101 and 1001 microseconds respectively via the FADT, > to > signify they're unsupported. > > Since the above change though, these virtualized deployments end up > loading > acpi_idle, and thus entering the default C1 C-State set by > acpi_processor_get_power_info_default(); this is undesirable for a > system > that's communicating to the OS it doesn't want C-States (missing > _CST, and > invalid C2/C3 in FADT). > > Make acpi_processor_get_power_info_fadt() return ENODEV in that case, > so > that acpi_processor_get_cstate_info() exits early and doesn't set > pr->flags.power = 1. > > Fixes: 496121c02127 ("ACPI: processor: idle: Allow probing on > platforms with one ACPI C-state") > Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz> LGTM. Reviewed-by: Zhang Rui <rui.zhang@intel.com> -rui > --- > drivers/acpi/processor_idle.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/acpi/processor_idle.c > b/drivers/acpi/processor_idle.c > index 586cc7d1d8aa..b181f7fc2090 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -268,6 +268,10 @@ static int > acpi_processor_get_power_info_fadt(struct acpi_processor *pr) > ACPI_CX_DESC_LEN, "ACPI P_LVL3 IOPORT > 0x%x", > pr->power.states[ACPI_STATE_C3].address); > > + if (!pr->power.states[ACPI_STATE_C2].address && > + !pr->power.states[ACPI_STATE_C3].address) > + return -ENODEV; > + > return 0; > } >
On Mon, Mar 31, 2025 at 3:09 AM Zhang, Rui <rui.zhang@intel.com> wrote: > > On Fri, 2025-03-28 at 15:30 +0100, Giovanni Gherdovich wrote: > > Prior to commit 496121c02127e9c460b436244c38260b044cc45a ("ACPI: > > processor: > > idle: Allow probing on platforms with one ACPI C-state"), the > > acpi_idle > > driver wouldn't load on systems without a valid C-State at least as > > deep > > as C2. The behavior was desirable for guests on hypervisors such as > > VMWare > > ESXi, which by default don't have the _CST ACPI method, and set the > > C2 and > > C3 latencies to 101 and 1001 microseconds respectively via the FADT, > > to > > signify they're unsupported. > > > > Since the above change though, these virtualized deployments end up > > loading > > acpi_idle, and thus entering the default C1 C-State set by > > acpi_processor_get_power_info_default(); this is undesirable for a > > system > > that's communicating to the OS it doesn't want C-States (missing > > _CST, and > > invalid C2/C3 in FADT). > > > > Make acpi_processor_get_power_info_fadt() return ENODEV in that case, > > so > > that acpi_processor_get_cstate_info() exits early and doesn't set > > pr->flags.power = 1. > > > > Fixes: 496121c02127 ("ACPI: processor: idle: Allow probing on > > platforms with one ACPI C-state") > > Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz> > > LGTM. > > Reviewed-by: Zhang Rui <rui.zhang@intel.com> Applied as 6.15-rc material, thanks! > > --- > > drivers/acpi/processor_idle.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/acpi/processor_idle.c > > b/drivers/acpi/processor_idle.c > > index 586cc7d1d8aa..b181f7fc2090 100644 > > --- a/drivers/acpi/processor_idle.c > > +++ b/drivers/acpi/processor_idle.c > > @@ -268,6 +268,10 @@ static int > > acpi_processor_get_power_info_fadt(struct acpi_processor *pr) > > ACPI_CX_DESC_LEN, "ACPI P_LVL3 IOPORT > > 0x%x", > > pr->power.states[ACPI_STATE_C3].address); > > > > + if (!pr->power.states[ACPI_STATE_C2].address && > > + !pr->power.states[ACPI_STATE_C3].address) > > + return -ENODEV; > > + > > return 0; > > } > > >
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 586cc7d1d8aa..b181f7fc2090 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -268,6 +268,10 @@ static int acpi_processor_get_power_info_fadt(struct acpi_processor *pr) ACPI_CX_DESC_LEN, "ACPI P_LVL3 IOPORT 0x%x", pr->power.states[ACPI_STATE_C3].address); + if (!pr->power.states[ACPI_STATE_C2].address && + !pr->power.states[ACPI_STATE_C3].address) + return -ENODEV; + return 0; }
Prior to commit 496121c02127e9c460b436244c38260b044cc45a ("ACPI: processor: idle: Allow probing on platforms with one ACPI C-state"), the acpi_idle driver wouldn't load on systems without a valid C-State at least as deep as C2. The behavior was desirable for guests on hypervisors such as VMWare ESXi, which by default don't have the _CST ACPI method, and set the C2 and C3 latencies to 101 and 1001 microseconds respectively via the FADT, to signify they're unsupported. Since the above change though, these virtualized deployments end up loading acpi_idle, and thus entering the default C1 C-State set by acpi_processor_get_power_info_default(); this is undesirable for a system that's communicating to the OS it doesn't want C-States (missing _CST, and invalid C2/C3 in FADT). Make acpi_processor_get_power_info_fadt() return ENODEV in that case, so that acpi_processor_get_cstate_info() exits early and doesn't set pr->flags.power = 1. Fixes: 496121c02127 ("ACPI: processor: idle: Allow probing on platforms with one ACPI C-state") Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz> --- drivers/acpi/processor_idle.c | 4 ++++ 1 file changed, 4 insertions(+)