On 2017/7/7 0:53, Marc Zyngier wrote:
> On 05/07/17 12:23, wanghaibin wrote:
>> Access GICV_APRn hardware register, SPEC says:
>>
>> When System register access is disabled for EL2, these registers access GICH_APR<n>,
>> and all active priorities for virtual machines are held in GICH_APR<n> regardless of
>> interrupt group.
>> When System register access is enabled for EL2, these registers access ICH_AP1R<n>_EL2,
>> and all active priorities for virtual machines are held in ICH_AP1R<n>_EL2 regardless
>> of interrupt group.
>>
>> Accordingly, the following two cases are implemented:
>> (1) GICv2 on GICv2, uaccess GICC_APRn info from GICH_APRn, and,
>> (2) GICv2 on GICv3, uaccess GICC_APRn info from ICH_AP1Rn.
>>
>> Signed-off-by: wanghaibin <wanghaibin.wang@xxxxxxxxxx>
>> ---
>> virt/kvm/arm/vgic/vgic-mmio-v2.c | 22 ++++++++++++++++++++--
>> virt/kvm/arm/vgic/vgic-mmio.c | 28 ++++++++++++++++++++++++++++
>> virt/kvm/arm/vgic/vgic.h | 3 +++
>> 3 files changed, 51 insertions(+), 2 deletions(-)
>>
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> index 63e0bbd..176b93f 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
>> @@ -303,6 +303,23 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu,
>> vgic_set_vmcr(vcpu, &vmcr);
>> }
>>
>> +static unsigned long vgic_v2_uaccess_read_activeprio(struct kvm_vcpu *vcpu,
>> + gpa_t addr, unsigned int len)
>> +{
>> + u32 idx = (addr & 0x0c) / sizeof(u32);
>> +
>> + return vgic_get_apr(vcpu, 0, idx);
>> +}
>> +
>> +static void vgic_v2_uaccess_write_activeprio(struct kvm_vcpu *vcpu,
>> + gpa_t addr, unsigned int len,
>> + unsigned long val)
>> +{
>> + u32 idx = (addr & 0x0c) / sizeof(u32);
>> +
>> + return vgic_set_apr(vcpu, 0, idx, val);
>> +}
>> +
>> static const struct vgic_register_region vgic_v2_dist_registers[] = {
>> REGISTER_DESC_WITH_LENGTH(GIC_DIST_CTRL,
>> vgic_mmio_read_v2_misc, vgic_mmio_write_v2_misc, 12,
>> @@ -361,8 +378,9 @@ static void vgic_mmio_write_vcpuif(struct kvm_vcpu *vcpu,
>> REGISTER_DESC_WITH_LENGTH(GIC_CPU_ALIAS_BINPOINT,
>> vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4,
>> VGIC_ACCESS_32bit),
>> - REGISTER_DESC_WITH_LENGTH(GIC_CPU_ACTIVEPRIO,
>> - vgic_mmio_read_raz, vgic_mmio_write_wi, 16,
>> + REGISTER_DESC_WITH_LENGTH_UACCESS(GIC_CPU_ACTIVEPRIO,
>> + vgic_mmio_read_raz, vgic_mmio_write_wi,
>> + vgic_v2_uaccess_read_activeprio, vgic_v2_uaccess_write_activeprio, 16,
>
> This 16 feels wrong. I now see why you had this index in the first
> patch. I think it begs the questions of what we're emulating. Are we
> showing a virtual GICv2 with only 5 bits of priority? Or something else?
Hi, Marc:
At present, Yes, I think it's only 5 bits of priority to show for GICv2.
But， the QEMU gicc apr put/get access code just like:
/* s->apr[n][cpu] -> GICC_APRn */
for (i = 0; i < 4; i++) {
reg = s->apr[i][cpu];
kvm_gicc_access(s, 0xd0 + i * 4, cpu, ®, true);
}
It get/put all gicc apr registers (the spec implement GICC_APR<n> (n = 0 - 3) to provides information about
interrupt active priorities), So I think the 16 is correct.
BTW: I notice that we show the 5 priority bits by VGIC_PRI_BITS to filter emulate write GICD_IPRIORITYRn bits （vgic_mmio_write_priority）
both emulate GICv2 and emulate GICv3.
Should we cancel this limit when (GICv2 on GICv3) and (GICv3 on GICv3) ?
I think we can show the bits which the hardware ICH_VTR_EL2.PRIbits support when the hardware is GICv3 to avoid priority bits lost.
Thanks.
>
>> VGIC_ACCESS_32bit),
>> REGISTER_DESC_WITH_LENGTH(GIC_CPU_IDENT,
>> vgic_mmio_read_vcpuif, vgic_mmio_write_vcpuif, 4,
>> diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
>> index 1c17b2a..42fe00d 100644
>> --- a/virt/kvm/arm/vgic/vgic-mmio.c
>> +++ b/virt/kvm/arm/vgic/vgic-mmio.c
>> @@ -454,6 +454,34 @@ static int match_region(const void *key, const void *elt)
>> sizeof(regions[0]), match_region);
>> }
>>
>> +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val)
>
> Same remark as before about "apr".
>
>> +{
>> + 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 {
>> + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
>> + vgic_v3_set_apr(vcpu, 1, idx, val);
>> + else
>> + vgic_v3_set_apr(vcpu, apr, idx, val);
>> + }
>> +}
>> +
>> +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, 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 {
>> + if (vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
>> + return vgic_v3_get_apr(vcpu, 1, idx);
>> + else
>> + return vgic_v3_get_apr(vcpu, apr, idx);
>> + }
>> +}
>> +
>> void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr)
>> {
>> if (kvm_vgic_global_state.type == VGIC_V2)
>> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
>> index 91d66ec..3cdc448 100644
>> --- a/virt/kvm/arm/vgic/vgic.h
>> +++ b/virt/kvm/arm/vgic/vgic.h
>> @@ -213,6 +213,9 @@ int vgic_v3_has_cpu_sysregs_attr(struct kvm_vcpu *vcpu, bool is_write, u64 id,
>> int vgic_v3_line_level_info_uaccess(struct kvm_vcpu *vcpu, bool is_write,
>> u32 intid, u64 *val);
>> int kvm_register_vgic_device(unsigned long type);
>> +
>> +void vgic_set_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx, u32 val);
>> +u32 vgic_get_apr(struct kvm_vcpu *vcpu, u8 apr, u32 idx);
>> 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);
>>
>
> Thanks,
>
> M.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
