Re: [Xen-devel] [BUG] Xen vm kernel crash in get_free_entries.

[Sander Eikelenboom <linux@xxxxxxxxxxxxxx>]

Friday, November 29, 2013, 12:54:16 PM, you wrote:

> On Thu, 28 Nov 2013, Konrad Rzeszutek Wilk wrote:
>> On Thu, Nov 28, 2013 at 02:56:53PM +0000, Stefano Stabellini wrote:
>> > On Tue, 26 Nov 2013, Konrad Rzeszutek Wilk wrote:
>> > > On Wed, Nov 13, 2013 at 09:40:58AM +0000, Ian Campbell wrote:
>> > > > On Tue, 2013-11-12 at 10:56 -0500, Konrad Rzeszutek Wilk wrote:
>> > > > > On Thu, Nov 07, 2013 at 01:47:03PM +0000, Ian Campbell wrote:
>> > > > > > On Thu, 2013-11-07 at 09:20 +0400, Astarta wrote:
>> > > > > > > Hello,
>> > > > > > > 
>> > > > > > > Let me bring some new life to this discussion.
>> > > > > > > 
>> > > > > > > I've investigated a bit and found another way to make  kernels 
>> > > > > > > starting 
>> > > > > > > from 3.8.x to boot on the VMs with platform device_id 0002.
>> > > > > > > Reverting of 
>> > > > > > > xen-grant-table-correctly-initialize-grant-table-version-1 
>> > > > > > > patch is not necessary.
>> > > > > > > 
>> > > > > > > We can simply modify struct pci_device_id platform_pci_tbl[] (in 
>> > > > > > > drivers/xen/platform-pci.c) to respect 0002 and 0000 device ids.
>> > > > > > > That makes the kernel (3.8.x and 3.11.6) to boot correctly, 
>> > > > > > > disks and 
>> > > > > > > network are also recognized.
>> > > > > > 
>> > > > > > I think this is just working around the problem, by avoiding the
>> > > > > > situation where the error occurs. You could just as well switch to
>> > > > > > platform device id < 2.
>> > > > > 
>> > > > > I am bit late to this discussion - but shouldn't there be something
>> > > > > in the kernel to deal with this?
>> > > > 
>> > > > Well, ideally the kernel wouldn't crash ;-)
>> > > 
>> > > This patch should solve that (untested, but at least compile tested):
>> > > Please test it.
>> > > 
>> > > >From f7d3581aa19a35ea3ff10b965b2b08843e923635 Mon Sep 17 00:00:00 2001
>> > > From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>> > > 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@xxxxxxxxxx>
>> > > ---
>> > >  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;
>> > 
>> > Why do we error out here? There is nothing to unplug in this case.
>> 
>> B/c otherwise the driver blows up when it acts on the XenBus and tries
>> to setup a grant. But the grant is initialized by the xen-platform-pci
>> which is not run.

> I see. Adding this check must be the real purpose of the patch then?
> In that case don't we need another check like this one in:

> drivers/char/tpm/xen-tpmfront.c:xen_tpmfront_init
> drivers/pci/xen-pcifront.c:pcifront_init

> and for consistency (it doesn't use grants right now)

> drivers/video/xen-fbfront.c:xenfb_init

> ?


Hi Konrad / Stefano,

Is there any follow up on this ?

--
Sander


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel