Re: [Qemu-devel] [PATCH v4 2/5] target/i386: Populate AMD Processor Cache Information

[Eduardo Habkost]

Hi,

Sorry for not reviewing the previous versions of this series (and
making it miss soft freeze).


On Mon, Mar 12, 2018 at 05:00:46PM -0400, Babu Moger wrote:
> From: Stanislav Lanci <address@hidden>
> 
> 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: Stanislav Lanci <address@hidden>
> Signed-off-by: Babu Moger <address@hidden>
> ---
>  target/i386/cpu.c | 65 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  target/i386/kvm.c | 29 ++++++++++++++++++++++---
>  2 files changed, 91 insertions(+), 3 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 42dd381..5fdbedd 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -118,6 +118,7 @@
>  #define L1I_LINE_SIZE         64
>  #define L1I_ASSOCIATIVITY      8
>  #define L1I_SETS              64
> +#define L1I_SETS_AMD         256
>  #define L1I_PARTITIONS         1
>  /* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 32KiB */
>  #define L1I_DESCRIPTOR CPUID_2_L1I_32KB_8WAY_64B
> @@ -129,7 +130,9 @@
>  /* Level 2 unified cache: */
>  #define L2_LINE_SIZE          64
>  #define L2_ASSOCIATIVITY      16
> +#define L2_ASSOCIATIVITY_AMD   8
>  #define L2_SETS             4096
> +#define L2_SETS_AMD         1024
>  #define L2_PARTITIONS          1
>  /* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 4MiB */
>  /*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
> @@ -146,6 +149,7 @@
>  #define L3_N_LINE_SIZE         64
>  #define L3_N_ASSOCIATIVITY     16
>  #define L3_N_SETS           16384
> +#define L3_N_SETS_AMD        8192
>  #define L3_N_PARTITIONS         1
>  #define L3_N_DESCRIPTOR CPUID_2_L3_16MB_16WAY_64B
>  #define L3_N_LINES_PER_TAG      1

I wouldn't like to add even more inconsistencies between
different CPUID leaves.

If you really wish to have different defaults on AMD and Intel,
let's either hide Intel-specific CPUID leaves when using AMD
values, or make all of them agree (and choose the defaults based
on CPU model or vendor id).

-- 
Eduardo