[PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

[PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

[james.liu]

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

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

    For example, "Injection Instruction Entry" (Table 18.25 in 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 so as to prevent EINJ module from loading
    failure if platform firmware can support Injection Instruction Entry in
    an EINJ table.

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

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 45c5c0e45..ab2f584b1 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
 	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);
@@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
 	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

Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

[kernel test robot]

Hi,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on v5.17 next-20220401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/james-liu-hpe-com/ACPI-OSL-Fix-the-memory-mapping-of-an-ACPI-GAS-that-addresses-a-data-structure/20220402-013755
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-a012 (https://download.01.org/0day-ci/archive/20220402/202204021406.lxRmdIm5-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c4a1b07d0979e7ff20d7d541af666d822d66b566)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/594496ff7d62c6d42b3c8a436fca46cc289aea67
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review james-liu-hpe-com/ACPI-OSL-Fix-the-memory-mapping-of-an-ACPI-GAS-that-addresses-a-data-structure/20220402-013755
        git checkout 594496ff7d62c6d42b3c8a436fca46cc289aea67
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/acpi/osl.c:463:29: warning: variable 'addr' is uninitialized when used here [-Wuninitialized]
                   return  acpi_os_map_iomem(addr, PAGE_SIZE);
                                             ^~~~
   drivers/acpi/osl.c:455:10: note: initialize the variable 'addr' to silence this warning
           u64 addr;
                   ^
                    = 0
   drivers/acpi/osl.c:487:25: warning: variable 'addr' is uninitialized when used here [-Wuninitialized]
                   map = acpi_map_lookup(addr, PAGE_SIZE);
                                         ^~~~
   drivers/acpi/osl.c:477:10: note: initialize the variable 'addr' to silence this warning
           u64 addr;
                   ^
                    = 0
   2 warnings generated.


vim +/addr +463 drivers/acpi/osl.c

   452	
   453	void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
   454	{
   455		u64 addr;
   456	
   457		if (gas->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
   458			return NULL;
   459	
   460		/* Handle a case that GAS is used to address an ACPI data structure */
   461		if (!gas->bit_width) {
   462			pr_info("An ACPI data structure at 0x%llx is mapped\n", addr);
 > 463			return  acpi_os_map_iomem(addr, PAGE_SIZE);
   464		}
   465	
   466		/* Handle possible alignment issues */
   467		memcpy(&addr, &gas->address, sizeof(addr));
   468		if (!addr)
   469			return NULL;
   470	
   471		return acpi_os_map_iomem(addr, gas->bit_width / 8);
   472	}
   473	EXPORT_SYMBOL(acpi_os_map_generic_address);
   474	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

[James Liu]

Hi Rafael and all,
Could you take a look at this patch? The mentioned bug blocks EINJ testing
when a firmware can correctly support ACPI 6.4 spec. 

On Fri, Apr 01, 2022 at 05:28:40PM +0000, james.liu@hpe.com wrote:
> From: James Liu <james.liu@hpe.com>
> 
>     Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
>     to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
>     to address a data structure; in this case, the GAS has the field of
>     "Register Bit Width" equal to 0.
> 
>     For example, "Injection Instruction Entry" (Table 18.25 in 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 so as to prevent EINJ module from loading
>     failure if platform firmware can support Injection Instruction Entry in
>     an EINJ table.
> 
> Signed-off-by: James Liu <james.liu@hpe.com>
> ---
>  drivers/acpi/osl.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 45c5c0e45..ab2f584b1 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
>  	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);
> @@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
>  	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
> 

Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

[James Liu]

Hi Rafael and all,
Could you take a look into this patches? The mentioned bug blocks EINJ
testing whenever a system firmware can correctly support GAS according 
to ACPI 6.4.

On Fri, Apr 01, 2022 at 05:28:40PM +0000, james.liu@hpe.com wrote:
> From: James Liu <james.liu@hpe.com>
> 
>     Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
>     to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
>     to address a data structure; in this case, the GAS has the field of
>     "Register Bit Width" equal to 0.
> 
>     For example, "Injection Instruction Entry" (Table 18.25 in 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 so as to prevent EINJ module from loading
>     failure if platform firmware can support Injection Instruction Entry in
>     an EINJ table.
> 
> Signed-off-by: James Liu <james.liu@hpe.com>
> ---
>  drivers/acpi/osl.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 45c5c0e45..ab2f584b1 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
>  	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);
> @@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
>  	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
> 

Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

[Rafael J. Wysocki]

On Tue, May 17, 2022 at 7:56 PM James Liu <james.liu@hpe.com> wrote:
>
> Hi Rafael and all,
> Could you take a look into this patches? The mentioned bug blocks EINJ
> testing whenever a system firmware can correctly support GAS according
> to ACPI 6.4.

The kernel test robot reported an issue with it.  Have you seen that report?

> On Fri, Apr 01, 2022 at 05:28:40PM +0000, james.liu@hpe.com wrote:
> > From: James Liu <james.liu@hpe.com>
> >
> >     Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
> >     to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
> >     to address a data structure; in this case, the GAS has the field of
> >     "Register Bit Width" equal to 0.
> >
> >     For example, "Injection Instruction Entry" (Table 18.25 in 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 so as to prevent EINJ module from loading
> >     failure if platform firmware can support Injection Instruction Entry in
> >     an EINJ table.
> >
> > Signed-off-by: James Liu <james.liu@hpe.com>
> > ---
> >  drivers/acpi/osl.c | 23 +++++++++++++++++++++--
> >  1 file changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > index 45c5c0e45..ab2f584b1 100644
> > --- a/drivers/acpi/osl.c
> > +++ b/drivers/acpi/osl.c
> > @@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
> >       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);
> > @@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
> >       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
> >

Re: [PATCH] ACPI: OSL: Fix the memory mapping of an ACPI GAS that addresses a data structure

[James Liu]

O Tue, May 17, 2022 at 08:25:55PM +0200, Rafael J. Wysocki wrote:
> On Tue, May 17, 2022 at 7:56 PM James Liu <james.liu@hpe.com> wrote:
> >
> > Hi Rafael and all,
> > Could you take a look into this patches? The mentioned bug blocks EINJ
> > testing whenever a system firmware can correctly support GAS according
> > to ACPI 6.4.
> 
> The kernel test robot reported an issue with it.  Have you seen that report?
>
Yes, I saw that the issue is warnings about the initialization of "u64 addr".
Will you suggest fixing it in another patch or just merge the change
(i.e., u64 addr = 0) into this one? Thanks for reviewing this.

> > On Fri, Apr 01, 2022 at 05:28:40PM +0000, james.liu@hpe.com wrote:
> > > From: James Liu <james.liu@hpe.com>
> > >
> > >     Modify acpi_os_map_generic_address and acpi_os_unmap_generic_address
> > >     to handle a case that a GAS table (i.e., Table 5.1 in ACPI 6.4) is used
> > >     to address a data structure; in this case, the GAS has the field of
> > >     "Register Bit Width" equal to 0.
> > >
> > >     For example, "Injection Instruction Entry" (Table 18.25 in 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 so as to prevent EINJ module from loading
> > >     failure if platform firmware can support Injection Instruction Entry in
> > >     an EINJ table.
> > >
> > > Signed-off-by: James Liu <james.liu@hpe.com>
> > > ---
> > >  drivers/acpi/osl.c | 23 +++++++++++++++++++++--
> > >  1 file changed, 21 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> > > index 45c5c0e45..ab2f584b1 100644
> > > --- a/drivers/acpi/osl.c
> > > +++ b/drivers/acpi/osl.c
> > > @@ -457,9 +457,15 @@ void __iomem *acpi_os_map_generic_address(struct acpi_generic_address *gas)
> > >       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);
> > > @@ -474,9 +480,22 @@ void acpi_os_unmap_generic_address(struct acpi_generic_address *gas)
> > >       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
> > >