Messages in this thread |  | | | From | Andi Kleen <> | | Subject | Re: [PATCH 8/9] x86, efi: EFI boot stub support | | Date | Thu, 11 Aug 2011 11:09:35 -0700 |
| |
Matt Fleming <matt@console-pimps.org> writes: > + > + status = efi_call_phys3(sys_table->boottime->allocate_pool, > + EFI_LOADER_DATA, sizeof(*idt), > + (void **)&idt); > + if (status != EFI_SUCCESS) > + goto fail; > + > + idt->size = 0; > + idt->address = 0; > + > + status = make_boot_params(boot_params, image, handle); > + if (status != EFI_SUCCESS) > + goto fail; > + > + memset((char *)gdt->address, 0x0, gdt->size); > + desc = (u64 *)gdt->address; > + > + /* > + * 4Gb - (0x100000*0x1000 = 4Gb) > + * base address=0 > + * code read/exec > + * granularity=4096, 386 (+5th nibble of limit) > + */ > + desc[2] = 0x00cf9a000000ffff; > + > + /* > + * 4Gb - (0x100000*0x1000 = 4Gb) > + * base address=0 > + * data read/write > + * granularity=4096, 386 (+5th nibble of limit) > + */ > + desc[3] = 0x00cf92000000ffff; > + > + /* Task segment value */ > + desc[4] = 0x0080890000000000;
The code would benefit from more variables/defines and less magic numbers.
I assume this is all virtual, otherwise it would be really scary.
> + asm volatile ("lidt %0" :: "m" (*idt)); > + asm volatile ("lgdt %0" :: "m" (*gdt)); > + > + asm volatile("cli");
::: "memory" to avoid moving
> + > + return boot_params; > +fail: > + return NULL;
Does the caller actually something useful here for NULL? Better to have messages when any of this fails.
> +int memcmp(const void *s1, const void *s2, size_t len) > +{ > + u8 diff; > + asm("repe; cmpsb; setnz %0" > + : "=qm" (diff), "+D" (s1), "+S" (s2), "+c" (len));
This doesn't describe to gcc that the inline assembler reads s1 and s2. At the minimum add a memory clobber.
> + > +/** > + * strlen - Find the length of a string > + * @s: The string to be sized > + */ > +size_t strlen(const char *s) > +{ > + const char *sc; > + > + for (sc = s; *sc != '\0'; ++sc) > + /* nothing */; > + return sc - s; > +}
Why not just link in/#include lib/string.c ?
-Andi -- ak@linux.intel.com -- Speaking for myself only
|  |