[PATCH 00/11] Fix arm64 issues highlighted by sparse and smatch

[PATCH 00/11] Fix arm64 issues highlighted by sparse and smatch

[Will Deacon]

Hi all,

After the static checking talk at kernel summit, I felt empowered to go
and run sparse and smatch over the arm64 defconfig. As I anticipated,
this spotted a few issues in the arch code, so I've fixed up the ones
that aren't just noise in this series. Most are simply incorrect
address space annotations, but fixing these is useful for spotting real
issues in the future.

I don't think any of these fix critical bugs, but they all deserve to be
addressed. I tagged the compat watchpoint fix for stable, as that code
has been broken since we merged the architecture port.

Cheers,

Will

Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Jean Pihet <jean.pihet@linaro.org>

--->8

Will Deacon (11):
  KVM: ARM/arm64: fix non-const declaration of function returning const
  KVM: ARM/arm64: fix broken __percpu annotation
  KVM: ARM/arm64: avoid returning negative error code as bool
  KVM: ARM/arm64: return -EFAULT if copy_from_user fails in
    set_timer_reg
  KVM: vgic: return int instead of bool when checking I/O ranges
  KVM: vgic: declare probe function pointer as const
  irqchip: gic: make gic_default_routable_irq_domain_ops static
  irqchip: gic-v3: declare rdist as __percpu pointer to __iomem pointer
  arm64: ptrace: fix compat hardware watchpoint reporting
  arm64: ptrace: fix compat reg getter/setter return values
  arm64: perf: don't rely on layout of pt_regs when grabbing sp or pc

 arch/arm/include/asm/kvm_host.h        |  2 +-
 arch/arm/kvm/arm.c                     |  2 +-
 arch/arm/kvm/coproc.c                  |  2 +-
 arch/arm/kvm/guest.c                   |  2 +-
 arch/arm64/include/asm/hw_breakpoint.h |  1 -
 arch/arm64/include/asm/kvm_host.h      |  4 ++--
 arch/arm64/kernel/perf_regs.c          |  6 ++++++
 arch/arm64/kernel/ptrace.c             | 13 +++++++++----
 arch/arm64/kvm/guest.c                 |  2 +-
 arch/arm64/kvm/sys_regs.c              |  2 +-
 drivers/irqchip/irq-gic-v3.c           |  2 +-
 drivers/irqchip/irq-gic.c              |  2 +-
 virt/kvm/arm/vgic.c                    |  6 +++---
 13 files changed, 28 insertions(+), 18 deletions(-)

-- 
2.1.0.rc1

[PATCH 01/11] KVM: ARM/arm64: fix non-const declaration of function returning const

[Will Deacon]

Sparse kicks up about a type mismatch for kvm_target_cpu:

arch/arm64/kvm/guest.c:271:25: error: symbol 'kvm_target_cpu' redeclared with different type (originally declared at ./arch/arm64/include/asm/kvm_host.h:45) - different modifiers

so fix this by adding the missing const attribute to the function
declaration.

Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/include/asm/kvm_host.h   | 2 +-
 arch/arm64/include/asm/kvm_host.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 6dfb404f6c46..fcb12a6f7db5 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -42,7 +42,7 @@
 
 struct kvm_vcpu;
 u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
-int kvm_target_cpu(void);
+int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
 void kvm_reset_coprocs(struct kvm_vcpu *vcpu);
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index e10c45a578e3..44094e559848 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -42,7 +42,7 @@
 #define KVM_VCPU_MAX_FEATURES 3
 
 struct kvm_vcpu;
-int kvm_target_cpu(void);
+int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
 int kvm_arch_dev_ioctl_check_extension(long ext);
 
-- 
2.1.0.rc1

