On 9/18/2018 10:57 PM, Duran, Leo wrote:
-----Original Message-----
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Tuesday, September 18, 2018 3:34 AM
To: Laszlo Ersek <ler...@redhat.com>; Duran, Leo <leo.du...@amd.com>;
edk2-devel@lists.01.org
Cc: Dong, Eric <eric.d...@intel.com>
Subject: Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling
MTRRs prior to MTRR change.
On 9/18/2018 12:38 AM, Laszlo Ersek wrote:
On 09/17/18 18:20, Duran, Leo wrote:
-----Original Message-----
From: Ni, Ruiyu <ruiyu...@intel.com>
Sent: Thursday, September 13, 2018 11:44 PM
To: Duran, Leo <leo.du...@amd.com>; Laszlo Ersek
<ler...@redhat.com>; edk2-devel@lists.01.org
Cc: Dong, Eric <eric.d...@intel.com>
Subject: Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip
disabling MTRRs prior to MTRR change.
On 9/14/2018 3:31 AM, Duran, Leo wrote:
-----Original Message-----
From: Ni, Ruiyu <ruiyu...@intel.com>
Sent: Wednesday, September 12, 2018 9:39 PM
To: Duran, Leo <leo.du...@amd.com>; Laszlo Ersek
<ler...@redhat.com>;
edk2-devel@lists.01.org
Cc: Dong, Eric <eric.d...@intel.com>
Subject: RE: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip
disabling MTRRs prior to MTRR change.
Leo,
Sorry I was in leave yesterday so didn't see the mail.
Which MSRs are shared? Only the
MSR_IA32_MTRR_DEF_TYPE_REGISTER?
Or all the MSRs that configures the CPU MTRR setting?
Hi Ray,
The MTTR config MSRs are also shared by threads within a core.
Hi Leo,
Do you think that fixing the caller is more proper?
Hi Ray,
Actually,
The proposed PCD is the simplest solution, as that works for us and does
not change the existing (default) flow.
That is,
I'd prefer making a decision about the PCD in platform-specific code,
rather than introducing complex detection and heuristics at the caller level in
EDK2 (just for AMD).
So, please approve the PCD.
Leo,
I agree with you on the first part "the PCD is the simplest solution".
But this really looks like a workaround of the real issue.
For a multiple-socket system, it may contain S sockets, each socket contains C
cores and each core contains T threads. In summary the system contains S *
C * T threads.
As you said all threads inside a core share the MTRR setting.
Do all cores inside a socket share the MTRR setting?
Do all sockets share the MTRR setting?
If one of the answer of above questions is "no", how can we configure the
PCD?
[Duran, Leo]
Hi Ray,
The MTTR settings are share by threads within a core (but each core has its
own, etc.)
The PCD would be set in our platform-specific code (e.g., it can be set at
build-time in the .DSC file).
As I mentioned,
We don't need (Mtrr.Enable=0) to change MTRR settings, so having the PCD to
skip (Mtrr.Enable=0) is reasonable for us.
Leo.
If the PCD is false, no thread disables the MTRR before programming it.
Is it safe? Per Intel's SDM, it's not.
Maybe it works in AMD's case. But I still suggest we change the caller,
which is more natural.
At least I'd like to see how potential-ugly the change can be.
We can then discuss how to make the ugly change better looking.
- From my side, if it works for you, it works for me. (The general
trend has been to avoid adding more PCDs to the "core" package DEC
files, but I'm 100% neutral on that.)
Laszlo
Laszlo,
Thanks for pointing out the general trend. Yes less PCDs are very welcomed.
To me, PCD is no different from protocol. And even worse, because it's very
easily to be over-used.
But I am not sure whether a PCD has to be introduced for this issue.
Maybe even we choose to fix the caller, the PCD is still needed. I am not
sure.
--
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
--
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
