From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for-4.5 v11 5/9] xen/arm: Allow hypervisor access to mem_access protected pages Date: Mon, 29 Sep 2014 16:07:28 +0100 Message-ID: <542975B0.2070805@linaro.org> References: <1411990609-22374-1-git-send-email-tklengyel@sec.in.tum.de> <1411990609-22374-6-git-send-email-tklengyel@sec.in.tum.de> <542968E6.2040408@linaro.org> <542971AF.1080207@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas K Lengyel Cc: wei.liu2@citrix.com, Ian Campbell , Tim Deegan , "xen-devel@lists.xen.org" , Stefano Stabellini , Daniel De Graaf , Tamas K Lengyel List-Id: xen-devel@lists.xenproject.org On 09/29/2014 03:57 PM, Tamas K Lengyel wrote: > > > On Mon, Sep 29, 2014 at 4:50 PM, Julien Grall > wrote: > > On 09/29/2014 03:44 PM, Tamas K Lengyel wrote: > > > + if ( rc < 0 ) > > > + return rc; > > > + > > > + /* > > > + * We do this first as this is faster in the default case when no > > > + * permission is set on the page. > > > + */ > > > + rc = p2m_get_mem_access(current->domain, paddr_to_pfn(ipa), &xma); > > > + if ( rc < 0 ) > > > + return rc; > > > + > > > + /* Let's check if mem_access limited the access. */ > > > + switch ( xma ) > > > + { > > > + default: > > > + case XENMEM_access_rwx: > > > > access_rwx is used to say there is no permission, right? If so, why > > don't you continue to check permission? > > > > > > So things are backward here. There has already been a MMU fault > > (get_page_from_gva failed) and we are trying to determine the cause of > > that fault. If the mem_access permission is rwx or rw, that means it had > > nothing to do with it so we go back to the original path. > > This is very confusing. As we should never go there, I would add an > ASSERT to catch buggy implementation. > > > Not sure what you mean. That's the best approach to really avoid adding > any overhead to those cases where mem_access is not in use. We don't > even have to check if its in use if get_page_from_gva just works (which > I assume is the vast majority of the cases). hmmmm... I'm wrong on this part from the beginning. Sorry for the noise. Regards, -- Julien Grall