[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 2/2] x86/setup: remap Xen image up to PFN_DOWN(__pa(_end))

>>> On 10.01.18 at 14:05, <daniel.kiper@xxxxxxxxxx> wrote:
> Current limit, PFN_DOWN(xen_phys_start), introduced by commit b280442
> (x86: make Xen early boot code relocatable) is not reliable. Potentially
> its value may fall below PFN_DOWN(__pa(_end)) and then part of Xen image
> may not be mapped after relocation. This will not happen in current code
> thanks to "x86/setup: do not relocate over current Xen image placement"
> patch. Though this safety measure may save a lot of debugging time when
> somebody decide to relax existing relocation restrictions one day.

I've gone back through the v2 discussion, and I continue to fail to
see what is being fixed here, even if just theoretically. It is bad
enough that the description here isn't clarifying this and I need to
go back to the earlier discussion, but it's even worse if even that
earlier discussion didn't really help. My conclusion is that you're
talking about a case where old and positions of Xen overlap, a
case which I thought patch 1 eliminates.

> Additionally, remapping will execute a bit faster due to this change.

Besides it hardly mattering - how come?

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -973,6 +973,11 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>              l3_pgentry_t *pl3e;
>              l2_pgentry_t *pl2e;
>              int i, j, k;
> +            /*
> +             * We have to calculate xen_remap_end_pfn before
> +             * xen_phys_start change.
> +             */
> +            unsigned long xen_remap_end_pfn = PFN_DOWN(__pa(_end));

In case you can provide a convincing reason for the patch to be
needed (or at least wanted) - the xen_ prefix is pointless here,
and you might help the compiler (and maybe also the reader) a
little by declaring the whole thing const.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.