[PATCH] xen: don't use PCI BIOS service for configuration space accesses

[PATCH] xen: don't use PCI BIOS service for configuration space accesses

[David Vrabel]

From: David Vrabel <david.vrabel@citrix.com>

The accessing PCI configuration space with the PCI BIOS service does
not work in PV guests.

This fixes boot on systems without MMCONFIG or where the BIOS hasn't
marked the MMCONFIG region as reserved in the e820 map.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Cc: stable@kernel.org
---
 arch/x86/xen/enlighten.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index b132ade..dbb5bb7 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -63,6 +63,7 @@
 #include <asm/stackprotector.h>
 #include <asm/hypervisor.h>
 #include <asm/mwait.h>
+#include <asm/pci_x86.h>
 
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
@@ -1365,7 +1366,9 @@ asmlinkage void __init xen_start_kernel(void)
 		/* Make sure ACS will be enabled */
 		pci_request_acs();
 	}
-		
+
+	/* PCI BIOS service won't work from a PV guest. */
+	pci_probe &= ~PCI_PROBE_BIOS;
 
 	xen_raw_console_write("about to get started...\n");
 
-- 
1.7.2.5

Re: [PATCH] xen: don't use PCI BIOS service for configuration space accesses

[Jan Beulich]

>>> On 13.04.12 at 14:08, David Vrabel <david.vrabel@citrix.com> wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> The accessing PCI configuration space with the PCI BIOS service does
> not work in PV guests.
> 
> This fixes boot on systems without MMCONFIG or where the BIOS hasn't
> marked the MMCONFIG region as reserved in the e820 map.

... and where "direct" access doesn't work either? Are there really
machines where Xen works on but this doesn't work? (Or, in case
this is disabled in your config, is it really useful to have
CONFIG_PCI_DIRECT disabled?)

That's just a comment on the description, the patch itself is fine
nevertheless (but should probably be sent to the x86 and/or PCI
maintainers).

Jan

> Signed-off-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

> Cc: stable@kernel.org 
> ---
>  arch/x86/xen/enlighten.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index b132ade..dbb5bb7 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -63,6 +63,7 @@
>  #include <asm/stackprotector.h>
>  #include <asm/hypervisor.h>
>  #include <asm/mwait.h>
> +#include <asm/pci_x86.h>
>  
>  #ifdef CONFIG_ACPI
>  #include <linux/acpi.h>
> @@ -1365,7 +1366,9 @@ asmlinkage void __init xen_start_kernel(void)
>  		/* Make sure ACS will be enabled */
>  		pci_request_acs();
>  	}
> -		
> +
> +	/* PCI BIOS service won't work from a PV guest. */
> +	pci_probe &= ~PCI_PROBE_BIOS;
>  
>  	xen_raw_console_write("about to get started...\n");
>  
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> http://lists.xen.org/xen-devel 

Re: [PATCH] xen: don't use PCI BIOS service for configuration space accesses

[David Vrabel]

On 13/04/12 13:29, Jan Beulich wrote:
>>>> On 13.04.12 at 14:08, David Vrabel <david.vrabel@citrix.com> wrote:
>> From: David Vrabel <david.vrabel@citrix.com>
>>
>> The accessing PCI configuration space with the PCI BIOS service does
>> not work in PV guests.
>>
>> This fixes boot on systems without MMCONFIG or where the BIOS hasn't
>> marked the MMCONFIG region as reserved in the e820 map.
> 
> ... and where "direct" access doesn't work either? Are there really
> machines where Xen works on but this doesn't work? (Or, in case
> this is disabled in your config, is it really useful to have
> CONFIG_PCI_DIRECT disabled?)

If you have CONFIG_PCI_GOANY (the default) BIOS is preferred over
direct.  So this change makes it skip BIOS and fall back to direct.

On the system I had saw the problem, the first call into the BIOS
service would hang the system.

> That's just a comment on the description, the patch itself is fine
> nevertheless (but should probably be sent to the x86 and/or PCI
> maintainers).

I was expecting Konrad to pick it up and forward it to the relevant
maintainer as appropriate.  Konrad, would you prefer if I sent to direct?

>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
>> Cc: stable@kernel.org 
>> ---
>>  arch/x86/xen/enlighten.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> index b132ade..dbb5bb7 100644
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -63,6 +63,7 @@
>>  #include <asm/stackprotector.h>
>>  #include <asm/hypervisor.h>
>>  #include <asm/mwait.h>
>> +#include <asm/pci_x86.h>
>>  
>>  #ifdef CONFIG_ACPI
>>  #include <linux/acpi.h>
>> @@ -1365,7 +1366,9 @@ asmlinkage void __init xen_start_kernel(void)
>>  		/* Make sure ACS will be enabled */
>>  		pci_request_acs();
>>  	}
>> -		
>> +
>> +	/* PCI BIOS service won't work from a PV guest. */
>> +	pci_probe &= ~PCI_PROBE_BIOS;
>>  
>>  	xen_raw_console_write("about to get started...\n");
>>  
>> -- 
>> 1.7.2.5
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org 
>> http://lists.xen.org/xen-devel 
> 
> 
> 

Re: [PATCH] xen: don't use PCI BIOS service for configuration space accesses

[Jan Beulich]

>>> On 13.04.12 at 14:55, David Vrabel <david.vrabel@citrix.com> wrote:
> On 13/04/12 13:29, Jan Beulich wrote:
>>>>> On 13.04.12 at 14:08, David Vrabel <david.vrabel@citrix.com> wrote:
>>> From: David Vrabel <david.vrabel@citrix.com>
>>>
>>> The accessing PCI configuration space with the PCI BIOS service does
>>> not work in PV guests.
>>>
>>> This fixes boot on systems without MMCONFIG or where the BIOS hasn't
>>> marked the MMCONFIG region as reserved in the e820 map.
>> 
>> ... and where "direct" access doesn't work either? Are there really
>> machines where Xen works on but this doesn't work? (Or, in case
>> this is disabled in your config, is it really useful to have
>> CONFIG_PCI_DIRECT disabled?)
> 
> If you have CONFIG_PCI_GOANY (the default) BIOS is preferred over
> direct.  So this change makes it skip BIOS and fall back to direct.

How is that? When I look at pci_arch_init(), I see pci_direct_probe()
being called first.

> On the system I had saw the problem, the first call into the BIOS
> service would hang the system.

Sure.

Jan

Re: [PATCH] xen: don't use PCI BIOS service for configuration space accesses

[David Vrabel]

On 13/04/12 15:15, Jan Beulich wrote:
>>>> On 13.04.12 at 14:55, David Vrabel <david.vrabel@citrix.com> wrote:
>> On 13/04/12 13:29, Jan Beulich wrote:
>>>>>> On 13.04.12 at 14:08, David Vrabel <david.vrabel@citrix.com> wrote:
>>>> From: David Vrabel <david.vrabel@citrix.com>
>>>>
>>>> The accessing PCI configuration space with the PCI BIOS service does
>>>> not work in PV guests.
>>>>
>>>> This fixes boot on systems without MMCONFIG or where the BIOS hasn't
>>>> marked the MMCONFIG region as reserved in the e820 map.
>>>
>>> ... and where "direct" access doesn't work either? Are there really
>>> machines where Xen works on but this doesn't work? (Or, in case
>>> this is disabled in your config, is it really useful to have
>>> CONFIG_PCI_DIRECT disabled?)
>>
>> If you have CONFIG_PCI_GOANY (the default) BIOS is preferred over
>> direct.  So this change makes it skip BIOS and fall back to direct.
> 
> How is that? When I look at pci_arch_init(), I see pci_direct_probe()
> being called first.

Hmm. Direct /is/ preferred over BIOS.  But the BIOS is initialized even
if direct is eventually used and it's the init of the BIOS that dies.

I'll fixup the description to make this clearer.

David

Re: [PATCH] xen: don't use PCI BIOS service for configuration space accesses

