[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [v3][PATCH 02/16] xen/x86/p2m: introduce set_identity_p2m_entry
On 2015/6/11 17:23, Jan Beulich wrote: On 11.06.15 at 10:23, <tiejun.chen@xxxxxxxxx> wrote:On 2015/6/11 15:33, Jan Beulich wrote:On 11.06.15 at 03:15, <tiejun.chen@xxxxxxxxx> wrote:We will create this sort of identity mapping as follows: If the gfn space is unoccupied, we just set the mapping. If the space is already occupied by 1:1 mappings, do nothing. Failed for any other cases. Signed-off-by: Tiejun Chen <tiejun.chen@xxxxxxxxx>First of all you continue to be copying each patch to every maintainer involved in some part of the series. Please limit theI just hope all involved guys can see this series on the whole to review. But,Cc list of each patch to the actual list of people needing to be Cc-ed on it (or you know explicitly wanting a copy).Next, I will just send them to each associated maintainer.--- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -898,6 +898,41 @@ int set_mmio_p2m_entry(struct domain *d, unsigned longgfn, 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; + + if ( paging_mode_translate(p2m->domain) ) + { + gfn_lock(p2m, gfn, 0); + + mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, 0, NULL); + + if ( p2mt == p2m_invalid || mfn_x(mfn) == INVALID_MFN )I'm not fundamentally opposed to this extra INVALID_MFN check, but could you please clarify for which P2M type you saw INVALID_MFN coming back here, and for which p2m_invalid cases you didn't also see INVALID_MFN? I.e. I'd really prefer a single check to be used when that can cover all cases.Actually, I initially adopted "!mfn_valid(mfn)" in our previous version. But Tim issued one comment about this, "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."Ah, I right, I now remember. In which case checking against INVALID_MFN would cover the MMIO case, but not the PoD one.But if I just keep his recommended check, you can see the following when I pass through IGD, (XEN) Cannot identity map d1:ad800, already mapped to ffffffffffffffff with p2mt:4. Looks "4" indicates p2m_mmio_dm, right?And it seems to me that this particular combination would need special treatment, i.e. you'd need if ( p2mt == p2m_invalid || (p2mt == p2m_mmio_dm && mfn_x(mfn) == INVALID_MFN) ) as long as p2m_invalid isn't the default type lookups return. But I'd recommend waiting for Tim to confirm or further adjust that. Sure. Thanks Tiejun _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |