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

[Markus Armbruster]

Peter Maydell  writes:

> 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
>
> Given Dan's review, I think that the osdep patches need another
> revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO
> patch here and tag rc3 with just that. If we need an rc4 (which

Uh, I had a question on that one:

Message-ID: <87tuo9j7hw@dusky.pond.sub.org>
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg02341.html

> on our current track record is not unlikely) we can put in some
> version of the osdep patches; if not, this isn't a regression
> since 5.2 so I'm happy releasing 6.0 with it still present.
>
> thanks
> -- PMM

Re: [PATCH 00/38] target/riscv: support packed extension v0.9.2

[LIU Zhiwei]

On 2021/4/15 下午12:46, Alistair Francis wrote:

On Tue, Apr 13, 2021 at 1:28 PM LIU Zhiwei  wrote:

ping +1.

On 2021/2/12 下午11:02, LIU Zhiwei wrote:

This patchset implements the packed extension for RISC-V on QEMU.

This patchset have passed all my direct Linux user mode cases(RV64) and
bare metal cases(RV32) on X86-64 Ubuntu host machine. I will later push
these test cases to my repo(https://github.com/romanheros/qemu.git
branch:packed-upstream-v1).

I have ported packed extension on RISU, but I didn't find a simulator or
hardware to compare with. If anyone have one, please let me know.

Features:
* support specification packed extension 
v0.9.2(https://github.com/riscv/riscv-p-spec/)
* support basic packed extension.
* support Zp64.

LIU Zhiwei (38):
target/riscv: implementation-defined constant parameters
target/riscv: Hoist vector functions
target/riscv: Fixup saturate subtract function

Thanks for the patches and sorry for the long delay.

I have applied patch 3 as it fixes a bug.
As for the other patches they are on both my review queue and Palmer's
review queue. It takes a lot of time to review these large patch
series, especially as I haven't been involved with the extension
development, so I have to both understand the extension and then
review the code.

If you would like to help speed things up you could review other
patches. That way I will have more time left to review your patches.


No worries. I fully understand the great efforts needed to review so 
many patches. Firstly, I will try to review as many  as I send.


Zhiwei


Alistair


target/riscv: 16-bit Addition & Subtraction Instructions
target/riscv: 8-bit Addition & Subtraction Instruction
target/riscv: SIMD 16-bit Shift Instructions
target/riscv: SIMD 8-bit Shift Instructions
target/riscv: SIMD 16-bit Compare Instructions
target/riscv: SIMD 8-bit Compare Instructions
target/riscv: SIMD 16-bit Multiply Instructions
target/riscv: SIMD 8-bit Multiply Instructions
target/riscv: SIMD 16-bit Miscellaneous Instructions
target/riscv: SIMD 8-bit Miscellaneous Instructions
target/riscv: 8-bit Unpacking Instructions
target/riscv: 16-bit Packing Instructions
target/riscv: Signed MSW 32x32 Multiply and Add Instructions
target/riscv: Signed MSW 32x16 Multiply and Add Instructions
target/riscv: Signed 16-bit Multiply 32-bit Add/Subtract Instructions
target/riscv: Signed 16-bit Multiply 64-bit Add/Subtract Instructions
target/riscv: Partial-SIMD Miscellaneous Instructions
target/riscv: 8-bit Multiply with 32-bit Add Instructions
target/riscv: 64-bit Add/Subtract Instructions
target/riscv: 32-bit Multiply 64-bit Add/Subtract Instructions
target/riscv: Signed 16-bit Multiply with 64-bit Add/Subtract
  Instructions
target/riscv: Non-SIMD Q15 saturation ALU Instructions
target/riscv: Non-SIMD Q31 saturation ALU Instructions
target/riscv: 32-bit Computation Instructions
target/riscv: Non-SIMD Miscellaneous Instructions
target/riscv: RV64 Only SIMD 32-bit Add/Subtract Instructions
target/riscv: RV64 Only SIMD 32-bit Shift Instructions
target/riscv: RV64 Only SIMD 32-bit Miscellaneous Instructions
target/riscv: RV64 Only SIMD Q15 saturating Multiply Instructions
target/riscv: RV64 Only 32-bit Multiply Instructions
target/riscv: RV64 Only 32-bit Multiply & Add Instructions
target/riscv: RV64 Only 32-bit Parallel Multiply & Add Instructions
target/riscv: RV64 Only Non-SIMD 32-bit Shift Instructions
target/riscv: RV64 Only 32-bit Packing Instructions
target/riscv: configure and turn on packed extension from command line

   target/riscv/cpu.c  |   32 +
   target/riscv/cpu.h  |6 +
   target/riscv/helper.h   |  332 ++
   target/riscv/insn32-64.decode   |   93 +-
   target/riscv/insn32.decode  |  285 ++
   target/riscv/insn_trans/trans_rvp.c.inc | 1224 +++
   target/riscv/internals.h|   50 +
   target/riscv/meson.build|1 +
   target/riscv/packed_helper.c| 3862 +++
   target/riscv/translate.c|3 +
   target/riscv/vector_helper.c|   90 +-
   11 files changed, 5912 insertions(+), 66 deletions(-)
   create mode 100644 target/riscv/insn_trans/trans_rvp.c.inc
   create mode 100644 target/riscv/packed_helper.c

[PATCH v2 3/4] 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.

Reviewed-by: Cédric Le Goater 
Tested-by: Cédric Le Goater 
Signed-off-by: Nicholas Piggin 
---
 hw/ppc/spapr_hcall.c|   3 +-
 target/ppc/cpu.h|   8 --
 target/ppc/excp_helper.c| 159 
 target/ppc/translate_init.c.inc |   2 +-
 4 files changed, 102 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..964a58cfdc 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -136,25 +136,105 @@ 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 then 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 specifically want to be in real mode (e.g., the 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],
+ * 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.
+ */
+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)) {

[PATCH v2 4/4] 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.

Reviewed-by: Cédric Le Goater 
Tested-by: Cédric Le Goater 
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| 51 +++--
 target/ppc/translate.c  |  3 +-
 target/ppc/translate_init.c.inc |  2 +-
 6 files changed, 62 insertions(+), 8 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 964a58cfdc..38a1482519 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -170,7 +170,27 @@ 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.
+ * the hypervisor in AIL mode if the guest is radix. 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
+ * interrupts that begin execution with MSR[HV]=1 (so both MSR[HV] 0->1 and
+ * MSR[HV] 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,
@@ -210,6 +230,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 

[PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model

[Nicholas Piggin]

Here's a rollup of where this ended up, hopefully it suits everyone's
preference. Thanks for the review and catching several issues.

Patches 1-3 are unchanged except for minor comment and changelog tweaks,
patch 4 contains fixes for the issues Cedric noticed.

Thanks,
Nick

Nicholas Piggin (4):
  target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour
  target/ppc: POWER10 supports scv
  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| 199 +++-
 target/ppc/translate.c  |   3 +-
 target/ppc/translate_init.c.inc |   6 +-
 6 files changed, 163 insertions(+), 68 deletions(-)

-- 
2.23.0

[PATCH v2 2/4] target/ppc: POWER10 supports scv

[Nicholas Piggin]

This must have slipped through the cracks between adding POWER10 support
and scv support.

Signed-off-by: Nicholas Piggin 
---
 target/ppc/translate_init.c.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index c03a7c4f52..70f9b9b150 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -9323,7 +9323,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
 pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
  POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
  POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
- POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
+ POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
 pcc->l1_dcache_size = 0x8000;
 pcc->l1_icache_size = 0x8000;
 pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
-- 
2.23.0

[PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour

[Nicholas Piggin]

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).

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",
-- 
2.23.0

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

[Nicholas Piggin]

Excerpts from Cédric Le Goater's message of April 15, 2021 1:54 am:
> On 4/14/21 5:23 AM, Nicholas Piggin wrote:
>> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
>> and it removes support for the LPCR[AIL]=0b10 mode.
> 
> This looks good but it's missing the MSR_LE setting. A part from that : 

Oh, and lpes as well. Looks like a mis-merged from my original patch.
Thanks for catching it, great.

> 
> Reviewed-by: Cédric Le Goater 
> 
> and 
> 
> Tested-by: Cédric Le Goater 

Thanks, this was tested after you added the MSR_LE bit?
> 
> distros using scv on P10 now need your patch to boot :
> 
> "powerpc/powernv: Enable HAIL (HV AIL) for ISA v3.1 processors"
> 
> I guess it will get merged in time. 

Yes, unfortunately. Real hardware crashes the same way though, so
nothing to be done about it.

Thanks,
Nick

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

[Nicholas Piggin]

Excerpts from Cédric Le Goater's message of April 15, 2021 1:24 am:
> On 4/14/21 5:23 AM, Nicholas Piggin wrote:
>> 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.
> 
> Reviewed-by: Cédric Le Goater 
> Tested-by: Cédric Le Goater 
> 
> Thanks for the effort and the documentation. One minor comment below,
> 
> C.
> 
>> 
>> 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,
>> -};
> 
> I kind of like these. No big deal.

My thinking was they actually are just a POWER8 model of the AIL bits 
(e.g., they don't represent scv properly or AIL=2 reserved in P10), and 
they spread the meaning over multiple files. After this patch it's all
just in that single function.

>> 
>> -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);
> 
> Could we keep this abort ? 

Well the abort is no longer there because we explicitly handle all 
cases, the reserved ones by just ignoring them. I don't know what
the hardware actually does if you tried to set it (it should ignore)
but I think this is nicer to not abort.

Thanks,
Nick

[Bug 1923629] Re: RISC-V Vector Instruction vssub.vv not saturating

[Alistair Francis]

Thanks for raising this bug case. A fix should be available soon.

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

Title:
  RISC-V Vector Instruction vssub.vv not saturating

Status in QEMU:
  New

Bug description:
  I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
  incorrect result of 0x8000 (should saturate to 0x7FFF).

  Here is the bit of the code:

vmv.v.i v16, 0
…
  8f040457  vssub.vvv8,v16,v8

  I believe the instruction encoding is correct (vssub.vv with vd = v8,
  vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.

  I’ve just tested with what I think is the latest branch (
  https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
  2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
  exists.

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

Re: [PATCH] docs: Add documentation for shakti_c machine

[Alistair Francis]

On Tue, Apr 13, 2021 at 3:44 AM Vijai Kumar K  wrote:
>
> Add documentation for Shakti C reference platform.
>
> Signed-off-by: Vijai Kumar K 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  docs/system/riscv/shakti-c.rst | 82 ++
>  1 file changed, 82 insertions(+)
>  create mode 100644 docs/system/riscv/shakti-c.rst
>
> diff --git a/docs/system/riscv/shakti-c.rst b/docs/system/riscv/shakti-c.rst
> new file mode 100644
> index 00..a6035d42b0
> --- /dev/null
> +++ b/docs/system/riscv/shakti-c.rst
> @@ -0,0 +1,82 @@
> +Shakti C Reference Platform (``shakti_c``)
> +==
> +
> +Shakti C Reference Platform is a reference platform based on arty a7 100t
> +for the Shakti SoC.
> +
> +Shakti SoC is a SoC based on the Shakti C-class processor core. Shakti C
> +is a 64bit RV64GCSUN processor core.
> +
> +For more details on Shakti SoC, please see:
> +https://gitlab.com/shaktiproject/cores/shakti-soc/-/blob/master/fpga/boards/artya7-100t/c-class/README.rst
> +
> +For more info on the Shakti C-class core, please see:
> +https://c-class.readthedocs.io/en/latest/
> +
> +Supported devices
> +-
> +
> +The ``shakti_c`` machine supports the following devices:
> +
> + * 1 C-class core
> + * Core Level Interruptor (CLINT)
> + * Platform-Level Interrupt Controller (PLIC)
> + * 1 UART
> +
> +Boot options
> +
> +
> +The ``shakti_c`` machine can start using the standard -bios
> +functionality for loading the baremetal application or opensbi.
> +
> +Boot the machine
> +
> +
> +Shakti SDK
> +~~
> +Shakti SDK can be used to generate the baremetal example UART applications.
> +
> +.. code-block:: bash
> +
> +   $ git clone https://gitlab.com/behindbytes/shakti-sdk.git
> +   $ cd shakti-sdk
> +   $ make software PROGRAM=loopback TARGET=artix7_100t
> +
> +Binary would be generated in:
> +  software/examples/uart_applns/loopback/output/loopback.shakti
> +
> +You could also download the precompiled example applicatons using below
> +commands.
> +
> +.. code-block:: bash
> +
> +   $ wget -c 
> https://gitlab.com/behindbytes/shakti-binaries/-/raw/master/sdk/shakti_sdk_qemu.zip
> +   $ unzip shakti_sdk_qemu.zip
> +
> +Then we can run the UART example using:
> +
> +.. code-block:: bash
> +
> +   $ qemu-system-riscv64 -M shakti_c -nographic \
> +  -bios path/to/shakti_sdk_qemu/loopback.shakti
> +
> +OpenSBI
> +~~~
> +We can also run OpenSBI with Test Payload.
> +
> +.. code-block:: bash
> +
> +   $ git clone https://github.com/riscv/opensbi.git -b v0.9
> +   $ cd opensbi
> +   $ wget -c 
> https://gitlab.com/behindbytes/shakti-binaries/-/raw/master/dts/shakti.dtb
> +   $ export CROSS_COMPILE=riscv64-unknown-elf-
> +   $ export FW_FDT_PATH=./shakti.dtb
> +   $ make PLATFORM=generic
> +
> +fw_payload.elf would be generated in 
> build/platform/generic/firmware/fw_payload.elf.
> +Boot it using the below qemu command.
> +
> +.. code-block:: bash
> +
> +   $ qemu-system-riscv64 -M shakti_c -nographic \
> +  -bios path/to/fw_payload.elf
> --
> 2.25.1
>
>
>

Re: [PATCH 00/38] target/riscv: support packed extension v0.9.2

[Alistair Francis]

On Tue, Apr 13, 2021 at 1:28 PM LIU Zhiwei  wrote:
>
> ping +1.
>
> On 2021/2/12 下午11:02, LIU Zhiwei wrote:
> > This patchset implements the packed extension for RISC-V on QEMU.
> >
> > This patchset have passed all my direct Linux user mode cases(RV64) and
> > bare metal cases(RV32) on X86-64 Ubuntu host machine. I will later push
> > these test cases to my repo(https://github.com/romanheros/qemu.git
> > branch:packed-upstream-v1).
> >
> > I have ported packed extension on RISU, but I didn't find a simulator or
> > hardware to compare with. If anyone have one, please let me know.
> >
> > Features:
> >* support specification packed extension 
> > v0.9.2(https://github.com/riscv/riscv-p-spec/)
> >* support basic packed extension.
> >* support Zp64.
> >
> > LIU Zhiwei (38):
> >target/riscv: implementation-defined constant parameters
> >target/riscv: Hoist vector functions
> >target/riscv: Fixup saturate subtract function

Thanks for the patches and sorry for the long delay.

I have applied patch 3 as it fixes a bug.

As for the other patches they are on both my review queue and Palmer's
review queue. It takes a lot of time to review these large patch
series, especially as I haven't been involved with the extension
development, so I have to both understand the extension and then
review the code.

If you would like to help speed things up you could review other
patches. That way I will have more time left to review your patches.

Alistair

> >target/riscv: 16-bit Addition & Subtraction Instructions
> >target/riscv: 8-bit Addition & Subtraction Instruction
> >target/riscv: SIMD 16-bit Shift Instructions
> >target/riscv: SIMD 8-bit Shift Instructions
> >target/riscv: SIMD 16-bit Compare Instructions
> >target/riscv: SIMD 8-bit Compare Instructions
> >target/riscv: SIMD 16-bit Multiply Instructions
> >target/riscv: SIMD 8-bit Multiply Instructions
> >target/riscv: SIMD 16-bit Miscellaneous Instructions
> >target/riscv: SIMD 8-bit Miscellaneous Instructions
> >target/riscv: 8-bit Unpacking Instructions
> >target/riscv: 16-bit Packing Instructions
> >target/riscv: Signed MSW 32x32 Multiply and Add Instructions
> >target/riscv: Signed MSW 32x16 Multiply and Add Instructions
> >target/riscv: Signed 16-bit Multiply 32-bit Add/Subtract Instructions
> >target/riscv: Signed 16-bit Multiply 64-bit Add/Subtract Instructions
> >target/riscv: Partial-SIMD Miscellaneous Instructions
> >target/riscv: 8-bit Multiply with 32-bit Add Instructions
> >target/riscv: 64-bit Add/Subtract Instructions
> >target/riscv: 32-bit Multiply 64-bit Add/Subtract Instructions
> >target/riscv: Signed 16-bit Multiply with 64-bit Add/Subtract
> >  Instructions
> >target/riscv: Non-SIMD Q15 saturation ALU Instructions
> >target/riscv: Non-SIMD Q31 saturation ALU Instructions
> >target/riscv: 32-bit Computation Instructions
> >target/riscv: Non-SIMD Miscellaneous Instructions
> >target/riscv: RV64 Only SIMD 32-bit Add/Subtract Instructions
> >target/riscv: RV64 Only SIMD 32-bit Shift Instructions
> >target/riscv: RV64 Only SIMD 32-bit Miscellaneous Instructions
> >target/riscv: RV64 Only SIMD Q15 saturating Multiply Instructions
> >target/riscv: RV64 Only 32-bit Multiply Instructions
> >target/riscv: RV64 Only 32-bit Multiply & Add Instructions
> >target/riscv: RV64 Only 32-bit Parallel Multiply & Add Instructions
> >target/riscv: RV64 Only Non-SIMD 32-bit Shift Instructions
> >target/riscv: RV64 Only 32-bit Packing Instructions
> >target/riscv: configure and turn on packed extension from command line
> >
> >   target/riscv/cpu.c  |   32 +
> >   target/riscv/cpu.h  |6 +
> >   target/riscv/helper.h   |  332 ++
> >   target/riscv/insn32-64.decode   |   93 +-
> >   target/riscv/insn32.decode  |  285 ++
> >   target/riscv/insn_trans/trans_rvp.c.inc | 1224 +++
> >   target/riscv/internals.h|   50 +
> >   target/riscv/meson.build|1 +
> >   target/riscv/packed_helper.c| 3862 +++
> >   target/riscv/translate.c|3 +
> >   target/riscv/vector_helper.c|   90 +-
> >   11 files changed, 5912 insertions(+), 66 deletions(-)
> >   create mode 100644 target/riscv/insn_trans/trans_rvp.c.inc
> >   create mode 100644 target/riscv/packed_helper.c
> >

Re: [Bug 1923629] [NEW] RISC-V Vector Instruction vssub.vv not saturating

[Alistair Francis]

On Thu, Apr 15, 2021 at 2:18 PM LIU Zhiwei  wrote:
>
> Hi Alistair,
>
> I think that this bug has been resolved in my packed-extension patch set[1].
>
> Would you mind to have a test and merge it before the whole patch set?

Great! Thanks

I have applied patch 3 for the next PR.

Alistair

>
> Thanks.
>
>
> Best Regards,
>
> Zhiwei
>
> [1]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg782125.html
>
>
>
> On 2021/4/15 上午11:57, Alistair Francis wrote:
> > + LIU Zhiwei and Kito Cheng
> >
> > Alistair
> >
> > On Wed, Apr 14, 2021 at 1:31 AM Tony Cole <1923...@bugs.launchpad.net> 
> > wrote:
> >> Public bug reported:
> >>
> >> I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
> >> incorrect result of 0x8000 (should saturate to 0x7FFF).
> >>
> >> Here is the bit of the code:
> >>
> >>  vmv.v.i v16, 0
> >>  …
> >> 8f040457vssub.vvv8,v16,v8
> >>
> >> I believe the instruction encoding is correct (vssub.vv with vd = v8,
> >> vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.
> >>
> >> I’ve just tested with what I think is the latest branch (
> >> https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
> >> 2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
> >> exists.
> >>
> >> ** Affects: qemu
> >>   Importance: Undecided
> >>   Status: New
> >>
> >>
> >> ** Tags: riscv vector
> >>
> >> --
> >> You received this bug notification because you are a member of qemu-
> >> devel-ml, which is subscribed to QEMU.
> >> https://bugs.launchpad.net/bugs/1923629
> >>
> >> Title:
> >>RISC-V Vector Instruction vssub.vv not saturating
> >>
> >> Status in QEMU:
> >>New
> >>
> >> Bug description:
> >>I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
> >>incorrect result of 0x8000 (should saturate to 0x7FFF).
> >>
> >>Here is the bit of the code:
> >>
> >>  vmv.v.i v16, 0
> >>  …
> >>8f040457  vssub.vvv8,v16,v8
> >>
> >>I believe the instruction encoding is correct (vssub.vv with vd = v8,
> >>vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.
> >>
> >>I’ve just tested with what I think is the latest branch (
> >>https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
> >>2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
> >>exists.
> >>
> >> To manage notifications about this bug go to:
> >> https://bugs.launchpad.net/qemu/+bug/1923629/+subscriptions
> >>

Re: [Bug 1923629] [NEW] RISC-V Vector Instruction vssub.vv not saturating

[LIU Zhiwei]

Hi Alistair,

I think that this bug has been resolved in my packed-extension patch set[1].

Would you mind to have a test and merge it before the whole patch set?

Thanks.


Best Regards,

Zhiwei

[1]: https://www.mail-archive.com/qemu-devel@nongnu.org/msg782125.html



On 2021/4/15 上午11:57, Alistair Francis wrote:

+ LIU Zhiwei and Kito Cheng

Alistair

On Wed, Apr 14, 2021 at 1:31 AM Tony Cole <1923...@bugs.launchpad.net> wrote:

Public bug reported:

I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
incorrect result of 0x8000 (should saturate to 0x7FFF).

Here is the bit of the code:

 vmv.v.i v16, 0
 …
8f040457vssub.vvv8,v16,v8

I believe the instruction encoding is correct (vssub.vv with vd = v8,
vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.

I’ve just tested with what I think is the latest branch (
https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
exists.

** Affects: qemu
  Importance: Undecided
  Status: New


** Tags: riscv vector

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

Title:
   RISC-V Vector Instruction vssub.vv not saturating

Status in QEMU:
   New

Bug description:
   I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
   incorrect result of 0x8000 (should saturate to 0x7FFF).

   Here is the bit of the code:

 vmv.v.i v16, 0
 …
   8f040457  vssub.vvv8,v16,v8

   I believe the instruction encoding is correct (vssub.vv with vd = v8,
   vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.

   I’ve just tested with what I think is the latest branch (
   https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
   2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
   exists.

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

Re: [PATCH v3 5/8] target/riscv: Implementation of enhanced PMP (ePMP)

[Alistair Francis]

On Wed, Apr 14, 2021 at 5:35 PM Bin Meng  wrote:
>
> Hi Alistair,
>
> On Tue, Apr 13, 2021 at 10:42 AM Alistair Francis
>  wrote:
> >
> > From: Hou Weiying 
> >
> > This commit adds support for ePMP v0.9.1.
> >
> > The ePMP spec can be found in:
> > https://docs.google.com/document/d/1Mh_aiHYxemL0umN3GTTw8vsbmzHZ_nxZXgjgOUzbvc8
> >
> > Signed-off-by: Hongzheng-Li 
> > Signed-off-by: Hou Weiying 
> > Signed-off-by: Myriad-Dreamin 
> > Message-Id: 
> > 
> > [ Changes by AF:
> >  - Rebase on master
> >  - Update to latest spec
> >  - Use a switch case to handle ePMP MML permissions
> >  - Fix a few bugs
> > ]
> > Signed-off-by: Alistair Francis 
> > ---
> >  target/riscv/pmp.c | 164 +
> >  1 file changed, 152 insertions(+), 12 deletions(-)
> >
> > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> > index e35988eec2..00f91d074f 100644
> > --- a/target/riscv/pmp.c
> > +++ b/target/riscv/pmp.c
> > @@ -90,11 +90,42 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, 
> > uint32_t pmp_index)
> >  static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t 
> > val)
> >  {
> >  if (pmp_index < MAX_RISCV_PMPS) {
> > -if (!pmp_is_locked(env, pmp_index)) {
> > -env->pmp_state.pmp[pmp_index].cfg_reg = val;
> > -pmp_update_rule(env, pmp_index);
> > +bool locked = true;
> > +
> > +if (riscv_feature(env, RISCV_FEATURE_EPMP)) {
> > +/* mseccfg.RLB is set */
> > +if (MSECCFG_RLB_ISSET(env)) {
> > +locked = false;
> > +}
> > +
> > +/* mseccfg.MML is not set */
> > +if (!MSECCFG_MML_ISSET(env) && !pmp_is_locked(env, pmp_index)) 
> > {
> > +locked = false;
> > +}
> > +
> > +/* mseccfg.MML is set */
> > +if (MSECCFG_MML_ISSET(env)) {
> > +/* not adding execute bit */
> > +if ((val & PMP_LOCK) != 0 && (val & PMP_EXEC) != PMP_EXEC) 
> > {
> > +locked = false;
> > +}
> > +/* shared region and not adding X bit */
> > +if ((val & PMP_LOCK) != PMP_LOCK &&
> > +(val & 0x7) != (PMP_WRITE | PMP_EXEC)) {
> > +locked = false;
> > +}
> > +}
> >  } else {
> > +if (!pmp_is_locked(env, pmp_index)) {
> > +locked = false;
> > +}
> > +}
> > +
> > +if (locked) {
> >  qemu_log_mask(LOG_GUEST_ERROR, "ignoring pmpcfg write - 
> > locked\n");
> > +} else {
> > +env->pmp_state.pmp[pmp_index].cfg_reg = val;
> > +pmp_update_rule(env, pmp_index);
> >  }
> >  } else {
> >  qemu_log_mask(LOG_GUEST_ERROR,
> > @@ -217,6 +248,32 @@ static bool pmp_hart_has_privs_default(CPURISCVState 
> > *env, target_ulong addr,
> >  {
> >  bool ret;
> >
> > +if (riscv_feature(env, RISCV_FEATURE_EPMP)) {
> > +if (MSECCFG_MMWP_ISSET(env)) {
> > +/*
> > + * The Machine Mode Whitelist Policy (mseccfg.MMWP) is set
> > + * so we default to deny all, even for M-mode.
> > + */
> > +*allowed_privs = 0;
> > +return false;
> > +} else if (MSECCFG_MML_ISSET(env)) {
> > +/*
> > + * The Machine Mode Lockdown (mseccfg.MML) bit is set
> > + * so we can only execute code in M-mode with an applicable
> > + * rule. Other modes are disabled.
> > + */
> > +if (mode == PRV_M && !(privs & PMP_EXEC)) {
> > +ret = true;
> > +*allowed_privs = PMP_READ | PMP_WRITE;
> > +} else {
> > +ret = false;
> > +*allowed_privs = 0;
> > +}
> > +
> > +return ret;
> > +}
> > +}
> > +
> >  if ((!riscv_feature(env, RISCV_FEATURE_PMP)) || (mode == PRV_M)) {
> >  /*
> >   * Privileged spec v1.10 states if HW doesn't implement any PMP 
> > entry
> > @@ -294,13 +351,94 @@ bool pmp_hart_has_privs(CPURISCVState *env, 
> > target_ulong addr,
> >  pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
> >
> >  /*
> > - * If the PMP entry is not off and the address is in range, do the 
> > priv
> > - * check
> > + * Convert the PMP permissions to match the truth table in the
> > + * ePMP spec.
> >   */
> > +const uint8_t epmp_operation =
> > +((env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) >> 4) |
> > +((env->pmp_state.pmp[i].cfg_reg & PMP_READ) << 2) |
> > +(env->pmp_state.pmp[i].cfg_reg & PMP_WRITE) |
> > +((env->pmp_state.pmp[i].cfg_reg & PMP_EXEC) >> 2);
> > +
> >  if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
> > -*allowed_privs = PMP_READ | PMP_WRITE | 

[Bug 1923197] Re: RISC-V priviledged instruction error

[Alistair Francis]

I'm guessing that this is a bug in your guest as it hasn't configured
PMP regions.

>From the RISC-V spec:

"
If no PMP entry matches an M-mode access, the access succeeds. If no PMP entry 
matches an
S-mode or U-mode access, but at least one PMP entry is implemented, the access 
fails.
"

Confusingly implemented here means implemented in hardware, not just
configured.

** Changed in: qemu
   Status: Confirmed => Invalid

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

Title:
  RISC-V priviledged instruction error

Status in QEMU:
  Invalid

Bug description:
  Hello when performing an MRET with MPP set to something else than 0b11
  in MSTATUS, 'Invalid Instruction' exception will be triggered. The
  problem appeared in code after version 5.2.0. Use following code to
  test.

    # setup interrupt handling for monitor mode
    la t0, entry_loop
    la t1, entry_trap
    li t2, 0x888
    li t3, 0x1880
    csrw mepc, t0
    csrw mtvec, t1
    csrs mie, t2
    csrs mstatus, t3

    # if supervisor mode not supported, then loop forever
    csrr t0, misa
    li t1, 0x4
    and t2, t1, t0
    beqz t2, 1f

    # setup interrupt i& exception delegation for supervisor mode
    li t0, 0xc000 # 3 GiB (entry address of supervisor)
    li t1, 0x1000
    li t2, 0x300
    li t3, 0x222
    csrw mepc, t0
    csrc mstatus, t1
    csrs medeleg, t2
    csrs mideleg, t3

    # pass mhartid as first parameter to supervisor
    csrr a0, mhartid

  1:
    mret

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

[Bug 1923197] Re: RISC-V priviledged instruction error

[Alistair Francis]

You can check this by reverting this QEMU commit:

commit d102f19a2085ac931cb998e6153b73248cca49f1
Author: Atish Patra 
Date:   Wed Dec 23 11:25:53 2020 -0800

target/riscv/pmp: Raise exception if no PMP entry is configured

As per the privilege specification, any access from S/U mode should fail
if no pmp region is configured.

Signed-off-by: Atish Patra 
Reviewed-by: Alistair Francis 
Message-id: 20201223192553.332508-1-atish.pa...@wdc.com
Signed-off-by: Alistair Francis 

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

Title:
  RISC-V priviledged instruction error

Status in QEMU:
  Invalid

Bug description:
  Hello when performing an MRET with MPP set to something else than 0b11
  in MSTATUS, 'Invalid Instruction' exception will be triggered. The
  problem appeared in code after version 5.2.0. Use following code to
  test.

    # setup interrupt handling for monitor mode
    la t0, entry_loop
    la t1, entry_trap
    li t2, 0x888
    li t3, 0x1880
    csrw mepc, t0
    csrw mtvec, t1
    csrs mie, t2
    csrs mstatus, t3

    # if supervisor mode not supported, then loop forever
    csrr t0, misa
    li t1, 0x4
    and t2, t1, t0
    beqz t2, 1f

    # setup interrupt i& exception delegation for supervisor mode
    li t0, 0xc000 # 3 GiB (entry address of supervisor)
    li t1, 0x1000
    li t2, 0x300
    li t3, 0x222
    csrw mepc, t0
    csrc mstatus, t1
    csrs medeleg, t2
    csrs mideleg, t3

    # pass mhartid as first parameter to supervisor
    csrr a0, mhartid

  1:
    mret

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

Issue Report: When VM memory is extremely large, downtime for RDMA migration is high. (64G mem --> extra 400ms)

[LIZHAOXIN1 [李照鑫]]

Hi:
When I tested RDMA live migration, I found that the downtime increased as the 
VM's memory increased.

My Mellanox network card is [ConnectX-4 LX] and the driver is MLNX-5.2, My VM 
memory size is 64GB, downtime is 430ms when I migrate using the following 
parameters:
virsh migrate --live --p2p --persistent --copy-storage-inc --auto-converge 
--verbose --listen-address 0.0.0.0 --rdma-pin-all --migrateuri 
rdma://192.168.0.2 [VM] qemu+tcp://192.168.0.2/system

The extra time, about 400ms, which is how long it takes RDMA to deregister 
memory (the function: ibv_dereg_mr) after memory migration is complete, is 
before qmp_cont and therefore part of downtime.

How do we reduce this downtime? Like deregister memory somewhere else?

If anything wrong, Please point out.
Thanks!

Re: [Bug 1923629] [NEW] RISC-V Vector Instruction vssub.vv not saturating

[Kito Cheng]

Add Frank, he is the SiFive's qemu maintainer.

On Thu, Apr 15, 2021 at 11:57 AM Alistair Francis  wrote:
>
> + LIU Zhiwei and Kito Cheng
>
> Alistair
>
> On Wed, Apr 14, 2021 at 1:31 AM Tony Cole <1923...@bugs.launchpad.net> wrote:
> >
> > Public bug reported:
> >
> > I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
> > incorrect result of 0x8000 (should saturate to 0x7FFF).
> >
> > Here is the bit of the code:
> >
> > vmv.v.i v16, 0
> > …
> > 8f040457vssub.vvv8,v16,v8
> >
> > I believe the instruction encoding is correct (vssub.vv with vd = v8,
> > vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.
> >
> > I’ve just tested with what I think is the latest branch (
> > https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
> > 2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
> > exists.
> >
> > ** Affects: qemu
> >  Importance: Undecided
> >  Status: New
> >
> >
> > ** Tags: riscv vector
> >
> > --
> > You received this bug notification because you are a member of qemu-
> > devel-ml, which is subscribed to QEMU.
> > https://bugs.launchpad.net/bugs/1923629
> >
> > Title:
> >   RISC-V Vector Instruction vssub.vv not saturating
> >
> > Status in QEMU:
> >   New
> >
> > Bug description:
> >   I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
> >   incorrect result of 0x8000 (should saturate to 0x7FFF).
> >
> >   Here is the bit of the code:
> >
> > vmv.v.i v16, 0
> > …
> >   8f040457  vssub.vvv8,v16,v8
> >
> >   I believe the instruction encoding is correct (vssub.vv with vd = v8,
> >   vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.
> >
> >   I’ve just tested with what I think is the latest branch (
> >   https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
> >   2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
> >   exists.
> >
> > To manage notifications about this bug go to:
> > https://bugs.launchpad.net/qemu/+bug/1923629/+subscriptions
> >

Re: [Bug 1923629] [NEW] RISC-V Vector Instruction vssub.vv not saturating

[Alistair Francis]

+ LIU Zhiwei and Kito Cheng

Alistair

On Wed, Apr 14, 2021 at 1:31 AM Tony Cole <1923...@bugs.launchpad.net> wrote:
>
> Public bug reported:
>
> I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
> incorrect result of 0x8000 (should saturate to 0x7FFF).
>
> Here is the bit of the code:
>
> vmv.v.i v16, 0
> …
> 8f040457vssub.vvv8,v16,v8
>
> I believe the instruction encoding is correct (vssub.vv with vd = v8,
> vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.
>
> I’ve just tested with what I think is the latest branch (
> https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
> 2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
> exists.
>
> ** Affects: qemu
>  Importance: Undecided
>  Status: New
>
>
> ** Tags: riscv vector
>
> --
> You received this bug notification because you are a member of qemu-
> devel-ml, which is subscribed to QEMU.
> https://bugs.launchpad.net/bugs/1923629
>
> Title:
>   RISC-V Vector Instruction vssub.vv not saturating
>
> Status in QEMU:
>   New
>
> Bug description:
>   I noticed doing a negate ( 0 – 0x8000 ) using vssub.vv produces an
>   incorrect result of 0x8000 (should saturate to 0x7FFF).
>
>   Here is the bit of the code:
>
> vmv.v.i v16, 0
> …
>   8f040457  vssub.vvv8,v16,v8
>
>   I believe the instruction encoding is correct (vssub.vv with vd = v8,
>   vs2 = v16, rs1 = v8), but the result does not saturate in QEMU.
>
>   I’ve just tested with what I think is the latest branch (
>   https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v7 commit 26 Feb
>   2021: 1151361fa7d45cc90d69086ccf1a4d8397931811 ) and the problem still
>   exists.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1923629/+subscriptions
>

[PATCH v6 4/4] net: Extend host forwarding to support IPv6

[Doug Evans]

Net option "-hostfwd" now supports IPv6 addresses.
Commands hostfwd_add, hostfwd_remove now support IPv6 addresses.

Tested:
avocado run tests/acceptance/hostfwd.py

Signed-off-by: Doug Evans 
---

Changes from v5:

Recognize ipv4=,ipv6= options.

 hmp-commands.hx | 18 ++-
 net/slirp.c | 54 +
 tests/acceptance/hostfwd.py | 94 +
 3 files changed, 155 insertions(+), 11 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 435c591a1c..05f88e893b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1322,7 +1322,8 @@ ERST
 {
 .name   = "hostfwd_add",
 .args_type  = "arg1:s,arg2:s?",
-.params = "[netdev_id] 
[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport",
+.params = "[netdev_id] [tcp|udp]:[hostaddr]:hostport\n"
+  "[,ipv4=on|off][,ipv6=on|off]-[guestaddr]:guestport",
 .help   = "redirect TCP or UDP connections from host to guest 
(requires -net user)",
 .cmd= hmp_hostfwd_add,
 },
@@ -1330,13 +1331,20 @@ ERST
 SRST
 ``hostfwd_add``
   Redirect TCP or UDP connections from host to guest (requires -net user).
+  IPV6 addresses are wrapped in square brackets, IPV4 addresses are not.
+
+  Examples:
+  hostfwd_add net0 tcp:127.0.0.1:10022-:22
+  hostfwd_add net0 tcp:[::1]:10022-[fe80::1:2:3:4]:22
+  hostfwd_add net0 ::10022,ipv6-:22
 ERST
 
 #ifdef CONFIG_SLIRP
 {
 .name   = "hostfwd_remove",
 .args_type  = "arg1:s,arg2:s?",
-.params = "[netdev_id] [tcp|udp]:[hostaddr]:hostport",
+.params = "[netdev_id] [tcp|udp]:[hostaddr]:hostport\n"
+  "[,ipv4=on|off][,ipv6=on|off]",
 .help   = "remove host-to-guest TCP or UDP redirection",
 .cmd= hmp_hostfwd_remove,
 },
@@ -1345,6 +1353,12 @@ ERST
 SRST
 ``hostfwd_remove``
   Remove host-to-guest TCP or UDP redirection.
+  IPV6 addresses are wrapped in square brackets, IPV4 addresses are not.
+
+  Examples:
+  hostfwd_remove net0 tcp:127.0.0.1:10022
+  hostfwd_remove net0 tcp:[::1]:10022
+  hostfwd_remove net0 ::10022,ipv6
 ERST
 
 {
diff --git a/net/slirp.c b/net/slirp.c
index 4be065c30b..82d4b71bef 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -664,25 +664,55 @@ static const char *parse_protocol(const char *str, bool 
*is_udp,
 return p;
 }
 
-static int parse_hostfwd_sockaddr(const char *str, int socktype,
+static int parse_hostfwd_sockaddr(const char *str, int family, int socktype,
   struct sockaddr_storage *saddr,
-  Error **errp)
+  bool *v6_only, Error **errp)
 {
 struct addrinfo hints, *res = NULL, *e;
 InetSocketAddress *addr = g_new(InetSocketAddress, 1);
 int gai_rc;
 int rc = -1;
+Error *err = NULL;
 
 const char *optstr = inet_parse_host_port(addr, str, errp);
 if (optstr == NULL) {
 goto fail_return;
 }
 
+if (inet_parse_ipv46(addr, optstr, errp) < 0) {
+goto fail_return;
+}
+
+if (v6_only) {
+bool v4 = addr->has_ipv4 && addr->ipv4;
+bool v6 = addr->has_ipv6 && addr->ipv6;
+*v6_only = v6 && !v4;
+}
+
 memset(, 0, sizeof(hints));
 hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */
 hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
 hints.ai_socktype = socktype;
-hints.ai_family = PF_INET;
+hints.ai_family = inet_ai_family_from_address(addr, );
+if (err) {
+error_propagate(errp, err);
+goto fail_return;
+}
+if (family != PF_UNSPEC) {
+/* Guest must use same family as host (for now). */
+if (hints.ai_family != PF_UNSPEC && hints.ai_family != family) {
+error_setg(errp,
+   "unexpected address family for %s: expecting %s",
+   str, family == PF_INET ? "ipv4" : "ipv6");
+goto fail_return;
+}
+hints.ai_family = family;
+}
+
+/* For backward compatibility, treat an empty host spec as IPv4. */
+if (*addr->host == '\0' && hints.ai_family == PF_UNSPEC) {
+hints.ai_family = PF_INET;
+}
 
 /*
  * Calling getaddrinfo for guest addresses is dubious, but addresses are
@@ -768,8 +798,8 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 flags |= SLIRP_HOSTFWD_UDP;
 }
 
-if (parse_hostfwd_sockaddr(p, is_udp ? SOCK_DGRAM : SOCK_STREAM,
-   _addr, ) < 0) {
+if (parse_hostfwd_sockaddr(p, PF_UNSPEC, is_udp ? SOCK_DGRAM : SOCK_STREAM,
+   _addr, /*v6_only=*/NULL, ) < 0) {
 goto fail_syntax;
 }
 
@@ -794,6 +824,7 @@ static int slirp_hostfwd(SlirpState *s, const char 
*redir_str, Error **errp)
 Error *error = NULL;
 int flags = 0;
 int port;
+bool v6_only;
 
 g_assert(redir_str != NULL);

[PATCH v6 3/4] net/slirp.c: Refactor address parsing

[Doug Evans]

... in preparation for adding ipv6 host forwarding support.

Tested:
avocado run tests/acceptance/hostfwd.py

Signed-off-by: Doug Evans 
---

Changes from v5:

Use InetSocketAddress and getaddrinfo().
Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.

 include/qemu/sockets.h  |   2 +
 net/slirp.c | 200 
 tests/acceptance/hostfwd.py |  91 
 util/qemu-sockets.c |  17 +--
 4 files changed, 241 insertions(+), 69 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 94f4e8de83..6fd71775ce 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -29,6 +29,8 @@ int socket_set_fast_reuse(int fd);
 #define SHUT_RDWR 2
 #endif
 
+int sockaddr_getport(const struct sockaddr *addr);
+
 int inet_ai_family_from_address(InetSocketAddress *addr,
 Error **errp);
 const char *inet_parse_host_port(InetSocketAddress *addr,
diff --git a/net/slirp.c b/net/slirp.c
index a01a0fccd3..4be065c30b 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -641,15 +641,108 @@ static SlirpState *slirp_lookup(Monitor *mon, const char 
*id)
 }
 }
 
+static const char *parse_protocol(const char *str, bool *is_udp,
+  Error **errp)
+{
+char buf[10];
+const char *p = str;
+
+if (get_str_sep(buf, sizeof(buf), , ':') < 0) {
+error_setg(errp, "missing protocol name separator");
+return NULL;
+}
+
+if (!strcmp(buf, "tcp") || buf[0] == '\0') {
+*is_udp = false;
+} else if (!strcmp(buf, "udp")) {
+*is_udp = true;
+} else {
+error_setg(errp, "bad protocol name '%s'", buf);
+return NULL;
+}
+
+return p;
+}
+
+static int parse_hostfwd_sockaddr(const char *str, int socktype,
+  struct sockaddr_storage *saddr,
+  Error **errp)
+{
+struct addrinfo hints, *res = NULL, *e;
+InetSocketAddress *addr = g_new(InetSocketAddress, 1);
+int gai_rc;
+int rc = -1;
+
+const char *optstr = inet_parse_host_port(addr, str, errp);
+if (optstr == NULL) {
+goto fail_return;
+}
+
+memset(, 0, sizeof(hints));
+hints.ai_flags = AI_PASSIVE; /* ignored if host is not ""(->NULL) */
+hints.ai_flags |= AI_NUMERICHOST | AI_NUMERICSERV;
+hints.ai_socktype = socktype;
+hints.ai_family = PF_INET;
+
+/*
+ * Calling getaddrinfo for guest addresses is dubious, but addresses are
+ * restricted to numeric only. Convert "" to NULL for getaddrinfo's
+ * benefit.
+ */
+gai_rc = getaddrinfo(*addr->host ? addr->host : NULL,
+ *addr->port ? addr->port : NULL, , );
+if (gai_rc != 0) {
+error_setg(errp, "address resolution failed for '%s': %s",
+   str, gai_strerror(gai_rc));
+goto fail_return;
+}
+if (res->ai_next != NULL) {
+/*
+ * The caller only wants one address, and except for "any" for both
+ * ipv4 and ipv6 (which we've already precluded above), we shouldn't
+ * get more than one. To assist debugging print all we find.
+ */
+GString *s = g_string_new(NULL);
+for (e = res; e != NULL; e = e->ai_next) {
+char host[NI_MAXHOST];
+char serv[NI_MAXSERV];
+int ret = getnameinfo((struct sockaddr *)e->ai_addr, e->ai_addrlen,
+  host, sizeof(host),
+  serv, sizeof(serv),
+  NI_NUMERICHOST | NI_NUMERICSERV);
+if (ret == 0) {
+g_string_append_printf(s, "\n  %s:%s", host, serv);
+} else {
+g_string_append_printf(s, "\n  unknown, got: %s",
+   gai_strerror(ret));
+}
+}
+error_setg(errp, "multiple addresses resolved for '%s':%s",
+   str, s->str);
+g_string_free(s, TRUE);
+goto fail_return;
+}
+
+memcpy(saddr, res->ai_addr, res->ai_addrlen);
+rc = 0;
+
+ fail_return:
+qapi_free_InetSocketAddress(addr);
+if (res) {
+freeaddrinfo(res);
+}
+return rc;
+}
+
 void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 {
-struct in_addr host_addr = { .s_addr = INADDR_ANY };
-int host_port;
-char buf[256];
+struct sockaddr_storage host_addr;
 const char *src_str, *p;
 SlirpState *s;
-int is_udp = 0;
+bool is_udp;
+Error *error = NULL;
 int err;
+int flags = 0;
 const char *arg1 = qdict_get_str(qdict, "arg1");
 const char *arg2 = qdict_get_try_str(qdict, "arg2");
 
@@ -664,110 +757,91 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict)
 return;
 }
 
+g_assert(src_str != NULL);
 p = src_str;
-if (!p || get_str_sep(buf, sizeof(buf), , 

[PATCH v6 2/4] util/qemu-sockets.c: Split host:port parsing out of inet_parse

[Doug Evans]

The parsing is moved into new function inet_parse_host_port.
Also split out is ipv4=flag, ipv6=flag processing into inet_parse_ipv46.
This is done in preparation for using these functions in net/slirp.c.

Signed-off-by: Doug Evans 
---

Changes from v5:

Also split out parsing of ipv4=on|off, ipv6=on|off

 include/qemu/sockets.h |  3 ++
 util/qemu-sockets.c| 65 +-
 2 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 7d1f813576..94f4e8de83 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -31,6 +31,9 @@ int socket_set_fast_reuse(int fd);
 
 int inet_ai_family_from_address(InetSocketAddress *addr,
 Error **errp);
+const char *inet_parse_host_port(InetSocketAddress *addr,
+ const char *str, Error **errp);
+int inet_parse_ipv46(InetSocketAddress *addr, const char *optstr, Error 
**errp);
 int inet_parse(InetSocketAddress *addr, const char *str, Error **errp);
 int inet_connect(const char *str, Error **errp);
 int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 8af0278f15..c0069f2565 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -615,14 +615,12 @@ static int inet_parse_flag(const char *flagname, const 
char *optstr, bool *val,
 return 0;
 }
 
-int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
+const char *inet_parse_host_port(InetSocketAddress *addr, const char *str,
+ Error **errp)
 {
-const char *optstr, *h;
 char host[65];
 char port[33];
-int to;
 int pos;
-char *begin;
 
 memset(addr, 0, sizeof(*addr));
 
@@ -632,38 +630,32 @@ int inet_parse(InetSocketAddress *addr, const char *str, 
Error **errp)
 host[0] = '\0';
 if (sscanf(str, ":%32[^,]%n", port, ) != 1) {
 error_setg(errp, "error parsing port in address '%s'", str);
-return -1;
+return NULL;
 }
 } else if (str[0] == '[') {
 /* IPv6 addr */
 if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, ) != 2) {
 error_setg(errp, "error parsing IPv6 address '%s'", str);
-return -1;
+return NULL;
 }
 } else {
 /* hostname or IPv4 addr */
 if (sscanf(str, "%64[^:]:%32[^,]%n", host, port, ) != 2) {
 error_setg(errp, "error parsing address '%s'", str);
-return -1;
+return NULL;
 }
 }
 
 addr->host = g_strdup(host);
 addr->port = g_strdup(port);
 
-/* parse options */
-optstr = str + pos;
-h = strstr(optstr, ",to=");
-if (h) {
-h += 4;
-if (sscanf(h, "%d%n", , ) != 1 ||
-(h[pos] != '\0' && h[pos] != ',')) {
-error_setg(errp, "error parsing to= argument");
-return -1;
-}
-addr->has_to = true;
-addr->to = to;
-}
+return str + pos;
+}
+
+int inet_parse_ipv46(InetSocketAddress *addr, const char *optstr, Error **errp)
+{
+char *begin;
+
 begin = strstr(optstr, ",ipv4");
 if (begin) {
 if (inet_parse_flag("ipv4", begin + 5, >ipv4, errp) < 0) {
@@ -678,6 +670,39 @@ int inet_parse(InetSocketAddress *addr, const char *str, 
Error **errp)
 }
 addr->has_ipv6 = true;
 }
+
+return 0;
+}
+
+int inet_parse(InetSocketAddress *addr, const char *str, Error **errp)
+{
+const char *optstr, *h;
+int to;
+int pos;
+char *begin;
+
+optstr = inet_parse_host_port(addr, str, errp);
+if (optstr == NULL) {
+return -1;
+}
+
+/* parse options */
+
+if (inet_parse_ipv46(addr, optstr, errp) < 0) {
+return -1;
+}
+
+h = strstr(optstr, ",to=");
+if (h) {
+h += 4;
+if (sscanf(h, "%d%n", , ) != 1 ||
+(h[pos] != '\0' && h[pos] != ',')) {
+error_setg(errp, "error parsing to= argument");
+return -1;
+}
+addr->has_to = true;
+addr->to = to;
+}
 begin = strstr(optstr, ",keep-alive");
 if (begin) {
 if (inet_parse_flag("keep-alive", begin + strlen(",keep-alive"),
-- 
2.31.1.295.g9ea45b61b8-goog

[PATCH v6 0/4] Add support for ipv6 host forwarding

[Doug Evans]

This patchset takes the original patch from Maxim,
https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html
and updates it.

Option hostfwd is extended to support ipv6 addresses.
Commands hostfwd_add, hostfwd_remove are extended as well.

The libslirp part of the patch has been committed upstream,
and is now in qemu. See patch 1/4.

Changes from v5:

1/4 slirp: Advance libslirp submodule to current master
NOTE TO REVIEWERS: It may be a better use of everyone's time if a
maintainer takes on advancing QEMU's libslirp to libslirp's master.
Beyond that, I really don't know what to do except submit this patch as
is currently provided.

2/4: util/qemu-sockets.c: Split host:port parsing out of inet_parse

Also split out parsing of ipv4=on|off, ipv6=on|off

3/4: net/slirp.c: Refactor address parsing

Use InetSocketAddress and getaddrinfo().
Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd.

4/4: net: Extend host forwarding to support IPv6

Recognize ipv4=,ipv6= options.

Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been deleted:
the churn on this patch series needs to be reduced.
This change is not required, and can easily be done in a later patch.

Changes from v4:

1/5 slirp: Advance libslirp submodule to add ipv6 host-forward support
NOTE TO REVIEWERS: I need some hand-holding to know what The Right
way to submit this particular patch is.

- no change

2/5 util/qemu-sockets.c: Split host:port parsing out of inet_parse

- move recognition of "[]:port" to separate patch
- allow passing NULL for ip_v6
- fix some formatting issues

3/5 inet_parse_host_and_addr: Recognize []:port (empty ipv6 address)

- new in this patchset revision

4/5 net/slirp.c: Refactor address parsing

- was 3/4 in v4
- fix some formatting issues

5/5 net: Extend host forwarding to support IPv6

- was 4/4 in v4
- fix some formatting issues

Changes from v3:

1/4 slirp: Advance libslirp submodule to add ipv6 host-forward support

- pick up latest libslirp patch to reject ipv6 addr-any for guest address
  - libslirp currently only provides a stateless DHCPv6 server, which means
it can't know in advance what the guest's IP address is, and thus
cannot do the "addr-any -> guest ip address" translation that is done
for ipv4

2/4 util/qemu-sockets.c: Split host:port parsing out of inet_parse

- this patch is new in v4
  - provides new utility: inet_parse_host_and_port, updates inet_parse
to use it

3/4 net/slirp.c: Refactor address parsing

- this patch renamed from 2/3 to 3/4
- call inet_parse_host_and_port from util/qemu-sockets.c
- added tests/acceptance/hostfwd.py

4/4 net: Extend host forwarding to support IPv6

- this patch renamed from 3/3 to 4/4
- ipv6 support added to existing hostfwd option, commands
  - instead of creating new ipv6 option, commands
- added tests to tests/acceptance/hostfwd.py

Changes from v2:
- split out libslirp commit
- clarify spelling of ipv6 addresses in docs
- tighten parsing of ipv6 addresses

Change from v1:
- libslirp part is now upstream
- net/slirp.c changes split into two pieces (refactor, add ipv6)
- added docs

Doug Evans (4):
  slirp: Advance libslirp submodule to add ipv6 host-forward support
  util/qemu-sockets.c: Split host:port parsing out of inet_parse
  net/slirp.c: Refactor address parsing
  net: Extend host forwarding to support IPv6

 hmp-commands.hx |  18 ++-
 include/qemu/sockets.h  |   5 +
 net/slirp.c | 236 ++--
 slirp   |   2 +-
 tests/acceptance/hostfwd.py | 185 
 util/qemu-sockets.c |  82 +
 6 files changed, 436 insertions(+), 92 deletions(-)
 create mode 100644 tests/acceptance/hostfwd.py

-- 
2.31.1.295.g9ea45b61b8-goog

[PATCH v6 1/4] slirp: Advance libslirp submodule to add ipv6 host-forward support

[Doug Evans]

5eraph (2):
  disable_dns option
  limit vnameserver_addr to port 53

Akihiro Suda (1):
  libslirp.h: fix SlirpConfig v3 documentation

Doug Evans (11):
  Add ipv6 host forward support
  tcpx_listen: Pass sizeof(addr) to memset
  Reject host forwarding to ipv6 "addr-any"
  Add /build/ to .gitignore
  New utility slirp_ether_ntoa
  m_cleanup_list: make static
  New API routine slirp_neighbor_info
  Move DEBUG_CALL("if_start") to DEBUG_VERBOSE_CALL
  tcpx_listen: tcp_newtcpcb doesn't fail
  slirp_add_host*fwd: Ensure all error paths set errno
  Perform lazy guest address resolution for IPv6

Dr. David Alan Gilbert (1):
  ip_stripoptions use memmove

Giuseppe Scrivano (1):
  socket: consume empty packets

Hafiz Abid Qadeer (1):
  Fix a typo that can cause slow socket response on Windows.

Jindrich Novy (4):
  Fix possible infinite loops and use-after-free
  Use secure string copy to avoid overflow
  Be sure to initialize sockaddr structure
  Check lseek() for failure

Marc-André Lureau (26):
  Merge branch 'master' into 'master'
  Merge branch 'fix-slirpconfig-3-doc' into 'master'
  Fix use-afte-free in ip_reass() (CVE-2020-1983)
  Update CHANGELOG
  Merge branch 'cve-2020-1983' into 'master'
  Release v4.3.0
  Merge branch 'release-v4.3.0' into 'master'
  changelog: post-release
  util: do not silently truncate
  Merge branch 'slirp-fmt-truncate' into 'master'
  Release v4.3.1
  Merge branch 'release-v4.3.1' into 'master'
  changelog: post-release
  .gitlab-ci: add a Coverity stage
  Merge branch 'coverity' into 'master'
  Merge branch 'ios-support' into 'master'
  Merge branch 'master' into 'master'
  Remove the QEMU-special make build-system
  Merge branch 'qemu' into 'master'
  Release v4.4.0
  Merge branch '4.4.0-release' into 'master'
  changelog: post-release
  Remove some needless (void)casts
  Fix unused variables
  Merge branch 'gitignore-build' into 'master'
  Merge branch 'macos-deployment-target' into 'master'

Nathaniel Wesley Filardo (1):
  fork_exec_child_setup: improve signal handling

Paolo Bonzini (2):
  meson: remove meson-dist script
  meson: support compiling as subproject

Philippe Mathieu-Daudé (3):
  Fix win32 builds by using the SLIRP_PACKED definition
  Fix constness warnings
  Remove unnecessary break

Prasad J Pandit (1):
  slirp: check pkt_len before reading protocol header

Ralf Haferkamp (2):
  Drop bogus IPv6 messages
  Fix MTU check

Samuel Thibault (45):
  Merge branch 'ip6_payload_len' into 'master'
  Merge branch 'lp1878043' into 'master'
  udp, udp6, icmp: handle TTL value
  icmp, icmp6: Add icmp_forward_error and icmp6_forward_error
  udp, udp6, icmp, icmp6: Enable forwarding errors on Linux
  TCPIPHDR_DELTA: Fix potential negative value
  sosendoob: better document what urgc is used for
  Merge branch 'G_GNUC_PRINTF' into 'master'
  Merge branch 'CVE-2020-29129' into 'master'
  Merge branch 'ttl' into 'master'
  Merge branch 'errors' into 'master'
  Merge branch 'consume-empty-packet' into 'master'
  Merge branch 'void' into 'master'
  Merge branch 'master' into 'master'
  Merge branch 'unused' into 'master'
  Merge branch 'socket_delay' into 'master'
  tcp_subr: simplify code
  Merge branch 'ipv6-host-fwd-9-patch' into 'master'
  Document the slirp API
  Complete timeout documentation
  Merge branch 'memset-sizeof' into 'master'
  Merge branch 'reject-ipv6-addr-any' into 'master'
  ip6_output: fix memory leak on fast-send
  Merge branch 'ndp-leak' into 'master'
  Merge branch 'memory_leaks' into 'master'
  TODO for generalizing the hostfwd calls
  socket.h: add missing sbuf.h inclusion
  Expose udpx_listen and tcpx_listen as taking sockaddr
  Disable polling for PRI on MacOS
  Merge branch 'macos-pri' into 'master'
  Merge branch 'x_listen' into 'master'
  udpx/tcpx_listen: Add missing const qualifier
  sockaddr_*: add missing const qualifiers
  Merge branch 'm-cleanup-list-prototype' into 'master'
  Merge branch 'neighbor-info' into 'master'
  udpx/tcpx_listen: Use struct sockaddr * types
  Add ipv4/ipv6-agnostic host forwarding functions
  hostfwd: Add SLIRP_HOSTFWD_V6ONLY flag
  Merge branch 'hostxfwd' into 'master'
  Merge branch 'verbose-if-start' into 'master'
  Remove slirp_add/remove_ipv6_hostfwd
  Merge branch 'listen-errno' into 'master'
  Merge branch 'newtcpcb-no-fail' into 'master'
  Merge branch 'listen_v6only' into 'master'
  Merge branch 'lazy-ipv6-resolution' into 'master'

Stefan Weil (1):
  Add G_GNUC_PRINTF to local function slirp_vsnprintf

WaluigiWare64 (1):
  Set macOS deployment target to macOS 10.4 Without a macOS deployment 
target, the 

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

[Kunkun Jiang]

Hi Eric,

On 2021/4/14 16:05, Auger Eric wrote:

Hi Kunkun,

On 4/14/21 3:45 AM, Kunkun Jiang wrote:

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
         

[ANNOUNCE] QEMU 6.0.0-rc3 is now available

[Michael Roth]

Hello,

On behalf of the QEMU Team, I'd like to announce the availability of the
fourth release candidate for the QEMU 6.0 release.  This release is meant
for testing purposes and should not be used in a production environment.

  http://download.qemu-project.org/qemu-6.0.0-rc3.tar.xz
  http://download.qemu-project.org/qemu-6.0.0-rc3.tar.xz.sig

A note from the maintainer:

  Hopefully rc3 will be the last rc for the 6.0 release, and we will
  make the release on Tuesday 20th. If any last-minute critical issues
  appear, we'll do an rc4 on the 20th and the release the week after.

You can help improve the quality of the QEMU 6.0 release by testing this
release and reporting bugs on Launchpad:

  https://bugs.launchpad.net/qemu/

The release plan, as well a documented known issues for release
candidates, are available at:

  http://wiki.qemu.org/Planning/6.0

Please add entries to the ChangeLog for the 6.0 release below:

  http://wiki.qemu.org/ChangeLog/6.0

Thank you to everyone involved!

Changes since rc2:

8fe9f1f891: Update version for v6.0.0-rc3 release (Peter Maydell)
438c61e086: qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code 
(Thomas Huth)
ace66791cd: vhost-user-fs: fix features handling (Anton Kuchin)
0267101af6: block/nbd: fix possible use after free of s->connect_thread 
(Vladimir Sementsov-Ogievskiy)
f4349ba966: target/mips: Fix TCG temporary leak in gen_cache_operation() 
(Philippe Mathieu-Daudé)
62271205bc: hw/isa/piix4: Migrate Reset Control Register (Philippe 
Mathieu-Daudé)
50fab4cc67: hw/isa/Kconfig: Add missing dependency VIA VT82C686 -> APM 
(Philippe Mathieu-Daudé)
2d18b4ca02: sphinx: qapidoc: Wrap "If" section body in a paragraph node (John 
Snow)
ce94fa7aa6: tests/qtest: add tests for am53c974 device (Mark Cave-Ayland)
607206948c: esp: ensure that do_cmd is set to zero before submitting an ESP 
select command (Mark Cave-Ayland)
324c880989: esp: don't reset async_len directly in esp_select() if cancelling 
request (Mark Cave-Ayland)
0ebb5fd805: esp: don't overflow cmdfifo if TC is larger than the cmdfifo size 
(Mark Cave-Ayland)
fbc6510e33: esp: don't overflow cmdfifo in get_cmd() (Mark Cave-Ayland)
fa7505c154: esp: don't underflow cmdfifo in do_cmd() (Mark Cave-Ayland)
9954575173: esp: ensure cmdfifo is not empty and current_dev is non-NULL (Mark 
Cave-Ayland)
7b320a8e67: esp: introduce esp_fifo_pop_buf() and use it instead of 
fifo8_pop_buf() (Mark Cave-Ayland)
c5fef9112b: esp: consolidate esp_cmdfifo_pop() into esp_fifo_pop() (Mark 
Cave-Ayland)
e5455b8c1c: esp: consolidate esp_cmdfifo_push() into esp_fifo_push() (Mark 
Cave-Ayland)
e392255766: esp: rework write_response() to avoid using the FIFO for DMA 
transactions (Mark Cave-Ayland)
0db895361b: esp: always check current_req is not NULL before use in DMA 
callbacks (Mark Cave-Ayland)
ff4a1daba6: esp: fix setting of ESPState mig_version_id when launching QEMU 
with -S option (Mark Cave-Ayland)
91c0a79891: hw/arm/mps2-tz: Assert if more than one RAM is attached to an MPC 
(Peter Maydell)
db2fc83aa4: hw/arm/mps2-tz: Fix MPC setting for AN524 SRAM block (Peter Maydell)
52c01ada86: exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1 (Richard Henderson)
ff38bca7d6: target/arm: Check PAGE_WRITE_ORG for MTE writeability (Richard 
Henderson)
eb42297a59: accel/tcg: Preserve PAGE_ANON when changing page permissions 
(Richard Henderson)
017a913af4: hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of 
StreamIDs (Zenghui Yu)
0c38f60783: hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} 
interrupts (Zenghui Yu)
98f84f5a4e: hw/block/nvme: drain namespaces on sq deletion (Klaus Jensen)
5cefe28708: hw/block/nvme: store aiocb in compare (Klaus Jensen)
d357230b20: hw/block/nvme: map prp fix if prp2 contains non-zero offset 
(Padmakar Kalghatgi)
a3d9f3a962: docs: add nvme emulation documentation (Klaus Jensen)
2b18fc794f: spapr.c: always pulse guest IRQ in spapr_core_unplug_request() 
(Daniel Henrique Barboza)
d522cb52e6: spapr: rollback 'unplug timeout' for CPU hotunplugs (Daniel 
Henrique Barboza)
0b47ec4b95: cpu/core: Fix "help" of CPU core device types (Greg Kurz)
cdeaed2778: i386: Add missing cpu feature bits in EPYC-Rome model (Babu Moger)
c2c731a4d3: test-blockjob: Test job_wait_unpaused() (Max Reitz)
53ddb9c892: job: Allow complete for jobs on standby (Max Reitz)
00769414cd: mirror: Do not enter a paused job on completion (Max Reitz)
c41f5b96ee: mirror: Move open_backing_file to exit_common (Max Reitz)
da64789d3a: hw/block/fdc: Fix 'fallback' property on sysbus floppy disk 
controllers (Philippe Mathieu-Daudé)
f940b0ac6f: iotests: Test mirror-top filter permissions (Max Reitz)
030262a6e4: iotests: add test for removing persistent bitmap from backing file 
(Vladimir Sementsov-Ogievskiy)
66f18320f7: iotests/qsd-jobs: Filter events in the first test (Max Reitz)
b084b420d9: block/rbd: fix memory leak in qemu_rbd_co_create_opts() (Stefano 
Garzarella)
c1c1f6cf51: block/rbd: fix memory leak in qemu_rbd_connect() 

Re: [PATCH v3 3/3] accel/tcg: Assert that tb->size != 0 after translation

[Max Filippov]

On Wed, Apr 14, 2021 at 12:43 PM Richard Henderson
 wrote:
>
> On 4/14/21 11:03 AM, Max Filippov wrote:
> > On Wed, Apr 14, 2021 at 9:51 AM Ilya Leoshkevich  wrote:
> >> On Wed, 2021-04-14 at 16:48 +0200, David Hildenbrand wrote:
> >>> Did you double-check the xtensa issue?
> >>
> >> Oh, I'm sorry, I completely forgot about that one. I just ran the
> >> test locally, and apparently it fails because of this new assert, so
> >> I'll have to write the 4th patch now. Thanks!
> >
> > Just curious, what xtensa issue?
>
> Returning from xtensa_tr_translate_insn with tb->size == 0.
>
> Basically, dc->base.pc_next needs to be incremented even for illegal
> instructions, preferably by the number of bytes consumed while determining 
> that
> the insn is illegal.

I see a few places where target/xtensa may do that. E.g. it does that on entry
to an exception handler to allow for debugging its first instruction.
No guest code
is consumed to make this decision, would size 1 work in that case?
I'll take a look.

-- 
Thanks.
-- Max

[Bug 1922611] Re: Acceptance Tests: migration fails on sparc target

[Cleber Rosa]

I can confirm this bug has been fixed.  Relevant test output:

VM launch command: './qemu-system-sparc -display none -vga none -chardev 
socket,id=mon,path=/tmp/avo_qemu_sock_g0w15g26/qemu-1672256-monitor.sock -mon 
chardev=mon,mode=control -incoming tcp:localhost:53800 -nodefaults'
>>> {'execute': 'qmp_capabilities'}
<<< {'return': {}}
VM launch command: './qemu-system-sparc -display none -vga none -chardev 
socket,id=mon,path=/tmp/avo_qemu_sock_ajodgya5/qemu-1672256-monitor.sock -mon 
chardev=mon,mode=control -nodefaults'
>>> {'execute': 'qmp_capabilities'}
<<< {'return': {}}
>>> {'execute': 'migrate', 'arguments': {'uri': 'tcp:localhost:53800'}}
<<< {'return': {}}
>>> {'execute': 'query-migrate'}
<<< {'return': {'blocked': False, 'status': 'setup'}}
>>> {'execute': 'query-migrate'}
<<< {'timestamp': {'seconds': 1618444112, 'microseconds': 790928}, 'event': 
'STOP'}
<<< {'return': {'blocked': False, 'status': 'completed', 'setup-time': 1, 
'downtime': 1, 'total-time': 17, 'ram': {'total': 135274496, 
'postcopy-requests': 0, 'dirty-sync-count': 2, 'multifd-bytes': 0, 
'pages-per-second': 0, 'page-size': 4096, 'remaining': 0, 'mbps': 282.253, 
'transferred': 528415, 'duplicate': 33170, 'dirty-pages-rate': 0, 'skipped': 0, 
'normal-bytes': 229376, 'normal': 56}}}
>>> {'execute': 'query-migrate'}
<<< {'timestamp': {'seconds': 1618444112, 'microseconds': 792061}, 'event': 
'RESUME'}
<<< {'return': {'blocked': False, 'status': 'completed'}}
>>> {'execute': 'query-migrate'}
<<< {'return': {'blocked': False, 'status': 'completed', 'setup-time': 1, 
'downtime': 1, 'total-time': 17, 'ram': {'total': 135274496, 
'postcopy-requests': 0, 'dirty-sync-count': 2, 'multifd-bytes': 0, 
'pages-per-second': 0, 'page-size': 4096, 'remaining': 0, 'mbps': 282.253, 
'transferred': 528415, 'duplicate': 33170, 'dirty-pages-rate': 0, 'skipped': 0, 
'normal-bytes': 229376, 'normal': 56}}}
>>> {'execute': 'query-migrate'}
<<< {'return': {'blocked': False, 'status': 'completed'}}
>>> {'execute': 'query-status'}
<<< {'return': {'status': 'running', 'singlestep': False, 'running': True}}
>>> {'execute': 'query-status'}
<<< {'return': {'status': 'postmigrate', 'singlestep': False, 'running': False}}
>>> {'execute': 'quit'}
<<< {'return': {}}
>>> {'execute': 'quit'}
<<< {'return': {}}
DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stdout.expected) => NOT FOUND (data sources: variant, test, file)
DATA (filename=stderr.expected) => NOT FOUND (data sources: variant, test, file)
PASS 1-tests/acceptance/migration.py:Migration.test_migration_with_tcp_localhost

** Changed in: qemu
   Status: New => Fix Committed

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

Title:
  Acceptance Tests: migration fails on sparc target

Status in QEMU:
  Fix Committed

Bug description:
  QEMU fails migration when using a sparc target.

  This cab be verified/reproduced with the
  `tests/acceptance/migration.py` test.  Running it with:

   $ make check-venv
   $ ./tests/venv/bin/avocado --show=test run -p qemu_bin=./qemu-system-sparc 
tests/acceptance/migration.py:Migration.test_migration_with_tcp_localhost

  Right after a QMP `query-migrate` is executed, communication with the
  monitor is lost:

  >>> {'execute': 'query-migrate'}
  <<< {'timestamp': {'seconds': 1617667984, 'microseconds': 330282}, 'event': 
'STOP'}
  <<< {'return': {'blocked': False, 'status': 'completed', 'setup-time': 0, 
'downtime': 1, 'total-time': 15, 'ram': {'total': 135274496, 
'postcopy-requests': 0, 'dirty-sync-count': 2, 'multifd-bytes': 0, 
'pages-per-second': 0, 'page-size': 4096, 'remaining': 0, 'mbps': 
301.22347, 'transferred': 528703, 'duplicate': 33202, 
'dirty-pages-rate': 0, 'skipped': 0, 'normal-bytes': 229376, 'normal': 56}}}
  >>> {'execute': 'query-migrate'}

  Reproduced traceback from: 
/var/lib/users/cleber/build/qemu/tests/venv/lib64/python3.7/site-packages/avocado/core/test.py:756
  Traceback (most recent call last):
File "/var/lib/users/cleber/build/qemu/tests/acceptance/migration.py", line 
80, in test_migration_with_tcp_localhost
  self.do_migrate(dest_uri)
File "/var/lib/users/cleber/build/qemu/tests/acceptance/migration.py", line 
69, in do_migrate
  self.assert_migration(source_vm, dest_vm)
File "/var/lib/users/cleber/build/qemu/tests/acceptance/migration.py", line 
41, in assert_migration
  args=(dst_vm,))
File 
"/var/lib/users/cleber/build/qemu/tests/venv/lib64/python3.7/site-packages/avocado/utils/wait.py",
 line 34, in wait_for
  output = func(*args, **kwargs)
File "/var/lib/users/cleber/build/qemu/tests/acceptance/migration.py", line 
31, in migration_finished
  return vm.command('query-migrate')['status'] in ('completed', 'failed')
File "/home/cleber/src/qemu/python/qemu/machine.py", line 572, in command
  return self._qmp.command(cmd, 

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

[Richard Henderson]

On 4/14/21 12:11 PM, Richard Henderson wrote:

static bool
allow_prefix_MLS(DisasContext *ctx, arg_D *a)
{
     int64_t imm;

     /* Require MLS prefix or no prefix. */
     if (ctx->prefix_type != PREFIX_MLS) {
     if (ctx->prefix_type == PREFIX_NONE) {
     return true;
     }
     gen_invalid(ctx);
     return false;
     }


Combined with the switch on prefix_type in translate_insn, I think this can 
just simplify to


if (ctx->prefix_type != PREFIX_MLS) {
return ctx->prefix_type == PREFIX_NONE;
}

because decode_legacy is only called from within PREFIX_NONE.


r~

Re: [PATCH RFC v5 07/12] hw/riscv: PLIC update external interrupt by KVM when kvm enabled

[Alistair Francis]

On Mon, Apr 12, 2021 at 4:57 PM Yifei Jiang  wrote:
>
> Only support supervisor external interrupt currently.
>
> Signed-off-by: Yifei Jiang 
> Signed-off-by: Yipeng Yin 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/intc/sifive_plic.c| 29 -
>  target/riscv/kvm-stub.c  |  5 +
>  target/riscv/kvm.c   | 20 
>  target/riscv/kvm_riscv.h |  1 +
>  4 files changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index 97a1a27a9a..2746eb7a05 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -31,6 +31,8 @@
>  #include "target/riscv/cpu.h"
>  #include "sysemu/sysemu.h"
>  #include "migration/vmstate.h"
> +#include "sysemu/kvm.h"
> +#include "kvm_riscv.h"
>
>  #define RISCV_DEBUG_PLIC 0
>
> @@ -147,15 +149,24 @@ static void sifive_plic_update(SiFivePLICState *plic)
>  continue;
>  }
>  int level = sifive_plic_irqs_pending(plic, addrid);
> -switch (mode) {
> -case PLICMode_M:
> -riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, 
> BOOL_TO_MASK(level));
> -break;
> -case PLICMode_S:
> -riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, 
> BOOL_TO_MASK(level));
> -break;
> -default:
> -break;
> +if (kvm_enabled()) {
> +if (mode == PLICMode_M) {
> +continue;
> +}
> +kvm_riscv_set_irq(RISCV_CPU(cpu), IRQ_S_EXT, level);
> +} else {
> +switch (mode) {
> +case PLICMode_M:
> +riscv_cpu_update_mip(RISCV_CPU(cpu),
> + MIP_MEIP, BOOL_TO_MASK(level));
> +break;
> +case PLICMode_S:
> +riscv_cpu_update_mip(RISCV_CPU(cpu),
> + MIP_SEIP, BOOL_TO_MASK(level));
> +break;
> +default:
> +break;
> +}
>  }
>  }
>
> diff --git a/target/riscv/kvm-stub.c b/target/riscv/kvm-stub.c
> index 39b96fe3f4..4e8fc31a21 100644
> --- a/target/riscv/kvm-stub.c
> +++ b/target/riscv/kvm-stub.c
> @@ -23,3 +23,8 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
>  {
>  abort();
>  }
> +
> +void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
> +{
> +abort();
> +}
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 79c931acb4..da63535812 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -453,6 +453,26 @@ void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
>  env->gpr[11] = cpu->env.fdt_addr;  /* a1 */
>  }
>
> +void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level)
> +{
> +int ret;
> +unsigned virq = level ? KVM_INTERRUPT_SET : KVM_INTERRUPT_UNSET;
> +
> +if (irq != IRQ_S_EXT) {
> +return;
> +}
> +
> +if (!kvm_enabled()) {
> +return;
> +}
> +
> +ret = kvm_vcpu_ioctl(CPU(cpu), KVM_INTERRUPT, );
> +if (ret < 0) {
> +perror("Set irq failed");
> +abort();
> +}
> +}
> +
>  bool kvm_arch_cpu_check_are_resettable(void)
>  {
>  return true;
> diff --git a/target/riscv/kvm_riscv.h b/target/riscv/kvm_riscv.h
> index f38c82bf59..ed281bdce0 100644
> --- a/target/riscv/kvm_riscv.h
> +++ b/target/riscv/kvm_riscv.h
> @@ -20,5 +20,6 @@
>  #define QEMU_KVM_RISCV_H
>
>  void kvm_riscv_reset_vcpu(RISCVCPU *cpu);
> +void kvm_riscv_set_irq(RISCVCPU *cpu, int irq, int level);
>
>  #endif
> --
> 2.19.1
>
>

Re: [PATCH RFC v5 06/12] target/riscv: Support start kernel directly by KVM

[Alistair Francis]

On Mon, Apr 12, 2021 at 4:56 PM Yifei Jiang  wrote:
>
> Get kernel and fdt start address in virt.c, and pass them to KVM
> when cpu reset. In addition, add kvm_riscv.h to place riscv specific
> interface.
>
> Signed-off-by: Yifei Jiang 
> Signed-off-by: Yipeng Yin 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  hw/riscv/boot.c  | 11 +++
>  hw/riscv/virt.c  |  7 +++
>  include/hw/riscv/boot.h  |  1 +
>  target/riscv/cpu.c   |  8 
>  target/riscv/cpu.h   |  3 +++
>  target/riscv/kvm-stub.c  | 25 +
>  target/riscv/kvm.c   | 13 +
>  target/riscv/kvm_riscv.h | 24 
>  target/riscv/meson.build |  2 +-
>  9 files changed, 93 insertions(+), 1 deletion(-)
>  create mode 100644 target/riscv/kvm-stub.c
>  create mode 100644 target/riscv/kvm_riscv.h
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 0d38bb7426..b9741a647d 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -290,3 +290,14 @@ void riscv_setup_rom_reset_vec(MachineState *machine, 
> RISCVHartArrayState *harts
>
>  return;
>  }
> +
> +void riscv_setup_direct_kernel(hwaddr kernel_addr, hwaddr fdt_addr)
> +{
> +CPUState *cs;
> +
> +for (cs = first_cpu; cs; cs = CPU_NEXT(cs)) {
> +RISCVCPU *riscv_cpu = RISCV_CPU(cs);
> +riscv_cpu->env.kernel_addr = kernel_addr;
> +riscv_cpu->env.fdt_addr = fdt_addr;
> +}
> +}
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index c0dc69ff33..4a1fca139c 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -728,6 +728,13 @@ static void virt_machine_init(MachineState *machine)
>virt_memmap[VIRT_MROM].size, kernel_entry,
>fdt_load_addr, machine->fdt);
>
> +/*
> + * Only direct boot kernel is currently supported for KVM VM,
> + * So here setup kernel start address and fdt address.
> + * TODO:Support firmware loading and integrate to TCG start
> + */
> +riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
> +
>  /* SiFive Test MMIO device */
>  sifive_test_create(memmap[VIRT_TEST].base);
>
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index 11a21dd584..28d838cc29 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -51,5 +51,6 @@ void riscv_rom_copy_firmware_info(MachineState *machine, 
> hwaddr rom_base,
>hwaddr rom_size,
>uint32_t reset_vec_size,
>uint64_t kernel_entry);
> +void riscv_setup_direct_kernel(hwaddr kernel_addr, hwaddr fdt_addr);
>
>  #endif /* RISCV_BOOT_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7d6ed80f6b..dd34ab4978 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -29,6 +29,8 @@
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
>  #include "fpu/softfloat-helpers.h"
> +#include "sysemu/kvm.h"
> +#include "kvm_riscv.h"
>
>  /* RISC-V CPU definitions */
>
> @@ -361,6 +363,12 @@ static void riscv_cpu_reset(DeviceState *dev)
>  cs->exception_index = EXCP_NONE;
>  env->load_res = -1;
>  set_default_nan_mode(1, >fp_status);
> +
> +#ifndef CONFIG_USER_ONLY
> +if (kvm_enabled()) {
> +kvm_riscv_reset_vcpu(cpu);
> +}
> +#endif
>  }
>
>  static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0a33d387ba..a489d94187 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -243,6 +243,9 @@ struct CPURISCVState {
>
>  /* Fields from here on are preserved across CPU reset. */
>  QEMUTimer *timer; /* Internal timer */
> +
> +hwaddr kernel_addr;
> +hwaddr fdt_addr;
>  };
>
>  OBJECT_DECLARE_TYPE(RISCVCPU, RISCVCPUClass,
> diff --git a/target/riscv/kvm-stub.c b/target/riscv/kvm-stub.c
> new file mode 100644
> index 00..39b96fe3f4
> --- /dev/null
> +++ b/target/riscv/kvm-stub.c
> @@ -0,0 +1,25 @@
> +/*
> + * QEMU KVM RISC-V specific function stubs
> + *
> + * Copyright (c) 2020 Huawei Technologies Co., Ltd
> + *
> + * 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 .
> + */
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "kvm_riscv.h"
> +
> +void kvm_riscv_reset_vcpu(RISCVCPU *cpu)
> +{
> +abort();
> +}
> diff --git 

Re: [PATCH RFC v5 05/12] target/riscv: Implement kvm_arch_put_registers

[Alistair Francis]

On Mon, Apr 12, 2021 at 4:56 PM Yifei Jiang  wrote:
>
> Put GPR CSR and FP registers to kvm by KVM_SET_ONE_REG ioctl
>
> Signed-off-by: Yifei Jiang 
> Signed-off-by: Yipeng Yin 
> ---
>  target/riscv/kvm.c | 142 -
>  1 file changed, 141 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 63485d7b65..9d1441952a 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -85,6 +85,31 @@ static int kvm_riscv_get_regs_core(CPUState *cs)
>  return ret;
>  }
>
> +static int kvm_riscv_put_regs_core(CPUState *cs)
> +{
> +int ret = 0;
> +int i;
> +target_ulong reg;
> +CPURISCVState *env = _CPU(cs)->env;
> +
> +reg = env->pc;
> +ret = kvm_set_one_reg(cs, RISCV_CORE_REG(env, regs.pc), );
> +if (ret) {
> +return ret;
> +}
> +
> +for (i = 1; i < 32; i++) {
> +__u64 id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, i);

Can you use uint64_t for the entire series instead?

> +reg = env->gpr[i];
> +ret = kvm_set_one_reg(cs, id, );
> +if (ret) {
> +return ret;
> +}
> +}
> +
> +return ret;
> +}
> +
>  static int kvm_riscv_get_regs_csr(CPUState *cs)
>  {
>  int ret = 0;
> @@ -148,6 +173,70 @@ static int kvm_riscv_get_regs_csr(CPUState *cs)
>  return ret;
>  }
>
> +static int kvm_riscv_put_regs_csr(CPUState *cs)
> +{
> +int ret = 0;
> +target_ulong reg;
> +CPURISCVState *env = _CPU(cs)->env;
> +
> +reg = env->mstatus;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, sstatus), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->mie;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, sie), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->stvec;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, stvec), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->sscratch;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, sscratch), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->sepc;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, sepc), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->scause;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, scause), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->sbadaddr;

This will change soon-ish as my next PR converts this to stval.

> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, stval), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->mip;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, sip), );
> +if (ret) {
> +return ret;
> +}
> +
> +reg = env->satp;
> +ret = kvm_set_one_reg(cs, RISCV_CSR_REG(env, satp), );
> +if (ret) {
> +return ret;
> +}
> +
> +return ret;
> +}
> +
> +

Double line here.

Otherwise:

Reviewed-by: Alistair Francis 

Alistair

>  static int kvm_riscv_get_regs_fp(CPUState *cs)
>  {
>  int ret = 0;
> @@ -181,6 +270,40 @@ static int kvm_riscv_get_regs_fp(CPUState *cs)
>  return ret;
>  }
>
> +static int kvm_riscv_put_regs_fp(CPUState *cs)
> +{
> +int ret = 0;
> +int i;
> +CPURISCVState *env = _CPU(cs)->env;
> +
> +if (riscv_has_ext(env, RVD)) {
> +uint64_t reg;
> +for (i = 0; i < 32; i++) {
> +reg = env->fpr[i];
> +ret = kvm_set_one_reg(cs, RISCV_FP_D_REG(env, i), );
> +if (ret) {
> +return ret;
> +}
> +}
> +return ret;
> +}
> +
> +if (riscv_has_ext(env, RVF)) {
> +uint32_t reg;
> +for (i = 0; i < 32; i++) {
> +reg = env->fpr[i];
> +ret = kvm_set_one_reg(cs, RISCV_FP_F_REG(env, i), );
> +if (ret) {
> +return ret;
> +}
> +}
> +return ret;
> +}
> +
> +return ret;
> +}
> +
> +
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>  KVM_CAP_LAST_INFO
>  };
> @@ -209,7 +332,24 @@ int kvm_arch_get_registers(CPUState *cs)
>
>  int kvm_arch_put_registers(CPUState *cs, int level)
>  {
> -return 0;
> +int ret = 0;
> +
> +ret = kvm_riscv_put_regs_core(cs);
> +if (ret) {
> +return ret;
> +}
> +
> +ret = kvm_riscv_put_regs_csr(cs);
> +if (ret) {
> +return ret;
> +}
> +
> +ret = kvm_riscv_put_regs_fp(cs);
> +if (ret) {
> +return ret;
> +}
> +
> +return ret;
>  }
>
>  int kvm_arch_release_virq_post(int virq)
> --
> 2.19.1
>
>

Re: [PATCH RFC v5 04/12] target/riscv: Implement kvm_arch_get_registers

[Alistair Francis]

On Mon, Apr 12, 2021 at 4:58 PM Yifei Jiang  wrote:
>
> Get GPR CSR and FP registers from kvm by KVM_GET_ONE_REG ioctl.
>
> Signed-off-by: Yifei Jiang 
> Signed-off-by: Yipeng Yin 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/kvm.c | 150 -
>  1 file changed, 149 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 0d924be33f..63485d7b65 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -50,13 +50,161 @@ static __u64 kvm_riscv_reg_id(CPURISCVState *env, __u64 
> type, __u64 idx)
>  return id;
>  }
>
> +#define RISCV_CORE_REG(env, name)  kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, 
> \
> + KVM_REG_RISCV_CORE_REG(name))
> +
> +#define RISCV_CSR_REG(env, name)  kvm_riscv_reg_id(env, KVM_REG_RISCV_CSR, \
> + KVM_REG_RISCV_CSR_REG(name))
> +
> +#define RISCV_FP_F_REG(env, idx)  kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_F, 
> idx)
> +
> +#define RISCV_FP_D_REG(env, idx)  kvm_riscv_reg_id(env, KVM_REG_RISCV_FP_D, 
> idx)
> +
> +static int kvm_riscv_get_regs_core(CPUState *cs)
> +{
> +int ret = 0;
> +int i;
> +target_ulong reg;
> +CPURISCVState *env = _CPU(cs)->env;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CORE_REG(env, regs.pc), );
> +if (ret) {
> +return ret;
> +}
> +env->pc = reg;
> +
> +for (i = 1; i < 32; i++) {
> +__u64 id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CORE, i);
> +ret = kvm_get_one_reg(cs, id, );
> +if (ret) {
> +return ret;
> +}
> +env->gpr[i] = reg;
> +}
> +
> +return ret;
> +}
> +
> +static int kvm_riscv_get_regs_csr(CPUState *cs)
> +{
> +int ret = 0;
> +target_ulong reg;
> +CPURISCVState *env = _CPU(cs)->env;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, sstatus), );
> +if (ret) {
> +return ret;
> +}
> +env->mstatus = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, sie), );
> +if (ret) {
> +return ret;
> +}
> +env->mie = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, stvec), );
> +if (ret) {
> +return ret;
> +}
> +env->stvec = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, sscratch), );
> +if (ret) {
> +return ret;
> +}
> +env->sscratch = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, sepc), );
> +if (ret) {
> +return ret;
> +}
> +env->sepc = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, scause), );
> +if (ret) {
> +return ret;
> +}
> +env->scause = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, stval), );
> +if (ret) {
> +return ret;
> +}
> +env->sbadaddr = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, sip), );
> +if (ret) {
> +return ret;
> +}
> +env->mip = reg;
> +
> +ret = kvm_get_one_reg(cs, RISCV_CSR_REG(env, satp), );
> +if (ret) {
> +return ret;
> +}
> +env->satp = reg;
> +
> +return ret;
> +}
> +
> +static int kvm_riscv_get_regs_fp(CPUState *cs)
> +{
> +int ret = 0;
> +int i;
> +CPURISCVState *env = _CPU(cs)->env;
> +
> +if (riscv_has_ext(env, RVD)) {
> +uint64_t reg;
> +for (i = 0; i < 32; i++) {
> +ret = kvm_get_one_reg(cs, RISCV_FP_D_REG(env, i), );
> +if (ret) {
> +return ret;
> +}
> +env->fpr[i] = reg;
> +}
> +return ret;
> +}
> +
> +if (riscv_has_ext(env, RVF)) {
> +uint32_t reg;
> +for (i = 0; i < 32; i++) {
> +ret = kvm_get_one_reg(cs, RISCV_FP_F_REG(env, i), );
> +if (ret) {
> +return ret;
> +}
> +env->fpr[i] = reg;
> +}
> +return ret;
> +}
> +
> +return ret;
> +}
> +
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>  KVM_CAP_LAST_INFO
>  };
>
>  int kvm_arch_get_registers(CPUState *cs)
>  {
> -return 0;
> +int ret = 0;
> +
> +ret = kvm_riscv_get_regs_core(cs);
> +if (ret) {
> +return ret;
> +}
> +
> +ret = kvm_riscv_get_regs_csr(cs);
> +if (ret) {
> +return ret;
> +}
> +
> +ret = kvm_riscv_get_regs_fp(cs);
> +if (ret) {
> +return ret;
> +}
> +
> +return ret;
>  }
>
>  int kvm_arch_put_registers(CPUState *cs, int level)
> --
> 2.19.1
>
>

Re: [PATCH RFC v5 09/12] target/riscv: Add host cpu type

[Alistair Francis]

On Mon, Apr 12, 2021 at 4:54 PM Yifei Jiang  wrote:
>
> 'host' type cpu is set isa to RVXLEN simply, more isa info
> will obtain from KVM in kvm_arch_init_vcpu()
>
> Signed-off-by: Yifei Jiang 
> Signed-off-by: Yipeng Yin 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 9 +
>  target/riscv/cpu.h | 1 +
>  2 files changed, 10 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index dd34ab4978..8132d35a92 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -216,6 +216,12 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
>  }
>  #endif
>
> +static void riscv_host_cpu_init(Object *obj)
> +{
> +CPURISCVState *env = _CPU(obj)->env;
> +set_misa(env, RVXLEN);
> +}
> +
>  static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
>  {
>  ObjectClass *oc;
> @@ -706,6 +712,9 @@ static const TypeInfo riscv_cpu_type_infos[] = {
>  .class_init = riscv_cpu_class_init,
>  },
>  DEFINE_CPU(TYPE_RISCV_CPU_ANY,  riscv_any_cpu_init),
> +#if defined(CONFIG_KVM)
> +DEFINE_CPU(TYPE_RISCV_CPU_HOST, riscv_host_cpu_init),
> +#endif
>  #if defined(TARGET_RISCV32)
>  DEFINE_CPU(TYPE_RISCV_CPU_BASE32,   rv32_base_cpu_init),
>  DEFINE_CPU(TYPE_RISCV_CPU_IBEX, rv32_ibex_cpu_init),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a489d94187..3ca3dad341 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -43,6 +43,7 @@
>  #define TYPE_RISCV_CPU_SIFIVE_E51   RISCV_CPU_TYPE_NAME("sifive-e51")
>  #define TYPE_RISCV_CPU_SIFIVE_U34   RISCV_CPU_TYPE_NAME("sifive-u34")
>  #define TYPE_RISCV_CPU_SIFIVE_U54   RISCV_CPU_TYPE_NAME("sifive-u54")
> +#define TYPE_RISCV_CPU_HOST RISCV_CPU_TYPE_NAME("host")
>
>  #if defined(TARGET_RISCV32)
>  # define TYPE_RISCV_CPU_BASETYPE_RISCV_CPU_BASE32
> --
> 2.19.1
>
>

Re: [PATCH RFC v5 03/12] target/riscv: Implement function kvm_arch_init_vcpu

[Alistair Francis]

On Mon, Apr 12, 2021 at 4:53 PM Yifei Jiang  wrote:
>
> Get isa info from kvm while kvm init.
>
> Signed-off-by: Yifei Jiang 
> Signed-off-by: Yipeng Yin 
> ---
>  target/riscv/kvm.c | 27 ++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/kvm.c b/target/riscv/kvm.c
> index 687dd4b621..0d924be33f 100644
> --- a/target/riscv/kvm.c
> +++ b/target/riscv/kvm.c
> @@ -38,6 +38,18 @@
>  #include "qemu/log.h"
>  #include "hw/loader.h"
>
> +static __u64 kvm_riscv_reg_id(CPURISCVState *env, __u64 type, __u64 idx)
> +{
> +__u64 id = KVM_REG_RISCV | type | idx;

Can you use uint64_t instead of __u64?

Once that is fixed:

Reviewed-by: Alistair Francis 

Alistair

> +
> +if (riscv_cpu_is_32bit(env)) {
> +id |= KVM_REG_SIZE_U32;
> +} else {
> +id |= KVM_REG_SIZE_U64;
> +}
> +return id;
> +}
> +
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>  KVM_CAP_LAST_INFO
>  };
> @@ -79,7 +91,20 @@ void kvm_arch_init_irq_routing(KVMState *s)
>
>  int kvm_arch_init_vcpu(CPUState *cs)
>  {
> -return 0;
> +int ret = 0;
> +target_ulong isa;
> +RISCVCPU *cpu = RISCV_CPU(cs);
> +CPURISCVState *env = >env;
> +__u64 id;
> +
> +id = kvm_riscv_reg_id(env, KVM_REG_RISCV_CONFIG, 
> KVM_REG_RISCV_CONFIG_REG(isa));
> +ret = kvm_get_one_reg(cs, id, );
> +if (ret) {
> +return ret;
> +}
> +env->misa = isa | RVXLEN;
> +
> +return ret;
>  }
>
>  int kvm_arch_msi_data_to_gsi(uint32_t data)
> --
> 2.19.1
>
>

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

[Alistair Francis]

Thanks for raising this. I have marked it as fixes as like you say it's
fixed in mainline.

** Changed in: qemu
   Status: New => Fix Committed

-- 
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:
  Fix Committed

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

[PATCH 1/3] Acceptance Tests: rename attribute holding the distro image checksum

[Cleber Rosa]

This renames the attribute that holds the checksum for the image Linux
distribution image used.

The current name of the attribute is not very descriptive.  Also, in
preparation for making the distribution used configurable, which will
add distro related parameters, attributes and tags, let's make the
naming of those more uniform.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 4 ++--
 tests/acceptance/boot_linux.py| 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 1062a851b9..aae1e5bbc9 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -307,7 +307,7 @@ class LinuxTest(Test, LinuxSSHMixIn):
 """
 
 timeout = 900
-chksum = None
+distro_checksum = None
 username = 'root'
 password = 'password'
 
@@ -355,7 +355,7 @@ def download_boot(self):
 try:
 boot = vmimage.get(
 'fedora', arch=image_arch, version='31',
-checksum=self.chksum,
+checksum=self.distro_checksum,
 algorithm='sha256',
 cache_dir=self.cache_dirs[0],
 snapshot_dir=self.workdir)
diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
index 314370fd1f..c7bc3a589e 100644
--- a/tests/acceptance/boot_linux.py
+++ b/tests/acceptance/boot_linux.py
@@ -20,7 +20,7 @@ class BootLinuxX8664(LinuxTest):
 :avocado: tags=arch:x86_64
 """
 
-chksum = 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
+distro_checksum = 
'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
 
 def test_pc_i440fx_tcg(self):
 """
@@ -66,7 +66,7 @@ class BootLinuxAarch64(LinuxTest):
 :avocado: tags=machine:gic-version=2
 """
 
-chksum = '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'
+distro_checksum = 
'1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'
 
 def add_common_args(self):
 self.vm.add_args('-bios',
@@ -119,7 +119,7 @@ class BootLinuxPPC64(LinuxTest):
 :avocado: tags=arch:ppc64
 """
 
-chksum = '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'
+distro_checksum = 
'7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'
 
 def test_pseries_tcg(self):
 """
@@ -136,7 +136,7 @@ class BootLinuxS390X(LinuxTest):
 :avocado: tags=arch:s390x
 """
 
-chksum = '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'
+distro_checksum = 
'4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'
 
 @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
 def test_s390_ccw_virtio_tcg(self):
-- 
2.25.4

[PATCH 0/3] Acceptance Tests: support choosing specific distro and version

[Cleber Rosa]

Because Fedora 31 will not suit all tests that depend on a Linux
guest, this allows for the configuration of the guest distribution.
It came out of a suggestion from Eric Auger, and it was actually a
feature I planned to submit for a while.

This is based on the following series:

 [PATCH v3 00/11] Acceptance Test: introduce base class for Linux based tests

A GitLab CI pipeline can be seen here:

 https://gitlab.com/cleber.gnu/qemu/-/pipelines

Note: I'll address the line length caught in the check-patch job as
soon as I find what was the outcome of the line limits for Python
code discussion.

Based-On: <20210412044644.55083-1-cr...@redhat.com>

Cleber Rosa (3):
  Acceptance Tests: rename attribute holding the distro image checksum
  Acceptance Tests: move definition of distro checksums to the framework
  Acceptance Tests: support choosing specific distro and version

 docs/devel/testing.rst| 65 ++
 tests/acceptance/avocado_qemu/__init__.py | 67 +--
 tests/acceptance/boot_linux.py|  8 ---
 3 files changed, 127 insertions(+), 13 deletions(-)

-- 
2.25.4

[PATCH 2/3] Acceptance Tests: move definition of distro checksums to the framework

[Cleber Rosa]

Instead of having, by default, the checksum in the tests, and the
definition of tests in the framework, let's keep them together.

A central definition for distributions is available, and it should
allow other known distros to be added more easily.

No behavior change is expected here, and tests can still define
a distro_checksum value if for some reason they want to override
the known distribution information.

Signed-off-by: Cleber Rosa 
---
 tests/acceptance/avocado_qemu/__init__.py | 34 +--
 tests/acceptance/boot_linux.py|  8 --
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index aae1e5bbc9..97093614d9 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -299,6 +299,30 @@ def ssh_command(self, command):
 return stdout_lines, stderr_lines
 
 
+#: A collection of known distros and their respective image checksum
+KNOWN_DISTROS = {
+'fedora': {
+'31': {
+'x86_64':
+{'checksum': 
'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
+'aarch64':
+{'checksum': 
'1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
+'ppc64':
+{'checksum': 
'7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
+'s390x':
+{'checksum': 
'4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
+}
+}
+}
+
+
+def get_known_distro_checksum(distro, distro_version, arch):
+try:
+return 
KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum')
+except AttributeError:
+return None
+
+
 class LinuxTest(Test, LinuxSSHMixIn):
 """Facilitates having a cloud-image Linux based available.
 
@@ -348,14 +372,20 @@ def download_boot(self):
 vmimage.QEMU_IMG = qemu_img
 
 self.log.info('Downloading/preparing boot image')
+distro = 'fedora'
+distro_version = '31'
+known_distro_checksum = get_known_distro_checksum(distro,
+  distro_version,
+  self.arch)
+distro_checksum = self.distro_checksum or known_distro_checksum
 # Fedora 31 only provides ppc64le images
 image_arch = self.arch
 if image_arch == 'ppc64':
 image_arch = 'ppc64le'
 try:
 boot = vmimage.get(
-'fedora', arch=image_arch, version='31',
-checksum=self.distro_checksum,
+distro, arch=image_arch, version=distro_version,
+checksum=distro_checksum,
 algorithm='sha256',
 cache_dir=self.cache_dirs[0],
 snapshot_dir=self.workdir)
diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
index c7bc3a589e..9e618c6daa 100644
--- a/tests/acceptance/boot_linux.py
+++ b/tests/acceptance/boot_linux.py
@@ -20,8 +20,6 @@ class BootLinuxX8664(LinuxTest):
 :avocado: tags=arch:x86_64
 """
 
-distro_checksum = 
'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'
-
 def test_pc_i440fx_tcg(self):
 """
 :avocado: tags=machine:pc
@@ -66,8 +64,6 @@ class BootLinuxAarch64(LinuxTest):
 :avocado: tags=machine:gic-version=2
 """
 
-distro_checksum = 
'1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'
-
 def add_common_args(self):
 self.vm.add_args('-bios',
  os.path.join(BUILD_DIR, 'pc-bios',
@@ -119,8 +115,6 @@ class BootLinuxPPC64(LinuxTest):
 :avocado: tags=arch:ppc64
 """
 
-distro_checksum = 
'7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'
-
 def test_pseries_tcg(self):
 """
 :avocado: tags=machine:pseries
@@ -136,8 +130,6 @@ class BootLinuxS390X(LinuxTest):
 :avocado: tags=arch:s390x
 """
 
-distro_checksum = 
'4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'
-
 @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
 def test_s390_ccw_virtio_tcg(self):
 """
-- 
2.25.4

[PATCH 3/3] Acceptance Tests: support choosing specific distro and version

[Cleber Rosa]

The tests based on the LinuxTest class give the test writer a ready to
use guest operating system, currently pinned to Fedora 31.

With this change, it's now possible to choose different distros and
versions, similar to how other tags and parameter can be set for the
target arch, accelerator, etc.

One of the reasons for this work, is that some development features
depend on updates on the guest side.  For instance the tests on
virtiofs_submounts.py, require newer kernels, and may benefit from
running, say on Fedora 34, without the need for a custom kernel.

Please notice that the pre-caching of the Fedora 31 images done during
the early stages of `make check-acceptance` (before the tests are
actually executed) are not expanded here to cover every new image
added.  But, the tests will download other needed images (and cache
them) during the first execution.

Signed-off-by: Cleber Rosa 
---
 docs/devel/testing.rst| 65 +++
 tests/acceptance/avocado_qemu/__init__.py | 47 
 2 files changed, 102 insertions(+), 10 deletions(-)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 4e42392810..19cbf532ae 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -922,6 +922,39 @@ The preserved value of the ``qemu_bin`` parameter or the 
result of the
 dynamic probe for a QEMU binary in the current working directory or
 source tree.
 
+LinuxTest
+~
+
+Besides the attributes present on the ``avocado_qemu.Test`` base
+class, the ``avocado_qemu.LinuxTest`` adds the following attributes:
+
+distro
+..
+
+The name of the Linux distribution used as the guest image for the
+test.  The name should match the **Provider** column on the list
+of images supported by the avocado.utils.vmimage library:
+
+https://avocado-framework.readthedocs.io/en/latest/guides/writer/libs/vmimage.html#supported-images
+
+distro_version
+..
+
+The version of the Linux distribution as the guest image for the
+test.  The name should match the **Version** column on the list
+of images supported by the avocado.utils.vmimage library:
+
+https://avocado-framework.readthedocs.io/en/latest/guides/writer/libs/vmimage.html#supported-images
+
+distro_checksum
+...
+
+The sha256 hash of the guest image file used for the test.
+
+If this value is not set in the code or by a test parameter (with the
+same name), no validation on the integrity of the image will be
+performed.
+
 Parameter reference
 ---
 
@@ -962,6 +995,38 @@ qemu_bin
 
 The exact QEMU binary to be used on QEMUMachine.
 
+LinuxTest
+~
+
+Besides the parameters present on the ``avocado_qemu.Test`` base
+class, the ``avocado_qemu.LinuxTest`` adds the following parameters:
+
+distro
+..
+
+The name of the Linux distribution used as the guest image for the
+test.  The name should match the **Provider** column on the list
+of images supported by the avocado.utils.vmimage library:
+
+https://avocado-framework.readthedocs.io/en/latest/guides/writer/libs/vmimage.html#supported-images
+
+distro_version
+..
+
+The version of the Linux distribution as the guest image for the
+test.  The name should match the **Version** column on the list
+of images supported by the avocado.utils.vmimage library:
+
+https://avocado-framework.readthedocs.io/en/latest/guides/writer/libs/vmimage.html#supported-images
+
+distro_checksum
+...
+
+The sha256 hash of the guest image file used for the test.
+
+If this value is not set in the code or by this parameter no
+validation on the integrity of the image will be performed.
+
 Skipping tests
 --
 The Avocado framework provides Python decorators which allow for easily skip
diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 97093614d9..6fd0016917 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -335,8 +335,39 @@ class LinuxTest(Test, LinuxSSHMixIn):
 username = 'root'
 password = 'password'
 
+def _set_distro(self):
+distro = self.params.get(
+'distro',
+default=self._get_unique_tag_val('distro'))
+if not distro:
+distro = 'fedora'
+self.distro = distro
+
+distro_version = self.params.get(
+'distro_version',
+default=self._get_unique_tag_val('distro_version'))
+if not distro_version:
+distro_version = '31'
+self.distro_version = distro_version
+
+# The distro checksum behaves differently than distro name and
+# version. First, it does not respect a tag with the same
+# name, given that it's not expected to be used for filtering
+# (distro name versions are the natural choice).  Second, the
+# order of precedence is: parameter, attribute and then value
+# from KNOWN_DISTROS.
+distro_checksum = 

Mac OS - Standalone Installer

[Richard Hill]

Hello Everyone!

 I would like to ask if anyone on this list is aware of a standalone installer 
for QEMU for Mac OS?

I am aware of the excellent work carried out by Stefan Weil for the Windows 
Platform and I wondered if something similar existed for the Mac?

The approach of using Homebrew will not work for what I am trying to achieve.

Many thanks in advance for any response / help.

regards

Richard

Re: [PATCH 0/2] virtiofsd: Enable xattr if xattrmap is used

[Connor Kuehl]

On Wed Apr 14, 2021 at 3:12 PM CDT, Carlos Venegas wrote:
>
> Using xattrmap for Kata Containers we found that xattr is should be used
> or xattrmap wont work. These patches enable xattr when -o xattrmap is
> used. Also, they add help for the xattrmap option on `virtiofsd --help`
> output.
>
> Carlos Venegas (2):
> virtiofsd: Allow use "-o xattrmap" without "-o xattr"
> virtiofsd: Add help for -o xattr-mapping

Good usability improvement.

For the series:

Reviewed-by: Connor Kuehl 

Re: [Virtio-fs] [PATCH 2/2] virtiofsd: Add help for -o xattr-mapping

[Connor Kuehl]

On Wed Apr 14, 2021 at 3:12 PM CDT, Carlos Venegas wrote:
> The option is not documented in help.
>
> Add small help about the option.
>
> Signed-off-by: Carlos Venegas 
> ---
> tools/virtiofsd/helper.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> index 28243b51b2..5e98ed702b 100644
> --- a/tools/virtiofsd/helper.c
> +++ b/tools/virtiofsd/helper.c
> @@ -172,6 +172,9 @@ void fuse_cmdline_help(void)
> " default: no_writeback\n"
> " -o xattr|no_xattr enable/disable xattr\n"
> " default: no_xattr\n"
> + " -o xattrmap= Enable xattr mapping (enables xattr)\n"
> + "  is a string consists of a series of rules\n"
> + " e.g. -o xattrmap=:map::user.virtiofs.:\n"

This is a helpful note, but it doesn't tell the whole story. I think
it'd be helpful to add one last note to this option which is to
recommend reading the virtiofsd(1) man-page for more information on
xattrmap rules.

Connor

RE: [PATCH 00/11] Add support for Blob resources feature

[Kasireddy, Vivek]

Hi Gerd,

> 
> > Any other ideas as to how to eliminate that Blit cleanly?
> 
> Well, "cleanly" pretty much implies "supported by toolkit".
[Kasireddy, Vivek] I was kind of hoping you'd not draw that implication :)
> 
> gtk glarea for example sets up a framebuffer and expects the application 
> render to that
> framebuffer.  So qemu glarea code does a fb-to-fb blit.
[Kasireddy, Vivek] Right, that is how it works today but we'd like to not have 
that
blit and instead submit the Qemu application buffer (i.e Guest FB) directly to 
the
compositor  -- so that it can be placed on a hardware plane should the 
compositor
decide to do so. Only then it'd be absolute zero-copy but the compositor may 
decide
to composite it which would mean another blit that we cannot avoid. 

I am trying to make a concerted effort to accomplish this using GTK/GLArea:
https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3410

But I get a feeling that it is inadequate as GTK/GLArea does not manage the 
wl_buffers
submitted to the compositor -- EGL does. I suspect we either need to use a new 
GTK
mechanism -- that perhaps does not exist yet -- or not use GTK at all for this.

I do understand that adding a new purely Wayland backend would make it 
redundant given
that GTK, SDL, Spice, etc already support Wayland; however, I do not see any 
good options
available for eliminating that blit.

Thanks,
Vivek

> 
> Other reasons are scaling and cursor rendering.  Not all reasons apply to all 
> UIs.  I think
> when using spice qemu doesn't blit (not fully sure what happens inside 
> spice-server), but it
> could very well be that the spice-client does the blit instead, i.e. we just 
> shift the issue to
> another place ...
> 
> take care,
>   Gerd

[PATCH v7 4/4] target/arm: set ID_AA64ISAR0.TLB to 2 for max AARCH64 CPU type

[Rebecca Cran]

Indicate support for FEAT_TLBIOS and FEAT_TLBIRANGE by setting
ID_AA64ISAR0.TLB to 2 for the max AARCH64 CPU type.

Signed-off-by: Rebecca Cran 
Reviewed-by: Richard Henderson 
---
 target/arm/cpu64.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index f0a9e968c9c1..f42803ecaf1d 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -651,6 +651,7 @@ static void aarch64_max_initfn(Object *obj)
 t = FIELD_DP64(t, ID_AA64ISAR0, DP, 1);
 t = FIELD_DP64(t, ID_AA64ISAR0, FHM, 1);
 t = FIELD_DP64(t, ID_AA64ISAR0, TS, 2); /* v8.5-CondM */
+t = FIELD_DP64(t, ID_AA64ISAR0, TLB, 2); /* FEAT_TLBIRANGE */
 t = FIELD_DP64(t, ID_AA64ISAR0, RNDR, 1);
 cpu->isar.id_aa64isar0 = t;
 
-- 
2.26.2

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

[Richard Henderson]

On 4/14/21 1:07 PM, Bruno Piazera Larsen wrote:
I think it would be worth moving all of the SPR code out to a separate file, 
apart from cpu_init.c.  There's a lot of it.  And, yes, I would move everything

that you can that is related out of translate.c.


Yeah, now that I look at the SPR code, I'm starting to think it's easier
I think it's what fabiano had in mind too, but we'll probably have 3 files,
spr_common.c, spr_tcg.c and spr_kvm.c. It's a bit of surgery, but it's
probably worth it, to avoid a mess of ifdefs.


Sounds good.

While waiting for a reply I tried this. It's really not, it creates about 6k 
errors.

I ended up moving everything that used it from cpu_init.c into translate.c.
create_ppc_opcodes and destroy_ppc_opcodes ended up going there, and
I added prototypes to internal.h to call them in the realize and unrealize
functions.


Moving into translate.c sounds like a good option.


r~

[PATCH v7 1/4] accel/tcg: Add TLB invalidation support for ranges of addresses

[Rebecca Cran]

Add functions to support the FEAT_TLBIRANGE ARMv8.4 feature that adds
TLB invalidation instructions to invalidate ranges of addresses.

Signed-off-by: Rebecca Cran 
---
 accel/tcg/cputlb.c  | 130 +++-
 include/exec/exec-all.h |  46 +++
 2 files changed, 173 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 8a7b779270a4..dc44967dcf8e 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -709,7 +709,7 @@ void tlb_flush_page_all_cpus_synced(CPUState *src, 
target_ulong addr)
 tlb_flush_page_by_mmuidx_all_cpus_synced(src, addr, ALL_MMUIDX_BITS);
 }
 
-static void tlb_flush_page_bits_locked(CPUArchState *env, int midx,
+static bool tlb_flush_page_bits_locked(CPUArchState *env, int midx,
target_ulong page, unsigned bits)
 {
 CPUTLBDesc *d = _tlb(env)->d[midx];
@@ -729,7 +729,7 @@ static void tlb_flush_page_bits_locked(CPUArchState *env, 
int midx,
   TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
   midx, page, mask);
 tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime());
-return;
+return true;
 }
 
 /* Check if we need to flush due to large pages.  */
@@ -738,13 +738,14 @@ static void tlb_flush_page_bits_locked(CPUArchState *env, 
int midx,
   TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
   midx, d->large_page_addr, d->large_page_mask);
 tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime());
-return;
+return true;
 }
 
 if (tlb_flush_entry_mask_locked(tlb_entry(env, midx, page), page, mask)) {
 tlb_n_used_entries_dec(env, midx);
 }
 tlb_flush_vtlb_page_mask_locked(env, midx, page, mask);
+return false;
 }
 
 typedef struct {
@@ -943,6 +944,129 @@ void 
tlb_flush_page_bits_by_mmuidx_all_cpus_synced(CPUState *src_cpu,
 }
 }
 
+typedef struct {
+target_ulong addr;
+target_ulong length;
+uint16_t idxmap;
+uint16_t bits;
+}  TLBFlushPageRangeBitsByMMUIdxData;
+
+static void
+tlb_flush_page_range_bits_by_mmuidx_async_0(CPUState *cpu,
+target_ulong addr,
+target_ulong length,
+uint16_t idxmap,
+unsigned bits)
+{
+CPUArchState *env = cpu->env_ptr;
+int mmu_idx;
+target_ulong l;
+target_ulong page = addr;
+bool full_flush;
+
+assert_cpu_is_self(cpu);
+
+tlb_debug("page addr:" TARGET_FMT_lx "/%u len: " TARGET_FMT_lx
+  " mmu_map:0x%x\n",
+  addr, bits, length, idxmap);
+
+qemu_spin_lock(_tlb(env)->c.lock);
+for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
+if ((idxmap >> mmu_idx) & 1) {
+for (l = 0; l < length; l += TARGET_PAGE_SIZE) {
+page = addr + l;
+full_flush = tlb_flush_page_bits_locked(env, mmu_idx,
+page, bits);
+if (full_flush) {
+break;
+}
+}
+}
+}
+qemu_spin_unlock(_tlb(env)->c.lock);
+
+for (l = 0; l < length; l += TARGET_PAGE_SIZE) {
+tb_flush_jmp_cache(cpu, page);
+}
+}
+
+static void
+tlb_flush_page_range_bits_by_mmuidx_async_1(CPUState *cpu,
+run_on_cpu_data data)
+{
+TLBFlushPageRangeBitsByMMUIdxData *d = data.host_ptr;
+
+tlb_flush_page_range_bits_by_mmuidx_async_0(cpu, d->addr, d->length,
+d->idxmap, d->bits);
+
+g_free(d);
+}
+
+void tlb_flush_page_range_bits_by_mmuidx(CPUState *cpu,
+ target_ulong addr,
+ target_ulong length,
+ uint16_t idxmap,
+ unsigned bits)
+{
+TLBFlushPageRangeBitsByMMUIdxData d;
+TLBFlushPageRangeBitsByMMUIdxData *p;
+
+/* This should already be page aligned */
+addr &= TARGET_PAGE_BITS;
+
+d.addr = addr & TARGET_PAGE_MASK;
+d.idxmap = idxmap;
+d.bits = bits;
+d.length = length;
+
+if (qemu_cpu_is_self(cpu)) {
+tlb_flush_page_range_bits_by_mmuidx_async_0(cpu, addr, length,
+idxmap, bits);
+} else {
+p = g_new(TLBFlushPageRangeBitsByMMUIdxData, 1);
+
+/* Allocate a structure, freed by the worker.  */
+*p = d;
+async_run_on_cpu(cpu, tlb_flush_page_range_bits_by_mmuidx_async_1,
+ RUN_ON_CPU_HOST_PTR(p));
+}
+}
+
+void tlb_flush_page_range_bits_by_mmuidx_all_cpus_synced(CPUState *src_cpu,
+ target_ulong addr,
+ target_ulong length,
+   

[PATCH v7 2/4] target/arm: Add support for FEAT_TLBIRANGE

[Rebecca Cran]

ARMv8.4 adds the mandatory FEAT_TLBIRANGE. It provides TLBI
maintenance instructions that apply to a range of input addresses.

Signed-off-by: Rebecca Cran 
---
 target/arm/cpu.h|   5 +
 target/arm/helper.c | 296 
 2 files changed, 301 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 193a49ec7fac..32b78a4ef587 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -4038,6 +4038,11 @@ static inline bool isar_feature_aa64_pauth_arch(const 
ARMISARegisters *id)
 return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, APA) != 0;
 }
 
+static inline bool isar_feature_aa64_tlbirange(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TLB) == 2;
+}
+
 static inline bool isar_feature_aa64_sb(const ARMISARegisters *id)
 {
 return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, SB) != 0;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index d9220be7c5a0..fedc82efa57e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4759,6 +4759,219 @@ static void tlbi_aa64_vae3is_write(CPUARMState *env, 
const ARMCPRegInfo *ri,
   ARMMMUIdxBit_SE3, bits);
 }
 
+#ifdef TARGET_AARCH64
+static uint64_t tlbi_aa64_range_get_length(CPUARMState *env,
+   uint64_t value)
+{
+unsigned int page_shift;
+unsigned int page_size_granule;
+uint64_t num;
+uint64_t scale;
+uint64_t exponent;
+uint64_t length;
+
+num = extract64(value, 39, 4);
+scale = extract64(value, 44, 2);
+page_size_granule = extract64(value, 46, 2);
+
+page_shift = page_size_granule * 2 + 10;
+
+if (page_size_granule == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "Invalid page size granule %d\n",
+  page_size_granule);
+return 0;
+}
+
+exponent = (5 * scale) + 1;
+length = (num + 1) << (exponent + page_shift);
+
+return length;
+}
+
+static void tlbi_aa64_rvae1_write(CPUARMState *env, const ARMCPRegInfo *ri,
+  uint64_t value)
+{
+/*
+ * Invalidate by VA range, EL1&0.
+ * Currently handles all of RVAE1, RVAAE1, RVAALE1 and RVALE1,
+ * since we don't support flush-for-specific-ASID-only or
+ * flush-last-level-only.
+ */
+ARMMMUIdx mmu_idx;
+int mask;
+int bits;
+uint64_t pageaddr;
+uint64_t length;
+
+CPUState *cs = env_cpu(env);
+mask = vae1_tlbmask(env);
+mmu_idx = ARM_MMU_IDX_A | ctz32(mask);
+if (regime_has_2_ranges(mmu_idx)) {
+pageaddr = sextract64(value, 0, 37) << TARGET_PAGE_BITS;
+} else {
+pageaddr = extract64(value, 0, 37) << TARGET_PAGE_BITS;
+}
+length = tlbi_aa64_range_get_length(env, value);
+bits = tlbbits_for_regime(env, mmu_idx, pageaddr);
+
+if (tlb_force_broadcast(env)) {
+tlb_flush_page_range_bits_by_mmuidx_all_cpus_synced(cs, pageaddr,
+length, mask,
+bits);
+} else {
+tlb_flush_page_range_bits_by_mmuidx(cs, pageaddr, length, mask,
+bits);
+}
+}
+
+static void tlbi_aa64_rvae1is_write(CPUARMState *env, const ARMCPRegInfo *ri,
+uint64_t value)
+{
+/*
+ * Invalidate by VA range, Inner/Outer Shareable EL1&0.
+ * Currently handles all of RVAE1IS, RVAE1OS, RVAAE1IS, RVAAE1OS,
+ * RVAALE1IS, RVAALE1OS, RVALE1IS and RVALE1OS, since we don't support
+ * flush-for-specific-ASID-only, flush-last-level-only or inner/outer
+ * shareable specific flushes.
+ */
+ARMMMUIdx mmu_idx;
+int mask;
+int bits;
+uint64_t pageaddr;
+uint64_t length;
+
+CPUState *cs = env_cpu(env);
+mask = vae1_tlbmask(env);
+mmu_idx = ARM_MMU_IDX_A | ctz32(mask);
+if (regime_has_2_ranges(mmu_idx)) {
+pageaddr = sextract64(value, 0, 37) << TARGET_PAGE_BITS;
+} else {
+pageaddr = extract64(value, 0, 37) << TARGET_PAGE_BITS;
+}
+length = tlbi_aa64_range_get_length(env, value);
+bits = tlbbits_for_regime(env, mmu_idx, pageaddr);
+
+tlb_flush_page_range_bits_by_mmuidx_all_cpus_synced(cs, pageaddr,
+length, mask,
+bits);
+}
+
+static void tlbi_aa64_rvae2_write(CPUARMState *env, const ARMCPRegInfo *ri,
+  uint64_t value)
+{
+/*
+ * Invalidate by VA range, EL2.
+ * Currently handles all of RVAE2 and RVALE2,
+ * since we don't support flush-for-specific-ASID-only or
+ * flush-last-level-only.
+ */
+bool secure;
+int mask;
+int bits;
+uint64_t pageaddr;
+uint64_t length;
+
+CPUState *cs = env_cpu(env);
+secure = arm_is_secure_below_el3(env);
+pageaddr = extract64(value, 0, 37) << TARGET_PAGE_BITS;

[PATCH v7 3/4] target/arm: Add support for FEAT_TLBIOS

[Rebecca Cran]

ARMv8.4 adds the mandatory FEAT_TLBIOS. It provides TLBI
maintenance instructions that extend to the Outer Shareable domain.

Signed-off-by: Rebecca Cran 
---
 target/arm/cpu.h|  5 ++
 target/arm/helper.c | 75 
 2 files changed, 80 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 32b78a4ef587..272fde83ca4e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -4043,6 +4043,11 @@ static inline bool isar_feature_aa64_tlbirange(const 
ARMISARegisters *id)
 return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TLB) == 2;
 }
 
+static inline bool isar_feature_aa64_tlbios(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64isar0, ID_AA64ISAR0, TLB) != 0;
+}
+
 static inline bool isar_feature_aa64_sb(const ARMISARegisters *id)
 {
 return FIELD_EX64(id->id_aa64isar1, ID_AA64ISAR1, SB) != 0;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index fedc82efa57e..0894ddca59f6 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7213,6 +7213,78 @@ static const ARMCPRegInfo tlbirange_reginfo[] = {
 REGINFO_SENTINEL
 };
 
+static const ARMCPRegInfo tlbios_reginfo[] = {
+{ .name = "TLBI_VMALLE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 0,
+  .access = PL1_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_vmalle1is_write },
+{ .name = "TLBI_ASIDE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 1, .opc2 = 2,
+  .access = PL1_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_vmalle1is_write },
+{ .name = "TLBI_RVAE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 5, .opc2 = 1,
+  .access = PL1_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae1is_write },
+{ .name = "TLBI_RVAAE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 5, .opc2 = 3,
+  .access = PL1_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae1is_write },
+   { .name = "TLBI_RVALE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 5, .opc2 = 5,
+  .access = PL1_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae1is_write },
+{ .name = "TLBI_RVAALE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 0, .crn = 8, .crm = 5, .opc2 = 7,
+  .access = PL1_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae1is_write },
+{ .name = "TLBI_ALLE2OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 0,
+  .access = PL2_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_alle2is_write },
+   { .name = "TLBI_ALLE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 4,
+  .access = PL2_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_alle1is_write },
+{ .name = "TLBI_VMALLS12E1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 1, .opc2 = 6,
+  .access = PL2_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_alle1is_write },
+{ .name = "TLBI_IPAS2E1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 0,
+  .access = PL2_W, .type = ARM_CP_NOP },
+{ .name = "TLBI_RIPAS2E1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 3,
+  .access = PL2_W, .type = ARM_CP_NOP },
+{ .name = "TLBI_IPAS2LE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 4,
+  .access = PL2_W, .type = ARM_CP_NOP },
+{ .name = "TLBI_RIPAS2LE1OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 4, .opc2 = 7,
+  .access = PL2_W, .type = ARM_CP_NOP },
+   { .name = "TLBI_RVAE2OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 5, .opc2 = 1,
+  .access = PL2_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae2is_write },
+   { .name = "TLBI_RVALE2OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 4, .crn = 8, .crm = 5, .opc2 = 5,
+  .access = PL2_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae2is_write },
+{ .name = "TLBI_ALLE3OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 0,
+  .access = PL3_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_alle3is_write },
+   { .name = "TLBI_RVAE3OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 5, .opc2 = 1,
+  .access = PL3_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae3is_write },
+   { .name = "TLBI_RVALE3OS", .state = ARM_CP_STATE_AA64,
+  .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 5, .opc2 = 5,
+  .access = PL3_W, .type = ARM_CP_NO_RAW,
+  .writefn = tlbi_aa64_rvae3is_write },
+REGINFO_SENTINEL
+};
+
 static uint64_t rndr_readfn(CPUARMState *env, const ARMCPRegInfo *ri)
 {
 Error *err = NULL;
@@ -8585,6 +8657,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 if 

[PATCH v7 0/4] Add support for FEAT_TLBIOS and FEAT_TLBIRANGE

[Rebecca Cran]

ARMv8.4 adds the mandatory FEAT_TLBIOS and FEAT_TLBIRANGE. 
They provides TLBI maintenance instructions that extend to the Outer
Shareable domain and that apply to a range of input addresses.

Changes from v6 to v7:

o Fixed the tlbi_aa64_rvae1_write the tlbi_aa64_rvae1is_write functions
  to pass the correct value into functions which use an ARMMMUIdx.

o Fixed comments in helper.c which referred to non-existent instructions.

Testing:

o Booted Linux 5.11 - verified the previous assert failure in qemu is
  resolved.
o Ran checkpatch.pl.

Rebecca Cran (4):
  accel/tcg: Add TLB invalidation support for ranges of addresses
  target/arm: Add support for FEAT_TLBIRANGE
  target/arm: Add support for FEAT_TLBIOS
  target/arm: set ID_AA64ISAR0.TLB to 2 for max AARCH64 CPU type

 accel/tcg/cputlb.c  | 130 ++-
 include/exec/exec-all.h |  46 +++
 target/arm/cpu.h|  10 +
 target/arm/cpu64.c  |   1 +
 target/arm/helper.c | 371 
 5 files changed, 555 insertions(+), 3 deletions(-)

-- 
2.26.2

[PATCH 1/2] virtiofsd: Allow use "-o xattrmap" without "-o xattr"

[Carlos Venegas]

When -o xattrmap is used, it will not work unless xattr is enabled.

This patch enables xattr when -o xattrmap is used.

Signed-off-by: Carlos Venegas 
---
 tools/virtiofsd/passthrough_ll.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index ddaf57305c..2337ea5a58 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3939,6 +3939,7 @@ int main(int argc, char *argv[])
 }
 
 if (lo.xattrmap) {
+lo.xattr = 1;
 parse_xattrmap();
 }
 
-- 
2.25.1

[PATCH 2/2] virtiofsd: Add help for -o xattr-mapping

[Carlos Venegas]

The option is not documented in help.

Add small help about the option.

Signed-off-by: Carlos Venegas 
---
 tools/virtiofsd/helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index 28243b51b2..5e98ed702b 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -172,6 +172,9 @@ void fuse_cmdline_help(void)
"   default: no_writeback\n"
"-o xattr|no_xattr  enable/disable xattr\n"
"   default: no_xattr\n"
+   "-o xattrmap=  Enable xattr mapping (enables 
xattr)\n"
+   "is a string consists of a 
series of rules\n"
+   "   e.g. -o 
xattrmap=:map::user.virtiofs.:\n"
"-o modcaps=CAPLIST Modify the list of capabilities\n"
"   e.g. -o modcaps=+sys_admin:-chown\n"
"--rlimit-nofile=  set maximum number of file 
descriptors\n"
-- 
2.25.1

[PATCH 0/2] virtiofsd: Enable xattr if xattrmap is used

[Carlos Venegas]

Using xattrmap for Kata Containers we found that xattr is should be used
or xattrmap wont work.  These patches enable xattr when -o xattrmap is
used. Also, they add help for the xattrmap  option on `virtiofsd --help` output.

Carlos Venegas (2):
  virtiofsd: Allow use "-o xattrmap" without "-o xattr"
  virtiofsd: Add help for -o xattr-mapping

 tools/virtiofsd/helper.c | 3 +++
 tools/virtiofsd/passthrough_ll.c | 1 +
 2 files changed, 4 insertions(+)

-- 
2.25.1

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

[Richard Henderson]

On 4/14/21 7:59 AM, Bruno Larsen (billionai) wrote:

All the code related to gdb has been moved from translate_init.c.inc
file to the gdbstub.c file, where it makes more sense.

This new version puts the prototypes in internal.h, to not expose
them unnecessarily.

Signed-off-by: Bruno Larsen (billionai) 
Suggested-by: Fabiano Rosas 
---
  target/ppc/gdbstub.c| 258 
  target/ppc/internal.h   |   5 +
  target/ppc/translate_init.c.inc | 254 +--
  3 files changed, 264 insertions(+), 253 deletions(-)


Reviewed-by: Richard Henderson 


+void ppc_gdb_init(CPUState *cs, PowerPCCPUClass *pcc)
+{
+
+if (pcc->insns_flags & PPC_FLOAT) {


Watch the extra blank lines.


r~

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

[Bruno Piazera Larsen]

> > 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);
>
> Anymore?  You mean after you've moved out everything related to 
> create_ppc_opcodes?  Sure.

yeah, that. Also after removing every to destroy the opcode table
(which isn't packaged in a neat function for some reason, it's loose
in the ppc_cpu_unrealize).

> > * 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
>
> Well, gen_* things are specifically translation related, since they emit tcg
> opcodes.  But I see it's used as part of a callback from the SPRs.
>
> I think it would be worth moving all of the SPR code out to a separate file,
> apart from cpu_init.c.  There's a lot of it.  And, yes, I would move 
> everything
> that you can that is related out of translate.c.

Yeah, now that I look at the SPR code, I'm starting to think it's easier
I think it's what fabiano had in mind too, but we'll probably have 3 files,
spr_common.c, spr_tcg.c and spr_kvm.c. It's a bit of surgery, but it's
probably worth it, to avoid a mess of ifdefs.

> > * move opcodes and invalid_handler into cpu_init.c, because they
> > are only used by stuff in this file.
> You could move the opcodes to a new file of its own, including 
> invalid_handler.
>   Moving them to cpu_init.c does not seem helpful.

While waiting for a reply I tried this. It's really not, it creates about 6k 
errors.
I ended up moving everything that used it from cpu_init.c into translate.c.
create_ppc_opcodes and destroy_ppc_opcodes ended up going there, and
I added prototypes to internal.h to call them in the realize and unrealize
functions.

> However, I think the surgery required to disentangle the legacy decoder and 
> all
>its macros is probably not worth the effort.
> What will be worth the effort is completing the decodetree conversion so that 
> the legacy decoder goes away entirely.

Yeah, I wanted to do that, but at this point I'm just following what the client
ordered. Maybe once we compile with tcg, it could be suggested, but I
wouldn't count on it.

Anyway, I don't think the disentangling I'm doing now would make that
process harder in the future. Let me know if it is


Bruno Piazera Larsen

Instituto de Pesquisas 
ELDORADO

Departamento Computação Embarcada

Analista de Software Trainee

Aviso Legal - Disclaimer

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

[Fabiano Rosas]

Bruno Piazera Larsen  writes:

>> > * 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.
>
> so we'd also move all callbacks to translate.c, right? RN, gen_write_xer
> is only used in spr_read_xer, which is defined in cpu_init.c

Yeah, move them away from the common file into a tcg-only file.

>
> From a quick glance, this would be almost 3k lines, so bigger patches
> are incoming (side note: I tried to use that git config to show that I only
> changed file names and deal better with code motion, but it doesn't
> appear to have worked, is the wiki correct about this?)
>
>> 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);
>> }
>
> by default, KVM already doesn't use the callbacks? Or would we have to
> also change where these registers are accessed? If the first one is right
> this looks easy enough.

KVM does not use the callbacks.

>> //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);
>> ...
>> }
>
> I'm guessing we'd need to do this to all gen_spr_* functions, this is just
> an example, right?

Yeah, so that's one of the downsides of this change I proposed.

>> 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.
>
> Instead of stubbing, we could also create macros that turn the function call
> into a nop if the config was disabled, and add "if kvm_enabled()" and
> "if tcg_enabled()" if needed. I don't see how TCG and KVM being mutually
> exclusive makes this easier, unless it has to do with where they are
> accessed (idk yet where that is).

If they were mutually exclusive we could solve most problems by having
the same signature for a function and compiling one or the other
depending on the config.

That would mean we would be able to move the whole gen_spr_* functions
to the accel-specific files. So:

//tcg.c
static void gen_spr_generic(CPUPPCState *env)
{
spr_register(env, SPR_FOO, "FOO", 0xf00, _foo, _foo);
spr_register(env, SPR_BAR, "BAR", 0xb4d, _bar, _bar);
}

//kvm.c
static void gen_spr_generic(CPUPPCState *env)
{
spr_register(env, SPR_FOO, "FOO", 0xf00, KVM_REG_FOO);
spr_register(env, SPR_BAR, "BAR", 0xb4d, KVM_REG_BAR);
}

//common.c
init_ppc_proc()
{
...
gen_spr_generic(env);
...
}

But we can't do this because we want to have a QEMU binary that supports
both accel types in certain scenarios.

>
> Another option is the solution I prototyped in [PATCH 2/4] in arch_dump.c,
> having ifdef encapsulating kvm and tcg calls, and if/else blocks. I'm also
> open to suggestions on how to do it better (:
>
>> > * 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.
>
> ok. But is it worth doing for the first cut?

I think it is. I don't see the issue. Aside from the opcodes destructor
you'll just move a chunk of code over. We do want cpu_init.c to be the
common file, right? So it cannot have TCG-only code in it. Better do it
now while we're (mostly) just moving code around.

>
> Also, looking now, I see definition for exception vectors and some
> exception handling code in it, which I'm not 100% sure what to do
> with.

These are tricky because there's some logic for 

[PATCH 2/2] util/async: print leaked BH name when AioContext finalizes

[Stefan Hajnoczi]

BHs must be deleted before the AioContext is finalized. If not, it's a
bug and probably indicates that some part of the program still expects
the BH to run in the future. That can lead to memory leaks, inconsistent
state, or just hangs.

Unfortunately the assert(flags & BH_DELETED) call in aio_ctx_finalize()
is difficult to debug because the assertion failure contains no
information about the BH!

Use the QEMUBH name field added in the previous patch to show a useful
error when a leaked BH is detected.

Suggested-by: Eric Ernst 
Signed-off-by: Stefan Hajnoczi 
---
 util/async.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/util/async.c b/util/async.c
index b6acb86520..2584fca249 100644
--- a/util/async.c
+++ b/util/async.c
@@ -344,8 +344,20 @@ aio_ctx_finalize(GSource *source)
 assert(QSIMPLEQ_EMPTY(>bh_slice_list));
 
 while ((bh = aio_bh_dequeue(>bh_list, ))) {
-/* qemu_bh_delete() must have been called on BHs in this AioContext */
-assert(flags & BH_DELETED);
+/*
+ * qemu_bh_delete() must have been called on BHs in this AioContext. In
+ * many cases memory leaks, hangs, or inconsistent state occur when a
+ * BH is leaked because something still expects it to run.
+ *
+ * If you hit this, fix the lifecycle of the BH so that
+ * qemu_bh_delete() and any associated cleanup is called before the
+ * AioContext is finalized.
+ */
+if (unlikely(!(flags & BH_DELETED))) {
+fprintf(stderr, "%s: BH '%s' leaked, aborting...\n",
+__func__, bh->name);
+abort();
+}
 
 g_free(bh);
 }
-- 
2.30.2

[PATCH 1/2] util/async: add a human-readable name to BHs for debugging

[Stefan Hajnoczi]

It can be difficult to debug issues with BHs in production environments.
Although BHs can usually be identified by looking up their ->cb()
function pointer, this requires debug information for the program. It is
also not possible to print human-readable diagnostics about BHs because
they have no identifier.

This patch adds a name to each BH. The name is not unique per instance
but differentiates between cb() functions, which is usually enough. It's
done by changing aio_bh_new() and friends to macros that stringify cb.

The next patch will use the name field when reporting leaked BHs.

Signed-off-by: Stefan Hajnoczi 
---
 include/block/aio.h| 31 ---
 include/qemu/main-loop.h   |  4 +++-
 tests/unit/ptimer-test-stubs.c |  2 +-
 util/async.c   |  9 +++--
 util/main-loop.c   |  4 ++--
 5 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/include/block/aio.h b/include/block/aio.h
index 5f342267d5..499668fef5 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -291,20 +291,45 @@ void aio_context_acquire(AioContext *ctx);
 /* Relinquish ownership of the AioContext. */
 void aio_context_release(AioContext *ctx);
 
+/**
+ * aio_bh_schedule_oneshot_full: Allocate a new bottom half structure that will
+ * run only once and as soon as possible.
+ *
+ * @name: A human-readable identifier for debugging purposes.
+ */
+void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, void 
*opaque,
+  const char *name);
+
 /**
  * aio_bh_schedule_oneshot: Allocate a new bottom half structure that will run
  * only once and as soon as possible.
+ *
+ * A convenience wrapper for aio_bh_schedule_oneshot_full() that uses cb as the
+ * name string.
  */
-void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+#define aio_bh_schedule_oneshot(ctx, cb, opaque) \
+aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb)))
 
 /**
- * aio_bh_new: Allocate a new bottom half structure.
+ * aio_bh_new_full: Allocate a new bottom half structure.
  *
  * Bottom halves are lightweight callbacks whose invocation is guaranteed
  * to be wait-free, thread-safe and signal-safe.  The #QEMUBH structure
  * is opaque and must be allocated prior to its use.
+ *
+ * @name: A human-readable identifier for debugging purposes.
  */
-QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
+const char *name);
+
+/**
+ * aio_bh_new: Allocate a new bottom half structure
+ *
+ * A convenience wrapper for aio_bh_new_full() that uses the cb as the name
+ * string.
+ */
+#define aio_bh_new(ctx, cb, opaque) \
+aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb)))
 
 /**
  * aio_notify: Force processing of pending events.
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index d6892fd208..c7e8a21b5d 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -312,7 +312,9 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
 
 void qemu_fd_register(int fd);
 
-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
+#define qemu_bh_new(cb, opaque) \
+qemu_bh_new_full((cb), (opaque), (stringify(cb)))
+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name);
 void qemu_bh_schedule_idle(QEMUBH *bh);
 
 enum {
diff --git a/tests/unit/ptimer-test-stubs.c b/tests/unit/ptimer-test-stubs.c
index 7f801a4d09..2a3ef58799 100644
--- a/tests/unit/ptimer-test-stubs.c
+++ b/tests/unit/ptimer-test-stubs.c
@@ -108,7 +108,7 @@ int64_t qemu_clock_deadline_ns_all(QEMUClockType type, int 
attr_mask)
 return deadline;
 }
 
-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
+QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name)
 {
 QEMUBH *bh = g_new(QEMUBH, 1);
 
diff --git a/util/async.c b/util/async.c
index 674dbefb7c..b6acb86520 100644
--- a/util/async.c
+++ b/util/async.c
@@ -57,6 +57,7 @@ enum {
 
 struct QEMUBH {
 AioContext *ctx;
+const char *name;
 QEMUBHFunc *cb;
 void *opaque;
 QSLIST_ENTRY(QEMUBH) next;
@@ -107,7 +108,8 @@ static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags)
 return bh;
 }
 
-void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
+void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb,
+  void *opaque, const char *name)
 {
 QEMUBH *bh;
 bh = g_new(QEMUBH, 1);
@@ -115,11 +117,13 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc 
*cb, void *opaque)
 .ctx = ctx,
 .cb = cb,
 .opaque = opaque,
+.name = name,
 };
 aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT);
 }
 
-QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
+QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
+const char *name)
 {
 

[PATCH 0/2] util/async: print leaked BH name when AioContext finalizes

[Stefan Hajnoczi]

Eric Ernst and I debugged a BH leak and it was more involved than it should be.
The problem is that BHs don't have a human-readable identifier, so low-level
debugging techniques and inferences about the code are required to figure out
which BH was leaked in production environments without easy debug access.

The leak ended up already being fixed upstream but let's improve diagnostics
for leaked BHs so that this becomes quick and easy in the future.

Stefan Hajnoczi (2):
  util/async: add a human-readable name to BHs for debugging
  util/async: print leaked BH name when AioContext finalizes

 include/block/aio.h| 31 ---
 include/qemu/main-loop.h   |  4 +++-
 tests/unit/ptimer-test-stubs.c |  2 +-
 util/async.c   | 25 +
 util/main-loop.c   |  4 ++--
 5 files changed, 55 insertions(+), 11 deletions(-)

-- 
2.30.2

Converting QEMU .raw to VMDK VMware

[Terrance Battle]

Hi,

I have a question, how do I go about converting a .raw snapshot to VMware VMDK? 
We're looking to move the .raw snapshot to  our new VMware environment for 
DevOps.

Thanks,

BlackSalt Technology Group
Terrance Battle | CEO
tbat...@thebstgroup.com
202.579.7334 (mobile)
www.thebstgroup.com

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

[Richard Henderson]

On 4/13/21 2:11 PM, Luis Pires wrote:

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(-)


Reviewed-by: Richard Henderson 



+if self.width is not None:
+if extracted < self.width:


Is it too ugly to use AND here?


r~

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

[Ed Davison]

The patch may be a bit beyond me at the moment as I use a package to
install this and would have to figure out how to download source, get it
configure, patched and compiled.  Whew!  Maybe ...

But here is my XML config file.

** Attachment added: "domain xml file"
   
https://bugs.launchpad.net/qemu/+bug/1923497/+attachment/5487970/+files/win10-virt-domain.xml

-- 
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

Re: [PATCH v3 3/3] accel/tcg: Assert that tb->size != 0 after translation

[Richard Henderson]

On 4/14/21 11:03 AM, Max Filippov wrote:

On Wed, Apr 14, 2021 at 9:51 AM Ilya Leoshkevich  wrote:

On Wed, 2021-04-14 at 16:48 +0200, David Hildenbrand wrote:

Did you double-check the xtensa issue?


Oh, I'm sorry, I completely forgot about that one. I just ran the
test locally, and apparently it fails because of this new assert, so
I'll have to write the 4th patch now. Thanks!


Just curious, what xtensa issue?


Returning from xtensa_tr_translate_insn with tb->size == 0.

Basically, dc->base.pc_next needs to be incremented even for illegal 
instructions, preferably by the number of bytes consumed while determining that 
the insn is illegal.



r~

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

[Richard Henderson]

On 4/13/21 10:43 AM, Bruno Piazera Larsen wrote:

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);


Anymore?  You mean after you've moved out everything related to 
create_ppc_opcodes?  Sure.



* 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


Well, gen_* things are specifically translation related, since they emit tcg 
opcodes.  But I see it's used as part of a callback from the SPRs.


I think it would be worth moving all of the SPR code out to a separate file, 
apart from cpu_init.c.  There's a lot of it.  And, yes, I would move everything 
that you can that is related out of translate.c.



* 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


Yes.


* move opcodes and invalid_handler into cpu_init.c, because they
are only used by stuff in this file.


You could move the opcodes to a new file of its own, including invalid_handler. 
 Moving them to cpu_init.c does not seem helpful.


However, I think the surgery required to disentangle the legacy decoder and all 
its macros is probably not worth the effort.  What will be worth the effort is 
completing the decodetree conversion so that the legacy decoder goes away entirely.



r~

Re: [PATCH RFC 0/7] RFC: Asynchronous QMP Draft

[John Snow]

First and foremost, thank you for reviewing this! It is very helpful to 
me to see what others think of this pet project I've been growing in the 
privacy of my own mind.


On 4/14/21 2:38 AM, Stefan Hajnoczi wrote:

Below are the API docs that I found helpful for understanding the big
picture.

The QMP.execute() API is nice.



Yes. It mimics (sync) qmp.command(), which I believe Eduardo Habkost 
wrote. I think it's the correct idea for a generic (QAPI-schema 
ignorant) QMP client library meant to be "used".


I think raising RPC in-band execution errors as exceptions is a nice 
"pythonic" way to do it.


(And, if desired, it is possible to use the QAPI generator to generate 
wrappers around this interface using type-safe arguments in a low-level 
SDK layer. I think that would be pretty swell. We are not there yet, 
though, and I'll focus on this layer first.)



Regarding QMP events, I can think of two approaches:
1. Callbacks
2. An async get_event(name=Optional[str]) -> object API
(plus get_event_nowait(name=Optional[str]) -> object)

(There's probably a third approach using async iterators but it's
similar to get_event().)

Both approaches are useful. The first is good in larger asynchronous
applications that perform many tasks concurrently. The second is good
when there is just one specific thing to do, like waiting for a block
job to complete.


(1) On callbacks:

Callbacks are what I meagerly mocked up; discord.py has a "cute" little 
hack that works like this:


bot = commands.bot(...)

@bot.event
async def on_ready():
print("Logged in as")
print(bot.user.name)
...

(See 
https://github.com/Rapptz/discord.py/blob/master/examples/basic_bot.py )


I find this to be extremely cute: the framework uses the name of the 
callback to determine which event you are registering, and uses the 
decorator to merely register the callback.


This makes a nice, uncomplicated way to plug coroutines into the state 
machine of the client loop in the most basic cases.


I thought it might be nice to try and mimic that design, by perhaps 
using the names of QMP events as their own 'queues', and then 
dispatching user callbacks as desired. (Possibly with one mega-queue 
that exists for ALL callbacks.)


For instance, something like this:

@qmp.event
async def job_status_block_job_ready(qmp, event):
...

or more generally,

@qmp.event_handler
async def my_own_event_handler(qmp, event):
...

I didn't spend much time on the actual queue or dispatch mechanism in my 
draft, though, but it could be "bolstered" into a more full-fledged API 
if desired.


One nice thing about this design is that events aren't "consumed" by a 
caller, they are just dispatched to anyone waiting on an event of that type.


As I recall, events getting "eaten" at the wrong time was a major burden 
when writing iotests that exercised multiple jobs, transactions, etc.


(A side note: a higher-level VM class that uses QMP may wish to capture 
certain events to record state changes, such that the state can be 
queried at an arbitrary point by any number of callers without needing 
to have witnessed the state change event personally. That's not really 
important here in the protocol library, though, which will pretend not 
to know which events exist -- but it's a consideration for making sure 
the design that IS chosen can be extensible to support that kind of thing.)



(2) On get_event or async iterators:

This is likely a good ad-hoc feature. Should it only work for events 
that are delivered from that moment in time, or should there be a 
"backlog" of events to deliver?


Should waiting on events in this manner "consume" the event from the 
backlog, if we have one?


My concern::

  await qmp.execute('blockdev-backup', {...etc...})
  async for event in qmp.get_events():
  ...


It's possible that an event we'd like to see has already occurred by the 
time we get around to invoking the async iterator. You'd really want to 
start checking for events *before* you issue the job request, but that 
involves tasks, and the code doesn't "flow" well anymore.


I don't have ideas, at-present, for how to make things like iotests 
"flow" well in a linear co-routine sense...


...although, maybe it's worth creating something like an Event Listener 
object that, from its creation, stashes events from that point forward. 
How about::


  async with qmp.event_listener() as events:
  await qmp.execute('blockdev-backup', {...})
  async for event in events:
  ...

Actually, that seems pretty cool. What do you think? I think it's fairly 
elegant for ad-hoc use. We could even extend the constructor to accept 
filtering criteria if we wanted to, later.


Possibly we could also augment the Event Listener object to support a 
few methods to facilitate blocking until a certain event occurs, like::


  async with qmp.event_listener() as events:
  await qmp.execute('blockdev-backup', {...})
  await 

tidying up osdep.h

[Peter Maydell]

(cc'ing people related to the recent 'extern "C"' patches and also
randomly Markus as somebody who's had opinions on header cleanups
in the past...)

osdep.h as it stands today is a mix of two things:
 (1) it has the "must be included by everybody" items:
   (a) config-host.h, poison.h, compiler.h
   (b) things which must be done before any system header is included
   (like defining __STDC_CONSTANT_MACROS or WIN32_LEAN_AND_MEAN)
   (c) includes of system headers which we need to then fix up for
   portability issues (eg redefining assert on mingw, defining
   fallback versions of missing macros)
 (2) it has declarations for a library of QEMU functions, some of which
 typically wrap and abstract away OS specifics (like qemu_create(),
 qemu_unlink()), and some of which seem to have just been dumped
 in here for convenience (like qemu_hw_version())

Every file needs (1), which is why we mandate osdep.h as the first
include; most files don't need a lot of the things in (2) (for instance
qemu_hw_version() is used in just half a dozen .c files). Is it worth
trying to split some of the type (2) items out into their own header files?

I suspect that the advantages would be primarily just making osdep.h
a bit clearer to read and less of an "attractive nuisance" for new
additions; I imagine the bulk of the extra compilation time represented
by osdep.h is going to be because it pulls in dozens of system
headers, most of which are going to be required under heading (1).

thanks
-- PMM

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

[Richard Henderson]

On 4/13/21 2:11 PM, Luis Pires wrote:

+++ b/target/ppc/translate/fixedpoint-impl.c.inc
@@ -0,0 +1,26 @@


Missing copyright+license header.


+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);
+}


Just a note about decodetree: this kind of thing is where you would use the 
same name for both patterns, so that you would not need to have a separate 
symbol for addi (or vice versa).


That said, I'm now having a bit of a read-up on power10, and I have some 
suggestions.


First, type 2 and type 3 prefixes modify existing instructions.  Which means 
that you are going to wind up with a lot of duplication.  Preferentially, we 
should avoid that.


One example of how to approach this is target/microblaze, which has an "imm" 
instruction prefix to extend a 16-bit immediate to a 32-bit immediate.  This 
can be worked into decodetree directly:


# Include any IMM prefix in the value reported.
%extimm 0:s16 !function=typeb_imm
@typeb  .. rd:5 ra:5  \
 imm=%extimm

static int typeb_imm(DisasContext *dc, int x)
{
if (dc->tb_flags & IMM_FLAG) {
return deposit32(dc->ext_imm, 0, 16, x);
}
return x;
}

static bool trans_imm(DisasContext *dc, arg_imm *arg)
{
if (invalid_delay_slot(dc, "imm")) {
return true;
}
dc->ext_imm = arg->imm << 16;
tcg_gen_movi_i32(cpu_imm, dc->ext_imm);
dc->tb_flags_to_set = IMM_FLAG;
return true;
}

We decode "imm" as a separate instruction, set some bits in DisasContext, and 
then use those bits while decoding the next instruction.


I think the exact mechanism that microblaze uses is going to be too simplistic 
for Power10, but the basic idea of modifying the "normal" instructions is still 
sound, I think.


Using addi+paddi as an example, what about

# All ppc formats have names -- use them.
%MLSr ie
prefix_MLS  01 10 -- r:1 -- ie:s18  

# TODO: decodetree extension -- allow :type after name.
# The SI field needs to be 64-bit for MLS:D-form.
%D  rt ra si:int64_t
@D  .. rt:5 ra:5 si:s16

ADDI001110 . .  @D


static bool
trans_prefix_MLS(DisasContext *ctx, arg_MLS *a)
{
if (cpu does not support prefixes ||
ctx->prefix_type != PREFIX_NONE) {
return false;
}
/* Record the prefix for the next instruction. */
ctx->prefix_type = PREFIX_MLS;
ctx->prefix_data.mls = *a;
return true;
}

static bool
allow_prefix_MLS(DisasContext *ctx, arg_D *a)
{
int64_t imm;

/* Require MLS prefix or no prefix. */
if (ctx->prefix_type != PREFIX_MLS) {
if (ctx->prefix_type == PREFIX_NONE) {
return true;
}
gen_invalid(ctx);
return false;
}

/*
 * Concatenate the two immediate fields.
 * Note that IE is sign-extended 18 bits,
 * so this forms a signed 34-bit constant.
 */
imm = deposit64(a->si, 16, 48, ctx->prefix_data.mls.ie);

/*
 * If R, then the constant is pc-relative,
 * and RA must be 0.
 */
if (ctx->prefix_data.mls.r) {
if (ctx->prefix_data.mls.ra != 0) {
gen_invalid(ctx);
return false;
}
imm += ctx->cia;
}
a->si = imm;
return true;
}

static bool
trans_ADDI(DisasContext *ctx, arg_D *a)
{
if (!allow_prefix_MLS(ctx, a)) {
return true;
}
if (a->ra) {
tcg_gen_addi_tl(cpu_gpr[a->rt],
cpu_gpr[a->ra], a->si);
} else {
tcg_gen_movi_tl(cpu_gpr[a->rt], a->si);
}
return true;
}

This approach seems like it will work fine for MLS and MMIR prefixes.  For 8LS, 
8RR, and MRR prefixes, we'll need some extra help within ppc_tr_translate_insn. 
 E.g.


insn = translator_ldl_swap(env, ctx->base.pc_next,
   need_byteswap(ctx));
switch (ctx->prefix_type) {
case PREFIX_NONE:
ok = decode_opcode_space_0(ctx, insn) ||
 decode_legacy(ctx, insn);
break;
case PREFIX_MLS:
case PREFIX_MMIRR:
ok = decode_opcode_space_0(ctx, insn);
break;
case PREFIX_8LS:
case PREFIX_8RR:
ok = decode_opcode_space_1(ctx, insn);
break;
case PREFIX_MRR:
/*
 * The only instruction with this prefix is PNOP.
 * TODO: diagnose the set of patterns that are illegal:
 * branches, rfebb, sync other than 

Re: [PATCH v5 12/14] hmp: Print "share" property of memory backends with "info memdev"

[Dr. David Alan Gilbert]

* David Hildenbrand (da...@redhat.com) wrote:
> Let's print the property.
> 
> Cc: Markus Armbruster 
> Cc: Eric Blake 
> Cc: Igor Mammedov 
> Signed-off-by: David Hildenbrand 

Reviewed-by: Dr. David Alan Gilbert 

> ---
>  hw/core/machine-hmp-cmds.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
> index 58248cffa3..004a92b3d6 100644
> --- a/hw/core/machine-hmp-cmds.c
> +++ b/hw/core/machine-hmp-cmds.c
> @@ -110,6 +110,8 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
> m->value->dump ? "true" : "false");
>  monitor_printf(mon, "  prealloc: %s\n",
> m->value->prealloc ? "true" : "false");
> +monitor_printf(mon, "  share: %s\n",
> +   m->value->share ? "true" : "false");
>  monitor_printf(mon, "  policy: %s\n",
> HostMemPolicy_str(m->value->policy));
>  visit_complete(v, );
> -- 
> 2.30.2
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

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

[Peter Maydell]

On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé  wrote:
>
> On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote:
> >  #ifdef _WIN32
> >  #include "sysemu/os-win32.h"
>
> This and os-posix.h both include other system headers. We don't currently
> have problem, so this is ok as the minimal fix for 6.0, but long term we
> need more work on this header to further narrow the extern {} block.

Maybe we should just move all the system header includes out of
both os-posix.h and os-win32.h ? We already have one header file
we've treated that way (sys/wait.h).

Alternatively we could leave os-win32.h and os-posix.h outside
osdep.h's extern block, and require that they both use an
extern block themselves for their declarations.

thanks
-- PMM

Re: [RFC PATCH 0/5] mptcp support

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berra...@redhat.com) wrote:
> On Mon, Apr 12, 2021 at 03:51:10PM +0100, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berra...@redhat.com) wrote:
> > > On Thu, Apr 08, 2021 at 08:11:54PM +0100, Dr. David Alan Gilbert (git) 
> > > wrote:
> > > > From: "Dr. David Alan Gilbert" 
> > > > 
> > > > Hi,
> > > >   This RFC set adds support for multipath TCP (mptcp),
> > > > in particular on the migration path - but should be extensible
> > > > to other users.
> > > > 
> > > >   Multipath-tcp is a bit like bonding, but at L3; you can use
> > > > it to handle failure, but can also use it to split traffic across
> > > > multiple interfaces.
> > > > 
> > > >   Using a pair of 10Gb interfaces, I've managed to get 19Gbps
> > > > (with the only tuning being using huge pages and turning the MTU up).
> > > > 
> > > >   It needs a bleeding-edge Linux kernel (in some older ones you get
> > > > false accept messages for the subflows), and a C lib that has the
> > > > constants defined (as current glibc does).
> > > > 
> > > >   To use it you just need to append ,mptcp to an address;
> > > > 
> > > >   -incoming tcp:0:,mptcp
> > > >   migrate -d tcp:192.168.11.20:,mptcp
> > > 
> > > What happens if you only enable mptcp flag on one side of the
> > > stream (whether client or server), does it degrade to boring
> > > old single path TCP, or does it result in an error ?
> > 
> > I've just tested this and it matches what pabeni said; it seems to just
> > fall back.
> > 
> > > >   I had a quick go at trying NBD as well, but I think it needs
> > > > some work with the parsing of NBD addresses.
> > > 
> > > In theory this is applicable to anywhere that we use sockets.
> > > Anywhere that is configured with the QAPI  SocketAddress /
> > > SocketAddressLegacy type will get it for free AFAICT.
> > 
> > That was my hope.
> > 
> > > Anywhere that is configured via QemuOpts will need an enhancement.
> > > 
> > > IOW, I would think NBD already works if you configure NBD via
> > > QMP with nbd-server-start, or block-export-add.  qemu-nbd will
> > > need cli options added.
> > > 
> > > The block layer clients for NBD, Gluster, Sheepdog and SSH also
> > > all get it for free when configured va QMP, or -blockdev AFAICT
> > 
> > Have you got some examples via QMP?
> > I'd failed trying -drive 
> > if=virtio,file=nbd://192.168.11.20:,mptcp=on/zero
> 
> I never remember the mapping to blockdev QAPI schema, especially
> when using legacy filename syntax with the URI.
> 
> Try instead
> 
>  -blockdev driver=nbd,host=192.168.11.20,port=,mptcp=on,id=disk0backend
>  -device virtio-blk,drive=disk0backend,id=disk0

That doesn't look like the right syntax, but it got me closer; and it's
working with no more code changes:

On the source:

qemu... -nographic -M none -drive if=none,file=my.qcow2,id=mydisk
(qemu) nbd_server_start 0.0.0.0:,mptcp=on
(qemu) nbd_server_add -w mydisk

On the destination:
-blockdev 
driver=nbd,server.type=inet,server.host=192.168.11.20,server.port=,server.mptcp=on,node-name=nbddisk,export=mydisk
 -device virtio-blk,drive=nbddisk,id=disk0

and it succesfully booted off it, and it looks like it has two flows.
(It didn't get that great a bandwidth, but I'm not sure where that's due
to).

Dave
> 
> 
> 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 :|
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[PATCH] include/qemu/osdep.h: Move system includes to top

[Peter Maydell]

Mostly osdep.h puts the system includes at the top of the file; but
there are a couple of exceptions where we include a system header
halfway through the file.  Move these up to the top with the rest
so that all the system headers we include are included before
we include os-win32.h or os-posix.h.

Signed-off-by: Peter Maydell 
---
 include/qemu/osdep.h | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ba15be9c569..38c96c72db2 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -104,6 +104,15 @@ extern int daemon(int, int);
 #include 
 #include 
 
+#ifdef CONFIG_IOVEC
+#include 
+#endif
+
+#if defined(__linux__) && defined(__sparc__)
+/* The SPARC definition of QEMU_VMALLOC_ALIGN needs SHMLBA */
+#include 
+#endif
+
 #ifndef _WIN32
 #include 
 #else
@@ -111,6 +120,10 @@ extern int daemon(int, int);
 #define WEXITSTATUS(x) (x)
 #endif
 
+#ifdef __APPLE__
+#include 
+#endif
+
 #ifdef _WIN32
 #include "sysemu/os-win32.h"
 #endif
@@ -119,10 +132,6 @@ extern int daemon(int, int);
 #include "sysemu/os-posix.h"
 #endif
 
-#ifdef __APPLE__
-#include 
-#endif
-
 #include "glib-compat.h"
 #include "qemu/typedefs.h"
 
@@ -459,7 +468,6 @@ void qemu_anon_ram_free(void *ptr, size_t size);
/* Use 1 MiB (segment size) alignment so gmap can be used by KVM. */
 #  define QEMU_VMALLOC_ALIGN (256 * 4096)
 #elif defined(__linux__) && defined(__sparc__)
-#include 
 #  define QEMU_VMALLOC_ALIGN MAX(qemu_real_host_page_size, SHMLBA)
 #else
 #  define QEMU_VMALLOC_ALIGN qemu_real_host_page_size
@@ -539,8 +547,6 @@ struct iovec {
 
 ssize_t readv(int fd, const struct iovec *iov, int iov_cnt);
 ssize_t writev(int fd, const struct iovec *iov, int iov_cnt);
-#else
-#include 
 #endif
 
 #ifdef _WIN32
-- 
2.20.1

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

[Peter Maydell]

On Wed, 14 Apr 2021 at 18:26, Daniel P. Berrangé  wrote:
>
> On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote:
> > 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"
> >  }

> This and os-posix.h both include other system headers. We don't currently
> have problem, so this is ok as the minimal fix for 6.0, but long term we
> need more work on this header to further narrow the extern {} block.

The other path where we can include system headers inside extern "C"
is that the code above still has dis-asm.h inside the extern C block,
but dis-asm.h includes qemu/bswap.h (midway down the file!) and bswap.h
in turn includes some system headers.

thanks
-- PMM

target/ppc: sPAPR invalid function calls when compiling without TCG

[Lucas Mateus Martins Araujo e Castro]

Hi, I have been working on billionai's patch to enable the --disable-tcg option 
on PowerPC and one of the problems is that 5 files in hw/ppc use functions 
implemented in mmu-hash64.c which is not compiled with --disable-tcg, I'd like 
to know how to correct the spapr function call, should I

  *   change the calls to generic functions that should call the correct 
function based on if kvm is being used or not,
  *   should I just implement said functions independently of mmu-hash64.c and 
always call them,
  *   should I just implement said functions independently of mmu-hash64.c and 
only call it with --disable-tcg option turned on,
  *   find a way to not have said calls when compiling without TCG as they're 
not necessary,
  *   just create a stub

Here's when each function is called by each .c:
spapr.c:
function do_lpcr_sync call ppc_store_lpcr

spapr_hcall.c:
function h_enter call ppc_hash64_hpte_page_shift_noslb, 
ppc_hash64_map_hptes and ppc_hash64_unmap_hptes
function remove_hpte call ppc_hash64_map_hptes, ppc_hash64_unmap_hptes and 
ppc_hash64_tlb_flush_hpte
function h_protect call ppc_hash64_map_hptes, ppc_hash64_unmap_hptes and 
ppc_hash64_tlb_flush_hpte
function h_read call ppc_hash64_map_hptes and ppc_hash64_unmap_hptes
function rehash_hpte call ppc_hash64_hpte_page_shift_noslb
function rehash_hpt call ppc_hash64_map_hptes and ppc_hash64_unmap_hptes

spapr_rtas.c:
function rtas_start_cpu call ppc_store_lpcr
function rtas_stop_self call ppc_store_lpcr

spapr_spapr_cpu_core.c
function spapr_reset_vcpu call ppc_store_lpcr
function spapr_cpu_set_entry_state call ppc_store_lpcr

spapr_caps.c:
function cap_large_decr_cpu_apply call ppc_store_lpcr
function cap_hpt_maxpagesize_cpu_apply call ppc_hash64_filter_pagesizes

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

Given Dan's review, I think that the osdep patches need another
revision. So my plan is to cherry-pick the CONFIG_VIRTIO_CRYPTO
patch here and tag rc3 with just that. If we need an rc4 (which
on our current track record is not unlikely) we can put in some
version of the osdep patches; if not, this isn't a regression
since 5.2 so I'm happy releasing 6.0 with it still present.

thanks
-- PMM

Re: [PATCH v3 3/3] accel/tcg: Assert that tb->size != 0 after translation

[Max Filippov]

On Wed, Apr 14, 2021 at 9:51 AM Ilya Leoshkevich  wrote:
> On Wed, 2021-04-14 at 16:48 +0200, David Hildenbrand wrote:
> > Did you double-check the xtensa issue?
>
> Oh, I'm sorry, I completely forgot about that one. I just ran the
> test locally, and apparently it fails because of this new assert, so
> I'll have to write the 4th patch now. Thanks!

Just curious, what xtensa issue?

-- 
Thanks.
-- Max

Re: [PATCH RFC 6/7] qmp_protocol: add QMP client implementation

[John Snow]

On 4/14/21 1:44 AM, Stefan Hajnoczi wrote:

On Tue, Apr 13, 2021 at 11:55:52AM -0400, John Snow wrote:

+async def _execute(self, msg: Message) -> object:
+"""
+The same as `execute_msg()`, but without safety mechanisms.
+
+Does not assign an execution ID and does not check that the form
+of the message being sent is valid.
+
+This method *Requires* an 'id' parameter to be set on the
+message, it will not set one for you like `execute()` or
+`execute_msg()`.
+
+Do not use "__aqmp#0" style IDs, use something else to avoid
+potential clashes. If this ID clashes with an ID presently
+in-use or otherwise clashes with the auto-generated IDs, the
+response routing mechanisms in _on_message may very well fail
+loudly enough to cause the entire loop to crash.
+
+The ID should be a str; or at least something JSON
+serializable. It *must* be hashable.
+"""
+exec_id = cast(str, msg['id'])
+self.logger.debug("Execute(%s): '%s'", exec_id,
+  msg.get('execute', msg.get('exec-oob')))
+
+queue: asyncio.Queue[Message] = asyncio.Queue(maxsize=1)
+task = create_task(self._bh_execute(msg, queue))


We're already in a coroutine, can we await queue.get() ourselves instead
of creating a new task?

I guess this is done in order to use Task.cancel() in _bh_disconnect()
but it seems simpler to use queue both for success and cancellation.
Fewer tasks are easier to reason about.



...queues do not have a cancellation signal :( :( :( :(

There's no way to "cancel" a queue:
https://docs.python.org/3/library/asyncio-queue.html#queue

You *could* craft a special message and inject an exception into the 
queue to notify the reader that the message will never arrive, but it 
feels like working against the intended mechanism of that primitive. It 
really feels like it wants to be wrapped in a *task*.


An earlier draft used an approach where it crafted a special "mailbox" 
object, comprised of message, event, and error fields. The waiter sets 
up a mailbox and then blocks on the event. Upon being notified of an 
event, the caller checks to see if the message OR the error field was 
filled.


I wound up removing it, because I felt it added too much custom 
machinery/terminology and instead went with Tasks and a queue with a 
depth of one.


Both feel like they are working against the intended mechanisms to a 
degree. I am open to suggestions here!


(It's also worth noting that iotests will want the ability to separate 
the queueing of a message and the waiting for that message. The current 
design only allows for send-and-wait, and not separate send-then-wait 
semantics. Tasks do provide a rather convenient handle if I want to 
split that mechanism out.)


All of the above options are a little hacky to me. Any thoughts or 
preferences?


--js

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

[John Snow]

On 4/13/21 4:07 PM, Stefan Hajnoczi wrote:

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().



Mostly just to associate the de/serialization methods of the 
unit-message with that data type, and it's nice for strict typing.


It does repeat a lot of boilerplate to just re-implement the 
dict-quacking; but I think I might actually be able to get around that 
by inheriting from MutableMapping to get all of that boilerplate "for free".


I'll see. I'll put it high on the list for the chopping block.

--js

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

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> 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?

Yes - many.

> 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.


-- 
Alex Bennée

[PATCH] migration/dirtyrate: make sample page count configurable

[huangy81]

From: Hyman Huang(黄勇) 

introduce optional sample-pages argument in calc-dirty-rate,
making sample page count per GB configurable so that more
accurate dirtyrate can be calculated.

Signed-off-by: Hyman Huang(黄勇) 
---
 migration/dirtyrate.c | 32 
 migration/dirtyrate.h |  8 +++-
 qapi/migration.json   | 13 ++---
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ccb9814..1e3ef0b 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -48,6 +48,16 @@ static bool is_sample_period_valid(int64_t sec)
 return true;
 }
 
+static bool is_sample_pages_valid(int64_t sec)
+{
+if (sec < MIN_SAMPLE_PAGE_COUNT ||
+sec > MAX_SAMPLE_PAGE_COUNT) {
+return false;
+}
+
+return true;
+}
+
 static int dirtyrate_set_state(int *state, int old_state, int new_state)
 {
 assert(new_state < DIRTY_RATE_STATUS__MAX);
@@ -72,13 +82,15 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
 info->status = CalculatingState;
 info->start_time = DirtyStat.start_time;
 info->calc_time = DirtyStat.calc_time;
+info->sample_pages = DirtyStat.sample_pages;
 
 trace_query_dirty_rate_info(DirtyRateStatus_str(CalculatingState));
 
 return info;
 }
 
-static void init_dirtyrate_stat(int64_t start_time, int64_t calc_time)
+static void init_dirtyrate_stat(int64_t start_time, int64_t calc_time,
+uint64_t sample_pages)
 {
 DirtyStat.total_dirty_samples = 0;
 DirtyStat.total_sample_count = 0;
@@ -86,6 +98,7 @@ static void init_dirtyrate_stat(int64_t start_time, int64_t 
calc_time)
 DirtyStat.dirty_rate = -1;
 DirtyStat.start_time = start_time;
 DirtyStat.calc_time = calc_time;
+DirtyStat.sample_pages = sample_pages;
 }
 
 static void update_dirtyrate_stat(struct RamblockDirtyInfo *info)
@@ -361,6 +374,7 @@ void *get_dirtyrate_thread(void *arg)
 int ret;
 int64_t start_time;
 int64_t calc_time;
+uint64_t sample_pages;
 
 ret = dirtyrate_set_state(, DIRTY_RATE_STATUS_UNSTARTED,
   DIRTY_RATE_STATUS_MEASURING);
@@ -371,7 +385,8 @@ void *get_dirtyrate_thread(void *arg)
 
 start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
 calc_time = config.sample_period_seconds;
-init_dirtyrate_stat(start_time, calc_time);
+sample_pages = config.sample_pages_per_gigabytes;
+init_dirtyrate_stat(start_time, calc_time, sample_pages);
 
 calculate_dirtyrate(config);
 
@@ -383,7 +398,8 @@ void *get_dirtyrate_thread(void *arg)
 return NULL;
 }
 
-void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
+void qmp_calc_dirty_rate(int64_t calc_time, bool has_sample_pages,
+ int64_t sample_pages, Error **errp)
 {
 static struct DirtyRateConfig config;
 QemuThread thread;
@@ -404,6 +420,13 @@ void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
 return;
 }
 
+if (has_sample_pages && !is_sample_pages_valid(sample_pages)) {
+error_setg(errp, "sample-pages is out of range[%d, %d].",
+ MIN_SAMPLE_PAGE_COUNT,
+ MAX_SAMPLE_PAGE_COUNT);
+return;
+}
+
 /*
  * Init calculation state as unstarted.
  */
@@ -415,7 +438,8 @@ void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
 }
 
 config.sample_period_seconds = calc_time;
-config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+config.sample_pages_per_gigabytes =
+has_sample_pages ? sample_pages : DIRTYRATE_DEFAULT_SAMPLE_PAGES;
 qemu_thread_create(, "get_dirtyrate", get_dirtyrate_thread,
(void *), QEMU_THREAD_DETACHED);
 }
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 6ec4295..5f987e2 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -15,7 +15,6 @@
 
 /*
  * Sample 512 pages per GB as default.
- * TODO: Make it configurable.
  */
 #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
 
@@ -35,6 +34,12 @@
 #define MIN_FETCH_DIRTYRATE_TIME_SEC  1
 #define MAX_FETCH_DIRTYRATE_TIME_SEC  60
 
+/*
+ * Take 128 as minimum for sample dirty pages
+ */
+#define MIN_SAMPLE_PAGE_COUNT 128
+#define MAX_SAMPLE_PAGE_COUNT 4096
+
 struct DirtyRateConfig {
 uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
 int64_t sample_period_seconds; /* time duration between two sampling */
@@ -63,6 +68,7 @@ struct DirtyRateStat {
 int64_t dirty_rate; /* dirty rate in MB/s */
 int64_t start_time; /* calculation start time in units of second */
 int64_t calc_time; /* time duration of two sampling in units of second */
+uint64_t sample_pages; /* sample pages per GB */
 };
 
 void *get_dirtyrate_thread(void *arg);
diff --git a/qapi/migration.json b/qapi/migration.json
index 9bf0bc4..868a867 100644
--- 

[PATCH v3 14/15] qemu_iotests: add option to show qemu binary logs on stdout

[Emanuele Giuseppe Esposito]

Using the flag -p, allow the qemu binary to print to stdout.
This helps especially when doing print-debugging.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/qemu-iotests/check  | 3 ++-
 tests/qemu-iotests/iotests.py | 9 +
 tests/qemu-iotests/testenv.py | 9 +++--
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 489178d9a4..84483922eb 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -33,6 +33,7 @@ def make_argparser() -> argparse.ArgumentParser:
help='pretty print output for make check')
 
 p.add_argument('-d', dest='debug', action='store_true', help='debug')
+p.add_argument('-p', dest='print', action='store_true', help='shows qemu 
binary prints to stdout')
 p.add_argument('-gdb', action='store_true',
help="start gdbserver with $GDB_QEMU options. \
  Default is localhost:12345")
@@ -117,7 +118,7 @@ if __name__ == '__main__':
   aiomode=args.aiomode, cachemode=args.cachemode,
   imgopts=args.imgopts, misalign=args.misalign,
   debug=args.debug, valgrind=args.valgrind,
-  gdb=args.gdb)
+  gdb=args.gdb, qprint=args.print)
 
 testfinder = TestFinder(test_dir=env.source_iotests)
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f9832558a0..52ff7332f8 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -79,6 +79,8 @@
 if os.environ.get('GDB_QEMU'):
 qemu_gdb = ['gdbserver'] + os.environ.get('GDB_QEMU').strip().split(' ')
 
+qemu_print = os.environ.get('PRINT_QEMU', False)
+
 imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
 output_dir = os.environ.get('OUTPUT_DIR', '.')
@@ -621,6 +623,13 @@ def _post_shutdown(self) -> None:
 super()._post_shutdown()
 self.subprocess_check_valgrind(qemu_valgrind)
 
+def _pre_launch(self) -> None:
+super()._pre_launch()
+if qemu_print and self._qemu_log_file != None:
+# set QEMU binary output to stdout
+self._qemu_log_file.close()
+self._qemu_log_file = None
+
 def add_object(self, opts):
 self._args.append('-object')
 self._args.append(opts)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 39ae7ace33..6ae099114e 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -73,7 +73,7 @@ class TestEnv(ContextManager['TestEnv']):
  'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
  'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
  'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_',
- 'GDB_QEMU']
+ 'GDB_QEMU', 'PRINT_QEMU']
 
 def get_env(self) -> Dict[str, str]:
 env = {}
@@ -165,7 +165,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
  misalign: bool = False,
  debug: bool = False,
  valgrind: bool = False,
- gdb: bool = False) -> None:
+ gdb: bool = False,
+ qprint: bool = False) -> None:
 self.imgfmt = imgfmt
 self.imgproto = imgproto
 self.aiomode = aiomode
@@ -173,6 +174,9 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
 self.misalign = misalign
 self.debug = debug
 
+if qprint:
+self.print_qemu = 'y'
+
 if gdb:
 self.gdb_qemu = os.environ.get('GDB_QEMU', 'localhost:12345')
 elif 'GDB_QEMU' in os.environ:
@@ -278,6 +282,7 @@ def print_env(self) -> None:
 SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
 GDB_QEMU  -- "{GDB_QEMU}"
 VALGRIND_QEMU -- "{VALGRIND_QEMU}"
+PRINT_QEMU--  "{PRINT_QEMU}"
 """
 
 args = collections.defaultdict(str, self.get_env())
-- 
2.30.2

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

[John Snow]

On 4/13/21 4:00 PM, Stefan Hajnoczi wrote:

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.



I meant to say "blocks on" or "awaits". The StreamWriter waits on the 
Writer task, the Writer task waits on the outbound queue. The outbound 
queue waits (ultimately) on execute() depositing something into the 
queue, and so on.




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.



Ah, don't despair!

It's a lot of docstrings and a lot of tiny little methods and 
boilerplate. I thought it helped keep the resulting QMP-specific bits 
looking much simpler and easy to digest.


One of the reasons it's split out here like this is because I also wrote 
a qtest protocol that uses the same infrastructure. I tried to keep both 
of those looking as simple as possible.


I thought it was difficult to get the underlying machinery operating 
smoothly, and I didn't like the idea of repeating so much code to 
implement two things. So, this was my attempt to share common code as 
best as I could manage it.



  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 

[PATCH v1] migration/dirtyrate: make sample page count configurable

[huangy81]

From: Hyman Huang(黄勇) 

introduce optional sample-pages argument in calc-dirty-rate,
making sample page count per GB configurable so that more
accurate dirtyrate can be calculated.

Signed-off-by: Hyman Huang(黄勇) 
---
 migration/dirtyrate.c | 32 
 migration/dirtyrate.h |  8 +++-
 qapi/migration.json   | 13 ++---
 3 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index ccb9814..43a531c 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -48,6 +48,16 @@ static bool is_sample_period_valid(int64_t sec)
 return true;
 }
 
+static bool is_sample_pages_valid(int64_t pages)
+{
+if (pages < MIN_SAMPLE_PAGE_COUNT ||
+pages > MAX_SAMPLE_PAGE_COUNT) {
+return false;
+}
+
+return true;
+}
+
 static int dirtyrate_set_state(int *state, int old_state, int new_state)
 {
 assert(new_state < DIRTY_RATE_STATUS__MAX);
@@ -72,13 +82,15 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
 info->status = CalculatingState;
 info->start_time = DirtyStat.start_time;
 info->calc_time = DirtyStat.calc_time;
+info->sample_pages = DirtyStat.sample_pages;
 
 trace_query_dirty_rate_info(DirtyRateStatus_str(CalculatingState));
 
 return info;
 }
 
-static void init_dirtyrate_stat(int64_t start_time, int64_t calc_time)
+static void init_dirtyrate_stat(int64_t start_time, int64_t calc_time,
+uint64_t sample_pages)
 {
 DirtyStat.total_dirty_samples = 0;
 DirtyStat.total_sample_count = 0;
@@ -86,6 +98,7 @@ static void init_dirtyrate_stat(int64_t start_time, int64_t 
calc_time)
 DirtyStat.dirty_rate = -1;
 DirtyStat.start_time = start_time;
 DirtyStat.calc_time = calc_time;
+DirtyStat.sample_pages = sample_pages;
 }
 
 static void update_dirtyrate_stat(struct RamblockDirtyInfo *info)
@@ -361,6 +374,7 @@ void *get_dirtyrate_thread(void *arg)
 int ret;
 int64_t start_time;
 int64_t calc_time;
+uint64_t sample_pages;
 
 ret = dirtyrate_set_state(, DIRTY_RATE_STATUS_UNSTARTED,
   DIRTY_RATE_STATUS_MEASURING);
@@ -371,7 +385,8 @@ void *get_dirtyrate_thread(void *arg)
 
 start_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME) / 1000;
 calc_time = config.sample_period_seconds;
-init_dirtyrate_stat(start_time, calc_time);
+sample_pages = config.sample_pages_per_gigabytes;
+init_dirtyrate_stat(start_time, calc_time, sample_pages);
 
 calculate_dirtyrate(config);
 
@@ -383,7 +398,8 @@ void *get_dirtyrate_thread(void *arg)
 return NULL;
 }
 
-void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
+void qmp_calc_dirty_rate(int64_t calc_time, bool has_sample_pages,
+ int64_t sample_pages, Error **errp)
 {
 static struct DirtyRateConfig config;
 QemuThread thread;
@@ -404,6 +420,13 @@ void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
 return;
 }
 
+if (has_sample_pages && !is_sample_pages_valid(sample_pages)) {
+error_setg(errp, "sample-pages is out of range[%d, %d].",
+ MIN_SAMPLE_PAGE_COUNT,
+ MAX_SAMPLE_PAGE_COUNT);
+return;
+}
+
 /*
  * Init calculation state as unstarted.
  */
