* [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU
@ 2018-05-10 20:41 Babu Moger
2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently Babu Moger
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw)
To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti
Cc: qemu-devel, kvm, geoff, kash, babu.moger
This series enables the TOPOEXT feature for AMD CPUs. This is required to
support hyperthreading on kvm guests.
This addresses the issues reported in these bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1481253
https://bugs.launchpad.net/qemu/+bug/1703506
v8:
Addressed feedback from Eduardo. Thanks Eduardo for being patient with me.
Tested on AMD EPYC server and also did some basic testing on intel box.
Summary of changes.
1. Reverted back l2 cache associativity. Kept it same as legacy.
2. Changed cache_info structure in X86CPUDefinition and CPUX86State to pointers.
3. Added legacy_cache property in PC_COMPAT_2_12 and initialized legacy_cache
based on static cache_info availability.
4. Squashed patch 4 and 5 and applied it before patch 3.
5. Added legacy cache check for cpuid[2] and cpuid[4] for consistancy.
6. Simplified NUM_SHARING_CACHE definition for readability,
7. Removed assert for core_id as it appeared redundant.
8. Simplified encode_cache_cpuid8000001d little bit.
9. Few more minor changes
v7:
Rebased on top of latest tree after 2.12 release and done few basic tests. There are
no changes except for few minor hunks. Hopefully this gets pulled into 2.13 release.
Please review, let me know of any feedback.
v6:
1.Fixed problem with patch#4(Add new property to control cache info). The parameter
legacy_cache should be "on" by default on machine type "pc-q35-2.10". This was
found by Alexandr Iarygin.
2.Fixed the l3 cache size for EPYC based machines(patch#3). Also, fixed the number of
logical processors sharing the cache(patch#6). Only L3 cache is shared by multiple
cores but not L1 or L2. This was a bug while decoding. This was found by Geoffrey McRae
and he verified the fix.
v5:
In this series I tried to address the feedback from Eduardo Habkost.
The discussion thread is here.
https://patchwork.kernel.org/patch/10299745/
The previous thread is here.
http://patchwork.ozlabs.org/cover/884885/
Reason for these changes.
The cache properties for AMD family of processors have changed from
previous releases. We don't want to display the new information on the
old family of processors as this might cause compatibility issues.
Changes:
1.Based the patches on top of Eduardo's(patch#1) patch.
Changed few things.
Moved the Cache definitions to cpu.h file.
Changed the CPUID_4 names to generic names.
2.Added a new propery "legacy-cache" in cpu object(patch#2). This can be
used to display the old property even if the host supports the new cache
properties.
3.Added cache information in X86CPUDefinition and CPUX86State
4.Patch 6-7 changed quite a bit from previous version does to new approach.
5.Addressed few issues with CPUID_8000_001d and CPUID_8000_001E.
v4:
1.Removed the checks under cpuid 0x8000001D leaf(patch #2). These check are
not necessary. Found this during internal review.
2.Added CPUID_EXT3_TOPOEXT feature for all the 17 family(patch #4). This was
found by Kash Pande during his testing.
3.Removed th hardcoded cpuid xlevel and dynamically extended if CPUID_EXT3_TOPOEXT
is supported(Suggested by Brijesh Singh).
v3:
1.Removed the patch #1. Radim mentioned that original typo problem is in
linux kernel header. qemu is just copying those files.
2.In previous version, I used the cpuid 4 definitions for AMDs cpuid leaf
0x8000001D. CPUID 4 is very intel specific and we dont want to expose those
details under AMD. I have renamed some of these definitions as generic.
These changes are in patch#1. Radim, let me know if this is what you intended.
3.Added assert to for core_id(Suggested by Radim Krcmár).
4.Changed the if condition under "L3 cache info"(Suggested by Gary Hook).
5.Addressed few more text correction and code cleanup(Suggested by Thomas Lendacky).
v2:
Fixed few more minor issues per Gary Hook's comments. Thank you Gary.
Removed the patch#1. We need to handle the instruction cache associativity
seperately. It varies based on the cpu family. I will comeback to that later.
Added two more typo corrections in patch#1 and patch#5.
v1:
Stanislav Lanci posted few patches earlier.
https://patchwork.kernel.org/patch/10040903/
Rebased his patches with few changes.
1.Spit the patches into two, separating cpuid functions
0x8000001D and 0x8000001E (Patch 2 and 3).
2.Removed the generic non-intel check and made a separate patch
with some changes(Patch 5).
3.Fixed L3_N_SETS_AMD(from 4096 to 8192) based on CPUID_Fn8000001D_ECX_x03.
Added 2 more patches.
Patch 1. Fixes cache associativity.
Patch 4. Adds TOPOEXT feature on AMD EPYC CPU.
Babu Moger (7):
i386: Add cache information in X86CPUDefinition
i386: Add new property to control cache info
i386: Initialize cache information for EPYC family processors
i386: Populate AMD Processor Cache Information for cpuid 0x8000001D
i386: Add support for CPUID_8000_001E for AMD
i386: Enable TOPOEXT feature on AMD EPYC CPU
i386: Remove generic SMT thread check
Eduardo Habkost (1):
i386: Helpers to encode cache information consistently
include/hw/i386/pc.h | 8 +
include/hw/i386/topology.h | 6 +
target/i386/cpu.c | 703 ++++++++++++++++++++++++++++++-------
target/i386/cpu.h | 65 ++++
target/i386/kvm.c | 29 +-
5 files changed, 677 insertions(+), 134 deletions(-)
--
2.17.0
^ permalink raw reply [flat|nested] 20+ messages in thread* [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger @ 2018-05-10 20:41 ` Babu Moger 2018-05-11 19:12 ` Eduardo Habkost 2018-07-16 13:20 ` Philippe Mathieu-Daudé 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 2/8] i386: Add cache information in X86CPUDefinition Babu Moger ` (6 subsequent siblings) 7 siblings, 2 replies; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger From: Eduardo Habkost <ehabkost@redhat.com> Instead of having a collection of macros that need to be used in complex expressions to build CPUID data, define a CPUCacheInfo struct that can hold information about a given cache. Helper functions will take a CPUCacheInfo struct as input to encode CPUID leaves for a cache. This will help us ensure consistency between cache information CPUID leaves, and make the existing inconsistencies in CPUID info more visible. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> --- target/i386/cpu.c | 495 ++++++++++++++++++++++++++++++++++------------ target/i386/cpu.h | 53 +++++ 2 files changed, 424 insertions(+), 124 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index a20fe26573..18835400f1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -56,33 +56,240 @@ #include "disas/capstone.h" +/* Helpers for building CPUID[2] descriptors: */ + +struct CPUID2CacheDescriptorInfo { + enum CacheType type; + int level; + int size; + int line_size; + int associativity; +}; -/* Cache topology CPUID constants: */ +#define KiB 1024 +#define MiB (1024 * 1024) -/* CPUID Leaf 2 Descriptors */ +/* + * Known CPUID 2 cache descriptors. + * From Intel SDM Volume 2A, CPUID instruction + */ +struct CPUID2CacheDescriptorInfo cpuid2_cache_descriptors[] = { + [0x06] = { .level = 1, .type = ICACHE, .size = 8 * KiB, + .associativity = 4, .line_size = 32, }, + [0x08] = { .level = 1, .type = ICACHE, .size = 16 * KiB, + .associativity = 4, .line_size = 32, }, + [0x09] = { .level = 1, .type = ICACHE, .size = 32 * KiB, + .associativity = 4, .line_size = 64, }, + [0x0A] = { .level = 1, .type = DCACHE, .size = 8 * KiB, + .associativity = 2, .line_size = 32, }, + [0x0C] = { .level = 1, .type = DCACHE, .size = 16 * KiB, + .associativity = 4, .line_size = 32, }, + [0x0D] = { .level = 1, .type = DCACHE, .size = 16 * KiB, + .associativity = 4, .line_size = 64, }, + [0x0E] = { .level = 1, .type = DCACHE, .size = 24 * KiB, + .associativity = 6, .line_size = 64, }, + [0x1D] = { .level = 2, .type = UNIFIED_CACHE, .size = 128 * KiB, + .associativity = 2, .line_size = 64, }, + [0x21] = { .level = 2, .type = UNIFIED_CACHE, .size = 256 * KiB, + .associativity = 8, .line_size = 64, }, + /* lines per sector is not supported cpuid2_cache_descriptor(), + * so descriptors 0x22, 0x23 are not included + */ + [0x24] = { .level = 2, .type = UNIFIED_CACHE, .size = 1 * MiB, + .associativity = 16, .line_size = 64, }, + /* lines per sector is not supported cpuid2_cache_descriptor(), + * so descriptors 0x25, 0x20 are not included + */ + [0x2C] = { .level = 1, .type = DCACHE, .size = 32 * KiB, + .associativity = 8, .line_size = 64, }, + [0x30] = { .level = 1, .type = ICACHE, .size = 32 * KiB, + .associativity = 8, .line_size = 64, }, + [0x41] = { .level = 2, .type = UNIFIED_CACHE, .size = 128 * KiB, + .associativity = 4, .line_size = 32, }, + [0x42] = { .level = 2, .type = UNIFIED_CACHE, .size = 256 * KiB, + .associativity = 4, .line_size = 32, }, + [0x43] = { .level = 2, .type = UNIFIED_CACHE, .size = 512 * KiB, + .associativity = 4, .line_size = 32, }, + [0x44] = { .level = 2, .type = UNIFIED_CACHE, .size = 1 * MiB, + .associativity = 4, .line_size = 32, }, + [0x45] = { .level = 2, .type = UNIFIED_CACHE, .size = 2 * MiB, + .associativity = 4, .line_size = 32, }, + [0x46] = { .level = 3, .type = UNIFIED_CACHE, .size = 4 * MiB, + .associativity = 4, .line_size = 64, }, + [0x47] = { .level = 3, .type = UNIFIED_CACHE, .size = 8 * MiB, + .associativity = 8, .line_size = 64, }, + [0x48] = { .level = 2, .type = UNIFIED_CACHE, .size = 3 * MiB, + .associativity = 12, .line_size = 64, }, + /* Descriptor 0x49 depends on CPU family/model, so it is not included */ + [0x4A] = { .level = 3, .type = UNIFIED_CACHE, .size = 6 * MiB, + .associativity = 12, .line_size = 64, }, + [0x4B] = { .level = 3, .type = UNIFIED_CACHE, .size = 8 * MiB, + .associativity = 16, .line_size = 64, }, + [0x4C] = { .level = 3, .type = UNIFIED_CACHE, .size = 12 * MiB, + .associativity = 12, .line_size = 64, }, + [0x4D] = { .level = 3, .type = UNIFIED_CACHE, .size = 16 * MiB, + .associativity = 16, .line_size = 64, }, + [0x4E] = { .level = 2, .type = UNIFIED_CACHE, .size = 6 * MiB, + .associativity = 24, .line_size = 64, }, + [0x60] = { .level = 1, .type = DCACHE, .size = 16 * KiB, + .associativity = 8, .line_size = 64, }, + [0x66] = { .level = 1, .type = DCACHE, .size = 8 * KiB, + .associativity = 4, .line_size = 64, }, + [0x67] = { .level = 1, .type = DCACHE, .size = 16 * KiB, + .associativity = 4, .line_size = 64, }, + [0x68] = { .level = 1, .type = DCACHE, .size = 32 * KiB, + .associativity = 4, .line_size = 64, }, + [0x78] = { .level = 2, .type = UNIFIED_CACHE, .size = 1 * MiB, + .associativity = 4, .line_size = 64, }, + /* lines per sector is not supported cpuid2_cache_descriptor(), + * so descriptors 0x79, 0x7A, 0x7B, 0x7C are not included. + */ + [0x7D] = { .level = 2, .type = UNIFIED_CACHE, .size = 2 * MiB, + .associativity = 8, .line_size = 64, }, + [0x7F] = { .level = 2, .type = UNIFIED_CACHE, .size = 512 * KiB, + .associativity = 2, .line_size = 64, }, + [0x80] = { .level = 2, .type = UNIFIED_CACHE, .size = 512 * KiB, + .associativity = 8, .line_size = 64, }, + [0x82] = { .level = 2, .type = UNIFIED_CACHE, .size = 256 * KiB, + .associativity = 8, .line_size = 32, }, + [0x83] = { .level = 2, .type = UNIFIED_CACHE, .size = 512 * KiB, + .associativity = 8, .line_size = 32, }, + [0x84] = { .level = 2, .type = UNIFIED_CACHE, .size = 1 * MiB, + .associativity = 8, .line_size = 32, }, + [0x85] = { .level = 2, .type = UNIFIED_CACHE, .size = 2 * MiB, + .associativity = 8, .line_size = 32, }, + [0x86] = { .level = 2, .type = UNIFIED_CACHE, .size = 512 * KiB, + .associativity = 4, .line_size = 64, }, + [0x87] = { .level = 2, .type = UNIFIED_CACHE, .size = 1 * MiB, + .associativity = 8, .line_size = 64, }, + [0xD0] = { .level = 3, .type = UNIFIED_CACHE, .size = 512 * KiB, + .associativity = 4, .line_size = 64, }, + [0xD1] = { .level = 3, .type = UNIFIED_CACHE, .size = 1 * MiB, + .associativity = 4, .line_size = 64, }, + [0xD2] = { .level = 3, .type = UNIFIED_CACHE, .size = 2 * MiB, + .associativity = 4, .line_size = 64, }, + [0xD6] = { .level = 3, .type = UNIFIED_CACHE, .size = 1 * MiB, + .associativity = 8, .line_size = 64, }, + [0xD7] = { .level = 3, .type = UNIFIED_CACHE, .size = 2 * MiB, + .associativity = 8, .line_size = 64, }, + [0xD8] = { .level = 3, .type = UNIFIED_CACHE, .size = 4 * MiB, + .associativity = 8, .line_size = 64, }, + [0xDC] = { .level = 3, .type = UNIFIED_CACHE, .size = 1.5 * MiB, + .associativity = 12, .line_size = 64, }, + [0xDD] = { .level = 3, .type = UNIFIED_CACHE, .size = 3 * MiB, + .associativity = 12, .line_size = 64, }, + [0xDE] = { .level = 3, .type = UNIFIED_CACHE, .size = 6 * MiB, + .associativity = 12, .line_size = 64, }, + [0xE2] = { .level = 3, .type = UNIFIED_CACHE, .size = 2 * MiB, + .associativity = 16, .line_size = 64, }, + [0xE3] = { .level = 3, .type = UNIFIED_CACHE, .size = 4 * MiB, + .associativity = 16, .line_size = 64, }, + [0xE4] = { .level = 3, .type = UNIFIED_CACHE, .size = 8 * MiB, + .associativity = 16, .line_size = 64, }, + [0xEA] = { .level = 3, .type = UNIFIED_CACHE, .size = 12 * MiB, + .associativity = 24, .line_size = 64, }, + [0xEB] = { .level = 3, .type = UNIFIED_CACHE, .size = 18 * MiB, + .associativity = 24, .line_size = 64, }, + [0xEC] = { .level = 3, .type = UNIFIED_CACHE, .size = 24 * MiB, + .associativity = 24, .line_size = 64, }, +}; + +/* + * "CPUID leaf 2 does not report cache descriptor information, + * use CPUID leaf 4 to query cache parameters" + */ +#define CACHE_DESCRIPTOR_UNAVAILABLE 0xFF -#define CPUID_2_L1D_32KB_8WAY_64B 0x2c -#define CPUID_2_L1I_32KB_8WAY_64B 0x30 -#define CPUID_2_L2_2MB_8WAY_64B 0x7d -#define CPUID_2_L3_16MB_16WAY_64B 0x4d +/* + * Return a CPUID 2 cache descriptor for a given cache. + * If no known descriptor is found, return CACHE_DESCRIPTOR_UNAVAILABLE + */ +static uint8_t cpuid2_cache_descriptor(CPUCacheInfo *cache) +{ + int i; + + assert(cache->size > 0); + assert(cache->level > 0); + assert(cache->line_size > 0); + assert(cache->associativity > 0); + for (i = 0; i < ARRAY_SIZE(cpuid2_cache_descriptors); i++) { + struct CPUID2CacheDescriptorInfo *d = &cpuid2_cache_descriptors[i]; + if (d->level == cache->level && d->type == cache->type && + d->size == cache->size && d->line_size == cache->line_size && + d->associativity == cache->associativity) { + return i; + } + } + return CACHE_DESCRIPTOR_UNAVAILABLE; +} /* CPUID Leaf 4 constants: */ /* EAX: */ -#define CPUID_4_TYPE_DCACHE 1 -#define CPUID_4_TYPE_ICACHE 2 -#define CPUID_4_TYPE_UNIFIED 3 +#define CACHE_TYPE_D 1 +#define CACHE_TYPE_I 2 +#define CACHE_TYPE_UNIFIED 3 -#define CPUID_4_LEVEL(l) ((l) << 5) +#define CACHE_LEVEL(l) (l << 5) -#define CPUID_4_SELF_INIT_LEVEL (1 << 8) -#define CPUID_4_FULLY_ASSOC (1 << 9) +#define CACHE_SELF_INIT_LEVEL (1 << 8) /* EDX: */ -#define CPUID_4_NO_INVD_SHARING (1 << 0) -#define CPUID_4_INCLUSIVE (1 << 1) -#define CPUID_4_COMPLEX_IDX (1 << 2) +#define CACHE_NO_INVD_SHARING (1 << 0) +#define CACHE_INCLUSIVE (1 << 1) +#define CACHE_COMPLEX_IDX (1 << 2) + +/* Encode CacheType for CPUID[4].EAX */ +#define CACHE_TYPE(t) (((t) == DCACHE) ? CACHE_TYPE_D : \ + ((t) == ICACHE) ? CACHE_TYPE_I : \ + ((t) == UNIFIED_CACHE) ? CACHE_TYPE_UNIFIED : \ + 0 /* Invalid value */) + + +/* Encode cache info for CPUID[4] */ +static void encode_cache_cpuid4(CPUCacheInfo *cache, + int num_apic_ids, int num_cores, + uint32_t *eax, uint32_t *ebx, + uint32_t *ecx, uint32_t *edx) +{ + assert(cache->size == cache->line_size * cache->associativity * + cache->partitions * cache->sets); + + assert(num_apic_ids > 0); + *eax = CACHE_TYPE(cache->type) | + CACHE_LEVEL(cache->level) | + (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0) | + ((num_cores - 1) << 26) | + ((num_apic_ids - 1) << 14); + + assert(cache->line_size > 0); + assert(cache->partitions > 0); + assert(cache->associativity > 0); + /* We don't implement fully-associative caches */ + assert(cache->associativity < cache->sets); + *ebx = (cache->line_size - 1) | + ((cache->partitions - 1) << 12) | + ((cache->associativity - 1) << 22); + + assert(cache->sets > 0); + *ecx = cache->sets - 1; + + *edx = (cache->no_invd_sharing ? CACHE_NO_INVD_SHARING : 0) | + (cache->inclusive ? CACHE_INCLUSIVE : 0) | + (cache->complex_indexing ? CACHE_COMPLEX_IDX : 0); +} + +/* Encode cache info for CPUID[0x80000005].ECX or CPUID[0x80000005].EDX */ +static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache) +{ + assert(cache->size % 1024 == 0); + assert(cache->lines_per_tag > 0); + assert(cache->associativity > 0); + assert(cache->line_size > 0); + return ((cache->size / 1024) << 24) | (cache->associativity << 16) | + (cache->lines_per_tag << 8) | (cache->line_size); +} #define ASSOC_FULL 0xFF @@ -100,57 +307,140 @@ a == ASSOC_FULL ? 0xF : \ 0 /* invalid value */) +/* + * Encode cache info for CPUID[0x80000006].ECX and CPUID[0x80000006].EDX + * @l3 can be NULL. + */ +static void encode_cache_cpuid80000006(CPUCacheInfo *l2, + CPUCacheInfo *l3, + uint32_t *ecx, uint32_t *edx) +{ + assert(l2->size % 1024 == 0); + assert(l2->associativity > 0); + assert(l2->lines_per_tag > 0); + assert(l2->line_size > 0); + *ecx = ((l2->size / 1024) << 16) | + (AMD_ENC_ASSOC(l2->associativity) << 12) | + (l2->lines_per_tag << 8) | (l2->line_size); + + if (l3) { + assert(l3->size % (512 * 1024) == 0); + assert(l3->associativity > 0); + assert(l3->lines_per_tag > 0); + assert(l3->line_size > 0); + *edx = ((l3->size / (512 * 1024)) << 18) | + (AMD_ENC_ASSOC(l3->associativity) << 12) | + (l3->lines_per_tag << 8) | (l3->line_size); + } else { + *edx = 0; + } +} /* Definitions of the hardcoded cache entries we expose: */ /* L1 data cache: */ -#define L1D_LINE_SIZE 64 -#define L1D_ASSOCIATIVITY 8 -#define L1D_SETS 64 -#define L1D_PARTITIONS 1 -/* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 32KiB */ -#define L1D_DESCRIPTOR CPUID_2_L1D_32KB_8WAY_64B +static CPUCacheInfo l1d_cache = { + .type = DCACHE, + .level = 1, + .size = 32 * KiB, + .self_init = 1, + .line_size = 64, + .associativity = 8, + .sets = 64, + .partitions = 1, + .no_invd_sharing = true, +}; + /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */ -#define L1D_LINES_PER_TAG 1 -#define L1D_SIZE_KB_AMD 64 -#define L1D_ASSOCIATIVITY_AMD 2 +static CPUCacheInfo l1d_cache_amd = { + .type = DCACHE, + .level = 1, + .size = 64 * KiB, + .self_init = 1, + .line_size = 64, + .associativity = 2, + .sets = 512, + .partitions = 1, + .lines_per_tag = 1, + .no_invd_sharing = true, +}; /* L1 instruction cache: */ -#define L1I_LINE_SIZE 64 -#define L1I_ASSOCIATIVITY 8 -#define L1I_SETS 64 -#define L1I_PARTITIONS 1 -/* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 32KiB */ -#define L1I_DESCRIPTOR CPUID_2_L1I_32KB_8WAY_64B +static CPUCacheInfo l1i_cache = { + .type = ICACHE, + .level = 1, + .size = 32 * KiB, + .self_init = 1, + .line_size = 64, + .associativity = 8, + .sets = 64, + .partitions = 1, + .no_invd_sharing = true, +}; + /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */ -#define L1I_LINES_PER_TAG 1 -#define L1I_SIZE_KB_AMD 64 -#define L1I_ASSOCIATIVITY_AMD 2 +static CPUCacheInfo l1i_cache_amd = { + .type = ICACHE, + .level = 1, + .size = 64 * KiB, + .self_init = 1, + .line_size = 64, + .associativity = 2, + .sets = 512, + .partitions = 1, + .lines_per_tag = 1, + .no_invd_sharing = true, +}; /* Level 2 unified cache: */ -#define L2_LINE_SIZE 64 -#define L2_ASSOCIATIVITY 16 -#define L2_SETS 4096 -#define L2_PARTITIONS 1 -/* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 4MiB */ +static CPUCacheInfo l2_cache = { + .type = UNIFIED_CACHE, + .level = 2, + .size = 4 * MiB, + .self_init = 1, + .line_size = 64, + .associativity = 16, + .sets = 4096, + .partitions = 1, + .no_invd_sharing = true, +}; + /*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */ -#define L2_DESCRIPTOR CPUID_2_L2_2MB_8WAY_64B +static CPUCacheInfo l2_cache_cpuid2 = { + .type = UNIFIED_CACHE, + .level = 2, + .size = 2 * MiB, + .line_size = 64, + .associativity = 8, +}; + + /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */ -#define L2_LINES_PER_TAG 1 -#define L2_SIZE_KB_AMD 512 +static CPUCacheInfo l2_cache_amd = { + .type = UNIFIED_CACHE, + .level = 2, + .size = 512 * KiB, + .line_size = 64, + .lines_per_tag = 1, + .associativity = 16, + .sets = 512, + .partitions = 1, +}; /* Level 3 unified cache: */ -#define L3_SIZE_KB 0 /* disabled */ -#define L3_ASSOCIATIVITY 0 /* disabled */ -#define L3_LINES_PER_TAG 0 /* disabled */ -#define L3_LINE_SIZE 0 /* disabled */ -#define L3_N_LINE_SIZE 64 -#define L3_N_ASSOCIATIVITY 16 -#define L3_N_SETS 16384 -#define L3_N_PARTITIONS 1 -#define L3_N_DESCRIPTOR CPUID_2_L3_16MB_16WAY_64B -#define L3_N_LINES_PER_TAG 1 -#define L3_N_SIZE_KB_AMD 16384 +static CPUCacheInfo l3_cache = { + .type = UNIFIED_CACHE, + .level = 3, + .size = 16 * MiB, + .line_size = 64, + .associativity = 16, + .sets = 16384, + .partitions = 1, + .lines_per_tag = 1, + .self_init = true, + .inclusive = true, + .complex_indexing = true, +}; /* TLB definitions: */ @@ -3301,85 +3591,53 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (!cpu->enable_l3_cache) { *ecx = 0; } else { - *ecx = L3_N_DESCRIPTOR; + *ecx = cpuid2_cache_descriptor(&l3_cache); } - *edx = (L1D_DESCRIPTOR << 16) | \ - (L1I_DESCRIPTOR << 8) | \ - (L2_DESCRIPTOR); + *edx = (cpuid2_cache_descriptor(&l1d_cache) << 16) | + (cpuid2_cache_descriptor(&l1i_cache) << 8) | + (cpuid2_cache_descriptor(&l2_cache_cpuid2)); break; case 4: /* cache info: needed for Core compatibility */ if (cpu->cache_info_passthrough) { host_cpuid(index, count, eax, ebx, ecx, edx); + /* QEMU gives out its own APIC IDs, never pass down bits 31..26. */ *eax &= ~0xFC000000; + if ((*eax & 31) && cs->nr_cores > 1) { + *eax |= (cs->nr_cores - 1) << 26; + } } else { *eax = 0; switch (count) { case 0: /* L1 dcache info */ - *eax |= CPUID_4_TYPE_DCACHE | \ - CPUID_4_LEVEL(1) | \ - CPUID_4_SELF_INIT_LEVEL; - *ebx = (L1D_LINE_SIZE - 1) | \ - ((L1D_PARTITIONS - 1) << 12) | \ - ((L1D_ASSOCIATIVITY - 1) << 22); - *ecx = L1D_SETS - 1; - *edx = CPUID_4_NO_INVD_SHARING; + encode_cache_cpuid4(&l1d_cache, + 1, cs->nr_cores, + eax, ebx, ecx, edx); break; case 1: /* L1 icache info */ - *eax |= CPUID_4_TYPE_ICACHE | \ - CPUID_4_LEVEL(1) | \ - CPUID_4_SELF_INIT_LEVEL; - *ebx = (L1I_LINE_SIZE - 1) | \ - ((L1I_PARTITIONS - 1) << 12) | \ - ((L1I_ASSOCIATIVITY - 1) << 22); - *ecx = L1I_SETS - 1; - *edx = CPUID_4_NO_INVD_SHARING; + encode_cache_cpuid4(&l1i_cache, + 1, cs->nr_cores, + eax, ebx, ecx, edx); break; case 2: /* L2 cache info */ - *eax |= CPUID_4_TYPE_UNIFIED | \ - CPUID_4_LEVEL(2) | \ - CPUID_4_SELF_INIT_LEVEL; - if (cs->nr_threads > 1) { - *eax |= (cs->nr_threads - 1) << 14; - } - *ebx = (L2_LINE_SIZE - 1) | \ - ((L2_PARTITIONS - 1) << 12) | \ - ((L2_ASSOCIATIVITY - 1) << 22); - *ecx = L2_SETS - 1; - *edx = CPUID_4_NO_INVD_SHARING; + encode_cache_cpuid4(&l2_cache, + cs->nr_threads, cs->nr_cores, + eax, ebx, ecx, edx); break; case 3: /* L3 cache info */ - if (!cpu->enable_l3_cache) { - *eax = 0; - *ebx = 0; - *ecx = 0; - *edx = 0; + pkg_offset = apicid_pkg_offset(cs->nr_cores, cs->nr_threads); + if (cpu->enable_l3_cache) { + encode_cache_cpuid4(&l3_cache, + (1 << pkg_offset), cs->nr_cores, + eax, ebx, ecx, edx); break; } - *eax |= CPUID_4_TYPE_UNIFIED | \ - CPUID_4_LEVEL(3) | \ - CPUID_4_SELF_INIT_LEVEL; - pkg_offset = apicid_pkg_offset(cs->nr_cores, cs->nr_threads); - *eax |= ((1 << pkg_offset) - 1) << 14; - *ebx = (L3_N_LINE_SIZE - 1) | \ - ((L3_N_PARTITIONS - 1) << 12) | \ - ((L3_N_ASSOCIATIVITY - 1) << 22); - *ecx = L3_N_SETS - 1; - *edx = CPUID_4_INCLUSIVE | CPUID_4_COMPLEX_IDX; - break; + /* fall through */ default: /* end of info */ - *eax = 0; - *ebx = 0; - *ecx = 0; - *edx = 0; + *eax = *ebx = *ecx = *edx = 0; break; } } - - /* QEMU gives out its own APIC IDs, never pass down bits 31..26. */ - if ((*eax & 31) && cs->nr_cores > 1) { - *eax |= (cs->nr_cores - 1) << 26; - } break; case 5: /* mwait info: needed for Core compatibility */ @@ -3583,10 +3841,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES); *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | \ (L1_ITLB_4K_ASSOC << 8) | (L1_ITLB_4K_ENTRIES); - *ecx = (L1D_SIZE_KB_AMD << 24) | (L1D_ASSOCIATIVITY_AMD << 16) | \ - (L1D_LINES_PER_TAG << 8) | (L1D_LINE_SIZE); - *edx = (L1I_SIZE_KB_AMD << 24) | (L1I_ASSOCIATIVITY_AMD << 16) | \ - (L1I_LINES_PER_TAG << 8) | (L1I_LINE_SIZE); + *ecx = encode_cache_cpuid80000005(&l1d_cache_amd); + *edx = encode_cache_cpuid80000005(&l1i_cache_amd); break; case 0x80000006: /* cache info (L2 cache) */ @@ -3602,18 +3858,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, (L2_DTLB_4K_ENTRIES << 16) | \ (AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) | \ (L2_ITLB_4K_ENTRIES); - *ecx = (L2_SIZE_KB_AMD << 16) | \ - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \ - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE); - if (!cpu->enable_l3_cache) { - *edx = ((L3_SIZE_KB / 512) << 18) | \ - (AMD_ENC_ASSOC(L3_ASSOCIATIVITY) << 12) | \ - (L3_LINES_PER_TAG << 8) | (L3_LINE_SIZE); - } else { - *edx = ((L3_N_SIZE_KB_AMD / 512) << 18) | \ - (AMD_ENC_ASSOC(L3_N_ASSOCIATIVITY) << 12) | \ - (L3_N_LINES_PER_TAG << 8) | (L3_N_LINE_SIZE); - } + encode_cache_cpuid80000006(&l2_cache_amd, + cpu->enable_l3_cache ? &l3_cache : NULL, + ecx, edx); break; case 0x80000007: *eax = 0; diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 1b219fafc4..fa03e2ced4 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1044,6 +1044,59 @@ typedef enum TPRAccess { TPR_ACCESS_WRITE, } TPRAccess; +/* Cache information data structures: */ + +enum CacheType { + DCACHE, + ICACHE, + UNIFIED_CACHE +}; + +typedef struct CPUCacheInfo { + enum CacheType type; + uint8_t level; + /* Size in bytes */ + uint32_t size; + /* Line size, in bytes */ + uint16_t line_size; + /* + * Associativity. + * Note: representation of fully-associative caches is not implemented + */ + uint8_t associativity; + /* Physical line partitions. CPUID[0x8000001D].EBX, CPUID[4].EBX */ + uint8_t partitions; + /* Number of sets. CPUID[0x8000001D].ECX, CPUID[4].ECX */ + uint32_t sets; + /* + * Lines per tag. + * AMD-specific: CPUID[0x80000005], CPUID[0x80000006]. + * (Is this synonym to @partitions?) + */ + uint8_t lines_per_tag; + + /* Self-initializing cache */ + bool self_init; + /* + * WBINVD/INVD is not guaranteed to act upon lower level caches of + * non-originating threads sharing this cache. + * CPUID[4].EDX[bit 0], CPUID[0x8000001D].EDX[bit 0] + */ + bool no_invd_sharing; + /* + * Cache is inclusive of lower cache levels. + * CPUID[4].EDX[bit 1], CPUID[0x8000001D].EDX[bit 1]. + */ + bool inclusive; + /* + * A complex function is used to index the cache, potentially using all + * address bits. CPUID[4].EDX[bit 2]. + */ + bool complex_indexing; +} CPUCacheInfo; + + + typedef struct CPUX86State { /* standard registers */ target_ulong regs[CPU_NB_REGS]; -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently Babu Moger @ 2018-05-11 19:12 ` Eduardo Habkost 2018-07-16 13:20 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 20+ messages in thread From: Eduardo Habkost @ 2018-05-11 19:12 UTC (permalink / raw) To: Babu Moger Cc: mst, marcel.apfelbaum, pbonzini, rth, mtosatti, qemu-devel, kvm, geoff, kash On Thu, May 10, 2018 at 03:41:41PM -0500, Babu Moger wrote: > From: Eduardo Habkost <ehabkost@redhat.com> > > Instead of having a collection of macros that need to be used in > complex expressions to build CPUID data, define a CPUCacheInfo > struct that can hold information about a given cache. Helper > functions will take a CPUCacheInfo struct as input to encode > CPUID leaves for a cache. > > This will help us ensure consistency between cache information > CPUID leaves, and make the existing inconsistencies in CPUID info > more visible. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Babu Moger <babu.moger@amd.com> > Tested-by: Geoffrey McRae <geoff@hostfission.com> CPUID compatibility is now being kept. Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Queued, thanks. -- Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently Babu Moger 2018-05-11 19:12 ` Eduardo Habkost @ 2018-07-16 13:20 ` Philippe Mathieu-Daudé 2018-07-16 19:52 ` Eduardo Habkost 1 sibling, 1 reply; 20+ messages in thread From: Philippe Mathieu-Daudé @ 2018-07-16 13:20 UTC (permalink / raw) To: Babu Moger, mst, marcel.apfelbaum, pbonzini, ehabkost, mtosatti Cc: rth, geoff, kash, qemu-devel, Aleksandar Markovic, aurelien Hi Eduardo, On 05/10/2018 05:41 PM, Babu Moger wrote: > From: Eduardo Habkost <ehabkost@redhat.com> > > Instead of having a collection of macros that need to be used in > complex expressions to build CPUID data, define a CPUCacheInfo > struct that can hold information about a given cache. Helper > functions will take a CPUCacheInfo struct as input to encode > CPUID leaves for a cache. > > This will help us ensure consistency between cache information > CPUID leaves, and make the existing inconsistencies in CPUID info > more visible. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > Signed-off-by: Babu Moger <babu.moger@amd.com> > Tested-by: Geoffrey McRae <geoff@hostfission.com> > --- > target/i386/cpu.c | 495 ++++++++++++++++++++++++++++++++++------------ > target/i386/cpu.h | 53 +++++ > 2 files changed, 424 insertions(+), 124 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index a20fe26573..18835400f1 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c [...] > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 1b219fafc4..fa03e2ced4 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -1044,6 +1044,59 @@ typedef enum TPRAccess { > TPR_ACCESS_WRITE, > } TPRAccess; > > +/* Cache information data structures: */ > + > +enum CacheType { > + DCACHE, I just noticed this clashes on mips host: mips-linux-gnu-gcc -iquote /tmp/qemu-test/build/. -iquote . -iquote /tmp/qemu-test/src/tcg -iquote /tmp/qemu-test/src/tcg/mips -I/tmp/qemu-test/src/linux-headers -I/tmp/qemu-test/build/linux-headers -iquote . -iquote /tmp/qemu-test/src -iquote /tmp/qemu-test/src/accel/tcg -iquote /tmp/qemu-test/src/include -I/usr/include/pixman-1 -Werror -DHAS_LIBSSH2_SFTP_FSYNC -pthread -I/usr/include/glib-2.0 -I/usr/lib/mips-linux-gnu/glib-2.0/include -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-strong -I/usr/include/p11-kit-1 -I/usr/include/libpng16 -I../linux-headers -iquote .. -iquote /tmp/qemu-test/src/target/i386 -DNEED_CPU_H -iquote /tmp/qemu-test/src/include -MMD -MP -MT exec.o -MF ./exec.d -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g -c -o exec.o /tmp/qemu-test/src/exec.c In file included from /usr/include/mips-linux-gnu/sys/cachectl.h:26:0, from /tmp/qemu-test/src/tcg/mips/tcg-target.h:202, from /tmp/qemu-test/src/include/exec/cpu-defs.h:29, from /tmp/qemu-test/src/target/i386/cpu.h:34, from /tmp/qemu-test/src/exec.c:23: /tmp/qemu-test/src/target/i386/cpu.h:1053:5: error: expected identifier before '(' token DCACHE, ^ /tmp/qemu-test/src/rules.mak:69: recipe for target 'exec.o' failed make[1]: *** [exec.o] Error 1 make[1]: Leaving directory '/tmp/qemu-test/build/x86_64-softmmu' Makefile:481: recipe for target 'subdir-x86_64-softmmu' failed make: *** [subdir-x86_64-softmmu] Error 2 $ fgrep -rn CACHE /usr/include/mips-linux-gnu/asm/cachectl.h 8:#ifndef _ASM_CACHECTL 9:#define _ASM_CACHECTL 14:#define ICACHE (1<<0) /* flush instruction cache */ 15:#define DCACHE (1<<1) /* writeback and flush data cache */ 16:#define BCACHE (ICACHE|DCACHE) /* flush both caches */ 23:#define CACHEABLE 0 /* make pages cacheable */ 24:#define UNCACHEABLE 1 /* make pages uncacheable */ 26:#endif /* _ASM_CACHECTL */ The offending commit is 7e3482f8248 but eventually went unnoticed until the "includes" refactor around 9edc19c9399? > + ICACHE, > + UNIFIED_CACHE > +}; > + > +typedef struct CPUCacheInfo { > + enum CacheType type; > + uint8_t level; > + /* Size in bytes */ > + uint32_t size; > + /* Line size, in bytes */ > + uint16_t line_size; [...] ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently 2018-07-16 13:20 ` Philippe Mathieu-Daudé @ 2018-07-16 19:52 ` Eduardo Habkost 2018-07-17 15:42 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 20+ messages in thread From: Eduardo Habkost @ 2018-07-16 19:52 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Babu Moger, mst, marcel.apfelbaum, pbonzini, mtosatti, rth, geoff, kash, qemu-devel, Aleksandar Markovic, aurelien On Mon, Jul 16, 2018 at 10:20:09AM -0300, Philippe Mathieu-Daudé wrote: [...] > > +enum CacheType { > > + DCACHE, > > I just noticed this clashes on mips host: [...] > In file included from /usr/include/mips-linux-gnu/sys/cachectl.h:26:0, > from /tmp/qemu-test/src/tcg/mips/tcg-target.h:202, > from /tmp/qemu-test/src/include/exec/cpu-defs.h:29, > from /tmp/qemu-test/src/target/i386/cpu.h:34, > from /tmp/qemu-test/src/exec.c:23: > /tmp/qemu-test/src/target/i386/cpu.h:1053:5: error: expected identifier > before '(' token > DCACHE, > ^ Oops. I will take a look and submit a fix. Thanks for the report! -- Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently 2018-07-16 19:52 ` Eduardo Habkost @ 2018-07-17 15:42 ` Philippe Mathieu-Daudé 0 siblings, 0 replies; 20+ messages in thread From: Philippe Mathieu-Daudé @ 2018-07-17 15:42 UTC (permalink / raw) To: Eduardo Habkost, mst, rth Cc: Babu Moger, marcel.apfelbaum, pbonzini, mtosatti, geoff, kash, qemu-devel, Aleksandar Markovic, aurelien Hi Eduardo, On 07/16/2018 04:52 PM, Eduardo Habkost wrote: > On Mon, Jul 16, 2018 at 10:20:09AM -0300, Philippe Mathieu-Daudé wrote: > [...] >>> +enum CacheType { >>> + DCACHE, >> >> I just noticed this clashes on mips host: > [...] >> In file included from /usr/include/mips-linux-gnu/sys/cachectl.h:26:0, >> from /tmp/qemu-test/src/tcg/mips/tcg-target.h:202, >> from /tmp/qemu-test/src/include/exec/cpu-defs.h:29, >> from /tmp/qemu-test/src/target/i386/cpu.h:34, >> from /tmp/qemu-test/src/exec.c:23: >> /tmp/qemu-test/src/target/i386/cpu.h:1053:5: error: expected identifier >> before '(' token >> DCACHE, >> ^ > > Oops. I will take a look and submit a fix. Thanks for the > report! While a quick fix is to simply rename the CacheType enums, I think this is an #include problem. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v8 2/8] i386: Add cache information in X86CPUDefinition 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently Babu Moger @ 2018-05-10 20:41 ` Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info Babu Moger ` (5 subsequent siblings) 7 siblings, 0 replies; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger Add cache information in X86CPUDefinition and CPUX86State. Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> --- target/i386/cpu.c | 1 + target/i386/cpu.h | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 18835400f1..3a74c4b1e4 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1105,6 +1105,7 @@ struct X86CPUDefinition { int stepping; FeatureWordArray features; const char *model_id; + CPUCaches *cache_info; }; static X86CPUDefinition builtin_x86_defs[] = { diff --git a/target/i386/cpu.h b/target/i386/cpu.h index fa03e2ced4..372f8b7ef5 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1096,6 +1096,12 @@ typedef struct CPUCacheInfo { } CPUCacheInfo; +typedef struct CPUCaches { + CPUCacheInfo l1d_cache; + CPUCacheInfo l1i_cache; + CPUCacheInfo l2_cache; + CPUCacheInfo l3_cache; +} CPUCaches; typedef struct CPUX86State { /* standard registers */ @@ -1282,6 +1288,7 @@ typedef struct CPUX86State { /* Features that were explicitly enabled/disabled */ FeatureWordArray user_features; uint32_t cpuid_model[12]; + CPUCaches *cache_info; /* MTRRs */ uint64_t mtrr_fixed[11]; -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 2/8] i386: Add cache information in X86CPUDefinition Babu Moger @ 2018-05-10 20:41 ` Babu Moger 2018-05-11 19:21 ` Eduardo Habkost 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 4/8] i386: Initialize cache information for EPYC family processors Babu Moger ` (4 subsequent siblings) 7 siblings, 1 reply; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger The property legacy-cache will be used to control the cache information. If user passes "-cpu legacy-cache" then older information will be displayed even if the hardware supports new information. Otherwise use the statically loaded cache definitions if available. Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> --- include/hw/i386/pc.h | 8 ++++ target/i386/cpu.c | 97 ++++++++++++++++++++++++++++++++------------ target/i386/cpu.h | 5 +++ 3 files changed, 84 insertions(+), 26 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 2e834e6ded..df15deefca 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -304,6 +304,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); +#define PC_COMPAT_2_12 \ + HW_COMPAT_2_12 \ + {\ + .driver = TYPE_X86_CPU,\ + .property = "legacy-cache",\ + .value = "on",\ + }, + #define PC_COMPAT_2_11 \ HW_COMPAT_2_11 \ {\ diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 3a74c4b1e4..d97b290b08 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -336,10 +336,14 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2, } } -/* Definitions of the hardcoded cache entries we expose: */ +/* + * Definitions of the hardcoded cache entries we expose: + * These are legacy cache values. If there is a need to change any + * of these values please use builtin_x86_defs + */ /* L1 data cache: */ -static CPUCacheInfo l1d_cache = { +static CPUCacheInfo legacy_l1d_cache = { .type = DCACHE, .level = 1, .size = 32 * KiB, @@ -352,7 +356,7 @@ static CPUCacheInfo l1d_cache = { }; /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */ -static CPUCacheInfo l1d_cache_amd = { +static CPUCacheInfo legacy_l1d_cache_amd = { .type = DCACHE, .level = 1, .size = 64 * KiB, @@ -366,7 +370,7 @@ static CPUCacheInfo l1d_cache_amd = { }; /* L1 instruction cache: */ -static CPUCacheInfo l1i_cache = { +static CPUCacheInfo legacy_l1i_cache = { .type = ICACHE, .level = 1, .size = 32 * KiB, @@ -379,7 +383,7 @@ static CPUCacheInfo l1i_cache = { }; /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */ -static CPUCacheInfo l1i_cache_amd = { +static CPUCacheInfo legacy_l1i_cache_amd = { .type = ICACHE, .level = 1, .size = 64 * KiB, @@ -393,7 +397,7 @@ static CPUCacheInfo l1i_cache_amd = { }; /* Level 2 unified cache: */ -static CPUCacheInfo l2_cache = { +static CPUCacheInfo legacy_l2_cache = { .type = UNIFIED_CACHE, .level = 2, .size = 4 * MiB, @@ -406,7 +410,7 @@ static CPUCacheInfo l2_cache = { }; /*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */ -static CPUCacheInfo l2_cache_cpuid2 = { +static CPUCacheInfo legacy_l2_cache_cpuid2 = { .type = UNIFIED_CACHE, .level = 2, .size = 2 * MiB, @@ -416,7 +420,7 @@ static CPUCacheInfo l2_cache_cpuid2 = { /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */ -static CPUCacheInfo l2_cache_amd = { +static CPUCacheInfo legacy_l2_cache_amd = { .type = UNIFIED_CACHE, .level = 2, .size = 512 * KiB, @@ -428,7 +432,7 @@ static CPUCacheInfo l2_cache_amd = { }; /* Level 3 unified cache: */ -static CPUCacheInfo l3_cache = { +static CPUCacheInfo legacy_l3_cache = { .type = UNIFIED_CACHE, .level = 3, .size = 16 * MiB, @@ -3243,6 +3247,10 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp) env->features[w] = def->features[w]; } + /* Store Cache information from the X86CPUDefinition if available */ + env->cache_info = def->cache_info; + cpu->legacy_cache = def->cache_info ? 0 : 1; + /* Special cases not set in the X86CPUDefinition structs: */ /* TODO: in-kernel irqchip for hvf */ if (kvm_enabled()) { @@ -3592,11 +3600,21 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (!cpu->enable_l3_cache) { *ecx = 0; } else { - *ecx = cpuid2_cache_descriptor(&l3_cache); + if (env->cache_info && !cpu->legacy_cache) { + *ecx = cpuid2_cache_descriptor(&env->cache_info->l3_cache); + } else { + *ecx = cpuid2_cache_descriptor(&legacy_l3_cache); + } + } + if (env->cache_info && !cpu->legacy_cache) { + *edx = (cpuid2_cache_descriptor(&env->cache_info->l1d_cache) << 16) | + (cpuid2_cache_descriptor(&env->cache_info->l1i_cache) << 8) | + (cpuid2_cache_descriptor(&env->cache_info->l2_cache)); + } else { + *edx = (cpuid2_cache_descriptor(&legacy_l1d_cache) << 16) | + (cpuid2_cache_descriptor(&legacy_l1i_cache) << 8) | + (cpuid2_cache_descriptor(&legacy_l2_cache_cpuid2)); } - *edx = (cpuid2_cache_descriptor(&l1d_cache) << 16) | - (cpuid2_cache_descriptor(&l1i_cache) << 8) | - (cpuid2_cache_descriptor(&l2_cache_cpuid2)); break; case 4: /* cache info: needed for Core compatibility */ @@ -3609,27 +3627,35 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, } } else { *eax = 0; + CPUCacheInfo *l1d, *l1i, *l2, *l3; + if (env->cache_info && !cpu->legacy_cache) { + l1d = &env->cache_info->l1d_cache; + l1i = &env->cache_info->l1i_cache; + l2 = &env->cache_info->l2_cache; + l3 = &env->cache_info->l3_cache; + } else { + l1d = &legacy_l1d_cache; + l1i = &legacy_l1i_cache; + l2 = &legacy_l2_cache; + l3 = &legacy_l3_cache; + } switch (count) { case 0: /* L1 dcache info */ - encode_cache_cpuid4(&l1d_cache, - 1, cs->nr_cores, + encode_cache_cpuid4(l1d, 1, cs->nr_cores, eax, ebx, ecx, edx); break; case 1: /* L1 icache info */ - encode_cache_cpuid4(&l1i_cache, - 1, cs->nr_cores, + encode_cache_cpuid4(l1i, 1, cs->nr_cores, eax, ebx, ecx, edx); break; case 2: /* L2 cache info */ - encode_cache_cpuid4(&l2_cache, - cs->nr_threads, cs->nr_cores, + encode_cache_cpuid4(l2, cs->nr_threads, cs->nr_cores, eax, ebx, ecx, edx); break; case 3: /* L3 cache info */ pkg_offset = apicid_pkg_offset(cs->nr_cores, cs->nr_threads); if (cpu->enable_l3_cache) { - encode_cache_cpuid4(&l3_cache, - (1 << pkg_offset), cs->nr_cores, + encode_cache_cpuid4(l3, (1 << pkg_offset), cs->nr_cores, eax, ebx, ecx, edx); break; } @@ -3842,8 +3868,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES); *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | \ (L1_ITLB_4K_ASSOC << 8) | (L1_ITLB_4K_ENTRIES); - *ecx = encode_cache_cpuid80000005(&l1d_cache_amd); - *edx = encode_cache_cpuid80000005(&l1i_cache_amd); + if (env->cache_info && !cpu->legacy_cache) { + *ecx = encode_cache_cpuid80000005(&env->cache_info->l1d_cache); + *edx = encode_cache_cpuid80000005(&env->cache_info->l1i_cache); + } else { + *ecx = encode_cache_cpuid80000005(&legacy_l1d_cache_amd); + *edx = encode_cache_cpuid80000005(&legacy_l1i_cache_amd); + } break; case 0x80000006: /* cache info (L2 cache) */ @@ -3859,9 +3890,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, (L2_DTLB_4K_ENTRIES << 16) | \ (AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) | \ (L2_ITLB_4K_ENTRIES); - encode_cache_cpuid80000006(&l2_cache_amd, - cpu->enable_l3_cache ? &l3_cache : NULL, - ecx, edx); + if (env->cache_info && !cpu->legacy_cache) { + encode_cache_cpuid80000006(&env->cache_info->l2_cache, + cpu->enable_l3_cache ? + &env->cache_info->l3_cache : NULL, + ecx, edx); + } else { + encode_cache_cpuid80000006(&legacy_l2_cache_amd, + cpu->enable_l3_cache ? + &legacy_l3_cache : NULL, + ecx, edx); + } break; case 0x80000007: *eax = 0; @@ -5039,6 +5078,12 @@ static Property x86_cpu_properties[] = { false), DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true), DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true), + /* + * lecacy_cache defaults to CPU model being chosen. This is set in + * x86_cpu_load_def based on cache_info which is initialized in + * builtin_x86_defs + */ + DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false), /* * From "Requirements for Implementing the Microsoft diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 372f8b7ef5..31715d167d 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1394,6 +1394,11 @@ struct X86CPU { */ bool enable_l3_cache; + /* Compatibility bits for old machine types. + * If true present the old cache topology information + */ + bool legacy_cache; + /* Compatibility bits for old machine types: */ bool enable_cpuid_0xb; -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info Babu Moger @ 2018-05-11 19:21 ` Eduardo Habkost 2018-05-11 20:21 ` Moger, Babu 0 siblings, 1 reply; 20+ messages in thread From: Eduardo Habkost @ 2018-05-11 19:21 UTC (permalink / raw) To: Babu Moger Cc: mst, marcel.apfelbaum, pbonzini, rth, mtosatti, qemu-devel, kvm, geoff, kash On Thu, May 10, 2018 at 03:41:43PM -0500, Babu Moger wrote: > The property legacy-cache will be used to control the cache information. > If user passes "-cpu legacy-cache" then older information will > be displayed even if the hardware supports new information. Otherwise > use the statically loaded cache definitions if available. > > Signed-off-by: Babu Moger <babu.moger@amd.com> > Tested-by: Geoffrey McRae <geoff@hostfission.com> > --- > include/hw/i386/pc.h | 8 ++++ > target/i386/cpu.c | 97 ++++++++++++++++++++++++++++++++------------ > target/i386/cpu.h | 5 +++ > 3 files changed, 84 insertions(+), 26 deletions(-) > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 2e834e6ded..df15deefca 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -304,6 +304,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > int e820_get_num_entries(void); > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > +#define PC_COMPAT_2_12 \ > + HW_COMPAT_2_12 \ > + {\ > + .driver = TYPE_X86_CPU,\ > + .property = "legacy-cache",\ > + .value = "on",\ > + }, This isn't enough if the pc-*-2.12 machine-type isn't using the macro. Before we do this, we need a commit similar to commit df47ce8af4a5, but adding pc-*-2.13 machine-types. The rest of the patch looks good to me, but I will suggest a clean up (that can be submitted a separate patch later, or included in v9) in a separate reply. -- Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info 2018-05-11 19:21 ` Eduardo Habkost @ 2018-05-11 20:21 ` Moger, Babu 2018-05-11 20:40 ` Eduardo Habkost 2018-05-11 20:47 ` Eduardo Habkost 0 siblings, 2 replies; 20+ messages in thread From: Moger, Babu @ 2018-05-11 20:21 UTC (permalink / raw) To: Eduardo Habkost Cc: mst@redhat.com, marcel.apfelbaum@gmail.com, pbonzini@redhat.com, rth@twiddle.net, mtosatti@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, geoff@hostfission.com, kash@tripleback.net > -----Original Message----- > From: Eduardo Habkost [mailto:ehabkost@redhat.com] > Sent: Friday, May 11, 2018 2:22 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > kvm@vger.kernel.org; geoff@hostfission.com; kash@tripleback.net > Subject: Re: [PATCH v8 3/8] i386: Add new property to control cache info > > On Thu, May 10, 2018 at 03:41:43PM -0500, Babu Moger wrote: > > The property legacy-cache will be used to control the cache information. > > If user passes "-cpu legacy-cache" then older information will > > be displayed even if the hardware supports new information. Otherwise > > use the statically loaded cache definitions if available. > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > Tested-by: Geoffrey McRae <geoff@hostfission.com> > > --- > > include/hw/i386/pc.h | 8 ++++ > > target/i386/cpu.c | 97 ++++++++++++++++++++++++++++++++----------- > - > > target/i386/cpu.h | 5 +++ > > 3 files changed, 84 insertions(+), 26 deletions(-) > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index 2e834e6ded..df15deefca 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -304,6 +304,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > > int e820_get_num_entries(void); > > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > > > +#define PC_COMPAT_2_12 \ > > + HW_COMPAT_2_12 \ > > + {\ > > + .driver = TYPE_X86_CPU,\ > > + .property = "legacy-cache",\ > > + .value = "on",\ > > + }, > > This isn't enough if the pc-*-2.12 machine-type isn't using the > macro. > > Before we do this, we need a commit similar to commit > df47ce8af4a5, but adding pc-*-2.13 machine-types. Ok. Sure. I think I got it. Will add pc-*-2.13 machine-types in v9 series. > > The rest of the patch looks good to me, but I will suggest a > clean up (that can be submitted a separate patch later, or > included in v9) in a separate reply. Either way is works for me. If it is simple enough we can add here. > > -- > Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info 2018-05-11 20:21 ` Moger, Babu @ 2018-05-11 20:40 ` Eduardo Habkost 2018-05-11 20:47 ` Eduardo Habkost 1 sibling, 0 replies; 20+ messages in thread From: Eduardo Habkost @ 2018-05-11 20:40 UTC (permalink / raw) To: Moger, Babu Cc: mst@redhat.com, marcel.apfelbaum@gmail.com, pbonzini@redhat.com, rth@twiddle.net, mtosatti@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, geoff@hostfission.com, kash@tripleback.net On Fri, May 11, 2018 at 08:21:50PM +0000, Moger, Babu wrote: > > > -----Original Message----- > > From: Eduardo Habkost [mailto:ehabkost@redhat.com] > > Sent: Friday, May 11, 2018 2:22 PM > > To: Moger, Babu <Babu.Moger@amd.com> > > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > > kvm@vger.kernel.org; geoff@hostfission.com; kash@tripleback.net > > Subject: Re: [PATCH v8 3/8] i386: Add new property to control cache info > > > > On Thu, May 10, 2018 at 03:41:43PM -0500, Babu Moger wrote: > > > The property legacy-cache will be used to control the cache information. > > > If user passes "-cpu legacy-cache" then older information will > > > be displayed even if the hardware supports new information. Otherwise > > > use the statically loaded cache definitions if available. > > > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > > Tested-by: Geoffrey McRae <geoff@hostfission.com> > > > --- > > > include/hw/i386/pc.h | 8 ++++ > > > target/i386/cpu.c | 97 ++++++++++++++++++++++++++++++++----------- > > - > > > target/i386/cpu.h | 5 +++ > > > 3 files changed, 84 insertions(+), 26 deletions(-) > > > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > > index 2e834e6ded..df15deefca 100644 > > > --- a/include/hw/i386/pc.h > > > +++ b/include/hw/i386/pc.h > > > @@ -304,6 +304,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > > > int e820_get_num_entries(void); > > > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > > > > > +#define PC_COMPAT_2_12 \ > > > + HW_COMPAT_2_12 \ > > > + {\ > > > + .driver = TYPE_X86_CPU,\ > > > + .property = "legacy-cache",\ > > > + .value = "on",\ > > > + }, > > > > This isn't enough if the pc-*-2.12 machine-type isn't using the > > macro. > > > > Before we do this, we need a commit similar to commit > > df47ce8af4a5, but adding pc-*-2.13 machine-types. > > Ok. Sure. I think I got it. Will add pc-*-2.13 machine-types in v9 series. > > > > > The rest of the patch looks good to me, but I will suggest a > > clean up (that can be submitted a separate patch later, or > > included in v9) in a separate reply. > > Either way is works for me. If it is simple enough we can add here. This is the clean up I was working on. Feel free to include it, or leave this to be applied as a follow-up later. >From b98b82dde371e850c65623f48bdb24df31a99b5d Mon Sep 17 00:00:00 2001 From: Eduardo Habkost <ehabkost@redhat.com> Date: Fri, 11 May 2018 16:59:34 -0300 Subject: [PATCH] Clean up cache CPUID code Always initialize CPUCaches structs with cache information, even if legacy_cache=true. Use different CPUCaches struct for CPUID[2], CPUID[4], and the AMD CPUID leaves. This will simplify a lot the logic inside cpu_x86_cpuid(). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target/i386/cpu.h | 14 ++++--- target/i386/cpu.c | 117 +++++++++++++++++++++++++++--------------------------- 2 files changed, 67 insertions(+), 64 deletions(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 31715d167d..88fdf80d56 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1097,10 +1097,10 @@ typedef struct CPUCacheInfo { typedef struct CPUCaches { - CPUCacheInfo l1d_cache; - CPUCacheInfo l1i_cache; - CPUCacheInfo l2_cache; - CPUCacheInfo l3_cache; + CPUCacheInfo *l1d_cache; + CPUCacheInfo *l1i_cache; + CPUCacheInfo *l2_cache; + CPUCacheInfo *l3_cache; } CPUCaches; typedef struct CPUX86State { @@ -1288,7 +1288,11 @@ typedef struct CPUX86State { /* Features that were explicitly enabled/disabled */ FeatureWordArray user_features; uint32_t cpuid_model[12]; - CPUCaches *cache_info; + /* Cache information for CPUID. When legacy-cache=on, the cache data + * on each CPUID leaf will be different, because we keep compatibility + * with old QEMU versions. + */ + CPUCaches cache_info_cpuid2, cache_info_cpuid4, cache_info_amd; /* MTRRs */ uint64_t mtrr_fixed[11]; diff --git a/target/i386/cpu.c b/target/i386/cpu.c index b20b8691a7..18ea5e3547 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1113,7 +1113,7 @@ struct X86CPUDefinition { }; static CPUCaches epyc_cache_info = { - .l1d_cache = { + .l1d_cache = &(CPUCacheInfo) { .type = DCACHE, .level = 1, .size = 32 * KiB, @@ -1125,7 +1125,7 @@ static CPUCaches epyc_cache_info = { .self_init = 1, .no_invd_sharing = true, }, - .l1i_cache = { + .l1i_cache = &(CPUCacheInfo) { .type = ICACHE, .level = 1, .size = 64 * KiB, @@ -1137,7 +1137,7 @@ static CPUCaches epyc_cache_info = { .self_init = 1, .no_invd_sharing = true, }, - .l2_cache = { + .l2_cache = &(CPUCacheInfo) { .type = UNIFIED_CACHE, .level = 2, .size = 512 * KiB, @@ -1147,7 +1147,7 @@ static CPUCaches epyc_cache_info = { .sets = 1024, .lines_per_tag = 1, }, - .l3_cache = { + .l3_cache = &(CPUCacheInfo) { .type = UNIFIED_CACHE, .level = 3, .size = 8 * MiB, @@ -3299,9 +3299,8 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp) env->features[w] = def->features[w]; } - /* Store Cache information from the X86CPUDefinition if available */ - env->cache_info = def->cache_info; - cpu->legacy_cache = def->cache_info ? 0 : 1; + /* legacy-cache defaults to 'off' if CPU model provides cache info */ + cpu->legacy_cache = !def->cache_info; /* Special cases not set in the X86CPUDefinition structs: */ /* TODO: in-kernel irqchip for hvf */ @@ -3652,21 +3651,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, if (!cpu->enable_l3_cache) { *ecx = 0; } else { - if (env->cache_info && !cpu->legacy_cache) { - *ecx = cpuid2_cache_descriptor(&env->cache_info->l3_cache); - } else { - *ecx = cpuid2_cache_descriptor(&legacy_l3_cache); - } - } - if (env->cache_info && !cpu->legacy_cache) { - *edx = (cpuid2_cache_descriptor(&env->cache_info->l1d_cache) << 16) | - (cpuid2_cache_descriptor(&env->cache_info->l1i_cache) << 8) | - (cpuid2_cache_descriptor(&env->cache_info->l2_cache)); - } else { - *edx = (cpuid2_cache_descriptor(&legacy_l1d_cache) << 16) | - (cpuid2_cache_descriptor(&legacy_l1i_cache) << 8) | - (cpuid2_cache_descriptor(&legacy_l2_cache_cpuid2)); + *ecx = cpuid2_cache_descriptor(env->cache_info_cpuid2.l3_cache); } + *edx = (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1d_cache) << 16) | + (cpuid2_cache_descriptor(env->cache_info_cpuid2.l1i_cache) << 8) | + (cpuid2_cache_descriptor(env->cache_info_cpuid2.l2_cache)); break; case 4: /* cache info: needed for Core compatibility */ @@ -3679,35 +3668,27 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, } } else { *eax = 0; - CPUCacheInfo *l1d, *l1i, *l2, *l3; - if (env->cache_info && !cpu->legacy_cache) { - l1d = &env->cache_info->l1d_cache; - l1i = &env->cache_info->l1i_cache; - l2 = &env->cache_info->l2_cache; - l3 = &env->cache_info->l3_cache; - } else { - l1d = &legacy_l1d_cache; - l1i = &legacy_l1i_cache; - l2 = &legacy_l2_cache; - l3 = &legacy_l3_cache; - } switch (count) { case 0: /* L1 dcache info */ - encode_cache_cpuid4(l1d, 1, cs->nr_cores, + encode_cache_cpuid4(env->cache_info_cpuid4.l1d_cache, + 1, cs->nr_cores, eax, ebx, ecx, edx); break; case 1: /* L1 icache info */ - encode_cache_cpuid4(l1i, 1, cs->nr_cores, + encode_cache_cpuid4(env->cache_info_cpuid4.l1i_cache, + 1, cs->nr_cores, eax, ebx, ecx, edx); break; case 2: /* L2 cache info */ - encode_cache_cpuid4(l2, cs->nr_threads, cs->nr_cores, + encode_cache_cpuid4(env->cache_info_cpuid4.l2_cache, + cs->nr_threads, cs->nr_cores, eax, ebx, ecx, edx); break; case 3: /* L3 cache info */ pkg_offset = apicid_pkg_offset(cs->nr_cores, cs->nr_threads); if (cpu->enable_l3_cache) { - encode_cache_cpuid4(l3, (1 << pkg_offset), cs->nr_cores, + encode_cache_cpuid4(env->cache_info_cpuid4.l3_cache, + (1 << pkg_offset), cs->nr_cores, eax, ebx, ecx, edx); break; } @@ -3920,13 +3901,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, (L1_ITLB_2M_ASSOC << 8) | (L1_ITLB_2M_ENTRIES); *ebx = (L1_DTLB_4K_ASSOC << 24) | (L1_DTLB_4K_ENTRIES << 16) | \ (L1_ITLB_4K_ASSOC << 8) | (L1_ITLB_4K_ENTRIES); - if (env->cache_info && !cpu->legacy_cache) { - *ecx = encode_cache_cpuid80000005(&env->cache_info->l1d_cache); - *edx = encode_cache_cpuid80000005(&env->cache_info->l1i_cache); - } else { - *ecx = encode_cache_cpuid80000005(&legacy_l1d_cache_amd); - *edx = encode_cache_cpuid80000005(&legacy_l1i_cache_amd); - } + *ecx = encode_cache_cpuid80000005(env->cache_info_amd.l1d_cache); + *edx = encode_cache_cpuid80000005(env->cache_info_amd.l1i_cache); break; case 0x80000006: /* cache info (L2 cache) */ @@ -3942,17 +3918,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, (L2_DTLB_4K_ENTRIES << 16) | \ (AMD_ENC_ASSOC(L2_ITLB_4K_ASSOC) << 12) | \ (L2_ITLB_4K_ENTRIES); - if (env->cache_info && !cpu->legacy_cache) { - encode_cache_cpuid80000006(&env->cache_info->l2_cache, - cpu->enable_l3_cache ? - &env->cache_info->l3_cache : NULL, - ecx, edx); - } else { - encode_cache_cpuid80000006(&legacy_l2_cache_amd, - cpu->enable_l3_cache ? - &legacy_l3_cache : NULL, - ecx, edx); - } + encode_cache_cpuid80000006(env->cache_info_amd.l2_cache, + cpu->enable_l3_cache ? + env->cache_info_amd.l3_cache : NULL, + ecx, edx); break; case 0x80000007: *eax = 0; @@ -4649,6 +4618,37 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) cpu->phys_bits = 32; } } + + /* Cache information initialization */ + if (!cpu->legacy_cache) { + if (!xcc->cpu_def || !xcc->cpu_def->cache_info) { + char *name = x86_cpu_class_get_model_name(xcc); + error_setg(errp, + "CPU model '%s' doesn't support legacy-cache=off", name); + g_free(name); + return; + } + env->cache_info_cpuid2 = env->cache_info_cpuid4 = env->cache_info_amd = + *xcc->cpu_def->cache_info; + } else { + /* Build legacy cache information */ + env->cache_info_cpuid2.l1d_cache = &legacy_l1d_cache; + env->cache_info_cpuid2.l1i_cache = &legacy_l1i_cache; + env->cache_info_cpuid2.l2_cache = &legacy_l2_cache_cpuid2; + env->cache_info_cpuid2.l3_cache = &legacy_l3_cache; + + env->cache_info_cpuid4.l1d_cache = &legacy_l1d_cache; + env->cache_info_cpuid4.l1i_cache = &legacy_l1i_cache; + env->cache_info_cpuid4.l2_cache = &legacy_l2_cache; + env->cache_info_cpuid4.l3_cache = &legacy_l3_cache; + + env->cache_info_amd.l1d_cache = &legacy_l1d_cache_amd; + env->cache_info_amd.l1i_cache = &legacy_l1i_cache_amd; + env->cache_info_amd.l2_cache = &legacy_l2_cache_amd; + env->cache_info_amd.l3_cache = &legacy_l3_cache; + } + + cpu_exec_realizefn(cs, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); @@ -5131,11 +5131,10 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, vmware_cpuid_freq, true), DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true), /* - * lecacy_cache defaults to CPU model being chosen. This is set in - * x86_cpu_load_def based on cache_info which is initialized in - * builtin_x86_defs + * lecacy_cache defaults to true unless the CPU model provides its + * own cache information (see x86_cpu_load_def()). */ - DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, false), + DEFINE_PROP_BOOL("legacy-cache", X86CPU, legacy_cache, true), /* * From "Requirements for Implementing the Microsoft -- 2.14.3 -- Eduardo ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info 2018-05-11 20:21 ` Moger, Babu 2018-05-11 20:40 ` Eduardo Habkost @ 2018-05-11 20:47 ` Eduardo Habkost 2018-05-14 16:44 ` Moger, Babu 1 sibling, 1 reply; 20+ messages in thread From: Eduardo Habkost @ 2018-05-11 20:47 UTC (permalink / raw) To: Moger, Babu Cc: geoff@hostfission.com, kvm@vger.kernel.org, mst@redhat.com, kash@tripleback.net, mtosatti@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, rth@twiddle.net On Fri, May 11, 2018 at 08:21:50PM +0000, Moger, Babu wrote: > > > -----Original Message----- > > From: Eduardo Habkost [mailto:ehabkost@redhat.com] > > Sent: Friday, May 11, 2018 2:22 PM > > To: Moger, Babu <Babu.Moger@amd.com> > > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > > kvm@vger.kernel.org; geoff@hostfission.com; kash@tripleback.net > > Subject: Re: [PATCH v8 3/8] i386: Add new property to control cache info > > > > On Thu, May 10, 2018 at 03:41:43PM -0500, Babu Moger wrote: > > > The property legacy-cache will be used to control the cache information. > > > If user passes "-cpu legacy-cache" then older information will > > > be displayed even if the hardware supports new information. Otherwise > > > use the statically loaded cache definitions if available. > > > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > > Tested-by: Geoffrey McRae <geoff@hostfission.com> > > > --- > > > include/hw/i386/pc.h | 8 ++++ > > > target/i386/cpu.c | 97 ++++++++++++++++++++++++++++++++----------- > > - > > > target/i386/cpu.h | 5 +++ > > > 3 files changed, 84 insertions(+), 26 deletions(-) > > > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > > index 2e834e6ded..df15deefca 100644 > > > --- a/include/hw/i386/pc.h > > > +++ b/include/hw/i386/pc.h > > > @@ -304,6 +304,14 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); > > > int e820_get_num_entries(void); > > > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > > > > > +#define PC_COMPAT_2_12 \ > > > + HW_COMPAT_2_12 \ > > > + {\ > > > + .driver = TYPE_X86_CPU,\ > > > + .property = "legacy-cache",\ > > > + .value = "on",\ > > > + }, > > > > This isn't enough if the pc-*-2.12 machine-type isn't using the > > macro. > > > > Before we do this, we need a commit similar to commit > > df47ce8af4a5, but adding pc-*-2.13 machine-types. > > Ok. Sure. I think I got it. Will add pc-*-2.13 machine-types in v9 series. Thanks. If you submit v9, please use the x86-next tree as base so you don't need to resubmit the patches that I have already queued. See MAINTAINERS for the git URL. -- Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info 2018-05-11 20:47 ` Eduardo Habkost @ 2018-05-14 16:44 ` Moger, Babu 0 siblings, 0 replies; 20+ messages in thread From: Moger, Babu @ 2018-05-14 16:44 UTC (permalink / raw) To: Eduardo Habkost Cc: geoff@hostfission.com, kvm@vger.kernel.org, mst@redhat.com, kash@tripleback.net, mtosatti@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, rth@twiddle.net > -----Original Message----- > From: Eduardo Habkost [mailto:ehabkost@redhat.com] > Sent: Friday, May 11, 2018 3:48 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: geoff@hostfission.com; kvm@vger.kernel.org; mst@redhat.com; > kash@tripleback.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > pbonzini@redhat.com; rth@twiddle.net > Subject: Re: [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control > cache info > > On Fri, May 11, 2018 at 08:21:50PM +0000, Moger, Babu wrote: > > > > > -----Original Message----- > > > From: Eduardo Habkost [mailto:ehabkost@redhat.com] > > > Sent: Friday, May 11, 2018 2:22 PM > > > To: Moger, Babu <Babu.Moger@amd.com> > > > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; > pbonzini@redhat.com; > > > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org; > > > kvm@vger.kernel.org; geoff@hostfission.com; kash@tripleback.net > > > Subject: Re: [PATCH v8 3/8] i386: Add new property to control cache info > > > > > > On Thu, May 10, 2018 at 03:41:43PM -0500, Babu Moger wrote: > > > > The property legacy-cache will be used to control the cache > information. > > > > If user passes "-cpu legacy-cache" then older information will > > > > be displayed even if the hardware supports new information. > Otherwise > > > > use the statically loaded cache definitions if available. > > > > > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > > > Tested-by: Geoffrey McRae <geoff@hostfission.com> > > > > --- > > > > include/hw/i386/pc.h | 8 ++++ > > > > target/i386/cpu.c | 97 ++++++++++++++++++++++++++++++++------ > ----- > > > - > > > > target/i386/cpu.h | 5 +++ > > > > 3 files changed, 84 insertions(+), 26 deletions(-) > > > > > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > > > index 2e834e6ded..df15deefca 100644 > > > > --- a/include/hw/i386/pc.h > > > > +++ b/include/hw/i386/pc.h > > > > @@ -304,6 +304,14 @@ int e820_add_entry(uint64_t, uint64_t, > uint32_t); > > > > int e820_get_num_entries(void); > > > > bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); > > > > > > > > +#define PC_COMPAT_2_12 \ > > > > + HW_COMPAT_2_12 \ > > > > + {\ > > > > + .driver = TYPE_X86_CPU,\ > > > > + .property = "legacy-cache",\ > > > > + .value = "on",\ > > > > + }, > > > > > > This isn't enough if the pc-*-2.12 machine-type isn't using the > > > macro. > > > > > > Before we do this, we need a commit similar to commit > > > df47ce8af4a5, but adding pc-*-2.13 machine-types. > > > > Ok. Sure. I think I got it. Will add pc-*-2.13 machine-types in v9 series. > > Thanks. If you submit v9, please use the x86-next tree as base > so you don't need to resubmit the patches that I have already > queued. See MAINTAINERS for the git URL. Submitted v9 version on top of you x86-next tree. Thanks > > -- > Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v8 4/8] i386: Initialize cache information for EPYC family processors 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger ` (2 preceding siblings ...) 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info Babu Moger @ 2018-05-10 20:41 ` Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 5/8] i386: Populate AMD Processor Cache Information for cpuid 0x8000001D Babu Moger ` (3 subsequent siblings) 7 siblings, 0 replies; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger Initialize pre-determined cache information for EPYC processors. Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> --- target/i386/cpu.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index d97b290b08..b20b8691a7 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1112,6 +1112,56 @@ struct X86CPUDefinition { CPUCaches *cache_info; }; +static CPUCaches epyc_cache_info = { + .l1d_cache = { + .type = DCACHE, + .level = 1, + .size = 32 * KiB, + .line_size = 64, + .associativity = 8, + .partitions = 1, + .sets = 64, + .lines_per_tag = 1, + .self_init = 1, + .no_invd_sharing = true, + }, + .l1i_cache = { + .type = ICACHE, + .level = 1, + .size = 64 * KiB, + .line_size = 64, + .associativity = 4, + .partitions = 1, + .sets = 256, + .lines_per_tag = 1, + .self_init = 1, + .no_invd_sharing = true, + }, + .l2_cache = { + .type = UNIFIED_CACHE, + .level = 2, + .size = 512 * KiB, + .line_size = 64, + .associativity = 8, + .partitions = 1, + .sets = 1024, + .lines_per_tag = 1, + }, + .l3_cache = { + .type = UNIFIED_CACHE, + .level = 3, + .size = 8 * MiB, + .line_size = 64, + .associativity = 16, + .partitions = 1, + .sets = 8192, + .lines_per_tag = 1, + .self_init = true, + .inclusive = true, + .complex_indexing = true, + }, +}; + static X86CPUDefinition builtin_x86_defs[] = { { .name = "qemu64", @@ -2306,6 +2356,7 @@ static X86CPUDefinition builtin_x86_defs[] = { CPUID_6_EAX_ARAT, .xlevel = 0x8000000A, .model_id = "AMD EPYC Processor", + .cache_info = &epyc_cache_info, }, { .name = "EPYC-IBPB", @@ -2352,6 +2403,7 @@ static X86CPUDefinition builtin_x86_defs[] = { CPUID_6_EAX_ARAT, .xlevel = 0x8000000A, .model_id = "AMD EPYC Processor (with IBPB)", + .cache_info = &epyc_cache_info, }, }; -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v8 5/8] i386: Populate AMD Processor Cache Information for cpuid 0x8000001D 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger ` (3 preceding siblings ...) 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 4/8] i386: Initialize cache information for EPYC family processors Babu Moger @ 2018-05-10 20:41 ` Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 6/8] i386: Add support for CPUID_8000_001E for AMD Babu Moger ` (2 subsequent siblings) 7 siblings, 0 replies; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger Add information for cpuid 0x8000001D leaf. Populate cache topology information for different cache types(Data Cache, Instruction Cache, L2 and L3) supported by 0x8000001D leaf. Please refer Processor Programming Reference (PPR) for AMD Family 17h Model for more details. Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> --- target/i386/cpu.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++ target/i386/kvm.c | 29 +++++++++++++++-- 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index b20b8691a7..6898042787 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -307,6 +307,14 @@ static uint32_t encode_cache_cpuid80000005(CPUCacheInfo *cache) a == ASSOC_FULL ? 0xF : \ 0 /* invalid value */) +/* Definitions used on CPUID Leaf 0x8000001D */ +/* Number of logical cores in a complex */ +#define CORES_IN_CMPLX 4 +/* Number of logical processors sharing cache */ +#define NUM_SHARING_CACHE(threads) ((threads > 1) ? \ + (((CORES_IN_CMPLX - 1) * threads) + 1) : \ + (CORES_IN_CMPLX - 1)) + /* * Encode cache info for CPUID[0x80000006].ECX and CPUID[0x80000006].EDX * @l3 can be NULL. @@ -336,6 +344,41 @@ static void encode_cache_cpuid80000006(CPUCacheInfo *l2, } } +/* Encode cache info for CPUID[8000001D] */ +static void encode_cache_cpuid8000001d(CPUCacheInfo *cache, int nr_threads, + uint32_t *eax, uint32_t *ebx, + uint32_t *ecx, uint32_t *edx) +{ + assert(cache->size == cache->line_size * cache->associativity * + cache->partitions * cache->sets); + + *eax = CACHE_TYPE(cache->type) | CACHE_LEVEL(cache->level) | + (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0); + + /* L3 is shared among multiple cores */ + if (cache->level == 3) { + *eax |= (NUM_SHARING_CACHE(nr_threads) << 14); + } else { + *eax |= ((nr_threads - 1) << 14); + } + + assert(cache->line_size > 0); + assert(cache->partitions > 0); + assert(cache->associativity > 0); + /* We don't implement fully-associative caches */ + assert(cache->associativity < cache->sets); + *ebx = (cache->line_size - 1) | + ((cache->partitions - 1) << 12) | + ((cache->associativity - 1) << 22); + + assert(cache->sets > 0); + *ecx = cache->sets - 1; + + *edx = (cache->no_invd_sharing ? CACHE_NO_INVD_SHARING : 0) | + (cache->inclusive ? CACHE_INCLUSIVE : 0) | + (cache->complex_indexing ? CACHE_COMPLEX_IDX : 0); +} + /* * Definitions of the hardcoded cache entries we expose: * These are legacy cache values. If there is a need to change any @@ -3993,6 +4036,42 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *edx = 0; } break; + case 0x8000001D: + *eax = 0; + CPUCacheInfo *l1d, *l1i, *l2, *l3; + if (env->cache_info && !cpu->legacy_cache) { + l1d = &env->cache_info->l1d_cache; + l1i = &env->cache_info->l1i_cache; + l2 = &env->cache_info->l2_cache; + l3 = &env->cache_info->l3_cache; + } else { + l1d = &legacy_l1d_cache_amd; + l1i = &legacy_l1i_cache_amd; + l2 = &legacy_l2_cache_amd; + l3 = &legacy_l3_cache; + } + switch (count) { + case 0: /* L1 dcache info */ + encode_cache_cpuid8000001d(l1d, cs->nr_threads, + eax, ebx, ecx, edx); + break; + case 1: /* L1 icache info */ + encode_cache_cpuid8000001d(l1i, cs->nr_threads, + eax, ebx, ecx, edx); + break; + case 2: /* L2 cache info */ + encode_cache_cpuid8000001d(l2, cs->nr_threads, + eax, ebx, ecx, edx); + break; + case 3: /* L3 cache info */ + encode_cache_cpuid8000001d(l3, cs->nr_threads, + eax, ebx, ecx, edx); + break; + default: /* end of info */ + *eax = *ebx = *ecx = *edx = 0; + break; + } + break; case 0xC0000000: *eax = env->cpuid_xlevel2; *ebx = 0; diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 6c49954e68..6e66f9c51d 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -967,9 +967,32 @@ int kvm_arch_init_vcpu(CPUState *cs) } c = &cpuid_data.entries[cpuid_i++]; - c->function = i; - c->flags = 0; - cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx); + switch (i) { + case 0x8000001d: + /* Query for all AMD cache information leaves */ + for (j = 0; ; j++) { + c->function = i; + c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX; + c->index = j; + cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx); + + if (c->eax == 0) { + break; + } + if (cpuid_i == KVM_MAX_CPUID_ENTRIES) { + fprintf(stderr, "cpuid_data is full, no space for " + "cpuid(eax:0x%x,ecx:0x%x)\n", i, j); + abort(); + } + c = &cpuid_data.entries[cpuid_i++]; + } + break; + default: + c->function = i; + c->flags = 0; + cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx); + break; + } } /* Call Centaur's CPUID instructions they are supported. */ -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v8 6/8] i386: Add support for CPUID_8000_001E for AMD 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger ` (4 preceding siblings ...) 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 5/8] i386: Populate AMD Processor Cache Information for cpuid 0x8000001D Babu Moger @ 2018-05-10 20:41 ` Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on AMD EPYC CPU Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 8/8] i386: Remove generic SMT thread check Babu Moger 7 siblings, 0 replies; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT feature is supported. This is required to support hyperthreading feature on AMD CPUs. This is supported via CPUID_8000_001E extended functions. Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> --- include/hw/i386/topology.h | 6 ++++++ target/i386/cpu.c | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h index 1ebaee0f76..4af746e50c 100644 --- a/include/hw/i386/topology.h +++ b/include/hw/i386/topology.h @@ -145,4 +145,10 @@ static inline apic_id_t x86_apicid_from_cpu_idx(unsigned nr_cores, return apicid_from_topo_ids(nr_cores, nr_threads, &topo); } +/* Definitions used on CPUID Leaf 0x8000001E */ +#define EXTENDED_APIC_ID(threads, socket_id, core_id, thread_id) \ + ((threads) ? \ + ((socket_id << 6) | (core_id << 1) | thread_id) : \ + ((socket_id << 6) | core_id)) + #endif /* HW_I386_TOPOLOGY_H */ diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 6898042787..5cfc7bb0e1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4072,6 +4072,13 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, break; } break; + case 0x8000001E: + *eax = EXTENDED_APIC_ID((cs->nr_threads - 1), + cpu->socket_id, cpu->core_id, cpu->thread_id); + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id; + *ecx = cpu->socket_id; + *edx = 0; + break; case 0xC0000000: *eax = env->cpuid_xlevel2; *ebx = 0; -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on AMD EPYC CPU 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger ` (5 preceding siblings ...) 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 6/8] i386: Add support for CPUID_8000_001E for AMD Babu Moger @ 2018-05-10 20:41 ` Babu Moger 2018-05-11 20:46 ` Eduardo Habkost 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 8/8] i386: Remove generic SMT thread check Babu Moger 7 siblings, 1 reply; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger Enable TOPOEXT feature on EPYC CPU. This is required to support hyperthreading on VM guests. Also extend xlevel to 0x8000001E. Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> --- target/i386/cpu.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 5cfc7bb0e1..575f2416a1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -2382,7 +2382,8 @@ static X86CPUDefinition builtin_x86_defs[] = { .features[FEAT_8000_0001_ECX] = CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | - CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, + CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM | + CPUID_EXT3_TOPOEXT, .features[FEAT_7_0_EBX] = CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED | @@ -2427,7 +2428,8 @@ static X86CPUDefinition builtin_x86_defs[] = { .features[FEAT_8000_0001_ECX] = CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | - CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, + CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM | + CPUID_EXT3_TOPOEXT, .features[FEAT_8000_0008_EBX] = CPUID_8000_0008_EBX_IBPB, .features[FEAT_7_0_EBX] = @@ -4540,6 +4542,11 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A); } + /* TOPOEXT feature requires 0x8000001E */ + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) { + x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001E); + } + /* SEV requires CPUID[0x8000001F] */ if (sev_enabled()) { x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001F); -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on AMD EPYC CPU 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on AMD EPYC CPU Babu Moger @ 2018-05-11 20:46 ` Eduardo Habkost 2018-05-11 22:16 ` Moger, Babu 0 siblings, 1 reply; 20+ messages in thread From: Eduardo Habkost @ 2018-05-11 20:46 UTC (permalink / raw) To: Babu Moger Cc: mst, marcel.apfelbaum, pbonzini, rth, mtosatti, geoff, kash, qemu-devel, kvm On Thu, May 10, 2018 at 03:41:47PM -0500, Babu Moger wrote: > Enable TOPOEXT feature on EPYC CPU. This is required to support > hyperthreading on VM guests. Also extend xlevel to 0x8000001E. > > Signed-off-by: Babu Moger <babu.moger@amd.com> > Tested-by: Geoffrey McRae <geoff@hostfission.com> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target/i386/cpu.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 5cfc7bb0e1..575f2416a1 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -2382,7 +2382,8 @@ static X86CPUDefinition builtin_x86_defs[] = { > .features[FEAT_8000_0001_ECX] = > CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH | > CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | > - CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, > + CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM | > + CPUID_EXT3_TOPOEXT, > .features[FEAT_7_0_EBX] = > CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 | > CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED | > @@ -2427,7 +2428,8 @@ static X86CPUDefinition builtin_x86_defs[] = { > .features[FEAT_8000_0001_ECX] = > CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH | > CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | > - CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, > + CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM | > + CPUID_EXT3_TOPOEXT, I forgot about one thing: you'll need to add EPYC.topoext=off and EPYC-IBPB.topoext=off to PC_COMPAT_2_12. > .features[FEAT_8000_0008_EBX] = > CPUID_8000_0008_EBX_IBPB, > .features[FEAT_7_0_EBX] = > @@ -4540,6 +4542,11 @@ static void x86_cpu_expand_features(X86CPU *cpu, Error **errp) > x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A); > } > > + /* TOPOEXT feature requires 0x8000001E */ > + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) { > + x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001E); > + } > + > /* SEV requires CPUID[0x8000001F] */ > if (sev_enabled()) { > x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001F); > -- > 2.17.0 > > -- Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on AMD EPYC CPU 2018-05-11 20:46 ` Eduardo Habkost @ 2018-05-11 22:16 ` Moger, Babu 0 siblings, 0 replies; 20+ messages in thread From: Moger, Babu @ 2018-05-11 22:16 UTC (permalink / raw) To: Eduardo Habkost Cc: mst@redhat.com, marcel.apfelbaum@gmail.com, pbonzini@redhat.com, rth@twiddle.net, mtosatti@redhat.com, geoff@hostfission.com, kash@tripleback.net, qemu-devel@nongnu.org, kvm@vger.kernel.org > -----Original Message----- > From: Eduardo Habkost [mailto:ehabkost@redhat.com] > Sent: Friday, May 11, 2018 3:47 PM > To: Moger, Babu <Babu.Moger@amd.com> > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com; > rth@twiddle.net; mtosatti@redhat.com; geoff@hostfission.com; > kash@tripleback.net; qemu-devel@nongnu.org; kvm@vger.kernel.org > Subject: Re: [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on > AMD EPYC CPU > > On Thu, May 10, 2018 at 03:41:47PM -0500, Babu Moger wrote: > > Enable TOPOEXT feature on EPYC CPU. This is required to support > > hyperthreading on VM guests. Also extend xlevel to 0x8000001E. > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > Tested-by: Geoffrey McRae <geoff@hostfission.com> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > target/i386/cpu.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > index 5cfc7bb0e1..575f2416a1 100644 > > --- a/target/i386/cpu.c > > +++ b/target/i386/cpu.c > > @@ -2382,7 +2382,8 @@ static X86CPUDefinition builtin_x86_defs[] = { > > .features[FEAT_8000_0001_ECX] = > > CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH | > > CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | > CPUID_EXT3_ABM | > > - CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, > > + CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM > | > > + CPUID_EXT3_TOPOEXT, > > .features[FEAT_7_0_EBX] = > > CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | > CPUID_7_0_EBX_AVX2 | > > CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | > CPUID_7_0_EBX_RDSEED | > > @@ -2427,7 +2428,8 @@ static X86CPUDefinition builtin_x86_defs[] = { > > .features[FEAT_8000_0001_ECX] = > > CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH | > > CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | > CPUID_EXT3_ABM | > > - CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM, > > + CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM > | > > + CPUID_EXT3_TOPOEXT, > > I forgot about one thing: you'll need to add EPYC.topoext=off and > EPYC-IBPB.topoext=off to PC_COMPAT_2_12. Ok. Will add it. > > > .features[FEAT_8000_0008_EBX] = > > CPUID_8000_0008_EBX_IBPB, > > .features[FEAT_7_0_EBX] = > > @@ -4540,6 +4542,11 @@ static void x86_cpu_expand_features(X86CPU > *cpu, Error **errp) > > x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000000A); > > } > > > > + /* TOPOEXT feature requires 0x8000001E */ > > + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) { > > + x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001E); > > + } > > + > > /* SEV requires CPUID[0x8000001F] */ > > if (sev_enabled()) { > > x86_cpu_adjust_level(cpu, &env->cpuid_min_xlevel, 0x8000001F); > > -- > > 2.17.0 > > > > > > -- > Eduardo ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v8 8/8] i386: Remove generic SMT thread check 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger ` (6 preceding siblings ...) 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on AMD EPYC CPU Babu Moger @ 2018-05-10 20:41 ` Babu Moger 7 siblings, 0 replies; 20+ messages in thread From: Babu Moger @ 2018-05-10 20:41 UTC (permalink / raw) To: mst, marcel.apfelbaum, pbonzini, rth, ehabkost, mtosatti Cc: qemu-devel, kvm, geoff, kash, babu.moger Remove generic non-intel check while validating hyperthreading support. Certain AMD CPUs can support hyperthreading now. CPU family with TOPOEXT feature can support hyperthreading now. Signed-off-by: Babu Moger <babu.moger@amd.com> Tested-by: Geoffrey McRae <geoff@hostfission.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> --- target/i386/cpu.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 575f2416a1..17803135ed 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4790,17 +4790,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) qemu_init_vcpu(cs); - /* Only Intel CPUs support hyperthreading. Even though QEMU fixes this - * issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX - * based on inputs (sockets,cores,threads), it is still better to gives + /* Most Intel and certain AMD CPUs support hyperthreading. Even though QEMU + * fixes this issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX + * based on inputs (sockets,cores,threads), it is still better to give * users a warning. * * NOTE: the following code has to follow qemu_init_vcpu(). Otherwise * cs->nr_threads hasn't be populated yet and the checking is incorrect. */ - if (!IS_INTEL_CPU(env) && cs->nr_threads > 1 && !ht_warned) { - error_report("AMD CPU doesn't support hyperthreading. Please configure" - " -smp options properly."); + if (IS_AMD_CPU(env) && + !(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) && + cs->nr_threads > 1 && !ht_warned) { + error_report("This family of AMD CPU doesn't support " + "hyperthreading. Please configure -smp " + "options properly."); ht_warned = true; } -- 2.17.0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-07-17 15:42 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-10 20:41 [Qemu-devel] [PATCH v8 0/8] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 1/8] i386: Helpers to encode cache information consistently Babu Moger 2018-05-11 19:12 ` Eduardo Habkost 2018-07-16 13:20 ` Philippe Mathieu-Daudé 2018-07-16 19:52 ` Eduardo Habkost 2018-07-17 15:42 ` Philippe Mathieu-Daudé 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 2/8] i386: Add cache information in X86CPUDefinition Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 3/8] i386: Add new property to control cache info Babu Moger 2018-05-11 19:21 ` Eduardo Habkost 2018-05-11 20:21 ` Moger, Babu 2018-05-11 20:40 ` Eduardo Habkost 2018-05-11 20:47 ` Eduardo Habkost 2018-05-14 16:44 ` Moger, Babu 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 4/8] i386: Initialize cache information for EPYC family processors Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 5/8] i386: Populate AMD Processor Cache Information for cpuid 0x8000001D Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 6/8] i386: Add support for CPUID_8000_001E for AMD Babu Moger 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 7/8] i386: Enable TOPOEXT feature on AMD EPYC CPU Babu Moger 2018-05-11 20:46 ` Eduardo Habkost 2018-05-11 22:16 ` Moger, Babu 2018-05-10 20:41 ` [Qemu-devel] [PATCH v8 8/8] i386: Remove generic SMT thread check Babu Moger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).