On Thu, Mar 25, 2010 at 10:39 AM, Stefan Reinauer
<stepan@coresystems.de> wrote:
On 3/25/10 5:18 PM, Myles Watson wrote:
>
>> On Thu, Mar 25, 2010 at 7:56 AM, Myles Watson <
mylesgw@gmail.com> wrote:
>>
>>
>>> It seems like others must have this problem of needing to force a binary
>>> blob into a struct.
>>>
>> Hi, I've only been following this at a distance. I assume this all
>> happens after memory is turned on. Why not just have a function to
>> fill the struct given the binary blob? I'm starting to worry about all
>> the tricks people want to play with gcc.
>>
> Memory is on. The blob is in flash, and the first bit of the blob is the
> header with the length field.
>
> We do a memcpy from flash to the struct,
You're right, since it gets copied to memory with the rest of coreboot_ram.
but after it's
copied it's not treated as a struct anymore.
Except when you patch it?
> but first we have to cast it to a
> header so we can read out the length to copy.
Which makes me think if there are not other ways to determine the size
of an array. Maybe sizeof(AmlCode) ?
I tried that. It doesn't work because it's an incomplete type.
> Gcc doesn't like the cast, so
> we just declare that we have the header stored in flash.
>
> One way to solve it would be to copy the header first, then read the length
> and copy the rest...
>
>
That's more copying than my variant with the extra pointer. Since we
copied (or uncompressed) the dsdt from flash to ram already before, and
are going to make another copy, I suggest rather keeping an extra 4
bytes, than copying the complete header for no apparent reason...
The reason I object to the void* method was that it just masked the problem so that gcc couldn't spot it. Casting to void* and back to a struct seems equivalent to just having it declared two different ways.
Copying the header first works (boot tested), and if you want to save the redundant copy, we could start after the header for the second copy.
Index: svn/src/mainboard/amd/serengeti_cheetah_fam10/acpi_tables.c
===================================================================
--- svn.orig/src/mainboard/amd/serengeti_cheetah_fam10/acpi_tables.c
+++ svn/src/mainboard/amd/serengeti_cheetah_fam10/acpi_tables.c
@@ -47,7 +47,7 @@ static void dump_mem(u32 start, u32 end)
#endif
extern const acpi_header_t AmlCode;
-extern const acpi_header_t AmlCode_ssdt;
+extern const unsigned char AmlCode_ssdt[];
#if CONFIG_ACPI_SSDTX_NUM >= 1
extern const acpi_header_t AmlCode_ssdt2;
@@ -264,8 +264,10 @@ unsigned long write_acpi_tables(unsigned
current = ( current + 0x0f) & -0x10;
printk(BIOS_DEBUG, "ACPI: * SSDT at %lx\n", current);
ssdt = (acpi_header_t *)current;
- current += AmlCode_ssdt.length;
- memcpy((void *)ssdt, &AmlCode_ssdt, AmlCode_ssdt.length);
+ memcpy(ssdt, &AmlCode_ssdt[0], sizeof(acpi_header_t));
+ current += ssdt->length;
+ printk(BIOS_DEBUG, "ssdt->length %x\n", ssdt->length);
+ memcpy(ssdt, &AmlCode_ssdt[0], ssdt->length);
//Here you need to set value in pci1234, sblk and sbdn in get_bus_conf.c
update_ssdt((void*)ssdt);
/* recalculate checksum */