Xen devel - Oct 2013 - [BUG] Xen vm kernel crash in get_free_entries.

Hello,

This is a some kind of a follow up to the 
http://lists.xen.org/archives/html/xen-devel/2013-09/msg03109.html (
Linux 3.11.1 HVM guest kernel crash when started with xl 
(get_free_entries)).

Looks like we''ve here the similar issue.

Xen VM with 3.8.13 kernel crashes in get_free_entries (kernel BUG at 
drivers/xen/grant-table.c:1181!), i.e in:
BUG_ON(grefs_per_grant_frame == 0);. 3.11.5 kernel behaves in exactly 
the same way.

Whole kernel log is attached. It does not contain  any "Grant tables 
using version" message.

Seems that gnttab_request_version() has was never executed, so 
grefs_per_grant_frame was never set.

Could you please advice on this issue?
I''m ready to provide any further information if required.

I''m not subscribed, so please add me to CC, so that I can see replies.


--
Marina


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

On 16/10/13 07:28, Astarta wrote:
> Hello,
> 
> This is a some kind of a follow up to the
> http://lists.xen.org/archives/html/xen-devel/2013-09/msg03109.html (
> Linux 3.11.1 HVM guest kernel crash when started with xl
> (get_free_entries)).
> 
> Looks like we''ve here the similar issue.
> 
> Xen VM with 3.8.13 kernel crashes in get_free_entries (kernel BUG at
> drivers/xen/grant-table.c:1181!), i.e in:
> BUG_ON(grefs_per_grant_frame == 0);. 3.11.5 kernel behaves in exactly
> the same way.
> 
> Whole kernel log is attached. It does not contain  any "Grant tables
> using version" message.
> 
> Seems that gnttab_request_version() has was never executed, so
> grefs_per_grant_frame was never set.
> 
> Could you please advice on this issue?
I think you''re missing the platform pci device required for PVHVM.
What''s your VM''s xl.cfg?

David

On Wed, Oct 16, 2013 at 02:29:07PM +0100, David Vrabel
wrote:
> On 16/10/13 07:28, Astarta wrote:
> > Hello,
> > 
> > This is a some kind of a follow up to the
> > http://lists.xen.org/archives/html/xen-devel/2013-09/msg03109.html (
> > Linux 3.11.1 HVM guest kernel crash when started with xl
> > (get_free_entries)).
> > 
> > Looks like we''ve here the similar issue.
> > 
> > Xen VM with 3.8.13 kernel crashes in get_free_entries (kernel BUG at
> > drivers/xen/grant-table.c:1181!), i.e in:
> > BUG_ON(grefs_per_grant_frame == 0);. 3.11.5 kernel behaves in exactly
> > the same way.
> > 
> > Whole kernel log is attached. It does not contain  any "Grant
tables
> > using version" message.
> > 
> > Seems that gnttab_request_version() has was never executed, so
> > grefs_per_grant_frame was never set.
> > 
> > Could you please advice on this issue?
> 
> I think you''re missing the platform pci device required for PVHVM.
> What''s your VM''s xl.cfg?
> 
Yes, exactly, I had "xen_platform_pci=0" in my VM''s xl.cfg,
and I assume Astarta has aswell.

That is because I wanted to test *without* PVHVM, and that is a valid
configuration imho..
(and works OK with xm/xend).

-- Pasi

On 10/17/2013 12:55 PM, Astarta wrote:
> On 10/16/2013 06:17 PM, Pasi Kärkkäinen wrote:
>> On Wed, Oct 16, 2013 at 02:29:07PM +0100, David Vrabel wrote:
>>> On 16/10/13 07:28, Astarta wrote:
>>>> Hello,
>>>>
>>>> This is a some kind of a follow up to the
>>>>
http://lists.xen.org/archives/html/xen-devel/2013-09/msg03109.html (
>>>> Linux 3.11.1 HVM guest kernel crash when started with xl
>>>> (get_free_entries)).
>>>>
>>>> Looks like we've here the similar issue.
>>>>
>>>> Xen VM with 3.8.13 kernel crashes in get_free_entries (kernel
BUG at
>>>> drivers/xen/grant-table.c:1181!), i.e in:
>>>> BUG_ON(grefs_per_grant_frame == 0);. 3.11.5 kernel behaves in
exactly
>>>> the same way.
>>>>
>>>> Whole kernel log is attached. It does not contain  any
"Grant tables
>>>> using version" message.
>>>>
>>>> Seems that gnttab_request_version() has was never executed, so
>>>> grefs_per_grant_frame was never set.
>>>>
>>>> Could you please advice on this issue?
>>> I think you're missing the platform pci device required for
PVHVM.
>>> What's your VM's xl.cfg?
>>>
>> Yes, exactly, I had "xen_platform_pci=0" in my VM's
xl.cfg,
>> and I assume Astarta has aswell.
>>
>> That is because I wanted to test *without* PVHVM, and that is a valid 
>> configuration imho..
>> (and works OK with xm/xend).
>>
>> -- Pasi
>
> Thanks for the suggestion, but there is no xl.cfg in /etc/xen/ (we're 
> using Citrix XenServer Host 6.2.0-70446c
>  on rhel 6.2), so there is no where to edit xen_platform_pci option.
>
> I've attached output from xe vm-list params=all for one of the problem 
> vm. Hope its also ok.
>
> Also 3.6 kernel works well with that configuration. I can provide it's 
> boot log if needed.
>
> Please suggest.
>
> -- 
> Marina

will answer by myself. Setting platform:device_id to 1 manually helps 
the 3.11.5 kernel to boot.
I used xe command for this in the below way.
xe vm-param-set uuid=<VM UUID> platform:device_id=0001

The issue that platform:device_id is 0002 for citrix xen by default and 
the latest kernel doesnt boot in this configuration...

Where do we go from there?

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

On Thu, Oct 17, 2013 at 11:04:29PM +0400, Astarta wrote:
> On 10/17/2013 12:55 PM, Astarta wrote:
> >On 10/16/2013 06:17 PM, Pasi Kärkkäinen wrote:
> >>On Wed, Oct 16, 2013 at 02:29:07PM +0100, David Vrabel wrote:
> >>>On 16/10/13 07:28, Astarta wrote:
> >>>>Hello,
> >>>>
> >>>>This is a some kind of a follow up to the
>
>>>>http://lists.xen.org/archives/html/xen-devel/2013-09/msg03109.html
(
> >>>>Linux 3.11.1 HVM guest kernel crash when started with xl
> >>>>(get_free_entries)).
> >>>>
> >>>>Looks like we''ve here the similar issue.
> >>>>
> >>>>Xen VM with 3.8.13 kernel crashes in get_free_entries
(kernel BUG at
> >>>>drivers/xen/grant-table.c:1181!), i.e in:
> >>>>BUG_ON(grefs_per_grant_frame == 0);. 3.11.5 kernel behaves
in exactly
> >>>>the same way.
> >>>>
> >>>>Whole kernel log is attached. It does not contain  any
"Grant tables
> >>>>using version" message.
> >>>>
> >>>>Seems that gnttab_request_version() has was never executed,
so
> >>>>grefs_per_grant_frame was never set.
> >>>>
> >>>>Could you please advice on this issue?
> >>>I think you''re missing the platform pci device
required for PVHVM.
> >>>What''s your VM''s xl.cfg?
> >>>
> >>Yes, exactly, I had "xen_platform_pci=0" in my
VM''s xl.cfg,
> >>and I assume Astarta has aswell.
> >>
> >>That is because I wanted to test *without* PVHVM, and that is a
> >>valid configuration imho..
> >>(and works OK with xm/xend).
> >>
> >>-- Pasi
> >
> >Thanks for the suggestion, but there is no xl.cfg in /etc/xen/
> >(we''re using Citrix XenServer Host 6.2.0-70446c
> > on rhel 6.2), so there is no where to edit xen_platform_pci option.
> >
> >I''ve attached output from xe vm-list params=all for one of the
> >problem vm. Hope its also ok.
> >
> >Also 3.6 kernel works well with that configuration. I can provide
> >it''s boot log if needed.
> >
> >Please suggest.
> >
> >-- 
> >Marina
> 
> 
> will answer by myself. Setting platform:device_id to 1 manually
> helps the 3.11.5 kernel to boot.
> I used xe command for this in the below way.
> xe vm-param-set uuid=<VM UUID> platform:device_id=0001
> 
> The issue that platform:device_id is 0002 for citrix xen by default
> and the latest kernel doesnt boot in this configuration...
> 
> Where do we go from there?
>
Well, it''s a Linux kernel bug, I assume, so it should be fixed.
More debugging is needed.

There is, and shouldn''t be, a requirement to have Xen platform PCI
device available..
That''s how you enable/disable PVHVM, after all. 

-- Pasi

On 17/10/13 20:28, Pasi Kärkkäinen wrote:
> On Thu, Oct 17, 2013 at 11:04:29PM +0400, Astarta wrote:
>> On 10/17/2013 12:55 PM, Astarta wrote:
>>> On 10/16/2013 06:17 PM, Pasi Kärkkäinen wrote:
>>>> On Wed, Oct 16, 2013 at 02:29:07PM +0100, David Vrabel wrote:
>>>>> On 16/10/13 07:28, Astarta wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This is a some kind of a follow up to the
>>>>>>
http://lists.xen.org/archives/html/xen-devel/2013-09/msg03109.html (
>>>>>> Linux 3.11.1 HVM guest kernel crash when started with
xl
>>>>>> (get_free_entries)).
>>>>>>
>>>>>> Looks like we''ve here the similar issue.
>>>>>>
>>>>>> Xen VM with 3.8.13 kernel crashes in get_free_entries
(kernel BUG at
>>>>>> drivers/xen/grant-table.c:1181!), i.e in:
>>>>>> BUG_ON(grefs_per_grant_frame == 0);. 3.11.5 kernel
behaves in exactly
>>>>>> the same way.
>>>>>>
>>>>>> Whole kernel log is attached. It does not contain  any
"Grant tables
>>>>>> using version" message.
>>>>>>
>>>>>> Seems that gnttab_request_version() has was never
executed, so
>>>>>> grefs_per_grant_frame was never set.
>>>>>>
>>>>>> Could you please advice on this issue?
>>>>> I think you''re missing the platform pci device
required for PVHVM.
>>>>> What''s your VM''s xl.cfg?
>>>>>
>>>> Yes, exactly, I had "xen_platform_pci=0" in my
VM''s xl.cfg,
>>>> and I assume Astarta has aswell.
>>>>
>>>> That is because I wanted to test *without* PVHVM, and that is a
>>>> valid configuration imho..
>>>> (and works OK with xm/xend).
>>>>
>>>> -- Pasi
>>>
>>> Thanks for the suggestion, but there is no xl.cfg in /etc/xen/
>>> (we''re using Citrix XenServer Host 6.2.0-70446c
>>> on rhel 6.2), so there is no where to edit xen_platform_pci option.
>>>
>>> I''ve attached output from xe vm-list params=all for one of
the
>>> problem vm. Hope its also ok.
>>>
>>> Also 3.6 kernel works well with that configuration. I can provide
>>> it''s boot log if needed.
>>>
>>> Please suggest.
>>>
>>> -- 
>>> Marina
>>
>>
>> will answer by myself. Setting platform:device_id to 1 manually
>> helps the 3.11.5 kernel to boot.
>> I used xe command for this in the below way.
>> xe vm-param-set uuid=<VM UUID> platform:device_id=0001
>>
>> The issue that platform:device_id is 0002 for citrix xen by default
>> and the latest kernel doesnt boot in this configuration...
>>
>> Where do we go from there?
>>
> 
> Well, it''s a Linux kernel bug, I assume, so it should be fixed.
> More debugging is needed.
> 
> There is, and shouldn''t be, a requirement to have Xen platform PCI
device available..
> That''s how you enable/disable PVHVM, after all.
I suspect some of the changes for ARM has caused this (because ARM is
sort of PVHVM without a platform PCI device) but I had a quick look and
couldn''t spot anything.  Stefano, any ideas?

David

On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
> I suspect some of the changes for ARM has caused this (because ARM is
> sort of PVHVM without a platform PCI device) but I had a quick look and
> couldn''t spot anything.  Stefano, any ideas?
If there is no platform device then we should never be going anywhere
near any of the grant table code...

From the log in the original post it looks like at least some parts of
the kernel think it is running PVHVM (i.e. it does the unplug and says
"Booting paravirtualized kernel on Xen HVM"). I don''t think
this should
not be the case if there is no platform pci device.

Could this be because XenServer uses this platform_device=2 thing, which
is enough to trigger some of the early setup (because the unplug
protocol is present on I/O ports 0x10) but then the PCI driver in Linux
doesn''t know about this ID and so never initialises the rest of it?

Astarta, which of these configurations have you tried:

 - No platform device at all
 - Platform device with ID == 1
 - Platform device with ID == 2

and what happened with each?

Ian.

On 10/18/2013 01:46 PM, Ian Campbell wrote:
> On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
>
>> I suspect some of the changes for ARM has caused this (because ARM is
>> sort of PVHVM without a platform PCI device) but I had a quick look and
>> couldn''t spot anything.  Stefano, any ideas?
> If there is no platform device then we should never be going anywhere
> near any of the grant table code...
>
>  From the log in the original post it looks like at least some parts of
> the kernel think it is running PVHVM (i.e. it does the unplug and says
> "Booting paravirtualized kernel on Xen HVM"). I don''t
think this should
> not be the case if there is no platform pci device.
>
> Could this be because XenServer uses this platform_device=2 thing, which
> is enough to trigger some of the early setup (because the unplug
> protocol is present on I/O ports 0x10) but then the PCI driver in Linux
> doesn''t know about this ID and so never initialises the rest of
it?
>
> Astarta, which of these configurations have you tried:
>
>   - No platform device at all
>   - Platform device with ID == 1
>   - Platform device with ID == 2
>
> and what happened with each?
>
> Ian.
>
Hello Ian,

3.11.5 kernel boots OK if platform:device_id=0001 (see attached 
vm_boot_log_id1.txt and VM config vm_cfg_id1.txt)
3.11.5 kernel crashes if platform:device_id=0002 (see 
vm_boot_log_id2.txt mv_cfg_id2.txt).

Sorry, but I really dont know and cannot find in google how to test w/o 
platform:device_id at all. My system doesnt have xl.cfg in /etc/xen/ and 
I use xe utility to change parameters.
I''ve tried not to specify platform:device_id leaving it empty (i.e. xe 
vm-param-set uuid=<VM UUID> platform:device_id= ). Kernel crashes. see 
vm_boot_log_id_empty.txt and vm_cfg_id_empty.txt ).


Let me know if there''s anything else I can do to shed some light to the
problem.

--
Marina









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

> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Ian Campbell
> Sent: 18 October 2013 10:46
> To: David Vrabel
> Cc: xen-devel@lists.xen.org; Astarta; Stefano Stabellini
> Subject: Re: [Xen-devel] [BUG] Xen vm kernel crash in get_free_entries.
> 
> On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
> 
> > I suspect some of the changes for ARM has caused this (because ARM is
> > sort of PVHVM without a platform PCI device) but I had a quick look
and
> > couldn''t spot anything.  Stefano, any ideas?
> 
> If there is no platform device then we should never be going anywhere
> near any of the grant table code...
> 
> From the log in the original post it looks like at least some parts of
> the kernel think it is running PVHVM (i.e. it does the unplug and says
> "Booting paravirtualized kernel on Xen HVM"). I don''t
think this should
> not be the case if there is no platform pci device.
> 
> Could this be because XenServer uses this platform_device=2 thing, which
> is enough to trigger some of the early setup (because the unplug
> protocol is present on I/O ports 0x10) but then the PCI driver in Linux
> doesn''t know about this ID and so never initialises the rest of
it?
> 
> Astarta, which of these configurations have you tried:
> 
>  - No platform device at all
>  - Platform device with ID == 1
>  - Platform device with ID == 2
> 
> and what happened with each?
> 
device_id will be 2 only if you use a windows template - which you generally
should not for hvm linux as that also has viridian=true.

  Paul

