Re: [PATCH 1/3] platform/x86/amd/hsmp: Check HSMP support on AMD family of processors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/23/2024 04:14, Suma Hegde wrote:
HSMP interface is supported only on few x86 processors from AMD.
Accessing HSMP registers on rest of the platforms might cause
unexpected behaviour. So add a check.

Also unavailability of this interface on rest of the processors
is not an error. Hence, use pr_info() instead of the pr_err() to
log the message.

Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@xxxxxxx>
---
  drivers/platform/x86/amd/hsmp.c | 54 ++++++++++++++++++++++++++++-----
  1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
index 1927be901108..807a5066dacc 100644
--- a/drivers/platform/x86/amd/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp.c
@@ -907,16 +907,48 @@ static int hsmp_plat_dev_register(void)
  	return ret;
  }
+/*
+ * This check is only needed for backward compatibility of previous platforms.
+ * All new platforms are expected to support ACPI based probing.
+ */
+static bool is_hsmp_supported(void)

I think it's better to use "legacy" in this function name to make it clear that this isn't a requirement for newer platforms going forward. Maybe like

legacy_hsmp_support()?

+{
+	bool ret = true;

As this is a simple boolean return with no needing to clear memory, I think you can just "return true" or "return false" everywhere in this function.

+
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+		return false;
+
+	switch (boot_cpu_data.x86) {
+	case 0x19:
+		switch (boot_cpu_data.x86_model) {
+		case 0x00 ... 0x1F:
+		case 0x30 ... 0x3F:
+		case 0x90 ... 0x9F:
+		case 0xA0 ... 0xAF:
+			break;
+		default:
+			ret = false;
+		}
+		break;
+	case 0x1A:
+		switch (boot_cpu_data.x86_model) {
+		case 0x00 ... 0x1F:
+			break;

There actually are going to be 0x1A platforms that don't use ACPI?
I guess I'm surprised.

+		default:
+			ret = false;
+		}
+		break;
+	default:
+		ret = false;
+	}
+
+	return ret;
+}
+
  static int __init hsmp_plt_init(void)
  {
  	int ret = -ENODEV;
- if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD || boot_cpu_data.x86 < 0x19) {
-		pr_err("HSMP is not supported on Family:%x model:%x\n",
-		       boot_cpu_data.x86, boot_cpu_data.x86_model);
-		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
@@ -930,7 +962,15 @@ static int __init hsmp_plt_init(void)
  		return ret;
if (!plat_dev.is_acpi_device) {
-		ret = hsmp_plat_dev_register();
+		if (is_hsmp_supported()) {
+			/* Not ACPI device, but supports HSMP, register a plat_dev */
+			ret = hsmp_plat_dev_register();
+		} else {
+			/* Not ACPI, Does not support HSMP */
+			pr_info("HSMP is not supported on Family:%x model:%x\n",
+				boot_cpu_data.x86, boot_cpu_data.x86_model);
+			ret = -ENODEV;
+		}
  		if (ret)
  			platform_driver_unregister(&amd_hsmp_driver);
  	}





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux