From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v2 3/5] xen/x86: Migrate to boot_info structure Date: Wed, 24 Sep 2014 20:00:09 +0100 Message-ID: <542314B9.1040105@citrix.com> References: <1411579162-27503-1-git-send-email-daniel.kiper@oracle.com> <1411579162-27503-4-git-send-email-daniel.kiper@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XWrnb-00012O-BZ for xen-devel@lists.xenproject.org; Wed, 24 Sep 2014 19:00:23 +0000 In-Reply-To: <1411579162-27503-4-git-send-email-daniel.kiper@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel Kiper , xen-devel@lists.xenproject.org Cc: keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, roy.franz@linaro.org, ning.sun@intel.com, jbeulich@suse.com, ross.philipson@citrix.com, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org List-Id: xen-devel@lists.xenproject.org On 24/09/14 18:19, Daniel Kiper wrote: > Break multiboot (v1) protocol dependency. It means that most of Xen code > (excluding preloader) could be bootloader agnostic and does not need almost > any knowledge about boot protocol. Additionally, we are able to pass all boot > data to __start_xen() in one bucket without any side channels. I do not mention > that we are also able to easily identify boot data in Xen code. > > Here is boot data flow for legacy BIOS platform: > > BIOS -> GRUB -> multiboot[12]* -> __reloc() -> MBD ->-\ > / > ------<------<------<------<------<------<----- > \ > \ > ---> __init_boot_info() -> boot_info_mb -> __start_xen() -> boot_info > / > BIOS ->-/ > > * multiboot2 is not implemented yet. Look for it in later patches. > > Here is boot data flow for EFI platform: > > EFI -> efi_start() -> boot_info_efi -> __start_xen() -> boot_info > > WARNING: ARM build could be broken by this patch. We need to agree boot_info > integration into ARM. Personally I think that it is worth storing all data > from any bootloader and preloader in boot_info on any architecture. This give > a chance to share more code between architectures. However, every architecture > should define its own boot_info (in relevant include/asm directory). Despite > that it looks that some parts of it could be common, e.g. modules data, > command line arguments, boot loader name, EFI data, etc., even if types > would not be the same. So, as it was stated above a lot of code could be > shared among architectures. > > Signed-off-by: Daniel Kiper This patch is basically unreviewable. Please split it up more, starting with misc cleanup such as video.h vs vga.h e.g. start with typedef struct { /* Boot loader name. */ char *boot_loader_name; } boot_info_t; And make some stub declarations which set and retrieve the loader name alone. Then over the course of the following patches, move small bits one at a time into this new structure, such as the command line then memory map, acpi tables, smbios tables etc. This should probably be at least 8 patches. ~Andrew