[PATCH] x86: export 'pcibios_enabled'

[PATCH] x86: export 'pcibios_enabled'

[Randy Dunlap]

From: Randy Dunlap <rdunlap@xenotime.net>

Export 'pcibios_enabled' so that when uvesafb is built as a
loadable module (on X86_32), the build will succeed.

ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
Cc:	Michal Januszewski <spock@gentoo.org>
Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc:	linux-fbdev@vger.kernel.org
Cc:	x86@kernel.org
---
Applies to mainline; found in linux-next.

 arch/x86/pci/pcbios.c |    1 +
 1 file changed, 1 insertion(+)

--- linux-next-20120313.orig/arch/x86/pci/pcbios.c
+++ linux-next-20120313/arch/x86/pci/pcbios.c
@@ -27,6 +27,7 @@
 #define PCIBIOS_HW_TYPE2_SPEC		0x20
 
 int pcibios_enabled;
+EXPORT_SYMBOL(pcibios_enabled);
 
 /* According to the BIOS specification at:
  * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could

Re: [PATCH] x86: export 'pcibios_enabled'

[H. Peter Anvin]

On 03/13/2012 01:30 PM, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> Export 'pcibios_enabled' so that when uvesafb is built as a
> loadable module (on X86_32), the build will succeed.
> 
> ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> 
> Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> Cc:	Michal Januszewski <spock@gentoo.org>
> Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc:	linux-fbdev@vger.kernel.org
> Cc:	x86@kernel.org
> ---
> Applies to mainline; found in linux-next.
> 
>  arch/x86/pci/pcbios.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> +++ linux-next-20120313/arch/x86/pci/pcbios.c
> @@ -27,6 +27,7 @@
>  #define PCIBIOS_HW_TYPE2_SPEC		0x20
>  
>  int pcibios_enabled;
> +EXPORT_SYMBOL(pcibios_enabled);
>  
>  /* According to the BIOS specification at:
>   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could

I would think this should be EXPORT_SYMBOL_GPL()... this seems like a
symbol with a very high likelihood to be abused in strange ways.

	-hpa

Re: [PATCH] x86: export 'pcibios_enabled'

[Alan Cox]

> >  
> >  int pcibios_enabled;
> > +EXPORT_SYMBOL(pcibios_enabled);
> >  
> >  /* According to the BIOS specification at:
> >   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
> 
> I would think this should be EXPORT_SYMBOL_GPL()... this seems like a
> symbol with a very high likelihood to be abused in strange ways.

We don't need to expose it anyway

uvesafb could look for any PCI vga class device - which I suspect is
what it *should* be doing ?

Alan

Re: [PATCH] x86: export 'pcibios_enabled'

[Florian Tobias Schandinat]

[CC'ing Wang YanQing, the author of the patch requiring it]

On 03/14/2012 09:29 AM, Alan Cox wrote:
>>>  
>>>  int pcibios_enabled;
>>> +EXPORT_SYMBOL(pcibios_enabled);
>>>  
>>>  /* According to the BIOS specification at:
>>>   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
>>
>> I would think this should be EXPORT_SYMBOL_GPL()... this seems like a
>> symbol with a very high likelihood to be abused in strange ways.
> 
> We don't need to expose it anyway
> 
> uvesafb could look for any PCI vga class device - which I suspect is
> what it *should* be doing ?

Would this really change depending on whether the page is NX-protected
or not?
Your suggestion sounds like it is about detecting whether there is any
graphic chip present or not while the patch is about fixing an oops
caused by NX-protection of the BIOS.


Best regards,

Florian Tobias Schandinat

Re: [PATCH] x86: export 'pcibios_enabled'

[Alan Cox]

> > uvesafb could look for any PCI vga class device - which I suspect is
> > what it *should* be doing ?
> 
> Would this really change depending on whether the page is NX-protected
> or not?
> Your suggestion sounds like it is about detecting whether there is any
> graphic chip present or not while the patch is about fixing an oops
> caused by NX-protection of the BIOS.

Right yes.. I misunderstood what you are trying to do. I'd assumed you
wanted to find the ROM and thus set it to the right mapping mode.

You can use set_memory_x() to mark memory executable (and _nx to set it back).

If you really need to know if NX is being used then the check

	if (__supported_pte_mask & PTE_NX)

will do the trick and the variable is exported.

I'd suggest however you wrap that in a cpu_has_nx() type macro somewhere
in the arch headers.

If you go poking around pcibios values you are going to get burned if
someone is ever bored enough to make NX and PCIBIOS work together
differently.

Alan

Re: [PATCH] x86: export 'pcibios_enabled'

[Wang YanQing]

On Wed, Mar 14, 2012 at 11:21:37AM +0000, Alan Cox wrote:
> You can use set_memory_x() to mark memory executable (and _nx to set it back).
> 
NO I can't, if I set_memory_x and don't set pcibios_enabled = 1, then
static_protections will still failed because pcibios_enabled = 0, 
and I don't want to use set_memory_x, because I don't want to give the user 
of uvesafb that feeling "I will lost BIOS NX protection if I choice uvesafb."

> If you really need to know if NX is being used then the check
> 
> 	if (__supported_pte_mask & PTE_NX)
> 
> will do the trick and the variable is exported.
I don't understand what do you mean, do you means CONFIG_X86_PAE for 32? Or CONFIG_X86_64?
when NX is being used, the pci bios is NX or not also depend on
ACPI.ACPI on or off all will influnce the code path in pci_arch_init,
decide the set_bios_x have chance to execute or not.See
https://lkml.org/lkml/2011/11/16/84

By the way _PAGE_NX instead of PTE_NX, right?

> I'd suggest however you wrap that in a cpu_has_nx() type macro somewhere
> in the arch headers.
> 
The same above.

> If you go poking around pcibios values you are going to get burned if
> someone is ever bored enough to make NX and PCIBIOS work together
> differently.
> 
Indeed according to 5bd5a452662bc37c54fb6828db1a3faf87e6511c, who bring me 
the trouble, check the pcibios_enabled is the only simple and good way to 
resolve the Oops I meet.

If you really don't want it, and if I am not all wrong about your "helper method",
you means you want export the below:
int check_pcibios_enabled()?
See https://lkml.org/lkml/2012/2/26/124

Thanks.

Re: [PATCH] x86: export 'pcibios_enabled'

[Wang YanQing]

On Thu, Mar 22, 2012 at 08:41:35AM +0800, Wang YanQing wrote:
> On Wed, Mar 14, 2012 at 11:21:37AM +0000, Alan Cox wrote:
> > You can use set_memory_x() to mark memory executable (and _nx to set it back).
> > 
> NO I can't, if I set_memory_x and don't set pcibios_enabled = 1, then
> static_protections will still failed because pcibios_enabled = 0, 
> and I don't want to use set_memory_x, because I don't want to give the user 
> of uvesafb that feeling "I will lost BIOS NX protection if I choice uvesafb."
> 
> > If you really need to know if NX is being used then the check
> > 
> > 	if (__supported_pte_mask & PTE_NX)
> > 
> > will do the trick and the variable is exported.
> I don't understand what do you mean, do you means CONFIG_X86_PAE for 32? Or CONFIG_X86_64?
> when NX is being used, the pci bios is NX or not also depend on
> ACPI.ACPI on or off all will influnce the code path in pci_arch_init,
> decide the set_bios_x have chance to execute or not.See
> https://lkml.org/lkml/2011/11/16/84
> 
> By the way _PAGE_NX instead of PTE_NX, right?
> 
> > I'd suggest however you wrap that in a cpu_has_nx() type macro somewhere
> > in the arch headers.
> > 
> The same above.
> 
> > If you go poking around pcibios values you are going to get burned if
> > someone is ever bored enough to make NX and PCIBIOS work together
> > differently.
> > 
> Indeed according to 5bd5a452662bc37c54fb6828db1a3faf87e6511c, who bring me 
> the trouble, check the pcibios_enabled is the only simple and good way to 
> resolve the Oops I meet.
> 
> If you really don't want it, and if I am not all wrong about your "helper method",
> you means you want export the below:
> int check_pcibios_enabled()?
> See https://lkml.org/lkml/2012/2/26/124
> 
> Thanks.
> 
Sorry for the delay reply and cross email threads,  Alan Cox.
But I still can't find a better way then to check the pcibios_enabled variable,
any better suggestion?

Re: [PATCH] x86: export 'pcibios_enabled'

[Wang YanQing]

On Tue, Mar 13, 2012 at 05:43:25PM -0700, H. Peter Anvin wrote:
> On 03/13/2012 01:30 PM, Randy Dunlap wrote:
> > From: Randy Dunlap <rdunlap@xenotime.net>
> > 
> > Export 'pcibios_enabled' so that when uvesafb is built as a
> > loadable module (on X86_32), the build will succeed.
> > 
> > ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> > 
> > Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> > Cc:	Michal Januszewski <spock@gentoo.org>
> > Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> > Cc:	linux-fbdev@vger.kernel.org
> > Cc:	x86@kernel.org
> > ---
> > Applies to mainline; found in linux-next.
> > 
> >  arch/x86/pci/pcbios.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> > +++ linux-next-20120313/arch/x86/pci/pcbios.c
> > @@ -27,6 +27,7 @@
> >  #define PCIBIOS_HW_TYPE2_SPEC		0x20
> >  
> >  int pcibios_enabled;
> > +EXPORT_SYMBOL(pcibios_enabled);
> >  
> >  /* According to the BIOS specification at:
> >   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
> 
> I would think this should be EXPORT_SYMBOL_GPL()... this seems like a
> symbol with a very high likelihood to be abused in strange ways.
> 
> 	-hpa
> 
Yes, I think EXPORT_SYMBOL_GPL is better. Indeed, there is another issue I 
meet about the pcibios NX protection code.
If I set "pcibios_enable = 1" forcely no matter whether set_bios_x had been executed, then
the BIOS code is not NX. The problem is,  if set_bios_x had not been executed, of course set_memory_x 
for the bios code page has no chance to execute, then why the BIOS code is not NX? Any comment?
Or because the default set for the BIOS range is X, if so we should set_memory_nx if pcibios_enable = 0.

Re: [PATCH] x86: export 'pcibios_enabled'

[Wang YanQing]

On Tue, Mar 13, 2012 at 01:30:27PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> Export 'pcibios_enabled' so that when uvesafb is built as a
> loadable module (on X86_32), the build will succeed.
> 
> ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> 
> Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> Cc:	Michal Januszewski <spock@gentoo.org>
> Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc:	linux-fbdev@vger.kernel.org
> Cc:	x86@kernel.org
> ---
> Applies to mainline; found in linux-next.
> 
>  arch/x86/pci/pcbios.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> +++ linux-next-20120313/arch/x86/pci/pcbios.c
> @@ -27,6 +27,7 @@
>  #define PCIBIOS_HW_TYPE2_SPEC		0x20
>  
>  int pcibios_enabled;
> +EXPORT_SYMBOL(pcibios_enabled);
>  
>  /* According to the BIOS specification at:
>   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
Hi, could you resend this patch as EXPORT_SYMBOL_GPL to x86 architecture maintainer?
Looks at the MAINTAINERS for the below:

X86 ARCHITECTURE (32-BIT AND 64-BIT)
M:	Thomas Gleixner <tglx@linutronix.de>
M:	Ingo Molnar <mingo@redhat.com>
M:	"H. Peter Anvin" <hpa@zytor.com>
M:	x86@kernel.org

[PATCH v2] x86: export 'pcibios_enabled' as GPL

[Randy Dunlap]

From: Randy Dunlap <rdunlap@xenotime.net>

Export 'pcibios_enabled' so that when uvesafb is built as a
loadable module (on X86_32), the build will succeed.

ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!

Patch v2 uses EXPORT_SYMBOL_GPL() as requested.

Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
Cc:	Michal Januszewski <spock@gentoo.org>
Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
Cc:	linux-fbdev@vger.kernel.org
Cc:	x86@kernel.org
---
 arch/x86/pci/pcbios.c |    1 +
 1 file changed, 1 insertion(+)

--- linux-next-20120313.orig/arch/x86/pci/pcbios.c
+++ linux-next-20120313/arch/x86/pci/pcbios.c
@@ -27,6 +27,7 @@
 #define PCIBIOS_HW_TYPE2_SPEC		0x20
 
 int pcibios_enabled;
+EXPORT_SYMBOL_GPL(pcibios_enabled);
 
 /* According to the BIOS specification at:
  * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could

Re: [PATCH v2] x86: export 'pcibios_enabled' as GPL

[Wang YanQing]

On Sun, Mar 18, 2012 at 06:03:41PM -0700, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@xenotime.net>
> 
> Export 'pcibios_enabled' so that when uvesafb is built as a
> loadable module (on X86_32), the build will succeed.
> 
> ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> 
> Patch v2 uses EXPORT_SYMBOL_GPL() as requested.
> 
> Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> Cc:	Michal Januszewski <spock@gentoo.org>
> Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> Cc:	linux-fbdev@vger.kernel.org
> Cc:	x86@kernel.org
> ---
>  arch/x86/pci/pcbios.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> +++ linux-next-20120313/arch/x86/pci/pcbios.c
> @@ -27,6 +27,7 @@
>  #define PCIBIOS_HW_TYPE2_SPEC		0x20
>  
>  int pcibios_enabled;
> +EXPORT_SYMBOL_GPL(pcibios_enabled);
>  
>  /* According to the BIOS specification at:
>   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
Acked-by: Wang YanQing <udknight@gmail.com>

Re: [PATCH v2] x86: export 'pcibios_enabled' as GPL

[Alan Cox]

On Wed, 21 Mar 2012 12:37:21 +0800
Wang YanQing <udknight@gmail.com> wrote:

> On Sun, Mar 18, 2012 at 06:03:41PM -0700, Randy Dunlap wrote:
> > From: Randy Dunlap <rdunlap@xenotime.net>
> > 
> > Export 'pcibios_enabled' so that when uvesafb is built as a
> > loadable module (on X86_32), the build will succeed.
> > 
> > ERROR: "pcibios_enabled" [drivers/video/uvesafb.ko] undefined!
> > 
> > Patch v2 uses EXPORT_SYMBOL_GPL() as requested.
> > 
> > Signed-off-by: Randy Dunlap <rdunlap@xenotime.net>
> > Cc:	Michal Januszewski <spock@gentoo.org>
> > Cc:	Florian Tobias Schandinat <FlorianSchandinat@gmx.de>
> > Cc:	linux-fbdev@vger.kernel.org
> > Cc:	x86@kernel.org
> > ---
> >  arch/x86/pci/pcbios.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > --- linux-next-20120313.orig/arch/x86/pci/pcbios.c
> > +++ linux-next-20120313/arch/x86/pci/pcbios.c
> > @@ -27,6 +27,7 @@
> >  #define PCIBIOS_HW_TYPE2_SPEC		0x20
> >  
> >  int pcibios_enabled;
> > +EXPORT_SYMBOL_GPL(pcibios_enabled);
> >  
> >  /* According to the BIOS specification at:
> >   * http://members.datafast.net.au/dft0802/specs/bios21.pdf, we could
> Acked-by: Wang YanQing <udknight@gmail.com>

NAKked again by Alan Cox <alan@linux.intel.com>

The fb code shouldn't be playing guessing games with thsi variable.
I've pointed out both how to check if NX is enabled and how to set pages
NX/non-NX.

Even if you wanted to check the pci bios state we should export a helper
method.

So NAK.

Alan