[PATCH v7 1/3] virt: vmgenid: change implementation to use a platform driver

Jason A. Donenfeld posted 3 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v7 1/3] virt: vmgenid: change implementation to use a platform driver
Posted by Jason A. Donenfeld 1 year, 9 months ago
From: Sudan Landge <sudanl@amazon.com>

Re-implement vmgenid as a platform driver in preparation for adding
devicetree bindings support in next commits.

Signed-off-by: Sudan Landge <sudanl@amazon.com>
Reviewed-by: Alexander Graf <graf@amazon.com>
[Jason: - Small style cleanups and refactoring.]
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/virt/vmgenid.c | 99 +++++++++++++++++++++++++++---------------
 1 file changed, 65 insertions(+), 34 deletions(-)

diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
index a1c467a0e9f7..aebbd24512c9 100644
--- a/drivers/virt/vmgenid.c
+++ b/drivers/virt/vmgenid.c
@@ -7,9 +7,10 @@
  * information to random.c.
  */
 
+#include <linux/acpi.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/acpi.h>
+#include <linux/platform_device.h>
 #include <linux/random.h>
 
 ACPI_MODULE_NAME("vmgenid");
@@ -21,19 +22,41 @@ struct vmgenid_state {
 	u8 this_id[VMGENID_SIZE];
 };
 
-static int vmgenid_add(struct acpi_device *device)
+static void vmgenid_notify(struct device *device)
+{
+	struct vmgenid_state *state = device->driver_data;
+	u8 old_id[VMGENID_SIZE];
+
+	memcpy(old_id, state->this_id, sizeof(old_id));
+	memcpy(state->this_id, state->next_id, sizeof(state->this_id));
+	if (!memcmp(old_id, state->this_id, sizeof(old_id)))
+		return;
+	add_vmfork_randomness(state->this_id, sizeof(state->this_id));
+}
+
+static void setup_vmgenid_state(struct vmgenid_state *state, void *virt_addr)
 {
+	state->next_id = virt_addr;
+	memcpy(state->this_id, state->next_id, sizeof(state->this_id));
+	add_device_randomness(state->this_id, sizeof(state->this_id));
+}
+
+static void vmgenid_acpi_handler(acpi_handle __always_unused handle,
+				 u32 __always_unused event, void *dev)
+{
+	vmgenid_notify(dev);
+}
+
+static int vmgenid_add_acpi(struct device *dev, struct vmgenid_state *state)
+{
+	struct acpi_device *device = ACPI_COMPANION(dev);
 	struct acpi_buffer parsed = { ACPI_ALLOCATE_BUFFER };
-	struct vmgenid_state *state;
 	union acpi_object *obj;
 	phys_addr_t phys_addr;
 	acpi_status status;
+	void *virt_addr;
 	int ret = 0;
 
-	state = devm_kmalloc(&device->dev, sizeof(*state), GFP_KERNEL);
-	if (!state)
-		return -ENOMEM;
-
 	status = acpi_evaluate_object(device->handle, "ADDR", NULL, &parsed);
 	if (ACPI_FAILURE(status)) {
 		ACPI_EXCEPTION((AE_INFO, status, "Evaluating ADDR"));
@@ -49,53 +72,61 @@ static int vmgenid_add(struct acpi_device *device)
 
 	phys_addr = (obj->package.elements[0].integer.value << 0) |
 		    (obj->package.elements[1].integer.value << 32);
-	state->next_id = devm_memremap(&device->dev, phys_addr, VMGENID_SIZE, MEMREMAP_WB);
-	if (IS_ERR(state->next_id)) {
-		ret = PTR_ERR(state->next_id);
+
+	virt_addr = devm_memremap(&device->dev, phys_addr, VMGENID_SIZE, MEMREMAP_WB);
+	if (IS_ERR(virt_addr)) {
+		ret = PTR_ERR(virt_addr);
 		goto out;
 	}
+	setup_vmgenid_state(state, virt_addr);
 
-	memcpy(state->this_id, state->next_id, sizeof(state->this_id));
-	add_device_randomness(state->this_id, sizeof(state->this_id));
-
-	device->driver_data = state;
+	status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
+					     vmgenid_acpi_handler, dev);
+	if (ACPI_FAILURE(status)) {
+		ret = -ENODEV;
+		goto out;
+	}
 
+	dev->driver_data = state;
 out:
 	ACPI_FREE(parsed.pointer);
 	return ret;
 }
 
-static void vmgenid_notify(struct acpi_device *device, u32 event)
+static int vmgenid_add(struct platform_device *pdev)
 {
-	struct vmgenid_state *state = acpi_driver_data(device);
-	u8 old_id[VMGENID_SIZE];
+	struct vmgenid_state *state;
+	struct device *dev = &pdev->dev;
+	int ret = 0;
 
-	memcpy(old_id, state->this_id, sizeof(old_id));
-	memcpy(state->this_id, state->next_id, sizeof(state->this_id));
-	if (!memcmp(old_id, state->this_id, sizeof(old_id)))
-		return;
-	add_vmfork_randomness(state->this_id, sizeof(state->this_id));
+	state = devm_kmalloc(dev, sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return -ENOMEM;
+
+	ret = vmgenid_add_acpi(dev, state);
+
+	if (ret)
+		devm_kfree(dev, state);
+	return ret;
 }
 
-static const struct acpi_device_id vmgenid_ids[] = {
+static const struct acpi_device_id vmgenid_acpi_ids[] = {
 	{ "VMGENCTR", 0 },
 	{ "VM_GEN_COUNTER", 0 },
 	{ }
 };
-
-static struct acpi_driver vmgenid_driver = {
-	.name = "vmgenid",
-	.ids = vmgenid_ids,
-	.owner = THIS_MODULE,
-	.ops = {
-		.add = vmgenid_add,
-		.notify = vmgenid_notify
-	}
+MODULE_DEVICE_TABLE(acpi, vmgenid_acpi_ids);
+
+static struct platform_driver vmgenid_plaform_driver = {
+	.probe      = vmgenid_add,
+	.driver     = {
+		.name   = "vmgenid",
+		.acpi_match_table = vmgenid_acpi_ids,
+	},
 };
 
-module_acpi_driver(vmgenid_driver);
+module_platform_driver(vmgenid_plaform_driver)
 
-MODULE_DEVICE_TABLE(acpi, vmgenid_ids);
 MODULE_DESCRIPTION("Virtual Machine Generation ID");
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
-- 
2.44.0
Re: [PATCH v7 1/3] virt: vmgenid: change implementation to use a platform driver
Posted by Landge, Sudan 1 year, 9 months ago

On 18/04/2024 13:12, Jason A. Donenfeld wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
> From: Sudan Landge <sudanl@amazon.com>
> 
> Re-implement vmgenid as a platform driver in preparation for adding
> devicetree bindings support in next commits.
> 
> Signed-off-by: Sudan Landge <sudanl@amazon.com>
> Reviewed-by: Alexander Graf <graf@amazon.com>
> [Jason: - Small style cleanups and refactoring.]
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>   drivers/virt/vmgenid.c | 99 +++++++++++++++++++++++++++---------------
>   1 file changed, 65 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/virt/vmgenid.c b/drivers/virt/vmgenid.c
> index a1c467a0e9f7..aebbd24512c9 100644
> --- a/drivers/virt/vmgenid.c
> +++ b/drivers/virt/vmgenid.c
> @@ -7,9 +7,10 @@
>    * information to random.c.
>    */
> 
> +#include <linux/acpi.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
> -#include <linux/acpi.h>
> +#include <linux/platform_device.h>
>   #include <linux/random.h>
> 
>   ACPI_MODULE_NAME("vmgenid");
> @@ -21,19 +22,41 @@ struct vmgenid_state {
>          u8 this_id[VMGENID_SIZE];
>   };
> 
> -static int vmgenid_add(struct acpi_device *device)
> +static void vmgenid_notify(struct device *device)
> +{
> +       struct vmgenid_state *state = device->driver_data;
> +       u8 old_id[VMGENID_SIZE];
> +
> +       memcpy(old_id, state->this_id, sizeof(old_id));
> +       memcpy(state->this_id, state->next_id, sizeof(state->this_id));
> +       if (!memcmp(old_id, state->this_id, sizeof(old_id)))
> +               return;
> +       add_vmfork_randomness(state->this_id, sizeof(state->this_id));
> +}
> +
> +static void setup_vmgenid_state(struct vmgenid_state *state, void *virt_addr)
>   {
> +       state->next_id = virt_addr;
> +       memcpy(state->this_id, state->next_id, sizeof(state->this_id));
> +       add_device_randomness(state->this_id, sizeof(state->this_id));
> +}
> +
> +static void vmgenid_acpi_handler(acpi_handle __always_unused handle,
> +                                u32 __always_unused event, void *dev)
> +{
> +       vmgenid_notify(dev);
> +}
> +
> +static int vmgenid_add_acpi(struct device *dev, struct vmgenid_state *state)
> +{
> +       struct acpi_device *device = ACPI_COMPANION(dev);
>          struct acpi_buffer parsed = { ACPI_ALLOCATE_BUFFER };
> -       struct vmgenid_state *state;
>          union acpi_object *obj;
>          phys_addr_t phys_addr;
>          acpi_status status;
> +       void *virt_addr;
>          int ret = 0;
> 
> -       state = devm_kmalloc(&device->dev, sizeof(*state), GFP_KERNEL);
> -       if (!state)
> -               return -ENOMEM;
> -
>          status = acpi_evaluate_object(device->handle, "ADDR", NULL, &parsed);
>          if (ACPI_FAILURE(status)) {
>                  ACPI_EXCEPTION((AE_INFO, status, "Evaluating ADDR"));
> @@ -49,53 +72,61 @@ static int vmgenid_add(struct acpi_device *device)
> 
>          phys_addr = (obj->package.elements[0].integer.value << 0) |
>                      (obj->package.elements[1].integer.value << 32);
> -       state->next_id = devm_memremap(&device->dev, phys_addr, VMGENID_SIZE, MEMREMAP_WB);
> -       if (IS_ERR(state->next_id)) {
> -               ret = PTR_ERR(state->next_id);
> +
> +       virt_addr = devm_memremap(&device->dev, phys_addr, VMGENID_SIZE, MEMREMAP_WB);
> +       if (IS_ERR(virt_addr)) {
> +               ret = PTR_ERR(virt_addr);
>                  goto out;
>          }
> +       setup_vmgenid_state(state, virt_addr);
> 
> -       memcpy(state->this_id, state->next_id, sizeof(state->this_id));
> -       add_device_randomness(state->this_id, sizeof(state->this_id));
> -
> -       device->driver_data = state;
> +       status = acpi_install_notify_handler(device->handle, ACPI_DEVICE_NOTIFY,
> +                                            vmgenid_acpi_handler, dev);
> +       if (ACPI_FAILURE(status)) {
> +               ret = -ENODEV;
> +               goto out;
> +       }
> 
> +       dev->driver_data = state;
>   out:
>          ACPI_FREE(parsed.pointer);
>          return ret;
>   }
> 
> -static void vmgenid_notify(struct acpi_device *device, u32 event)
> +static int vmgenid_add(struct platform_device *pdev)
>   {
> -       struct vmgenid_state *state = acpi_driver_data(device);
> -       u8 old_id[VMGENID_SIZE];
> +       struct vmgenid_state *state;
> +       struct device *dev = &pdev->dev;
> +       int ret = 0;
> 
> -       memcpy(old_id, state->this_id, sizeof(old_id));
> -       memcpy(state->this_id, state->next_id, sizeof(state->this_id));
> -       if (!memcmp(old_id, state->this_id, sizeof(old_id)))
> -               return;
> -       add_vmfork_randomness(state->this_id, sizeof(state->this_id));
> +       state = devm_kmalloc(dev, sizeof(*state), GFP_KERNEL);
> +       if (!state)
> +               return -ENOMEM;
> +
> +       ret = vmgenid_add_acpi(dev, state);
> +
> +       if (ret)
> +               devm_kfree(dev, state);
> +       return ret;
>   }
> 
> -static const struct acpi_device_id vmgenid_ids[] = {
> +static const struct acpi_device_id vmgenid_acpi_ids[] = {
>          { "VMGENCTR", 0 },
>          { "VM_GEN_COUNTER", 0 },
>          { }
>   };
> -
> -static struct acpi_driver vmgenid_driver = {
> -       .name = "vmgenid",
> -       .ids = vmgenid_ids,
> -       .owner = THIS_MODULE,
> -       .ops = {
> -               .add = vmgenid_add,
> -               .notify = vmgenid_notify
> -       }
> +MODULE_DEVICE_TABLE(acpi, vmgenid_acpi_ids);
> +
> +static struct platform_driver vmgenid_plaform_driver = {
> +       .probe      = vmgenid_add,
> +       .driver     = {
> +               .name   = "vmgenid",
> +               .acpi_match_table = vmgenid_acpi_ids,
> +       },
>   };
> 
> -module_acpi_driver(vmgenid_driver);
> +module_platform_driver(vmgenid_plaform_driver)
> 
> -MODULE_DEVICE_TABLE(acpi, vmgenid_ids);
>   MODULE_DESCRIPTION("Virtual Machine Generation ID");
>   MODULE_LICENSE("GPL v2");
>   MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
> --
> 2.44.0
> 
Since I am on leave, looping in Babis to review/verify the patches.