[edk2-devel] [PATCH v3 00/11] SEV-ES guest support fixes and cleanup
Lendacky, Thomas
thomas.lendacky at amd.com
Fri Nov 6 17:48:32 UTC 2020
On 11/6/20 10:46 AM, Laszlo Ersek wrote:
> 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) \
I'll make one minor change here, s/WORD/QWORD/.
>> + (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),
Yup, that works, all tests passed.
> then I suggest posting v4 -- Liming should please re-check the udpated
> MdePkg patch, and then we can merge the series.
Ok, I'll submit a v4 and without Liming's Reviewed-by on the first patch.
Thanks,
Tom
>
> Thanks!
> Laszlo
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67098): https://edk2.groups.io/g/devel/message/67098
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list