Re: [PATCH v2] platform/x86/amd: Don't allow HSMP to be loaded on non-server hardware

[Mario Limonciello]

On 4/18/24 04:04, Hans de Goede wrote:
> Hi,
>
> On 4/16/24 8:20 PM, Mario Limonciello wrote:
> > From: Mario Limonciello <mario.limonciello@xxxxxxx>
> >
> > 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 <mario.limonciello@xxxxxxx>
> > ---
> > v1->v2:
> >   * use pm preferred profile instead
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>
> 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