[Qemu-devel] [PATCH 3/3] deal with guest panicked event

[Wen Congyang]

When the guest is panicked, it will write 0x1 to the port 0x505. So if
qemu reads 0x1 from this port, we can do the folloing three things
according to the parameter -onpanic:
1. emit QEVENT_GUEST_PANICKED only
2. emit QEVENT_GUEST_PANICKED and pause VM
3. emit QEVENT_GUEST_PANICKED and quit VM

Note: if we emit QEVENT_GUEST_PANICKED only, and the management
application does not receive this event(the management may not
run when the event is emitted), the management won't know the
guest is panicked.

Signed-off-by: Wen Congyang 
---
 kvm-all.c|   84 ++
 kvm-stub.c   |9 ++
 kvm.h|3 ++
 monitor.c|3 ++
 monitor.h|1 +
 qapi-schema.json |6 +++-
 qemu-options.hx  |   14 +
 qmp.c|3 +-
 vl.c |   17 ++-
 9 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 9b73ccf..b5b0531 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -19,6 +19,7 @@
 #include 
 
 #include 
+#include 
 
 #include "qemu-common.h"
 #include "qemu-barrier.h"
@@ -29,6 +30,8 @@
 #include "bswap.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "iorange.h"
+#include "qemu-objects.h"
 
 /* This check must be after config-host.h is included */
 #ifdef CONFIG_EVENTFD
@@ -1707,3 +1710,84 @@ int kvm_on_sigbus(int code, void *addr)
 {
 return kvm_arch_on_sigbus(code, addr);
 }
+
+/* Possible values for action parameter. */
+#define PANICKED_REPORT 1   /* emit QEVENT_GUEST_PANICKED only */
+#define PANICKED_PAUSE  2   /* emit QEVENT_GUEST_PANICKED and pause VM */
+#define PANICKED_QUIT   3   /* emit QEVENT_GUEST_PANICKED and quit VM */
+
+static int panicked_action = PANICKED_REPORT;
+
+static void kvm_pv_port_read(IORange *iorange, uint64_t offset, unsigned width,
+ uint64_t *data)
+{
+*data = (1 << KVM_PV_FEATURE_PANICKED);
+}
+
+static void panicked_mon_event(const char *action)
+{
+QObject *data;
+
+data = qobject_from_jsonf("{ 'action': %s }", action);
+monitor_protocol_event(QEVENT_GUEST_PANICKED, data);
+qobject_decref(data);
+}
+
+static void panicked_perform_action(void)
+{
+switch(panicked_action) {
+case PANICKED_REPORT:
+panicked_mon_event("report");
+break;
+
+case PANICKED_PAUSE:
+panicked_mon_event("pause");
+vm_stop(RUN_STATE_GUEST_PANICKED);
+break;
+
+case PANICKED_QUIT:
+panicked_mon_event("quit");
+exit(0);
+break;
+}
+}
+
+static void kvm_pv_port_write(IORange *iorange, uint64_t offset, unsigned 
width,
+  uint64_t data)
+{
+if (data == KVM_PV_PANICKED)
+panicked_perform_action();
+}
+
+static void kvm_pv_port_destructor(IORange *iorange)
+{
+g_free(iorange);
+}
+
+static IORangeOps pv_io_range_ops = {
+.read = kvm_pv_port_read,
+.write = kvm_pv_port_write,
+.destructor = kvm_pv_port_destructor,
+};
+
+void kvm_pv_port_init(void)
+{
+IORange *pv_io_range = g_malloc(sizeof(IORange));
+
+iorange_init(pv_io_range, &pv_io_range_ops, 0x505, 1);
+ioport_register(pv_io_range);
+}
+
+int select_panicked_action(const char *p)
+{
+if (strcasecmp(p, "report") == 0)
+panicked_action = PANICKED_REPORT;
+else if (strcasecmp(p, "pause") == 0)
+panicked_action = PANICKED_PAUSE;
+else if (strcasecmp(p, "quit") == 0)
+panicked_action = PANICKED_QUIT;
+else
+return -1;
+
+return 0;
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index 47c573d..4cf977e 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -128,3 +128,12 @@ int kvm_on_sigbus(int code, void *addr)
 {
 return 1;
 }
+
+void kvm_pv_port_init(void)
+{
+}
+
+int select_panicked_action(const char *p)
+{
+return -1;
+}
diff --git a/kvm.h b/kvm.h
index 4ccae8c..95075cf 100644
--- a/kvm.h
+++ b/kvm.h
@@ -60,6 +60,9 @@ int kvm_has_gsi_routing(void);
 
 int kvm_allows_irq0_override(void);
 
+void kvm_pv_port_init(void);
+int select_panicked_action(const char *p);
+
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUArchState *env);
 
diff --git a/monitor.c b/monitor.c
index 12a6fe2..83cb059 100644
--- a/monitor.c
+++ b/monitor.c
@@ -493,6 +493,9 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 case QEVENT_WAKEUP:
 event_name = "WAKEUP";
 break;
+case QEVENT_GUEST_PANICKED:
+event_name = "GUEST_PANICKED";
+break;
 default:
 abort();
 break;
diff --git a/monitor.h b/monitor.h
index 0d49800..94e8a3c 100644
--- a/monitor.h
+++ b/monitor.h
@@ -41,6 +41,7 @@ typedef enum MonitorEvent {
 QEVENT_DEVICE_TRAY_MOVED,
 QEVENT_SUSPEND,
 QEVENT_WAKEUP,
+QEVENT_GUEST_PANICKED,
 QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/qapi-schema.json b/qapi-schema.json
index 2ca7195..ee5c9e9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -119,11 +119,15 @@
 # @

[Qemu-devel] [PATCH 2/3] update linux headers

[Wen Congyang]

Signed-off-by: Wen Congyang 
---
 linux-headers/linux/kvm_para.h |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/linux-headers/linux/kvm_para.h b/linux-headers/linux/kvm_para.h
index 7bdcf93..5618758 100644
--- a/linux-headers/linux/kvm_para.h
+++ b/linux-headers/linux/kvm_para.h
@@ -20,6 +20,14 @@
 #define KVM_HC_FEATURES3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE  4
 
+#define KVM_PV_PORT(0x505UL)
+
+/* The bit of the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED1
+
 /*
  * hypercalls use architecture specific
  */
-- 
1.7.1

[Qemu-devel] [PATCH 1/3] start vm after reseting it

[Wen Congyang]

The guest should run after reseting it, but it does not
run if its old state is RUN_STATE_INTERNAL_ERROR or RUN_STATE_PAUSED.

Signed-off-by: Wen Congyang 
---
 vl.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 23ab3a3..7f5fed8 100644
--- a/vl.c
+++ b/vl.c
@@ -1539,6 +1539,7 @@ static bool main_loop_should_exit(void)
 if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
 runstate_check(RUN_STATE_SHUTDOWN)) {
 runstate_set(RUN_STATE_PAUSED);
+vm_start();
 }
 }
 if (qemu_powerdown_requested()) {
-- 
1.7.1

[Qemu-devel] [PATCH] kvm: notify host when guest panicked

[Wen Congyang]

We can know the guest is panicked when the guest runs on xen.
But we do not have such feature on kvm.

Another purpose of this feature is: management app(for example:
libvirt) can do auto dump when the guest is panicked. If management
app does not do auto dump, the guest's user can do dump by hand if
he sees the guest is panicked.

We have three solutions to implement this feature:
1. use vmcall
2. use I/O port
3. use virtio-serial.

We have decided to avoid touching hypervisor. The reason why I choose
choose the I/O port is:
1. it is easier to implememt
2. it does not depend any virtual device
3. it can work when startint the kernel

Signed-off-by: Wen Congyang 
---
 arch/ia64/include/asm/kvm_para.h|5 +
 arch/powerpc/include/asm/kvm_para.h |5 +
 arch/s390/include/asm/kvm_para.h|5 +
 arch/x86/include/asm/kvm_para.h |7 +++
 arch/x86/kernel/kvm.c   |   14 ++
 include/linux/kvm_para.h|   15 +++
 6 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_para.h b/arch/ia64/include/asm/kvm_para.h
index 1588aee..a890096 100644
--- a/arch/ia64/include/asm/kvm_para.h
+++ b/arch/ia64/include/asm/kvm_para.h
@@ -26,6 +26,11 @@ static inline unsigned int kvm_arch_para_features(void)
return 0;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+   return 0;
+}
+
 #endif
 
 #endif
diff --git a/arch/powerpc/include/asm/kvm_para.h 
b/arch/powerpc/include/asm/kvm_para.h
index 7b754e7..b5f7c35 100644
--- a/arch/powerpc/include/asm/kvm_para.h
+++ b/arch/powerpc/include/asm/kvm_para.h
@@ -206,6 +206,11 @@ static inline unsigned int kvm_arch_para_features(void)
return r;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+   return 0;
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* __POWERPC_KVM_PARA_H__ */
diff --git a/arch/s390/include/asm/kvm_para.h b/arch/s390/include/asm/kvm_para.h
index 6964db2..21a8d18 100644
--- a/arch/s390/include/asm/kvm_para.h
+++ b/arch/s390/include/asm/kvm_para.h
@@ -149,6 +149,11 @@ static inline unsigned int kvm_arch_para_features(void)
return 0;
 }
 
+static inline unsigned int kvm_arch_pv_features(void)
+{
+   return 0;
+}
+
 #endif
 
 #endif /* __S390_KVM_PARA_H */
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 734c376..f66b16e 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -208,6 +208,13 @@ static inline void kvm_disable_steal_time(void)
 }
 #endif
 
+#ifdef KVM_PV_PORT
+static inline unsigned int kvm_arch_pv_features(void)
+{
+   return inl(KVM_PV_PORT);
+}
+#endif
+
 #endif /* __KERNEL__ */
 
 #endif /* _ASM_X86_KVM_PARA_H */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index b8ba6e4..adfde45 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -335,6 +335,17 @@ static struct notifier_block kvm_pv_reboot_nb = {
.notifier_call = kvm_pv_reboot_notify,
 };
 
+static int
+kvm_pv_panic_notify(struct notifier_block *nb, unsigned long code, void 
*unused)
+{
+   outl(KVM_PV_PANICKED, KVM_PV_PORT);
+   return NOTIFY_DONE;
+}
+
+static struct notifier_block kvm_pv_panic_nb = {
+   .notifier_call = kvm_pv_panic_notify,
+};
+
 static u64 kvm_steal_clock(int cpu)
 {
u64 steal;
@@ -421,6 +432,9 @@ void __init kvm_guest_init(void)
 
paravirt_ops_setup();
register_reboot_notifier(&kvm_pv_reboot_nb);
+   if (kvm_pv_has_feature(KVM_PV_FEATURE_PANICKED))
+   atomic_notifier_chain_register(&panic_notifier_list,
+   &kvm_pv_panic_nb);
for (i = 0; i < KVM_TASK_SLEEP_HASHSIZE; i++)
spin_lock_init(&async_pf_sleepers[i].lock);
if (kvm_para_has_feature(KVM_FEATURE_ASYNC_PF))
diff --git a/include/linux/kvm_para.h b/include/linux/kvm_para.h
index ff476dd..849b0d5 100644
--- a/include/linux/kvm_para.h
+++ b/include/linux/kvm_para.h
@@ -20,6 +20,14 @@
 #define KVM_HC_FEATURES3
 #define KVM_HC_PPC_MAP_MAGIC_PAGE  4
 
+#define KVM_PV_PORT(0x505UL)
+
+/* The bit of the value read from KVM_PV_PORT */
+#define KVM_PV_FEATURE_PANICKED0
+
+/* The value writen to KVM_PV_PORT */
+#define KVM_PV_PANICKED1
+
 /*
  * hypercalls use architecture specific
  */
@@ -33,5 +41,12 @@ static inline int kvm_para_has_feature(unsigned int feature)
return 1;
return 0;
 }
+
+static inline int kvm_pv_has_feature(unsigned int feature)
+{
+   if (kvm_arch_pv_features() & (1UL << feature))
+   return 1;
+   return 0;
+}
 #endif /* __KERNEL__ */
 #endif /* __LINUX_KVM_PARA_H */
-- 
1.7.1

Re: [Qemu-devel] ppc: CPU reset must flush translation buffer

[Benjamin Herrenschmidt]

On Mon, 2012-05-21 at 08:16 +0200, Alexander Graf wrote:
> 
> On 21.05.2012, at 04:01, Benjamin Herrenschmidt
>  wrote:
> 
> > Without that, reset from SLOF crashes in full emulation.
> > 
> > Reported-by: Thomas Huth 
> > Signed-off-by: Benjamin Herrenschmidt 
> > ---
> > target-ppc/translate_init.c |1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/target-ppc/translate_init.c
> b/target-ppc/translate_init.c
> > index ae03065..fbf7705 100644
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -10285,6 +10285,7 @@ static void ppc_cpu_reset(CPUState *s)
> > env->error_code = 0;
> > /* Flush all TLBs */
> > tlb_flush(env, 1);
> > +tb_flush(env);
> 
> Shouldn't this be true for all CPUs? I remember talking about reset
> with Peter a while ago... but don't remember the conclusions :)

Possibly. I noticed other targets do that too (ARM iirc), in this case I
think it's the ROM being reloaded that doesn't flush the cached
translations for the vectors (I -think-, that's from memory). But there
could be all sort of other context changes, so it seems like the safest
thing to do.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH 01/15] Openrisc: add target stub

[Jia Liu]

Hi Blue,

On Sat, May 19, 2012 at 4:51 PM, Blue Swirl  wrote:
> On Thu, May 17, 2012 at 8:35 AM, Jia Liu  wrote:
>> add the openrisc target stub and basic implementation.
>>
>> Signed-off-by: Jia Liu 
>> ---
>>  Makefile.target                  |    3 +
>>  arch_init.c                      |    2 +
>>  arch_init.h                      |    1 +
>>  configure                        |    8 +-
>>  cpu-exec.c                       |    2 +
>>  default-configs/or32-softmmu.mak |    6 +
>>  elf.h                            |    2 +
>>  poison.h                         |    1 +
>>  target-openrisc/cpu-qom.h        |   70 +
>>  target-openrisc/cpu.c            |   74 ++
>>  target-openrisc/cpu.h            |  299 
>> ++
>>  target-openrisc/helper.c         |   67 +
>>  target-openrisc/helper.h         |   23 +++
>>  target-openrisc/machine.c        |   76 ++
>>  target-openrisc/mem.c            |   43 ++
>>  target-openrisc/mem_helper.c     |   46 ++
>>  target-openrisc/translate.c      |   90 
>>  17 files changed, 812 insertions(+), 1 deletion(-)
>>  create mode 100644 default-configs/or32-softmmu.mak
>>  create mode 100644 target-openrisc/cpu-qom.h
>>  create mode 100644 target-openrisc/cpu.c
>>  create mode 100644 target-openrisc/cpu.h
>>  create mode 100644 target-openrisc/helper.c
>>  create mode 100644 target-openrisc/helper.h
>>  create mode 100644 target-openrisc/machine.c
>>  create mode 100644 target-openrisc/mem.c
>>  create mode 100644 target-openrisc/mem_helper.c
>>  create mode 100644 target-openrisc/translate.c
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 1582904..0ffb29c 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -83,9 +83,11 @@ libobj-$(CONFIG_TCG_INTERPRETER) += tci.o
>>  libobj-y += fpu/softfloat.o
>>  ifneq ($(TARGET_BASE_ARCH), sparc)
>>  ifneq ($(TARGET_BASE_ARCH), alpha)
>> +ifneq ($(TARGET_BASE_ARCH), openrisc)
>>  libobj-y += op_helper.o
>>  endif
>>  endif
>> +endif
>>  libobj-y += helper.o
>>  ifneq ($(TARGET_BASE_ARCH), ppc)
>>  libobj-y += cpu.o
>> @@ -99,6 +101,7 @@ endif
>>  libobj-$(TARGET_SPARC) += int32_helper.o
>>  libobj-$(TARGET_SPARC64) += int64_helper.o
>>  libobj-$(TARGET_ALPHA) += int_helper.o fpu_helper.o sys_helper.o 
>> mem_helper.o
>> +libobj-$(TARGET_OPENRISC) += mem.o mem_helper.o
>>
>>  libobj-y += disas.o
>>  libobj-$(CONFIG_TCI_DIS) += tci-dis.o
>> diff --git a/arch_init.c b/arch_init.c
>> index 988adca..55b608d 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -71,6 +71,8 @@ int graphic_depth = 15;
>>  #define QEMU_ARCH QEMU_ARCH_MICROBLAZE
>>  #elif defined(TARGET_MIPS)
>>  #define QEMU_ARCH QEMU_ARCH_MIPS
>> +#elif defined(TARGET_OPENRISC)
>> +#define QEMU_ARCH QEMU_ARCH_OPENRISC
>>  #elif defined(TARGET_PPC)
>>  #define QEMU_ARCH QEMU_ARCH_PPC
>>  #elif defined(TARGET_S390X)
>> diff --git a/arch_init.h b/arch_init.h
>> index c7cb94a..3dfea3b 100644
>> --- a/arch_init.h
>> +++ b/arch_init.h
>> @@ -16,6 +16,7 @@ enum {
>>     QEMU_ARCH_SH4 = 1024,
>>     QEMU_ARCH_SPARC = 2048,
>>     QEMU_ARCH_XTENSA = 4096,
>> +    QEMU_ARCH_OPENRISC = 8192,
>>  };
>>
>>  extern const uint32_t arch_type;
>> diff --git a/configure b/configure
>> index b55a792..63e2372 100755
>> --- a/configure
>> +++ b/configure
>> @@ -924,6 +924,7 @@ mips-softmmu \
>>  mipsel-softmmu \
>>  mips64-softmmu \
>>  mips64el-softmmu \
>> +or32-softmmu \
>>  ppc-softmmu \
>>  ppcemb-softmmu \
>>  ppc64-softmmu \
>> @@ -3460,7 +3461,7 @@ target_arch2=`echo $target | cut -d '-' -f 1`
>>  target_bigendian="no"
>>
>>  case "$target_arch2" in
>> -  
>> armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
>> +  
>> armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|or32|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
>>   target_bigendian=yes
>>   ;;
>>  esac
>> @@ -3588,6 +3589,11 @@ case "$target_arch2" in
>>     target_phys_bits=64
>>     target_long_alignment=8
>>   ;;
>> +  or32)
>> +    TARGET_ARCH=openrisc
>> +    TARGET_BASE_ARCH=openrisc
>> +    target_phys_bits=32
>> +  ;;
>>   ppc)
>>     gdb_xml_files="power-core.xml power-fpu.xml power-altivec.xml 
>> power-spe.xml"
>>     target_phys_bits=64
>> diff --git a/cpu-exec.c b/cpu-exec.c
>> index 0344cd5..ba10db1 100644
>> --- a/cpu-exec.c
>> +++ b/cpu-exec.c
>> @@ -222,6 +222,7 @@ int cpu_exec(CPUArchState *env)
>>  #elif defined(TARGET_LM32)
>>  #elif defined(TARGET_MICROBLAZE)
>>  #elif defined(TARGET_MIPS)
>> +#elif defined(TARGET_OPENRISC)
>>  #elif defined(TARGET_SH4)
>>  #elif defined(TARGET_CRIS)
>>  #elif defined(TARGET_S390X)
>> @@ -620,6 +621,7 @@ int cpu_exec(CPUArchState *env)
>>               | env->cc_dest | (env->cc_x << 4);
>>  #elif defined(TARGET_MICROBLAZE)
>>  #elif defined(TARGET_MIPS)
>> +#elif defined(TARGET_OPENRISC)
>>  #elif defined(TARGET_SH4)
>>  #elif defined(TARGET_ALPHA)
>>  #elif defined(TARGET_CRI

Re: [Qemu-devel] [PATCH 02/15] Openrisc: add MMU support

[Jia Liu]

Hi Blue,

On Sat, May 19, 2012 at 3:41 PM, Blue Swirl  wrote:
> On Thu, May 17, 2012 at 8:35 AM, Jia Liu  wrote:
>> add the openrisc MMU support.
>>
>> Signed-off-by: Jia Liu 
>> ---
>>  Makefile.target              |    2 +
>>  hw/openrisc_cpudev.h         |   30 ++
>>  hw/openrisc_pic.c            |   31 ++
>>  target-openrisc/mem.c        |  220 
>> +-
>>  target-openrisc/mem_helper.c |   25 +
>>  5 files changed, 307 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/openrisc_cpudev.h
>>  create mode 100644 hw/openrisc_pic.c
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 0ffb29c..79c75f6 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -390,6 +390,8 @@ obj-xtensa-y += core-dc232b.o
>>  obj-xtensa-y += core-dc233c.o
>>  obj-xtensa-y += core-fsf.o
>>
>> +obj-openrisc-y += openrisc_pic.o
>> +
>>  main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>>
>>  monitor.o: hmp-commands.h qmp-commands-old.h
>> diff --git a/hw/openrisc_cpudev.h b/hw/openrisc_cpudev.h
>> new file mode 100644
>> index 000..ca7d064
>> --- /dev/null
>> +++ b/hw/openrisc_cpudev.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Qemu Openrisc CPU device support.
>> + *
>> + *  Copyright (c) 2011-2012 Jia Liu 
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Library General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Library General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA  02110-1301 
>> USA
>> + */
>> +
>> +#ifndef HW_OPENRISC_CPUDEV_H
>> +#define HW_OPENRISC_CPUDEV_H
>> +
>> +/* openrisc_pic.c */
>> +void cpu_openrisc_pic_init(CPUOPENRISCState *env);
>> +
>> +/* openrisc_timer.c*/
>> +void cpu_openrisc_clock_init(CPUOPENRISCState *env);
>
> The implementation is not added by this patch.
>

I'll add it here.

>> +
>> +#endif
>> diff --git a/hw/openrisc_pic.c b/hw/openrisc_pic.c
>> new file mode 100644
>> index 000..0abdd50
>> --- /dev/null
>> +++ b/hw/openrisc_pic.c
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Generic  OPENRISC Programmable Interrupt Controller support.
>> + *
>> + *  Copyright (c) 2011-2012 Jia Liu 
>> + *                          Feng Gao 
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Library General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Library General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA  02110-1301 
>> USA
>> + */
>> +
>> +#include "hw.h"
>> +#include "openrisc_cpudev.h"
>> +#include "cpu.h"
>> +
>> +/* Reset PIC */
>> +void cpu_openrisc_pic_reset(CPUOPENRISCState *env)
>> +{
>> +    env->picmr = 0x;
>> +    env->picsr = 0x;
>> +}
>> diff --git a/target-openrisc/mem.c b/target-openrisc/mem.c
>> index 57a0b1b..b26da38 100644
>> --- a/target-openrisc/mem.c
>> +++ b/target-openrisc/mem.c
>> @@ -28,10 +28,224 @@
>>  #endif
>>
>>  #if !defined(CONFIG_USER_ONLY)
>> +enum {
>> +    TLBRET_INVALID = -3,
>> +    TLBRET_NOMATCH = -2,
>> +    TLBRET_BADADDR = -1,
>> +    TLBRET_MATCH = 0
>> +};
>> +
>> +tlb_entry itlb_table[ITLB_WAYS][ITLB_SIZE];
>> +tlb_entry dtlb_table[DTLB_WAYS][DTLB_SIZE];
>
> These should be inside CPUOPENRISCState, not globals.
>

thanks, I'll move them there.

>> +#endif
>> +
>> +#if !defined(CONFIG_USER_ONLY)
>> +/* no MMU emulation */
>> +int get_phys_nommu(CPUOPENRISCState *env, target_phys_addr_t *physical,
>> +                   int *prot, target_ulong address, int rw)
>> +{
>> +    *physical = address;
>> +    *prot = PAGE_READ | PAGE_WRITE;
>> +    return TLBRET_MATCH;
>> +}
>> +int get_phys_code(CPUOPENRISCState *env, target_phys_addr_t *physical,
>> +                  int *prot, target_ulong address, int rw)
>> +{
>> +    int vpn = address >> TARGET_PAGE_BITS;
>> +    int idx = vpn & ITLB_MASK;
>> +    int right = 0;
>> +
>> +    if ((env->itlb[0][idx].mr >> TARGET_PAGE_BITS) != vpn) {
>> +        return

Re: [Qemu-devel] ppc: CPU reset must flush translation buffer

[Alexander Graf]

On 21.05.2012, at 04:01, Benjamin Herrenschmidt  
wrote:

> Without that, reset from SLOF crashes in full emulation.
> 
> Reported-by: Thomas Huth 
> Signed-off-by: Benjamin Herrenschmidt 
> ---
> target-ppc/translate_init.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index ae03065..fbf7705 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -10285,6 +10285,7 @@ static void ppc_cpu_reset(CPUState *s)
> env->error_code = 0;
> /* Flush all TLBs */
> tlb_flush(env, 1);
> +tb_flush(env);

Shouldn't this be true for all CPUs? I remember talking about reset with Peter 
a while ago... but don't remember the conclusions :)


Alex

[Qemu-devel] Weird iscsi/fd-event issue since recent merge of event system changes

[ronnie sahlberg]

List, Kevin,

Since this merge :
commit 1f8bcac09af61e58c5121aa0a932190700ad554d
Merge: cb4c254 1042ec9
Author: Anthony Liguori 
Date:   Mon Apr 23 14:27:04 2012 -0500

Merge remote-tracking branch 'kwolf/for-anthony' into staging

* kwolf/for-anthony: (38 commits)
  qemu-iotests: Fix test 031 for qcow2 v3 support
  qemu-iotests: Add -o and make v3 the default for qcow2
  qcow2: Zero write support
  qemu-iotests: Test backing file COW with zero clusters
  qemu-iotests: add a simple test for write_zeroes
  qcow2: Support for feature table header extension
  qcow2: Support reading zero clusters
  qcow2: Version 3 images
  qcow2: Ignore reserved bits in check_refcounts
  qcow2: Ignore reserved bits in refcount table entries
  qcow2: Simplify count_cow_clusters
  qcow2: Refactor qcow2_free_any_clusters
  qcow2: Ignore reserved bits in L1/L2 entries
  qcow2: Fail write_compressed when overwriting data
  qcow2: Ignore reserved bits in count_contiguous_clusters()
  qcow2: Ignore reserved bits in get_cluster_offset
  qcow2: Save disk size in snapshot header
  Specification for qcow2 version 3
  qcow2: Fix refcount block allocation during qcow2_alloc_cluster_at()
  iotests: Resolve test failures caused by hostname
  ...

I am seeing some weirdness when using iscsi.
I have isolated it to this particular commit, but since it is 3900
lines in sinze I have not yet found the exact change that triggers
this particular behaviour.

It shows up when using an iscsi device to boot from, where when during
the bios boot and later grub boot almost all I/O has a pause of 55ms
between them.

During boot the bios and later grub will read a lot of data, primarily
sequentially and one single block at a time.
After these changes were applied there is now an approximate 55ms
delay between all these I/O, causing the boot process to become very
slow.


I have not yet found the exact part of this big patch that cause this
slowdown, but will continue investigating.

I am posting this here in case someone has any idea  or if 55ms rings any bells.


regards
ronnie sahlberg

Re: [Qemu-devel] Memory Tracking API

[Jaspal]

On 05/18/2012 12:17 AM, Richard W.M. Jones wrote:

On Thu, May 17, 2012 at 11:36:24PM +0530, Jaspal wrote:

Hi ,

Is it possible to keep a count of reads / writes taking place in a
vm using qemu ( using kvm as hypervisor ) ? Is there a api ( or any
patch ) for it ?

Memory reads and writes is surely going to generate a huge
amount of output!

There are various DEBUG_* symbols at the top of exec.c and ioport.c.
I've only used a few of these:

   DEBUG_UNASSIGNED - prints a message when an unmapped page is
   referenced (TCG only, presumably?)

   DEBUG_IOPORT - prints a message when any I/O port is referenced

   DEBUG_UNUSED_IOPORT - prints a message when a non-emulated I/O port
   is referenced

There are several more if you look at the code.

Rich.



When are these functions called : kvm_read_guest_page , 
kvm_read_guest_atomic , kvm_write_guest_page present in kvm_main.c  ? 
When qemu wants to read/write to a page ? If qemu has to read/write on 
the vm's memory ( RAM ) , does the process always involve kvm ?


Thanks ,
Jaspal

Re: [Qemu-devel] [PATCH 01/15] Openrisc: add target stub

[Jia Liu]

Hi Andreas,

Thanks, I'm trying fix them all.

On Thu, May 17, 2012 at 10:14 PM, Andreas Färber  wrote:
> Am 17.05.2012 10:35, schrieb Jia Liu:
>> add the openrisc target stub and basic implementation.
>>
>> Signed-off-by: Jia Liu 
>> ---
>> diff --git a/target-openrisc/cpu-qom.h b/target-openrisc/cpu-qom.h
>> new file mode 100644
>> index 000..8c936df
>> --- /dev/null
>> +++ b/target-openrisc/cpu-qom.h
>
> First of all, if you design your new target cleanly, there's no strict
> need for a cpu-qom.h header - it mainly served to separate new QOM code
> from legacy code. If you want, you can put the code directly into cpu.h
> just as well.
>
>> @@ -0,0 +1,70 @@
>> +/*
>> + *  QEMU Openrisc CPU
>> + *
>> + *  Copyright (c) 2012 Jia Liu 
>
> Minor nitpick: I guess this was copied from some other header? Uses a
> two-space indentation here and one-space below.
>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see 
>> .
>> + */
>> +
>> +#ifndef QEMU_OPENRISC_CPU_QOM_H
>> +#define QEMU_OPENRISC_CPU_QOM_H
>> +
>> +#include "qemu/cpu.h"
>
>> +#include "cpu.h"
>
> Please don't. This was a big mistake of mine that I've been correcting
> on the qom-next branch, onto which you should rebase a new target such
> as this by the way. It leads to really ugly problems when someone
> includes cpu-qom.h and the new struct below is not yet defined for
> functions using it there.
>
>> +
>> +#define TYPE_OPENRISC_CPU "or32"
>> +
>> +#define OPENRISC_CPU_CLASS(klass) \
>> +    OBJECT_CLASS_CHECK(OPENRISCCPUClass, (klass), TYPE_OPENRISC_CPU)
>> +#define OPENRISC_CPU(obj) \
>> +    OBJECT_CHECK(OPENRISCCPU, (obj), TYPE_OPENRISC_CPU)
>> +#define OPENRISC_CPU_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(OPENRISCCPUClass, (obj), TYPE_OPENRISC_CPU)
>> +
>> +/**
>> + * OPENRISCCPUClass:
>> + * @parent_reset: The parent class' reset handler.
>> + *
>> + * A Openrisc CPU model.
>> + */
>> +typedef struct OPENRISCCPUClass {
>> +    /*< private >*/
>> +    CPUClass parent_class;
>> +    /*< public >*/
>> +
>> +    void (*parent_reset)(CPUState *cpu);
>> +} OPENRISCCPUClass;
>
> Pleave avoid unnecessary uppercase spelling: OpenRISCCPUClass? That
> distinguishes it from the all-uppercase cast macros.
>
> Or OpenriscCPUClass as you spell it elsewhere?
>
>> +
>> +/**
>> + * OPENRISCCPU:
>> + * @env: #CPUOPENRISCState
>> + *
>> + * A Openrisc CPU.
>> + */
>> +typedef struct OPENRISCCPU {
>> +    /*< private >*/
>> +    CPUState parent_obj;
>> +    /*< public >*/
>> +
>> +    CPUOPENRISCState env;
>> +} OPENRISCCPU;
>
> OpenRISCCPU? OpenriscCPU?
>
>> +
>> +static inline OPENRISCCPU *openrisc_env_get_cpu(CPUOPENRISCState *env)
>> +{
>> +    return OPENRISC_CPU(container_of(env, OPENRISCCPU, env));
>> +}
>> +
>> +#define ENV_GET_CPU(e) CPU(openrisc_env_get_cpu(e))
>> +
>> +#endif /* QEMU_OPENRISC_CPU_QOM_H */
>> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
>> new file mode 100644
>> index 000..01e65a2
>> --- /dev/null
>> +++ b/target-openrisc/cpu.c
>> @@ -0,0 +1,74 @@
>> +/*
>> + *  QEMU Openrisc CPU
>> + *
>> + *  Copyright (c) 2012 Jia Liu 
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library; if not, see 
>> .
>> + */
>> +
>> +#include "cpu.h"
>> +#include "cpu-qom.h"
>
> cpu.h already does #include "cpu-qom.h", so no need to include it here
> again.
>
>> +#include "qemu-common.h"
>> +
>> +
>> +/* CPUClass::reset() */
>> +static void openrisc_cpu_reset(CPUState *s)
>> +{
>> +    OPENRISCCPU *cpu = OPENRISC_CPU(s);
>> +    OPENRISCCPUClass *occ = OPENRISC_CPU_GET_CLASS(cpu);
>> +    CPUOPENRISCState *env = &cpu->env;
>> +
>> +    occ->parent_reset(s);
>> +
>> +    openrisc_reset(env);
>
> Shouldn't this be inline here? And don't forge

[Qemu-devel] ppc: CPU reset must flush translation buffer

[Benjamin Herrenschmidt]

Without that, reset from SLOF crashes in full emulation.

Reported-by: Thomas Huth 
Signed-off-by: Benjamin Herrenschmidt 
---
 target-ppc/translate_init.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index ae03065..fbf7705 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10285,6 +10285,7 @@ static void ppc_cpu_reset(CPUState *s)
 env->error_code = 0;
 /* Flush all TLBs */
 tlb_flush(env, 1);
+tb_flush(env);
 }
 
 static void ppc_cpu_initfn(Object *obj)

[Qemu-devel] [PATCH] Add a memory barrier to guest memory access functions

[Benjamin Herrenschmidt]

The emulated devices can run simultaneously with the guest, so
we need to be careful with ordering of load and stores done by
them to the guest system memory, which need to be observed in
the right order by the guest operating system.

This adds barriers to some standard guest memory access functions
along with a comment explaining the semantics to exec.c

Signed-off-by: Benjamin Herrenschmidt 
---

So after the discussion with Paolo, I removed the specific accessors,
just used a normal smp_mb() in only two places, cpu_physical_memory_rw
and cpu_physical_memory_map.

I don't see an obvious need to provide a "relaxed" variant of the
later at this stage, a quick grep doesn't seem to show that most cases
where it's used are either not performance sensitive or the barrier
makes sense, but feel free to prove me wrong :-)

If we really want that, my suggestion would be to change the "is_write"
flag into a proper bitmask of direction and relaxed attribute (which we
can use for more attributes in the future if needed). 

Also, we probably want an smp_mb() when shooting MSIs (not LSIs, those
are not ordered, that's why the guest driver needs to do an MMIO read
after an LSI, but MSIs are). I haven't looked at that yet, we can do
it from a separate patch if needed.

 exec.c |   37 +
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/exec.c b/exec.c
index 363ec98..997dbb0 100644
--- a/exec.c
+++ b/exec.c
@@ -16,6 +16,34 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see .
  */
+
+/*
+ * Note on memory barriers usage:
+ *
+ * In order for emulated devices "DMA" operations to appear
+ * with a consistent ordering to the guest, we provide some
+ * amount of ordering guarantees:
+ *
+ * cpy_physical_memory_rw() (and all functions using it) is
+ * ordered vs. all previous accesses (it begins with a full
+ * memory barrier)
+ *
+ * This include all the new dma_* accessors.
+ *
+ * The old variants of ld* and st* that have not been convered
+ * to dma_ are not ordered. Users are reponsible for using their
+ * own ordering.
+ *
+ * cpu_physical_memory_map() provides a memory barrier vs. all
+ * previous accesses. There is no implicit barrier on unmap.
+ * If ordering is required between accessed within the map/unmmap
+ * sequence, then it needs to be done explicitely.
+ *
+ * This means that a typical block driver using map/unmap accross
+ * the transfer of a block followed by dma_ writes to signal
+ * completion or interrupt shouldn't require the addition of
+ * explicit barriers.
+ */ 
 #include "config.h"
 #ifdef _WIN32
 #include 
@@ -25,6 +53,7 @@
 #endif
 
 #include "qemu-common.h"
+#include "qemu-barrier.h"
 #include "cpu.h"
 #include "tcg.h"
 #include "hw/hw.h"
@@ -3516,6 +3545,10 @@ void cpu_physical_memory_rw(target_phys_addr_t addr, 
uint8_t *buf,
 target_phys_addr_t page;
 MemoryRegionSection *section;
 
+/* Provides ordering vs. previous accesses, see comments
+ * at the top of this file */
+smp_mb();
+
 while (len > 0) {
 page = addr & TARGET_PAGE_MASK;
 l = (page + TARGET_PAGE_SIZE) - addr;
@@ -3713,6 +3746,10 @@ void *cpu_physical_memory_map(target_phys_addr_t addr,
 ram_addr_t rlen;
 void *ret;
 
+/* Provides ordering vs. previous accesses, see comments
+ * at the top of this file */
+smp_mb();
+
 while (len > 0) {
 page = addr & TARGET_PAGE_MASK;
 l = (page + TARGET_PAGE_SIZE) - addr;

[Qemu-devel] [PATCH 06/13 - UPDATED] ide/ahci: Use universal DMA helper functions

[Benjamin Herrenschmidt]

The AHCI device can provide both PCI and SysBus AHCI device
emulations.  For this reason, it wasn't previously converted to use
the pci_dma_*() helper functions.  Now that we have universal DMA
helper functions, this converts AHCI to use them.

The DMAContext is obtained from pci_dma_context() in the PCI case and
set to NULL in the SysBus case (i.e. we assume for now that a SysBus
AHCI has no IOMMU translation).

Cc: Kevin Wolf 
Cc: Michael S. Tsirkin 

Signed-off-by: David Gibson 
Signed-off-by: Benjamin Herrenschmidt 
---

The previous version missed a couple of conversions, possibly
stuff that was added since the patch was originally written. 

 hw/ide/ahci.c |   51 +--
 hw/ide/ahci.h |3 ++-
 hw/ide/ich.c  |2 +-
 3 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 96d8f62..895a756 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -172,17 +172,18 @@ static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
 ahci_check_irq(s);
 }
 
-static void map_page(uint8_t **ptr, uint64_t addr, uint32_t wanted)
+static void map_page(AHCIState *s, uint8_t **ptr, uint64_t addr,
+ uint32_t wanted)
 {
 target_phys_addr_t len = wanted;
 
 if (*ptr) {
-cpu_physical_memory_unmap(*ptr, len, 1, len);
+dma_memory_unmap(s->dma, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
 }
 
-*ptr = cpu_physical_memory_map(addr, &len, 1);
+*ptr = dma_memory_map(s->dma, addr, &len, DMA_DIRECTION_FROM_DEVICE);
 if (len < wanted) {
-cpu_physical_memory_unmap(*ptr, len, 1, len);
+dma_memory_unmap(s->dma, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
 *ptr = NULL;
 }
 }
@@ -195,24 +196,24 @@ static void  ahci_port_write(AHCIState *s, int port, int 
offset, uint32_t val)
 switch (offset) {
 case PORT_LST_ADDR:
 pr->lst_addr = val;
-map_page(&s->dev[port].lst,
+map_page(s, &s->dev[port].lst,
  ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 1024);
 s->dev[port].cur_cmd = NULL;
 break;
 case PORT_LST_ADDR_HI:
 pr->lst_addr_hi = val;
-map_page(&s->dev[port].lst,
+map_page(s, &s->dev[port].lst,
  ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 1024);
 s->dev[port].cur_cmd = NULL;
 break;
 case PORT_FIS_ADDR:
 pr->fis_addr = val;
-map_page(&s->dev[port].res_fis,
+map_page(s, &s->dev[port].res_fis,
  ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
 break;
 case PORT_FIS_ADDR_HI:
 pr->fis_addr_hi = val;
-map_page(&s->dev[port].res_fis,
+map_page(s, &s->dev[port].res_fis,
  ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
 break;
 case PORT_IRQ_STAT:
@@ -588,7 +589,7 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
 AHCIPortRegs *pr = &ad->port_regs;
 uint8_t *d2h_fis;
 int i;
-target_phys_addr_t cmd_len = 0x80;
+dma_addr_t cmd_len = 0x80;
 int cmd_mapped = 0;
 
 if (!ad->res_fis || !(pr->cmd & PORT_CMD_FIS_RX)) {
@@ -598,7 +599,8 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
 if (!cmd_fis) {
 /* map cmd_fis */
 uint64_t tbl_addr = le64_to_cpu(ad->cur_cmd->tbl_addr);
-cmd_fis = cpu_physical_memory_map(tbl_addr, &cmd_len, 0);
+cmd_fis = dma_memory_map(ad->hba->dma, tbl_addr, &cmd_len,
+ DMA_DIRECTION_TO_DEVICE);
 cmd_mapped = 1;
 }
 
@@ -630,7 +632,8 @@ static void ahci_write_fis_d2h(AHCIDevice *ad, uint8_t 
*cmd_fis)
 ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
 
 if (cmd_mapped) {
-cpu_physical_memory_unmap(cmd_fis, cmd_len, 0, cmd_len);
+dma_memory_unmap(ad->hba->dma, cmd_fis, cmd_len,
+ DMA_DIRECTION_TO_DEVICE, cmd_len);
 }
 }
 
@@ -640,8 +643,8 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList 
*sglist)
 uint32_t opts = le32_to_cpu(cmd->opts);
 uint64_t prdt_addr = le64_to_cpu(cmd->tbl_addr) + 0x80;
 int sglist_alloc_hint = opts >> AHCI_CMD_HDR_PRDT_LEN;
-target_phys_addr_t prdt_len = (sglist_alloc_hint * sizeof(AHCI_SG));
-target_phys_addr_t real_prdt_len = prdt_len;
+dma_addr_t prdt_len = (sglist_alloc_hint * sizeof(AHCI_SG));
+dma_addr_t real_prdt_len = prdt_len;
 uint8_t *prdt;
 int i;
 int r = 0;
@@ -652,7 +655,8 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList 
*sglist)
 }
 
 /* map PRDT */
-if (!(prdt = cpu_physical_memory_map(prdt_addr, &prdt_len, 0))){
+if (!(prdt = dma_memory_map(ad->hba->dma, prdt_addr, &prdt_len,
+DMA_DIRECTION_TO_DEVICE))){
 DPRINTF(ad->port_no, "map fai

Re: [Qemu-devel] [PATCH qom-next v2 1/3] xilinx_zynq: Use cpu_arm_init() to obtain ARMCPU

[Peter Crosthwaite]

Cant see it affecting the change pattern to the zynq boot though, so
theres no blocker.

Acked-by: Peter A.G. Crosthwaite 

On Sat, May 19, 2012 at 2:01 AM, Andreas Färber  wrote:
> Needed for arm_load_kernel().
>
> Signed-off-by: Andreas Färber 
> ---
>  hw/xilinx_zynq.c |    8 
>  1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c
> index 7290c64..68349e2 100644
> --- a/hw/xilinx_zynq.c
> +++ b/hw/xilinx_zynq.c
> @@ -50,7 +50,7 @@ static void zynq_init(ram_addr_t ram_size, const char 
> *boot_device,
>                         const char *kernel_filename, const char 
> *kernel_cmdline,
>                         const char *initrd_filename, const char *cpu_model)
>  {
> -    CPUARMState *env = NULL;
> +    ARMCPU *cpu;
>     MemoryRegion *address_space_mem = get_system_memory();
>     MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
>     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
> @@ -66,12 +66,12 @@ static void zynq_init(ram_addr_t ram_size, const char 
> *boot_device,
>         cpu_model = "cortex-a9";
>     }
>
> -    env = cpu_init(cpu_model);
> -    if (!env) {
> +    cpu = cpu_arm_init(cpu_model);
> +    if (!cpu) {
>         fprintf(stderr, "Unable to find CPU definition\n");
>         exit(1);
>     }
> -    irqp = arm_pic_init_cpu(env);
> +    irqp = arm_pic_init_cpu(&cpu->env);
>     cpu_irq = irqp[ARM_PIC_CPU_IRQ];
>
>     /* max 2GB ram */
> --
> 1.7.7
>

Re: [Qemu-devel] [PATCH 13/13] iommu: Add a memory barrier to DMA RW function

[Benjamin Herrenschmidt]

On Sat, 2012-05-19 at 09:24 +0200, Paolo Bonzini wrote:

> I guess the C11/C++ guys required an isync barrier after either loads or
> stores, because they need to order the load/store vs. code accessing
> other memory.  This is not needed in QEMU because all guest accesses go
> through cpu_physical_memory_rw (or has its own barriers).

I am not sure, I don't quite see what it buys them really. I'd have to
ask Paul McKenney, he probably knows :-)

Cheers,
Ben.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

On Sun, May 20, 2012 at 12:39:13PM -0400, Kevin O'Connor wrote:
> On Sun, May 20, 2012 at 07:25:40PM +0300, Avi Kivity wrote:
> > On 05/20/2012 07:16 PM, Kevin O'Connor wrote:
> > > > Here we in agreement, and I was against patching till it was 
> > > > unavoidable,
> > > > but than pci hotplug started using it, and afterwards processor
> > > > definitions, so no point in avoiding it now by using inferior methods.
> > >
> > > I agree as well.
> > >
> > > What's the background to needing to have dynamic S3/S4 definitions?
> > > (Why will some qemu instances be able to sleep and not others?)
> > >
> > 
> > Backwards compatibility.  qemu has a -M machine-type option that expose
> > an old qemu's guest-visible attributes.  If an old qemu didn't support
> > S3, then -M old shouldn't either.
> 
> The DSDT has claimed S3, S4, and S5 support since SeaBIOS has
> supported ACPI.  What's the background to the requirement to stop
> claiming support for it?
> 
Not all guests have working S3/S4 implementation. We want management to
be able to disable S3/S4 for such guests. S4 value changes since we want
to distinguish between S4 and S5 in qemu.

--
Gleb.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Kevin O'Connor]

On Sun, May 20, 2012 at 07:25:40PM +0300, Avi Kivity wrote:
> On 05/20/2012 07:16 PM, Kevin O'Connor wrote:
> > > Here we in agreement, and I was against patching till it was unavoidable,
> > > but than pci hotplug started using it, and afterwards processor
> > > definitions, so no point in avoiding it now by using inferior methods.
> >
> > I agree as well.
> >
> > What's the background to needing to have dynamic S3/S4 definitions?
> > (Why will some qemu instances be able to sleep and not others?)
> >
> 
> Backwards compatibility.  qemu has a -M machine-type option that expose
> an old qemu's guest-visible attributes.  If an old qemu didn't support
> S3, then -M old shouldn't either.

The DSDT has claimed S3, S4, and S5 support since SeaBIOS has
supported ACPI.  What's the background to the requirement to stop
claiming support for it?

-Kevin

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity]

On 05/20/2012 07:16 PM, Kevin O'Connor wrote:
> > Here we in agreement, and I was against patching till it was unavoidable,
> > but than pci hotplug started using it, and afterwards processor
> > definitions, so no point in avoiding it now by using inferior methods.
>
> I agree as well.
>
> What's the background to needing to have dynamic S3/S4 definitions?
> (Why will some qemu instances be able to sleep and not others?)
>

Backwards compatibility.  qemu has a -M machine-type option that expose
an old qemu's guest-visible attributes.  If an old qemu didn't support
S3, then -M old shouldn't either.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Kevin O'Connor]

On Sun, May 20, 2012 at 06:15:44PM +0300, Gleb Natapov wrote:
> On Sun, May 20, 2012 at 05:46:46PM +0300, Avi Kivity wrote:
> > On 05/20/2012 05:43 PM, Gleb Natapov wrote:
> > > > 
> > > > Or it can be a fixed address in low memory, or a scratch register in
> > > > hardware.
> > > > 
> > > Both will work (fixed addresses are better be avoided and who needs
> > > another PV device), but I do not see how either of them is better then
> > > patching. What is your concern?
> > >
> > 
> > Patching is harder to maintain.  Unfortunately it's unavoidable.
> > 
> Here we in agreement, and I was against patching till it was unavoidable,
> but than pci hotplug started using it, and afterwards processor
> definitions, so no point in avoiding it now by using inferior methods.

I agree as well.

What's the background to needing to have dynamic S3/S4 definitions?
(Why will some qemu instances be able to sleep and not others?)

-Kevin

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Kevin O'Connor]

On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity wrote:
> What about
> 
>   If (Fcfg(...)) {
> Method()...
>   }
> 
> ?
> 
> (i.e.. define the method conditionally at runtime)

As Gleb points out, this wont work.  AML defines a static
device/method/variable tree heirarchy.  Only the return values of
methods is programmable.  This completely confused me when I first
started looking at AML.

ACPI is truly a bizarre spec.

-Kevin

Re: [Qemu-devel] [RFC][PATCH v2 00/11] uq/master: irqfd-based interrupt injection for virtio/vhost

[Michael S. Tsirkin]

On Thu, May 17, 2012 at 10:32:28AM -0300, Jan Kiszka wrote:
> After this series, to only reasons to still use qemu-kvm for production
> purposes will be PCI device assignment

Yay!

By the way, there are probably not many reasons to keep the
assignment code out of qemu.git. It duplicates a ton of
code from core pci, but that's easier to fix in-tree
than out of tree.

-- 
MST

Re: [Qemu-devel] [PATCH] PPC: mpc8544ds: Span initial TLB entry over as much RAM as we need

[Alexander Graf]

On 20.05.2012, at 15:58, Andreas Färber  wrote:

> Am 19.05.2012 00:53, schrieb Alexander Graf:
>> The initial TLB entry is supposed to help us run the guest -kernel payload.
>> This means the guest needs to be able to access its own memory, the initrd
>> memory and the device tree.
>> 
>> So far we only statically reserved a TLB entry from [0;256M[. This patch
>> fixes it to span from [0;dt_end[, allowing the guest payload to access
>> everything initially.
>> 
>> Reported-by: Stuart Yoder 
>> Signed-off-by: Alexander Graf 
>> ---
>> hw/ppce500_mpc8544ds.c |   41 +++--
>> 1 files changed, 27 insertions(+), 14 deletions(-)
>> 
>> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
>> index f1dfbe1..42a63aa 100644
>> --- a/hw/ppce500_mpc8544ds.c
>> +++ b/hw/ppce500_mpc8544ds.c
>> @@ -31,6 +31,7 @@
>> #include "elf.h"
>> #include "sysbus.h"
>> #include "exec-memory.h"
>> +#include "host-utils.h"
>> 
>> #define BINARY_DEVICE_TREE_FILE"mpc8544ds.dtb"
>> #define UIMAGE_LOAD_BASE   0
>> @@ -55,6 +56,7 @@
>> struct boot_info
>> {
>>uint32_t dt_base;
>> +uint32_t dt_size;
>>uint32_t entry;
>> };
>> 
>> @@ -164,7 +166,11 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>>}
>> 
>>ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
>> +if (ret < 0) {
>> +goto out;
>> +}
>>g_free(fdt);
>> +ret = fdt_size;
>> 
>> out:
>> #endif
>> @@ -172,23 +178,27 @@ out:
>>return ret;
>> }
>> 
>> -/* Create -kernel TLB entries for BookE, linearly spanning 256MB.  */
>> +/* Create -kernel TLB entries for BookE.  */
>> static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size)
>> {
>> -return ffs(size >> 10) - 1;
>> +return 63 - clz64(size >> 10);
>> }
>> 
>> -static void mmubooke_create_initial_mapping(CPUPPCState *env,
>> - target_ulong va,
>> - target_phys_addr_t pa)
>> +static void mmubooke_create_initial_mapping(CPUPPCState *env)
>> {
>> +struct boot_info *bi = env->load_info;
>>ppcmas_tlb_t *tlb = booke206_get_tlbm(env, 1, 0, 0);
>> -target_phys_addr_t size;
>> -
>> -size = (booke206_page_size_to_tlb(256 * 1024 * 1024) << 
>> MAS1_TSIZE_SHIFT);
>> +target_phys_addr_t size, dt_end;
>> +int ps;
>> +
>> +/* Our initial TLB entry needs to cover everything from 0 to
>> +   the device tree top */
>> +dt_end = bi->dt_base + bi->dt_size;
>> +ps = booke206_page_size_to_tlb(dt_end) + 1;
>> +size = (ps << MAS1_TSIZE_SHIFT);
>>tlb->mas1 = MAS1_VALID | size;
>> -tlb->mas2 = va & TARGET_PAGE_MASK;
>> -tlb->mas7_3 = pa & TARGET_PAGE_MASK;
>> +tlb->mas2 = 0;
>> +tlb->mas7_3 = 0;
>>tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX;
>> 
>>env->tlb_dirty = true;
>> @@ -218,7 +228,7 @@ static void mpc8544ds_cpu_reset(void *opaque)
>>env->gpr[1] = (16<<20) - 8;
>>env->gpr[3] = bi->dt_base;
>>env->nip = bi->entry;
>> -mmubooke_create_initial_mapping(env, 0, 0);
>> +mmubooke_create_initial_mapping(env);
>> }
>> 
>> static void mpc8544ds_init(ram_addr_t ram_size,
>> @@ -374,13 +384,15 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>>/* If we're loading a kernel directly, we must load the device tree too. 
>> */
>>if (kernel_filename) {
>>struct boot_info *boot_info;
>> +int dt_size;
>> 
>> #ifndef CONFIG_FDT
>>cpu_abort(env, "Compiled without FDT support - can't load kernel\n");
>> #endif
>> -dt_base = (kernel_size + DTC_LOAD_PAD) & ~DTC_PAD_MASK;
>> -if (mpc8544_load_device_tree(env, dt_base, ram_size,
>> -initrd_base, initrd_size, kernel_cmdline) < 0) {
>> +dt_base = (loadaddr + kernel_size + DTC_LOAD_PAD) & ~DTC_PAD_MASK;
>> +dt_size = mpc8544_load_device_tree(env, dt_base, ram_size, 
>> initrd_base,
>> +   initrd_size, kernel_cmdline);
>> +if (dt_size < 0) {
>>fprintf(stderr, "couldn't load device tree\n");
>>exit(1);
>>}
>> @@ -388,6 +400,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>>boot_info = env->load_info;
> 
> This is turned into cpu->load_info in my series. Since this patch is not
> marked 1.1, please ack and apply my patches first, so that we don't run
> into conflicts with qom-next. Thanks.

Yeah, of course. Rebasing this on top of your series should be trivial - and 
it's not crucial for 1.1 IMHO.

Alex

> 
> Andreas
> 
>>boot_info->entry = entry;
>>boot_info->dt_base = dt_base;
>> +boot_info->dt_size = dt_size;
>>}
>> 
>>if (kvm_enabled()) {
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

On Sun, May 20, 2012 at 05:46:46PM +0300, Avi Kivity wrote:
> On 05/20/2012 05:43 PM, Gleb Natapov wrote:
> > > 
> > > Or it can be a fixed address in low memory, or a scratch register in
> > > hardware.
> > > 
> > Both will work (fixed addresses are better be avoided and who needs
> > another PV device), but I do not see how either of them is better then
> > patching. What is your concern?
> >
> 
> Patching is harder to maintain.  Unfortunately it's unavoidable.
> 
Here we in agreement, and I was against patching till it was unavoidable,
but than pci hotplug started using it, and afterwards processor
definitions, so no point in avoiding it now by using inferior methods.
 
--
Gleb.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity]

On 05/20/2012 05:43 PM, Gleb Natapov wrote:
> > 
> > Or it can be a fixed address in low memory, or a scratch register in
> > hardware.
> > 
> Both will work (fixed addresses are better be avoided and who needs
> another PV device), but I do not see how either of them is better then
> patching. What is your concern?
>

Patching is harder to maintain.  Unfortunately it's unavoidable.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [RFC][PATCH v2 00/11] uq/master: irqfd-based interrupt injection for virtio/vhost

[Avi Kivity]

On 05/20/2012 05:42 PM, Michael S. Tsirkin wrote:
> On Thu, May 17, 2012 at 10:32:28AM -0300, Jan Kiszka wrote:
> > After this series, to only reasons to still use qemu-kvm for production
> > purposes will be PCI device assignment
>
> Yay!
>
> By the way, there are probably not many reasons to keep the
> assignment code out of qemu.git. It duplicates a ton of
> code from core pci, but that's easier to fix in-tree
> than out of tree.

Right.  And Jan, if you want to push device assignment to qemu.git,
please update it in qemu-kvm.git instead of rewriting it in qemu.git.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

On Sun, May 20, 2012 at 05:34:56PM +0300, Avi Kivity wrote:
> On 05/20/2012 04:57 PM, Gleb Natapov wrote:
> > On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity wrote:
> > > On 05/20/2012 03:59 PM, Gleb Natapov wrote:
> > > > > > > > > 
> > > > > > > > > Do we actually have to patch the DSDT?  Or can _S3 etc be 
> > > > > > > > > made into
> > > > > > > > > functions instead? (and talk to the bios, or even to fwcfg 
> > > > > > > > > directly?)
> > > > > > > > > 
> > > > > > > > We better not talk to fwcfg after OSPM is started since this is 
> > > > > > > > firmware
> > > > > > > > confing interface.
> > > > > > > 
> > > > > > > Why not?  The OS isn't going to talk to it, so we can have a 
> > > > > > > driver in ACPI.
> > > > > > > 
> > > > > > The OS is going to talk to it since the OS is the one who interprets
> > > > > > AML. 
> > > > > 
> > > > > I meant, not directly.  So the driver in ACPI has exclusive access.
> > > > > 
> > > > What's the difference?
> > > 
> > > ACPI is firmware, not OS.
> > AML is a data provided by firmware. AML's runtime is different from 
> > firmware's.
> 
> It's still firmware.
> 
We have to agree to disagree here :) It's just a data for OS to use as
far as I am concern.

> > > > > 
> > > > > It's an alternative to patching AML.  Sure it takes some effort to 
> > > > > write
> > > > > the driver, but afterwards we can modify the guest behaviour more
> > > > > easily.  One possible client is -M old, so you can revert to previous
> > > > > behaviour depending on fwcfg data.
> > > > -M old is easy to support with the current patch. You just set new
> > > > properties to compatibility values. The code is written with this in
> > > > mind. And this is not an alternative to patching AML as I am trying to
> > > > explain to you below. You can eliminate patching of s4 value, but that's
> > > > it, you still need to patch out _S3/_S4 names.
> > > 
> > > What about
> > > 
> > >   If (Fcfg(...)) {
> > > Method()...
> > >   }
> > > 
> > > ?
> > syntax error, unexpected PARSEOP_IF
> 
> Unfortunately the ACPI spec forbids this construct, so either patching
> or double complication is necessary.
> 
It's not double if we will take all possible combinations into account.

> > > 
> > > (i.e.. define the method conditionally at runtime)
> > > 
> > > >  
> > > > > 
> > > > > (we don't need a driver in AML to avoid patching, we can have AML talk
> > > > > to the bios and the bios drive fwcfg; but I think we'll find uses for 
> > > > > a
> > > > > driver).
> > > > I am not sure what you mean. AML can't talk to the bios. It can read
> > > > values that bios put somewhere. 
> > > 
> > > That's what I meant - communicate through memory.
> > > 
> > What's the benefit? The patching is still needed. You need to pass
> > address of OperationRegion() to AML. You can do it either by patching or
> > by creating OperationRegion() code dynamically.
> 
> Or it can be a fixed address in low memory, or a scratch register in
> hardware.
> 
Both will work (fixed addresses are better be avoided and who needs
another PV device), but I do not see how either of them is better then
patching. What is your concern?

> >
> > > > I do not see advantage of this method
> > > > and it requires patching still.
> > > 
> > > For the existence of the names?  Yes, if we can't avoid it it's a
> > > problem.  But if we can avoid patching, we should.
> > > 
> > If we can, we should, but we can't as far as I see. The patching was here 
> > long before
> > this patch.
> 
> I agree we probably can't.
> 
> -- 
> error compiling committee.c: too many arguments to function

--
Gleb.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity]

On 05/20/2012 04:57 PM, Gleb Natapov wrote:
> On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity wrote:
> > On 05/20/2012 03:59 PM, Gleb Natapov wrote:
> > > > > > > > 
> > > > > > > > Do we actually have to patch the DSDT?  Or can _S3 etc be made 
> > > > > > > > into
> > > > > > > > functions instead? (and talk to the bios, or even to fwcfg 
> > > > > > > > directly?)
> > > > > > > > 
> > > > > > > We better not talk to fwcfg after OSPM is started since this is 
> > > > > > > firmware
> > > > > > > confing interface.
> > > > > > 
> > > > > > Why not?  The OS isn't going to talk to it, so we can have a driver 
> > > > > > in ACPI.
> > > > > > 
> > > > > The OS is going to talk to it since the OS is the one who interprets
> > > > > AML. 
> > > > 
> > > > I meant, not directly.  So the driver in ACPI has exclusive access.
> > > > 
> > > What's the difference?
> > 
> > ACPI is firmware, not OS.
> AML is a data provided by firmware. AML's runtime is different from 
> firmware's.

It's still firmware.

> > > > 
> > > > It's an alternative to patching AML.  Sure it takes some effort to write
> > > > the driver, but afterwards we can modify the guest behaviour more
> > > > easily.  One possible client is -M old, so you can revert to previous
> > > > behaviour depending on fwcfg data.
> > > -M old is easy to support with the current patch. You just set new
> > > properties to compatibility values. The code is written with this in
> > > mind. And this is not an alternative to patching AML as I am trying to
> > > explain to you below. You can eliminate patching of s4 value, but that's
> > > it, you still need to patch out _S3/_S4 names.
> > 
> > What about
> > 
> >   If (Fcfg(...)) {
> > Method()...
> >   }
> > 
> > ?
> syntax error, unexpected PARSEOP_IF

Unfortunately the ACPI spec forbids this construct, so either patching
or double complication is necessary.

> > 
> > (i.e.. define the method conditionally at runtime)
> > 
> > >  
> > > > 
> > > > (we don't need a driver in AML to avoid patching, we can have AML talk
> > > > to the bios and the bios drive fwcfg; but I think we'll find uses for a
> > > > driver).
> > > I am not sure what you mean. AML can't talk to the bios. It can read
> > > values that bios put somewhere. 
> > 
> > That's what I meant - communicate through memory.
> > 
> What's the benefit? The patching is still needed. You need to pass
> address of OperationRegion() to AML. You can do it either by patching or
> by creating OperationRegion() code dynamically.

Or it can be a fixed address in low memory, or a scratch register in
hardware.

>
> > > I do not see advantage of this method
> > > and it requires patching still.
> > 
> > For the existence of the names?  Yes, if we can't avoid it it's a
> > problem.  But if we can avoid patching, we should.
> > 
> If we can, we should, but we can't as far as I see. The patching was here 
> long before
> this patch.

I agree we probably can't.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [RFC][PATCH v2 00/11] uq/master: irqfd-based interrupt injection for virtio/vhost

[Avi Kivity]

On 05/17/2012 04:32 PM, Jan Kiszka wrote:
> [ changes in v2: rebase over uq/master ]
>
> This series is another major milestone of merging qemu-kvm into
> upstream. It implements the required interfaces and logic to directly
> inject MSI-X interrupts generated by the vhost-net kernel module into
> the KVM in-kernel irqchip. This involves
>  - establishing MSI vector notifiers, so far triggered on relevant MSI-X
>configuration changes of subscribed PCI devices
>  - support for static vIRQ-to-MSI routes
>  - an API for linking an IRQFD with such a vIRQ
>  - the usage of these services in virtio-pci to enable direct injection
>
> The series also contains some smaller refactorings of the KVM IRQ
> routing API such as automatic committing of route changes. It applies on
> top of the KVM MSI support series [1] posted recently. The complete
> stack is available at
>
> git://git.kiszka.org/qemu-kvm.git queues/kvm-msi-irqfd
>
> If the proposes API is acceptable, I will also provide some morphing
> patches for qemu-kvm to make the merge of both trees smoother.
>
> After this series, to only reasons to still use qemu-kvm for production
> purposes will be PCI device assignment and potential dependencies on
> legacy command line switches as well as vmstate formats (when requiring
> backward migration support). However, the majority of users should be
> able to switch to upstream QEMU seamlessly and finally receive the same
> level of performance on x86.
>
>

Thanks, applied.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 01/15] Openrisc: add target stub

[Andreas Färber]

Am 18.05.2012 04:56, schrieb 陳韋任:
>>> This is a mix of two ways of doing the same thing. You should only use
>>> VMState for new code.
>>>
>>
>> did you mean I should not use QEMUFile?
> 
>   I guess what Andreas means is there are two ways to do cpu_save/cpu_load,
> the one you wrote is the old style. For the brand new target, you should use
> VMState completely.

Exactly what I meant. Although I'm working towards making that even
easier by leveraging code from Juan's vmstate series and having the CPUs
just store a pointer to their VMStateDescription like devices do.

Andreas

> You can take target-lm32/machine.c as an example.
> 
> ---
> void cpu_save(QEMUFile *f, void *opaque)
> {
> vmstate_save_state(f, &vmstate_cpu, opaque);
> }
> 
> int cpu_load(QEMUFile *f, void *opaque, int version_id)
> {
> return vmstate_load_state(f, &vmstate_cpu, opaque, version_id);
> }
> ---
> 
>   Leave to vmstate_save_state/vmstate_load_state (savevm.c) do the real 
> things.
> 
> Regards,
> chenwj
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 01/15] Openrisc: add target stub

[Andreas Färber]

Am 19.05.2012 10:51, schrieb Blue Swirl:
> On Thu, May 17, 2012 at 8:35 AM, Jia Liu  wrote:
>> +
>> +typedef struct CPUOPENRISCState CPUOPENRISCState;
>> +struct CPUOPENRISCState {
>> +target_ulong gpr[32];   /* General registers */
>> +uint32_t sr;/* Supervisor register */
>> +target_ulong machi; /* Multiply register  MACHI */
>> +target_ulong maclo; /* Multiply register  MACLO */
>> +target_ulong fpmaddhi;  /* Multiply and add float register FPMADDHI */
>> +target_ulong fpmaddlo;  /* Multiply and add float register FPMADDLO */
>> +target_ulong epcr;  /* Exception PC register */
>> +target_ulong eear;  /* Exception EA register */
>> +uint32_t esr;   /* Exception supervisor register */
>> +void *irq[32];  /* Interrupt irq input */
> 
> CPU reset usually zeros all fields up to breakpoints field in
> CPU_COMMON. Then these and the MMU function pointers below would be
> broken, please move below CPU_COMMON.

...or into OpenRISCCPU, if it is not used with offsetof().

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] Is it possible to retrieve pre-process information in QEMU?

[Mulyadi Santosa]

Hi...

On Thu, May 17, 2012 at 10:09 AM, 陳韋任  wrote:
> Hi all,
>
>  I would like to know if I can retrieve pre-process information in QEMU
> system mode. For example, I want to know each process's page fault ratio.
> Is there a way to do that?

logically, it's possible, but you need to locate the task_struct of
each processes first. Using GDB, that might be a bit easier but still
not easy.

Why not just monitor it inside the guest? using system tap for example?

-- 
regards,

Mulyadi Santosa
Freelance Linux trainer and consultant

blog: the-hydra.blogspot.com
training: mulyaditraining.blogspot.com

Re: [Qemu-devel] [PATCH] PPC: mpc8544ds: Span initial TLB entry over as much RAM as we need

[Andreas Färber]

Am 19.05.2012 00:53, schrieb Alexander Graf:
> The initial TLB entry is supposed to help us run the guest -kernel payload.
> This means the guest needs to be able to access its own memory, the initrd
> memory and the device tree.
> 
> So far we only statically reserved a TLB entry from [0;256M[. This patch
> fixes it to span from [0;dt_end[, allowing the guest payload to access
> everything initially.
> 
> Reported-by: Stuart Yoder 
> Signed-off-by: Alexander Graf 
> ---
>  hw/ppce500_mpc8544ds.c |   41 +++--
>  1 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
> index f1dfbe1..42a63aa 100644
> --- a/hw/ppce500_mpc8544ds.c
> +++ b/hw/ppce500_mpc8544ds.c
> @@ -31,6 +31,7 @@
>  #include "elf.h"
>  #include "sysbus.h"
>  #include "exec-memory.h"
> +#include "host-utils.h"
>  
>  #define BINARY_DEVICE_TREE_FILE"mpc8544ds.dtb"
>  #define UIMAGE_LOAD_BASE   0
> @@ -55,6 +56,7 @@
>  struct boot_info
>  {
>  uint32_t dt_base;
> +uint32_t dt_size;
>  uint32_t entry;
>  };
>  
> @@ -164,7 +166,11 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
>  }
>  
>  ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
> +if (ret < 0) {
> +goto out;
> +}
>  g_free(fdt);
> +ret = fdt_size;
>  
>  out:
>  #endif
> @@ -172,23 +178,27 @@ out:
>  return ret;
>  }
>  
> -/* Create -kernel TLB entries for BookE, linearly spanning 256MB.  */
> +/* Create -kernel TLB entries for BookE.  */
>  static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size)
>  {
> -return ffs(size >> 10) - 1;
> +return 63 - clz64(size >> 10);
>  }
>  
> -static void mmubooke_create_initial_mapping(CPUPPCState *env,
> - target_ulong va,
> - target_phys_addr_t pa)
> +static void mmubooke_create_initial_mapping(CPUPPCState *env)
>  {
> +struct boot_info *bi = env->load_info;
>  ppcmas_tlb_t *tlb = booke206_get_tlbm(env, 1, 0, 0);
> -target_phys_addr_t size;
> -
> -size = (booke206_page_size_to_tlb(256 * 1024 * 1024) << 
> MAS1_TSIZE_SHIFT);
> +target_phys_addr_t size, dt_end;
> +int ps;
> +
> +/* Our initial TLB entry needs to cover everything from 0 to
> +   the device tree top */
> +dt_end = bi->dt_base + bi->dt_size;
> +ps = booke206_page_size_to_tlb(dt_end) + 1;
> +size = (ps << MAS1_TSIZE_SHIFT);
>  tlb->mas1 = MAS1_VALID | size;
> -tlb->mas2 = va & TARGET_PAGE_MASK;
> -tlb->mas7_3 = pa & TARGET_PAGE_MASK;
> +tlb->mas2 = 0;
> +tlb->mas7_3 = 0;
>  tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX;
>  
>  env->tlb_dirty = true;
> @@ -218,7 +228,7 @@ static void mpc8544ds_cpu_reset(void *opaque)
>  env->gpr[1] = (16<<20) - 8;
>  env->gpr[3] = bi->dt_base;
>  env->nip = bi->entry;
> -mmubooke_create_initial_mapping(env, 0, 0);
> +mmubooke_create_initial_mapping(env);
>  }
>  
>  static void mpc8544ds_init(ram_addr_t ram_size,
> @@ -374,13 +384,15 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>  /* If we're loading a kernel directly, we must load the device tree too. 
> */
>  if (kernel_filename) {
>  struct boot_info *boot_info;
> +int dt_size;
>  
>  #ifndef CONFIG_FDT
>  cpu_abort(env, "Compiled without FDT support - can't load kernel\n");
>  #endif
> -dt_base = (kernel_size + DTC_LOAD_PAD) & ~DTC_PAD_MASK;
> -if (mpc8544_load_device_tree(env, dt_base, ram_size,
> -initrd_base, initrd_size, kernel_cmdline) < 0) {
> +dt_base = (loadaddr + kernel_size + DTC_LOAD_PAD) & ~DTC_PAD_MASK;
> +dt_size = mpc8544_load_device_tree(env, dt_base, ram_size, 
> initrd_base,
> +   initrd_size, kernel_cmdline);
> +if (dt_size < 0) {
>  fprintf(stderr, "couldn't load device tree\n");
>  exit(1);
>  }
> @@ -388,6 +400,7 @@ static void mpc8544ds_init(ram_addr_t ram_size,
>  boot_info = env->load_info;

This is turned into cpu->load_info in my series. Since this patch is not
marked 1.1, please ack and apply my patches first, so that we don't run
into conflicts with qom-next. Thanks.

Andreas

>  boot_info->entry = entry;
>  boot_info->dt_base = dt_base;
> +boot_info->dt_size = dt_size;
>  }
>  
>  if (kvm_enabled()) {


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

On Sun, May 20, 2012 at 04:39:01PM +0300, Avi Kivity wrote:
> On 05/20/2012 03:59 PM, Gleb Natapov wrote:
> > > > > > > 
> > > > > > > Do we actually have to patch the DSDT?  Or can _S3 etc be made 
> > > > > > > into
> > > > > > > functions instead? (and talk to the bios, or even to fwcfg 
> > > > > > > directly?)
> > > > > > > 
> > > > > > We better not talk to fwcfg after OSPM is started since this is 
> > > > > > firmware
> > > > > > confing interface.
> > > > > 
> > > > > Why not?  The OS isn't going to talk to it, so we can have a driver 
> > > > > in ACPI.
> > > > > 
> > > > The OS is going to talk to it since the OS is the one who interprets
> > > > AML. 
> > > 
> > > I meant, not directly.  So the driver in ACPI has exclusive access.
> > > 
> > What's the difference?
> 
> ACPI is firmware, not OS.
AML is a data provided by firmware. AML's runtime is different from firmware's.

> 
> > > > We may want to disable fwcfg after OS bootup at all in the feature.
> > > > Who knows what kind of sensitive information we may want to pass by it
> > > > in the feature? May be something TPM related? 
> > > 
> > > fwcfg is for passing information to the guest.  If you want to hide
> > > something from the guest, just don't put it in fwcfg.
> > > 
> > Where to put it if we want to pass it to a firmware, but not an OS.
> > That was the point of fwcfg. If you want to pass something to a guest OS
> > use virtio-serial.
> 
> See above.
> 
> > > > And I do not see any advantage
> > > > of using fwcfg from AML.
> > > 
> > > It's an alternative to patching AML.  Sure it takes some effort to write
> > > the driver, but afterwards we can modify the guest behaviour more
> > > easily.  One possible client is -M old, so you can revert to previous
> > > behaviour depending on fwcfg data.
> > -M old is easy to support with the current patch. You just set new
> > properties to compatibility values. The code is written with this in
> > mind. And this is not an alternative to patching AML as I am trying to
> > explain to you below. You can eliminate patching of s4 value, but that's
> > it, you still need to patch out _S3/_S4 names.
> 
> What about
> 
>   If (Fcfg(...)) {
> Method()...
>   }
> 
> ?
syntax error, unexpected PARSEOP_IF

> 
> (i.e.. define the method conditionally at runtime)
> 
> >  
> > > 
> > > (we don't need a driver in AML to avoid patching, we can have AML talk
> > > to the bios and the bios drive fwcfg; but I think we'll find uses for a
> > > driver).
> > I am not sure what you mean. AML can't talk to the bios. It can read
> > values that bios put somewhere. 
> 
> That's what I meant - communicate through memory.
> 
What's the benefit? The patching is still needed. You need to pass
address of OperationRegion() to AML. You can do it either by patching or
by creating OperationRegion() code dynamically.

> > I do not see advantage of this method
> > and it requires patching still.
> 
> For the existence of the names?  Yes, if we can't avoid it it's a
> problem.  But if we can avoid patching, we should.
> 
If we can, we should, but we can't as far as I see. The patching was here long 
before
this patch.

> > > 
> > > >
> > > > > >  Regardless, presence of _S3 name or method is all
> > > > > > that needed for OS enabling S3 option. If _S3 is defined as a 
> > > > > > method it
> > > > > > has to return Package() otherwise iasl refuses to compile it.
> > > > > 
> > > > > Can't we Return (Package (...) { ... }) or equivalent? 
> > > > > 
> > > > We can, how does it help?
> > > >
> > > 
> > > The contents of the package can be determined at runtime.
> > > 
> > And? _S3 name should not exists at all in order to disable S3, not return
> > something different.
> >
> 
> See above.
> 
Does not work for me, can you send me a patch that works?

--
Gleb.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity]

On 05/20/2012 03:59 PM, Gleb Natapov wrote:
> > > > > > 
> > > > > > Do we actually have to patch the DSDT?  Or can _S3 etc be made into
> > > > > > functions instead? (and talk to the bios, or even to fwcfg 
> > > > > > directly?)
> > > > > > 
> > > > > We better not talk to fwcfg after OSPM is started since this is 
> > > > > firmware
> > > > > confing interface.
> > > > 
> > > > Why not?  The OS isn't going to talk to it, so we can have a driver in 
> > > > ACPI.
> > > > 
> > > The OS is going to talk to it since the OS is the one who interprets
> > > AML. 
> > 
> > I meant, not directly.  So the driver in ACPI has exclusive access.
> > 
> What's the difference?

ACPI is firmware, not OS.

> > > We may want to disable fwcfg after OS bootup at all in the feature.
> > > Who knows what kind of sensitive information we may want to pass by it
> > > in the feature? May be something TPM related? 
> > 
> > fwcfg is for passing information to the guest.  If you want to hide
> > something from the guest, just don't put it in fwcfg.
> > 
> Where to put it if we want to pass it to a firmware, but not an OS.
> That was the point of fwcfg. If you want to pass something to a guest OS
> use virtio-serial.

See above.

> > > And I do not see any advantage
> > > of using fwcfg from AML.
> > 
> > It's an alternative to patching AML.  Sure it takes some effort to write
> > the driver, but afterwards we can modify the guest behaviour more
> > easily.  One possible client is -M old, so you can revert to previous
> > behaviour depending on fwcfg data.
> -M old is easy to support with the current patch. You just set new
> properties to compatibility values. The code is written with this in
> mind. And this is not an alternative to patching AML as I am trying to
> explain to you below. You can eliminate patching of s4 value, but that's
> it, you still need to patch out _S3/_S4 names.

What about

  If (Fcfg(...)) {
Method()...
  }

?

(i.e.. define the method conditionally at runtime)

>  
> > 
> > (we don't need a driver in AML to avoid patching, we can have AML talk
> > to the bios and the bios drive fwcfg; but I think we'll find uses for a
> > driver).
> I am not sure what you mean. AML can't talk to the bios. It can read
> values that bios put somewhere. 

That's what I meant - communicate through memory.

> I do not see advantage of this method
> and it requires patching still.

For the existence of the names?  Yes, if we can't avoid it it's a
problem.  But if we can avoid patching, we should.

> > 
> > >
> > > > >  Regardless, presence of _S3 name or method is all
> > > > > that needed for OS enabling S3 option. If _S3 is defined as a method 
> > > > > it
> > > > > has to return Package() otherwise iasl refuses to compile it.
> > > > 
> > > > Can't we Return (Package (...) { ... }) or equivalent? 
> > > > 
> > > We can, how does it help?
> > >
> > 
> > The contents of the package can be determined at runtime.
> > 
> And? _S3 name should not exists at all in order to disable S3, not return
> something different.
>

See above.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

On Sun, May 20, 2012 at 03:47:02PM +0300, Avi Kivity wrote:
> On 05/20/2012 03:36 PM, Gleb Natapov wrote:
> > On Sun, May 20, 2012 at 03:30:50PM +0300, Avi Kivity wrote:
> > > On 05/20/2012 03:15 PM, Gleb Natapov wrote:
> > > > On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote:
> > > > > On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> > > > > > QEMU may want to disable guest's S3/S4 support and it wants to 
> > > > > > distinguish
> > > > > > between regular powerdown and S4 powerdown. To support that new 
> > > > > > fw_cfg
> > > > > > option was added that passes supported system states and what value 
> > > > > > should
> > > > > > guest use to enter each state. States are passed in 6 byte array. 
> > > > > > Each
> > > > > > byte represents one system state. If byte at offset X has its MSB 
> > > > > > set
> > > > > > it means that system state X is supported and to enter it guest 
> > > > > > should
> > > > > > use the value from lowest 7 bits. Patch also detects old QEMU and 
> > > > > > uses
> > > > > > values that work in backwards compatible way there.
> > > > > >
> > > > > 
> > > > > 
> > > > > Do we actually have to patch the DSDT?  Or can _S3 etc be made into
> > > > > functions instead? (and talk to the bios, or even to fwcfg directly?)
> > > > > 
> > > > We better not talk to fwcfg after OSPM is started since this is firmware
> > > > confing interface.
> > > 
> > > Why not?  The OS isn't going to talk to it, so we can have a driver in 
> > > ACPI.
> > > 
> > The OS is going to talk to it since the OS is the one who interprets
> > AML. 
> 
> I meant, not directly.  So the driver in ACPI has exclusive access.
> 
What's the difference?

> > We may want to disable fwcfg after OS bootup at all in the feature.
> > Who knows what kind of sensitive information we may want to pass by it
> > in the feature? May be something TPM related? 
> 
> fwcfg is for passing information to the guest.  If you want to hide
> something from the guest, just don't put it in fwcfg.
> 
Where to put it if we want to pass it to a firmware, but not an OS.
That was the point of fwcfg. If you want to pass something to a guest OS
use virtio-serial.

> > And I do not see any advantage
> > of using fwcfg from AML.
> 
> It's an alternative to patching AML.  Sure it takes some effort to write
> the driver, but afterwards we can modify the guest behaviour more
> easily.  One possible client is -M old, so you can revert to previous
> behaviour depending on fwcfg data.
-M old is easy to support with the current patch. You just set new
properties to compatibility values. The code is written with this in
mind. And this is not an alternative to patching AML as I am trying to
explain to you below. You can eliminate patching of s4 value, but that's
it, you still need to patch out _S3/_S4 names.
 
> 
> (we don't need a driver in AML to avoid patching, we can have AML talk
> to the bios and the bios drive fwcfg; but I think we'll find uses for a
> driver).
I am not sure what you mean. AML can't talk to the bios. It can read
values that bios put somewhere. I do not see advantage of this method
and it requires patching still.

> 
> >
> > > >  Regardless, presence of _S3 name or method is all
> > > > that needed for OS enabling S3 option. If _S3 is defined as a method it
> > > > has to return Package() otherwise iasl refuses to compile it.
> > > 
> > > Can't we Return (Package (...) { ... }) or equivalent? 
> > > 
> > We can, how does it help?
> >
> 
> The contents of the package can be determined at runtime.
> 
And? _S3 name should not exists at all in order to disable S3, not return
something different.

--
Gleb.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity]

On 05/20/2012 03:36 PM, Gleb Natapov wrote:
> On Sun, May 20, 2012 at 03:30:50PM +0300, Avi Kivity wrote:
> > On 05/20/2012 03:15 PM, Gleb Natapov wrote:
> > > On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote:
> > > > On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> > > > > QEMU may want to disable guest's S3/S4 support and it wants to 
> > > > > distinguish
> > > > > between regular powerdown and S4 powerdown. To support that new fw_cfg
> > > > > option was added that passes supported system states and what value 
> > > > > should
> > > > > guest use to enter each state. States are passed in 6 byte array. Each
> > > > > byte represents one system state. If byte at offset X has its MSB set
> > > > > it means that system state X is supported and to enter it guest should
> > > > > use the value from lowest 7 bits. Patch also detects old QEMU and uses
> > > > > values that work in backwards compatible way there.
> > > > >
> > > > 
> > > > 
> > > > Do we actually have to patch the DSDT?  Or can _S3 etc be made into
> > > > functions instead? (and talk to the bios, or even to fwcfg directly?)
> > > > 
> > > We better not talk to fwcfg after OSPM is started since this is firmware
> > > confing interface.
> > 
> > Why not?  The OS isn't going to talk to it, so we can have a driver in ACPI.
> > 
> The OS is going to talk to it since the OS is the one who interprets
> AML. 

I meant, not directly.  So the driver in ACPI has exclusive access.

> We may want to disable fwcfg after OS bootup at all in the feature.
> Who knows what kind of sensitive information we may want to pass by it
> in the feature? May be something TPM related? 

fwcfg is for passing information to the guest.  If you want to hide
something from the guest, just don't put it in fwcfg.

> And I do not see any advantage
> of using fwcfg from AML.

It's an alternative to patching AML.  Sure it takes some effort to write
the driver, but afterwards we can modify the guest behaviour more
easily.  One possible client is -M old, so you can revert to previous
behaviour depending on fwcfg data.

(we don't need a driver in AML to avoid patching, we can have AML talk
to the bios and the bios drive fwcfg; but I think we'll find uses for a
driver).

>
> > >  Regardless, presence of _S3 name or method is all
> > > that needed for OS enabling S3 option. If _S3 is defined as a method it
> > > has to return Package() otherwise iasl refuses to compile it.
> > 
> > Can't we Return (Package (...) { ... }) or equivalent? 
> > 
> We can, how does it help?
>

The contents of the package can be determined at runtime.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

On Sun, May 20, 2012 at 03:30:50PM +0300, Avi Kivity wrote:
> On 05/20/2012 03:15 PM, Gleb Natapov wrote:
> > On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote:
> > > On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> > > > QEMU may want to disable guest's S3/S4 support and it wants to 
> > > > distinguish
> > > > between regular powerdown and S4 powerdown. To support that new fw_cfg
> > > > option was added that passes supported system states and what value 
> > > > should
> > > > guest use to enter each state. States are passed in 6 byte array. Each
> > > > byte represents one system state. If byte at offset X has its MSB set
> > > > it means that system state X is supported and to enter it guest should
> > > > use the value from lowest 7 bits. Patch also detects old QEMU and uses
> > > > values that work in backwards compatible way there.
> > > >
> > > 
> > > 
> > > Do we actually have to patch the DSDT?  Or can _S3 etc be made into
> > > functions instead? (and talk to the bios, or even to fwcfg directly?)
> > > 
> > We better not talk to fwcfg after OSPM is started since this is firmware
> > confing interface.
> 
> Why not?  The OS isn't going to talk to it, so we can have a driver in ACPI.
> 
The OS is going to talk to it since the OS is the one who interprets
AML. We may want to disable fwcfg after OS bootup at all in the feature.
Who knows what kind of sensitive information we may want to pass by it
in the feature? May be something TPM related? And I do not see any advantage
of using fwcfg from AML.

> >  Regardless, presence of _S3 name or method is all
> > that needed for OS enabling S3 option. If _S3 is defined as a method it
> > has to return Package() otherwise iasl refuses to compile it.
> 
> Can't we Return (Package (...) { ... }) or equivalent? 
> 
We can, how does it help?

--
Gleb.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity]

On 05/20/2012 03:15 PM, Gleb Natapov wrote:
> On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote:
> > On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> > > QEMU may want to disable guest's S3/S4 support and it wants to distinguish
> > > between regular powerdown and S4 powerdown. To support that new fw_cfg
> > > option was added that passes supported system states and what value should
> > > guest use to enter each state. States are passed in 6 byte array. Each
> > > byte represents one system state. If byte at offset X has its MSB set
> > > it means that system state X is supported and to enter it guest should
> > > use the value from lowest 7 bits. Patch also detects old QEMU and uses
> > > values that work in backwards compatible way there.
> > >
> > 
> > 
> > Do we actually have to patch the DSDT?  Or can _S3 etc be made into
> > functions instead? (and talk to the bios, or even to fwcfg directly?)
> > 
> We better not talk to fwcfg after OSPM is started since this is firmware
> confing interface.

Why not?  The OS isn't going to talk to it, so we can have a driver in ACPI.

>  Regardless, presence of _S3 name or method is all
> that needed for OS enabling S3 option. If _S3 is defined as a method it
> has to return Package() otherwise iasl refuses to compile it.

Can't we Return (Package (...) { ... }) or equivalent? 

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

On Sun, May 20, 2012 at 02:44:51PM +0300, Avi Kivity wrote:
> On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> > QEMU may want to disable guest's S3/S4 support and it wants to distinguish
> > between regular powerdown and S4 powerdown. To support that new fw_cfg
> > option was added that passes supported system states and what value should
> > guest use to enter each state. States are passed in 6 byte array. Each
> > byte represents one system state. If byte at offset X has its MSB set
> > it means that system state X is supported and to enter it guest should
> > use the value from lowest 7 bits. Patch also detects old QEMU and uses
> > values that work in backwards compatible way there.
> >
> 
> 
> Do we actually have to patch the DSDT?  Or can _S3 etc be made into
> functions instead? (and talk to the bios, or even to fwcfg directly?)
> 
We better not talk to fwcfg after OSPM is started since this is firmware
confing interface. Regardless, presence of _S3 name or method is all
that needed for OS enabling S3 option. If _S3 is defined as a method it
has to return Package() otherwise iasl refuses to compile it.

--
Gleb.

Re: [Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Avi Kivity]

On 05/20/2012 12:03 PM, Gleb Natapov wrote:
> QEMU may want to disable guest's S3/S4 support and it wants to distinguish
> between regular powerdown and S4 powerdown. To support that new fw_cfg
> option was added that passes supported system states and what value should
> guest use to enter each state. States are passed in 6 byte array. Each
> byte represents one system state. If byte at offset X has its MSB set
> it means that system state X is supported and to enter it guest should
> use the value from lowest 7 bits. Patch also detects old QEMU and uses
> values that work in backwards compatible way there.
>


Do we actually have to patch the DSDT?  Or can _S3 etc be made into
functions instead? (and talk to the bios, or even to fwcfg directly?)

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [SeaBIOS] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.

[Gleb Natapov]

On Sun, May 20, 2012 at 01:32:18PM +0300, Alon Levy wrote:
> On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb Natapov wrote:
> > 
> > Signed-off-by: Gleb Natapov 
> > ---
> >  tools/acpi_extract.py |6 +-
> >  1 files changed, 5 insertions(+), 1 deletions(-)
> > 
> > diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
> > index 5f613e4..8038269 100755
> > --- a/tools/acpi_extract.py
> > +++ b/tools/acpi_extract.py
> > @@ -121,7 +121,11 @@ def aml_name_string(offset):
> >  if (aml[offset] != 0x08):
> >  die( "Name offset 0x%x: expected 0x08 actual 0x%x" %
> >   (offset, aml[offset]));
> > -return offset + 1;
> > +offset += 1
> > +# Block Name Modifier. Skip it.
> > +if (aml[offset] == 0x5c or aml[offset] == 0x5e):
> 
> You don't need parenthesis around the whole conditional.
Rest of the code has it. Better to keep same style :)

> 
> > +offset += 1
> > +return offset;
> >  
> >  # Given data offset, find dword const offset
> >  def aml_data_dword_const(offset):
> > -- 
> > 1.7.7.3
> > 
> > 
> > ___
> > SeaBIOS mailing list
> > seab...@seabios.org
> > http://www.seabios.org/mailman/listinfo/seabios

--
Gleb.

Re: [Qemu-devel] [SeaBIOS] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.

[Alon Levy]

On Sun, May 20, 2012 at 12:03:38PM +0300, Gleb Natapov wrote:
> 
> Signed-off-by: Gleb Natapov 
> ---
>  tools/acpi_extract.py |6 +-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
> index 5f613e4..8038269 100755
> --- a/tools/acpi_extract.py
> +++ b/tools/acpi_extract.py
> @@ -121,7 +121,11 @@ def aml_name_string(offset):
>  if (aml[offset] != 0x08):
>  die( "Name offset 0x%x: expected 0x08 actual 0x%x" %
>   (offset, aml[offset]));
> -return offset + 1;
> +offset += 1
> +# Block Name Modifier. Skip it.
> +if (aml[offset] == 0x5c or aml[offset] == 0x5e):

You don't need parenthesis around the whole conditional.

> +offset += 1
> +return offset;
>  
>  # Given data offset, find dword const offset
>  def aml_data_dword_const(offset):
> -- 
> 1.7.7.3
> 
> 
> ___
> SeaBIOS mailing list
> seab...@seabios.org
> http://www.seabios.org/mailman/listinfo/seabios

Re: [Qemu-devel] [PATCH v3] pci: call object_unparent() before free_qdev()

[Michael S. Tsirkin]

On Sun, May 20, 2012 at 05:57:45PM +0800, Amos Kong wrote:
> Start VM with 8 multiple-function block devs, hot-removing
> those block devs by 'device_del ...' would cause qemu abort.
> 
> | (qemu) device_del virti0-0-0
> | (qemu) **
> |ERROR:qom/object.c:389:object_delete: assertion failed: (obj->ref == 0)
> 
> It's a regression introduced by commit 57c9fafe
> 
> The whole PCI slot should be removed once. Currently only one func
> is cleaned in pci_unplug_device(), if you try to remove a single
> func by monitor cmd.
> 
> free_qdev() are called for all functions in slot,
> but unparent_delete() is only called for one
> function.
> 
> ---
> aliguori has a better resolution, better to do it in 1.2
> 
> v2: fix warning: too many arguments for format
> v3: move object_unparent() to acpi_piix_eject_slot()
> 
> Signed-off-by: Amos Kong 

commit is mangled up a bit.  It should be:

subject: 

commit log

Signed-off-by: 

--- 

Versioning info

diff


No need to repost just we that. But we also need to update other pci
hotplug users: hw//shpc.c hw//pcie.c
Not sure about pci-hotplug.c (calls qdev_free on error
handling) - add a virtio blk function with wrong drive
parameter using pci_add and see. Anything else?

> ---
>  hw/acpi_piix4.c |1 +
>  hw/pci.c|1 -
>  2 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index 585da4e..0345490 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -299,6 +299,7 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, 
> unsigned slots)
>  if (pc->no_hotplug) {
>  slot_free = false;
>  } else {
> +object_unparent(OBJECT(dev));
>  qdev_free(qdev);
>  }
>  }
> diff --git a/hw/pci.c b/hw/pci.c
> index b706e69..c1ebdde 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1527,7 +1527,6 @@ static int pci_unplug_device(DeviceState *qdev)
>  qerror_report(QERR_DEVICE_NO_HOTPLUG, 
> object_get_typename(OBJECT(dev)));
>  return -1;
>  }
> -object_unparent(OBJECT(dev));
>  return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
>   PCI_HOTPLUG_DISABLED);
>  }
> -- 
> 1.7.1

Re: [Qemu-devel] [Qemu-ppc] [PATCH] booke_206_tlbwe: Discard invalid bits in MAS2

[Alexander Graf]

On 20.05.2012, at 12:15, Alexander Graf  wrote:

> 
> 
> On 09.05.2012, at 15:28, Fabien Chouteau  wrote:
> 
>> The size of EPN field in MAS2 depends on page size. This patch adds a
>> mask to discard invalid bits in EPN field.
>> 
>> Definition of EPN field from e500v2 RM:
>> EPN Effective page number: Depending on page size, only the bits
>> associated with a page boundary are valid. Bits that represent offsets
>> within a page are ignored and should be cleared.
>> 
>> There is a similar (but more complicated) definition in PowerISA V2.06.
>> 
>> Signed-off-by: Fabien Chouteau 
>> ---
>> target-ppc/op_helper.c |   10 --
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
>> index 4ef2332..6bc64ad 100644
>> --- a/target-ppc/op_helper.c
>> +++ b/target-ppc/op_helper.c
>> @@ -4227,6 +4227,8 @@ void helper_booke206_tlbwe(void)
>>uint32_t tlbncfg, tlbn;
>>ppcmas_tlb_t *tlb;
>>uint32_t size_tlb, size_ps;
>> +target_ulong mask;
>> +
>> 
>>switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) {
>>case MAS0_WQ_ALWAYS:
>> @@ -4289,8 +4291,12 @@ void helper_booke206_tlbwe(void)
>>tlb->mas1 |= (tlbncfg & TLBnCFG_MINSIZE) >> 12;
>>}
>> 
>> -/* XXX needs to change when supporting 64-bit e500 */
>> -tlb->mas2 = env->spr[SPR_BOOKE_MAS2] & 0x;
>> +/* Make a mask from TLB size to discard invalid bits in EPN field */
>> +mask = ~(booke206_tlb_to_page_size(env, tlb)
> 
> This breaks execution of -cpu with qemu-system-ppc64, no?

-cpu e500 I mean of course :).

Alex

Re: [Qemu-devel] [PATCH] booke_206_tlbwe: Discard invalid bits in MAS2

[Alexander Graf]

On 09.05.2012, at 15:28, Fabien Chouteau  wrote:

> The size of EPN field in MAS2 depends on page size. This patch adds a
> mask to discard invalid bits in EPN field.
> 
> Definition of EPN field from e500v2 RM:
> EPN Effective page number: Depending on page size, only the bits
> associated with a page boundary are valid. Bits that represent offsets
> within a page are ignored and should be cleared.
> 
> There is a similar (but more complicated) definition in PowerISA V2.06.
> 
> Signed-off-by: Fabien Chouteau 
> ---
> target-ppc/op_helper.c |   10 --
> 1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index 4ef2332..6bc64ad 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -4227,6 +4227,8 @@ void helper_booke206_tlbwe(void)
> uint32_t tlbncfg, tlbn;
> ppcmas_tlb_t *tlb;
> uint32_t size_tlb, size_ps;
> +target_ulong mask;
> +
> 
> switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) {
> case MAS0_WQ_ALWAYS:
> @@ -4289,8 +4291,12 @@ void helper_booke206_tlbwe(void)
> tlb->mas1 |= (tlbncfg & TLBnCFG_MINSIZE) >> 12;
> }
> 
> -/* XXX needs to change when supporting 64-bit e500 */
> -tlb->mas2 = env->spr[SPR_BOOKE_MAS2] & 0x;
> +/* Make a mask from TLB size to discard invalid bits in EPN field */
> +mask = ~(booke206_tlb_to_page_size(env, tlb)

This breaks execution of -cpu with qemu-system-ppc64, no?

(sorry for the late reply)


Alex

[Qemu-devel] [PATCH v3] pci: call object_unparent() before free_qdev()

[Amos Kong]

Start VM with 8 multiple-function block devs, hot-removing
those block devs by 'device_del ...' would cause qemu abort.

| (qemu) device_del virti0-0-0
| (qemu) **
|ERROR:qom/object.c:389:object_delete: assertion failed: (obj->ref == 0)

It's a regression introduced by commit 57c9fafe

The whole PCI slot should be removed once. Currently only one func
is cleaned in pci_unplug_device(), if you try to remove a single
func by monitor cmd.

free_qdev() are called for all functions in slot,
but unparent_delete() is only called for one
function.

---
aliguori has a better resolution, better to do it in 1.2

v2: fix warning: too many arguments for format
v3: move object_unparent() to acpi_piix_eject_slot()

Signed-off-by: Amos Kong 
---
 hw/acpi_piix4.c |1 +
 hw/pci.c|1 -
 2 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 585da4e..0345490 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -299,6 +299,7 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned 
slots)
 if (pc->no_hotplug) {
 slot_free = false;
 } else {
+object_unparent(OBJECT(dev));
 qdev_free(qdev);
 }
 }
diff --git a/hw/pci.c b/hw/pci.c
index b706e69..c1ebdde 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1527,7 +1527,6 @@ static int pci_unplug_device(DeviceState *qdev)
 qerror_report(QERR_DEVICE_NO_HOTPLUG, 
object_get_typename(OBJECT(dev)));
 return -1;
 }
-object_unparent(OBJECT(dev));
 return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
  PCI_HOTPLUG_DISABLED);
 }
-- 
1.7.1

[Qemu-devel] [PATCH 3/3] Get system state configuration from QEMU and patch DSDT with it.

[Gleb Natapov]

QEMU may want to disable guest's S3/S4 support and it wants to distinguish
between regular powerdown and S4 powerdown. To support that new fw_cfg
option was added that passes supported system states and what value should
guest use to enter each state. States are passed in 6 byte array. Each
byte represents one system state. If byte at offset X has its MSB set
it means that system state X is supported and to enter it guest should
use the value from lowest 7 bits. Patch also detects old QEMU and uses
values that work in backwards compatible way there.

Signed-off-by: Gleb Natapov 
---
 src/acpi-dsdt.dsl  |   32 -
 src/acpi-dsdt.hex  |   42 +---
 src/acpi.c |   15 
 src/ssdt-pcihp.dsl |   36 ++
 src/ssdt-pcihp.hex |  185 +---
 5 files changed, 172 insertions(+), 138 deletions(-)

diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 4bdc268..37899fc 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -604,38 +604,6 @@ DefinitionBlock (
 }
 }
 
-
-/
- * Suspend
- /
-
-/*
- * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
- * must match piix4 emulation.
- */
-Name (\_S3, Package (0x04)
-{
-0x01,  /* PM1a_CNT.SLP_TYP */
-0x01,  /* PM1b_CNT.SLP_TYP */
-Zero,  /* reserved */
-Zero   /* reserved */
-})
-Name (\_S4, Package (0x04)
-{
-Zero,  /* PM1a_CNT.SLP_TYP */
-Zero,  /* PM1b_CNT.SLP_TYP */
-Zero,  /* reserved */
-Zero   /* reserved */
-})
-Name (\_S5, Package (0x04)
-{
-Zero,  /* PM1a_CNT.SLP_TYP */
-Zero,  /* PM1b_CNT.SLP_TYP */
-Zero,  /* reserved */
-Zero   /* reserved */
-})
-
-
 /
  * CPU hotplug
  /
diff --git a/src/acpi-dsdt.hex b/src/acpi-dsdt.hex
index a4af597..8678fbf 100644
--- a/src/acpi-dsdt.hex
+++ b/src/acpi-dsdt.hex
@@ -3,12 +3,12 @@ static unsigned char AmlCode[] = {
 0x53,
 0x44,
 0x54,
-0x21,
-0x11,
+0xfd,
+0x10,
 0x0,
 0x0,
 0x1,
-0xe8,
+0x4a,
 0x42,
 0x58,
 0x50,
@@ -3925,42 +3925,6 @@ static unsigned char AmlCode[] = {
 0x52,
 0x51,
 0x30,
-0x8,
-0x5f,
-0x53,
-0x33,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x1,
-0x1,
-0x0,
-0x0,
-0x8,
-0x5f,
-0x53,
-0x34,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x0,
-0x0,
-0x0,
-0x0,
-0x8,
-0x5f,
-0x53,
-0x35,
-0x5f,
-0x12,
-0x6,
-0x4,
-0x0,
-0x0,
-0x0,
-0x0,
 0x10,
 0x49,
 0xe,
diff --git a/src/acpi.c b/src/acpi.c
index 30888b9..06ffe0a 100644
--- a/src/acpi.c
+++ b/src/acpi.c
@@ -492,6 +492,8 @@ extern void link_time_assertion(void);
 
 static void* build_pcihp(void)
 {
+char *sys_states;
+int sys_state_size;
 u32 rmvc_pcrm;
 int i;
 
@@ -523,6 +525,19 @@ static void* build_pcihp(void)
 }
 }
 
+sys_states = romfile_loadfile("etc/system-states", &sys_state_size);
+if (!sys_states || sys_state_size != 6)
+sys_states = (char[]){128, 0, 0, 129, 128, 128};
+
+if (!(sys_states[3] & 128))
+ssdt[acpi_s3_name[0]] = 'X';
+if (!(sys_states[4] & 128))
+ssdt[acpi_s4_name[0]] = 'X';
+else
+ssdt[acpi_s4_pkg[0] + 1] = ssdt[acpi_s4_pkg[0] + 3] = sys_states[4] & 
127;
+((struct acpi_table_header*)ssdt)->checksum = 0;
+((struct acpi_table_header*)ssdt)->checksum -= checksum(ssdt, 
sizeof(ssdp_pcihp_aml));
+
 return ssdt;
 }
 
diff --git a/src/ssdt-pcihp.dsl b/src/ssdt-pcihp.dsl
index 4b435b8..12555e2 100644
--- a/src/ssdt-pcihp.dsl
+++ b/src/ssdt-pcihp.dsl
@@ -95,4 +95,40 @@ DefinitionBlock ("ssdt-pcihp.aml", "SSDT", 0x01, "BXPC", 
"BXSSDTPCIHP", 0x1)
 gen_pci_hotplug(1f)
 }
 }
+
+Scope(\) {
+/
+ * Suspend
+ /
+
+/*
+ * S3 (suspend-to-ram), S4 (suspend-to-disk) and S5 (power-off) type codes:
+ * must match piix4 emulation.
+ */
+
+ACPI_EXTRACT_NAME_STRING acpi_s3_name
+Name (_S3, Package (0x04)
+{
+One,  /* PM1a_CNT.SLP_TYP */
+One,  /* PM1b_CNT.SLP_TYP */
+Zero,  /* reserved */
+Zero   /* reserved */
+})
+ACPI_EXTRACT_NAME_STRING acpi_s4_name
+ACPI_EXTRACT_PKG_START acpi_s4_pkg
+Name (_S4, Package (0x04)
+{
+0x2,  /* PM1a_CNT.SLP_TYP */
+0x2,  /* PM1b_CNT.SLP_TYP */
+Zero,  /* reserved */
+Zero   /* reserved */
+})
+Name (_S5, Package (0x04)
+{
+Zero,  /* PM1a_CNT.SLP_TYP */
+Zero,  /* PM1b_CNT.SLP_TYP */
+Zero,  /* reserved */
+Zero   /* reserved */
+})
+}
 }
diff --git a/sr

[Qemu-devel] [PATCH 2/3] Add ACPI_EXTRACT_PKG_START macro parsing

[Gleb Natapov]

It allows to extract the beginning of a Package object content.

Signed-off-by: Gleb Natapov 
---
 tools/acpi_extract.py |   12 
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
index 8038269..167a322 100755
--- a/tools/acpi_extract.py
+++ b/tools/acpi_extract.py
@@ -29,6 +29,7 @@
 # ACPI_EXTRACT_PROCESSOR_START - start of Processor() block
 # ACPI_EXTRACT_PROCESSOR_STRING - extract a NameString from Processor()
 # ACPI_EXTRACT_PROCESSOR_END - offset at last byte of Processor() + 1
+# ACPI_EXTRACT_PKG_START - start of Package block
 #
 # ACPI_EXTRACT_ALL_CODE - create an array storing the generated AML bytecode
 # 
@@ -185,6 +186,15 @@ def aml_processor_end(offset):
 pkglen = aml_pkglen(offset)
 return offset + pkglen
 
+def aml_package_start(offset):
+offset = aml_name_string(offset) + 4
+# 0x12 PkgLength NumElements PackageElementList
+if (aml[offset] != 0x12):
+die( "Name offset 0x%x: expected 0x12 actual 0x%x" %
+ (offset, aml[offset]));
+offset += 1
+return offset + aml_pkglen_bytes(offset) + 1
+
 lineno = 0
 for line in fileinput.input():
 # Strip trailing newline
@@ -267,6 +277,8 @@ for i in range(len(asl)):
 offset = aml_processor_string(offset)
 elif (directive == "ACPI_EXTRACT_PROCESSOR_END"):
 offset = aml_processor_end(offset)
+elif (directive == "ACPI_EXTRACT_PKG_START"):
+offset = aml_package_start(offset)
 else:
 die("Unsupported directive %s" % directive)
 
-- 
1.7.7.3

[Qemu-devel] [PATCH 1/3] Fix aml_name_string() to recognize block name modifiers.

[Gleb Natapov]

Signed-off-by: Gleb Natapov 
---
 tools/acpi_extract.py |6 +-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/tools/acpi_extract.py b/tools/acpi_extract.py
index 5f613e4..8038269 100755
--- a/tools/acpi_extract.py
+++ b/tools/acpi_extract.py
@@ -121,7 +121,11 @@ def aml_name_string(offset):
 if (aml[offset] != 0x08):
 die( "Name offset 0x%x: expected 0x08 actual 0x%x" %
  (offset, aml[offset]));
-return offset + 1;
+offset += 1
+# Block Name Modifier. Skip it.
+if (aml[offset] == 0x5c or aml[offset] == 0x5e):
+offset += 1
+return offset;
 
 # Given data offset, find dword const offset
 def aml_data_dword_const(offset):
