diff mbox

KVM: PPC: Add generic single register ioctls

Message ID 1325823314-12758-1-git-send-email-agraf@suse.de
State New, archived
Headers show

Commit Message

Alexander Graf Jan. 6, 2012, 4:15 a.m. UTC 
Right now we transfer a static struct every time we want to get or set
registers. Unfortunately, over time we realize that there are more of
these than we thought of before and the extensibility and flexibility of
transferring a full struct every time is limited.

So this is a new approach to the problem. With these new ioctls, we can
get and set a single register that is identified by an ID. This allows for
very precise and limited transmittal of data. When we later realize that
it's a better idea to shove over multiple registers at once, we can reuse
most of the infrastructure and simply implement a GET_MANY_REGS / SET_MANY_REGS
interface.

The only downpoint I see to this one is that it needs to pad to 1024 bits
(hardware is already on 512 bit registers, so I wanted to leave some room)
which is slightly too much for transmitting only 64 bits. But if that's all
the tradeoff we have to do for getting an extensible interface, I'd say go
for it nevertheless.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - rename KVM_ONE_REG to KVM_REG
  - change semantics to include size in constant
    and a user pointer to write/read to/from
  - update documentation respectively

v2 -> v3:

  - make GET_ONE_REG ioctl write-only

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Scott Wood Jan. 6, 2012, 7:32 p.m. UTC | #1 
On 01/05/2012 10:15 PM, Alexander Graf wrote:
> Right now we transfer a static struct every time we want to get or set
> registers. Unfortunately, over time we realize that there are more of
> these than we thought of before and the extensibility and flexibility of
> transferring a full struct every time is limited.
> 
> So this is a new approach to the problem. With these new ioctls, we can
> get and set a single register that is identified by an ID. This allows for
> very precise and limited transmittal of data. When we later realize that
> it's a better idea to shove over multiple registers at once, we can reuse
> most of the infrastructure and simply implement a GET_MANY_REGS / SET_MANY_REGS
> interface.
> 
> The only downpoint I see to this one is that it needs to pad to 1024 bits
> (hardware is already on 512 bit registers, so I wanted to leave some room)
> which is slightly too much for transmitting only 64 bits. But if that's all
> the tradeoff we have to do for getting an extensible interface, I'd say go
> for it nevertheless.

The comment about padding is no longer relevant.

> +/*
> + * Architecture specific registers are to be defined in arch headers and
> + * ORed with the arch identifier.
> + */
> +#define KVM_REG_PPC		0x1000000000000000ULL
> +#define KVM_REG_X86		0x2000000000000000ULL
> +#define KVM_REG_IA64		0x3000000000000000ULL
> +#define KVM_REG_ARM		0x4000000000000000ULL
> +#define KVM_REG_S390		0x5000000000000000ULL
> +
> +#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

Why not just encode directly as number of bytes?

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf Jan. 7, 2012, 12:52 a.m. UTC | #2 
On 06.01.2012, at 20:32, Scott Wood <scottwood@freescale.com> wrote:

> On 01/05/2012 10:15 PM, Alexander Graf wrote:
>> Right now we transfer a static struct every time we want to get or set
>> registers. Unfortunately, over time we realize that there are more of
>> these than we thought of before and the extensibility and flexibility of
>> transferring a full struct every time is limited.
>> 
>> So this is a new approach to the problem. With these new ioctls, we can
>> get and set a single register that is identified by an ID. This allows for
>> very precise and limited transmittal of data. When we later realize that
>> it's a better idea to shove over multiple registers at once, we can reuse
>> most of the infrastructure and simply implement a GET_MANY_REGS / SET_MANY_REGS
>> interface.
>> 
>> The only downpoint I see to this one is that it needs to pad to 1024 bits
>> (hardware is already on 512 bit registers, so I wanted to leave some room)
>> which is slightly too much for transmitting only 64 bits. But if that's all
>> the tradeoff we have to do for getting an extensible interface, I'd say go
>> for it nevertheless.
> 
> The comment about padding is no longer relevant.
> 
>> +/*
>> + * Architecture specific registers are to be defined in arch headers and
>> + * ORed with the arch identifier.
>> + */
>> +#define KVM_REG_PPC        0x1000000000000000ULL
>> +#define KVM_REG_X86        0x2000000000000000ULL
>> +#define KVM_REG_IA64        0x3000000000000000ULL
>> +#define KVM_REG_ARM        0x4000000000000000ULL
>> +#define KVM_REG_S390        0x5000000000000000ULL
>> +
>> +#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
> 
> Why not just encode directly as number of bytes?

Because this is 1 << n bytes :)

Alex

> 
> -Scott
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Jan. 9, 2012, 7:07 p.m. UTC | #3 
On 01/06/2012 06:52 PM, Alexander Graf wrote:
> 
> 
> On 06.01.2012, at 20:32, Scott Wood <scottwood@freescale.com> wrote:
> 
>> On 01/05/2012 10:15 PM, Alexander Graf wrote:
>>
>>> +/*
>>> + * Architecture specific registers are to be defined in arch headers and
>>> + * ORed with the arch identifier.
>>> + */
>>> +#define KVM_REG_PPC        0x1000000000000000ULL
>>> +#define KVM_REG_X86        0x2000000000000000ULL
>>> +#define KVM_REG_IA64        0x3000000000000000ULL
>>> +#define KVM_REG_ARM        0x4000000000000000ULL
>>> +#define KVM_REG_S390        0x5000000000000000ULL
>>> +
>>> +#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
>>
>> Why not just encode directly as number of bytes?
> 
> Because this is 1 << n bytes :)

Some registers may not be a power-of-2 number of bytes (e.g. x86 segment
descriptors), and we've got plenty of space to spare in the id.

It's probably not worth another respin, though -- we can just document
right/left justification in the event that we have such a register.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf Jan. 9, 2012, 8:11 p.m. UTC | #4 
Am 09.01.2012 um 20:07 schrieb Scott Wood <scottwood@freescale.com>:

> On 01/06/2012 06:52 PM, Alexander Graf wrote:
>> 
>> 
>> On 06.01.2012, at 20:32, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On 01/05/2012 10:15 PM, Alexander Graf wrote:
>>> 
>>>> +/*
>>>> + * Architecture specific registers are to be defined in arch headers and
>>>> + * ORed with the arch identifier.
>>>> + */
>>>> +#define KVM_REG_PPC        0x1000000000000000ULL
>>>> +#define KVM_REG_X86        0x2000000000000000ULL
>>>> +#define KVM_REG_IA64        0x3000000000000000ULL
>>>> +#define KVM_REG_ARM        0x4000000000000000ULL
>>>> +#define KVM_REG_S390        0x5000000000000000ULL
>>>> +
>>>> +#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
>>> 
>>> Why not just encode directly as number of bytes?
>> 
>> Because this is 1 << n bytes :)
> 
> Some registers may not be a power-of-2 number of bytes (e.g. x86 segment
> descriptors), and we've got plenty of space to spare in the id.

If they're not a power of 2, we can still bmp it to the next power of 2 size, no?

Alex

> 
> It's probably not worth another respin, though -- we can just document
> right/left justification in the event that we have such a register.
> 
> -Scott
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Scott Wood Jan. 9, 2012, 8:12 p.m. UTC | #5 
On 01/09/2012 02:11 PM, Alexander Graf wrote:
> 
> 
> Am 09.01.2012 um 20:07 schrieb Scott Wood <scottwood@freescale.com>:
> 
>> On 01/06/2012 06:52 PM, Alexander Graf wrote:
>>>
>>>
>>> On 06.01.2012, at 20:32, Scott Wood <scottwood@freescale.com> wrote:
>>>
>>>> On 01/05/2012 10:15 PM, Alexander Graf wrote:
>>>>
>>>>> +/*
>>>>> + * Architecture specific registers are to be defined in arch headers and
>>>>> + * ORed with the arch identifier.
>>>>> + */
>>>>> +#define KVM_REG_PPC        0x1000000000000000ULL
>>>>> +#define KVM_REG_X86        0x2000000000000000ULL
>>>>> +#define KVM_REG_IA64        0x3000000000000000ULL
>>>>> +#define KVM_REG_ARM        0x4000000000000000ULL
>>>>> +#define KVM_REG_S390        0x5000000000000000ULL
>>>>> +
>>>>> +#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
>>>>
>>>> Why not just encode directly as number of bytes?
>>>
>>> Because this is 1 << n bytes :)
>>
>> Some registers may not be a power-of-2 number of bytes (e.g. x86 segment
>> descriptors), and we've got plenty of space to spare in the id.
> 
> If they're not a power of 2, we can still bmp it to the next power of 2 size, no?

Yes, that's what I meant by it not being a huge deal and documenting
whether the padding is on the right or the left.  Just seems a little
more awkward than necessary, given that we have room to encode it as a
plain linear size.

-Scott

--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Graf Jan. 9, 2012, 8:14 p.m. UTC | #6 
Am 09.01.2012 um 21:12 schrieb Scott Wood <scottwood@freescale.com>:

> On 01/09/2012 02:11 PM, Alexander Graf wrote:
>> 
>> 
>> Am 09.01.2012 um 20:07 schrieb Scott Wood <scottwood@freescale.com>:
>> 
>>> On 01/06/2012 06:52 PM, Alexander Graf wrote:
>>>> 
>>>> 
>>>> On 06.01.2012, at 20:32, Scott Wood <scottwood@freescale.com> wrote:
>>>> 
>>>>> On 01/05/2012 10:15 PM, Alexander Graf wrote:
>>>>> 
>>>>>> +/*
>>>>>> + * Architecture specific registers are to be defined in arch headers and
>>>>>> + * ORed with the arch identifier.
>>>>>> + */
>>>>>> +#define KVM_REG_PPC        0x1000000000000000ULL
>>>>>> +#define KVM_REG_X86        0x2000000000000000ULL
>>>>>> +#define KVM_REG_IA64        0x3000000000000000ULL
>>>>>> +#define KVM_REG_ARM        0x4000000000000000ULL
>>>>>> +#define KVM_REG_S390        0x5000000000000000ULL
>>>>>> +
>>>>>> +#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
>>>>> 
>>>>> Why not just encode directly as number of bytes?
>>>> 
>>>> Because this is 1 << n bytes :)
>>> 
>>> Some registers may not be a power-of-2 number of bytes (e.g. x86 segment
>>> descriptors), and we've got plenty of space to spare in the id.
>> 
>> If they're not a power of 2, we can still bmp it to the next power of 2 size, no?
> 
> Yes, that's what I meant by it not being a huge deal and documenting
> whether the padding is on the right or the left.  Just seems a little
> more awkward than necessary, given that we have room to encode it as a
> plain linear size.

Well I thought it makes things easier, since variable types are usually in a power of 2 :). But I'm certainly not attached to it.

Alex

> 
> -Scott
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index a4d7b4d..8494214 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -1523,6 +1523,46 @@  following algorithm:
 Some guests configure the LINT1 NMI input to cause a panic, aiding in
 debugging.
 
+4.65 KVM_SET_ONE_REG
+
+Capability: KVM_CAP_ONE_REG
+Architectures: all
+Type: vcpu ioctl
+Parameters: struct kvm_one_reg (in)
+Returns: 0 on success, negative value on failure
+
+struct kvm_one_reg {
+       __u64 id;
+       __u64 addr;
+};
+
+Using this ioctl, a single vcpu register can be set to a specific value
+defined by user space with the passed in struct kvm_one_reg, where id
+refers to the register identifier as described below and addr is a pointer
+to a variable with the respective size. There can be architecture agnostic
+and architecture specific registers. Each have their own range of operation
+and their own constants and width. To keep track of the implemented
+registers, find a list below:
+
+  Arch  |       Register        | Width (bits)
+        |                       |
+
+4.66 KVM_GET_ONE_REG
+
+Capability: KVM_CAP_ONE_REG
+Architectures: all
+Type: vcpu ioctl
+Parameters: struct kvm_one_reg (in and out)
+Returns: 0 on success, negative value on failure
+
+This ioctl allows to receive the value of a single register implemented
+in a vcpu. The register to read is indicated by the "id" field of the
+kvm_one_reg struct passed in. On success, the register value can be found
+at the memory location pointed to by "addr".
+
+The list of registers accessible using this interface is identical to the
+list in 4.64.
+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 76993eb..8bdd75b 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -214,6 +214,7 @@  int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_PPC_UNSET_IRQ:
 	case KVM_CAP_PPC_IRQ_LEVEL:
 	case KVM_CAP_ENABLE_CAP:
+	case KVM_CAP_ONE_REG:
 		r = 1;
 		break;
 #ifndef CONFIG_KVM_BOOK3S_64_HV
@@ -642,6 +643,32 @@  static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
 	return r;
 }
 
+static int kvm_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu,
+				      struct kvm_one_reg *reg)
+{
+	int r = -EINVAL;
+
+	switch (reg->id) {
+	default:
+		break;
+	}
+
+	return r;
+}
+
+static int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu *vcpu,
+				      struct kvm_one_reg *reg)
+{
+	int r = -EINVAL;
+
+	switch (reg->id) {
+	default:
+		break;
+	}
+
+	return r;
+}
+
 int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
                                     struct kvm_mp_state *mp_state)
 {
@@ -681,6 +708,20 @@  long kvm_arch_vcpu_ioctl(struct file *filp,
 		break;
 	}
 
+	case KVM_SET_ONE_REG:
+	case KVM_GET_ONE_REG:
+	{
+		struct kvm_one_reg reg;
+		r = -EFAULT;
+		if (copy_from_user(&reg, argp, sizeof(reg)))
+			goto out;
+		if (ioctl == KVM_SET_ONE_REG)
+			r = kvm_vcpu_ioctl_set_one_reg(vcpu, &reg);
+		else
+			r = kvm_vcpu_ioctl_get_one_reg(vcpu, &reg);
+		break;
+	}
+
 #ifdef CONFIG_KVM_E500
 	case KVM_DIRTY_TLB: {
 		struct kvm_dirty_tlb dirty;
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index 8d40db7..4b63ec1 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -557,6 +557,7 @@  struct kvm_ppc_pvinfo {
 #define KVM_CAP_MAX_VCPUS 66       /* returns max vcpus per vm */
 #define KVM_CAP_PPC_PAPR 68
 #define KVM_CAP_SW_TLB 69
+#define KVM_CAP_ONE_REG 70
 #define KVM_CAP_S390_GMAP 71
 #define KVM_CAP_TSC_DEADLINE_TIMER 72
 
@@ -653,6 +654,37 @@  struct kvm_dirty_tlb {
 	__u32 num_dirty;
 };
 
+/* Available with KVM_CAP_ONE_REG */
+
+#define KVM_REG_ARCH_MASK	0xff00000000000000ULL
+#define KVM_REG_GENERIC		0x0000000000000000ULL
+
+/*
+ * Architecture specific registers are to be defined in arch headers and
+ * ORed with the arch identifier.
+ */
+#define KVM_REG_PPC		0x1000000000000000ULL
+#define KVM_REG_X86		0x2000000000000000ULL
+#define KVM_REG_IA64		0x3000000000000000ULL
+#define KVM_REG_ARM		0x4000000000000000ULL
+#define KVM_REG_S390		0x5000000000000000ULL
+
+#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
+
+struct kvm_one_reg {
+	__u64 id;
+	__u64 addr;
+};
+
 /*
  * ioctls for VM fds
  */
@@ -781,6 +813,9 @@  struct kvm_dirty_tlb {
 #define KVM_ALLOCATE_RMA	  _IOR(KVMIO,  0xa9, struct kvm_allocate_rma)
 /* Available with KVM_CAP_SW_TLB */
 #define KVM_DIRTY_TLB		  _IOW(KVMIO,  0xaa, struct kvm_dirty_tlb)
+/* Available with KVM_CAP_ONE_REG */
+#define KVM_GET_ONE_REG		  _IOW(KVMIO,  0xab, struct kvm_one_reg)
+#define KVM_SET_ONE_REG		  _IOW(KVMIO,  0xac, struct kvm_one_reg)
 
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)