Re: [PATCH v6 6/7] arm/arm64: vgic-new: Implement VGICv3 CPU interface access

2016-09-22 Thread Marc Zyngier
On 20/09/16 07:12, vijay.kil...@gmail.com wrote:
> From: Vijaya Kumar K 
> 
> VGICv3 CPU interface registers are accessed using
> KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
> as 64-bit. The cpu MPIDR value is passed along with register id.
> is used to identify the cpu for registers access.
> 
> The version of VGIC v3 specification is define here
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html
> 
> Signed-off-by: Pavel Fedin 
> Signed-off-by: Vijaya Kumar K 
> ---
>  arch/arm64/include/uapi/asm/kvm.h   |   3 +
>  arch/arm64/kvm/Makefile |   1 +
>  include/kvm/arm_vgic.h  |   9 +
>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 +++
>  virt/kvm/arm/vgic/vgic-mmio-v3.c|  19 +++
>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 327 
> 
>  virt/kvm/arm/vgic/vgic-v3.c |   7 +
>  virt/kvm/arm/vgic/vgic.h|   4 +
>  8 files changed, 397 insertions(+)
> 
> diff --git a/arch/arm64/include/uapi/asm/kvm.h 
> b/arch/arm64/include/uapi/asm/kvm.h
> index 56dc08d..91c7137 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
>   (0xULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
>  #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT  0
>  #define   KVM_DEV_ARM_VGIC_OFFSET_MASK   (0xULL << 
> KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
> +#define   KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0x)
>  #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS 3
>  #define KVM_DEV_ARM_VGIC_GRP_CTRL4
>  #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> +#define KVM_DEV_ARM_VGIC_CPU_SYSREGS6
> +
>  #define   KVM_DEV_ARM_VGIC_CTRL_INIT 0
>  
>  /* Device Control API on vcpu fd */
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index d50a82a..1a14e29 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -32,5 +32,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
> +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
>  kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
>  kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 002f092..b986c25 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -71,6 +71,15 @@ struct vgic_global {
>  
>   /* GIC system register CPU interface */
>   struct static_key_false gicv3_cpuif;
> +
> + /* Cache ICH_VTR_EL2 reg value */
> + u32 ich_vtr_el2;
> +
> + /* Cache guest priority bits */
> + u32 num_pri_bits;
> +
> + /* Cache guest interrupt ID bits */
> + u32 num_id_bits;

I'm going to cry now... Have you noticed that you are caching
guest-dependent values in a global structure? What happens when you have
more than one?

Caching the *host* values are fine, but the guest's? Come on...

>  };
>  
>  extern struct vgic_global kvm_vgic_global_state;
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 
> b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index 6c7d30c..da532d1 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -504,6 +504,14 @@ static int vgic_v3_attr_regs_access(struct kvm_device 
> *dev,
>   if (!is_write)
>   *reg = tmp32;
>   break;
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 regid;
> +
> + regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
> + ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
> +   regid, reg);
> + break;
> + }
>   default:
>   ret = -EINVAL;
>   break;
> @@ -537,6 +545,15 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>   reg = tmp32;
>   return vgic_v3_attr_regs_access(dev, attr, ®, true);
>   }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> + u64 reg;
> +
> + if (get_user(reg, uaddr))
> + return -EFAULT;
> +
> + return vgic_v3_attr_regs_access(dev, attr, ®, true);
> + }
>   }
>   return -ENXIO;
>  }
> @@ -563,6 +580,15 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
>   tmp32 = reg;
>   return put_user(tmp32, uaddr);
>   }
> + case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
> + u64 __user *uaddr = (u64 __user *)(long)attr->addr;
> + u64 reg;
> +
> + ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
> + if (ret)
> + return ret;
> + return put_user(reg, uad

[PATCH v6 6/7] arm/arm64: vgic-new: Implement VGICv3 CPU interface access

2016-09-19 Thread vijay . kilari
From: Vijaya Kumar K 

VGICv3 CPU interface registers are accessed using
KVM_DEV_ARM_VGIC_CPU_SYSREGS ioctl. These registers are accessed
as 64-bit. The cpu MPIDR value is passed along with register id.
is used to identify the cpu for registers access.

The version of VGIC v3 specification is define here
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-July/445611.html

Signed-off-by: Pavel Fedin 
Signed-off-by: Vijaya Kumar K 
---
 arch/arm64/include/uapi/asm/kvm.h   |   3 +
 arch/arm64/kvm/Makefile |   1 +
 include/kvm/arm_vgic.h  |   9 +
 virt/kvm/arm/vgic/vgic-kvm-device.c |  27 +++
 virt/kvm/arm/vgic/vgic-mmio-v3.c|  19 +++
 virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 327 
 virt/kvm/arm/vgic/vgic-v3.c |   7 +
 virt/kvm/arm/vgic/vgic.h|   4 +
 8 files changed, 397 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/kvm.h 
b/arch/arm64/include/uapi/asm/kvm.h
index 56dc08d..91c7137 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -206,9 +206,12 @@ struct kvm_arch_memory_slot {
(0xULL << KVM_DEV_ARM_VGIC_V3_MPIDR_SHIFT)
 #define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT0
 #define   KVM_DEV_ARM_VGIC_OFFSET_MASK (0xULL << 
KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+#define   KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK (0x)
 #define KVM_DEV_ARM_VGIC_GRP_NR_IRQS   3
 #define KVM_DEV_ARM_VGIC_GRP_CTRL  4
 #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
+#define KVM_DEV_ARM_VGIC_CPU_SYSREGS6
+
 #define   KVM_DEV_ARM_VGIC_CTRL_INIT   0
 
 /* Device Control API on vcpu fd */
diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
index d50a82a..1a14e29 100644
--- a/arch/arm64/kvm/Makefile
+++ b/arch/arm64/kvm/Makefile
@@ -32,5 +32,6 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o
+kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-sys-reg-v3.o
 kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o
 kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 002f092..b986c25 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -71,6 +71,15 @@ struct vgic_global {
 
/* GIC system register CPU interface */
struct static_key_false gicv3_cpuif;
+
+   /* Cache ICH_VTR_EL2 reg value */
+   u32 ich_vtr_el2;
+
+   /* Cache guest priority bits */
+   u32 num_pri_bits;
+
+   /* Cache guest interrupt ID bits */
+   u32 num_id_bits;
 };
 
 extern struct vgic_global kvm_vgic_global_state;
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 
b/virt/kvm/arm/vgic/vgic-kvm-device.c
index 6c7d30c..da532d1 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -504,6 +504,14 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
if (!is_write)
*reg = tmp32;
break;
+   case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
+   u64 regid;
+
+   regid = (attr->attr & KVM_DEV_ARM_VGIC_SYSREG_INSTR_MASK);
+   ret = vgic_v3_cpu_sysregs_uaccess(vcpu, is_write,
+ regid, reg);
+   break;
+   }
default:
ret = -EINVAL;
break;
@@ -537,6 +545,15 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
reg = tmp32;
return vgic_v3_attr_regs_access(dev, attr, ®, true);
}
+   case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
+   u64 __user *uaddr = (u64 __user *)(long)attr->addr;
+   u64 reg;
+
+   if (get_user(reg, uaddr))
+   return -EFAULT;
+
+   return vgic_v3_attr_regs_access(dev, attr, ®, true);
+   }
}
return -ENXIO;
 }
@@ -563,6 +580,15 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
tmp32 = reg;
return put_user(tmp32, uaddr);
}
+   case KVM_DEV_ARM_VGIC_CPU_SYSREGS: {
+   u64 __user *uaddr = (u64 __user *)(long)attr->addr;
+   u64 reg;
+
+   ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
+   if (ret)
+   return ret;
+   return put_user(reg, uaddr);
+   }
}
 
return -ENXIO;
@@ -581,6 +607,7 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
break;
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
+   case KVM_DEV_ARM_VGIC_CPU_SYSREGS:
return vgic_v3_has_attr_regs(dev, attr);
case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
return 0;
diff --git a/virt/