From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Vrabel <david.vrabel@citrix.com>
Subject: Re: [PATCH 2/2] x86: map portion of kexec crash area
 that is within the direct map area
Date: Wed, 8 Jan 2014 17:57:43 +0000
Message-ID: <52CD9197.50305@citrix.com>
References: <1389196601-12219-1-git-send-email-david.vrabel@citrix.com>
	<1389196601-12219-3-git-send-email-david.vrabel@citrix.com>
	<52CD8D1A0200007800111981@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <52CD8D1A0200007800111981@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Don Slutz <dslutz@verizon.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On 08/01/14 16:38, Jan Beulich wrote:
>>>> On 08.01.14 at 16:56, David Vrabel <david.vrabel@citrix.com> wrote:
>> +    if ( kexec_crash_area.size )
> 
> Wouldn't this better also include a kexec_crash_area.start range
> check?

It's a "if there is a crash area" check.  It seems fine as-is to me.

>> +    {
>> +        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).

Yes, you're right.  This needs to be:

   if ( e > s )
       map_pages_to_xen(...)

David