Re: [PATCH v2 3/4] target/i386: add the missing features for Icelake-Server CPU model
On 3/28/2020 11:06 AM, Chenyi Qiang wrote:
Add the SHA_NI and AVX512IFMA feature bits in FEAT_7_0_EBX, RDPID
feature bit in FEAT_7_0_ECX and FSRM feature bit in FEAT_7_0_EDX.
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b01421c6bb..babb074abf 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3552,6 +3552,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
{ /* end of list */ }
},
},
+{
+.version = 3,
Maybe you need to make it as version = 4, since 3 is used to add
ARCH_CAPABILITIES bits and the Patch is queued:
https://lore.kernel.org/qemu-devel/20200323183936.ga3...@habkost.net/
+.props = (PropValue[]) {
+{ "sha-ni", "on" },
+{ "avx512ifma", "on" },
+{ "rdpid", "on" },
+{ "fsrm", "on" },
+{ /* end of list */ }
+},
+},
{ /* end of list */ }
}
},
Re: [PATCH v6 61/61] target/riscv: configure and turn on vector extension from command line
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Vector extension is default off. The only way to use vector extension is > 1. use cpu rv32 or rv64 > 2. turn on it by command line > "-cpu rv64,x-v=true,vlen=128,elen=64,vext_spec=v0.7.1". > > vlen is the vector register length, default value is 128 bit. > elen is the max operator size in bits, default value is 64 bit. > vext_spec is the vector specification version, default value is v0.7.1. > These properties can be specified with other values. > > Signed-off-by: LIU Zhiwei > --- > target/riscv/cpu.c | 44 +++- > target/riscv/cpu.h | 2 ++ > 2 files changed, 45 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 59/61] target/riscv: vector register gather instruction
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 9 ++ > target/riscv/insn32.decode | 3 + > target/riscv/insn_trans/trans_rvv.inc.c | 127 > target/riscv/vector_helper.c| 64 > 4 files changed, 203 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 58/61] target/riscv: vector slide instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 17 > target/riscv/insn32.decode | 7 ++ > target/riscv/insn_trans/trans_rvv.inc.c | 17 > target/riscv/vector_helper.c| 128 > 4 files changed, 169 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 57/61] target/riscv: floating-point scalar move instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +/* Floating-Point Scalar Move Instructions */
> +static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
> +{
> +if (!s->vill && has_ext(s, RVF) &&
> +(s->mstatus_fs != 0) && (s->sew != 0)) {
> +#ifdef HOST_WORDS_BIGENDIAN
> +int ofs = vreg_ofs(s, a->rs2) + ((7 >> s->sew) << s->sew);
> +#else
> +int ofs = vreg_ofs(s, a->rs2);
> +#endif
Use endian_ofs from patch 55.
> +switch (s->sew) {
> +case MO_8:
> +tcg_gen_ld8u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> +tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> +0xff00ULL);
> +break;
MO_8 should be illegal.
> +case MO_16:
> +tcg_gen_ld16u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> +tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> +0xULL);
> +break;
> +case MO_32:
> +tcg_gen_ld32u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> +tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> +0xULL);
> +break;
> +default:
> +if (has_ext(s, RVD)) {
> +tcg_gen_ld_i64(cpu_fpr[a->rd], cpu_env, ofs);
> +} else {
> +tcg_gen_ld32u_i64(cpu_fpr[a->rd], cpu_env, ofs);
> +tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> +0xULL);
> +}
> +break;
Maybe better with MO_64 and default: g_assert_not_reached().
> +static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
> +{
> +if (!s->vill && has_ext(s, RVF) && (s->sew != 0)) {
> +TCGv_ptr dest;
> +TCGv_i64 src1;
> +static gen_helper_vfmv_s_f * const fns[3] = {
> +gen_helper_vfmv_s_f_h,
> +gen_helper_vfmv_s_f_w,
> +gen_helper_vfmv_s_f_d
You shouldn't need to duplicate the vmv_s_x_* helpers.
r~
Re: [PATCH v6 55/61] target/riscv: integer extract instruction
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +/* Integer Extract Instruction */
> +static void extract_element(TCGv dest, TCGv_ptr base,
> +int ofs, int sew)
> +{
> +switch (sew) {
> +case MO_8:
> +tcg_gen_ld8u_tl(dest, base, ofs);
> +break;
> +case MO_16:
> +tcg_gen_ld16u_tl(dest, base, ofs);
> +break;
> +default:
> +tcg_gen_ld32u_tl(dest, base, ofs);
> +break;
> +#if TARGET_LONG_BITS == 64
> +case MO_64:
> +tcg_gen_ld_i64(dest, base, ofs);
> +break;
> +#endif
> +}
> +}
I just remembered that this doesn't handle HOST_WORDS_BIGENDIAN properly -- the
MO_64 case for TARGET_LONG_BITS == 32.
Because we computed the offset for MO_64, not MO_32, we need
case MO_64:
if (TARGET_LONG_BITS == 64) {
tcg_gen_ld_i64(dest, base, ofs);
break;
}
#ifdef HOST_WORDS_BIGENDIAN
ofs += 4;
#endif
/* fall through */
case MO_32:
tcg_gen_ld32u_tl(dest, base, ofs);
break;
default:
g_assert_not_reached();
r~
Re: [PATCH v3 2/2] net/colo-compare.c: handling of the full primary or secondary queue
Lukas Straub 於 2020年3月28日 週六 上午2:28寫道：
>
> On Sat, 28 Mar 2020 02:20:21 +0800
> Derek Su wrote:
>
> > Lukas Straub 於 2020年3月28日 週六 上午1:46寫道：
> > >
> > > On Wed, 25 Mar 2020 17:43:54 +0800
> > > Derek Su wrote:
> > >
> > > > The pervious handling of the full primary or queue is only dropping
> > > > the packet. If there are lots of clients to the guest VM,
> > > > the "drop" will lead to the lost of the networking connection
> > > > until next checkpoint.
> > > >
> > > > To address the issue, this patch drops the packet firstly.
> > > > Then, send all queued primary packets, remove all queued secondary
> > > > packets and do checkpoint.
> > > >
> > > > Signed-off-by: Derek Su
> > > > ---
> > > > net/colo-compare.c | 41 ++---
> > > > 1 file changed, 30 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/net/colo-compare.c b/net/colo-compare.c
> > > > index cdd87b2aa8..1a52f50fbe 100644
> > > > --- a/net/colo-compare.c
> > > > +++ b/net/colo-compare.c
> > > > @@ -125,6 +125,12 @@ static const char *colo_mode[] = {
> > > > [SECONDARY_IN] = "secondary",
> > > > };
> > > >
> > > > +enum {
> > > > +QUEUE_INSERT_ERR = -1,
> > > > +QUEUE_INSERT_OK = 0,
> > > > +QUEUE_INSERT_FULL = 1,
> > > > +};
> > > > +
> > > > static int compare_chr_send(CompareState *s,
> > > > const uint8_t *buf,
> > > > uint32_t size,
> > > > @@ -211,8 +217,10 @@ static int colo_insert_packet(GQueue *queue,
> > > > Packet *pkt, uint32_t *max_ack)
> > > > }
> > > >
> > > > /*
> > > > - * Return 0 on success, if return -1 means the pkt
> > > > - * is unsupported(arp and ipv6) and will be sent later
> > > > + * Return QUEUE_INSERT_OK on success.
> > > > + * If return QUEUE_INSERT_FULL means list is full, and
> > > > + * QUEUE_INSERT_ERR means the pkt is unsupported(arp and ipv6) and
> > > > + * will be sent later
> > > > */
> > > > static int packet_enqueue(CompareState *s, int mode, Connection **con)
> > > > {
> > > > @@ -234,7 +242,7 @@ static int packet_enqueue(CompareState *s, int
> > > > mode, Connection **con)
> > > > if (parse_packet_early(pkt)) {
> > > > packet_destroy(pkt, NULL);
> > > > pkt = NULL;
> > > > -return -1;
> > > > +return QUEUE_INSERT_ERR;
> > > > }
> > > > fill_connection_key(pkt, );
> > > >
> > > > @@ -258,11 +266,12 @@ static int packet_enqueue(CompareState *s, int
> > > > mode, Connection **con)
> > > > "drop packet", colo_mode[mode]);
> > > > packet_destroy(pkt, NULL);
> > > > pkt = NULL;
> > > > +return QUEUE_INSERT_FULL;
> > > > }
> > > >
> > > > *con = conn;
> > > >
> > > > -return 0;
> > > > +return QUEUE_INSERT_OK;
> > > > }
> > > >
> > > > static inline bool after(uint32_t seq1, uint32_t seq2)
> > > > @@ -995,17 +1004,22 @@ static void
> > > > compare_pri_rs_finalize(SocketReadState *pri_rs)
> > > > {
> > > > CompareState *s = container_of(pri_rs, CompareState, pri_rs);
> > > > Connection *conn = NULL;
> > > > +int ret;
> > > >
> > > > -if (packet_enqueue(s, PRIMARY_IN, )) {
> > > > +ret = packet_enqueue(s, PRIMARY_IN, );
> > > > +if (ret == QUEUE_INSERT_OK) {
> > > > +/* compare packet in the specified connection */
> > > > +colo_compare_connection(conn, s);
> > > > +} else if (ret == QUEUE_INSERT_FULL) {
> > > > +g_queue_foreach(>conn_list, colo_flush_packets, s);
> > > > +colo_compare_inconsistency_notify(s);
> > > > +} else {
> > > > trace_colo_compare_main("primary: unsupported packet in");
> > > > compare_chr_send(s,
> > > > pri_rs->buf,
> > > > pri_rs->packet_len,
> > > > pri_rs->vnet_hdr_len,
> > > > false);
> > > > -} else {
> > > > -/* compare packet in the specified connection */
> > > > -colo_compare_connection(conn, s);
> > > > }
> > > > }
> > > >
> > > > @@ -1013,12 +1027,17 @@ static void
> > > > compare_sec_rs_finalize(SocketReadState *sec_rs)
> > > > {
> > > > CompareState *s = container_of(sec_rs, CompareState, sec_rs);
> > > > Connection *conn = NULL;
> > > > +int ret;
> > > >
> > > > -if (packet_enqueue(s, SECONDARY_IN, )) {
> > > > -trace_colo_compare_main("secondary: unsupported packet in");
> > > > -} else {
> > > > +ret = packet_enqueue(s, SECONDARY_IN, );
> > > > +if (ret == QUEUE_INSERT_OK) {
> > > > /* compare packet in the specified connection */
> > > > colo_compare_connection(conn, s);
> > > > +} else if (ret == QUEUE_INSERT_FULL) {
> > > > +g_queue_foreach(>conn_list, colo_flush_packets, s);
> > > > +colo_compare_inconsistency_notify(s);
> > > > +} else {
> > > > +trace_colo_compare_main("secondary: unsupported packet in");
> > > >
Re: [PATCH v6 41/61] target/riscv: vector floating-point merge instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +for (i = 0; i < vl; i++) {\
> +if (!vm && !vext_elem_mask(v0, mlen, i)) {\
> +ETYPE s2 = *((ETYPE *)vs2 + H(i));\
> +*((ETYPE *)vd + H1(i)) = s2; \
H1 should be H.
> +} else { \
> +*((ETYPE *)vd + H(i)) = (ETYPE)s1;\
> +} \
You can also hoist the s2 dereference out of the IF, and let the assignment be
unconditional.
*((ETYPE *)vd + H(i))
= (!vm && !vext_elem_mask(v0, mlen, i) ? s2 : s1);
r~
[PATCH v2 3/4] target/i386: add the missing features for Icelake-Server CPU model
Add the SHA_NI and AVX512IFMA feature bits in FEAT_7_0_EBX, RDPID
feature bit in FEAT_7_0_ECX and FSRM feature bit in FEAT_7_0_EDX.
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b01421c6bb..babb074abf 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3552,6 +3552,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
{ /* end of list */ }
},
},
+{
+.version = 3,
+.props = (PropValue[]) {
+{ "sha-ni", "on" },
+{ "avx512ifma", "on" },
+{ "rdpid", "on" },
+{ "fsrm", "on" },
+{ /* end of list */ }
+},
+},
{ /* end of list */ }
}
},
--
2.17.1
[PATCH v2 2/4] target/i386: add fast short REP MOV support
For CPUs support fast short REP MOV[CPUID.(EAX=7,ECX=0):EDX(bit4)], e.g
Icelake and Tigerlake, expose it to the guest VM.
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4123d5910e..b01421c6bb 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1077,7 +1077,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] =
{
.type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, "avx512-4vnniw", "avx512-4fmaps",
-NULL, NULL, NULL, NULL,
+"fsrm", NULL, NULL, NULL,
NULL, NULL, "md-clear", NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL /* pconfig */, NULL,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 60d797d594..f7ba813cab 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -770,6 +770,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2)
/* AVX512 Multiply Accumulation Single Precision */
#define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3)
+/* Fast Short Rep Mov */
+#define CPUID_7_0_EDX_FSRM (1U << 4)
/* Speculation Control */
#define CPUID_7_0_EDX_SPEC_CTRL (1U << 26)
/* Single Thread Indirect Branch Predictors */
--
2.17.1
[PATCH v2 4/4] target/i386: modify Icelake-Client and Icelake-Server CPU model number
According to the Intel Icelake family list, Icelake-Client uses model
number 125(0x7D) and Icelake-Server uses model number 106(0x6A).
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index babb074abf..5bd52f2c4b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3328,7 +3328,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
.level = 0xd,
.vendor = CPUID_VENDOR_INTEL,
.family = 6,
-.model = 126,
+.model = 125,
.stepping = 0,
.features[FEAT_1_EDX] =
CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
@@ -3441,7 +3441,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
.level = 0xd,
.vendor = CPUID_VENDOR_INTEL,
.family = 6,
-.model = 134,
+.model = 106,
.stepping = 0,
.features[FEAT_1_EDX] =
CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
--
2.17.1
[PATCH v2 1/4] target/i386: add missing vmx features for several CPU models
Add some missing VMX features in Skylake-Server, Cascadelake-Server and
Icelake-Server CPU models based on the output of Paolo's script.
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 34b511f078..4123d5910e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3059,6 +3059,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
VMX_SECONDARY_EXEC_RDRAND_EXITING |
VMX_SECONDARY_EXEC_ENABLE_INVPCID |
VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS
|
VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
+.features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
.xlevel = 0x8008,
.model_id = "Intel Xeon Processor (Skylake)",
.versions = (X86CPUVersionDefinition[]) {
@@ -3187,6 +3188,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
VMX_SECONDARY_EXEC_RDRAND_EXITING |
VMX_SECONDARY_EXEC_ENABLE_INVPCID |
VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS
|
VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
+.features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
.xlevel = 0x8008,
.model_id = "Intel Xeon Processor (Cascadelake)",
.versions = (X86CPUVersionDefinition[]) {
@@ -3534,7 +3536,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT |
VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
VMX_SECONDARY_EXEC_RDRAND_EXITING |
VMX_SECONDARY_EXEC_ENABLE_INVPCID |
- VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS,
+ VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS
|
+ VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
+.features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
.xlevel = 0x8008,
.model_id = "Intel Xeon Processor (Icelake)",
.versions = (X86CPUVersionDefinition[]) {
--
2.17.1
[PATCH v2 0/4] modify CPU model info
Add the missing VMX features in Skylake-Server, Cascadelake-Server and Icelake-Server CPU models. In Icelake-Server CPU model, it also lacks sha_ni, avx512ifma, rdpid and fsrm. The model numbers of Icelake-Client and Icelake-Server need to be fixed. Changes in v2: - add missing features as a new version of CPU model - add the support of FSRM Chenyi Qiang (4): target/i386: add missing vmx features for several CPU models target/i386: add fast short REP MOV support target/i386: add the missing features for Icelake-Server CPU model target/i386: modify Icelake-Client and Icelake-Server CPU model number target/i386/cpu.c | 22 ++ target/i386/cpu.h | 2 ++ 2 files changed, 20 insertions(+), 4 deletions(-) -- 2.17.1
Re: [PATCH v6 40/61] target/riscv: vector floating-point classify instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/fpu_helper.c | 33 + > target/riscv/helper.h | 4 ++ > target/riscv/insn32.decode | 1 + > target/riscv/insn_trans/trans_rvv.inc.c | 3 + > target/riscv/internals.h| 5 ++ > target/riscv/vector_helper.c| 93 + > 6 files changed, 109 insertions(+), 30 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 39/61] target/riscv: vector floating-point compare instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +static uint8_t vmfne16(uint16_t a, uint16_t b, float_status *s)
> +{
> +int compare = float16_compare_quiet(a, b, s);
> +return compare != float_relation_equal &&
> + compare != float_relation_unordered;
> +}
> +
> +static uint8_t vmfne32(uint32_t a, uint32_t b, float_status *s)
> +{
> +int compare = float32_compare_quiet(a, b, s);
> +return compare != float_relation_equal &&
> + compare != float_relation_unordered;
> +}
> +
> +static uint8_t vmfne64(uint64_t a, uint64_t b, float_status *s)
> +{
> +int compare = float64_compare_quiet(a, b, s);
> +return compare != float_relation_equal &&
> + compare != float_relation_unordered;
> +}
This is incorrect -- the result should be true for unordered. The text for
0.7.1 does not specify, but this is the normal interpretation of NE. The text
for 0.8 explicitly says that the result is 1 for NaN.
r~
Re: [PATCH v2] monitor/hmp-cmds: add units for migrate_parameters.
On 3/28/20 2:02 AM, Dr. David Alan Gilbert wrote: * Mao Zhongyi (maozhon...@cmss.chinamobile.com) wrote: When running: (qemu) info migrate_parameters announce-initial: 50 ms announce-max: 550 ms announce-step: 100 ms compress-wait-thread: on ... max-bandwidth: 33554432 bytes/second downtime-limit: 300 milliseconds x-checkpoint-delay: 2 ... xbzrle-cache-size: 67108864 add units for the parameters 'x-checkpoint-delay' and 'xbzrle-cache-size', it's easier to read, also move milliseconds to ms to keep the same style. Signed-off-by: Mao Zhongyi Thanks Reviewed-by: Dr. David Alan Gilbert (info migrate could also be fixed, but that's a separate issue) Yes, will fix it in a separated patch. Thanks, Mao --- monitor/hmp-cmds.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 2a900a528a..790fad3afe 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -436,11 +436,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH), params->max_bandwidth); assert(params->has_downtime_limit); -monitor_printf(mon, "%s: %" PRIu64 " milliseconds\n", +monitor_printf(mon, "%s: %" PRIu64 " ms\n", MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT), params->downtime_limit); assert(params->has_x_checkpoint_delay); -monitor_printf(mon, "%s: %u\n", +monitor_printf(mon, "%s: %u ms\n", MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY), params->x_checkpoint_delay); assert(params->has_block_incremental); @@ -453,7 +453,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, "%s: %s\n", MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION), MultiFDCompression_str(params->multifd_compression)); -monitor_printf(mon, "%s: %" PRIu64 "\n", +monitor_printf(mon, "%s: %" PRIu64 " bytes\n", MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE), params->xbzrle_cache_size); monitor_printf(mon, "%s: %" PRIu64 "\n", -- 2.17.1 -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v6 29/61] target/riscv: vector narrowing fixed-point clip instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 13 +++ > target/riscv/insn32.decode | 6 ++ > target/riscv/insn_trans/trans_rvv.inc.c | 8 ++ > target/riscv/vector_helper.c| 137 > 4 files changed, 164 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 28/61] target/riscv: vector single-width scaling shift instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 17 > target/riscv/insn32.decode | 6 ++ > target/riscv/insn_trans/trans_rvv.inc.c | 8 ++ > target/riscv/vector_helper.c| 117 > 4 files changed, 148 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 27/61] target/riscv: vector widening saturating scaled multiply-add
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 22 +++ > target/riscv/insn32.decode | 7 + > target/riscv/insn_trans/trans_rvv.inc.c | 9 ++ > target/riscv/vector_helper.c| 205 > 4 files changed, 243 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 25/61] target/riscv: vector single-width averaging add and subtract
On 3/27/20 6:07 PM, LIU Zhiwei wrote:
>
>
> On 2020/3/28 8:32, Richard Henderson wrote:
>> On 3/18/20 8:46 PM, LIU Zhiwei wrote:
>>> +static inline int32_t asub32(CPURISCVState *env, int vxrm, int32_t a,
>>> int32_t b)
>>> +{
>>> +int64_t res = (int64_t)a - b;
>>> +uint8_t round = get_round(vxrm, res, 1);
>>> +
>>> +return (res >> 1) + round;
>>> +}
>>> +
>>>
>>> I find a corner case here. As the spec said in Section 13.2
>>>
>>> "There can be no overflow in the result".
>>>
>>> If the a is 0x7fff, b is 0x8000, and the round mode is round to
>>> up(rnu),
>>> then the result is (0x7fff - 0x8000 + 1) >> 1, equals 0x8000,
>>> according the v0.7.1
>> That's why we used int64_t as the intermediate type:
>>
>> 0x7fff - 0x8000 + 1
>> = 0x7fff + 0x8000 + 1
>> = 0x + 1
>> = 0x0001
>>
>> Shift that right by 1 and you do indeed get 0x8000.
>> There's no saturation involved.
>
> The minuend 0x7fff is INT32_MAX, and the subtrahend 0x8000 is
> INT32_MIN.
>
> The difference between the minuend and the subtrahend should be a positive
> number. But the result here is 0x8000.
>
> So it is overflow. However, according to the spec, it should not overflow.
Unless I'm missing something, the spec is wrong about "there can be no
overflow", the above being a counter-example.
Do you have hardware to compare against? Perhaps it is in fact "overflow is
ignored", as the 0.8 spec says for vasubu?
I wouldn't add saturation, because the spec says nothing about saturation, and
does mention truncation, at least for vasubu.
r~
Re: [PATCH v6 26/61] target/riscv: vector single-width fractional multiply with rounding and saturation
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +static int64_t vsmul64(CPURISCVState *env, int vxrm, int64_t a, int64_t b)
> +{
> +uint8_t round;
> +uint64_t hi_64, lo_64, Hi62;
> +uint8_t hi62, hi63, lo63;
> +
> +muls64(_64, _64, a, b);
> +hi62 = extract64(hi_64, 62, 1);
> +lo63 = extract64(lo_64, 63, 1);
> +hi63 = extract64(hi_64, 63, 1);
> +Hi62 = extract64(hi_64, 0, 62);
This seems like way more work than necessary.
> +if (hi62 != hi63) {
> +env->vxsat = 0x1;
> +return INT64_MAX;
> +}
This can only happen for a == b == INT64_MIN.
Perhaps just test exactly that and move it above the multiply?
> +round = get_round(vxrm, lo_64, 63);
> +if (round && (Hi62 == 0x3fff) && lo63) {
> +env->vxsat = 0x1;
> +return hi62 ? INT64_MIN : INT64_MAX;
> +} else {
> +if (lo63 && round) {
> +return (hi_64 + 1) << 1;
> +} else {
> +return (hi_64 << 1) | lo63 | round;
> +}
> +}
/* Cannot overflow, as there are always
2 sign bits after multiply. */
ret = (hi_64 << 1) | (lo_64 >> 63);
if (round) {
if (ret == INT64_MAX) {
env->vxsat = 1;
} else {
ret += 1;
}
}
return ret;
r~
Re: [PATCH v6 25/61] target/riscv: vector single-width averaging add and subtract
On 2020/3/28 8:32, Richard Henderson wrote:
On 3/18/20 8:46 PM, LIU Zhiwei wrote:
+static inline int32_t asub32(CPURISCVState *env, int vxrm, int32_t a, int32_t
b)
+{
+int64_t res = (int64_t)a - b;
+uint8_t round = get_round(vxrm, res, 1);
+
+return (res >> 1) + round;
+}
+
I find a corner case here. As the spec said in Section 13.2
"There can be no overflow in the result".
If the a is 0x7fff, b is 0x8000, and the round mode is round to
up(rnu),
then the result is (0x7fff - 0x8000 + 1) >> 1, equals 0x8000,
according the v0.7.1
That's why we used int64_t as the intermediate type:
0x7fff - 0x8000 + 1
= 0x7fff + 0x8000 + 1
= 0x + 1
= 0x0001
Shift that right by 1 and you do indeed get 0x8000.
There's no saturation involved.
The minuend 0x7fff is INT32_MAX, and the subtrahend 0x8000 is
INT32_MIN.
The difference between the minuendand the subtrahend should be a
positive number. But the result here is 0x8000.
So it is overflow. However, according to the spec, it should not overflow.
I think a special process for (INT*_MAX - INT*_MIN) is needed.
Zhiwei
For int64_t we computed signed overflow to do the same thing.
r~
PCI device perf limitation
Hi, I noticed a perf issue in my own pci device, and managed to reproduce it with the pci-testdev with a small patch. Here: 1. diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c 2. index 188de4d9cc..b2e225d25b 100644 3. --- a/hw/misc/pci-testdev.c 4. +++ b/hw/misc/pci-testdev.c 5. @@ -252,7 +252,7 @@ static void pci_testdev_realize(PCIDevice *pci_dev , Error **errp) 6. pci_conf[PCI_INTERRUPT_PIN] = 0; /* no interrupt pin */ 7. 8. memory_region_init_io(>mmio, OBJECT(d), _testdev_mmio_ops, d, 9. - "pci-testdev-mmio", IOTEST_MEMSIZE * 2); 10. + "pci-testdev-mmio", 256 * 1024 * 1024); 11. memory_region_init_io(>portio, OBJECT(d), _testdev_pio_ops, d, 12. "pci-testdev-portio", IOTEST_IOSIZE * 2); 13. pci_register_bar(pci_dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, >mmio ); Before my patch, I could start a VM with 32 of these devices happily. I can ssh to the VM within a minute. I also see that once the VM is up, I see small amount of kvm activity, i.e. less than 100 kvm exits a second. After the patch, I can start no more than 3 devices. If I do, I seems like QEMU gets to a state of thrashing. The VM never comes up, and I see the following: kvm statistics - summary Event Total %Total CurAvg/s kvm_fpu52166411 46.8 2773738 kvm_userspace_exit 26083186 23.4 1386869 kvm_vcpu_wakeup25415616 22.8 1386869 I tried to trace the kvm exit reason, but I get: qemu-system-x86 83801 [090] 10892345.982869: kvm:kvm_userspace_exit: reason KVM_EXIT_UNKNOWN (0) I am wondering if this is known performance limitation on io memory region? Is there a way around it? Thanks, Mo
[PATCH v4 1/2] virtio-blk: delete vqs on the error path in realize()
virtio_vqs forgot to free on the error path in realize(). Fix that.
The asan stack:
Direct leak of 14336 byte(s) in 1 object(s) allocated from:
#0 0x7f58b93fd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f58b858249d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x5562cc627f49 in virtio_add_queue /mnt/sdb/qemu/hw/virtio/virtio.c:2413
#3 0x5562cc4b524a in virtio_blk_device_realize
/mnt/sdb/qemu/hw/block/virtio-blk.c:1202
#4 0x5562cc613050 in virtio_device_realize
/mnt/sdb/qemu/hw/virtio/virtio.c:3615
#5 0x5562ccb7a568 in device_set_realized /mnt/sdb/qemu/hw/core/qdev.c:891
#6 0x5562cd39cd45 in property_set_bool /mnt/sdb/qemu/qom/object.c:2238
Reported-by: Euler Robot
Signed-off-by: Pan Nengyuan
Reviewed-by: Stefano Garzarella
---
v2->v1:
- Fix incorrect free in v1, it will cause a uaf.
---
Cc: Stefan Hajnoczi
Cc: Kevin Wolf
Cc: Max Reitz
Cc: qemu-bl...@nongnu.org
---
hw/block/virtio-blk.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 142863a3b2..97ba8a2187 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1204,6 +1204,9 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
virtio_blk_data_plane_create(vdev, conf, >dataplane, );
if (err != NULL) {
error_propagate(errp, err);
+for (i = 0; i < conf->num_queues; i++) {
+virtio_del_queue(vdev, i);
+}
virtio_cleanup(vdev);
return;
}
--
2.18.2
[PATCH v4 0/2] fix two virtio queues memleak
This series fix two vqs leak: 1. Do delete vqs on the error path in virtio_blk_device_realize(). 2. Do delete vqs in virtio_iommu_device_unrealize() to fix another leaks. v2->v1: - Fix incorrect free in virtio_blk_device_realize, it will cause a uaf. v3->v2: - Also clean 's->as_by_busptr' hash table in virtio_iommu_device_unrealize.(Suggested by Stefano Garzarella) v4->v3: - update patch2/2 subject message and move g_hash_table_destroy() at the beggining of unrealize(). Pan Nengyuan (2): virtio-blk: delete vqs on the error path in realize() virtio-iommu: avoid memleak in the unrealize hw/block/virtio-blk.c| 3 +++ hw/virtio/virtio-iommu.c | 3 +++ 2 files changed, 6 insertions(+) -- 2.18.2
[PATCH v4 2/2] virtio-iommu: avoid memleak in the unrealize
req_vq/event_vq forgot to free in unrealize. Fix that. And also do clean 's->as_by_busptr' hash table in unrealize to fix another leak. Signed-off-by: Pan Nengyuan Acked-by: Eric Auger --- Cc: Eric Auger Cc: Stefan Hajnoczi --- v3->v1/v2: - Also clean 's->as_by_busptr' hash table in unrealize.(Suggested by Stefano Garzarella) v4->v3: - update subject msg and move g_hash_table_destroy to the beginning of unrealize. --- hw/virtio/virtio-iommu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index 4cee8083bc..22ba8848c2 100644 --- a/hw/virtio/virtio-iommu.c +++ b/hw/virtio/virtio-iommu.c @@ -693,9 +693,12 @@ static void virtio_iommu_device_unrealize(DeviceState *dev, Error **errp) VirtIODevice *vdev = VIRTIO_DEVICE(dev); VirtIOIOMMU *s = VIRTIO_IOMMU(dev); +g_hash_table_destroy(s->as_by_busptr); g_tree_destroy(s->domains); g_tree_destroy(s->endpoints); +virtio_delete_queue(s->req_vq); +virtio_delete_queue(s->event_vq); virtio_cleanup(vdev); } -- 2.18.2
Re: [PATCH v6 25/61] target/riscv: vector single-width averaging add and subtract
On 3/18/20 8:46 PM, LIU Zhiwei wrote:
> +static inline int32_t asub32(CPURISCVState *env, int vxrm, int32_t a,
> int32_t b)
> +{
> +int64_t res = (int64_t)a - b;
> +uint8_t round = get_round(vxrm, res, 1);
> +
> +return (res >> 1) + round;
> +}
> +
>
> I find a corner case here. As the spec said in Section 13.2
>
> "There can be no overflow in the result".
>
> If the a is 0x7fff, b is 0x8000, and the round mode is round to
> up(rnu),
> then the result is (0x7fff - 0x8000 + 1) >> 1, equals 0x8000,
> according the v0.7.1
That's why we used int64_t as the intermediate type:
0x7fff - 0x8000 + 1
= 0x7fff + 0x8000 + 1
= 0x + 1
= 0x0001
Shift that right by 1 and you do indeed get 0x8000.
There's no saturation involved.
For int64_t we computed signed overflow to do the same thing.
r~
Re: [PATCH v3 2/2] virtio-iommu: delete vqs in unrealize to fix memleak
On 3/28/2020 12:26 AM, Stefano Garzarella wrote: > On Fri, Mar 27, 2020 at 05:56:42PM +0800, Pan Nengyuan wrote: >> req_vq/event_vq forgot to free in unrealize. Fix that. >> And aslo do clean 's->as_by_busptr' hash table in unrealize to fix another >> leak. > > s/aslo/also > > Maybe we can also update the subject in something like this: > "virtio-iommu: avoid memleak in the unrealize" OK. > >> >> Signed-off-by: Pan Nengyuan >> Acked-by: Eric Auger >> --- >> Cc: Eric Auger >> --- >> v3->v1/v2: >> - Aslo clean 's->as_by_busptr' hash table in unrealize.(Suggested by Stefano >> Garzarella) >> --- >> hw/virtio/virtio-iommu.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c >> index 4cee8083bc..694698fa0f 100644 >> --- a/hw/virtio/virtio-iommu.c >> +++ b/hw/virtio/virtio-iommu.c >> @@ -696,7 +696,10 @@ static void virtio_iommu_device_unrealize(DeviceState >> *dev, Error **errp) >> g_tree_destroy(s->domains); >> g_tree_destroy(s->endpoints); >> >> +virtio_delete_queue(s->req_vq); >> +virtio_delete_queue(s->event_vq); >> virtio_cleanup(vdev); >> +g_hash_table_destroy(s->as_by_busptr); > > If you need to respin, you could move g_hash_table_destroy() > at the beggining of unrealize(), just to follow a reverse order of > realize(). OK. Thanks. > > Thanks, > Stefano > >> } >> >> static void virtio_iommu_device_reset(VirtIODevice *vdev) >> -- >> 2.18.2 >> >> >
[Bug 1868116] Re: QEMU monitor no longer works
This bug was fixed in the package vte2.91 - 0.60.0-2ubuntu2 --- vte2.91 (0.60.0-2ubuntu2) focal; urgency=medium * debian/libvte-2.91-0.install - Dropped files duplicated in libvte-2.91-common * debian/control.in - Add appropriate Breaks/Replaces for moved files. -- Ken VanDine Fri, 27 Mar 2020 16:07:28 -0400 ** Changed in: vte2.91 (Ubuntu) Status: Triaged => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1868116 Title: QEMU monitor no longer works Status in QEMU: New Status in qemu package in Ubuntu: Triaged Status in vte2.91 package in Ubuntu: Fix Released Bug description: Repro: VTE $ meson _build && ninja -C _build && ninja -C _build install qemu: $ ../configure --python=/usr/bin/python3 --disable-werror --disable-user --disable-linux-user --disable-docs --disable-guest-agent --disable-sdl --enable-gtk --disable-vnc --disable-xen --disable-brlapi --disable-fdt --disable-hax --disable-vde --disable-netmap --disable-rbd --disable-libiscsi --disable-libnfs --disable-smartcard --disable-libusb --disable-usb-redir --disable-seccomp --disable-glusterfs --disable-tpm --disable-numa --disable-opengl --disable-virglrenderer --disable-xfsctl --disable-vxhs --disable-slirp --disable-blobs --target-list=x86_64-softmmu --disable-rdma --disable-pvrdma --disable-attr --disable-vhost-net --disable-vhost-vsock --disable-vhost-scsi --disable-vhost-crypto --disable-vhost-user --disable-spice --disable-qom-cast-debug --disable-vxhs --disable-bochs --disable-cloop --disable-dmg --disable-qcow1 --disable-vdi --disable-vvfat --disable-qed --disable-parallels --disable-sheepdog --disable-avx2 --disable-nettle --disable-gnutls --disable-capstone --disable-tools --disable-libpmem --disable-iconv --disable-cap-ng $ make Test: $ LD_LIBRARY_PATH=/usr/local/lib/x86_64-linux-gnu/:$LD_LIBRARY_PATH ./build/x86_64-softmmu/qemu-system-x86_64 -enable-kvm --drive media=cdrom,file=http://archive.ubuntu.com/ubuntu/dists/bionic/main/installer-amd64/current/images/netboot/mini.iso - switch to monitor with CTRL+ALT+2 - try to enter something Affects head of both usptream git repos. --- original bug --- It was observed that the QEMU console (normally accessible using Ctrl+Alt+2) accepts no input, so it can't be used. This is being problematic because there are cases where it's required to send commands to the guest, or key combinations that the host would grab (as Ctrl-Alt-F1 or Alt-F4). ProblemType: Bug DistroRelease: Ubuntu 20.04 Package: qemu 1:4.2-3ubuntu2 Uname: Linux 5.6.0-rc6+ x86_64 ApportVersion: 2.20.11-0ubuntu20 Architecture: amd64 CurrentDesktop: XFCE Date: Thu Mar 19 12:16:31 2020 Dependencies: InstallationDate: Installed on 2017-06-13 (1009 days ago) InstallationMedia: Xubuntu 17.04 "Zesty Zapus" - Release amd64 (20170412) KvmCmdLine: COMMAND STAT EUID RUID PIDPPID %CPU COMMAND qemu-system-x86 Sl+ 1000 1000 34275 25235 29.2 qemu-system-x86_64 -m 4G -cpu Skylake-Client -device virtio-vga,virgl=true,xres=1280,yres=720 -accel kvm -device nec-usb-xhci -serial vc -serial stdio -hda /home/usuario/Sistemas/androidx86.img -display gtk,gl=on -device usb-audio kvm-nx-lpage-re S0 0 34284 2 0.0 [kvm-nx-lpage-re] kvm-pit/34275 S0 0 34286 2 0.0 [kvm-pit/34275] MachineType: LENOVO 80UG ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-5.6.0-rc6+ root=UUID=6b4ae5c0-c78c-49a6-a1ba-029192618a7a ro quiet ro kvm.ignore_msrs=1 kvm.report_ignored_msrs=0 kvm.halt_poll_ns=0 kvm.halt_poll_ns_grow=0 i915.enable_gvt=1 i915.fastboot=1 cgroup_enable=memory swapaccount=1 zswap.enabled=1 zswap.zpool=z3fold resume=UUID=a82e38a0-8d20-49dd-9cbd-de7216b589fc log_buf_len=16M usbhid.quirks=0x0079:0x0006:0x10 config_scsi_mq_default=y scsi_mod.use_blk_mq=1 mtrr_gran_size=64M mtrr_chunk_size=64M nbd.nbds_max=2 nbd.max_part=63 SourcePackage: qemu UpgradeStatus: Upgraded to focal on 2019-12-22 (87 days ago) dmi.bios.date: 08/09/2018 dmi.bios.vendor: LENOVO dmi.bios.version: 0XCN45WW dmi.board.asset.tag: NO Asset Tag dmi.board.name: Toronto 4A2 dmi.board.vendor: LENOVO dmi.board.version: SDK0J40679 WIN dmi.chassis.asset.tag: NO Asset Tag dmi.chassis.type: 10 dmi.chassis.vendor: LENOVO dmi.chassis.version: Lenovo ideapad 310-14ISK dmi.modalias: dmi:bvnLENOVO:bvr0XCN45WW:bd08/09/2018:svnLENOVO:pn80UG:pvrLenovoideapad310-14ISK:rvnLENOVO:rnToronto4A2:rvrSDK0J40679WIN:cvnLENOVO:ct10:cvrLenovoideapad310-14ISK: dmi.product.family: IDEAPAD dmi.product.name: 80UG dmi.product.sku: LENOVO_MT_80UG_BU_idea_FM_Lenovo ideapad 310-14ISK dmi.product.version: Lenovo ideapad 310-14ISK dmi.sys.vendor: LENOVO mtime.conffile..etc.apport.crashdb.conf: 2019-08-29T08:39:36.787240 To manage notifications about this bug go to:
Re: [PATCH v6 24/61] target/riscv: vector single-width saturating add and subtract
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 33 ++ > target/riscv/insn32.decode | 10 + > target/riscv/insn_trans/trans_rvv.inc.c | 16 + > target/riscv/vector_helper.c| 389 > 4 files changed, 448 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 23/61] target/riscv: vector integer merge and move instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +if (s->vl_eq_vlmax) {
> +#ifdef TARGET_RISCV64
> +tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
> + MAXSZ(s), MAXSZ(s), s1);
> +#else
> +tcg_gen_gvec_dup_i32(s->sew, vreg_ofs(s, a->rd),
> + MAXSZ(s), MAXSZ(s), s1);
> +#endif
Note to self: Add tcg_gen_gvec_dup_tl to tcg-op-gvec.h.
> +switch (s->sew) {
> +case 0:
> +tcg_gen_gvec_dup8i(vreg_ofs(s, a->rd),
> + MAXSZ(s), MAXSZ(s), simm);
> +break;
> +case 1:
> +tcg_gen_gvec_dup16i(vreg_ofs(s, a->rd),
> +MAXSZ(s), MAXSZ(s), simm);
> +break;
> +case 2:
> +tcg_gen_gvec_dup32i(vreg_ofs(s, a->rd),
> +MAXSZ(s), MAXSZ(s), simm);
> +break;
> +default:
> +tcg_gen_gvec_dup64i(vreg_ofs(s, a->rd),
> +MAXSZ(s), MAXSZ(s), simm);
> +break;
> +}
Note to self: Add tcg_gen_gvec_dup_imm(vece, ...).
Neither are your problem, but we should remember to update this code.
Reviewed-by: Richard Henderson
r~
Re: [PATCH v6 18/61] target/riscv: vector single-width integer multiply instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +static int64_t do_mulhsu_d(int64_t s2, uint64_t s1)
> +{
> +uint64_t hi_64, lo_64, abs_s2 = s2;
> +
> +if (s2 < 0) {
> +abs_s2 = -s2;
> +}
> +mulu64(_64, _64, abs_s2, s1);
> +if (s2 < 0) {
> +lo_64 = ~lo_64;
> +hi_64 = ~hi_64;
> +if (lo_64 == UINT64_MAX) {
> +lo_64 = 0;
> +hi_64 += 1;
> +} else {
> +lo_64 += 1;
> +}
> +}
> +
> +return hi_64;
> +}
Missed the improvement here. See tcg_gen_mulsu2_i64.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH v6 12/61] target/riscv: vector integer add-with-carry / subtract-with-borrow instructions
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 33 ++ > target/riscv/insn32.decode | 11 ++ > target/riscv/insn_trans/trans_rvv.inc.c | 78 + > target/riscv/vector_helper.c| 148 > 4 files changed, 270 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 10/61] target/riscv: vector single-width integer add and subtract
On 3/17/20 8:06 AM, LIU Zhiwei wrote:
> +if (a->vm && s->vl_eq_vlmax) {
> +gvec_fn(s->sew, vreg_ofs(s, a->rd),
> +vreg_ofs(s, a->rs2), vreg_ofs(s, a->rs1),
> +MAXSZ(s), MAXSZ(s));
Indentation is off here.
> +static inline bool
> +do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn *gvec_fn,
> + gen_helper_opivx *fn)
> +{
> +if (!opivx_check(s, a)) {
> +return false;
> +}
> +
> +if (a->vm && s->vl_eq_vlmax) {
> +TCGv_i64 src1 = tcg_temp_new_i64();
> +TCGv tmp = tcg_temp_new();
> +
> +gen_get_gpr(tmp, a->rs1);
> +tcg_gen_ext_tl_i64(src1, tmp);
> +gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2),
> +src1, MAXSZ(s), MAXSZ(s));
> +
> +tcg_temp_free_i64(src1);
> +tcg_temp_free(tmp);
> +return true;
> +} else {
> +return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
> +}
> +return true;
> +}
This final return is unreachable, and I'm sure some static analyzer (e.g.
Coverity) will complain.
Since the if-then has a return, we can drop the else like so:
if (a->vm && s->vl_eq_vlmax) {
...
return true;
}
return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH v6 09/61] target/riscv: add vector amo operations
On 3/17/20 8:06 AM, LIU Zhiwei wrote: > Vector AMOs operate as if aq and rl bits were zero on each element > with regard to ordering relative to other instructions in the same hart. > Vector AMOs provide no ordering guarantee between element operations > in the same vector AMO instruction > > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 29 + > target/riscv/insn32-64.decode | 11 ++ > target/riscv/insn32.decode | 13 +++ > target/riscv/insn_trans/trans_rvv.inc.c | 134 ++ > target/riscv/internals.h| 1 + > target/riscv/vector_helper.c| 143 > 6 files changed, 331 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v6 05/61] target/riscv: add an internals.h header
On 3/17/20 8:05 AM, LIU Zhiwei wrote: > The internals.h keeps things that are not relevant to the actual architecture, > only to the implementation, separate. > > Signed-off-by: LIU Zhiwei > --- > target/riscv/internals.h | 24 > 1 file changed, 24 insertions(+) > create mode 100644 target/riscv/internals.h Reviewed-by: Richard Henderson r~
[PATCH] softfloat: Fix BAD_SHIFT from normalizeFloatx80Subnormal
All other calls to normalize*Subnormal detect zero input before
the call -- this is the only outlier. This case can happen with
+0.0 + +0.0 = +0.0 or -0.0 + -0.0 = -0.0, so return a zero of
the correct sign.
Reported-by: Coverity (CID 1421991)
Signed-off-by: Richard Henderson
---
fpu/softfloat.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 301ce3b537..ae6ba71854 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5856,6 +5856,9 @@ static floatx80 addFloatx80Sigs(floatx80 a, floatx80 b,
flag zSign,
zSig1 = 0;
zSig0 = aSig + bSig;
if ( aExp == 0 ) {
+if (zSig0 == 0) {
+return packFloatx80(zSign, 0, 0);
+}
normalizeFloatx80Subnormal( zSig0, , );
goto roundAndPack;
}
--
2.20.1
Re: [PATCH] qemu-user: fix build with LLVM lld 10
On 3/27/20 3:51 AM, Laurent Vivier wrote: >> The Plan is still to drop this whole section of code. >> >> However, it's still blocked on getting the x86_64 vsyscall patches upstream. > > Richard, > > will you propose another fix to fix build with LLVM lld 10? Sent. r~
Re: [PATCH v1 5/7] fpu/softfloat: avoid undefined behaviour when normalising empty sigs
On Fri, 27 Mar 2020 at 22:27, Richard Henderson wrote: > I wonder if I have the energy to petition the committee to drop, for C202? all > of the "undefined" nonsense that only applies to sign-magnitute and > ones-compliment computers, which haven't been seen since the 70's... There was certainly a proposal to do that (I think from a Google engineer) for C++, I forget whether the equivalent C change has also been proposed. > > That said, is it valid for this function to be called with a zero > > aSig value ? I think all these normalizeFloat*Subnormal() functions > > assume non-zero sig input, and the only callsite where it's not clearly > > obvious that this is obvious that the sig input is non-zero is the call to > > normalizeFloatx80Subnormal() from addFloatx80Sigs(). So perhaps we > > just need to check and fix that callsite ?? > > You're right -- addFloatx80Sigs is the only use out of 26 that doesn't have a > preceding check for 0. Mmm. My vote is for fixing addFloatx80Sigs -- now we just need to figure out what the desired behaviour is. thanks -- PMM
Re: [PATCH v1 5/7] fpu/softfloat: avoid undefined behaviour when normalising empty sigs
On 3/27/20 3:09 AM, Peter Maydell wrote:
> On Fri, 27 Mar 2020 at 09:49, Alex Bennée wrote:
>>
>> The undefined behaviour checker pointed out that a shift of 64 would
>> lead to undefined behaviour.
>>
>> Signed-off-by: Alex Bennée
>> ---
>> fpu/softfloat.c | 11 ---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index 301ce3b537b..444d35920dd 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -3834,9 +3834,14 @@ void normalizeFloatx80Subnormal(uint64_t aSig,
>> int32_t *zExpPtr,
>> {
>> int8_t shiftCount;
>>
>> -shiftCount = clz64(aSig);
>> -*zSigPtr = aSig<> -*zExpPtr = 1 - shiftCount;
>> +if (aSig) {
>> +shiftCount = clz64(aSig);
>> +*zSigPtr = aSig << shiftCount;
>> +*zExpPtr = 1 - shiftCount;
>> +} else {
>> +*zSigPtr = 0;
>> +*zExpPtr = 1 - 64;
>> +}
>> }
>
> Ah yes, I saw this one in Coverity: CID 1421991.
>
> RTH marked the Coverity issue as a false positive with the rationale
> "We assume an out-of-range shift count is merely IMPLEMENTATION DEFINED
> and not UNDEFINED (in the Arm ARM sense), and so cannot turn a 0 value
> into a non-zero value."
> but I think I disagree with that. We can assume that for the TCG IR
> where we get to define shift semantics because we're doing the codegen,
> but we can't assume it in C code, because it's not included in the set
> of extended guarantees provided by -fwrapv as far as I know.
Perhaps. Of course we also know from our broad knowledge of architectures,
that a compiler would really have to go out of its way for this to happen.
I really hate C in this way, sometimes.
I wonder if I have the energy to petition the committee to drop, for C202? all
of the "undefined" nonsense that only applies to sign-magnitute and
ones-compliment computers, which haven't been seen since the 70's...
> That said, is it valid for this function to be called with a zero
> aSig value ? I think all these normalizeFloat*Subnormal() functions
> assume non-zero sig input, and the only callsite where it's not clearly
> obvious that this is obvious that the sig input is non-zero is the call to
> normalizeFloatx80Subnormal() from addFloatx80Sigs(). So perhaps we
> just need to check and fix that callsite ??
You're right -- addFloatx80Sigs is the only use out of 26 that doesn't have a
preceding check for 0.
r~
Re: [PATCH v1] s390x: Reject unaligned RAM sizes
On Fri, 27 Mar 2020 17:53:39 +0100
David Hildenbrand wrote:
> On 27.03.20 17:46, Igor Mammedov wrote:
> > On Fri, 27 Mar 2020 17:05:34 +0100
> > Christian Borntraeger wrote:
> >
> >> On 27.03.20 17:01, David Hildenbrand wrote:
> >>> On 27.03.20 16:34, Christian Borntraeger wrote:
>
>
> On 27.03.20 16:29, David Hildenbrand wrote:
> > Historically, we fixed up the RAM size (rounded it down), to fit into
> > storage increments. Since commit 3a12fc61af5c ("390x/s390-virtio-ccw:
> > use
> > memdev for RAM"), we no longer consider the fixed-up size when
> > allcoating the RAM block - which will break migration.
> >
> > Let's simply drop that manual fixup code and let the user supply sane
> > RAM sizes. This will bail out early when trying to migrate (and make
> > an existing guest with e.g., 12345 MB non-migratable), but maybe we
> > should have rejected such RAM sizes right from the beginning.
> >
> > As we no longer fixup maxram_size as well, make other users use ram_size
> > instead. Keep using maxram_size when setting the maximum ram size in
> > KVM,
> > as that will come in handy in the future when supporting memory hotplug
> > (in contrast, storage keys and storage attributes for hotplugged memory
> > will have to be migrated per RAM block in the future).
> >
> > This fixes (or rather rejects early):
> >
> > 1. Migrating older QEMU to upstream QEMU (e.g., with "-m 1235M"), as the
> >RAM block size changed.
>
> Not sure I like this variant. Instead of breaking migration (that was
> accidentially done by Igors changes) we now reject migration from older
> QEMUs to 5.0. This is not going to help those that still have such guests
> running and want to migrate.
> >>>
> >>> As Igor mentioned on another channel, you most probably can migrate an
> >>> older guest by starting it on the target with a fixed-up size.
> >>>
> >>> E.g., migrate an old QEMU "-m 1235M" to a new QEMU "-m 1234M"
> >>
> >> Yes, that should probably work.
> > I'm in process of testing it.
it works
> >
> >>> Not sure how many such weird-size VMs we actually do have in practice.
> >>
> >> I am worried about some automated deployments where tooling has created
> >> these sizes for dozens or hundreds of containers in VMS and so.
>
> IIRC, e.g., Kata usually uses 2048MB. Not sure about others, but I'd be
> surprised if it's not multiples of, say, 128MB.
>
> > Yep, it's possible but then that tooling/configs should be fixed to work
> > with
> > new QEMU that validates user's input.
> >
>
> Yeah, and mention it in the cover letter, +eventually a "fixup" table
> (e.g., old_size < X, has to be aligned to Y).
>
> One alternative is to have an early fixup hack in QEMU, that fixes up
> the sizes as we did before (and eventually warns the user). Not sure if
> we really want/need that.
That would require at least a callback at machine level,
also practice shows warnings are of no use.
If someone insist on using wrong size with new QEMU, one can write a wrapper
script to workaround the issue (if they don't wish to fix configs/tooling).
I don't like keeping hacks to fix user mistakes in QEMU and on top of that
adding infrastructure for that (QEMU is already too much complicated).
Since in this case it break migration only partially, it's sufficient
to print error message that suggest correct size to use (like it's been done
for other boards, s390 is the last remaining that is doing ram_size fixups).
That way user could fix config and restart migration.
Re: [PATCH v1 3/7] tests/tcg: remove extraneous pasting macros
On 3/27/20 2:49 AM, Alex Bennée wrote: > We are not using them and they just get in the way. > > Signed-off-by: Alex Bennée > --- > tests/tcg/x86_64/system/boot.S | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
[PATCH for-5.0 v3 6/7] configure: Override the os default with --disable-pie
Some distributions, e.g. Ubuntu 19.10, enable PIE by default. If for some reason one wishes to build a non-pie binary, we must provide additional options to override. At the same time, reorg the code to an elif chain. Reviewed-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Signed-off-by: Richard Henderson --- v3: Update for QEMU_LDFLAGS. --- configure | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/configure b/configure index 4dead416ae..b304bd0cf8 100755 --- a/configure +++ b/configure @@ -2124,19 +2124,18 @@ if compile_prog "-Werror -fno-pie" "-no-pie"; then LDFLAGS_NOPIE="-no-pie" fi -if test "$pie" != "no" ; then - if compile_prog "-fPIE -DPIE" "-pie"; then -QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS" -QEMU_LDFLAGS="-pie $QEMU_LDFLAGS" -pie="yes" - else -if test "$pie" = "yes"; then - error_exit "PIE not available due to missing toolchain support" -else - echo "Disabling PIE due to missing toolchain support" - pie="no" -fi - fi +if test "$pie" = "no"; then + QEMU_CFLAGS="$CFLAGS_NOPIE $QEMU_CFLAGS" + QEMU_LDFLAGS="$LDFLAGS_NOPIE $QEMU_LDFLAGS" +elif compile_prog "-fPIE -DPIE" "-pie"; then + QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS" + QEMU_LDFLAGS="-pie $QEMU_LDFLAGS" + pie="yes" +elif test "$pie" = "yes"; then + error_exit "PIE not available due to missing toolchain support" +else + echo "Disabling PIE due to missing toolchain support" + pie="no" fi # Detect support for PT_GNU_RELRO + DT_BIND_NOW. -- 2.20.1
Re: [PATCH v1 2/7] linux-user: protect fcntl64 with an #ifdef
On 3/27/20 2:49 AM, Alex Bennée wrote: > Checking TARGET_ABI_BITS is sketchy - we should check for the presence > of the define to be sure. Also clean up the white space while we are > there. > > Signed-off-by: Alex Bennée > --- > linux-user/syscall.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
[PATCH for-5.0 v3 4/7] configure: Always detect -no-pie toolchain support
The CFLAGS_NOPIE and LDFLAGS_NOPIE variables are used
in pc-bios/optionrom/Makefile, which has nothing to do
with the PIE setting of the main qemu executables.
This overrides any operating system default to build
all executables as PIE, which is important for ROMs.
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Thomas Huth
Signed-off-by: Richard Henderson
---
configure | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/configure b/configure
index cbde833f6e..7ba18783cb 100755
--- a/configure
+++ b/configure
@@ -2107,26 +2107,24 @@ if ! compile_prog "-Werror" "" ; then
"Thread-Local Storage (TLS). Please upgrade to a version that does."
fi
-if test "$pie" != "no" ; then
- cat > $TMPC << EOF
+cat > $TMPC << EOF
#ifdef __linux__
# define THREAD __thread
#else
# define THREAD
#endif
-
static THREAD int tls_var;
-
int main(void) { return tls_var; }
-
EOF
- # check we support --no-pie first...
- if compile_prog "-Werror -fno-pie" "-no-pie"; then
-CFLAGS_NOPIE="-fno-pie"
-LDFLAGS_NOPIE="-nopie"
- fi
+# Check we support --no-pie first; we will need this for building ROMs.
+if compile_prog "-Werror -fno-pie" "-no-pie"; then
+ CFLAGS_NOPIE="-fno-pie"
+ LDFLAGS_NOPIE="-no-pie"
+fi
+
+if test "$pie" != "no" ; then
if compile_prog "-fPIE -DPIE" "-pie"; then
QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
QEMU_LDFLAGS="-pie $QEMU_LDFLAGS"
--
2.20.1
Re: [PATCH v1 1/7] elf-ops: bail out if we have no function symbols
On 3/27/20 2:49 AM, Alex Bennée wrote: > It's perfectly possible to have no function symbols in your elf file > and if we do the undefined behaviour sanitizer rightly complains about > us passing NULL to qsort. Check nsyms before we go ahead. > > Signed-off-by: Alex Bennée > --- > include/hw/elf_ops.h | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
[PATCH for-5.0 v3 3/7] configure: Do not force pie=no for non-x86
PIE is supported on many other hosts besides x86. The default for non-x86 is now the same as x86: pie is used if supported, and may be forced via --enable/--disable-pie. The original commit (40d6444e91c) said: "Non-x86 are not changed, as they require TCG changes" but I think that's wrong -- there's nothing about PIE that affects TCG one way or another. Tested on aarch64 (bionic) and ppc64le (centos 7) hosts. Tested-by: Alex Bennée Reviewed-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- configure | 10 -- 1 file changed, 10 deletions(-) diff --git a/configure b/configure index 2afcae61df..cbde833f6e 100755 --- a/configure +++ b/configure @@ -2107,16 +2107,6 @@ if ! compile_prog "-Werror" "" ; then "Thread-Local Storage (TLS). Please upgrade to a version that does." fi -if test "$pie" = ""; then - case "$cpu-$targetos" in -i386-Linux|x86_64-Linux|x32-Linux|i386-OpenBSD|x86_64-OpenBSD) - ;; -*) - pie="no" - ;; - esac -fi - if test "$pie" != "no" ; then cat > $TMPC << EOF -- 2.20.1
[PATCH for-5.0 v3 2/7] tcg: Remove softmmu code_gen_buffer fixed address
The commentary talks about "in concert with the addresses
assigned in the relevant linker script", except there is no
linker script for softmmu, nor has there been for some time.
(Do not confuse the user-only linker script editing that was
removed in the previous patch, because user-only does not
use this code_gen_buffer allocation method.)
Reviewed-by: Alex Bennée
Reviewed-by: Thomas Huth
Signed-off-by: Richard Henderson
---
accel/tcg/translate-all.c | 37 +
1 file changed, 5 insertions(+), 32 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 78914154bf..9924e66d1f 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1043,47 +1043,20 @@ static inline void *alloc_code_gen_buffer(void)
{
int prot = PROT_WRITE | PROT_READ | PROT_EXEC;
int flags = MAP_PRIVATE | MAP_ANONYMOUS;
-uintptr_t start = 0;
size_t size = tcg_ctx->code_gen_buffer_size;
void *buf;
-/* Constrain the position of the buffer based on the host cpu.
- Note that these addresses are chosen in concert with the
- addresses assigned in the relevant linker script file. */
-# if defined(__PIE__) || defined(__PIC__)
-/* Don't bother setting a preferred location if we're building
- a position-independent executable. We're more likely to get
- an address near the main executable if we let the kernel
- choose the address. */
-# elif defined(__x86_64__) && defined(MAP_32BIT)
-/* Force the memory down into low memory with the executable.
- Leave the choice of exact location with the kernel. */
-flags |= MAP_32BIT;
-/* Cannot expect to map more than 800MB in low memory. */
-if (size > 800u * 1024 * 1024) {
-tcg_ctx->code_gen_buffer_size = size = 800u * 1024 * 1024;
-}
-# elif defined(__sparc__)
-start = 0x4000ul;
-# elif defined(__s390x__)
-start = 0x9000ul;
-# elif defined(__mips__)
-# if _MIPS_SIM == _ABI64
-start = 0x12800ul;
-# else
-start = 0x0800ul;
-# endif
-# endif
-
-buf = mmap((void *)start, size, prot, flags, -1, 0);
+buf = mmap(NULL, size, prot, flags, -1, 0);
if (buf == MAP_FAILED) {
return NULL;
}
#ifdef __mips__
if (cross_256mb(buf, size)) {
-/* Try again, with the original still mapped, to avoid re-acquiring
- that 256mb crossing. This time don't specify an address. */
+/*
+ * Try again, with the original still mapped, to avoid re-acquiring
+ * the same 256mb crossing.
+ */
size_t size2;
void *buf2 = mmap(NULL, size, prot, flags, -1, 0);
switch ((int)(buf2 != MAP_FAILED)) {
--
2.20.1
[PATCH for-5.0 v3 1/7] configure: Drop adjustment of textseg
This adjustment was random and unnecessary. The user mode startup code in probe_guest_base() will choose a value for guest_base that allows the host qemu binary to not conflict with the guest binary. With modern distributions, this isn't even used, as the default is PIE, which does the same job in a more portable way. Reviewed-by: Alex Bennée Reviewed-by: Thomas Huth Signed-off-by: Richard Henderson --- v2: Remove mention of config-host.ld from make distclean --- configure | 47 --- Makefile | 2 +- 2 files changed, 1 insertion(+), 48 deletions(-) diff --git a/configure b/configure index 89fe881dd4..2afcae61df 100755 --- a/configure +++ b/configure @@ -6498,49 +6498,6 @@ if test "$cpu" = "s390x" ; then fi fi -# Probe for the need for relocating the user-only binary. -if ( [ "$linux_user" = yes ] || [ "$bsd_user" = yes ] ) && [ "$pie" = no ]; then - textseg_addr= - case "$cpu" in -arm | i386 | ppc* | s390* | sparc* | x86_64 | x32) - # ??? Rationale for choosing this address - textseg_addr=0x6000 - ;; -mips) - # A 256M aligned address, high in the address space, with enough - # room for the code_gen_buffer above it before the stack. - textseg_addr=0x6000 - ;; - esac - if [ -n "$textseg_addr" ]; then -cat > $TMPC &1; then -error_exit \ -"We need to link the QEMU user mode binaries at a" \ -"specific text address. Unfortunately your linker" \ -"doesn't support either the -Ttext-segment option or" \ -"printing the default linker script with --verbose." \ -"If you don't want the user mode binaries, pass the" \ -"--disable-user option to configure." - fi - - $ld --verbose | sed \ --e '1,/==/d' \ --e '/==/,$d' \ --e "s/[.] = [0-9a-fx]* [+] SIZEOF_HEADERS/. = $textseg_addr + SIZEOF_HEADERS/" \ --e "s/__executable_start = [0-9a-fx]*/__executable_start = $textseg_addr/" > config-host.ld - textseg_ldflags="-Wl,-T../config-host.ld" -fi - fi -fi - # Check that the C++ compiler exists and works with the C compiler. # All the QEMU_CXXFLAGS are based on QEMU_CFLAGS. Keep this at the end to don't miss any other that could be added. if has $cxx; then @@ -8175,10 +8132,6 @@ if test "$gprof" = "yes" ; then fi fi -if test "$target_linux_user" = "yes" || test "$target_bsd_user" = "yes" ; then - ldflags="$ldflags $textseg_ldflags" -fi - # Newer kernels on s390 check for an S390_PGSTE program header and # enable the pgste page table extensions in that case. This makes # the vm.allocate_pgste sysctl unnecessary. We enable this program diff --git a/Makefile b/Makefile index fc2808fb4b..84ef881600 100644 --- a/Makefile +++ b/Makefile @@ -795,7 +795,7 @@ rm -f $(MANUAL_BUILDDIR)/$1/objects.inv $(MANUAL_BUILDDIR)/$1/searchindex.js $(M endef distclean: clean - rm -f config-host.mak config-host.h* config-host.ld $(DOCS) + rm -f config-host.mak config-host.h* $(DOCS) rm -f tests/tcg/config-*.mak rm -f config-all-devices.mak config-all-disas.mak config.status rm -f $(SUBDIR_DEVICES_MAK) -- 2.20.1
[PATCH for-5.0 v3 7/7] configure: Support -static-pie if requested
Recent toolchains support static and pie at the same time. As with normal dynamic builds, allow --static to default to PIE if supported by the toolchain. Allow --enable/--disable-pie to override the default. Reviewed-by: Alex Bennée Signed-off-by: Richard Henderson --- v2: Fix --disable-pie --static v3: Update for QEMU_LDFLAGS. --- configure | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/configure b/configure index b304bd0cf8..a16ae9a2be 100755 --- a/configure +++ b/configure @@ -1067,7 +1067,6 @@ for opt do ;; --static) static="yes" -QEMU_LDFLAGS="-static $QEMU_LDFLAGS" QEMU_PKG_CONFIG_FLAGS="--static $QEMU_PKG_CONFIG_FLAGS" ;; --mandir=*) mandir="$optarg" @@ -2089,11 +2088,6 @@ if test "$static" = "yes" ; then if test "$modules" = "yes" ; then error_exit "static and modules are mutually incompatible" fi - if test "$pie" = "yes" ; then -error_exit "static and pie are mutually incompatible" - else -pie="no" - fi fi # Unconditional check for compiler __thread support @@ -2124,7 +2118,18 @@ if compile_prog "-Werror -fno-pie" "-no-pie"; then LDFLAGS_NOPIE="-no-pie" fi -if test "$pie" = "no"; then +if test "$static" = "yes"; then + if test "$pie" != "no" && compile_prog "-fPIE -DPIE" "-static-pie"; then +QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS" +QEMU_LDFLAGS="-static-pie $QEMU_LDFLAGS" +pie="yes" + elif test "$pie" = "yes"; then +error_exit "-static-pie not available due to missing toolchain support" + else +QEMU_LDFLAGS="-static $QEMU_LDFLAGS" +pie="no" + fi +elif test "$pie" = "no"; then QEMU_CFLAGS="$CFLAGS_NOPIE $QEMU_CFLAGS" QEMU_LDFLAGS="$LDFLAGS_NOPIE $QEMU_LDFLAGS" elif compile_prog "-fPIE -DPIE" "-pie"; then -- 2.20.1
[PATCH for-5.0 v3 5/7] configure: Unnest detection of -z, relro and -z, now
There is nothing about these options that is related to PIE. Use them unconditionally. Reviewed-by: Alex Bennée Reviewed-by: Fangrui Song Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- v2: Do not split into two tests. v3: Update to QEMU_LDFLAGS. --- configure | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/configure b/configure index 7ba18783cb..4dead416ae 100755 --- a/configure +++ b/configure @@ -2129,9 +2129,6 @@ if test "$pie" != "no" ; then QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS" QEMU_LDFLAGS="-pie $QEMU_LDFLAGS" pie="yes" -if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then - QEMU_LDFLAGS="-Wl,-z,relro -Wl,-z,now $QEMU_LDFLAGS" -fi else if test "$pie" = "yes"; then error_exit "PIE not available due to missing toolchain support" @@ -2142,6 +2139,12 @@ if test "$pie" != "no" ; then fi fi +# Detect support for PT_GNU_RELRO + DT_BIND_NOW. +# The combination is known as "full relro", because .got.plt is read-only too. +if compile_prog "" "-Wl,-z,relro -Wl,-z,now" ; then + QEMU_LDFLAGS="-Wl,-z,relro -Wl,-z,now $QEMU_LDFLAGS" +fi + ## # __sync_fetch_and_and requires at least -march=i486. Many toolchains # use i686 as default anyway, but for those that don't, an explicit -- 2.20.1
[PATCH for-5.0 v3 0/7] configure: Improve PIE and other linkage
Version 3 is a rebase from January, with s/LDCONFIG/QEMU_LDCONFIG/. I plan on including this in the next tcg-next pull request, if there is no other commentary. r~ Richard Henderson (7): configure: Drop adjustment of textseg tcg: Remove softmmu code_gen_buffer fixed address configure: Do not force pie=no for non-x86 configure: Always detect -no-pie toolchain support configure: Unnest detection of -z,relro and -z,now configure: Override the os default with --disable-pie configure: Support -static-pie if requested configure | 116 +++--- Makefile | 2 +- accel/tcg/translate-all.c | 37 ++-- 3 files changed, 38 insertions(+), 117 deletions(-) -- 2.20.1
Re: [PATCH v1] s390x: Reject unaligned RAM sizes
On Fri, 27 Mar 2020 17:51:23 +0100
David Hildenbrand wrote:
> On 27.03.20 17:48, Igor Mammedov wrote:
> > On Fri, 27 Mar 2020 16:29:30 +0100
> > David Hildenbrand wrote:
> >
> >> Historically, we fixed up the RAM size (rounded it down), to fit into
> >> storage increments. Since commit 3a12fc61af5c ("390x/s390-virtio-ccw: use
> >> memdev for RAM"), we no longer consider the fixed-up size when
> >> allcoating the RAM block - which will break migration.
> >>
> >> Let's simply drop that manual fixup code and let the user supply sane
> >> RAM sizes. This will bail out early when trying to migrate (and make
> >> an existing guest with e.g., 12345 MB non-migratable), but maybe we
> >> should have rejected such RAM sizes right from the beginning.
> >>
> >> As we no longer fixup maxram_size as well, make other users use ram_size
> >> instead. Keep using maxram_size when setting the maximum ram size in KVM,
> >> as that will come in handy in the future when supporting memory hotplug
> >> (in contrast, storage keys and storage attributes for hotplugged memory
> >> will have to be migrated per RAM block in the future).
> >>
> >> This fixes (or rather rejects early):
> >>
> >> 1. Migrating older QEMU to upstream QEMU (e.g., with "-m 1235M"), as the
> >>RAM block size changed.
> >>
> >> 2. Migrating upstream QEMU to upstream QEMU (e.g., with "-m 1235M"), as
> >>we receive storage attributes for memory we don't expect (as we fixed up
> >>ram_size and maxram_size).
> >>
> >> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
> >> Reported-by: Lukáš Doktor
> >> Cc: Igor Mammedov
> >> Cc: Dr. David Alan Gilbert
> >> Signed-off-by: David Hildenbrand
> >> ---
> >> hw/s390x/s390-skeys.c| 4 +---
> >> hw/s390x/s390-stattrib-kvm.c | 7 ++-
> >> hw/s390x/sclp.c | 21 +++--
> >> 3 files changed, 14 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> >> index 5da6e5292f..2545b1576b 100644
> >> --- a/hw/s390x/s390-skeys.c
> >> +++ b/hw/s390x/s390-skeys.c
> >> @@ -11,7 +11,6 @@
> >>
> >> #include "qemu/osdep.h"
> >> #include "qemu/units.h"
> >> -#include "hw/boards.h"
> >> #include "hw/s390x/storage-keys.h"
> >> #include "qapi/error.h"
> >> #include "qapi/qapi-commands-misc-target.h"
> >> @@ -174,9 +173,8 @@ out:
> >> static void qemu_s390_skeys_init(Object *obj)
> >> {
> >> QEMUS390SKeysState *skeys = QEMU_S390_SKEYS(obj);
> >> -MachineState *machine = MACHINE(qdev_get_machine());
> >>
> >> -skeys->key_count = machine->maxram_size / TARGET_PAGE_SIZE;
> >> +skeys->key_count = ram_size / TARGET_PAGE_SIZE;
> >
> > why are you dropping machine->foo all around and switching to global
> > ram_size?
> > (I'd rather do other way around)
>
> Not sure what the latest best practice is. I can also simply convert to
> machine->ram_size if that's the right thing to do.
My understanding of it was not to use globals if possible.
(I planned on removing global ram_size an leave only machine->ram_size
but that a bit tricky since things tend to explode once a global touched,
so it needs some more thought/patience)
Re: [PATCH v6 5/8] gpiolib: Introduce gpiod_set_config()
On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven wrote: > The GPIO Aggregator will need a method to forward a .set_config() call > to its parent gpiochip. This requires obtaining the gpio_chip and > offset for a given gpio_desc. While gpiod_to_chip() is public, > gpio_chip_hwgpio() is not, so there is currently no method to obtain the > needed GPIO offset parameter. > > Hence introduce a public gpiod_set_config() helper, which invokes the > .set_config() callback through a gpio_desc pointer, like is done for > most other gpio_chip callbacks. > > Rewrite the existing gpiod_set_debounce() helper as a wrapper around > gpiod_set_config(), to avoid duplication. > > Signed-off-by: Geert Uytterhoeven > --- > v6: > - New. Patch applied in preparation for the next kernel cycle so we get Geert's patch stack down. Yours, Linus Walleij
Re: [PATCH v6 5/8] gpiolib: Introduce gpiod_set_config()
On Fri, Mar 27, 2020 at 9:45 AM Geert Uytterhoeven wrote: > On Thu, Mar 26, 2020 at 10:26 PM Linus Walleij > wrote: > > On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven > > wrote: > > > The GPIO Aggregator will need a method to forward a .set_config() call > > > to its parent gpiochip. This requires obtaining the gpio_chip and > > > offset for a given gpio_desc. While gpiod_to_chip() is public, > > > gpio_chip_hwgpio() is not, so there is currently no method to obtain the > > > needed GPIO offset parameter. > > > > > > Hence introduce a public gpiod_set_config() helper, which invokes the > > > .set_config() callback through a gpio_desc pointer, like is done for > > > most other gpio_chip callbacks. > > > > > > Rewrite the existing gpiod_set_debounce() helper as a wrapper around > > > gpiod_set_config(), to avoid duplication. > > > > > > Signed-off-by: Geert Uytterhoeven > > > --- > > > v6: > > > - New. > > > > This is nice, I tried to actually just apply this (you also sent some > > two cleanups that I tried to apply) byt Yue's cleanup patch > > commit d18fddff061d2796525e6d4a958cb3d30aed8efd > > "gpiolib: Remove duplicated function gpio_do_set_config()" > > makes none of them apply :/ > > /me confused. > > That commit was reverted later, so it shouldn't matter. > > I have just verified, and both my full series and just this single > patch, do apply fine to all of current gpio/for-next, linus/master, and > next-20200327. They even apply fine to gpio/for-next before or after > the two cleanups I sent, too. > > What am I missing? Ah I see, it is because my development branch is based on v5.6-rc1. So I have to merge in a later -rc where this revert is applied so that this applies. Yours, Linus Walleij
Re: Error building Qemu 2.12.0 on Fedora 31 GCC 9.2.1 with the below error
On Fri, 27 Mar 2020 at 20:49, Viktor Madarasz wrote: > > Hi > > Im trying to build Qemu 2.12.0 on Fedora 31 with GCC 9.2.1 as this > particular qemu version is the only one working for my > qemu-systems-ppc64 emulation I need > > ./configure runs with no problem but running make breaks at this point. > If you try to build an old QEMU on a new distro like this you're likely to run a bunch of minor build bugs that have been fixed in mainline. You will probably need to trawl through the git history or the mailing list archives to find the relevant commits which fix them (searching for error messages is often a good tactic). You should also choose configure options to minimise the amount of source you build that you don't need. If you only need qemu-system-ppc64, then pass configure the "--target-list=ppc64-softmmu" argument and it will build only that and won't try to build the linux-user code at all. If configure of that vintage supports --disable-tools that is also going to be useful (and if some bit of source that looks uninteresting fails check to see if there's a relevant --disable-something to just stop building it.) thanks -- PMM
[Bug 1869426] Re: 5.0rc0->4.2 serial migraiton
git bisect says:
c9808d602813bce4fada7bf9ecc463aa779b73f7 is the first bad commit
commit c9808d602813bce4fada7bf9ecc463aa779b73f7
Author: Marc-André Lureau
Date: Tue Oct 22 01:02:50 2019 +0200
serial: realize the serial device
Instead of calling serial_realize_core(), use the QDev realize
callback.
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1869426
Title:
5.0rc0->4.2 serial migraiton
Status in QEMU:
New
Bug description:
Migrating from 5.0rc0->4.2 with pc-q35-4.2 we get an error:
Unknown savevm section or instance 'serial' 1
dumping the migration streams it looks like 5.0 is duplicating the
serial migration data:
"serial (26)": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
},
"serial (27)": {
"state": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
}
},
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1869426/+subscriptions
[Bug 1869426] Re: 5.0rc0->4.2 serial migraiton
Marc-Andre: I think you're ending up with two top level objects with
vmsd's
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1869426
Title:
5.0rc0->4.2 serial migraiton
Status in QEMU:
New
Bug description:
Migrating from 5.0rc0->4.2 with pc-q35-4.2 we get an error:
Unknown savevm section or instance 'serial' 1
dumping the migration streams it looks like 5.0 is duplicating the
serial migration data:
"serial (26)": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
},
"serial (27)": {
"state": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
}
},
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1869426/+subscriptions
Re: AIO_WAIT_WHILE questions
Patchew URL: https://patchew.org/QEMU/1242491200.59.1585326983...@webmail.proxmox.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: AIO_WAIT_WHILE questions Message-id: 1242491200.59.1585326983...@webmail.proxmox.com Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === From https://github.com/patchew-project/qemu 77a48a7..127fe86 master -> master Switched to a new branch 'test' a128ad0 AIO_WAIT_WHILE questions === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 26 lines checked Commit a128ad0b1009 (AIO_WAIT_WHILE questions) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/1242491200.59.1585326983...@webmail.proxmox.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Error building Qemu 2.12.0 on Fedora 31 GCC 9.2.1 with the below error
Hi
Im trying to build Qemu 2.12.0 on Fedora 31 with GCC 9.2.1 as this
particular qemu version is the only one working for my
qemu-systems-ppc64 emulation I need
./configure runs with no problem but running make breaks at this point.
--
CC aarch64_be-linux-user/accel/tcg/cpu-exec-common.o
CC aarch64_be-linux-user/accel/tcg/translate-all.o
CC aarch64_be-linux-user/accel/tcg/translator.o
CC aarch64_be-linux-user/accel/tcg/user-exec.o
CC aarch64_be-linux-user/accel/tcg/user-exec-stub.o
CC aarch64_be-linux-user/linux-user/main.o
CC aarch64_be-linux-user/linux-user/syscall.o
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/ioctls.h:176:9:
error: ‘SIOCGSTAMP’ undeclared here (not in a function); did you mean
‘SIOCSRARP’?
176 | IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
| ^~
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/syscall.c:5678:23:
note: in definition of macro ‘IOCTL’
5678 | { TARGET_ ## cmd, cmd, #cmd, access, 0, { __VA_ARGS__ } },
| ^~~
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/ioctls.h:177:9:
error: ‘SIOCGSTAMPNS’ undeclared here (not in a function); did you mean
‘SIOCGSTAMP_OLD’?
177 | IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
| ^~~~
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/syscall.c:5678:23:
note: in definition of macro ‘IOCTL’
5678 | { TARGET_ ## cmd, cmd, #cmd, access, 0, { __VA_ARGS__ } },
| ^~~
make[1]: *** [/home/viktormadarasz/Emulator/qemu-2.12.0/rules.mak:66:
linux-user/syscall.o] Error 1
make: *** [Makefile:478: subdir-aarch64_be-linux-user] Error 2
[viktormadarasz@m93pserver qemu-2.12.0]$ make >> makeerror
make[1]: flex: Command not found
make[1]: bison: Command not found
make[1]: flex: Command not found
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/ioctls.h:176:9:
error: ‘SIOCGSTAMP’ undeclared here (not in a function); did you mean
‘SIOCSRARP’?
176 | IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
| ^~
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/syscall.c:5678:23:
note: in definition of macro ‘IOCTL’
5678 | { TARGET_ ## cmd, cmd, #cmd, access, 0, { __VA_ARGS__ } },
| ^~~
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/ioctls.h:177:9:
error: ‘SIOCGSTAMPNS’ undeclared here (not in a function); did you mean
‘SIOCGSTAMP_OLD’?
177 | IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
| ^~~~
/home/viktormadarasz/Emulator/qemu-2.12.0/linux-user/syscall.c:5678:23:
note: in definition of macro ‘IOCTL’
5678 | { TARGET_ ## cmd, cmd, #cmd, access, 0, { __VA_ARGS__ } },
| ^~~
make[1]: *** [/home/viktormadarasz/Emulator/qemu-2.12.0/rules.mak:66:
linux-user/syscall.o] Error 1
make: *** [Makefile:478: subdir-aarch64_be-linux-user] Error 2
-
Thanks a lot for helping
Viktor
[PULL 5/5] cmd646-ide: use qdev gpio rather than qemu_allocate_irqs()
From: Mark Cave-Ayland
This prevents the memory from qemu_allocate_irqs() from being leaked which
can in some cases be spotted by Coverity (CID 1421984).
Signed-off-by: Mark Cave-Ayland
Message-id: 20200324210519.2974-4-mark.cave-ayl...@ilande.co.uk
Signed-off-by: John Snow
---
hw/ide/cmd646.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 699f25824d..c254631485 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -249,8 +249,8 @@ static void cmd646_pci_config_write(PCIDevice *d, uint32_t
addr, uint32_t val,
static void pci_cmd646_ide_realize(PCIDevice *dev, Error **errp)
{
PCIIDEState *d = PCI_IDE(dev);
+DeviceState *ds = DEVICE(dev);
uint8_t *pci_conf = dev->config;
-qemu_irq *irq;
int i;
pci_conf[PCI_CLASS_PROG] = 0x8f;
@@ -291,16 +291,15 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error
**errp)
/* TODO: RST# value should be 0 */
pci_conf[PCI_INTERRUPT_PIN] = 0x01; // interrupt on pin 1
-irq = qemu_allocate_irqs(cmd646_set_irq, d, 2);
+qdev_init_gpio_in(ds, cmd646_set_irq, 2);
for (i = 0; i < 2; i++) {
-ide_bus_new(>bus[i], sizeof(d->bus[i]), DEVICE(dev), i, 2);
-ide_init2(>bus[i], irq[i]);
+ide_bus_new(>bus[i], sizeof(d->bus[i]), ds, i, 2);
+ide_init2(>bus[i], qdev_get_gpio_in(ds, i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
ide_register_restart_cb(>bus[i]);
}
-g_free(irq);
}
static void pci_cmd646_ide_exitfn(PCIDevice *dev)
--
2.21.1
[PULL 4/5] via-ide: use qdev gpio rather than qemu_allocate_irqs()
From: Mark Cave-Ayland
This prevents the memory from qemu_allocate_irqs() from being leaked which
can in some cases be spotted by Coverity (CID 1421984).
Signed-off-by: Mark Cave-Ayland
Message-id: 20200324210519.2974-3-mark.cave-ayl...@ilande.co.uk
Signed-off-by: John Snow
---
hw/ide/via.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 2a55b7fbc6..be09912b33 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -160,6 +160,7 @@ static void via_ide_reset(DeviceState *dev)
static void via_ide_realize(PCIDevice *dev, Error **errp)
{
PCIIDEState *d = PCI_IDE(dev);
+DeviceState *ds = DEVICE(dev);
uint8_t *pci_conf = dev->config;
int i;
@@ -187,9 +188,10 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
bmdma_setup_bar(d);
pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, >bmdma_bar);
+qdev_init_gpio_in(ds, via_ide_set_irq, 2);
for (i = 0; i < 2; i++) {
-ide_bus_new(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
-ide_init2(>bus[i], qemu_allocate_irq(via_ide_set_irq, d, i));
+ide_bus_new(>bus[i], sizeof(d->bus[i]), ds, i, 2);
+ide_init2(>bus[i], qdev_get_gpio_in(ds, i));
bmdma_init(>bus[i], >bmdma[i], d);
d->bmdma[i].bus = >bus[i];
--
2.21.1
[PULL 2/5] hw/ide/sii3112: Use qdev gpio rather than qemu_allocate_irqs()
From: Peter Maydell
Coverity points out (CID 1421984) that we are leaking the
memory returned by qemu_allocate_irqs(). We can avoid this
leak by switching to using qdev_init_gpio_in(); the base
class finalize will free the irqs that this allocates under
the hood.
Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: John Snow
Tested-by: BALATON Zoltan
Message-id: 20200323151715.29454-1-peter.mayd...@linaro.org
[Maintainer edit: replace `DEVICE(dev)` by `ds` --js]
Signed-off-by: John Snow
---
hw/ide/sii3112.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 06605d7af2..d69079c3d9 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -251,8 +251,8 @@ static void sii3112_pci_realize(PCIDevice *dev, Error
**errp)
{
SiI3112PCIState *d = SII3112_PCI(dev);
PCIIDEState *s = PCI_IDE(dev);
+DeviceState *ds = DEVICE(dev);
MemoryRegion *mr;
-qemu_irq *irq;
int i;
pci_config_set_interrupt_pin(dev->config, 1);
@@ -280,10 +280,10 @@ static void sii3112_pci_realize(PCIDevice *dev, Error
**errp)
memory_region_init_alias(mr, OBJECT(d), "sii3112.bar4", >mmio, 0, 16);
pci_register_bar(dev, 4, PCI_BASE_ADDRESS_SPACE_IO, mr);
-irq = qemu_allocate_irqs(sii3112_set_irq, d, 2);
+qdev_init_gpio_in(ds, sii3112_set_irq, 2);
for (i = 0; i < 2; i++) {
-ide_bus_new(>bus[i], sizeof(s->bus[i]), DEVICE(dev), i, 1);
-ide_init2(>bus[i], irq[i]);
+ide_bus_new(>bus[i], sizeof(s->bus[i]), ds, i, 1);
+ide_init2(>bus[i], qdev_get_gpio_in(ds, i));
bmdma_init(>bus[i], >bmdma[i], s);
s->bmdma[i].bus = >bus[i];
--
2.21.1
[PULL 3/5] via-ide: don't use PCI level for legacy IRQs
From: Mark Cave-Ayland The PCI level calculation was accidentally left in when rebasing from a previous patchset. Since both IRQs are driven separately, the value being passed into the IRQ handler should be used directly. Signed-off-by: Mark Cave-Ayland Message-id: 20200324210519.2974-2-mark.cave-ayl...@ilande.co.uk Signed-off-by: John Snow --- hw/ide/via.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/ide/via.c b/hw/ide/via.c index 8de4945cc1..2a55b7fbc6 100644 --- a/hw/ide/via.c +++ b/hw/ide/via.c @@ -112,7 +112,6 @@ static void via_ide_set_irq(void *opaque, int n, int level) d->config[0x70 + n * 8] &= ~0x80; } -level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80); qemu_set_irq(isa_get_irq(NULL, 14 + n), level); } -- 2.21.1
[Bug 1869426] [NEW] 5.0rc0->4.2 serial migraiton
Public bug reported:
Migrating from 5.0rc0->4.2 with pc-q35-4.2 we get an error:
Unknown savevm section or instance 'serial' 1
dumping the migration streams it looks like 5.0 is duplicating the
serial migration data:
"serial (26)": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
},
"serial (27)": {
"state": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
}
},
** Affects: qemu
Importance: Undecided
Status: New
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1869426
Title:
5.0rc0->4.2 serial migraiton
Status in QEMU:
New
Bug description:
Migrating from 5.0rc0->4.2 with pc-q35-4.2 we get an error:
Unknown savevm section or instance 'serial' 1
dumping the migration streams it looks like 5.0 is duplicating the
serial migration data:
"serial (26)": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
},
"serial (27)": {
"state": {
"divider": "0x000c",
"rbr": "0x00",
"ier": "0x00",
"iir": "0x01",
"lcr": "0x00",
"mcr": "0x00",
"lsr": "0x60",
"msr": "0xb0",
"scr": "0x00",
"fcr_vmstate": "0x00"
}
},
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1869426/+subscriptions
[PULL 1/5] fdc/i8257: implement verify transfer mode
From: Sven Schnelle
While working on the Tulip driver i tried to write some Teledisk images to
a floppy image which didn't work. Turned out that Teledisk checks the written
data by issuing a READ command to the FDC but running the DMA controller
in VERIFY mode. As we ignored the DMA request in that case, the DMA transfer
never finished, and Teledisk reported an error.
The i8257 spec says about verify transfers:
3) DMA verify, which does not actually involve the transfer of data. When an
8257 channel is in the DMA verify mode, it will respond the same as described
for transfer operations, except that no memory or I/O read/write control signals
will be generated.
Hervé proposed to remove all the dma_mode_ok stuff from fdc to have a more
clear boundary between DMA and FDC, so this patch also does that.
Suggested-by: Hervé Poussineau
Signed-off-by: Sven Schnelle
Reviewed-by: Hervé Poussineau
Signed-off-by: John Snow
---
include/hw/isa/isa.h | 1 -
hw/block/fdc.c | 61 +---
hw/dma/i8257.c | 20 ++-
3 files changed, 31 insertions(+), 51 deletions(-)
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index e9ac1f1205..59a4d4b50a 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -56,7 +56,6 @@ typedef int (*IsaDmaTransferHandler)(void *opaque, int nchan,
int pos,
typedef struct IsaDmaClass {
InterfaceClass parent;
-IsaDmaTransferMode (*get_transfer_mode)(IsaDma *obj, int nchan);
bool (*has_autoinitialization)(IsaDma *obj, int nchan);
int (*read_memory)(IsaDma *obj, int nchan, void *buf, int pos, int len);
int (*write_memory)(IsaDma *obj, int nchan, void *buf, int pos, int len);
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 22e954e0dc..33bc9e2f92 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1714,53 +1714,28 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int
direction)
}
fdctrl->eot = fdctrl->fifo[6];
if (fdctrl->dor & FD_DOR_DMAEN) {
-IsaDmaTransferMode dma_mode;
+/* DMA transfer is enabled. */
IsaDmaClass *k = ISADMA_GET_CLASS(fdctrl->dma);
-bool dma_mode_ok;
-/* DMA transfer are enabled. Check if DMA channel is well programmed */
-dma_mode = k->get_transfer_mode(fdctrl->dma, fdctrl->dma_chann);
-FLOPPY_DPRINTF("dma_mode=%d direction=%d (%d - %d)\n",
- dma_mode, direction,
- (128 << fdctrl->fifo[5]) *
+
+FLOPPY_DPRINTF("direction=%d (%d - %d)\n",
+ direction, (128 << fdctrl->fifo[5]) *
(cur_drv->last_sect - ks + 1), fdctrl->data_len);
-switch (direction) {
-case FD_DIR_SCANE:
-case FD_DIR_SCANL:
-case FD_DIR_SCANH:
-dma_mode_ok = (dma_mode == ISADMA_TRANSFER_VERIFY);
-break;
-case FD_DIR_WRITE:
-dma_mode_ok = (dma_mode == ISADMA_TRANSFER_WRITE);
-break;
-case FD_DIR_READ:
-dma_mode_ok = (dma_mode == ISADMA_TRANSFER_READ);
-break;
-case FD_DIR_VERIFY:
-dma_mode_ok = true;
-break;
-default:
-dma_mode_ok = false;
-break;
-}
-if (dma_mode_ok) {
-/* No access is allowed until DMA transfer has completed */
-fdctrl->msr &= ~FD_MSR_RQM;
-if (direction != FD_DIR_VERIFY) {
-/* Now, we just have to wait for the DMA controller to
- * recall us...
- */
-k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
-k->schedule(fdctrl->dma);
-} else {
-/* Start transfer */
-fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
-fdctrl->data_len);
-}
-return;
+
+/* No access is allowed until DMA transfer has completed */
+fdctrl->msr &= ~FD_MSR_RQM;
+if (direction != FD_DIR_VERIFY) {
+/*
+ * Now, we just have to wait for the DMA controller to
+ * recall us...
+ */
+k->hold_DREQ(fdctrl->dma, fdctrl->dma_chann);
+k->schedule(fdctrl->dma);
} else {
-FLOPPY_DPRINTF("bad dma_mode=%d direction=%d\n", dma_mode,
- direction);
+/* Start transfer */
+fdctrl_transfer_handler(fdctrl, fdctrl->dma_chann, 0,
+fdctrl->data_len);
}
+return;
}
FLOPPY_DPRINTF("start non-DMA transfer\n");
fdctrl->msr |= FD_MSR_NONDMA | FD_MSR_RQM;
diff --git a/hw/dma/i8257.c b/hw/dma/i8257.c
index ef15c06d77..1b3435ab58 100644
--- a/hw/dma/i8257.c
+++ b/hw/dma/i8257.c
@@ -292,12 +292,6 @@ static uint64_t i8257_read_cont(void *opaque, hwaddr
nport, unsigned size)
return val;
}
-static IsaDmaTransferMode
[PULL 0/5] Ide patches
The following changes since commit cfe68ae025f704f336d7dd3d1903ce37b445831d: Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-03-26 20:55:54 +) are available in the Git repository at: https://github.com/jnsnow/qemu.git tags/ide-pull-request for you to fetch changes up to cbf4c9ac9c000f7caf1bfee031041b62d2b000c8: cmd646-ide: use qdev gpio rather than qemu_allocate_irqs() (2020-03-27 14:30:08 -0400) Pull request Mark Cave-Ayland (3): via-ide: don't use PCI level for legacy IRQs via-ide: use qdev gpio rather than qemu_allocate_irqs() cmd646-ide: use qdev gpio rather than qemu_allocate_irqs() Peter Maydell (1): hw/ide/sii3112: Use qdev gpio rather than qemu_allocate_irqs() Sven Schnelle (1): fdc/i8257: implement verify transfer mode include/hw/isa/isa.h | 1 - hw/block/fdc.c | 61 +--- hw/dma/i8257.c | 20 ++- hw/ide/cmd646.c | 9 +++ hw/ide/sii3112.c | 8 +++--- hw/ide/via.c | 7 ++--- 6 files changed, 43 insertions(+), 63 deletions(-) -- 2.21.1
Re: AIO_WAIT_WHILE questions
Patchew URL:
https://patchew.org/QEMU/1242491200.59.1585326983...@webmail.proxmox.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==8595==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
qemu-system-x86_64: /tmp/qemu-test/src/block/block-backend.c:1271: int
blk_prw(BlockBackend *, int64_t, uint8_t *, int64_t, CoroutineEntry *,
BdrvRequestFlags): Assertion `ctx_->lock_count == 1' failed.
Broken pipe
/tmp/qemu-test/src/tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death
from signal 6 (Aborted)
ERROR - too few tests run (expected 13, got 9)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:636: check-qtest-x86_64]
Error 1
make: *** Waiting for unfinished jobs
PASS 1 check-qjson /literals/keyword
PASS 2 check-qjson /literals/string/escaped
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-coroutine"
==8649==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==8649==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top:
0x7ffe80cca000; bottom 0x7f7b14c1; size: 0x00836c0ba000 (564453416960)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==8664==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 27 test-aio /aio-gsource/event/wait/no-flush-cb
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-aio-multithread -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-aio-multithread"
==8669==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 test-aio-multithread /aio/multi/mutex/contended
---
PASS 6 test-throttle /throttle/detach_attach
PASS 7 test-throttle /throttle/config_functions
PASS 8 test-throttle /throttle/accounting
==8703==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 9 test-throttle /throttle/groups
PASS 10 test-throttle /throttle/config/enabled
PASS 11 test-throttle /throttle/config/conflicting
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-thread-pool"
==8707==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
---
PASS 39 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_4
PASS 40 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_after_truncate
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-bdrv-drain -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-bdrv-drain"
==8779==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
test-bdrv-drain: /tmp/qemu-test/src/block/io.c:429: void
bdrv_do_drained_begin(BlockDriverState *, _Bool, BdrvChild *, _Bool, _Bool):
Assertion `ctx_->lock_count == 1' failed.
ERROR - too few tests run (expected 42, got 0)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:641: check-unit] Error 1
Traceback (most recent call last):
File "./tests/docker/docker.py", line 664, in
sys.exit(main())
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run',
Re: [PATCH] qtest: add tulip test case
Patchew URL: https://patchew.org/QEMU/20200327161146.16402-1-liq...@163.com/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==6166==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-coroutine"
==6211==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==6211==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top:
0x7ffee7b7c000; bottom 0x7fa15e601000; size: 0x005d8957b000 (401736183808)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==6226==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 12 fdc-test /x86_64/fdc/read_no_dma_19
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img
tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="ide-test"
==6234==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 28 test-aio /aio-gsource/timer/schedule
PASS 1 ide-test /x86_64/ide/identify
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-aio-multithread -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-aio-multithread"
==6243==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
==6240==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
PASS 2 test-aio-multithread /aio/multi/schedule
==6260==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==6271==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
==6282==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-throttle"
==6299==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-thread-pool"
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
==6303==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==6305==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 6 test-thread-pool /thread-pool/cancel-async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
Re: [PATCH v4] qcow2: Forbid discard in qcow2 v2 images with backing files
On 3/27/20 1:59 PM, Alberto Garcia wrote: A discard request deallocates the selected clusters so they read back as zeroes. This is done by clearing the cluster offset field and setting QCOW_OFLAG_ZERO in the L2 entry. This flag is however only supported when qcow_version >= 3. In older images the cluster is simply deallocated, exposing any possible stale data from the backing file. Since discard is an advisory operation it's safer to simply forbid it in this scenario. Note that we are adding this check to qcow2_co_pdiscard() and not to qcow2_cluster_discard() or discard_in_l2_slice() because the last two are also used by qcow2_snapshot_create() to discard the clusters used by the VM state. In this case there's no risk of exposing stale data to the guest and we really want that the clusters are always discarded. Signed-off-by: Alberto Garcia --- v4: - Show output of qemu-img map when there's no backing file [Eric] Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PULL v2 for 5.0 00/12] testing updates (+ one mttcg change)
On Fri, 27 Mar 2020 at 18:13, Alex Bennée wrote: > > The following changes since commit cfe68ae025f704f336d7dd3d1903ce37b445831d: > > Merge remote-tracking branch > 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging > (2020-03-26 20:55:54 +) > > are available in the Git repository at: > > https://github.com/stsquad/qemu.git tags/pull-testing-270320-2 > > for you to fetch changes up to 41e1f0e2256e4c21bc5671ecb64191e776477c35: > > .travis.yml: Add a KVM-only s390x job (2020-03-27 13:43:20 +) > > > Testing updates: > > - docker updates (various dependencies) > - travis updates (s390x KVM build) > - tweak qemu/atomic.h headers in event of clash > - test/vm updates (NetBSD -> 9.0, FreeBSD -> 12.1) > - disable MTTCG for mips64/mips64el > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0 for any user-visible changes. -- PMM
Re: [PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files
On Fri 27 Mar 2020 07:57:40 PM CET, Eric Blake wrote:
>> +/* If the image does not support QCOW_OFLAG_ZERO then discarding
>> + * clusters could expose stale data from the backing file. */
>> +if (s->qcow_version < 3 && bs->backing) {
>> +return -ENOTSUP;
>> +}
>
> Hmm. Should we blindly always fail for v2, or can we be a little bit
> smarter and still discard a cluster in the top layer if the backing
> layer does not also have it allocated?
Not sure if that's worth it. I only wanted to fix what looks like a
potential security bug so I prefer to keep it simple. qcow2 v3 has been
out for many years already.
Berto
[PATCH v4] qcow2: Forbid discard in qcow2 v2 images with backing files
A discard request deallocates the selected clusters so they read back
as zeroes. This is done by clearing the cluster offset field and
setting QCOW_OFLAG_ZERO in the L2 entry.
This flag is however only supported when qcow_version >= 3. In older
images the cluster is simply deallocated, exposing any possible stale
data from the backing file.
Since discard is an advisory operation it's safer to simply forbid it
in this scenario.
Note that we are adding this check to qcow2_co_pdiscard() and not to
qcow2_cluster_discard() or discard_in_l2_slice() because the last
two are also used by qcow2_snapshot_create() to discard the clusters
used by the VM state. In this case there's no risk of exposing stale
data to the guest and we really want that the clusters are always
discarded.
Signed-off-by: Alberto Garcia
---
v4:
- Show output of qemu-img map when there's no backing file [Eric]
v3:
- Rebase and change iotest number
- Show output of qemu-img map in iotest 290 [Kevin]
- Use the l2_offset and rb_offset variables in iotest 060
v2:
- Don't create the image with compat=0.10 in iotest 060 [Max]
- Use $TEST_IMG.base for the backing image name in iotest 289 [Max]
- Add list of unsupported options to iotest 289 [Max]
block/qcow2.c | 6 +++
tests/qemu-iotests/060 | 12 ++---
tests/qemu-iotests/060.out | 2 -
tests/qemu-iotests/290 | 97 ++
tests/qemu-iotests/290.out | 61
tests/qemu-iotests/group | 1 +
6 files changed, 170 insertions(+), 9 deletions(-)
create mode 100755 tests/qemu-iotests/290
create mode 100644 tests/qemu-iotests/290.out
diff --git a/block/qcow2.c b/block/qcow2.c
index 2bb536b014..e8cbcc1ec1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3784,6 +3784,12 @@ static coroutine_fn int
qcow2_co_pdiscard(BlockDriverState *bs,
int ret;
BDRVQcow2State *s = bs->opaque;
+/* If the image does not support QCOW_OFLAG_ZERO then discarding
+ * clusters could expose stale data from the backing file. */
+if (s->qcow_version < 3 && bs->backing) {
+return -ENOTSUP;
+}
+
if (!QEMU_IS_ALIGNED(offset | bytes, s->cluster_size)) {
assert(bytes < s->cluster_size);
/* Ignore partial clusters, except for the special case of the
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 043f12904a..32c0ecce9e 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -160,18 +160,16 @@ TEST_IMG=$BACKING_IMG _make_test_img 1G
$QEMU_IO -c 'write 0k 64k' "$BACKING_IMG" | _filter_qemu_io
-# compat=0.10 is required in order to make the following discard actually
-# unallocate the sector rather than make it a zero sector - we want COW, after
-# all.
-_make_test_img -o 'compat=0.10' -b "$BACKING_IMG" 1G
+_make_test_img -b "$BACKING_IMG" 1G
# Write two clusters, the second one enforces creation of an L2 table after
# the first data cluster.
$QEMU_IO -c 'write 0k 64k' -c 'write 512M 64k' "$TEST_IMG" | _filter_qemu_io
-# Discard the first cluster. This cluster will soon enough be reallocated and
+# Free the first cluster. This cluster will soon enough be reallocated and
# used for COW.
-$QEMU_IO -c 'discard 0k 64k' "$TEST_IMG" | _filter_qemu_io
+poke_file "$TEST_IMG" "$l2_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
+poke_file "$TEST_IMG" "$(($rb_offset+10))" "\x00\x00"
# Now, corrupt the image by marking the second L2 table cluster as free.
-poke_file "$TEST_IMG" '131084' "\x00\x00" # 0x2000c
+poke_file "$TEST_IMG" "$(($rb_offset+12))" "\x00\x00"
# Start a write operation requiring COW on the image stopping it right before
# doing the read; then, trigger the corruption prevention by writing anything
to
# any unallocated cluster, leading to an attempt to overwrite the second L2
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index d27692a33c..09caaea865 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -105,8 +105,6 @@ wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
wrote 65536/65536 bytes at offset 536870912
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-discard 65536/65536 bytes at offset 0
-64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qcow2: Marking image as corrupt: Preventing invalid write on metadata
(overlaps with active L2 table); further corruption events will be suppressed
blkdebug: Suspended request '0'
write failed: Input/output error
diff --git a/tests/qemu-iotests/290 b/tests/qemu-iotests/290
new file mode 100755
index 00..776b65e915
--- /dev/null
+++ b/tests/qemu-iotests/290
@@ -0,0 +1,97 @@
+#!/usr/bin/env bash
+#
+# Test how 'qemu-io -c discard' behaves on v2 and v3 qcow2 images
+#
+# Copyright (C) 2020 Igalia, S.L.
+# Author: Alberto Garcia
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software
Re: [PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files
On 3/27/20 11:48 AM, Alberto Garcia wrote:
A discard request deallocates the selected clusters so they read back
as zeroes. This is done by clearing the cluster offset field and
setting QCOW_OFLAG_ZERO in the L2 entry.
This flag is however only supported when qcow_version >= 3. In older
images the cluster is simply deallocated, exposing any possible stale
data from the backing file.
Since discard is an advisory operation it's safer to simply forbid it
in this scenario.
Note that we are adding this check to qcow2_co_pdiscard() and not to
qcow2_cluster_discard() or discard_in_l2_slice() because the last
two are also used by qcow2_snapshot_create() to discard the clusters
used by the VM state. In this case there's no risk of exposing stale
data to the guest and we really want that the clusters are always
discarded.
Signed-off-by: Alberto Garcia
---
+++ b/block/qcow2.c
@@ -3784,6 +3784,12 @@ static coroutine_fn int
qcow2_co_pdiscard(BlockDriverState *bs,
int ret;
BDRVQcow2State *s = bs->opaque;
+/* If the image does not support QCOW_OFLAG_ZERO then discarding
+ * clusters could expose stale data from the backing file. */
+if (s->qcow_version < 3 && bs->backing) {
+return -ENOTSUP;
+}
+
Hmm. Should we blindly always fail for v2, or can we be a little bit
smarter and still discard a cluster in the top layer if the backing
layer does not also have it allocated? In other words, is it also worth
checking bdrv_is_allocated(), and avoiding the -ENOTSUP failure in the
case where the backing chain does not have any allocation of the same
range (at which point, discarding the cluster in the top layer will read
as zeroes rather than as stale data)?
But that's a minor optimization for an older file format; it's just as
easy to tell users that if they want maximum discarding power on their
active layer, then they should be using v3 images.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files
On 3/27/20 1:43 PM, Alberto Garcia wrote: On Fri 27 Mar 2020 07:13:04 PM CET, Eric Blake wrote: +for qcow2_compat in 0.10 1.1; do +echo "# Create an image with compat=$qcow2_compat without a backing file" +_make_test_img -o "compat=$qcow2_compat" 128k + +echo "# Fill all clusters with data and then discard them" +$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io + +echo "# Read the data from the discarded clusters" +$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io +done Should this loop also inspect qemu-img map output? I guess we can, although here the image is completely unallocated in both cases. Which shows that even for v2 images, discard DOES do something when there is no backing file (even if it is now a no-op when there is a backing file after this patch). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 2/3] io: Support shutdown of TLS channel
On 3/27/20 12:43 PM, Daniel P. Berrangé wrote: I don't think it is acceptable to do this loop here. The gnutls_bye() function triggers several I/O operations which could block. Looping like this means we busy-wait, blocking this thread for as long as I/O is blocking on the socket. Hmm, good point. Should we give qio_channel_tls_shutdown a bool parameter that says whether it should wait (good for use when we are being run in a coroutine and can deal with the additional I/O) or just fail with -EAGAIN (which the caller can ignore if it is not worried)? A bool would not be sufficient, because the caller would need to know which direction to wait for I/O on. Looking at the code it does a flush of outstanding data, then sends some bytes, and then receives some bytes. The write will probably work most of the time, but the receive is almost always going to return -EAGAIN. So I don't think that failing with EGAIN is going to be much of a benefit. Here, I'm guessing you're talking about gnutls lib/record.c. But note: for GNUTLS_SHUT_WR, it only does _gnutls_io_write_flush and gnutls_alert_send; the use of _gnutls_recv_int is reserved for GNUTLS_SHUT_RDWR. When informing the other end that you are not going to write any more, you don't have to wait for a reply. If we must call gnutls_bye(), then it needs to be done in a way that can integrate with the main loop so it poll()'s / unblocks the current coroutine/thread. This makes the whole thing significantly more complex to deal with, especially if the shutdown is being done in cleanup paths which ordinarily are expected to execute without blocking on I/O. This is the big reason why i never made any attempt to use gnutls_bye(). We _are_ using gnutls_bye(GNUTLS_SHUT_RDWR) on the close() path (which is Are you sure ? AFAIK there is no existing usage of gnutls_bye() at all in QEMU. Oh, I'm confusing that with nbdkit, which does use gnutls_bye(GNUTLS_SHUT_RDWR) but does not wait for a response (at least, not yet). indeed a cleanup path where not blocking is worthwhile) even without this patch, but the question is whether using gnutls_bye(GNUTLS_SHUT_WR) in the normal data path, where we are already using coroutines to manage callbacks, can benefit the remote endpoint, giving them a chance to see clean shutdown instead of abrupt termination. I'm not convinced the clean shutdown at the TLS level does anything important given that we already have done a clean shutdown at the NBD level. Okay, then for now, I'll still try and see if I can fix the libnbd/nbdkit hang without relying on qemu sending a clean gnutls_bye(GNUTLS_SHUT_WR). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 0/3] nbd: Try for cleaner TLS shutdown
Patchew URL: https://patchew.org/QEMU/20200327161936.2225989-1-ebl...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === io/channel-tls.o: In function `qio_channel_tls_shutdown': /tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown' /tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown' collect2: error: ld returned 1 exit status make: *** [scsi/qemu-pr-helper] Error 1 make: *** Waiting for unfinished jobs io/channel-tls.o: In function `qio_channel_tls_shutdown': /tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown' /tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown' collect2: error: ld returned 1 exit status make: *** [qemu-nbd] Error 1 io/channel-tls.o: In function `qio_channel_tls_shutdown': /tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown' /tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown' collect2: error: ld returned 1 exit status make: *** [qemu-io] Error 1 io/channel-tls.o: In function `qio_channel_tls_shutdown': /tmp/qemu-test/src/io/channel-tls.c:373: undefined reference to `qcrypto_tls_session_shutdown' /tmp/qemu-test/src/io/channel-tls.c:376: undefined reference to `qcrypto_tls_session_shutdown' collect2: error: ld returned 1 exit status make: *** [qemu-storage-daemon] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=89d889bd9dd84a6592e526b967f6a8cc', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-xtkcxxmy/src/docker-src.2020-03-27-14.42.04.29238:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=89d889bd9dd84a6592e526b967f6a8cc make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-xtkcxxmy/src' make: *** [docker-run-test-quick@centos7] Error 2 real2m13.487s user0m8.419s The full log is available at http://patchew.org/logs/20200327161936.2225989-1-ebl...@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files
On Fri 27 Mar 2020 07:13:04 PM CET, Eric Blake wrote: >> +for qcow2_compat in 0.10 1.1; do >> +echo "# Create an image with compat=$qcow2_compat without a backing >> file" >> +_make_test_img -o "compat=$qcow2_compat" 128k >> + >> +echo "# Fill all clusters with data and then discard them" >> +$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io >> +$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io >> + >> +echo "# Read the data from the discarded clusters" >> +$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io >> +done > > Should this loop also inspect qemu-img map output? I guess we can, although here the image is completely unallocated in both cases. Berto
Re: [PATCH v2] monitor/hmp-cmds: add units for migrate_parameters.
On Sat, Mar 28, 2020 at 12:14:54AM +0800, Mao Zhongyi wrote: > When running: > (qemu) info migrate_parameters > announce-initial: 50 ms > announce-max: 550 ms > announce-step: 100 ms > compress-wait-thread: on > ... > max-bandwidth: 33554432 bytes/second > downtime-limit: 300 milliseconds > x-checkpoint-delay: 2 > ... > xbzrle-cache-size: 67108864 > > add units for the parameters 'x-checkpoint-delay' and > 'xbzrle-cache-size', it's easier to read, also move > milliseconds to ms to keep the same style. > > Signed-off-by: Mao Zhongyi > --- > monitor/hmp-cmds.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Stefano Garzarella Thanks, Stefano > > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > index 2a900a528a..790fad3afe 100644 > --- a/monitor/hmp-cmds.c > +++ b/monitor/hmp-cmds.c > @@ -436,11 +436,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const > QDict *qdict) > MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH), > params->max_bandwidth); > assert(params->has_downtime_limit); > -monitor_printf(mon, "%s: %" PRIu64 " milliseconds\n", > +monitor_printf(mon, "%s: %" PRIu64 " ms\n", > MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT), > params->downtime_limit); > assert(params->has_x_checkpoint_delay); > -monitor_printf(mon, "%s: %u\n", > +monitor_printf(mon, "%s: %u ms\n", > MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY), > params->x_checkpoint_delay); > assert(params->has_block_incremental); > @@ -453,7 +453,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const > QDict *qdict) > monitor_printf(mon, "%s: %s\n", > MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION), > MultiFDCompression_str(params->multifd_compression)); > -monitor_printf(mon, "%s: %" PRIu64 "\n", > +monitor_printf(mon, "%s: %" PRIu64 " bytes\n", > MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE), > params->xbzrle_cache_size); > monitor_printf(mon, "%s: %" PRIu64 "\n", > -- > 2.17.1 > > >
Re: [PATCH v8 00/74] per-CPU locks
On Fri, 27 Mar 2020 at 01:14, Emilio G. Cota wrote: > > (Apologies if I missed some Cc's; I was not Cc'ed in patch 0 > so I'm blindly crafting a reply.) Sorry I forgot to including you in patch 0, my bad. Will be sure to include you in the future. > On Thu, Mar 26, 2020 at 15:30:43 -0400, Robert Foley wrote: > > This is a continuation of the series created by Emilio Cota. > > We are picking up this patch set with the goal to apply > > any fixes or updates needed to get this accepted. > > Thanks for picking this up! > > > Listed below are the changes for this version of the patch, > > aside from the merge related changes. > > > > Changes for V8: > > - Fixed issue where in rr mode we could destroy the BQL twice. > > I remember doing little to no testing in record-replay mode, so > there should be more bugs hiding in there :-) Thanks for the tip! We will give record-replay some extra testing to hopefully shake some things out. :) > > > - Found/fixed bug that had been hit in testing previously during > > the last consideration of this patch. > > We reproduced the issue hit in the qtest: bios-tables-test. > > The issue was introduced by dropping the BQL, and found us > > (very rarely) missing the condition variable wakeup in > > qemu_tcg_rr_cpu_thread_fn(). > > Aah, this one: > https://patchwork.kernel.org/patch/10838149/#22516931 > How did you identify the problem? Was it code inspection or using a tool > like rr? I remember this being hard to reproduce reliably. Same here, it was hard to reproduce. I did try to use rr on some shorter runs but no luck there. We ran it overnight on one of our ARM servers and it eventually reproduced after about 12 hours in a loop across all the bios-table-test(s) (no rr). Never got it to reproduce on an x86 server. It was fairly consistent too on the same ARM host, it always reproduced within 8-12 hrs or so, and we were able to reproduce it several times. Thanks & Regards, -Rob
Re: [PATCH v3 2/2] net/colo-compare.c: handling of the full primary or secondary queue
On Sat, 28 Mar 2020 02:20:21 +0800
Derek Su wrote:
> Lukas Straub 於 2020年3月28日 週六 上午1:46寫道：
> >
> > On Wed, 25 Mar 2020 17:43:54 +0800
> > Derek Su wrote:
> >
> > > The pervious handling of the full primary or queue is only dropping
> > > the packet. If there are lots of clients to the guest VM,
> > > the "drop" will lead to the lost of the networking connection
> > > until next checkpoint.
> > >
> > > To address the issue, this patch drops the packet firstly.
> > > Then, send all queued primary packets, remove all queued secondary
> > > packets and do checkpoint.
> > >
> > > Signed-off-by: Derek Su
> > > ---
> > > net/colo-compare.c | 41 ++---
> > > 1 file changed, 30 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/net/colo-compare.c b/net/colo-compare.c
> > > index cdd87b2aa8..1a52f50fbe 100644
> > > --- a/net/colo-compare.c
> > > +++ b/net/colo-compare.c
> > > @@ -125,6 +125,12 @@ static const char *colo_mode[] = {
> > > [SECONDARY_IN] = "secondary",
> > > };
> > >
> > > +enum {
> > > +QUEUE_INSERT_ERR = -1,
> > > +QUEUE_INSERT_OK = 0,
> > > +QUEUE_INSERT_FULL = 1,
> > > +};
> > > +
> > > static int compare_chr_send(CompareState *s,
> > > const uint8_t *buf,
> > > uint32_t size,
> > > @@ -211,8 +217,10 @@ static int colo_insert_packet(GQueue *queue, Packet
> > > *pkt, uint32_t *max_ack)
> > > }
> > >
> > > /*
> > > - * Return 0 on success, if return -1 means the pkt
> > > - * is unsupported(arp and ipv6) and will be sent later
> > > + * Return QUEUE_INSERT_OK on success.
> > > + * If return QUEUE_INSERT_FULL means list is full, and
> > > + * QUEUE_INSERT_ERR means the pkt is unsupported(arp and ipv6) and
> > > + * will be sent later
> > > */
> > > static int packet_enqueue(CompareState *s, int mode, Connection **con)
> > > {
> > > @@ -234,7 +242,7 @@ static int packet_enqueue(CompareState *s, int mode,
> > > Connection **con)
> > > if (parse_packet_early(pkt)) {
> > > packet_destroy(pkt, NULL);
> > > pkt = NULL;
> > > -return -1;
> > > +return QUEUE_INSERT_ERR;
> > > }
> > > fill_connection_key(pkt, );
> > >
> > > @@ -258,11 +266,12 @@ static int packet_enqueue(CompareState *s, int
> > > mode, Connection **con)
> > > "drop packet", colo_mode[mode]);
> > > packet_destroy(pkt, NULL);
> > > pkt = NULL;
> > > +return QUEUE_INSERT_FULL;
> > > }
> > >
> > > *con = conn;
> > >
> > > -return 0;
> > > +return QUEUE_INSERT_OK;
> > > }
> > >
> > > static inline bool after(uint32_t seq1, uint32_t seq2)
> > > @@ -995,17 +1004,22 @@ static void
> > > compare_pri_rs_finalize(SocketReadState *pri_rs)
> > > {
> > > CompareState *s = container_of(pri_rs, CompareState, pri_rs);
> > > Connection *conn = NULL;
> > > +int ret;
> > >
> > > -if (packet_enqueue(s, PRIMARY_IN, )) {
> > > +ret = packet_enqueue(s, PRIMARY_IN, );
> > > +if (ret == QUEUE_INSERT_OK) {
> > > +/* compare packet in the specified connection */
> > > +colo_compare_connection(conn, s);
> > > +} else if (ret == QUEUE_INSERT_FULL) {
> > > +g_queue_foreach(>conn_list, colo_flush_packets, s);
> > > +colo_compare_inconsistency_notify(s);
> > > +} else {
> > > trace_colo_compare_main("primary: unsupported packet in");
> > > compare_chr_send(s,
> > > pri_rs->buf,
> > > pri_rs->packet_len,
> > > pri_rs->vnet_hdr_len,
> > > false);
> > > -} else {
> > > -/* compare packet in the specified connection */
> > > -colo_compare_connection(conn, s);
> > > }
> > > }
> > >
> > > @@ -1013,12 +1027,17 @@ static void
> > > compare_sec_rs_finalize(SocketReadState *sec_rs)
> > > {
> > > CompareState *s = container_of(sec_rs, CompareState, sec_rs);
> > > Connection *conn = NULL;
> > > +int ret;
> > >
> > > -if (packet_enqueue(s, SECONDARY_IN, )) {
> > > -trace_colo_compare_main("secondary: unsupported packet in");
> > > -} else {
> > > +ret = packet_enqueue(s, SECONDARY_IN, );
> > > +if (ret == QUEUE_INSERT_OK) {
> > > /* compare packet in the specified connection */
> > > colo_compare_connection(conn, s);
> > > +} else if (ret == QUEUE_INSERT_FULL) {
> > > +g_queue_foreach(>conn_list, colo_flush_packets, s);
> > > +colo_compare_inconsistency_notify(s);
> > > +} else {
> > > +trace_colo_compare_main("secondary: unsupported packet in");
> > > }
> > > }
> > >
> >
> > Hi,
> > I don't think we have to flush here because the (post-)checkpoint event
> > will flush the packets for us.
> >
>
> Hi,
> Yes, the periodical checkpoint can flush the packets.
>
> But, if many clients send many packets to the vm,
> there is a high
Re: [PATCH v1] s390x: Reject unaligned RAM sizes
On 27.03.20 19:16, Halil Pasic wrote:
> On Fri, 27 Mar 2020 17:46:20 +0100
> Igor Mammedov wrote:
>
>> On Fri, 27 Mar 2020 17:05:34 +0100
>> Christian Borntraeger wrote:
>>
>>> On 27.03.20 17:01, David Hildenbrand wrote:
On 27.03.20 16:34, Christian Borntraeger wrote:
>
>
> On 27.03.20 16:29, David Hildenbrand wrote:
>> Historically, we fixed up the RAM size (rounded it down), to fit into
>> storage increments. Since commit 3a12fc61af5c ("390x/s390-virtio-ccw: use
>> memdev for RAM"), we no longer consider the fixed-up size when
>> allcoating the RAM block - which will break migration.
>>
>> Let's simply drop that manual fixup code and let the user supply sane
>> RAM sizes. This will bail out early when trying to migrate (and make
>> an existing guest with e.g., 12345 MB non-migratable), but maybe we
>> should have rejected such RAM sizes right from the beginning.
>>
>> As we no longer fixup maxram_size as well, make other users use ram_size
>> instead. Keep using maxram_size when setting the maximum ram size in KVM,
>> as that will come in handy in the future when supporting memory hotplug
>> (in contrast, storage keys and storage attributes for hotplugged memory
>> will have to be migrated per RAM block in the future).
>>
>> This fixes (or rather rejects early):
>>
>> 1. Migrating older QEMU to upstream QEMU (e.g., with "-m 1235M"), as the
>>RAM block size changed.
>
> Not sure I like this variant. Instead of breaking migration (that was
> accidentially done by Igors changes) we now reject migration from older
> QEMUs to 5.0. This is not going to help those that still have such guests
> running and want to migrate.
As Igor mentioned on another channel, you most probably can migrate an
older guest by starting it on the target with a fixed-up size.
E.g., migrate an old QEMU "-m 1235M" to a new QEMU "-m 1234M"
>>>
>>> Yes, that should probably work.
>> I'm in process of testing it.
>>
Not sure how many such weird-size VMs we actually do have in practice.
>>>
>>> I am worried about some automated deployments where tooling has created
>>> these sizes for dozens or hundreds of containers in VMS and so.
>> Yep, it's possible but then that tooling/configs should be fixed to work with
>> new QEMU that validates user's input.
>>
>
> @David: I'm a little confused. Is this fix about adding user input
> validation, or is it about changing what valid inputs are?
We used to silently fixup the user input instead of bailing out. So it's
rather the latter. It was never the right choice to silently fixup
instead of just bailing out. We never should have done that.
>
> I don't see this alignment requirement documented, so my guess is the
> latter. And then, I'm not sure I'm sold on this.
Well, we can add documentation with the next release. It's something to
end up in the release notes.
If somebody specified "-m 1235", he always received "1234 MB" and never
"1235 MB". You could also call that a BUG, which should have bailed out
right from the start (there wasn't even a warning).
--
Thanks,
David / dhildenb
Re: [PATCH v8 00/74] per-CPU locks
Emilio G. Cota writes: > (Apologies if I missed some Cc's; I was not Cc'ed in patch 0 > so I'm blindly crafting a reply.) > >> Changes for V8: >> - Fixed issue where in rr mode we could destroy the BQL twice. > > I remember doing little to no testing in record-replay mode, so > there should be more bugs hiding in there :-) Well we have two (very simple) rr tests in check-tcg now - so there is that ;-) > On a related note, I've done some work to get QEMU-system to work > under thread sanitizer, since tsan now supports our longjmp-based > coroutines (hurrah!). When did that go in? Will I need a new toolchain and -ltsan to make it work? > My idea was to integrate tsan in QEMU (i.e. > bring tsan warnings to 0) before (re)trying to merge the > per-CPU lock patchset; this would minimize the potential for > regressions, which from my personal viewpoint seems like a reasonable > thing to do especially now that I have little time to work on QEMU. > > If there's interest in doing the tsan work first, then I'd be > happy to send to the list as soon as this weekend the changes that > I have so far [1]. Please do - I've been cleaning up some of the other things the sanitizer has found and it certainly won't hurt to get thread sanitizer working again. -- Alex Bennée
[PULL v2 12/12] .travis.yml: Add a KVM-only s390x job
From: Philippe Mathieu-Daudé
Add a job to build QEMU on s390x with TCG disabled, so
this configuration won't bitrot over time.
This job is quick, running check-unit: Ran for 5 min 30 sec
https://travis-ci.org/github/philmd/qemu/jobs/665456423
Acked-by: Cornelia Huck
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Alex Bennée
Reviewed-by: Richard Henderson
Message-Id: <20200322154015.25358-1-phi...@redhat.com>
Message-Id: <20200323161514.23952-12-alex.ben...@linaro.org>
diff --git a/.travis.yml b/.travis.yml
index 5672d129ec6..e0c72210b7a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -525,6 +525,48 @@ jobs:
$(exit $BUILD_RC);
fi
+- name: "[s390x] GCC check (KVM)"
+ arch: s390x
+ dist: bionic
+ addons:
+apt_packages:
+ - libaio-dev
+ - libattr1-dev
+ - libbrlapi-dev
+ - libcap-ng-dev
+ - libgcrypt20-dev
+ - libgnutls28-dev
+ - libgtk-3-dev
+ - libiscsi-dev
+ - liblttng-ust-dev
+ - libncurses5-dev
+ - libnfs-dev
+ - libnss3-dev
+ - libpixman-1-dev
+ - libpng-dev
+ - librados-dev
+ - libsdl2-dev
+ - libseccomp-dev
+ - liburcu-dev
+ - libusb-1.0-0-dev
+ - libvdeplug-dev
+ - libvte-2.91-dev
+ # Tests dependencies
+ - genisoimage
+ env:
+- TEST_CMD="make check-unit"
+- CONFIG="--disable-containers --disable-tcg --enable-kvm
--disable-tools"
+ script:
+- ( cd ${SRC_DIR} ; git submodule update --init roms/SLOF )
+- BUILD_RC=0 && make -j${JOBS} || BUILD_RC=$?
+- |
+ if [ "$BUILD_RC" -eq 0 ] ; then
+ mv pc-bios/s390-ccw/*.img pc-bios/ ;
+ ${TEST_CMD} ;
+ else
+ $(exit $BUILD_RC);
+ fi
+
# Release builds
# The make-release script expect a QEMU version, so our tag must start
with a 'v'.
# This is the case when release candidate tags are created.
--
2.20.1
Re: [PATCH v3 2/2] net/colo-compare.c: handling of the full primary or secondary queue
Lukas Straub 於 2020年3月28日 週六 上午1:46寫道：
>
> On Wed, 25 Mar 2020 17:43:54 +0800
> Derek Su wrote:
>
> > The pervious handling of the full primary or queue is only dropping
> > the packet. If there are lots of clients to the guest VM,
> > the "drop" will lead to the lost of the networking connection
> > until next checkpoint.
> >
> > To address the issue, this patch drops the packet firstly.
> > Then, send all queued primary packets, remove all queued secondary
> > packets and do checkpoint.
> >
> > Signed-off-by: Derek Su
> > ---
> > net/colo-compare.c | 41 ++---
> > 1 file changed, 30 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/colo-compare.c b/net/colo-compare.c
> > index cdd87b2aa8..1a52f50fbe 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -125,6 +125,12 @@ static const char *colo_mode[] = {
> > [SECONDARY_IN] = "secondary",
> > };
> >
> > +enum {
> > +QUEUE_INSERT_ERR = -1,
> > +QUEUE_INSERT_OK = 0,
> > +QUEUE_INSERT_FULL = 1,
> > +};
> > +
> > static int compare_chr_send(CompareState *s,
> > const uint8_t *buf,
> > uint32_t size,
> > @@ -211,8 +217,10 @@ static int colo_insert_packet(GQueue *queue, Packet
> > *pkt, uint32_t *max_ack)
> > }
> >
> > /*
> > - * Return 0 on success, if return -1 means the pkt
> > - * is unsupported(arp and ipv6) and will be sent later
> > + * Return QUEUE_INSERT_OK on success.
> > + * If return QUEUE_INSERT_FULL means list is full, and
> > + * QUEUE_INSERT_ERR means the pkt is unsupported(arp and ipv6) and
> > + * will be sent later
> > */
> > static int packet_enqueue(CompareState *s, int mode, Connection **con)
> > {
> > @@ -234,7 +242,7 @@ static int packet_enqueue(CompareState *s, int mode,
> > Connection **con)
> > if (parse_packet_early(pkt)) {
> > packet_destroy(pkt, NULL);
> > pkt = NULL;
> > -return -1;
> > +return QUEUE_INSERT_ERR;
> > }
> > fill_connection_key(pkt, );
> >
> > @@ -258,11 +266,12 @@ static int packet_enqueue(CompareState *s, int mode,
> > Connection **con)
> > "drop packet", colo_mode[mode]);
> > packet_destroy(pkt, NULL);
> > pkt = NULL;
> > +return QUEUE_INSERT_FULL;
> > }
> >
> > *con = conn;
> >
> > -return 0;
> > +return QUEUE_INSERT_OK;
> > }
> >
> > static inline bool after(uint32_t seq1, uint32_t seq2)
> > @@ -995,17 +1004,22 @@ static void compare_pri_rs_finalize(SocketReadState
> > *pri_rs)
> > {
> > CompareState *s = container_of(pri_rs, CompareState, pri_rs);
> > Connection *conn = NULL;
> > +int ret;
> >
> > -if (packet_enqueue(s, PRIMARY_IN, )) {
> > +ret = packet_enqueue(s, PRIMARY_IN, );
> > +if (ret == QUEUE_INSERT_OK) {
> > +/* compare packet in the specified connection */
> > +colo_compare_connection(conn, s);
> > +} else if (ret == QUEUE_INSERT_FULL) {
> > +g_queue_foreach(>conn_list, colo_flush_packets, s);
> > +colo_compare_inconsistency_notify(s);
> > +} else {
> > trace_colo_compare_main("primary: unsupported packet in");
> > compare_chr_send(s,
> > pri_rs->buf,
> > pri_rs->packet_len,
> > pri_rs->vnet_hdr_len,
> > false);
> > -} else {
> > -/* compare packet in the specified connection */
> > -colo_compare_connection(conn, s);
> > }
> > }
> >
> > @@ -1013,12 +1027,17 @@ static void compare_sec_rs_finalize(SocketReadState
> > *sec_rs)
> > {
> > CompareState *s = container_of(sec_rs, CompareState, sec_rs);
> > Connection *conn = NULL;
> > +int ret;
> >
> > -if (packet_enqueue(s, SECONDARY_IN, )) {
> > -trace_colo_compare_main("secondary: unsupported packet in");
> > -} else {
> > +ret = packet_enqueue(s, SECONDARY_IN, );
> > +if (ret == QUEUE_INSERT_OK) {
> > /* compare packet in the specified connection */
> > colo_compare_connection(conn, s);
> > +} else if (ret == QUEUE_INSERT_FULL) {
> > +g_queue_foreach(>conn_list, colo_flush_packets, s);
> > +colo_compare_inconsistency_notify(s);
> > +} else {
> > +trace_colo_compare_main("secondary: unsupported packet in");
> > }
> > }
> >
>
> Hi,
> I don't think we have to flush here because the (post-)checkpoint event will
> flush the packets for us.
>
Hi,
Yes, the periodical checkpoint can flush the packets.
But, if many clients send many packets to the vm,
there is a high probability that packets are dropped because the
primary/secondary queues are always full.
It causes lots of re-transmission between clients and vm and
deteriorate service response to clients.
Sincerely,
Derek Su
> Regards,
> Lukas Straub
[PULL v2 09/12] tests/docker: Install gcrypt devel package in Debian image
From: Philippe Mathieu-Daudé In commit 6f8bbb374be we enabled building with the gcrypt library on the the Debian 'x86 host', which was based on Debian Stretch. Later in commit 698a71edbed we upgraded the Debian base image to Buster. Apparently Debian Stretch was listing gcrypt as a QEMU dependency, but this is not the case anymore in Buster, so we need to install it manually (it it not listed by 'apt-get -s build-dep qemu' in the common debian10.docker anymore). This fixes: $ ../configure $QEMU_CONFIGURE_OPTS ERROR: User requested feature gcrypt configure was not able to find it. Install gcrypt devel >= 1.5.0 Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200322120104.21267-3-phi...@redhat.com> Message-Id: <20200323161514.23952-9-alex.ben...@linaro.org> diff --git a/tests/docker/dockerfiles/debian-amd64.docker b/tests/docker/dockerfiles/debian-amd64.docker index d4849f509f4..957f0bc2e79 100644 --- a/tests/docker/dockerfiles/debian-amd64.docker +++ b/tests/docker/dockerfiles/debian-amd64.docker @@ -16,6 +16,7 @@ RUN apt update && \ apt install -y --no-install-recommends \ libbz2-dev \ liblzo2-dev \ +libgcrypt20-dev \ librdmacm-dev \ libsasl2-dev \ libsnappy-dev \ -- 2.20.1
Re: [PATCH v1] s390x: Reject unaligned RAM sizes
On Fri, 27 Mar 2020 17:46:20 +0100
Igor Mammedov wrote:
> On Fri, 27 Mar 2020 17:05:34 +0100
> Christian Borntraeger wrote:
>
> > On 27.03.20 17:01, David Hildenbrand wrote:
> > > On 27.03.20 16:34, Christian Borntraeger wrote:
> > >>
> > >>
> > >> On 27.03.20 16:29, David Hildenbrand wrote:
> > >>> Historically, we fixed up the RAM size (rounded it down), to fit into
> > >>> storage increments. Since commit 3a12fc61af5c ("390x/s390-virtio-ccw:
> > >>> use
> > >>> memdev for RAM"), we no longer consider the fixed-up size when
> > >>> allcoating the RAM block - which will break migration.
> > >>>
> > >>> Let's simply drop that manual fixup code and let the user supply sane
> > >>> RAM sizes. This will bail out early when trying to migrate (and make
> > >>> an existing guest with e.g., 12345 MB non-migratable), but maybe we
> > >>> should have rejected such RAM sizes right from the beginning.
> > >>>
> > >>> As we no longer fixup maxram_size as well, make other users use ram_size
> > >>> instead. Keep using maxram_size when setting the maximum ram size in
> > >>> KVM,
> > >>> as that will come in handy in the future when supporting memory hotplug
> > >>> (in contrast, storage keys and storage attributes for hotplugged memory
> > >>> will have to be migrated per RAM block in the future).
> > >>>
> > >>> This fixes (or rather rejects early):
> > >>>
> > >>> 1. Migrating older QEMU to upstream QEMU (e.g., with "-m 1235M"), as the
> > >>>RAM block size changed.
> > >>
> > >> Not sure I like this variant. Instead of breaking migration (that was
> > >> accidentially done by Igors changes) we now reject migration from older
> > >> QEMUs to 5.0. This is not going to help those that still have such guests
> > >> running and want to migrate.
> > >
> > > As Igor mentioned on another channel, you most probably can migrate an
> > > older guest by starting it on the target with a fixed-up size.
> > >
> > > E.g., migrate an old QEMU "-m 1235M" to a new QEMU "-m 1234M"
> >
> > Yes, that should probably work.
> I'm in process of testing it.
>
> > > Not sure how many such weird-size VMs we actually do have in practice.
> >
> > I am worried about some automated deployments where tooling has created
> > these sizes for dozens or hundreds of containers in VMS and so.
> Yep, it's possible but then that tooling/configs should be fixed to work with
> new QEMU that validates user's input.
>
@David: I'm a little confused. Is this fix about adding user input
validation, or is it about changing what valid inputs are?
I don't see this alignment requirement documented, so my guess is the
latter. And then, I'm not sure I'm sold on this.
Regards,
Halil
[PULL v2 10/12] tests/docker: Use Python3 PyYAML in the Fedora image
From: Philippe Mathieu-Daudé The Python2 PyYAML is now pointless, switch to the Python3 version. Fixes: bcbf27947 (docker: move tests from python2 to python3) Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200322120104.21267-4-phi...@redhat.com> Message-Id: <20200323161514.23952-10-alex.ben...@linaro.org> diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 019eb12dcb1..174979c7af4 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -79,8 +79,8 @@ ENV PACKAGES \ perl-Test-Harness \ pixman-devel \ python3 \ +python3-PyYAML \ python3-sphinx \ -PyYAML \ rdma-core-devel \ SDL2-devel \ snappy-devel \ -- 2.20.1
[PULL v2 11/12] tests/docker: Add libepoxy and libudev packages to the Fedora image
From: Philippe Mathieu-Daudé Install optional dependencies of QEMU to get better coverage. Suggested-by: Peter Maydell Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200322120104.21267-5-phi...@redhat.com> Message-Id: <20200323161514.23952-11-alex.ben...@linaro.org> diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 174979c7af4..4bd2c953af8 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -29,6 +29,7 @@ ENV PACKAGES \ libblockdev-mpath-devel \ libcap-ng-devel \ libcurl-devel \ +libepoxy-devel \ libfdt-devel \ libiscsi-devel \ libjpeg-devel \ @@ -38,6 +39,7 @@ ENV PACKAGES \ libseccomp-devel \ libssh-devel \ libubsan \ +libudev-devel \ libusbx-devel \ libxml2-devel \ libzstd-devel \ -- 2.20.1
[PULL v2 05/12] tests/vm: update NetBSD to 9.0
From: Gerd Hoffmann
The installer supports GPT now, so the install workflow has changed a
bit. Also: run without VGA device. This works around a bug in the
seabios sercon code and makes the bootloader menu show up on the serial
line, so we can drop the quirk for that.
Signed-off-by: Gerd Hoffmann
Signed-off-by: Alex Bennée
Message-Id: <20200310083218.26355-5-kra...@redhat.com>
Message-Id: <20200323161514.23952-5-alex.ben...@linaro.org>
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index f3257bc245a..b10c9d429de 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -22,8 +22,8 @@ class NetBSDVM(basevm.BaseVM):
name = "netbsd"
arch = "x86_64"
-link =
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-8.1/images/NetBSD-8.1-amd64.iso;
-csum =
"718f275b7e0879599bdac95630c5e3f2184700032fdb6cdebf3bdd63687898c48ff3f08f57b89f4437a86cdd8ea07c01a39d432dbb37e1e4b008f4985f98da3f"
+link =
"https://cdn.netbsd.org/pub/NetBSD/NetBSD-9.0/images/NetBSD-9.0-amd64.iso;
+csum =
"34da4882ee61bdbf69f241195a8933dc800949d30b43fc6988da853d57fc2b8cac50cf97a0d2adaf93250b4e329d189c1a8b83c33bd515226f37745d50c33369"
size = "20G"
pkgs = [
# tools
@@ -86,42 +86,31 @@ class NetBSDVM(basevm.BaseVM):
self.boot(img_tmp, extra_args = [
"-bios", "pc-bios/bios-256k.bin",
"-machine", "graphics=off",
-"-device", "VGA",
"-cdrom", iso
])
self.console_init()
-self.console_wait("Primary Bootstrap")
-
-# serial console boot menu output doesn't work for some
-# reason, so we have to fly blind ...
-for char in list("5consdev com0\n"):
-time.sleep(0.2)
-self.console_send(char)
-self.console_consume()
+self.console_wait_send("3. Drop to boot prompt", "3")
+self.console_wait_send("> ", "consdev com0\n")
self.console_wait_send("> ", "boot\n")
self.console_wait_send("Terminal type","xterm\n")
self.console_wait_send("a: Installation messages", "a\n")
-self.console_wait_send("b: US-English","b\n")
self.console_wait_send("a: Install NetBSD","a\n")
self.console_wait("Shall we continue?")
self.console_wait_send("b: Yes", "b\n")
self.console_wait_send("a: ld0", "a\n")
+self.console_wait_send("a: Guid Partition Table", "a\n")
self.console_wait_send("a: This is the correct", "a\n")
-self.console_wait_send("b: Use the entire disk", "b\n")
-self.console_wait("NetBSD bootcode")
-self.console_wait_send("a: Yes", "a\n")
-self.console_wait_send("b: Use existing part", "b\n")
+self.console_wait_send("b: Use default part", "b\n")
self.console_wait_send("x: Partition sizes ok","x\n")
-self.console_wait_send("for your NetBSD disk", "\n")
self.console_wait("Shall we continue?")
self.console_wait_send("b: Yes", "b\n")
self.console_wait_send("b: Use serial port com0", "b\n")
self.console_wait_send("f: Set serial baud rate", "f\n")
self.console_wait_send("a: 9600", "a\n")
-self.console_wait_send("x: Exit", "x\n")
+self.console_wait_send("x: Continue", "x\n")
self.console_wait_send("a: Full installation", "a\n")
self.console_wait_send("a: CD-ROM","a\n")
--
2.20.1
[PULL v2 07/12] configure: disable MTTCG for MIPS guests
While debugging check-acceptance failures I found an instability in the mips64el test case. Briefly the test case: retry.py -n 100 -c -- ./mips64el-softmmu/qemu-system-mips64el \ -display none -vga none -serial mon:stdio \ -machine malta -kernel ./vmlinux-4.7.0-rc1.I6400 \ -cpu I6400 -smp 8 -vga std \ -append "printk.time=0 clocksource=GIC console=tty0 console=ttyS0 panic=-1" \ --no-reboot Reports about a 9% failure rate: Results summary: 0: 91 times (91.00%), avg time 5.547 (0.45 varience/0.67 deviation) -6: 9 times (9.00%), avg time 3.394 (0.02 varience/0.13 deviation) Ran command 100 times, 91 passes When re-run with "--accel tcg,thread=single" the instability goes away. Results summary: 0: 100 times (100.00%), avg time 17.318 (249.76 varience/15.80 deviation) Ran command 100 times, 100 passes Which seems to indicate there is some aspect of the MIPS MTTCG fixes that has been missed. Ideally we would fix that but I'm afraid I don't have time to investigate and am not super familiar with the architecture anyway. In lieu of someone tracking down the failure lets disable it for now. Signed-off-by: Alex Bennée Acked-by: Philippe Mathieu-Daudé Reviewed-by: Philippe Mathieu-Daudé Acked-by: Richard Henderson Reviewed-by: Aleksandar Markovic Cc: Aurelien Jarno Cc: Aleksandar Rikalo Message-Id: <20200323161514.23952-7-alex.ben...@linaro.org> diff --git a/configure b/configure index 89fe881dd46..e225a1e3ffe 100755 --- a/configure +++ b/configure @@ -7887,7 +7887,7 @@ case "$target_name" in TARGET_SYSTBL_ABI=n32 ;; mips64|mips64el) -mttcg="yes" +mttcg="no" TARGET_ARCH=mips64 TARGET_BASE_ARCH=mips echo "TARGET_ABI_MIPSN64=y" >> $config_target_mak -- 2.20.1
[PULL v2 04/12] tests/vm: update FreeBSD to 12.1
From: Gerd Hoffmann Signed-off-by: Gerd Hoffmann Signed-off-by: Alex Bennée Message-Id: <20200310083218.26355-4-kra...@redhat.com> Message-Id: <20200323161514.23952-4-alex.ben...@linaro.org> diff --git a/tests/vm/freebsd b/tests/vm/freebsd index 58166766d91..298967fe9cf 100755 --- a/tests/vm/freebsd +++ b/tests/vm/freebsd @@ -24,8 +24,8 @@ class FreeBSDVM(basevm.BaseVM): name = "freebsd" arch = "x86_64" -link = "https://download.freebsd.org/ftp/releases/ISO-IMAGES/12.0/FreeBSD-12.0-RELEASE-amd64-disc1.iso.xz; -csum = "1d40015bea89d05b8bd13e2ed80c40b522a9ec1abd8e7c8b80954fb485fb99db" +link = "https://download.freebsd.org/ftp/releases/ISO-IMAGES/12.1/FreeBSD-12.1-RELEASE-amd64-disc1.iso.xz; +csum = "7394c3f60a1e236e7bd3a05809cf43ae39a3b8e5d42d782004cf2f26b1cfcd88" size = "20G" pkgs = [ # build tools -- 2.20.1
[PULL v2 08/12] tests/docker: Keep package list sorted
From: Philippe Mathieu-Daudé Keep package list sorted, this eases rebase/cherry-pick. Fixes: 3a6784813 Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200322120104.21267-2-phi...@redhat.com> Message-Id: <20200323161514.23952-8-alex.ben...@linaro.org> diff --git a/tests/docker/dockerfiles/centos7.docker b/tests/docker/dockerfiles/centos7.docker index cdd72de7ebf..9a2a2e515d7 100644 --- a/tests/docker/dockerfiles/centos7.docker +++ b/tests/docker/dockerfiles/centos7.docker @@ -2,6 +2,8 @@ FROM centos:7 RUN yum install -y epel-release centos-release-xen-48 RUN yum -y update + +# Please keep this list sorted alphabetically ENV PACKAGES \ bison \ bzip2 \ @@ -19,6 +21,7 @@ ENV PACKAGES \ libepoxy-devel \ libfdt-devel \ librdmacm-devel \ +libzstd-devel \ lzo-devel \ make \ mesa-libEGL-devel \ @@ -33,7 +36,6 @@ ENV PACKAGES \ tar \ vte-devel \ xen-devel \ -zlib-devel \ -libzstd-devel +zlib-devel RUN yum install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index a658c02..019eb12dcb1 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -1,4 +1,6 @@ FROM fedora:30 + +# Please keep this list sorted alphabetically ENV PACKAGES \ bc \ bison \ @@ -38,6 +40,7 @@ ENV PACKAGES \ libubsan \ libusbx-devel \ libxml2-devel \ +libzstd-devel \ llvm \ lzo-devel \ make \ @@ -92,8 +95,7 @@ ENV PACKAGES \ vte291-devel \ which \ xen-devel \ -zlib-devel \ -libzstd-devel +zlib-devel ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3 RUN dnf install -y $PACKAGES -- 2.20.1
[PULL v2 02/12] tests/vm: write raw console log
From: Gerd Hoffmann
Run "tail -f /var/tmp/*/qemu*console.raw" in another terminal
to watch the install console.
Signed-off-by: Gerd Hoffmann
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20200310083218.26355-2-kra...@redhat.com>
Message-Id: <20200323161514.23952-2-alex.ben...@linaro.org>
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 8400b0e07f6..c53fd354d95 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -213,6 +213,9 @@ class BaseVM(object):
def console_init(self, timeout = 120):
vm = self._guest
vm.console_socket.settimeout(timeout)
+self.console_raw_path = os.path.join(vm._temp_dir,
+ vm._name + "-console.raw")
+self.console_raw_file = open(self.console_raw_path, 'wb')
def console_log(self, text):
for line in re.split("[\r\n]", text):
@@ -234,6 +237,9 @@ class BaseVM(object):
while True:
try:
chars = vm.console_socket.recv(1)
+if self.console_raw_file:
+self.console_raw_file.write(chars)
+self.console_raw_file.flush()
except socket.timeout:
sys.stderr.write("console: *** read timeout ***\n")
sys.stderr.write("console: waiting for: '%s'\n" % expect)
--
2.20.1
[PULL v2 06/12] tests/vm: fix basevm config
When the patch was merged it was part of a longer series which had
already merged the config changes. Semu-revert the config related
changes for now so things will build.
Fixes: b081986c85fd2
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20200323161514.23952-6-alex.ben...@linaro.org>
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index cffe7c4600e..756ccf7acae 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -358,23 +358,23 @@ class BaseVM(object):
"local-hostname: {}-guest\n".format(name)])
mdata.close()
udata = open(os.path.join(cidir, "user-data"), "w")
-print("guest user:pw {}:{}".format(self._config['guest_user'],
- self._config['guest_pass']))
+print("guest user:pw {}:{}".format(self.GUEST_USER,
+ self.GUEST_PASS))
udata.writelines(["#cloud-config\n",
"chpasswd:\n",
" list: |\n",
- "root:%s\n" % self._config['root_pass'],
- "%s:%s\n" % (self._config['guest_user'],
- self._config['guest_pass']),
+ "root:%s\n" % self.ROOT_PASS,
+ "%s:%s\n" % (self.GUEST_USER,
+ self.GUEST_PASS),
" expire: False\n",
"users:\n",
- " - name: %s\n" % self._config['guest_user'],
+ " - name: %s\n" % self.GUEST_USER,
"sudo: ALL=(ALL) NOPASSWD:ALL\n",
"ssh-authorized-keys:\n",
- "- %s\n" % self._config['ssh_pub_key'],
+ "- %s\n" % SSH_PUB_KEY,
" - name: root\n",
"ssh-authorized-keys:\n",
- "- %s\n" % self._config['ssh_pub_key'],
+ "- %s\n" % SSH_PUB_KEY,
"locale: en_US.UTF-8\n"])
proxy = os.environ.get("http_proxy")
if not proxy is None:
--
2.20.1
[PULL v2 03/12] tests/vm: move vga setup
From: Gerd Hoffmann
Move '-device VGA' from basevm.py to the guests, so they have
the chance to opt out and run without display device.
Signed-off-by: Gerd Hoffmann
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20200310083218.26355-3-kra...@redhat.com>
Message-Id: <20200323161514.23952-3-alex.ben...@linaro.org>
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index c53fd354d95..cffe7c4600e 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -179,7 +179,6 @@ class BaseVM(object):
def boot(self, img, extra_args=[]):
args = self._args + [
-"-device", "VGA",
"-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
"-device", "virtio-blk,drive=drive0,bootindex=0"]
args += self._data_args + extra_args
diff --git a/tests/vm/fedora b/tests/vm/fedora
index 4843b4175e0..bd9c6cf295c 100755
--- a/tests/vm/fedora
+++ b/tests/vm/fedora
@@ -82,6 +82,7 @@ class FedoraVM(basevm.BaseVM):
self.boot(img_tmp, extra_args = [
"-bios", "pc-bios/bios-256k.bin",
"-machine", "graphics=off",
+"-device", "VGA",
"-cdrom", iso
])
self.console_init(300)
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 86770878b67..58166766d91 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -92,6 +92,7 @@ class FreeBSDVM(basevm.BaseVM):
self.boot(img_tmp, extra_args = [
"-bios", "pc-bios/bios-256k.bin",
"-machine", "graphics=off",
+"-device", "VGA",
"-cdrom", iso
])
self.console_init()
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 55590f46015..f3257bc245a 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -86,6 +86,7 @@ class NetBSDVM(basevm.BaseVM):
self.boot(img_tmp, extra_args = [
"-bios", "pc-bios/bios-256k.bin",
"-machine", "graphics=off",
+"-device", "VGA",
"-cdrom", iso
])
self.console_init()
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index ab6abbedab5..0b705f49452 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -82,6 +82,7 @@ class OpenBSDVM(basevm.BaseVM):
self.boot(img_tmp, extra_args = [
"-bios", "pc-bios/bios-256k.bin",
"-machine", "graphics=off",
+"-device", "VGA",
"-cdrom", iso
])
self.console_init()
diff --git a/tests/vm/ubuntu.i386 b/tests/vm/ubuntu.i386
index 3266038fbde..15707753353 100755
--- a/tests/vm/ubuntu.i386
+++ b/tests/vm/ubuntu.i386
@@ -36,7 +36,10 @@ class UbuntuX86VM(basevm.BaseVM):
img_tmp = img + ".tmp"
subprocess.check_call(["cp", "-f", cimg, img_tmp])
self.exec_qemu_img("resize", img_tmp, "50G")
-self.boot(img_tmp, extra_args = ["-cdrom", self.gen_cloud_init_iso()])
+self.boot(img_tmp, extra_args = [
+"-device", "VGA",
+"-cdrom", self.gen_cloud_init_iso()
+])
self.wait_ssh()
self.ssh_root_check("touch /etc/cloud/cloud-init.disabled")
self.ssh_root_check("apt-get update")
--
2.20.1
Re: [PATCH v3] qcow2: Forbid discard in qcow2 v2 images with backing files
On 3/27/20 11:48 AM, Alberto Garcia wrote: A discard request deallocates the selected clusters so they read back as zeroes. This is done by clearing the cluster offset field and setting QCOW_OFLAG_ZERO in the L2 entry. This flag is however only supported when qcow_version >= 3. In older images the cluster is simply deallocated, exposing any possible stale data from the backing file. Since discard is an advisory operation it's safer to simply forbid it in this scenario. Note that we are adding this check to qcow2_co_pdiscard() and not to qcow2_cluster_discard() or discard_in_l2_slice() because the last two are also used by qcow2_snapshot_create() to discard the clusters used by the VM state. In this case there's no risk of exposing stale data to the guest and we really want that the clusters are always discarded. Signed-off-by: Alberto Garcia --- +++ b/tests/qemu-iotests/290 + +echo +echo "### Test 'qemu-io -c discard' on a QCOW2 image without a backing file" +echo +for qcow2_compat in 0.10 1.1; do +echo "# Create an image with compat=$qcow2_compat without a backing file" +_make_test_img -o "compat=$qcow2_compat" 128k + +echo "# Fill all clusters with data and then discard them" +$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io + +echo "# Read the data from the discarded clusters" +$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io +done Should this loop also inspect qemu-img map output? + +echo +echo "### Test 'qemu-io -c discard' on a QCOW2 image with a backing file" +echo + +echo "# Create a backing image and fill it with data" +BACKING_IMG="$TEST_IMG.base" +TEST_IMG="$BACKING_IMG" _make_test_img 128k +$QEMU_IO -c 'write -P 0xff 0 128k' "$BACKING_IMG" | _filter_qemu_io + +for qcow2_compat in 0.10 1.1; do +echo "# Create an image with compat=$qcow2_compat and a backing file" +_make_test_img -o "compat=$qcow2_compat" -b "$BACKING_IMG" + +echo "# Fill all clusters with data and then discard them" +$QEMU_IO -c 'write -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c 'discard 0 128k' "$TEST_IMG" | _filter_qemu_io + +echo "# Read the data from the discarded clusters" +if [ "$qcow2_compat" = "1.1" ]; then +# In qcow2 v3 clusters are zeroed (with QCOW_OFLAG_ZERO) +$QEMU_IO -c 'read -P 0x00 0 128k' "$TEST_IMG" | _filter_qemu_io +else +# In qcow2 v2 if there's a backing image we cannot zero the clusters +# without exposing the backing file data so discard does nothing +$QEMU_IO -c 'read -P 0x01 0 128k' "$TEST_IMG" | _filter_qemu_io +fi + +echo "# Output of qemu-img map" +$QEMU_IMG map "$TEST_IMG" | _filter_testdir +done But I agree this was the more interesting one, so we at least have decent coverage of the change itself. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
[PULL v2 for 5.0 00/12] testing updates (+ one mttcg change)
The following changes since commit cfe68ae025f704f336d7dd3d1903ce37b445831d: Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-5.0-pull-request' into staging (2020-03-26 20:55:54 +) are available in the Git repository at: https://github.com/stsquad/qemu.git tags/pull-testing-270320-2 for you to fetch changes up to 41e1f0e2256e4c21bc5671ecb64191e776477c35: .travis.yml: Add a KVM-only s390x job (2020-03-27 13:43:20 +) Testing updates: - docker updates (various dependencies) - travis updates (s390x KVM build) - tweak qemu/atomic.h headers in event of clash - test/vm updates (NetBSD -> 9.0, FreeBSD -> 12.1) - disable MTTCG for mips64/mips64el Alex Bennée (3): qemu/atomic.h: add #ifdef guards for stdatomic.h tests/vm: fix basevm config configure: disable MTTCG for MIPS guests Gerd Hoffmann (4): tests/vm: write raw console log tests/vm: move vga setup tests/vm: update FreeBSD to 12.1 tests/vm: update NetBSD to 9.0 Philippe Mathieu-Daudé (5): tests/docker: Keep package list sorted tests/docker: Install gcrypt devel package in Debian image tests/docker: Use Python3 PyYAML in the Fedora image tests/docker: Add libepoxy and libudev packages to the Fedora image .travis.yml: Add a KVM-only s390x job configure| 2 +- include/qemu/atomic.h| 6 .travis.yml | 42 tests/docker/dockerfiles/centos7.docker | 6 ++-- tests/docker/dockerfiles/debian-amd64.docker | 1 + tests/docker/dockerfiles/fedora.docker | 10 +-- tests/vm/basevm.py | 23 +-- tests/vm/fedora | 1 + tests/vm/freebsd | 5 ++-- tests/vm/netbsd | 24 +--- tests/vm/openbsd | 1 + tests/vm/ubuntu.i386 | 5 +++- 12 files changed, 91 insertions(+), 35 deletions(-) -- 2.20.1
[PULL v2 01/12] qemu/atomic.h: add #ifdef guards for stdatomic.h
Deep inside the FreeBSD netmap headers we end up including stdatomic.h which clashes with qemu's atomic functions which are modelled along the C11 standard. To avoid a massive rename lets just ifdef around the problem. Signed-off-by: Alex Bennée Message-Id: <20200326170121.13045-1-alex.ben...@linaro.org> Reviewed-by: Richard Henderson diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index f9cd24c8994..ff72db51154 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -208,11 +208,14 @@ /* Provide shorter names for GCC atomic builtins, return old value */ #define atomic_fetch_inc(ptr) __atomic_fetch_add(ptr, 1, __ATOMIC_SEQ_CST) #define atomic_fetch_dec(ptr) __atomic_fetch_sub(ptr, 1, __ATOMIC_SEQ_CST) + +#ifndef atomic_fetch_add #define atomic_fetch_add(ptr, n) __atomic_fetch_add(ptr, n, __ATOMIC_SEQ_CST) #define atomic_fetch_sub(ptr, n) __atomic_fetch_sub(ptr, n, __ATOMIC_SEQ_CST) #define atomic_fetch_and(ptr, n) __atomic_fetch_and(ptr, n, __ATOMIC_SEQ_CST) #define atomic_fetch_or(ptr, n) __atomic_fetch_or(ptr, n, __ATOMIC_SEQ_CST) #define atomic_fetch_xor(ptr, n) __atomic_fetch_xor(ptr, n, __ATOMIC_SEQ_CST) +#endif #define atomic_inc_fetch(ptr)__atomic_add_fetch(ptr, 1, __ATOMIC_SEQ_CST) #define atomic_dec_fetch(ptr)__atomic_sub_fetch(ptr, 1, __ATOMIC_SEQ_CST) @@ -392,11 +395,14 @@ /* Provide shorter names for GCC atomic builtins. */ #define atomic_fetch_inc(ptr) __sync_fetch_and_add(ptr, 1) #define atomic_fetch_dec(ptr) __sync_fetch_and_add(ptr, -1) + +#ifndef atomic_fetch_add #define atomic_fetch_add(ptr, n) __sync_fetch_and_add(ptr, n) #define atomic_fetch_sub(ptr, n) __sync_fetch_and_sub(ptr, n) #define atomic_fetch_and(ptr, n) __sync_fetch_and_and(ptr, n) #define atomic_fetch_or(ptr, n) __sync_fetch_and_or(ptr, n) #define atomic_fetch_xor(ptr, n) __sync_fetch_and_xor(ptr, n) +#endif #define atomic_inc_fetch(ptr) __sync_add_and_fetch(ptr, 1) #define atomic_dec_fetch(ptr) __sync_add_and_fetch(ptr, -1) -- 2.20.1
Re: [PATCH v2] monitor/hmp-cmds: add units for migrate_parameters.
* Mao Zhongyi (maozhon...@cmss.chinamobile.com) wrote: > When running: > (qemu) info migrate_parameters > announce-initial: 50 ms > announce-max: 550 ms > announce-step: 100 ms > compress-wait-thread: on > ... > max-bandwidth: 33554432 bytes/second > downtime-limit: 300 milliseconds > x-checkpoint-delay: 2 > ... > xbzrle-cache-size: 67108864 > > add units for the parameters 'x-checkpoint-delay' and > 'xbzrle-cache-size', it's easier to read, also move > milliseconds to ms to keep the same style. > > Signed-off-by: Mao Zhongyi Thanks Reviewed-by: Dr. David Alan Gilbert (info migrate could also be fixed, but that's a separate issue) > --- > monitor/hmp-cmds.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > index 2a900a528a..790fad3afe 100644 > --- a/monitor/hmp-cmds.c > +++ b/monitor/hmp-cmds.c > @@ -436,11 +436,11 @@ void hmp_info_migrate_parameters(Monitor *mon, const > QDict *qdict) > MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH), > params->max_bandwidth); > assert(params->has_downtime_limit); > -monitor_printf(mon, "%s: %" PRIu64 " milliseconds\n", > +monitor_printf(mon, "%s: %" PRIu64 " ms\n", > MigrationParameter_str(MIGRATION_PARAMETER_DOWNTIME_LIMIT), > params->downtime_limit); > assert(params->has_x_checkpoint_delay); > -monitor_printf(mon, "%s: %u\n", > +monitor_printf(mon, "%s: %u ms\n", > MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY), > params->x_checkpoint_delay); > assert(params->has_block_incremental); > @@ -453,7 +453,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const > QDict *qdict) > monitor_printf(mon, "%s: %s\n", > MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_COMPRESSION), > MultiFDCompression_str(params->multifd_compression)); > -monitor_printf(mon, "%s: %" PRIu64 "\n", > +monitor_printf(mon, "%s: %" PRIu64 " bytes\n", > MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE), > params->xbzrle_cache_size); > monitor_printf(mon, "%s: %" PRIu64 "\n", > -- > 2.17.1 > > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
