Re: [RFC PATCH v2 02/10] OvmfPkg/ResetVector: add memory encryption mask when SEV is enabled

[Brijesh Singh <brijesh.ksingh@gmail.com>]

On Thu, Mar 23, 2017 at 11:16 AM, Laszlo Ersek <lersek@redhat.com> wrote:

> On 03/23/17 16:05, Brijesh Singh wrote:
> > On Wed, Mar 22, 2017 at 3:20 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> >> In the below logic, which branch exactly (to NoSev) will be taken on
> >> Intel processors?
> >>
> >>
> > The below check should branch to NoSev  on Intel processor
> > (please note that 0x8000_001F is new leaf and may not exist on older AMD
> > processor hence we will also branch to NoSev on AMD processor which does
> > not support this leaf)
> >
> > *; Check if we have a valid (0x8000_001F) CPUID leaf
> > **mov       eax, 0x80000000
> > **cpuid
> > **cmp       eax, 0x8000001f
> > **jl        NoSev*
>
> Yes, I figured that maybe this "highest supported cpuid leaf" check
> would catch Intel processors.
>
> But, what prevents a future Intel processor from exposing such a high
> CPUID leaf, for a completely different purpose? Should we not perform
> some kind of vendor-id check? (I'm quite unfamiliar with CPUID, as you
> can tell, I just seem to recall a comment (from Liming perhaps?) for one
> of your MdeModulePkg patches, where he said that directly using a
> software-defined CPUID leaf (?) would not be portable across all CPU
> vendors, or some such.)
>
>

I had similar question in my mind.  Before creating the patch I checked
with
AMD architecture team. The response was, CPUID 0x8000_001F definition
is fixed for x86 architecture. So, if Intel ever decides to report this
CPUID leaf
then it will comply to exact same definition.

I think vendor check is weak, e.g user could change the vendor string
through the qemu property which will break this loop. Because of this, I
wanted
to avoid any kind of vendor string or hypervisor defined CPUID checks
inside the code.

BTW, in my previous patches I was using KVM software defined CPUID leaf and
I
got rid of it because hypervisor's are flexiable to define that CPUID in
whatever
way they want.

-Brijesh