Hello Igor,

On 10/18/19 18:18, Igor Mammedov wrote:
> On Thu, 10 Oct 2019 16:56:18 +0200
> Laszlo Ersek <ler...@redhat.com> wrote:

[...]

>> Right now, I can't offer testing for patch#3 (I'm quite far from the
>> point where I'll be actually looking for a hotplugged CPU :) ), but
>> based on the docs patches #1 and #2, and my proposed updates, I can
>> rework my "possible CPU count detection" in OVMF.
>>
>> Do I need to check in OVMF specifically whether the "modern" CPU hotplug
>> register block is available? Can you tell me what the oldest machine
>> types are that support the modern interface?
> See 679dd1a95 (pc: use new CPU hotplug interface since 2.7 machine type)
> 
> 
>> Hmm... Commit abd49bc2ed2f ("docs: update ACPI CPU hotplug spec with new
>> protocol", 2016-06-24) seems relevant. First released in v2.7.0. I think
>> I should detect whether this interface is available.
> Can you make detection based on QEMU version (is dynamic detection really 
> necessary)?

Thanks for following up on this; indeed it has been my remaining open
question in this thread.

I attempted to investigate a bit myself a few days ago, and indeed I can
now find the commit hash "679dd1a95" in my bash history.

So... If I can somehow query the QEMU machine type version -- or
emulator version? -- inside the guest, I'm perfectly happy using that. I
don't need dynamic detection specifically for this feature, I just need
something to deduce the feature from.

The problem is that the "possible CPU count" determination would run in
OVMF in every case, regardless of whether OVMF was built with -D
SMM_REQUIRE, and regardless of board type (q35 vs. i440fx). Requiring
QEMU v2.7.0 or later just for booting OVMF would be a bit steep.

So the idea is:
- check (somehow) if QEMU has the modern hotplug register block
  - available: great, now set the boot CPU count and the possible CPU
               count independently of each other
  - otherwise: set both values to the boot CPU count (which we can fetch
               from fw_cfg, like we've always done)


>> 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.
> 
>> BTW, can I dynamically detect support for the GET_CPU_ID command too?
>> I'm thinking, when I enumerate / count all possible CPUs, I can at once
>> fetch the arch IDs for all of them. If I only get zeros from the command
>> data registers, across all CPUs, in response to GET_CPU_ID, then the
>> command is not available.
> 
> APICID == 0 is valid value, so one would be need to account for ' -smp 1 '
> case where the only valid selector is 0 that leads to APIC ID = 0
> 
> If counted maxcpus > 1, then what you suggest will work
> if you pick the last CPU (apic id != 0). (at least for x86 guest,
> I don't know if it's fine wrt ARM guest)
> 
> May be dynamic detection is just over-engineering.

Dynamically detecting the presence of the GET_CPU_ID command is not
important. That's because GET_CPU_ID is for a new use case (CPU hotplug
with SMM), which has never worked before. So we can't regress it.

We'll just modify the OvmfPkg/README file to spell out the minimum
machine type requirement:

  don't try to hotplug a CPU when running OVMF built with SMM_REQUIRE,
  unless your machine type is pc-q35-4.2+

which is exactly what we did for base SMM:

  * The minimum required QEMU machine type is "pc-q35-2.5".

The "possible CPU count" determination is more risky because that will
modify a code path that's active on every boot, in every possible
environment.

I think I can rework my OVMF series for the "possible CPU count"
fetching now.

Thank you!
Laszlo


Reply via email to