diff mbox series

[v3,2/6] efi/cper: Export cper_mem_err_unpack() for use by modules

Message ID 20230411180302.2678736-3-terry.bowman@amd.com
State New
Headers show
Series None | expand

Commit Message

Terry Bowman April 11, 2023, 6:02 p.m. UTC
The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
logging. This is not currently possible if CXL is built as a loadable
module because cper_print_aer() depends on cper_mem_err_unpack() which
is not exported.

Export cper_mem_err_unpack() to enable cper_print_aer() usage in
CXL and other loadable modules.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-efi@vger.kernel.org
---
 drivers/firmware/efi/cper.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Ard Biesheuvel April 12, 2023, 11:04 a.m. UTC | #1
On Tue, 11 Apr 2023 at 20:03, Terry Bowman <terry.bowman@amd.com> wrote:
>
> The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
> logging. This is not currently possible if CXL is built as a loadable
> module because cper_print_aer() depends on cper_mem_err_unpack() which
> is not exported.
>
> Export cper_mem_err_unpack() to enable cper_print_aer() usage in
> CXL and other loadable modules.
>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: linux-efi@vger.kernel.org

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

> ---
>  drivers/firmware/efi/cper.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 35c37f667781..ff15e12160ae 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -350,6 +350,7 @@ const char *cper_mem_err_unpack(struct trace_seq *p,
>
>         return ret;
>  }
> +EXPORT_SYMBOL_GPL(cper_mem_err_unpack);
>
>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem,
>         int len)
> --
> 2.34.1
>
Jonathan Cameron April 13, 2023, 4:08 p.m. UTC | #2
On Tue, 11 Apr 2023 13:02:58 -0500
Terry Bowman <terry.bowman@amd.com> wrote:

> The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
> logging. This is not currently possible if CXL is built as a loadable
> module because cper_print_aer() depends on cper_mem_err_unpack() which
> is not exported.
> 
> Export cper_mem_err_unpack() to enable cper_print_aer() usage in
> CXL and other loadable modules.

No problem with the export, but I'm struggling to see the path that needs it.
Could you give a little more detail, perhaps a call path?

Thanks,

Jonathan

> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: linux-efi@vger.kernel.org
> ---
>  drivers/firmware/efi/cper.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 35c37f667781..ff15e12160ae 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -350,6 +350,7 @@ const char *cper_mem_err_unpack(struct trace_seq *p,
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(cper_mem_err_unpack);
>  
>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem,
>  	int len)
Terry Bowman April 13, 2023, 7:40 p.m. UTC | #3
Hi Jonathan,

On 4/13/23 11:08, Jonathan Cameron wrote:
> On Tue, 11 Apr 2023 13:02:58 -0500
> Terry Bowman <terry.bowman@amd.com> wrote:
> 
>> The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
>> logging. This is not currently possible if CXL is built as a loadable
>> module because cper_print_aer() depends on cper_mem_err_unpack() which
>> is not exported.
>>
>> Export cper_mem_err_unpack() to enable cper_print_aer() usage in
>> CXL and other loadable modules.
> 
> No problem with the export, but I'm struggling to see the path that needs it.
> Could you give a little more detail, perhaps a call path?
> 

The cper_print_aer() is used to log RCH dport AER errors. This is needed 
because the RCH dport AER errors are not handled directly by the AER port 
driver. I'll add these details to the patch.

Regards,
Terry

> Thanks,
> 
> Jonathan
> 
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Cc: Ard Biesheuvel <ardb@kernel.org>
>> Cc: linux-efi@vger.kernel.org
>> ---
>>  drivers/firmware/efi/cper.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
>> index 35c37f667781..ff15e12160ae 100644
>> --- a/drivers/firmware/efi/cper.c
>> +++ b/drivers/firmware/efi/cper.c
>> @@ -350,6 +350,7 @@ const char *cper_mem_err_unpack(struct trace_seq *p,
>>  
>>  	return ret;
>>  }
>> +EXPORT_SYMBOL_GPL(cper_mem_err_unpack);
>>  
>>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem,
>>  	int len)
>
Jonathan Cameron April 14, 2023, 11:48 a.m. UTC | #4
On Thu, 13 Apr 2023 14:40:10 -0500
Terry Bowman <Terry.Bowman@amd.com> wrote:

> Hi Jonathan,
> 
> On 4/13/23 11:08, Jonathan Cameron wrote:
> > On Tue, 11 Apr 2023 13:02:58 -0500
> > Terry Bowman <terry.bowman@amd.com> wrote:
> >   
> >> The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
> >> logging. This is not currently possible if CXL is built as a loadable
> >> module because cper_print_aer() depends on cper_mem_err_unpack() which
> >> is not exported.
> >>
> >> Export cper_mem_err_unpack() to enable cper_print_aer() usage in
> >> CXL and other loadable modules.  
> > 
> > No problem with the export, but I'm struggling to see the path that needs it.
> > Could you give a little more detail, perhaps a call path?
> >   
> 
> The cper_print_aer() is used to log RCH dport AER errors. This is needed 
> because the RCH dport AER errors are not handled directly by the AER port 
> driver. I'll add these details to the patch.

Ah. I wasn't particularly clear.  cper_print_aer() is fine, but oddly
I'm not seeing where that results in a call to cper_mem_err_unpack()

More than possible my grep skills are failing me!

Jonathan

> 
> Regards,
> Terry
> 
> > Thanks,
> > 
> > Jonathan
> >   
> >>
> >> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> >> Cc: Ard Biesheuvel <ardb@kernel.org>
> >> Cc: linux-efi@vger.kernel.org
> >> ---
> >>  drivers/firmware/efi/cper.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> >> index 35c37f667781..ff15e12160ae 100644
> >> --- a/drivers/firmware/efi/cper.c
> >> +++ b/drivers/firmware/efi/cper.c
> >> @@ -350,6 +350,7 @@ const char *cper_mem_err_unpack(struct trace_seq *p,
> >>  
> >>  	return ret;
> >>  }
> >> +EXPORT_SYMBOL_GPL(cper_mem_err_unpack);
> >>  
> >>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem,
> >>  	int len)  
> >
Robert Richter April 14, 2023, 12:44 p.m. UTC | #5
On 14.04.23 12:48:05, Jonathan Cameron wrote:
> On Thu, 13 Apr 2023 14:40:10 -0500
> Terry Bowman <Terry.Bowman@amd.com> wrote:
> 
> > Hi Jonathan,
> > 
> > On 4/13/23 11:08, Jonathan Cameron wrote:
> > > On Tue, 11 Apr 2023 13:02:58 -0500
> > > Terry Bowman <terry.bowman@amd.com> wrote:
> > >   
> > >> The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
> > >> logging. This is not currently possible if CXL is built as a loadable
> > >> module because cper_print_aer() depends on cper_mem_err_unpack() which
> > >> is not exported.
> > >>
> > >> Export cper_mem_err_unpack() to enable cper_print_aer() usage in
> > >> CXL and other loadable modules.  
> > > 
> > > No problem with the export, but I'm struggling to see the path that needs it.
> > > Could you give a little more detail, perhaps a call path?
> > >   
> > 
> > The cper_print_aer() is used to log RCH dport AER errors. This is needed 
> > because the RCH dport AER errors are not handled directly by the AER port 
> > driver. I'll add these details to the patch.
> 
> Ah. I wasn't particularly clear.  cper_print_aer() is fine, but oddly
> I'm not seeing where that results in a call to cper_mem_err_unpack()
> 
> More than possible my grep skills are failing me!

No worries, it is used in some odd tracepoint macro magic included
with ras_event.h.

