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

2016-09-19 Thread Peter Maydell
On 19 September 2016 at 08:36, Vijay Kilari  wrote:
> In order to track the PRI and ID bits written by guest,
> VGIC needs to store these values when ICC_CTRL_EL1 is updated.
> However,  QEMU is reseting VGIC by writing 0's to all the
> registers after VGIC initialization and hence the back up values are
> always reset to 0 and hence when guest read back, VGIC returns wrong value.
>
> One option is to drop VGIC reset from QEMU which is not doing much.

No, if QEMU's reset is not resetting registers to the right value
the correct thing to do is fix the reset code. The way device
reset is supposed to work is that QEMU knows the right values
to use and it writes them to the kernel.

QEMU doesn't reset ICC_CTLR_EL1 to 0 for TCG:
cs->icc_ctlr_el1[GICV3_NS] = ICC_CTLR_EL1_A3V |
(1 << ICC_CTLR_EL1_IDBITS_SHIFT) |
(7 << ICC_CTLR_EL1_PRIBITS_SHIFT);

but I don't think that code gets run for the KVM VGIC.

thanks
-- PMM
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


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

2016-09-19 Thread Vijay Kilari
Hi Marc , Peter

On Sat, Sep 17, 2016 at 5:07 PM, Marc Zyngier  wrote:
> On Sat, 17 Sep 2016 11:58:48 +0530
> Vijay Kilari  wrote:
>
>> On Fri, Sep 16, 2016 at 10:37 PM, Marc Zyngier  wrote:
>> > On 16/09/16 17:57, Vijay Kilari wrote:
>> >> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier  
>> >> wrote:
>> >>> On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
>>   virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
>>   virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
>>   virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
>>  
>>   virt/kvm/arm/vgic/vgic.h|  10 ++
>>   7 files changed, 385 insertions(+)
>> >
>> > [...]
>> >
>>  diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c 
>>  b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>  new file mode 100644
>>  index 000..8e4f403
>>  --- /dev/null
>>  +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>  @@ -0,0 +1,296 @@
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +#include 
>>  +
>>  +#include "vgic.h"
>>  +#include "vgic-mmio.h"
>>  +#include "sys_regs.h"
>>  +
>>  +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct 
>>  sys_reg_params *p,
>>  + const struct sys_reg_desc *r)
>>  +{
>>  + struct vgic_vmcr vmcr;
>>  + u64 val;
>>  + u32 ich_vtr;
>>  +
>>  + vgic_get_vmcr(vcpu, &vmcr);
>>  + if (p->is_write) {
>>  + val = p->regval;
>>  + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
>>  + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
>>  +   ICC_CTLR_EL1_CBPR_SHIFT) << 
>>  ICH_VMCR_CBPR_SHIFT;
>>  + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
>>  +  ICC_CTLR_EL1_EOImode_SHIFT) << 
>>  ICH_VMCR_EOIM_SHIFT;
>>  + vgic_set_vmcr(vcpu, &vmcr);
>> >>>
>> >>> You've ignored my comments again: "What if userspace writes something
>> >>> that is incompatible with the current configuration? Wrong number of ID
>> >>> bits, or number of priorities?"
>> >>
>> >> IMO, In case of incompatibility,
>> >> If ID bits and PRI bits are less than HW supported, it is ok.
>> >
>> > Yes. But you also need to track of what the guest has programmed in
>> > order to be able to migrate it back to its original configuration.
>>
>> You mean the vgic has to track/store the ID and PRI bits that guest
>> has programmed
>> and return the same when guest reads back instead of
>> returning HW supported value for ICC_CTLR_EL1 reg access?.
>
> If you have two hosts (A and B), A having 5 bits of priority and B
> having 7 bits, you should be able to migrate from A to B, and then from
> B to A. Which means you have to preserve what the guest knows to be its
> configuration, even if you run on a more capable system. Otherwise,
> you're a bit stuck.
>
> You probably won't be able to hide the discrepancy from inside the
> guest though (the guest will be able to observe the change), but this
> is better than nothing.

In order to track the PRI and ID bits written by guest,
VGIC needs to store these values when ICC_CTRL_EL1 is updated.
However,  QEMU is reseting VGIC by writing 0's to all the
registers after VGIC initialization and hence the back up values are
always reset to 0 and hence when guest read back, VGIC returns wrong value.