On 10/18/2013 03:06 PM, Paul Durrant wrote:
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>> bounces@lists.xen.org] On Behalf Of Ian Campbell
>> Sent: 18 October 2013 10:46
>> To: David Vrabel
>> Cc: xen-devel@lists.xen.org; Astarta; Stefano Stabellini
>> Subject: Re: [Xen-devel] [BUG] Xen vm kernel crash in get_free_entries.
>>
>> On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
>>
>>> I suspect some of the changes for ARM has caused this (because ARM
is
>>> sort of PVHVM without a platform PCI device) but I had a quick look
and
>>> couldn''t spot anything.  Stefano, any ideas?
>> If there is no platform device then we should never be going anywhere
>> near any of the grant table code...
>>
>>  From the log in the original post it looks like at least some parts of
>> the kernel think it is running PVHVM (i.e. it does the unplug and says
>> "Booting paravirtualized kernel on Xen HVM"). I
don''t think this should
>> not be the case if there is no platform pci device.
>>
>> Could this be because XenServer uses this platform_device=2 thing,
which
>> is enough to trigger some of the early setup (because the unplug
>> protocol is present on I/O ports 0x10) but then the PCI driver in Linux
>> doesn''t know about this ID and so never initialises the rest
of it?
>>
>> Astarta, which of these configurations have you tried:
>>
>>   - No platform device at all
>>   - Platform device with ID == 1
>>   - Platform device with ID == 2
>>
>> and what happened with each?
>>
> device_id will be 2 only if you use a windows template - which you
generally should not for hvm linux as that also has viridian=true.
>
>    Paul
You''re right. The problem VMs are running Windows OS (Win2008 R2 and 
Windows 2012), Linux here is a kind of Live CD from where these VMs are 
backuped.

Friday, October 18, 2013, 1:06:52 PM, you wrote:
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>> bounces@lists.xen.org] On Behalf Of Ian Campbell
>> Sent: 18 October 2013 10:46
>> To: David Vrabel
>> Cc: xen-devel@lists.xen.org; Astarta; Stefano Stabellini
>> Subject: Re: [Xen-devel] [BUG] Xen vm kernel crash in get_free_entries.
>> 
>> On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
>> 
>> > I suspect some of the changes for ARM has caused this (because ARM
is
>> > sort of PVHVM without a platform PCI device) but I had a quick
look and
>> > couldn''t spot anything.  Stefano, any ideas?
>> 
>> If there is no platform device then we should never be going anywhere
>> near any of the grant table code...
>> 
>> From the log in the original post it looks like at least some parts of
>> the kernel think it is running PVHVM (i.e. it does the unplug and says
>> "Booting paravirtualized kernel on Xen HVM"). I
don''t think this should
>> not be the case if there is no platform pci device.
>> 
>> Could this be because XenServer uses this platform_device=2 thing,
which
>> is enough to trigger some of the early setup (because the unplug
>> protocol is present on I/O ports 0x10) but then the PCI driver in Linux
>> doesn''t know about this ID and so never initialises the rest
of it?
>> 
>> Astarta, which of these configurations have you tried:
>> 
>>  - No platform device at all
>>  - Platform device with ID == 1
>>  - Platform device with ID == 2
>> 
>> and what happened with each?
>> 
> device_id will be 2 only if you use a windows template - which you
generally should not for hvm linux as that also has viridian=true.
What option sets the device id to 2 in a cfg file ?
I have encountered it on the "normal" xen project as well, so
it''s not only xenserver.

No other related options in my cfg file as far as i see, only the
xen_platform_pci=0

It seemed to be introduced in the 3.8 kernel series.

--
Sander
>   Paul

> -----Original Message-----
> From: Sander Eikelenboom [mailto:linux@eikelenboom.it]
> Sent: 18 October 2013 12:28
> To: Paul Durrant
> Cc: Ian Campbell; David Vrabel; Stefano Stabellini; Astarta; xen-
> devel@lists.xen.org
> Subject: Re: [Xen-devel] [BUG] Xen vm kernel crash in get_free_entries.
> 
> 
> Friday, October 18, 2013, 1:06:52 PM, you wrote:
> 
> >> -----Original Message-----
> >> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> >> bounces@lists.xen.org] On Behalf Of Ian Campbell
> >> Sent: 18 October 2013 10:46
> >> To: David Vrabel
> >> Cc: xen-devel@lists.xen.org; Astarta; Stefano Stabellini
> >> Subject: Re: [Xen-devel] [BUG] Xen vm kernel crash in
get_free_entries.
> >>
> >> On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
> >>
> >> > I suspect some of the changes for ARM has caused this
(because ARM is
> >> > sort of PVHVM without a platform PCI device) but I had a
quick look and
> >> > couldn''t spot anything.  Stefano, any ideas?
> >>
> >> If there is no platform device then we should never be going
anywhere
> >> near any of the grant table code...
> >>
> >> From the log in the original post it looks like at least some
parts of
> >> the kernel think it is running PVHVM (i.e. it does the unplug and
says
> >> "Booting paravirtualized kernel on Xen HVM"). I
don''t think this should
> >> not be the case if there is no platform pci device.
> >>
> >> Could this be because XenServer uses this platform_device=2 thing,
> which
> >> is enough to trigger some of the early setup (because the unplug
> >> protocol is present on I/O ports 0x10) but then the PCI driver in
Linux
> >> doesn''t know about this ID and so never initialises the
rest of it?
> >>
> >> Astarta, which of these configurations have you tried:
> >>
> >>  - No platform device at all
> >>  - Platform device with ID == 1
> >>  - Platform device with ID == 2
> >>
> >> and what happened with each?
> >>
> 
> > device_id will be 2 only if you use a windows template - which you
> generally should not for hvm linux as that also has viridian=true.
> 
> What option sets the device id to 2 in a cfg file ?
> I have encountered it on the "normal" xen project as well, so
it''s not only
> xenserver.
> 
The device_id hack is only in XenServer.

  Paul
> No other related options in my cfg file as far as i see, only the
> xen_platform_pci=0
> 
> It seemed to be introduced in the 3.8 kernel series.
> 
> --
> Sander
> 
> >   Paul
> 
>

> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Astarta
> Sent: 18 October 2013 11:32
> To: Ian Campbell; David Vrabel
> Cc: xen-devel@lists.xen.org; Stefano Stabellini
> Subject: Re: [Xen-devel] [BUG] Xen vm kernel crash in get_free_entries.
> 
> On 10/18/2013 01:46 PM, Ian Campbell wrote:
> > On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
> >
> >> I suspect some of the changes for ARM has caused this (because ARM
is
> >> sort of PVHVM without a platform PCI device) but I had a quick
look and
> >> couldn''t spot anything.  Stefano, any ideas?
> > If there is no platform device then we should never be going anywhere
> > near any of the grant table code...
> >
> >  From the log in the original post it looks like at least some parts
of
> > the kernel think it is running PVHVM (i.e. it does the unplug and says
> > "Booting paravirtualized kernel on Xen HVM"). I
don''t think this should
> > not be the case if there is no platform pci device.
> >
> > Could this be because XenServer uses this platform_device=2 thing,
which
> > is enough to trigger some of the early setup (because the unplug
> > protocol is present on I/O ports 0x10) but then the PCI driver in
Linux
> > doesn''t know about this ID and so never initialises the rest
of it?
> >
> > Astarta, which of these configurations have you tried:
> >
> >   - No platform device at all
> >   - Platform device with ID == 1
> >   - Platform device with ID == 2
> >
> > and what happened with each?
> >
> > Ian.
> >
> 
> Hello Ian,
> 
> 3.11.5 kernel boots OK if platform:device_id=0001 (see attached
> vm_boot_log_id1.txt and VM config vm_cfg_id1.txt)
> 3.11.5 kernel crashes if platform:device_id=0002 (see
> vm_boot_log_id2.txt mv_cfg_id2.txt).
> 
> Sorry, but I really dont know and cannot find in google how to test w/o
> platform:device_id at all. My system doesnt have xl.cfg in /etc/xen/ and
> I use xe utility to change parameters.
> I''ve tried not to specify platform:device_id leaving it empty
(i.e. xe
> vm-param-set uuid=<VM UUID> platform:device_id= ). Kernel crashes.
see
> vm_boot_log_id_empty.txt and vm_cfg_id_empty.txt ).
> 
If you don''t specify device_id in your platform parameters then
XenServer QEMU will default to the standard value of 1, so it will be the same
as upstream

  Paul
> 
> Let me know if there''s anything else I can do to shed some light
to the
> problem.
> 
> --
> Marina
>

On Fri, Oct 18, 2013 at 10:46:04AM +0100, Ian Campbell
wrote:
> On Fri, 2013-10-18 at 10:31 +0100, David Vrabel wrote:
> 
> > I suspect some of the changes for ARM has caused this (because ARM is
> > sort of PVHVM without a platform PCI device) but I had a quick look
and
> > couldn''t spot anything.  Stefano, any ideas?
> 
> If there is no platform device then we should never be going anywhere
> near any of the grant table code...
> 
> From the log in the original post it looks like at least some parts of
> the kernel think it is running PVHVM (i.e. it does the unplug and says
> "Booting paravirtualized kernel on Xen HVM"). I don''t
think this should
> not be the case if there is no platform pci device.
> 
> Could this be because XenServer uses this platform_device=2 thing, which
> is enough to trigger some of the early setup (because the unplug
> protocol is present on I/O ports 0x10) but then the PCI driver in Linux
> doesn''t know about this ID and so never initialises the rest of
it?
> 
> Astarta, which of these configurations have you tried:
> 
>  - No platform device at all
>  - Platform device with ID == 1
>  - Platform device with ID == 2
> 
> and what happened with each?
> 
Just to clarify I didn''t test with XenServer,
I was testing with upstream Xen 4.2.3, where xen_platform_pci=0 causes the
crash..
but weirdly enough only when using xl.. with xm/xend it works OK with and
without platform pci device.

-- Pasi

On Fri, 2013-10-18 at 17:15 +0300, Pasi Kärkkäinen
wrote:
> I was testing with upstream Xen 4.2.3, where xen_platform_pci=0 causes the
crash..
Please can you provide a guest dmesg of the crash as well as details of
your guest/host configuration.

Under the circumstances it seems like the qemu logs and the qemu command
line parameters used in both xl and xm cases would be useful too.

Ian.


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

On Fri, Oct 18, 2013 at 03:19:02PM +0100, Ian Campbell
wrote:
> On Fri, 2013-10-18 at 17:15 +0300, Pasi Kärkkäinen wrote:
> > I was testing with upstream Xen 4.2.3, where xen_platform_pci=0 causes
the crash..
> 
> Please can you provide a guest dmesg of the crash as well as details of
> your guest/host configuration.
> 
> Under the circumstances it seems like the qemu logs and the qemu command
> line parameters used in both xl and xm cases would be useful too.
> 
Yep, I will, Konrad already asked for those in the other thread.

The problem is i''m away from the testbox currently.. and it''s
not remotely accessibly.
but I''ll post the logs/details later.

-- Pasi

Friday, October 18, 2013, 4:19:02 PM, you wrote:
> On Fri, 2013-10-18 at 17:15 +0300, Pasi Kärkkäinen wrote:
>> I was testing with upstream Xen 4.2.3, where xen_platform_pci=0 causes
the crash..
> Please can you provide a guest dmesg of the crash as well as details of
> your guest/host configuration.
> Under the circumstances it seems like the qemu logs and the qemu command
> line parameters used in both xl and xm cases would be useful too.
> Ian.

On a 3.12-rc5 kernel a straight revert of:
commit d0b4d64aadb9f4a90669848de9ef3819050a98cd xen/grant-table: correctly
initialize grant table version 1

makes a HVM guest (qemu-xen-traditional) with xen_platform_pci=0 boot again
using xl, haven't tested it with xend.

Dmesg of the guest still gives "booting a paravirtualized kernel on Xen
HVM" .. but that is probably correct.

--
Sander





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

On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
> Friday, October 18, 2013, 4:19:02 PM, you wrote:
>
>> On Fri, 2013-10-18 at 17:15 +0300, Pasi Kärkkäinen wrote:
>>> I was testing with upstream Xen 4.2.3, where xen_platform_pci=0
causes the crash..
>> Please can you provide a guest dmesg of the crash as well as details of
>> your guest/host configuration.
>> Under the circumstances it seems like the qemu logs and the qemu
command
>> line parameters used in both xl and xm cases would be useful too.
>> Ian.
>
> On a 3.12-rc5 kernel a straight revert of:
> commit d0b4d64aadb9f4a90669848de9ef3819050a98cd xen/grant-table: correctly
initialize grant table version 1
>
> makes a HVM guest (qemu-xen-traditional) with xen_platform_pci=0 boot again
using xl, haven't tested it with xend.
>
> Dmesg of the guest still gives "booting a paravirtualized kernel on
Xen HVM" .. but that is probably correct.
>
> --
> Sander
>
>
>
>

Great catch!
I also confirm that 3.11.5 kernel boots just fine after reverting of 
'correctly initialize grant table version 1' patch.

Thank you :)

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

On Sat, 2013-10-19 at 14:51 +0400, Astarta wrote:
> On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
> > makes a HVM guest (qemu-xen-traditional) with xen_platform_pci=0 boot
again using xl, haven''t tested it with xend.
> >
> Great catch!
> I also confirm that 3.11.5 kernel boots just fine after reverting of 
> ''correctly initialize grant table version 1'' patch.
This could just be down to that patch adding some BUG_ONs to catch bad
things going on, e.g. the one in gnttab_expand which I think is being
hit here.

I have a feeling that it is still wrong (but just more benign) to be
hitting that call chain in a configuration where there is no platform
device driver running. IOW reverting that patch removes the obvious
symptom (blowing up) but not the root cause, i.e. the patch is doing its
job.

Ian.

Saturday, October 19, 2013, 1:03:17 PM, you wrote:
> On Sat, 2013-10-19 at 14:51 +0400, Astarta wrote:
>> On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
>> > makes a HVM guest (qemu-xen-traditional) with xen_platform_pci=0
boot again using xl, haven''t tested it with xend.
>> >
>> Great catch!
>> I also confirm that 3.11.5 kernel boots just fine after reverting of 
>> ''correctly initialize grant table version 1'' patch.
> This could just be down to that patch adding some BUG_ONs to catch bad
> things going on, e.g. the one in gnttab_expand which I think is being
> hit here.
> I have a feeling that it is still wrong (but just more benign) to be
> hitting that call chain in a configuration where there is no platform
> device driver running. IOW reverting that patch removes the obvious
> symptom (blowing up) but not the root cause, i.e. the patch is doing its
> job.
That was my suspicion too, but at least it seems like some starting point
of further debugging.
(and indication of the kernels affected since this commit went to stable as
well)

Since i was still seeing the "Booting PV enabled guest on Xen HVM" is
was wondering
what is supposed to happen when there are some combinations ....

xen HVM xen_platform_pci=0 + guest kernel without PV guest support and without
xen pv drivers (net + block)

xen HVM xen_platform_pci=0 + guest kernel with PV guest support but without xen
pv drivers (net + block)

xen HVM xen_platform_pci=0 + guest kernel with PV guest support and with xen pv
drivers (net + block)
-- This is the configuration that hits the bug described here.

xen HVM xen_platform_pci=1 + guest kernel without PV guest support and without
xen pv drivers (net + block)

xen HVM xen_platform_pci=1 + guest kernel with PV guest support and without xen
pv drivers (net + block)

xen HVM xen_platform_pci=1 + guest kernel with PV guest support and with xen pv
drivers (net + block)


Booting a guest kernel with PV support as HVM but without using PV
doesn''t seem possible with a .cfg option ?
(yes it''s a hypothetical option (performance wise), as is running with
a guest kernel which supports PV drivers,
 but not using them with xen_platform_pci=0 .. but it is useful for debugging )

--
Sander
> Ian.

On Sat, Oct 19, 2013 at 12:03:17PM +0100, Ian Campbell
wrote:
> On Sat, 2013-10-19 at 14:51 +0400, Astarta wrote:
> > On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
> 
> > > makes a HVM guest (qemu-xen-traditional) with xen_platform_pci=0
boot again using xl, haven''t tested it with xend.
> > >
> > Great catch!
> > I also confirm that 3.11.5 kernel boots just fine after reverting of 
> > ''correctly initialize grant table version 1'' patch.
> 
> This could just be down to that patch adding some BUG_ONs to catch bad
> things going on, e.g. the one in gnttab_expand which I think is being
> hit here.
Indeed, the BUG_ON was added to ensure that the grant table system is
initialized before we attempt to expand the grant table space.
> I have a feeling that it is still wrong (but just more benign) to be
> hitting that call chain in a configuration where there is no platform
> device driver running. IOW reverting that patch removes the obvious
> symptom (blowing up) but not the root cause, i.e. the patch is doing its
> job.
The initialization of the grant table is deferred when running on a
HVM guest.

drivers/xen/grant-table.c:

static int __gnttab_init(void)
{
        /* Delay grant-table initialization in the PV on HVM case */
        if (xen_hvm_domain())
                return 0;

        if (!xen_pv_domain())
                return -ENODEV;

        return gnttab_init();
}

The Xen platform PCI driver initializes it when it binds to the PCI
device:

drivers/xen/platform-pci.c:

static int platform_pci_init(struct pci_dev *pdev,
                             const struct pci_device_id *ent)
{
...
        max_nr_gframes = gnttab_max_grant_frames();
        xen_hvm_resume_frames = alloc_xen_mmio(PAGE_SIZE *
	max_nr_gframes);
        ret = gnttab_init();
        if (ret)
                goto out;
        xenbus_probe(NULL);
        return 0;
...

Lots of initialization depends on the presence of the Xen platform PCI
device, I don''t see how PV enlightenment can be expected to work if
you don''t enable the PCI device.

--msw

On 21/10/13 11:29, Matt Wilson wrote:
> On Sat, Oct 19, 2013 at 12:03:17PM +0100, Ian Campbell wrote:
>> On Sat, 2013-10-19 at 14:51 +0400, Astarta wrote:
>>> On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
>>
>>>> makes a HVM guest (qemu-xen-traditional) with
xen_platform_pci=0 boot again using xl, haven''t tested it with xend.
>>>>
>>> Great catch!
>>> I also confirm that 3.11.5 kernel boots just fine after reverting
of
>>> ''correctly initialize grant table version 1''
patch.
>>
>> This could just be down to that patch adding some BUG_ONs to catch bad
>> things going on, e.g. the one in gnttab_expand which I think is being
>> hit here.
> 
> Indeed, the BUG_ON was added to ensure that the grant table system is
> initialized before we attempt to expand the grant table space.
> 
>> I have a feeling that it is still wrong (but just more benign) to be
>> hitting that call chain in a configuration where there is no platform
>> device driver running. IOW reverting that patch removes the obvious
>> symptom (blowing up) but not the root cause, i.e. the patch is doing
its
>> job.
> 
> The initialization of the grant table is deferred when running on a
> HVM guest.
> 
> drivers/xen/grant-table.c:
> 
> static int __gnttab_init(void)
> {
>         /* Delay grant-table initialization in the PV on HVM case */
>         if (xen_hvm_domain())
>                 return 0;
> 
>         if (!xen_pv_domain())
>                 return -ENODEV;
> 
>         return gnttab_init();
> }
> 
> The Xen platform PCI driver initializes it when it binds to the PCI
> device:
> 
> drivers/xen/platform-pci.c:
> 
> static int platform_pci_init(struct pci_dev *pdev,
>                              const struct pci_device_id *ent)
> {
> ...
>         max_nr_gframes = gnttab_max_grant_frames();
>         xen_hvm_resume_frames = alloc_xen_mmio(PAGE_SIZE *
> 	max_nr_gframes);
>         ret = gnttab_init();
>         if (ret)
>                 goto out;
>         xenbus_probe(NULL);
>         return 0;
> ...
> 
> Lots of initialization depends on the presence of the Xen platform PCI
> device, I don''t see how PV enlightenment can be expected to work
if
> you don''t enable the PCI device.
Yes, I think we need to defer the initialization of xenbus to
platform_pci_init() as well (and perhaps some other bits and pieces as
well?).

David

On Sat, Oct 19, 2013 at 01:58:50PM +0200, Sander Eikelenboom
wrote:
> 
> Saturday, October 19, 2013, 1:03:17 PM, you wrote:
> 
> > On Sat, 2013-10-19 at 14:51 +0400, Astarta wrote:
> >> On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
> 
> >> > makes a HVM guest (qemu-xen-traditional) with
xen_platform_pci=0 boot again using xl, haven''t tested it with xend.
> >> >
> >> Great catch!
> >> I also confirm that 3.11.5 kernel boots just fine after reverting
of
> >> ''correctly initialize grant table version 1''
patch.
> 
> > This could just be down to that patch adding some BUG_ONs to catch bad
> > things going on, e.g. the one in gnttab_expand which I think is being
> > hit here.
> 
> > I have a feeling that it is still wrong (but just more benign) to be
> > hitting that call chain in a configuration where there is no platform
> > device driver running. IOW reverting that patch removes the obvious
> > symptom (blowing up) but not the root cause, i.e. the patch is doing
its
> > job.
> 
> That was my suspicion too, but at least it seems like some starting point
> of further debugging.
> (and indication of the kernels affected since this commit went to stable as
well)
> 
> Since i was still seeing the "Booting PV enabled guest on Xen
HVM" is was wondering
> what is supposed to happen when there are some combinations ....
This is the enlightenment code noticing that it''s running in a HVM
guest under Xen via the hypervisor cpuid leaf (cpuid leaf
0x40000000).
> xen HVM xen_platform_pci=0 + guest kernel without PV guest support and
without xen pv drivers (net + block)
This should work.
> xen HVM xen_platform_pci=0 + guest kernel with PV guest support but without
xen pv drivers (net + block)
This should work.
> xen HVM xen_platform_pci=0 + guest kernel with PV guest support and with
xen pv drivers (net + block)
> -- This is the configuration that hits the bug described here.
I don''t see how this can be expected to work - the PV net and block
devices need the facilities that are initialized by the Xen platform
PCI device to operate. Of course it shouldn''t crash either, it should
just use emulated devices instead of xen-netfront/xen-blkfront.
> xen HVM xen_platform_pci=1 + guest kernel without PV guest support and
without xen pv drivers (net + block)
This should work.
> xen HVM xen_platform_pci=1 + guest kernel with PV guest support and without
xen pv drivers (net + block)
This should work.
> xen HVM xen_platform_pci=1 + guest kernel with PV guest support and with
xen pv drivers (net + block)
This should work.
> Booting a guest kernel with PV support as HVM but without using PV
doesn''t seem possible with a .cfg option ?
> (yes it''s a hypothetical option (performance wise), as is running
with a guest kernel which supports PV drivers,
>  but not using them with xen_platform_pci=0 .. but it is useful for
debugging )
AFAICT the expected behavior would be to for the guest kernel to use
basic enlightenment for CPU operations (hotplug, timers) but no PV IO
support (net + block). But perhaps I''m missing something since you
theoretically don''t need the PCI device if you have event channel
callback support in the guest kernel and sufficient support in the
hypervisor.

--msw

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.
> IMO, there is no need to add new fields with device id 0002 and device 
> id 0000 to platform_pci_tbl[] , we can modify the existing one to use 
> PCI_ANY_ID instead of PCI_DEVICE_ID_XEN_PLATFORM (which is 0001), so if 
> we have PCI_VENDOR_ID_XEN there is no need to pay attention on device id.
That omits the possibility that a future rev might differ in some
meaningful way though.

Ian.
> 
> So the patch is more than simple. See attached. I''ve tested the
resulted
> kernel in my environment (with device ids 0002, 0001 and 0000) and it 
> seems to work well.
> 
> 
> --
> Marina
> 
> On 10/21/2013 02:55 PM, Matt Wilson wrote:
> > On Sat, Oct 19, 2013 at 01:58:50PM +0200, Sander Eikelenboom wrote:
> >> Saturday, October 19, 2013, 1:03:17 PM, you wrote:
> >>
> >>> On Sat, 2013-10-19 at 14:51 +0400, Astarta wrote:
> >>>> On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
> >>>>> makes a HVM guest (qemu-xen-traditional) with
xen_platform_pci=0 boot again using xl, haven''t tested it with xend.
> >>>>>
> >>>> Great catch!
> >>>> I also confirm that 3.11.5 kernel boots just fine after
reverting of
> >>>> ''correctly initialize grant table version
1'' patch.
> >>> This could just be down to that patch adding some BUG_ONs to
catch bad
> >>> things going on, e.g. the one in gnttab_expand which I think
is being
> >>> hit here.
> >>> I have a feeling that it is still wrong (but just more benign)
to be
> >>> hitting that call chain in a configuration where there is no
platform
> >>> device driver running. IOW reverting that patch removes the
obvious
> >>> symptom (blowing up) but not the root cause, i.e. the patch is
doing its
> >>> job.
> >> That was my suspicion too, but at least it seems like some
starting point
> >> of further debugging.
> >> (and indication of the kernels affected since this commit went to
stable as well)
> >>
> >> Since i was still seeing the "Booting PV enabled guest on Xen
HVM" is was wondering
> >> what is supposed to happen when there are some combinations ....
> > This is the enlightenment code noticing that it''s running in
a HVM
> > guest under Xen via the hypervisor cpuid leaf (cpuid leaf
> > 0x40000000).
> >
> >> xen HVM xen_platform_pci=0 + guest kernel without PV guest support
and without xen pv drivers (net + block)
> > This should work.
> >
> >> xen HVM xen_platform_pci=0 + guest kernel with PV guest support
but without xen pv drivers (net + block)
> > This should work.
> >
> >> xen HVM xen_platform_pci=0 + guest kernel with PV guest support
and with xen pv drivers (net + block)
> >> -- This is the configuration that hits the bug described here.
> > I don''t see how this can be expected to work - the PV net and
block
> > devices need the facilities that are initialized by the Xen platform
> > PCI device to operate. Of course it shouldn''t crash either,
it should
> > just use emulated devices instead of xen-netfront/xen-blkfront.
> >
> >> xen HVM xen_platform_pci=1 + guest kernel without PV guest support
and without xen pv drivers (net + block)
> > This should work.
> >
> >> xen HVM xen_platform_pci=1 + guest kernel with PV guest support
and without xen pv drivers (net + block)
> > This should work.
> >
> >> xen HVM xen_platform_pci=1 + guest kernel with PV guest support
and with xen pv drivers (net + block)
> > This should work.
> >
> >> Booting a guest kernel with PV support as HVM but without using PV
doesn''t seem possible with a .cfg option ?
> >> (yes it''s a hypothetical option (performance wise), as is
running with a guest kernel which supports PV drivers,
> >>   but not using them with xen_platform_pci=0 .. but it is useful
for debugging )
> > AFAICT the expected behavior would be to for the guest kernel to use
> > basic enlightenment for CPU operations (hotplug, timers) but no PV IO
> > support (net + block). But perhaps I''m missing something
since you
> > theoretically don''t need the PCI device if you have event
channel
> > callback support in the guest kernel and sufficient support in the
> > hypervisor.
> >
> > --msw
> 
>

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?
> 
> > IMO, there is no need to add new fields with device id 0002 and device
> > id 0000 to platform_pci_tbl[] , we can modify the existing one to use 
> > PCI_ANY_ID instead of PCI_DEVICE_ID_XEN_PLATFORM (which is 0001), so
if
> > we have PCI_VENDOR_ID_XEN there is no need to pay attention on device
id.
> 
> That omits the possibility that a future rev might differ in some
> meaningful way though.
> 
> Ian.
> 
> > 
> > So the patch is more than simple. See attached. I''ve tested
the resulted
> > kernel in my environment (with device ids 0002, 0001 and 0000) and it 
> > seems to work well.
> > 
> > 
> > --
> > Marina
> > 
> > On 10/21/2013 02:55 PM, Matt Wilson wrote:
> > > On Sat, Oct 19, 2013 at 01:58:50PM +0200, Sander Eikelenboom
wrote:
> > >> Saturday, October 19, 2013, 1:03:17 PM, you wrote:
> > >>
> > >>> On Sat, 2013-10-19 at 14:51 +0400, Astarta wrote:
> > >>>> On 10/19/2013 03:14 AM, Sander Eikelenboom wrote:
> > >>>>> makes a HVM guest (qemu-xen-traditional) with
xen_platform_pci=0 boot again using xl, haven''t tested it with xend.
> > >>>>>
> > >>>> Great catch!
> > >>>> I also confirm that 3.11.5 kernel boots just fine
after reverting of
> > >>>> ''correctly initialize grant table version
1'' patch.
> > >>> This could just be down to that patch adding some BUG_ONs
to catch bad
> > >>> things going on, e.g. the one in gnttab_expand which I
think is being
> > >>> hit here.
> > >>> I have a feeling that it is still wrong (but just more
benign) to be
> > >>> hitting that call chain in a configuration where there is
no platform
> > >>> device driver running. IOW reverting that patch removes
the obvious
> > >>> symptom (blowing up) but not the root cause, i.e. the
patch is doing its
> > >>> job.
> > >> That was my suspicion too, but at least it seems like some
starting point
> > >> of further debugging.
> > >> (and indication of the kernels affected since this commit
went to stable as well)
> > >>
> > >> Since i was still seeing the "Booting PV enabled guest
on Xen HVM" is was wondering
> > >> what is supposed to happen when there are some combinations
....
> > > This is the enlightenment code noticing that it''s
running in a HVM
> > > guest under Xen via the hypervisor cpuid leaf (cpuid leaf
> > > 0x40000000).
> > >
> > >> xen HVM xen_platform_pci=0 + guest kernel without PV guest
support and without xen pv drivers (net + block)
> > > This should work.
> > >
> > >> xen HVM xen_platform_pci=0 + guest kernel with PV guest
support but without xen pv drivers (net + block)
> > > This should work.
> > >
> > >> xen HVM xen_platform_pci=0 + guest kernel with PV guest
support and with xen pv drivers (net + block)
> > >> -- This is the configuration that hits the bug described
here.
> > > I don''t see how this can be expected to work - the PV
net and block
> > > devices need the facilities that are initialized by the Xen
platform
> > > PCI device to operate. Of course it shouldn''t crash
either, it should
> > > just use emulated devices instead of xen-netfront/xen-blkfront.
> > >
> > >> xen HVM xen_platform_pci=1 + guest kernel without PV guest
support and without xen pv drivers (net + block)
> > > This should work.
> > >
> > >> xen HVM xen_platform_pci=1 + guest kernel with PV guest
support and without xen pv drivers (net + block)
> > > This should work.
> > >
> > >> xen HVM xen_platform_pci=1 + guest kernel with PV guest
support and with xen pv drivers (net + block)
> > > This should work.
> > >
> > >> Booting a guest kernel with PV support as HVM but without
using PV doesn''t seem possible with a .cfg option ?
> > >> (yes it''s a hypothetical option (performance wise),
as is running with a guest kernel which supports PV drivers,
> > >>   but not using them with xen_platform_pci=0 .. but it is
useful for debugging )
> > > AFAICT the expected behavior would be to for the guest kernel to
use
> > > basic enlightenment for CPU operations (hotplug, timers) but no
PV IO
> > > support (net + block). But perhaps I''m missing something
since you
> > > theoretically don''t need the PCI device if you have
event channel
> > > callback support in the guest kernel and sufficient support in
the
> > > hypervisor.
> > >
> > > --msw
> > 
> > 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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 ;-)

I don''t think it would be appropriate for Linux to bind to the device
with ID 2, since that has been assigned to a vendor
http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,hvm,pvdrivers.h.html
it should be up to them to define the ABI of that device and provide a
Linux binding if they want.

Perhaps the unplug protocol
http://xenbits.xen.org/docs/unstable/misc/xen-command-line.html needs
expanding to blacklist products based on the presence absence of
whichever platform device subids there are.

I''m a bit confused because I thought the requirement was that the base
level platform device was always present and the secondary ones (like
the Citrix PV device with ID 2) would only ever be additional. Maybe
I''m
wrong about that.

Ian.

On Wed, 2013-11-13 at 09:40 +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 ;-)
> 
> I don''t think it would be appropriate for Linux to bind to the
device
> with ID 2, since that has been assigned to a vendor
>
http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,hvm,pvdrivers.h.html
> it should be up to them to define the ABI of that device and provide a
> Linux binding if they want.
> 
> Perhaps the unplug protocol
> http://xenbits.xen.org/docs/unstable/misc/xen-command-line.html needs
> expanding to blacklist products based on the presence absence of
> whichever platform device subids there are.
> 
> I''m a bit confused because I thought the requirement was that the
base
> level platform device was always present and the secondary ones (like
> the Citrix PV device with ID 2) would only ever be additional. Maybe
I''m
> wrong about that.
It turns out that some of the conclusions in 
http://lists.xen.org/archives/html/xen-devel/2013-11/msg01796.html
are relevant here, especially the subthread from
http://lists.xen.org/archives/html/xen-devel/2013-11/msg01842.html
onwards...

