From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH 2/2] altp2m: Implement p2m_get_mem_access for altp2m views Date: Thu, 28 Jan 2016 10:50:13 +0000 Message-ID: <20160128105013.GK25660@citrix.com> References: <1453925201-15926-1-git-send-email-tlengyel@novetta.com> <1453925201-15926-2-git-send-email-tlengyel@novetta.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1aOk9Z-00055P-LF for xen-devel@lists.xenproject.org; Thu, 28 Jan 2016 10:50:17 +0000 Content-Disposition: inline In-Reply-To: <1453925201-15926-2-git-send-email-tlengyel@novetta.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: Tamas K Lengyel Cc: Wei Liu , Ian Campbell , Razvan Cojocaru , Stefano Stabellini , George Dunlap , Andrew Cooper , Ian Jackson , Stefano Stabellini , Jan Beulich , xen-devel@lists.xenproject.org, Keir Fraser List-Id: xen-devel@lists.xenproject.org On Wed, Jan 27, 2016 at 01:06:41PM -0700, Tamas K Lengyel wrote: > Extend the existing get_mem_access memop to allow querying permissions in > altp2m views as well. > > Signed-off-by: Tamas K Lengyel > Cc: Ian Jackson > Cc: Stefano Stabellini > Cc: Ian Campbell > Cc: Wei Liu > Cc: Razvan Cojocaru > Cc: Stefano Stabellini > Cc: George Dunlap > Cc: Keir Fraser > Cc: Jan Beulich > Cc: Andrew Cooper > --- > tools/libxc/include/xenctrl.h | 3 ++- > tools/libxc/xc_mem_access.c | 8 +++++--- > tools/tests/xen-access/xen-access.c | 5 ++++- The code looks sensible. Some nits and question below. > xen/arch/arm/p2m.c | 4 ++-- > xen/arch/x86/mm/p2m.c | 23 +++++++++++++++++++---- > xen/common/mem_access.c | 2 +- > xen/include/xen/p2m-common.h | 3 ++- > 7 files changed, 35 insertions(+), 13 deletions(-) > > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index b4e57d8..09d9f62 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -2065,7 +2065,8 @@ int xc_set_mem_access(xc_interface *xch, domid_t domain_id, > * Gets the mem access for the given page (returned in access on success) > */ > int xc_get_mem_access(xc_interface *xch, domid_t domain_id, > - uint64_t pfn, xenmem_access_t *access); > + uint64_t pfn, uint16_t altp2m_idx, > + xenmem_access_t *access); > The same questions regarding stability of these functions apply here as well. > /* > * Instructions causing a mem_access violation can be emulated by Xen > diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c > index d6fb409..a44865d 100644 > --- a/tools/libxc/xc_mem_access.c > +++ b/tools/libxc/xc_mem_access.c > @@ -46,14 +46,16 @@ int xc_set_mem_access(xc_interface *xch, > int xc_get_mem_access(xc_interface *xch, > domid_t domain_id, > uint64_t pfn, > + uint16_t altp2m_idx, > xenmem_access_t *access) > { > int rc; > xen_mem_access_op_t mao = > { > - .op = XENMEM_access_op_get_access, > - .domid = domain_id, > - .pfn = pfn > + .op = XENMEM_access_op_get_access, > + .domid = domain_id, > + .pfn = pfn, > + .altp2m_idx = altp2m_idx > }; > > rc = do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao)); > diff --git a/tools/tests/xen-access/xen-access.c b/tools/tests/xen-access/xen-access.c > index 2300e9a..d9dda62 100644 > --- a/tools/tests/xen-access/xen-access.c > +++ b/tools/tests/xen-access/xen-access.c > @@ -571,7 +571,10 @@ int main(int argc, char *argv[]) > > switch (req.reason) { > case VM_EVENT_REASON_MEM_ACCESS: > - rc = xc_get_mem_access(xch, domain_id, req.u.mem_access.gfn, &access); > + rc = xc_get_mem_access(xch, domain_id, req.u.mem_access.gfn, > + ((req.flags & VM_EVENT_FLAG_ALTERNATE_P2M) ? req.altp2m_idx : 0), Ling too long. > + &access > + ); And fold this ")" to previous line? > if (rc < 0) > { > ERROR("Error %d getting mem_access event\n", rc); > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c > index 8e9b4be..932c6e2 100644 > --- a/xen/arch/arm/p2m.c > +++ b/xen/arch/arm/p2m.c > @@ -1666,7 +1666,7 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, const struct npfec npfec) > if ( !p2m->mem_access_enabled ) > return true; > > - rc = p2m_get_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), &xma); > + rc = p2m_get_mem_access(v->domain, _gfn(paddr_to_pfn(gpa)), 0, &xma); > if ( rc ) > return true; > > @@ -1847,7 +1847,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr, > return 0; > } > > -int p2m_get_mem_access(struct domain *d, gfn_t gfn, > +int p2m_get_mem_access(struct domain *d, gfn_t gfn, unsigned long altp2m_idx, > xenmem_access_t *access) > { > int ret; > diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c > index 95bf7ce..18068e8 100644 > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -1572,7 +1572,9 @@ void p2m_mem_access_emulate_check(struct vcpu *v, > bool_t violation = 1; > const struct vm_event_mem_access *data = &rsp->u.mem_access; > > - if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), &access) == 0 ) > + if ( p2m_get_mem_access(v->domain, _gfn(data->gfn), > + altp2m_active(v->domain) ? vcpu_altp2m(v).p2midx : 0, Line too long. Wei.