Re: [PATCH v3 2/7] target/i386: Add new EPYC CPU versions with updated cache_info

[Moger, Babu]

Hi Maksim,

On 4/25/23 07:51, Maksim Davydov wrote:
> 
> On 4/24/23 19:33, Babu Moger wrote:
>> From: Michael Roth <michael.roth@amd.com>
>>
>> Introduce new EPYC cpu versions: EPYC-v4 and EPYC-Rome-v3.
>> The only difference vs. older models is an updated cache_info with
>> the 'complex_indexing' bit unset, since this bit is not currently
>> defined for AMD and may cause problems should it be used for
>> something else in the future. Setting this bit will also cause
>> CPUID validation failures when running SEV-SNP guests.
>>
>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>   target/i386/cpu.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 118 insertions(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index e3d9eaa307..c1bc47661d 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -1707,6 +1707,56 @@ static const CPUCaches epyc_cache_info = {
>>       },
>>   };
>>   +static CPUCaches epyc_v4_cache_info = {
>> +    .l1d_cache = &(CPUCacheInfo) {
>> +        .type = DATA_CACHE,
>> +        .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 = &(CPUCacheInfo) {
>> +        .type = INSTRUCTION_CACHE,
>> +        .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 = &(CPUCacheInfo) {
>> +        .type = UNIFIED_CACHE,
>> +        .level = 2,
>> +        .size = 512 * KiB,
>> +        .line_size = 64,
>> +        .associativity = 8,
>> +        .partitions = 1,
>> +        .sets = 1024,
>> +        .lines_per_tag = 1,
>> +    },
>> +    .l3_cache = &(CPUCacheInfo) {
>> +        .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 = false,
>> +    },
>> +};
>> +
>>   static const CPUCaches epyc_rome_cache_info = {
>>       .l1d_cache = &(CPUCacheInfo) {
>>           .type = DATA_CACHE,
>> @@ -1757,6 +1807,56 @@ static const CPUCaches epyc_rome_cache_info = {
>>       },
>>   };
>>   +static const CPUCaches epyc_rome_v3_cache_info = {
>> +    .l1d_cache = &(CPUCacheInfo) {
>> +        .type = DATA_CACHE,
>> +        .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 = &(CPUCacheInfo) {
>> +        .type = INSTRUCTION_CACHE,
>> +        .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,
>> +    },
>> +    .l2_cache = &(CPUCacheInfo) {
>> +        .type = UNIFIED_CACHE,
>> +        .level = 2,
>> +        .size = 512 * KiB,
>> +        .line_size = 64,
>> +        .associativity = 8,
>> +        .partitions = 1,
>> +        .sets = 1024,
>> +        .lines_per_tag = 1,
>> +    },
>> +    .l3_cache = &(CPUCacheInfo) {
>> +        .type = UNIFIED_CACHE,
>> +        .level = 3,
>> +        .size = 16 * MiB,
>> +        .line_size = 64,
>> +        .associativity = 16,
>> +        .partitions = 1,
>> +        .sets = 16384,
>> +        .lines_per_tag = 1,
>> +        .self_init = true,
>> +        .inclusive = true,
>> +        .complex_indexing = false,
>> +    },
>> +};
>> +
>>   static const CPUCaches epyc_milan_cache_info = {
>>       .l1d_cache = &(CPUCacheInfo) {
>>           .type = DATA_CACHE,
>> @@ -4091,6 +4191,15 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>>                       { /* end of list */ }
>>                   }
>>               },
>> +            {
>> +                .version = 4,
>> +                .props = (PropValue[]) {
>> +                    { "model-id",
>> +                      "AMD EPYC-v4 Processor" },
>> +                    { /* end of list */ }
>> +                },
>> +                .cache_info = &epyc_v4_cache_info
>> +            },
>>               { /* end of list */ }
>>           }
>>       },
>> @@ -4210,6 +4319,15 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>>                       { /* end of list */ }
>>                   }
>>               },
>> +            {
>> +                .version = 3,
>> +                .props = (PropValue[]) {
>> +                    { "model-id",
>> +                      "AMD EPYC-Rome-v3 Processor" },
> What do you think about adding more information to the model name to reveal
> its key feature? For instance, model-id can be "EPYC-Rome-v3 (NO INDEXING)",
> because only cache info was affected. Or alias can be used to achieve
> the same effect. It works well in

Actually, we already thought about it. But decided against it. Reason is,
when we add "(NO INDEXING)" to v3, we need to keep text in all the future
revisions v4 etc and other cpu models. Otherwise it will give the
impression that newer versions does not support "NO indexing". Hope it helps.

> "EPYC-v2 <-> AMD EPYC Processor (with IBPB) <-> EPYC-IBPB"
>> +                    { /* end of list */ }
>> +                },
>> +                .cache_info = &epyc_rome_v3_cache_info
>> +            },
>>               { /* end of list */ }
>>           }
>>       },
> 

-- 
Thanks
Babu Moger