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

[Laszlo Ersek]

(I've been dropped from the address list, not sure why)

On 10/22/19 19:17, Christophe de Dinechin wrote:
> 
> 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 <address@hidden>
>>>> ---
>>>>  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.

I prefer my proposal because it translates to source code more directly
(a ladder of "if / else" pairs, or similar).

(I know that some programming languages support "unless"; I could never
wrap my brain around that idea! :) )

Thanks
Laszlo