Re: [Qemu-devel] [PATCH 24/26] acpi, acpi_piix: factor out GPE logic
On Sun, Apr 17, 2011 at 06:53:12PM +0300, Avi Kivity wrote:
On 04/17/2011 04:50 PM, Isaku Yamahata wrote:
On Sun, Apr 17, 2011 at 04:17:51PM +0300, Avi Kivity wrote:
On 03/16/2011 11:29 AM, Isaku Yamahata wrote:
factor out ACPI GPE logic. Later it will be used by ICH9 ACPI.
I think this patch is causing qemu-kvm failures on migration:
(gdb) bt
#0 0x0049aff4 in qemu_put_be16s (f=0x1a74490, pv=0x2c02580,
size=2) at hw/hw.h:108
#1 put_uint16 (f=0x1a74490, pv=0x2c02580, size=2) at savevm.c:855
#2 0x0049c3e4 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0b00, opaque=0x1842ef0) at savevm.c:1436
#3 0x0049c3b6 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0aa0, opaque=0x1842b90) at savevm.c:1434
#4 0x0049c6f1 in vmstate_save (mon=value optimized out,
f=0x1a74490) at savevm.c:1459
#5 qemu_savevm_state_complete (mon=value optimized out, f=0x1a74490)
at savevm.c:1600
#6 0x0049455a in migrate_fd_put_ready (opaque=0x1847890) at
migration.c:383
#7 0x004ce2eb in qemu_run_timers (clock=value optimized out)
at qemu-timer.c:505
#8 0x004ce806 in qemu_run_all_timers () at qemu-timer.c:619
#9 0x00419463 in main_loop_wait (nonblocking=value optimized
out) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339
#10 0x00433927 in kvm_main_loop () at
/build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590
#11 0x0041a3a6 in main_loop (argc=value optimized out,
argv=value optimized out, envp=value optimized out)
at /build/home/tlv/akivity/qemu-kvm/vl.c:1369
#12 main (argc=value optimized out, argv=value optimized out,
envp=value optimized out) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257
The vmstate being migrated is gpe.
+#define VMSTATE_GPE_ARRAY(_field, _state)\
+ { \
+ .name = (stringify(_field)), \
+ .version_id = 0,\
+ .num= GPE_LEN, \
+ .info =vmstate_info_uint16, \
+ .size = sizeof(uint16_t), \
+ .flags = VMS_ARRAY | VMS_POINTER, \
+ .offset = vmstate_offset_pointer(_state, _field, uint8_t), \
+ }
+
static const VMStateDescription vmstate_gpe = {
.name = gpe,
.version_id = 1,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField []) {
-VMSTATE_UINT16(sts, struct gpe_regs),
-VMSTATE_UINT16(en, struct gpe_regs),
+VMSTATE_GPE_ARRAY(sts, ACPIGPE),
+VMSTATE_GPE_ARRAY(en, ACPIGPE),
VMSTATE_END_OF_LIST()
}
};
I'm no vmstate expert, but this does look odd. Why both VMS_ARRAY and
VMS_POINTER? aren't we trying to save/restore a simple 16-bit value? Or
at least we did before this patch.
That's right. the difference is, the new member type became uint8_t*.
Does the following help?
Unforunately not. The core is very similar though:
Hmm but it seems that there is another issue.
With the previous patch, I confirmed sts and en are saved as expected.
The related VMStateDescription are defined in acpi_piix.c in
the following order.
vmstate_acpi calls vmstate_gpe. and vmstate_gpe vmstate_acpi.
However, your call trace shows something different.
(I'm assuming the stack trace is correct. gdb or compiler optimization?)
static const VMStateDescription vmstate_gpe = {
...
static const VMStateDescription vmstate_acpi = {
.fields = (VMStateField []) {
...
VMSTATE_STRUCT(gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
#0 0x0049aff4 in qemu_put_be16s (f=0x2427490, pv=0x1ae7580,
size=2) at hw/hw.h:108
#1 put_uint16 (f=0x2427490, pv=0x1ae7580, size=2) at savevm.c:855
#2 0x0049c3e4 in vmstate_save_state (f=0x2427490,
vmsd=0x6f0b00, opaque=0x21f5ef0) at savevm.c:1436
vmsd = 0x6f0b00 = vmstate_gpe correct???
opaque = 0x21f5ef0 = PIIX4PMState-gpe
#3 0x0049c3b6 in vmstate_save_state (f=0x2427490,
vmsd=0x6f0aa0, opaque=0x21f5b90) at savevm.c:1434
vmsd = 0x6f0aa0 = vmstate_acpi correct?
opaque = 0x21f5b90 = PIIX4PMState
opaque seems correct as
0x21f5b90 - 0x21f5ef0 = 0x348 = offsetof(PIIX4PMState, gpe)
On my environment the stack trace looks like
(gdb) where
#0 vmstate_save_state (f=0x25d16d0, vmsd=0x695940, opaque=0x283a218)
at savevm.c:1374
#1 0x004bb4f7 in vmstate_save_state (f=0x25d16d0, vmsd=0x695a40,
opaque=0x2839ed0) at savevm.c:1412
...
(gdb) print vmstate_gpe
$32 = (const VMStateDescription *) 0x695940
(gdb) print vmstate_acpi
$33 = (const VMStateDescription *) 0x695a40
thanks,
#4 0x0049c6f1 in vmstate_save
Re: [Qemu-devel] [PATCH 24/26] acpi, acpi_piix: factor out GPE logic
On 04/18/2011 10:47 AM, Isaku Yamahata wrote:
Hmm but it seems that there is another issue.
With the previous patch, I confirmed sts and en are saved as expected.
The related VMStateDescription are defined in acpi_piix.c in
the following order.
vmstate_acpi calls vmstate_gpe. andvmstate_gpe vmstate_acpi.
(gdb) p vmstate_gpe
$1 = (const VMStateDescription *) 0x6f0b00
(gdb) p vmstate_acpi
$2 = (const VMStateDescription *) 0x6f0aa0
So no.
However, your call trace shows something different.
(I'm assuming the stack trace is correct. gdb or compiler optimization?)
static const VMStateDescription vmstate_gpe = {
...
static const VMStateDescription vmstate_acpi = {
.fields = (VMStateField []) {
...
VMSTATE_STRUCT(gpe, PIIX4PMState, 2, vmstate_gpe, ACPIGPE),
#0 0x0049aff4 in qemu_put_be16s (f=0x2427490, pv=0x1ae7580,
size=2) at hw/hw.h:108
#1 put_uint16 (f=0x2427490, pv=0x1ae7580, size=2) at savevm.c:855
#2 0x0049c3e4 in vmstate_save_state (f=0x2427490,
vmsd=0x6f0b00, opaque=0x21f5ef0) at savevm.c:1436
vmsd = 0x6f0b00 =vmstate_gpe correct???
Correct.
opaque = 0x21f5ef0 =PIIX4PMState-gpe
#3 0x0049c3b6 in vmstate_save_state (f=0x2427490,
vmsd=0x6f0aa0, opaque=0x21f5b90) at savevm.c:1434
vmsd = 0x6f0aa0 =vmstate_acpi correct?
Correct.
opaque = 0x21f5b90 =PIIX4PMState
opaque seems correct as
0x21f5b90 - 0x21f5ef0 = 0x348 = offsetof(PIIX4PMState, gpe)
On my environment the stack trace looks like
(gdb) where
#0 vmstate_save_state (f=0x25d16d0, vmsd=0x695940, opaque=0x283a218)
at savevm.c:1374
#1 0x004bb4f7 in vmstate_save_state (f=0x25d16d0, vmsd=0x695a40,
opaque=0x2839ed0) at savevm.c:1412
...
(gdb) printvmstate_gpe
$32 = (const VMStateDescription *) 0x695940
(gdb) printvmstate_acpi
$33 = (const VMStateDescription *) 0x695a40
Are you using qemu-kvm.git or qemu.git? I think there are indeed two
issues, your patch fixes the first and there is another that is specific
to qemu-kvm.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
Re: [Qemu-devel] [PATCH 24/26] acpi, acpi_piix: factor out GPE logic
On Mon, Apr 18, 2011 at 11:22:40AM +0300, Avi Kivity wrote: Are you using qemu-kvm.git or qemu.git? I think there are indeed two issues, your patch fixes the first and there is another that is specific to qemu-kvm. I'm using qemu.git. I've start to have a look at qemu-kvm.git, -- yamahata
Re: [Qemu-devel] [PATCH 24/26] acpi, acpi_piix: factor out GPE logic
On 03/16/2011 11:29 AM, Isaku Yamahata wrote:
factor out ACPI GPE logic. Later it will be used by ICH9 ACPI.
I think this patch is causing qemu-kvm failures on migration:
(gdb) bt
#0 0x0049aff4 in qemu_put_be16s (f=0x1a74490, pv=0x2c02580,
size=2) at hw/hw.h:108
#1 put_uint16 (f=0x1a74490, pv=0x2c02580, size=2) at savevm.c:855
#2 0x0049c3e4 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0b00, opaque=0x1842ef0) at savevm.c:1436
#3 0x0049c3b6 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0aa0, opaque=0x1842b90) at savevm.c:1434
#4 0x0049c6f1 in vmstate_save (mon=value optimized out,
f=0x1a74490) at savevm.c:1459
#5 qemu_savevm_state_complete (mon=value optimized out, f=0x1a74490)
at savevm.c:1600
#6 0x0049455a in migrate_fd_put_ready (opaque=0x1847890) at
migration.c:383
#7 0x004ce2eb in qemu_run_timers (clock=value optimized out)
at qemu-timer.c:505
#8 0x004ce806 in qemu_run_all_timers () at qemu-timer.c:619
#9 0x00419463 in main_loop_wait (nonblocking=value optimized
out) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339
#10 0x00433927 in kvm_main_loop () at
/build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590
#11 0x0041a3a6 in main_loop (argc=value optimized out,
argv=value optimized out, envp=value optimized out)
at /build/home/tlv/akivity/qemu-kvm/vl.c:1369
#12 main (argc=value optimized out, argv=value optimized out,
envp=value optimized out) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257
The vmstate being migrated is gpe.
+#define VMSTATE_GPE_ARRAY(_field, _state)\
+ { \
+ .name = (stringify(_field)), \
+ .version_id = 0,\
+ .num= GPE_LEN, \
+ .info =vmstate_info_uint16, \
+ .size = sizeof(uint16_t), \
+ .flags = VMS_ARRAY | VMS_POINTER, \
+ .offset = vmstate_offset_pointer(_state, _field, uint8_t), \
+ }
+
static const VMStateDescription vmstate_gpe = {
.name = gpe,
.version_id = 1,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField []) {
-VMSTATE_UINT16(sts, struct gpe_regs),
-VMSTATE_UINT16(en, struct gpe_regs),
+VMSTATE_GPE_ARRAY(sts, ACPIGPE),
+VMSTATE_GPE_ARRAY(en, ACPIGPE),
VMSTATE_END_OF_LIST()
}
};
I'm no vmstate expert, but this does look odd. Why both VMS_ARRAY and
VMS_POINTER? aren't we trying to save/restore a simple 16-bit value? Or
at least we did before this patch.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 24/26] acpi, acpi_piix: factor out GPE logic
On Sun, Apr 17, 2011 at 04:17:51PM +0300, Avi Kivity wrote:
On 03/16/2011 11:29 AM, Isaku Yamahata wrote:
factor out ACPI GPE logic. Later it will be used by ICH9 ACPI.
I think this patch is causing qemu-kvm failures on migration:
(gdb) bt
#0 0x0049aff4 in qemu_put_be16s (f=0x1a74490, pv=0x2c02580,
size=2) at hw/hw.h:108
#1 put_uint16 (f=0x1a74490, pv=0x2c02580, size=2) at savevm.c:855
#2 0x0049c3e4 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0b00, opaque=0x1842ef0) at savevm.c:1436
#3 0x0049c3b6 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0aa0, opaque=0x1842b90) at savevm.c:1434
#4 0x0049c6f1 in vmstate_save (mon=value optimized out,
f=0x1a74490) at savevm.c:1459
#5 qemu_savevm_state_complete (mon=value optimized out, f=0x1a74490)
at savevm.c:1600
#6 0x0049455a in migrate_fd_put_ready (opaque=0x1847890) at
migration.c:383
#7 0x004ce2eb in qemu_run_timers (clock=value optimized out)
at qemu-timer.c:505
#8 0x004ce806 in qemu_run_all_timers () at qemu-timer.c:619
#9 0x00419463 in main_loop_wait (nonblocking=value optimized
out) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339
#10 0x00433927 in kvm_main_loop () at
/build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590
#11 0x0041a3a6 in main_loop (argc=value optimized out,
argv=value optimized out, envp=value optimized out)
at /build/home/tlv/akivity/qemu-kvm/vl.c:1369
#12 main (argc=value optimized out, argv=value optimized out,
envp=value optimized out) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257
The vmstate being migrated is gpe.
+#define VMSTATE_GPE_ARRAY(_field, _state)\
+ { \
+ .name = (stringify(_field)), \
+ .version_id = 0,\
+ .num= GPE_LEN, \
+ .info =vmstate_info_uint16, \
+ .size = sizeof(uint16_t), \
+ .flags = VMS_ARRAY | VMS_POINTER, \
+ .offset = vmstate_offset_pointer(_state, _field, uint8_t), \
+ }
+
static const VMStateDescription vmstate_gpe = {
.name = gpe,
.version_id = 1,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField []) {
-VMSTATE_UINT16(sts, struct gpe_regs),
-VMSTATE_UINT16(en, struct gpe_regs),
+VMSTATE_GPE_ARRAY(sts, ACPIGPE),
+VMSTATE_GPE_ARRAY(en, ACPIGPE),
VMSTATE_END_OF_LIST()
}
};
I'm no vmstate expert, but this does look odd. Why both VMS_ARRAY and
VMS_POINTER? aren't we trying to save/restore a simple 16-bit value? Or
at least we did before this patch.
That's right. the difference is, the new member type became uint8_t*.
Does the following help?
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 96f5222..3a8fece 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -214,10 +214,9 @@ static int vmstate_acpi_post_load(void *opaque, int
version_id)
{ \
.name = (stringify(_field)), \
.version_id = 0,\
- .num= GPE_LEN, \
.info = vmstate_info_uint16, \
.size = sizeof(uint16_t), \
- .flags = VMS_ARRAY | VMS_POINTER, \
+ .flags = VMS_SINGLE | VMS_POINTER, \
.offset = vmstate_offset_pointer(_state, _field, uint8_t), \
}
--
yamahata
Re: [Qemu-devel] [PATCH 24/26] acpi, acpi_piix: factor out GPE logic
On 04/17/2011 04:50 PM, Isaku Yamahata wrote:
On Sun, Apr 17, 2011 at 04:17:51PM +0300, Avi Kivity wrote:
On 03/16/2011 11:29 AM, Isaku Yamahata wrote:
factor out ACPI GPE logic. Later it will be used by ICH9 ACPI.
I think this patch is causing qemu-kvm failures on migration:
(gdb) bt
#0 0x0049aff4 in qemu_put_be16s (f=0x1a74490, pv=0x2c02580,
size=2) at hw/hw.h:108
#1 put_uint16 (f=0x1a74490, pv=0x2c02580, size=2) at savevm.c:855
#2 0x0049c3e4 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0b00, opaque=0x1842ef0) at savevm.c:1436
#3 0x0049c3b6 in vmstate_save_state (f=0x1a74490,
vmsd=0x6f0aa0, opaque=0x1842b90) at savevm.c:1434
#4 0x0049c6f1 in vmstate_save (mon=value optimized out,
f=0x1a74490) at savevm.c:1459
#5 qemu_savevm_state_complete (mon=value optimized out, f=0x1a74490)
at savevm.c:1600
#6 0x0049455a in migrate_fd_put_ready (opaque=0x1847890) at
migration.c:383
#7 0x004ce2eb in qemu_run_timers (clock=value optimized out)
at qemu-timer.c:505
#8 0x004ce806 in qemu_run_all_timers () at qemu-timer.c:619
#9 0x00419463 in main_loop_wait (nonblocking=value optimized
out) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339
#10 0x00433927 in kvm_main_loop () at
/build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590
#11 0x0041a3a6 in main_loop (argc=value optimized out,
argv=value optimized out, envp=value optimized out)
at /build/home/tlv/akivity/qemu-kvm/vl.c:1369
#12 main (argc=value optimized out, argv=value optimized out,
envp=value optimized out) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257
The vmstate being migrated is gpe.
+#define VMSTATE_GPE_ARRAY(_field, _state)\
+ { \
+ .name = (stringify(_field)), \
+ .version_id = 0,\
+ .num= GPE_LEN, \
+ .info =vmstate_info_uint16, \
+ .size = sizeof(uint16_t), \
+ .flags = VMS_ARRAY | VMS_POINTER, \
+ .offset = vmstate_offset_pointer(_state, _field, uint8_t), \
+ }
+
static const VMStateDescription vmstate_gpe = {
.name = gpe,
.version_id = 1,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields = (VMStateField []) {
-VMSTATE_UINT16(sts, struct gpe_regs),
-VMSTATE_UINT16(en, struct gpe_regs),
+VMSTATE_GPE_ARRAY(sts, ACPIGPE),
+VMSTATE_GPE_ARRAY(en, ACPIGPE),
VMSTATE_END_OF_LIST()
}
};
I'm no vmstate expert, but this does look odd. Why both VMS_ARRAY and
VMS_POINTER? aren't we trying to save/restore a simple 16-bit value? Or
at least we did before this patch.
That's right. the difference is, the new member type became uint8_t*.
Does the following help?
Unforunately not. The core is very similar though:
#0 0x0049aff4 in qemu_put_be16s (f=0x2427490, pv=0x1ae7580,
size=2) at hw/hw.h:108
#1 put_uint16 (f=0x2427490, pv=0x1ae7580, size=2) at savevm.c:855
#2 0x0049c3e4 in vmstate_save_state (f=0x2427490,
vmsd=0x6f0b00, opaque=0x21f5ef0) at savevm.c:1436
#3 0x0049c3b6 in vmstate_save_state (f=0x2427490,
vmsd=0x6f0aa0, opaque=0x21f5b90) at savevm.c:1434
#4 0x0049c6f1 in vmstate_save (mon=value optimized out,
f=0x2427490) at savevm.c:1459
#5 qemu_savevm_state_complete (mon=value optimized out, f=0x2427490)
at savevm.c:1600
#6 0x0049455a in migrate_fd_put_ready (opaque=0x21fa8c0) at
migration.c:383
#7 0x004ce2eb in qemu_run_timers (clock=value optimized out)
at qemu-timer.c:505
#8 0x004ce806 in qemu_run_all_timers () at qemu-timer.c:619
#9 0x00419463 in main_loop_wait (nonblocking=value optimized
out) at /build/home/tlv/akivity/qemu-kvm/vl.c:1339
#10 0x00433927 in kvm_main_loop () at
/build/home/tlv/akivity/qemu-kvm/qemu-kvm.c:1590
#11 0x0041a3a6 in main_loop (argc=value optimized out,
argv=value optimized out, envp=value optimized out)
at /build/home/tlv/akivity/qemu-kvm/vl.c:1369
#12 main (argc=value optimized out, argv=value optimized out,
envp=value optimized out) at /build/home/tlv/akivity/qemu-kvm/vl.c:3257
--
error compiling committee.c: too many arguments to function
