On Mon, Oct 31, 2011 at 08:53:11AM +0100, 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.
>
> Signed-off-by: Alexander Graf <agraf@xxxxxxx>
> ---
> Documentation/virtual/kvm/api.txt | 47 ++++++++++++++++++++++++++++++++++
> arch/powerpc/kvm/powerpc.c | 51 +++++++++++++++++++++++++++++++++++++
> include/linux/kvm.h | 32 +++++++++++++++++++++++
> 3 files changed, 130 insertions(+), 0 deletions(-)
I don't see the benefit of this generalization, the current structure where
context information is hardcoded in the data transmitted works well.
Avi?
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index ab1136f..a23fe62 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1482,6 +1482,53 @@ is supported; 2 if the processor requires all virtual machines to have
> an RMA, or 1 if the processor can use an RMA but doesn't require it,
> because it supports the Virtual RMA (VRMA) facility.
>
> +4.64 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;
> + union {
> + __u8 reg8;
> + __u16 reg16;
> + __u32 reg32;
> + __u64 reg64;
> + __u8 reg128[16];
> + __u8 reg256[32];
> + __u8 reg512[64];
> + __u8 reg1024[128];
> + } u;
> +};
> +
> +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. 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.65 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
> +in the respective width field of the struct after this call.
> +
> +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 e75c5ac..39cdb3f 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
> @@ -627,6 +628,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)
> {
> @@ -666,6 +693,30 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> break;
> }
>
> + case KVM_GET_ONE_REG:
> + {
> + struct kvm_one_reg reg;
> + r = -EFAULT;
> + if (copy_from_user(®, argp, sizeof(reg)))
> + goto out;
> + r = kvm_vcpu_ioctl_get_one_reg(vcpu, ®);
> + if (copy_to_user(argp, ®, sizeof(reg))) {
> + r = -EFAULT;
> + goto out;
> + }
> + break;
> + }
> +
> + case KVM_SET_ONE_REG:
> + {
> + struct kvm_one_reg reg;
> + r = -EFAULT;
> + if (copy_from_user(®, argp, sizeof(reg)))
> + goto out;
> + r = kvm_vcpu_ioctl_set_one_reg(vcpu, ®);
> + 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 a6b1295..e652a7b 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
>
> #ifdef KVM_CAP_IRQ_ROUTING
> @@ -652,6 +653,34 @@ struct kvm_dirty_tlb {
> __u32 num_dirty;
> };
>
> +/* Available with KVM_CAP_ONE_REG */
> +
> +#define KVM_ONE_REG_GENERIC 0x0000000000000000ULL
> +
> +/*
> + * Architecture specific registers are to be defined in arch headers and
> + * ORed with the arch identifier.
> + */
> +#define KVM_ONE_REG_PPC 0x1000000000000000ULL
> +#define KVM_ONE_REG_X86 0x2000000000000000ULL
> +#define KVM_ONE_REG_IA64 0x3000000000000000ULL
> +#define KVM_ONE_REG_ARM 0x4000000000000000ULL
> +#define KVM_ONE_REG_S390 0x5000000000000000ULL
> +
> +struct kvm_one_reg {
> + __u64 id;
> + union {
> + __u8 reg8;
> + __u16 reg16;
> + __u32 reg32;
> + __u64 reg64;
> + __u8 reg128[16];
> + __u8 reg256[32];
> + __u8 reg512[64];
> + __u8 reg1024[128];
> + } u;
> +};
> +
> /*
> * ioctls for VM fds
> */
> @@ -780,6 +809,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 _IOWR(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)
>
> --
> 1.6.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
