Live migration using a specified networking adapter

[Jing-Wei Su]

Hello experts,


I have a network topology like this diagram.

When start live migration moving a VM from Host A to B,

the migration process uses either 10GbE (10.0.0.1) or 1 GbE (10.0.0.2),

but the user cannot specify the source NIC by current migrate command.


To solve the problem, my rough idea is to add a source ipv4:port argument,

the migration command seems like

```

migrate -b tcp:10.0.0.1: -d tcp:10.0.0.3:.

```


Is it an available solution? Or, is there any concern and sugesstion?

Besides the idea, is there any good way to this issue?


   +-+
   | 10GbE switch|
   +-+
   |||
   |||
   |||
  10.0.0.1 |10.0.0.2|10.0.0.3|
   +-+--+-++-+  +--+--+-+
   |  |10GbE NIC | |1GbE NIC||  |  |10GbE NIE | |
   |  +--+ ++|  |  +--+ |
   | |  |   |
   |  +-+|  |   |
   |  |   VM||  |   |
   |  +-+|  |   |
   +-+  +---+
   Host A Host B


Thank you.


Regards,

Derek.

Re: [PATCH v2 9/9] target/riscv: Consolidate RV32/64 16-bit instructions

[Richard Henderson]

On 4/13/21 4:34 PM, Alistair Francis wrote:

This patch removes the insn16-32.decode and insn16-64.decode decode
files and consolidates the instructions into the general RISC-V
insn16.decode decode tree.

This means that all of the instructions are avaliable in both the 32-bit
and 64-bit builds. This also means that we run a check to ensure we are
running a 64-bit softmmu before we execute the 64-bit only instructions.
This allows us to include the 32-bit instructions in the 64-bit build,
while also ensuring that 32-bit only software can not execute the
instructions.

Signed-off-by: Alistair Francis
---


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 8/9] target/riscv: Consolidate RV32/64 32-bit instructions

[Richard Henderson]

On 4/13/21 4:34 PM, Alistair Francis wrote:

-#ifndef CONFIG_USER_ONLY
-# ifdef TARGET_RISCV32
-#  define is_32bit(ctx)  true
-# else
+#ifdef TARGET_RISCV32
+# define is_32bit(ctx)  true
+#else
  static inline bool is_32bit(DisasContext *ctx)
  {
-return !(ctx->misa & RV64);
+return (ctx->misa & RV32) == RV32;


Why the change here?  Also note the previous comment about fixing this to false 
for TARGET_RISCV64 && CONFIG_USER_ONLY.



  static bool trans_fcvt_l_d(DisasContext *ctx, arg_fcvt_l_d *a)
  {
  REQUIRE_FPU;
  REQUIRE_EXT(ctx, RVD);
+REQUIRE_64BIT(ctx);


I think you should always put the 64-bit check first.
That way, on TARGET_RISCV32, the entire function folds away.

  
-TCGv t0 = tcg_temp_new();

+TCGv_i64 t0 = tcg_temp_new_i64();
  gen_set_rm(ctx, a->rm);
  gen_helper_fcvt_l_d(t0, cpu_env, cpu_fpr[a->rs1]);
-gen_set_gpr(a->rd, t0);
-tcg_temp_free(t0);
+gen_set_gpr(a->rd, (TCGv) t0);


So... I really don't like the cast.

This is fixable one of two ways.
(1) Change the real helper to use target_ulong.
(2) Use the gen_helper_* stubs that I talked about in reply to v1.


@@ -390,8 +390,9 @@ static bool trans_fmv_x_d(DisasContext *ctx, arg_fmv_x_d *a)
  {
  REQUIRE_FPU;
  REQUIRE_EXT(ctx, RVD);
+REQUIRE_64BIT(ctx);
  
-gen_set_gpr(a->rd, cpu_fpr[a->rs1]);

+gen_set_gpr(a->rd, (TCGv) cpu_fpr[a->rs1]);


This one's different, and might be worth

#ifdef TARGET_RISCV64
  gen_set_gpr
#else
  qemu_build_not_reached
#endif


+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -303,11 +303,11 @@ static bool trans_fmv_x_w(DisasContext *ctx, arg_fmv_x_w 
*a)
  
  TCGv t0 = tcg_temp_new();
  
-#if defined(TARGET_RISCV64)

-tcg_gen_ext32s_tl(t0, cpu_fpr[a->rs1]);
-#else
-tcg_gen_extrl_i64_i32(t0, cpu_fpr[a->rs1]);
-#endif
+if (!is_32bit(ctx)) {
+tcg_gen_ext32s_tl((TCGv) t0, (TCGv) cpu_fpr[a->rs1]);
+} else {
+tcg_gen_extrl_i64_i32((TCGv_i32) t0, cpu_fpr[a->rs1]);
+}


I think you should leave this ifdef alone.  The ifdef has determined the size 
of target_ulong and thus the size of TCGv, and thus the correct move to use.


If TARGET_RISCV64 and is_32bit, the high bits are ignored; the fact that they 
happen to be copies of the sign bit is irrelevant.



r~

Re: [PATCH v1 3/3] target/ppc: Add POWER10 exception model

[Nicholas Piggin]

Excerpts from Fabiano Rosas's message of April 14, 2021 1:53 am:
> Nicholas Piggin  writes:
> 
>> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
>> and it removes support for the LPCR[AIL]=0b10 mode.
>>
>> Signed-off-by: Nicholas Piggin 
>> ---

[snip]

Thanks for the suggestions, I will consider them if people prefer to go 
this way rather than the "cleanup first" approach in the RFC I just 
posted.

Thanks,
Nick

[RFC PATCH 2/2] target/ppc: Add POWER10 exception model

[Nicholas Piggin]

POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
and it removes support for the LPCR[AIL]=0b10 mode.

Signed-off-by: Nicholas Piggin 
---
 hw/ppc/spapr_hcall.c|  7 +-
 target/ppc/cpu-qom.h|  2 ++
 target/ppc/cpu.h|  5 ++--
 target/ppc/excp_helper.c| 43 +
 target/ppc/translate.c  |  3 ++-
 target/ppc/translate_init.c.inc |  2 +-
 6 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 2fbe04a689..6802cd4dc8 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1396,7 +1396,12 @@ static target_ulong 
h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
 }
 
 if (mflags == 1) {
-/* AIL=1 is reserved */
+/* AIL=1 is reserved in POWER8/POWER9 */
+return H_UNSUPPORTED_FLAG;
+}
+
+if (mflags == 2 && (pcc->insns_flags2 & PPC2_ISA310)) {
+/* AIL=2 is also reserved in POWER10 (ISA v3.1) */
 return H_UNSUPPORTED_FLAG;
 }
 
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 118baf8d41..06b6571bc9 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -116,6 +116,8 @@ enum powerpc_excp_t {
 POWERPC_EXCP_POWER8,
 /* POWER9 exception model   */
 POWERPC_EXCP_POWER9,
+/* POWER10 exception model   */
+POWERPC_EXCP_POWER10,
 };
 
 /*/
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5200a16d23..9d35cdfa92 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,10 +354,11 @@ typedef struct ppc_v3_pate_t {
 #define LPCR_PECE_U_SHIFT (63 - 19)
 #define LPCR_PECE_U_MASK  (0x7ull << LPCR_PECE_U_SHIFT)
 #define LPCR_HVEE PPC_BIT(17) /* Hypervisor Virt Exit Enable */
-#define LPCR_RMLS_SHIFT   (63 - 37)
+#define LPCR_RMLS_SHIFT   (63 - 37)   /* RMLS (removed in ISA v3.0) */
 #define LPCR_RMLS (0xfull << LPCR_RMLS_SHIFT)
+#define LPCR_HAIL PPC_BIT(37) /* ISA v3.1 HV AIL=3 equivalent */
 #define LPCR_ILE  PPC_BIT(38)
-#define LPCR_AIL_SHIFT(63 - 40)  /* Alternate interrupt location */
+#define LPCR_AIL_SHIFT(63 - 40)   /* Alternate interrupt location */
 #define LPCR_AIL  (3ull << LPCR_AIL_SHIFT)
 #define LPCR_UPRT PPC_BIT(41) /* Use Process Table */
 #define LPCR_EVIRTPPC_BIT(42) /* Enhanced Virtualisation */
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 9ff316767c..19931361a0 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -172,6 +172,26 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState 
*env, int excp,
  *
  * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be
  * sent to the hypervisor in AIL mode if the guest is radix (LPCR[HR]=1).
+ * This is good for performance but allows the guest to influence the
+ * AIL of hypervisor interrupts using its MSR, and also the hypervisor
+ * must disallow guest interrupts (MSR[HV] 0->0) from using AIL if the
+ * hypervisor does not want to use AIL for its MSR[HV] 0->1 interrupts.
+ *
+ * POWER10 addresses those issues with a new LPCR[HAIL] bit that is
+ * applied to interrupt that begin execution with MSR[HV]=1 (so both
+ * MSR[HV] 0->1 and 1->1).
+ *
+ * HAIL=1 is equivalent to AIL=3, for interrupts delivered with MSR[HV]=1.
+ *
+ * POWER10 behaviour is
+ * | LPCR[AIL] | LPCR[HAIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
+ * +---++-+-+-+-+
+ * | a | h  | 00/01/10| 0   | 0   | 0   |
+ * | a | h  | 11  | 0   | 0   | a   |
+ * | a | h  | x   | 0   | 1   | h   |
+ * | a | h  | 00/01/10| 1   | 1   | 0   |
+ * | a | h  | 11  | 1   | 1   | h   |
+ * ++
  */
 static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int 
excp,
   target_ulong msr,
@@ -211,6 +231,29 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int 
excp_model, int excp,
 /* AIL=1 is reserved */
 return;
 }
+
+} else if (excp_model == POWERPC_EXCP_POWER10) {
+if (!mmu_all_on && !hv_escalation) {
+/*
+ * AIL works for HV interrupts even with guest MSR[IR/DR] disabled.
+ * Guest->guest and HV->HV interrupts do require MMU on.
+ */
+return;
+}
+
+if (*new_msr & MSR_HVB) {
+if (!(env->spr[SPR_LPCR] & LPCR_HAIL)) {
+/* HV interrupts depend on LPCR[HAIL] */
+return;
+}
+ail = 3; /* HAIL=1 gives AIL=3 behaviour for HV interrupts */
+} else {
+ail = 

Re: [PATCH v1 1/3] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour

[Nicholas Piggin]

Excerpts from Fabiano Rosas's message of April 13, 2021 11:48 pm:
> Nicholas Piggin  writes:
> 
>> ISA v3.0 radix guest execution has a quirk in AIL behaviour such that
>> the LPCR[AIL] value can apply to hypervisor interrupts.
>>
>> This affects machines that emulate HV=1 mode (i.e., powernv9).
>>
> 
> Ah ok, so we actually want to replicate the quirk in the pnv
> machine. Took me a while.

Yes. Quirk is probably the wrong word for me to use. It's architected 
behaviour so it must be implemented, it's just slightly surprising /
easy to miss.

> 
> Reviewed-by: Fabiano Rosas 

Thanks,
Nick

> 
>> Signed-off-by: Nicholas Piggin 
>> ---
>>  target/ppc/excp_helper.c | 17 +
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 85de7e6c90..b8881c0f85 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -791,14 +791,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>> excp_model, int excp)
>>  #endif
>>  
>>  /*
>> - * AIL only works if there is no HV transition and we are running
>> - * with translations enabled
>> + * AIL only works if MSR[IR] and MSR[DR] are both enabled.
>>   */
>> -if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1) ||
>> -((new_msr & MSR_HVB) && !(msr & MSR_HVB))) {
>> +if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
>>  ail = 0;
>>  }
>>  
>> +/*
>> + * AIL does not work if there is a MSR[HV] 0->1 transition and the
>> + * partition is in HPT mode. For radix guests, such interrupts are
>> + * allowed to be delivered to the hypervisor in ail mode.
>> + */
>> +if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
>> +if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
>> +ail = 0;
>> +}
>> +}
>> +
>>  vector = env->excp_vectors[excp];
>>  if (vector == (target_ulong)-1ULL) {
>>  cpu_abort(cs, "Raised an exception without defined vector %d\n",
> 

[RFC PATCH 1/2] target/ppc: rework AIL logic in interrupt delivery

[Nicholas Piggin]

The AIL logic is becoming unmanageable spread all over powerpc_excp(),
and it is slated to get even worse with POWER10 support.

Move it all to a new helper function.

Signed-off-by: Nicholas Piggin 
---
 hw/ppc/spapr_hcall.c|   3 +-
 target/ppc/cpu.h|   8 --
 target/ppc/excp_helper.c| 161 
 target/ppc/translate_init.c.inc |   2 +-
 4 files changed, 104 insertions(+), 70 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 7b5cd3553c..2fbe04a689 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1395,7 +1395,8 @@ static target_ulong 
h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
 return H_P4;
 }
 
-if (mflags == AIL_RESERVED) {
+if (mflags == 1) {
+/* AIL=1 is reserved */
 return H_UNSUPPORTED_FLAG;
 }
 
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e73416da68..5200a16d23 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2375,14 +2375,6 @@ enum {
 HMER_XSCOM_STATUS_MASK  = PPC_BITMASK(21, 23),
 };
 
-/* Alternate Interrupt Location (AIL) */
-enum {
-AIL_NONE= 0,
-AIL_RESERVED= 1,
-AIL_0001_8000   = 2,
-AIL_C000___4000 = 3,
-};
-
 /*/
 
 #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index b8881c0f85..9ff316767c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -136,25 +136,107 @@ static int powerpc_reset_wakeup(CPUState *cs, 
CPUPPCState *env, int excp,
 return POWERPC_EXCP_RESET;
 }
 
-static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail)
+/*
+ * AIL - Alternate Interrupt Location, a mode that allows interrupts to be
+ * taken with the MMU on, and which uses an alternate location (e.g., so the
+ * kernel/hv can map the vectors there with an effective address).
+ *
+ * An interrupt is considered to be taken "with AIL" or "AIL applies" if they
+ * are delivered in this way. AIL requires the LPCR to be set to enable this
+ * mode, and a number of conditions have to be true for AIL to apply.
+ *
+ * First of all, SRESET, MCE, and HMI are always delivered without AIL,
+ * because they are specifically want to be in real mode (e.g., MCE might
+ * be signaling a SLB multi-hit which requires SLB flush before the MMU can
+ * be enabled).
+ *
+ * After that, behaviour depends on the current MSR[IR], MSR[DR], MSR[HV], and
+ * whether or not the interrupt changes MSR[HV] from 0 to 1, and the current
+ * radix mode (LPCR[HR]).
+ *
+ * POWER8, POWER9 with LPCR[HR]=0
+ * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
+ * +---+-+-+-+-+
+ * | a | 00/01/10| x   | x   | 0   |
+ * | a | 11  | 0   | 1   | 0   |
+ * | a | 11  | 1   | 1   | a   |
+ * | a | 11  | 0   | 0   | a   |
+ * +---+
+ *
+ * POWER9 with LPCR[HR]=1
+ * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
+ * +---+-+-+-+-+
+ * | a | 00/01/10| x   | x   | 0   |
+ * | a | 11  | x   | x   | a   |
+ * +---+
+ *
+ * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be
+ * sent to the hypervisor in AIL mode if the guest is radix (LPCR[HR]=1).
+ */
+static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int 
excp,
+  target_ulong msr,
+  target_ulong *new_msr,
+  target_ulong *vector)
 {
-uint64_t offset = 0;
+#if defined(TARGET_PPC64)
+CPUPPCState *env = >env;
+bool mmu_all_on = ((msr >> MSR_IR) & 1) && ((msr >> MSR_DR) & 1);
+bool hv_escalation = !(msr & MSR_HVB) && (*new_msr & MSR_HVB);
+int ail = 0;
+
+if (excp == POWERPC_EXCP_MCHECK ||
+excp == POWERPC_EXCP_RESET ||
+excp == POWERPC_EXCP_HV_MAINT) {
+/* SRESET, MCE, HMI never apply AIL */
+return;
+}
 
-switch (ail) {
-case AIL_NONE:
-break;
-case AIL_0001_8000:
-offset = 0x18000;
-break;
-case AIL_C000___4000:
-offset = 0xc0004000ull;
-break;
-default:
-cpu_abort(cs, "Invalid AIL combination %d\n", ail);
-break;
+if (excp_model == POWERPC_EXCP_POWER8 ||
+excp_model == POWERPC_EXCP_POWER9) {
+if (!mmu_all_on) {
+/* AIL only works if MSR[IR] and MSR[DR] are both enabled. */
+return;
+}
+if (hv_escalation && !(env->spr[SPR_LPCR] & LPCR_HR)) {
+/*
+ * AIL does not 

[RFC PATCH 0/2] ppc: rework AIL logic, add POWER10 exception model

[Nicholas Piggin]

This applies on top of patches 1,2 from the previous series (i.e., these
two patches replace patch 3).

Function should be the same, but this way seems much cleaner. It does
include a "cleanup" patch before the POWER10 fix, but arguably this is
a better way to go even as a bug fix (backport, etc).

Comments, opinions?

Thanks,
Nick

Nicholas Piggin (2):
  target/ppc: rework AIL logic in interrupt delivery
  target/ppc: Add POWER10 exception model

 hw/ppc/spapr_hcall.c|   8 +-
 target/ppc/cpu-qom.h|   2 +
 target/ppc/cpu.h|  13 +-
 target/ppc/excp_helper.c| 204 ++--
 target/ppc/translate.c  |   3 +-
 target/ppc/translate_init.c.inc |   4 +-
 6 files changed, 160 insertions(+), 74 deletions(-)

-- 
2.23.0

Re: [PATCH v2 5/9] target/riscv: Remove the hardcoded SATP_MODE macro

[Richard Henderson]

On 4/13/21 4:34 PM, Alistair Francis wrote:

Signed-off-by: Alistair Francis
---
  target/riscv/cpu_bits.h   | 11 ---
  target/riscv/cpu_helper.c | 24 ++--
  target/riscv/csr.c| 20 
  target/riscv/monitor.c| 22 +-
  4 files changed, 51 insertions(+), 26 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2 4/9] target/riscv: Remove the hardcoded MSTATUS_SD macro

[Richard Henderson]

On 4/13/21 4:33 PM, Alistair Francis wrote:

+#ifndef CONFIG_USER_ONLY
+# ifdef TARGET_RISCV32
+#  define is_32bit(ctx)  true
+# else
+static inline bool is_32bit(DisasContext *ctx)
+{
+return !(ctx->misa & RV64);
+}
+# endif
+#endif


It's going to be soon enough when this is used by user-only too.
I'd structure this as

#ifdef TARGET_RISCV32
# define is_32bit(ctx)  true
#elif defined(CONFIG_USER_ONLY)
# define is_32bit(ctx)  false
#else
static inline...
#endif


  tmp = tcg_temp_new();
+sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
+
+
  tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));


Careful with the extra whitespace.

Reviewed-by: Richard Henderson 


r~

[Bug 1923693] [NEW] Lack of architecture in gdbstub makes debugging confusing

[kallisti5]

Public bug reported:

I spent some quality time debugging GEF and came to a conclusion here:
https://github.com/hugsy/gef/issues/598#issuecomment-819174169

tldr;

* gdb_arch_name was undefined on riscv
* this bug was fixed recently via 
https://github.com/qemu/qemu/commit/edf647864bdab84ed4b1a4f47ea05be6bb075c69


* An undefined gdb_arch_name results in qemu's gdbstub omitting the 
 xml.
* gdb translates a missing  as "auto" which breaks a lot of stuff.
* tracking down where "auto" comes from is a bit confusing and time consuming.


It might be better to report a missing / blank gdb_arch_name as 
"unknown" instead of omitting the block completely.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1923693

Title:
  Lack of architecture in gdbstub makes debugging confusing

Status in QEMU:
  New

Bug description:
  I spent some quality time debugging GEF and came to a conclusion here:
  https://github.com/hugsy/gef/issues/598#issuecomment-819174169

  tldr;

  * gdb_arch_name was undefined on riscv
  * this bug was fixed recently via 
https://github.com/qemu/qemu/commit/edf647864bdab84ed4b1a4f47ea05be6bb075c69

  
  * An undefined gdb_arch_name results in qemu's gdbstub omitting the 
 xml.
  * gdb translates a missing  as "auto" which breaks a lot of 
stuff.
  * tracking down where "auto" comes from is a bit confusing and time consuming.

  
  It might be better to report a missing / blank gdb_arch_name as 
"unknown" instead of omitting the block completely.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1923693/+subscriptions

[Bug 1923692] [NEW] qemu 5.2.0: Add reconnect option support for netdev socket

[Mark Karpelès]

Public bug reported:

Most of qemu socket accepting options (such as chardev) accept among
other things a "reconnect" option.

netdev socket however returns: Invalid parameter 'reconnect'

It would make sense that available options for socket links be at least
partially normalized (also see issue
https://bugs.launchpad.net/qemu/+bug/1903470 which was closed without
resolution).

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1923692

Title:
  qemu 5.2.0: Add reconnect option support for netdev socket

Status in QEMU:
  New

Bug description:
  Most of qemu socket accepting options (such as chardev) accept among
  other things a "reconnect" option.

  netdev socket however returns: Invalid parameter 'reconnect'

  It would make sense that available options for socket links be at
  least partially normalized (also see issue
  https://bugs.launchpad.net/qemu/+bug/1903470 which was closed without
  resolution).

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1923692/+subscriptions

Re: [PATCH v1 3/3] target/ppc: Add POWER10 exception model

[Nicholas Piggin]

Excerpts from Cédric Le Goater's message of April 14, 2021 3:09 am:
> On 4/13/21 5:53 PM, Fabiano Rosas wrote:
>> Nicholas Piggin  writes:
>> 
>>> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
>>> and it removes support for the LPCR[AIL]=0b10 mode.
>>>
>>> Signed-off-by: Nicholas Piggin 
>>> ---
>>>  hw/ppc/spapr_hcall.c|   5 ++
>>>  target/ppc/cpu-qom.h|   2 +
>>>  target/ppc/cpu.h|   5 +-
>>>  target/ppc/excp_helper.c| 114 ++--
>>>  target/ppc/translate.c  |   3 +-
>>>  target/ppc/translate_init.c.inc |   2 +-
>>>  6 files changed, 107 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>>> index 7b5cd3553c..2f65f20f72 100644
>>> --- a/hw/ppc/spapr_hcall.c
>>> +++ b/hw/ppc/spapr_hcall.c
>>> @@ -1399,6 +1399,11 @@ static target_ulong 
>>> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>>>  return H_UNSUPPORTED_FLAG;
>>>  }
>>>  
>>> +if (mflags == AIL_0001_8000 && (pcc->insns_flags2 & PPC2_ISA310)) {
>>> +/* AIL 2 is also reserved in ISA v3.1 */
>>> +return H_UNSUPPORTED_FLAG;
>>> +}
>>> +
>>>  spapr_set_all_lpcrs(mflags << LPCR_AIL_SHIFT, LPCR_AIL);
>>>  
>>>  return H_SUCCESS;
>>> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
>>> index 118baf8d41..06b6571bc9 100644
>>> --- a/target/ppc/cpu-qom.h
>>> +++ b/target/ppc/cpu-qom.h
>>> @@ -116,6 +116,8 @@ enum powerpc_excp_t {
>>>  POWERPC_EXCP_POWER8,
>>>  /* POWER9 exception model   */
>>>  POWERPC_EXCP_POWER9,
>>> +/* POWER10 exception model   */
>>> +POWERPC_EXCP_POWER10,
>>>  };
>>>  
>>>  
>>> /*/
>>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>>> index e73416da68..24e53e0ac3 100644
>>> --- a/target/ppc/cpu.h
>>> +++ b/target/ppc/cpu.h
>>> @@ -354,10 +354,11 @@ typedef struct ppc_v3_pate_t {
>>>  #define LPCR_PECE_U_SHIFT (63 - 19)
>>>  #define LPCR_PECE_U_MASK  (0x7ull << LPCR_PECE_U_SHIFT)
>>>  #define LPCR_HVEE PPC_BIT(17) /* Hypervisor Virt Exit Enable */
>>> -#define LPCR_RMLS_SHIFT   (63 - 37)
>>> +#define LPCR_RMLS_SHIFT   (63 - 37)   /* RMLS (removed in ISA v3.0) */
>>>  #define LPCR_RMLS (0xfull << LPCR_RMLS_SHIFT)
>>> +#define LPCR_HAIL PPC_BIT(37) /* ISA v3.1 HV AIL=3 equivalent */
>>>  #define LPCR_ILE  PPC_BIT(38)
>>> -#define LPCR_AIL_SHIFT(63 - 40)  /* Alternate interrupt location */
>>> +#define LPCR_AIL_SHIFT(63 - 40)   /* Alternate interrupt location */
>>>  #define LPCR_AIL  (3ull << LPCR_AIL_SHIFT)
>>>  #define LPCR_UPRT PPC_BIT(41) /* Use Process Table */
>>>  #define LPCR_EVIRTPPC_BIT(42) /* Enhanced Virtualisation */
>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>> index b8881c0f85..e8bf0598b4 100644
>>> --- a/target/ppc/excp_helper.c
>>> +++ b/target/ppc/excp_helper.c
>>> @@ -197,7 +197,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>  CPUState *cs = CPU(cpu);
>>>  CPUPPCState *env = >env;
>>>  target_ulong msr, new_msr, vector;
>>> -int srr0, srr1, asrr0, asrr1, lev = -1, ail;
>>> +int srr0, srr1, asrr0, asrr1, lev = -1;
>>> +int ail, hail, using_ail;
>>>  bool lpes0;
>>>  
>>>  qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
>>> @@ -239,24 +240,39 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>>> excp_model, int excp)
>>>   * On anything else, we behave as if LPES0 is 1
>>>   * (externals don't alter MSR:HV)
>>>   *
>>> - * AIL is initialized here but can be cleared by
>>> - * selected exceptions
>>> + * ail/hail are initialized here but can be cleared by
>>> + * selected exceptions, and other conditions afterwards.
>>>   */
>>>  #if defined(TARGET_PPC64)
>>>  if (excp_model == POWERPC_EXCP_POWER7 ||
>>>  excp_model == POWERPC_EXCP_POWER8 ||
>>> -excp_model == POWERPC_EXCP_POWER9) {
>>> +excp_model == POWERPC_EXCP_POWER9 ||
>>> +excp_model == POWERPC_EXCP_POWER10) {
>>>  lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>>>  if (excp_model != POWERPC_EXCP_POWER7) {
>>>  ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
>>>  } else {
>>>  ail = 0;
>>>  }
>>> +if (excp_model == POWERPC_EXCP_POWER10) {
>>> +if (AIL_0001_8000) {
>> 
>> if (2)
>> 
>>> +ail = 0; /* AIL=2 is reserved in ISA v3.1 */
>>> +}
>>> +
>>> +if (env->spr[SPR_LPCR] & LPCR_HAIL) {
>>> +hail = AIL_C000___4000;
>>> +} else {
>>> +hail = 0;
>>> +}
>>> +} else {
>>> +hail = ail;
>> 
>> I think it's better if we set hail = 0 here. Read on...
>> 
>>> +}
>>>  } else
>>>  #endif /* 

Re: [RFC v9 15/29] vfio: Set up nested stage mappings

[Kunkun Jiang]

On 2021/4/13 20:57, Auger Eric wrote:

Hi Kunkun,

On 4/13/21 2:10 PM, Kunkun Jiang wrote:

Hi Eric,

On 2021/4/11 20:08, Eric Auger wrote:

In nested mode, legacy vfio_iommu_map_notify cannot be used as
there is no "caching" mode and we do not trap on map.

On Intel, vfio_iommu_map_notify was used to DMA map the RAM
through the host single stage.

With nested mode, we need to setup the stage 2 and the stage 1
separately. This patch introduces a prereg_listener to setup
the stage 2 mapping.

The stage 1 mapping, owned by the guest, is passed to the host
when the guest invalidates the stage 1 configuration, through
a dedicated PCIPASIDOps callback. Guest IOTLB invalidations
are cascaded downto the host through another IOMMU MR UNMAP
notifier.

Signed-off-by: Eric Auger 

---

v7 -> v8:
- properly handle new IOMMUTLBEntry fields and especially
    propagate DOMAIN and PASID based invalidations

v6 -> v7:
- remove PASID based invalidation

v5 -> v6:
- add error_report_err()
- remove the abort in case of nested stage case

v4 -> v5:
- use VFIO_IOMMU_SET_PASID_TABLE
- use PCIPASIDOps for config notification

v3 -> v4:
- use iommu_inv_pasid_info for ASID invalidation

v2 -> v3:
- use VFIO_IOMMU_ATTACH_PASID_TABLE
- new user API
- handle leaf

v1 -> v2:
- adapt to uapi changes
- pass the asid
- pass IOMMU_NOTIFIER_S1_CFG when initializing the config notifier
---
   hw/vfio/common.c | 139 +--
   hw/vfio/pci.c    |  21 +++
   hw/vfio/trace-events |   2 +
   3 files changed, 157 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 0cd7ef2139..e369d451e7 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -595,6 +595,73 @@ static bool vfio_get_xlat_addr(IOMMUTLBEntry
*iotlb, void **vaddr,
   return true;
   }
   +/* Propagate a guest IOTLB invalidation to the host (nested mode) */
+static void vfio_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry
*iotlb)
+{
+    VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
+    struct vfio_iommu_type1_cache_invalidate ustruct = {};
+    VFIOContainer *container = giommu->container;
+    int ret;
+
+    assert(iotlb->perm == IOMMU_NONE);
+
+    ustruct.argsz = sizeof(ustruct);
+    ustruct.flags = 0;
+    ustruct.info.argsz = sizeof(struct iommu_cache_invalidate_info);
+    ustruct.info.version = IOMMU_CACHE_INVALIDATE_INFO_VERSION_1;
+    ustruct.info.cache = IOMMU_CACHE_INV_TYPE_IOTLB;
+
+    switch (iotlb->granularity) {
+    case IOMMU_INV_GRAN_DOMAIN:
+    ustruct.info.granularity = IOMMU_INV_GRANU_DOMAIN;
+    break;
+    case IOMMU_INV_GRAN_PASID:
+    {
+    struct iommu_inv_pasid_info *pasid_info;
+    int archid = -1;
+
+    pasid_info = _info;
+    ustruct.info.granularity = IOMMU_INV_GRANU_PASID;
+    if (iotlb->flags & IOMMU_INV_FLAGS_ARCHID) {
+    pasid_info->flags |= IOMMU_INV_ADDR_FLAGS_ARCHID;
+    archid = iotlb->arch_id;
+    }
+    pasid_info->archid = archid;
+    trace_vfio_iommu_asid_inv_iotlb(archid);
+    break;
+    }
+    case IOMMU_INV_GRAN_ADDR:
+    {
+    hwaddr start = iotlb->iova + giommu->iommu_offset;
+    struct iommu_inv_addr_info *addr_info;
+    size_t size = iotlb->addr_mask + 1;
+    int archid = -1;
+
+    addr_info = _info;
+    ustruct.info.granularity = IOMMU_INV_GRANU_ADDR;
+    if (iotlb->leaf) {
+    addr_info->flags |= IOMMU_INV_ADDR_FLAGS_LEAF;
+    }
+    if (iotlb->flags & IOMMU_INV_FLAGS_ARCHID) {
+    addr_info->flags |= IOMMU_INV_ADDR_FLAGS_ARCHID;
+    archid = iotlb->arch_id;
+    }
+    addr_info->archid = archid;
+    addr_info->addr = start;
+    addr_info->granule_size = size;
+    addr_info->nb_granules = 1;
+    trace_vfio_iommu_addr_inv_iotlb(archid, start, size,
+    1, iotlb->leaf);
+    break;
+    }

Should we pass a size to  host kernel here, even if vSMMU doesn't support
RIL or guest kernel doesn't use RIL?

It will cause TLBI issue in  this scenario: Guest kernel issues a TLBI cmd
without "range" (tg = 0) to invalidate a 2M huge page. Then qemu passed
the iova and size (4K) to host kernel. Finally, host kernel issues a
TLBI cmd
with "range" (4K) which can not invalidate the TLB entry of 2M huge page.
(pSMMU supports RIL)

In that case the guest will loop over all 4K images belonging to the 2M
huge page and invalidate each of them. This should turn into qemu
notifications for each 4kB page, no? This is totally inefficient, hence

The guest will not loop over all 4K images belonging to the 2M huge page.
The iommu_iotlb_gather->pgsize will be 2M, if a page is 2M huge page. The
gather->pgsize will be passed to __arm_smmu_tlb_inv_range as "granule":

iommu_iotlb_gather_add_page
    iommu_iotlb_sync
        domain->ops->iotlb_sync
            arm_smmu_iotlb_sync
            arm_smmu_tlb_inv_range_domain
                    

[Bug 1923689] [NEW] sig-abort / coredump observed from aio_ctx_finalize

[Eric]

Public bug reported:

Observing occasional sig-abort based on v5.2.0 (tag) of QEMU. The VMM is
configured for Kata use case, launching with a nvdimm/pmem based rootfs,
and a set of workloads which are heavily utilizing virtio-fs.

Sample qemu-cmdline:
/usr/bin/qemu-kata-system-x86_64
-name sandbox-9dc314445bbb2cd02e6d30126ea8355a4f8acd36c866ea32171486931dc2b99c
-uuid cd58d78d-ad44-4d26-9eab-66efab3fb23b
-machine pc,accel=kvm,kernel_irqchip,nvdimm=on
-cpu host,pmu=off
-qmp 
unix:/run/vc/vm/9dc314445bbb2cd02e6d30126ea8355a4f8acd36c866ea32171486931dc2b99c/qmp.sock,server,nowait
-m 2048M,slots=10,maxmem=386381M
-device 
pci-bridge,bus=pci.0,id=pci-bridge-0,chassis_nr=1,shpc=on,addr=2,romfile=
-device virtio-serial-pci,disable-modern=false,id=serial0,romfile=,max_ports=2
-device virtconsole,chardev=charconsole0,id=console0
-chardev 
socket,id=charconsole0,path=/run/vc/vm/9dc314445bbb2cd02e6d30126ea8355a4f8acd36c866ea32171486931dc2b99c/console.sock,server,nowait
-device nvdimm,id=nv0,memdev=mem0
-object 
memory-backend-file,id=mem0,mem-path=/usr/share/kata-containers/kata-containers.img,size=536870912
-object rng-random,id=rng0,filename=/dev/urandom
-device virtio-rng-pci,rng=rng0,romfile=
-device 
vhost-vsock-pci,disable-modern=false,vhostfd=3,id=vsock-3054067214,guest-cid=3054067214,romfile=
-chardev 
socket,id=char-770bb156466e8ed5,path=/run/vc/vm/9dc314445bbb2cd02e6d30126ea8355a4f8acd36c866ea32171486931dc2b99c/vhost-fs.sock
-device vhost-user-fs-pci,chardev=char-770bb156466e8ed5,tag=kataShared,romfile=
-netdev tap,id=network-0,vhost=on,vhostfds=4,fds=5
-device 
driver=virtio-net-pci,netdev=network-0,mac=9e:ad:0c:d1:58:e0,disable-modern=false,mq=on,vectors=4,romfile=
-rtc base=utc,driftfix=slew,clock=host
-global kvm-pit.lost_tick_policy=discard
-vga none
-no-user-config
-nodefaults
-nographic
--no-reboot
-daemonize
-object memory-backend-file,id=dimm1,size=2048M,mem-path=/dev/shm,share=on
-numa node,memdev=dimm1
-kernel /usr/share/kata-containers/vmlinuz
-append tsc=reliable no_timer_check rcupdate.rcu_expedited=1 i8042.direct=1 
i8042.dumbkbd=1 i8042.nopnp=1 i8042.noaux=1 noreplace-smp reboot=k console=hvc0 
console=hvc1 cryptomgr.notests net.ifnames=0 pci=lastbus=0 root=/dev/pmem0p1 
rootflags=dax,data=ordered,errors=remount-ro ro rootfstype=ext4 quiet 
systemd.show_status=false panic=1 nr_cpus=32 
systemd.unit=kata-containers.target systemd.mask=systemd-networkd.service 
systemd.mask=systemd-networkd.socket
-pidfile 
/run/vc/vm/9dc314445bbb2cd02e6d30126ea8355a4f8acd36c866ea32171486931dc2b99c/pid
-smp 1,cores=1,threads=1,sockets=32,maxcpus=32

>From the core file I was able to obtain a backtrace:

```
(gdb) info thread
  Id   Target Id Frame
  6Thread 0x7f92feffd700 (LWP 14678) 0x7f93b23a0a35 in 
pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
  5Thread 0x7f92f700 (LWP 13860) 0x7f93b23a0a35 in 
pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
  4Thread 0x7f930dcff700 (LWP 13572) 0x7f93b23a0a35 in 
pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
  3Thread 0x7f92ff7fe700 (LWP 14179) 0x7f93b23a0a35 in 
pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
  2Thread 0x7f93aed03700 (LWP 13565) 0x7f93b20bfd19 in syscall () from 
/lib64/libc.so.6
* 1Thread 0x7f93c718dcc0 (LWP 13564) 0x7f93b1ffd3d7 in raise () from 
/lib64/libc.so.6
(gdb) bt trace
No symbol table is loaded.  Use the "file" command.
(gdb) bt
#0  0x7f93b1ffd3d7 in raise () from /lib64/libc.so.6
#1  0x7f93b1ffeac8 in abort () from /lib64/libc.so.6
#2  0x7f93b1ff61a6 in __assert_fail_base () from /lib64/libc.so.6
#3  0x7f93b1ff6252 in __assert_fail () from /lib64/libc.so.6
#4  0x007c6955 in aio_ctx_finalize ()
#5  0x7f93c64223d1 in g_source_unref_internal () from 
/lib64/libglib-2.0.so.0
#6  0x7f93c64225f5 in g_source_iter_next () from /lib64/libglib-2.0.so.0
#7  0x7f93c642362d in g_main_context_unref () from /lib64/libglib-2.0.so.0
#8  0x7f93c6425628 in g_main_loop_unref () from /lib64/libglib-2.0.so.0
#9  0x006dbaa0 in iothread_instance_finalize ()
#10 0x006c01e9 in object_unref ()
#11 0x006be647 in object_property_del_child ()
#12 0x0075ad79 in monitor_cleanup ()
#13 0x00630635 in qemu_cleanup ()
#14 0x0040fed3 in main ()
```

I *think* we're hitting this assert: 
https://github.com/qemu/qemu/blob/master/util/async.c#L339 based on 
```
(gdb) up
#4  0x007c6955 in aio_ctx_finalize ()
```

The error is relatively infrequent, but a catastrophic core dump none
the less.

Please let me know if there's more I can pull from the core, or more
info I can share to help facilitate debugging this error.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1923689

Title:
  sig-abort / coredump observed 

Re: [PATCH] decodetree: Allow custom var width load functions

[Richard Henderson]

On 4/13/21 11:16 AM, Luis Pires wrote:

This is useful in situations where you want decodetree
to handle variable width instructions but you want to
provide custom code to load the instructions. Suppressing
the generation of the load function is necessary to avoid
compilation errors due to the load function being unused.

This will be used by the PowerPC decodetree code.

Signed-off-by: Luis Pires
---
  scripts/decodetree.py | 18 +++---
  1 file changed, 11 insertions(+), 7 deletions(-)


So, in the previous cases where we have very controlled "variable length", 
we've simply created separate decode files each with fixed length.


E.g. for arm thumb, in thumb_tr_translate_insn:

insn = arm_lduw_code(env, dc->base.pc_next, dc->sctlr_b);
is_16bit = thumb_insn_is_16bit(dc, dc->base.pc_next, insn);
...
if (is_16bit) {
disas_thumb_insn(dc, insn);
} else {
disas_thumb2_insn(dc, insn);
}

Similarly, riscv's decode_opc.

I would think that Power would work the same, with 32-bit and 64-bit 
instructions.  I'd like you to talk me through what you want to do here.



r~

Re: [PATCH v2] target/s390x: Fix translation exception on illegal instruction

[Richard Henderson]

On 4/13/21 9:52 AM, Ilya Leoshkevich wrote:

Hitting an uretprobe in a s390x TCG guest causes a SIGSEGV. What
happens is:

* uretprobe maps a userspace page containing an invalid instruction.
* uretprobe replaces the target function's return address with the
   address of that page.
* When tb_gen_code() is called on that page, tb->size ends up being 0
   (because the page starts with the invalid instruction), which causes
   virt_page2 to point to the previous page.
* The previous page is not mapped, so this causes a spurious
   translation exception.

The bug is that tb->size must never be 0: even if there is an illegal
instruction, the instruction bytes that have been looked at must count
towards tb->size. So adjust s390x's translate_one() to act this way
for both illegal instructions and instructions that are known to
generate exceptions.

Also add an assertion to tb_gen_code() in order to detect such
situations in future.

Signed-off-by: Ilya Leoshkevich
---


Reviewed-by: Richard Henderson 

r~

[PATCH v2 7/9] target/riscv: Remove an unused CASE_OP_32_64 macro

[Alistair Francis]

Signed-off-by: Alistair Francis 
Reviewed-by: Richard Henderson 
---
 target/riscv/translate.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 74636b9db7..ba8fb2cda3 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -67,12 +67,6 @@ typedef struct DisasContext {
 CPUState *cs;
 } DisasContext;
 
-#ifdef TARGET_RISCV64
-#define CASE_OP_32_64(X) case X: case glue(X, W)
-#else
-#define CASE_OP_32_64(X) case X
-#endif
-
 static inline bool has_ext(DisasContext *ctx, uint32_t ext)
 {
 return ctx->misa & ext;
-- 
2.31.1

[PATCH v2 4/9] target/riscv: Remove the hardcoded MSTATUS_SD macro

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_bits.h  | 10 --
 target/riscv/csr.c   | 12 ++--
 target/riscv/translate.c | 20 ++--
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 8caab23b62..dd643d0f63 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -387,16 +387,6 @@
 #define MXL_RV642
 #define MXL_RV128   3
 
-#if defined(TARGET_RISCV32)
-#define MSTATUS_SD MSTATUS32_SD
-#define MISA_MXL MISA32_MXL
-#define MXL_VAL MXL_RV32
-#elif defined(TARGET_RISCV64)
-#define MSTATUS_SD MSTATUS64_SD
-#define MISA_MXL MISA64_MXL
-#define MXL_VAL MXL_RV64
-#endif
-
 /* sstatus CSR bits */
 #define SSTATUS_UIE 0x0001
 #define SSTATUS_SIE 0x0002
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 832c3bf7fd..6052b2d6e9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -492,7 +492,11 @@ static int write_mstatus(CPURISCVState *env, int csrno, 
target_ulong val)
 
 dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
 ((mstatus & MSTATUS_XS) == MSTATUS_XS);
-mstatus = set_field(mstatus, MSTATUS_SD, dirty);
+if (riscv_cpu_is_32bit(env)) {
+mstatus = set_field(mstatus, MSTATUS32_SD, dirty);
+} else {
+mstatus = set_field(mstatus, MSTATUS64_SD, dirty);
+}
 env->mstatus = mstatus;
 
 return 0;
@@ -564,7 +568,11 @@ static int write_misa(CPURISCVState *env, int csrno, 
target_ulong val)
 }
 
 /* misa.MXL writes are not supported by QEMU */
-val = (env->misa & MISA_MXL) | (val & ~MISA_MXL);
+if (riscv_cpu_is_32bit(env)) {
+val = (env->misa & MISA32_MXL) | (val & ~MISA32_MXL);
+} else {
+val = (env->misa & MISA64_MXL) | (val & ~MISA64_MXL);
+}
 
 /* flush translation cache */
 if (val != env->misa) {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 2f9f5ccc62..74636b9db7 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -78,6 +78,17 @@ static inline bool has_ext(DisasContext *ctx, uint32_t ext)
 return ctx->misa & ext;
 }
 
+#ifndef CONFIG_USER_ONLY
+# ifdef TARGET_RISCV32
+#  define is_32bit(ctx)  true
+# else
+static inline bool is_32bit(DisasContext *ctx)
+{
+return !(ctx->misa & RV64);
+}
+# endif
+#endif
+
 /*
  * RISC-V requires NaN-boxing of narrower width floating point values.
  * This applies when a 32-bit value is assigned to a 64-bit FP register.
@@ -369,6 +380,8 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong 
imm)
 static void mark_fs_dirty(DisasContext *ctx)
 {
 TCGv tmp;
+target_ulong sd;
+
 if (ctx->mstatus_fs == MSTATUS_FS) {
 return;
 }
@@ -376,13 +389,16 @@ static void mark_fs_dirty(DisasContext *ctx)
 ctx->mstatus_fs = MSTATUS_FS;
 
 tmp = tcg_temp_new();
+sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
+
+
 tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
-tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS_SD);
+tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
 tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
 
 if (ctx->virt_enabled) {
 tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
-tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS_SD);
+tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
 tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
 }
 tcg_temp_free(tmp);
-- 
2.31.1

[PATCH v2 9/9] target/riscv: Consolidate RV32/64 16-bit instructions

[Alistair Francis]

This patch removes the insn16-32.decode and insn16-64.decode decode
files and consolidates the instructions into the general RISC-V
insn16.decode decode tree.

This means that all of the instructions are avaliable in both the 32-bit
and 64-bit builds. This also means that we run a check to ensure we are
running a 64-bit softmmu before we execute the 64-bit only instructions.
This allows us to include the 32-bit instructions in the 64-bit build,
while also ensuring that 32-bit only software can not execute the
instructions.

Signed-off-by: Alistair Francis 
---
 target/riscv/insn16-32.decode   | 28 ---
 target/riscv/insn16-64.decode   | 36 -
 target/riscv/insn16.decode  | 30 +
 target/riscv/insn_trans/trans_rvi.c.inc |  6 +
 target/riscv/meson.build| 11 +++-
 5 files changed, 39 insertions(+), 72 deletions(-)
 delete mode 100644 target/riscv/insn16-32.decode
 delete mode 100644 target/riscv/insn16-64.decode

diff --git a/target/riscv/insn16-32.decode b/target/riscv/insn16-32.decode
deleted file mode 100644
index 0819b17028..00
--- a/target/riscv/insn16-32.decode
+++ /dev/null
@@ -1,28 +0,0 @@
-#
-# RISC-V translation routines for the RVXI Base Integer Instruction Set.
-#
-# Copyright (c) 2018 Peer Adelt, peer.ad...@hni.uni-paderborn.de
-#Bastian Koppelmann, kbast...@mail.uni-paderborn.de
-#
-# This program is free software; you can redistribute it and/or modify it
-# under the terms and conditions of the GNU General Public License,
-# version 2 or later, as published by the Free Software Foundation.
-#
-# This program is distributed in the hope it will be useful, but WITHOUT
-# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
-# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
-# more details.
-#
-# You should have received a copy of the GNU General Public License along with
-# this program.  If not, see .
-
-# *** RV32C Standard Extension (Quadrant 0) ***
-flw   011  ... ... .. ... 00 @cl_w
-fsw   111  ... ... .. ... 00 @cs_w
-
-# *** RV32C Standard Extension (Quadrant 1) ***
-jal   001 ... 01 @cjrd=1  # C.JAL
-
-# *** RV32C Standard Extension (Quadrant 2) ***
-flw   011 .  .  . 10 @c_lwsp
-fsw   111 .  .  . 10 @c_swsp
diff --git a/target/riscv/insn16-64.decode b/target/riscv/insn16-64.decode
deleted file mode 100644
index 672e1e916f..00
--- a/target/riscv/insn16-64.decode
+++ /dev/null
@@ -1,36 +0,0 @@
-#
-# RISC-V translation routines for the RVXI Base Integer Instruction Set.
-#
-# Copyright (c) 2018 Peer Adelt, peer.ad...@hni.uni-paderborn.de
-#Bastian Koppelmann, kbast...@mail.uni-paderborn.de
-#
-# This program is free software; you can redistribute it and/or modify it
-# under the terms and conditions of the GNU General Public License,
-# version 2 or later, as published by the Free Software Foundation.
-#
-# This program is distributed in the hope it will be useful, but WITHOUT
-# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
-# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
-# more details.
-#
-# You should have received a copy of the GNU General Public License along with
-# this program.  If not, see .
-
-# *** RV64C Standard Extension (Quadrant 0) ***
-ld011  ... ... .. ... 00 @cl_d
-sd111  ... ... .. ... 00 @cs_d
-
-# *** RV64C Standard Extension (Quadrant 1) ***
-{
-  illegal 001 -  0  - 01 # c.addiw, RES rd=0
-  addiw   001 .  .  . 01 @ci
-}
-subw  100 1 11 ... 00 ... 01 @cs_2
-addw  100 1 11 ... 01 ... 01 @cs_2
-
-# *** RV64C Standard Extension (Quadrant 2) ***
-{
-  illegal 011 -  0  - 10 # c.ldsp, RES rd=0
-  ld  011 .  .  . 10 @c_ldsp
-}
-sd111 .  .  . 10 @c_sdsp
diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 1cb93876fe..2e9212663c 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -92,6 +92,16 @@ lw010  ... ... .. ... 00 @cl_w
 fsd   101  ... ... .. ... 00 @cs_d
 sw110  ... ... .. ... 00 @cs_w
 
+# *** RV32C and RV64C specific Standard Extension (Quadrant 0) ***
+{
+  ld  011  ... ... .. ... 00 @cl_d
+  flw 011  ... ... .. ... 00 @cl_w
+}
+{
+  sd  111  ... ... .. ... 00 @cs_d
+  fsw 111  ... ... .. ... 00 @cs_w
+}
+
 # *** RV32/64C Standard Extension (Quadrant 1) ***
 addi  000 .  .  . 01 @ci
 addi  010 .  .  . 01 @c_li
@@ -111,6 +121,15 @@ jal   101 ... 01 @cjrd=0  # C.J
 beq   110  ... ...  . 01 @cb_z
 

[PATCH v2 5/9] target/riscv: Remove the hardcoded SATP_MODE macro

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_bits.h   | 11 ---
 target/riscv/cpu_helper.c | 24 ++--
 target/riscv/csr.c| 20 
 target/riscv/monitor.c| 22 +-
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index dd643d0f63..6a816ce9c2 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -452,17 +452,6 @@
 #define SATP64_ASID 0x0000ULL
 #define SATP64_PPN  0x0FFFULL
 
-#if defined(TARGET_RISCV32)
-#define SATP_MODE   SATP32_MODE
-#define SATP_ASID   SATP32_ASID
-#define SATP_PPNSATP32_PPN
-#endif
-#if defined(TARGET_RISCV64)
-#define SATP_MODE   SATP64_MODE
-#define SATP_ASID   SATP64_ASID
-#define SATP_PPNSATP64_PPN
-#endif
-
 /* VM modes (mstatus.vm) privileged ISA 1.9.1 */
 #define VM_1_09_MBARE   0
 #define VM_1_09_MBB 1
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b065ddb681..e5e9339458 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -403,11 +403,21 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
 
 if (first_stage == true) {
 if (use_background) {
-base = (hwaddr)get_field(env->vsatp, SATP_PPN) << PGSHIFT;
-vm = get_field(env->vsatp, SATP_MODE);
+if (riscv_cpu_is_32bit(env)) {
+base = (hwaddr)get_field(env->vsatp, SATP32_PPN) << PGSHIFT;
+vm = get_field(env->vsatp, SATP32_MODE);
+} else {
+base = (hwaddr)get_field(env->vsatp, SATP64_PPN) << PGSHIFT;
+vm = get_field(env->vsatp, SATP64_MODE);
+}
 } else {
-base = (hwaddr)get_field(env->satp, SATP_PPN) << PGSHIFT;
-vm = get_field(env->satp, SATP_MODE);
+if (riscv_cpu_is_32bit(env)) {
+base = (hwaddr)get_field(env->satp, SATP32_PPN) << PGSHIFT;
+vm = get_field(env->satp, SATP32_MODE);
+} else {
+base = (hwaddr)get_field(env->satp, SATP64_PPN) << PGSHIFT;
+vm = get_field(env->satp, SATP64_MODE);
+}
 }
 widened = 0;
 } else {
@@ -622,8 +632,10 @@ static void raise_mmu_exception(CPURISCVState *env, 
target_ulong address,
 CPUState *cs = env_cpu(env);
 int page_fault_exceptions, vm;
 
-if (first_stage) {
-vm = get_field(env->satp, SATP_MODE);
+if (first_stage && riscv_cpu_is_32bit(env)) {
+vm = get_field(env->satp, SATP32_MODE);
+} else if (first_stage) {
+vm = get_field(env->satp, SATP64_MODE);
 } else if (riscv_cpu_is_32bit(env)) {
 vm = get_field(env->hgatp, SATP32_MODE);
 } else {
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6052b2d6e9..8abfe33b29 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -927,21 +927,33 @@ static int read_satp(CPURISCVState *env, int csrno, 
target_ulong *val)
 
 static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
 {
+int vm, mask, asid;
+
 if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
 return 0;
 }
-if (validate_vm(env, get_field(val, SATP_MODE)) &&
-((val ^ env->satp) & (SATP_MODE | SATP_ASID | SATP_PPN)))
-{
+
+if (riscv_cpu_is_32bit(env)) {
+vm = validate_vm(env, get_field(val, SATP32_MODE));
+mask = (val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN);
+asid = (val ^ env->satp) & SATP32_ASID;
+} else {
+vm = validate_vm(env, get_field(val, SATP64_MODE));
+mask = (val ^ env->satp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN);
+asid = (val ^ env->satp) & SATP64_ASID;
+}
+
+if (vm && mask) {
 if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
 return -RISCV_EXCP_ILLEGAL_INST;
 } else {
-if ((val ^ env->satp) & SATP_ASID) {
+if (asid) {
 tlb_flush(env_cpu(env));
 }
 env->satp = val;
 }
 }
+
 return 0;
 }
 
diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
index e51188f919..f7e6ea72b3 100644
--- a/target/riscv/monitor.c
+++ b/target/riscv/monitor.c
@@ -150,9 +150,14 @@ static void mem_info_svxx(Monitor *mon, CPUArchState *env)
 target_ulong last_size;
 int last_attr;
 
-base = (hwaddr)get_field(env->satp, SATP_PPN) << PGSHIFT;
+if (riscv_cpu_is_32bit(env)) {
+base = (hwaddr)get_field(env->satp, SATP32_PPN) << PGSHIFT;
+vm = get_field(env->satp, SATP32_MODE);
+} else {
+base = (hwaddr)get_field(env->satp, SATP64_PPN) << PGSHIFT;
+vm = get_field(env->satp, SATP64_MODE);
+}
 
-vm = get_field(env->satp, SATP_MODE);
 switch (vm) {
 case VM_1_10_SV32:
 levels = 2;
@@ -215,9 +220,16 @@ 

[PATCH v2 8/9] target/riscv: Consolidate RV32/64 32-bit instructions

[Alistair Francis]

This patch removes the insn32-64.decode decode file and consolidates the
instructions into the general RISC-V insn32.decode decode tree.

This means that all of the instructions are avaliable in both the 32-bit
and 64-bit builds. This also means that we run a check to ensure we are
running a 64-bit softmmu before we execute the 64-bit only instructions.
This allows us to include the 32-bit instructions in the 64-bit build,
while also ensuring that 32-bit only software can not execute the
instructions.

Signed-off-by: Alistair Francis 
---
 target/riscv/helper.h   |  2 -
 target/riscv/insn32-64.decode   | 88 -
 target/riscv/insn32.decode  | 67 ++-
 target/riscv/translate.c| 19 +++---
 target/riscv/vector_helper.c|  4 --
 target/riscv/insn_trans/trans_rva.c.inc | 14 +++-
 target/riscv/insn_trans/trans_rvd.c.inc | 43 ++--
 target/riscv/insn_trans/trans_rvf.c.inc | 40 +--
 target/riscv/insn_trans/trans_rvh.c.inc |  5 +-
 target/riscv/insn_trans/trans_rvi.c.inc | 16 +++--
 target/riscv/insn_trans/trans_rvm.c.inc |  7 +-
 target/riscv/insn_trans/trans_rvv.c.inc | 39 +--
 target/riscv/meson.build|  2 +-
 13 files changed, 172 insertions(+), 174 deletions(-)
 delete mode 100644 target/riscv/insn32-64.decode

diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index e3f3f41e89..cda1811512 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -241,7 +241,6 @@ DEF_HELPER_5(vlhuff_v_w, void, ptr, ptr, tl, env, i32)
 DEF_HELPER_5(vlhuff_v_d, void, ptr, ptr, tl, env, i32)
 DEF_HELPER_5(vlwuff_v_w, void, ptr, ptr, tl, env, i32)
 DEF_HELPER_5(vlwuff_v_d, void, ptr, ptr, tl, env, i32)
-#ifdef TARGET_RISCV64
 DEF_HELPER_6(vamoswapw_v_d, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamoswapd_v_d, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamoaddw_v_d,  void, ptr, ptr, tl, ptr, env, i32)
@@ -260,7 +259,6 @@ DEF_HELPER_6(vamominuw_v_d, void, ptr, ptr, tl, ptr, env, 
i32)
 DEF_HELPER_6(vamominud_v_d, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamomaxuw_v_d, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamomaxud_v_d, void, ptr, ptr, tl, ptr, env, i32)
-#endif
 DEF_HELPER_6(vamoswapw_v_w, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamoaddw_v_w,  void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vamoxorw_v_w,  void, ptr, ptr, tl, ptr, env, i32)
diff --git a/target/riscv/insn32-64.decode b/target/riscv/insn32-64.decode
deleted file mode 100644
index 8157dee8b7..00
--- a/target/riscv/insn32-64.decode
+++ /dev/null
@@ -1,88 +0,0 @@
-#
-# RISC-V translation routines for the RV Instruction Set.
-#
-# Copyright (c) 2018 Peer Adelt, peer.ad...@hni.uni-paderborn.de
-#Bastian Koppelmann, kbast...@mail.uni-paderborn.de
-#
-# This program is free software; you can redistribute it and/or modify it
-# under the terms and conditions of the GNU General Public License,
-# version 2 or later, as published by the Free Software Foundation.
-#
-# This program is distributed in the hope it will be useful, but WITHOUT
-# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
-# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
-# more details.
-#
-# You should have received a copy of the GNU General Public License along with
-# this program.  If not, see .
-
-# This is concatenated with insn32.decode for risc64 targets.
-# Most of the fields and formats are there.
-
-%sh520:5
-
-@sh5 ...  . .  ... . ...   shamt=%sh5  %rs1 
%rd
-
-# *** RV64I Base Instruction Set (in addition to RV32I) ***
-lwu     . 110 . 011 @i
-ld      . 011 . 011 @i
-sd   ... .  . 011 . 0100011 @s
-addiw   . 000 . 0011011 @i
-slliw000 .  . 001 . 0011011 @sh5
-srliw000 .  . 101 . 0011011 @sh5
-sraiw010 .  . 101 . 0011011 @sh5
-addw 000 .  . 000 . 0111011 @r
-subw 010 .  . 000 . 0111011 @r
-sllw 000 .  . 001 . 0111011 @r
-srlw 000 .  . 101 . 0111011 @r
-sraw 010 .  . 101 . 0111011 @r
-
-# *** RV64M Standard Extension (in addition to RV32M) ***
-mulw 001 .  . 000 . 0111011 @r
-divw 001 .  . 100 . 0111011 @r
-divuw001 .  . 101 . 0111011 @r
-remw 001 .  . 110 . 0111011 @r
-remuw001 .  . 111 . 0111011 @r
-
-# *** RV64A Standard Extension (in addition to RV32A) ***
-lr_d   00010 . . 0 . 011 . 010 @atom_ld
-sc_d   00011 . . . . 011 . 010 @atom_st
-amoswap_d  1 . . . . 011 . 010 @atom_st
-amoadd_d   0 . . . . 011 . 010 @atom_st
-amoxor_d   00100 . 

[PATCH v2 1/9] target/riscv: Remove the hardcoded RVXLEN macro

[Alistair Francis]

Signed-off-by: Alistair Francis 
Reviewed-by: Richard Henderson 
---
 target/riscv/cpu.h | 6 --
 target/riscv/cpu.c | 6 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0a33d387ba..ef838f5fbf 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -53,12 +53,6 @@
 #define RV32 ((target_ulong)1 << (TARGET_LONG_BITS - 2))
 #define RV64 ((target_ulong)2 << (TARGET_LONG_BITS - 2))
 
-#if defined(TARGET_RISCV32)
-#define RVXLEN RV32
-#elif defined(TARGET_RISCV64)
-#define RVXLEN RV64
-#endif
-
 #define RV(x) ((target_ulong)1 << (x - 'A'))
 
 #define RVI RV('I')
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7d6ed80f6b..92c3195531 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -147,7 +147,11 @@ static void set_resetvec(CPURISCVState *env, int resetvec)
 static void riscv_any_cpu_init(Object *obj)
 {
 CPURISCVState *env = _CPU(obj)->env;
-set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+#if defined(TARGET_RISCV32)
+set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+#elif defined(TARGET_RISCV64)
+set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+#endif
 set_priv_version(env, PRIV_VERSION_1_11_0);
 }
 
-- 
2.31.1

[PATCH v2 6/9] target/riscv: Remove the unused HSTATUS_WPRI macro

[Alistair Francis]

Signed-off-by: Alistair Francis 
Reviewed-by: Richard Henderson 
---
 target/riscv/cpu_bits.h | 6 --
 1 file changed, 6 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 6a816ce9c2..9f6fbe3dc5 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -416,12 +416,6 @@
 #define HSTATUS32_WPRI   0xFF8FF87E
 #define HSTATUS64_WPRI   0xFF8FF87EULL
 
-#if defined(TARGET_RISCV32)
-#define HSTATUS_WPRI HSTATUS32_WPRI
-#elif defined(TARGET_RISCV64)
-#define HSTATUS_WPRI HSTATUS64_WPRI
-#endif
-
 #define HCOUNTEREN_CY(1 << 0)
 #define HCOUNTEREN_TM(1 << 1)
 #define HCOUNTEREN_IR(1 << 2)
-- 
2.31.1

[PATCH v2 3/9] target/riscv: Remove the hardcoded HGATP_MODE macro

[Alistair Francis]

Signed-off-by: Alistair Francis 
Reviewed-by: Richard Henderson 
---
 target/riscv/cpu_bits.h   | 11 ---
 target/riscv/cpu_helper.c | 24 +++-
 2 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 969dd05eae..8caab23b62 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -207,17 +207,6 @@
 #define CSR_HTIMEDELTA  0x605
 #define CSR_HTIMEDELTAH 0x615
 
-#if defined(TARGET_RISCV32)
-#define HGATP_MODE   SATP32_MODE
-#define HGATP_VMID   SATP32_ASID
-#define HGATP_PPNSATP32_PPN
-#endif
-#if defined(TARGET_RISCV64)
-#define HGATP_MODE   SATP64_MODE
-#define HGATP_VMID   SATP64_ASID
-#define HGATP_PPNSATP64_PPN
-#endif
-
 /* Virtual CSRs */
 #define CSR_VSSTATUS0x200
 #define CSR_VSIE0x204
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 21c54ef561..b065ddb681 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -411,8 +411,13 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 }
 widened = 0;
 } else {
-base = (hwaddr)get_field(env->hgatp, HGATP_PPN) << PGSHIFT;
-vm = get_field(env->hgatp, HGATP_MODE);
+if (riscv_cpu_is_32bit(env)) {
+base = (hwaddr)get_field(env->hgatp, SATP32_PPN) << PGSHIFT;
+vm = get_field(env->hgatp, SATP32_MODE);
+} else {
+base = (hwaddr)get_field(env->hgatp, SATP64_PPN) << PGSHIFT;
+vm = get_field(env->hgatp, SATP64_MODE);
+}
 widened = 2;
 }
 /* status.SUM will be ignored if execute on background */
@@ -615,16 +620,17 @@ static void raise_mmu_exception(CPURISCVState *env, 
target_ulong address,
 bool first_stage, bool two_stage)
 {
 CPUState *cs = env_cpu(env);
-int page_fault_exceptions;
+int page_fault_exceptions, vm;
+
 if (first_stage) {
-page_fault_exceptions =
-get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
-!pmp_violation;
+vm = get_field(env->satp, SATP_MODE);
+} else if (riscv_cpu_is_32bit(env)) {
+vm = get_field(env->hgatp, SATP32_MODE);
 } else {
-page_fault_exceptions =
-get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
-!pmp_violation;
+vm = get_field(env->hgatp, SATP64_MODE);
 }
+page_fault_exceptions = vm != VM_1_10_MBARE && !pmp_violation;
+
 switch (access_type) {
 case MMU_INST_FETCH:
 if (riscv_cpu_virt_enabled(env) && !first_stage) {
-- 
2.31.1

[PATCH v2 2/9] target/riscv: Remove the hardcoded SSTATUS_SD macro

[Alistair Francis]

Signed-off-by: Alistair Francis 
Reviewed-by: Richard Henderson 
---
 target/riscv/cpu_bits.h | 6 --
 target/riscv/csr.c  | 9 -
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index caf4599207..969dd05eae 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -423,12 +423,6 @@
 #define SSTATUS32_SD0x8000
 #define SSTATUS64_SD0x8000ULL
 
-#if defined(TARGET_RISCV32)
-#define SSTATUS_SD SSTATUS32_SD
-#elif defined(TARGET_RISCV64)
-#define SSTATUS_SD SSTATUS64_SD
-#endif
-
 /* hstatus CSR bits */
 #define HSTATUS_VSBE 0x0020
 #define HSTATUS_GVA  0x0040
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d2585395bf..832c3bf7fd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -418,7 +418,7 @@ static const target_ulong delegable_excps =
 (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT));
 static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
 SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
-SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD;
+SSTATUS_SUM | SSTATUS_MXR;
 static const target_ulong sip_writable_mask = SIP_SSIP | MIP_USIP | MIP_UEIP;
 static const target_ulong hip_writable_mask = MIP_VSSIP;
 static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP | 
MIP_VSEIP;
@@ -738,6 +738,13 @@ static int rmw_mip(CPURISCVState *env, int csrno, 
target_ulong *ret_value,
 static int read_sstatus(CPURISCVState *env, int csrno, target_ulong *val)
 {
 target_ulong mask = (sstatus_v1_10_mask);
+
+if (riscv_cpu_is_32bit(env)) {
+mask |= SSTATUS32_SD;
+} else {
+mask |= SSTATUS64_SD;
+}
+
 *val = env->mstatus & mask;
 return 0;
 }
-- 
2.31.1

[PATCH v2 0/9] RISC-V: Steps towards running 32-bit guests on

[Alistair Francis]

This is another step towards running 32-bit CPU code on the 64-bit
softmmu builds for RISC-V.

I have tested this and am able to run some 32-bit code, but eventually
hit some issue.  This series doesn't allow users to use 32-bit CPUs with
64-bit softmmu builds as it doesn't work yet. This series instead just
gets us a little closer to being able to and removes more hardcoded
macros so hopefully others also stop using them for new code.

v2:
 - Update the decode tree setup
 - Address other review comments

Alistair Francis (9):
  target/riscv: Remove the hardcoded RVXLEN macro
  target/riscv: Remove the hardcoded SSTATUS_SD macro
  target/riscv: Remove the hardcoded HGATP_MODE macro
  target/riscv: Remove the hardcoded MSTATUS_SD macro
  target/riscv: Remove the hardcoded SATP_MODE macro
  target/riscv: Remove the unused HSTATUS_WPRI macro
  target/riscv: Remove an unused CASE_OP_32_64 macro
  target/riscv: Consolidate RV32/64 32-bit instructions
  target/riscv: Consolidate RV32/64 16-bit instructions

 target/riscv/cpu.h  |  6 --
 target/riscv/cpu_bits.h | 44 -
 target/riscv/helper.h   |  2 -
 target/riscv/insn16-32.decode   | 28 
 target/riscv/insn16-64.decode   | 36 --
 target/riscv/insn16.decode  | 30 +
 target/riscv/insn32-64.decode   | 88 -
 target/riscv/insn32.decode  | 67 ++-
 target/riscv/cpu.c  |  6 +-
 target/riscv/cpu_helper.c   | 46 +
 target/riscv/csr.c  | 41 ++--
 target/riscv/monitor.c  | 22 +--
 target/riscv/translate.c| 33 ++
 target/riscv/vector_helper.c|  4 --
 target/riscv/insn_trans/trans_rva.c.inc | 14 +++-
 target/riscv/insn_trans/trans_rvd.c.inc | 43 ++--
 target/riscv/insn_trans/trans_rvf.c.inc | 40 +--
 target/riscv/insn_trans/trans_rvh.c.inc |  5 +-
 target/riscv/insn_trans/trans_rvi.c.inc | 22 +--
 target/riscv/insn_trans/trans_rvm.c.inc |  7 +-
 target/riscv/insn_trans/trans_rvv.c.inc | 39 +--
 target/riscv/meson.build| 13 ++--
 22 files changed, 311 insertions(+), 325 deletions(-)
 delete mode 100644 target/riscv/insn16-32.decode
 delete mode 100644 target/riscv/insn16-64.decode
 delete mode 100644 target/riscv/insn32-64.decode

-- 
2.31.1

Issues with modifying pc in a sigaction handler

[Devin Hussey]

In a toy project I was doing
(https://github.com/easyaspi314/ThumbGolf), I found that qemu will
incorrectly handle modifying pc in a handler.

Specifically, on platforms with instruction alignment requirements
(most notably ARM), if you set the pc to an odd address, QEMU will
start reading unaligned instructions.

Naturally, this is frustrating when dealing with ARM Thumb functions
which have the lowest bit set when referenced, as you must manually
clear the Thumb bit instead of it being implicit on hardware.

The following code exhibits this bug for ARM:

---
#include 
#include 
#include 

static void hello(void)
{
printf("Hello,");
}

static void handler(int signo, siginfo_t *si, void *data)
{
ucontext_t *uc = (ucontext_t *)data;
// Effectively bl hello although we assume thumb state
uc->uc_mcontext.arm_lr = uc->uc_mcontext.arm_pc + 2 | 1;
uc->uc_mcontext.arm_pc = (unsigned long)
}

int main(void)
{
// Set up the signal handler
struct sigaction sa, osa;
sa.sa_flags = SA_ONSTACK | SA_RESTART | SA_NODEFER | SA_SIGINFO;
sa.sa_sigaction = handler;
sigaction(SIGILL, , );
sigaction(SIGTRAP, , );

// Throw a SIGILL, which we do a runtime patch to call hello().
// Make sure we mark the caller saved registers.
__asm__ ("udf #0" ::: "r0", "r1", "r2", "r3", "r12", "lr", "memory");
printf(" world!\n");
}
---
Compile with:
clang -O2 -march=armv7-a -mthumb file.c -static

(The same should happen with GCC).

On hardware (specifically, a Snapdragon 730g in Termux on Android 11),
the code prints "Hello, world!" and exits normally.

However, qemu-arm will get tripped up by the pc being odd, and execute this:

---
... snip

IN: main
0x00010288:  4c05   ldr  r4, [pc, #0x14]
0x0001028a:  de00   udf  #0


IN: handler
0x000102a4:  4804   ldr  r0, [pc, #0x10]
0x000102a6:  6dd1   ldr  r1, [r2, #0x5c]
0x000102a8:  4478   add  r0, pc
0x000102aa:  3102   adds r1, #2
0x000102ac:  f041 0101  orr  r1, r1, #1
0x000102b0:  e9c2 1016  strd r1, r0, [r2, #0x58]
0x000102b4:  4770   bx   lr


IN: __restore_rt
0x0004c36c:  e3a070ad  mov  r7, #0xad
0x0004c370:  ef00  svc  #0


IN: hello
0x000102bd:  7848   ldrb r0, [r1, #1]
0x000102bf:  0644   lsls r4, r0, #0x19
0x000102c1:  5cf0   ldrb r0, [r6, r3]
0x000102c3:  06bb   lsls r3, r7, #0x1a
0x000102c5:  053e   lsls r6, r7, #0x14
0x000102c7:  f000 89b5  beq.w#0x90635

qemu: uncaught target signal 11 (Segmentation fault) - core dumped

---

Note the odd addresses in hello().

It should be interpreted as so, which happens when you manually clear
the Thumb bit:

---
0x000102b8:  4801   ldr  r0, [pc, #4]
 0x000102ba:  4478   add  r0, pc
  0x000102bc:  f006
bb5c  b.w  #0x16978 (printf)
---

Re: [PATCH 5/5] target/ppc: Implement paddi and replace addi insns

[Philippe Mathieu-Daudé]

Hi Luis,

On 4/13/21 11:11 PM, Luis Pires wrote:
> This implements the Power ISA 3.1 prefixed (64-bit) paddi
> instruction, while also replacing the legacy addi implementation.
> Both using the decode tree.
> 
> Signed-off-by: Luis Pires 
> Signed-off-by: Matheus Ferst 
> ---
>  target/ppc/ppc.decode  |  8 +++
>  target/ppc/translate.c | 15 +
>  target/ppc/translate/fixedpoint-impl.c.inc | 26 ++
>  3 files changed, 35 insertions(+), 14 deletions(-)
>  create mode 100644 target/ppc/translate/fixedpoint-impl.c.inc
> 
> diff --git a/target/ppc/ppc.decode b/target/ppc/ppc.decode
> index 84bb73ac19..c87174dc20 100644
> --- a/target/ppc/ppc.decode
> +++ b/target/ppc/ppc.decode
> @@ -16,3 +16,11 @@
>  # You should have received a copy of the GNU Lesser General Public
>  # License along with this library; if not, see 
> .
>  #
> +
> +%p_D8_SI32:s18 0:16
> +
> +# Fixed-Point Facility Instructions
> +   r rt ra si
> +
> +paddi   01 10 0 -- r:1 -- .. 001110 rt:5 ra:5 
>  si=%p_D8_SI 

IIUC you should be able to do something like catch ra=0 first ...:

{
  addi_movi   01 10 0 -- r:1 -- .. 001110 rt:5 .
 si=%p_D8_SI  ra=0
  addi   01 10 0 -- r:1 -- .. 001110 rt:5 ra:5
 si=%p_D8_SI 
}

> +addi001110 rt:5 ra:5 si:s16  r=0
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 0f123f7b80..2ff192c9e5 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -945,19 +945,6 @@ GEN_INT_ARITH_ADD(addex, 0x05, cpu_ov, 1, 1, 0);
>  /* addze  addze.  addzeo  addzeo.*/
>  GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, cpu_ca, 1, 1, 0)
>  GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, cpu_ca, 1, 1, 1)
> -/* addi */
> -static void gen_addi(DisasContext *ctx)
> -{
> -target_long simm = SIMM(ctx->opcode);
> -
> -if (rA(ctx->opcode) == 0) {
> -/* li case */
> -tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm);
> -} else {
> -tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)],
> -cpu_gpr[rA(ctx->opcode)], simm);
> -}
> -}
>  /* addic  addic.*/
>  static inline void gen_op_addic(DisasContext *ctx, bool compute_rc0)
>  {
> @@ -6967,6 +6954,7 @@ static target_ulong 
> ppc_peek_next_insn_size(DisasContext *ctx)
>  }
>  
>  #include "decode-ppc.c.inc"
> +#include "translate/fixedpoint-impl.c.inc"
>  
>  #include "translate/fp-impl.c.inc"
>  
> @@ -7091,7 +7079,6 @@ GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x0060, 
> PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x0001, PPC_NONE, PPC2_ISA205),
>  GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x0041, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x0001, PPC_ISEL),
> -GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x, PPC_INTEGER),
>  GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x, PPC_INTEGER),
>  GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x, PPC_INTEGER),
>  GEN_HANDLER(addis, 0x0F, 0xFF, 0xFF, 0x, PPC_INTEGER),
> diff --git a/target/ppc/translate/fixedpoint-impl.c.inc 
> b/target/ppc/translate/fixedpoint-impl.c.inc
> new file mode 100644
> index 00..8620954b57
> --- /dev/null
> +++ b/target/ppc/translate/fixedpoint-impl.c.inc
> @@ -0,0 +1,26 @@
> +static bool trans_paddi(DisasContext *ctx, arg_paddi *a)

(Nitpick, use the format: arg_addi, not arg_paddi).

> +{
> +if (a->r == 0) {
> +if (a->ra == 0) {
> +/* li case */
> +tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
> +} else {
> +tcg_gen_addi_tl(cpu_gpr[a->rt],
> +cpu_gpr[a->ra], a->si);
> +}
> +} else {
> +if (a->ra == 0) {
> +tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_nip, a->si);
> +} else {
> +/* invalid form */
> +gen_invalid(ctx);
> +}
> +}
> +
> +return true;
> +}
> +
> +static bool trans_addi(DisasContext *ctx, arg_addi *a)
> +{
> +return trans_paddi(ctx, a);
> +}

... which then simplifies the trans_OPCODE() logic:

static bool trans_addi_movi(DisasContext *ctx, arg_addi *a)
{
if (a->r == 0) {
/* li case */
tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
} else {
tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_nip, a->si);
}

return true;
}

static bool trans_addi(DisasContext *ctx, arg_addi *a)
{
if (a->r == 0) {
tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_gpr[a->ra], a->si);
} else {
/* invalid form */
gen_invalid(ctx);
}

return true;
}

Maybe you can do the same with the r bit to remove the dual addi_movi.

Regards,

Phil.

[PATCHv2 1/1] Support monitor chardev hotswap with QMP

[Li Zhang]

For some scenarios, it needs to hot-add a monitor device.
But QEMU doesn't support hotplug yet. It also works by adding
a monitor with null backend by default and then change its
backend to socket by QMP command "chardev-change".

So this patch is to support monitor chardev hotswap with QMP.

Signed-off-by: Li Zhang 
Reviewed-by: Pankaj Gupta 
---
 v1 -> v2: 
  - Change mutex lock mon_lock section
  - Fix indentation problems

 monitor/monitor-internal.h |  3 +++
 monitor/monitor.c  |  2 +-
 monitor/qmp.c  | 43 +++---
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 9c3a09cb01..1b80c74883 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -183,4 +183,7 @@ void help_cmd(Monitor *mon, const char *name);
 void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
 int hmp_compare_cmd(const char *name, const char *list);
 
+gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
+   void *opaque);
+
 #endif
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 636bcc81c5..16a3620d02 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -157,7 +157,7 @@ static inline bool monitor_is_hmp_non_interactive(const 
Monitor *mon)
 
 static void monitor_flush_locked(Monitor *mon);
 
-static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
+gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
   void *opaque)
 {
 Monitor *mon = opaque;
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 2b0308f933..5fa65401ae 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -44,6 +44,7 @@ struct QMPRequest {
 Error *err;
 };
 typedef struct QMPRequest QMPRequest;
+static void monitor_qmp_set_handlers_bh(void *opaque);
 
 QmpCommandList qmp_commands, qmp_cap_negotiation_commands;
 
@@ -477,7 +478,36 @@ void monitor_data_destroy_qmp(MonitorQMP *mon)
 g_queue_free(mon->qmp_requests);
 }
 
-static void monitor_qmp_setup_handlers_bh(void *opaque)
+static int monitor_qmp_change(void *opaque)
+{
+MonitorQMP *mon = opaque;
+
+mon->common.use_io_thread = qemu_chr_has_feature(mon->common.chr.chr,
+QEMU_CHAR_FEATURE_GCONTEXT);
+
+if (mon->common.use_io_thread) {
+aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
+monitor_qmp_set_handlers_bh, mon);
+} else {
+qemu_chr_fe_set_handlers(>common.chr, monitor_can_read,
+ monitor_qmp_read, monitor_qmp_event,
+ monitor_qmp_change, >common, NULL, true);
+}
+
+qemu_mutex_lock(>common.mon_lock);
+if (mon->common.out_watch) {
+g_source_remove(mon->common.out_watch);
+mon->common.out_watch = qemu_chr_fe_add_watch(>common.chr,
+G_IO_OUT | G_IO_HUP,
+monitor_unblocked,
+>common);
+}
+qemu_mutex_unlock(>common.mon_lock);
+
+return 0;
+}
+
+static void monitor_qmp_set_handlers_bh(void *opaque)
 {
 MonitorQMP *mon = opaque;
 GMainContext *context;
@@ -487,7 +517,14 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
 assert(context);
 qemu_chr_fe_set_handlers(>common.chr, monitor_can_read,
  monitor_qmp_read, monitor_qmp_event,
- NULL, >common, context, true);
+ monitor_qmp_change, >common, context, true);
+
+}
+
+static void monitor_qmp_setup_handlers_bh(void *opaque)
+{
+MonitorQMP *mon = opaque;
+monitor_qmp_set_handlers_bh(mon);
 monitor_list_append(>common);
 }
 
@@ -528,7 +565,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error 
**errp)
 } else {
 qemu_chr_fe_set_handlers(>common.chr, monitor_can_read,
  monitor_qmp_read, monitor_qmp_event,
- NULL, >common, NULL, true);
+ monitor_qmp_change, >common, NULL, true);
 monitor_list_append(>common);
 }
 }
-- 
2.25.1

RE: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg

[Fabiano Rosas]

Bruno Piazera Larsen  writes:

>> I'm actually not sure if we'll want translate_init.c for !tcg builds.
>> It's *primarily* for TCG, but we still need at least some of the cpu
>> state structure for KVM, and some of that is initialized in
>> translate_init.
>>
>> I think it will probably make more sense to leave it in for a first
>> cut.  Later refinement might end up removing it.
>>
>> The whole #include translate_init.c.inc thing might make for some
>> awkward fiddling in this, of course.
>
> I just checked, there is going to be some shuffling of functions
> around, as there are some static variables defined on translate.c,
> and used in translate_init.c.inc, some functions needed for KVM
> on translate.c and some TCG only functions in the
> translate_init.c.inc.
>
> The trivial path is to:
> * rename translate_init.c.inc to cpu_init.c (since it has to do with
> initial definitions for CPUs, and it's not related to translating
> anymore);

Below I'm assuming we have one place for TCG stuff and other for KVM
stuff, whatever this particular discussion ends up producing.

> * move gen_write_xer and gen_read_xer into cpu_init.c, as they're
> used for some sprs, and whatever needs to be moved with it

I'd leave them where they are currently. Instead what I think we should
do is to find a way to not need the uea/oea/hea|read/write callbacks
with KVM.

Maybe extract a function from _spr_register that sets what is common for
both tcg and kvm (num, name, initial_value, AFAICT). Then alter the
gen_spr* functions to first create all registers and then call both
configs to supplement:

//tcg.c
static void tcg_gen_spr_generic(CPUPPCState *env)
{
// these only set the callbacks
spr_register(env, SPR_FOO,
 SPR_NOACCESS, SPR_NOACCESS,
 _read_foo, _write_foo);
spr_register(env, SPR_BAR,
 SPR_NOACCESS, SPR_NOACCESS,
 _read_bar, _write_bar);
}

//kvm.c
static void kvm_gen_spr_generic(CPUPPCState *env)
{
// these only set one_reg_id
spr_register_kvm(env, SPR_FOO, KVM_REG_PPC_FOO);
spr_register_kvm(env, SPR_BAR, KVM_REG_PPC_BAR);
}

//common.c
static void gen_spr_generic(CPUPPCState *env)
{
// these only set name, num, initial value
spr_register(env, SPR_FOO, "FOO", 0xf00);
spr_register(env, SPR_BAR, "BAR", 0xb4d);
...

// have these stubbed if not chosen via config
tcg_gen_spr_generic(env);
kvm_gen_spr_generic(env);
}

init_ppc_proc()
{
...
gen_spr_generic(env);
...
}

Can anyone see a better way? That would be much easier if we could
afford to say that TCG and KVM are mutually exclusive for a given build,
but I don't think they are.

> * move is_indirect_opcode and ind_table to translate.c, since they
> are used to translate ppc instructions, and the things defined for
> these functions

Makes sense. This and the other part below about callback tables would
be mostly about moving code so it's a candidate for coming soon.

> * Figure out what needs to be added to the includes for both
> files to compile
> * move opcodes and invalid_handler into cpu_init.c, because they
> are only used by stuff in this file.
>
> I'm just not sure about this last point. The stuff that use opcodes
> create the callback tables for TCG, AFAICT. The better plan would
> be to move all of that to tanslate.c, but might be a lot.

translate.c seems like a better place indeed.

[Bug 1923497] Re: bios_linker_loader_add_checksum: Assertion `start_offset < file->blob->len' failed

[Ed Davison]

Hmmm.  Well, I don't know what the command line was.  I use Virtual
Machine Manager (virt-manager.org) for my interface to the VM and it
does the startup.  The error shows up when I start the VM.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1923497

Title:
  bios_linker_loader_add_checksum: Assertion `start_offset <
  file->blob->len' failed

Status in QEMU:
  New

Bug description:
  Trying boot/start a Windows 10 VM.  Worked until recently when this
  error started showing up.

  I have the following installed on Fedora 33:
  qemu-kvm-5.1.0-9.fc33.x86_64

  This is the error:

  Error starting domain: internal error: process exited while connecting
  to monitor: qemu-system-x86_64:
  /builddir/build/BUILD/qemu-5.1.0/hw/acpi/bios-linker-loader.c:239:
  bios_linker_loader_add_checksum: Assertion `start_offset <
  file->blob->len' failed.

  Traceback (most recent call last):
File "/usr/share/virt-manager/virtManager/asyncjob.py", line 65, in 
cb_wrapper
  callback(asyncjob, *args, **kwargs)
File "/usr/share/virt-manager/virtManager/asyncjob.py", line 101, in tmpcb
  callback(*args, **kwargs)
File "/usr/share/virt-manager/virtManager/object/libvirtobject.py", line 
57, in newfn
  ret = fn(self, *args, **kwargs)
File "/usr/share/virt-manager/virtManager/object/domain.py", line 1329, in 
startup
  self._backend.create()
File "/usr/lib64/python3.9/site-packages/libvirt.py", line 1234, in create
  if ret == -1: raise libvirtError ('virDomainCreate() failed', dom=self)
  libvirt.libvirtError: internal error: process exited while connecting to 
monitor: qemu-system-x86_64: 
/builddir/build/BUILD/qemu-5.1.0/hw/acpi/bios-linker-loader.c:239: 
bios_linker_loader_add_checksum: Assertion `start_offset < file->blob->len' 
failed.

  I see this were referenced in a patch from some time ago and
  supposedly fixed.  Here is the patch info I was able to find:

  http://next.patchew.org/QEMU/1515677902-23436-1-git-send-email-
  peter.mayd...@linaro.org/1515677902-23436-10-git-send-email-
  peter.mayd...@linaro.org/

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1923497/+subscriptions

[PATCH] qemu-iotest: Test NBD hole reporting for qcow2

[Nir Soffer]

In commit commit 0da9856851dcca09222a1467e16ddd05dc66e460

nbd: server: Report holes for raw images

we changed the way holes are reported for raw images, but also how
known-zero portions of qcow2 files are reported. This was not covered by
iotests, and revealed recently by libnbd tests[1].

Add the missing tests for single qcow2 image and qcow2 image with a
backing file.

[1] https://listman.redhat.com/archives/libguestfs/2021-April/msg00050.html

Signed-off-by: Nir Soffer 
---
 tests/qemu-iotests/314   | 96 
 tests/qemu-iotests/314.out   | 34 +
 tests/qemu-iotests/common.rc |  1 +
 3 files changed, 131 insertions(+)
 create mode 100755 tests/qemu-iotests/314
 create mode 100644 tests/qemu-iotests/314.out

diff --git a/tests/qemu-iotests/314 b/tests/qemu-iotests/314
new file mode 100755
index 00..81c0169eac
--- /dev/null
+++ b/tests/qemu-iotests/314
@@ -0,0 +1,96 @@
+#!/usr/bin/env bash
+# group: rw quick
+#
+# Test qemu-nbd base:allocation metacontext
+#
+# Copyright (C) 2021 Nir Soffer 
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program 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 General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+# owner=nir...@gmail.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+rm -f "$TEST_DIR/server.log"
+nbd_server_stop
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.nbd
+
+_supported_fmt qcow2
+_supported_proto nbd
+_supported_os Linux
+_require_command QEMU_NBD
+
+TEST_IMG="nbd+unix:///?socket=$nbd_unix_socket"
+
+echo
+echo "=== Single image ==="
+echo
+
+$QEMU_IMG create -f "$IMGFMT" -o cluster_size=64k \
+"$TEST_IMG_FILE" 384k | _filter_img_create_filenames
+
+$QEMU_IO -f $IMGFMT -c "write -P 1 0k 64k" "$TEST_IMG_FILE" | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -c "write -P 2 64k 512" "$TEST_IMG_FILE" | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -c "write -z 192k 64k" "$TEST_IMG_FILE" | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -c "write -z 256k 512" "$TEST_IMG_FILE" | _filter_qemu_io
+
+nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
+
+echo
+$QEMU_NBD_PROG --list -k $nbd_unix_socket >/dev/null
+$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+nbd_server_stop
+
+echo
+echo "=== Image with backing file ==="
+echo
+
+$QEMU_IMG create -f "$IMGFMT" -o cluster_size=64k \
+"$TEST_IMG_FILE.base" 384k | _filter_img_create_filenames
+
+$QEMU_IO -f $IMGFMT -c "write -P 1 0k 64k" "$TEST_IMG_FILE.base" | 
_filter_qemu_io
+$QEMU_IO -f $IMGFMT -c "write -P 2 64k 512" "$TEST_IMG_FILE.base" | 
_filter_qemu_io
+
+$QEMU_IMG create -f "$IMGFMT" -o cluster_size=64k \
+-b "$TEST_IMG_FILE.base" -F $IMGFMT "$TEST_IMG_FILE" | 
_filter_img_create_filenames
+
+$QEMU_IO -f $IMGFMT -c "write -z 192k 64k" "$TEST_IMG_FILE" | _filter_qemu_io
+$QEMU_IO -f $IMGFMT -c "write -z 256k 512" "$TEST_IMG_FILE" | _filter_qemu_io
+
+nbd_server_start_unix_socket -f $IMGFMT "$TEST_IMG_FILE"
+
+echo
+$QEMU_NBD_PROG --list -k $nbd_unix_socket >/dev/null
+$QEMU_IMG map -f raw --output=json "$TEST_IMG" | _filter_qemu_img_map
+
+nbd_server_stop
+
+# success, all done
+echo
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/314.out b/tests/qemu-iotests/314.out
new file mode 100644
index 00..df7eef023f
--- /dev/null
+++ b/tests/qemu-iotests/314.out
@@ -0,0 +1,34 @@
+QA output created by 314
+
+=== Single image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT cluster_size=65536 extended_l2=off 
compression_type=zlib size=393216 lazy_refcounts=off refcount_bits=16
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 65536
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 196608
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset 262144
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+[{ "start": 0, "length": 131072, "depth": 0, "zero": false, "data": true, 
"offset": OFFSET},
+{ "start": 131072, "length": 262144, "depth": 0, "zero": true, "data": false, 
"offset": OFFSET}]
+
+=== Image with backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT cluster_size=65536 
extended_l2=off compression_type=zlib size=393216 lazy_refcounts=off 

Re: [PATCH] docs: Add a QEMU Code of Conduct and Conflict Resolution Policy document

[Paolo Bonzini]

Il mar 13 apr 2021, 18:25 Daniel P. Berrangé  ha
scritto:

> Since this was derived from the Fedora CoC, you might be interested to
> know that Fedora is currently revisiting its CoC:
>
>
> https://communityblog.fedoraproject.org/policy-proposal-new-code-of-conduct/
>
> The first comment on that post from mattdm gives clarity as to why they
> feel the need to revisit it


Interesting, thanks. Indeed the changes that I brought over from the
Contributor Covenant (and that were also there, albeit more verbosely, in
the Django code of conduct) have the purpose of making the text more
specific in case "being excellent to each other" just isn't enough. We also
have the separate conflict resolution guide that covers the third point
that Matt made in his post.

Having some confirmation that those things *were* missing from the current
Fedora CoC is good. In fact in the meanwhile I found a few other cases
(such as the Microsoft open source code of conduct,
https://opensource.microsoft.com/codeofconduct/) that merged a less
prescriptive text ultimately derived from Fedora with parts of the
Contributor Covenant, so it looks like others have had the same idea in the
past.

Paolo

>

[PATCH 3/5] decodetree: Allow custom var width load functions

[Luis Pires]

This is useful in situations where you want decodetree
to handle variable width instructions but you want to
provide custom code to load the instructions. Suppressing
the generation of the load function is necessary to avoid
compilation errors due to the load function being unused.

This will be used by the PowerPC decodetree code.

Signed-off-by: Luis Pires 
---
 scripts/decodetree.py | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 935b2964e0..a62b8d8d76 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1278,9 +1278,10 @@ def main():
 global anyextern
 
 decode_scope = 'static '
+noloadfunc = False
 
 long_opts = ['decode=', 'translate=', 'output=', 'insnwidth=',
- 'static-decode=', 'varinsnwidth=']
+ 'static-decode=', 'varinsnwidth=', 'noloadfunc']
 try:
 (opts, args) = getopt.gnu_getopt(sys.argv[1:], 'o:vw:', long_opts)
 except getopt.GetoptError as err:
@@ -1312,6 +1313,8 @@ def main():
 deposit_function = 'deposit64'
 elif insnwidth != 32:
 error(0, 'cannot handle insns of width', insnwidth)
+elif o == '--noloadfunc':
+noloadfunc = True
 else:
 assert False, 'unhandled option'
 
@@ -1401,12 +1404,13 @@ def main():
 output(i4, 'return false;\n')
 output('}\n')
 
-if variablewidth:
-output('\n', decode_scope, insntype, ' ', decode_function,
-   '_load(DisasContext *ctx)\n{\n',
-   '', insntype, ' insn = 0;\n\n')
-stree.output_code(4, 0, 0, 0)
-output('}\n')
+if not noloadfunc:
+if variablewidth:
+output('\n', decode_scope, insntype, ' ', decode_function,
+   '_load(DisasContext *ctx)\n{\n',
+   '', insntype, ' insn = 0;\n\n')
+stree.output_code(4, 0, 0, 0)
+output('}\n')
 
 if output_file:
 output_fd.close()
-- 
2.25.1

[PATCH 5/5] target/ppc: Implement paddi and replace addi insns

[Luis Pires]

This implements the Power ISA 3.1 prefixed (64-bit) paddi
instruction, while also replacing the legacy addi implementation.
Both using the decode tree.

Signed-off-by: Luis Pires 
Signed-off-by: Matheus Ferst 
---
 target/ppc/ppc.decode  |  8 +++
 target/ppc/translate.c | 15 +
 target/ppc/translate/fixedpoint-impl.c.inc | 26 ++
 3 files changed, 35 insertions(+), 14 deletions(-)
 create mode 100644 target/ppc/translate/fixedpoint-impl.c.inc

diff --git a/target/ppc/ppc.decode b/target/ppc/ppc.decode
index 84bb73ac19..c87174dc20 100644
--- a/target/ppc/ppc.decode
+++ b/target/ppc/ppc.decode
@@ -16,3 +16,11 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library; if not, see .
 #
+
+%p_D8_SI32:s18 0:16
+
+# Fixed-Point Facility Instructions
+   r rt ra si
+
+paddi   01 10 0 -- r:1 -- .. 001110 rt:5 ra:5 
 si=%p_D8_SI 
+addi001110 rt:5 ra:5 si:s16  r=0
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0f123f7b80..2ff192c9e5 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -945,19 +945,6 @@ GEN_INT_ARITH_ADD(addex, 0x05, cpu_ov, 1, 1, 0);
 /* addze  addze.  addzeo  addzeo.*/
 GEN_INT_ARITH_ADD_CONST(addze, 0x06, 0, cpu_ca, 1, 1, 0)
 GEN_INT_ARITH_ADD_CONST(addzeo, 0x16, 0, cpu_ca, 1, 1, 1)
-/* addi */
-static void gen_addi(DisasContext *ctx)
-{
-target_long simm = SIMM(ctx->opcode);
-
-if (rA(ctx->opcode) == 0) {
-/* li case */
-tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], simm);
-} else {
-tcg_gen_addi_tl(cpu_gpr[rD(ctx->opcode)],
-cpu_gpr[rA(ctx->opcode)], simm);
-}
-}
 /* addic  addic.*/
 static inline void gen_op_addic(DisasContext *ctx, bool compute_rc0)
 {
@@ -6967,6 +6954,7 @@ static target_ulong ppc_peek_next_insn_size(DisasContext 
*ctx)
 }
 
 #include "decode-ppc.c.inc"
+#include "translate/fixedpoint-impl.c.inc"
 
 #include "translate/fp-impl.c.inc"
 
@@ -7091,7 +7079,6 @@ GEN_HANDLER_E(cmpeqb, 0x1F, 0x00, 0x07, 0x0060, 
PPC_NONE, PPC2_ISA300),
 GEN_HANDLER_E(cmpb, 0x1F, 0x1C, 0x0F, 0x0001, PPC_NONE, PPC2_ISA205),
 GEN_HANDLER_E(cmprb, 0x1F, 0x00, 0x06, 0x0041, PPC_NONE, PPC2_ISA300),
 GEN_HANDLER(isel, 0x1F, 0x0F, 0xFF, 0x0001, PPC_ISEL),
-GEN_HANDLER(addi, 0x0E, 0xFF, 0xFF, 0x, PPC_INTEGER),
 GEN_HANDLER(addic, 0x0C, 0xFF, 0xFF, 0x, PPC_INTEGER),
 GEN_HANDLER2(addic_, "addic.", 0x0D, 0xFF, 0xFF, 0x, PPC_INTEGER),
 GEN_HANDLER(addis, 0x0F, 0xFF, 0xFF, 0x, PPC_INTEGER),
diff --git a/target/ppc/translate/fixedpoint-impl.c.inc 
b/target/ppc/translate/fixedpoint-impl.c.inc
new file mode 100644
index 00..8620954b57
--- /dev/null
+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -0,0 +1,26 @@
+static bool trans_paddi(DisasContext *ctx, arg_paddi *a)
+{
+if (a->r == 0) {
+if (a->ra == 0) {
+/* li case */
+tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
+} else {
+tcg_gen_addi_tl(cpu_gpr[a->rt],
+cpu_gpr[a->ra], a->si);
+}
+} else {
+if (a->ra == 0) {
+tcg_gen_addi_tl(cpu_gpr[a->rt], cpu_nip, a->si);
+} else {
+/* invalid form */
+gen_invalid(ctx);
+}
+}
+
+return true;
+}
+
+static bool trans_addi(DisasContext *ctx, arg_addi *a)
+{
+return trans_paddi(ctx, a);
+}
-- 
2.25.1

[PATCH 0/5] Base for adding PowerPC 64-bit instructions

[Luis Pires]

This series provides the basic infrastructure for adding the new
32/64-bit instructions in Power ISA 3.1 to target/ppc.

It starts by changing decodetree.py to support 64-bit instructions
as well as custom variable-width instruction load functions.

Then it changes the target/ppc code to allow 32- and 64-bit instructions
to be decoded using decodetree, and finishes by adding the implementation
for 2 simple instructions to demonstrate the new approach:
- addi (replacing the legacy implementation)
- paddi (new)

Luis Pires (5):
  decodetree: Add support for 64-bit instructions
  decodetree: Fix empty input files for varinsnwidth
  decodetree: Allow custom var width load functions
  target/ppc: Base changes to allow 32/64-bit insns
  target/ppc: Implement paddi and replace addi insns

 docs/devel/decodetree.rst  |   5 +-
 scripts/decodetree.py  |  55 --
 target/ppc/cpu.h   |   1 +
 target/ppc/meson.build |   5 +
 target/ppc/ppc.decode  |  26 +++
 target/ppc/translate.c | 206 +++--
 target/ppc/translate/fixedpoint-impl.c.inc |  26 +++
 7 files changed, 250 insertions(+), 74 deletions(-)
 create mode 100644 target/ppc/ppc.decode
 create mode 100644 target/ppc/translate/fixedpoint-impl.c.inc

-- 
2.25.1

[PATCH 4/5] target/ppc: Base changes to allow 32/64-bit insns

[Luis Pires]

These changes add the basic support for 32- and 64-bit instruction
decoding using decodetree.

Apart from the instruction decoding itself, it also takes care of
some pre-requisite changes, such as removing hard-coded instruction
sizes throughout the code and raising an alignment exception should
a prefixed instruction cross a 64-byte boundary.

Signed-off-by: Luis Pires 
---
 target/ppc/cpu.h   |   1 +
 target/ppc/meson.build |   5 ++
 target/ppc/ppc.decode  |  18 
 target/ppc/translate.c | 191 -
 4 files changed, 174 insertions(+), 41 deletions(-)
 create mode 100644 target/ppc/ppc.decode

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e73416da68..9bb2805a22 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -148,6 +148,7 @@ enum {
 POWERPC_EXCP_ALIGN_PROT= 0x04,  /* Access cross protection boundary  */
 POWERPC_EXCP_ALIGN_BAT = 0x05,  /* Access cross a BAT/seg boundary   */
 POWERPC_EXCP_ALIGN_CACHE   = 0x06,  /* Impossible dcbz access*/
+POWERPC_EXCP_ALIGN_INSN= 0x07,  /* Pref. insn x-ing 64-byte boundary */
 /* Exception subtypes for POWERPC_EXCP_PROGRAM   */
 /* FP exceptions */
 POWERPC_EXCP_FP= 0x10,
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index bbfef90e08..feed383a2b 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -1,4 +1,9 @@
+gen = [
+  decodetree.process('ppc.decode', extra_args: ['--varinsnwidth=64', 
'--noloadfunc'])
+]
+
 ppc_ss = ss.source_set()
+ppc_ss.add(gen)
 ppc_ss.add(files(
   'cpu-models.c',
   'cpu.c',
diff --git a/target/ppc/ppc.decode b/target/ppc/ppc.decode
new file mode 100644
index 00..84bb73ac19
--- /dev/null
+++ b/target/ppc/ppc.decode
@@ -0,0 +1,18 @@
+#
+# Power ISA instruction decode definitions.
+#
+# Copyright (c) 2021 Luis Pires 
+#
+# 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 .
+#
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0984ce637b..0f123f7b80 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -154,6 +154,11 @@ void ppc_translate_init(void)
 /* internal defines */
 struct DisasContext {
 DisasContextBase base;
+
+/*
+ * 'opcode' should be considered deprecated and be used only
+ * by legacy non-decodetree code
+ */
 uint32_t opcode;
 uint32_t exception;
 /* Routine used to access memory */
@@ -180,6 +185,8 @@ struct DisasContext {
 uint32_t flags;
 uint64_t insns_flags;
 uint64_t insns_flags2;
+target_ulong insn_size;
+CPUPPCState *env;
 };
 
 /* Return true iff byteswap is needed in a scalar memop */
@@ -199,6 +206,10 @@ static inline bool need_byteswap(const DisasContext *ctx)
 # define NARROW_MODE(C)  0
 #endif
 
+#if defined(DO_PPC_STATISTICS)
+static uint64_t ppc_decodetree_hit_count;
+#endif
+
 struct opc_handler_t {
 /* invalid bits for instruction 1 (Rc(opcode) == 0) */
 uint32_t inval1;
@@ -254,7 +265,7 @@ static void gen_exception_err(DisasContext *ctx, uint32_t 
excp, uint32_t error)
  * faulting instruction
  */
 if (ctx->exception == POWERPC_EXCP_NONE) {
-gen_update_nip(ctx, ctx->base.pc_next - 4);
+gen_update_nip(ctx, ctx->base.pc_next - ctx->insn_size);
 }
 t0 = tcg_const_i32(excp);
 t1 = tcg_const_i32(error);
@@ -273,7 +284,7 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
  * faulting instruction
  */
 if (ctx->exception == POWERPC_EXCP_NONE) {
-gen_update_nip(ctx, ctx->base.pc_next - 4);
+gen_update_nip(ctx, ctx->base.pc_next - ctx->insn_size);
 }
 t0 = tcg_const_i32(excp);
 gen_helper_raise_exception(cpu_env, t0);
@@ -3113,7 +3124,8 @@ static void gen_eieio(DisasContext *ctx)
  */
 if (!(ctx->insns_flags2 & PPC2_ISA300)) {
 qemu_log_mask(LOG_GUEST_ERROR, "invalid eieio using bit 6 at @"
-  TARGET_FMT_lx "\n", ctx->base.pc_next - 4);
+  TARGET_FMT_lx "\n",
+  ctx->base.pc_next - ctx->insn_size);
 } else {
 bar = TCG_MO_ST_LD;
 }
@@ -3782,14 +3794,14 @@ static void gen_b(DisasContext *ctx)
 li = LI(ctx->opcode);
 li = (li ^ 0x0200) - 0x0200;
 if 

[PATCH 2/5] decodetree: Fix empty input files for varinsnwidth

[Luis Pires]

Decodetree would throw an error when the input file was empty
and --varinsnwidth was specified.

Signed-off-by: Luis Pires 
---
 scripts/decodetree.py | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 4e18f52a65..935b2964e0 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1178,11 +1178,12 @@ def output_code(self, i, extracted, outerbits, 
outermask):
 ind = str_indent(i)
 
 # If we need to load more bytes, do so now.
-if extracted < self.width:
-output(ind, 'insn = ', decode_function,
-   '_load_bytes(ctx, insn, {0}, {1});\n'
-   .format(extracted // 8, self.width // 8));
-extracted = self.width
+if self.width is not None:
+if extracted < self.width:
+output(ind, 'insn = ', decode_function,
+   '_load_bytes(ctx, insn, {0}, {1});\n'
+   .format(extracted // 8, self.width // 8));
+extracted = self.width
 output(ind, 'return insn;\n')
 # end SizeLeaf
 
-- 
2.25.1

[PATCH 1/5] decodetree: Add support for 64-bit instructions

[Luis Pires]

Allow '64' to be specified for the instruction width command line params
and use the appropriate insn/field data types, mask, extract and deposit
functions in that case.

This will be used to implement the new 64-bit Power ISA 3.1 instructions.

Signed-off-by: Luis Pires 
---
 docs/devel/decodetree.rst |  5 +++--
 scripts/decodetree.py | 26 +-
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/docs/devel/decodetree.rst b/docs/devel/decodetree.rst
index 74f66bf46e..d776dae14f 100644
--- a/docs/devel/decodetree.rst
+++ b/docs/devel/decodetree.rst
@@ -40,8 +40,9 @@ and returns an integral value extracted from there.
 
 A field with no ``unnamed_fields`` and no ``!function`` is in error.
 
-FIXME: the fields of the structure into which this result will be stored
-is restricted to ``int``.  Which means that we cannot expand 64-bit items.
+The fields of the structure into which this result will be stored are
+defined as ``int`` when the instruction size is set to 16 or 32 bits
+and as ``int64_t`` when the instruction size is set to 64 bits.
 
 Field examples:
 
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 4637b633e7..4e18f52a65 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -42,6 +42,10 @@
 output_fd = None
 insntype = 'uint32_t'
 decode_function = 'decode'
+field_data_type = 'int'
+extract_function = 'extract32'
+sextract_function = 'sextract32'
+deposit_function = 'deposit32'
 
 # An identifier for C.
 re_C_ident = '[a-zA-Z][a-zA-Z0-9_]*'
@@ -185,9 +189,9 @@ def __str__(self):
 
 def str_extract(self):
 if self.sign:
-extr = 'sextract32'
+extr = sextract_function
 else:
-extr = 'extract32'
+extr = extract_function
 return '{0}(insn, {1}, {2})'.format(extr, self.pos, self.len)
 
 def __eq__(self, other):
@@ -215,8 +219,9 @@ def str_extract(self):
 if pos == 0:
 ret = f.str_extract()
 else:
-ret = 'deposit32({0}, {1}, {2}, {3})' \
-  .format(ret, pos, 32 - pos, f.str_extract())
+ret = '{4}({0}, {1}, {2}, {3})' \
+  .format(ret, pos, insnwidth - pos, f.str_extract(),
+  deposit_function)
 pos += f.len
 return ret
 
@@ -311,7 +316,7 @@ def output_def(self):
 if not self.extern:
 output('typedef struct {\n')
 for n in self.fields:
-output('int ', n, ';\n')
+output('', field_data_type, ' ', n, ';\n')
 output('} ', self.struct_name(), ';\n\n')
 # end Arguments
 
@@ -1264,6 +1269,10 @@ def main():
 global insntype
 global insnmask
 global decode_function
+global extract_function
+global sextract_function
+global deposit_function
+global field_data_type
 global variablewidth
 global anyextern
 
@@ -1293,6 +1302,13 @@ def main():
 if insnwidth == 16:
 insntype = 'uint16_t'
 insnmask = 0x
+elif insnwidth == 64:
+insntype = 'uint64_t'
+insnmask = 0x
+field_data_type = 'int64_t'
+extract_function = 'extract64'
+sextract_function = 'sextract64'
+deposit_function = 'deposit64'
 elif insnwidth != 32:
 error(0, 'cannot handle insns of width', insnwidth)
 else:
-- 
2.25.1

Re: [PULL 0/1] Block patch for 6.0-rc3

[Peter Maydell]

On Tue, 13 Apr 2021 at 14:39, Max Reitz  wrote:
>
> The following changes since commit dce628a97fde2594f99d738883a157f05aa0a14f:
>
>   Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.0-20210412' 
> into staging (2021-04-13 13:05:07 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/XanClic/qemu.git tags/pull-block-2021-04-13
>
> for you to fetch changes up to 0267101af64292c9a84fd9319a763ddfbce9ddc7:
>
>   block/nbd: fix possible use after free of s->connect_thread (2021-04-13 
> 15:35:12 +0200)
>
> (This is the patch for which Vladimir has sent a pull request yesterday.)
>
> 
> Block patches for 6.0-rc3:
> - Use-after-free fix for block/nbd.c
>
> 
> Vladimir Sementsov-Ogievskiy (1):
>   block/nbd: fix possible use after free of s->connect_thread


Applied, thanks.

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

-- PMM

Re: [PATCH] linux-user/elfload: fix filling psinfo->pr_psargs

[no-reply]

Patchew URL: https://patchew.org/QEMU/20210413205814.22821-1-...@linux.ibm.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210413205814.22821-1-...@linux.ibm.com
Subject: [PATCH] linux-user/elfload: fix filling psinfo->pr_psargs

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20210413205814.22821-1-...@linux.ibm.com -> 
patchew/20210413205814.22821-1-...@linux.ibm.com
Switched to a new branch 'test'
01536fa linux-user/elfload: fix filling psinfo->pr_psargs

=== OUTPUT BEGIN ===
ERROR: braces {} are necessary for all arms of this statement
#35: FILE: linux-user/elfload.c:3635:
+if (copy_from_user(>pr_psargs, ts->info->arg_strings, len))
[...]

total: 1 errors, 0 warnings, 12 lines checked

Commit 01536fa0c80b (linux-user/elfload: fix filling psinfo->pr_psargs) has 
style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210413205814.22821-1-...@linux.ibm.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH] linux-user/elfload: fix filling psinfo->pr_psargs

[Ilya Leoshkevich]

The current code dumps the memory between arg_start and arg_end,
which contains the argv pointers. This results in the

Core was generated by ``

message when opening the core file in GDB. This is because the code is
supposed to dump the actual arg strings. Fix by using arg_strings and
env_strings instead of arg_start and arg_end.

Signed-off-by: Ilya Leoshkevich 
---
 linux-user/elfload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 4e45bd1539..cffcebfe45 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3662,10 +3662,10 @@ static int fill_psinfo(struct target_elf_prpsinfo 
*psinfo, const TaskState *ts)
 
 (void) memset(psinfo, 0, sizeof (*psinfo));
 
-len = ts->info->arg_end - ts->info->arg_start;
+len = ts->info->env_strings - ts->info->arg_strings;
 if (len >= ELF_PRARGSZ)
 len = ELF_PRARGSZ - 1;
-if (copy_from_user(>pr_psargs, ts->info->arg_start, len))
+if (copy_from_user(>pr_psargs, ts->info->arg_strings, len))
 return -EFAULT;
 for (i = 0; i < len; i++)
 if (psinfo->pr_psargs[i] == 0)
-- 
2.29.2

[PATCH] linux-user/elfload: add s390x core dumping support

[Ilya Leoshkevich]

Provide the following definitions required by the common code:

* ELF_NREG: with the value of sizeof(s390_regs) / sizeof(long).
* target_elf_gregset_t: define it like all the other arches do.
* elf_core_copy_regs(): similar to kernel's s390_regs_get().
* USE_ELF_CORE_DUMP.
* ELF_EXEC_PAGESIZE.

Signed-off-by: Ilya Leoshkevich 
---
 linux-user/elfload.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c6731013fd..4e45bd1539 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1385,6 +1385,39 @@ static inline void init_thread(struct target_pt_regs 
*regs, struct image_info *i
 regs->gprs[15] = infop->start_stack;
 }
 
+/* See linux kernel: arch/s390/include/uapi/asm/ptrace.h (s390_regs).  */
+#define ELF_NREG 27
+typedef target_elf_greg_t target_elf_gregset_t[ELF_NREG];
+
+enum {
+TARGET_REG_PSWM = 0,
+TARGET_REG_PSWA = 1,
+TARGET_REG_GPRS = 2,
+TARGET_REG_ARS = 18,
+TARGET_REG_ORIG_R2 = 26,
+};
+
+static void elf_core_copy_regs(target_elf_gregset_t *regs,
+   const CPUS390XState *env)
+{
+int i;
+uint32_t *aregs;
+
+(*regs)[TARGET_REG_PSWM] = tswapreg(env->psw.mask);
+(*regs)[TARGET_REG_PSWA] = tswapreg(env->psw.addr);
+for (i = 0; i < 16; i++) {
+(*regs)[TARGET_REG_GPRS + i] = tswapreg(env->regs[i]);
+}
+aregs = (uint32_t *)&((*regs)[TARGET_REG_ARS]);
+for (i = 0; i < 16; i++) {
+aregs[i] = tswap32(env->aregs[i]);
+}
+(*regs)[TARGET_REG_ORIG_R2] = 0;
+}
+
+#define USE_ELF_CORE_DUMP
+#define ELF_EXEC_PAGESIZE 4096
+
 #endif /* TARGET_S390X */
 
 #ifdef TARGET_RISCV
-- 
2.29.2

Re: [RFC v12 03/65] arm: tcg: only build under CONFIG_TCG

[Philippe Mathieu-Daudé]

On 3/26/21 8:35 PM, Claudio Fontana wrote:
> Signed-off-by: Claudio Fontana 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Alex Bennée 
> ---
>  target/arm/tcg/meson.build | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/arm/tcg/meson.build b/target/arm/tcg/meson.build
> index 0bd4e9d954..3b4146d079 100644
> --- a/target/arm/tcg/meson.build
> +++ b/target/arm/tcg/meson.build
> @@ -12,9 +12,9 @@ gen = [
>decodetree.process('t16.decode', extra_args: ['-w', '16', 
> '--static-decode=disas_t16']),
>  ]
>  
> -arm_ss.add(gen)
> +arm_ss.add(when: 'CONFIG_TCG', if_true: gen)
>  
> -arm_ss.add(files(
> +arm_ss.add(when: 'CONFIG_TCG', if_true: files(
>'translate.c',
>'helper.c',
>'iwmmxt_helper.c',
> @@ -28,7 +28,7 @@ arm_ss.add(files(
>'debug_helper.c',
>  ))
>  
> -arm_ss.add(when: 'TARGET_AARCH64', if_true: files(
> +arm_ss.add(when: ['TARGET_AARCH64','CONFIG_TCG'], if_true: files(
>'translate-a64.c',
>'translate-sve.c',
>'helper-a64.c',
> 

Isn't it clearer to use in target/arm/meson.build:

if 'CONFIG_TCG' in config_all
  subdir('tcg')
endif

?

Similarly in the next patch for target/arm/tcg/meson.build:

if have_user
  subdir('user')
endif
if have_system
  subdir('sysemu')
endif

Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3

[Peter Maydell]

On Tue, 13 Apr 2021 at 17:18, Paolo Bonzini  wrote:
>
> The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 
> 12:12:09 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241:
>
>   qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 
> 18:04:23 +0200)
>
> 
> * Fix C++ compilation of qemu/osdep.h.
> * Fix -object cryptodev-vhost-user
>
> 
> Paolo Bonzini (2):
>   osdep: include glib-compat.h before other QEMU headers
>   osdep: protect qemu/osdep.h with extern "C"
>
> Thomas Huth (1):
>   qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code

This still seems to have the same version of patch 2 that I gave
review comments on, and which you haven't replied to ?

thanks
-- PMM

[Bug 1923663] [NEW] Can't(?) disable default floppy drive any more in qemu 6.0

[Adam Williamson]

Public bug reported:

There's a documented change in qemu 6.0:

https://qemu-project.gitlab.io/qemu/system/removed-features.html#floppy-
controllers-drive-properties-removed-in-6-0

where you can't configure floppy controller device properties with
-global any more. However, there's a thing you could do with the old
parameter which I can't figure out a way to do with the documented
replacement. openQA passed exactly this argument:

-global isa-fdc.driveA=

and that has the effect of removing/disabling the default floppy
drive/controller. If you just run `qemu-system-i686` (no other args)
you'll see the VM briefly try to boot from a floppy drive; if you run
`qemu-system-i686 -global isa-fdc.driveA=` (with an earlier version of
qemu, obviously) you'll see it does not do so.

I can't see a way to do this with `-device floppy`. Going by the docs,
the equivalent should be:

-device floppy,unit=0,drive=

but that does not seem to have the same effect. If you run `qemu-
system-i686 -device floppy,unit=0,drive=`, it still tries to boot from a
floppy drive.

I see there's a -nodefaults option that disables *all* default devices,
but I don't think that's what we want here either. We might want the
other default devices, we just don't want the floppy drive.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1923663

Title:
  Can't(?) disable default floppy drive any more in qemu 6.0

Status in QEMU:
  New

Bug description:
  There's a documented change in qemu 6.0:

  https://qemu-project.gitlab.io/qemu/system/removed-features.html
  #floppy-controllers-drive-properties-removed-in-6-0

  where you can't configure floppy controller device properties with
  -global any more. However, there's a thing you could do with the old
  parameter which I can't figure out a way to do with the documented
  replacement. openQA passed exactly this argument:

  -global isa-fdc.driveA=

  and that has the effect of removing/disabling the default floppy
  drive/controller. If you just run `qemu-system-i686` (no other args)
  you'll see the VM briefly try to boot from a floppy drive; if you run
  `qemu-system-i686 -global isa-fdc.driveA=` (with an earlier version of
  qemu, obviously) you'll see it does not do so.

  I can't see a way to do this with `-device floppy`. Going by the docs,
  the equivalent should be:

  -device floppy,unit=0,drive=

  but that does not seem to have the same effect. If you run `qemu-
  system-i686 -device floppy,unit=0,drive=`, it still tries to boot from
  a floppy drive.

  I see there's a -nodefaults option that disables *all* default
  devices, but I don't think that's what we want here either. We might
  want the other default devices, we just don't want the floppy drive.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1923663/+subscriptions

Re: [PATCH for-6.0] x86: acpi: use offset instead of pointer when using build_header()

[Michael S. Tsirkin]

On Tue, Apr 13, 2021 at 05:21:10PM +0200, Igor Mammedov wrote:
> On Tue, 13 Apr 2021 09:53:17 -0400
> "Michael S. Tsirkin"  wrote:
> 
> > On Tue, Apr 13, 2021 at 03:18:16PM +0200, Igor Mammedov wrote:
> > > On Tue, 13 Apr 2021 08:14:56 -0400
> > > "Michael S. Tsirkin"  wrote:
> > >   
> > > > On Tue, Apr 13, 2021 at 07:14:00AM -0400, Igor Mammedov wrote:  
> > > > > Do the same as in commit
> > > > >  (4d027afeb3a97 Virt: ACPI: fix qemu assert due to re-assigned table 
> > > > > data address)  
> > 
> > Format:
> > 
> > commit 4d027afeb3a97 ("Virt: ACPI: fix qemu assert due to re-assigned table 
> > data address")
> > 
> > > > > for remaining tables that happen to use saved at
> > > > > the beginning pointer to build header to avoid assert
> > > > > when table_data is relocated due to implicit re-size.
> > > > > 
> > > > > Reported-in: https://bugs.launchpad.net/bugs/1923497
> > > > 
> > > > Doesn't this fix the bug? If so -
> > > > Isn't this Fixes: ?  
> > > that's buried in history
> > > 
> > > Fixes: 243bdb79fb0b2ed hw/arm/virt-acpi-build: Generate RSDT table
> > > Fixes: cb51ac2ffe3649e hw/arm/virt: generate 64-bit addressable ACPI 
> > > objects
> > > Fixes: 4338416064303aa acpi: Move build_tpm2() in the generic part
> > > Fixes: 72c194f7e75cb64 i386: ACPI table generation code from seabios
> > > Fixes: 711b20b479aa96e Add ACPI tables for TPM  
> > 
> > 
> > I just mean:
> > 
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1921138
> > 
> > as opposed to Reported.
> > 
> > Also do we CC stable for this?
> Given it's reported on 5.1, it is a good idea to cc stable.
> 
> Shall I repost with fixed: "Fixes:..."?


Yes. If you can copy problem description from launchpad/ARM
commit into commit log, that's also a good idea.

> > 
> > >
> > > > > Signed-off-by: Igor Mammedov   
> > 
> > 
> > patch itself ok:
> > 
> > Acked-by: Michael S. Tsirkin 
> > 
> > > > > ---
> > > > > PS:
> > > > >  I have build_header() refactoring patch that requires offset
> > > > >  instead of pointer, to make it harder to misuse but it's
> > > > >  a bit intrusive for last minute fixes. So here goes simplified
> > > > >  variant, and I'll post refactoring patch for 6.1. later.
> > > > > ---
> > > > >  hw/acpi/aml-build.c  | 15 +--
> > > > >  hw/i386/acpi-build.c |  8 ++--
> > > > >  2 files changed, 15 insertions(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> > > > > index d33ce8954a..f0035d2b4a 100644
> > > > > --- a/hw/acpi/aml-build.c
> > > > > +++ b/hw/acpi/aml-build.c
> > > > > @@ -1830,6 +1830,7 @@ build_rsdt(GArray *table_data, BIOSLinker 
> > > > > *linker, GArray *table_offsets,
> > > > >  int i;
> > > > >  unsigned rsdt_entries_offset;
> > > > >  AcpiRsdtDescriptorRev1 *rsdt;
> > > > > +int rsdt_start = table_data->len;
> > > > >  const unsigned table_data_len = (sizeof(uint32_t) * 
> > > > > table_offsets->len);
> > > > >  const unsigned rsdt_entry_size = 
> > > > > sizeof(rsdt->table_offset_entry[0]);
> > > > >  const size_t rsdt_len = sizeof(*rsdt) + table_data_len;
> > > > > @@ -1846,7 +1847,8 @@ build_rsdt(GArray *table_data, BIOSLinker 
> > > > > *linker, GArray *table_offsets,
> > > > >  ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
> > > > >  }
> > > > >  build_header(linker, table_data,
> > > > > - (void *)rsdt, "RSDT", rsdt_len, 1, oem_id, 
> > > > > oem_table_id);
> > > > > + (void *)(table_data->data + rsdt_start),
> > > > > + "RSDT", rsdt_len, 1, oem_id, oem_table_id);
> > > > >  }
> > > > >  
> > > > >  /* Build xsdt table */
> > > > > @@ -1857,6 +1859,7 @@ build_xsdt(GArray *table_data, BIOSLinker 
> > > > > *linker, GArray *table_offsets,
> > > > >  int i;
> > > > >  unsigned xsdt_entries_offset;
> > > > >  AcpiXsdtDescriptorRev2 *xsdt;
> > > > > +int xsdt_start = table_data->len;
> > > > >  const unsigned table_data_len = (sizeof(uint64_t) * 
> > > > > table_offsets->len);
> > > > >  const unsigned xsdt_entry_size = 
> > > > > sizeof(xsdt->table_offset_entry[0]);
> > > > >  const size_t xsdt_len = sizeof(*xsdt) + table_data_len;
> > > > > @@ -1873,7 +1876,8 @@ build_xsdt(GArray *table_data, BIOSLinker 
> > > > > *linker, GArray *table_offsets,
> > > > >  ACPI_BUILD_TABLE_FILE, ref_tbl_offset);
> > > > >  }
> > > > >  build_header(linker, table_data,
> > > > > - (void *)xsdt, "XSDT", xsdt_len, 1, oem_id, 
> > > > > oem_table_id);
> > > > > + (void *)(table_data->data + xsdt_start),
> > > > > + "XSDT", xsdt_len, 1, oem_id, oem_table_id);
> > > > >  }
> > > > >  
> > > > >  void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t 
> > > > > base,
> > > > > @@ -2053,10 +2057,9 @@ void build_tpm2(GArray *table_data, BIOSLinker 
> > > > > *linker, GArray *tcpalog,
> > > > >  uint64_t control_area_start_address;
> > > > >  TPMIf 

Re: [PATCH RFC 4/7] message: add QMP Message type

[Stefan Hajnoczi]

On Tue, Apr 13, 2021 at 11:55:50AM -0400, John Snow wrote:
> This is an abstraction that represents a single message either sent to
> or received from the server. It is used to subclass the
> AsyncProtocol(Generic[T]) type.
> 
> It was written such that it can be populated by either raw data or by a
> dict, with the other form being generated on-demand, as-needed.
> 
> It behaves almost exactly like a dict, but has some extra methods and a
> special constructor. (It should quack fairly convincingly.)
> 
> Signed-off-by: John Snow 
> ---
>  message.py | 196 +
>  1 file changed, 196 insertions(+)
>  create mode 100644 message.py
> 
> diff --git a/message.py b/message.py
> new file mode 100644
> index 000..5c7e828
> --- /dev/null
> +++ b/message.py
> @@ -0,0 +1,196 @@
> +"""
> +QMP Message format and errors.
> +
> +This module provides the `Message` class, which represents a single QMP
> +message sent to or from the server. Several error-classes that depend on
> +knowing the format of this message are also included here.
> +"""
> +
> +import json
> +from json import JSONDecodeError
> +from typing import (
> +Dict,
> +ItemsView,
> +Iterable,
> +KeysView,
> +Optional,
> +Union,
> +ValuesView,
> +)
> +
> +from error import (
> +DeserializationError,
> +ProtocolError,
> +UnexpectedTypeError,
> +)
> +
> +
> +class Message:
> +"""
> +Represents a single QMP protocol message.
> +
> +QMP uses JSON objects as its basic communicative unit; so this
> +object behaves like a MutableMapping. It may be instantiated from
> +either another mapping (like a dict), or from raw bytes that still
> +need to be deserialized.
> +
> +:param value: Initial value, if any.
> +:param eager: When true, attempt to serialize (or deserialize) the
> +  initial value immediately, such that conversion exceptions
> +  are raised during the call to the initialization method.
> +"""

Why define this class instead of using dicts? It's a very fancy way of
calling json.dumps() and json.loads().


signature.asc
Description: PGP signature

Re: [PULL 0/3] target-arm queue

[Peter Maydell]

On Tue, 13 Apr 2021 at 13:07, Peter Maydell  wrote:
>
> A few last patches to go in for rc3...
>
> The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging (2021-04-12 
> 12:12:09 +0100)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20210413
>
> for you to fetch changes up to 2d18b4ca023ca1a3aee18064251d6e6e1084f3eb:
>
>   sphinx: qapidoc: Wrap "If" section body in a paragraph node (2021-04-13 
> 10:14:58 +0100)
>
> 
> target-arm queue:
>  * Fix MPC setting for AN524 SRAM block
>  * sphinx: qapidoc: Wrap "If" section body in a paragraph node
>


Applied, thanks.

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

-- PMM

Re: [PATCH RFC 3/7] protocol: generic async message-based protocol loop

[Stefan Hajnoczi]

On Tue, Apr 13, 2021 at 11:55:49AM -0400, John Snow wrote:
> This module provides the protocol-agnostic framework upon which QMP will
> be built. I also have (not included in this series) a qtest
> implementation that uses this same framework, which is why it is split
> into two portions like this.
> 
> The design uses two independent tasks in the "bottol half", a writer and
> a reader. These tasks run for the duration of the connection and
> independently send and receive messages, respectively.
> 
> A third task, disconnect, is scheduled whenever an error occurs and
> facilitates coalescing of the other two tasks. MultiException is used in
> this case if *both* tasks should have Exceptions that need to be
> reported, though at the time of writing, I think this circumstance might
> only be a theoretical concern.
> 
> The generic model here does not provide execute(), but the model for QMP
> is informative for how this class is laid out. Below, QMP's execute()
> function deposits a message into the outbound queue. The writer task
> wakes up to process the queue and deposits information in the write
> buffer, where the message is finally dispatched. Meanwhile, the
> execute() call is expected to block on an RPC mailbox waiting for a
> reply from the server.
> 
> On the return trip, the reader wakes up when data arrives in the
> buffer. The message is deserialized and handed off to the protocol layer
> to route accordingly. QMP will route this message into either the Event
> queue or one of the pending RPC mailboxes.
> 
> Upon this message being routed to the correct RPC mailbox, execute()
> will be woken up and allowed to process the reply and deliver it back to
> the caller.
> 
> The reason for separating the inbound and outbound tasks to such an
> extreme degree is to allow for designs and extensions where this
> asynchronous loop may be launched in a separate thread. In this model,
> it is possible to use a synchronous, thread-safe function to deposit new
> messages into the outbound queue; this was seen as a viable way to offer
> solid synchronous bindings while still allowing events to be processed
> truly asynchronously.
> 
> Separating it this way also allows us to fairly easily support
> Out-of-band executions with little additional effort; essentially all
> commands are treated as out-of-band.
> 
> The execute graph:
> 
>+-+
>| caller  |
>+-+
> |
> v
>+-+
>  + |execute()| <--+
>  | +-+|
>  ||
> ---
>  v|
> ++++---+   +--+---+
> |Mailboxes||Event Queue|   |Outbound Queue|
> ++++--++   +--+---+
>  ||   ^
>  vv   |
>   +--++---+   +---+---+
>   | Reader Task/Coroutine |   | Writer Task/Coroutine |
>   +---+---+   +---+---+
>   |   ^
>   v   |
> +-+--+  +-+--+
> |StreamReader|  |StreamWriter|
> ++  ++

The arrow directions confuse me. I don't understand what they convey.

> 
> Signed-off-by: John Snow 
> ---
>  protocol.py | 704 

Yikes, this is complex. I'm not sure the abstractions are worth the
cost. Hopefully everything will be tied up with a simple high-level API
later in the series.

>  1 file changed, 704 insertions(+)
>  create mode 100644 protocol.py
> 
> diff --git a/protocol.py b/protocol.py
> new file mode 100644
> index 000..27d1558
> --- /dev/null
> +++ b/protocol.py
> @@ -0,0 +1,704 @@
> +"""
> +Async message-based protocol support.
> +
> +This module provides a generic framework for sending and receiving
> +messages over an asyncio stream.
> +
> +`AsyncProtocol` is an abstract class that implements the core mechanisms
> +of a simple send/receive protocol, and is designed to be extended.
> +
> +`AsyncTasks` provides a container class that aggregates tasks that make
> +up the loop used by `AsyncProtocol`.
> +"""
> +
> +import asyncio
> +from asyncio import StreamReader, StreamWriter
> +import logging
> +from ssl import SSLContext
> +from typing import (
> +Any,
> +Awaitable,
> +Callable,
> +Coroutine,
> +Iterator,
> +List,
> +Generic,
> +Optional,
> +Tuple,
> +TypeVar,
> +Union,
> +)
> +
> +from error import (
> +ConnectError,
> +MultiException,
> +StateError,
> +)
> +from 

Re: [RFC PATCH-for-6.1 0/9] hw/clock: Strengthen machine (non-qdev) clock propagation

[Eduardo Habkost]

On Mon, Apr 12, 2021 at 11:44:29AM +0100, Peter Maydell wrote:
> On Mon, 12 Apr 2021 at 11:31, Philippe Mathieu-Daudé  wrote:
> > TIL MachineClass::reset().
> >
> > - hw/hppa/machine.c
> > - hw/i386/pc.c
> >
> >   Used to reset CPUs manually because CPUs aren't sysbus-reset.
> 
> pc_machine_reset() is not resetting the CPUs -- it is
> re-resetting the APIC devices, which looks like it is a
> workaround for a reset-ordering or other problem. I'm not
> sure where the CPUs are being reset...

CPU reset code was moved from pc.c:pc_cpu_reset() to
cpu.c:x86_cpu_machine_reset_cb() in commit 65dee3805259
("target-i386: move cpu_reset and reset callback to cpu.c").
It's not clear to me why.

-- 
Eduardo

RE: [PATCH] decodetree: Allow empty input files for var width

[Luis Fernando Fujita Pires]

Please ignore this. I'll resend as part of a patch series.

Luis Pires
Instituto de Pesquisas ELDORADO
Departamento de Computação Embarcada
Aviso Legal - Disclaimer


-Original Message-
From: Luis Pires  
Sent: terça-feira, 13 de abril de 2021 15:10
To: qemu-devel@nongnu.org
Cc: richard.hender...@linaro.org; Luis Fernando Fujita Pires 

Subject: [PATCH] decodetree: Allow empty input files for var width

This was broken when varinsnwidth was specified.

Signed-off-by: Luis Pires 
---
 scripts/decodetree.py | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py index 
3450a2a08d..fef5eeaf42 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1177,11 +1177,12 @@ def output_code(self, i, extracted, outerbits, 
outermask):
 ind = str_indent(i)
 
 # If we need to load more bytes, do so now.
-if extracted < self.width:
-output(ind, 'insn = ', decode_function,
-   '_load_bytes(ctx, insn, {0}, {1});\n'
-   .format(extracted // 8, self.width // 8));
-extracted = self.width
+if self.width is not None:
+if extracted < self.width:
+output(ind, 'insn = ', decode_function,
+   '_load_bytes(ctx, insn, {0}, {1});\n'
+   .format(extracted // 8, self.width // 8));
+extracted = self.width
 output(ind, 'return insn;\n')
 # end SizeLeaf
 
--
2.25.1

RE: [PATCH] decodetree: Allow custom var width load functions

[Luis Fernando Fujita Pires]

Please ignore this. I'll resend as part of a patch series.

-Original Message-
From: Luis Pires  
Sent: terça-feira, 13 de abril de 2021 15:16
To: qemu-devel@nongnu.org
Cc: richard.hender...@linaro.org; qemu-...@nongnu.org; Luis Fernando Fujita 
Pires 
Subject: [PATCH] decodetree: Allow custom var width load functions

This is useful in situations where you want decodetree to handle variable width 
instructions but you want to provide custom code to load the instructions. 
Suppressing the generation of the load function is necessary to avoid 
compilation errors due to the load function being unused.

This will be used by the PowerPC decodetree code.

Signed-off-by: Luis Pires 
---
 scripts/decodetree.py | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py index 
fef5eeaf42..c88dbdb4c3 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1277,9 +1277,10 @@ def main():
 global anyextern
 
 decode_scope = 'static '
+noloadfunc = False
 
 long_opts = ['decode=', 'translate=', 'output=', 'insnwidth=',
- 'static-decode=', 'varinsnwidth=']
+ 'static-decode=', 'varinsnwidth=', 'noloadfunc']
 try:
 (opts, args) = getopt.gnu_getopt(sys.argv[1:], 'o:vw:', long_opts)
 except getopt.GetoptError as err:
@@ -1311,6 +1312,8 @@ def main():
 deposit_function = 'deposit64'
 elif insnwidth != 32:
 error(0, 'cannot handle insns of width', insnwidth)
+elif o == '--noloadfunc':
+noloadfunc = True
 else:
 assert False, 'unhandled option'
 
@@ -1400,12 +1403,13 @@ def main():
 output(i4, 'return false;\n')
 output('}\n')
 
-if variablewidth:
-output('\n', decode_scope, insntype, ' ', decode_function,
-   '_load(DisasContext *ctx)\n{\n',
-   '', insntype, ' insn = 0;\n\n')
-stree.output_code(4, 0, 0, 0)
-output('}\n')
+if not noloadfunc:
+if variablewidth:
+output('\n', decode_scope, insntype, ' ', decode_function,
+   '_load(DisasContext *ctx)\n{\n',
+   '', insntype, ' insn = 0;\n\n')
+stree.output_code(4, 0, 0, 0)
+output('}\n')
 
 if output_file:
 output_fd.close()
--
2.25.1

Re: [PATCH] target/i386: Add CPU model versions supporting 'xsaves'

[Eduardo Habkost]

On Mon, Apr 12, 2021 at 09:39:52AM +0200, Vitaly Kuznetsov wrote:
> Hyper-V 2016 refuses to boot on Skylake+ CPU models because they lack
> 'xsaves'/'vmx-xsaves' features and this diverges from real hardware. The
> same issue emerges with AMD "EPYC" CPU model prior to version 3 which got
> 'xsaves' added. EPYC-Rome/EPYC-Milan CPU models have 'xsaves' enabled from
> the very beginning so the comment blaming KVM to explain why other CPUs
> lack 'xsaves' is likely outdated.
> 
> Signed-off-by: Vitaly Kuznetsov 

I'm queueing this for 6.1.  Thanks!

-- 
Eduardo

[PATCH] decodetree: Allow empty input files for var width

[Luis Pires]

This was broken when varinsnwidth was specified.

Signed-off-by: Luis Pires 
---
 scripts/decodetree.py | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 3450a2a08d..fef5eeaf42 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1177,11 +1177,12 @@ def output_code(self, i, extracted, outerbits, 
outermask):
 ind = str_indent(i)
 
 # If we need to load more bytes, do so now.
-if extracted < self.width:
-output(ind, 'insn = ', decode_function,
-   '_load_bytes(ctx, insn, {0}, {1});\n'
-   .format(extracted // 8, self.width // 8));
-extracted = self.width
+if self.width is not None:
+if extracted < self.width:
+output(ind, 'insn = ', decode_function,
+   '_load_bytes(ctx, insn, {0}, {1});\n'
+   .format(extracted // 8, self.width // 8));
+extracted = self.width
 output(ind, 'return insn;\n')
 # end SizeLeaf
 
-- 
2.25.1

[PATCH] decodetree: Allow custom var width load functions

[Luis Pires]

This is useful in situations where you want decodetree
to handle variable width instructions but you want to
provide custom code to load the instructions. Suppressing
the generation of the load function is necessary to avoid
compilation errors due to the load function being unused.

This will be used by the PowerPC decodetree code.

Signed-off-by: Luis Pires 
---
 scripts/decodetree.py | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index fef5eeaf42..c88dbdb4c3 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -1277,9 +1277,10 @@ def main():
 global anyextern
 
 decode_scope = 'static '
+noloadfunc = False
 
 long_opts = ['decode=', 'translate=', 'output=', 'insnwidth=',
- 'static-decode=', 'varinsnwidth=']
+ 'static-decode=', 'varinsnwidth=', 'noloadfunc']
 try:
 (opts, args) = getopt.gnu_getopt(sys.argv[1:], 'o:vw:', long_opts)
 except getopt.GetoptError as err:
@@ -1311,6 +1312,8 @@ def main():
 deposit_function = 'deposit64'
 elif insnwidth != 32:
 error(0, 'cannot handle insns of width', insnwidth)
+elif o == '--noloadfunc':
+noloadfunc = True
 else:
 assert False, 'unhandled option'
 
@@ -1400,12 +1403,13 @@ def main():
 output(i4, 'return false;\n')
 output('}\n')
 
-if variablewidth:
-output('\n', decode_scope, insntype, ' ', decode_function,
-   '_load(DisasContext *ctx)\n{\n',
-   '', insntype, ' insn = 0;\n\n')
-stree.output_code(4, 0, 0, 0)
-output('}\n')
+if not noloadfunc:
+if variablewidth:
+output('\n', decode_scope, insntype, ' ', decode_function,
+   '_load(DisasContext *ctx)\n{\n',
+   '', insntype, ' insn = 0;\n\n')
+stree.output_code(4, 0, 0, 0)
+output('}\n')
 
 if output_file:
 output_fd.close()
-- 
2.25.1

Re: [PATCH v2] vhost-user-blk: Fail gracefully on too large queue size

[Raphael Norwitz]

On Tue, Apr 13, 2021 at 06:56:54PM +0200, Kevin Wolf wrote:
> virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so
> vhost_user_blk_device_realize() should check this before calling it.
> 
> Simple reproducer:
> 
> qemu-system-x86_64 \
> -chardev null,id=foo \
> -device vhost-user-blk-pci,queue-size=4096,chardev=foo
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935014
> Signed-off-by: Kevin Wolf 
> ---
>  hw/block/vhost-user-blk.c | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Raphael Norwitz 

> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 0b5b9d44cd..f5e9682703 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -467,6 +467,11 @@ static void vhost_user_blk_device_realize(DeviceState 
> *dev, Error **errp)
>  error_setg(errp, "vhost-user-blk: queue size must be non-zero");
>  return;
>  }
> +if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
> +error_setg(errp, "vhost-user-blk: queue size must not exceed %d",
> +   VIRTQUEUE_MAX_SIZE);
> +return;
> +}
>  
>  if (!vhost_user_init(>vhost_user, >chardev, errp)) {
>  return;
> -- 
> 2.30.2
> 

[Bug 1923648] [NEW] macOS App Nap feature gradually freezes QEMU process

[Vasiliy Nikonov]

Public bug reported:

macOS version: 10.15.2
QEMU versions: 5.2.0 (from MacPorts)
   5.2.92 (v6.0.0-rc2-23-g9692c7b037)

If the QEMU window is not visible (hidden, minimized or another
application is in full screen mode), the QEMU process gradually freezes:
it still runs, but the VM does not respond to external requests such as
Telnet or SSH until the QEMU window is visible on the desktop.

This behavior is due to the work of the macOS App Nap function:
https://developer.apple.com/library/archive/documentation/Performance/Conceptual/power_efficiency_guidelines_osx/AppNap.html#//apple_ref/doc/uid/TP40013929-CH2-SW1

It doesn't matter how the process is started -- as a background job or
as a foreground shell process in case QEMU has a desktop window.

My VM does not have a display output, only a serial line, most likely if
the VM was using OpenGL, or playing sound (or any other App Nap
triggers), then the problem would never have been detected.

In my case only one starting way without this problem:
sudo qemu-system-x86_64 -nodefaults \
-cpu host -accel hvf -smp 1 -m 384 \
-device virtio-blk-pci,drive=flash0 \
-drive 
file=/vios-adventerprisek9-m.vmdk.SPA.156-1.T.vmdk,if=none,format=vmdk,id=flash0
 \
-device e1000,netdev=local -netdev 
tap,id=local,ifname=tap0,script=no,downscript=no \
-serial stdio -display none

The typical way from the internet to disable App Nap doesn't work:
defaults write NSGlobalDomain NSAppSleepDisabled -bool YES

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: app-nap freezes macos

** Tags removed: appnap
** Tags added: app-nap

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1923648

Title:
  macOS App Nap feature gradually freezes QEMU process

Status in QEMU:
  New

Bug description:
  macOS version: 10.15.2
  QEMU versions: 5.2.0 (from MacPorts)
 5.2.92 (v6.0.0-rc2-23-g9692c7b037)

  If the QEMU window is not visible (hidden, minimized or another
  application is in full screen mode), the QEMU process gradually
  freezes: it still runs, but the VM does not respond to external
  requests such as Telnet or SSH until the QEMU window is visible on the
  desktop.

  This behavior is due to the work of the macOS App Nap function:
  
https://developer.apple.com/library/archive/documentation/Performance/Conceptual/power_efficiency_guidelines_osx/AppNap.html#//apple_ref/doc/uid/TP40013929-CH2-SW1

  It doesn't matter how the process is started -- as a background job or
  as a foreground shell process in case QEMU has a desktop window.

  My VM does not have a display output, only a serial line, most likely
  if the VM was using OpenGL, or playing sound (or any other App Nap
  triggers), then the problem would never have been detected.

  In my case only one starting way without this problem:
  sudo qemu-system-x86_64 -nodefaults \
  -cpu host -accel hvf -smp 1 -m 384 \
  -device virtio-blk-pci,drive=flash0 \
  -drive 
file=/vios-adventerprisek9-m.vmdk.SPA.156-1.T.vmdk,if=none,format=vmdk,id=flash0
 \
  -device e1000,netdev=local -netdev 
tap,id=local,ifname=tap0,script=no,downscript=no \
  -serial stdio -display none

  The typical way from the internet to disable App Nap doesn't work:
  defaults write NSGlobalDomain NSAppSleepDisabled -bool YES

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1923648/+subscriptions

RE: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg

[Bruno Piazera Larsen]

> I'm actually not sure if we'll want translate_init.c for !tcg builds.
> It's *primarily* for TCG, but we still need at least some of the cpu
> state structure for KVM, and some of that is initialized in
> translate_init.
>
> I think it will probably make more sense to leave it in for a first
> cut.  Later refinement might end up removing it.
>
> The whole #include translate_init.c.inc thing might make for some
> awkward fiddling in this, of course.

I just checked, there is going to be some shuffling of functions
around, as there are some static variables defined on translate.c,
and used in translate_init.c.inc, some functions needed for KVM
on translate.c and some TCG only functions in the
translate_init.c.inc.

The trivial path is to:
* rename translate_init.c.inc to cpu_init.c (since it has to do with
initial definitions for CPUs, and it's not related to translating
anymore);
* move gen_write_xer and gen_read_xer into cpu_init.c, as they're
used for some sprs, and whatever needs to be moved with it
* move is_indirect_opcode and ind_table to translate.c, since they
are used to translate ppc instructions, and the things defined for
these functions
* Figure out what needs to be added to the includes for both
files to compile
* move opcodes and invalid_handler into cpu_init.c, because they
are only used by stuff in this file.

I'm just not sure about this last point. The stuff that use opcodes
create the callback tables for TCG, AFAICT. The better plan would
be to move all of that to tanslate.c, but might be a lot.

Can I follow the trivial plan for the first cut and leave a TODO in
the code for a better solution in the future? Or is there a nuance
about one of those functions that I have not understood?


Bruno Piazera Larsen

Instituto de Pesquisas 
ELDORADO

Departamento Computação Embarcada

Analista de Software Trainee

Aviso Legal - Disclaimer


De: David Gibson
Enviadas: Terça-feira, 13 de Abril de 2021 03:40
Para: Bruno Piazera Larsen
Cc: Fabiano Rosas; Thomas Huth; qemu-devel@nongnu.org; Luis Fernando Fujita 
Pires; Andre Fernando da Silva; Lucas Mateus Martins Araujo e Castro; Fernando 
Eckhardt Valle; qemu-...@nongnu.org; lagar...@br.ibm.com; Matheus Kowalczuk 
Ferst
Assunto: Re: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg

On Mon, Apr 12, 2021 at 12:05:31PM +, Bruno Piazera Larsen wrote:
> > A general advice for this whole series is: make sure you add in some
> > words explaining why you decided to make a particular change. It will be
> > much easier to review if we know what were the logical steps leading to
> > the change.
>
> Fair point, I should've thought about that.
>
> > > This commit does the necessary code motion from translate_init.c.inc
> >
> > For instance, I don't immediately see why these changes are necessary. I
> > see that translate_init.c.inc already has some `#ifdef CONFIG_TCG`, so
> > why do we need to move a bunch of code into cpu.c instead of just adding
> > more code under ifdef CONFIG_TCG? (I'm not saying it's wrong, just trying to
> > understand the reasoning).
>
> There are 3 main reasons for this decision. The first is kind of silly, but 
> when I read translate.c my mind jumped to translating machine code to TCG, 
> and the amount of TCGv variables at the start reinforced this notion.
> The second was that both s390x and i386 removed it (translate.c) from 
> compilation, so I had no good reason to doubt it.
> The last (and arguably most important) is that translate.c is many thousands 
> of lines long (translate_init.c.inc alone was almost 11k). The whole point of 
> disabling TCG is to speed up compilation and reduce the final file size, so I 
> think it makes sense to remove that big file.
> And the final nail in the coffin is that at no point did it cross my mind to 
> keep the init part of translation, but remove the rest
>
> Also, I'm not a fan of big ifdefs, because it's kinda hard to follow them 
> when viewing code in vim. Adding that to already having a cpu.c file, where 
> it makes sense (to me, at least) to add all CPU related functions, just 
> sounded like a good idea.

I think those are all sound reasons; I think not compiling translate.c
for !tcg builds is the right choice.  We might, however, need to
"rescue" certain functions from there by moving them to another file
so they can be used by KVM code as well.

Re: [RFC v12 62/65] target/arm: refactor arm_cpu_finalize_features into cpu64

[Claudio Fontana]

On 3/28/21 9:15 PM, Richard Henderson wrote:
> On 3/28/21 1:12 PM, Richard Henderson wrote:
>> On 3/26/21 1:36 PM, Claudio Fontana wrote:
>>> +++ b/target/arm/monitor.c
>>> @@ -184,9 +184,11 @@ CpuModelExpansionInfo 
>>> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>>>   if (!err) {
>>>   visit_check_struct(visitor, );
>>>   }
>>> +#ifdef TARGET_AARCH64
>>>   if (!err) {
>>> -    arm_cpu_finalize_features(ARM_CPU(obj), );
>>> +    aarch64_cpu_finalize_features(ARM_CPU(obj), );
>>>   }
>>> +#endif /* TARGET_AARCH64 */
>>>   visit_end_struct(visitor, NULL);
>>>   visit_free(visitor);
>>>   if (err) {
>>> @@ -195,7 +197,9 @@ CpuModelExpansionInfo 
>>> *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
>>>   return NULL;
>>>   }
>>>   } else {
>>> -    arm_cpu_finalize_features(ARM_CPU(obj), _abort);
>>> +#ifdef TARGET_AARCH64
>>> +    aarch64_cpu_finalize_features(ARM_CPU(obj), _abort);
>>> +#endif /* TARGET_AARCH64 */
>>
>> These ifdefs are not an improvement.

Right, here we cannot use is_a64(), and I have not found another way other than 
adding a stub for the function?

> 
> And are actively wrong, since we've lost the runtime test for 
> ARM_FEATURE_AARCH64.
> 
> r~
> 

Right will fix.

Thanks

C

Re: [PATCH 13/13] target/arm: Make translate-neon.c.inc its own compilation unit

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> Switch translate-neon.c.inc from being #included into translate.c
> to being its own compilation unit.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate-a32.h   |  3 +++
>  .../arm/{translate-neon.c.inc => translate-neon.c}   | 12 +++-
>  target/arm/translate.c   |  3 ---
>  target/arm/meson.build   |  7 ---
>  4 files changed, 14 insertions(+), 11 deletions(-)
>  rename target/arm/{translate-neon.c.inc => translate-neon.c} (99%)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2] vhost-user-blk: Fail gracefully on too large queue size

[Philippe Mathieu-Daudé]

On 4/13/21 6:56 PM, Kevin Wolf wrote:
> virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so
> vhost_user_blk_device_realize() should check this before calling it.
> 
> Simple reproducer:
> 
> qemu-system-x86_64 \
> -chardev null,id=foo \
> -device vhost-user-blk-pci,queue-size=4096,chardev=foo
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935014
> Signed-off-by: Kevin Wolf 
> ---
>  hw/block/vhost-user-blk.c | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

Re: [PATCH v1 3/3] target/ppc: Add POWER10 exception model

[Cédric Le Goater]

On 4/13/21 5:53 PM, Fabiano Rosas wrote:
> Nicholas Piggin  writes:
> 
>> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
>> and it removes support for the LPCR[AIL]=0b10 mode.
>>
>> Signed-off-by: Nicholas Piggin 
>> ---
>>  hw/ppc/spapr_hcall.c|   5 ++
>>  target/ppc/cpu-qom.h|   2 +
>>  target/ppc/cpu.h|   5 +-
>>  target/ppc/excp_helper.c| 114 ++--
>>  target/ppc/translate.c  |   3 +-
>>  target/ppc/translate_init.c.inc |   2 +-
>>  6 files changed, 107 insertions(+), 24 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 7b5cd3553c..2f65f20f72 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1399,6 +1399,11 @@ static target_ulong 
>> h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>>  return H_UNSUPPORTED_FLAG;
>>  }
>>  
>> +if (mflags == AIL_0001_8000 && (pcc->insns_flags2 & PPC2_ISA310)) {
>> +/* AIL 2 is also reserved in ISA v3.1 */
>> +return H_UNSUPPORTED_FLAG;
>> +}
>> +
>>  spapr_set_all_lpcrs(mflags << LPCR_AIL_SHIFT, LPCR_AIL);
>>  
>>  return H_SUCCESS;
>> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
>> index 118baf8d41..06b6571bc9 100644
>> --- a/target/ppc/cpu-qom.h
>> +++ b/target/ppc/cpu-qom.h
>> @@ -116,6 +116,8 @@ enum powerpc_excp_t {
>>  POWERPC_EXCP_POWER8,
>>  /* POWER9 exception model   */
>>  POWERPC_EXCP_POWER9,
>> +/* POWER10 exception model   */
>> +POWERPC_EXCP_POWER10,
>>  };
>>  
>>  
>> /*/
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index e73416da68..24e53e0ac3 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -354,10 +354,11 @@ typedef struct ppc_v3_pate_t {
>>  #define LPCR_PECE_U_SHIFT (63 - 19)
>>  #define LPCR_PECE_U_MASK  (0x7ull << LPCR_PECE_U_SHIFT)
>>  #define LPCR_HVEE PPC_BIT(17) /* Hypervisor Virt Exit Enable */
>> -#define LPCR_RMLS_SHIFT   (63 - 37)
>> +#define LPCR_RMLS_SHIFT   (63 - 37)   /* RMLS (removed in ISA v3.0) */
>>  #define LPCR_RMLS (0xfull << LPCR_RMLS_SHIFT)
>> +#define LPCR_HAIL PPC_BIT(37) /* ISA v3.1 HV AIL=3 equivalent */
>>  #define LPCR_ILE  PPC_BIT(38)
>> -#define LPCR_AIL_SHIFT(63 - 40)  /* Alternate interrupt location */
>> +#define LPCR_AIL_SHIFT(63 - 40)   /* Alternate interrupt location */
>>  #define LPCR_AIL  (3ull << LPCR_AIL_SHIFT)
>>  #define LPCR_UPRT PPC_BIT(41) /* Use Process Table */
>>  #define LPCR_EVIRTPPC_BIT(42) /* Enhanced Virtualisation */
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index b8881c0f85..e8bf0598b4 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -197,7 +197,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>> excp_model, int excp)
>>  CPUState *cs = CPU(cpu);
>>  CPUPPCState *env = >env;
>>  target_ulong msr, new_msr, vector;
>> -int srr0, srr1, asrr0, asrr1, lev = -1, ail;
>> +int srr0, srr1, asrr0, asrr1, lev = -1;
>> +int ail, hail, using_ail;
>>  bool lpes0;
>>  
>>  qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
>> @@ -239,24 +240,39 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int 
>> excp_model, int excp)
>>   * On anything else, we behave as if LPES0 is 1
>>   * (externals don't alter MSR:HV)
>>   *
>> - * AIL is initialized here but can be cleared by
>> - * selected exceptions
>> + * ail/hail are initialized here but can be cleared by
>> + * selected exceptions, and other conditions afterwards.
>>   */
>>  #if defined(TARGET_PPC64)
>>  if (excp_model == POWERPC_EXCP_POWER7 ||
>>  excp_model == POWERPC_EXCP_POWER8 ||
>> -excp_model == POWERPC_EXCP_POWER9) {
>> +excp_model == POWERPC_EXCP_POWER9 ||
>> +excp_model == POWERPC_EXCP_POWER10) {
>>  lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>>  if (excp_model != POWERPC_EXCP_POWER7) {
>>  ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
>>  } else {
>>  ail = 0;
>>  }
>> +if (excp_model == POWERPC_EXCP_POWER10) {
>> +if (AIL_0001_8000) {
> 
> if (2)
> 
>> +ail = 0; /* AIL=2 is reserved in ISA v3.1 */
>> +}
>> +
>> +if (env->spr[SPR_LPCR] & LPCR_HAIL) {
>> +hail = AIL_C000___4000;
>> +} else {
>> +hail = 0;
>> +}
>> +} else {
>> +hail = ail;
> 
> I think it's better if we set hail = 0 here. Read on...
> 
>> +}
>>  } else
>>  #endif /* defined(TARGET_PPC64) */
>>  {
>>  lpes0 = true;
>>  ail = 0;
>> +hail = 0;
>>  }
>>  
>>  /*
>> @@ -316,6 +332,7 @@ static inline void 

Re: [PATCH 09/13] target/arm: Move vfp_reg_ptr() to translate-neon.c.inc

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> The function vfp_reg_ptr() is used only in translate-neon.c.inc;
> move it there.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate.c  | 7 ---
>  target/arm/translate-neon.c.inc | 7 +++
>  2 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20210413160850.240064-1-pbonz...@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210413160850.240064-1-pbonz...@redhat.com
Subject: [PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
487722a qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code
588f61f osdep: protect qemu/osdep.h with extern "C"
5316327 osdep: include glib-compat.h before other QEMU headers

=== OUTPUT BEGIN ===
1/3 Checking commit 5316327519a7 (osdep: include glib-compat.h before other 
QEMU headers)
2/3 Checking commit 588f61fea9ae (osdep: protect qemu/osdep.h with extern "C")
WARNING: architecture specific defines should be avoided
#51: FILE: include/qemu/compiler.h:14:
+#ifdef __cplusplus

ERROR: storage class should be at the beginning of the declaration
#52: FILE: include/qemu/compiler.h:15:
+#define QEMU_EXTERN_C extern "C"

ERROR: storage class should be at the beginning of the declaration
#54: FILE: include/qemu/compiler.h:17:
+#define QEMU_EXTERN_C extern

WARNING: architecture specific defines should be avoided
#77: FILE: include/qemu/osdep.h:116:
+#ifdef __cplusplus

WARNING: architecture specific defines should be avoided
#88: FILE: include/qemu/osdep.h:730:
+#ifdef __cplusplus

total: 2 errors, 3 warnings, 47 lines checked

Patch 2/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/3 Checking commit 487722a3d4ef (qapi/qom.json: Do not use 
CONFIG_VIRTIO_CRYPTO in common code)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210413160850.240064-1-pbonz...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH 08/13] target/arm: Make translate-vfp.c.inc its own compilation unit

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> Switch translate-vfp.c.inc from being #included into translate.c
> to being its own compilation unit.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate-a32.h  |  2 ++
>  target/arm/{translate-vfp.c.inc => translate-vfp.c} | 12 +++-
>  target/arm/translate.c  |  3 +--
>  target/arm/meson.build  |  5 +++--
>  4 files changed, 13 insertions(+), 9 deletions(-)
>  rename target/arm/{translate-vfp.c.inc => translate-vfp.c} (99%)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 12/13] target/arm: Make functions used by translate-neon global

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> Make the remaining functions needed by the translate-neon code
> global.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate-a32.h |  8 
>  target/arm/translate.c | 10 ++
>  2 files changed, 10 insertions(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 09/13] target/arm: Move vfp_reg_ptr() to translate-neon.c.inc

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> The function vfp_reg_ptr() is used only in translate-neon.c.inc;
> move it there.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate.c  | 7 ---
>  target/arm/translate-neon.c.inc | 7 +++
>  2 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 06/13] target/arm: Move vfp_{load, store}_reg{32, 64} to translate-vfp.c.inc

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> The functions vfp_load_reg32(), vfp_load_reg64(), vfp_store_reg32()
> and vfp_store_reg64() are used only in translate-vfp.c.inc. Move
> them to that file.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate.c | 20 
>  target/arm/translate-vfp.c.inc | 20 
>  2 files changed, 20 insertions(+), 20 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 07/13] target/arm: Make functions used by translate-vfp global

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> Make the remaining functions which are needed by translate-vfp.c.inc
> global.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate-a32.h | 32 
>  target/arm/translate.c | 37 ++---
>  2 files changed, 38 insertions(+), 31 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] vhost-user-blk: Fail gracefully on too large queue size

[Kevin Wolf]

Am 13.04.2021 um 18:52 hat Kevin Wolf geschrieben:
> virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so
> vhost_user_blk_device_realize() should check this before calling it.
> 
> Simple reproducer:
> 
> qemu-system-x86_64 \
> -chardev null,id=foo \
> -device vhost-user-blk-pci,queue-size=4096,chardev=foo
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935014
> Signed-off-by: Kevin Wolf 
> ---
>  hw/block/vhost-user-blk.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 0b5b9d44cd..531e4ea063 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -467,6 +467,11 @@ static void vhost_user_blk_device_realize(DeviceState 
> *dev, Error **errp)
>  error_setg(errp, "vhost-user-blk: queue size must be non-zero");
>  return;
>  }
> +if (s->queue_size) {

Sorry, obviously I didn't send the right one... Please look at v2 where
this actually checks what the commit message promises.

Kevin

> +error_setg(errp, "vhost-user-blk: queue size must not exceed %d",
> +   VIRTQUEUE_MAX_SIZE);
> +return;
> +}
>  
>  if (!vhost_user_init(>vhost_user, >chardev, errp)) {
>  return;
> -- 
> 2.30.2
> 

Re: [PATCH 01/13] target/arm: Move constant expanders to translate.h

[Philippe Mathieu-Daudé]

On 4/13/21 6:07 PM, Peter Maydell wrote:
> Some of the constant expanders defined in translate.c are generically
> useful and will be used by the separate C files for VFP and Neon once
> they are created; move the expander definitions to translate.h.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target/arm/translate.h | 24 
>  target/arm/translate.c | 24 
>  2 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 423b0e08df0..4c0b6e8fc42 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -116,6 +116,30 @@ extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
>  extern TCGv_i64 cpu_exclusive_addr;
>  extern TCGv_i64 cpu_exclusive_val;
>  
> +/*
> + * Constant expanders for the decoders.
> + */
> +
> +static inline int negate(DisasContext *s, int x)
> +{
> +return -x;
> +}
> +
> +static inline int plus_2(DisasContext *s, int x)
> +{
> +return x + 2;
> +}
> +
> +static inline int times_2(DisasContext *s, int x)
> +{
> +return x * 2;
> +}
> +
> +static inline int times_4(DisasContext *s, int x)
> +{
> +return x * 4;
> +}

Being static inlined, I wonder if these shouldn't belong
to "exec/translator.h" or another generic helper header
(because I ended using similar helpers in MIPS).

Can be done later tho, so:
Reviewed-by: Philippe Mathieu-Daudé 

[PATCH v2] vhost-user-blk: Fail gracefully on too large queue size

[Kevin Wolf]

virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so
vhost_user_blk_device_realize() should check this before calling it.

Simple reproducer:

qemu-system-x86_64 \
-chardev null,id=foo \
-device vhost-user-blk-pci,queue-size=4096,chardev=foo

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935014
Signed-off-by: Kevin Wolf 
---
 hw/block/vhost-user-blk.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 0b5b9d44cd..f5e9682703 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -467,6 +467,11 @@ static void vhost_user_blk_device_realize(DeviceState 
*dev, Error **errp)
 error_setg(errp, "vhost-user-blk: queue size must be non-zero");
 return;
 }
+if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
+error_setg(errp, "vhost-user-blk: queue size must not exceed %d",
+   VIRTQUEUE_MAX_SIZE);
+return;
+}
 
 if (!vhost_user_init(>vhost_user, >chardev, errp)) {
 return;
-- 
2.30.2

[PATCH v2] target/s390x: Fix translation exception on illegal instruction

[Ilya Leoshkevich]

Hitting an uretprobe in a s390x TCG guest causes a SIGSEGV. What
happens is:

* uretprobe maps a userspace page containing an invalid instruction.
* uretprobe replaces the target function's return address with the
  address of that page.
* When tb_gen_code() is called on that page, tb->size ends up being 0
  (because the page starts with the invalid instruction), which causes
  virt_page2 to point to the previous page.
* The previous page is not mapped, so this causes a spurious
  translation exception.

The bug is that tb->size must never be 0: even if there is an illegal
instruction, the instruction bytes that have been looked at must count
towards tb->size. So adjust s390x's translate_one() to act this way
for both illegal instructions and instructions that are known to
generate exceptions.

Also add an assertion to tb_gen_code() in order to detect such
situations in future.

Signed-off-by: Ilya Leoshkevich 
---

v1: https://lists.nongnu.org/archive/html/qemu-devel/2021-04/msg02037.html
v1 -> v2: Fix target/s390x instead of trying to tolerate tb->size == 0
  in tb_gen_code().

 accel/tcg/translate-all.c |  1 +
 target/s390x/translate.c  | 16 +++-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ba6ab09790..93b2dae112 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1913,6 +1913,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 
 tcg_ctx->cpu = env_cpu(env);
 gen_intermediate_code(cpu, tb, max_insns);
+assert(tb->size != 0);
 tcg_ctx->cpu = NULL;
 max_insns = tb->icount;
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 4f953ddfba..e243624d2a 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6412,7 +6412,8 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 qemu_log_mask(LOG_UNIMP, "unimplemented opcode 0x%02x%02x\n",
   s->fields.op, s->fields.op2);
 gen_illegal_opcode(s);
-return DISAS_NORETURN;
+ret = DISAS_NORETURN;
+goto out;
 }
 
 #ifndef CONFIG_USER_ONLY
@@ -6428,7 +6429,8 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 /* privileged instruction */
 if ((s->base.tb->flags & FLAG_MASK_PSTATE) && (insn->flags & IF_PRIV)) 
{
 gen_program_exception(s, PGM_PRIVILEGED);
-return DISAS_NORETURN;
+ret = DISAS_NORETURN;
+goto out;
 }
 
 /* if AFP is not enabled, instructions and registers are forbidden */
@@ -6455,7 +6457,8 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 }
 if (dxc) {
 gen_data_exception(dxc);
-return DISAS_NORETURN;
+ret = DISAS_NORETURN;
+goto out;
 }
 }
 
@@ -6463,7 +6466,8 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 if (insn->flags & IF_VEC) {
 if (!((s->base.tb->flags & FLAG_MASK_VECTOR))) {
 gen_data_exception(0xfe);
-return DISAS_NORETURN;
+ret = DISAS_NORETURN;
+goto out;
 }
 }
 
@@ -6484,7 +6488,8 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 (insn->spec & SPEC_r1_f128 && !is_fp_pair(get_field(s, r1))) ||
 (insn->spec & SPEC_r2_f128 && !is_fp_pair(get_field(s, r2 {
 gen_program_exception(s, PGM_SPECIFICATION);
-return DISAS_NORETURN;
+ret = DISAS_NORETURN;
+goto out;
 }
 }
 
@@ -6544,6 +6549,7 @@ static DisasJumpType translate_one(CPUS390XState *env, 
DisasContext *s)
 }
 #endif
 
+out:
 /* Advance to the next instruction.  */
 s->base.pc_next = s->pc_tmp;
 return ret;
-- 
2.29.2

[PATCH] vhost-user-blk: Fail gracefully on too large queue size

[Kevin Wolf]

virtio_add_queue() aborts when queue_size > VIRTQUEUE_MAX_SIZE, so
vhost_user_blk_device_realize() should check this before calling it.

Simple reproducer:

qemu-system-x86_64 \
-chardev null,id=foo \
-device vhost-user-blk-pci,queue-size=4096,chardev=foo

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935014
Signed-off-by: Kevin Wolf 
---
 hw/block/vhost-user-blk.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 0b5b9d44cd..531e4ea063 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -467,6 +467,11 @@ static void vhost_user_blk_device_realize(DeviceState 
*dev, Error **errp)
 error_setg(errp, "vhost-user-blk: queue size must be non-zero");
 return;
 }
+if (s->queue_size) {
+error_setg(errp, "vhost-user-blk: queue size must not exceed %d",
+   VIRTQUEUE_MAX_SIZE);
+return;
+}
 
 if (!vhost_user_init(>vhost_user, >chardev, errp)) {
 return;
-- 
2.30.2

Re: [PATCH 3/3] hw/arm/mps2-tz: Implement AN524 memory remapping via machine property

[Philippe Mathieu-Daudé]

On 4/12/21 3:43 PM, Peter Maydell wrote:
> The AN524 FPGA image supports two memory maps, which differ
> in where the QSPI and BRAM are. In the default map, the BRAM
> is at 0x_, and the QSPI at 0x2800_. In the second
> map, they are the other way around.
> 
> In hardware, the initial mapping can be selected by the user
> by writing either "REMAP: BRAM" (the default) or "REMAP: QSPI"
> in the board configuration file. The guest can also dynamically
> change the mapping via the SCC CFG_REG0 register.
> 
> Implement this functionality for QEMU, using a machine property
> "remap" with valid values "BRAM" and "QSPI" to allow the user to set
> the initial mapping, in the same way they can on the FPGA, and
> wiring up the bit from the SCC register to also switch the mapping.
> 
> Signed-off-by: Peter Maydell 
> ---
>  docs/system/arm/mps2.rst |  10 
>  hw/arm/mps2-tz.c | 106 ++-
>  2 files changed, 115 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/system/arm/mps2.rst b/docs/system/arm/mps2.rst
> index f83b1517871..8a75beb3a08 100644
> --- a/docs/system/arm/mps2.rst
> +++ b/docs/system/arm/mps2.rst
> @@ -45,3 +45,13 @@ Differences between QEMU and real hardware:
>flash, but only as simple ROM, so attempting to rewrite the flash
>from the guest will fail
>  - QEMU does not model the USB controller in MPS3 boards
> +
> +Machine-specific options
> +
> +
> +The following machine-specific options are supported:
> +
> +remap
> +  Supported for ``mps3-an524`` only.
> +  Set ``BRAM``/``QSPI`` to select the initial memory mapping. The
> +  default is ``BRAM``.
> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> index 25016e464d9..e9806779326 100644
> --- a/hw/arm/mps2-tz.c
> +++ b/hw/arm/mps2-tz.c
> @@ -55,6 +55,7 @@
>  #include "hw/boards.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/reset.h"
>  #include "hw/misc/unimp.h"
>  #include "hw/char/cmsdk-apb-uart.h"
>  #include "hw/timer/cmsdk-apb-timer.h"
> @@ -72,6 +73,7 @@
>  #include "hw/core/split-irq.h"
>  #include "hw/qdev-clock.h"
>  #include "qom/object.h"
> +#include "hw/irq.h"
>  
>  #define MPS2TZ_NUMIRQ_MAX 96
>  #define MPS2TZ_RAM_MAX 5
> @@ -153,6 +155,9 @@ struct MPS2TZMachineState {
>  SplitIRQ cpu_irq_splitter[MPS2TZ_NUMIRQ_MAX];
>  Clock *sysclk;
>  Clock *s32kclk;
> +
> +int remap;

Maybe bool, ...

> +qemu_irq remap_irq;
>  };
>  
>  #define TYPE_MPS2TZ_MACHINE "mps2tz"
> @@ -228,6 +233,10 @@ static const RAMInfo an505_raminfo[] = { {
>  },
>  };
>  
> +/*
> + * Note that the addresses and MPC numbering here should match up
> + * with those used in remap_memory(), which can swap the BRAM and QSPI.
> + */
>  static const RAMInfo an524_raminfo[] = { {
>  .name = "bram",
>  .base = 0x,
> @@ -457,6 +466,7 @@ static MemoryRegion *make_scc(MPS2TZMachineState *mms, 
> void *opaque,
>  
>  object_initialize_child(OBJECT(mms), "scc", scc, TYPE_MPS2_SCC);
>  sccdev = DEVICE(scc);
> +qdev_prop_set_uint32(sccdev, "scc-cfg0", mms->remap);

... and here:

   qdev_prop_set_uint32(sccdev, "scc-cfg0", mms->remap ? 1 : 0);

as remap is a bit and scc-cfg0 could have other bits set in the future.

>  qdev_prop_set_uint32(sccdev, "scc-cfg4", 0x2);
>  qdev_prop_set_uint32(sccdev, "scc-aid", 0x0028);
>  qdev_prop_set_uint32(sccdev, "scc-id", mmc->scc_id);
> @@ -573,6 +583,50 @@ static MemoryRegion *make_mpc(MPS2TZMachineState *mms, 
> void *opaque,
>  return sysbus_mmio_get_region(SYS_BUS_DEVICE(mpc), 0);
>  }
>  
> +static hwaddr boot_mem_base(MPS2TZMachineState *mms)
> +{
> +/*
> + * Return the canonical address of the block which will be mapped
> + * at address 0x0 (i.e. where the vector table is).
> + * This is usually 0, but if the AN524 alternate memory map is
> + * enabled it will be the base address of the QSPI block.
> + */
> +return mms->remap ? 0x2800 : 0;
> +}
> +
> +static void remap_memory(MPS2TZMachineState *mms, int map)
> +{
> +/*
> + * Remap the memory for the AN524. 'map' is the value of
> + * SCC CFG_REG0 bit 0, i.e. 0 for the default map and 1
> + * for the "option 1" mapping where QSPI is at address 0.
> + *
> + * Effectively we need to swap around the "upstream" ends of
> + * MPC 0 and MPC 1.
> + */
> +MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_GET_CLASS(mms);
> +int i;
> +
> +if (mmc->fpga_type != FPGA_AN524) {
> +return;
> +}
> +

This is done in the machine_reset() handler and during QDev realization,
so probably not important, but since it is reachable by an IRQ handler,
maybe it is better (thinking about code copy/pasting) to wrap this with:

   memory_region_transaction_begin();

> +for (i = 0; i < 2; i++) {
> +TZMPC *mpc = >mpc[i];
> +MemoryRegion *upstream = sysbus_mmio_get_region(SYS_BUS_DEVICE(mpc), 
> 1);
> +   

Re: [PATCH] tests/acceptance: Add a 'virt_kvm' test using the GICv3

[Philippe Mathieu-Daudé]

Hi Alex,

On 4/12/21 7:55 PM, Philippe Mathieu-Daudé wrote:
> On 4/6/21 7:12 PM, Alex Bennée wrote:
>>
>> Philippe Mathieu-Daudé  writes:
>>
>>> On 3/31/21 5:45 PM, Alex Bennée wrote:

 Philippe Mathieu-Daudé  writes:

> The current 'virt_kvm' test is restricted to GICv2, but can also
> work with a GICv3. Duplicate it but add a GICv3 test which can be
> tested on some hardware.
>
> Noticed while running:
>
>  $ avocado --show=app run -t machine:virt tests/acceptance/
>  ...
>  (2/6) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm: 
> ERROR: Unexpected empty reply from server (1.82 s)
>
> The job.log content is:
>
>   L0351 DEBUG| Output: 'qemu-system-aarch64: host does not support 
> in-kernel GICv2 emulation\n'
>
> With this patch:
>
>  $ avocado --show=app run -t device:gicv3 tests/acceptance/
>  (1/1)
>  tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm_gicv3:
>  PASS (55.10 s)

 On the new aarch64 machine which is GICv3 I get the following:

  (006/142) 
 tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm_gicv2: 
 ERROR: Unexpected empty reply from server (0.47 s)

 which it shouldn't have run. However:

   ./tests/venv/bin/avocado --show=app run -t device:gic3 tests/acceptance/
   Test Suite could not be create. No test references provided nor any 
 other arguments resolved into tests

 Is this something that has regressed or am I doing it wrong?
>>>
>>> Typo in the tag: "device:gic3" -> "device:gicv3"
>>
>> Doh!
>>
>> But what about:
>>
>> /tests/venv/bin/avocado run 
>> tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm_gicv2
>> JOB ID : 396696d8f9d31d970878cb46025b2ced76f3623f
>> JOB LOG: 
>> /home/alex/avocado/job-results/job-2021-04-06T17.11-396696d/job.log
>>  (1/1) tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm_gicv2: 
>> ERROR: Unexpected empty reply from server (0.65 s)
>> RESULTS: PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
>> CANCEL 0
>> JOB TIME   : 0.96 s
>>
>> why doesn't that skip?
> 
> /home/phil/avocado/job-results/job-2021-04-12T17.51-efdca81/job.log
> 2021-04-12 17:52:44,589 machine  L0389 DEBUG| Output:
> "qemu-system-aarch64: Could not find ROM image
> '/home/phil/qemu/build/host/pc-bios/edk2-aarch64-code.fd'\n"
> 
> Missing prerequisite:
> 
> $ ninja pc-bios/edk2-aarch64-code.fd
> [1/1] Generating edk2-aarch64-code.fd with a custom command (wrapped by
> meson to capture output)
> 
> Then we are good:
> 
> $ avocado --show=app,console run -t device:gicv3 tests/acceptance
> JOB ID : e84401e5cc3ae53a3094c79491e661385cc7b4a7
> JOB LOG:
> /home/phil/avocado/job-results/job-2021-04-12T17.53-e84401e/job.log
>  (1/1)
> tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_kvm_gicv3:
> PASS (16.38 s)
> RESULTS: PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
> CANCEL 0
> JOB TIME   : 16.70 s
> 
> Probably some missing dependency in Makefile/Meson?

Are you using multiple build directories?

I could reproduce doing:

$ mkdir A B
$ cd A
$ make check-qtest-aarch64
$ avocado --show=app,console run -t device:gicv3 tests/acceptance
$ cd ../B
$ ninja qemu-system-aarch64
$ avocado --show=app,console run -t device:gicv3 tests/acceptance

In A edk2-aarch64-code.fd has been expanded in A/pc-bios/,
in B it isn't.

check-acceptance is a Makefile rule, not a ninja one...
I suppose we need to convert it to ninja to be able to use the
rest of the dependencies checks.

Cc'ing Paolo because I'm not sure what the best move and where
to plug things.

Re: [PATCH] docs: Add a QEMU Code of Conduct and Conflict Resolution Policy document

[Daniel P . Berrangé]

On Wed, Mar 31, 2021 at 05:05:27PM +0200, Paolo Bonzini wrote:
> In an ideal world, we would all get along together very well, always be
> polite and never end up in huge conflicts. And even if there are conflicts,
> we would always handle each other fair and respectfully. Unfortunately,
> this is not an ideal world and sometimes people forget how to interact with
> each other in a professional and respectful way. Fortunately, this seldom
> happens in the QEMU community, but for such rare cases it is preferrable
> to have a basic code of conduct document available to show to people
> who are misbehaving.  In case that does not help yet, we should also have
> a conflict resolution policy ready that can be applied in the worst case.
> 
> The Code of Conduct document tries to be short and to the point while
> trying to remain friendly and welcoming; it is based on the Fedora Code
> of Conduct[1] with extra detail added based on the Contributor Covenant
> 1.3.0[2].  Other proposals included the Contributor Covenant 1.3.0 itself
> or the Django Code of Conduct[3] (which is also a derivative of Fedora's)
> but, in any case, there was agreement on keeping the conflict resolution
> policy separate from the CoC itself.
> 
> An important point is whether to apply the code of conduct to violations
> that occur outside public spaces.  The text herein restricts that to
> individuals acting as a representative or a member of the project or
> its community.  This is intermediate between the Contributor Covenant
> (which only mentions representatives of the community, for example using
> an official project e-mail address or posting via an official social media
> account), and the Django Code of Conduct, which says that violations of
> this code outside these spaces "may" be considered but does not limit
> this further.

Since this was derived from the Fedora CoC, you might be interested to
know that Fedora is currently revisiting its CoC:

  https://communityblog.fedoraproject.org/policy-proposal-new-code-of-conduct/

The first comment on that post from mattdm gives clarity as to why they
feel the need to revisit it

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

RE: [PATCH v2] target/ppc: code motion from translate_init.c.inc to gdbstub.c

[Bruno Piazera Larsen]

> > +/* gdbstub.c */
> > +void ppc_gdb_init(CPUState *cs, PowerPCCPUClass *ppc);
> > +gchar *ppc_gdb_arch_name(CPUState *cs);
>
> These should probably go into internal.h and not cpu.h.
> These do not need to be exposed outside of target/ppc/.

Makes sense, I can do that. Is such a small change worth a v3, or do I fix this 
as I send the disable-tcg patch series?

> > +#include "exec/helper-proto.h"
>
> Not ideal, but ok temporarily.

yeah, this is only here until we figure out how to deal with the vscr stuff.

> > +gdb_get_reg32(buf, helper_mfvscr(env));
> ...
> > +helper_mtvscr(env, ldl_p(mem_buf));
>
> These should be modeled on e.g. store_fpscr, where there's a non-"helper"
> function to be called, which is then called by the "helper" function.
>
> Obviously, splitting that out should be a separate patch.

We already expected to fix this in the disable-tcg patch series, but this 
reference does help (:
However, checking now, store_fpscr is defined in fpu_helper.c, which is mostly 
TCG stuff. Any
idea where we could move the store_* functions to have them compile in the !tcg 
build?

My best guess is to do it in cpu.c, as the other files compiled with the same 
options are cpu-models.c and
gdbstub.c.


Bruno Piazera Larsen

Instituto de Pesquisas 
ELDORADO

Departamento Computação Embarcada

Analista de Software Trainee

Aviso Legal - Disclaimer

Re: [PATCH 0/3] mps3-an524: support memory remapping

[Philippe Mathieu-Daudé]

On 4/13/21 6:29 PM, Philippe Mathieu-Daudé wrote:> On 4/12/21 4:48 PM,
Peter Maydell wrote:
>> On Mon, 12 Apr 2021 at 15:37, Philippe Mathieu-Daudé  wrote:
>>> On 4/12/21 3:43 PM, Peter Maydell wrote:
 The AN524 FPGA image supports two memory maps, which differ
 in where the QSPI and BRAM are. In the default map, the BRAM
 is at 0x_, and the QSPI at 0x2800_. In the second
 map, they are the other way around.

 In hardware, the initial mapping can be selected by the user
 by writing either "REMAP: BRAM" (the default) or "REMAP: QSPI"
 in the board configuration file. The guest can also dynamically
 change the mapping via the SCC CFG_REG0 register.

 This patchset adds support for the feature to QEMU's model;
 the user-sets-the-initial-mapping part is a new machine property
 which can be set with "-M remap=QSPI".

 This is needed for some guest images -- for instance the
 Arm TF-M binaries -- which assume they have the QSPI layout.
>>>
>>> I tend to see machine property set on the command line similar
>>> to hardware wire jumpers, externally set (by an operator having
>>> access to the hardware, not guest code).
>>>
>>> Here the remap behavior you described is triggered by the guest.
>>> Usually this is done by a bootloader code before running the
>>> guest code.
>>> Couldn't we have the same result using a booloader (like -bios
>>> cmd line option) rather than modifying internal peripheral state?
>>
> 
> (
> 
>> In the real hardware, the handling of the board configuration
>> file is done by the "Motherboard Configuration Controller", which
>> is an entirely separate microcontroller on the dev board but outside
>> the FPGA, and which is responsible for things like loading image
>> files off the SD card and writing them to memory, setting a bunch
>> of initial configuration including the remap setting but also
>> things like setting the oscillators to the values that this
>> particular FPGA image needs. It's also what makes the board
>> appear to a connected computer as a USB mass storage device so
>> you can update the SD card files via USB cable rather than doing
>> lots of plugging and unplugging, and it is what loads the FPGA
>> image off SD card and into the FPGA in the first place.
> 
> ) [*]
> 
>> QEMU is never going to implement the MCC as a real emulated
>> guest CPU; instead our models hard-code some of the things it
>> does. I think that a machine property (a thing set externally
>> to the guest CPU and valid before any guest CPU code executes)
>> is a reasonable way to implement the remap setting, which from
>> the point of view of the CPU inside the FPGA is a thing set
>> externally and valid before any guest CPU code executes.
> 
> OK now I understand the picture, the MCC is external. In that case
> the machine property is a clean way to address that.
> 
> Could you add the first paragraph of your answer ([*]) in patch 3
> description (before the current comment) to make it clearer?

(In case you agree, no need to respin).

[PULL v2 3/3] qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code

[Paolo Bonzini]

From: Thomas Huth 

The ObjectType enum and ObjectOptions are included from qapi-types-qom.h
into common code. We should not use target-specific config switches like
CONFIG_VIRTIO_CRYPTO here, since this is not defined in common code and
thus the enum will look differently between common and target specific
code. For this case, it's hopefully enough to check for CONFIG_VHOST_CRYPTO
only (which is a host specific config switch, i.e. it's the same on all
targets).

Signed-off-by: Thomas Huth 
Message-Id: <20210412160710.639800-1-th...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 qapi/qom.json | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi/qom.json b/qapi/qom.json
index db5ac419b1..cd0e76d564 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -752,7 +752,7 @@
 'cryptodev-backend',
 'cryptodev-backend-builtin',
 { 'name': 'cryptodev-vhost-user',
-  'if': 'defined(CONFIG_VIRTIO_CRYPTO) && defined(CONFIG_VHOST_CRYPTO)' },
+  'if': 'defined(CONFIG_VHOST_CRYPTO)' },
 'dbus-vmstate',
 'filter-buffer',
 'filter-dump',
@@ -809,7 +809,7 @@
   'cryptodev-backend':  'CryptodevBackendProperties',
   'cryptodev-backend-builtin':  'CryptodevBackendProperties',
   'cryptodev-vhost-user':   { 'type': 'CryptodevVhostUserProperties',
-  'if': 'defined(CONFIG_VIRTIO_CRYPTO) && 
defined(CONFIG_VHOST_CRYPTO)' },
+  'if': 'defined(CONFIG_VHOST_CRYPTO)' },
   'dbus-vmstate':   'DBusVMStateProperties',
   'filter-buffer':  'FilterBufferProperties',
   'filter-dump':'FilterDumpProperties',
-- 
2.30.1

Re: [PATCH 2/3] hw/misc/mps2-scc: Support using CFG0 bit 0 for remapping

[Philippe Mathieu-Daudé]

On 4/12/21 3:43 PM, Peter Maydell wrote:
> On some boards, SCC config register CFG0 bit 0 controls whether
> parts of the board memory map are remapped. Support this with:
>  * a device property scc-cfg0 so the board can specify the
>initial value of the CFG0 register
>  * an outbound GPIO line which tracks bit 0 and which the board
>can wire up to provide the remapping
> 
> Signed-off-by: Peter Maydell 
> ---
>  include/hw/misc/mps2-scc.h |  9 +
>  hw/misc/mps2-scc.c | 13 ++---
>  2 files changed, 19 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 0/3] mps3-an524: support memory remapping

[Philippe Mathieu-Daudé]

Hi Peter,

On 4/12/21 4:48 PM, Peter Maydell wrote:
> On Mon, 12 Apr 2021 at 15:37, Philippe Mathieu-Daudé  wrote:
>> On 4/12/21 3:43 PM, Peter Maydell wrote:
>>> The AN524 FPGA image supports two memory maps, which differ
>>> in where the QSPI and BRAM are. In the default map, the BRAM
>>> is at 0x_, and the QSPI at 0x2800_. In the second
>>> map, they are the other way around.
>>>
>>> In hardware, the initial mapping can be selected by the user
>>> by writing either "REMAP: BRAM" (the default) or "REMAP: QSPI"
>>> in the board configuration file. The guest can also dynamically
>>> change the mapping via the SCC CFG_REG0 register.
>>>
>>> This patchset adds support for the feature to QEMU's model;
>>> the user-sets-the-initial-mapping part is a new machine property
>>> which can be set with "-M remap=QSPI".
>>>
>>> This is needed for some guest images -- for instance the
>>> Arm TF-M binaries -- which assume they have the QSPI layout.
>>
>> I tend to see machine property set on the command line similar
>> to hardware wire jumpers, externally set (by an operator having
>> access to the hardware, not guest code).
>>
>> Here the remap behavior you described is triggered by the guest.
>> Usually this is done by a bootloader code before running the
>> guest code.
>> Couldn't we have the same result using a booloader (like -bios
>> cmd line option) rather than modifying internal peripheral state?
> 

(

> In the real hardware, the handling of the board configuration
> file is done by the "Motherboard Configuration Controller", which
> is an entirely separate microcontroller on the dev board but outside
> the FPGA, and which is responsible for things like loading image
> files off the SD card and writing them to memory, setting a bunch
> of initial configuration including the remap setting but also
> things like setting the oscillators to the values that this
> particular FPGA image needs. It's also what makes the board
> appear to a connected computer as a USB mass storage device so
> you can update the SD card files via USB cable rather than doing
> lots of plugging and unplugging, and it is what loads the FPGA
> image off SD card and into the FPGA in the first place.

) [*]

> QEMU is never going to implement the MCC as a real emulated
> guest CPU; instead our models hard-code some of the things it
> does. I think that a machine property (a thing set externally
> to the guest CPU and valid before any guest CPU code executes)
> is a reasonable way to implement the remap setting, which from
> the point of view of the CPU inside the FPGA is a thing set
> externally and valid before any guest CPU code executes.

OK now I understand the picture, the MCC is external. In that case
the machine property is a clean way to address that.

Could you add the first paragraph of your answer ([*]) in patch 3
description (before the current comment) to make it clearer?

Thanks for the clarification,

Phil.

[PULL v2 2/3] osdep: protect qemu/osdep.h with extern "C"

[Paolo Bonzini]

System headers may include templates if compiled with a C++ compiler,
which cause the compiler to complain if qemu/osdep.h is included
within a C++ source file's 'extern "C"' block.  Add
an 'extern "C"' block directly to qemu/osdep.h, so that
system headers can be kept out of it.

There is a stray declaration early in qemu/osdep.h, which needs
to be special cased.  Add a definition in qemu/compiler.h to
make it look nice.

config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h
are included outside the 'extern "C"' block; that is not
an issue because they consist entirely of preprocessor directives.

Signed-off-by: Paolo Bonzini 
---
 disas/nanomips.cpp  |  2 +-
 include/qemu/compiler.h |  6 ++
 include/qemu/osdep.h| 10 +-
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 2b09655271..8ddef897f0 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -27,8 +27,8 @@
  *  Reference Manual", Revision 01.01, April 27, 2018
  */
 
-extern "C" {
 #include "qemu/osdep.h"
+extern "C" {
 #include "disas/dis-asm.h"
 }
 
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index cf28bb2bcd..091c45248b 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -11,6 +11,12 @@
 #define QEMU_STATIC_ANALYSIS 1
 #endif
 
+#ifdef __cplusplus
+#define QEMU_EXTERN_C extern "C"
+#else
+#define QEMU_EXTERN_C extern
+#endif
+
 #define QEMU_NORETURN __attribute__ ((__noreturn__))
 
 #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result))
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b67b0a1e8c..3f8785a471 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -57,7 +57,7 @@
 #define daemon qemu_fake_daemon_function
 #include 
 #undef daemon
-extern int daemon(int, int);
+QEMU_EXTERN_C int daemon(int, int);
 #endif
 
 #ifdef _WIN32
@@ -113,6 +113,10 @@ extern int daemon(int, int);
 
 #include "glib-compat.h"
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
 #endif
@@ -723,4 +727,8 @@ static inline int platform_does_not_support_system(const 
char *command)
 }
 #endif /* !HAVE_SYSTEM_FUNCTION */
 
+#ifdef __cplusplus
+}
+#endif
+
 #endif
-- 
2.30.1

[PATCH 13/13] target/arm: Make translate-neon.c.inc its own compilation unit

[Peter Maydell]

Switch translate-neon.c.inc from being #included into translate.c
to being its own compilation unit.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-a32.h   |  3 +++
 .../arm/{translate-neon.c.inc => translate-neon.c}   | 12 +++-
 target/arm/translate.c   |  3 ---
 target/arm/meson.build   |  7 ---
 4 files changed, 14 insertions(+), 11 deletions(-)
 rename target/arm/{translate-neon.c.inc => translate-neon.c} (99%)

diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h
index f165f15cc47..d3aeb5a19c9 100644
--- a/target/arm/translate-a32.h
+++ b/target/arm/translate-a32.h
@@ -24,6 +24,9 @@
 bool disas_m_nocp(DisasContext *dc, uint32_t insn);
 bool disas_vfp(DisasContext *s, uint32_t insn);
 bool disas_vfp_uncond(DisasContext *s, uint32_t insn);
+bool disas_neon_dp(DisasContext *s, uint32_t insn);
+bool disas_neon_ls(DisasContext *s, uint32_t insn);
+bool disas_neon_shared(DisasContext *s, uint32_t insn);
 
 void load_reg_var(DisasContext *s, TCGv_i32 var, int reg);
 void arm_gen_condlabel(DisasContext *s);
diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c
similarity index 99%
rename from target/arm/translate-neon.c.inc
rename to target/arm/translate-neon.c
index c6f8bc259a1..6532d69f134 100644
--- a/target/arm/translate-neon.c.inc
+++ b/target/arm/translate-neon.c
@@ -20,11 +20,13 @@
  * License along with this library; if not, see .
  */
 
-/*
- * This file is intended to be included from translate.c; it uses
- * some macros and definitions provided by that file.
- * It might be possible to convert it to a standalone .c file eventually.
- */
+#include "qemu/osdep.h"
+#include "tcg/tcg-op.h"
+#include "tcg/tcg-op-gvec.h"
+#include "exec/exec-all.h"
+#include "exec/gen-icount.h"
+#include "translate.h"
+#include "translate-a32.h"
 
 static inline int plus1(DisasContext *s, int x)
 {
diff --git a/target/arm/translate.c b/target/arm/translate.c
index f6d71d03a3a..b00344b933d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1149,9 +1149,6 @@ void write_neon_element64(TCGv_i64 src, int reg, int ele, 
MemOp memop)
 
 #define ARM_CP_RW_BIT   (1 << 20)
 
-/* Include the Neon decoder */
-#include "translate-neon.c.inc"
-
 static inline void iwmmxt_load_reg(TCGv_i64 var, int reg)
 {
 tcg_gen_ld_i64(var, cpu_env, offsetof(CPUARMState, iwmmxt.regs[reg]));
diff --git a/target/arm/meson.build b/target/arm/meson.build
index f6360f33f11..5bfaf43b500 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -1,8 +1,8 @@
 gen = [
   decodetree.process('sve.decode', extra_args: '--decode=disas_sve'),
-  decodetree.process('neon-shared.decode', extra_args: 
'--static-decode=disas_neon_shared'),
-  decodetree.process('neon-dp.decode', extra_args: 
'--static-decode=disas_neon_dp'),
-  decodetree.process('neon-ls.decode', extra_args: 
'--static-decode=disas_neon_ls'),
+  decodetree.process('neon-shared.decode', extra_args: 
'--decode=disas_neon_shared'),
+  decodetree.process('neon-dp.decode', extra_args: '--decode=disas_neon_dp'),
+  decodetree.process('neon-ls.decode', extra_args: '--decode=disas_neon_ls'),
   decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
   decodetree.process('vfp-uncond.decode', extra_args: 
'--decode=disas_vfp_uncond'),
   decodetree.process('m-nocp.decode', extra_args: '--decode=disas_m_nocp'),
@@ -27,6 +27,7 @@ arm_ss.add(files(
   'tlb_helper.c',
   'translate.c',
   'translate-m-nocp.c',
+  'translate-neon.c',
   'translate-vfp.c',
   'vec_helper.c',
   'vfp_helper.c',
-- 
2.20.1

[PATCH 07/13] target/arm: Make functions used by translate-vfp global

[Peter Maydell]

Make the remaining functions which are needed by translate-vfp.c.inc
global.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-a32.h | 32 
 target/arm/translate.c | 37 ++---
 2 files changed, 38 insertions(+), 31 deletions(-)

diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h
index e0e03245f6f..a874253321d 100644
--- a/target/arm/translate-a32.h
+++ b/target/arm/translate-a32.h
@@ -30,6 +30,11 @@ void read_neon_element32(TCGv_i32 dest, int reg, int ele, 
MemOp memop);
 void read_neon_element64(TCGv_i64 dest, int reg, int ele, MemOp memop);
 void write_neon_element32(TCGv_i32 src, int reg, int ele, MemOp memop);
 void write_neon_element64(TCGv_i64 src, int reg, int ele, MemOp memop);
+TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, int ofs);
+void gen_set_pc_im(DisasContext *s, target_ulong val);
+void gen_lookup_tb(DisasContext *s);
+long vfp_reg_offset(bool dp, unsigned reg);
+long neon_full_reg_offset(unsigned reg);
 
 static inline TCGv_i32 load_cpu_offset(int offset)
 {
@@ -57,6 +62,8 @@ static inline TCGv_i32 load_reg(DisasContext *s, int reg)
 return tmp;
 }
 
+void store_reg(DisasContext *s, int reg, TCGv_i32 var);
+
 void gen_aa32_ld_i32(DisasContext *s, TCGv_i32 val, TCGv_i32 a32,
  int index, MemOp opc);
 void gen_aa32_st_i32(DisasContext *s, TCGv_i32 val, TCGv_i32 a32,
@@ -101,4 +108,29 @@ static inline void gen_aa32_st64(DisasContext *s, TCGv_i64 
val,
 gen_aa32_st_i64(s, val, a32, index, MO_Q | s->be_data);
 }
 
+#if defined(CONFIG_USER_ONLY)
+#define IS_USER(s) 1
+#else
+#define IS_USER(s) (s->user)
+#endif
+
+static inline void gen_set_condexec(DisasContext *s)
+{
+if (s->condexec_mask) {
+uint32_t val = (s->condexec_cond << 4) | (s->condexec_mask >> 1);
+TCGv_i32 tmp = tcg_temp_new_i32();
+tcg_gen_movi_i32(tmp, val);
+store_cpu_field(tmp, condexec_bits);
+}
+}
+
+static inline void gen_set_cpsr(TCGv_i32 var, uint32_t mask)
+{
+TCGv_i32 tmp_mask = tcg_const_i32(mask);
+gen_helper_cpsr_write(cpu_env, var, tmp_mask);
+tcg_temp_free_i32(tmp_mask);
+}
+/* Set NZCV flags from the high 4 bits of var.  */
+#define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
+
 #endif
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 2daabb5fb6f..9522002d34e 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -52,12 +52,6 @@
 #include "translate.h"
 #include "translate-a32.h"
 
-#if defined(CONFIG_USER_ONLY)
-#define IS_USER(s) 1
-#else
-#define IS_USER(s) (s->user)
-#endif
-
 /* These are TCG temporaries used only by the legacy iwMMXt decoder */
 static TCGv_i64 cpu_V0, cpu_V1, cpu_M0;
 /* These are TCG globals which alias CPUARMState fields */
@@ -209,7 +203,7 @@ void load_reg_var(DisasContext *s, TCGv_i32 var, int reg)
  * This is used for load/store for which use of PC implies (literal),
  * or ADD that implies ADR.
  */
-static TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, int ofs)
+TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, int ofs)
 {
 TCGv_i32 tmp = tcg_temp_new_i32();
 
@@ -223,7 +217,7 @@ static TCGv_i32 add_reg_for_lit(DisasContext *s, int reg, 
int ofs)
 
 /* Set a CPU register.  The source must be a temporary and will be
marked as dead.  */
-static void store_reg(DisasContext *s, int reg, TCGv_i32 var)
+void store_reg(DisasContext *s, int reg, TCGv_i32 var)
 {
 if (reg == 15) {
 /* In Thumb mode, we must ignore bit 0.
@@ -265,15 +259,6 @@ static void store_sp_checked(DisasContext *s, TCGv_i32 var)
 #define gen_uxtb16(var) gen_helper_uxtb16(var, var)
 
 
-static inline void gen_set_cpsr(TCGv_i32 var, uint32_t mask)
-{
-TCGv_i32 tmp_mask = tcg_const_i32(mask);
-gen_helper_cpsr_write(cpu_env, var, tmp_mask);
-tcg_temp_free_i32(tmp_mask);
-}
-/* Set NZCV flags from the high 4 bits of var.  */
-#define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
-
 static void gen_exception_internal(int excp)
 {
 TCGv_i32 tcg_excp = tcg_const_i32(excp);
@@ -697,17 +682,7 @@ void arm_gen_test_cc(int cc, TCGLabel *label)
 arm_free_cc();
 }
 
-static inline void gen_set_condexec(DisasContext *s)
-{
-if (s->condexec_mask) {
-uint32_t val = (s->condexec_cond << 4) | (s->condexec_mask >> 1);
-TCGv_i32 tmp = tcg_temp_new_i32();
-tcg_gen_movi_i32(tmp, val);
-store_cpu_field(tmp, condexec_bits);
-}
-}
-
-static inline void gen_set_pc_im(DisasContext *s, target_ulong val)
+void gen_set_pc_im(DisasContext *s, target_ulong val)
 {
 tcg_gen_movi_i32(cpu_R[15], val);
 }
@@ -1033,7 +1008,7 @@ static void gen_exception_el(DisasContext *s, int excp, 
uint32_t syn,
 }
 
 /* Force a TB lookup after an instruction that changes the CPU state.  */
-static inline void gen_lookup_tb(DisasContext *s)
+void gen_lookup_tb(DisasContext *s)
 {
 tcg_gen_movi_i32(cpu_R[15], s->base.pc_next);
 s->base.is_jmp = DISAS_EXIT;
@@ -1068,7 +1043,7 

[PATCH 09/13] target/arm: Move vfp_reg_ptr() to translate-neon.c.inc

[Peter Maydell]

The function vfp_reg_ptr() is used only in translate-neon.c.inc;
move it there.

Signed-off-by: Peter Maydell 
---
 target/arm/translate.c  | 7 ---
 target/arm/translate-neon.c.inc | 7 +++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 2ed02df05e0..5eece97584d 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1158,13 +1158,6 @@ void write_neon_element64(TCGv_i64 src, int reg, int 
ele, MemOp memop)
 }
 }
 
-static TCGv_ptr vfp_reg_ptr(bool dp, int reg)
-{
-TCGv_ptr ret = tcg_temp_new_ptr();
-tcg_gen_addi_ptr(ret, cpu_env, vfp_reg_offset(dp, reg));
-return ret;
-}
-
 #define ARM_CP_RW_BIT   (1 << 20)
 
 /* Include the Neon decoder */
diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc
index f6c68e30ab2..c6f8bc259a1 100644
--- a/target/arm/translate-neon.c.inc
+++ b/target/arm/translate-neon.c.inc
@@ -60,6 +60,13 @@ static inline int neon_3same_fp_size(DisasContext *s, int x)
 #include "decode-neon-ls.c.inc"
 #include "decode-neon-shared.c.inc"
 
+static TCGv_ptr vfp_reg_ptr(bool dp, int reg)
+{
+TCGv_ptr ret = tcg_temp_new_ptr();
+tcg_gen_addi_ptr(ret, cpu_env, vfp_reg_offset(dp, reg));
+return ret;
+}
+
 static void neon_load_element(TCGv_i32 var, int reg, int ele, MemOp mop)
 {
 long offset = neon_element_offset(reg, ele, mop & MO_SIZE);
-- 
2.20.1

[PATCH 11/13] target/arm: Move NeonGenThreeOpEnvFn typedef to translate.h

[Peter Maydell]

Move the NeonGenThreeOpEnvFn typedef to translate.h together
with the other similar typedefs.

Signed-off-by: Peter Maydell 
---
 target/arm/translate.h | 2 ++
 target/arm/translate.c | 3 ---
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/arm/translate.h b/target/arm/translate.h
index a9f90e3ed4c..ad3dbc0e8dc 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -406,6 +406,8 @@ typedef void NeonGenOneOpFn(TCGv_i32, TCGv_i32);
 typedef void NeonGenOneOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32);
 typedef void NeonGenTwoOpFn(TCGv_i32, TCGv_i32, TCGv_i32);
 typedef void NeonGenTwoOpEnvFn(TCGv_i32, TCGv_ptr, TCGv_i32, TCGv_i32);
+typedef void NeonGenThreeOpEnvFn(TCGv_i32, TCGv_env, TCGv_i32,
+ TCGv_i32, TCGv_i32);
 typedef void NeonGenTwo64OpFn(TCGv_i64, TCGv_i64, TCGv_i64);
 typedef void NeonGenTwo64OpEnvFn(TCGv_i64, TCGv_ptr, TCGv_i64, TCGv_i64);
 typedef void NeonGenNarrowFn(TCGv_i32, TCGv_i64);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 7439c56d34b..7f95ab16613 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -66,9 +66,6 @@ static const char * const regnames[] =
 { "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
   "r8", "r9", "r10", "r11", "r12", "r13", "r14", "pc" };
 
-/* Function prototypes for gen_ functions calling Neon helpers.  */
-typedef void NeonGenThreeOpEnvFn(TCGv_i32, TCGv_env, TCGv_i32,
- TCGv_i32, TCGv_i32);
 
 /* initialize TCG globals.  */
 void arm_translate_init(void)
-- 
2.20.1

[PATCH 08/13] target/arm: Make translate-vfp.c.inc its own compilation unit

[Peter Maydell]

Switch translate-vfp.c.inc from being #included into translate.c
to being its own compilation unit.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-a32.h  |  2 ++
 target/arm/{translate-vfp.c.inc => translate-vfp.c} | 12 +++-
 target/arm/translate.c  |  3 +--
 target/arm/meson.build  |  5 +++--
 4 files changed, 13 insertions(+), 9 deletions(-)
 rename target/arm/{translate-vfp.c.inc => translate-vfp.c} (99%)

diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h
index a874253321d..96e6eafbde4 100644
--- a/target/arm/translate-a32.h
+++ b/target/arm/translate-a32.h
@@ -22,6 +22,8 @@
 
 /* Prototypes for autogenerated disassembler functions */
 bool disas_m_nocp(DisasContext *dc, uint32_t insn);
+bool disas_vfp(DisasContext *s, uint32_t insn);
+bool disas_vfp_uncond(DisasContext *s, uint32_t insn);
 
 void load_reg_var(DisasContext *s, TCGv_i32 var, int reg);
 void arm_gen_condlabel(DisasContext *s);
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c
similarity index 99%
rename from target/arm/translate-vfp.c.inc
rename to target/arm/translate-vfp.c
index 873a6237ea1..a9069458082 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c
@@ -20,11 +20,13 @@
  * License along with this library; if not, see .
  */
 
-/*
- * This file is intended to be included from translate.c; it uses
- * some macros and definitions provided by that file.
- * It might be possible to convert it to a standalone .c file eventually.
- */
+#include "qemu/osdep.h"
+#include "tcg/tcg-op.h"
+#include "tcg/tcg-op-gvec.h"
+#include "exec/exec-all.h"
+#include "exec/gen-icount.h"
+#include "translate.h"
+#include "translate-a32.h"
 
 /* Include the generated VFP decoder */
 #include "decode-vfp.c.inc"
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 9522002d34e..2ed02df05e0 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1167,8 +1167,7 @@ static TCGv_ptr vfp_reg_ptr(bool dp, int reg)
 
 #define ARM_CP_RW_BIT   (1 << 20)
 
-/* Include the VFP and Neon decoders */
-#include "translate-vfp.c.inc"
+/* Include the Neon decoder */
 #include "translate-neon.c.inc"
 
 static inline void iwmmxt_load_reg(TCGv_i64 var, int reg)
diff --git a/target/arm/meson.build b/target/arm/meson.build
index bbee1325bc4..f6360f33f11 100644
--- a/target/arm/meson.build
+++ b/target/arm/meson.build
@@ -3,8 +3,8 @@ gen = [
   decodetree.process('neon-shared.decode', extra_args: 
'--static-decode=disas_neon_shared'),
   decodetree.process('neon-dp.decode', extra_args: 
'--static-decode=disas_neon_dp'),
   decodetree.process('neon-ls.decode', extra_args: 
'--static-decode=disas_neon_ls'),
-  decodetree.process('vfp.decode', extra_args: '--static-decode=disas_vfp'),
-  decodetree.process('vfp-uncond.decode', extra_args: 
'--static-decode=disas_vfp_uncond'),
+  decodetree.process('vfp.decode', extra_args: '--decode=disas_vfp'),
+  decodetree.process('vfp-uncond.decode', extra_args: 
'--decode=disas_vfp_uncond'),
   decodetree.process('m-nocp.decode', extra_args: '--decode=disas_m_nocp'),
   decodetree.process('a32.decode', extra_args: '--static-decode=disas_a32'),
   decodetree.process('a32-uncond.decode', extra_args: 
'--static-decode=disas_a32_uncond'),
@@ -27,6 +27,7 @@ arm_ss.add(files(
   'tlb_helper.c',
   'translate.c',
   'translate-m-nocp.c',
+  'translate-vfp.c',
   'vec_helper.c',
   'vfp_helper.c',
   'cpu_tcg.c',
-- 
2.20.1

[PATCH 06/13] target/arm: Move vfp_{load, store}_reg{32, 64} to translate-vfp.c.inc

[Peter Maydell]

The functions vfp_load_reg32(), vfp_load_reg64(), vfp_store_reg32()
and vfp_store_reg64() are used only in translate-vfp.c.inc. Move
them to that file.

Signed-off-by: Peter Maydell 
---
 target/arm/translate.c | 20 
 target/arm/translate-vfp.c.inc | 20 
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index fd248b101f2..2daabb5fb6f 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1103,26 +1103,6 @@ static long vfp_reg_offset(bool dp, unsigned reg)
 }
 }
 
-static inline void vfp_load_reg64(TCGv_i64 var, int reg)
-{
-tcg_gen_ld_i64(var, cpu_env, vfp_reg_offset(true, reg));
-}
-
-static inline void vfp_store_reg64(TCGv_i64 var, int reg)
-{
-tcg_gen_st_i64(var, cpu_env, vfp_reg_offset(true, reg));
-}
-
-static inline void vfp_load_reg32(TCGv_i32 var, int reg)
-{
-tcg_gen_ld_i32(var, cpu_env, vfp_reg_offset(false, reg));
-}
-
-static inline void vfp_store_reg32(TCGv_i32 var, int reg)
-{
-tcg_gen_st_i32(var, cpu_env, vfp_reg_offset(false, reg));
-}
-
 void read_neon_element32(TCGv_i32 dest, int reg, int ele, MemOp memop)
 {
 long off = neon_element_offset(reg, ele, memop);
diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
index 16a730b7bdd..873a6237ea1 100644
--- a/target/arm/translate-vfp.c.inc
+++ b/target/arm/translate-vfp.c.inc
@@ -30,6 +30,26 @@
 #include "decode-vfp.c.inc"
 #include "decode-vfp-uncond.c.inc"
 
+static inline void vfp_load_reg64(TCGv_i64 var, int reg)
+{
+tcg_gen_ld_i64(var, cpu_env, vfp_reg_offset(true, reg));
+}
+
+static inline void vfp_store_reg64(TCGv_i64 var, int reg)
+{
+tcg_gen_st_i64(var, cpu_env, vfp_reg_offset(true, reg));
+}
+
+static inline void vfp_load_reg32(TCGv_i32 var, int reg)
+{
+tcg_gen_ld_i32(var, cpu_env, vfp_reg_offset(false, reg));
+}
+
+static inline void vfp_store_reg32(TCGv_i32 var, int reg)
+{
+tcg_gen_st_i32(var, cpu_env, vfp_reg_offset(false, reg));
+}
+
 /*
  * The imm8 encodes the sign bit, enough bits to represent an exponent in
  * the range 011xx to 100xx, and the most significant 4 bits of
-- 
2.20.1

[PATCH 04/13] target/arm: Split m-nocp trans functions into their own file

[Peter Maydell]

Currently the trans functions for m-nocp.decode all live in
translate-vfp.inc.c; move them out into their own translation unit,
translate-m-nocp.c.

The trans_* functions here are pure code motion with no changes.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-a32.h |   3 +
 target/arm/translate-m-nocp.c  | 221 +
 target/arm/translate.c |   1 -
 target/arm/translate-vfp.c.inc | 196 -
 target/arm/meson.build |   3 +-
 5 files changed, 226 insertions(+), 198 deletions(-)
 create mode 100644 target/arm/translate-m-nocp.c

diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h
index c5d937b27e8..cb451f70a42 100644
--- a/target/arm/translate-a32.h
+++ b/target/arm/translate-a32.h
@@ -20,6 +20,9 @@
 #ifndef TARGET_ARM_TRANSLATE_A64_H
 #define TARGET_ARM_TRANSLATE_A64_H
 
+/* Prototypes for autogenerated disassembler functions */
+bool disas_m_nocp(DisasContext *dc, uint32_t insn);
+
 void load_reg_var(DisasContext *s, TCGv_i32 var, int reg);
 void arm_gen_condlabel(DisasContext *s);
 bool vfp_access_check(DisasContext *s);
diff --git a/target/arm/translate-m-nocp.c b/target/arm/translate-m-nocp.c
new file mode 100644
index 000..d47eb8e1535
--- /dev/null
+++ b/target/arm/translate-m-nocp.c
@@ -0,0 +1,221 @@
+/*
+ *  ARM translation: M-profile NOCP special-case instructions
+ *
+ *  Copyright (c) 2020 Linaro, Ltd.
+ *
+ * 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 .
+ */
+
+#include "qemu/osdep.h"
+#include "tcg/tcg-op.h"
+#include "translate.h"
+#include "translate-a32.h"
+
+#include "decode-m-nocp.c.inc"
+
+/*
+ * Decode VLLDM and VLSTM are nonstandard because:
+ *  * if there is no FPU then these insns must NOP in
+ *Secure state and UNDEF in Nonsecure state
+ *  * if there is an FPU then these insns do not have
+ *the usual behaviour that vfp_access_check() provides of
+ *being controlled by CPACR/NSACR enable bits or the
+ *lazy-stacking logic.
+ */
+static bool trans_VLLDM_VLSTM(DisasContext *s, arg_VLLDM_VLSTM *a)
+{
+TCGv_i32 fptr;
+
+if (!arm_dc_feature(s, ARM_FEATURE_M) ||
+!arm_dc_feature(s, ARM_FEATURE_V8)) {
+return false;
+}
+
+if (a->op) {
+/*
+ * T2 encoding ({D0-D31} reglist): v8.1M and up. We choose not
+ * to take the IMPDEF option to make memory accesses to the stack
+ * slots that correspond to the D16-D31 registers (discarding
+ * read data and writing UNKNOWN values), so for us the T2
+ * encoding behaves identically to the T1 encoding.
+ */
+if (!arm_dc_feature(s, ARM_FEATURE_V8_1M)) {
+return false;
+}
+} else {
+/*
+ * T1 encoding ({D0-D15} reglist); undef if we have 32 Dregs.
+ * This is currently architecturally impossible, but we add the
+ * check to stay in line with the pseudocode. Note that we must
+ * emit code for the UNDEF so it takes precedence over the NOCP.
+ */
+if (dc_isar_feature(aa32_simd_r32, s)) {
+unallocated_encoding(s);
+return true;
+}
+}
+
+/*
+ * If not secure, UNDEF. We must emit code for this
+ * rather than returning false so that this takes
+ * precedence over the m-nocp.decode NOCP fallback.
+ */
+if (!s->v8m_secure) {
+unallocated_encoding(s);
+return true;
+}
+/* If no fpu, NOP. */
+if (!dc_isar_feature(aa32_vfp, s)) {
+return true;
+}
+
+fptr = load_reg(s, a->rn);
+if (a->l) {
+gen_helper_v7m_vlldm(cpu_env, fptr);
+} else {
+gen_helper_v7m_vlstm(cpu_env, fptr);
+}
+tcg_temp_free_i32(fptr);
+
+/* End the TB, because we have updated FP control bits */
+s->base.is_jmp = DISAS_UPDATE_EXIT;
+return true;
+}
+
+static bool trans_VSCCLRM(DisasContext *s, arg_VSCCLRM *a)
+{
+int btmreg, topreg;
+TCGv_i64 zero;
+TCGv_i32 aspen, sfpa;
+
+if (!dc_isar_feature(aa32_m_sec_state, s)) {
+/* Before v8.1M, fall through in decode to NOCP check */
+return false;
+}
+
+/* Explicitly UNDEF because this takes precedence over NOCP */
+if (!arm_dc_feature(s, ARM_FEATURE_M_MAIN) || !s->v8m_secure) {
+unallocated_encoding(s);
+return true;
+}
+
+if 

[PATCH 03/13] target/arm: Make functions used by m-nocp global

[Peter Maydell]

We want to split out the .c.inc files which are currently included
into translate.c so they are separate compilation units.  To do this
we need to make some functions which are currently file-local to
translate.c have global scope; create a translate-a32.h paralleling
the existing translate-a64.h as a place for these declarations to
live, so that code moved into the new compilation units can call
them.

The functions made global here are those required by the
m-nocp.decode functions, except that I have converted the whole
family of {read,write}_neon_element* and also both the load_cpu and
store_cpu functions for consistency, even though m-nocp only wants a
few functions from each.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-a32.h | 57 ++
 target/arm/translate.c | 39 +--
 target/arm/translate-vfp.c.inc |  2 +-
 3 files changed, 65 insertions(+), 33 deletions(-)
 create mode 100644 target/arm/translate-a32.h

diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h
new file mode 100644
index 000..c5d937b27e8
--- /dev/null
+++ b/target/arm/translate-a32.h
@@ -0,0 +1,57 @@
+/*
+ *  AArch32 translation, common definitions.
+ *
+ * Copyright (c) 2021 Linaro, Ltd.
+ *
+ * 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 .
+ */
+
+#ifndef TARGET_ARM_TRANSLATE_A64_H
+#define TARGET_ARM_TRANSLATE_A64_H
+
+void load_reg_var(DisasContext *s, TCGv_i32 var, int reg);
+void arm_gen_condlabel(DisasContext *s);
+bool vfp_access_check(DisasContext *s);
+void read_neon_element32(TCGv_i32 dest, int reg, int ele, MemOp memop);
+void read_neon_element64(TCGv_i64 dest, int reg, int ele, MemOp memop);
+void write_neon_element32(TCGv_i32 src, int reg, int ele, MemOp memop);
+void write_neon_element64(TCGv_i64 src, int reg, int ele, MemOp memop);
+
+static inline TCGv_i32 load_cpu_offset(int offset)
+{
+TCGv_i32 tmp = tcg_temp_new_i32();
+tcg_gen_ld_i32(tmp, cpu_env, offset);
+return tmp;
+}
+
+#define load_cpu_field(name) load_cpu_offset(offsetof(CPUARMState, name))
+
+static inline void store_cpu_offset(TCGv_i32 var, int offset)
+{
+tcg_gen_st_i32(var, cpu_env, offset);
+tcg_temp_free_i32(var);
+}
+
+#define store_cpu_field(var, name) \
+store_cpu_offset(var, offsetof(CPUARMState, name))
+
+/* Create a new temporary and set it to the value of a CPU register.  */
+static inline TCGv_i32 load_reg(DisasContext *s, int reg)
+{
+TCGv_i32 tmp = tcg_temp_new_i32();
+load_reg_var(s, tmp, reg);
+return tmp;
+}
+
+#endif
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 24f50dea669..fb86427b11c 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -50,6 +50,7 @@
 #define ENABLE_ARCH_8 arm_dc_feature(s, ARM_FEATURE_V8)
 
 #include "translate.h"
+#include "translate-a32.h"
 
 #if defined(CONFIG_USER_ONLY)
 #define IS_USER(s) 1
@@ -101,7 +102,7 @@ void arm_translate_init(void)
 }
 
 /* Generate a label used for skipping this instruction */
-static void arm_gen_condlabel(DisasContext *s)
+void arm_gen_condlabel(DisasContext *s)
 {
 if (!s->condjmp) {
 s->condlabel = gen_new_label();
@@ -187,24 +188,6 @@ static inline int get_a32_user_mem_index(DisasContext *s)
 }
 }
 
-static inline TCGv_i32 load_cpu_offset(int offset)
-{
-TCGv_i32 tmp = tcg_temp_new_i32();
-tcg_gen_ld_i32(tmp, cpu_env, offset);
-return tmp;
-}
-
-#define load_cpu_field(name) load_cpu_offset(offsetof(CPUARMState, name))
-
-static inline void store_cpu_offset(TCGv_i32 var, int offset)
-{
-tcg_gen_st_i32(var, cpu_env, offset);
-tcg_temp_free_i32(var);
-}
-
-#define store_cpu_field(var, name) \
-store_cpu_offset(var, offsetof(CPUARMState, name))
-
 /* The architectural value of PC.  */
 static uint32_t read_pc(DisasContext *s)
 {
@@ -212,7 +195,7 @@ static uint32_t read_pc(DisasContext *s)
 }
 
 /* Set a variable to the value of a CPU register.  */
-static void load_reg_var(DisasContext *s, TCGv_i32 var, int reg)
+void load_reg_var(DisasContext *s, TCGv_i32 var, int reg)
 {
 if (reg == 15) {
 tcg_gen_movi_i32(var, read_pc(s));
@@ -221,14 +204,6 @@ static void load_reg_var(DisasContext *s, TCGv_i32 var, 
int reg)
 }
 }
 
-/* Create a new temporary and set it to the value of a CPU register.  */
-static inline TCGv_i32 load_reg(DisasContext *s, int 

[PATCH 01/13] target/arm: Move constant expanders to translate.h

[Peter Maydell]

Some of the constant expanders defined in translate.c are generically
useful and will be used by the separate C files for VFP and Neon once
they are created; move the expander definitions to translate.h.

Signed-off-by: Peter Maydell 
---
 target/arm/translate.h | 24 
 target/arm/translate.c | 24 
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/target/arm/translate.h b/target/arm/translate.h
index 423b0e08df0..4c0b6e8fc42 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -116,6 +116,30 @@ extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
 extern TCGv_i64 cpu_exclusive_addr;
 extern TCGv_i64 cpu_exclusive_val;
 
+/*
+ * Constant expanders for the decoders.
+ */
+
+static inline int negate(DisasContext *s, int x)
+{
+return -x;
+}
+
+static inline int plus_2(DisasContext *s, int x)
+{
+return x + 2;
+}
+
+static inline int times_2(DisasContext *s, int x)
+{
+return x * 2;
+}
+
+static inline int times_4(DisasContext *s, int x)
+{
+return x * 4;
+}
+
 static inline int arm_dc_feature(DisasContext *dc, int feature)
 {
 return (dc->features & (1ULL << feature)) != 0;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 62b1c2081b6..0e30892d54e 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -109,30 +109,6 @@ static void arm_gen_condlabel(DisasContext *s)
 }
 }
 
-/*
- * Constant expanders for the decoders.
- */
-
-static int negate(DisasContext *s, int x)
-{
-return -x;
-}
-
-static int plus_2(DisasContext *s, int x)
-{
-return x + 2;
-}
-
-static int times_2(DisasContext *s, int x)
-{
-return x * 2;
-}
-
-static int times_4(DisasContext *s, int x)
-{
-return x * 4;
-}
-
 /* Flags for the disas_set_da_iss info argument:
  * lower bits hold the Rt register number, higher bits are flags.
  */
-- 
2.20.1

[PATCH 02/13] target/arm: Share unallocated_encoding() and gen_exception_insn()

[Peter Maydell]

The unallocated_encoding() function is the same in both
translate-a64.c and translate.c; make the translate.c function global
and drop the translate-a64.c version.  To do this we need to also
share gen_exception_insn(), which currently exists in two slightly
different versions for A32 and A64: merge those into a single
function that can work for both.

This will be useful for splitting up translate.c, which will require
unallocated_encoding() to no longer be file-local.  It's also
hopefully less confusing to have only one version of the function
rather than two.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-a64.h |  2 --
 target/arm/translate.h |  3 +++
 target/arm/translate-a64.c | 15 ---
 target/arm/translate.c | 14 +-
 4 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h
index 3668b671ddb..ee15f084982 100644
--- a/target/arm/translate-a64.h
+++ b/target/arm/translate-a64.h
@@ -18,8 +18,6 @@
 #ifndef TARGET_ARM_TRANSLATE_A64_H
 #define TARGET_ARM_TRANSLATE_A64_H
 
-void unallocated_encoding(DisasContext *s);
-
 #define unsupported_encoding(s, insn)\
 do { \
 qemu_log_mask(LOG_UNIMP, \
diff --git a/target/arm/translate.h b/target/arm/translate.h
index 4c0b6e8fc42..a9f90e3ed4c 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -226,6 +226,9 @@ void arm_test_cc(DisasCompare *cmp, int cc);
 void arm_free_cc(DisasCompare *cmp);
 void arm_jump_cc(DisasCompare *cmp, TCGLabel *label);
 void arm_gen_test_cc(int cc, TCGLabel *label);
+void unallocated_encoding(DisasContext *s);
+void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
+uint32_t syn, uint32_t target_el);
 
 /* Return state of Alternate Half-precision flag, caller frees result */
 static inline TCGv_i32 get_ahp_flag(void)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 0b42e53500e..4ce28ec54db 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -360,14 +360,6 @@ static void gen_exception_internal_insn(DisasContext *s, 
uint64_t pc, int excp)
 s->base.is_jmp = DISAS_NORETURN;
 }
 
-static void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
-   uint32_t syndrome, uint32_t target_el)
-{
-gen_a64_set_pc_im(pc);
-gen_exception(excp, syndrome, target_el);
-s->base.is_jmp = DISAS_NORETURN;
-}
-
 static void gen_exception_bkpt_insn(DisasContext *s, uint32_t syndrome)
 {
 TCGv_i32 tcg_syn;
@@ -438,13 +430,6 @@ static inline void gen_goto_tb(DisasContext *s, int n, 
uint64_t dest)
 }
 }
 
-void unallocated_encoding(DisasContext *s)
-{
-/* Unallocated and reserved encodings are uncategorized */
-gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-   default_exception_el(s));
-}
-
 static void init_tmp_a64_array(DisasContext *s)
 {
 #ifdef CONFIG_DEBUG_TCG
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 0e30892d54e..24f50dea669 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -1042,11 +1042,15 @@ static void gen_exception_internal_insn(DisasContext 
*s, uint32_t pc, int excp)
 s->base.is_jmp = DISAS_NORETURN;
 }
 
-static void gen_exception_insn(DisasContext *s, uint32_t pc, int excp,
-   int syn, uint32_t target_el)
+void gen_exception_insn(DisasContext *s, uint64_t pc, int excp,
+uint32_t syn, uint32_t target_el)
 {
-gen_set_condexec(s);
-gen_set_pc_im(s, pc);
+if (s->aarch64) {
+gen_a64_set_pc_im(pc);
+} else {
+gen_set_condexec(s);
+gen_set_pc_im(s, pc);
+}
 gen_exception(excp, syn, target_el);
 s->base.is_jmp = DISAS_NORETURN;
 }
@@ -1063,7 +1067,7 @@ static void gen_exception_bkpt_insn(DisasContext *s, 
uint32_t syn)
 s->base.is_jmp = DISAS_NORETURN;
 }
 
-static void unallocated_encoding(DisasContext *s)
+void unallocated_encoding(DisasContext *s)
 {
 /* Unallocated and reserved encodings are uncategorized */
 gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),
-- 
2.20.1

[PULL v2 1/3] osdep: include glib-compat.h before other QEMU headers

[Paolo Bonzini]

glib-compat.h is sort of like a system header, and it needs to include
system headers (glib.h) that may dislike being included under
'extern "C"'.  Move it right after all system headers and before
all other QEMU headers.

Signed-off-by: Paolo Bonzini 
---
 include/qemu/osdep.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ba15be9c56..b67b0a1e8c 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -111,6 +111,8 @@ extern int daemon(int, int);
 #define WEXITSTATUS(x) (x)
 #endif
 
+#include "glib-compat.h"
+
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
 #endif
@@ -123,7 +125,6 @@ extern int daemon(int, int);
 #include 
 #endif
 
-#include "glib-compat.h"
 #include "qemu/typedefs.h"
 
 /*
-- 
2.30.1

[PATCH 00/13] target/arm: Split translate-*.c.inc into separate compilation units

[Peter Maydell]

When we first converted the A32/T32 frontends to use decodetree,
we put the trans* functions for VFP and Neon into their own
separate files, but used the preprocessor to just #include those
files into translate.c. This was a pragmatic arrangement to avoid
having to also rearrange translate.c which had a lot of utility
functions shared between the main integer decode and the VFP/Neon.

However, having translate.c effectively one enormous file with
all of the integer, Neon and VFP decode in it means it can
push the limits of what some compilers can handle (especially
on 32-bit hosts if the compiler has bugs where it uses too much
memory either during optimisation or while generating debug info).

This series breaks out all the translate-*.c.inc into separate
compilation units.

(My other motivation for this series is that I'm due to start
working on MVE soon. I want that to be its own compilation unit,
the same way translate-sve.c is, and having the various "load
bits of neon vector register" helpers available as globals will
be necessary for that.)

thanks
-- PMM

Peter Maydell (13):
  target/arm: Move constant expanders to translate.h
  target/arm: Share unallocated_encoding() and gen_exception_insn()
  target/arm: Make functions used by m-nocp global
  target/arm: Split m-nocp trans functions into their own file
  target/arm: Move gen_aa32 functions to translate-a32.h
  target/arm: Move vfp_{load,store}_reg{32,64} to translate-vfp.c.inc
  target/arm: Make functions used by translate-vfp global
  target/arm: Make translate-vfp.c.inc its own compilation unit
  target/arm: Move vfp_reg_ptr() to translate-neon.c.inc
  target/arm: Delete unused typedef
  target/arm: Move NeonGenThreeOpEnvFn typedef to translate.h
  target/arm: Make functions used by translate-neon global
  target/arm: Make translate-neon.c.inc its own compilation unit

 target/arm/translate-a32.h| 149 
 target/arm/translate-a64.h|   2 -
 target/arm/translate.h|  29 +++
 target/arm/translate-a64.c|  15 --
 target/arm/translate-m-nocp.c | 221 +
 ...{translate-neon.c.inc => translate-neon.c} |  19 +-
 .../{translate-vfp.c.inc => translate-vfp.c}  | 230 +++---
 target/arm/translate.c| 210 +++-
 target/arm/meson.build|  15 +-
 9 files changed, 482 insertions(+), 408 deletions(-)
 create mode 100644 target/arm/translate-a32.h
 create mode 100644 target/arm/translate-m-nocp.c
 rename target/arm/{translate-neon.c.inc => translate-neon.c} (99%)
 rename target/arm/{translate-vfp.c.inc => translate-vfp.c} (94%)

-- 
2.20.1

[PULL v2 0/3] osdep.h + QOM changes for QEMU 6.0-rc3

[Paolo Bonzini]

The following changes since commit c1e90def01bdb8fcbdbebd9d1eaa8e4827ece620:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' 
into staging (2021-04-12 12:12:09 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 1a0b186eaf3d1ce63dc7bf608d618b9ca62b6241:

  qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code (2021-04-13 
18:04:23 +0200)


* Fix C++ compilation of qemu/osdep.h.
* Fix -object cryptodev-vhost-user


Paolo Bonzini (2):
  osdep: include glib-compat.h before other QEMU headers
  osdep: protect qemu/osdep.h with extern "C"

Thomas Huth (1):
  qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code

 disas/nanomips.cpp  |  2 +-
 include/qemu/compiler.h |  6 ++
 include/qemu/osdep.h| 13 +++--
 qapi/qom.json   |  4 ++--
 4 files changed, 20 insertions(+), 5 deletions(-)
-- 
2.30.1

[PATCH 12/13] target/arm: Make functions used by translate-neon global

[Peter Maydell]

Make the remaining functions needed by the translate-neon code
global.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-a32.h |  8 
 target/arm/translate.c | 10 ++
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/target/arm/translate-a32.h b/target/arm/translate-a32.h
index 96e6eafbde4..f165f15cc47 100644
--- a/target/arm/translate-a32.h
+++ b/target/arm/translate-a32.h
@@ -37,6 +37,8 @@ void gen_set_pc_im(DisasContext *s, target_ulong val);
 void gen_lookup_tb(DisasContext *s);
 long vfp_reg_offset(bool dp, unsigned reg);
 long neon_full_reg_offset(unsigned reg);
+long neon_element_offset(int reg, int element, MemOp memop);
+void gen_rev16(TCGv_i32 dest, TCGv_i32 var);
 
 static inline TCGv_i32 load_cpu_offset(int offset)
 {
@@ -135,4 +137,10 @@ static inline void gen_set_cpsr(TCGv_i32 var, uint32_t 
mask)
 /* Set NZCV flags from the high 4 bits of var.  */
 #define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
 
+/* Swap low and high halfwords.  */
+static inline void gen_swap_half(TCGv_i32 dest, TCGv_i32 var)
+{
+tcg_gen_rotri_i32(dest, var, 16);
+}
+
 #endif
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 7f95ab16613..f6d71d03a3a 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -319,7 +319,7 @@ static void gen_smul_dual(TCGv_i32 a, TCGv_i32 b)
 }
 
 /* Byteswap each halfword.  */
-static void gen_rev16(TCGv_i32 dest, TCGv_i32 var)
+void gen_rev16(TCGv_i32 dest, TCGv_i32 var)
 {
 TCGv_i32 tmp = tcg_temp_new_i32();
 TCGv_i32 mask = tcg_const_i32(0x00ff00ff);
@@ -340,12 +340,6 @@ static void gen_revsh(TCGv_i32 dest, TCGv_i32 var)
 tcg_gen_ext16s_i32(dest, var);
 }
 
-/* Swap low and high halfwords.  */
-static void gen_swap_half(TCGv_i32 dest, TCGv_i32 var)
-{
-tcg_gen_rotri_i32(dest, var, 16);
-}
-
 /* Dual 16-bit add.  Result placed in t0 and t1 is marked as dead.
 tmp = (t0 ^ t1) & 0x8000;
 t0 &= ~0x8000;
@@ -1047,7 +1041,7 @@ long neon_full_reg_offset(unsigned reg)
  * Return the offset of a 2**SIZE piece of a NEON register, at index ELE,
  * where 0 is the least significant end of the register.
  */
-static long neon_element_offset(int reg, int element, MemOp memop)
+long neon_element_offset(int reg, int element, MemOp memop)
 {
 int element_size = 1 << (memop & MO_SIZE);
 int ofs = element * element_size;
-- 
2.20.1

Re: [PATCH] cutils: fix memory leak in get_relocated_path()

[Philippe Mathieu-Daudé]

On 4/13/21 1:47 PM, Stefano Garzarella wrote:
> On Tue, Apr 13, 2021 at 12:59:36PM +0200, Philippe Mathieu-Daudé wrote:
>> Is this fix aiming at 6.0 release?
> 
> The leak is minimal, but the fix is very simple.
> So, I think it can go if someone has a pull request to send with other
> patches, but I'm not sure with which tree.

I'd say Paolo...

Cc'ing Daniel/Marc-André who have a good GLib understanding.

> 
> Thanks,
> Stefano
> 
>>
>> On 4/12/21 7:02 PM, Stefano Garzarella wrote:
>>> get_relocated_path() allocates a GString object and returns the
>>> character data (C string) to the caller without freeing the memory
>>> allocated for that object as reported by valgrind:
>>>
>>>   24 bytes in 1 blocks are definitely lost in loss record 2,805 of 6,532
>>>  at 0x4839809: malloc (vg_replace_malloc.c:307)
>>>  by 0x55AABB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8)
>>>  by 0x55C2481: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.8)
>>>  by 0x55C4827: g_string_sized_new (in
>>> /usr/lib64/libglib-2.0.so.0.6600.8)
>>>  by 0x55C4CEA: g_string_new (in /usr/lib64/libglib-2.0.so.0.6600.8)
>>>  by 0x906314: get_relocated_path (cutils.c:1036)
>>>  by 0x6E1F77: qemu_read_default_config_file (vl.c:2122)
>>>  by 0x6E1F77: qemu_init (vl.c:2687)
>>>  by 0x3E3AF8: main (main.c:49)
>>>
>>> Let's use g_string_free(gstring, false) to free only the GString object
>>> and transfer the ownership of the character data to the caller.
>>>
>>> Fixes: f4f5ed2cbd ("cutils: introduce get_relocated_path")
>>> Signed-off-by: Stefano Garzarella 
>>> ---
>>>  util/cutils.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/util/cutils.c b/util/cutils.c
>>> index ee908486da..c9b91e7535 100644
>>> --- a/util/cutils.c
>>> +++ b/util/cutils.c
>>> @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir)
>>>  assert(G_IS_DIR_SEPARATOR(dir[-1]));
>>>  g_string_append(result, dir - 1);
>>>  }
>>> -    return result->str;
>>> +    return g_string_free(result, false);
>>>  }
>>>
>>
>