public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Laszlo Ersek" <lersek@redhat.com>
To: devel@edk2.groups.io, garrett.kirkendall@amd.com
Cc: Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD
Date: Fri, 19 Jun 2020 16:48:24 +0200	[thread overview]
Message-ID: <509e8366-8c57-f96d-8684-a548ef148831@redhat.com> (raw)
In-Reply-To: <20200618152245.6483-3-Garrett.Kirkendall@amd.com>

On 06/18/20 17:22, Kirkendall, Garrett wrote:
> AMD does not support MSR_IA32_MISC_ENABLE.  Accessing that register
> causes and exception on AMD processors.  If Execution Disable is
> supported, but if the processor is an AMD processor, skip manipulating
> MSR_IA32_MISC_ENABLE[34] XD Disable bit.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> ---
>
> Notes:
>     Tested on Intel hardware with Laszlo Ersek's help
>
>     (1) downloaded two Linux images from provided links.
>     (2) Test using a 32-bit guest on an Intel host (standing in your edk2 tree, with the patches applied):
>
>     $ build -a IA32 -b DEBUG -p OvmfPkg/OvmfPkgIa32.dsc -t GCC5 -D SMM_REQUIRE
>
>     $ qemu-system-i386 \
>         -cpu coreduo,-nx \
>         -machine q35,smm=on,accel=kvm \
>         -m 4096 \
>         -smp 4 \
>         -global driver=cfi.pflash01,property=secure,value=on \
>         -drive if=pflash,format=raw,unit=0,readonly=on,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_CODE.fd \
>         -drive if=pflash,format=raw,unit=1,snapshot=on,file=Build/OvmfIa32/DEBUG_GCC5/FV/OVMF_VARS.fd \
>         -drive id=hdd,if=none,format=qcow2,snapshot=on,file=fedora-30-efi-systemd-i686.qcow2 \
>         -device virtio-scsi-pci,id=scsi0 \
>         -device scsi-hd,drive=hdd,bus=scsi0.0,bootindex=1
>
>     (Once you get a login prompt, feel free to interrupt QEMU with Ctrl-C.)
>
>     (3) Test using a 64-bit guest on an Intel host:
>
>     $ build -a IA32 -a X64 -b DEBUG -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 -D SMM_REQUIRE
>
>     $ qemu-system-x86_64 \
>         -cpu host \
>         -machine q35,smm=on,accel=kvm \
>         -m 4096 \
>         -smp 4 \
>         -global driver=cfi.pflash01,property=secure,value=on \
>         -drive if=pflash,format=raw,unit=0,readonly=on,file=Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_CODE.fd \
>         -drive if=pflash,format=raw,unit=1,snapshot=on,file=Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_VARS.fd \
>         -drive id=hdd,if=none,format=qcow2,snapshot=on,file=fedora-31-efi-grub2-x86_64.qcow2 \
>         -device virtio-scsi-pci,id=scsi0 \
>         -device scsi-hd,drive=hdd,bus=scsi0.0,bootindex=1
>
>     Tested on real AMD Hardware
>
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h |  3 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c         |  9 ++++++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   | 19 +++++++++++++++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm    | 20 ++++++++++++++++++--
>  4 files changed, 46 insertions(+), 5 deletions(-)

For this patch:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

However, can you please clarify one bit, with regard to testing:

I understand the regression tests in a 32-bit guest on Intel, and in a
64-bit guests on Intel. I also understand the test on a physical AMD
machine (in other words, you built a physical platform firmware for an
AMD board, flashed it, and tested it.)

However, your notes do not seem to mention test (4) that I requested:

"(4) Test using a 64-bit guest on an AMD host -- just repeat step (3) on
an AMD host."

https://edk2.groups.io/g/devel/message/61344

(alternative link:
<http://mid.mail-archive.com/5f2fd5a9-2107-503a-406b-de08529dcb56@redhat.com>)

That is also important. Test (3) intentionally uses "-cpu host", so the
guest will see a different CPU model when run on an Intel host (3) vs.
on an AMD host (4). I'd like to know that the patched 64-bit SMI entry
code continues working in both guest environments (i.e. when seeing
either an Intel CPU model or an AMD CPU model).

I did not request the same "duality" with test (2) -- as you see, there
I wrote "-cpu coreduo,-nx"; i.e., Intel only. The reason is that "-cpu
coreduo,-nx" is more or less the only 32-bit SMM environment that I
generally know about, and verify -- I simply do not have a 32-bit AMD
CPU model "baseline" to test against. This is why I didn't request "-cpu
host" in (2), and consequently, why I didn't request (2) to be run on
both AMD and Intel hosts.

So... Can you please run test (4) too -- i.e., 64-bit guest on an AMD
host, with "-cpu host"?

Thanks!
Laszlo


  reply	other threads:[~2020-06-19 14:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-18 15:22 [PATCH v2 0/2] AMD procesor MSR_IA32_MISC_ENABLE Kirkendall, Garrett
2020-06-18 15:22 ` [PATCH v2 1/2] UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib Kirkendall, Garrett
2020-06-18 15:22 ` [PATCH v2 2/2] UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on AMD Kirkendall, Garrett
2020-06-19 14:48   ` Laszlo Ersek [this message]
2020-06-19  1:00 ` [PATCH v2 0/2] AMD procesor MSR_IA32_MISC_ENABLE Dong, Eric
2020-06-19 14:35   ` [edk2-devel] " Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=509e8366-8c57-f96d-8684-a548ef148831@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox