| Message ID | 20230411180302.2678736-3-terry.bowman@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl/pci: Add support for RCH RAS error handling | expand |
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 >
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)
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) >
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) > >
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
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) >>>> >>
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 --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)
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(+)