From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH] KVM: PPC: Book3S: Add support for H_IPOLL and H_XIRR_X in XICS emulation Date: Wed, 29 May 2013 18:38:23 -0500 Message-ID: <1369870703.18630.49@snotra> References: <1369788078.3928.28.camel@pasglop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Paul Mackerras , , , , Alexander Graf , Gleb Natapov , Marcelo Tosatti To: Benjamin Herrenschmidt Return-path: Received: from co9ehsobe001.messaging.microsoft.com ([207.46.163.24]:32682 "EHLO co9outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S967108Ab3E2Xid convert rfc822-to-8bit (ORCPT ); Wed, 29 May 2013 19:38:33 -0400 In-Reply-To: <1369788078.3928.28.camel@pasglop> (from benh@kernel.crashing.org on Tue May 28 19:41:18 2013) Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On 05/28/2013 07:41:18 PM, Benjamin Herrenschmidt wrote: > On Tue, 2013-05-28 at 12:41 -0500, Scott Wood wrote: > > > I believe Alex is staying far away from e-mail on his vacation. > He's > > asked me to fill in for him while he's gone. > > > > The patch itself seems reasonable (though I don't know much about > XICS, > > and do have one question...), but I'll leave it up to > Gleb/Marcelo/Ben > > if it should go in for 3.10 and via which tree. I understand the > > desire to not have an incomplete ABI in a released version, but > Linus > > is already grumbling about how much went into rc3, and you say the > > hcalls aren't currently used... Are they likely to be used in any > > timeframe in which we'd reasonably care about 3.10? > > Yes. I'd like to have them in. Their implementation is actually fairly > trivial and they cannot be emulated by qemu if the rest of the XICS is > in the kernel, so it's a problem. OK. Does it make more sense for you to take it as Paul suggested, or for Gleb or Marcelo to pick it up directly? > > > + /* These requests don't have real-mode implementations at > > > present */ > > > + switch (req) { > > > + case H_XIRR_X: > > > + res = kvmppc_h_xirr(vcpu); > > > + kvmppc_set_gpr(vcpu, 4, res); > > > + kvmppc_set_gpr(vcpu, 5, get_tb()); > > > + return rc; > > > + case H_IPOLL: > > > + rc = kvmppc_h_ipoll(vcpu, kvmppc_get_gpr(vcpu, 4)); > > > + return rc; > > > + } > > > + > > > /* Check for real mode returning too hard */ > > > if (xics->real_mode) > > > return kvmppc_xics_rm_complete(vcpu, req); > > > > Could you explain what's going on here relative to > > kvmppc_xics_rm_complete()? What does "returning too hard" mean, and > > why must rm_action not be checked for these hcalls? > > This is related to how we handle some hcalls in real mode as a fast > path. The real-mode stuff cannot handle cases that require for > example a > re-emit of the interrupt, a reject, etc... so in some cases, it > returns > H_TOO_HARD which causes KVM to exit and try to handle the hcall again > in > kernel virtual mode. > > When doing so as the result of a XICS hcall, it sets a bit mask of > "tasks" to handle in virtual mode (because it will have already > partially done the operation, it cannot just re-play the whole hcall). > > So when real-mode is supported we must not just call the normal > virtual > mode version of the hcalls, we instead go to kvmppc_xics_rm_complete() > to handle those "tasks". > > However, for those 2 "missing" hcalls, we have no real mode > implementation at all (we didn't bother, we will do that later if > needed, it's purely a performance issue). So we need to fully handle > them in virtual mode, and we know there will be no "tasks" to handle > in > rm_complete. Then rm_action should always be 0 for these hcalls, right? So there's no correctness reason to keep the hcalls in separate switch statements. You shave off a few cycles checking rm_action, at the cost of needing to change kvmppc_xics_hcall() if a real-mode version of these hcalls is ever done. -Scott