Skip to contentSkip to conversation

x86/amd_nb: Add PCI device IDs for family 17h, model 71h

PATCH Superseded Marcel Bocu 6 participants 19 messages Jul 18, 2019 – Jul 22, 2019 1 message not in the archive

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 x86/amd_nb: Add PCI device IDs for family 17h, model 71h

The AMD Ryzen gen 3 processors came with a different PCI IDs for the function 3 & 4 which are used to access the SMN interface. The root PCI address however remained at the same address as the model 30h.

Adding the F3/F4 PCI IDs respectively to the misc and link ids appear to be sufficient for k10temp, so let's add them and follow up on the patch if other functions need more tweaking.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc "Woods, Brian" <[email protected]>
Cc Clemens Ladisch <[email protected]>
Cc Jean Delvare <[email protected]>
Cc Guenter Roeck <[email protected]>
---
 arch/x86/kernel/amd_nb.c | 3 +++
 include/linux/pci_ids.h  | 1 +
 2 files changed, 4 insertions(+)
arch/x86/kernel/amd_nb.c +3 -0
index d63e63b7d1d9..297dc50da3bd 100644
@@ -21,6 +21,7 @@
 #define PCI_DEVICE_ID_AMD_17H_DF_F4	0x1464
 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec
 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494
+#define PCI_DEVICE_ID_AMD_17H_M71H_DF_F4 0x1444
 
 /* Protect the PCI config register pairs used for SMN and DF indirect access. */
 static DEFINE_MUTEX(smn_mutex);
@@ -50,6 +51,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M71H_DF_F3) },
 	{}
 };
 EXPORT_SYMBOL_GPL(amd_nb_misc_ids);
@@ -63,6 +65,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F4) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M71H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
 	{}
 };
include/linux/pci_ids.h +1 -0
index c842735a4f45..5f99cfac9c06 100644
@@ -548,6 +548,7 @@
 #define PCI_DEVICE_ID_AMD_17H_DF_F3	0x1463
 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F3 0x15eb
 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493
+#define PCI_DEVICE_ID_AMD_17H_M71H_DF_F3 0x1443
 #define PCI_DEVICE_ID_AMD_CNB17H_F3	0x1703
 #define PCI_DEVICE_ID_AMD_LANCE		0x2000
 #define PCI_DEVICE_ID_AMD_LANCE_HOME	0x2001
Signature · 1 line
2.22.0
#1 lore raw

PATCH 2/2 hwmon/k10temp: Add support for AMD family 17h, model 71h CPUs

It would seem like model 71h is behaving in the same way as model 30h, so let's just add the new F3 PCI ID to the list of compatible devices.

Unlike previous Ryzen/Threadripper, Ryzen gen 3 processors do not need temperature offsets anymore. This has been reported in the press and verified on my Ryzen 3700X by checking that the idle temperature reported by k10temp is matching the temperature reported by the firmware.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc "Woods, Brian" <[email protected]>
Cc Clemens Ladisch <[email protected]>
Cc Jean Delvare <[email protected]>
Cc Guenter Roeck <[email protected]>
---
 drivers/hwmon/k10temp.c | 1 +
 1 file changed, 1 insertion(+)
drivers/hwmon/k10temp.c +1 -0
index c77e89239dcd..a9c1108b377e 100644
@@ -349,6 +349,7 @@ static const struct pci_device_id k10temp_id_table[] = {
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) },
+	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M71H_DF_F3) },
 	{ PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) },
 	{}
 };
Signature · 1 line
2.22.0
#2 lore raw

PATCH v2 2/2 hwmon/k10temp: Add support for AMD family 17h, model 70h CPUs

It would seem like model 70h is behaving in the same way as model 30h, so let's just add the new F3 PCI ID to the list of compatible devices.

Unlike previous Ryzen/Threadripper, Ryzen gen 3 processors do not need temperature offsets anymore. This has been reported in the press and verified on my Ryzen 3700X by checking that the idle temperature reported by k10temp is matching the temperature reported by the firmware.

Vicki Pfau sent an identical patch after I checked that no-one had written this patch. I would have been happy about dropping my patch but unlike for his patch series, I had already Cc:ed the x86 people and they already reviewed the changes. Since Vicki has not answered to any email after his initial series, let's assume he is on vacation and let's avoid duplication of reviews from the maintainers and merge my series. To acknowledge Vicki's anteriority, I added his S-o-b to the patch.

v2, suggested by Guenter Roeck and Brian Woods:
- rename from 71h to 70h

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc "Woods, Brian" <[email protected]>
Cc Clemens Ladisch <[email protected]>
Cc Jean Delvare <[email protected]>
Cc Guenter Roeck <[email protected]>
---
 drivers/hwmon/k10temp.c | 1 +
 1 file changed, 1 insertion(+)
drivers/hwmon/k10temp.c +1 -0
index c77e89239dcd..5c1dddde193c 100644
@@ -349,6 +349,7 @@ static const struct pci_device_id k10temp_id_table[] = {
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) },
 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) },
+	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
 	{ PCI_VDEVICE(HYGON, PCI_DEVICE_ID_AMD_17H_DF_F3) },
 	{}
 };
Signature · 1 line
2.22.0
maintainer Applied
#3 lore raw

Re: [PATCH v2 2/2] hwmon/k10temp: Add support for AMD family 17h, model 70h CPUs

Quoted 18 lines · Marcel Bocu

It would seem like model 70h is behaving in the same way as model 30h, so let's just add the new F3 PCI ID to the list of compatible devices.

Unlike previous Ryzen/Threadripper, Ryzen gen 3 processors do not need temperature offsets anymore. This has been reported in the press and verified on my Ryzen 3700X by checking that the idle temperature reported by k10temp is matching the temperature reported by the firmware.

Vicki Pfau sent an identical patch after I checked that no-one had written this patch. I would have been happy about dropping my patch but unlike for his patch series, I had already Cc:ed the x86 people and they already reviewed the changes. Since Vicki has not answered to any email after his initial series, let's assume he is on vacation and let's avoid duplication of reviews from the maintainers and merge my series. To acknowledge Vicki's anteriority, I added his S-o-b to the patch.

Quoted 7 lines

v2, suggested by Guenter Roeck and Brian Woods:
- rename from 71h to 70h

Applied to hwmon-next.

Note that I made a small change to the description of each patch:

s/ he / she /
s/his/her/

Guenter

#4 lore raw

Re: [PATCH v2 2/2] hwmon/k10temp: Add support for AMD family 17h, model 70h CPUs

Quoted 26 lines · Guenter Roeck
Quoted 18 lines · Marcel Bocu

It would seem like model 70h is behaving in the same way as model 30h, so let's just add the new F3 PCI ID to the list of compatible devices.

Unlike previous Ryzen/Threadripper, Ryzen gen 3 processors do not need temperature offsets anymore. This has been reported in the press and verified on my Ryzen 3700X by checking that the idle temperature reported by k10temp is matching the temperature reported by the firmware.

Vicki Pfau sent an identical patch after I checked that no-one had written this patch. I would have been happy about dropping my patch but unlike for his patch series, I had already Cc:ed the x86 people and they already reviewed the changes. Since Vicki has not answered to any email after his initial series, let's assume he is on vacation and let's avoid duplication of reviews from the maintainers and merge my series. To acknowledge Vicki's anteriority, I added his S-o-b to the patch.

Quoted 7 lines

v2, suggested by Guenter Roeck and Brian Woods:
- rename from 71h to 70h

Quoted 6 lines

Applied to hwmon-next.

Note that I made a small change to the description of each patch:

s/ he / she /
s/his/her/

Thanks! I'll go find my brown bag and repeat 100 times "don't assume gender!".

Now, let's see if we can find more sensors to expose :)

Marcel

Quoted 3 lines

Guenter

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 9 lines · Guenter Roeck
Quoted 8 lines · Marcel Bocu

The AMD Ryzen gen 3 processors came with a different PCI IDs for the function 3 & 4 which are used to access the SMN interface. The root PCI address however remained at the same address as the model 30h.

Adding the F3/F4 PCI IDs respectively to the misc and link ids appear to be sufficient for k10temp, so let's add them and follow up on the patch if other functions need more tweaking.

Quoted 5 lines
Quoted 3 lines

How is this version of the patch series different to the first version ?

