[Qemu-devel] [PATCH 08/13] kvm: x86: Inject pending MCE events on state writeback
The current way of injecting MCE events without updating of and
synchronizing with the CPUState is broken and causes spurious
corruptions of the MCE-related parts of the CPUState.
As a first step towards a fix, enhance the state writeback code with
support for injecting events that are pending in the CPUState. A pending
exception will then be signaled via cpu_interrupt(CPU_INTERRUPT_MCE).
And, just like for TCG, we need to leave the halt state when
CPU_INTERRUPT_MCE is pending (left broken for the to-be-removed old KVM
code).
This will also allow to unify TCG and KVM injection code.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
target-i386/kvm.c | 75 +---
1 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f909661..46f45db 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -467,6 +467,44 @@ void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t
status,
#endif /* !KVM_CAP_MCE*/
}
+static int kvm_inject_mce_oldstyle(CPUState *env)
+{
+#ifdef KVM_CAP_MCE
+if (kvm_has_vcpu_events()) {
+return 0;
+}
+if (env-interrupt_request CPU_INTERRUPT_MCE) {
+unsigned int bank, bank_num = env-mcg_cap 0xff;
+struct kvm_x86_mce mce;
+
+/* We must not raise CPU_INTERRUPT_MCE if it's not supported. */
+assert(env-mcg_cap);
+
+env-interrupt_request = ~CPU_INTERRUPT_MCE;
+
+/*
+ * There must be at least one bank in use if CPU_INTERRUPT_MCE was set.
+ * Find it and use its values for the event injection.
+ */
+for (bank = 0; bank bank_num; bank++) {
+if (env-mce_banks[bank * 4 + 1] MCI_STATUS_VAL) {
+break;
+}
+}
+assert(bank bank_num);
+
+mce.bank = bank;
+mce.status = env-mce_banks[bank * 4 + 1];
+mce.mcg_status = env-mcg_status;
+mce.addr = env-mce_banks[bank * 4 + 2];
+mce.misc = env-mce_banks[bank * 4 + 3];
+
+return kvm_vcpu_ioctl(env, KVM_X86_SET_MCE, mce);
+}
+#endif /* KVM_CAP_MCE */
+return 0;
+}
+
static void cpu_update_state(void *opaque, int running, int reason)
{
CPUState *env = opaque;
@@ -1375,10 +1413,25 @@ static int kvm_put_vcpu_events(CPUState *env, int level)
return 0;
}
-events.exception.injected = (env-exception_injected = 0);
-events.exception.nr = env-exception_injected;
-events.exception.has_error_code = env-has_error_code;
-events.exception.error_code = env-error_code;
+if (env-interrupt_request CPU_INTERRUPT_MCE) {
+/* We must not raise CPU_INTERRUPT_MCE if it's not supported. */
+assert(env-mcg_cap);
+
+env-interrupt_request = ~CPU_INTERRUPT_MCE;
+if (env-exception_injected == EXCP08_DBLE) {
+/* this means triple fault */
+qemu_system_reset_request();
+env-exit_request = 1;
+}
+events.exception.injected = 1;
+events.exception.nr = EXCP12_MCHK;
+events.exception.has_error_code = 0;
+} else {
+events.exception.injected = (env-exception_injected = 0);
+events.exception.nr = env-exception_injected;
+events.exception.has_error_code = env-has_error_code;
+events.exception.error_code = env-error_code;
+}
events.interrupt.injected = (env-interrupt_injected = 0);
events.interrupt.nr = env-interrupt_injected;
@@ -1539,6 +1592,11 @@ int kvm_arch_put_registers(CPUState *env, int level)
if (ret 0) {
return ret;
}
+/* must be before kvm_put_msrs */
+ret = kvm_inject_mce_oldstyle(env);
+if (ret 0) {
+return ret;
+}
ret = kvm_put_msrs(env, level);
if (ret 0) {
return ret;
@@ -1678,10 +1736,17 @@ void kvm_arch_post_run(CPUState *env, struct kvm_run
*run)
int kvm_arch_process_irqchip_events(CPUState *env)
{
if (kvm_irqchip_in_kernel()) {
+if (env-interrupt_request CPU_INTERRUPT_MCE) {
+kvm_cpu_synchronize_state(env);
+if (env-mp_state == KVM_MP_STATE_HALTED) {
+env-mp_state = KVM_MP_STATE_RUNNABLE;
+}
+}
return 0;
}
-if (env-interrupt_request (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+if (env-interrupt_request
+(CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI | CPU_INTERRUPT_MCE)) {
env-halted = 0;
}
if (env-interrupt_request CPU_INTERRUPT_INIT) {
--
1.7.1
[Qemu-devel] [PATCH 06/13] Synchronize VCPU states before reset
This is required to support keeping VCPU states across a system reset.
If we do not read the current state before the reset,
cpu_synchronize_all_post_reset may write back incorrect state
information.
The first user of this will be MCE MSR synchronization which currently
works around the missing cpu_synchronize_all_states.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
vl.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/vl.c b/vl.c
index b436952..7751843 100644
--- a/vl.c
+++ b/vl.c
@@ -1452,6 +1452,7 @@ static void main_loop(void)
}
if (qemu_reset_requested()) {
pause_all_vcpus();
+cpu_synchronize_all_states();
qemu_system_reset();
resume_all_vcpus();
}
--
1.7.1
[Qemu-devel] [PATCH 10/13] kvm: x86: Clean up kvm_setup_mce
There is nothing to abstract here. Fold kvm_setup_mce into its caller
and fix up the error reporting (return code of kvm_vcpu_ioctl holds the
error value).
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
target-i386/kvm.c | 11 ---
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 8be093b..7cdff65 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -187,11 +187,6 @@ static int kvm_get_mce_cap_supported(KVMState *s, uint64_t
*mce_cap,
return -ENOSYS;
}
-static int kvm_setup_mce(CPUState *env, uint64_t *mcg_cap)
-{
-return kvm_vcpu_ioctl(env, KVM_X86_SETUP_MCE, mcg_cap);
-}
-
static void kvm_mce_inject(CPUState *env, target_phys_addr_t paddr, int code)
{
uint64_t status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN |
@@ -446,6 +441,7 @@ int kvm_arch_init_vcpu(CPUState *env)
kvm_check_extension(env-kvm_state, KVM_CAP_MCE) 0) {
uint64_t mcg_cap;
int banks;
+int ret;
if (kvm_get_mce_cap_supported(env-kvm_state, mcg_cap, banks)) {
perror(kvm_get_mce_cap_supported FAILED);
@@ -454,8 +450,9 @@ int kvm_arch_init_vcpu(CPUState *env)
banks = MCE_BANKS_DEF;
mcg_cap = MCE_CAP_DEF;
mcg_cap |= banks;
-if (kvm_setup_mce(env, mcg_cap)) {
-perror(kvm_setup_mce FAILED);
+ret = kvm_vcpu_ioctl(env, KVM_X86_SETUP_MCE, mcg_cap);
+if (ret 0) {
+fprintf(stderr, KVM_X86_SETUP_MCE: %s, strerror(-ret));
} else {
env-mcg_cap = mcg_cap;
}
--
1.7.1
[Qemu-devel] [PATCH 00/13] [uq/master] Patch queue, part IV (MCE edition)
And the show goes on: This 4th round contains a rework of the MCE handling, mostly for KVM, but also few bits used in emulation mode are affected. Major contributions are: - fixed MCE injection race with other events - kick VCPU out of halt on asynchronous MCEs - unified TCG and KVM MCE injection code - unpoison pages on guest reboot (Huang Ying) This won't be the last uq/master series during my effort to consolidate qemu-kvm and upstream. A few more patches piled up again that will be sent later on separately. CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com CC: Huang Ying ying.hu...@intel.com CC: Jin Dongming jin.dongm...@np.css.fujitsu.com Huang Ying (2): Add qemu_ram_remap KVM, MCE, unpoison memory address across reboot Jan Kiszka (11): x86: Account for MCE in cpu_has_work x86: Perform implicit mcg_status reset x86: Small cleanups of MCE helpers x86: Refine error reporting of MCE injection services x86: Optionally avoid injecting AO MCEs while others are pending Synchronize VCPU states before reset kvm: x86: Move MCE functions together kvm: x86: Inject pending MCE events on state writeback kvm: x86: Consolidate TCG and KVM MCE injection code kvm: x86: Clean up kvm_setup_mce kvm: x86: Fail kvm_arch_init_vcpu if MCE initialization fails cpu-all.h |8 +- cpu-common.h |1 + exec.c| 61 +++- monitor.c | 11 +- qemu-common.h |6 +- target-i386/cpu.h | 11 +- target-i386/exec.h| 15 +- target-i386/helper.c | 185 --- target-i386/kvm.c | 472 - target-i386/kvm_x86.h | 25 --- vl.c |1 + 11 files changed, 400 insertions(+), 396 deletions(-) delete mode 100644 target-i386/kvm_x86.h
[Qemu-devel] [PATCH 09/13] kvm: x86: Consolidate TCG and KVM MCE injection code
This switches KVM's MCE injection path to cpu_x86_inject_mce, both for
SIGBUS and monitor initiated events. This means we prepare the MCA MSRs
in the VCPUState also for KVM.
We have to drop the MSRs writeback restrictions for this purpose which
is now safe as every uncoordinated MSR injection is removed with this
patch.
We have to execute every per-VCPU event setup on the target VCPU as we
are performing a read-modify-write on its state.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
target-i386/helper.c | 105
target-i386/kvm.c | 256 +++--
target-i386/kvm_x86.h | 25 -
3 files changed, 96 insertions(+), 290 deletions(-)
delete mode 100644 target-i386/kvm_x86.h
diff --git a/target-i386/helper.c b/target-i386/helper.c
index e3ef40c..a08309f 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -27,7 +27,6 @@
#include exec-all.h
#include qemu-common.h
#include kvm.h
-#include kvm_x86.h
#ifndef CONFIG_USER_ONLY
#include sysemu.h
#include monitor.h
@@ -1067,29 +1066,42 @@ static void breakpoint_handler(CPUState *env)
prev_debug_excp_handler(env);
}
-static void
-qemu_inject_x86_mce(Monitor *mon, CPUState *cenv, int bank, uint64_t status,
-uint64_t mcg_status, uint64_t addr, uint64_t misc,
-int flags)
+typedef struct MCEInjectionParams {
+Monitor *mon;
+CPUState *env;
+int bank;
+uint64_t status;
+uint64_t mcg_status;
+uint64_t addr;
+uint64_t misc;
+int flags;
+} MCEInjectionParams;
+
+static void do_inject_x86_mce(void *data)
{
-uint64_t mcg_cap = cenv-mcg_cap;
-uint64_t *banks = cenv-mce_banks + 4 * bank;
+MCEInjectionParams *params = data;
+CPUState *cenv = params-env;
+uint64_t *banks = cenv-mce_banks + 4 * params-bank;
+
+cpu_synchronize_state(cenv);
/*
* If there is an MCE exception being processed, ignore this SRAO MCE
* unless unconditional injection was requested.
*/
-if (!(flags MCE_INJECT_UNCOND_AO) !(status MCI_STATUS_AR)
+if (!(params-flags MCE_INJECT_UNCOND_AO)
+ !(params-status MCI_STATUS_AR)
(cenv-mcg_status MCG_STATUS_MCIP)) {
return;
}
-if (status MCI_STATUS_UC) {
+
+if (params-status MCI_STATUS_UC) {
/*
* if MSR_MCG_CTL is not all 1s, the uncorrected error
* reporting is disabled
*/
-if ((mcg_cap MCG_CTL_P) cenv-mcg_ctl != ~(uint64_t)0) {
-monitor_printf(mon,
+if ((cenv-mcg_cap MCG_CTL_P) cenv-mcg_ctl != ~(uint64_t)0) {
+monitor_printf(params-mon,
CPU %d: Uncorrected error reporting disabled\n,
cenv-cpu_index);
return;
@@ -1100,35 +1112,39 @@ qemu_inject_x86_mce(Monitor *mon, CPUState *cenv, int
bank, uint64_t status,
* reporting is disabled for the bank
*/
if (banks[0] != ~(uint64_t)0) {
-monitor_printf(mon, CPU %d: Uncorrected error reporting disabled
- for bank %d\n, cenv-cpu_index, bank);
+monitor_printf(params-mon,
+ CPU %d: Uncorrected error reporting disabled for
+bank %d\n,
+ cenv-cpu_index, params-bank);
return;
}
if ((cenv-mcg_status MCG_STATUS_MCIP) ||
!(cenv-cr[4] CR4_MCE_MASK)) {
-monitor_printf(mon, CPU %d: Previous MCE still in progress,
-raising triple fault\n, cenv-cpu_index);
+monitor_printf(params-mon,
+ CPU %d: Previous MCE still in progress, raising
+triple fault\n,
+ cenv-cpu_index);
qemu_log_mask(CPU_LOG_RESET, Triple fault\n);
qemu_system_reset_request();
return;
}
if (banks[1] MCI_STATUS_VAL) {
-status |= MCI_STATUS_OVER;
+params-status |= MCI_STATUS_OVER;
}
-banks[2] = addr;
-banks[3] = misc;
-cenv-mcg_status = mcg_status;
-banks[1] = status;
+banks[2] = params-addr;
+banks[3] = params-misc;
+cenv-mcg_status = params-mcg_status;
+banks[1] = params-status;
cpu_interrupt(cenv, CPU_INTERRUPT_MCE);
} else if (!(banks[1] MCI_STATUS_VAL)
|| !(banks[1] MCI_STATUS_UC)) {
if (banks[1] MCI_STATUS_VAL) {
-status |= MCI_STATUS_OVER;
+params-status |= MCI_STATUS_OVER;
}
-banks[2] = addr;
-banks[3] = misc;
-banks[1] = status;
+banks[2] = params-addr;
+banks[3] = params-misc;
+banks[1] =
[Qemu-devel] [PATCH 05/13] x86: Optionally avoid injecting AO MCEs while others are pending
Allow to tell cpu_x86_inject_mce that it should ignore Action Optional
MCE events when the target VCPU is still processing another one. This
will be used by KVM soon.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
monitor.c|7 +--
target-i386/cpu.h|5 -
target-i386/helper.c | 26 +++---
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/monitor.c b/monitor.c
index 662df7c..ae20927 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2709,12 +2709,15 @@ static void do_inject_mce(Monitor *mon, const QDict
*qdict)
uint64_t mcg_status = qdict_get_int(qdict, mcg_status);
uint64_t addr = qdict_get_int(qdict, addr);
uint64_t misc = qdict_get_int(qdict, misc);
-int broadcast = qdict_get_try_bool(qdict, broadcast, 0);
+int flags = MCE_INJECT_UNCOND_AO;
+if (qdict_get_try_bool(qdict, broadcast, 0)) {
+flags |= MCE_INJECT_BROADCAST;
+}
for (cenv = first_cpu; cenv != NULL; cenv = cenv-next_cpu) {
if (cenv-cpu_index == cpu_index) {
cpu_x86_inject_mce(mon, cenv, bank, status, mcg_status, addr, misc,
- broadcast);
+ flags);
break;
}
}
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 486af1d..d0eae75 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -987,8 +987,11 @@ static inline void cpu_get_tb_cpu_state(CPUState *env,
target_ulong *pc,
void do_cpu_init(CPUState *env);
void do_cpu_sipi(CPUState *env);
+#define MCE_INJECT_BROADCAST1
+#define MCE_INJECT_UNCOND_AO2
+
void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv, int bank,
uint64_t status, uint64_t mcg_status, uint64_t addr,
-uint64_t misc, int broadcast);
+uint64_t misc, int flags);
#endif /* CPU_I386_H */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 462d332..e3ef40c 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1069,11 +1069,20 @@ static void breakpoint_handler(CPUState *env)
static void
qemu_inject_x86_mce(Monitor *mon, CPUState *cenv, int bank, uint64_t status,
-uint64_t mcg_status, uint64_t addr, uint64_t misc)
+uint64_t mcg_status, uint64_t addr, uint64_t misc,
+int flags)
{
uint64_t mcg_cap = cenv-mcg_cap;
uint64_t *banks = cenv-mce_banks + 4 * bank;
+/*
+ * If there is an MCE exception being processed, ignore this SRAO MCE
+ * unless unconditional injection was requested.
+ */
+if (!(flags MCE_INJECT_UNCOND_AO) !(status MCI_STATUS_AR)
+ (cenv-mcg_status MCG_STATUS_MCIP)) {
+return;
+}
if (status MCI_STATUS_UC) {
/*
* if MSR_MCG_CTL is not all 1s, the uncorrected error
@@ -1127,7 +1136,7 @@ qemu_inject_x86_mce(Monitor *mon, CPUState *cenv, int
bank, uint64_t status,
void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv, int bank,
uint64_t status, uint64_t mcg_status, uint64_t addr,
-uint64_t misc, int broadcast)
+uint64_t misc, int flags)
{
unsigned bank_num = cenv-mcg_cap 0xff;
CPUState *env;
@@ -1145,27 +1154,30 @@ void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv,
int bank,
monitor_printf(mon, Invalid MCE status code\n);
return;
}
-if (broadcast !cpu_x86_support_mca_broadcast(cenv)) {
+if ((flags MCE_INJECT_BROADCAST)
+ !cpu_x86_support_mca_broadcast(cenv)) {
monitor_printf(mon, Guest CPU does not support MCA broadcast\n);
return;
}
if (kvm_enabled()) {
-if (broadcast) {
+if (flags MCE_INJECT_BROADCAST) {
flag |= MCE_BROADCAST;
}
kvm_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc, flag);
} else {
-qemu_inject_x86_mce(mon, cenv, bank, status, mcg_status, addr, misc);
-if (broadcast) {
+qemu_inject_x86_mce(mon, cenv, bank, status, mcg_status, addr, misc,
+flags);
+if (flags MCE_INJECT_BROADCAST) {
for (env = first_cpu; env != NULL; env = env-next_cpu) {
if (cenv == env) {
continue;
}
qemu_inject_x86_mce(mon, env, 1,
MCI_STATUS_VAL | MCI_STATUS_UC,
-MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0);
+MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0,
+flags);
}
}
}
--
1.7.1
[Qemu-devel] [PATCH 12/13] Add qemu_ram_remap
From: Huang Ying ying.hu...@intel.com
qemu_ram_remap() unmaps the specified RAM pages, then re-maps these
pages again. This is used by KVM HWPoison support to clear HWPoisoned
page tables across guest rebooting, so that a new page may be
allocated later to recover the memory error.
[ Jan: style fixlets ]
Signed-off-by: Huang Ying ying.hu...@intel.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
cpu-all.h|4 +++
cpu-common.h |1 +
exec.c | 61 +-
3 files changed, 65 insertions(+), 1 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index caf5e6c..4f4631d 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -863,10 +863,14 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env,
target_ulong addr);
extern int phys_ram_fd;
extern ram_addr_t ram_size;
+/* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */
+#define RAM_PREALLOC_MASK (1 0)
+
typedef struct RAMBlock {
uint8_t *host;
ram_addr_t offset;
ram_addr_t length;
+uint32_t flags;
char idstr[256];
QLIST_ENTRY(RAMBlock) next;
#if defined(__linux__) !defined(TARGET_S390X)
diff --git a/cpu-common.h b/cpu-common.h
index 54d21d4..ef4e8da 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -50,6 +50,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const
char *name,
ram_addr_t size, void *host);
ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size);
void qemu_ram_free(ram_addr_t addr);
+void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
/* This should only be used for ram local to a device. */
void *qemu_get_ram_ptr(ram_addr_t addr);
/* Same but slower, to use for migration, where the order of
diff --git a/exec.c b/exec.c
index d611100..e486458 100644
--- a/exec.c
+++ b/exec.c
@@ -2867,6 +2867,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev,
const char *name,
if (host) {
new_block-host = host;
+new_block-flags |= RAM_PREALLOC_MASK;
} else {
if (mem_path) {
#if defined (__linux__) !defined(TARGET_S390X)
@@ -2920,7 +2921,9 @@ void qemu_ram_free(ram_addr_t addr)
QLIST_FOREACH(block, ram_list.blocks, next) {
if (addr == block-offset) {
QLIST_REMOVE(block, next);
-if (mem_path) {
+if (block-flags RAM_PREALLOC_MASK) {
+;
+} else if (mem_path) {
#if defined (__linux__) !defined(TARGET_S390X)
if (block-fd) {
munmap(block-host, block-length);
@@ -2943,6 +2946,62 @@ void qemu_ram_free(ram_addr_t addr)
}
+void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
+{
+RAMBlock *block;
+ram_addr_t offset;
+int flags;
+void *area, *vaddr;
+
+QLIST_FOREACH(block, ram_list.blocks, next) {
+offset = addr - block-offset;
+if (offset block-length) {
+vaddr = block-host + offset;
+if (block-flags RAM_PREALLOC_MASK) {
+;
+} else {
+flags = MAP_FIXED;
+munmap(vaddr, length);
+if (mem_path) {
+#if defined(__linux__) !defined(TARGET_S390X)
+if (block-fd) {
+#ifdef MAP_POPULATE
+flags |= mem_prealloc ? MAP_POPULATE | MAP_SHARED :
+MAP_PRIVATE;
+#else
+flags |= MAP_PRIVATE;
+#endif
+area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
+flags, block-fd, offset);
+} else {
+flags |= MAP_PRIVATE | MAP_ANONYMOUS;
+area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
+flags, -1, 0);
+}
+#endif
+} else {
+#if defined(TARGET_S390X) defined(CONFIG_KVM)
+flags |= MAP_SHARED | MAP_ANONYMOUS;
+area = mmap(vaddr, length, PROT_EXEC|PROT_READ|PROT_WRITE,
+flags, -1, 0);
+#else
+flags |= MAP_PRIVATE | MAP_ANONYMOUS;
+area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
+flags, -1, 0);
+#endif
+}
+if (area != vaddr) {
+fprintf(stderr, Could not remap addr: %lx@%lx\n,
+length, addr);
+exit(1);
+}
+qemu_madvise(vaddr, length, QEMU_MADV_MERGEABLE);
+}
+return;
+}
+}
+}
+
/* Return a host pointer to ram allocated with qemu_ram_alloc.
With the exception of the softmmu code in this file, this should
only be used for local memory (e.g. video ram) that the device owns,
--
1.7.1
[Qemu-devel] [PATCH 01/13] x86: Account for MCE in cpu_has_work
MCEs can be injected asynchronously, so they can also terminate the halt
state.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
target-i386/exec.h | 15 ++-
1 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/target-i386/exec.h b/target-i386/exec.h
index fc8945b..d050dd0 100644
--- a/target-i386/exec.h
+++ b/target-i386/exec.h
@@ -293,15 +293,12 @@ static inline void load_eflags(int eflags, int
update_mask)
static inline int cpu_has_work(CPUState *env)
{
-int work;
-
-work = (env-interrupt_request CPU_INTERRUPT_HARD)
- (env-eflags IF_MASK);
-work |= env-interrupt_request CPU_INTERRUPT_NMI;
-work |= env-interrupt_request CPU_INTERRUPT_INIT;
-work |= env-interrupt_request CPU_INTERRUPT_SIPI;
-
-return work;
+return ((env-interrupt_request CPU_INTERRUPT_HARD)
+(env-eflags IF_MASK)) ||
+ (env-interrupt_request (CPU_INTERRUPT_NMI |
+ CPU_INTERRUPT_INIT |
+ CPU_INTERRUPT_SIPI |
+ CPU_INTERRUPT_MCE));
}
static inline int cpu_halted(CPUState *env) {
--
1.7.1
[Qemu-devel] [PATCHv2] Handle icount for powerpc tbl/tbu/decr load and store.
Handle option '-icount X' on powerpc targets.
Signed-off-by: Tristan Gingold ging...@adacore.com
---
target-ppc/translate_init.c | 42 ++
1 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 907535e..27aff74 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -154,12 +154,26 @@ static void spr_read_ureg (void *opaque, int gprn, int
sprn)
#if !defined(CONFIG_USER_ONLY)
static void spr_read_decr (void *opaque, int gprn, int sprn)
{
+if (use_icount) {
+gen_io_start();
+}
gen_helper_load_decr(cpu_gpr[gprn]);
+if (use_icount) {
+gen_io_end();
+gen_stop_exception(opaque);
+}
}
static void spr_write_decr (void *opaque, int sprn, int gprn)
{
+if (use_icount) {
+gen_io_start();
+}
gen_helper_store_decr(cpu_gpr[gprn]);
+if (use_icount) {
+gen_io_end();
+gen_stop_exception(opaque);
+}
}
#endif
@@ -167,12 +181,26 @@ static void spr_write_decr (void *opaque, int sprn, int
gprn)
/* Time base */
static void spr_read_tbl (void *opaque, int gprn, int sprn)
{
+if (use_icount) {
+gen_io_start();
+}
gen_helper_load_tbl(cpu_gpr[gprn]);
+if (use_icount) {
+gen_io_end();
+gen_stop_exception(opaque);
+}
}
static void spr_read_tbu (void *opaque, int gprn, int sprn)
{
+if (use_icount) {
+gen_io_start();
+}
gen_helper_load_tbu(cpu_gpr[gprn]);
+if (use_icount) {
+gen_io_end();
+gen_stop_exception(opaque);
+}
}
__attribute__ (( unused ))
@@ -190,12 +218,26 @@ static void spr_read_atbu (void *opaque, int gprn, int
sprn)
#if !defined(CONFIG_USER_ONLY)
static void spr_write_tbl (void *opaque, int sprn, int gprn)
{
+if (use_icount) {
+gen_io_start();
+}
gen_helper_store_tbl(cpu_gpr[gprn]);
+if (use_icount) {
+gen_io_end();
+gen_stop_exception(opaque);
+}
}
static void spr_write_tbu (void *opaque, int sprn, int gprn)
{
+if (use_icount) {
+gen_io_start();
+}
gen_helper_store_tbu(cpu_gpr[gprn]);
+if (use_icount) {
+gen_io_end();
+gen_stop_exception(opaque);
+}
}
__attribute__ (( unused ))
--
1.7.3.GIT
[Qemu-devel] [PATCH 02/13] x86: Perform implicit mcg_status reset
Reorder mcg_status in CPUState to achive automatic clearing on reset.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
target-i386/cpu.h|3 ++-
target-i386/helper.c |2 --
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 5f1df8b..75156e7 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -687,6 +687,8 @@ typedef struct CPUX86State {
uint64_t pat;
+uint64_t mcg_status;
+
/* exception/interrupt handling */
int error_code;
int exception_is_int;
@@ -741,7 +743,6 @@ typedef struct CPUX86State {
struct DeviceState *apic_state;
uint64_t mcg_cap;
-uint64_t mcg_status;
uint64_t mcg_ctl;
uint64_t mce_banks[MCE_BANKS_DEF*4];
diff --git a/target-i386/helper.c b/target-i386/helper.c
index f0c546d..f41416f 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -101,8 +101,6 @@ void cpu_reset(CPUX86State *env)
env-dr[7] = DR7_FIXED_1;
cpu_breakpoint_remove_all(env, BP_CPU);
cpu_watchpoint_remove_all(env, BP_CPU);
-
-env-mcg_status = 0;
}
void cpu_x86_close(CPUX86State *env)
--
1.7.1
[Qemu-devel] [PATCH 13/13] KVM, MCE, unpoison memory address across reboot
From: Huang Ying ying.hu...@intel.com
In Linux kernel HWPoison processing implementation, the virtual
address in processes mapping the error physical memory page is marked
as HWPoison. So that, the further accessing to the virtual
address will kill corresponding processes with SIGBUS.
If the error physical memory page is used by a KVM guest, the SIGBUS
will be sent to QEMU, and QEMU will simulate a MCE to report that
memory error to the guest OS. If the guest OS can not recover from
the error (for example, the page is accessed by kernel code), guest OS
will reboot the system. But because the underlying host virtual
address backing the guest physical memory is still poisoned, if the
guest system accesses the corresponding guest physical memory even
after rebooting, the SIGBUS will still be sent to QEMU and MCE will be
simulated. That is, guest system can not recover via rebooting.
In fact, across rebooting, the contents of guest physical memory page
need not to be kept. We can allocate a new host physical page to
back the corresponding guest physical address.
This patch fixes this issue in QEMU-KVM via calling qemu_ram_remap()
to clear the corresponding page table entry, so that make it possible
to allocate a new page to recover the issue.
[ Jan: rebasing and tiny cleanups]
Signed-off-by: Huang Ying ying.hu...@intel.com
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
target-i386/kvm.c | 36
1 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 8eda78b..45e366a 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -173,7 +173,40 @@ static int get_para_features(CPUState *env)
}
#endif /* CONFIG_KVM_PARA */
+typedef struct HWPoisonPage {
+ram_addr_t ram_addr;
+QLIST_ENTRY(HWPoisonPage) list;
+} HWPoisonPage;
+
+static QLIST_HEAD(, HWPoisonPage) hwpoison_page_list =
+QLIST_HEAD_INITIALIZER(hwpoison_page_list);
+
+static void kvm_unpoison_all(void *param)
+{
+HWPoisonPage *page, *next_page;
+
+QLIST_FOREACH_SAFE(page, hwpoison_page_list, list, next_page) {
+QLIST_REMOVE(page, list);
+qemu_ram_remap(page-ram_addr, TARGET_PAGE_SIZE);
+qemu_free(page);
+}
+}
+
#ifdef KVM_CAP_MCE
+static void kvm_hwpoison_page_add(ram_addr_t ram_addr)
+{
+HWPoisonPage *page;
+
+QLIST_FOREACH(page, hwpoison_page_list, list) {
+if (page-ram_addr == ram_addr) {
+return;
+}
+}
+page = qemu_malloc(sizeof(HWPoisonPage));
+page-ram_addr = ram_addr;
+QLIST_INSERT_HEAD(hwpoison_page_list, page, list);
+}
+
static int kvm_get_mce_cap_supported(KVMState *s, uint64_t *mce_cap,
int *max_banks)
{
@@ -233,6 +266,7 @@ int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void
*hvaddr)
hardware_memory_error();
}
}
+kvm_hwpoison_page_add(ram_addr);
kvm_mce_inject(env, gpaddr, code);
} else
#endif /* KVM_CAP_MCE */
@@ -263,6 +297,7 @@ int kvm_arch_on_sigbus(int code, void *hvaddr)
QEMU itself instead of guest system!: %p\n, hvaddr);
return 0;
}
+kvm_hwpoison_page_add(ram_addr);
kvm_mce_inject(first_cpu, gpaddr, code);
} else
#endif /* KVM_CAP_MCE */
@@ -577,6 +612,7 @@ int kvm_arch_init(KVMState *s)
fprintf(stderr, e820_add_entry() table is full\n);
return ret;
}
+qemu_register_reset(kvm_unpoison_all, NULL);
return 0;
}
--
1.7.1
[Qemu-devel] [PATCH 07/13] kvm: x86: Move MCE functions together
Pure function suffling to avoid multiple #ifdef KVM_CAP_MCE sections,
no functional changes. While at it, annotate some #ifdef sections.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
target-i386/kvm.c | 346 ++---
1 files changed, 171 insertions(+), 175 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0aa0a41..f909661 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -172,7 +172,7 @@ static int get_para_features(CPUState *env)
#endif
return features;
}
-#endif
+#endif /* CONFIG_KVM_PARA */
#ifdef KVM_CAP_MCE
static int kvm_get_mce_cap_supported(KVMState *s, uint64_t *mce_cap,
@@ -273,8 +273,174 @@ static void kvm_inject_x86_mce_on(CPUState *env, struct
kvm_x86_mce *mce,
run_on_cpu(env, kvm_do_inject_x86_mce, data);
}
-static void kvm_mce_broadcast_rest(CPUState *env);
-#endif
+static void kvm_mce_broadcast_rest(CPUState *env)
+{
+struct kvm_x86_mce mce = {
+.bank = 1,
+.status = MCI_STATUS_VAL | MCI_STATUS_UC,
+.mcg_status = MCG_STATUS_MCIP | MCG_STATUS_RIPV,
+.addr = 0,
+.misc = 0,
+};
+CPUState *cenv;
+
+/* Broadcast MCA signal for processor version 06H_EH and above */
+if (cpu_x86_support_mca_broadcast(env)) {
+for (cenv = first_cpu; cenv != NULL; cenv = cenv-next_cpu) {
+if (cenv == env) {
+continue;
+}
+kvm_inject_x86_mce_on(cenv, mce, ABORT_ON_ERROR);
+}
+}
+}
+
+static void kvm_mce_inj_srar_dataload(CPUState *env, target_phys_addr_t paddr)
+{
+struct kvm_x86_mce mce = {
+.bank = 9,
+.status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN
+ | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S
+ | MCI_STATUS_AR | 0x134,
+.mcg_status = MCG_STATUS_MCIP | MCG_STATUS_EIPV,
+.addr = paddr,
+.misc = (MCM_ADDR_PHYS 6) | 0xc,
+};
+int r;
+
+r = kvm_set_mce(env, mce);
+if (r 0) {
+fprintf(stderr, kvm_set_mce: %s\n, strerror(errno));
+abort();
+}
+kvm_mce_broadcast_rest(env);
+}
+
+static void kvm_mce_inj_srao_memscrub(CPUState *env, target_phys_addr_t paddr)
+{
+struct kvm_x86_mce mce = {
+.bank = 9,
+.status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN
+ | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S
+ | 0xc0,
+.mcg_status = MCG_STATUS_MCIP | MCG_STATUS_RIPV,
+.addr = paddr,
+.misc = (MCM_ADDR_PHYS 6) | 0xc,
+};
+int r;
+
+r = kvm_set_mce(env, mce);
+if (r 0) {
+fprintf(stderr, kvm_set_mce: %s\n, strerror(errno));
+abort();
+}
+kvm_mce_broadcast_rest(env);
+}
+
+static void kvm_mce_inj_srao_memscrub2(CPUState *env, target_phys_addr_t paddr)
+{
+struct kvm_x86_mce mce = {
+.bank = 9,
+.status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN
+ | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S
+ | 0xc0,
+.mcg_status = MCG_STATUS_MCIP | MCG_STATUS_RIPV,
+.addr = paddr,
+.misc = (MCM_ADDR_PHYS 6) | 0xc,
+};
+
+kvm_inject_x86_mce_on(env, mce, ABORT_ON_ERROR);
+kvm_mce_broadcast_rest(env);
+}
+#endif /* KVM_CAP_MCE */
+
+static void hardware_memory_error(void)
+{
+fprintf(stderr, Hardware memory error!\n);
+exit(1);
+}
+
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+#ifdef KVM_CAP_MCE
+void *vaddr;
+ram_addr_t ram_addr;
+target_phys_addr_t paddr;
+
+if ((env-mcg_cap MCG_SER_P) addr
+ (code == BUS_MCEERR_AR
+|| code == BUS_MCEERR_AO)) {
+vaddr = (void *)addr;
+if (qemu_ram_addr_from_host(vaddr, ram_addr) ||
+!kvm_physical_memory_addr_from_ram(env-kvm_state, ram_addr,
paddr)) {
+fprintf(stderr, Hardware memory error for memory used by
+QEMU itself instead of guest system!\n);
+/* Hope we are lucky for AO MCE */
+if (code == BUS_MCEERR_AO) {
+return 0;
+} else {
+hardware_memory_error();
+}
+}
+
+if (code == BUS_MCEERR_AR) {
+/* Fake an Intel architectural Data Load SRAR UCR */
+kvm_mce_inj_srar_dataload(env, paddr);
+} else {
+/*
+ * If there is an MCE excpetion being processed, ignore
+ * this SRAO MCE
+ */
+if (!kvm_mce_in_progress(env)) {
+/* Fake an Intel architectural Memory scrubbing UCR */
+kvm_mce_inj_srao_memscrub(env, paddr);
+}
+}
+} else
+#endif /* KVM_CAP_MCE */
+{
+if (code == BUS_MCEERR_AO) {
+return 0;
+
[Qemu-devel] [PATCH 03/13] x86: Small cleanups of MCE helpers
Fix some code style issues, use proper headers, and align to cpu_x86
naming scheme. No functional changes.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
cpu-all.h|4
monitor.c|2 +-
target-i386/cpu.h|5 +
target-i386/helper.c | 41 -
4 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index 87b0f86..caf5e6c 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -971,8 +971,4 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
uint8_t *buf, int len, int is_write);
-void cpu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
-uint64_t mcg_status, uint64_t addr, uint64_t misc,
-int broadcast);
-
#endif /* CPU_ALL_H */
diff --git a/monitor.c b/monitor.c
index 22ae3bb..45b0cc2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2713,7 +2713,7 @@ static void do_inject_mce(Monitor *mon, const QDict
*qdict)
for (cenv = first_cpu; cenv != NULL; cenv = cenv-next_cpu) {
if (cenv-cpu_index == cpu_index cenv-mcg_cap) {
-cpu_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc,
+cpu_x86_inject_mce(cenv, bank, status, mcg_status, addr, misc,
broadcast);
break;
}
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 75156e7..52bb48e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -986,4 +986,9 @@ static inline void cpu_get_tb_cpu_state(CPUState *env,
target_ulong *pc,
void do_cpu_init(CPUState *env);
void do_cpu_sipi(CPUState *env);
+
+void cpu_x86_inject_mce(CPUState *cenv, int bank, uint64_t status,
+uint64_t mcg_status, uint64_t addr, uint64_t misc,
+int broadcast);
+
#endif /* CPU_I386_H */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index f41416f..ba3bed9 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -28,6 +28,9 @@
#include qemu-common.h
#include kvm.h
#include kvm_x86.h
+#ifndef CONFIG_USER_ONLY
+#include sysemu.h
+#endif
//#define DEBUG_MMU
@@ -1063,11 +1066,9 @@ static void breakpoint_handler(CPUState *env)
prev_debug_excp_handler(env);
}
-/* This should come from sysemu.h - if we could include it here... */
-void qemu_system_reset_request(void);
-
-static void qemu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
-uint64_t mcg_status, uint64_t addr, uint64_t misc)
+static void
+qemu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
+uint64_t mcg_status, uint64_t addr, uint64_t misc)
{
uint64_t mcg_cap = cenv-mcg_cap;
uint64_t *banks = cenv-mce_banks;
@@ -1077,15 +1078,17 @@ static void qemu_inject_x86_mce(CPUState *cenv, int
bank, uint64_t status,
* reporting is disabled
*/
if ((status MCI_STATUS_UC) (mcg_cap MCG_CTL_P)
-cenv-mcg_ctl != ~(uint64_t)0)
+cenv-mcg_ctl != ~(uint64_t)0) {
return;
+}
banks += 4 * bank;
/*
* if MSR_MCi_CTL is not all 1s, the uncorrected error
* reporting is disabled for the bank
*/
-if ((status MCI_STATUS_UC) banks[0] != ~(uint64_t)0)
+if ((status MCI_STATUS_UC) banks[0] != ~(uint64_t)0) {
return;
+}
if (status MCI_STATUS_UC) {
if ((cenv-mcg_status MCG_STATUS_MCIP) ||
!(cenv-cr[4] CR4_MCE_MASK)) {
@@ -1095,8 +1098,9 @@ static void qemu_inject_x86_mce(CPUState *cenv, int bank,
uint64_t status,
qemu_system_reset_request();
return;
}
-if (banks[1] MCI_STATUS_VAL)
+if (banks[1] MCI_STATUS_VAL) {
status |= MCI_STATUS_OVER;
+}
banks[2] = addr;
banks[3] = misc;
cenv-mcg_status = mcg_status;
@@ -1104,16 +1108,18 @@ static void qemu_inject_x86_mce(CPUState *cenv, int
bank, uint64_t status,
cpu_interrupt(cenv, CPU_INTERRUPT_MCE);
} else if (!(banks[1] MCI_STATUS_VAL)
|| !(banks[1] MCI_STATUS_UC)) {
-if (banks[1] MCI_STATUS_VAL)
+if (banks[1] MCI_STATUS_VAL) {
status |= MCI_STATUS_OVER;
+}
banks[2] = addr;
banks[3] = misc;
banks[1] = status;
-} else
+} else {
banks[1] |= MCI_STATUS_OVER;
+}
}
-void cpu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
+void cpu_x86_inject_mce(CPUState *cenv, int bank, uint64_t status,
uint64_t mcg_status, uint64_t addr, uint64_t misc,
int broadcast)
{
@@ -1155,15 +1161,16 @@ void cpu_inject_x86_mce(CPUState *cenv, int bank,
uint64_t status,
static void mce_init(CPUX86State
[Qemu-devel] [PATCH 04/13] x86: Refine error reporting of MCE injection services
As this service is used by the human monitor, make sure that errors get
reported to the right channel, and also raise the verbosity.
This requires to move Monitor typedef in qemu-common.h to resolve the
include dependency.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
monitor.c|4 +-
qemu-common.h|6 ++--
target-i386/cpu.h|6 ++--
target-i386/helper.c | 79 +-
4 files changed, 54 insertions(+), 41 deletions(-)
diff --git a/monitor.c b/monitor.c
index 45b0cc2..662df7c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2712,8 +2712,8 @@ static void do_inject_mce(Monitor *mon, const QDict
*qdict)
int broadcast = qdict_get_try_bool(qdict, broadcast, 0);
for (cenv = first_cpu; cenv != NULL; cenv = cenv-next_cpu) {
-if (cenv-cpu_index == cpu_index cenv-mcg_cap) {
-cpu_x86_inject_mce(cenv, bank, status, mcg_status, addr, misc,
+if (cenv-cpu_index == cpu_index) {
+cpu_x86_inject_mce(mon, cenv, bank, status, mcg_status, addr, misc,
broadcast);
break;
}
diff --git a/qemu-common.h b/qemu-common.h
index a4d9c21..6ac29cc 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -18,6 +18,9 @@ typedef struct QEMUFile QEMUFile;
typedef struct QEMUBH QEMUBH;
typedef struct DeviceState DeviceState;
+struct Monitor;
+typedef struct Monitor Monitor;
+
/* we put basic includes here to avoid repeating them in device drivers */
#include stdlib.h
#include stdio.h
@@ -324,9 +327,6 @@ void qemu_iovec_to_buffer(QEMUIOVector *qiov, void *buf);
void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count);
void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count);
-struct Monitor;
-typedef struct Monitor Monitor;
-
/* Convert a byte between binary and BCD. */
static inline uint8_t to_bcd(uint8_t val)
{
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 52bb48e..486af1d 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -987,8 +987,8 @@ static inline void cpu_get_tb_cpu_state(CPUState *env,
target_ulong *pc,
void do_cpu_init(CPUState *env);
void do_cpu_sipi(CPUState *env);
-void cpu_x86_inject_mce(CPUState *cenv, int bank, uint64_t status,
-uint64_t mcg_status, uint64_t addr, uint64_t misc,
-int broadcast);
+void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv, int bank,
+uint64_t status, uint64_t mcg_status, uint64_t addr,
+uint64_t misc, int broadcast);
#endif /* CPU_I386_H */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index ba3bed9..462d332 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -30,6 +30,7 @@
#include kvm_x86.h
#ifndef CONFIG_USER_ONLY
#include sysemu.h
+#include monitor.h
#endif
//#define DEBUG_MMU
@@ -1067,33 +1068,38 @@ static void breakpoint_handler(CPUState *env)
}
static void
-qemu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
+qemu_inject_x86_mce(Monitor *mon, CPUState *cenv, int bank, uint64_t status,
uint64_t mcg_status, uint64_t addr, uint64_t misc)
{
uint64_t mcg_cap = cenv-mcg_cap;
-uint64_t *banks = cenv-mce_banks;
-
-/*
- * if MSR_MCG_CTL is not all 1s, the uncorrected error
- * reporting is disabled
- */
-if ((status MCI_STATUS_UC) (mcg_cap MCG_CTL_P)
-cenv-mcg_ctl != ~(uint64_t)0) {
-return;
-}
-banks += 4 * bank;
-/*
- * if MSR_MCi_CTL is not all 1s, the uncorrected error
- * reporting is disabled for the bank
- */
-if ((status MCI_STATUS_UC) banks[0] != ~(uint64_t)0) {
-return;
-}
+uint64_t *banks = cenv-mce_banks + 4 * bank;
+
if (status MCI_STATUS_UC) {
+/*
+ * if MSR_MCG_CTL is not all 1s, the uncorrected error
+ * reporting is disabled
+ */
+if ((mcg_cap MCG_CTL_P) cenv-mcg_ctl != ~(uint64_t)0) {
+monitor_printf(mon,
+ CPU %d: Uncorrected error reporting disabled\n,
+ cenv-cpu_index);
+return;
+}
+
+/*
+ * if MSR_MCi_CTL is not all 1s, the uncorrected error
+ * reporting is disabled for the bank
+ */
+if (banks[0] != ~(uint64_t)0) {
+monitor_printf(mon, CPU %d: Uncorrected error reporting disabled
+ for bank %d\n, cenv-cpu_index, bank);
+return;
+}
+
if ((cenv-mcg_status MCG_STATUS_MCIP) ||
!(cenv-cr[4] CR4_MCE_MASK)) {
-fprintf(stderr, injects mce exception while previous
-one is in progress!\n);
+monitor_printf(mon, CPU %d: Previous MCE still in progress,
+
[Qemu-devel] [PATCH] ppc: init_excp_7x0: fix hreset entry point.
From: gingold gingold@020d506d-db78-4e55-b2a8-6c851f84c332 According to the PowePC 750 user's manual, the vector offset for system reset (both /HRESET and /SRESET) is 0x00100. Signed-off-by: Tristan Gingold ging...@adacore.com --- target-ppc/translate_init.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 5d856f5..907535e 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -2925,7 +2925,7 @@ static void init_excp_7x0 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700; env-hreset_excp_prefix = 0xUL; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0xFFF00100UL; #endif } -- 1.7.3.GIT
[Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr
From: Fabien Chouteau chout...@adacore.com
Although the support of this register may be uncomplete, there are no
reason to prevent the debugger from reading or writing it.
Signed-off-by: Tristan Gingold ging...@adacore.com
---
gdbstub.c |5 +++--
target-ppc/translate_init.c |5 ++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 5c9a1c9..70870fb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -722,7 +722,7 @@ static int cpu_gdb_read_register(CPUState *env, uint8_t
*mem_buf, int n)
{
if (gdb_has_xml)
return 0;
-GET_REG32(0); /* fpscr */
+GET_REG32(env-fpscr);
}
}
}
@@ -770,7 +770,8 @@ static int cpu_gdb_write_register(CPUState *env, uint8_t
*mem_buf, int n)
/* fpscr */
if (gdb_has_xml)
return 0;
-return 4;
+env-fpscr = ldtul_p(mem_buf);
+return sizeof(target_ulong);
}
}
return 0;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index dfcd949..5d856f5 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9381,8 +9381,7 @@ static int gdb_get_float_reg(CPUState *env, uint8_t
*mem_buf, int n)
return 8;
}
if (n == 32) {
-/* FPSCR not implemented */
-memset(mem_buf, 0, 4);
+stl_p(mem_buf, env-fpscr);
return 4;
}
return 0;
@@ -9395,7 +9394,7 @@ static int gdb_set_float_reg(CPUState *env, uint8_t
*mem_buf, int n)
return 8;
}
if (n == 32) {
-/* FPSCR not implemented */
+env-fpscr = ldl_p(mem_buf);
return 4;
}
return 0;
--
1.7.3.GIT
[Qemu-devel] [PATCH 11/13] kvm: x86: Fail kvm_arch_init_vcpu if MCE initialization fails
There is no reason to continue if the kernel claims to support MCE but
then fails to process our request.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
CC: Huang Ying ying.hu...@intel.com
CC: Hidetoshi Seto seto.hideto...@jp.fujitsu.com
CC: Jin Dongming jin.dongm...@np.css.fujitsu.com
---
target-i386/kvm.c | 30 +-
1 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7cdff65..8eda78b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -443,20 +443,24 @@ int kvm_arch_init_vcpu(CPUState *env)
int banks;
int ret;
-if (kvm_get_mce_cap_supported(env-kvm_state, mcg_cap, banks)) {
-perror(kvm_get_mce_cap_supported FAILED);
-} else {
-if (banks MCE_BANKS_DEF)
-banks = MCE_BANKS_DEF;
-mcg_cap = MCE_CAP_DEF;
-mcg_cap |= banks;
-ret = kvm_vcpu_ioctl(env, KVM_X86_SETUP_MCE, mcg_cap);
-if (ret 0) {
-fprintf(stderr, KVM_X86_SETUP_MCE: %s, strerror(-ret));
-} else {
-env-mcg_cap = mcg_cap;
-}
+ret = kvm_get_mce_cap_supported(env-kvm_state, mcg_cap, banks);
+if (ret 0) {
+fprintf(stderr, kvm_get_mce_cap_supported: %s, strerror(-ret));
+return ret;
}
+
+if (banks MCE_BANKS_DEF) {
+banks = MCE_BANKS_DEF;
+}
+mcg_cap = MCE_CAP_DEF;
+mcg_cap |= banks;
+ret = kvm_vcpu_ioctl(env, KVM_X86_SETUP_MCE, mcg_cap);
+if (ret 0) {
+fprintf(stderr, KVM_X86_SETUP_MCE: %s, strerror(-ret));
+return ret;
+}
+
+env-mcg_cap = mcg_cap;
}
#endif
--
1.7.1
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
Am 14.02.2011 20:34, schrieb Anthony Liguori:
On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
On Mon, 14 Feb 2011 08:39:11 -0600
Anthony Liguorianth...@codemonkey.ws wrote:
On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
So the question is: how does the schema based design support extending
commands or events? Does it require adding new commands/events?
Well, let me ask you, how do we do that today?
Let's say that I want to add a new parameter to the `change' function so
that I can include a salt parameter as part of the password.
The way we'd do this today is by checking for the 'salt' parameter in
qdict, and if it's not present, use a random salt or something like that.
You likely want to do what you did before. Of course that you have to
consider if what you're doing is extending an existing command or badly
overloading it (like change is today), in this case you'll want to add
a new command instead.
But yes, the use-case here is extending an existing command.
However, if I'm a QMP client, how can I tell whether you're going to
ignore my salt parameter or actually use it? Nothing in QMP tells me
this today. If I set the salt parameter in the `change' command, I'll
just get a success message.
I'm sorry?
{ execute: change, arguments: { device: vnc, target: password,
arg: 1234, salt: r1 } }
{error: {class: InvalidParameter, desc: Invalid parameter 'salt',
data: {name: salt}}}
So I'm supposed to execute the command, and if execution fails, drop the
new parameter? If we add a few optional parameters, does that mean I
have to try every possible combination of parameters?
How is that different from trying out multiple commands? In the end, you
always need some meta information like a schema in order to avoid trying
out which parameters the server supports.
Anyway, I think there's a second interesting point: Adding parameters
does cause these problems, but it's different for data sent from qemu to
the client (return values and events). If we add more information there,
an older client can just ignore it, without even looking at a schema.
So I think we should consider this for return values and definitely do
it for events. Sending out five different messages for a single event
that are completely redundant and only differ in the number of fields is
just insane (okay, they wouldn't actually get on the wire because a
client registers only for one of them, but the code for generating them
must exist).
You're arguing that we should extend commands by adding new parameters.
I'm saying that's a bad interface. If we need to change a command, we
should introduce a new command. It's a well understood mechanism for
maintaining compatibility (just about every C library does exactly this).
I'm yet undecided about adding parameters. I have a feeling that you
might be right here.
Kevin
Re: [Qemu-devel] [PATCH 1/6] target-arm: Fix rounding constant addition for Neon shift instructions.
On 14.02.2011 19:12, Peter Maydell wrote:
On 11 February 2011 15:10, christophe.l...@st.com wrote:
+uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
+{
+int32_t dest;
+int32_t val = (int32_t)valop;
+int8_t shift = (int8_t)shiftop;
+if (shift = 32) {
+dest = 0;
+} else if (shift -32) {
+dest = val 31;
This is the wrong answer: large rounding right shifts give zero.
+} else if (shift == -32) {
+dest = val 31;
+dest++;
+dest = 1;
These three lines will always result in dest becoming
0 regardless of the input value.
You are right. Actually, I just intended to fix the case where
-32 shift 0, and merely re-instanciated the preceding macro with a known
size of 32.
You comments also apply to the 8 and 16 bits variants in that macro.
I am too respectful of existing code :-)
Christophe.
Re: [Qemu-devel] [PATCH 06/10] vmmouse: convert to qdev
Anthony Liguori anth...@codemonkey.ws writes: On 02/12/2011 11:03 AM, Markus Armbruster wrote: Blue Swirlblauwir...@gmail.com writes: Convert to qdev, also add a proper reset function. [...] Pointer properties are for dirty hacks only. Is there really no better solution? Why does it have to be a property? vmmouse is really just an extension to the PS2 Mouse. It's definitely not an ISA device. In terms of qdev enablement, I would just make it a boolean option to the PS2Mouse and not expose it as a top level device at all. It cannot exist without a PS2Mouse. Which means making it a separate qdev is wrong. That wrongness gave rise to the dirty pointer property. Pointer property serves as canary again. What now? PS: Grumpy reviewer venting: review can keep such mistakes out of the code, but it got committed less than two days after it was posted. That, and the lack of proper reference headers bounced it several places down my review queue.
[Qemu-devel] [PATCH] tracetool: Add optional argument to specify dtrace probe names
From: Jes Sorensen jes.soren...@redhat.com
Optional feature allowing a user to generate the probe list to match
the name of the binary, in case they wish to install qemu under a
different name than qemu-{system,user},arch
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
scripts/tracetool | 14 ++
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/scripts/tracetool b/scripts/tracetool
index e046683..0200afa 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -33,6 +33,7 @@ Options:
--binary [path] Full path to QEMU binary
--target-arch [arch] QEMU emulator target arch
--target-type [type] QEMU emulator target type ('system' or 'user')
+ --probe-name [name] Specify alternative name for dtrace probes
EOF
exit 1
@@ -472,7 +473,7 @@ linetostap_dtrace()
# Define prototype for probe arguments
cat EOF
-probe qemu.$targettype.$targetarch.$name = process($binary).mark($name)
+probe $probename.$name = process($binary).mark($name)
{
EOF
@@ -570,18 +571,21 @@ tracetostap()
echo SystemTAP tapset generator not applicable to $backend backend
exit 1
fi
-if [ -z $binary ]; then
+if [ -z $probename -a -z $binary ]; then
echo --binary is required for SystemTAP tapset generator
exit 1
fi
-if [ -z $targettype ]; then
+if [ -z $probename -a -z $targettype ]; then
echo --target-type is required for SystemTAP tapset generator
exit 1
fi
-if [ -z $targetarch ]; then
+if [ -z $probename -a -z $targetarch ]; then
echo --target-arch is required for SystemTAP tapset generator
exit 1
fi
+if [ -z $probename ]; then
+ probename=qemu.$targettype.$targetarch;
+fi
echo /* This file is autogenerated by tracetool, do not edit. */
convert stap
}
@@ -592,6 +596,7 @@ output=
binary=
targettype=
targetarch=
+probename=
until [ -z $1 ]
@@ -602,6 +607,7 @@ do
--binary) shift ; binary=$1 ;;
--target-arch) shift ; targetarch=$1 ;;
--target-type) shift ; targettype=$1 ;;
+--probe-name) shift ; probename=$1 ;;
-h | -c | -d) output=${1#-} ;;
--stap) output=${1#--} ;;
--
1.7.4
Re: [Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr
On 15 February 2011 08:59, Tristan Gingold ging...@adacore.com wrote: @@ -770,7 +770,8 @@ static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n) /* fpscr */ if (gdb_has_xml) return 0; - return 4; + env-fpscr = ldtul_p(mem_buf); + return sizeof(target_ulong); } } return 0; Not a PPC expert, but this doesn't look right; for instance if you change the rounding mode by fiddling with the FPSCR in the debugger this won't update the softfloat rounding mode settings. (that is, it lets the visible state in env-fpscr get out of sync with the hidden state of the model). Also we probably shouldn't be letting the debugger change reserved fpscr bits. (Side note: linux-user/signal.c:restore_user_regs() appears to have a similar fpscr-related bug.) -- PMM
Re: [Qemu-devel] [PATCH 05/10] vmport: convert to qdev
Blue Swirl blauwir...@gmail.com writes:
On Sat, Feb 12, 2011 at 6:57 PM, Markus Armbruster arm...@redhat.com wrote:
Blue Swirl blauwir...@gmail.com writes:
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
hw/pc.h | 1 -
hw/pc_piix.c | 2 --
hw/vmport.c | 24 +---
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/hw/pc.h b/hw/pc.h
index a048768..603a2a3 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -65,7 +65,6 @@ void hpet_pit_disable(void);
void hpet_pit_enable(void);
/* vmport.c */
-void vmport_init(void);
void vmport_register(unsigned char command, IOPortReadFunc *func,
void *opaque);
/* vmmouse.c */
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7b74473..d0bd0cd 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -86,8 +86,6 @@ static void pc_init1(ram_addr_t ram_size,
pc_cpus_init(cpu_model);
- vmport_init();
-
/* allocate ram and load rom/bios */
pc_memory_init(ram_size, kernel_filename, kernel_cmdline,
initrd_filename,
below_4g_mem_size, above_4g_mem_size);
diff --git a/hw/vmport.c b/hw/vmport.c
index 6c9d7c9..4432be0 100644
--- a/hw/vmport.c
+++ b/hw/vmport.c
@@ -26,6 +26,7 @@
#include pc.h
#include sysemu.h
#include kvm.h
+#include qdev.h
//#define VMPORT_DEBUG
@@ -37,6 +38,7 @@
typedef struct _VMPortState
{
+ ISADevice dev;
IOPortReadFunc *func[VMPORT_ENTRIES];
void *opaque[VMPORT_ENTRIES];
} VMPortState;
@@ -100,12 +102,28 @@ static uint32_t vmport_cmd_ram_size(void
*opaque, uint32_t addr)
return ram_size;
}
-void vmport_init(void)
+static int vmport_initfn(ISADevice *dev)
{
- register_ioport_read(0x5658, 1, 4, vmport_ioport_read, port_state);
- register_ioport_write(0x5658, 1, 4, vmport_ioport_write, port_state);
+ VMPortState *s = DO_UPCAST(VMPortState, dev, dev);
+ register_ioport_read(0x5658, 1, 4, vmport_ioport_read, s);
+ register_ioport_write(0x5658, 1, 4, vmport_ioport_write, s);
+ isa_init_ioport(dev, 0x5658);
/* Register some generic port commands */
vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, NULL);
vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size, NULL);
+ return 0;
}
+
+static ISADeviceInfo vmport_info = {
+ .qdev.name = vmport,
+ .qdev.size = sizeof(VMPortState),
+ .qdev.no_user = 1,
+ .init = vmport_initfn,
+};
+
+static void vmport_dev_register(void)
+{
+ isa_qdev_register(vmport_info);
+}
+device_init(vmport_dev_register)
Old code has pc_init1() call vmport_init(). Where does your code create
qdev vmport? And what's happening with port_state? It's still used
by vmport_register(), but no longer connected to the I/O ports. Can't
see how vmport_register() has any effect anymore.
I fixed it in the committed version.
Did you post v2 to the list for review?
Have you tested this?
Sure.
Here's how your v2 creates and initializes the qdev:
diff --git a/hw/pc.c b/hw/pc.c
index fcee09a..c698161 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1133,6 +1133,7 @@ void pc_basic_device_init(qemu_irq *isa_irq,
a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
i8042 = isa_create_simple(i8042);
i8042_setup_a20_line(i8042, a20_line[0]);
+vmport_init();
vmmouse_init(i8042);
port92 = isa_create_simple(port92);
port92_init(port92, a20_line[1]);
This allocates a new VMPortState, and registers callbacks for port 5658:
@@ -100,12 +102,28 @@ static uint32_t vmport_cmd_ram_size(void *opaque,
uint32_t addr)
return ram_size;
}
-void vmport_init(void)
+static int vmport_initfn(ISADevice *dev)
{
-register_ioport_read(0x5658, 1, 4, vmport_ioport_read, port_state);
-register_ioport_write(0x5658, 1, 4, vmport_ioport_write, port_state);
+VMPortState *s = DO_UPCAST(VMPortState, dev, dev);
+register_ioport_read(0x5658, 1, 4, vmport_ioport_read, s);
+register_ioport_write(0x5658, 1, 4, vmport_ioport_write, s);
+isa_init_ioport(dev, 0x5658);
/* Register some generic port commands */
vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, NULL);
vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size, NULL);
+return 0;
}
The callbacks receive the qdev VMPortState as argument.
vmport_register() still updates global port_state. I.e. it no longer
has any effect whatsoever on what the ports do.
Maybe I'm dense, but I can't see how this can work.
Re: [Qemu-devel] [PATCH] gdbstub/ppc: handle read and write of fpscr
On Feb 15, 2011, at 11:22 AM, Peter Maydell wrote: On 15 February 2011 08:59, Tristan Gingold ging...@adacore.com wrote: @@ -770,7 +770,8 @@ static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n) /* fpscr */ if (gdb_has_xml) return 0; -return 4; +env-fpscr = ldtul_p(mem_buf); +return sizeof(target_ulong); } } return 0; Not a PPC expert, but this doesn't look right; for instance if you change the rounding mode by fiddling with the FPSCR in the debugger this won't update the softfloat rounding mode settings. (that is, it lets the visible state in env-fpscr get out of sync with the hidden state of the model). Also we probably shouldn't be letting the debugger change reserved fpscr bits. Indeed, you're right. We initially were interested in reading fpscr, and I wrote the writing part without thinking enough. Tristan.
Re: [Qemu-devel] KVM call agenda for Feb 15
On (Mon) 14 Feb 2011 [16:18:10], Anthony Liguori wrote: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. -rc2 is tagged and waiting for announcement. Please take a look at -rc2 and make sure there is nothing critical missing. Will tag 0.14.0 very late tomorrow but unless there's something critical, it'll be 0.14.0-rc2 with an updated version. The discussion on new machine migration - old machine migration trailed off. The virtio-serial compat patches for older machine types (pc-0.13, pc-0.12) can be picked if that's desirable. (btw since I maintain virtio-serial, I can take care of any such issues. Taking care of these issues for all machine types can be daunting, but we've got to start somewhere.) Amit
Re: [Qemu-devel] KVM call agenda for Feb 15
On 02/15/2011 12:18 AM, Anthony Liguori wrote: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. -rc2 is tagged and waiting for announcement. Please take a look at -rc2 and make sure there is nothing critical missing. Will tag 0.14.0 very late tomorrow but unless there's something critical, it'll be 0.14.0-rc2 with an updated version. Aren't you going to let users play with rc2 for a week or so? If not, what's the point? just push out 0.14.0. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
Make smpboot available not only for Linux but for all setups.
Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
hw/arm_boot.c | 17 +
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 620550b..a68b396 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -268,16 +268,17 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info
*info)
}
rom_add_blob_fixed(bootloader, bootloader, sizeof(bootloader),
info-loader_start);
-if (info-nb_cpus 1) {
-smpboot[10] = info-smp_priv_base;
-for (n = 0; n sizeof(smpboot) / 4; n++) {
-smpboot[n] = tswap32(smpboot[n]);
-}
-rom_add_blob_fixed(smpboot, smpboot, sizeof(smpboot),
- info-smp_loader_start);
-}
info-initrd_size = initrd_size;
}
+
+if (info-nb_cpus 1) {
+smpboot[10] = info-smp_priv_base;
+for (n = 0; n sizeof(smpboot) / 4; n++) {
+smpboot[n] = tswap32(smpboot[n]);
+}
+rom_add_blob_fixed(smpboot, smpboot, sizeof(smpboot),
+ info-smp_loader_start);
+}
info-is_linux = is_linux;
qemu_register_reset(main_cpu_reset, env);
}
--
1.7.2.3
Adam
--
Adam a...@os.inf.tu-dresden.de
Lackorzynski http://os.inf.tu-dresden.de/~adam/
[Qemu-devel] [PATCH 2/3] target-arm: Fix soft interrupt in GIC distributor
Fix selection of target list filter mode. Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de --- hw/arm_gic.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/arm_gic.c b/hw/arm_gic.c index e6b1953..0e934ec 100644 --- a/hw/arm_gic.c +++ b/hw/arm_gic.c @@ -549,10 +549,10 @@ static void gic_dist_writel(void *opaque, target_phys_addr_t offset, mask = (value 16) ALL_CPU_MASK; break; case 1: -mask = 1 cpu; +mask = ALL_CPU_MASK ^ (1 cpu); break; case 2: -mask = ALL_CPU_MASK ^ (1 cpu); +mask = 1 cpu; break; default: DPRINTF(Bad Soft Int target filter\n); -- 1.7.2.3 Adam -- Adam a...@os.inf.tu-dresden.de Lackorzynski http://os.inf.tu-dresden.de/~adam/
[Qemu-devel] [PATCH 3/3] target-arm: Implement cp15 VA-PA translation
Implement VA-PA translations by cp15-c7 that went through unchanged
previously.
Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
target-arm/cpu.h|1 +
target-arm/helper.c | 51 +--
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c9febfa..603574b 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -126,6 +126,7 @@ typedef struct CPUARMState {
uint32_t c6_region[8]; /* MPU base/size registers. */
uint32_t c6_insn; /* Fault address registers. */
uint32_t c6_data;
+uint32_t c7_par; /* Translation result. */
uint32_t c9_insn; /* Cache lockdown registers. */
uint32_t c9_data;
uint32_t c13_fcse; /* FCSE PID. */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7f63a28..32cc795 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1456,8 +1456,52 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn,
uint32_t val)
case 7: /* Cache control. */
env-cp15.c15_i_max = 0x000;
env-cp15.c15_i_min = 0xff0;
-/* No cache, so nothing to do. */
-/* ??? MPCore has VA to PA translation functions. */
+/* No cache, so nothing to do except VA-PA translations. */
+if (arm_feature(env, ARM_FEATURE_V6)) {
+switch (crm) {
+case 4:
+env-cp15.c7_par = val;
+break;
+case 8: {
+uint32_t phys_addr;
+target_ulong page_size;
+int prot;
+int ret, is_user;
+int access_type;
+
+switch (op2) {
+case 0: /* priv read */
+is_user = 0;
+access_type = 0;
+break;
+case 1: /* priv write */
+is_user = 0;
+access_type = 1;
+break;
+case 2: /* user read */
+is_user = 1;
+access_type = 0;
+break;
+case 3: /* user write */
+is_user = 1;
+access_type = 1;
+break;
+default: /* 4-7 are only available with TZ */
+goto bad_reg;
+}
+ret = get_phys_addr_v6(env, val, access_type, is_user,
+ phys_addr, prot, page_size);
+if (ret == 0) {
+env-cp15.c7_par = phys_addr;
+if (page_size TARGET_PAGE_SIZE)
+env-cp15.c7_par |= 1 1;
+} else {
+env-cp15.c7_par = ret | 1;
+}
+break;
+}
+}
+}
break;
case 8: /* MMU TLB control. */
switch (op2) {
@@ -1789,6 +1833,9 @@ uint32_t HELPER(get_cp15)(CPUState *env, uint32_t insn)
}
}
case 7: /* Cache control. */
+if (crm == 4 op2 == 0) {
+return env-cp15.c7_par;
+}
/* FIXME: Should only clear Z flag if destination is r15. */
env-ZF = 0;
return 0;
--
1.7.2.3
Adam
--
Adam a...@os.inf.tu-dresden.de
Lackorzynski http://os.inf.tu-dresden.de/~adam/
Re: [Qemu-devel] NBD block device backend - 'improvements'
Am 14.02.2011 21:32, schrieb Stefan Hajnoczi: On Mon, Feb 14, 2011 at 7:40 PM, Nicholas Thomas n...@lupine.me.uk wrote: I've written a patch that changes the behaviour - instead of exiting at startup, we wait for the NBD connection to be established, and we hang on reads and writes until the connection is re-established. Hi Nick, I think reconnect is a useful feature. For more info on submitting patches to QEMU, please see http://wiki.qemu.org/Contribute/SubmitAPatch. It contains a few points like sending patches inline instead of as an attachment (makes review easier), using Signed-off-by:, and references to QEMU coding style. I'm interested in getting the changes merged upstream, so I thought I'd get in early and ask if you'd be interested in the patch, in principle; whether the old behaviour would need to be preserved, making the new behaviour accessible via a config option (-drive file=nbd:127.0.0.1:5000:retry=forever,... ?); and whether I'm going about the changes in a sane way (I've attached the current version of the patch). block/nbd.c needs to be made asynchronous in order for this change to work. And even then it's not free of problem: For example qemu_aio_flush() will hang. We're having all kinds of fun with NFS servers that go away and let requests hang indefinitely. So maybe what we should add is a timeout option which defaults to 0 (fail immediately, like today) Otherwise the only thing you can do is to block QEMU and the VM, and even that shouldn't be done using sleep(2) because that could stop the I/O thread which processes the QEMU monitor and other external interfaces. See below for more info. Unconditionally stopping the VM from a block driver sounds wrong to me. If you want to have this behaviour, the block driver should return an error and you should use werror=stop. Another thing I've noticed is that the nbd library ( /nbd.c ) is not IPv6-compatible ( -drive file=nbd:\:\:1:5000, for instance ) - I don't have a patch for that yet, but I'm going to need to write one :) - presumably you'd like that merging upstream too (and I should make the library use the functions provided in qemu_socket.h) ? IPv6 would be nice and if you can consolidate that in qemu_socket.h, then that's a win for non-nbd socket users too. Agreed. Kevin
[Qemu-devel] [FYI] memory leak in 0.14.0rc1 ?
Hi, perhaps this is the wrong list. If so then please accept my apology. I am trying out the opensuse kvm rpm from the Virtualization repository: http://download.opensuse.org/repositories/Virtualization/openSUSE_11.3/x86_64/kvm-0.14.0.rc1-67.1.x86_64.rpm I have installed winxp and run the machine as /usr/bin/qemu-kvm -name xp.home -m 768 On the machine I have compiled perl from the sources. After a fresh start, when I try to run the perl test suite the process size grows to ~2500 Mb with an RSS of 2.2 Gb. On a second run of the test suite the process size and RSS continue to grow. I'd expect the memory consumption to be limited somewhere between 768 Mb and 1 Gb. See also https://bugzilla.novell.com/show_bug.cgi?id=671809 Torsten Förtsch
[Qemu-devel] Re: [PATCH] tracetool: Add optional argument to specify dtrace probe names
On Tue, Feb 15, 2011 at 11:10:22AM +0100, jes.soren...@redhat.com wrote:
From: Jes Sorensen jes.soren...@redhat.com
Optional feature allowing a user to generate the probe list to match
the name of the binary, in case they wish to install qemu under a
different name than qemu-{system,user},arch
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
scripts/tracetool | 14 ++
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/scripts/tracetool b/scripts/tracetool
index e046683..0200afa 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -33,6 +33,7 @@ Options:
--binary [path] Full path to QEMU binary
--target-arch [arch] QEMU emulator target arch
--target-type [type] QEMU emulator target type ('system' or 'user')
+ --probe-name [name] Specify alternative name for dtrace probes
It's not obvious to me what --probe-name does given this description.
How about:
--probe-prefix [prefix] Prefix for dtrace probe names (default:
qemu-$targettype-$targetarch)
Stefan
Re: [Qemu-devel] KVM call agenda for Feb 15
Anthony Liguori anth...@codemonkey.ws writes: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. - QAPI and QMP events - qdev future Do we need a qdev tree and a maintainer for it? I don't really have a coherent plan for the second one yet so let's just discuss this as time permits.
[Qemu-devel] [PATCH v2] tracetool: Add optional argument to specify dtrace probe names
From: Jes Sorensen jes.soren...@redhat.com
Optional feature allowing a user to generate the probe list to match
the name of the binary, in case they wish to install qemu under a
different name than qemu-{system,user},arch
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
scripts/tracetool | 21 ++---
1 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/scripts/tracetool b/scripts/tracetool
index e046683..b4d74ec 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -30,9 +30,11 @@ Output formats:
--stap Generate .stp file (DTrace with SystemTAP only)
Options:
- --binary [path] Full path to QEMU binary
- --target-arch [arch] QEMU emulator target arch
- --target-type [type] QEMU emulator target type ('system' or 'user')
+ --binary [path]Full path to QEMU binary
+ --target-arch [arch]QEMU emulator target arch
+ --target-type [type]QEMU emulator target type ('system' or 'user')
+ --probe-prefix [prefix] Prefix for dtrace probe names
+ (default: qemu-\$targettype-\$targetarch)
EOF
exit 1
@@ -472,7 +474,7 @@ linetostap_dtrace()
# Define prototype for probe arguments
cat EOF
-probe qemu.$targettype.$targetarch.$name = process($binary).mark($name)
+probe $probeprefix.$name = process($binary).mark($name)
{
EOF
@@ -570,18 +572,21 @@ tracetostap()
echo SystemTAP tapset generator not applicable to $backend backend
exit 1
fi
-if [ -z $binary ]; then
+if [ -z $probeprefix -a -z $binary ]; then
echo --binary is required for SystemTAP tapset generator
exit 1
fi
-if [ -z $targettype ]; then
+if [ -z $probeprefix -a -z $targettype ]; then
echo --target-type is required for SystemTAP tapset generator
exit 1
fi
-if [ -z $targetarch ]; then
+if [ -z $probeprefix -a -z $targetarch ]; then
echo --target-arch is required for SystemTAP tapset generator
exit 1
fi
+if [ -z $probeprefix ]; then
+ probeprefix=qemu.$targettype.$targetarch;
+fi
echo /* This file is autogenerated by tracetool, do not edit. */
convert stap
}
@@ -592,6 +597,7 @@ output=
binary=
targettype=
targetarch=
+probeprefix=
until [ -z $1 ]
@@ -602,6 +608,7 @@ do
--binary) shift ; binary=$1 ;;
--target-arch) shift ; targetarch=$1 ;;
--target-type) shift ; targettype=$1 ;;
+--probe-prefix) shift ; probeprefix=$1 ;;
-h | -c | -d) output=${1#-} ;;
--stap) output=${1#--} ;;
--
1.7.4
Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
On 15 February 2011 10:48, Adam Lackorzynski a...@os.inf.tu-dresden.de wrote: Make smpboot available not only for Linux but for all setups. I'm not convinced about this. I think if you're providing a raw image for an SMP system (rather than a Linux kernel) then it's your job to provide an image which handles the bootup of the secondary CPUs, the same way it would be if you were providing a ROM image for real hardware. -- PMM
Re: [Qemu-devel] [PATCH v2] tracetool: Add optional argument to specify dtrace probe names
On Tue, Feb 15, 2011 at 12:34 PM, jes.soren...@redhat.com wrote:
From: Jes Sorensen jes.soren...@redhat.com
Optional feature allowing a user to generate the probe list to match
the name of the binary, in case they wish to install qemu under a
different name than qemu-{system,user},arch
Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
scripts/tracetool | 21 ++---
1 files changed, 14 insertions(+), 7 deletions(-)
Acked-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
On Tue Feb 15, 2011 at 13:01:08 +, Peter Maydell wrote:
On 15 February 2011 10:48, Adam Lackorzynski a...@os.inf.tu-dresden.de
wrote:
Make smpboot available not only for Linux but for all setups.
I'm not convinced about this. I think if you're providing a raw
image for an SMP system (rather than a Linux kernel) then it's
your job to provide an image which handles the bootup of the
secondary CPUs, the same way it would be if you were providing
a ROM image for real hardware.
Ok, this is one possibility. Another one would be something like this:
Subject: [PATCH] target-arm: Provide entry vector for non-linux systems
Non-Linux systems must provide their own code for secondary CPU boot-up.
We use the same entry point as on the first CPU.
Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
hw/realview.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/hw/realview.c b/hw/realview.c
index 6eb6c6a..574bc11 100644
--- a/hw/realview.c
+++ b/hw/realview.c
@@ -112,7 +112,11 @@ static void secondary_cpu_reset(void *opaque)
/* Set entry point for secondary CPUs. This assumes we're using
the init code from arm_boot.c. Real hardware resets all CPUs
the same. */
- env-regs[15] = SMP_BOOT_ADDR;
+ if (realview_binfo.is_linux) {
+ env-regs[15] = SMP_BOOT_ADDR;
+ } else {
+ env-regs[15] = realview_binfo.entry;
+ }
}
/* The following two lists must be consistent. */
--
1.7.2.3
Adam
--
Adam a...@os.inf.tu-dresden.de
Lackorzynski http://os.inf.tu-dresden.de/~adam/
[Qemu-devel] [PATCH 1/2] pxa2xx_keypad: enhance emulation of KPAS, KPASMKP regs
Add emulation of KPAS register and proper emulation of
KPASMKP regs, so now driver supports multipresses and properly
works with Linux driver.
Signed-off-by: Vasily Khoruzhick anars...@gmail.com
---
hw/pxa2xx_keypad.c | 114
1 files changed, 62 insertions(+), 52 deletions(-)
diff --git a/hw/pxa2xx_keypad.c b/hw/pxa2xx_keypad.c
index 4c99917..5b8890b 100644
--- a/hw/pxa2xx_keypad.c
+++ b/hw/pxa2xx_keypad.c
@@ -82,22 +82,39 @@
struct PXA2xxKeyPadState {
qemu_irqirq;
struct keymap *map;
+int pressed_cnt;
uint32_tkpc;
uint32_tkpdk;
uint32_tkprec;
uint32_tkpmk;
uint32_tkpas;
-uint32_tkpasmkp0;
-uint32_tkpasmkp1;
-uint32_tkpasmkp2;
-uint32_tkpasmkp3;
+uint32_tkpasmkp[4];
uint32_tkpkdi;
};
+static void pxa27x_keypad_find_pressed_key(PXA2xxKeyPadState *kp, int *row,
int *col)
+{
+int i;
+for (i = 0; i 4; i++)
+{
+*col = i * 2;
+for (*row = 0; *row 8; (*row)++) {
+if (kp-kpasmkp[i] (1 *row))
+return;
+}
+*col = i * 2 + 1;
+for (*row = 0; *row 8; (*row)++) {
+if (kp-kpasmkp[i] (1 (*row + 16)))
+return;
+}
+}
+}
+
static void pxa27x_keyboard_event (PXA2xxKeyPadState *kp, int keycode)
{
-int row, col,rel;
+int row, col, rel, assert_irq = 0;
+uint32_t val;
if(!(kp-kpc KPC_ME)) /* skip if not enabled */
return;
@@ -108,46 +125,39 @@ static void pxa27x_keyboard_event (PXA2xxKeyPadState *kp,
int keycode)
rel = (keycode 0x80) ? 1 : 0; /* key release from qemu */
keycode = ~(0x80); /* strip qemu key release bit */
+
row = kp-map[keycode].row;
col = kp-map[keycode].column;
if(row == -1 || col == -1)
return;
-switch (col) {
-case 0:
-case 1:
-if(rel)
-kp-kpasmkp0 = ~(0x);
-else
-kp-kpasmkp0 |= KPASMKPx_MKC(row,col);
-break;
-case 2:
-case 3:
-if(rel)
-kp-kpasmkp1 = ~(0x);
-else
-kp-kpasmkp1 |= KPASMKPx_MKC(row,col);
-break;
-case 4:
-case 5:
-if(rel)
-kp-kpasmkp2 = ~(0x);
-else
-kp-kpasmkp2 |= KPASMKPx_MKC(row,col);
-break;
-case 6:
-case 7:
-if(rel)
-kp-kpasmkp3 = ~(0x);
-else
-kp-kpasmkp3 |= KPASMKPx_MKC(row,col);
-break;
-} /* switch */
+
+val = KPASMKPx_MKC(row, col);
+if (rel) {
+if (kp-kpasmkp[col / 2] val) {
+kp-kpasmkp[col / 2] = ~val;
+kp-pressed_cnt--;
+assert_irq = 1;
+}
+} else {
+if (!(kp-kpasmkp[col / 2] val)) {
+kp-kpasmkp[col / 2] |= val;
+kp-pressed_cnt++;
+assert_irq = 1;
+}
+}
+kp-kpas = ((kp-pressed_cnt 0x1f) 26) | (0xf 4) | 0xf;
+if (kp-pressed_cnt == 1) {
+kp-kpas = ~((0xf 4) | 0xf);
+if (rel)
+pxa27x_keypad_find_pressed_key(kp, row, col);
+kp-kpas |= ((row 0xf) 4) | (col 0xf);
+}
goto out;
}
return;
out:
-if(kp-kpc KPC_MIE) {
+if (assert_irq (kp-kpc KPC_MIE)) {
kp-kpc |= KPC_MI;
qemu_irq_raise(kp-irq);
}
@@ -194,16 +204,16 @@ static uint32_t pxa2xx_keypad_read(void *opaque,
target_phys_addr_t offset)
return s-kpas;
break;
case KPASMKP0:
-return s-kpasmkp0;
+return s-kpasmkp[0];
break;
case KPASMKP1:
-return s-kpasmkp1;
+return s-kpasmkp[1];
break;
case KPASMKP2:
-return s-kpasmkp2;
+return s-kpasmkp[2];
break;
case KPASMKP3:
-return s-kpasmkp3;
+return s-kpasmkp[3];
break;
case KPKDI:
return s-kpkdi;
@@ -237,16 +247,16 @@ static void pxa2xx_keypad_write(void *opaque,
s-kpas = value;
break;
case KPASMKP0:
-s-kpasmkp0 = value;
+s-kpasmkp[0] = value;
break;
case KPASMKP1:
-s-kpasmkp1 = value;
+s-kpasmkp[1] = value;
break;
case KPASMKP2:
-s-kpasmkp2 = value;
+s-kpasmkp[2] = value;
break;
case KPASMKP3:
-s-kpasmkp3 = value;
+s-kpasmkp[3] = value;
break;
case KPKDI:
s-kpkdi = value;
@@ -278,10 +288,10 @@ static void pxa2xx_keypad_save(QEMUFile *f, void *opaque)
qemu_put_be32s(f, s-kprec);
qemu_put_be32s(f, s-kpmk);
qemu_put_be32s(f, s-kpas);
-qemu_put_be32s(f, s-kpasmkp0);
-qemu_put_be32s(f,
[Qemu-devel] [PATCH 2/2] pxa2xx_keypad: Handle 0xe0xx keycodes
Add handling of 0xe0xx keycodes to pxa2xx_driver.
Extended keycodes in keymap should be marked with most significant
bit set (i.e. 0x80). Without this patch it's not possible to handle
i.e. cursor keys.
Signed-off-by: Vasily Khoruzhick anars...@gmail.com
---
hw/pxa2xx_keypad.c | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/hw/pxa2xx_keypad.c b/hw/pxa2xx_keypad.c
index 5b8890b..d77dbf1 100644
--- a/hw/pxa2xx_keypad.c
+++ b/hw/pxa2xx_keypad.c
@@ -83,6 +83,7 @@ struct PXA2xxKeyPadState {
qemu_irqirq;
struct keymap *map;
int pressed_cnt;
+int alt_code;
uint32_tkpc;
uint32_tkpdk;
@@ -116,6 +117,11 @@ static void pxa27x_keyboard_event (PXA2xxKeyPadState *kp,
int keycode)
int row, col, rel, assert_irq = 0;
uint32_t val;
+if (keycode == 0xe0) {
+kp-alt_code = 1;
+return;
+}
+
if(!(kp-kpc KPC_ME)) /* skip if not enabled */
return;
@@ -125,6 +131,10 @@ static void pxa27x_keyboard_event (PXA2xxKeyPadState *kp,
int keycode)
rel = (keycode 0x80) ? 1 : 0; /* key release from qemu */
keycode = ~(0x80); /* strip qemu key release bit */
+if (kp-alt_code) {
+keycode |= 0x80;
+kp-alt_code = 0;
+}
row = kp-map[keycode].row;
col = kp-map[keycode].column;
--
1.7.4
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
On Mon, 14 Feb 2011 14:15:27 -0600 Anthony Liguori anth...@codemonkey.ws wrote: On 02/14/2011 01:58 PM, Luiz Capitulino wrote: No, of course not, our plan has always been to do this via an schema, the only reason we don't do this today is lack of time/help. Understood--I'm here to help now :-) We need to expose the schema, I'm not saying we shouldn't. But we don't today. You're arguing that we should extend commands by adding new parameters. Commands and events, you haven't commented on events yet and that seems a bit worse than commands. Events are just commands that never return a value and are posted. IOW: client - server message == command server - client message == event However, we limit events to have no return value and semantically, when an event is invoked, the message is only posted (we're not guaranteed that the client has seen it). The reason for the lack of symmetry is to simplify the programming model. If we had to wait for an event to be ack'd and receive a return value, it would involve a lot of ugly callback registrations. But beyond those few limitations, events and commands should be treated exactly the same IMHO. I disagree. Let's say we have added three new arguments to the command foo, and now we have foo1, foo2 and foo3. I'm a quite old distro and only have foo, which command should I backport? All of them? Only the latest? I can't see how easier this is. Backporting APIs will almost always suck. 1) if we have to add three arguments to a command, we've failed in our API design after an initial release Maybe. Still, having to add a new command because we wanted to make a simple extension seems a bit overkill to me. I'm not sure how common this is going to be, however when we discussed QMP originally there were a lot on emphasis on this kind of feature. What makes it pretty hard to work on this project is that we are always changing our mind in a number of details. 2) it depends on what level of functionality the distro needs. The more complicated we make the API, the harder it will be to write clients against it. If we have four ways to do the same thing (even if that's via one command or four separate ones), writing a client is going to suck no matter what. Yes, but I still do not see how easier it's going to be for a client writer if s/he has to choose among four commands that do the same thing. Please, note that I do see the internal benefits, as always, the disagreement is about the wire protocol. For C, yes. But one of the main goals of a high level protocol is to be language independent, isn't it? Yes, which means first-class support for a variety of languages with C being a very important one. Look, although I did _not_ check any code yet, your description of the QAPI looks really exciting. I'm not against it, what bothers me though is this number of small limitations we're imposing to the wire protocol. Why don't we make libqmp internal only? This way we're free to change it whatever we want. libqmp is a test of how easy it is to use QMP from an external application. If we can't keep libqmp stable, then that means tools like libvirt will always have a hard time using QMP. Proper C support is important. We cannot make it impossible to write a useful C client API. I wouldn't say it's impossible, but anyway, the important point here is that we disagree about the side effects QAPI is going to introduce in QMP, I don't know how to solve this, maybe we can discuss this upstream, but I'm not sure the situation will change much. Should we vote? :) Let's wait until I actually post patches... My purpose of this thread was to get some feedback on using signal/slots as an abstraction in QEMU. Ok. The QMP conversion is almost done, I'll be able to post patches very soon. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
On 15 February 2011 13:12, Adam Lackorzynski a...@os.inf.tu-dresden.de wrote:
On Tue Feb 15, 2011 at 13:01:08 +, Peter Maydell wrote:
On 15 February 2011 10:48, Adam Lackorzynski a...@os.inf.tu-dresden.de
wrote:
Make smpboot available not only for Linux but for all setups.
I'm not convinced about this. I think if you're providing a raw
image for an SMP system (rather than a Linux kernel) then it's
your job to provide an image which handles the bootup of the
secondary CPUs, the same way it would be if you were providing
a ROM image for real hardware.
Ok, this is one possibility. Another one would be something like this:
@@ -112,7 +112,11 @@ static void secondary_cpu_reset(void *opaque)
/* Set entry point for secondary CPUs. This assumes we're using
the init code from arm_boot.c. Real hardware resets all CPUs
the same. */
- env-regs[15] = SMP_BOOT_ADDR;
+ if (realview_binfo.is_linux) {
+ env-regs[15] = SMP_BOOT_ADDR;
+ } else {
+ env-regs[15] = realview_binfo.entry;
+ }
}
Moving in the right direction, but it would be cleaner if the secondary
CPU reset was handled inside arm_boot.c, I think (there is a TODO
in that file to that effect). Then we could get rid of the cpu reset
hook from realview.c.
-- PMM
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
On Tue, 15 Feb 2011 10:20:01 +0100
Kevin Wolf kw...@redhat.com wrote:
Am 14.02.2011 20:34, schrieb Anthony Liguori:
On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
On Mon, 14 Feb 2011 08:39:11 -0600
Anthony Liguorianth...@codemonkey.ws wrote:
On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
So the question is: how does the schema based design support extending
commands or events? Does it require adding new commands/events?
Well, let me ask you, how do we do that today?
Let's say that I want to add a new parameter to the `change' function so
that I can include a salt parameter as part of the password.
The way we'd do this today is by checking for the 'salt' parameter in
qdict, and if it's not present, use a random salt or something like that.
You likely want to do what you did before. Of course that you have to
consider if what you're doing is extending an existing command or badly
overloading it (like change is today), in this case you'll want to add
a new command instead.
But yes, the use-case here is extending an existing command.
However, if I'm a QMP client, how can I tell whether you're going to
ignore my salt parameter or actually use it? Nothing in QMP tells me
this today. If I set the salt parameter in the `change' command, I'll
just get a success message.
I'm sorry?
{ execute: change, arguments: { device: vnc, target:
password, arg: 1234, salt: r1 } }
{error: {class: InvalidParameter, desc: Invalid parameter
'salt', data: {name: salt}}}
So I'm supposed to execute the command, and if execution fails, drop the
new parameter? If we add a few optional parameters, does that mean I
have to try every possible combination of parameters?
How is that different from trying out multiple commands? In the end, you
always need some meta information like a schema in order to avoid trying
out which parameters the server supports.
Anyway, I think there's a second interesting point: Adding parameters
does cause these problems, but it's different for data sent from qemu to
the client (return values and events). If we add more information there,
an older client can just ignore it, without even looking at a schema.
So I think we should consider this for return values and definitely do
it for events. Sending out five different messages for a single event
that are completely redundant and only differ in the number of fields is
just insane (okay, they wouldn't actually get on the wire because a
client registers only for one of them, but the code for generating them
must exist).
That's my point when I asked about events in the other thread.
You're arguing that we should extend commands by adding new parameters.
I'm saying that's a bad interface. If we need to change a command, we
should introduce a new command. It's a well understood mechanism for
maintaining compatibility (just about every C library does exactly this).
I'm yet undecided about adding parameters. I have a feeling that you
might be right here.
Kevin
Re: [Qemu-devel] KVM call agenda for Feb 15
On Mon, Feb 14, 2011 at 01:54:41PM -0600, Anthony Liguori wrote: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. - QAPI and QMP events - qdev future Making new Seabios stable release for qemu-0.14? -- Gleb.
[Qemu-devel] [PATCH 00/10] Fix Neon shift instructions
This patch series fixes bugs in the Neon shift instructions VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN, VQSHRN, VSHRN, VQSHRUN, VRSHL, and VQRSHL. It is based on the v3 patchset Christophe sent recently, with some fixes for minor nits in those patches, plus some patches from me which fix problems with shifts by large shift counts and an issue with overlapping source and destination registers. With this patchset qemu passes random instruction sequence testing for all these instruction patterns. Christophe Lyon (5): target-arm: Fix rounding constant addition for Neon shifts target-arm: Fix unsigned VRSHL.s8 and .s16 right shifts by type width target-arm: fix unsigned 64 bit right shifts. target-arm: fix Neon VQSHRN and VSHRN. target-arm: fix decoding of Neon 64 bit shifts. Peter Maydell (5): target-arm: Fix signed VRSHL by large shift counts target-arm: Fix saturated values for Neon right shifts target-arm: Fix signed VQRSHL by large shift counts target-arm: Fix unsigned VQRSHL by large shift counts target-arm: Fix shift by immediate and narrow where src,dest overlap target-arm/neon_helper.c | 232 +++--- target-arm/translate.c | 71 ++ 2 files changed, 250 insertions(+), 53 deletions(-)
[Qemu-devel] [PATCH 10/10] target-arm: Fix shift by immediate and narrow where src, dest overlap
For Neon shifts by immediate and narrow, correctly handle the case
where the source registers and the destination registers overlap
(the second pass should use the original register contents, not the
results of the first pass).
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/translate.c | 38 +++---
1 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index a02b20f..4d5d305 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4839,31 +4839,47 @@ static int disas_neon_data_insn(CPUState * env,
DisasContext *s, uint32_t insn)
abort();
}
+if (size == 3) {
+neon_load_reg64(cpu_V0, rm);
+neon_load_reg64(cpu_V1, rm + 1);
+} else {
+tmp4 = neon_load_reg(rm + 1, 0);
+tmp5 = neon_load_reg(rm + 1, 1);
+}
for (pass = 0; pass 2; pass++) {
if (size == 3) {
-neon_load_reg64(cpu_V0, rm + pass);
+TCGv_i64 in;
+if (pass == 0) {
+in = cpu_V0;
+} else {
+in = cpu_V1;
+}
if (q) {
if (input_unsigned) {
-gen_helper_neon_rshl_u64(cpu_V0, cpu_V0,
- tmp64);
+gen_helper_neon_rshl_u64(cpu_V0, in, tmp64);
} else {
-gen_helper_neon_rshl_s64(cpu_V0, cpu_V0,
- tmp64);
+gen_helper_neon_rshl_s64(cpu_V0, in, tmp64);
}
} else {
if (input_unsigned) {
-gen_helper_neon_shl_u64(cpu_V0, cpu_V0,
-tmp64);
+gen_helper_neon_shl_u64(cpu_V0, in, tmp64);
} else {
-gen_helper_neon_shl_s64(cpu_V0, cpu_V0,
-tmp64);
+gen_helper_neon_shl_s64(cpu_V0, in, tmp64);
}
}
} else {
-tmp = neon_load_reg(rm + pass, 0);
+if (pass == 0) {
+tmp = neon_load_reg(rm, 0);
+} else {
+tmp = tmp4;
+}
gen_neon_shift_narrow(size, tmp, tmp2, q,
input_unsigned);
-tmp3 = neon_load_reg(rm + pass, 1);
+if (pass == 0) {
+tmp3 = neon_load_reg(rm, 1);
+} else {
+tmp3 = tmp5;
+}
gen_neon_shift_narrow(size, tmp3, tmp2, q,
input_unsigned);
tcg_gen_concat_i32_i64(cpu_V0, tmp, tmp3);
--
1.7.1
[Qemu-devel] [PATCH 03/10] target-arm: Fix unsigned VRSHL.s8 and .s16 right shifts by type width
From: Christophe Lyon christophe.l...@st.com
Fix handling of unsigned VRSHL.s8 and .s16 right shifts by the type
width.
Signed-off-by: Christophe Lyon christophe.l...@st.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/neon_helper.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index f692640..2930b5e 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -605,7 +605,7 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
shiftop)
tmp -(ssize_t)sizeof(src1) * 8) { \
dest = 0; \
} else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
-dest = src1 (tmp - 1); \
+dest = src1 (-tmp - 1); \
} else if (tmp 0) { \
dest = (src1 + (1 (-1 - tmp))) -tmp; \
} else { \
--
1.7.1
[Qemu-devel] [PATCH 05/10] target-arm: Fix saturated values for Neon right shifts
Fix value returned by signed 8 and 16 bit qrshl helpers
when the result has saturated.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/neon_helper.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index a8885fa..7859f0b 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -886,7 +886,10 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t
val, uint64_t shiftop)
dest = src1 tmp; \
if ((dest tmp) != src1) { \
SET_QC(); \
-dest = src1 31; \
+dest = (uint32_t)(1 (sizeof(src1) * 8 - 1)); \
+if (src1 0) { \
+dest--; \
+} \
} \
}} while (0)
NEON_VOP_ENV(qrshl_s8, neon_s8, 4)
--
1.7.1
[Qemu-devel] [PATCH 08/10] target-arm: Fix signed VQRSHL by large shift counts
Handle the case of signed VQRSHL by a shift count of the width of the
data type or larger, which must be special cased in the qrshl_s*
helper functions.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/neon_helper.c | 34 +++---
1 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 7859f0b..f8f6db6 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -880,7 +880,19 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t
val, uint64_t shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp 0) { \
+if (tmp = (ssize_t)sizeof(src1) * 8) { \
+if (src1) { \
+SET_QC(); \
+dest = (1 (sizeof(src1) * 8 - 1)); \
+if (src1 0) { \
+dest--; \
+} \
+} else { \
+dest = 0; \
+} \
+} else if (tmp = -(ssize_t)sizeof(src1) * 8) { \
+dest = 0; \
+} else if (tmp 0) { \
dest = (src1 + (1 (-1 - tmp))) -tmp; \
} else { \
dest = src1 tmp; \
@@ -903,7 +915,16 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t
valop, uint32_t shiftop)
int32_t dest;
int32_t val = (int32_t)valop;
int8_t shift = (int8_t)shiftop;
-if (shift 0) {
+if (shift = 32) {
+if (val) {
+SET_QC();
+dest = (val 31) ^ ~SIGNBIT;
+} else {
+dest = 0;
+}
+} else if (shift = -32) {
+dest = 0;
+} else if (shift 0) {
int64_t big_dest = ((int64_t)val + (1 (-1 - shift)));
dest = big_dest -shift;
} else {
@@ -923,7 +944,14 @@ uint64_t HELPER(neon_qrshl_s64)(CPUState *env, uint64_t
valop, uint64_t shiftop)
int8_t shift = (uint8_t)shiftop;
int64_t val = valop;
-if (shift 0) {
+if (shift = 64) {
+if (val) {
+SET_QC();
+val = (val 63) ^ ~SIGNBIT64;
+}
+} else if (shift = -64) {
+val = 0;
+} else if (shift 0) {
val = (-shift - 1);
if (val == INT64_MAX) {
/* In this case, it means that the rounding constant is 1,
--
1.7.1
[Qemu-devel] [PATCH 07/10] target-arm: fix decoding of Neon 64 bit shifts.
From: Christophe Lyon christophe.l...@st.com
Fix decoding of 64 bits variants of VSHRN, VRSHRN, VQSHRN, VQSHRUN,
VQRSHRN, VQRSHRUN, taking into account whether inputs are unsigned
or not.
Signed-off-by: Christophe Lyon christophe.l...@st.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/translate.c | 45 ++---
1 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index bd26b1b..a02b20f 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4815,6 +4815,8 @@ static int disas_neon_data_insn(CPUState * env,
DisasContext *s, uint32_t insn)
} else if (op 10) {
/* Shift by immediate and narrow:
VSHRN, VRSHRN, VQSHRN, VQRSHRN. */
+int input_unsigned = (op == 8) ? !u : u;
+
shift = shift - (1 (size + 3));
size++;
switch (size) {
@@ -4841,33 +4843,46 @@ static int disas_neon_data_insn(CPUState * env,
DisasContext *s, uint32_t insn)
if (size == 3) {
neon_load_reg64(cpu_V0, rm + pass);
if (q) {
- if (u)
-gen_helper_neon_rshl_u64(cpu_V0, cpu_V0, tmp64);
- else
-gen_helper_neon_rshl_s64(cpu_V0, cpu_V0, tmp64);
+if (input_unsigned) {
+gen_helper_neon_rshl_u64(cpu_V0, cpu_V0,
+ tmp64);
+} else {
+gen_helper_neon_rshl_s64(cpu_V0, cpu_V0,
+ tmp64);
+}
} else {
- if (u)
-gen_helper_neon_shl_u64(cpu_V0, cpu_V0, tmp64);
- else
-gen_helper_neon_shl_s64(cpu_V0, cpu_V0, tmp64);
+if (input_unsigned) {
+gen_helper_neon_shl_u64(cpu_V0, cpu_V0,
+tmp64);
+} else {
+gen_helper_neon_shl_s64(cpu_V0, cpu_V0,
+tmp64);
+}
}
} else {
tmp = neon_load_reg(rm + pass, 0);
-gen_neon_shift_narrow(size, tmp, tmp2, q, u);
+gen_neon_shift_narrow(size, tmp, tmp2, q,
+ input_unsigned);
tmp3 = neon_load_reg(rm + pass, 1);
-gen_neon_shift_narrow(size, tmp3, tmp2, q, u);
+gen_neon_shift_narrow(size, tmp3, tmp2, q,
+ input_unsigned);
tcg_gen_concat_i32_i64(cpu_V0, tmp, tmp3);
dead_tmp(tmp);
dead_tmp(tmp3);
}
tmp = new_tmp();
-if (op == 8 !u) {
-gen_neon_narrow(size - 1, tmp, cpu_V0);
+if (op == 8) {
+if (u) { /* VQSHRUN / VQRSHRUN */
+gen_neon_unarrow_sats(size - 1, tmp, cpu_V0);
+} else { /* VSHRN / VRSHRN */
+gen_neon_narrow(size - 1, tmp, cpu_V0);
+}
} else {
-if (op == 8)
-gen_neon_narrow_sats(size - 1, tmp, cpu_V0);
-else
+if (u) { /* VQSHRN / VQRSHRN */
gen_neon_narrow_satu(size - 1, tmp, cpu_V0);
+} else { /* VQSHRN / VQRSHRN */
+gen_neon_narrow_sats(size - 1, tmp, cpu_V0);
+}
}
neon_store_reg(rd, pass, tmp);
} /* for pass */
--
1.7.1
[Qemu-devel] [PATCH 06/10] target-arm: fix Neon VQSHRN and VSHRN.
From: Christophe Lyon christophe.l...@st.com
Call the normal shift helpers instead of the rounding ones.
Signed-off-by: Christophe Lyon christophe.l...@st.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/translate.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 362d1d0..bd26b1b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4095,8 +4095,8 @@ static inline void gen_neon_shift_narrow(int size, TCGv
var, TCGv shift,
} else {
if (u) {
switch (size) {
-case 1: gen_helper_neon_rshl_u16(var, var, shift); break;
-case 2: gen_helper_neon_rshl_u32(var, var, shift); break;
+case 1: gen_helper_neon_shl_u16(var, var, shift); break;
+case 2: gen_helper_neon_shl_u32(var, var, shift); break;
default: abort();
}
} else {
--
1.7.1
[Qemu-devel] [PATCH 04/10] target-arm: fix unsigned 64 bit right shifts.
From: Christophe Lyon christophe.l...@st.com
Fix range of shift amounts which always give 0 as result.
Signed-off-by: Christophe Lyon christophe.l...@st.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/neon_helper.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 2930b5e..a8885fa 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -639,7 +639,7 @@ uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t
shiftop)
uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
-if (shift = 64 || shift 64) {
+if (shift = 64 || shift -64) {
val = 0;
} else if (shift == -64) {
/* Rounding a 1-bit result just preserves that bit. */
--
1.7.1
[Qemu-devel] [PATCH 01/10] target-arm: Fix rounding constant addition for Neon shifts
From: Christophe Lyon christophe.l...@st.com
Handle cases where adding the rounding constant could overflow in Neon
shift instructions: VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN.
Signed-off-by: Christophe Lyon christophe.l...@st.com
[peter.mayd...@linaro.org: fix handling of large shifts in rshl_s32,
calculate signed saturated value as other functions do.]
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/neon_helper.c | 139 ++
1 files changed, 127 insertions(+), 12 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index dc09968..cc63636 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -558,9 +558,28 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t
shiftop)
}} while (0)
NEON_VOP(rshl_s8, neon_s8, 4)
NEON_VOP(rshl_s16, neon_s16, 2)
-NEON_VOP(rshl_s32, neon_s32, 1)
#undef NEON_FN
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator. */
+uint32_t HELPER(neon_rshl_s32)(uint32_t valop, uint32_t shiftop)
+{
+int32_t dest;
+int32_t val = (int32_t)valop;
+int8_t shift = (int8_t)shiftop;
+if ((shift = 32) || (shift = -32)) {
+dest = 0;
+} else if (shift 0) {
+int64_t big_dest = ((int64_t)val + (1 (-1 - shift)));
+dest = big_dest -shift;
+} else {
+dest = val shift;
+}
+return dest;
+}
+
+/* Handling addition overflow with 64 bits inputs values is more
+ * tricky than with 32 bits values. */
uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
@@ -574,7 +593,16 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
shiftop)
val++;
val = 1;
} else if (shift 0) {
-val = (val + ((int64_t)1 (-1 - shift))) -shift;
+val = (-shift - 1);
+if (val == INT64_MAX) {
+/* In this case, it means that the rounding constant is 1,
+ * and the addition would overflow. Return the actual
+ * result directly. */
+val = 0x4000LL;
+} else {
+val++;
+val = 1;
+}
} else {
val = shift;
}
@@ -596,9 +624,29 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
shiftop)
}} while (0)
NEON_VOP(rshl_u8, neon_u8, 4)
NEON_VOP(rshl_u16, neon_u16, 2)
-NEON_VOP(rshl_u32, neon_u32, 1)
#undef NEON_FN
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator. */
+uint32_t HELPER(neon_rshl_u32)(uint32_t val, uint32_t shiftop)
+{
+uint32_t dest;
+int8_t shift = (int8_t)shiftop;
+if (shift = 32 || shift -32) {
+dest = 0;
+} else if (shift == -32) {
+dest = val 31;
+} else if (shift 0) {
+uint64_t big_dest = ((uint64_t)val + (1 (-1 - shift)));
+dest = big_dest -shift;
+} else {
+dest = val shift;
+}
+return dest;
+}
+
+/* Handling addition overflow with 64 bits inputs values is more
+ * tricky than with 32 bits values. */
uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t shiftop)
{
int8_t shift = (uint8_t)shiftop;
@@ -607,9 +655,17 @@ uint64_t HELPER(neon_rshl_u64)(uint64_t val, uint64_t
shiftop)
} else if (shift == -64) {
/* Rounding a 1-bit result just preserves that bit. */
val = 63;
-} if (shift 0) {
-val = (val + ((uint64_t)1 (-1 - shift))) -shift;
-val = -shift;
+} else if (shift 0) {
+val = (-shift - 1);
+if (val == UINT64_MAX) {
+/* In this case, it means that the rounding constant is 1,
+ * and the addition would overflow. Return the actual
+ * result directly. */
+val = 0x8000ULL;
+} else {
+val++;
+val = 1;
+}
} else {
val = shift;
}
@@ -784,14 +840,43 @@ uint64_t HELPER(neon_qshlu_s64)(CPUState *env, uint64_t
valop, uint64_t shiftop)
}} while (0)
NEON_VOP_ENV(qrshl_u8, neon_u8, 4)
NEON_VOP_ENV(qrshl_u16, neon_u16, 2)
-NEON_VOP_ENV(qrshl_u32, neon_u32, 1)
#undef NEON_FN
+/* The addition of the rounding constant may overflow, so we use an
+ * intermediate 64 bits accumulator. */
+uint32_t HELPER(neon_qrshl_u32)(CPUState *env, uint32_t val, uint32_t shiftop)
+{
+uint32_t dest;
+int8_t shift = (int8_t)shiftop;
+if (shift 0) {
+uint64_t big_dest = ((uint64_t)val + (1 (-1 - shift)));
+dest = big_dest -shift;
+} else {
+dest = val shift;
+if ((dest shift) != val) {
+SET_QC();
+dest = ~0;
+}
+}
+return dest;
+}
+
+/* Handling addition overflow with 64 bits inputs values is more
+ * tricky than with 32 bits values. */
uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
Re: [Qemu-devel] [PATCH 2/6] target-arm: fix Neon right shifts with shift amount == input width.
-dest = src1 (tmp - 1); \
+dest = src1 (-tmp - 1); \
dest++; \
dest = 1; \
Again, these three lines have the same effect as dest = 0,
so we can fold into the previous if().
} else if (tmp 0) { \
@@ -594,7 +594,7 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
shiftop)
val = 0;
} else if (shift -64) {
val = 63;
You didn't change this case, but it is the wrong answer:
should be 0.
-} else if (shift == -63) {
+} else if (shift == -64) {
val = 63;
val++;
val = 1;
Always results in 0.
Oops sorry, in these 3 cases, I just fixed obvious typos but didn't question
the actual code.
[Qemu-devel] [PATCH 02/10] target-arm: Fix signed VRSHL by large shift counts
Correctly handle VRSHL of signed values by a shift count of the
width of the data type or larger, which must be special-cased in the
rshl_s* helper functions.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/neon_helper.c | 17 +++--
1 files changed, 3 insertions(+), 14 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index cc63636..f692640 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -543,14 +543,9 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t
shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp = (ssize_t)sizeof(src1) * 8) { \
+if ((tmp = (ssize_t)sizeof(src1) * 8) \
+|| (tmp = -(ssize_t)sizeof(src1) * 8)) { \
dest = 0; \
-} else if (tmp -(ssize_t)sizeof(src1) * 8) { \
-dest = src1 (sizeof(src1) * 8 - 1); \
-} else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
-dest = src1 (tmp - 1); \
-dest++; \
-dest = 1; \
} else if (tmp 0) { \
dest = (src1 + (1 (-1 - tmp))) -tmp; \
} else { \
@@ -584,14 +579,8 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t
shiftop)
{
int8_t shift = (int8_t)shiftop;
int64_t val = valop;
-if (shift = 64) {
+if ((shift = 64) || (shift = -64)) {
val = 0;
-} else if (shift -64) {
-val = 63;
-} else if (shift == -63) {
-val = 63;
-val++;
-val = 1;
} else if (shift 0) {
val = (-shift - 1);
if (val == INT64_MAX) {
--
1.7.1
Re: [Qemu-devel] KVM call agenda for Feb 15
On 02/15/2011 06:11 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. - QAPI and QMP events - qdev future Do we need a qdev tree and a maintainer for it? I think we need a clear design and direction first. Regards, Anthony Liguori I don't really have a coherent plan for the second one yet so let's just discuss this as time permits.
[Qemu-devel] [PATCH 09/10] target-arm: Fix unsigned VQRSHL by large shift counts
Correctly handle VQRSHL of unsigned values by a shift count of the
width of the data type or larger, which must be special-cased in the
qrshl_u* helper functions.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
target-arm/neon_helper.c | 37 ++---
1 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index f8f6db6..c813fa0 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -818,7 +818,18 @@ uint64_t HELPER(neon_qshlu_s64)(CPUState *env, uint64_t
valop, uint64_t shiftop)
#define NEON_FN(dest, src1, src2) do { \
int8_t tmp; \
tmp = (int8_t)src2; \
-if (tmp 0) { \
+if (tmp = (ssize_t)sizeof(src1) * 8) { \
+if (src1) { \
+SET_QC(); \
+dest = ~0; \
+} else { \
+dest = 0; \
+} \
+} else if (tmp -(ssize_t)sizeof(src1) * 8) { \
+dest = 0; \
+} else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
+dest = src1 (sizeof(src1) * 8 - 1); \
+} else if (tmp 0) { \
dest = (src1 + (1 (-1 - tmp))) -tmp; \
} else { \
dest = src1 tmp; \
@@ -837,7 +848,18 @@ uint32_t HELPER(neon_qrshl_u32)(CPUState *env, uint32_t
val, uint32_t shiftop)
{
uint32_t dest;
int8_t shift = (int8_t)shiftop;
-if (shift 0) {
+if (shift = 32) {
+if (val) {
+SET_QC();
+dest = ~0;
+} else {
+dest = 0;
+}
+} else if (shift -32) {
+dest = 0;
+} else if (shift == -32) {
+dest = val 31;
+} else if (shift 0) {
uint64_t big_dest = ((uint64_t)val + (1 (-1 - shift)));
dest = big_dest -shift;
} else {
@@ -855,7 +877,16 @@ uint32_t HELPER(neon_qrshl_u32)(CPUState *env, uint32_t
val, uint32_t shiftop)
uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t val, uint64_t shiftop)
{
int8_t shift = (int8_t)shiftop;
-if (shift 0) {
+if (shift = 64) {
+if (val) {
+SET_QC();
+val = ~0;
+}
+} else if (shift -64) {
+val = 0;
+} else if (shift == -64) {
+val = 63;
+} else if (shift 0) {
val = (-shift - 1);
if (val == UINT64_MAX) {
/* In this case, it means that the rounding constant is 1,
--
1.7.1
Re: [Qemu-devel] [PATCH v3 0/6] target-arm: Fix Neon shift instructions.
On 14.02.2011 19:18, Peter Maydell wrote: On 11 February 2011 15:10, christophe.l...@st.com wrote: From: Christophe Lyon christophe.l...@st.com This patch series provides fixes such that ARM Neon instructions VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN, VQSHRN, VSHRN, VQSHRUN now pass all my tests. I have reworked all these patches and I hope they are now easier to review. Thanks; this was indeed a lot easier to review. Mostly this is OK, there are a few things: * minor style issues * handling of very large shift counts is not right (both in code you wrote and existing routines) Yes, I mostly copied existing code to fix the parts I identified as wrong, but indeed very large shift amounts are not part of my tests. * the shift-and-narrow loop doesn't handle the case where pass 1 reads registers pass 0 writes I have patches which (sitting on top of your 6) fix all these and give a clean pass on the random instruction set testing for the shift instructions. Unless you object, I think the simplest thing will be for me to just fix the minor nits I identified in your patches and then post a combined series of your patches and mine. OK, thanks. It also seems that the neon_shl_s* helpers don't handle correctly large negative shift amounts. Christophe.
What's QAPI? (was: [Qemu-devel] [RFC] qapi: events in QMP)
Anthony Liguori anth...@codemonkey.ws writes: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. Have you put your thinking behind the QAPI branch in writing anywhere? Ideally in comparable detail as your discussion of events in QAPI (which I snipped). [...]
Re: [Qemu-devel] KVM call agenda for Feb 15
On 02/15/2011 04:44 AM, Avi Kivity wrote: On 02/15/2011 12:18 AM, Anthony Liguori wrote: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. -rc2 is tagged and waiting for announcement. Please take a look at -rc2 and make sure there is nothing critical missing. Will tag 0.14.0 very late tomorrow but unless there's something critical, it'll be 0.14.0-rc2 with an updated version. Aren't you going to let users play with rc2 for a week or so? If not, what's the point? just push out 0.14.0. If you think this is worth doing, we can try for 0.15.0. But this has been the plan for 0.14.0 and I'd like to stick to the plan (for a change) :-) Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 4/6] target-arm: fix saturated values for Neon right shifts.
@@ -924,7 +924,11 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t
valop, uint32_t shiftop)
dest = val shift;
if ((dest shift) != val) {
SET_QC();
-dest = (uint32_t)(1 (sizeof(val) * 8 - 1)) - (val 0 ? 1 :
0);
+if (val 0) {
+dest = INT32_MIN;
+} else {
+dest = INT32_MAX;
+}
Again, right answers but the way most of the rest of the code
forces a 32 bit value to signed saturation is
dest = (val 31) ^ ~SIGNBIT;
Indeed; I hadn't given a close look at how saturation is performed elsewhere.
Anyway I find my version more readable ;-)
Quite a few times, I have wondered whether there are rules in QEmu development
helping decide between performance of the code vs readability/maintainability?
Christophe.
Re: [Qemu-devel] KVM call agenda for Feb 15
On 02/15/2011 03:58 PM, Anthony Liguori wrote: On 02/15/2011 04:44 AM, Avi Kivity wrote: On 02/15/2011 12:18 AM, Anthony Liguori wrote: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. -rc2 is tagged and waiting for announcement. Please take a look at -rc2 and make sure there is nothing critical missing. Will tag 0.14.0 very late tomorrow but unless there's something critical, it'll be 0.14.0-rc2 with an updated version. Aren't you going to let users play with rc2 for a week or so? If not, what's the point? just push out 0.14.0. If you think this is worth doing, we can try for 0.15.0. But this has been the plan for 0.14.0 and I'd like to stick to the plan (for a change) :-) I'm fine either way. I just think it's strange to release an rc and a .0 back to back. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: What's QAPI?
On 02/15/2011 08:07 AM, Markus Armbruster wrote: Anthony Liguorianth...@codemonkey.ws writes: Hi, In my QAPI branch[1], I've now got almost every existing QMP command converted with (hopefully) all of the hard problems solved. There is only one remaining thing to attack before posting for inclusion and that's events. Here's my current thinking about what to do. Have you put your thinking behind the QAPI branch in writing anywhere? Ideally in comparable detail as your discussion of events in QAPI (which I snipped). It's an evolution of http://wiki.qemu.org/Features/QMP2 which grew out of our discussions at KVM Forum. I need to update the page considerably and we try to do that before the community call. Regards, Anthony Liguori [...]
Re: [Qemu-devel] [PATCH v3 0/6] target-arm: Fix Neon shift instructions.
On 15 February 2011 14:03, Christophe Lyon christophe.l...@st.com wrote: It also seems that the neon_shl_s* helpers don't handle correctly large negative shift amounts. They look OK to me (large -ve shift is a huge right shift, which for signed values should propagate the sign bit to all bit positions, so we shift-right by typesize - 1), and I didn't see any problems with them when I was doing my testing. What do you think they get wrong? -- PMM
[Qemu-devel] Re: qemu compiling error on ppc64: kvm.c:81: error: struct kvm_sregs has no member named pvr
Hrm. This means that your kernel headers in /usr/include/linux are too old. Can you try and find out which kernel version they are from please Yes, kernel headers version is 2.6.32. For the time being, I copied some of the header files from kvm/arch/powerpc/include/asm/ to kernel headers and qemu build was successful. Thanks a lot. Awesome! I'm eager to hear how well it works for you :) I collected some performance stats using kvm_stat. To get more information about exit counts of individual instructions, I configured CFLAGS_emulate.o with DDEBUG flag. Now, I can see output of pr_debug(Emulating opcode %d / %d\n, get_op(inst), get_xop(inst)); instruction in kernel logs. I have two queries: 1. The count of Emulating opcode statement in /var/log/kern.log (3 million) is coming out to be much less than emulated_inst_exits (22 million) using kvm_stat. 2. How to configure makefiles to get output of printk statements inside kvm/arch/powerpc/kvm/trace.h Also, every time I start kvm, I get this error KVM: Couldn't find level irq capability. Expect the VM to stall at times. After some time, the VM just hangs. I traced this output to qemu/target-ppc/kvm.c Is there any way to avoid it. Thanks, Dushyant
Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
On Tue Feb 15, 2011 at 13:37:44 +, Peter Maydell wrote:
On 15 February 2011 13:12, Adam Lackorzynski a...@os.inf.tu-dresden.de
wrote:
On Tue Feb 15, 2011 at 13:01:08 +, Peter Maydell wrote:
On 15 February 2011 10:48, Adam Lackorzynski a...@os.inf.tu-dresden.de
wrote:
Make smpboot available not only for Linux but for all setups.
I'm not convinced about this. I think if you're providing a raw
image for an SMP system (rather than a Linux kernel) then it's
your job to provide an image which handles the bootup of the
secondary CPUs, the same way it would be if you were providing
a ROM image for real hardware.
Ok, this is one possibility. Another one would be something like this:
@@ -112,7 +112,11 @@ static void secondary_cpu_reset(void *opaque)
/* Set entry point for secondary CPUs. This assumes we're using
the init code from arm_boot.c. Real hardware resets all CPUs
the same. */
- env-regs[15] = SMP_BOOT_ADDR;
+ if (realview_binfo.is_linux) {
+ env-regs[15] = SMP_BOOT_ADDR;
+ } else {
+ env-regs[15] = realview_binfo.entry;
+ }
}
Moving in the right direction, but it would be cleaner if the secondary
CPU reset was handled inside arm_boot.c, I think (there is a TODO
in that file to that effect). Then we could get rid of the cpu reset
hook from realview.c.
Like the following?
Subject: [PATCH] target-arm: Integrate secondary CPU reset in arm_boot
Integrate secondary CPU reset into arm_boot, removing it from realview.c.
On non-Linux systems secondary CPUs start with the same entry as the boot
CPU.
Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
hw/arm_boot.c | 23 +++
hw/realview.c | 14 --
2 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 620550b..41e99d1 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -175,7 +175,7 @@ static void set_kernel_args_old(struct arm_boot_info *info,
}
}
-static void main_cpu_reset(void *opaque)
+static void do_cpu_reset(void *opaque)
{
CPUState *env = opaque;
struct arm_boot_info *info = env-boot_info;
@@ -187,16 +187,20 @@ static void main_cpu_reset(void *opaque)
env-regs[15] = info-entry 0xfffe;
env-thumb = info-entry 1;
} else {
-env-regs[15] = info-loader_start;
-if (old_param) {
-set_kernel_args_old(info, info-initrd_size,
+if (env == first_cpu) {
+env-regs[15] = info-loader_start;
+if (old_param) {
+set_kernel_args_old(info, info-initrd_size,
+info-loader_start);
+} else {
+set_kernel_args(info, info-initrd_size,
info-loader_start);
+}
} else {
-set_kernel_args(info, info-initrd_size, info-loader_start);
+env-regs[15] = info-smp_loader_start;
}
}
}
-/* TODO: Reset secondary CPUs. */
}
void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
@@ -217,7 +221,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info
*info)
if (info-nb_cpus == 0)
info-nb_cpus = 1;
-env-boot_info = info;
#ifdef TARGET_WORDS_BIGENDIAN
big_endian = 1;
@@ -279,5 +282,9 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info
*info)
info-initrd_size = initrd_size;
}
info-is_linux = is_linux;
-qemu_register_reset(main_cpu_reset, env);
+
+for (; env; env = env-next_cpu) {
+env-boot_info = info;
+qemu_register_reset(do_cpu_reset, env);
+}
}
diff --git a/hw/realview.c b/hw/realview.c
index 6eb6c6a..fae444a 100644
--- a/hw/realview.c
+++ b/hw/realview.c
@@ -104,17 +104,6 @@ static struct arm_boot_info realview_binfo = {
.smp_loader_start = SMP_BOOT_ADDR,
};
-static void secondary_cpu_reset(void *opaque)
-{
- CPUState *env = opaque;
-
- cpu_reset(env);
- /* Set entry point for secondary CPUs. This assumes we're using
- the init code from arm_boot.c. Real hardware resets all CPUs
- the same. */
- env-regs[15] = SMP_BOOT_ADDR;
-}
-
/* The following two lists must be consistent. */
enum realview_board_type {
BOARD_EB,
@@ -176,9 +165,6 @@ static void realview_init(ram_addr_t ram_size,
}
irqp = arm_pic_init_cpu(env);
cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
-if (n 0) {
-qemu_register_reset(secondary_cpu_reset, env);
-}
}
if (arm_feature(env, ARM_FEATURE_V7)) {
if (is_mpcore) {
--
1.7.2.3
Adam
--
Adam a...@os.inf.tu-dresden.de
Lackorzynski http://os.inf.tu-dresden.de/~adam/
[Qemu-devel] Re: qemu compiling error on ppc64: kvm.c:81: error: struct kvm_sregs has no member named pvr
On 15.02.2011, at 15:21, Dushyant Bansal wrote: Hrm. This means that your kernel headers in /usr/include/linux are too old. Can you try and find out which kernel version they are from please Yes, kernel headers version is 2.6.32. For the time being, I copied some of the header files from kvm/arch/powerpc/include/asm/ to kernel headers and qemu build was successful. Thanks a lot. Awesome! I'm eager to hear how well it works for you :) I collected some performance stats using kvm_stat. To get more information about exit counts of individual instructions, I configured CFLAGS_emulate.o with DDEBUG flag. Now, I can see output of pr_debug(Emulating opcode %d / %d\n, get_op(inst), get_xop(inst)); instruction in kernel logs. I have two queries: 1. The count of Emulating opcode statement in /var/log/kern.log (3 million) is coming out to be much less than emulated_inst_exits (22 million) using kvm_stat. The kernel log ring buffer is probably faster filled than read :). 2. How to configure makefiles to get output of printk statements inside kvm/arch/powerpc/kvm/trace.h Better don't make them printks - just use the tracing framework. I'd write up a small howto here myself, but I'm pretty much on the jump to my plane for vacation. Avi, could you please guide him a bit on how to get data out of tracepoints? Also, every time I start kvm, I get this error KVM: Couldn't find level irq capability. Expect the VM to stall at times. After some time, the VM just hangs. I traced this output to qemu/target-ppc/kvm.c Is there any way to avoid it. This means exactly what it means. There are two possible causes: 1) Your kernel is too old and doesn't support the capability 2) Qemu was compiled with kernel headers that don't know the capability yet If it's 2, just copy some of the kvm*.h files from your kernel source over to /usr/include/linux or /usr/include/asm-powerpc and fix them up to compile (remove __user). Alex
Re: [Qemu-devel] [FYI] memory leak in 0.14.0rc1 ?
2011/2/15 Torsten Förtsch torsten.foert...@gmx.net: I am trying out the opensuse kvm rpm from the Virtualization repository: http://download.opensuse.org/repositories/Virtualization/openSUSE_11.3/x86_64/kvm-0.14.0.rc1-67.1.x86_64.rpm I have installed winxp and run the machine as /usr/bin/qemu-kvm -name xp.home -m 768 Are you able to try QEMU 0.14.0-rc2 from source? $ git clone git://git.qemu.org/qemu.git $ git checkout v0.14.0-rc2 $ ./configure --target-list=x86_64-softmmu --enable-io-thread --disable-strip --prefix=/usr $ make $ x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 768 -name xp.home ... Also, please post your full KVM command-line so we know what devices and features you have enabled. Stefan
Re: [Qemu-devel] [PATCH v3 0/6] target-arm: Fix Neon shift instructions.
On 15.02.2011 15:21, Peter Maydell wrote: On 15 February 2011 14:03, Christophe Lyon christophe.l...@st.com wrote: It also seems that the neon_shl_s* helpers don't handle correctly large negative shift amounts. They look OK to me (large -ve shift is a huge right shift, which for signed values should propagate the sign bit to all bit positions, so we shift-right by typesize - 1), and I didn't see any problems with them when I was doing my testing. What do you think they get wrong? Sorry, you are right. I was confused by comparing with the shifts with rounding. Maybe we should add a few comments in a few places... :-)
Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
Luiz Capitulino lcapitul...@redhat.com writes:
On Mon, 14 Feb 2011 13:34:11 -0600
Anthony Liguori anth...@codemonkey.ws wrote:
On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
On Mon, 14 Feb 2011 08:39:11 -0600
Anthony Liguorianth...@codemonkey.ws wrote:
On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
So the question is: how does the schema based design support extending
commands or events? Does it require adding new commands/events?
Well, let me ask you, how do we do that today?
Let's say that I want to add a new parameter to the `change' function so
that I can include a salt parameter as part of the password.
The way we'd do this today is by checking for the 'salt' parameter in
qdict, and if it's not present, use a random salt or something like that.
You likely want to do what you did before. Of course that you have to
consider if what you're doing is extending an existing command or badly
overloading it (like change is today), in this case you'll want to add
a new command instead.
But yes, the use-case here is extending an existing command.
However, if I'm a QMP client, how can I tell whether you're going to
ignore my salt parameter or actually use it? Nothing in QMP tells me
this today. If I set the salt parameter in the `change' command, I'll
just get a success message.
I'm sorry?
{ execute: change, arguments: { device: vnc, target:
password, arg: 1234, salt: r1 } }
{error: {class: InvalidParameter, desc: Invalid parameter
'salt', data: {name: salt}}}
So I'm supposed to execute the command, and if execution fails, drop the
new parameter? If we add a few optional parameters, does that mean I
have to try every possible combination of parameters?
No, of course not, our plan has always been to do this via an schema,
Correct.
the only reason we don't do this today is lack of time/help.
Disagree on only. QMP didn't just suffer from lack of pushing power.
On the contrary, it suffered from too much pushing in opposite
directions.
[...]
Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
Hi Adam,
Moving in the right direction, but it would be cleaner if the secondary
CPU reset was handled inside arm_boot.c, I think (there is a TODO
in that file to that effect). Then we could get rid of the cpu reset
hook from realview.c.
Like the following?
This assumes that all the ARM SMP platforms are booting their
secondary CPU the same way as the emulated Realview.
For example, I'm currently writing a Tegra2 (dual A9) SoC emulation
and the second CPU is halted when the platform starts and I cannot
re-use the current smpboot firmware chunk. My current workaround is to
use info-nb_cpus = 1 and do the init in the board code. Forcing the
reset function will probably not help.
Subject: [PATCH] target-arm: Integrate secondary CPU reset in arm_boot
Integrate secondary CPU reset into arm_boot, removing it from realview.c.
On non-Linux systems secondary CPUs start with the same entry as the boot
CPU.
Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
hw/arm_boot.c | 23 +++
hw/realview.c | 14 --
2 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 620550b..41e99d1 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -175,7 +175,7 @@ static void set_kernel_args_old(struct arm_boot_info
*info,
}
}
-static void main_cpu_reset(void *opaque)
+static void do_cpu_reset(void *opaque)
{
CPUState *env = opaque;
struct arm_boot_info *info = env-boot_info;
@@ -187,16 +187,20 @@ static void main_cpu_reset(void *opaque)
env-regs[15] = info-entry 0xfffe;
env-thumb = info-entry 1;
} else {
- env-regs[15] = info-loader_start;
- if (old_param) {
- set_kernel_args_old(info, info-initrd_size,
+ if (env == first_cpu) {
+ env-regs[15] = info-loader_start;
+ if (old_param) {
+ set_kernel_args_old(info, info-initrd_size,
+ info-loader_start);
+ } else {
+ set_kernel_args(info, info-initrd_size,
info-loader_start);
+ }
} else {
- set_kernel_args(info, info-initrd_size, info-loader_start);
+ env-regs[15] = info-smp_loader_start;
}
}
}
- /* TODO: Reset secondary CPUs. */
}
void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
@@ -217,7 +221,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info
*info)
if (info-nb_cpus == 0)
info-nb_cpus = 1;
- env-boot_info = info;
#ifdef TARGET_WORDS_BIGENDIAN
big_endian = 1;
@@ -279,5 +282,9 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info
*info)
info-initrd_size = initrd_size;
}
info-is_linux = is_linux;
- qemu_register_reset(main_cpu_reset, env);
+
+ for (; env; env = env-next_cpu) {
+ env-boot_info = info;
+ qemu_register_reset(do_cpu_reset, env);
+ }
}
diff --git a/hw/realview.c b/hw/realview.c
index 6eb6c6a..fae444a 100644
--- a/hw/realview.c
+++ b/hw/realview.c
@@ -104,17 +104,6 @@ static struct arm_boot_info realview_binfo = {
.smp_loader_start = SMP_BOOT_ADDR,
};
-static void secondary_cpu_reset(void *opaque)
-{
- CPUState *env = opaque;
-
- cpu_reset(env);
- /* Set entry point for secondary CPUs. This assumes we're using
- the init code from arm_boot.c. Real hardware resets all CPUs
- the same. */
- env-regs[15] = SMP_BOOT_ADDR;
-}
-
/* The following two lists must be consistent. */
enum realview_board_type {
BOARD_EB,
@@ -176,9 +165,6 @@ static void realview_init(ram_addr_t ram_size,
}
irqp = arm_pic_init_cpu(env);
cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
- if (n 0) {
- qemu_register_reset(secondary_cpu_reset, env);
- }
}
if (arm_feature(env, ARM_FEATURE_V7)) {
if (is_mpcore) {
--
1.7.2.3
--
Vincent
[Qemu-devel] Re: qemu compiling error on ppc64: kvm.c:81: error: 'struct kvm_sregs' has no member named 'pvr'
On Tuesday 15 February 2011 08:03 PM, Alexander Graf wrote: On 15.02.2011, at 15:21, Dushyant Bansal wrote: Hrm. This means that your kernel headers in /usr/include/linux are too old. Can you try and find out which kernel version they are from please Yes, kernel headers version is 2.6.32. For the time being, I copied some of the header files from kvm/arch/powerpc/include/asm/ to kernel headers and qemu build was successful. Thanks a lot. Awesome! I'm eager to hear how well it works for you :) I collected some performance stats using kvm_stat. To get more information about exit counts of individual instructions, I configured CFLAGS_emulate.o with DDEBUG flag. Now, I can see output of pr_debug(Emulating opcode %d / %d\n, get_op(inst), get_xop(inst)); instruction in kernel logs. I have two queries: 1. The count of Emulating opcode statement in /var/log/kern.log (3 million) is coming out to be much less than emulated_inst_exits (22 million) using kvm_stat. The kernel log ring buffer is probably faster filled than read :). 2. How to configure makefiles to get output of printk statements inside kvm/arch/powerpc/kvm/trace.h Better don't make them printks - just use the tracing framework. I'd write up a small howto here myself, but I'm pretty much on the jump to my plane for vacation. Avi, could you please guide him a bit on how to get data out of tracepoints? Thanks for the quick reply :) I have added some more trace parameters in the tracing framework and currently, it is working fine. 1. Add new field in struct kvm_vcpu_stat (kvm_host.h) 2. Add corresponding entry in struct kvm_stats_debugfs_item debugfs_entries[] (book3s.c) 3. Increment or Decrement that field where ever necessary. I can see stats for the new parameter in kvm_stat output. I guess, this approach is okay. This means exactly what it means. There are two possible causes: 1) Your kernel is too old and doesn't support the capability 2) Qemu was compiled with kernel headers that don't know the capability yet If it's 2, just copy some of the kvm*.h files from your kernel source over to /usr/include/linux or /usr/include/asm-powerpc and fix them up to compile (remove __user). I think, its reason 2. I'll try this. Thanks, Dushyant
Re: [Qemu-devel] KVM call agenda for Feb 15
On Mon, Feb 14, 2011 at 10:18 PM, Anthony Liguori anth...@codemonkey.ws wrote: On 02/14/2011 11:56 AM, Chris Wright wrote: Please send in any agenda items you are interested in covering. -rc2 is tagged and waiting for announcement. Please take a look at -rc2 and make sure there is nothing critical missing. Will tag 0.14.0 very late tomorrow but unless there's something critical, it'll be 0.14.0-rc2 with an updated version. Most of my -rc2 testing is done now and has passed. http://wiki.qemu.org/Planning/0.14/Testing Stefan
Re: [Qemu-devel] [PATCH REBASE/RESEND 2/4] virtio-serial: Add description fields for qdev properties
On 02/04/2011 12:18 AM, Amit Shah wrote:
Document the parameters for the virtserialport and virtioconsole
devices.
Example:
$ ./x86_64-softmmu/qemu-system-x86_64 -device virtserialport,?
virtserialport.nr=uint32, The 'number' for the port for \
predictable port numbers. Use this to spawn ports if you \
plan to migrate the guest.
virtserialport.chardev=chr, The chardev to associate this port with.
virtserialport.name=string, Name for the port that's exposed to \
the guest for port discovery.
Signed-off-by: Amit Shahamit.s...@redhat.com
Acked-by: Markus Armbrusterarm...@redhat.com
---
hw/virtio-console.c | 17 ++---
hw/virtio-serial.h | 13 +
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index a32e628..29e5600 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -98,11 +98,13 @@ static VirtIOSerialPortInfo virtconsole_info = {
.init = virtconsole_initfn,
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
-DEFINE_PROP_UINT8(is_console, VirtConsole, port.is_console, 1, ),
+DEFINE_PROP_UINT8(is_console, VirtConsole, port.is_console, 1,
+ PROP_VIRTSERIAL_IS_CONSOLE_DESC),
DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID,
- ),
-DEFINE_PROP_CHR(chardev, VirtConsole, chr, ),
-DEFINE_PROP_STRING(name, VirtConsole, port.name, ),
+ PROP_VIRTSERIAL_NR_DESC),
+DEFINE_PROP_CHR(chardev, VirtConsole, chr, PROP_VIRTSERIAL_CHR_DESC),
+DEFINE_PROP_STRING(name, VirtConsole, port.name,
+ PROP_VIRTSERIAL_NAME_DESC),
DEFINE_PROP_END_OF_LIST(),
},
};
@@ -129,9 +131,10 @@ static VirtIOSerialPortInfo virtserialport_info = {
.exit = virtconsole_exitfn,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT32(nr, VirtConsole, port.id, VIRTIO_CONSOLE_BAD_ID,
- ),
-DEFINE_PROP_CHR(chardev, VirtConsole, chr, ),
-DEFINE_PROP_STRING(name, VirtConsole, port.name, ),
+ PROP_VIRTSERIAL_NR_DESC),
+DEFINE_PROP_CHR(chardev, VirtConsole, chr, PROP_VIRTSERIAL_CHR_DESC),
+DEFINE_PROP_STRING(name, VirtConsole, port.name,
+ PROP_VIRTSERIAL_NAME_DESC),
DEFINE_PROP_END_OF_LIST(),
},
};
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index a308196..2c5e336 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -57,6 +57,19 @@ struct virtio_console_control {
/* == In-qemu interface == */
+#define PROP_VIRTSERIAL_IS_CONSOLE_DESC \
+An hvc console will be spawned in the guest if this is set.
+
+#define PROP_VIRTSERIAL_NR_DESC \
+The 'number' for the port for predictable port numbers. Use this to \
+spawn ports if you plan to migrate the guest.
+
+#define PROP_VIRTSERIAL_CHR_DESC\
+The chardev to associate this port with.
+
+#define PROP_VIRTSERIAL_NAME_DESC\
+Name for the port that's exposed to the guest for port discovery.
+
Why are you using a #define instead of inlining the docs?
Regards,
Anthony Liguori
typedef struct VirtIOSerial VirtIOSerial;
typedef struct VirtIOSerialBus VirtIOSerialBus;
typedef struct VirtIOSerialPort VirtIOSerialPort;
Re: [Qemu-devel] [PATCH REBASE/RESEND 0/4] Auto-document qdev devices
On 02/04/2011 12:18 AM, Amit Shah wrote: Hello, This is yet another rebase of the patchset I'd sent earlier. The usual notes apply: this is just the start, just getting the framework in place and a few examples so that people can then pick up and start documenting their devices and options. We want to see all of the devices covered, and hopefully turn on build_bug_on() on an empty doc string. Maintainers should perhaps also look for patches that introduce options without documentation. That's the long-term goal (0.15-final). For short-term, I'll be preparing follow-on patches that add doc strings for a few more options and perhaps bug people based on git history as to what documentation is to be added for some options. Also to incorporate Markus's comments on beautifying output. The earlier this patchset goes in the better since it'll reduce conflicts and rebases needed. If this looks acceptable, please apply! I think we need to approach this in such a way that we generate not only inline documentation but out of line documentation. We need a way to extract the docs into a file. That could mean having something like a .hx file or just doing some clever things with grep DEFINE_PROP. Regards, Anthony Liguori Amit Shah (4): qdev: Add a description field for qdev properties for documentation virtio-serial: Add description fields for qdev properties net.h: Add description fields for qdev properites block_int.h: Provide documentation for common block qdev properties block_int.h | 20 +++- hw/a9mpcore.c |2 +- hw/acpi_piix4.c |2 +- hw/apic.c |4 +- hw/applesmc.c |4 +- hw/arm11mpcore.c|4 +- hw/arm_sysctl.c |4 +- hw/armv7m.c |2 +- hw/cs4231a.c|6 ++-- hw/debugcon.c |6 ++-- hw/eccmemctl.c |2 +- hw/escc.c | 16 +- hw/etraxfs_pic.c|3 +- hw/fdc.c| 14 hw/fw_cfg.c |4 +- hw/grlib_apbuart.c |2 +- hw/grlib_gptimer.c |6 ++-- hw/grlib_irqmp.c|4 +- hw/gus.c|8 ++-- hw/hda-audio.c |2 +- hw/hpet.c |4 +- hw/i2c.c|2 +- hw/ide/cmd646.c |2 +- hw/ide/isa.c|6 ++-- hw/ide/qdev.c |6 ++-- hw/integratorcp.c |2 +- hw/intel-hda.c |6 ++-- hw/ioh3420.c|8 ++-- hw/ivshmem.c| 15 + hw/lance.c |2 +- hw/m48t59.c | 12 hw/mc146818rtc.c|2 +- hw/ne2000-isa.c |4 +- hw/parallel.c |8 ++-- hw/pci.c| 10 +++--- hw/pxa2xx_gpio.c|4 +- hw/qdev-addr.h |4 +- hw/qdev.c |3 +- hw/qdev.h | 75 +-- hw/s390-virtio-bus.c|2 +- hw/sb16.c | 10 +++--- hw/scsi-bus.c |2 +- hw/scsi-disk.c |9 -- hw/serial.c |8 ++-- hw/slavio_timer.c |2 +- hw/smbus_eeprom.c |2 +- hw/sparc32_dma.c|4 +- hw/spitz.c |5 ++- hw/sun4m.c |2 +- hw/sun4m_iommu.c|2 +- hw/sun4u.c |2 +- hw/syborg_fb.c |4 +- hw/syborg_interrupt.c |2 +- hw/syborg_keyboard.c|2 +- hw/syborg_pointer.c |4 +- hw/syborg_serial.c |2 +- hw/syborg_timer.c |2 +- hw/syborg_virtio.c |6 ++-- hw/tcx.c| 10 +++--- hw/usb-bus.c|2 +- hw/usb-msd.c|2 +- hw/usb-ohci.c |4 +- hw/usb-serial.c |4 +- hw/virtio-blk.h |4 +- hw/virtio-console.c | 19 +++ hw/virtio-net.h | 51 --- hw/virtio-pci.c | 26 hw/virtio-serial.h | 13 hw/virtio.h |2 +- hw/vt82c686.c |2 +- hw/xilinx_ethlite.c |6 ++- hw/xilinx_intc.c|3 +- hw/xilinx_timer.c |4 +- hw/xio3130_downstream.c |8 ++-- hw/xio3130_upstream.c |4 +- net.h | 12 +-- usb-linux.c |8 ++-- 77 files changed, 301 insertions(+), 245 deletions(-)
[Qemu-devel] Re: What's QAPI?
On 02/15/2011 08:07 AM, Markus Armbruster wrote:
Anthony Liguorianth...@codemonkey.ws writes:
Hi,
In my QAPI branch[1], I've now got almost every existing QMP command
converted with (hopefully) all of the hard problems solved. There is
only one remaining thing to attack before posting for inclusion and
that's events. Here's my current thinking about what to do.
Have you put your thinking behind the QAPI branch in writing anywhere?
Ideally in comparable detail as your discussion of events in QAPI (which
I snipped).
http://wiki.qemu.org/Features/QAPI
Inlined here in case anyone wants to discuss:
== Implementation ==
QAPI uses a schema/IDL written in JSON to automatically generate the
types and
marshalling information for QMP commands. It is used to generate both the
server side marshalling interfaces and a client library.
A typical function definition looks like this:
##
# @change:
#
# This command is multiple commands multiplexed together. Generally
speaking,
# it should not be used in favor of the single purpose alternatives
such as
# @change-vnc-listen, @change-vnc-password, and @change-blockdev.
#
# @device: This is normally the name of a block device but it may also
be 'vnc'.
# when it's 'vnc', then sub command depends on @target
#
# @target: If @device is a block device, then this is the new filename.
# If @device is 'vnc', then if the value 'password' selects
the vnc
# change password command. Otherwise, this specifies a new
server URI
# address to listen to for VNC connections.
#
# @arg:If @device is a block device, then this is an optional
format to open
# the device with.
# If @device is 'vnc' and @target is 'password', this is the
new VNC
# password to set. If this argument is an empty string, then
no future
# logins will be allowed.
#
# Returns: Nothing on success.
# If @device is not a valid block device, DeviceNotFound
# If @format is not a valid block format, InvalidBlockFormat
# If the new block device is encrypted, DeviceEncrypted.
Note that
# if this error is returned, the device has been opened
successfully
# and an additional call to @block_passwd is required to set the
# device's password. The behavior of reads and writes to the
block
# device between when these calls are executed is undefined.
#
# Notes: It is strongly recommended that this interface is not used
especially
# for changing block devices.
#
# Since: 0.14.0
##
[ 'change', {'device': 'str', 'target': 'str'}, {'arg': 'str'}, 'none' ]
The comments above the function are written in gtk-doc format and meant
to be
extracted to generate both protocol documentation and libqmp documentation.
The first element of the list is the command name, in this case, 'change'.
The second element of the list is a dictionary containing the required
arguments
for the function. The key is the argument name and the value is the
type. The
type can be a string or a complex type (see section on Typing).
The third element is a dictionary of optional arguments that follows the
same
rules as the required arguments.
The final list element is the return type of the function. 'none' is a
special
type representing a void return value.
== QMP Server ==
This definition will generate the following signature in qemu/qmp.h:
void qmp_change(const char *device, const char *target, bool has_arg,
const char *arg, Error **errp);
Which is then implemented in the appropriate place in QEMU. The optional
arguments always are prefixed with a boolean argument to indicate
whether the
option has been specified. The final argument is a pointer to an Error
object
in the fashion of GError. The caller of this function is will check for a
non-NULL error object to determine if the function failed. errp can be
set to
NULL if the caller does not care about failure.
Currently, Error is roughly identical to QError with the exception that
it only
supports simple key/value arguments.
== Complex Types ==
Some commands take or return complex types. It's usually a good idea to
define
complex types outside of the function definition. An example of a command
that returns a complex type is below:
##
# @VersionInfo:
#
# A description of QEMU's version.
#
# @qemu.major: The major version of QEMU
#
# @qemu.minor: The minor version of QEMU
#
# @qemu.micro: The micro version of QEMU. By current convention, a micro
# version of 50 signifies a development branch. A micro
version
# greater than or equal to 90 signifies a release
candidate for
# the next minor version. A micro version of less than 50
# signifies a stable release.
#
# @package: QEMU will always set this field to an empty string.
Downstream
#
[Qemu-devel] KVM call minutes for Feb 15
QAPI and QMP - Anthony adding a new wiki page to describe all of this - specified in formal schema using JSON - includes documenation in javadoc-like syntax - can generate api (possibly protocol) docs - documenting each command and expected errors - creates marshalling functions and C interfaces - can generate C library - facilitates unit tests/regression tests - new and old code both exist in Anthony's tree - allows unit tests to run on both to verify - will remove old and force a flag day on merging in for 0.15 - still need to convert human monitor commands - goal to convert all of human monitor to QMP - events? - still not consumable from internal use - model signals and slots - similar to notifier lists, but can pass arbitrary data - client connects to signal via QMP - how to extend? - optional parameters (ABI bump) - no way to know if client is aware of and consuming the optional parameters - add new events - client required to register for new events when the know about them, server can generate different logic based on clients capability - first release may not include shared library (lack of libconf/autotool) - could - QMP session in default well-known location - allows iteration of all running QMP sessions - per-user directory to handle user-level isolation qdev future - have an object model, but can't do polymorphism (i.e. bus level) - could use more oop style, use GObject, use C++...no great ideas - no major qdev plans for 0.15 - would be useful to have the ability to do device level unit testing - cleaner device model, better encapsulation - this is both the device side interfaces, but also interfaces back to qemu - ability to do something like a virtual PCI bus to be a test harness to interact with a device - back to the GObject, oop, C++ questions? - IDL based code generation to generate VMState in effort to make migration more verifiable - VMState - need to focus on serialized guest visible state - start with all state and remove obviously internal only state - start with only guest visible state (structure separation) - verfiable - need a qdev tree maintainer? - some disagreement on exactly how much - qdev autodoc patches? (posted and ack'd multiple times) bad patches committed that are not on list - please inform of specifics incidents, this should not be happening SeaBIOS update? - w/out we will have features that can't be used - need a release.. - 0.15 will need good planning and dates and communication with Kevin 0.14-rc2 tagged please review for any missing patches, 0.14.0 likely tagged late today revisit new - old migration - Amit offers virtio-serial patches and some legwork - tabled discussion to list, possibly next week's call
[Qemu-devel] [PATCHv2 1/3] e1000: multi-buffer packet support
e1000 supports multi-buffer packets larger than rxbuf_size.
This fixes the following (on linux):
- in guest: ifconfig eth1 mtu 16110
- in host: ifconfig tap0 mtu 16110
ping -s 16082 guest-ip
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
hw/e1000.c | 39 ---
1 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index af101bd..050ce02 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -642,6 +642,9 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
uint16_t vlan_special = 0;
uint8_t vlan_status = 0, vlan_offset = 0;
uint8_t min_buf[MIN_BUF_SIZE];
+size_t desc_offset;
+size_t desc_size;
+size_t total_size;
if (!(s-mac_reg[RCTL] E1000_RCTL_EN))
return -1;
@@ -654,12 +657,6 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
size = sizeof(min_buf);
}
-if (size s-rxbuf_size) {
-DBGOUT(RX, packet too large for buffers (%lu %d)\n,
- (unsigned long)size, s-rxbuf_size);
-return -1;
-}
-
if (!receive_filter(s, buf, size))
return size;
@@ -672,8 +669,16 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
}
rdh_start = s-mac_reg[RDH];
+desc_offset = 0;
+total_size = size + fcs_len(s);
do {
+desc_size = total_size - desc_offset;
+if (desc_size s-rxbuf_size) {
+desc_size = s-rxbuf_size;
+}
if (s-mac_reg[RDH] == s-mac_reg[RDT] s-check_rxov) {
+/* Discard all data written so far */
+s-mac_reg[RDH] = rdh_start;
set_ics(s, 0, E1000_ICS_RXO);
return -1;
}
@@ -683,10 +688,22 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
desc.special = vlan_special;
desc.status |= (vlan_status | E1000_RXD_STAT_DD);
if (desc.buffer_addr) {
-cpu_physical_memory_write(le64_to_cpu(desc.buffer_addr),
- (void *)(buf + vlan_offset), size);
-desc.length = cpu_to_le16(size + fcs_len(s));
-desc.status |= E1000_RXD_STAT_EOP|E1000_RXD_STAT_IXSM;
+if (desc_offset size) {
+size_t copy_size = size - desc_offset;
+if (copy_size s-rxbuf_size) {
+copy_size = s-rxbuf_size;
+}
+cpu_physical_memory_write(le64_to_cpu(desc.buffer_addr),
+ (void *)(buf + desc_offset +
vlan_offset),
+ copy_size);
+}
+desc_offset += desc_size;
+if (desc_offset = total_size) {
+desc.length = cpu_to_le16(desc_size);
+desc.status |= E1000_RXD_STAT_EOP | E1000_RXD_STAT_IXSM;
+} else {
+desc.length = cpu_to_le16(desc_size);
+}
} else { // as per intel docs; skip descriptors with null buf addr
DBGOUT(RX, Null RX descriptor!!\n);
}
@@ -702,7 +719,7 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
set_ics(s, 0, E1000_ICS_RXO);
return -1;
}
-} while (desc.buffer_addr == 0);
+} while (desc_offset total_size);
s-mac_reg[GPRC]++;
s-mac_reg[TPR]++;
--
1.7.3.2.91.g446ac
[Qemu-devel] [PATCHv2 2/3] e1000: clear EOP for multi-buffer descriptors
The e1000 spec says: if software statically allocates
buffers, and uses memory read to check for completed descriptors, it
simply has to zero the status byte in the descriptor to make it ready
for reuse by hardware. This is not a hardware requirement (moving the
hardware tail pointer is), but is necessary for performing an in–memory
scan.
Thus the guest does not have to clear the status byte. In case it
doesn't we need to clear EOP for all descriptors
except the last. While I don't know of any such guests,
it's probably a good idea to stick to the spec.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
Reported-by: Juan Quintela quint...@redhat.com
---
hw/e1000.c |6 --
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index 050ce02..2943a1a 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -698,11 +698,13 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
copy_size);
}
desc_offset += desc_size;
+desc.length = cpu_to_le16(desc_size);
if (desc_offset = total_size) {
-desc.length = cpu_to_le16(desc_size);
desc.status |= E1000_RXD_STAT_EOP | E1000_RXD_STAT_IXSM;
} else {
-desc.length = cpu_to_le16(desc_size);
+/* Guest zeroing out status is not a hardware requirement.
+ Clear EOP in case guest didn't do it. */
+desc.status = ~E1000_RXD_STAT_EOP;
}
} else { // as per intel docs; skip descriptors with null buf addr
DBGOUT(RX, Null RX descriptor!!\n);
--
1.7.3.2.91.g446ac
[Qemu-devel] [PATCHv2 0/3] e1000: multi-buffer packet support
e1000 supports multi-buffer packets larger than rxbuf_size. This fixes the following (on linux): - in guest: ifconfig eth1 mtu 16110 - in host: ifconfig tap0 mtu 16110 ping -s 16082 guest-ip Red Hat bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=602205 Changes from v1: fix buffer overflow reported by Kevin added a patch to fix EOP spec violation reported by Juan added a patch to fix spec violation noted by myself Michael S. Tsirkin (3): e1000: multi-buffer packet support e1000: clear EOP for multi-buffer descriptors e1000: verify we have buffers, upfront hw/e1000.c | 61 +++ 1 files changed, 48 insertions(+), 13 deletions(-) -- 1.7.3.2.91.g446ac
[Qemu-devel] [PATCHv2 3/3] e1000: verify we have buffers, upfront
The spec says: Any descriptor with a non-zero status byte has been
processed by the hardware, and is ready to be handled by the software.
Thus, once we change a descriptor status to non-zero we should
never move the head backwards and try to reuse this
descriptor from hardware.
This actually happened with a multibuffer packet
that arrives when we don't have enough buffers.
Fix by checking that we have enough buffers upfront
so we never need to discard the packet midway through.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
hw/e1000.c | 28 ++--
1 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index 2943a1a..0a4574c 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -631,6 +631,24 @@ e1000_can_receive(VLANClientState *nc)
return (s-mac_reg[RCTL] E1000_RCTL_EN);
}
+static bool e1000_has_rxbufs(E1000State *s, size_t total_size)
+{
+int bufs;
+/* Fast-path short packets */
+if (total_size = s-rxbuf_size) {
+return s-mac_reg[RDH] != s-mac_reg[RDT] || !s-check_rxov;
+}
+if (s-mac_reg[RDH] s-mac_reg[RDT]) {
+bufs = s-mac_reg[RDT] - s-mac_reg[RDH];
+} else if (s-mac_reg[RDH] s-mac_reg[RDT] || !s-check_rxov) {
+bufs = s-mac_reg[RDLEN] / sizeof(struct e1000_rx_desc) +
+s-mac_reg[RDT] - s-mac_reg[RDH];
+} else {
+return false;
+}
+return total_size = bufs * s-rxbuf_size;
+}
+
static ssize_t
e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
{
@@ -671,17 +689,15 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
size_t size)
rdh_start = s-mac_reg[RDH];
desc_offset = 0;
total_size = size + fcs_len(s);
+if (!e1000_has_rxbufs(s, total_size)) {
+set_ics(s, 0, E1000_ICS_RXO);
+return -1;
+}
do {
desc_size = total_size - desc_offset;
if (desc_size s-rxbuf_size) {
desc_size = s-rxbuf_size;
}
-if (s-mac_reg[RDH] == s-mac_reg[RDT] s-check_rxov) {
-/* Discard all data written so far */
-s-mac_reg[RDH] = rdh_start;
-set_ics(s, 0, E1000_ICS_RXO);
-return -1;
-}
base = ((uint64_t)s-mac_reg[RDBAH] 32) + s-mac_reg[RDBAL] +
sizeof(desc) * s-mac_reg[RDH];
cpu_physical_memory_read(base, (void *)desc, sizeof(desc));
--
1.7.3.2.91.g446ac
[Qemu-devel] Re: [PATCH 00/10] Fix Neon shift instructions
On 15.02.2011 14:44, Peter Maydell wrote: This patch series fixes bugs in the Neon shift instructions VRSHR, VRSRA, VQRSHRN, VQRSHRUN, VRSHRN, VQSHRN, VSHRN, VQSHRUN, VRSHL, and VQRSHL. It is based on the v3 patchset Christophe sent recently, with some fixes for minor nits in those patches, plus some patches from me which fix problems with shifts by large shift counts and an issue with overlapping source and destination registers. With this patchset qemu passes random instruction sequence testing for all these instruction patterns. FWIW, I confirm that with this patchset, qemu passes all my tests on shift instruction. Peter, thank you for adapting my patches and transforming my patch #4 into your patch #5. Christophe.
Re: [Qemu-devel] [PATCH REBASE/RESEND 1/4] qdev: Add a description field for qdev properties for documentation
On 02/04/2011 12:18 AM, Amit Shah wrote: Add a 'description' along with each qdev property to document the input each qdev property takes. Signed-off-by: Amit Shahamit.s...@redhat.com Acked-by: Markus Armbrusterarm...@redhat.com --- block_int.h | 14 hw/a9mpcore.c |2 +- hw/acpi_piix4.c |2 +- hw/apic.c |4 +- hw/applesmc.c |4 +- hw/arm11mpcore.c|4 +- hw/arm_sysctl.c |4 +- hw/armv7m.c |2 +- hw/cs4231a.c|6 ++-- hw/debugcon.c |6 ++-- hw/eccmemctl.c |2 +- hw/escc.c | 16 +- hw/etraxfs_pic.c|3 +- hw/fdc.c| 14 hw/fw_cfg.c |4 +- hw/grlib_apbuart.c |2 +- hw/grlib_gptimer.c |6 ++-- hw/grlib_irqmp.c|4 +- hw/gus.c|8 ++-- hw/hda-audio.c |2 +- hw/hpet.c |4 +- hw/i2c.c|2 +- hw/ide/cmd646.c |2 +- hw/ide/isa.c|6 ++-- hw/ide/qdev.c |6 ++-- hw/integratorcp.c |2 +- hw/intel-hda.c |6 ++-- hw/ioh3420.c|8 ++-- hw/ivshmem.c| 15 + hw/lance.c |2 +- hw/m48t59.c | 12 hw/mc146818rtc.c|2 +- hw/ne2000-isa.c |4 +- hw/parallel.c |8 ++-- hw/pci.c| 10 +++--- hw/pxa2xx_gpio.c|4 +- hw/qdev-addr.h |4 +- hw/qdev.c |3 +- hw/qdev.h | 75 +-- hw/s390-virtio-bus.c|2 +- hw/sb16.c | 10 +++--- hw/scsi-bus.c |2 +- hw/scsi-disk.c |9 -- hw/serial.c |8 ++-- hw/slavio_timer.c |2 +- hw/smbus_eeprom.c |2 +- hw/sparc32_dma.c|4 +- hw/spitz.c |5 ++- hw/sun4m.c |2 +- hw/sun4m_iommu.c|2 +- hw/sun4u.c |2 +- hw/syborg_fb.c |4 +- hw/syborg_interrupt.c |2 +- hw/syborg_keyboard.c|2 +- hw/syborg_pointer.c |4 +- hw/syborg_serial.c |2 +- hw/syborg_timer.c |2 +- hw/syborg_virtio.c |6 ++-- hw/tcx.c| 10 +++--- hw/usb-bus.c|2 +- hw/usb-msd.c|2 +- hw/usb-ohci.c |4 +- hw/usb-serial.c |4 +- hw/virtio-blk.h |4 +- hw/virtio-console.c | 16 + hw/virtio-net.h | 51 --- hw/virtio-pci.c | 26 hw/virtio.h |2 +- hw/vt82c686.c |2 +- hw/xilinx_ethlite.c |6 ++- hw/xilinx_intc.c|3 +- hw/xilinx_timer.c |4 +- hw/xio3130_downstream.c |8 ++-- hw/xio3130_upstream.c |4 +- net.h |8 ++-- usb-linux.c |8 ++-- 76 files changed, 276 insertions(+), 244 deletions(-) diff --git a/block_int.h b/block_int.h index 6ebdc3e..fdde005 100644 --- a/block_int.h +++ b/block_int.h @@ -250,15 +250,15 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) } #define DEFINE_BLOCK_PROPERTIES(_state, _conf) \ -DEFINE_PROP_DRIVE(drive, _state, _conf.bs), \ +DEFINE_PROP_DRIVE(drive, _state, _conf.bs, ), \ DEFINE_PROP_UINT16(logical_block_size, _state,\ - _conf.logical_block_size, 512), \ + _conf.logical_block_size, 512, ), \ DEFINE_PROP_UINT16(physical_block_size, _state, \ - _conf.physical_block_size, 512), \ -DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \ -DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0),\ -DEFINE_PROP_INT32(bootindex, _state, _conf.bootindex, -1),\ + _conf.physical_block_size, 512, ), \ +DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0, ), \ +DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0, ), \ +DEFINE_PROP_INT32(bootindex, _state, _conf.bootindex, -1, ), \ DEFINE_PROP_UINT32(discard_granularity, _state, \ - _conf.discard_granularity, 0) + _conf.discard_granularity, 0, ) This is pretty horribly ugly. If we were going this, we should at least introduce new defines that include a documentation field and not just add empty documentation fields. Regards, Anthony Liguori
Re: [Qemu-devel] KVM call minutes for Feb 8
On Mon, Feb 14, 2011 at 11:47 PM, Anthony Liguori anth...@codemonkey.ws wrote: On 02/14/2011 03:25 PM, Blue Swirl wrote: I'd still like to have the inline wrapper over the factory interface, probably with similar signature to isa_serial_new. Then there would be two functions, one going through qdev and the other bypassing it. I don't see how that would be useful. The callers of the direct interface would force linkage between them and so it would be impossible to build QEMU with that device. We don't need that flexibility for every device though, but I don't see any advantages for using the direct interface either. Why shouldn't we want all devices to be exposed to the user? For example, there are still devices which don't show up in 'info qtree', which is a shame. Showing up in info qtree is goodness, but I'm talking about allowing a user to directly instantiate a device. Any device we expose to the user through -device needs to maintain a compatible interface forever. For our own sanity, I think we should try to expose as little as possible. Restricting the users from adding arbitrary devices is a different issue. Dropping qdev support to prevent user from adding the device seems draconian, what's wrong with no_user flag? A good example of a device that we should model through qdev but not expose via -device is actually SerialState. You wouldn't want users to add any serial ports? What should be do with serial ports then, always enable a full set of ports? How would the user use them? Today, we have ISASerialState which embeds SerialState. We can also create a MMIO version of SerialState although there's no direct structure that wraps that. Ideally, SerialState would be a proper qdev device that is embedded in both ISASerialState and MMIOSerialState (or pick a better name). info qtree should show a has-a relationship for these devices. I think the devices shown in qtree should always have some relationship to real devices. If ICH10 contains all possible onboard devices, including for example HPET, e1000 and SATA, that could use a has-a relationship to show the composition but otherwise I fear this would only increase complexity with no gain.
Re: [Qemu-devel] [FYI] memory leak in 0.14.0rc1 ?
On Tuesday, February 15, 2011 15:43:32 Stefan Hajnoczi wrote: I have installed winxp and run the machine as /usr/bin/qemu-kvm -name xp.home -m 768 Are you able to try QEMU 0.14.0-rc2 from source? $ git clone git://git.qemu.org/qemu.git $ git checkout v0.14.0-rc2 $ ./configure --target-list=x86_64-softmmu --enable-io-thread --disable-strip --prefix=/usr $ make $ x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 768 -name xp.home ... Now, the process size stays around 1300 Mb and RSS is very constant at 794 Mb. But the VM is much slower. Although, according to info kvm kvm is enabled. What options do I have to use to get optimal performance? Also, please post your full KVM command-line so we know what devices and features you have enabled. x86_64-softmmu/qemu-system-x86_64 \ -name xp.home -boot order=dc \ -drive file=/.../xp.home/root.raw,if=virtio \ -drive file=/.../config.cpio,if=floppy,index=1 \ -m 768 \ -usb \ -rtc base=utc,clock=host \ -vga std \ -cpu host \ -smp 2 \ -enable-kvm \ -balloon virtio \ -vnc 127.0.0.1:15 \ -monitor unix:/.../xp.home/.monitor,server,nowait \ -serial unix:/.../xp.home/.ttyS0,server,nowait \ -net nic,vlan=0,model=virtio,macaddr=E0:23:DA:00:00:05 \ -net tap,vlan=0,name=ext,ifname=qe5,script=/.../qemu-ifup,downscript=/.../qemu-ifdown \ -net nic,vlan=1,model=virtio,macaddr=F0:23:DA:00:00:05 \ -net tap,vlan=1,name=int,ifname=qi5,script=/.../qemu-ifup,downscript=/.../qemu-ifdown \ -rtc base=localtime \ -usbdevice tablet In the original command there was another option boot=on on the 1st drive. And, the -daemonize option was given. BTW, the monitor still says it's QEMU 0.13.92, not 0.14. Torsten Förtsch
Re: [Qemu-devel] [PATCH 06/10] vmmouse: convert to qdev
On Tue, Feb 15, 2011 at 12:07 PM, Markus Armbruster arm...@redhat.com wrote: Anthony Liguori anth...@codemonkey.ws writes: On 02/12/2011 11:03 AM, Markus Armbruster wrote: Blue Swirlblauwir...@gmail.com writes: Convert to qdev, also add a proper reset function. [...] Pointer properties are for dirty hacks only. Is there really no better solution? Why does it have to be a property? vmmouse is really just an extension to the PS2 Mouse. It's definitely not an ISA device. In terms of qdev enablement, I would just make it a boolean option to the PS2Mouse and not expose it as a top level device at all. It cannot exist without a PS2Mouse. Which means making it a separate qdev is wrong. That wrongness gave rise to the dirty pointer property. Pointer property serves as canary again. What now? I don't find pointer property use so dirty, but I'll try to combine the devices to see whether that makes sense. PS: Grumpy reviewer venting: review can keep such mistakes out of the code, but it got committed less than two days after it was posted. Did not: http://lists.nongnu.org/archive/html/qemu-devel/2011-02/msg00396.html http://git.qemu.org/qemu.git/commit/?id=91c9e09147ba1f3604a3d5d29b4de7702082a33f Thank you for reviewing.
Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
Hi, On Tue Feb 15, 2011 at 10:02:05 -0500, Vincent Palatin wrote: Moving in the right direction, but it would be cleaner if the secondary CPU reset was handled inside arm_boot.c, I think (there is a TODO in that file to that effect). Then we could get rid of the cpu reset hook from realview.c. Like the following? This assumes that all the ARM SMP platforms are booting their secondary CPU the same way as the emulated Realview. For example, I'm currently writing a Tegra2 (dual A9) SoC emulation and the second CPU is halted when the platform starts and I cannot re-use the current smpboot firmware chunk. My current workaround is to use info-nb_cpus = 1 and do the init in the board code. Forcing the reset function will probably not help. The smpboot code also halts the CPUs, i.e. they are waiting in wfi. What would you like to have instead? Maybe it's not so different... Adam -- Adam a...@os.inf.tu-dresden.de Lackorzynski http://os.inf.tu-dresden.de/~adam/
Re: [Qemu-devel] [PATCH 05/10] vmport: convert to qdev
On Tue, Feb 15, 2011 at 12:22 PM, Markus Armbruster arm...@redhat.com wrote:
Blue Swirl blauwir...@gmail.com writes:
On Sat, Feb 12, 2011 at 6:57 PM, Markus Armbruster arm...@redhat.com wrote:
Blue Swirl blauwir...@gmail.com writes:
Signed-off-by: Blue Swirl blauwir...@gmail.com
---
hw/pc.h | 1 -
hw/pc_piix.c | 2 --
hw/vmport.c | 24 +---
3 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/hw/pc.h b/hw/pc.h
index a048768..603a2a3 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -65,7 +65,6 @@ void hpet_pit_disable(void);
void hpet_pit_enable(void);
/* vmport.c */
-void vmport_init(void);
void vmport_register(unsigned char command, IOPortReadFunc *func,
void *opaque);
/* vmmouse.c */
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 7b74473..d0bd0cd 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -86,8 +86,6 @@ static void pc_init1(ram_addr_t ram_size,
pc_cpus_init(cpu_model);
- vmport_init();
-
/* allocate ram and load rom/bios */
pc_memory_init(ram_size, kernel_filename, kernel_cmdline,
initrd_filename,
below_4g_mem_size, above_4g_mem_size);
diff --git a/hw/vmport.c b/hw/vmport.c
index 6c9d7c9..4432be0 100644
--- a/hw/vmport.c
+++ b/hw/vmport.c
@@ -26,6 +26,7 @@
#include pc.h
#include sysemu.h
#include kvm.h
+#include qdev.h
//#define VMPORT_DEBUG
@@ -37,6 +38,7 @@
typedef struct _VMPortState
{
+ ISADevice dev;
IOPortReadFunc *func[VMPORT_ENTRIES];
void *opaque[VMPORT_ENTRIES];
} VMPortState;
@@ -100,12 +102,28 @@ static uint32_t vmport_cmd_ram_size(void
*opaque, uint32_t addr)
return ram_size;
}
-void vmport_init(void)
+static int vmport_initfn(ISADevice *dev)
{
- register_ioport_read(0x5658, 1, 4, vmport_ioport_read, port_state);
- register_ioport_write(0x5658, 1, 4, vmport_ioport_write, port_state);
+ VMPortState *s = DO_UPCAST(VMPortState, dev, dev);
+ register_ioport_read(0x5658, 1, 4, vmport_ioport_read, s);
+ register_ioport_write(0x5658, 1, 4, vmport_ioport_write, s);
+ isa_init_ioport(dev, 0x5658);
/* Register some generic port commands */
vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, NULL);
vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size, NULL);
+ return 0;
}
+
+static ISADeviceInfo vmport_info = {
+ .qdev.name = vmport,
+ .qdev.size = sizeof(VMPortState),
+ .qdev.no_user = 1,
+ .init = vmport_initfn,
+};
+
+static void vmport_dev_register(void)
+{
+ isa_qdev_register(vmport_info);
+}
+device_init(vmport_dev_register)
Old code has pc_init1() call vmport_init(). Where does your code create
qdev vmport? And what's happening with port_state? It's still used
by vmport_register(), but no longer connected to the I/O ports. Can't
see how vmport_register() has any effect anymore.
I fixed it in the committed version.
Did you post v2 to the list for review?
No, since v1 got no review.
Have you tested this?
Sure.
Here's how your v2 creates and initializes the qdev:
diff --git a/hw/pc.c b/hw/pc.c
index fcee09a..c698161 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1133,6 +1133,7 @@ void pc_basic_device_init(qemu_irq *isa_irq,
a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
i8042 = isa_create_simple(i8042);
i8042_setup_a20_line(i8042, a20_line[0]);
+ vmport_init();
vmmouse_init(i8042);
port92 = isa_create_simple(port92);
port92_init(port92, a20_line[1]);
This allocates a new VMPortState, and registers callbacks for port 5658:
@@ -100,12 +102,28 @@ static uint32_t vmport_cmd_ram_size(void *opaque,
uint32_t addr)
return ram_size;
}
-void vmport_init(void)
+static int vmport_initfn(ISADevice *dev)
{
- register_ioport_read(0x5658, 1, 4, vmport_ioport_read, port_state);
- register_ioport_write(0x5658, 1, 4, vmport_ioport_write, port_state);
+ VMPortState *s = DO_UPCAST(VMPortState, dev, dev);
+ register_ioport_read(0x5658, 1, 4, vmport_ioport_read, s);
+ register_ioport_write(0x5658, 1, 4, vmport_ioport_write, s);
+ isa_init_ioport(dev, 0x5658);
/* Register some generic port commands */
vmport_register(VMPORT_CMD_GETVERSION, vmport_cmd_get_version, NULL);
vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size, NULL);
+ return 0;
}
The callbacks receive the qdev VMPortState as argument.
vmport_register() still updates global port_state. I.e. it no longer
has any effect whatsoever on what the ports do.
Maybe I'm dense, but I can't see how this can work.
Good catch, it doesn't. Probably vmport_register() should take
ISADevice* parameter to provide the state, instead of using static
state (which would be easy one-line change).
But if all this is going to be thrown into ps2.c, it may
Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups
On 15 February 2011 15:02, Vincent Palatin vincent.palatin_q...@polytechnique.org wrote: This assumes that all the ARM SMP platforms are booting their secondary CPU the same way as the emulated Realview. For example, I'm currently writing a Tegra2 (dual A9) SoC emulation and the second CPU is halted when the platform starts and I cannot re-use the current smpboot firmware chunk. My current workaround is to use info-nb_cpus = 1 and do the init in the board code. Forcing the reset function will probably not help. My instinct here is to say models should follow the hardware. So if the Tegra2 SOC holds secondary cores in reset until you do something magic to a reset controller, the model should behave the same way. Realview boards just start all the cores at once, and that's how the board model ought to behave. The code in hw/arm_boot.c is really to my mind a debug convenience for passing a bare kernel. It's a secondary thing and shouldn't drive how we model what happens at reset. (I'd expect that a tegra2 model would probably not use arm_boot.c; the omap3 beagle model in meego doesn't. There's just too much stuff that the boot ROM and the boot loader need to do to go around emulating it all in qemu, so it's better to just pass an sd image or a ROM image that includes the boot loader and the kernel, I think.) -- PMM
Re: [Qemu-devel] [PATCH 06/10] vmmouse: convert to qdev
On Tue, Feb 15, 2011 at 7:22 PM, Blue Swirl blauwir...@gmail.com wrote: On Tue, Feb 15, 2011 at 12:07 PM, Markus Armbruster arm...@redhat.com wrote: Anthony Liguori anth...@codemonkey.ws writes: On 02/12/2011 11:03 AM, Markus Armbruster wrote: Blue Swirlblauwir...@gmail.com writes: Convert to qdev, also add a proper reset function. [...] Pointer properties are for dirty hacks only. Is there really no better solution? Why does it have to be a property? vmmouse is really just an extension to the PS2 Mouse. It's definitely not an ISA device. In terms of qdev enablement, I would just make it a boolean option to the PS2Mouse and not expose it as a top level device at all. It cannot exist without a PS2Mouse. Which means making it a separate qdev is wrong. That wrongness gave rise to the dirty pointer property. Pointer property serves as canary again. What now? I don't find pointer property use so dirty, but I'll try to combine the devices to see whether that makes sense. ps2.c is actually a library which implements core parts of PS/2 mouse and keyboard. It is used by pckbd.c (i8042) and pl050.c, so if we want to get rid of it, all three should be merged. Perhaps instead the file should be just renamed to libps2.c. As a side note, Makefile dependencies are not optimal, the file should only be compiled when either CONFIG_PCKBD is set (most architectures) or the target is ARM (pl050). vmport.c is only needed by vmmouse.c. It still implements some unrelated functions. vmmouse.c does not register any ISA ports by itself, so keeping it as a separate ISADevice does not make much sense. Merging would let us get rid of the useless registration, vmport.c is also compiled now even though it may be unused if there is no vmmouse. Merging pckbd.c with vmmouse.c: one problem is that pckbd.c is compiled in hwlib, but vmmouse via vmport needs to access CPU registers. Actually the interface between them is quite slim, only i8042_isa_mouse_fake_event(). Maybe this can be replaced with a qemu_irq, so we get rid of the pointer property.
[Qemu-devel] [PATCH] linux-user: Support the epoll syscalls
Support the epoll family of syscalls: epoll_create(), epoll_create1(),
epoll_ctl(), epoll_wait() and epoll_pwait(). Note that epoll_create1()
and epoll_pwait() are later additions, so we have to test separately
in configure for their presence.
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
configure | 54 +++
linux-user/syscall.c | 107 +
linux-user/syscall_defs.h | 13 +
3 files changed, 174 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 598e8e1..8b0fdcd 100755
--- a/configure
+++ b/configure
@@ -2136,6 +2136,51 @@ if compile_prog ; then
dup3=yes
fi
+# check for epoll support
+epoll=no
+cat $TMPC EOF
+#include sys/epoll.h
+
+int main(void)
+{
+epoll_create(0);
+return 0;
+}
+EOF
+if compile_prog $ARCH_CFLAGS ; then
+ epoll=yes
+fi
+
+# epoll_create1 and epoll_pwait are later additions
+# so we must check separately for their presence
+epoll_create1=no
+cat $TMPC EOF
+#include sys/epoll.h
+
+int main(void)
+{
+epoll_create1(0);
+return 0;
+}
+EOF
+if compile_prog $ARCH_CFLAGS ; then
+ epoll_create1=yes
+fi
+
+epoll_pwait=no
+cat $TMPC EOF
+#include sys/epoll.h
+
+int main(void)
+{
+epoll_pwait(0, 0, 0, 0, 0);
+return 0;
+}
+EOF
+if compile_prog $ARCH_CFLAGS ; then
+ epoll_pwait=yes
+fi
+
# Check if tools are available to build documentation.
if test $docs != no ; then
if has makeinfo has pod2man; then
@@ -2668,6 +2713,15 @@ fi
if test $dup3 = yes ; then
echo CONFIG_DUP3=y $config_host_mak
fi
+if test $epoll = yes ; then
+ echo CONFIG_EPOLL=y $config_host_mak
+fi
+if test $epoll_create1 = yes ; then
+ echo CONFIG_EPOLL_CREATE1=y $config_host_mak
+fi
+if test $epoll_pwait = yes ; then
+ echo CONFIG_EPOLL_PWAIT=y $config_host_mak
+fi
if test $inotify = yes ; then
echo CONFIG_INOTIFY=y $config_host_mak
fi
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 4412a9b..cf8a4c3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -66,6 +66,9 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
#ifdef CONFIG_EVENTFD
#include sys/eventfd.h
#endif
+#ifdef CONFIG_EPOLL
+#include sys/epoll.h
+#endif
#define termios host_termios
#define winsize host_winsize
@@ -7612,6 +7615,110 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
break;
#endif
#endif
+#if defined(CONFIG_EPOLL)
+#if defined(TARGET_NR_epoll_create)
+case TARGET_NR_epoll_create:
+ret = get_errno(epoll_create(arg1));
+break;
+#endif
+#if defined(TARGET_NR_epoll_create1) defined(CONFIG_EPOLL_CREATE1)
+case TARGET_NR_epoll_create1:
+ret = get_errno(epoll_create1(arg1));
+break;
+#endif
+#if defined(TARGET_NR_epoll_ctl)
+case TARGET_NR_epoll_ctl:
+{
+struct epoll_event ep;
+struct epoll_event *epp = 0;
+if (arg4) {
+struct target_epoll_event *target_ep;
+if (!lock_user_struct(VERIFY_READ, target_ep, arg4, 1)) {
+goto efault;
+}
+ep.events = tswap32(target_ep-events);
+/* The epoll_data_t union is just opaque data to the kernel,
+ * so we transfer all 64 bits across and need not worry what
+ * actual data type it is.
+ */
+ep.data.u64 = tswap64(target_ep-data.u64);
+unlock_user_struct(target_ep, arg4, 0);
+epp = ep;
+}
+ret = get_errno(epoll_ctl(arg1, arg2, arg3, epp));
+break;
+}
+#endif
+
+#if defined(TARGET_NR_epoll_pwait) defined(CONFIG_EPOLL_PWAIT)
+#define IMPLEMENT_EPOLL_PWAIT
+#endif
+#if defined(TARGET_NR_epoll_wait) || defined(IMPLEMENT_EPOLL_PWAIT)
+#if defined(TARGET_NR_epoll_wait)
+case TARGET_NR_epoll_wait:
+#endif
+#if defined(IMPLEMENT_EPOLL_PWAIT)
+case TARGET_NR_epoll_pwait:
+#endif
+{
+struct target_epoll_event *target_ep;
+struct epoll_event *ep;
+int epfd = arg1;
+int maxevents = arg3;
+int timeout = arg4;
+
+target_ep = lock_user(VERIFY_WRITE, arg2,
+ maxevents * sizeof(struct target_epoll_event),
1);
+if (!target_ep) {
+goto efault;
+}
+
+ep = alloca(maxevents * sizeof(struct epoll_event));
+
+switch (num) {
+#if defined(IMPLEMENT_EPOLL_PWAIT)
+case TARGET_NR_epoll_pwait:
+{
+target_sigset_t *target_set;
+sigset_t _set, *set = _set;
+
+if (arg5) {
+target_set = lock_user(VERIFY_READ, arg5,
+ sizeof(target_sigset_t), 1);
+if (!target_set) {
+unlock_user(target_ep, arg2, 0);
+goto efault;
+}
+target_to_host_sigset(set, target_set);
+unlock_user(target_set, arg5, 0);
+} else {
+
[Qemu-devel] [PATCH] fix halt emulation with icount and CONFIG_IOTHREAD
Note: to be applied to uq/master.
In icount mode, halt emulation should take into account the nearest event when
sleeping.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
Reported-and-tested-by: Edgar E. Iglesias edgar.igles...@gmail.com
diff --git a/cpus.c b/cpus.c
index 468544c..21c3eba 100644
--- a/cpus.c
+++ b/cpus.c
@@ -770,7 +770,7 @@ static void qemu_tcg_wait_io_event(void)
CPUState *env;
while (all_cpu_threads_idle()) {
-qemu_cond_timedwait(tcg_halt_cond, qemu_global_mutex, 1000);
+qemu_cond_timedwait(tcg_halt_cond, qemu_global_mutex,
qemu_calculate_timeout());
}
qemu_mutex_unlock(qemu_global_mutex);
diff --git a/vl.c b/vl.c
index b436952..8ba7e9d 100644
--- a/vl.c
+++ b/vl.c
@@ -1335,7 +1335,7 @@ void main_loop_wait(int nonblocking)
if (nonblocking)
timeout = 0;
else {
-timeout = qemu_calculate_timeout();
+timeout = 1000;
qemu_bh_update_timeout(timeout);
}
[Qemu-devel] Re: [PATCH] fix halt emulation with icount and CONFIG_IOTHREAD
On 2011-02-15 18:54, Marcelo Tosatti wrote:
Note: to be applied to uq/master.
In icount mode, halt emulation should take into account the nearest event
when sleeping.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
Reported-and-tested-by: Edgar E. Iglesias edgar.igles...@gmail.com
diff --git a/cpus.c b/cpus.c
index 468544c..21c3eba 100644
--- a/cpus.c
+++ b/cpus.c
@@ -770,7 +770,7 @@ static void qemu_tcg_wait_io_event(void)
CPUState *env;
while (all_cpu_threads_idle()) {
-qemu_cond_timedwait(tcg_halt_cond, qemu_global_mutex, 1000);
+qemu_cond_timedwait(tcg_halt_cond, qemu_global_mutex,
qemu_calculate_timeout());
checkpatch.pl would complain here.
More important: Paolo was proposing patches to eliminate all those fishy
cond_wait timeouts. That's probably the better way to go. The timeouts
only paper over missing signaling.
}
qemu_mutex_unlock(qemu_global_mutex);
diff --git a/vl.c b/vl.c
index b436952..8ba7e9d 100644
--- a/vl.c
+++ b/vl.c
@@ -1335,7 +1335,7 @@ void main_loop_wait(int nonblocking)
if (nonblocking)
timeout = 0;
else {
-timeout = qemu_calculate_timeout();
+timeout = 1000;
qemu_bh_update_timeout(timeout);
}
Isn't this path also relevant for !IOTHREAD? What's the impact of this
change for that configuration?
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
[Qemu-devel] Re: Comparing New Image Formats: FVD vs. QED
Chunqiang Tang/Watson/IBM wrote on 01/28/2011 05:13:27 PM: As you requested, I set up a wiki page for FVD at http://wiki.qemu.org/Features/FVD . It includes a summary of FVD, a detailed specification of FVD, and a comparison of the design and performance of FVD and QED. See the figure at http://wiki.qemu.org/Features/FVD/Compare . This figure shows that the file creation throughput of NetApp's PostMark benchmark under FVD is 74.9% to 215% higher than that under QED. Hi Anthony, Please let me know if more information is needed. I would appreciate your feedback and advice on the best way to proceed with FVD. BTW, I recently added QCOW2 into the performance comparison figure on wiki. Regards, ChunQiang (CQ) Tang Homepage: http://www.research.ibm.com/people/c/ctang
[Qemu-devel] Re: [PATCH] fix halt emulation with icount and CONFIG_IOTHREAD
On Tue, Feb 15, 2011 at 07:58:53PM +0100, Jan Kiszka wrote:
On 2011-02-15 18:54, Marcelo Tosatti wrote:
Note: to be applied to uq/master.
In icount mode, halt emulation should take into account the nearest event
when sleeping.
Signed-off-by: Marcelo Tosatti mtosa...@redhat.com
Reported-and-tested-by: Edgar E. Iglesias edgar.igles...@gmail.com
diff --git a/cpus.c b/cpus.c
index 468544c..21c3eba 100644
--- a/cpus.c
+++ b/cpus.c
@@ -770,7 +770,7 @@ static void qemu_tcg_wait_io_event(void)
CPUState *env;
while (all_cpu_threads_idle()) {
-qemu_cond_timedwait(tcg_halt_cond, qemu_global_mutex, 1000);
+qemu_cond_timedwait(tcg_halt_cond, qemu_global_mutex,
qemu_calculate_timeout());
checkpatch.pl would complain here.
More important: Paolo was proposing patches to eliminate all those fishy
cond_wait timeouts. That's probably the better way to go. The timeouts
only paper over missing signaling.
}
qemu_mutex_unlock(qemu_global_mutex);
diff --git a/vl.c b/vl.c
index b436952..8ba7e9d 100644
--- a/vl.c
+++ b/vl.c
@@ -1335,7 +1335,7 @@ void main_loop_wait(int nonblocking)
if (nonblocking)
timeout = 0;
else {
-timeout = qemu_calculate_timeout();
+timeout = 1000;
qemu_bh_update_timeout(timeout);
}
Isn't this path also relevant for !IOTHREAD? What's the impact of this
change for that configuration?
Timeout changes from 5s to 1s.
[Qemu-devel] Is this an invalid combination?
Hi, We just noticed an issue flagged by a libvirt based test. This same command line didn't used to fail, and I wanted to be sure that this is behaving as intended. When the following command line is used on the current qemu version: x86_64-softmmu/qemu-system-x86_64 -kernel /boot/vmlinuz -drive file=~/disk0.raw,if=none,id=foo,boot=on -device virtio-blk-pci,drive=foo We get the following error reported: Two devices with same boot index 0 Previous versions of qemu did not flag this as an error condition. I can see that we are indicating two different boot sources here, so I would guess the command line is invalid, but wanted to be sure. Bruce
[Qemu-devel] [PATCH 1/2] linux-user: add rmdir() strace
Signed-off-by: Laurent Vivier laur...@vivier.eu
---
linux-user/strace.c| 12
linux-user/strace.list |3 +++
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 183..05277c0 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -876,6 +876,18 @@ print_mkdirat(const struct syscallname *name,
}
#endif
+#ifdef TARGET_NR_rmdir
+static void
+print_rmdir(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_string(arg0, 0);
+print_syscall_epilogue(name);
+}
+#endif
+
#ifdef TARGET_NR_mknod
static void
print_mknod(const struct syscallname *name,
diff --git a/linux-user/strace.list b/linux-user/strace.list
index d7be0e7..563a67f 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -495,6 +495,9 @@
#ifdef TARGET_NR_mkdirat
{ TARGET_NR_mkdirat, mkdirat , NULL, print_mkdirat, NULL },
#endif
+#ifdef TARGET_NR_rmdir
+{ TARGET_NR_rmdir, rmdir , NULL, print_rmdir, NULL },
+#endif
#ifdef TARGET_NR_mknod
{ TARGET_NR_mknod, mknod , NULL, print_mknod, NULL },
#endif
--
1.7.1
[Qemu-devel] [PATCH 2/2] linux-user: in linux-user/strace.c, tswap() is useless
Syscall parameters are already swapped by the caller.
This patch removes useless tswap() from strace.c
$ QEMU_STRACE=1 chroot /m68k mknod myramdisk b 1 1
with tswap()
...
29944 mknod(myramdisk,02663020) = 0
...
without tswap()
...
30042 mknod(myramdisk,S_IFBLK|0666,makedev(1,1)) = 0
...
natively:
$ strace touch mytouch
...
open(mytouch, O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666) = 3
...
$ QEMU_STRACE=1 chroot /m68k touch mytouch
with tswap()
...
30368 open(/usr/share/locale/locale.alias,O_RDONLY) = 3
30368 fstat64(50331648,0x4080032c) = 0
...
30368 open(mytouch,O_RDONLY|O_CREAT|O_LARGEFILE|O_NOCTTY|O_NONBLOCK|0x1) = 0
...
without tswap()
...
30572 open(/usr/share/locale/locale.alias,O_RDONLY) = 3
30572 fstat64(3,0x4080032c) = 0
...
30572 open(mytouch,O_WRONLY|O_CREAT|O_LARGEFILE|O_NOCTTY|O_NONBLOCK,0666) = 0
Signed-off-by: Laurent Vivier laur...@vivier.eu
---
linux-user/strace.c | 71 +--
1 files changed, 29 insertions(+), 42 deletions(-)
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 05277c0..86ad5aa 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -441,14 +441,11 @@ get_comma(int last)
}
static void
-print_flags(const struct flags *f, abi_long tflags, int last)
+print_flags(const struct flags *f, abi_long flags, int last)
{
const char *sep = ;
-int flags;
int n;
-flags = (int)tswap32(tflags);
-
if ((flags == 0) (f-f_value == 0)) {
gemu_log(%s%s, f-f_string, get_comma(last));
return;
@@ -476,10 +473,8 @@ print_flags(const struct flags *f, abi_long tflags, int
last)
}
static void
-print_at_dirfd(abi_long tdirfd, int last)
+print_at_dirfd(abi_long dirfd, int last)
{
-int dirfd = tswap32(tdirfd);
-
#ifdef AT_FDCWD
if (dirfd == AT_FDCWD) {
gemu_log(AT_FDCWD%s, get_comma(last));
@@ -490,11 +485,10 @@ print_at_dirfd(abi_long tdirfd, int last)
}
static void
-print_file_mode(abi_long tmode, int last)
+print_file_mode(abi_long mode, int last)
{
const char *sep = ;
const struct flags *m;
-mode_t mode = (mode_t)tswap32(tmode);
for (m = mode_flags[0]; m-f_string != NULL; m++) {
if ((m-f_value mode) == m-f_value) {
@@ -514,10 +508,8 @@ print_file_mode(abi_long tmode, int last)
}
static void
-print_open_flags(abi_long tflags, int last)
+print_open_flags(abi_long flags, int last)
{
-int flags = tswap32(tflags);
-
print_flags(open_access_flags, flags TARGET_O_ACCMODE, 1);
flags = ~TARGET_O_ACCMODE;
if (flags == 0) {
@@ -620,7 +612,7 @@ print_accept(const struct syscallname *name,
abi_long arg3, abi_long arg4, abi_long arg5)
{
print_syscall_prologue(name);
-print_raw_param(%d, tswap32(arg0), 0);
+print_raw_param(%d, arg0, 0);
print_pointer(arg1, 0);
print_number(arg2, 1);
print_syscall_epilogue(name);
@@ -698,7 +690,7 @@ print_execv(const struct syscallname *name,
{
print_syscall_prologue(name);
print_string(arg0, 0);
-print_raw_param(0x TARGET_ABI_FMT_lx, tswapl(arg1), 1);
+print_raw_param(0x TARGET_ABI_FMT_lx, arg1, 1);
print_syscall_epilogue(name);
}
#endif
@@ -742,13 +734,8 @@ print_fchownat(const struct syscallname *name,
print_syscall_prologue(name);
print_at_dirfd(arg0, 0);
print_string(arg1, 0);
-#ifdef USE_UID16
-print_raw_param(%d, tswap16(arg2), 0);
-print_raw_param(%d, tswap16(arg3), 0);
-#else
-print_raw_param(%d, tswap32(arg2), 0);
-print_raw_param(%d, tswap32(arg3), 0);
-#endif
+print_raw_param(%d, arg2, 0);
+print_raw_param(%d, arg3, 0);
print_flags(at_file_flags, arg4, 1);
print_syscall_epilogue(name);
}
@@ -761,7 +748,7 @@ print_fcntl(const struct syscallname *name,
abi_long arg3, abi_long arg4, abi_long arg5)
{
print_syscall_prologue(name);
-print_raw_param(%d, tswap32(arg0), 0);
+print_raw_param(%d, arg0, 0);
print_flags(fcntl_flags, arg1, 0);
/*
* TODO: check flags and print following argument only
@@ -842,7 +829,7 @@ print_fstat(const struct syscallname *name,
abi_long arg3, abi_long arg4, abi_long arg5)
{
print_syscall_prologue(name);
-print_raw_param(%d, tswap32(arg0), 0);
+print_raw_param(%d, arg0, 0);
print_pointer(arg1, 1);
print_syscall_epilogue(name);
}
@@ -894,14 +881,14 @@ print_mknod(const struct syscallname *name,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
-int hasdev = (tswapl(arg1) (S_IFCHR|S_IFBLK));
+int hasdev = (arg1 (S_IFCHR|S_IFBLK));
print_syscall_prologue(name);
print_string(arg0, 0);
print_file_mode(arg1, (hasdev == 0));
if (hasdev) {
-print_raw_param(makedev(%d, major(tswapl(arg2)), 0);
-print_raw_param(%d), minor(tswapl(arg2)), 1);
+print_raw_param(makedev(%d, major(arg2), 0);
+print_raw_param(%d), minor(arg2), 1);
}
print_syscall_epilogue(name);
}
Re: [Qemu-devel] [FYI] memory leak in 0.14.0rc1 ?
2011/2/15 Torsten Förtsch torsten.foert...@gmx.net: On Tuesday, February 15, 2011 15:43:32 Stefan Hajnoczi wrote: I have installed winxp and run the machine as /usr/bin/qemu-kvm -name xp.home -m 768 Are you able to try QEMU 0.14.0-rc2 from source? $ git clone git://git.qemu.org/qemu.git $ git checkout v0.14.0-rc2 $ ./configure --target-list=x86_64-softmmu --enable-io-thread --disable-strip --prefix=/usr $ make $ x86_64-softmmu/qemu-system-x86_64 -enable-kvm -m 768 -name xp.home ... Now, the process size stays around 1300 Mb and RSS is very constant at 794 Mb. Thank you for checking this. This is probably a Suse-specific or qemu-kvm issue. But the VM is much slower. Although, according to info kvm kvm is enabled. What options do I have to use to get optimal performance? Is it general sluggishness or do you notice something specific like disk or networking? I usually set -drive cache=none which tries to bypass the host page cache and does not sync to disk after every write operation. Stefan
Re: [Qemu-devel] Is this an invalid combination?
On 02/15/2011 02:07 PM, Bruce Rogers wrote: Hi, We just noticed an issue flagged by a libvirt based test. This same command line didn't used to fail, and I wanted to be sure that this is behaving as intended. When the following command line is used on the current qemu version: x86_64-softmmu/qemu-system-x86_64 -kernel /boot/vmlinuz -drive file=~/disk0.raw,if=none,id=foo,boot=on -device virtio-blk-pci,drive=foo We get the following error reported: Two devices with same boot index 0 Previous versions of qemu did not flag this as an error condition. Upstream QEMU does not have a boolean boot flag although I guess we ignore it in -drive which sucks :-/ In upstream QEMU, the BIOS can boot just fine from a virtio device. What you're seeing is that we've apparently overloaded the boot flag in upstream qemu to mean boot index. Gleb, what's the right invocation here? Regards, ANthony Liguori I can see that we are indicating two different boot sources here, so I would guess the command line is invalid, but wanted to be sure. Bruce
Re: [Qemu-devel] Is this an invalid combination?
On Tue, Feb 15, 2011 at 02:21:41PM -0600, Anthony Liguori wrote: On 02/15/2011 02:07 PM, Bruce Rogers wrote: Hi, We just noticed an issue flagged by a libvirt based test. This same command line didn't used to fail, and I wanted to be sure that this is behaving as intended. When the following command line is used on the current qemu version: x86_64-softmmu/qemu-system-x86_64 -kernel /boot/vmlinuz -drive file=~/disk0.raw,if=none,id=foo,boot=on -device virtio-blk-pci,drive=foo We get the following error reported: Two devices with same boot index 0 Previous versions of qemu did not flag this as an error condition. Upstream QEMU does not have a boolean boot flag although I guess we ignore it in -drive which sucks :-/ In upstream QEMU, the BIOS can boot just fine from a virtio device. What you're seeing is that we've apparently overloaded the boot flag in upstream qemu to mean boot index. Gleb, what's the right invocation here? Just drop boot=on. Qemu-kvm registers extboot and some other bootrom (which one?) with the same boot index. This should be fixed, but dropping boot=on is the right solution in any case. Actually I want to remove extboot from qemu-kvm at all. It will not make it upstream anyway. Regards, ANthony Liguori I can see that we are indicating two different boot sources here, so I would guess the command line is invalid, but wanted to be sure. Bruce -- Gleb.
