[PATCH] video:uvesafb: Fix oops that uvesafb try to execute NX-protected page

Ok! I try to check pcibios_enabled first, but get some opposition by Alan Cox,
but I want to make thing work and fix the oops, so I choice the simple way to
check the (__supported_pte_mask & _PAGE_NX) instead of to check this variable plus
pci kernel boot parameter, pci mmconfig works or not, and more, and more. It is not
the best method, but it works and maybe all will feel happy.

Signed-off-by: Wang YanQing <udknight@gmail.com>
---
 drivers/video/uvesafb.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index 260cca7..26e83d7 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c

@@ -815,8 +815,15 @@ static int __devinit uvesafb_vbe_init(struct fb_info *info)
 	par->pmi_setpal = pmi_setpal;
 	par->ypan = ypan;
 
-	if (par->pmi_setpal || par->ypan)
-		uvesafb_vbe_getpmi(task, par);
+	if (par->pmi_setpal || par->ypan) {
+		if (__supported_pte_mask & _PAGE_NX) {
+			par->pmi_setpal = par->ypan = 0;
+			printk(KERN_WARNING "uvesafb: NX protection is actively."
+				"We have better not to use the PMI.\n");
+		} else {
+			uvesafb_vbe_getpmi(task, par);
+		}
+	}
 #else
 	/* The protected mode interface is not available on non-x86. */
 	par->pmi_setpal = par->ypan = 0;

-- 
1.7.9.2.315.g25a78

On Tue, 27 Mar 2012 18:01:36 +0800
Wang YanQing [off-list ref] wrote:

Ok! I try to check pcibios_enabled first, but get some opposition by Alan Cox,
but I want to make thing work and fix the oops, so I choice the simple way to
check the (__supported_pte_mask & _PAGE_NX) instead of to check this variable plus
pci kernel boot parameter, pci mmconfig works or not, and more, and more. It is not
the best method, but it works and maybe all will feel happy.

Okay let me ask the obvious question - why is it not the best method ?

Apart from adding a helper in the includes for the arch code of

	static inline is_nx_enabled(void)
	{
		return !!(__supported_pte_mask & _PAGE_NX);
	}

is there anything else it lacks ?

Yes ideally we'd set the relevant ROM areas executable, but for a simple
fix is there anything else that's a problem with it ?

Alan

On Tue, Mar 27, 2012 at 02:32:43PM +0100, Alan Cox wrote:

On Tue, 27 Mar 2012 18:01:36 +0800
Wang YanQing [off-list ref] wrote:

quoted

Ok! I try to check pcibios_enabled first, but get some opposition by Alan Cox,
but I want to make thing work and fix the oops, so I choice the simple way to
check the (__supported_pte_mask & _PAGE_NX) instead of to check this variable plus
pci kernel boot parameter, pci mmconfig works or not, and more, and more. It is not
the best method, but it works and maybe all will feel happy.

Okay let me ask the obvious question - why is it not the best method ?

Apart from adding a helper in the includes for the arch code of

	static inline is_nx_enabled(void)
	{
		return !!(__supported_pte_mask & _PAGE_NX);
	}

is there anything else it lacks ?

Yes ideally we'd set the relevant ROM areas executable, but for a simple
fix is there anything else that's a problem with it ?

Ok! Maybe you had missed my previous reply
http://permalink.gmane.org/gmane.linux.kernel/1272433
It is not the best method, because the check is not enough.
I means when NX is actively, the pci bios is NX or not also depend on
the code path in pci_arch_init which will be influenced by the acpi on or off, pci kernel boot
parameter, even kernel config like pci access method PCI_GOANY, PCI_GOMMCONFIG, or PCI_GODIRECT,
but if I check the pcibios_enabled, all the above can be ignored.

if uvesafb use the PMI when PCI BIOS is X, it can get the better work efficience then use the redraw 
method as a fallback when do the panning.

On 3/27/2012 9:32 PM, Alan Cox wrote:

On Tue, 27 Mar 2012 18:01:36 +0800
Wang YanQing [off-list ref] wrote:

quoted

Ok! I try to check pcibios_enabled first, but get some opposition by Alan Cox,
but I want to make thing work and fix the oops, so I choice the simple way to
check the (__supported_pte_mask & _PAGE_NX) instead of to check this variable plus
pci kernel boot parameter, pci mmconfig works or not, and more, and more. It is not
the best method, but it works and maybe all will feel happy.

Okay let me ask the obvious question - why is it not the best method ?

Apart from adding a helper in the includes for the arch code of

	static inline is_nx_enabled(void)
	{
		return !!(__supported_pte_mask & _PAGE_NX);
	}

is there anything else it lacks ?

maybe giving this inline function a return type ?

On 03/27/2012 01:32 PM, Alan Cox wrote:

On Tue, 27 Mar 2012 18:01:36 +0800
Wang YanQing [off-list ref] wrote:

quoted

Ok! I try to check pcibios_enabled first, but get some opposition by Alan Cox,
but I want to make thing work and fix the oops, so I choice the simple way to
check the (__supported_pte_mask & _PAGE_NX) instead of to check this variable plus
pci kernel boot parameter, pci mmconfig works or not, and more, and more. It is not
the best method, but it works and maybe all will feel happy.

Wang, could you please write a better commit message, at least you could
include the oops that your patch fixes.

Apart from adding a helper in the includes for the arch code of

	static inline is_nx_enabled(void)
	{
		return !!(__supported_pte_mask & _PAGE_NX);
	}

As this check is already done in a few other places I think there should
be a separate patch that adds this helper and ports all existing code to
use it.

So, does anyone have any objection against me taking this patch as is?


Thanks,

Florian Tobias Schandinat

On Wed, Mar 28, 2012 at 08:52:38AM +0800, Wang YanQing wrote:

On Tue, Mar 27, 2012 at 02:32:43PM +0100, Alan Cox wrote:

quoted

On Tue, 27 Mar 2012 18:01:36 +0800
Wang YanQing [off-list ref] wrote:

quoted

Ok! I try to check pcibios_enabled first, but get some opposition by Alan Cox,
but I want to make thing work and fix the oops, so I choice the simple way to
check the (__supported_pte_mask & _PAGE_NX) instead of to check this variable plus
pci kernel boot parameter, pci mmconfig works or not, and more, and more. It is not
the best method, but it works and maybe all will feel happy.

Okay let me ask the obvious question - why is it not the best method ?

Apart from adding a helper in the includes for the arch code of

	static inline is_nx_enabled(void)
	{
		return !!(__supported_pte_mask & _PAGE_NX);
	}

is there anything else it lacks ?

Yes ideally we'd set the relevant ROM areas executable, but for a simple
fix is there anything else that's a problem with it ?

Ok! Maybe you had missed my previous reply
http://permalink.gmane.org/gmane.linux.kernel/1272433
It is not the best method, because the check is not enough.
I means when NX is actively, the pci bios is NX or not also depend on
the code path in pci_arch_init which will be influenced by the acpi on or off, pci kernel boot
parameter, even kernel config like pci access method PCI_GOANY, PCI_GOMMCONFIG, or PCI_GODIRECT,
but if I check the pcibios_enabled, all the above can be ignored.

if uvesafb use the PMI when PCI BIOS is X, it can get the better work efficience then use the redraw 
method as a fallback when do the panning.

Alan

I am just curious, I want to know what I describe above is right a little,
or wrong about all the aspect.
thanks.

On Sun, Apr 01, 2012 at 09:05:30AM +0800, Wang YanQing wrote:

On Wed, Mar 28, 2012 at 08:52:38AM +0800, Wang YanQing wrote:

quoted

On Tue, Mar 27, 2012 at 02:32:43PM +0100, Alan Cox wrote:

quoted

On Tue, 27 Mar 2012 18:01:36 +0800
Wang YanQing [off-list ref] wrote:

quoted

Ok! I try to check pcibios_enabled first, but get some opposition by Alan Cox,
but I want to make thing work and fix the oops, so I choice the simple way to
check the (__supported_pte_mask & _PAGE_NX) instead of to check this variable plus
pci kernel boot parameter, pci mmconfig works or not, and more, and more. It is not
the best method, but it works and maybe all will feel happy.

Okay let me ask the obvious question - why is it not the best method ?

Apart from adding a helper in the includes for the arch code of

	static inline is_nx_enabled(void)
	{
		return !!(__supported_pte_mask & _PAGE_NX);
	}

is there anything else it lacks ?

Yes ideally we'd set the relevant ROM areas executable, but for a simple
fix is there anything else that's a problem with it ?

Ok! Maybe you had missed my previous reply
http://permalink.gmane.org/gmane.linux.kernel/1272433
It is not the best method, because the check is not enough.
I means when NX is actively, the pci bios is NX or not also depend on
the code path in pci_arch_init which will be influenced by the acpi on or off, pci kernel boot
parameter, even kernel config like pci access method PCI_GOANY, PCI_GOMMCONFIG, or PCI_GODIRECT,
but if I check the pcibios_enabled, all the above can be ignored.

if uvesafb use the PMI when PCI BIOS is X, it can get the better work efficience then use the redraw 
method as a fallback when do the panning.

Alan

I am just curious, I want to know what I describe above is right a little,
or wrong about all the aspect.
thanks.

Hi Alan, are you decided to not reply this any? 
But maybe we are still here to wait for your proposal to 
decide the final proper solution to fix the bug in kernel.

Thanks

Hi Alan, are you decided to not reply this any? 

I'm not aware of anythign that needs adding to this. Any logic about
whether PCI BIOS methods can be called belongs in the PCI layer, as does
the necessary logic for marking ROMs executable if asked by a driver.

uvesafb isn't the only user of PCI methods that may be affected and the
PCI layer is the place that should export the method to decide this.

That will also then work sanely cross platform - since in general on non
x86 boxes the method can simply return "no"

Alan