On 16/01/13 15:29, Christoffer Dall wrote:
> [...]
>
>>> diff --git a/arch/arm/include/asm/kvm_vgic.h b/arch/arm/include/asm/kvm_vgic.h
>>> index 1ace491..f9d1977 100644
>>> --- a/arch/arm/include/asm/kvm_vgic.h
>>> +++ b/arch/arm/include/asm/kvm_vgic.h
>>> @@ -33,6 +33,7 @@
>>> #define VGIC_NR_PRIVATE_IRQS (VGIC_NR_SGIS + VGIC_NR_PPIS)
>>> #define VGIC_NR_SHARED_IRQS (VGIC_NR_IRQS - VGIC_NR_PRIVATE_IRQS)
>>> #define VGIC_MAX_CPUS KVM_MAX_VCPUS
>>> +#define VGIC_MAX_LRS 64
>>
>> Consider this instead (for the reason below)
>> #define VGIC_MAX_LRS (1 << 7)
>>
>
> so here you mean (1 << 6), right?
No. We have a 6 bit field that contains (NR_LRS - 1). So the maximum
value is (0b111111 + 1), which is (1 << 7).
>
>>> /* Sanity checks... */
>>> #if (VGIC_MAX_CPUS > 8)
>>> @@ -120,7 +121,7 @@ struct vgic_cpu {
>>> DECLARE_BITMAP( pending_shared, VGIC_NR_SHARED_IRQS);
>>>
>>> /* Bitmap of used/free list registers */
>>> - DECLARE_BITMAP( lr_used, 64);
>>> + DECLARE_BITMAP( lr_used, VGIC_MAX_LRS);
>>>
>>> /* Number of list registers on this CPU */
>>> int nr_lr;
>>> @@ -132,7 +133,7 @@ struct vgic_cpu {
>>> u32 vgic_eisr[2]; /* Saved only */
>>> u32 vgic_elrsr[2]; /* Saved only */
>>> u32 vgic_apr;
>>> - u32 vgic_lr[64]; /* A15 has only 4... */
>>> + u32 vgic_lr[VGIC_MAX_LRS];
>>> #endif
>>> };
>>>
>>> diff --git a/arch/arm/kvm/vgic.c b/arch/arm/kvm/vgic.c
>>> index a0d283c..90a99fd 100644
>>> --- a/arch/arm/kvm/vgic.c
>>> +++ b/arch/arm/kvm/vgic.c
>>> @@ -1345,6 +1345,8 @@ int kvm_vgic_hyp_init(void)
>>>
>>> vgic_nr_lr = readl_relaxed(vgic_vctrl_base + GICH_VTR);
>>> vgic_nr_lr = (vgic_nr_lr & 0x1f) + 1;
>>
>> There is a bug here. It should be:
>> vgic_nr_lr = (vgic_nr_lr & 0x2f) + 1;
>>
>
> and here you mean (vgic_nr_lr & 0x3f) + 1
> right?
Neither. 0x2f is the right value. See the GIC spec, 5.3.2, GICH_VTR
register.
M.
--
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
