[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0 8/8] acpi: cpuhp: spec: document procedure for enabli
From: |
Laszlo Ersek |
Subject: |
Re: [PATCH for-5.0 8/8] acpi: cpuhp: spec: document procedure for enabling modern CPU hotplug |
Date: |
Thu, 5 Dec 2019 15:07:53 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 12/04/19 18:05, Igor Mammedov wrote:
> Describe how to enable and detect modern CPU hotplug interface.
> Detection part is based on new CPHP_GET_CPU_ID_CMD command,
> introduced by "acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command" patch.
>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> docs/specs/acpi_cpu_hotplug.txt | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
Could we make this usecase / workflow independent of the new
CPHP_GET_CPU_ID_CMD command please?
I'd like to suggest the following:
> diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
> index bb33144..667b264 100644
> --- a/docs/specs/acpi_cpu_hotplug.txt
> +++ b/docs/specs/acpi_cpu_hotplug.txt
> @@ -15,14 +15,14 @@ CPU present bitmap for:
> PIIX-PM (IO port 0xaf00-0xaf1f, 1-byte access)
> One bit per CPU. Bit position reflects corresponding CPU APIC ID.
> Read-only.
> The first DWORD in bitmap is used in write mode to switch from legacy
> - to new CPU hotplug interface, write 0 into it to do switch.
> + to modern CPU hotplug interface, write 0 into it to do switch.
> ---------------------------------------------------------------
> QEMU sets corresponding CPU bit on hot-add event and issues SCI
> with GPE.2 event set. CPU present map is read by ACPI BIOS GPE.2 handler
> to notify OS about CPU hot-add events. CPU hot-remove isn't supported.
>
> =====================================
> -ACPI CPU hotplug interface registers:
> +Modern ACPI CPU hotplug interface registers:
> -------------------------------------
> Register block base address:
> ICH9-LPC IO port 0x0cd8
> @@ -105,6 +105,24 @@ write access:
> other values: reserved
>
> Typical usecases:
> + - (x86) Detecting and enabling modern CPU hotplug interface.
(1) I think we can drop the (x86) restriction. (Because, we don't need
to depend on APIC ID specifics; see below.)
> + QEMU starts with legacy CPU hotplug interface enabled. Detecting
> and
> + switching to modern interface is based on the 2 legacy CPU hotplug
> features:
> + 1. Writes into CPU bitmap are ignored.
> + 2. CPU bitmap always has bit#0 set, corresponding to boot CPU.
> +
> + Use following steps to detect and enable modern CPU hotplug
> interface:
> + 1. Store 0x0 to the 'CPU selector' register,
> + attempting to switch to modern mode
> + 2. Store 0x0 to the 'CPU selector' register,
> + to ensure valid selector value
OK thus far.
> + 3. Store 0x3 to the 'Command field' register,
> + sets the 'Command data 2' register into architecture specific
> + CPU identifier mode
(2) Can we please store command 0 here
(CPHP_GET_NEXT_CPU_WITH_EVENT_CMD)?
That might change the selector value, yes. (Even if that happens, the
new selector will be *valid*.)
But the main point is that, with 0 stored to the command register, one
of the following *four* cases will hold, subsequently:
(2a) if register block is totally missing:
--> Offset#0 will read as all-bits-one (unassigned read)
(2b) if register block is legacy-only:
--> Offset#0 will read as nonzero, due to CPU#0 being always present
(2c) if the modern register block is active, but the "Command data 2"
register is *not* yet described in the spec file:
--> Offset#0 will read as zero, because it is *reserved*:
> read access:
> offset:
> [0x0-0x3] reserved <---- HERE
(2d) if the modern register block is active, and the "Command data 2"
register *is* described in the spec file:
--> the "Command data 2" register (at offset#0) will read as zero,
because:
> read access:
> offset:
> [0x0-0x3] Command data 2: (DWORD access)
> if last stored 'Command field' value:
> 3: upper 32 bits of architecture specific identifying CPU
> value
> (n x86 case: 0x0)
> other values: reserved <------ HERE
and then step#4 applies just the same:
On 12/04/19 18:05, Igor Mammedov wrote:
> + 4. Read the 'Command data 2' register.
> + If read value is 0x0, the modern interface is enabled.
> + Otherwise legacy or no CPU hotplug interface available
> +
because "read value is 0x0" corresponds to the *union* of cases (2c) and
(2d) -- namely, "the modern register block is active".
My proposal above is what I implemented for OVMF in October:
[edk2-devel] [PATCH v2 3/3]
OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug
http://mid.mail-archive.com/address@hidden
and it works very well.
So the benefits would be:
- I wouldn't have to rewrite that (complex!) patch :) ,
- the logic would not store the new CPHP_GET_CPU_ID_CMD command, only
read offset#0,
- the logic would not be x86 specific (it would not have to rely on the
most significant 32 bits of the 64-bit arch-specific CPU identifier --
here: the APIC ID -- being zero).
Furthermore,
(3) In step#4, I suggest replacing 'Command data 2' with "offset 0",
(4) finally, I'd suggest squashing this patch (updated as requested
above) into patch "acpi: cpuhp: spec: add typical usecases".
Using my suggestion (2), you can define the "modern interface
enablement" workflow as well, without any dependency on
CPHP_GET_CPU_ID_CMD. The only thing that's necessary is the small update
from (3), and then you can describe all three important use cases in one
go, in patch#6.
And then you can introduce CPHP_GET_CPU_ID_CMD in the last patch
(patch#7), while staying compatible with the previously-documented
workflows.
Pretty please? :)
Thanks!
Laszlo
> - Get a cpu with pending event
> 1. Store 0x0 to the 'CPU selector' register.
> 2. Store 0x0 to the 'Command field' register.
>