[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 1/2] altp2m: Merge p2m_set_altp2m_mem_access and p2m_set_mem_access
>>> On 28.01.16 at 21:58, <tlengyel@xxxxxxxxxxx> wrote: > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -1777,14 +1777,57 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned > long gla, > return (p2ma == p2m_access_n2rwx); > } > > +static int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain > *hp2m, > + struct p2m_domain *ap2m, p2m_access_t a, > + unsigned long gfn) I think new functions would better not use "unsigned long" for frame numbers. > +{ > + mfn_t mfn; > + p2m_type_t t; > + p2m_access_t old_a; > + unsigned int page_order; > + int rc; > + > + mfn = ap2m->get_entry(ap2m, gfn, &t, &old_a, 0, NULL, NULL); > + > + /* Check host p2m if no valid entry in alternate */ > + if ( !mfn_valid(mfn) ) > + { > + mfn = hp2m->get_entry(hp2m, gfn, &t, &old_a, > + P2M_ALLOC | P2M_UNSHARE, &page_order, NULL); > + > + rc = -ESRCH; > + if ( !mfn_valid(mfn) || t != p2m_ram_rw ) > + goto out; > + > + /* If this is a superpage, copy that first */ > + if ( page_order != PAGE_ORDER_4K ) > + { > + unsigned long mask = ~((1UL << page_order) - 1); > + gfn_t gfn2 = _gfn(gfn & mask); > + mfn_t mfn2 = _mfn(mfn_x(mfn) & mask); > + > + rc = ap2m->set_entry(ap2m, gfn_x(gfn2), mfn2, page_order, t, > old_a, 1); > + if ( rc ) > + goto out; > + } > + } > + > + rc = ap2m->set_entry(ap2m, gfn, mfn, PAGE_ORDER_4K, t, a, > + (current->domain != d)); > + > + out: > + return rc; > +} With there not being any involved error handling here, I don't think using a label and goto is warranted here. But I'll leave the ultimate decision to George, of course. > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -423,11 +423,14 @@ struct xen_mem_access_op { > /* xenmem_access_t */ > uint8_t access; > domid_t domid; > + uint16_t altp2m_idx; > + uint16_t _pad; > /* > * Number of pages for set op > * Ignored on setting default access and other ops > */ > uint32_t nr; > + uint32_t _pad2; Repeating what I had said on v1: So this is a tools only interface, yes. But it's not versioned (other than e.g. domctl and sysctl), so altering the interface structure is at least fragile. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |