On Tue, Jun 28, 2022 at 03:57:38PM +0200, Ard Biesheuvel wrote:

On Tue, 28 Jun 2022 at 15:47, Sudeep Holla [off-list ref] wrote:

quoted

On Tue, Jun 28, 2022 at 03:16:26PM +0200, Ard Biesheuvel wrote:

quoted

On Tue, 28 Jun 2022 at 14:59, Sudeep Holla [off-list ref] wrote:

quoted

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 <redacted>
---
 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 (local)
[2] https://lore.kernel.org/r/20220628125346.693304-3-sudeep.holla@arm.com/ (local)

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.

quoted

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