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 a20fe26..b6c1592 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 = 8,
+ .sets = 1024,
+ .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 1b219fa..fa03e2c 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.7.4
Hi,
I was about to apply this because I assumed it was the same patch
I sent in March, but then I found this:
On Thu, Apr 26, 2018 at 11:26:41AM -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>
[...]
> -#define L2_ASSOCIATIVITY 16
[...]
> /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
> +static CPUCacheInfo l2_cache_amd = {
[...]
> + .associativity = 8,
[...]
> +};
[...]
> case 0x80000006:
[...]
> - *ecx = (L2_SIZE_KB_AMD << 16) | \
> - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
[...]
> + encode_cache_cpuid80000006(&l2_cache_amd,
> + cpu->enable_l3_cache ? &l3_cache : NULL,
> + ecx, edx);
[...]
The structs added by this patch are supposed to represent the
legacy cache sizes, and must match the old code. My original
patch set l2_cache_amd.associativity=16 because of that.
This patch changes 0x80000006 from associativity=16 to
associativity=8. Why?
--
Eduardo
Eduardo,
Thanks for all the comments. Will respond to each one separately.
> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Monday, May 7, 2018 2:05 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mst@redhat.com; marcel@redhat.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 v7 1/9] i386: Helpers to encode cache
> information consistently
>
> Hi,
>
> I was about to apply this because I assumed it was the same patch
> I sent in March, but then I found this:
>
> On Thu, Apr 26, 2018 at 11:26:41AM -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>
> [...]
> > -#define L2_ASSOCIATIVITY 16
> [...]
> > /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
> > +static CPUCacheInfo l2_cache_amd = {
> [...]
> > + .associativity = 8,
> [...]
> > +};
> [...]
> > case 0x80000006:
> [...]
> > - *ecx = (L2_SIZE_KB_AMD << 16) | \
> > - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> > - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
> [...]
> > + encode_cache_cpuid80000006(&l2_cache_amd,
> > + cpu->enable_l3_cache ? &l3_cache : NULL,
> > + ecx, edx);
> [...]
>
> The structs added by this patch are supposed to represent the
> legacy cache sizes, and must match the old code. My original
> patch set l2_cache_amd.associativity=16 because of that.
>
> This patch changes 0x80000006 from associativity=16 to
> associativity=8. Why?
The original code had a bug here. The associativity should have been 8.
My earlier response from the thread
http://patchwork.ozlabs.org/patch/884880/
This should have been 8-way. This is a bug. Will fix.
This should have been (AMD_ENC_ASSOC(L2_ASSOCIATIVITY_AMD) << 12)
>
> --
> Eduardo
On Mon, May 07, 2018 at 09:14:27PM +0000, Moger, Babu wrote:
> Eduardo,
> Thanks for all the comments. Will respond to each one separately.
>
> > -----Original Message-----
> > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > Sent: Monday, May 7, 2018 2:05 PM
> > To: Moger, Babu <Babu.Moger@amd.com>
> > Cc: mst@redhat.com; marcel@redhat.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 v7 1/9] i386: Helpers to encode cache
> > information consistently
> >
> > Hi,
> >
> > I was about to apply this because I assumed it was the same patch
> > I sent in March, but then I found this:
> >
> > On Thu, Apr 26, 2018 at 11:26:41AM -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>
> > [...]
> > > -#define L2_ASSOCIATIVITY 16
> > [...]
> > > /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
> > > +static CPUCacheInfo l2_cache_amd = {
> > [...]
> > > + .associativity = 8,
> > [...]
> > > +};
> > [...]
> > > case 0x80000006:
> > [...]
> > > - *ecx = (L2_SIZE_KB_AMD << 16) | \
> > > - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> > > - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
> > [...]
> > > + encode_cache_cpuid80000006(&l2_cache_amd,
> > > + cpu->enable_l3_cache ? &l3_cache : NULL,
> > > + ecx, edx);
> > [...]
> >
> > The structs added by this patch are supposed to represent the
> > legacy cache sizes, and must match the old code. My original
> > patch set l2_cache_amd.associativity=16 because of that.
> >
> > This patch changes 0x80000006 from associativity=16 to
> > associativity=8. Why?
>
> The original code had a bug here. The associativity should have been 8.
> My earlier response from the thread
> http://patchwork.ozlabs.org/patch/884880/
>
> This should have been 8-way. This is a bug. Will fix.
> This should have been (AMD_ENC_ASSOC(L2_ASSOCIATIVITY_AMD) << 12)
If we want to change the associativity, we must keep the old
values on older machine-types, which was the whole purpose of the
"legacy-cache" property.
I suggest using the new X86CPUDefinition::cache_info field if you
want to make AMD CPUs report different associativity.
--
Eduardo
> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Monday, May 7, 2018 4:27 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;
> marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> Subject: Re: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode cache
> information consistently
>
> On Mon, May 07, 2018 at 09:14:27PM +0000, Moger, Babu wrote:
> > Eduardo,
> > Thanks for all the comments. Will respond to each one separately.
> >
> > > -----Original Message-----
> > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > Sent: Monday, May 7, 2018 2:05 PM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: mst@redhat.com; marcel@redhat.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 v7 1/9] i386: Helpers to encode cache
> > > information consistently
> > >
> > > Hi,
> > >
> > > I was about to apply this because I assumed it was the same patch
> > > I sent in March, but then I found this:
> > >
> > > On Thu, Apr 26, 2018 at 11:26:41AM -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>
> > > [...]
> > > > -#define L2_ASSOCIATIVITY 16
> > > [...]
> > > > /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
> > > > +static CPUCacheInfo l2_cache_amd = {
> > > [...]
> > > > + .associativity = 8,
> > > [...]
> > > > +};
> > > [...]
> > > > case 0x80000006:
> > > [...]
> > > > - *ecx = (L2_SIZE_KB_AMD << 16) | \
> > > > - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> > > > - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
> > > [...]
> > > > + encode_cache_cpuid80000006(&l2_cache_amd,
> > > > + cpu->enable_l3_cache ? &l3_cache : NULL,
> > > > + ecx, edx);
> > > [...]
> > >
> > > The structs added by this patch are supposed to represent the
> > > legacy cache sizes, and must match the old code. My original
> > > patch set l2_cache_amd.associativity=16 because of that.
> > >
> > > This patch changes 0x80000006 from associativity=16 to
> > > associativity=8. Why?
> >
> > The original code had a bug here. The associativity should have been 8.
> > My earlier response from the thread
> > http://patchwork.ozlabs.org/patch/884880/
> >
> > This should have been 8-way. This is a bug. Will fix.
> > This should have been (AMD_ENC_ASSOC(L2_ASSOCIATIVITY_AMD) <<
> 12)
>
> If we want to change the associativity, we must keep the old
> values on older machine-types, which was the whole purpose of the
> "legacy-cache" property.
>
> I suggest using the new X86CPUDefinition::cache_info field if you
> want to make AMD CPUs report different associativity.
Ok. Sure. I will change it. Thanks
>
> --
> Eduardo
Hi Eduardo, One more comment below.
> -----Original Message-----
> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org]
> On Behalf Of Moger, Babu
> Sent: Monday, May 7, 2018 5:48 PM
> To: Eduardo Habkost <ehabkost@redhat.com>
> Cc: geoff@hostfission.com; kvm@vger.kernel.org; mst@redhat.com;
> kash@tripleback.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> Subject: RE: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode cache
> information consistently
>
>
>
> > -----Original Message-----
> > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > Sent: Monday, May 7, 2018 4:27 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;
> > marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> > Subject: Re: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode cache
> > information consistently
> >
> > On Mon, May 07, 2018 at 09:14:27PM +0000, Moger, Babu wrote:
> > > Eduardo,
> > > Thanks for all the comments. Will respond to each one separately.
> > >
> > > > -----Original Message-----
> > > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > > Sent: Monday, May 7, 2018 2:05 PM
> > > > To: Moger, Babu <Babu.Moger@amd.com>
> > > > Cc: mst@redhat.com; marcel@redhat.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 v7 1/9] i386: Helpers to encode
> cache
> > > > information consistently
> > > >
> > > > Hi,
> > > >
> > > > I was about to apply this because I assumed it was the same patch
> > > > I sent in March, but then I found this:
> > > >
> > > > On Thu, Apr 26, 2018 at 11:26:41AM -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>
> > > > [...]
> > > > > -#define L2_ASSOCIATIVITY 16
> > > > [...]
> > > > > /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
> > > > > +static CPUCacheInfo l2_cache_amd = {
> > > > [...]
> > > > > + .associativity = 8,
> > > > [...]
> > > > > +};
> > > > [...]
> > > > > case 0x80000006:
> > > > [...]
> > > > > - *ecx = (L2_SIZE_KB_AMD << 16) | \
> > > > > - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> > > > > - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
> > > > [...]
> > > > > + encode_cache_cpuid80000006(&l2_cache_amd,
> > > > > + cpu->enable_l3_cache ? &l3_cache : NULL,
> > > > > + ecx, edx);
> > > > [...]
> > > >
> > > > The structs added by this patch are supposed to represent the
> > > > legacy cache sizes, and must match the old code. My original
> > > > patch set l2_cache_amd.associativity=16 because of that.
> > > >
> > > > This patch changes 0x80000006 from associativity=16 to
> > > > associativity=8. Why?
Keeping this to 16 will be a problem. It breaks the size rule(line size *associativity * partitions * sets).
It asserts violating that rule. Now I remember. That is why I changed it to 8. I would think it would be
better to fix it now.
> > >
> > > The original code had a bug here. The associativity should have been 8.
> > > My earlier response from the thread
> > > http://patchwork.ozlabs.org/patch/884880/
> > >
> > > This should have been 8-way. This is a bug. Will fix.
> > > This should have been (AMD_ENC_ASSOC(L2_ASSOCIATIVITY_AMD) <<
> > 12)
> >
> > If we want to change the associativity, we must keep the old
> > values on older machine-types, which was the whole purpose of the
> > "legacy-cache" property.
> >
> > I suggest using the new X86CPUDefinition::cache_info field if you
> > want to make AMD CPUs report different associativity.
>
> Ok. Sure. I will change it. Thanks
>
> >
> > --
> > Eduardo
On Tue, May 08, 2018 at 06:40:23PM +0000, Moger, Babu wrote:
> Hi Eduardo, One more comment below.
>
> > -----Original Message-----
> > From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org]
> > On Behalf Of Moger, Babu
> > Sent: Monday, May 7, 2018 5:48 PM
> > To: Eduardo Habkost <ehabkost@redhat.com>
> > Cc: geoff@hostfission.com; kvm@vger.kernel.org; mst@redhat.com;
> > kash@tripleback.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> > marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> > Subject: RE: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode cache
> > information consistently
> >
> >
> >
> > > -----Original Message-----
> > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > Sent: Monday, May 7, 2018 4:27 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;
> > > marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> > > Subject: Re: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode cache
> > > information consistently
> > >
> > > On Mon, May 07, 2018 at 09:14:27PM +0000, Moger, Babu wrote:
> > > > Eduardo,
> > > > Thanks for all the comments. Will respond to each one separately.
> > > >
> > > > > -----Original Message-----
> > > > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > > > Sent: Monday, May 7, 2018 2:05 PM
> > > > > To: Moger, Babu <Babu.Moger@amd.com>
> > > > > Cc: mst@redhat.com; marcel@redhat.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 v7 1/9] i386: Helpers to encode
> > cache
> > > > > information consistently
> > > > >
> > > > > Hi,
> > > > >
> > > > > I was about to apply this because I assumed it was the same patch
> > > > > I sent in March, but then I found this:
> > > > >
> > > > > On Thu, Apr 26, 2018 at 11:26:41AM -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>
> > > > > [...]
> > > > > > -#define L2_ASSOCIATIVITY 16
> > > > > [...]
> > > > > > /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4 */
> > > > > > +static CPUCacheInfo l2_cache_amd = {
> > > > > [...]
> > > > > > + .associativity = 8,
> > > > > [...]
> > > > > > +};
> > > > > [...]
> > > > > > case 0x80000006:
> > > > > [...]
> > > > > > - *ecx = (L2_SIZE_KB_AMD << 16) | \
> > > > > > - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> > > > > > - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
> > > > > [...]
> > > > > > + encode_cache_cpuid80000006(&l2_cache_amd,
> > > > > > + cpu->enable_l3_cache ? &l3_cache : NULL,
> > > > > > + ecx, edx);
> > > > > [...]
> > > > >
> > > > > The structs added by this patch are supposed to represent the
> > > > > legacy cache sizes, and must match the old code. My original
> > > > > patch set l2_cache_amd.associativity=16 because of that.
> > > > >
> > > > > This patch changes 0x80000006 from associativity=16 to
> > > > > associativity=8. Why?
>
> Keeping this to 16 will be a problem. It breaks the size rule(line size *associativity * partitions * sets).
> It asserts violating that rule. Now I remember. That is why I changed it to 8. I would think it would be
> better to fix it now.
You can fix it, no problem. You just need to make sure
CPUID[0x80000006] data for old machine-types without topoext will
stay the same.
Remember that l2_cache_amd is just for the legacy values enabled
by legacy-cache=on. Non-legacy values with more consistent data
can be set at the CPU model table.
The current CPUID data is:
#define L2_SIZE_KB_AMD 512
#define L2_ASSOCIATIVITY 16
#define L2_LINES_PER_TAG 1
#define L2_LINE_SIZE 64
[...]
case 0x80000006:
*ecx = (L2_SIZE_KB_AMD << 16) | \
(AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
(L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
This means l2_cache_amd.size must be 512KiB,
l2_cache_amd.associativity must be 16, l2_cache_amd.lines_per_tag
must be 1, and l2_cache_amd.line_size must be 64.
l2_cache_amd.partitions and l2_cache_amd.sets, on the other hand,
can be set to any value you wish.
Another option is to simply disable CPUID[0x8000001d] if
legacy-cache=on, so we don't need to worry about consistency on
legacy cache entries that are known to be inconsistent.
If we want to be extra safe/consistent, we can automatically set
legacy-cache=off if topoext is enabled, and prevent QEMU from
starting if topoext is enabled on a CPU without
X86CPUDefinition.cache_info.
We have lots of freedom on what to do when topoext is enabled or
when using new machine-types. We just can't change the CPUID
data of old machine-types when topoext is disabled.
>
> > > >
> > > > The original code had a bug here. The associativity should have been 8.
> > > > My earlier response from the thread
> > > > http://patchwork.ozlabs.org/patch/884880/
> > > >
> > > > This should have been 8-way. This is a bug. Will fix.
> > > > This should have been (AMD_ENC_ASSOC(L2_ASSOCIATIVITY_AMD) <<
> > > 12)
> > >
> > > If we want to change the associativity, we must keep the old
> > > values on older machine-types, which was the whole purpose of the
> > > "legacy-cache" property.
> > >
> > > I suggest using the new X86CPUDefinition::cache_info field if you
> > > want to make AMD CPUs report different associativity.
> >
> > Ok. Sure. I will change it. Thanks
> >
> > >
> > > --
> > > Eduardo
--
Eduardo
> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Tuesday, May 8, 2018 2:08 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;
> marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> Subject: Re: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode cache
> information consistently
>
> On Tue, May 08, 2018 at 06:40:23PM +0000, Moger, Babu wrote:
> > Hi Eduardo, One more comment below.
> >
> > > -----Original Message-----
> > > From: kvm-owner@vger.kernel.org [mailto:kvm-
> owner@vger.kernel.org]
> > > On Behalf Of Moger, Babu
> > > Sent: Monday, May 7, 2018 5:48 PM
> > > To: Eduardo Habkost <ehabkost@redhat.com>
> > > Cc: geoff@hostfission.com; kvm@vger.kernel.org; mst@redhat.com;
> > > kash@tripleback.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> > > marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> > > Subject: RE: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode cache
> > > information consistently
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > > Sent: Monday, May 7, 2018 4:27 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;
> > > > marcel@redhat.com; pbonzini@redhat.com; rth@twiddle.net
> > > > Subject: Re: [Qemu-devel] [PATCH v7 1/9] i386: Helpers to encode
> cache
> > > > information consistently
> > > >
> > > > On Mon, May 07, 2018 at 09:14:27PM +0000, Moger, Babu wrote:
> > > > > Eduardo,
> > > > > Thanks for all the comments. Will respond to each one separately.
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > > > > Sent: Monday, May 7, 2018 2:05 PM
> > > > > > To: Moger, Babu <Babu.Moger@amd.com>
> > > > > > Cc: mst@redhat.com; marcel@redhat.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 v7 1/9] i386: Helpers to encode
> > > cache
> > > > > > information consistently
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I was about to apply this because I assumed it was the same patch
> > > > > > I sent in March, but then I found this:
> > > > > >
> > > > > > On Thu, Apr 26, 2018 at 11:26:41AM -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>
> > > > > > [...]
> > > > > > > -#define L2_ASSOCIATIVITY 16
> > > > > > [...]
> > > > > > > /*FIXME: CPUID leaf 0x80000006 is inconsistent with leaves 2 & 4
> */
> > > > > > > +static CPUCacheInfo l2_cache_amd = {
> > > > > > [...]
> > > > > > > + .associativity = 8,
> > > > > > [...]
> > > > > > > +};
> > > > > > [...]
> > > > > > > case 0x80000006:
> > > > > > [...]
> > > > > > > - *ecx = (L2_SIZE_KB_AMD << 16) | \
> > > > > > > - (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> > > > > > > - (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
> > > > > > [...]
> > > > > > > + encode_cache_cpuid80000006(&l2_cache_amd,
> > > > > > > + cpu->enable_l3_cache ? &l3_cache : NULL,
> > > > > > > + ecx, edx);
> > > > > > [...]
> > > > > >
> > > > > > The structs added by this patch are supposed to represent the
> > > > > > legacy cache sizes, and must match the old code. My original
> > > > > > patch set l2_cache_amd.associativity=16 because of that.
> > > > > >
> > > > > > This patch changes 0x80000006 from associativity=16 to
> > > > > > associativity=8. Why?
> >
> > Keeping this to 16 will be a problem. It breaks the size rule(line size
> *associativity * partitions * sets).
> > It asserts violating that rule. Now I remember. That is why I changed it to 8.
> I would think it would be
> > better to fix it now.
>
> You can fix it, no problem. You just need to make sure
> CPUID[0x80000006] data for old machine-types without topoext will
> stay the same.
>
> Remember that l2_cache_amd is just for the legacy values enabled
> by legacy-cache=on. Non-legacy values with more consistent data
> can be set at the CPU model table.
>
> The current CPUID data is:
>
> #define L2_SIZE_KB_AMD 512
> #define L2_ASSOCIATIVITY 16
> #define L2_LINES_PER_TAG 1
> #define L2_LINE_SIZE 64
> [...]
> case 0x80000006:
> *ecx = (L2_SIZE_KB_AMD << 16) | \
> (AMD_ENC_ASSOC(L2_ASSOCIATIVITY) << 12) | \
> (L2_LINES_PER_TAG << 8) | (L2_LINE_SIZE);
>
>
> This means l2_cache_amd.size must be 512KiB,
> l2_cache_amd.associativity must be 16, l2_cache_amd.lines_per_tag
> must be 1, and l2_cache_amd.line_size must be 64.
>
> l2_cache_amd.partitions and l2_cache_amd.sets, on the other hand,
> can be set to any value you wish.
Ok. Let me adjust sets and partitions. Thanks
>
> Another option is to simply disable CPUID[0x8000001d] if
> legacy-cache=on, so we don't need to worry about consistency on
> legacy cache entries that are known to be inconsistent.
>
> If we want to be extra safe/consistent, we can automatically set
> legacy-cache=off if topoext is enabled, and prevent QEMU from
> starting if topoext is enabled on a CPU without
> X86CPUDefinition.cache_info.
>
> We have lots of freedom on what to do when topoext is enabled or
> when using new machine-types. We just can't change the CPUID
> data of old machine-types when topoext is disabled.
>
>
> >
> > > > >
> > > > > The original code had a bug here. The associativity should have been
> 8.
> > > > > My earlier response from the thread
> > > > > http://patchwork.ozlabs.org/patch/884880/
> > > > >
> > > > > This should have been 8-way. This is a bug. Will fix.
> > > > > This should have been (AMD_ENC_ASSOC(L2_ASSOCIATIVITY_AMD)
> <<
> > > > 12)
> > > >
> > > > If we want to change the associativity, we must keep the old
> > > > values on older machine-types, which was the whole purpose of the
> > > > "legacy-cache" property.
> > > >
> > > > I suggest using the new X86CPUDefinition::cache_info field if you
> > > > want to make AMD CPUs report different associativity.
> > >
> > > Ok. Sure. I will change it. Thanks
> > >
> > > >
> > > > --
> > > > Eduardo
>
> --
> Eduardo
© 2016 - 2025 Red Hat, Inc.