[PATCH 01/11] KVM: ARM/arm64: fix non-const declaration of function returning const

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> Sparse kicks up about a type mismatch for kvm_target_cpu:
> 
> arch/arm64/kvm/guest.c:271:25: error: symbol 'kvm_target_cpu' redeclared with different type (originally declared at ./arch/arm64/include/asm/kvm_host.h:45) - different modifiers
> 
> so fix this by adding the missing const attribute to the function
> declaration.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
>  arch/arm/include/asm/kvm_host.h   | 2 +-
>  arch/arm64/include/asm/kvm_host.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
> index 6dfb404f6c46..fcb12a6f7db5 100644
> --- a/arch/arm/include/asm/kvm_host.h
> +++ b/arch/arm/include/asm/kvm_host.h
> @@ -42,7 +42,7 @@
>  
>  struct kvm_vcpu;
>  u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
> -int kvm_target_cpu(void);
> +int __attribute_const__ kvm_target_cpu(void);
>  int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
>  void kvm_reset_coprocs(struct kvm_vcpu *vcpu);
>  
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index e10c45a578e3..44094e559848 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -42,7 +42,7 @@
>  #define KVM_VCPU_MAX_FEATURES 3
>  
>  struct kvm_vcpu;
> -int kvm_target_cpu(void);
> +int __attribute_const__ kvm_target_cpu(void);
>  int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
>  int kvm_arch_dev_ioctl_check_extension(long ext);
>  
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 01/11] KVM: ARM/arm64: fix non-const declaration of function returning const

[Christoffer Dall]

On Tue, Aug 26, 2014 at 03:13:20PM +0100, Will Deacon wrote:
> Sparse kicks up about a type mismatch for kvm_target_cpu:
> 
> arch/arm64/kvm/guest.c:271:25: error: symbol 'kvm_target_cpu' redeclared with different type (originally declared at ./arch/arm64/include/asm/kvm_host.h:45) - different modifiers
> 
> so fix this by adding the missing const attribute to the function
> declaration.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

[PATCH 02/11] KVM: ARM/arm64: fix broken __percpu annotation

[Will Deacon]

Running sparse results in a bunch of noisy address space mismatches
thanks to the broken __percpu annotation on kvm_get_running_vcpus.

This function returns a pcpu pointer to a pointer, not a pointer to a
pcpu pointer. This patch fixes the annotation, which kills the warnings
from sparse.

Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kvm/arm.c                | 2 +-
 arch/arm64/include/asm/kvm_host.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index a99e0cdf8ba2..00a5982ca5bc 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -82,7 +82,7 @@ struct kvm_vcpu *kvm_arm_get_running_vcpu(void)
 /**
  * kvm_arm_get_running_vcpus - get the per-CPU array of currently running vcpus.
  */
-struct kvm_vcpu __percpu **kvm_get_running_vcpus(void)
+struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
 {
 	return &kvm_arm_running_vcpu;
 }
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 44094e559848..50431d36732b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -193,7 +193,7 @@ static inline int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)
 }
 
 struct kvm_vcpu *kvm_arm_get_running_vcpu(void);
-struct kvm_vcpu __percpu **kvm_get_running_vcpus(void);
+struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void);
 
 u64 kvm_call_hyp(void *hypfn, ...);
 
-- 
2.1.0.rc1

[PATCH 02/11] KVM: ARM/arm64: fix broken __percpu annotation

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> Running sparse results in a bunch of noisy address space mismatches
> thanks to the broken __percpu annotation on kvm_get_running_vcpus.
> 
> This function returns a pcpu pointer to a pointer, not a pointer to a
> pcpu pointer. This patch fixes the annotation, which kills the warnings
> from sparse.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M..

> ---
>  arch/arm/kvm/arm.c                | 2 +-
>  arch/arm64/include/asm/kvm_host.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
> index a99e0cdf8ba2..00a5982ca5bc 100644
> --- a/arch/arm/kvm/arm.c
> +++ b/arch/arm/kvm/arm.c
> @@ -82,7 +82,7 @@ struct kvm_vcpu *kvm_arm_get_running_vcpu(void)
>  /**
>   * kvm_arm_get_running_vcpus - get the per-CPU array of currently running vcpus.
>   */
> -struct kvm_vcpu __percpu **kvm_get_running_vcpus(void)
> +struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void)
>  {
>  	return &kvm_arm_running_vcpu;
>  }
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 44094e559848..50431d36732b 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -193,7 +193,7 @@ static inline int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)
>  }
>  
>  struct kvm_vcpu *kvm_arm_get_running_vcpu(void);
> -struct kvm_vcpu __percpu **kvm_get_running_vcpus(void);
> +struct kvm_vcpu * __percpu *kvm_get_running_vcpus(void);
>  
>  u64 kvm_call_hyp(void *hypfn, ...);
>  
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 03/11] KVM: ARM/arm64: avoid returning negative error code as bool

[Will Deacon]

is_valid_cache returns true if the specified cache is valid.
Unfortunately, if the parameter passed it out of range, we return
-ENOENT, which ends up as true leading to potential hilarity.

This patch returns false on the failure path instead.

Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kvm/coproc.c     | 2 +-
 arch/arm64/kvm/sys_regs.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
index 37a0fe1bb9bb..7928dbdf2102 100644
--- a/arch/arm/kvm/coproc.c
+++ b/arch/arm/kvm/coproc.c
@@ -791,7 +791,7 @@ static bool is_valid_cache(u32 val)
 	u32 level, ctype;
 
 	if (val >= CSSELR_MAX)
-		return -ENOENT;
+		return false;
 
 	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
         level = (val >> 1);
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 5805e7c4a4dd..4cc3b719208e 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1218,7 +1218,7 @@ static bool is_valid_cache(u32 val)
 	u32 level, ctype;
 
 	if (val >= CSSELR_MAX)
-		return -ENOENT;
+		return false;
 
 	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
 	level = (val >> 1);
-- 
2.1.0.rc1

[PATCH 03/11] KVM: ARM/arm64: avoid returning negative error code as bool

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> is_valid_cache returns true if the specified cache is valid.
> Unfortunately, if the parameter passed it out of range, we return
> -ENOENT, which ends up as true leading to potential hilarity.
> 
> This patch returns false on the failure path instead.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

With a brown paper bag on:

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
>  arch/arm/kvm/coproc.c     | 2 +-
>  arch/arm64/kvm/sys_regs.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 37a0fe1bb9bb..7928dbdf2102 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -791,7 +791,7 @@ static bool is_valid_cache(u32 val)
>  	u32 level, ctype;
>  
>  	if (val >= CSSELR_MAX)
> -		return -ENOENT;
> +		return false;
>  
>  	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
>          level = (val >> 1);
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 5805e7c4a4dd..4cc3b719208e 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1218,7 +1218,7 @@ static bool is_valid_cache(u32 val)
>  	u32 level, ctype;
>  
>  	if (val >= CSSELR_MAX)
> -		return -ENOENT;
> +		return false;
>  
>  	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
>  	level = (val >> 1);
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 03/11] KVM: ARM/arm64: avoid returning negative error code as bool

[Christoffer Dall]

On Tue, Aug 26, 2014 at 03:13:22PM +0100, Will Deacon wrote:
> is_valid_cache returns true if the specified cache is valid.
> Unfortunately, if the parameter passed it out of range, we return
> -ENOENT, which ends up as true leading to potential hilarity.
> 
> This patch returns false on the failure path instead.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

nice one!

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

[PATCH 04/11] KVM: ARM/arm64: return -EFAULT if copy_from_user fails in set_timer_reg

[Will Deacon]

We currently return the number of bytes not copied if set_timer_reg
fails, which is almost certainly not what userspace would like.

This patch returns -EFAULT instead.

Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kvm/guest.c   | 2 +-
 arch/arm64/kvm/guest.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
index 813e49258690..cc0b78769bd8 100644
--- a/arch/arm/kvm/guest.c
+++ b/arch/arm/kvm/guest.c
@@ -163,7 +163,7 @@ static int set_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 
 	ret = copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id));
 	if (ret != 0)
-		return ret;
+		return -EFAULT;
 
 	return kvm_arm_timer_set_reg(vcpu, reg->id, val);
 }
diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index 8d1ec2887a26..76794692c20b 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -174,7 +174,7 @@ static int set_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 
 	ret = copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id));
 	if (ret != 0)
-		return ret;
+		return -EFAULT;
 
 	return kvm_arm_timer_set_reg(vcpu, reg->id, val);
 }
-- 
2.1.0.rc1

[PATCH 04/11] KVM: ARM/arm64: return -EFAULT if copy_from_user fails in set_timer_reg

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> We currently return the number of bytes not copied if set_timer_reg
> fails, which is almost certainly not what userspace would like.
> 
> This patch returns -EFAULT instead.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
>  arch/arm/kvm/guest.c   | 2 +-
>  arch/arm64/kvm/guest.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kvm/guest.c b/arch/arm/kvm/guest.c
> index 813e49258690..cc0b78769bd8 100644
> --- a/arch/arm/kvm/guest.c
> +++ b/arch/arm/kvm/guest.c
> @@ -163,7 +163,7 @@ static int set_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  
>  	ret = copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id));
>  	if (ret != 0)
> -		return ret;
> +		return -EFAULT;
>  
>  	return kvm_arm_timer_set_reg(vcpu, reg->id, val);
>  }
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 8d1ec2887a26..76794692c20b 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -174,7 +174,7 @@ static int set_timer_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  
>  	ret = copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id));
>  	if (ret != 0)
> -		return ret;
> +		return -EFAULT;
>  
>  	return kvm_arm_timer_set_reg(vcpu, reg->id, val);
>  }
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 04/11] KVM: ARM/arm64: return -EFAULT if copy_from_user fails in set_timer_reg

[Christoffer Dall]

On Tue, Aug 26, 2014 at 03:13:23PM +0100, Will Deacon wrote:
> We currently return the number of bytes not copied if set_timer_reg
> fails, which is almost certainly not what userspace would like.
> 
> This patch returns -EFAULT instead.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

[PATCH 05/11] KVM: vgic: return int instead of bool when checking I/O ranges

[Will Deacon]

vgic_ioaddr_overlap claims to return a bool, but in reality it returns
an int. Shut sparse up by fixing the type signature.

Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 virt/kvm/arm/vgic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 73eba793b17f..d1cfe672b9d7 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1690,7 +1690,7 @@ out:
 	return ret;
 }
 
-static bool vgic_ioaddr_overlap(struct kvm *kvm)
+static int vgic_ioaddr_overlap(struct kvm *kvm)
 {
 	phys_addr_t dist = kvm->arch.vgic.vgic_dist_base;
 	phys_addr_t cpu = kvm->arch.vgic.vgic_cpu_base;
-- 
2.1.0.rc1

[PATCH 05/11] KVM: vgic: return int instead of bool when checking I/O ranges

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> vgic_ioaddr_overlap claims to return a bool, but in reality it returns
> an int. Shut sparse up by fixing the type signature.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
>  virt/kvm/arm/vgic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 73eba793b17f..d1cfe672b9d7 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -1690,7 +1690,7 @@ out:
>  	return ret;
>  }
>  
> -static bool vgic_ioaddr_overlap(struct kvm *kvm)
> +static int vgic_ioaddr_overlap(struct kvm *kvm)
>  {
>  	phys_addr_t dist = kvm->arch.vgic.vgic_dist_base;
>  	phys_addr_t cpu = kvm->arch.vgic.vgic_cpu_base;
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 05/11] KVM: vgic: return int instead of bool when checking I/O ranges

[Christoffer Dall]

On Tue, Aug 26, 2014 at 03:13:24PM +0100, Will Deacon wrote:
> vgic_ioaddr_overlap claims to return a bool, but in reality it returns
> an int. Shut sparse up by fixing the type signature.
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

