2022-10-26 07:56:21

by Yupeng Li

[permalink] [raw]
Subject: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

When disabled CONFIG_SUSPEND and CONFIG_DEBUG_FS, get_metrics_table
and amd_pmc_idlemask_read is defined under two conditions of this,
pmc build with implicit declaration of function error.Some build error
messages are as follows:

CC [M] drivers/platform/x86/amd/pmc.o
drivers/platform/x86/amd/pmc.c: In function ‘smu_fw_info_show’:
drivers/platform/x86/amd/pmc.c:436:6: error: implicit declaration of function ‘get_metrics_table’ [-Werror=implicit-function-declaration]
436 | if (get_metrics_table(dev, &table))
| ^~~~~~~~~~~~~~~~~
drivers/platform/x86/amd/pmc.c: In function ‘amd_pmc_idlemask_show’:
drivers/platform/x86/amd/pmc.c:508:8: error: implicit declaration of function ‘amd_pmc_idlemask_read’; did you mean ‘amd_pmc_idlemask_show’? [-Werror=implicit-function-declaration]
508 | rc = amd_pmc_idlemask_read(dev, NULL, s);
| ^~~~~~~~~~~~~~~~~~~~~
| amd_pmc_idlemask_show
cc1: some warnings being treated as errors

Signed-off-by: Yupeng Li <[email protected]>
Reviewed-by: Caicai <[email protected]>
---
drivers/platform/x86/amd/pmc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index ce859b300712..2b08039921b8 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -433,8 +433,10 @@ static int smu_fw_info_show(struct seq_file *s, void *unused)
struct smu_metrics table;
int idx;

+#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
if (get_metrics_table(dev, &table))
return -EINVAL;
+#endif

seq_puts(s, "\n=== SMU Statistics ===\n");
seq_printf(s, "Table Version: %d\n", table.table_version);
@@ -503,11 +505,12 @@ static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
if (rc)
return rc;
}
-
if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
+#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
rc = amd_pmc_idlemask_read(dev, NULL, s);
if (rc)
return rc;
+#endif
} else {
seq_puts(s, "Unsupported SMU version for Idlemask\n");
}
--
2.34.1



2022-11-07 10:56:54

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

Hi,

On 10/26/22 09:25, Yupeng Li wrote:
> When disabled CONFIG_SUSPEND and CONFIG_DEBUG_FS, get_metrics_table
> and amd_pmc_idlemask_read is defined under two conditions of this,
> pmc build with implicit declaration of function error.Some build error
> messages are as follows:
>
> CC [M] drivers/platform/x86/amd/pmc.o
> drivers/platform/x86/amd/pmc.c: In function ‘smu_fw_info_show’:
> drivers/platform/x86/amd/pmc.c:436:6: error: implicit declaration of function ‘get_metrics_table’ [-Werror=implicit-function-declaration]
> 436 | if (get_metrics_table(dev, &table))
> | ^~~~~~~~~~~~~~~~~
> drivers/platform/x86/amd/pmc.c: In function ‘amd_pmc_idlemask_show’:
> drivers/platform/x86/amd/pmc.c:508:8: error: implicit declaration of function ‘amd_pmc_idlemask_read’; did you mean ‘amd_pmc_idlemask_show’? [-Werror=implicit-function-declaration]
> 508 | rc = amd_pmc_idlemask_read(dev, NULL, s);
> | ^~~~~~~~~~~~~~~~~~~~~
> | amd_pmc_idlemask_show
> cc1: some warnings being treated as errors
>
> Signed-off-by: Yupeng Li <[email protected]>
> Reviewed-by: Caicai <[email protected]>

Thank you for your patch, however I do not believe that this is the correct fix:

> ---
> drivers/platform/x86/amd/pmc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
> index ce859b300712..2b08039921b8 100644
> --- a/drivers/platform/x86/amd/pmc.c
> +++ b/drivers/platform/x86/amd/pmc.c
> @@ -433,8 +433,10 @@ static int smu_fw_info_show(struct seq_file *s, void *unused)
> struct smu_metrics table;
> int idx;
>
> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> if (get_metrics_table(dev, &table))
> return -EINVAL;
> +#endif
>
> seq_puts(s, "\n=== SMU Statistics ===\n");
> seq_printf(s, "Table Version: %d\n", table.table_version);

The table variable gets used here, but now it no longer has been initialized by
get_metrics_table() so just #ifdef-ing out the get_metrics_table() call is wrong.

> @@ -503,11 +505,12 @@ static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
> if (rc)
> return rc;
> }
> -
> if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> rc = amd_pmc_idlemask_read(dev, NULL, s);
> if (rc)
> return rc;
> +#endif
> } else {
> seq_puts(s, "Unsupported SMU version for Idlemask\n");
> }

Same thing except now nothing gets send to the struct seq_file *s,
so this is wrong too.

Shyam / Mario, can you take a look at the reported compile errors please?

Regards,

Hans


2022-11-07 13:54:35

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

On 11/7/22 04:28, Hans de Goede wrote:
> Hi,
>
> On 10/26/22 09:25, Yupeng Li wrote:
>> When disabled CONFIG_SUSPEND and CONFIG_DEBUG_FS, get_metrics_table
>> and amd_pmc_idlemask_read is defined under two conditions of this,
>> pmc build with implicit declaration of function error.Some build error
>> messages are as follows:
>>
>> CC [M] drivers/platform/x86/amd/pmc.o
>> drivers/platform/x86/amd/pmc.c: In function ‘smu_fw_info_show’:
>> drivers/platform/x86/amd/pmc.c:436:6: error: implicit declaration of function ‘get_metrics_table’ [-Werror=implicit-function-declaration]
>> 436 | if (get_metrics_table(dev, &table))
>> | ^~~~~~~~~~~~~~~~~
>> drivers/platform/x86/amd/pmc.c: In function ‘amd_pmc_idlemask_show’:
>> drivers/platform/x86/amd/pmc.c:508:8: error: implicit declaration of function ‘amd_pmc_idlemask_read’; did you mean ‘amd_pmc_idlemask_show’? [-Werror=implicit-function-declaration]
>> 508 | rc = amd_pmc_idlemask_read(dev, NULL, s);
>> | ^~~~~~~~~~~~~~~~~~~~~
>> | amd_pmc_idlemask_show
>> cc1: some warnings being treated as errors
>>
>> Signed-off-by: Yupeng Li <[email protected]>
>> Reviewed-by: Caicai <[email protected]>
>
> Thank you for your patch, however I do not believe that this is the correct fix:
>
>> ---
>> drivers/platform/x86/amd/pmc.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
>> index ce859b300712..2b08039921b8 100644
>> --- a/drivers/platform/x86/amd/pmc.c
>> +++ b/drivers/platform/x86/amd/pmc.c
>> @@ -433,8 +433,10 @@ static int smu_fw_info_show(struct seq_file *s, void *unused)
>> struct smu_metrics table;
>> int idx;
>>
>> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
>> if (get_metrics_table(dev, &table))
>> return -EINVAL;
>> +#endif
>>
>> seq_puts(s, "\n=== SMU Statistics ===\n");
>> seq_printf(s, "Table Version: %d\n", table.table_version);
>
> The table variable gets used here, but now it no longer has been initialized by
> get_metrics_table() so just #ifdef-ing out the get_metrics_table() call is wrong.
>
>> @@ -503,11 +505,12 @@ static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
>> if (rc)
>> return rc;
>> }
>> -
>> if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
>> +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
>> rc = amd_pmc_idlemask_read(dev, NULL, s);
>> if (rc)
>> return rc;
>> +#endif
>> } else {
>> seq_puts(s, "Unsupported SMU version for Idlemask\n");
>> }
>
> Same thing except now nothing gets send to the struct seq_file *s,
> so this is wrong too.
>
> Shyam / Mario, can you take a look at the reported compile errors please?
>
> Regards,
>
> Hans
>

What kernel is this? Shyam removed CONFIG_DEBUGFS in b37fe34c83. So is
this failure still happening in 6.1-rc4?


2022-11-11 15:59:39

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

On Mon, Nov 07, 2022 at 06:27:52AM -0600, Mario Limonciello wrote:
> On 11/7/22 04:28, Hans de Goede wrote:
> > Hi,
> >
> > On 10/26/22 09:25, Yupeng Li wrote:
> > > When disabled CONFIG_SUSPEND and CONFIG_DEBUG_FS, get_metrics_table
> > > and amd_pmc_idlemask_read is defined under two conditions of this,
> > > pmc build with implicit declaration of function error.Some build error
> > > messages are as follows:
> > >
> > > CC [M] drivers/platform/x86/amd/pmc.o
> > > drivers/platform/x86/amd/pmc.c: In function ‘smu_fw_info_show’:
> > > drivers/platform/x86/amd/pmc.c:436:6: error: implicit declaration of function ‘get_metrics_table’ [-Werror=implicit-function-declaration]
> > > 436 | if (get_metrics_table(dev, &table))
> > > | ^~~~~~~~~~~~~~~~~
> > > drivers/platform/x86/amd/pmc.c: In function ‘amd_pmc_idlemask_show’:
> > > drivers/platform/x86/amd/pmc.c:508:8: error: implicit declaration of function ‘amd_pmc_idlemask_read’; did you mean ‘amd_pmc_idlemask_show’? [-Werror=implicit-function-declaration]
> > > 508 | rc = amd_pmc_idlemask_read(dev, NULL, s);
> > > | ^~~~~~~~~~~~~~~~~~~~~
> > > | amd_pmc_idlemask_show
> > > cc1: some warnings being treated as errors
> > >
> > > Signed-off-by: Yupeng Li <[email protected]>
> > > Reviewed-by: Caicai <[email protected]>
> >
> > Thank you for your patch, however I do not believe that this is the correct fix:
> >
> > > ---
> > > drivers/platform/x86/amd/pmc.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
> > > index ce859b300712..2b08039921b8 100644
> > > --- a/drivers/platform/x86/amd/pmc.c
> > > +++ b/drivers/platform/x86/amd/pmc.c
> > > @@ -433,8 +433,10 @@ static int smu_fw_info_show(struct seq_file *s, void *unused)
> > > struct smu_metrics table;
> > > int idx;
> > > +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> > > if (get_metrics_table(dev, &table))
> > > return -EINVAL;
> > > +#endif
> > > seq_puts(s, "\n=== SMU Statistics ===\n");
> > > seq_printf(s, "Table Version: %d\n", table.table_version);
> >
> > The table variable gets used here, but now it no longer has been initialized by
> > get_metrics_table() so just #ifdef-ing out the get_metrics_table() call is wrong.
> >
> > > @@ -503,11 +505,12 @@ static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
> > > if (rc)
> > > return rc;
> > > }
> > > -
> > > if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
> > > +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> > > rc = amd_pmc_idlemask_read(dev, NULL, s);
> > > if (rc)
> > > return rc;
> > > +#endif
> > > } else {
> > > seq_puts(s, "Unsupported SMU version for Idlemask\n");
> > > }
> >
> > Same thing except now nothing gets send to the struct seq_file *s,
> > so this is wrong too.
> >
> > Shyam / Mario, can you take a look at the reported compile errors please?
> >
> > Regards,
> >
> > Hans
> >
>
> What kernel is this? Shyam removed CONFIG_DEBUGFS in b37fe34c83. So is
> this failure still happening in 6.1-rc4?

It managed to trickle into stable too:

https://bugzilla.kernel.org/show_bug.cgi?id=216679

Adding bug reporter to Cc too.

From a quick glance, I'd drop all that ifdeffery and add empty stubs.
IMNSVHO.

Thx.

--
Regards/Gruss,
Boris.

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

2022-11-11 16:14:26

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

On Fri, Nov 11, 2022 at 03:16:10PM +0000, Limonciello, Mario wrote:
> I sent up a patch for it, but Hans hasn't applied it.
> https://patchwork.kernel.org/project/platform-driver-x86/patch/[email protected]/

Yap, that fixes it.

So yeah, Hans, make sure to Cc: stable too when applying.

Thx.

--
Regards/Gruss,
Boris.

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

2022-11-11 16:32:42

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

[Public]



