Re: [PATCH] OvmfPkg/Sec: Clear the Cache Disable flag in the CR0 register

[Jordan Justen <jordan.l.justen@intel.com>]

On 2019-02-18 05:23:28, Laszlo Ersek wrote:
> generic comment (applies to all NASM usage I guess):
> 
> On 02/18/19 11:10, Jordan Justen wrote:
> 
> > +    mov     eax, cr0
> > +    and     eax, ~(1 << 30)
> > +    mov     cr0, eax
> 
> > +    mov     rax, cr0
> > +    and     eax, ~(1 << 30)
> > +    mov     cr0, rax
> 
> I've read up on the << and ~ operators in the NASM documentation, and I
> think the above build-time calculations of the masks are well-defined
> and correct.
> 
> - bit shifts are always unsigned
> - given bit position 30, ~(1 << 30) will be a value with 32 bits
> - bit-neg simply flips bits (one's complement)
> 
> On the other hand, I find these NASM specifics counter-intuitive. The
> expression ~(1 << 30) looks like valid C, but in C, it means a quite
> different thing.

Can you elaborate? I guess there might be something subtly different,
but for the most part it means the same thing, right?

> I think calculating the mask with "strict dword" somehow (not exactly
> sure how) would make this more readable;

Oh, are you saying that (1 << 30) doesn't necessarily mean we are
operating on a 32-bit value?

> or else the BTR instruction would.

Yeah, I guess this works.

> Opinions? (Again, pertaining to all NASM usage in edk2.)

As always, my opinion is to avoid writing assembly code. :)

We actually had a version that set this just before the decompress in
SecMain.c. Then I noted that we were initializing temp-ram here, so I
moved it, even though the memory init doesn't take a significant
amount of time compared to the decompress. Maybe we should just do
that instead?

-Jordan