[PATCH 06/11] KVM: vgic: declare probe function pointer as const

[Will Deacon]

We extract the vgic probe function from the of_device_id data pointer,
which is const. Kill the sparse warning by ensuring that the local
function pointer is also marked as const.

Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 virt/kvm/arm/vgic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index d1cfe672b9d7..efe6eee2e7eb 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1557,8 +1557,8 @@ static const struct of_device_id vgic_ids[] = {
 int kvm_vgic_hyp_init(void)
 {
 	const struct of_device_id *matched_id;
-	int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
-			  const struct vgic_params **);
+	const int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
+				const struct vgic_params **);
 	struct device_node *vgic_node;
 	int ret;
 
-- 
2.1.0.rc1

[PATCH 06/11] KVM: vgic: declare probe function pointer as const

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> We extract the vgic probe function from the of_device_id data pointer,
> which is const. Kill the sparse warning by ensuring that the local
> function pointer is also marked as const.
> 
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
>  virt/kvm/arm/vgic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index d1cfe672b9d7..efe6eee2e7eb 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -1557,8 +1557,8 @@ static const struct of_device_id vgic_ids[] = {
>  int kvm_vgic_hyp_init(void)
>  {
>  	const struct of_device_id *matched_id;
> -	int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
> -			  const struct vgic_params **);
> +	const int (*vgic_probe)(struct device_node *,const struct vgic_ops **,
> +				const struct vgic_params **);
>  	struct device_node *vgic_node;
>  	int ret;
>  
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 06/11] KVM: vgic: declare probe function pointer as const

[Christoffer Dall]

On Tue, Aug 26, 2014 at 03:13:25PM +0100, Will Deacon wrote:
> We extract the vgic probe function from the of_device_id data pointer,
> which is const. Kill the sparse warning by ensuring that the local
> function pointer is also marked as const.
> 
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

[PATCH 07/11] irqchip: gic: make gic_default_routable_irq_domain_ops static

[Will Deacon]

The internal irq domain ops for the GIC are not used directly anywhere
else, so make them static. This gets rid of a sparse warning on the
file.

Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/irqchip/irq-gic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4b959e606fe8..dda6dbc23565 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -867,7 +867,7 @@ static int gic_routable_irq_domain_xlate(struct irq_domain *d,
 	return 0;
 }
 
-const struct irq_domain_ops gic_default_routable_irq_domain_ops = {
+static const struct irq_domain_ops gic_default_routable_irq_domain_ops = {
 	.map = gic_routable_irq_domain_map,
 	.unmap = gic_routable_irq_domain_unmap,
 	.xlate = gic_routable_irq_domain_xlate,
-- 
2.1.0.rc1

[PATCH 07/11] irqchip: gic: make gic_default_routable_irq_domain_ops static

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> The internal irq domain ops for the GIC are not used directly anywhere
> else, so make them static. This gets rid of a sparse warning on the
> file.
> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
>  drivers/irqchip/irq-gic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 4b959e606fe8..dda6dbc23565 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -867,7 +867,7 @@ static int gic_routable_irq_domain_xlate(struct irq_domain *d,
>  	return 0;
>  }
>  
> -const struct irq_domain_ops gic_default_routable_irq_domain_ops = {
> +static const struct irq_domain_ops gic_default_routable_irq_domain_ops = {
>  	.map = gic_routable_irq_domain_map,
>  	.unmap = gic_routable_irq_domain_unmap,
>  	.xlate = gic_routable_irq_domain_xlate,
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 08/11] irqchip: gic-v3: declare rdist as __percpu pointer to __iomem pointer

[Will Deacon]

The __percpu __iomem annotations on the rdist base are contradictory
and confuse static checkers such as sparse.

This patch fixes the anotations so that rdist is described as a __percpu
pointer to an __iomem pointer.

Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 drivers/irqchip/irq-gic-v3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 57eaa5a0b1e3..4d7b7c28c554 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -36,7 +36,7 @@
 struct gic_chip_data {
 	void __iomem		*dist_base;
 	void __iomem		**redist_base;
-	void __percpu __iomem	**rdist;
+	void __iomem * __percpu	*rdist;
 	struct irq_domain	*domain;
 	u64			redist_stride;
 	u32			redist_regions;
-- 
2.1.0.rc1

[PATCH 08/11] irqchip: gic-v3: declare rdist as __percpu pointer to __iomem pointer

[Marc Zyngier]

On 26/08/14 15:13, Will Deacon wrote:
> The __percpu __iomem annotations on the rdist base are contradictory
> and confuse static checkers such as sparse.
> 
> This patch fixes the anotations so that rdist is described as a __percpu
> pointer to an __iomem pointer.
> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.

> ---
>  drivers/irqchip/irq-gic-v3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 57eaa5a0b1e3..4d7b7c28c554 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -36,7 +36,7 @@
>  struct gic_chip_data {
>  	void __iomem		*dist_base;
>  	void __iomem		**redist_base;
> -	void __percpu __iomem	**rdist;
> +	void __iomem * __percpu	*rdist;
>  	struct irq_domain	*domain;
>  	u64			redist_stride;
>  	u32			redist_regions;
> 


-- 
Jazz is not dead. It just smells funny...

[PATCH 09/11] arm64: ptrace: fix compat hardware watchpoint reporting

[Will Deacon]

I'm not sure what I was on when I wrote this, but when iterating over
the hardware watchpoint array (hbp_watch_array), our index is off by
ARM_MAX_BRP, so we walk off the end of our thread_struct...

... except, a dodgy condition in the loop means that it never executes
at all (bp cannot be NULL).

This patch fixes the code so that we remove the bp check and use the
correct index for accessing the watchpoint structures.

Cc: <stable@vger.kernel.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/hw_breakpoint.h | 1 -
 arch/arm64/kernel/ptrace.c             | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index d064047612b1..52b484b6aa1a 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -79,7 +79,6 @@ static inline void decode_ctrl_reg(u32 reg,
  */
 #define ARM_MAX_BRP		16
 #define ARM_MAX_WRP		16
-#define ARM_MAX_HBP_SLOTS	(ARM_MAX_BRP + ARM_MAX_WRP)
 
 /* Virtual debug register bases. */
 #define AARCH64_DBG_REG_BVR	0
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 70526cfda056..2ac998878001 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -87,7 +87,8 @@ static void ptrace_hbptriggered(struct perf_event *bp,
 			break;
 		}
 	}
-	for (i = ARM_MAX_BRP; i < ARM_MAX_HBP_SLOTS && !bp; ++i) {
+
+	for (i = 0; i < ARM_MAX_WRP; ++i) {
 		if (current->thread.debug.hbp_watch[i] == bp) {
 			info.si_errno = -((i << 1) + 1);
 			break;
-- 
2.1.0.rc1

[PATCH 10/11] arm64: ptrace: fix compat reg getter/setter return values

[Will Deacon]

copy_{to,from}_user return the number of bytes remaining on failure, not
an error code.

This patch returns -EFAULT when the copy operation didn't complete,
rather than expose the number of bytes not copied directly to userspace.

Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/ptrace.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 2ac998878001..fe63ac5e9bf5 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -663,8 +663,10 @@ static int compat_gpr_get(struct task_struct *target,
 			kbuf += sizeof(reg);
 		} else {
 			ret = copy_to_user(ubuf, &reg, sizeof(reg));
-			if (ret)
+			if (ret) {
+				ret = -EFAULT;
 				break;
+			}
 
 			ubuf += sizeof(reg);
 		}
@@ -702,8 +704,10 @@ static int compat_gpr_set(struct task_struct *target,
 			kbuf += sizeof(reg);
 		} else {
 			ret = copy_from_user(&reg, ubuf, sizeof(reg));
-			if (ret)
-				return ret;
+			if (ret) {
+				ret = -EFAULT;
+				break;
+			}
 
 			ubuf += sizeof(reg);
 		}
-- 
2.1.0.rc1

[PATCH 11/11] arm64: perf: don't rely on layout of pt_regs when grabbing sp or pc

[Will Deacon]

The current perf_regs code relies on sp and pc sitting just off the end
of the pt_regs->regs array. This is ugly and fragile, so this patch
checks for these register explicitly and returns the appropriate field.

Cc: Jean Pihet <jean.pihet@linaro.org>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/perf_regs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
index 422ebd63b619..6762ad705587 100644
--- a/arch/arm64/kernel/perf_regs.c
+++ b/arch/arm64/kernel/perf_regs.c
@@ -24,6 +24,12 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
 			return regs->compat_lr;
 	}
 
+	if ((u32)idx == PERF_REG_ARM64_SP)
+		return regs->sp;
+
+	if ((u32)idx == PERF_REG_ARM64_PC)
+		return regs->pc;
+
 	return regs->regs[idx];
 }
 
-- 
2.1.0.rc1

[PATCH 11/11] arm64: perf: don't rely on layout of pt_regs when grabbing sp or pc

[Jean Pihet]

Hi Will,

On 26 August 2014 16:13, Will Deacon <will.deacon@arm.com> wrote:
> The current perf_regs code relies on sp and pc sitting just off the end
> of the pt_regs->regs array. This is ugly and fragile, so this patch
> checks for these register explicitly and returns the appropriate field.
That is correct but costs you some code to test the index.

Acked-by: Jean Pihet <jean.pihet@linaro.org>

Jean

>
> Cc: Jean Pihet <jean.pihet@linaro.org>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/kernel/perf_regs.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c
> index 422ebd63b619..6762ad705587 100644
> --- a/arch/arm64/kernel/perf_regs.c
> +++ b/arch/arm64/kernel/perf_regs.c
> @@ -24,6 +24,12 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
>                         return regs->compat_lr;
>         }
>
> +       if ((u32)idx == PERF_REG_ARM64_SP)
> +               return regs->sp;
> +
> +       if ((u32)idx == PERF_REG_ARM64_PC)
> +               return regs->pc;
> +
>         return regs->regs[idx];
>  }
>
> --
> 2.1.0.rc1
>

[PATCH 00/11] Fix arm64 issues highlighted by sparse and smatch

[Christoffer Dall]

On Tue, Aug 26, 2014 at 03:13:19PM +0100, Will Deacon wrote:
> Hi all,
> 
> After the static checking talk at kernel summit, I felt empowered to go
> and run sparse and smatch over the arm64 defconfig. As I anticipated,
> this spotted a few issues in the arch code, so I've fixed up the ones
> that aren't just noise in this series. Most are simply incorrect
> address space annotations, but fixing these is useful for spotting real
> issues in the future.
> 
> I don't think any of these fix critical bugs, but they all deserve to be
> addressed. I tagged the compat watchpoint fix for stable, as that code
> has been broken since we merged the architecture port.
> 
Thanks for these!

Do you want me to queue the kvm ones or did you have other plans to
upstream this entire series as one?

Also, do you consider these fixes for 3.17 (what about cc for stable in
that case), or for 3.18?

-Christoffer

[PATCH 00/11] Fix arm64 issues highlighted by sparse and smatch

[Will Deacon]

Hi Christoffer,

On Tue, Aug 26, 2014 at 03:56:14PM +0100, Christoffer Dall wrote:
> On Tue, Aug 26, 2014 at 03:13:19PM +0100, Will Deacon wrote:
> > Hi all,
> > 
> > After the static checking talk at kernel summit, I felt empowered to go
> > and run sparse and smatch over the arm64 defconfig. As I anticipated,
> > this spotted a few issues in the arch code, so I've fixed up the ones
> > that aren't just noise in this series. Most are simply incorrect
> > address space annotations, but fixing these is useful for spotting real
> > issues in the future.
> > 
> > I don't think any of these fix critical bugs, but they all deserve to be
> > addressed. I tagged the compat watchpoint fix for stable, as that code
> > has been broken since we merged the architecture port.
> > 
> 
> Do you want me to queue the kvm ones or did you have other plans to
> upstream this entire series as one?

They're all independent, so please take the kvm ones via your tree. I can
send a separate pull if you prefer? Jason -- would you like an
irqchip-specific pull for those?

> Also, do you consider these fixes for 3.17 (what about cc for stable in
> that case), or for 3.18?

The only one I CC'd stable for was the compat ptrace watchpoint patch,
because that's something that we've been getting wrong and will cause
breakage with a 32-bit GDB. I don't consider the others urgent (i.e. happy
if you wait until 3.18).

Cheers,

Will

[PATCH 00/11] Fix arm64 issues highlighted by sparse and smatch

[Christoffer Dall]

On Tue, Aug 26, 2014 at 06:05:29PM +0100, Will Deacon wrote:
> Hi Christoffer,
> 
> On Tue, Aug 26, 2014 at 03:56:14PM +0100, Christoffer Dall wrote:
> > On Tue, Aug 26, 2014 at 03:13:19PM +0100, Will Deacon wrote:
> > > Hi all,
> > > 
> > > After the static checking talk at kernel summit, I felt empowered to go
> > > and run sparse and smatch over the arm64 defconfig. As I anticipated,
> > > this spotted a few issues in the arch code, so I've fixed up the ones
> > > that aren't just noise in this series. Most are simply incorrect
> > > address space annotations, but fixing these is useful for spotting real
> > > issues in the future.
> > > 
> > > I don't think any of these fix critical bugs, but they all deserve to be
> > > addressed. I tagged the compat watchpoint fix for stable, as that code
> > > has been broken since we merged the architecture port.
> > > 
> > 
> > Do you want me to queue the kvm ones or did you have other plans to
> > upstream this entire series as one?
> 
> They're all independent, so please take the kvm ones via your tree. I can
> send a separate pull if you prefer? Jason -- would you like an
> irqchip-specific pull for those?
> 
> > Also, do you consider these fixes for 3.17 (what about cc for stable in
> > that case), or for 3.18?
> 
> The only one I CC'd stable for was the compat ptrace watchpoint patch,
> because that's something that we've been getting wrong and will cause
> breakage with a 32-bit GDB. I don't consider the others urgent (i.e. happy
> if you wait until 3.18).
> 
Applied to queue, thanks.
-Christoffer

[PATCH 00/11] Fix arm64 issues highlighted by sparse and smatch

[Jason Cooper]

On Tue, Aug 26, 2014 at 03:13:19PM +0100, Will Deacon wrote:
> Hi all,
> 
> After the static checking talk at kernel summit, I felt empowered to go
> and run sparse and smatch over the arm64 defconfig. As I anticipated,
> this spotted a few issues in the arch code, so I've fixed up the ones
> that aren't just noise in this series. Most are simply incorrect
> address space annotations, but fixing these is useful for spotting real
> issues in the future.
> 
> I don't think any of these fix critical bugs, but they all deserve to be
> addressed. I tagged the compat watchpoint fix for stable, as that code
> has been broken since we merged the architecture port.
> 
> Cheers,
> 
> Will
> 
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Jean Pihet <jean.pihet@linaro.org>
> 
> --->8
> 
> Will Deacon (11):
>   irqchip: gic: make gic_default_routable_irq_domain_ops static
>   irqchip: gic-v3: declare rdist as __percpu pointer to __iomem pointer

>  drivers/irqchip/irq-gic-v3.c           |  2 +-
>  drivers/irqchip/irq-gic.c              |  2 +-

Applied to irqchip/urgent (yes, not urgent, but intended for the -rc's)
with Marc's Ack's

thx,

Jason.