[PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Philippe Mathieu-Daudé]

This series aims to ease looking at the '-d unimp' output reported
by the UnimplementedDevice.

- read/write accesses are now aligned
- the value format width uses the access size
- the offset (address) uses the size of the memory region it belongs

Series fully reviewed.

Since v1:
- Use DIV_ROUND_UP instead of ROUND_UP (rth)
- Added rth R-b tags

$ git backport-diff -u v1 -r origin/master..
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/3:[----] [--] 'hw/misc/unimp: Display value after offset'
002/3:[----] [--] 'hw/misc/unimp: Display the value with width of the access size'
003/3:[0002] [FC] 'hw/misc/unimp: Display the offset with width of the region size'

Philippe Mathieu-Daudé (3):
  hw/misc/unimp: Display value after offset
  hw/misc/unimp: Display the value with width of the access size
  hw/misc/unimp: Display the offset with width of the region size

 include/hw/misc/unimp.h |  1 +
 hw/misc/unimp.c         | 14 ++++++++------
 2 files changed, 9 insertions(+), 6 deletions(-)

-- 
2.21.3

[PATCH v2 1/3] hw/misc/unimp: Display value after offset

[Philippe Mathieu-Daudé]

To better align the read/write accesses, display the value after
the offset (read accesses only display the offset).

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/unimp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/misc/unimp.c b/hw/misc/unimp.c
index bc4084d344..ee2e536c8b 100644
--- a/hw/misc/unimp.c
+++ b/hw/misc/unimp.c
@@ -22,7 +22,7 @@ static uint64_t unimp_read(void *opaque, hwaddr offset, unsigned size)
 {
     UnimplementedDeviceState *s = UNIMPLEMENTED_DEVICE(opaque);
 
-    qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read "
+    qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read  "
                   "(size %d, offset 0x%" HWADDR_PRIx ")\n",
                   s->name, size, offset);
     return 0;
@@ -34,9 +34,9 @@ static void unimp_write(void *opaque, hwaddr offset,
     UnimplementedDeviceState *s = UNIMPLEMENTED_DEVICE(opaque);
 
     qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
-                  "(size %d, value 0x%" PRIx64
-                  ", offset 0x%" HWADDR_PRIx ")\n",
-                  s->name, size, value, offset);
+                  "(size %d, offset 0x%" HWADDR_PRIx
+                  ", value 0x%" PRIx64 ")\n",
+                  s->name, size, offset, value);
 }
 
 static const MemoryRegionOps unimp_ops = {
-- 
2.21.3

[PATCH v2 2/3] hw/misc/unimp: Display the value with width of the access size

[Philippe Mathieu-Daudé]

To quickly notice the access size, display the value with the
width of the access (i.e. 16-bit access is displayed 0x0000,
while 8-bit access 0x00).

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/misc/unimp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/misc/unimp.c b/hw/misc/unimp.c
index ee2e536c8b..b4b318db1c 100644
--- a/hw/misc/unimp.c
+++ b/hw/misc/unimp.c
@@ -35,8 +35,8 @@ static void unimp_write(void *opaque, hwaddr offset,
 
     qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
                   "(size %d, offset 0x%" HWADDR_PRIx
-                  ", value 0x%" PRIx64 ")\n",
-                  s->name, size, offset, value);
+                  ", value 0x%0*" PRIx64 ")\n",
+                  s->name, size, offset, size << 1, value);
 }
 
 static const MemoryRegionOps unimp_ops = {
-- 
2.21.3

[PATCH v2 3/3] hw/misc/unimp: Display the offset with width of the region size

[Philippe Mathieu-Daudé]

To have a better idea of how big is the region where the offset
belongs, display the value with the width of the region size
(i.e. a region of 0x1000 bytes uses 0x000 format).

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v1: Use DIV_ROUND_UP (rth)
---
 include/hw/misc/unimp.h |  1 +
 hw/misc/unimp.c         | 10 ++++++----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/hw/misc/unimp.h b/include/hw/misc/unimp.h
index 4c1d13c9bf..c63968a2cd 100644
--- a/include/hw/misc/unimp.h
+++ b/include/hw/misc/unimp.h
@@ -20,6 +20,7 @@
 typedef struct {
     SysBusDevice parent_obj;
     MemoryRegion iomem;
+    unsigned offset_fmt_width;
     char *name;
     uint64_t size;
 } UnimplementedDeviceState;
diff --git a/hw/misc/unimp.c b/hw/misc/unimp.c
index b4b318db1c..6cfc5727f0 100644
--- a/hw/misc/unimp.c
+++ b/hw/misc/unimp.c
@@ -23,8 +23,8 @@ static uint64_t unimp_read(void *opaque, hwaddr offset, unsigned size)
     UnimplementedDeviceState *s = UNIMPLEMENTED_DEVICE(opaque);
 
     qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read  "
-                  "(size %d, offset 0x%" HWADDR_PRIx ")\n",
-                  s->name, size, offset);
+                  "(size %d, offset 0x%0*" HWADDR_PRIx ")\n",
+                  s->name, size, s->offset_fmt_width, offset);
     return 0;
 }
 
@@ -34,9 +34,9 @@ static void unimp_write(void *opaque, hwaddr offset,
     UnimplementedDeviceState *s = UNIMPLEMENTED_DEVICE(opaque);
 
     qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
-                  "(size %d, offset 0x%" HWADDR_PRIx
+                  "(size %d, offset 0x%0*" HWADDR_PRIx
                   ", value 0x%0*" PRIx64 ")\n",
-                  s->name, size, offset, size << 1, value);
+                  s->name, size, s->offset_fmt_width, offset, size << 1, value);
 }
 
 static const MemoryRegionOps unimp_ops = {
@@ -63,6 +63,8 @@ static void unimp_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    s->offset_fmt_width = DIV_ROUND_UP(64 - clz64(s->size - 1), 4);
+
     memory_region_init_io(&s->iomem, OBJECT(s), &unimp_ops, s,
                           s->name, s->size);
     sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem);
-- 
2.21.3

Re: [PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Philippe Mathieu-Daudé]

On 8/12/20 9:02 PM, Philippe Mathieu-Daudé wrote:
> This series aims to ease looking at the '-d unimp' output reported
> by the UnimplementedDevice.
> 
> - read/write accesses are now aligned
> - the value format width uses the access size
> - the offset (address) uses the size of the memory region it belongs
> 
> Series fully reviewed.

ping?

Re: [PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Peter Maydell]

On Sat, 22 Aug 2020 at 21:02, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 8/12/20 9:02 PM, Philippe Mathieu-Daudé wrote:
> > This series aims to ease looking at the '-d unimp' output reported
> > by the UnimplementedDevice.
> >
> > - read/write accesses are now aligned
> > - the value format width uses the access size
> > - the offset (address) uses the size of the memory region it belongs
> >
> > Series fully reviewed.
>
> ping?

What tree were you expecting these to go in via? Probably worth
being specific about who you're pinging in this kind of case ;-)

-- PMM

Re: [PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

Hi Peter,

Le lun. 24 août 2020 17:01, Peter Maydell <peter.maydell@linaro.org> a
écrit :

> On Sat, 22 Aug 2020 at 21:02, Philippe Mathieu-Daudé <f4bug@amsat.org>
> wrote:
> >
> > On 8/12/20 9:02 PM, Philippe Mathieu-Daudé wrote:
> > > This series aims to ease looking at the '-d unimp' output reported
> > > by the UnimplementedDevice.
> > >
> > > - read/write accesses are now aligned
> > > - the value format width uses the access size
> > > - the offset (address) uses the size of the memory region it belongs
> > >
> > > Series fully reviewed.
> >
> > ping?
>
> What tree were you expecting these to go in via? Probably worth
> being specific about who you're pinging in this kind of case ;-)
>

Ok understood for the next time. In this case you are listed as maintainer
:) Maybe you can Ack the series and then I'll ask to merge it via the
trivial or misc trees.


> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 1609 bytes --]

Re: [PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Philippe Mathieu-Daudé]

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

Le mer. 26 août 2020 11:54, Philippe Mathieu-Daudé <f4bug@amsat.org> a
écrit :

> Hi Peter,
>
> Le lun. 24 août 2020 17:01, Peter Maydell <peter.maydell@linaro.org> a
> écrit :
>
>> On Sat, 22 Aug 2020 at 21:02, Philippe Mathieu-Daudé <f4bug@amsat.org>
>> wrote:
>> >
>> > On 8/12/20 9:02 PM, Philippe Mathieu-Daudé wrote:
>> > > This series aims to ease looking at the '-d unimp' output reported
>> > > by the UnimplementedDevice.
>> > >
>> > > - read/write accesses are now aligned
>> > > - the value format width uses the access size
>> > > - the offset (address) uses the size of the memory region it belongs
>> > >
>> > > Series fully reviewed.
>> >
>> > ping?
>>
>> What tree were you expecting these to go in via? Probably worth
>> being specific about who you're pinging in this kind of case ;-)
>>
>
> Ok understood for the next time. In this case you are listed as maintainer
> :) Maybe you can Ack the series and then I'll ask to merge it via the
> trivial or misc trees.
>

I just realized qemu-trivial was correctly cc'ed.


>
>> -- PMM
>>
>

[-- Attachment #2: Type: text/html, Size: 2320 bytes --]

Re: [PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Peter Maydell]

On Wed, 26 Aug 2020 at 10:55, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> What tree were you expecting these to go in via? Probably worth
>> being specific about who you're pinging in this kind of case ;-)
>
>
> Ok understood for the next time. In this case you are listed as
> maintainer :)

Oh, so I am, I had forgotten that :-)

Anyway, I'll take it via the arm tree. My point was really just
that if you're asking for something to be done it's useful to
specify who you're asking if it's not completely obvious, because
otherwise everybody's liable to assume somebody else will be
dealing with it...

thanks
-- PMM