[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC 1/2] x86/mem_access: Make the mem_access ops generic
>> -int mem_access_memop(struct domain *d, xen_mem_event_op_t *meo) >> +int mem_access_memop(XEN_GUEST_HANDLE_PARAM(void) arg) >> { >> int rc; >> + xen_mem_access_op_t mao; >> + struct domain *d; >> + >> + if ( copy_from_guest(&mao, arg, 1) ) >> + return -EFAULT; >> + >> + rc = rcu_lock_live_remote_domain_by_id(mao.domid, &d); >> + if ( rc ) >> + return rc; >> + >> + if ( !is_hvm_domain(d) ) >> + return -EINVAL; >> + >> + rc = xsm_mem_event_op(XSM_TARGET, d, XENMEM_access_op); >> + if ( rc ) >> + goto out; >> >> if ( unlikely(!d->mem_event->access.ring_page) ) >> return -ENODEV; >> >> - switch( meo->op ) >> + switch ( mao.op ) >> { >> case XENMEM_access_op_resume: >> { >> p2m_mem_access_resume(d); >> rc = 0; >> + break; >> + } >> + >> + case XENMEM_access_op_set_access: >> + { > >Please no pointless braces. Sorry, I will get rid of them. >> + rc = -EINVAL; >> + >> + if ( (mao.pfn != ~0ull) && >> + (((mao.pfn + mao.nr - 1) < mao.pfn) || >> + ((mao.pfn + mao.nr - 1) > domain_get_maximum_gpfn(d))) ) >> + break; >> + >> + rc = p2m_set_mem_access(d, mao.pfn, mao.nr, >mao.xenmem_access); >> + if ( rc > 0 ) >> + { >> + mao.pfn += mao.nr - rc; >> + mao.nr = rc; >> + if ( __copy_to_guest(arg, &mao, 1) ) >> + rc = -EFAULT; >> + else >> + rc = hypercall_create_continuation(__HYPERVISOR_memory_op, >"lh", >> + XENMEM_access_op, >> + arg); > >As already said - this should be done without playing with the interface >structure. All you need for this is to pass down the upper "cmd" bits from >do_memory_op(). OK, I will give that a shot. >> @@ -162,36 +163,31 @@ >DEFINE_XEN_GUEST_HANDLE(xen_hvm_xentrace_t); >> /* Following tools-only interfaces may change in future. */ #if >> defined(__XEN__) || defined(__XEN_TOOLS__) >> >> +/* Deprecated by XENMEM_access_op_set_access */ >> #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 r-x, but automatically >> - * change to r-w on a write */ >> - HVMMEM_access_n2rwx, /* Log access: starts off as n, automatically >> >> - * goes to rwx, generating an event without >> - * pausing the vcpu */ >> - HVMMEM_access_default /* Take the domain default */ >> + HVMMEM_access_n = XENMEM_access_n, >> + HVMMEM_access_r = XENMEM_access_r, >> + HVMMEM_access_w = XENMEM_access_w, >> + HVMMEM_access_rw = XENMEM_access_rw, >> + HVMMEM_access_x = XENMEM_access_x, >> + HVMMEM_access_rx = XENMEM_access_rx, >> + HVMMEM_access_wx = XENMEM_access_wx, >> + HVMMEM_access_rwx = XENMEM_access_rwx, >> + /* >> + * Page starts off as r-x, but automatically >> + * change to r-w on a write >> + */ >> + HVMMEM_access_rx2rw = XENMEM_access_rx2rw, >> + /* >> + * Log access: starts off as n, automatically >> + * goes to rwx, generating an event without >> + * pausing the vcpu >> + */ >> + HVMMEM_access_n2rwx = XENMEM_access_n2rwx, >> + /* Take the domain default */ >> + HVMMEM_access_default = XENMEM_access_default >> } hvmmem_access_t; > >So what is the reason for you to keep this while deleting the interface >structures? I was keeping to maintain backward compatibility as I thought I need to retain xc_hvm_[sg]et_mem_access(). But after IanC's asking me to nuke them, there is no reason for the struct and enum to hang around. >> @@ -379,6 +376,56 @@ struct xen_mem_event_op { typedef struct >> xen_mem_event_op xen_mem_event_op_t; >> DEFINE_XEN_GUEST_HANDLE(xen_mem_event_op_t); >> >> +#define XENMEM_access_op 21 >> +#define XENMEM_access_op_resume 0 >> +#define XENMEM_access_op_set_access 1 >> +#define XENMEM_access_op_get_access 2 >> + >> +typedef enum { >> + XENMEM_access_n, >> + XENMEM_access_r, >> + XENMEM_access_w, >> + XENMEM_access_rw, >> + XENMEM_access_x, >> + XENMEM_access_rx, >> + XENMEM_access_wx, >> + XENMEM_access_rwx, >> + /* >> + * Page starts off as r-x, but automatically >> + * change to r-w on a write >> + */ >> + XENMEM_access_rx2rw, >> + /* >> + * Log access: starts off as n, automatically >> + * goes to rwx, generating an event without >> + * pausing the vcpu >> + */ >> + XENMEM_access_n2rwx, >> + /* Take the domain default */ >> + XENMEM_access_default >> +} xenmem_access_t; >> + >> +struct xen_mem_access_op { >> + /* XENMEM_access_op_* */ >> + uint8_t op; >> + domid_t domid; >> + /* xenmem_access_t */ >> + uint16_t xenmem_access; > >If you limited this to 8 bits and put it above domid, you'd get away without >any >implicit padding (which I would otherwise ask you to make explicit). OK I will make it a uint8_t and move it above domid. >Also I don't see the need for the xenmem_ prefix. I will get rid of the prefix. What about the enum? Should I call it mem_access_t and MEM_access_*? Thanks, Aravindh _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |