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.
> + return guid_equal((guid_t *)obj->buffer.pointer, &acpi_hsmp_uuid);
> +
> + return false;
> +}
> static int hsmp_pltdrv_probe(struct platform_device *pdev)
> {
> + struct acpi_device *adev;
> + u16 sock_ind = 0;
> int ret;
>
> - plat_dev.sock = devm_kzalloc(&pdev->dev,
> - (plat_dev.num_sockets * sizeof(struct hsmp_socket)),
> - GFP_KERNEL);
> - if (!plat_dev.sock)
> - return -ENOMEM;
> -
> - ret = init_socket_objects(&pdev->dev);
> - if (ret)
> - return ret;
> -
> - plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
> - plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
> - plat_dev.hsmp_device.fops = &hsmp_fops;
> - plat_dev.hsmp_device.parent = &pdev->dev;
> - plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
> - plat_dev.hsmp_device.mode = 0644;
> + /*
> + * On ACPI supported BIOS, there is an ACPI HSMP device added for
> + * each socket, so the per socket probing, but the memory allocated for
> + * sockets should be contiguous to access it as an array,
> + * Hence allocate memory for all the sockets at once instead of allocating
> + * on each probe.
> + */
> + if (!plat_dev.is_probed) {
> + plat_dev.sock = devm_kzalloc(&pdev->dev,
> + (plat_dev.num_sockets * sizeof(struct hsmp_socket)),
> + GFP_KERNEL);
I know you're just moving code around here so no need to do it in this
patchset but this should use devm_kcalloc() and there are also extra
parenthesis.
--
i.
