Re: [Xen-devel] [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
On Thu, Jan 19, 2017 at 09:25:08AM -0500, Doug Goldstein wrote: > 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 Do you use QEMU with KVM enabled? I was hit twice by an issue (I do not remember the detail right now) when OVMF was running in KVM. When I disabled KVM then everything worked without any issue. It does not happen often but... > 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. Hmmm... I have not seen that anywhere. And nobody except you reported such issue. Is it happening with GRUB2 and/or iPXE? Anyway, I have just released v12. It contains various fixes including yours. I hope that I have not missed anything. Please take a look. In the meantime I will try to find one of machines mentioned above and reproduce this issue there. Daniel ___ 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
Re: [Xen-devel] [PATCH v4 5/5] fix: add multiboot2 protocol support for EFI platforms
On Wed, Jan 18, 2017 at 10:40:07PM -0500, Doug Goldstein wrote: > 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. I do not know why you read this in that way. I have just stated the fact. No more no less. > > 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. I do not fully agree, however, I respect your point of view. Though I am not going to discuss this any longer. I have just stated my preference clearly earlier. Maintainers are the Supreme Court and they will decide what to do. And I am not going to discuss this too. I prefer to focus on my work. > 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. Daniel ___ 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/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 5/5] fix: add multiboot2 protocol support for EFI platforms
On Wed, Jan 18, 2017 at 07:52:44AM -0700, Jan Beulich wrote: > >>> On 18.01.17 at 15:17, wrote: > > 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 > > Reviewed-by: Jan Beulich > with ... > > > --- 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 must fit in 64kb. In testing > > + * the low memory stack never exceeded 1kb so just require that the > > + * trampolines fit in 63kb, leaving 1kb for the stack. > > + */ > > +ASSERT((trampoline_end - trampoline_start) < KB(63), > > +"not enough room for trampolines and low memory stack") > > ... 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? 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. Daniel ___ 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 18/01/17 14:17, Doug Goldstein wrote: > 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 LGTM, but this really does want squashing. ~Andrew ___ 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 18.01.17 at 15:17, wrote: > 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 Reviewed-by: Jan Beulich with ... > --- 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 must fit in 64kb. In testing > + * the low memory stack never exceeded 1kb so just require that the > + * trampolines fit in 63kb, leaving 1kb for the stack. > + */ > +ASSERT((trampoline_end - trampoline_start) < KB(63), > +"not enough room for trampolines and low memory stack") ... the comment style here fixed (which could be done upon commit or when Daniel merges this back into his series). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
