[PATCH 5/6] disas/cris: Remove struct cris_disasm_data

[Richard Henderson]

As the structure contains one element, pass the element around
instead of the structure.

Signed-off-by: Richard Henderson 
---
 disas/cris.c | 187 +--
 1 file changed, 76 insertions(+), 111 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index 692cd4d163..71c292188a 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -1239,30 +1239,11 @@ cris_cc_strings[] =
 enum cris_disass_family
  { cris_dis_v0_v10, cris_dis_common_v10_v32, cris_dis_v32 };
 
-/* Stored in the disasm_info->private_data member.  */
-struct cris_disasm_data
-{
-  /* Whether this code is flagged as crisv32.  FIXME: Should be an enum
- that includes "compatible".  */
-  enum cris_disass_family distype;
-};
-
-static int cris_constraint
-  (const char *, unsigned, unsigned, struct cris_disasm_data *);
-
-/* Parse disassembler options and store state in info.  FIXME: For the
-   time being, we abuse static variables.  */
-
-static void
-cris_parse_disassembler_options (struct cris_disasm_data *disdata,
- char *disassembler_options,
- enum cris_disass_family distype)
-{
-  disdata->distype = distype;
-}
+static int cris_constraint(const char *, unsigned, unsigned,
+   enum cris_disass_family);
 
 static const struct cris_spec_reg *
