-----Original Message-----
From: Radim Krčmář [mailto:rkrcmar@redhat.com]
Sent: Thursday, March 1, 2018 1:56 PM
To: Moger, Babu <Babu.Moger@amd.com>
Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
pixo@polepetko.eu; Hook, Gary [off-list ref]
Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
Information

2018-03-01 15:55+0000, Moger, Babu:

quoted

Radim, Thanks for your comments. I am working on the changes.
But, I need few clarifications on your comments. Please see inline.

quoted

-----Original Message-----
From: Radim Krčmář [mailto:rkrcmar@redhat.com]
Sent: Wednesday, February 28, 2018 12:09 PM
To: Moger, Babu <Babu.Moger@amd.com>
Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
pixo@polepetko.eu; Hook, Gary [off-list ref]
Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
Information

2018-02-23 21:30-0500, Babu Moger:

quoted

From: Stanislav Lanci <redacted>

Adds information about cache size and topology from cpuid 0x8000001D

leaf

quoted

for different cache types on AMD processors.

Signed-off-by: Stanislav Lanci <redacted>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---

diff --git a/target/i386/cpu.c b/target/i386/cpu.c

@@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env,
+                *ebx = (L1D_LINE_SIZE - 1) | \
+                       ((L1D_PARTITIONS - 1) << 12) | \
+                       ((L1D_ASSOCIATIVITY - 1) << 22);
+                *ecx = L1D_SETS - 1;

These numbers seem to have the same meaning as CPUID 4, but have
conflicting values.

I am not sure about conflicting values. Looking at the specs(page 78

CPUID_Fn8000001D_EBX_x00)

quoted

https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-
0Fh.pdf

quoted

 It looks correct to me.

I agree.  My comment is misplaced -- it should have been under the place
that uses *_AMD macros.

I wanted to point out that CPUID in QEMU is very Intel-focused and
always contains CPUID leaf 4, which has fields of the very same meaning,
but with different values.

quoted

quoted

I think we should not expose CPUID 4 with AMD CPUs or at least when

they

quoted

quoted

have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).

Can you please elaborate on these comments?
Did you mean we should remove the check CPUID_EXT3_TOPOEXT and

remove all CPUID 4 references?

CPUID 4 should have never been present when emulating AMD CPUs, but it's
worse now that the numbers are conflicting.

I meant to hide CPUID 4 for all AMD CPUs on future machine types, or at
least when CPUID_EXT3_TOPOEXT is enabled.

Sorry, I think I created some confusion here by using CPUID 4 definitions which are
mostly for intel.  Let me rework on this. I will repost the patches.  Thanks for the
feedback.

Keeping the current logic not a big problem as CPUID 4 should never be
used by operating systems on AMD nor trusted inside a VM.  Bringing the
emulation closer to real state would be nice, but this can definitely be
done later (aka never).

quoted

quoted

The numbers looks like real hardware,

Do you want me to change anything here?

No, I was just commending,

thanks.