From: Mario Limonciello <[email protected]>
If the HSMP driver is compiled into the kernel or a module manually loaded
on client hardware it can cause problems with the functionality of the PMC
module since it probes a mailbox with a different definition on servers.
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
Signed-off-by: Mario Limonciello <[email protected]>
---
v1->v2:
* use pm preferred profile instead
---
drivers/platform/x86/amd/hsmp.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
index 1927be901108..102a49c3e945 100644
--- a/drivers/platform/x86/amd/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp.c
@@ -907,6 +907,17 @@ static int hsmp_plat_dev_register(void)
return ret;
}
+static bool hsmp_supported_profile(void)
+{
+ switch (acpi_gbl_FADT.preferred_profile) {
+ case PM_ENTERPRISE_SERVER:
+ case PM_SOHO_SERVER:
+ case PM_PERFORMANCE_SERVER:
+ return true;
+ }
+ return false;
+}
+
static int __init hsmp_plt_init(void)
{
int ret = -ENODEV;
@@ -917,6 +928,11 @@ static int __init hsmp_plt_init(void)
return ret;
}
+ if (!hsmp_supported_profile()) {
+ pr_err("HSMP is only supported on servers");
+ return ret;
+ }
+
/*
* amd_nb_num() returns number of SMN/DF interfaces present in the system
* if we have N SMN/DF interfaces that ideally means N sockets
--
2.43.0
Hi,
On 4/16/24 8:20 PM, Mario Limonciello wrote:
> From: Mario Limonciello <[email protected]>
>
> If the HSMP driver is compiled into the kernel or a module manually loaded
> on client hardware it can cause problems with the functionality of the PMC
> module since it probes a mailbox with a different definition on servers.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
> Signed-off-by: Mario Limonciello <[email protected]>
> ---
> v1->v2:
> * use pm preferred profile instead
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <[email protected]>
Mario, should this go in as a fix for the 6.9 cylce, or is
this for-next material ? (I'm not sure what to do myself)
Regards,
Hans
> ---
> drivers/platform/x86/amd/hsmp.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
> index 1927be901108..102a49c3e945 100644
> --- a/drivers/platform/x86/amd/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp.c
> @@ -907,6 +907,17 @@ static int hsmp_plat_dev_register(void)
> return ret;
> }
>
> +static bool hsmp_supported_profile(void)
> +{
> + switch (acpi_gbl_FADT.preferred_profile) {
> + case PM_ENTERPRISE_SERVER:
> + case PM_SOHO_SERVER:
> + case PM_PERFORMANCE_SERVER:
> + return true;
> + }
> + return false;
> +}
> +
> static int __init hsmp_plt_init(void)
> {
> int ret = -ENODEV;
> @@ -917,6 +928,11 @@ static int __init hsmp_plt_init(void)
> return ret;
> }
>
> + if (!hsmp_supported_profile()) {
> + pr_err("HSMP is only supported on servers");
> + return ret;
> + }
> +
> /*
> * amd_nb_num() returns number of SMN/DF interfaces present in the system
> * if we have N SMN/DF interfaces that ideally means N sockets
On 4/18/24 04:04, Hans de Goede wrote:
> Hi,
>
> On 4/16/24 8:20 PM, Mario Limonciello wrote:
>> From: Mario Limonciello <[email protected]>
>>
>> If the HSMP driver is compiled into the kernel or a module manually loaded
>> on client hardware it can cause problems with the functionality of the PMC
>> module since it probes a mailbox with a different definition on servers.
>>
>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
>> Signed-off-by: Mario Limonciello <[email protected]>
>> ---
>> v1->v2:
>> * use pm preferred profile instead
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <[email protected]>
>
> Mario, should this go in as a fix for the 6.9 cylce, or is
> this for-next material ? (I'm not sure what to do myself)
The main risk with this patch is if there are servers that previously
loaded amd-hsmp no longer working because of a BIOS bug to exporting the
incorrect profile. I think this is quite unlikely but not non-zero.
To at least give some time for anything like that to be raised I feel
this should go to for-next.
Ideally I do want to see it go to stable kernels after we're all
sufficiently happy though. Random bug reports to me like the ones I
added to the commit message get raised mostly by people who compile
their own (stable) kernels and enable all the AMD stuff because they
have AMD hardware.
So how about we target for-next, but also add a stable tag for when it
gets merged in the 6.10 cycle?
>
> Regards,
>
> Hans
>
>
>
>
>> ---
>> drivers/platform/x86/amd/hsmp.c | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
>> index 1927be901108..102a49c3e945 100644
>> --- a/drivers/platform/x86/amd/hsmp.c
>> +++ b/drivers/platform/x86/amd/hsmp.c
>> @@ -907,6 +907,17 @@ static int hsmp_plat_dev_register(void)
>> return ret;
>> }
>>
>> +static bool hsmp_supported_profile(void)
>> +{
>> + switch (acpi_gbl_FADT.preferred_profile) {
>> + case PM_ENTERPRISE_SERVER:
>> + case PM_SOHO_SERVER:
>> + case PM_PERFORMANCE_SERVER:
>> + return true;
>> + }
>> + return false;
>> +}
>> +
>> static int __init hsmp_plt_init(void)
>> {
>> int ret = -ENODEV;
>> @@ -917,6 +928,11 @@ static int __init hsmp_plt_init(void)
>> return ret;
>> }
>>
>> + if (!hsmp_supported_profile()) {
>> + pr_err("HSMP is only supported on servers");
>> + return ret;
>> + }
>> +
>> /*
>> * amd_nb_num() returns number of SMN/DF interfaces present in the system
>> * if we have N SMN/DF interfaces that ideally means N sockets
>
HI,
On 4/18/24 1:27 PM, Mario Limonciello wrote:
>
>
> On 4/18/24 04:04, Hans de Goede wrote:
>> Hi,
>>
>> On 4/16/24 8:20 PM, Mario Limonciello wrote:
>>> From: Mario Limonciello <[email protected]>
>>>
>>> If the HSMP driver is compiled into the kernel or a module manually loaded
>>> on client hardware it can cause problems with the functionality of the PMC
>>> module since it probes a mailbox with a different definition on servers.
>>>
>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
>>> Signed-off-by: Mario Limonciello <[email protected]>
>>> ---
>>> v1->v2:
>>> * use pm preferred profile instead
>>
>> Thanks, patch looks good to me:
>>
>> Reviewed-by: Hans de Goede <[email protected]>
>>
>> Mario, should this go in as a fix for the 6.9 cylce, or is
>> this for-next material ? (I'm not sure what to do myself)
> The main risk with this patch is if there are servers that previously loaded amd-hsmp no longer working because of a BIOS bug to exporting the incorrect profile. I think this is quite unlikely but not non-zero.
>
> To at least give some time for anything like that to be raised I feel this should go to for-next.
>
> Ideally I do want to see it go to stable kernels after we're all sufficiently happy though. Random bug reports to me like the ones I added to the commit message get raised mostly by people who compile their own (stable) kernels and enable all the AMD stuff because they have AMD hardware.
>
> So how about we target for-next, but also add a stable tag for when it gets merged in the 6.10 cycle?
Works for me. I'll merge this during my next round of patch merging and I'll
add a Cc: stable while merging.
Regards,
Hans
>>> ---
>>> drivers/platform/x86/amd/hsmp.c | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
>>> index 1927be901108..102a49c3e945 100644
>>> --- a/drivers/platform/x86/amd/hsmp.c
>>> +++ b/drivers/platform/x86/amd/hsmp.c
>>> @@ -907,6 +907,17 @@ static int hsmp_plat_dev_register(void)
>>> return ret;
>>> }
>>> +static bool hsmp_supported_profile(void)
>>> +{
>>> + switch (acpi_gbl_FADT.preferred_profile) {
>>> + case PM_ENTERPRISE_SERVER:
>>> + case PM_SOHO_SERVER:
>>> + case PM_PERFORMANCE_SERVER:
>>> + return true;
>>> + }
>>> + return false;
>>> +}
>>> +
>>> static int __init hsmp_plt_init(void)
>>> {
>>> int ret = -ENODEV;
>>> @@ -917,6 +928,11 @@ static int __init hsmp_plt_init(void)
>>> return ret;
>>> }
>>> + if (!hsmp_supported_profile()) {
>>> + pr_err("HSMP is only supported on servers");
>>> + return ret;
>>> + }
>>> +
>>> /*
>>> * amd_nb_num() returns number of SMN/DF interfaces present in the system
>>> * if we have N SMN/DF interfaces that ideally means N sockets
>>
>
On Thu, 18 Apr 2024, Mario Limonciello wrote:
> On 4/18/24 04:04, Hans de Goede wrote:
> > On 4/16/24 8:20 PM, Mario Limonciello wrote:
> > > From: Mario Limonciello <[email protected]>
> > >
> > > If the HSMP driver is compiled into the kernel or a module manually loaded
> > > on client hardware it can cause problems with the functionality of the PMC
> > > module since it probes a mailbox with a different definition on servers.
> > >
> > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
> > > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
> > > Signed-off-by: Mario Limonciello <[email protected]>
> > > ---
> > > v1->v2:
> > > * use pm preferred profile instead
> >
> > Thanks, patch looks good to me:
> >
> > Reviewed-by: Hans de Goede <[email protected]>
> >
> > Mario, should this go in as a fix for the 6.9 cylce, or is
> > this for-next material ? (I'm not sure what to do myself)
> The main risk with this patch is if there are servers that previously loaded
> amd-hsmp no longer working because of a BIOS bug to exporting the incorrect
> profile. I think this is quite unlikely but not non-zero.
>
> To at least give some time for anything like that to be raised I feel this
> should go to for-next.
I was also thinking it would be better to route this through for-next.
> Ideally I do want to see it go to stable kernels after we're all sufficiently
> happy though. Random bug reports to me like the ones I added to the commit
> message get raised mostly by people who compile their own (stable) kernels and
> enable all the AMD stuff because they have AMD hardware.
>
> So how about we target for-next, but also add a stable tag for when it gets
> merged in the 6.10 cycle?
That's possible but if you want to retain true control over it, don't add
stable tag at all now. You can send it on your own volition into stable
address later once the change is in Linus' tree and your "happy" condition
is met (Option 3 in Documentation/process/stable-kernel-rules.rst).
Otherwise, stable will autoselect it the moment it lands into Linus' tree
and you don't have much control over the timeline from that point on (I've
seen stable folks to grumble when somebody asked to delay including a
patch marked for stable, their reasoning was that their autotools keep
reselecting the patch over and over again).
--
i.
On 4/18/24 08:51, Ilpo Järvinen wrote:
> On Thu, 18 Apr 2024, Mario Limonciello wrote:
>> On 4/18/24 04:04, Hans de Goede wrote:
>>> On 4/16/24 8:20 PM, Mario Limonciello wrote:
>>>> From: Mario Limonciello <[email protected]>
>>>>
>>>> If the HSMP driver is compiled into the kernel or a module manually loaded
>>>> on client hardware it can cause problems with the functionality of the PMC
>>>> module since it probes a mailbox with a different definition on servers.
>>>>
>>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
>>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
>>>> Signed-off-by: Mario Limonciello <[email protected]>
>>>> ---
>>>> v1->v2:
>>>> * use pm preferred profile instead
>>>
>>> Thanks, patch looks good to me:
>>>
>>> Reviewed-by: Hans de Goede <[email protected]>
>>>
>>> Mario, should this go in as a fix for the 6.9 cylce, or is
>>> this for-next material ? (I'm not sure what to do myself)
>> The main risk with this patch is if there are servers that previously loaded
>> amd-hsmp no longer working because of a BIOS bug to exporting the incorrect
>> profile. I think this is quite unlikely but not non-zero.
>>
>> To at least give some time for anything like that to be raised I feel this
>> should go to for-next.
>
> I was also thinking it would be better to route this through for-next.
>
>> Ideally I do want to see it go to stable kernels after we're all sufficiently
>> happy though. Random bug reports to me like the ones I added to the commit
>> message get raised mostly by people who compile their own (stable) kernels and
>> enable all the AMD stuff because they have AMD hardware.
>>
>> So how about we target for-next, but also add a stable tag for when it gets
>> merged in the 6.10 cycle?
>
> That's possible but if you want to retain true control over it, don't add
> stable tag at all now. You can send it on your own volition into stable
> address later once the change is in Linus' tree and your "happy" condition
> is met (Option 3 in Documentation/process/stable-kernel-rules.rst).
>
> Otherwise, stable will autoselect it the moment it lands into Linus' tree
> and you don't have much control over the timeline from that point on (I've
> seen stable folks to grumble when somebody asked to delay including a
> patch marked for stable, their reasoning was that their autotools keep
> reselecting the patch over and over again).
>
I don't feel a strong need for a specific timing. The timeline of of it
going to the stable trees when 6.10-rc1~ish seems fine by me.
Hi,
On 4/16/24 8:20 PM, Mario Limonciello wrote:
> From: Mario Limonciello <[email protected]>
>
> If the HSMP driver is compiled into the kernel or a module manually loaded
> on client hardware it can cause problems with the functionality of the PMC
> module since it probes a mailbox with a different definition on servers.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
> Signed-off-by: Mario Limonciello <[email protected]>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> v1->v2:
> * use pm preferred profile instead
> ---
> drivers/platform/x86/amd/hsmp.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
> index 1927be901108..102a49c3e945 100644
> --- a/drivers/platform/x86/amd/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp.c
> @@ -907,6 +907,17 @@ static int hsmp_plat_dev_register(void)
> return ret;
> }
>
> +static bool hsmp_supported_profile(void)
> +{
> + switch (acpi_gbl_FADT.preferred_profile) {
> + case PM_ENTERPRISE_SERVER:
> + case PM_SOHO_SERVER:
> + case PM_PERFORMANCE_SERVER:
> + return true;
> + }
> + return false;
> +}
> +
> static int __init hsmp_plt_init(void)
> {
> int ret = -ENODEV;
> @@ -917,6 +928,11 @@ static int __init hsmp_plt_init(void)
> return ret;
> }
>
> + if (!hsmp_supported_profile()) {
> + pr_err("HSMP is only supported on servers");
> + return ret;
> + }
> +
> /*
> * amd_nb_num() returns number of SMN/DF interfaces present in the system
> * if we have N SMN/DF interfaces that ideally means N sockets