On 26.09.2013, at 03:36, Alexander Graf wrote: > > On 26.09.2013, at 03:15, Alexander Graf wrote: > >> >> On 26.09.2013, at 02:54, Christoffer Dall wrote: >> >>> On Thu, Sep 26, 2013 at 12:37:03AM +0200, Alexander Graf wrote: >>>> >>>> On 25.09.2013, at 23:30, Christoffer Dall wrote: >>>> >>>>> On Sun, Aug 25, 2013 at 04:24:20PM +0100, Alexander Graf wrote: >>>>>> >>>>>> On 23.08.2013, at 20:20, Christoffer Dall wrote: >>>>>> >>>>>>> Implement support for the CPU interface register access driven by MMIO >>>>>>> address offsets from the CPU interface base address. Useful for user >>>>>>> space to support save/restore of the VGIC state. >>>>>>> >>>>>>> This commit adds support only for the same logic as the current VGIC >>>>>>> support, and no more. For example, the active priority registers are >>>>>>> handled as RAZ/WI, just like setting priorities on the emulated >>>>>>> distributor. >>>>>>> >>>>>>> Signed-off-by: Christoffer Dall <christoffer.d...@linaro.org> >>>>>>> --- >>>>>>> virt/kvm/arm/vgic.c | 66 >>>>>>> +++++++++++++++++++++++++++++++++++++++++++++++---- >>>>>>> 1 file changed, 62 insertions(+), 4 deletions(-) >>>>>>> >>>>>>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c >>>>>>> index d44b5a1..257dbae 100644 >>>>>>> --- a/virt/kvm/arm/vgic.c >>>>>>> +++ b/virt/kvm/arm/vgic.c >>>>>>> @@ -1684,9 +1684,67 @@ int kvm_vgic_addr(struct kvm *kvm, unsigned long >>>>>>> type, u64 *addr, bool write) >>>>>>> static bool handle_cpu_mmio_misc(struct kvm_vcpu *vcpu, >>>>>>> struct kvm_exit_mmio *mmio, >>>>>>> phys_addr_t offset) >>>>>>> { >>>>>>> - return true; >>>>>>> + struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu; >>>>>>> + u32 reg, mask = 0, shift = 0; >>>>>>> + bool updated = false; >>>>>>> + >>>>>>> + switch (offset & ~0x3) { >>>>>>> + case GIC_CPU_CTRL: >>>>>>> + mask = GICH_VMCR_CTRL_MASK; >>>>>>> + shift = GICH_VMCR_CTRL_SHIFT; >>>>>>> + break; >>>>>>> + case GIC_CPU_PRIMASK: >>>>>>> + mask = GICH_VMCR_PRIMASK_MASK; >>>>>>> + shift = GICH_VMCR_PRIMASK_SHIFT; >>>>>>> + break; >>>>>>> + case GIC_CPU_BINPOINT: >>>>>>> + mask = GICH_VMCR_BINPOINT_MASK; >>>>>>> + shift = GICH_VMCR_BINPOINT_SHIFT; >>>>>>> + break; >>>>>>> + case GIC_CPU_ALIAS_BINPOINT: >>>>>>> + mask = GICH_VMCR_ALIAS_BINPOINT_MASK; >>>>>>> + shift = GICH_VMCR_ALIAS_BINPOINT_SHIFT; >>>>>>> + break; >>>>>>> + } >>>>>>> + >>>>>>> + if (!mmio->is_write) { >>>>>>> + reg = (vgic_cpu->vgic_vmcr & mask) >> shift; >>>>>>> + memcpy(mmio->data, ®, sizeof(reg)); >>>>>>> + } else { >>>>>>> + memcpy(®, mmio->data, sizeof(reg)); >>>>>>> + reg = (reg << shift) & mask; >>>>>>> + if (reg != (vgic_cpu->vgic_vmcr & mask)) >>>>>>> + updated = true; >>>>>>> + vgic_cpu->vgic_vmcr &= ~mask; >>>>>>> + vgic_cpu->vgic_vmcr |= reg; >>>>>>> + } >>>>>>> + return updated; >>>>>>> +} >>>>>>> + >>>>>>> +static bool handle_mmio_abpr(struct kvm_vcpu *vcpu, >>>>>>> + struct kvm_exit_mmio *mmio, phys_addr_t >>>>>>> offset) >>>>>>> +{ >>>>>>> + return handle_cpu_mmio_misc(vcpu, mmio, GIC_CPU_ALIAS_BINPOINT); >>>>>>> +} >>>>>>> + >>>>>>> +static bool handle_cpu_mmio_ident(struct kvm_vcpu *vcpu, >>>>>>> + struct kvm_exit_mmio *mmio, >>>>>>> + phys_addr_t offset) >>>>>>> +{ >>>>>>> + u32 reg; >>>>>>> + >>>>>>> + if (mmio->is_write) >>>>>>> + return false; >>>>>>> + >>>>>>> + reg = 0x0002043B; >>>>>> >>>>>> This wants a comment and probably also a #define :). >>>>>> >>>>> >>>>> Marc, where does the 0x4b0 product id code come from for the distributor >>>>> IIDR? >>>>> >>>>> Would this be satisfying? >> >> ^ >> >>>>> >>>>> >>>>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c >>>>> index 5214424..558be38 100644 >>>>> --- a/virt/kvm/arm/vgic.c >>>>> +++ b/virt/kvm/arm/vgic.c >>>>> @@ -71,6 +71,9 @@ >>>>> #define VGIC_ADDR_UNDEF (-1) >>>>> #define IS_VGIC_ADDR_UNDEF(_x) ((_x) == VGIC_ADDR_UNDEF) >>>>> >>>>> +#define GIC_PRODUCT_ID 0x4b0 >>>> >>>> This is a specific GIC version. PL390 for example is 0x3b0: >>>> >>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0416b/Beiggeba.html >>>> >>>> That should be reflected in the #define. If it means "GICv2" then it >>>> should be GICV2_PRODUCT_ID for example. >>>> >>> >>> I know what field in the register it is thanks :) >>> >>> But I couldn't find 0x4b0 anywhere in the docs, so I'm asking >>> Marc where he got it from. I don't believe it means GICv2, but a >> >> Ah, ok. Then the answer to your question above is a simple "no" as the name >> doesn't really tell us everything we want to know yet :). >> >>> specific implementation of a GICv2, and once I have more info I can >>> change the define name, I suspect this is potentially something made-up >>> to indicate that this is the KVM VGIC... >> >> Hrm, makes sense. So that also explains why there's a special version field. > > It doesn't explain why it only gets set in one of the IIDR variants though. > Is this on purpose? From what I can tell, the CPU and Distributor interfaces > both should return the same number here.
Hrm. Curious. According to http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0438i/BABGBHBG.html the proper values for IIDR on an A15 are: GICD_IIDR 0x0000043B GICC_IIDR 0x0002043B So what do the fields mean in each register? GICD_IIDR: [31:24] ProductID [23:20] - [19:16] Variant [15:12] Revision [11:0] Implementer GICC_IIDR: [31:20] ProductID [19:16] Architecture version [15:12] Revision [11:0] Implementer So while 19:16 in GICC denotes the "Architecture version" (GICv2 for us), it means "Variant" for GICD. What a mess. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html