[RFC PATCH v3 5/5] arm/arm64: vgic-new: Implement KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO ioctl
Christoffer Dall
christoffer.dall at linaro.org
Tue Aug 30 07:00:34 PDT 2016
On Wed, Aug 24, 2016 at 04:50:09PM +0530, vijay.kilari at gmail.com wrote:
> From: Vijaya Kumar K <Vijaya.Kumar at cavium.com>
>
> Userspace requires to store and restore of line_level for
> level triggered interrupts. For this ioctl KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO
> is defined.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar at cavium.com>
> ---
> arch/arm64/include/uapi/asm/kvm.h | 6 +++++
> virt/kvm/arm/vgic/vgic-kvm-device.c | 44 ++++++++++++++++++++++++++++++++++++-
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 19 ++++++++++++++++
> virt/kvm/arm/vgic/vgic-mmio.c | 34 ++++++++++++++++++++++++++++
> virt/kvm/arm/vgic/vgic-mmio.h | 6 +++++
> virt/kvm/arm/vgic/vgic.h | 3 +++
> 6 files changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index b13c944..45c56d7 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -209,6 +209,12 @@ struct kvm_arch_memory_slot {
> #define KVM_DEV_ARM_VGIC_GRP_CTRL 4
> #define KVM_DEV_ARM_VGIC_GRP_REDIST_REGS 5
> #define KVM_DEV_ARM_VGIC_CPU_SYSREGS 6
> +#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 9
This should be 10, bits 0 through 9 gives you 10 to work with.
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
> + (0x7fffffULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
this mask is also wrong, 32 - 10 == 22, not 23.
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK 0x1ff
this is also wrong, you have 10 bits, not 9 bits.
Hint: the max SPI number is around 1024.
> +#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_VAL 1
This should really be KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO_LEVEL. Why is 0
not a valid value?
>
> #define KVM_DEV_ARM_VGIC_CTRL_INIT 0
> #define KVM_DEV_ARM_VGIC_SYSREG_MASK (KVM_REG_ARM64_SYSREG_OP0_MASK | \
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c b/virt/kvm/arm/vgic/vgic-kvm-device.c
> index 74e5c38..7e3bc49 100644
> --- a/virt/kvm/arm/vgic/vgic-kvm-device.c
> +++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
> @@ -509,6 +509,23 @@ static int vgic_attr_regs_access_v3(struct kvm_device *dev,
> regid, reg);
> break;
> }
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> + unsigned int info, intid;
> +
> + info = (attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT;
> + if (info == KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_VAL) {
> + intid = attr->attr &
> + KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK;
> + ret = vgic_v3_line_level_info_uaccess(vcpu, is_write,
> + intid, &tmp32);
> + if (!is_write)
> + *reg = tmp32;
> + } else {
> + ret = -EINVAL;
> + }
> + break;
> + }
> default:
> ret = -EINVAL;
> break;
> @@ -551,6 +568,17 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
>
> return vgic_attr_regs_access_v3(dev, attr, ®, true);
> }
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> + u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> + u64 reg;
> + u32 tmp32;
here we have more type fun, which I assume you fix based on my comments
on a previous patch.
> +
> + if (get_user(tmp32, uaddr))
> + return -EFAULT;
> +
> + reg = tmp32;
> + return vgic_attr_regs_access_v3(dev, attr, ®, true);
> + }
> }
> return -ENXIO;
> }
> @@ -588,8 +616,19 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
> ret = put_user(reg, uaddr);
> return ret;
> }
> - }
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
> + u32 __user *uaddr = (u32 __user *)(long)attr->addr;
> + u64 reg;
> + u32 tmp32;
>
> + ret = vgic_attr_regs_access_v3(dev, attr, ®, false);
> + if (ret)
> + return ret;
> + tmp32 = reg;
> + ret = put_user(tmp32, uaddr);
> + return ret;
> + }
> + }
> return -ENXIO;
> }
>
> @@ -610,11 +649,14 @@ static int vgic_v3_has_attr(struct kvm_device *dev,
> return vgic_v3_has_attr_regs(dev, attr);
> case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> return 0;
> + case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
> + return 0;
probably you should check the info field here as well
> case KVM_DEV_ARM_VGIC_GRP_CTRL:
> switch (attr->attr) {
> case KVM_DEV_ARM_VGIC_CTRL_INIT:
> return 0;
> }
> + break;
> }
> return -ENXIO;
> }
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index 61abea0..fde1472 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -789,3 +789,22 @@ int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>
> return vgic_v3_uaccess(vcpu, dev, is_write, offset, val);
> }
> +
> +int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> + u32 intid, u32 *val)
> +{
> + unsigned int len = 4;
> + u8 buf[4];
> + int ret;
> +
> + if (is_write) {
> + vgic_data_host_to_mmio_bus(buf, len, *val);
why do you involve the mmio bus in this?
> + ret = vgic_write_irq_line_level_info(vcpu, intid, len, buf);
> + } else {
> + ret = vgic_read_irq_line_level_info(vcpu, intid, len, buf);
> + if (!ret)
> + *val = vgic_data_mmio_bus_to_host(buf, len);
> + }
> +
> + return ret;
> +}
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
> index 38f2c75..74d0449 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
> @@ -391,6 +391,40 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
> }
> }
>
> +int vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
> + unsigned int len, void *val)
> +{
> + unsigned long data = 0;
> + int i;
> +
> + for (i = 0; i < len * 8; i++) {
> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> + if (irq->line_level)
> + data |= (1U << i);
> + }
> + vgic_data_host_to_mmio_bus(val, len, data);
why???
> +
> + return 0;
> +}
> +
> +int vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
> + unsigned int len, const void *val)
> +{
> + int i;
> + unsigned long data = vgic_data_mmio_bus_to_host(val, len);
why???
> +
> + for_each_set_bit(i, &data, len * 8) {
> + struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
> +
> + spin_lock(&irq->irq_lock);
> + irq->line_level = true;
> + spin_unlock(&irq->irq_lock);
> + }
> +
> + return 0;
> +}
> +
> static int match_region(const void *key, const void *elt)
> {
> const unsigned int offset = (unsigned long)key;
> diff --git a/virt/kvm/arm/vgic/vgic-mmio.h b/virt/kvm/arm/vgic/vgic-mmio.h
> index b97a97b..b03c4e7 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio.h
> +++ b/virt/kvm/arm/vgic/vgic-mmio.h
> @@ -183,6 +183,12 @@ int vgic_mmio_uaccess_read(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
> int vgic_mmio_uaccess_write(struct kvm_vcpu *vcpu, struct kvm_io_device *dev,
> gpa_t addr, int len, const void *val);
>
> +int vgic_read_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
> + unsigned int len, void *val);
> +
> +int vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid,
> + unsigned int len, const void *val);
> +
> unsigned int vgic_v2_init_dist_iodev(struct vgic_io_device *dev);
>
> unsigned int vgic_v3_init_dist_iodev(struct vgic_io_device *dev);
> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
> index 20eab36c..b8ee5b9 100644
> --- a/virt/kvm/arm/vgic/vgic.h
> +++ b/virt/kvm/arm/vgic/vgic.h
> @@ -100,6 +100,9 @@ int vgic_v3_cpu_sysregs_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> u64 id, u64 *val);
> int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
> u64 *reg);
> +int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
> + u32 intid, u32 *val);
> +
> #else
> static inline void vgic_v3_process_maintenance(struct kvm_vcpu *vcpu)
> {
> --
> 1.9.1
>
More information about the linux-arm-kernel
mailing list