Re: [PATCH 2/4] kvm: arm/arm64: vgic-v3: add ICH_AP[01]Rn accessors for GICv3

[wanghaibin <wanghaibin.wang@huawei.com>]

On 2017/7/7 0:18, Marc Zyngier wrote:

> On 05/07/17 12:23, wanghaibin wrote:
>> Compared to GICv2 hardware, GICv3 support ICH_AP[01]Rn for Group [01]
>> interrupts.
> 
> This doesn't describe what the patch is trying to achieve.


Will fix.

> 
>>
>> Signed-off-by: wanghaibin <wanghaibin.wang@huawei.com>
>> ---
>>  virt/kvm/arm/vgic/vgic-v3.c | 20 ++++++++++++++++++++
>>  virt/kvm/arm/vgic/vgic.h    |  2 ++
>>  2 files changed, 22 insertions(+)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
>> index 030248e..361da71 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 apr, u32 idx, u32 val)
> 
> The "apr" parameter is actually the group number, and should be exposed
> as such.
>


Will fix.


>> +{
>> +	struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
>> +
>> +	if (apr)
>> +		cpu_if->vgic_ap1r[idx] = val;
>> +	else
>> +		cpu_if->vgic_ap0r[idx] = val;
> 
> How do you guarantee that you're not accessing outside of this GIC's
> preemption capability?
> 


Only GICC_APRn mmio uaccess for vGICv2 and ICC_APRn uaccess sysreg uaccess will calculate
and pass the idx parameter to here, and these action can guarantee the idx only can be set 0,1,2,3.

Thanks.


>> +}
>> +
>> +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx)
>> +{
>> +	struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
>> +
>> +	if (apr)
>> +		return cpu_if->vgic_ap1r[idx];
>> +	else
>> +		return cpu_if->vgic_ap0r[idx];
>> +}
>> +
>>  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 8791b9a..91d66ec 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 apr, u32 idx, u32 val);
>> +u32 vgic_v3_get_apr(struct kvm_vcpu *vcpu, u8 apr, 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.