RE: [PATCH 2/3] KVM: arm64: Correctly handle zero register in system register accesses

From: Pavel Fedin <p.fedin@samsung.com>
To: 'Marc Zyngier' <marc.zyngier@arm.com>,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org
Subject: RE: [PATCH 2/3] KVM: arm64: Correctly handle zero register in system register accesses
Date: Thu, 03 Dec 2015 14:08:29 +0300	[thread overview]
Message-ID: <00cd01d12dba$f14ac070$d3e04150$@samsung.com> (raw)
In-Reply-To: <56601E36.5070700@arm.com>

 Hello!

> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 87a64e8..a667228 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -102,7 +102,7 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
> >
> >  	BUG_ON(!p->is_write);
> >
> > -	val = *vcpu_reg(vcpu, p->Rt);
> > +	val = *p->val;
> 
> Why does it have to be a pointer? You could just have "val = p->val" if
> you carried the actual value instead of a pointer to the stack variable
> holding that value.

 There's only one concern for pointer approach. Actually, this refactor is based on my vGICv3 live migration API patch set:
http://www.spinics.net/lists/kvm/msg124205.html
http://www.spinics.net/lists/kvm/msg124202.html

 It's simply more convenient to use a pointer for exchange with userspace, see vgic_v3_cpu_regs_access() and callers. I wouldn't
like to refactor the code again. What's your opinion on this?
 And of course i'll fix up the rest.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia