From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tim Deegan Subject: Re: [PATCH] nestedhvm: fix write access fault on ro mapping Date: Thu, 2 Aug 2012 11:45:24 +0100 Message-ID: <20120802104524.GA11437@ocelot.phlegethon.org> References: <5008166B.6010603@amd.com> <20120726182111.GB4135@ocelot.phlegethon.org> <5012822E.2030603@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <5012822E.2030603@amd.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: Christoph Egger Cc: "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org At 13:57 +0200 on 27 Jul (1343397454), Christoph Egger wrote: > >> @@ -1291,6 +1291,8 @@ int hvm_hap_nested_page_fault(unsigned l > >> if ( !handle_mmio() ) > >> hvm_inject_hw_exception(TRAP_gp_fault, 0); > >> return 1; > >> + case NESTEDHVM_PAGEFAULT_READONLY: > >> + break; > > > > Don't we have to translate the faulting PA into an L1 address before > > letting the rest of this fault handler run? It explicitly operates on > > the hostp2m. > > > > If we do that, we should probably do it for NESTEDHVM_PAGEFAULT_ERROR, > > rather than special-casing READONLY. That way any other > > automatically-fixed types (like the p2m_access magic) will be covered > > too. > > How do you differentiate if the error happened from walking l1 npt or > host npt ? > In the first case it isn't possible to provide l1 address. It must be _possible_; after all we managed to detect the error. :) In any case it's definitely wrong to carry on with this handler with the wrong address in hand. So I wonder why this patch actually works for you. Does replacing the 'break' above with 'return 1' also fix the problem? In the short term, do you only care about pages that are read-only for log-dirty tracking? For the L1 walk, that should be handled by the PT walker's own calls to paging_mark_dirty(), and the nested-p2m handler could potentially take care of the other case by calling paging_mark_dirty() (for writes!) before calling nestedhap_walk_L0_p2m(). Tim.