From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mukesh Rathor <mukesh.rathor@oracle.com>
Subject: Re: [RFC 0 PATCH 3/3] PVH dom0: construct_dom0 changes
Date: Thu, 26 Sep 2013 17:17:37 -0700
Message-ID: <20130926171737.071f118f@mantra.us.oracle.com>
References: <1380142988-9487-1-git-send-email-mukesh.rathor@oracle.com>
	<1380142988-9487-4-git-send-email-mukesh.rathor@oracle.com>
	<5244064102000078000F69AF@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>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <mukesh.rathor@oracle.com>) id 1VPLke-0008Ej-BJ
	for xen-devel@lists.xenproject.org; Fri, 27 Sep 2013 00:17:44 +0000
In-Reply-To: <5244064102000078000F69AF@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: xen-devel <xen-devel@lists.xenproject.org>, keir.xen@gmail.com
List-Id: xen-devel@lists.xenproject.org

On Thu, 26 Sep 2013 09:02:41 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 25.09.13 at 23:03, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > @@ -307,6 +308,136 @@ static void __init
> > process_dom0_ioports_disable(void) }
> >  }
> >  
> > +/*
> > + * Set the 1:1 map for all non-RAM regions for dom 0. Thus, dom0
> > will have
> > + * the entire io region mapped in the EPT/NPT.
> > + *
> > + * PVH FIXME: The following doesn't map MMIO ranges when they sit
> > above the
> > + *            highest E820 covered address.
> 
> This absolutely needs fixing before this can go in.

Any suggestions on how to fix it? Mapping all the way to end could
result in a huge hap table. 


> > +    const struct e820entry *entry;
> > +    unsigned int i, nump;
> > +    int rc;
> > +
> > +    for ( i = 0, entry = e820.map; i < e820.nr_map; i++, entry++ )
> > +    {
> > +        end = entry->addr + entry->size;
> > +
> > +        if ( entry->type == E820_RAM || entry->type ==
> > E820_UNUSABLE ||
> 
> The inclusion of E820_UNUSABLE here clearly needs an explanation
> (ideally in the shape of a code comment).
> 
> > +             i == e820.nr_map - 1 )
> > +        {
> > +            start_pfn = PFN_DOWN(start);
> > +            end_pfn = PFN_UP(end);
> > +
> > +            if ( entry->type == E820_RAM || entry->type ==
> > E820_UNUSABLE )
> > +                end_pfn = PFN_UP(entry->addr);
> 
> Better coded as if/else construct (making more obvious what the
> intention here is - it took me quite some time to see that what
> you're doing here is correct, because the general case really is
> what sits in the if() body, and the exception is what gets done
> before the if(), which is counter intuitive).

Based on xen_set_identity_and_release linux function. Took me a while
to figure that one too. So, I tried simplifying it, but in the end
came back to this. Not sure where I would the else.

thanks
Mukesh