Re: 回复: 回复: [PATCH v2 01/11] MdePkg, OvmfPkg: Clean up GHCB field offsets and save area

["Laszlo Ersek" <lersek@redhat.com>]

On 10/20/20 15:10, Tom Lendacky wrote:
> On 10/20/20 3:31 AM, Laszlo Ersek wrote:
>> On 10/20/20 03:08, gaoliming wrote:
>>> Laszlo and Tom:
>>>
>>>> -----邮件原件-----
>>>> 发件人: Laszlo Ersek <lersek@redhat.com>
>>>> 发送时间: 2020年10月20日 4:42
>>>> 收件人: Tom Lendacky <thomas.lendacky@amd.com>; gaoliming
>>>> <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
>>>> 抄送: 'Brijesh Singh' <brijesh.singh@amd.com>; 'Michael D Kinney'
>>>> <michael.d.kinney@intel.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>;
>>>> 'Jordan Justen' <jordan.l.justen@intel.com>; 'Ard Biesheuvel'
>>>> <ard.biesheuvel@arm.com>
>>>> 主题: Re: 回复: [PATCH v2 01/11] MdePkg, OvmfPkg: Clean up GHCB field
>>>> offsets and save area
>>>>
>>>> On 10/19/20 14:50, Tom Lendacky wrote:
>>>>> On 10/18/20 8:41 PM, gaoliming wrote:
>>>>
>>>>>> Please separate the patch for the change in OvmfPkg.
>>>>>> One commit can't cross the different packages.
>>>>>> I understand this is the incompatible change. But, we still need to follow
>>>>>> this rule.
>>>>
>>>> I disagree.
>>>>
>>>> We should do whatever we can for avoiding cross-package patches, but in
>>>> some cases, they are unavoidable. This is one of those cases.
>>>
>>> I suggest to define enum GHCB_QWORD_OFFSET, then use typedef GHCB_QWORD_OFFSET GHCB_REGISTER; in Ghcb.h
>>> The comments can be added here to describe it is for compatibility. The old one is not recommend.
>>>
>>> Then, the change in MdePkg is compatible. Next patch is to update OvmfPkg VmgExit to consume GHCB_QWORD_OFFSET. 
>>
>> Ah, I totally missed that we could use typedef to bridge the gap. That
>> indeed allows us to do the rename in three steps (only for the type
>> name, the enum constant identifiers can stay the same). After the
>> rename, the enum constant values can be cleaned up in a separate (4th)
>> patch.
> 
> It seems like a lot of churn for just renaming.

Yes, it is. (I guess it helps downstreams that only want to cherry-pick
patches from specific packages.)

> If there are no
> objections, I'll keep the GHCB_REGISTER name. The important piece is the
> change from hardcoding the offset values to using a calculated value.

If the GHCB_REGISTER type name works for you, it certainly works for me :)

Thanks
Laszlo