Re: [Xen-devel] [PATCH] xen: arm: correct use of find_next_bit

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.01.14 at 15:23, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> find_next_bit takes a "const unsigned long *" but forcing a cast of an
> "uint32_t *" throws away the alignment constraints and ends up causing an
> alignment fault on arm64 if the input happened to be 4 but not 8 byte 
> aligned.
> 
> Instead of casting use a temporary variable of the right type.
> 
> I've had a look around for similar constructs and the only thing I found was
> maintenance_interrupt which cases a uint64_t down to an unsigned long, which
> although perhaps not best advised is safe I think.
> 
> This was observed with the AArch64 Linaro toolchain 2013.12 but I think that
> is just coincidental due to subtle changes to the stack layout etc.
> 
> Reported-by: Fu Wei <fu.wei@xxxxxxxxxx>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
>  xen/arch/arm/vgic.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 90e9707..553411d 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -362,11 +362,12 @@ read_as_zero:
>  
>  static void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
>  {
> +    const unsigned long mask = r;

Why don't you just change the type of "r" to "unsigned long"?

Jan

>      struct pending_irq *p;
>      unsigned int irq;
>      int i = 0;
>  
> -    while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 
> ) 
> {
> +    while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>          irq = i + (32 * n);
>          p = irq_to_pending(v, irq);
>          clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> @@ -379,11 +380,12 @@ static void vgic_disable_irqs(struct vcpu *v, uint32_t 
> r, int n)
>  
>  static void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>  {
> +    const unsigned long mask = r;
>      struct pending_irq *p;
>      unsigned int irq;
>      int i = 0;
>  
> -    while ( (i = find_next_bit((const long unsigned int *) &r, 32, i)) < 32 
> ) 
> {
> +    while ( (i = find_next_bit(&mask, 32, i)) < 32 ) {
>          irq = i + (32 * n);
>          p = irq_to_pending(v, irq);
>          set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 




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