[PATCH v1 0/2] PCI: Include human-readable BAR and window sizes

[PATCH v1 0/2] PCI: Include human-readable BAR and window sizes

[Bjorn Helgaas]

PCI and other subsystems often use %pR to decode and print memory ranges.
The size of the range is useful to know but hard to read off at a glance,
e.g., 

  BAR 0 [mem 0xfea80000-0xfea9ffff]

Add more user-friendly size information to the %pR format, e.g.,

  BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]

%pR is also used to print I/O port, bus number, and DMA resources, where
the size is not as relevant, so this only applies to IORESOURCE_MEM
resources.

Bjorn Helgaas (2):
  vsprintf: Add %pR human-readable size
  PCI: Include human-readable sizes in resource assignment messages


vfio_pci_info_atomic_cap
 drivers/pci/setup-bus.c | 46 +++++++++++++++++++++++++++++------------
 drivers/pci/setup-res.c | 11 ++++++----
 lib/vsprintf.c          | 18 ++++++++++++----
 3 files changed, 54 insertions(+), 21 deletions(-)

-- 
2.53.0

[PATCH v1 1/2] vsprintf: Add %pR human-readable size

[Bjorn Helgaas]

Include human-readable size when printing struct resource memory ranges
(not including DMA, bus number, or I/O port ranges) with the %pR format to
make it easier to read, e.g.,

  - pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff]
  + pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 lib/vsprintf.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 2bc6ef483576..c49044b6dbee 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1094,15 +1094,17 @@ char *resource_string(char *buf, char *end, struct resource *res,
 	/* 32-bit res (sizeof==4): 10 chars in dec, 10 in hex ("0x" + 8)
 	 * 64-bit res (sizeof==8): 20 chars in dec, 18 in hex ("0x" + 16) */
 #define RSRC_BUF_SIZE		((2 * sizeof(resource_size_t)) + 4)
+#define RSRC_STR_SIZE		sizeof(" (xxxx.xxx MiB)")
 #define FLAG_BUF_SIZE		(2 * sizeof(res->flags))
-#define DECODED_BUF_SIZE	sizeof("[mem - 64bit pref window disabled]")
-#define RAW_BUF_SIZE		sizeof("[mem - flags 0x]")
-	char sym[MAX(2*RSRC_BUF_SIZE + DECODED_BUF_SIZE,
-		     2*RSRC_BUF_SIZE + FLAG_BUF_SIZE + RAW_BUF_SIZE)];
+#define DECODED_BUF_SIZE	sizeof("[mem - () 64bit pref window disabled]")
+#define RAW_BUF_SIZE		sizeof("[mem - () flags 0x]")
+	char sym[MAX(2*RSRC_BUF_SIZE + RSRC_STR_SIZE + DECODED_BUF_SIZE,
+		     2*RSRC_BUF_SIZE + RSRC_STR_SIZE + FLAG_BUF_SIZE + RAW_BUF_SIZE)];
 
 	char *p = sym, *pend = sym + sizeof(sym);
 	bool decode = fmt[0] == 'R';
 	const struct printf_spec *specp;
+	char size_buf[32];
 
 	if (check_pointer(&buf, end, res, spec))
 		return buf;
@@ -1134,6 +1136,14 @@ char *resource_string(char *buf, char *end, struct resource *res,
 	} else {
 		p = hex_range(p, pend, res->start, res->end, *specp);
 	}
+	if (res->flags & IORESOURCE_MEM) {
+		*p++ = ' ';
+		*p++ = '(';
+		string_get_size(resource_size(res), 1, STRING_UNITS_2,
+				size_buf, sizeof(size_buf));
+		p = string_nocheck(p, pend, size_buf, str_spec);
+		*p++ = ')';
+	}
 	if (decode) {
 		if (res->flags & IORESOURCE_MEM_64)
 			p = string_nocheck(p, pend, " 64bit", str_spec);
-- 
2.53.0

Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a KUnit test regression by unconditionally appending a human-readable resource size for memory ranges, affecting the raw '%pr' format as well as the decoded '%pR' format. The commit message states it adds the size to '%pR', but the code incorrectly modifies both formats, breaking the printf test suite.
--

--- Patch [1]: [PATCH v1 1/2] vsprintf: Add %pR human-readable size ---
commit d77ec66543a5589a88168c27ffcdb3693182c105
Author: Bjorn Helgaas <helgaas@kernel.org>

vsprintf: Add %pR human-readable size

Include human-readable size when printing struct resource memory ranges
with the %pR format to make it easier to read.

> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 2bc6ef483576c..c49044b6dbeeb 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
[ ... ]
> @@ -1134,6 +1136,14 @@ char *resource_string(char *buf, char *end, struct resource *res,
>  	} else {
>  		p = hex_range(p, pend, res->start, res->end, *specp);
>  	}
> +	if (res->flags & IORESOURCE_MEM) {
> +		*p++ = ' ';
> +		*p++ = '(';
> +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> +				size_buf, sizeof(size_buf));
> +		p = string_nocheck(p, pend, size_buf, str_spec);
> +		*p++ = ')';
> +	}

[Severity: Medium]
Does this code inadvertently modify the raw '%pr' format as well?

The commit message states it adds the size to '%pR', but this check doesn't
verify the 'decode' boolean before appending the human-readable size.

This unconditionally appends the human-readable resource size for memory ranges,
which affects the raw '%pr' format and breaks KUnit tests expecting the
unmodified output:

lib/tests/printf_kunit.c:struct_resource() {
    ...
    test("[mem 0xc0ffee00 flags 0x200]",
         "%pr", &test_resource);
    ...
}

>  	if (decode) {
>  		if (res->flags & IORESOURCE_MEM_64)
>  			p = string_nocheck(p, pend, " 64bit", str_spec);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808172308.282591-1-bhelgaas@google.com?part=1

Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size

[Andy Shevchenko]

On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:
> Include human-readable size when printing struct resource memory ranges
> (not including DMA, bus number, or I/O port ranges) with the %pR format to
> make it easier to read, e.g.,
> 
>   - pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff]
>   + pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]

...

>  lib/vsprintf.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)

Sorry, we do not accept this without added test cases.

...

> +	if (res->flags & IORESOURCE_MEM) {

Besides we have resource_type() macro, why only this? I would expect IO as well
as 64-bit MEM version.

> +		*p++ = ' ';
> +		*p++ = '(';
> +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> +				size_buf, sizeof(size_buf));

> +		p = string_nocheck(p, pend, size_buf, str_spec);

Why do you need this? Use returned value from string_get_size() directly.

> +		*p++ = ')';
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size

[Bjorn Helgaas]

On Sat, Aug 08, 2026 at 08:43:30PM +0300, Andy Shevchenko wrote:
> On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:
> > Include human-readable size when printing struct resource memory ranges
> > (not including DMA, bus number, or I/O port ranges) with the %pR format to
> > make it easier to read, e.g.,
> > 
> >   - pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff]
> >   + pci 0000:00:02.0: BAR 0 [mem 0xfea80000-0xfea9ffff (128 KiB)]
> 
> ...
> 
> >  lib/vsprintf.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> Sorry, we do not accept this without added test cases.

TIL about these tests, they look great and I'll fix this up and update
printk-formats.rst as well, thank you!

> ...
> 
> > +	if (res->flags & IORESOURCE_MEM) {
> 
> Besides we have resource_type() macro, why only this? I would expect
> IO as well as 64-bit MEM version.

We could use resource_type() here, but would have to check other bits
directly (IORESOURCE_PREFETCH, IORESOURCE_MEM_64, etc) since they're
not included.

I considered including IO and certainly could do that.  IO resources
are smaller and it's not as interesting, but maybe we should.

64-bit MEM resources are already included because they have both
IORESOURCE_MEM and IORESOURCE_MEM_64 set.

> > +		*p++ = ' ';
> > +		*p++ = '(';
> > +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> > +				size_buf, sizeof(size_buf));
> 
> > +		p = string_nocheck(p, pend, size_buf, str_spec);
> 
> Why do you need this? Use returned value from string_get_size() directly.

string_get_size() doesn't return a char * like string_nocheck() et al
do.  We could do something like "p += string_get_size(..., p, pend - p)",
but it's a little messy because the return value is the length that
*would* have been written if it was truncated, so I think we'd have to
check for "p > pend".  I'll ponder this one.

> > +		*p++ = ')';
> > +	}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v1 1/2] vsprintf: Add %pR human-readable size

[Andy Shevchenko]

On Sat, Aug 08, 2026 at 06:45:30PM -0500, Bjorn Helgaas wrote:
> On Sat, Aug 08, 2026 at 08:43:30PM +0300, Andy Shevchenko wrote:
> > On Sat, Aug 08, 2026 at 12:23:07PM -0500, Bjorn Helgaas wrote:

...

> > > +	if (res->flags & IORESOURCE_MEM) {
> > 
> > Besides we have resource_type() macro, why only this? I would expect
> > IO as well as 64-bit MEM version.
> 
> We could use resource_type() here, but would have to check other bits
> directly (IORESOURCE_PREFETCH, IORESOURCE_MEM_64, etc) since they're
> not included.

Sure.

> I considered including IO and certainly could do that.  IO resources
> are smaller and it's not as interesting, but maybe we should.
> 
> 64-bit MEM resources are already included because they have both
> IORESOURCE_MEM and IORESOURCE_MEM_64 set.

Is it guaranteed to have MEM when MEM_64 is set?

...

> > > +		*p++ = ' ';
> > > +		*p++ = '(';
> > > +		string_get_size(resource_size(res), 1, STRING_UNITS_2,
> > > +				size_buf, sizeof(size_buf));
> > 
> > > +		p = string_nocheck(p, pend, size_buf, str_spec);
> > 
> > Why do you need this? Use returned value from string_get_size() directly.
> 
> string_get_size() doesn't return a char * like string_nocheck() et al
> do.  We could do something like "p += string_get_size(..., p, pend - p)",
> but it's a little messy

See the use of string_escape_mem() in the same file.

> because the return value is the length that
> *would* have been written if it was truncated, so I think we'd have to
> check for "p > pend".  I'll ponder this one.

Yes, and that's exactly what we want from the vsnprintf() as a returned value.
What I mean is to use the given buffer directly without any intermediate one.
The *p++ = 'xxx' will become an idiomatic

	if (end > ...)
		*buf = 'xxx';
	++buf;

which vsprintf.c is full of.

> > > +		*p++ = ')';
> > > +	}

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages

[Bjorn Helgaas]

Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
"0x1000000" size, in resource-related messages.  Also consistently include
the "0x" prefix.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/setup-bus.c | 46 +++++++++++++++++++++++++++++------------
 drivers/pci/setup-res.c | 11 ++++++----
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index c0a949f2c995..4f62ba6f4d7f 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -28,6 +28,7 @@
 #include <linux/limits.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
+#include <linux/string_helpers.h>
 #include <linux/acpi.h>
 #include "pci.h"
 
@@ -427,6 +428,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
 	struct resource *res;
 	const char *res_name;
 	resource_size_t add_size, align;
+	char size_buf[32];
 	int idx;
 
 	list_for_each_entry_safe(add_res, tmp, realloc_head, list) {
@@ -460,10 +462,14 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
 		} else if (add_size > 0 || !IS_ALIGNED(res->start, align)) {
 			res->flags |= add_res->flags &
 				 (IORESOURCE_STARTALIGN|IORESOURCE_SIZEALIGN);
-			if (pci_reassign_resource(dev, idx, add_size, align))
-				pci_info(dev, "%s %pR: failed to add optional %llx\n",
+			if (pci_reassign_resource(dev, idx, add_size, align)) {
+				string_get_size(add_size, 1, STRING_UNITS_2,
+                                        size_buf, sizeof(size_buf));
+				pci_info(dev, "%s %pR: failed to add optional %#llx (%s)\n",
 					 res_name, res,
-					 (unsigned long long) add_size);
+					 (unsigned long long) add_size,
+					 size_buf);
+			}
 		}
 out:
 		list_del(&add_res->list);
@@ -1076,6 +1082,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t add_size,
 	resource_size_t size = 0, size0 = 0, size1 = 0;
 	resource_size_t children_add_size = 0;
 	resource_size_t min_align, align;
+	char size_buf[32];
 
 	if (!b_res)
 		return;
@@ -1138,11 +1145,14 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t add_size,
 	b_res->flags |= IORESOURCE_STARTALIGN;
 	if (bus->self && size1 > size0 && realloc_head) {
 		b_res->flags &= ~IORESOURCE_DISABLED;
+		add_size = size1 - size0;
 		pci_dev_res_add_to_list(realloc_head, bus->self, b_res,
-					size1 - size0, min_align);
-		pci_info(bus->self, "bridge window %pR to %pR add_size %llx\n",
+					add_size, min_align);
+		string_get_size(add_size, 1, STRING_UNITS_2,
+                                size_buf, sizeof(size_buf));
+		pci_info(bus->self, "bridge window %pR to %pR add_size %#llx (%s)\n",
 			 b_res, &bus->busn_res,
-			 (unsigned long long) size1 - size0);
+			 (unsigned long long) add_size, size_buf);
 	}
 }
 
@@ -1284,6 +1294,7 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
 	resource_size_t aligns[28] = {}; /* Alignments from 1MB to 128TB */
 	int order, max_order;
 	resource_size_t children_add_size = 0;
+	char size_buf[32], align_buf[32];
 	resource_size_t add_align = 0;
 
 	if (!b_res)
@@ -1378,10 +1389,14 @@ static void pbus_size_mem(struct pci_bus *bus, struct resource *b_res,
 		add_size = size1 > size0 ? size1 - size0 : 0;
 		pci_dev_res_add_to_list(realloc_head, bus->self, b_res,
 					add_size, add_align);
-		pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n",
+		string_get_size(add_size, 1, STRING_UNITS_2, size_buf,
+				sizeof(size_buf));
+		string_get_size(add_align, 1, STRING_UNITS_2, align_buf,
+				sizeof(align_buf));
+		pci_info(bus->self, "bridge window %pR to %pR add_size %#llx (%s) add_align %#llx (%s)\n",
 			   b_res, &bus->busn_res,
-			   (unsigned long long) add_size,
-			   (unsigned long long) add_align);
+			   (unsigned long long) add_size, size_buf,
+			   (unsigned long long) add_align, align_buf);
 	}
 }
 
@@ -1857,6 +1872,7 @@ static void adjust_bridge_window(struct pci_dev *bridge, struct resource *res,
 {
 	resource_size_t add_size, size = resource_size(res);
 	struct pci_dev_resource *dev_res;
+	char size_buf[32];
 
 	if (resource_assigned(res))
 		return;
@@ -1866,8 +1882,10 @@ static void adjust_bridge_window(struct pci_dev *bridge, struct resource *res,
 
 	if (new_size > size) {
 		add_size = new_size - size;
-		pci_dbg(bridge, "bridge window %pR extended by %pa\n", res,
-			&add_size);
+		string_get_size(add_size, 1, STRING_UNITS_2,
+				size_buf, sizeof(size_buf));
+		pci_dbg(bridge, "bridge window %pR extended by %pa (%s)\n", res,
+			&add_size, size_buf);
 	} else if (new_size < size) {
 		int idx = pci_resource_num(bridge, res);
 
@@ -1900,8 +1918,10 @@ static void adjust_bridge_window(struct pci_dev *bridge, struct resource *res,
 		add_size = size - new_size;
 		if (add_size < dev_res->add_size) {
 			dev_res->add_size -= add_size;
-			pci_dbg(bridge, "bridge window %pR optional size shrunken by %pa\n",
-				res, &add_size);
+			string_get_size(add_size, 1, STRING_UNITS_2,
+					size_buf, sizeof(size_buf));
+			pci_dbg(bridge, "bridge window %pR optional size shrunken by %pa (%s)\n",
+				res, &add_size, size_buf);
 		} else {
 			pci_dbg(bridge, "bridge window %pR optional size removed\n",
 				res);
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index 376f09630a4a..707c405000b8 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -20,6 +20,7 @@
 #include <linux/ioport.h>
 #include <linux/cache.h>
 #include <linux/slab.h>
+#include <linux/string_helpers.h>
 #include "pci.h"
 
 static void pci_std_update_resource(struct pci_dev *dev, int resno)
@@ -431,6 +432,7 @@ int pci_reassign_resource(struct pci_dev *dev, int resno,
 	const char *res_name = pci_resource_name(dev, resno);
 	unsigned long flags;
 	resource_size_t new_size;
+	char size_buf[32];
 	int ret;
 
 	if (res->flags & IORESOURCE_PCI_FIXED)
@@ -444,19 +446,20 @@ int pci_reassign_resource(struct pci_dev *dev, int resno,
 		return -EINVAL;
 	}
 
+	string_get_size(addsize, 1, STRING_UNITS_2, size_buf, sizeof(size_buf));
 	new_size = resource_size(res) + addsize;
 	ret = _pci_assign_resource(dev, resno, new_size, min_align);
 	if (ret) {
 		res->flags = flags;
-		pci_info(dev, "%s %pR: failed to expand by %#llx\n",
-			 res_name, res, (unsigned long long) addsize);
+		pci_info(dev, "%s %pR: failed to expand by %#llx (%s)\n",
+			 res_name, res, (unsigned long long) addsize, size_buf);
 		return ret;
 	}
 
 	res->flags &= ~IORESOURCE_UNSET;
 	res->flags &= ~IORESOURCE_STARTALIGN;
-	pci_info(dev, "%s %pR: reassigned; expanded by %#llx\n",
-		 res_name, res, (unsigned long long) addsize);
+	pci_info(dev, "%s %pR: reassigned; expanded by %#llx (%s)\n",
+		 res_name, res, (unsigned long long) addsize, size_buf);
 	if (resno < PCI_BRIDGE_RESOURCES)
 		pci_update_resource(dev, resno);
 
-- 
2.53.0

Re: [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages

[sashiko-bot]

> Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
> "0x1000000" size, in resource-related messages.  Also consistently include
> the "0x" prefix.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260808172308.282591-1-bhelgaas@google.com?part=2

Re: [PATCH v1 2/2] PCI: Include human-readable sizes in resource assignment messages

[Andy Shevchenko]

On Sat, Aug 08, 2026 at 12:23:08PM -0500, Bjorn Helgaas wrote:
> Include human-readable sizes, e.g., "16.0 MiB", in addition to the hex
> "0x1000000" size, in resource-related messages.  Also consistently include
> the "0x" prefix.

Instead of repeating many times the %#llx (%s) and accompanying
string_get_size() calls can we rather introduce a (sub-)extension
to %p[R] (perhaps against 'R' to print only size) and use it?

-- 
With Best Regards,
Andy Shevchenko