Series comparison

-[Qemu-devel] [PULL 0/4] Block patches
+[PULL for-7.0 0/2] Block patches
-The following changes since commit 20d6c7312f1b812bb9c750f4087f69ac8485cc90:
+The following changes since commit 15ef89d2a1a7b93845a6b09c2ee8e1979f6eb30b:
-  Merge remote-tracking branch 'remotes/palmer/tags/riscv-for-master-3.2-part1' into staging (2019-01-03 13:26:30 +0000)
+  Update version for v7.0.0-rc1 release (2022-03-22 22:58:44 +0000)
 are available in the Git repository at:
-  git://github.com/stefanha/qemu.git tags/block-pull-request
+  https://gitlab.com/stefanha/qemu.git tags/block-pull-request
-for you to fetch changes up to 39a0408e768cd00142f5b57d27ab234282bf4df5:
+for you to fetch changes up to 2539eade4f689eda7e9fe45486f18334bfbafaf0:
-  dmg: don't skip zero chunk (2019-01-04 11:15:09 +0000)
+  hw: Fix misleading hexadecimal format (2022-03-24 10:38:42 +0000)
 ----------------------------------------------------------------
 Pull request
-Bug fixes for the .dmg image file format.
+Philippe found cases where the 0x%d format string was used, leading to
 misleading output. The patches look harmless and could save people time, so I
 think it's worth including them in 7.0.
 ----------------------------------------------------------------
-Julio Faracco (1):
+Philippe Mathieu-Daudé (2):
-  dmg: Fixing wrong dmg block type value for block terminator.
+  block: Fix misleading hexadecimal format
   hw: Fix misleading hexadecimal format
-yuchenlin (3):
+ block/parallels-ext.c | 2 +-
-  dmg: fix binary search
+ hw/i386/sgx.c         | 2 +-
-  dmg: use enumeration type instead of hard coding number
+ hw/i386/trace-events  | 6 +++---
-  dmg: don't skip zero chunk
+ hw/misc/trace-events  | 4 ++--
+ hw/scsi/trace-events  | 4 ++--
- block/dmg.c | 31 ++++++++++++++++++++-----------
+files changed, 9 insertions(+), 9 deletions(-)
 file changed, 20 insertions(+), 11 deletions(-)
 --
-.20.1
+.35.1

-[Qemu-devel] [PULL 1/4] dmg: Fixing wrong dmg block type value for block terminator.
+[PULL for-7.0 1/2] block: Fix misleading hexadecimal format
-From: Julio Faracco <jcfaracco@gmail.com>
+From: Philippe Mathieu-Daudé <f4bug@amsat.org>
-This is a trivial patch to fix a wrong value for block terminator.
+"0x%u" format is very misleading, replace by "0x%x".
 The old value was 0x7fffffff which is wrong. It was not affecting the
 code because QEMU dmg block is not handling block terminator right now.
 Neverthless, it should be fixed.
-Signed-off-by: Julio Faracco <jcfaracco@gmail.com>
+Found running:
-Reviewed-by: yuchenlin <yuchenlin@synology.com>
-Message-id: 20181228145055.18039-1-jcfaracco@gmail.com
+  $ git grep -E '0x%[0-9]*([lL]*|" ?PRI)[dDuU]' block/
 Inspired-by: Richard Henderson <richard.henderson@linaro.org>
 Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
 Reviewed-by: Hanna Reitz <hreitz@redhat.com>
 Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
 Reviewed-by: Denis V. Lunev <den@openvz.org>
 Message-id: 20220323114718.58714-2-philippe.mathieu.daude@gmail.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/dmg.c | 2 +-
+ block/parallels-ext.c | 2 +-
 file changed, 1 insertion(+), 1 deletion(-)
-diff --git a/block/dmg.c b/block/dmg.c
+diff --git a/block/parallels-ext.c b/block/parallels-ext.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
+--- a/block/parallels-ext.c
-+++ b/block/dmg.c
++++ b/block/parallels-ext.c
-@@ -XXX,XX +XXX,XX @@ enum {
+@@ -XXX,XX +XXX,XX @@ static int parallels_parse_format_extension(BlockDriverState *bs,
-     UDBZ,
+             break;
-     ULFO,
-     UDCM = 0x7ffffffe, /* Comments */
+         default:
--    UDLE               /* Last Entry */
+-            error_setg(errp, "Unknown feature: 0x%" PRIu64, fh.magic);
-+    UDLE = 0xffffffff  /* Last Entry */
++            error_setg(errp, "Unknown feature: 0x%" PRIx64, fh.magic);
- };
+             goto fail;
+         }
- static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
 --
-.20.1
+.35.1

-[Qemu-devel] [PULL 2/4] dmg: fix binary search
+Deleted patch
-From: yuchenlin <npes87184@gmail.com>
-There is a possible hang in original binary search implementation. That is
-if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.
-The chunk1 will be still 4, and so on.
-Signed-off-by: yuchenlin <npes87184@gmail.com>
-Message-id: 20190103114700.9686-2-npes87184@gmail.com
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- block/dmg.c | 10 +++++++---
-file changed, 7 insertions(+), 3 deletions(-)
-diff --git a/block/dmg.c b/block/dmg.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
-+++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
- {
-     /* binary search */
-     uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
--    while (chunk1 != chunk2) {
-+    while (chunk1 <= chunk2) {
-         chunk3 = (chunk1 + chunk2) / 2;
-         if (s->sectors[chunk3] > sector_num) {
--            chunk2 = chunk3;
-+            if (chunk3 == 0) {
-+                goto err;
-+            }
-+            chunk2 = chunk3 - 1;
-         } else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
-             return chunk3;
-         } else {
--            chunk1 = chunk3;
-+            chunk1 = chunk3 + 1;
-         }
-     }
-+err:
-     return s->n_chunks; /* error */
- }
---
-.20.1

-[Qemu-devel] [PULL 3/4] dmg: use enumeration type instead of hard coding number
+Deleted patch
-From: yuchenlin <npes87184@gmail.com>
-Signed-off-by: yuchenlin <npes87184@gmail.com>
-Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
-Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
-Message-id: 20190103114700.9686-3-npes87184@gmail.com
-Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
----
- block/dmg.c | 4 ++--
-file changed, 2 insertions(+), 2 deletions(-)
-diff --git a/block/dmg.c b/block/dmg.c
-index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
-+++ b/block/dmg.c
-@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
-         /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
-          * therefore be unbounded. */
--        if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-+        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
-             error_report("sector count %" PRIu64 " for chunk %" PRIu32
-                          " is larger than max (%u)",
-                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
-@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
-         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
-          * s->uncompressed_chunk may be too small to cover the large all-zeroes
-          * section. dmg_read_chunk is called to find s->current_chunk */
--        if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
-+        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
-             qemu_iovec_memset(qiov, i * 512, 0, 512);
-             continue;
-         }
---
-.20.1

-[Qemu-devel] [PULL 4/4] dmg: don't skip zero chunk
+[PULL for-7.0 2/2] hw: Fix misleading hexadecimal format
-From: yuchenlin <npes87184@gmail.com>
+From: Philippe Mathieu-Daudé <f4bug@amsat.org>
-The dmg file has many tables which describe: "start from sector XXX to
+"0x%u" format is very misleading, replace by "0x%x".
 sector XXX, the compression method is XXX and where the compressed data
 resides on".
-Each sector in the expanded file should be covered by a table. The table
+Found running:
 will describe the offset of compressed data (or raw depends on the type)
 in the dmg.
-For example:
+  $ git grep -E '0x%[0-9]*([lL]*|" ?PRI)[dDuU]' hw/
-[-----------The expanded file------------]
+Inspired-by: Richard Henderson <richard.henderson@linaro.org>
-[---bzip table ---]/* zeros */[---zlib---]
+Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
-    ^
+Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
-    | if we want to read this sector.
+Message-id: 20220323114718.58714-3-philippe.mathieu.daude@gmail.com
 we will find bzip table which contains this sector, and get the
 compressed data offset, read it from dmg, uncompress it, finally write to
 expanded file.
 If we skip zero chunk (table), some sector cannot find the table which
 will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
 and finally causing dmg_co_preadv() return EIO.
 See:
 [-----------The expanded file------------]
 [---bzip table ---]/* zeros */[---zlib---]
                     ^
                     | if we want to read this sector.
 Oops, we cannot find the table contains it...
 In the original implementation, we don't have zero table. When we try to
 read sector inside the zero chunk. We will get EIO, and skip reading.
 After this patch, we treat zero chunk the same as ignore chunk, it will
 directly write zero and avoid some sector may not find the table.
 After this patch:
 [-----------The expanded file------------]
 [---bzip table ---][--zeros--][---zlib---]
 Signed-off-by: yuchenlin <npes87184@gmail.com>
 Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
 Message-id: 20190103114700.9686-4-npes87184@gmail.com
 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
 ---
- block/dmg.c | 19 ++++++++++++-------
+ hw/i386/sgx.c        | 2 +-
-file changed, 12 insertions(+), 7 deletions(-)
+ hw/i386/trace-events | 6 +++---
  hw/misc/trace-events | 4 ++--
  hw/scsi/trace-events | 4 ++--
 files changed, 8 insertions(+), 8 deletions(-)
-diff --git a/block/dmg.c b/block/dmg.c
+diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
 index XXXXXXX..XXXXXXX 100644
---- a/block/dmg.c
+--- a/hw/i386/sgx.c
-+++ b/block/dmg.c
++++ b/hw/i386/sgx.c
-@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
+@@ -XXX,XX +XXX,XX @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
-     case UDRW: /* copy */
+     }
-         uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
-         break;
+     if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) {
--    case UDIG: /* zero */
+-        error_report("Size of all 'sgx-epc' =0x%"PRIu64" causes EPC to wrap",
-+    case UDZE: /* zero */
++        error_report("Size of all 'sgx-epc' =0x%"PRIx64" causes EPC to wrap",
-+    case UDIG: /* ignore */
+                      sgx_epc->size);
-         /* as the all-zeroes block may be large, it is treated specially: the
+         exit(EXIT_FAILURE);
-          * sector is not copied from a large buffer, a simple memset is used
+     }
-          * instead. Therefore uncompressed_sectors does not need to be set. */
+diff --git a/hw/i386/trace-events b/hw/i386/trace-events
-@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {
+index XXXXXXX..XXXXXXX 100644
- static bool dmg_is_known_block_type(uint32_t entry_type)
+--- a/hw/i386/trace-events
- {
++++ b/hw/i386/trace-events
-     switch (entry_type) {
+@@ -XXX,XX +XXX,XX @@ vtd_fault_disabled(void) "Fault processing disabled for context entry"
-+    case UDZE:    /* zeros */
+ vtd_replay_ce_valid(const char *mode, uint8_t bus, uint8_t dev, uint8_t fn, uint16_t domain, uint64_t hi, uint64_t lo) "%s: replay valid context device %02"PRIx8":%02"PRIx8".%02"PRIx8" domain 0x%"PRIx16" hi 0x%"PRIx64" lo 0x%"PRIx64
-     case UDRW:    /* uncompressed */
+ vtd_replay_ce_invalid(uint8_t bus, uint8_t dev, uint8_t fn) "replay invalid context device %02"PRIx8":%02"PRIx8".%02"PRIx8
--    case UDIG:    /* zeroes */
+ vtd_page_walk_level(uint64_t addr, uint32_t level, uint64_t start, uint64_t end) "walk (base=0x%"PRIx64", level=%"PRIu32") iova range 0x%"PRIx64" - 0x%"PRIx64
-+    case UDIG:    /* ignore */
+-vtd_page_walk_one(uint16_t domain, uint64_t iova, uint64_t gpa, uint64_t mask, int perm) "domain 0x%"PRIu16" iova 0x%"PRIx64" -> gpa 0x%"PRIx64" mask 0x%"PRIx64" perm %d"
-     case UDZO:    /* zlib */
++vtd_page_walk_one(uint16_t domain, uint64_t iova, uint64_t gpa, uint64_t mask, int perm) "domain 0x%"PRIx16" iova 0x%"PRIx64" -> gpa 0x%"PRIx64" mask 0x%"PRIx64" perm %d"
-         return true;
+ vtd_page_walk_one_skip_map(uint64_t iova, uint64_t mask, uint64_t translated) "iova 0x%"PRIx64" mask 0x%"PRIx64" translated 0x%"PRIx64
-     case UDBZ:    /* bzip2 */
+ vtd_page_walk_one_skip_unmap(uint64_t iova, uint64_t mask) "iova 0x%"PRIx64" mask 0x%"PRIx64
-@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
+ vtd_page_walk_skip_read(uint64_t iova, uint64_t next) "Page walk skip iova 0x%"PRIx64" - 0x%"PRIx64" due to unable to read"
-         /* sector count */
+ vtd_page_walk_skip_reserve(uint64_t iova, uint64_t next) "Page walk skip iova 0x%"PRIx64" - 0x%"PRIx64" due to rsrv set"
-         s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
+ vtd_switch_address_space(uint8_t bus, uint8_t slot, uint8_t fn, bool on) "Device %02x:%02x.%x switching address space (iommu enabled=%d)"
+ vtd_as_unmap_whole(uint8_t bus, uint8_t slot, uint8_t fn, uint64_t iova, uint64_t size) "Device %02x:%02x.%x start 0x%"PRIx64" size 0x%"PRIx64
--        /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
+-vtd_translate_pt(uint16_t sid, uint64_t addr) "source id 0x%"PRIu16", iova 0x%"PRIx64
--         * therefore be unbounded. */
+-vtd_pt_enable_fast_path(uint16_t sid, bool success) "sid 0x%"PRIu16" %d"
--        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
++vtd_translate_pt(uint16_t sid, uint64_t addr) "source id 0x%"PRIx16", iova 0x%"PRIx64
-+        /* all-zeroes sector (type UDZE and UDIG) does not need to be
++vtd_pt_enable_fast_path(uint16_t sid, bool success) "sid 0x%"PRIx16" %d"
-+         * "uncompressed" and can therefore be unbounded. */
+ vtd_irq_generate(uint64_t addr, uint64_t data) "addr 0x%"PRIx64" data 0x%"PRIx64
-+        if (s->types[i] != UDZE && s->types[i] != UDIG
+ vtd_reg_read(uint64_t addr, uint64_t size) "addr 0x%"PRIx64" size 0x%"PRIx64
-+            && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+ vtd_reg_write(uint64_t addr, uint64_t size, uint64_t val) "addr 0x%"PRIx64" size 0x%"PRIx64" value 0x%"PRIx64
-             error_report("sector count %" PRIu64 " for chunk %" PRIu32
+diff --git a/hw/misc/trace-events b/hw/misc/trace-events
-                          " is larger than max (%u)",
+index XXXXXXX..XXXXXXX 100644
-                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
+--- a/hw/misc/trace-events
-@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
++++ b/hw/misc/trace-events
-                 return -1;
+@@ -XXX,XX +XXX,XX @@
-             }
+ # See docs/devel/tracing.rst for syntax documentation.
-             break;
--        case UDIG: /* zero */
+ # allwinner-cpucfg.c
-+        case UDZE: /* zeros */
+-allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "id %u, reset_addr 0x%" PRIu32
-+        case UDIG: /* ignore */
++allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "id %u, reset_addr 0x%" PRIx32
-             /* see dmg_read, it is treated specially. No buffer needs to be
+ allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
-              * pre-filled, the zeroes can be set directly. */
+ allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
-             break;
-@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+@@ -XXX,XX +XXX,XX @@ imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" PRIx64 "value 0x%08
-         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
-          * s->uncompressed_chunk may be too small to cover the large all-zeroes
+ # mos6522.c
-          * section. dmg_read_chunk is called to find s->current_chunk */
+ mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d"
--        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
+-mos6522_get_next_irq_time(uint16_t latch, int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 " delta_next=0x%"PRId64
-+        if (s->types[s->current_chunk] == UDZE
++mos6522_get_next_irq_time(uint16_t latch, int64_t d, int64_t delta) "latch=%d counter=0x%"PRIx64 " delta_next=0x%"PRIx64
-+            || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
+ mos6522_set_sr_int(void) "set sr_int"
-             qemu_iovec_memset(qiov, i * 512, 0, 512);
+ mos6522_write(uint64_t addr, const char *name, uint64_t val) "reg=0x%"PRIx64 " [%s] val=0x%"PRIx64
-             continue;
+ mos6522_read(uint64_t addr, const char *name, unsigned val) "reg=0x%"PRIx64 " [%s] val=0x%x"
-         }
+diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/scsi/trace-events
 +++ b/hw/scsi/trace-events
@@ -XXX,XX +XXX,XX @@ lsi_bad_phase_interrupt(void) "Phase mismatch interrupt"
  lsi_bad_selection(uint32_t id) "Selected absent target %"PRIu32
  lsi_do_dma_unavailable(void) "DMA no data available"
  lsi_do_dma(uint64_t addr, int len) "DMA addr=0x%"PRIx64" len=%d"
 -lsi_queue_command(uint32_t tag) "Queueing tag=0x%"PRId32
 +lsi_queue_command(uint32_t tag) "Queueing tag=0x%"PRIx32
  lsi_add_msg_byte_error(void) "MSG IN data too long"
  lsi_add_msg_byte(uint8_t data) "MSG IN 0x%02x"
  lsi_reselect(int id) "Reselected target %d"
@@ -XXX,XX +XXX,XX @@ lsi_do_msgout_noop(void) "MSG: No Operation"
  lsi_do_msgout_extended(uint8_t msg, uint8_t len) "Extended message 0x%x (len %d)"
  lsi_do_msgout_ignored(const char *msg) "%s (ignored)"
  lsi_do_msgout_simplequeue(uint8_t select_tag) "SIMPLE queue tag=0x%x"
 -lsi_do_msgout_abort(uint32_t tag) "MSG: ABORT TAG tag=0x%"PRId32
 +lsi_do_msgout_abort(uint32_t tag) "MSG: ABORT TAG tag=0x%"PRIx32
  lsi_do_msgout_clearqueue(uint32_t tag) "MSG: CLEAR QUEUE tag=0x%"PRIx32
  lsi_do_msgout_busdevicereset(uint32_t tag) "MSG: BUS DEVICE RESET tag=0x%"PRIx32
  lsi_do_msgout_select(int id) "Select LUN %d"
 --
-.20.1
+.35.1

From: yuchenlin <npes87184@gmail.com>

There is a possible hang in original binary search implementation. That is
if chunk1 = 4, chunk2 = 5, chunk3 = 4, and we go else case.

The chunk1 will be still 4, and so on.

Signed-off-by: yuchenlin <npes87184@gmail.com>
Message-id: 20190103114700.9686-2-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
 {
     /* binary search */
     uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
-    while (chunk1 != chunk2) {
+    while (chunk1 <= chunk2) {
         chunk3 = (chunk1 + chunk2) / 2;
         if (s->sectors[chunk3] > sector_num) {
-            chunk2 = chunk3;
+            if (chunk3 == 0) {
+                goto err;
+            }
+            chunk2 = chunk3 - 1;
         } else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
             return chunk3;
         } else {
-            chunk1 = chunk3;
+            chunk1 = chunk3 + 1;
         }
     }
+err:
     return s->n_chunks; /* error */
 }
 
-- 
2.20.1

From: yuchenlin <npes87184@gmail.com>

Signed-off-by: yuchenlin <npes87184@gmail.com>
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190103114700.9686-3-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
 
         /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
          * therefore be unbounded. */
-        if (s->types[i] != 2 && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
             error_report("sector count %" PRIu64 " for chunk %" PRIu32
                          " is larger than max (%u)",
                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
          * s->uncompressed_chunk may be too small to cover the large all-zeroes
          * section. dmg_read_chunk is called to find s->current_chunk */
-        if (s->types[s->current_chunk] == 2) { /* all zeroes block entry */
+        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
             qemu_iovec_memset(qiov, i * 512, 0, 512);
             continue;
         }
-- 
2.20.1

From: yuchenlin <npes87184@gmail.com>

The dmg file has many tables which describe: "start from sector XXX to
sector XXX, the compression method is XXX and where the compressed data
resides on".

Each sector in the expanded file should be covered by a table. The table
will describe the offset of compressed data (or raw depends on the type)
in the dmg.

For example:

[-----------The expanded file------------]
[---bzip table ---]/* zeros */[---zlib---]
    ^
    | if we want to read this sector.

we will find bzip table which contains this sector, and get the
compressed data offset, read it from dmg, uncompress it, finally write to
expanded file.

If we skip zero chunk (table), some sector cannot find the table which
will cause search_chunk() return s->n_chunks, dmg_read_chunk() return -1
and finally causing dmg_co_preadv() return EIO.

See:

[-----------The expanded file------------]
[---bzip table ---]/* zeros */[---zlib---]
                    ^
                    | if we want to read this sector.

Oops, we cannot find the table contains it...

In the original implementation, we don't have zero table. When we try to
read sector inside the zero chunk. We will get EIO, and skip reading.

After this patch, we treat zero chunk the same as ignore chunk, it will
directly write zero and avoid some sector may not find the table.

After this patch:

[-----------The expanded file------------]
[---bzip table ---][--zeros--][---zlib---]

Signed-off-by: yuchenlin <npes87184@gmail.com>
Reviewed-by: Julio Faracco <jcfaracco@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20190103114700.9686-4-npes87184@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index XXXXXXX..XXXXXXX 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
     case UDRW: /* copy */
         uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512);
         break;
-    case UDIG: /* zero */
+    case UDZE: /* zero */
+    case UDIG: /* ignore */
         /* as the all-zeroes block may be large, it is treated specially: the
          * sector is not copied from a large buffer, a simple memset is used
          * instead. Therefore uncompressed_sectors does not need to be set. */
@@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState {
 static bool dmg_is_known_block_type(uint32_t entry_type)
 {
     switch (entry_type) {
+    case UDZE:    /* zeros */
     case UDRW:    /* uncompressed */
-    case UDIG:    /* zeroes */
+    case UDIG:    /* ignore */
     case UDZO:    /* zlib */
         return true;
     case UDBZ:    /* bzip2 */
@@ -XXX,XX +XXX,XX @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
         /* sector count */
         s->sectorcounts[i] = buff_read_uint64(buffer, offset + 0x10);
 
-        /* all-zeroes sector (type 2) does not need to be "uncompressed" and can
-         * therefore be unbounded. */
-        if (s->types[i] != UDIG && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+        /* all-zeroes sector (type UDZE and UDIG) does not need to be
+         * "uncompressed" and can therefore be unbounded. */
+        if (s->types[i] != UDZE && s->types[i] != UDIG
+            && s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
             error_report("sector count %" PRIu64 " for chunk %" PRIu32
                          " is larger than max (%u)",
                          s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
@@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
                 return -1;
             }
             break;
-        case UDIG: /* zero */
+        case UDZE: /* zeros */
+        case UDIG: /* ignore */
             /* see dmg_read, it is treated specially. No buffer needs to be
              * pre-filled, the zeroes can be set directly. */
             break;
@@ -XXX,XX +XXX,XX @@ dmg_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         /* Special case: current chunk is all zeroes. Do not perform a memcpy as
          * s->uncompressed_chunk may be too small to cover the large all-zeroes
          * section. dmg_read_chunk is called to find s->current_chunk */
-        if (s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
+        if (s->types[s->current_chunk] == UDZE
+            || s->types[s->current_chunk] == UDIG) { /* all zeroes block entry */
             qemu_iovec_memset(qiov, i * 512, 0, 512);
             continue;
         }
-- 
2.20.1

The following changes since commit 15ef89d2a1a7b93845a6b09c2ee8e1979f6eb30b:

Update version for v7.0.0-rc1 release (2022-03-22 22:58:44 +0000)

are available in the Git repository at:

https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 2539eade4f689eda7e9fe45486f18334bfbafaf0:

hw: Fix misleading hexadecimal format (2022-03-24 10:38:42 +0000)

----------------------------------------------------------------
Pull request

Philippe found cases where the 0x%d format string was used, leading to
misleading output. The patches look harmless and could save people time, so I
think it's worth including them in 7.0.

----------------------------------------------------------------

Philippe Mathieu-Daudé (2):
  block: Fix misleading hexadecimal format
  hw: Fix misleading hexadecimal format

-- 
2.35.1

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

"0x%u" format is very misleading, replace by "0x%x".

Found running:

$ git grep -E '0x%[0-9]*([lL]*|" ?PRI)[dDuU]' block/

Inspired-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Message-id: 20220323114718.58714-2-philippe.mathieu.daude@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels-ext.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/parallels-ext.c b/block/parallels-ext.c
index XXXXXXX..XXXXXXX 100644
--- a/block/parallels-ext.c
+++ b/block/parallels-ext.c
@@ -XXX,XX +XXX,XX @@ static int parallels_parse_format_extension(BlockDriverState *bs,
             break;
 
         default:
-            error_setg(errp, "Unknown feature: 0x%" PRIu64, fh.magic);
+            error_setg(errp, "Unknown feature: 0x%" PRIx64, fh.magic);
             goto fail;
         }
 
-- 
2.35.1

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

"0x%u" format is very misleading, replace by "0x%x".

Found running:

$ git grep -E '0x%[0-9]*([lL]*|" ?PRI)[dDuU]' hw/

Inspired-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20220323114718.58714-3-philippe.mathieu.daude@gmail.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/i386/sgx.c        | 2 +-
 hw/i386/trace-events | 6 +++---
 hw/misc/trace-events | 4 ++--
 hw/scsi/trace-events | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/i386/sgx.c b/hw/i386/sgx.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/i386/sgx.c
+++ b/hw/i386/sgx.c
@@ -XXX,XX +XXX,XX @@ void pc_machine_init_sgx_epc(PCMachineState *pcms)
     }
 
     if ((sgx_epc->base + sgx_epc->size) < sgx_epc->base) {
-        error_report("Size of all 'sgx-epc' =0x%"PRIu64" causes EPC to wrap",
+        error_report("Size of all 'sgx-epc' =0x%"PRIx64" causes EPC to wrap",
                      sgx_epc->size);
         exit(EXIT_FAILURE);
     }
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -XXX,XX +XXX,XX @@ vtd_fault_disabled(void) "Fault processing disabled for context entry"
 vtd_replay_ce_valid(const char *mode, uint8_t bus, uint8_t dev, uint8_t fn, uint16_t domain, uint64_t hi, uint64_t lo) "%s: replay valid context device %02"PRIx8":%02"PRIx8".%02"PRIx8" domain 0x%"PRIx16" hi 0x%"PRIx64" lo 0x%"PRIx64
 vtd_replay_ce_invalid(uint8_t bus, uint8_t dev, uint8_t fn) "replay invalid context device %02"PRIx8":%02"PRIx8".%02"PRIx8
 vtd_page_walk_level(uint64_t addr, uint32_t level, uint64_t start, uint64_t end) "walk (base=0x%"PRIx64", level=%"PRIu32") iova range 0x%"PRIx64" - 0x%"PRIx64
-vtd_page_walk_one(uint16_t domain, uint64_t iova, uint64_t gpa, uint64_t mask, int perm) "domain 0x%"PRIu16" iova 0x%"PRIx64" -> gpa 0x%"PRIx64" mask 0x%"PRIx64" perm %d"
+vtd_page_walk_one(uint16_t domain, uint64_t iova, uint64_t gpa, uint64_t mask, int perm) "domain 0x%"PRIx16" iova 0x%"PRIx64" -> gpa 0x%"PRIx64" mask 0x%"PRIx64" perm %d"
 vtd_page_walk_one_skip_map(uint64_t iova, uint64_t mask, uint64_t translated) "iova 0x%"PRIx64" mask 0x%"PRIx64" translated 0x%"PRIx64
 vtd_page_walk_one_skip_unmap(uint64_t iova, uint64_t mask) "iova 0x%"PRIx64" mask 0x%"PRIx64
 vtd_page_walk_skip_read(uint64_t iova, uint64_t next) "Page walk skip iova 0x%"PRIx64" - 0x%"PRIx64" due to unable to read"
 vtd_page_walk_skip_reserve(uint64_t iova, uint64_t next) "Page walk skip iova 0x%"PRIx64" - 0x%"PRIx64" due to rsrv set"
 vtd_switch_address_space(uint8_t bus, uint8_t slot, uint8_t fn, bool on) "Device %02x:%02x.%x switching address space (iommu enabled=%d)"
 vtd_as_unmap_whole(uint8_t bus, uint8_t slot, uint8_t fn, uint64_t iova, uint64_t size) "Device %02x:%02x.%x start 0x%"PRIx64" size 0x%"PRIx64
-vtd_translate_pt(uint16_t sid, uint64_t addr) "source id 0x%"PRIu16", iova 0x%"PRIx64
-vtd_pt_enable_fast_path(uint16_t sid, bool success) "sid 0x%"PRIu16" %d"
+vtd_translate_pt(uint16_t sid, uint64_t addr) "source id 0x%"PRIx16", iova 0x%"PRIx64
+vtd_pt_enable_fast_path(uint16_t sid, bool success) "sid 0x%"PRIx16" %d"
 vtd_irq_generate(uint64_t addr, uint64_t data) "addr 0x%"PRIx64" data 0x%"PRIx64
 vtd_reg_read(uint64_t addr, uint64_t size) "addr 0x%"PRIx64" size 0x%"PRIx64
 vtd_reg_write(uint64_t addr, uint64_t size, uint64_t val) "addr 0x%"PRIx64" size 0x%"PRIx64" value 0x%"PRIx64
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -XXX,XX +XXX,XX @@
 # See docs/devel/tracing.rst for syntax documentation.
 
 # allwinner-cpucfg.c
-allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "id %u, reset_addr 0x%" PRIu32
+allwinner_cpucfg_cpu_reset(uint8_t cpu_id, uint32_t reset_addr) "id %u, reset_addr 0x%" PRIx32
 allwinner_cpucfg_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
 allwinner_cpucfg_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %" PRIu32
 
@@ -XXX,XX +XXX,XX @@ imx7_gpr_write(uint64_t offset, uint64_t value) "addr 0x%08" PRIx64 "value 0x%08
 
 # mos6522.c
 mos6522_set_counter(int index, unsigned int val) "T%d.counter=%d"
-mos6522_get_next_irq_time(uint16_t latch, int64_t d, int64_t delta) "latch=%d counter=0x%"PRId64 " delta_next=0x%"PRId64
+mos6522_get_next_irq_time(uint16_t latch, int64_t d, int64_t delta) "latch=%d counter=0x%"PRIx64 " delta_next=0x%"PRIx64
 mos6522_set_sr_int(void) "set sr_int"
 mos6522_write(uint64_t addr, const char *name, uint64_t val) "reg=0x%"PRIx64 " [%s] val=0x%"PRIx64
 mos6522_read(uint64_t addr, const char *name, unsigned val) "reg=0x%"PRIx64 " [%s] val=0x%x"
diff --git a/hw/scsi/trace-events b/hw/scsi/trace-events
index XXXXXXX..XXXXXXX 100644
--- a/hw/scsi/trace-events
+++ b/hw/scsi/trace-events
@@ -XXX,XX +XXX,XX @@ lsi_bad_phase_interrupt(void) "Phase mismatch interrupt"
 lsi_bad_selection(uint32_t id) "Selected absent target %"PRIu32
 lsi_do_dma_unavailable(void) "DMA no data available"
 lsi_do_dma(uint64_t addr, int len) "DMA addr=0x%"PRIx64" len=%d"
-lsi_queue_command(uint32_t tag) "Queueing tag=0x%"PRId32
+lsi_queue_command(uint32_t tag) "Queueing tag=0x%"PRIx32
 lsi_add_msg_byte_error(void) "MSG IN data too long"
 lsi_add_msg_byte(uint8_t data) "MSG IN 0x%02x"
 lsi_reselect(int id) "Reselected target %d"
@@ -XXX,XX +XXX,XX @@ lsi_do_msgout_noop(void) "MSG: No Operation"
 lsi_do_msgout_extended(uint8_t msg, uint8_t len) "Extended message 0x%x (len %d)"
 lsi_do_msgout_ignored(const char *msg) "%s (ignored)"
 lsi_do_msgout_simplequeue(uint8_t select_tag) "SIMPLE queue tag=0x%x"
-lsi_do_msgout_abort(uint32_t tag) "MSG: ABORT TAG tag=0x%"PRId32
+lsi_do_msgout_abort(uint32_t tag) "MSG: ABORT TAG tag=0x%"PRIx32
 lsi_do_msgout_clearqueue(uint32_t tag) "MSG: CLEAR QUEUE tag=0x%"PRIx32
 lsi_do_msgout_busdevicereset(uint32_t tag) "MSG: BUS DEVICE RESET tag=0x%"PRIx32
 lsi_do_msgout_select(int id) "Select LUN %d"
-- 
2.35.1