On 2017/7/25 19:25, Marc Zyngier wrote:

> On Mon, Jul 17 2017 at  6:23:31 pm BST, wanghaibin 
> <wanghaibin.w...@huawei.com> 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.w...@huawei.com>
>> ---
>>  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@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to