Re: [PATCH] x86/hvm: Extend HVM cpuid leaf with vcpu id

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 10/12/14 06:00, Matt Wilson wrote:
> On Thu, Nov 06, 2014 at 09:27:59PM +0000, Andrew Cooper wrote:
>> On 06/11/2014 19:32, Konrad Rzeszutek Wilk wrote:
>>> On Thu, Nov 06, 2014 at 03:07:10PM +0000, Paul Durrant wrote:
>>>> To perform certain hypercalls HVM guests need to use Xen's idea of
>>> What are those 'certain'?
>> Some HVM hypercalls take a vcpu_id as a parameter.  See the
>> functionally-unrelated-but-companion patch "x86/hvm: Add per-vcpu evtchn
>> upcalls".
>>
>> HVM guests currently have no way of obtaining a xen vcpu_id for a given
>> vcpu, and therefore have no way of passing an appropriate parameter to
>> the hypercalls.
> Sorry for chiming in late...
>
> That's not strictly true. You can use the processor index in ACPI.

The LAPIC objects in the MADT/APIC table?  That is still constructed
with the broken LAPIC_ID = 2 * processor_id logic.

>
>> As it currently happens, HVM guests can (and sadly do) take their APIC
>> id, divide by 2, and end up with a number which happens to match the Xen
>> vcpu_id.  This only works because Xen's allocation of APIC IDs is wrong
>> on AMD hardware and Intel Hardware with hyperthreading, and is an issue
>> I plan to fix in the not too distant future.  (It is one of the quagmire
>> of issues relating to cpuid policies)
> This is absolutely the wrong thing to do. If a guest OS is doing this,
> it is doing the wrong thing. FreeBSD went down this erroneous path...
>
> See http://lists.xen.org/archives/html/xen-devel/2013-05/msg03156.html
>
>>>> vcpu id, which may well not match the guest OS idea of CPU id.
>>> Can you elaborate more on the use case please? And why
>>> only restrict this to HVM guests? Why not PV?
>> PV guests unconditionally know their vcpu_id's right from the start, and
>> indeed need them to bring up APs.  HVM guests currently only know APIC IDs.
> No, don't assume anything about APIC IDs mapping to Xen virtual
> processor index. This will break things on EC2 instances that expose
> proper CPU topology through CPUID, and this does *not* follow the
> "index * 2" formula.
>
>>>> This patch adds vcpu id to the HVM cpuid leaf allowing the guest
>>>> to build a mapping.
>>> Don't we have existing hypercalls to get this? Ah VCPUOP_get_physid
>>> is what I was thinking of and that does not seem to be what
>>> you want?
>> This hypercall is only valid for pinned vcpus (i.e. only dom0 with
>> opt_dom0_vcpus_pin), and gives back the underlying APIC ID, and the ACPI
>> processor ID for that APIC ID.
>>
>> The APIC ID can be obtained from cpuid as the vcpus are pinned, and the
>> ACPI ID can be obtained from the ACPI tables, as the domain is dom0. 
>> Ergo, this hypercall is quite redundant, has nothing at all to do with
>> the problem Paul is trying to solve, and appears buggy as it hands back
>> two 64bit values, shifted 32bits apart, in a 64bit integer.
> And the Xen vCPU index can be determined by the processor object IDs
> in DSDT.

The processor object IDs have no requirement in the spec to be in order,
or contiguous.  A DSDT processor object refers back to the MADT, which
in turn still has the broken logic.

Also, any solution using ACPI tables is going to be an issue for PVH guests.

~Andrew