Ian.

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@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
> 
> I don''t think it would be appropriate for Linux to bind to the
device
> with ID 2, since that has been assigned to a vendor
>
http://xenbits.xen.org/docs/unstable/hypercall/x86_64/include,public,hvm,pvdrivers.h.html
> it should be up to them to define the ABI of that device and provide a
> Linux binding if they want.
> 
> Perhaps the unplug protocol
> http://xenbits.xen.org/docs/unstable/misc/xen-command-line.html needs
> expanding to blacklist products based on the presence absence of
> whichever platform device subids there are.
> 
> I''m a bit confused because I thought the requirement was that the
base
> level platform device was always present and the secondary ones (like
> the Citrix PV device with ID 2) would only ever be additional. Maybe
I''m
> wrong about that.
> 
> Ian.
>

Tuesday, November 26, 2013, 9:08:47 PM, you 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.
Hi Konrad,

I just gave it a shot on qemu-xen-traditional (since support for
xen_platform_pci=0 is not upstream (yet))
It doesn''t crash anymore on boot, but the guest now doesn''t
have any disks, so it ends in busybox because it can''t find a root
partition.
I would have expected it to be using qemu emulated disks now, but a "cat
/proc/partitions" in busybox shows nothing.
But that''s perhaps qemu code ejecting the emulated disks anyhow ...

In the config file i use:
 disk = [ ''phy:/dev/xen_vms/testvm,hda,w'' ]

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

Tuesday, November 26, 2013, 11:00:15 PM, you wrote:

> Tuesday, November 26, 2013, 9:08:47 PM, you 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.
> Hi Konrad,
> I just gave it a shot on qemu-xen-traditional (since support for
xen_platform_pci=0 is not upstream (yet))
> It doesn''t crash anymore on boot, but the guest now
doesn''t have any disks, so it ends in busybox because it can''t
find a root partition.
> I would have expected it to be using qemu emulated disks now, but a
"cat /proc/partitions" in busybox shows nothing.
> But that''s perhaps qemu code ejecting the emulated disks anyhow
...
Or i probably haven''t got drivers for that compiled in ..
let''s have another look ..
> In the config file i use:
>  disk = [ ''phy:/dev/xen_vms/testvm,hda,w'' ]
> --
> Sander
>> 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 */

Tuesday, November 26, 2013, 11:00:15 PM, you wrote:

> Tuesday, November 26, 2013, 9:08:47 PM, you 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.
> Hi Konrad,
> I just gave it a shot on qemu-xen-traditional (since support for
xen_platform_pci=0 is not upstream (yet))
> It doesn''t crash anymore on boot, but the guest now
doesn''t have any disks, so it ends in busybox because it can''t
find a root partition.
> I would have expected it to be using qemu emulated disks now, but a
"cat /proc/partitions" in busybox shows nothing.
> But that''s perhaps qemu code ejecting the emulated disks anyhow
...
Hi Konrad,

With the linux PATA modules compiled in it now also boots fine with
qemu-xen-traditional and xen_platform_pci=0

Thanks,

Sander
> In the config file i use:
>  disk = [ ''phy:/dev/xen_vms/testvm,hda,w'' ]
> --
> Sander
>> 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 */

Sander Eikelenboom <linux@eikelenboom.it> wrote:
>
>Tuesday, November 26, 2013, 11:00:15 PM, you wrote:
>
>
>> Tuesday, November 26, 2013, 9:08:47 PM, you 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.
>
>> Hi Konrad,
>
>> I just gave it a shot on qemu-xen-traditional (since support for
>xen_platform_pci=0 is not upstream (yet))
>> It doesn''t crash anymore on boot, but the guest now
doesn''t have any
>disks, so it ends in busybox because it can''t find a root
partition.
>> I would have expected it to be using qemu emulated disks now, but a
>"cat /proc/partitions" in busybox shows nothing.
>> But that''s perhaps qemu code ejecting the emulated disks
anyhow ...
>
>Hi Konrad,
>
>With the linux PATA modules compiled in it now also boots fine with
>qemu-xen-traditional and xen_platform_pci=0
Wohoooo! I will be tack on Tested and Reported by tag on it.

Thank you for quick testing!
>
>Thanks,
>
>Sander
>
>> In the config file i use:
>>  disk = [ ''phy:/dev/xen_vms/testvm,hda,w'' ]
>
>> --
>> Sander
>
>>> 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 */

Wednesday, November 27, 2013, 12:05:01 AM, you wrote:
> Sander Eikelenboom <linux@eikelenboom.it> wrote:
>>
>>Tuesday, November 26, 2013, 11:00:15 PM, you wrote:
>>
>>
>>> Tuesday, November 26, 2013, 9:08:47 PM, you 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.
>>
>>> Hi Konrad,
>>
>>> I just gave it a shot on qemu-xen-traditional (since support for
>>xen_platform_pci=0 is not upstream (yet))
>>> It doesn''t crash anymore on boot, but the guest now
doesn''t have any
>>disks, so it ends in busybox because it can''t find a root
partition.
>>> I would have expected it to be using qemu emulated disks now, but a
>>"cat /proc/partitions" in busybox shows nothing.
>>> But that''s perhaps qemu code ejecting the emulated disks
anyhow ...
>>
>>Hi Konrad,
>>
>>With the linux PATA modules compiled in it now also boots fine with
>>qemu-xen-traditional and xen_platform_pci=0
> Wohoooo! I will be tack on Tested and Reported by tag on it.
> Thank you for quick testing!
Thanks , don''t know if you want to consider it for stable as well,
seems to be since the 3.8 series ..