They seem pretty much identical except for the macro's name (71h vs 70h) and the fact that I already Cc:ed the x86 crew. I had checked this weekend if there were patches already for this, but I guess Vicki sent his patches right after I checked. Sorry for the noise!

Could anyone add Vicki Pfau (I don't have his email address because I subscribed to this list after he sent his patches), and then we can ask him if he already submited his patches to x86 or not.

In any case, whatever patchset gets selected for inclusion, I suggest we both sign off on the commit (I do not care about authorship). I will anyway try to follow up with other patches to access the chipset's temperature and the fan speed.

Sorry again for the noise!
Marcel

maintainer
#7 lore raw

Hi Marcel,

Quoted 10 lines · Marcel Bocu
Quoted 9 lines · Guenter Roeck
Quoted 8 lines · Marcel Bocu

The AMD Ryzen gen 3 processors came with a different PCI IDs for the function 3 & 4 which are used to access the SMN interface. The root PCI address however remained at the same address as the model 30h.

Adding the F3/F4 PCI IDs respectively to the misc and link ids appear to be sufficient for k10temp, so let's add them and follow up on the patch if other functions need more tweaking.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

How is this version of the patch series different to the first version ?

They seem pretty much identical except for the macro's name (71h vs 70h)

Normally (or at least so far) the device ID specifications are for a group of models, not just for one chip in the group. Traditionally, AMD uses the same PCI ID for model numbers 7xh, so 70h would probably be more appropriate. Of course, that is hard to say without documentation from AMD, and I don't see anything published for Ryzen 3000.

Quoted 4 lines

and the fact that I already Cc:ed the x86 crew. I had checked this weekend if there were patches already for this, but I guess Vicki sent his patches right after I checked. Sorry for the noise!

Quoted 9 lines

Could anyone add Vicki Pfau (I don't have his email address because I subscribed to this list after he sent his patches), and then we can ask him if he already submited his patches to x86 or not.

In any case, whatever patchset gets selected for inclusion, I suggest we both sign off on the commit (I do not care about authorship). I will anyway try to follow up with other patches to access the chipset's temperature and the fan speed.

Wouldn't that require patches to the Super-IO chip on your board ? That seems orthogonal to this patch series (and to the R3000 CPU).

Quoted 1 line

Sorry again for the noise!

Sorry myself, I didn't realize that the patches were from different people since they looked similar.

Guenter

#8 lore raw
Quoted 19 lines · Guenter Roeck

Hi Marcel,

Quoted 16 lines · Marcel Bocu
Quoted 14 lines · Guenter Roeck
Quoted 11 lines · Marcel Bocu

The AMD Ryzen gen 3 processors came with a different PCI IDs for the function 3 & 4 which are used to access the SMN interface. The root PCI address however remained at the same address as the model 30h.

Adding the F3/F4 PCI IDs respectively to the misc and link ids appear to be sufficient for k10temp, so let's add them and follow up on the patch if other functions need more tweaking.

How is this version of the patch series different to the first version ?

Quoted 7 lines
Quoted 1 line

They seem pretty much identical except for the macro's name (71h vs 70h)

Normally (or at least so far) the device ID specifications are for a group of models, not just for one chip in the group. Traditionally, AMD uses the same PCI ID for model numbers 7xh, so 70h would probably be more appropriate. Of course, that is hard to say without documentation from AMD, and I don't see anything published for Ryzen 3000.

Makes sense. Thanks for giving me some historical background!

Quoted 9 lines
Quoted 9 lines

and the fact that I already Cc:ed the x86 crew. I had checked this weekend if there were patches already for this, but I guess Vicki sent his patches right after I checked. Sorry for the noise!

Could anyone add Vicki Pfau (I don't have his email address because I subscribed to this list after he sent his patches), and then we can ask him if he already submited his patches to x86 or not.

Quoted 8 lines
Quoted 5 lines

In any case, whatever patchset gets selected for inclusion, I suggest we both sign off on the commit (I do not care about authorship). I will anyway try to follow up with other patches to access the chipset's temperature and the fan speed.

Wouldn't that require patches to the Super-IO chip on your board ? That seems orthogonal to this patch series (and to the R3000 CPU).

Yes, but what I meant is that I don't mind not having authorship on these patches since most of my work will be on adding new features. Sorry for not being clear.

Quoted 5 lines
Quoted 2 lines

Sorry again for the noise!

Sorry myself, I didn't realize that the patches were from different people since they looked similar.

I can't blame you for that! Could anyone in this thread add Vicki Pfau to figure out what's the best course of action?

We could merge his patches (if he contacted the x86 crew), make a v2 of mine (71h -> 70h, add his Signed-off-by?) and merge, or some other option?

Marcel

Quoted 3 lines

Guenter

maintainer
#9 lore raw
Quoted 53 lines · Marcel Bocu
Quoted 19 lines · Guenter Roeck

Hi Marcel,

Quoted 16 lines · Marcel Bocu
Quoted 14 lines · Guenter Roeck
Quoted 11 lines · Marcel Bocu

The AMD Ryzen gen 3 processors came with a different PCI IDs for the function 3 & 4 which are used to access the SMN interface. The root PCI address however remained at the same address as the model 30h.

Adding the F3/F4 PCI IDs respectively to the misc and link ids appear to be sufficient for k10temp, so let's add them and follow up on the patch if other functions need more tweaking.

How is this version of the patch series different to the first version ?

Quoted 7 lines
Quoted 1 line

They seem pretty much identical except for the macro's name (71h vs 70h)

Normally (or at least so far) the device ID specifications are for a group of models, not just for one chip in the group. Traditionally, AMD uses the same PCI ID for model numbers 7xh, so 70h would probably be more appropriate. Of course, that is hard to say without documentation from AMD, and I don't see anything published for Ryzen 3000.

Makes sense. Thanks for giving me some historical background!

Quoted 9 lines
Quoted 9 lines

and the fact that I already Cc:ed the x86 crew. I had checked this weekend if there were patches already for this, but I guess Vicki sent his patches right after I checked. Sorry for the noise!

Could anyone add Vicki Pfau (I don't have his email address because I subscribed to this list after he sent his patches), and then we can ask him if he already submited his patches to x86 or not.

Quoted 8 lines
Quoted 5 lines

In any case, whatever patchset gets selected for inclusion, I suggest we both sign off on the commit (I do not care about authorship). I will anyway try to follow up with other patches to access the chipset's temperature and the fan speed.

Wouldn't that require patches to the Super-IO chip on your board ? That seems orthogonal to this patch series (and to the R3000 CPU).

Yes, but what I meant is that I don't mind not having authorship on these patches since most of my work will be on adding new features. Sorry for not being clear.

Quoted 1 line
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Sorry again for the noise!

Sorry myself, I didn't realize that the patches were from different people since they looked similar.

I can't blame you for that! Could anyone in this thread add Vicki Pfau to figure out what's the best course of action?

Copied. Can the two of you sort this out ?

Note that Vicki's patch series is in patchwork:

	https://patchwork.kernel.org/patch/11043277/
	https://patchwork.kernel.org/patch/11043271/
Quoted 3 lines

We could merge his patches (if he contacted the x86 crew), make a v2 of mine (71h -> 70h, add his Signed-off-by?) and merge, or some other option?

Either case, we'll need feedback from x86 maintainers. They are not exactly happy if anyone pushes a patch into arch/x86 without their approval.

Guenter

Quoted 6 lines

Marcel

Quoted 3 lines

Guenter

#10 lore raw
Quoted 5 lines · Guenter Roeck
Quoted 3 lines

We could merge his patches (if he contacted the x86 crew), make a v2 of mine (71h -> 70h, add his Signed-off-by?) and merge, or some other option?

Either case, we'll need feedback from x86 maintainers. They are not exactly happy if anyone pushes a patch into arch/x86 without their approval.

Adding those PCI ids looks straight forward. So feel free to route them through hwmon with:

maintainer
#11 lore raw
Quoted 9 lines · Thomas Gleixner
Quoted 5 lines · Guenter Roeck
Quoted 3 lines

We could merge his patches (if he contacted the x86 crew), make a v2 of mine (71h -> 70h, add his Signed-off-by?) and merge, or some other option?

Either case, we'll need feedback from x86 maintainers. They are not exactly happy if anyone pushes a patch into arch/x86 without their approval.

Adding those PCI ids looks straight forward. So feel free to route them through hwmon with:

... but before you do that, let's wait for Brian to confirm the proper model range.

Thx.

Signature · 5 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
Acked-by
#12 lore raw
Quoted 8 lines · Borislav Petkov
Quoted 7 lines · Thomas Gleixner
Quoted 5 lines · Guenter Roeck
Quoted 3 lines

We could merge his patches (if he contacted the x86 crew), make a v2 of mine (71h -> 70h, add his Signed-off-by?) and merge, or some other option?

Either case, we'll need feedback from x86 maintainers. They are not exactly happy if anyone pushes a patch into arch/x86 without their approval.

Quoted 14 lines
Quoted 2 lines

Adding those PCI ids looks straight forward. So feel free to route them through hwmon with:

... but before you do that, let's wait for Brian to confirm the proper model range.

Thx.

Signature · 5 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Generally we use X0-XF for a model of processors. Starting at 70h would be preferred.

The PCI devices match our internal documentation for what it's worth. I was going to test these patches but the system I borrowed was requisitioned and hasn't been returned yet so I can't test them now, but I don't want to delay these anymore.

Assuming the change from 71h -> 70h, feel free to add:

Signature · 1 line
Brian Woods
maintainer
#13 lore raw
Quoted 24 lines · Brian
Quoted 8 lines · Borislav Petkov
Quoted 7 lines · Thomas Gleixner
Quoted 5 lines · Guenter Roeck
Quoted 3 lines

We could merge his patches (if he contacted the x86 crew), make a v2 of mine (71h -> 70h, add his Signed-off-by?) and merge, or some other option?

Either case, we'll need feedback from x86 maintainers. They are not exactly happy if anyone pushes a patch into arch/x86 without their approval.

Quoted 14 lines
Quoted 2 lines

Adding those PCI ids looks straight forward. So feel free to route them through hwmon with:

... but before you do that, let's wait for Brian to confirm the proper model range.

Thx.

Signature · 5 lines
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
Quoted 7 lines

Generally we use X0-XF for a model of processors. Starting at 70h would be preferred.

The PCI devices match our internal documentation for what it's worth. I was going to test these patches but the system I borrowed was requisitioned and hasn't been returned yet so I can't test them now, but I don't want to delay these anymore.

With 3900X, and the series applied, I get:

k10temp-pci-00c3

Adapter: PCI adapter
Tdie:         +45.1°C  (high = +70.0°C)
Tctl:         +45.1°C  

which looks about right.

Interesting, though. I thought there are two CPU dies on this chip. I guess the temperature sensor is on the IO block ? If so, are there additional sensors on the CPU dies ?

Guenter

Quoted 6 lines

Assuming the change from 71h -> 70h, feel free to add:

Signature · 1 line
Brian Woods
#14 lore raw
Quoted 8 lines · Guenter Roeck

With 3900X, and the series applied, I get:

k10temp-pci-00c3

Adapter: PCI adapter
Tdie:         +45.1°C  (high = +70.0°C)
Tctl:         +45.1°C  
Quoted 8 lines

which looks about right.

Interesting, though. I thought there are two CPU dies on this chip. I guess the temperature sensor is on the IO block ? If so, are there additional sensors on the CPU dies ?

Guenter

That's good to know. Thanks for testing it.

What happens is the IOD takes the max temperature of the chiplets in the package and presents that as the temperature of the package. It works the same way as it does in Rome (server parts). For better or worse, you just get the max temperature of the chiplets rather than the temperatures of the individual chiplets.

Signature · 1 line
Brian Woods
#15 lore raw
Quoted 18 lines · Brian
Quoted 8 lines · Guenter Roeck

With 3900X, and the series applied, I get:

k10temp-pci-00c3

Adapter: PCI adapter
Tdie:         +45.1°C  (high = +70.0°C)
Tctl:         +45.1°C  
Quoted 8 lines

which looks about right.

Interesting, though. I thought there are two CPU dies on this chip. I guess the temperature sensor is on the IO block ? If so, are there additional sensors on the CPU dies ?

Guenter

Quoted 8 lines

That's good to know. Thanks for testing it.

What happens is the IOD takes the max temperature of the chiplets in the package and presents that as the temperature of the package. It works the same way as it does in Rome (server parts). For better or worse, you just get the max temperature of the chiplets rather than the temperatures of the individual chiplets.

Out of curiosity, is it the maximum temperature of all chiplets, or just the non-powergated ones? Because this might explain why I get so much variance in the idle temperature (40 <-> 50°C in a matter of a second with a mostly-idle processor). This variance is visible on linux, but not at all on the firmware's configuration interface.

One other option would be the stock fan not being tight-enough... but apparently quite a few people have the issue. I'll try tightening it!

Marcel

#16 lore raw

I'm getting similar variance. Compiling Linux seems to spike the temperature above 70, even with a new CPU cooler, so I'm wondering if there might be an offset I'm missing. It may just be the fan being too slow (going to be reconfiguring the BIOS settings today).

The reason I haven't replied sooner is because I've been busy with a new job, and I wasn't sure if my phone would send a properly (un)formatted email. I'm fine with whosever patch gets in, so long as temperature reading works on my machine.

Also, for what it's worth, I'm not a "he". Vicki is generally a female name.

Vicki

Quoted 38 lines
Quoted 20 lines · Marcel Bocu
Quoted 11 lines · Brian

On Mon, Jul 22, 2019 at 09:51:05AM -0700, Guenter Roeck wrote:

With 3900X, and the series applied, I get:

k10temp-pci-00c3

Adapter: PCI adapter
Tdie:         +45.1°C  (high = +70.0°C)
Tctl:         +45.1°C  
Quoted 7 lines

which looks about right.

Interesting, though. I thought there are two CPU dies on this chip. I guess the temperature sensor is on the IO block ? If so, are there additional sensors on the CPU dies ?

Guenter

Quoted 7 lines

That's good to know. Thanks for testing it.

What happens is the IOD takes the max temperature of the chiplets in the package and presents that as the temperature of the package. It works the same way as it does in Rome (server parts). For better or worse, you just get the max temperature of the chiplets rather than the temperatures of the individual chiplets.

Out of curiosity, is it the maximum temperature of all chiplets, or just the non-powergated ones? Because this might explain why I get so much variance in the idle temperature (40 <-> 50°C in a matter of a second with a mostly-idle processor). This variance is visible on linux, but not at all on the firmware's configuration interface.

One other option would be the stock fan not being tight-enough... but apparently quite a few people have the issue. I'll try tightening it!

Marcel

#17 lore raw
Quoted 1 line · Vicki Pfau

I'm getting similar variance. Compiling Linux seems to spike the temperature above 70, even with a new CPU cooler, so I'm wondering if there might be an offset I'm missing. It may just be the fan being too slow (going to be reconfiguring the BIOS settings today).

Thanks for the information! Compiling the kernel gets me to 83°C with the stock fan (Ryzen 3700X), and I think I get thermally throttled at this point.

Quoted 2 lines

The reason I haven't replied sooner is because I've been busy with a new job, and I wasn't sure if my phone would send a properly (un)formatted email.

I see! No worries, and good luck with your new job!

Quoted 1 line

I'm fine with whosever patch gets in, so long as temperature reading works on my machine.

Given that our patches were looking exactly the same, it's safe to assume it will be fine.

Quoted 2 lines

Also, for what it's worth, I'm not a "he". Vicki is generally a female name.

I did not know this. Sorry for assuming your gender, and thanks for correcting me!

Marcel

Quoted 42 lines

Vicki

Quoted 38 lines
Quoted 20 lines · Marcel Bocu
Quoted 11 lines · Brian

On Mon, Jul 22, 2019 at 09:51:05AM -0700, Guenter Roeck wrote:

With 3900X, and the series applied, I get:

k10temp-pci-00c3

Adapter: PCI adapter
Tdie:         +45.1°C  (high = +70.0°C)
Tctl:         +45.1°C  
Quoted 7 lines

which looks about right.

Interesting, though. I thought there are two CPU dies on this chip. I guess the temperature sensor is on the IO block ? If so, are there additional sensors on the CPU dies ?

Guenter

Quoted 7 lines

That's good to know. Thanks for testing it.

What happens is the IOD takes the max temperature of the chiplets in the package and presents that as the temperature of the package. It works the same way as it does in Rome (server parts). For better or worse, you just get the max temperature of the chiplets rather than the temperatures of the individual chiplets.

Out of curiosity, is it the maximum temperature of all chiplets, or just the non-powergated ones? Because this might explain why I get so much variance in the idle temperature (40 <-> 50°C in a matter of a second with a mostly-idle processor). This variance is visible on linux, but not at all on the firmware's configuration interface.

One other option would be the stock fan not being tight-enough... but apparently quite a few people have the issue. I'll try tightening it!

Marcel

maintainer
#18 lore raw
Quoted 7 lines · Marcel Bocu
Quoted 1 line · Vicki Pfau

I'm getting similar variance. Compiling Linux seems to spike the temperature above 70, even with a new CPU cooler, so I'm wondering if there might be an offset I'm missing. It may just be the fan being too slow (going to be reconfiguring the BIOS settings today).

Thanks for the information! Compiling the kernel gets me to 83°C with the stock fan (Ryzen 3700X), and I think I get thermally throttled at this point.

Same here, with 3900X. The cores run at ~4GHz each, with stock cooler. Idle temperature seems to be around 35C.

Guenter

#19 lore raw

PATCH v2 1/2 x86/amd_nb: Add PCI device IDs for family 17h, model 70h

In reply to Marcel Bocu

The AMD Ryzen gen 3 processors came with a different PCI IDs for the function 3 & 4 which are used to access the SMN interface. The root PCI address however remained at the same address as the model 30h.

Adding the F3/F4 PCI IDs respectively to the misc and link ids appear to be sufficient for k10temp, so let's add them and follow up on the patch if other functions need more tweaking.

Vicki Pfau sent an identical patch after I checked that no-one had written this patch. I would have been happy about dropping my patch but unlike for his patch series, I had already Cc:ed the x86 people and they already reviewed the changes. Since Vicki has not answered to any email after his initial series, let's assume he is on vacation and let's avoid duplication of reviews from the maintainers and merge my series. To acknowledge Vicki's anteriority, I added his S-o-b to the patch.

v2, suggested by Guenter Roeck and Brian Woods:
- rename from 71h to 70h

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc "Woods, Brian" <[email protected]>
Cc Clemens Ladisch <[email protected]>
Cc Jean Delvare <[email protected]>
Cc Guenter Roeck <[email protected]>
---
 arch/x86/kernel/amd_nb.c | 3 +++
 include/linux/pci_ids.h  | 1 +
 2 files changed, 4 insertions(+)
arch/x86/kernel/amd_nb.c +3 -0
index d63e63b7d1d9..251c795b4eb3 100644
@@ -21,6 +21,7 @@
 #define PCI_DEVICE_ID_AMD_17H_DF_F4	0x1464
 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F4 0x15ec
 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F4 0x1494
+#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F4 0x1444
 
 /* Protect the PCI config register pairs used for SMN and DF indirect access. */
 static DEFINE_MUTEX(smn_mutex);
@@ -50,6 +51,7 @@ const struct pci_device_id amd_nb_misc_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F3) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F3) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F3) },
 	{}
 };
 EXPORT_SYMBOL_GPL(amd_nb_misc_ids);
@@ -63,6 +65,7 @@ static const struct pci_device_id amd_nb_link_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M10H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M30H_DF_F4) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_17H_M70H_DF_F4) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_CNB17H_F4) },
 	{}
 };
include/linux/pci_ids.h +1 -0
index c842735a4f45..4b97f427cc92 100644
@@ -548,6 +548,7 @@
 #define PCI_DEVICE_ID_AMD_17H_DF_F3	0x1463
 #define PCI_DEVICE_ID_AMD_17H_M10H_DF_F3 0x15eb
 #define PCI_DEVICE_ID_AMD_17H_M30H_DF_F3 0x1493
+#define PCI_DEVICE_ID_AMD_17H_M70H_DF_F3 0x1443
 #define PCI_DEVICE_ID_AMD_CNB17H_F3	0x1703
 #define PCI_DEVICE_ID_AMD_LANCE		0x2000
 #define PCI_DEVICE_ID_AMD_LANCE_HOME	0x2001
Signature · 1 line
2.22.0