[PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Sudeep Holla]

Currently, the arch_efi_call_virt() assumes all users of it will have
defined a type 'efi_##f##_t' to make use of it. It is unnecessarily
forcing the users to create a new typedef when __efi_rt_asm_wrapper()
actually expects void pointer.

Simplify the arch_efi_call_virt() macro by using typeof(p->f) which must
be a pointer as required by __efi_rt_asm_wrapper() and eliminate the
explicit need for efi_##f##_t type for every user of this macro.

This change is done to align with implementations on other similar
architectures.

Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/include/asm/efi.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Hi,

Reference for this change [1] and in particular[2]

Regards,
Sudeep

[1] https://lore.kernel.org/r/20220628125346.693304-1-sudeep.holla@arm.com
[2] https://lore.kernel.org/r/20220628125346.693304-3-sudeep.holla@arm.com/

diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index 27218eabbf9a..d4a405c9b4b6 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -26,8 +26,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
 
 #define arch_efi_call_virt(p, f, args...)				\
 ({									\
-	efi_##f##_t *__f;						\
-	__f = p->f;							\
+	typeof(p->f) __f = p->f;					\
 	__f(args);							\
 })
 
-- 
2.37.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Ard Biesheuvel]

On Tue, 28 Jun 2022 at 14:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Currently, the arch_efi_call_virt() assumes all users of it will have
> defined a type 'efi_##f##_t' to make use of it. It is unnecessarily
> forcing the users to create a new typedef when __efi_rt_asm_wrapper()
> actually expects void pointer.
>
> Simplify the arch_efi_call_virt() macro by using typeof(p->f) which must
> be a pointer as required by __efi_rt_asm_wrapper() and eliminate the
> explicit need for efi_##f##_t type for every user of this macro.
>
> This change is done to align with implementations on other similar
> architectures.
>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Russell King <linux@armlinux.org.uk>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm/include/asm/efi.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> Hi,
>
> Reference for this change [1] and in particular[2]
>
> Regards,
> Sudeep
>
> [1] https://lore.kernel.org/r/20220628125346.693304-1-sudeep.holla@arm.com
> [2] https://lore.kernel.org/r/20220628125346.693304-3-sudeep.holla@arm.com/
>
> diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
> index 27218eabbf9a..d4a405c9b4b6 100644
> --- a/arch/arm/include/asm/efi.h
> +++ b/arch/arm/include/asm/efi.h
> @@ -26,8 +26,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
>
>  #define arch_efi_call_virt(p, f, args...)                              \
>  ({                                                                     \
> -       efi_##f##_t *__f;                                               \
> -       __f = p->f;                                                     \
> +       typeof(p->f) __f = p->f;                                        \
>         __f(args);                                                      \
>  })
>

I think this could simply be

#define arch_efi_call_virt(p, f, args...) ((p)->f(args))

no?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Sudeep Holla]

On Tue, Jun 28, 2022 at 03:16:26PM +0200, Ard Biesheuvel wrote:
> On Tue, 28 Jun 2022 at 14:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > Currently, the arch_efi_call_virt() assumes all users of it will have
> > defined a type 'efi_##f##_t' to make use of it. It is unnecessarily
> > forcing the users to create a new typedef when __efi_rt_asm_wrapper()
> > actually expects void pointer.
> >
> > Simplify the arch_efi_call_virt() macro by using typeof(p->f) which must
> > be a pointer as required by __efi_rt_asm_wrapper() and eliminate the
> > explicit need for efi_##f##_t type for every user of this macro.
> >
> > This change is done to align with implementations on other similar
> > architectures.
> >
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  arch/arm/include/asm/efi.h | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > Hi,
> >
> > Reference for this change [1] and in particular[2]
> >
> > Regards,
> > Sudeep
> >
> > [1] https://lore.kernel.org/r/20220628125346.693304-1-sudeep.holla@arm.com
> > [2] https://lore.kernel.org/r/20220628125346.693304-3-sudeep.holla@arm.com/
> >
> > diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
> > index 27218eabbf9a..d4a405c9b4b6 100644
> > --- a/arch/arm/include/asm/efi.h
> > +++ b/arch/arm/include/asm/efi.h
> > @@ -26,8 +26,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
> >
> >  #define arch_efi_call_virt(p, f, args...)                              \
> >  ({                                                                     \
> > -       efi_##f##_t *__f;                                               \
> > -       __f = p->f;                                                     \
> > +       typeof(p->f) __f = p->f;                                        \
> >         __f(args);                                                      \
> >  })
> >
> 
> I think this could simply be
> 
> #define arch_efi_call_virt(p, f, args...) ((p)->f(args))
> 
> no?

