Hi Jonathan,
> -----Original Message-----
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Sent: Friday, December 15, 2023 5:12 PM
> To: Russell King (Oracle) <rmk+kernel@xxxxxxxxxxxxxxx>
> Cc: linux-pm@xxxxxxxxxxxxxxx; loongarch@xxxxxxxxxxxxxxx; linux-
> acpi@xxxxxxxxxxxxxxx; linux-arch@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> riscv@xxxxxxxxxxxxxxxxxxx; kvmarm@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx; acpica-
> devel@xxxxxxxxxxxxxxxxxxxxxxxxx; linux-csky@xxxxxxxxxxxxxxx; linux-
> doc@xxxxxxxxxxxxxxx; linux-ia64@xxxxxxxxxxxxxxx; linux-
> parisc@xxxxxxxxxxxxxxx; Salil Mehta <salil.mehta@xxxxxxxxxx>; Jean-Philippe
> Brucker <jean-philippe@xxxxxxxxxx>; Jianyong Wu <Jianyong.Wu@xxxxxxx>;
> Justin He <Justin.He@xxxxxxx>; James Morse <James.Morse@xxxxxxx>;
> Jose Marinho <Jose.Marinho@xxxxxxx>; Samer El-Haj-Mahmoud <Samer.El-
> Haj-Mahmoud@xxxxxxx>
> Subject: Re: [PATCH RFC v3 20/21] ACPI: Add _OSC bits to advertise OS
> support for toggling CPU present/enabled
>
> On Wed, 13 Dec 2023 12:50:54 +0000
> Russell King (Oracle) <rmk+kernel@xxxxxxxxxxxxxxx> wrote:
>
> > From: James Morse <james.morse@xxxxxxx>
> >
> > Platform firmware can disabled a CPU, or make it not-present by making
> > an eject-request notification, then waiting for the os to make it
> > offline
> OS
>
> > and call _EJx. After the firmware updates _STA with the new status.
> >
> > Not all operating systems support this. For arm64 making CPUs
> > not-present has never been supported. For all ACPI architectures,
> > making CPUs disabled has recently been added. Firmware can't know what
> the OS has support for.
> >
> > Add two new _OSC bits to advertise whether the OS supports the _STA
> > enabled or present bits being toggled for CPUs. This will be important
> > for arm64 if systems that support physical CPU hotplug ever appear as
> > arm64 linux doesn't currently support this, so firmware shouldn't try.
> >
> > Advertising this support to firmware is useful for cloud orchestrators
> > to know whether they can scale a particular VM by adding CPUs.
> >
> > Signed-off-by: James Morse <james.morse@xxxxxxx>
> > Tested-by: Miguel Luis <miguel.luis@xxxxxxxxxx>
> > Tested-by: Vishnu Pajjuri <vishnu@xxxxxxxxxxxxxxxxxxxxxx>
> > Tested-by: Jianyong Wu <jianyong.wu@xxxxxxx>
>
> I'm very much in favor of this _OSC but it hasn't been accepted yet I think...
> https://bugzilla.tianocore.org/show_bug.cgi?id=4481
>
> Jose? Github suggests you are the proposer on this.
The addition of these _OSC bits was proposed by us on the forum in question.
The forum opted to pause the definition until additional practical information could be provided on the use-cases.
If anyone is interested in progressing the _OSC bit definition, you are invited to express that interest in the Bugzilla ticket.
Information that you should provide to increase the chances of the ticket being reopened:
- use-case for the new _OSC bits,
- what breaks (if anything) without the proposed _OSC bits.
We did receive additional comments:
- the proposed _OSC bits are not generic: the bits simply convey whether the guest OS understands CPU hot-plug, but it says nothing about the number of CPUs that the OS supports.
- There could be alternate schemes that do not rely on spec changes. E.g. there could be a hypervisor IMPDEF mechanism to describe if an OS image supports CPU hot-plug.
>
> btw v4 looks ok but v5 in the tianocore github seems to have lost the actual
> OSC part.
Agree that, if we do progress with this spec change, v4 is the correct formulation we should adopt.
Regards,
Jose
>
> Jonathan
>
> > ---
> > I'm assuming Loongarch machines do not support physical CPU hotplug.
> >
> > Changes since RFC v3:
> > * Drop ia64 changes
> > * Update James' comment below "---" to remove reference to ia64
> >
> > Outstanding comment:
> > https://lore.kernel.org/r/20230914175021.000018fd@xxxxxxxxxx
>
>
>
> > ---
> > arch/x86/Kconfig | 1 +
> > drivers/acpi/Kconfig | 9 +++++++++
> > drivers/acpi/acpi_processor.c | 14 +++++++++++++-
> > drivers/acpi/bus.c | 16 ++++++++++++++++
> > include/linux/acpi.h | 4 ++++
> > 5 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index
> > 64fc7c475ab0..33fc4dcd950c 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -60,6 +60,7 @@ config X86
> > select ACPI_LEGACY_TABLES_LOOKUP if ACPI
> > select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
> > select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR
> && HOTPLUG_CPU
> > + select ACPI_HOTPLUG_IGNORE_OSC if ACPI &&
> HOTPLUG_CPU
> > select ARCH_32BIT_OFF_T if X86_32
> > select ARCH_CLOCKSOURCE_INIT
> > select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index
> > 9c5a43d0aff4..020e7c0ab985 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -311,6 +311,15 @@ config ACPI_HOTPLUG_PRESENT_CPU
> > depends on ACPI_PROCESSOR && HOTPLUG_CPU
> > select ACPI_CONTAINER
> >
> > +config ACPI_HOTPLUG_IGNORE_OSC
> > + bool
> > + depends on ACPI_HOTPLUG_PRESENT_CPU
> > + help
> > + Ignore whether firmware acknowledged support for toggling the CPU
> > + present bit in _STA. Some architectures predate the _OSC bits, so
> > + firmware doesn't know to do this.
> > +
> > +
> > config ACPI_PROCESSOR_AGGREGATOR
> > tristate "Processor Aggregator"
> > depends on ACPI_PROCESSOR
> > diff --git a/drivers/acpi/acpi_processor.c
> > b/drivers/acpi/acpi_processor.c index ea12e70dfd39..5bb207a7a1dd
> > 100644
> > --- a/drivers/acpi/acpi_processor.c
> > +++ b/drivers/acpi/acpi_processor.c
> > @@ -182,6 +182,18 @@ static void __init acpi_pcc_cpufreq_init(void)
> > static void __init acpi_pcc_cpufreq_init(void) {} #endif /*
> > CONFIG_X86 */
> >
> > +static bool acpi_processor_hotplug_present_supported(void)
> > +{
> > + if (!IS_ENABLED(CONFIG_ACPI_HOTPLUG_PRESENT_CPU))
> > + return false;
> > +
> > + /* x86 systems pre-date the _OSC bit */
> > + if (IS_ENABLED(CONFIG_ACPI_HOTPLUG_IGNORE_OSC))
> > + return true;
> > +
> > + return osc_sb_hotplug_present_support_acked;
> > +}
> > +
> > /* Initialization */
> > static int acpi_processor_make_present(struct acpi_processor *pr) {
> > @@ -189,7 +201,7 @@ static int acpi_processor_make_present(struct
> acpi_processor *pr)
> > acpi_status status;
> > int ret;
> >
> > - if (!IS_ENABLED(CONFIG_ACPI_HOTPLUG_PRESENT_CPU)) {
> > + if (!acpi_processor_hotplug_present_supported()) {
> > pr_err_once("Changing CPU present bit is not supported\n");
> > return -ENODEV;
> > }
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index
> > 72e64c0718c9..7122450739d6 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -298,6 +298,13 @@
> > EXPORT_SYMBOL_GPL(osc_sb_native_usb4_support_confirmed);
> >
> > bool osc_sb_cppc2_support_acked;
> >
> > +/*
> > + * ACPI 6.? Proposed Operating System Capabilities for modifying CPU
> > + * present/enable.
> > + */
> > +bool osc_sb_hotplug_enabled_support_acked;
> > +bool osc_sb_hotplug_present_support_acked;
> > +
> > static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48";
> > static void acpi_bus_osc_negotiate_platform_control(void)
> > {
> > @@ -346,6 +353,11 @@ static void
> > acpi_bus_osc_negotiate_platform_control(void)
> >
> > if (!ghes_disable)
> > capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT;
> > +
> > + capbuf[OSC_SUPPORT_DWORD] |=
> OSC_SB_HOTPLUG_ENABLED_SUPPORT;
> > + if (IS_ENABLED(CONFIG_ACPI_HOTPLUG_PRESENT_CPU))
> > + capbuf[OSC_SUPPORT_DWORD] |=
> OSC_SB_HOTPLUG_PRESENT_SUPPORT;
> > +
> > if (ACPI_FAILURE(acpi_get_handle(NULL, "\\_SB", &handle)))
> > return;
> >
> > @@ -383,6 +395,10 @@ static void
> acpi_bus_osc_negotiate_platform_control(void)
> > capbuf_ret[OSC_SUPPORT_DWORD] &
> OSC_SB_NATIVE_USB4_SUPPORT;
> > osc_cpc_flexible_adr_space_confirmed =
> > capbuf_ret[OSC_SUPPORT_DWORD] &
> OSC_SB_CPC_FLEXIBLE_ADR_SPACE;
> > + osc_sb_hotplug_enabled_support_acked =
> > + capbuf_ret[OSC_SUPPORT_DWORD] &
> OSC_SB_HOTPLUG_ENABLED_SUPPORT;
> > + osc_sb_hotplug_present_support_acked =
> > + capbuf_ret[OSC_SUPPORT_DWORD] &
> OSC_SB_HOTPLUG_PRESENT_SUPPORT;
> > }
> >
> > kfree(context.ret.pointer);
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h index
> > 00be66683505..c572abac803c 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -559,12 +559,16 @@ acpi_status acpi_run_osc(acpi_handle handle,
> struct acpi_osc_context *context);
> > #define OSC_SB_NATIVE_USB4_SUPPORT 0x00040000
> > #define OSC_SB_PRM_SUPPORT 0x00200000
> > #define OSC_SB_FFH_OPR_SUPPORT 0x00400000
> > +#define OSC_SB_HOTPLUG_ENABLED_SUPPORT 0x00800000
> > +#define OSC_SB_HOTPLUG_PRESENT_SUPPORT 0x01000000
> >
> > extern bool osc_sb_apei_support_acked; extern bool
> > osc_pc_lpi_support_confirmed; extern bool
> > osc_sb_native_usb4_support_confirmed;
> > extern bool osc_sb_cppc2_support_acked; extern bool
> > osc_cpc_flexible_adr_space_confirmed;
> > +extern bool osc_sb_hotplug_enabled_support_acked;
> > +extern bool osc_sb_hotplug_present_support_acked;
> >
> > /* USB4 Capabilities */
> > #define OSC_USB_USB3_TUNNELING 0x00000001
|