Re: [PATCH 0/2] Handle xen_platform_pci=0 case

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

> > [    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 */