[edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup

[Laszlo Ersek]

On 11/06/20 07:29, Tom Lendacky wrote:
> On 11/5/20 8:34 AM, Tom Lendacky wrote:
>> On 11/4/20 9:29 PM, Laszlo Ersek wrote:

>>> I've submitted PR#1086 [...] but CI seems slower than usual today,
>>> and I really need some sleep, so I won't wait for CI. Tom, if the PR
>>> succeeds, please close TianoCore#3008, noting the commit range, and
>>> please also follow up in this thread with the commit range.
>>
>> Thanks, Laszlo!
>>
>> It looks like it failed because it doesn't like the use of the
>> "sizeof (UINT64)". I suppose I can change that to just hard code a
>> value of 8. Let me know what you think.
>
> I did verify that changing the "sizeof (UINT64)" to "8" in the first
> patch results in all CI tests passing. I can re-submit with that, if
> you feel that is the best course of action.


So this is the error (from ECC), if I understand correctly:

> 2020-11-05T03:25:05.2175761Z ERROR - EFI coding style error
> 2020-11-05T03:25:05.2176262Z ERROR - *Error code: 8005
> 2020-11-05T03:25:05.2177026Z ERROR - *Variable name does not follow the rules: 1. First character should be upper case 2. Must contain lower case characters 3. No white space characters 4. Global variable name must start with a 'g'
> 2020-11-05T03:25:05.2177773Z ERROR - *file: //home/vsts/work/1/s/MdePkg/Include/Register/Amd/Ghcb.h
> 2020-11-05T03:25:05.2180810Z ERROR - *Line number: 114
> 2020-11-05T03:25:05.2181426Z ERROR - *Member variable [GHCB_REGISTER.(UINT64)] NOT follow naming convention.

for such lines as:

>   GhcbCpl          = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),

I think this is a bug in CheckMemberVariableFormat(), in
"BaseTools/Source/Python/Ecc/c.py".

I don't exactly understand how that ECC method functions. However,
squashing the following update into the first (MdePkg) patch of this
series seems to work:

> diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
> index 1fce64d5b7cd..3814a79e0619 100644
> --- a/MdePkg/Include/Register/Amd/Ghcb.h
> +++ b/MdePkg/Include/Register/Amd/Ghcb.h
> @@ -111,17 +111,20 @@ typedef PACKED struct {
>    UINT32                 GhcbUsage;
>  } GHCB;
>
> +#define GHCB_SAVE_AREA_WORD_OFFSET(RegisterField) \
> +  (OFFSET_OF (GHCB, SaveArea.RegisterField) / sizeof (UINT64))
> +
>  typedef enum {
> -  GhcbCpl          = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),
> -  GhcbRax          = OFFSET_OF (GHCB, SaveArea.Rax) / sizeof (UINT64),
> -  GhcbRbx          = OFFSET_OF (GHCB, SaveArea.Rbx) / sizeof (UINT64),
> -  GhcbRcx          = OFFSET_OF (GHCB, SaveArea.Rcx) / sizeof (UINT64),
> -  GhcbRdx          = OFFSET_OF (GHCB, SaveArea.Rdx) / sizeof (UINT64),
> -  GhcbXCr0         = OFFSET_OF (GHCB, SaveArea.XCr0) / sizeof (UINT64),
> -  GhcbSwExitCode   = OFFSET_OF (GHCB, SaveArea.SwExitCode) / sizeof (UINT64),
> -  GhcbSwExitInfo1  = OFFSET_OF (GHCB, SaveArea.SwExitInfo1) / sizeof (UINT64),
> -  GhcbSwExitInfo2  = OFFSET_OF (GHCB, SaveArea.SwExitInfo2) / sizeof (UINT64),
> -  GhcbSwScratch    = OFFSET_OF (GHCB, SaveArea.SwScratch) / sizeof (UINT64),
> +  GhcbCpl          = GHCB_SAVE_AREA_WORD_OFFSET (Cpl),
> +  GhcbRax          = GHCB_SAVE_AREA_WORD_OFFSET (Rax),
> +  GhcbRbx          = GHCB_SAVE_AREA_WORD_OFFSET (Rbx),
> +  GhcbRcx          = GHCB_SAVE_AREA_WORD_OFFSET (Rcx),
> +  GhcbRdx          = GHCB_SAVE_AREA_WORD_OFFSET (Rdx),
> +  GhcbXCr0         = GHCB_SAVE_AREA_WORD_OFFSET (XCr0),
> +  GhcbSwExitCode   = GHCB_SAVE_AREA_WORD_OFFSET (SwExitCode),
> +  GhcbSwExitInfo1  = GHCB_SAVE_AREA_WORD_OFFSET (SwExitInfo1),
> +  GhcbSwExitInfo2  = GHCB_SAVE_AREA_WORD_OFFSET (SwExitInfo2),
> +  GhcbSwScratch    = GHCB_SAVE_AREA_WORD_OFFSET (SwScratch),
>  } GHCB_REGISTER;
>
>  typedef union {

I build-tested this update with gcc, and also verified that it passes
ECC, by running CI locally. (I reproduced the bogus ECC error message
locally at first, of course, to see if the update makes a difference.)

> PROGRESS - --Running MdePkg: EccCheck Test NO-TARGET --
> PROGRESS - --->Test Success: EccCheck Test NO-TARGET

If it works for you as well (check with a personal build PR perhaps),
then I suggest posting v4 -- Liming should please re-check the udpated
MdePkg patch, and then we can merge the series.

Thanks!
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67097): https://edk2.groups.io/g/devel/message/67097
Mute This Topic: https://groups.io/mt/77888107/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-