--
Sander
>>
>>Thanks,
>>
>>Sander
>>
>>> In the config file i use:
>>>  disk = [ ''phy:/dev/xen_vms/testvm,hda,w'' ]
>>
>>> --
>>> Sander
>>
>>>> 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 */

On Tue, 2013-11-26 at 15:08 -0500, Konrad Rzeszutek Wilk
wrote:
> 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())
I think !xen_has_pv_devices() or some such would be a better
name.
> 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)
         ^ stray space, but I think you wanted an inline here anyway?

Or you could move this to arch/x86/xen/platform-pci-unplug.c and then
xen_platform_pci_unplug could be unexported, which seems like a good
thing to do if the logic to using it is as complex as below.
> +{
>  
> +	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;
There are some cases where xen_unplug_emulated_devices doesn''t update
xen_platform_pci_unplug, specifically when xen_emul_unplug contains
UNNECESSARY or NEVER. I think the above logic covers that case but it
might be simpler to always set it? At that point on the first if is
necessary?
> +	}
> +	return false;
> +}
>  #endif /* _XEN_PLATFORM_PCI_H */

On Wed, Nov 27, 2013 at 09:36:55AM +0000, Ian Campbell
wrote:
> On Tue, 2013-11-26 at 15:08 -0500, Konrad Rzeszutek Wilk wrote:
> > 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())
> 
> I think !xen_has_pv_devices() or some such would be a better name.
<nods>
> > 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)
> 
>          ^ stray space, but I think you wanted an inline here anyway?
Yup.
> 
> Or you could move this to arch/x86/xen/platform-pci-unplug.c and then
> xen_platform_pci_unplug could be unexported, which seems like a good
> thing to do if the logic to using it is as complex as below.
I was thinking about it - but then there is a bit of a problem with
!CONFIG_PVHVM && CONFIG_XEN_BLKFRONT for example. Which means that
platform-pci-unplug.c won''t be built, but the xen-blkfront will and
it needs the xen_has_pv_devices()). Hence sticking it in a header.
> 
> > +{
> >  
> > +	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;
> 
> There are some cases where xen_unplug_emulated_devices doesn''t
update
> xen_platform_pci_unplug, specifically when xen_emul_unplug contains
> UNNECESSARY or NEVER. I think the above logic covers that case but it
> might be simpler to always set it? At that point on the first if is
> necessary?
<nods>

Thanks for your review!
> 
> > +	}
> > +	return false;
> > +}
> >  #endif /* _XEN_PLATFORM_PCI_H */
> 
>

On Wed, 2013-11-27 at 09:24 -0500, Konrad Rzeszutek Wilk
wrote:
> On Wed, Nov 27, 2013 at 09:36:55AM +0000, Ian Campbell wrote:
> > On Tue, 2013-11-26 at 15:08 -0500, Konrad Rzeszutek Wilk wrote:
> > > 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())
> > 
> > I think !xen_has_pv_devices() or some such would be a better name.
> 
> <nods>
> > > 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)
> > 
> >          ^ stray space, but I think you wanted an inline here anyway?
> 
> Yup.
> > 
> > Or you could move this to arch/x86/xen/platform-pci-unplug.c and then
> > xen_platform_pci_unplug could be unexported, which seems like a good
> > thing to do if the logic to using it is as complex as below.
> 
> I was thinking about it - but then there is a bit of a problem with
> !CONFIG_PVHVM && CONFIG_XEN_BLKFRONT for example. Which means that
> platform-pci-unplug.c won''t be built, but the xen-blkfront will
and
> it needs the xen_has_pv_devices()). Hence sticking it in a header.
Is that not just a case of #define xen_has_pc_devices 1 with the
appropriate ifndef CONFIG_PVHVM in the header (the other case being the
prototype for the out of line version)? That''s a pretty common pattern
for things which rely on a patricular CONFIG_FOO to be useful.

Ian.

On Wed, Nov 27, 2013 at 03:58:55PM +0000, Ian Campbell
wrote:
> On Wed, 2013-11-27 at 09:24 -0500, Konrad Rzeszutek Wilk wrote:
> > On Wed, Nov 27, 2013 at 09:36:55AM +0000, Ian Campbell wrote:
> > > On Tue, 2013-11-26 at 15:08 -0500, Konrad Rzeszutek Wilk wrote:
> > > > 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())
> > > 
> > > I think !xen_has_pv_devices() or some such would be a better
name.
> > 
> > <nods>
> > > > 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)
> > > 
> > >          ^ stray space, but I think you wanted an inline here
anyway?
> > 
> > Yup.
> > > 
> > > Or you could move this to arch/x86/xen/platform-pci-unplug.c and
then
> > > xen_platform_pci_unplug could be unexported, which seems like a
good
> > > thing to do if the logic to using it is as complex as below.
> > 
> > I was thinking about it - but then there is a bit of a problem with
> > !CONFIG_PVHVM && CONFIG_XEN_BLKFRONT for example. Which means
that
> > platform-pci-unplug.c won''t be built, but the xen-blkfront
will and
> > it needs the xen_has_pv_devices()). Hence sticking it in a header.
> 
> Is that not just a case of #define xen_has_pc_devices 1 with the
> appropriate ifndef CONFIG_PVHVM in the header (the other case being the
> prototype for the out of line version)? That''s a pretty common
pattern
> for things which rely on a patricular CONFIG_FOO to be useful.
Yes. That would do it too (duh!)
> 
> Ian.
>

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@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;
Why do we error out here? There is nothing to unplug in this case.

>  	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)
> +{
static inline

> +	if (!xen_domain())
> +		return true;
> +
> +	if (xen_hvm_domain()) {
> +		if (xen_platform_pci_unplug & (XEN_UNPLUG_UNNECESSARY |
XEN_UNPLUG_NEVER))
This check wasn''t present before and I don''t think should be
present
now: xen_platform_pci_unplug cannot be set to XEN_UNPLUG_NEVER, see the
beginning of xen_unplug_emulated_devices. Also XEN_UNPLUG_UNNECESSARY is
meant to say that the user knows what she is doing and no unplug is
necessary to safely use PV drivers, see the commit message of
1dc7ce99b091a11cce0f34456c1ffcb928f17edd.

> +			return true;
> +		if (xen_platform_pci_unplug == 0)
> +			return true;
> +	}
> +	return false;
> +}
>  #endif /* _XEN_PLATFORM_PCI_H */

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@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;
> 
> 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.
> 
> 
> >  	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)
> > +{
> 
> static inline
> 
> 
> > +	if (!xen_domain())
> > +		return true;
> > +
> > +	if (xen_hvm_domain()) {
> > +		if (xen_platform_pci_unplug & (XEN_UNPLUG_UNNECESSARY |
XEN_UNPLUG_NEVER))
> 
> This check wasn''t present before and I don''t think should
be present
> now: xen_platform_pci_unplug cannot be set to XEN_UNPLUG_NEVER, see the
> beginning of xen_unplug_emulated_devices. Also XEN_UNPLUG_UNNECESSARY is
> meant to say that the user knows what she is doing and no unplug is
> necessary to safely use PV drivers, see the commit message of
> 1dc7ce99b091a11cce0f34456c1ffcb928f17edd.
> 
> 
> > +			return true;
> > +		if (xen_platform_pci_unplug == 0)
> > +			return true;
> > +	}
> > +	return false;
> > +}
> >  #endif /* _XEN_PLATFORM_PCI_H */
>

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@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;
> > 
> > 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

?

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@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;
>> > 
>> > 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

On Mon, Dec 09, 2013 at 01:57:37PM +0100, Sander Eikelenboom
wrote:
> 
> 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@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;
> >> > 
> >> > 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 ?
I need to respin it. Um, will do that later on today. Thanks for
poking me!
> 
> --
> Sander
>