Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 26 May 2020 19:25:27 +0200
From:   Oscar Carter <oscar.carter@gmx.com>
To:     Marc Zyngier <maz@kernel.org>
Cc:     Oscar Carter <oscar.carter@gmx.com>,
        Kees Cook <keescook@chromium.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Jason Cooper <jason@lakedaemon.net>,
        kernel-hardening@lists.openwall.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] drivers/irqchip: Remove function callback casts
Message-ID: <20200526172527.GA3465@ubuntu>
References: <20200524162220.10186-1-oscar.carter@gmx.com>
 <87tv04z3kq.wl-maz@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87tv04z3kq.wl-maz@kernel.org>
User-Agent: Mutt/1.10.1 (2018-07-13)
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, May 25, 2020 at 11:55:33AM +0100, Marc Zyngier wrote:
> On Sun, 24 May 2020 17:22:20 +0100,
> Oscar Carter <oscar.carter@gmx.com> wrote:
> >
> >  include/linux/acpi.h    | 11 +++++++++++
>
> You now need to Cc the ACPI maintainers.

Sorry for forgetting.

> >  include/linux/irqchip.h |  5 +++--
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index d661cd0ee64d..fed49b276a90 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1154,6 +1154,17 @@ struct acpi_probe_entry {
> >  			.driver_data =3D data, 				\
> >  		   }
> >
> > +#define ACPI_DECLARE_SUBTABLE_PROBE_ENTRY(table, name, table_id,	\
> > +					  subtable, valid, data, fn)	\
> > +	static const struct acpi_probe_entry __acpi_probe_##name	\
> > +		__used __section(__##table##_acpi_probe_table) =3D {	\
> > +			.id =3D table_id,					\
> > +			.type =3D subtable,				\
> > +			.subtable_valid =3D valid,			\
> > +			.probe_subtbl =3D (acpi_tbl_entry_handler)fn,	\
>
> It strikes me that under the guise of removing function casts, you are
> actually adding one! And this cast is actually hiding all sorts of
> sins (remove it, and see things exploding).

Yes, if I remove it I see things exploiding. I'm very sorry.

> I've fixed it with the patch below (ACPI is such a mess of data
> structure case...).

> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index d7006ef18a0d..3870e9d4d3a8 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -2117,7 +2117,7 @@ static void __init gic_acpi_setup_kvm_info(void)
>  }
>
>  static int __init
> -gic_acpi_init(struct acpi_subtable_header *header, const unsigned long =
end)
> +gic_acpi_init(union acpi_subtable_headers *header, const unsigned long =
end)
>  {
>  	struct acpi_madt_generic_distributor *dist;
>  	struct fwnode_handle *domain_handle;
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 30ab623343d3..fc431857ce90 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1593,7 +1593,7 @@ static void __init gic_acpi_setup_kvm_info(void)
>  	gic_set_kvm_info(&gic_v2_kvm_info);
>  }
>
> -static int __init gic_v2_acpi_init(struct acpi_subtable_header *header,
> +static int __init gic_v2_acpi_init(union acpi_subtable_headers *header,
>  				   const unsigned long end)
>  {
>  	struct acpi_madt_generic_distributor *dist;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index fed49b276a90..4f4ddbfce3d3 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1150,7 +1150,7 @@ struct acpi_probe_entry {
>  			.id =3D table_id,					\
>  			.type =3D subtable,				\
>  			.subtable_valid =3D valid,			\
> -			.probe_table =3D (acpi_tbl_table_handler)fn,	\
> +			.probe_table =3D fn,				\
>  			.driver_data =3D data, 				\
>  		   }
>
> @@ -1161,7 +1161,7 @@ struct acpi_probe_entry {
>  			.id =3D table_id,					\
>  			.type =3D subtable,				\
>  			.subtable_valid =3D valid,			\
> -			.probe_subtbl =3D (acpi_tbl_entry_handler)fn,	\
> +			.probe_subtbl =3D fn,				\
>  			.driver_data =3D data,				\
>  		}
>

Thanks for your help with this patch. I will do all the necessary changes
and I will resend a new version.

Do you mind if I add these two lines:

Co-developed-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>

in the next version to give you credit?

> Thanks,
>
> 	M.
>
> --
> Without deviation from the norm, progress is not possible.

Thanks,
Oscar