[PATCH] ia64: Don't call SAL < 3.2 for extended config space

[PATCH] ia64: Don't call SAL < 3.2 for extended config space

[Matthew Wilcox]

On Mon, Oct 12, 2009 at 09:56:28AM -0400, Brad Spengler wrote:
> > So how about we go back to adding that check ... this does require that
> > SGI's 750 machine reports its SAL revision correctly.  Could you send
> > the dmesg?
> 
> The dmesg of the working kernel:
> http://grsecurity.net/~spender/dmesg.txt

Thanks, that's reporting SAL 3.0, so it will be correctly caught by
this patch:

---

From: Matthew Wilcox <willy@linux.intel.com>
Subject: Require SAL 3.2 in order to do extended config space ops

We had assumed that SAL firmware would return an error if it didn't
understand extended config space.  Unfortunately, the SAL on the SGI 750
doesn't do that, it panics the machine.  So, condition the extended PCI
config space accesses on SAL revision 3.2.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 7de76dd..61363cc 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -56,10 +56,13 @@ int raw_pci_read(unsigned int seg, unsigned int bus, unsigned int devfn,
 	if ((seg | reg) <= 255) {
 		addr = PCI_SAL_ADDRESS(seg, bus, devfn, reg);
 		mode = 0;
-	} else {
+	} else if (sal_revision >= SAL_VERSION_CODE(3,2))
 		addr = PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg);
 		mode = 1;
+	} else {
+		return -EINVAL;
 	}
+
 	result = ia64_sal_pci_config_read(addr, mode, len, &data);
 	if (result != 0)
 		return -EINVAL;
@@ -80,9 +83,11 @@ int raw_pci_write(unsigned int seg, unsigned int bus, unsigned int devfn,
 	if ((seg | reg) <= 255) {
 		addr = PCI_SAL_ADDRESS(seg, bus, devfn, reg);
 		mode = 0;
-	} else {
+	} else if (sal_revision >= SAL_VERSION_CODE(3,2))
 		addr = PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg);
 		mode = 1;
+	} else {
+		return -EINVAL;
 	}
 	result = ia64_sal_pci_config_write(addr, mode, len, value);
 	if (result != 0)

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] ia64: Don't call SAL < 3.2 for extended config space

[Brad Spengler]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

> +	} else if (sal_revision >= SAL_VERSION_CODE(3,2))

Looks like a missing ending { here ^
> +	} else if (sal_revision >= SAL_VERSION_CODE(3,2))
and here ^

I'll test it to confirm it fixes the problem.

-Brad

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] ia64: Don't call SAL < 3.2 for extended config space

[Brad Spengler]

[-- Attachment #1: Type: text/plain, Size: 1925 bytes --]

I've confirmed that the below patch (with the syntax fixes already 
mentioned) resolves the issue on the SGI 750.

-Brad

> From: Matthew Wilcox <willy@linux.intel.com>
> Subject: Require SAL 3.2 in order to do extended config space ops
> 
> We had assumed that SAL firmware would return an error if it didn't
> understand extended config space.  Unfortunately, the SAL on the SGI 750
> doesn't do that, it panics the machine.  So, condition the extended PCI
> config space accesses on SAL revision 3.2.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> 
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index 7de76dd..61363cc 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -56,10 +56,13 @@ int raw_pci_read(unsigned int seg, unsigned int bus, unsigned int devfn,
>  	if ((seg | reg) <= 255) {
>  		addr = PCI_SAL_ADDRESS(seg, bus, devfn, reg);
>  		mode = 0;
> -	} else {
> +	} else if (sal_revision >= SAL_VERSION_CODE(3,2))
>  		addr = PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg);
>  		mode = 1;
> +	} else {
> +		return -EINVAL;
>  	}
> +
>  	result = ia64_sal_pci_config_read(addr, mode, len, &data);
>  	if (result != 0)
>  		return -EINVAL;
> @@ -80,9 +83,11 @@ int raw_pci_write(unsigned int seg, unsigned int bus, unsigned int devfn,
>  	if ((seg | reg) <= 255) {
>  		addr = PCI_SAL_ADDRESS(seg, bus, devfn, reg);
>  		mode = 0;
> -	} else {
> +	} else if (sal_revision >= SAL_VERSION_CODE(3,2))
>  		addr = PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg);
>  		mode = 1;
> +	} else {
> +		return -EINVAL;
>  	}
>  	result = ia64_sal_pci_config_write(addr, mode, len, value);
>  	if (result != 0)
> 
> -- 
> Matthew Wilcox				Intel Open Source Technology Centre
> "Bill, look, we understand that you're interested in selling us this
> operating system, but compare it to ours.  We can't possibly take such
> a retrograde step."

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] ia64: Don't call SAL < 3.2 for extended config space

[Jesse Barnes]

Ah, the SGI 750.  Brings back memories.

I've dropped the ball on this one though; maybe Tony already picked it
up?  Tony?

Thanks,
Jesse

On Tue, 13 Oct 2009 00:58:08 -0400
spender@grsecurity.net (Brad Spengler) wrote:

> I've confirmed that the below patch (with the syntax fixes already 
> mentioned) resolves the issue on the SGI 750.
> 
> -Brad
> 
> > From: Matthew Wilcox <willy@linux.intel.com>
> > Subject: Require SAL 3.2 in order to do extended config space ops
> > 
> > We had assumed that SAL firmware would return an error if it didn't
> > understand extended config space.  Unfortunately, the SAL on the
> > SGI 750 doesn't do that, it panics the machine.  So, condition the
> > extended PCI config space accesses on SAL revision 3.2.
> > 
> > Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> > 
> > diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> > index 7de76dd..61363cc 100644
> > --- a/arch/ia64/pci/pci.c
> > +++ b/arch/ia64/pci/pci.c
> > @@ -56,10 +56,13 @@ int raw_pci_read(unsigned int seg, unsigned int
> > bus, unsigned int devfn, if ((seg | reg) <= 255) {
> >  		addr = PCI_SAL_ADDRESS(seg, bus, devfn, reg);
> >  		mode = 0;
> > -	} else {
> > +	} else if (sal_revision >= SAL_VERSION_CODE(3,2))
> >  		addr = PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg);
> >  		mode = 1;
> > +	} else {
> > +		return -EINVAL;
> >  	}
> > +
> >  	result = ia64_sal_pci_config_read(addr, mode, len, &data);
> >  	if (result != 0)
> >  		return -EINVAL;
> > @@ -80,9 +83,11 @@ int raw_pci_write(unsigned int seg, unsigned int
> > bus, unsigned int devfn, if ((seg | reg) <= 255) {
> >  		addr = PCI_SAL_ADDRESS(seg, bus, devfn, reg);
> >  		mode = 0;
> > -	} else {
> > +	} else if (sal_revision >= SAL_VERSION_CODE(3,2))
> >  		addr = PCI_SAL_EXT_ADDRESS(seg, bus, devfn, reg);
> >  		mode = 1;
> > +	} else {
> > +		return -EINVAL;
> >  	}
> >  	result = ia64_sal_pci_config_write(addr, mode, len, value);
> >  	if (result != 0)
> > 
> > -- 
> > Matthew Wilcox				Intel Open Source
> > Technology Centre "Bill, look, we understand that you're interested
> > in selling us this operating system, but compare it to ours.  We
> > can't possibly take such a retrograde step."


-- 
Jesse Barnes, Intel Open Source Technology Center

RE: [PATCH] ia64: Don't call SAL < 3.2 for extended config space

[Luck, Tony]

> I've dropped the ball on this one though; maybe Tony already picked it
> up?  Tony?

Yes.  It went in to Linus' tree with the git pull I sent on Monday (just
prior to -rc6).

Commit: adcd74034 ...

-Tony

Re: [PATCH] ia64: Don't call SAL < 3.2 for extended config space

[Jesse Barnes]

On Wed, 4 Nov 2009 09:20:13 -0800
"Luck, Tony" <tony.luck@intel.com> wrote:

> > I've dropped the ball on this one though; maybe Tony already picked
> > it up?  Tony?
> 
> Yes.  It went in to Linus' tree with the git pull I sent on Monday
> (just prior to -rc6).
> 
> Commit: adcd74034 ...

Great, thanks for confirming.

-- 
Jesse Barnes, Intel Open Source Technology Center