[Konrad Rzeszutek Wilk]

On Fri, Apr 13, 2012 at 01:55:11PM +0100, David Vrabel wrote:
> On 13/04/12 13:29, Jan Beulich wrote:
> >>>> On 13.04.12 at 14:08, David Vrabel <david.vrabel@citrix.com> wrote:
> >> From: David Vrabel <david.vrabel@citrix.com>
> >>
> >> The accessing PCI configuration space with the PCI BIOS service does
> >> not work in PV guests.
> >>
> >> This fixes boot on systems without MMCONFIG or where the BIOS hasn't
> >> marked the MMCONFIG region as reserved in the e820 map.
> > 
> > ... and where "direct" access doesn't work either? Are there really
> > machines where Xen works on but this doesn't work? (Or, in case
> > this is disabled in your config, is it really useful to have
> > CONFIG_PCI_DIRECT disabled?)
> 
> If you have CONFIG_PCI_GOANY (the default) BIOS is preferred over
> direct.  So this change makes it skip BIOS and fall back to direct.
> 
> On the system I had saw the problem, the first call into the BIOS
> service would hang the system.
> 
> > That's just a comment on the description, the patch itself is fine
> > nevertheless (but should probably be sent to the x86 and/or PCI
> > maintainers).
> 
> I was expecting Konrad to pick it up and forward it to the relevant
> maintainer as appropriate.  Konrad, would you prefer if I sent to direct?

You can send it to me. But I think it makes sense to stick
this in arch/x86/pci/xen.c - if it is not to late in teh bootup cycle?

> 
> >> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> > 
> > Acked-by: Jan Beulich <jbeulich@suse.com>
> > 
> >> Cc: stable@kernel.org 
> >> ---
> >>  arch/x86/xen/enlighten.c |    5 ++++-
> >>  1 files changed, 4 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> >> index b132ade..dbb5bb7 100644
> >> --- a/arch/x86/xen/enlighten.c
> >> +++ b/arch/x86/xen/enlighten.c
> >> @@ -63,6 +63,7 @@
> >>  #include <asm/stackprotector.h>
> >>  #include <asm/hypervisor.h>
> >>  #include <asm/mwait.h>
> >> +#include <asm/pci_x86.h>
> >>  
> >>  #ifdef CONFIG_ACPI
> >>  #include <linux/acpi.h>
> >> @@ -1365,7 +1366,9 @@ asmlinkage void __init xen_start_kernel(void)
> >>  		/* Make sure ACS will be enabled */
> >>  		pci_request_acs();
> >>  	}
> >> -		
> >> +
> >> +	/* PCI BIOS service won't work from a PV guest. */
> >> +	pci_probe &= ~PCI_PROBE_BIOS;
> >>  
> >>  	xen_raw_console_write("about to get started...\n");
> >>  
> >> -- 
> >> 1.7.2.5
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xen.org 
> >> http://lists.xen.org/xen-devel 
> > 
> > 
> > 

Re: [PATCH] xen: don't use PCI BIOS service for configuration space accesses

[David Vrabel]

On 16/04/12 16:10, Konrad Rzeszutek Wilk wrote:
> 
> But I think it makes sense to stick this in arch/x86/pci/xen.c - if
> it is not to late in teh bootup cycle?

pci_xen_init() is called early enough I think, but is only called for
domU (not dom0).  I can change this if you prefer.

David

Re: [PATCH] xen: don't use PCI BIOS service for configuration space accesses

[Konrad Rzeszutek Wilk]

On Mon, Apr 16, 2012 at 04:40:47PM +0100, David Vrabel wrote:
> On 16/04/12 16:10, Konrad Rzeszutek Wilk wrote:
> > 
> > But I think it makes sense to stick this in arch/x86/pci/xen.c - if
> > it is not to late in teh bootup cycle?
> 
> pci_xen_init() is called early enough I think, but is only called for
> domU (not dom0).  I can change this if you prefer.

For dom0 it could be done in: pci_xen_initial_domain ?
> 
> David