[LinuxBIOS] [PATCH] fix resource end in K8 amdk8/northbridge.c

[Rudolf Marek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

This patch fixes the resource end in amdk8/northbridge.c

Signed-off-by: Rudolf Marek

Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFG/ZYo3J9wPJqZRNURAgtlAJ9JB2xTVLkl3kaQ37JY2JxZ7PFyeQCfUoH6
kwz4YzvLlUpaeNCUjswi0b4=
=uWpJ
-----END PGP SIGNATURE-----

[Carl-Daniel Hailfinger]

On 29.09.2007 02:02, Rudolf Marek wrote:
> This patch fixes the resource end in amdk8/northbridge.c
>
> Signed-off-by: Rudolf Marek

If this fixes resources going backwards, it is
Acked-by: Carl-Daniel Hailfinger

Can somebody please commit?
> Rudolf
> Index: src/northbridge/amd/amdk8/northbridge.c
>
> --- src/northbridge/amd/amdk8/northbridge.c	(revision 2776)
> +++ src/northbridge/amd/amdk8/northbridge.c	(working copy)
> @@ -562,7 +562,7 @@
>    base  |= (resource->base >> 8) & 0xffffff00;
>    base  |= 3;
>    limit &= 0x00000048;
> limit |= ((resource->base + resource->size) >> 8) & 0xffffff00;
> limit |= (resource_end(resource) >> 8) & 0xffffff00;
> limit |= (resource->index & 3) << 4;
> limit |= (nodeid & 7);
> f1_write_config32(reg + 0x4, limit);

[Uwe Hermann]

On Sat, Sep 29, 2007 at 02:02:48AM +0200, Rudolf Marek wrote:
> Index: src/northbridge/amd/amdk8/northbridge.c
>
> --- src/northbridge/amd/amdk8/northbridge.c	(revision 2776)
> +++ src/northbridge/amd/amdk8/northbridge.c	(working copy)
> @@ -562,7 +562,7 @@
>    base  |= (resource->base >> 8) & 0xffffff00;
>    base  |= 3;
>    limit &= 0x00000048;
> limit |= ((resource->base + resource->size) >> 8) & 0xffffff00;
> limit |= (resource_end(resource) >> 8) & 0xffffff00;
> limit |= (resource->index & 3) << 4;
> limit |= (nodeid & 7);
> f1_write_config32(reg + 0x4, limit);

What exactly is broken in the current code and how does it show? How can
we test the effect of the fix?

Thanks, Uwe.

[Carl-Daniel Hailfinger]

On 01.10.2007 22:43, Uwe Hermann wrote:
> On Sat, Sep 29, 2007 at 02:02:48AM +0200, Rudolf Marek wrote:
> > Index: src/northbridge/amd/amdk8/northbridge.c
> >
> > --- src/northbridge/amd/amdk8/northbridge.c	(revision 2776)
> > +++ src/northbridge/amd/amdk8/northbridge.c	(working copy)
> > @@ -562,7 +562,7 @@
> >    base  |= (resource->base >> 8) & 0xffffff00;
> >    base  |= 3;
> >    limit &= 0x00000048;
> > limit |= ((resource->base + resource->size) >> 8) & 0xffffff00;
> > limit |= (resource_end(resource) >> 8) & 0xffffff00;
> > limit |= (resource->index & 3) << 4;
> > limit |= (nodeid & 7);
> > f1_write_config32(reg + 0x4, limit);
>
> What exactly is broken in the current code and how does it show? How can
> we test the effect of the fix?

Resources go backwards (end before start). That should be corrected with
the patch.

I don't have the hardware to verify.

Carl-Daniel

[Rudolf Marek]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello

Sorry for the delay.

Uwe Hermann wrote:
> On Sat, Sep 29, 2007 at 02:02:48AM +0200, Rudolf Marek wrote:
> > Index: src/northbridge/amd/amdk8/northbridge.c
> >
> > --- src/northbridge/amd/amdk8/northbridge.c	(revision 2776)
> > +++ src/northbridge/amd/amdk8/northbridge.c	(working copy)
> > @@ -562,7 +562,7 @@
> >    base  |= (resource->base >> 8) & 0xffffff00;
> >    base  |= 3;
> >    limit &= 0x00000048;
> > limit |= ((resource->base + resource->size) >> 8) & 0xffffff00;
> > limit |= (resource_end(resource) >> 8) & 0xffffff00;
> > limit |= (resource->index & 3) << 4;
> > limit |= (nodeid & 7);
> > f1_write_config32(reg + 0x4, limit);
>
> What exactly is broken in the current code and how does it show? How can
> we test the effect of the fix?

Well I reverted the patch and have forgotten :/ Without the patch my machine
suffers random lockup, when DMA from SATA/IDE hit the 0xC0000-0xCFFFF RAM memory.

BIOS-e820: 0000000000001000 - 00000000000a0000 (usable)
 BIOS-e820: 00000000000c0000 - 00000000000f0000 (usable)
 BIOS-e820: 0000000000100000 - 0000000040000000 (usable)

This is my memory layout.

Without the patch, when VGA is used you will get:
(I'm using my dump ressource util, which I have posted some time ago)

DRAM map: #0 0x0000000000 - 0x003fffffff Access: R/W  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:0
DRAM map: #1 0x0000000000 - 0x0000ffffff Access: /  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:1
DRAM map: #2 0x0000000000 - 0x0000ffffff Access: /  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:2
DRAM map: #3 0x0000000000 - 0x0000ffffff Access: /  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:3
DRAM map: #4 0x0000000000 - 0x0000ffffff Access: /  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:4
DRAM map: #5 0x0000000000 - 0x0000ffffff Access: /  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:5
DRAM map: #6 0x0000000000 - 0x0000ffffff Access: /  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:6
DRAM map: #7 0x0000000000 - 0x0000ffffff Access: /  IntlvEN:0x0 IntlvSEL:0x0
Dstnode:7
MMIO map: #0 0x0000000000 - 0x000000ffff Access: /     Dstnode:0 DstLink 0

MMIO map: #1 0x0000000000 - 0x000000ffff Access: /     Dstnode:0 DstLink 0

MMIO map: #2 0x0000000000 - 0x000000ffff Access: /     Dstnode:0 DstLink 0

MMIO map: #3 0x0000000000 - 0x000000ffff Access: /     Dstnode:0 DstLink 0

MMIO map: #4 0x0000000000 - 0x000000ffff Access: /     Dstnode:0 DstLink 0

MMIO map: #5 0x00000a0000 - 0x00000cffff Access: R/W     Dstnode:0 DstLink 0

Mote the region size! it is 64kb larger than it should be...

MMIO map: #6 0x00e0000000 - 0x00f02fffff Access: R/W     Dstnode:0 DstLink 0

MMIO map: #7 0x00c0000000 - 0x00d7ffffff Access: R/W     Dstnode:0 DstLink 0

MMIO map: #0  0x001000 - 0x004fff Access: R/W   VGA Dstnode:0 DstLink 0

MMIO map: #1  0x000000 - 0x000fff Access: /    Dstnode:0 DstLink 0

MMIO map: #2  0x000000 - 0x000fff Access: /    Dstnode:0 DstLink 0

MMIO map: #3  0x000000 - 0x000fff Access: /    Dstnode:0 DstLink 0

So with the patch applied, it gets correct again:

MMIO map: #5 0x00000a0000 - 0x00000bffff Access: R/W     Dstnode:0 DstLink 0

Btw maybe you can take the util and put it to some SVN repo, it has proven useful ;)

Thanks,

Rudolf
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHD+PR3J9wPJqZRNURAoHHAJ9wK4KkzmnRP1i3nq62KPeONehFbgCgxmri
vtbxueWcDK4mviJHBQFMs40=
=y1yi
-----END PGP SIGNATURE-----

[Marc Jones]

Rudolf Marek wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hello
>
> Sorry for the delay.
>
> Uwe Hermann wrote:
> > On Sat, Sep 29, 2007 at 02:02:48AM +0200, Rudolf Marek wrote:
> > > Index: src/northbridge/amd/amdk8/northbridge.c
> > >
> > > --- src/northbridge/amd/amdk8/northbridge.c	(revision 2776)
> > > +++ src/northbridge/amd/amdk8/northbridge.c	(working copy)
> > > @@ -562,7 +562,7 @@
> > >    base  |= (resource->base >> 8) & 0xffffff00;
> > >    base  |= 3;
> > >    limit &= 0x00000048;
> > > limit |= ((resource->base + resource->size) >> 8) & 0xffffff00;
> > > limit |= (resource_end(resource) >> 8) & 0xffffff00;
> > > limit |= (resource->index & 3) << 4;
> > > limit |= (nodeid & 7);
> > > f1_write_config32(reg + 0x4, limit);
> >
> > What exactly is broken in the current code and how does it show? How can
> > we test the effect of the fix?
>
> Well I reverted the patch and have forgotten :/ Without the patch my machine
> suffers random lockup, when DMA from SATA/IDE hit the 0xC0000-0xCFFFF RAM memory.

...
> MMIO map: #5 0x00000a0000 - 0x00000cffff Access: R/W     Dstnode:0 DstLink 0
>
> Mote the region size! it is 64kb larger than it should be...

...
> So with the patch applied, it gets correct again:
>
> MMIO map: #5 0x00000a0000 - 0x00000bffff Access: R/W     Dstnode:0 DstLink 0
>
> Btw maybe you can take the util and put it to some SVN repo, it has proven useful ;)
>
> Thanks,
>
> Rudolf

