From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Wed, 10 Jul 2013 00:12:31 +0000 Subject: Re: [PATCH 2/2] KVM: PPC: Book3E: Get vcpu's last instruction for emulation Message-Id: <1373415151.8183.213@snotra> List-Id: References: In-Reply-To: (from agraf@suse.de on Tue Jul 9 16:45:10 2013) MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Alexander Graf Cc: Mihai Caraman , kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, linuxppc-dev@lists.ozlabs.org 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. -Scott From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from am1outboundpool.messaging.microsoft.com (am1ehsobe003.messaging.microsoft.com [213.199.154.206]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (Client CN "mail.global.frontbridge.com", Issuer "MSIT Machine Auth CA 2" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 892DB2C007E for ; Wed, 10 Jul 2013 10:12:43 +1000 (EST) Date: Tue, 9 Jul 2013 19:12:31 -0500 From: Scott Wood Subject: Re: [PATCH 2/2] KVM: PPC: Book3E: Get vcpu's last instruction for emulation To: Alexander Graf In-Reply-To: (from agraf@suse.de on Tue Jul 9 16:45:10 2013) Message-ID: <1373415151.8183.213@snotra> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Cc: Mihai Caraman , linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 07/09/2013 04:45:10 PM, Alexander Graf wrote: >=20 > On 28.06.2013, at 11:20, Mihai Caraman wrote: >=20 > > + /* Get page size */ > > + if (MAS0_GET_TLBSEL(mfspr(SPRN_MAS0)) =3D=3D 0) > > + psize_shift =3D PAGE_SHIFT; > > + else > > + psize_shift =3D MAS1_GET_TSIZE(mas1) + 10; > > + > > + mas7_mas3 =3D (((u64) mfspr(SPRN_MAS7)) << 32) | > > + mfspr(SPRN_MAS3); > > + addr =3D (mas7_mas3 & (~0ULL << psize_shift)) | > > + (geaddr & ((1ULL << psize_shift) - 1ULL)); > > + > > + /* Map a page and get guest's instruction */ > > + page =3D pfn_to_page(addr >> PAGE_SHIFT); >=20 > While looking at this I just realized that you're missing a check =20 > here. What if our IP is in some PCI BAR? Or can't we execute from =20 > those? We at least need to check pfn_valid() first. That'll just keep us from =20 accessing a bad pointer in the host kernel, though -- it won't make the =20 emulation actually work. If we need that, we'll probably need to =20 create a temporary TLB entry manually. -Scott= 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: Tue, 9 Jul 2013 19:12:31 -0500 Message-ID: <1373415151.8183.213@snotra> References: 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: In-Reply-To: (from agraf@suse.de on Tue Jul 9 16:45:10 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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. -Scott