Yes, I came to similar conclusion just after sending this out as I started to
look if we can have one generic definition for arm/arm64/riscv/loongarch.

I am yet to figure out how asm/efi.h and linux/efi.h are included so that
we can have generic definition in linux/efi.h and x86 can undefine that
and redefine its own version.

Does that make sense ?

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Ard Biesheuvel]

On Tue, 28 Jun 2022 at 15:47, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Tue, Jun 28, 2022 at 03:16:26PM +0200, Ard Biesheuvel wrote:
> > On Tue, 28 Jun 2022 at 14:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > Currently, the arch_efi_call_virt() assumes all users of it will have
> > > defined a type 'efi_##f##_t' to make use of it. It is unnecessarily
> > > forcing the users to create a new typedef when __efi_rt_asm_wrapper()
> > > actually expects void pointer.
> > >
> > > Simplify the arch_efi_call_virt() macro by using typeof(p->f) which must
> > > be a pointer as required by __efi_rt_asm_wrapper() and eliminate the
> > > explicit need for efi_##f##_t type for every user of this macro.
> > >
> > > This change is done to align with implementations on other similar
> > > architectures.
> > >
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Cc: Russell King <linux@armlinux.org.uk>
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > ---
> > >  arch/arm/include/asm/efi.h | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > Hi,
> > >
> > > Reference for this change [1] and in particular[2]
> > >
> > > Regards,
> > > Sudeep
> > >
> > > [1] https://lore.kernel.org/r/20220628125346.693304-1-sudeep.holla@arm.com
> > > [2] https://lore.kernel.org/r/20220628125346.693304-3-sudeep.holla@arm.com/
> > >
> > > diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
> > > index 27218eabbf9a..d4a405c9b4b6 100644
> > > --- a/arch/arm/include/asm/efi.h
> > > +++ b/arch/arm/include/asm/efi.h
> > > @@ -26,8 +26,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
> > >
> > >  #define arch_efi_call_virt(p, f, args...)                              \
> > >  ({                                                                     \
> > > -       efi_##f##_t *__f;                                               \
> > > -       __f = p->f;                                                     \
> > > +       typeof(p->f) __f = p->f;                                        \
> > >         __f(args);                                                      \
> > >  })
> > >
> >
> > I think this could simply be
> >
> > #define arch_efi_call_virt(p, f, args...) ((p)->f(args))
> >
> > no?
>
> Yes, I came to similar conclusion just after sending this out as I started to
> look if we can have one generic definition for arm/arm64/riscv/loongarch.
>

Not really - arm64 has the asm wrapper, and loongarch is only halfway
merged so I'm not sure yet if this is the final form.

> I am yet to figure out how asm/efi.h and linux/efi.h are included so that
> we can have generic definition in linux/efi.h and x86 can undefine that
> and redefine its own version.
>
> Does that make sense ?
>

I appreciate the effort, but for now, let's just fix the ones we need
to fix (and the ARM one too while we're at it). PRM can only be
enabled on x86 and arm64 anyway.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Sudeep Holla]

