Skip to content

Commit

Permalink
hwmon: (k10temp) Check return value of hygon_read_temp()
Browse files Browse the repository at this point in the history
Check the return value of amd_smn_read() before saving a value. This
ensures invalid values aren't saved or used.

There are three cases here with slightly different behavior.

1) read_tempreg_nb_zen():
	This is a function pointer which does not include a return code.
	In this case, set the register value to 0 on failure. This
	enforces Read-as-Zero behavior.

2) k10temp_read_temp():
	This function does have return codes, so return the error code
	from the failed register read. Continued operation is not
	necessary, since there is no valid data from the register.
	Furthermore, if the register value was set to 0, then the
	following operation would underflow.

3) k10temp_get_ccd_support():
	This function reads the same register from multiple CCD
	instances in a loop. And a bitmask is formed if a specific bit
	is set in each register instance. The loop should continue on a
	failed register read, skipping the bit check.

WangYuli:
  When check amd_smn_read(), don't forget Hygon.
  Thanks for code review from Hygon.

Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.11-rc6&id=c2d79cc5455c891de6c93e1e0c73d806e299c54f
Link: d693d4a
Signed-off-by: WangYuli <[email protected]>
  • Loading branch information
Avenger-285714 committed Sep 11, 2024
1 parent 628b05f commit e11dfa1
Showing 1 changed file with 27 additions and 14 deletions.
41 changes: 27 additions & 14 deletions drivers/hwmon/k10temp.c
Original file line number Diff line number Diff line change
Expand Up @@ -208,21 +208,24 @@ static int k10temp_read_labels(struct device *dev,
return 0;
}

static void hygon_read_temp(struct k10temp_data *data, int channel,
static int hygon_read_temp(struct k10temp_data *data, int channel,
u32 *regval)
{
struct hygon_private *h_priv;
int ret = -EOPNOTSUPP;

h_priv = (struct hygon_private *)data->priv;
if ((channel - 2) < h_priv->index_2nd)
amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
regval);
ret = amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
regval);
else
amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
ZEN_CCD_TEMP(h_priv->offset_2nd,
channel - 2 - h_priv->index_2nd),
regval);
ret = amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
ZEN_CCD_TEMP(h_priv->offset_2nd,
channel - 2 - h_priv->index_2nd),
regval);

return ret;
}

static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
Expand All @@ -247,15 +250,14 @@ static int k10temp_read_temp(struct device *dev, u32 attr, int channel,
break;
case 2 ... 13: /* Tccd{1-12} */
if (hygon_f18h_m4h())
hygon_read_temp(data, channel, &regval);
ret = hygon_read_temp(data, channel, &regval);
else {
ret = amd_smn_read(amd_pci_dev_to_node_id(data->pdev),
ZEN_CCD_TEMP(data->ccd_offset, channel - 2),
&regval);

if (ret)
return ret;
}
if (ret)
return ret;

*val = (regval & ZEN_CCD_TEMP_MASK) * 125 - 49000;
break;
Expand Down Expand Up @@ -442,10 +444,21 @@ static void k10temp_get_ccd_support_2nd(struct pci_dev *pdev,

h_priv = (struct hygon_private *)data->priv;
for (i = h_priv->index_2nd; i < limit; i++) {
amd_smn_read(amd_pci_dev_to_node_id(pdev),
/*
* Ignore inaccessible CCDs.
*
* Some systems will return a register value of 0, and the TEMP_VALID
* bit check below will naturally fail.
*
* Other systems will return a PCI_ERROR_RESPONSE (0xFFFFFFFF) for
* the register value. And this will incorrectly pass the TEMP_VALID
* bit check.
*/
if (amd_smn_read(amd_pci_dev_to_node_id(pdev),
ZEN_CCD_TEMP(h_priv->offset_2nd,
i - h_priv->index_2nd),
&regval);
&regval))
continue;
if (regval & ZEN_CCD_TEMP_VALID)
data->show_temp |= BIT(TCCD_BIT(i));
}
Expand Down

0 comments on commit e11dfa1

Please sign in to comment.