All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure
@ 2022-05-22 21:43 james.liu
       [not found] ` <PH7PR84MB1958F5A9F4F05EFE72A9E906E6A29@PH7PR84MB1958.NAMPRD84.PROD.OUTLOOK.COM>
  2022-06-29 14:27 ` Rafael J. Wysocki
  0 siblings, 2 replies; 3+ messages in thread
From: james.liu @ 2022-05-22 21:43 UTC (permalink / raw)
  To: rafael, lenb
  Cc: linux-acpi, linux-kernel, rwright, david.chang, clayc, james.liu

From: James Liu <james.liu@hpe.com>

    Modify acpi_os_map_generic_address() & acpi_os_unmap_generic_address()
    to correctly handle cases that a GAS table (i.e., Table 5.1, ACPI 6.4)
    is used to address a data structure; in the case, the GAS has the field
    of "Register Bit Width" equal to 0.

    For example, "Injection Instruction Entry" (Table 18.25, ACPI 6.4) has
    a RegisterRegion field that is a GAS that points to a data structure
    SET_ERROR_TYPE_WITH_ADDRESS (Table 18.30), which is required when using
    EINJ (Error Injection module).

    This fix preserves a fairly sufficient memory space (i.e. page size) to
    store the data structure to prevent EINJ module from loading failure if
    platform firmware can correctly support Injection Instruction Entry in
    an EINJ table.

