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

[Maksim Davydov <davydov-max@xxxxxxxxxxxxxx>]

On 4/24/23 19:33, Babu Moger wrote:
> From: Michael Roth <michael.roth@xxxxxxx>
>
> 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@xxxxxxx>
> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> ---
>   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
"EPYC-v2 <-> AMD EPYC Processor (with IBPB) <-> EPYC-IBPB"
> +                    { /* end of list */ }
> +                },
> +                .cache_info = &epyc_rome_v3_cache_info
> +            },
>               { /* end of list */ }
>           }
>       },

--
Best regards,
Maksim Davydov