From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Guo <wei.guo.simon@gmail.com>
Subject: Re: [PATCH 22/26] KVM: PPC: Book3S PR: add emulation for trechkpt in
 PR KVM.
Date: Tue, 30 Jan 2018 11:13:49 +0800
Message-ID: <20180130031348.GH3261@simonLocalRHEL7.x64>
References: <1515665499-31710-1-git-send-email-wei.guo.simon@gmail.com>
 <1515665499-31710-23-git-send-email-wei.guo.simon@gmail.com>
 <20180123093644.GN3924@fergus.ozlabs.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
        kvm-ppc@vger.kernel.org
To: Paul Mackerras <paulus@ozlabs.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pg0-f66.google.com ([74.125.83.66]:46914 "EHLO
        mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752200AbeA3DNx (ORCPT <rfc822;kvm@vger.kernel.org>);
        Mon, 29 Jan 2018 22:13:53 -0500
Content-Disposition: inline
In-Reply-To: <20180123093644.GN3924@fergus.ozlabs.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Hi Paul,
On Tue, Jan 23, 2018 at 08:36:44PM +1100, Paul Mackerras wrote:
> On Thu, Jan 11, 2018 at 06:11:35PM +0800, wei.guo.simon@gmail.com wrote:
> > From: Simon Guo <wei.guo.simon@gmail.com>
> > 
> > This patch adds host emulation when guest PR KVM executes "trechkpt.",
> > which is a privileged instruction and will trap into host.
> > 
> > We firstly copy vcpu ongoing content into vcpu tm checkpoint
> > content, then perform kvmppc_restore_tm_pr() to do trechkpt.
> > with updated vcpu tm checkpoint vals.
> > 
> > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
> 
> [snip]
> 
> > +static void kvmppc_emulate_trchkpt(struct kvm_vcpu *vcpu)
> > +{
> > +	unsigned long guest_msr = kvmppc_get_msr(vcpu);
> > +
> > +	preempt_disable();
> > +	vcpu->arch.save_msr_tm = MSR_TS_S;
> > +	vcpu->arch.save_msr_tm &= ~(MSR_FP | MSR_VEC | MSR_VSX);
> 
> This looks odd, since you are clearing bits when you have just set
> save_msr_tm to a constant value that doesn't have these bits set.
> This could be taken as a sign that the previous line has a bug and you
> meant "|=" or something similar instead of "=".  I think you probably
> did mean "=", in which case you should remove the line clearing
> FP/VEC/VSX.

I will rework and remove "save_msr_tm" from the code.

Thanks,
- Simon