-- 
1.7.7.3

[Qemu-devel] [PATCH 1/2] Make pointer to fw_cfg device global.

[Gleb Natapov]

There can be only one fw_cfg device, so saving global reference to it
removes the need to pass its pointer around.

Signed-off-by: Gleb Natapov 
---
 hw/fw_cfg.c   |  110 +++--
 hw/fw_cfg.h   |   15 +++
 hw/loader.c   |   10 +
 hw/loader.h   |1 -
 hw/multiboot.c|   17 
 hw/multiboot.h|3 +-
 hw/pc.c   |   63 ++
 hw/ppc_newworld.c |   43 ++---
 hw/ppc_oldworld.c |   43 ++---
 hw/sun4m.c|   93 +---
 hw/sun4u.c|   35 -
 11 files changed, 207 insertions(+), 226 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 7b3b576..8c50473 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -48,7 +48,7 @@ typedef struct FWCfgEntry {
 FWCfgCallback callback;
 } FWCfgEntry;
 
-struct FWCfgState {
+static struct FWCfgState {
 SysBusDevice busdev;
 MemoryRegion ctl_iomem, data_iomem, comb_iomem;
 uint32_t ctl_iobase, data_iobase;
@@ -57,7 +57,7 @@ struct FWCfgState {
 uint16_t cur_entry;
 uint32_t cur_offset;
 Notifier machine_ready;
-};
+} *fw_cfg;
 
 #define JPG_FILE 0
 #define BMP_FILE 1
@@ -113,7 +113,7 @@ error:
 return NULL;
 }
 
-static void fw_cfg_bootsplash(FWCfgState *s)
+static void fw_cfg_bootsplash(void)
 {
 int boot_splash_time = -1;
 const char *boot_splash_filename = NULL;
@@ -148,7 +148,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
 /* use little endian format */
 qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time & 0xff);
 qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time >> 8) & 0xff);
-fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2);
+fw_cfg_add_file("etc/boot-menu-wait", qemu_extra_params_fw, 2);
 }
 
 /* insert splash file if user configurated */
@@ -173,10 +173,10 @@ static void fw_cfg_bootsplash(FWCfgState *s)
 
 /* insert data */
 if (file_type == JPG_FILE) {
-fw_cfg_add_file(s, "bootsplash.jpg",
+fw_cfg_add_file("bootsplash.jpg",
 boot_splash_filedata, boot_splash_filedata_size);
 } else {
-fw_cfg_add_file(s, "bootsplash.bmp",
+fw_cfg_add_file("bootsplash.bmp",
 boot_splash_filedata, boot_splash_filedata_size);
 }
 g_free(filename);
@@ -359,122 +359,126 @@ static const VMStateDescription vmstate_fw_cfg = {
 }
 };
 
-int fw_cfg_add_bytes(FWCfgState *s, uint16_t key, uint8_t *data, uint32_t len)
+int fw_cfg_add_bytes(uint16_t key, uint8_t *data, uint32_t len)
 {
 int arch = !!(key & FW_CFG_ARCH_LOCAL);
 
 key &= FW_CFG_ENTRY_MASK;
 
-if (key >= FW_CFG_MAX_ENTRY)
+if (key >= FW_CFG_MAX_ENTRY || !fw_cfg) {
 return 0;
+}
 
-s->entries[arch][key].data = data;
-s->entries[arch][key].len = len;
+fw_cfg->entries[arch][key].data = data;
+fw_cfg->entries[arch][key].len = len;
 
 return 1;
 }
 
-int fw_cfg_add_i16(FWCfgState *s, uint16_t key, uint16_t value)
+int fw_cfg_add_i16(uint16_t key, uint16_t value)
 {
 uint16_t *copy;
 
 copy = g_malloc(sizeof(value));
 *copy = cpu_to_le16(value);
-return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
+return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
 }
 
-int fw_cfg_add_i32(FWCfgState *s, uint16_t key, uint32_t value)
+int fw_cfg_add_i32(uint16_t key, uint32_t value)
 {
 uint32_t *copy;
 
 copy = g_malloc(sizeof(value));
 *copy = cpu_to_le32(value);
-return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
+return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
 }
 
-int fw_cfg_add_i64(FWCfgState *s, uint16_t key, uint64_t value)
+int fw_cfg_add_i64(uint16_t key, uint64_t value)
 {
 uint64_t *copy;
 
 copy = g_malloc(sizeof(value));
 *copy = cpu_to_le64(value);
-return fw_cfg_add_bytes(s, key, (uint8_t *)copy, sizeof(value));
+return fw_cfg_add_bytes(key, (uint8_t *)copy, sizeof(value));
 }
 
-int fw_cfg_add_callback(FWCfgState *s, uint16_t key, FWCfgCallback callback,
+int fw_cfg_add_callback(uint16_t key, FWCfgCallback callback,
 void *callback_opaque, uint8_t *data, size_t len)
 {
 int arch = !!(key & FW_CFG_ARCH_LOCAL);
 
-if (!(key & FW_CFG_WRITE_CHANNEL))
+if (!(key & FW_CFG_WRITE_CHANNEL) || !fw_cfg) {
 return 0;
+}
 
 key &= FW_CFG_ENTRY_MASK;
 
 if (key >= FW_CFG_MAX_ENTRY || len > 65535)
 return 0;
 
-s->entries[arch][key].data = data;
-s->entries[arch][key].len = len;
-s->entries[arch][key].callback_opaque = callback_opaque;
-s->entries[arch][key].callback = callback;
+fw_cfg->entries[arch][key].data = data;
+fw_cfg->entries[arch][key].len = len;
+fw_cfg->entries[arch][key].callback_opaque = callback_opaque;
+fw_cfg->entries[arch][key].callback

[Qemu-devel] [PATCH 2/2] Add PIIX4 properties to control PM system states.

[Gleb Natapov]

This patch adds two things. First it allows QEMU to distinguish between
regular powerdown and S4 powerdown. Later separate QMP notification will
be added for S4 powerdown. Second it allows S3/S4 states to be disabled
from QEMU command line. Some guests known to be broken with regards to
power management, but allow to use it anyway. Using new properties
management will be able to disable S3/S4 for such guests.

Supported system state are passed to a firmware using new fw_cfg file.
The file contains  6 byte array. Each byte represents one system
state. If byte at offset X has its MSB set it means that system state
X is supported and to enter it guest should use the value from lowest 3
bits.

Signed-off-by: Gleb Natapov 
---
 hw/acpi.c   |5 -
 hw/acpi.h   |2 +-
 hw/acpi_piix4.c |   16 +++-
 hw/vt82c686.c   |2 +-
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 5d521e5..effc7ec 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -370,7 +370,7 @@ void acpi_pm1_cnt_init(ACPIREGS *ar)
 qemu_register_wakeup_notifier(&ar->wakeup);
 }
 
-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4)
 {
 ar->pm1.cnt.cnt = val & ~(ACPI_BITMASK_SLEEP_ENABLE);
 
@@ -385,6 +385,9 @@ void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val)
 qemu_system_suspend_request();
 break;
 default:
+if (sus_typ == s4) { /* S4 request */
+qemu_system_shutdown_request();
+}
 break;
 }
 }
diff --git a/hw/acpi.h b/hw/acpi.h
index fe8cdb4..7337f41 100644
--- a/hw/acpi.h
+++ b/hw/acpi.h
@@ -139,7 +139,7 @@ void acpi_pm1_evt_reset(ACPIREGS *ar);
 
 /* PM1a_CNT: piix and ich9 don't implement PM1b CNT. */
 void acpi_pm1_cnt_init(ACPIREGS *ar);
-void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val);
+void acpi_pm1_cnt_write(ACPIREGS *ar, uint16_t val, char s4);
 void acpi_pm1_cnt_update(ACPIREGS *ar,
  bool sci_enable, bool sci_disable);
 void acpi_pm1_cnt_reset(ACPIREGS *ar);
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 585da4e..883314d 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -27,6 +27,7 @@
 #include "sysemu.h"
 #include "range.h"
 #include "ioport.h"
+#include "fw_cfg.h"
 
 //#define DEBUG
 
@@ -71,6 +72,10 @@ typedef struct PIIX4PMState {
 struct pci_status pci0_status;
 uint32_t pci0_hotplug_enable;
 uint32_t pci0_slot_device_present;
+
+uint8_t disable_s3;
+uint8_t disable_s4;
+uint8_t s4_val;
 } PIIX4PMState;
 
 static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s);
@@ -123,7 +128,7 @@ static void pm_ioport_write(IORange *ioport, uint64_t addr, 
unsigned width,
 pm_update_sci(s);
 break;
 case 0x04:
-acpi_pm1_cnt_write(&s->ar, val);
+acpi_pm1_cnt_write(&s->ar, val, s->s4_val);
 break;
 default:
 break;
@@ -425,6 +430,7 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t 
smb_io_base,
 {
 PCIDevice *dev;
 PIIX4PMState *s;
+uint8_t suspend[6] = {128, 0, 0, 129, 128, 128};
 
 dev = pci_create(bus, devfn, "PIIX4_PM");
 qdev_prop_set_uint32(&dev->qdev, "smb_io_base", smb_io_base);
@@ -437,11 +443,19 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t 
smb_io_base,
 
 qdev_init_nofail(&dev->qdev);
 
+suspend[3] = 1 | ((!s->disable_s3) << 7);
+suspend[4] = s->s4_val | ((!s->disable_s4) << 7);
+
+fw_cfg_add_file("etc/system-states", g_memdup(suspend, 6), 6);
+
 return s->smb.smbus;
 }
 
 static Property piix4_pm_properties[] = {
 DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
+DEFINE_PROP_UINT8("disable_s3", PIIX4PMState, disable_s3, 0),
+DEFINE_PROP_UINT8("disable_s4", PIIX4PMState, disable_s4, 0),
+DEFINE_PROP_UINT8("s4_val", PIIX4PMState, s4_val, 2),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/vt82c686.c b/hw/vt82c686.c
index 6fb7950..5d7c00c 100644
--- a/hw/vt82c686.c
+++ b/hw/vt82c686.c
@@ -210,7 +210,7 @@ static void pm_ioport_writew(void *opaque, uint32_t addr, 
uint32_t val)
 pm_update_sci(s);
 break;
 case 0x04:
-acpi_pm1_cnt_write(&s->ar, val);
+acpi_pm1_cnt_write(&s->ar, val, 0);
 break;
 default:
 break;
-- 
1.7.7.3