[PATCH v3] drm/xe/hwmon: Update xe hwmon with couple of fixes
Karthik Poosa
karthik.poosa at intel.com
Tue Apr 2 18:09:56 UTC 2024
Address potential overflows in result of multiplication of two lower
precision (u32) operands before widening it to higher precision (u64).
Initialize variables which were being used uninitialized.
v2:
- Rebase.
- In xe_hwmon_process_reg, set argument 'value' to 0 on failure.
With this change caller function need not initialize value to 0.
v3:
- Update commit msg as per review comments (Rodrigo, Lucas).
- Update xe_hwmon_get_reg. Return bool from xe_hwmon_get_reg
and output xe_reg. This is to have abstracted usage of xe_reg. (Lucas)
Fixes: 4446fcf220ce ("drm/xe/hwmon: Expose power1_max_interval")
Signed-off-by: Karthik Poosa <karthik.poosa at intel.com>
Reviewed-by: Anshuman Gupta <anshuman.gupta at intel.com>
---
drivers/gpu/drm/xe/xe_hwmon.c | 60 ++++++++++++++++++++---------------
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_hwmon.c b/drivers/gpu/drm/xe/xe_hwmon.c
index 7e8caac838e0..8c805ace592c 100644
--- a/drivers/gpu/drm/xe/xe_hwmon.c
+++ b/drivers/gpu/drm/xe/xe_hwmon.c
@@ -79,46 +79,48 @@ struct xe_hwmon {
struct xe_hwmon_energy_info ei[CHANNEL_MAX];
};
-static u32 xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg hwmon_reg, int channel)
+static bool xe_hwmon_get_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg hwmon_reg,
+ int channel, struct xe_reg *reg)
{
struct xe_device *xe = gt_to_xe(hwmon->gt);
- struct xe_reg reg = XE_REG(0);
+
+ *reg = XE_REG(0);
switch (hwmon_reg) {
case REG_PKG_RAPL_LIMIT:
if (xe->info.platform == XE_PVC && channel == CHANNEL_PKG)
- reg = PVC_GT0_PACKAGE_RAPL_LIMIT;
+ *reg = PVC_GT0_PACKAGE_RAPL_LIMIT;
else if ((xe->info.platform == XE_DG2) && (channel == CHANNEL_PKG))
- reg = PCU_CR_PACKAGE_RAPL_LIMIT;
+ *reg = PCU_CR_PACKAGE_RAPL_LIMIT;
break;
case REG_PKG_POWER_SKU:
if (xe->info.platform == XE_PVC && channel == CHANNEL_PKG)
- reg = PVC_GT0_PACKAGE_POWER_SKU;
+ *reg = PVC_GT0_PACKAGE_POWER_SKU;
else if ((xe->info.platform == XE_DG2) && (channel == CHANNEL_PKG))
- reg = PCU_CR_PACKAGE_POWER_SKU;
+ *reg = PCU_CR_PACKAGE_POWER_SKU;
break;
case REG_PKG_POWER_SKU_UNIT:
if (xe->info.platform == XE_PVC)
- reg = PVC_GT0_PACKAGE_POWER_SKU_UNIT;
+ *reg = PVC_GT0_PACKAGE_POWER_SKU_UNIT;
else if (xe->info.platform == XE_DG2)
- reg = PCU_CR_PACKAGE_POWER_SKU_UNIT;
+ *reg = PCU_CR_PACKAGE_POWER_SKU_UNIT;
break;
case REG_GT_PERF_STATUS:
if (xe->info.platform == XE_DG2 && channel == CHANNEL_PKG)
- reg = GT_PERF_STATUS;
+ *reg = GT_PERF_STATUS;
break;
case REG_PKG_ENERGY_STATUS:
if (xe->info.platform == XE_PVC && channel == CHANNEL_PKG)
- reg = PVC_GT0_PLATFORM_ENERGY_STATUS;
+ *reg = PVC_GT0_PLATFORM_ENERGY_STATUS;
else if ((xe->info.platform == XE_DG2) && (channel == CHANNEL_PKG))
- reg = PCU_CR_PACKAGE_ENERGY_STATUS;
+ *reg = PCU_CR_PACKAGE_ENERGY_STATUS;
break;
default:
drm_warn(&xe->drm, "Unknown xe hwmon reg id: %d\n", hwmon_reg);
- break;
+ return false;
}
- return reg.raw;
+ return true;
}
static void xe_hwmon_process_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg hwmon_reg,
@@ -127,10 +129,10 @@ static void xe_hwmon_process_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg hwmon
{
struct xe_reg reg;
- reg.raw = xe_hwmon_get_reg(hwmon, hwmon_reg, channel);
-
- if (!reg.raw)
+ if (!xe_hwmon_get_reg(hwmon, hwmon_reg, channel, ®)) {
+ *value = 0;
return;
+ }
switch (operation) {
case REG_READ32:
@@ -143,6 +145,7 @@ static void xe_hwmon_process_reg(struct xe_hwmon *hwmon, enum xe_hwmon_reg hwmon
*value = xe_mmio_read64_2x32(hwmon->gt, reg);
break;
default:
+ *value = 0;
drm_warn(>_to_xe(hwmon->gt)->drm, "Invalid xe hwmon reg operation: %d\n",
operation);
break;
@@ -298,7 +301,7 @@ xe_hwmon_power_max_interval_show(struct device *dev, struct device_attribute *at
* As y can be < 2, we compute tau4 = (4 | x) << y
* and then add 2 when doing the final right shift to account for units
*/
- tau4 = ((1 << x_w) | x) << y;
+ tau4 = (u64)((1 << x_w) | x) << y;
/* val in hwmon interface units (millisec) */
out = mul_u64_u32_shr(tau4, SF_TIME, hwmon->scl_shift_time + x_w);
@@ -339,7 +342,7 @@ xe_hwmon_power_max_interval_store(struct device *dev, struct device_attribute *a
r = FIELD_PREP(PKG_MAX_WIN, PKG_MAX_WIN_DEFAULT);
x = REG_FIELD_GET(PKG_MAX_WIN_X, r);
y = REG_FIELD_GET(PKG_MAX_WIN_Y, r);
- tau4 = ((1 << x_w) | x) << y;
+ tau4 = (u64)((1 << x_w) | x) << y;
max_win = mul_u64_u32_shr(tau4, SF_TIME, hwmon->scl_shift_time + x_w);
if (val > max_win)
@@ -397,10 +400,11 @@ static umode_t xe_hwmon_attributes_visible(struct kobject *kobj,
struct device *dev = kobj_to_dev(kobj);
struct xe_hwmon *hwmon = dev_get_drvdata(dev);
int ret = 0;
+ struct xe_reg reg;
xe_pm_runtime_get(gt_to_xe(hwmon->gt));
- ret = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, index) ? attr->mode : 0;
+ ret = xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, index, ®) ? attr->mode : 0;
xe_pm_runtime_put(gt_to_xe(hwmon->gt));
@@ -493,19 +497,20 @@ static umode_t
xe_hwmon_power_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
{
u32 uval;
+ struct xe_reg reg;
switch (attr) {
case hwmon_power_max:
- return xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel) ? 0664 : 0;
+ return xe_hwmon_get_reg(hwmon, REG_PKG_RAPL_LIMIT, channel, ®) ? 0664 : 0;
case hwmon_power_rated_max:
- return xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU, channel) ? 0444 : 0;
+ return xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU, channel, ®) ? 0444 : 0;
case hwmon_power_crit:
if (channel == CHANNEL_PKG)
return (xe_hwmon_pcode_read_i1(hwmon->gt, &uval) ||
!(uval & POWER_SETUP_I1_WATTS)) ? 0 : 0644;
break;
case hwmon_power_label:
- return xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU_UNIT, channel) ? 0444 : 0;
+ return xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU_UNIT, channel, ®) ? 0444 : 0;
default:
return 0;
}
@@ -585,10 +590,12 @@ xe_hwmon_curr_write(struct xe_hwmon *hwmon, u32 attr, int channel, long val)
static umode_t
xe_hwmon_in_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
{
+ struct xe_reg reg;
+
switch (attr) {
case hwmon_in_input:
case hwmon_in_label:
- return xe_hwmon_get_reg(hwmon, REG_GT_PERF_STATUS, channel) ? 0444 : 0;
+ return xe_hwmon_get_reg(hwmon, REG_GT_PERF_STATUS, channel, ®) ? 0444 : 0;
default:
return 0;
}
@@ -609,10 +616,12 @@ xe_hwmon_in_read(struct xe_hwmon *hwmon, u32 attr, int channel, long *val)
static umode_t
xe_hwmon_energy_is_visible(struct xe_hwmon *hwmon, u32 attr, int channel)
{
+ struct xe_reg reg;
+
switch (attr) {
case hwmon_energy_input:
case hwmon_energy_label:
- return xe_hwmon_get_reg(hwmon, REG_PKG_ENERGY_STATUS, channel) ? 0444 : 0;
+ return xe_hwmon_get_reg(hwmon, REG_PKG_ENERGY_STATUS, channel, ®) ? 0444 : 0;
default:
return 0;
}
@@ -758,12 +767,13 @@ xe_hwmon_get_preregistration_info(struct xe_device *xe)
long energy;
u64 val_sku_unit = 0;
int channel;
+ struct xe_reg reg;
/*
* The contents of register PKG_POWER_SKU_UNIT do not change,
* so read it once and store the shift values.
*/
- if (xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU_UNIT, 0)) {
+ if (xe_hwmon_get_reg(hwmon, REG_PKG_POWER_SKU_UNIT, 0, ®)) {
xe_hwmon_process_reg(hwmon, REG_PKG_POWER_SKU_UNIT,
REG_READ32, &val_sku_unit, 0, 0, 0);
hwmon->scl_shift_power = REG_FIELD_GET(PKG_PWR_UNIT, val_sku_unit);
--
2.25.1
More information about the Intel-xe
mailing list