Hi, Here is a little series that attempt to fix the issue regarding xen_platform_pci=0 not been handled. There are two patches, the first one adds an option to specifies the QEMU machine that a user wants and we handle the xen_platform_pci=0 case using the new option. The new options "qemu_machine_override" will help if one want to try a q35 based device model. Otherwise, it will be used by libxl to switch to the "pc" machine instead of the "xenfv" one when necessary, as the only difference between both (since QEMU 1.6) is the presence of the xen-platform pci device. Regards, Anthony PERARD (2): libxl: adding support to use -machine option of QEMU. libxl: Handle xen_platform_pci=0 case with qemu-xen. tools/libxl/libxl_dm.c | 31 +++++++++++++++++++++++++++++-- tools/libxl/libxl_types.idl | 1 + tools/libxl/xl_cmdimpl.c | 3 +++ 3 files changed, 33 insertions(+), 2 deletions(-) -- Anthony PERARD
Anthony PERARD
2013-Nov-22 15:13 UTC
[PATCH 1/2] libxl: adding support to use -machine option of QEMU.
It adds a new config option, "qemu_machine_override".
This can be used in the future to switch to a Q35 based device model. It
can also be used on a recent QEMU to avoid adding the xen-platform
device used to setup PV drivers in the guest.
Two possible values for now are "pc" or "xenfv" but there
are
equivalents and xenfv is the default. A possible future use could be
qemu_machine_override="q35" when this machine will be able to start
successfully on Xen.
When passed to qemu, libxl automatically adds ",accel=xen" to the
machine option.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_dm.c | 7 +++++--
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 3 +++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 292e351..3d4a913 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -608,7 +608,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc
*gc,
}
for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
flexarray_append(dm_args, b_info->extra[i]);
- flexarray_append(dm_args, "-M");
+ flexarray_append(dm_args, "-machine");
switch (b_info->type) {
case LIBXL_DOMAIN_TYPE_PV:
flexarray_append(dm_args, "xenpv");
@@ -616,7 +616,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc
*gc,
flexarray_append(dm_args, b_info->extra_pv[i]);
break;
case LIBXL_DOMAIN_TYPE_HVM:
- flexarray_append(dm_args, "xenfv");
+ if (b_info->qemu_machine)
+ flexarray_append(dm_args, GCSPRINTF("%s,accel=xen",
b_info->qemu_machine));
+ else
+ flexarray_append(dm_args, "xenfv");
for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] !=
NULL; i++)
flexarray_append(dm_args, b_info->extra_hvm[i]);
break;
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index cba8eff..4141501 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -317,6 +317,7 @@ libxl_domain_build_info =
Struct("domain_build_info",[
# if you set device_model you must set device_model_version too
("device_model", string),
("device_model_ssidref", uint32),
+ ("qemu_machine", string),
# extra parameters pass directly to qemu, NULL terminated
("extra", libxl_string_list),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 341863e..4de6858 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1491,6 +1491,9 @@ skip_vfb:
}
+ xlu_cfg_replace_string (config, "qemu_machine_override",
+ &b_info->qemu_machine, 0);
+
xlu_cfg_replace_string (config, "device_model_override",
&b_info->device_model, 0);
if (!xlu_cfg_get_string (config, "device_model_version",
&buf, 0)) {
--
Anthony PERARD
Anthony PERARD
2013-Nov-22 15:13 UTC
[PATCH 2/2] libxl: Handle xen_platform_pci=0 case with qemu-xen.
This should result in QEMU *not* adding the xen-platform device.
Since QEMU 1.6, this can be achieved by using a different qemu machine.
The one used by libxl is "xenfv", but using QEMU >=1.6 with
"-machine
pc,accel=xen" works as well with only one difference compared to
"xenfv", there is no xen-platform device.
One more things, if "qemu_machine_override" is set by the user, then
we
check if it''s necessary to add the xen-platform device.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_dm.c | 28 ++++++++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 3d4a913..33caa2b 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -390,6 +390,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc
*gc,
const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config);
const libxl_sdl_info *sdl = dm_sdl(guest_config);
const char *keymap = dm_keymap(guest_config);
+ const char *machine = b_info->qemu_machine;
flexarray_t *dm_args;
int i;
uint64_t ram_size;
@@ -608,6 +609,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc
*gc,
}
for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
flexarray_append(dm_args, b_info->extra[i]);
+
+ if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
+ if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) {
+ if (b_info->qemu_machine) {
+ /* Check the qemu machine asked for, it can be
"xenfv" which
+ * will add xen-platform, or it can be "pc" or
"pc-*" which
+ * in this case will need to add the device here. Anything
+ * else is either a mistake or a machine not supported by
+ * xen. */
+ if (!strncmp("pc", b_info->qemu_machine, 2)) {
+ flexarray_vappend(dm_args, "-device",
"xen-platform");
+ }
+ }
+ } else {
+ /* Switching here to the machine "pc" which does not add
+ * the xen-platform device instead of the default "xenfv"
machine.
+ */
+ if (!b_info->qemu_machine) {
+ machine = "pc";
+ }
+ }
+ }
+
flexarray_append(dm_args, "-machine");
switch (b_info->type) {
case LIBXL_DOMAIN_TYPE_PV:
@@ -616,8 +640,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc
*gc,
flexarray_append(dm_args, b_info->extra_pv[i]);
break;
case LIBXL_DOMAIN_TYPE_HVM:
- if (b_info->qemu_machine)
- flexarray_append(dm_args, GCSPRINTF("%s,accel=xen",
b_info->qemu_machine));
+ if (machine)
+ flexarray_append(dm_args, GCSPRINTF("%s,accel=xen",
machine));
else
flexarray_append(dm_args, "xenfv");
for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] !=
NULL; i++)
--
Anthony PERARD
xen@bugs.xenproject.org
2013-Nov-22 15:30 UTC
Processed: Re: [PATCH 0/2] Handle xen_platform_pci=0 case
Processing commands for xen@bugs.xenproject.org:> graft 20 ^Graft `<1385133191-23033-1-git-send-email-anthony.perard@citrix.com>'' onto #20> thanksFinished processing. Modified/created Bugs: - 20: http://bugs.xenproject.org/xen/bug/20 --- Xen Hypervisor Bug Tracker See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on reporting bugs Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues
Il 22/11/2013 16:13, Anthony PERARD ha scritto:> Hi, > > Here is a little series that attempt to fix the issue regarding > xen_platform_pci=0 not been handled. > > There are two patches, the first one adds an option to specifies the QEMU > machine that a user wants and we handle the xen_platform_pci=0 case using the > new option. > > The new options "qemu_machine_override" will help if one want to try a q35 > based device model. Otherwise, it will be used by libxl to switch to the "pc" > machine instead of the "xenfv" one when necessary, as the only difference > between both (since QEMU 1.6) is the presence of the xen-platform pci device. > > Regards, > > Anthony PERARD (2): > libxl: adding support to use -machine option of QEMU. > libxl: Handle xen_platform_pci=0 case with qemu-xen. > > tools/libxl/libxl_dm.c | 31 +++++++++++++++++++++++++++++-- > tools/libxl/libxl_types.idl | 1 + > tools/libxl/xl_cmdimpl.c | 3 +++ > 3 files changed, 33 insertions(+), 2 deletions(-) >Tested with upstream qemu 1.6 from staging: vi Config.mk QEMU_UPSTREAM_URL ?= git://xenbits.xen.org/staging/qemu-upstream-unstable.git QEMU_UPSTREAM_REVISION ?= master With xen_platform_pci=0 Fedora19 hvm domU gave me kernel panic on boot. xl create -vvv, xl dmesg and serial domUs boot (with calltrace) logs on attachments. With xen_platform_pci=1 boot correctly. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
On Fri, 2013-11-22 at 15:13 +0000, Anthony PERARD wrote:> There are two patches, the first one adds an option to specifies the QEMU > machine that a user wants and we handle the xen_platform_pci=0 case using the > new option.I''m thinking we should make this new option an enum of actually supported machine types, rather than just a free reign. Makes more sense from a "what do we support" PoV.> > The new options "qemu_machine_override" will help if one want to try a q35 > based device model. Otherwise, it will be used by libxl to switch to the "pc" > machine instead of the "xenfv" one when necessary, as the only difference > between both (since QEMU 1.6) is the presence of the xen-platform pci device. > > Regards, > > Anthony PERARD (2): > libxl: adding support to use -machine option of QEMU. > libxl: Handle xen_platform_pci=0 case with qemu-xen. > > tools/libxl/libxl_dm.c | 31 +++++++++++++++++++++++++++++-- > tools/libxl/libxl_types.idl | 1 + > tools/libxl/xl_cmdimpl.c | 3 +++ > 3 files changed, 33 insertions(+), 2 deletions(-) >
On Fri, Nov 22, 2013 at 04:49:06PM +0100, Fabio Fantoni wrote:> Il 22/11/2013 16:13, Anthony PERARD ha scritto: > >Hi, > > > >Here is a little series that attempt to fix the issue regarding > >xen_platform_pci=0 not been handled. > > > >There are two patches, the first one adds an option to specifies the QEMU > >machine that a user wants and we handle the xen_platform_pci=0 case using the > >new option. > > > >The new options "qemu_machine_override" will help if one want to try a q35 > >based device model. Otherwise, it will be used by libxl to switch to the "pc" > >machine instead of the "xenfv" one when necessary, as the only difference > >between both (since QEMU 1.6) is the presence of the xen-platform pci device. > > > >Regards, > > > >Anthony PERARD (2): > > libxl: adding support to use -machine option of QEMU. > > libxl: Handle xen_platform_pci=0 case with qemu-xen. > > > > tools/libxl/libxl_dm.c | 31 +++++++++++++++++++++++++++++-- > > tools/libxl/libxl_types.idl | 1 + > > tools/libxl/xl_cmdimpl.c | 3 +++ > > 3 files changed, 33 insertions(+), 2 deletions(-) > > > > Tested with upstream qemu 1.6 from staging: > vi Config.mk > QEMU_UPSTREAM_URL ?> git://xenbits.xen.org/staging/qemu-upstream-unstable.git > QEMU_UPSTREAM_REVISION ?= master > > With xen_platform_pci=0 Fedora19 hvm domU gave me kernel panic on boot. > xl create -vvv, xl dmesg and serial domUs boot (with calltrace) logs on > attachments. > > With xen_platform_pci=1 boot correctly. > >> [ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-3.11.6-200.fc19.x86_64 root=/d > ev/mapper/fedora-root ro rd.lvm.lv=fedora/swap rd.md=0 rd.dm=0 rd.luks=0 vconso > le.font=latarcyrheb-sun16 rd.lvm.lv=fedora/root vconsole.keymap=it2 rhgb debug > console=hvc0 console=ttyS0 LANG=it_IT.UTF-8[...]> [ 1.575205] scsi 0:0:0:0: Direct-Access ATA QEMU HARDDISK 1.6. P > Q: 0 ANSI: 5 > [ 1.575328] sd 0:0:0:0: Attached scsi generic sg0 type 0 > [ 1.575381] sd 0:0:0:0: [sda] 20480000 512-byte logical blocks: (10.4 GB/9.7 > 6 GiB) > [ 1.575427] sd 0:0:0:0: [sda] Write Protect is off > [ 1.575429] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 > [ 1.575445] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doe > sn''t support DPO or FUA > [ 1.661471] sda: sda1 sda2 > [ 1.665373] ------------[ cut here ]------------ > [ 1.665704] sd 0:0:0:0: [sda] Attached SCSI disk > [ 1.666342] kernel BUG at drivers/xen/grant-table.c:1189! > [ 1.666342] invalid opcode: 0000 [#1] SMP > [ 1.666342] Modules linked in: > [ 1.666342] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.11.6-200.fc19.x86_64 > #1 > [ 1.666342] Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/22/2013 > [ 1.666342] task: ffff88007a7b0000 ti: ffff88007a7b8000 task.ti: ffff88007a7 > b8000 > [ 1.666342] RIP: 0010:[<ffffffff813a0c0f>] [<ffffffff813a0c0f>] get_free_en > tries+0x2cf/0x2e0 > [ 1.666342] RSP: 0000:ffff88007a7b9bd8 EFLAGS: 00010046 > [ 1.666342] RAX: 0000000000000286 RBX: 0000000000000001 RCX: 000000000000000 > 0 > [ 1.666342] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff81fc7d7 > 0 > [ 1.666342] RBP: ffff88007a7b9c20 R08: 0000000000000000 R09: 000000000000fff > c > [ 1.666342] R10: 0000000000000000 R11: 0000000000000000 R12: 000000000000028 > 6 > [ 1.666342] R13: 0000000000037032 R14: 0000000000000000 R15: 000000000000000 > 0 > [ 1.666342] FS: 0000000000000000(0000) GS:ffff88007b600000(0000) knlGS:0000 > 000000000000 > [ 1.666342] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > [ 1.666342] CR2: 0000000000000000 CR3: 0000000001c0c000 CR4: 00000000000006f > 0 > [ 1.666342] Stack: > [ 1.666342] ffff8800370174b0 ffff880037017408 ffff88007a7b9c20 00000001814b > 68b6 > [ 1.666342] ffff880037024600 0000000000000000 0000000000037032 000000000000 > 0000 > [ 1.666342] 0000000000000000 ffff88007a7b9c50 ffffffff813a0c43 ffff88003702 > 4600 > [ 1.666342] Call Trace: > [ 1.666342] [<ffffffff813a0c43>] gnttab_grant_foreign_access+0x23/0x60 > [ 1.666342] [<ffffffff814c7e58>] xenkbd_connect_backend+0x58/0x2c0 > [ 1.666342] [<ffffffff814c848a>] xenkbd_probe+0x2fa/0x340 > [ 1.666342] [<ffffffff813a856e>] xenbus_dev_probe+0x8e/0x170 > [ 1.666342] [<ffffffff813a9ba8>] xenbus_frontend_dev_probe+0x48/0x50 > [ 1.666342] [<ffffffff813f21a7>] driver_probe_device+0x87/0x390 > [ 1.666342] [<ffffffff813f2583>] __driver_attach+0x93/0xa0 > [ 1.666342] [<ffffffff813f24f0>] ? __device_attach+0x40/0x40 > [ 1.666342] [<ffffffff813f00e3>] bus_for_each_dev+0x63/0xa0 > [ 1.666342] [<ffffffff813f1bfe>] driver_attach+0x1e/0x20 > [ 1.666342] [<ffffffff813f1798>] bus_add_driver+0x1e8/0x2a0 > [ 1.666342] [<ffffffff81d59578>] ? lifebook_module_init+0x1b/0x1b > [ 1.666342] [<ffffffff813f2bc4>] driver_register+0x74/0x150 > [ 1.666342] [<ffffffff81d59578>] ? lifebook_module_init+0x1b/0x1b > [ 1.666342] [<ffffffff813a7daa>] xenbus_register_driver_common+0x1a/0x20 > [ 1.666342] [<ffffffff813aa078>] xenbus_register_frontend+0x28/0x50 > [ 1.666342] [<ffffffff81d595a3>] xenkbd_init+0x2b/0x34 > [ 1.666342] [<ffffffff810020fa>] do_one_initcall+0xfa/0x1b0 > [ 1.666342] [<ffffffff81086785>] ? parse_args+0x225/0x400 > [ 1.666342] [<ffffffff81d0f078>] kernel_init_freeable+0x177/0x1ff > [ 1.666342] [<ffffffff81d0e898>] ? do_early_param+0x88/0x88 > [ 1.666342] [<ffffffff8163df40>] ? rest_init+0x80/0x80 > [ 1.666342] [<ffffffff8163df4e>] kernel_init+0xe/0x190 > [ 1.666342] [<ffffffff81656dec>] ret_from_fork+0x7c/0xb0 > [ 1.666342] [<ffffffff8163df40>] ? rest_init+0x80/0x80 > [ 1.666342] Code: 8b 05 9e 71 c2 00 44 8b 2d 83 71 c2 00 e9 62 fe ff ff 66 2 > e 0f 1f 84 00 00 00 00 00 b8 e4 ff ff ff e9 2a ff ff ff 44 89 c7 eb 84 <0f> 0b > 0f 0b 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 > [ 1.666342] RIP [<ffffffff813a0c0f>] get_free_entries+0x2cf/0x2e0 > [ 1.666342] RSP <ffff88007a7b9bd8> > [ 1.666342] ---[ end trace 6eb44b05748c7d42 ]--- > [ 2.122137] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0 > 000000bLooks like I have a similaire issue with the module xen_kbdfront, but later in the boot, an the guest is running fine. I think that could be an hidden bug which did not show up before, I''ll try to understand the issue. Thank you for testing. -- Anthony PERARD
On 22/11/13 15:13, Anthony PERARD wrote:> Hi, > > Here is a little series that attempt to fix the issue regarding > xen_platform_pci=0 not been handled. > > There are two patches, the first one adds an option to specifies the QEMU > machine that a user wants and we handle the xen_platform_pci=0 case using the > new option. > > The new options "qemu_machine_override" will help if one want to try a q35 > based device model. Otherwise, it will be used by libxl to switch to the "pc" > machine instead of the "xenfv" one when necessary, as the only difference > between both (since QEMU 1.6) is the presence of the xen-platform pci device. > > Regards, > > Anthony PERARD (2): > libxl: adding support to use -machine option of QEMU. > libxl: Handle xen_platform_pci=0 case with qemu-xen.As I understand it, this is a fix to functionality which should be supported, so doesn''t need a freeze exception at this point. -George
On Fri, Nov 22, 2013 at 03:56:13PM +0000, Ian Campbell wrote:> On Fri, 2013-11-22 at 15:13 +0000, Anthony PERARD wrote: > > There are two patches, the first one adds an option to specifies the QEMU > > machine that a user wants and we handle the xen_platform_pci=0 case using the > > new option. > > I''m thinking we should make this new option an enum of actually > supported machine types, rather than just a free reign. Makes more sense > from a "what do we support" PoV.And start with the following list ? pc pc-i440fx-1.7 pc-i440fx-1.6 pc-i440fx-1.5 pc-i440fx-1.4 pc-1.3 pc-1.2 pc-1.1 pc-1.0 pc-0.15 pc-0.14 pc-0.13 pc-0.12 pc-0.11 pc-0.10 xenfv I wonder how many of this will work. I suspect most of them. The only down side is that it will be necessary to patch libxl if one want to try, let say q35. If we only keep xenfv and pc in the list, it might be better to just not expose the option to the config file, for now. But yes, having a list for the supported feature point of view will be better. -- Anthony PERARD
On Fri, 2013-11-22 at 17:18 +0000, Anthony PERARD wrote:> On Fri, Nov 22, 2013 at 03:56:13PM +0000, Ian Campbell wrote: > > On Fri, 2013-11-22 at 15:13 +0000, Anthony PERARD wrote: > > > There are two patches, the first one adds an option to specifies the QEMU > > > machine that a user wants and we handle the xen_platform_pci=0 case using the > > > new option. > > > > I''m thinking we should make this new option an enum of actually > > supported machine types, rather than just a free reign. Makes more sense > > from a "what do we support" PoV. > > And start with the following list ?Uh, that''s quite long.> pc > pc-i440fx-1.7 > pc-i440fx-1.6 > pc-i440fx-1.5 > pc-i440fx-1.4 > pc-1.3 > pc-1.2 > pc-1.1 > pc-1.0 > pc-0.15 > pc-0.14 > pc-0.13 > pc-0.12 > pc-0.11 > pc-0.10 > xenfv > > I wonder how many of this will work. I suspect most of them.Work != Warantied by Xen.org (whatever that means)> The only > down side is that it will be necessary to patch libxl if one want to > try, let say q35. > > If we only keep xenfv and pc in the list, it might be better to just not > expose the option to the config file, for now. > > But yes, having a list for the supported feature point of view will be > better.I can just about see having a non "_override" enum *and* and _override string. But ick. Ian.
On 22/11/13 17:18, Anthony PERARD wrote:> On Fri, Nov 22, 2013 at 03:56:13PM +0000, Ian Campbell wrote: >> On Fri, 2013-11-22 at 15:13 +0000, Anthony PERARD wrote: >>> There are two patches, the first one adds an option to specifies the QEMU >>> machine that a user wants and we handle the xen_platform_pci=0 case using the >>> new option. >> I''m thinking we should make this new option an enum of actually >> supported machine types, rather than just a free reign. Makes more sense >> from a "what do we support" PoV. > And start with the following list ? > pc > pc-i440fx-1.7 > pc-i440fx-1.6 > pc-i440fx-1.5 > pc-i440fx-1.4 > pc-1.3 > pc-1.2 > pc-1.1 > pc-1.0 > pc-0.15 > pc-0.14 > pc-0.13 > pc-0.12 > pc-0.11 > pc-0.10 > xenfv > > I wonder how many of this will work. I suspect most of them. The only > down side is that it will be necessary to patch libxl if one want to > try, let say q35.In a way I think that''s the point -- by allowing the user to specify an arbitrary sting, we are implicitly supporting a gigantic range of things which may work in one release or in one particular configuration and then break in a different one. By making an enum, we bake in the exact ones which we consider to be valid. Also, we''ve been trying to get away from exposing a bunch of qemu-isms to the user via libxl and the xl config file. What might the *user* be trying to do in specifying this option? We should make sure the interface specifies things the user actually wants, and then use qemu''s options to implement that, rather than asking the user to specify qemu stuff directly. -George
On Fri, 2013-11-22 at 17:23 +0000, Ian Campbell wrote:> I can just about see having a non "_override" enum *and* and _override > string. But ick.Maybe the string override can be via device_model_args? Ian.
On Fri, Nov 22, 2013 at 05:31:19PM +0000, Ian Campbell wrote:> On Fri, 2013-11-22 at 17:23 +0000, Ian Campbell wrote: > > > I can just about see having a non "_override" enum *and* and _override > > string. But ick. > > Maybe the string override can be via device_model_args?Looks like it is possible to have an extra -machine in device_model_args_hvm, and qemu will use the last one. So it is probably best to not expose the qemu_machine at all. -- Anthony PERARD
Il 22/11/2013 17:54, Anthony PERARD ha scritto:> On Fri, Nov 22, 2013 at 04:49:06PM +0100, Fabio Fantoni wrote: >> Il 22/11/2013 16:13, Anthony PERARD ha scritto: >>> Hi, >>> >>> Here is a little series that attempt to fix the issue regarding >>> xen_platform_pci=0 not been handled. >>> >>> There are two patches, the first one adds an option to specifies the QEMU >>> machine that a user wants and we handle the xen_platform_pci=0 case using the >>> new option. >>> >>> The new options "qemu_machine_override" will help if one want to try a q35 >>> based device model. Otherwise, it will be used by libxl to switch to the "pc" >>> machine instead of the "xenfv" one when necessary, as the only difference >>> between both (since QEMU 1.6) is the presence of the xen-platform pci device.About q35 in theory should be missing only the implementation in hvmloader, is there a draft somewhere to try or to should be made?>>> >>> Regards, >>> >>> Anthony PERARD (2): >>> libxl: adding support to use -machine option of QEMU. >>> libxl: Handle xen_platform_pci=0 case with qemu-xen. >>> >>> tools/libxl/libxl_dm.c | 31 +++++++++++++++++++++++++++++-- >>> tools/libxl/libxl_types.idl | 1 + >>> tools/libxl/xl_cmdimpl.c | 3 +++ >>> 3 files changed, 33 insertions(+), 2 deletions(-) >>> >> Tested with upstream qemu 1.6 from staging: >> vi Config.mk >> QEMU_UPSTREAM_URL ?>> git://xenbits.xen.org/staging/qemu-upstream-unstable.git >> QEMU_UPSTREAM_REVISION ?= master >> >> With xen_platform_pci=0 Fedora19 hvm domU gave me kernel panic on boot. >> xl create -vvv, xl dmesg and serial domUs boot (with calltrace) logs on >> attachments. >> >> With xen_platform_pci=1 boot correctly. >> >> > >> [ 0.000000] Command line: BOOT_IMAGE=/vmlinuz-3.11.6-200.fc19.x86_64 root=/d >> ev/mapper/fedora-root ro rd.lvm.lv=fedora/swap rd.md=0 rd.dm=0 rd.luks=0 vconso >> le.font=latarcyrheb-sun16 rd.lvm.lv=fedora/root vconsole.keymap=it2 rhgb debug >> console=hvc0 console=ttyS0 LANG=it_IT.UTF-8 > [...] >> [ 1.575205] scsi 0:0:0:0: Direct-Access ATA QEMU HARDDISK 1.6. P >> Q: 0 ANSI: 5 >> [ 1.575328] sd 0:0:0:0: Attached scsi generic sg0 type 0 >> [ 1.575381] sd 0:0:0:0: [sda] 20480000 512-byte logical blocks: (10.4 GB/9.7 >> 6 GiB) >> [ 1.575427] sd 0:0:0:0: [sda] Write Protect is off >> [ 1.575429] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00 >> [ 1.575445] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doe >> sn''t support DPO or FUA >> [ 1.661471] sda: sda1 sda2 >> [ 1.665373] ------------[ cut here ]------------ >> [ 1.665704] sd 0:0:0:0: [sda] Attached SCSI disk >> [ 1.666342] kernel BUG at drivers/xen/grant-table.c:1189! >> [ 1.666342] invalid opcode: 0000 [#1] SMP >> [ 1.666342] Modules linked in: >> [ 1.666342] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.11.6-200.fc19.x86_64 >> #1 >> [ 1.666342] Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/22/2013 >> [ 1.666342] task: ffff88007a7b0000 ti: ffff88007a7b8000 task.ti: ffff88007a7 >> b8000 >> [ 1.666342] RIP: 0010:[<ffffffff813a0c0f>] [<ffffffff813a0c0f>] get_free_en >> tries+0x2cf/0x2e0 >> [ 1.666342] RSP: 0000:ffff88007a7b9bd8 EFLAGS: 00010046 >> [ 1.666342] RAX: 0000000000000286 RBX: 0000000000000001 RCX: 000000000000000 >> 0 >> [ 1.666342] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff81fc7d7 >> 0 >> [ 1.666342] RBP: ffff88007a7b9c20 R08: 0000000000000000 R09: 000000000000fff >> c >> [ 1.666342] R10: 0000000000000000 R11: 0000000000000000 R12: 000000000000028 >> 6 >> [ 1.666342] R13: 0000000000037032 R14: 0000000000000000 R15: 000000000000000 >> 0 >> [ 1.666342] FS: 0000000000000000(0000) GS:ffff88007b600000(0000) knlGS:0000 >> 000000000000 >> [ 1.666342] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b >> [ 1.666342] CR2: 0000000000000000 CR3: 0000000001c0c000 CR4: 00000000000006f >> 0 >> [ 1.666342] Stack: >> [ 1.666342] ffff8800370174b0 ffff880037017408 ffff88007a7b9c20 00000001814b >> 68b6 >> [ 1.666342] ffff880037024600 0000000000000000 0000000000037032 000000000000 >> 0000 >> [ 1.666342] 0000000000000000 ffff88007a7b9c50 ffffffff813a0c43 ffff88003702 >> 4600 >> [ 1.666342] Call Trace: >> [ 1.666342] [<ffffffff813a0c43>] gnttab_grant_foreign_access+0x23/0x60 >> [ 1.666342] [<ffffffff814c7e58>] xenkbd_connect_backend+0x58/0x2c0 >> [ 1.666342] [<ffffffff814c848a>] xenkbd_probe+0x2fa/0x340 >> [ 1.666342] [<ffffffff813a856e>] xenbus_dev_probe+0x8e/0x170 >> [ 1.666342] [<ffffffff813a9ba8>] xenbus_frontend_dev_probe+0x48/0x50 >> [ 1.666342] [<ffffffff813f21a7>] driver_probe_device+0x87/0x390 >> [ 1.666342] [<ffffffff813f2583>] __driver_attach+0x93/0xa0 >> [ 1.666342] [<ffffffff813f24f0>] ? __device_attach+0x40/0x40 >> [ 1.666342] [<ffffffff813f00e3>] bus_for_each_dev+0x63/0xa0 >> [ 1.666342] [<ffffffff813f1bfe>] driver_attach+0x1e/0x20 >> [ 1.666342] [<ffffffff813f1798>] bus_add_driver+0x1e8/0x2a0 >> [ 1.666342] [<ffffffff81d59578>] ? lifebook_module_init+0x1b/0x1b >> [ 1.666342] [<ffffffff813f2bc4>] driver_register+0x74/0x150 >> [ 1.666342] [<ffffffff81d59578>] ? lifebook_module_init+0x1b/0x1b >> [ 1.666342] [<ffffffff813a7daa>] xenbus_register_driver_common+0x1a/0x20 >> [ 1.666342] [<ffffffff813aa078>] xenbus_register_frontend+0x28/0x50 >> [ 1.666342] [<ffffffff81d595a3>] xenkbd_init+0x2b/0x34 >> [ 1.666342] [<ffffffff810020fa>] do_one_initcall+0xfa/0x1b0 >> [ 1.666342] [<ffffffff81086785>] ? parse_args+0x225/0x400 >> [ 1.666342] [<ffffffff81d0f078>] kernel_init_freeable+0x177/0x1ff >> [ 1.666342] [<ffffffff81d0e898>] ? do_early_param+0x88/0x88 >> [ 1.666342] [<ffffffff8163df40>] ? rest_init+0x80/0x80 >> [ 1.666342] [<ffffffff8163df4e>] kernel_init+0xe/0x190 >> [ 1.666342] [<ffffffff81656dec>] ret_from_fork+0x7c/0xb0 >> [ 1.666342] [<ffffffff8163df40>] ? rest_init+0x80/0x80 >> [ 1.666342] Code: 8b 05 9e 71 c2 00 44 8b 2d 83 71 c2 00 e9 62 fe ff ff 66 2 >> e 0f 1f 84 00 00 00 00 00 b8 e4 ff ff ff e9 2a ff ff ff 44 89 c7 eb 84 <0f> 0b >> 0f 0b 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 >> [ 1.666342] RIP [<ffffffff813a0c0f>] get_free_entries+0x2cf/0x2e0 >> [ 1.666342] RSP <ffff88007a7b9bd8> >> [ 1.666342] ---[ end trace 6eb44b05748c7d42 ]--- >> [ 2.122137] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0 >> 000000b > Looks like I have a similaire issue with the module xen_kbdfront, but > later in the boot, an the guest is running fine. I think that could be > an hidden bug which did not show up before, I''ll try to understand the > issue. > > Thank you for testing. >If you need more tests and/or informations tell me and I''ll do and post them.
On Mon, Nov 25, 2013 at 10:04:59AM +0100, Fabio Fantoni wrote:> Il 22/11/2013 17:54, Anthony PERARD ha scritto: > >On Fri, Nov 22, 2013 at 04:49:06PM +0100, Fabio Fantoni wrote: > >>Il 22/11/2013 16:13, Anthony PERARD ha scritto: > >>>Hi, > >>> > >>>Here is a little series that attempt to fix the issue regarding > >>>xen_platform_pci=0 not been handled. > >>> > >>>There are two patches, the first one adds an option to specifies the QEMU > >>>machine that a user wants and we handle the xen_platform_pci=0 case using the > >>>new option. > >>> > >>>The new options "qemu_machine_override" will help if one want to try a q35 > >>>based device model. Otherwise, it will be used by libxl to switch to the "pc" > >>>machine instead of the "xenfv" one when necessary, as the only difference > >>>between both (since QEMU 1.6) is the presence of the xen-platform pci device. > > About q35 in theory should be missing only the implementation in hvmloader, > is there a draft somewhere to try or to should be made?No, sorry, no draft. In hvmloader, there is an easy change, comment the assert that prevent hvmloader from starting, but that only the beginning. -- Anthony PERARD
Konrad Rzeszutek Wilk
2013-Nov-26 19:08 UTC
Re: [PATCH 0/2] Handle xen_platform_pci=0 case
> > [ 1.665373] ------------[ cut here ]------------ > > [ 1.665704] sd 0:0:0:0: [sda] Attached SCSI disk > > [ 1.666342] kernel BUG at drivers/xen/grant-table.c:1189! > > [ 1.666342] invalid opcode: 0000 [#1] SMP > > [ 1.666342] Modules linked in: > > [ 1.666342] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.11.6-200.fc19.x86_64 > > #1 > > [ 1.666342] Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/22/2013 > > [ 1.666342] task: ffff88007a7b0000 ti: ffff88007a7b8000 task.ti: ffff88007a7 > > b8000 > > [ 1.666342] RIP: 0010:[<ffffffff813a0c0f>] [<ffffffff813a0c0f>] get_free_en > > tries+0x2cf/0x2e0 > > [ 1.666342] RSP: 0000:ffff88007a7b9bd8 EFLAGS: 00010046 > > [ 1.666342] RAX: 0000000000000286 RBX: 0000000000000001 RCX: 000000000000000 > > 0 > > [ 1.666342] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff81fc7d7 > > 0 > > [ 1.666342] RBP: ffff88007a7b9c20 R08: 0000000000000000 R09: 000000000000fff > > c > > [ 1.666342] R10: 0000000000000000 R11: 0000000000000000 R12: 000000000000028 > > 6 > > [ 1.666342] R13: 0000000000037032 R14: 0000000000000000 R15: 000000000000000 > > 0 > > [ 1.666342] FS: 0000000000000000(0000) GS:ffff88007b600000(0000) knlGS:0000 > > 000000000000 > > [ 1.666342] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > > [ 1.666342] CR2: 0000000000000000 CR3: 0000000001c0c000 CR4: 00000000000006f > > 0 > > [ 1.666342] Stack: > > [ 1.666342] ffff8800370174b0 ffff880037017408 ffff88007a7b9c20 00000001814b > > 68b6 > > [ 1.666342] ffff880037024600 0000000000000000 0000000000037032 000000000000 > > 0000 > > [ 1.666342] 0000000000000000 ffff88007a7b9c50 ffffffff813a0c43 ffff88003702 > > 4600 > > [ 1.666342] Call Trace: > > [ 1.666342] [<ffffffff813a0c43>] gnttab_grant_foreign_access+0x23/0x60 > > [ 1.666342] [<ffffffff814c7e58>] xenkbd_connect_backend+0x58/0x2c0 > > [ 1.666342] [<ffffffff814c848a>] xenkbd_probe+0x2fa/0x340 > > [ 1.666342] [<ffffffff813a856e>] xenbus_dev_probe+0x8e/0x170 > > [ 1.666342] [<ffffffff813a9ba8>] xenbus_frontend_dev_probe+0x48/0x50 > > [ 1.666342] [<ffffffff813f21a7>] driver_probe_device+0x87/0x390 > > [ 1.666342] [<ffffffff813f2583>] __driver_attach+0x93/0xa0 > > [ 1.666342] [<ffffffff813f24f0>] ? __device_attach+0x40/0x40 > > [ 1.666342] [<ffffffff813f00e3>] bus_for_each_dev+0x63/0xa0 > > [ 1.666342] [<ffffffff813f1bfe>] driver_attach+0x1e/0x20 > > [ 1.666342] [<ffffffff813f1798>] bus_add_driver+0x1e8/0x2a0 > > [ 1.666342] [<ffffffff81d59578>] ? lifebook_module_init+0x1b/0x1b > > [ 1.666342] [<ffffffff813f2bc4>] driver_register+0x74/0x150 > > [ 1.666342] [<ffffffff81d59578>] ? lifebook_module_init+0x1b/0x1b > > [ 1.666342] [<ffffffff813a7daa>] xenbus_register_driver_common+0x1a/0x20 > > [ 1.666342] [<ffffffff813aa078>] xenbus_register_frontend+0x28/0x50 > > [ 1.666342] [<ffffffff81d595a3>] xenkbd_init+0x2b/0x34 > > [ 1.666342] [<ffffffff810020fa>] do_one_initcall+0xfa/0x1b0 > > [ 1.666342] [<ffffffff81086785>] ? parse_args+0x225/0x400 > > [ 1.666342] [<ffffffff81d0f078>] kernel_init_freeable+0x177/0x1ff > > [ 1.666342] [<ffffffff81d0e898>] ? do_early_param+0x88/0x88 > > [ 1.666342] [<ffffffff8163df40>] ? rest_init+0x80/0x80 > > [ 1.666342] [<ffffffff8163df4e>] kernel_init+0xe/0x190 > > [ 1.666342] [<ffffffff81656dec>] ret_from_fork+0x7c/0xb0 > > [ 1.666342] [<ffffffff8163df40>] ? rest_init+0x80/0x80 > > [ 1.666342] Code: 8b 05 9e 71 c2 00 44 8b 2d 83 71 c2 00 e9 62 fe ff ff 66 2 > > e 0f 1f 84 00 00 00 00 00 b8 e4 ff ff ff e9 2a ff ff ff 44 89 c7 eb 84 <0f> 0b > > 0f 0b 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 66 66 66 66 > > [ 1.666342] RIP [<ffffffff813a0c0f>] get_free_entries+0x2cf/0x2e0 > > [ 1.666342] RSP <ffff88007a7b9bd8> > > [ 1.666342] ---[ end trace 6eb44b05748c7d42 ]--- > > [ 2.122137] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0 > > 000000b > > Looks like I have a similaire issue with the module xen_kbdfront, but > later in the boot, an the guest is running fine. I think that could be > an hidden bug which did not show up before, I''ll try to understand the > issue.This looks familiar. I thought we had discussed it and it came out of this: t d0b4d64aadb9f4a90669848de9ef3819050a98cd Author: Matt Wilson <msw@amazon.com> Date: Tue Jan 15 13:21:27 2013 +0000 xen/grant-table: correctly initialize grant table version 1 Commit 85ff6acb075a484780b3d763fdf41596d8fc0970 (xen/granttable: Grant tables V2 implementation) changed the GREFS_PER_GRANT_FRAME macro from a constant to a conditional expression. The expression depends on grant_table_version being appropriately set. Unfortunately, at init time grant_table_version will be 0. The GREFS_PER_GRANT_FRAME conditional expression checks for "grant_table_version == 1", and therefore returns the number of grant references per frame for v2. This causes gnttab_init() to allocate fewer pages for gnttab_list, as a frame can old half the number of v2 entries than v1 entries. After gnttab_resume() is called, grant_table_version is appropriately set. nr_init_grefs will then be miscalculated and gnttab_free_count will hold a value larger than the actual number of free gref entries. If a guest is heavily utilizing improperly initialized v1 grant tables, memory corruption can occur. One common manifestation is corruption of the vmalloc list, resulting in a poisoned pointer derefrence when accessing /proc/meminfo or /proc/vmallocinfo: And the reason is that since the xen-platform-pci is not called we never called ''gnttab_setup'' (or rather ''gnttab_init()''). I am not really sure how to fix this right - we don''t want to load any of the PV drivers, but at the same time the ''xen_domain()'' check is turned ''on'' by: xen_hvm_guest_init(). which is OK. But if we add in the PV drivers a check for: if (xen_domain()) { if (xen_hvm_domain() && !xen-platform-pci-init-value)) return -ENODEV; } that means we won''t be able to load the drivers in random order. Such as xen-blkfront loaded before xen-platform-pci. But that is OK, those modules have a dependency on that driver (I hope!). Something like this? (Untested, not even compiled) diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c index 0a78524..3e480f1 100644 --- a/arch/x86/xen/platform-pci-unplug.c +++ b/arch/x86/xen/platform-pci-unplug.c @@ -69,6 +69,22 @@ static int check_platform_magic(void) return 0; } +bool xen_err_out(void) +{ + if (!xen_domain()) + return true; + + if (xen_hvm_domain()) { + if (xen_platform_pci_unplug & (XEN_UNPLUG_UNNECESSARY | XEN_UNPLUG_NEVER)) + return true; + if (xen_platform_pci_unplug == 0) + return true; + } + return false; +} +EXPORT_SYMBOL_GPL(xen_err_out); + +} void xen_unplug_emulated_devices(void) { int r; diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 432db1b..bcbaf0b 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -2074,7 +2074,7 @@ static int __init xlblk_init(void) if (!xen_domain()) return -ENODEV; - if (xen_hvm_domain() && !xen_platform_pci_unplug) + if (xen_err_out()) return -ENODEV; if (register_blkdev(XENVBD_MAJOR, DEV_NAME)) { diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index e21c181..e3640d6 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -380,6 +380,9 @@ static int __init xenkbd_init(void) if (xen_initial_domain()) return -ENODEV; + if (xen_err_out()) + return -ENODEV; + return xenbus_register_frontend(&xenkbd_driver); } diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index e59acb1..be2744b 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -2115,7 +2115,7 @@ static int __init netif_init(void) if (!xen_domain()) return -ENODEV; - if (xen_hvm_domain() && !xen_platform_pci_unplug) + if (xen_err_out()) return -ENODEV; pr_info("Initialising Xen virtual ethernet driver\n"); diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c index 129bf84..b1c0f2a 100644 --- a/drivers/xen/xenbus/xenbus_probe_frontend.c +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c @@ -496,7 +496,7 @@ subsys_initcall(xenbus_probe_frontend_init); #ifndef MODULE static int __init boot_wait_for_devices(void) { - if (xen_hvm_domain() && !xen_platform_pci_unplug) + if (xen_err_out()) return -ENODEV; ready_to_wait_for_devices = 1; diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h index 438c256..2120f90 100644 --- a/include/xen/platform_pci.h +++ b/include/xen/platform_pci.h @@ -47,5 +47,6 @@ static inline int xen_must_unplug_disks(void) { } extern int xen_platform_pci_unplug; +extern bool xen_err_out(void); #endif /* _XEN_PLATFORM_PCI_H */
Konrad Rzeszutek Wilk
2013-Nov-26 20:09 UTC
Re: [PATCH 0/2] Handle xen_platform_pci=0 case
> Something like this? (Untested, not even compiled) >Please try this one: From f7d3581aa19a35ea3ff10b965b2b08843e923635 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Date: Tue, 26 Nov 2013 15:05:40 -0500 Subject: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don''t blow up. *TODO* Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- drivers/block/xen-blkfront.c | 2 +- drivers/input/misc/xen-kbdfront.c | 4 ++++ drivers/net/xen-netfront.c | 2 +- drivers/xen/xenbus/xenbus_probe_frontend.c | 2 +- include/xen/platform_pci.h | 13 +++++++++++++ 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 432db1b..bcbaf0b 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -2074,7 +2074,7 @@ static int __init xlblk_init(void) if (!xen_domain()) return -ENODEV; - if (xen_hvm_domain() && !xen_platform_pci_unplug) + if (xen_err_out()) return -ENODEV; if (register_blkdev(XENVBD_MAJOR, DEV_NAME)) { diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c index e21c181..9d250cf 100644 --- a/drivers/input/misc/xen-kbdfront.c +++ b/drivers/input/misc/xen-kbdfront.c @@ -29,6 +29,7 @@ #include <xen/interface/io/fbif.h> #include <xen/interface/io/kbdif.h> #include <xen/xenbus.h> +#include <xen/platform_pci.h> struct xenkbd_info { struct input_dev *kbd; @@ -380,6 +381,9 @@ static int __init xenkbd_init(void) if (xen_initial_domain()) return -ENODEV; + if (xen_err_out()) + return -ENODEV; + return xenbus_register_frontend(&xenkbd_driver); } diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index e59acb1..be2744b 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -2115,7 +2115,7 @@ static int __init netif_init(void) if (!xen_domain()) return -ENODEV; - if (xen_hvm_domain() && !xen_platform_pci_unplug) + if (xen_err_out()) return -ENODEV; pr_info("Initialising Xen virtual ethernet driver\n"); diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c index 129bf84..b1c0f2a 100644 --- a/drivers/xen/xenbus/xenbus_probe_frontend.c +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c @@ -496,7 +496,7 @@ subsys_initcall(xenbus_probe_frontend_init); #ifndef MODULE static int __init boot_wait_for_devices(void) { - if (xen_hvm_domain() && !xen_platform_pci_unplug) + if (xen_err_out()) return -ENODEV; ready_to_wait_for_devices = 1; diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h index 438c256..a5bbd0b 100644 --- a/include/xen/platform_pci.h +++ b/include/xen/platform_pci.h @@ -47,5 +47,18 @@ static inline int xen_must_unplug_disks(void) { } extern int xen_platform_pci_unplug; +static bool xen_err_out(void) +{ + if (!xen_domain()) + return true; + + if (xen_hvm_domain()) { + if (xen_platform_pci_unplug & (XEN_UNPLUG_UNNECESSARY | XEN_UNPLUG_NEVER)) + return true; + if (xen_platform_pci_unplug == 0) + return true; + } + return false; +} #endif /* _XEN_PLATFORM_PCI_H */ -- 1.8.3.1
On Tue, Nov 26, 2013 at 03:09:56PM -0500, Konrad Rzeszutek Wilk wrote:> > Something like this? (Untested, not even compiled) > > > > Please try this one: > > From f7d3581aa19a35ea3ff10b965b2b08843e923635 Mon Sep 17 00:00:00 2001 > From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Date: Tue, 26 Nov 2013 15:05:40 -0500 > Subject: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don''t blow up. > > *TODO* > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>Looks like Linux is more happy with this patch. I have apply it on top of Linux 3.12.1 and no more "kernel BUG at drivers/xen/grant-table.c:1189!" in linux debug. Also, with the patch, my guest will shutdown (without, something prevent from shuting down complitly). So, this patch is working for me. Thanks.> --- > drivers/block/xen-blkfront.c | 2 +- > drivers/input/misc/xen-kbdfront.c | 4 ++++ > drivers/net/xen-netfront.c | 2 +- > drivers/xen/xenbus/xenbus_probe_frontend.c | 2 +- > include/xen/platform_pci.h | 13 +++++++++++++ > 5 files changed, 20 insertions(+), 3 deletions(-) > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c > index 432db1b..bcbaf0b 100644 > --- a/drivers/block/xen-blkfront.c > +++ b/drivers/block/xen-blkfront.c > @@ -2074,7 +2074,7 @@ static int __init xlblk_init(void) > if (!xen_domain()) > return -ENODEV; > > - if (xen_hvm_domain() && !xen_platform_pci_unplug) > + if (xen_err_out()) > return -ENODEV; > > if (register_blkdev(XENVBD_MAJOR, DEV_NAME)) { > diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c > index e21c181..9d250cf 100644 > --- a/drivers/input/misc/xen-kbdfront.c > +++ b/drivers/input/misc/xen-kbdfront.c > @@ -29,6 +29,7 @@ > #include <xen/interface/io/fbif.h> > #include <xen/interface/io/kbdif.h> > #include <xen/xenbus.h> > +#include <xen/platform_pci.h> > > struct xenkbd_info { > struct input_dev *kbd; > @@ -380,6 +381,9 @@ static int __init xenkbd_init(void) > if (xen_initial_domain()) > return -ENODEV; > > + if (xen_err_out()) > + return -ENODEV; > + > return xenbus_register_frontend(&xenkbd_driver); > } > > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c > index e59acb1..be2744b 100644 > --- a/drivers/net/xen-netfront.c > +++ b/drivers/net/xen-netfront.c > @@ -2115,7 +2115,7 @@ static int __init netif_init(void) > if (!xen_domain()) > return -ENODEV; > > - if (xen_hvm_domain() && !xen_platform_pci_unplug) > + if (xen_err_out()) > return -ENODEV; > > pr_info("Initialising Xen virtual ethernet driver\n"); > diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c > index 129bf84..b1c0f2a 100644 > --- a/drivers/xen/xenbus/xenbus_probe_frontend.c > +++ b/drivers/xen/xenbus/xenbus_probe_frontend.c > @@ -496,7 +496,7 @@ subsys_initcall(xenbus_probe_frontend_init); > #ifndef MODULE > static int __init boot_wait_for_devices(void) > { > - if (xen_hvm_domain() && !xen_platform_pci_unplug) > + if (xen_err_out()) > return -ENODEV; > > ready_to_wait_for_devices = 1; > diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h > index 438c256..a5bbd0b 100644 > --- a/include/xen/platform_pci.h > +++ b/include/xen/platform_pci.h > @@ -47,5 +47,18 @@ static inline int xen_must_unplug_disks(void) { > } > > extern int xen_platform_pci_unplug; > +static bool xen_err_out(void) > +{ > > + if (!xen_domain()) > + return true; > + > + if (xen_hvm_domain()) { > + if (xen_platform_pci_unplug & (XEN_UNPLUG_UNNECESSARY | XEN_UNPLUG_NEVER)) > + return true; > + if (xen_platform_pci_unplug == 0) > + return true; > + } > + return false; > +} > #endif /* _XEN_PLATFORM_PCI_H */ > -- > 1.8.3.1 >-- Anthony PERARD
Stefano Stabellini
2013-Nov-29 12:29 UTC
Re: [PATCH 1/2] libxl: adding support to use -machine option of QEMU.
On Fri, 22 Nov 2013, Anthony PERARD wrote:> It adds a new config option, "qemu_machine_override". > > This can be used in the future to switch to a Q35 based device model. It > can also be used on a recent QEMU to avoid adding the xen-platform > device used to setup PV drivers in the guest. > > Two possible values for now are "pc" or "xenfv" but there are > equivalents and xenfv is the default. A possible future use could be > qemu_machine_override="q35" when this machine will be able to start > successfully on Xen. > > When passed to qemu, libxl automatically adds ",accel=xen" to the > machine option. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>Given that you are introducing a new xl option, shouldn''t you add documention for it as part of this patch?> tools/libxl/libxl_dm.c | 7 +++++-- > tools/libxl/libxl_types.idl | 1 + > tools/libxl/xl_cmdimpl.c | 3 +++ > 3 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 292e351..3d4a913 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -608,7 +608,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > } > for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++) > flexarray_append(dm_args, b_info->extra[i]); > - flexarray_append(dm_args, "-M"); > + flexarray_append(dm_args, "-machine"); > switch (b_info->type) { > case LIBXL_DOMAIN_TYPE_PV: > flexarray_append(dm_args, "xenpv"); > @@ -616,7 +616,10 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > flexarray_append(dm_args, b_info->extra_pv[i]); > break; > case LIBXL_DOMAIN_TYPE_HVM: > - flexarray_append(dm_args, "xenfv"); > + if (b_info->qemu_machine) > + flexarray_append(dm_args, GCSPRINTF("%s,accel=xen", b_info->qemu_machine)); > + else > + flexarray_append(dm_args, "xenfv"); > for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) > flexarray_append(dm_args, b_info->extra_hvm[i]); > break; > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index cba8eff..4141501 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -317,6 +317,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ > # if you set device_model you must set device_model_version too > ("device_model", string), > ("device_model_ssidref", uint32), > + ("qemu_machine", string), > > # extra parameters pass directly to qemu, NULL terminated > ("extra", libxl_string_list), > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 341863e..4de6858 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -1491,6 +1491,9 @@ skip_vfb: > } > > > + xlu_cfg_replace_string (config, "qemu_machine_override", > + &b_info->qemu_machine, 0); > + > xlu_cfg_replace_string (config, "device_model_override", > &b_info->device_model, 0); > if (!xlu_cfg_get_string (config, "device_model_version", &buf, 0)) { > -- > Anthony PERARD >
Stefano Stabellini
2013-Nov-29 12:31 UTC
Re: [PATCH 2/2] libxl: Handle xen_platform_pci=0 case with qemu-xen.
On Fri, 22 Nov 2013, Anthony PERARD wrote:> This should result in QEMU *not* adding the xen-platform device. > > Since QEMU 1.6, this can be achieved by using a different qemu machine. > The one used by libxl is "xenfv", but using QEMU >=1.6 with "-machine > pc,accel=xen" works as well with only one difference compared to > "xenfv", there is no xen-platform device. > > One more things, if "qemu_machine_override" is set by the user, then we > check if it''s necessary to add the xen-platform device. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > tools/libxl/libxl_dm.c | 28 ++++++++++++++++++++++++++-- > 1 file changed, 26 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 3d4a913..33caa2b 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -390,6 +390,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config); > const libxl_sdl_info *sdl = dm_sdl(guest_config); > const char *keymap = dm_keymap(guest_config); > + const char *machine = b_info->qemu_machine; > flexarray_t *dm_args; > int i; > uint64_t ram_size; > @@ -608,6 +609,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > } > for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++) > flexarray_append(dm_args, b_info->extra[i]); > + > + if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) { > + if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) { > + if (b_info->qemu_machine) { > + /* Check the qemu machine asked for, it can be "xenfv" which > + * will add xen-platform, or it can be "pc" or "pc-*" which > + * in this case will need to add the device here. Anything > + * else is either a mistake or a machine not supported by > + * xen. */in that case it would be appropriate to print a warning or an error> + if (!strncmp("pc", b_info->qemu_machine, 2)) { > + flexarray_vappend(dm_args, "-device", "xen-platform"); > + } > + } > + } else { > + /* Switching here to the machine "pc" which does not add > + * the xen-platform device instead of the default "xenfv" machine. > + */ > + if (!b_info->qemu_machine) { > + machine = "pc"; > + } > + } > + } > + > flexarray_append(dm_args, "-machine"); > switch (b_info->type) { > case LIBXL_DOMAIN_TYPE_PV: > @@ -616,8 +640,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, > flexarray_append(dm_args, b_info->extra_pv[i]); > break; > case LIBXL_DOMAIN_TYPE_HVM: > - if (b_info->qemu_machine) > - flexarray_append(dm_args, GCSPRINTF("%s,accel=xen", b_info->qemu_machine)); > + if (machine) > + flexarray_append(dm_args, GCSPRINTF("%s,accel=xen", machine)); > else > flexarray_append(dm_args, "xenfv"); > for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) > -- > Anthony PERARD >
Fabio Fantoni
2013-Nov-29 12:49 UTC
Re: [PATCH 2/2] libxl: Handle xen_platform_pci=0 case with qemu-xen.
Il 29/11/2013 13:31, Stefano Stabellini ha scritto:> On Fri, 22 Nov 2013, Anthony PERARD wrote: >> This should result in QEMU *not* adding the xen-platform device. >> >> Since QEMU 1.6, this can be achieved by using a different qemu machine. >> The one used by libxl is "xenfv", but using QEMU >=1.6 with "-machine >> pc,accel=xen" works as well with only one difference compared to >> "xenfv", there is no xen-platform device. >> >> One more things, if "qemu_machine_override" is set by the user, then we >> check if it''s necessary to add the xen-platform device. >> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> --- >> tools/libxl/libxl_dm.c | 28 ++++++++++++++++++++++++++-- >> 1 file changed, 26 insertions(+), 2 deletions(-) >> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c >> index 3d4a913..33caa2b 100644 >> --- a/tools/libxl/libxl_dm.c >> +++ b/tools/libxl/libxl_dm.c >> @@ -390,6 +390,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, >> const libxl_vnc_info *vnc = libxl__dm_vnc(guest_config); >> const libxl_sdl_info *sdl = dm_sdl(guest_config); >> const char *keymap = dm_keymap(guest_config); >> + const char *machine = b_info->qemu_machine; >> flexarray_t *dm_args; >> int i; >> uint64_t ram_size; >> @@ -608,6 +609,29 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, >> } >> for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++) >> flexarray_append(dm_args, b_info->extra[i]); >> + >> + if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) { >> + if (libxl_defbool_val(b_info->u.hvm.xen_platform_pci)) { >> + if (b_info->qemu_machine) { >> + /* Check the qemu machine asked for, it can be "xenfv" which >> + * will add xen-platform, or it can be "pc" or "pc-*" which >> + * in this case will need to add the device here. Anything >> + * else is either a mistake or a machine not supported by >> + * xen. */ > in that case it would be appropriate to print a warning or an errorThis version is old. There is a v2 of that patch already on git: http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=2bc047635b51abd41c917aa2b813211ee0de2c38 And also doc patch: http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=80507420f96b06d68a5b23c18998486330b49657> > >> + if (!strncmp("pc", b_info->qemu_machine, 2)) { >> + flexarray_vappend(dm_args, "-device", "xen-platform"); >> + } >> + } >> + } else { >> + /* Switching here to the machine "pc" which does not add >> + * the xen-platform device instead of the default "xenfv" machine. >> + */ >> + if (!b_info->qemu_machine) { >> + machine = "pc"; >> + } >> + } >> + } >> + >> flexarray_append(dm_args, "-machine"); >> switch (b_info->type) { >> case LIBXL_DOMAIN_TYPE_PV: >> @@ -616,8 +640,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, >> flexarray_append(dm_args, b_info->extra_pv[i]); >> break; >> case LIBXL_DOMAIN_TYPE_HVM: >> - if (b_info->qemu_machine) >> - flexarray_append(dm_args, GCSPRINTF("%s,accel=xen", b_info->qemu_machine)); >> + if (machine) >> + flexarray_append(dm_args, GCSPRINTF("%s,accel=xen", machine)); >> else >> flexarray_append(dm_args, "xenfv"); >> for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) >> -- >> Anthony PERARD >> > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Il 25/11/2013 12:11, Anthony PERARD ha scritto:> On Mon, Nov 25, 2013 at 10:04:59AM +0100, Fabio Fantoni wrote: >> Il 22/11/2013 17:54, Anthony PERARD ha scritto: >>> On Fri, Nov 22, 2013 at 04:49:06PM +0100, Fabio Fantoni wrote: >>>> Il 22/11/2013 16:13, Anthony PERARD ha scritto: >>>>> Hi, >>>>> >>>>> Here is a little series that attempt to fix the issue regarding >>>>> xen_platform_pci=0 not been handled. >>>>> >>>>> There are two patches, the first one adds an option to specifies the QEMU >>>>> machine that a user wants and we handle the xen_platform_pci=0 case using the >>>>> new option. >>>>> >>>>> The new options "qemu_machine_override" will help if one want to try a q35 >>>>> based device model. Otherwise, it will be used by libxl to switch to the "pc" >>>>> machine instead of the "xenfv" one when necessary, as the only difference >>>>> between both (since QEMU 1.6) is the presence of the xen-platform pci device. >> About q35 in theory should be missing only the implementation in hvmloader, >> is there a draft somewhere to try or to should be made? > No, sorry, no draft. In hvmloader, there is an easy change, comment the > assert that prevent hvmloader from starting, but that only the > beginning. >Thanks for reply. Today I did the first tests with q35 on xen. I found that disks/cdrom not works with q35 and old qemu paramters used by libxl. With new qemu parameters works and works also with sata instead ide. Probably next week I''ll do libxl patches for initial draft of q35 support, new qemu parameters for disk/cdrom using upstream qemu and sata support. I tried to boot Saucy hvm DomU with q35 and sata but qemu crash on kernel loading, tried with both with and without xen-platform. On attachment the serial kernel logs of both cases. Can you take a look? Thanks for any reply and sorry for my bad english. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Friday, November 29, 2013, 5:06:20 PM, you wrote:> Il 25/11/2013 12:11, Anthony PERARD ha scritto: >> On Mon, Nov 25, 2013 at 10:04:59AM +0100, Fabio Fantoni wrote: >>> Il 22/11/2013 17:54, Anthony PERARD ha scritto: >>>> On Fri, Nov 22, 2013 at 04:49:06PM +0100, Fabio Fantoni wrote: >>>>> Il 22/11/2013 16:13, Anthony PERARD ha scritto: >>>>>> Hi, >>>>>> >>>>>> Here is a little series that attempt to fix the issue regarding >>>>>> xen_platform_pci=0 not been handled. >>>>>> >>>>>> There are two patches, the first one adds an option to specifies the QEMU >>>>>> machine that a user wants and we handle the xen_platform_pci=0 case using the >>>>>> new option. >>>>>> >>>>>> The new options "qemu_machine_override" will help if one want to try a q35 >>>>>> based device model. Otherwise, it will be used by libxl to switch to the "pc" >>>>>> machine instead of the "xenfv" one when necessary, as the only difference >>>>>> between both (since QEMU 1.6) is the presence of the xen-platform pci device. >>> About q35 in theory should be missing only the implementation in hvmloader, >>> is there a draft somewhere to try or to should be made? >> No, sorry, no draft. In hvmloader, there is an easy change, comment the >> assert that prevent hvmloader from starting, but that only the >> beginning. >>> Thanks for reply.> Today I did the first tests with q35 on xen. > I found that disks/cdrom not works with q35 and old qemu paramters used > by libxl. > With new qemu parameters works and works also with sata instead ide. > Probably next week I''ll do libxl patches for initial draft of q35 > support, new qemu parameters for disk/cdrom using upstream qemu and sata > support.> I tried to boot Saucy hvm DomU with q35 and sata but qemu crash on > kernel loading, tried with both with and without xen-platform. > On attachment the serial kernel logs of both cases. > Can you take a look?The logs are of the "xen-platform-pci=0" case i guess ? Was your kernel patched with Konrad''s draft patch ? (the oops is the same) http://www.gossamer-threads.com/lists/xen/devel/308017#308017 Would be nice if the Q35 model works without too much effort :-)> Thanks for any reply and sorry for my bad english.