From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH v2 20/23] x86: add multiboot2 protocol support for EFI platforms Date: Tue, 11 Aug 2015 11:23:27 -0400 Message-ID: <20150811152327.GA32231__37154.6167800718$1439306708$gmane$org@l.oracle.com> References: <1437402558-7313-1-git-send-email-daniel.kiper@oracle.com> <1437402558-7313-21-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.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZPBP5-0001Al-PR for xen-devel@lists.xenproject.org; Tue, 11 Aug 2015 15:23:51 +0000 Content-Disposition: inline In-Reply-To: <1437402558-7313-21-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 Cc: jgross@suse.com, grub-devel@gnu.org, wei.liu2@citrix.com, keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, roy.franz@linaro.org, ning.sun@intel.com, david.vrabel@citrix.com, jbeulich@suse.com, phcoder@gmail.com, xen-devel@lists.xenproject.org, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org List-Id: xen-devel@lists.xenproject.org > +run_bs: > + push %rax > + push %rdi > + > + /* Initialize BSS (no nasty surprises!). */ > + lea __bss_start(%rip),%rdi > + lea __bss_end(%rip),%rcx > + sub %rdi,%rcx > + shr $3,%rcx > + xor %eax,%eax > + rep stosq This means we are using the %es segment. The multiboot1 is pretty clear that it should cover up to 4GB. Would it be worth adding a comment about that? > + > + pop %rdi > + > + /* > + * IN: %rdi - EFI ImageHandle, %rsi - EFI SystemTable. > + * OUT: %rax - multiboot2.mem_lower. Do not get this value from > + * MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag. It could be bogus on > + * EFI platforms. > + */ > + call efi_multiboot2 However the function prototype for efi_multiboot2 is 'void', not 'int' - so it would not return anything! > + > + /* Convert multiboot2.mem_lower to bytes/16. */ Should we check to make sure it is valid? With those weird machines you seem to have run into I am not actually sure what a valid value is :-( .. snip.. > diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c > index c5ae369..d30fe89 100644 > --- a/xen/arch/x86/efi/stub.c > +++ b/xen/arch/x86/efi/stub.c > @@ -13,6 +13,11 @@ struct efi __read_mostly efi = { > .smbios3 = EFI_INVALID_TABLE_ADDR > }; > > +void __init efi_multiboot2(void) unsigned int __init .. > +{ > + /* TODO: Fail if entered! */ And maybe just 'return 0'; ? > +}