Re: [RFC 1/3] acpi: cpuhp: fix 'Command data' description is spec

[Christophe de Dinechin <dinechin@redhat.com>]

Laszlo Ersek writes:

> On 10/10/19 15:31, Laszlo Ersek wrote:
>> 2nd round:
>>
>> On 10/09/19 15:22, Igor Mammedov wrote:
>>> QEMU returns 0, in case of erro or invalid value in 'Command field',
>>> make spec match reality, i.e.
>>
>> AHA! so this is exactly where you meant to list the particular cases
>> when "command data" reads as 0:
>> - CPU >= max_cpus selected,
>> - CPU with pending events asked for, but none found
>>
>>> Also fix returned value description  in case 'Command field' == 0x0,
>>> it's in not PXM but CPU selector value with pending event
>>>
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>>  docs/specs/acpi_cpu_hotplug.txt | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt
>>> index ee219c8358..ac5903b2b1 100644
>>> --- a/docs/specs/acpi_cpu_hotplug.txt
>>> +++ b/docs/specs/acpi_cpu_hotplug.txt
>>> @@ -44,9 +44,10 @@ read access:
>>>             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
>>> +          in case of error or unsupported command reads is 0x0
>>>            current 'Command field' value:
>>> -              0: returns PXM value corresponding to device
>>> +              0: returns CPU selector value corresponding to a CPU with
>>> +                 pending event.
>>>
>>>  write access:
>>>      offset:
>>>
>>
>> How about:
>>
>>     [0x8] Command data: (DWORD access, little endian)
>>           If the "CPU selector" value last stored by the guest refers to
>>           an impossible CPU, then 0.
>>           Otherwise, if the "Command field" value last stored by the
>>           guest differs from 0, then 0.
>>           Otherwise, if there is at least one CPU with a pending event,
>>           then that CPU has been selected; the command data register
>>           returns that selector.
>>           Otherwise, 0.
>
> Hmmm not exactly. Let me try again.
>
>     [0x8] Command data: (DWORD access, little endian)
>           If the "CPU selector" value last stored by the guest refers to
>           an impossible CPU, then 0.
>           Otherwise, if the "Command field" value last stored by the
>           guest differs from 0, then 0.
>           Otherwise, the currently selected CPU.

How about phrasing it to put the more general case first, e.g.

    [0x8] Command data: (DWORD access, little endian)

          The currently selected CPU, unless:
          - The "CPU selector" value refers to an impossible CPU
          - The "Command field" value last stored by the guest differs
            from 0
          In these last two cases, the value is 0.

>
> Thanks,
> Laszlo


--
Cheers,
Christophe de Dinechin (IRC c3d)