Re: [Xen-devel] [PATCH for-4.8] libxc/x86: Report consistent initial APIC value for PV guests

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 11/10/2016 09:55 AM, Andrew Cooper wrote:
> On 10/11/16 14:50, Boris Ostrovsky wrote:
>> Currently hypervisor provides PV guest's CPUID(1).EBX[31:24] (initial
>> APIC ID) with contents of that field on the processor that launched
>> the guest. This results in the guest reporting different initial
>> APIC IDs across runs.
>>
>> We should be consistent in how this value is reported, let's set
>> it to 0 (which is also what Linux guests expect).
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> This surely wants to go along with:

Probably, although Linux PV always reports APIC ID as zero (whole PV
APIC is a mess there as it is tied to topology discovery and we don't do
this well, to put it charitably).

> andrewcoop@andrewcoop:/local/xen.git/xen$ git diff
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index b51b51b..bdf9339 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -985,6 +985,10 @@ void pv_cpuid(struct cpu_user_regs *regs)
>          uint32_t tmp, _ecx, _ebx;
>  
>      case 0x00000001:
> +        /* Fix up VLAPIC details. */
> +        b &= 0x00FFFFFFu;
> +        b |= (curr->vcpu_id * 2) << 24;

Do we also need to multiply by two for PV guests? Or is it just to be
consistent with HVM?


-boris

> +
>          c &= pv_featureset[FEATURESET_1c];
>          d &= pv_featureset[FEATURESET_1d];
>  
>
> Which brings the PV CPUID handling in line with HVM handling.  Otherwise
> a guest will see an APIC ID of 0 in all vcpus, which will surely confuse it.
>
> ~Andrew
>
>> ---
>>
>> I think this should go to stable branches as well. This has been causing
>> problems lately in Linux with introduction of topology maps.
>>
>>
>>  tools/libxc/xc_cpuid_x86.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
>> index d761805..1f26294 100644
>> --- a/tools/libxc/xc_cpuid_x86.c
>> +++ b/tools/libxc/xc_cpuid_x86.c
>> @@ -618,6 +618,12 @@ static void xc_cpuid_pv_policy(xc_interface *xch,
>>          /* Host topology exposed to PV guest.  Provide host value. */
>>          bool host_htt = regs[3] & bitmaskof(X86_FEATURE_HTT);
>>  
>> +        /*
>> +         * Don't pick host's Initial APIC ID which can change from run
>> +         * to run. 
>> +         */
>> +        regs[1] &= 0x00ffffffu;
>> +
>>          regs[2] = info->featureset[featureword_of(X86_FEATURE_SSE3)];
>>          regs[3] = (info->featureset[featureword_of(X86_FEATURE_FPU)] &
>>                     ~bitmaskof(X86_FEATURE_HTT));


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel