| Message ID | 1479906118-15832-6-git-send-email-vijay.kilari@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Nov 23, 2016 at 06:31:52PM +0530, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > > ICC_VMCR_EL2 supports virtual access to ICC_IGRPEN1_EL1.Enable > and ICC_IGRPEN0_EL1.Enable fields. Add grpen0 and grpen1 member > variables to struct vmcr to support read and write of these fields. > > Also refactor vgic_set_vmcr and vgic_get_vmcr() code. > Drop ICH_VMCR_CTLR_SHIFT and ICH_VMCR_CTLR_MASK macros and instead > use ICH_VMCR_EOI* and ICH_VMCR_CBPR* macros > . > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > --- > include/linux/irqchip/arm-gic-v3.h | 2 -- > virt/kvm/arm/vgic/vgic-mmio-v2.c | 16 ---------------- > virt/kvm/arm/vgic/vgic-mmio.c | 16 ++++++++++++++++ > virt/kvm/arm/vgic/vgic-v3.c | 22 ++++++++++++++++++++-- > virt/kvm/arm/vgic/vgic.h | 5 +++++ > 5 files changed, 41 insertions(+), 20 deletions(-) > > diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h > index b4f8287..406fc3e 100644 > --- a/include/linux/irqchip/arm-gic-v3.h > +++ b/include/linux/irqchip/arm-gic-v3.h > @@ -404,8 +404,6 @@ > #define ICH_HCR_EN (1 << 0) > #define ICH_HCR_UIE (1 << 1) > > -#define ICH_VMCR_CTLR_SHIFT 0 > -#define ICH_VMCR_CTLR_MASK (0x21f << ICH_VMCR_CTLR_SHIFT) > #define ICH_VMCR_CBPR_SHIFT 4 > #define ICH_VMCR_CBPR_MASK (1 << ICH_VMCR_CBPR_SHIFT) > #define ICH_VMCR_EOIM_SHIFT 9 > diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c > index 2cb04b7..ad353b5 100644 > --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c > +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c > @@ -212,22 +212,6 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu, > } > } > > -static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > -{ > - if (kvm_vgic_global_state.type == VGIC_V2) > - vgic_v2_set_vmcr(vcpu, vmcr); > - else > - vgic_v3_set_vmcr(vcpu, vmcr); > -} > - > -static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > -{ > - if (kvm_vgic_global_state.type == VGIC_V2) > - vgic_v2_get_vmcr(vcpu, vmcr); > - else > - vgic_v3_get_vmcr(vcpu, vmcr); > -} > - > #define GICC_ARCH_VERSION_V2 0x2 > > /* These are for userland accesses only, there is no guest-facing emulation. */ > diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c > index 0d1bc98..f81e0e5 100644 > --- a/virt/kvm/arm/vgic/vgic-mmio.c > +++ b/virt/kvm/arm/vgic/vgic-mmio.c > @@ -416,6 +416,22 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev, > return -ENXIO; > } > > +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > +{ > + if (kvm_vgic_global_state.type == VGIC_V2) > + vgic_v2_set_vmcr(vcpu, vmcr); > + else > + vgic_v3_set_vmcr(vcpu, vmcr); > +} > + > +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > +{ > + if (kvm_vgic_global_state.type == VGIC_V2) > + vgic_v2_get_vmcr(vcpu, vmcr); > + else > + vgic_v3_get_vmcr(vcpu, vmcr); > +} > + > /* > * kvm_mmio_read_buf() returns a value in a format where it can be converted > * to a byte array and be directly observed as the guest wanted it to appear > diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c > index 9f0dae3..a3ff04b 100644 > --- a/virt/kvm/arm/vgic/vgic-v3.c > +++ b/virt/kvm/arm/vgic/vgic-v3.c > @@ -175,10 +175,19 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) > { > u32 vmcr; > > - vmcr = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & ICH_VMCR_CTLR_MASK; > + /* > + * Ignore the FIQen bit, because GIC emulation always implies > + * SRE=1 which means the vFIQEn bit is also RES1. > + */ > + vmcr = (vmcrp->ctlr & ICC_CTLR_EL1_EOImode_MASK) >> > + ICC_CTLR_EL1_EOImode_SHIFT; > + vmcr = (vmcr << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; Nit: I think this can be written more nicely as: vmcr = ((vmcrp->ctlr >> ICC_CTLR_EL1_EOImode_SHIFT) << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; > + vmcr |= (vmcrp->ctlr << ICH_VMCR_CBPR_SHIFT) & ICH_VMCR_CBPR_MASK; > vmcr |= (vmcrp->abpr << ICH_VMCR_BPR1_SHIFT) & ICH_VMCR_BPR1_MASK; > vmcr |= (vmcrp->bpr << ICH_VMCR_BPR0_SHIFT) & ICH_VMCR_BPR0_MASK; > vmcr |= (vmcrp->pmr << ICH_VMCR_PMR_SHIFT) & ICH_VMCR_PMR_MASK; > + vmcr |= (vmcrp->grpen0 << ICH_VMCR_ENG0_SHIFT) & ICH_VMCR_ENG0_MASK; > + vmcr |= (vmcrp->grpen1 << ICH_VMCR_ENG1_SHIFT) & ICH_VMCR_ENG1_MASK; > > vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr = vmcr; > } > @@ -187,10 +196,19 @@ void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) > { > u32 vmcr = vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr; > > - vmcrp->ctlr = (vmcr & ICH_VMCR_CTLR_MASK) >> ICH_VMCR_CTLR_SHIFT; > + /* > + * Ignore the FIQen bit, because GIC emulation always implies > + * SRE=1 which means the vFIQEn bit is also RES1. > + */ > + vmcrp->ctlr = (vmcr & ICH_VMCR_EOIM_MASK) >> ICH_VMCR_EOIM_SHIFT; > + vmcrp->ctlr = (vmcrp->ctlr << ICC_CTLR_EL1_EOImode_SHIFT) & > + ICC_CTLR_EL1_EOImode_MASK; similarly, this could be written as: vmcrp->ctlr = ((vmcr >> ICH_VMCR_EOIM_SHIFT) << ICC_CTLR_EL1_EOImode_SHIFT) & ICC_CTLR_EL1_EOImode_MASK; > + vmcrp->ctlr |= (vmcr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT; > vmcrp->abpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT; > vmcrp->bpr = (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT; > vmcrp->pmr = (vmcr & ICH_VMCR_PMR_MASK) >> ICH_VMCR_PMR_SHIFT; > + vmcrp->grpen0 = (vmcr & ICH_VMCR_ENG0_MASK) >> ICH_VMCR_ENG0_SHIFT; > + vmcrp->grpen1 = (vmcr & ICH_VMCR_ENG1_MASK) >> ICH_VMCR_ENG1_SHIFT; > } > > #define INITIAL_PENDBASER_VALUE \ > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h > index 91f58b2..9232791 100644 > --- a/virt/kvm/arm/vgic/vgic.h > +++ b/virt/kvm/arm/vgic/vgic.h > @@ -78,6 +78,9 @@ struct vgic_vmcr { > u32 abpr; > u32 bpr; > u32 pmr; > + /* Below member variable are valid only for GICv3 */ > + u32 grpen0; > + u32 grpen1; > }; > > struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu, > @@ -138,6 +141,8 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write, > int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write, > int offset, u32 *val); > int kvm_register_vgic_device(unsigned long type); > +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); > int vgic_lazy_init(struct kvm *kvm); > int vgic_init(struct kvm *kvm); > > -- > 1.9.1 > My comments on style above notwithstanding: Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Vijay, On 28/11/2016 15:28, Christoffer Dall wrote: > On Wed, Nov 23, 2016 at 06:31:52PM +0530, vijay.kilari@gmail.com wrote: >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> >> ICC_VMCR_EL2 supports virtual access to ICC_IGRPEN1_EL1.Enable >> and ICC_IGRPEN0_EL1.Enable fields. Add grpen0 and grpen1 member >> variables to struct vmcr to support read and write of these fields. >> >> Also refactor vgic_set_vmcr and vgic_get_vmcr() code. >> Drop ICH_VMCR_CTLR_SHIFT and ICH_VMCR_CTLR_MASK macros and instead >> use ICH_VMCR_EOI* and ICH_VMCR_CBPR* macros >> . >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> --- >> include/linux/irqchip/arm-gic-v3.h | 2 -- >> virt/kvm/arm/vgic/vgic-mmio-v2.c | 16 ---------------- >> virt/kvm/arm/vgic/vgic-mmio.c | 16 ++++++++++++++++ >> virt/kvm/arm/vgic/vgic-v3.c | 22 ++++++++++++++++++++-- >> virt/kvm/arm/vgic/vgic.h | 5 +++++ >> 5 files changed, 41 insertions(+), 20 deletions(-) >> >> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h >> index b4f8287..406fc3e 100644 >> --- a/include/linux/irqchip/arm-gic-v3.h >> +++ b/include/linux/irqchip/arm-gic-v3.h >> @@ -404,8 +404,6 @@ >> #define ICH_HCR_EN (1 << 0) >> #define ICH_HCR_UIE (1 << 1) >> >> -#define ICH_VMCR_CTLR_SHIFT 0 >> -#define ICH_VMCR_CTLR_MASK (0x21f << ICH_VMCR_CTLR_SHIFT) >> #define ICH_VMCR_CBPR_SHIFT 4 >> #define ICH_VMCR_CBPR_MASK (1 << ICH_VMCR_CBPR_SHIFT) >> #define ICH_VMCR_EOIM_SHIFT 9 >> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c >> index 2cb04b7..ad353b5 100644 >> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c >> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c >> @@ -212,22 +212,6 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu, >> } >> } >> >> -static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >> -{ >> - if (kvm_vgic_global_state.type == VGIC_V2) >> - vgic_v2_set_vmcr(vcpu, vmcr); >> - else >> - vgic_v3_set_vmcr(vcpu, vmcr); >> -} >> - >> -static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >> -{ >> - if (kvm_vgic_global_state.type == VGIC_V2) >> - vgic_v2_get_vmcr(vcpu, vmcr); >> - else >> - vgic_v3_get_vmcr(vcpu, vmcr); >> -} >> - >> #define GICC_ARCH_VERSION_V2 0x2 >> >> /* These are for userland accesses only, there is no guest-facing emulation. */ >> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c >> index 0d1bc98..f81e0e5 100644 >> --- a/virt/kvm/arm/vgic/vgic-mmio.c >> +++ b/virt/kvm/arm/vgic/vgic-mmio.c >> @@ -416,6 +416,22 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev, >> return -ENXIO; >> } >> >> +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >> +{ >> + if (kvm_vgic_global_state.type == VGIC_V2) >> + vgic_v2_set_vmcr(vcpu, vmcr); >> + else >> + vgic_v3_set_vmcr(vcpu, vmcr); >> +} >> + >> +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >> +{ >> + if (kvm_vgic_global_state.type == VGIC_V2) >> + vgic_v2_get_vmcr(vcpu, vmcr); >> + else >> + vgic_v3_get_vmcr(vcpu, vmcr); >> +} >> + >> /* >> * kvm_mmio_read_buf() returns a value in a format where it can be converted >> * to a byte array and be directly observed as the guest wanted it to appear >> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c >> index 9f0dae3..a3ff04b 100644 >> --- a/virt/kvm/arm/vgic/vgic-v3.c >> +++ b/virt/kvm/arm/vgic/vgic-v3.c >> @@ -175,10 +175,19 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) >> { >> u32 vmcr; >> >> - vmcr = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & ICH_VMCR_CTLR_MASK; >> + /* >> + * Ignore the FIQen bit, because GIC emulation always implies >> + * SRE=1 which means the vFIQEn bit is also RES1. >> + */ >> + vmcr = (vmcrp->ctlr & ICC_CTLR_EL1_EOImode_MASK) >> >> + ICC_CTLR_EL1_EOImode_SHIFT; >> + vmcr = (vmcr << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; I am not able to understand why we use ICC_CTLR _*macros here? Please could you explain it to me? Besides if we want to ignore the FIQen bit can't we change the ICH_VMCR_CTLR_MASK value? Thanks Eric > > Nit: I think this can be written more nicely as: > vmcr = ((vmcrp->ctlr >> ICC_CTLR_EL1_EOImode_SHIFT) > << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; > >> + vmcr |= (vmcrp->ctlr << ICH_VMCR_CBPR_SHIFT) & ICH_VMCR_CBPR_MASK; >> vmcr |= (vmcrp->abpr << ICH_VMCR_BPR1_SHIFT) & ICH_VMCR_BPR1_MASK; >> vmcr |= (vmcrp->bpr << ICH_VMCR_BPR0_SHIFT) & ICH_VMCR_BPR0_MASK; >> vmcr |= (vmcrp->pmr << ICH_VMCR_PMR_SHIFT) & ICH_VMCR_PMR_MASK; >> + vmcr |= (vmcrp->grpen0 << ICH_VMCR_ENG0_SHIFT) & ICH_VMCR_ENG0_MASK; >> + vmcr |= (vmcrp->grpen1 << ICH_VMCR_ENG1_SHIFT) & ICH_VMCR_ENG1_MASK; >> >> vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr = vmcr; >> } >> @@ -187,10 +196,19 @@ void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) >> { >> u32 vmcr = vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr; >> >> - vmcrp->ctlr = (vmcr & ICH_VMCR_CTLR_MASK) >> ICH_VMCR_CTLR_SHIFT; >> + /* >> + * Ignore the FIQen bit, because GIC emulation always implies >> + * SRE=1 which means the vFIQEn bit is also RES1. >> + */ >> + vmcrp->ctlr = (vmcr & ICH_VMCR_EOIM_MASK) >> ICH_VMCR_EOIM_SHIFT; >> + vmcrp->ctlr = (vmcrp->ctlr << ICC_CTLR_EL1_EOImode_SHIFT) & >> + ICC_CTLR_EL1_EOImode_MASK; > > similarly, this could be written as: > vmcrp->ctlr = ((vmcr >> ICH_VMCR_EOIM_SHIFT) << > ICC_CTLR_EL1_EOImode_SHIFT) & ICC_CTLR_EL1_EOImode_MASK; > >> + vmcrp->ctlr |= (vmcr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT; >> vmcrp->abpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT; >> vmcrp->bpr = (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT; >> vmcrp->pmr = (vmcr & ICH_VMCR_PMR_MASK) >> ICH_VMCR_PMR_SHIFT; >> + vmcrp->grpen0 = (vmcr & ICH_VMCR_ENG0_MASK) >> ICH_VMCR_ENG0_SHIFT; >> + vmcrp->grpen1 = (vmcr & ICH_VMCR_ENG1_MASK) >> ICH_VMCR_ENG1_SHIFT; >> } >> >> #define INITIAL_PENDBASER_VALUE \ >> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h >> index 91f58b2..9232791 100644 >> --- a/virt/kvm/arm/vgic/vgic.h >> +++ b/virt/kvm/arm/vgic/vgic.h >> @@ -78,6 +78,9 @@ struct vgic_vmcr { >> u32 abpr; >> u32 bpr; >> u32 pmr; >> + /* Below member variable are valid only for GICv3 */ >> + u32 grpen0; >> + u32 grpen1; >> }; >> >> struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu, >> @@ -138,6 +141,8 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write, >> int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write, >> int offset, u32 *val); >> int kvm_register_vgic_device(unsigned long type); >> +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> int vgic_lazy_init(struct kvm *kvm); >> int vgic_init(struct kvm *kvm); >> >> -- >> 1.9.1 >> > My comments on style above notwithstanding: > > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Dec 08, 2016 at 12:52:39PM +0100, Auger Eric wrote: > Hi Vijay, > > On 28/11/2016 15:28, Christoffer Dall wrote: > > On Wed, Nov 23, 2016 at 06:31:52PM +0530, vijay.kilari@gmail.com wrote: > >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > >> > >> ICC_VMCR_EL2 supports virtual access to ICC_IGRPEN1_EL1.Enable > >> and ICC_IGRPEN0_EL1.Enable fields. Add grpen0 and grpen1 member > >> variables to struct vmcr to support read and write of these fields. > >> > >> Also refactor vgic_set_vmcr and vgic_get_vmcr() code. > >> Drop ICH_VMCR_CTLR_SHIFT and ICH_VMCR_CTLR_MASK macros and instead > >> use ICH_VMCR_EOI* and ICH_VMCR_CBPR* macros > >> . > >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > >> --- > >> include/linux/irqchip/arm-gic-v3.h | 2 -- > >> virt/kvm/arm/vgic/vgic-mmio-v2.c | 16 ---------------- > >> virt/kvm/arm/vgic/vgic-mmio.c | 16 ++++++++++++++++ > >> virt/kvm/arm/vgic/vgic-v3.c | 22 ++++++++++++++++++++-- > >> virt/kvm/arm/vgic/vgic.h | 5 +++++ > >> 5 files changed, 41 insertions(+), 20 deletions(-) > >> > >> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h > >> index b4f8287..406fc3e 100644 > >> --- a/include/linux/irqchip/arm-gic-v3.h > >> +++ b/include/linux/irqchip/arm-gic-v3.h > >> @@ -404,8 +404,6 @@ > >> #define ICH_HCR_EN (1 << 0) > >> #define ICH_HCR_UIE (1 << 1) > >> > >> -#define ICH_VMCR_CTLR_SHIFT 0 > >> -#define ICH_VMCR_CTLR_MASK (0x21f << ICH_VMCR_CTLR_SHIFT) > >> #define ICH_VMCR_CBPR_SHIFT 4 > >> #define ICH_VMCR_CBPR_MASK (1 << ICH_VMCR_CBPR_SHIFT) > >> #define ICH_VMCR_EOIM_SHIFT 9 > >> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c > >> index 2cb04b7..ad353b5 100644 > >> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c > >> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c > >> @@ -212,22 +212,6 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu, > >> } > >> } > >> > >> -static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >> -{ > >> - if (kvm_vgic_global_state.type == VGIC_V2) > >> - vgic_v2_set_vmcr(vcpu, vmcr); > >> - else > >> - vgic_v3_set_vmcr(vcpu, vmcr); > >> -} > >> - > >> -static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >> -{ > >> - if (kvm_vgic_global_state.type == VGIC_V2) > >> - vgic_v2_get_vmcr(vcpu, vmcr); > >> - else > >> - vgic_v3_get_vmcr(vcpu, vmcr); > >> -} > >> - > >> #define GICC_ARCH_VERSION_V2 0x2 > >> > >> /* These are for userland accesses only, there is no guest-facing emulation. */ > >> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c > >> index 0d1bc98..f81e0e5 100644 > >> --- a/virt/kvm/arm/vgic/vgic-mmio.c > >> +++ b/virt/kvm/arm/vgic/vgic-mmio.c > >> @@ -416,6 +416,22 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev, > >> return -ENXIO; > >> } > >> > >> +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >> +{ > >> + if (kvm_vgic_global_state.type == VGIC_V2) > >> + vgic_v2_set_vmcr(vcpu, vmcr); > >> + else > >> + vgic_v3_set_vmcr(vcpu, vmcr); > >> +} > >> + > >> +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >> +{ > >> + if (kvm_vgic_global_state.type == VGIC_V2) > >> + vgic_v2_get_vmcr(vcpu, vmcr); > >> + else > >> + vgic_v3_get_vmcr(vcpu, vmcr); > >> +} > >> + > >> /* > >> * kvm_mmio_read_buf() returns a value in a format where it can be converted > >> * to a byte array and be directly observed as the guest wanted it to appear > >> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c > >> index 9f0dae3..a3ff04b 100644 > >> --- a/virt/kvm/arm/vgic/vgic-v3.c > >> +++ b/virt/kvm/arm/vgic/vgic-v3.c > >> @@ -175,10 +175,19 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) > >> { > >> u32 vmcr; > >> > >> - vmcr = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & ICH_VMCR_CTLR_MASK; > >> + /* > >> + * Ignore the FIQen bit, because GIC emulation always implies > >> + * SRE=1 which means the vFIQEn bit is also RES1. > >> + */ > >> + vmcr = (vmcrp->ctlr & ICC_CTLR_EL1_EOImode_MASK) >> > >> + ICC_CTLR_EL1_EOImode_SHIFT; > >> + vmcr = (vmcr << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; > I am not able to understand why we use ICC_CTLR _*macros here? Please > could you explain it to me? Besides if we want to ignore the FIQen bit > can't we change the ICH_VMCR_CTLR_MASK value? > This first statement is setting the vmcr to the ctlr's bit[1], but placed in bit[0], and then the next statement is moving that bit value to the corresponding place in the vmcr. I think this is correct (although a little opaque). There's also a newer series on the list, just so you know. Thanks, -Christoffer _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Christoffer, On 08/12/2016 13:21, Christoffer Dall wrote: > On Thu, Dec 08, 2016 at 12:52:39PM +0100, Auger Eric wrote: >> Hi Vijay, >> >> On 28/11/2016 15:28, Christoffer Dall wrote: >>> On Wed, Nov 23, 2016 at 06:31:52PM +0530, vijay.kilari@gmail.com wrote: >>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >>>> >>>> ICC_VMCR_EL2 supports virtual access to ICC_IGRPEN1_EL1.Enable >>>> and ICC_IGRPEN0_EL1.Enable fields. Add grpen0 and grpen1 member >>>> variables to struct vmcr to support read and write of these fields. >>>> >>>> Also refactor vgic_set_vmcr and vgic_get_vmcr() code. >>>> Drop ICH_VMCR_CTLR_SHIFT and ICH_VMCR_CTLR_MASK macros and instead >>>> use ICH_VMCR_EOI* and ICH_VMCR_CBPR* macros >>>> . >>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >>>> --- >>>> include/linux/irqchip/arm-gic-v3.h | 2 -- >>>> virt/kvm/arm/vgic/vgic-mmio-v2.c | 16 ---------------- >>>> virt/kvm/arm/vgic/vgic-mmio.c | 16 ++++++++++++++++ >>>> virt/kvm/arm/vgic/vgic-v3.c | 22 ++++++++++++++++++++-- >>>> virt/kvm/arm/vgic/vgic.h | 5 +++++ >>>> 5 files changed, 41 insertions(+), 20 deletions(-) >>>> >>>> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h >>>> index b4f8287..406fc3e 100644 >>>> --- a/include/linux/irqchip/arm-gic-v3.h >>>> +++ b/include/linux/irqchip/arm-gic-v3.h >>>> @@ -404,8 +404,6 @@ >>>> #define ICH_HCR_EN (1 << 0) >>>> #define ICH_HCR_UIE (1 << 1) >>>> >>>> -#define ICH_VMCR_CTLR_SHIFT 0 >>>> -#define ICH_VMCR_CTLR_MASK (0x21f << ICH_VMCR_CTLR_SHIFT) >>>> #define ICH_VMCR_CBPR_SHIFT 4 >>>> #define ICH_VMCR_CBPR_MASK (1 << ICH_VMCR_CBPR_SHIFT) >>>> #define ICH_VMCR_EOIM_SHIFT 9 >>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c >>>> index 2cb04b7..ad353b5 100644 >>>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c >>>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c >>>> @@ -212,22 +212,6 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu, >>>> } >>>> } >>>> >>>> -static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >>>> -{ >>>> - if (kvm_vgic_global_state.type == VGIC_V2) >>>> - vgic_v2_set_vmcr(vcpu, vmcr); >>>> - else >>>> - vgic_v3_set_vmcr(vcpu, vmcr); >>>> -} >>>> - >>>> -static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >>>> -{ >>>> - if (kvm_vgic_global_state.type == VGIC_V2) >>>> - vgic_v2_get_vmcr(vcpu, vmcr); >>>> - else >>>> - vgic_v3_get_vmcr(vcpu, vmcr); >>>> -} >>>> - >>>> #define GICC_ARCH_VERSION_V2 0x2 >>>> >>>> /* These are for userland accesses only, there is no guest-facing emulation. */ >>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c >>>> index 0d1bc98..f81e0e5 100644 >>>> --- a/virt/kvm/arm/vgic/vgic-mmio.c >>>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c >>>> @@ -416,6 +416,22 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev, >>>> return -ENXIO; >>>> } >>>> >>>> +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >>>> +{ >>>> + if (kvm_vgic_global_state.type == VGIC_V2) >>>> + vgic_v2_set_vmcr(vcpu, vmcr); >>>> + else >>>> + vgic_v3_set_vmcr(vcpu, vmcr); >>>> +} >>>> + >>>> +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) >>>> +{ >>>> + if (kvm_vgic_global_state.type == VGIC_V2) >>>> + vgic_v2_get_vmcr(vcpu, vmcr); >>>> + else >>>> + vgic_v3_get_vmcr(vcpu, vmcr); >>>> +} >>>> + >>>> /* >>>> * kvm_mmio_read_buf() returns a value in a format where it can be converted >>>> * to a byte array and be directly observed as the guest wanted it to appear >>>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c >>>> index 9f0dae3..a3ff04b 100644 >>>> --- a/virt/kvm/arm/vgic/vgic-v3.c >>>> +++ b/virt/kvm/arm/vgic/vgic-v3.c >>>> @@ -175,10 +175,19 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) >>>> { >>>> u32 vmcr; >>>> >>>> - vmcr = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & ICH_VMCR_CTLR_MASK; >>>> + /* >>>> + * Ignore the FIQen bit, because GIC emulation always implies >>>> + * SRE=1 which means the vFIQEn bit is also RES1. >>>> + */ >>>> + vmcr = (vmcrp->ctlr & ICC_CTLR_EL1_EOImode_MASK) >> >>>> + ICC_CTLR_EL1_EOImode_SHIFT; >>>> + vmcr = (vmcr << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; > >> I am not able to understand why we use ICC_CTLR _*macros here? Please >> could you explain it to me? Besides if we want to ignore the FIQen bit >> can't we change the ICH_VMCR_CTLR_MASK value? >> > This first statement is setting the vmcr to the ctlr's bit[1], but > placed in bit[0], and then the next statement is moving that bit value > to the corresponding place in the vmcr. I think this is correct > (although a little opaque). Yes the question was more about the semantic of the vmcrp->ctlr field. I thought it was supposed to store ICH_VMCR_EL2 ctrl bits as they are and not with a different layout. > > There's also a newer series on the list, just so you know. Argh OK I missed it. I will check the diffs in the AArch64 related patches. Thanks Eric > > Thanks, > -Christoffer > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Dec 08, 2016 at 01:50:43PM +0100, Auger Eric wrote: > Hi Christoffer, > > On 08/12/2016 13:21, Christoffer Dall wrote: > > On Thu, Dec 08, 2016 at 12:52:39PM +0100, Auger Eric wrote: > >> Hi Vijay, > >> > >> On 28/11/2016 15:28, Christoffer Dall wrote: > >>> On Wed, Nov 23, 2016 at 06:31:52PM +0530, vijay.kilari@gmail.com wrote: > >>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > >>>> > >>>> ICC_VMCR_EL2 supports virtual access to ICC_IGRPEN1_EL1.Enable > >>>> and ICC_IGRPEN0_EL1.Enable fields. Add grpen0 and grpen1 member > >>>> variables to struct vmcr to support read and write of these fields. > >>>> > >>>> Also refactor vgic_set_vmcr and vgic_get_vmcr() code. > >>>> Drop ICH_VMCR_CTLR_SHIFT and ICH_VMCR_CTLR_MASK macros and instead > >>>> use ICH_VMCR_EOI* and ICH_VMCR_CBPR* macros > >>>> . > >>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > >>>> --- > >>>> include/linux/irqchip/arm-gic-v3.h | 2 -- > >>>> virt/kvm/arm/vgic/vgic-mmio-v2.c | 16 ---------------- > >>>> virt/kvm/arm/vgic/vgic-mmio.c | 16 ++++++++++++++++ > >>>> virt/kvm/arm/vgic/vgic-v3.c | 22 ++++++++++++++++++++-- > >>>> virt/kvm/arm/vgic/vgic.h | 5 +++++ > >>>> 5 files changed, 41 insertions(+), 20 deletions(-) > >>>> > >>>> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h > >>>> index b4f8287..406fc3e 100644 > >>>> --- a/include/linux/irqchip/arm-gic-v3.h > >>>> +++ b/include/linux/irqchip/arm-gic-v3.h > >>>> @@ -404,8 +404,6 @@ > >>>> #define ICH_HCR_EN (1 << 0) > >>>> #define ICH_HCR_UIE (1 << 1) > >>>> > >>>> -#define ICH_VMCR_CTLR_SHIFT 0 > >>>> -#define ICH_VMCR_CTLR_MASK (0x21f << ICH_VMCR_CTLR_SHIFT) > >>>> #define ICH_VMCR_CBPR_SHIFT 4 > >>>> #define ICH_VMCR_CBPR_MASK (1 << ICH_VMCR_CBPR_SHIFT) > >>>> #define ICH_VMCR_EOIM_SHIFT 9 > >>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c > >>>> index 2cb04b7..ad353b5 100644 > >>>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c > >>>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c > >>>> @@ -212,22 +212,6 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu, > >>>> } > >>>> } > >>>> > >>>> -static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >>>> -{ > >>>> - if (kvm_vgic_global_state.type == VGIC_V2) > >>>> - vgic_v2_set_vmcr(vcpu, vmcr); > >>>> - else > >>>> - vgic_v3_set_vmcr(vcpu, vmcr); > >>>> -} > >>>> - > >>>> -static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >>>> -{ > >>>> - if (kvm_vgic_global_state.type == VGIC_V2) > >>>> - vgic_v2_get_vmcr(vcpu, vmcr); > >>>> - else > >>>> - vgic_v3_get_vmcr(vcpu, vmcr); > >>>> -} > >>>> - > >>>> #define GICC_ARCH_VERSION_V2 0x2 > >>>> > >>>> /* These are for userland accesses only, there is no guest-facing emulation. */ > >>>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c > >>>> index 0d1bc98..f81e0e5 100644 > >>>> --- a/virt/kvm/arm/vgic/vgic-mmio.c > >>>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c > >>>> @@ -416,6 +416,22 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev, > >>>> return -ENXIO; > >>>> } > >>>> > >>>> +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >>>> +{ > >>>> + if (kvm_vgic_global_state.type == VGIC_V2) > >>>> + vgic_v2_set_vmcr(vcpu, vmcr); > >>>> + else > >>>> + vgic_v3_set_vmcr(vcpu, vmcr); > >>>> +} > >>>> + > >>>> +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) > >>>> +{ > >>>> + if (kvm_vgic_global_state.type == VGIC_V2) > >>>> + vgic_v2_get_vmcr(vcpu, vmcr); > >>>> + else > >>>> + vgic_v3_get_vmcr(vcpu, vmcr); > >>>> +} > >>>> + > >>>> /* > >>>> * kvm_mmio_read_buf() returns a value in a format where it can be converted > >>>> * to a byte array and be directly observed as the guest wanted it to appear > >>>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c > >>>> index 9f0dae3..a3ff04b 100644 > >>>> --- a/virt/kvm/arm/vgic/vgic-v3.c > >>>> +++ b/virt/kvm/arm/vgic/vgic-v3.c > >>>> @@ -175,10 +175,19 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) > >>>> { > >>>> u32 vmcr; > >>>> > >>>> - vmcr = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & ICH_VMCR_CTLR_MASK; > >>>> + /* > >>>> + * Ignore the FIQen bit, because GIC emulation always implies > >>>> + * SRE=1 which means the vFIQEn bit is also RES1. > >>>> + */ > >>>> + vmcr = (vmcrp->ctlr & ICC_CTLR_EL1_EOImode_MASK) >> > >>>> + ICC_CTLR_EL1_EOImode_SHIFT; > >>>> + vmcr = (vmcr << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; > > > >> I am not able to understand why we use ICC_CTLR _*macros here? Please > >> could you explain it to me? Besides if we want to ignore the FIQen bit > >> can't we change the ICH_VMCR_CTLR_MASK value? > >> > > This first statement is setting the vmcr to the ctlr's bit[1], but > > placed in bit[0], and then the next statement is moving that bit value > > to the corresponding place in the vmcr. I think this is correct > > (although a little opaque). > Yes the question was more about the semantic of the vmcrp->ctlr field. I > thought it was supposed to store ICH_VMCR_EL2 ctrl bits as they are and > not with a different layout. my understanding is that vmcrp->ctlr is a representation of the GICC_CTLR field and its bit layout (based on the existin v2 code, unless I misread it) and the vmcr is the vmcr field. I don't care much either way, as long as it's clear what the semantics are. One problem with storing parts of the VMCR in the vmcrp->ctlr field is that I don't think there's an architectural definition of the concept 'ICH_VMCR_EL2 ctrl bits'. Thanks, -Christoffer _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h index b4f8287..406fc3e 100644 --- a/include/linux/irqchip/arm-gic-v3.h +++ b/include/linux/irqchip/arm-gic-v3.h @@ -404,8 +404,6 @@ #define ICH_HCR_EN (1 << 0) #define ICH_HCR_UIE (1 << 1) -#define ICH_VMCR_CTLR_SHIFT 0 -#define ICH_VMCR_CTLR_MASK (0x21f << ICH_VMCR_CTLR_SHIFT) #define ICH_VMCR_CBPR_SHIFT 4 #define ICH_VMCR_CBPR_MASK (1 << ICH_VMCR_CBPR_SHIFT) #define ICH_VMCR_EOIM_SHIFT 9 diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c index 2cb04b7..ad353b5 100644 --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c @@ -212,22 +212,6 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu, } } -static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) -{ - if (kvm_vgic_global_state.type == VGIC_V2) - vgic_v2_set_vmcr(vcpu, vmcr); - else - vgic_v3_set_vmcr(vcpu, vmcr); -} - -static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) -{ - if (kvm_vgic_global_state.type == VGIC_V2) - vgic_v2_get_vmcr(vcpu, vmcr); - else - vgic_v3_get_vmcr(vcpu, vmcr); -} - #define GICC_ARCH_VERSION_V2 0x2 /* These are for userland accesses only, there is no guest-facing emulation. */ diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c index 0d1bc98..f81e0e5 100644 --- a/virt/kvm/arm/vgic/vgic-mmio.c +++ b/virt/kvm/arm/vgic/vgic-mmio.c @@ -416,6 +416,22 @@ int vgic_validate_mmio_region_addr(struct kvm_device *dev, return -ENXIO; } +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) +{ + if (kvm_vgic_global_state.type == VGIC_V2) + vgic_v2_set_vmcr(vcpu, vmcr); + else + vgic_v3_set_vmcr(vcpu, vmcr); +} + +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr) +{ + if (kvm_vgic_global_state.type == VGIC_V2) + vgic_v2_get_vmcr(vcpu, vmcr); + else + vgic_v3_get_vmcr(vcpu, vmcr); +} + /* * kvm_mmio_read_buf() returns a value in a format where it can be converted * to a byte array and be directly observed as the guest wanted it to appear diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c index 9f0dae3..a3ff04b 100644 --- a/virt/kvm/arm/vgic/vgic-v3.c +++ b/virt/kvm/arm/vgic/vgic-v3.c @@ -175,10 +175,19 @@ void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) { u32 vmcr; - vmcr = (vmcrp->ctlr << ICH_VMCR_CTLR_SHIFT) & ICH_VMCR_CTLR_MASK; + /* + * Ignore the FIQen bit, because GIC emulation always implies + * SRE=1 which means the vFIQEn bit is also RES1. + */ + vmcr = (vmcrp->ctlr & ICC_CTLR_EL1_EOImode_MASK) >> + ICC_CTLR_EL1_EOImode_SHIFT; + vmcr = (vmcr << ICH_VMCR_EOIM_SHIFT) & ICH_VMCR_EOIM_MASK; + vmcr |= (vmcrp->ctlr << ICH_VMCR_CBPR_SHIFT) & ICH_VMCR_CBPR_MASK; vmcr |= (vmcrp->abpr << ICH_VMCR_BPR1_SHIFT) & ICH_VMCR_BPR1_MASK; vmcr |= (vmcrp->bpr << ICH_VMCR_BPR0_SHIFT) & ICH_VMCR_BPR0_MASK; vmcr |= (vmcrp->pmr << ICH_VMCR_PMR_SHIFT) & ICH_VMCR_PMR_MASK; + vmcr |= (vmcrp->grpen0 << ICH_VMCR_ENG0_SHIFT) & ICH_VMCR_ENG0_MASK; + vmcr |= (vmcrp->grpen1 << ICH_VMCR_ENG1_SHIFT) & ICH_VMCR_ENG1_MASK; vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr = vmcr; } @@ -187,10 +196,19 @@ void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) { u32 vmcr = vcpu->arch.vgic_cpu.vgic_v3.vgic_vmcr; - vmcrp->ctlr = (vmcr & ICH_VMCR_CTLR_MASK) >> ICH_VMCR_CTLR_SHIFT; + /* + * Ignore the FIQen bit, because GIC emulation always implies + * SRE=1 which means the vFIQEn bit is also RES1. + */ + vmcrp->ctlr = (vmcr & ICH_VMCR_EOIM_MASK) >> ICH_VMCR_EOIM_SHIFT; + vmcrp->ctlr = (vmcrp->ctlr << ICC_CTLR_EL1_EOImode_SHIFT) & + ICC_CTLR_EL1_EOImode_MASK; + vmcrp->ctlr |= (vmcr & ICH_VMCR_CBPR_MASK) >> ICH_VMCR_CBPR_SHIFT; vmcrp->abpr = (vmcr & ICH_VMCR_BPR1_MASK) >> ICH_VMCR_BPR1_SHIFT; vmcrp->bpr = (vmcr & ICH_VMCR_BPR0_MASK) >> ICH_VMCR_BPR0_SHIFT; vmcrp->pmr = (vmcr & ICH_VMCR_PMR_MASK) >> ICH_VMCR_PMR_SHIFT; + vmcrp->grpen0 = (vmcr & ICH_VMCR_ENG0_MASK) >> ICH_VMCR_ENG0_SHIFT; + vmcrp->grpen1 = (vmcr & ICH_VMCR_ENG1_MASK) >> ICH_VMCR_ENG1_SHIFT; } #define INITIAL_PENDBASER_VALUE \ diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h index 91f58b2..9232791 100644 --- a/virt/kvm/arm/vgic/vgic.h +++ b/virt/kvm/arm/vgic/vgic.h @@ -78,6 +78,9 @@ struct vgic_vmcr { u32 abpr; u32 bpr; u32 pmr; + /* Below member variable are valid only for GICv3 */ + u32 grpen0; + u32 grpen1; }; struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu, @@ -138,6 +141,8 @@ int vgic_v3_dist_uaccess(struct kvm_vcpu *vcpu, bool is_write, int vgic_v3_redist_uaccess(struct kvm_vcpu *vcpu, bool is_write, int offset, u32 *val); int kvm_register_vgic_device(unsigned long type); +void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); +void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); int vgic_lazy_init(struct kvm *kvm); int vgic_init(struct kvm *kvm);