From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [RFC PATCH 0/4] Add support for EPT execute only for nested hypervisors Date: Thu, 23 Jun 2016 13:03:27 +0800 Message-ID: <576B6D9F.9020806@linux.intel.com> References: <1466478746-14153-1-git-send-email-bsd@redhat.com> <576A169B.5090608@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, pbonzini@redhat.com To: Bandan Das Return-path: Received: from mga11.intel.com ([192.55.52.93]:33801 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750971AbcFWFGz (ORCPT ); Thu, 23 Jun 2016 01:06:55 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 06/23/2016 12:34 AM, Bandan Das wrote: > Xiao Guangrong writes: > >> On 06/21/2016 11:12 AM, Bandan Das wrote: >>> I spoke to Paolo about this a while back and thought to post this as >>> RFC while I am thinking of adding some unit tests. >>> >>> Background: ESX refuses to run as L1 if support for EPT execute only isn't >>> found. I am not really sure if it uses it for anything since just advertising >>> the bits seems to work but adding the necessary plumbing seemed like a good idea. >>> >>> Xiao, I took the liberty of adding you based on "git blame" :) >> >> Thank you, Bandan! I really hope all mmu related patches can be CCed to me >> in case if i am too busy to watch patches in kvm mail list. :) > > We all try but it's easy to miss interested people in the cc list :) > >> Some points are missed in this patchset: >> 1) You also need to tech is_present_gpte() about this fact. > > If I understood right, this isn't needed anymore since a check > for rsvd bits follows and is_present_gpte already does pte & 7. is_present_gpte() and FNAME(is_present_gpte) are different paths. Currently, it is safe as is_present_gpte() is only used for 32 bit guests, however, making it consistent can avoid potential bugs in the further. > >> 2) update_permission_bitmask() need to be fixed as it always expects >> that read-access is available (actually, read-access is equal with >> user-access if ept is shadowed). > Do you mean this part of update_permission_bitmask() ? > ... > } else > /* Not really needed: no U/S accesses on ept */ > u = 1; Yes.