Re: [PATCH v3 03/13] VMX: implement suppress #VE.

[George Dunlap <george.dunlap@eu.citrix.com>]

On 07/13/2015 08:40 AM, Jan Beulich wrote:
>>>> On 10.07.15 at 21:30, <ravi.sahita@intel.com> wrote:
>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>> Sent: Thursday, July 09, 2015 6:01 AM
>>>>>> On 01.07.15 at 20:09, <edmund.h.white@intel.com> wrote:
>>>> @@ -232,6 +235,15 @@ static int ept_set_middle_entry(struct p2m_domain
>>>> @@ -1134,6 +1151,13 @@ int ept_p2m_init(struct p2m_domain *p2m)
>>>>          p2m->flush_hardware_cached_dirty = ept_flush_pml_buffers;
>>>>      }
>>>>
>>>> +    table =
>>>> + map_domain_page(pagetable_get_pfn(p2m_get_pagetable(p2m)));
>>>> +
>>>> +    for ( i = 0; i < EPT_PAGETABLE_ENTRIES; i++ )
>>>> +        table[i].suppress_ve = 1;
>>>> +
>>>> +    unmap_domain_page(table);
>>>
>>> ... why is this needed? Bit 63 is documented to be ignored in PML4Es (just 
>> like
>>> in all other intermediate page tables).
>>
>> Valid point - this has no negative side-effects per se so we didn't change 
>> this.
> 
> Taking "we didn't change this" to refer to v3 -> v4, I still think this
> should be dropped if it isn't needed. There can only be confusion
> arising from code having no purpose.

Just want to call out the general principle Jan refers to here: Nobody
can remember everything about all the details of how the code and the
hardware works; there's just too much to keep in your head all at one
time.  And in any case, people who are not maintainers need to be able
to understand the code to modify it.

The result is that we naturally use the code itself to remind us, or
give us a hint, what the rest of the code or what the hardware does; if
we see a check for NULL, we tend to assume that the pointer may actually
be NULL; if we see a flag being passed, we tend to assume that the flag
will have an effect.

The general principle for making the code easier to grasp, and for
reducing the cognitive load on people trying to understand and modify
it, is to enhance this.  Write code which implies the truth about other
bits of the codebase or the hardware; avoid writing code which will
mislead someone into thinking something false about the other bits of
the codebase or the hardware.

 -George