Re: [Xen-devel] [PATCH for 4.5] xen/arm: Initialize the domain vgic lock

2015-01-06 Thread Ian Campbell
On Fri, 2014-12-19 at 09:23 -0500, Konrad Rzeszutek Wilk wrote:
 On Thu, Dec 18, 2014 at 09:47:37AM +, Ian Campbell wrote:
  On Wed, 2014-12-17 at 15:40 +, Julien Grall wrote:
   The domain vgic lock is used uninitialized.
   
   Signed-off-by: Julien Grall julien.gr...@linaro.org
  
  Acked-by: Ian Campbell ian.campb...@citrix.com
 
 Release-Acked-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com

Applied.



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH for 4.5] xen/arm: Initialize the domain vgic lock

2014-12-18 Thread Ian Campbell
On Wed, 2014-12-17 at 15:40 +, Julien Grall wrote:
 The domain vgic lock is used uninitialized.
 
 Signed-off-by: Julien Grall julien.gr...@linaro.org

Acked-by: Ian Campbell ian.campb...@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.

 ---
  xen/arch/arm/vgic.c | 2 ++
  1 file changed, 2 insertions(+)
 
 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 )




___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH for 4.5] xen/arm: Initialize the domain vgic lock

2014-12-18 Thread Julien Grall

Hi Ian,

On 18/12/2014 09:47, Ian Campbell wrote:

On Wed, 2014-12-17 at 15:40 +, Julien Grall wrote:

The domain vgic lock is used uninitialized.

Signed-off-by: Julien Grall julien.gr...@linaro.org


Acked-by: Ian Campbell ian.campb...@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,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH for 4.5] xen/arm: Initialize the domain vgic lock

2014-12-18 Thread Ian Campbell
On Thu, 2014-12-18 at 12:05 +, Julien Grall wrote:
 Hi Ian,
 
 On 18/12/2014 09:47, Ian Campbell wrote:
  On Wed, 2014-12-17 at 15:40 +, Julien Grall wrote:
  The domain vgic lock is used uninitialized.
 
  Signed-off-by: Julien Grall julien.gr...@linaro.org
 
  Acked-by: Ian Campbell ian.campb...@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?

 What I meant if the current usage is fine. But if we want to debug 
 spinlock, it won't work.
 
 Regards,
 



___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH for 4.5] xen/arm: Initialize the domain vgic lock

2014-12-18 Thread Julien Grall



On 18/12/2014 12:12, Ian Campbell wrote:

On Thu, 2014-12-18 at 12:05 +, Julien Grall wrote:

Hi Ian,

On 18/12/2014 09:47, Ian Campbell wrote:

On Wed, 2014-12-17 at 15:40 +, Julien Grall wrote:

The domain vgic lock is used uninitialized.

Signed-off-by: Julien Grall julien.gr...@linaro.org


Acked-by: Ian Campbell ian.campb...@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,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCH for 4.5] xen/arm: Initialize the domain vgic lock

2014-12-17 Thread Julien Grall
The domain vgic lock is used uninitialized.

Signed-off-by: Julien Grall julien.gr...@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(+)

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 )
-- 
2.1.3


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel