From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 2/2] KVM: PPC: Book3E: Get vcpu's last instruction for emulation Date: Wed, 10 Jul 2013 13:37:06 -0500 Message-ID: <1373481426.8183.219@snotra> References: <27E6FCB1-322F-410D-A87C-D5410C7374E7@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Mihai Caraman , , , To: Alexander Graf Return-path: Received: from co9ehsobe001.messaging.microsoft.com ([207.46.163.24]:22686 "EHLO co9outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754825Ab3GJShM convert rfc822-to-8bit (ORCPT ); Wed, 10 Jul 2013 14:37:12 -0400 In-Reply-To: <27E6FCB1-322F-410D-A87C-D5410C7374E7@suse.de> (from agraf@suse.de on Wed Jul 10 05:18:10 2013) Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On 07/10/2013 05:18:10 AM, Alexander Graf wrote: > > On 10.07.2013, at 02:12, Scott Wood wrote: > > > On 07/09/2013 04:45:10 PM, Alexander Graf wrote: > >> On 28.06.2013, at 11:20, Mihai Caraman wrote: > >> > + /* Get page size */ > >> > + if (MAS0_GET_TLBSEL(mfspr(SPRN_MAS0)) == 0) > >> > + psize_shift = PAGE_SHIFT; > >> > + else > >> > + psize_shift = MAS1_GET_TSIZE(mas1) + 10; > >> > + > >> > + mas7_mas3 = (((u64) mfspr(SPRN_MAS7)) << 32) | > >> > + mfspr(SPRN_MAS3); > >> > + addr = (mas7_mas3 & (~0ULL << psize_shift)) | > >> > + (geaddr & ((1ULL << psize_shift) - 1ULL)); > >> > + > >> > + /* Map a page and get guest's instruction */ > >> > + page = pfn_to_page(addr >> PAGE_SHIFT); > >> While looking at this I just realized that you're missing a check > here. What if our IP is in some PCI BAR? Or can't we execute from > those? > > > > We at least need to check pfn_valid() first. That'll just keep us > from accessing a bad pointer in the host kernel, though -- it won't > make the emulation actually work. If we need that, we'll probably > need to create a temporary TLB entry manually. > > ioremap()? That's a bit heavy... also we'd need to deal with cacheability. This code is already engaged in directly creating TLB entries, so it doesn't seem like much of a stretch to create one for this. It should be faster than ioremap() or kmap_atomic(). The one complication is allocating the virtual address space, but maybe we could just use the page that kmap_atomic would have used? Of course, if we want to handle execution from other than normal kernel memory, we'll need to make sure that the virtual address space is allocated even when highmem is not present (e.g. 64-bit). > However, if we were walking the guest TLB cache instead we would get > a guest physical address which we can always resolve to a host > virtual address. > > I'm not sure how important that whole use case is though. Maybe we > should just error out to the guest for now. It's not that important, now that we are using hugetlb rather than directly mapping a large hunk of reserved memory. It would be nice to handle it though, if we can do without too much hassle. And I think manually creating a TLB entry could be faster than kmap_atomic(), or searching the guest TLB and then doing a reverse memslot lookup. -Scott