| Message ID | 1379694292-1601-12-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
I do not know, but this patch might introduce a regression. If I specify: -smp 2,sockets=1,cores=2,threads=1 to a Windows 2012 R2 Server it crashes at boot time. -smp 2 works. git bisect start # good: [62ecc3a0e3c77a4944c92a02dd7fae2ab1f2290d] Update VERSION for 1.6.1 release git bisect good 62ecc3a0e3c77a4944c92a02dd7fae2ab1f2290d # bad: [964668b03d26f0b5baa5e5aff0c966f4fcb76e9e] Update version for 1.7.0-rc0 release git bisect bad 964668b03d26f0b5baa5e5aff0c966f4fcb76e9e # good: [1ee2daeb6448312d6d0e22175f5c1b9b01f8974c] Update version for 1.6.0 git bisect good 1ee2daeb6448312d6d0e22175f5c1b9b01f8974c # bad: [03cfd8faa7ffb7201e2949b99c2f35b1fef7078b] linux-user: add support of binfmt_misc 'O' flag git bisect bad 03cfd8faa7ffb7201e2949b99c2f35b1fef7078b # good: [5a93d5c2abc719bd44f6c9fbeed88d3cae712606] Merge remote-tracking branch 'mjt/trivial-patches' into staging git bisect good 5a93d5c2abc719bd44f6c9fbeed88d3cae712606 # good: [a27292b5d7545509bfa171922516d2033c570205] virtio-scsi: Make type virtio-scsi-common abstract git bisect good a27292b5d7545509bfa171922516d2033c570205 # good: [469936ae0a9891b2de7e46743f683535b0819bee] target-i386: Fix segment cache dump git bisect good 469936ae0a9891b2de7e46743f683535b0819bee # bad: [3e4be9c29784df09c364b52a55e826a0b05b950e] Merge remote-tracking branch 'qemu-kvm/uq/master' into staging git bisect bad 3e4be9c29784df09c364b52a55e826a0b05b950e # good: [2571f8f5fbaea5dc3bdcd84737f109b459576e90] Merge remote-tracking branch 'spice/spice.v74' into staging git bisect good 2571f8f5fbaea5dc3bdcd84737f109b459576e90 # good: [c5daeae1b4ddff97d605bd954a7c2a2b2cf6040f] linux-headers: update to 3.11 git bisect good c5daeae1b4ddff97d605bd954a7c2a2b2cf6040f # good: [ceae18bd74e8940ff79935a257c72e665b084bcc] lsi: add 53C810 variant git bisect good ceae18bd74e8940ff79935a257c72e665b084bcc # bad: [f010bc643a2759e87e989c3e4e85f15ec71ae98f] target-i386: add feature kvm_pv_unhalt git bisect bad f010bc643a2759e87e989c3e4e85f15ec71ae98f # bad: [4f2656079f903efcd0d8224cbc79170ad3ee5b70] linux-headers: update to 3.12-rc1 git bisect bad 4f2656079f903efcd0d8224cbc79170ad3ee5b70 # bad: [787aaf5703a702094f395db6795e74230282cd62] target-i386: forward CPUID cache leaves when -cpu host is used git bisect bad 787aaf5703a702094f395db6795e74230282cd62 Peter On 20.09.2013 18:24, Paolo Bonzini wrote: > From: Benoît Canet <benoit@irqsave.net> > > Some users running cpu intensive tasks checking the cache CPUID leaves at > startup and making decisions based on the result reported that the guest was > not reflecting the host CPUID leaves when -cpu host is used. > > This patch fix this. > > Signed-off-by: Benoît Canet <benoit@irqsave.net> > [Rename new field to cache_info_passthrough - Paolo] > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target-i386/cpu-qom.h | 3 +++ > target-i386/cpu.c | 19 +++++++++++++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h > index c4447c2..f4fab15 100644 > --- a/target-i386/cpu-qom.h > +++ b/target-i386/cpu-qom.h > @@ -70,6 +70,9 @@ typedef struct X86CPU { > bool hyperv_relaxed_timing; > int hyperv_spinlock_attempts; > > + /* if true the CPUID code directly forward host cache leaves to the guest */ > + bool cache_info_passthrough; > + > /* Features that were filtered out because of missing host capabilities */ > uint32_t filtered_features[FEATURE_WORDS]; > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index c36345e..46edd75 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -486,6 +486,7 @@ typedef struct x86_def_t { > int stepping; > FeatureWordArray features; > char model_id[48]; > + bool cache_info_passthrough; > } x86_def_t; > > #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) > @@ -1139,6 +1140,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > assert(kvm_enabled()); > > x86_cpu_def->name = "host"; > + x86_cpu_def->cache_info_passthrough = true; > host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); > x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); > > @@ -1888,6 +1890,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) > env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX]; > env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX]; > env->cpuid_xlevel2 = def->xlevel2; > + cpu->cache_info_passthrough = def->cache_info_passthrough; > > object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp); > } > @@ -2062,6 +2065,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > break; > case 2: > /* cache info: needed for Pentium Pro compatibility */ > + if (cpu->cache_info_passthrough) { > + host_cpuid(index, 0, eax, ebx, ecx, edx); > + break; > + } > *eax = 1; /* Number of CPUID[EAX=2] calls required */ > *ebx = 0; > *ecx = 0; > @@ -2071,6 +2078,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > break; > case 4: > /* cache info: needed for Core compatibility */ > + if (cpu->cache_info_passthrough) { > + host_cpuid(index, count, eax, ebx, ecx, edx); > + break; > + } > if (cs->nr_cores > 1) { > *eax = (cs->nr_cores - 1) << 26; > } else { > @@ -2228,6 +2239,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > break; > case 0x80000005: > /* cache info (L1 cache) */ > + if (cpu->cache_info_passthrough) { > + host_cpuid(index, 0, eax, ebx, ecx, edx); > + break; > + } > *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) | \ > (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES); > *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | \ > @@ -2239,6 +2254,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, > break; > case 0x80000006: > /* cache info (L2 cache) */ > + if (cpu->cache_info_passthrough) { > + host_cpuid(index, 0, eax, ebx, ecx, edx); > + break; > + } > *eax = (AMD_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) | \ > (L2_DTLB_2M_ENTRIES << 16) | \ > (AMD_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) | \
On 18.11.2013 16:23, Peter Lieven wrote: > I do not know, but this patch might introduce a regression. > > If I specify: -smp 2,sockets=1,cores=2,threads=1 to a Windows 2012 R2 Server it crashes > at boot time. -smp 2 works. for Linux /proc/cpuinfo reveals no cpu layout information (sibliings, cores, threads etc.) with this patch applied and a manual socket,core,thread configuration. > > git bisect start > # good: [62ecc3a0e3c77a4944c92a02dd7fae2ab1f2290d] Update VERSION for 1.6.1 release > git bisect good 62ecc3a0e3c77a4944c92a02dd7fae2ab1f2290d > # bad: [964668b03d26f0b5baa5e5aff0c966f4fcb76e9e] Update version for 1.7.0-rc0 release > git bisect bad 964668b03d26f0b5baa5e5aff0c966f4fcb76e9e > # good: [1ee2daeb6448312d6d0e22175f5c1b9b01f8974c] Update version for 1.6.0 > git bisect good 1ee2daeb6448312d6d0e22175f5c1b9b01f8974c > # bad: [03cfd8faa7ffb7201e2949b99c2f35b1fef7078b] linux-user: add support of binfmt_misc 'O' flag > git bisect bad 03cfd8faa7ffb7201e2949b99c2f35b1fef7078b > # good: [5a93d5c2abc719bd44f6c9fbeed88d3cae712606] Merge remote-tracking branch 'mjt/trivial-patches' into staging > git bisect good 5a93d5c2abc719bd44f6c9fbeed88d3cae712606 > # good: [a27292b5d7545509bfa171922516d2033c570205] virtio-scsi: Make type virtio-scsi-common abstract > git bisect good a27292b5d7545509bfa171922516d2033c570205 > # good: [469936ae0a9891b2de7e46743f683535b0819bee] target-i386: Fix segment cache dump > git bisect good 469936ae0a9891b2de7e46743f683535b0819bee > # bad: [3e4be9c29784df09c364b52a55e826a0b05b950e] Merge remote-tracking branch 'qemu-kvm/uq/master' into staging > git bisect bad 3e4be9c29784df09c364b52a55e826a0b05b950e > # good: [2571f8f5fbaea5dc3bdcd84737f109b459576e90] Merge remote-tracking branch 'spice/spice.v74' into staging > git bisect good 2571f8f5fbaea5dc3bdcd84737f109b459576e90 > # good: [c5daeae1b4ddff97d605bd954a7c2a2b2cf6040f] linux-headers: update to 3.11 > git bisect good c5daeae1b4ddff97d605bd954a7c2a2b2cf6040f > # good: [ceae18bd74e8940ff79935a257c72e665b084bcc] lsi: add 53C810 variant > git bisect good ceae18bd74e8940ff79935a257c72e665b084bcc > # bad: [f010bc643a2759e87e989c3e4e85f15ec71ae98f] target-i386: add feature kvm_pv_unhalt > git bisect bad f010bc643a2759e87e989c3e4e85f15ec71ae98f > # bad: [4f2656079f903efcd0d8224cbc79170ad3ee5b70] linux-headers: update to 3.12-rc1 > git bisect bad 4f2656079f903efcd0d8224cbc79170ad3ee5b70 > # bad: [787aaf5703a702094f395db6795e74230282cd62] target-i386: forward CPUID cache leaves when -cpu host is used > git bisect bad 787aaf5703a702094f395db6795e74230282cd62 > > Peter > > On 20.09.2013 18:24, Paolo Bonzini wrote: >> From: Benoît Canet <benoit@irqsave.net> >> >> Some users running cpu intensive tasks checking the cache CPUID leaves at >> startup and making decisions based on the result reported that the guest was >> not reflecting the host CPUID leaves when -cpu host is used. >> >> This patch fix this. >> >> Signed-off-by: Benoît Canet <benoit@irqsave.net> >> [Rename new field to cache_info_passthrough - Paolo] >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> target-i386/cpu-qom.h | 3 +++ >> target-i386/cpu.c | 19 +++++++++++++++++++ >> 2 files changed, 22 insertions(+) >> >> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h >> index c4447c2..f4fab15 100644 >> --- a/target-i386/cpu-qom.h >> +++ b/target-i386/cpu-qom.h >> @@ -70,6 +70,9 @@ typedef struct X86CPU { >> bool hyperv_relaxed_timing; >> int hyperv_spinlock_attempts; >> + /* if true the CPUID code directly forward host cache leaves to the guest */ >> + bool cache_info_passthrough; >> + >> /* Features that were filtered out because of missing host capabilities */ >> uint32_t filtered_features[FEATURE_WORDS]; >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index c36345e..46edd75 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -486,6 +486,7 @@ typedef struct x86_def_t { >> int stepping; >> FeatureWordArray features; >> char model_id[48]; >> + bool cache_info_passthrough; >> } x86_def_t; >> #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) >> @@ -1139,6 +1140,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) >> assert(kvm_enabled()); >> x86_cpu_def->name = "host"; >> + x86_cpu_def->cache_info_passthrough = true; >> host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); >> x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); >> @@ -1888,6 +1890,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) >> env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX]; >> env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX]; >> env->cpuid_xlevel2 = def->xlevel2; >> + cpu->cache_info_passthrough = def->cache_info_passthrough; >> object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp); >> } >> @@ -2062,6 +2065,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, >> break; >> case 2: >> /* cache info: needed for Pentium Pro compatibility */ >> + if (cpu->cache_info_passthrough) { >> + host_cpuid(index, 0, eax, ebx, ecx, edx); >> + break; >> + } >> *eax = 1; /* Number of CPUID[EAX=2] calls required */ >> *ebx = 0; >> *ecx = 0; >> @@ -2071,6 +2078,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, >> break; >> case 4: >> /* cache info: needed for Core compatibility */ >> + if (cpu->cache_info_passthrough) { >> + host_cpuid(index, count, eax, ebx, ecx, edx); >> + break; >> + } >> if (cs->nr_cores > 1) { >> *eax = (cs->nr_cores - 1) << 26; >> } else { >> @@ -2228,6 +2239,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, >> break; >> case 0x80000005: >> /* cache info (L1 cache) */ >> + if (cpu->cache_info_passthrough) { >> + host_cpuid(index, 0, eax, ebx, ecx, edx); >> + break; >> + } >> *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) | \ >> (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES); >> *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | \ >> @@ -2239,6 +2254,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, >> break; >> case 0x80000006: >> /* cache info (L2 cache) */ >> + if (cpu->cache_info_passthrough) { >> + host_cpuid(index, 0, eax, ebx, ecx, edx); >> + break; >> + } >> *eax = (AMD_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) | \ >> (L2_DTLB_2M_ENTRIES << 16) | \ >> (AMD_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) | \ > >
Il 18/11/2013 16:37, Peter Lieven ha scritto: >> >> >> If I specify: -smp 2,sockets=1,cores=2,threads=1 to a Windows 2012 R2 >> Server it crashes >> at boot time. -smp 2 works. > for Linux /proc/cpuinfo reveals no cpu layout information (sibliings, > cores, threads etc.) with > this patch applied and a manual socket,core,thread configuration. What's the full command line? Paolo
Am 18.11.2013 17:11, schrieb Paolo Bonzini: > Il 18/11/2013 16:37, Peter Lieven ha scritto: >>> >>> If I specify: -smp 2,sockets=1,cores=2,threads=1 to a Windows 2012 R2 >>> Server it crashes >>> at boot time. -smp 2 works. >> for Linux /proc/cpuinfo reveals no cpu layout information (sibliings, >> cores, threads etc.) with >> this patch applied and a manual socket,core,thread configuration. > What's the full command line? The essential part is -enable-kvm -smp 2,sockets=1,cores=2,threads=1 -cpu host. I believe the corect fix could be to disabled the cache leave forwarding as soon as the user specifies his own socket/core/thread layout. Peter
On 18.11.2013 17:11, Paolo Bonzini wrote: > Il 18/11/2013 16:37, Peter Lieven ha scritto: >>> >>> If I specify: -smp 2,sockets=1,cores=2,threads=1 to a Windows 2012 R2 >>> Server it crashes >>> at boot time. -smp 2 works. >> for Linux /proc/cpuinfo reveals no cpu layout information (sibliings, >> cores, threads etc.) with >> this patch applied and a manual socket,core,thread configuration. > What's the full command line? ~/git/qemu$ x86_64-softmmu/qemu-system-x86_64 -m 2048 -drive if=virtio,file=iscsi://172.21.200.45/iqn.2001-05.com.equallogic:0-8a0906-9d95c510a-344001d54795289f-2012-r2-1-7-0/0,format=raw,cache=writeback,aio=native -smp 2,cores=2,threads=1,sockets=1 -cpu host -monitor stdio -vnc :1 -enable-kvm -usb -usbdevice tablet -vga cirrus -global virtio-blk-pci.scsi=off -serial null -parallel null -boot c With just -smp 2 it works. However, have a look at my other email I think there is a bug in smp_parse, because -smp 2 yields cpus=2,cores=1,threads=1,sockets=1 whereas I think cores should be 2. Peter
Il 19/11/2013 11:25, Peter Lieven ha scritto: >> > > ~/git/qemu$ x86_64-softmmu/qemu-system-x86_64 -m 2048 -drive > if=virtio,file=iscsi://172.21.200.45/iqn.2001-05.com.equallogic:0-8a0906-9d95c510a-344001d54795289f-2012-r2-1-7-0/0,format=raw,cache=writeback,aio=native > -smp 2,cores=2,threads=1,sockets=1 -cpu host -monitor stdio -vnc :1 > -enable-kvm -usb -usbdevice tablet -vga cirrus -global > virtio-blk-pci.scsi=off -serial null -parallel null -boot c What is your host CPU's topology > With just -smp 2 it works. However, have a look at my other email I > think there is a bug in smp_parse, because -smp 2 yields > cpus=2,cores=1,threads=1,sockets=1 whereas I think cores should > be 2. The code matching the comment in vl.c ("compute missing values, prefer sockets over cores over threads") would be like "-smp cpu=2,cores=1,threads=1,sockets=2", giving this code: if (cpus == 0) { sockets = sockets > 0 ? sockets : 1; cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; cpus = cores * threads * sockets; } else if (sockets == 0) { cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; sockets = cpus / (cores * threads); } else if (cores == 0) { threads = threads > 0 ? threads : 1; cores = cpus / (sockets * threads); } else { threads = cpus / (sockets * cores); } What you suggest is cores over threads over sockets: if (cpus == 0) { cores = cores > 0 ? cores : 1; threads = threads > 0 ? threads : 1; sockets = sockets > 0 ? sockets : 1; cpus = cores * threads * sockets; } else if (cores == 0) { threads = threads > 0 ? threads : 1; sockets = sockets > 0 ? sockets : 1; cores = cpus / (threads * sockets); } else if (threads == 0) { sockets = sockets > 0 ? sockets : 1; threads = cpus / (cores * sockets); } else { sockets = cpus / (cores * threads); } Can you test which of these two work? But I agree it's best to disable cache-leaf forwarding. Paolo
On 19.11.2013 11:47, Paolo Bonzini wrote: > Il 19/11/2013 11:25, Peter Lieven ha scritto: >> ~/git/qemu$ x86_64-softmmu/qemu-system-x86_64 -m 2048 -drive >> if=virtio,file=iscsi://172.21.200.45/iqn.2001-05.com.equallogic:0-8a0906-9d95c510a-344001d54795289f-2012-r2-1-7-0/0,format=raw,cache=writeback,aio=native >> -smp 2,cores=2,threads=1,sockets=1 -cpu host -monitor stdio -vnc :1 >> -enable-kvm -usb -usbdevice tablet -vga cirrus -global >> virtio-blk-pci.scsi=off -serial null -parallel null -boot c > What is your host CPU's topology I tested it with 1 socket, 2 cores, 2 threads per core (my workstation) and 2 sockets, 8 cores per socket, 2 threads per thread. Both crash. > >> With just -smp 2 it works. However, have a look at my other email I >> think there is a bug in smp_parse, because -smp 2 yields >> cpus=2,cores=1,threads=1,sockets=1 whereas I think cores should >> be 2. > The code matching the comment in vl.c ("compute missing values, prefer > sockets over cores over threads") would be like "-smp > cpu=2,cores=1,threads=1,sockets=2", giving this code: > > if (cpus == 0) { > sockets = sockets > 0 ? sockets : 1; > cores = cores > 0 ? cores : 1; > threads = threads > 0 ? threads : 1; > cpus = cores * threads * sockets; > } else if (sockets == 0) { > cores = cores > 0 ? cores : 1; > threads = threads > 0 ? threads : 1; > sockets = cpus / (cores * threads); > } else if (cores == 0) { > threads = threads > 0 ? threads : 1; > cores = cpus / (sockets * threads); > } else { > threads = cpus / (sockets * cores); > } I am fine with either of the both variants, it should just be consistent ;-) > > What you suggest is cores over threads over sockets: > > if (cpus == 0) { > cores = cores > 0 ? cores : 1; > threads = threads > 0 ? threads : 1; > sockets = sockets > 0 ? sockets : 1; > cpus = cores * threads * sockets; > } else if (cores == 0) { > threads = threads > 0 ? threads : 1; > sockets = sockets > 0 ? sockets : 1; > cores = cpus / (threads * sockets); > } else if (threads == 0) { > sockets = sockets > 0 ? sockets : 1; > threads = cpus / (cores * sockets); > } else { > sockets = cpus / (cores * threads); > } > > Can you test which of these two work? But I agree it's best to disable > cache-leaf forwarding. The problem is, its broken because at least cpuid index 4 includes a hint to the number of cores and threads. I think we have to disable the cache leaf forwarding if the qemu cpu topology does not match the host topology. I also tried to fix index 4, but this alone seems to be not enough. at least in index 2 seems also to be some info about cores and threads (which is currently not there).
On 19.11.2013 11:47, Paolo Bonzini wrote: > Il 19/11/2013 11:25, Peter Lieven ha scritto: >> ~/git/qemu$ x86_64-softmmu/qemu-system-x86_64 -m 2048 -drive >> if=virtio,file=iscsi://172.21.200.45/iqn.2001-05.com.equallogic:0-8a0906-9d95c510a-344001d54795289f-2012-r2-1-7-0/0,format=raw,cache=writeback,aio=native >> -smp 2,cores=2,threads=1,sockets=1 -cpu host -monitor stdio -vnc :1 >> -enable-kvm -usb -usbdevice tablet -vga cirrus -global >> virtio-blk-pci.scsi=off -serial null -parallel null -boot c > What is your host CPU's topology > >> With just -smp 2 it works. However, have a look at my other email I >> think there is a bug in smp_parse, because -smp 2 yields >> cpus=2,cores=1,threads=1,sockets=1 whereas I think cores should >> be 2. > The code matching the comment in vl.c ("compute missing values, prefer > sockets over cores over threads") would be like "-smp > cpu=2,cores=1,threads=1,sockets=2", giving this code: > > if (cpus == 0) { > sockets = sockets > 0 ? sockets : 1; > cores = cores > 0 ? cores : 1; > threads = threads > 0 ? threads : 1; > cpus = cores * threads * sockets; > } else if (sockets == 0) { > cores = cores > 0 ? cores : 1; > threads = threads > 0 ? threads : 1; > sockets = cpus / (cores * threads); > } else if (cores == 0) { > threads = threads > 0 ? threads : 1; > cores = cpus / (sockets * threads); > } else { > threads = cpus / (sockets * cores); > } > > What you suggest is cores over threads over sockets: > > if (cpus == 0) { > cores = cores > 0 ? cores : 1; > threads = threads > 0 ? threads : 1; > sockets = sockets > 0 ? sockets : 1; > cpus = cores * threads * sockets; > } else if (cores == 0) { > threads = threads > 0 ? threads : 1; > sockets = sockets > 0 ? sockets : 1; > cores = cpus / (threads * sockets); > } else if (threads == 0) { > sockets = sockets > 0 ? sockets : 1; > threads = cpus / (cores * sockets); > } else { > sockets = cpus / (cores * threads); > } > > Can you test which of these two work? But I agree it's best to disable > cache-leaf forwarding. The first does make windows boot again and it calculates a correct combination of cpus, threads, cores and sockets. But I think the reason it boots is because cores=threads=1. As its more intuitive (I think) I would prefer your "cores over threads over socket ". The last thing I would think of is emulating more than 1 socket. -smp N would then mean, N cores, no hyper-threading, 1 socket.
On 19.11.2013 13:03, Peter Lieven wrote: > On 19.11.2013 11:47, Paolo Bonzini wrote: >> Il 19/11/2013 11:25, Peter Lieven ha scritto: >>> ~/git/qemu$ x86_64-softmmu/qemu-system-x86_64 -m 2048 -drive >>> if=virtio,file=iscsi://172.21.200.45/iqn.2001-05.com.equallogic:0-8a0906-9d95c510a-344001d54795289f-2012-r2-1-7-0/0,format=raw,cache=writeback,aio=native >>> -smp 2,cores=2,threads=1,sockets=1 -cpu host -monitor stdio -vnc :1 >>> -enable-kvm -usb -usbdevice tablet -vga cirrus -global >>> virtio-blk-pci.scsi=off -serial null -parallel null -boot c >> What is your host CPU's topology >> >>> With just -smp 2 it works. However, have a look at my other email I >>> think there is a bug in smp_parse, because -smp 2 yields >>> cpus=2,cores=1,threads=1,sockets=1 whereas I think cores should >>> be 2. >> The code matching the comment in vl.c ("compute missing values, prefer >> sockets over cores over threads") would be like "-smp >> cpu=2,cores=1,threads=1,sockets=2", giving this code: >> >> if (cpus == 0) { >> sockets = sockets > 0 ? sockets : 1; >> cores = cores > 0 ? cores : 1; >> threads = threads > 0 ? threads : 1; >> cpus = cores * threads * sockets; >> } else if (sockets == 0) { >> cores = cores > 0 ? cores : 1; >> threads = threads > 0 ? threads : 1; >> sockets = cpus / (cores * threads); >> } else if (cores == 0) { >> threads = threads > 0 ? threads : 1; >> cores = cpus / (sockets * threads); >> } else { >> threads = cpus / (sockets * cores); >> } >> >> What you suggest is cores over threads over sockets: >> >> if (cpus == 0) { >> cores = cores > 0 ? cores : 1; >> threads = threads > 0 ? threads : 1; >> sockets = sockets > 0 ? sockets : 1; >> cpus = cores * threads * sockets; >> } else if (cores == 0) { >> threads = threads > 0 ? threads : 1; >> sockets = sockets > 0 ? sockets : 1; >> cores = cpus / (threads * sockets); >> } else if (threads == 0) { >> sockets = sockets > 0 ? sockets : 1; >> threads = cpus / (cores * sockets); >> } else { >> sockets = cpus / (cores * threads); >> } >> >> Can you test which of these two work? But I agree it's best to disable >> cache-leaf forwarding. > The first does make windows boot again and it calculates a > correct combination of cpus, threads, cores and sockets. But > I think the reason it boots is because cores=threads=1. Forgot to mention: In this case the information about cores and threads is not retreived from additional indexes. bits 16..23 in ebx in index 0x00000001 are zero. So bottom line, the whole cache leaf passthru thing only worked because of a bug in smp_parse yielding threads and cores 1 by default.
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index c4447c2..f4fab15 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -70,6 +70,9 @@ typedef struct X86CPU { bool hyperv_relaxed_timing; int hyperv_spinlock_attempts; + /* if true the CPUID code directly forward host cache leaves to the guest */ + bool cache_info_passthrough; + /* Features that were filtered out because of missing host capabilities */ uint32_t filtered_features[FEATURE_WORDS]; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index c36345e..46edd75 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -486,6 +486,7 @@ typedef struct x86_def_t { int stepping; FeatureWordArray features; char model_id[48]; + bool cache_info_passthrough; } x86_def_t; #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) @@ -1139,6 +1140,7 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) assert(kvm_enabled()); x86_cpu_def->name = "host"; + x86_cpu_def->cache_info_passthrough = true; host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); @@ -1888,6 +1890,7 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp) env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX]; env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX]; env->cpuid_xlevel2 = def->xlevel2; + cpu->cache_info_passthrough = def->cache_info_passthrough; object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp); } @@ -2062,6 +2065,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 2: /* cache info: needed for Pentium Pro compatibility */ + if (cpu->cache_info_passthrough) { + host_cpuid(index, 0, eax, ebx, ecx, edx); + break; + } *eax = 1; /* Number of CPUID[EAX=2] calls required */ *ebx = 0; *ecx = 0; @@ -2071,6 +2078,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 4: /* cache info: needed for Core compatibility */ + if (cpu->cache_info_passthrough) { + host_cpuid(index, count, eax, ebx, ecx, edx); + break; + } if (cs->nr_cores > 1) { *eax = (cs->nr_cores - 1) << 26; } else { @@ -2228,6 +2239,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 0x80000005: /* cache info (L1 cache) */ + if (cpu->cache_info_passthrough) { + host_cpuid(index, 0, eax, ebx, ecx, edx); + break; + } *eax = (L1_DTLB_2M_ASSOC << 24) | (L1_DTLB_2M_ENTRIES << 16) | \ (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES); *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | \ @@ -2239,6 +2254,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; case 0x80000006: /* cache info (L2 cache) */ + if (cpu->cache_info_passthrough) { + host_cpuid(index, 0, eax, ebx, ecx, edx); + break; + } *eax = (AMD_ENC_ASSOC(L2_DTLB_2M_ASSOC) << 28) | \ (L2_DTLB_2M_ENTRIES << 16) | \ (AMD_ENC_ASSOC(L2_ITLB_2M_ASSOC) << 12) | \