From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [RFC PATCH 12/16]: PVH xen: return PVH features during creation, etc... Date: Thu, 17 Jan 2013 15:36:17 -0800 Message-ID: <20130117153617.203b1aeb@mantra.us.oracle.com> References: <20130111180736.36c662d6@mantra.us.oracle.com> <50F4060C02000078000B5453@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <50F4060C02000078000B5453@nat28.tlf.novell.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: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org On Mon, 14 Jan 2013 12:20:12 +0000 "Jan Beulich" wrote: > >>> On 12.01.13 at 03:07, Mukesh Rathor > >>> wrote: > > @@ -910,6 +915,10 @@ int emulate_forced_invalid_op(struct cpu > > /* Check for forced emulation signature: ud2 ; .ascii "xen". */ > > if ( (rc = copy_from_user(sig, (char *)eip, sizeof(sig))) != > > 0 ) { > > + /* PVH: fixme: hmm... what do we do for PVH? */ > > + if ( is_pvh_vcpu(current) ) > > The fixme and check ought to sit earlier - the copy_from_user() > above isn't valid there. And I don't see how you would validly > get here anyway - you don't need to intercept GP faults to > emulate guest CPUID invocations. Yup, I need raw_copy like later. I guess I went back and forth between supporting XEN_EMULATE_PREFIX or not, since a cpuid can be trapped via vmexit. But we need to support it from user apps, so I need to fix this to raw_copy. > I don't think so - propagate_page_fault() should do the right thing > in that case, if you can validly get here for a PVH guest. Agree, I need to make propgate_page_fault() inject PF into the PVH guest. Working on it now. > > @@ -1566,6 +1586,10 @@ static int guest_io_okay( > > int user_mode = !(v->arch.flags & TF_kernel_mode); > > #define TOGGLE_MODE() if ( user_mode ) toggle_guest_mode(v) > > > > + /* for PVH we check this in vmexit for > > EXIT_REASON_IO_INSTRUCTION */ > > + if (is_pvh_vcpu(v)) > > The why would it get here at all? From, emulate_privileged_op(). I should change the comment to say we don't need to check again, as we check at vmexit. We won't get to emulate_privileged_op() if check fails. Easier to add that in guest_io_okay() than to change every place in emulate_privileged_op() where guest_io_okay() is called and not call it for PVH. > > @@ -2132,7 +2157,8 @@ int emulate_privileged_op(struct cpu_use > > > > case 0xfa: /* CLI */ > > case 0xfb: /* STI */ > > - if ( v->arch.pv_vcpu.iopl < (guest_kernel_mode(v, regs) ? > > 1 : 3) ) > > + if ( !is_pvh_vcpu(v) && > > This ought to be impossible. You mean call to emulate STI/CLI for PVH. Correct. I could just remove it. I went thru looking for places that were using pv_vcpu.iopl. > > @@ -444,6 +444,8 @@ static long register_guest_callback(stru > > long ret = 0; > > struct vcpu *v = current; > > > > + NO_PVH_ASSERT_VCPU(v); > > Either the code is unreachable for a PVH guest (in which case the > assert is likely superfluous, or you need to return an error here > rather than asserting. superfluous, hence it's a debug assert to catch any places I might have missed. I plan to remove them later when PVH is stable. Hope it can stay for a little bit :). thanks, Mukesh