From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [PATCH v1 2/8]: PVH mmu changes Date: Tue, 25 Sep 2012 17:27:05 -0700 Message-ID: <20120925172705.369e1483@mantra.us.oracle.com> References: <20120921121556.1a0ea8af@mantra.us.oracle.com> <1348488991.3452.69.camel@zakaz.uk.xensource.com> 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: Stefano Stabellini Cc: "Xen-devel@lists.xensource.com" , Ian Campbell , Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org On Mon, 24 Sep 2012 13:24:12 +0100 Stefano Stabellini wrote: > On Mon, 24 Sep 2012, Ian Campbell wrote: > > On Mon, 2012-09-24 at 12:55 +0100, Stefano Stabellini wrote: > > > There are few code style issues on this patch, I suggest you run > > > it through scripts/checkpatch.pl, it should be able to catch all > > > these errors. > > > > It would also be nice to starting having some changelogs entries for > > these patches for the next posting. There's a lot of complex stuff > > > > + return count; > > > > > > Who is going to remove the corresponding mapping from the vma? > > > Also we might be able to replace the flush_tlb_all with a > > > flush_tlb_range. > > > > I'm not convinced that a guest level TLB flush is either necessary > > or sufficient here. What we are doing is removing entries from the > > P2M which means that we need to do the appropriate HAP flush in the > > hypervisor, which must necessarily invalidate any stage 1 mappings > > which this flush might also touch (i.e. the HAP flush must be a > > super set of this flush). > > > > Without the HAP flush in the hypervisor you risk guests being able > > to see old p2m mappings via the TLB entries which is a security > > issue AFAICT. > > Yes, you are right, we need a flush in the hypervisor to flush the > EPT. It could probably live in the implementation of > XENMEM_add_to_physmap. > > This one should be just for the vma mappings, so in the case of > xen_unmap_domain_mfn_range is unnecessary (given that it is > not removing the vma mappings). My head spins looking at INVEPT and INVVPID docs, but doesn't it already happen in ept_set_entry(): if ( needs_sync ) ept_sync_domain(p2m->domain);