Joe Epstein
2010-Dec-29 07:27 UTC
[Xen-devel] [PATCH 3 of 5] mem_access: public interface changes
* Introduces a domain create flag to determine whether a memory event access handler is required for the domain when permissions fail. * Memory event request/response modification to accommodate useful fields for access memory events Signed-off-by: Joe Epstein <jepstein98@gmail.com> diff -r 4e108cf56d07 xen/include/public/domctl.h --- a/xen/include/public/domctl.h Mon Dec 27 08:00:09 2010 +0000 +++ b/xen/include/public/domctl.h Tue Dec 28 22:34:01 2010 -0800 @@ -47,17 +47,20 @@ uint32_t ssidref; xen_domain_handle_t handle; /* Is this an HVM guest (as opposed to a PV guest)? */ -#define _XEN_DOMCTL_CDF_hvm_guest 0 -#define XEN_DOMCTL_CDF_hvm_guest (1U<<_XEN_DOMCTL_CDF_hvm_guest) +#define _XEN_DOMCTL_CDF_hvm_guest 0 +#define XEN_DOMCTL_CDF_hvm_guest (1U<<_XEN_DOMCTL_CDF_hvm_guest) /* Use hardware-assisted paging if available? */ -#define _XEN_DOMCTL_CDF_hap 1 -#define XEN_DOMCTL_CDF_hap (1U<<_XEN_DOMCTL_CDF_hap) +#define _XEN_DOMCTL_CDF_hap 1 +#define XEN_DOMCTL_CDF_hap (1U<<_XEN_DOMCTL_CDF_hap) /* Should domain memory integrity be verifed by tboot during Sx? */ -#define _XEN_DOMCTL_CDF_s3_integrity 2 -#define XEN_DOMCTL_CDF_s3_integrity (1U<<_XEN_DOMCTL_CDF_s3_integrity) +#define _XEN_DOMCTL_CDF_s3_integrity 2 +#define XEN_DOMCTL_CDF_s3_integrity (1U<<_XEN_DOMCTL_CDF_s3_integrity) /* Disable out-of-sync shadow page tables? */ -#define _XEN_DOMCTL_CDF_oos_off 3 -#define XEN_DOMCTL_CDF_oos_off (1U<<_XEN_DOMCTL_CDF_oos_off) +#define _XEN_DOMCTL_CDF_oos_off 3 +#define XEN_DOMCTL_CDF_oos_off (1U<<_XEN_DOMCTL_CDF_oos_off) + /* Require mem_event listener for access; else pause */ +#define _XEN_DOMCTL_CDF_access_required 4 +#define XEN_DOMCTL_CDF_access_required (1U<<_XEN_DOMCTL_CDF_access_required) uint32_t flags; }; typedef struct xen_domctl_createdomain xen_domctl_createdomain_t; @@ -714,7 +717,7 @@ /* * Page memory in and out. */ -#define XEN_DOMCTL_MEM_EVENT_OP_PAGING (1 << 0) +#define XEN_DOMCTL_MEM_EVENT_OP_PAGING 1 /* Domain memory paging */ #define XEN_DOMCTL_MEM_EVENT_OP_PAGING_NOMINATE 0 @@ -722,6 +725,12 @@ #define XEN_DOMCTL_MEM_EVENT_OP_PAGING_PREP 2 #define XEN_DOMCTL_MEM_EVENT_OP_PAGING_RESUME 3 +/* + * Access permissions + */ +#define XEN_DOMCTL_MEM_EVENT_OP_ACCESS 2 +#define XEN_DOMCTL_MEM_EVENT_OP_ACCESS_RESUME 0 + struct xen_domctl_mem_event_op { uint32_t op; /* XEN_DOMCTL_MEM_EVENT_OP_* */ uint32_t mode; /* XEN_DOMCTL_MEM_EVENT_ENABLE_* */ diff -r 4e108cf56d07 xen/include/public/hvm/hvm_op.h --- a/xen/include/public/hvm/hvm_op.h Mon Dec 27 08:00:09 2010 +0000 +++ b/xen/include/public/hvm/hvm_op.h Tue Dec 28 22:34:01 2010 -0800 @@ -158,4 +158,45 @@ typedef struct xen_hvm_xentrace xen_hvm_xentrace_t; DEFINE_XEN_GUEST_HANDLE(xen_hvm_xentrace_t); +#define HVMOP_set_mem_access 12 +typedef enum { + HVMMEM_access_n, + HVMMEM_access_r, + HVMMEM_access_w, + HVMMEM_access_rw, + HVMMEM_access_x, + HVMMEM_access_rx, + HVMMEM_access_wx, + HVMMEM_access_rwx, + HVMMEM_access_rx2rw, /* Page starts off as read-execute, but automatically change + * to read-write on a write */ + HVMMEM_access_default /* Take the domain default */ +} hvmmem_access_t; +/* Notify that a region of memory is to have specific access types */ +struct xen_hvm_set_mem_access { + /* Domain to be updated. */ + domid_t domid; + /* Memory type */ + hvmmem_access_t hvmmem_access; + /* First pfn, or ~0ull to set the default access for new pages */ + uint64_t first_pfn; + /* Number of pages, ignored on setting default access */ + uint64_t nr; +}; +typedef struct xen_hvm_set_mem_access xen_hvm_set_mem_access_t; +DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_mem_access_t); + +#define HVMOP_get_mem_access 13 +/* Notify that a region of memory is to have specific access types */ +struct xen_hvm_get_mem_access { + /* Domain to be queried. */ + domid_t domid; + /* Memory type: OUT */ + hvmmem_access_t hvmmem_access; + /* pfn, or ~0ull for default access for new pages. IN */ + uint64_t pfn; +}; +typedef struct xen_hvm_get_mem_access xen_hvm_get_mem_access_t; +DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_mem_access_t); + #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */ diff -r 4e108cf56d07 xen/include/public/mem_event.h --- a/xen/include/public/mem_event.h Mon Dec 27 08:00:09 2010 +0000 +++ b/xen/include/public/mem_event.h Tue Dec 28 22:34:01 2010 -0800 @@ -26,6 +26,11 @@ #include "xen.h" #include "io/ring.h" +/* Memory event type */ +#define MEM_EVENT_TYPE_SHARED 0 +#define MEM_EVENT_TYPE_PAGING 1 +#define MEM_EVENT_TYPE_ACCESS 2 + /* Memory event flags */ #define MEM_EVENT_FLAG_VCPU_PAUSED (1 << 0) @@ -34,10 +39,21 @@ } mem_event_shared_page_t; typedef struct mem_event_st { + uint16_t type; + uint16_t flags; + uint32_t vcpu_id; + uint64_t gfn; + uint64_t offset; + uint64_t gla; /* if gla_valid */ + uint32_t p2mt; - uint32_t vcpu_id; - uint64_t flags; + + uint32_t access_r:1; + uint32_t access_w:1; + uint32_t access_x:1; + uint32_t gla_valid:1; + uint32_t available:28; } mem_event_request_t, mem_event_response_t; DEFINE_RING_TYPES(mem_event, mem_event_request_t, mem_event_response_t); diff -r 4e108cf56d07 xen/common/domctl.c --- a/xen/common/domctl.c Mon Dec 27 08:00:09 2010 +0000 +++ b/xen/common/domctl.c Tue Dec 28 22:35:53 2010 -0800 @@ -398,7 +398,7 @@ if ( supervisor_mode_kernel || (op->u.createdomain.flags & ~(XEN_DOMCTL_CDF_hvm_guest | XEN_DOMCTL_CDF_hap | - XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off)) ) + XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off | XEN_DOMCTL_CDF_access_required)) ) break; dom = op->domain; @@ -434,6 +434,8 @@ domcr_flags |= DOMCRF_s3_integrity; if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_oos_off ) domcr_flags |= DOMCRF_oos_off; + if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_access_required ) + domcr_flags |= DOMCRF_access_required; ret = -ENOMEM; d = domain_create(dom, domcr_flags, op->u.createdomain.ssidref); diff -r 4e108cf56d07 xen/include/xen/sched.h --- a/xen/include/xen/sched.h Mon Dec 27 08:00:09 2010 +0000 +++ b/xen/include/xen/sched.h Tue Dec 28 22:38:35 2010 -0800 @@ -403,22 +403,24 @@ struct domain *domain_create( domid_t domid, unsigned int domcr_flags, ssidref_t ssidref); /* DOMCRF_hvm: Create an HVM domain, as opposed to a PV domain. */ -#define _DOMCRF_hvm 0 -#define DOMCRF_hvm (1U<<_DOMCRF_hvm) +#define _DOMCRF_hvm 0 +#define DOMCRF_hvm (1U<<_DOMCRF_hvm) /* DOMCRF_hap: Create a domain with hardware-assisted paging. */ -#define _DOMCRF_hap 1 -#define DOMCRF_hap (1U<<_DOMCRF_hap) +#define _DOMCRF_hap 1 +#define DOMCRF_hap (1U<<_DOMCRF_hap) /* DOMCRF_s3_integrity: Create a domain with tboot memory integrity protection by tboot */ -#define _DOMCRF_s3_integrity 2 -#define DOMCRF_s3_integrity (1U<<_DOMCRF_s3_integrity) +#define _DOMCRF_s3_integrity 2 +#define DOMCRF_s3_integrity (1U<<_DOMCRF_s3_integrity) /* DOMCRF_dummy: Create a dummy domain (not scheduled; not on domain list) */ -#define _DOMCRF_dummy 3 -#define DOMCRF_dummy (1U<<_DOMCRF_dummy) +#define _DOMCRF_dummy 3 +#define DOMCRF_dummy (1U<<_DOMCRF_dummy) /* DOMCRF_oos_off: dont use out-of-sync optimization for shadow page tables */ -#define _DOMCRF_oos_off 4 -#define DOMCRF_oos_off (1U<<_DOMCRF_oos_off) - +#define _DOMCRF_oos_off 4 +#define DOMCRF_oos_off (1U<<_DOMCRF_oos_off) +/* DOMCRF_access_required: mem_event listener required for access; else pause */ +#define _DOMCRF_access_required 5 +#define DOMCRF_access_required (1U<<_DOMCRF_access_required) /* * rcu_lock_domain_by_id() is more efficient than get_domain_by_id(). * This is the preferred function if the returned domain reference _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jan-04 14:45 UTC
Re: [Xen-devel] [PATCH 3 of 5] mem_access: public interface changes
I didn''t review the whole series I just happened to notice this while reviewing the use of hypercall buffers in libxc: On Wed, 2010-12-29 at 07:27 +0000, Joe Epstein wrote:> +struct xen_hvm_set_mem_access {The definition of this structure needs to come along with the first use of it, which in this case would be the 1/5 patch which adds the hypercall. Similarly for any other interface changes etc. You need the code changes to be with the interface changes, or otherwise to construct your patch series such that you add the new interface, convert everything to it and then remove the old interface. Otherwise everything is broken mid-way through the series, it should compile and work at each step/patch. It also makes review hard since each patch doesn''t really stand alone. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Joe Epstein
2011-Jan-04 15:37 UTC
Re: [Xen-devel] [PATCH 3 of 5] mem_access: public interface changes
Sorry about that. I had considered the whole series as one logical change, and broke it up by sets of directories to reduce the patch size and yet kept the changes local so that there aren''t too many places to look when verifying that the patch went in correctly. I admit that that ordering isn''t the easiest to read through. I''ll refactor the chunks for both this series and the mem_access 2 series into more paired up units and resubmit them today. It will be identical code, for anyone who has applied the ones already submitted, but will be in intermediately compilable patches. The patches will end up being longer for that, so I hope that''s okay. The other alternative is one long patch, which I will attach in the alternative to my 0 of X email to make it easier to apply. Thanks On Tue, Jan 4, 2011 at 6:45 AM, Ian Campbell <Ian.Campbell@citrix.com>wrote:> I didn''t review the whole series I just happened to notice this while > reviewing the use of hypercall buffers in libxc: > > On Wed, 2010-12-29 at 07:27 +0000, Joe Epstein wrote: > > +struct xen_hvm_set_mem_access { > > The definition of this structure needs to come along with the first use > of it, which in this case would be the 1/5 patch which adds the > hypercall. > > Similarly for any other interface changes etc. You need the code changes > to be with the interface changes, or otherwise to construct your patch > series such that you add the new interface, convert everything to it and > then remove the old interface. > > Otherwise everything is broken mid-way through the series, it should > compile and work at each step/patch. > > It also makes review hard since each patch doesn''t really stand alone. > > Ian. > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel