Re: [RFC 3/3] acpi: cpuhp: add CPHP_GET_CPU_ID_CMD command

[Laszlo Ersek]

On 10/21/19 15:06, Laszlo Ersek wrote:
> On 10/18/19 18:18, Igor Mammedov wrote:
>> On Thu, 10 Oct 2019 16:56:18 +0200
>> Laszlo Ersek <address@hidden> wrote:

[...]

>>> Can I use the following sequence to detect whether the interface is
>>> available?
>>>
>>> 1. Store 0x0 to command register.
>>> 2. Store 0x0 to selector register.
>>> 3. Read 'command data' register.
>>> 4. If value read is 0, the interface is available.
>>
>> By default legacy register block layout is in place
>> (i.e. present cpus bitmap) where 1st byte is guarantied to be ">0" as it has
>> at least the boot CPU bit set and writes to legacy bitmap are ignored.
>>
>> Currently AML code code does switching to modern interface, see
>> docs/specs/acpi_cpu_hotplug.txt:
>> "
>>   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.
>> "
>> related code "if (opts.has_legacy_cphp) {" and cpu_status_write()
>>
>> Considering firmware runs the first, it should enable modern interface
>> on its own
>>   1. Store 0x0 to selector register (actually it's store into bitmap to 
>> attempt switch). 
>> and to check if interface is present
>>   2. Store 0x0 to selector register (to ensure valid selector value 
>> (otherwise command is ignored))
>>   3. Store 0x0 to command register (to be able to read back selector from 
>> command data)
>>   4. Store 0x0 to selector register (because #3 can select the a cpu with 
>> events if any)
>>       be aware libvirt may start QEMU in paused mode (hotplug context) and 
>> hotplugs extra CPUs
>>       with device_add and then let guest run. So firmware may see present 
>> CPUs with events
>>       at boot time.
>>   5. Read 'command data' register.
>>   6. If value read is 0, the interface is available.
> 
> Perfect!
> 
> Basically this is prepending two "write 0 to selector register" steps to
> what I suspected. I certainly couldn't figure out the "switch to modern"
> step, and whether initializing the selector to something valid was
> needed at boot. Now I know. :) Thanks!
> 
>>
>>> (Because I assume that unmapped IO ports read as all-bits-one. Is that
>>> right?)
>> that's right but ports are mapped to legacy CPU bitmap, you can't count on 
>> all-bits-one case here.

It seems I rejoiced too soon.

When we read the command data register in the last step, that is at
offset 0x8 in the register block. Considering the legacy "CPU present
bitmap", if no CPU is present in that range, then the firmware could
read a zero value. I got confused because I thought we were reading at
offset 0, which would always have bit0 set (for CPU#0).

Can we detect the modern interface like this:

1. store 0x0 to selector register (attempt to switch)
2. read one byte at offset 0 in the register block
3. if bit#0 is set, the modern interface is unavailable;
   otherwise (= bit#0 clear), the modern interface is available

Here's why:

- if even the legacy interface is missing, then step 2 is an unassigned
  read, hence the value read is all-bits-one; bit#0 is set

- if only the legacy interface is available, then bit#0 stands for
  CPU#0, it will be set

- if the switch-over in step 1 is successful, then offset 0 is reserved,
  hence it returns all-bits-zero.

With this, if we ever assigned offset 0 for reading, then we'd have to
define it with bit#0 constantly clear.

Thanks,
Laszlo