lspci: Slot Power Limit values above EFh

lspci: Slot Power Limit values above EFh

[Pali Rohár]

Hello!

PCI Express Base Specification rev. 3.0 has the following definition for
the Slot Power Limit Value:

=======================================================================
When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
Limit Value exceeds EFh, the following alternative encodings are used:
  F0h = 250 W Slot Power Limit
  F1h = 275 W Slot Power Limit
  F2h = 300 W Slot Power Limit
  F3h to FFh = Reserved for Slot Power Limit values above 300 W
=======================================================================

But the function power_limit() in ls-caps.c does not handle value above
EFh according to this definition.

Here is a simple patch which fixes it for values F0h..F2h. But I'm not
sure how (reserved) values above F2h should be handled.

diff --git a/ls-caps.c b/ls-caps.c
index db56556971cb..bc1eaa15017d 100644
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -659,6 +659,9 @@ static int exp_downstream_port(int type)
 static float power_limit(int value, int scale)
 {
   static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
+  static const int scale0_values[3] = { 250, 275, 300 }; /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
+  if (scale == 0 && value >= 0xF0)
+    value = scale0_values[(value > 0xF2 ? 0xF2 : value) & 0xF];
   return value * scales[scale];
 }
 

Re: lspci: Slot Power Limit values above EFh

[Matthew Wilcox]

On Sat, Apr 03, 2021 at 01:48:57PM +0200, Pali Rohár wrote:
> Hello!
> 
> PCI Express Base Specification rev. 3.0 has the following definition for
> the Slot Power Limit Value:

FWIW, it's the same in rev 5.  I had thought they might add even more
power encodings, but no.

> But the function power_limit() in ls-caps.c does not handle value above
> EFh according to this definition.
> 
> Here is a simple patch which fixes it for values F0h..F2h. But I'm not
> sure how (reserved) values above F2h should be handled.
> 
> diff --git a/ls-caps.c b/ls-caps.c
> index db56556971cb..bc1eaa15017d 100644
> --- a/ls-caps.c
> +++ b/ls-caps.c
> @@ -659,6 +659,9 @@ static int exp_downstream_port(int type)
>  static float power_limit(int value, int scale)
>  {
>    static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
> +  static const int scale0_values[3] = { 250, 275, 300 }; /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
> +  if (scale == 0 && value >= 0xF0)
> +    value = scale0_values[(value > 0xF2 ? 0xF2 : value) & 0xF];
>    return value * scales[scale];
>  }

How about ...

  if (scale == 0)
    {
      if (value > 0xf2)
        return 0.0f/0;
      if (value >= 0xf0)
        return scale0_values[value - 0xf0];
    }

(btw, a float is the same size as an int -- 32 bits, so there's no
benefit to storing the values as an int and doing the conversion at runtime.
may as well have the compiler put the right set of bits in the binary)

in the caller:

  if ((type == PCI_EXP_TYPE_ENDPOINT) || (type == PCI_EXP_TYPE_UPSTREAM) ||
      (type == PCI_EXP_TYPE_PCI_BRIDGE))
    {
      float power = power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18,
				(t & PCI_EXP_DEVCAP_PWR_SCL) >> 26));
      if (isnan(power))
        printf(" SlotPowerLimit >300W");
      else
	printf(" SlotPowerLimit %.3fW", power);
    }

[PATCH v2] lspci: Show Slot Power Limit values above EFh

[Pali Rohár]

PCI Express Base Specification rev. 3.0 has the following definition for
the Slot Power Limit Value:

=======================================================================
When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
Limit Value exceeds EFh, the following alternative encodings are used:
  F0h = 250 W Slot Power Limit
  F1h = 275 W Slot Power Limit
  F2h = 300 W Slot Power Limit
  F3h to FFh = Reserved for Slot Power Limit values above 300 W
=======================================================================

Replace function power_limit() by show_power_limit() which also prints
power limit value. Show reserved value as string ">300W" and omit usage of
floating point variables as it is not needed.
---
 ls-caps.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/ls-caps.c b/ls-caps.c
index db56556971cb..7fa6c1da45bd 100644
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -656,10 +656,27 @@ static int exp_downstream_port(int type)
 	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
 }
 
-static float power_limit(int value, int scale)
+static void show_power_limit(int value, int scale)
 {
-  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
-  return value * scales[scale];
+  static const int scales[4] = { 1000, 100, 10, 1 };
+  static const int scale0_values[3] = { 250, 275, 300 };
+  if (scale == 0 && value >= 0xF0) {
+    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
+    if (value >= 0xF3) {
+      printf(">300W");
+      return;
+    }
+    value = scale0_values[value - 0xF0];
+  }
+  value *= scales[scale];
+  printf("%d", value / 1000);
+  if (value % 10)
+    printf(".%03d", value % 1000);
+  else if (value % 100)
+    printf(".%02d", (value / 10) % 100);
+  else if (value % 1000)
+    printf(".%d", (value / 100) % 10);
+  printf("W");
 }
 
 static const char *latency_l0s(int value)
@@ -700,10 +717,10 @@ static void cap_express_dev(struct device *d, int where, int type)
     printf(" FLReset%c",
 	FLAG(t, PCI_EXP_DEVCAP_FLRESET));
   if ((type == PCI_EXP_TYPE_ENDPOINT) || (type == PCI_EXP_TYPE_UPSTREAM) ||
-      (type == PCI_EXP_TYPE_PCI_BRIDGE))
-    printf(" SlotPowerLimit %.3fW",
-	power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18,
-		    (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26));
+      (type == PCI_EXP_TYPE_PCI_BRIDGE)) {
+    printf(" SlotPowerLimit ");
+    show_power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18, (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26);
+  }
   printf("\n");
 
   w = get_conf_word(d, where + PCI_EXP_DEVCTL);
@@ -871,9 +888,10 @@ static void cap_express_slot(struct device *d, int where)
 	FLAG(t, PCI_EXP_SLTCAP_PWRI),
 	FLAG(t, PCI_EXP_SLTCAP_HPC),
 	FLAG(t, PCI_EXP_SLTCAP_HPS));
-  printf("\t\t\tSlot #%d, PowerLimit %.3fW; Interlock%c NoCompl%c\n",
-	(t & PCI_EXP_SLTCAP_PSN) >> 19,
-	power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15),
+  printf("\t\t\tSlot #%d, PowerLimit ",
+	(t & PCI_EXP_SLTCAP_PSN) >> 19);
+  show_power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15);
+  printf("; Interlock%c NoCompl%c\n",
 	FLAG(t, PCI_EXP_SLTCAP_INTERLOCK),
 	FLAG(t, PCI_EXP_SLTCAP_NOCMDCOMP));
 
-- 
2.20.1

Re: [PATCH v2] lspci: Show Slot Power Limit values above EFh

[Matthew Wilcox]

On Mon, Nov 01, 2021 at 03:47:40PM +0100, Pali Rohár wrote:
> PCI Express Base Specification rev. 3.0 has the following definition for
> the Slot Power Limit Value:
> 
> =======================================================================
> When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
> Limit Value exceeds EFh, the following alternative encodings are used:
>   F0h = 250 W Slot Power Limit
>   F1h = 275 W Slot Power Limit
>   F2h = 300 W Slot Power Limit
>   F3h to FFh = Reserved for Slot Power Limit values above 300 W
> =======================================================================
> 
> Replace function power_limit() by show_power_limit() which also prints
> power limit value. Show reserved value as string ">300W" and omit usage of
> floating point variables as it is not needed.

I don't understand why you want to avoid the use of floating point here?

> +++ b/ls-caps.c
> @@ -656,10 +656,27 @@ static int exp_downstream_port(int type)
>  	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
>  }
>  
> -static float power_limit(int value, int scale)
> +static void show_power_limit(int value, int scale)
>  {
> -  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
> -  return value * scales[scale];
> +  static const int scales[4] = { 1000, 100, 10, 1 };
> +  static const int scale0_values[3] = { 250, 275, 300 };
> +  if (scale == 0 && value >= 0xF0) {
> +    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
> +    if (value >= 0xF3) {
> +      printf(">300W");
> +      return;
> +    }
> +    value = scale0_values[value - 0xF0];
> +  }
> +  value *= scales[scale];
> +  printf("%d", value / 1000);
> +  if (value % 10)
> +    printf(".%03d", value % 1000);
> +  else if (value % 100)
> +    printf(".%02d", (value / 10) % 100);
> +  else if (value % 1000)
> +    printf(".%d", (value / 100) % 10);
> +  printf("W");

Wouldn't this be clearer if written as:

static void show_power_limit(int value, int scale)
{
  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
  static const int scale0_values[3] = { 250, 275, 300 };

  if (scale == 0 && value >= 0xF0) {
    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
    if (value >= 0xF3) {
      printf(">300W");
      return;
    }
    value = scale0_values[value - 0xF0];
  }
  printf("%.3fW", value * scales[scale]);
}

Re: [PATCH v2] lspci: Show Slot Power Limit values above EFh

[Pali Rohár]

On Monday 01 November 2021 15:03:31 Matthew Wilcox wrote:
> On Mon, Nov 01, 2021 at 03:47:40PM +0100, Pali Rohár wrote:
> > PCI Express Base Specification rev. 3.0 has the following definition for
> > the Slot Power Limit Value:
> > 
> > =======================================================================
> > When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
> > Limit Value exceeds EFh, the following alternative encodings are used:
> >   F0h = 250 W Slot Power Limit
> >   F1h = 275 W Slot Power Limit
> >   F2h = 300 W Slot Power Limit
> >   F3h to FFh = Reserved for Slot Power Limit values above 300 W
> > =======================================================================
> > 
> > Replace function power_limit() by show_power_limit() which also prints
> > power limit value. Show reserved value as string ">300W" and omit usage of
> > floating point variables as it is not needed.
> 
> I don't understand why you want to avoid the use of floating point here?

Because library does not use floating point. So I thought that it is a
good idea to not use it neither for printing power limit.

I can change it, just I wanted to hear project / library preference.

> > +++ b/ls-caps.c
> > @@ -656,10 +656,27 @@ static int exp_downstream_port(int type)
> >  	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
> >  }
> >  
> > -static float power_limit(int value, int scale)
> > +static void show_power_limit(int value, int scale)
> >  {
> > -  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
> > -  return value * scales[scale];
> > +  static const int scales[4] = { 1000, 100, 10, 1 };
> > +  static const int scale0_values[3] = { 250, 275, 300 };
> > +  if (scale == 0 && value >= 0xF0) {
> > +    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
> > +    if (value >= 0xF3) {
> > +      printf(">300W");
> > +      return;
> > +    }
> > +    value = scale0_values[value - 0xF0];
> > +  }
> > +  value *= scales[scale];
> > +  printf("%d", value / 1000);
> > +  if (value % 10)
> > +    printf(".%03d", value % 1000);
> > +  else if (value % 100)
> > +    printf(".%02d", (value / 10) % 100);
> > +  else if (value % 1000)
> > +    printf(".%d", (value / 100) % 10);
> > +  printf("W");
> 
> Wouldn't this be clearer if written as:
> 
> static void show_power_limit(int value, int scale)
> {
>   static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
>   static const int scale0_values[3] = { 250, 275, 300 };
> 
>   if (scale == 0 && value >= 0xF0) {
>     /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
>     if (value >= 0xF3) {
>       printf(">300W");
>       return;
>     }
>     value = scale0_values[value - 0xF0];
>   }
>   printf("%.3fW", value * scales[scale]);
> }
> 

Re: [PATCH v2] lspci: Show Slot Power Limit values above EFh

[Martin Mareš]

Hi!

> Because library does not use floating point. So I thought that it is a
> good idea to not use it neither for printing power limit.
> 
> I can change it, just I wanted to hear project / library preference.

Floating point in lspci is perfectly fine and it's the preferred solution.

I would hesitate for a moment before using it in the library, but lspci
is definitely OK.

					Martin

[PATCH v3 pciutils] lspci: Show Slot Power Limit values above EFh

[Pali Rohár]

PCI Express Base Specification rev. 3.0 has the following definition for
the Slot Power Limit Value:

=======================================================================
When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
Limit Value exceeds EFh, the following alternative encodings are used:
  F0h = 250 W Slot Power Limit
  F1h = 275 W Slot Power Limit
  F2h = 300 W Slot Power Limit
  F3h to FFh = Reserved for Slot Power Limit values above 300 W
=======================================================================

Replace function power_limit() by show_power_limit() which also prints
power limit value. Show reserved value as string ">300W".
---
v3: Use floating point and %g printf format to avoid trailing zeros.
---
 ls-caps.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/ls-caps.c b/ls-caps.c
index db56556971cb..6f5613794790 100644
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -656,10 +656,20 @@ static int exp_downstream_port(int type)
 	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
 }
 
