Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Andrew Cooper <andrew.cooper3@citrix.com> writes:

> On 29/06/16 13:16, Vitaly Kuznetsov wrote:
>> Andrew Cooper <andrew.cooper3@citrix.com> writes:
>>
>>> On 28/06/16 17:47, Vitaly Kuznetsov wrote:
>>>> @@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block *self, unsigned long action,
>>>>  	int cpu = (long)hcpu;
>>>>  	switch (action) {
>>>>  	case CPU_UP_PREPARE:
>>>> +		/* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
>>>> +		per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
>>> Please do not assume or propagate this brokenness.  It is incorrect in
>>> the general case, and I will be fixing in the hypervisor in due course.
>>>
>>> Always read the APIC_ID from the LAPIC, per regular hardware.
>> (I'm probbaly missing something important - please bear with me)
>>
>> The problem here is that I need to get _other_ CPU's id before any code
>> is executed on that CPU (or, at least, this is the current state of
>> affairs if you look at xen_hvm_cpu_up()) so I can't use CPUID/do MSR
>> reads/... The only option I see here is to rely on ACPI (MADT) data
>> which is stored in x86_cpu_to_apicid (and that's what cpu_physical_id()
>> gives us). MADT also has processor id which connects it to DSDT but I'm
>> not sure Linux keeps this data. But this is something fixable I guess.
>
> Hmm yes - that is a tricky issue.
>
> It is not safe or correct to assume that xen_vcpu_id is APICID / 2.
>
> This is currently the case for most modern versions of Xen, but isn't
> the case for older versions, and won't be the case in the future when I
> (or someone else) fixes topology representation for guests.
>
> For this to work, we need one or more of:
>
> 1) to provide the guest a full mapping from APIC_ID to vcpu id at boot
> time.

So can we rely on ACPI data? Especially on MADT and processor ids there?
I think we can always guarantee that processor ids there match vCPU
ids. If yes I can try saving this data when we parse MADT.

> 2) add a new interface where the guest can explicitly query "what is the
> vcpu id for the entity with this APIC_ID".
> 3) Allow HVM guests to identify a vcpu in a hypercall by APIC_ID.
>
> 3 is the cleaner approach, but given that vcpu ids have already leaked
> into an HVM domains idea of the world, 1 or 2 is probably a better
> ladder to dig us out of this hole.

It would be nice to avoid hypervisor changes but if we have to modify it
we can fail all secondary CPUs for now when we detect that CPU0's vCPU
id is not 0 (and CPU0 gets its id with CPUID).

-- 
  Vitaly