2024-04-15 21:52:33

by srinivas pandruvada

Subject: [PATCH] platform/x86/intel-uncore-freq: Don't present root domain on error

If none of the clusters are added because of some error, fail to load
driver without presenting root domain. In this case root domain will
present invalid data.

Signed-off-by: Srinivas Pandruvada <[email protected]>
Fixes: 01c10f88c9b7 ("platform/x86/intel-uncore-freq: tpmi: Provide cluster level control")
Cc: <[email protected]> # 6.5+
---
This error can be reproduced in the pre production hardware only.
So can go through regular cycle and they apply to stable.

.../x86/intel/uncore-frequency/uncore-frequency-tpmi.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
index bd75d61ff8a6..587437211d72 100644
--- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
+++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
@@ -240,6 +240,7 @@ static int uncore_probe(struct auxiliary_device *auxdev, const struct auxiliary_
bool read_blocked = 0, write_blocked = 0;
struct intel_tpmi_plat_info *plat_info;
struct tpmi_uncore_struct *tpmi_uncore;
+ bool uncore_sysfs_added = false;
int ret, i, pkg = 0;
int num_resources;

@@ -384,9 +385,15 @@ static int uncore_probe(struct auxiliary_device *auxdev, const struct auxiliary_
}
/* Point to next cluster offset */
cluster_offset >>= UNCORE_MAX_CLUSTER_PER_DOMAIN;
+ uncore_sysfs_added = true;
}
}

+ if (!uncore_sysfs_added) {
+ ret = -ENODEV;
+ goto remove_clusters;
+ }
+
auxiliary_set_drvdata(auxdev, tpmi_uncore);

tpmi_uncore->root_cluster.root_domain = true;
--
2.40.1

2024-04-22 13:26:12

by Hans de Goede

Subject: Re: [PATCH] platform/x86/intel-uncore-freq: Don't present root domain on error

Hi,

On 4/15/24 11:52 PM, Srinivas Pandruvada wrote:
> If none of the clusters are added because of some error, fail to load
> driver without presenting root domain. In this case root domain will
> present invalid data.
>
> Signed-off-by: Srinivas Pandruvada <[email protected]>
> Fixes: 01c10f88c9b7 ("platform/x86/intel-uncore-freq: tpmi: Provide cluster level control")
> Cc: <[email protected]> # 6.5+
> ---
> This error can be reproduced in the pre production hardware only.
> So can go through regular cycle and they apply to stable.

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans




>
> .../x86/intel/uncore-frequency/uncore-frequency-tpmi.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
> index bd75d61ff8a6..587437211d72 100644
> --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
> +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-tpmi.c
> @@ -240,6 +240,7 @@ static int uncore_probe(struct auxiliary_device *auxdev, const struct auxiliary_
> bool read_blocked = 0, write_blocked = 0;
> struct intel_tpmi_plat_info *plat_info;
> struct tpmi_uncore_struct *tpmi_uncore;
> + bool uncore_sysfs_added = false;
> int ret, i, pkg = 0;
> int num_resources;
>
> @@ -384,9 +385,15 @@ static int uncore_probe(struct auxiliary_device *auxdev, const struct auxiliary_
> }
> /* Point to next cluster offset */
> cluster_offset >>= UNCORE_MAX_CLUSTER_PER_DOMAIN;
> + uncore_sysfs_added = true;
> }
> }
>
> + if (!uncore_sysfs_added) {
> + ret = -ENODEV;
> + goto remove_clusters;
> + }
> +
> auxiliary_set_drvdata(auxdev, tpmi_uncore);
>
> tpmi_uncore->root_cluster.root_domain = true;