I have looked this over and agree with Rudolf. The VGA area can't go 
into the 0xC0000 ROM shadow area.

resource->base = 0xa0000;
    resource->size = 0x20000;
...
    limit |= ((resource->base + resource->size) >> 8) & 0xffffff00;

The problem is that the size is not base 0 aligned. The following would 
also fix the bug and makes it more clear why it failed.

limit |= ((resource->base + (resource->size - 1)) >> 8) & 0xffffff00;

I think Rudolf's use of resource_end() is better than the inline math.

Marc

[Uwe Hermann]

On Fri, Oct 12, 2007 at 03:36:36PM -0600, Marc Jones wrote:
> > Well I reverted the patch and have forgotten :/ Without the patch my 
> > machine
> > suffers random lockup, when DMA from SATA/IDE hit the 0xC0000-0xCFFFF RAM 
> > memory.
>
> ...
> > MMIO map: #5 0x00000a0000 - 0x00000cffff Access: R/W     Dstnode:0 DstLink 
> > 0
> > Mote the region size! it is 64kb larger than it should be...
>
> ...
> > So with the patch applied, it gets correct again:
> > MMIO map: #5 0x00000a0000 - 0x00000bffff Access: R/W     Dstnode:0 DstLink 
> > 0
> > Btw maybe you can take the util and put it to some SVN repo, it has proven 
> > useful ;)
> > Thanks,
> > Rudolf
>
> I have looked this over and agree with Rudolf. The VGA area can't go into 
> the 0xC0000 ROM shadow area.

Yep, I verified the patch on my MS-7260 today, and it works as expected.

Committed in r2853.

Uwe.