Re: [Qemu-devel] [PATCH 15/33] docs: update ACPI CPU hotplug spec with new protocol

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, May 17, 2016 at 04:43:07PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  docs/specs/acpi_cpu_hotplug.txt | 88 +++++++++++++++++++++++++++++++++++------
>  1 file changed, 76 insertions(+), 12 deletions(-)
> 
> diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
> index 340b751..c5bce6a 100644
> --- a/docs/specs/acpi_cpu_hotplug.txt
> +++ b/docs/specs/acpi_cpu_hotplug.txt
> @@ -4,21 +4,85 @@ QEMU<->ACPI BIOS CPU hotplug interface
>  QEMU supports CPU hotplug via ACPI. This document
>  describes the interface between QEMU and the ACPI BIOS.
>  
> -ACPI GPE block (IO ports 0xafe0-0xafe3, byte access):
> ------------------------------------------
> -
> -Generic ACPI GPE block. Bit 2 (GPE.2) used to notify CPU
> -hot-add/remove event to ACPI BIOS, via SCI interrupt.
> +ACPI BIOS GPE.2 handler is dedicated for notifying OS about CPU hot-add
> +and hot-remove events.
>  
> +============================================
> +Legacy ACPI CPU hotplug interface registers:
> +--------------------------------------------
>  CPU present bitmap for:
> +  One bit per CPU. Bit position reflects corresponding CPU APIC ID. Read-only.
>    ICH9-LPC (IO port 0x0cd8-0xcf7, 1-byte access)
>    PIIX-PM  (IO port 0xaf00-0xaf1f, 1-byte access)
>  ---------------------------------------------------------------
> -One bit per CPU. Bit position reflects corresponding CPU APIC ID.
> -Read-only.
> +QEMU sets corresponding CPU bit on hot-add event and issues SCI
> +with GPE.2 event set. CPU present map 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:
> +-------------------------------------
> +Register block base address:
> +    ICH9-LPC IO port 0x0cd8
> +    PIIX-PM  IO port 0xaf00

OK but this means we either use legacy or new one,
bot both, which is problematic for people using old seabios
without acpi loading support and with -M pc.

I don't say we must support them with >255 CPUs,
but I do say we should make an effort for simple
setups with <255 CPUs.


> +Register block size:
> +    ACPI_CPU_HOTPLUG_REG_LEN = 12
> +
> +read access:

So this implies acpi must scan all cpus on each event, and
this seems too aggressive.

I think we need something hierarchical where
you read one level and know which cpus to probe.


> +    offset:
> +    [0x0-0x3] reserved
> +    [0x4] CPU device status fields: (1 byte access)
> +        bits:
> +           0: Device is enabled and may be used by guest
> +           1: Device insert event, used to distinguish device for which
> +              no device check event to OSPM was issued.
> +              It's valid only when bit 0 is set.
> +           2: Device remove event, used to distinguish device for which
> +              no device eject request to OSPM was issued.
> +           3-7: reserved and should be ignored by OSPM
> +    [0x5-0x7] reserved
> +    [0x8] Command data: (DWORD access)
> +          in case of error or unsupported command reads is 0xFFFFFFFF
> +          current 'Command field' value:
> +              0: returns PXM value corresponding to device
> +
> +write access:
> +    offset:
> +    [0x0-0x3] CPU selector: (DWORD access)
> +              selects active CPU device. All following accesses to other
> +              registers will read/store data from/to selected CPU.

I've been thinking - is it time to add an EmbeddedControl interface?
This way we have another namespace.
Not insisting on it, just an idea.

> +    [0x4] CPU device control fields: (1 byte access)
> +        bits:
> +            0: reserved, OSPM must clear it before writing to register.
> +            1: if set to 1 clears device insert event, set by OSPM
> +               after it has emitted device check event for the
> +               selected CPU device
> +            2: if set to 1 clears device remove event, set by OSPM
> +               after it has emitted device eject request for the
> +               selected CPU device
> +            3: if set to 1 initiates device eject, set by OSPM when it
> +               triggers CPU device removal and calls _EJ0 method
> +            4-7: reserved, OSPM must clear them before writing to register
> +    [0x5] Command field: (1 byte access)
> +          value:
> +            0: following reads from 'Command data' register returns PXM
> +               value of device
> +            1: following writes to 'Command data' register set OST event
> +               register in QEMU
> +            2: following writes to 'Command data' register set OST status
> +               register in QEMU
> +            other values: reserved
> +    [0x6-0x7] reserved
> +    [0x8] Command data: (DWORD access)
> +          current 'Command field' value:
> +              1: stores value into OST event register
> +              2: stores value into OST status register, triggers
> +                 ACPI_DEVICE_OST QMP event from QEMU to external applications
> +                 with current values of OST event and status registers.
> +            other values: reserved
>  
> -CPU hot-add/remove notification:
> ------------------------------------------------------
> -QEMU sets/clears corresponding CPU bit on hot-add/remove event.
> -CPU present map read by ACPI BIOS GPE.2 handler to notify OS of CPU
> -hot-(un)plug events.
> +Selecting CPU device beyond possible range has no effect on platform:
> +   - write accesses to CPU hot-plug registers not documented above are
> +     ignored
> +   - read accesses to CPU hot-plug registers not documented above return
> +     all bits set to 1.

why not 0?

> -- 
> 1.8.3.1