Re: [PATCH v7 16/27] KVM: arm64: Factor out core register ID enumeration

Dave P Martin
2019-04-02T09:54:36Z

On Tue, Apr 02, 2019 at 10:32:34AM +0100, Marc Zyngier wrote:
> On Tue, 02 Apr 2019 09:59:19 +0100,
> Dave Martin <Dave.Martin@arm.com> wrote:
> >
> > On Tue, Apr 02, 2019 at 03:41:56AM +0100, Marc Zyngier wrote:

[...]

> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> > > index 086ab0508d69..4f7b26bbf671 100644
> > > --- a/arch/arm64/kvm/guest.c
> > > +++ b/arch/arm64/kvm/guest.c
> > > @@ -604,22 +604,22 @@ int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)

[...]

> > > ret = kvm_arm_copy_fw_reg_indices(vcpu, uindices);
> > > -if (ret)
> > > +if (ret < 0)
> > > return ret;
> > > uindices += kvm_arm_get_fw_num_regs(vcpu);
> > >
> > > ret = copy_timer_indices(vcpu, uindices);
> > > -if (ret)
> > > +if (ret < 0)
> > > return ret;
> > > uindices += NUM_TIMER_REGS;
> >
> > For these two, the interface is not really the same. These don't
> > return the number of registers, so return 0 on success. "< 0" here
> > could be a trap for the future, though the risk looks low.
>
> I realised that, but since they all return a negative value on error,
> I went for a similar error handling.

Sure, this is more potential future maintenance thing -- and anyway not
a big deal.

I agree that your changes look like they work.

> > I can have a go at some rework on top to make this more consistent,
> > but I'd rather not muddy the water further for the moment.
> >
> > Any view on that?
>
> I don't think there is any urgency on that front. For now, I'd like to
> make sure things do not regress. Once we're sure we have something
> stable, we'll be able to make it shiny.

Agreed.

I won't worry about this for now, but we can discuss it later.

Cheers
---Dave
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.