This patch removes all printk format warnings from my LBv3 build (Qemu x86 config). This was tested with gcc 4.1.0 on x86. However, the code at various locations is not 64bit clean at all and will probably break down once we have more than 4GB of RAM. My patch doesn't change that, it is merely intended to fix the warnings for improper printk usage.
Beware: Before the patch is committed, a diff of the output of a LB boot (before and after the patch) needs to be verified. Each change of output should look reasonable, if not, please holler so I can fix the code.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: LinuxBIOSv3/superio/winbond/w83627hf/superio.c =================================================================== --- LinuxBIOSv3/superio/winbond/w83627hf/superio.c (Revision 464) +++ LinuxBIOSv3/superio/winbond/w83627hf/superio.c (Arbeitskopie) @@ -107,7 +107,7 @@ value &= 0xff & hwm_reg_values[i+1]; value |= 0xff & hwm_reg_values[i+2];
- printk(BIOS_SPEW, "base = 0x%04x, reg = 0x%02x, value = 0x%02x\r\n", base, reg,value); + printk(BIOS_SPEW, "base = 0x%04lx, reg = 0x%02x, value = 0x%02x\r\n", base, reg,value);
pnp_write_index(base, reg, value); } Index: LinuxBIOSv3/device/device.c =================================================================== --- LinuxBIOSv3/device/device.c (Revision 464) +++ LinuxBIOSv3/device/device.c (Arbeitskopie) @@ -130,10 +130,10 @@ int i; printk(BIOS_SPEW, "%s: find %s\n", __func__, dev_id_string(id)); for (i = 0; all_constructors[i]; i++) { - printk(BIOS_SPEW, "%s: check all_constructors[i] 0x%lx\n", + printk(BIOS_SPEW, "%s: check all_constructors[i] %p\n", __func__, all_constructors[i]); for (c = all_constructors[i]; c->ops; c++) { - printk(BIOS_SPEW, "%s: cons 0x%lx, cons id %s\n", + printk(BIOS_SPEW, "%s: cons %p, cons id %s\n", __func__, c, dev_id_string(&c->id)); if ((!c->ops) || (!c->ops->constructor)) { continue; @@ -164,7 +164,7 @@ struct constructor *c;
c = find_constructor(id); - printk(BIOS_SPEW, "%s: constructor is 0x%lx\n", __func__, c); + printk(BIOS_SPEW, "%s: constructor is %p\n", __func__, c);
if(c && c->ops && c->ops->constructor) c->ops->constructor(dev, c); @@ -485,7 +485,7 @@ base += size;
printk(BIOS_SPEW, - "%s %02x * [0x%08Lx - 0x%08Lx] %s\n", + "%s %02lx * [0x%08Lx - 0x%08Lx] %s\n", dev_path(dev), resource->index, resource->base, Index: LinuxBIOSv3/device/pnp_device.c =================================================================== --- LinuxBIOSv3/device/pnp_device.c (Revision 464) +++ LinuxBIOSv3/device/pnp_device.c (Arbeitskopie) @@ -87,7 +87,7 @@ { if (!(resource->flags & IORESOURCE_ASSIGNED)) { printk(BIOS_ERR, - "ERROR: %s %02x %s size: 0x%010Lx not assigned\n", + "ERROR: %s %02lx %s size: 0x%010Lx not assigned\n", dev_path(dev), resource->index, resource_type(resource), resource->size); return; @@ -101,7 +101,7 @@ } else if (resource->flags & IORESOURCE_IRQ) { pnp_set_irq(dev, resource->index, resource->base); } else { - printk(BIOS_ERR, "ERROR: %s %02x unknown resource type\n", + printk(BIOS_ERR, "ERROR: %s %02lx unknown resource type\n", dev_path(dev), resource->index); return; } Index: LinuxBIOSv3/device/pci_rom.c =================================================================== --- LinuxBIOSv3/device/pci_rom.c (Revision 464) +++ LinuxBIOSv3/device/pci_rom.c (Arbeitskopie) @@ -47,7 +47,7 @@ return NULL; }
- printk(BIOS_DEBUG, "ROM address for %s = %x\n", dev_path(dev), + printk(BIOS_DEBUG, "ROM address for %s = %lx\n", dev_path(dev), rom_address);
if (!dev->on_mainboard) { @@ -131,7 +131,7 @@ return NULL; // Only one VGA supported. #endif printk(BIOS_DEBUG, - "Copying VGA ROM image from 0x%x to 0x%x, 0x%x bytes\n", + "Copying VGA ROM image from %p to 0x%x, 0x%x bytes\n", rom_header, PCI_VGA_RAM_IMAGE_START, rom_size); memcpy((void *)PCI_VGA_RAM_IMAGE_START, rom_header, rom_size); vga_inited = 1; @@ -139,7 +139,7 @@ #endif } else { printk(BIOS_DEBUG, - "Copying non-VGA ROM image from 0x%x to 0x%x, 0x%x bytes\n", + "Copying non-VGA ROM image from %p to %p, 0x%x bytes\n", rom_header, pci_ram_image_start, rom_size); memcpy(pci_ram_image_start, rom_header, rom_size); pci_ram_image_start += rom_size; Index: LinuxBIOSv3/device/pci_device.c =================================================================== --- LinuxBIOSv3/device/pci_device.c (Revision 464) +++ LinuxBIOSv3/device/pci_device.c (Arbeitskopie) @@ -210,7 +210,7 @@ if (moving == 0) { if (value != 0) { printk(BIOS_DEBUG, - "%s register %02x(%08x), read-only ignoring it\n", + "%s register %02lx(%08lx), read-only ignoring it\n", dev_path(dev), index, value); } resource->flags = 0; @@ -309,7 +309,7 @@ if (moving == 0) { if (value != 0) { printk(BIOS_DEBUG, - "%s register %02x(%08x), read-only ignoring it\n", + "%s register %02lx(%08lx), read-only ignoring it\n", dev_path(dev), index, value); } resource->flags = 0; @@ -456,7 +456,7 @@ /* Make certain the resource has actually been set. */ if (!(resource->flags & IORESOURCE_ASSIGNED)) { printk(BIOS_ERR, - "ERROR: %s %02x %s size: 0x%010Lx not assigned\n", + "ERROR: %s %02lx %s size: 0x%010Lx not assigned\n", dev_path(dev), resource->index, resource_type(resource), resource->size); return; @@ -537,7 +537,7 @@ } else { /* Don't let me think I stored the resource. */ resource->flags &= ~IORESOURCE_STORED; - printk(BIOS_ERR, "ERROR: invalid resource->index %x\n", + printk(BIOS_ERR, "ERROR: invalid resource->index %lx\n", resource->index); } report_resource_stored(dev, resource, ""); Index: LinuxBIOSv3/device/device_util.c =================================================================== --- LinuxBIOSv3/device/device_util.c (Revision 464) +++ LinuxBIOSv3/device/device_util.c (Arbeitskopie) @@ -602,7 +602,7 @@ #endif } printk(BIOS_DEBUG, - "%s %02x <- [0x%010Lx - 0x%010Lx] %s%s%s\n", + "%s %02lx <- [0x%010Lx - 0x%010Lx] %s%s%s\n", dev_path(dev), resource->index, base, end, buf, resource_type(resource), comment); @@ -643,7 +643,7 @@ resource_search_t search, void *gp) { struct device *curdev; - printk(BIOS_SPEW, "%s: mask %x type %x \n", __func__, type_mask, type); + printk(BIOS_SPEW, "%s: mask %lx type %lx \n", __func__, type_mask, type); for (curdev = all_devices; curdev; curdev = curdev->next) { int i; printk(BIOS_SPEW, @@ -656,9 +656,9 @@ for (i = 0; i < curdev->resources; i++) { struct resource *resource = &curdev->resource[i]; printk(BIOS_SPEW, - "%s: dev %s, resource %d, flags %x base 0x%lx size 0x%lx\n", + "%s: dev %s, resource %d, flags %lx base 0x%llx size 0x%llx\n", __func__, curdev->dtsname, i, resource->flags, - (u32) resource->base, (u32) resource->size); + resource->base, resource->size); /* If it isn't the right kind of resource ignore it. */ if ((resource->flags & type_mask) != type) { continue; Index: LinuxBIOSv3/lib/elfboot.c =================================================================== --- LinuxBIOSv3/lib/elfboot.c (Revision 464) +++ LinuxBIOSv3/lib/elfboot.c (Arbeitskopie) @@ -69,9 +69,9 @@ mtype = mem->map[i].type; mstart = unpack_lb64(mem->map[i].start); mend = mstart + unpack_lb64(mem->map[i].size); - printk(BIOS_ERR, " [0x%016lx, 0x%016lx) %s\n", - (unsigned long)mstart, - (unsigned long)mend, + printk(BIOS_ERR, " [0x%016llx, 0x%016llx) %s\n", + mstart, + mend, (mtype == LB_MEM_RAM)?"RAM":"Reserved"); } @@ -101,14 +101,14 @@ printk(BIOS_DEBUG, "Dropping empty segment\n"); continue; } - printk(BIOS_DEBUG, "New segment addr 0x%lx size 0x%lx offset 0x%lx filesize 0x%lx\n", + printk(BIOS_DEBUG, "New segment addr 0x%x size 0x%x offset 0x%x filesize 0x%x\n", phdr[i].p_paddr, phdr[i].p_memsz, phdr[i].p_offset, phdr[i].p_filesz); /* Clean up the values */ size = phdr[i].p_filesz; if (phdr[i].p_filesz > phdr[i].p_memsz) { size = phdr[i].p_memsz; } - printk(BIOS_DEBUG, "(cleaned up) New segment addr 0x%lx size 0x%lx offset 0x%lx\n", + printk(BIOS_DEBUG, "(cleaned up) New segment addr 0x%x size 0x%x offset 0x%x\n", phdr[i].p_paddr, size, phdr[i].p_offset);
/* Verify the memory addresses in the segment are valid */ @@ -150,7 +150,7 @@ /* what the hell is boot_successful? */ //boot_successful();
- printk(BIOS_DEBUG, "Jumping to boot code at 0x%x\n", entry); + printk(BIOS_DEBUG, "Jumping to boot code at %p\n", entry); post_code(0xfe);
/* Jump to kernel */ Index: LinuxBIOSv3/northbridge/intel/i440bxemulation/i440bx.c =================================================================== --- LinuxBIOSv3/northbridge/intel/i440bxemulation/i440bx.c (Revision 464) +++ LinuxBIOSv3/northbridge/intel/i440bxemulation/i440bx.c (Arbeitskopie) @@ -79,7 +79,7 @@ resource->size = ((resource_t) sizek) << 10; resource->flags = IORESOURCE_MEM | IORESOURCE_CACHEABLE | IORESOURCE_FIXED | IORESOURCE_STORED | IORESOURCE_ASSIGNED; - printk(BIOS_DEBUG, "%s: add ram resoource %d bytes\n", __func__, + printk(BIOS_DEBUG, "%s: add ram resoource %lld bytes\n", __func__, resource->size); }
Index: LinuxBIOSv3/util/x86emu/vm86.c =================================================================== --- LinuxBIOSv3/util/x86emu/vm86.c (Revision 464) +++ LinuxBIOSv3/util/x86emu/vm86.c (Arbeitskopie) @@ -543,14 +543,14 @@ eax, ebx, ecx, edx); printk(BIOS_DEBUG, "biosint: ebp 0x%lx esp 0x%lx edi 0x%lx esi 0x%lx\n", ebp, esp, edi, esi); - printk(BIOS_DEBUG, "biosint: ip 0x%x cs 0x%x flags 0x%x\n", + printk(BIOS_DEBUG, "biosint: ip 0x%lx cs 0x%lx flags 0x%lx\n", ip, cs, flags);
// cases in a good compiler are just as good as your own tables. switch (intnumber) { case 0 ... 15: // These are not BIOS service, but the CPU-generated exceptions - printk(BIOS_INFO, "biosint: Oops, exception %u\n", intnumber); + printk(BIOS_INFO, "biosint: Oops, exception %lu\n", intnumber); if (esp < 0x1000) { printk(BIOS_DEBUG, "Stack contents: "); while (esp < 0x1000) { @@ -578,7 +578,7 @@ &ebx, &edx, &ecx, &eax, &flags); break; default: - printk(BIOS_INFO, "BIOSINT: Unsupport int #0x%x\n", + printk(BIOS_INFO, "BIOSINT: Unsupport int #0x%lx\n", intnumber); break; } Index: LinuxBIOSv3/arch/x86/linuxbios_table.c =================================================================== --- LinuxBIOSv3/arch/x86/linuxbios_table.c (Revision 464) +++ LinuxBIOSv3/arch/x86/linuxbios_table.c (Arbeitskopie) @@ -177,8 +177,8 @@ { int entries;
- printk(BIOS_DEBUG, "%s: start 0x%lx size 0x%lx\n", - __func__, (u32)start, (u32)size); + printk(BIOS_DEBUG, "%s: start 0x%llx size 0x%llx\n", + __func__, start, size);
entries = (mem->size - sizeof(*mem))/sizeof(mem->map[0]); mem->map[entries].start = pack_lb64(start); @@ -231,7 +231,7 @@ head->table_checksum = compute_ip_checksum(first_rec, head->table_bytes); head->header_checksum = 0; head->header_checksum = compute_ip_checksum(head, sizeof(*head)); - printk(BIOS_DEBUG,"Wrote LinuxBIOS table at: %p - %p checksum %lx\n", + printk(BIOS_DEBUG,"Wrote LinuxBIOS table at: %p - %p checksum %x\n", head, rec, head->table_checksum); return (unsigned long)rec; } @@ -244,7 +244,7 @@ printk(BIOS_DEBUG, "%s: # entries %d\n", __func__, entries); for(i = 0; i < entries; i++) printk(BIOS_INFO, " #%d: base 0x%x size 0x%x\n", - i, (u32)mem->map[i].start.lo, mem->map[i].size.lo); + i, mem->map[i].start.lo, mem->map[i].size.lo); /* Sort the lb memory ranges */ for(i = 0; i < entries; i++) {
Thanks very much for this. I have a suggestion. Nowadays, for things that are pointers, even if they are not typed as such, I've taken to this style:
On 8/9/07, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
printk(BIOS_SPEW, "base = 0x%04lx, reg = 0x%02x, value = 0x%02x\r\n", base, reg,value);
vvv vvvvvvv printk(BIOS_SPEW, "base = %p, reg = 0x%02x, value = 0x%02x\r\n", (void *) base, reg,value);
printk(BIOS_DEBUG, "ROM address for %s = %lx\n", dev_path(dev), rom_address);
vvvvv printk(BIOS_DEBUG, "ROM address for %s = %p\n", dev_path(dev),(void *) rom_address);
Why do this? It's actually more portable, even across plan 9 and linux. It will probably work correctly in 64 bit mode. And, that rom_address really *is* an address.
I am wondering what people think of this idea. Comments?
On another note, we're experimenting, for various reasons, with -fPIC mode on v3, so as to support execute-in-place. I have a question I would like to toss at some willing experimenters. If anyone is interested I will ask in this forum. Let me know.
thanks again
ron
On Fri, Aug 10, 2007 at 09:31:28AM -0700, ron minnich wrote:
%p
I am wondering what people think of this idea. Comments?
It's the right thing to do. Are there any drawbacks?
On another note, we're experimenting, for various reasons, with -fPIC mode on v3, so as to support execute-in-place. I have a question I would like to toss at some willing experimenters. If anyone is interested I will ask in this forum. Let me know.
This makes sense too. What are the drawbacks? :)
//Peter
Peter Stuge wrote::
On another note, we're experimenting, for various reasons, with -fPIC mode on v3, so as to support execute-in-place. I have a question I would like to toss at some willing experimenters. If anyone is interested I will ask in this forum. Let me know.
This makes sense too. What are the drawbacks? :)
One of four registers gets permanently reserved. The code gets bigger and about 10% slower.
Stefan
On 8/10/07, Stefan Reinauer stefan.reinauer@coresystems.de wrote:
This makes sense too. What are the drawbacks? :)
One of four registers gets permanently reserved. The code gets bigger and about 10% slower.
but linuxbios is infinitely fast, and hence takes 0 time.
110% * 0 is 0. Therefore I prove that it is fine :-)
ron
ron minnich wrote:
On 8/10/07, Stefan Reinauer stefan.reinauer@coresystems.de wrote:
This makes sense too. What are the drawbacks? :)
One of four registers gets permanently reserved. The code gets bigger and about 10% slower.
but linuxbios is infinitely fast, and hence takes 0 time.
110% * 0 is 0. Therefore I prove that it is fine :-)
ron
The speed is no issue in an O(1) environment, sure. The size might be. Award & Co have an 8K bootblock. We're planning on going from 16 to 64k now?
With tomorrows flashes getting bigger, we might not have that problem forever.
On 8/10/07, Stefan Reinauer stepan@coresystems.de wrote:
Award & Co have an 8K bootblock. We're planning on going from 16 to 64k now?
If Award is 8K, then we can argue that smaller bios'es suck more.
I would like to take a chance on betting on flash getting bigger. I just talked to a company yesterday and they said "we can't afford to buy a smaller flash than 32 MB, so we did not worry about making our plan 9 install more compact"
ron
On Friday 10 August 2007 21:13, ron minnich wrote:
On 8/10/07, Stefan Reinauer stepan@coresystems.de wrote:
Award & Co have an 8K bootblock. We're planning on going from 16 to 64k now?
If Award is 8K, then we can argue that smaller bios'es suck more.
I would like to take a chance on betting on flash getting bigger. I just talked to a company yesterday and they said "we can't afford to buy a smaller flash than 32 MB, so we did not worry about making our plan 9 install more compact"
Please consider not all chipsets can handle such large devices. So its sometimes not important if you can buy larger flash devices: You can't use it (on my system the BIOS window is only up to 16MiB, so how to handle 32MiB or more?). But maybe my system is too old...
Juergen
On another note, we're experimenting, for various reasons, with -fPIC mode on v3, so as to support execute-in-place. I have a question I would like to toss at some willing experimenters. If anyone is interested I will ask in this forum. Let me know.
This makes sense too. What are the drawbacks? :)
One of four registers gets permanently reserved.
Two of eight instead of one of eight, or three of eight instead of two of eight when you have a frame pointer (but you shouldn't, it doesn't help anything).
The code gets bigger and about 10% slower.
Yes.
Segher
Please read through the whole mail. The first part is about printf issues, while the second part points out much more important problems with 64bit resources and stuff. Thanks.
On 10.08.2007 18:31, ron minnich wrote:
Thanks very much for this.
I have another patch pending which changes %Lx (L is unspecified for integers, happens to work) to %llx ("official" long long for integers).
Quoting from the printf(3) man page:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
The length modifier Here, `integer conversion' stands for d, i, o, u, x, or X conversion.
hh A following integer conversion corresponds to a signed char or unsigned char argument, or a following n conversion corresponds to a pointer to a signed char argument.
h A following integer conversion corresponds to a short int or unsigned short int argument, or a following n conversion corresponds to a pointer to a short int argument.
l (ell) A following integer conversion corresponds to a long int or unsigned long int argument.
ll (ell-ell). A following integer conversion corresponds to a long long int or unsigned long long int argument.
L A following a, A, e, E, f, F, g, or G conversion corresponds to a long double argument.
z A following integer conversion corresponds to a size_t or ssize_t argument.
t A following integer conversion corresponds to a ptrdiff_t argument. <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
Conclusions: * "L" is for floats only. * "t" or "z" might be handy for resource sizes. * "h" and "hh" for u16 and u8 seem to be unknown to most coders (except in util/dtc) and gcc doesn't warn about them. Should investigate usefulness for us.
I have a suggestion. Nowadays, for things that are pointers, even if they are not typed as such, I've taken to this style:
On 8/9/07, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
./superio/winbond/w83627hf/superio.c:init_hwm() +printk(BIOS_SPEW, "base = 0x%04lx, reg = 0x%02x, value = 0x%02x\r\n", base, reg,value);
vv vvvvvvvprintk(BIOS_SPEW, "base = %p, reg = 0x%02x, value = 0x%02x\r\n", (void *) base, reg,value);
printk(BIOS_DEBUG, "ROM address for %s = %lx\n", dev_path(dev), rom_address);vv vvvvvvvvprintk(BIOS_DEBUG, "ROM address for %s = %p\n", dev_path(dev), (void *) rom_address);
Do I understand this correctly? (void *) conversion and usage of %p for things that are essentially pointers.
Why do this? It's actually more portable, even across plan 9 and linux. It will probably work correctly in 64 bit mode. And, that rom_address really *is* an address.
I like it, but... in the first example it seems this was an error, the base is not an address, but some u16 value. Stefan? Can you tell us what the reason is?
The second example is more correct, but we still face a problem: resource_t is u64 and as long as we don't use long mode, %p will expect 32bit values. However, almost everything being a resource_t (base, size, limit) is essentially a pointer of a pointerdiff. Now how do we handle that?
And while we're at correct typing, shouldn't arch/x86/linuxbios_table.c use resource_t in most places where it uses u64?
I have a lot of open questions: * How do we handle accesses to resources above 4 GB when we confine ourselves to 32bit code? * How do 32bit operating systems handle resources above 4 GB? Should we just avoid locating resources up there? Only do it if unavoidable? Consider a machine with 4 GB of RAM and a graphics card with 512 MB. That combination can be bought today. Now what will happen if video RAM grows to 4 GB? Can we boot such a machine in 32bit mode at all? Should we show a warning? Refuse to boot any non-64bit OS? * Should we have some preference setting which controls 64bit resource allocation? * Is there any point trying to handle 64bit resources on 32bit machines? * Do we have to compile 64bit code for 64bit machines?
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
I have another patch pending which changes %Lx (L is unspecified for integers, happens to work) to %llx ("official" long long for integers).
It doesn't "happen to work", it is implemented so that it works. %llx will not work if you don't change printk.
I agree we should make it compatible to printf.
Conclusions:
- "L" is for floats only.
- "t" or "z" might be handy for resource sizes.
- "h" and "hh" for u16 and u8 seem to be unknown to most coders (except
in util/dtc) and gcc doesn't warn about them. Should investigate usefulness for us.
Then L really makes no sense. Then we want to drop it from the code completely to prevent people from using it with the wrong expectations (like I did, because I figured its meaning out from the code, not the man page of printf)
printk(BIOS_DEBUG, "ROM address for %s = %p\n", dev_path(dev), (void *) rom_address);
Do I understand this correctly? (void *) conversion and usage of %p for things that are essentially pointers.
What's unclear?
Why do this? It's actually more portable, even across plan 9 and linux. It will probably work correctly in 64 bit mode. And, that rom_address really *is* an address.
I like it, but... in the first example it seems this was an error, the base is not an address, but some u16 value. Stefan? Can you tell us what the reason is?
rom_address a 16bit value? Which part of the code? Sorry I lost you.
Are you talking about IO accesses which have a 16bit address space?
The second example is more correct, but we still face a problem: resource_t is u64 and as long as we don't use long mode, %p will expect 32bit values. However, almost everything being a resource_t (base, size, limit) is essentially a pointer of a pointerdiff. Now how do we handle that?
Which second example?
And while we're at correct typing, shouldn't arch/x86/linuxbios_table.c use resource_t in most places where it uses u64?
Interesting idea. You might be correct. resource_t should be a u64 in fact. It might be something different due to alignment requirements of the pci structures
I have a lot of open questions:
- How do we handle accesses to resources above 4 GB when we confine
ourselves to 32bit code?
We don't confine ourselfes. There's PAE
- How do 32bit operating systems handle resources above 4 GB? Should we
just avoid locating resources up there? Only do it if unavoidable? Consider a machine with 4 GB of RAM and a graphics card with 512 MB. That combination can be bought today. Now what will happen if video RAM grows to 4 GB? Can we boot such a machine in 32bit mode at all? Should we show a warning? Refuse to boot any non-64bit OS?
32bit OSes can not handle system resources above 4GB except with tricks (PAE). That is why 32bit OSes are dying out and are not being used anymore in environments where resources above 4GB happen.
If you run a 32bit OS on a machine with 4G of RAM you simply can not be helped. Linux starts having severe performance problems in 32bit mode when you cross the 1G border. Which basically every machine does today, except lowend hobbyist stuff and embedded systems.
- Should we have some preference setting which controls 64bit resource
allocation?
We do have a CONFIG variable for that, and it packs resources into 32bit per default. This default will change at some point because a 3G PCI hole in 4G space makes only little sense. This is done on some machines so that _in theory_ you _could_ run a 32bit OS.
- Is there any point trying to handle 64bit resources on 32bit machines?
Not that I would know of. PCI resources can prefer to go to 64bit space, but on a 32bit machine that means you poke them out of visibility.
- Do we have to compile 64bit code for 64bit machines?
No. Thanks to PAE we can access address space above 4G too. But in fact we want to go 64bit mode. The really broken thing with x86_64's long mode is that it does not work without paging enabled. That makes the effort to use it in the bios harder than the gain.
For an OS and other high level applications like that things are different though.
On 15.08.2007 18:15, Stefan Reinauer wrote:
Carl-Daniel Hailfinger wrote:
I have another patch pending which changes %Lx (L is unspecified for integers, happens to work) to %llx ("official" long long for integers).
It doesn't "happen to work", it is implemented so that it works. %llx will not work if you don't change printk.
Quoting from v3/lib/vtxprintf.c: if (*fmt == 'h' || *fmt == 'l' || *fmt == 'L') { qualifier = *fmt; ++fmt; if (*fmt == 'l') { qualifier = 'L'; ++fmt; }
%llx already works.
I agree we should make it compatible to printf.
OK, I'll send followup patches.
Conclusions:
- "L" is for floats only.
- "t" or "z" might be handy for resource sizes.
- "h" and "hh" for u16 and u8 seem to be unknown to most coders (except
in util/dtc) and gcc doesn't warn about them. Should investigate usefulness for us.
Then L really makes no sense. Then we want to drop it from the code completely to prevent people from using it with the wrong expectations (like I did, because I figured its meaning out from the code, not the man page of printf)
Will send patches.
printk(BIOS_DEBUG, "ROM address for %s = %p\n", dev_path(dev), (void *) rom_address);
Do I understand this correctly? (void *) conversion and usage of %p for things that are essentially pointers.
What's unclear?
Nothing. I was just trying to make sure I had understood Ron correctly.
Why do this? It's actually more portable, even across plan 9 and linux. It will probably work correctly in 64 bit mode. And, that rom_address really *is* an address.
I like it, but... in the first example it seems this was an error, the base is not an address, but some u16 value. Stefan? Can you tell us what the reason is?
rom_address a 16bit value? Which part of the code? Sorry I lost you.
Are you talking about IO accesses which have a 16bit address space?
I was talking about ./superio/winbond/w83627hf/superio.c:init_hwm() where Ron assumed that base was a pointer, however init_hwm defines it as u16. Which is somewhat interesting because w83627hf_init() calls init_hwm(res0->base + HWM_INDEX_PORT) where res0->base is resource_t aka u64. That means we have an implicit truncation from u64 to u16. Is that intentional?
The second example is more correct, but we still face a problem: resource_t is u64 and as long as we don't use long mode, %p will expect 32bit values. However, almost everything being a resource_t (base, size, limit) is essentially a pointer of a pointerdiff. Now how do we handle that?
Which second example?
Reading that text again, it seems I was totally unclear.
./device/pci_rom.c:pci_rom_probe(): printk(BIOS_DEBUG, "ROM address for %s = %lx\n", dev_path(dev), rom_address);
I was wondering whether rom_address (which is essentially a pointer) should be resource_t. Also, how should we printk anything that's resource_t? Everything with type resource_t is essentially a pointer. %p won't work unless we compile for 64bit. Maybe introduce %P or some other ugly hack.
And while we're at correct typing, shouldn't arch/x86/linuxbios_table.c use resource_t in most places where it uses u64?
Interesting idea. You might be correct. resource_t should be a u64 in fact. It might be something different due to alignment requirements of the pci structures
It is typedef'ed to u64, so there aren't any differences. But when we use resource_t instead of u64, we still have the freedom to define it to u32 for some architectures.
I have a lot of open questions:
- How do we handle accesses to resources above 4 GB when we confine
ourselves to 32bit code?
We don't confine ourselfes. There's PAE
Which we don't want to handle in LB.
- How do 32bit operating systems handle resources above 4 GB? Should we
just avoid locating resources up there? Only do it if unavoidable? Consider a machine with 4 GB of RAM and a graphics card with 512 MB. That combination can be bought today. Now what will happen if video RAM grows to 4 GB? Can we boot such a machine in 32bit mode at all? Should we show a warning? Refuse to boot any non-64bit OS?
32bit OSes can not handle system resources above 4GB except with tricks (PAE). That is why 32bit OSes are dying out and are not being used anymore in environments where resources above 4GB happen.
The interesting question is whether we can set up resources in a way that allows 32bit OSes to boot, e.g. by "moving" RAM to above 4G and making it inaccessible that way.
If you run a 32bit OS on a machine with 4G of RAM you simply can not be helped. Linux starts having severe performance problems in 32bit mode when you cross the 1G border. Which basically every machine does today, except lowend hobbyist stuff and embedded systems.
Yes, but we should allow it to boot if this is technically possible.
- Should we have some preference setting which controls 64bit resource
allocation?
We do have a CONFIG variable for that, and it packs resources into 32bit per default. This default will change at some point because a 3G PCI hole in 4G space makes only little sense. This is done on some machines so that _in theory_ you _could_ run a 32bit OS.
There is not such CONFIG variable in v3.
- Is there any point trying to handle 64bit resources on 32bit machines?
Not that I would know of. PCI resources can prefer to go to 64bit space, but on a 32bit machine that means you poke them out of visibility.
OK.
- Do we have to compile 64bit code for 64bit machines?
No. Thanks to PAE we can access address space above 4G too. But in fact
Ugly.
we want to go 64bit mode. The really broken thing with x86_64's long mode is that it does not work without paging enabled. That makes the effort to use it in the bios harder than the gain.
Seems we are caught between a rock and a hard place. PAE is ugly, long mode is difficult.
For an OS and other high level applications like that things are different though.
Yes.
Regards, Carl-Daniel
* Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net [070820 22:50]:
I was talking about ./superio/winbond/w83627hf/superio.c:init_hwm() where Ron assumed that base was a pointer, however init_hwm defines it as u16. Which is somewhat interesting because w83627hf_init() calls init_hwm(res0->base + HWM_INDEX_PORT) where res0->base is resource_t aka u64. That means we have an implicit truncation from u64 to u16. Is that intentional?
Hm this looks like a shortcoming of our device model. The superio base address is 0x2e or 0x4e
./device/pci_rom.c:pci_rom_probe(): printk(BIOS_DEBUG, "ROM address for %s = %lx\n", dev_path(dev), rom_address);
I was wondering whether rom_address (which is essentially a pointer) should be resource_t.
In the current PCI standard the rom address is always a 32bit value. In theory ROMs might be mapped above 4G in future standards. It's nothing we need to handle too soon though in that scope. (ie. when that happens we are probably not going to have x86 pcbios option roms anymore)
Also, how should we printk anything that's resource_t? Everything with type resource_t is essentially a pointer. %p won't work unless we compile for 64bit. Maybe introduce %P or some other ugly hack.
Hm. probably %P is a good idea. or %lp to stay consistent?
We don't confine ourselfes. There's PAE
Which we don't want to handle in LB.
We already do.
32bit OSes can not handle system resources above 4GB except with tricks (PAE). That is why 32bit OSes are dying out and are not being used anymore in environments where resources above 4GB happen.
The interesting question is whether we can set up resources in a way that allows 32bit OSes to boot, e.g. by "moving" RAM to above 4G and making it inaccessible that way.
That's what happens in the default configuration if you dont set CONFIG_PCI_64BIT_PREF_MEM to 1.
If you run a 32bit OS on a machine with 4G of RAM you simply can not be helped. Linux starts having severe performance problems in 32bit mode when you cross the 1G border. Which basically every machine does today, except lowend hobbyist stuff and embedded systems.
Yes, but we should allow it to boot if this is technically possible.
It is allowed and possible.
we want to go 64bit mode. The really broken thing with x86_64's long mode is that it does not work without paging enabled. That makes the effort to use it in the bios harder than the gain.
Seems we are caught between a rock and a hard place. PAE is ugly, long mode is difficult.
Not only difficult, it is impossible for most of our code. For long mode you need paging. For paging you need page tables. These need to be in RAM. RAM is not initialized yet.
It might not be a problem when switching to long mode before entering stage2. That would disallow our calls into stage0/1 functions that we currently do. Might be a low price to pay.
Then there's the next question: What do we do with OSes and payloads that have no 64bit entry point? Go back to 32bit mode?
Stefan
-
Carl-Daniel Hailfinger -
Juergen Beisert -
Peter Stuge -
ron minnich -
Segher Boessenkool -
Stefan Reinauer -
Stefan Reinauer