linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
       [not found] <e0e1cf62-6e49-1524-a370-41532c5b4ac7 () arm ! com>
@ 2020-11-10 12:00 ` yaoaili126
  2020-11-10 14:54   ` Borislav Petkov
  0 siblings, 1 reply; 11+ messages in thread
From: yaoaili126 @ 2020-11-10 12:00 UTC (permalink / raw)
  To: james.morse; +Cc: rjw, lenb, tony.luck, bp, linux-acpi, YANGFENG1, yaoaili

From: Aili Yao <yaoaili@kingsoft.com>

Hi!
Thank you for all the suggestions and comments!
I have re-submited the patch following your comments
I really need to read Documentation/process more detailly!

Thanks

Best Regards!

Aili Yao

> -----Original Message-----
> From: James Morse [mailto:james.morse@arm.com]
> Sent: Saturday, November 7, 2020 3:35 AM
> To: yaoaili126@163.com; rjw@rjwysocki.net; lenb@kernel.org
> Cc: tony.luck@intel.com; bp@alien8.de; linux-acpi@vger.kernel.org;
> YANGFENG1<YANGFENG1@kingsoft.com>; yaoaili<yaoaili@kingsoft.com>
> Subject: Re: [PATCH] ACPI, APEI, Fix incorrect return value of
> apei_map_generic_address
> 
> Hello,
> 
> On 02/11/2020 02:47, yaoaili126@163.com wrote:
> > From: Aili Yao <yaoaili@kingsoft.com>
> >
> > From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
> > acpi_os_map_generic_address() return
> > value"),acpi_os_map_generic_address
> > will return logical address or NULL for error, but
> > pre_map_gar_callback and related apei_map_generic_address ,for
> > ACPI_ADR_SPACE_SYSTEM_IO case, it should be also return 0,as it's a
> > normal case, but now it will return -ENXIO. so check it out for such
> > case to avoid einj module initialization fail.
> 
> (Nit: To make the commit message easier to read, please put '()' after
> function names, and spaces after commas.)
> 
> 
> > Tested-by: Tony Luck <tony.luck@intel.com>
> 
> > Signed-off-by: James Morse <james.morse@arm.com>
> 
> You can't add other peoples 'signed off'. This is for tracking the path a patch
> takes, and that each person who touches it 'signs off' their changes for the
> open-source license. See the 'Developer's Certificate of Origin 1.1' in
> Documentation/process/submitting-patches.rst'.
> 
> Please remove this tag.
> 
> 
> > Signed-off-by: Aili Yao <yaoaili@kingsoft.com>
> 
> As this fixes the bug where the einj module can't be loaded, I think its
> appropriate for the stable kernels. The tags to do that are:
> Fixes: 6915564dc5a8 ("ACPI: OSL: Change the type of
> acpi_os_map_generic_address() return
> value")
> Cc: <stable@vger.kernel.org>
> 
> 
> With that, please add my:
> Reviewed-by: James Morse <james.morse@arm.com>
> 
> 
> Thanks!
> 
> James
> 
> 
> 
> > diff --git a/drivers/acpi/apei/apei-base.c
> > b/drivers/acpi/apei/apei-base.c index 552fd9ffaca4..3294cc8dc073
> > 100644
> > --- a/drivers/acpi/apei/apei-base.c
> > +++ b/drivers/acpi/apei/apei-base.c
> > @@ -633,6 +633,10 @@ int apei_map_generic_address(struct
> acpi_generic_address *reg)
> >  	if (rc)
> >  		return rc;
> >
> > +	/* IO space doesn't need mapping */
> > +	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> > +		return 0;
> > +
> >  	if (!acpi_os_map_generic_address(reg))
> >  		return -ENXIO;
> >
> >



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

* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
  2020-11-10 12:00 ` [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address yaoaili126
@ 2020-11-10 14:54   ` Borislav Petkov
  2020-11-12  9:24     ` Aili Yao
  0 siblings, 1 reply; 11+ messages in thread
From: Borislav Petkov @ 2020-11-10 14:54 UTC (permalink / raw)
  To: yaoaili126
  Cc: james.morse, rjw, lenb, tony.luck, linux-acpi, YANGFENG1, yaoaili

On Tue, Nov 10, 2020 at 04:00:02AM -0800, yaoaili126@163.com wrote:
> From: Aili Yao <yaoaili@kingsoft.com>
> 
> Hi!
> Thank you for all the suggestions and comments!
> I have re-submited the patch following your comments
> I really need to read Documentation/process more detailly!

Yes, and while doing that please read about top-posting too and stop
doing it.

Thanks!

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
  2020-11-10 14:54   ` Borislav Petkov
@ 2020-11-12  9:24     ` Aili Yao
  2020-11-12 10:12       ` Aili Yao
  0 siblings, 1 reply; 11+ messages in thread
From: Aili Yao @ 2020-11-12  9:24 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: james.morse, rjw, lenb, tony.luck, linux-acpi, YANGFENG1, yaoaili126

Hi, Thanks for your reply! 
I am really sorry for the chaos from me! 


On Tue, 10 Nov 2020 15:54:50 +0100
Borislav Petkov <bp@alien8.de> wrote:

> On Tue, Nov 10, 2020 at 04:00:02AM -0800, yaoaili126@163.com wrote:
> > From: Aili Yao <yaoaili@kingsoft.com>
> > 
> > Hi!
> > Thank you for all the suggestions and comments!
> > I have re-submited the patch following your comments
> > I really need to read Documentation/process more detailly!  
> 
> Yes, and while doing that please read about top-posting too and stop
> doing it.
I am sorry I don't fully get it, please help point it out and I will avoid
doing it again.
1.The top-posting is the Documents in Documentation/ ?
2.I am sorry for the wrong doings, Can you be more clearly.
Then I will avoid doing it.

> 
> Thanks!
> 

Thanks
Aili Yao

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

* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
  2020-11-12  9:24     ` Aili Yao
@ 2020-11-12 10:12       ` Aili Yao
  2020-11-12 10:32         ` Borislav Petkov
  0 siblings, 1 reply; 11+ messages in thread
From: Aili Yao @ 2020-11-12 10:12 UTC (permalink / raw)
  To: Borislav Petkov, lenb; +Cc: james.morse, rjw, tony.luck, linux-acpi, YANGFENG1

Oh, Do you mean the document Posting.rst?
Is the wrong doing  BUILD error from robot?

I am really sorry for this!
Before submit anything, I will do more build test and real test.
I will try my best not doing it again. 

If not this, please help point out!.

Thank you!

On Thu, 12 Nov 2020 17:24:07 +0800
Aili Yao <yaoaili@kingsoft.com> wrote:

> Hi, Thanks for your reply! 
> I am really sorry for the chaos from me! 
> 
> 
> On Tue, 10 Nov 2020 15:54:50 +0100
> Borislav Petkov <bp@alien8.de> wrote:
> 
> > On Tue, Nov 10, 2020 at 04:00:02AM -0800, yaoaili126@163.com wrote:  
> > > From: Aili Yao <yaoaili@kingsoft.com>
> > > 
> > > Hi!
> > > Thank you for all the suggestions and comments!
> > > I have re-submited the patch following your comments
> > > I really need to read Documentation/process more detailly!    
> > 
> > Yes, and while doing that please read about top-posting too and stop
> > doing it.  
> I am sorry I don't fully get it, please help point it out and I will avoid
> doing it again.
> 1.The top-posting is the Documents in Documentation/ ?
> 2.I am sorry for the wrong doings, Can you be more clearly.
> Then I will avoid doing it.
> 
> > 
> > Thanks!
> >   
> 
> Thanks
> Aili Yao


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

* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
  2020-11-12 10:12       ` Aili Yao
@ 2020-11-12 10:32         ` Borislav Petkov
  2020-11-12 11:30           ` Aili Yao
  0 siblings, 1 reply; 11+ messages in thread
From: Borislav Petkov @ 2020-11-12 10:32 UTC (permalink / raw)
  To: Aili Yao; +Cc: lenb, james.morse, rjw, tony.luck, linux-acpi, YANGFENG1

On Thu, Nov 12, 2020 at 06:12:03PM +0800, Aili Yao wrote:
> If not this, please help point out!.

If there's something you're not clear on, searching the kernel tree
might help:

[ ~/kernel/linux> git grep top-post Documentation/
Documentation/process/2.Process.rst:444:- Avoid top-posting (the practice of putting your answer above the quoted

and now you open that document and read:

"- Avoid top-posting (the practice of putting your answer above the quoted
  text you are responding to).  It makes your response harder to read and
  makes a poor impression."

Here's another example:

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

Also, read this:

http://daringfireball.net/2007/07/on_top

Also, you can look at mail threads on lkml to see how most people reply
to mail - that also gives a good example what to do.

I hope it is clear now.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
  2020-11-12 10:32         ` Borislav Petkov
@ 2020-11-12 11:30           ` Aili Yao
  0 siblings, 0 replies; 11+ messages in thread
From: Aili Yao @ 2020-11-12 11:30 UTC (permalink / raw)
  To: Borislav Petkov; +Cc: lenb, james.morse, rjw, tony.luck, linux-acpi, YANGFENG1

On Thu, 12 Nov 2020 11:32:58 +0100
Borislav Petkov <bp@alien8.de> wrote:

> I hope it is clear now.

Yeah, It's clear!
I will avoid doing that.
Thanks

Aili Yao





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

* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
  2020-11-02  2:47 ` yaoaili126
  2020-11-03 17:11   ` Luck, Tony
@ 2020-11-06 19:34   ` James Morse
  1 sibling, 0 replies; 11+ messages in thread
From: James Morse @ 2020-11-06 19:34 UTC (permalink / raw)
  To: yaoaili126, rjw, lenb; +Cc: tony.luck, bp, linux-acpi, yangfeng1, yaoaili

Hello,

On 02/11/2020 02:47, yaoaili126@163.com wrote:
> From: Aili Yao <yaoaili@kingsoft.com>
> 
> From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
> acpi_os_map_generic_address() return value"),acpi_os_map_generic_address
> will return logical address or NULL for error, but
> pre_map_gar_callback and related apei_map_generic_address ,for
> ACPI_ADR_SPACE_SYSTEM_IO case, it should be also return 0,as it's a
> normal case, but now it will return -ENXIO. so check it out for such
> case to avoid einj module initialization fail.

(Nit: To make the commit message easier to read, please put '()' after function names, and
spaces after commas.)


> Tested-by: Tony Luck <tony.luck@intel.com>

> Signed-off-by: James Morse <james.morse@arm.com>

You can't add other peoples 'signed off'. This is for tracking the path a patch takes, and
that each person who touches it 'signs off' their changes for the open-source license. See
the 'Developer's Certificate of Origin 1.1' in Documentation/process/submitting-patches.rst'.

Please remove this tag.


> Signed-off-by: Aili Yao <yaoaili@kingsoft.com>

As this fixes the bug where the einj module can't be loaded, I think its appropriate for
the stable kernels. The tags to do that are:
Fixes: 6915564dc5a8 ("ACPI: OSL: Change the type of acpi_os_map_generic_address() return
value")
Cc: <stable@vger.kernel.org>


With that, please add my:
Reviewed-by: James Morse <james.morse@arm.com>


Thanks!

James



> diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
> index 552fd9ffaca4..3294cc8dc073 100644
> --- a/drivers/acpi/apei/apei-base.c
> +++ b/drivers/acpi/apei/apei-base.c
> @@ -633,6 +633,10 @@ int apei_map_generic_address(struct acpi_generic_address *reg)
>  	if (rc)
>  		return rc;
>  
> +	/* IO space doesn't need mapping */
> +	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> +		return 0;
> +
>  	if (!acpi_os_map_generic_address(reg))
>  		return -ENXIO;
>  
> 


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

* Re: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
  2020-11-02  2:47 ` yaoaili126
@ 2020-11-03 17:11   ` Luck, Tony
  2020-11-06 19:34   ` James Morse
  1 sibling, 0 replies; 11+ messages in thread
From: Luck, Tony @ 2020-11-03 17:11 UTC (permalink / raw)
  To: yaoaili126; +Cc: rjw, lenb, james.morse, bp, linux-acpi, yangfeng1, yaoaili

On Sun, Nov 01, 2020 at 06:47:26PM -0800, yaoaili126@163.com wrote:
> From: Aili Yao <yaoaili@kingsoft.com>
> 
> >From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
> acpi_os_map_generic_address() return value"),acpi_os_map_generic_address
> will return logical address or NULL for error, but
> pre_map_gar_callback and related apei_map_generic_address ,for
> ACPI_ADR_SPACE_SYSTEM_IO case, it should be also return 0,as it's a
> normal case, but now it will return -ENXIO. so check it out for such
> case to avoid einj module initialization fail.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Aili Yao <yaoaili@kingsoft.com>
> Tested-by: Tony Luck <tony.luck@intel.com>

Yes. Tested this version too. Works fine.

-Tony

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

* RE: [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
       [not found] <20201102023332.7638-1-yaoaili126 () 163 ! com>
@ 2020-11-02  3:00 ` yaoaili126
  0 siblings, 0 replies; 11+ messages in thread
From: yaoaili126 @ 2020-11-02  3:00 UTC (permalink / raw)
  To: rjw, lenb; +Cc: james.morse, tony.luck, bp, linux-acpi, yangfeng1, yaoaili

I am really sorry for this error mail, This mix other modification for other 
issue that have not been tested!
Please ingore this mail, and I will re-submit.
Sorry for This!

> -----Original Message-----
> From: yaoaili126@163.com [mailto:yaoaili126@163.com]
> Sent: Monday, November 2, 2020 10:34 AM
> To: rjw@rjwysocki.net; lenb@kernel.org
> Cc: james.morse@arm.com; tony.luck@intel.com; bp@alien8.de; linux-
> acpi@vger.kernel.org; YANGFENG1<YANGFENG1@kingsoft.com>;
> yaoaili<yaoaili@kingsoft.com>
> Subject: [PATCH] ACPI, APEI, Fix incorrect return value of
> apei_map_generic_address
> 
> From: Aili Yao <yaoaili@kingsoft.com>
> 
> From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
> acpi_os_map_generic_address() return
> value"),acpi_os_map_generic_address
> will return logical address or NULL for error, but pre_map_gar_callback and
> related apei_map_generic_address ,for ACPI_ADR_SPACE_SYSTEM_IO case,
> it should be also return 0,as it's a normal case, but now it will return -ENXIO.
> so check it out for such case to avoid einj module initialization fail.
> 
> Tested-by: Tony Luck <tony.luck@intel.com>
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Aili Yao <yaoaili@kingsoft.com>
> ---
>  arch/x86/kernel/cpu/mce/core.c |  2 +
>  drivers/acpi/apei/apei-base.c  |  4 ++
>  drivers/acpi/apei/ghes.c       | 67 ++++++++++++++++++++++++++++++++++
>  include/acpi/ghes.h            |  2 +
>  4 files changed, 75 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/mce/core.c
> b/arch/x86/kernel/cpu/mce/core.c index 4102b866e7c0..22efa708ef53
> 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -341,6 +341,8 @@ static void mce_panic(const char *msg, struct mce
> *final, char *exp)
>  		if (!apei_err)
>  			apei_err = apei_write_mce(final);
>  	}
> +	/* Print possible additional cper error info, get cper cleared */
> +	ghes_in_mce_cper_entry_check();
>  	if (cpu_missing)
>  		pr_emerg(HW_ERR "Some CPUs didn't answer in
> synchronization\n");
>  	if (exp)
> diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
> index 552fd9ffaca4..3294cc8dc073 100644
> --- a/drivers/acpi/apei/apei-base.c
> +++ b/drivers/acpi/apei/apei-base.c
> @@ -633,6 +633,10 @@ int apei_map_generic_address(struct
> acpi_generic_address *reg)
>  	if (rc)
>  		return rc;
> 
> +	/* IO space doesn't need mapping */
> +	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
> +		return 0;
> +
>  	if (!acpi_os_map_generic_address(reg))
>  		return -ENXIO;
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index
> fce7ade2aba9..6f5d89b54561 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -1147,9 +1147,76 @@ static void ghes_nmi_remove(struct ghes *ghes)
>  	 */
>  	synchronize_rcu();
>  }
> +
> +int ghes_in_mce_cper_entry_check(void)
> +{
> +	int rc = -ENOENT;
> +	struct ghes *ghes;
> +	struct list_head *rcu_list = &ghes_nmi;
> +	enum fixed_addresses fixmap_idx = FIX_APEI_GHES_NMI;
> +	struct acpi_hest_generic_status *estatus, tmp_header;
> +	struct ghes_estatus_node *estatus_node;
> +	u32 len, node_len;
> +	u64 buf_paddr;
> +	int sev;
> +
> +	/* if NMI handler already in process, let NMI do its job */
> +	if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
> +		return 0;
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(ghes, rcu_list, list) {
> +		rc = __ghes_peek_estatus(ghes, &tmp_header, &buf_paddr,
> fixmap_idx);
> +		if (rc) {
> +			ghes_clear_estatus(ghes, &tmp_header, buf_paddr,
> fixmap_idx);
> +			return rc;
> +		}
> +
> +		rc = __ghes_check_estatus(ghes, &tmp_header);
> +		if (rc) {
> +			ghes_clear_estatus(ghes, &tmp_header, buf_paddr,
> fixmap_idx);
> +			return rc;
> +		}
> +
> +		len = cper_estatus_len(&tmp_header);
> +		node_len = GHES_ESTATUS_NODE_LEN(len);
> +		estatus_node = (void *)gen_pool_alloc(ghes_estatus_pool,
> node_len);
> +		if (!estatus_node) {
> +			/*
> +			 * Going to panic, No need to keep the error.
> +			 */
> +			ghes_clear_estatus(ghes, &tmp_header, buf_paddr,
> fixmap_idx);
> +			return -ENOMEM;
> +		}
> +		estatus_node->ghes = ghes;
> +		estatus_node->generic = ghes->generic;
> +		estatus_node->task_work.func = NULL;
> +		estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
> +
> +		if (__ghes_read_estatus(estatus, buf_paddr, fixmap_idx, len))
> {
> +			ghes_clear_estatus(ghes, estatus, buf_paddr,
> fixmap_idx);
> +			return -ENOENT;
> +		}
> +
> +		/*
> +		 * As we are going to panic, and preemt the possible NMI
> handing,
> +		 * dump all the info and get it cleared.
> +		 */
> +		ghes_print_queued_estatus();
> +		__ghes_print_estatus(KERN_EMERG, ghes->generic, estatus);
> +		ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
> +
> +		gen_pool_free(ghes_estatus_pool, (unsigned
> long)estatus_node,
> +		      node_len);
> +	}
> +	rcu_read_unlock();
> +	atomic_dec(&ghes_in_nmi);
> +	return rc;
> +}
>  #else /* CONFIG_HAVE_ACPI_APEI_NMI */
>  static inline void ghes_nmi_add(struct ghes *ghes) { }  static inline void
> ghes_nmi_remove(struct ghes *ghes) { }
> +int ghes_in_mce_cper_entry_check(void) {}
>  #endif /* CONFIG_HAVE_ACPI_APEI_NMI */
> 
>  static void ghes_nmi_init_cxt(void)
> diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h index
> 34fb3431a8f3..be1ee0e993d2 100644
> --- a/include/acpi/ghes.h
> +++ b/include/acpi/ghes.h
> @@ -145,4 +145,6 @@ int ghes_notify_sea(void);  static inline int
> ghes_notify_sea(void) { return -ENOENT; }  #endif
> 
> +int ghes_in_mce_cper_entry_check(void);
> +
>  #endif /* GHES_H */
> --
> 2.18.4

Thanks
Aili Yao


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

* [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
       [not found] <28dc453f-40b8-8263-5aeb-f8979f54a941 () arm ! com>
  2020-11-02  2:33 ` yaoaili126
@ 2020-11-02  2:47 ` yaoaili126
  2020-11-03 17:11   ` Luck, Tony
  2020-11-06 19:34   ` James Morse
  1 sibling, 2 replies; 11+ messages in thread
From: yaoaili126 @ 2020-11-02  2:47 UTC (permalink / raw)
  To: rjw, lenb; +Cc: james.morse, tony.luck, bp, linux-acpi, yangfeng1, yaoaili

From: Aili Yao <yaoaili@kingsoft.com>

From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
acpi_os_map_generic_address() return value"),acpi_os_map_generic_address
will return logical address or NULL for error, but
pre_map_gar_callback and related apei_map_generic_address ,for
ACPI_ADR_SPACE_SYSTEM_IO case, it should be also return 0,as it's a
normal case, but now it will return -ENXIO. so check it out for such
case to avoid einj module initialization fail.

Tested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Aili Yao <yaoaili@kingsoft.com>
---
 drivers/acpi/apei/apei-base.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 552fd9ffaca4..3294cc8dc073 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -633,6 +633,10 @@ int apei_map_generic_address(struct acpi_generic_address *reg)
 	if (rc)
 		return rc;
 
+	/* IO space doesn't need mapping */
+	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+		return 0;
+
 	if (!acpi_os_map_generic_address(reg))
 		return -ENXIO;
 
-- 
2.18.4


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

* [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address
       [not found] <28dc453f-40b8-8263-5aeb-f8979f54a941 () arm ! com>
@ 2020-11-02  2:33 ` yaoaili126
  2020-11-02  2:47 ` yaoaili126
  1 sibling, 0 replies; 11+ messages in thread
From: yaoaili126 @ 2020-11-02  2:33 UTC (permalink / raw)
  To: rjw, lenb; +Cc: james.morse, tony.luck, bp, linux-acpi, yangfeng1, yaoaili

From: Aili Yao <yaoaili@kingsoft.com>

From commit 6915564dc5a8 ("ACPI: OSL: Change the type of
acpi_os_map_generic_address() return value"),acpi_os_map_generic_address
will return logical address or NULL for error, but
pre_map_gar_callback and related apei_map_generic_address ,for
ACPI_ADR_SPACE_SYSTEM_IO case, it should be also return 0,as it's a
normal case, but now it will return -ENXIO. so check it out for such
case to avoid einj module initialization fail.

Tested-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Aili Yao <yaoaili@kingsoft.com>
---
 arch/x86/kernel/cpu/mce/core.c |  2 +
 drivers/acpi/apei/apei-base.c  |  4 ++
 drivers/acpi/apei/ghes.c       | 67 ++++++++++++++++++++++++++++++++++
 include/acpi/ghes.h            |  2 +
 4 files changed, 75 insertions(+)

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 4102b866e7c0..22efa708ef53 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -341,6 +341,8 @@ static void mce_panic(const char *msg, struct mce *final, char *exp)
 		if (!apei_err)
 			apei_err = apei_write_mce(final);
 	}
+	/* Print possible additional cper error info, get cper cleared */
+	ghes_in_mce_cper_entry_check();
 	if (cpu_missing)
 		pr_emerg(HW_ERR "Some CPUs didn't answer in synchronization\n");
 	if (exp)
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 552fd9ffaca4..3294cc8dc073 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -633,6 +633,10 @@ int apei_map_generic_address(struct acpi_generic_address *reg)
 	if (rc)
 		return rc;
 
+	/* IO space doesn't need mapping */
+	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+		return 0;
+
 	if (!acpi_os_map_generic_address(reg))
 		return -ENXIO;
 
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index fce7ade2aba9..6f5d89b54561 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1147,9 +1147,76 @@ static void ghes_nmi_remove(struct ghes *ghes)
 	 */
 	synchronize_rcu();
 }
+
+int ghes_in_mce_cper_entry_check(void)
+{
+	int rc = -ENOENT;
+	struct ghes *ghes;
+	struct list_head *rcu_list = &ghes_nmi;
+	enum fixed_addresses fixmap_idx = FIX_APEI_GHES_NMI;
+	struct acpi_hest_generic_status *estatus, tmp_header;
+	struct ghes_estatus_node *estatus_node;
+	u32 len, node_len;
+	u64 buf_paddr;
+	int sev;
+
+	/* if NMI handler already in process, let NMI do its job */
+	if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(ghes, rcu_list, list) {
+		rc = __ghes_peek_estatus(ghes, &tmp_header, &buf_paddr, fixmap_idx);
+		if (rc) {
+			ghes_clear_estatus(ghes, &tmp_header, buf_paddr, fixmap_idx);
+			return rc;
+		}
+
+		rc = __ghes_check_estatus(ghes, &tmp_header);
+		if (rc) {
+			ghes_clear_estatus(ghes, &tmp_header, buf_paddr, fixmap_idx);
+			return rc;
+		}
+
+		len = cper_estatus_len(&tmp_header);
+		node_len = GHES_ESTATUS_NODE_LEN(len);
+		estatus_node = (void *)gen_pool_alloc(ghes_estatus_pool, node_len);
+		if (!estatus_node) {
+			/*
+			 * Going to panic, No need to keep the error.
+			 */
+			ghes_clear_estatus(ghes, &tmp_header, buf_paddr, fixmap_idx);
+			return -ENOMEM;
+		}
+		estatus_node->ghes = ghes;
+		estatus_node->generic = ghes->generic;
+		estatus_node->task_work.func = NULL;
+		estatus = GHES_ESTATUS_FROM_NODE(estatus_node);
+
+		if (__ghes_read_estatus(estatus, buf_paddr, fixmap_idx, len)) {
+			ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
+			return -ENOENT;
+		}
+
+		/*
+		 * As we are going to panic, and preemt the possible NMI handing,
+		 * dump all the info and get it cleared.
+		 */
+		ghes_print_queued_estatus();
+		__ghes_print_estatus(KERN_EMERG, ghes->generic, estatus);
+		ghes_clear_estatus(ghes, estatus, buf_paddr, fixmap_idx);
+
+		gen_pool_free(ghes_estatus_pool, (unsigned long)estatus_node,
+		      node_len);
+	}
+	rcu_read_unlock();
+	atomic_dec(&ghes_in_nmi);
+	return rc;
+}
 #else /* CONFIG_HAVE_ACPI_APEI_NMI */
 static inline void ghes_nmi_add(struct ghes *ghes) { }
 static inline void ghes_nmi_remove(struct ghes *ghes) { }
+int ghes_in_mce_cper_entry_check(void) {}
 #endif /* CONFIG_HAVE_ACPI_APEI_NMI */
 
 static void ghes_nmi_init_cxt(void)
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 34fb3431a8f3..be1ee0e993d2 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -145,4 +145,6 @@ int ghes_notify_sea(void);
 static inline int ghes_notify_sea(void) { return -ENOENT; }
 #endif
 
+int ghes_in_mce_cper_entry_check(void);
+
 #endif /* GHES_H */
-- 
2.18.4


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

end of thread, other threads:[~2020-11-12 11:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <e0e1cf62-6e49-1524-a370-41532c5b4ac7 () arm ! com>
2020-11-10 12:00 ` [PATCH] ACPI, APEI, Fix incorrect return value of apei_map_generic_address yaoaili126
2020-11-10 14:54   ` Borislav Petkov
2020-11-12  9:24     ` Aili Yao
2020-11-12 10:12       ` Aili Yao
2020-11-12 10:32         ` Borislav Petkov
2020-11-12 11:30           ` Aili Yao
     [not found] <20201102023332.7638-1-yaoaili126 () 163 ! com>
2020-11-02  3:00 ` yaoaili126
     [not found] <28dc453f-40b8-8263-5aeb-f8979f54a941 () arm ! com>
2020-11-02  2:33 ` yaoaili126
2020-11-02  2:47 ` yaoaili126
2020-11-03 17:11   ` Luck, Tony
2020-11-06 19:34   ` James Morse

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).