On Tue, 28 Jun 2022 at 15:47, Sudeep Holla <sudeep.holla@xxxxxxx> 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@xxxxxxx> 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@xxxxxxxxxx> > > > Cc: Russell King <linux@xxxxxxxxxxxxxxx> > > > Signed-off-by: Sudeep Holla <sudeep.holla@xxxxxxx> > > > --- > > > 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@xxxxxxx > > > [2] https://lore.kernel.org/r/20220628125346.693304-3-sudeep.holla@xxxxxxx/ > > > > > > 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.
