Re: [edk2-devel] [PATCH RFC v2 04/28] MdePkg: Define the Page State Change VMGEXIT structures

["Brijesh Singh" <brijesh.singh@amd.com>]

On 5/5/21 1:24 PM, Laszlo Ersek wrote:
> On 05/04/21 20:53, Brijesh Singh wrote:
>> On 5/4/21 7:33 AM, Laszlo Ersek wrote:
>>> On 04/30/21 13:51, Brijesh Singh wrote:
>>>> +typedef PACKED struct {
>>>> +  UINT16 CurrentEntry;
>>>> +  UINT16 EndEntry;
>>>> +  UINT32 Rsvd;
>>>> +} SNP_PAGE_STATE_HEADER;
>>> (9) If my understanding of the spec is correct, "EndEntry" has
>>> *inclusive* meaning. That's unusual. Any particular reason for not
>>> making "EndEntry" exclusive (in the spec)?
>> Sometimes guest may need to fill only few entries. The "EndEntry" will
>> give hint to hypervisor that it should stop processing after it reached
>> to the EndEntry.
> I understood the purpose of EndEntry.
>
> My question is why the entry identified by EndEntry *itself* needs to be
> processed.
>
> Put differently, I'm asking why your loop controlling expression is
>
>   Info->Header.CurrentEntry <= Info->Header.EndEntry            [1]
>
> and why not
>
>   Info->Header.CurrentEntry < Info->Header.EndEntry             [2]
>
> in patch #21 ("OvmfPkg/MemEncryptSevLib: Add support to validate system
> RAM").
>
> In case [1], EndEntry is an inclusive limit.
>
> In case [2], EndEntry would be an exclusive limit.
>
> My question is why the spec defines EndEntry with inclusive rather than
> exclusive meaning.
>
> Using inclusive low bounds with exclusive high bounds has mental advantages:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.cs.utexas.edu%2Fusers%2FEWD%2Ftranscriptions%2FEWD08xx%2FEWD831.html&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Ce837bb9a71bd47a8337b08d90ff303ae%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637558358685096067%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=uhtfVW6Q%2FIpHwI%2BfNM3DVur92b2KFB5pjaL1dgZtLC8%3D&amp;reserved=0
>
> Is there a specific reason for which the spec chose the inclusive high
> bound?

Tom and I talked to see if one of us remembers the reason for choosing
the EndEntry inclusive. Unfortunately, we were not able to recall or
trace from where this request came in, but now it is in the spec, and we
have to live with it :(

-Brijesh