| Message ID | 1478789424-716-1-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 10, 2016 at 09:50:24AM -0500, 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@oracle.com> I will defer this to x86 maintainers. > --- > > 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)); > -- > 2.7.4 >
>>> On 11.11.16 at 16:16, <wei.liu2@citrix.com> wrote: > On Thu, Nov 10, 2016 at 09:50:24AM -0500, 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@oracle.com> > > I will defer this to x86 maintainers. Changing from a random (wrong) value to a deterministic (but still wrong) value certainly won't make things worse. IOW the patch can have my ack if needed, but I thought I had seen Andrew already give his. Jan
On 11/11/16 15:32, Jan Beulich wrote: >>>> On 11.11.16 at 16:16, <wei.liu2@citrix.com> wrote: >> On Thu, Nov 10, 2016 at 09:50:24AM -0500, 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@oracle.com> >> I will defer this to x86 maintainers. > Changing from a random (wrong) value to a deterministic (but still > wrong) value certainly won't make things worse. IOW the patch > can have my ack if needed, but I thought I had seen Andrew > already give his. I don't think I had explicitly, but here... Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On Fri, Nov 11, 2016 at 03:33:11PM +0000, Andrew Cooper wrote: > On 11/11/16 15:32, Jan Beulich wrote: > >>>> On 11.11.16 at 16:16, <wei.liu2@citrix.com> wrote: > >> On Thu, Nov 10, 2016 at 09:50:24AM -0500, 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@oracle.com> > >> I will defer this to x86 maintainers. > > Changing from a random (wrong) value to a deterministic (but still > > wrong) value certainly won't make things worse. IOW the patch > > can have my ack if needed, but I thought I had seen Andrew > > already give his. > > I don't think I had explicitly, but here... > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Applied.
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));
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@oracle.com> --- 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(+)