-static float power_limit(int value, int scale)
+static void show_power_limit(int value, int scale)
 {
   static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
-  return value * scales[scale];
+  static const int scale0_values[3] = { 250, 275, 300 };
+
+  if (scale == 0 && value >= 0xF0) {
+    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
+    if (value >= 0xF3) {
+      printf(">300W");
+      return;
+    }
+    value = scale0_values[value - 0xF0];
+  }
+  printf("%gW", value * scales[scale]);
 }
 
 static const char *latency_l0s(int value)
@@ -700,10 +710,10 @@ static void cap_express_dev(struct device *d, int where, int type)
     printf(" FLReset%c",
 	FLAG(t, PCI_EXP_DEVCAP_FLRESET));
   if ((type == PCI_EXP_TYPE_ENDPOINT) || (type == PCI_EXP_TYPE_UPSTREAM) ||
-      (type == PCI_EXP_TYPE_PCI_BRIDGE))
-    printf(" SlotPowerLimit %.3fW",
-	power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18,
-		    (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26));
+      (type == PCI_EXP_TYPE_PCI_BRIDGE)) {
+    printf(" SlotPowerLimit ");
+    show_power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18, (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26);
+  }
   printf("\n");
 
   w = get_conf_word(d, where + PCI_EXP_DEVCTL);
@@ -871,9 +881,10 @@ static void cap_express_slot(struct device *d, int where)
 	FLAG(t, PCI_EXP_SLTCAP_PWRI),
 	FLAG(t, PCI_EXP_SLTCAP_HPC),
 	FLAG(t, PCI_EXP_SLTCAP_HPS));
-  printf("\t\t\tSlot #%d, PowerLimit %.3fW; Interlock%c NoCompl%c\n",
-	(t & PCI_EXP_SLTCAP_PSN) >> 19,
-	power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15),
+  printf("\t\t\tSlot #%d, PowerLimit ",
+	(t & PCI_EXP_SLTCAP_PSN) >> 19);
+  show_power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15);
+  printf("; Interlock%c NoCompl%c\n",
 	FLAG(t, PCI_EXP_SLTCAP_INTERLOCK),
 	FLAG(t, PCI_EXP_SLTCAP_NOCMDCOMP));
 
-- 
2.20.1

Re: [PATCH v3 pciutils] lspci: Show Slot Power Limit values above EFh

[Martin Mareš]

> +  if (scale == 0 && value >= 0xF0) {
> +    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
> +    if (value >= 0xF3) {
> +      printf(">300W");
> +      return;
> +    }
> +    value = scale0_values[value - 0xF0];
> +  }

Thanks!

One more request: please fix coding style to match the rest of lspci.

				Martin

[PATCH v4 pciutils] lspci: Show Slot Power Limit values above EFh

[Pali Rohár]

PCI Express Base Specification rev. 3.0 has the following definition for
the Slot Power Limit Value:

=======================================================================
When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
Limit Value exceeds EFh, the following alternative encodings are used:
  F0h = 250 W Slot Power Limit
  F1h = 275 W Slot Power Limit
  F2h = 300 W Slot Power Limit
  F3h to FFh = Reserved for Slot Power Limit values above 300 W
=======================================================================

Replace function power_limit() by show_power_limit() which also prints
power limit value. Show reserved value as string ">300W".
---
v4: Fix coding style
---
 ls-caps.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/ls-caps.c b/ls-caps.c
index db56556971cb..495d73a92e29 100644
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -656,10 +656,22 @@ static int exp_downstream_port(int type)
 	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
 }
 
-static float power_limit(int value, int scale)
+static void show_power_limit(int value, int scale)
 {
   static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
-  return value * scales[scale];
+  static const int scale0_values[3] = { 250, 275, 300 };
+
+  if (scale == 0 && value >= 0xF0)
+    {
+      /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
+      if (value >= 0xF3)
+        {
+          printf(">300W");
+          return;
+        }
+      value = scale0_values[value - 0xF0];
+    }
+  printf("%gW", value * scales[scale]);
 }
 
 static const char *latency_l0s(int value)
@@ -701,9 +713,10 @@ static void cap_express_dev(struct device *d, int where, int type)
 	FLAG(t, PCI_EXP_DEVCAP_FLRESET));
   if ((type == PCI_EXP_TYPE_ENDPOINT) || (type == PCI_EXP_TYPE_UPSTREAM) ||
       (type == PCI_EXP_TYPE_PCI_BRIDGE))
-    printf(" SlotPowerLimit %.3fW",
-	power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18,
-		    (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26));
+    {
+      printf(" SlotPowerLimit ");
+      show_power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18, (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26);
+    }
   printf("\n");
 
   w = get_conf_word(d, where + PCI_EXP_DEVCTL);
@@ -871,9 +884,10 @@ static void cap_express_slot(struct device *d, int where)
 	FLAG(t, PCI_EXP_SLTCAP_PWRI),
 	FLAG(t, PCI_EXP_SLTCAP_HPC),
 	FLAG(t, PCI_EXP_SLTCAP_HPS));
-  printf("\t\t\tSlot #%d, PowerLimit %.3fW; Interlock%c NoCompl%c\n",
-	(t & PCI_EXP_SLTCAP_PSN) >> 19,
-	power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15),
+  printf("\t\t\tSlot #%d, PowerLimit ",
+	(t & PCI_EXP_SLTCAP_PSN) >> 19);
+  show_power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15);
+  printf("; Interlock%c NoCompl%c\n",
 	FLAG(t, PCI_EXP_SLTCAP_INTERLOCK),
 	FLAG(t, PCI_EXP_SLTCAP_NOCMDCOMP));
 
-- 
2.20.1

Re: [PATCH v4 pciutils] lspci: Show Slot Power Limit values above EFh

[Martin Mareš]

Hi!

> PCI Express Base Specification rev. 3.0 has the following definition for
> the Slot Power Limit Value:
> 
> =======================================================================
> When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
> Limit Value exceeds EFh, the following alternative encodings are used:
>   F0h = 250 W Slot Power Limit
>   F1h = 275 W Slot Power Limit
>   F2h = 300 W Slot Power Limit
>   F3h to FFh = Reserved for Slot Power Limit values above 300 W
> =======================================================================
> 
> Replace function power_limit() by show_power_limit() which also prints
> power limit value. Show reserved value as string ">300W".

Thanks, applied.

					Martin