Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms

From: Daniel Kiper <daniel.kiper@oracle.com>
To: Doug Goldstein <cardoe@cardoe.com>
Cc: Juergen Gross <JGross@suse.com>,
	sstabellini@kernel.org, konrad.wilk@oracle.com,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	pgnet.dev@gmail.com, ning.sun@intel.com, julien.grall@arm.com,
	Jan Beulich <JBeulich@suse.com>,
	qiaowei.ren@intel.com, xen-devel@lists.xen.org,
	gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH 4/4] x86: add multiboot2 protocol support for EFI platforms
Date: Mon, 16 Jan 2017 15:11:57 +0100	[thread overview]
Message-ID: <20170116141157.GT23864@olila.local.net-space.pl> (raw)
In-Reply-To: <bc9aa387-3a34-73ac-023d-e66dfe096430@cardoe.com>

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, <cardoe@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

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel