This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smith joe@settoplinux.org
Joseph Smith wrote:
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smith joe@settoplinux.org
One issue, but other than that it is
Acked-by: Peter Stuge peter@stuge.se
+++ src/northbridge/intel/i82830/i82830_smihandler.c (working copy) @@ -32,7 +32,7 @@ extern unsigned char *mbi; extern u32 mbi_len;
-// #define DEBUG_SMI_I82830 +#define DEBUG_SMI_I82830
Really enable debugging here by default? Should this maybe come from Kconfig somehow. It could e.g. depend on some existing value in Kconfig.
//Peter
On 05/21/2010 12:27 PM, Peter Stuge wrote:
Joseph Smith wrote:
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smithjoe@settoplinux.org
One issue, but other than that it is
Acked-by: Peter Stugepeter@stuge.se
+++ src/northbridge/intel/i82830/i82830_smihandler.c (working copy) @@ -32,7 +32,7 @@ extern unsigned char *mbi; extern u32 mbi_len;
-// #define DEBUG_SMI_I82830 +#define DEBUG_SMI_I82830
Really enable debugging here by default? Should this maybe come from Kconfig somehow. It could e.g. depend on some existing value in Kconfig.
Oops, thanks for the catch. I was actually going to use Kconfig to enable it in the next patch.
On 5/21/10 7:25 AM, Joseph Smith wrote:
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smith joe@settoplinux.org
God catch... However, please don't commit this... it breaks the implementation of MBI, the code should stay completely module agnostic. There should be a solution that works without adding an exception for every module supported (plus, I think the rules are based on the length of the name, so any module 203 and 204 with different names will break your assumptions.
Stefan
On 05/21/2010 03:17 PM, Stefan Reinauer wrote:
On 5/21/10 7:25 AM, Joseph Smith wrote:
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smith joe@settoplinux.org
God catch... However, please don't commit this... it breaks the implementation of MBI, the code should stay completely module agnostic. There should be a solution that works without adding an exception for every module supported (plus, I think the rules are based on the length of the name, so any module 203 and 204 with different names will break your assumptions.
Hmm. I have gone through a bunch of VBT's (type 203) and dozens of Flexaims (type 204) (every version available). All the VBT's (type 203) have the same data structure of a certain size and all the Flexaims (type 204) have the same data structures of a certain size. The sizes of the data structures are different. With the existing code you can get one or the other working but not both. Hence the "type" switch to distinguish between the two data structure sizes.
As far as I have seen for the i830 these are the only two types of MBI modules, VBT's (type 203) and Flexaims (type 204). Unfortunately i don't know what the implementation of MBI is supposed to look like, I am just going by what works (thinking outside the box).
If you have a completely module agnostic way to determine the different data structure sizes, I am all ears.
I think the rules are based on the length of the name, so any module 203 and 204 with different names will break your assumptions.
No, it shouldn't. All VBT's (type 203) only have "VBT" as the name and The way I setup the name for Flexaims (type 204) is based on the name_len so it is actually module agnostic.
On 05/21/2010 04:20 PM, Joseph Smith wrote:
On 05/21/2010 03:17 PM, Stefan Reinauer wrote:
On 5/21/10 7:25 AM, Joseph Smith wrote:
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smith joe@settoplinux.org
God catch... However, please don't commit this... it breaks the implementation of MBI, the code should stay completely module agnostic. There should be a solution that works without adding an exception for every module supported (plus, I think the rules are based on the length of the name, so any module 203 and 204 with different names will break your assumptions.
Hmm. I have gone through a bunch of VBT's (type 203) and dozens of Flexaims (type 204) (every version available). All the VBT's (type 203) have the same data structure of a certain size and all the Flexaims (type 204) have the same data structures of a certain size. The sizes of the data structures are different. With the existing code you can get one or the other working but not both. Hence the "type" switch to distinguish between the two data structure sizes.
As far as I have seen for the i830 these are the only two types of MBI modules, VBT's (type 203) and Flexaims (type 204). Unfortunately i don't know what the implementation of MBI is supposed to look like, I am just going by what works (thinking outside the box).
If you have a completely module agnostic way to determine the different data structure sizes, I am all ears.
I think the rules are based on the length of the name, so any module 203 and 204 with different names will break your assumptions.
No, it shouldn't. All VBT's (type 203) only have "VBT" as the name and The way I setup the name for Flexaims (type 204) is based on the name_len so it is actually module agnostic.
OK, let's get technical on why the switch(mbi_header->type) _HAS_ to happen. First lets take a look at the VBT (type 203). The first data structure is the mbi_header:
|- MBI_GetObjectHeader | |- objnum = 0 | |- header_id = f6f0 | |- attributes = 9750 | |- size = 7c0 | |- name_len = 3 | |- type = 203 | |- header_ext = 0 | |- name[0] = 56
F0 F6 50 97 7C 00 03 FF 03 02 00 00 00 00 00 00
This data structure will always be 16 bytes len. Pretty basic.
Ok, now lets look at the second data structure mbi_header->name_len. The mbi_header reports that this is 3 bytes long but the space allocated will actually always be 16 bytes long:
| |- MBI module 'VBT' found.
56 42 54 00 00 00 00 00 00 00 00 00 00 00 00 00
The first three bytes signify the mbi name "VBT", and the rest of the bytes are just padding, but again the space allocated for mbi_header->name_len will always be 16 bytes long. In this case there needs to be a slight hack to signify the 16 bytes. This situation only happens in VBT (type 203) thus the need for the switch.
Next the:
| |- headerlen = 32
Reports the total byte size of the mbi header. This signify's how many bytes get copied to the bios area.
And last the mbi object area. This is the area of the mbi module minus the headerlen where the real data/code begins. This is reported from the mbi header size:
| |- objectlen = 7c0 or | |- size = 7c0
This signify's how many bytes get copied to the bios area.
-----------------------
Ok now let's take a look at the Flexaims (type 204) and compare the differences.
|- MBI_GetObjectHeader | |- objnum = 1 | |- header_id = f6f0 | |- attributes = 9750 | |- size = 810 | |- name_len = 40 | |- type = 204 | |- header_ext = 0 | |- name[0] = 24 | |- MBI module '$AIM@' found.
F0 F6 50 97 81 00 40 FF 04 02 00 00 00 00 00 00
This data structure will always be 16 bytes len. Pretty basic.
Ok, now lets look at the second data structure mbi_header->name_len. The mbi_header reports that this is 64(0x40) bytes long, which is correct:
24 41 49 4D 40 00 00 00 09 08 00 00 00 03 00 00 01 00 00 00 00 00 05 00 06 00 43 58 2D 32 35 38 00 00 00 00 00 00 00 00 00 00 00 00 02 00 02 00 9C EE 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Next the:
| |- headerlen = 80
Reports the total byte size of the mbi header (16 + 64). This signify's how many bytes get copied to the bios area.
And last the mbi object area. This is the area of the mbi module minus the headerlen where the real data/code begins. This is reported from the mbi header size:
| |- objectlen = 810 or | |- size = 810
This signify's how many bytes get copied to the bios area.
----------------
So hopefully now you can clearly see the main point of the switch(mbi_header->type) is to determine if the module is a VBT (type 203) there needs to be a slight hack to the second data structure mbi_header->name_len to make sure it allocates the full 16 byte area. If not it throws everything else off.
And we can't just say always use 16 bytes for this area for everything because that will throw the Flexaims (type 204) off because the mbi_header->name_len could be different sizes.
Again, I have confirmed this with many different actual raw data of the i830 mbi modules. The only other solution I can come up with is a simple if statement:
if (mbi_header->type == 0x0203) { mbi_header->name_len = 16; }
If that works for you please let me know and I will submit a new patch.
Hello, Let's try this again. This way is as dynamic as possibly possible.
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smith joe@settoplinux.org
On 5/27/10 7:20 AM, Joseph Smith wrote:
Hello, Let's try this again. This way is as dynamic as possibly possible.
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Ok, I tried to understand what's wrong from reading your patch and it seems the name fields need to be properly aligned. I created this patch. Can you try and see if it helps, please?
Stefan
On Thu, 27 May 2010 18:22:27 +0200, Stefan Reinauer stepan@coresystems.de wrote:
On 5/27/10 7:20 AM, Joseph Smith wrote:
Hello, Let's try this again. This way is as dynamic as possibly possible.
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Ok, I tried to understand what's wrong from reading your patch and it seems the name fields need to be properly aligned. I created this patch. Can you try and see if it helps, please?
Sure no problem Stefan, I will be glad to test it as soon as I can :-)
I wasn't really sure what the ALIGN() did exactly, but if it basically does what my take 2 patch does that would be sweet!
On 05/27/2010 12:22 PM, Stefan Reinauer wrote:
On 5/27/10 7:20 AM, Joseph Smith wrote:
Hello, Let's try this again. This way is as dynamic as possibly possible.
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Ok, I tried to understand what's wrong from reading your patch and it seems the name fields need to be properly aligned. I created this patch. Can you try and see if it helps, please?
YAHOO, you are the man Stefan:-) Works Great! Thank you. One minor adjustment, had to add the len for MBI_GetObject because it is part of the for() loop. (new patch attached)
Acked-by: Joseph Smith joe@settoplinux.org
On 05/21/2010 03:17 PM, Stefan Reinauer wrote:
On 5/21/10 7:25 AM, Joseph Smith wrote:
This patch improves the i82830 MBI SMI Handler. It is now able to load Intel vbios VBT and Flexaim modules. Build and boot tested.
Signed-off-by: Joseph Smith joe@settoplinux.org
God catch... However, please don't commit this... it breaks the implementation of MBI, the code should stay completely module agnostic. There should be a solution that works without adding an exception for every module supported (plus, I think the rules are based on the length of the name, so any module 203 and 204 with different names will break your assumptions.
Even the existing code is not "completely module agnostic". There are little + _some_number_ through out the code to adjust the data structure sizes.......