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

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

[Ard Biesheuvel]

> 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;
>

Hi all,

I noticed this patch in kvmarm/next, and I think it is wrong.

Sparse complains about the fact that you are assigning the value of a
pointer to const data to a pointer to non-const data, and this is not
what is happening. Dereferencing a function pointer does not give you
an lvalue, so a function pointer points to const data by definition,
and the assignment is fine and sparse is wrong. The fact that you can
squelch the warning by typing the function pointer as returning a
by-value const result only emphasizes how confused sparse gets about
this.

-- 
Ard.

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

[Will Deacon]

On Sun, Sep 07, 2014 at 11:34:57AM +0100, Ard Biesheuvel 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>
> > ---
> >  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;
> >
> 
> Hi all,

Hi Ard,

> I noticed this patch in kvmarm/next, and I think it is wrong.
> 
> Sparse complains about the fact that you are assigning the value of a
> pointer to const data to a pointer to non-const data, and this is not
> what is happening. Dereferencing a function pointer does not give you
> an lvalue, so a function pointer points to const data by definition,
> and the assignment is fine and sparse is wrong. The fact that you can
> squelch the warning by typing the function pointer as returning a
> by-value const result only emphasizes how confused sparse gets about
> this.

Great, if you're fixing sparse (looks like you are) then we can drop this
patch from -next.

Cheers,

Will

[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 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>