[Xen-devel] [PATCH] gdbsx: prefer privcmd character device
Prefer using the character device over the proc file if the character
device exists.
CC: Elena Ufimtseva <elena.ufimts...@oracle.com>
CC: Ian Jackson <ian.jack...@eu.citrix.com>
CC: Stefano Stabellini <stefano.stabell...@eu.citrix.com>
CC: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Doug Goldstein <car...@cardoe.com>
---
So this was originally submitted with 9c89dc95201 and 7d418eab3b6 and
was rejected since the goal was to convert gdbsx to use libxc but that
hasn't happened. /dev/xen/privcmd should be preferred and this change
makes that happen. It would be nice if we landed this with the plan
to convert gdbsx happening when it happens.
---
tools/debugger/gdbsx/xg/xg_main.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/debugger/gdbsx/xg/xg_main.c
b/tools/debugger/gdbsx/xg/xg_main.c
index 7ebf91435b..cc640d1d82 100644
--- a/tools/debugger/gdbsx/xg/xg_main.c
+++ b/tools/debugger/gdbsx/xg/xg_main.c
@@ -126,9 +126,11 @@ xg_init()
int flags, saved_errno;
XGTRC("E\n");
-if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
-perror("Failed to open /proc/xen/privcmd\n");
-return -1;
+if ((_dom0_fd=open("/dev/xen/privcmd", O_RDWR)) == -1) {
+if ((_dom0_fd=open("/proc/xen/privcmd", O_RDWR)) == -1) {
+perror("Failed to open /dev/xen/privcmd or /proc/xen/privcmd\n");
+return -1;
+}
}
/* Although we return the file handle as the 'xc handle' the API
* does not specify / guarentee that this integer is in fact
--
2.13.6
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] Xen 4.6 Hypervisor Fails to Boot and is Hanged in "Turning on Paging"
On 10/27/17 7:41 AM, Julien Grall wrote: > Hi, > > On 27/10/17 08:23, Srini wrote: >> Xen 4.6 Hypervisor Fails to Boot and is Hanged in "Turning on Paging": > > Again, as I already said on embedded-pv-devel ML, why are you using Xen > 4.6? It was released 2 years ago and supported anymore. I have an idea why see below.. > > You should try Xen 4.9 first and report if it still does not work. Agreed. >> EVM : TI DRA7XX OMAP5 >> Xen versions - 4.6 >> Uboot version - 2016.05 >> Kernel Version - 4.4 >> Dev Host - 14.04 14.04 here would imply he's running some Ubuntu for ARM build. Ubuntu 14.04 shipped with Xen 4.6. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x86/boot: fix early error display
On 10/27/17 1:37 AM, Jan Beulich wrote: >>>> On 26.10.17 at 23:05, <car...@cardoe.com> wrote: >> On 10/18/17 4:50 AM, Jan Beulich wrote: >>>>>> On 17.10.17 at 23:41, <car...@cardoe.com> wrote: >>>> From: David Esler <drumandst...@gmail.com> >>>> >>>> In 9180f5365524 a change was made to the send_chr function to take in >>>> C-strings and print out a character at a time until a NULL was >>>> encountered. However there is no code to increment the current character >>>> position resulting in an endless loop of the first character. This adds >>>> a simple increment. >>> >>> This description is not accurate (it should have changed together with >>> the change to how you fix the issue) - with VGA the increment does >>> happen. Hence "display" in the title is perhaps also at least misleading. >>> I would be fine to adjust both while committing (and then adding my >>> R-b), but feel free to propose an alternative. >> >> Jan, >> >> Can you queue this for 4.9 as well? That's where we ran into the issue >> in the first place. > > That how I did understand it, so I've queued this already, but for > it to become eligible to applying to 4.9 it first needs to pass the > push gate on master. > > Jan > Of course. I didn't mean to imply this should jump any existing process. I had just realized that I didn't explicitly mention versions in my original email and I just wanted to make sure I was explicit instead of assuming. Thanks again. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x86/boot: fix early error display
On 10/18/17 4:50 AM, Jan Beulich wrote: >>>> On 17.10.17 at 23:41, <car...@cardoe.com> wrote: >> From: David Esler <drumandst...@gmail.com> >> >> In 9180f5365524 a change was made to the send_chr function to take in >> C-strings and print out a character at a time until a NULL was >> encountered. However there is no code to increment the current character >> position resulting in an endless loop of the first character. This adds >> a simple increment. > > This description is not accurate (it should have changed together with > the change to how you fix the issue) - with VGA the increment does > happen. Hence "display" in the title is perhaps also at least misleading. > I would be fine to adjust both while committing (and then adding my > R-b), but feel free to propose an alternative. Jan, Can you queue this for 4.9 as well? That's where we ran into the issue in the first place. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 10/24/17 5:20 PM, Daniel Kiper wrote: > On Tue, Oct 24, 2017 at 10:40:26PM +0100, Andrew Cooper wrote: >> On 24/10/2017 22:11, Daniel Kiper wrote: >>> On Tue, Oct 24, 2017 at 09:22:20PM +0100, Andrew Cooper wrote: >>>> On 24/10/17 21:08, Daniel Kiper wrote: >>>>> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >>>>>> The EFI multiboot2 entry point currently requires EFI BootServices to >>>>>> not have been exited however the header currently tells the boot >>>>>> loader that Xen optionally supports EFI BootServices having been exited. >>>>>> With this change Xen properly advertises that EFI must not be exited >>>>>> allowing the boot loader to report an error that it cannot boot Xen if >>>>>> it is unable to meet its needs. >>>>>> >>>>>> Signed-off-by: Doug Goldstein <car...@cardoe.com> >>>>>> --- >>>>>> >>>>>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as >>>>>> staging. I am trying to get multiboot2 support for iPXE and upstream >>>>>> is concerned that leaving EFI BootServices enabled will not be >>>>>> compatible with their aims to support Secure Boot. So when I build >>>>> Hmmm... What are exact arguments for that? How do they implement e.g. >>>>> chain loading then? What about the shim support? >>>>> >>>>>> my iPXE without support for passing on Boot Services, Xen will be >>>>>> loaded by iPXE but then it will fall down with "ERR: Bootloader >>>>>> shutdown EFI x64 boot services!" implying that this is required. By >>>>>> having Xen expose in its header that its required it allows me to >>>>>> handle the situation gracefully in iPXE. >>>>>> >>>>>> To quote the multiboot2 spec exact: >>>>>> >>>>>> "This tag indicates that payload supports starting without terminating >>>>>> boot services." >>>>>> >>>>>> Unfortunately the spec is a bit vague and how I am reading it is: >>>>>> - no tag = exit boot services in the boot loader >>>>>> - tag present marked optional = boot loader can or cannot exit boot >>>>>> services >>>>>> - tag present marked required = boot loader cannot exit boot services >>>>> NACK, please take a look at section 3.1.4, Multiboot2 information request >>>>> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the >>>>> bootloader >>>>> than you think. >>>> The meaning of tag, if understood by Grub, is "don't exit boot services >>>> before passing control". >>> Yep. >>> >>>> The tag is currently marked as optional, which means Grub is free to >>>> ignore it if it doesn't understand it, resulting in EBS being called >>>> before passing control. >>> Yep, but you are forgetting about legacy BIOS platforms with old GRUB2. >>> Right now it is possible to boot Xen via Multiboot2 in such configs. >>> If you set this flag to REQUIRED then old GRUB2 on BIOS platforms will >>> not boot Xen in such cases. If we do not care about that then OK. However, >>> then I would request additional line or so to the commit message saying >>> that such configs are broken deliberately because... >> >> Such older cases wouldn't boot either, because Xen would bail out saying >> "I didn't retain BS like I need". > > Nope, you should remember that legacy entry point (__start) will be used then. But based on what you said the spec says the boot loader only has to understand the tag and not actually do anything with it. So marking it required wouldn't affect anything cause Grub would understand the tag in the BIOS boot case and not do anything with it. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] libxc: don't fail domain creation when unpacking initrd fails
On 10/16/17 11:48 AM, Andrew Cooper wrote: > On 16/10/17 17:19, Jan Beulich wrote: >>>>> On 16.10.17 at 17:45, <ian.jack...@eu.citrix.com> wrote: > > I've been bitten by this issue several times before, and a fix would be > nice. Same here. > > IMO, the toolstack should not be making assumptions about the initrd, > and shouldn't be touching it. It is the users responsibility to provide > an initrd which its kernel can read. > > Furthermore, leaving the decompression to the kernel reduces the dom0 > attack surface. This. So many this. I do recall bringing this up at a meet up a while back and the concern was breaking someone's workflow. Maybe we can put a warning that the behavior is deprecated for X number of releases before deleting it. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 10/24/17 3:22 PM, Andrew Cooper wrote: > On 24/10/17 21:08, Daniel Kiper wrote: >> On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >>> The EFI multiboot2 entry point currently requires EFI BootServices to >>> not have been exited however the header currently tells the boot >>> loader that Xen optionally supports EFI BootServices having been exited. >>> With this change Xen properly advertises that EFI must not be exited >>> allowing the boot loader to report an error that it cannot boot Xen if >>> it is unable to meet its needs. >>> >>> Signed-off-by: Doug Goldstein <car...@cardoe.com> >>> --- >>> >>> This should likely be applied against Xen 4.9 and Xen 4.10 as well as >>> staging. I am trying to get multiboot2 support for iPXE and upstream >>> is concerned that leaving EFI BootServices enabled will not be >>> compatible with their aims to support Secure Boot. So when I build >> Hmmm... What are exact arguments for that? How do they implement e.g. >> chain loading then? What about the shim support? >> >>> my iPXE without support for passing on Boot Services, Xen will be >>> loaded by iPXE but then it will fall down with "ERR: Bootloader >>> shutdown EFI x64 boot services!" implying that this is required. By >>> having Xen expose in its header that its required it allows me to >>> handle the situation gracefully in iPXE. >>> >>> To quote the multiboot2 spec exact: >>> >>> "This tag indicates that payload supports starting without terminating >>> boot services." >>> >>> Unfortunately the spec is a bit vague and how I am reading it is: >>> - no tag = exit boot services in the boot loader >>> - tag present marked optional = boot loader can or cannot exit boot services >>> - tag present marked required = boot loader cannot exit boot services >> NACK, please take a look at section 3.1.4, Multiboot2 information request >> in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the >> bootloader >> than you think. > > The meaning of tag, if understood by Grub, is "don't exit boot services > before passing control". > > The tag is currently marked as optional, which means Grub is free to > ignore it if it doesn't understand it, resulting in EBS being called > before passing control. > > Xen cannot cope with with EBS having been called, so must not be passed > control under those circumstances. > > Doug's patch marks it as non-optional which, by that section above, > requires Grub to fail with an error rather than proceeding, if it does > not understand the tag. > > > By my reading, Doug's patch looks correct. > > How does your interpretation of the spec differ? > > ~Andrew > So I've been sitting here reading it for a bit. I'm guessing what Daniel is arguing is that the spec says that the boot loader MUST understand a tag if its marked as required and does not have to understand it if its marked as optional. The next sentence then seems to be a total escape hatch because it seems to imply that the boot loader doesn't have to respect any tag regardless of its required or optional settings. Which seems to defeat the purpose of having any info requests at all. And results in no guarantees that if your binary requires something that it will get it before being executed. And therefore requires a binary to support all cases always. If that's truly the case you are arguing for Daniel then this whole spec really has too big of a loophole to be safely considered as useful. I know that's a bit harsh but as more tags are added over time the matrix of required support will snowball. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 10/24/17 3:08 PM, Daniel Kiper wrote: > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >> >> Unfortunately the spec is a bit vague and how I am reading it is: >> - no tag = exit boot services in the boot loader >> - tag present marked optional = boot loader can or cannot exit boot services >> - tag present marked required = boot loader cannot exit boot services > > NACK, please take a look at section 3.1.4, Multiboot2 information request > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader > than you think. In fact since there is some confusion here then could you possibly expand some of the sections with examples to help clarify? -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
On 10/24/17 3:08 PM, Daniel Kiper wrote: > On Tue, Oct 24, 2017 at 02:40:41PM -0500, Doug Goldstein wrote: >> The EFI multiboot2 entry point currently requires EFI BootServices to >> not have been exited however the header currently tells the boot >> loader that Xen optionally supports EFI BootServices having been exited. >> With this change Xen properly advertises that EFI must not be exited >> allowing the boot loader to report an error that it cannot boot Xen if >> it is unable to meet its needs. >> >> Signed-off-by: Doug Goldstein <car...@cardoe.com> >> --- >> >> This should likely be applied against Xen 4.9 and Xen 4.10 as well as >> staging. I am trying to get multiboot2 support for iPXE and upstream >> is concerned that leaving EFI BootServices enabled will not be >> compatible with their aims to support Secure Boot. So when I build > > Hmmm... What are exact arguments for that? How do they implement e.g. > chain loading then? What about the shim support? Look they have concerns about it. As we've talked about this in the past and I encourage you communicate with them. You are the author of the multiboot2 spec. I'm just trying to do my best to PXE boot Xen on EFI systems and make all upstreams (Xen & iPXE) happy. >> >> Unfortunately the spec is a bit vague and how I am reading it is: >> - no tag = exit boot services in the boot loader >> - tag present marked optional = boot loader can or cannot exit boot services >> - tag present marked required = boot loader cannot exit boot services > > NACK, please take a look at section 3.1.4, Multiboot2 information request > in Multiboot2 spec. OPTIONAL/REQUIRED has different meaning for the bootloader > than you think. > I still don't see any issue with my interpretation based on what you pointed me to. There's a hole here with what Xen asks for of the boot loader to do. The boot loader is told that Xen optionally supports the boot loader not exiting boot services when in fact Xen requires the boot loader to not exit boot services. Somehow we need to convey this to the boot loader. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/boot: fix MB2 header to require EFI BS
The EFI multiboot2 entry point currently requires EFI BootServices to not have been exited however the header currently tells the boot loader that Xen optionally supports EFI BootServices having been exited. With this change Xen properly advertises that EFI must not be exited allowing the boot loader to report an error that it cannot boot Xen if it is unable to meet its needs. Signed-off-by: Doug Goldstein <car...@cardoe.com> --- This should likely be applied against Xen 4.9 and Xen 4.10 as well as staging. I am trying to get multiboot2 support for iPXE and upstream is concerned that leaving EFI BootServices enabled will not be compatible with their aims to support Secure Boot. So when I build my iPXE without support for passing on Boot Services, Xen will be loaded by iPXE but then it will fall down with "ERR: Bootloader shutdown EFI x64 boot services!" implying that this is required. By having Xen expose in its header that its required it allows me to handle the situation gracefully in iPXE. To quote the multiboot2 spec exact: "This tag indicates that payload supports starting without terminating boot services." Unfortunately the spec is a bit vague and how I am reading it is: - no tag = exit boot services in the boot loader - tag present marked optional = boot loader can or cannot exit boot services - tag present marked required = boot loader cannot exit boot services In the future I would like to add support to Xen to allow it to run without boot services but presently that support isn't there. --- xen/arch/x86/boot/head.S | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 9cc35da558..f76c2c0664 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -98,8 +98,8 @@ multiboot2_header_start: 0, /* Number of the lines - no preference. */ \ 0 /* Number of bits per pixel - no preference. */ -/* Request that ExitBootServices() not be called. */ -mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL) +/* Require that ExitBootServices() not be called. */ +mb2ht_init MB2_HT(EFI_BS), MB2_HT(REQUIRED) /* EFI64 Multiboot2 entry point. */ mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \ -- 2.13.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] x86/boot: fix early error display
On 10/18/17 4:50 AM, Jan Beulich wrote: >>>> On 17.10.17 at 23:41, <car...@cardoe.com> wrote: >> From: David Esler <drumandst...@gmail.com> >> >> In 9180f5365524 a change was made to the send_chr function to take in >> C-strings and print out a character at a time until a NULL was >> encountered. However there is no code to increment the current character >> position resulting in an endless loop of the first character. This adds >> a simple increment. > > This description is not accurate (it should have changed together with > the change to how you fix the issue) - with VGA the increment does > happen. Hence "display" in the title is perhaps also at least misleading. > I would be fine to adjust both while committing (and then adding my > R-b), but feel free to propose an alternative. David and I are both ok with you changing the wording as necessary. I apologize for not updating the commit message. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/2] x86/boot: fix early error display
From: David Esler <drumandst...@gmail.com> In 9180f5365524 a change was made to the send_chr function to take in C-strings and print out a character at a time until a NULL was encountered. However there is no code to increment the current character position resulting in an endless loop of the first character. This adds a simple increment. Reviewed-by: Doug Goldstein <car...@cardoe.com> Signed-off-by: David Esler <drumandst...@gmail.com> --- xen/arch/x86/boot/head.S | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index fd6fc337fe..9cc35da558 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -173,10 +173,11 @@ not_multiboot: .Lget_vtb: mov sym_esi(vga_text_buffer),%edi .Lsend_chr: -mov (%esi),%bl -test%bl,%bl# Terminate on '\0' sentinel +lodsb +test%al,%al# Terminate on '\0' sentinel je .Lhalt mov $0x3f8+5,%dx # UART Line Status Register +mov %al,%bl 2: in %dx,%al test$0x20,%al # Test THR Empty flag je 2b @@ -185,7 +186,7 @@ not_multiboot: out %al,%dx# Send a character over the serial line test%edi,%edi # Is the VGA text buffer available? jz .Lsend_chr -movsb # Write a character to the VGA text buffer +stosb # Write a character to the VGA text buffer mov $7,%al stosb # Write an attribute to the VGA text buffer jmp .Lsend_chr -- 2.13.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/2] x86/boot: rename send_chr to print_err
From: David Esler <drumandst...@gmail.com> The send_chr function sends an entire C-string and not one character and doesn't necessarily just send it over the serial UART anymore so rename it to print_err so that its closer in name to what it does. Reviewed-by: Doug Goldstein <car...@cardoe.com> Signed-off-by: David Esler <drumandst...@gmail.com> --- xen/arch/x86/boot/head.S | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 9cc35da558..475c678f2c 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -161,7 +161,7 @@ not_multiboot: */ add $sym_offs(.Lbad_ldr_nbs),%esi # Error message xor %edi,%edi # No VGA text buffer -jmp .Lsend_chr +jmp .Lprint_err .Lmb2_efi_ia_32: /* * Here we are on EFI IA-32 platform. Then reliable vga_text_buffer zap is @@ -169,10 +169,10 @@ not_multiboot: */ add $sym_offs(.Lbad_efi_msg),%esi # Error message xor %edi,%edi # No VGA text buffer -jmp .Lsend_chr +jmp .Lprint_err .Lget_vtb: mov sym_esi(vga_text_buffer),%edi -.Lsend_chr: +.Lprint_err: lodsb test%al,%al# Terminate on '\0' sentinel je .Lhalt @@ -185,11 +185,11 @@ not_multiboot: mov %bl,%al out %al,%dx# Send a character over the serial line test%edi,%edi # Is the VGA text buffer available? -jz .Lsend_chr +jz .Lprint_err stosb # Write a character to the VGA text buffer mov $7,%al stosb # Write an attribute to the VGA text buffer -jmp .Lsend_chr +jmp .Lprint_err .Lhalt: hlt jmp .Lhalt -- 2.13.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/boot: rename send_chr to print_err
On 10/13/17 2:40 AM, Jan Beulich wrote: >>>> On 12.10.17 at 22:56, <andrew.coop...@citrix.com> wrote: >> On 12/10/2017 21:50, Doug Goldstein wrote: >>> From: David Esler <drumandst...@gmail.com> >>> >>> The send_chr function sends an entire C-string and not one character and >>> doesn't necessarily just send it over the serial UART anymore so rename >>> it to print_err so that its closer in name to what it does. >>> >>> Reviewed-by: Doug Goldstein <car...@cardoe.com> >>> Signed-off-by: David Esler <drumandst...@gmail.com> >> >> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com> >> >> This should also be included in 4.10 IMO. > > I'm not convinced - this is merely a cleanup style patch, the more > that the label really serves dual purpose and hence the original > name isn't all that wrong anyway. > > Jan > I purposefully broke it out so that it could be discussed. Prior to the commit I referenced in patch 1 the function sent out a character. Now it requires the supplied data to be a C-string (NULL terminated) so I would hope that you could agree that at else the "chr" part of "send_chr" is incorrect and should likely be "str" or "err". But again this patch isn't really important its more to try to make the label match the behavior of the code below the label and if its unwanted then it can be dropped. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/boot: fix early error display
> On Oct 12, 2017, at 4:27 PM, Daniel Kiper <daniel.ki...@oracle.com> wrote: > >> On Thu, Oct 12, 2017 at 03:50:06PM -0500, Doug Goldstein wrote: >> From: David Esler <drumandst...@gmail.com> >> >> In 9180f5365524 a change was made to the send_chr function to take in >> C-strings and print out a character at a time until a NULL was >> encountered. However there is no code to increment the current character >> position resulting in an endless loop of the first character. This adds >> a simple increment. >> >> Reviewed-by: Doug Goldstein <car...@cardoe.com> >> Signed-off-by: David Esler <drumandst...@gmail.com> >> --- >> xen/arch/x86/boot/head.S | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S >> index fd6fc337fe..f48bbbd2e5 100644 >> --- a/xen/arch/x86/boot/head.S >> +++ b/xen/arch/x86/boot/head.S >> @@ -174,6 +174,7 @@ not_multiboot: >> mov sym_esi(vga_text_buffer),%edi >> .Lsend_chr: >> mov (%esi),%bl >> +inc %esi >> test%bl,%bl# Terminate on '\0' sentinel >> je .Lhalt >> mov $0x3f8+5,%dx # UART Line Status Register > > I have a feeling that you have tested this on machine without > VGA text buffer available. Then your fix works. However, if VGA > text buffer is available then %esi is increased twice. First time > by inc here and once again by movsb below. So, I think that the > issue have to be fixed in a bit different way. > > Daniel Correct. It was an EFI machine with serial only where I saw it in action. David has put together a new change and I’ll get it submitted tomorrow. — Doug ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 1/2] x86/boot: fix early error display
From: David Esler <drumandst...@gmail.com> In 9180f5365524 a change was made to the send_chr function to take in C-strings and print out a character at a time until a NULL was encountered. However there is no code to increment the current character position resulting in an endless loop of the first character. This adds a simple increment. Reviewed-by: Doug Goldstein <car...@cardoe.com> Signed-off-by: David Esler <drumandst...@gmail.com> --- xen/arch/x86/boot/head.S | 1 + 1 file changed, 1 insertion(+) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index fd6fc337fe..f48bbbd2e5 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -174,6 +174,7 @@ not_multiboot: mov sym_esi(vga_text_buffer),%edi .Lsend_chr: mov (%esi),%bl +inc %esi test%bl,%bl# Terminate on '\0' sentinel je .Lhalt mov $0x3f8+5,%dx # UART Line Status Register -- 2.13.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 2/2] x86/boot: rename send_chr to print_err
From: David Esler <drumandst...@gmail.com> The send_chr function sends an entire C-string and not one character and doesn't necessarily just send it over the serial UART anymore so rename it to print_err so that its closer in name to what it does. Reviewed-by: Doug Goldstein <car...@cardoe.com> Signed-off-by: David Esler <drumandst...@gmail.com> --- xen/arch/x86/boot/head.S | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index f48bbbd2e5..22348b1bbe 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -161,7 +161,7 @@ not_multiboot: */ add $sym_offs(.Lbad_ldr_nbs),%esi # Error message xor %edi,%edi # No VGA text buffer -jmp .Lsend_chr +jmp .Lprint_err .Lmb2_efi_ia_32: /* * Here we are on EFI IA-32 platform. Then reliable vga_text_buffer zap is @@ -169,10 +169,10 @@ not_multiboot: */ add $sym_offs(.Lbad_efi_msg),%esi # Error message xor %edi,%edi # No VGA text buffer -jmp .Lsend_chr +jmp .Lprint_err .Lget_vtb: mov sym_esi(vga_text_buffer),%edi -.Lsend_chr: +.Lprint_err: mov (%esi),%bl inc %esi test%bl,%bl# Terminate on '\0' sentinel @@ -185,11 +185,11 @@ not_multiboot: mov %bl,%al out %al,%dx# Send a character over the serial line test%edi,%edi # Is the VGA text buffer available? -jz .Lsend_chr +jz .Lprint_err movsb # Write a character to the VGA text buffer mov $7,%al stosb # Write an attribute to the VGA text buffer -jmp .Lsend_chr +jmp .Lprint_err .Lhalt: hlt jmp .Lhalt -- 2.13.5 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH for-4.10] travis: disable UBSAN
On 10/10/17 4:15 AM, Wei Liu wrote: > The stock compiler in travis doesn't support -fsanitize=undefined. > > Signed-off-by: Wei Liu <wei.l...@citrix.com> > --- > Cc: Doug Goldstein <car...@cardoe.com> > --- > xen/tools/kconfig/allrandom.config | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xen/tools/kconfig/allrandom.config > b/xen/tools/kconfig/allrandom.config > index c7753ac4ad..76f74320b5 100644 > --- a/xen/tools/kconfig/allrandom.config > +++ b/xen/tools/kconfig/allrandom.config > @@ -1,3 +1,4 @@ > # Explicit option choices not subject to regular RANDCONFIG > > CONFIG_GCOV_FORMAT_AUTODETECT=y > +CONFIG_UBSAN=n > Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] travis: install ghostscript
On 6/27/17 4:53 AM, Wei Liu wrote: > Signed-off-by: Wei Liu <wei.l...@citrix.com> > --- > Cc: Doug Goldstein <car...@cardoe.com> > --- > .travis.yml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/.travis.yml b/.travis.yml > index 9121fcca40..f93dd6868e 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -71,6 +71,7 @@ addons: > - g++-5 > - seabios > - checkpolicy > +- ghostscript > # we must set CXX manually instead of using 'language: cpp' due to > # travis-ci/travis-ci#3871 > before_script: > Acked-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] XenStore protocol endianness
Just wanted to quickly clarify since I don't see it mentioned anywhere in the docs. The XenStore protocol doesn't seem to define endianness anywhere (it matters for the header). In practice its little endian since that's where its running but I was hoping someone could conclusively nail it down. Thanks. -- Doug Goldstein ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 7/9] x86: make Xen early boot code relocatable
On 4/13/17 9:11 AM, Daniel Kiper wrote: > On Fri, Apr 07, 2017 at 05:23:33AM -0600, Jan Beulich wrote: >>>>> On 21.02.17 at 20:19, <daniel.ki...@oracle.com> wrote: >>> Every multiboot protocol (regardless of version) compatible image must >>> specify its load address (in ELF or multiboot header). Multiboot protocol >>> compatible loader have to load image at specified address. However, there >>> is no guarantee that the requested memory region (in case of Xen it starts >>> at 2 MiB and ends at ~5 MiB) where image should be loaded initially is a RAM >>> and it is free (legacy BIOS platforms are merciful for Xen but I found at >>> least one EFI platform on which Xen load address conflicts with EFI boot >>> services; it is Dell PowerEdge R820 with latest firmware). To cope with that >>> problem we must make Xen early boot code relocatable and help boot loader to >>> relocate image in proper way by suggesting, not requesting specific load >>> addresses as it is right now, allowed address ranges. This patch does >>> former. >>> It does not add multiboot2 protocol interface which is done in "x86: add >>> multiboot2 protocol support for relocatable images" patch. >>> >>> This patch changes following things: >>> - %esi register is used as a storage for Xen image load base address; >>> it is mostly unused in early boot code and preserved during C functions >>> calls in 32-bit mode, >>> - %fs is used as base for Xen data relative addressing in 32-bit code >>> if it is possible; %esi is used for that thing during error printing >>> because it is not always possible to properly and efficiently >>> initialize %fs. >>> >>> Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> >> >> Reviewed-by: Jan Beulich <jbeul...@suse.com> > > It looks that everything passed through test gate and landed in master. > So, this way we have full multiboot2 support in Xen. This means that > you can boot Xen using GRUB2 on EFI platforms. > > I would like to thank everybody who helped me to make it happen. > Especially Jan who patiently reviewed whole series many times > and replied for my stupid questions. > > Daniel > Congrats Daniel. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms
On 3/15/17 9:38 AM, Daniel Kiper wrote: > On Wed, Mar 15, 2017 at 09:27:27AM -0500, Doug Goldstein wrote: >> On 3/15/17 6:35 AM, Daniel Kiper wrote: >>> On Thu, Mar 09, 2017 at 02:02:49PM -0600, Doug Goldstein wrote: >>> >>> [...] >>> >>>> Still missing 'xl info'. >>> >>> Got Intel NUC5i3MYHE (internally it is NUC5i3MYBE board) into my hands. >>> I have put 8 GiB RAM and 500 GB SATA 3 into it. Updated BIOS/EFI to 0041 >>> version (it is the latest one). Installed latest Debian testing (Debian >>> GNU/Linux 9 (stretch)), built GRUB2 and Xen, with and without relocation >>> patches, on it. Everything works (I left machine working last night). >>> Guest boots without any issue. Please take look at attached logs. >>> >>> Doug, could you tell me how exactly did you test your machine? I need OS >>> type, version, C version (GCC, clang, anything else), bintuils version, >>> etc. "xl dmesg", "xl info" and "dmesg" full outputs are welcome too. >>> >>> Daniel >>> >> >> I thought I already responded to Konrad saying that latest staging + >> relocation patches also comes up. > > I do not remember it. Maybe I have missed that. > >> My guess is that it is related to the IOMMU "fix" that Andrew and Jan >> did by #if 0'ing out some of ebmalloc. But I'm not sure. I haven't had > > I reenabled free_ebmalloc_unused_mem() during QEMU tests last week. > It has not changed anything. I will do the same on my NUC. > >> time to look at any of this stuff lately. I went to ELC and then >> vacation and then managed to hurt myself on vacation so I've been away >> from my computer a bit. >> >> All my branches are available in https://github.com/cardoe/xen and I've >> been on Ubuntu 16.04. > > I will try this too. Thanks for update. > > Daniel > Where's the branch you are using on that NUC? Because you can't be using plain staging on there because the firmware version you reported dead locks when the EFI GetTime() is called. Its a known issue by Intel. So you must be patching that out of your Xen? -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms
On 3/15/17 6:35 AM, Daniel Kiper wrote: > On Thu, Mar 09, 2017 at 02:02:49PM -0600, Doug Goldstein wrote: > > [...] > >> Still missing 'xl info'. > > Got Intel NUC5i3MYHE (internally it is NUC5i3MYBE board) into my hands. > I have put 8 GiB RAM and 500 GB SATA 3 into it. Updated BIOS/EFI to 0041 > version (it is the latest one). Installed latest Debian testing (Debian > GNU/Linux 9 (stretch)), built GRUB2 and Xen, with and without relocation > patches, on it. Everything works (I left machine working last night). > Guest boots without any issue. Please take look at attached logs. > > Doug, could you tell me how exactly did you test your machine? I need OS > type, version, C version (GCC, clang, anything else), bintuils version, > etc. "xl dmesg", "xl info" and "dmesg" full outputs are welcome too. > > Daniel > I thought I already responded to Konrad saying that latest staging + relocation patches also comes up. My guess is that it is related to the IOMMU "fix" that Andrew and Jan did by #if 0'ing out some of ebmalloc. But I'm not sure. I haven't had time to look at any of this stuff lately. I went to ELC and then vacation and then managed to hurt myself on vacation so I've been away from my computer a bit. All my branches are available in https://github.com/cardoe/xen and I've been on Ubuntu 16.04. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms
On 3/7/17 9:44 PM, Konrad Rzeszutek Wilk wrote: > On Tue, Mar 07, 2017 at 12:39:04AM +0100, Daniel Kiper wrote: >> On Wed, Feb 22, 2017 at 09:04:17AM -0800, Doug Goldstein wrote: >> >> [...] >> >>> I'm currently at ELC and then on vacation so I don't have access to any >>> of the machines currently myself. However the machine I most use to test >>> is a NUC5i5MYHE and a NUC5i3MYHE if you want to ask around if someone >>> has one internally. But that's why I gave QEMU as an example. >>> >>> I was using qemu master from a few weeks ago. I'll have to find the >>> revision for you. But the command line I use is: >>> >>> -enable-kvm -M pc-q35-2.8 -device intel-iommu -cpu host -m 2048 -smp 2 >>> -drive if=pflash,format=raw,file=/tmp/tmp.EiR6ixmYzV -global >>> isa-debugcon.iobase=0x402 -debugcon file:/tmp/tmp.nuvEXUWfnA -monitor >>> stdio -chardev socket,host=127.0.0.1,port=25914,id=S0,server,nowait >>> -device isa-serial,chardev=S0 -device piix3-usb-uhci -device usb-tablet >>> -netdev id=net0,type=tap -device >>> virtio-net-pci,netdev=net0,mac=52:54:00:12:34:56 -boot order=n -device >>> qxl-vga -gdb tcp::14952 >> >> Sadly, my colleagues and I are not able to reproduce the problem on any of >> machines available for us (available on the market and some development >> stuff in our labs). I did tests with QEMU (I am not able to run it with >> "-device intel-iommu" on my machine; I have to investigate this). Everything >> works. Joao did some tests on Intel NUC D34010WYK second generation. >> Everything works. So, Konrad ordered Intel NUC NUC5i3MYHE for me. I am >> waiting for delivery. Doug, could you tell me what distro, Xen, etc. you >> have installed on that NUC? I would like to test same config as yours on >> this machine. > > I had a chat with Doug on IRC and: > - I had tested earlier on AMD, while he has only Intel boxes, > - He was wondering if this was an IOMMU issue. > > So to double-check that, I installed Ubuntu 16.10 on my X11SAE > SuperMicro, which has an Haswell E3-1245 v5 and with IOMMU enabled. > > I tested the 'origin/staging' xen.gz build with the upstream grub2 > (I just used the 'master' branch) first and also just booting xen.efi. > > Both worked fine. Well if this was really the IOMMU issue then there's already a patch in staging which noops out part of the memory allocator from the first part of the series that was causing problems. > > Then I used v16 of Daniel's patches (this thread). They are also > now ongit://xenbits.xen.org/people/konradwilk/xen.git mb2.v16 > also the same way - as xen.efi and then using grub.efi and booting it > (see below) > > All worked fine. > > konrad-Super-Server login: [ 188.181526] reboot: Restarting system > (XEN) Hardware Dom0 shutdown: rebooting machine > (XEN) APIC error on CPU0: 40(00) > > ... reboot. So as I've mentioned you have to run 'xl info' and look at nr_cpus to see the issue. > Starting Notify bootloader > tha[^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G^G[0;32m OK [0m] Started > Notify bootloader that boot was successful. > > Ubuntu 16.10 konrad-Super-Server hvc0 > > konrad-Super-Server login: > Still missing 'xl info'. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] build: add --with-rundir option to configure
On 2/24/17 10:14 AM, Juergen Gross wrote: > On 24/02/17 17:06, Doug Goldstein wrote: >> On 2/22/17 1:53 AM, Juergen Gross wrote: >>> On 20/02/17 16:19, Andrew Cooper wrote: >>>> On 20/02/17 14:43, Juergen Gross wrote: >>>>> On 20/02/17 15:31, Wei Liu wrote: >>>>>> On Thu, Feb 16, 2017 at 08:47:07AM +0100, Juergen Gross wrote: >>>>>>> There have been reports that Fedora 25 uses /run instead of /var/run. >>>>>>> >>>>>>> Add a --with-rundir option ito configure to be able to specify that >>>>>> I've read this thread but I'm not sure if I need to take any action or >>>>>> all the comments addressed -- especially the part about autoconf. >>>>> Andrew, are you fine with my answer regarding autoconf? Or do you have >>>>> some information regarding --runstatedir which could help? >>>> Oh sorry. Didn't realise I was blocking here. I have no specific >>>> information, other than the quick search I did. >>>> >>>> Can't the future problem be worked around just with if autoconf version >>>> < 2.70 ? >>> I don't think it is possible to add configure options other than >>> --disable-*, --enable-*, --with-* or --without-* by other means than >>> patching general.m4 of autoconf. I don't think we want to do that. >>> >>> So the possibilities are: >>> >>> 1. don't support /run instead of /var/run via configure >>> 2. patch autoconf to support --runstatedir >>> 3. take this patch adding support via --with-rundir and possibly >>>switch over to --runstatedir when a new autoconf version is >>>available >>> >>> I'm in favor of (3.). >>> >>> >>> Juergen >> FWIW, many distros have already pulled the patch into their autoconf so >> its available so you wouldn't really have to do anything. >> >> Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=759647 >> Ubuntu: 16.04 and newer have it >> Gentoo: no link handy but I know its there > > openSUSE: not available > > I don't think its a good idea to rely on _all_ relevant distributions > having done the backport. > > > Juergen > I was under the impression that patches against the configure script are just to the .ac file and the committer is responsible to regenerate the configure script that's committed and its suppose to happen on a Debian machine. But that being said I would suggest that openSUSE pull in the patch as well. And I'm also not trying to convince people to not go with option 3. Feel free to ignore me as noise. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] build: add --with-rundir option to configure
On 2/22/17 1:53 AM, Juergen Gross wrote: > On 20/02/17 16:19, Andrew Cooper wrote: >> On 20/02/17 14:43, Juergen Gross wrote: >>> On 20/02/17 15:31, Wei Liu wrote: >>>> On Thu, Feb 16, 2017 at 08:47:07AM +0100, Juergen Gross wrote: >>>>> There have been reports that Fedora 25 uses /run instead of /var/run. >>>>> >>>>> Add a --with-rundir option ito configure to be able to specify that >>>> I've read this thread but I'm not sure if I need to take any action or >>>> all the comments addressed -- especially the part about autoconf. >>> Andrew, are you fine with my answer regarding autoconf? Or do you have >>> some information regarding --runstatedir which could help? >> >> Oh sorry. Didn't realise I was blocking here. I have no specific >> information, other than the quick search I did. >> >> Can't the future problem be worked around just with if autoconf version >> < 2.70 ? > > I don't think it is possible to add configure options other than > --disable-*, --enable-*, --with-* or --without-* by other means than > patching general.m4 of autoconf. I don't think we want to do that. > > So the possibilities are: > > 1. don't support /run instead of /var/run via configure > 2. patch autoconf to support --runstatedir > 3. take this patch adding support via --with-rundir and possibly >switch over to --runstatedir when a new autoconf version is >available > > I'm in favor of (3.). > > > Juergen FWIW, many distros have already pulled the patch into their autoconf so its available so you wouldn't really have to do anything. Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=759647 Ubuntu: 16.04 and newer have it Gentoo: no link handy but I know its there -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] build: add --with-rundir option to configure
On 2/22/17 5:37 AM, Wei Liu wrote: > On Wed, Feb 22, 2017 at 08:53:24AM +0100, Juergen Gross wrote: >> On 20/02/17 16:19, Andrew Cooper wrote: >>> On 20/02/17 14:43, Juergen Gross wrote: >>>> On 20/02/17 15:31, Wei Liu wrote: >>>>> On Thu, Feb 16, 2017 at 08:47:07AM +0100, Juergen Gross wrote: >>>>>> There have been reports that Fedora 25 uses /run instead of /var/run. >>>>>> >>>>>> Add a --with-rundir option ito configure to be able to specify that >>>>> I've read this thread but I'm not sure if I need to take any action or >>>>> all the comments addressed -- especially the part about autoconf. >>>> Andrew, are you fine with my answer regarding autoconf? Or do you have >>>> some information regarding --runstatedir which could help? >>> >>> Oh sorry. Didn't realise I was blocking here. I have no specific >>> information, other than the quick search I did. >>> >>> Can't the future problem be worked around just with if autoconf version >>> < 2.70 ? >> >> I don't think it is possible to add configure options other than >> --disable-*, --enable-*, --with-* or --without-* by other means than >> patching general.m4 of autoconf. I don't think we want to do that. >> >> So the possibilities are: >> >> 1. don't support /run instead of /var/run via configure >> 2. patch autoconf to support --runstatedir >> 3. take this patch adding support via --with-rundir and possibly >>switch over to --runstatedir when a new autoconf version is >>available > > Option 3 but we need to have that for eternity. :-) > > Wei. Cause --runstatedir will never rear its head I believe I proposed --runstatedir 3+ years ago (I can't recall how long ago) and the maintainers said go idea we'll do a quick release with that on top of 2.69 and then work on the upcoming big release. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms
On 2/22/17 7:34 AM, Daniel Kiper wrote: > On Wed, Feb 22, 2017 at 06:42:40AM -0700, Jan Beulich wrote: >>>>> On 21.02.17 at 20:24, <daniel.ki...@oracle.com> wrote: >>> On Tue, Feb 21, 2017 at 08:19:53PM +0100, Daniel Kiper wrote: >>>> This way Xen can be loaded on EFI platforms using GRUB2 and >>>> other boot loaders which support multiboot2 protocol. >>>> >>>> Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> >>>> --- >>>> v16 - suggestions/fixes: >>>> - improve comments in error handling >>>> (suggested by Jan Beulich). >>> >>> Diff between v15 and v16: >> >> While I'm still not really happy with the changes done, and the VGA >> (or not) handling in general, after discussing this yet another time >> with Andrew we've decided to put in at least the first 4 patches. The > > Thanks a lot! > >> rest of the series will need to have the reported regression (by >> Doug) taken care of to become a candidate for committing. > > I tried to reproduce this on my machines and some guys from my team > did also some tests. We were not able to reproduce anything reported > by Doug. Recently I repeated the tests on QEMU 2.8.0 with OVMF from > https://www.kraxel.org/repos/jenkins/edk2/ (20170213.b2458.g5b97eb4). > Without any issues too. I need more details about tests environments > (exact host kernel version and config, QEMU and OVMF versions and config, > exact Xen and dom0 kernel versions and config, etc.). It would be nice > to get access to one of these physical machines on which issues are > surfacing too. > > Daniel > I'm currently at ELC and then on vacation so I don't have access to any of the machines currently myself. However the machine I most use to test is a NUC5i5MYHE and a NUC5i3MYHE if you want to ask around if someone has one internally. But that's why I gave QEMU as an example. I was using qemu master from a few weeks ago. I'll have to find the revision for you. But the command line I use is: -enable-kvm -M pc-q35-2.8 -device intel-iommu -cpu host -m 2048 -smp 2 -drive if=pflash,format=raw,file=/tmp/tmp.EiR6ixmYzV -global isa-debugcon.iobase=0x402 -debugcon file:/tmp/tmp.nuvEXUWfnA -monitor stdio -chardev socket,host=127.0.0.1,port=25914,id=S0,server,nowait -device isa-serial,chardev=S0 -device piix3-usb-uhci -device usb-tablet -netdev id=net0,type=tap -device virtio-net-pci,netdev=net0,mac=52:54:00:12:34:56 -boot order=n -device qxl-vga -gdb tcp::14952 -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v16 4/9] x86: add multiboot2 protocol support for EFI platforms
On 2/22/17 5:42 AM, Jan Beulich wrote: >>>> On 21.02.17 at 20:24, <daniel.ki...@oracle.com> wrote: >> On Tue, Feb 21, 2017 at 08:19:53PM +0100, Daniel Kiper wrote: >>> This way Xen can be loaded on EFI platforms using GRUB2 and >>> other boot loaders which support multiboot2 protocol. >>> >>> Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> >>> --- >>> v16 - suggestions/fixes: >>> - improve comments in error handling >>> (suggested by Jan Beulich). >> >> Diff between v15 and v16: > > While I'm still not really happy with the changes done, and the VGA > (or not) handling in general, after discussing this yet another time > with Andrew we've decided to put in at least the first 4 patches. The > rest of the series will need to have the reported regression (by > Doug) taken care of to become a candidate for committing. > > Jan > I'm on travel for the next week and half but when I return I had planned on tackling the issue. For anyone else interested its reproducible with QEMU booting smp with 2 CPUs and using OVMF. Xen just never sees more than the boot CPU. I'd recommend using QEMU master since they landed some fixes so that the debugger can handle switching from 64-bit to 32-bit and back again. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v15 05/10] x86: add multiboot2 protocol support for EFI platforms
On 2/16/17 3:49 PM, Daniel Kiper wrote: > On Thu, Feb 16, 2017 at 02:29:45AM -0700, Jan Beulich wrote: >>>>> On 15.02.17 at 22:53, <daniel.ki...@oracle.com> wrote: >>> On Wed, Feb 15, 2017 at 03:22:02AM -0700, Jan Beulich wrote: >>>>>>> On 14.02.17 at 19:38, <daniel.ki...@oracle.com> wrote: >>>>> --- a/xen/arch/x86/boot/head.S >>>>> +++ b/xen/arch/x86/boot/head.S >>>>> @@ -394,10 +394,18 @@ __start: >>>>> >>>>> /* EFI IA-32 platforms are not supported. */ >>>>> cmpl$MULTIBOOT2_TAG_TYPE_EFI32,MB2_tag_type(%ecx) >>>>> +/* >>>>> + * Here we should zap vga_text_buffer. However, we can disable >>>>> + * VGA updates in simpler and more reliable way later. >>>>> + */ >>>>> je .Lmb2_efi_ia_32 >>>>> >>>>> /* Bootloader shutdown EFI x64 boot services. */ >>>>> cmpl$MULTIBOOT2_TAG_TYPE_EFI64,MB2_tag_type(%ecx) >>>>> +/* >>>>> + * Here we should zap vga_text_buffer. However, we can disable >>>>> + * VGA updates in simpler and more reliable way later. >>>>> + */ >>>>> je .Lmb2_no_bs >>>> >>>> I'm afraid I don't view these comments as helpful in understanding >>>> the whole situation. That's partly because I don't follow both the >>>> "simpler" and "more reliable" parts (using just the information here, >>> >>> OK, I will clarify it. >>> >>>> i.e. leaving aside what you've given as explanation earlier, albeit I >>>> don't think that was fully clarifying things either), and partly >>>> because I continue to think that the explanation should go where >>>> the labels are (which is what I had meant to suggest with my >>>> comment placement in reply to v14). Nor does the adjustment >>> >>> OK. >>> >>>> above help (me) understand the correctness of the dual use of >>>> .Lmb2_no_bs. >>> >>> What do you mean by "dual use of .Lmb2_no_bs."? I would like to be sure. >> >> As said in v14 review, it's being jumped to from two rather different >> places, and hence the VGA aspect isn't obviously the same for both. > > OK, I will try to clarify. If a bootloader called us using __efi64_mb2_start > we are sure that we are running on EFI platform and there is no VGA there. > It means that we can safely zap vga_text_buffer unconditionally in first steps > (we do that in second instruction). Then we do not need to take care about > that in case of error. And one of these errors is lack of > MULTIBOOT2_TAG_TYPE_EFI_BS > tag. It means that EFI boot services are shutdown. So, we are in black hole. > We have to inform user about that and halt the system. And that is why we Not looking at the code but the words here. If ExitBootServices() has been called we should be able to still boot if the memory map was passed along. Are we deferring that use case to a follow on? -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] x86/EFI: fix build when using GNU Make 4.1
Since c/s eee5909e9d (x86/EFI: use less crude a way of generating the build ID) builds have been broken when using GNU Make 4.1. This is because there are no dependencies on buildid.o and as such GNU Make does not build it. This adds a dependency so that it is built. Note: This patch is different than I was going to originally submit, I had used runtime.o but while testing this I saw Daniel Kiper's v14 of his multiboot2 series come along and I modified my patch so that he would not have to rebase. Signed-off-by: Doug Goldstein <car...@cardoe.com> --- CC: Jan Beulich <jbeul...@suse.com> CC: Andrew Cooper <andrew.coop...@citrix.com> Jan, This needs to be backported to 4.8 as well. --- xen/arch/x86/efi/Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile index ad3fdf7..af5324f 100644 --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -14,5 +14,7 @@ extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o %.o: %.ihex $(OBJCOPY) -I ihex -O binary $< $@ +boot.init.o: buildid.o + stub.o: $(extra-y) nogcov-$(efi) += stub.o -- 2.10.2 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v14 2/9] efi: build xen.gz with EFI code
On 2/2/17 4:41 PM, Daniel Kiper wrote: > On Thu, Feb 02, 2017 at 11:01:12PM +0100, Daniel Kiper wrote: >> Build xen.gz with EFI code. We need this to support multiboot2 >> protocol on EFI platforms. >> >> If we wish to load non-ELF file using multiboot (v1) or multiboot2 then >> it must contain "linear" (or "flat") representation of code and data. >> This is requirement of both boot protocols. Currently, PE file contains >> many sections which are not "linear" (one after another without any holes) >> or even do not have representation in a file (e.g. BSS). From EFI point >> of view everything is OK and works. However, this file layout cannot be >> properly interpreted by multiboot protocols family. In theory there is >> a chance that we could build proper PE file (from multiboot protocols POV) >> using current build system. However, it means that xen.efi further diverge >> from Xen ELF file (in terms of contents and build method). On the other >> hand ELF has all needed properties. So, it means that this is good starting >> point for further development. Additionally, I think that this is also good >> starting point for further xen.efi code and build optimizations. It looks >> that there is a chance that finally we can generate xen.efi directly from >> Xen ELF using just simple objcopy or other tool. This way we will have one >> Xen binary which can be loaded by three boot protocols: EFI native loader, >> multiboot (v1) and multiboot2. >> >> Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> >> Acked-by: Jan Beulich <jbeul...@suse.com> >> Reviewed-by: Doug Goldstein <car...@cardoe.com> >> --- >> v14 - suggestions/fixes: >> - at least GNU Make 4.1 does not build efi/buildid.o if nothing >> depends on it; so, add "boot.init.o: buildid.o" dependency to >> force efi/buildid.o on some versions of make; I hope that this >> small change does not invalidate Acked-by/Reviewed-by; however, >> I am dropping Tested-by >> (discovered by Konrad Rzeszutek Wilk and Marcos Matsunaga). > > Diff as Doug asked: > > diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile > index 442f3fc..3edff1c 100644 > --- a/xen/arch/x86/efi/Makefile > +++ b/xen/arch/x86/efi/Makefile > @@ -8,6 +8,8 @@ efi := $(if $(efi),$(shell rm disabled)y) > %.o: %.ihex > $(OBJCOPY) -I ihex -O binary $< $@ > > +boot.init.o: buildid.o > + > obj-y := stub.o > obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o > extra-$(efi) += buildid.o > FWIW, I had a similar fix for this cause I ran into it earlier today. I used runtime.o but I've switched to using boot.init.o so as to not conflict with you. This issue affects a lot more than this series so I'm going to mail mine once the tests finish at: https://travis-ci.org/cardoe/xen/builds/197819721 -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v14 0/9] x86: multiboot2 protocol support
On 2/2/17 4:01 PM, Daniel Kiper wrote: > Hi, > > I am sending fourteenth version of multiboot2 protocol support for > legacy BIOS and EFI platforms. This patch series release contains > fixes for all known/confirmed issues. > > The final goal is xen.efi binary file which could be loaded by EFI > loader, multiboot (v1) protocol (only on legacy BIOS platforms) and > multiboot2 protocol. This way we will have: > - smaller Xen code base, > - one code base for xen.gz and xen.efi, > - one build method for xen.gz and xen.efi; > xen.efi will be extracted from xen(-syms) > file using objcopy or special custom tool, > - xen.efi build will not so strongly depend > on a given GCC and binutils version. > > Here is short list of changes since v13: > - changed patches: 2, 4. Want to make diffs of the diffs like last time? I know Jan and I appreciated that and it made it easier to review. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v1 0/7] Make building xSplice patches easier
On 5/6/16 10:48 AM, Ross Lagerwall wrote: > Here is a set of changes to make building xSplice patches easier. > Tested to boot on x86. > Compile-tested on arm. > > This is probably too late to make it into 4.7, but hey, if someone wants > to put it in I've CC'd Wei. Ross, What happened with this series? Some of these patches still appear un-applied and they appear relevant still. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v13 4/9] x86: add multiboot2 protocol support for EFI platforms
On 1/25/17 4:11 PM, Daniel Kiper wrote: > This way Xen can be loaded on EFI platforms using GRUB2 and > other boot loaders which support multiboot2 protocol. > > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> > --- > v13 - suggestions/fixes: > - move vga_text_buffer and efi_platform to .init.data section > (suggested by Jan Beulich), > - reduce number of error branches in EFI code in xen/arch/x86/boot/head.S > (suggested by Jan Beulich), > - rename run_bs label to .Lrun_bs > (suggested by Jan Beulich), > - align the stack as UEFI spec requires > (suggested by Jan Beulich), > - change trampoline region memory layout > (suggested by Jan Beulich), > - revert changes in efi_arch_pre_exit_boot() > (suggested by Jan Beulich), > - relocate_trampoline() must set trampoline_phys for all bootloaders; > otherwise fallback allocator is always used if Xen was loaded with > Multiboot2 protocol, > - change err type in efi_multiboot2() to "static const CHAR16 __initconst" > (suggested by Jan Beulich), > - change asm "g" constraint to "rm" in efi_multiboot2() > (suggested by Jan Beulich), > - improve comments > (suggested by Jan Beulich and Doug Goldstein). This is a huge change and would really be helpful to have the diff of what's changed to work from. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/23/17 9:45 AM, Daniel Kiper wrote: > On Mon, Jan 23, 2017 at 09:35:55AM -0600, Doug Goldstein wrote: >> On 1/23/17 7:08 AM, Daniel Kiper wrote: >>> On Fri, Jan 20, 2017 at 10:54:12PM +0100, Daniel Kiper wrote: >>>> On Fri, Jan 20, 2017 at 02:42:30PM -0500, Doug Goldstein wrote: >>>>> On 1/19/17 8:34 PM, Daniel Kiper wrote: >>>>>> Hi, >>>>>> >>>>>> I am sending twelfth version of multiboot2 protocol support for >>>>>> legacy BIOS and EFI platforms. This patch series release contains >>>>>> fixes for all known/confirmed issues. >>>>> >>>>> With my fix to efi_multiboot2() in 5/10 and the entire series applied, I >>>>> get the following on some of the systems I have access to: >>>>> >>>>> (XEN) [2.000533] HVM: HAP page sizes: 4kB, 2MB, 1GB >>>>> (XEN) [7.012109] Stuck ?? >>>>> (XEN) [7.012129] Failed to bring up CPU 1 (error -5) >>>>> (XEN) [ 12.023606] Stuck ?? >>>>> (XEN) [ 12.023622] Failed to bring up CPU 2 (error -5) >>>>> (XEN) [ 17.035099] Stuck ?? >>>>> (XEN) [ 17.035115] Failed to bring up CPU 3 (error -5) >>>>> (XEN) [ 17.035116] Brought up 1 CPUs >>>>> >>>>> On other machines they reset when setting PAGING into cr0 (actually the >>>>> jmp following it) on line 124 of trampoline.S >>>> >>>> Thanks! I will take a look. >>>> >>>>> If I drop the series to just 2-5 against staging (since patch 1 has >>>>> already gone in) and apply the fix to efi_multiboot2() then all the >>>>> machines I presently have access to boot. >>>> >>>> Great! >>>> >>>>> Effectively the fix to efi_multiboot2() gets us back to the same level >>>>> of hardware support that v11 + my v5 was at for 1-5. So I will extend my: >>>>> >>>>> Reviewed-by: Doug Goldstein <car...@cardoe.com> >>>>> Tested-by: Doug Goldstein <car...@cardoe.com> >>>> >>>> Thanks! >>>> >>>>> on the condition that the fix is applied to 5/10 prior to commit. >>>> >>>> Will do. >>>> >>>> By the way, I have asked my team colleagues to do more tests of this >>>> series. >>>> I will come back to you if I have something in hand. >>> >>> Once you told me that you applied some patches on top of my patch series to >>> get >>> it working. Is it still true? If you still use some extra patches could you >>> send >>> me them? What about ExitBootServices() call? Did you disabled it? If yes on >>> which >>> machines it have to be disabled? >>> >>> Daniel >>> >> >> I previously used the patch that I linked to you authored by Konrad. I >> have since switched to the patches that XenServer uses to no-op >> efi_get_time() and to map additional ranges of reserved memory to make >> EBS() work. > > Could you send me them? > > Daniel > I apply them only for 2 out of the roughly dozen machines. Its not those changes. https://github.com/xenserver/xen-4.7.pg/blob/master/master/0002-efi-Ensure-incorrectly-typed-runtime-services-get-ma.patch https://github.com/xenserver/xen-4.7.pg/blob/master/master/0001-x86-time-Don-t-use-EFI-s-GetTime-call.patch -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/23/17 8:28 AM, Konrad Rzeszutek Wilk wrote: > On Mon, Jan 23, 2017 at 02:08:58PM +0100, Daniel Kiper wrote: >> On Fri, Jan 20, 2017 at 10:54:12PM +0100, Daniel Kiper wrote: >>> On Fri, Jan 20, 2017 at 02:42:30PM -0500, Doug Goldstein wrote: >>>> On 1/19/17 8:34 PM, Daniel Kiper wrote: >>>>> Hi, >>>>> >>>>> I am sending twelfth version of multiboot2 protocol support for >>>>> legacy BIOS and EFI platforms. This patch series release contains >>>>> fixes for all known/confirmed issues. >>>> >>>> With my fix to efi_multiboot2() in 5/10 and the entire series applied, I >>>> get the following on some of the systems I have access to: > > > Both me and Daniel seem to have test machines with EFI that are not soo .. > defective. What machines? Model numbers? Manufacture? I'd also argue that this series continues to write into reserved memory regions (as confirmed by the fix to 5/10 that I sent in) and you're able to still boot so I wouldn't put much value into the hardware you've been using. > > Could you mention which boxes have these kinds of trouble so we can > get to the bottom of this? HP ML10v2 Lenovo T430 Intel NUC NUC5i5MYHE Intel NUC NUC5i3MYHE (oddly has a pretty different firmware than above) Mercury LDS3506 Dell Precision 3420 (co-worker says this is setup to boot BIOS) ASUS M3A78-CM motherboard ASUS AMD motherboard (I don't have it near me and its not powered on right now) SuperMicro (I don't have it near me and its not powered on right now so I can't figure it out) QEMU + OVMF from Ubuntu 16.10 QEMU (master from last week) + OVMF (master from last week) The two unknowns are machines I've got at home and I had unplugged everything while I was on vacation to hopefully avoid lightning strike damage. > > [Is it by any chance a T420 or x230 laptop - I am using legacy BIOS on them > as I couldn't even get them to suspend properly] I'm using EFI on my T430. > > And would it be possible to get serial/power access to this box (Intel NUC?) > to figure out what is going on? Likely not. Both those operations are only available over AMT which is connected on our internal corporate network. I can see if I can bring it home for a period of time and give you access from my home. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/23/17 7:08 AM, Daniel Kiper wrote: > On Fri, Jan 20, 2017 at 10:54:12PM +0100, Daniel Kiper wrote: >> On Fri, Jan 20, 2017 at 02:42:30PM -0500, Doug Goldstein wrote: >>> On 1/19/17 8:34 PM, Daniel Kiper wrote: >>>> Hi, >>>> >>>> I am sending twelfth version of multiboot2 protocol support for >>>> legacy BIOS and EFI platforms. This patch series release contains >>>> fixes for all known/confirmed issues. >>> >>> With my fix to efi_multiboot2() in 5/10 and the entire series applied, I >>> get the following on some of the systems I have access to: >>> >>> (XEN) [2.000533] HVM: HAP page sizes: 4kB, 2MB, 1GB >>> (XEN) [7.012109] Stuck ?? >>> (XEN) [7.012129] Failed to bring up CPU 1 (error -5) >>> (XEN) [ 12.023606] Stuck ?? >>> (XEN) [ 12.023622] Failed to bring up CPU 2 (error -5) >>> (XEN) [ 17.035099] Stuck ?? >>> (XEN) [ 17.035115] Failed to bring up CPU 3 (error -5) >>> (XEN) [ 17.035116] Brought up 1 CPUs >>> >>> On other machines they reset when setting PAGING into cr0 (actually the >>> jmp following it) on line 124 of trampoline.S >> >> Thanks! I will take a look. >> >>> If I drop the series to just 2-5 against staging (since patch 1 has >>> already gone in) and apply the fix to efi_multiboot2() then all the >>> machines I presently have access to boot. >> >> Great! >> >>> Effectively the fix to efi_multiboot2() gets us back to the same level >>> of hardware support that v11 + my v5 was at for 1-5. So I will extend my: >>> >>> Reviewed-by: Doug Goldstein <car...@cardoe.com> >>> Tested-by: Doug Goldstein <car...@cardoe.com> >> >> Thanks! >> >>> on the condition that the fix is applied to 5/10 prior to commit. >> >> Will do. >> >> By the way, I have asked my team colleagues to do more tests of this series. >> I will come back to you if I have something in hand. > > Once you told me that you applied some patches on top of my patch series to > get > it working. Is it still true? If you still use some extra patches could you > send > me them? What about ExitBootServices() call? Did you disabled it? If yes on > which > machines it have to be disabled? > > Daniel > I previously used the patch that I linked to you authored by Konrad. I have since switched to the patches that XenServer uses to no-op efi_get_time() and to map additional ranges of reserved memory to make EBS() work. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/20/17 2:42 PM, Doug Goldstein wrote: > Effectively the fix to efi_multiboot2() gets us back to the same level > of hardware support that v11 + my v5 was at for 1-5. So I will extend my: > > Reviewed-by: Doug Goldstein <car...@cardoe.com> > Tested-by: Doug Goldstein <car...@cardoe.com> > > on the condition that the fix is applied to 5/10 prior to commit. > I forgot to note that I had to implement most of Jan's fixes to 5/10 as well. I didn't do the register change in .Lrun_bs and some of the stylistic changes. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/19/17 8:34 PM, Daniel Kiper wrote: > Hi, > > I am sending twelfth version of multiboot2 protocol support for > legacy BIOS and EFI platforms. This patch series release contains > fixes for all known/confirmed issues. With my fix to efi_multiboot2() in 5/10 and the entire series applied, I get the following on some of the systems I have access to: (XEN) [2.000533] HVM: HAP page sizes: 4kB, 2MB, 1GB (XEN) [7.012109] Stuck ?? (XEN) [7.012129] Failed to bring up CPU 1 (error -5) (XEN) [ 12.023606] Stuck ?? (XEN) [ 12.023622] Failed to bring up CPU 2 (error -5) (XEN) [ 17.035099] Stuck ?? (XEN) [ 17.035115] Failed to bring up CPU 3 (error -5) (XEN) [ 17.035116] Brought up 1 CPUs On other machines they reset when setting PAGING into cr0 (actually the jmp following it) on line 124 of trampoline.S If I drop the series to just 2-5 against staging (since patch 1 has already gone in) and apply the fix to efi_multiboot2() then all the machines I presently have access to boot. Effectively the fix to efi_multiboot2() gets us back to the same level of hardware support that v11 + my v5 was at for 1-5. So I will extend my: Reviewed-by: Doug Goldstein <car...@cardoe.com> Tested-by: Doug Goldstein <car...@cardoe.com> on the condition that the fix is applied to 5/10 prior to commit. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 05/10] x86: add multiboot2 protocol support for EFI platforms
On 1/19/17 8:34 PM, Daniel Kiper wrote: > diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h > index 62c010e..c1285ad 100644 > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > + > +efi_exit_boot(ImageHandle, SystemTable); > + > +/* Return highest allocated memory address below 1 MiB. */ > +return cfg.addr + cfg.size; So my comment about overwriting memory on 02/10 was spot on but made the incorrect conclusion that it was before hand and not after. And here's the issue. I believe what you meant to do was: return cfg.addr + MBI_SIZE; I can't see how this booted for you with OVMF because for all the different versions I've tried with the original code its writing over reserved memory that QEMU uses for the graphics buffers. Which immediately results in the trampolines being overwritten with console data. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/20/17 12:21 PM, Daniel Kiper wrote: > On Fri, Jan 20, 2017 at 11:22:21AM -0500, Doug Goldstein wrote: >> On 1/19/17 8:34 PM, Daniel Kiper wrote: >>> Hi, >>> >>> I am sending twelfth version of multiboot2 protocol support for >>> legacy BIOS and EFI platforms. This patch series release contains >>> fixes for all known/confirmed issues. >> >> What machines did you test this series on? It fails to boot with iPXE > > I have done tests on OVMF and IBM System x3550 M2. It works on both machines. > I have tested multiboot/multiboot2 on legacy BIOS, multiboot2 on EFI and > xen.efi on EFI. If it works on both OVMF and IBM System x3550 M2 I do not > have any problems anywhere else. Or it happens very rarely. What version of OVMF? I've tried the version that ships with Ubuntu 16.10 and a build of master from last week. I've also tried with QEMU from Ubuntu 16.10 and master from earlier this week. I've also tried with and without KVM. None have worked for me. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 05/10] x86: add multiboot2 protocol support for EFI platforms
On 1/19/17 8:34 PM, Daniel Kiper wrote: > This way Xen can be loaded on EFI platforms using GRUB2 and > other boot loaders which support multiboot2 protocol. > > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> > --- > v12 - suggestions/fixes: > - rename __efi64_start to __efi64_mb2_start > (suggested by Andrew Cooper), Andrew actually asked for __mb2_efi64_start > + > +__efi64_mb2_start: here. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 02/10] x86: add multiboot2 protocol support
On 1/19/17 8:34 PM, Daniel Kiper wrote: > Add multiboot2 protocol support. Alter min memory limit handling as we > now may not find it from either multiboot (v1) or multiboot2. > > This way we are laying the foundation for EFI + GRUB2 + Xen development. > > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> > Reviewed-by: Jan Beulich <jbeul...@suse.com> > Reviewed-by: Doug Goldstein <car...@cardoe.com> > --- > v12 - suggestions/fixes: > - replace TABs with spaces in xen/include/xen/multiboot2.h > (suggested by Doug Goldstein). > > v9 - suggestions/fixes: >- use .L label instead of numeric one in multiboot2 data scanning loop; > I hope that this change does not invalidate Jan's Reviewed-by > (suggested by Jan Beulich). > > v8 - suggestions/fixes: >- use sizeof(/) instead of sizeof() > if it is possible > (suggested by Jan Beulich). > > v7 - suggestions/fixes: >- rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively > (suggested by Jan Beulich), >- initialize mbi_out->flags using "|=" instead of "=" > (suggested by Jan Beulich), >- use sizeof(*mmap_dst) instead of sizeof(memory_map_t) > if it makes sense > (suggested by Jan Beulich). > > v6 - suggestions/fixes: >- properly index multiboot2_tag_mmap_t.entries[] > (suggested by Jan Beulich), >- do not index mbi_out_mods[] beyond its end > (suggested by Andrew Cooper), >- reduce number of casts > (suggested by Andrew Cooper and Jan Beulich), >- add braces to increase code readability > (suggested by Andrew Cooper). > > v5 - suggestions/fixes: >- check multiboot2_tag_mmap_t.entry_size before > multiboot2_tag_mmap_t.entries[] use > (suggested by Jan Beulich), >- properly index multiboot2_tag_mmap_t.entries[] > (suggested by Jan Beulich), >- use "type name[]" instad of "type name[0]" > in xen/include/xen/multiboot2.h > (suggested by Jan Beulich), >- remove unneeded comment > (suggested by Jan Beulich). > > v4 - suggestions/fixes: >- avoid assembly usage in xen/arch/x86/boot/reloc.c, >- fix boundary check issue and optimize > for() loops in mbi2_mbi(), >- move to stdcall calling convention, >- remove unneeded typeof() from ALIGN_UP() macro > (suggested by Jan Beulich), >- add and use NULL definition in xen/arch/x86/boot/reloc.c > (suggested by Jan Beulich), >- do not read data beyond the end of multiboot2 > information in xen/arch/x86/boot/head.S > (suggested by Jan Beulich), >- add :req to some .macro arguments > (suggested by Jan Beulich), >- use cmovcc if possible, >- add .L to multiboot2_header_end label > (suggested by Jan Beulich), >- add .L to multiboot2_proto label > (suggested by Jan Beulich), >- improve label names > (suggested by Jan Beulich). > > v3 - suggestions/fixes: >- reorder reloc() arguments > (suggested by Jan Beulich), >- remove .L from multiboot2 header labels > (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk), >- take into account alignment when skipping multiboot2 fixed part > (suggested by Konrad Rzeszutek Wilk), >- create modules data if modules count != 0 > (suggested by Jan Beulich), >- improve macros > (suggested by Jan Beulich), >- reduce number of casts > (suggested by Jan Beulich), >- use const if possible > (suggested by Jan Beulich), >- drop static and __used__ attribute from reloc() > (suggested by Jan Beulich), >- remove isolated/stray __packed attribute from > multiboot2_memory_map_t type definition > (suggested by Jan Beulich), >- reformat xen/include/xen/multiboot2.h > (suggested by Konrad Rzeszutek Wilk), >- improve comments > (suggested by Konrad Rzeszutek Wilk), >- remove hard tabs > (suggested by Jan Beulich and Konrad Rzeszutek Wilk). > > v2 - suggestions/fixes: >- generate multiboot2 header using macros > (suggested by Jan Beulich), >- improve comments > (suggested by Jan Beulich), >- simplify assembly in xen/arch/x86/boot/head.S > (suggested by Jan Beulich), >- do not include include/xen/compiler.h > in xen/arch/x86/boot/reloc.c > (suggested by Jan Beulich), >- do not read data beyond the end of multiboot2 information > (suggested by Jan Beulich). > > v2 - not fixed yet: >- dynamic dependency generation for xen/arch/x86/boot/reloc.S; > this requires more work; I
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/20/17 12:21 PM, Daniel Kiper wrote: > On Fri, Jan 20, 2017 at 11:22:21AM -0500, Doug Goldstein wrote: >> On 1/19/17 8:34 PM, Daniel Kiper wrote: >>> Hi, >>> >>> I am sending twelfth version of multiboot2 protocol support for >>> legacy BIOS and EFI platforms. This patch series release contains >>> fixes for all known/confirmed issues. >> >> What machines did you test this series on? It fails to boot with iPXE > > I have done tests on OVMF and IBM System x3550 M2. It works on both machines. > I have tested multiboot/multiboot2 on legacy BIOS, multiboot2 on EFI and > xen.efi on EFI. If it works on both OVMF and IBM System x3550 M2 I do not > have any problems anywhere else. Or it happens very rarely. "it happens very rarely" is a bit concerning to me here. > >> and grub on an Intel NUC and another board I've got. I've also just > > Let's focus on GRUB2. It is well tested setup. If it works then we can > move to iPXE. By the way, could you try xen.efi from current upstream > and with my patches. It should work without any issue. I'm using grub at the revision you mentioned a while back. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 01/10] x86/boot: implement early command line parser in C
On 1/20/17 11:37 AM, Doug Goldstein wrote: > On 1/19/17 8:34 PM, Daniel Kiper wrote: >> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile >> index 5fdb5ae..6d20646 100644 >> --- a/xen/arch/x86/boot/Makefile >> +++ b/xen/arch/x86/boot/Makefile >> @@ -1,8 +1,15 @@ >> obj-bin-y += head.o >> >> -RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h >> $(BASEDIR)/include/xen/multiboot.h >> +DEFS_H_DEPS = defs.h $(BASEDIR)/include/xen/stdbool.h >> >> -head.o: reloc.S >> +CMDLINE_DEPS = $(DEFS_H_DEPS) video.h >> + >> +RELOC_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/include/xen/multiboot.h > > So when this was patch 8 this previously had: > > +RELOC_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/include/xen/multiboot.h \ > + $(BASEDIR)/include/xen/multiboot2.h > > Which its now moved to patch 1 so obviously can't include multiboot2.h > but then patch 2 in this series fails to add this and patch 5 fails to > add this. > Ignore this. I missed it in patch 2 at first. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 01/10] x86/boot: implement early command line parser in C
On 1/19/17 8:34 PM, Daniel Kiper wrote: > diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile > index 5fdb5ae..6d20646 100644 > --- a/xen/arch/x86/boot/Makefile > +++ b/xen/arch/x86/boot/Makefile > @@ -1,8 +1,15 @@ > obj-bin-y += head.o > > -RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h > $(BASEDIR)/include/xen/multiboot.h > +DEFS_H_DEPS = defs.h $(BASEDIR)/include/xen/stdbool.h > > -head.o: reloc.S > +CMDLINE_DEPS = $(DEFS_H_DEPS) video.h > + > +RELOC_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/include/xen/multiboot.h So when this was patch 8 this previously had: +RELOC_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/include/xen/multiboot.h \ +$(BASEDIR)/include/xen/multiboot2.h Which its now moved to patch 1 so obviously can't include multiboot2.h but then patch 2 in this series fails to add this and patch 5 fails to add this. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 00/10] x86: multiboot2 protocol support
On 1/19/17 8:34 PM, Daniel Kiper wrote: > Hi, > > I am sending twelfth version of multiboot2 protocol support for > legacy BIOS and EFI platforms. This patch series release contains > fixes for all known/confirmed issues. What machines did you test this series on? It fails to boot with iPXE and grub on an Intel NUC and another board I've got. I've also just tried to apply 1-5 and it fails to boot as well. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 05/10] x86: add multiboot2 protocol support for EFI platforms
On 1/19/17 8:34 PM, Daniel Kiper wrote: > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S > index 84cf44d..b8f727a 100644 > --- a/xen/arch/x86/boot/head.S > +++ b/xen/arch/x86/boot/head.S > -2: /* Reserve 64kb for the trampoline */ > -sub $0x1000,%ecx > +/* Reserve memory for the trampoline. */ /* Reserve memory for the trampoline and the low memory stack */ > +sub $((MB_TRAMPOLINE_SIZE+MB_TRAMPOLINE_STACK_SIZE)>>4),%ecx > > /* From arch/x86/smpboot.c: start_eip had better be page-aligned! */ > xor %cl, %cl > diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h > index 62c010e..c1285ad 100644 > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -550,7 +551,12 @@ static void __init efi_arch_memory_setup(void) > > /* Allocate space for trampoline (in first Mb). */ > cfg.addr = 0x10; > -cfg.size = trampoline_end - trampoline_start; > + > +if ( efi_enabled(EFI_LOADER) ) > +cfg.size = trampoline_end - trampoline_start; > +else > +cfg.size = MB_TRAMPOLINE_SIZE + MB_TRAMPOLINE_STACK_SIZE + MBI_SIZE; Why MBI_SIZE? I don't see that being copied in that region or living there? -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 10/10] x86: add multiboot2 protocol support for relocatable images
On 1/19/17 8:34 PM, Daniel Kiper wrote: > Add multiboot2 protocol support for relocatable images. Only GRUB2 with > "multiboot2: Add support for relocatable images" patch understands > that feature. Older multiboot protocol (regardless of version) > compatible loaders ignore it and everything works as usual. > > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> > Acked-by: Jan Beulich <jbeul...@suse.com> > Reviewed-by: Doug Goldstein <car...@cardoe.com> FWIW, I can't find where I offered my R-b for this patch. Its part of the series I've said fails on my hardware. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 09/10] x86/boot: rename sym_phys() to sym_offs()
On 1/19/17 8:34 PM, Daniel Kiper wrote: > This way macro name better describes its function. > Currently it is used to calculate symbol offset in > relation to the beginning of Xen image mapping. > However, value returned by sym_offs() for a given > symbol is not always equal its physical address. > > There is no functional change. > > Suggested-by: Jan Beulich <jbeul...@suse.com> > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> > Acked-by: Jan Beulich <jbeul...@suse.com> > Reviewed-by: Doug Goldstein <car...@cardoe.com> FWIW, I can't find where I offered my R-b for this patch. Its part of the series I've said fails on my hardware. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 07/10] x86/setup: use XEN_IMG_OFFSET instead of...
On 1/19/17 8:34 PM, Daniel Kiper wrote: > ..calculating its value during runtime. > > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> > Acked-by: Jan Beulich <jbeul...@suse.com> > Reviewed-by: Doug Goldstein <car...@cardoe.com> FWIW, I can't find where I offered my R-b for this patch. Its part of the series I've said fails on my hardware. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 06/10] x86: change default load address from 1 MiB to 2 MiB
On 1/19/17 8:34 PM, Daniel Kiper wrote: > Subsequent patches introducing relocatable early boot code play with > page tables using 2 MiB huge pages. If load address is not aligned at > 2 MiB then code touching such page tables must have special cases for > start and end of Xen image memory region. So, let's make life easier > and move default load address from 1 MiB to 2 MiB. This way page table > code will be nice and easy. Hence, there is a chance that it will be > less error prone too... :-))) > > Additionally, drop first 2 MiB mapping from Xen image mapping. > It is no longer needed. > > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> > Reviewed-by: Jan Beulich <jbeul...@suse.com> > Reviewed-by: Doug Goldstein <car...@cardoe.com> FWIW, I can't find where I offered my R-b for this patch. Its part of the series I've said fails on my hardware. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/4] multiboot2 protocol support
On 1/19/17 3:31 AM, Jan Beulich wrote: >>>> On 18.01.17 at 18:38, <george.dun...@citrix.com> wrote: >> >> What's controversial about it? > > The not insignificant amount of assembly code it adds, when our > overall goal is to reduce the amount of assembly code. But > Andrew has meanwhile indicated he's okay for this to go in as is. > I will want to go over the whole patch once more though before > committing it. I completely agree with you on the assembly vs C. I want to follow this up with some conversions to C but my original goal was to land some basic multiboot2 support since we've had this series outstanding for years. I was just trying to help this series move forward and believe we can do improvements after the fact. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
On 1/19/17 6:56 AM, Daniel Kiper wrote: >> Can you tell me what were the fixes to the relocation code? > > Unfortunately I have not received any details from you. Just vague > statements that it does not work. So, I am not able to say anything > about that. If you provide more details I am happy to help. We discussed this on your original v11 thread. It fails on Intel NUCs, Lenovo T430, Dell PowerEdge something (I'm on vacation so I can't see the model number), HP Proliant ML10, QEMU (with OVMF) and some other broads that I don't remember the model number off of my head. Its about a dozen machines all together. They fail in two different ways: - the other CPUs are reported as stuck and the machine boots but 'xl info' reports 1 available CPU. - when the non-CPUs are brought up it dies when setting paging back into cr0. Every single machine fails in one of these two ways. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 1/3] x86/mtrr: drop positive_have_wrcomb()
The only call to have_wrcomb() was always to the generic implementation.
positive_have_wrcomb() was unused.
Signed-off-by: Doug Goldstein <car...@cardoe.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
---
Retaining ACK from <57b480fd027800106...@prv-mh.provo.novell.com>
---
---
xen/arch/x86/cpu/mtrr/generic.c | 5 -
xen/arch/x86/cpu/mtrr/mtrr.h| 2 --
2 files changed, 7 deletions(-)
diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index b7d3293..8d4537a 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -557,11 +557,6 @@ static int generic_have_wrcomb(void)
return (config & (1ULL << 10));
}
-int positive_have_wrcomb(void)
-{
- return 1;
-}
-
/* generic structure...
*/
const struct mtrr_ops generic_mtrr_ops = {
diff --git a/xen/arch/x86/cpu/mtrr/mtrr.h b/xen/arch/x86/cpu/mtrr/mtrr.h
index 53d369d..ec168f9 100644
--- a/xen/arch/x86/cpu/mtrr/mtrr.h
+++ b/xen/arch/x86/cpu/mtrr/mtrr.h
@@ -31,8 +31,6 @@ extern int generic_validate_add_page(unsigned long base,
unsigned long size,
extern const struct mtrr_ops generic_mtrr_ops;
-extern int positive_have_wrcomb(void);
-
/* library functions for processor-specific routines */
struct set_mtrr_context {
unsigned long flags;
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 2/3] x86/mtrr: drop unused func prototypes and struct
These weren't used so drop them.
Signed-off-by: Doug Goldstein <car...@cardoe.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
Retained R-b from <57b48184027800106...@prv-mh.provo.novell.com>
---
xen/arch/x86/cpu/mtrr/mtrr.h | 15 ---
1 file changed, 15 deletions(-)
diff --git a/xen/arch/x86/cpu/mtrr/mtrr.h b/xen/arch/x86/cpu/mtrr/mtrr.h
index ec168f9..bb57def 100644
--- a/xen/arch/x86/cpu/mtrr/mtrr.h
+++ b/xen/arch/x86/cpu/mtrr/mtrr.h
@@ -31,18 +31,6 @@ extern int generic_validate_add_page(unsigned long base,
unsigned long size,
extern const struct mtrr_ops generic_mtrr_ops;
-/* library functions for processor-specific routines */
-struct set_mtrr_context {
- unsigned long flags;
- unsigned long cr4val;
- uint64_t deftype;
- u32 ccr3;
-};
-
-void set_mtrr_done(struct set_mtrr_context *ctxt);
-void set_mtrr_cache_disable(struct set_mtrr_context *ctxt);
-void set_mtrr_prepare_save(struct set_mtrr_context *ctxt);
-
void get_mtrr_state(void);
extern void set_mtrr_ops(const struct mtrr_ops *);
@@ -56,6 +44,3 @@ extern const struct mtrr_ops *mtrr_if;
extern unsigned int num_var_ranges;
void mtrr_state_warn(void);
-
-extern int amd_init_mtrr(void);
-extern int cyrix_init_mtrr(void);
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 3/3] x86/mtrr: convert use_intel_if u32 to bool
This field is always only 1 currently but may allow 0 in the future so
convert it to a bool to provide proper range checking by the compiler.
Signed-off-by: Doug Goldstein <car...@cardoe.com>
---
xen/arch/x86/cpu/mtrr/generic.c | 2 +-
xen/arch/x86/cpu/mtrr/mtrr.h| 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/cpu/mtrr/generic.c b/xen/arch/x86/cpu/mtrr/generic.c
index 8d4537a..104baf9 100644
--- a/xen/arch/x86/cpu/mtrr/generic.c
+++ b/xen/arch/x86/cpu/mtrr/generic.c
@@ -560,7 +560,7 @@ static int generic_have_wrcomb(void)
/* generic structure...
*/
const struct mtrr_ops generic_mtrr_ops = {
- .use_intel_if = 1,
+ .use_intel_if = true,
.set_all = generic_set_all,
.get = generic_get_mtrr,
.get_free_region = generic_get_free_region,
diff --git a/xen/arch/x86/cpu/mtrr/mtrr.h b/xen/arch/x86/cpu/mtrr/mtrr.h
index bb57def..ae4aad9 100644
--- a/xen/arch/x86/cpu/mtrr/mtrr.h
+++ b/xen/arch/x86/cpu/mtrr/mtrr.h
@@ -9,7 +9,7 @@
struct mtrr_ops {
u32 vendor;
- u32 use_intel_if;
+ booluse_intel_if;
// void(*init)(void);
void(*set)(unsigned int reg, unsigned long base,
unsigned long size, mtrr_type type);
@@ -39,7 +39,7 @@ extern u64 size_or_mask, size_and_mask;
extern const struct mtrr_ops *mtrr_if;
#define is_cpu(vnd)(mtrr_if && mtrr_if->vendor == X86_VENDOR_##vnd)
-#define use_intel()(mtrr_if && mtrr_if->use_intel_if == 1)
+#define use_intel()(mtrr_if && mtrr_if->use_intel_if == true)
extern unsigned int num_var_ranges;
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 0/3] x86/mtrr: basic cleanups
Retained the parts that were not objected to by plans to disable mtrrs on PVH containers. Doug Goldstein (3): x86/mtrr: drop positive_have_wrcomb() x86/mtrr: drop unused func prototypes and struct x86/mtrr: convert use_intel_if u32 to bool xen/arch/x86/cpu/mtrr/generic.c | 7 +-- xen/arch/x86/cpu/mtrr/mtrr.h| 21 ++--- 2 files changed, 3 insertions(+), 25 deletions(-) base-commit: 12ec20c732a63f26dc243a847343b8b796c2d88c -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once
On 1/18/17 2:40 PM, Andrew Cooper wrote: > With accurate vendor information available, the shared bits can be sorted out > during recalculation, rather than at query time in the legacy cpuid path. > > This means that: > * Duplication can be dropped from the automatically generated cpuid data. > * The toolstack need not worry about setting them appropriately. > * They can be dropped from the system maximum featuresets. > > While editing gen-cpuid.py, reflow some comments which exceeded the expected > line length. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 5/6] x86/cpuid: Handle the long vendor string in guest_cpuid()
On 1/18/17 2:40 PM, Andrew Cooper wrote: > Leaves 0x8002 through 0x8004 are plain ASCII text, and require no > specific recalculation. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 in guest_cpuid()
On 1/18/17 2:40 PM, Andrew Cooper wrote: > The calculations for p->extd.max_leaf are reworked to force a value of at > least 0x8000, and to take the domains chosen vendor into account when > clamping maximum value. > > The high short vendor information is clobbered or duplicated according to the > chosen vendor. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/6] x86/cpuid: Handle leaf 0 in guest_cpuid()
On 1/18/17 2:40 PM, Andrew Cooper wrote: > Calculate a domains x86_vendor early in recalculate_cpuid_policy(); subsequent > patches need to make other recalculation decisions based on it. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid()
On 1/18/17 2:40 PM, Andrew Cooper wrote: > Include a min() against the appropriate ARRAY_SIZE(), and ASSERT() that > max_subleaf is within ARRAY_SIZE(). > > This is more robust to unexpected problems in a release build of Xen. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
On 1/18/17 2:40 PM, Andrew Cooper wrote: > The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper > mask, but nothing thusfar has prevented the features being visible in thus far? Could be the difference between British English and American English. > HVM-based control domains (where there is no toolstack decision to hide the > features). > > As a side effect of calling nestedhvm_enabled() earlier during domain > creation, it needs to cope with the params[] array array not having been array array? > allocated. > > Reported-by: Roger Pau Monné <roger@citrix.com> > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 2/5] efi: build xen.gz with EFI code
From: Daniel Kiper <daniel.ki...@oracle.com>
Build xen.gz with EFI code. We need this to support multiboot2
protocol on EFI platforms.
If we wish to load non-ELF file using multiboot (v1) or multiboot2 then
it must contain "linear" (or "flat") representation of code and data.
This is requirement of both boot protocols. Currently, PE file contains
many sections which are not "linear" (one after another without any holes)
or even do not have representation in a file (e.g. BSS). From EFI point
of view everything is OK and works. However, this file layout cannot be
properly interpreted by multiboot protocols family. In theory there is
a chance that we could build proper PE file (from multiboot protocols POV)
using current build system. However, it means that xen.efi further diverge
from Xen ELF file (in terms of contents and build method). On the other
hand ELF has all needed properties. So, it means that this is good starting
point for further development. Additionally, I think that this is also good
starting point for further xen.efi code and build optimizations. It looks
that there is a chance that finally we can generate xen.efi directly from
Xen ELF using just simple objcopy or other tool. This way we will have one
Xen binary which can be loaded by three boot protocols: EFI native loader,
multiboot (v1) and multiboot2.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
v6 - suggestions/fixes:
- improve efi_enabled() checks in efi_runtime_call()
(suggested by Jan Beulich).
v5 - suggestions/fixes:
- properly calculate efi symbol address in
xen/arch/x86/xen.lds.S (I hope that this
change does not invalidate Jan's ACK).
v4 - suggestions/fixes:
- functions should return -ENOSYS instead
of -EOPNOTSUPP if EFI runtime services
are not available
(suggested by Jan Beulich),
- remove stale bits from xen/arch/x86/Makefile
(suggested by Jan Beulich).
v3 - suggestions/fixes:
- check for EFI platform in EFI code
(suggested by Jan Beulich),
- fix Makefiles
(suggested by Jan Beulich),
- improve commit message
(suggested by Jan Beulich).
v2 - suggestions/fixes:
- build EFI code only if it is supported in a given build environment
(suggested by Jan Beulich).
---
---
xen/arch/x86/Makefile | 2 +-
xen/arch/x86/efi/Makefile | 12
xen/arch/x86/xen.lds.S| 4 ++--
xen/common/efi/boot.c | 3 +++
xen/common/efi/runtime.c | 9 +
5 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 7f6b5d7..2e22cdf 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -219,6 +219,6 @@ efi/mkreloc: efi/mkreloc.c
clean::
rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d
- rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.o efi/.*.d efi/*.efi
efi/disabled efi/mkreloc
+ rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/disabled efi/mkreloc
rm -f boot/reloc.S boot/reloc.lnk boot/reloc.bin
rm -f note.o
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index ad3fdf7..442f3fc 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,18 +1,14 @@
CFLAGS += -fshort-wchar
-obj-y += stub.o
-
-create = test -e $(1) || touch -t 19990101 $(1)
-
efi := y$(shell rm -f disabled)
efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c
check.c 2>disabled && echo y))
efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
-efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o);
$(call create,runtime.o)))
-
-extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o
+efi := $(if $(efi),$(shell rm disabled)y)
%.o: %.ihex
$(OBJCOPY) -I ihex -O binary $< $@
-stub.o: $(extra-y)
+obj-y := stub.o
+obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
+extra-$(efi) += buildid.o
nogcov-$(efi) += stub.o
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 7676de9..b0b1c9b 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -270,10 +270,10 @@ SECTIONS
.pad : {
. = ALIGN(MB(16));
} :text
-#else
- efi = .;
#endif
+ efi = DEFINED(efi) ? efi : .;
+
/* Sections to be discarded */
/DISCARD/ : {
*(.exit.text)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3e5e4ab..df8c702 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1251,6 +1251,9 @@ void __init efi_init_memory(void)
} *extra, *extra_head = NULL;
#endif
+if ( !efi_enabled(EFI_BOOT) )
+return;
+
printk(XENLOG_INFO "EFI memory map:%s\n",
map_bs ? " (mapping BootS
[Xen-devel] [PATCH v5 0/5] multiboot2 protocol support
This is a series based on v11 of Daniel Kiper's "x86: multiboot2 protocol support" series. It aims to collect up all the fixes and changes that Andrew Cooper, Jan Beulich and myself discovered in code review and testing on actual hardware. I've had problems with the relocation portion of the series so I've dropped it as all the hardware I am needing to support presently for my $EMPLOYER does not load anything at the 1mb mark. To me this adds MB2 support for all pieces of hardware that don't have things located at 1mb so it's an incremental step. I've also dropped the early command line conversion to C as it was done in support of the relocation changes and therefore not necessary. In the end my goal is to help Daniel out by providing the portion of the series that works on half a dozen physical machines I've tested with and integrates all changes as discussed on the v11 thread. The reason I am posting this is that Daniel has said he won't be able to address feedback and issues identified for another 2 weeks but my requirements from my $EMPLOYER are more immediate than that. Feel free to grab this series at: https://github.com/cardoe/xen/tree/doug-mb2-v5 v5 - All changes contained within 5/5 along with a description there. v4 - All changes contained within 5/5 along with a description there. v3 - address review comments by Jan Beulich. They are contained within 5/5. v2 - separate my fixes from Daniel's original series - add back some ACKs I accidentally dropped Daniel Kiper (4): x86: add multiboot2 protocol support efi: build xen.gz with EFI code efi: create new early memory allocator x86: add multiboot2 protocol support for EFI platforms Doug Goldstein (1): fix: add multiboot2 protocol support for EFI platforms xen/arch/x86/Makefile | 2 +- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 361 +-- xen/arch/x86/boot/reloc.c | 148 - xen/arch/x86/efi/Makefile | 12 +- xen/arch/x86/efi/efi-boot.h | 63 +++-- xen/arch/x86/efi/stub.c | 38 +++- xen/arch/x86/setup.c | 3 +- xen/arch/x86/x86_64/asm-offsets.c | 11 +- xen/arch/x86/xen.lds.S| 16 +- xen/common/efi/boot.c | 64 +- xen/common/efi/runtime.c | 9 +- xen/include/xen/config.h | 1 +- xen/include/xen/multiboot2.h | 169 +++- 14 files changed, 852 insertions(+), 48 deletions(-) create mode 100644 xen/include/xen/multiboot2.h base-commit: 98be5ffc05e689e2131f175ed95b011a7270db67 -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v5 5/5] fix: add multiboot2 protocol support for EFI platforms
This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
support for EFI platforms'.
- fix incorrect assembly (identified by Andrew Cooper)
- fix issue where the trampoline size was left as 0 and the
way the memory is allocated for the trampolines we would go to
the end of an available section and then subtract off the size
to decide where to place it. The end result was that we would
always copy the trampolines and the 32-bit stack into some
form of reserved memory after the conventional region we
wanted to put things into. On some systems this did not
manifest as a crash while on others it did. Reworked the
changes to always reserve 64kb for both the stack and the size
of the trampolines. Added a build time assert to make sure we have
enough room always.
Signed-off-by: Doug Goldstein <car...@cardoe.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
---
Doug v5 - change comment style in xen.lds.S as requested by Jan Beulich.
- fix comment around ExitBootServices()
(suggested by Andrew Cooper)
- change multiboot2 efi entry point name to __mb2_efi64_start
(suggested by Andrew Cooper)
Doug v4 - change wording around "stack base"
(found by Jan Beulich)
- added build time assert as suggested by Jan Beulich
- added a KB() macro to make our sizes consistent with MB() and
GB().
Doug v3 - drop ASSERTs since they are runtime only without any output.
This should be completely mitigated by using max() and
ensuring we have a sane value.
(found by Jan Beulich)
- removed extra_mem variable that was incorrectly left behind.
(found by Jan Beulich)
- fix comment around the "start of stack"
(found by Jan Beulich)
Doug v2 - new in this version to help show what's changed
---
---
xen/arch/x86/boot/head.S| 7 ---
xen/arch/x86/efi/efi-boot.h | 8
xen/arch/x86/efi/stub.c | 2 +-
xen/arch/x86/xen.lds.S | 8
xen/include/xen/config.h| 1 +
5 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index ac93df0..f2e8cc9 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -89,12 +89,12 @@ multiboot2_header_start:
0, /* Number of the lines - no preference. */ \
0 /* Number of bits per pixel - no preference. */
-/* Inhibit bootloader from calling ExitBootServices(). */
+/* Request that ExitBootServices() not be called. */
mb2ht_init MB2_HT(EFI_BS), MB2_HT(OPTIONAL)
/* EFI64 entry point. */
mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \
- sym_phys(__efi64_start)
+ sym_phys(__mb2_efi64_start)
/* Multiboot2 header end tag. */
mb2ht_init MB2_HT(END), MB2_HT(REQUIRED)
@@ -169,7 +169,7 @@ not_multiboot:
.code64
-__efi64_start:
+__mb2_efi64_start:
cld
/* VGA is not available on EFI platforms. */
@@ -519,6 +519,7 @@ trampoline_setup:
1:
/* Switch to low-memory stack. */
mov sym_phys(trampoline_phys),%edi
+/* The stack ends 64kb after the location of trampoline_phys */
lea 0x1(%edi),%esp
lea trampoline_boot_cpu_entry-trampoline_start(%edi),%eax
pushl $BOOT_CS32
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index dc857d8..d2ebf21 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -146,8 +146,6 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
{
struct e820entry *e;
unsigned int i;
-/* Check for extra mem for mbi data if Xen is loaded via multiboot2
protocol. */
-UINTN extra_mem = efi_enabled(EFI_LOADER) ? 0 : (64 << 10);
/* Populate E820 table and check trampoline area availability. */
e = e820map - 1;
@@ -170,8 +168,7 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
/* fall through */
case EfiConventionalMemory:
if ( !trampoline_phys && desc->PhysicalStart + len <= 0x10 &&
- len >= cfg.size + extra_mem &&
- desc->PhysicalStart + len > cfg.addr )
+ len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
/* fall through */
case EfiLoaderCode:
@@ -686,6 +683,9 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle,
EFI_SYSTEM_TABLE *SystemTa
setup_efi_pci();
efi_variables();
+/* This is the maximum size of our trampoline + our low memory stack */
+cfg.size = KB(64);
+
if ( gop )
efi_set_gop_mode(gop, gop_mode);
diff --git a/x
[Xen-devel] [PATCH v5 3/5] efi: create new early memory allocator
From: Daniel Kiper <daniel.ki...@oracle.com> There is a problem with place_string() which is used as early memory allocator. It gets memory chunks starting from start symbol and goes down. Sadly this does not work when Xen is loaded using multiboot2 protocol because then the start lives on 1 MiB address and we should not allocate a memory from below of it. So, I tried to use mem_lower address calculated by GRUB2. However, this solution works only on some machines. There are machines in the wild (e.g. Dell PowerEdge R820) which uses first ~640 KiB for boot services code or data... :-((( Hence, we need new memory allocator for Xen EFI boot code which is quite simple and generic and could be used by place_string() and efi_arch_allocate_mmap_buffer(). I think about following solutions: 1) We could use native EFI allocation functions (e.g. AllocatePool() or AllocatePages()) to get memory chunk. However, later (somewhere in __start_xen()) we must copy its contents to safe place or reserve it in e820 memory map and map it in Xen virtual address space. This means that the code referring to Xen command line, loaded modules and EFI memory map, mostly in __start_xen(), will be further complicated and diverge from legacy BIOS cases. Additionally, both former things have to be placed below 4 GiB because their addresses are stored in multiboot_info_t structure which has 32-bit relevant members. 2) We may allocate memory area statically somewhere in Xen code which could be used as memory pool for early dynamic allocations. Looks quite simple. Additionally, it would not depend on EFI at all and could be used on legacy BIOS platforms if we need it. However, we must carefully choose size of this pool. We do not want increase Xen binary size too much and waste too much memory but also we must fit at least memory map on x86 EFI platforms. As I saw on small machine, e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more than 200 entries. Every entry on x86-64 platform is 40 bytes in size. So, it means that we need more than 8 KiB for EFI memory map only. Additionally, if we use this memory pool for Xen and modules command line storage (it would be used when xen.efi is executed as EFI application) then we should add, I think, about 1 KiB. In this case, to be on safe side, we should assume at least 64 KiB pool for early memory allocations. Which is about 4 times of our earlier calculations. However, during discussion on Xen-devel Jan Beulich suggested that just in case we should use 1 MiB memory pool like it is in original place_string() implementation. So, let's use 1 MiB as it was proposed. If we think that we should not waste unallocated memory in the pool on running system then we can mark this region as __initdata and move all required data to dynamically allocated places somewhere in __start_xen(). 2a) We could put memory pool into .bss.page_aligned section. Then allocate memory chunks starting from the lowest address. After init phase we can free unused portion of the memory pool as in case of .init.text or .init.data sections. This way we do not need to allocate any space in image file and freeing of unused area in the memory pool is very simple. Now #2a solution is implemented because it is quite simple and requires limited number of changes, especially in __start_xen(). The new allocator is quite generic and can be used on ARM platforms too. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Acked-by: Jan Beulich <jbeul...@suse.com> Acked-by: Julien Grall <julien.gr...@arm.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- Doug v1 - removed stale paragraph v11 - suggestions/fixes: - #ifdef only EBMALLOC_SIZE from ebmalloc machinery (suggested by Jan Beulich). v10 - suggestions/fixes: - remove unneeded ARM free_ebmalloc_unused_mem() stub. v9 - suggestions/fixes: - call free_ebmalloc_unused_mem() from efi_init_memory() instead of xen/arch/arm/setup.c:init_done() (suggested by Jan Beulich), - improve comments. v8 - suggestions/fixes: - disable whole ebmalloc machinery on ARM platforms, - add comment saying what should be done before enabling ebmalloc on ARM, (suggested by Julien Grall), - move ebmalloc code before efi-boot.h inclusion and remove unneeded forward declaration (suggested by Jan Beulich), - remove free_ebmalloc_unused_mem() call from xen/arch/arm/setup.c:init_done() (suggested by Julien Grall), - improve commit message. v7 - suggestions/fixes: - enable most of ebmalloc machinery on ARM platforms (suggested by Jan Beulich), - remove unneeded cast (suggested by Jan Beulich), - wrap long line (suggested by Jan Beulich), - improve commit message. v6 - suggestions/fixes: - optimize ebmalloc allocator, - move ebmalloc machinery to xen/com
[Xen-devel] [PATCH v5 4/5] x86: add multiboot2 protocol support for EFI platforms
From: Daniel Kiper <daniel.ki...@oracle.com> This way Xen can be loaded on EFI platforms using GRUB2 and other boot loaders which support multiboot2 protocol. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> Tested-by: Doug Goldstein <car...@cardoe.com> --- Doug v2 - dropped all my changes and moved them into their own patch Doug v1 - fix incorrect assembly (identified by Andrew Cooper) - fix issue where the trampoline size was left as 0 and the way the memory is allocated for the trampolines we would go to the end of an available section and then subtract off the size to decide where to place it. The end result was that we would always copy the trampolines and the 32-bit stack into some form of reserved memory after the conventional region we wanted to put things into. On some systems this did not manifest as a crash while on others it did. Reworked the changes to always reserve 64kb for both the stack and the size of the trampolines. Added an ASSERT to make sure we never blow through this size. v10 - suggestions/fixes: - replace ljmpl with lretq (suggested by Andrew Cooper), - introduce efi_platform to increase code readability (suggested by Andrew Cooper). v9 - suggestions/fixes: - use .L labels instead of numeric ones in multiboot2 data scanning loops (suggested by Jan Beulich). v8 - suggestions/fixes: - use __bss_start(%rip)/__bss_end(%rip) instead of of .startof.(.bss)(%rip)/$.sizeof.(.bss) because latter is not tested extensively in different built environments yet (suggested by Andrew Cooper), - fix multiboot2 data scanning loop in x86_32 code (suggested by Jan Beulich), - add check for extra mem for mbi data if Xen is loaded via multiboot2 protocol on EFI platform (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v7 - suggestions/fixes: - do not allocate twice memory for trampoline if we were loaded via multiboot2 protocol on EFI platform, - wrap long line (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v6 - suggestions/fixes: - improve label names in assembly error printing code (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups and fixes (suggested by Jan Beulich). v4 - suggestions/fixes: - remove redundant BSS alignment, - update BSS alignment check, - use __set_bit() instead of set_bit() if possible (suggested by Jan Beulich), - call efi_arch_cpu() from efi_multiboot2() even if the same work is done later in other place right now (suggested by Jan Beulich), - xen/arch/x86/efi/stub.c:efi_multiboot2() fail properly on EFI platforms, - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - use 32-bit registers in x86_64 code if possible (suggested by Jan Beulich), - multiboot2 information address is 64-bit in x86_64 code, so, treat it is as is (suggested by Jan Beulich), - use cmovcc if possible, - leave only one space between rep and stosq (suggested by Jan Beulich), - improve error handling, - improve early error messages, (suggested by Jan Beulich), - improve early error messages printing code, - improve label names (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups. v3 - suggestions/fixes: - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - improve segment registers initialization (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich and Konrad Rzeszutek Wilk), - improve commit message (suggested by Jan Beulich). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - switch CPU to x86_32 mode before jumping to 32-bit code (suggested by Andrew Cooper), - reduce code changes to increase patch readability (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - ignore MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag on EFI platform and find on my own multiboot2.mem_lower value, - stop execution if EFI platform is detected in legacy BIOS path. --- --- xen/arch/x86/boot/head.S | 263 +-- xen/arch/x86/efi/efi-boot.h | 54 +- xen/arch/x86/efi/stub.c | 38 - xen/arch/x86/x86_64/asm-offsets.c | 2 +- xen/arch/x86/xen.lds.S| 4 +- xen/common/efi/boot.c | 11 +- 6 files changed, 349 insertions(+), 23 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/ar
[Xen-devel] [PATCH v5 1/5] x86: add multiboot2 protocol support
From: Daniel Kiper <daniel.ki...@oracle.com> Add multiboot2 protocol support. Alter min memory limit handling as we now may not find it from either multiboot (v1) or multiboot2. This way we are laying the foundation for EFI + GRUB2 + Xen development. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Jan Beulich <jbeul...@suse.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- v9 - suggestions/fixes: - use .L label instead of numeric one in multiboot2 data scanning loop; I hope that this change does not invalidate Jan's Reviewed-by (suggested by Jan Beulich). v8 - suggestions/fixes: - use sizeof(/) instead of sizeof() if it is possible (suggested by Jan Beulich). v7 - suggestions/fixes: - rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively (suggested by Jan Beulich), - initialize mbi_out->flags using "|=" instead of "=" (suggested by Jan Beulich), - use sizeof(*mmap_dst) instead of sizeof(memory_map_t) if it makes sense (suggested by Jan Beulich). v6 - suggestions/fixes: - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - do not index mbi_out_mods[] beyond its end (suggested by Andrew Cooper), - reduce number of casts (suggested by Andrew Cooper and Jan Beulich), - add braces to increase code readability (suggested by Andrew Cooper). v5 - suggestions/fixes: - check multiboot2_tag_mmap_t.entry_size before multiboot2_tag_mmap_t.entries[] use (suggested by Jan Beulich), - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - use "type name[]" instad of "type name[0]" in xen/include/xen/multiboot2.h (suggested by Jan Beulich), - remove unneeded comment (suggested by Jan Beulich). v4 - suggestions/fixes: - avoid assembly usage in xen/arch/x86/boot/reloc.c, - fix boundary check issue and optimize for() loops in mbi2_mbi(), - move to stdcall calling convention, - remove unneeded typeof() from ALIGN_UP() macro (suggested by Jan Beulich), - add and use NULL definition in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - add :req to some .macro arguments (suggested by Jan Beulich), - use cmovcc if possible, - add .L to multiboot2_header_end label (suggested by Jan Beulich), - add .L to multiboot2_proto label (suggested by Jan Beulich), - improve label names (suggested by Jan Beulich). v3 - suggestions/fixes: - reorder reloc() arguments (suggested by Jan Beulich), - remove .L from multiboot2 header labels (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk), - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - create modules data if modules count != 0 (suggested by Jan Beulich), - improve macros (suggested by Jan Beulich), - reduce number of casts (suggested by Jan Beulich), - use const if possible (suggested by Jan Beulich), - drop static and __used__ attribute from reloc() (suggested by Jan Beulich), - remove isolated/stray __packed attribute from multiboot2_memory_map_t type definition (suggested by Jan Beulich), - reformat xen/include/xen/multiboot2.h (suggested by Konrad Rzeszutek Wilk), - improve comments (suggested by Konrad Rzeszutek Wilk), - remove hard tabs (suggested by Jan Beulich and Konrad Rzeszutek Wilk). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - simplify assembly in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - do not include include/xen/compiler.h in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information (suggested by Jan Beulich). v2 - not fixed yet: - dynamic dependency generation for xen/arch/x86/boot/reloc.S; this requires more work; I am not sure that it pays because potential patch requires more changes than addition of just multiboot2.h to Makefile (suggested by Jan Beulich), - isolated/stray __packed attribute usage for multiboot2_memory_map_t (suggested by Jan Beulich). --- --- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 107 +++- xen/arch/x86/boot/reloc.c | 148 ++- xen/arch/x86/x86_64/asm-offsets.c | 9 ++- xen/include/xen/multiboot2.h | 169 +++- 5 files changed, 426 insertions(+), 10 deletions(-) create mode 100644 xen/include/xen/multiboot2.h diff --git a/xen/arch/x86/boot/
Re: [Xen-devel] [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
On 1/18/17 4:25 PM, Daniel Kiper wrote: > On Wed, Jan 18, 2017 at 07:52:44AM -0700, Jan Beulich wrote: >> >> ... the comment style here fixed (which could be done upon commit >> or when Daniel merges this back into his series). > > Hmmm... Why this patch and #0 was not CC-ed to me? > You say this like I did it intentionally to upset you. I switched to using git-series and as a result my workflow changed slightly. When I glanced at 1/5 it had you CCed and I assumed this one was as well. > Anyway, it looks that I have cleared my backlog to some extent and now I am > able to take a stab on v12. There is a chance that I will have it with Doug's > and some additional fixes on Friday or Monday. So then there should be no issue with this landing before that or not. The changes for you will be identical. A simple rebase --onto would actually be easier than folding this into your larger series. Can you tell me what were the fixes to the relocation code? -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
On 1/18/17 2:51 PM, Andrew Cooper wrote: > On 18/01/17 14:17, Doug Goldstein wrote: >> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S >> index d423fd8..ac93df0 100644 >> --- a/xen/arch/x86/boot/head.S >> +++ b/xen/arch/x86/boot/head.S >> @@ -89,6 +89,13 @@ multiboot2_header_start: >> 0, /* Number of the lines - no preference. */ \ >> 0 /* Number of bits per pixel - no preference. */ >> >> +/* Inhibit bootloader from calling ExitBootServices(). */ > > /* Request that ExitBootServices() not be called. */ > > This tag doesn't make any guarantees. > Agreed. The multiboot2 spec is clear in that regard. I have tested the case when the bootloader ignores this and it does work. >> +.code64 >> + >> +__efi64_start: > > __mb2_efi64_start: > > This entry point is distinct from the PE efi64 entry point in > common/efi/boot.c I agree here as well. It does need one other update however. On line 97. /* EFI64 entry point. */ mb2ht_init MB2_HT(ENTRY_ADDRESS_EFI64), MB2_HT(OPTIONAL), \ sym_phys(__efi64_start) I can reroll the series or if people are comfortable with committing this then the change can be done then. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen/common: Drop function calls for Xen compile/version information
On 1/18/17 12:04 PM, Andrew Cooper wrote: > Last time I tried LTO: > > * The GCC build did work, but the net binary was bigger rather than > smaller, and it successfully boot > * The Clang build worked, made a much smaller binary, but due to a clang > bug, "optimised" code into using SSE, and blew up spectacularly when > booting. > > From experiments with XTF, Clang 3.9 LTO is far more comprehensive, and > is basically capable of optimising entire XTF tests into one single test > function, and properly respects the "no FPU ever" options used during boot. > > ~Andrew FWIW, some of the clang issues I reported against LTO and FPU bits are marked as fixed in 3.9 and 4.0. Not sure if things got backported. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v4 4/5] x86: add multiboot2 protocol support for EFI platforms
From: Daniel Kiper <daniel.ki...@oracle.com> This way Xen can be loaded on EFI platforms using GRUB2 and other boot loaders which support multiboot2 protocol. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> Tested-by: Doug Goldstein <car...@cardoe.com> --- Doug v2 - dropped all my changes and moved them into their own patch Doug v1 - fix incorrect assembly (identified by Andrew Cooper) - fix issue where the trampoline size was left as 0 and the way the memory is allocated for the trampolines we would go to the end of an available section and then subtract off the size to decide where to place it. The end result was that we would always copy the trampolines and the 32-bit stack into some form of reserved memory after the conventional region we wanted to put things into. On some systems this did not manifest as a crash while on others it did. Reworked the changes to always reserve 64kb for both the stack and the size of the trampolines. Added an ASSERT to make sure we never blow through this size. v10 - suggestions/fixes: - replace ljmpl with lretq (suggested by Andrew Cooper), - introduce efi_platform to increase code readability (suggested by Andrew Cooper). v9 - suggestions/fixes: - use .L labels instead of numeric ones in multiboot2 data scanning loops (suggested by Jan Beulich). v8 - suggestions/fixes: - use __bss_start(%rip)/__bss_end(%rip) instead of of .startof.(.bss)(%rip)/$.sizeof.(.bss) because latter is not tested extensively in different built environments yet (suggested by Andrew Cooper), - fix multiboot2 data scanning loop in x86_32 code (suggested by Jan Beulich), - add check for extra mem for mbi data if Xen is loaded via multiboot2 protocol on EFI platform (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v7 - suggestions/fixes: - do not allocate twice memory for trampoline if we were loaded via multiboot2 protocol on EFI platform, - wrap long line (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v6 - suggestions/fixes: - improve label names in assembly error printing code (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups and fixes (suggested by Jan Beulich). v4 - suggestions/fixes: - remove redundant BSS alignment, - update BSS alignment check, - use __set_bit() instead of set_bit() if possible (suggested by Jan Beulich), - call efi_arch_cpu() from efi_multiboot2() even if the same work is done later in other place right now (suggested by Jan Beulich), - xen/arch/x86/efi/stub.c:efi_multiboot2() fail properly on EFI platforms, - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - use 32-bit registers in x86_64 code if possible (suggested by Jan Beulich), - multiboot2 information address is 64-bit in x86_64 code, so, treat it is as is (suggested by Jan Beulich), - use cmovcc if possible, - leave only one space between rep and stosq (suggested by Jan Beulich), - improve error handling, - improve early error messages, (suggested by Jan Beulich), - improve early error messages printing code, - improve label names (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups. v3 - suggestions/fixes: - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - improve segment registers initialization (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich and Konrad Rzeszutek Wilk), - improve commit message (suggested by Jan Beulich). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - switch CPU to x86_32 mode before jumping to 32-bit code (suggested by Andrew Cooper), - reduce code changes to increase patch readability (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - ignore MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag on EFI platform and find on my own multiboot2.mem_lower value, - stop execution if EFI platform is detected in legacy BIOS path. --- --- xen/arch/x86/boot/head.S | 263 +-- xen/arch/x86/efi/efi-boot.h | 54 +- xen/arch/x86/efi/stub.c | 38 - xen/arch/x86/x86_64/asm-offsets.c | 2 +- xen/arch/x86/xen.lds.S| 4 +- xen/common/efi/boot.c | 11 +- 6 files changed, 349 insertions(+), 23 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/ar
[Xen-devel] [PATCH v4 3/5] efi: create new early memory allocator
From: Daniel Kiper <daniel.ki...@oracle.com> There is a problem with place_string() which is used as early memory allocator. It gets memory chunks starting from start symbol and goes down. Sadly this does not work when Xen is loaded using multiboot2 protocol because then the start lives on 1 MiB address and we should not allocate a memory from below of it. So, I tried to use mem_lower address calculated by GRUB2. However, this solution works only on some machines. There are machines in the wild (e.g. Dell PowerEdge R820) which uses first ~640 KiB for boot services code or data... :-((( Hence, we need new memory allocator for Xen EFI boot code which is quite simple and generic and could be used by place_string() and efi_arch_allocate_mmap_buffer(). I think about following solutions: 1) We could use native EFI allocation functions (e.g. AllocatePool() or AllocatePages()) to get memory chunk. However, later (somewhere in __start_xen()) we must copy its contents to safe place or reserve it in e820 memory map and map it in Xen virtual address space. This means that the code referring to Xen command line, loaded modules and EFI memory map, mostly in __start_xen(), will be further complicated and diverge from legacy BIOS cases. Additionally, both former things have to be placed below 4 GiB because their addresses are stored in multiboot_info_t structure which has 32-bit relevant members. 2) We may allocate memory area statically somewhere in Xen code which could be used as memory pool for early dynamic allocations. Looks quite simple. Additionally, it would not depend on EFI at all and could be used on legacy BIOS platforms if we need it. However, we must carefully choose size of this pool. We do not want increase Xen binary size too much and waste too much memory but also we must fit at least memory map on x86 EFI platforms. As I saw on small machine, e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more than 200 entries. Every entry on x86-64 platform is 40 bytes in size. So, it means that we need more than 8 KiB for EFI memory map only. Additionally, if we use this memory pool for Xen and modules command line storage (it would be used when xen.efi is executed as EFI application) then we should add, I think, about 1 KiB. In this case, to be on safe side, we should assume at least 64 KiB pool for early memory allocations. Which is about 4 times of our earlier calculations. However, during discussion on Xen-devel Jan Beulich suggested that just in case we should use 1 MiB memory pool like it is in original place_string() implementation. So, let's use 1 MiB as it was proposed. If we think that we should not waste unallocated memory in the pool on running system then we can mark this region as __initdata and move all required data to dynamically allocated places somewhere in __start_xen(). 2a) We could put memory pool into .bss.page_aligned section. Then allocate memory chunks starting from the lowest address. After init phase we can free unused portion of the memory pool as in case of .init.text or .init.data sections. This way we do not need to allocate any space in image file and freeing of unused area in the memory pool is very simple. Now #2a solution is implemented because it is quite simple and requires limited number of changes, especially in __start_xen(). The new allocator is quite generic and can be used on ARM platforms too. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Acked-by: Jan Beulich <jbeul...@suse.com> Acked-by: Julien Grall <julien.gr...@arm.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- Doug v1 - removed stale paragraph v11 - suggestions/fixes: - #ifdef only EBMALLOC_SIZE from ebmalloc machinery (suggested by Jan Beulich). v10 - suggestions/fixes: - remove unneeded ARM free_ebmalloc_unused_mem() stub. v9 - suggestions/fixes: - call free_ebmalloc_unused_mem() from efi_init_memory() instead of xen/arch/arm/setup.c:init_done() (suggested by Jan Beulich), - improve comments. v8 - suggestions/fixes: - disable whole ebmalloc machinery on ARM platforms, - add comment saying what should be done before enabling ebmalloc on ARM, (suggested by Julien Grall), - move ebmalloc code before efi-boot.h inclusion and remove unneeded forward declaration (suggested by Jan Beulich), - remove free_ebmalloc_unused_mem() call from xen/arch/arm/setup.c:init_done() (suggested by Julien Grall), - improve commit message. v7 - suggestions/fixes: - enable most of ebmalloc machinery on ARM platforms (suggested by Jan Beulich), - remove unneeded cast (suggested by Jan Beulich), - wrap long line (suggested by Jan Beulich), - improve commit message. v6 - suggestions/fixes: - optimize ebmalloc allocator, - move ebmalloc machinery to xen/com
[Xen-devel] [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
support for EFI platforms'.
- fix incorrect assembly (identified by Andrew Cooper)
- fix issue where the trampoline size was left as 0 and the
way the memory is allocated for the trampolines we would go to
the end of an available section and then subtract off the size
to decide where to place it. The end result was that we would
always copy the trampolines and the 32-bit stack into some
form of reserved memory after the conventional region we
wanted to put things into. On some systems this did not
manifest as a crash while on others it did. Reworked the
changes to always reserve 64kb for both the stack and the size
of the trampolines. Added a build time assert to make sure we have
enough room always.
Signed-off-by: Doug Goldstein <car...@cardoe.com>
---
Doug v4 - change wording around "stack base"
(found by Jan Beulich)
- added build time assert as suggested by Jan Beulich
- added a KB() macro to make our sizes consistent with MB() and
GB().
Doug v3 - drop ASSERTs since they are runtime only without any output.
This should be completely mitigated by using max() and
ensuring we have a sane value.
(found by Jan Beulich)
- removed extra_mem variable that was incorrectly left behind.
(found by Jan Beulich)
- fix comment around the "start of stack"
(found by Jan Beulich)
Doug v2 - new in this version to help show what's changed
---
---
xen/arch/x86/boot/head.S| 1 +
xen/arch/x86/efi/efi-boot.h | 8
xen/arch/x86/efi/stub.c | 2 +-
xen/arch/x86/xen.lds.S | 7 +++
xen/include/xen/config.h| 1 +
5 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index ac93df0..df5fdab 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -519,6 +519,7 @@ trampoline_setup:
1:
/* Switch to low-memory stack. */
mov sym_phys(trampoline_phys),%edi
+/* The stack ends 64kb after the location of trampoline_phys */
lea 0x1(%edi),%esp
lea trampoline_boot_cpu_entry-trampoline_start(%edi),%eax
pushl $BOOT_CS32
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index dc857d8..d2ebf21 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -146,8 +146,6 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
{
struct e820entry *e;
unsigned int i;
-/* Check for extra mem for mbi data if Xen is loaded via multiboot2
protocol. */
-UINTN extra_mem = efi_enabled(EFI_LOADER) ? 0 : (64 << 10);
/* Populate E820 table and check trampoline area availability. */
e = e820map - 1;
@@ -170,8 +168,7 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
/* fall through */
case EfiConventionalMemory:
if ( !trampoline_phys && desc->PhysicalStart + len <= 0x10 &&
- len >= cfg.size + extra_mem &&
- desc->PhysicalStart + len > cfg.addr )
+ len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
/* fall through */
case EfiLoaderCode:
@@ -686,6 +683,9 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle,
EFI_SYSTEM_TABLE *SystemTa
setup_efi_pci();
efi_variables();
+/* This is the maximum size of our trampoline + our low memory stack */
+cfg.size = KB(64);
+
if ( gop )
efi_set_gop_mode(gop, gop_mode);
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 6ea6aa1..b81adc0 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -33,7 +33,7 @@ paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
* not be directly supported by C compiler.
*/
asm volatile(
-"call %2 \n"
+"call *%2 \n"
"0: hlt \n"
"jmp 0b \n"
: "+c" (StdErr), "+d" (err) : "g" (StdErr->OutputString)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index e0e2529..504348e 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -333,3 +333,10 @@ ASSERT(IS_ALIGNED(trampoline_start, 4), "trampoline_start
misaligned")
ASSERT(IS_ALIGNED(trampoline_end, 4), "trampoline_end misaligned")
ASSERT(IS_ALIGNED(__bss_start, 8), "__bss_start misaligned")
ASSERT(IS_ALIGNED(__bss_end,8), "__bss_end misaligned")
+
+/* The trampolines and the low memory stack mu
[Xen-devel] [PATCH v4 1/5] x86: add multiboot2 protocol support
From: Daniel Kiper <daniel.ki...@oracle.com> Add multiboot2 protocol support. Alter min memory limit handling as we now may not find it from either multiboot (v1) or multiboot2. This way we are laying the foundation for EFI + GRUB2 + Xen development. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Jan Beulich <jbeul...@suse.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- v9 - suggestions/fixes: - use .L label instead of numeric one in multiboot2 data scanning loop; I hope that this change does not invalidate Jan's Reviewed-by (suggested by Jan Beulich). v8 - suggestions/fixes: - use sizeof(/) instead of sizeof() if it is possible (suggested by Jan Beulich). v7 - suggestions/fixes: - rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively (suggested by Jan Beulich), - initialize mbi_out->flags using "|=" instead of "=" (suggested by Jan Beulich), - use sizeof(*mmap_dst) instead of sizeof(memory_map_t) if it makes sense (suggested by Jan Beulich). v6 - suggestions/fixes: - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - do not index mbi_out_mods[] beyond its end (suggested by Andrew Cooper), - reduce number of casts (suggested by Andrew Cooper and Jan Beulich), - add braces to increase code readability (suggested by Andrew Cooper). v5 - suggestions/fixes: - check multiboot2_tag_mmap_t.entry_size before multiboot2_tag_mmap_t.entries[] use (suggested by Jan Beulich), - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - use "type name[]" instad of "type name[0]" in xen/include/xen/multiboot2.h (suggested by Jan Beulich), - remove unneeded comment (suggested by Jan Beulich). v4 - suggestions/fixes: - avoid assembly usage in xen/arch/x86/boot/reloc.c, - fix boundary check issue and optimize for() loops in mbi2_mbi(), - move to stdcall calling convention, - remove unneeded typeof() from ALIGN_UP() macro (suggested by Jan Beulich), - add and use NULL definition in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - add :req to some .macro arguments (suggested by Jan Beulich), - use cmovcc if possible, - add .L to multiboot2_header_end label (suggested by Jan Beulich), - add .L to multiboot2_proto label (suggested by Jan Beulich), - improve label names (suggested by Jan Beulich). v3 - suggestions/fixes: - reorder reloc() arguments (suggested by Jan Beulich), - remove .L from multiboot2 header labels (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk), - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - create modules data if modules count != 0 (suggested by Jan Beulich), - improve macros (suggested by Jan Beulich), - reduce number of casts (suggested by Jan Beulich), - use const if possible (suggested by Jan Beulich), - drop static and __used__ attribute from reloc() (suggested by Jan Beulich), - remove isolated/stray __packed attribute from multiboot2_memory_map_t type definition (suggested by Jan Beulich), - reformat xen/include/xen/multiboot2.h (suggested by Konrad Rzeszutek Wilk), - improve comments (suggested by Konrad Rzeszutek Wilk), - remove hard tabs (suggested by Jan Beulich and Konrad Rzeszutek Wilk). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - simplify assembly in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - do not include include/xen/compiler.h in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information (suggested by Jan Beulich). v2 - not fixed yet: - dynamic dependency generation for xen/arch/x86/boot/reloc.S; this requires more work; I am not sure that it pays because potential patch requires more changes than addition of just multiboot2.h to Makefile (suggested by Jan Beulich), - isolated/stray __packed attribute usage for multiboot2_memory_map_t (suggested by Jan Beulich). --- --- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 107 +++- xen/arch/x86/boot/reloc.c | 148 ++- xen/arch/x86/x86_64/asm-offsets.c | 9 ++- xen/include/xen/multiboot2.h | 169 +++- 5 files changed, 426 insertions(+), 10 deletions(-) create mode 100644 xen/include/xen/multiboot2.h diff --git a/xen/arch/x86/boot/
[Xen-devel] [PATCH v4 2/5] efi: build xen.gz with EFI code
From: Daniel Kiper <daniel.ki...@oracle.com>
Build xen.gz with EFI code. We need this to support multiboot2
protocol on EFI platforms.
If we wish to load non-ELF file using multiboot (v1) or multiboot2 then
it must contain "linear" (or "flat") representation of code and data.
This is requirement of both boot protocols. Currently, PE file contains
many sections which are not "linear" (one after another without any holes)
or even do not have representation in a file (e.g. BSS). From EFI point
of view everything is OK and works. However, this file layout cannot be
properly interpreted by multiboot protocols family. In theory there is
a chance that we could build proper PE file (from multiboot protocols POV)
using current build system. However, it means that xen.efi further diverge
from Xen ELF file (in terms of contents and build method). On the other
hand ELF has all needed properties. So, it means that this is good starting
point for further development. Additionally, I think that this is also good
starting point for further xen.efi code and build optimizations. It looks
that there is a chance that finally we can generate xen.efi directly from
Xen ELF using just simple objcopy or other tool. This way we will have one
Xen binary which can be loaded by three boot protocols: EFI native loader,
multiboot (v1) and multiboot2.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
v6 - suggestions/fixes:
- improve efi_enabled() checks in efi_runtime_call()
(suggested by Jan Beulich).
v5 - suggestions/fixes:
- properly calculate efi symbol address in
xen/arch/x86/xen.lds.S (I hope that this
change does not invalidate Jan's ACK).
v4 - suggestions/fixes:
- functions should return -ENOSYS instead
of -EOPNOTSUPP if EFI runtime services
are not available
(suggested by Jan Beulich),
- remove stale bits from xen/arch/x86/Makefile
(suggested by Jan Beulich).
v3 - suggestions/fixes:
- check for EFI platform in EFI code
(suggested by Jan Beulich),
- fix Makefiles
(suggested by Jan Beulich),
- improve commit message
(suggested by Jan Beulich).
v2 - suggestions/fixes:
- build EFI code only if it is supported in a given build environment
(suggested by Jan Beulich).
---
---
xen/arch/x86/Makefile | 2 +-
xen/arch/x86/efi/Makefile | 12
xen/arch/x86/xen.lds.S| 4 ++--
xen/common/efi/boot.c | 3 +++
xen/common/efi/runtime.c | 9 +
5 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 7f6b5d7..2e22cdf 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -219,6 +219,6 @@ efi/mkreloc: efi/mkreloc.c
clean::
rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d
- rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.o efi/.*.d efi/*.efi
efi/disabled efi/mkreloc
+ rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/disabled efi/mkreloc
rm -f boot/reloc.S boot/reloc.lnk boot/reloc.bin
rm -f note.o
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index ad3fdf7..442f3fc 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,18 +1,14 @@
CFLAGS += -fshort-wchar
-obj-y += stub.o
-
-create = test -e $(1) || touch -t 19990101 $(1)
-
efi := y$(shell rm -f disabled)
efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c
check.c 2>disabled && echo y))
efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
-efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o);
$(call create,runtime.o)))
-
-extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o
+efi := $(if $(efi),$(shell rm disabled)y)
%.o: %.ihex
$(OBJCOPY) -I ihex -O binary $< $@
-stub.o: $(extra-y)
+obj-y := stub.o
+obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
+extra-$(efi) += buildid.o
nogcov-$(efi) += stub.o
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 7676de9..b0b1c9b 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -270,10 +270,10 @@ SECTIONS
.pad : {
. = ALIGN(MB(16));
} :text
-#else
- efi = .;
#endif
+ efi = DEFINED(efi) ? efi : .;
+
/* Sections to be discarded */
/DISCARD/ : {
*(.exit.text)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3e5e4ab..df8c702 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1251,6 +1251,9 @@ void __init efi_init_memory(void)
} *extra, *extra_head = NULL;
#endif
+if ( !efi_enabled(EFI_BOOT) )
+return;
+
printk(XENLOG_INFO "EFI memory map:%s\n",
map_bs ? " (mapping BootS
[Xen-devel] [PATCH v4 0/5] multiboot2 protocol support
This is a series based on v11 of Daniel Kiper's "x86: multiboot2 protocol support" series. It aims to collect up all the fixes and changes that Andrew Cooper, Jan Beulich and myself discovered in code review and testing on actual hardware. I've had problems with the relocation portion of the series so I've dropped it as all the hardware I am needing to support presently for my $EMPLOYER does not load anything at the 1mb mark. To me this adds MB2 support for all pieces of hardware that don't have things located at 1mb so it's an incremental step. I've also dropped the early command line conversion to C as it was done in support of the relocation changes and therefore not necessary. In the end my goal is to help Daniel out by providing the portion of the series that works on half a dozen physical machines I've tested with and integrates all changes as discussed on the v11 thread. The reason I am posting this is that Daniel has said he won't be able to address feedback and issues identified for another 2 weeks but my requirements from my $EMPLOYER are more immediate than that. Feel free to grab this series at: https://github.com/cardoe/xen/tree/doug-mb2-v4 v4 - All changes contained within 5/5 along with a description there. v3 - address review comments by Jan Beulich. They are contained within 5/5. v2 - separate my fixes from Daniel's original series - add back some ACKs I accidentally dropped Daniel Kiper (4): x86: add multiboot2 protocol support efi: build xen.gz with EFI code efi: create new early memory allocator x86: add multiboot2 protocol support for EFI platforms Doug Goldstein (1): fix: add multiboot2 protocol support for EFI platforms xen/arch/x86/Makefile | 2 +- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 361 +-- xen/arch/x86/boot/reloc.c | 148 - xen/arch/x86/efi/Makefile | 12 +- xen/arch/x86/efi/efi-boot.h | 63 +++-- xen/arch/x86/efi/stub.c | 38 +++- xen/arch/x86/setup.c | 3 +- xen/arch/x86/x86_64/asm-offsets.c | 11 +- xen/arch/x86/xen.lds.S| 15 +- xen/common/efi/boot.c | 64 +- xen/common/efi/runtime.c | 9 +- xen/include/xen/config.h | 1 +- xen/include/xen/multiboot2.h | 169 +++- 14 files changed, 851 insertions(+), 48 deletions(-) create mode 100644 xen/include/xen/multiboot2.h base-commit: 98be5ffc05e689e2131f175ed95b011a7270db67 -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] fix: add multiboot2 protocol support for EFI platforms
On 1/18/17 8:41 AM, Jan Beulich wrote: >>>> On 18.01.17 at 14:33, <car...@cardoe.com> wrote: >> On 1/18/17 4:52 AM, Jan Beulich wrote: >>>>>> On 17.01.17 at 21:07, <car...@cardoe.com> wrote: >>>> @@ -686,6 +683,10 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle, >>>> EFI_SYSTEM_TABLE *SystemTa >>>> setup_efi_pci(); >>>> efi_variables(); >>>> >>>> +/* This is the maximum size of our trampoline + our low memory stack >>>> */ >>>> +cfg.size = max_t(UINTN, 64 << 10, >>>> +(trampoline_end - trampoline_start) + 4096); >>> >>> Considering the consuming code further up, I don't understand the >>> reason for the addition of 4096 here. And if there is a reason, I'm >>> pretty sure you actually mean PAGE_SIZE. >> >> You are correct. Given that the assembly is hardcoded at 64k there is no >> reason for this. I had kicked around doing a similar max() call in >> assembly instead of hardcoding the value but didn't do it. So I should >> just remove this. > > Well, I don't mind the max() (albeit it's not very useful, especially > if trampoline size would ever get pretty close to 64k). And as said > in reply to the earlier version - I think this would better be checked > at build time (see the various ASSERT()s at the end of xen.lds.S). > > Jan > Thank you. I had looked for BUILD_ASSERT() and didn't see it so I didn't know what mechanism we had which is why I didn't add it. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 5/5] fix: add multiboot2 protocol support for EFI platforms
On 1/18/17 4:52 AM, Jan Beulich wrote:
>>>> On 17.01.17 at 21:07, <car...@cardoe.com> wrote:
>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -519,6 +519,7 @@ trampoline_setup:
>> 1:
>> /* Switch to low-memory stack. */
>> mov sym_phys(trampoline_phys),%edi
>> +/* The stack base is 64kb after the location of trampoline_phys */
>> lea 0x1(%edi),%esp
>
> I'm sorry for being picky, but the stack _base_ isn't where the stack
> pointer should point initially, or else there would be no room on the
> stack at all. Perhaps you had a reason for not using the wording I
> did suggest, but whatever wording is chosen should not risk to cause
> confusion (otherwise I think we'd be better off not adding any
> comment here).
So this was my perception. Maybe I'm totally wrong here.
|--|-|--|
a b c d
example values
a = 0x0 (0)
b = 0x8c000 (trampoline_phys)
c = 0x9c000 (base of stack as it grows towards 0x8c000)
d = 0x10 (1mb)
movsym_phys(trampoline_phys),%edi
lea0x1(%edi),%esp
I would think that 0x1 + %edi would be the base or start of the
stack since its growing downwards towards b.
I'll use "ends" however and resubmit.
>
>> --- a/xen/arch/x86/efi/efi-boot.h
>> +++ b/xen/arch/x86/efi/efi-boot.h
>> @@ -146,8 +146,6 @@ static void __init
>> efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
>> {
>> struct e820entry *e;
>> unsigned int i;
>> -/* Check for extra mem for mbi data if Xen is loaded via multiboot2
>> protocol. */
>> -UINTN extra_mem = efi_enabled(EFI_LOADER) ? 0 : (64 << 10);
>>
>> /* Populate E820 table and check trampoline area availability. */
>> e = e820map - 1;
>> @@ -170,8 +168,7 @@ static void __init
>> efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
>> /* fall through */
>> case EfiConventionalMemory:
>> if ( !trampoline_phys && desc->PhysicalStart + len <= 0x10
>> &&
>> - len >= cfg.size + extra_mem &&
>> - desc->PhysicalStart + len > cfg.addr )
>> + len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
>> cfg.addr = (desc->PhysicalStart + len - cfg.size) &
>> PAGE_MASK;
>> /* fall through */
>> case EfiLoaderCode:
>> @@ -686,6 +683,10 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle,
>> EFI_SYSTEM_TABLE *SystemTa
>> setup_efi_pci();
>> efi_variables();
>>
>> +/* This is the maximum size of our trampoline + our low memory stack */
>> +cfg.size = max_t(UINTN, 64 << 10,
>> +(trampoline_end - trampoline_start) + 4096);
>
> Considering the consuming code further up, I don't understand the
> reason for the addition of 4096 here. And if there is a reason, I'm
> pretty sure you actually mean PAGE_SIZE.
>
> Jan
>
You are correct. Given that the assembly is hardcoded at 64k there is no
reason for this. I had kicked around doing a similar max() call in
assembly instead of hardcoding the value but didn't do it. So I should
just remove this.
--
Doug Goldstein
signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 5/5] fix: add multiboot2 protocol support for EFI platforms
This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
support for EFI platforms'.
- fix incorrect assembly (identified by Andrew Cooper)
- fix issue where the trampoline size was left as 0 and the
way the memory is allocated for the trampolines we would go to
the end of an available section and then subtract off the size
to decide where to place it. The end result was that we would
always copy the trampolines and the 32-bit stack into some
form of reserved memory after the conventional region we
wanted to put things into. On some systems this did not
manifest as a crash while on others it did. Reworked the
changes to always reserve 64kb for both the stack and the size
of the trampolines.
Signed-off-by: Doug Goldstein <car...@cardoe.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
Doug v3 - drop ASSERTs since they are runtime only without any output.
This should be completely mitigated by using max() and
ensuring we have a sane value.
(found by Jan Beulich)
- removed extra_mem variable that was incorrectly left behind.
(found by Jan Beulich)
- fix comment around the "start of stack"
(found by Jan Beulich)
Doug v2 - new in this version to help show what's changed
---
---
xen/arch/x86/boot/head.S| 1 +
xen/arch/x86/efi/efi-boot.h | 9 +
xen/arch/x86/efi/stub.c | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index ac93df0..d288959 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -519,6 +519,7 @@ trampoline_setup:
1:
/* Switch to low-memory stack. */
mov sym_phys(trampoline_phys),%edi
+/* The stack base is 64kb after the location of trampoline_phys */
lea 0x1(%edi),%esp
lea trampoline_boot_cpu_entry-trampoline_start(%edi),%eax
pushl $BOOT_CS32
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index dc857d8..a73134c 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -146,8 +146,6 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
{
struct e820entry *e;
unsigned int i;
-/* Check for extra mem for mbi data if Xen is loaded via multiboot2
protocol. */
-UINTN extra_mem = efi_enabled(EFI_LOADER) ? 0 : (64 << 10);
/* Populate E820 table and check trampoline area availability. */
e = e820map - 1;
@@ -170,8 +168,7 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
/* fall through */
case EfiConventionalMemory:
if ( !trampoline_phys && desc->PhysicalStart + len <= 0x10 &&
- len >= cfg.size + extra_mem &&
- desc->PhysicalStart + len > cfg.addr )
+ len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
/* fall through */
case EfiLoaderCode:
@@ -686,6 +683,10 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle,
EFI_SYSTEM_TABLE *SystemTa
setup_efi_pci();
efi_variables();
+/* This is the maximum size of our trampoline + our low memory stack */
+cfg.size = max_t(UINTN, 64 << 10,
+(trampoline_end - trampoline_start) + 4096);
+
if ( gop )
efi_set_gop_mode(gop, gop_mode);
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 6ea6aa1..b81adc0 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -33,7 +33,7 @@ paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
* not be directly supported by C compiler.
*/
asm volatile(
-"call %2 \n"
+"call *%2 \n"
"0: hlt \n"
"jmp 0b \n"
: "+c" (StdErr), "+d" (err) : "g" (StdErr->OutputString)
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 3/5] efi: create new early memory allocator
From: Daniel Kiper <daniel.ki...@oracle.com> There is a problem with place_string() which is used as early memory allocator. It gets memory chunks starting from start symbol and goes down. Sadly this does not work when Xen is loaded using multiboot2 protocol because then the start lives on 1 MiB address and we should not allocate a memory from below of it. So, I tried to use mem_lower address calculated by GRUB2. However, this solution works only on some machines. There are machines in the wild (e.g. Dell PowerEdge R820) which uses first ~640 KiB for boot services code or data... :-((( Hence, we need new memory allocator for Xen EFI boot code which is quite simple and generic and could be used by place_string() and efi_arch_allocate_mmap_buffer(). I think about following solutions: 1) We could use native EFI allocation functions (e.g. AllocatePool() or AllocatePages()) to get memory chunk. However, later (somewhere in __start_xen()) we must copy its contents to safe place or reserve it in e820 memory map and map it in Xen virtual address space. This means that the code referring to Xen command line, loaded modules and EFI memory map, mostly in __start_xen(), will be further complicated and diverge from legacy BIOS cases. Additionally, both former things have to be placed below 4 GiB because their addresses are stored in multiboot_info_t structure which has 32-bit relevant members. 2) We may allocate memory area statically somewhere in Xen code which could be used as memory pool for early dynamic allocations. Looks quite simple. Additionally, it would not depend on EFI at all and could be used on legacy BIOS platforms if we need it. However, we must carefully choose size of this pool. We do not want increase Xen binary size too much and waste too much memory but also we must fit at least memory map on x86 EFI platforms. As I saw on small machine, e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more than 200 entries. Every entry on x86-64 platform is 40 bytes in size. So, it means that we need more than 8 KiB for EFI memory map only. Additionally, if we use this memory pool for Xen and modules command line storage (it would be used when xen.efi is executed as EFI application) then we should add, I think, about 1 KiB. In this case, to be on safe side, we should assume at least 64 KiB pool for early memory allocations. Which is about 4 times of our earlier calculations. However, during discussion on Xen-devel Jan Beulich suggested that just in case we should use 1 MiB memory pool like it is in original place_string() implementation. So, let's use 1 MiB as it was proposed. If we think that we should not waste unallocated memory in the pool on running system then we can mark this region as __initdata and move all required data to dynamically allocated places somewhere in __start_xen(). 2a) We could put memory pool into .bss.page_aligned section. Then allocate memory chunks starting from the lowest address. After init phase we can free unused portion of the memory pool as in case of .init.text or .init.data sections. This way we do not need to allocate any space in image file and freeing of unused area in the memory pool is very simple. Now #2a solution is implemented because it is quite simple and requires limited number of changes, especially in __start_xen(). The new allocator is quite generic and can be used on ARM platforms too. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Acked-by: Jan Beulich <jbeul...@suse.com> Acked-by: Julien Grall <julien.gr...@arm.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- Doug v1 - removed stale paragraph v11 - suggestions/fixes: - #ifdef only EBMALLOC_SIZE from ebmalloc machinery (suggested by Jan Beulich). v10 - suggestions/fixes: - remove unneeded ARM free_ebmalloc_unused_mem() stub. v9 - suggestions/fixes: - call free_ebmalloc_unused_mem() from efi_init_memory() instead of xen/arch/arm/setup.c:init_done() (suggested by Jan Beulich), - improve comments. v8 - suggestions/fixes: - disable whole ebmalloc machinery on ARM platforms, - add comment saying what should be done before enabling ebmalloc on ARM, (suggested by Julien Grall), - move ebmalloc code before efi-boot.h inclusion and remove unneeded forward declaration (suggested by Jan Beulich), - remove free_ebmalloc_unused_mem() call from xen/arch/arm/setup.c:init_done() (suggested by Julien Grall), - improve commit message. v7 - suggestions/fixes: - enable most of ebmalloc machinery on ARM platforms (suggested by Jan Beulich), - remove unneeded cast (suggested by Jan Beulich), - wrap long line (suggested by Jan Beulich), - improve commit message. v6 - suggestions/fixes: - optimize ebmalloc allocator, - move ebmalloc machinery to xen/com
[Xen-devel] [PATCH v3 1/5] x86: add multiboot2 protocol support
From: Daniel Kiper <daniel.ki...@oracle.com> Add multiboot2 protocol support. Alter min memory limit handling as we now may not find it from either multiboot (v1) or multiboot2. This way we are laying the foundation for EFI + GRUB2 + Xen development. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Jan Beulich <jbeul...@suse.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- v9 - suggestions/fixes: - use .L label instead of numeric one in multiboot2 data scanning loop; I hope that this change does not invalidate Jan's Reviewed-by (suggested by Jan Beulich). v8 - suggestions/fixes: - use sizeof(/) instead of sizeof() if it is possible (suggested by Jan Beulich). v7 - suggestions/fixes: - rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively (suggested by Jan Beulich), - initialize mbi_out->flags using "|=" instead of "=" (suggested by Jan Beulich), - use sizeof(*mmap_dst) instead of sizeof(memory_map_t) if it makes sense (suggested by Jan Beulich). v6 - suggestions/fixes: - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - do not index mbi_out_mods[] beyond its end (suggested by Andrew Cooper), - reduce number of casts (suggested by Andrew Cooper and Jan Beulich), - add braces to increase code readability (suggested by Andrew Cooper). v5 - suggestions/fixes: - check multiboot2_tag_mmap_t.entry_size before multiboot2_tag_mmap_t.entries[] use (suggested by Jan Beulich), - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - use "type name[]" instad of "type name[0]" in xen/include/xen/multiboot2.h (suggested by Jan Beulich), - remove unneeded comment (suggested by Jan Beulich). v4 - suggestions/fixes: - avoid assembly usage in xen/arch/x86/boot/reloc.c, - fix boundary check issue and optimize for() loops in mbi2_mbi(), - move to stdcall calling convention, - remove unneeded typeof() from ALIGN_UP() macro (suggested by Jan Beulich), - add and use NULL definition in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - add :req to some .macro arguments (suggested by Jan Beulich), - use cmovcc if possible, - add .L to multiboot2_header_end label (suggested by Jan Beulich), - add .L to multiboot2_proto label (suggested by Jan Beulich), - improve label names (suggested by Jan Beulich). v3 - suggestions/fixes: - reorder reloc() arguments (suggested by Jan Beulich), - remove .L from multiboot2 header labels (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk), - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - create modules data if modules count != 0 (suggested by Jan Beulich), - improve macros (suggested by Jan Beulich), - reduce number of casts (suggested by Jan Beulich), - use const if possible (suggested by Jan Beulich), - drop static and __used__ attribute from reloc() (suggested by Jan Beulich), - remove isolated/stray __packed attribute from multiboot2_memory_map_t type definition (suggested by Jan Beulich), - reformat xen/include/xen/multiboot2.h (suggested by Konrad Rzeszutek Wilk), - improve comments (suggested by Konrad Rzeszutek Wilk), - remove hard tabs (suggested by Jan Beulich and Konrad Rzeszutek Wilk). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - simplify assembly in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - do not include include/xen/compiler.h in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information (suggested by Jan Beulich). v2 - not fixed yet: - dynamic dependency generation for xen/arch/x86/boot/reloc.S; this requires more work; I am not sure that it pays because potential patch requires more changes than addition of just multiboot2.h to Makefile (suggested by Jan Beulich), - isolated/stray __packed attribute usage for multiboot2_memory_map_t (suggested by Jan Beulich). --- --- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 107 +++- xen/arch/x86/boot/reloc.c | 148 ++- xen/arch/x86/x86_64/asm-offsets.c | 9 ++- xen/include/xen/multiboot2.h | 169 +++- 5 files changed, 426 insertions(+), 10 deletions(-) create mode 100644 xen/include/xen/multiboot2.h diff --git a/xen/arch/x86/boot/
[Xen-devel] [PATCH v3 4/5] x86: add multiboot2 protocol support for EFI platforms
From: Daniel Kiper <daniel.ki...@oracle.com> This way Xen can be loaded on EFI platforms using GRUB2 and other boot loaders which support multiboot2 protocol. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> Tested-by: Doug Goldstein <car...@cardoe.com> --- Doug v2 - dropped all my changes and moved them into their own patch Doug v1 - fix incorrect assembly (identified by Andrew Cooper) - fix issue where the trampoline size was left as 0 and the way the memory is allocated for the trampolines we would go to the end of an available section and then subtract off the size to decide where to place it. The end result was that we would always copy the trampolines and the 32-bit stack into some form of reserved memory after the conventional region we wanted to put things into. On some systems this did not manifest as a crash while on others it did. Reworked the changes to always reserve 64kb for both the stack and the size of the trampolines. Added an ASSERT to make sure we never blow through this size. v10 - suggestions/fixes: - replace ljmpl with lretq (suggested by Andrew Cooper), - introduce efi_platform to increase code readability (suggested by Andrew Cooper). v9 - suggestions/fixes: - use .L labels instead of numeric ones in multiboot2 data scanning loops (suggested by Jan Beulich). v8 - suggestions/fixes: - use __bss_start(%rip)/__bss_end(%rip) instead of of .startof.(.bss)(%rip)/$.sizeof.(.bss) because latter is not tested extensively in different built environments yet (suggested by Andrew Cooper), - fix multiboot2 data scanning loop in x86_32 code (suggested by Jan Beulich), - add check for extra mem for mbi data if Xen is loaded via multiboot2 protocol on EFI platform (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v7 - suggestions/fixes: - do not allocate twice memory for trampoline if we were loaded via multiboot2 protocol on EFI platform, - wrap long line (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v6 - suggestions/fixes: - improve label names in assembly error printing code (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups and fixes (suggested by Jan Beulich). v4 - suggestions/fixes: - remove redundant BSS alignment, - update BSS alignment check, - use __set_bit() instead of set_bit() if possible (suggested by Jan Beulich), - call efi_arch_cpu() from efi_multiboot2() even if the same work is done later in other place right now (suggested by Jan Beulich), - xen/arch/x86/efi/stub.c:efi_multiboot2() fail properly on EFI platforms, - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - use 32-bit registers in x86_64 code if possible (suggested by Jan Beulich), - multiboot2 information address is 64-bit in x86_64 code, so, treat it is as is (suggested by Jan Beulich), - use cmovcc if possible, - leave only one space between rep and stosq (suggested by Jan Beulich), - improve error handling, - improve early error messages, (suggested by Jan Beulich), - improve early error messages printing code, - improve label names (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups. v3 - suggestions/fixes: - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - improve segment registers initialization (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich and Konrad Rzeszutek Wilk), - improve commit message (suggested by Jan Beulich). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - switch CPU to x86_32 mode before jumping to 32-bit code (suggested by Andrew Cooper), - reduce code changes to increase patch readability (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - ignore MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag on EFI platform and find on my own multiboot2.mem_lower value, - stop execution if EFI platform is detected in legacy BIOS path. --- --- xen/arch/x86/boot/head.S | 263 +-- xen/arch/x86/efi/efi-boot.h | 54 +- xen/arch/x86/efi/stub.c | 38 - xen/arch/x86/x86_64/asm-offsets.c | 2 +- xen/arch/x86/xen.lds.S| 4 +- xen/common/efi/boot.c | 11 +- 6 files changed, 349 insertions(+), 23 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/ar
[Xen-devel] [PATCH v3 0/5] multiboot2 protocol support
This is a series based on v11 of Daniel Kiper's "x86: multiboot2 protocol support" series. It aims to collect up all the fixes and changes that Andrew Cooper, Jan Beulich and myself discovered in code review and testing on actual hardware. I've had problems with the relocation portion of the series so I've dropped it as all the hardware I am needing to support presently for my $EMPLOYER does not load anything at the 1mb mark. To me this adds MB2 support for all pieces of hardware that don't have things located at 1mb so it's an incremental step. I've also dropped the early command line conversion to C as it was done in support of the relocation changes and therefore not necessary. In the end my goal is to help Daniel out by providing the portion of the series that works on half a dozen physical machines I've tested with and integrates all changes as discussed on the v11 thread. The reason I am posting this is that Daniel has said he won't be able to address feedback and issues identified for another 2 weeks but my requirements from my $EMPLOYER are more immediate than that. Feel free to grab this series at: https://github.com/cardoe/xen/tree/doug-mb2-v3 v3 - address review comments by Jan Beulich. They are contained within 5/5. v2 - separate my fixes from Daniel's original series - add back some ACKs I accidentally dropped Daniel Kiper (4): x86: add multiboot2 protocol support efi: build xen.gz with EFI code efi: create new early memory allocator x86: add multiboot2 protocol support for EFI platforms Doug Goldstein (1): fix: add multiboot2 protocol support for EFI platforms xen/arch/x86/Makefile | 2 +- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 361 +-- xen/arch/x86/boot/reloc.c | 148 - xen/arch/x86/efi/Makefile | 12 +- xen/arch/x86/efi/efi-boot.h | 64 +++-- xen/arch/x86/efi/stub.c | 38 +++- xen/arch/x86/setup.c | 3 +- xen/arch/x86/x86_64/asm-offsets.c | 11 +- xen/arch/x86/xen.lds.S| 8 +- xen/common/efi/boot.c | 64 +- xen/common/efi/runtime.c | 9 +- xen/include/xen/multiboot2.h | 169 +++- 13 files changed, 844 insertions(+), 48 deletions(-) create mode 100644 xen/include/xen/multiboot2.h base-commit: 98be5ffc05e689e2131f175ed95b011a7270db67 -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 2/5] efi: build xen.gz with EFI code
From: Daniel Kiper <daniel.ki...@oracle.com>
Build xen.gz with EFI code. We need this to support multiboot2
protocol on EFI platforms.
If we wish to load non-ELF file using multiboot (v1) or multiboot2 then
it must contain "linear" (or "flat") representation of code and data.
This is requirement of both boot protocols. Currently, PE file contains
many sections which are not "linear" (one after another without any holes)
or even do not have representation in a file (e.g. BSS). From EFI point
of view everything is OK and works. However, this file layout cannot be
properly interpreted by multiboot protocols family. In theory there is
a chance that we could build proper PE file (from multiboot protocols POV)
using current build system. However, it means that xen.efi further diverge
from Xen ELF file (in terms of contents and build method). On the other
hand ELF has all needed properties. So, it means that this is good starting
point for further development. Additionally, I think that this is also good
starting point for further xen.efi code and build optimizations. It looks
that there is a chance that finally we can generate xen.efi directly from
Xen ELF using just simple objcopy or other tool. This way we will have one
Xen binary which can be loaded by three boot protocols: EFI native loader,
multiboot (v1) and multiboot2.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
v6 - suggestions/fixes:
- improve efi_enabled() checks in efi_runtime_call()
(suggested by Jan Beulich).
v5 - suggestions/fixes:
- properly calculate efi symbol address in
xen/arch/x86/xen.lds.S (I hope that this
change does not invalidate Jan's ACK).
v4 - suggestions/fixes:
- functions should return -ENOSYS instead
of -EOPNOTSUPP if EFI runtime services
are not available
(suggested by Jan Beulich),
- remove stale bits from xen/arch/x86/Makefile
(suggested by Jan Beulich).
v3 - suggestions/fixes:
- check for EFI platform in EFI code
(suggested by Jan Beulich),
- fix Makefiles
(suggested by Jan Beulich),
- improve commit message
(suggested by Jan Beulich).
v2 - suggestions/fixes:
- build EFI code only if it is supported in a given build environment
(suggested by Jan Beulich).
---
---
xen/arch/x86/Makefile | 2 +-
xen/arch/x86/efi/Makefile | 12
xen/arch/x86/xen.lds.S| 4 ++--
xen/common/efi/boot.c | 3 +++
xen/common/efi/runtime.c | 9 +
5 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 7f6b5d7..2e22cdf 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -219,6 +219,6 @@ efi/mkreloc: efi/mkreloc.c
clean::
rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d
- rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.o efi/.*.d efi/*.efi
efi/disabled efi/mkreloc
+ rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/disabled efi/mkreloc
rm -f boot/reloc.S boot/reloc.lnk boot/reloc.bin
rm -f note.o
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index ad3fdf7..442f3fc 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,18 +1,14 @@
CFLAGS += -fshort-wchar
-obj-y += stub.o
-
-create = test -e $(1) || touch -t 19990101 $(1)
-
efi := y$(shell rm -f disabled)
efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c
check.c 2>disabled && echo y))
efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
-efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o);
$(call create,runtime.o)))
-
-extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o
+efi := $(if $(efi),$(shell rm disabled)y)
%.o: %.ihex
$(OBJCOPY) -I ihex -O binary $< $@
-stub.o: $(extra-y)
+obj-y := stub.o
+obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
+extra-$(efi) += buildid.o
nogcov-$(efi) += stub.o
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 7676de9..b0b1c9b 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -270,10 +270,10 @@ SECTIONS
.pad : {
. = ALIGN(MB(16));
} :text
-#else
- efi = .;
#endif
+ efi = DEFINED(efi) ? efi : .;
+
/* Sections to be discarded */
/DISCARD/ : {
*(.exit.text)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3e5e4ab..df8c702 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1251,6 +1251,9 @@ void __init efi_init_memory(void)
} *extra, *extra_head = NULL;
#endif
+if ( !efi_enabled(EFI_BOOT) )
+return;
+
printk(XENLOG_INFO "EFI memory map:%s\n",
map_bs ? " (mapping BootS
Re: [Xen-devel] [PATCH] xen/common: Drop function calls for Xen compile/version information
On 1/16/17 8:04 AM, Andrew Cooper wrote: > The chageset/version/compile information is currently exported as a set of > function calls into a separate translation unit, which is inefficient for all > callers. > > Replace the function calls with externs pointing appropriately into .rodata, > which allows all users to generate code referencing the data directly. > > No functional change, but causes smaller and more efficient compiled code. > > Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
On 1/16/17 9:11 AM, Daniel Kiper wrote: > On Mon, Jan 16, 2017 at 08:41:08AM -0500, Doug Goldstein wrote: >> On 1/16/17 7:50 AM, Daniel Kiper wrote: >>> On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote: >>>>>>> On 13.01.17 at 20:21, <car...@cardoe.com> wrote: >>>>> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) >>>>> - fix issue where the trampoline size was left as 0 and the >>>>> way the memory is allocated for the trampolines we would go to >>>>> the end of an available section and then subtract off the size >>>>> to decide where to place it. The end result was that we would >>>>> always copy the trampolines and the 32-bit stack into some >>>>> form of reserved memory after the conventional region we >>>>> wanted to put things into. On some systems this did not >>>>> manifest as a crash while on others it did. Reworked the >>>>> changes to always reserve 64kb for both the stack and the size >>>>> of the trampolines. Added an ASSERT to make sure we never blow >>>>> through this size. >>>> >>>> Without having looked at the patch in detail, but knowing I did closely >>>> look at earlier versions (and iirc I was mostly fine with v10) the way >>>> the above is written would require me to either inter-diff the patches, >>>> or re-review the whole thing. For a large patch like this it would be >>>> rather helpful to be quite a bit more specific as to where exactly in the >>>> patch changes were made. >>> >>> I would prefer to not have this patch series applied because it will make me >>> more difficult to prepare v12. I hope that I will post it in about 2 weeks. >>> Though I am going to take into account all comments posted by Doug for v11. >>> >>> Daniel >>> >> >> Why? They're the first 4 patches remaining of your series. It'll >> literally be the following commands: >> >> git fetch origin >> git rebase origin/staging > > If you changed something then probably this is not so simple. > Second, Jan asked me to reorder some patches in the series. > Last but not least, your patch series does not contain whole > functionality which is needed to properly load Xen using > multiboot2 protocol on most EFI platforms. So, as above. > Though I appreciate your feedback and I will take all > your changes into account. > > Daniel > Yes there are some code changes which is the point of me sending this. But the work for you is the same as having to rebase your series over the past few years. Its still a rebase. Its the same thing that everyone submitting patches has to do when they submit another version of their patches. I believe you identified 1 machine that had an issue with the 1mb region. I've used a 12 machines to test this series and these 4 patches work on those machines. The relocation part of the series works on NONE of the machines I've tested with. So I would argue the opposite. I still haven't identified what's wrong with the relocation part of the series. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 5/5] fix: add multiboot2 protocol support for EFI platforms
This should be squashed into the 4/4 patch 'x86: add multiboot2 protocol
support for EFI platforms'.
- fix incorrect assembly (identified by Andrew Cooper)
- fix issue where the trampoline size was left as 0 and the
way the memory is allocated for the trampolines we would go to
the end of an available section and then subtract off the size
to decide where to place it. The end result was that we would
always copy the trampolines and the 32-bit stack into some
form of reserved memory after the conventional region we
wanted to put things into. On some systems this did not
manifest as a crash while on others it did. Reworked the
changes to always reserve 64kb for both the stack and the size
of the trampolines. Added an ASSERT to make sure we never blow
through this size.
Signed-off-by: Doug Goldstein <car...@cardoe.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
Doug v2 - new in this version to help show what's changed
---
---
xen/arch/x86/boot/head.S| 1 +
xen/arch/x86/efi/efi-boot.h | 9 +++--
xen/arch/x86/efi/stub.c | 2 +-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index ac93df0..876a6b1 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -519,6 +519,7 @@ trampoline_setup:
1:
/* Switch to low-memory stack. */
mov sym_phys(trampoline_phys),%edi
+/* The stack starts 64kb after the location of trampoline_phys */
lea 0x1(%edi),%esp
lea trampoline_boot_cpu_entry-trampoline_start(%edi),%eax
pushl $BOOT_CS32
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index dc857d8..af97fb9 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -170,9 +170,10 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
/* fall through */
case EfiConventionalMemory:
if ( !trampoline_phys && desc->PhysicalStart + len <= 0x10 &&
- len >= cfg.size + extra_mem &&
- desc->PhysicalStart + len > cfg.addr )
+ len >= cfg.size && desc->PhysicalStart + len > cfg.addr ) {
+ASSERT(cfg.size > 0);
cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
+}
/* fall through */
case EfiLoaderCode:
case EfiLoaderData:
@@ -686,6 +687,10 @@ paddr_t __init efi_multiboot2(EFI_HANDLE ImageHandle,
EFI_SYSTEM_TABLE *SystemTa
setup_efi_pci();
efi_variables();
+/* This is the maximum size of our trampoline + our low memory stack */
+cfg.size = 64 << 10;
+ASSERT(cfg.size >= ((trampoline_end - trampoline_start) + 4096));
+
if ( gop )
efi_set_gop_mode(gop, gop_mode);
diff --git a/xen/arch/x86/efi/stub.c b/xen/arch/x86/efi/stub.c
index 6ea6aa1..b81adc0 100644
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -33,7 +33,7 @@ paddr_t __init noreturn efi_multiboot2(EFI_HANDLE ImageHandle,
* not be directly supported by C compiler.
*/
asm volatile(
-"call %2 \n"
+"call *%2 \n"
"0: hlt \n"
"jmp 0b \n"
: "+c" (StdErr), "+d" (err) : "g" (StdErr->OutputString)
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 0/5] multiboot2 protocol support
This is a series based on v11 of Daniel Kiper's "x86: multiboot2 protocol support" series. It aims to collect up all the fixes and changes that Andrew Cooper, Jan Beulich and myself discovered in code review and testing on actual hardware. I've had problems with the relocation portion of the series so I've dropped it as all the hardware I am needing to support presently for my $EMPLOYER does not load anything at the 1mb mark. To me this adds MB2 support for all pieces of hardware that don't have things located at 1mb so it's an incremental step. I've also dropped the early command line conversion to C as it was done in support of the relocation changes and therefore not necessary. In the end my goal is to help Daniel out by providing the portion of the series that works on half a dozen physical machines I've tested with and integrates all changes as discussed on the v11 thread. The reason I am posting this is that Daniel has said he won't be able to address feedback and issues identified for another 2 weeks but my requirements from my $EMPLOYER are more immediate than that. v2 - separate my fixes from Daniel's original series - add back some ACKs I accidentally dropped Daniel Kiper (4): x86: add multiboot2 protocol support efi: build xen.gz with EFI code efi: create new early memory allocator x86: add multiboot2 protocol support for EFI platforms Doug Goldstein (1): fix: add multiboot2 protocol support for EFI platforms xen/arch/x86/Makefile | 2 +- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 361 +-- xen/arch/x86/boot/reloc.c | 148 - xen/arch/x86/efi/Makefile | 12 +- xen/arch/x86/efi/efi-boot.h | 70 -- xen/arch/x86/efi/stub.c | 38 +++- xen/arch/x86/setup.c | 3 +- xen/arch/x86/x86_64/asm-offsets.c | 11 +- xen/arch/x86/xen.lds.S| 8 +- xen/common/efi/boot.c | 64 +- xen/common/efi/runtime.c | 9 +- xen/include/xen/multiboot2.h | 169 +++- 13 files changed, 849 insertions(+), 49 deletions(-) create mode 100644 xen/include/xen/multiboot2.h base-commit: 98be5ffc05e689e2131f175ed95b011a7270db67 -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 4/5] x86: add multiboot2 protocol support for EFI platforms
From: Daniel Kiper <daniel.ki...@oracle.com> This way Xen can be loaded on EFI platforms using GRUB2 and other boot loaders which support multiboot2 protocol. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> Tested-by: Doug Goldstein <car...@cardoe.com> --- Doug v2 - dropped all my changes and moved them into their own patch Doug v1 - fix incorrect assembly (identified by Andrew Cooper) - fix issue where the trampoline size was left as 0 and the way the memory is allocated for the trampolines we would go to the end of an available section and then subtract off the size to decide where to place it. The end result was that we would always copy the trampolines and the 32-bit stack into some form of reserved memory after the conventional region we wanted to put things into. On some systems this did not manifest as a crash while on others it did. Reworked the changes to always reserve 64kb for both the stack and the size of the trampolines. Added an ASSERT to make sure we never blow through this size. v10 - suggestions/fixes: - replace ljmpl with lretq (suggested by Andrew Cooper), - introduce efi_platform to increase code readability (suggested by Andrew Cooper). v9 - suggestions/fixes: - use .L labels instead of numeric ones in multiboot2 data scanning loops (suggested by Jan Beulich). v8 - suggestions/fixes: - use __bss_start(%rip)/__bss_end(%rip) instead of of .startof.(.bss)(%rip)/$.sizeof.(.bss) because latter is not tested extensively in different built environments yet (suggested by Andrew Cooper), - fix multiboot2 data scanning loop in x86_32 code (suggested by Jan Beulich), - add check for extra mem for mbi data if Xen is loaded via multiboot2 protocol on EFI platform (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v7 - suggestions/fixes: - do not allocate twice memory for trampoline if we were loaded via multiboot2 protocol on EFI platform, - wrap long line (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich). v6 - suggestions/fixes: - improve label names in assembly error printing code (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups and fixes (suggested by Jan Beulich). v4 - suggestions/fixes: - remove redundant BSS alignment, - update BSS alignment check, - use __set_bit() instead of set_bit() if possible (suggested by Jan Beulich), - call efi_arch_cpu() from efi_multiboot2() even if the same work is done later in other place right now (suggested by Jan Beulich), - xen/arch/x86/efi/stub.c:efi_multiboot2() fail properly on EFI platforms, - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - use 32-bit registers in x86_64 code if possible (suggested by Jan Beulich), - multiboot2 information address is 64-bit in x86_64 code, so, treat it is as is (suggested by Jan Beulich), - use cmovcc if possible, - leave only one space between rep and stosq (suggested by Jan Beulich), - improve error handling, - improve early error messages, (suggested by Jan Beulich), - improve early error messages printing code, - improve label names (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - various minor cleanups. v3 - suggestions/fixes: - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - improve segment registers initialization (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich and Konrad Rzeszutek Wilk), - improve commit message (suggested by Jan Beulich). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - switch CPU to x86_32 mode before jumping to 32-bit code (suggested by Andrew Cooper), - reduce code changes to increase patch readability (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - ignore MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO tag on EFI platform and find on my own multiboot2.mem_lower value, - stop execution if EFI platform is detected in legacy BIOS path. --- --- xen/arch/x86/boot/head.S | 263 +-- xen/arch/x86/efi/efi-boot.h | 54 +- xen/arch/x86/efi/stub.c | 38 - xen/arch/x86/x86_64/asm-offsets.c | 2 +- xen/arch/x86/xen.lds.S| 4 +- xen/common/efi/boot.c | 11 +- 6 files changed, 349 insertions(+), 23 deletions(-) diff --git a/xen/arch/x86/boot/head.S b/xen/ar
[Xen-devel] [PATCH v2 1/5] x86: add multiboot2 protocol support
From: Daniel Kiper <daniel.ki...@oracle.com> Add multiboot2 protocol support. Alter min memory limit handling as we now may not find it from either multiboot (v1) or multiboot2. This way we are laying the foundation for EFI + GRUB2 + Xen development. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Jan Beulich <jbeul...@suse.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- v9 - suggestions/fixes: - use .L label instead of numeric one in multiboot2 data scanning loop; I hope that this change does not invalidate Jan's Reviewed-by (suggested by Jan Beulich). v8 - suggestions/fixes: - use sizeof(/) instead of sizeof() if it is possible (suggested by Jan Beulich). v7 - suggestions/fixes: - rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively (suggested by Jan Beulich), - initialize mbi_out->flags using "|=" instead of "=" (suggested by Jan Beulich), - use sizeof(*mmap_dst) instead of sizeof(memory_map_t) if it makes sense (suggested by Jan Beulich). v6 - suggestions/fixes: - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - do not index mbi_out_mods[] beyond its end (suggested by Andrew Cooper), - reduce number of casts (suggested by Andrew Cooper and Jan Beulich), - add braces to increase code readability (suggested by Andrew Cooper). v5 - suggestions/fixes: - check multiboot2_tag_mmap_t.entry_size before multiboot2_tag_mmap_t.entries[] use (suggested by Jan Beulich), - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - use "type name[]" instad of "type name[0]" in xen/include/xen/multiboot2.h (suggested by Jan Beulich), - remove unneeded comment (suggested by Jan Beulich). v4 - suggestions/fixes: - avoid assembly usage in xen/arch/x86/boot/reloc.c, - fix boundary check issue and optimize for() loops in mbi2_mbi(), - move to stdcall calling convention, - remove unneeded typeof() from ALIGN_UP() macro (suggested by Jan Beulich), - add and use NULL definition in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - add :req to some .macro arguments (suggested by Jan Beulich), - use cmovcc if possible, - add .L to multiboot2_header_end label (suggested by Jan Beulich), - add .L to multiboot2_proto label (suggested by Jan Beulich), - improve label names (suggested by Jan Beulich). v3 - suggestions/fixes: - reorder reloc() arguments (suggested by Jan Beulich), - remove .L from multiboot2 header labels (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk), - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - create modules data if modules count != 0 (suggested by Jan Beulich), - improve macros (suggested by Jan Beulich), - reduce number of casts (suggested by Jan Beulich), - use const if possible (suggested by Jan Beulich), - drop static and __used__ attribute from reloc() (suggested by Jan Beulich), - remove isolated/stray __packed attribute from multiboot2_memory_map_t type definition (suggested by Jan Beulich), - reformat xen/include/xen/multiboot2.h (suggested by Konrad Rzeszutek Wilk), - improve comments (suggested by Konrad Rzeszutek Wilk), - remove hard tabs (suggested by Jan Beulich and Konrad Rzeszutek Wilk). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - simplify assembly in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - do not include include/xen/compiler.h in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information (suggested by Jan Beulich). v2 - not fixed yet: - dynamic dependency generation for xen/arch/x86/boot/reloc.S; this requires more work; I am not sure that it pays because potential patch requires more changes than addition of just multiboot2.h to Makefile (suggested by Jan Beulich), - isolated/stray __packed attribute usage for multiboot2_memory_map_t (suggested by Jan Beulich). --- --- xen/arch/x86/boot/Makefile| 3 +- xen/arch/x86/boot/head.S | 107 +++- xen/arch/x86/boot/reloc.c | 148 ++- xen/arch/x86/x86_64/asm-offsets.c | 9 ++- xen/include/xen/multiboot2.h | 169 +++- 5 files changed, 426 insertions(+), 10 deletions(-) create mode 100644 xen/include/xen/multiboot2.h diff --git a/xen/arch/x86/boot/
[Xen-devel] [PATCH v2 2/5] efi: build xen.gz with EFI code
From: Daniel Kiper <daniel.ki...@oracle.com>
Build xen.gz with EFI code. We need this to support multiboot2
protocol on EFI platforms.
If we wish to load non-ELF file using multiboot (v1) or multiboot2 then
it must contain "linear" (or "flat") representation of code and data.
This is requirement of both boot protocols. Currently, PE file contains
many sections which are not "linear" (one after another without any holes)
or even do not have representation in a file (e.g. BSS). From EFI point
of view everything is OK and works. However, this file layout cannot be
properly interpreted by multiboot protocols family. In theory there is
a chance that we could build proper PE file (from multiboot protocols POV)
using current build system. However, it means that xen.efi further diverge
from Xen ELF file (in terms of contents and build method). On the other
hand ELF has all needed properties. So, it means that this is good starting
point for further development. Additionally, I think that this is also good
starting point for further xen.efi code and build optimizations. It looks
that there is a chance that finally we can generate xen.efi directly from
Xen ELF using just simple objcopy or other tool. This way we will have one
Xen binary which can be loaded by three boot protocols: EFI native loader,
multiboot (v1) and multiboot2.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Acked-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Doug Goldstein <car...@cardoe.com>
---
v6 - suggestions/fixes:
- improve efi_enabled() checks in efi_runtime_call()
(suggested by Jan Beulich).
v5 - suggestions/fixes:
- properly calculate efi symbol address in
xen/arch/x86/xen.lds.S (I hope that this
change does not invalidate Jan's ACK).
v4 - suggestions/fixes:
- functions should return -ENOSYS instead
of -EOPNOTSUPP if EFI runtime services
are not available
(suggested by Jan Beulich),
- remove stale bits from xen/arch/x86/Makefile
(suggested by Jan Beulich).
v3 - suggestions/fixes:
- check for EFI platform in EFI code
(suggested by Jan Beulich),
- fix Makefiles
(suggested by Jan Beulich),
- improve commit message
(suggested by Jan Beulich).
v2 - suggestions/fixes:
- build EFI code only if it is supported in a given build environment
(suggested by Jan Beulich).
---
---
xen/arch/x86/Makefile | 2 +-
xen/arch/x86/efi/Makefile | 12
xen/arch/x86/xen.lds.S| 4 ++--
xen/common/efi/boot.c | 3 +++
xen/common/efi/runtime.c | 9 +
5 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 7f6b5d7..2e22cdf 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -219,6 +219,6 @@ efi/mkreloc: efi/mkreloc.c
clean::
rm -f asm-offsets.s *.lds boot/*.o boot/*~ boot/core boot/mkelf32
rm -f $(BASEDIR)/.xen-syms.[0-9]* boot/.*.d
- rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.o efi/.*.d efi/*.efi
efi/disabled efi/mkreloc
+ rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.efi efi/disabled efi/mkreloc
rm -f boot/reloc.S boot/reloc.lnk boot/reloc.bin
rm -f note.o
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index ad3fdf7..442f3fc 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -1,18 +1,14 @@
CFLAGS += -fshort-wchar
-obj-y += stub.o
-
-create = test -e $(1) || touch -t 19990101 $(1)
-
efi := y$(shell rm -f disabled)
efi := $(if $(efi),$(shell $(CC) $(filter-out $(CFLAGS-y) .%.d,$(CFLAGS)) -c
check.c 2>disabled && echo y))
efi := $(if $(efi),$(shell $(LD) -mi386pep --subsystem=10 -o check.efi check.o
2>disabled && echo y))
-efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o);
$(call create,runtime.o)))
-
-extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o buildid.o
+efi := $(if $(efi),$(shell rm disabled)y)
%.o: %.ihex
$(OBJCOPY) -I ihex -O binary $< $@
-stub.o: $(extra-y)
+obj-y := stub.o
+obj-$(efi) := boot.init.o compat.o relocs-dummy.o runtime.o
+extra-$(efi) += buildid.o
nogcov-$(efi) += stub.o
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 7676de9..b0b1c9b 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -270,10 +270,10 @@ SECTIONS
.pad : {
. = ALIGN(MB(16));
} :text
-#else
- efi = .;
#endif
+ efi = DEFINED(efi) ? efi : .;
+
/* Sections to be discarded */
/DISCARD/ : {
*(.exit.text)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 3e5e4ab..df8c702 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1251,6 +1251,9 @@ void __init efi_init_memory(void)
} *extra, *extra_head = NULL;
#endif
+if ( !efi_enabled(EFI_BOOT) )
+return;
+
printk(XENLOG_INFO "EFI memory map:%s\n",
map_bs ? " (mapping BootS
[Xen-devel] [PATCH v2 3/5] efi: create new early memory allocator
From: Daniel Kiper <daniel.ki...@oracle.com> There is a problem with place_string() which is used as early memory allocator. It gets memory chunks starting from start symbol and goes down. Sadly this does not work when Xen is loaded using multiboot2 protocol because then the start lives on 1 MiB address and we should not allocate a memory from below of it. So, I tried to use mem_lower address calculated by GRUB2. However, this solution works only on some machines. There are machines in the wild (e.g. Dell PowerEdge R820) which uses first ~640 KiB for boot services code or data... :-((( Hence, we need new memory allocator for Xen EFI boot code which is quite simple and generic and could be used by place_string() and efi_arch_allocate_mmap_buffer(). I think about following solutions: 1) We could use native EFI allocation functions (e.g. AllocatePool() or AllocatePages()) to get memory chunk. However, later (somewhere in __start_xen()) we must copy its contents to safe place or reserve it in e820 memory map and map it in Xen virtual address space. This means that the code referring to Xen command line, loaded modules and EFI memory map, mostly in __start_xen(), will be further complicated and diverge from legacy BIOS cases. Additionally, both former things have to be placed below 4 GiB because their addresses are stored in multiboot_info_t structure which has 32-bit relevant members. 2) We may allocate memory area statically somewhere in Xen code which could be used as memory pool for early dynamic allocations. Looks quite simple. Additionally, it would not depend on EFI at all and could be used on legacy BIOS platforms if we need it. However, we must carefully choose size of this pool. We do not want increase Xen binary size too much and waste too much memory but also we must fit at least memory map on x86 EFI platforms. As I saw on small machine, e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more than 200 entries. Every entry on x86-64 platform is 40 bytes in size. So, it means that we need more than 8 KiB for EFI memory map only. Additionally, if we use this memory pool for Xen and modules command line storage (it would be used when xen.efi is executed as EFI application) then we should add, I think, about 1 KiB. In this case, to be on safe side, we should assume at least 64 KiB pool for early memory allocations. Which is about 4 times of our earlier calculations. However, during discussion on Xen-devel Jan Beulich suggested that just in case we should use 1 MiB memory pool like it is in original place_string() implementation. So, let's use 1 MiB as it was proposed. If we think that we should not waste unallocated memory in the pool on running system then we can mark this region as __initdata and move all required data to dynamically allocated places somewhere in __start_xen(). 2a) We could put memory pool into .bss.page_aligned section. Then allocate memory chunks starting from the lowest address. After init phase we can free unused portion of the memory pool as in case of .init.text or .init.data sections. This way we do not need to allocate any space in image file and freeing of unused area in the memory pool is very simple. Now #2a solution is implemented because it is quite simple and requires limited number of changes, especially in __start_xen(). The new allocator is quite generic and can be used on ARM platforms too. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Acked-by: Jan Beulich <jbeul...@suse.com> Acked-by: Julien Grall <julien.gr...@arm.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- Doug v1 - removed stale paragraph v11 - suggestions/fixes: - #ifdef only EBMALLOC_SIZE from ebmalloc machinery (suggested by Jan Beulich). v10 - suggestions/fixes: - remove unneeded ARM free_ebmalloc_unused_mem() stub. v9 - suggestions/fixes: - call free_ebmalloc_unused_mem() from efi_init_memory() instead of xen/arch/arm/setup.c:init_done() (suggested by Jan Beulich), - improve comments. v8 - suggestions/fixes: - disable whole ebmalloc machinery on ARM platforms, - add comment saying what should be done before enabling ebmalloc on ARM, (suggested by Julien Grall), - move ebmalloc code before efi-boot.h inclusion and remove unneeded forward declaration (suggested by Jan Beulich), - remove free_ebmalloc_unused_mem() call from xen/arch/arm/setup.c:init_done() (suggested by Julien Grall), - improve commit message. v7 - suggestions/fixes: - enable most of ebmalloc machinery on ARM platforms (suggested by Jan Beulich), - remove unneeded cast (suggested by Jan Beulich), - wrap long line (suggested by Jan Beulich), - improve commit message. v6 - suggestions/fixes: - optimize ebmalloc allocator, - move ebmalloc machinery to xen/com
Re: [Xen-devel] [PATCH 3/4] efi: create new early memory allocator
On 1/16/17 6:52 AM, Jan Beulich wrote: >>>> On 13.01.17 at 20:21, <car...@cardoe.com> wrote: >> From: Daniel Kiper <daniel.ki...@oracle.com> >> >> There is a problem with place_string() which is used as early memory >> allocator. It gets memory chunks starting from start symbol and goes >> down. Sadly this does not work when Xen is loaded using multiboot2 >> protocol because then the start lives on 1 MiB address and we should >> not allocate a memory from below of it. So, I tried to use mem_lower >> address calculated by GRUB2. However, this solution works only on some >> machines. There are machines in the wild (e.g. Dell PowerEdge R820) >> which uses first ~640 KiB for boot services code or data... :-((( >> Hence, we need new memory allocator for Xen EFI boot code which is >> quite simple and generic and could be used by place_string() and >> efi_arch_allocate_mmap_buffer(). I think about following solutions: >> >> 1) We could use native EFI allocation functions (e.g. AllocatePool() >>or AllocatePages()) to get memory chunk. However, later (somewhere >>in __start_xen()) we must copy its contents to safe place or reserve >>it in e820 memory map and map it in Xen virtual address space. This >>means that the code referring to Xen command line, loaded modules and >>EFI memory map, mostly in __start_xen(), will be further complicated >>and diverge from legacy BIOS cases. Additionally, both former things >>have to be placed below 4 GiB because their addresses are stored in >>multiboot_info_t structure which has 32-bit relevant members. >> >> 2) We may allocate memory area statically somewhere in Xen code which >>could be used as memory pool for early dynamic allocations. Looks >>quite simple. Additionally, it would not depend on EFI at all and >>could be used on legacy BIOS platforms if we need it. However, we >>must carefully choose size of this pool. We do not want increase Xen >>binary size too much and waste too much memory but also we must fit >>at least memory map on x86 EFI platforms. As I saw on small machine, >>e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more >>than 200 entries. Every entry on x86-64 platform is 40 bytes in size. >>So, it means that we need more than 8 KiB for EFI memory map only. >>Additionally, if we use this memory pool for Xen and modules command >>line storage (it would be used when xen.efi is executed as EFI >> application) >>then we should add, I think, about 1 KiB. In this case, to be on safe >>side, we should assume at least 64 KiB pool for early memory allocations. >>Which is about 4 times of our earlier calculations. However, during >>discussion on Xen-devel Jan Beulich suggested that just in case we should >>use 1 MiB memory pool like it is in original place_string() >> implementation. >>So, let's use 1 MiB as it was proposed. If we think that we should not >>waste unallocated memory in the pool on running system then we can mark >>this region as __initdata and move all required data to dynamically >>allocated places somewhere in __start_xen(). >> >> 2a) We could put memory pool into .bss.page_aligned section. Then allocate >> memory chunks starting from the lowest address. After init phase we can >> free unused portion of the memory pool as in case of .init.text or >> .init.data >> sections. This way we do not need to allocate any space in image file and >> freeing of unused area in the memory pool is very simple. >> >> Now #2a solution is implemented because it is quite simple and requires >> limited number of changes, especially in __start_xen(). >> >> The new allocator is quite generic and can be used on ARM platforms too. >> >> Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> >> Reviewed-by: Doug Goldstein <car...@cardoe.com> > > You've lost (at least) Julien's and my ack here. > > Jan > Yes. My mistake. I pulled in the patch wholesale without adding the acks for this patch since they were replies. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
On 1/16/17 7:02 AM, Jan Beulich wrote: >>>> On 13.01.17 at 20:21, <car...@cardoe.com> wrote: >> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) >> - fix issue where the trampoline size was left as 0 and the >> way the memory is allocated for the trampolines we would go to >> the end of an available section and then subtract off the size >> to decide where to place it. The end result was that we would >> always copy the trampolines and the 32-bit stack into some >> form of reserved memory after the conventional region we >> wanted to put things into. On some systems this did not >> manifest as a crash while on others it did. Reworked the >> changes to always reserve 64kb for both the stack and the size >> of the trampolines. Added an ASSERT to make sure we never blow >> through this size. > > Without having looked at the patch in detail, but knowing I did closely > look at earlier versions (and iirc I was mostly fine with v10) the way > the above is written would require me to either inter-diff the patches, > or re-review the whole thing. For a large patch like this it would be > rather helpful to be quite a bit more specific as to where exactly in the > patch changes were made. > > Jan > I'll submit a diff against v11 to help show the difference. I can also submit a difference against v10 if you want as well. -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
On 1/16/17 7:50 AM, Daniel Kiper wrote: > On Mon, Jan 16, 2017 at 05:02:05AM -0700, Jan Beulich wrote: >>>>> On 13.01.17 at 20:21, <car...@cardoe.com> wrote: >>> Doug v1 - fix incorrect assembly (identified by Andrew Cooper) >>> - fix issue where the trampoline size was left as 0 and the >>> way the memory is allocated for the trampolines we would go to >>> the end of an available section and then subtract off the size >>> to decide where to place it. The end result was that we would >>> always copy the trampolines and the 32-bit stack into some >>> form of reserved memory after the conventional region we >>> wanted to put things into. On some systems this did not >>> manifest as a crash while on others it did. Reworked the >>> changes to always reserve 64kb for both the stack and the size >>> of the trampolines. Added an ASSERT to make sure we never blow >>> through this size. >> >> Without having looked at the patch in detail, but knowing I did closely >> look at earlier versions (and iirc I was mostly fine with v10) the way >> the above is written would require me to either inter-diff the patches, >> or re-review the whole thing. For a large patch like this it would be >> rather helpful to be quite a bit more specific as to where exactly in the >> patch changes were made. > > I would prefer to not have this patch series applied because it will make me > more difficult to prepare v12. I hope that I will post it in about 2 weeks. > Though I am going to take into account all comments posted by Doug for v11. > > Daniel > Why? They're the first 4 patches remaining of your series. It'll literally be the following commands: git fetch origin git rebase origin/staging -- Doug Goldstein signature.asc Description: OpenPGP digital signature ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 3/4] efi: create new early memory allocator
From: Daniel Kiper <daniel.ki...@oracle.com> There is a problem with place_string() which is used as early memory allocator. It gets memory chunks starting from start symbol and goes down. Sadly this does not work when Xen is loaded using multiboot2 protocol because then the start lives on 1 MiB address and we should not allocate a memory from below of it. So, I tried to use mem_lower address calculated by GRUB2. However, this solution works only on some machines. There are machines in the wild (e.g. Dell PowerEdge R820) which uses first ~640 KiB for boot services code or data... :-((( Hence, we need new memory allocator for Xen EFI boot code which is quite simple and generic and could be used by place_string() and efi_arch_allocate_mmap_buffer(). I think about following solutions: 1) We could use native EFI allocation functions (e.g. AllocatePool() or AllocatePages()) to get memory chunk. However, later (somewhere in __start_xen()) we must copy its contents to safe place or reserve it in e820 memory map and map it in Xen virtual address space. This means that the code referring to Xen command line, loaded modules and EFI memory map, mostly in __start_xen(), will be further complicated and diverge from legacy BIOS cases. Additionally, both former things have to be placed below 4 GiB because their addresses are stored in multiboot_info_t structure which has 32-bit relevant members. 2) We may allocate memory area statically somewhere in Xen code which could be used as memory pool for early dynamic allocations. Looks quite simple. Additionally, it would not depend on EFI at all and could be used on legacy BIOS platforms if we need it. However, we must carefully choose size of this pool. We do not want increase Xen binary size too much and waste too much memory but also we must fit at least memory map on x86 EFI platforms. As I saw on small machine, e.g. IBM System x3550 M2 with 8 GiB RAM, memory map may contain more than 200 entries. Every entry on x86-64 platform is 40 bytes in size. So, it means that we need more than 8 KiB for EFI memory map only. Additionally, if we use this memory pool for Xen and modules command line storage (it would be used when xen.efi is executed as EFI application) then we should add, I think, about 1 KiB. In this case, to be on safe side, we should assume at least 64 KiB pool for early memory allocations. Which is about 4 times of our earlier calculations. However, during discussion on Xen-devel Jan Beulich suggested that just in case we should use 1 MiB memory pool like it is in original place_string() implementation. So, let's use 1 MiB as it was proposed. If we think that we should not waste unallocated memory in the pool on running system then we can mark this region as __initdata and move all required data to dynamically allocated places somewhere in __start_xen(). 2a) We could put memory pool into .bss.page_aligned section. Then allocate memory chunks starting from the lowest address. After init phase we can free unused portion of the memory pool as in case of .init.text or .init.data sections. This way we do not need to allocate any space in image file and freeing of unused area in the memory pool is very simple. Now #2a solution is implemented because it is quite simple and requires limited number of changes, especially in __start_xen(). The new allocator is quite generic and can be used on ARM platforms too. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> Reviewed-by: Doug Goldstein <car...@cardoe.com> --- Doug v1 - removed stale paragraph v11 - suggestions/fixes: - #ifdef only EBMALLOC_SIZE from ebmalloc machinery (suggested by Jan Beulich). v10 - suggestions/fixes: - remove unneeded ARM free_ebmalloc_unused_mem() stub. v9 - suggestions/fixes: - call free_ebmalloc_unused_mem() from efi_init_memory() instead of xen/arch/arm/setup.c:init_done() (suggested by Jan Beulich), - improve comments. v8 - suggestions/fixes: - disable whole ebmalloc machinery on ARM platforms, - add comment saying what should be done before enabling ebmalloc on ARM, (suggested by Julien Grall), - move ebmalloc code before efi-boot.h inclusion and remove unneeded forward declaration (suggested by Jan Beulich), - remove free_ebmalloc_unused_mem() call from xen/arch/arm/setup.c:init_done() (suggested by Julien Grall), - improve commit message. v7 - suggestions/fixes: - enable most of ebmalloc machinery on ARM platforms (suggested by Jan Beulich), - remove unneeded cast (suggested by Jan Beulich), - wrap long line (suggested by Jan Beulich), - improve commit message. v6 - suggestions/fixes: - optimize ebmalloc allocator, - move ebmalloc machinery to xen/common/efi/boot.c (suggested by Jan Beulich), - enforce PAGE_SIZE ebmalloc_mem alignment (sugges