One option is to drop VGIC reset from QEMU which is not doing much.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


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

2016-09-18 Thread Marc Zyngier
On Sun, 18 Sep 2016 12:00:01 +0530
Vijay Kilari  wrote:

> Hi Marc,
> 
> On Fri, Sep 16, 2016 at 10:37 PM, Marc Zyngier  wrote:
> > On 16/09/16 17:57, Vijay Kilari wrote:  
> >> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier  
> >> wrote:  
> >>> On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
>   virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
>   virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
>   virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
>  
>   virt/kvm/arm/vgic/vgic.h|  10 ++
>   7 files changed, 385 insertions(+)  
> >
> > [...]
> >  
>  diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c 
>  b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>  new file mode 100644
>  index 000..8e4f403
>  --- /dev/null
>  +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>  @@ -0,0 +1,296 @@
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +
>  +#include "vgic.h"
>  +#include "vgic-mmio.h"
>  +#include "sys_regs.h"
>  +
>  +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct 
>  sys_reg_params *p,
>  + const struct sys_reg_desc *r)
>  +{
>  + struct vgic_vmcr vmcr;
>  + u64 val;
>  + u32 ich_vtr;
>  +
>  + vgic_get_vmcr(vcpu, &vmcr);
>  + if (p->is_write) {
>  + val = p->regval;
>  + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
>  + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
>  +   ICC_CTLR_EL1_CBPR_SHIFT) << 
>  ICH_VMCR_CBPR_SHIFT;
>  + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
>  +  ICC_CTLR_EL1_EOImode_SHIFT) << 
>  ICH_VMCR_EOIM_SHIFT;
>  + vgic_set_vmcr(vcpu, &vmcr);  
> >>>
> >>> You've ignored my comments again: "What if userspace writes something
> >>> that is incompatible with the current configuration? Wrong number of ID
> >>> bits, or number of priorities?"  
> >>
> >> IMO, In case of incompatibility,
> >> If ID bits and PRI bits are less than HW supported, it is ok.  
> >
> > Yes. But you also need to track of what the guest has programmed in
> > order to be able to migrate it back to its original configuration.
> >  
> >> If ID bits and PRI bits are greater than HW supported, then warn would be 
> >> good
> >> enough. Please suggest the behaviour that you think it should be.  
> >
> > No, it is an error, plain and simple. You cannot run in this condition.
> >  
> >>  
> >>>  
>  + } else {
>  + ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
>  +
>  + val = 0;
>  + val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
>  + ICH_VTR_PRI_BITS_SHIFT) << 
>  ICC_CTLR_EL1_PRI_BITS_SHIFT;
>  + val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
>  + ICH_VTR_ID_BITS_SHIFT) << 
>  ICC_CTLR_EL1_ID_BITS_SHIFT;
>  + val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> 
>  ICH_VTR_SEIS_SHIFT)
>  + << ICC_CTLR_EL1_SEIS_SHIFT;
>  + val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
>  + << ICC_CTLR_EL1_A3V_SHIFT;
>  + val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
>  + ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
>  + val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
>  + ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
>  +
>  + p->regval = val;
>  + }
>  +
>  + return true;
>  +}
>  +
>  +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params 
>  *p,
>  +const struct sys_reg_desc *r)
>  +{
>  + struct vgic_vmcr vmcr;
>  +
>  + vgic_get_vmcr(vcpu, &vmcr);
>  + if (p->is_write) {
>  + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> 
>  ICC_PMR_EL1_SHIFT;
>  + vgic_set_vmcr(vcpu, &vmcr);
>  + } else {
>  + p->regval = (vmcr.pmr << 

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

2016-09-17 Thread Vijay Kilari
Hi Marc,

On Fri, Sep 16, 2016 at 10:37 PM, Marc Zyngier  wrote:
> On 16/09/16 17:57, Vijay Kilari wrote:
>> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier  wrote:
>>> On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
  virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
 
  virt/kvm/arm/vgic/vgic.h|  10 ++
  7 files changed, 385 insertions(+)
>
> [...]
>
 diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c 
 b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
 new file mode 100644
 index 000..8e4f403
 --- /dev/null
 +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
 @@ -0,0 +1,296 @@
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +
 +#include "vgic.h"
 +#include "vgic-mmio.h"
 +#include "sys_regs.h"
 +
 +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params 
 *p,
 + const struct sys_reg_desc *r)
 +{
 + struct vgic_vmcr vmcr;
 + u64 val;
 + u32 ich_vtr;
 +
 + vgic_get_vmcr(vcpu, &vmcr);
 + if (p->is_write) {
 + val = p->regval;
 + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
 + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
 +   ICC_CTLR_EL1_CBPR_SHIFT) << 
 ICH_VMCR_CBPR_SHIFT;
 + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
 +  ICC_CTLR_EL1_EOImode_SHIFT) << 
 ICH_VMCR_EOIM_SHIFT;
 + vgic_set_vmcr(vcpu, &vmcr);
>>>
>>> You've ignored my comments again: "What if userspace writes something
>>> that is incompatible with the current configuration? Wrong number of ID
>>> bits, or number of priorities?"
>>
>> IMO, In case of incompatibility,
>> If ID bits and PRI bits are less than HW supported, it is ok.
>
> Yes. But you also need to track of what the guest has programmed in
> order to be able to migrate it back to its original configuration.
>
>> If ID bits and PRI bits are greater than HW supported, then warn would be 
>> good
>> enough. Please suggest the behaviour that you think it should be.
>
> No, it is an error, plain and simple. You cannot run in this condition.
>
>>
>>>
 + } else {
 + ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
 +
 + val = 0;
 + val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
 + ICH_VTR_PRI_BITS_SHIFT) << 
 ICC_CTLR_EL1_PRI_BITS_SHIFT;
 + val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
 + ICH_VTR_ID_BITS_SHIFT) << ICC_CTLR_EL1_ID_BITS_SHIFT;
 + val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT)
 + << ICC_CTLR_EL1_SEIS_SHIFT;
 + val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
 + << ICC_CTLR_EL1_A3V_SHIFT;
 + val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
 + ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
 + val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
 + ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
 +
 + p->regval = val;
 + }
 +
 + return true;
 +}
 +
 +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params 
 *p,
 +const struct sys_reg_desc *r)
 +{
 + struct vgic_vmcr vmcr;
 +
 + vgic_get_vmcr(vcpu, &vmcr);
 + if (p->is_write) {
 + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> 
 ICC_PMR_EL1_SHIFT;
 + vgic_set_vmcr(vcpu, &vmcr);
 + } else {
 + p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & 
 ICC_PMR_EL1_MASK;
 + }
 +
 + return true;
 +}
 +
 +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params 
 *p,
 + const struct sys_reg_desc *r)
 +{
 + struct vgic_vmcr vmcr;
 +
 + vgic_get_vmcr(vcpu, &vmcr);
 + if (

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

2016-09-17 Thread Marc Zyngier
On Sat, 17 Sep 2016 11:58:48 +0530
Vijay Kilari  wrote:

> On Fri, Sep 16, 2016 at 10:37 PM, Marc Zyngier  wrote:
> > On 16/09/16 17:57, Vijay Kilari wrote:  
> >> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier  
> >> wrote:  
> >>> On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
>   virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
>   virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
>   virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
>  
>   virt/kvm/arm/vgic/vgic.h|  10 ++
>   7 files changed, 385 insertions(+)  
> >
> > [...]
> >  
>  diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c 
>  b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>  new file mode 100644
>  index 000..8e4f403
>  --- /dev/null
>  +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>  @@ -0,0 +1,296 @@
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +#include 
>  +
>  +#include "vgic.h"
>  +#include "vgic-mmio.h"
>  +#include "sys_regs.h"
>  +
>  +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct 
>  sys_reg_params *p,
>  + const struct sys_reg_desc *r)
>  +{
>  + struct vgic_vmcr vmcr;
>  + u64 val;
>  + u32 ich_vtr;
>  +
>  + vgic_get_vmcr(vcpu, &vmcr);
>  + if (p->is_write) {
>  + val = p->regval;
>  + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
>  + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
>  +   ICC_CTLR_EL1_CBPR_SHIFT) << 
>  ICH_VMCR_CBPR_SHIFT;
>  + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
>  +  ICC_CTLR_EL1_EOImode_SHIFT) << 
>  ICH_VMCR_EOIM_SHIFT;
>  + vgic_set_vmcr(vcpu, &vmcr);  
> >>>
> >>> You've ignored my comments again: "What if userspace writes something
> >>> that is incompatible with the current configuration? Wrong number of ID
> >>> bits, or number of priorities?"  
> >>
> >> IMO, In case of incompatibility,
> >> If ID bits and PRI bits are less than HW supported, it is ok.  
> >
> > Yes. But you also need to track of what the guest has programmed in
> > order to be able to migrate it back to its original configuration.  
> 
> You mean the vgic has to track/store the ID and PRI bits that guest
> has programmed
> and return the same when guest reads back instead of
> returning HW supported value for ICC_CTLR_EL1 reg access?.

If you have two hosts (A and B), A having 5 bits of priority and B
having 7 bits, you should be able to migrate from A to B, and then from
B to A. Which means you have to preserve what the guest knows to be its
configuration, even if you run on a more capable system. Otherwise,
you're a bit stuck.

You probably won't be able to hide the discrepancy from inside the
guest though (the guest will be able to observe the change), but this
is better than nothing.

Thanks,

M.
-- 
Jazz is not dead. It just smells funny.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


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

2016-09-16 Thread Vijay Kilari
On Fri, Sep 16, 2016 at 10:37 PM, Marc Zyngier  wrote:
> On 16/09/16 17:57, Vijay Kilari wrote:
>> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier  wrote:
>>> On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
  virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
 
  virt/kvm/arm/vgic/vgic.h|  10 ++
  7 files changed, 385 insertions(+)
>
> [...]
>
 diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c 
 b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
 new file mode 100644
 index 000..8e4f403
 --- /dev/null
 +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
 @@ -0,0 +1,296 @@
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +#include 
 +
 +#include "vgic.h"
 +#include "vgic-mmio.h"
 +#include "sys_regs.h"
 +
 +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params 
 *p,
 + const struct sys_reg_desc *r)
 +{
 + struct vgic_vmcr vmcr;
 + u64 val;
 + u32 ich_vtr;
 +
 + vgic_get_vmcr(vcpu, &vmcr);
 + if (p->is_write) {
 + val = p->regval;
 + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
 + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
 +   ICC_CTLR_EL1_CBPR_SHIFT) << 
 ICH_VMCR_CBPR_SHIFT;
 + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
 +  ICC_CTLR_EL1_EOImode_SHIFT) << 
 ICH_VMCR_EOIM_SHIFT;
 + vgic_set_vmcr(vcpu, &vmcr);
