[Qemu-devel] [PULL 08/44] ppc: Fix signal delivery in ppc-user and ppc64-user
From: Benjamin Herrenschmidt
There were a number of bugs in the implementation:
- The structure alignment was wrong for 64-bit.
- Also 64-bit only does RT signals.
- On 64-bit, we need to put a pointer to the (aligned) vector registers
in the frame and use it for restoring
- We had endian bugs when saving/restoring vector registers
- My recent fixes for exception NIP broke sigreturn in user mode
causing us to resume one instruction too far.
- Add VSR second halves
Signed-off-by: Benjamin Herrenschmidt
Signed-off-by: David Gibson
---
linux-user/main.c | 2 +-
linux-user/ppc/syscall_nr.h | 2 +
linux-user/signal.c | 124 +++-
3 files changed, 90 insertions(+), 38 deletions(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index 3ad70f8..ece5ef4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1992,12 +1992,12 @@ void cpu_loop(CPUPPCState *env)
if (ret == -TARGET_ERESTARTSYS) {
break;
}
-env->nip += 4;
if (ret == (target_ulong)(-TARGET_QEMU_ESIGRETURN)) {
/* Returning from a successful sigreturn syscall.
Avoid corrupting register state. */
break;
}
+env->nip += 4;
if (ret > (target_ulong)(-515)) {
env->crf[0] |= 0x1;
ret = -ret;
diff --git a/linux-user/ppc/syscall_nr.h b/linux-user/ppc/syscall_nr.h
index 46ed8a6..afa3654 100644
--- a/linux-user/ppc/syscall_nr.h
+++ b/linux-user/ppc/syscall_nr.h
@@ -120,7 +120,9 @@
#define TARGET_NR_sysinfo116
#define TARGET_NR_ipc117
#define TARGET_NR_fsync 118
+#if !defined(TARGET_PPC64)
#define TARGET_NR_sigreturn 119
+#endif
#define TARGET_NR_clone 120
#define TARGET_NR_setdomainname 121
#define TARGET_NR_uname 122
diff --git a/linux-user/signal.c b/linux-user/signal.c
index d3ac0e2..501a4fe 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -4408,7 +4408,12 @@ struct target_mcontext {
target_ulong mc_gregs[48];
/* Includes fpscr. */
uint64_t mc_fregs[33];
+#if defined(TARGET_PPC64)
+/* Pointer to the vector regs */
+target_ulong v_regs;
+#else
target_ulong mc_pad[2];
+#endif
/* We need to handle Altivec and SPE at the same time, which no
kernel needs to do. Fortunately, the kernel defines this bit to
be Altivec-register-large all the time, rather than trying to
@@ -4418,15 +4423,30 @@ struct target_mcontext {
uint32_t spe[33];
/* Altivec vector registers. The packing of VSCR and VRSAVE
varies depending on whether we're PPC64 or not: PPC64 splits
- them apart; PPC32 stuffs them together. */
+ them apart; PPC32 stuffs them together.
+ We also need to account for the VSX registers on PPC64
+*/
#if defined(TARGET_PPC64)
-#define QEMU_NVRREG 34
+#define QEMU_NVRREG (34 + 16)
+/* On ppc64, this mcontext structure is naturally *unaligned*,
+ * or rather it is aligned on a 8 bytes boundary but not on
+ * a 16 bytes one. This pad fixes it up. This is also why the
+ * vector regs are referenced by the v_regs pointer above so
+ * any amount of padding can be added here
+ */
+target_ulong pad;
#else
+/* On ppc32, we are already aligned to 16 bytes */
#define QEMU_NVRREG 33
#endif
-ppc_avr_t altivec[QEMU_NVRREG];
+/* We cannot use ppc_avr_t here as we do *not* want the implied
+ * 16-bytes alignment that would result from it. This would have
+ * the effect of making the whole struct target_mcontext aligned
+ * which breaks the layout of struct target_ucontext on ppc64.
+ */
+uint64_t altivec[QEMU_NVRREG][2];
#undef QEMU_NVRREG
-} mc_vregs __attribute__((__aligned__(16)));
+} mc_vregs;
};
/* See arch/powerpc/include/asm/sigcontext.h. */
@@ -4580,6 +4600,16 @@ static target_ulong get_sigframe(struct target_sigaction
*ka,
return (oldsp - frame_size) & ~0xFUL;
}
+#if ((defined(TARGET_WORDS_BIGENDIAN) && defined(HOST_WORDS_BIGENDIAN)) || \
+ (!defined(HOST_WORDS_BIGENDIAN) && !defined(TARGET_WORDS_BIGENDIAN)))
+#define PPC_VEC_HI 0
+#define PPC_VEC_LO 1
+#else
+#define PPC_VEC_HI 1
+#define PPC_VEC_LO 0
+#endif
+
+
static void save_user_regs(CPUPPCState *env, struct target_mcontext *frame)
{
target_ulong msr = env->msr;
@@ -4606,18 +4636,33 @@ static void save_user_regs(CPUPPCState *env, struct
target_mcontext *frame)
/* Save Altivec registers if necessary. */
if (env->insns_flags & PPC_ALTIVEC) {
+uint32_t *vrsave;
for (i = 0; i < ARRAY_SIZE(env->avr); i++) {
ppc_avr_t *avr = &env->avr[i];
-ppc_avr_t *vreg = &frame->mc_vregs.
[Qemu-devel] [PULL 22/44] target-ppc: consolidate load operations
From: Nikunj A Dadhania
Implement macro to consolidate load operations using newer
tcg_gen_qemu_ld functions.
Signed-off-by: Nikunj A Dadhania
Signed-off-by: David Gibson
---
target-ppc/translate.c | 58 +-
1 file changed, 20 insertions(+), 38 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index a27f455..6606969 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2462,50 +2462,32 @@ static inline void gen_align_no_le(DisasContext *ctx)
}
/*** Integer load ***/
-static inline void gen_qemu_ld8u(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-tcg_gen_qemu_ld8u(arg1, arg2, ctx->mem_idx);
-}
-
-static inline void gen_qemu_ld16u(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-TCGMemOp op = MO_UW | ctx->default_tcg_memop_mask;
-tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
-}
-
-static inline void gen_qemu_ld16s(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-TCGMemOp op = MO_SW | ctx->default_tcg_memop_mask;
-tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
-}
+#define DEF_MEMOP(op) ((op) | ctx->default_tcg_memop_mask)
-static inline void gen_qemu_ld32u(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-TCGMemOp op = MO_UL | ctx->default_tcg_memop_mask;
-tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
+#define GEN_QEMU_LOAD_TL(ldop, op) \
+static void glue(gen_qemu_, ldop)(DisasContext *ctx,\
+ TCGv val, \
+ TCGv addr)\
+{ \
+tcg_gen_qemu_ld_tl(val, addr, ctx->mem_idx, op);\
}
-static void gen_qemu_ld32u_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
-{
-TCGv tmp = tcg_temp_new();
-gen_qemu_ld32u(ctx, tmp, addr);
-tcg_gen_extu_tl_i64(val, tmp);
-tcg_temp_free(tmp);
-}
+GEN_QEMU_LOAD_TL(ld8u, DEF_MEMOP(MO_UB))
+GEN_QEMU_LOAD_TL(ld16u, DEF_MEMOP(MO_UW))
+GEN_QEMU_LOAD_TL(ld16s, DEF_MEMOP(MO_SW))
+GEN_QEMU_LOAD_TL(ld32u, DEF_MEMOP(MO_UL))
+GEN_QEMU_LOAD_TL(ld32s, DEF_MEMOP(MO_SL))
-static inline void gen_qemu_ld32s(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-TCGMemOp op = MO_SL | ctx->default_tcg_memop_mask;
-tcg_gen_qemu_ld_tl(arg1, arg2, ctx->mem_idx, op);
+#define GEN_QEMU_LOAD_64(ldop, op) \
+static void glue(gen_qemu_, glue(ldop, _i64))(DisasContext *ctx,\
+ TCGv_i64 val, \
+ TCGv addr) \
+{ \
+tcg_gen_qemu_ld_i64(val, addr, ctx->mem_idx, op); \
}
-static void gen_qemu_ld32s_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
-{
-TCGv tmp = tcg_temp_new();
-gen_qemu_ld32s(ctx, tmp, addr);
-tcg_gen_ext_tl_i64(val, tmp);
-tcg_temp_free(tmp);
-}
+GEN_QEMU_LOAD_64(ld32u, DEF_MEMOP(MO_UL))
+GEN_QEMU_LOAD_64(ld32s, DEF_MEMOP(MO_SL))
static inline void gen_qemu_ld64(DisasContext *ctx, TCGv_i64 arg1, TCGv arg2)
{
--
2.7.4
[Qemu-devel] [PULL 27/44] target-ppc: convert st[16, 32, 64]r to use new macro
From: Nikunj A Dadhania
Make byte-swap routines use the common GEN_QEMU_STORE macro
Signed-off-by: Nikunj A Dadhania
Signed-off-by: David Gibson
---
target-ppc/translate.c | 32 ++--
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 254ad40..60668c2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2510,6 +2510,9 @@ GEN_QEMU_STORE_TL(st8, DEF_MEMOP(MO_UB))
GEN_QEMU_STORE_TL(st16, DEF_MEMOP(MO_UW))
GEN_QEMU_STORE_TL(st32, DEF_MEMOP(MO_UL))
+GEN_QEMU_STORE_TL(st16r, BSWAP_MEMOP(MO_UW))
+GEN_QEMU_STORE_TL(st32r, BSWAP_MEMOP(MO_UL))
+
#define GEN_QEMU_STORE_64(stop, op) \
static void glue(gen_qemu_, glue(stop, _i64))(DisasContext *ctx, \
TCGv_i64 val, \
@@ -2521,6 +2524,10 @@ static void glue(gen_qemu_, glue(stop,
_i64))(DisasContext *ctx, \
GEN_QEMU_STORE_64(st32, DEF_MEMOP(MO_UL))
GEN_QEMU_STORE_64(st64, DEF_MEMOP(MO_Q))
+#if defined(TARGET_PPC64)
+GEN_QEMU_STORE_64(st64r, BSWAP_MEMOP(MO_Q))
+#endif
+
#define GEN_LD(name, ldop, opc, type) \
static void glue(gen_, name)(DisasContext *ctx)
\
{ \
@@ -2844,34 +2851,15 @@ GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER);
#if defined(TARGET_PPC64)
/* ldbrx */
GEN_LDX_E(ldbr, ld64ur_i64, 0x14, 0x10, PPC_NONE, PPC2_DBRX, CHK_NONE);
+/* stdbrx */
+GEN_STX_E(stdbr, st64r_i64, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE);
#endif /* TARGET_PPC64 */
/* sthbrx */
-static inline void gen_qemu_st16r(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-TCGMemOp op = MO_UW | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
-tcg_gen_qemu_st_tl(arg1, arg2, ctx->mem_idx, op);
-}
GEN_STX(sthbr, st16r, 0x16, 0x1C, PPC_INTEGER);
-
/* stwbrx */
-static inline void gen_qemu_st32r(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-TCGMemOp op = MO_UL | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
-tcg_gen_qemu_st_tl(arg1, arg2, ctx->mem_idx, op);
-}
GEN_STX(stwbr, st32r, 0x16, 0x14, PPC_INTEGER);
-#if defined(TARGET_PPC64)
-/* stdbrx */
-static inline void gen_qemu_st64r(DisasContext *ctx, TCGv arg1, TCGv arg2)
-{
-TCGMemOp op = MO_Q | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
-tcg_gen_qemu_st_i64(arg1, arg2, ctx->mem_idx, op);
-}
-GEN_STX_E(stdbr, st64r, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE);
-#endif /* TARGET_PPC64 */
-
/***Integer load and store multiple***/
/* lmw */
@@ -6619,7 +6607,7 @@ GEN_STS(stw, st32, 0x04, PPC_INTEGER)
#if defined(TARGET_PPC64)
GEN_STUX(std, st64_i64, 0x15, 0x05, PPC_64B)
GEN_STX(std, st64_i64, 0x15, 0x04, PPC_64B)
-GEN_STX_E(stdbr, st64r, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE)
+GEN_STX_E(stdbr, st64r_i64, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE)
GEN_STX_HVRM(stdcix, st64_i64, 0x15, 0x1f, PPC_CILDST)
GEN_STX_HVRM(stwcix, st32, 0x15, 0x1c, PPC_CILDST)
GEN_STX_HVRM(sthcix, st16, 0x15, 0x1d, PPC_CILDST)
--
2.7.4
[Qemu-devel] [PULL 19/44] spapr_vio: convert to trace framework instead of DPRINTF
From: Laurent Vivier
Signed-off-by: Laurent Vivier
Reviewed-by: Eric Blake
Signed-off-by: David Gibson
---
hw/ppc/spapr_vio.c | 17 +++--
hw/ppc/trace-events | 4
2 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 497028f..d68dd35 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -36,19 +36,10 @@
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_vio.h"
#include "hw/ppc/xics.h"
+#include "trace.h"
#include
-/* #define DEBUG_SPAPR */
-
-#ifdef DEBUG_SPAPR
-#define DPRINTF(fmt, ...) \
-do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-do { } while (0)
-#endif
-
static Property spapr_vio_props[] = {
DEFINE_PROP_UINT32("irq", VIOsPAPRDevice, irq, 0), \
DEFINE_PROP_END_OF_LIST(),
@@ -202,9 +193,7 @@ static target_ulong h_reg_crq(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
dev->crq.qsize = queue_len;
dev->crq.qnext = 0;
-DPRINTF("CRQ for dev 0x" TARGET_FMT_lx " registered at 0x"
-TARGET_FMT_lx "/0x" TARGET_FMT_lx "\n",
-reg, queue_addr, queue_len);
+trace_spapr_vio_h_reg_crq(reg, queue_addr, queue_len);
return H_SUCCESS;
}
@@ -214,7 +203,7 @@ static target_ulong free_crq(VIOsPAPRDevice *dev)
dev->crq.qsize = 0;
dev->crq.qnext = 0;
-DPRINTF("CRQ for dev 0x%" PRIx32 " freed\n", dev->reg);
+trace_spapr_vio_free_crq(dev->reg);
return H_SUCCESS;
}
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index 34b5ec3..2297ead 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -64,6 +64,10 @@ spapr_rtas_get_sensor_state_invalid(uint32_t index) "sensor
index: 0x%"PRIx32
spapr_rtas_ibm_configure_connector_invalid(uint32_t index) "DRC index:
0x%"PRIx32
spapr_rtas_ibm_configure_connector_missing_fdt(uint32_t index) "DRC index:
0x%"PRIx32
+# hw/ppc/spapr_vio.c
+spapr_vio_h_reg_crq(uint64_t reg, uint64_t queue_addr, uint64_t queue_len)
"CRQ for dev 0x%" PRIx64 " registered at 0x%" PRIx64 "/0x%" PRIx64
+spapr_vio_free_crq(uint32_t reg) "CRQ for dev 0x%" PRIx32 " freed"
+
# hw/ppc/ppc.c
ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds)
"adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
--
2.7.4
[Qemu-devel] [PULL 10/44] libqos: define SPAPR libqos functions
From: Laurent Vivier
Define spapr_alloc_init()/spapr_alloc_init_flags()/spapr_alloc_uninit()
to allocate and use SPAPR guest memory
Define qtest_spapr_vboot()/qtest_spapr_boot()/qtest_spapr_shutdown()
to start SPAPR guest with QOSState initialized for it (memory management)
Move qtest_irq_intercept_in() from generic part to PC part.
Signed-off-by: Laurent Vivier
Reviewed-by: Greg Kurz
Reviewed-by: David Gibson
Signed-off-by: David Gibson
---
tests/Makefile.include | 2 ++
tests/libqos/libqos-pc.c| 2 ++
tests/libqos/libqos-spapr.c | 30 ++
tests/libqos/libqos-spapr.h | 10 ++
tests/libqos/libqos.c | 1 -
tests/libqos/malloc-spapr.c | 38 ++
tests/libqos/malloc-spapr.h | 17 +
7 files changed, 99 insertions(+), 1 deletion(-)
create mode 100644 tests/libqos/libqos-spapr.c
create mode 100644 tests/libqos/libqos-spapr.h
create mode 100644 tests/libqos/malloc-spapr.c
create mode 100644 tests/libqos/malloc-spapr.h
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 6052a38..dba35a3 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -583,6 +583,8 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o
$(test-crypto-obj-y)
libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
+libqos-spapr-obj-y = $(libqos-obj-y) tests/libqos/malloc-spapr.o
+libqos-spapr-obj-y += tests/libqos/libqos-spapr.o
libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
libqos-pc-obj-y += tests/libqos/ahci.o
diff --git a/tests/libqos/libqos-pc.c b/tests/libqos/libqos-pc.c
index 72b5e3b..df34092 100644
--- a/tests/libqos/libqos-pc.c
+++ b/tests/libqos/libqos-pc.c
@@ -21,6 +21,8 @@ QOSState *qtest_pc_boot(const char *cmdline_fmt, ...)
qs = qtest_vboot(&qos_ops, cmdline_fmt, ap);
va_end(ap);
+qtest_irq_intercept_in(global_qtest, "ioapic");
+
return qs;
}
diff --git a/tests/libqos/libqos-spapr.c b/tests/libqos/libqos-spapr.c
new file mode 100644
index 000..f19408b
--- /dev/null
+++ b/tests/libqos/libqos-spapr.c
@@ -0,0 +1,30 @@
+#include "qemu/osdep.h"
+#include "libqos/libqos-spapr.h"
+#include "libqos/malloc-spapr.h"
+
+static QOSOps qos_ops = {
+.init_allocator = spapr_alloc_init_flags,
+.uninit_allocator = spapr_alloc_uninit
+};
+
+QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap)
+{
+return qtest_vboot(&qos_ops, cmdline_fmt, ap);
+}
+
+QOSState *qtest_spapr_boot(const char *cmdline_fmt, ...)
+{
+QOSState *qs;
+va_list ap;
+
+va_start(ap, cmdline_fmt);
+qs = qtest_vboot(&qos_ops, cmdline_fmt, ap);
+va_end(ap);
+
+return qs;
+}
+
+void qtest_spapr_shutdown(QOSState *qs)
+{
+return qtest_shutdown(qs);
+}
diff --git a/tests/libqos/libqos-spapr.h b/tests/libqos/libqos-spapr.h
new file mode 100644
index 000..dcb5c43
--- /dev/null
+++ b/tests/libqos/libqos-spapr.h
@@ -0,0 +1,10 @@
+#ifndef LIBQOS_SPAPR_H
+#define LIBQOS_SPAPR_H
+
+#include "libqos/libqos.h"
+
+QOSState *qtest_spapr_vboot(const char *cmdline_fmt, va_list ap);
+QOSState *qtest_spapr_boot(const char *cmdline_fmt, ...);
+void qtest_spapr_shutdown(QOSState *qs);
+
+#endif
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index c7ba441..a852dc5 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -20,7 +20,6 @@ QOSState *qtest_vboot(QOSOps *ops, const char *cmdline_fmt,
va_list ap)
cmdline = g_strdup_vprintf(cmdline_fmt, ap);
qs->qts = qtest_start(cmdline);
qs->ops = ops;
-qtest_irq_intercept_in(global_qtest, "ioapic");
if (ops && ops->init_allocator) {
qs->alloc = ops->init_allocator(ALLOC_NO_FLAGS);
}
diff --git a/tests/libqos/malloc-spapr.c b/tests/libqos/malloc-spapr.c
new file mode 100644
index 000..006404a
--- /dev/null
+++ b/tests/libqos/malloc-spapr.c
@@ -0,0 +1,38 @@
+/*
+ * libqos malloc support for SPAPR
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqos/malloc-spapr.h"
+
+#include "qemu-common.h"
+
+#define PAGE_SIZE 4096
+
+/* Memory must be a multiple of 256 MB,
+ * so we have at least 256MB
+ */
+#define SPAPR_MIN_SIZE 0x1000
+
+void spapr_alloc_uninit(QGuestAllocator *allocator)
+{
+alloc_uninit(allocator);
+}
+
+QGuestAllocator *spapr_alloc_init_flags(QAllocOpts flags)
+{
+QGuestAllocator *s;
+
+s = alloc_init_flags(flags, 1 << 20, SPAPR_MIN_SIZE);
+alloc_set_page_size(s, PAGE_SIZE);
+
+return s;
+}
+
+QGuestAllocator *spapr_alloc_init(void)
+{
+return spapr_alloc_init_flags(ALLOC_NO_FLAGS);
+}
diff --git a/tests/libqos/malloc-spapr.h b/tests/libqos/malloc-spapr.h
new file mode 100644
index 000..64d0e77
--- /dev/null
+++ b/tests/libqos/m
[Qemu-devel] [PULL 20/44] spapr_llan: convert to trace framework instead of DPRINTF
From: Laurent Vivier
Signed-off-by: Laurent Vivier
Reviewed-by: Eric Blake
Signed-off-by: David Gibson
---
hw/net/spapr_llan.c | 61 ++---
hw/net/trace-events | 16 ++
2 files changed, 41 insertions(+), 36 deletions(-)
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 4bb95a5..01ecb02 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -34,20 +34,13 @@
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_vio.h"
#include "sysemu/sysemu.h"
+#include "trace.h"
#include
#define ETH_ALEN6
#define MAX_PACKET_SIZE 65536
-/*#define DEBUG*/
-
-#ifdef DEBUG
-#define DPRINTF(fmt...) do { fprintf(stderr, fmt); } while (0)
-#else
-#define DPRINTF(fmt...)
-#endif
-
/* Compatibility flags for migration */
#define SPAPRVLAN_FLAG_RX_BUF_POOLS_BIT 0
#define SPAPRVLAN_FLAG_RX_BUF_POOLS (1 << SPAPRVLAN_FLAG_RX_BUF_POOLS_BIT)
@@ -158,8 +151,10 @@ static vlan_bd_t
spapr_vlan_get_rx_bd_from_pool(VIOsPAPRVLANDevice *dev,
return 0;
}
-DPRINTF("Found buffer: pool=%d count=%d rxbufs=%d\n", pool,
-dev->rx_pool[pool]->count, dev->rx_bufs);
+
+trace_spapr_vlan_get_rx_bd_from_pool_found(pool,
+ dev->rx_pool[pool]->count,
+ dev->rx_bufs);
/* Remove the buffer from the pool */
dev->rx_pool[pool]->count--;
@@ -186,8 +181,8 @@ static vlan_bd_t
spapr_vlan_get_rx_bd_from_page(VIOsPAPRVLANDevice *dev,
}
bd = vio_ldq(&dev->sdev, dev->buf_list + buf_ptr);
-DPRINTF("use_buf_ptr=%d bd=0x%016llx\n",
-buf_ptr, (unsigned long long)bd);
+
+trace_spapr_vlan_get_rx_bd_from_page(buf_ptr, (uint64_t)bd);
} while ((!(bd & VLAN_BD_VALID) || VLAN_BD_LEN(bd) < size + 8)
&& buf_ptr != dev->use_buf_ptr);
@@ -200,7 +195,7 @@ static vlan_bd_t
spapr_vlan_get_rx_bd_from_page(VIOsPAPRVLANDevice *dev,
dev->use_buf_ptr = buf_ptr;
vio_stq(&dev->sdev, dev->buf_list + dev->use_buf_ptr, 0);
-DPRINTF("Found buffer: ptr=%d rxbufs=%d\n", dev->use_buf_ptr,
dev->rx_bufs);
+trace_spapr_vlan_get_rx_bd_from_page_found(dev->use_buf_ptr, dev->rx_bufs);
return bd;
}
@@ -215,8 +210,7 @@ static ssize_t spapr_vlan_receive(NetClientState *nc, const
uint8_t *buf,
uint64_t handle;
uint8_t control;
-DPRINTF("spapr_vlan_receive() [%s] rx_bufs=%d\n", sdev->qdev.id,
-dev->rx_bufs);
+trace_spapr_vlan_receive(sdev->qdev.id, dev->rx_bufs);
if (!dev->isopen) {
return -1;
@@ -244,7 +238,7 @@ static ssize_t spapr_vlan_receive(NetClientState *nc, const
uint8_t *buf,
return -1;
}
-DPRINTF("spapr_vlan_receive: DMA write completed\n");
+trace_spapr_vlan_receive_dma_completed();
/* Update the receive queue */
control = VLAN_RXQC_TOGGLE | VLAN_RXQC_VALID;
@@ -258,12 +252,11 @@ static ssize_t spapr_vlan_receive(NetClientState *nc,
const uint8_t *buf,
vio_sth(sdev, VLAN_BD_ADDR(rxq_bd) + dev->rxq_ptr + 2, 8);
vio_stb(sdev, VLAN_BD_ADDR(rxq_bd) + dev->rxq_ptr, control);
-DPRINTF("wrote rxq entry (ptr=0x%llx): 0x%016llx 0x%016llx\n",
-(unsigned long long)dev->rxq_ptr,
-(unsigned long long)vio_ldq(sdev, VLAN_BD_ADDR(rxq_bd) +
-dev->rxq_ptr),
-(unsigned long long)vio_ldq(sdev, VLAN_BD_ADDR(rxq_bd) +
-dev->rxq_ptr + 8));
+trace_spapr_vlan_receive_wrote(dev->rxq_ptr,
+ vio_ldq(sdev, VLAN_BD_ADDR(rxq_bd) +
+ dev->rxq_ptr),
+ vio_ldq(sdev, VLAN_BD_ADDR(rxq_bd) +
+ dev->rxq_ptr + 8));
dev->rxq_ptr += 16;
if (dev->rxq_ptr >= VLAN_BD_LEN(rxq_bd)) {
@@ -580,8 +573,8 @@ static target_long
spapr_vlan_add_rxbuf_to_pool(VIOsPAPRVLANDevice *dev,
qsort(dev->rx_pool, RX_MAX_POOLS, sizeof(dev->rx_pool[0]),
rx_pool_size_compare);
pool = spapr_vlan_get_rx_pool_id(dev, size);
-DPRINTF("created RX pool %d for size %lld\n", pool,
-VLAN_BD_LEN(buf));
+trace_spapr_vlan_add_rxbuf_to_pool_create(pool,
+ VLAN_BD_LEN(buf));
break;
}
}
@@ -591,8 +584,8 @@ static target_long
spapr_vlan_add_rxbuf_to_pool(VIOsPAPRVLANDevice *dev,
return H_RESOURCE;
}
-DPRINTF("h_add_llan_buf(): Add buf using pool %i (size %lli, count=%i)\n",
-pool, VLAN_BD_LEN(buf), dev->rx_pool[pool]->count);
+trace_spapr_vlan_add_rxbuf_to_pool(pool, VLAN_BD_LEN(buf),
+ dev->rx_pool[pool]->count);
dev->rx_pool[pool]->bds[dev->rx_pool[pool]->co
[Qemu-devel] [PULL 00/44] ppc-for-2.8 queue 20160922
The following changes since commit a008535b9fa396226ff9cf78b8ac5f3584bda58e: build-sys: fix make install regression (2016-09-20 11:32:43 +0100) are available in the git repository at: git://github.com/dgibson/qemu.git tags/ppc-for-2.8-20160922 for you to fetch changes up to 2832da4b6fc549d5feb2cf9fe53ad98cee894327: monitor: fix crash for platforms without a CPU 0 (2016-09-22 15:53:01 +1000) ppc patch queue 2016-09-22 This is my second pull request of ppc and spapr related patches for qemu-2.8. Included here are * TCG implementations for more POWER9 instructions * Some preliminary XICS fixes in preparataion for the pnv machine type * A significant ADB (Macintosh kbd/mouse) cleanup * Some conversions to use trace instead of debug macros * Fixes to correctly handle global TLB flush synchronization in TCG. This is already a bug, but it will have much more impact when we get MTTCG * Add more qtest testcases for Power * Some MAINTAINERS updates * Assorted bugfixes This touches some test files and monitor.c which are technically outside the ppc code, but coming through this tree because the changes are primarily of interest to ppc. Benjamin Herrenschmidt (3): ppc: restrict the use of the rfi instruction ppc: Fix signal delivery in ppc-user and ppc64-user ppc/xics: An ICS with offset 0 is assumed to be uninitialized Bharata B Rao (1): spapr: Introduce sPAPRCPUCoreClass David Gibson (1): monitor: fix crash for platforms without a CPU 0 Greg Kurz (1): MAINTAINERS: add sPAPR tests John Arbuckle (4): adb-keys.h: initial commit adb.c: add support for QKeyCode adb.c: correct several key assignments adb.c: prevent NO_KEY value from going to guest Laurent Vivier (8): qtest: replace strtoXX() by qemu_strtoXX() libqos: define SPAPR libqos functions tests: add RTAS command in the protocol spapr_drc: convert to trace framework instead of DPRINTF spapr_rtas: convert to trace framework instead of DPRINTF spapr_vio: convert to trace framework instead of DPRINTF spapr_llan: convert to trace framework instead of DPRINTF spapr_vscsi: convert to trace framework instead of DPRINTF Michael Walle (1): linux-user: ppc64: fix ARCH_206 bit in AT_HWCAP Nathan Whitehorn (1): Enable H_CLEAR_MOD and H_CLEAR_REF hypercalls on KVM/PPC64. Nikunj A Dadhania (16): target-ppc: consolidate load operations target-ppc: convert ld64 to use new macro target-ppc: convert ld[16,32,64]ur to use new macro target-ppc: consolidate store operations target-ppc: convert st64 to use new macro target-ppc: convert st[16,32,64]r to use new macro target-ppc: consolidate load with reservation target-ppc: move out stqcx impementation target-ppc: consolidate store conditional target-ppc: add xxspltib instruction target-ppc: add lxsi[bw]zx instruction target-ppc: add stxsi[bh]x instruction target-ppc: add TLB_NEED_LOCAL_FLUSH flag target-ppc: add flag in check_tlb_flush() target-ppc: tlbie/tlbivax should have global effect ppc/xics: account correct irq status Rajalakshmi Srinivasaraghavan (5): target-ppc: add vector insert instructions target-ppc: add vector extract instructions target-ppc: add vector count trailing zeros instructions target-ppc: add vector bit permute doubleword instruction target-ppc: add vector permute right indexed instruction Ravi Bangoria (1): target-ppc: implement darn instruction Thomas Huth (2): MAINTAINERS: Add some missing ppc-related files ppc/kvm: Mark 64kB page size support as disabled if not available MAINTAINERS | 15 ++ hw/input/adb.c | 229 hw/intc/xics.c | 7 +- hw/net/spapr_llan.c | 61 +++--- hw/net/trace-events | 16 ++ hw/ppc/spapr.c | 9 +- hw/ppc/spapr_cpu_core.c | 104 - hw/ppc/spapr_drc.c | 54 ++--- hw/ppc/spapr_hcall.c| 6 +- hw/ppc/spapr_rtas.c | 49 +++-- hw/ppc/spapr_vio.c | 17 +- hw/ppc/trace-events | 33 +++ hw/scsi/spapr_vscsi.c | 88 hw/scsi/trace-events| 27 +++ include/hw/input/adb-keys.h | 141 include/hw/ppc/spapr_cpu_core.h | 11 +- include/hw/ppc/spapr_rtas.h | 10 + include/hw/ppc/xics.h | 2 +- linux-user/elfload.c| 8 +- linux-user/main.c | 2 +- linux-user/ppc/syscall_nr.h | 2 + linux-user/signal.c | 124 +++ monitor.c
[Qemu-devel] [PULL 09/44] qtest: replace strtoXX() by qemu_strtoXX()
From: Laurent Vivier
Check the result of qemu_strtoXX() and assert
if the string cannot be converted.
Signed-off-by: Laurent Vivier
Reviewed-by: David Gibson
Reviewed-by: Greg Kurz
Signed-off-by: David Gibson
---
qtest.c | 49 ++---
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/qtest.c b/qtest.c
index ce4c6db..649f7b2 100644
--- a/qtest.c
+++ b/qtest.c
@@ -27,6 +27,7 @@
#include "qemu/config-file.h"
#include "qemu/option.h"
#include "qemu/error-report.h"
+#include "qemu/cutils.h"
#define MAX_IRQ 256
@@ -325,12 +326,13 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
} else if (strcmp(words[0], "outb") == 0 ||
strcmp(words[0], "outw") == 0 ||
strcmp(words[0], "outl") == 0) {
-uint16_t addr;
-uint32_t value;
+unsigned long addr;
+unsigned long value;
g_assert(words[1] && words[2]);
-addr = strtoul(words[1], NULL, 0);
-value = strtoul(words[2], NULL, 0);
+g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
+g_assert(qemu_strtoul(words[2], NULL, 0, &value) == 0);
+g_assert(addr <= 0x);
if (words[0][3] == 'b') {
cpu_outb(addr, value);
@@ -344,11 +346,12 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
} else if (strcmp(words[0], "inb") == 0 ||
strcmp(words[0], "inw") == 0 ||
strcmp(words[0], "inl") == 0) {
-uint16_t addr;
+unsigned long addr;
uint32_t value = -1U;
g_assert(words[1]);
-addr = strtoul(words[1], NULL, 0);
+g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
+g_assert(addr <= 0x);
if (words[0][2] == 'b') {
value = cpu_inb(addr);
@@ -367,8 +370,8 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
uint64_t value;
g_assert(words[1] && words[2]);
-addr = strtoull(words[1], NULL, 0);
-value = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, &value) == 0);
if (words[0][5] == 'b') {
uint8_t data = value;
@@ -396,7 +399,7 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
uint64_t value = UINT64_C(-1);
g_assert(words[1]);
-addr = strtoull(words[1], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
if (words[0][4] == 'b') {
uint8_t data;
@@ -422,8 +425,8 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
char *enc;
g_assert(words[1] && words[2]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
data = g_malloc(len);
cpu_physical_memory_read(addr, data, len);
@@ -444,8 +447,8 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
gchar *b64_data;
g_assert(words[1] && words[2]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
data = g_malloc(len);
cpu_physical_memory_read(addr, data, len);
@@ -461,8 +464,8 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
size_t data_len;
g_assert(words[1] && words[2] && words[3]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
data_len = strlen(words[3]);
if (data_len < 3) {
@@ -487,12 +490,12 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
} else if (strcmp(words[0], "memset") == 0) {
uint64_t addr, len;
uint8_t *data;
-uint8_t pattern;
+unsigned long pattern;
g_assert(words[1] && words[2] && words[3]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
-pattern = strtoull(words[3], NULL, 0);
+g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
+g_assert(qemu_strtoul(words[3], NULL, 0, &pattern) == 0);
if (len) {
data = g_malloc(len);
@@ -510,8 +513,8 @@ static void qtest_process_command(CharDriverState *chr,
gchar **words)
gsize out_len;
g_assert(words[1] && words[2] && words[3]);
-addr = strtoull(words[1], NULL, 0);
-len = strtoull(words[2], NULL, 0);
+
[Qemu-devel] [PULL 13/44] adb-keys.h: initial commit
From: John Arbuckle
Add the adb-keys.h file. It maps ADB transition key codes with values.
Signed-off-by: John Arbuckle
Signed-off-by: David Gibson
---
include/hw/input/adb-keys.h | 141
1 file changed, 141 insertions(+)
create mode 100644 include/hw/input/adb-keys.h
diff --git a/include/hw/input/adb-keys.h b/include/hw/input/adb-keys.h
new file mode 100644
index 000..525fba8
--- /dev/null
+++ b/include/hw/input/adb-keys.h
@@ -0,0 +1,141 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2016 John Arbuckle
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/*
+ * adb-keys.h
+ *
+ * Provides an enum of all the Macintosh keycodes.
+ * Additional information:
http://www.archive.org/stream/apple-guide-macintosh-family-hardware/Apple_Guide_to_the_Macintosh_Family_Hardware_2e#page/n345/mode/2up
+ * page 308
+ */
+
+#ifndef ADB_KEYS_H
+#define ADB_KEYS_H
+
+enum {
+ADB_KEY_A = 0x00,
+ADB_KEY_B = 0x0b,
+ADB_KEY_C = 0x08,
+ADB_KEY_D = 0x02,
+ADB_KEY_E = 0x0e,
+ADB_KEY_F = 0x03,
+ADB_KEY_G = 0x05,
+ADB_KEY_H = 0x04,
+ADB_KEY_I = 0x22,
+ADB_KEY_J = 0x26,
+ADB_KEY_K = 0x28,
+ADB_KEY_L = 0x25,
+ADB_KEY_M = 0x2e,
+ADB_KEY_N = 0x2d,
+ADB_KEY_O = 0x1f,
+ADB_KEY_P = 0x23,
+ADB_KEY_Q = 0x0c,
+ADB_KEY_R = 0x0f,
+ADB_KEY_S = 0x01,
+ADB_KEY_T = 0x11,
+ADB_KEY_U = 0x20,
+ADB_KEY_V = 0x09,
+ADB_KEY_W = 0x0d,
+ADB_KEY_X = 0x07,
+ADB_KEY_Y = 0x10,
+ADB_KEY_Z = 0x06,
+
+ADB_KEY_0 = 0x1d,
+ADB_KEY_1 = 0x12,
+ADB_KEY_2 = 0x13,
+ADB_KEY_3 = 0x14,
+ADB_KEY_4 = 0x15,
+ADB_KEY_5 = 0x17,
+ADB_KEY_6 = 0x16,
+ADB_KEY_7 = 0x1a,
+ADB_KEY_8 = 0x1c,
+ADB_KEY_9 = 0x19,
+
+ADB_KEY_GRAVE_ACCENT = 0x32,
+ADB_KEY_MINUS = 0x1b,
+ADB_KEY_EQUAL = 0x18,
+ADB_KEY_DELETE = 0x33,
+ADB_KEY_CAPS_LOCK = 0x39,
+ADB_KEY_TAB = 0x30,
+ADB_KEY_RETURN = 0x24,
+ADB_KEY_LEFT_BRACKET = 0x21,
+ADB_KEY_RIGHT_BRACKET = 0x1e,
+ADB_KEY_BACKSLASH = 0x2a,
+ADB_KEY_SEMICOLON = 0x29,
+ADB_KEY_APOSTROPHE = 0x27,
+ADB_KEY_COMMA = 0x2b,
+ADB_KEY_PERIOD = 0x2f,
+ADB_KEY_FORWARD_SLASH = 0x2c,
+ADB_KEY_LEFT_SHIFT = 0x38,
+ADB_KEY_RIGHT_SHIFT = 0x7b,
+ADB_KEY_SPACEBAR = 0x31,
+ADB_KEY_LEFT_CONTROL = 0x36,
+ADB_KEY_RIGHT_CONTROL = 0x7d,
+ADB_KEY_LEFT_OPTION = 0x3a,
+ADB_KEY_RIGHT_OPTION = 0x7c,
+ADB_KEY_COMMAND = 0x37,
+
+ADB_KEY_KP_0 = 0x52,
+ADB_KEY_KP_1 = 0x53,
+ADB_KEY_KP_2 = 0x54,
+ADB_KEY_KP_3 = 0x55,
+ADB_KEY_KP_4 = 0x56,
+ADB_KEY_KP_5 = 0x57,
+ADB_KEY_KP_6 = 0x58,
+ADB_KEY_KP_7 = 0x59,
+ADB_KEY_KP_8 = 0x5b,
+ADB_KEY_KP_9 = 0x5c,
+ADB_KEY_KP_PERIOD = 0x41,
+ADB_KEY_KP_ENTER = 0x4c,
+ADB_KEY_KP_PLUS = 0x45,
+ADB_KEY_KP_SUBTRACT = 0x4e,
+ADB_KEY_KP_MULTIPLY = 0x43,
+ADB_KEY_KP_DIVIDE = 0x4b,
+ADB_KEY_KP_EQUAL = 0x51,
+ADB_KEY_KP_CLEAR = 0x47,
+
+ADB_KEY_UP = 0x3e,
+ADB_KEY_DOWN = 0x3d,
+ADB_KEY_LEFT = 0x3b,
+ADB_KEY_RIGHT = 0x3c,
+
+ADB_KEY_HELP = 0x72,
+ADB_KEY_HOME = 0x73,
+ADB_KEY_PAGE_UP = 0x74,
+ADB_KEY_PAGE_DOWN = 0x79,
+ADB_KEY_END = 0x77,
+ADB_KEY_FORWARD_DELETE = 0x75,
+
+ADB_KEY_ESC = 0x35,
+ADB_KEY_F1 = 0x7a,
+ADB_KEY_F2 = 0x78,
+ADB_KEY_F3 = 0x63,
+ADB_KEY_F4 = 0x76,
+ADB_KEY_F5 = 0x60,
+ADB_KEY_F6 = 0x61,
+ADB_KEY_F7 = 0x62,
+ADB_KEY_F8 = 0x64,
+ADB_KEY_F9 = 0x65,
+ADB_KEY_F10 = 0x6d,
+ADB_KEY_F11 = 0x67,
+ADB_KEY_F12 = 0x6f,
+ADB_KEY_F13 = 0x69,
+ADB_KEY_F14 = 0x6b,
+ADB_KEY_F15 = 0x71,
+
+ADB_KEY_VOLUME_UP = 0x48,
+ADB_KEY_VOLUME_DOWN = 0x49,
+ADB_KEY_VOLUME_MUTE = 0x4a,
+ADB_KEY_POWER = 0x7f7f
+};
+
+/* Could not find the value for this key. */
+/* #define ADB_KEY_EJECT */
+
+#endif /* ADB_KEYS_H */
--
2.7.4
[Qemu-devel] [PULL 06/44] target-ppc: add vector bit permute doubleword instruction
From: Rajalakshmi Srinivasaraghavan
Add vbpermd instruction from ISA 3.0.
Signed-off-by: Rajalakshmi Srinivasaraghavan
Signed-off-by: David Gibson
---
target-ppc/helper.h | 1 +
target-ppc/int_helper.c | 20
target-ppc/translate/vmx-impl.inc.c | 1 +
target-ppc/translate/vmx-ops.inc.c | 1 +
4 files changed, 23 insertions(+)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index b11c39a..e148861 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -333,6 +333,7 @@ DEF_HELPER_2(vpopcntb, void, avr, avr)
DEF_HELPER_2(vpopcnth, void, avr, avr)
DEF_HELPER_2(vpopcntw, void, avr, avr)
DEF_HELPER_2(vpopcntd, void, avr, avr)
+DEF_HELPER_3(vbpermd, void, avr, avr, avr)
DEF_HELPER_3(vbpermq, void, avr, avr, avr)
DEF_HELPER_2(vgbbd, void, avr, avr)
DEF_HELPER_3(vpmsumb, void, avr, avr, avr)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 4d1582d..b12af95 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1128,12 +1128,32 @@ void helper_vperm(CPUPPCState *env, ppc_avr_t *r,
ppc_avr_t *a, ppc_avr_t *b,
#if defined(HOST_WORDS_BIGENDIAN)
#define VBPERMQ_INDEX(avr, i) ((avr)->u8[(i)])
+#define VBPERMD_INDEX(i) (i)
#define VBPERMQ_DW(index) (((index) & 0x40) != 0)
+#define EXTRACT_BIT(avr, i, index) (extract64((avr)->u64[i], index, 1))
#else
#define VBPERMQ_INDEX(avr, i) ((avr)->u8[15-(i)])
+#define VBPERMD_INDEX(i) (1 - i)
#define VBPERMQ_DW(index) (((index) & 0x40) == 0)
+#define EXTRACT_BIT(avr, i, index) \
+(extract64((avr)->u64[1 - i], 63 - index, 1))
#endif
+void helper_vbpermd(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
+{
+int i, j;
+ppc_avr_t result = { .u64 = { 0, 0 } };
+VECTOR_FOR_INORDER_I(i, u64) {
+for (j = 0; j < 8; j++) {
+int index = VBPERMQ_INDEX(b, (i * 8) + j);
+if (index < 64 && EXTRACT_BIT(a, i, index)) {
+result.u64[VBPERMD_INDEX(i)] |= (0x80 >> j);
+}
+}
+}
+*r = result;
+}
+
void helper_vbpermq(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b)
{
int i;
diff --git a/target-ppc/translate/vmx-impl.inc.c
b/target-ppc/translate/vmx-impl.inc.c
index 982feff..cb89330 100644
--- a/target-ppc/translate/vmx-impl.inc.c
+++ b/target-ppc/translate/vmx-impl.inc.c
@@ -792,6 +792,7 @@ GEN_VXFORM_DUAL(vclzw, PPC_NONE, PPC2_ALTIVEC_207, \
vpopcntw, PPC_NONE, PPC2_ALTIVEC_207)
GEN_VXFORM_DUAL(vclzd, PPC_NONE, PPC2_ALTIVEC_207, \
vpopcntd, PPC_NONE, PPC2_ALTIVEC_207)
+GEN_VXFORM(vbpermd, 6, 23);
GEN_VXFORM(vbpermq, 6, 21);
GEN_VXFORM_NOA(vgbbd, 6, 20);
GEN_VXFORM(vpmsumb, 4, 16)
diff --git a/target-ppc/translate/vmx-ops.inc.c
b/target-ppc/translate/vmx-ops.inc.c
index 7172cdc..a944671 100644
--- a/target-ppc/translate/vmx-ops.inc.c
+++ b/target-ppc/translate/vmx-ops.inc.c
@@ -261,6 +261,7 @@ GEN_VXFORM_DUAL(vclzh, vpopcnth, 1, 29, PPC_NONE,
PPC2_ALTIVEC_207),
GEN_VXFORM_DUAL(vclzw, vpopcntw, 1, 30, PPC_NONE, PPC2_ALTIVEC_207),
GEN_VXFORM_DUAL(vclzd, vpopcntd, 1, 31, PPC_NONE, PPC2_ALTIVEC_207),
+GEN_VXFORM_300(vbpermd, 6, 23),
GEN_VXFORM_207(vbpermq, 6, 21),
GEN_VXFORM_207(vgbbd, 6, 20),
GEN_VXFORM_207(vpmsumb, 4, 16),
--
2.7.4
[Qemu-devel] [PULL 15/44] adb.c: correct several key assignments
From: John Arbuckle
The original pc_to_adb_keycode mapping did have several keys that were
incorrectly mapped. This patch fixes these mappings.
Signed-off-by: John Arbuckle
Signed-off-by: David Gibson
---
hw/input/adb.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/input/adb.c b/hw/input/adb.c
index 18c220b..12c6333 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -196,7 +196,7 @@ int qcode_to_adb_keycode[] = {
[Q_KEY_CODE_SHIFT_R] = ADB_KEY_RIGHT_SHIFT,
[Q_KEY_CODE_ALT] = ADB_KEY_LEFT_OPTION,
[Q_KEY_CODE_ALT_R] = ADB_KEY_RIGHT_OPTION,
-[Q_KEY_CODE_ALTGR] = 0,
+[Q_KEY_CODE_ALTGR] = ADB_KEY_RIGHT_OPTION,
[Q_KEY_CODE_CTRL] = ADB_KEY_LEFT_CONTROL,
[Q_KEY_CODE_CTRL_R]= ADB_KEY_RIGHT_CONTROL,
[Q_KEY_CODE_META_L]= ADB_KEY_COMMAND,
@@ -269,13 +269,13 @@ int qcode_to_adb_keycode[] = {
[Q_KEY_CODE_F10] = ADB_KEY_F10,
[Q_KEY_CODE_F11] = ADB_KEY_F11,
[Q_KEY_CODE_F12] = ADB_KEY_F12,
-[Q_KEY_CODE_PRINT] = 0,
-[Q_KEY_CODE_SYSRQ] = 0,
+[Q_KEY_CODE_PRINT] = ADB_KEY_F13,
+[Q_KEY_CODE_SYSRQ] = ADB_KEY_F13,
[Q_KEY_CODE_SCROLL_LOCK] = ADB_KEY_F14,
-[Q_KEY_CODE_PAUSE] = 0,
+[Q_KEY_CODE_PAUSE] = ADB_KEY_F15,
[Q_KEY_CODE_NUM_LOCK] = ADB_KEY_KP_CLEAR,
-[Q_KEY_CODE_KP_EQUALS] = 0,
+[Q_KEY_CODE_KP_EQUALS] = ADB_KEY_KP_EQUAL,
[Q_KEY_CODE_KP_DIVIDE] = ADB_KEY_KP_DIVIDE,
[Q_KEY_CODE_KP_MULTIPLY] = ADB_KEY_KP_MULTIPLY,
[Q_KEY_CODE_KP_SUBTRACT] = ADB_KEY_KP_SUBTRACT,
@@ -298,7 +298,7 @@ int qcode_to_adb_keycode[] = {
[Q_KEY_CODE_LEFT] = ADB_KEY_LEFT,
[Q_KEY_CODE_RIGHT] = ADB_KEY_RIGHT,
-[Q_KEY_CODE_HELP] = 0,
+[Q_KEY_CODE_HELP] = ADB_KEY_HELP,
[Q_KEY_CODE_INSERT]= ADB_KEY_HELP,
[Q_KEY_CODE_DELETE]= ADB_KEY_FORWARD_DELETE,
[Q_KEY_CODE_HOME] = ADB_KEY_HOME,
--
2.7.4
[Qemu-devel] [PULL 12/44] MAINTAINERS: add sPAPR tests
From: Greg Kurz Signed-off-by: Greg Kurz Signed-off-by: David Gibson --- MAINTAINERS | 4 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index a9fab46..847b614 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -628,6 +628,10 @@ F: pc-bios/spapr-rtas.bin F: pc-bios/slof.bin F: docs/specs/ppc-spapr-hcalls.txt F: docs/specs/ppc-spapr-hotplug.txt +F: tests/spapr* +F: tests/libqos/*spapr* +F: tests/rtas* +F: tests/libqos/rtas* virtex_ml507 M: Edgar E. Iglesias -- 2.7.4
Re: [Qemu-devel] pseries-2.6 migration from QEMU-2.6 to QEMU-2.7 broken
On 09/22/2016 08:00 AM, Bharata B Rao wrote:
> On Thu, Sep 22, 2016 at 03:30:08PM +1000, David Gibson wrote:
>> On Thu, Sep 22, 2016 at 10:51:05AM +0530, Bharata B Rao wrote:
>>> Hi,
>>>
>>> Nikunj and I realized that migrating pseries-2.6 guest from QEMU-2.6
>>> to newer QEMU-2.7 is broken like this:
>>>
>>> qemu-system-ppc64: error while loading state for instance 0x0 of device
>>> 'cpu'
>>> qemu-system-ppc64: load of migration failed: Invalid argument
>>>
>>> Bisecting tells us that 4e0806110c8b896ceff3490f15a616e8b3165efe
>>> (ppc: Add PPC_64H instruction flag to POWER7 and POWER8) is the
>>> first bad commit. Along with this there are other 3 similar commits
>>> which add new bits to insns_flags and insns_flags2 fields of POWER7
>>> and POWER8 CPUs.
>>>
>>> 4e0806110c8b896ceff3490f15a616e8b3165efe Adds PPC_64H to POWER7 and POWER8
>>> dfdd3e43620a6cd4f2be31da5a257c84a16fc000 Adds PPC_64BX to POWER7
>>> b781537560e3b968b6fe1395e3d07bd67f0009ba Adds PPC_CILDST to POWER7 and
>>> POWER8
>>> 7778a575c7055276afdd01737e9d1029a65f923d Adds PPC2_PM_ISA206 to POWER7 and
>>> POWER8
>>>
>>> The flag values are expected to remain same for a machine version for
>>> the migration to succeed, but this expectation is broken now. Should
>>> we make the addition of these flags conditional on machine type version ?
>>> But these flags are part of POWER8 CPU definition which is common for
>>> both pseries and upcoming powernv.
>>
>> Can you step me through how the new flags are breaking the migration?
>> It's not immediately obvious to me.
>
> Here is what I understand. Given below is the pruned vmstate_ppc_cpu
> data structure.
>
> const VMStateDescription vmstate_ppc_cpu = {
> .name = "cpu",
> .fields = (VMStateField[]) {
> /* Sanity checking */
> VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
> VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
> VMSTATE_END_OF_LIST()
> },
> };
>
> When pseries-2.6 guest is started at the source with QEMU-2.6, insns_flags
> and insns_flags2 will not have PPC_64H, PPC_64BX, PPC_CILDST, PPC2_PM_ISA206
> set. However at the target when pseries-2.6 guest is started with QEMU-2.7,
> these flags will be set. And I believe VMSTATE_UINT64_EQUAL checks above
> will cause migration to fail.
So does this mean that we can not add support for new instructions in
qemu without breaking migration with older versions ?
If so, that is really bad, we need to find a way to fix this. Should we
add a 'version' to insns_flags* ?
C.
[Qemu-devel] [PULL 01/44] MAINTAINERS: Add some missing ppc-related files
From: Thomas Huth There are some powerpc related files in the QEMU source tree which are currently not covered by the MAINTAINERS file and thus not properly classified by the get_maintainer.pl script. So let's add them to the proper sections. Signed-off-by: Thomas Huth Signed-off-by: David Gibson --- MAINTAINERS | 11 +++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 09d13bf..a9fab46 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -172,6 +172,7 @@ L: qemu-...@nongnu.org S: Maintained F: target-ppc/ F: hw/ppc/ +F: include/hw/ppc/ F: disas/ppc.c S390 @@ -574,6 +575,9 @@ L: qemu-...@nongnu.org S: Supported F: hw/ppc/e500.[hc] F: hw/ppc/e500plat.c +F: include/hw/ppc/ppc_e500.h +F: include/hw/pci-host/ppce500.h +F: pc-bios/u-boot.e500 mpc8544ds M: Alexander Graf @@ -591,6 +595,8 @@ F: hw/ppc/mac_newworld.c F: hw/pci-host/uninorth.c F: hw/pci-bridge/dec.[hc] F: hw/misc/macio/ +F: include/hw/ppc/mac_dbdma.h +F: hw/nvram/mac_nvram.c Old World M: Alexander Graf @@ -618,6 +624,10 @@ F: include/hw/*/spapr* F: hw/*/xics* F: include/hw/*/xics* F: pc-bios/spapr-rtas/* +F: pc-bios/spapr-rtas.bin +F: pc-bios/slof.bin +F: docs/specs/ppc-spapr-hcalls.txt +F: docs/specs/ppc-spapr-hotplug.txt virtex_ml507 M: Edgar E. Iglesias @@ -815,6 +825,7 @@ M: Alexander Graf L: qemu-...@nongnu.org S: Odd Fixes F: hw/ppc/ppc4*.c +F: include/hw/ppc/ppc4xx.h ppce500 M: Alexander Graf -- 2.7.4
[Qemu-devel] [PULL 04/44] target-ppc: add vector extract instructions
From: Rajalakshmi Srinivasaraghavan
The following vector extract instructions are added from ISA 3.0.
vextractub - Vector Extract Unsigned Byte
vextractuh - Vector Extract Unsigned Halfword
vextractuw - Vector Extract Unsigned Word
vextractd - Vector Extract Unsigned Doubleword
Signed-off-by: Rajalakshmi Srinivasaraghavan
Signed-off-by: David Gibson
---
target-ppc/helper.h | 4
target-ppc/int_helper.c | 25 +
target-ppc/translate/vmx-impl.inc.c | 10 ++
target-ppc/translate/vmx-ops.inc.c | 10 +++---
4 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 5ae10bc..686ce79 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -250,6 +250,10 @@ DEF_HELPER_2(vspltisw, void, avr, i32)
DEF_HELPER_3(vspltb, void, avr, avr, i32)
DEF_HELPER_3(vsplth, void, avr, avr, i32)
DEF_HELPER_3(vspltw, void, avr, avr, i32)
+DEF_HELPER_3(vextractub, void, avr, avr, i32)
+DEF_HELPER_3(vextractuh, void, avr, avr, i32)
+DEF_HELPER_3(vextractuw, void, avr, avr, i32)
+DEF_HELPER_3(vextractd, void, avr, avr, i32)
DEF_HELPER_3(vinsertb, void, avr, avr, i32)
DEF_HELPER_3(vinserth, void, avr, avr, i32)
DEF_HELPER_3(vinsertw, void, avr, avr, i32)
diff --git a/target-ppc/int_helper.c b/target-ppc/int_helper.c
index 66a3d87..9b81d91 100644
--- a/target-ppc/int_helper.c
+++ b/target-ppc/int_helper.c
@@ -1812,6 +1812,31 @@ VINSERT(h, u16)
VINSERT(w, u32)
VINSERT(d, u64)
#undef VINSERT
+#if defined(HOST_WORDS_BIGENDIAN)
+#define VEXTRACT(suffix, element)\
+void helper_vextract##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t index) \
+{\
+uint32_t es = sizeof(r->element[0]); \
+memmove(&r->u8[8 - es], &b->u8[index], es); \
+memset(&r->u8[8], 0, 8); \
+memset(&r->u8[0], 0, 8 - es);\
+}
+#else
+#define VEXTRACT(suffix, element)\
+void helper_vextract##suffix(ppc_avr_t *r, ppc_avr_t *b, uint32_t index) \
+{\
+uint32_t es = sizeof(r->element[0]); \
+uint32_t s = (16 - index) - es; \
+memmove(&r->u8[8], &b->u8[s], es); \
+memset(&r->u8[0], 0, 8); \
+memset(&r->u8[8 + es], 0, 8 - es); \
+}
+#endif
+VEXTRACT(ub, u8)
+VEXTRACT(uh, u16)
+VEXTRACT(uw, u32)
+VEXTRACT(d, u64)
+#undef VEXTRACT
#define VSPLTI(suffix, element, splat_type) \
void helper_vspltis##suffix(ppc_avr_t *r, uint32_t splat) \
diff --git a/target-ppc/translate/vmx-impl.inc.c
b/target-ppc/translate/vmx-impl.inc.c
index 59ae68a..8e66ea0 100644
--- a/target-ppc/translate/vmx-impl.inc.c
+++ b/target-ppc/translate/vmx-impl.inc.c
@@ -664,6 +664,10 @@ static void glue(gen_, name)(DisasContext *ctx)
\
GEN_VXFORM_UIMM(vspltb, 6, 8);
GEN_VXFORM_UIMM(vsplth, 6, 9);
GEN_VXFORM_UIMM(vspltw, 6, 10);
+GEN_VXFORM_UIMM_SPLAT(vextractub, 6, 8, 15);
+GEN_VXFORM_UIMM_SPLAT(vextractuh, 6, 9, 14);
+GEN_VXFORM_UIMM_SPLAT(vextractuw, 6, 10, 12);
+GEN_VXFORM_UIMM_SPLAT(vextractd, 6, 11, 8);
GEN_VXFORM_UIMM_SPLAT(vinsertb, 6, 12, 15);
GEN_VXFORM_UIMM_SPLAT(vinserth, 6, 13, 14);
GEN_VXFORM_UIMM_SPLAT(vinsertw, 6, 14, 12);
@@ -672,6 +676,12 @@ GEN_VXFORM_UIMM_ENV(vcfux, 5, 12);
GEN_VXFORM_UIMM_ENV(vcfsx, 5, 13);
GEN_VXFORM_UIMM_ENV(vctuxs, 5, 14);
GEN_VXFORM_UIMM_ENV(vctsxs, 5, 15);
+GEN_VXFORM_DUAL(vspltb, PPC_NONE, PPC2_ALTIVEC_207,
+ vextractub, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vsplth, PPC_NONE, PPC2_ALTIVEC_207,
+ vextractuh, PPC_NONE, PPC2_ISA300);
+GEN_VXFORM_DUAL(vspltw, PPC_NONE, PPC2_ALTIVEC_207,
+ vextractuw, PPC_NONE, PPC2_ISA300);
GEN_VXFORM_DUAL(vspltisb, PPC_NONE, PPC2_ALTIVEC_207,
vinsertb, PPC_NONE, PPC2_ISA300);
GEN_VXFORM_DUAL(vspltish, PPC_NONE, PPC2_ALTIVEC_207,
diff --git a/target-ppc/translate/vmx-ops.inc.c
b/target-ppc/translate/vmx-ops.inc.c
index e6abeae..01d36bb 100644
--- a/target-ppc/translate/vmx-ops.inc.c
+++ b/target-ppc/translate/vmx-ops.inc.c
@@ -197,6 +197,13 @@ GEN_VXRFORM_DUAL(vcmpbfp, vcmpgtsd, 3, 15, PPC_ALTIVEC,
PPC_NONE)
#define GEN_VXFORM_DUAL_INV(name0, name1, opc2, opc3, inval0, inval1, type) \
GEN_OPCODE_DUAL(name0##_##name1, 0x04, opc2, opc3, inval0, inval1, type, \
PPC_NONE)
+GEN_VXFORM_DUAL_INV(vspltb, vextractub, 6, 8, 0x, 0x10,
+
[Qemu-devel] [PULL 02/44] ppc: restrict the use of the rfi instruction
From: Benjamin Herrenschmidt
Power ISA 2.x has deleted the rfi instruction and rfid shoud be used
instead on cpus following this instruction set or later.
This will raise an invalid exception when rfi is used on such
processors: Book3S 64-bit processors.
Signed-off-by: Benjamin Herrenschmidt
Reviewed-by: David Gibson
[clg: the required fix in openbios, commit b747b6acc272 ('ppc: use
rfid when running under a CPU from the 970 family.'), is now
merged in qemu under commit 5cebd885d0d2 ('Update OpenBIOS
images to b747b6a built from submodule.') ]
Signed-off-by: Cédric Le Goater
Reviewed-by: Thomas Huth
Signed-off-by: David Gibson
---
target-ppc/translate.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 618334a..f01ce1e 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -3585,10 +3585,13 @@ static void gen_rfi(DisasContext *ctx)
#if defined(CONFIG_USER_ONLY)
GEN_PRIV;
#else
-/* FIXME: This instruction doesn't exist anymore on 64-bit server
- * processors compliant with arch 2.x, we should remove it there,
- * but we need to fix OpenBIOS not to use it on 970 first
+/* This instruction doesn't exist anymore on 64-bit server
+ * processors compliant with arch 2.x
*/
+if (ctx->insns_flags & PPC_SEGMENT_64B) {
+gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+return;
+}
/* Restore CPU state */
CHK_SV;
gen_update_cfar(ctx, ctx->nip - 4);
--
2.7.4
[Qemu-devel] [PATCH] qtest: fix make check complaint in crypto module
CCtests/test-crypto-tlscredsx509.o CCtests/crypto-tls-x509-helpers.o CCtests/pkix_asn1_tab.o tests/pkix_asn1_tab.c:7:22: warning: libtasn1.h: No such file or directory tests/pkix_asn1_tab.c:9: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘pkix_asn1_tab’ make: *** [tests/pkix_asn1_tab.o] Error 1 Signed-off-by: Gonglei --- tests/Makefile.include | 4 1 file changed, 4 insertions(+) diff --git a/tests/Makefile.include b/tests/Makefile.include index e560ecb..ad6950f 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -92,8 +92,10 @@ gcov-files-test-write-threshold-y = block/write-threshold.c check-unit-y += tests/test-crypto-hash$(EXESUF) check-unit-y += tests/test-crypto-cipher$(EXESUF) check-unit-y += tests/test-crypto-secret$(EXESUF) +ifeq ($(CONFIG_TASN1),y) check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF) check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlssession$(EXESUF) +endif ifneq (,$(findstring qemu-ga,$(TOOLS))) check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF) endif @@ -101,7 +103,9 @@ check-unit-y += tests/test-timed-average$(EXESUF) check-unit-y += tests/test-io-task$(EXESUF) check-unit-y += tests/test-io-channel-socket$(EXESUF) check-unit-y += tests/test-io-channel-file$(EXESUF) +ifeq ($(CONFIG_TASN1),y) check-unit-$(CONFIG_GNUTLS) += tests/test-io-channel-tls$(EXESUF) +endif check-unit-y += tests/test-io-channel-command$(EXESUF) check-unit-y += tests/test-io-channel-buffer$(EXESUF) check-unit-y += tests/test-base64$(EXESUF) -- 1.7.12.4
Re: [Qemu-devel] [PATCH] net: mcf: check buffer descriptor length
Hello Paolo,
+-- On Wed, 21 Sep 2016, Paolo Bonzini wrote --+
| Not exactly, because addr changes on every call to mcf_fec_read_bd.
True, but the initial address 's->tx_descriptor' and 's->etdsr' could be set
by user in imx_fec_write(). If bd.flags is set to FEC_BD_W, addr is reset to
initial s->tx_descriptor value of s->etdsr.
if ((bd.flags & FEC_BD_W) != 0) {
addr = s->etdsr;
}
| You can add a limit (e.g. 1024 or 2048 descriptors), but the patches are
| incorrect.
Okay, I'll send a revised patch.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
[Qemu-devel] [PATCH] crypto: add CTR mode support
Introduce CTR mode support for the cipher APIs.
CTR mode uses a counter rather than a traditional IV.
The counter has additional properties, including a nonce
and initial counter block. We reuse the ctx->iv as
the counter for conveniences.
Both libgcrypt and nettle support CTR mode, the
cipher-builtin doesn't support yet.
Signed-off-by: Gonglei
---
crypto/cipher-gcrypt.c | 25 -
crypto/cipher-nettle.c | 15 -
crypto/cipher.c| 1 +
include/crypto/cipher.h| 6 ++---
qapi/crypto.json | 3 ++-
tests/test-crypto-cipher.c | 55 ++
6 files changed, 94 insertions(+), 11 deletions(-)
diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c
index da3f4c7..97b015a 100644
--- a/crypto/cipher-gcrypt.c
+++ b/crypto/cipher-gcrypt.c
@@ -48,6 +48,7 @@ struct QCryptoCipherGcrypt {
gcry_cipher_hd_t handle;
gcry_cipher_hd_t tweakhandle;
size_t blocksize;
+/* Initialization vector or Counter */
uint8_t *iv;
};
@@ -69,6 +70,9 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
case QCRYPTO_CIPHER_MODE_CBC:
gcrymode = GCRY_CIPHER_MODE_CBC;
break;
+case QCRYPTO_CIPHER_MODE_CTR:
+gcrymode = GCRY_CIPHER_MODE_CTR;
+break;
default:
error_setg(errp, "Unsupported cipher mode %s",
QCryptoCipherMode_lookup[mode]);
@@ -339,12 +343,21 @@ int qcrypto_cipher_setiv(QCryptoCipher *cipher,
if (ctx->iv) {
memcpy(ctx->iv, iv, niv);
} else {
-gcry_cipher_reset(ctx->handle);
-err = gcry_cipher_setiv(ctx->handle, iv, niv);
-if (err != 0) {
-error_setg(errp, "Cannot set IV: %s",
- gcry_strerror(err));
-return -1;
+if (cipher->mode == QCRYPTO_CIPHER_MODE_CTR) {
+err = gcry_cipher_setctr(ctx->handle, iv, niv);
+if (err != 0) {
+error_setg(errp, "Cannot set Counter: %s",
+ gcry_strerror(err));
+return -1;
+}
+} else {
+gcry_cipher_reset(ctx->handle);
+err = gcry_cipher_setiv(ctx->handle, iv, niv);
+if (err != 0) {
+error_setg(errp, "Cannot set IV: %s",
+ gcry_strerror(err));
+return -1;
+}
}
}
diff --git a/crypto/cipher-nettle.c b/crypto/cipher-nettle.c
index 879d831..4b673aa 100644
--- a/crypto/cipher-nettle.c
+++ b/crypto/cipher-nettle.c
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
typedef void (*QCryptoCipherNettleFuncWrapper)(const void *ctx,
size_t length,
@@ -186,7 +187,7 @@ struct QCryptoCipherNettle {
QCryptoCipherNettleFuncNative alg_decrypt_native;
QCryptoCipherNettleFuncWrapper alg_encrypt_wrapper;
QCryptoCipherNettleFuncWrapper alg_decrypt_wrapper;
-
+/* Initialization vector or Counter */
uint8_t *iv;
size_t blocksize;
};
@@ -225,6 +226,7 @@ QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm
alg,
case QCRYPTO_CIPHER_MODE_ECB:
case QCRYPTO_CIPHER_MODE_CBC:
case QCRYPTO_CIPHER_MODE_XTS:
+case QCRYPTO_CIPHER_MODE_CTR:
break;
default:
error_setg(errp, "Unsupported cipher mode %s",
@@ -430,6 +432,12 @@ int qcrypto_cipher_encrypt(QCryptoCipher *cipher,
ctx->iv, len, out, in);
break;
+case QCRYPTO_CIPHER_MODE_CTR:
+ctr_crypt(ctx->ctx, ctx->alg_encrypt_native,
+ctx->blocksize, ctx->iv,
+len, out, in);
+break;
+
default:
error_setg(errp, "Unsupported cipher mode %s",
QCryptoCipherMode_lookup[cipher->mode]);
@@ -469,6 +477,11 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
ctx->alg_encrypt_wrapper, ctx->alg_decrypt_wrapper,
ctx->iv, len, out, in);
break;
+case QCRYPTO_CIPHER_MODE_CTR:
+ctr_crypt(ctx->ctx, ctx->alg_encrypt_native,
+ctx->blocksize, ctx->iv,
+len, out, in);
+break;
default:
error_setg(errp, "Unsupported cipher mode %s",
diff --git a/crypto/cipher.c b/crypto/cipher.c
index cafb454..a9bca41 100644
--- a/crypto/cipher.c
+++ b/crypto/cipher.c
@@ -55,6 +55,7 @@ static bool mode_need_iv[QCRYPTO_CIPHER_MODE__MAX] = {
[QCRYPTO_CIPHER_MODE_ECB] = false,
[QCRYPTO_CIPHER_MODE_CBC] = true,
[QCRYPTO_CIPHER_MODE_XTS] = true,
+[QCRYPTO_CIPHER_MODE_CTR] = true,
};
diff --git a/include/crypto/cipher.h b/include/crypto/cipher.h
index 376654d..f9015e1 100644
--- a/include/crypto/cipher.h
+++ b/include/crypto/cipher.h
@@ -213,16 +213,16 @@ int qcrypto_cipher_decrypt(QCryptoCipher *cipher,
/**
* qcrypto_cipher_setiv:
* @cipher: the cipher object
- * @iv: the initialization vector bytes
+ * @iv:
Re: [Qemu-devel] KVM-PR is broken with current QEMU
On Wed, 21 Sep 2016 07:45:35 +1000
Benjamin Herrenschmidt wrote:
> On Tue, 2016-09-20 at 13:44 +0200, Thomas Huth wrote:
> >
> > Seems like KVM PR is using the "degraded" ISA variants (without the
> > 1TB
> > segments), but the new POWERPC_MMU_64K flag has not been added to
> > those.
> > Has this been done on purpose, or was this just by accident?
> > I can make KVM PR working again with the following patch:
> >
> > diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> > index 2864105..36694cb 100644
> > --- a/target-ppc/cpu-qom.h
> > +++ b/target-ppc/cpu-qom.h
> > @@ -81,6 +81,7 @@ enum powerpc_mmu_t {
> > | POWERPC_MMU_AMR | 0x0003,
> > /* Architecture 2.06 "degraded" (no 1T segments) */
> > POWERPC_MMU_2_06a = POWERPC_MMU_64 | POWERPC_MMU_AMR
> > + | POWERPC_MMU_64K
> > | 0x0003,
> > /* Architecture 2.07 variant */
> > POWERPC_MMU_2_07 = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> > @@ -88,6 +89,7 @@ enum powerpc_mmu_t {
> > | POWERPC_MMU_AMR | 0x0004,
> > /* Architecture 2.07 "degraded" (no 1T segments) */
> > POWERPC_MMU_2_07a = POWERPC_MMU_64 | POWERPC_MMU_AMR
> > + | POWERPC_MMU_64K
> > | 0x0004,
> > };
> >
> > However, not sure whether this is the right fix ... Cédric, Ben, any
> > ideas?
>
> Oh I thought I had removed the degraded variants ... Definitely looks
> like an accident. I *think* PR KVM supports 64K pages, no ? If not,
> then we shouldn't enable the flag.. somebody needs to check the kernel.
Yes, it supports 64k pages - but only on POWER8, not on POWER8E or
POWER8NVL yet. I've posted a patch to fix this here:
https://patchwork.ozlabs.org/patch/672841/
Thomas
Re: [Qemu-devel] [PATCH v2 2/2] e1000: fix buliding complaint
On 2016年09月13日 14:00, Dmitry Fleytman wrote: >On 9 Sep 2016, at 10:15 AM, Gonglei (Arei) wrote: > >Who can pick up this patch? Dmitry or Jason? Thanks! Jason, would you please? Picked in my tree. Thanks
Re: [Qemu-devel] [PATCH V14 00/12] Introduce COLO-Proxy
On 2016年09月08日 17:46, Zhang Chen wrote: COLO-proxy is a part of COLO project. COLO project is composed of COLO-frame, COLO-proxy and block-replication. It is used to compare the network package to help COLO decide whether to do checkpoint. With COLO-proxy's help, COLO greatly improves the performance. The filter-redirector, filter-mirror, colo-compare and filter-rewriter compose the COLO-proxy. COLO-compare It is used to compare the network package to help COLO decide whether to do checkpoint. Filter-rewriter It will rewrite some of secondary packet to make secondary guest's connection established successfully. In this module we will rewrite tcp packet's ack to the secondary from primary,and rewrite tcp packet's seq to the primary from secondary. The full version in this github: https://github.com/zhangckid/qemu/tree/colo-v2.7-proxy-mode-compare-and-rewriter-sep8 v14: - fix some coding style problems - move patch "add docs/colo-proxy.txt" to the end of this series - fix the mingw build issue Applied, thanks. v13: - add docs/colo-proxy.txt - add MAINTAINERS - remove unnecessary .h - remove QTAILQ_ENTRY(CompareState) - fix some comments - add find_and_check_chardev() to avoid code duplication - remove the "is_unix" in patch3 - change error_report() to trace in patch4 - use l2hdr_len here instead of ETH_HLEP - fix code style - remove colo_rm_connection() - remove hashtable_size - change g_queue_foreach() to g_queue_find_custom() in patch7 - change trace_colo_compare_tcp_miscompare() to fprintf() in patch8 - add codes not queue vlan packets v12: - add qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext to this series as the first patch. - update COLO net ascii figure. - add chardev socket check. - fix some typo. - add some comments. - rename net/colo-base.c to net/colo.c - rename network/transport_layer to network/transport_header. - move the job that clear coon_list when hashtable_size oversize to connection_get. - reuse connection_destroy() do colo_rm_connection(). - fix pkt mem leak in colo_compare_connection(). (result be released in g_queue_remove(), so it were not leak) - rename thread_name "compare" to "colo-compare". - change icmp compare to memcmp(). v11: - Make patch 5 to a independent patch series. [PATCH V3] qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext - For Jason's comments, merge filter-rewriter to this series. (patch 7,8,9) - Add reverse_connection_key() - remove conn_list in filter-rewriter - remove unprocessed_connections - add some comments v10: - fix typo - Should we make patch 5 independent with this series? This patch just add a API for qemu-char. v9: p5: - use chr_update_read_handler_full() replace the chr_update_read_handler() - use io_watch_poll_prepare_full() replace the io_watch_poll_prepare() - use io_watch_poll_funcs_full replace the io_watch_poll_funcs - avoid code duplication v8: p5: - add new patch: qemu-char: Add qemu_chr_add_handlers_full() for GMaincontext v7: p5: - add [PATCH]qemu-char: Fix context for g_source_attach() in this patch series. v6: p6: - add more commit log. - fix icmp comparison to compare all packet. p5: - add more cpmments in commit log. - change REGULAR_CHECK_MS to REGULAR_PACKET_CHECK_MS - make check old packet independent to compare thread - remove thread_status p4: - change this patch only about Connection and ConnectionKey. - add some comments in commit log. - remove mode in fill_connection_key(). - fix some comments and bug. - move colo_conn_state to patch of "work with colo-frame" - remove conn_list_lock. - add MAX_QUEUE_SIZE, if primary_list or secondary_list biger than MAX_QUEUE_SIZE we will drop packet. p3: - add new independent kernel jhash patch. p2: - add new independent colo-base patch. p1: - add a ascii figure and some comments to explain it - move trace.h to p2 - move QTAILQ_HEAD(, CompareState) net_compares to patch of "work with colo-frame" - add some comments in qemu-option.hx v5: p3: - comments from Jason we poll and handle chardev in comapre thread, Through this way, there's no need for extra synchronization with main loop this depend on another patch: qemu-char: Fix context for g_source_attach() - remove QemuEvent p2: - remove conn->list_lock p1: - move compare_pri/sec_chr_in to p3 - move compare_chr_send to p2 v4: p4: - add some comments - fix some trace-events - fix tcp compare error p3: - add rcu_read_lock(). - fix trace name - fix jason's other comments - rebase some Dave's branch function p2: - colo_compare_connection() change g_queue_push_head() to - g_queu
Re: [Qemu-devel] [PATCH v4 7/9] ppc/xics: Add "native" XICS subclass
On 09/22/2016 02:02 AM, David Gibson wrote:
> On Mon, Sep 19, 2016 at 11:59:35AM +0530, Nikunj A Dadhania wrote:
>> From: Benjamin Herrenschmidt
>>
>> This provides MMIO based ICP access as found on POWER8
>>
>> Signed-off-by: Benjamin Herrenschmidt
>> Signed-off-by: Nikunj A Dadhania
>> ---
>> default-configs/ppc64-softmmu.mak | 3 +-
>> hw/intc/Makefile.objs | 1 +
>> hw/intc/xics_native.c | 295
>> ++
>> include/hw/ppc/xics.h | 14 ++
>> 4 files changed, 312 insertions(+), 1 deletion(-)
>> create mode 100644 hw/intc/xics_native.c
>>
>> diff --git a/default-configs/ppc64-softmmu.mak
>> b/default-configs/ppc64-softmmu.mak
>> index c4be59f..315e30b 100644
>> --- a/default-configs/ppc64-softmmu.mak
>> +++ b/default-configs/ppc64-softmmu.mak
>> @@ -48,8 +48,9 @@ CONFIG_PLATFORM_BUS=y
>> CONFIG_ETSEC=y
>> CONFIG_LIBDECNUMBER=y
>> # For pSeries
>> -CONFIG_XICS=$(CONFIG_PSERIES)
>> +CONFIG_XICS=$(or $(CONFIG_PSERIES),$(CONFIG_POWERNV))
>> CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
>> +CONFIG_XICS_NATIVE=$(CONFIG_POWERNV)
>> CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
>> # For PReP
>> CONFIG_MC146818RTC=y
>> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
>> index 05ec21b..7be5dfc 100644
>> --- a/hw/intc/Makefile.objs
>> +++ b/hw/intc/Makefile.objs
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
>> obj-$(CONFIG_SH4) += sh_intc.o
>> obj-$(CONFIG_XICS) += xics.o
>> obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
>> +obj-$(CONFIG_XICS_NATIVE) += xics_native.o
>> obj-$(CONFIG_XICS_KVM) += xics_kvm.o
>> obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
>> obj-$(CONFIG_S390_FLIC) += s390_flic.o
>> diff --git a/hw/intc/xics_native.c b/hw/intc/xics_native.c
>> new file mode 100644
>> index 000..26e45cc
>> --- /dev/null
>> +++ b/hw/intc/xics_native.c
>> @@ -0,0 +1,295 @@
>> +/*
>> + * QEMU PowerPC hardware System Emulator
>> + *
>> + * Native version of ICS/ICP
>> + *
>> + * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> copy
>> + * of this software and associated documentation files (the "Software"), to
>> deal
>> + * in the Software without restriction, including without limitation the
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/hw.h"
>> +#include "trace.h"
>> +#include "qemu/timer.h"
>> +#include "hw/ppc/xics.h"
>> +#include "qapi/visitor.h"
>> +#include "qapi/error.h"
>> +
>> +#include
>> +
>> +/* #define DEBUG_MM(fmt...) printf(fmt) */
>> +#define DEBUG_MM(fmt...)do { } while (0)
>> +
>> +static void xics_native_initfn(Object *obj)
>> +{
>> +XICSState *xics = XICS_NATIVE(obj);
>> +
>> +QLIST_INIT(&xics->ics);
>
> Isn't this redundant with the QLIST_INIT() in the common initfn?
>
>> +}
>> +
>> +static uint64_t icp_mm_read(void *opaque, hwaddr addr, unsigned width)
>> +{
>> +XICSState *s = opaque;
>> +int32_t cpu_id, server;
>> +uint32_t val;
>> +ICPState *ss;
>> +bool byte0 = (width == 1 && (addr & 0x3) == 0);
>> +
>> +cpu_id = (addr & (ICP_MM_SIZE - 1)) >> 12;
>> +server = get_cpu_index_by_dt_id(cpu_id);
>> +if (server < 0) {
>> +fprintf(stderr, "XICS: Bad ICP server %d\n", server);
>
> Please use error_report() instead of raw fprintf()s.
>
>> +goto bad_access;
>> +}
>> +ss = &s->ss[server];
>> +
>> +switch (addr & 0xffc) {
>> +case 0: /* poll */
>> +val = icp_ipoll(ss, NULL);
>> +if (byte0) {
>> +val >>= 24;
>> +} else if (width != 4) {
>> +goto bad_access;
>> +}
>> +break;
>> +case 4: /* xirr */
>> +if (byte0) {
>> +val = icp_ipoll(ss, NULL) >> 24;
>> +} else if (width == 4) {
>> +val = icp_accept(ss);
>> +} else {
>> +goto bad_access;
>> +}
>> +break;
>> +case 12:
>> +if (byte0) {
>> +val = ss->mfrr;
>> +
Re: [Qemu-devel] [PATCH v7 0/7] linux-user: Fix miscellaneous Mips-specific issues
On Wed, Sep 21, 2016 at 07:12:20PM +, Riku Voipio wrote: > On Wed, Sep 21, 2016 at 02:16:54PM +0100, Leon Alrae wrote: > > On Mon, Sep 19, 2016 at 01:44:37PM +0200, Aleksandar Markovic wrote: > > > From: Aleksandar Markovic > > > > > > v6->v7: > > > > > > - Rebased to the latest code. > > > - Patch 1/1 expanded to act on alpha and sh4. > > > - Naming in patch 4/7 synced with kernel naming. > > > - Change code style of patch 5/7. > > > - Corrected spelling in all patches. > > > > > > v5->v6: > > > > > > - Corrected two instances of wrong field type in the patch on > > > target_flock. > > > - Added a patch that corrects handling of EDQUOT error code for Mips. > > > - Added a patch that adds missing Mips-related items in strace.list. > > > > > > v4->v5: > > > > > > - Commit messages improved. > > > > > > v3->v4: > > > > > > - Added a patch on agrument rearangement. > > > > > > v2->v3: > > > > > > - Minor fixes in the commit messages. > > > > > > v1->v2: > > > > > > - Improved a comment in the patch about target_semid64_ds (now 4/4). > > > - Added a patch that fixes TARGET_SIOCATMARK for Mips. > > > - Changed order of patches to be more structured. > > > > > > This series fixes several wrong definitions of preprocessor constants and > > > structures in Qemu user mode, as well as two other Mips-related problems. > > > > > > Aleksandar Markovic (7): > > > linux-user: Fix TARGET_SIOCATMARK definition for Mips > > > linux-user: Fix TARGET_F_GETOWN definition for Mips > > > linux-user: Fix structure target_flock definition for Mips > > > linux-user: Fix structure target_semid64_ds definition for Mips > > > linux-user: Fix certain argument alignment cases for Mips64 > > > linux-user: Add missing TARGET_EDQUOT error code for Mips > > > linux-user: Add missing Mips syscalls items in strace.list > > > > > > linux-user/mips/target_structs.h | 16 ++ > > > linux-user/mips/target_syscall.h | 2 + > > > linux-user/mips64/target_syscall.h | 2 + > > > linux-user/strace.list | 114 > > > + > > > linux-user/syscall.c | 3 +- > > > linux-user/syscall_defs.h | 12 +++- > > > 6 files changed, 147 insertions(+), 2 deletions(-) > > > > Applied to target-mips queue, thanks. > > That's a bit unorthodox way but you have my acked-by then.. Since these are mips-specific fixes I assumed it doesn't really matter whether it goes via your or my tree (I picked up a few patches like this in the past). Anyway, thanks for ack and next time I'll check with you. Leon
Re: [Qemu-devel] [PATCH] linux-user: fix mremap for 64bit targets on 32bit hosts
Hi,
On Sat, Sep 17, 2016 at 09:20:14PM -0400, Felix Janda wrote:
> Signed-off-by: Felix Janda
Have you run the mremap tests of ltp with this on your host/guest
combo?
> ---
> linux-user/mmap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index c4371d9..4882816 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -682,7 +682,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> old_size,
>
> if (flags & MREMAP_FIXED) {
> host_addr = (void *) syscall(__NR_mremap, g2h(old_addr),
> - old_size, new_size,
> + (size_t) old_size, (size_t) new_size,
> flags,
> g2h(new_addr));
>
> @@ -701,7 +701,7 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> old_size,
> host_addr = MAP_FAILED;
> } else {
> host_addr = (void *) syscall(__NR_mremap, g2h(old_addr),
> - old_size, new_size,
> + (size_t) old_size, (size_t)
> new_size,
> flags | MREMAP_FIXED,
> g2h(mmap_start));
> if (reserved_va) {
> --
> 2.7.3
>
Re: [Qemu-devel] [PATCH v4 5/9] ppc/xics: Use a helper to add a new ICS
On 09/22/2016 01:40 AM, David Gibson wrote:
> On Mon, Sep 19, 2016 at 11:59:33AM +0530, Nikunj A Dadhania wrote:
>> From: Benjamin Herrenschmidt
>>
>> Signed-off-by: Benjamin Herrenschmidt
>> [Move object allocation and adding child to the helper]
>> Signed-off-by: Nikunj A Dadhania
>> Reviewed-by: David Gibson
>> ---
>> hw/intc/xics.c| 10 ++
>> hw/intc/xics_spapr.c | 6 +-
>> include/hw/ppc/xics.h | 1 +
>> 3 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index 05e938f..c7901c4 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -109,6 +109,16 @@ static void xics_common_reset(DeviceState *d)
>> }
>> }
>>
>> +void xics_add_ics(XICSState *xics)
>> +{
>> +ICSState *ics;
>> +
>> +ics = ICS(object_new(TYPE_ICS));
>> +object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
>
> You'll need to construct a name here so you don't have all the ics
> objects called an indistinguishable "ics".
Yes, exactly, and so PowerNV does not use it because at least three ics
are needed :
qemu) info qom-tree
/machine (powernv-machine)
/unattached (container)
/sysbus (System)
/ipmi-bt[0] (qemu:memory-region)
/device[0] (pnv-phb3)
/ics-phb-lsi (ics)
/ics-phb-msi (phb3-msi)
...
/psi (pnv-psi)
/xscom-psi[0] (qemu:memory-region)
/psihb[0] (qemu:memory-region)
/ics-psi (ics)
I think we can drop that patch.
However some routine like this one :
+void xics_insert_ics(XICSState *xics, ICSState *ics)
+{
+ics->xics = xics;
+QLIST_INSERT_HEAD(&xics->ics, ics, list);
+}
+
would be useful to hide the list details below xics :
/* link in the PSI ICS */
xics_insert_ics(XICS_COMMON(&chip->xics), &chip->psi.ics);
/* insert the ICS in XICS */
xics_insert_ics(xics, phb->lsi_ics);
xics_insert_ics(xics, ICS_BASE(phb->msis));
Cheers,
C.
>> +ics->xics = xics;
>> +QLIST_INSERT_HEAD(&xics->ics, ics, list);
>> +}
>> +
>> static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
>>void *opaque, Error **errp)
>> {
>> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
>> index 0b0845d..270f20e 100644
>> --- a/hw/intc/xics_spapr.c
>> +++ b/hw/intc/xics_spapr.c
>> @@ -305,12 +305,8 @@ static void xics_spapr_realize(DeviceState *dev, Error
>> **errp)
>> static void xics_spapr_initfn(Object *obj)
>> {
>> XICSState *xics = XICS_SPAPR(obj);
>> -ICSState *ics;
>>
>> -ics = ICS(object_new(TYPE_ICS));
>> -object_property_add_child(obj, "ics", OBJECT(ics), NULL);
>> -ics->xics = xics;
>> -QLIST_INSERT_HEAD(&xics->ics, ics, list);
>> +xics_add_ics(xics);
>> }
>>
>> static void xics_spapr_class_init(ObjectClass *oc, void *data)
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index e49a2da..a7a1e54 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -197,5 +197,6 @@ void ics_write_xive(ICSState *ics, int nr, int server,
>> void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>>
>> ICSState *xics_find_source(XICSState *icp, int irq);
>> +void xics_add_ics(XICSState *xics);
>>
>> #endif /* XICS_H */
>
Re: [Qemu-devel] pseries-2.6 migration from QEMU-2.6 to QEMU-2.7 broken
On Thu, Sep 22, 2016 at 04:07:21PM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2016-09-22 at 10:51 +0530, Bharata B Rao wrote: > > The flag values are expected to remain same for a machine version for > > the migration to succeed, but this expectation is broken now. Should > > we make the addition of these flags conditional on machine type > > version ? > > But these flags are part of POWER8 CPU definition which is common for > > both pseries and upcoming powernv. > > Does this affect KVM ? (And if yes why on earth would KVM give a flying > f*** about the TCG instruction flags ?) ... If not, then I think we can > safely not care. Yes, KVM migration is broken. Regards, Bharata.
[Qemu-devel] [PATCH] pci-testdev: enhance to support new testcases
pci-testdev is used mostly in kvm-unit-test for some eventfd tests.
However I see it a good framework for other tests as well (e.g., the
IOMMU unit test in the future). So enhanced it to support more
testcases.
The original memory handlers and protocol are strict and not easy to
change (we need to keep the old behavior of pci-testdev). So I added a
new parameter for the device, and memory ops will be dynamically handled
depending on what testcase it is configured. To specify a new test case
for pci-testdev, we use:
-device pci-testdev,testcase=XXX
The default will be "eventfd", which is the original behavior for
pci-testdev. In the future, we can just add new testcase for pci-testdev
to achieve different goals.
Signed-off-by: Peter Xu
---
This is kind-of a RFC since I am not sure whether this is a good way.
I did run the default kvm-unit-test cases on x86 to make sure it
didn't break anything. In case this is not good, I didn't write any
further test cases (e.g., emulate device DMAR/IR operations) yet. If
we like the idea, I can move on.
Please kindly review. Thanks.
hw/misc/pci-testdev.c | 80 ---
1 file changed, 76 insertions(+), 4 deletions(-)
diff --git a/hw/misc/pci-testdev.c b/hw/misc/pci-testdev.c
index 7d59902..b25d673 100644
--- a/hw/misc/pci-testdev.c
+++ b/hw/misc/pci-testdev.c
@@ -22,6 +22,19 @@
#include "hw/pci/pci.h"
#include "qemu/event_notifier.h"
#include "sysemu/kvm.h"
+#include "qemu/error-report.h"
+
+/* Type: 0 for MMIO write, 1 for PIO write. */
+typedef void (*pci_testdev_write_op)(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size, int type);
+typedef uint64_t (*pci_testdev_read_op)(void *opaque, hwaddr addr,
+unsigned size);
+
+struct testcase {
+const char *name;
+pci_testdev_write_op write_op;
+pci_testdev_read_op read_op;
+};
typedef struct PCITestDevHdr {
uint8_t test;
@@ -85,6 +98,8 @@ typedef struct PCITestDevState {
MemoryRegion portio;
IOTest *tests;
int current;
+char *testcase_name;
+struct testcase *testcase;
} PCITestDevState;
#define TYPE_PCI_TEST_DEV "pci-testdev"
@@ -200,22 +215,58 @@ pci_testdev_read(void *opaque, hwaddr addr, unsigned size)
return buf[addr];
}
+/*
+ * To add a new test, we need to implement both write_op and read_op,
+ * and add a new "struct testcase" into the global pci_testcases[].
+ */
+struct testcase pci_testcases[] = {
+{"eventfd", pci_testdev_write, pci_testdev_read},
+{NULL, NULL, NULL},
+};
+
+#define FOREACH_TEST_CASE(n) for (n = &pci_testcases[0]; n->name; n++)
+
+static struct testcase *
+pci_testdev_find_testcase(char *name)
+{
+struct testcase *test;
+
+FOREACH_TEST_CASE(test) {
+if (!strcmp(test->name, name)) {
+return test;
+}
+}
+return NULL;
+}
+
+static uint64_t
+pci_testdev_common_read(void *opaque, hwaddr addr, unsigned size)
+{
+PCITestDevState *d = opaque;
+pci_testdev_read_op read_op = d->testcase->read_op;
+return read_op(opaque, addr, size);
+}
+
static void
pci_testdev_mmio_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
{
-pci_testdev_write(opaque, addr, val, size, 0);
+PCITestDevState *d = opaque;
+pci_testdev_write_op write_op = d->testcase->write_op;
+write_op(opaque, addr, val, size, 0);
}
static void
pci_testdev_pio_write(void *opaque, hwaddr addr, uint64_t val,
unsigned size)
{
-pci_testdev_write(opaque, addr, val, size, 1);
+PCITestDevState *d = opaque;
+pci_testdev_write_op write_op = d->testcase->write_op;
+write_op(opaque, addr, val, size, 1);
}
static const MemoryRegionOps pci_testdev_mmio_ops = {
-.read = pci_testdev_read,
+.read = pci_testdev_common_read,
.write = pci_testdev_mmio_write,
.endianness = DEVICE_LITTLE_ENDIAN,
.impl = {
@@ -225,7 +276,7 @@ static const MemoryRegionOps pci_testdev_mmio_ops = {
};
static const MemoryRegionOps pci_testdev_pio_ops = {
-.read = pci_testdev_read,
+.read = pci_testdev_common_read,
.write = pci_testdev_pio_write,
.endianness = DEVICE_LITTLE_ENDIAN,
.impl = {
@@ -281,6 +332,21 @@ static void pci_testdev_realize(PCIDevice *pci_dev, Error
**errp)
assert(r >= 0);
test->hasnotifier = true;
}
+
+if (!d->testcase_name) {
+d->testcase_name = (char *)"eventfd";
+}
+
+d->testcase = pci_testdev_find_testcase(d->testcase_name);
+if (!d->testcase) {
+struct testcase *test;
+error_report("Invalid test case. Currently support: {");
+FOREACH_TEST_CASE(test) {
+error_report("\t\"%s\", ", test->name);
+}
+error_report("}");
+exit(1);
+}
}
static void
@@ -305,6 +371,11 @@ static void qdev_pci_testdev_reset(DeviceState *dev)
pci_testdev_reset(d);
}
Re: [Qemu-devel] pseries-2.6 migration from QEMU-2.6 to QEMU-2.7 broken
On Thu, 2016-09-22 at 10:51 +0530, Bharata B Rao wrote: > The flag values are expected to remain same for a machine version for > the migration to succeed, but this expectation is broken now. Should > we make the addition of these flags conditional on machine type > version ? > But these flags are part of POWER8 CPU definition which is common for > both pseries and upcoming powernv. Does this affect KVM ? (And if yes why on earth would KVM give a flying f*** about the TCG instruction flags ?) ... If not, then I think we can safely not care. This migration business is making life really really really hard ... Ben.
Re: [Qemu-devel] pseries-2.6 migration from QEMU-2.6 to QEMU-2.7 broken
On Thu, Sep 22, 2016 at 03:30:08PM +1000, David Gibson wrote:
> On Thu, Sep 22, 2016 at 10:51:05AM +0530, Bharata B Rao wrote:
> > Hi,
> >
> > Nikunj and I realized that migrating pseries-2.6 guest from QEMU-2.6
> > to newer QEMU-2.7 is broken like this:
> >
> > qemu-system-ppc64: error while loading state for instance 0x0 of device
> > 'cpu'
> > qemu-system-ppc64: load of migration failed: Invalid argument
> >
> > Bisecting tells us that 4e0806110c8b896ceff3490f15a616e8b3165efe
> > (ppc: Add PPC_64H instruction flag to POWER7 and POWER8) is the
> > first bad commit. Along with this there are other 3 similar commits
> > which add new bits to insns_flags and insns_flags2 fields of POWER7
> > and POWER8 CPUs.
> >
> > 4e0806110c8b896ceff3490f15a616e8b3165efe Adds PPC_64H to POWER7 and POWER8
> > dfdd3e43620a6cd4f2be31da5a257c84a16fc000 Adds PPC_64BX to POWER7
> > b781537560e3b968b6fe1395e3d07bd67f0009ba Adds PPC_CILDST to POWER7 and
> > POWER8
> > 7778a575c7055276afdd01737e9d1029a65f923d Adds PPC2_PM_ISA206 to POWER7 and
> > POWER8
> >
> > The flag values are expected to remain same for a machine version for
> > the migration to succeed, but this expectation is broken now. Should
> > we make the addition of these flags conditional on machine type version ?
> > But these flags are part of POWER8 CPU definition which is common for
> > both pseries and upcoming powernv.
>
> Can you step me through how the new flags are breaking the migration?
> It's not immediately obvious to me.
Here is what I understand. Given below is the pruned vmstate_ppc_cpu
data structure.
const VMStateDescription vmstate_ppc_cpu = {
.name = "cpu",
.fields = (VMStateField[]) {
/* Sanity checking */
VMSTATE_UINT64_EQUAL(env.insns_flags, PowerPCCPU),
VMSTATE_UINT64_EQUAL(env.insns_flags2, PowerPCCPU),
VMSTATE_END_OF_LIST()
},
};
When pseries-2.6 guest is started at the source with QEMU-2.6, insns_flags
and insns_flags2 will not have PPC_64H, PPC_64BX, PPC_CILDST, PPC2_PM_ISA206
set. However at the target when pseries-2.6 guest is started with QEMU-2.7,
these flags will be set. And I believe VMSTATE_UINT64_EQUAL checks above
will cause migration to fail.
Regards,
Bharata.
Re: [Qemu-devel] pseries-2.6 migration from QEMU-2.6 to QEMU-2.7 broken
On Thu, Sep 22, 2016 at 10:51:05AM +0530, Bharata B Rao wrote: > Hi, > > Nikunj and I realized that migrating pseries-2.6 guest from QEMU-2.6 > to newer QEMU-2.7 is broken like this: > > qemu-system-ppc64: error while loading state for instance 0x0 of device 'cpu' > qemu-system-ppc64: load of migration failed: Invalid argument > > Bisecting tells us that 4e0806110c8b896ceff3490f15a616e8b3165efe > (ppc: Add PPC_64H instruction flag to POWER7 and POWER8) is the > first bad commit. Along with this there are other 3 similar commits > which add new bits to insns_flags and insns_flags2 fields of POWER7 > and POWER8 CPUs. > > 4e0806110c8b896ceff3490f15a616e8b3165efe Adds PPC_64H to POWER7 and POWER8 > dfdd3e43620a6cd4f2be31da5a257c84a16fc000 Adds PPC_64BX to POWER7 > b781537560e3b968b6fe1395e3d07bd67f0009ba Adds PPC_CILDST to POWER7 and POWER8 > 7778a575c7055276afdd01737e9d1029a65f923d Adds PPC2_PM_ISA206 to POWER7 and > POWER8 > > The flag values are expected to remain same for a machine version for > the migration to succeed, but this expectation is broken now. Should > we make the addition of these flags conditional on machine type version ? > But these flags are part of POWER8 CPU definition which is common for > both pseries and upcoming powernv. Can you step me through how the new flags are breaking the migration? It's not immediately obvious to me. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v6 3/3] intel_iommu: allow UNMAP notifiers
On Thu, Sep 22, 2016 at 03:24:43PM +1000, David Gibson wrote: > On Wed, Sep 21, 2016 at 12:58:56PM +0800, Peter Xu wrote: > > Intel vIOMMU is still lacking of a complete IOMMU notifier mechanism. > > Before that is achieved, let's open a door for vhost DMAR support, which > > only requires cache invalidations (UNMAP operations). > > > > Meanwhile, converting hw_error() to error_report() and exit(1), to make > > the error messages clean and obvious (so no CPU registers will be > > dumped). > > > > Reviewed-by: David Gibson > > Uh.. I didn't send an R-b for this. In fact I explicitly said I > didn't think it should be applied until notifications have actually > been implemented by the intel viommu. I still think that, and think > this should just be dropped. Please refer to: https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg03034.html So this line is there since v5. I took it an honor (and also with my thankfulness) to have added your r-b line here. I assume what you meant before was: the patch content is okay, but you would suggest to drop this patch in this series, and merge this until we got a real implementations for the notifiers. IMHO that does not mean "remove your r-b in this patch". If you meant to remove this line (I think not?), please confirm and I can remove it. I posted patch 3 just to make sure everything is coherent, and let Paolo decide which way to choose (since I still think it's okay actually... but again both are ok to me). Also it'll be easier for Jason to track this down as well (so when Jason sees that Paolo dropped patch 3, he'll naturally pick it up). If you still insist on dropping this patch, I'll do it in v7. Thanks. -- peterx
Re: [Qemu-devel] [PATCH v3 00/10] virtio-crypto: introduce framework and device emulation
Hi Michael, Stefan and Paolo, Would you please help to review virtio stuff in the patch set? Thanks! Regards, -Gonglei > -Original Message- > From: Gonglei (Arei) > Sent: Monday, September 19, 2016 4:16 PM > Subject: [PATCH v3 00/10] virtio-crypto: introduce framework and device > emulation > > Changes since v2: > According to Daniel's comments: > - drop cryptodev kernel module as a cryptodev backend > - rename crypto stuff to cryptodev stuff > - change some files' license to GPLv2+ > - remove cryptodev command line instead of QOM to define the cryptodev > backend > - rename all functions and structures in crypto sub-directory. > - add full inline documentation for cryptodev.h > And: > - drop crypto-queue.c [Paolo] > - merge some patches > > Great thanks to Daniel and Paolo. Please review again, thanks! > > Changes since v1: > - rmmove mixed endian-ness handler for virtio-crypto device, just >use little-endian. [mst] > - add sg list support according virtio-crypto spec v10 (will be posted soon). > - fix a memory leak in session handler. > - add a feature page link in qemu.org > (http://qemu-project.org/Features/VirtioCrypto) > - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in > qapi-schema.json > - rebase the latest qemu master tree. > > > This patch series realize the framework and emulation of a new > virtio crypto device, which is similar with virtio net device. > > - I introduce the cryptodev backend as the client of virtio crypto device >which can be realized by different methods, such as > cryptodev-backend-gcrypt in my series, >vhost-crypto kernel module, vhost-user etc. > - The patch set abides by the virtio crypto speccification. > - The virtio crypto support symmetric algorithms (including CIPHER and > algorithm chainning) >at present, except HASH, MAC and AEAD services. > - unsupport hot plug/unplug cryptodev backend at this moment. > > Firstly build QEMU with libgcrypt cryptography support. > > QEMU can then be started using the following parameters: > > qemu-system-x86_64 \ > [...] \ > -object cryptodev-backend-gcrypt,id=cryptodev0 \ > -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \ > [...] > > The front-end linux kernel driver (Experimental at present) is publicly > accessible > from: > >https://github.com/gongleiarei/virtio-crypto-linux-driver.git > > After insmod virtio-crypto.ko, you can use cryptodev-linux test the crypto > function > in the guest. For example: > > linux-guest:/home/gonglei/cryptodev-linux/tests # ./cipher - > requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver > virtio_crypto_aes_cbc > AES Test passed > requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver > virtio_crypto_aes_cbc > requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver > virtio_crypto_aes_cbc > Test passed > > QEMU code also can be accessible from: > > https://github.com/gongleiarei/qemu.git > > branch virtio-crypto > > For more information, please see: > http://qemu-project.org/Features/VirtioCrypto > > > Gonglei (10): > cryptodev: introduce cryptodev backend interface > cryptodev: add symmetric algorithm operation stuff > virtio-crypto: introduce virtio_crypto.h > cryptodev: introduce gcrypt lib as a new cryptodev backend > virtio-crypto: add virtio crypto device emulation > virtio-crypto-pci: add virtio crypto pci support > virtio-crypto: set capacity of algorithms supported > virtio-crypto: add control queue handler > virtio-crypto: add data queue processing handler > cryptodev: introduce an unified wrapper for crypto operation > > crypto/Makefile.objs | 2 + > crypto/cryptodev-gcrypt.c | 329 + > crypto/cryptodev.c | 243 +++ > hw/virtio/Makefile.objs| 2 + > hw/virtio/virtio-crypto-pci.c | 76 +++ > hw/virtio/virtio-crypto.c | 904 > + > hw/virtio/virtio-pci.h | 15 + > include/crypto/cryptodev.h | 276 > include/hw/virtio/virtio-crypto.h | 100 +++ > include/standard-headers/linux/virtio_crypto.h | 466 + > qemu-options.hx| 18 + > 11 files changed, 2431 insertions(+) > create mode 100644 crypto/cryptodev-gcrypt.c > create mode 100644 crypto/cryptodev.c > create mode 100644 hw/virtio/virtio-crypto-pci.c > create mode 100644 hw/virtio/virtio-crypto.c > create mode 100644 include/crypto/cryptodev.h > create mode 100644 include/hw/virtio/virtio-crypto.h > create mode 100644 include/standard-headers/linux/virtio_crypto.h > > -- > 1.7.12.4 >
Re: [Qemu-devel] Missing fix for ui/gtk.c?
On Mi, 2016-09-21 at 11:33 -0500, Wei Huang wrote: > Hi, > > I found that you had a fix for setlocale() in ui/gtk.c. Without it, > non-ASCII can't display correctly on Fedora 24. But I didn't see it in > QEMU tree. Did it slip through the cracks? Problem is setting the locale has side effects elsewhere in qemu ... cheers, Gerd
Re: [Qemu-devel] [PATCH v2] ide: Fix memory leak in ide_register_restart_cb()
On Thu, Sep 22, 2016 at 1:08 AM, John Snow wrote:
>
>
> On 09/21/2016 01:53 PM, Ashijeet Acharya wrote:
>>
>> Fix a memory leak in ide_register_restart_cb() in hw/ide/core.c and add
>> idebus_unrealize() in hw/ide/qdev.c to have calls to
>> qemu_del_vm_change_state_handler() to deal with the dangling change
>> state handler during hot-unplugging ide devices which might lead to a
>> crash.
>>
>
> Minor rebase issue, but it's trivially resolved.
>
>
>> Signed-off-by: Ashijeet Acharya
>> ---
>> Changes in v2:
>> -v1 was corrupted at line 64
>> -Move idebus_unrealize() below ide_bus_class_init()
>> ---
>> hw/ide/core.c | 2 +-
>> hw/ide/qdev.c | 13 +
>> include/hw/ide/internal.h | 1 +
>> 3 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 45b6df1..eecbb47 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -2582,7 +2582,7 @@ static void ide_restart_cb(void *opaque, int
>> running, RunState state)
>> void ide_register_restart_cb(IDEBus *bus)
>> {
>> if (bus->dma->ops->restart_dma) {
>> -qemu_add_vm_change_state_handler(ide_restart_cb, bus);
>> +bus->vmstate = qemu_add_vm_change_state_handler(ide_restart_cb,
>> bus);
>> }
>> }
>>
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index 2eb055a..c94f9f8 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -31,6 +31,7 @@
>> /* - */
>>
>> static char *idebus_get_fw_dev_path(DeviceState *dev);
>> +static void idebus_unrealize(DeviceState *qdev, Error **errp);
>>
>> static Property ide_props[] = {
>> DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
>> @@ -44,6 +45,17 @@ static void ide_bus_class_init(ObjectClass *klass, void
>> *data)
>> k->get_fw_dev_path = idebus_get_fw_dev_path;
>> }
>>
>> +static void idebus_unrealize(DeviceState *qdev, Error **errp)
>> +{
>> +IDEBus *bus = DO_UPCAST(IDEBus, qbus, qdev->parent_bus);
>> +
>> +if (bus->dma->ops->restart_dma) {
>> +if (bus->vmstate) {
>> +qemu_del_vm_change_state_handler(bus->vmstate);
>> +}
>> +}
>> +}
>> +
>
>
> Naive question: I saw Paolo say that this should be conditional on
> bus->dma->ops->restart_dma -- Why can't we just say if (bus->vmstate) ?
>
> I see that we only allocate the change state handler when restart_dma is
> present, but this makes this portion of the code look funny when if
> (bus->vmstate) would be just as simple, no?
>
I had a similar thought, because other pieces of code also do it in
"if (bus->vmstate)" manner but maybe I was missing on something
important and ended up coding how Paolo instructed me.
> (Unless we can't rely on its NULL initialization or some such.)
Maybe, but shouldn't the same logic apply elsewhere then?
>
>> static const TypeInfo ide_bus_info = {
>> .name = TYPE_IDE_BUS,
>> .parent = TYPE_BUS,
>> @@ -355,6 +367,7 @@ static void ide_device_class_init(ObjectClass *klass,
>> void *data)
>> k->init = ide_qdev_init;
>> set_bit(DEVICE_CATEGORY_STORAGE, k->categories);
>> k->bus_type = TYPE_IDE_BUS;
>> +k->unrealize = idebus_unrealize;
>> k->props = ide_props;
>> }
>>
>> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
>> index 7824bc3..2103261 100644
>> --- a/include/hw/ide/internal.h
>> +++ b/include/hw/ide/internal.h
>> @@ -480,6 +480,7 @@ struct IDEBus {
>> uint8_t retry_unit;
>> int64_t retry_sector_num;
>> uint32_t retry_nsector;
>> +VMChangeStateEntry *vmstate;
>
>
> Hmm, I was going to complain and say that 'vmstate' is a generic name for
> this field, but it's by far the most common name for this task. I cede!
>
>> };
>>
>> #define TYPE_IDE_DEVICE "ide-device"
>>
>
> Seems good otherwise, thank you!
Thanks!
Ashijeet
Re: [Qemu-devel] [PATCH v4 3/8] block: don't make snapshots for filters
> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Wednesday, September 21, 2016 6:37 PM
> To: Pavel Dovgalyuk; qemu-devel@nongnu.org
> Cc: peter.mayd...@linaro.org; jasow...@redhat.com; quint...@redhat.com;
> m...@redhat.com
> Subject: Re: [PATCH v4 3/8] block: don't make snapshots for filters
>
>
>
> On 21/09/2016 13:33, Pavel Dovgalyuk wrote:
> > This patch disables snapshotting for block driver filters.
> > It is needed, because snapshots should be created
> > in underlying disk images, not in filters itself.
>
> I don't understand this patch. Who would take care of doing the
> bdrv_snapshot_goto on the underlying image?
loadvm needs to make bdrv_snapshot_goto. It discovers top-level replay driver
and
tries to perform bdrv_snapshot_goto for it.
Patches 3 and 4 fix that. They allow passing through the replay driver and
make saving/restoring of the actual image.
Pavel Dovgalyuk
>
> > Signed-off-by: Pavel Dovgalyuk
> > ---
> > block/snapshot.c |3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/block/snapshot.c b/block/snapshot.c
> > index bf5c2ca..8998b8b 100644
> > --- a/block/snapshot.c
> > +++ b/block/snapshot.c
> > @@ -184,6 +184,9 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
> > if (!drv) {
> > return -ENOMEDIUM;
> > }
> > +if (drv->is_filter) {
> > +return 0;
> > +}
> > if (drv->bdrv_snapshot_goto) {
> > return drv->bdrv_snapshot_goto(bs, snapshot_id);
> > }
> >
Re: [Qemu-devel] [PATCH v4 0/8] replay additions
> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > On 21/09/2016 13:33, Pavel Dovgalyuk wrote: > > This set of patches includes several fixes for replay and > > adds network record/replay for network devices. It also makes possible > > saving/restoring vmstate in replay mode. > > > > Record and replay for network interactions is performed with the network > > filter. > > Each backend must have its own instance of the replay filter as follows: > > -netdev user,id=net1 -device rtl8139,netdev=net1 > > -object filter-replay,id=replay,netdev=net1 > > > > This patches add rrsnapshot option for icount. rrshapshot option creates > > start snapshot at record and loads it at replay. It allows preserving > > the state of disk images used by virtual machine. This vm state can also > > use used to roll back the execution while replaying. > > > > This set of patches includes fixes and additions for icount and > > record/replay implementation: > > - VM start/stop in replay mode > > - Network interaction record/replay > > - overlay option for blkreplay filter > > - rrsnapshot option for record/replay > > - vmstate fix for integratorcp ARM platform > > > > v4 changes: > > - Overlay option is removed from blkreplay driver (as suggested by Paolo > > Bonzini) > > - Minor changes > > > > v3 changes: > > - Added rrsnapshot option for specifying the initial snapshot name (as > > suggested by Paolo > Bonzini) > > - Minor changes > > When you post v5, I suspect it's best if you move patches 5-7 first, > because those are the ones I'm going to merge myself. And what about network support (patch 1)? Nobody has commented it in any of the iterations. Pavel Dovgalyuk
Re: [Qemu-devel] [PATCH 0/7] virtio: avoid inappropriate QEMU termination
On Wed, 09/21 09:29, Eric Blake wrote: > On 09/21/2016 09:01 AM, Fam Zheng wrote: > > On Wed, 09/21 15:44, Greg Kurz wrote: > >> On Wed, 21 Sep 2016 06:35:04 -0700 (PDT) > >> no-re...@patchew.org wrote: > >> > >>> Hi, > >>> > >>> Your series failed automatic build test. Please find the testing commands > >>> and > >>> their output below. If you have docker installed, you can probably > >>> reproduce it > >>> locally. > >>> > >> > >> Heh... of course patchew doesn't know about Stefan's series. :) > > > > Usually those patchsets fail to apply and patchew keeps quiet. This series > > is > > the first victim of a exception, seemingly. :) > > While we're at it, is there a way to make the error messages include the > last 15 lines or so of the build log first, and then the full build log? > Usually, the failure is easy to identify from the tail of the message, > but having to scroll to the bottom past all the successful portion of > the log is slower than seeing the tail up front. At the same time, the > full log is useful for the cases where the failure occurs earlier than > the last 15 lines. Scrolling to bottom is quite easy in any email client, while more sections in a long email demand extra effort to manage and read. I doubt adding a "tail of log" section just before the full log helps us much. I'd rather keep it simple and stupid. > > > > >> > >> Is there an appropriate way to avoid complaints when sending a patchset > >> that > >> isn't based on QEMU master ? > > > > Currently we don't, but we can invent one. Like: > > > > Base: http://github.com/someone/qemu branch > > I like that idea (these patches apply from that point; then it is up to > patchew whether to bother chasing down that point or just skipping the > series) > > > > > for the "base on maintainer tree" case, or just inform patchew to "pull" > > instead of apply: > > > > Git: http://github.com/someone/qemu topic > > Maybe Pull: instead of Git:, but again a nice idea. I'm adding them to my list. > > > > > > for the "base on the other series" case. > > > > Supporting this is not complicated, the tricky thing is to remember to put > > that > > magic in cover letters every time a series doesn't base on master. > > > > Any thoughts on the harder question? :) > > Which one was the harder question? How to get people to remember to use > magic that patchew can recognize? Yes. :) > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
Re: [Qemu-devel] [PATCH v4 2/8] replay: save/load initial state
> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > On 21/09/2016 17:49, Pavel Dovgalyuk wrote: > >> From: Paolo Bonzini [mailto:pbonz...@redhat.com] > >> On 21/09/2016 13:33, Pavel Dovgalyuk wrote: > >>> +New VM snapshots may be created in replay mode. They can be used later > >>> +to recover the desired VM state. All VM states created in replay mode > >>> +are associated with the moment of time in the replay scenario. > >>> +After recovering the VM state replay will start from that position. > >>> + > >>> +Default starting snapshot name may be specified with icount field > >>> +rrsnapshot as follows: > >>> + -icount shift=7,rr=record,rrfile=replay.bin,rrsnapshot=snapshot_name > >>> + > >>> +This snapshot is created at start of recording and restored at start > >>> +of replaying. It also can be loaded while replaying to roll back > >>> +the execution. > >> > >> Should you mention somewhere that you need -snapshot for raw images? > > > > Do you mean when rrsnapshot is used with raw images? > > I mean that (if I understand correctly) raw images are corrupted if you > don't use -snapshot (the record overwrites them, and the replay reads > overwritten data). I tried running record/replay with raw image. Record finished without an error, but replay couldn't start, because there was no snapshot to load. I think the solution is checking savevm return code and exiting if snapshot cannot be created. Pavel Dovgalyuk
Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU
On Thu, 22 Sep 2016 11:57:15 +1000 David Gibson wrote: > On Wed, Sep 21, 2016 at 10:22:11AM +0200, Thomas Huth wrote: > > On 20.09.2016 16:39, Cédric Le Goater wrote: > > > On 09/20/2016 04:24 PM, Thomas Huth wrote: > > >> On 20.09.2016 16:04, Cédric Le Goater wrote: > > [...] > > >>> There are other issues after in the guest (kernel crashing). But I think > > >>> these are related to TM which is not supported in KVM-PR. I am not sure > > >>> where we are on that point. > > >> > > >> There was a patch some months ago: > > >> > > >> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html > > >> > > >> ... but I think it has never been included, as far as I can see. > > > > > > and with that patch, the guest fully boots. But David had some concerns > > > on the way it is done. It would be nice to put some cycle on this. > > > > Looking at the mail thread, I think TM should be currently disabled for > > both, KVM-PR and TCG, i.e. only enabled for KVM-HV. The TM support in > > TCG is just fake, since TBEGIN always fails. > > Right. So there's two questions here > > 1) Is qemu correctly advertising availability of TM in the device > tree? If I've got that right, it's currently always advertising TM, even if it's not really available (in TCG mode and PR mode). > If not we need to fix that, which might involve adding a kernel > capability for the PR case. > > 2) Is the kvm unit test properly checking for availability of TM > before executing? Not yet. That's why it would be good to get a proper way for testing for the availability of TM --> i.e. something like Anton's patch. > > Once we've got proper TM support in TCG, this can be easily changed > > within QEMU. And once we've got TM support in KVM-PR, I think we should > > also introduce a capability flag to KVM which can be used to inform QEMU > > about this. > > > > So I think Anton's patch currently just lacks the check for TCG. > > Anton, if you've got some spare minutes, could you maybe send an updated > > version of that patch? > > Sorry, which patch of Anton's? This one: https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00415.html Thomas pgpg23pEuegyx.pgp Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v6 1/3] memory: introduce IOMMUNotifier and its caps
On Wed, Sep 21, 2016 at 12:58:54PM +0800, Peter Xu wrote:
> IOMMU Notifier list is used for notifying IO address mapping changes.
> Currently VFIO is the only user.
>
> However it is possible that future consumer like vhost would like to
> only listen to part of its notifications (e.g., cache invalidations).
>
> This patch introduced IOMMUNotifier and IOMMUNotfierFlag bits for a
> finer grained control of it.
>
> IOMMUNotifier contains a bitfield for the notify consumer describing
> what kind of notification it is interested in. Currently two kinds of
> notifications are defined:
>
> - IOMMU_NOTIFIER_MAP:for newly mapped entries (additions)
> - IOMMU_NOTIFIER_UNMAP: for entries to be removed (cache invalidates)
>
> When registering the IOMMU notifier, we need to specify one or multiple
> types of messages to listen to.
>
> When notifications are triggered, its type will be checked against the
> notifier's type bits, and only notifiers with registered bits will be
> notified.
>
> (For any IOMMU implementation, an in-place mapping change should be
> notified with an UNMAP followed by a MAP.)
Ok, I wasn't clear. I meant a big fat comment in the *code*, not just
in the commit message. It should not be necessary to look at the
commit history to figure out how to use an interface correctly
Even a comment in the code is barely adequate, compared to designing
the interface signatures such that it's obvious.
Please bear in mind:
http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html
and http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html
Couple of other tiny nits that I wouldn't bother with it weren't for
the above.
>
> Signed-off-by: Peter Xu
> ---
> hw/vfio/common.c | 3 ++-
> include/exec/memory.h | 38 +++---
> include/hw/vfio/vfio-common.h | 2 +-
> memory.c | 37 -
> 4 files changed, 62 insertions(+), 18 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index b313e7c..89a993b 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -293,7 +293,7 @@ static bool
> vfio_listener_skipped_section(MemoryRegionSection *section)
> section->offset_within_address_space & (1ULL << 63);
> }
>
> -static void vfio_iommu_map_notify(Notifier *n, void *data)
> +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *data)
This change leaves a now pointless IOMMUTLBEntry *iotlb = data a few
lines below.
> {
> VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
> VFIOContainer *container = giommu->container;
> @@ -454,6 +454,7 @@ static void vfio_listener_region_add(MemoryListener
> *listener,
> section->offset_within_region;
> giommu->container = container;
> giommu->n.notify = vfio_iommu_map_notify;
> +giommu->n.notifier_flags = IOMMU_NOTIFIER_ALL;
> QLIST_INSERT_HEAD(&container->giommu_list, giommu, giommu_next);
>
> memory_region_register_iommu_notifier(giommu->iommu, &giommu->n);
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 3e4d416..a3ec7aa 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -67,6 +67,27 @@ struct IOMMUTLBEntry {
> IOMMUAccessFlags perm;
> };
>
> +/*
> + * Bitmap for differnet IOMMUNotifier capabilities. Each notifier can
s/differnet/different/
> + * register with one or multiple IOMMU Notifier capability bit(s).
> + */
> +typedef enum {
> +IOMMU_NOTIFIER_NONE = 0,
> +/* Notify cache invalidations */
> +IOMMU_NOTIFIER_UNMAP = 0x1,
> +/* Notify entry changes (newly created entries) */
> +IOMMU_NOTIFIER_MAP = 0x2,
> +} IOMMUNotifierFlag;
> +
> +#define IOMMU_NOTIFIER_ALL (IOMMU_NOTIFIER_MAP | IOMMU_NOTIFIER_UNMAP)
> +
> +struct IOMMUNotifier {
> +void (*notify)(struct IOMMUNotifier *notifier, IOMMUTLBEntry *data);
> +IOMMUNotifierFlag notifier_flags;
> +QLIST_ENTRY(IOMMUNotifier) node;
> +};
> +typedef struct IOMMUNotifier IOMMUNotifier;
> +
> /* New-style MMIO accessors can indicate that the transaction failed.
> * A zero (MEMTX_OK) response means success; anything else is a failure
> * of some kind. The memory subsystem will bitwise-OR together results
> @@ -201,7 +222,7 @@ struct MemoryRegion {
> const char *name;
> unsigned ioeventfd_nb;
> MemoryRegionIoeventfd *ioeventfds;
> -NotifierList iommu_notify;
> +QLIST_HEAD(, IOMMUNotifier) iommu_notify;
> };
>
> /**
> @@ -620,11 +641,12 @@ void memory_region_notify_iommu(MemoryRegion *mr,
> * IOMMU translation entries.
> *
> * @mr: the memory region to observe
> - * @n: the notifier to be added; the notifier receives a pointer to an
> - * #IOMMUTLBEntry as the opaque value; the pointer ceases to be
> - * valid on exit from the notifier.
> + * @n: the IOMMUNotifier to be added; the notify callback receives a
> + * pointer to an #IOMMUTL
Re: [Qemu-devel] [PATCH v6 3/3] intel_iommu: allow UNMAP notifiers
On Wed, Sep 21, 2016 at 12:58:56PM +0800, Peter Xu wrote:
> Intel vIOMMU is still lacking of a complete IOMMU notifier mechanism.
> Before that is achieved, let's open a door for vhost DMAR support, which
> only requires cache invalidations (UNMAP operations).
>
> Meanwhile, converting hw_error() to error_report() and exit(1), to make
> the error messages clean and obvious (so no CPU registers will be
> dumped).
>
> Reviewed-by: David Gibson
Uh.. I didn't send an R-b for this. In fact I explicitly said I
didn't think it should be applied until notifications have actually
been implemented by the intel viommu. I still think that, and think
this should just be dropped.
> Signed-off-by: Peter Xu
> ---
> hw/i386/intel_iommu.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 9d49be7..e4c3681 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -1980,10 +1980,14 @@ static void
> vtd_iommu_notify_flag_changed(MemoryRegion *iommu,
> {
> VTDAddressSpace *vtd_as = container_of(iommu, VTDAddressSpace, iommu);
>
> -hw_error("Device at bus %s addr %02x.%d requires iommu notifier which "
> - "is currently not supported by intel-iommu emulation",
> - vtd_as->bus->qbus.name, PCI_SLOT(vtd_as->devfn),
> - PCI_FUNC(vtd_as->devfn));
> +if (new & IOMMU_NOTIFIER_MAP) {
> +error_report("Device at bus %s addr %02x.%d requires iommu "
> + "notifier which is currently not supported by "
> + "intel-iommu emulation",
> + vtd_as->bus->qbus.name, PCI_SLOT(vtd_as->devfn),
> + PCI_FUNC(vtd_as->devfn));
> +exit(1);
> +}
> }
>
> static const VMStateDescription vtd_vmstate = {
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
[Qemu-devel] pseries-2.6 migration from QEMU-2.6 to QEMU-2.7 broken
Hi, Nikunj and I realized that migrating pseries-2.6 guest from QEMU-2.6 to newer QEMU-2.7 is broken like this: qemu-system-ppc64: error while loading state for instance 0x0 of device 'cpu' qemu-system-ppc64: load of migration failed: Invalid argument Bisecting tells us that 4e0806110c8b896ceff3490f15a616e8b3165efe (ppc: Add PPC_64H instruction flag to POWER7 and POWER8) is the first bad commit. Along with this there are other 3 similar commits which add new bits to insns_flags and insns_flags2 fields of POWER7 and POWER8 CPUs. 4e0806110c8b896ceff3490f15a616e8b3165efe Adds PPC_64H to POWER7 and POWER8 dfdd3e43620a6cd4f2be31da5a257c84a16fc000 Adds PPC_64BX to POWER7 b781537560e3b968b6fe1395e3d07bd67f0009ba Adds PPC_CILDST to POWER7 and POWER8 7778a575c7055276afdd01737e9d1029a65f923d Adds PPC2_PM_ISA206 to POWER7 and POWER8 The flag values are expected to remain same for a machine version for the migration to succeed, but this expectation is broken now. Should we make the addition of these flags conditional on machine type version ? But these flags are part of POWER8 CPU definition which is common for both pseries and upcoming powernv. Regards, Bharata.
Re: [Qemu-devel] [PATCH qemu] spapr_pci: Add numa node id
On Wed, Sep 14, 2016 at 07:03:50AM -0500, Michael Roth wrote:
> Quoting Alexey Kardashevskiy (2016-09-14 04:39:10)
> > On 14/09/16 09:29, Michael Roth wrote:
> > > Quoting Alexey Kardashevskiy (2016-07-27 03:03:38)
> > >> This adds a numa id property to a PHB to allow linking passed PCI device
> > >> to CPU/memory. It is up to the management stack to do CPU/memory pinning
> > >> to the node with the actual PCI device.
> > >
> > > It looks like x86 relies on PCIBus->numa_node() method (via
> > > pci_bus_numa_node()) to encode similar PCIBus affinities
> > > into ACPI tables, and currently exposes it via
> > > -device pci-[-express]-expander-bus,numa_node=X.
> >
> >
> >
> > Well, until we allow DMA windows per PCI bus (not per PHB as it is now),
> > this won't make much sense for us (unless I am missing something here).
>
> I didn't consider that it's not a bus-level setting; I think
> you're right that re-using the interface to both store/retrieve doesn't
> make much sense in that case.
>
> My thought that was that since pci_bus_numa_node() could in theory come
> to be relied upon by common PCI code, that we should use it as well. But
> even if it doesn't make sense for us to use it, wouldn't it make sense to
> still set PCIBus->numa_node (in addition to the PHB-wide value) in the
> off-chance that common code does come to rely on
> pci_bus_numa_node()?
Yes, it would be a good idea to set the PCIBus node value to inherit
the one that's set for the host bridge, just in case any generic code
looks at it in future.
>
> >
> >
> > > Maybe we should implement it using this existing
> > > PCIBus->numa_node() interface?
> > >
> > > We'd still have to expose numa_node as a spapr-pci-host-bridge
> > > device option though. Not sure if there's a more common way
> > > to expose it that might be easier for libvirt to discover. As it
> > > stands we'd need to add spapr-pci-host-bridge to a libvirt
> > > whitelist that currently only covers pci-expander-bus.
> > >
> > > Cc'ing Shiva who was looking into the libvirt side.
> > >
> > > One comment below:
> > >
> > >>
> > >> Signed-off-by: Alexey Kardashevskiy
> > >> ---
> > >> hw/ppc/spapr_pci.c | 13 +
> > >> include/hw/pci-host/spapr.h | 2 ++
> > >> 2 files changed, 15 insertions(+)
> > >>
> > >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > >> index 949c44f..af5394a 100644
> > >> --- a/hw/ppc/spapr_pci.c
> > >> +++ b/hw/ppc/spapr_pci.c
> > >> @@ -47,6 +47,7 @@
> > >> #include "sysemu/device_tree.h"
> > >> #include "sysemu/kvm.h"
> > >> #include "sysemu/hostmem.h"
> > >> +#include "sysemu/numa.h"
> > >>
> > >> #include "hw/vfio/vfio.h"
> > >>
> > >> @@ -1544,6 +1545,7 @@ static Property spapr_phb_properties[] = {
> > >> DEFINE_PROP_BOOL("ddw", sPAPRPHBState, ddw_enabled, true),
> > >> DEFINE_PROP_UINT64("pgsz", sPAPRPHBState, page_size_mask,
> > >> (1ULL << 12) | (1ULL << 16)),
> > >> +DEFINE_PROP_UINT32("node", sPAPRPHBState, numa_node, -1),
> > >> DEFINE_PROP_END_OF_LIST(),
> > >> };
> > >>
> > >> @@ -1805,6 +1807,11 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> > >> cpu_to_be32(1),
> > >> cpu_to_be32(RTAS_IBM_RESET_PE_DMA_WINDOW)
> > >> };
> > >> +uint32_t associativity[] = {cpu_to_be32(0x4),
> > >> +cpu_to_be32(0x0),
> > >> +cpu_to_be32(0x0),
> > >> +cpu_to_be32(0x0),
> > >> +cpu_to_be32(phb->numa_node)};
> > >> sPAPRTCETable *tcet;
> > >> PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
> > >> sPAPRFDT s_fdt;
> > >> @@ -1837,6 +1844,12 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> > >> &ddw_extensions, sizeof(ddw_extensions)));
> > >> }
> > >>
> > >> +/* Advertise NUMA via ibm,associativity */
> > >> +if (nb_numa_nodes > 1) {
> > >> +_FDT(fdt_setprop(fdt, bus_off, "ibm,associativity",
> > >> associativity,
> > >> + sizeof(associativity)));
> > >> +}
> > >
> > > LoPAPR 15.3 seems to suggest that ibm,associativity-reference-points is
> > > required alongside ibm,associativity for each DT node it appears in,
> > > and since we hardcode "Form 1" affinity it should be done similarly as
> > > the entry we place in the top-level DT node.
> >
> >
> > Hm, okay, I'll add it. There is a question to Bharata - why do we have 4s
> > in spapr_create_fdt_skel()'s refpoints? Just a random pick?
> >
> > vec5[5]==0x80 means we are doing "Form1" and these 4s are far/near
> > distances?
> >
> >
> > >> +
> > >> /* Build the interrupt-map, this must matches what is done
> > >> * in pci_spapr_map_irq
> > >> */
> > >> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> > >> index 5adc603..53c4b2d 100644
> > >> --- a/include/hw/pci-host/spapr.h
> > >> +++ b/include/hw/pci-host/spapr.h
> > >> @@ -75,6 +75,8 @@
Re: [Qemu-devel] [PATCH qemu] spapr_pci: Add numa node id
On Wed, Sep 14, 2016 at 07:39:10PM +1000, Alexey Kardashevskiy wrote:
> On 14/09/16 09:29, Michael Roth wrote:
> > Quoting Alexey Kardashevskiy (2016-07-27 03:03:38)
> >> This adds a numa id property to a PHB to allow linking passed PCI device
> >> to CPU/memory. It is up to the management stack to do CPU/memory pinning
> >> to the node with the actual PCI device.
> >
> > It looks like x86 relies on PCIBus->numa_node() method (via
> > pci_bus_numa_node()) to encode similar PCIBus affinities
> > into ACPI tables, and currently exposes it via
> > -device pci-[-express]-expander-bus,numa_node=X.
>
> Well, until we allow DMA windows per PCI bus (not per PHB as it is now),
> this won't make much sense for us (unless I am missing something
> here).
I can't actually see any sane way we could have NUMA associativity of
PCI at any finer granularity than the host bridge level. I suspect
the only reason it works that way on x86 is due to some of the weird
stuff PC does to make what are effectively different host bridges
appear as different buses in a single logical domain.
>
> > Maybe we should implement it using this existing
> > PCIBus->numa_node() interface?
> >
> > We'd still have to expose numa_node as a spapr-pci-host-bridge
> > device option though. Not sure if there's a more common way
> > to expose it that might be easier for libvirt to discover. As it
> > stands we'd need to add spapr-pci-host-bridge to a libvirt
> > whitelist that currently only covers pci-expander-bus.
> >
> > Cc'ing Shiva who was looking into the libvirt side.
I think we should change the actual name of the option to "numa_node"
to match the option used on the pxb, though.
> >
> > One comment below:
> >
> >>
> >> Signed-off-by: Alexey Kardashevskiy
> >> ---
> >> hw/ppc/spapr_pci.c | 13 +
> >> include/hw/pci-host/spapr.h | 2 ++
> >> 2 files changed, 15 insertions(+)
> >>
> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> >> index 949c44f..af5394a 100644
> >> --- a/hw/ppc/spapr_pci.c
> >> +++ b/hw/ppc/spapr_pci.c
> >> @@ -47,6 +47,7 @@
> >> #include "sysemu/device_tree.h"
> >> #include "sysemu/kvm.h"
> >> #include "sysemu/hostmem.h"
> >> +#include "sysemu/numa.h"
> >>
> >> #include "hw/vfio/vfio.h"
> >>
> >> @@ -1544,6 +1545,7 @@ static Property spapr_phb_properties[] = {
> >> DEFINE_PROP_BOOL("ddw", sPAPRPHBState, ddw_enabled, true),
> >> DEFINE_PROP_UINT64("pgsz", sPAPRPHBState, page_size_mask,
> >> (1ULL << 12) | (1ULL << 16)),
> >> +DEFINE_PROP_UINT32("node", sPAPRPHBState, numa_node, -1),
> >> DEFINE_PROP_END_OF_LIST(),
> >> };
> >>
> >> @@ -1805,6 +1807,11 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> >> cpu_to_be32(1),
> >> cpu_to_be32(RTAS_IBM_RESET_PE_DMA_WINDOW)
> >> };
> >> +uint32_t associativity[] = {cpu_to_be32(0x4),
> >> +cpu_to_be32(0x0),
> >> +cpu_to_be32(0x0),
> >> +cpu_to_be32(0x0),
> >> +cpu_to_be32(phb->numa_node)};
> >> sPAPRTCETable *tcet;
> >> PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
> >> sPAPRFDT s_fdt;
> >> @@ -1837,6 +1844,12 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> >> &ddw_extensions, sizeof(ddw_extensions)));
> >> }
> >>
> >> +/* Advertise NUMA via ibm,associativity */
> >> +if (nb_numa_nodes > 1) {
> >> +_FDT(fdt_setprop(fdt, bus_off, "ibm,associativity", associativity,
> >> + sizeof(associativity)));
> >> +}
> >
> > LoPAPR 15.3 seems to suggest that ibm,associativity-reference-points is
> > required alongside ibm,associativity for each DT node it appears in,
> > and since we hardcode "Form 1" affinity it should be done similarly as
> > the entry we place in the top-level DT node.
>
>
> Hm, okay, I'll add it. There is a question to Bharata - why do we have 4s
> in spapr_create_fdt_skel()'s refpoints? Just a random pick?
>
> vec5[5]==0x80 means we are doing "Form1" and these 4s are far/near distances?
>
>
> >> +
> >> /* Build the interrupt-map, this must matches what is done
> >> * in pci_spapr_map_irq
> >> */
> >> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> >> index 5adc603..53c4b2d 100644
> >> --- a/include/hw/pci-host/spapr.h
> >> +++ b/include/hw/pci-host/spapr.h
> >> @@ -75,6 +75,8 @@ struct sPAPRPHBState {
> >> bool ddw_enabled;
> >> uint64_t page_size_mask;
> >> uint64_t dma64_win_addr;
> >> +
> >> +uint32_t numa_node;
> >> };
> >>
> >> #define SPAPR_PCI_MAX_INDEX 255
> >>
> >
>
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v5 1/8] linux-user: Add support for adjtimex() syscall
On Wed, Sep 14, 2016 at 10:19:54PM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic
>
> This patch implements Qemu user mode adjtimex() syscall support.
>
> Syscall adjtimex() reads and optionally sets parameters for a clock
> adjustment algorithm used in network synchonization or similar scenarios.
>
> The implementation is based on invocation of host's adjtimex(), and
> its key part is in the correspondent case segment of the main switch
> statement of the function do_syscall(), in file linux-user/syscalls.c.
> Also, support for related structure "timex" is added to the file
> linux-user/syscall_defs.h, based on its definition in Linux kernel. All
> necessary conversions of the data structures from target to host and from
> host to target are covered. Two new functions, target_to_host_timex() and
> host_to_target_timex(), are provided for the purpose of such conversions.
> Moreover, the relevant support for "-strace" Qemu option is included in
> files linux-user/strace.c and linux-user/strace.list.
>
> This patch also fixes failures of LTP tests adjtimex01 and adjtimex02, if
> executed in Qemu user mode.
>
> Signed-off-by: Aleksandar Rikalo
> Signed-off-by: Aleksandar Markovic
> ---
> linux-user/strace.c | 12 +++
> linux-user/strace.list| 2 +-
> linux-user/syscall.c | 90
> ++-
> linux-user/syscall_defs.h | 28 +++
> 4 files changed, 130 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index cc10dc4..7ddcaf8 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -919,6 +919,18 @@ print_access(const struct syscallname *name,
> }
> #endif
>
> +#ifdef TARGET_NR_adjtimex
The ifdef isn't needed, all linux platorms support adjtimex.
> +static void
> +print_adjtimex(const struct syscallname *name,
> +abi_long arg0, abi_long arg1, abi_long arg2,
> +abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +print_syscall_prologue(name);
> +print_pointer(arg0, 1);
> +print_syscall_epilogue(name);
> +}
> +#endif
> +
> #ifdef TARGET_NR_brk
> static void
> print_brk(const struct syscallname *name,
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index aa967a2..9a665a8 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -16,7 +16,7 @@
> { TARGET_NR_add_key, "add_key" , NULL, NULL, NULL },
> #endif
> #ifdef TARGET_NR_adjtimex
> -{ TARGET_NR_adjtimex, "adjtimex" , NULL, NULL, NULL },
> +{ TARGET_NR_adjtimex, "adjtimex" , NULL, print_adjtimex, NULL },
> #endif
> #ifdef TARGET_NR_afs_syscall
> { TARGET_NR_afs_syscall, "afs_syscall" , NULL, NULL, NULL },
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ca06943..5643840 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -35,6 +35,7 @@
> #include
> #include
> #include
> +#include
> #ifdef __ia64__
> int __clone2(int (*fn)(void *), void *child_stack_base,
> size_t stack_size, int flags, void *arg, ...);
> @@ -6578,6 +6579,78 @@ static inline abi_long target_ftruncate64(void
> *cpu_env, abi_long arg1,
> }
> #endif
>
> +#ifdef TARGET_NR_adjtimex
> +static inline abi_long target_to_host_timex(struct timex *host_buf,
> +abi_long target_addr)
> +{
> +struct target_timex *target_buf;
> +
> +if (!lock_user_struct(VERIFY_READ, target_buf, target_addr, 1)) {
> +return -TARGET_EFAULT;
> +}
> +
> +host_buf->modes = tswap32(target_buf->modes);
> +host_buf->offset = tswapal(target_buf->offset);
> +host_buf->freq = tswapal(target_buf->freq);
> +host_buf->maxerror = tswapal(target_buf->maxerror);
> +host_buf->esterror = tswapal(target_buf->esterror);
> +host_buf->status = tswap32(target_buf->status);
> +host_buf->constant = tswapal(target_buf->constant);
> +host_buf->precision = tswapal(target_buf->precision);
> +host_buf->tolerance = tswapal(target_buf->tolerance);
> +host_buf->time.tv_sec = tswapal(target_buf->time.tv_sec);
> +host_buf->time.tv_usec = tswapal(target_buf->time.tv_usec);
> +host_buf->tick = tswapal(target_buf->tick);
> +host_buf->ppsfreq = tswapal(target_buf->ppsfreq);
> +host_buf->jitter = tswapal(target_buf->jitter);
> +host_buf->shift = tswap32(target_buf->shift);
> +host_buf->stabil = tswapal(target_buf->stabil);
> +host_buf->jitcnt = tswapal(target_buf->jitcnt);
> +host_buf->calcnt = tswapal(target_buf->calcnt);
> +host_buf->errcnt = tswapal(target_buf->errcnt);
> +host_buf->stbcnt = tswapal(target_buf->stbcnt);
> +host_buf->tai = tswap32(target_buf->tai);
> +
> +unlock_user_struct(target_buf, target_addr, 0);
> +return 0;
> +}
> +
> +static inline abi_long host_to_target_timex(abi_long target_addr,
> +struct timex *host_buf)
> +{
> +struct target_timex *target_buf;
> +
> +if (!loc
Re: [Qemu-devel] [PATCH for-2.8 00/18] pc: q35: x2APIC support in kvm_apic mode
Hi, we had 3 problems left here. 1. IRQremapping can't work with x2apic_cluster mode. 2. apic_id > 255 can't receive devices interrupts. 3. windows crash when present IRQremapping capability to it. I test with latest kernel v4.8-rc7 and find all of them are unsolved. Also in the qemu and kernel code bases, I couldn't find some patches to solve them. Will you fix these problems or leave them to communities? Thanks, -Chao On Tue, Aug 09, 2016 at 02:51:16PM +0200, Radim Krčmář wrote: >2016-08-09 16:19+0800, Chao Gao: >> On Tue, Aug 09, 2016 at 02:18:15PM +0800, Peter Xu wrote: >>>I think the problem is with x2apic. Currently, x2apic is enabled in >>>vIOMMU when kernel irqchip is used. This is problematic, since >>>actually we are throughing away dest_id[31:8] without Radim's patches, >>>meanwhile I see that by default x2apic is using cluster mode. >>> >>>In cluster mode, 8 bits will possibly not suffice (I think the reason >>>why >17 vcpus will bring trouble is that each cluster has 16 vcpus, >>>we'll have trouble if we have more than one cluster). >>> >>>To temporarily solve your issue, you should not only need "-global >>>ioapic.version=0x20" in QEMU command line, but also add "x2apic_phys" >>>to you guest kernel boot parameter, to force guest boot with x2apic >>>physical mode (not cluster mode). Though this can only work for <255 >>>vcpus. IMHO we may still need to wait for Radim's patches to test >255 >>>case. >> >> ok, after adding "x2apic_phys" to guest kernel boot parameter, I >> boot up a 288(yes, 288) vcpus guest successfully with command >> qemu-system-x86_64 -boot c -m 4096 -sdl --enable-kvm \ >> -M kernel-irqchip=split -bios bios.bin -smp cpus=288 -hda vdisk.img \ >> -device intel-iommu,intremap=on -machine q35 >> Also, I can see interrupts on those cpu with inital apicid>255 from >> /proc/cpuinfo and /proc/interrupts. > >Great, thanks for testing! >Only IPIs will be correctly delivered to apic_id > 255 without few more >patches on the QEMU side, though. >
Re: [Qemu-devel] [PATCH] linux-user: fix TARGET_NR_select
On Sun, Sep 04, 2016 at 03:49:22PM +0200, Laurent Vivier wrote: > Le 11/07/2016 à 18:59, Peter Maydell a écrit : > > On 8 July 2016 at 00:17, Laurent Vivier wrote: > >> TARGET_NR_select can have three different implementations: > >> > >> 1- to always return -ENOSYS > >> > >> microblaze, ppc, ppc64 > >> > >> -> TARGET_WANT_NI_OLD_SELECT > >> > >> 2- to take parameters from a structure pointed by arg1 > >> (kernel sys_old_select) > >> > >> i386, arm, m68k > >> > >> -> TARGET_WANT_OLD_SYS_SELECT > >> > >> 3- to take parameters from arg[1-5] > >> (kernel sys_select) > >> > >> x86_64, alpha, s390x, > >> cris, sparc, sparc64 > >> > >> Some (new) architectures don't define NR_select, > >> > >> 4- but only NR__newselect with sys_select: > >> > >> mips, mips64, sh > >> > >> 5- don't define NR__newselect, and use pselect6 syscall: > >> > >> aarch64, openrisc, tilegx, unicore32 > >> > >> Reported-by: Timothy Pearson > >> Reported-by: Allan Wirth > >> Suggested-by: Peter Maydell > >> Signed-off-by: Laurent Vivier > >> --- > > > > Reviewed-by: Peter Maydell > > > > It seems this one has missed 2.7 window, perhaps it can queued for 2.8? Thanks for the reminder, applied to linux-user que Riku
Re: [Qemu-devel] [Qemu-ppc] KVM-PR is broken with current QEMU
On Wed, Sep 21, 2016 at 10:22:11AM +0200, Thomas Huth wrote: > On 20.09.2016 16:39, Cédric Le Goater wrote: > > On 09/20/2016 04:24 PM, Thomas Huth wrote: > >> On 20.09.2016 16:04, Cédric Le Goater wrote: > [...] > >>> There are other issues after in the guest (kernel crashing). But I think > >>> these are related to TM which is not supported in KVM-PR. I am not sure > >>> where we are on that point. > >> > >> There was a patch some months ago: > >> > >> https://lists.gnu.org/archive/html/qemu-ppc/2016-04/msg00046.html > >> > >> ... but I think it has never been included, as far as I can see. > > > > and with that patch, the guest fully boots. But David had some concerns > > on the way it is done. It would be nice to put some cycle on this. > > Looking at the mail thread, I think TM should be currently disabled for > both, KVM-PR and TCG, i.e. only enabled for KVM-HV. The TM support in > TCG is just fake, since TBEGIN always fails. Right. So there's two questions here 1) Is qemu correctly advertising availability of TM in the device tree? If not we need to fix that, which might involve adding a kernel capability for the PR case. 2) Is the kvm unit test properly checking for availability of TM before executing? > Once we've got proper TM support in TCG, this can be easily changed > within QEMU. And once we've got TM support in KVM-PR, I think we should > also introduce a capability flag to KVM which can be used to inform QEMU > about this. > > So I think Anton's patch currently just lacks the check for TCG. > Anton, if you've got some spare minutes, could you maybe send an updated > version of that patch? Sorry, which patch of Anton's? -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[Qemu-devel] [PATCH] qapi: make the json schema files more regular.
This makes it easier to parse the schema file for tool generation:
each paragraph is either a non-docstring comment, or a docstring
immediately followed by a Python dict describing an API item.
Signed-off-by: David Anderson
---
qapi-schema.json | 3 +--
qapi/block-core.json | 5 -
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index e507061..edd803f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -752,6 +752,7 @@
'cpu-throttle-increment': 'int',
'tls-creds': 'str',
'tls-hostname': 'str'} }
+
##
# @query-migrate-parameters
#
@@ -4115,7 +4116,6 @@
#
# Since 1.6
##
-
{ 'struct': 'RxFilterInfo',
'data': {
'name': 'str',
@@ -4335,7 +4335,6 @@
#
# Since: 2.1
##
-
{ 'struct': 'Memdev',
'data': {
'size': 'size',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 24223fd..e12fbd3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -25,7 +25,6 @@
# Since: 1.3
#
##
-
{ 'struct': 'SnapshotInfo',
'data': { 'id': 'str', 'name': 'str', 'vm-state-size': 'int',
'date-sec': 'int', 'date-nsec': 'int',
@@ -81,7 +80,6 @@
#
# Since: 1.7
##
-
{ 'union': 'ImageInfoSpecific',
'data': {
'qcow2': 'ImageInfoSpecificQCow2',
@@ -129,7 +127,6 @@
# Since: 1.3
#
##
-
{ 'struct': 'ImageInfo',
'data': {'filename': 'str', 'format': 'str', '*dirty-flag': 'bool',
'*actual-size': 'int', 'virtual-size': 'int',
@@ -181,7 +178,6 @@
# Since: 1.4
#
##
-
{ 'struct': 'ImageCheck',
'data': {'filename': 'str', 'format': 'str', 'check-errors': 'int',
'*image-end-offset': 'int', '*corruptions': 'int', '*leaks': 'int',
@@ -518,7 +514,6 @@
#
# Since: 2.5
##
-
{ 'struct': 'BlockDeviceTimedStats',
'data': { 'interval_length': 'int', 'min_rd_latency_ns': 'int',
'max_rd_latency_ns': 'int', 'avg_rd_latency_ns': 'int',
--
2.9.3
Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration
> My concern is that a type id seems arbitrary but we're specifying that > it be unique. We already have something unique, the name. So why try > to make the type id unique as well? A vendor can accidentally create > their vendor driver so that a given name means something very > specific. On the other hand they need to be extremely deliberate to > coordinate that a type id means a unique thing across all their product > lines. > Let me clarify, type id should be unique in the list of mdev_supported_types. You can't have 2 directories in with same name. >>> >>> Of course, but does that mean it's only unique to the machine I'm >>> currently running on? Let's say I have a Tesla P100 on my system and >>> type-id 11 is named "GRID-M60-0B". At some point in the future I >>> replace the Tesla P100 with a Q1000 (made up). Is type-id 11 on that >>> new card still going to be a "GRID-M60-0B"? If not then we've based >>> our XML on the wrong attribute. If the new device does not support >>> "GRID-M60-0B" then we should generate an error, not simply initialize >>> whatever type-id 11 happens to be on this new card. >>> >> >> If there are 2 M60 in the system then you would find '11' type directory >> in mdev_supported_types of both M60. If you have P100, '11' type would >> not be there in its mdev_supported_types, it will have different types. >> >> For example, if you replace M60 with P100, but XML is not updated. XML >> have type '11'. When libvirt would try to create mdev device, libvirt >> would have to find 'create' file in sysfs in following directory format: >> >> --- mdev_supported_types >> |-- 11 >> | |-- create >> >> but now for P100, '11' directory is not there, so libvirt should throw >> error on not able to find '11' directory. > > This really seems like an accident waiting to happen. What happens > when the user replaces their M60 with an Intel XYZ device that happens > to expose a type 11 mdev class gpu device? How is libvirt supposed to > know that the XML used to refer to a GRID-M60-0B and now it's an > INTEL-IGD-XYZ? Doesn't basing the XML entry on the name and removing > yet another arbitrary requirement that we have some sort of globally > unique type-id database make a lot of sense? The same issue applies > for simple debug-ability, if I'm reviewing the XML for a domain and the > name is the primary index for the mdev device, I know what it is. > Seeing type-id='11' is meaningless. > Let me clarify again, type '11' is a string that vendor driver would define (see my previous reply below) it could be "11" or "GRID-M60-0B". If 2 vendors used same string we can't control that. right? Lets remove 'id' from type id in XML if that is the concern. Supported types is going to be defined by vendor driver, so let vendor driver decide what to use for directory name and same should be used in device xml file, it could be '11' or "GRID M60-0B": my-vgpu pci__86_00_0 ... Kirti
Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration
> From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Thursday, September 22, 2016 11:02 AM > > On Thu, 22 Sep 2016 02:33:01 + > "Tian, Kevin" wrote: > > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > > Sent: Wednesday, September 21, 2016 12:37 PM > > > > > > On Wed, 21 Sep 2016 03:56:21 + > > > "Tian, Kevin" wrote: > > > > > > > > From: Kirti Wankhede [mailto:kwankh...@nvidia.com] > > > > > Sent: Tuesday, September 20, 2016 10:22 PM > > > > > >> > > > > > >> 'max_instance': > > > > > >> Read-Only file. Mandatory. > > > > > >> Returns integer. Returns maximum mdev device could be created > > > > > >> at the moment when this file is read. This count would be updated > > > > > >> by > > > > > >> vendor driver. Before creating mdev device of this type, check if > > > > > >> max_instance is > 0. > > > > > > > > > > > > Didn't we discuss this being being something like > > > > > > "available_instances" > > > > > > with a "devices" sub-directory linking current devices of that type? > > > > > > > > > > > > > > > > I'm ok with 'available_instances' as name of this file. > > > > > Yes, mdev device directory will have links to devices of that type. I > > > > > think that is part of mdev core module discussion. I'm trying to list > > > > > sysfs files for libvirt interface here to focus more on libvirt > > > > > interface. Hence didn't add that. I'll make sure to add all such > > > > > details > > > > > in future. > > > > > > > > > > > > > > > > > > Definitely you need provide those details since they also contain > > > > libvirt interface, e.g. 'destroy' which we discussed should be in > > > > mdev directory. > > > > > > $ find /sys/devices -name remove | wc -l > > > 24 > > > $ find /sys/devices -name destroy | wc -l > > > 0 > > > > > > 'remove' is the sysfs deconstructor we should use. > > > > make sense. > > > > > > > > > Another example is class-specific attributes such > > > > as 'resolution', etc. which you listed under 'display class'. All those > > > > attributes should be moved to mdev directory. Only class ID is > > > > required under each type. > > > > > > Can you elaborate on what you're proposing here? If we don't have > > > attributes like 'resolution' under the type-id then we can't describe > > > to the user the features of the mdev before we create it. I don't > > > think anybody wants a 'create it and find out' type of interface. > > > Thanks, > > > > > > > I think such way would be racy. What about two clients creating mdev > > devices simultaneously? How to guarantee their configuration of class > > specific attributes not mutual-impacted since before creation any such > > configuration would be global under that type? > > A simple mutex in the sysfs store attribute to serialize, return write > error if insufficient resources to create additional devices... Easy. > Let's not exaggerate the issue. > > > My feeling is that we'd better keep create simple - just write a UUID > > to "type-id/create". Any class-specific attribute, if we really want to > > support, should be able to be individually configured with required > > resource allocated incrementally on top of basic resources allocated > > at create stage. Then libvirt can set those attributes between create > > and open a mdev device. If this direction is accepted, then naturally > > such attributes should be put under mdev directory. Possibly we > > don't need a class description under type-id. libvirt just checks directly > > whether any known class represented in each mdev directory (vendor > > driver will expose it on demand), and then operate attributes under > > that class. > > It seems like this just moves the hypothetical race to the point where > we're adjusting individual attributes for the mdev device, but now the > problem is worse because there's no simple locking or serialization > that guarantees we can get all the attributes we want. We might get > half the attributes for one device and half the attributes for another > and not be able to complete initialization of either. I think we also > lose a lot of our ability to expose pre-defined configurations to the > user and the vendor's test matrix explodes. We also can't provide any > sort of reasonable estimate of how many devices can be created because > we don't know the resource requirements of each device. I don't like > this idea at all, sorry. Thanks, > Yes, it's a valid concern. Alex, curious about your opinion of supporting some device-agnostic optional attributes. One example is to set QoS parameters (priority, weight, etc.), which may be enabled by vendor driver to allow dynamic adjustment even after a mdev device is created. Do we still put a QoS class under type-id, meaning must-be-configured before creation (not a bad limitation since from SLA p.o.v all QoS attributes should be negotiated down in advance)? Would there be any concern if a type-id contains multiple classes (e.g. include both 'gpu'
Re: [Qemu-devel] [PATCH for 2.8 2/2] Added Traditional Chinese translation
On Thu, Sep 22, 2016 at 11:16:03AM +0800, Fam Zheng wrote: > On Thu, 09/22 10:34, Peter Xu wrote: > > So we are using traditional Chinese? That's fine to me though. :) > > We have had simplified Chinese in the tree since years ago. This one is in > addition to that. Yeah I just realized that. :) -- peterx
Re: [Qemu-devel] [PATCH for 2.8 2/2] Added Traditional Chinese translation
On Wed, Sep 21, 2016 at 09:44:03PM -0500, Wei Huang wrote: > > > On 09/21/2016 09:34 PM, Peter Xu wrote: > > On Wed, Sep 21, 2016 at 11:27:24AM -0500, Wei Huang wrote: > >> Add Traditional Chinese (zh_TW) translation to QEMU. The translations > >> use other virt software, such as virt-manger, as references. > >> > >> CC: Peter Xu > >> CC: Fam Zheng > >> Signed-off-by: Wei Huang > > > > Reviewed-by: Peter Xu > > > > So we are using traditional Chinese? That's fine to me though. :) > > > > How can I try it? I tried to: > > > > $ LANG="zh_CN.UTF-8" $QEMU_CMDLINE > > > > But what I see is still "???" on all the buttons. I must have missed > > some steps here. > > The language will be picked up automatically based on locale setting. I > saw QEMU is missing zh_TW and just went ahead to add it. What you did is > right, but there is a bug in QEMU. You can apply the patch in > https://patchwork.ozlabs.org/patch/558872/ and revert 2cb5d2a4 to > display the characters correctly. Yes it worked. Thanks! -- peterx
Re: [Qemu-devel] [PATCH for 2.8 2/2] Added Traditional Chinese translation
On Thu, 09/22 10:34, Peter Xu wrote: > So we are using traditional Chinese? That's fine to me though. :) We have had simplified Chinese in the tree since years ago. This one is in addition to that. Fam
Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration
On Thu, 22 Sep 2016 02:33:01 + "Tian, Kevin" wrote: > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > Sent: Wednesday, September 21, 2016 12:37 PM > > > > On Wed, 21 Sep 2016 03:56:21 + > > "Tian, Kevin" wrote: > > > > > > From: Kirti Wankhede [mailto:kwankh...@nvidia.com] > > > > Sent: Tuesday, September 20, 2016 10:22 PM > > > > >> > > > > >> 'max_instance': > > > > >> Read-Only file. Mandatory. > > > > >> Returns integer. Returns maximum mdev device could be created > > > > >> at the moment when this file is read. This count would be updated by > > > > >> vendor driver. Before creating mdev device of this type, check if > > > > >> max_instance is > 0. > > > > > > > > > > Didn't we discuss this being being something like > > > > > "available_instances" > > > > > with a "devices" sub-directory linking current devices of that type? > > > > > > > > > > > > > I'm ok with 'available_instances' as name of this file. > > > > Yes, mdev device directory will have links to devices of that type. I > > > > think that is part of mdev core module discussion. I'm trying to list > > > > sysfs files for libvirt interface here to focus more on libvirt > > > > interface. Hence didn't add that. I'll make sure to add all such details > > > > in future. > > > > > > > > > > > > > > Definitely you need provide those details since they also contain > > > libvirt interface, e.g. 'destroy' which we discussed should be in > > > mdev directory. > > > > $ find /sys/devices -name remove | wc -l > > 24 > > $ find /sys/devices -name destroy | wc -l > > 0 > > > > 'remove' is the sysfs deconstructor we should use. > > make sense. > > > > > > Another example is class-specific attributes such > > > as 'resolution', etc. which you listed under 'display class'. All those > > > attributes should be moved to mdev directory. Only class ID is > > > required under each type. > > > > Can you elaborate on what you're proposing here? If we don't have > > attributes like 'resolution' under the type-id then we can't describe > > to the user the features of the mdev before we create it. I don't > > think anybody wants a 'create it and find out' type of interface. > > Thanks, > > > > I think such way would be racy. What about two clients creating mdev > devices simultaneously? How to guarantee their configuration of class > specific attributes not mutual-impacted since before creation any such > configuration would be global under that type? A simple mutex in the sysfs store attribute to serialize, return write error if insufficient resources to create additional devices... Easy. Let's not exaggerate the issue. > My feeling is that we'd better keep create simple - just write a UUID > to "type-id/create". Any class-specific attribute, if we really want to > support, should be able to be individually configured with required > resource allocated incrementally on top of basic resources allocated > at create stage. Then libvirt can set those attributes between create > and open a mdev device. If this direction is accepted, then naturally > such attributes should be put under mdev directory. Possibly we > don't need a class description under type-id. libvirt just checks directly > whether any known class represented in each mdev directory (vendor > driver will expose it on demand), and then operate attributes under > that class. It seems like this just moves the hypothetical race to the point where we're adjusting individual attributes for the mdev device, but now the problem is worse because there's no simple locking or serialization that guarantees we can get all the attributes we want. We might get half the attributes for one device and half the attributes for another and not be able to complete initialization of either. I think we also lose a lot of our ability to expose pre-defined configurations to the user and the vendor's test matrix explodes. We also can't provide any sort of reasonable estimate of how many devices can be created because we don't know the resource requirements of each device. I don't like this idea at all, sorry. Thanks, Alex
Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration
> From: Tian, Kevin > Sent: Thursday, September 22, 2016 10:33 AM > > > > > > Another example is class-specific attributes such > > > as 'resolution', etc. which you listed under 'display class'. All those > > > attributes should be moved to mdev directory. Only class ID is > > > required under each type. > > > > Can you elaborate on what you're proposing here? If we don't have > > attributes like 'resolution' under the type-id then we can't describe > > to the user the features of the mdev before we create it. I don't > > think anybody wants a 'create it and find out' type of interface. > > Thanks, > > > > I think such way would be racy. What about two clients creating mdev > devices simultaneously? How to guarantee their configuration of class > specific attributes not mutual-impacted since before creation any such > configuration would be global under that type? > > My feeling is that we'd better keep create simple - just write a UUID > to "type-id/create". Any class-specific attribute, if we really want to > support, should be able to be individually configured with required > resource allocated incrementally on top of basic resources allocated > at create stage. Then libvirt can set those attributes between create > and open a mdev device. If this direction is accepted, then naturally > such attributes should be put under mdev directory. Possibly we > don't need a class description under type-id. libvirt just checks directly > whether any known class represented in each mdev directory (vendor > driver will expose it on demand), and then operate attributes under > that class. > Have a better understanding of your concern now. User needs to know and set a list of attributes before requesting to create a new mdev device. From this angle all attributes should be enumerated per type-id. But as I explained above, writing multiple sysfs nodes to create a mdev device is racy. We either need a way to guarantee transactional operations on those nodes, or having type-id directory to only expose supported attributes while programming those attributes has to be through per-mdev directory after mdev device is created... Thanks Kevin
Re: [Qemu-devel] [PATCH v2 2/8] nvdimm acpi: prebuild nvdimm devices for available slots
On 09/21/2016 07:48 PM, Igor Mammedov wrote:
On Fri, 12 Aug 2016 14:54:04 +0800
Xiao Guangrong wrote:
For each NVDIMM present or intended to be supported by platform,
platform firmware also exposes an ACPI Namespace Device under
the root device
So it builds nvdimm devices for all slots to support vNVDIMM hotplug
Signed-off-by: Xiao Guangrong
---
hw/acpi/nvdimm.c| 41 -
hw/i386/acpi-build.c| 2 +-
include/hw/mem/nvdimm.h | 3 ++-
3 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 5454c0f..0e2b9f0 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -886,12 +886,11 @@ static void nvdimm_build_device_dsm(Aml *dev, uint32_t
handle)
aml_append(dev, method);
}
-static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
+static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
{
-for (; device_list; device_list = device_list->next) {
-DeviceState *dev = device_list->data;
-int slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP,
- NULL);
+uint32_t slot;
+
+for (slot = 0; slot < ram_slots; slot++) {
uint32_t handle = nvdimm_slot_to_handle(slot);
Aml *nvdimm_dev;
@@ -912,9 +911,9 @@ static void nvdimm_build_nvdimm_devices(GSList
*device_list, Aml *root_dev)
}
}
-static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
- GArray *table_data, BIOSLinker *linker,
- GArray *dsm_dma_arrea)
+static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
+ BIOSLinker *linker, GArray *dsm_dma_arrea,
+ uint32_t ram_slots)
{
Aml *ssdt, *sb_scope, *dev, *field;
int mem_addr_offset, nvdimm_ssdt;
@@ -1003,7 +1002,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray
*table_offsets,
/* 0 is reserved for root device. */
nvdimm_build_device_dsm(dev, 0);
-nvdimm_build_nvdimm_devices(device_list, dev);
+nvdimm_build_nvdimm_devices(dev, ram_slots);
aml_append(sb_scope, dev);
aml_append(ssdt, sb_scope);
@@ -1028,17 +1027,25 @@ static void nvdimm_build_ssdt(GSList *device_list,
GArray *table_offsets,
}
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
- BIOSLinker *linker, GArray *dsm_dma_arrea)
+ BIOSLinker *linker, GArray *dsm_dma_arrea,
+ uint32_t ram_slots)
{
GSList *device_list;
-/* no NVDIMM device is plugged. */
device_list = nvdimm_get_plugged_device_list();
-if (!device_list) {
-return;
+
+/* NVDIMM device is plugged. */
+if (device_list) {
+nvdimm_build_nfit(device_list, table_offsets, table_data, linker);
+g_slist_free(device_list);
+}
+
+/*
+ * NVDIMM device is allowed to be plugged only if there has available
s/has/is/
Will fix. Thank you for pointing it out.
+ * slot.
+ */
+if (ram_slots) {
another question:
Is NFIT table generated above sufficient without below nvdim SSDT?
maybe you should put
if (!ram_slots) {
return;
}
at the function start?
Not needed.
As the NFIT table is created only if there is nvdimm device already plugged
that means QEMU must have available ram-slots.
Re: [Qemu-devel] [PATCH for 2.8 2/2] Added Traditional Chinese translation
On 09/21/2016 09:34 PM, Peter Xu wrote: > On Wed, Sep 21, 2016 at 11:27:24AM -0500, Wei Huang wrote: >> Add Traditional Chinese (zh_TW) translation to QEMU. The translations >> use other virt software, such as virt-manger, as references. >> >> CC: Peter Xu >> CC: Fam Zheng >> Signed-off-by: Wei Huang > > Reviewed-by: Peter Xu > > So we are using traditional Chinese? That's fine to me though. :) > > How can I try it? I tried to: > > $ LANG="zh_CN.UTF-8" $QEMU_CMDLINE > > But what I see is still "???" on all the buttons. I must have missed > some steps here. The language will be picked up automatically based on locale setting. I saw QEMU is missing zh_TW and just went ahead to add it. What you did is right, but there is a bug in QEMU. You can apply the patch in https://patchwork.ozlabs.org/patch/558872/ and revert 2cb5d2a4 to display the characters correctly. -Wei > > Thanks, > > -- peterx >
Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Wednesday, September 21, 2016 12:43 PM
>
> On Wed, 21 Sep 2016 04:10:53 +
> "Tian, Kevin" wrote:
>
> > > From: Kirti Wankhede [mailto:kwankh...@nvidia.com]
> > > Sent: Wednesday, September 21, 2016 12:23 AM
> > >
> > >
> > > On 9/20/2016 8:13 PM, Alex Williamson wrote:
> > > > On Tue, 20 Sep 2016 19:51:58 +0530
> > > > Kirti Wankhede wrote:
> > > >
> > > >> On 9/20/2016 3:06 AM, Alex Williamson wrote:
> > > >>> On Tue, 20 Sep 2016 02:05:52 +0530
> > > >>> Kirti Wankhede wrote:
> > > >>>
> > > Hi libvirt experts,
> > >
> > >
> > > 'create':
> > > Write-only file. Mandatory.
> > > Accepts string to create mediated device.
> > >
> > > 'name':
> > > Read-Only file. Mandatory.
> > > Returns string, the name of that type id.
> > >
> > > 'fb_length':
> > > Read-only file. Mandatory.
> > > Returns {K,M,G}, size of framebuffer.
> > > >>>
> > > >>> This can't be mandatory, it's only relevant to vGPU devices, vGPUs are
> > > >>> just one user of mediated devices.
> > > >>>
> > > >>
> > > >> As per our discussion in BOF, we would define class and its attributes.
> > > >> As I mentioned above these are attributes of "display" class.
> > > >
> > > > Just as I didn't know here, how does libvirt know the class of a given
> > > > type id?
> > > >
> > >
> > > Yes, we need some way to identify the class as Daniel mentioned in his
> > > suggestion. Add a file 'class' in mdev_supported_types that would return
> > > the class.
> > >
> > >
> >
> > 'display' class or 'gpu' class? display represents only part of GPU
> > functionalities. I assume you want to provide an unique class ID
> > for each type, instead of allowing multiple classes each identifying
> > a subset of controllable GPU resources (e.g. 'display', 'render', etc.)...
>
> Not sure where you're going with this, we're not creating a programming
> interface for the device, that exists through the vfio API. I believe
> the intention here is simply a user level classification for the
> purpose of being able to interpret the attributes provided in a logical
> way.
I'm not against that purpose.
My main intention is that 'display' is not an accurate classification here.
If we allow classification in 'display' level, it implies more finer-grained
classifications may be further defined upon which vendor specific GPU
resources is allowed to be configured, which might be an overkill. Here
we just need a general classification like 'gpu' to cover all possible
vendor-agnostic attributes beyond display.
>
> > for unique class ID, I once considered whether it could be directly
> > inherited from class of parent device, since typically a vendor driver
> > creates mdev devices using the same type as physical device. But later
> > I realized one potential value of keep a class field for mdev types,
> > e.g. if we want to extend mdev to FPGA which could be programmed
> > as different classes of functionalities. :-)
>
> And how do we generically determine the class of a parent device? We
> cannot assume the parent device is PCI. Thanks,
>
Yes, this is also a good point.
Thanks
Kevin
Re: [Qemu-devel] [PATCH for 2.8 2/2] Added Traditional Chinese translation
On Wed, Sep 21, 2016 at 11:27:24AM -0500, Wei Huang wrote: > Add Traditional Chinese (zh_TW) translation to QEMU. The translations > use other virt software, such as virt-manger, as references. > > CC: Peter Xu > CC: Fam Zheng > Signed-off-by: Wei Huang Reviewed-by: Peter Xu So we are using traditional Chinese? That's fine to me though. :) How can I try it? I tried to: $ LANG="zh_CN.UTF-8" $QEMU_CMDLINE But what I see is still "???" on all the buttons. I must have missed some steps here. Thanks, -- peterx
Re: [Qemu-devel] [RFC v2] libvirt vGPU QEMU integration
> From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Wednesday, September 21, 2016 12:37 PM > > On Wed, 21 Sep 2016 03:56:21 + > "Tian, Kevin" wrote: > > > > From: Kirti Wankhede [mailto:kwankh...@nvidia.com] > > > Sent: Tuesday, September 20, 2016 10:22 PM > > > >> > > > >> 'max_instance': > > > >> Read-Only file. Mandatory. > > > >> Returns integer. Returns maximum mdev device could be created > > > >> at the moment when this file is read. This count would be updated by > > > >> vendor driver. Before creating mdev device of this type, check if > > > >> max_instance is > 0. > > > > > > > > Didn't we discuss this being being something like "available_instances" > > > > with a "devices" sub-directory linking current devices of that type? > > > > > > > > > > I'm ok with 'available_instances' as name of this file. > > > Yes, mdev device directory will have links to devices of that type. I > > > think that is part of mdev core module discussion. I'm trying to list > > > sysfs files for libvirt interface here to focus more on libvirt > > > interface. Hence didn't add that. I'll make sure to add all such details > > > in future. > > > > > > > > > > Definitely you need provide those details since they also contain > > libvirt interface, e.g. 'destroy' which we discussed should be in > > mdev directory. > > $ find /sys/devices -name remove | wc -l > 24 > $ find /sys/devices -name destroy | wc -l > 0 > > 'remove' is the sysfs deconstructor we should use. make sense. > > > Another example is class-specific attributes such > > as 'resolution', etc. which you listed under 'display class'. All those > > attributes should be moved to mdev directory. Only class ID is > > required under each type. > > Can you elaborate on what you're proposing here? If we don't have > attributes like 'resolution' under the type-id then we can't describe > to the user the features of the mdev before we create it. I don't > think anybody wants a 'create it and find out' type of interface. > Thanks, > I think such way would be racy. What about two clients creating mdev devices simultaneously? How to guarantee their configuration of class specific attributes not mutual-impacted since before creation any such configuration would be global under that type? My feeling is that we'd better keep create simple - just write a UUID to "type-id/create". Any class-specific attribute, if we really want to support, should be able to be individually configured with required resource allocated incrementally on top of basic resources allocated at create stage. Then libvirt can set those attributes between create and open a mdev device. If this direction is accepted, then naturally such attributes should be put under mdev directory. Possibly we don't need a class description under type-id. libvirt just checks directly whether any known class represented in each mdev directory (vendor driver will expose it on demand), and then operate attributes under that class. Thanks Kevin
Re: [Qemu-devel] [PATCH] eepro100: Frame Reception
On 2016年08月24日 00:46, Mateus Krepsky Ludwich wrote:
Changed E100 device so it updates EOF, F, and Actual Count fields of Receive
Frame Descriptor (RFD).
Actual Count fields were updated before this patch I think?
Assuming RFD.actual_count equals to size.
Signed-off-by: Mateus Krepsky Ludwich
---
hw/net/eepro100.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index bab4dbf..c0f3816 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1739,6 +1739,7 @@ static ssize_t nic_receive(NetClientState *nc,
const uint8_t * buf, size_t size)
&rx, sizeof(eepro100_rx_t));
uint16_t rfd_command = le16_to_cpu(rx.command);
uint16_t rfd_size = le16_to_cpu(rx.size);
+uint16_t rfd_count = size | 0xc000 ; /* Setting EOF and F bits */
if (size > rfd_size) {
logout("Receive buffer (%" PRId16 " bytes) too small for data "
@@ -1756,6 +1757,8 @@ static ssize_t nic_receive(NetClientState *nc,
const uint8_t * buf, size_t size)
offsetof(eepro100_rx_t, status), rfd_status);
stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
offsetof(eepro100_rx_t, count), size);
Then this line can be removed, or what's maybe even better, you can just
reuse rfd_size.
+stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
+offsetof(eepro100_rx_t, count), rfd_count);
/* Early receive interrupt not supported. */
#if 0
eepro100_er_interrupt(s);
We need a better title, e.g "Fixing EOF and F filed when updating RFD"
Thanks
Re: [Qemu-devel] [PATCH 0/7] virtio: avoid inappropriate QEMU termination
On 2016/9/21 21:13, Greg Kurz wrote: > This series is a follow up to Stefan's work to eradicate most calls to > exit() we currently have in the virtio code. > > It addresses all exit() call sites in the blk, net and scsi device code, > where the error is about a missing or malformed in/out header sent by > the guest. They are converted to use virtio_error() and stop any processing, > instead of exiting. > Actually if you just stop procesing when encounter a missing in/out header but send a interrupt to the guest, the guest maybe be stuck. virtio_net_handle_ctrl() is an example, the guest frontend driver infinite loop to wait the interrupt's coming. The guest can't work anymore though you don't exit the Qemu process . Regards, -Gonglei > The remaining call sites are related to a host misconfiguration or a > migration stream issue. > > The 9P code currently calls assert() instead of exit(), but it also about > malformed or missing headers, so it gets converted the same way. > > Next work will be to check all assert() call sites in the device code, in > case some of them actually refer to a bug in the guest, and should be > converted to use virtio_error() as well. > > --- > > Greg Kurz (7): > virtio-9p: handle handle_9p_output() error > virtio-blk: handle virtio_blk_handle_request() errors > virtio-net: handle virtio_net_handle_ctrl() error > virtio-net: handle virtio_net_receive() errors > virtio-net: handle virtio_net_flush_tx() errors > virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error() > virtio-scsi: handle virtio_scsi_set_config() error > > > hw/9pfs/virtio-9p-device.c | 14 ++-- > hw/block/virtio-blk.c | 27 +++ > hw/net/virtio-net.c| 51 > +--- > hw/scsi/virtio-scsi.c | 21 ++ > 4 files changed, 70 insertions(+), 43 deletions(-) > > -- > Greg > > > >
Re: [Qemu-devel] [PATCH] rbd : disable rbd_cache_writethrough_until_flush for cache=unsafe
On 07/27/2016 05:26 AM, Alexandre Derumier wrote:
ceph force writethrough until a flush is detected.
With cache=unsafe, we never send flush.
So we need to tell to ceph
to set rbd_cache_writethrough_until_flush=false in this case.
This speedup a lot qemu-img convert which use cache=unsafe by default
Signed-off-by: Alexandre Derumier
Sorry for the delay - this is correct and needed since this option
appeared by default in Ceph hammer.
Reviewed-by: Josh Durgin
---
block/rbd.c | 4
1 file changed, 4 insertions(+)
diff --git a/block/rbd.c b/block/rbd.c
index 0106fea..f3af6c8 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -552,6 +552,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict
*options, int flags,
rados_conf_set(s->cluster, "rbd_cache", "true");
}
+if (flags & BDRV_O_NO_FLUSH) {
+rados_conf_set(s->cluster, "rbd_cache_writethrough_until_flush",
"false");
+}
+
r = rados_connect(s->cluster);
if (r < 0) {
error_setg_errno(errp, -r, "error connecting");
Re: [Qemu-devel] [PATCH] monitor: fix crash for platforms without a CPU 0
On Wed, Sep 21, 2016 at 09:14:02AM -0300, Eduardo Habkost wrote:
> On Wed, Sep 21, 2016 at 03:29:26PM +1000, David Gibson wrote:
> > Now that we allow CPU hot unplug on a few platforms, we can end up in a
> > situation where we don't have a CPU with index 0. Or at least we could,
> > if we didn't have code to explicitly prohibit unplug of CPU 0.
> >
> > Longer term we want to allow CPU 0 unplug, this patch is an early step in
> > allowing this, by removing an assumption in the monitor code that CPU 0
> > always exists.
> >
> > Signed-off-by: Cédric Le Goater
> > [dwg: Rewrote commit message to better explain background]
> > Signed-off-by: David Gibson
> > ---
> > monitor.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Anyone want to volunteer to take this through their tree? If not, I
> > can take it through my ppc tree.
> >
> > diff --git a/monitor.c b/monitor.c
> > index 8bb8bbf..83c4edf 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1025,7 +1025,7 @@ int monitor_set_cpu(int cpu_index)
> > CPUState *mon_get_cpu(void)
> > {
> > if (!cur_mon->mon_cpu) {
> > -monitor_set_cpu(0);
> > +monitor_set_cpu(first_cpu->cpu_index);
>
> So, we are replacing the "CPU 0 always exists" assumption with a
> "first_cpu is always non-NULL" assumption.
Well, we're replacing "CPU 0 is always present" assumption with "At
least one CPU is always present", which is a strictly weaker
constraint.
> But considering that the first_cpu assumption already exists
> elsewhere and those cases can be found easily using grep, I think
> this is OK. So:
>
> Reviewed-by: Eduardo Habkost
>
> BTW, it is also possible to crash QEMU by unplugging the current
> monitor CPU;
Ah... good point.
> (qemu) device_add
> qemu64-x86_64-cpu,socket-id=2,core-id=0,thread-id=0,id=mycpu
> (qemu) cpu 2
> (qemu) device_del mycpu
> (qemu) info registers
> qemu:qemu_cpu_kick_thread: No such process
> $
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v4 9/9] ppc/xics: move set_nr_{irqs, servers} to xics.c
On Mon, Sep 19, 2016 at 11:59:37AM +0530, Nikunj A Dadhania wrote:
> Get this duplicate code in the base implementation.
>
> Signed-off-by: Nikunj A Dadhania
> ---
> hw/intc/xics.c| 74
> ---
> hw/intc/xics_kvm.c| 34 +++
> hw/intc/xics_native.c | 51 +--
> hw/intc/xics_spapr.c | 49 +-
> include/hw/ppc/xics.h | 19 ++---
> 5 files changed, 126 insertions(+), 101 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 4ac2d00..9fbf962 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -54,8 +54,9 @@ int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
> void xics_cpu_destroy(XICSState *xics, PowerPCCPU *cpu)
> {
> CPUState *cs = CPU(cpu);
> -ICPState *ss = &xics->ss[cs->cpu_index];
> +ICPState *ss;
>
> +ss = xics->ss + sizeof(ICPState) * cs->cpu_index;
> assert(cs->cpu_index < xics->nr_servers);
> assert(cs == ss->cs);
>
> @@ -67,9 +68,10 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
> {
> CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> -ICPState *ss = &xics->ss[cs->cpu_index];
> +ICPState *ss;
> XICSStateClass *info = XICS_COMMON_GET_CLASS(xics);
>
> +ss = xics->ss + sizeof(ICPState) * cs->cpu_index;
> assert(cs->cpu_index < xics->nr_servers);
>
> ss->cs = cs;
> @@ -101,10 +103,12 @@ static void xics_common_reset(DeviceState *d)
> {
> XICSState *xics = XICS_COMMON(d);
> ICSState *ics;
> +ICPState *ss;
> int i;
>
> for (i = 0; i < xics->nr_servers; i++) {
> -device_reset(DEVICE(&xics->ss[i]));
> +ss = xics->ss + sizeof(ICPState) * i;
> +device_reset(DEVICE(ss));
> }
>
> QLIST_FOREACH(ics, &xics->ics, list) {
> @@ -193,6 +197,7 @@ static void xics_common_initfn(Object *obj)
> XICSState *xics = XICS_COMMON(obj);
>
> QLIST_INIT(&xics->ics);
> +xics->ss_class = object_class_by_name(TYPE_ICP);
> object_property_add(obj, "nr_irqs", "int",
> xics_prop_get_nr_irqs, xics_prop_set_nr_irqs,
> NULL, NULL, NULL);
> @@ -201,7 +206,15 @@ static void xics_common_initfn(Object *obj)
> NULL, NULL, NULL);
>
> /* For exclusive use of monitor command */
> -g_xics = XICS_COMMON(obj);
> +g_xics = xics;
Looks like this change should be folded into the patch introducing g_xics.
> +}
> +
> +static void xics_common_realize(DeviceState *dev, Error **errp)
> +{
> +XICSState *xics = XICS_COMMON(dev);
> +XICSStateClass *xsc = XICS_COMMON_GET_CLASS(xics);
> +
> +xsc->realize(dev, errp);
> }
>
> static void xics_common_class_init(ObjectClass *oc, void *data)
> @@ -209,6 +222,7 @@ static void xics_common_class_init(ObjectClass *oc, void
> *data)
> DeviceClass *dc = DEVICE_CLASS(oc);
>
> dc->reset = xics_common_reset;
> +dc->realize = xics_common_realize;
> }
>
> static const TypeInfo xics_common_info = {
> @@ -277,7 +291,7 @@ static void icp_check_ipi(ICPState *ss)
>
> static void icp_resend(XICSState *xics, int server)
> {
> -ICPState *ss = xics->ss + server;
> +ICPState *ss = xics->ss + server * sizeof(ICPState);
> ICSState *ics;
>
> if (ss->mfrr < CPPR(ss)) {
> @@ -290,7 +304,7 @@ static void icp_resend(XICSState *xics, int server)
>
> void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
> {
> -ICPState *ss = xics->ss + server;
> +ICPState *ss = xics->ss + server * sizeof(ICPState);
> uint8_t old_cppr;
> uint32_t old_xisr;
>
> @@ -317,7 +331,7 @@ void icp_set_cppr(XICSState *xics, int server, uint8_t
> cppr)
>
> void icp_set_mfrr(XICSState *xics, int server, uint8_t mfrr)
> {
> -ICPState *ss = xics->ss + server;
> +ICPState *ss = xics->ss + server * sizeof(ICPState);
>
> ss->mfrr = mfrr;
> if (mfrr < CPPR(ss)) {
> @@ -349,7 +363,7 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
>
> void icp_eoi(XICSState *xics, int server, uint32_t xirr)
> {
> -ICPState *ss = xics->ss + server;
> +ICPState *ss = xics->ss + server * sizeof(ICPState);
> ICSState *ics;
> uint32_t irq;
>
> @@ -370,7 +384,7 @@ void icp_eoi(XICSState *xics, int server, uint32_t xirr)
> static void icp_irq(ICSState *ics, int server, int nr, uint8_t priority)
> {
> XICSState *xics = ics->xics;
> -ICPState *ss = xics->ss + server;
> +ICPState *ss = xics->ss + server * sizeof(ICPState);
>
> trace_xics_icp_irq(server, nr, priority);
>
> @@ -488,6 +502,14 @@ static const TypeInfo icp_info = {
> .class_size = sizeof(ICPStateClass),
> };
>
> +static const TypeInfo icp_native_info = {
> +.name = TYPE_NATIVE_ICP,
> +.parent = TYPE_ICP,
> +.instance_size = sizeof(ICPNative),
> +.class_init = icp_class_init,
> +.class_size = sizeof(ICPStateClass),
> +};
>
Re: [Qemu-devel] [PATCH] monitor: fix crash for platforms without a CPU 0
On Wed, Sep 21, 2016 at 10:50:49AM -0400, Luiz Capitulino wrote:
> On Wed, 21 Sep 2016 15:29:26 +1000
> David Gibson wrote:
>
> > Now that we allow CPU hot unplug on a few platforms, we can end up in a
> > situation where we don't have a CPU with index 0. Or at least we could,
> > if we didn't have code to explicitly prohibit unplug of CPU 0.
> >
> > Longer term we want to allow CPU 0 unplug, this patch is an early step in
> > allowing this, by removing an assumption in the monitor code that CPU 0
> > always exists.
> >
> > Signed-off-by: Cédric Le Goater
> > [dwg: Rewrote commit message to better explain background]
> > Signed-off-by: David Gibson
> > ---
> > monitor.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Anyone want to volunteer to take this through their tree? If not, I
> > can take it through my ppc tree.
>
> Please do.
Ok, merged to ppc-for-2.8.
>
> Reviewed-by: Luiz Capitulino
>
> >
> > diff --git a/monitor.c b/monitor.c
> > index 8bb8bbf..83c4edf 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -1025,7 +1025,7 @@ int monitor_set_cpu(int cpu_index)
> > CPUState *mon_get_cpu(void)
> > {
> > if (!cur_mon->mon_cpu) {
> > -monitor_set_cpu(0);
> > +monitor_set_cpu(first_cpu->cpu_index);
> > }
> > cpu_synchronize_state(cur_mon->mon_cpu);
> > return cur_mon->mon_cpu;
>
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v4 5/9] ppc/xics: Use a helper to add a new ICS
On Mon, Sep 19, 2016 at 11:59:33AM +0530, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt
>
> Signed-off-by: Benjamin Herrenschmidt
> [Move object allocation and adding child to the helper]
> Signed-off-by: Nikunj A Dadhania
> Reviewed-by: David Gibson
> ---
> hw/intc/xics.c| 10 ++
> hw/intc/xics_spapr.c | 6 +-
> include/hw/ppc/xics.h | 1 +
> 3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 05e938f..c7901c4 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -109,6 +109,16 @@ static void xics_common_reset(DeviceState *d)
> }
> }
>
> +void xics_add_ics(XICSState *xics)
> +{
> +ICSState *ics;
> +
> +ics = ICS(object_new(TYPE_ICS));
> +object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
You'll need to construct a name here so you don't have all the ics
objects called an indistinguishable "ics".
> +ics->xics = xics;
> +QLIST_INSERT_HEAD(&xics->ics, ics, list);
> +}
> +
> static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
>void *opaque, Error **errp)
> {
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 0b0845d..270f20e 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -305,12 +305,8 @@ static void xics_spapr_realize(DeviceState *dev, Error
> **errp)
> static void xics_spapr_initfn(Object *obj)
> {
> XICSState *xics = XICS_SPAPR(obj);
> -ICSState *ics;
>
> -ics = ICS(object_new(TYPE_ICS));
> -object_property_add_child(obj, "ics", OBJECT(ics), NULL);
> -ics->xics = xics;
> -QLIST_INSERT_HEAD(&xics->ics, ics, list);
> +xics_add_ics(xics);
> }
>
> static void xics_spapr_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index e49a2da..a7a1e54 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -197,5 +197,6 @@ void ics_write_xive(ICSState *ics, int nr, int server,
> void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>
> ICSState *xics_find_source(XICSState *icp, int irq);
> +void xics_add_ics(XICSState *xics);
>
> #endif /* XICS_H */
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH v4 7/9] ppc/xics: Add "native" XICS subclass
On Mon, Sep 19, 2016 at 11:59:35AM +0530, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt
>
> This provides MMIO based ICP access as found on POWER8
>
> Signed-off-by: Benjamin Herrenschmidt
> Signed-off-by: Nikunj A Dadhania
> ---
> default-configs/ppc64-softmmu.mak | 3 +-
> hw/intc/Makefile.objs | 1 +
> hw/intc/xics_native.c | 295
> ++
> include/hw/ppc/xics.h | 14 ++
> 4 files changed, 312 insertions(+), 1 deletion(-)
> create mode 100644 hw/intc/xics_native.c
>
> diff --git a/default-configs/ppc64-softmmu.mak
> b/default-configs/ppc64-softmmu.mak
> index c4be59f..315e30b 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -48,8 +48,9 @@ CONFIG_PLATFORM_BUS=y
> CONFIG_ETSEC=y
> CONFIG_LIBDECNUMBER=y
> # For pSeries
> -CONFIG_XICS=$(CONFIG_PSERIES)
> +CONFIG_XICS=$(or $(CONFIG_PSERIES),$(CONFIG_POWERNV))
> CONFIG_XICS_SPAPR=$(CONFIG_PSERIES)
> +CONFIG_XICS_NATIVE=$(CONFIG_POWERNV)
> CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
> # For PReP
> CONFIG_MC146818RTC=y
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 05ec21b..7be5dfc 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -31,6 +31,7 @@ obj-$(CONFIG_RASPI) += bcm2835_ic.o bcm2836_control.o
> obj-$(CONFIG_SH4) += sh_intc.o
> obj-$(CONFIG_XICS) += xics.o
> obj-$(CONFIG_XICS_SPAPR) += xics_spapr.o
> +obj-$(CONFIG_XICS_NATIVE) += xics_native.o
> obj-$(CONFIG_XICS_KVM) += xics_kvm.o
> obj-$(CONFIG_ALLWINNER_A10_PIC) += allwinner-a10-pic.o
> obj-$(CONFIG_S390_FLIC) += s390_flic.o
> diff --git a/hw/intc/xics_native.c b/hw/intc/xics_native.c
> new file mode 100644
> index 000..26e45cc
> --- /dev/null
> +++ b/hw/intc/xics_native.c
> @@ -0,0 +1,295 @@
> +/*
> + * QEMU PowerPC hardware System Emulator
> + *
> + * Native version of ICS/ICP
> + *
> + * Copyright (c) 2010,2011 David Gibson, IBM Corporation.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> deal
> + * in the Software without restriction, including without limitation the
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/hw.h"
> +#include "trace.h"
> +#include "qemu/timer.h"
> +#include "hw/ppc/xics.h"
> +#include "qapi/visitor.h"
> +#include "qapi/error.h"
> +
> +#include
> +
> +/* #define DEBUG_MM(fmt...) printf(fmt) */
> +#define DEBUG_MM(fmt...)do { } while (0)
> +
> +static void xics_native_initfn(Object *obj)
> +{
> +XICSState *xics = XICS_NATIVE(obj);
> +
> +QLIST_INIT(&xics->ics);
Isn't this redundant with the QLIST_INIT() in the common initfn?
> +}
> +
> +static uint64_t icp_mm_read(void *opaque, hwaddr addr, unsigned width)
> +{
> +XICSState *s = opaque;
> +int32_t cpu_id, server;
> +uint32_t val;
> +ICPState *ss;
> +bool byte0 = (width == 1 && (addr & 0x3) == 0);
> +
> +cpu_id = (addr & (ICP_MM_SIZE - 1)) >> 12;
> +server = get_cpu_index_by_dt_id(cpu_id);
> +if (server < 0) {
> +fprintf(stderr, "XICS: Bad ICP server %d\n", server);
Please use error_report() instead of raw fprintf()s.
> +goto bad_access;
> +}
> +ss = &s->ss[server];
> +
> +switch (addr & 0xffc) {
> +case 0: /* poll */
> +val = icp_ipoll(ss, NULL);
> +if (byte0) {
> +val >>= 24;
> +} else if (width != 4) {
> +goto bad_access;
> +}
> +break;
> +case 4: /* xirr */
> +if (byte0) {
> +val = icp_ipoll(ss, NULL) >> 24;
> +} else if (width == 4) {
> +val = icp_accept(ss);
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 12:
> +if (byte0) {
> +val = ss->mfrr;
> +} else {
> +goto bad_access;
> +}
> +break;
> +case 16:
> +if (width == 4) {
> +val = ss->links[0];
> +} else {
> +goto bad_acce
Re: [Qemu-devel] [PATCH v4 8/9] ppc/xics: Add xics to the monitor "info pic" command
On Mon, Sep 19, 2016 at 11:59:36AM +0530, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt
>
> Useful to debug interrupt problems.
>
> Signed-off-by: Benjamin Herrenschmidt
> Signed-off-by: Nikunj A Dadhania
> ---
> hmp-commands-info.hx | 2 ++
> hw/intc/xics.c| 38 ++
> hw/ppc/ppc.c | 14 ++
> include/hw/ppc/ppc.h | 1 +
> include/hw/ppc/xics.h | 4 +++-
> monitor.c | 4
> 6 files changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 74446c6..0929cb7 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -203,6 +203,8 @@ ETEXI
> .mhandler.cmd = sun4m_hmp_info_pic,
> #elif defined(TARGET_LM32)
> .mhandler.cmd = lm32_hmp_info_pic,
> +#elif defined(TARGET_PPC)
> +.mhandler.cmd = ppc_hmp_info_pic,
It's bad enough that the existing ones treat what's really a machine
type dependent thing as an arch dependent thing without adding more.
I need to dust off my patches which try to clean up the whole info pic mess.
> #else
> .mhandler.cmd = hmp_info_pic,
> #endif
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index b15751e..4ac2d00 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -36,6 +36,9 @@
> #include "hw/ppc/xics.h"
> #include "qemu/error-report.h"
> #include "qapi/visitor.h"
> +#include "monitor/monitor.h"
> +
> +static XICSState *g_xics;
>
> int xics_get_cpu_index_by_dt_id(int cpu_dt_id)
> {
> @@ -196,6 +199,9 @@ static void xics_common_initfn(Object *obj)
> object_property_add(obj, "nr_servers", "int",
> xics_prop_get_nr_servers, xics_prop_set_nr_servers,
> NULL, NULL, NULL);
> +
> +/* For exclusive use of monitor command */
> +g_xics = XICS_COMMON(obj);
I don't think you need this, you should be able to get to the xics via
qdev_get_machine().
> }
>
> static void xics_common_class_init(ObjectClass *oc, void *data)
> @@ -677,6 +683,38 @@ static int ics_simple_dispatch_post_load(void *opaque,
> int version_id)
> return 0;
> }
>
> +void xics_hmp_info_pic(Monitor *mon, const QDict *qdict)
> +{
> +ICSState *ics;
> +uint32_t i;
> +
> +for (i = 0; i < g_xics->nr_servers; i++) {
This will SEGV on machine types which aren't xics based.
> +ICPState *icp = &g_xics->ss[i];
> +
> +if (!icp->output) {
> +continue;
> +}
> +monitor_printf(mon, "CPU %d XIRR=%08x (%p) PP=%02x MFRR=%02x\n",
> + i, icp->xirr, icp->xirr_owner,
> + icp->pending_priority, icp->mfrr);
> +}
> +QLIST_FOREACH(ics, &g_xics->ics, list) {
> +monitor_printf(mon, "ICS %4x..%4x %p\n",
> + ics->offset, ics->offset + ics->nr_irqs - 1, ics);
> +for (i = 0; i < ics->nr_irqs; i++) {
> +ICSIRQState *irq = ics->irqs + i;
> +
> +if (!(irq->flags & XICS_FLAGS_IRQ_MASK)) {
> +continue;
> +}
> +monitor_printf(mon, " %4x %s %02x %02x\n",
> + ics->offset + i,
> + (irq->flags & XICS_FLAGS_IRQ_LSI) ? "LSI" : "MSI",
> + irq->priority, irq->status);
> +}
> +}
> +}
> +
> static const VMStateDescription vmstate_ics_simple_irq = {
> .name = "ics/irq",
> .version_id = 2,
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 8945869..bc73428 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -27,6 +27,7 @@
> #include "hw/hw.h"
> #include "hw/ppc/ppc.h"
> #include "hw/ppc/ppc_e500.h"
> +#include "hw/i386/pc.h"
> #include "qemu/timer.h"
> #include "sysemu/sysemu.h"
> #include "sysemu/cpus.h"
> @@ -39,6 +40,10 @@
> #include "kvm_ppc.h"
> #include "trace.h"
>
> +#if defined(TARGET_PPC64)
> +#include "hw/ppc/xics.h"
> +#endif
> +
> //#define PPC_DEBUG_IRQ
> //#define PPC_DEBUG_TB
>
> @@ -1376,3 +1381,12 @@ void ppc_cpu_parse_features(const char *cpu_model)
> cc->parse_features(typename, model_pieces[1], &error_fatal);
> g_strfreev(model_pieces);
> }
> +
> +void ppc_hmp_info_pic(Monitor *mon, const QDict *qdict)
> +{
> +/* Call in turn every PIC around. OpenPIC doesn't have one yet */
> +#ifdef TARGET_PPC64
> +xics_hmp_info_pic(mon, qdict);
> +#endif
> +hmp_info_pic(mon, qdict);
> +}
> diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
> index 00c1fb1..69c4b19 100644
> --- a/include/hw/ppc/ppc.h
> +++ b/include/hw/ppc/ppc.h
> @@ -3,6 +3,7 @@
>
> #include "target-ppc/cpu-qom.h"
>
> +void ppc_hmp_info_pic(Monitor *mon, const QDict *qdict);
> void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level);
>
> /* PowerPC hardware exceptions management helpers */
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index f81155c..58bb7c6 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -220,4 +220,6 @
Re: [Qemu-devel] [PATCH] Add resolutions via the command-line
On Wed, 2016-09-21 at 19:18 -0400, G 3 wrote: > > That can be done, but I was hoping to be able to do this via a > command-line switch. But you still do that ! Your switch puts stuff in /options which OpenBIOS then picks up to construct the final list that it puts in the driver node. > That would make things so much easier on the user. Altering > properties in OpenBIOS can be challenging using forth. OpenBIOS does > support using > startup > scripts so maybe someone could make a script that adds user > resolutions to the > QEMU,VGA node from the options node. > > I'm hoping you had a look at version three of my patch. If there > are > any other issues > with it, I would like to know before making version 4. I'm really not fan of the whole ASCII parsing business. In any case, I'm a bit too busy right now, I'll try to get back onto this next week.
Re: [Qemu-devel] [PATCH] net: hmp_host_net_remove: Del the -net option of the removed host_net
On 2016年09月05日 17:11, Shmulik Ladkani wrote:
Upon hmp_host_net_remove(), the appropriate -net client is deleted
(according to the given vlan_id and device id), as well as the
corresponsing hub port.
However, the relevant '-net' option that was added by former
hmp_host_net_add() call is still present in "net" options group.
This makes the following legit HMP sequence erroneous:
(qemu) host_net_add tap id=n1,ifname=tap1,script=no,downscript=no,vlan=1
(qemu) host_net_remove 1 n1
(qemu) host_net_add tap id=n1,ifname=tap1,script=no,downscript=no,vlan=1
Duplicate ID 'n1' for net
Fix, by deleting the stored '-net' option associated with the given
device id.
Signed-off-by: Shmulik Ladkani
---
net/net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/net.c b/net/net.c
index d51cb29882..0bec096d75 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1179,6 +1179,7 @@ void hmp_host_net_remove(Monitor *mon, const QDict *qdict)
qemu_del_net_client(nc->peer);
qemu_del_net_client(nc);
+qemu_opts_del(qemu_opts_find(qemu_find_opts("net"), device));
}
void netdev_add(QemuOpts *opts, Error **errp)
Applied, thanks.
Re: [Qemu-devel] [PATCH] Add resolutions via the command-line
On Sep 21, 2016, at 5:53 PM, Benjamin Herrenschmidt wrote: On Wed, 2016-09-21 at 14:26 -0400, G 3 wrote: Nodes like chose, aliases, openprom are of class IOService. options is of class IODTNVRAM. It looks like this class has problems. I'm thinking since Alexander Graf did work in the mac_nvram.c file, he might know what is wrong. What is wrong is purely that MacOS X limits how you do things on that node. Have a look at the OS X kernel source :-) The implementation of the support libraries to run NDRVs is all there (module IOGraphicsFamily iirc). I will check the IOGraphicsFamily kernel extension source code. I found another way to access the options node in Mac OS X. Use this command: ioreg -c IODTNVRAM. It stops at the options node and prints this error message: "ioreg: error: can't obtain properties" in QEMU. On a real Power Mac the command prints all the properties of the options node just fine. Correct, part of the problem is that I think we don't emulate the NVRAM in a format that OS X understands, a problem for another day. If you know any more info please share. I would like to help fix this problem. I don't know about using binary. The way we do it now seems just fine. ASCII parsing in a driver sucks. All the code needed parse ASCII is already in the patch. But I will try to see things your way. I just need to figure out how to write to the /chosen node from the QEMU command line. I found a function called OpenBIOS_set_var() in openbios_firmware_abi.h, but it would require an address to write to. So where do I find the address of the /chosen node No, you don't. Do as I said. Have OpenBIOS construct the list of resolutions and put it in the node of the device itself. That can be done, but I was hoping to be able to do this via a command-line switch. That would make things so much easier on the user. Altering properties in OpenBIOS can be challenging using forth. OpenBIOS does support using startup scripts so maybe someone could make a script that adds user resolutions to the QEMU,VGA node from the options node. I'm hoping you had a look at version three of my patch. If there are any other issues with it, I would like to know before making version 4.
Re: [Qemu-devel] [PATCH v6 RFC] block/vxhs: Initial commit to add Veritas HyperScale VxHS block device support
libqnio source has been updated. qemu changes should now build properly.
https://github.com/MittalAshish/libqnio.git
I will work on the other suggestions and get back.
Regards,
Ashish
On Wed, Sep 21, 2016 at 8:03 AM, Paolo Bonzini wrote:
>
>
> On 21/09/2016 03:07, Ashish Mittal wrote:
>> +int32_t vxhs_qnio_iio_writev(void *qnio_ctx, uint32_t rfd, struct iovec
>> *iov,
>> +int iovcnt, uint64_t offset,
>> +void *ctx, uint32_t flags);
>> +int32_t vxhs_qnio_iio_readv(void *qnio_ctx, uint32_t rfd, struct iovec *iov,
>> +int iovcnt, uint64_t offset,
>> +void *ctx, uint32_t flags);
>> +int32_t vxhs_qnio_iio_ioctl(void *apictx, uint32_t rfd, uint32_t opcode,
>> +int64_t *in, void *ctx,
>> +uint32_t flags);
>
> Since you have wrappers for this, please use less verbose arguments, such as
>
> - BDRVVXHSState *s instead of the void * (qnio_ctx = s->qnio_ctx)
>
> - int idx instead of uint32_t rfd (rfd = s->vdisk_hostinfo[idx].vdisk_rfd)
>
> - the QEMUIOVector * instead of the iov/iovcnt pair
>
> Likewise I suggest adding a wrapper
>
> void vxhs_qnio_iio_close(BDRVVXHSState *s, int idx)
> {
> if (s->vdisk_hostinfo[idx].vdisk_rfd >= 0) {
> iio_devclose(s->qnio_ctx, 0, s->vdisk_hostinfo[i].vdisk_rfd);
> s->vdisk_hostinfo[i].vdisk_rfd = -1;
> }
>
>
> if (s->vdisk_hostinfo[i].qnio_cfd >= 0) {
> iio_close(s->qnio_ctx, s->vdisk_hostinfo[i].qnio_cfd);
> s->vdisk_hostinfo[i].qnio_cfd = -1;
> }
> }
>
> (Likewise, iio_open/iio_devopen always happen in pairs and always build the
> openflame URI, so that's another candidate for a wrapper function).
>
> Also on the topic of closing:
>
> - there's no loop that initializes vdisk_rfd's and qnio_cfd's to -1.
>
> - here you are closing a vdisk_rfd twice:
>
> +if (s->vdisk_hostinfo[s->vdisk_cur_host_idx].vdisk_rfd >= 0) {
> +iio_devclose(s->qnio_ctx, 0,
> +s->vdisk_hostinfo[s->vdisk_cur_host_idx].vdisk_rfd);
> +}
>
> because later you have another call to iio_devclose within
> "for (i = 0; i < VXHS_MAX_HOSTS; i++) {". (It's also the only place
> that calls iio_devclose and not iio_close).
>
>>
>> +if (s->vdisk_hostinfo[index].qnio_cfd < 0) {
>> +s->vdisk_hostinfo[index].qnio_cfd =
>> +iio_open(global_qnio_ctx, of_vsa_addr, 0);
>
> s->qnio_ctx seems to be always equal to global_qnio_ctx. If that's
> how the API works that's fine, however please use s->qnio_ctx consistently.
> Initialize it early.
>
>> + * Return Value:
>> + * On Success : return VXHS_VECTOR_ALIGNED
>> + * On Failure : return VXHS_VECTOR_NOT_ALIGNED.
>> + */
>> +int vxhs_is_iovector_read_aligned(struct iovec *iov, int niov, size_t
>> sector)
>
> Pass a QEMUIOVector here too.
>
>> +{
>> +int i;
>> +
>> +if (!iov || niov == 0) {
>> +return VXHS_VECTOR_ALIGNED;
>> +}
>
> Unnecessary "if". The loop below never rolls if niov == 0, and you should
> never have "!iov && niov > 0".
>
>> +for (i = 0; i < niov; i++) {
>> +if (iov[i].iov_len % sector != 0) {
>> +return VXHS_VECTOR_NOT_ALIGNED;
>> +}
>> +}
>> +return VXHS_VECTOR_ALIGNED;
>> +}
>
> Please return just true or false.
>
>> +void *vxhs_convert_iovector_to_buffer(struct iovec *iov, int niov,
>> + size_t sector)
>> +{
>> +void *buf = NULL;
>> +size_t size = 0;
>> +
>> +if (!iov || niov == 0) {
>> +return buf;
>> +}
>> +
>> +size = vxhs_calculate_iovec_size(iov, niov);
>
> If you have the QEMUIOVector, vxhs_calculate_iovec_size is just qiov->size.
>
>> +buf = qemu_memalign(sector, size);
>> +if (!buf) {
>> +trace_vxhs_convert_iovector_to_buffer(size);
>> +errno = -ENOMEM;
>> +return NULL;
>> +}
>> +return buf;
>> +}
>> +
>
> This function should use qemu_try_memalign, not qemu_memalign. But it is
> obviously not very well tested, because the !iov || niov == 0 case doesn't
> set errno and returns NULL.
>
> You should just use qemu_try_memalign(qiov->size, BDRV_SECTOR_SIZE) in the
> caller.
>
> However, why is alignment check necessary for read but not for write?
>
>>
>> +if (ret == -1) {
>> +trace_vxhs_qnio_iio_writev_err(i, iov[i].iov_len,
>> errno);
>> +/*
>> + * Need to adjust the AIOCB segment count to prevent
>> + * blocking of AIOCB completion within QEMU block
>> driver.
>> + */
>> +if (segcount > 0 && (segcount - nsio) > 0) {
>> +vxhs_dec_acb_segment_count(ctx, segcount - nsio);
>> +}
>> +return ret;
>
> Here you return, so it is
Re: [Qemu-devel] [PATCH 16/16] cpus-common: lock-free fast path for cpu_exec_start/end
On Mon, Sep 19, 2016 at 14:50:59 +0200, Paolo Bonzini wrote:
(snip)
> @@ -212,24 +216,75 @@ void end_exclusive(void)
> /* Wait for exclusive ops to finish, and begin cpu execution. */
> void cpu_exec_start(CPUState *cpu)
> {
> -qemu_mutex_lock(&qemu_cpu_list_mutex);
> -exclusive_idle();
> cpu->running = true;
> -qemu_mutex_unlock(&qemu_cpu_list_mutex);
> +
> +/* Write cpu->running before reading pending_cpus. */
> +smp_mb();
> +
> +/* 1. start_exclusive saw cpu->running == true and pending_cpus >= 1.
> + * After taking the lock we'll see cpu->has_waiter == true and run---not
> + * for long because start_exclusive kicked us. cpu_exec_end will
> + * decrement pending_cpus and signal the waiter.
> + *
> + * 2. start_exclusive saw cpu->running == false but pending_cpus >= 1.
> + * This includes the case when an exclusive item is running now.
> + * Then we'll see cpu->has_waiter == false and wait for the item to
> + * complete.
> + *
> + * 3. pending_cpus == 0. Then start_exclusive is definitely going to
> + * see cpu->running == true, and it will kick the CPU.
> + */
> +if (pending_cpus) {
I'd consider doing
if (unlikely(pending_cpus)) {
since the exclusive is a slow path and will be more so in the near future.
> +qemu_mutex_lock(&qemu_cpu_list_mutex);
> +if (!cpu->has_waiter) {
> +/* Not counted in pending_cpus, let the exclusive item
> + * run. Since we have the lock, set cpu->running to true
> + * while holding it instead of retrying.
> + */
> +cpu->running = false;
> +exclusive_idle();
> +/* Now pending_cpus is zero. */
> +cpu->running = true;
> +} else {
> +/* Counted in pending_cpus, go ahead. */
> +}
> +qemu_mutex_unlock(&qemu_cpu_list_mutex);
> +}
> }
>
> /* Mark cpu as not executing, and release pending exclusive ops. */
> void cpu_exec_end(CPUState *cpu)
> {
> -qemu_mutex_lock(&qemu_cpu_list_mutex);
> cpu->running = false;
> -if (pending_cpus > 1) {
> -pending_cpus--;
> -if (pending_cpus == 1) {
> -qemu_cond_signal(&exclusive_cond);
> +
> +/* Write cpu->running before reading pending_cpus. */
> +smp_mb();
> +
> +/* 1. start_exclusive saw cpu->running == true. Then it will increment
> + * pending_cpus and wait for exclusive_cond. After taking the lock
> + * we'll see cpu->has_waiter == true.
> + *
> + * 2. start_exclusive saw cpu->running == false but here pending_cpus >=
> 1.
> + * This includes the case when an exclusive item started after setting
> + * cpu->running to false and before we read pending_cpus. Then we'll see
> + * cpu->has_waiter == false and not touch pending_cpus. The next call to
> + * cpu_exec_start will run exclusive_idle if still necessary, thus
> waiting
> + * for the item to complete.
> + *
> + * 3. pending_cpus == 0. Then start_exclusive is definitely going to
> + * see cpu->running == false, and it can ignore this CPU until the
> + * next cpu_exec_start.
> + */
> +if (pending_cpus) {
ditto
> +qemu_mutex_lock(&qemu_cpu_list_mutex);
> +if (cpu->has_waiter) {
> +cpu->has_waiter = false;
> +if (--pending_cpus == 1) {
> +qemu_cond_signal(&exclusive_cond);
> +}
(snip)
Another suggestion is to consistently access pending_cpus atomically,
since now we're accessing it with and without the CPU list mutex held.
Thanks,
Emilio
Re: [Qemu-devel] [PATCH 16/16] cpus-common: lock-free fast path for cpu_exec_start/end
On Wed, Sep 21, 2016 at 20:19:18 +0200, Paolo Bonzini wrote:
(snip)
> No, this is not true. Barriers order stores and loads within a thread
> _and_ establish synchronizes-with edges.
>
> In the example above you are violating causality:
>
> - cpu0 stores cpu->running before loading pending_cpus
>
> - because pending_cpus == 0, cpu1 stores pending_cpus = 1 after cpu0
> loads it
>
> - cpu1 loads cpu->running after it stores pending_cpus
OK. So I simplified the example to understand this better:
cpu0cpu1
{ A = B = 0, r0 and r1 are private variables }
x = 1 y = 1
smp_mb()smp_mb()
r0 = y r1 = x
Turns out this is scenario 10 here: https://lwn.net/Articles/573436/
The source of my confusion was not paying due attention to smp_mb,
which is necessary for maintaining transitivity.
> > Is there a performance (scalability) reason behind this patch?
>
> Yes: it speeds up all cpu_exec_start/end, _not_ start/end_exclusive.
>
> With this patch, as long as there are no start/end_exclusive (which are
> supposed to be rare) there is no contention on multiple CPUs doing
> cpu_exec_start/end.
>
> Without it, as CPUs increase, the global cpu_list_mutex is going to
> become a bottleneck.
I see. Scalability-wise I wouldn't expect much improvement with MTTCG
full-system, given that the iothread lock is still acquired on every
CPU loop exit (just like in KVM). However, for user-mode this should
yield measurable improvements =D
Thanks,
E.
Re: [Qemu-devel] [PATCH] Add resolutions via the command-line
On Wed, 2016-09-21 at 14:26 -0400, G 3 wrote: > > Nodes like chose, aliases, openprom are of class IOService. options > is of class IODTNVRAM. It looks like this class has problems. I'm > thinking since Alexander Graf did work in the mac_nvram.c file, he > might know what is wrong. What is wrong is purely that MacOS X limits how you do things on that node. Have a look at the OS X kernel source :-) The implementation of the support libraries to run NDRVs is all there (module IOGraphicsFamily iirc). > I found another way to access the options > node in Mac OS X. Use this command: ioreg -c IODTNVRAM. It stops at > the options node and prints this error message: "ioreg: error: can't > obtain properties" in QEMU. On a real Power Mac the command prints > all the properties of the options node just fine. Correct, part of the problem is that I think we don't emulate the NVRAM in a format that OS X understands, a problem for another day. > I don't know about using binary. The way we do it now seems just fine. ASCII parsing in a driver sucks. > I just need to figure out how to write to the /chosen node from the > QEMU command line. I found a function called OpenBIOS_set_var() in > openbios_firmware_abi.h, but it would require an address to write to. > So where do I find the address of the /chosen node No, you don't. Do as I said. Have OpenBIOS construct the list of resolutions and put it in the node of the device itself. Cheers, Ben.
Re: [Qemu-devel] [PATCH v2 4/4] iotests: fix expected error message
- Original Message - > On 09/21/2016 03:10 PM, Marc-André Lureau wrote: > > Missing argument returns a corresponding error message for all types > > now. > > > > The "old" qmp dispatch code didn't return QERR_MISSING_PARAMETER for > > argument structure members (it returned QERR_INVALID_PARAMETER). Only > > for top-level argument it did return QERR_MISSING_PARAMETER. It is > > preferable to have a consistent error for missing fields in inner > > structs as well. > > > > Signed-off-by: Marc-André Lureau > > Reviewed-by: Alberto Garcia > > --- > > tests/qemu-iotests/087.out | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Yes, this looks better. > > If the message is changed earlier in this series, then this should be > squashed into that patch (3/4?); if the message changed as the result of > an earlier commit, the commit message should call out the id at which > the tree was temporarily broken. I checked, it's the previous patch that introduce the change (I wasn't sure given that in alternate case qapi already returned MISSING_PARAMETER), it can be squashed with it.
Re: [Qemu-devel] [PATCH v2 3/4] qapi: return a 'missing parameter' error
On 09/21/2016 03:10 PM, Marc-André Lureau wrote: > The 'old' dispatch code returned a QERR_MISSING_PARAMETER for missing > parameters, but the qapi qmp_dispatch() code uses > QERR_INVALID_PARAMETER_TYPE. > > Improve qapi code to return QERR_INVALID_PARAMETER_TYPE where > appropriate. > > Signed-off-by: Marc-André Lureau > Reviewed-by: Alberto Garcia > --- > qapi/qmp-input-visitor.c | 73 > +--- > 1 file changed, 56 insertions(+), 17 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 4/4] iotests: fix expected error message
On 09/21/2016 03:10 PM, Marc-André Lureau wrote: > Missing argument returns a corresponding error message for all types > now. > > The "old" qmp dispatch code didn't return QERR_MISSING_PARAMETER for > argument structure members (it returned QERR_INVALID_PARAMETER). Only > for top-level argument it did return QERR_MISSING_PARAMETER. It is > preferable to have a consistent error for missing fields in inner > structs as well. > > Signed-off-by: Marc-André Lureau > Reviewed-by: Alberto Garcia > --- > tests/qemu-iotests/087.out | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Yes, this looks better. If the message is changed earlier in this series, then this should be squashed into that patch (3/4?); if the message changed as the result of an earlier commit, the commit message should call out the id at which the tree was temporarily broken. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] Fix tlb_vaddr_to_host with CONFIG_USER_ONLY
We use the wrong argument name for the g2h() macro !
The result ends up being something like (target_ulong)(uint64) + guest_base
which is obviously wrong.
Signed-off-by: Benjamin Herrenschmidt
---
include/exec/cpu_ldst.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index b573df5..6eb5fe8 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -401,7 +401,7 @@ static inline void *tlb_vaddr_to_host(CPUArchState *env,
target_ulong addr,
int access_type, int mmu_idx)
{
#if defined(CONFIG_USER_ONLY)
-return g2h(vaddr);
+return g2h(addr);
#else
int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
CPUTLBEntry *tlbentry = &env->tlb_table[mmu_idx][index];
Re: [Qemu-devel] [PATCH v2 2/4] qapi: assert list entry has a value
On 09/21/2016 03:10 PM, Marc-André Lureau wrote:
> This helps to figure out the expectations.
>
> Signed-off-by: Marc-André Lureau
> ---
> qapi/qmp-input-visitor.c | 1 +
> 1 file changed, 1 insertion(+)
Reviewed-by: Eric Blake
>
> diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
> index be52aaf..c1019d6 100644
> --- a/qapi/qmp-input-visitor.c
> +++ b/qapi/qmp-input-visitor.c
> @@ -84,6 +84,7 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
> assert(qobject_type(qobj) == QTYPE_QLIST);
> assert(!name);
> ret = qlist_entry_obj(tos->entry);
> +assert(ret);
> if (consume) {
> tos->entry = qlist_next(tos->entry);
> }
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2 1/4] qapi: add assert about root value
On 09/21/2016 03:10 PM, Marc-André Lureau wrote: > qiv->root should not be null, make that clearer with some assert. > > Signed-off-by: Marc-André Lureau > --- > qapi/qmp-input-visitor.c | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 5/6] target-i386: Automatically set level/xlevel/xlevel2 when needed
On 09/21/2016 01:14 PM, Eduardo Habkost wrote:
On Wed, Sep 21, 2016 at 12:53:08PM -0700, Richard Henderson wrote:
On 09/21/2016 11:26 AM, Eduardo Habkost wrote:
+/* Set cpuid_*level* based on cpuid_min_*level, if not explicitly set */
+if (!env->cpuid_level) {
+env->cpuid_level = env->cpuid_min_level;
+}
+if (!env->cpuid_xlevel) {
+env->cpuid_xlevel = env->cpuid_min_xlevel;
+}
+if (!env->cpuid_xlevel2) {
+env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
}
Why are we not bounding them by MIN, if it's really a minimum?
Not sure I understand what you mean. Do you mean silently
changing the value even if it was explicitly set by the user?
You're changing it if the user explicitly set the level to 0, aren't you?
Or is that merely an oversight and you really need the levels defaulted to some
magic value like -1?
+/* Enable auto level-increase for CPUID[7].ECX features */
+bool cpuid_auto_level_7_0_ecx;
+
+/* Enable auto level-increase for CPUID[6] features */
+bool cpuid_auto_level_6;
Why two variables? Seems like only one is needed for backward
compatibility.
It's true that we don't really need two variables to implement
pc-2.7 compatibility. I just implemented it this way because
having two separate variables looks clearer to me. I wouldn't
know how I would name the variable if it controlled both
CPUID[7].ECX and CPUID[6] at the same time (any suggestions?).
cpuid_auto_level_compat(_27)?
r~
Re: [Qemu-devel] [PATCH] dma: xlnx-zynq-devcfg: Fix up XLNX_ZYNQ_DEVCFG_R_MAX
On Wed, Sep 21, 2016 at 11:09 AM, Nathan Rossi wrote: > Whilst according to the Zynq TRM this device covers a register region of > 0x000 - 0x120. The register region is also shared with XADCIF prefix > registers at 0x100 and above. Due to how the devcfg and the xadc devices > are implemented in QEMU these are separate models with individual mmio > regions. As such the region registered by the devcfg overlaps with the > xadc when initialized in a machine model (e.g. xilinx-zynq-a9). > > This patch fixes up the incorrect region size, where > XLNX_ZYNQ_DEVCFG_R_MAX is missing its '/ 4' causing it to be 0x460 in > size. As well as setting the region size to the 0x0 - 0x100 region so > that an xadc device instance can be registered in the correct region to > pair with the devcfg device instance. > > Mapping with XLNX_ZYNQ_DEVCFG_R_MAX = 0x118: > dev: xlnx.ps7-dev-cfg, id "" > mmio f8007000/0460 > dev: xlnx,zynq-xadc, id "" > mmio f8007100/0020 > > Mapping with XLNX_ZYNQ_DEVCFG_R_MAX = 0x100 / 4: > dev: xlnx.ps7-dev-cfg, id "" > mmio f8007000/0100 > dev: xlnx,zynq-xadc, id "" > mmio f8007100/0020 > > Signed-off-by: Nathan Rossi Good catch. What came up that caused you to find this? Can this go via the target-arm queue Peter? Reviewed-by: Alistair Francis Thanks, Alistair > --- > include/hw/dma/xlnx-zynq-devcfg.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/hw/dma/xlnx-zynq-devcfg.h > b/include/hw/dma/xlnx-zynq-devcfg.h > index d40e5c8df6..9f5119a89a 100644 > --- a/include/hw/dma/xlnx-zynq-devcfg.h > +++ b/include/hw/dma/xlnx-zynq-devcfg.h > @@ -34,7 +34,7 @@ > #define XLNX_ZYNQ_DEVCFG(obj) \ > OBJECT_CHECK(XlnxZynqDevcfg, (obj), TYPE_XLNX_ZYNQ_DEVCFG) > > -#define XLNX_ZYNQ_DEVCFG_R_MAX 0x118 > +#define XLNX_ZYNQ_DEVCFG_R_MAX (0x100 / 4) > > #define XLNX_ZYNQ_DEVCFG_DMA_CMD_FIFO_LEN 10 > > -- > 2.9.3 >
Re: [Qemu-devel] Default CPU for NMI injection (QMP and IPMI)
On 09/21/2016 03:20 PM, Eduardo Habkost wrote: Hi, I was looking at the monitor code handling the "current CPU", and noticed that qmp_inject_nmi() looks suspicious: it is a QMP command, but uses monitor_get_cpu_index(). In addition to the "inject-nmi" QMP command, qmp_inject_nmi() is used at: * hmp_inject_nmi() * ipmi_do_hw_op() (IPMI_SEND_NMI operation) This confused me, so I would like to know: 1) What exactly "default CPU" is supposed to mean in the "inject-nmi" QMP command documentation? 2) To which CPU(s) are NMIs supposed to be sent when triggered by IPMI messages? I don't know how to test the IPMI code, but it looks like it will crash if QEMU runs without any monitor. It doesn't matter which CPU it goes to. I haven't tested without a monitor, so I'm not sure. Does another interface into the NMI code need to be added? -corey
[Qemu-devel] Default CPU for NMI injection (QMP and IPMI)
Hi, I was looking at the monitor code handling the "current CPU", and noticed that qmp_inject_nmi() looks suspicious: it is a QMP command, but uses monitor_get_cpu_index(). In addition to the "inject-nmi" QMP command, qmp_inject_nmi() is used at: * hmp_inject_nmi() * ipmi_do_hw_op() (IPMI_SEND_NMI operation) This confused me, so I would like to know: 1) What exactly "default CPU" is supposed to mean in the "inject-nmi" QMP command documentation? 2) To which CPU(s) are NMIs supposed to be sent when triggered by IPMI messages? I don't know how to test the IPMI code, but it looks like it will crash if QEMU runs without any monitor. -- Eduardo
Re: [Qemu-devel] [PATCH 2/2] target-mips: reimplement SC instruction and use cmpxchg
On 09/21/2016 01:07 AM, Leon Alrae wrote: +tcg_gen_brcond_tl(TCG_COND_EQ, addr, cpu_lladdr, l1); +tcg_temp_free(addr); +tcg_gen_movi_tl(t0, 0); +tcg_gen_br(done); + +gen_set_label(l1); +/* generate cmpxchg */ +val = tcg_temp_new(); +gen_load_gpr(val, rt); +tcg_gen_atomic_cmpxchg_tl(t0, cpu_lladdr, cpu_llval, val, + ctx->mem_idx, tcg_mo); +tcg_gen_setcond_tl(TCG_COND_EQ, t0, t0, cpu_llval); +tcg_temp_free(val); + +gen_set_label(done); +/* store the result into the register */ +gen_store_gpr(t0, rt); tcg_temp_free(t0); The only thing I would change is to duplicate the gen_store_gpr into both branches, so that we don't have to store t0 into the stack across the blocks. Otherwise, Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH 5/6] target-i386: Automatically set level/xlevel/xlevel2 when needed
On Wed, Sep 21, 2016 at 12:53:08PM -0700, Richard Henderson wrote:
> On 09/21/2016 11:26 AM, Eduardo Habkost wrote:
> > +/* Set cpuid_*level* based on cpuid_min_*level, if not explicitly set
> > */
> > +if (!env->cpuid_level) {
> > +env->cpuid_level = env->cpuid_min_level;
> > +}
> > +if (!env->cpuid_xlevel) {
> > +env->cpuid_xlevel = env->cpuid_min_xlevel;
> > +}
> > +if (!env->cpuid_xlevel2) {
> > +env->cpuid_xlevel2 = env->cpuid_min_xlevel2;
> > }
>
> Why are we not bounding them by MIN, if it's really a minimum?
Not sure I understand what you mean. Do you mean silently
changing the value even if it was explicitly set by the user?
In those cases, I don't see what's the benefit of doing something
different from what the user explicitly asked for (considering
that it would require adding more compat code to keep the pc-2.7
behavior).
>
> > +/* Enable auto level-increase for CPUID[7].ECX features */
> > +bool cpuid_auto_level_7_0_ecx;
> > +
> > +/* Enable auto level-increase for CPUID[6] features */
> > +bool cpuid_auto_level_6;
>
> Why two variables? Seems like only one is needed for backward
> compatibility.
It's true that we don't really need two variables to implement
pc-2.7 compatibility. I just implemented it this way because
having two separate variables looks clearer to me. I wouldn't
know how I would name the variable if it controlled both
CPUID[7].ECX and CPUID[6] at the same time (any suggestions?).
> You're not considering adding a new one when cpuid[8].ebx is
> invented are you?
I'm not. Those were added only to allow us to implement pc-2.7
compatibility.
--
Eduardo
[Qemu-devel] [PATCH v2 4/4] iotests: fix expected error message
Missing argument returns a corresponding error message for all types
now.
The "old" qmp dispatch code didn't return QERR_MISSING_PARAMETER for
argument structure members (it returned QERR_INVALID_PARAMETER). Only
for top-level argument it did return QERR_MISSING_PARAMETER. It is
preferable to have a consistent error for missing fields in inner
structs as well.
Signed-off-by: Marc-André Lureau
Reviewed-by: Alberto Garcia
---
tests/qemu-iotests/087.out | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index a95c4b0..b213db2 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -60,7 +60,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
encryption=on
Testing: -S
QMP_VERSION
{"return": {}}
-{"error": {"class": "GenericError", "desc": "Invalid parameter type for
'driver', expected: string"}}
+{"error": {"class": "GenericError", "desc": "Parameter 'driver' is missing"}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event":
"SHUTDOWN"}
--
2.10.0
[Qemu-devel] [PATCH v2 3/4] qapi: return a 'missing parameter' error
The 'old' dispatch code returned a QERR_MISSING_PARAMETER for missing
parameters, but the qapi qmp_dispatch() code uses
QERR_INVALID_PARAMETER_TYPE.
Improve qapi code to return QERR_INVALID_PARAMETER_TYPE where
appropriate.
Signed-off-by: Marc-André Lureau
Reviewed-by: Alberto Garcia
---
qapi/qmp-input-visitor.c | 73 +---
1 file changed, 56 insertions(+), 17 deletions(-)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index c1019d6..6f85664 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -56,7 +56,7 @@ static QmpInputVisitor *to_qiv(Visitor *v)
static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
const char *name,
- bool consume)
+ bool consume, Error **errp)
{
StackObject *tos;
QObject *qobj;
@@ -80,6 +80,9 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
bool removed = g_hash_table_remove(tos->h, name);
assert(removed);
}
+if (!ret) {
+error_setg(errp, QERR_MISSING_PARAMETER, name);
+}
} else {
assert(qobject_type(qobj) == QTYPE_QLIST);
assert(!name);
@@ -165,13 +168,16 @@ static void qmp_input_start_struct(Visitor *v, const char
*name, void **obj,
size_t size, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
-QObject *qobj = qmp_input_get_object(qiv, name, true);
+QObject *qobj = qmp_input_get_object(qiv, name, true, errp);
Error *err = NULL;
if (obj) {
*obj = NULL;
}
-if (!qobj || qobject_type(qobj) != QTYPE_QDICT) {
+if (!qobj) {
+return;
+}
+if (qobject_type(qobj) != QTYPE_QDICT) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"QDict");
return;
@@ -193,10 +199,13 @@ static void qmp_input_start_list(Visitor *v, const char
*name,
GenericList **list, size_t size, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
-QObject *qobj = qmp_input_get_object(qiv, name, true);
+QObject *qobj = qmp_input_get_object(qiv, name, true, errp);
const QListEntry *entry;
-if (!qobj || qobject_type(qobj) != QTYPE_QLIST) {
+if (!qobj) {
+return;
+}
+if (qobject_type(qobj) != QTYPE_QLIST) {
if (list) {
*list = NULL;
}
@@ -234,11 +243,10 @@ static void qmp_input_start_alternate(Visitor *v, const
char *name,
bool promote_int, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
-QObject *qobj = qmp_input_get_object(qiv, name, false);
+QObject *qobj = qmp_input_get_object(qiv, name, false, errp);
+*obj = NULL;
if (!qobj) {
-*obj = NULL;
-error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
return;
}
*obj = g_malloc0(size);
@@ -252,8 +260,13 @@ static void qmp_input_type_int64(Visitor *v, const char
*name, int64_t *obj,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
-QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
+QObject *qobj = qmp_input_get_object(qiv, name, true, errp);
+QInt *qint;
+if (!qobj) {
+return;
+}
+qint = qobject_to_qint(qobj);
if (!qint) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"integer");
@@ -268,8 +281,13 @@ static void qmp_input_type_uint64(Visitor *v, const char
*name, uint64_t *obj,
{
/* FIXME: qobject_to_qint mishandles values over INT64_MAX */
QmpInputVisitor *qiv = to_qiv(v);
-QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true));
+QObject *qobj = qmp_input_get_object(qiv, name, true, errp);
+QInt *qint;
+if (!qobj) {
+return;
+}
+qint = qobject_to_qint(qobj);
if (!qint) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"integer");
@@ -283,8 +301,13 @@ static void qmp_input_type_bool(Visitor *v, const char
*name, bool *obj,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
-QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true));
+QObject *qobj = qmp_input_get_object(qiv, name, true, errp);
+QBool *qbool;
+if (!qobj) {
+return;
+}
+qbool = qobject_to_qbool(qobj);
if (!qbool) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
"boolean");
@@ -298,10 +321,15 @@ static void qmp_input_type_str(Visitor *v, const char
*name, char **obj,
Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
-QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true));
+QObje
[Qemu-devel] [PATCH v2 2/4] qapi: assert list entry has a value
This helps to figure out the expectations.
Signed-off-by: Marc-André Lureau
---
qapi/qmp-input-visitor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index be52aaf..c1019d6 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -84,6 +84,7 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
assert(qobject_type(qobj) == QTYPE_QLIST);
assert(!name);
ret = qlist_entry_obj(tos->entry);
+assert(ret);
if (consume) {
tos->entry = qlist_next(tos->entry);
}
--
2.10.0
[Qemu-devel] [PATCH] timer: a9gtimer: check auto-increment register value
From: Prasad J Pandit
ARM A9MP processor has a peripheral timer with an auto-increment
register, which holds an increment step value. A user could set
this value to zero, when auto-increment control bit is enabled.
This leads to an infinite loop in 'a9_gtimer_update' while
updating comparator value. Add check to avoid it.
Reported-by: Li Qiang
Signed-off-by: Prasad J Pandit
---
hw/timer/a9gtimer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/timer/a9gtimer.c b/hw/timer/a9gtimer.c
index 772f85f..3f752ce 100644
--- a/hw/timer/a9gtimer.c
+++ b/hw/timer/a9gtimer.c
@@ -85,7 +85,7 @@ static void a9_gtimer_update(A9GTimerState *s, bool sync)
while (gtb->compare < update.new) {
DB_PRINT("Compare event happened for CPU %d\n", i);
gtb->status = 1;
-if (gtb->control & R_CONTROL_AUTO_INCREMENT) {
+if (gtb->inc && gtb->control & R_CONTROL_AUTO_INCREMENT) {
DB_PRINT("Auto incrementing timer compare by %" PRId32
"\n",
gtb->inc);
gtb->compare += gtb->inc;
--
2.5.5
[Qemu-devel] [PATCH v2 0/4] qapi: return 'missing parameter' error
Hi, 'monitor: use qmp_dispatch()' patch broke some iotests expecting a 'missing parameter' error. This series fixes qapi visitors to return this error for all types. v2: - add some assert() for expected values in qmp-input-visitor, making it more explicit when qmp_input_get_object() may return NULL - squash the *obj = NULL changes with the missing arg error patch - move qobject_to_*(qobj) calls after checking qobj != NULL - update commit messages Marc-André Lureau (4): qapi: add assert about root value qapi: assert list entry has a value qapi: return a 'missing parameter' error iotests: fix expected error message qapi/qmp-input-visitor.c | 76 +++--- tests/qemu-iotests/087.out | 2 +- 2 files changed, 60 insertions(+), 18 deletions(-) -- 2.10.0
[Qemu-devel] [PATCH v2 1/4] qapi: add assert about root value
qiv->root should not be null, make that clearer with some assert.
Signed-off-by: Marc-André Lureau
---
qapi/qmp-input-visitor.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 64dd392..be52aaf 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -64,6 +64,7 @@ static QObject *qmp_input_get_object(QmpInputVisitor *qiv,
if (QSLIST_EMPTY(&qiv->stack)) {
/* Starting at root, name is ignored. */
+assert(qiv->root);
return qiv->root;
}
@@ -384,6 +385,7 @@ Visitor *qmp_input_visitor_new(QObject *obj, bool strict)
{
QmpInputVisitor *v;
+assert(obj);
v = g_malloc0(sizeof(*v));
v->visitor.type = VISITOR_INPUT;
--
2.10.0
Re: [Qemu-devel] [PATCH v3] scripts: Add a script to check for bug URLs in the git log
On 21/09/2016 21:42, Thomas Huth wrote: > Basic idea of this script is to check the git log for URLs > to the QEMU bugtracker at launchpad.net and to figure out > whether the related bug has been marked there as "Fix released" > (i.e. closed) already. So this script can e.g. be used after > each public release of QEMU to check whether there are any > bug tickets that could be moved from "Fix committed" (or another > state if the author of the patch forgot to update the bug ticket) > to "Fix released". > > Signed-off-by: Thomas Huth > --- > v3: Adressed Eric's review comments from v2 (fixed bashisms, more > POSIX compliance, use lower-case variable names, etc.) Queued with this squashed in: diff --git a/scripts/show-fixed-bugs.sh b/scripts/show-fixed-bugs.sh index 9dcc0f7..6e64edd 100755 --- a/scripts/show-fixed-bugs.sh +++ b/scripts/show-fixed-bugs.sh @@ -23,7 +23,7 @@ while getopts "s:e:cbh" opt; do done if [ "x$start" = "x" ]; then -start=`git tag -l 'v[0-9]*\.[0-9]*.0' | tail -n 2 | head -n 1` +start=`git tag -l 'v[0-9]*\.[0-9]*\.0' | tail -n 2 | head -n 1` fi if [ "x$end" = "x" ]; then end=`git tag -l 'v[0-9]*\.[0-9]*.0' | tail -n 1` @@ -31,7 +31,7 @@ fi if [ "x$start" = "x" ] || [ "x$end" = "x" ]; then echo "Could not determine start or end revision ... Please note that this" -echo "script must be run from a checked out git repository of QEMU!" +echo "script must be run from a checked out git repository of QEMU." exit 1 fi @@ -47,7 +47,7 @@ for i in $bug_urls ; do echo " $i" ; done if [ "x$check_if_open" = "x1" ]; then echo -echo "Checking which ones are still opened..." +echo "Checking which ones are still open..." for i in $bug_urls ; do if ! curl -s -L "$i" | grep "value status" | grep -q "Fix Released" ; then echo " $i"
Re: [Qemu-devel] [PATCH] qmp: fix object-add assert() without props
On 21/09/2016 21:41, Marc-André Lureau wrote:
> Since commit ad739706bbadee49, user_creatable_add_type() expects to be
> given a qdict. However, if object-add is called without props, you reach
> the assert: "qemu/qom/object_interfaces.c:115: user_creatable_add_type:
> Assertion `qdict' failed.", because the qdict isn't created in this
> case (it's optional).
>
> Furthermore, qmp_input_visitor_new() is not meant to be called without a
> dict, and a further commit will assert in this situation.
>
> If none given, create an empty qdict in qmp to avoid the
> user_creatable_add_type() assert(qdict).
>
> Signed-off-by: Marc-André Lureau
> ---
> qmp.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/qmp.c b/qmp.c
> index 6733463..8078038 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -665,7 +665,7 @@ void qmp_add_client(const char *protocol, const char
> *fdname,
> void qmp_object_add(const char *type, const char *id,
> bool has_props, QObject *props, Error **errp)
> {
> -const QDict *pdict = NULL;
> +QDict *pdict;
> Visitor *v;
> Object *obj;
>
> @@ -675,14 +675,19 @@ void qmp_object_add(const char *type, const char *id,
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE, "props", "dict");
> return;
> }
> +} else {
> +pdict = qdict_new();
> }
>
> -v = qmp_input_visitor_new(props, true);
> +v = qmp_input_visitor_new(QOBJECT(pdict), true);
> obj = user_creatable_add_type(type, id, pdict, v, errp);
> visit_free(v);
> if (obj) {
> object_unref(obj);
> }
> +if (!props) {
> +qobject_decref(QOBJECT(pdict));
Or QDECREF(pdict).
In any case,
Reviewed-by: Paolo Bonzini
> +}
> }
>
> void qmp_object_del(const char *id, Error **errp)
>
Re: [Qemu-devel] [PATCH 15/30] qmp-commands: move 'query-migrate-parameters' doc to schema
Hi
- Original Message -
> On 09/13/2016 08:01 AM, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau
> > ---
> > docs/qmp-commands.txt | 29 -
> > qapi-schema.json | 13 +
> > 2 files changed, 13 insertions(+), 29 deletions(-)
> >
>
> > +++ b/qapi-schema.json
> > @@ -1011,6 +1011,19 @@
> > # Returns: @MigrationParameters
> > #
> > # Since: 2.4
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-migrate-parameters" }
> > +# <- { "return": {
> > +# "decompress-threads": 2,
> > +# "cpu-throttle-increment": 10,
> > +# "compress-threads": 8,
> > +# "compress-level": 1,
> > +# "cpu-throttle-initial": 20
> > +# }
> > +#}
> > +#
> > ##
> > { 'command': 'query-migrate-parameters',
> >'returns': 'MigrationParameters' }
>
> The example lacks 'cpu-throttle-increment', 'tls-creds', and
> 'tls-hostname'; do we want to take this opportunity to touch it up?
I suggest to put a [...] in the returned example, as this example could grow
again, and there isn't much to learn from that query.
> Meanwhile, I have a series that touches this code, and will obviously
> create a merge conflict for whoever gets in second:
> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01946.html
Yes, the more we wait to review the series, the more conflicts we will get.
There is still over 100 patches to go, I'll send the next 30.
> At any rate, 11-15 are
> Reviewed-by: Eric Blake
thanks
Re: [Qemu-devel] [PATCH v3] scripts: Add a script to check for bug URLs in the git log
On 09/21/2016 02:42 PM, Thomas Huth wrote: > Basic idea of this script is to check the git log for URLs > to the QEMU bugtracker at launchpad.net and to figure out > whether the related bug has been marked there as "Fix released" > (i.e. closed) already. So this script can e.g. be used after > each public release of QEMU to check whether there are any > bug tickets that could be moved from "Fix committed" (or another > state if the author of the patch forgot to update the bug ticket) > to "Fix released". > > Signed-off-by: Thomas Huth > --- > v3: Adressed Eric's review comments from v2 (fixed bashisms, more > POSIX compliance, use lower-case variable names, etc.) > > + > +if [ "x$start" = "x" ]; then > +start=`git tag -l 'v[0-9]*\.[0-9]*.0' | tail -n 2 | head -n 1` > +fi > +if [ "x$end" = "x" ]; then > +end=`git tag -l 'v[0-9]*\.[0-9]*.0' | tail -n 1` This would match v1.100 (not that we are likely to have that tag); both lines are missing a \ before the second '.' if you are trying to constrain it to just vX.Y.Z forms. > +fi > + > +if [ "x$start" = "x" ] || [ "x$end" = "x" ]; then > +echo "Could not determine start or end revision ... Please note that > this" > +echo "script must be run from a checked out git repository of QEMU!" I'd drop the ending !; we don't need to shout at the user. At any rate, you have indeed addressed the portability issues I pointed out, so with the missing \ fixed, I'm okay with adding: Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
