On 10/09/19 15:22, Igor Mammedov wrote: > Clarify values of "CPU selector' register and add workflows for
mismatched quotes (double vs. single) > * finding CPU with pending 'insert/remove' event > * enumerating present/non present CPUs > > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > --- > docs/specs/acpi_cpu_hotplug.txt | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/docs/specs/acpi_cpu_hotplug.txt b/docs/specs/acpi_cpu_hotplug.txt > index ac5903b2b1..43c5a193f0 100644 > --- a/docs/specs/acpi_cpu_hotplug.txt > +++ b/docs/specs/acpi_cpu_hotplug.txt > @@ -54,6 +54,7 @@ write access: > [0x0-0x3] CPU selector: (DWORD access) Please clarify the endianness. > selects active CPU device. All following accesses to other > registers will read/store data from/to selected CPU. > + Valid values: [0 .. max_cpus) Nice; appreciate the bracket on the left side vs. the paren on the right side! > [0x4] CPU device control fields: (1 byte access) > bits: > 0: reserved, OSPM must clear it before writing to register. > @@ -93,3 +94,24 @@ Selecting CPU device beyond possible range has no effect > on platform: > ignored > - read accesses to CPU hot-plug registers not documented above return > all bits set to 0. > + > +Typical usecases: > + - Get a cpu with pending event > + 1. write 0x0 into 'Command field' register > + 2. read from 'Command data' register, CPU selector value (CPU's UID in > ACPI > + tables) OK. I suggest putting this as: "read the CPU selector value (the CPU's UID in the ACPI tables) from the 'Command data' register" > and event for selected CPU from 'CPU device status fields' OK. > + register. If there aren't pending events, CPU selector value doesn't OK. I suggest s/aren't/are no/ > + change So this feels important: *change* is relative to a previous value. In order to determine change, I have to - either read the "command data" register before writing 0x0 to "command", and then compare the old value against the new value - or even set "command data" to a bogus value myself (against which I can compare the new value, after writing the command register). So, what is the previous selector value that the change is relative to? > and 'insert' and 'remove' bits are not set. Ah, so is the order of steps actually this: 1. write 0x0 to command 2. read device status field 3. if bit#1 or bit#2 is set (insert or remove event), read CPU selector affected by those event(s) from the command data field 4. otherwise, no pending event ? > + - Enumerate CPUs present/non present CPUs. > + 1. set iterator to 0x0 OK > + 2. write 0x0 into 'Command field' register ... this may update the device status field, and the command data field (to a selector with pending events) > and then iterator > + into 'CPU selector' register. ... so in case command 0x0 selected a CPU with pending events, we ignore that, and select our iterator anyway. OK. > + 3. read 'enabled' flag for selected CPU from 'CPU device status fields' > + register OK > + 4. to continue to the next CPU, increment iterator OK > and repeat step 2 not sure why writing 0x0 to "command" again is useful, but I'll see it below; OK > + 5. read 'Command data' register oookay... so if writing 0x0 to command selected a CPU with pending events, we get the selector of *that* CPU (regardless of what iterator we have presently) Otherwise we get an indeterminate value. > + 5.1 if 'Command data' register matches iterator continue to step 3. uhhh... what? :) At this point, the command data register can be in two states: - if the last 0x0 command selected a CPU with events pending, then that selector is available in the command data register. I don't understand why comparing that against the iterator is helpful. - If there was no CPU with pending events, we're comparing an indeterminate value against the iterator. Why? I think the "command data" field must change under some circumstances that are currently not documented. (I.e. it seems like "command data" does not *only* change when command 0x0 can find a CPU with pending events.) Thanks Laszlo > + (read presence bit for the next CPU) > + 5.2 if 'Command data' register has not changed, there is not CPU > + corresponding to iterator value and the last valid iterator value > + equals to 'max_cpus' + 1 >