Signed-off-by: James Liu <james.liu@hpe.com>
---
 drivers/acpi/osl.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 45c5c0e45..99f987c8c 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -452,14 +452,20 @@ EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
 
 void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
 {
-	u64 addr;
+	u64 addr = 0;
 
 	if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
 		return NULL;
 
+	/* Handle a case that GAS is used to address an ACPI data structure */
+	if (!gas->bit_width) {
+		pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
+		return  acpi_os_map_iomem(addr, PAGE_SIZE);
+	}
+
 	/* Handle possible alignment issues */
 	memcpy(&addr, &gas->address, sizeof(addr));
-	if (!addr || !gas->bit_width)
+	if (!addr)
 		return NULL;
 
 	return acpi_os_map_iomem(addr, gas->bit_width / 8);
@@ -468,15 +474,28 @@ EXPORT_SYMBOL(acpi_os_map_generic_address);
 
 void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
 {
-	u64 addr;
+	u64 addr = 0;
 	struct acpi_ioremap *map;
 
 	if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
 		return;
 
+	/* Handle a case that GAS is used to address an ACPI data structure */
+	if (!gas->bit_width) {
+		pr_info("An ACPI data structure at 0x%llx is unmapped\n", addr);
+		mutex_lock(&acpi_ioremap_lock);
+		map = acpi_map_lookup(addr, PAGE_SIZE);
+		if (!map) {
+			mutex_unlock(&acpi_ioremap_lock);
+			return;
+		}
+		acpi_os_drop_map_ref(map);
+		mutex_unlock(&acpi_ioremap_lock);
+	}
+
 	/* Handle possible alignment issues */
 	memcpy(&addr, &gas->address, sizeof(addr));
-	if (!addr || !gas->bit_width)
+	if (!addr)
 		return;
 
 	mutex_lock(&acpi_ioremap_lock);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH v2] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure
       [not found] ` <PH7PR84MB1958F5A9F4F05EFE72A9E906E6A29@PH7PR84MB1958.NAMPRD84.PROD.OUTLOOK.COM>
@ 2022-06-06  9:16   ` James Liu
  0 siblings, 0 replies; 3+ messages in thread
From: James Liu @ 2022-06-06  9:16 UTC (permalink / raw)
  To: rafael; +Cc: lenb, linux-acpi, linux-kernel

Hi Rafael, the reported warning (i.e., uninitialized local variable) in V1
by the robot has been fixed. Could you check V2 of this patch? Thanks.

> Subject: [PATCH v2] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure 
>  
> From: James Liu <james.liu@hpe.com>
> 
>     Modify acpi_os_map_generic_address() & acpi_os_unmap_generic_address()
>     to correctly handle cases that a GAS table (i.e., Table 5.1, ACPI 6.4)
>     is used to address a data structure; in the case, the GAS has the field
>     of "Register Bit Width" equal to 0.
> 
>     For example, "Injection Instruction Entry" (Table 18.25, ACPI 6.4) has
>     a RegisterRegion field that is a GAS that points to a data structure
>     SET_ERROR_TYPE_WITH_ADDRESS (Table 18.30), which is required when using
>     EINJ (Error Injection module).
> 
>     This fix preserves a fairly sufficient memory space (i.e. page size) to
>     store the data structure to prevent EINJ module from loading failure if
>     platform firmware can correctly support Injection Instruction Entry in
>     an EINJ table.
> 
> Signed-off-by: James Liu <james.liu@hpe.com>
> ---
>  drivers/acpi/osl.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 45c5c0e45..99f987c8c 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -452,14 +452,20 @@ EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
>  
>  void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
>  {
> -       u64 addr;
> +       u64 addr = 0;
>  
>          if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
>                  return NULL;
>  
> +       /* Handle a case that GAS is used to address an ACPI data structure */
> +       if (!gas->bit_width) {
> +               pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
> +               return  acpi_os_map_iomem(addr, PAGE_SIZE);
> +       }
> +
>          /* Handle possible alignment issues */
>          memcpy(&addr, &gas->address, sizeof(addr));
> -       if (!addr || !gas->bit_width)
> +       if (!addr)
>                  return NULL;
>  
>          return acpi_os_map_iomem(addr, gas->bit_width / 8);
> @@ -468,15 +474,28 @@ EXPORT_SYMBOL(acpi_os_map_generic_address);
>  
>  void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
>  {
> -       u64 addr;
> +       u64 addr = 0;
>          struct acpi_ioremap *map;
>  
>          if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
>                  return;
>  
> +       /* Handle a case that GAS is used to address an ACPI data structure */
> +       if (!gas->bit_width) {
> +               pr_info("An ACPI data structure at 0x%llx is unmapped\n", addr);
> +               mutex_lock(&acpi_ioremap_lock);
> +               map = acpi_map_lookup(addr, PAGE_SIZE);
> +               if (!map) {
> +                       mutex_unlock(&acpi_ioremap_lock);
> +                       return;
> +               }
> +               acpi_os_drop_map_ref(map);
> +               mutex_unlock(&acpi_ioremap_lock);
> +       }
> +
>          /* Handle possible alignment issues */
>          memcpy(&addr, &gas->address, sizeof(addr));
> -       if (!addr || !gas->bit_width)
> +       if (!addr)
>                  return;
>  
>          mutex_lock(&acpi_ioremap_lock);
> -- 
> 2.25.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure
  2022-05-22 21:43 [PATCH v2] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure james.liu
       [not found] ` <PH7PR84MB1958F5A9F4F05EFE72A9E906E6A29@PH7PR84MB1958.NAMPRD84.PROD.OUTLOOK.COM>
@ 2022-06-29 14:27 ` Rafael J. Wysocki
  1 sibling, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2022-06-29 14:27 UTC (permalink / raw)
  To: James Liu
  Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List,
	Linux Kernel Mailing List, rwright, david.chang, clayc

On Sun, May 22, 2022 at 11:44 PM <james.liu@hpe.com> wrote:
>
> From: James Liu <james.liu@hpe.com>
>
>     Modify acpi_os_map_generic_address() & acpi_os_unmap_generic_address()
>     to correctly handle cases that a GAS table (i.e., Table 5.1, ACPI 6.4)
>     is used to address a data structure; in the case, the GAS has the field
>     of "Register Bit Width" equal to 0.
>
>     For example, "Injection Instruction Entry" (Table 18.25, ACPI 6.4) has
>     a RegisterRegion field that is a GAS that points to a data structure
>     SET_ERROR_TYPE_WITH_ADDRESS (Table 18.30), which is required when using
>     EINJ (Error Injection module).
>
>     This fix preserves a fairly sufficient memory space (i.e. page size) to

The hard-coded PAGE_SIZE allocation size is kind of a concern, because
the spec doesn't seem to limit the size of the structure pointed to by
the GAS to a page.

Also it is not clear from the spec that the RegisterRegion field in
Table 18.25 points to a structure.  On the contrary, Section 18.6.2
says that it points to a register.

In any case, I don't think it is OK to use
acpi_os_map_generic_address() to map an address from a GAS pointing to
a data structure.  acpi_os_map_memory() appears to be the right
interface to map such memory regions.

>     store the data structure to prevent EINJ module from loading failure if
>     platform firmware can correctly support Injection Instruction Entry in
>     an EINJ table.
>
> Signed-off-by: James Liu <james.liu@hpe.com>
> ---
>  drivers/acpi/osl.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 45c5c0e45..99f987c8c 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -452,14 +452,20 @@ EXPORT_SYMBOL_GPL(acpi_os_unmap_memory);
>
>  void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
>  {
> -       u64 addr;
> +       u64 addr = 0;
>
>         if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
>                 return NULL;
>
> +       /* Handle a case that GAS is used to address an ACPI data structure */
> +       if (!gas->bit_width) {
> +               pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
> +               return  acpi_os_map_iomem(addr, PAGE_SIZE);
> +       }
> +
>         /* Handle possible alignment issues */
>         memcpy(&addr, &gas->address, sizeof(addr));
> -       if (!addr || !gas->bit_width)
> +       if (!addr)
>                 return NULL;
>
>         return acpi_os_map_iomem(addr, gas->bit_width / 8);
> @@ -468,15 +474,28 @@ EXPORT_SYMBOL(acpi_os_map_generic_address);
>
>  void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
>  {
> -       u64 addr;
> +       u64 addr = 0;
>         struct acpi_ioremap *map;
>
>         if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
>                 return;
>
> +       /* Handle a case that GAS is used to address an ACPI data structure */
> +       if (!gas->bit_width) {
> +               pr_info("An ACPI data structure at 0x%llx is unmapped\n", addr);
> +               mutex_lock(&acpi_ioremap_lock);
> +               map = acpi_map_lookup(addr, PAGE_SIZE);
> +               if (!map) {
> +                       mutex_unlock(&acpi_ioremap_lock);
> +                       return;
> +               }
> +               acpi_os_drop_map_ref(map);
> +               mutex_unlock(&acpi_ioremap_lock);
> +       }
> +
>         /* Handle possible alignment issues */
>         memcpy(&addr, &gas->address, sizeof(addr));
> -       if (!addr || !gas->bit_width)
> +       if (!addr)
>                 return;
>
>         mutex_lock(&acpi_ioremap_lock);
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-06-29 14:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-22 21:43 [PATCH v2] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure james.liu
     [not found] ` <PH7PR84MB1958F5A9F4F05EFE72A9E906E6A29@PH7PR84MB1958.NAMPRD84.PROD.OUTLOOK.COM>
2022-06-06  9:16   ` James Liu
2022-06-29 14:27 ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.