> -----Original Message-----
> From: Borislav Petkov <[email protected]>
> Sent: Friday, November 11, 2022 09:15
> To: Limonciello, Mario <[email protected]>
> Cc: Hans de Goede <[email protected]>; Yupeng Li
> <[email protected]>; S-k, Shyam-sundar <Shyam-sundar.S-
> [email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; smf-
> [email protected]
> Subject: Re: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency
> errors.
>
> On Mon, Nov 07, 2022 at 06:27:52AM -0600, Mario Limonciello wrote:
> > On 11/7/22 04:28, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 10/26/22 09:25, Yupeng Li wrote:
> > > > When disabled CONFIG_SUSPEND and CONFIG_DEBUG_FS,
> get_metrics_table
> > > > and amd_pmc_idlemask_read is defined under two conditions of this,
> > > > pmc build with implicit declaration of function error.Some build error
> > > > messages are as follows:
> > > >
> > > > CC [M] drivers/platform/x86/amd/pmc.o
> > > > drivers/platform/x86/amd/pmc.c: In function 'smu_fw_info_show':
> > > > drivers/platform/x86/amd/pmc.c:436:6: error: implicit declaration of
> function 'get_metrics_table' [-Werror=implicit-function-declaration]
> > > > 436 | if (get_metrics_table(dev, &table))
> > > > | ^~~~~~~~~~~~~~~~~
> > > > drivers/platform/x86/amd/pmc.c: In function
> 'amd_pmc_idlemask_show':
> > > > drivers/platform/x86/amd/pmc.c:508:8: error: implicit declaration of
> function 'amd_pmc_idlemask_read'; did you mean
> 'amd_pmc_idlemask_show'? [-Werror=implicit-function-declaration]
> > > > 508 | rc = amd_pmc_idlemask_read(dev, NULL, s);
> > > > | ^~~~~~~~~~~~~~~~~~~~~
> > > > | amd_pmc_idlemask_show
> > > > cc1: some warnings being treated as errors
> > > >
> > > > Signed-off-by: Yupeng Li <[email protected]>
> > > > Reviewed-by: Caicai <[email protected]>
> > >
> > > Thank you for your patch, however I do not believe that this is the correct
> fix:
> > >
> > > > ---
> > > > drivers/platform/x86/amd/pmc.c | 5 ++++-
> > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/platform/x86/amd/pmc.c
> b/drivers/platform/x86/amd/pmc.c
> > > > index ce859b300712..2b08039921b8 100644
> > > > --- a/drivers/platform/x86/amd/pmc.c
> > > > +++ b/drivers/platform/x86/amd/pmc.c
> > > > @@ -433,8 +433,10 @@ static int smu_fw_info_show(struct seq_file
> *s, void *unused)
> > > > struct smu_metrics table;
> > > > int idx;
> > > > +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> > > > if (get_metrics_table(dev, &table))
> > > > return -EINVAL;
> > > > +#endif
> > > > seq_puts(s, "\n=== SMU Statistics ===\n");
> > > > seq_printf(s, "Table Version: %d\n", table.table_version);
> > >
> > > The table variable gets used here, but now it no longer has been
> initialized by
> > > get_metrics_table() so just #ifdef-ing out the get_metrics_table() call is
> wrong.
> > >
> > > > @@ -503,11 +505,12 @@ static int amd_pmc_idlemask_show(struct
> seq_file *s, void *unused)
> > > > if (rc)
> > > > return rc;
> > > > }
> > > > -
> > > > if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
> > > > +#if defined(CONFIG_SUSPEND) || defined(CONFIG_DEBUG_FS)
> > > > rc = amd_pmc_idlemask_read(dev, NULL, s);
> > > > if (rc)
> > > > return rc;
> > > > +#endif
> > > > } else {
> > > > seq_puts(s, "Unsupported SMU version for Idlemask\n");
> > > > }
> > >
> > > Same thing except now nothing gets send to the struct seq_file *s,
> > > so this is wrong too.
> > >
> > > Shyam / Mario, can you take a look at the reported compile errors
> please?
> > >
> > > Regards,
> > >
> > > Hans
> > >
> >
> > What kernel is this? Shyam removed CONFIG_DEBUGFS in b37fe34c83. So
> is
> > this failure still happening in 6.1-rc4?
>
> It managed to trickle into stable too:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D216679&amp;data=05%7C01%7Cm
> ario.limonciello%40amd.com%7C29820718c1334539d74e08dac3f782f2%7C3dd
> 8961fe4884e608e11a82d994e183d%7C0%7C0%7C638037765076195581%7CUn
> known%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6
> Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=NcjgeDsIS%2
> BcW84%2FMcl9hKFNfL5Cw%2F2rYU8yXXsh2H5E%3D&amp;reserved=0
>
> Adding bug reporter to Cc too.
>
> From a quick glance, I'd drop all that ifdeffery and add empty stubs.
> IMNSVHO.
>

I sent up a patch for it, but Hans hasn't applied it.
https://patchwork.kernel.org/project/platform-driver-x86/patch/[email protected]/

2022-11-15 11:58:58

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH 1/1] platform/x86/amd: Fix pmc compile dependency errors.

Hi,

On 11/11/22 16:39, Borislav Petkov wrote:
> On Fri, Nov 11, 2022 at 03:16:10PM +0000, Limonciello, Mario wrote:
>> I sent up a patch for it, but Hans hasn't applied it.
>> https://patchwork.kernel.org/project/platform-driver-x86/patch/[email protected]/
>
> Yap, that fixes it.
>
> So yeah, Hans, make sure to Cc: stable too when applying.

Done. I'll send this out in my next fixes pull-req to Linus
before the end of this week.

Regards,

Hans