From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v5][PATCH 1/2] xen:x86:mm:p2m: introduce set_identity_p2m_entry Date: Tue, 29 Jul 2014 19:08:33 +0800 Message-ID: <53D780B1.70002@intel.com> References: <1406616052-11973-1-git-send-email-tiejun.chen@intel.com> <53D763C5020000780002719D@mail.emea.novell.com> <53D74EDC.4010100@intel.com> <53D7754302000078000272AB@mail.emea.novell.com> <53D76555.5000601@intel.com> <53D78B1D0200007800027393@mail.emea.novell.com> <53D774E3.1080008@intel.com> <53D7933B02000078000273C1@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53D7933B02000078000273C1@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: yang.z.zhang@intel.com, kevin.tian@intel.com, tim@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 2014/7/29 18:27, Jan Beulich wrote: >>>> On 29.07.14 at 12:18, wrote: >> On 2014/7/29 17:53, Jan Beulich wrote: >>>>>> On 29.07.14 at 11:11, wrote: >>>> On 2014/7/29 16:19, Jan Beulich wrote: >>>>> ? Of course it may still be necessary to also inspect the obtained p2mt >>>>> and a. >>>>> >>>> >>>> Are you saying this? >>>> >>>> if ( !p2m_is_valid(p2mt) || >>>> !mfn_valid(mfn) || >>>> (a != p2m_access_rw) ) >>> >>> I'm afraid that's not enough context to know whether what you >>> mean to do is sufficient. Plus !p2m_is_valid() is too weak. You >>> simply need to properly think through what should happen if you >>> find a valid mapping, but any of the tuple (mfn, p2mt, a) don't >>> match what you intend to be there. >>> >> >> Actually as I understand we can create these mapping only in one case of >> !mfn_valid(mfn). For others scenarios we just return with that warning >> message no matter what that tuple is explicitly. So here I try to >> understand why you're saying we need check more by show this condition >> combination. > > Perhaps, but with the exception that at least if the entire tuple > matches you should return success (and not print anything). > There might be further cases where an existing mapping would > be good enough (like a being p2m_access_rwx), but perhaps > there's not much point in trying to deal with them without explicit > need. > I think the following cases should be enough: #1: !mfn_valid(mfn) We can create those mapping safely. #2: mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2m_access_rw We already have these matched mappings. #3: Others Return with that waring message: "Cannot identity map d%d:%lx, already mapped to %lx but mismatch.\n" So what about this? @@ -858,6 +858,35 @@ 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); } +int set_identity_p2m_entry(struct domain *d, unsigned long gfn) +{ + 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) ) + ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K, p2m_mmio_direct, + p2m_access_rw); + else if ( mfn_x(mfn) == gfn && + p2mt == p2m_mmio_direct && + a == p2m_access_rw ) + ret = 0; + else + printk(XENLOG_G_WARNING + "Cannot identity map d%d:%lx, already mapped to %lx but mismatch.\n", + d->domain_id, gfn, mfn_x(mfn)); + + gfn_unlock(p2m, gfn, 0); + + return ret; +} + /* Returns: 0 for success, -errno for failure */ int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn) { Thanks Tiejun