Re: [PATCH v11 3/4] drivers/acpi: Introduce Platform Firmware Runtime Update Telemetry

[Rafael J. Wysocki]

On Sun, Nov 21, 2021 at 4:17 PM Chen Yu <yu.c.chen@xxxxxxxxx> wrote:
>

First of all, the subject of the patch should be "Introduce Platform
Firmware Runtime Telemetry", because there is no "update" part in it.

> Platform Firmware Runtime Update(PFRU) Telemetry Service is part of RoT
> (Root of Trust), which allows PFRU handler and other PFRU drivers to
> produce telemetry data to upper layer OS consumer at runtime.

The above paragraph is redundant IMO.  It provides a little useful
information IMV.

> The Linux kernel provides interfaces for the user to query the parameters
> of telemetry data, and the user could read out the telemetry data
> accordingly.

In fact, this driver allows user space to fetch telemetry data from
the firmware with the help of the Platform Firmware Runtime Telemetry
interface.

> PFRU and PFRU Telemetry both invoke _DSM to trigger the low level actions.

"PFRU and PFRT " or "PFRU and PFR Telemetry ", please.

> However the input parameters and ACPI package result as well as the
> functions are different from each other. It is hard to extract the common
> code between them, so introduce separated files for them.

I would write the above in the following way:

"Both PFRU and PFRT are based on ACPI _DSM interfaces located under
special device objects in the ACPI Namespace, but these interfaces are
different from each other, so it is better to provide a separate
driver from each of them.  However, they share some common definitions
and naming conventions."

> Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> ---

[cut the history]

> ---
>  drivers/acpi/pfru/Makefile         |   2 +-
>  drivers/acpi/pfru/pfru_telemetry.c | 457 +++++++++++++++++++++++++++++
>  include/uapi/linux/pfru.h          |  88 ++++++
>  3 files changed, 546 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/acpi/pfru/pfru_telemetry.c
>
> diff --git a/drivers/acpi/pfru/Makefile b/drivers/acpi/pfru/Makefile
> index 098cbe80cf3d..9ba11efe10b3 100644
> --- a/drivers/acpi/pfru/Makefile
> +++ b/drivers/acpi/pfru/Makefile

Actually, I'm not sure if a separate directory for this is needed.

The files (pfrut.h, pfr_update.c and pfr_telemetry.c) can be located
directly in drivers/acpi/ as far as I'm concerned.

> @@ -1,2 +1,2 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_ACPI_PFRU) += pfru_update.o
> +obj-$(CONFIG_ACPI_PFRU) += pfru_update.o pfru_telemetry.o

Please also update Kconfig with the information regarding PFRT (which
is not going to be added by the previous patch I suppose).

> diff --git a/drivers/acpi/pfru/pfru_telemetry.c b/drivers/acpi/pfru/pfru_telemetry.c
> new file mode 100644
> index 000000000000..5140a4591c9e
> --- /dev/null
> +++ b/drivers/acpi/pfru/pfru_telemetry.c
> @@ -0,0 +1,457 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ACPI Platform Firmware Runtime Update Telemetry Driver

"ACPI Platform Runtime Telemetry driver"

> + *
> + * Copyright (C) 2021 Intel Corporation
> + * Author: Chen Yu <yu.c.chen@xxxxxxxxx>

Please describe the driver here at least briefly (what it is for etc.).

> + */
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/mm.h>
> +#include <linux/platform_device.h>
> +#include <linux/string.h>
> +#include <linux/uaccess.h>
> +#include <linux/uio.h>
> +#include <linux/uuid.h>
> +
> +#include <uapi/linux/pfru.h>
> +
> +#define PFRU_LOG_EXEC_IDX      0
> +#define PFRU_LOG_HISTORY_IDX   1
> +
> +#define PFRU_LOG_ERR           0
> +#define PFRU_LOG_WARN  1
> +#define PFRU_LOG_INFO  2
> +#define PFRU_LOG_VERB  4
> +
> +#define PFRU_FUNC_SET_LEV              1
> +#define PFRU_FUNC_GET_LEV              2
> +#define PFRU_FUNC_GET_DATA             3
> +
> +#define PFRU_REVID_1           1
> +#define PFRU_REVID_2           2
> +#define PFRU_DEFAULT_REV_ID    PFRU_REVID_1

I would use the PFRT_ prefix in the above symbols.

> +
> +enum log_index {
> +       LOG_STATUS_IDX = 0,
> +       LOG_EXT_STATUS_IDX = 1,
> +       LOG_MAX_SZ_IDX = 2,
> +       LOG_CHUNK1_LO_IDX = 3,
> +       LOG_CHUNK1_HI_IDX = 4,
> +       LOG_CHUNK1_SZ_IDX = 5,
> +       LOG_CHUNK2_LO_IDX = 6,
> +       LOG_CHUNK2_HI_IDX = 7,
> +       LOG_CHUNK2_SZ_IDX = 8,
> +       LOG_ROLLOVER_CNT_IDX = 9,
> +       LOG_RESET_CNT_IDX = 10,
> +       LOG_NR_IDX
> +};
> +
> +struct pfru_log_device {

'pfrt_log_device"

> +       int index;
> +       struct pfru_log_info info;
> +       struct device *parent_dev;
> +       struct miscdevice miscdev;
> +};
> +
> +static const guid_t pfru_log_guid =
> +       GUID_INIT(0x75191659, 0x8178, 0x4D9D, 0xB8, 0x8F, 0xAC, 0x5E,
> +                 0x5E, 0x93, 0xE8, 0xBF);

The UUID needs to be documented.

> +
> +static DEFINE_IDA(pfru_log_ida);

pfrt_log_ida

Generally, I would use the pfrt_ prefix in the names below instead of pfru_.

> +
> +static inline struct pfru_log_device *to_pfru_log_dev(struct file *file)
> +{
> +       return container_of(file->private_data, struct pfru_log_device, miscdev);
> +}
> +
> +static int get_pfru_log_data_info(struct pfru_log_data_info *data_info,
> +                                 struct pfru_log_device *pfru_log_dev)
> +{
> +       acpi_handle handle = ACPI_HANDLE(pfru_log_dev->parent_dev);
> +       union acpi_object *out_obj, in_obj, in_buf;
> +       int ret = -EINVAL;
> +
> +       memset(&in_obj, 0, sizeof(in_obj));
> +       memset(&in_buf, 0, sizeof(in_buf));
> +       in_obj.type = ACPI_TYPE_PACKAGE;
> +       in_obj.package.count = 1;
> +       in_obj.package.elements = &in_buf;
> +       in_buf.type = ACPI_TYPE_INTEGER;
> +       in_buf.integer.value = pfru_log_dev->info.log_type;
> +
> +       out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_guid,
> +                                         pfru_log_dev->info.log_revid, PFRU_FUNC_GET_DATA,
> +                                         &in_obj, ACPI_TYPE_PACKAGE);
> +       if (!out_obj)
> +               return ret;
> +
> +       if (out_obj->package.count < LOG_NR_IDX)
> +               goto free_acpi_buffer;
> +
> +       if (out_obj->package.elements[LOG_STATUS_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->status = out_obj->package.elements[LOG_STATUS_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_EXT_STATUS_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->ext_status =
> +               out_obj->package.elements[LOG_EXT_STATUS_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_MAX_SZ_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->max_data_size =
> +               out_obj->package.elements[LOG_MAX_SZ_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_CHUNK1_LO_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->chunk1_addr_lo =
> +               out_obj->package.elements[LOG_CHUNK1_LO_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_CHUNK1_HI_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->chunk1_addr_hi =
> +               out_obj->package.elements[LOG_CHUNK1_HI_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_CHUNK1_SZ_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->chunk1_size =
> +               out_obj->package.elements[LOG_CHUNK1_SZ_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_CHUNK2_LO_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->chunk2_addr_lo =
> +               out_obj->package.elements[LOG_CHUNK2_LO_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_CHUNK2_HI_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->chunk2_addr_hi =
> +               out_obj->package.elements[LOG_CHUNK2_HI_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_CHUNK2_SZ_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->chunk2_size =
> +               out_obj->package.elements[LOG_CHUNK2_SZ_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_ROLLOVER_CNT_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->rollover_cnt =
> +               out_obj->package.elements[LOG_ROLLOVER_CNT_IDX].integer.value;
> +
> +       if (out_obj->package.elements[LOG_RESET_CNT_IDX].type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       data_info->reset_cnt =
> +               out_obj->package.elements[LOG_RESET_CNT_IDX].integer.value;

Like in some functions in the other patch, it would be good to reduce
the number of goto statements and possibly redundant updates by
grouping the checks.

> +
> +       ret = 0;
> +
> +free_acpi_buffer:
> +       kfree(out_obj);
> +
> +       return ret;
> +}
> +
> +static int set_pfru_log_level(int level, struct pfru_log_device *pfru_log_dev)
> +{
> +       acpi_handle handle = ACPI_HANDLE(pfru_log_dev->parent_dev);
> +       union acpi_object *out_obj, *obj, in_obj, in_buf;
> +       enum pfru_dsm_status status;
> +       int ret = -EINVAL;
> +
> +       memset(&in_obj, 0, sizeof(in_obj));
> +       memset(&in_buf, 0, sizeof(in_buf));
> +       in_obj.type = ACPI_TYPE_PACKAGE;
> +       in_obj.package.count = 1;
> +       in_obj.package.elements = &in_buf;
> +       in_buf.type = ACPI_TYPE_INTEGER;
> +       in_buf.integer.value = level;
> +
> +       out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_guid,
> +                                         pfru_log_dev->info.log_revid, PFRU_FUNC_SET_LEV,
> +                                         &in_obj, ACPI_TYPE_PACKAGE);
> +       if (!out_obj)
> +               return -EINVAL;
> +
> +       obj = &out_obj->package.elements[0];
> +       status = obj->integer.value;
> +       if (status)
> +               goto free_acpi_buffer;
> +
> +       obj = &out_obj->package.elements[1];
> +       status = obj->integer.value;
> +       if (status)
> +               goto free_acpi_buffer;
> +
> +       ret = 0;
> +
> +free_acpi_buffer:
> +       kfree(out_obj);
> +
> +       return ret;
> +}
> +
> +static int get_pfru_log_level(struct pfru_log_device *pfru_log_dev)
> +{
> +       acpi_handle handle = ACPI_HANDLE(pfru_log_dev->parent_dev);
> +       union acpi_object *out_obj, *obj;
> +       enum pfru_dsm_status status;
> +       int ret = -EINVAL;
> +
> +       out_obj = acpi_evaluate_dsm_typed(handle, &pfru_log_guid,
> +                                         pfru_log_dev->info.log_revid, PFRU_FUNC_GET_LEV,
> +                                         NULL, ACPI_TYPE_PACKAGE);
> +       if (!out_obj)
> +               return -EINVAL;
> +
> +       obj = &out_obj->package.elements[0];
> +       if (obj->type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       status = obj->integer.value;
> +       if (status)
> +               goto free_acpi_buffer;
> +
> +       obj = &out_obj->package.elements[1];
> +       if (obj->type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       status = obj->integer.value;
> +       if (status)
> +               goto free_acpi_buffer;
> +
> +       obj = &out_obj->package.elements[2];
> +       if (obj->type != ACPI_TYPE_INTEGER)
> +               goto free_acpi_buffer;
> +
> +       ret = obj->integer.value;
> +
> +free_acpi_buffer:
> +       kfree(out_obj);
> +
> +       return ret;
> +}
> +
> +static int valid_log_level(u32 level)
> +{
> +       return level == PFRU_LOG_ERR || level == PFRU_LOG_WARN ||
> +              level == PFRU_LOG_INFO || level == PFRU_LOG_VERB;
> +}
> +
> +static int valid_log_type(u32 type)
> +{
> +       return type == PFRU_LOG_EXEC_IDX || type == PFRU_LOG_HISTORY_IDX;
> +}
> +
> +static inline int valid_log_revid(u32 id)
> +{
> +       return id == PFRU_REVID_1 || id == PFRU_REVID_2;
> +}
> +
> +static long pfru_log_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +       struct pfru_log_device *pfru_log_dev = to_pfru_log_dev(file);
> +       struct pfru_log_data_info data_info;
> +       struct pfru_log_info info;
> +       void __user *p;
> +       int ret = 0;
> +
> +       p = (void __user *)arg;
> +
> +       switch (cmd) {
> +       case PFRU_LOG_IOC_SET_INFO:
> +               if (copy_from_user(&info, p, sizeof(info)))
> +                       return -EFAULT;
> +
> +               if (valid_log_revid(info.log_revid))
> +                       pfru_log_dev->info.log_revid = info.log_revid;
> +
> +               if (valid_log_level(info.log_level)) {
> +                       ret = set_pfru_log_level(info.log_level, pfru_log_dev);
> +                       if (ret < 0)
> +                               return ret;
> +
> +                       pfru_log_dev->info.log_level = info.log_level;
> +               }
> +
> +               if (valid_log_type(info.log_type))
> +                       pfru_log_dev->info.log_type = info.log_type;
> +
> +               return 0;

Please use blank lines to separate the cases visually from each other.

> +       case PFRU_LOG_IOC_GET_INFO:
> +               info.log_level = get_pfru_log_level(pfru_log_dev);
> +               if (ret < 0)
> +                       return ret;
> +
> +               info.log_type = pfru_log_dev->info.log_type;
> +               info.log_revid = pfru_log_dev->info.log_revid;
> +               if (copy_to_user(p, &info, sizeof(info)))
> +                       return -EFAULT;
> +
> +               return 0;
> +       case PFRU_LOG_IOC_GET_DATA_INFO:
> +               ret = get_pfru_log_data_info(&data_info, pfru_log_dev);
> +               if (ret)
> +                       return ret;
> +
> +               if (copy_to_user(p, &data_info, sizeof(struct pfru_log_data_info)))
> +                       return -EFAULT;
> +
> +               return 0;
> +       default:
> +               return -ENOTTY;
> +       }
> +}
> +
> +static int
> +pfru_log_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +       struct pfru_log_device *pfru_log_dev;
> +       struct pfru_log_data_info info;
> +       unsigned long psize, vsize;
> +       phys_addr_t base_addr;
> +       int ret;
> +
> +       if (vma->vm_flags & VM_WRITE)
> +               return -EROFS;
> +
> +       /* changing from read to write with mprotect is not allowed */
> +       vma->vm_flags &= ~VM_MAYWRITE;
> +
> +       pfru_log_dev = to_pfru_log_dev(file);
> +
> +       ret = get_pfru_log_data_info(&info, pfru_log_dev);
> +       if (ret)
> +               return ret;
> +
> +       base_addr = (phys_addr_t)((info.chunk2_addr_hi << 32) | info.chunk2_addr_lo);
> +       /* pfru update has not been launched yet */
> +       if (!base_addr)
> +               return -ENODEV;
> +
> +       psize = info.max_data_size;
> +       /* base address and total buffer size must be page aligned */
> +       if (!PAGE_ALIGNED(base_addr) || !PAGE_ALIGNED(psize))
> +               return -ENODEV;
> +
> +       vsize = vma->vm_end - vma->vm_start;
> +       if (vsize > psize)
> +               return -EINVAL;
> +
> +       vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +       if (io_remap_pfn_range(vma, vma->vm_start, PFN_DOWN(base_addr),
> +                              vsize, vma->vm_page_prot))
> +               return -EAGAIN;
> +
> +       return 0;
> +}
> +
> +static const struct file_operations acpi_pfru_log_fops = {
> +       .owner          = THIS_MODULE,
> +       .mmap           = pfru_log_mmap,
> +       .unlocked_ioctl = pfru_log_ioctl,
> +       .llseek         = noop_llseek,
> +};
> +
> +static int acpi_pfru_log_remove(struct platform_device *pdev)
> +{
> +       struct pfru_log_device *pfru_log_dev = platform_get_drvdata(pdev);
> +
> +       misc_deregister(&pfru_log_dev->miscdev);
> +
> +       return 0;
> +}
> +
> +static void pfru_log_put_idx(void *data)
> +{
> +       struct pfru_log_device *pfru_log_dev = data;
> +
> +       ida_free(&pfru_log_ida, pfru_log_dev->index);
> +}
> +
> +static int acpi_pfru_log_probe(struct platform_device *pdev)
> +{
> +       acpi_handle handle = ACPI_HANDLE(&pdev->dev);
> +       struct pfru_log_device *pfru_log_dev;
> +       int ret;
> +
> +       if (!acpi_has_method(handle, "_DSM")) {
> +               dev_dbg(&pdev->dev, "Missing _DSM\n");
> +               return -ENODEV;
> +       }
> +
> +       pfru_log_dev = devm_kzalloc(&pdev->dev, sizeof(*pfru_log_dev), GFP_KERNEL);
> +       if (!pfru_log_dev)
> +               return -ENOMEM;
> +
> +       ret = ida_alloc(&pfru_log_ida, GFP_KERNEL);
> +       if (ret < 0)
> +               return ret;
> +
> +       pfru_log_dev->index = ret;
> +       ret = devm_add_action_or_reset(&pdev->dev, pfru_log_put_idx, pfru_log_dev);
> +       if (ret)
> +               return ret;
> +
> +       pfru_log_dev->info.log_revid = PFRU_DEFAULT_REV_ID;
> +       pfru_log_dev->parent_dev = &pdev->dev;
> +
> +       pfru_log_dev->miscdev.minor = MISC_DYNAMIC_MINOR;
> +       pfru_log_dev->miscdev.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> +                                                   "pfru_telemetry%d",

I would call it "pfr_telemetry" (without the "u" in the prefix).

> +                                                   pfru_log_dev->index);
> +       if (!pfru_log_dev->miscdev.name)
> +               return -ENOMEM;
> +
> +       pfru_log_dev->miscdev.nodename = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> +                                                       "acpi_pfru_telemetry%d",

And this one (analogously) "acpi_pfr_telemetry".

> +                                                       pfru_log_dev->index);
> +       if (!pfru_log_dev->miscdev.nodename)
> +               return -ENOMEM;
> +
> +       pfru_log_dev->miscdev.fops = &acpi_pfru_log_fops;
> +       pfru_log_dev->miscdev.parent = &pdev->dev;
> +
> +       ret = misc_register(&pfru_log_dev->miscdev);
> +       if (ret)
> +               return ret;
> +
> +       platform_set_drvdata(pdev, pfru_log_dev);
> +
> +       return 0;
> +}
> +
> +static const struct acpi_device_id acpi_pfru_log_ids[] = {
> +       {"INTC1081"},
> +       {}
> +};
> +MODULE_DEVICE_TABLE(acpi, acpi_pfru_log_ids);
> +
> +static struct platform_driver acpi_pfru_log_driver = {
> +       .driver = {
> +               .name = "pfru_telemetry",
> +               .acpi_match_table = acpi_pfru_log_ids,
> +       },
> +       .probe = acpi_pfru_log_probe,
> +       .remove = acpi_pfru_log_remove,
> +};
> +module_platform_driver(acpi_pfru_log_driver);
> +
> +MODULE_DESCRIPTION("Platform Firmware Runtime Update Telemetry driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/uapi/linux/pfru.h b/include/uapi/linux/pfru.h
> index fed50cb01309..2f17714e05b7 100644
> --- a/include/uapi/linux/pfru.h
> +++ b/include/uapi/linux/pfru.h
> @@ -171,4 +171,92 @@ struct pfru_updated_result {
>         __u64 high_exec_time;
>  };
>
> +/**
> + * struct pfru_log_data_info - Log Data from telemetry service.
> + * @status: Indicator of whether this update succeed.
> + * @ext_status: Implementation specific update result.
> + * @chunk1_addr_lo: Low 32bit physical address of the telemetry data chunk1
> + *                  starting address.
> + * @chunk1_addr_hi: High 32bit physical address of the telemetry data chunk1
> + *                  starting address.
> + * @chunk2_addr_lo: Low 32bit physical address of the telemetry data chunk2
> + *                  starting address.
> + * @chunk2_addr_hi: High 32bit physical address of the telemetry data chunk2
> + *                  starting address.
> + * @max_data_size: Maximum supported size of data of all data chunks combined.
> + * @chunk1_size: Data size in bytes of the telemetry data chunk1 buffer.
> + * @chunk2_size: Data size in bytes of the telemetry data chunk2 buffer.
> + * @rollover_cnt: Number of times telemetry data buffer is overwritten
> + *                since telemetry buffer reset.
> + * @reset_cnt: Number of times telemetry services resets that results in
> + *             rollover count and data chunk buffers are reset.
> + */
> +struct pfru_log_data_info {
> +       __u32 status;
> +       __u32 ext_status;
> +       __u64 chunk1_addr_lo;
> +       __u64 chunk1_addr_hi;
> +       __u64 chunk2_addr_lo;
> +       __u64 chunk2_addr_hi;
> +       __u32 max_data_size;
> +       __u32 chunk1_size;
> +       __u32 chunk2_size;
> +       __u32 rollover_cnt;
> +       __u32 reset_cnt;
> +};
> +
> +/**
> + * struct pfru_log_info - Telemetry log information.
> + * @log_level: The telemetry log level.
> + * @log_type: The telemetry log type(history and execution).
> + * @log_revid: The telemetry log revision id.
> + */
> +struct pfru_log_info {
> +       __u32 log_level;
> +       __u32 log_type;
> +       __u32 log_revid;
> +};
> +
> +/**
> + * PFRU_LOG_IOC_SET_INFO - _IOW(PFRU_MAGIC_FOR_IOCTL, 0x06,
> + *                             struct pfru_log_info)
> + *
> + * Return:
> + * * 0                 - success
> + * * -EFAULT           - fail to get the setting parameter
> + * * -EINVAL           - fail to set the log level
> + *
> + * Set the PFRU log level and log type. The input information is
> + * a struct pfru_log_info.
> + */
> +#define PFRU_LOG_IOC_SET_INFO _IOW(PFRU_MAGIC_FOR_IOCTL, 0x06, struct pfru_log_info)
> +
> +/**
> + * PFRU_LOG_IOC_GET_INFO - _IOR(PFRU_MAGIC_FOR_IOCTL, 0x07,
> + *                             struct pfru_log_info)
> + *
> + * Return:
> + * * 0                 - success
> + * * -EINVAL           - fail to get the log level
> + * * -EFAULT           - fail to copy the result back to userspace
> + *
> + * Retrieve log level and log type of the PFRU telemetry. The information is
> + * a struct pfru_log_info.
> + */
> +#define PFRU_LOG_IOC_GET_INFO _IOR(PFRU_MAGIC_FOR_IOCTL, 0x07, struct pfru_log_info)
> +
> +/**
> + * PFRU_LOG_IOC_GET_DATA_INFO - _IOR(PFRU_MAGIC_FOR_IOCTL, 0x08,
> + *                                  struct pfru_log_data_info)
> + *
> + * Return:
> + * * 0                 - success
> + * * -EINVAL           - fail to get the log buffer information
> + * * -EFAULT           - fail to copy the log buffer information to userspace
> + *
> + * Retrieve data information about the PFRU telemetry. The information
> + * is a struct pfru_log_data_info.
> + */
> +#define PFRU_LOG_IOC_GET_DATA_INFO _IOR(PFRU_MAGIC_FOR_IOCTL, 0x08, struct pfru_log_data_info)
> +
>  #endif /* __PFRU_H__ */

And of course it needs to be rebased to take the changes in the
previous patch into account.

Overall, it's almost there IMO.