On Tue, Jun 28, 2022 at 03:57:38PM +0200, Ard Biesheuvel wrote:
> On Tue, 28 Jun 2022 at 15:47, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Tue, Jun 28, 2022 at 03:16:26PM +0200, Ard Biesheuvel wrote:
> > > On Tue, 28 Jun 2022 at 14:59, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > Currently, the arch_efi_call_virt() assumes all users of it will have
> > > > defined a type 'efi_##f##_t' to make use of it. It is unnecessarily
> > > > forcing the users to create a new typedef when __efi_rt_asm_wrapper()
> > > > actually expects void pointer.
> > > >
> > > > Simplify the arch_efi_call_virt() macro by using typeof(p->f) which must
> > > > be a pointer as required by __efi_rt_asm_wrapper() and eliminate the
> > > > explicit need for efi_##f##_t type for every user of this macro.
> > > >
> > > > This change is done to align with implementations on other similar
> > > > architectures.
> > > >
> > > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > > Cc: Russell King <linux@armlinux.org.uk>
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > > ---
> > > >  arch/arm/include/asm/efi.h | 3 +--
> > > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > >
> > > > Hi,
> > > >
> > > > Reference for this change [1] and in particular[2]
> > > >
> > > > Regards,
> > > > Sudeep
> > > >
> > > > [1] https://lore.kernel.org/r/20220628125346.693304-1-sudeep.holla@arm.com
> > > > [2] https://lore.kernel.org/r/20220628125346.693304-3-sudeep.holla@arm.com/
> > > >
> > > > diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
> > > > index 27218eabbf9a..d4a405c9b4b6 100644
> > > > --- a/arch/arm/include/asm/efi.h
> > > > +++ b/arch/arm/include/asm/efi.h
> > > > @@ -26,8 +26,7 @@ int efi_set_mapping_permissions(struct mm_struct *mm, efi_memory_desc_t *md);
> > > >
> > > >  #define arch_efi_call_virt(p, f, args...)                              \
> > > >  ({                                                                     \
> > > > -       efi_##f##_t *__f;                                               \
> > > > -       __f = p->f;                                                     \
> > > > +       typeof(p->f) __f = p->f;                                        \
> > > >         __f(args);                                                      \
> > > >  })
> > > >
> > >
> > > I think this could simply be
> > >
> > > #define arch_efi_call_virt(p, f, args...) ((p)->f(args))
> > >
> > > no?
> >
> > Yes, I came to similar conclusion just after sending this out as I started to
> > look if we can have one generic definition for arm/arm64/riscv/loongarch.
> >
> 
> Not really - arm64 has the asm wrapper, and loongarch is only halfway
> merged so I'm not sure yet if this is the final form.
>

Aargh! arm64 was typo, indeed arm64 has wrapper. I meant to refer other 3 archs.

> > I am yet to figure out how asm/efi.h and linux/efi.h are included so that
> > we can have generic definition in linux/efi.h and x86 can undefine that
> > and redefine its own version.
> >
> > Does that make sense ?
> >
> 
> I appreciate the effort, but for now, let's just fix the ones we need
> to fix (and the ARM one too while we're at it). PRM can only be
> enabled on x86 and arm64 anyway.

True. OK then I will just update ARM version and leave loongarch as is.

--
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Ard Biesheuvel]

On Tue, 28 Jun 2022 at 16:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Tue, Jun 28, 2022 at 03:57:38PM +0200, Ard Biesheuvel wrote:
> > On Tue, 28 Jun 2022 at 15:47, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
...

> > > I am yet to figure out how asm/efi.h and linux/efi.h are included so that
> > > we can have generic definition in linux/efi.h and x86 can undefine that
> > > and redefine its own version.
> > >
> > > Does that make sense ?
> > >
> >
> > I appreciate the effort, but for now, let's just fix the ones we need
> > to fix (and the ARM one too while we're at it). PRM can only be
> > enabled on x86 and arm64 anyway.
>
> True. OK then I will just update ARM version and leave loongarch as is.
>

Actually, this was rather straight-forward so I folded this change
into your ARM patch.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Sudeep Holla]

