[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v3][PATCH 03/16] xen/vtd: create RMRR mapping
On 2015/6/12 10:43, Chen, Tiejun wrote: On 2015/6/11 22:07, Tim Deegan wrote:At 17:31 +0800 on 11 Jun (1434043916), Chen, Tiejun wrote:while ( base_pfn < end_pfn ) { - int err = intel_iommu_map_page(d, base_pfn, base_pfn, - IOMMUF_readable|IOMMUF_writable); + int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw); if ( err ) return err;Tim has another comment to replace earlier unmap withYes, I knew this.guest_physmap_remove_page() which will call iommu unmap internally. Please include this change too.But, guest_physmap_remove_page() | + p2m_remove_page() | + iommu_unmap_page() | + p2m_set_entry(p2m, gfn, _mfn(INVALID_MFN), xxx) I think this already remove these pages both on ept/vt-d sides, right?Yes; this is about this code further up in the same function: while ( base_pfn < end_pfn ) { if ( intel_iommu_unmap_page(d, base_pfn) ) ret = -ENXIO; base_pfn++; } which ought to be calling guest_physmap_remove_page() or similar, to make sure that both iommu and EPT mappings get removed.I still just think current implementation might be fine at this point. We have two scenarios here, the case of shared ept and the case of non-shared ept. But no matter what case we're tracking, shouldn't guest_physmap_remove_page() always call p2m->set_entry() to clear *all* *valid* mfn which is owned by a given VM? And p2m->set_entry() also calls iommu_unmap_page() internally. So nothing special should further consider. If I'm wrong or misunderstanding, please correct me :) Sorry for my misunderstanding to this. Right now Kevin help me understand what you mean. Sounds like we should address something specific to unmap rmrr here. So I will do this as follows: #1. Provide a clear helper +int clear_identity_p2m_entry(struct domain *d, unsigned long gfn, + unsigned int page_order) +{ + struct p2m_domain *p2m = p2m_get_hostp2m(d); + int ret; + gfn_lock(p2m, gfn, page_order); + ret = p2m_remove_page(p2m, gfn, gfn, page_order); + gfn_unlock(p2m, gfn, page_order); + return ret; +} + #2. Call such a helper@@ -1840,7 +1840,7 @@ static int rmrr_identity_mapping(struct domain *d, bool_t map, while ( base_pfn < end_pfn ) { - if ( intel_iommu_unmap_page(d, base_pfn) ) + if ( clear_identity_p2m_entry(d, base_pfn, 0) ) ret = -ENXIO; base_pfn++; } Is this right? Thanks Tiejun _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |