Re: [PATCH 3/4] hw/i386: add facility to expose CPU topology over fw-cfg

[Igor Mammedov]

On Wed, 9 Oct 2019 23:01:21 +0200
Laszlo Ersek <address@hidden> wrote:

> On 10/08/19 17:59, Igor Mammedov wrote:
> > On Tue,  8 Oct 2019 12:52:58 +0200
> > Laszlo Ersek <address@hidden> wrote:
> >   
> >> FW_CFG_MAX_CPUS exposes the (exclusive) maximum APIC ID to guest firmware,
> >> due to historical reasons. That value is not useful to edk2, however. For
> >> supporting VCPU hotplug, edk2 needs:
> >>
> >> - the boot CPU count (already exposed in FW_CFG_NB_CPUS),
> >>
> >> - and the maximum foreseen CPU count (tracked in
> >>   "MachineState.smp.max_cpus", but not currently exposed).  
> > one can get it with current QEMU without adding new fgcfg
> > (albeit in a bit awkward way)
> > 
> > max_cpu count can be derived indirectly as result of cpu hotplug
> > enumeration (IO interface at 0x0cd8-0xcf7) by writing/reading
> > to/from selector register (see ACPI_CPU_SELECTOR_OFFSET_WR)
> > until read value stops changing values (i.e. max cpu count
> > is reached). One also can figure out present/non-present
> > cpu status by reading flags register.  
> 
> What do you mean by "read value stops changing values"?
> 
> I assume I have to write the CPU index (in incrementing fashion) to
> offset 0 in the register block.
> 
> - What byte order?
> - What offset / width do I need to read back? What endianness? :)
Since it's ACPI oriented oriented, it's supposed to be little-endian.
But spec doesn't mention it and apparently code I wrote back then
have bugs in this regard.



> - What is the expected value once I run out of the possible CPU range?
> (I tried to figure these out from "docs/specs/acpi_cpu_hotplug.txt", but
> I can't find the answers in it. Apologies.)

The apology is all mine. I should've written better spec/code.
I'll fix it and update spec/code to match expected byte-order.

As for a way to enumerate CPUs an APIC ID, I've just posted patches
updating spec with example workflows and exposing APIC ID
in the interface.

  [RFC 0/3] acpi: cphp: add CPHP_GET_CPU_ID_CMD command to cpu hotplug  MMIO 
interface


> Other than that, I'm fine with this method. Hopefully the IO port
> accesses (on every boot) won't slow down the boot much (esp. in SEV
> guests, where they are more costly).
> 
> 
> >> Add a new fw-cfg file to expose "max_cpus".
> >>
> >> While at it, expose the rest of the topology too (die / core / thread
> >> counts), because I expect that the VCPU hotplug feature for OVMF will
> >> ultimately need those too, and the data size is not large. This is
> >> slightly complicated by the fact that the die count is specific to
> >> PCMachineState, but fw_cfg_arch_create() intends to be PC-independent (see
> >> commit 149c50cabcc4).  
> > Could you clarify why topology info is necessary?  
> 
> (Done in the subsequent message, but I'll answer here too, below.)
> 
> 
> > Potentially it's possible to extend cpu hotplug ABI to report
> > arch specific apic-id (x86) or mpidr (arm) if firmware really
> > needs to know topology and let guest to decode it according
> > to CPU's spec.  
> 
> This would be very nice.
> 
> For the hotplug use case, the internal structure / topology of the
> APIC-ID actually appears irrelevant. What's needed is that the "host
> CPU", handling the hotplug SMI, can *somehow* deduce the APIC-ID of the
> new CPU. (The edk2 code suggests that, on physical platforms, the RAS
> controller passes the new APIC-ID the the "host CPU".) The edk2
> infrastructure uses APIC-ID's as the unique key for identifying CPUs.
> 
> The topology info was supposed to allow OVMF to calculate the APIC-ID
> from scratch, based on the sequential CPU index (retrieved from the ACPI
> hotplug register block).
> 
> > So far there were no need for it as all possible cpus are
> > described in ACPI tables passed to guest, but I'm not going
> > to suggest to parse them on firmware side as it's too complicated :)  
> 
> Thanks, that's appreciated :)
> 
> > PS:
> > The reason we started building ACPI tables in QEMU, was never
> > ending story of adding more ABI and supporting it afterwards.
> > So I'd try to avoid doing it if it can be helped.  
> 
> Sure, I don't insist.
> 
> If the hotplug register block can expose the APIC-IDs as "opaque"
> integers, and they match the APIC-IDs read on the actual processors,
> things should work.
> 
> Thanks,
> Laszlo