-Robert
Terry Bowman April 14, 2023, 3:17 p.m. UTC | #6
On 4/14/23 08:24, Terry Bowman wrote:
> 
> 
> On 4/14/23 06:48, Jonathan Cameron wrote:
>> On Thu, 13 Apr 2023 14:40:10 -0500
>> Terry Bowman <Terry.Bowman@amd.com> wrote:
>>
>>> Hi Jonathan,
>>>
>>> On 4/13/23 11:08, Jonathan Cameron wrote:
>>>> On Tue, 11 Apr 2023 13:02:58 -0500
>>>> Terry Bowman <terry.bowman@amd.com> wrote:
>>>>   
>>>>> The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
>>>>> logging. This is not currently possible if CXL is built as a loadable
>>>>> module because cper_print_aer() depends on cper_mem_err_unpack() which
>>>>> is not exported.
>>>>>
>>>>> Export cper_mem_err_unpack() to enable cper_print_aer() usage in
>>>>> CXL and other loadable modules.  
>>>>
>>>> No problem with the export, but I'm struggling to see the path that needs it.
>>>> Could you give a little more detail, perhaps a call path?
>>>>   
>>>
>>> The cper_print_aer() is used to log RCH dport AER errors. This is needed 
>>> because the RCH dport AER errors are not handled directly by the AER port 
>>> driver. I'll add these details to the patch.
>>
>> Ah. I wasn't particularly clear.  cper_print_aer() is fine, but oddly
>> I'm not seeing where that results in a call to cper_mem_err_unpack()
>>
>> More than possible my grep skills are failing me!
>>
>> Jonathan
>>
> 
> I see. Without this patch, if include/ras/ras_event.h  cper_mem_err_unpack() 
> 
> We use 
> 
> 

Testing shows this patch is no longer needed. This patch was required for earlier
implementation calling the aer trace macros directly. I will remove this
patch in next patchset revision.

Regards,
Terry

>>>
>>> Regards,
>>> Terry
>>>
>>>> Thanks,
>>>>
>>>> Jonathan
>>>>   
>>>>>
>>>>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>>>>> Cc: Ard Biesheuvel <ardb@kernel.org>
>>>>> Cc: linux-efi@vger.kernel.org
>>>>> ---
>>>>>  drivers/firmware/efi/cper.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
>>>>> index 35c37f667781..ff15e12160ae 100644
>>>>> --- a/drivers/firmware/efi/cper.c
>>>>> +++ b/drivers/firmware/efi/cper.c
>>>>> @@ -350,6 +350,7 @@ const char *cper_mem_err_unpack(struct trace_seq *p,
>>>>>  
>>>>>  	return ret;
>>>>>  }
>>>>> +EXPORT_SYMBOL_GPL(cper_mem_err_unpack);
>>>>>  
>>>>>  static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem,
>>>>>  	int len)  
>>>>   
>>
Dan Williams April 17, 2023, 11:08 p.m. UTC | #7
Terry Bowman wrote:
> The CXL driver plans to use cper_print_aer() for restricted CXL host (RCH)
> logging. This is not currently possible if CXL is built as a loadable
> module because cper_print_aer() depends on cper_mem_err_unpack() which
> is not exported.
> 
> Export cper_mem_err_unpack() to enable cper_print_aer() usage in
> CXL and other loadable modules.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: linux-efi@vger.kernel.org
> ---
>  drivers/firmware/efi/cper.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index 35c37f667781..ff15e12160ae 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -350,6 +350,7 @@ const char *cper_mem_err_unpack(struct trace_seq *p,
>  
>  	return ret;
>  }
> +EXPORT_SYMBOL_GPL(cper_mem_err_unpack);

Looks ok to me. You could make it:

EXPORT_SYMBOL_NS_GPL(cper_mem_err_unpack, CXL)

...to make it clear that this is really only meant to be consumed by the
CXL subsystem. That was also the approach taken with the otherwise
internal-only insert_resource_expand_to_fit() symbol.
diff mbox series

Patch

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index 35c37f667781..ff15e12160ae 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -350,6 +350,7 @@  const char *cper_mem_err_unpack(struct trace_seq *p,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(cper_mem_err_unpack);
 
 static void cper_print_mem(const char *pfx, const struct cper_sec_mem_err *mem,
 	int len)