Hi Hans,
On 1/2/2024 6:11 PM, Hans de Goede wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
Hi Suma,
On 12/22/23 09:51, Ilpo Järvinen wrote:
On Thu, 21 Dec 2023, Suma Hegde wrote:
ACPI table provides mailbox base address and register offset
information. The base address is provided as part of CRS method
and mailbox offsets are provided through DSD table.
Sockets are differentiated by UIDs.
Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@xxxxxxx>
---
Changes since v1:
1. Define amd_hsmp_acpi_rdwr() for doing mailbox memory mapped io
2. Add a check to see if mailbox register offsets are set in
hsmp_read_acpi_dsd()
3. Add a check to see if sock->mbinfo.base_addr sockck->mbinfo.size are
set in hsmp_read_acpi_crs()
4. Change order of the statements in switch case ACPI_RESOURCE_TYPE_FIXED_MEMORY32
in hsmp_resource()
5. Add hsmp_test() after hsmp_parse_acpi_table() call
6. Add r.end < r.start check in hsmp_resource()
7. Add !dsd error check in hsmp_read_acpi_dsd
Changes since v2:
1. Change EINVAL to ENODEV in hsmp_read_acpi_dsd()
2. Change EINVAL to ENOENT in hsmp_read_acpi_dsd()
3. Use resource_size() in hsmp_resource()
drivers/platform/x86/amd/hsmp.c | 324 +++++++++++++++++++++++++++++---
1 file changed, 296 insertions(+), 28 deletions(-)
diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
index e77d4cd83a07..46924c572055 100644
--- a/drivers/platform/x86/amd/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp.c
@@ -18,9 +18,11 @@
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/semaphore.h>
+#include <linux/acpi.h>
#define DRIVER_NAME "amd_hsmp"
-#define DRIVER_VERSION "2.0"
+#define DRIVER_VERSION "2.2"
+#define ACPI_HSMP_DEVICE_HID "AMDI0097"
/* HSMP Status / Error codes */
#define HSMP_STATUS_NOT_READY 0x00
@@ -54,6 +56,11 @@
#define HSMP_ATTR_GRP_NAME_SIZE 10
+/* These are the strings specified in ACPI table */
+#define MSG_IDOFF_STR "MsgIdOffset"
+#define MSG_ARGOFF_STR "MsgArgOffset"
+#define MSG_RESPOFF_STR "MsgRspOffset"
+
struct hsmp_mbaddr_info {
u32 base_addr;
u32 msg_id_off;
@@ -66,6 +73,7 @@ struct hsmp_socket {
struct bin_attribute hsmp_attr;
struct hsmp_mbaddr_info mbinfo;
void __iomem *metric_tbl_addr;
+ void __iomem *virt_base_addr;
struct semaphore hsmp_sem;
char name[HSMP_ATTR_GRP_NAME_SIZE];
struct pci_dev *root;
@@ -78,12 +86,14 @@ struct hsmp_plat_device {
struct hsmp_socket *sock;
u32 proto_ver;
u16 num_sockets;
+ bool is_acpi_device;
+ bool is_probed;
};
static struct hsmp_plat_device plat_dev;
-static int amd_hsmp_rdwr(struct hsmp_socket *sock, u32 offset,
- u32 *value, bool write)
+static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write)
{
int ret;
@@ -101,8 +111,29 @@ static int amd_hsmp_rdwr(struct hsmp_socket *sock, u32 offset,
return ret;
}
+static void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write)
+{
+ if (write)
+ iowrite32(*value, sock->virt_base_addr + offset);
+ else
+ *value = ioread32(sock->virt_base_addr + offset);
+}
+
+static int amd_hsmp_rdwr(struct hsmp_socket *sock, u32 offset,
+ u32 *value, bool write)
+{
+ if (plat_dev.is_acpi_device)
+ amd_hsmp_acpi_rdwr(sock, offset, value, write);
+ else
+ return amd_hsmp_pci_rdwr(sock, offset, value, write);
+
+ return 0;
+}
+
/*
- * Send a message to the HSMP port via PCI-e config space registers.
+ * Send a message to the HSMP port via PCI-e config space registers
+ * or by writing to MMIO space.
*
* The caller is expected to zero out any unused arguments.
* If a response is expected, the number of response words should be greater than 0.
@@ -450,6 +481,9 @@ static int hsmp_create_sysfs_interface(void)
int ret;
u16 i;
+ if (plat_dev.is_acpi_device)
+ return 0;
This comes out of nowhere... Why proto_ver isn't enough to handle this?
If it's needed, would the check perhaps belong to
hsmp_is_sock_attr_visible() instead?
/* String formatting is currently limited to u8 sockets */
if (WARN_ON(plat_dev.num_sockets > U8_MAX))
return -ERANGE;
@@ -487,13 +521,188 @@ static int hsmp_create_sysfs_interface(void)
return devm_device_add_groups(plat_dev.sock[0].dev, hsmp_attr_grps);
}
-static int hsmp_cache_proto_ver(void)
+/* This is the UUID used for HSMP */
+static const guid_t acpi_hsmp_uuid = GUID_INIT(0xb74d619d, 0x5707, 0x48bd,
+ 0xa6, 0x9f, 0x4e, 0xa2,
+ 0x87, 0x1f, 0xc2, 0xf6);
+
+static inline bool is_acpi_hsmp_uuid(union acpi_object *obj)
+{
+ if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length == 16)
16 -> UUID_SIZE.
Please submit a v4 addressing Ilpo's review remarks on this patch.
And while at it please also fix the subject-prefix to be:
platform/x86/amd/hsmp:
As suggested by Thomas.
There have been a bunch of drivers/platform/x86/amd changes merged
recently, please base your v4 on top of:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
to avoid conflicts.
Sure, I will send v4 with addressing review comments and re basing on top of
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Thanks and Regards,
Suma
Regards,
Hans