-spec_reg_info (unsigned int sreg, enum cris_disass_family distype)
+spec_reg_info(unsigned int sreg, enum cris_disass_family distype)
 {
   int i;
 
@@ -1309,9 +1290,9 @@ number_of_bits (unsigned int val)
 /* Get an entry in the opcode-table.  */
 
 static const struct cris_opcode *
-get_opcode_entry (unsigned int insn,
-  unsigned int prefix_insn,
-  struct cris_disasm_data *disdata)
+get_opcode_entry(unsigned int insn,
+ unsigned int prefix_insn,
+ enum cris_disass_family distype)
 {
   /* For non-prefixed insns, we keep a table of pointers, indexed by the
  insn code.  Each entry is initialized when found to be NULL.  */
@@ -1349,7 +1330,7 @@ get_opcode_entry (unsigned int insn,
   const struct cris_opcode *popcodep
 = (opc_table[prefix_insn] != NULL
? opc_table[prefix_insn]
-   : get_opcode_entry (prefix_insn, NO_CRIS_PREFIX, disdata));
+   : get_opcode_entry(prefix_insn, NO_CRIS_PREFIX, distype));
 
   if (popcodep == NULL)
 return NULL;
@@ -1406,7 +1387,7 @@ get_opcode_entry (unsigned int insn,
 {
   int level_of_match;
 
-  if (disdata->distype == cris_dis_v32)
+  if (distype == cris_dis_v32)
 {
   switch (opcodep->applicable_version)
 {
@@ -1469,10 +1450,8 @@ get_opcode_entry (unsigned int insn,
   if ((opcodep->match & insn) == opcodep->match
   && (opcodep->lose & insn) == 0
   && ((level_of_match
-   = cris_constraint (opcodep->args,
-  insn,
-  prefix_insn,
-  disdata))
+   = cris_constraint(opcodep->args, insn,
+ prefix_insn, distype))
   >= 0)
   && ((level_of_match
+= 2 * number_of_bits (opcodep->match
@@ -1509,10 +1488,10 @@ get_opcode_entry (unsigned int insn,
indicating the confidence in the match (higher is better).  */
 
 static int
-cris_constraint (const char *cs,
- unsigned int insn,
- unsigned int prefix_insn,
- struct cris_disasm_data *disdata)
+cris_constraint(const char *cs,
+unsigned int insn,
+unsigned int prefix_insn,
+enum cris_disass_family distype)
 {
   int retval = 0;
   int tmp;
@@ -1526,7 +1505,7 @@ cris_constraint (const char *cs,
 /* Do not recognize "pop" if there's a prefix and then only for
v0..v10.  */
 if (prefix_insn != NO_CRIS_PREFIX
-|| disdata->distype != cris_dis_v0_v10)
+|| distype != cris_dis_v0_v10)
   return -1;
 break;
 
@@ -1569,7 +1548,7 @@ cris_constraint (const char *cs,
 if (insn & 0x400)
   {
 const struct cris_opcode *prefix_opcodep
-  = get_opcode_entry (prefix_insn, NO_CRIS_PREFIX, disdata);
+  = get_opcode_entry(prefix_insn, NO_CRIS_PREFIX, distype);
 
 if (prefix_opcodep->match == DIP_OPCODE)
   return -1;
@@ -1589,7 +1568,7 @@ cris_constraint (const char *cs,
   {
 /* Match the prefix insn to BDAPQ.  */
 const struct cris_opcode *prefix_opcodep
-  = get_opcode_entry (prefix_insn, NO_CRIS_PREFIX, disdata);
+  = get_opcode_entry(prefix_insn, NO_CRIS_PREFIX, distype);
 
 if (prefix_opcodep->match == BDAP_QUICK_OPCODE)
   {
@@ -1

[PATCH 3/6] disas/cris: Drop with_reg_prefix

[Richard Henderson]

The *_without_reg_prefix functions are all commented out.
Remove them, remove all 'with_reg_prefix' parameters,
and remove all of the conditions that test them.

Signed-off-by: Richard Henderson 
---
 disas/cris.c | 188 +--
 1 file changed, 32 insertions(+), 156 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index 1cc8752104..27f71a8257 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -1692,13 +1692,11 @@ format_dec (long number, char *outbuffer, size_t 
outsize, int signedp)
 static char *
 format_reg (struct cris_disasm_data *disdata,
 int regno,
-char *outbuffer_start,
-bfd_boolean with_reg_prefix)
+char *outbuffer_start)
 {
   char *outbuffer = outbuffer_start;
 
-  if (with_reg_prefix)
-*outbuffer++ = REGISTER_PREFIX_CHAR;
+  *outbuffer++ = REGISTER_PREFIX_CHAR;
 
   switch (regno)
 {
@@ -1726,14 +1724,12 @@ format_reg (struct cris_disasm_data *disdata,
 
 static char *
 format_sup_reg (unsigned int regno,
-char *outbuffer_start,
-bfd_boolean with_reg_prefix)
+char *outbuffer_start)
 {
   char *outbuffer = outbuffer_start;
   int i;
 
-  if (with_reg_prefix)
-*outbuffer++ = REGISTER_PREFIX_CHAR;
+  *outbuffer++ = REGISTER_PREFIX_CHAR;
 
   for (i = 0; cris_support_regs[i].name != NULL; i++)
 if (cris_support_regs[i].number == regno)
@@ -1845,8 +1841,7 @@ print_with_operands (const struct cris_opcode *opcodep,
 it.  */
  const struct cris_opcode *prefix_opcodep,
  unsigned int prefix_insn,
- unsigned char *prefix_buffer,
- bfd_boolean with_reg_prefix)
+ unsigned char *prefix_buffer)
 {
   /* Get a buffer of somewhat reasonable size where we store
  intermediate parts of the insn.  */
@@ -1908,12 +1903,11 @@ print_with_operands (const struct cris_opcode *opcodep,
 switch (*s)
   {
   case 'T':
-tp = format_sup_reg ((insn >> 12) & 15, tp, with_reg_prefix);
+tp = format_sup_reg ((insn >> 12) & 15, tp);
 break;
 
   case 'A':
-if (with_reg_prefix)
-  *tp++ = REGISTER_PREFIX_CHAR;
+*tp++ = REGISTER_PREFIX_CHAR;
 *tp++ = 'a';
 *tp++ = 'c';
 *tp++ = 'r';
@@ -1945,11 +1939,11 @@ print_with_operands (const struct cris_opcode *opcodep,
 
   case 'D':
   case 'r':
-tp = format_reg (disdata, insn & 15, tp, with_reg_prefix);
+tp = format_reg (disdata, insn & 15, tp);
 break;
 
   case 'R':
-tp = format_reg (disdata, (insn >> 12) & 15, tp, with_reg_prefix);
+tp = format_reg (disdata, (insn >> 12) & 15, tp);
 break;
 
   case 'n':
@@ -2132,7 +2126,7 @@ print_with_operands (const struct cris_opcode *opcodep,
   {
 if (insn & 0x400)
   {
-tp = format_reg (disdata, insn & 15, tp, with_reg_prefix);
+tp = format_reg (disdata, insn & 15, tp);
 *tp++ = '=';
   }
 
@@ -2174,8 +2168,7 @@ print_with_operands (const struct cris_opcode *opcodep,
 info->target2 = prefix_insn & 15;
 
 *tp++ = '[';
-tp = format_reg (disdata, prefix_insn & 15, tp,
- with_reg_prefix);
+tp = format_reg (disdata, prefix_insn & 15, tp);
 if (prefix_insn & 0x400)
   *tp++ = '+';
 *tp++ = ']';
@@ -2191,8 +2184,7 @@ print_with_operands (const struct cris_opcode *opcodep,
 number -= 256;
 
   /* Output "reg+num" or, if num < 0, "reg-num".  */
-  tp = format_reg (disdata, (prefix_insn >> 12) & 15, tp,
-   with_reg_prefix);
+  tp = format_reg (disdata, (prefix_insn >> 12) & 15, tp);
   if (number >= 0)
 *tp++ = '+';
   tp = FORMAT_DEC (number, tp, 1);
@@ -2205,11 +2197,9 @@ print_with_operands (const struct cris_opcode *opcodep,
 
   case BIAP_OPCODE:
 /* Output "r+R.m".  */
-tp = format_reg (disdata, prefix_insn & 15, tp,
- with_reg_prefix);
+tp = format_reg (disdata, prefix_insn & 15, tp);
 *tp++ = '+';
-tp = format_reg (disdata, (prefix_insn >> 12) & 15, tp,
- with_reg_prefix);
+tp = format_reg (disdata, (prefix_insn >> 12) & 15, tp);
 *tp++ = '.';
 *tp++ = mode_char[(prefix_insn >> 4) & 3];
 
@@ -2226,8 +2216,7 @@ print_with_operands (const struct cris_opcode *opcodep,
  

[PATCH 1/6] disas/cris: Untabify

[Richard Henderson]

Nothing but whitespace changes.

Signed-off-by: Richard Henderson 
---
 disas/cris.c | 2266 +-
 1 file changed, 1133 insertions(+), 1133 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index 409a224c5d..d62f9e3264 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -53,64 +53,64 @@ along with this program; if not, see 
.  */
 const struct cris_spec_reg
 cris_spec_regs[] =
 {
-  {"bz",  0,  1, cris_ver_v32p,   NULL},
-  {"p0",  0,  1, 0,   NULL},
-  {"vr",  1,  1, 0,   NULL},
-  {"p1",  1,  1, 0,   NULL},
+  {"bz",  0,  1, cris_ver_v32p,NULL},
+  {"p0",  0,  1, 0,NULL},
+  {"vr",  1,  1, 0,NULL},
+  {"p1",  1,  1, 0,NULL},
   {"pid", 2,  1, cris_ver_v32p,NULL},
-  {"p2",  2,  1, cris_ver_v32p,   NULL},
+  {"p2",  2,  1, cris_ver_v32p,NULL},
   {"p2",  2,  1, cris_ver_warning, NULL},
   {"srs", 3,  1, cris_ver_v32p,NULL},
-  {"p3",  3,  1, cris_ver_v32p,   NULL},
+  {"p3",  3,  1, cris_ver_v32p,NULL},
   {"p3",  3,  1, cris_ver_warning, NULL},
-  {"wz",  4,  2, cris_ver_v32p,   NULL},
-  {"p4",  4,  2, 0,   NULL},
+  {"wz",  4,  2, cris_ver_v32p,NULL},
+  {"p4",  4,  2, 0,NULL},
   {"ccr", 5,  2, cris_ver_v0_10,   NULL},
-  {"exs", 5,  4, cris_ver_v32p,   NULL},
+  {"exs", 5,  4, cris_ver_v32p,NULL},
   {"p5",  5,  2, cris_ver_v0_10,   NULL},
-  {"p5",  5,  4, cris_ver_v32p,   NULL},
-  {"dcr0",6,  2, cris_ver_v0_3,   NULL},
-  {"eda", 6,  4, cris_ver_v32p,   NULL},
-  {"p6",  6,  2, cris_ver_v0_3,   NULL},
-  {"p6",  6,  4, cris_ver_v32p,   NULL},
+  {"p5",  5,  4, cris_ver_v32p,NULL},
+  {"dcr0",6,  2, cris_ver_v0_3,NULL},
+  {"eda", 6,  4, cris_ver_v32p,NULL},
+  {"p6",  6,  2, cris_ver_v0_3,NULL},
+  {"p6",  6,  4, cris_ver_v32p,NULL},
   {"dcr1/mof", 7, 4, cris_ver_v10p,
"Register `dcr1/mof' with ambiguous size specified.  Guessing 4 bytes"},
   {"dcr1/mof", 7, 2, cris_ver_v0_3,
"Register `dcr1/mof' with ambiguous size specified.  Guessing 2 bytes"},
-  {"mof", 7,  4, cris_ver_v10p,   NULL},
-  {"dcr1",7,  2, cris_ver_v0_3,   NULL},
-  {"p7",  7,  4, cris_ver_v10p,   NULL},
-  {"p7",  7,  2, cris_ver_v0_3,   NULL},
-  {"dz",  8,  4, cris_ver_v32p,   NULL},
-  {"p8",  8,  4, 0,   NULL},
+  {"mof", 7,  4, cris_ver_v10p,NULL},
+  {"dcr1",7,  2, cris_ver_v0_3,NULL},
+  {"p7",  7,  4, cris_ver_v10p,NULL},
+  {"p7",  7,  2, cris_ver_v0_3,NULL},
+  {"dz",  8,  4, cris_ver_v32p,NULL},
+  {"p8",  8,  4, 0,NULL},
   {"ibr", 9,  4, cris_ver_v0_10,   NULL},
-  {"ebp", 9,  4, cris_ver_v32p,   NULL},
-  {"p9",  9,  4, 0,   NULL},
+  {"ebp", 9,  4, cris_ver_v32p,NULL},
+  {"p9",  9,  4, 0,NULL},
   {"irp", 10, 4, cris_ver_v0_10,   NULL},
-  {"erp", 10, 4, cris_ver_v32p,   NULL},
-  {"p10", 10, 4, 0,   NULL},
-  {"srp", 11, 4, 0,   NULL},
-  {"p11", 11, 4, 0,   NULL},
+  {"erp", 10, 4, cris_ver_v32p,NULL},
+  {"p10", 10, 4, 0,NULL},
+  {"srp", 11, 4, 0,NULL},
+  {"p11", 11, 4, 0,NULL},
   /* For disassembly use only.  Accept at assembly with a warning.  */
   {"bar/dtp0", 12, 4, cris_ver_warning,
"Ambiguous register `bar/dtp0' specified"},
-  {"nrp", 12, 4, cris_ver_v32p,   NULL},
+  {"nrp", 12, 4, cris_ver_v32p,NULL},
   {"bar", 12, 4, cris_ver_v8_10,   NULL},
-  {"dtp0",12, 4, cris_ver_v0_3,   NULL},
-  {"p12", 12, 4, 0,   NULL},
+  {"dtp0",12, 4, cris_ver_v0_3,NULL},
+  {"p12", 12, 4, 0,NULL},
   /* For disassembly use only.  Accept at assembly with a warning.  */
   {"dccr/dtp1",13, 4, cris_ver_warning,
"Ambiguous register `dccr/dtp1' specified"},
-  {"ccs", 13, 4, cris_ver_v32p,   NULL},
+  {"ccs", 13, 4, cris_ver_v32p,NULL},
   {"dccr",13, 4, cris_ver_v8_10,   NULL},
-  {"dtp1",13, 4, cris_ver_v0_3,   NULL},
-  {"p13", 13, 4, 0,   NULL},
+  {"dtp1",13, 4, cris_ver_v0_3,NULL},
+  {"p13", 13, 4, 0,NULL},
   {"brp", 14, 4, cris_ver_v3_10,   NULL},
-  {"usp", 14, 4, cris_ver_v32p,   NULL},
-  {"p14", 14, 4, cris_ver_v3p,NULL},
-  {"usp", 15, 4, cris_ver_v10,NULL},
-  {"spc", 15, 4, cris_ver_v32p,   NULL},
-  {"p15", 15, 4, cris_ver_v10p,   NULL},
+  {"usp", 14, 4, cris_ver_v32p,NULL},
+  {"p14", 14, 4, cris_ver_v3p, NULL},
+  {"usp", 15, 4, cris_ver_v10, NULL},
+  {"spc", 15, 4, cris_ver_v32p,NULL},
+  {"p15", 15, 4, cris_ver_v10p,NULL},
   {NULL, 0, 0, cris_ver_version_all, NULL}
 };
 
@@ -151,53 +151,53 @@ const struct cris_support_reg cris_support_regs[] =
  Operand-matching characters:
  [ ] , space
  

[PATCH 4/6] disas/cris: Use GString in print_with_operands and subroutines

[Richard Henderson]

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Use GString in the core of the disassembler instead of buffering
the string locally.

Instead of info->print_address_func, use format_hex for addresses.
Printing a hex number is what print_address_func does, and using
format_hex properly truncates the 32-bit bit address.  E.g.

-0x00080988:  move.d 0xfeda49ff,$r4
+0x00080988:  move.d 0xfeda49ff,$r4

Signed-off-by: Richard Henderson 
---
 disas/cris.c | 288 +--
 1 file changed, 96 insertions(+), 192 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index 27f71a8257..692cd4d163 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -1663,85 +1663,74 @@ cris_constraint (const char *cs,
 
 /* Format number as hex with a leading "0x" into outbuffer.  */
 
-static char *
-format_hex (unsigned long number,
-char *outbuffer)
+static void
+format_hex(unsigned long number, GString *str)
 {
   /* Truncate negative numbers on >32-bit hosts.  */
   number &= 0x;
 
-  sprintf (outbuffer, "0x%lx", number);
-
-  return outbuffer + strlen (outbuffer);
+  g_string_append_printf(str, "0x%lx", number);
 }
 
 /* Format number as decimal into outbuffer.  Parameter signedp says
whether the number should be formatted as signed (!= 0) or
unsigned (== 0).  */
 
-static char *
-format_dec (long number, char *outbuffer, size_t outsize, int signedp)
+static void
+format_dec(long number, GString *str, int signedp)
 {
-  snprintf (outbuffer, outsize, signedp ? "%ld" : "%lu", number);
-
-  return outbuffer + strlen (outbuffer);
+  if (signedp)
+g_string_append_printf(str, "%ld", number);
+  else
+g_string_append_printf(str, "%lu", number);
 }
 
 /* Format the name of the general register regno into outbuffer.  */
 
-static char *
-format_reg (struct cris_disasm_data *disdata,
-int regno,
-char *outbuffer_start)
+static void
+format_reg(struct cris_disasm_data *disdata, int regno, GString *str)
 {
-  char *outbuffer = outbuffer_start;
-
-  *outbuffer++ = REGISTER_PREFIX_CHAR;
+  g_string_append_c(str, REGISTER_PREFIX_CHAR);
 
   switch (regno)
 {
 case 15:
   /* For v32, there is no context in which we output PC.  */
   if (disdata->distype == cris_dis_v32)
-strcpy (outbuffer, "acr");
+g_string_append(str, "acr");
   else
-strcpy (outbuffer, "pc");
+g_string_append(str, "pc");
   break;
 
 case 14:
-  strcpy (outbuffer, "sp");
+  g_string_append(str, "sp");
   break;
 
 default:
-  sprintf (outbuffer, "r%d", regno);
+  g_string_append_printf(str, "r%d", regno);
   break;
 }
-
-  return outbuffer_start + strlen (outbuffer_start);
 }
 
 /* Format the name of a support register into outbuffer.  */
 
-static char *
-format_sup_reg (unsigned int regno,
-char *outbuffer_start)
+static void
+format_sup_reg(unsigned int regno, GString *str)
 {
-  char *outbuffer = outbuffer_start;
   int i;
 
-  *outbuffer++ = REGISTER_PREFIX_CHAR;
+  g_string_append_c(str, REGISTER_PREFIX_CHAR);
 
   for (i = 0; cris_support_regs[i].name != NULL; i++)
 if (cris_support_regs[i].number == regno)
   {
-sprintf (outbuffer, "%s", cris_support_regs[i].name);
-return outbuffer_start + strlen (outbuffer_start);
+g_string_append(str, cris_support_regs[i].name);
+return;
   }
 
   /* There's supposed to be register names covering all numbers, though
  some may be generic names.  */
-  sprintf (outbuffer, "format_sup_reg-BUG");
-  return outbuffer_start + strlen (outbuffer_start);
+  g_string_append(str, "format_sup_reg-BUG");
 }
 
 /* Return the length of an instruction.  */
@@ -1797,8 +1786,8 @@ bytes_to_skip (unsigned int insn,
 
 /* Print condition code flags.  */
 
-static char *
-print_flags (struct cris_disasm_data *disdata, unsigned int insn, char *cp)
+static void
+print_flags(struct cris_disasm_data *disdata, unsigned int insn, GString *str)
 {
   /* Use the v8 (Etrax 100) flag definitions for disassembly.
  The differences with v0 (Etrax 1..4) vs. Svinto are:
@@ -1815,17 +1804,9 @@ print_flags (struct cris_disasm_data *disdata, unsigned 
int insn, char *cp)
 
   for (i = 0; i < 8; i++)
 if (flagbits & (1 << i))
-  *cp++ = fnames[i];
-
-  return cp;
+  g_string_append_c(str, fnames[i]);
 }
 
-#define FORMAT_DEC(number, tp, signedp)  \
-format_dec (number, tp, ({\
-assert(tp >= temp && tp <= temp + sizeof(temp)); \
-temp + sizeof(temp) - tp;\
-}), signedp)
-
 /* Print out an insn with its operands, and update the info->insn_type
fields.  The prefix_opcodep and the rest hold a prefix insn that is
supposed to be output as an address mode.  */
@@ -1843,19 +1824,13 @@ print_with_operands (const struct cris_opcode *opcodep,
  unsigned in

[PATCH 0/6] disas/cris: Use GString instead of sprintf

[Richard Henderson]

More sprintf cleanup encouraged by the Apple deprecation.
Probably there's a more minimal patch.  On the other hand,
there's certainly a larger cleanup possible.


r~


Richard Henderson (6):
  disas/cris: Untabify
  disas/cris: Remove TRACE_CASE and related code
  disas/cris: Drop with_reg_prefix
  disas/cris: Use GString in print_with_operands and subroutines
  disas/cris: Remove struct cris_disasm_data
  disas/cris: Improve output of register names

 disas/cris.c | 2498 +-
 1 file changed, 1070 insertions(+), 1428 deletions(-)

-- 
2.34.1

[PATCH 6/6] disas/cris: Improve output of register names

[Richard Henderson]

Add REGISTER_PREFIX as a string literal that may be concatenated
with other string literals.  Use a table of the 16 register names
instead of using g_string_append_printf.

Signed-off-by: Richard Henderson 
---
 disas/cris.c | 45 +
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index 71c292188a..01ea63c428 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -1234,6 +1234,7 @@ cris_cc_strings[] =
 #endif
 
 /* Sometimes we prefix all registers with this character.  */
+#define REGISTER_PREFIX  "$"
 #define REGISTER_PREFIX_CHAR '$'
 
 enum cris_disass_family
@@ -1669,26 +1670,31 @@ format_dec(long number, GString *str, int signedp)
 static void
 format_reg(enum cris_disass_family distype, int regno, GString *str)
 {
-  g_string_append_c(str, REGISTER_PREFIX_CHAR);
+  static const char reg_names[16][5] = {
+REGISTER_PREFIX "r0",
+REGISTER_PREFIX "r1",
+REGISTER_PREFIX "r2",
+REGISTER_PREFIX "r3",
+REGISTER_PREFIX "r4",
+REGISTER_PREFIX "r5",
+REGISTER_PREFIX "r6",
+REGISTER_PREFIX "r7",
+REGISTER_PREFIX "r8",
+REGISTER_PREFIX "r9",
+REGISTER_PREFIX "r10",
+REGISTER_PREFIX "r11",
+REGISTER_PREFIX "r12",
+REGISTER_PREFIX "r13",
+REGISTER_PREFIX "sp",
+REGISTER_PREFIX "pc",
+  };
+  const char *name = reg_names[regno];
 
-  switch (regno)
-{
-case 15:
-  /* For v32, there is no context in which we output PC.  */
-  if (distype == cris_dis_v32)
-g_string_append(str, "acr");
-  else
-g_string_append(str, "pc");
-  break;
+  /* For v32, there is no context in which we output PC.  */
+  if (regno == 15 && distype == cris_dis_v32)
+name = REGISTER_PREFIX "acr";
 
-case 14:
-  g_string_append(str, "sp");
-  break;
-
-default:
-  g_string_append_printf(str, "r%d", regno);
-  break;
-}
+  g_string_append(str, name);
 }
 
 /* Format the name of a support register into outbuffer.  */
@@ -1861,8 +1867,7 @@ print_with_operands(const struct cris_opcode *opcodep,
 break;
 
   case 'A':
-g_string_append_c(str, REGISTER_PREFIX_CHAR);
-g_string_append(str, "acr");
+g_string_append(str, REGISTER_PREFIX "acr");
 break;
 
   case '[':
-- 
2.34.1

[PATCH 2/6] disas/cris: Remove TRACE_CASE and related code

[Richard Henderson]

The disassembler_options variable is never set within QEMU,
so this code is never enabled.  Remove it all.

Signed-off-by: Richard Henderson 
---
 disas/cris.c | 114 ++-
 1 file changed, 3 insertions(+), 111 deletions(-)

diff --git a/disas/cris.c b/disas/cris.c
index d62f9e3264..1cc8752104 100644
--- a/disas/cris.c
+++ b/disas/cris.c
@@ -1236,58 +1236,17 @@ cris_cc_strings[] =
 /* Sometimes we prefix all registers with this character.  */
 #define REGISTER_PREFIX_CHAR '$'
 
-/* Whether or not to trace the following sequence:
-   sub* X,r%d
-   bound* Y,r%d
-   adds.w [pc+r%d.w],pc
-
-   This is the assembly form of a switch-statement in C.
-   The "sub is optional.  If there is none, then X will be zero.
-   X is the value of the first case,
-   Y is the number of cases (including default).
-
-   This results in case offsets printed on the form:
-case N: -> case_address
-   where N is an estimation on the corresponding 'case' operand in C,
-   and case_address is where execution of that case continues after the
-   sequence presented above.
-
-   The old style of output was to print the offsets as instructions,
-   which made it hard to follow "case"-constructs in the disassembly,
-   and caused a lot of annoying warnings about undefined instructions.
-
-   FIXME: Make this optional later.  */
-#ifndef TRACE_CASE
-#define TRACE_CASE (disdata->trace_case)
-#endif
-
 enum cris_disass_family
  { cris_dis_v0_v10, cris_dis_common_v10_v32, cris_dis_v32 };
 
 /* Stored in the disasm_info->private_data member.  */
 struct cris_disasm_data
 {
-  /* Whether to print something less confusing if we find something
- matching a switch-construct.  */
-  bfd_boolean trace_case;
-
   /* Whether this code is flagged as crisv32.  FIXME: Should be an enum
  that includes "compatible".  */
   enum cris_disass_family distype;
 };
 
-/* Value of first element in switch.  */
-static long case_offset = 0;
-
-/* How many more case-offsets to print.  */
-static long case_offset_counter = 0;
-
-/* Number of case offsets.  */
-static long no_of_case_offsets = 0;
-
-/* Candidate for next case_offset.  */
-static long last_immediate = 0;
-
 static int cris_constraint
   (const char *, unsigned, unsigned, struct cris_disasm_data *);
 
@@ -1299,11 +1258,6 @@ cris_parse_disassembler_options (struct cris_disasm_data 
*disdata,
  char *disassembler_options,
  enum cris_disass_family distype)
 {
-  /* Default true.  */
-  disdata->trace_case
-= (disassembler_options == NULL
-   || (strcmp (disassembler_options, "nocase") != 0));
-
   disdata->distype = distype;
 }
 
@@ -1711,18 +1665,13 @@ cris_constraint (const char *cs,
 
 static char *
 format_hex (unsigned long number,
-char *outbuffer,
-struct cris_disasm_data *disdata)
+char *outbuffer)
 {
   /* Truncate negative numbers on >32-bit hosts.  */
   number &= 0x;
 
   sprintf (outbuffer, "0x%lx", number);
 
-  /* Save this value for the "case" support.  */
-  if (TRACE_CASE)
-last_immediate = number;
-
   return outbuffer + strlen (outbuffer);
 }
 
@@ -1733,7 +1682,6 @@ format_hex (unsigned long number,
 static char *
 format_dec (long number, char *outbuffer, size_t outsize, int signedp)
 {
-  last_immediate = number;
   snprintf (outbuffer, outsize, signedp ? "%ld" : "%lu", number);
 
   return outbuffer + strlen (outbuffer);
@@ -2138,7 +2086,7 @@ print_with_operands (const struct cris_opcode *opcodep,
 info->target = number;
   }
 else
-  tp = format_hex (number, tp, disdata);
+  tp = format_hex (number, tp);
   }
   }
 else
@@ -2273,11 +2221,6 @@ print_with_operands (const struct cris_opcode *opcodep,
  ? CRIS_DIS_FLAG_MEM_TARGET2_MULT4
  : ((prefix_insn & 0x8000)
 ? CRIS_DIS_FLAG_MEM_TARGET2_MULT2 : 0)));
-
-/* Is it the casejump?  It's a "adds.w [pc+r%d.w],pc".  */
-if (insn == 0xf83f && (prefix_insn & ~0xf000) == 0x55f)
-  /* Then start interpreting data as offsets.  */
-  case_offset_counter = no_of_case_offsets;
 break;
 
   case BDAP_INDIR_OPCODE:
@@ -2514,31 +2457,6 @@ print_with_operands (const struct cris_opcode *opcodep,
prefix_opcodep->name, prefix_opcodep->args);
 
   (*info->fprintf_func) (info->stream, "%s", temp);
-
-  /* Get info for matching case-tables, if we don't have any active.
- We assume that the last constant seen is used; either in the insn
- itself or in a "move.d const,rN, sub.d rN,rM"-like sequence.  */
-  if (TRACE_CASE && case_offset_counter == 0)
-{
-  if (CONST_STRNEQ (opcodep->name, "sub"))
-case_offset = last_imme

Re: [PATCH v6 10/12] hw/mem/cxl_type3: Add dpa range validation for accesses to DC regions

[Gregory Price]

On Mon, Mar 25, 2024 at 12:02:28PM -0700, nifan@gmail.com wrote:
> From: Fan Ni 
> 
> All dpa ranges in the DC regions are invalid to access until an extent
> covering the range has been added. Add a bitmap for each region to
> record whether a DC block in the region has been backed by DC extent.
> For the bitmap, a bit in the bitmap represents a DC block. When a DC
> extent is added, all the bits of the blocks in the extent will be set,
> which will be cleared when the extent is released.
> 
> Reviewed-by: Jonathan Cameron 
> Signed-off-by: Fan Ni 
> ---
>  hw/cxl/cxl-mailbox-utils.c  |  6 +++
>  hw/mem/cxl_type3.c  | 76 +
>  include/hw/cxl/cxl_device.h |  7 
>  3 files changed, 89 insertions(+)
> 
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 7094e007b9..a0d2239176 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -1620,6 +1620,7 @@ static CXLRetCode cmd_dcd_add_dyn_cap_rsp(const struct 
> cxl_cmd *cmd,
>  
>  cxl_insert_extent_to_extent_list(extent_list, dpa, len, NULL, 0);
>  ct3d->dc.total_extent_count += 1;
> +ct3_set_region_block_backed(ct3d, dpa, len);
>  
>  ent = QTAILQ_FIRST(&ct3d->dc.extents_pending);
>  cxl_remove_extent_from_extent_list(&ct3d->dc.extents_pending, ent);

while looking at the MHD code, we had decided to "reserve" the blocks in
the bitmap in the call to `qmp_cxl_process_dynamic_capacity` in order to
prevent a potential double-allocation (basically we need to sanity check
that two hosts aren't reserving the region PRIOR to the host being
notified).

I did not see any checks in the `qmp_cxl_process_dynamic_capacity` path
to prevent pending extents from being double-allocated.  Is this an
explicit choice?

I can see, for example, why you may want to allow the following in the
pending list: [Add X, Remove X, Add X].  I just want to know if this is
intentional or not. If not, you may consider adding a pending check
during the sanity check phase of `qmp_cxl_process_dynamic_capacity`

~Gregory

Re: [PULL 0/1] target/sparc late fix

[Richard Henderson]

On 4/12/24 11:54, Richard Henderson wrote:

Since this problem has 4 issues open, let's get it for 9.0.


r~


The following changes since commit be72d6ab361a26878752467a17289066dfd5bc28:


I've updated the tag to 2786a3f8d3a047cc21271380324c0b7d8217f238
to include M Bazz's tested-by tag.


r~



   Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging 
(2024-04-12 16:01:04 +0100)

are available in the Git repository at:

   https://gitlab.com/rth7680/qemu.git tags/pull-sp-20240412

for you to fetch changes up to c84f5198b0b676ad67962b5250af1b0d0842e319:

   target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT (2024-04-12 
11:48:26 -0700)


target/sparc: Fix ASI_USERTXT for Solaris gdb crashes


Richard Henderson (1):
   target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT

  target/sparc/helper.h  |  3 +++
  target/sparc/ldst_helper.c | 65 --
  target/sparc/translate.c   | 48 --
  3 files changed, 94 insertions(+), 22 deletions(-)

Re: [PATCH 1/4] Revert "migration: modify test_multifd_tcp_none() to use new QAPI syntax"

[Peter Xu]

On Fri, Apr 12, 2024 at 11:58:43AM -0300, Fabiano Rosas wrote:
> Peter Xu  writes:
> 
> > On Thu, Apr 11, 2024 at 04:41:16PM -0300, Fabiano Rosas wrote:
> >> Peter Xu  writes:
> >> 
> >> > On Thu, Apr 11, 2024 at 11:31:08PM +0530, Het Gala wrote:
> >> >> I just wanted to highlight couple of pointers:
> >> >> 1. though we are using 'channels' in the precopy tests for 'migrate' 
> >> >> QAPI,
> >> >> we
> >> >>    use the old uri for 'migrate-incoming' QAPI.
> >> >> 2. We do not cover other 'channels' abi, only have tcp path tested.
> >> >> 
> >> >> So, the TO-DOs could be:
> >> >> 1. Omit the 4th patch here, which introduced postcopy qtests with 
> >> >> 'channels'
> >> >>    interface OR have 'channels' interface with other than tcp transport
> >> >>    (file, exec, vsock, etc) so as to cover different code paths.
> >> >> 2. Extend channels interface to migrate-incoming QAPI for precopy qtests
> >> >
> >> > You can see whether Fabiano has anything to say, but what you proposed
> >> > looks good to me.
> >> 
> >> Ok, so what about we convert some of the 'plain' tests into channels to
> >> cover all transports?
> >> 
> >> - tcp: test_multifd_tcp_none  (this one we already did)
> >> - file: test_precopy_file
> >> - unix: test_precopy_unix_plain
> >> - exec: test_analyze_script
> >> - fd: test_migrate_precopy_fd_socket
> >> 
> >> Those^, plus the validate_uri that's already in next should cover
> >> everything.
> >> 
> >> We don't need to do this at once, by the way.
> >> 
> >> Moreover:
> >> 
> >> - leave all test strings untouched to preserve bisecting;
> >> 
> >> - let's not bother adding "channels" and "uri" to the test string
> >>   anymore. The channels API should be taken for granted at this point, I
> >>   don't expect we start hitting bugs that will require us to run either
> >>   foo/uri/plain or foo/channels/plain, so there's not much point in
> >>   making the distinction.
> >
> > Do you mean we can put "uri:" aside?  Maybe I misunderstood..
> 
> I mean the test name does not need to specify "channels" vs. "uri"
> because that should never be broken to the point that we actually need
> to go fetch those tests by name. We'd still have at least 1 test for
> each transport with channels and (existing) at least 1 test for each
> transport with uri.
> 
> >
> > The matrix previously was (I think.. when this series posted):
> >
> >   [tcp, unix, file, exec, fd] x [uri, channels] x [precopy, postcopy]
> >
> > Drop postcopy as doesn't seem to have any special paths:
> >
> >   [tcp, unix, file, exec, fd] x [uri, channels]
> >
> > So logically we should still cover these, right?
> 
> Right, I'm just suggesting we convert some tests to use channels, one
> for each transport, to test the channels API in full. The rest of the
> existing tests as well as future tests need not have a uri (or channel)
> variant. Just one of them is enough.

Ah so that's the "test string"; sounds all good.

-- 
Peter Xu

Re: [PATCH v2] ppc440_pcix: Do not expose a bridge device on PCI bus

[BALATON Zoltan]

On Thu, 11 Apr 2024, BALATON Zoltan wrote:

Real 460EX SoC apparently does not expose a bridge device and having
it appear on PCI bus confuses an AmigaOS file system driver that uses
this to detect which machine it is running on.

Signed-off-by: BALATON Zoltan 
---
Here's another version that keeps the values and only drops the device
so it's even less likely it could break anything, in case this can be
accepted for 9.0.


Looks like there will be an rc4 so can this one and
https://patchew.org/QEMU/20240410222543.0ea534e6...@zero.eik.bme.hu/
be also merged for 9.0 please?

Regards,
BALATON Zoltan


hw/pci-host/ppc440_pcix.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/pci-host/ppc440_pcix.c b/hw/pci-host/ppc440_pcix.c
index 1926ae2a27..ef212d99aa 100644
--- a/hw/pci-host/ppc440_pcix.c
+++ b/hw/pci-host/ppc440_pcix.c
@@ -52,7 +52,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC440PCIXState, PPC440_PCIX_HOST)
struct PPC440PCIXState {
PCIHostState parent_obj;

-PCIDevice *dev;
+uint8_t config[PCI_CONFIG_SPACE_SIZE];
struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
uint32_t sts;
@@ -171,7 +171,7 @@ static void ppc440_pcix_reg_write4(void *opaque, hwaddr 
addr,
trace_ppc440_pcix_reg_write(addr, val, size);
switch (addr) {
case PCI_VENDOR_ID ... PCI_MAX_LAT:
-stl_le_p(s->dev->config + addr, val);
+stl_le_p(s->config + addr, val);
break;

case PCIX0_POM0LAL:
@@ -302,7 +302,7 @@ static uint64_t ppc440_pcix_reg_read4(void *opaque, hwaddr 
addr,

switch (addr) {
case PCI_VENDOR_ID ... PCI_MAX_LAT:
-val = ldl_le_p(s->dev->config + addr);
+val = ldl_le_p(s->config + addr);
break;

case PCIX0_POM0LAL:
@@ -498,10 +498,7 @@ static void ppc440_pcix_realize(DeviceState *dev, Error 
**errp)
memory_region_init(&s->iomem, OBJECT(dev), "pci-io", 64 * KiB);
h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
 ppc440_pcix_map_irq, &s->irq, &s->busmem, &s->iomem,
- PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
-
-s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
-   TYPE_PPC4xx_HOST_BRIDGE);
+ PCI_DEVFN(1, 0), 1, TYPE_PCI_BUS);

memory_region_init(&s->bm, OBJECT(s), "bm-ppc440-pcix", UINT64_MAX);
memory_region_add_subregion(&s->bm, 0x0, &s->busmem);

Re: [PATCH] target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT

[M Bazz]

Hi Philippe,

On Fri, Apr 12, 2024 at 7:14 AM Philippe Mathieu-Daudé
 wrote:
>
> Hi Bazz,
>
> On 12/4/24 06:18, M Bazz wrote:
> > On Thu, Apr 11, 2024, 10:15 PM Richard Henderson
> > mailto:richard.hender...@linaro.org>> wrote:
> >
> > Reads are done with execute access.  It is not clear whether writes
> > are legal at all -- for now, leave helper_st_asi unchanged, so that
> > we continue to raise an mmu fault.
> >
> > This generalizes the exiting code for ASI_KERNELTXT to be usable for
> > ASI_USERTXT as well, by passing down the MemOpIdx to use.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2281
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2059
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1609
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1166
> > 
> > Signed-off-by: Richard Henderson  > >
> > ---
> >   target/sparc/helper.h  |  3 ++
> >   target/sparc/ldst_helper.c | 65 ++
> >   target/sparc/translate.c   | 49 ++--
> >   3 files changed, 95 insertions(+), 22 deletions(-)
>
> > Hi Richard,
> >
> > I see this is in your hands now. I agree with your take on leaving
> > writes alone. I'm also grateful for the opportunity to collaborate with you.
> >
> > It brings a smile for the community members who will be touched by this
> > amazing contribution. I see them happily realizing that this perplexing
> > bug has been solved, and in our case finally able to use the debuggers
> > we love! :D
>
> Does that mean you tested this patch and it resolves your issues?
>
> If so, could you add your 'Tested-by: M Bazz ' tag
> when committing this patch?
>
> Regards,
>
> Phil.

Yes, I can confirm the issue is resolved. Richard has already made the
pull request. I am appreciative of the fast movement on this.

I would like to have my "Tested-by" tag added to the PR, if it's no
trouble. How might I get it there?

Tested-by: M Bazz 

>
> > Thanks for the proper fix, qemu sensei!
> >
> > -bazz
> >
>

Re: [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line

[Richard Henderson]

On 4/12/24 10:41, Philippe Mathieu-Daudé wrote:

-void qemu_hexdump_line(char *line, const void *bufptr, size_t len)
+GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
  {
-    const char *buf = bufptr;
-    int i, c;
+    const uint8_t *buf = vbuf;
+    size_t i;
-    if (len > QEMU_HEXDUMP_LINE_BYTES) {
-    len = QEMU_HEXDUMP_LINE_BYTES;
+    if (str == NULL) {
+    /* Estimate the length of the output to avoid reallocs. */
+    i = len * 3 + len / 4;
+    str = g_string_sized_new(i + 1);
  }


[*]
  else {
    g_string_truncate(str, 0);
  }


...

@@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void *bufptr, 
size_t len)
  *line = '\0';
  }
+#define QEMU_HEXDUMP_LINE_BYTES 16
  #define QEMU_HEXDUMP_LINE_WIDTH \
  (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
  void qemu_hexdump(FILE *fp, const char *prefix,
    const void *bufptr, size_t size)
  {
-    char line[QEMU_HEXDUMP_LINE_LEN];
+    g_autoptr(GString) str = g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
  char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
  size_t b, len;
  for (b = 0; b < size; b += len) {
  len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
-    qemu_hexdump_line(line, bufptr + b, len);
+    g_string_truncate(str, 0);


Shouldn't we truncate in [*] ?


The usage in tpm puts several lines together in one string,
adding \n in between, for output in one go.


r~

[PULL 1/1] target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT

[Richard Henderson]

Reads are done with execute access.  It is not clear whether writes
are legal at all -- for now, leave helper_st_asi unchanged, so that
we continue to raise an mmu fault.

This generalizes the exiting code for ASI_KERNELTXT to be usable for
ASI_USERTXT as well, by passing down the MemOpIdx to use.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2281
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2059
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1609
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1166
Signed-off-by: Richard Henderson 
Acked-by: Mark Cave-Ayland 
---
 target/sparc/helper.h  |  3 ++
 target/sparc/ldst_helper.c | 65 ++
 target/sparc/translate.c   | 48 ++--
 3 files changed, 94 insertions(+), 22 deletions(-)

diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index e55fad5b8c..b8087d0d2b 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -32,6 +32,9 @@ DEF_HELPER_FLAGS_3(udiv, TCG_CALL_NO_WG, i64, env, tl, tl)
 DEF_HELPER_FLAGS_3(sdiv, TCG_CALL_NO_WG, i64, env, tl, tl)
 DEF_HELPER_3(taddcctv, tl, env, tl, tl)
 DEF_HELPER_3(tsubcctv, tl, env, tl, tl)
+#if !defined(CONFIG_USER_ONLY) && !defined(TARGET_SPARC64)
+DEF_HELPER_FLAGS_3(ld_code, TCG_CALL_NO_WG, i64, env, tl, i32)
+#endif
 #if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
 DEF_HELPER_FLAGS_4(ld_asi, TCG_CALL_NO_WG, i64, env, tl, int, i32)
 DEF_HELPER_FLAGS_5(st_asi, TCG_CALL_NO_WG, void, env, tl, i64, int, i32)
diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index e581bb42ac..2846a86cc4 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -585,7 +585,6 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
 #if defined(DEBUG_MXCC) || defined(DEBUG_ASI)
 uint32_t last_addr = addr;
 #endif
-MemOpIdx oi;
 
 do_check_align(env, addr, size - 1, GETPC());
 switch (asi) {
@@ -684,24 +683,6 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
 case ASI_M_DIAGS:   /* Turbosparc DTLB Diagnostic */
 case ASI_M_IODIAG:  /* Turbosparc IOTLB Diagnostic */
 break;
-case ASI_KERNELTXT: /* Supervisor code access */
-oi = make_memop_idx(memop, cpu_mmu_index(env_cpu(env), true));
-switch (size) {
-case 1:
-ret = cpu_ldb_code_mmu(env, addr, oi, GETPC());
-break;
-case 2:
-ret = cpu_ldw_code_mmu(env, addr, oi, GETPC());
-break;
-default:
-case 4:
-ret = cpu_ldl_code_mmu(env, addr, oi, GETPC());
-break;
-case 8:
-ret = cpu_ldq_code_mmu(env, addr, oi, GETPC());
-break;
-}
-break;
 case ASI_M_TXTC_TAG:   /* SparcStation 5 I-cache tag */
 case ASI_M_TXTC_DATA:  /* SparcStation 5 I-cache data */
 case ASI_M_DATAC_TAG:  /* SparcStation 5 D-cache tag */
@@ -779,7 +760,6 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
 case 0x4c: /* SuperSPARC MMU Breakpoint Action */
 ret = env->mmubpaction;
 break;
-case ASI_USERTXT: /* User code access, XXX */
 default:
 sparc_raise_mmu_fault(cs, addr, false, false, asi, size, GETPC());
 ret = 0;
@@ -787,6 +767,8 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
 
 case ASI_USERDATA: /* User data access */
 case ASI_KERNELDATA: /* Supervisor data access */
+case ASI_USERTXT: /* User code access */
+case ASI_KERNELTXT: /* Supervisor code access */
 case ASI_P: /* Implicit primary context data access (v9 only?) */
 case ASI_M_BYPASS:/* MMU passthrough */
 case ASI_LEON_BYPASS: /* LEON MMU passthrough */
@@ -1161,6 +1143,49 @@ void helper_st_asi(CPUSPARCState *env, target_ulong 
addr, uint64_t val,
 #endif
 }
 
+uint64_t helper_ld_code(CPUSPARCState *env, target_ulong addr, uint32_t oi)
+{
+MemOp mop = get_memop(oi);
+uintptr_t ra = GETPC();
+uint64_t ret;
+
+switch (mop & MO_SIZE) {
+case MO_8:
+ret = cpu_ldb_code_mmu(env, addr, oi, ra);
+if (mop & MO_SIGN) {
+ret = (int8_t)ret;
+}
+break;
+case MO_16:
+ret = cpu_ldw_code_mmu(env, addr, oi, ra);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap16(ret);
+}
+if (mop & MO_SIGN) {
+ret = (int16_t)ret;
+}
+break;
+case MO_32:
+ret = cpu_ldl_code_mmu(env, addr, oi, ra);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap32(ret);
+}
+if (mop & MO_SIGN) {
+ret = (int32_t)ret;
+}
+break;
+case MO_64:
+ret = cpu_ldq_code_mmu(env, addr, oi, ra);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap64(ret);
+}
+break;
+default:
+g_assert_not_reached();
+}
+return ret;
+}
+
 #endif /* CONFIG_USER_ONLY */
 #else /*

[PULL 0/1] target/sparc late fix

[Richard Henderson]

Since this problem has 4 issues open, let's get it for 9.0.


r~


The following changes since commit be72d6ab361a26878752467a17289066dfd5bc28:

  Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging 
(2024-04-12 16:01:04 +0100)

are available in the Git repository at:

  https://gitlab.com/rth7680/qemu.git tags/pull-sp-20240412

for you to fetch changes up to c84f5198b0b676ad67962b5250af1b0d0842e319:

  target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT (2024-04-12 
11:48:26 -0700)


target/sparc: Fix ASI_USERTXT for Solaris gdb crashes


Richard Henderson (1):
  target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT

 target/sparc/helper.h  |  3 +++
 target/sparc/ldst_helper.c | 65 --
 target/sparc/translate.c   | 48 --
 3 files changed, 94 insertions(+), 22 deletions(-)

Re: [PATCH] target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT

[Mark Cave-Ayland]

On 12/04/2024 03:15, Richard Henderson wrote:


Reads are done with execute access.  It is not clear whether writes
are legal at all -- for now, leave helper_st_asi unchanged, so that
we continue to raise an mmu fault.

This generalizes the exiting code for ASI_KERNELTXT to be usable for
ASI_USERTXT as well, by passing down the MemOpIdx to use.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2281
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2059
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1609
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1166
Signed-off-by: Richard Henderson 
---
  target/sparc/helper.h  |  3 ++
  target/sparc/ldst_helper.c | 65 ++
  target/sparc/translate.c   | 49 ++--
  3 files changed, 95 insertions(+), 22 deletions(-)

diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index e55fad5b8c..b8087d0d2b 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -32,6 +32,9 @@ DEF_HELPER_FLAGS_3(udiv, TCG_CALL_NO_WG, i64, env, tl, tl)
  DEF_HELPER_FLAGS_3(sdiv, TCG_CALL_NO_WG, i64, env, tl, tl)
  DEF_HELPER_3(taddcctv, tl, env, tl, tl)
  DEF_HELPER_3(tsubcctv, tl, env, tl, tl)
+#if !defined(CONFIG_USER_ONLY) && !defined(TARGET_SPARC64)
+DEF_HELPER_FLAGS_3(ld_code, TCG_CALL_NO_WG, i64, env, tl, i32)
+#endif
  #if !defined(CONFIG_USER_ONLY) || defined(TARGET_SPARC64)
  DEF_HELPER_FLAGS_4(ld_asi, TCG_CALL_NO_WG, i64, env, tl, int, i32)
  DEF_HELPER_FLAGS_5(st_asi, TCG_CALL_NO_WG, void, env, tl, i64, int, i32)
diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index e581bb42ac..2846a86cc4 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -585,7 +585,6 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
  #if defined(DEBUG_MXCC) || defined(DEBUG_ASI)
  uint32_t last_addr = addr;
  #endif
-MemOpIdx oi;
  
  do_check_align(env, addr, size - 1, GETPC());

  switch (asi) {
@@ -684,24 +683,6 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
  case ASI_M_DIAGS:   /* Turbosparc DTLB Diagnostic */
  case ASI_M_IODIAG:  /* Turbosparc IOTLB Diagnostic */
  break;
-case ASI_KERNELTXT: /* Supervisor code access */
-oi = make_memop_idx(memop, cpu_mmu_index(env_cpu(env), true));
-switch (size) {
-case 1:
-ret = cpu_ldb_code_mmu(env, addr, oi, GETPC());
-break;
-case 2:
-ret = cpu_ldw_code_mmu(env, addr, oi, GETPC());
-break;
-default:
-case 4:
-ret = cpu_ldl_code_mmu(env, addr, oi, GETPC());
-break;
-case 8:
-ret = cpu_ldq_code_mmu(env, addr, oi, GETPC());
-break;
-}
-break;
  case ASI_M_TXTC_TAG:   /* SparcStation 5 I-cache tag */
  case ASI_M_TXTC_DATA:  /* SparcStation 5 I-cache data */
  case ASI_M_DATAC_TAG:  /* SparcStation 5 D-cache tag */
@@ -779,7 +760,6 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
  case 0x4c: /* SuperSPARC MMU Breakpoint Action */
  ret = env->mmubpaction;
  break;
-case ASI_USERTXT: /* User code access, XXX */
  default:
  sparc_raise_mmu_fault(cs, addr, false, false, asi, size, GETPC());
  ret = 0;
@@ -787,6 +767,8 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
  
  case ASI_USERDATA: /* User data access */

  case ASI_KERNELDATA: /* Supervisor data access */
+case ASI_USERTXT: /* User code access */
+case ASI_KERNELTXT: /* Supervisor code access */
  case ASI_P: /* Implicit primary context data access (v9 only?) */
  case ASI_M_BYPASS:/* MMU passthrough */
  case ASI_LEON_BYPASS: /* LEON MMU passthrough */
@@ -1161,6 +1143,49 @@ void helper_st_asi(CPUSPARCState *env, target_ulong 
addr, uint64_t val,
  #endif
  }
  
+uint64_t helper_ld_code(CPUSPARCState *env, target_ulong addr, uint32_t oi)

+{
+MemOp mop = get_memop(oi);
+uintptr_t ra = GETPC();
+uint64_t ret;
+
+switch (mop & MO_SIZE) {
+case MO_8:
+ret = cpu_ldb_code_mmu(env, addr, oi, ra);
+if (mop & MO_SIGN) {
+ret = (int8_t)ret;
+}
+break;
+case MO_16:
+ret = cpu_ldw_code_mmu(env, addr, oi, ra);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap16(ret);
+}
+if (mop & MO_SIGN) {
+ret = (int16_t)ret;
+}
+break;
+case MO_32:
+ret = cpu_ldl_code_mmu(env, addr, oi, ra);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap32(ret);
+}
+if (mop & MO_SIGN) {
+ret = (int32_t)ret;
+}
+break;
+case MO_64:
+ret = cpu_ldq_code_mmu(env, addr, oi, ra);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap64(ret);
+}
+break;
+default:
+g_assert_not_reached();
+}
+

Re: [PATCH v3 11/27] backends/tpm: Use qemu_hexdump_line to avoid sprintf

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

From: Philippe Mathieu-Daudé 

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.

Reviewed-by: Stefan Berger 
Signed-off-by: Philippe Mathieu-Daudé 
[rth: Keep the linebreaks every 16 bytes]
Signed-off-by: Richard Henderson 
---
  backends/tpm/tpm_util.c | 24 ++--
  1 file changed, 10 insertions(+), 14 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 08/27] hw/scsi/scsi-disk: Use qemu_hexdump_line to avoid sprintf

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

From: Philippe Mathieu-Daudé 

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.

Note that this drops the "0x" prefix to every byte, which should
be of no consequence to tracing.

Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
  hw/scsi/scsi-disk.c | 13 +++--
  1 file changed, 3 insertions(+), 10 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 07/27] system/qtest: Replace sprintf by qemu_hexdump_line

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

From: Philippe Mathieu-Daudé 

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1.
Using qemu_hexdump_line both fixes the deprecation warning and
simplifies the code base.

Signed-off-by: Philippe Mathieu-Daudé `
[rth: Use qemu_hexdump_line]
Signed-off-by: Richard Henderson 
---
  system/qtest.c | 12 
  1 file changed, 4 insertions(+), 8 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 05/27] util/hexdump: Inline g_string_append_printf "%02x"

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

Trivial arithmetic can be used for emitting the nibbles,
rather than full-blown printf formatting.

Signed-off-by: Richard Henderson 
---
  util/hexdump.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 04/27] util/hexdump: Add unit_len and block_len to qemu_hexdump_line

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

Generalize the current 1 byte unit and 4 byte blocking
within the output.

Signed-off-by: Richard Henderson 
---
  include/qemu/cutils.h  |  6 +-
  hw/virtio/vhost-vdpa.c |  2 +-
  util/hexdump.c | 30 +-
  3 files changed, 27 insertions(+), 11 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 03/27] util/hexdump: Use a GString for qemu_hexdump_line

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

Allocate a new, or append to an existing GString instead of
using a fixed sized buffer.  Require the caller to determine
the length of the line -- do not bound len here.

Signed-off-by: Richard Henderson 
---
  include/qemu/cutils.h  | 15 ++-
  hw/virtio/vhost-vdpa.c | 14 --
  util/hexdump.c | 29 +
  3 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index d0c5386e6c..7311fb36ca 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -252,12 +252,17 @@ static inline const char *yes_no(bool b)
   */
  int parse_debug_env(const char *name, int max, int initial);
  
-/*

- * Hexdump a line of a byte buffer into a hexadecimal/ASCII buffer
+/**
+ * qemu_hexdump_line:
+ * @str: GString into which to append
+ * @buf: buffer to dump
+ * @len: number of bytes to dump
+ *
+ * Append @len bytes of @buf as hexadecimal into @str.
+ * If @str is NULL, allocate a new string and return it;
+ * otherwise return @str.
   */
-#define QEMU_HEXDUMP_LINE_BYTES 16 /* Number of bytes to dump */
-#define QEMU_HEXDUMP_LINE_LEN 75   /* Number of characters in line */
-void qemu_hexdump_line(char *line, const void *bufptr, size_t len);
+GString *qemu_hexdump_line(GString *str, const void *buf, size_t len);




diff --git a/util/hexdump.c b/util/hexdump.c
index dbc536fe84..521e346bc6 100644
--- a/util/hexdump.c
+++ b/util/hexdump.c
@@ -16,22 +16,25 @@
  #include "qemu/osdep.h"
  #include "qemu/cutils.h"
  
-void qemu_hexdump_line(char *line, const void *bufptr, size_t len)

+GString *qemu_hexdump_line(GString *str, const void *vbuf, size_t len)
  {
-const char *buf = bufptr;
-int i, c;
+const uint8_t *buf = vbuf;
+size_t i;
  
-if (len > QEMU_HEXDUMP_LINE_BYTES) {

-len = QEMU_HEXDUMP_LINE_BYTES;
+if (str == NULL) {
+/* Estimate the length of the output to avoid reallocs. */
+i = len * 3 + len / 4;
+str = g_string_sized_new(i + 1);
  }


[*]
 else {
   g_string_truncate(str, 0);
 }

  
  for (i = 0; i < len; i++) {

  if (i != 0 && (i % 4) == 0) {
-*line++ = ' ';
+g_string_append_c(str, ' ');
  }
-line += sprintf(line, " %02x", (unsigned char)buf[i]);
+g_string_append_printf(str, " %02x", buf[i]);
  }
-*line = '\0';
+
+return str;
  }
  
  static void asciidump_line(char *line, const void *bufptr, size_t len)

@@ -49,24 +52,26 @@ static void asciidump_line(char *line, const void *bufptr, 
size_t len)
  *line = '\0';
  }
  
+#define QEMU_HEXDUMP_LINE_BYTES 16

  #define QEMU_HEXDUMP_LINE_WIDTH \
  (QEMU_HEXDUMP_LINE_BYTES * 2 + QEMU_HEXDUMP_LINE_BYTES / 4)
  
  void qemu_hexdump(FILE *fp, const char *prefix,

const void *bufptr, size_t size)
  {
-char line[QEMU_HEXDUMP_LINE_LEN];
+g_autoptr(GString) str = g_string_sized_new(QEMU_HEXDUMP_LINE_WIDTH + 1);
  char ascii[QEMU_HEXDUMP_LINE_BYTES + 1];
  size_t b, len;
  
  for (b = 0; b < size; b += len) {

  len = MIN(size - b, QEMU_HEXDUMP_LINE_BYTES);
  
-qemu_hexdump_line(line, bufptr + b, len);

+g_string_truncate(str, 0);


Shouldn't we truncate in [*] ?


+qemu_hexdump_line(str, bufptr + b, len);
  asciidump_line(ascii, bufptr + b, len);
  
-fprintf(fp, "%s: %04x: %-*s %s\n",

-prefix, b, QEMU_HEXDUMP_LINE_WIDTH, line, ascii);
+fprintf(fp, "%s: %04zx: %-*s %s\n",
+prefix, b, QEMU_HEXDUMP_LINE_WIDTH, str->str, ascii);
  }
  
  }

Re: [PATCH] MAINTAINERS: Update my email address

[Philippe Mathieu-Daudé]

Hi Bin,

On 12/4/24 14:37, Bin Meng wrote:

The Wind River email address might change in the future. Use my
personal email address instead.

Signed-off-by: Bin Meng 

---

  MAINTAINERS | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f1f6922025..50729a0985 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -332,7 +332,7 @@ F: tests/tcg/ppc*/*
  RISC-V TCG CPUs
  M: Palmer Dabbelt 
  M: Alistair Francis 
-M: Bin Meng 
+M: Bin Meng 


Do you mind confirming that from your windriver.com
email while you still have it?

Thanks,

Phil.

Re: [PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr

[Philippe Mathieu-Daudé]

On 12/4/24 18:08, Peter Maydell wrote:

The npcm7xx_clk and npcm7xx_gcr device reset methods look at
the ResetType argument and only handle RESET_TYPE_COLD,
producing a warning if another reset type is passed. This
is different from how every other three-phase-reset method
we have works, and makes it difficult to add new reset types.

A better pattern is "assume that any reset type you don't know
about should be handled like RESET_TYPE_COLD"; switch these
devices to do that. Then adding a new reset type will only
need to touch those devices where its behaviour really needs
to be different from the standard cold reset.

Signed-off-by: Peter Maydell 
---
  hw/misc/npcm7xx_clk.c | 13 +++--
  hw/misc/npcm7xx_gcr.c | 12 
  2 files changed, 7 insertions(+), 18 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 6/6] reset: Add RESET_TYPE_SNAPSHOT_LOAD

[Philippe Mathieu-Daudé]

On 12/4/24 18:08, Peter Maydell wrote:

Some devices and machines need to handle the reset before a vmsave
snapshot is loaded differently -- the main user is the handling of
RNG seed information, which does not want to put a new RNG seed into
a ROM blob when we are doing a snapshot load.

Currently this kind of reset handling is supported only for:
  * TYPE_MACHINE reset methods, which take a ShutdownCause argument
  * reset functions registered with qemu_register_reset_nosnapshotload

To allow a three-phase-reset device to also distinguish "snapshot
load" reset from the normal kind, add a new ResetType
RESET_TYPE_SNAPSHOT_LOAD. All our existing reset methods ignore
the reset type, so we don't need to update any device code.

Add the enum type, and make qemu_devices_reset() use the
right reset type for the ShutdownCause it is passed. This
allows us to get rid of the device_reset_reason global we
were using to implement qemu_register_reset_nosnapshotload().

Signed-off-by: Peter Maydell 
---
  docs/devel/reset.rst| 17 ++---
  include/hw/resettable.h |  1 +
  hw/core/reset.c | 15 ---
  hw/core/resettable.c|  4 
  4 files changed, 19 insertions(+), 18 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 6/6] reset: Add RESET_TYPE_SNAPSHOT_LOAD

[Richard Henderson]

On 4/12/24 09:08, Peter Maydell wrote:

Some devices and machines need to handle the reset before a vmsave
snapshot is loaded differently -- the main user is the handling of
RNG seed information, which does not want to put a new RNG seed into
a ROM blob when we are doing a snapshot load.

Currently this kind of reset handling is supported only for:
  * TYPE_MACHINE reset methods, which take a ShutdownCause argument
  * reset functions registered with qemu_register_reset_nosnapshotload

To allow a three-phase-reset device to also distinguish "snapshot
load" reset from the normal kind, add a new ResetType
RESET_TYPE_SNAPSHOT_LOAD. All our existing reset methods ignore
the reset type, so we don't need to update any device code.

Add the enum type, and make qemu_devices_reset() use the
right reset type for the ShutdownCause it is passed. This
allows us to get rid of the device_reset_reason global we
were using to implement qemu_register_reset_nosnapshotload().

Signed-off-by: Peter Maydell
---
  docs/devel/reset.rst| 17 ++---
  include/hw/resettable.h |  1 +
  hw/core/reset.c | 15 ---
  hw/core/resettable.c|  4 
  4 files changed, 19 insertions(+), 18 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 5/6] docs/devel/reset: Update to new API for hold and exit phase methods

[Philippe Mathieu-Daudé]

On 12/4/24 18:08, Peter Maydell wrote:

Update the reset documentation's example code to match the new API
for the hold and exit phase method APIs where they take a ResetType
argument.

Signed-off-by: Peter Maydell 
---
  docs/devel/reset.rst | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 4/6] hw, target: Add ResetType argument to hold and exit phase methods

[Richard Henderson]

On 4/12/24 09:08, Peter Maydell wrote:

We pass a ResetType argument to the Resettable class enter
phase method, but we don't pass it to hold and exit, even though
the callsites have it readily available. This means that if
a device cared about the ResetType it would need to record it
in the enter phase method to use later on. Pass the type to
all three of the phase methods to avoid having to do that.

Commit created with

   for dir in hw target include; do \
   spatch --macro-file scripts/cocci-macro-file.h \
  --sp-file scripts/coccinelle/reset-type.cocci \
  --keep-comments --smpl-spacing --in-place \
  --include-headers --dir $dir; done

and no manual edits.

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 5/6] docs/devel/reset: Update to new API for hold and exit phase methods

[Richard Henderson]

On 4/12/24 09:08, Peter Maydell wrote:

Update the reset documentation's example code to match the new API
for the hold and exit phase method APIs where they take a ResetType
argument.

Signed-off-by: Peter Maydell
---
  docs/devel/reset.rst | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()

[Daniel Henrique Barboza]

On 4/12/24 13:52, Aleksei Filippov wrote:



On 12.04.2024 19:00, Daniel Henrique Barboza wrote:


Thanks for giving it a go. You're right, this patch alone is not enough and 
we'll
need your patch too.

But note that, with what you've said in mind, your patch will also end up 
setting
mtval2 and env->guest_phys_fault_addr in case a PMP fault occurs during the
get_physical_address() right at the start of second stage:

 if (ret == TRANSLATE_SUCCESS) {
 /* Second stage lookup */
 im_address = pa;

 ret = get_physical_address(env, &pa, &prot2, im_address, NULL,
    access_type, MMUIdx_U, false, true,
    false);


I think your patch needs to also prevent env->guest_phys_fault_addr to be set 
when
ret == TRANSLATE_PMP_FAIL.

With these changes in your patch, and this patch, we're free to set 
"first_stage_error = false;"
at the start of second stage lookup, keeping consistency, because 
raise_mmu_exception is now
able to deal with it. I can amend this change in this patch. This patch would 
prioritize
PMP errors, your patch will fix the problem with mtval2.

Let me know what do you think. If you agree I can re-send both patches together.


Thanks,


Daniel


Oh, I actually missed that, thx, you are right. I could prepare patch to fix
this, do you want it in this thread or in previous with only my patch in?


If you don't mind, please re-send it together in a new thread with this patch 
too.

Include this one as is, then include yours as a complement that fixes the 
problem
with mtval2 that my patch missed. You can use the explanation you gave me as the
new commit msg for your patch. E.g:

"target/riscv: do not set mtval2 for non guest-page faults

Previous patch fixed the PMP priority in raise_mmu_exception() but we're still
setting mtval2 incorrectly. In riscv_cpu_tlb_fill(), after pmp check in 2 stage
translation part, mtval2 will be set in case of successes 2 stage translation 
but
failed pmp check.

In this case we gonna set mtval2 via env->guest_phys_fault_addr in context of
riscv_cpu_tlb_fill(), as this was a guest-page-fault, but it didn't and mtval2
should be zero, according to RISCV privileged spec sect. 9.4.4: When a guest
page-fault is taken into M-mode, mtval2 is written with either zero or guest
physical address that faulted, shifted by 2 bits. *For other traps, mtval2
is set to zero...* "


Thanks,

Daniel

Re: [PATCH 2/6] allwinner-i2c, adm1272: Use device_cold_reset() for software-triggered reset

[Richard Henderson]

On 4/12/24 09:08, Peter Maydell wrote:

Rather than directly calling the device's implementation of its 'hold'
reset phase, call device_cold_reset(). This means we don't have to
adjust this callsite when we add another argument to the function
signature for the hold and exit reset methods.

Signed-off-by: Peter Maydell
---
  hw/i2c/allwinner-i2c.c | 3 +--
  hw/sensor/adm1272.c| 2 +-
  2 files changed, 2 insertions(+), 3 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr

[Richard Henderson]

On 4/12/24 09:08, Peter Maydell wrote:

The npcm7xx_clk and npcm7xx_gcr device reset methods look at
the ResetType argument and only handle RESET_TYPE_COLD,
producing a warning if another reset type is passed. This
is different from how every other three-phase-reset method
we have works, and makes it difficult to add new reset types.

A better pattern is "assume that any reset type you don't know
about should be handled like RESET_TYPE_COLD"; switch these
devices to do that. Then adding a new reset type will only
need to touch those devices where its behaviour really needs
to be different from the standard cold reset.

Signed-off-by: Peter Maydell
---
  hw/misc/npcm7xx_clk.c | 13 +++--
  hw/misc/npcm7xx_gcr.c | 12 
  2 files changed, 7 insertions(+), 18 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()

[Daniel Henrique Barboza]

On 4/12/24 14:12, Peter Maydell wrote:

On Tue, 9 Apr 2024 at 18:53, Daniel Henrique Barboza
 wrote:


raise_mmu_exception(), as is today, is prioritizing guest page faults by
checking first if virt_enabled && !first_stage, and then considering the
regular inst/load/store faults.

There's no mention in the spec about guest page fault being a higher
priority that PMP faults. In fact, privileged spec section 3.7.1 says:

"Attempting to fetch an instruction from a PMP region that does not have
execute permissions raises an instruction access-fault exception.
Attempting to execute a load or load-reserved instruction which accesses
a physical address within a PMP region without read permissions raises a
load access-fault exception. Attempting to execute a store,
store-conditional, or AMO instruction which accesses a physical address
within a PMP region without write permissions raises a store
access-fault exception."

So, in fact, we're doing it wrong - PMP faults should always be thrown,
regardless of also being a first or second stage fault.

The way riscv_cpu_tlb_fill() and get_physical_address() work is
adequate: a TRANSLATE_PMP_FAIL error is immediately reported and
reflected in the 'pmp_violation' flag. What we need is to change
raise_mmu_exception() to prioritize it.

Reported-by: Joseph Chan 
Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU translation 
stage")
Signed-off-by: Daniel Henrique Barboza 


I guess from the Fixes: git commit hash that this isn't a regression
since 8.2 ? That would make it too late for 9.0 at this point in
the release cycle.


I don't think it's critical enough to push it for 9.0 now. We'll settle for
9.1 and then Michael can pick it for 9.0-stable.


Thanks,

Daniel



thanks
-- PMM

Re: [PATCH v2 01/13] tests: Remove Ubuntu 20.04 container

[Philippe Mathieu-Daudé]

On 12/4/24 15:24, Thomas Huth wrote:

Since Ubuntu 22.04 is now available since two years, we can stop
actively supporting the previous LTS version of Ubuntu now.

Signed-off-by: Thomas Huth 
---
  tests/docker/dockerfiles/ubuntu2004.docker | 157 -
  tests/lcitool/refresh  |   1 -
  2 files changed, 158 deletions(-)
  delete mode 100644 tests/docker/dockerfiles/ubuntu2004.docker


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()

[Peter Maydell]

On Tue, 9 Apr 2024 at 18:53, Daniel Henrique Barboza
 wrote:
>
> raise_mmu_exception(), as is today, is prioritizing guest page faults by
> checking first if virt_enabled && !first_stage, and then considering the
> regular inst/load/store faults.
>
> There's no mention in the spec about guest page fault being a higher
> priority that PMP faults. In fact, privileged spec section 3.7.1 says:
>
> "Attempting to fetch an instruction from a PMP region that does not have
> execute permissions raises an instruction access-fault exception.
> Attempting to execute a load or load-reserved instruction which accesses
> a physical address within a PMP region without read permissions raises a
> load access-fault exception. Attempting to execute a store,
> store-conditional, or AMO instruction which accesses a physical address
> within a PMP region without write permissions raises a store
> access-fault exception."
>
> So, in fact, we're doing it wrong - PMP faults should always be thrown,
> regardless of also being a first or second stage fault.
>
> The way riscv_cpu_tlb_fill() and get_physical_address() work is
> adequate: a TRANSLATE_PMP_FAIL error is immediately reported and
> reflected in the 'pmp_violation' flag. What we need is to change
> raise_mmu_exception() to prioritize it.
>
> Reported-by: Joseph Chan 
> Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU 
> translation stage")
> Signed-off-by: Daniel Henrique Barboza 

I guess from the Fixes: git commit hash that this isn't a regression
since 8.2 ? That would make it too late for 9.0 at this point in
the release cycle.

thanks
-- PMM

Re: [PATCH v2 12/13] block/ssh: Use URI parsing code from glib

[Markus Armbruster]

Eric Blake  writes:

> On Fri, Apr 12, 2024 at 03:24:14PM +0200, Thomas Huth wrote:
>> Since version 2.66, glib has useful URI parsing functions, too.
>> Use those instead of the QEMU-internal ones to be finally able
>> to get rid of the latter.
>> 
>> Reviewed-by: Richard W.M. Jones 
>> Signed-off-by: Thomas Huth 
>> ---
>>  block/ssh.c | 75 -
>>  1 file changed, 40 insertions(+), 35 deletions(-)
>> 
>
>>  
>> -if (g_strcmp0(uri->scheme, "ssh") != 0) {
>> +if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) {
>
> Yet another case-sensitive spot to consider.
>
>>  
>> -qdict_put_str(options, "path", uri->path);
>> -
>> -/* Pick out any query parameters that we understand, and ignore
>> - * the rest.
>> - */
>> -for (i = 0; i < qp->n; ++i) {
>> -if (strcmp(qp->p[i].name, "host_key_check") == 0) {
>> -qdict_put_str(options, "host_key_check", qp->p[i].value);
>> +qdict_put_str(options, "path", uri_path);
>> +
>> +uri_query = g_uri_get_query(uri);
>> +if (uri_query) {
>> +g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE);
>> +while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) {
>> +if (!qp_name || !qp_value || gerror) {
>> +warn_report("Failed to parse SSH URI parameters '%s'.",
>> +uri_query);
>> +break;
>> +}
>> +/*
>> + * Pick out the query parameters that we understand, and ignore
>> + * (or rather warn about) the rest.
>> + */
>> +if (g_str_equal(qp_name, "host_key_check")) {
>> +qdict_put_str(options, "host_key_check", qp_value);
>> +} else {
>> +warn_report("Unsupported parameter '%s' in URI.", qp_name);
>
> Do we want the trailing '.' in warn_report?

We do not.

> The warning is new; it was not in the old code, nor mentioned in the
> commit message.  It seems like a good idea, but we should be more
> intentional if we intend to make that change.

Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()

[Aleksei Filippov]

On 12.04.2024 19:00, Daniel Henrique Barboza wrote:


Thanks for giving it a go. You're right, this patch alone is not enough and 
we'll
need your patch too.

But note that, with what you've said in mind, your patch will also end up 
setting
mtval2 and env->guest_phys_fault_addr in case a PMP fault occurs during the
get_physical_address() right at the start of second stage:

     if (ret == TRANSLATE_SUCCESS) {
     /* Second stage lookup */
     im_address = pa;

     ret = get_physical_address(env, &pa, &prot2, im_address, NULL,
    access_type, MMUIdx_U, false, true,
    false);


I think your patch needs to also prevent env->guest_phys_fault_addr to be set 
when
ret == TRANSLATE_PMP_FAIL.

With these changes in your patch, and this patch, we're free to set 
"first_stage_error = false;"
at the start of second stage lookup, keeping consistency, because 
raise_mmu_exception is now
able to deal with it. I can amend this change in this patch. This patch would 
prioritize

PMP errors, your patch will fix the problem with mtval2.

Let me know what do you think. If you agree I can re-send both patches together.


Thanks,


Daniel


Oh, I actually missed that, thx, you are right. I could prepare patch to fix
this, do you want it in this thread or in previous with only my patch in?

--
Sincerely,
Aleksei Filippov

Re: [PATCH 4/6] hw, target: Add ResetType argument to hold and exit phase methods

[Edgar E. Iglesias]

On Fri, Apr 12, 2024 at 6:09 PM Peter Maydell  wrote:
>
> We pass a ResetType argument to the Resettable class enter
> phase method, but we don't pass it to hold and exit, even though
> the callsites have it readily available. This means that if
> a device cared about the ResetType it would need to record it
> in the enter phase method to use later on. Pass the type to
> all three of the phase methods to avoid having to do that.
>
> Commit created with
>
>   for dir in hw target include; do \
>   spatch --macro-file scripts/cocci-macro-file.h \
>  --sp-file scripts/coccinelle/reset-type.cocci \
>  --keep-comments --smpl-spacing --in-place \
>  --include-headers --dir $dir; done
>
> and no manual edits.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Edgar E. Iglesias 


> ---
>  include/hw/resettable.h |  4 ++--
>  hw/adc/npcm7xx_adc.c|  2 +-
>  hw/arm/pxa2xx_pic.c |  2 +-
>  hw/arm/smmu-common.c|  2 +-
>  hw/arm/smmuv3.c |  4 ++--
>  hw/arm/stellaris.c  | 10 +-
>  hw/audio/asc.c  |  2 +-
>  hw/char/cadence_uart.c  |  2 +-
>  hw/char/sifive_uart.c   |  2 +-
>  hw/core/cpu-common.c|  2 +-
>  hw/core/qdev.c  |  4 ++--
>  hw/core/reset.c |  2 +-
>  hw/core/resettable.c|  4 ++--
>  hw/display/virtio-vga.c |  4 ++--
>  hw/gpio/npcm7xx_gpio.c  |  2 +-
>  hw/gpio/pl061.c |  2 +-
>  hw/gpio/stm32l4x5_gpio.c|  2 +-
>  hw/hyperv/vmbus.c   |  2 +-
>  hw/i2c/allwinner-i2c.c  |  2 +-
>  hw/i2c/npcm7xx_smbus.c  |  2 +-
>  hw/input/adb.c  |  2 +-
>  hw/input/ps2.c  | 12 ++--
>  hw/intc/arm_gic_common.c|  2 +-
>  hw/intc/arm_gic_kvm.c   |  4 ++--
>  hw/intc/arm_gicv3_common.c  |  2 +-
>  hw/intc/arm_gicv3_its.c |  4 ++--
>  hw/intc/arm_gicv3_its_common.c  |  2 +-
>  hw/intc/arm_gicv3_its_kvm.c |  4 ++--
>  hw/intc/arm_gicv3_kvm.c |  4 ++--
>  hw/intc/xics.c  |  2 +-
>  hw/m68k/q800-glue.c |  2 +-
>  hw/misc/djmemc.c|  2 +-
>  hw/misc/iosb.c  |  2 +-
>  hw/misc/mac_via.c   |  8 
>  hw/misc/macio/cuda.c|  4 ++--
>  hw/misc/macio/pmu.c |  4 ++--
>  hw/misc/mos6522.c   |  2 +-
>  hw/misc/npcm7xx_mft.c   |  2 +-
>  hw/misc/npcm7xx_pwm.c   |  2 +-
>  hw/misc/stm32l4x5_exti.c|  2 +-
>  hw/misc/stm32l4x5_rcc.c | 10 +-
>  hw/misc/stm32l4x5_syscfg.c  |  2 +-
>  hw/misc/xlnx-versal-cframe-reg.c|  2 +-
>  hw/misc/xlnx-versal-crl.c   |  2 +-
>  hw/misc/xlnx-versal-pmc-iou-slcr.c  |  2 +-
>  hw/misc/xlnx-versal-trng.c  |  2 +-
>  hw/misc/xlnx-versal-xramc.c |  2 +-
>  hw/misc/xlnx-zynqmp-apu-ctrl.c  |  2 +-
>  hw/misc/xlnx-zynqmp-crf.c   |  2 +-
>  hw/misc/zynq_slcr.c |  4 ++--
>  hw/net/can/xlnx-zynqmp-can.c|  2 +-
>  hw/net/e1000.c  |  2 +-
>  hw/net/e1000e.c |  2 +-
>  hw/net/igb.c|  2 +-
>  hw/net/igbvf.c  |  2 +-
>  hw/nvram/xlnx-bbram.c   |  2 +-
>  hw/nvram/xlnx-versal-efuse-ctrl.c   |  2 +-
>  hw/nvram/xlnx-zynqmp-efuse.c|  2 +-
>  hw/pci-bridge/cxl_root_port.c   |  4 ++--
>  hw/pci-bridge/pcie_root_port.c  |  2 +-
>  hw/pci-host/bonito.c|  2 +-
>  hw/pci-host/pnv_phb.c   |  4 ++--
>  hw/pci-host/pnv_phb3_msi.c  |  4 ++--
>  hw/pci/pci.c|  4 ++--
>  hw/rtc/mc146818rtc.c|  2 +-
>  hw/s390x/css-bridge.c   |  2 +-
>  hw/sensor/adm1266.c |  2 +-
>  hw/sensor/adm1272.c |  2 +-
>  hw/sensor/isl_pmbus_vr.c| 10 +-
>  hw/sensor/max31785.c|  2 +-
>  hw/sensor/max34451.c|  2 +-
>  hw/ssi/npcm7xx_fiu.c|  2 +-
>  hw/timer/etraxfs_timer.c|  2 +-
>  hw/timer/npcm7xx_timer.c|  2 +-
>  hw/usb/hcd-dwc2.c   |  8 
>  hw/usb/xlnx-versal-usb2-ctrl-regs.c |  2 +-
>  hw/virtio/virtio-pci.c  |  2 +-
>  target/arm/cpu.c|  4 ++--
>  target/avr/cpu.c|  4 ++--
>  target/cris/cpu.c   |  4 ++--
>  target/hexagon/cpu.c|  4 ++--
>  target/i386/cpu.c   |  4 ++--
>  target/loongarch/cpu.c  |  4 ++--
>  target/m68k/cpu.c   |  4 ++--
>  target/microblaze/cpu.c |  4 ++--
>  target/mips/cpu.c   |  4 ++--
>  target/nios2/cpu.c 

Re: [PATCH v3 17/51] target/arm: Add cpu properties for SME

[Richard Henderson]

On 4/12/24 04:36, Peter Maydell wrote:

+  4) Disable the 512-bit vector length.  This results in all the other
+ lengths supported by ``max`` defaulting to enabled
+ (128, 256, 1024 and 2048)::
+
+ $ qemu-system-aarch64 -M virt -cpu max,sve512=off
+


I just noticed this while I was trying to understand the
SME and SVE property documentation -- the example 4 here
is in the SME property examples section, but it's changing
sve512, not sme512. Is that an error, or intentional?


Error.


In the SME section we say that all other lengths default
to enabled, but in the SVE section we say that the
smaller lengths default to enabled but the longer
lengths are disabled. Is:
  * the SVE part wrong?
  * the SME part wrong?
  * the behaviour deliberately different for SVE and SME
vector lengths? (If so, we should say so explicitly to
highlight that to users).


The behaviour is deliberately different.

See R_JRCSH and especially I_FQKMN:

For example, this means that the set of supported SVLs might be
discontiguous and might not start at the smallest permitted SVL.

whereas for SVE, the implementation is required to support all powers of 2 up to the 
maximum (see e.g. the text for ZCR_EL3.LEN).



r~

Re: [PULL 0/2] Final build system fixes for 9.0

[Peter Maydell]

On Fri, 12 Apr 2024 at 11:04, Paolo Bonzini  wrote:
>
> The following changes since commit 02e16ab9f4f19c4bdd17c51952d70e2ded74c6bf:
>
>   Update version for v9.0.0-rc3 release (2024-04-10 18:05:18 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 2d6d995709482cc8b6a76dbb5334a28001a14a9a:
>
>   meson.build: Disable -fzero-call-used-regs on OpenBSD (2024-04-12 12:02:12 
> +0200)
>
> 
> build system fixes
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0
for any user-visible changes.

-- PMM

Re: [PATCH v4 02/10] hw/core: create Resettable QOM interface

[Peter Maydell]

On Fri, 12 Apr 2024 at 14:38, Edgar E. Iglesias
 wrote:
>
> On Fri, Apr 12, 2024 at 3:05 PM Peter Maydell  
> wrote:
> >
> > On Thu, 11 Apr 2024 at 18:23, Philippe Mathieu-Daudé  
> > wrote:
> > >
> > > On 11/4/24 15:43, Peter Maydell wrote:
> > > > On Wed, 21 Aug 2019 at 17:34, Damien Hedde  
> > > > wrote:
> > > >>
> > > >> This commit defines an interface allowing multi-phase reset. This aims
> > > >> to solve a problem of the actual single-phase reset (built in
> > > >> DeviceClass and BusClass): reset behavior is dependent on the order
> > > >> in which reset handlers are called. In particular doing external
> > > >> side-effect (like setting an qemu_irq) is problematic because receiving
> > > >> object may not be reset yet.
> > > >
> > > > So, I wanted to drag up this ancient patch to ask a couple
> > > > of Resettable questions, because I'm working on adding a
> > > > new ResetType (the equivalent of SHUTDOWN_CAUSE_SNAPSHOT_LOAD).
> > > >
> > > >> +/**
> > > >> + * ResetType:
> > > >> + * Types of reset.
> > > >> + *
> > > >> + * + Cold: reset resulting from a power cycle of the object.
> > > >> + *
> > > >> + * TODO: Support has to be added to handle more types. In particular,
> > > >> + * ResetState structure needs to be expanded.
> > > >> + */
> > > >
> > > > Does anybody remember what this TODO comment is about? What
> > > > in particular would need to be in the ResetState struct
> > > > to allow another type to be added?
> > >
> > > IIRC this comes from this discussion:
> > > https://lore.kernel.org/qemu-devel/7c193b33-8188-2cda-cbf2-fb5452544...@greensocs.com/
> > > Updated in this patch (see after '---' description):
> > > https://lore.kernel.org/qemu-devel/20191018150630.31099-9-damien.he...@greensocs.com/
> >
> > Hmm, I can't see anything in there that mentions this
> > TODO or what we'd need more ResetState fields to handle.
> > I guess I'll go ahead with adding my new ResetType and ignore
> > this TODO, because I can't see any reason why we need to
> > do anything in particular for a new ResetType...
> >
> > > >
> > > >> +typedef enum ResetType {
> > > >> +RESET_TYPE_COLD,
> > > >> +} ResetType;
> > > >
> > > >> +typedef void (*ResettableInitPhase)(Object *obj, ResetType type);
> > > >> +typedef void (*ResettableHoldPhase)(Object *obj);
> > > >> +typedef void (*ResettableExitPhase)(Object *obj);
> > > >
> > > > Was there a reason why we only pass the ResetType to the init
> > > > phase method, and not also to the hold and exit phases ?
> > > > Given that many devices don't need to implement init, it
> > > > seems awkward to require them to do so just to stash the
> > > > ResetType somewhere so they can look at it in the hold
> > > > or exit phase, so I was thinking about adding the argument
> > > > to the other two phase methods.
> > >
> > > You are right, the type should be propagated to to all phase
> > > handlers.
> >
> > I have some patches which do this; I'll probably send them out
> > in a series next week once I've figured out whether they fit
> > better in with other patches that give the motivation.

> I don't remember the details on your first questions but I also agree
> with adding the type to the other callbacks!

I've now posted the series that adds the type the the hold
and exit callbacks, and adds a new RESET_TYPE_SNAPSHOT_LOAD:

https://patchew.org/QEMU/20240412160809.1260625-1-peter.mayd...@linaro.org/

thanks
-- PMM

[PATCH 1/6] hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr

[Peter Maydell]

The npcm7xx_clk and npcm7xx_gcr device reset methods look at
the ResetType argument and only handle RESET_TYPE_COLD,
producing a warning if another reset type is passed. This
is different from how every other three-phase-reset method
we have works, and makes it difficult to add new reset types.

A better pattern is "assume that any reset type you don't know
about should be handled like RESET_TYPE_COLD"; switch these
devices to do that. Then adding a new reset type will only
need to touch those devices where its behaviour really needs
to be different from the standard cold reset.

Signed-off-by: Peter Maydell 
---
 hw/misc/npcm7xx_clk.c | 13 +++--
 hw/misc/npcm7xx_gcr.c | 12 
 2 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/hw/misc/npcm7xx_clk.c b/hw/misc/npcm7xx_clk.c
index ac1622c38aa..2098c85ee01 100644
--- a/hw/misc/npcm7xx_clk.c
+++ b/hw/misc/npcm7xx_clk.c
@@ -873,20 +873,13 @@ static void npcm7xx_clk_enter_reset(Object *obj, 
ResetType type)
 
 QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
 
-switch (type) {
-case RESET_TYPE_COLD:
-memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
-s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-npcm7xx_clk_update_all_clocks(s);
-return;
-}
-
+memcpy(s->regs, cold_reset_values, sizeof(cold_reset_values));
+s->ref_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+npcm7xx_clk_update_all_clocks(s);
 /*
  * A small number of registers need to be reset on a core domain reset,
  * but no such reset type exists yet.
  */
-qemu_log_mask(LOG_UNIMP, "%s: reset type %d not implemented.",
-  __func__, type);
 }
 
 static void npcm7xx_clk_init_clock_hierarchy(NPCM7xxCLKState *s)
diff --git a/hw/misc/npcm7xx_gcr.c b/hw/misc/npcm7xx_gcr.c
index 9252f9d1488..c4c4e246d7e 100644
--- a/hw/misc/npcm7xx_gcr.c
+++ b/hw/misc/npcm7xx_gcr.c
@@ -159,14 +159,10 @@ static void npcm7xx_gcr_enter_reset(Object *obj, 
ResetType type)
 
 QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
 
-switch (type) {
-case RESET_TYPE_COLD:
-memcpy(s->regs, cold_reset_values, sizeof(s->regs));
-s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
-s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
-s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
-break;
-}
+memcpy(s->regs, cold_reset_values, sizeof(s->regs));
+s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
+s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
+s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
 }
 
 static void npcm7xx_gcr_realize(DeviceState *dev, Error **errp)
-- 
2.34.1

[PATCH 5/6] docs/devel/reset: Update to new API for hold and exit phase methods

[Peter Maydell]

Update the reset documentation's example code to match the new API
for the hold and exit phase method APIs where they take a ResetType
argument.

Signed-off-by: Peter Maydell 
---
 docs/devel/reset.rst | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst
index 2ea85e7779b..49baa1ea271 100644
--- a/docs/devel/reset.rst
+++ b/docs/devel/reset.rst
@@ -150,25 +150,25 @@ in reset.
 mydev->var = 0;
 }
 
-static void mydev_reset_hold(Object *obj)
+static void mydev_reset_hold(Object *obj, ResetType type)
 {
 MyDevClass *myclass = MYDEV_GET_CLASS(obj);
 MyDevState *mydev = MYDEV(obj);
 /* call parent class hold phase */
 if (myclass->parent_phases.hold) {
-myclass->parent_phases.hold(obj);
+myclass->parent_phases.hold(obj, type);
 }
 /* set an IO */
 qemu_set_irq(mydev->irq, 1);
 }
 
-static void mydev_reset_exit(Object *obj)
+static void mydev_reset_exit(Object *obj, ResetType type)
 {
 MyDevClass *myclass = MYDEV_GET_CLASS(obj);
 MyDevState *mydev = MYDEV(obj);
 /* call parent class exit phase */
 if (myclass->parent_phases.exit) {
-myclass->parent_phases.exit(obj);
+myclass->parent_phases.exit(obj, type);
 }
 /* clear an IO */
 qemu_set_irq(mydev->irq, 0);
-- 
2.34.1

[PATCH 4/6] hw, target: Add ResetType argument to hold and exit phase methods

[Peter Maydell]

We pass a ResetType argument to the Resettable class enter
phase method, but we don't pass it to hold and exit, even though
the callsites have it readily available. This means that if
a device cared about the ResetType it would need to record it
in the enter phase method to use later on. Pass the type to
all three of the phase methods to avoid having to do that.

Commit created with

  for dir in hw target include; do \
  spatch --macro-file scripts/cocci-macro-file.h \
 --sp-file scripts/coccinelle/reset-type.cocci \
 --keep-comments --smpl-spacing --in-place \
 --include-headers --dir $dir; done

and no manual edits.

Signed-off-by: Peter Maydell 
---
 include/hw/resettable.h |  4 ++--
 hw/adc/npcm7xx_adc.c|  2 +-
 hw/arm/pxa2xx_pic.c |  2 +-
 hw/arm/smmu-common.c|  2 +-
 hw/arm/smmuv3.c |  4 ++--
 hw/arm/stellaris.c  | 10 +-
 hw/audio/asc.c  |  2 +-
 hw/char/cadence_uart.c  |  2 +-
 hw/char/sifive_uart.c   |  2 +-
 hw/core/cpu-common.c|  2 +-
 hw/core/qdev.c  |  4 ++--
 hw/core/reset.c |  2 +-
 hw/core/resettable.c|  4 ++--
 hw/display/virtio-vga.c |  4 ++--
 hw/gpio/npcm7xx_gpio.c  |  2 +-
 hw/gpio/pl061.c |  2 +-
 hw/gpio/stm32l4x5_gpio.c|  2 +-
 hw/hyperv/vmbus.c   |  2 +-
 hw/i2c/allwinner-i2c.c  |  2 +-
 hw/i2c/npcm7xx_smbus.c  |  2 +-
 hw/input/adb.c  |  2 +-
 hw/input/ps2.c  | 12 ++--
 hw/intc/arm_gic_common.c|  2 +-
 hw/intc/arm_gic_kvm.c   |  4 ++--
 hw/intc/arm_gicv3_common.c  |  2 +-
 hw/intc/arm_gicv3_its.c |  4 ++--
 hw/intc/arm_gicv3_its_common.c  |  2 +-
 hw/intc/arm_gicv3_its_kvm.c |  4 ++--
 hw/intc/arm_gicv3_kvm.c |  4 ++--
 hw/intc/xics.c  |  2 +-
 hw/m68k/q800-glue.c |  2 +-
 hw/misc/djmemc.c|  2 +-
 hw/misc/iosb.c  |  2 +-
 hw/misc/mac_via.c   |  8 
 hw/misc/macio/cuda.c|  4 ++--
 hw/misc/macio/pmu.c |  4 ++--
 hw/misc/mos6522.c   |  2 +-
 hw/misc/npcm7xx_mft.c   |  2 +-
 hw/misc/npcm7xx_pwm.c   |  2 +-
 hw/misc/stm32l4x5_exti.c|  2 +-
 hw/misc/stm32l4x5_rcc.c | 10 +-
 hw/misc/stm32l4x5_syscfg.c  |  2 +-
 hw/misc/xlnx-versal-cframe-reg.c|  2 +-
 hw/misc/xlnx-versal-crl.c   |  2 +-
 hw/misc/xlnx-versal-pmc-iou-slcr.c  |  2 +-
 hw/misc/xlnx-versal-trng.c  |  2 +-
 hw/misc/xlnx-versal-xramc.c |  2 +-
 hw/misc/xlnx-zynqmp-apu-ctrl.c  |  2 +-
 hw/misc/xlnx-zynqmp-crf.c   |  2 +-
 hw/misc/zynq_slcr.c |  4 ++--
 hw/net/can/xlnx-zynqmp-can.c|  2 +-
 hw/net/e1000.c  |  2 +-
 hw/net/e1000e.c |  2 +-
 hw/net/igb.c|  2 +-
 hw/net/igbvf.c  |  2 +-
 hw/nvram/xlnx-bbram.c   |  2 +-
 hw/nvram/xlnx-versal-efuse-ctrl.c   |  2 +-
 hw/nvram/xlnx-zynqmp-efuse.c|  2 +-
 hw/pci-bridge/cxl_root_port.c   |  4 ++--
 hw/pci-bridge/pcie_root_port.c  |  2 +-
 hw/pci-host/bonito.c|  2 +-
 hw/pci-host/pnv_phb.c   |  4 ++--
 hw/pci-host/pnv_phb3_msi.c  |  4 ++--
 hw/pci/pci.c|  4 ++--
 hw/rtc/mc146818rtc.c|  2 +-
 hw/s390x/css-bridge.c   |  2 +-
 hw/sensor/adm1266.c |  2 +-
 hw/sensor/adm1272.c |  2 +-
 hw/sensor/isl_pmbus_vr.c| 10 +-
 hw/sensor/max31785.c|  2 +-
 hw/sensor/max34451.c|  2 +-
 hw/ssi/npcm7xx_fiu.c|  2 +-
 hw/timer/etraxfs_timer.c|  2 +-
 hw/timer/npcm7xx_timer.c|  2 +-
 hw/usb/hcd-dwc2.c   |  8 
 hw/usb/xlnx-versal-usb2-ctrl-regs.c |  2 +-
 hw/virtio/virtio-pci.c  |  2 +-
 target/arm/cpu.c|  4 ++--
 target/avr/cpu.c|  4 ++--
 target/cris/cpu.c   |  4 ++--
 target/hexagon/cpu.c|  4 ++--
 target/i386/cpu.c   |  4 ++--
 target/loongarch/cpu.c  |  4 ++--
 target/m68k/cpu.c   |  4 ++--
 target/microblaze/cpu.c |  4 ++--
 target/mips/cpu.c   |  4 ++--
 target/nios2/cpu.c  |  4 ++--
 target/openrisc/cpu.c   |  4 ++--
 target/ppc/cpu_init.c   |  4 ++--
 target/riscv/cpu.c  |  4 ++--
 target/rx/cpu.c |  4 ++--
 target/sh4/cpu.c|  4 ++--
 target/sparc/cpu.c  |  4 ++--
 t

[PATCH 0/6] reset: Add RESET_TYPE_SNAPSHOT_LOAD

[Peter Maydell]

Some devices and machines need to handle the reset before a vmsave
snapshot is loaded differently -- the main user is the handling of
RNG seed information, which does not want to put a new RNG seed into
a ROM blob when we are doing a snapshot load.

Currently this kind of reset handling is supported only for:
 * TYPE_MACHINE reset methods, which take a ShutdownCause argument
 * reset functions registered with qemu_register_reset_nosnapshotload

Adding a new ResetType that indicates the pre-snapshot-load reset
allows code that implements reset via Resettable to also do this.
As an immediate consequence, that means we can clean up the ugly
global variable in reset.c that we use to implement the
qemu_register_reset_nosnapshotload() callbacks. Slightly longer
term, I'm looking at making TYPE_MACHINE a subtype of TYPE_DEVICE,
and converting the non-standard reset method of MachineClass into
a Resettable as a result, so we'll need this ResetType there too.

Adding a new reset type makes plain an awkwardness in our current
Resettable API: we only pass the ResetType in to the 'enter' phase
method, not 'hold' and 'exit', even though we have it available
in the calling code. To avoid annoying workarounds like implementing
an 'enter' method that stashes the ResetType for the later 'hold'
or 'exit' method to use, we change the signatures of the 'hold'
and 'exit' methods to also pass the ResetType. Patch 3 is a
Coccinelle script which implements the transformation of the code,
and patch 4 is the result of running that script, with no manual
tweaks. This is really the biggest part of this patchset.

Patch 6 is the actual addition of the new ResetType, which isn't
very complicated. There is a TODO comment in resettable.h that
claims that "ResettableState structure has to be expanded" to
add more ResetTypes, but I can't see any reason why this should
be, and I didn't find any discussion in the mailing list archives
about it, so I have gone ahead anyway...

thanks
-- PMM

Peter Maydell (6):
  hw/misc: Don't special case RESET_TYPE_COLD in npcm7xx_clk, gcr
  allwinner-i2c, adm1272: Use device_cold_reset() for software-triggered
reset
  scripts/coccinelle: New script to add ResetType to hold and exit
phases
  hw, target: Add ResetType argument to hold and exit phase methods
  docs/devel/reset: Update to new API for hold and exit phase methods
  reset: Add RESET_TYPE_SNAPSHOT_LOAD

 docs/devel/reset.rst|  25 --
 scripts/coccinelle/reset-type.cocci | 133 
 include/hw/resettable.h |   5 +-
 hw/adc/npcm7xx_adc.c|   2 +-
 hw/arm/pxa2xx_pic.c |   2 +-
 hw/arm/smmu-common.c|   2 +-
 hw/arm/smmuv3.c |   4 +-
 hw/arm/stellaris.c  |  10 +--
 hw/audio/asc.c  |   2 +-
 hw/char/cadence_uart.c  |   2 +-
 hw/char/sifive_uart.c   |   2 +-
 hw/core/cpu-common.c|   2 +-
 hw/core/qdev.c  |   4 +-
 hw/core/reset.c |  17 ++--
 hw/core/resettable.c|   8 +-
 hw/display/virtio-vga.c |   4 +-
 hw/gpio/npcm7xx_gpio.c  |   2 +-
 hw/gpio/pl061.c |   2 +-
 hw/gpio/stm32l4x5_gpio.c|   2 +-
 hw/hyperv/vmbus.c   |   2 +-
 hw/i2c/allwinner-i2c.c  |   5 +-
 hw/i2c/npcm7xx_smbus.c  |   2 +-
 hw/input/adb.c  |   2 +-
 hw/input/ps2.c  |  12 +--
 hw/intc/arm_gic_common.c|   2 +-
 hw/intc/arm_gic_kvm.c   |   4 +-
 hw/intc/arm_gicv3_common.c  |   2 +-
 hw/intc/arm_gicv3_its.c |   4 +-
 hw/intc/arm_gicv3_its_common.c  |   2 +-
 hw/intc/arm_gicv3_its_kvm.c |   4 +-
 hw/intc/arm_gicv3_kvm.c |   4 +-
 hw/intc/xics.c  |   2 +-
 hw/m68k/q800-glue.c |   2 +-
 hw/misc/djmemc.c|   2 +-
 hw/misc/iosb.c  |   2 +-
 hw/misc/mac_via.c   |   8 +-
 hw/misc/macio/cuda.c|   4 +-
 hw/misc/macio/pmu.c |   4 +-
 hw/misc/mos6522.c   |   2 +-
 hw/misc/npcm7xx_clk.c   |  13 +--
 hw/misc/npcm7xx_gcr.c   |  12 +--
 hw/misc/npcm7xx_mft.c   |   2 +-
 hw/misc/npcm7xx_pwm.c   |   2 +-
 hw/misc/stm32l4x5_exti.c|   2 +-
 hw/misc/stm32l4x5_rcc.c |  10 +--
 hw/misc/stm32l4x5_syscfg.c  |   2 +-
 hw/misc/xlnx-versal-cframe-reg.c|   2 +-
 hw/misc/xlnx-versal-crl.c   |   2 +-
 hw/misc/xlnx-versal-pmc-iou-slcr.c  |   2 +-
 hw/misc/xlnx-versal-trng.c  |   2 +-
 hw/misc/xlnx-versal-xramc.c |   2 +-
 hw/misc/xlnx-zynqmp-apu-ctrl.c  |   2 +-
 hw/misc/xlnx-zynqmp-crf.c   |   2 +-
 hw/misc/zynq_slcr.c |   4 +-
 hw/net/can/xlnx-zynqmp-can.c|   2 +-
 hw/net/e1000.c

[PATCH 2/6] allwinner-i2c, adm1272: Use device_cold_reset() for software-triggered reset

[Peter Maydell]

Rather than directly calling the device's implementation of its 'hold'
reset phase, call device_cold_reset(). This means we don't have to
adjust this callsite when we add another argument to the function
signature for the hold and exit reset methods.

Signed-off-by: Peter Maydell 
---
 hw/i2c/allwinner-i2c.c | 3 +--
 hw/sensor/adm1272.c| 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/i2c/allwinner-i2c.c b/hw/i2c/allwinner-i2c.c
index 8abcc39a5c2..96c20c86372 100644
--- a/hw/i2c/allwinner-i2c.c
+++ b/hw/i2c/allwinner-i2c.c
@@ -385,8 +385,7 @@ static void allwinner_i2c_write(void *opaque, hwaddr offset,
 break;
 case TWI_SRST_REG:
 if (((value & TWI_SRST_MASK) == 0) && (s->srst & TWI_SRST_MASK)) {
-/* Perform reset */
-allwinner_i2c_reset_hold(OBJECT(s));
+device_cold_reset(DEVICE(s));
 }
 s->srst = value & TWI_SRST_MASK;
 break;
diff --git a/hw/sensor/adm1272.c b/hw/sensor/adm1272.c
index 1f7c8abb838..a19557ec9ea 100644
--- a/hw/sensor/adm1272.c
+++ b/hw/sensor/adm1272.c
@@ -386,7 +386,7 @@ static int adm1272_write_data(PMBusDevice *pmdev, const 
uint8_t *buf,
 break;
 
 case ADM1272_MFR_POWER_CYCLE:
-adm1272_exit_reset((Object *)s);
+device_cold_reset(DEVICE(s));
 break;
 
 case ADM1272_HYSTERESIS_LOW:
-- 
2.34.1

[PATCH 6/6] reset: Add RESET_TYPE_SNAPSHOT_LOAD

[Peter Maydell]

Some devices and machines need to handle the reset before a vmsave
snapshot is loaded differently -- the main user is the handling of
RNG seed information, which does not want to put a new RNG seed into
a ROM blob when we are doing a snapshot load.

Currently this kind of reset handling is supported only for:
 * TYPE_MACHINE reset methods, which take a ShutdownCause argument
 * reset functions registered with qemu_register_reset_nosnapshotload

To allow a three-phase-reset device to also distinguish "snapshot
load" reset from the normal kind, add a new ResetType
RESET_TYPE_SNAPSHOT_LOAD. All our existing reset methods ignore
the reset type, so we don't need to update any device code.

Add the enum type, and make qemu_devices_reset() use the
right reset type for the ShutdownCause it is passed. This
allows us to get rid of the device_reset_reason global we
were using to implement qemu_register_reset_nosnapshotload().

Signed-off-by: Peter Maydell 
---
 docs/devel/reset.rst| 17 ++---
 include/hw/resettable.h |  1 +
 hw/core/reset.c | 15 ---
 hw/core/resettable.c|  4 
 4 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst
index 49baa1ea271..9746a4e8a0b 100644
--- a/docs/devel/reset.rst
+++ b/docs/devel/reset.rst
@@ -27,9 +27,7 @@ instantly reset an object, without keeping it in reset state, 
just call
 ``resettable_reset()``. These functions take two parameters: a pointer to the
 object to reset and a reset type.
 
-Several types of reset will be supported. For now only cold reset is defined;
-others may be added later. The Resettable interface handles reset types with an
-enum:
+The Resettable interface handles reset types with an enum ``ResetType``:
 
 ``RESET_TYPE_COLD``
   Cold reset is supported by every resettable object. In QEMU, it means we 
reset
@@ -37,6 +35,19 @@ enum:
   from what is a real hardware cold reset. It differs from other resets (like
   warm or bus resets) which may keep certain parts untouched.
 
+``RESET_TYPE_SNAPSHOT_LOAD``
+  This is called for a reset which is being done to put the system into a
+  clean state prior to loading a snapshot. (This corresponds to a reset
+  with ``SHUTDOWN_CAUSE_SNAPSHOT_LOAD``.) Almost all devices should treat
+  this the same as ``RESET_TYPE_COLD``. The main exception is devices which
+  have some non-deterministic state they want to reinitialize to a different
+  value on each cold reset, such as RNG seed information, and which they
+  must not reinitialize on a snapshot-load reset.
+
+Devices which implement reset methods must treat any unknown ``ResetType``
+as equivalent to ``RESET_TYPE_COLD``; this will reduce the amount of
+existing code we need to change if we add more types in future.
+
 Calling ``resettable_reset()`` is equivalent to calling
 ``resettable_assert_reset()`` then ``resettable_release_reset()``. It is
 possible to interleave multiple calls to these three functions. There may
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
index 3161e471c9b..7e249deb8b5 100644
--- a/include/hw/resettable.h
+++ b/include/hw/resettable.h
@@ -35,6 +35,7 @@ typedef struct ResettableState ResettableState;
  */
 typedef enum ResetType {
 RESET_TYPE_COLD,
+RESET_TYPE_SNAPSHOT_LOAD,
 } ResetType;
 
 /*
diff --git a/hw/core/reset.c b/hw/core/reset.c
index f9fef45e050..58dfc8db3dc 100644
--- a/hw/core/reset.c
+++ b/hw/core/reset.c
@@ -43,13 +43,6 @@ static ResettableContainer *get_root_reset_container(void)
 return root_reset_container;
 }
 
-/*
- * Reason why the currently in-progress qemu_devices_reset() was called.
- * If we made at least SHUTDOWN_CAUSE_SNAPSHOT_LOAD have a corresponding
- * ResetType we could perhaps avoid the need for this global.
- */
-static ShutdownCause device_reset_reason;
-
 /*
  * This is an Object which implements Resettable simply to call the
  * callback function in the hold phase.
@@ -77,8 +70,7 @@ static void legacy_reset_hold(Object *obj, ResetType type)
 {
 LegacyReset *lr = LEGACY_RESET(obj);
 
-if (device_reset_reason == SHUTDOWN_CAUSE_SNAPSHOT_LOAD &&
-lr->skip_on_snapshot_load) {
+if (type == RESET_TYPE_SNAPSHOT_LOAD && lr->skip_on_snapshot_load) {
 return;
 }
 lr->func(lr->opaque);
@@ -180,8 +172,9 @@ void qemu_unregister_resettable(Object *obj)
 
 void qemu_devices_reset(ShutdownCause reason)
 {
-device_reset_reason = reason;
+ResetType type = (reason == SHUTDOWN_CAUSE_SNAPSHOT_LOAD) ?
+RESET_TYPE_SNAPSHOT_LOAD : RESET_TYPE_COLD;
 
 /* Reset the simulation */
-resettable_reset(OBJECT(get_root_reset_container()), RESET_TYPE_COLD);
+resettable_reset(OBJECT(get_root_reset_container()), type);
 }
diff --git a/hw/core/resettable.c b/hw/core/resettable.c
index bebf7f10b26..6dd3e3dc487 100644
--- a/hw/core/resettable.c
+++ b/hw/core/resettable.c
@@ -48,8 +48,6 @@ void resettable_reset(Object *obj, ResetType type)
 
 void resettable_ass

[PATCH 3/6] scripts/coccinelle: New script to add ResetType to hold and exit phases

[Peter Maydell]

We pass a ResetType argument to the Resettable class enter phase
method, but we don't pass it to hold and exit, even though the
callsites have it readily available.  This means that if a device
cared about the ResetType it would need to record it in the enter
phase method to use later on.  We should pass the type to all three
of the phase methods to avoid having to do that.

This coccinelle script adds the ResetType argument to the hold and
exit phases of the Resettable interface.

The first part of the script (rules holdfn_assigned, holdfn_defined,
exitfn_assigned, exitfn_defined) update implementations of the
interface within device models, both to change the signature of their
method implementations and to pass on the reset type when they invoke
reset on some other device.

The second part of the script is various special cases:
 * method callsites in resettable_phase_hold(), resettable_phase_exit()
   and device_phases_reset()
 * updating the typedefs for the methods
 * isl_pmbus_vr.c has some code where one device's reset method directly
   calls the implementation of a different device's method

Signed-off-by: Peter Maydell 
---
The structure here is a bit of an experiment: usually I would make
the coccinelle script cover the main mechanical change and do the
special cases by hand-editing. But I thought it might be clearer to
have the entire next commit be made by coccinelle, so reviewers don't
have to go hunting through a 99% automated commit for the 1% hand
written part. Let me know whether you like this or not...
---
 scripts/coccinelle/reset-type.cocci | 133 
 1 file changed, 133 insertions(+)
 create mode 100644 scripts/coccinelle/reset-type.cocci

diff --git a/scripts/coccinelle/reset-type.cocci 
b/scripts/coccinelle/reset-type.cocci
new file mode 100644
index 000..14abdd7bd0c
--- /dev/null
+++ b/scripts/coccinelle/reset-type.cocci
@@ -0,0 +1,133 @@
+// Convert device code using three-phase reset to add a ResetType
+// argument to implementations of ResettableHoldPhase and
+// ResettableEnterPhase methods.
+//
+// Copyright Linaro Ltd 2024
+// SPDX-License-Identifier: GPL-2.0-or-later
+//
+// for dir in include hw target; do \
+// spatch --macro-file scripts/cocci-macro-file.h \
+//--sp-file scripts/coccinelle/reset-type.cocci \
+//--keep-comments --smpl-spacing --in-place --include-headers \
+//--dir $dir; done
+//
+// This coccinelle script aims to produce a complete change that needs
+// no human interaction, so as well as the generic "update device
+// implementations of the hold and exit phase methods" it includes
+// the special-case transformations needed for the core code and for
+// one device model that does something a bit nonstandard. Those
+// special cases are at the end of the file.
+
+// Look for where we use a function as a ResettableHoldPhase method,
+// either by directly assigning it to phases.hold or by calling
+// resettable_class_set_parent_phases, and remember the function name.
+@ holdfn_assigned @
+identifier enterfn, holdfn, exitfn;
+identifier rc;
+expression e;
+@@
+ResettableClass *rc;
+...
+(
+ rc->phases.hold = holdfn;
+|
+ resettable_class_set_parent_phases(rc, enterfn, holdfn, exitfn, e);
+)
+
+// Look for the definition of the function we found in holdfn_assigned,
+// and add the new argument. If the function calls a hold function
+// itself (probably chaining to the parent class reset) then add the
+// new argument there too.
+@ holdfn_defined @
+identifier holdfn_assigned.holdfn;
+typedef Object;
+identifier obj;
+expression parent;
+@@
+-holdfn(Object *obj)
++holdfn(Object *obj, ResetType type)
+{
+<...
+-parent.hold(obj)
++parent.hold(obj, type)
+...>
+}
+
+// Similarly for ResettableExitPhase.
+@ exitfn_assigned @
+identifier enterfn, holdfn, exitfn;
+identifier rc;
+expression e;
+@@
+ResettableClass *rc;
+...
+(
+ rc->phases.exit = exitfn;
+|
+ resettable_class_set_parent_phases(rc, enterfn, holdfn, exitfn, e);
+)
+@ exitfn_defined @
+identifier exitfn_assigned.exitfn;
+typedef Object;
+identifier obj;
+expression parent;
+@@
+-exitfn(Object *obj)
++exitfn(Object *obj, ResetType type)
+{
+<...
+-parent.exit(obj)
++parent.exit(obj, type)
+...>
+}
+
+// SPECIAL CASES ONLY BELOW HERE
+// We use a python scripted constraint on the position of the match
+// to ensure that they only match in a particular function. See
+// https://public-inbox.org/git/alpine.DEB.2.21.1808240652370.2344@hadrien/
+// which recommends this as the way to do "match only in this function".
+
+// Special case: isl_pmbus_vr.c has some reset methods calling others directly
+@ isl_pmbus_vr @
+identifier obj;
+@@
+- isl_pmbus_vr_exit_reset(obj);
++ isl_pmbus_vr_exit_reset(obj, type);
+
+// Special case: device_phases_reset() needs to pass RESET_TYPE_COLD
+@ device_phases_reset_hold @
+expression obj;
+identifier rc;
+identifier phase;
+position p : script:python() { p[0].current_element == "device

Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()

[Daniel Henrique Barboza]

On 4/12/24 11:15, Aleksei Filippov wrote:



On 09.04.2024 20:52, Daniel Henrique Barboza wrote:

raise_mmu_exception(), as is today, is prioritizing guest page faults by
checking first if virt_enabled && !first_stage, and then considering the
regular inst/load/store faults.

There's no mention in the spec about guest page fault being a higher
priority that PMP faults. In fact, privileged spec section 3.7.1 says:

"Attempting to fetch an instruction from a PMP region that does not have
execute permissions raises an instruction access-fault exception.
Attempting to execute a load or load-reserved instruction which accesses
a physical address within a PMP region without read permissions raises a
load access-fault exception. Attempting to execute a store,
store-conditional, or AMO instruction which accesses a physical address
within a PMP region without write permissions raises a store
access-fault exception."

So, in fact, we're doing it wrong - PMP faults should always be thrown,
regardless of also being a first or second stage fault.

The way riscv_cpu_tlb_fill() and get_physical_address() work is
adequate: a TRANSLATE_PMP_FAIL error is immediately reported and
reflected in the 'pmp_violation' flag. What we need is to change
raise_mmu_exception() to prioritize it.

Reported-by: Joseph Chan 
Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU translation 
stage")
Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/cpu_helper.c | 22 --
  1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index fc090d729a..e3a7797d00 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1176,28 +1176,30 @@ static void raise_mmu_exception(CPURISCVState *env, 
target_ulong address,

  switch (access_type) {
  case MMU_INST_FETCH:
-    if (env->virt_enabled && !first_stage) {
+    if (pmp_violation) {
+    cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
+    } else if (env->virt_enabled && !first_stage) {
  cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
  } else {
-    cs->exception_index = pmp_violation ?
-    RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
+    cs->exception_index = RISCV_EXCP_INST_PAGE_FAULT;
  }
  break;
  case MMU_DATA_LOAD:
-    if (two_stage && !first_stage) {
+    if (pmp_violation) {
+    cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
+    } else if (two_stage && !first_stage) {
  cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
  } else {
-    cs->exception_index = pmp_violation ?
-    RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
+    cs->exception_index = RISCV_EXCP_LOAD_PAGE_FAULT;
  }
  break;
  case MMU_DATA_STORE:
-    if (two_stage && !first_stage) {
+    if (pmp_violation) {
+    cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+    } else if (two_stage && !first_stage) {
  cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
  } else {
-    cs->exception_index = pmp_violation ?
-    RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
-    RISCV_EXCP_STORE_PAGE_FAULT;
+    cs->exception_index = RISCV_EXCP_STORE_PAGE_FAULT;
  }
  break;
  default:



Just tested your patch and found out that we still need to fix `if else` in
riscv_cpu_tlb_fill() after pmp check in 2 stage translation part, as I suggested
before, cz the problem with mtval2 will happened in case of successes 2 stage
translation but failed pmp check. In this case we gonna set
mtval2(env->guest_phys_fault_addr in context of riscv_cpu_tlb_fill()) as this
was a guest-page-fault, but it didn't and mtval2 should be zero, according to
RISCV privileged spec sect. 9.4.4: When a guest page-fault is taken into M-mode,
mtval2 is written with either zero or guest physical address that faulted,
shifted by 2 bits. *For other traps, mtval2 is set to zero...*


Thanks for giving it a go. You're right, this patch alone is not enough and 
we'll
need your patch too.

But note that, with what you've said in mind, your patch will also end up 
setting
mtval2 and env->guest_phys_fault_addr in case a PMP fault occurs during the
get_physical_address() right at the start of second stage:

if (ret == TRANSLATE_SUCCESS) {
/* Second stage lookup */
im_address = pa;

ret = get_physical_address(env, &pa, &prot2, im_address, NULL,
   access_type, MMUIdx_U, false, true,
   false);


I think your patch needs to also prevent env->guest_phys_fault_addr to be set 
when
ret == TRANSLATE_PMP_FAIL.

With these changes in your patch, and this patch, we're free to set 
"first_stage_error = fa

Re: [PATCH 02/12] hw/vfio/pci: Replace sprintf() by g_strdup_printf()

[Alex Williamson]

On Wed, 10 Apr 2024 18:06:03 +0200
Philippe Mathieu-Daudé  wrote:

> sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
> resulting in painful developper experience. Use g_strdup_printf()
> instead.

Isn't this code only compiled for Linux hosts?  Maybe still a valid
change, but the rationale seems irrelevant.  Thanks,

Alex

> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/vfio/pci.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 64780d1b79..cc3cc89122 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2442,10 +2442,9 @@ void vfio_pci_post_reset(VFIOPCIDevice *vdev)
>  
>  bool vfio_pci_host_match(PCIHostDeviceAddress *addr, const char *name)
>  {
> -char tmp[13];
> -
> -sprintf(tmp, "%04x:%02x:%02x.%1x", addr->domain,
> -addr->bus, addr->slot, addr->function);
> +g_autofree char *tmp = g_strdup_printf("%04x:%02x:%02x.%1x",
> +   addr->domain, addr->bus,
> +   addr->slot, addr->function);
>  
>  return (strcmp(tmp, name) == 0);
>  }

Re: [PATCH] dma-helpers: Fix iovec alignment

[Eric Blake]

On Fri, Apr 12, 2024 at 10:06:17AM +0200, Stefan Fritsch wrote:
> Commit 99868af3d0 changed the hardcoded constant BDRV_SECTOR_SIZE to a
> dynamic field 'align' but introduced a bug. qemu_iovec_discard_back()
> is now passed the wanted iov length instead of the actually required
> amount that should be removed from the end of the iov.
> 
> The bug can likely only be hit in uncommon configurations, e.g. with
> icount enabled or when reading from disk directly to device memory.
> 
> Fixes: 99868af3d0a75cf6 ("dma-helpers: explicitly pass alignment into DMA 
> helpers")
> Signed-off-by: Stefan Fritsch 
> ---
>  system/dma-helpers.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Wow, that bug has been latent for a while (2016, v2.8.0).  As such, I
don't think it's worth holding up 9.0 for, but it is definitely worth
cc'ing qemu-stable (done now).

> 
> diff --git a/system/dma-helpers.c b/system/dma-helpers.c
> index 9b221cf94e..c9677fd39b 100644
> --- a/system/dma-helpers.c
> +++ b/system/dma-helpers.c
> @@ -174,8 +174,7 @@ static void dma_blk_cb(void *opaque, int ret)
>  }
>  
>  if (!QEMU_IS_ALIGNED(dbs->iov.size, dbs->align)) {
> -qemu_iovec_discard_back(&dbs->iov,
> -QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align));
> +qemu_iovec_discard_back(&dbs->iov, dbs->iov.size % dbs->align);

Before the regression, it was:

-if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
-qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK);
+if (!QEMU_IS_ALIGNED(dbs->iov.size, dbs->align)) {
+qemu_iovec_discard_back(&dbs->iov,

If dbs->align is always a power of two, we can use '& (dbs->align -
1)' to avoid a hardware division, to match the original code; bug
QEMU_IS_ALIGNED does not require a power of two, so your choice of '%
dbs->align' seems reasonable.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 1/4] Revert "migration: modify test_multifd_tcp_none() to use new QAPI syntax"

[Fabiano Rosas]

Peter Xu  writes:

> On Thu, Apr 11, 2024 at 04:41:16PM -0300, Fabiano Rosas wrote:
>> Peter Xu  writes:
>> 
>> > On Thu, Apr 11, 2024 at 11:31:08PM +0530, Het Gala wrote:
>> >> I just wanted to highlight couple of pointers:
>> >> 1. though we are using 'channels' in the precopy tests for 'migrate' QAPI,
>> >> we
>> >>    use the old uri for 'migrate-incoming' QAPI.
>> >> 2. We do not cover other 'channels' abi, only have tcp path tested.
>> >> 
>> >> So, the TO-DOs could be:
>> >> 1. Omit the 4th patch here, which introduced postcopy qtests with 
>> >> 'channels'
>> >>    interface OR have 'channels' interface with other than tcp transport
>> >>    (file, exec, vsock, etc) so as to cover different code paths.
>> >> 2. Extend channels interface to migrate-incoming QAPI for precopy qtests
>> >
>> > You can see whether Fabiano has anything to say, but what you proposed
>> > looks good to me.
>> 
>> Ok, so what about we convert some of the 'plain' tests into channels to
>> cover all transports?
>> 
>> - tcp: test_multifd_tcp_none  (this one we already did)
>> - file: test_precopy_file
>> - unix: test_precopy_unix_plain
>> - exec: test_analyze_script
>> - fd: test_migrate_precopy_fd_socket
>> 
>> Those^, plus the validate_uri that's already in next should cover
>> everything.
>> 
>> We don't need to do this at once, by the way.
>> 
>> Moreover:
>> 
>> - leave all test strings untouched to preserve bisecting;
>> 
>> - let's not bother adding "channels" and "uri" to the test string
>>   anymore. The channels API should be taken for granted at this point, I
>>   don't expect we start hitting bugs that will require us to run either
>>   foo/uri/plain or foo/channels/plain, so there's not much point in
>>   making the distinction.
>
> Do you mean we can put "uri:" aside?  Maybe I misunderstood..

I mean the test name does not need to specify "channels" vs. "uri"
because that should never be broken to the point that we actually need
to go fetch those tests by name. We'd still have at least 1 test for
each transport with channels and (existing) at least 1 test for each
transport with uri.

>
> The matrix previously was (I think.. when this series posted):
>
>   [tcp, unix, file, exec, fd] x [uri, channels] x [precopy, postcopy]
>
> Drop postcopy as doesn't seem to have any special paths:
>
>   [tcp, unix, file, exec, fd] x [uri, channels]
>
> So logically we should still cover these, right?

Right, I'm just suggesting we convert some tests to use channels, one
for each transport, to test the channels API in full. The rest of the
existing tests as well as future tests need not have a uri (or channel)
variant. Just one of them is enough.

Re: [PATCH v2 13/13] util/uri: Remove the old URI parsing code

[Eric Blake]

On Fri, Apr 12, 2024 at 03:24:15PM +0200, Thomas Huth wrote:
> Now that we switched all consumers of the URI code to use the URI
> parsing functions from glib instead, we can remove our internal
> URI parsing code since it is not used anymore.
> 
> Signed-off-by: Thomas Huth 
> ---
>  include/qemu/uri.h |   99 ---
>  util/uri.c | 1466 
>  util/meson.build   |2 +-
>  3 files changed, 1 insertion(+), 1566 deletions(-)
>  delete mode 100644 include/qemu/uri.h
>  delete mode 100644 util/uri.c

Nice.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 12/13] block/ssh: Use URI parsing code from glib

[Eric Blake]

On Fri, Apr 12, 2024 at 03:24:14PM +0200, Thomas Huth wrote:
> Since version 2.66, glib has useful URI parsing functions, too.
> Use those instead of the QEMU-internal ones to be finally able
> to get rid of the latter.
> 
> Reviewed-by: Richard W.M. Jones 
> Signed-off-by: Thomas Huth 
> ---
>  block/ssh.c | 75 -
>  1 file changed, 40 insertions(+), 35 deletions(-)
> 

>  
> -if (g_strcmp0(uri->scheme, "ssh") != 0) {
> +if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) {

Yet another case-sensitive spot to consider.

>  
> -qdict_put_str(options, "path", uri->path);
> -
> -/* Pick out any query parameters that we understand, and ignore
> - * the rest.
> - */
> -for (i = 0; i < qp->n; ++i) {
> -if (strcmp(qp->p[i].name, "host_key_check") == 0) {
> -qdict_put_str(options, "host_key_check", qp->p[i].value);
> +qdict_put_str(options, "path", uri_path);
> +
> +uri_query = g_uri_get_query(uri);
> +if (uri_query) {
> +g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE);
> +while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) {
> +if (!qp_name || !qp_value || gerror) {
> +warn_report("Failed to parse SSH URI parameters '%s'.",
> +uri_query);
> +break;
> +}
> +/*
> + * Pick out the query parameters that we understand, and ignore
> + * (or rather warn about) the rest.
> + */
> +if (g_str_equal(qp_name, "host_key_check")) {
> +qdict_put_str(options, "host_key_check", qp_value);
> +} else {
> +warn_report("Unsupported parameter '%s' in URI.", qp_name);

Do we want the trailing '.' in warn_report?

The warning is new; it was not in the old code, nor mentioned in the
commit message.  It seems like a good idea, but we should be more
intentional if we intend to make that change.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PULL 0/2] Final build system fixes for 9.0

[Peter Maydell]

On Fri, 12 Apr 2024 at 11:04, Paolo Bonzini  wrote:
>
> The following changes since commit 02e16ab9f4f19c4bdd17c51952d70e2ded74c6bf:
>
>   Update version for v9.0.0-rc3 release (2024-04-10 18:05:18 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 2d6d995709482cc8b6a76dbb5334a28001a14a9a:
>
>   meson.build: Disable -fzero-call-used-regs on OpenBSD (2024-04-12 12:02:12 
> +0200)
>
> 
> build system fixes
>
> 
> Matheus Tavares Bernardino (1):
>   Makefile: fix use of -j without an argument
>
> Thomas Huth (1):
>   meson.build: Disable -fzero-call-used-regs on OpenBSD

Something's gone wrong with your pullreq : that tags/for-upstream
doesn't have these commits, it still has the contents from your
April 9th pullreq.

thanks
-- PMM

Re: [PATCH v3 24/27] hw/net/rocker: Replace sprintf() by snprintf()

[Richard Henderson]

On 4/12/24 03:58, Philippe Mathieu-Daudé wrote:

On 12/4/24 09:33, Richard Henderson wrote:

From: Philippe Mathieu-Daudé 

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience. Use snprintf() instead.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240411104340.6617-7-phi...@linaro.org>
Signed-off-by: Richard Henderson 
---
  hw/net/rocker/rocker.c | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)




  switch (offset) {
  case ROCKER_DMA_DESC_ADDR_OFFSET:
-    sprintf(buf, "Ring[%s] ADDR", ring_name);
+    snprintf(buf, sizeofbuf), "Ring[%s] ADDR", ring_name);


Ideally we should convert the DEBUG_FOO guards to trace events,
to avoid to maintain dead code.


Grr, I knew there was another of these that needed fixing up.


r~

Re: [PATCH v2 11/13] block/nfs: Use URI parsing code from glib

[Eric Blake]

On Fri, Apr 12, 2024 at 03:24:13PM +0200, Thomas Huth wrote:
> Since version 2.66, glib has useful URI parsing functions, too.
> Use those instead of the QEMU-internal ones to be finally able
> to get rid of the latter.
> 
> Signed-off-by: Thomas Huth 
> ---
>  block/nfs.c | 110 ++--
>  1 file changed, 54 insertions(+), 56 deletions(-)
> 

>  }
> -if (g_strcmp0(uri->scheme, "nfs") != 0) {
> +if (!g_str_equal(g_uri_get_scheme(uri), "nfs")) {

Another case where we should be considering whether g_ascii_strcasecmp
is better, as a separate patch.

> -for (i = 0; i < qp->n; i++) {
> -uint64_t val;
> -if (!qp->p[i].value) {
> -error_setg(errp, "Value for NFS parameter expected: %s",
> -   qp->p[i].name);
> -goto out;
> -}
> -if (parse_uint_full(qp->p[i].value, 0, &val)) {
> -error_setg(errp, "Illegal value for NFS parameter: %s",

Pre-existing,...

> +
> +uri_query = g_uri_get_query(uri);
> +if (uri_query) {
> +g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE);
> +while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) {
> +uint64_t val;
> +if (!qp_name || gerror) {
> +error_setg(errp, "Failed to parse NFS parameter");
> +return -EINVAL;
> +}
> +if (!qp_value) {
> +error_setg(errp, "Value for NFS parameter expected: %s",
> +   qp_name);
> +return -EINVAL;
> +}
> +if (parse_uint_full(qp_value, 0, &val)) {
> +error_setg(errp, "Illegal value for NFS parameter: %s",

...but since we're touching it, I prefer 'Invalid' over 'Illegal' (any
error message implying that you broke a law when you passed in bad
data is a bit aggressive).  Not a show-stopper to leave it alone.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 10/13] block/nbd: Use URI parsing code from glib

[Eric Blake]

On Fri, Apr 12, 2024 at 03:24:12PM +0200, Thomas Huth wrote:
> Since version 2.66, glib has useful URI parsing functions, too.
> Use those instead of the QEMU-internal ones to be finally able
> to get rid of the latter. The g_uri_get_host() also takes care
> of removing the square brackets from IPv6 addresses, so we can
> drop that part of the QEMU code now, too.
> 
> Reviewed-by: Richard W.M. Jones 
> Signed-off-by: Thomas Huth 
> ---
>  block/nbd.c | 76 ++---
>  1 file changed, 37 insertions(+), 39 deletions(-)
> 

>  
>  /* transport */
> -if (!g_strcmp0(uri->scheme, "nbd")) {
> +uri_scheme = g_uri_get_scheme(uri);
> +if (!g_strcmp0(uri_scheme, "nbd")) {
>  is_unix = false;

As with gluster, we should probably be using g_ascii_strcasecmp() here
to match the RFC; again, worth a separate patch.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH v2 09/13] block/gluster: Use URI parsing code from glib

[Eric Blake]

On Fri, Apr 12, 2024 at 09:40:18AM -0500, Eric Blake wrote:
> > @@ -364,57 +363,57 @@ static int 
> > qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
> >  QAPI_LIST_PREPEND(gconf->server, gsconf);
> >  
> >  /* transport */
> > -if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
> > +uri_scheme = g_uri_get_scheme(uri);
> > +if (!uri_scheme || !strcmp(uri_scheme, "gluster")) {
> 
> Pre-existing, but per RFC 3986, we should probably be using strcasecmp
> for scheme comparisons (I'm not sure if g_uri_parse guarantees a
> lower-case return, even when the user passed in upper case).  That can
> be a separate patch.

Even beter, g_ascii_strcasecmp() (since strcasecmp can be
locale-specific which is generally not what we need here)


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

[PATCH for-9.0?] usb-storage: Fix BlockConf defaults

[Kevin Wolf]

Commit 30896374 started to pass the full BlockConf from usb-storage to
scsi-disk, while previously only a few select properties would be
forwarded. This enables the user to set more properties, e.g. the block
size, that are actually taking effect.

However, now the calls to blkconf_apply_backend_options() and
blkconf_blocksizes() in usb_msd_storage_realize() that modify some of
these properties take effect, too, instead of being silently ignored.
This means at least that the block sizes get an unconditional default of
512 bytes before the configuration is passed to scsi-disk.

Before commit 30896374, the property wouldn't be set for scsi-disk and
therefore the device dependent defaults would apply - 512 for scsi-hd,
but 2048 for scsi-cd. The latter default has now become 512, too, which
makes at least Windows 11 installation fail when installing from
usb-storage.

Fix this by simply not calling these functions any more in usb-storage
and passing BlockConf on unmodified (except for the BlockBackend). The
same functions are called by the SCSI code anyway and it sets the right
defaults for the actual media type.

Fixes: 308963746169 ('scsi: Don't ignore most usb-storage properties')
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2260
Reported-by: Jonas Svensson
Signed-off-by: Kevin Wolf 
---
Considering this a candidate for 9.0 given that we're already having an
rc4, it's a regression from 8.2 and breaks installing Windows from USB

 hw/usb/dev-storage-classic.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/hw/usb/dev-storage-classic.c b/hw/usb/dev-storage-classic.c
index 50a3ad6285..6147387dc6 100644
--- a/hw/usb/dev-storage-classic.c
+++ b/hw/usb/dev-storage-classic.c
@@ -38,15 +38,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error 
**errp)
 return;
 }
 
-if (!blkconf_blocksizes(&s->conf, errp)) {
-return;
-}
-
-if (!blkconf_apply_backend_options(&s->conf, !blk_supports_write_perm(blk),
-   true, errp)) {
-return;
-}
-
 /*
  * Hack alert: this pretends to be a block device, but it's really
  * a SCSI bus that can serve only a single device, which it
-- 
2.44.0

Re: [PATCH v2 09/13] block/gluster: Use URI parsing code from glib

[Eric Blake]

On Fri, Apr 12, 2024 at 03:24:11PM +0200, Thomas Huth wrote:
> Since version 2.66, glib has useful URI parsing functions, too.
> Use those instead of the QEMU-internal ones to be finally able
> to get rid of the latter.
> 
> Signed-off-by: Thomas Huth 
> ---
>  block/gluster.c | 71 -
>  1 file changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/block/gluster.c b/block/gluster.c
> index cc74af06dc..1c9505f8bb 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -17,7 +17,6 @@
>  #include "qapi/error.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qerror.h"
> -#include "qemu/uri.h"
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "qemu/option.h"
> @@ -289,9 +288,9 @@ static void glfs_clear_preopened(glfs_t *fs)
>  }
>  }
>  
> -static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path)
> +static int parse_volume_options(BlockdevOptionsGluster *gconf, const char 
> *path)

Is it worth mentioning in the commit message that this includes a
const-correctness tweak?

> @@ -364,57 +363,57 @@ static int 
> qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
>  QAPI_LIST_PREPEND(gconf->server, gsconf);
>  
>  /* transport */
> -if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
> +uri_scheme = g_uri_get_scheme(uri);
> +if (!uri_scheme || !strcmp(uri_scheme, "gluster")) {

Pre-existing, but per RFC 3986, we should probably be using strcasecmp
for scheme comparisons (I'm not sure if g_uri_parse guarantees a
lower-case return, even when the user passed in upper case).  That can
be a separate patch.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org

Re: [PATCH 1/4] Revert "migration: modify test_multifd_tcp_none() to use new QAPI syntax"

[Peter Xu]

On Thu, Apr 11, 2024 at 04:41:16PM -0300, Fabiano Rosas wrote:
> Peter Xu  writes:
> 
> > On Thu, Apr 11, 2024 at 11:31:08PM +0530, Het Gala wrote:
> >> I just wanted to highlight couple of pointers:
> >> 1. though we are using 'channels' in the precopy tests for 'migrate' QAPI,
> >> we
> >>    use the old uri for 'migrate-incoming' QAPI.
> >> 2. We do not cover other 'channels' abi, only have tcp path tested.
> >> 
> >> So, the TO-DOs could be:
> >> 1. Omit the 4th patch here, which introduced postcopy qtests with 
> >> 'channels'
> >>    interface OR have 'channels' interface with other than tcp transport
> >>    (file, exec, vsock, etc) so as to cover different code paths.
> >> 2. Extend channels interface to migrate-incoming QAPI for precopy qtests
> >
> > You can see whether Fabiano has anything to say, but what you proposed
> > looks good to me.
> 
> Ok, so what about we convert some of the 'plain' tests into channels to
> cover all transports?
> 
> - tcp: test_multifd_tcp_none  (this one we already did)
> - file: test_precopy_file
> - unix: test_precopy_unix_plain
> - exec: test_analyze_script
> - fd: test_migrate_precopy_fd_socket
> 
> Those^, plus the validate_uri that's already in next should cover
> everything.
> 
> We don't need to do this at once, by the way.
> 
> Moreover:
> 
> - leave all test strings untouched to preserve bisecting;
> 
> - let's not bother adding "channels" and "uri" to the test string
>   anymore. The channels API should be taken for granted at this point, I
>   don't expect we start hitting bugs that will require us to run either
>   foo/uri/plain or foo/channels/plain, so there's not much point in
>   making the distinction.

Do you mean we can put "uri:" aside?  Maybe I misunderstood..

The matrix previously was (I think.. when this series posted):

  [tcp, unix, file, exec, fd] x [uri, channels] x [precopy, postcopy]

Drop postcopy as doesn't seem to have any special paths:

  [tcp, unix, file, exec, fd] x [uri, channels]

So logically we should still cover these, right?

Thanks,

-- 
Peter Xu

Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()

[Aleksei Filippov]

On 09.04.2024 20:52, Daniel Henrique Barboza wrote:

raise_mmu_exception(), as is today, is prioritizing guest page faults by
checking first if virt_enabled && !first_stage, and then considering the
regular inst/load/store faults.

There's no mention in the spec about guest page fault being a higher
priority that PMP faults. In fact, privileged spec section 3.7.1 says:

"Attempting to fetch an instruction from a PMP region that does not have
execute permissions raises an instruction access-fault exception.
Attempting to execute a load or load-reserved instruction which accesses
a physical address within a PMP region without read permissions raises a
load access-fault exception. Attempting to execute a store,
store-conditional, or AMO instruction which accesses a physical address
within a PMP region without write permissions raises a store
access-fault exception."

So, in fact, we're doing it wrong - PMP faults should always be thrown,
regardless of also being a first or second stage fault.

The way riscv_cpu_tlb_fill() and get_physical_address() work is
adequate: a TRANSLATE_PMP_FAIL error is immediately reported and
reflected in the 'pmp_violation' flag. What we need is to change
raise_mmu_exception() to prioritize it.

Reported-by: Joseph Chan 
Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU translation 
stage")
Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/cpu_helper.c | 22 --
  1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index fc090d729a..e3a7797d00 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1176,28 +1176,30 @@ static void raise_mmu_exception(CPURISCVState *env, 
target_ulong address,

  switch (access_type) {
  case MMU_INST_FETCH:
-if (env->virt_enabled && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT;
+} else if (env->virt_enabled && !first_stage) {
  cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
  } else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_INST_PAGE_FAULT;
  }
  break;
  case MMU_DATA_LOAD:
-if (two_stage && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
+} else if (two_stage && !first_stage) {
  cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
  } else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_LOAD_PAGE_FAULT;
  }
  break;
  case MMU_DATA_STORE:
-if (two_stage && !first_stage) {
+if (pmp_violation) {
+cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+} else if (two_stage && !first_stage) {
  cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
  } else {
-cs->exception_index = pmp_violation ?
-RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
-RISCV_EXCP_STORE_PAGE_FAULT;
+cs->exception_index = RISCV_EXCP_STORE_PAGE_FAULT;
  }
  break;
  default:



Just tested your patch and found out that we still need to fix `if else` in
riscv_cpu_tlb_fill() after pmp check in 2 stage translation part, as I suggested
before, cz the problem with mtval2 will happened in case of successes 2 stage
translation but failed pmp check. In this case we gonna set
mtval2(env->guest_phys_fault_addr in context of riscv_cpu_tlb_fill()) as this
was a guest-page-fault, but it didn't and mtval2 should be zero, according to
RISCV privileged spec sect. 9.4.4: When a guest page-fault is taken into M-mode,
mtval2 is written with either zero or guest physical address that faulted,
shifted by 2 bits. *For other traps, mtval2 is set to zero...*
--
Sincerely,
Aleksei Filippov

Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling

[Peter Xu]

Yu,

On Thu, Apr 11, 2024 at 06:36:54PM +0200, Yu Zhang wrote:
> > 1) Either a CI test covering at least the major RDMA paths, or at least
> > periodically tests for each QEMU release will be needed.
> We use a batch of regression test cases for the stack, which covers the
> test for QEMU. I did such test for most of the QEMU releases planned as
> candidates for rollout.

The least I can think of is a few tests in one release.  Definitely too
less if one release can already break..

> 
> The migration test needs a pair of (either physical or virtual) servers with
> InfiniBand network, which makes it difficult to do on a single server. The
> nested VM could be a possible approach, for which we may need virtual
> InfiniBand network. Is SoftRoCE [1] a choice? I will try it and let you know.
> 
> [1]  https://enterprise-support.nvidia.com/s/article/howto-configure-soft-roce

Does it require a kernel driver?  The less host kernel / hardware /
.. dependencies the better.

I am wondering whether there can be a library doing everything in
userspace, translating RDMA into e.g. socket messages (so maybe ultimately
that's something like IP->rdma->IP.. just to cover the "rdma" procedures),
then that'll work for CI reliably.

Please also see my full list, though, especially entry 4).  Thanks already
for looking for solutions on the tests, but I don't want to waste your time
then found that tests are not enough even if ready.  I think we need people
that understand these stuff well enough, have dedicated time and look after
it.

Thanks,

-- 
Peter Xu

Re: [PATCH v4 02/10] hw/core: create Resettable QOM interface

[Edgar E. Iglesias]

On Fri, Apr 12, 2024 at 3:05 PM Peter Maydell  wrote:
>
> On Thu, 11 Apr 2024 at 18:23, Philippe Mathieu-Daudé  
> wrote:
> >
> > On 11/4/24 15:43, Peter Maydell wrote:
> > > On Wed, 21 Aug 2019 at 17:34, Damien Hedde  
> > > wrote:
> > >>
> > >> This commit defines an interface allowing multi-phase reset. This aims
> > >> to solve a problem of the actual single-phase reset (built in
> > >> DeviceClass and BusClass): reset behavior is dependent on the order
> > >> in which reset handlers are called. In particular doing external
> > >> side-effect (like setting an qemu_irq) is problematic because receiving
> > >> object may not be reset yet.
> > >
> > > So, I wanted to drag up this ancient patch to ask a couple
> > > of Resettable questions, because I'm working on adding a
> > > new ResetType (the equivalent of SHUTDOWN_CAUSE_SNAPSHOT_LOAD).
> > >
> > >> +/**
> > >> + * ResetType:
> > >> + * Types of reset.
> > >> + *
> > >> + * + Cold: reset resulting from a power cycle of the object.
> > >> + *
> > >> + * TODO: Support has to be added to handle more types. In particular,
> > >> + * ResetState structure needs to be expanded.
> > >> + */
> > >
> > > Does anybody remember what this TODO comment is about? What
> > > in particular would need to be in the ResetState struct
> > > to allow another type to be added?
> >
> > IIRC this comes from this discussion:
> > https://lore.kernel.org/qemu-devel/7c193b33-8188-2cda-cbf2-fb5452544...@greensocs.com/
> > Updated in this patch (see after '---' description):
> > https://lore.kernel.org/qemu-devel/20191018150630.31099-9-damien.he...@greensocs.com/
>
> Hmm, I can't see anything in there that mentions this
> TODO or what we'd need more ResetState fields to handle.
> I guess I'll go ahead with adding my new ResetType and ignore
> this TODO, because I can't see any reason why we need to
> do anything in particular for a new ResetType...
>
> > >
> > >> +typedef enum ResetType {
> > >> +RESET_TYPE_COLD,
> > >> +} ResetType;
> > >
> > >> +typedef void (*ResettableInitPhase)(Object *obj, ResetType type);
> > >> +typedef void (*ResettableHoldPhase)(Object *obj);
> > >> +typedef void (*ResettableExitPhase)(Object *obj);
> > >
> > > Was there a reason why we only pass the ResetType to the init
> > > phase method, and not also to the hold and exit phases ?
> > > Given that many devices don't need to implement init, it
> > > seems awkward to require them to do so just to stash the
> > > ResetType somewhere so they can look at it in the hold
> > > or exit phase, so I was thinking about adding the argument
> > > to the other two phase methods.
> >
> > You are right, the type should be propagated to to all phase
> > handlers.
>
> I have some patches which do this; I'll probably send them out
> in a series next week once I've figured out whether they fit
> better in with other patches that give the motivation.
>

Hi,

I don't remember the details on your first questions but I also agree
with adding the type to the other callbacks!

Cheers,
Edgar

[PATCH v2 05/13] .travis.yml: Update the jobs to Ubuntu 22.04

[Thomas Huth]

According to our support policy, we'll soon drop our official support
for Ubuntu 20.04 ("Focal Fossa") in QEMU. Thus we should update the
Travis jobs now to a newer release (Ubuntu 22.04 - "Jammy Jellyfish")
for future testing. Since all jobs are using this release now, we
can drop the entries from the individual jobs and use the global
setting again.

Signed-off-by: Thomas Huth 
---
 .travis.yml | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 8a3ae76a7c..56a2a01e14 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,5 +1,5 @@
 os: linux
-dist: focal
+dist: jammy
 language: c
 compiler:
   - gcc
@@ -7,7 +7,7 @@ cache:
   # There is one cache per branch and compiler version.
   # characteristics of each job are used to identify the cache:
   # - OS name (currently only linux)
-  # - OS distribution (for Linux, bionic or focal)
+  # - OS distribution (e.g. "jammy" for Linux)
   # - Names and values of visible environment variables set in .travis.yml or 
Settings panel
   timeout: 1200
   ccache: true
@@ -83,7 +83,6 @@ jobs:
 
 - name: "[aarch64] GCC check-tcg"
   arch: arm64
-  dist: focal
   addons:
 apt_packages:
   - libaio-dev
@@ -119,7 +118,6 @@ jobs:
 
 - name: "[ppc64] GCC check-tcg"
   arch: ppc64le
-  dist: focal
   addons:
 apt_packages:
   - libaio-dev
@@ -154,7 +152,6 @@ jobs:
 
 - name: "[s390x] GCC check-tcg"
   arch: s390x
-  dist: focal
   addons:
 apt_packages:
   - libaio-dev
@@ -199,7 +196,6 @@ jobs:
 
 - name: "[s390x] GCC (other-system)"
   arch: s390x
-  dist: focal
   addons:
 apt_packages:
   - libaio-dev
@@ -229,7 +225,6 @@ jobs:
 
 - name: "[s390x] GCC (user)"
   arch: s390x
-  dist: focal
   addons:
 apt_packages:
   - libgcrypt20-dev
@@ -244,8 +239,7 @@ jobs:
 
 - name: "[s390x] Clang (disable-tcg)"
   arch: s390x
-  dist: focal
-  compiler: clang-10
+  compiler: clang
   addons:
 apt_packages:
   - libaio-dev
@@ -271,7 +265,6 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
-  - clang-10
   env:
 - TEST_CMD="make check-unit"
 - CONFIG="--disable-containers --disable-tcg --enable-kvm 
--disable-tools
-- 
2.44.0

[PATCH v2 11/13] block/nfs: Use URI parsing code from glib

[Thomas Huth]

Since version 2.66, glib has useful URI parsing functions, too.
Use those instead of the QEMU-internal ones to be finally able
to get rid of the latter.

Signed-off-by: Thomas Huth 
---
 block/nfs.c | 110 ++--
 1 file changed, 54 insertions(+), 56 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index f737e19cd3..07f50b7402 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -38,7 +38,6 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
-#include "qemu/uri.h"
 #include "qemu/cutils.h"
 #include "sysemu/replay.h"
 #include "qapi/qapi-visit-block-core.h"
@@ -79,77 +78,76 @@ typedef struct NFSRPC {
 
 static int nfs_parse_uri(const char *filename, QDict *options, Error **errp)
 {
-URI *uri = NULL;
-QueryParams *qp = NULL;
-int ret = -EINVAL, i;
+g_autoptr(GUri) uri = g_uri_parse(filename, G_URI_FLAGS_NONE, NULL);
+GUriParamsIter qp;
+const char *uri_server, *uri_path, *uri_query;
+char *qp_name, *qp_value;
+GError *gerror = NULL;
 
-uri = uri_parse(filename);
 if (!uri) {
 error_setg(errp, "Invalid URI specified");
-goto out;
+return -EINVAL;
 }
-if (g_strcmp0(uri->scheme, "nfs") != 0) {
+if (!g_str_equal(g_uri_get_scheme(uri), "nfs")) {
 error_setg(errp, "URI scheme must be 'nfs'");
-goto out;
+return -EINVAL;
 }
 
-if (!uri->server) {
+uri_server = g_uri_get_host(uri);
+if (!uri_server || !uri_server[0]) {
 error_setg(errp, "missing hostname in URI");
-goto out;
+return -EINVAL;
 }
 
-if (!uri->path) {
+uri_path = g_uri_get_path(uri);
+if (!uri_path || !uri_path[0]) {
 error_setg(errp, "missing file path in URI");
-goto out;
-}
-
-qp = query_params_parse(uri->query);
-if (!qp) {
-error_setg(errp, "could not parse query parameters");
-goto out;
+return -EINVAL;
 }
 
-qdict_put_str(options, "server.host", uri->server);
+qdict_put_str(options, "server.host", uri_server);
 qdict_put_str(options, "server.type", "inet");
-qdict_put_str(options, "path", uri->path);
-
-for (i = 0; i < qp->n; i++) {
-uint64_t val;
-if (!qp->p[i].value) {
-error_setg(errp, "Value for NFS parameter expected: %s",
-   qp->p[i].name);
-goto out;
-}
-if (parse_uint_full(qp->p[i].value, 0, &val)) {
-error_setg(errp, "Illegal value for NFS parameter: %s",
-   qp->p[i].name);
-goto out;
-}
-if (!strcmp(qp->p[i].name, "uid")) {
-qdict_put_str(options, "user", qp->p[i].value);
-} else if (!strcmp(qp->p[i].name, "gid")) {
-qdict_put_str(options, "group", qp->p[i].value);
-} else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
-qdict_put_str(options, "tcp-syn-count", qp->p[i].value);
-} else if (!strcmp(qp->p[i].name, "readahead")) {
-qdict_put_str(options, "readahead-size", qp->p[i].value);
-} else if (!strcmp(qp->p[i].name, "pagecache")) {
-qdict_put_str(options, "page-cache-size", qp->p[i].value);
-} else if (!strcmp(qp->p[i].name, "debug")) {
-qdict_put_str(options, "debug", qp->p[i].value);
-} else {
-error_setg(errp, "Unknown NFS parameter name: %s",
-   qp->p[i].name);
-goto out;
+qdict_put_str(options, "path", uri_path);
+
+uri_query = g_uri_get_query(uri);
+if (uri_query) {
+g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE);
+while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) {
+uint64_t val;
+if (!qp_name || gerror) {
+error_setg(errp, "Failed to parse NFS parameter");
+return -EINVAL;
+}
+if (!qp_value) {
+error_setg(errp, "Value for NFS parameter expected: %s",
+   qp_name);
+return -EINVAL;
+}
+if (parse_uint_full(qp_value, 0, &val)) {
+error_setg(errp, "Illegal value for NFS parameter: %s",
+   qp_name);
+return -EINVAL;
+}
+if (g_str_equal(qp_name, "uid")) {
+qdict_put_str(options, "user", qp_value);
+} else if (g_str_equal(qp_name, "gid")) {
+qdict_put_str(options, "group", qp_value);
+} else if (g_str_equal(qp_name, "tcp-syncnt")) {
+qdict_put_str(options, "tcp-syn-count", qp_value);
+} else if (g_str_equal(qp_name, "readahead")) {
+qdict_put_str(options, "readahead-size", qp_value);
+} else if (g_str_equal(qp_name, "pagecache")) {
+qdict_put_str(options, "page-cache-size", qp_value);
+} else if (g_st

[PATCH v2 12/13] block/ssh: Use URI parsing code from glib

[Thomas Huth]

Since version 2.66, glib has useful URI parsing functions, too.
Use those instead of the QEMU-internal ones to be finally able
to get rid of the latter.

Reviewed-by: Richard W.M. Jones 
Signed-off-by: Thomas Huth 
---
 block/ssh.c | 75 -
 1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 2748253d4a..18ae565e55 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -37,7 +37,6 @@
 #include "qemu/ctype.h"
 #include "qemu/cutils.h"
 #include "qemu/sockets.h"
-#include "qemu/uri.h"
 #include "qapi/qapi-visit-sockets.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "qapi/qmp/qdict.h"
@@ -181,65 +180,71 @@ static void sftp_error_trace(BDRVSSHState *s, const char 
*op)
 
 static int parse_uri(const char *filename, QDict *options, Error **errp)
 {
-URI *uri = NULL;
-QueryParams *qp;
+g_autoptr(GUri) uri = g_uri_parse(filename, G_URI_FLAGS_NONE, NULL);
+const char *uri_host, *uri_path, *uri_user, *uri_query;
 char *port_str;
-int i;
+int port;
+g_autoptr(GError) gerror = NULL;
+char *qp_name, *qp_value;
+GUriParamsIter qp;
 
-uri = uri_parse(filename);
 if (!uri) {
 return -EINVAL;
 }
 
-if (g_strcmp0(uri->scheme, "ssh") != 0) {
+if (g_strcmp0(g_uri_get_scheme(uri), "ssh") != 0) {
 error_setg(errp, "URI scheme must be 'ssh'");
-goto err;
+return -EINVAL;
 }
 
-if (!uri->server || strcmp(uri->server, "") == 0) {
+uri_host = g_uri_get_host(uri);
+if (!uri_host || g_str_equal(uri_host, "")) {
 error_setg(errp, "missing hostname in URI");
-goto err;
+return -EINVAL;
 }
 
-if (!uri->path || strcmp(uri->path, "") == 0) {
+uri_path = g_uri_get_path(uri);
+if (!uri_path || g_str_equal(uri_path, "")) {
 error_setg(errp, "missing remote path in URI");
-goto err;
-}
-
-qp = query_params_parse(uri->query);
-if (!qp) {
-error_setg(errp, "could not parse query parameters");
-goto err;
+return -EINVAL;
 }
 
-if(uri->user && strcmp(uri->user, "") != 0) {
-qdict_put_str(options, "user", uri->user);
+uri_user = g_uri_get_user(uri);
+if (uri_user && !g_str_equal(uri_user, "")) {
+qdict_put_str(options, "user", uri_user);
 }
 
-qdict_put_str(options, "server.host", uri->server);
+qdict_put_str(options, "server.host", uri_host);
 
-port_str = g_strdup_printf("%d", uri->port ?: 22);
+port = g_uri_get_port(uri);
+port_str = g_strdup_printf("%d", port > 0 ? port : 22);
 qdict_put_str(options, "server.port", port_str);
 g_free(port_str);
 
-qdict_put_str(options, "path", uri->path);
-
-/* Pick out any query parameters that we understand, and ignore
- * the rest.
- */
-for (i = 0; i < qp->n; ++i) {
-if (strcmp(qp->p[i].name, "host_key_check") == 0) {
-qdict_put_str(options, "host_key_check", qp->p[i].value);
+qdict_put_str(options, "path", uri_path);
+
+uri_query = g_uri_get_query(uri);
+if (uri_query) {
+g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE);
+while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) {
+if (!qp_name || !qp_value || gerror) {
+warn_report("Failed to parse SSH URI parameters '%s'.",
+uri_query);
+break;
+}
+/*
+ * Pick out the query parameters that we understand, and ignore
+ * (or rather warn about) the rest.
+ */
+if (g_str_equal(qp_name, "host_key_check")) {
+qdict_put_str(options, "host_key_check", qp_value);
+} else {
+warn_report("Unsupported parameter '%s' in URI.", qp_name);
+}
 }
 }
 
-query_params_free(qp);
-uri_free(uri);
 return 0;
-
- err:
-uri_free(uri);
-return -EINVAL;
 }
 
 static bool ssh_has_filename_options_conflict(QDict *options, Error **errp)
-- 
2.44.0

[PATCH v2 08/13] Remove glib compatibility code that is not required anymore

[Thomas Huth]

Now that we bumped the minumum glib version to 2.66, we can drop
the old code.

Suggested-by: Paolo Bonzini 
Signed-off-by: Thomas Huth 
---
 qga/commands-posix-ssh.c |  8 
 util/error-report.c  | 10 --
 2 files changed, 18 deletions(-)

diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index b0e0b1d674..cc1f5a708e 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -288,7 +288,6 @@ qmp_guest_ssh_get_authorized_keys(const char *username, 
Error **errp)
 }
 
 #ifdef QGA_BUILD_UNIT_TEST
-#if GLIB_CHECK_VERSION(2, 60, 0)
 static const strList test_key2 = {
 .value = (char *)"algo key2 comments"
 };
@@ -484,11 +483,4 @@ int main(int argc, char *argv[])
 
 return g_test_run();
 }
-#else
-int main(int argc, char *argv[])
-{
-g_test_message("test skipped, needs glib >= 2.60");
-return 0;
-}
-#endif /* GLIB_2_60 */
 #endif /* BUILD_UNIT_TEST */
diff --git a/util/error-report.c b/util/error-report.c
index 6e44a55732..1b17c11de1 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -172,18 +172,8 @@ static void print_loc(void)
 static char *
 real_time_iso8601(void)
 {
-#if GLIB_CHECK_VERSION(2,62,0)
 g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
-/* ignore deprecation warning, since GLIB_VERSION_MAX_ALLOWED is 2.56 */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 return g_date_time_format_iso8601(dt);
-#pragma GCC diagnostic pop
-#else
-GTimeVal tv;
-g_get_current_time(&tv);
-return g_time_val_to_iso8601(&tv);
-#endif
 }
 
 /*
-- 
2.44.0

[PATCH v2 03/13] tests/docker/dockerfiles: Run lcitool-refresh after the lcitool update

[Thomas Huth]

This update adds the removing of the EXTERNALLY-MANAGED marker files
that has been added to the lcitool recently.

Signed-off-by: Thomas Huth 
---
 tests/docker/dockerfiles/alpine.docker| 3 ++-
 tests/docker/dockerfiles/centos8.docker   | 1 +
 tests/docker/dockerfiles/debian-amd64-cross.docker| 3 ++-
 tests/docker/dockerfiles/debian-arm64-cross.docker| 3 ++-
 tests/docker/dockerfiles/debian-armel-cross.docker| 3 ++-
 tests/docker/dockerfiles/debian-armhf-cross.docker| 3 ++-
 tests/docker/dockerfiles/debian-i686-cross.docker | 3 ++-
 tests/docker/dockerfiles/debian-mips64el-cross.docker | 3 ++-
 tests/docker/dockerfiles/debian-mipsel-cross.docker   | 3 ++-
 tests/docker/dockerfiles/debian-ppc64el-cross.docker  | 3 ++-
 tests/docker/dockerfiles/debian-riscv64-cross.docker  | 3 ++-
 tests/docker/dockerfiles/debian-s390x-cross.docker| 3 ++-
 tests/docker/dockerfiles/debian.docker| 1 +
 tests/docker/dockerfiles/fedora-win64-cross.docker| 3 ++-
 tests/docker/dockerfiles/fedora.docker| 1 +
 tests/docker/dockerfiles/opensuse-leap.docker | 1 +
 tests/docker/dockerfiles/ubuntu2204.docker| 1 +
 17 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/tests/docker/dockerfiles/alpine.docker 
b/tests/docker/dockerfiles/alpine.docker
index 42f6928627..cd9d7af1ce 100644
--- a/tests/docker/dockerfiles/alpine.docker
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -116,7 +116,8 @@ RUN apk update && \
 zlib-static \
 zstd \
 zstd-dev && \
-apk list | sort > /packages.txt && \
+rm -f /usr/lib*/python3*/EXTERNALLY-MANAGED && \
+apk list --installed | sort > /packages.txt && \
 mkdir -p /usr/libexec/ccache-wrappers && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/c++ && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \
diff --git a/tests/docker/dockerfiles/centos8.docker 
b/tests/docker/dockerfiles/centos8.docker
index d97c30e96a..ea618bf352 100644
--- a/tests/docker/dockerfiles/centos8.docker
+++ b/tests/docker/dockerfiles/centos8.docker
@@ -123,6 +123,7 @@ RUN dnf distro-sync -y && \
 zstd && \
 dnf autoremove -y && \
 dnf clean all -y && \
+rm -f /usr/lib*/python3*/EXTERNALLY-MANAGED && \
 rpm -qa | sort > /packages.txt && \
 mkdir -p /usr/libexec/ccache-wrappers && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/c++ && \
diff --git a/tests/docker/dockerfiles/debian-amd64-cross.docker 
b/tests/docker/dockerfiles/debian-amd64-cross.docker
index 00bdc06021..d0b0e9778e 100644
--- a/tests/docker/dockerfiles/debian-amd64-cross.docker
+++ b/tests/docker/dockerfiles/debian-amd64-cross.docker
@@ -64,7 +64,8 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 eatmydata apt-get autoremove -y && \
 eatmydata apt-get autoclean -y && \
 sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \
-dpkg-reconfigure locales
+dpkg-reconfigure locales && \
+rm -f /usr/lib*/python3*/EXTERNALLY-MANAGED
 
 ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers"
 ENV LANG "en_US.UTF-8"
diff --git a/tests/docker/dockerfiles/debian-arm64-cross.docker 
b/tests/docker/dockerfiles/debian-arm64-cross.docker
index 2dae3777f7..8cb225740e 100644
--- a/tests/docker/dockerfiles/debian-arm64-cross.docker
+++ b/tests/docker/dockerfiles/debian-arm64-cross.docker
@@ -64,7 +64,8 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 eatmydata apt-get autoremove -y && \
 eatmydata apt-get autoclean -y && \
 sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \
-dpkg-reconfigure locales
+dpkg-reconfigure locales && \
+rm -f /usr/lib*/python3*/EXTERNALLY-MANAGED
 
 ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers"
 ENV LANG "en_US.UTF-8"
diff --git a/tests/docker/dockerfiles/debian-armel-cross.docker 
b/tests/docker/dockerfiles/debian-armel-cross.docker
index 75342c09b0..e6f37418ed 100644
--- a/tests/docker/dockerfiles/debian-armel-cross.docker
+++ b/tests/docker/dockerfiles/debian-armel-cross.docker
@@ -65,7 +65,8 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 eatmydata apt-get autoremove -y && \
 eatmydata apt-get autoclean -y && \
 sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \
-dpkg-reconfigure locales
+dpkg-reconfigure locales && \
+rm -f /usr/lib*/python3*/EXTERNALLY-MANAGED
 
 RUN /usr/bin/pip3 install tomli
 
diff --git a/tests/docker/dockerfiles/debian-armhf-cross.docker 
b/tests/docker/dockerfiles/debian-armhf-cross.docker
index 180ed836e6..407a014f57 100644
--- a/tests/docker/dockerfiles/debian-armhf-cross.docker
+++ b/tests/docker/dockerfiles/debian-armhf-cross.docker
@@ -64,7 +64,8 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 eatmydata apt-get autoremove -y && \
 eatmydata apt-get autoclean -y && \
 sed -Ei 's,^# (en_US\.UTF-8 .*)$,\1,' /etc/locale.gen && \
-dpkg-reconfigure locales
+dpkg-reconfigure locales && \
+rm 

[PATCH v2 10/13] block/nbd: Use URI parsing code from glib

[Thomas Huth]

Since version 2.66, glib has useful URI parsing functions, too.
Use those instead of the QEMU-internal ones to be finally able
to get rid of the latter. The g_uri_get_host() also takes care
of removing the square brackets from IPv6 addresses, so we can
drop that part of the QEMU code now, too.

Reviewed-by: Richard W.M. Jones 
Signed-off-by: Thomas Huth 
---
 block/nbd.c | 76 ++---
 1 file changed, 37 insertions(+), 39 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index ef05f7cdfd..589d28af83 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -31,7 +31,6 @@
 #include "qemu/osdep.h"
 
 #include "trace.h"
-#include "qemu/uri.h"
 #include "qemu/option.h"
 #include "qemu/cutils.h"
 #include "qemu/main-loop.h"
@@ -1514,30 +1513,31 @@ static void nbd_client_close(BlockDriverState *bs)
 
 static int nbd_parse_uri(const char *filename, QDict *options)
 {
-URI *uri;
+g_autoptr(GUri) uri = g_uri_parse(filename, G_URI_FLAGS_NONE, NULL);
+g_autoptr(GHashTable) qp = NULL;
 const char *p;
-QueryParams *qp = NULL;
-int ret = 0;
+int qp_n;
 bool is_unix;
+const char *uri_scheme, *uri_query, *uri_server;
+int uri_port;
 
-uri = uri_parse(filename);
 if (!uri) {
 return -EINVAL;
 }
 
 /* transport */
-if (!g_strcmp0(uri->scheme, "nbd")) {
+uri_scheme = g_uri_get_scheme(uri);
+if (!g_strcmp0(uri_scheme, "nbd")) {
 is_unix = false;
-} else if (!g_strcmp0(uri->scheme, "nbd+tcp")) {
+} else if (!g_strcmp0(uri_scheme, "nbd+tcp")) {
 is_unix = false;
-} else if (!g_strcmp0(uri->scheme, "nbd+unix")) {
+} else if (!g_strcmp0(uri_scheme, "nbd+unix")) {
 is_unix = true;
 } else {
-ret = -EINVAL;
-goto out;
+return -EINVAL;
 }
 
-p = uri->path ? uri->path : "";
+p = g_uri_get_path(uri) ?: "";
 if (p[0] == '/') {
 p++;
 }
@@ -1545,52 +1545,50 @@ static int nbd_parse_uri(const char *filename, QDict 
*options)
 qdict_put_str(options, "export", p);
 }
 
-qp = query_params_parse(uri->query);
-if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) {
-ret = -EINVAL;
-goto out;
+uri_query = g_uri_get_query(uri);
+if (uri_query) {
+qp = g_uri_parse_params(uri_query, -1, "&", G_URI_PARAMS_NONE, NULL);
+if (!qp) {
+return -EINVAL;
+}
+qp_n = g_hash_table_size(qp);
+if (qp_n > 1 || (is_unix && !qp_n) || (!is_unix && qp_n)) {
+return -EINVAL;
+}
+ }
+
+uri_server = g_uri_get_host(uri);
+if (uri_server && !uri_server[0]) {
+uri_server = NULL;
 }
+uri_port = g_uri_get_port(uri);
 
 if (is_unix) {
 /* nbd+unix:///export?socket=path */
-if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
-ret = -EINVAL;
-goto out;
+const char *uri_socket = g_hash_table_lookup(qp, "socket");
+if (uri_server || uri_port != -1 || !uri_socket) {
+return -EINVAL;
 }
 qdict_put_str(options, "server.type", "unix");
-qdict_put_str(options, "server.path", qp->p[0].value);
+qdict_put_str(options, "server.path", uri_socket);
 } else {
-QString *host;
 char *port_str;
 
 /* nbd[+tcp]://host[:port]/export */
-if (!uri->server) {
-ret = -EINVAL;
-goto out;
-}
-
-/* strip braces from literal IPv6 address */
-if (uri->server[0] == '[') {
-host = qstring_from_substr(uri->server, 1,
-   strlen(uri->server) - 1);
-} else {
-host = qstring_from_str(uri->server);
+if (!uri_server) {
+return -EINVAL;
 }
 
 qdict_put_str(options, "server.type", "inet");
-qdict_put(options, "server.host", host);
+qdict_put_str(options, "server.host", uri_server);
 
-port_str = g_strdup_printf("%d", uri->port ?: NBD_DEFAULT_PORT);
+port_str = g_strdup_printf("%d", uri_port > 0 ? uri_port
+  : NBD_DEFAULT_PORT);
 qdict_put_str(options, "server.port", port_str);
 g_free(port_str);
 }
 
-out:
-if (qp) {
-query_params_free(qp);
-}
-uri_free(uri);
-return ret;
+return 0;
 }
 
 static bool nbd_has_filename_options_conflict(QDict *options, Error **errp)
-- 
2.44.0

[PATCH v2 09/13] block/gluster: Use URI parsing code from glib

[Thomas Huth]

Since version 2.66, glib has useful URI parsing functions, too.
Use those instead of the QEMU-internal ones to be finally able
to get rid of the latter.

Signed-off-by: Thomas Huth 
---
 block/gluster.c | 71 -
 1 file changed, 35 insertions(+), 36 deletions(-)

diff --git a/block/gluster.c b/block/gluster.c
index cc74af06dc..1c9505f8bb 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -17,7 +17,6 @@
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
-#include "qemu/uri.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qemu/option.h"
@@ -289,9 +288,9 @@ static void glfs_clear_preopened(glfs_t *fs)
 }
 }
 
-static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path)
+static int parse_volume_options(BlockdevOptionsGluster *gconf, const char 
*path)
 {
-char *p, *q;
+const char *p, *q;
 
 if (!path) {
 return -EINVAL;
@@ -349,13 +348,13 @@ static int parse_volume_options(BlockdevOptionsGluster 
*gconf, char *path)
 static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
   const char *filename)
 {
+g_autoptr(GUri) uri = g_uri_parse(filename, G_URI_FLAGS_NONE, NULL);
+g_autoptr(GHashTable) qp = NULL;
 SocketAddress *gsconf;
-URI *uri;
-QueryParams *qp = NULL;
 bool is_unix = false;
-int ret = 0;
+const char *uri_scheme, *uri_query, *uri_server;
+int uri_port, ret;
 
-uri = uri_parse(filename);
 if (!uri) {
 return -EINVAL;
 }
@@ -364,57 +363,57 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster 
*gconf,
 QAPI_LIST_PREPEND(gconf->server, gsconf);
 
 /* transport */
-if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
+uri_scheme = g_uri_get_scheme(uri);
+if (!uri_scheme || !strcmp(uri_scheme, "gluster")) {
 gsconf->type = SOCKET_ADDRESS_TYPE_INET;
-} else if (!strcmp(uri->scheme, "gluster+tcp")) {
+} else if (!strcmp(uri_scheme, "gluster+tcp")) {
 gsconf->type = SOCKET_ADDRESS_TYPE_INET;
-} else if (!strcmp(uri->scheme, "gluster+unix")) {
+} else if (!strcmp(uri_scheme, "gluster+unix")) {
 gsconf->type = SOCKET_ADDRESS_TYPE_UNIX;
 is_unix = true;
-} else if (!strcmp(uri->scheme, "gluster+rdma")) {
+} else if (!strcmp(uri_scheme, "gluster+rdma")) {
 gsconf->type = SOCKET_ADDRESS_TYPE_INET;
 warn_report("rdma feature is not supported, falling back to tcp");
 } else {
-ret = -EINVAL;
-goto out;
+return -EINVAL;
 }
 
-ret = parse_volume_options(gconf, uri->path);
+ret = parse_volume_options(gconf, g_uri_get_path(uri));
 if (ret < 0) {
-goto out;
+return ret;
 }
 
-qp = query_params_parse(uri->query);
-if (qp->n > 1 || (is_unix && !qp->n) || (!is_unix && qp->n)) {
-ret = -EINVAL;
-goto out;
+uri_query = g_uri_get_query(uri);
+if (uri_query) {
+qp = g_uri_parse_params(uri_query, -1, "&", G_URI_PARAMS_NONE, NULL);
+if (!qp) {
+return -EINVAL;
+}
+ret = g_hash_table_size(qp);
+if (ret > 1 || (is_unix && !ret) || (!is_unix && ret)) {
+return -EINVAL;
+}
 }
 
+uri_server = g_uri_get_host(uri);
+uri_port = g_uri_get_port(uri);
+
 if (is_unix) {
-if (uri->server || uri->port) {
-ret = -EINVAL;
-goto out;
-}
-if (strcmp(qp->p[0].name, "socket")) {
-ret = -EINVAL;
-goto out;
+char *uri_socket = g_hash_table_lookup(qp, "socket");
+if (uri_server || uri_port != -1 || !uri_socket) {
+return -EINVAL;
 }
-gsconf->u.q_unix.path = g_strdup(qp->p[0].value);
+gsconf->u.q_unix.path = g_strdup(uri_socket);
 } else {
-gsconf->u.inet.host = g_strdup(uri->server ? uri->server : 
"localhost");
-if (uri->port) {
-gsconf->u.inet.port = g_strdup_printf("%d", uri->port);
+gsconf->u.inet.host = g_strdup(uri_server ? uri_server : "localhost");
+if (uri_port > 0) {
+gsconf->u.inet.port = g_strdup_printf("%d", uri_port);
 } else {
 gsconf->u.inet.port = g_strdup_printf("%d", GLUSTER_DEFAULT_PORT);
 }
 }
 
-out:
-if (qp) {
-query_params_free(qp);
-}
-uri_free(uri);
-return ret;
+return 0;
 }
 
 static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
-- 
2.44.0

[PATCH v2 00/13] Drop old distros, bump glib and switch to glib URI parsing code

[Thomas Huth]

In the QEMU 9.1 development cycle, we can drop the support for
Ubuntu 20.04 and CentOS 8 since the following major versions of
these distributions are available since 2 years already.

This allows us to bump the minimum version of glib to 2.66 which
comes with a nice set of URI parsing functions. By switching to
these parsing functions, we can finally drop our own URI parsing
code in util/uri.c.

NB: We also need to update some of the custom runners in our CI
environment first (since they still use Ubuntu 20.04).

v2:
- Added Paolo's patch to bump the external CI runners
- Added patch to drop more glib compatibility hunks
- Use g_autoptr() in the URI patches for simplification
- Don't allow port 0 in the URIs

Paolo Bonzini (1):
  ci: move external build environment setups to CentOS Stream 9

Thomas Huth (12):
  tests: Remove Ubuntu 20.04 container
  tests/lcitool/libvirt-ci: Update to the latest master branch
  tests/docker/dockerfiles: Run lcitool-refresh after the lcitool update
  tests: Update our CI to use CentOS Stream 9 instead of 8
  .travis.yml: Update the jobs to Ubuntu 22.04
  Bump minimum glib version to v2.66
  Remove glib compatibility code that is not required anymore
  block/gluster: Use URI parsing code from glib
  block/nbd: Use URI parsing code from glib
  block/nfs: Use URI parsing code from glib
  block/ssh: Use URI parsing code from glib
  util/uri: Remove the old URI parsing code

 meson.build   |   16 +-
 include/glib-compat.h |   27 +-
 include/qemu/uri.h|   99 --
 block/gluster.c   |   71 +-
 block/nbd.c   |   76 +-
 block/nfs.c   |  110 +-
 block/ssh.c   |   75 +-
 qga/commands-posix-ssh.c  |   12 +-
 util/error-report.c   |   10 -
 util/uri.c| 1466 -
 .gitlab-ci.d/buildtest.yml|   16 +-
 .gitlab-ci.d/container-core.yml   |4 +-
 .travis.yml   |   13 +-
 .../stream/{8 => 9}/build-environment.yml |   31 +-
 .../stream/{8 => 9}/x86_64/configure  |4 +-
 .../stream/{8 => 9}/x86_64/test-avocado   |0
 scripts/ci/setup/build-environment.yml|   44 +-
 tests/docker/dockerfiles/alpine.docker|3 +-
 .../{centos8.docker => centos9.docker}|   35 +-
 .../dockerfiles/debian-amd64-cross.docker |3 +-
 .../dockerfiles/debian-arm64-cross.docker |3 +-
 .../dockerfiles/debian-armel-cross.docker |3 +-
 .../dockerfiles/debian-armhf-cross.docker |3 +-
 .../dockerfiles/debian-i686-cross.docker  |3 +-
 .../dockerfiles/debian-mips64el-cross.docker  |3 +-
 .../dockerfiles/debian-mipsel-cross.docker|3 +-
 .../dockerfiles/debian-ppc64el-cross.docker   |3 +-
 .../dockerfiles/debian-riscv64-cross.docker   |3 +-
 .../dockerfiles/debian-s390x-cross.docker |3 +-
 tests/docker/dockerfiles/debian.docker|1 +
 .../dockerfiles/fedora-win64-cross.docker |3 +-
 tests/docker/dockerfiles/fedora.docker|1 +
 tests/docker/dockerfiles/opensuse-leap.docker |1 +
 tests/docker/dockerfiles/ubuntu2004.docker|  157 --
 tests/docker/dockerfiles/ubuntu2204.docker|1 +
 tests/lcitool/libvirt-ci  |2 +-
 tests/lcitool/mappings.yml|   20 -
 tests/lcitool/refresh |3 +-
 tests/vm/centos   |4 +-
 util/meson.build  |2 +-
 40 files changed, 265 insertions(+), 2072 deletions(-)
 delete mode 100644 include/qemu/uri.h
 delete mode 100644 util/uri.c
 rename scripts/ci/org.centos/stream/{8 => 9}/build-environment.yml (75%)
 rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/configure (98%)
 rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/test-avocado (100%)
 rename tests/docker/dockerfiles/{centos8.docker => centos9.docker} (82%)
 delete mode 100644 tests/docker/dockerfiles/ubuntu2004.docker

-- 
2.44.0

[PATCH v2 07/13] Bump minimum glib version to v2.66

[Thomas Huth]

Now that we dropped support for CentOS 8 and Ubuntu 20.04, we can
look into bumping the glib version to a new minimum for further
clean-ups. According to repology.org, available versions are:

 CentOS Stream 9:   2.66.7
 Debian 11: 2.66.8
 Fedora 38: 2.74.1
 Freebsd:   2.78.4
 Homebrew:  2.80.0
 Openbsd:   2.78.4
 OpenSuse leap 15.5:2.70.5
 pkgsrc_current:2.78.4
 Ubuntu 22.04:  2.72.1

Thus it should be safe to bump the minimum glib version to 2.66 now.
Version 2.66 comes with new functions for URI parsing which will
allow further clean-ups in the following patches.

Signed-off-by: Thomas Huth 
---
 meson.build  | 16 +---
 include/glib-compat.h| 27 ++-
 qga/commands-posix-ssh.c |  4 ++--
 3 files changed, 5 insertions(+), 42 deletions(-)

diff --git a/meson.build b/meson.build
index c9c3217ba4..c0aaceffc0 100644
--- a/meson.build
+++ b/meson.build
@@ -865,7 +865,7 @@ have_xen_pci_passthrough = 
get_option('xen_pci_passthrough') \
 
 # When bumping glib minimum version, please check also whether to increase
 # the _WIN32_WINNT setting in osdep.h according to the value from glib
-glib_req_ver = '>=2.56.0'
+glib_req_ver = '>=2.66.0'
 glib_pc = dependency('glib-2.0', version: glib_req_ver, required: true,
 method: 'pkg-config')
 glib_cflags = []
@@ -906,20 +906,6 @@ if not cc.compiles('''
 to the right pkg-config files for your build target.''')
 endif
 
-# Silence clang warnings triggered by glib < 2.57.2
-if not cc.compiles('''
-  #include 
-  typedef struct Foo {
-int i;
-  } Foo;
-  static void foo_free(Foo *f)
-  {
-g_free(f);
-  }
-  G_DEFINE_AUTOPTR_CLEANUP_FUNC(Foo, foo_free)
-  int main(void) { return 0; }''', dependencies: glib_pc, args: 
['-Wunused-function', '-Werror'])
-  glib_cflags += cc.get_supported_arguments('-Wno-unused-function')
-endif
 glib = declare_dependency(dependencies: [glib_pc, gmodule],
   compile_args: glib_cflags,
   version: glib_pc.version())
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 43a562974d..86be439ba0 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -19,12 +19,12 @@
 /* Ask for warnings for anything that was marked deprecated in
  * the defined version, or before. It is a candidate for rewrite.
  */
-#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_56
+#define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_66
 
 /* Ask for warnings if code tries to use function that did not
  * exist in the defined version. These risk breaking builds
  */
-#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_56
+#define GLIB_VERSION_MAX_ALLOWED GLIB_VERSION_2_66
 
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
@@ -105,29 +105,6 @@ static inline gpointer g_memdup2_qemu(gconstpointer mem, 
gsize byte_size)
 }
 #define g_memdup2(m, s) g_memdup2_qemu(m, s)
 
-#if defined(G_OS_UNIX)
-/*
- * Note: The fallback implementation is not MT-safe, and it returns a copy of
- * the libc passwd (must be g_free() after use) but not the content. Because of
- * these important differences the caller must be aware of, it's not #define 
for
- * GLib API substitution.
- */
-static inline struct passwd *
-g_unix_get_passwd_entry_qemu(const gchar *user_name, GError **error)
-{
-#if GLIB_CHECK_VERSION(2, 64, 0)
-return g_unix_get_passwd_entry(user_name, error);
-#else
-struct passwd *p = getpwnam(user_name);
-if (!p) {
-g_set_error_literal(error, G_UNIX_ERROR, 0, g_strerror(errno));
-return NULL;
-}
-return (struct passwd *)g_memdup(p, sizeof(*p));
-#endif
-}
-#endif /* G_OS_UNIX */
-
 static inline bool
 qemu_g_test_slow(void)
 {
diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index 236f80de44..b0e0b1d674 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -35,7 +35,7 @@ test_get_passwd_entry(const gchar *user_name, GError **error)
 return p;
 }
 
-#define g_unix_get_passwd_entry_qemu(username, err) \
+#define g_unix_get_passwd_entry(username, err) \
test_get_passwd_entry(username, err)
 #endif
 
@@ -45,7 +45,7 @@ get_passwd_entry(const char *username, Error **errp)
 g_autoptr(GError) err = NULL;
 struct passwd *p;
 
-p = g_unix_get_passwd_entry_qemu(username, &err);
+p = g_unix_get_passwd_entry(username, &err);
 if (p == NULL) {
 error_setg(errp, "failed to lookup user '%s': %s",
username, err->message);
-- 
2.44.0

[PATCH v2 04/13] tests: Update our CI to use CentOS Stream 9 instead of 8

[Thomas Huth]

RHEL 9 (and thus also the derivatives) are available since two years
now, so according to QEMU's support policy, we can drop the active
support for the previous major version 8 now.
Thus upgrade our CentOS Stream container to major version 9 now.

Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/buildtest.yml| 16 -
 .gitlab-ci.d/container-core.yml   |  4 +--
 .../{centos8.docker => centos9.docker}| 34 +++
 tests/lcitool/mappings.yml| 20 ---
 tests/lcitool/refresh |  2 +-
 tests/vm/centos   |  4 +--
 6 files changed, 26 insertions(+), 54 deletions(-)
 rename tests/docker/dockerfiles/{centos8.docker => centos9.docker} (82%)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index cfdff175c3..9f34c650d6 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -158,9 +158,9 @@ build-system-centos:
 - .native_build_job_template
 - .native_build_artifact_template
   needs:
-job: amd64-centos8-container
+job: amd64-centos9-container
   variables:
-IMAGE: centos8
+IMAGE: centos9
 CONFIGURE_ARGS: --disable-nettle --enable-gcrypt --enable-vfio-user-server
   --enable-modules --enable-trace-backends=dtrace --enable-docs
 TARGETS: ppc64-softmmu or1k-softmmu s390x-softmmu
@@ -242,7 +242,7 @@ check-system-centos:
 - job: build-system-centos
   artifacts: true
   variables:
-IMAGE: centos8
+IMAGE: centos9
 MAKE_CHECK_ARGS: check
 
 avocado-system-centos:
@@ -251,7 +251,7 @@ avocado-system-centos:
 - job: build-system-centos
   artifacts: true
   variables:
-IMAGE: centos8
+IMAGE: centos9
 MAKE_CHECK_ARGS: check-avocado
 AVOCADO_TAGS: arch:ppc64 arch:or1k arch:s390x arch:x86_64 arch:rx
   arch:sh4 arch:nios2
@@ -327,9 +327,9 @@ avocado-system-flaky:
 build-tcg-disabled:
   extends: .native_build_job_template
   needs:
-job: amd64-centos8-container
+job: amd64-centos9-container
   variables:
-IMAGE: centos8
+IMAGE: centos9
   script:
 - mkdir build
 - cd build
@@ -651,9 +651,9 @@ build-tci:
 build-without-defaults:
   extends: .native_build_job_template
   needs:
-job: amd64-centos8-container
+job: amd64-centos9-container
   variables:
-IMAGE: centos8
+IMAGE: centos9
 CONFIGURE_ARGS:
   --without-default-devices
   --without-default-features
diff --git a/.gitlab-ci.d/container-core.yml b/.gitlab-ci.d/container-core.yml
index 08f8450fa1..5459447676 100644
--- a/.gitlab-ci.d/container-core.yml
+++ b/.gitlab-ci.d/container-core.yml
@@ -1,10 +1,10 @@
 include:
   - local: '/.gitlab-ci.d/container-template.yml'
 
-amd64-centos8-container:
+amd64-centos9-container:
   extends: .container_job_template
   variables:
-NAME: centos8
+NAME: centos9
 
 amd64-fedora-container:
   extends: .container_job_template
diff --git a/tests/docker/dockerfiles/centos8.docker 
b/tests/docker/dockerfiles/centos9.docker
similarity index 82%
rename from tests/docker/dockerfiles/centos8.docker
rename to tests/docker/dockerfiles/centos9.docker
index ea618bf352..6cf47ce786 100644
--- a/tests/docker/dockerfiles/centos8.docker
+++ b/tests/docker/dockerfiles/centos9.docker
@@ -1,15 +1,14 @@
 # THIS FILE WAS AUTO-GENERATED
 #
-#  $ lcitool dockerfile --layers all centos-stream-8 qemu
+#  $ lcitool dockerfile --layers all centos-stream-9 qemu
 #
 # https://gitlab.com/libvirt/libvirt-ci
 
-FROM quay.io/centos/centos:stream8
+FROM quay.io/centos/centos:stream9
 
 RUN dnf distro-sync -y && \
 dnf install 'dnf-command(config-manager)' -y && \
-dnf config-manager --set-enabled -y powertools && \
-dnf install -y centos-release-advanced-virtualization && \
+dnf config-manager --set-enabled -y crb && \
 dnf install -y epel-release && \
 dnf install -y epel-next-release && \
 dnf install -y \
@@ -42,7 +41,6 @@ RUN dnf distro-sync -y && \
 glib2-static \
 glibc-langpack-en \
 glibc-static \
-glusterfs-api-devel \
 gnutls-devel \
 gtk3-devel \
 hostname \
@@ -82,6 +80,7 @@ RUN dnf distro-sync -y && \
 lzo-devel \
 make \
 mesa-libgbm-devel \
+meson \
 mtools \
 ncurses-devel \
 nettle-devel \
@@ -95,25 +94,25 @@ RUN dnf distro-sync -y && \
 pixman-devel \
 pkgconfig \
 pulseaudio-libs-devel \
-python38 \
-python38-PyYAML \
-python38-numpy \
-python38-pip \
-python38-setuptools \
-python38-wheel \
+python3 \
+python3-PyYAML \
+python3-numpy \
+python3-pillow \
+python3-pip \
+python3-sphinx \
+python3-sphinx_rtd_theme \
+python3-tomli \
 rdma-core-devel \
 sed \
 snappy-devel \
 socat \
 spice-protocol \
-spice-server-devel \
 

[PATCH v2 13/13] util/uri: Remove the old URI parsing code

[Thomas Huth]

Now that we switched all consumers of the URI code to use the URI
parsing functions from glib instead, we can remove our internal
URI parsing code since it is not used anymore.

Signed-off-by: Thomas Huth 
---
 include/qemu/uri.h |   99 ---
 util/uri.c | 1466 
 util/meson.build   |2 +-
 3 files changed, 1 insertion(+), 1566 deletions(-)
 delete mode 100644 include/qemu/uri.h
 delete mode 100644 util/uri.c

diff --git a/include/qemu/uri.h b/include/qemu/uri.h
deleted file mode 100644
index 255e61f452..00
--- a/include/qemu/uri.h
+++ /dev/null
@@ -1,99 +0,0 @@
-/**
- * Summary: library of generic URI related routines
- * Description: library of generic URI related routines
- *  Implements RFC 2396
- *
- * Copyright (C) 1998-2003 Daniel Veillard.  All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to 
deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL THE
- * DANIEL VEILLARD BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
- * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- * Except as contained in this notice, the name of Daniel Veillard shall not
- * be used in advertising or otherwise to promote the sale, use or other
- * dealings in this Software without prior written authorization from him.
- *
- * Author: Daniel Veillard
- **
- * Copyright (C) 2007 Red Hat, Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library. If not, see 
.
- *
- * Authors:
- *Richard W.M. Jones 
- *
- * Utility functions to help parse and assemble query strings.
- */
-
-#ifndef QEMU_URI_H
-#define QEMU_URI_H
-
-/**
- * URI:
- *
- * A parsed URI reference. This is a struct containing the various fields
- * as described in RFC 2396 but separated for further processing.
- */
-typedef struct URI {
-char *scheme;  /* the URI scheme */
-char *opaque;  /* opaque part */
-char *authority;   /* the authority part */
-char *server;  /* the server part */
-char *user;/* the user part */
-int port;  /* the port number */
-char *path;/* the path string */
-char *fragment;/* the fragment identifier */
-int cleanup;   /* parsing potentially unclean URI */
-char *query;   /* the query string (as it appears in the URI) */
-} URI;
-
-URI *uri_new(void);
-URI *uri_parse(const char *str);
-URI *uri_parse_raw(const char *str, int raw);
-int uri_parse_into(URI *uri, const char *str);
-char *uri_to_string(URI *uri);
-void uri_free(URI *uri);
-
-/* Single web service query parameter 'name=value'. */
-typedef struct QueryParam {
-  char *name;  /* Name (unescaped). */
-  char *value; /* Value (unescaped). */
-  int ignore;  /* Ignore this field in qparam_get_query */
-} QueryParam;
-
-/* Set of parameters. */
-typedef struct QueryParams {
-  int n;   /* number of parameters used */
-  int alloc;   /* allocated space */
-  QueryParam *p;   /* array of parameters */
-} QueryParams;
-
-QueryParams *query_params_new(int init_alloc);
-QueryParams *query_params_parse(const char *query);
-void query_params_free(QueryParams *ps);
-
-#endif /* QEMU_URI_H */
diff --git a/util/uri.c b/util/uri.c
deleted file mode 100644
index 573174bf47..00
--- a/util/uri.c
+++ /dev/null
@@ -1,1466 +0,0 @@
-/**
- * uri.c: set of generic URI related routines
- *
- * Reference: RFCs 3986, 2732 and 2373
- *
- * Copyright (C) 1998-2003 Daniel Veillard.  All Rights Reserved.
- *
- * Permission is hereby g

[PATCH v2 01/13] tests: Remove Ubuntu 20.04 container

[Thomas Huth]

Since Ubuntu 22.04 is now available since two years, we can stop
actively supporting the previous LTS version of Ubuntu now.

Signed-off-by: Thomas Huth 
---
 tests/docker/dockerfiles/ubuntu2004.docker | 157 -
 tests/lcitool/refresh  |   1 -
 2 files changed, 158 deletions(-)
 delete mode 100644 tests/docker/dockerfiles/ubuntu2004.docker

diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
b/tests/docker/dockerfiles/ubuntu2004.docker
deleted file mode 100644
index d3e212060c..00
--- a/tests/docker/dockerfiles/ubuntu2004.docker
+++ /dev/null
@@ -1,157 +0,0 @@
-# THIS FILE WAS AUTO-GENERATED
-#
-#  $ lcitool dockerfile --layers all ubuntu-2004 qemu
-#
-# https://gitlab.com/libvirt/libvirt-ci
-
-FROM docker.io/library/ubuntu:20.04
-
-RUN export DEBIAN_FRONTEND=noninteractive && \
-apt-get update && \
-apt-get install -y eatmydata && \
-eatmydata apt-get dist-upgrade -y && \
-eatmydata apt-get install --no-install-recommends -y \
-  bash \
-  bc \
-  bison \
-  bsdmainutils \
-  bzip2 \
-  ca-certificates \
-  ccache \
-  clang \
-  dbus \
-  debianutils \
-  diffutils \
-  exuberant-ctags \
-  findutils \
-  flex \
-  g++ \
-  gcc \
-  gcovr \
-  gettext \
-  git \
-  hostname \
-  libaio-dev \
-  libasan6 \
-  libasound2-dev \
-  libattr1-dev \
-  libbrlapi-dev \
-  libbz2-dev \
-  libc6-dev \
-  libcacard-dev \
-  libcap-ng-dev \
-  libcapstone-dev \
-  libcmocka-dev \
-  libcurl4-gnutls-dev \
-  libdaxctl-dev \
-  libdrm-dev \
-  libepoxy-dev \
-  libfdt-dev \
-  libffi-dev \
-  libfuse3-dev \
-  libgbm-dev \
-  libgcrypt20-dev \
-  libglib2.0-dev \
-  libglusterfs-dev \
-  libgnutls28-dev \
-  libgtk-3-dev \
-  libibumad-dev \
-  libibverbs-dev \
-  libiscsi-dev \
-  libjemalloc-dev \
-  libjpeg-turbo8-dev \
-  libjson-c-dev \
-  liblttng-ust-dev \
-  liblzo2-dev \
-  libncursesw5-dev \
-  libnfs-dev \
-  libnuma-dev \
-  libpam0g-dev \
-  libpcre2-dev \
-  libpixman-1-dev \
-  libpmem-dev \
-  libpng-dev \
-  libpulse-dev \
-  librbd-dev \
-  librdmacm-dev \
-  libsasl2-dev \
-  libsdl2-dev \
-  libsdl2-image-dev \
-  libseccomp-dev \
-  libselinux1-dev \
-  libslirp-dev \
-  libsnappy-dev \
-  libsndio-dev \
-  libspice-protocol-dev \
-  libspice-server-dev \
-  libssh-dev \
-  libsystemd-dev \
-  libtasn1-6-dev \
-  libubsan1 \
-  libudev-dev \
-  libusb-1.0-0-dev \
-  libusbredirhost-dev \
-  libvdeplug-dev \
-  libvirglrenderer-dev \
-  libvte-2.91-dev \
-  libxen-dev \
-  libzstd-dev \
-  llvm \
-  locales \
-  make \
-  mtools \
-  multipath-tools \
-  ncat \
-  nettle-dev \
-  ninja-build \
-  openssh-client \
-  pkgconf \
-  python3 \
-  python3-numpy \
-  python3-opencv \
-  python3-pillow \
-  python3-pip \
-  python3-setuptools \
-  python3-sphinx \
-  python3-sphinx-rtd-theme \
-  python3-venv \
-  python3-wheel \
-  python3-yaml \
- 

[PATCH v2 02/13] tests/lcitool/libvirt-ci: Update to the latest master branch

[Thomas Huth]

We need the latest fixes for the lcitool to be able to properly
update our CentOS docker file to CentOS Stream 9.

Signed-off-by: Thomas Huth 
---
 tests/lcitool/libvirt-ci | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci
index 77c800186f..cec6703971 16
--- a/tests/lcitool/libvirt-ci
+++ b/tests/lcitool/libvirt-ci
@@ -1 +1 @@
-Subproject commit 77c800186f34b21be7660750577cc5582a914deb
+Subproject commit cec67039719becbfbab866f9c23574f389cf9559
-- 
2.44.0

[PATCH v2 06/13] ci: move external build environment setups to CentOS Stream 9

[Thomas Huth]

From: Paolo Bonzini 

RHEL 9 (and thus also the derivatives) are available since two years
now, so according to QEMU's support policy, we can drop the active
support for the previous major version 8 now.

Thus upgrade our CentOS Stream build environment playbooks to major
version 9 now.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Thomas Huth 
Message-ID: <20240412103708.27650-1-pbonz...@redhat.com>
Signed-off-by: Thomas Huth 
---
 .../stream/{8 => 9}/build-environment.yml | 31 ++---
 .../stream/{8 => 9}/x86_64/configure  |  4 +-
 .../stream/{8 => 9}/x86_64/test-avocado   |  0
 scripts/ci/setup/build-environment.yml| 44 +++
 4 files changed, 34 insertions(+), 45 deletions(-)
 rename scripts/ci/org.centos/stream/{8 => 9}/build-environment.yml (75%)
 rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/configure (98%)
 rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/test-avocado (100%)

diff --git a/scripts/ci/org.centos/stream/8/build-environment.yml 
b/scripts/ci/org.centos/stream/9/build-environment.yml
similarity index 75%
rename from scripts/ci/org.centos/stream/8/build-environment.yml
rename to scripts/ci/org.centos/stream/9/build-environment.yml
index 1ead77e2cb..cd29fe6f27 100644
--- a/scripts/ci/org.centos/stream/8/build-environment.yml
+++ b/scripts/ci/org.centos/stream/9/build-environment.yml
@@ -2,32 +2,32 @@
 - name: Installation of extra packages to build QEMU
   hosts: all
   tasks:
-- name: Extra check for CentOS Stream 8
+- name: Extra check for CentOS Stream 9
   lineinfile:
 path: /etc/redhat-release
-line: CentOS Stream release 8
+line: CentOS Stream release 9
 state: present
   check_mode: yes
-  register: centos_stream_8
+  register: centos_stream_9
 
-- name: Enable EPEL repo on CentOS Stream 8
+- name: Enable EPEL repo on CentOS Stream 9
   dnf:
 name:
   - epel-release
 state: present
   when:
-- centos_stream_8
+- centos_stream_9
 
-- name: Enable PowerTools repo on CentOS Stream 8
+- name: Enable CRB repo on CentOS Stream 9
   ini_file:
-path: /etc/yum.repos.d/CentOS-Stream-PowerTools.repo
-section: powertools
+path: /etc/yum.repos.d/centos.repo
+section: crb
 option: enabled
 value: "1"
   when:
-- centos_stream_8
+- centos_stream_9
 
-- name: Install basic packages to build QEMU on CentOS Stream 8
+- name: Install basic packages to build QEMU on CentOS Stream 9
   dnf:
 name:
   - bzip2
@@ -42,7 +42,6 @@
   - gettext
   - git
   - glib2-devel
-  - glusterfs-api-devel
   - gnutls-devel
   - libaio-devel
   - libcap-ng-devel
@@ -61,22 +60,24 @@
   - lzo-devel
   - make
   - mesa-libEGL-devel
+  - meson
   - nettle-devel
   - ninja-build
   - nmap-ncat
   - numactl-devel
   - pixman-devel
-  - python38
+  - python3
+  - python3-pip
   - python3-sphinx
+  - python3-sphinx_rtd_theme
+  - python3-tomli
   - rdma-core-devel
   - redhat-rpm-config
   - snappy-devel
-  - spice-glib-devel
-  - spice-server-devel
   - systemd-devel
   - systemtap-sdt-devel
   - tar
   - zlib-devel
 state: present
   when:
-- centos_stream_8
+- centos_stream_9
diff --git a/scripts/ci/org.centos/stream/8/x86_64/configure 
b/scripts/ci/org.centos/stream/9/x86_64/configure
similarity index 98%
rename from scripts/ci/org.centos/stream/8/x86_64/configure
rename to scripts/ci/org.centos/stream/9/x86_64/configure
index 76781f17f4..1b6f40fd78 100755
--- a/scripts/ci/org.centos/stream/8/x86_64/configure
+++ b/scripts/ci/org.centos/stream/9/x86_64/configure
@@ -16,7 +16,7 @@
 # that patches adding downstream specific devices are not available.
 #
 ../configure \
---python=/usr/bin/python3.8 \
+--python=/usr/bin/python3.9 \
 --prefix="/usr" \
 --libdir="/usr/lib64" \
 --datadir="/usr/share" \
@@ -157,7 +157,6 @@
 --enable-docs \
 --enable-fdt \
 --enable-gcrypt \
---enable-glusterfs \
 --enable-gnutls \
 --enable-guest-agent \
 --enable-iconv \
@@ -180,7 +179,6 @@
 --enable-seccomp \
 --enable-snappy \
 --enable-smartcard \
---enable-spice \
 --enable-system \
 --enable-tcg \
 --enable-tools \
diff --git a/scripts/ci/org.centos/stream/8/x86_64/test-avocado 
b/scripts/ci/org.centos/stream/9/x86_64/test-avocado
similarity index 100%
rename from scripts/ci/org.centos/stream/8/x86_64/test-avocado
rename to scripts/ci/org.centos/stream/9/x86_64/test-avocado
diff --git a/scripts/ci/setup/build-environment.yml 
b/scripts/ci/setup/build-environment.yml
index f344d1a850..9b7d96c01b 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -174,26 +174

Re: [PATCH v4 02/10] hw/core: create Resettable QOM interface

[Peter Maydell]

On Thu, 11 Apr 2024 at 18:23, Philippe Mathieu-Daudé  wrote:
>
> On 11/4/24 15:43, Peter Maydell wrote:
> > On Wed, 21 Aug 2019 at 17:34, Damien Hedde  
> > wrote:
> >>
> >> This commit defines an interface allowing multi-phase reset. This aims
> >> to solve a problem of the actual single-phase reset (built in
> >> DeviceClass and BusClass): reset behavior is dependent on the order
> >> in which reset handlers are called. In particular doing external
> >> side-effect (like setting an qemu_irq) is problematic because receiving
> >> object may not be reset yet.
> >
> > So, I wanted to drag up this ancient patch to ask a couple
> > of Resettable questions, because I'm working on adding a
> > new ResetType (the equivalent of SHUTDOWN_CAUSE_SNAPSHOT_LOAD).
> >
> >> +/**
> >> + * ResetType:
> >> + * Types of reset.
> >> + *
> >> + * + Cold: reset resulting from a power cycle of the object.
> >> + *
> >> + * TODO: Support has to be added to handle more types. In particular,
> >> + * ResetState structure needs to be expanded.
> >> + */
> >
> > Does anybody remember what this TODO comment is about? What
> > in particular would need to be in the ResetState struct
> > to allow another type to be added?
>
> IIRC this comes from this discussion:
> https://lore.kernel.org/qemu-devel/7c193b33-8188-2cda-cbf2-fb5452544...@greensocs.com/
> Updated in this patch (see after '---' description):
> https://lore.kernel.org/qemu-devel/20191018150630.31099-9-damien.he...@greensocs.com/

Hmm, I can't see anything in there that mentions this
TODO or what we'd need more ResetState fields to handle.
I guess I'll go ahead with adding my new ResetType and ignore
this TODO, because I can't see any reason why we need to
do anything in particular for a new ResetType...

> >
> >> +typedef enum ResetType {
> >> +RESET_TYPE_COLD,
> >> +} ResetType;
> >
> >> +typedef void (*ResettableInitPhase)(Object *obj, ResetType type);
> >> +typedef void (*ResettableHoldPhase)(Object *obj);
> >> +typedef void (*ResettableExitPhase)(Object *obj);
> >
> > Was there a reason why we only pass the ResetType to the init
> > phase method, and not also to the hold and exit phases ?
> > Given that many devices don't need to implement init, it
> > seems awkward to require them to do so just to stash the
> > ResetType somewhere so they can look at it in the hold
> > or exit phase, so I was thinking about adding the argument
> > to the other two phase methods.
>
> You are right, the type should be propagated to to all phase
> handlers.

I have some patches which do this; I'll probably send them out
in a series next week once I've figured out whether they fit
better in with other patches that give the motivation.

thanks
-- PMM

[PATCH] MAINTAINERS: Update my email address

[Bin Meng]

The Wind River email address might change in the future. Use my
personal email address instead.

Signed-off-by: Bin Meng 

---

 MAINTAINERS | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index f1f6922025..50729a0985 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -332,7 +332,7 @@ F: tests/tcg/ppc*/*
 RISC-V TCG CPUs
 M: Palmer Dabbelt 
 M: Alistair Francis 
-M: Bin Meng 
+M: Bin Meng 
 R: Weiwei Li 
 R: Daniel Henrique Barboza 
 R: Liu Zhiwei 
@@ -1614,7 +1614,7 @@ F: include/hw/riscv/opentitan.h
 F: include/hw/*/ibex_*.h
 
 Microchip PolarFire SoC Icicle Kit
-M: Bin Meng 
+M: Bin Meng 
 L: qemu-ri...@nongnu.org
 S: Supported
 F: docs/system/riscv/microchip-icicle-kit.rst
@@ -1641,7 +1641,7 @@ F: include/hw/char/shakti_uart.h
 
 SiFive Machines
 M: Alistair Francis 
-M: Bin Meng 
+M: Bin Meng 
 M: Palmer Dabbelt 
 L: qemu-ri...@nongnu.org
 S: Supported
@@ -2137,7 +2137,7 @@ F: hw/ssi/xilinx_*
 
 SD (Secure Card)
 M: Philippe Mathieu-Daudé 
-M: Bin Meng 
+M: Bin Meng 
 L: qemu-bl...@nongnu.org
 S: Odd Fixes
 F: include/hw/sd/sd*
-- 
2.34.1

Re: [PATCH for-9.1 4/9] Bump minimum glib version to v2.66

[Paolo Bonzini]

On 4/12/24 12:58, Thomas Huth wrote:

On 12/04/2024 12.16, Paolo Bonzini wrote:

On Thu, Mar 28, 2024 at 3:06 PM Thomas Huth  wrote:


Now that we dropped support for CentOS 8 and Ubuntu 20.04, we can
look into bumping the glib version to a new minimum for further
clean-ups. According to repology.org, available versions are:

  CentOS Stream 9:   2.66.7
  Debian 11: 2.66.8
  Fedora 38: 2.74.1
  Freebsd:   2.78.4
  Homebrew:  2.80.0
  Openbsd:   2.78.4
  OpenSuse leap 15.5:    2.70.5
  pkgsrc_current:    2.78.4
  Ubuntu 22.04:  2.72.1

Thus it should be safe to bump the minimum glib version to 2.66 now.
Version 2.66 comes with new functions for URI parsing which will
allow further clean-ups in the following patches.


Missing:

diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index b0e0b1d674f..cc1f5a708e4 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -288,7 +288,6 @@ qmp_guest_ssh_get_authorized_keys(
  }

  #ifdef QGA_BUILD_UNIT_TEST
-#if GLIB_CHECK_VERSION(2, 60, 0)
  static const strList test_key2 = {
  .value = (char *)"algo key2 comments"
  };
@@ -484,11 +483,4 @@ int main(int argc, char *argv[])

  return g_test_run();
  }
-#else
-int main(int argc, char *argv[])
-{
-    g_test_message("test skipped, needs glib >= 2.60");
-    return 0;
-}
-#endif /* GLIB_2_60 */
  #endif /* BUILD_UNIT_TEST */


Indeed! And there seems to be another GLIB_CHECK_VERSION(2,62,0) check 
in util/error-report.c which we likely can clean up now, too!


Ok, I'll squash the above and

diff --git a/util/error-report.c b/util/error-report.c
index 6e44a557321..1b17c11de19 100644
--- a/util/error-report.c
+++ b/util/error-report.c
@@ -172,18 +172,8 @@ static void print_loc(void)
 static char *
 real_time_iso8601(void)
 {
-#if GLIB_CHECK_VERSION(2,62,0)
 g_autoptr(GDateTime) dt = g_date_time_new_now_utc();
-/* ignore deprecation warning, since GLIB_VERSION_MAX_ALLOWED is 2.56 */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
 return g_date_time_format_iso8601(dt);
-#pragma GCC diagnostic pop
-#else
-GTimeVal tv;
-g_get_current_time(&tv);
-return g_time_val_to_iso8601(&tv);
-#endif
 }
 
 /*


then.  As an aside, we probably can also drop:

/*
 * gtk_widget_set_double_buffered() was deprecated in 3.14.
 * It is required for opengl rendering on X11 though.  A
 * proper replacement (native opengl support) is only
 * available in 3.16+.  Silence the warning if possible.
 */
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
gtk_widget_set_double_buffered(vc->gfx.drawing_area, FALSE);
#pragma GCC diagnostic pop


and


#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
/*
 * check if RBD image is a clone (= has a parent).
 *
 * rbd_get_parent_info is deprecated from Nautilus onwards, but the
 * replacement rbd_get_parent is not present in Luminous and Mimic.
 */
if (rbd_get_parent_info(s->image, NULL, 0, NULL, 0, NULL, 0) != -ENOENT) {
return status;
}
#pragma GCC diagnostic pop


(Nautilus is Ceph 14, it's in all of CentOS Stream 9, Ubuntu 20.04 and
Debian 11) but I have no idea what the replacement would be. :/

Paolo

Re: [PATCH] Makefile: fix use of -j without an argument

[Matheus Tavares Bernardino]

On Fri, 12 Apr 2024 10:02:54 +0200 Paolo Bonzini  wrote:
>
> On Thu, Apr 11, 2024 at 5:46 PM Matheus Tavares Bernardino
>  wrote:
> > +$(if $(filter -j, $(MAKEFLAGS)) \
> > +,, \
> > +$(or \
> > + $(filter -l% -j%, $(MAKEFLAGS)), \
> > + $(if $(filter --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \
> > +) -d keepdepfile
> 
> This is more easily written as $(filter-out -j, $(or ...)).
> 
> I've sent a v2.

Thanks! 

Re: [PATCH] Makefile: preserve --jobserver-auth argument when calling ninja

[Paolo Bonzini]

On Fri, Apr 12, 2024 at 1:52 PM Fiona Ebner  wrote:
>
> Am 02.04.24 um 10:17 schrieb Martin Hundebøll:
> > Qemu wraps its call to ninja in a Makefile. Since ninja, as opposed to
> > make, utilizes all CPU cores by default, the qemu Makefile translates
> > the absense of a `-jN` argument into `-j1`. This breaks jobserver
> > functionality, so update the -jN mangling to take the --jobserver-auth
> > argument into considerationa too.
> >
> > Signed-off-by: Martin Hundebøll 
> > ---
> >  Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 8f36990335..183756018f 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -142,7 +142,7 @@ MAKE.k = $(findstring k,$(firstword $(filter-out 
> > --%,$(MAKEFLAGS
> >  MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS
> >  MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq)
> >  NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \
> > -$(filter-out -j, $(lastword -j1 $(filter -l% -j%, $(MAKEFLAGS \
> > +$(or $(filter -l% -j%, $(MAKEFLAGS)), $(if $(filter 
> > --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \
> >  -d keepdepfile
> >  ninja-cmd-goals = $(or $(MAKECMDGOALS), all)
> >  ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g))
>
> Hi,
>
> unfortunately, this patch breaks build when specifying just '-j' as a
> make flag (i.e. without a number), because it will now end up being
> passed to ninja:

Yep, I've sent a pull request with the fix.

Paolo

Re: [PATCH] Makefile: preserve --jobserver-auth argument when calling ninja

[Fiona Ebner]

Am 02.04.24 um 10:17 schrieb Martin Hundebøll:
> Qemu wraps its call to ninja in a Makefile. Since ninja, as opposed to
> make, utilizes all CPU cores by default, the qemu Makefile translates
> the absense of a `-jN` argument into `-j1`. This breaks jobserver
> functionality, so update the -jN mangling to take the --jobserver-auth
> argument into considerationa too.
> 
> Signed-off-by: Martin Hundebøll 
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 8f36990335..183756018f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -142,7 +142,7 @@ MAKE.k = $(findstring k,$(firstword $(filter-out 
> --%,$(MAKEFLAGS
>  MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS
>  MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq)
>  NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \
> -$(filter-out -j, $(lastword -j1 $(filter -l% -j%, $(MAKEFLAGS \
> +$(or $(filter -l% -j%, $(MAKEFLAGS)), $(if $(filter 
> --jobserver-auth=%, $(MAKEFLAGS)),, -j1)) \
>  -d keepdepfile
>  ninja-cmd-goals = $(or $(MAKECMDGOALS), all)
>  ninja-cmd-goals += $(foreach g, $(MAKECMDGOALS), $(.ninja-goals.$g))

Hi,

unfortunately, this patch breaks build when specifying just '-j' as a
make flag (i.e. without a number), because it will now end up being
passed to ninja:

> $ make -j
> changing dir to build for make ""...
> make[1]: Entering directory '/home/febner/repos/qemu/build'
> ninja: fatal: invalid -j parameter

Best Regards,
Fiona

Re: [PATCH v3 17/51] target/arm: Add cpu properties for SME

[Peter Maydell]

On Mon, 20 Jun 2022 at 19:08, Richard Henderson
 wrote:
>
> Mirror the properties for SVE.  The main difference is
> that any arbitrary set of powers of 2 may be supported,
> and not the stricter constraints that apply to SVE.

> +SME CPU Property Examples
> +-
> +
> +  1) Disable SME::
> +
> + $ qemu-system-aarch64 -M virt -cpu max,sme=off
> +
> +  2) Implicitly enable all vector lengths for the ``max`` CPU type::
> +
> + $ qemu-system-aarch64 -M virt -cpu max
> +
> +  3) Only enable the 256-bit vector length::
> +
> + $ qemu-system-aarch64 -M virt -cpu max,sme256=on
> +
> +  3) Enable the 256-bit and 1024-bit vector lengths::
> +
> + $ qemu-system-aarch64 -M virt -cpu max,sme256=on,sme1024=on
> +
> +  4) Disable the 512-bit vector length.  This results in all the other
> + lengths supported by ``max`` defaulting to enabled
> + (128, 256, 1024 and 2048)::
> +
> + $ qemu-system-aarch64 -M virt -cpu max,sve512=off
> +

I just noticed this while I was trying to understand the
SME and SVE property documentation -- the example 4 here
is in the SME property examples section, but it's changing
sve512, not sme512. Is that an error, or intentional? (If the
latter, the explanation could be enlarged upon to say that
it's changing behaviour of the SME vector length by adjusting
the SVE vector length.)

Also, the documentation for the SVE properties uses this
same command line, but it describes the effects differently:

# Disable the 512-bit vector length and all larger vector lengths,
# since 512 is a power-of-two. This results in all the smaller,
# uninitialized lengths (128, 256, and 384) defaulting to enabled:
#
# $ qemu-system-aarch64 -M virt -cpu max,sve512=off

In the SME section we say that all other lengths default
to enabled, but in the SVE section we say that the
smaller lengths default to enabled but the longer
lengths are disabled. Is:
 * the SVE part wrong?
 * the SME part wrong?
 * the behaviour deliberately different for SVE and SME
   vector lengths? (If so, we should say so explicitly to
   highlight that to users).

thanks
-- PMM

Re: [PATCH-for-9.0?] docs: i386: pc: Update maximum CPU numbers for PC Q35

[Zhao Liu]

Hi Philippe,

On Fri, Apr 12, 2024 at 11:57:35AM +0200, Philippe Mathieu-Daudé wrote:
> Date: Fri, 12 Apr 2024 11:57:35 +0200
> From: Philippe Mathieu-Daudé 
> Subject: Re: [PATCH-for-9.0?] docs: i386: pc: Update maximum CPU numbers
>  for PC Q35
> 
> > -SMP is supported with up to 255 CPUs.
> > +SMP is supported with up to 255 CPUs (and 4096 CPUs for PC Q35 machine).
> 
> This comment is not accurate since a while, IIUC:
> 
> Up to q35-2.7: 255
> q35-2.8: 288
> q35-8.0+: 1024
> q35-9.0: 4096

Yes, I think there should be no need to mention older versions here?

Since the Q35's 4096 CPUs change will be stated in the v9.0 release, I
doubt we should synchronize the doc update (so I added the "for-v9.0?"
tag to throw this question out).

Thanks,
Zhao

Re: [PATCH 1/2] hw: Fix problem with the A*MPCORE switches in the Kconfig files

[Thomas Huth]

On 12/04/2024 13.10, Philippe Mathieu-Daudé wrote:

On 12/4/24 08:20, Thomas Huth wrote:

A9MPCORE, ARM11MPCORE and A15MPCORE are defined twice, once in
hw/cpu/Kconfig and once in hw/arm/Kconfig. This is only possible
by accident, since hw/cpu/Kconfig is never included from hw/Kconfig.
Fix it by declaring the switches only in hw/cpu/Kconfig (since the
related files reside in the hw/cpu/ folder) and by making sure that
the file hw/cpu/Kconfig is now properly included from hw/Kconfig.

Signed-off-by: Thomas Huth 
---
  hw/Kconfig |  1 +
  hw/arm/Kconfig | 15 ---
  hw/cpu/Kconfig | 12 +---
  3 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/hw/Kconfig b/hw/Kconfig
index 2c00936c28..9567cc475d 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -48,6 +48,7 @@ source watchdog/Kconfig
  # arch Kconfig
  source arm/Kconfig
+source cpu/Kconfig
  source alpha/Kconfig
  source avr/Kconfig
  source cris/Kconfig
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 893a7bff66..d97015c45c 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -678,21 +678,6 @@ config ZAURUS
  select NAND
  select ECC
-config A9MPCORE
-    bool
-    select A9_GTIMER
-    select A9SCU   # snoop control unit
-    select ARM_GIC
-    select ARM_MPTIMER
-
-config A15MPCORE
-    bool
-    select ARM_GIC
-
-config ARM11MPCORE
-    bool
-    select ARM11SCU
-
  config ARMSSE
  bool
  select ARM_V7M
diff --git a/hw/cpu/Kconfig b/hw/cpu/Kconfig
index 1767d028ac..f776e884cd 100644
--- a/hw/cpu/Kconfig
+++ b/hw/cpu/Kconfig
@@ -1,8 +1,14 @@
-config ARM11MPCORE
-    bool
-
  config A9MPCORE
  bool
+    select A9_GTIMER
+    select A9SCU   # snoop control unit
+    select ARM_GIC
+    select ARM_MPTIMER
  config A15MPCORE
  bool
+    select ARM_GIC
+
+config ARM11MPCORE
+    bool
+    select ARM11SCU


I thought 
https://lore.kernel.org/qemu-devel/20231212162935.42910-6-phi...@linaro.org/

  was already merged :/ I'll look at what is missing and respin.


Oh, ok. But I'd prefer to keep the hw/cpu/Kconfig file since it will be used 
in the 2nd patch here, too.


 Thomas

Re: [PULL 0/2] Final build system fixes for 9.0

[Peter Maydell]

On Fri, 12 Apr 2024 at 12:07, Philippe Mathieu-Daudé  wrote:
>
> Hi Peter,
>
> On 12/4/24 12:03, Paolo Bonzini wrote:
> > The following changes since commit 02e16ab9f4f19c4bdd17c51952d70e2ded74c6bf:
> >
> >Update version for v9.0.0-rc3 release (2024-04-10 18:05:18 +0100)
> >
> > are available in the Git repository at:
> >
> >https://gitlab.com/bonzini/qemu.git tags/for-upstream
> >
> > for you to fetch changes up to 2d6d995709482cc8b6a76dbb5334a28001a14a9a:
> >
> >meson.build: Disable -fzero-call-used-regs on OpenBSD (2024-04-12 
> > 12:02:12 +0200)
> >
> > 
> > build system fixes
>
> Since these 2 patches don't modify what we can build with v9.0.0-rc3,
> would it be acceptable to merge them without having to produce a
> v9.0.0-rc4 tag before the final release?

As a principle, I don't ever do a final release that doesn't
match the last rc tag. If the changes are minimal I might
reduce the time between last-rc and final release.

But we'll see if anything else turns up that needs to go
into 9.0. There was so much late stuff for rc3 that I
have a feeling this might not be the only 9.0 pullreq.

thanks
-- PMM

Re: [PATCH] target/sparc: Use GET_ASI_CODE for ASI_KERNELTXT and ASI_USERTXT

[Philippe Mathieu-Daudé]

Hi Bazz,

On 12/4/24 06:18, M Bazz wrote:
On Thu, Apr 11, 2024, 10:15 PM Richard Henderson 
mailto:richard.hender...@linaro.org>> wrote:


Reads are done with execute access.  It is not clear whether writes
are legal at all -- for now, leave helper_st_asi unchanged, so that
we continue to raise an mmu fault.

This generalizes the exiting code for ASI_KERNELTXT to be usable for
ASI_USERTXT as well, by passing down the MemOpIdx to use.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2281

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2059

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1609

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1166

Signed-off-by: Richard Henderson mailto:richard.hender...@linaro.org>>
---
  target/sparc/helper.h      |  3 ++
  target/sparc/ldst_helper.c | 65 ++
  target/sparc/translate.c   | 49 ++--
  3 files changed, 95 insertions(+), 22 deletions(-)



Hi Richard,

I see this is in your hands now. I agree with your take on leaving 
writes alone. I'm also grateful for the opportunity to collaborate with you.


It brings a smile for the community members who will be touched by this 
amazing contribution. I see them happily realizing that this perplexing 
bug has been solved, and in our case finally able to use the debuggers 
we love! :D


Does that mean you tested this patch and it resolves your issues?

If so, could you add your 'Tested-by: M Bazz ' tag
when committing this patch?

Regards,

Phil.


Thanks for the proper fix, qemu sensei!

-bazz

Re: [PATCH 1/2] hw: Fix problem with the A*MPCORE switches in the Kconfig files

[Philippe Mathieu-Daudé]

On 12/4/24 08:20, Thomas Huth wrote:

A9MPCORE, ARM11MPCORE and A15MPCORE are defined twice, once in
hw/cpu/Kconfig and once in hw/arm/Kconfig. This is only possible
by accident, since hw/cpu/Kconfig is never included from hw/Kconfig.
Fix it by declaring the switches only in hw/cpu/Kconfig (since the
related files reside in the hw/cpu/ folder) and by making sure that
the file hw/cpu/Kconfig is now properly included from hw/Kconfig.

Signed-off-by: Thomas Huth 
---
  hw/Kconfig |  1 +
  hw/arm/Kconfig | 15 ---
  hw/cpu/Kconfig | 12 +---
  3 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/hw/Kconfig b/hw/Kconfig
index 2c00936c28..9567cc475d 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -48,6 +48,7 @@ source watchdog/Kconfig
  
  # arch Kconfig

  source arm/Kconfig
+source cpu/Kconfig
  source alpha/Kconfig
  source avr/Kconfig
  source cris/Kconfig
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 893a7bff66..d97015c45c 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -678,21 +678,6 @@ config ZAURUS
  select NAND
  select ECC
  
-config A9MPCORE

-bool
-select A9_GTIMER
-select A9SCU   # snoop control unit
-select ARM_GIC
-select ARM_MPTIMER
-
-config A15MPCORE
-bool
-select ARM_GIC
-
-config ARM11MPCORE
-bool
-select ARM11SCU
-
  config ARMSSE
  bool
  select ARM_V7M
diff --git a/hw/cpu/Kconfig b/hw/cpu/Kconfig
index 1767d028ac..f776e884cd 100644
--- a/hw/cpu/Kconfig
+++ b/hw/cpu/Kconfig
@@ -1,8 +1,14 @@
-config ARM11MPCORE
-bool
-
  config A9MPCORE
  bool
+select A9_GTIMER
+select A9SCU   # snoop control unit
+select ARM_GIC
+select ARM_MPTIMER
  
  config A15MPCORE

  bool
+select ARM_GIC
+
+config ARM11MPCORE
+bool
+select ARM11SCU


I thought 
https://lore.kernel.org/qemu-devel/20231212162935.42910-6-phi...@linaro.org/

 was already merged :/ I'll look at what is missing and respin.

Re: [PULL 0/2] Final build system fixes for 9.0

[Paolo Bonzini]

> Since these 2 patches don't modify what we can build with v9.0.0-rc3,
> would it be acceptable to merge them without having to produce a
> v9.0.0-rc4 tag before the final release?

I didn't want to ask you about that, but I agree it would not be an issue.

Paolo

Re: [PULL 0/2] Final build system fixes for 9.0

[Philippe Mathieu-Daudé]

Hi Peter,

On 12/4/24 12:03, Paolo Bonzini wrote:

The following changes since commit 02e16ab9f4f19c4bdd17c51952d70e2ded74c6bf:

   Update version for v9.0.0-rc3 release (2024-04-10 18:05:18 +0100)

are available in the Git repository at:

   https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 2d6d995709482cc8b6a76dbb5334a28001a14a9a:

   meson.build: Disable -fzero-call-used-regs on OpenBSD (2024-04-12 12:02:12 
+0200)


build system fixes


Since these 2 patches don't modify what we can build with v9.0.0-rc3,
would it be acceptable to merge them without having to produce a
v9.0.0-rc4 tag before the final release?



Matheus Tavares Bernardino (1):
   Makefile: fix use of -j without an argument

Thomas Huth (1):
   meson.build: Disable -fzero-call-used-regs on OpenBSD

  Makefile| 9 +++--
  meson.build | 6 +-
  2 files changed, 12 insertions(+), 3 deletions(-)

Re: [PATCH v3 27/27] target/i386/kvm: Improve KVM_EXIT_NOTIFY warnings

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  target/i386/kvm/kvm.c | 12 ++--
  1 file changed, 6 insertions(+), 6 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH for-9.1 4/9] Bump minimum glib version to v2.66

[Thomas Huth]

On 12/04/2024 12.16, Paolo Bonzini wrote:

On Thu, Mar 28, 2024 at 3:06 PM Thomas Huth  wrote:


Now that we dropped support for CentOS 8 and Ubuntu 20.04, we can
look into bumping the glib version to a new minimum for further
clean-ups. According to repology.org, available versions are:

  CentOS Stream 9:   2.66.7
  Debian 11: 2.66.8
  Fedora 38: 2.74.1
  Freebsd:   2.78.4
  Homebrew:  2.80.0
  Openbsd:   2.78.4
  OpenSuse leap 15.5:2.70.5
  pkgsrc_current:2.78.4
  Ubuntu 22.04:  2.72.1

Thus it should be safe to bump the minimum glib version to 2.66 now.
Version 2.66 comes with new functions for URI parsing which will
allow further clean-ups in the following patches.


Missing:

diff --git a/qga/commands-posix-ssh.c b/qga/commands-posix-ssh.c
index b0e0b1d674f..cc1f5a708e4 100644
--- a/qga/commands-posix-ssh.c
+++ b/qga/commands-posix-ssh.c
@@ -288,7 +288,6 @@ qmp_guest_ssh_get_authorized_keys(
  }

  #ifdef QGA_BUILD_UNIT_TEST
-#if GLIB_CHECK_VERSION(2, 60, 0)
  static const strList test_key2 = {
  .value = (char *)"algo key2 comments"
  };
@@ -484,11 +483,4 @@ int main(int argc, char *argv[])

  return g_test_run();
  }
-#else
-int main(int argc, char *argv[])
-{
-g_test_message("test skipped, needs glib >= 2.60");
-return 0;
-}
-#endif /* GLIB_2_60 */
  #endif /* BUILD_UNIT_TEST */


Indeed! And there seems to be another GLIB_CHECK_VERSION(2,62,0) check in 
util/error-report.c which we likely can clean up now, too!


 Thomas

Re: [PATCH v5 1/3] qio: add support for SO_PEERCRED for socket channel

[Paolo Bonzini]

On Thu, Apr 11, 2024 at 2:14 PM Anthony Harivel  wrote:
>
> The function qio_channel_get_peercred() returns a pointer to the
> credentials of the peer process connected to this socket.
>
> This credentials structure is defined in  as follows:
>
> struct ucred {
> pid_t pid;/* Process ID of the sending process */
> uid_t uid;/* User ID of the sending process */
> gid_t gid;/* Group ID of the sending process */
> };
>
> The use of this function is possible only for connected AF_UNIX stream
> sockets and for AF_UNIX stream and datagram socket pairs.
>
> On platform other than Linux, the function return 0.
>
> Signed-off-by: Anthony Harivel 
> ---
>  include/io/channel.h | 21 +
>  io/channel-socket.c  | 28 
>  io/channel.c | 13 +
>  3 files changed, 62 insertions(+)
>
> diff --git a/include/io/channel.h b/include/io/channel.h
> index 7986c49c713a..bdf0bca92ae2 100644
> --- a/include/io/channel.h
> +++ b/include/io/channel.h
> @@ -160,6 +160,9 @@ struct QIOChannelClass {
>void *opaque);
>  int (*io_flush)(QIOChannel *ioc,
>  Error **errp);
> +int (*io_peerpid)(QIOChannel *ioc,
> +   unsigned int *pid,
> +   Error **errp);
>  };
>
>  /* General I/O handling functions */
> @@ -981,4 +984,22 @@ int coroutine_mixed_fn 
> qio_channel_writev_full_all(QIOChannel *ioc,
>  int qio_channel_flush(QIOChannel *ioc,
>Error **errp);
>
> +/**
> + * qio_channel_get_peercred:
> + * @ioc: the channel object
> + * @pid: pointer to pid
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Returns the pid of the peer process connected to this socket.
> + *
> + * The use of this function is possible only for connected
> + * AF_UNIX stream sockets and for AF_UNIX stream and datagram
> + * socket pairs on Linux.
> + * Return -1 on error with pid -1 for the non-Linux OS.

with pid -1 -> and set *pid to -1.

> + */
>  static const TypeInfo qio_channel_socket_info = {
> diff --git a/io/channel.c b/io/channel.c
> index a1f12f8e9096..e3f17c24a00f 100644
> --- a/io/channel.c
> +++ b/io/channel.c
> @@ -548,6 +548,19 @@ void qio_channel_set_cork(QIOChannel *ioc,
>  }
>  }
>
> +int qio_channel_get_peerpid(QIOChannel *ioc,
> + unsigned int *pid,
> + Error **errp)
> +{
> +QIOChannelClass *klass = QIO_CHANNEL_GET_CLASS(ioc);
> +
> +if (!klass->io_peerpid) {
> +error_setg(errp, "Channel does not support peer pid");

Missing for consistency:

+*pid = -1;

> +return -1;
> +}
> +klass->io_peerpid(ioc, pid, errp);
> +return 0;

The error from klass->io_peerpid is ignored:

-klass->io_peerpid(ioc, pid, errp);
-return 0;
+return klass->io_peerpid(ioc, pid, errp);

Paolo

Re: [PATCH v3 24/27] hw/net/rocker: Replace sprintf() by snprintf()

[Philippe Mathieu-Daudé]

On 12/4/24 09:33, Richard Henderson wrote:

From: Philippe Mathieu-Daudé 

sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience. Use snprintf() instead.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20240411104340.6617-7-phi...@linaro.org>
Signed-off-by: Richard Henderson 
---
  hw/net/rocker/rocker.c | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)




  switch (offset) {
  case ROCKER_DMA_DESC_ADDR_OFFSET:
-sprintf(buf, "Ring[%s] ADDR", ring_name);
+snprintf(buf, sizeofbuf), "Ring[%s] ADDR", ring_name);


Ideally we should convert the DEBUG_FOO guards to trace events,
to avoid to maintain dead code.

Re: [PATCH v5 0/3] Add support for the RAPL MSRs series

[Paolo Bonzini]

On Thu, Apr 11, 2024 at 2:14 PM Anthony Harivel  wrote:
>
> Dear maintainers,
>
> First of all, thank you very much for your review of my patch
> [1].
>
> In this version (v5), I have attempted to address all the problems
> addressed by Daniel during the last review. I've been more careful with
> all the remarks made.
>
> However, one question remains unanswered pointing the issue with the
> location of "/var/local/run/qemu-vmsr-helper.sock", created by
> compute_default_paths(). QEMU is not allowed to reach the socket here.

If I understand correctly the question, that is expected. This is a
privileged functionality and therefore it requires manual intervention
to change the owner of the socket and allow QEMU to access it.

Paolo

> Thank you again for your continued guidance.
>
> v4 -> v5
> 
>
> - correct qio_channel_get_peerpid: return pid = -1 in case of error
> - Vmsr_helper: compile only for x86
> - Vmsr_helper: use qio_channel_read/write_all
> - Vmsr_helper: abandon user/group
> - Vmsr_energy.c: correct all error_report
> - Vmsr thread: compute default socket path only once
> - Vmsr thread: open socket only once
> - Pass relevant QEMU CI
>
> v3 -> v4
> 
>
> - Correct memory leaks with AddressSanitizer
> - Add sanity check for QEMU and qemu-vmsr-helper for checking if host is
>   INTEL and if RAPL is activated.
> - Rename poor variables naming for easier comprehension
> - Move code that checks Host before creating the VMSR thread
> - Get rid of libnuma: create function that read sysfs for reading the
>   Host topology instead
>
> v2 -> v3
> 
>
> - Move all memory allocations from Clib to Glib
> - Compile on *BSD (working on Linux only)
> - No more limitation on the virtual package: each vCPU that belongs to
>   the same virtual package is giving the same results like expected on
>   a real CPU.
>   This has been tested topology like:
>  -smp 4,sockets=2
>  -smp 16,sockets=4,cores=2,threads=2
>
> v1 -> v2
> 
>
> - To overcome the CVE-2020-8694 a socket communication is created
>   to a priviliged helper
> - Add the priviliged helper (qemu-vmsr-helper)
> - Add SO_PEERCRED in qio channel socket
>
> RFC -> v1
> -
>
> - Add vmsr_* in front of all vmsr specific function
> - Change malloc()/calloc()... with all glib equivalent
> - Pre-allocate all dynamic memories when possible
> - Add a Documentation of implementation, limitation and usage
>
> Best regards,
> Anthony
>
> [1]: https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg04417.html
>
> Anthony Harivel (3):
>   qio: add support for SO_PEERCRED for socket channel
>   tools: build qemu-vmsr-helper
>   Add support for RAPL MSRs in KVM/Qemu
>
>  accel/kvm/kvm-all.c  |  27 ++
>  contrib/systemd/qemu-vmsr-helper.service |  15 +
>  contrib/systemd/qemu-vmsr-helper.socket  |   9 +
>  docs/specs/index.rst |   1 +
>  docs/specs/rapl-msr.rst  | 155 +++
>  docs/tools/index.rst |   1 +
>  docs/tools/qemu-vmsr-helper.rst  |  89 
>  include/io/channel.h |  21 +
>  include/sysemu/kvm.h |   2 +
>  include/sysemu/kvm_int.h |  32 ++
>  io/channel-socket.c  |  28 ++
>  io/channel.c |  13 +
>  meson.build  |   7 +
>  target/i386/cpu.h|   8 +
>  target/i386/kvm/kvm-cpu.c|   9 +
>  target/i386/kvm/kvm.c| 428 ++
>  target/i386/kvm/meson.build  |   1 +
>  target/i386/kvm/vmsr_energy.c| 335 ++
>  target/i386/kvm/vmsr_energy.h|  99 +
>  tools/i386/qemu-vmsr-helper.c| 529 +++
>  tools/i386/rapl-msr-index.h  |  28 ++
>  21 files changed, 1837 insertions(+)
>  create mode 100644 contrib/systemd/qemu-vmsr-helper.service
>  create mode 100644 contrib/systemd/qemu-vmsr-helper.socket
>  create mode 100644 docs/specs/rapl-msr.rst
>  create mode 100644 docs/tools/qemu-vmsr-helper.rst
>  create mode 100644 target/i386/kvm/vmsr_energy.c
>  create mode 100644 target/i386/kvm/vmsr_energy.h
>  create mode 100644 tools/i386/qemu-vmsr-helper.c
>  create mode 100644 tools/i386/rapl-msr-index.h
>
> --
> 2.44.0
>

Re: [PATCH] ci: move external build environment setups to CentOS Stream 9

[Thomas Huth]

On 12/04/2024 12.37, Paolo Bonzini wrote:

RHEL 9 (and thus also the derivatives) are available since two years
now, so according to QEMU's support policy, we can drop the active
support for the previous major version 8 now.

Thus upgrade our CentOS Stream build environment playbooks to major
version 9 now.

Signed-off-by: Paolo Bonzini 
---
  .../stream/{8 => 9}/build-environment.yml | 31 ++---
  .../stream/{8 => 9}/x86_64/configure  |  4 +-
  .../stream/{8 => 9}/x86_64/test-avocado   |  0
  scripts/ci/setup/build-environment.yml| 44 +++
  4 files changed, 34 insertions(+), 45 deletions(-)
  rename scripts/ci/org.centos/stream/{8 => 9}/build-environment.yml (75%)
  rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/configure (98%)
  rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/test-avocado (100%)


Looks sane to me!

Reviewed-by: Thomas Huth 

[PATCH] ci: move external build environment setups to CentOS Stream 9

[Paolo Bonzini]

RHEL 9 (and thus also the derivatives) are available since two years
now, so according to QEMU's support policy, we can drop the active
support for the previous major version 8 now.

Thus upgrade our CentOS Stream build environment playbooks to major
version 9 now.

Signed-off-by: Paolo Bonzini 
---
 .../stream/{8 => 9}/build-environment.yml | 31 ++---
 .../stream/{8 => 9}/x86_64/configure  |  4 +-
 .../stream/{8 => 9}/x86_64/test-avocado   |  0
 scripts/ci/setup/build-environment.yml| 44 +++
 4 files changed, 34 insertions(+), 45 deletions(-)
 rename scripts/ci/org.centos/stream/{8 => 9}/build-environment.yml (75%)
 rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/configure (98%)
 rename scripts/ci/org.centos/stream/{8 => 9}/x86_64/test-avocado (100%)

diff --git a/scripts/ci/org.centos/stream/8/build-environment.yml 
b/scripts/ci/org.centos/stream/9/build-environment.yml
similarity index 75%
rename from scripts/ci/org.centos/stream/8/build-environment.yml
rename to scripts/ci/org.centos/stream/9/build-environment.yml
index 1ead77e2cbf..cd29fe6f275 100644
--- a/scripts/ci/org.centos/stream/8/build-environment.yml
+++ b/scripts/ci/org.centos/stream/9/build-environment.yml
@@ -2,32 +2,32 @@
 - name: Installation of extra packages to build QEMU
   hosts: all
   tasks:
-- name: Extra check for CentOS Stream 8
+- name: Extra check for CentOS Stream 9
   lineinfile:
 path: /etc/redhat-release
-line: CentOS Stream release 8
+line: CentOS Stream release 9
 state: present
   check_mode: yes
-  register: centos_stream_8
+  register: centos_stream_9
 
-- name: Enable EPEL repo on CentOS Stream 8
+- name: Enable EPEL repo on CentOS Stream 9
   dnf:
 name:
   - epel-release
 state: present
   when:
-- centos_stream_8
+- centos_stream_9
 
-- name: Enable PowerTools repo on CentOS Stream 8
+- name: Enable CRB repo on CentOS Stream 9
   ini_file:
-path: /etc/yum.repos.d/CentOS-Stream-PowerTools.repo
-section: powertools
+path: /etc/yum.repos.d/centos.repo
+section: crb
 option: enabled
 value: "1"
   when:
-- centos_stream_8
+- centos_stream_9
 
-- name: Install basic packages to build QEMU on CentOS Stream 8
+- name: Install basic packages to build QEMU on CentOS Stream 9
   dnf:
 name:
   - bzip2
@@ -42,7 +42,6 @@
   - gettext
   - git
   - glib2-devel
-  - glusterfs-api-devel
   - gnutls-devel
   - libaio-devel
   - libcap-ng-devel
@@ -61,22 +60,24 @@
   - lzo-devel
   - make
   - mesa-libEGL-devel
+  - meson
   - nettle-devel
   - ninja-build
   - nmap-ncat
   - numactl-devel
   - pixman-devel
-  - python38
+  - python3
+  - python3-pip
   - python3-sphinx
+  - python3-sphinx_rtd_theme
+  - python3-tomli
   - rdma-core-devel
   - redhat-rpm-config
   - snappy-devel
-  - spice-glib-devel
-  - spice-server-devel
   - systemd-devel
   - systemtap-sdt-devel
   - tar
   - zlib-devel
 state: present
   when:
-- centos_stream_8
+- centos_stream_9
diff --git a/scripts/ci/org.centos/stream/8/x86_64/configure 
b/scripts/ci/org.centos/stream/9/x86_64/configure
similarity index 98%
rename from scripts/ci/org.centos/stream/8/x86_64/configure
rename to scripts/ci/org.centos/stream/9/x86_64/configure
index 76781f17f41..1b6f40fd785 100755
--- a/scripts/ci/org.centos/stream/8/x86_64/configure
+++ b/scripts/ci/org.centos/stream/9/x86_64/configure
@@ -16,7 +16,7 @@
 # that patches adding downstream specific devices are not available.
 #
 ../configure \
---python=/usr/bin/python3.8 \
+--python=/usr/bin/python3.9 \
 --prefix="/usr" \
 --libdir="/usr/lib64" \
 --datadir="/usr/share" \
@@ -157,7 +157,6 @@
 --enable-docs \
 --enable-fdt \
 --enable-gcrypt \
---enable-glusterfs \
 --enable-gnutls \
 --enable-guest-agent \
 --enable-iconv \
@@ -180,7 +179,6 @@
 --enable-seccomp \
 --enable-snappy \
 --enable-smartcard \
---enable-spice \
 --enable-system \
 --enable-tcg \
 --enable-tools \
diff --git a/scripts/ci/org.centos/stream/8/x86_64/test-avocado 
b/scripts/ci/org.centos/stream/9/x86_64/test-avocado
similarity index 100%
rename from scripts/ci/org.centos/stream/8/x86_64/test-avocado
rename to scripts/ci/org.centos/stream/9/x86_64/test-avocado
diff --git a/scripts/ci/setup/build-environment.yml 
b/scripts/ci/setup/build-environment.yml
index f344d1a8509..9b7d96c01b2 100644
--- a/scripts/ci/setup/build-environment.yml
+++ b/scripts/ci/setup/build-environment.yml
@@ -174,26 +174,26 @@
 - ansible_facts['distribution_version'] == '22.04'
 - ansible_facts['architecture'] == 'aarch64'
 
-