RE: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@xxxxxxxxxx]
> Sent: Thursday, March 1, 2018 1:56 PM
> To: Moger, Babu <Babu.Moger@xxxxxxx>
> Cc: pbonzini@xxxxxxxxxx; rth@xxxxxxxxxxx; ehabkost@xxxxxxxxxx;
> mtosatti@xxxxxxxxxx; qemu-devel@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx;
> pixo@xxxxxxxxxxxx; Hook, Gary <Gary.Hook@xxxxxxx>
> Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> Information
> 
> 2018-03-01 15:55+0000, Moger, Babu:
> > Radim, Thanks for your comments. I am working on the changes.
> > But, I need few clarifications on your comments. Please see inline.
> >
> > > -----Original Message-----
> > > From: Radim Krčmář [mailto:rkrcmar@xxxxxxxxxx]
> > > Sent: Wednesday, February 28, 2018 12:09 PM
> > > To: Moger, Babu <Babu.Moger@xxxxxxx>
> > > Cc: pbonzini@xxxxxxxxxx; rth@xxxxxxxxxxx; ehabkost@xxxxxxxxxx;
> > > mtosatti@xxxxxxxxxx; qemu-devel@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx;
> > > pixo@xxxxxxxxxxxx; Hook, Gary <Gary.Hook@xxxxxxx>
> > > Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> > > Information
> > >
> > > 2018-02-23 21:30-0500, Babu Moger:
> > > > From: Stanislav Lanci <pixo@xxxxxxxxxxxx>
> > > >
> > > > Adds information about cache size and topology from cpuid 0x8000001D
> > > leaf
> > > > for different cache types on AMD processors.
> > > >
> > > > Signed-off-by: Stanislav Lanci <pixo@xxxxxxxxxxxx>
> > > > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > > > ---
> > > > 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)
> >
> https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-
> 0Fh.pdf
> >  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.
> 
> > > I think we should not expose CPUID 4 with AMD CPUs or at least when
> they
> > > 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).
> 
> > > The numbers looks like real hardware,
> >
> > Do you want me to change anything here?
> 
> No, I was just commending,
> 
> thanks.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux