* Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-10-15 14:54 ` [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values Shyam Sundar S K
@ 2025-10-15 15:03 ` Mario Limonciello
2025-10-16 8:13 ` Shen, Yijun
2025-11-05 16:46 ` Ilpo Järvinen
2 siblings, 0 replies; 10+ messages in thread
From: Mario Limonciello @ 2025-10-15 15:03 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede, ilpo.jarvinen
Cc: platform-driver-x86, Patil.Reddy, Yijun.Shen
On 10/15/25 9:54 AM, Shyam Sundar S K wrote:
> Custom BIOS input values can be updated by multiple sources, such as power
> mode changes and sensor events, each triggering a custom BIOS input event.
> When these events occur in rapid succession, new data may overwrite
> previous values before they are processed, resulting in lost updates.
>
> To address this, introduce a fixed-size, power-of-two ring buffer to
> capture every custom BIOS input event, storing both the pending request
> and its associated input values. Access to the ring buffer is synchronized
> using a mutex.
>
> The previous use of memset() to clear the pending request structure after
> each event is removed, as each BIOS input value is now copied into the
> buffer as a snapshot. Consumers now process entries directly from the ring
> buffer, making explicit clearing of the pending request structure
> unnecessary.
>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v2:
> - Add dev_WARN_ONCE()
> - Change variable name rb_mutex to cbi_mutex
> - Move tail increment logic above pending request check
>
> drivers/platform/x86/amd/pmf/acpi.c | 41 +++++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/core.c | 2 ++
> drivers/platform/x86/amd/pmf/pmf.h | 20 +++++++++++++
> drivers/platform/x86/amd/pmf/spc.c | 23 +++++++++------
> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
> 5 files changed, 79 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 13c4fec2c7ef..870a56f1fe07 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
> req, sizeof(*req));
> }
>
> +/* Store custom BIOS inputs data in ring buffer */
> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
> +{
> + struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> + struct bios_input_entry entry = { };
> + int i;
> +
> + guard(mutex)(&pmf_dev->cbi_mutex);
> +
> + switch (pmf_dev->cpu_id) {
> + case AMD_CPU_ID_PS:
> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> + entry.val[i] = pmf_dev->req1.custom_policy[i];
> + entry.preq = pmf_dev->req1.pending_req;
> + break;
> + case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> + case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> + entry.val[i] = pmf_dev->req.custom_policy[i];
> + entry.preq = pmf_dev->req.pending_req;
> + break;
> + default:
> + return;
> + }
> +
> + if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
> + rb->data[rb->head] = entry;
> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> + } else {
> + /* Rare case: ensures the newest BIOS input value is kept */
> + dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
> + rb->data[rb->head] = entry;
> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> + }
> +}
> +
> static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
> {
> if (!pdev->cb_flag)
> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
> dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
>
> amd_pmf_handle_early_preq(pmf_dev);
> +
> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> }
>
> static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
>
> amd_pmf_handle_early_preq(pmf_dev);
> +
> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> }
>
> static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index bc544a4a5266..8b97eba00dd3 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
> mutex_init(&dev->lock);
> mutex_init(&dev->update_mutex);
> mutex_init(&dev->cb_mutex);
> + mutex_init(&dev->cbi_mutex);
>
> apmf_acpi_init(dev);
> platform_set_drvdata(pdev, dev);
> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
> mutex_destroy(&dev->lock);
> mutex_destroy(&dev->update_mutex);
> mutex_destroy(&dev->cb_mutex);
> + mutex_destroy(&dev->cbi_mutex);
> }
>
> static const struct attribute_group *amd_pmf_driver_groups[] = {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 2145df4128cd..6cb1e228c48e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,6 +12,7 @@
> #define PMF_H
>
> #include <linux/acpi.h>
> +#include <linux/circ_buf.h>
> #include <linux/input.h>
> #include <linux/platform_device.h>
> #include <linux/platform_profile.h>
> @@ -120,6 +121,7 @@ struct cookie_header {
> #define APTS_MAX_STATES 16
> #define CUSTOM_BIOS_INPUT_BITS GENMASK(16, 7)
> #define BIOS_INPUTS_MAX 10
> +#define CUSTOM_BIOS_INPUT_RB_SZ 64 /* Must be power of two for CIRC_* macros */
>
> typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>
> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
> u32 custom_bios_inputs[BIOS_INPUTS_MAX];
> };
>
> +/**
> + * struct bios_input_entry - Snapshot of custom BIOS input event
> + * @val: Array of custom BIOS input values
> + * @preq: Pending request value associated with this event
> + */
> +struct bios_input_entry {
> + u32 val[BIOS_INPUTS_MAX];
> + u32 preq;
> +};
> +
> +struct cbi_ring_buffer {
> + struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> + int head;
> + int tail;
> +};
> +
> struct amd_pmf_dev {
> void __iomem *regbase;
> void __iomem *smu_virt_addr;
> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
> struct apmf_sbios_req_v1 req1;
> struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
> bool cb_flag; /* To handle first custom BIOS input */
> + struct cbi_ring_buffer cbi_buf;
> + struct mutex cbi_mutex; /* Protects ring buffer access */
> };
>
> struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 85192c7536b8..669680ce580a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
> static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> struct ta_pmf_enact_table *in)
> {
> + struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> + struct bios_input_entry entry = { };
> unsigned int i;
>
> + guard(mutex)(&pdev->cbi_mutex);
> +
> for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
>
> - if (!(pdev->req.pending_req || pdev->req1.pending_req))
> + if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
> + return; /* return if ring buffer is empty */
> +
> + entry = rb->data[rb->tail];
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +
> + /* If no active custom BIOS input pending request, do not consume further work */
> + if (!entry.preq)
> return;
>
> if (!pdev->smart_pc_enabled)
> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> case PMF_IF_V1:
> if (!is_apmf_bios_input_notifications_supported(pdev))
> return;
> - amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
> - pdev->req1.custom_policy, in);
> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
> break;
> case PMF_IF_V2:
> - amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
> - pdev->req.custom_policy, in);
> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
> break;
> default:
> break;
> }
> -
> - /* Clear pending requests after handling */
> - memset(&pdev->req, 0, sizeof(pdev->req));
> - memset(&pdev->req1, 0, sizeof(pdev->req1));
> }
>
> static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..add742e33e1e 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
> status = ret == TA_PMF_TYPE_SUCCESS;
> if (status) {
> dev->cb_flag = true;
> + dev->cbi_buf.head = 0;
> + dev->cbi_buf.tail = 0;
> break;
> }
> amd_pmf_tee_deinit(dev);
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-10-15 14:54 ` [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values Shyam Sundar S K
2025-10-15 15:03 ` Mario Limonciello
@ 2025-10-16 8:13 ` Shen, Yijun
2025-11-05 16:46 ` Ilpo Järvinen
2 siblings, 0 replies; 10+ messages in thread
From: Shen, Yijun @ 2025-10-16 8:13 UTC (permalink / raw)
To: Shyam Sundar S K, hdegoede@redhat.com,
ilpo.jarvinen@linux.intel.com
Cc: platform-driver-x86@vger.kernel.org, Patil Rajesh,
Limonciello, Mario
> -----Original Message-----
> From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Sent: Wednesday, October 15, 2025 10:55 PM
> To: hdegoede@redhat.com; ilpo.jarvinen@linux.intel.com
> Cc: platform-driver-x86@vger.kernel.org; Patil Rajesh
> <Patil.Reddy@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Shen, Yijun <Yijun.Shen@dell.com>; Shyam
> Sundar S K <Shyam-sundar.S-k@amd.com>
> Subject: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store
> custom BIOS input values
>
>
> [EXTERNAL EMAIL]
>
> Custom BIOS input values can be updated by multiple sources, such as power
> mode changes and sensor events, each triggering a custom BIOS input event.
> When these events occur in rapid succession, new data may overwrite
> previous values before they are processed, resulting in lost updates.
>
> To address this, introduce a fixed-size, power-of-two ring buffer to capture
> every custom BIOS input event, storing both the pending request and its
> associated input values. Access to the ring buffer is synchronized using a
> mutex.
>
> The previous use of memset() to clear the pending request structure after
> each event is removed, as each BIOS input value is now copied into the buffer
> as a snapshot. Consumers now process entries directly from the ring buffer,
> making explicit clearing of the pending request structure unnecessary.
>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Verified this patch on the issued system.
Tested-By: Yijun Shen <Yijun.Shen@Dell.com>
> ---
> v2:
> - Add dev_WARN_ONCE()
> - Change variable name rb_mutex to cbi_mutex
> - Move tail increment logic above pending request check
>
> drivers/platform/x86/amd/pmf/acpi.c | 41 +++++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/core.c | 2 ++
> drivers/platform/x86/amd/pmf/pmf.h | 20 +++++++++++++
> drivers/platform/x86/amd/pmf/spc.c | 23 +++++++++------
> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
> 5 files changed, 79 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c
> b/drivers/platform/x86/amd/pmf/acpi.c
> index 13c4fec2c7ef..870a56f1fe07 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev
> *pdev, struct apmf_sbios_req *req
> req,
> sizeof(*req));
> }
>
> +/* Store custom BIOS inputs data in ring buffer */ static void
> +amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev) {
> + struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> + struct bios_input_entry entry = { };
> + int i;
> +
> + guard(mutex)(&pmf_dev->cbi_mutex);
> +
> + switch (pmf_dev->cpu_id) {
> + case AMD_CPU_ID_PS:
> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> + entry.val[i] = pmf_dev->req1.custom_policy[i];
> + entry.preq = pmf_dev->req1.pending_req;
> + break;
> + case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> + case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> + entry.val[i] = pmf_dev->req.custom_policy[i];
> + entry.preq = pmf_dev->req.pending_req;
> + break;
> + default:
> + return;
> + }
> +
> + if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0)
> {
> + rb->data[rb->head] = entry;
> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ -
> 1);
> + } else {
> + /* Rare case: ensures the newest BIOS input value is kept */
> + dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input
> value, data may be lost\n");
> + rb->data[rb->head] = entry;
> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ -
> 1);
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> + }
> +}
> +
> static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev) {
> if (!pdev->cb_flag)
> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle
> handle, u32 event, void *data)
> dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev-
> >req.pending_req);
>
> amd_pmf_handle_early_preq(pmf_dev);
> +
> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> }
>
> static void apmf_event_handler_v1(acpi_handle handle, u32 event, void
> *data) @@ -374,6 +413,8 @@ static void
> apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n",
> pmf_dev->req1.pending_req);
>
> amd_pmf_handle_early_preq(pmf_dev);
> +
> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> }
>
> static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> diff --git a/drivers/platform/x86/amd/pmf/core.c
> b/drivers/platform/x86/amd/pmf/core.c
> index bc544a4a5266..8b97eba00dd3 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device
> *pdev)
> mutex_init(&dev->lock);
> mutex_init(&dev->update_mutex);
> mutex_init(&dev->cb_mutex);
> + mutex_init(&dev->cbi_mutex);
>
> apmf_acpi_init(dev);
> platform_set_drvdata(pdev, dev);
> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device
> *pdev)
> mutex_destroy(&dev->lock);
> mutex_destroy(&dev->update_mutex);
> mutex_destroy(&dev->cb_mutex);
> + mutex_destroy(&dev->cbi_mutex);
> }
>
> static const struct attribute_group *amd_pmf_driver_groups[] = { diff --git
> a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index 2145df4128cd..6cb1e228c48e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,6 +12,7 @@
> #define PMF_H
>
> #include <linux/acpi.h>
> +#include <linux/circ_buf.h>
> #include <linux/input.h>
> #include <linux/platform_device.h>
> #include <linux/platform_profile.h>
> @@ -120,6 +121,7 @@ struct cookie_header {
> #define APTS_MAX_STATES 16
> #define CUSTOM_BIOS_INPUT_BITS GENMASK(16, 7)
> #define BIOS_INPUTS_MAX 10
> +#define CUSTOM_BIOS_INPUT_RB_SZ 64 /* Must be power of two for
> CIRC_* macros */
>
> typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void
> *data);
>
> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
> u32 custom_bios_inputs[BIOS_INPUTS_MAX];
> };
>
> +/**
> + * struct bios_input_entry - Snapshot of custom BIOS input event
> + * @val: Array of custom BIOS input values
> + * @preq: Pending request value associated with this event */ struct
> +bios_input_entry {
> + u32 val[BIOS_INPUTS_MAX];
> + u32 preq;
> +};
> +
> +struct cbi_ring_buffer {
> + struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> + int head;
> + int tail;
> +};
> +
> struct amd_pmf_dev {
> void __iomem *regbase;
> void __iomem *smu_virt_addr;
> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
> struct apmf_sbios_req_v1 req1;
> struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS
> inputs */
> bool cb_flag; /* To handle first custom BIOS
> input */
> + struct cbi_ring_buffer cbi_buf;
> + struct mutex cbi_mutex; /* Protects ring buffer
> access */
> };
>
> struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/spc.c
> b/drivers/platform/x86/amd/pmf/spc.c
> index 85192c7536b8..669680ce580a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct
> amd_pmf_dev *pdev, u32 pending_req static void
> amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> struct ta_pmf_enact_table *in)
> {
> + struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> + struct bios_input_entry entry = { };
> unsigned int i;
>
> + guard(mutex)(&pdev->cbi_mutex);
> +
> for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> amd_pmf_set_ta_custom_bios_input(in, i, pdev-
> >cb_prev.custom_bios_inputs[i]);
>
> - if (!(pdev->req.pending_req || pdev->req1.pending_req))
> + if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
> + return; /* return if ring buffer is empty */
> +
> + entry = rb->data[rb->tail];
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +
> + /* If no active custom BIOS input pending request, do not consume
> further work */
> + if (!entry.preq)
> return;
>
> if (!pdev->smart_pc_enabled)
> @@ -165,20 +176,14 @@ static void
> amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> case PMF_IF_V1:
> if (!is_apmf_bios_input_notifications_supported(pdev))
> return;
> - amd_pmf_update_bios_inputs(pdev, pdev-
> >req1.pending_req, custom_bios_inputs_v1,
> - pdev->req1.custom_policy, in);
> + amd_pmf_update_bios_inputs(pdev, entry.preq,
> custom_bios_inputs_v1,
> +entry.val, in);
> break;
> case PMF_IF_V2:
> - amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req,
> custom_bios_inputs,
> - pdev->req.custom_policy, in);
> + amd_pmf_update_bios_inputs(pdev, entry.preq,
> custom_bios_inputs,
> +entry.val, in);
> break;
> default:
> break;
> }
> -
> - /* Clear pending requests after handling */
> - memset(&pdev->req, 0, sizeof(pdev->req));
> - memset(&pdev->req1, 0, sizeof(pdev->req1));
> }
>
> static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct
> ta_pmf_enact_table *in) diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
> b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..add742e33e1e 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev
> *dev)
> status = ret == TA_PMF_TYPE_SUCCESS;
> if (status) {
> dev->cb_flag = true;
> + dev->cbi_buf.head = 0;
> + dev->cbi_buf.tail = 0;
> break;
> }
> amd_pmf_tee_deinit(dev);
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-10-15 14:54 ` [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values Shyam Sundar S K
2025-10-15 15:03 ` Mario Limonciello
2025-10-16 8:13 ` Shen, Yijun
@ 2025-11-05 16:46 ` Ilpo Järvinen
2025-11-07 6:58 ` Shyam Sundar S K
2 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2025-11-05 16:46 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: hdegoede, platform-driver-x86, Patil.Reddy, mario.limonciello,
Yijun.Shen
On Wed, 15 Oct 2025, Shyam Sundar S K wrote:
> Custom BIOS input values can be updated by multiple sources, such as power
> mode changes and sensor events, each triggering a custom BIOS input event.
> When these events occur in rapid succession, new data may overwrite
> previous values before they are processed, resulting in lost updates.
>
> To address this, introduce a fixed-size, power-of-two ring buffer to
> capture every custom BIOS input event, storing both the pending request
> and its associated input values. Access to the ring buffer is synchronized
> using a mutex.
>
> The previous use of memset() to clear the pending request structure after
> each event is removed, as each BIOS input value is now copied into the
> buffer as a snapshot. Consumers now process entries directly from the ring
> buffer, making explicit clearing of the pending request structure
> unnecessary.
>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> v2:
> - Add dev_WARN_ONCE()
> - Change variable name rb_mutex to cbi_mutex
> - Move tail increment logic above pending request check
>
> drivers/platform/x86/amd/pmf/acpi.c | 41 +++++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/core.c | 2 ++
> drivers/platform/x86/amd/pmf/pmf.h | 20 +++++++++++++
> drivers/platform/x86/amd/pmf/spc.c | 23 +++++++++------
> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
> 5 files changed, 79 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 13c4fec2c7ef..870a56f1fe07 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
> req, sizeof(*req));
> }
>
> +/* Store custom BIOS inputs data in ring buffer */
> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
> +{
> + struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> + struct bios_input_entry entry = { };
Too generic struct name.
> + int i;
> +
> + guard(mutex)(&pmf_dev->cbi_mutex);
> +
> + switch (pmf_dev->cpu_id) {
> + case AMD_CPU_ID_PS:
> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> + entry.val[i] = pmf_dev->req1.custom_policy[i];
> + entry.preq = pmf_dev->req1.pending_req;
> + break;
> + case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> + case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
Add include.
> + entry.val[i] = pmf_dev->req.custom_policy[i];
> + entry.preq = pmf_dev->req.pending_req;
> + break;
> + default:
> + return;
> + }
> +
> + if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
> + rb->data[rb->head] = entry;
> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> + } else {
> + /* Rare case: ensures the newest BIOS input value is kept */
> + dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
Is this really WARN_ONCE thing? That is, does it indicate a bug in the
kernel code?
Add include.
> + rb->data[rb->head] = entry;
> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
Since you're under lock, can you make room separately first and then add
the entry on a common path?
> + }
> +}
> +
> static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
> {
> if (!pdev->cb_flag)
> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
> dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
>
> amd_pmf_handle_early_preq(pmf_dev);
> +
> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> }
>
> static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
>
> amd_pmf_handle_early_preq(pmf_dev);
> +
> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> }
>
> static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index bc544a4a5266..8b97eba00dd3 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
> mutex_init(&dev->lock);
> mutex_init(&dev->update_mutex);
> mutex_init(&dev->cb_mutex);
> + mutex_init(&dev->cbi_mutex);
>
> apmf_acpi_init(dev);
> platform_set_drvdata(pdev, dev);
> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
> mutex_destroy(&dev->lock);
> mutex_destroy(&dev->update_mutex);
> mutex_destroy(&dev->cb_mutex);
> + mutex_destroy(&dev->cbi_mutex);
> }
>
> static const struct attribute_group *amd_pmf_driver_groups[] = {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 2145df4128cd..6cb1e228c48e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,6 +12,7 @@
> #define PMF_H
>
> #include <linux/acpi.h>
> +#include <linux/circ_buf.h>
> #include <linux/input.h>
> #include <linux/platform_device.h>
> #include <linux/platform_profile.h>
> @@ -120,6 +121,7 @@ struct cookie_header {
> #define APTS_MAX_STATES 16
> #define CUSTOM_BIOS_INPUT_BITS GENMASK(16, 7)
> #define BIOS_INPUTS_MAX 10
> +#define CUSTOM_BIOS_INPUT_RB_SZ 64 /* Must be power of two for CIRC_* macros */
This is not really size but there are n entries. The total size will be
something else. I'd call it NUM or ENTRIES, or something like that.
Perhaps CUSTOM_BIOS_INPUT_RING_ENTRIES but that is a bit to the long side
of things?
> typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>
> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
> u32 custom_bios_inputs[BIOS_INPUTS_MAX];
> };
>
> +/**
> + * struct bios_input_entry - Snapshot of custom BIOS input event
> + * @val: Array of custom BIOS input values
> + * @preq: Pending request value associated with this event
> + */
> +struct bios_input_entry {
> + u32 val[BIOS_INPUTS_MAX];
> + u32 preq;
> +};
> +
> +struct cbi_ring_buffer {
> + struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> + int head;
> + int tail;
> +};
> +
> struct amd_pmf_dev {
> void __iomem *regbase;
> void __iomem *smu_virt_addr;
> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
> struct apmf_sbios_req_v1 req1;
> struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
> bool cb_flag; /* To handle first custom BIOS input */
> + struct cbi_ring_buffer cbi_buf;
> + struct mutex cbi_mutex; /* Protects ring buffer access */
Include missing?
> };
>
> struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 85192c7536b8..669680ce580a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
> static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> struct ta_pmf_enact_table *in)
> {
> + struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> + struct bios_input_entry entry = { };
> unsigned int i;
>
> + guard(mutex)(&pdev->cbi_mutex);
> +
> for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
>
> - if (!(pdev->req.pending_req || pdev->req1.pending_req))
> + if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
Include missing.
As a general comment unrelated to this patch, these files to be missing
other headers too.
> + return; /* return if ring buffer is empty */
> +
> + entry = rb->data[rb->tail];
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +
> + /* If no active custom BIOS input pending request, do not consume further work */
> + if (!entry.preq)
> return;
>
> if (!pdev->smart_pc_enabled)
> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> case PMF_IF_V1:
> if (!is_apmf_bios_input_notifications_supported(pdev))
> return;
> - amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
> - pdev->req1.custom_policy, in);
> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
> break;
> case PMF_IF_V2:
> - amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
> - pdev->req.custom_policy, in);
> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
Could we pass the entry instead now?
> break;
> default:
> break;
> }
> -
> - /* Clear pending requests after handling */
> - memset(&pdev->req, 0, sizeof(pdev->req));
> - memset(&pdev->req1, 0, sizeof(pdev->req1));
> }
>
> static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..add742e33e1e 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
> status = ret == TA_PMF_TYPE_SUCCESS;
> if (status) {
> dev->cb_flag = true;
> + dev->cbi_buf.head = 0;
> + dev->cbi_buf.tail = 0;
> break;
> }
> amd_pmf_tee_deinit(dev);
>
--
i.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-11-05 16:46 ` Ilpo Järvinen
@ 2025-11-07 6:58 ` Shyam Sundar S K
2025-11-07 10:22 ` Ilpo Järvinen
0 siblings, 1 reply; 10+ messages in thread
From: Shyam Sundar S K @ 2025-11-07 6:58 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: hdegoede, platform-driver-x86, Patil.Reddy, mario.limonciello,
Yijun.Shen
On 11/5/2025 22:16, Ilpo Järvinen wrote:
> On Wed, 15 Oct 2025, Shyam Sundar S K wrote:
>
>> Custom BIOS input values can be updated by multiple sources, such as power
>> mode changes and sensor events, each triggering a custom BIOS input event.
>> When these events occur in rapid succession, new data may overwrite
>> previous values before they are processed, resulting in lost updates.
>>
>> To address this, introduce a fixed-size, power-of-two ring buffer to
>> capture every custom BIOS input event, storing both the pending request
>> and its associated input values. Access to the ring buffer is synchronized
>> using a mutex.
>>
>> The previous use of memset() to clear the pending request structure after
>> each event is removed, as each BIOS input value is now copied into the
>> buffer as a snapshot. Consumers now process entries directly from the ring
>> buffer, making explicit clearing of the pending request structure
>> unnecessary.
>>
>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> v2:
>> - Add dev_WARN_ONCE()
>> - Change variable name rb_mutex to cbi_mutex
>> - Move tail increment logic above pending request check
>>
>> drivers/platform/x86/amd/pmf/acpi.c | 41 +++++++++++++++++++++++++++
>> drivers/platform/x86/amd/pmf/core.c | 2 ++
>> drivers/platform/x86/amd/pmf/pmf.h | 20 +++++++++++++
>> drivers/platform/x86/amd/pmf/spc.c | 23 +++++++++------
>> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
>> 5 files changed, 79 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
>> index 13c4fec2c7ef..870a56f1fe07 100644
>> --- a/drivers/platform/x86/amd/pmf/acpi.c
>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
>> req, sizeof(*req));
>> }
>>
>> +/* Store custom BIOS inputs data in ring buffer */
>> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
>> +{
>> + struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
>> + struct bios_input_entry entry = { };
>
> Too generic struct name.
>
>> + int i;
>> +
>> + guard(mutex)(&pmf_dev->cbi_mutex);
>> +
>> + switch (pmf_dev->cpu_id) {
>> + case AMD_CPU_ID_PS:
>> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
>> + entry.val[i] = pmf_dev->req1.custom_policy[i];
>> + entry.preq = pmf_dev->req1.pending_req;
>> + break;
>> + case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
>> + case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
>> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>
> Add include.
>
>> + entry.val[i] = pmf_dev->req.custom_policy[i];
>> + entry.preq = pmf_dev->req.pending_req;
>> + break;
>> + default:
>> + return;
>> + }
>> +
>> + if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
>> + rb->data[rb->head] = entry;
>> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>> + } else {
>> + /* Rare case: ensures the newest BIOS input value is kept */
>> + dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
>
> Is this really WARN_ONCE thing? That is, does it indicate a bug in the
> kernel code?
If we hit this case, we might end up in a debug spew and hence put it
under WARN_ONCE (that way we are not lost in debugging the odd cases,
if we are under this scenario).
Not a bug in the kernel code..
>
> Add include.
>
>> + rb->data[rb->head] = entry;
>> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>
> Since you're under lock, can you make room separately first and then add
> the entry on a common path?
>
>> + }
>> +}
>> +
>> static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
>> {
>> if (!pdev->cb_flag)
>> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
>> dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
>>
>> amd_pmf_handle_early_preq(pmf_dev);
>> +
>> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
>> }
>>
>> static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
>> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
>> dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
>>
>> amd_pmf_handle_early_preq(pmf_dev);
>> +
>> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
>> }
>>
>> static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>> index bc544a4a5266..8b97eba00dd3 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>> mutex_init(&dev->lock);
>> mutex_init(&dev->update_mutex);
>> mutex_init(&dev->cb_mutex);
>> + mutex_init(&dev->cbi_mutex);
>>
>> apmf_acpi_init(dev);
>> platform_set_drvdata(pdev, dev);
>> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
>> mutex_destroy(&dev->lock);
>> mutex_destroy(&dev->update_mutex);
>> mutex_destroy(&dev->cb_mutex);
>> + mutex_destroy(&dev->cbi_mutex);
>> }
>>
>> static const struct attribute_group *amd_pmf_driver_groups[] = {
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index 2145df4128cd..6cb1e228c48e 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -12,6 +12,7 @@
>> #define PMF_H
>>
>> #include <linux/acpi.h>
>> +#include <linux/circ_buf.h>
>> #include <linux/input.h>
>> #include <linux/platform_device.h>
>> #include <linux/platform_profile.h>
>> @@ -120,6 +121,7 @@ struct cookie_header {
>> #define APTS_MAX_STATES 16
>> #define CUSTOM_BIOS_INPUT_BITS GENMASK(16, 7)
>> #define BIOS_INPUTS_MAX 10
>> +#define CUSTOM_BIOS_INPUT_RB_SZ 64 /* Must be power of two for CIRC_* macros */
>
> This is not really size but there are n entries. The total size will be
> something else. I'd call it NUM or ENTRIES, or something like that.
> Perhaps CUSTOM_BIOS_INPUT_RING_ENTRIES but that is a bit to the long side
> of things?
>
>> typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>>
>> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
>> u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>> };
>>
>> +/**
>> + * struct bios_input_entry - Snapshot of custom BIOS input event
>> + * @val: Array of custom BIOS input values
>> + * @preq: Pending request value associated with this event
>> + */
>> +struct bios_input_entry {
>> + u32 val[BIOS_INPUTS_MAX];
>> + u32 preq;
>> +};
>> +
>> +struct cbi_ring_buffer {
>> + struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
>> + int head;
>> + int tail;
>> +};
>> +
>> struct amd_pmf_dev {
>> void __iomem *regbase;
>> void __iomem *smu_virt_addr;
>> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
>> struct apmf_sbios_req_v1 req1;
>> struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
>> bool cb_flag; /* To handle first custom BIOS input */
>> + struct cbi_ring_buffer cbi_buf;
>> + struct mutex cbi_mutex; /* Protects ring buffer access */
>
> Include missing?
yes..
>
>> };
>>
>> struct apmf_sps_prop_granular_v2 {
>> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
>> index 85192c7536b8..669680ce580a 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
>> static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>> struct ta_pmf_enact_table *in)
>> {
>> + struct cbi_ring_buffer *rb = &pdev->cbi_buf;
>> + struct bios_input_entry entry = { };
>> unsigned int i;
>>
>> + guard(mutex)(&pdev->cbi_mutex);
>> +
>> for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>> amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
>>
>> - if (!(pdev->req.pending_req || pdev->req1.pending_req))
>> + if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
>
> Include missing.
circ_buf.h is included in pmf.h since acpi.c and spc.c are using it.
>
> As a general comment unrelated to this patch, these files to be missing
> other headers too.
Yeah. I get your point. Let me add all the relavent missing headers
and respin a new version.
ack to other comments in the patch.
Thanks,
Shyam
>
>> + return; /* return if ring buffer is empty */
>> +
>> + entry = rb->data[rb->tail];
>> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>> +
>> + /* If no active custom BIOS input pending request, do not consume further work */
>> + if (!entry.preq)
>> return;
>>
>> if (!pdev->smart_pc_enabled)
>> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>> case PMF_IF_V1:
>> if (!is_apmf_bios_input_notifications_supported(pdev))
>> return;
>> - amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
>> - pdev->req1.custom_policy, in);
>> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
>> break;
>> case PMF_IF_V2:
>> - amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
>> - pdev->req.custom_policy, in);
>> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
>
> Could we pass the entry instead now?
>
>> break;
>> default:
>> break;
>> }
>> -
>> - /* Clear pending requests after handling */
>> - memset(&pdev->req, 0, sizeof(pdev->req));
>> - memset(&pdev->req1, 0, sizeof(pdev->req1));
>> }
>>
>> static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 6e8116bef4f6..add742e33e1e 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>> status = ret == TA_PMF_TYPE_SUCCESS;
>> if (status) {
>> dev->cb_flag = true;
>> + dev->cbi_buf.head = 0;
>> + dev->cbi_buf.tail = 0;
>> break;
>> }
>> amd_pmf_tee_deinit(dev);
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-11-07 6:58 ` Shyam Sundar S K
@ 2025-11-07 10:22 ` Ilpo Järvinen
2025-11-07 10:41 ` Shyam Sundar S K
0 siblings, 1 reply; 10+ messages in thread
From: Ilpo Järvinen @ 2025-11-07 10:22 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: hdegoede, platform-driver-x86, Patil.Reddy, mario.limonciello,
Yijun.Shen
[-- Attachment #1: Type: text/plain, Size: 11732 bytes --]
On Fri, 7 Nov 2025, Shyam Sundar S K wrote:
> On 11/5/2025 22:16, Ilpo Järvinen wrote:
> > On Wed, 15 Oct 2025, Shyam Sundar S K wrote:
> >
> >> Custom BIOS input values can be updated by multiple sources, such as power
> >> mode changes and sensor events, each triggering a custom BIOS input event.
> >> When these events occur in rapid succession, new data may overwrite
> >> previous values before they are processed, resulting in lost updates.
> >>
> >> To address this, introduce a fixed-size, power-of-two ring buffer to
> >> capture every custom BIOS input event, storing both the pending request
> >> and its associated input values. Access to the ring buffer is synchronized
> >> using a mutex.
> >>
> >> The previous use of memset() to clear the pending request structure after
> >> each event is removed, as each BIOS input value is now copied into the
> >> buffer as a snapshot. Consumers now process entries directly from the ring
> >> buffer, making explicit clearing of the pending request structure
> >> unnecessary.
> >>
> >> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> >> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> ---
> >> v2:
> >> - Add dev_WARN_ONCE()
> >> - Change variable name rb_mutex to cbi_mutex
> >> - Move tail increment logic above pending request check
> >>
> >> drivers/platform/x86/amd/pmf/acpi.c | 41 +++++++++++++++++++++++++++
> >> drivers/platform/x86/amd/pmf/core.c | 2 ++
> >> drivers/platform/x86/amd/pmf/pmf.h | 20 +++++++++++++
> >> drivers/platform/x86/amd/pmf/spc.c | 23 +++++++++------
> >> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
> >> 5 files changed, 79 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> >> index 13c4fec2c7ef..870a56f1fe07 100644
> >> --- a/drivers/platform/x86/amd/pmf/acpi.c
> >> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> >> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
> >> req, sizeof(*req));
> >> }
> >>
> >> +/* Store custom BIOS inputs data in ring buffer */
> >> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
> >> +{
> >> + struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> >> + struct bios_input_entry entry = { };
> >
> > Too generic struct name.
> >
> >> + int i;
> >> +
> >> + guard(mutex)(&pmf_dev->cbi_mutex);
> >> +
> >> + switch (pmf_dev->cpu_id) {
> >> + case AMD_CPU_ID_PS:
> >> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> >> + entry.val[i] = pmf_dev->req1.custom_policy[i];
> >> + entry.preq = pmf_dev->req1.pending_req;
> >> + break;
> >> + case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> >> + case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> >> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> >
> > Add include.
> >
> >> + entry.val[i] = pmf_dev->req.custom_policy[i];
> >> + entry.preq = pmf_dev->req.pending_req;
> >> + break;
> >> + default:
> >> + return;
> >> + }
> >> +
> >> + if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
> >> + rb->data[rb->head] = entry;
> >> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> >> + } else {
> >> + /* Rare case: ensures the newest BIOS input value is kept */
> >> + dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
> >
> > Is this really WARN_ONCE thing? That is, does it indicate a bug in the
> > kernel code?
>
> If we hit this case, we might end up in a debug spew and hence put it
> under WARN_ONCE (that way we are not lost in debugging the odd cases,
> if we are under this scenario).
Why doesn't printing just a dev_warn() suffice (it should be just as
visible in the log provided as the stacktrace, etc. is)? Why you need
stacktrace, etc. to differentiate?
--
i.
> Not a bug in the kernel code..
>
> >
> > Add include.
> >
> >> + rb->data[rb->head] = entry;
> >> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> >> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> >
> > Since you're under lock, can you make room separately first and then add
> > the entry on a common path?
> >
> >> + }
> >> +}
> >> +
> >> static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
> >> {
> >> if (!pdev->cb_flag)
> >> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
> >> dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
> >>
> >> amd_pmf_handle_early_preq(pmf_dev);
> >> +
> >> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> >> }
> >>
> >> static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> >> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> >> dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
> >>
> >> amd_pmf_handle_early_preq(pmf_dev);
> >> +
> >> + amd_pmf_custom_bios_inputs_rb(pmf_dev);
> >> }
> >>
> >> static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> >> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> >> index bc544a4a5266..8b97eba00dd3 100644
> >> --- a/drivers/platform/x86/amd/pmf/core.c
> >> +++ b/drivers/platform/x86/amd/pmf/core.c
> >> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
> >> mutex_init(&dev->lock);
> >> mutex_init(&dev->update_mutex);
> >> mutex_init(&dev->cb_mutex);
> >> + mutex_init(&dev->cbi_mutex);
> >>
> >> apmf_acpi_init(dev);
> >> platform_set_drvdata(pdev, dev);
> >> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
> >> mutex_destroy(&dev->lock);
> >> mutex_destroy(&dev->update_mutex);
> >> mutex_destroy(&dev->cb_mutex);
> >> + mutex_destroy(&dev->cbi_mutex);
> >> }
> >>
> >> static const struct attribute_group *amd_pmf_driver_groups[] = {
> >> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> >> index 2145df4128cd..6cb1e228c48e 100644
> >> --- a/drivers/platform/x86/amd/pmf/pmf.h
> >> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> >> @@ -12,6 +12,7 @@
> >> #define PMF_H
> >>
> >> #include <linux/acpi.h>
> >> +#include <linux/circ_buf.h>
> >> #include <linux/input.h>
> >> #include <linux/platform_device.h>
> >> #include <linux/platform_profile.h>
> >> @@ -120,6 +121,7 @@ struct cookie_header {
> >> #define APTS_MAX_STATES 16
> >> #define CUSTOM_BIOS_INPUT_BITS GENMASK(16, 7)
> >> #define BIOS_INPUTS_MAX 10
> >> +#define CUSTOM_BIOS_INPUT_RB_SZ 64 /* Must be power of two for CIRC_* macros */
> >
> > This is not really size but there are n entries. The total size will be
> > something else. I'd call it NUM or ENTRIES, or something like that.
> > Perhaps CUSTOM_BIOS_INPUT_RING_ENTRIES but that is a bit to the long side
> > of things?
> >
> >> typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
> >>
> >> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
> >> u32 custom_bios_inputs[BIOS_INPUTS_MAX];
> >> };
> >>
> >> +/**
> >> + * struct bios_input_entry - Snapshot of custom BIOS input event
> >> + * @val: Array of custom BIOS input values
> >> + * @preq: Pending request value associated with this event
> >> + */
> >> +struct bios_input_entry {
> >> + u32 val[BIOS_INPUTS_MAX];
> >> + u32 preq;
> >> +};
> >> +
> >> +struct cbi_ring_buffer {
> >> + struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> >> + int head;
> >> + int tail;
> >> +};
> >> +
> >> struct amd_pmf_dev {
> >> void __iomem *regbase;
> >> void __iomem *smu_virt_addr;
> >> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
> >> struct apmf_sbios_req_v1 req1;
> >> struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
> >> bool cb_flag; /* To handle first custom BIOS input */
> >> + struct cbi_ring_buffer cbi_buf;
> >> + struct mutex cbi_mutex; /* Protects ring buffer access */
> >
> > Include missing?
>
> yes..
>
> >
> >> };
> >>
> >> struct apmf_sps_prop_granular_v2 {
> >> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> >> index 85192c7536b8..669680ce580a 100644
> >> --- a/drivers/platform/x86/amd/pmf/spc.c
> >> +++ b/drivers/platform/x86/amd/pmf/spc.c
> >> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
> >> static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> >> struct ta_pmf_enact_table *in)
> >> {
> >> + struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> >> + struct bios_input_entry entry = { };
> >> unsigned int i;
> >>
> >> + guard(mutex)(&pdev->cbi_mutex);
> >> +
> >> for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> >> amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
> >>
> >> - if (!(pdev->req.pending_req || pdev->req1.pending_req))
> >> + if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
> >
> > Include missing.
>
> circ_buf.h is included in pmf.h since acpi.c and spc.c are using it.
>
> >
> > As a general comment unrelated to this patch, these files to be missing
> > other headers too.
>
> Yeah. I get your point. Let me add all the relavent missing headers
> and respin a new version.
>
> ack to other comments in the patch.
>
> Thanks,
> Shyam
> >
> >> + return; /* return if ring buffer is empty */
> >> +
> >> + entry = rb->data[rb->tail];
> >> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> >> +
> >> + /* If no active custom BIOS input pending request, do not consume further work */
> >> + if (!entry.preq)
> >> return;
> >>
> >> if (!pdev->smart_pc_enabled)
> >> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> >> case PMF_IF_V1:
> >> if (!is_apmf_bios_input_notifications_supported(pdev))
> >> return;
> >> - amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
> >> - pdev->req1.custom_policy, in);
> >> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
> >> break;
> >> case PMF_IF_V2:
> >> - amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
> >> - pdev->req.custom_policy, in);
> >> + amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
> >
> > Could we pass the entry instead now?
> >
> >> break;
> >> default:
> >> break;
> >> }
> >> -
> >> - /* Clear pending requests after handling */
> >> - memset(&pdev->req, 0, sizeof(pdev->req));
> >> - memset(&pdev->req1, 0, sizeof(pdev->req1));
> >> }
> >>
> >> static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
> >> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> >> index 6e8116bef4f6..add742e33e1e 100644
> >> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> >> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> >> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
> >> status = ret == TA_PMF_TYPE_SUCCESS;
> >> if (status) {
> >> dev->cb_flag = true;
> >> + dev->cbi_buf.head = 0;
> >> + dev->cbi_buf.tail = 0;
> >> break;
> >> }
> >> amd_pmf_tee_deinit(dev);
> >>
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-11-07 10:22 ` Ilpo Järvinen
@ 2025-11-07 10:41 ` Shyam Sundar S K
0 siblings, 0 replies; 10+ messages in thread
From: Shyam Sundar S K @ 2025-11-07 10:41 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: platform-driver-x86, Patil.Reddy, mario.limonciello, Yijun.Shen,
hansg
On 11/7/2025 15:52, Ilpo Järvinen wrote:
> On Fri, 7 Nov 2025, Shyam Sundar S K wrote:
>> On 11/5/2025 22:16, Ilpo Järvinen wrote:
>>> On Wed, 15 Oct 2025, Shyam Sundar S K wrote:
>>>
>>>> Custom BIOS input values can be updated by multiple sources, such as power
>>>> mode changes and sensor events, each triggering a custom BIOS input event.
>>>> When these events occur in rapid succession, new data may overwrite
>>>> previous values before they are processed, resulting in lost updates.
>>>>
>>>> To address this, introduce a fixed-size, power-of-two ring buffer to
>>>> capture every custom BIOS input event, storing both the pending request
>>>> and its associated input values. Access to the ring buffer is synchronized
>>>> using a mutex.
>>>>
>>>> The previous use of memset() to clear the pending request structure after
>>>> each event is removed, as each BIOS input value is now copied into the
>>>> buffer as a snapshot. Consumers now process entries directly from the ring
>>>> buffer, making explicit clearing of the pending request structure
>>>> unnecessary.
>>>>
>>>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>>>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>> ---
>>>> v2:
>>>> - Add dev_WARN_ONCE()
>>>> - Change variable name rb_mutex to cbi_mutex
>>>> - Move tail increment logic above pending request check
>>>>
>>>> drivers/platform/x86/amd/pmf/acpi.c | 41 +++++++++++++++++++++++++++
>>>> drivers/platform/x86/amd/pmf/core.c | 2 ++
>>>> drivers/platform/x86/amd/pmf/pmf.h | 20 +++++++++++++
>>>> drivers/platform/x86/amd/pmf/spc.c | 23 +++++++++------
>>>> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
>>>> 5 files changed, 79 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
>>>> index 13c4fec2c7ef..870a56f1fe07 100644
>>>> --- a/drivers/platform/x86/amd/pmf/acpi.c
>>>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>>>> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
>>>> req, sizeof(*req));
>>>> }
>>>>
>>>> +/* Store custom BIOS inputs data in ring buffer */
>>>> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
>>>> +{
>>>> + struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
>>>> + struct bios_input_entry entry = { };
>>>
>>> Too generic struct name.
>>>
>>>> + int i;
>>>> +
>>>> + guard(mutex)(&pmf_dev->cbi_mutex);
>>>> +
>>>> + switch (pmf_dev->cpu_id) {
>>>> + case AMD_CPU_ID_PS:
>>>> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
>>>> + entry.val[i] = pmf_dev->req1.custom_policy[i];
>>>> + entry.preq = pmf_dev->req1.pending_req;
>>>> + break;
>>>> + case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
>>>> + case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
>>>> + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>>>
>>> Add include.
>>>
>>>> + entry.val[i] = pmf_dev->req.custom_policy[i];
>>>> + entry.preq = pmf_dev->req.pending_req;
>>>> + break;
>>>> + default:
>>>> + return;
>>>> + }
>>>> +
>>>> + if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
>>>> + rb->data[rb->head] = entry;
>>>> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>>>> + } else {
>>>> + /* Rare case: ensures the newest BIOS input value is kept */
>>>> + dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
>>>
>>> Is this really WARN_ONCE thing? That is, does it indicate a bug in the
>>> kernel code?
>>
>> If we hit this case, we might end up in a debug spew and hence put it
>> under WARN_ONCE (that way we are not lost in debugging the odd cases,
>> if we are under this scenario).
>
> Why doesn't printing just a dev_warn() suffice (it should be just as
> visible in the log provided as the stacktrace, etc. is)? Why you need
> stacktrace, etc. to differentiate?
>
Yes, that was the intent. But let me try with dev_warn() again..
Thanks,
Shyam
^ permalink raw reply [flat|nested] 10+ messages in thread