Re: 回复: 回复: [edk2-devel] [PATCH v2] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT.

["Laszlo Ersek" <lersek@redhat.com>]

On 09/04/20 09:32, Fan Jeff wrote:
> Laszlo,
>
> Why sync the BSP's CR3/GDT/IDT values for AP when AP wakes up instead
> of using old cached BSP's CR3/GDT/IDT values when CPU driver
> initiallly dispatched is that we CANNOT assume original values are
> still valid during POST phase.
>
> On this senario, BSP's CR3/GDT/IDT are just like input parameters for
> one function. Validating them are necessary.
>
> For example, DebugAgentDxe driver may be loaded in UEFI shell to setup
> source level debugging enviroment of EDKII debugger tools.  It may
> setup new IDT space.

Then DebugAgentDxe should be changed to allocate the new IDT in the
32-bit address space.

I don't think it's acceptable that loading DebugAgentDxe from the UEFI
shell may or may not render the MP services protocol unusable. What if
the programmer wants to debug something related to MP services? "I've
loaded DebugAgentDxe, but now I cannot start the payload I want to
debug."

As far as I can see,
"SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgent/DxeDebugAgentLib.c"
uses the static variable "mIdtEntryTable" as IDT. I think it should be
possible to replace "mIdtEntryTable" with a dynamically allocated
object. The allocation should be restricted to 32-bit. It should be
possible to perform the allocation very early (perhaps in the library
constructor).

Again, I'm OK with adding ASSERT()s to FillExchangeInfoData(). If IDT /
GDT / CR3 are out of 32-bit address space, then that's a programming
error, or a platform misconfiguration. It's not something we should try
to dynamically recover from.

The MP services protocol implementation may not expect that the
CR3/GDT/IDT remain unchanged on the BSP during the DXE/BDS phases, but
it does expect them to remain under 4GB.

- A UEFI driver or app must not break this assumption at all, because a
  UEFI driver or app has no business messing with these artifacts.

- Whereas a DXE driver (such as DebugAgentDxe), exactly because it is
  part of the platform firmware, is expected to collaborate with CpuDxe
  / MpInitLib, and to satisfy the invariants.

Thanks,
Laszlo