| Message ID | 1418830815-4720-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
Hi Ian, On 18/12/2014 09:47, Ian Campbell wrote: > On Wed, 2014-12-17 at 15:40 +0000, Julien Grall wrote: >> The domain vgic lock is used uninitialized. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > >> --- >> This is a bug fix for Xen 4.5 and Xen 4.4. The vgic lock is used >> unitialized. Luckily we only use the field "raw" which is reset to 0 >> during the domain allocation. >> >> There is no harm to apply for Xen 4.5 because it will correctly set >> the spin_lock structure for a later usage. > > By your above reasoning there is also no point, is there? That said, I > think we should take this since as you say it is harmless and good > practice to initialise spinlocks even if not strictly necessary. It's necessary to initialize spinlocks, not all the field of the spinlock is using the value 0 at initialization time. What I meant if the current usage is fine. But if we want to debug spinlock, it won't work. Regards,
On 18/12/2014 12:12, Ian Campbell wrote: > On Thu, 2014-12-18 at 12:05 +0000, Julien Grall wrote: >> Hi Ian, >> >> On 18/12/2014 09:47, Ian Campbell wrote: >>> On Wed, 2014-12-17 at 15:40 +0000, Julien Grall wrote: >>>> The domain vgic lock is used uninitialized. >>>> >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>> >>> Acked-by: Ian Campbell <ian.campbell@citrix.com> >>> >>>> --- >>>> This is a bug fix for Xen 4.5 and Xen 4.4. The vgic lock is used >>>> unitialized. Luckily we only use the field "raw" which is reset to 0 >>>> during the domain allocation. >>>> >>>> There is no harm to apply for Xen 4.5 because it will correctly set >>>> the spin_lock structure for a later usage. >>> >>> By your above reasoning there is also no point, is there? That said, I >>> think we should take this since as you say it is harmless and good >>> practice to initialise spinlocks even if not strictly necessary. >> >> It's necessary to initialize spinlocks, not all the field of the >> spinlock is using the value 0 at initialization time. > > You said "...we only use the field "raw" which is reset to 0...". Was > that statement inaccurate? It's accurate in the current. I should have give more details earlier, sorry. Regards,
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 97061ce..b8bd38b 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -90,6 +90,8 @@ int domain_vgic_init(struct domain *d) return -ENODEV; } + spin_lock_init(&d->arch.vgic.lock); + d->arch.vgic.shared_irqs = xzalloc_array(struct vgic_irq_rank, DOMAIN_NR_RANKS(d)); if ( d->arch.vgic.shared_irqs == NULL )
The domain vgic lock is used uninitialized. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- This is a bug fix for Xen 4.5 and Xen 4.4. The vgic lock is used unitialized. Luckily we only use the field "raw" which is reset to 0 during the domain allocation. There is no harm to apply for Xen 4.5 because it will correctly set the spin_lock structure for a later usage. --- xen/arch/arm/vgic.c | 2 ++ 1 file changed, 2 insertions(+)