On Mon, Sep 8, 2014 at 9:12 AM, Ian Campbell Ian.Campbell@citrix.com wrote:
On Sun, 2014-09-07 at 20:54 -0700, Roy Franz wrote:
@@ -618,6 +731,32 @@ ENTRY(lookup_processor_type) mov x0, #0 ret
+ENTRY(efi_xen_start)
/** Turn off cache and MMU as XEN expects. EFI enables them, but alsoIt's just "Xen", not "XEN" (throughout).
OK.
* mandates a 1:1 (unity) VA->PA mapping, so we can turn off the* MMU while executing EFI code before entering XEN.* The EFI loader calls this to start XEN.*/Last time there was a handy comment about preserving x0 here. I think it would also (or perhaps instead) be good to have a comment before the entry giving the "prototype" for this function, since it is called from C.
OK, I'll add that.
mov x20, x0bl __flush_dcache_allic ialluisTwo too many spaces before iall.
diff --git a/xen/common/efi/Makefile b/xen/common/efi/Makefile index 4313a4e..fea712e 100644 --- a/xen/common/efi/Makefile +++ b/xen/common/efi/Makefile @@ -1,5 +1,5 @@ CFLAGS += -fshort-wchar
+ifneq ($(XEN_TARGET_ARCH),arm64)
This does still build on arm32, right?
I suppose the discussion on patch #1 will have some knock on effects here.
Most of the stuff should be common with arm32, although I haven't tried it. The main thing missing for arm32 is the PE/COFF header. I'm not intending to build the EFI stuff for arm32, but it's possible I've borked the build - I forgot to try that on this patchset.
diff --git a/xen/include/asm-arm/arm64/efibind.h b/xen/include/asm-arm/arm64/efibind.h new file mode 100644 index 0000000..2b0bf40 --- /dev/null +++ b/xen/include/asm-arm/arm64/efibind.h
diff --git a/xen/include/asm-arm/efi-boot.h b/xen/include/asm-arm/efi-boot.h new file mode 100644 index 0000000..91a637e --- /dev/null +++ b/xen/include/asm-arm/efi-boot.h
+static void __init *lookup_fdt_config_table(EFI_SYSTEM_TABLE *sys_table) +{
- const EFI_GUID fdt_guid = DEVICE_TREE_GUID;
- EFI_CONFIGURATION_TABLE *tables;
- void *fdt = NULL;
- int i;
- tables = sys_table->ConfigurationTable;
- for ( i = 0; i < sys_table->NumberOfTableEntries; i++ )
- {
if ( match_guid(&tables[i].VendorGuid, &fdt_guid) ){fdt = tables[i].VendorTable;break;}- }
- return fdt;
+} +static EFI_STATUS __init efi_get_memory_map(void **map,
Missing a blank line after the previous function, here and elsewhere. I see you've gone from 2 blanks to 0 since last time ;-)
I'll add the proper number of blank lines :)
- fdt_val64 = cpu_to_fdt64((u64)(unsigned long)sys_table);
(uintptr_t) is arguably more correct than (unsigned long) for this purpose. and again below.
Yup, I'll fix that.
- status = fdt_setprop(fdt, node, "linux,uefi-system-table",
&fdt_val64, sizeof(fdt_val64));- if ( status )
goto fdt_set_fail;- fdt_val64 = cpu_to_fdt64((u64)(unsigned long)memory_map);
- status = fdt_setprop(fdt, node, "linux,uefi-mmap-start",
&fdt_val64, sizeof(fdt_val64));[...]
++static void __init efi_arch_post_exit_boot(void) +{
- efi_xen_start((unsigned long)fdt);
You might as well make the pseudoprototype be a pointer if that is what fdt actually is.
OK. That will save a cast, and the value in the register is the same.
+static void __init efi_arch_load_addr_check(EFI_LOADED_IMAGE *loaded_image) +{
- if ( (unsigned long)loaded_image->ImageBase & ((1 << 20) - 1) )
blexit(L"Xen must be loaded at a 2MByte boundary.");This isn't true any more, Xen only needs to be at a 4K boundary now.
I will update this (along with the alignment requested in the PE/COFF header) and give it a spin.
Looking good, all of the above is pretty minor stuff, thanks!
Ian.
I'm hoping this is in good enough shape for 4.5 - I should be able to address all the feedback received so far quite quickly. Should I wait for more feedback, or prepare an updated patchset addressing the feedback so far?
Thanks, Roy