From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tim Deegan <tim@xen.org>
Subject: Re: [RFC][PATCH 05/13] xen/x86/p2m: introduce
	set_identity_p2m_entry
Date: Thu, 16 Apr 2015 16:05:14 +0100
Message-ID: <20150416150514.GG13441@deinos.phlegethon.org>
References: <1428657724-3498-1-git-send-email-tiejun.chen@intel.com>
	<1428657724-3498-6-git-send-email-tiejun.chen@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <1428657724-3498-6-git-send-email-tiejun.chen@intel.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: Tiejun Chen <tiejun.chen@intel.com>
Cc: kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, andrew.cooper3@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, JBeulich@suse.com, yang.z.zhang@intel.com
List-Id: xen-devel@lists.xenproject.org

Hi,

At 17:21 +0800 on 10 Apr (1428686516), Tiejun Chen wrote:
> @@ -862,6 +862,36 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
>      return set_typed_p2m_entry(d, gfn, mfn, p2m_mmio_direct, access);
>  }
>  
> +int set_identity_p2m_entry(struct domain *d, unsigned long gfn,
> +                           p2m_access_t p2ma)
> +{
> +    p2m_type_t p2mt;
> +    p2m_access_t a;
> +    mfn_t mfn;
> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +    int ret = -EBUSY;
> +
> +    gfn_lock(p2m, gfn, 0);
> +
> +    mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL);
> +
> +    if ( !mfn_valid(mfn) )

I don't think this check is quite right -- for example, this p2m entry
might be an MMIO mapping or a PoD entry.  "if ( p2mt == p2m_invalid )"
would be better.

> +        ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K, p2m_mmio_direct,

This line seems to be > 80 chars -- can you wrap it a bit earlier, please?

Cheers,

Tim.