@@ -415,7 +438,8 @@ void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
 }
 
 config.sample_period_seconds = calc_time;
-config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+config.sample_pages_per_gigabytes =
+has_sample_pages ? sample_pages : DIRTYRATE_DEFAULT_SAMPLE_PAGES;
 qemu_thread_create(, "get_dirtyrate", get_dirtyrate_thread,
(void *), QEMU_THREAD_DETACHED);
 }
diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
index 6ec4295..5f987e2 100644
--- a/migration/dirtyrate.h
+++ b/migration/dirtyrate.h
@@ -15,7 +15,6 @@
 
 /*
  * Sample 512 pages per GB as default.
- * TODO: Make it configurable.
  */
 #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
 
@@ -35,6 +34,12 @@
 #define MIN_FETCH_DIRTYRATE_TIME_SEC  1
 #define MAX_FETCH_DIRTYRATE_TIME_SEC  60
 
+/*
+ * Take 128 as minimum for sample dirty pages
+ */
+#define MIN_SAMPLE_PAGE_COUNT 128
+#define MAX_SAMPLE_PAGE_COUNT 4096
+
 struct DirtyRateConfig {
 uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
 int64_t sample_period_seconds; /* time duration between two sampling */
@@ -63,6 +68,7 @@ struct DirtyRateStat {
 int64_t dirty_rate; /* dirty rate in MB/s */
 int64_t start_time; /* calculation start time in units of second */
 int64_t calc_time; /* time duration of two sampling in units of second */
+uint64_t sample_pages; /* sample pages per GB */
 };
 
 void *get_dirtyrate_thread(void *arg);
diff --git a/qapi/migration.json b/qapi/migration.json
index 9bf0bc4..868a867 100644
--- 

[PATCH v3 10/15] qemu_iotests: extent QMP socket timeout when using valgrind

[Emanuele Giuseppe Esposito]

As with gdbserver, valgrind delays the test execution, so
the default QMP socket timeout timeout too soon.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 python/qemu/machine.py| 2 +-
 tests/qemu-iotests/iotests.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index d6142271c2..dce96e1858 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -410,7 +410,7 @@ def _launch(self) -> None:
shell=False,
close_fds=False)
 
-if 'gdbserver' in self._wrapper:
+if 'gdbserver' in self._wrapper or 'valgrind' in self._wrapper:
 self._qmp_timer = None
 self._post_launch()
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index a2e8604674..94597433fa 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -489,7 +489,7 @@ def log(msg: Msg,
 
 class Timeout:
 def __init__(self, seconds, errmsg="Timeout"):
-if qemu_gdb:
+if qemu_gdb or qemu_valgrind:
 self.seconds = 3000
 else:
 self.seconds = seconds
@@ -700,7 +700,7 @@ def qmp_to_opts(self, obj):
 return ','.join(output_list)
 
 def get_qmp_events(self, wait: bool = False) -> List[QMPMessage]:
-if qemu_gdb:
+if qemu_gdb or qemu_valgrind:
 wait = 0.0
 return super().get_qmp_events(wait=wait)
 
-- 
2.30.2

[PATCH v3 07/15] qemu-iotests: add gdbserver option to script tests too

[Emanuele Giuseppe Esposito]

The only limitation here is that running a script with gdbserver
will make the test output mismatch with the expected
results, making the test fail.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/qemu-iotests/common.rc | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 65cdba5723..53a3310fee 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -166,8 +166,14 @@ _qemu_wrapper()
 if [ -n "${QEMU_NEED_PID}" ]; then
 echo $BASHPID > "${QEMU_TEST_DIR}/qemu-${_QEMU_HANDLE}.pid"
 fi
+
+GDB="${QEMU_PROG}"
+if [ ! -z ${GDB_QEMU} ]; then
+GDB="gdbserver ${GDB_QEMU} ${GDB}"
+fi
+
 VALGRIND_QEMU="${VALGRIND_QEMU_VM}" _qemu_proc_exec 
"${VALGRIND_LOGFILE}" \
-"$QEMU_PROG" $QEMU_OPTIONS "$@"
+   $GDB $QEMU_OPTIONS "$@"
 )
 RETVAL=$?
 _qemu_proc_valgrind_log "${VALGRIND_LOGFILE}" $RETVAL
-- 
2.30.2

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

[Daniel P . Berrangé]

On Tue, Apr 13, 2021 at 06:08:49PM +0200, Paolo Bonzini wrote:
> 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"
>  }

disas/arm-a64.c  also has an 'extern "C"' block around
an include of qemu/osdep.h.   Do we need a similar
fix for that file, or are we no longer using that
bit of code ?

>  
> 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"

This and os-posix.h both include other system headers. We don't currently
have problem, so this is ok as the minimal fix for 6.0, but long term we
need more work on this header to further narrow the extern {} block.

So assuming my question about disas/arm-a64.c is a non-issue, then


Reviewed-by: Daniel P. Berrangé 


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 :|

[PATCH v3 06/15] qemu_iotests: insert gdbserver command line as wrapper for qemu binary

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/qemu-iotests/iotests.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 380527245e..4f3fb13915 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -582,7 +582,8 @@ class VM(qtest.QEMUQtestMachine):
 
 def __init__(self, path_suffix=''):
 name = "qemu%s-%d" % (path_suffix, os.getpid())
-super().__init__(qemu_prog, qemu_opts, name=name,
+super().__init__(qemu_prog, qemu_opts, wrapper=qemu_gdb,
+ name=name,
  test_dir=test_dir,
  socket_scm_helper=socket_scm_helper,
  sock_dir=sock_dir)
-- 
2.30.2

[PATCH v3 15/15] docs/devel/testing: add -p option to the debug section of QEMU iotests

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito 
---
 docs/devel/testing.rst | 4 
 1 file changed, 4 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 62902cfd2d..0c18fc4571 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -246,6 +246,10 @@ given as options to the ``check`` script:
 * ``-d`` (debug) just increases the logging verbosity, showing
   for example the QMP commands and answers.
 
+* ``-p`` (print) allows QEMU binary stdout to be shown in the
+  test console, instead of saving it into a log file in
+  ``$TEST_DIR/qemu-machine-``.
+
 Test case groups
 
 
-- 
2.30.2

[PATCH v3 11/15] qemu_iotests: allow valgrind to read/delete the generated log file

[Emanuele Giuseppe Esposito]

When using -valgrind on the script tests, it generates a log file
in $TEST_DIR that is either read (if valgrind finds problems) or
otherwise deleted. Provide the same exact behavior when using
-valgrind on the python tests.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/qemu-iotests/iotests.py | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 94597433fa..aef67e3a86 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -600,6 +600,26 @@ def __init__(self, path_suffix=''):
  sock_dir=sock_dir)
 self._num_drives = 0
 
+def subprocess_check_valgrind(self, valgrind) -> None:
+
+if not valgrind:
+return
+
+valgrind_filename =  test_dir + "/" + str(self._popen.pid) + 
".valgrind"
+
+if self.exitcode() == 99:
+with open(valgrind_filename) as f:
+content = f.readlines()
+for line in content:
+print(line, end ="")
+print("")
+else:
+os.remove(valgrind_filename)
+
+def _post_shutdown(self) -> None:
+super()._post_shutdown()
+self.subprocess_check_valgrind(qemu_valgrind)
+
 def add_object(self, opts):
 self._args.append('-object')
 self._args.append(opts)
-- 
2.30.2

[PATCH v3 04/15] qemu-iotests: add option to attach gdbserver

[Emanuele Giuseppe Esposito]

Add -gdb flag and GDB_QEMU environmental variable
to python tests to attach a gdbserver to each qemu instance.

if -gdb is not provided but $GDB_QEMU is set, ignore the
environmental variable.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/qemu-iotests/check  |  6 +-
 tests/qemu-iotests/iotests.py |  4 
 tests/qemu-iotests/testenv.py | 15 ---
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index d1c87ceaf1..6186495eee 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -33,6 +33,9 @@ def make_argparser() -> argparse.ArgumentParser:
help='pretty print output for make check')
 
 p.add_argument('-d', dest='debug', action='store_true', help='debug')
+p.add_argument('-gdb', action='store_true',
+   help="start gdbserver with $GDB_QEMU options. \
+ Default is localhost:12345")
 p.add_argument('-misalign', action='store_true',
help='misalign memory allocations')
 p.add_argument('--color', choices=['on', 'off', 'auto'],
@@ -112,7 +115,8 @@ if __name__ == '__main__':
 env = TestEnv(imgfmt=args.imgfmt, imgproto=args.imgproto,
   aiomode=args.aiomode, cachemode=args.cachemode,
   imgopts=args.imgopts, misalign=args.misalign,
-  debug=args.debug, valgrind=args.valgrind)
+  debug=args.debug, valgrind=args.valgrind,
+  gdb=args.gdb)
 
 testfinder = TestFinder(test_dir=env.source_iotests)
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 90d0b62523..05d0dc0751 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -75,6 +75,10 @@
 qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
 qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
 
+qemu_gdb = []
+if os.environ.get('GDB_QEMU'):
+qemu_gdb = ['gdbserver'] + os.environ.get('GDB_QEMU').strip().split(' ')
+
 imgfmt = os.environ.get('IMGFMT', 'raw')
 imgproto = os.environ.get('IMGPROTO', 'file')
 output_dir = os.environ.get('OUTPUT_DIR', '.')
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1..e131ff42cb 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -72,7 +72,8 @@ class TestEnv(ContextManager['TestEnv']):
  'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO',
  'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
  'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
- 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']
+ 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_',
+ 'GDB_QEMU']
 
 def get_env(self) -> Dict[str, str]:
 env = {}
@@ -163,7 +164,8 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
  imgopts: Optional[str] = None,
  misalign: bool = False,
  debug: bool = False,
- valgrind: bool = False) -> None:
+ valgrind: bool = False,
+ gdb: bool = False) -> None:
 self.imgfmt = imgfmt
 self.imgproto = imgproto
 self.aiomode = aiomode
@@ -171,6 +173,11 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: 
str,
 self.misalign = misalign
 self.debug = debug
 
+if gdb:
+self.gdb_qemu = os.environ.get('GDB_QEMU', 'localhost:12345')
+elif 'GDB_QEMU' in os.environ:
+del os.environ['GDB_QEMU']
+
 if valgrind:
 self.valgrind_qemu = 'y'
 
@@ -268,7 +275,9 @@ def print_env(self) -> None:
 PLATFORM  -- {platform}
 TEST_DIR  -- {TEST_DIR}
 SOCK_DIR  -- {SOCK_DIR}
-SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}"""
+SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
+GDB_QEMU  -- "{GDB_QEMU}"
+"""
 
 args = collections.defaultdict(str, self.get_env())
 
-- 
2.30.2

[PATCH v3 12/15] qemu_iotests: insert valgrind command line as wrapper for qemu binary

[Emanuele Giuseppe Esposito]

The priority will be given to gdb command line, meaning if the -gdb
parameter and -valgrind are given, gdb will be wrapped around
the qemu binary.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/qemu-iotests/iotests.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index aef67e3a86..f9832558a0 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -593,7 +593,8 @@ class VM(qtest.QEMUQtestMachine):
 
 def __init__(self, path_suffix=''):
 name = "qemu%s-%d" % (path_suffix, os.getpid())
-super().__init__(qemu_prog, qemu_opts, wrapper=qemu_gdb,
+wrapper = qemu_gdb if qemu_gdb else qemu_valgrind
+super().__init__(qemu_prog, qemu_opts, wrapper=wrapper,
  name=name,
  test_dir=test_dir,
  socket_scm_helper=socket_scm_helper,
-- 
2.30.2

[PATCH v3 08/15] docs/devel/testing: add -gdb option to the debugging section of QEMU iotests

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito 
---
 docs/devel/testing.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index b7e2370e7e..2ee77a057b 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -229,6 +229,13 @@ Debugging a test case
 QEMU iotests offers some options to debug a failing test, that can be
 given as options to the ``check`` script:
 
+* ``-gdb`` wraps ``gdbsever`` to the QEMU binary,
+  so it is possible to connect to it via gdb.
+  One way to do so is via ``gdb -iex "target remote $GDB_QEMU"``
+  The default address is ``localhost:12345``, and can be changed
+  by setting the ``$GDB_QEMU`` environmental variable.
+  The final command line will be ``gdbserver $GDB_QEMU $QEMU ...``
+
 * ``-d`` (debug) just increases the logging verbosity, showing
   for example the QMP commands and answers.
 
-- 
2.30.2

[PATCH v3 03/15] docs/devel/testing: add debug section to the QEMU iotests chapter

[Emanuele Giuseppe Esposito]

Introduce the "Debugging a test case" section, in preparation
to the additional flags that will be added in the next patches.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 docs/devel/testing.rst | 8 
 1 file changed, 8 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 1434a50cc4..b7e2370e7e 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -224,6 +224,14 @@ another application on the host may have locked the file, 
possibly leading to a
 test failure.  If using such devices are explicitly desired, consider adding
 ``locking=off`` option to disable image locking.
 
+Debugging a test case
+---
+QEMU iotests offers some options to debug a failing test, that can be
+given as options to the ``check`` script:
+
+* ``-d`` (debug) just increases the logging verbosity, showing
+  for example the QMP commands and answers.
+
 Test case groups
 
 
-- 
2.30.2

[PATCH v3 13/15] docs/devel/testing: add -valgrind option to the debug section of QEMU iotests

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito 
---
 docs/devel/testing.rst | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 2ee77a057b..62902cfd2d 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -236,6 +236,13 @@ given as options to the ``check`` script:
   by setting the ``$GDB_QEMU`` environmental variable.
   The final command line will be ``gdbserver $GDB_QEMU $QEMU ...``
 
+* ``-valgrind`` wraps a valgrind instance to QEMU. If it detects
+  warnings, it will print and save the log in
+  ``$TEST_DIR/.valgrind``.
+  The final command line will be ``valgrind --log-file=$TEST_DIR/
+  .valgrind --error-exitcode=99 $QEMU ...``
+  Note: if used together with ``-gdb``, this command will be ignored.
+
 * ``-d`` (debug) just increases the logging verbosity, showing
   for example the QMP commands and answers.
 
-- 
2.30.2

[PATCH v3 09/15] qemu_iotests: extend the check script to support valgrind for python tests

[Emanuele Giuseppe Esposito]

Currently, the check script only parses the option and sets the
VALGRIND_QEMU environmental variable to "y".
Add another local python variable that prepares the command line,
identical to the one provided in the test scripts.

Because the python script does not know in advance the valgring
PID to assign to the log file name, use the "%p" flag in valgrind
log file name that automatically puts the process PID at runtime.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 tests/qemu-iotests/check  |  7 ---
 tests/qemu-iotests/iotests.py | 11 +++
 tests/qemu-iotests/testenv.py |  1 +
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 6186495eee..489178d9a4 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -36,6 +36,10 @@ def make_argparser() -> argparse.ArgumentParser:
 p.add_argument('-gdb', action='store_true',
help="start gdbserver with $GDB_QEMU options. \
  Default is localhost:12345")
+p.add_argument('-valgrind', action='store_true',
+help='use valgrind, sets VALGRIND_QEMU environment '
+'variable')
+
 p.add_argument('-misalign', action='store_true',
help='misalign memory allocations')
 p.add_argument('--color', choices=['on', 'off', 'auto'],
@@ -86,9 +90,6 @@ def make_argparser() -> argparse.ArgumentParser:
 g_bash.add_argument('-o', dest='imgopts',
 help='options to pass to qemu-img create/convert, '
 'sets IMGOPTS environment variable')
-g_bash.add_argument('-valgrind', action='store_true',
-help='use valgrind, sets VALGRIND_QEMU environment '
-'variable')
 
 g_sel = p.add_argument_group('test selecting options',
  'The following options specify test set '
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 4f3fb13915..a2e8604674 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -96,6 +96,17 @@
 sys.stderr.write('Please run this test via the "check" script\n')
 sys.exit(os.EX_USAGE)
 
+qemu_valgrind = []
+if os.environ.get('VALGRIND_QEMU') == "y" and \
+os.environ.get('NO_VALGRIND') != "y":
+valgrind_logfile = "--log-file=" + test_dir.strip()
+# %p allows to put the valgrind process PID, since
+# we don't know it a priori (subprocess.Peopen is
+# not yet invoked)
+valgrind_logfile += "/%p.valgrind"
+
+qemu_valgrind = ['valgrind', valgrind_logfile, '--error-exitcode=99']
+
 socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper')
 
 luks_default_secret_object = 'secret,id=keysec0,data=' + \
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index e131ff42cb..39ae7ace33 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -277,6 +277,7 @@ def print_env(self) -> None:
 SOCK_DIR  -- {SOCK_DIR}
 SOCKET_SCM_HELPER -- {SOCKET_SCM_HELPER}
 GDB_QEMU  -- "{GDB_QEMU}"
+VALGRIND_QEMU -- "{VALGRIND_QEMU}"
 """
 
 args = collections.defaultdict(str, self.get_env())
-- 
2.30.2

[PATCH v3 05/15] qemu-iotests: delay QMP socket timers

[Emanuele Giuseppe Esposito]

Attaching a gdbserver implies that the qmp socket
should wait indefinitely for an answer from QEMU.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 python/qemu/machine.py|  3 +++
 tests/qemu-iotests/iotests.py | 10 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 12752142c9..d6142271c2 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -409,6 +409,9 @@ def _launch(self) -> None:
stderr=subprocess.STDOUT,
shell=False,
close_fds=False)
+
+if 'gdbserver' in self._wrapper:
+self._qmp_timer = None
 self._post_launch()
 
 def _early_cleanup(self) -> None:
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 05d0dc0751..380527245e 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -478,7 +478,10 @@ def log(msg: Msg,
 
 class Timeout:
 def __init__(self, seconds, errmsg="Timeout"):
-self.seconds = seconds
+if qemu_gdb:
+self.seconds = 3000
+else:
+self.seconds = seconds
 self.errmsg = errmsg
 def __enter__(self):
 signal.signal(signal.SIGALRM, self.timeout)
@@ -684,6 +687,11 @@ def qmp_to_opts(self, obj):
 output_list += [key + '=' + obj[key]]
 return ','.join(output_list)
 
+def get_qmp_events(self, wait: bool = False) -> List[QMPMessage]:
+if qemu_gdb:
+wait = 0.0
+return super().get_qmp_events(wait=wait)
+
 def get_qmp_events_filtered(self, wait=60.0):
 result = []
 for ev in self.get_qmp_events(wait=wait):
-- 
2.30.2

[PATCH v3 02/15] python: qemu: pass the wrapper field from QEMUQtestmachine to QEMUMachine

[Emanuele Giuseppe Esposito]

Signed-off-by: Emanuele Giuseppe Esposito 
---
 python/qemu/qtest.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 39a0cf62fe..c18eae96c6 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -111,6 +111,7 @@ class QEMUQtestMachine(QEMUMachine):
 def __init__(self,
  binary: str,
  args: Sequence[str] = (),
+ wrapper: Sequence[str] = (),
  name: Optional[str] = None,
  test_dir: str = "/var/tmp",
  socket_scm_helper: Optional[str] = None,
@@ -119,7 +120,8 @@ def __init__(self,
 name = "qemu-%d" % os.getpid()
 if sock_dir is None:
 sock_dir = test_dir
-super().__init__(binary, args, name=name, test_dir=test_dir,
+super().__init__(binary, args, wrapper=wrapper, name=name,
+ test_dir=test_dir,
  socket_scm_helper=socket_scm_helper,
  sock_dir=sock_dir)
 self._qtest: Optional[QEMUQtestProtocol] = None
-- 
2.30.2

[PATCH v3 01/15] python: qemu: add timer parameter for qmp.accept socket

[Emanuele Giuseppe Esposito]

Add a new _qmp_timer field to the QEMUMachine class.
The default timer is 15 sec, as per the default in the
qmp accept() function.

Signed-off-by: Emanuele Giuseppe Esposito 
---
 python/qemu/machine.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337..12752142c9 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -110,6 +110,7 @@ def __init__(self,
 self._binary = binary
 self._args = list(args)
 self._wrapper = wrapper
+self._qmp_timer = 15.0
 
 self._name = name or "qemu-%d" % os.getpid()
 self._test_dir = test_dir
@@ -323,7 +324,7 @@ def _pre_launch(self) -> None:
 
 def _post_launch(self) -> None:
 if self._qmp_connection:
-self._qmp.accept()
+self._qmp.accept(self._qmp_timer)
 
 def _post_shutdown(self) -> None:
 """
-- 
2.30.2

[PATCH v3 00/15] qemu_iotests: improve debugging options

[Emanuele Giuseppe Esposito]

This series adds the option to attach gdbserver and valgrind
to the QEMU binary running in qemu_iotests.
It also allows to redirect QEMU binaries output of the python tests
to the stdout, instead of a log file.

Patches 1-6 introduce the -gdb option to both python and bash tests, 
7-10 extend the already existing -valgrind flag to work also on 
python tests, and patch 11 introduces -p to enable logging to stdout.

In particular, patches 1,2,4,8 focus on extending the QMP socket timers
when using gdb/valgrind, otherwise the python tests will fail due to
delays in the QMP responses.

This series is tested on the previous serie
"qemu-iotests: quality of life improvements"
but independent from it, so it can be applied separately.

Signed-off-by: Emanuele Giuseppe Esposito 
---
v3:
- Introduce the class field _qmp_timer instead of a function parameter
in the _post_launch() function [John]
- style and cleanup fixes in iotests.py [Paolo]


Emanuele Giuseppe Esposito (15):
  python: qemu: add timer parameter for qmp.accept socket
  python: qemu: pass the wrapper field from QEMUQtestmachine to
QEMUMachine
  docs/devel/testing: add debug section to the QEMU iotests chapter
  qemu-iotests: add option to attach gdbserver
  qemu-iotests: delay QMP socket timers
  qemu_iotests: insert gdbserver command line as wrapper for qemu binary
  qemu-iotests: add gdbserver option to script tests too
  docs/devel/testing: add -gdb option to the debugging section of QEMU
iotests
  qemu_iotests: extend the check script to support valgrind for python
tests
  qemu_iotests: extent QMP socket timeout when using valgrind
  qemu_iotests: allow valgrind to read/delete the generated log file
  qemu_iotests: insert valgrind command line as wrapper for qemu binary
  docs/devel/testing: add -valgrind option to the debug section of QEMU
iotests
  qemu_iotests: add option to show qemu binary logs on stdout
  docs/devel/testing: add -p option to the debug section of QEMU iotests

 docs/devel/testing.rst| 26 
 python/qemu/machine.py|  6 +++-
 python/qemu/qtest.py  |  4 ++-
 tests/qemu-iotests/check  | 14 ++---
 tests/qemu-iotests/common.rc  |  8 -
 tests/qemu-iotests/iotests.py | 58 +--
 tests/qemu-iotests/testenv.py | 21 +++--
 7 files changed, 125 insertions(+), 12 deletions(-)

-- 
2.30.2

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

[Daniel P . Berrangé]

On Tue, Apr 13, 2021 at 06:08:48PM +0200, Paolo Bonzini wrote:
> 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(-)

Reviewed-by: Daniel P. Berrangé 


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 :|