Re: [Xen-devel] [PATCH 2/2] x86: map portion of kexec crash area that is within the direct map area

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.01.14 at 16:56, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> +    if ( kexec_crash_area.size )

Wouldn't this better also include a kexec_crash_area.start range
check?

> +    {
> +        unsigned long s = PFN_DOWN(kexec_crash_area.start);
> +        unsigned long e = min(s + PFN_UP(kexec_crash_area.size),
> +                              PFN_UP(__pa(HYPERVISOR_VIRT_END - 1)));
> +
> +        map_pages_to_xen((unsigned long)__va(kexec_crash_area.start),
> +                         s, e - s, PAGE_HYPERVISOR);

map_pages_to_xen() doesn't tolerate a huge count resulting when
e < s (which is possible due to the min() above).

Furthermore, with PFN compression and a badly specified address
range (overlapping a hole) this would likely crash during boot. While
that mistake might later also lead to problems, I think it would be
better if booting nevertheless succeeded.

And of course the whole thing will break the moment we allow RAM
to fall into PFN compression holes (either by not using some small
amount of memory in order to be able to cover a larger total
amount, like would be desirable in cases where we need to chop
off a large piece at the top, but could do with not using a couple
of gigabytes relocated from the range below 4Gb, or via command
line option), which is why I told you that using map_domain_page()
for the kexec crash area is a bad thing in the first place.

Jan

> +    }



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