From mboxrd@z Thu Jan  1 00:00:00 1970
From: Rusty Russell <rusty@rustcorp.com.au>
Subject: Re: [RFC 0/5] Making KVM_GET_ONE_REG/KVM_SET_ONE_REG generic.
Date: Wed, 05 Sep 2012 16:18:27 +0930
Message-ID: <87ipbtj77o.fsf@rustcorp.com.au>
References: <877gsia8rm.fsf@rustcorp.com.au> <87627y3p1r.fsf@rustcorp.com.au> <CAFEAcA-PRvmuUzibqqd_eNVh_NDpBnFAJAO6mnU-t_ugK7dK2A@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Rusty Russell <rusty.russell@linaro.org>,
	Avi Kivity <avi@redhat.com>,
	Christoffer Dall <c.dall@virtualopensystems.com>,
	Alexander Graf <agraf@suse.de>, kvmarm@lists.cs.columbia.edu,
	kvm-devel <kvm@vger.kernel.org>
To: Peter Maydell <peter.maydell@linaro.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from ozlabs.org ([203.10.76.45]:38289 "EHLO ozlabs.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1756736Ab2IEIVw (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 5 Sep 2012 04:21:52 -0400
In-Reply-To: <CAFEAcA-PRvmuUzibqqd_eNVh_NDpBnFAJAO6mnU-t_ugK7dK2A@mail.gmail.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Peter Maydell <peter.maydell@linaro.org> writes:
> On 1 September 2012 13:28, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Rusty Russell (8):
>>       KVM: ARM: Fix walk_msrs()
>>       KVM: Move KVM_SET_ONE_REG/KVM_GET_ONE_REG to generic code.
>>       KVM: Add KVM_REG_SIZE() helper.
>>       KVM: ARM: use KVM_SET_ONE_REG/KVM_GET_ONE_REG.
>>       KVM: Add KVM_VCPU_GET_REG_LIST.
>>       KVM: ARM: Use KVM_VCPU_GET_REG_LIST.
>>       KVM: ARM: Access all registers via KVM_GET_ONE_REG/KVM_SET_ONE_REG.
>>       KVM ARM: Update api.txt
>
> So I was thinking about this, and I remembered that the SET_ONE_REG/
> GET_ONE_REG API has userspace pass a pointer to the variable the
> kernel should read/write (unlike the _MSR x86 ioctls, where the
> actual data value is sent back and forth in the struct). Further,
> the kernel only writes a data value of the size of the register
> (rather than always reading/writing a uint64_t).
>
> This is a problem because it means userspace needs to know the
> size of each register, and the kernel doesn't provide any way
> to determine the size. This defeats the idea that userspace should
> be able to migrate kernel register state without having to know
> the semantics of all the registers involved.

It's there.  There are bits in the id which indicate the size:

#define KVM_REG_SIZE_SHIFT	52
#define KVM_REG_SIZE_MASK	0x00f0000000000000ULL
#define KVM_REG_SIZE_U8		0x0000000000000000ULL
#define KVM_REG_SIZE_U16	0x0010000000000000ULL
#define KVM_REG_SIZE_U32	0x0020000000000000ULL
#define KVM_REG_SIZE_U64	0x0030000000000000ULL
#define KVM_REG_SIZE_U128	0x0040000000000000ULL
#define KVM_REG_SIZE_U256	0x0050000000000000ULL
#define KVM_REG_SIZE_U512	0x0060000000000000ULL
#define KVM_REG_SIZE_U1024	0x0070000000000000ULL

And my patches added a helper:

#define KVM_REG_SIZE(id)						\
	(1U << (((id) & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT))

> I could live with "always read/write 64 bits". I definitely don't
> want to have to deal with matching up register widths to accesses
> in userspace, please.

I changed my mind about the old scheme when I realized we have to deal
with 128-bit FPU registers.

Cheers,
Rusty.