PV-GRUB fix (actually MiniOS) when PV guest launched with vfb=[..] entry.
PATCH 1/1 PV-GRUB fix (actually MiniOS) when PV guest launched with vfb=[..] entry.
Hey Keir,
This patch fixes the conditions when a PV guest is launched using the PV-grub and if the 'vfb' entry is in the configuration file.
The patch has been tested with PV guests. I tried to test it with stubdomains but the stubdomains (before the patch and after the patch) hangs on:
[ 319.039038] Pid: 4450, comm: qemu-dm Not tainted 2.6.32.15-kms.nv.10 #3 X8DTN [ 319.046191] RIP: e030:[<ffffffff810093aa>] [<ffffffff810093aa>] hypercall_page+0x3aa/0x100b [ 319.054641] RSP: e02b:ffff88008a72bb30 EFLAGS: 00000202 [ 319.059991] RAX: 0000000000000000 RBX: ffff88008b79f658 RCX: ffffffff810093aa [ 319.067147] RDX: ffff88008a72bb64 RSI: 00000000deadbeef RDI: 00000000deadbeef [ 319.074303] RBP: ffff88008a72bb68 R08: ffff8800b9fa1f00 R09: 0000000000000000 [ 319.081459] R10: 00007fae7e9b2000 R11: 0000000000000202 R12: 0000000000000001 [ 319.088617] R13: 00000000000010fe R14: 0000000000000000 R15: 0000004a523a3b56 [ 319.095776] FS: 00007fae7fe1e700(0000) GS:ffff880004e06000(0000) knlGS:0000000000000000 [ 319.103882] CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b [ 319.109656] CR2: 0000000002a53830 CR3: 000000008a774000 CR4: 0000000000002660 [ 319.116815] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 319.123973] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 319.131133] Call Trace: [ 319.133645] [<ffffffff812b9d59>] ? xen_poll_irq_timeout+0x47/0x51 [ 319.139844] [<ffffffff812b9d73>] xen_poll_irq+0x10/0x12 [ 319.145196] [<ffffffff810100c4>] xen_spin_lock_slow+0xe5/0x193 [ 319.151142] [<ffffffff8101020b>] __xen_spin_lock+0x99/0xcd [ 319.156751] [<ffffffff8101025c>] xen_spin_lock+0xb/0xd [ 319.162017] [<ffffffff81481152>] _spin_lock+0xe/0x12 [ 319.167105] [<ffffffff812c2034>] mn_invl_range_start+0x30/0x118 [ 319.173140] [<ffffffff811075a3>] __mmu_notifier_invalidate_range_start+0x33/0x59 [ 319.180640] [<ffffffff810ef041>] apply_to_page_range+0x4f/0x338 [ 319.186674] [<ffffffff812c21cb>] ? find_grant_ptes+0x0/0x12e [ 319.192451] [<ffffffff81010229>] ? __xen_spin_lock+0xb7/0xcd [ 319.198229] [<ffffffff812c1f4a>] gntdev_mmap+0x130/0x1ea [ 319.203669] [<ffffffff810f5f91>] mmap_region+0x2a6/0x49f [ 319.209106] [<ffffffff811eebbc>] ? file_map_prot_check+0x9b/0xa7 [ 319.215225] [<ffffffff810f641a>] do_mmap_pgoff+0x290/0x2f6 [ 319.220835] [<ffffffff810e9e55>] sys_mmap_pgoff+0xf6/0x1b4 [ 319.226444] [<ffffffff81016fdc>] sys_mmap+0x22/0x24 [ 319.231443] [<ffffffff81012cb2>] system_call_fastpath+0x16/0x1b
Haven't tracked that one down :-(
Thought looking at the output of the stubdomain launch, it only initializes the blkfront, console, and netfront.
This patch touches the pcifront, fbfront and kbdfront so the patch should not impact the stubdomain functionality.
PATCH 1/1 mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1277908182 14400
# Node ID 76e7d0258f65e4ab1b105cd70a96f2a411d5ca22
# Parent a24dbfcbdf695f49867d5881ea20ab40f18aea98
mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init.
With the QEMU change 805ed3b20492d2f4bb465bfda65cedd286e23209 ("Wait for frontend state Connected before connecting the backend"), where QEMU backend (say VNC framebuffer) will ONLY call ops->connect (which when successfull, will move the backend state from XenbusStateInitWait to XenbusStateConnected) when the frontend state is in XenbusStateConnected.
Previous to that git commit it would call ops->connect also if the frontend was in XenbusStateInitialized state.
Without this patch, the MiniOS (in my case pvgrub) would hang in the fbfront and kbfront threads waiting for the backend to change its state from InitWait to Connected. Which the backend would not do as the ops->connect in QEMU was never called.
The c/s 21260 "mini-os: Revert 21106:b20f897d6010 "Fix xenbus initialisation" fixes this for the blkfront and netfront. Unfortunately
it did not fix it for the fbfront, kbdfront and pcifront which this patch does.
The patch fixes pv-grub hanging at:
"
******************* FBFRONT for device/vfb/0 **********
******************* KBDFRONT for device/vkbd/0 ********** " and makes it now go to:
"
******************* FBFRONT for device/vfb/0 **********
******************* KBDFRONT for device/vkbd/0 **********
backend at /local/domain/0/backend/vkbd/11/0
/local/domain/0/backend/vkbd/11/0 connected
************************** KBDFRONT
Thread "kbdfront" exited.
backend at /local/domain/0/backend/vfb/11/0
/local/domain/0/backend/vfb/11/0 connected
"
and you use VNC to see the GRUB menu.
extras/mini-os/fbfront.c +12 -2
@@ -124,7 +124,12 @@ }snprintf (path,sizeof (path),"%s/state" , nodename); - err =xenbus_switch_state (xbt, path, XenbusStateInitialised); +/* In the past we would have made this Initialized. But with a QEMU +* update 805ed3b that requires the frontend to be Connected state +* to progress the backend to move from InitWait to Connected. +* The QEMU bug fix was meant _only_ for XenFB but it affects every +* device. */ + err =xenbus_switch_state (xbt, path, XenbusStateConnected);if (err) {printk ("error writing initialized: %s\n" , err);free (err);
@@ -485,7 +490,12 @@ }snprintf (path,sizeof (path),"%s/state" , nodename); - err =xenbus_switch_state (xbt, path, XenbusStateInitialised); +/* In the past we would have made this Initialized. But with a QEMU +* update 805ed3b that requires the frontend to be Connected state +* to progress the backend to move from InitWait to Connected. +* The QEMU bug fix was meant _only_ for XenFB but it affects every +* device. */ + err =xenbus_switch_state (xbt, path, XenbusStateConnected);if (err) { message ="switching state" ; goto abort_transaction;
extras/mini-os/pcifront.c +6 -1
@@ -204,7 +204,12 @@ }snprintf (path,sizeof (path),"%s/state" , nodename); - err =xenbus_switch_state (xbt, path, XenbusStateInitialised); +/* In the past we would have made this Initialized. But with a QEMU +* update 805ed3b that requires the frontend to be Connected state +* to progress the backend to move from InitWait to Connected. +* The QEMU bug fix was meant _only_ for XenFB but it affects every +* device. */ + err =xenbus_switch_state (xbt, path, XenbusStateConnected);if (err) { message ="switching state" ; goto abort_transaction;
Re: [PATCH 1 of 1] mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init
Thank you for the patch Konrad.
I think this fix shows us that 805ed3b20492d2f4bb465bfda65cedd286e23209 was the wrong fix:
commit 805ed3b20492d2f4bb465bfda65cedd286e23209
Author: Ian Jackson <[email protected]> Date: Fri May 21 15:46:55 2010 +0100
Wait for frontend state Connected before connecting the backend
The frontend of the framebuffer set a value (request-abs-pointer) and go
to the state Connected. The backend must read this value only when the
frontend has the state Connected.
The problem was that the backend can be sure that the linux xenfb frontend wrote request-abs-pointer only after the frontend state is Connected.
In order to do that properly we need a new hook in qemu xen_backend: we should probably rename the current connect hook to initialise and create a new connect hook that would be implemented by xenfb to read request-abs-pointer.
Quoted 105 lines · Konrad Rzeszutek Wilk
# HG changeset patch
# User Konrad Rzeszutek Wilk <[email protected]>
# Date 1277908182 14400
# Node ID76e7d0258f65e4ab1b105cd70a96f2a411d5ca22
# Parenta24dbfcbdf695f49867d5881ea20ab40f18aea98
mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init.With the QEMU change
805ed3b20492d2f4bb465bfda65cedd286e23209("Wait for frontend state Connected before connecting the backend"), where QEMU backend (say VNC framebuffer) will ONLY callops->connect(which when successfull, will move the backend state from XenbusStateInitWait to XenbusStateConnected) when the frontend state is in XenbusStateConnected.Previous to that git commit it would call
ops->connectalso if the frontend was in XenbusStateInitialized state.Without this patch, the MiniOS (in my case pvgrub) would hang in the fbfront and kbfront threads waiting for the backend to change its state from InitWait to Connected. Which the backend would not do as the
ops->connectin QEMU was never called.The c/s 21260 "mini-os: Revert 21106:b20f897d6010 "Fix xenbus initialisation" fixes this for the blkfront and netfront. Unfortunately
it did not fix it for the fbfront, kbdfront and pcifront which this patch does.The patch fixes pv-grub hanging at:
"
******************* FBFRONT for device/vfb/0 ***************************** KBDFRONT for device/vkbd/0 ********** " and makes it now go to:
"
******************* FBFRONT for device/vfb/0 ***************************** KBDFRONT for device/vkbd/0 **********
backend at /local/domain/0/backend/vkbd/11/0
/local/domain/0/backend/vkbd/11/0 connected
************************** KBDFRONT
Thread "kbdfront" exited.
backend at /local/domain/0/backend/vfb/11/0
/local/domain/0/backend/vfb/11/0 connected
"
and you use VNC to see the GRUB menu.diff -r
a24dbfcbdf69-r76e7d0258f65extras/mini-os/fbfront.cextras/mini-os/fbfront.c +12
-2@@ -124,7 +124,12 @@ }snprintf (path,sizeof (path),"%s/state" , nodename); - err =xenbus_switch_state (xbt, path, XenbusStateInitialised); +/* In the past we would have made this Initialized. But with a QEMU +* update 805ed3b that requires the frontend to be Connected state +* to progress the backend to move from InitWait to Connected. +* The QEMU bug fix was meant _only_ for XenFB but it affects every +* device. */ + err =xenbus_switch_state (xbt, path, XenbusStateConnected);if (err) {printk ("error writing initialized: %s\n" , err);free (err);@@ -485,7 +490,12 @@ }snprintf (path,sizeof (path),"%s/state" , nodename); - err =xenbus_switch_state (xbt, path, XenbusStateInitialised); +/* In the past we would have made this Initialized. But with a QEMU +* update 805ed3b that requires the frontend to be Connected state +* to progress the backend to move from InitWait to Connected. +* The QEMU bug fix was meant _only_ for XenFB but it affects every +* device. */ + err =xenbus_switch_state (xbt, path, XenbusStateConnected);if (err) { message ="switching state" ; goto abort_transaction;diff -r
a24dbfcbdf69-r76e7d0258f65extras/mini-os/pcifront.cextras/mini-os/pcifront.c +6
-1@@ -204,7 +204,12 @@ }snprintf (path,sizeof (path),"%s/state" , nodename); - err =xenbus_switch_state (xbt, path, XenbusStateInitialised); +/* In the past we would have made this Initialized. But with a QEMU +* update 805ed3b that requires the frontend to be Connected state +* to progress the backend to move from InitWait to Connected. +* The QEMU bug fix was meant _only_ for XenFB but it affects every +* device. */ + err =xenbus_switch_state (xbt, path, XenbusStateConnected);if (err) { message ="switching state" ; goto abort_transaction;_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Re: [PATCH 1 of 1] mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init
Quoted 18 lines · Stefano Stabellini
Thank you for the patch Konrad.
I think this fix shows us that
805ed3b20492d2f4bb465bfda65cedd286e23209was the wrong fix:commit
805ed3b20492d2f4bb465bfda65cedd286e23209Author: Ian Jackson <[email protected]> Date: Fri May 21 15:46:55 2010 +0100Wait for frontend state Connected before connecting the backend
The frontend of the framebuffer set a value (request-abs-pointer) and go
to the state Connected. The backend must read this value only when the
frontend has the state Connected.
Quoted 7 lines
The problem was that the backend can be sure that the linux xenfb frontend wrote request-abs-pointer only after the frontend state is Connected.
In order to do that properly we need a new hook in qemuxen_backend: we should probably rename the current connect hook to initialise and create a new connect hook that would be implemented by xenfb to read request-abs-pointer.
Uhh. How about a compromise. Lets put this hac^H^H^Hpatch in, and when the QEMU fix is ready, yank this out and also the c/s 21260:
"mini-os: Revert 21106:b20f897d6010 "Fix xenbus initialisation"
Jeremy Fitzhardinge ([email protected]) reports that this fixes HVM+stubdom."
which was fixing the same exact problem I did, but on the stubdomain side?
Re: [PATCH 1 of 1] mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init
Quoted 30 lines · Konrad Rzeszutek Wilk
Quoted 18 lines · Stefano Stabellini
Thank you for the patch Konrad.
I think this fix shows us that
805ed3b20492d2f4bb465bfda65cedd286e23209was the wrong fix:commit
805ed3b20492d2f4bb465bfda65cedd286e23209Author: Ian Jackson <[email protected]> Date: Fri May 21 15:46:55 2010 +0100Wait for frontend state Connected before connecting the backend
The frontend of the framebuffer set a value (request-abs-pointer) and go
to the state Connected. The backend must read this value only when the
frontend has the state Connected.Quoted 7 lines
The problem was that the backend can be sure that the linux xenfb frontend wrote request-abs-pointer only after the frontend state is Connected.
In order to do that properly we need a new hook in qemuxen_backend: we should probably rename the current connect hook to initialise and create a new connect hook that would be implemented by xenfb to read request-abs-pointer.Uhh. How about a compromise. Lets put this hac^H^H^Hpatch in, and when the QEMU fix is ready, yank this out and also the c/s 21260:
Quoted 8 lines
"mini-os: Revert 21106:b20f897d6010 "Fix xenbus initialisation"
Jeremy Fitzhardinge ([email protected]) reports that this fixes HVM+stubdom."
which was fixing the same exact problem I did, but on the stubdomain side?
What you suggested, or we just revert 805ed3b20492d2f4bb465bfda65cedd286e23209 right now and wait for a proper fix for that.
Ian, what do you think?
Re: [PATCH 1 of 1] mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 1 of 1] mini-os: PV fronted MUST be in XenbusStateConnected not XenbusStateInitialized during init"):
Quoted 3 lines
What you suggested, or we just revert
805ed3b20492d2f4bb465bfda65cedd286e23209right now and wait for a proper fix for that.
I think we should revert 805ed3.
Ian.
Quoted 4 lines · Konrad Rzeszutek Wilk
Hey Keir,
This patch fixes the conditions when a PV guest is launched using the PV-grub and if the 'vfb' entry is in the configuration file.
Stefano deals with minios and stubdom patches now (cc'ed him).
-- Keir
Quoted 57 lines
The patch has been tested with PV guests. I tried to test it with stubdomains but the stubdomains (before the patch and after the patch) hangs on:
[ 319.039038] Pid: 4450, comm: qemu-dm Not tainted 2.6.32.15-kms.nv.10 #3 X8DTN [ 319.046191] RIP: e030:[<ffffffff810093aa>] [<ffffffff810093aa>] hypercall_page+0x3aa/0x100b [ 319.054641] RSP: e02b:ffff88008a72bb30 EFLAGS: 00000202 [ 319.059991] RAX: 0000000000000000 RBX: ffff88008b79f658 RCX: ffffffff810093aa [ 319.067147] RDX: ffff88008a72bb64 RSI: 00000000deadbeef RDI: 00000000deadbeef [ 319.074303] RBP: ffff88008a72bb68 R08: ffff8800b9fa1f00 R09: 0000000000000000 [ 319.081459] R10: 00007fae7e9b2000 R11: 0000000000000202 R12: 0000000000000001 [ 319.088617] R13: 00000000000010fe R14: 0000000000000000 R15: 0000004a523a3b56 [ 319.095776] FS: 00007fae7fe1e700(0000) GS:ffff880004e06000(0000) knlGS:0000000000000000 [ 319.103882] CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b [ 319.109656] CR2: 0000000002a53830 CR3: 000000008a774000 CR4: 0000000000002660 [ 319.116815] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 319.123973] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 319.131133] Call Trace: [ 319.133645] [<ffffffff812b9d59>] ? xen_poll_irq_timeout+0x47/0x51 [ 319.139844] [<ffffffff812b9d73>] xen_poll_irq+0x10/0x12 [ 319.145196] [<ffffffff810100c4>] xen_spin_lock_slow+0xe5/0x193 [ 319.151142] [<ffffffff8101020b>] __xen_spin_lock+0x99/0xcd [ 319.156751] [<ffffffff8101025c>] xen_spin_lock+0xb/0xd [ 319.162017] [<ffffffff81481152>] _spin_lock+0xe/0x12 [ 319.167105] [<ffffffff812c2034>] mn_invl_range_start+0x30/0x118 [ 319.173140] [<ffffffff811075a3>] __mmu_notifier_invalidate_range_start+0x33/0x59 [ 319.180640] [<ffffffff810ef041>] apply_to_page_range+0x4f/0x338 [ 319.186674] [<ffffffff812c21cb>] ? find_grant_ptes+0x0/0x12e [ 319.192451] [<ffffffff81010229>] ? __xen_spin_lock+0xb7/0xcd [ 319.198229] [<ffffffff812c1f4a>] gntdev_mmap+0x130/0x1ea [ 319.203669] [<ffffffff810f5f91>] mmap_region+0x2a6/0x49f [ 319.209106] [<ffffffff811eebbc>] ? file_map_prot_check+0x9b/0xa7 [ 319.215225] [<ffffffff810f641a>] do_mmap_pgoff+0x290/0x2f6 [ 319.220835] [<ffffffff810e9e55>] sys_mmap_pgoff+0xf6/0x1b4 [ 319.226444] [<ffffffff81016fdc>] sys_mmap+0x22/0x24 [ 319.231443] [<ffffffff81012cb2>] system_call_fastpath+0x16/0x1bHaven't tracked that one down :-(
Thought looking at the output of the stubdomain launch, it only initializes the blkfront, console, and netfront.
This patch touches the pcifront, fbfront and kbdfront so the patch should not impact the stubdomain functionality.
Quoted 42 lines · Konrad Rzeszutek Wilk
Hey Keir,
This patch fixes the conditions when a PV guest is launched using the PV-grub and if the 'vfb' entry is in the configuration file.
The patch has been tested with PV guests. I tried to test it with stubdomains but the stubdomains (before the patch and after the patch) hangs on:
[ 319.039038] Pid: 4450, comm: qemu-dm Not tainted 2.6.32.15-kms.nv.10 #3 X8DTN [ 319.046191] RIP: e030:[<ffffffff810093aa>] [<ffffffff810093aa>] hypercall_page+0x3aa/0x100b [ 319.054641] RSP: e02b:ffff88008a72bb30 EFLAGS: 00000202 [ 319.059991] RAX: 0000000000000000 RBX: ffff88008b79f658 RCX: ffffffff810093aa [ 319.067147] RDX: ffff88008a72bb64 RSI: 00000000deadbeef RDI: 00000000deadbeef [ 319.074303] RBP: ffff88008a72bb68 R08: ffff8800b9fa1f00 R09: 0000000000000000 [ 319.081459] R10: 00007fae7e9b2000 R11: 0000000000000202 R12: 0000000000000001 [ 319.088617] R13: 00000000000010fe R14: 0000000000000000 R15: 0000004a523a3b56 [ 319.095776] FS: 00007fae7fe1e700(0000) GS:ffff880004e06000(0000) knlGS:0000000000000000 [ 319.103882] CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b [ 319.109656] CR2: 0000000002a53830 CR3: 000000008a774000 CR4: 0000000000002660 [ 319.116815] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 319.123973] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 319.131133] Call Trace: [ 319.133645] [<ffffffff812b9d59>] ? xen_poll_irq_timeout+0x47/0x51 [ 319.139844] [<ffffffff812b9d73>] xen_poll_irq+0x10/0x12 [ 319.145196] [<ffffffff810100c4>] xen_spin_lock_slow+0xe5/0x193 [ 319.151142] [<ffffffff8101020b>] __xen_spin_lock+0x99/0xcd [ 319.156751] [<ffffffff8101025c>] xen_spin_lock+0xb/0xd [ 319.162017] [<ffffffff81481152>] _spin_lock+0xe/0x12 [ 319.167105] [<ffffffff812c2034>] mn_invl_range_start+0x30/0x118 [ 319.173140] [<ffffffff811075a3>] __mmu_notifier_invalidate_range_start+0x33/0x59 [ 319.180640] [<ffffffff810ef041>] apply_to_page_range+0x4f/0x338 [ 319.186674] [<ffffffff812c21cb>] ? find_grant_ptes+0x0/0x12e [ 319.192451] [<ffffffff81010229>] ? __xen_spin_lock+0xb7/0xcd [ 319.198229] [<ffffffff812c1f4a>] gntdev_mmap+0x130/0x1ea [ 319.203669] [<ffffffff810f5f91>] mmap_region+0x2a6/0x49f [ 319.209106] [<ffffffff811eebbc>] ? file_map_prot_check+0x9b/0xa7 [ 319.215225] [<ffffffff810f641a>] do_mmap_pgoff+0x290/0x2f6 [ 319.220835] [<ffffffff810e9e55>] sys_mmap_pgoff+0xf6/0x1b4 [ 319.226444] [<ffffffff81016fdc>] sys_mmap+0x22/0x24 [ 319.231443] [<ffffffff81012cb2>] system_call_fastpath+0x16/0x1b
Quoted 7 lines
Haven't tracked that one down :-(
Thought looking at the output of the stubdomain launch, it only initializes the blkfront, console, and netfront.
This patch touches the pcifront, fbfront and kbdfront so the patch should not impact the stubdomain functionality.
The patch is OK but I would rather fix the qemu backend properly than modify all the possible frontends.
Regarding qemu stubdoms, this is a known problem:
http://old.nabble.com/-PATCH-1-of-1--stubdom-hanging-at-creation-td29013845.html