[PATCH 0/3] ACPI: Enable Platform Runtime Mechanism(PRM) support on ARM64

[PATCH 0/3] ACPI: Enable Platform Runtime Mechanism(PRM) support on ARM64

[Sudeep Holla]

Hi,

This tiny series is to enable Platform Runtime Mechanism(PRM) support on
ARM64. Not much changes. Just enabling the build and moving the config
option so that it is not listed under the topmost menu for ARM64.

Regards,
Sudeep

Sudeep Holla (3):
  arm64: efi: Simplify arch_efi_call_virt macro by not using efi_##f##_t type
  ACPI: Enable Platform Runtime Mechanism(PRM) support on ARM64
  ACPI: Move PRM config option under the main ACPI config

 arch/arm64/include/asm/efi.h |  4 +---
 drivers/acpi/Kconfig         | 30 +++++++++++++++---------------
 2 files changed, 16 insertions(+), 18 deletions(-)

--
2.36.1

[PATCH 1/3] arm64: efi: Simplify arch_efi_call_virt macro by not using efi_##f##_t type

[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 typecasting p->f to (void *)
as required by __efi_rt_asm_wrapper() and eliminate the explicit need
for efi_##f##_t type for every user of this macro.

This is needed now in preparation to enable PRMT support on ARM64.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/include/asm/efi.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Hi Ard,

I am not sure if you prefer to add type for each users of this or this
is acceptable. I see only compile time advantage but I am not sure if
it make sense to add typedefs in ACPI PRMT driver just for this reason.

Let me know.

Regards,
Sudeep

diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index ad55079abe47..263d7fd67207 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -29,9 +29,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;							\
-	__efi_rt_asm_wrapper(__f, #f, args);				\
+	__efi_rt_asm_wrapper((void *)p->f, #f, args);			\
 })
 
 #define arch_efi_call_virt_teardown()					\
-- 
2.36.1

Re: [PATCH 1/3] arm64: efi: Simplify arch_efi_call_virt macro by not using efi_##f##_t type

[Ard Biesheuvel]

On Fri, 24 Jun 2022 at 17:23, 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 typecasting p->f to (void *)
> as required by __efi_rt_asm_wrapper() and eliminate the explicit need
> for efi_##f##_t type for every user of this macro.
>

Can't we just use typeof() here?

__efi_rt_asm_wrapper() was intended as a temporary thing, so I'd
prefer to avoid starting to rely on the void* type of its first
argument.


> This is needed now in preparation to enable PRMT support on ARM64.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/include/asm/efi.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> Hi Ard,
>
> I am not sure if you prefer to add type for each users of this or this
> is acceptable. I see only compile time advantage but I am not sure if
> it make sense to add typedefs in ACPI PRMT driver just for this reason.
>
> Let me know.
>
> Regards,
> Sudeep
>
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index ad55079abe47..263d7fd67207 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -29,9 +29,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;                                                     \
> -       __efi_rt_asm_wrapper(__f, #f, args);                            \
> +       __efi_rt_asm_wrapper((void *)p->f, #f, args);                   \
>  })
>
>  #define arch_efi_call_virt_teardown()                                  \
> --
> 2.36.1
>

Re: [PATCH 1/3] arm64: efi: Simplify arch_efi_call_virt macro by not using efi_##f##_t type

[Sudeep Holla]

On Fri, Jun 24, 2022 at 07:45:14PM +0200, Ard Biesheuvel wrote:
> On Fri, 24 Jun 2022 at 17:23, 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 typecasting p->f to (void *)
> > as required by __efi_rt_asm_wrapper() and eliminate the explicit need
> > for efi_##f##_t type for every user of this macro.
> >
>
> Can't we just use typeof() here?

I had tried that, but unless p->f is pointer of some type, we will get
the warning as it is passed without a cast to __efi_rt_asm_wrapper().

> __efi_rt_asm_wrapper() was intended as a temporary thing, so I'd
> prefer to avoid starting to rely on the void* type of its first
> argument.
>

Fair enough. Can we expect p->f to be some pointer then ? If yes, then
PRMT driver needs to change the handler_addr from u64 to some pointer
which sounds OK to me.

--
Regards,
Sudeep

Re: [PATCH 1/3] arm64: efi: Simplify arch_efi_call_virt macro by not using efi_##f##_t type

[Ard Biesheuvel]

On Sat, 25 Jun 2022 at 12:11, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Fri, Jun 24, 2022 at 07:45:14PM +0200, Ard Biesheuvel wrote:
> > On Fri, 24 Jun 2022 at 17:23, 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 typecasting p->f to (void *)
> > > as required by __efi_rt_asm_wrapper() and eliminate the explicit need
> > > for efi_##f##_t type for every user of this macro.
> > >
> >
> > Can't we just use typeof() here?
>
> I had tried that, but unless p->f is pointer of some type, we will get
> the warning as it is passed without a cast to __efi_rt_asm_wrapper().
>
> > __efi_rt_asm_wrapper() was intended as a temporary thing, so I'd
> > prefer to avoid starting to rely on the void* type of its first
> > argument.
> >
>
> Fair enough. Can we expect p->f to be some pointer then ? If yes, then
> PRMT driver needs to change the handler_addr from u64 to some pointer
> which sounds OK to me.
>

We are dealing with function pointers here, so passing those as u64 is
just sloppy.

[PATCH 2/3] ACPI: Enable Platform Runtime Mechanism(PRM) support on ARM64

[Sudeep Holla]

There is interest to make use of PRM(Platform Runtime Mechanism) even on
ARM64 ACPI platforms. Allow PRM to be enabled on ARM64 platforms. It will
be enabled by default as on x86_64.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/acpi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 1e34f846508f..d08b7408f0a5 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -592,7 +592,7 @@ config X86_PM_TIMER
 
 config ACPI_PRMT
 	bool "Platform Runtime Mechanism Support"
-	depends on EFI && X86_64
+	depends on EFI && (X86_64 || ARM64)
 	default y
 	help
 	  Platform Runtime Mechanism (PRM) is a firmware interface exposing a
-- 
2.36.1

Re: [PATCH 2/3] ACPI: Enable Platform Runtime Mechanism(PRM) support on ARM64

[Ard Biesheuvel]

On Fri, 24 Jun 2022 at 17:23, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> There is interest to make use of PRM(Platform Runtime Mechanism) even on
> ARM64 ACPI platforms. Allow PRM to be enabled on ARM64 platforms. It will
> be enabled by default as on x86_64.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  drivers/acpi/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 1e34f846508f..d08b7408f0a5 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -592,7 +592,7 @@ config X86_PM_TIMER
>
>  config ACPI_PRMT
>         bool "Platform Runtime Mechanism Support"
> -       depends on EFI && X86_64
> +       depends on EFI && (X86_64 || ARM64)
>         default y
>         help
>           Platform Runtime Mechanism (PRM) is a firmware interface exposing a
> --
> 2.36.1
>

[PATCH 3/3] ACPI: Move PRM config option under the main ACPI config

[Sudeep Holla]

Currently PRM(Platform Runtime Mechanism) config options is list along
with the mainA CPI (Advanced Configuration and Power Interface) option
at the same level. On ARM64 platforms unlike x86, ACPI option is listed
at the topmost level of configuration menu. It is rather very confusing
to see PRM option also listed along with ACPI in the topmost level.

Move the same under ACPI config option. No functional change, just changes
the level of visibility of this option under the configuration menu.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/acpi/Kconfig | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index d08b7408f0a5..218b5b59df31 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -572,6 +572,21 @@ source "drivers/acpi/pmic/Kconfig"
 config ACPI_VIOT
 	bool
 
+config ACPI_PRMT
+	bool "Platform Runtime Mechanism Support"
+	depends on EFI && (X86_64 || ARM64)
+	default y
+	help
+	  Platform Runtime Mechanism (PRM) is a firmware interface exposing a
+	  set of binary executables that can be called from the AML interpreter
+	  or directly from device drivers.
+
+	  Say Y to enable the AML interpreter to execute the PRM code.
+
+	  While this feature is optional in principle, leaving it out may
+	  substantially increase computational overhead related to the
+	  initialization of some server systems.
+
 endif	# ACPI
 
 config X86_PM_TIMER
@@ -589,18 +604,3 @@ config X86_PM_TIMER
 
 	  You should nearly always say Y here because many modern
 	  systems require this timer.
-
-config ACPI_PRMT
-	bool "Platform Runtime Mechanism Support"
-	depends on EFI && (X86_64 || ARM64)
-	default y
-	help
-	  Platform Runtime Mechanism (PRM) is a firmware interface exposing a
-	  set of binary executables that can be called from the AML interpreter
-	  or directly from device drivers.
-
-	  Say Y to enable the AML interpreter to execute the PRM code.
-
-	  While this feature is optional in principle, leaving it out may
-	  substantially increase computational overhead related to the
-	  initialization of some server systems.
-- 
2.36.1

Re: [PATCH 3/3] ACPI: Move PRM config option under the main ACPI config

[Ard Biesheuvel]

On Fri, 24 Jun 2022 at 17:23, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Currently PRM(Platform Runtime Mechanism) config options is list along
> with the mainA CPI (Advanced Configuration and Power Interface) option
> at the same level. On ARM64 platforms unlike x86, ACPI option is listed
> at the topmost level of configuration menu. It is rather very confusing
> to see PRM option also listed along with ACPI in the topmost level.
>
> Move the same under ACPI config option. No functional change, just changes
> the level of visibility of this option under the configuration menu.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  drivers/acpi/Kconfig | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index d08b7408f0a5..218b5b59df31 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -572,6 +572,21 @@ source "drivers/acpi/pmic/Kconfig"
>  config ACPI_VIOT
>         bool
>
> +config ACPI_PRMT
> +       bool "Platform Runtime Mechanism Support"
> +       depends on EFI && (X86_64 || ARM64)
> +       default y
> +       help
> +         Platform Runtime Mechanism (PRM) is a firmware interface exposing a
> +         set of binary executables that can be called from the AML interpreter
> +         or directly from device drivers.
> +
> +         Say Y to enable the AML interpreter to execute the PRM code.
> +
> +         While this feature is optional in principle, leaving it out may
> +         substantially increase computational overhead related to the
> +         initialization of some server systems.
> +
>  endif  # ACPI
>
>  config X86_PM_TIMER
> @@ -589,18 +604,3 @@ config X86_PM_TIMER
>
>           You should nearly always say Y here because many modern
>           systems require this timer.
> -
> -config ACPI_PRMT
> -       bool "Platform Runtime Mechanism Support"
> -       depends on EFI && (X86_64 || ARM64)
> -       default y
> -       help
> -         Platform Runtime Mechanism (PRM) is a firmware interface exposing a
> -         set of binary executables that can be called from the AML interpreter
> -         or directly from device drivers.
> -
> -         Say Y to enable the AML interpreter to execute the PRM code.
> -
> -         While this feature is optional in principle, leaving it out may
> -         substantially increase computational overhead related to the
> -         initialization of some server systems.
> --
> 2.36.1
>