>>>
>>> You've ignored my comments again: "What if userspace writes something
>>> that is incompatible with the current configuration? Wrong number of ID
>>> bits, or number of priorities?"
>>
>> IMO, In case of incompatibility,
>> If ID bits and PRI bits are less than HW supported, it is ok.
>
> Yes. But you also need to track of what the guest has programmed in
> order to be able to migrate it back to its original configuration.

You mean the vgic has to track/store the ID and PRI bits that guest
has programmed
and return the same when guest reads back instead of
returning HW supported value for ICC_CTLR_EL1 reg access?.

>
>> If ID bits and PRI bits are greater than HW supported, then warn would be 
>> good
>> enough. Please suggest the behaviour that you think it should be.
>
> No, it is an error, plain and simple. You cannot run in this condition.
>
[...]
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


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

2016-09-16 Thread Marc Zyngier
On 16/09/16 17:57, Vijay Kilari wrote:
> On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier  wrote:
>> On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
>>>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
>>>  virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
>>>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
>>> 
>>>  virt/kvm/arm/vgic/vgic.h|  10 ++
>>>  7 files changed, 385 insertions(+)

[...]

>>> diff --git a/virt/kvm/arm/vgic/vgic-sys-reg-v3.c 
>>> b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>> new file mode 100644
>>> index 000..8e4f403
>>> --- /dev/null
>>> +++ b/virt/kvm/arm/vgic/vgic-sys-reg-v3.c
>>> @@ -0,0 +1,296 @@
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include "vgic.h"
>>> +#include "vgic-mmio.h"
>>> +#include "sys_regs.h"
>>> +
>>> +static bool access_gic_ctlr(struct kvm_vcpu *vcpu, struct sys_reg_params 
>>> *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> + u64 val;
>>> + u32 ich_vtr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + val = p->regval;
>>> + vmcr.ctlr &= ~(ICH_VMCR_CBPR_MASK | ICH_VMCR_EOIM_MASK);
>>> + vmcr.ctlr |= ((val & ICC_CTLR_EL1_CBPR_MASK) >>
>>> +   ICC_CTLR_EL1_CBPR_SHIFT) << ICH_VMCR_CBPR_SHIFT;
>>> + vmcr.ctlr |= ((val & ICC_CTLR_EL1_EOImode_MASK) >>
>>> +  ICC_CTLR_EL1_EOImode_SHIFT) << 
>>> ICH_VMCR_EOIM_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>
>> You've ignored my comments again: "What if userspace writes something
>> that is incompatible with the current configuration? Wrong number of ID
>> bits, or number of priorities?"
> 
> IMO, In case of incompatibility,
> If ID bits and PRI bits are less than HW supported, it is ok.

Yes. But you also need to track of what the guest has programmed in
order to be able to migrate it back to its original configuration.

> If ID bits and PRI bits are greater than HW supported, then warn would be good
> enough. Please suggest the behaviour that you think it should be.

No, it is an error, plain and simple. You cannot run in this condition.

> 
>>
>>> + } else {
>>> + ich_vtr = kvm_call_hyp(__vgic_v3_get_ich_vtr_el2);
>>> +
>>> + val = 0;
>>> + val |= ((ich_vtr & ICH_VTR_PRI_BITS_MASK) >>
>>> + ICH_VTR_PRI_BITS_SHIFT) << 
>>> ICC_CTLR_EL1_PRI_BITS_SHIFT;
>>> + val |= ((ich_vtr & ICH_VTR_ID_BITS_MASK) >>
>>> + ICH_VTR_ID_BITS_SHIFT) << ICC_CTLR_EL1_ID_BITS_SHIFT;
>>> + val |= ((ich_vtr & ICH_VTR_SEIS_MASK) >> ICH_VTR_SEIS_SHIFT)
>>> + << ICC_CTLR_EL1_SEIS_SHIFT;
>>> + val |= ((ich_vtr & ICH_VTR_A3V_MASK) >> ICH_VTR_A3V_SHIFT)
>>> + << ICC_CTLR_EL1_A3V_SHIFT;
>>> + val |= ((vmcr.ctlr & ICH_VMCR_CBPR_MASK) >>
>>> + ICH_VMCR_CBPR_SHIFT) << ICC_CTLR_EL1_CBPR_SHIFT;
>>> + val |= ((vmcr.ctlr & ICH_VMCR_EOIM_MASK) >>
>>> + ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT;
>>> +
>>> + p->regval = val;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_pmr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>>> +const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.pmr = (p->regval & ICC_PMR_EL1_MASK) >> 
>>> ICC_PMR_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->regval = (vmcr.pmr << ICC_PMR_EL1_SHIFT) & 
>>> ICC_PMR_EL1_MASK;
>>> + }
>>> +
>>> + return true;
>>> +}
>>> +
>>> +static bool access_gic_bpr0(struct kvm_vcpu *vcpu, struct sys_reg_params 
>>> *p,
>>> + const struct sys_reg_desc *r)
>>> +{
>>> + struct vgic_vmcr vmcr;
>>> +
>>> + vgic_get_vmcr(vcpu, &vmcr);
>>> + if (p->is_write) {
>>> + vmcr.bpr = (p->regval & ICC_BPR0_EL1_MASK) >>
>>> + ICC_BPR0_EL1_SHIFT;
>>> + vgic_set_vmcr(vcpu, &vmcr);
>>> + } else {
>>> + p->reg

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

2016-09-16 Thread Vijay Kilari
On Fri, Sep 16, 2016 at 8:06 PM, Marc Zyngier  wrote:
> On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
>>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
>>  virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
>>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
>> 
>>  virt/kvm/arm/vgic/vgic.h|  10 ++
>>  7 files changed, 385 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/linux/irqchip/arm-gic-v3.h 
>> b/include/linux/irqchip/arm-gic-v3.h
>> index 88d83d3..d4e9c7d 100644
>> --- a/include/linux/irqchip/arm-gic-v3.h
>> +++ b/include/linux/irqchip/arm-gic-v3.h
>> @@ -355,6 +355,27 @@
>>  #define ICC_CTLR_EL1_EOImode_SHIFT   (1)
>>  #define ICC_CTLR_EL1_EOImode_drop_dir(0U << 
>> ICC_CTLR_EL1_EOImode_SHIFT)
>>  #define ICC_CTLR_EL1_EOImode_drop(1U << ICC_CTLR_EL1_EOImode_SHIFT)
>> +#define ICC_CTLR_EL1_EOImode_MASK(1 << ICC_CTLR_EL1_EOImode_SHIFT)
>> +#define ICC_CTLR_EL1_CBPR_SHIFT  (0)
>> +#define ICC_CTLR_EL1_CBPR_MASK   (1 << ICC_CTLR_EL1_CBPR_SHIFT)
>> +#define ICC_CTLR_EL1_PRI_BITS_SHIFT  (8)
>> +#define ICC_CTLR_EL1_PRI_BITS_MASK   (0x7 << ICC_CTLR_EL1_PRI_BITS_SHIFT)
>> +#define ICC_CTLR_EL1_ID_BITS_SHIFT   (11)
>> +#define ICC_CTLR_EL1_ID_BITS_MASK(0x7 << ICC_CTLR_EL1_ID_BITS_SHIFT)
>> +#define ICC_CTLR_EL1_SEIS_SHIFT  (14)
>> +#define ICC_CTLR_EL1_SEIS_MASK   (0x1 << 
>> ICC_CTLR_EL1_SEIS_SHIFT)
>> +#define ICC_CTLR_EL1_A3V_SHIFT   (15)
>> +#define ICC_CTLR_EL1_A3V_MASK(0x1 << ICC_CTLR_EL1_A3V_SHIFT)
>> +#define ICC_PMR_EL1_SHIFT(0)
>> +#define ICC_PMR_EL1_MASK (0xff << ICC_PMR_EL1_SHIFT)
>> +#define ICC_BPR0_EL1_SHIFT   (0)
>> +#define ICC_BPR0_EL1_MASK(0x7 << ICC_BPR0_EL1_SHIFT)
>> +#define ICC_BPR1_EL1_SHIFT   (0)
>> +#define ICC_BPR1_EL1_MASK(0x7 << ICC_BPR1_EL1_SHIFT)
>> +#define ICC_IGRPEN0_EL1_SHIFT(0)
>> +#define ICC_IGRPEN0_EL1_MASK (1 << ICC_IGRPEN0_EL1_SHIFT)
>> +#define ICC_IGRPEN1_EL1_SHIFT(0)
>> +#define ICC_IGRPEN1_EL1_MASK (1 << ICC_IGRPEN1_EL1_SHIFT)
>>  #define ICC_SRE_EL1_SRE  (1U << 0)
>>
>>  /*
>> @@ -398,6 +419,15 @@
>>  #define ICH_VMCR_ENG1_SHIFT  1
>>  #define ICH_VMCR_ENG1_MASK   (1 << ICH_VMCR_ENG1_SHIFT)
>>
>> +#define ICH_VTR_PRI_BITS_SHIFT   29
>> +#define ICH_VTR_PRI_BITS_MASK(7 << ICH_VTR_PRI_BITS_SHIFT)
>> +#define ICH_VTR_ID_BITS_SHIFT23
>> +#define ICH_VTR_ID_BITS_MASK (7 << ICH_VTR_ID_BITS_SHIFT)
>> +#define ICH_VTR_SEIS_SHIFT   22
>> +#define ICH_VTR_SEIS_MASK(1 << ICH_VTR_SEIS_SHIFT)
>> +#define ICH_VTR_A3V_SHIFT21
>> +#define ICH_VTR_A3V_MASK (1 << ICH_VTR_A3V_SHIFT)
>> +
>
> Same thing here. Please group all the additions to this file into a
> single, independent patch.
>
>>  #de

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

2016-09-16 Thread Marc Zyngier
On 16/09/16 13:20, 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/linux/irqchip/arm-gic-v3.h  |  30 
>  virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
>  virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
>  virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
> 
>  virt/kvm/arm/vgic/vgic.h|  10 ++
>  7 files changed, 385 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/linux/irqchip/arm-gic-v3.h 
> b/include/linux/irqchip/arm-gic-v3.h
> index 88d83d3..d4e9c7d 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -355,6 +355,27 @@
>  #define ICC_CTLR_EL1_EOImode_SHIFT   (1)
>  #define ICC_CTLR_EL1_EOImode_drop_dir(0U << 
> ICC_CTLR_EL1_EOImode_SHIFT)
>  #define ICC_CTLR_EL1_EOImode_drop(1U << ICC_CTLR_EL1_EOImode_SHIFT)
> +#define ICC_CTLR_EL1_EOImode_MASK(1 << ICC_CTLR_EL1_EOImode_SHIFT)
> +#define ICC_CTLR_EL1_CBPR_SHIFT  (0)
> +#define ICC_CTLR_EL1_CBPR_MASK   (1 << ICC_CTLR_EL1_CBPR_SHIFT)
> +#define ICC_CTLR_EL1_PRI_BITS_SHIFT  (8)
> +#define ICC_CTLR_EL1_PRI_BITS_MASK   (0x7 << ICC_CTLR_EL1_PRI_BITS_SHIFT)
> +#define ICC_CTLR_EL1_ID_BITS_SHIFT   (11)
> +#define ICC_CTLR_EL1_ID_BITS_MASK(0x7 << ICC_CTLR_EL1_ID_BITS_SHIFT)
> +#define ICC_CTLR_EL1_SEIS_SHIFT  (14)
> +#define ICC_CTLR_EL1_SEIS_MASK   (0x1 << ICC_CTLR_EL1_SEIS_SHIFT)
> +#define ICC_CTLR_EL1_A3V_SHIFT   (15)
> +#define ICC_CTLR_EL1_A3V_MASK(0x1 << ICC_CTLR_EL1_A3V_SHIFT)
> +#define ICC_PMR_EL1_SHIFT(0)
> +#define ICC_PMR_EL1_MASK (0xff << ICC_PMR_EL1_SHIFT)
> +#define ICC_BPR0_EL1_SHIFT   (0)
> +#define ICC_BPR0_EL1_MASK(0x7 << ICC_BPR0_EL1_SHIFT)
> +#define ICC_BPR1_EL1_SHIFT   (0)
> +#define ICC_BPR1_EL1_MASK(0x7 << ICC_BPR1_EL1_SHIFT)
> +#define ICC_IGRPEN0_EL1_SHIFT(0)
> +#define ICC_IGRPEN0_EL1_MASK (1 << ICC_IGRPEN0_EL1_SHIFT)
> +#define ICC_IGRPEN1_EL1_SHIFT(0)
> +#define ICC_IGRPEN1_EL1_MASK (1 << ICC_IGRPEN1_EL1_SHIFT)
>  #define ICC_SRE_EL1_SRE  (1U << 0)
>  
>  /*
> @@ -398,6 +419,15 @@
>  #define ICH_VMCR_ENG1_SHIFT  1
>  #define ICH_VMCR_ENG1_MASK   (1 << ICH_VMCR_ENG1_SHIFT)
>  
> +#define ICH_VTR_PRI_BITS_SHIFT   29
> +#define ICH_VTR_PRI_BITS_MASK(7 << ICH_VTR_PRI_BITS_SHIFT)
> +#define ICH_VTR_ID_BITS_SHIFT23
> +#define ICH_VTR_ID_BITS_MASK (7 << ICH_VTR_ID_BITS_SHIFT)
> +#define ICH_VTR_SEIS_SHIFT   22
> +#define ICH_VTR_SEIS_MASK(1 << ICH_VTR_SEIS_SHIFT)
> +#define ICH_VTR_A3V_SHIFT21
> +#define ICH_VTR_A3V_MASK (1 << ICH_VTR_A3V_SHIFT)
> +

Same thing here. Please group all the additions to this file into a
single, independent patch.

>  #define ICC_IAR1_EL1_SPURIOUS0x3ff
>  
>  #define ICC_SRE_EL2_SRE  (1 << 0)
> diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 

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

2016-09-16 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/linux/irqchip/arm-gic-v3.h  |  30 
 virt/kvm/arm/vgic/vgic-kvm-device.c |  27 
 virt/kvm/arm/vgic/vgic-mmio-v3.c|  18 +++
 virt/kvm/arm/vgic/vgic-sys-reg-v3.c | 296 
 virt/kvm/arm/vgic/vgic.h|  10 ++
 7 files changed, 385 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/linux/irqchip/arm-gic-v3.h 
b/include/linux/irqchip/arm-gic-v3.h
index 88d83d3..d4e9c7d 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -355,6 +355,27 @@
 #define ICC_CTLR_EL1_EOImode_SHIFT (1)
 #define ICC_CTLR_EL1_EOImode_drop_dir  (0U << ICC_CTLR_EL1_EOImode_SHIFT)
 #define ICC_CTLR_EL1_EOImode_drop  (1U << ICC_CTLR_EL1_EOImode_SHIFT)
+#define ICC_CTLR_EL1_EOImode_MASK  (1 << ICC_CTLR_EL1_EOImode_SHIFT)
+#define ICC_CTLR_EL1_CBPR_SHIFT(0)
+#define ICC_CTLR_EL1_CBPR_MASK (1 << ICC_CTLR_EL1_CBPR_SHIFT)
+#define ICC_CTLR_EL1_PRI_BITS_SHIFT(8)
+#define ICC_CTLR_EL1_PRI_BITS_MASK (0x7 << ICC_CTLR_EL1_PRI_BITS_SHIFT)
+#define ICC_CTLR_EL1_ID_BITS_SHIFT (11)
+#define ICC_CTLR_EL1_ID_BITS_MASK  (0x7 << ICC_CTLR_EL1_ID_BITS_SHIFT)
+#define ICC_CTLR_EL1_SEIS_SHIFT(14)
+#define ICC_CTLR_EL1_SEIS_MASK (0x1 << ICC_CTLR_EL1_SEIS_SHIFT)
+#define ICC_CTLR_EL1_A3V_SHIFT (15)
+#define ICC_CTLR_EL1_A3V_MASK  (0x1 << ICC_CTLR_EL1_A3V_SHIFT)
+#define ICC_PMR_EL1_SHIFT  (0)
+#define ICC_PMR_EL1_MASK   (0xff << ICC_PMR_EL1_SHIFT)
+#define ICC_BPR0_EL1_SHIFT (0)
+#define ICC_BPR0_EL1_MASK  (0x7 << ICC_BPR0_EL1_SHIFT)
+#define ICC_BPR1_EL1_SHIFT (0)
+#define ICC_BPR1_EL1_MASK  (0x7 << ICC_BPR1_EL1_SHIFT)
+#define ICC_IGRPEN0_EL1_SHIFT  (0)
+#define ICC_IGRPEN0_EL1_MASK   (1 << ICC_IGRPEN0_EL1_SHIFT)
+#define ICC_IGRPEN1_EL1_SHIFT  (0)
+#define ICC_IGRPEN1_EL1_MASK   (1 << ICC_IGRPEN1_EL1_SHIFT)
 #define ICC_SRE_EL1_SRE(1U << 0)
 
 /*
@@ -398,6 +419,15 @@
 #define ICH_VMCR_ENG1_SHIFT1
 #define ICH_VMCR_ENG1_MASK (1 << ICH_VMCR_ENG1_SHIFT)
 
+#define ICH_VTR_PRI_BITS_SHIFT 29
+#define ICH_VTR_PRI_BITS_MASK  (7 << ICH_VTR_PRI_BITS_SHIFT)
+#define ICH_VTR_ID_BITS_SHIFT  23
+#define ICH_VTR_ID_BITS_MASK   (7 << ICH_VTR_ID_BITS_SHIFT)
+#define ICH_VTR_SEIS_SHIFT 22
+#define ICH_VTR_SEIS_MASK  (1 << ICH_VTR_SEIS_SHIFT)
+#define ICH_VTR_A3V_SHIFT  21
+#define ICH_VTR_A3V_MASK   (1 << ICH_VTR_A3V_SHIFT)
+
 #define ICC_IAR1_EL1_SPURIOUS  0x3ff
 
 #define ICC_SRE_EL2_SRE(1 << 0)
diff --git a/virt/kvm/arm/vgic/vgic-kvm-device.c 
b/virt/kvm/arm/vgic/vgic-kvm-device.c
index a4656fc..a48101f 100644
--- a/virt/kvm/arm/vgic/vgic-kvm-device.c
+++ b/virt/kvm/arm/vgic/vgic-kvm-device.c
@@ -506,6 +506,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