On 2017/7/25 19:25, Marc Zyngier wrote: > On Mon, Jul 17 2017 at 6:23:31 pm BST, wanghaibin <wanghaibin.wang@xxxxxxxxxx> wrote: >> This patch is used for GICv2 on GICv3. >> >> About GICV_APRn hardware register access,the SPEC says: >> When System register access is enabled for EL2, these registers access >> ICH_AP1Rn_EL2, and all active priorities for virtual machines are held >> in ICH_AP1Rn_EL2 regardless of interrupt group. >> >> For GICv3 hardware, we access the active priorities from ICH_AP1Rn_EL2 >> in this scene. >> >> Signed-off-by: wanghaibin <wanghaibin.wang@xxxxxxxxxx> >> --- >> virt/kvm/arm/vgic/vgic-mmio.c | 12 ++++++++++++ >> virt/kvm/arm/vgic/vgic-v3.c | 20 ++++++++++++++++++++ >> virt/kvm/arm/vgic/vgic.h | 2 ++ >> 3 files changed, 34 insertions(+) >> >> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c >> index f6f3681..3b648a7 100644 >> --- a/virt/kvm/arm/vgic/vgic-mmio.c >> +++ b/virt/kvm/arm/vgic/vgic-mmio.c >> @@ -456,14 +456,26 @@ static int match_region(const void *key, const void *elt) >> >> void vgic_set_apr(struct kvm_vcpu *vcpu, u32 idx, u32 val) >> { >> + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; >> + >> if (kvm_vgic_global_state.type == VGIC_V2) >> vgic_v2_set_apr(vcpu, idx, val); >> + else { > > Coding style. Will fix. Thanks > >> + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) >> + vgic_v3_set_apr(vcpu, 1, idx, val); >> + } >> } >> >> u32 vgic_get_apr(struct kvm_vcpu *vcpu, u32 idx) >> { >> + u32 vgic_model = vcpu->kvm->arch.vgic.vgic_model; >> + >> if (kvm_vgic_global_state.type == VGIC_V2) >> return vgic_v2_get_apr(vcpu, idx); >> + else { > > Same here. > >> + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2) >> + return vgic_v3_get_apr(vcpu, 1, idx); >> + } >> >> return 0; >> } >> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c >> index 030248e..da40681 100644 >> --- a/virt/kvm/arm/vgic/vgic-v3.c >> +++ b/virt/kvm/arm/vgic/vgic-v3.c >> @@ -156,6 +156,26 @@ void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr) >> vcpu->arch.vgic_cpu.vgic_v3.vgic_lr[lr] = 0; >> } >> >> +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 group, u32 idx, u32 val) >> +{ >> + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; >> + >> + if (group) >> + cpu_if->vgic_ap1r[idx] = val; >> + else >> + cpu_if->vgic_ap0r[idx] = val; >> +} >> + >> +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 group, u32 idx) >> +{ >> + struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; >> + >> + if (group) >> + return cpu_if->vgic_ap1r[idx]; >> + else >> + return cpu_if->vgic_ap0r[idx]; >> +} >> + > > How do we ensure that these APRs are even valid? We can have anything > from 5 to 7 bits of preemption, and thus 1 to 4 APRs. The rest is > UNDEFined. Sorry, Marc, I can't follow you. Do you suspect that an undefined APR register(implemented by ICH_VTR_EL2.PRIbits) will be accessed here? This code just access the soft vgic_ap[0,1]r structure, and I notice that, this structure saved the hardware APRn info under the constraint of ICH_VTR_EL2.PREbits implemented when save the vcpu content. the soft vgic_ap[0,1]r structure will keep 0 value when the hw APRn UNDEFined. Thanks. > >> void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcrp) >> { >> struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3; >> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h >> index 441ded7..19b0f8b 100644 >> --- a/virt/kvm/arm/vgic/vgic.h >> +++ b/virt/kvm/arm/vgic/vgic.h >> @@ -181,6 +181,8 @@ static inline void vgic_get_irq_kref(struct vgic_irq *irq) >> void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr); >> void vgic_v3_clear_lr(struct kvm_vcpu *vcpu, int lr); >> void vgic_v3_set_underflow(struct kvm_vcpu *vcpu); >> +void vgic_v3_set_apr(struct kvm_vcpu *vcpu, u8 group, u32 idx, u32 val); >> +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 group, u32 idx); >> void vgic_v3_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> void vgic_v3_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr); >> void vgic_v3_enable(struct kvm_vcpu *vcpu); > > Thanks, > > M. _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm