[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <87sgfmzd8g.wl-maz@kernel.org>
Date: Tue, 26 May 2020 20:51:27 +0100
From: Marc Zyngier <maz@...nel.org>
To: Oscar Carter <oscar.carter@....com>
Cc: Kees Cook <keescook@...omium.org>,
Thomas Gleixner <tglx@...utronix.de>,
Jason Cooper <jason@...edaemon.net>,
kernel-hardening@...ts.openwall.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drivers/irqchip: Remove function callback casts
On Tue, 26 May 2020 18:25:27 +0100,
Oscar Carter <oscar.carter@....com> wrote:
>
> 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@....com> wrote:
> > >
> > > include/linux/acpi.h | 11 +++++++++++
> >
> > You now need to Cc the ACPI maintainers.
>
> Sorry for forgetting.
No worries. In general, use scripts/get_maintainers.pl to find out who
to Cc.
>
> > > 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 = 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) = { \
> > > + .id = table_id, \
> > > + .type = subtable, \
> > > + .subtable_valid = valid, \
> > > + .probe_subtbl = (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.
No need to be sorry. We all learn from past mistakes.
>
> > 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 = table_id, \
> > .type = subtable, \
> > .subtable_valid = valid, \
> > - .probe_table = (acpi_tbl_table_handler)fn, \
> > + .probe_table = fn, \
> > .driver_data = data, \
> > }
> >
> > @@ -1161,7 +1161,7 @@ struct acpi_probe_entry {
> > .id = table_id, \
> > .type = subtable, \
> > .subtable_valid = valid, \
> > - .probe_subtbl = (acpi_tbl_entry_handler)fn, \
> > + .probe_subtbl = fn, \
> > .driver_data = 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@...nel.org>
> Signed-off-by: Marc Zyngier <maz@...nel.org>
>
> in the next version to give you credit?
Sure, that's very kind of you to offer.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
