From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v3 03/13] VMX: implement suppress #VE. Date: Tue, 14 Jul 2015 12:18:21 +0100 Message-ID: <55A4EFFD.7030601@eu.citrix.com> References: <1435774177-6345-1-git-send-email-edmund.h.white@intel.com> <1435774177-6345-4-git-send-email-edmund.h.white@intel.com> <559E8CAF020000780008EC87@mail.emea.novell.com> <55A38775020000780008FF35@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A38775020000780008FF35@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Ravi Sahita Cc: Tim Deegan , Wei Liu , Andrew Cooper , Ian Jackson , Edmund H White , "xen-devel@lists.xen.org" , "tlengyel@novetta.com" , Daniel De Graaf List-Id: xen-devel@lists.xenproject.org On 07/13/2015 08:40 AM, Jan Beulich wrote: >>>> On 10.07.15 at 21:30, wrote: >>> From: Jan Beulich [mailto:JBeulich@suse.com] >>> Sent: Thursday, July 09, 2015 6:01 AM >>>>>> On 01.07.15 at 20:09, 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