On Tue, Jun 28, 2022 at 07:58:38PM +0200, Ard Biesheuvel wrote:
> On Tue, 28 Jun 2022 at 16:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Tue, Jun 28, 2022 at 03:57:38PM +0200, Ard Biesheuvel wrote:
> > > On Tue, 28 Jun 2022 at 15:47, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> ...
> 
> > > > I am yet to figure out how asm/efi.h and linux/efi.h are included so that
> > > > we can have generic definition in linux/efi.h and x86 can undefine that
> > > > and redefine its own version.
> > > >
> > > > Does that make sense ?
> > > >
> > >
> > > I appreciate the effort, but for now, let's just fix the ones we need
> > > to fix (and the ARM one too while we're at it). PRM can only be
> > > enabled on x86 and arm64 anyway.
> >
> > True. OK then I will just update ARM version and leave loongarch as is.
> >
> 
> Actually, this was rather straight-forward so I folded this change
> into your ARM patch.

I see you have the generic version for all archs except arm64 and x86 as
we discussed earlier. Since you have even included the arm64 changes, the
PRMT enablement patches need to routed via your tree now as it depends on
the change you have in your -next.

Are you OK with that if Rafael agrees ? I can ask him on the other thread.
No further changes are needed. Let me know.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Ard Biesheuvel]

On Wed, 29 Jun 2022 at 10:57, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Tue, Jun 28, 2022 at 07:58:38PM +0200, Ard Biesheuvel wrote:
> > On Tue, 28 Jun 2022 at 16:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Tue, Jun 28, 2022 at 03:57:38PM +0200, Ard Biesheuvel wrote:
> > > > On Tue, 28 Jun 2022 at 15:47, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > >
> > ...
> >
> > > > > I am yet to figure out how asm/efi.h and linux/efi.h are included so that
> > > > > we can have generic definition in linux/efi.h and x86 can undefine that
> > > > > and redefine its own version.
> > > > >
> > > > > Does that make sense ?
> > > > >
> > > >
> > > > I appreciate the effort, but for now, let's just fix the ones we need
> > > > to fix (and the ARM one too while we're at it). PRM can only be
> > > > enabled on x86 and arm64 anyway.
> > >
> > > True. OK then I will just update ARM version and leave loongarch as is.
> > >
> >
> > Actually, this was rather straight-forward so I folded this change
> > into your ARM patch.
>
> I see you have the generic version for all archs except arm64 and x86 as
> we discussed earlier. Since you have even included the arm64 changes, the
> PRMT enablement patches need to routed via your tree now as it depends on
> the change you have in your -next.
>
> Are you OK with that if Rafael agrees ? I can ask him on the other thread.
> No further changes are needed. Let me know.
>

Yes, that is fine. Or I can put that patch on a stable branch by itself.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: efi: Simplify arch_efi_call_virt() macro by using typeof()

[Sudeep Holla]

On Wed, Jun 29, 2022 at 10:58:29AM +0200, Ard Biesheuvel wrote:
> On Wed, 29 Jun 2022 at 10:57, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Tue, Jun 28, 2022 at 07:58:38PM +0200, Ard Biesheuvel wrote:
> > > On Tue, 28 Jun 2022 at 16:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > On Tue, Jun 28, 2022 at 03:57:38PM +0200, Ard Biesheuvel wrote:
> > > > > On Tue, 28 Jun 2022 at 15:47, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > > >
> > > ...
> > >
> > > > > > I am yet to figure out how asm/efi.h and linux/efi.h are included so that
> > > > > > we can have generic definition in linux/efi.h and x86 can undefine that
> > > > > > and redefine its own version.
> > > > > >
> > > > > > Does that make sense ?
> > > > > >
> > > > >
> > > > > I appreciate the effort, but for now, let's just fix the ones we need
> > > > > to fix (and the ARM one too while we're at it). PRM can only be
> > > > > enabled on x86 and arm64 anyway.
> > > >
> > > > True. OK then I will just update ARM version and leave loongarch as is.
> > > >
> > >
> > > Actually, this was rather straight-forward so I folded this change
> > > into your ARM patch.
> >
> > I see you have the generic version for all archs except arm64 and x86 as
> > we discussed earlier. Since you have even included the arm64 changes, the
> > PRMT enablement patches need to routed via your tree now as it depends on
> > the change you have in your -next.
> >
> > Are you OK with that if Rafael agrees ? I can ask him on the other thread.
> > No further changes are needed. Let me know.
> >
> 
> Yes, that is fine. Or I can put that patch on a stable branch by itself.

Thanks I will check with Rafael now.

-- 
Regards,
Sudeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel