[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3d4aa16f-ba12-4efe-bdfc-4e7ef059e09d@amazon.co.uk>
Date: Fri, 19 Apr 2024 14:31:02 +0100
From: "Landge, Sudan" <sudanl@...zon.co.uk>
To: "Jason A. Donenfeld" <Jason@...c4.com>, <tytso@....edu>,
<robh@...nel.org>, <krzk@...nel.org>, <conor+dt@...nel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<sudanl@...zon.com>, <graf@...zon.com>, <dwmw@...zon.co.uk>,
<krzysztof.kozlowski@...aro.org>, <bchalios@...zon.es>,
<xmarcalx@...zon.co.uk>
Subject: Re: [PATCH v7 1/3] virt: vmgenid: change implementation to use a platform
driver
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@...zon.com>
>
> Re-implement vmgenid as a platform driver in preparation for adding
> devicetree bindings support in next commits.
>
> Signed-off-by: Sudan Landge <sudanl@...zon.com>
> Reviewed-by: Alexander Graf <graf@...zon.com>
> [Jason: - Small style cleanups and refactoring.]
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.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@...c4.com>");
> --
> 2.44.0
>
Since I am on leave, looping in Babis to review/verify the patches.
Powered by blists - more mailing lists