Memory event interface improvements
PATCH 1/1 Memory event interface improvements
In this patch series we improve the management of congestion in the memory events ring. We ensure no guest events are lost, even in the face of unbounded flooding from foreign maps, or balloon.
Also, we enable resumption of mem events via an event channel kick from user-space to Xen. This is more light-weight and scalable than the current domctl interface, and allows for batching as well.
xen/common/memory.c | 29 +++++- xen/arch/x86/hvm/hvm.c | 20 ++- xen/arch/x86/mm/mem_event.c | 205 ++++++++++++++++++++++++++++++--------- xen/arch/x86/mm/mem_sharing.c | 27 +++- xen/arch/x86/mm/p2m.c | 104 ++++++++++--------- xen/common/memory.c | 7 +- xen/include/asm-x86/mem_event.h | 16 ++- xen/include/asm-x86/p2m.h | 6 +- xen/include/xen/mm.h | 2 + xen/include/xen/sched.h | 5 +- xen/arch/ia64/vmx/vmx_init.c | 2 +- xen/arch/x86/hvm/hvm.c | 7 +- xen/arch/x86/mm/mem_event.c | 3 +- xen/common/event_channel.c | 75 +++++++++++--- xen/include/xen/event.h | 5 +- xen/include/xen/sched.h | 2 +- xen/arch/x86/mm/mem_access.c | 3 +- xen/arch/x86/mm/p2m.c | 3 +- xen/include/asm-x86/p2m.h | 2 +- xen/arch/x86/mm/mem_event.c | 26 +++- 20 files changed, 389 insertions(+), 160 deletions(-)
PATCH 1/1 Allow decrease_reservation to be preempted if remove_page returns negative
xen/common/memory.c | 29 ++++++++++++++++++++++++++++- 1 files changed, 28 insertions(+), 1 deletions(-)
This will allow for handling of full paging rings in a more graceful manner. ATM, remove_page does not return negative values, so this code is not yet exercised.
xen/common/memory.c +28 -1
@@ -226,6 +226,8 @@ static void decrease_reservation(structfor ( i = a->nr_done; i < a->nr_extents; i++ ) { +int one_remove_succeeded =0 ; +if (hypercall_preempt_check () ) { a->preempted =1 ;
@@ -255,8 +257,33 @@ static void decrease_reservation(structcontinue ;for ( j =0 ; j < (1 << a->extent_order); j++ ) -if ( !guest_remove_page (a->domain, gmfn + j) ) + { +int rv =guest_remove_page (a->domain, gmfn + j); +/* negative rv is a result of a mem event ring full +* in the presence of paging. We preempt the hypercall */ +if ( rv <0 ) + { +/* Indicate we're done with this extent */ +if ((j+1 ) == (1 << a->extent_order)) + i++; + a->preempted =1 ; +raise_softirq (SCHEDULE_SOFTIRQ ); goto out; + } +/* rv of zero means we tried to remove a gfn with no backing +* mfn. It can be a bad argument, or, a continuation in the midst +* of an extent. Heuristically determine second case. */ +if ( !rv ) + { +/* Has to be first extent */ +if ( i != a->nr_done ) + goto out; +/* No previous remove in this extent must have succeeded */ +if ( one_remove_succeeded ) + goto out; + }else + one_remove_succeeded =1 ; + } } out:
PATCH 1/1 Improve ring management for memory events. Do not lose guest events
In reply to Andres Lagar-Cavillaxen/arch/x86/hvm/hvm.c | 20 ++- xen/arch/x86/mm/mem_event.c | 205 ++++++++++++++++++++++++++++++--------- xen/arch/x86/mm/mem_sharing.c | 27 +++- xen/arch/x86/mm/p2m.c | 104 ++++++++++--------- xen/common/memory.c | 7 +- xen/include/asm-x86/mem_event.h | 16 ++- xen/include/asm-x86/p2m.h | 6 +- xen/include/xen/mm.h | 2 + xen/include/xen/sched.h | 5 +- 9 files changed, 268 insertions(+), 124 deletions(-)
The memevent code currently has a mechanism for reserving space in the ring before putting an event, but each caller must individually ensure that the vCPUs are correctly paused if no space is available.
This fixes that issue by reversing the semantics: we ensure that enough space is always left for one event per vCPU in the ring. If, after putting the current request, this constraint will be violated by the current vCPU when putting putting another request in the ring, we pause the vCPU.
For mem events caused by foreign vcpus, we simply drop the event if the space constraint will be violated. Foreign vcpus are expected to retry mapping operations (and thus the associated paging populate mem events will be re issued).
Finally, guests can cause an unbound number of paging drop events via the balloon -> decrease_reservation hypercall. Thus, notify the hypercall there is no more space in the ring so a continuation is scheduled.
With all these mechanisms, no guest events are lost and there is no need for wait-queues. Our testing includes 1. ballooning down 512 MiBs; 2. a mem event mode in which every page access in a four-vCPU guest results in an event, with no vCPU pausing, and the four vCPUs touching all RAM. No guest events were lost in either case, and qemu-dm had no mapping problems.
Additionally, we ensure that no events are lost by Xen as a consumer if multiple responses land on the ring for a single domctl. While the current domctl-based notifications to Xen disallow batching, this is required for later patches.
Finally, mark_and_pause_vcpus was misleading, beacause it wasn't strictly pausing the vcpu's, rather sending them to sleep. Change to actual vcpu_pause, which protects wakeups via an atomic count. This is useful when an event pauses a vcpu and the vcpu gets mark and paused due to ring congestion.
xen/arch/x86/hvm/hvm.c +14 -6
@@ -4108,7 +4108,6 @@ static int hvm_memory_event_traps(long pstruct vcpu * v = current;struct domain *d = v->domain;mem_event_request_t req; -int rc;if ( !(p &HVMPME_MODE_MASK ) )return 0 ;
@@ -4116,10 +4115,6 @@ static int hvm_memory_event_traps(long pif ( (p & HVMPME_onchangeonly) && (value == old) )return 1 ; - rc =mem_event_check_ring (d, &d->mem_access); -if ( rc ) -return rc; -memset (&req,0 ,sizeof (req)); req.type =MEM_EVENT_TYPE_ACCESS ; req.reason = reason;
@@ -4139,7 +4134,20 @@ static int hvm_memory_event_traps(long p req.gla_valid =1 ; } -mem_event_put_request (d, &d->mem_access, &req); +if (mem_event_put_request (d, &d->mem_access, &req) == -ENOSYS ) + { +/* rc == -ENOSYS +* If there was no ring to send the event, then simply unpause the +* vcpu (if we had previously paused it). It will retry the +* instruction (with the exception "handled") and go on */ +if ( req.flags &MEM_EVENT_FLAG_VCPU_PAUSED ) +vcpu_unpause (v); +/* rc == -EBUSY +* If the ring is full, the vcpu has been marked and paused, +* and the exception has been communicated to the consumer. +* Once there is room in the ring, the vcpu will be woken up +* and will retry. */ + }return 1 ; }
xen/arch/x86/mm/mem_event.c +154 -51
280 lines not on this page · raw message
xen/arch/x86/mm/mem_sharing.c +19 -8
@@ -281,12 +281,19 @@ static struct page_info* mem_sharing_allvcpu_pause_nosync (v); req.flags |=MEM_EVENT_FLAG_VCPU_PAUSED ; -if (mem_event_check_ring (d, &d->mem_share))return page; - req.gfn = gfn; req.p2mt = p2m_ram_shared; req.vcpu_id = v->vcpu_id; -mem_event_put_request (d, &d->mem_share, &req); + +/* If there is no ring, and we're out of memory, then +* there is no way out. */ +if (mem_event_put_request (d, &d->mem_share, &req) == -ENOSYS ) + { +gdprintk (XENLOG_ERR , +"Failed alloc on unshare path for %hu and no ring " +"to upcall\n" , d->domain_id); +domain_crash (d); + }return page; }
@@ -300,12 +307,16 @@ int mem_sharing_sharing_resume(struct do {mem_event_response_t rsp; -/* Get request off the ring */ -mem_event_get_response (&d->mem_share, &rsp); +/* Get all requests off the ring */ +while (mem_event_get_response (d, &d->mem_share, &rsp) ) + { +/* Unpause domain/vcpu */ +if ( rsp.flags &MEM_EVENT_FLAG_VCPU_PAUSED ) +vcpu_unpause (d->vcpu[rsp.vcpu_id]); + } -/* Unpause domain/vcpu */ -if ( rsp.flags &MEM_EVENT_FLAG_VCPU_PAUSED ) -vcpu_unpause (d->vcpu[rsp.vcpu_id]); +/* Unpause any domains that were paused because the ring was full */ +mem_event_unpause_vcpus (d, &d->mem_paging);return 0 ; }
xen/arch/x86/mm/p2m.c +56 -48
189 lines not on this page · raw message
xen/common/memory.c +5 -2
@@ -165,10 +165,13 @@ int guest_remove_page(struct domain *d, mfn =mfn_x (get_gfn (d, gmfn, &p2mt));if (unlikely (p2m_is_paging (p2mt)) ) { +int rc;guest_physmap_remove_page (d, gmfn, mfn,0 ); -p2m_mem_paging_drop_page (d, gmfn); + rc =p2m_mem_paging_drop_page (d, gmfn);put_gfn (d, gmfn); -return 1 ; +/* drop_page returns zero on success, we return 1 on success. +* drop_page returns negative on error, never returns 1. */ +return rc ? rc :1 ; }#else mfn =gmfn_to_mfn (d, gmfn);
xen/include/asm-x86/mem_event.h +11 -5
@@ -25,12 +25,18 @@#define __MEM_EVENT_H__/* Pauses VCPU while marking pause flag for mem event */ -void mem_event_mark_and_pause (struct vcpu *v); +void mem_event_mark_and_pause (struct vcpu *v,struct mem_event_domain *med);int mem_event_check_ring (struct domain *d,struct mem_event_domain *med); -void mem_event_put_req_producers (struct mem_event_domain *med); -void mem_event_put_request (struct domain *d,struct mem_event_domain *med,mem_event_request_t *req); -void mem_event_get_response (struct mem_event_domain *med,mem_event_response_t *rsp); -void mem_event_unpause_vcpus (struct domain *d); +void mem_event_unpause_vcpus (struct domain *d,struct mem_event_domain *med); + +/* Returns 0 on success, -ENOSYS if there is no ring, -EBUSY if there is +* a ring or no space. FOr success or -EBUSY. the vCPU is left blocked to +* ensure that the ring does not lose events. In general, put_request should +* not fail, as it attempts to ensure that each vCPU can put at least one req. */ +int mem_event_put_request (struct domain *d,struct mem_event_domain *med, +mem_event_request_t *req); +int mem_event_get_response (struct domain *d,struct mem_event_domain *med, +mem_event_response_t *rsp);int mem_event_domctl (struct domain *d,xen_domctl_mem_event_op_t *mec,XEN_GUEST_HANDLE (void ) u_domctl);
xen/include/asm-x86/p2m.h +3 -3
@@ -475,7 +475,7 @@ int p2m_mem_paging_nominate(struct domai/* Evict a frame */ int p2m_mem_paging_evict (struct domain *d,unsigned long gfn);/* Tell xenpaging to drop a paged out frame */ -void p2m_mem_paging_drop_page (struct domain *d,unsigned long gfn); +int p2m_mem_paging_drop_page (struct domain *d,unsigned long gfn);/* Start populating a paged out frame */ void p2m_mem_paging_populate (struct domain *d,unsigned long gfn);/* Prepare the p2m for paging a frame in */
@@ -483,8 +483,8 @@ int p2m_mem_paging_prep(struct domain *d/* Resume normal operation (in case a domain was paused) */ void p2m_mem_paging_resume (struct domain *d);#else -static inline void p2m_mem_paging_drop_page (struct domain *d,unsigned long gfn) -{ } +static inline int p2m_mem_paging_drop_page (struct domain *d,unsigned long gfn) +{return 0 ; }static inline void p2m_mem_paging_populate (struct domain *d,unsigned long gfn) { }#endif
xen/include/xen/mm.h +2 -0
@@ -318,6 +318,8 @@ page_list_splice(struct page_list_head *void scrub_one_page (struct page_info *); +/* Returns 1 on success, 0 on error, negative if the ring +* for event propagation is full in the presence of paging */ int guest_remove_page (struct domain *d,unsigned long gmfn);#define RAM_TYPE_CONVENTIONAL 0x00000001
xen/include/xen/sched.h +4 -1
@@ -183,13 +183,16 @@ struct mem_event_domain {/* ring lock */ spinlock_t ring_lock; -unsigned int req_producers;/* shared page */ mem_event_shared_page_t *shared_page;/* shared ring page */ void *ring_page;/* front-end ring */ mem_event_front_ring_t front_ring; +/* the number of vCPUs blocked */ +unsigned int blocked; +/* The last vcpu woken up */ +unsigned int last_vcpu_wake_up;/* event channel port (vcpu0 only) */ int xen_port; };
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
Quoted 12 lines · Andres Lagar-Cavilla
xen/arch/x86/hvm/hvm.c | 20 ++- xen/arch/x86/mm/mem_event.c | 205 ++++++++++++++++++++++++++++++--------- xen/arch/x86/mm/mem_sharing.c | 27 +++- xen/arch/x86/mm/p2m.c | 104 ++++++++++--------- xen/common/memory.c | 7 +- xen/include/asm-x86/mem_event.h | 16 ++- xen/include/asm-x86/p2m.h | 6 +- xen/include/xen/mm.h | 2 + xen/include/xen/sched.h | 5 +- 9 files changed, 268 insertions(+), 124 deletions(-)
Quoted 3 lines
The memevent code currently has a mechanism for reserving space in the ring before putting an event, but each caller must individually ensure that the vCPUs are correctly paused if no space is available.
I have an improved patch which uses wait queues in mem_event_put_request() and also the new wake_up_nr(). Using pause here and wait queues in get_gfn does not mix well AFAICS. My wait queue patch for get_gfn is not yet finished.
I propose to use wait queues for both mem_event and get_gfn.
Olaf
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
Quoted 21 lines · Andres Lagar-Cavilla
Quoted 14 lines
xen/arch/x86/hvm/hvm.c | 20 ++- xen/arch/x86/mm/mem_event.c | 205 ++++++++++++++++++++++++++++++--------- xen/arch/x86/mm/mem_sharing.c | 27 +++- xen/arch/x86/mm/p2m.c | 104 ++++++++++--------- xen/common/memory.c | 7 +- xen/include/asm-x86/mem_event.h | 16 ++- xen/include/asm-x86/p2m.h | 6 +- xen/include/xen/mm.h | 2 + xen/include/xen/sched.h | 5 +- 9 files changed, 268 insertions(+), 124 deletions(-)Quoted 5 lines
The memevent code currently has a mechanism for reserving space in the ring
before putting an event, but each caller must individually ensure that the
vCPUs are correctly paused if no space is available.
Quoted 6 lines
I have an improved patch which uses wait queues in
mem_event_put_request()and also the newwake_up_nr(). Using pause here and wait queues inget_gfndoes not mix well AFAICS. My wait queue patch forget_gfnis not yet finished.I propose to use wait queues for both
mem_eventandget_gfn.
Well, given the patch I submitted, my position ought to be clear :)
I have four cents to add:
- Our patch works. We're blasting the ring with multi-vcpu events. Nothing is ever lost, no vcpu is left blocked and forgotten.
- This isn't a problem that necessitates wait-queues for solving. Just careful logic.
- I am not sure what your concerns about the mix are. get_gfn* would call populate on a paged out gfn, and then go to sleep if it's a guest vcpu. With our patch, the guest vcpu event is guaranteed to go in the ring. vcpu pausing will stack (and unwind) properly.
- This patch does not preclude wait queues where they are absolutely needed. I think once wait queues are in, they'll be a welcome breakthrough for hypervisor code that just can't handle paged out pages gracefully. I look forward to that.
Andres
Quoted 3 lines
Olaf
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
Quoted 7 lines · Andres Lagar-Cavilla
- This isn't a problem that necessitates wait-queues for solving. Just careful logic.
- I am not sure what your concerns about the mix are. get_gfn* would call populate on a paged out gfn, and then go to sleep if it's a guest vcpu. With our patch, the guest vcpu event is guaranteed to go in the ring. vcpu pausing will stack (and unwind) properly.
Today I sent my current version of wait queues for mem_event and paging. Unfortunately the earlier versions of mem_event had bugs which caused trouble also for the paging change.
Please have a look wether my changes work for you.
I see you have a change to mem_event_get_response() which pulls all requests instead of just one. Thats currently a noop for paging, but it will be used once paging can get rid of the domctls.
I added p2m_mem_paging_wait() which calls p2m_mem_paging_populate(), then goes to sleep until p2m_mem_paging_get_entry() indicates the gfn is back. If I understand your change correctly a guest vcpu can always place a request. If p2m_mem_paging_populate() happens to fail to put a request, what is supposed to happen in p2m_mem_paging_wait()? Should it skip the wait_event() and return to its caller?
With my implementation both p2m_mem_paging_wait() and p2m_mem_paging_populate() will stop exectution until either the gfn is back or if there is room in the buffer. There is no need for exit code handling.
Olaf
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
In reply to Andres Lagar-CavillaQuoted 8 lines · Andres Lagar-Cavilla
The memevent code currently has a mechanism for reserving space in the ring before putting an event, but each caller must individually ensure that the vCPUs are correctly paused if no space is available.
This fixes that issue by reversing the semantics: we ensure that enough space is always left for one event per vCPU in the ring. If, after putting the current request, this constraint will be violated by the current vCPU when putting putting another request in the ring, we pause the vCPU.
What about operations that touch more than one page of guest memory? (E.g., pagetable walks, emulated faults and task switches). Can't they still fill up the ring?
IIRC there are still cases where we need wait-queues anyway (when we hit a paged-out page after an non-idempotent action has already been taken). Is the purpose of this change just to reduce the number of wait-queue uses or do you think you can do without them entirely?
Cheers,
Tim.
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
Quoted 10 lines · Tim Deegan
Quoted 8 lines · Andres Lagar-Cavilla
The memevent code currently has a mechanism for reserving space in the ring before putting an event, but each caller must individually ensure that the vCPUs are correctly paused if no space is available.
This fixes that issue by reversing the semantics: we ensure that enough space is always left for one event per vCPU in the ring. If, after putting the current request, this constraint will be violated by the current vCPU when putting putting another request in the ring, we pause the vCPU.
Quoted 8 lines
What about operations that touch more than one page of guest memory? (E.g., pagetable walks, emulated faults and task switches). Can't they still fill up the ring?
IIRC there are still cases where we need wait-queues anyway (when we hit a paged-out page after an non-idempotent action has already been taken). Is the purpose of this change just to reduce the number of wait-queue uses or do you think you can do without them entirely?
It's definitely not possible to do without entirely. I'm pretty sure Andres agrees at least that far.
-- Keir
Quoted 3 lines
Cheers,
Tim.
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
Quoted 20 lines · Tim Deegan
Quoted 19 lines
Quoted 6 lines · Andres Lagar-Cavilla
The memevent code currently has a mechanism for reserving space in the ring
before putting an event, but each caller must individually ensure that the
vCPUs are correctly paused if no space is available.Quoted 6 lines
This fixes that issue by reversing the semantics: we ensure that enough space
is always left for one event per vCPU in the ring. If, after putting the
current request, this constraint will be violated by the current vCPU when putting putting another request in the ring, we pause the vCPU.What about operations that touch more than one page of guest memory? (E.g., pagetable walks, emulated faults and task switches). Can't they still fill up the ring?
Quoted 8 lines
Quoted 4 lines
IIRC there are still cases where we need wait-queues anyway (when we hit a paged-out page after an non-idempotent action has already been taken). Is the purpose of this change just to reduce the number of wait-queue uses or do you think you can do without them entirely?
It's definitely not possible to do without entirely. I'm pretty sure Andres
agrees at least that far.
I do agree that far :)
Andres
Quoted 9 lines
-- Keir
Quoted 3 lines
Cheers,
Tim.
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
In reply to Tim DeeganQuoted 14 lines
Quoted 6 lines · Andres Lagar-Cavilla
The memevent code currently has a mechanism for reserving space in the ring
before putting an event, but each caller must individually ensure that the
vCPUs are correctly paused if no space is available.Quoted 6 lines
This fixes that issue by reversing the semantics: we ensure that enough space
is always left for one event per vCPU in the ring. If, after putting the
current request, this constraint will be violated by the current vCPU when putting putting another request in the ring, we pause the vCPU.
Quoted 3 lines
What about operations that touch more than one page of guest memory? (E.g., pagetable walks, emulated faults and task switches). Can't they still fill up the ring?
Those only generate events on paging, which would go to sleep on the first fault with a wait queue.
There is one case in which the guest vcpu can generate unbound events, and that is balloon down -> decrease_reservation -> paging_drop events. I handle that with preemption of the decrease_reservation hypercall.
Quoted 5 lines
IIRC there are still cases where we need wait-queues anyway (when we hit a paged-out page after an non-idempotent action has already been taken). Is the purpose of this change just to reduce the number of wait-queue uses or do you think you can do without them entirely?
Certainly, there's no way around wait-queues, for, say hvm_copy with a paged out page.
Andres
Quoted 5 lines
Cheers,
Tim.
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
In reply to Andres Lagar-CavillaQuoted 40 lines · Andres Lagar-Cavilla
xen/arch/x86/mm/mem_event.c +33
-1@@ -133,31 +185,95 @@ static int mem_event_disable(struct mem_return 0 ; } -void mem_event_put_request (struct domain *d,struct mem_event_domain *med,mem_event_request_t *req) +static inline int mem_event_ring_free (struct domain *d,struct mem_event_domain *med) { +int free_requests; + + free_requests =RING_FREE_REQUESTS (&med->front_ring); +if (unlikely (free_requests < d->max_vcpus) ) + { +/* This may happen during normal operation (hopefully not often). */ +gdprintk (XENLOG_INFO ,"mem_event request slots for domain %d: %d\n" , + d->domain_id, free_requests); + } + +return free_requests; +} + +/* Return values +* zero: success +* -ENOSYS: no ring +* -EAGAIN: ring is full and the event has not been transmitted. +* Only foreign vcpu's get EAGAIN +* -EBUSY: guest vcpu has been paused due to ring congestion +*/ +int mem_event_put_request (struct domain *d,struct mem_event_domain *med,mem_event_request_t *req) +{ +int ret =0 ; +int foreign = (d != current->domain); +/* +* We ensure that each vcpu can put at least *one* event -- because some +* events are not repeatable, such as dropping a page. This will ensure no +* vCPU is left with an event that they must place on the ring, but cannot. +* They will be paused after the event is placed. +* See large comment below in mem_event_unpause_vcpus(). +*/ +if ( !foreign &&mem_event_ring_free (d, med) < d->max_vcpus )
xen/arch/x86/mm/mem_event.c static int mem_event_disable(struct mem_ · line 232
Quoted 10 lines
xen/arch/x86/mm/mem_event.c +6
-0+ { +mem_event_mark_and_pause (current, med); + ret = -EBUSY ; + }mem_event_ring_unlock (med);notify_via_xen_event_channel (d, med->xen_port); + +return ret;
What will happen if the guest has more vcpus than r->nr_ents in the ring buffer? To me it looks like no event can be placed into the ring and -EBUSY is returned instead.
Olaf
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
Quoted 57 lines · Andres Lagar-Cavilla
Quoted 56 lines
xen/arch/x86/mm/mem_event.c +7
-1@@ -133,31 +185,95 @@ static int mem_event_disable(struct mem_return 0 ; } -void mem_event_put_request (struct domain *d,struct mem_event_domain *med, mem_event_request_t *req) +static inline int mem_event_ring_free (struct domain *d,struct mem_event_domain *med) { +int free_requests; + + free_requests =RING_FREE_REQUESTS (&med->front_ring); +if (unlikely (free_requests < d->max_vcpus) ) + { +/* This may happen during normal operation (hopefully not often). */
+ gdprintk(XENLOG_INFO, "mem_eventrequest slots for domain %d:
%d\n",xen/arch/x86/mm/mem_event.c +14
-0+ d->domain_id, free_requests); + } + +return free_requests; +} + +/* Return values +* zero: success +* -ENOSYS: no ring +* -EAGAIN: ring is full and the event has not been transmitted. +* Only foreign vcpu's get EAGAIN +* -EBUSY: guest vcpu has been paused due to ring congestion +*/ +int mem_event_put_request(struct domain *d, struct mem_event_domain*med,
mem_event_request_t*req)xen/arch/x86/mm/mem_event.c +5
-0+{ +int ret =0 ; +int foreign = (d != current->domain); +/* +* We ensure that each vcpu can put at least *one* event -- because some
+ * events are not repeatable, such as dropping a page. This will
ensure no
+ * vCPU is left with an event that they must place on the ring, but
cannot.xen/arch/x86/mm/mem_event.c +8
-0+* They will be paused after the event is placed. +* See large comment below in mem_event_unpause_vcpus(). +*/ +if ( !foreign &&mem_event_ring_free (d, med) < d->max_vcpus ) + { +mem_event_mark_and_pause (current, med); + ret = -EBUSY ; + }mem_event_ring_unlock (med);
Quoted 8 lines
Review
Quoted 3 lines
notify_via_xen_event_channel(d, med->xen_port);(quoted diff) +2
-0+ + return ret;What will happen if the guest has more vcpus than
r->nr_entsin the ring buffer? To me it looks like no event can be placed into the ring and-EBUSYis returned instead.
MAX_HVM_VCPUS sits at 128 right now. Haven't compile checked, but that probably means we would need a two page ring. And then, when 1024-cpu hosts arrive and we grow MAX_HVM_VCPUS, we grow the ring size again.
Or, we could limit the constraint to the number of online vcpus, which would get somewhat tricky for vcpu hot-plugging.
I can fix that separately, once there is a decision on which way to go re ring management.
Andres
Quoted 3 lines
Olaf
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
Quoted 3 lines · Andres Lagar-Cavilla
MAX_HVM_VCPUSsits at 128 right now. Haven't compile checked, but that probably means we would need a two page ring. And then, when 1024-cpu hosts arrive and we growMAX_HVM_VCPUS, we grow the ring size again.
The ring has 64 entries.
Quoted 5 lines
Or, we could limit the constraint to the number of online vcpus, which would get somewhat tricky for vcpu hot-plugging.
I can fix that separately, once there is a decision on which way to go re ring management.
I just sent "[PATCH] mem_event: use wait queue when ring is full" to the list. This version ought to work, it takes the request from both target and foreign cpus into account and leaves at lease one slot for the target.
Olaf
Re: [PATCH 2 of 5] Improve ring management for memory events. Do not lose guest events
I just pushed all that I had on mem event. I split it into two series. One that adds features that are independent on how we choose to manage the ring.
Hopefully we'll concur these features are useful (such as kicking Xen to consume batched responses directly with an event channel, no domctl needed). And that they should go in the tree. I've been using them for a month already.
The second series is a refreshed post of our version of ring management, sans wait queues. With both up to date version in hand, we should be able to reach a consensus on which to use.
Thanks
Andres
Quoted 24 lines · Andres Lagar-Cavilla
Quoted 4 lines
MAX_HVM_VCPUSsits at 128 right now. Haven't compile checked, but that probably means we would need a two page ring. And then, when 1024-cpu hosts arrive and we growMAX_HVM_VCPUS, we grow the ring size again.The ring has 64 entries.
Quoted 6 lines
Or, we could limit the constraint to the number of online vcpus, which would get somewhat tricky for vcpu hot-plugging.
I can fix that separately, once there is a decision on which way to go re
ring management.I just sent "[PATCH]
mem_event: use wait queue when ring is full" to the list. This version ought to work, it takes the request from both target and foreign cpus into account and leaves at lease one slot for the target.Olaf
PATCH 1/1 Create a generic callback mechanism for Xen-bound event channels
In reply to Andres Lagar-Cavillaxen/arch/ia64/vmx/vmx_init.c | 2 +- xen/arch/x86/hvm/hvm.c | 7 ++- xen/arch/x86/mm/mem_event.c | 3 +- xen/common/event_channel.c | 75 ++++++++++++++++++++++++++++++++++--------- xen/include/xen/event.h | 5 ++- xen/include/xen/sched.h | 2 +- 6 files changed, 70 insertions(+), 24 deletions(-)
For event channels for which Xen is the consumer, there currently is a single action. With this patch, we allow event channel creators to specify a generic callback (or no callback). Because the expectation is that there will be few callbacks, they are stored in a small table.
xen/arch/ia64/vmx/vmx_init.c +1 -1
@@ -377,7 +377,7 @@ vmx_vcpu_initialise(struct vcpu *v) {struct vmx_ioreq_page *iorp = &v->domain->arch.hvm_domain.ioreq; -int rc =alloc_unbound_xen_event_channel (v,0 ); +int rc =alloc_unbound_xen_event_channel (v,0 ,NULL );if (rc <0 )return rc; v->arch.arch_vmx.xen_port = rc;
xen/arch/x86/hvm/hvm.c +4 -3
@@ -979,7 +979,7 @@ int hvm_vcpu_initialise(struct vcpu *v) goto fail3;/* Create ioreq event channel. */ - rc =alloc_unbound_xen_event_channel (v,0 ); + rc =alloc_unbound_xen_event_channel (v,0 ,NULL );if ( rc <0 ) goto fail4;
@@ -989,7 +989,7 @@ int hvm_vcpu_initialise(struct vcpu *v)if ( v->vcpu_id ==0 ) {/* Create bufioreq event channel. */ - rc =alloc_unbound_xen_event_channel (v,0 ); + rc =alloc_unbound_xen_event_channel (v,0 ,NULL );if ( rc <0 ) goto fail2; v->domain->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_EVTCHN ] = rc;
@@ -3551,7 +3551,8 @@ long do_hvm_op(unsigned long op, XEN_GUE for_each_vcpu ( d, v ) {int old_port, new_port; - new_port =alloc_unbound_xen_event_channel (v, a.value); + new_port =alloc_unbound_xen_event_channel ( + v, a.value,NULL );if ( new_port <0 ) { rc = new_port;
xen/arch/x86/mm/mem_event.c +2 -1
@@ -96,7 +96,8 @@ static int mem_event_enable(struct domai/* Allocate event channel */ rc =alloc_unbound_xen_event_channel (d->vcpu[0 ], - current->domain->domain_id); + current->domain->domain_id, +NULL );if ( rc <0 ) goto err;
xen/common/event_channel.c +58 -17
141 lines not on this page · raw message
xen/include/xen/event.h +4 -1
@@ -51,8 +51,11 @@ int evtchn_unmask(unsigned int port);void evtchn_move_pirqs (struct vcpu *v);/* Allocate/free a Xen-attached event channel port. */ +typedef void (*xen_event_channel_notification_t )( +struct vcpu *v,unsigned int port);int alloc_unbound_xen_event_channel ( -struct vcpu *local_vcpu,domid_t remote_domid); +struct vcpu *local_vcpu,domid_t remote_domid, +xen_event_channel_notification_t notification_fn);void free_xen_event_channel (struct vcpu *local_vcpu,int port);
xen/include/xen/sched.h +1 -1
@@ -47,7 +47,7 @@ struct evtchn#define ECS_VIRQ 5/* Channel is bound to a virtual IRQ line. */ #define ECS_IPI 6/* Channel is bound to a virtual IPI line. */ u8 state;/* ECS_* */ -u8 consumer_is_xen;/* Consumed by Xen or by guest? */ +u8 xen_consumer;/* Consumer in Xen, if any? (0 = send to guest) */ u16 notify_vcpu_id;/* VCPU for local delivery notification */ union { struct {
PATCH 1/1 Make the prototype of p2m_mem_access_resume consistent
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/mem_access.c | 3 +-- xen/arch/x86/mm/p2m.c | 3 +-- xen/include/asm-x86/p2m.h | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-)
xen/arch/x86/mm/mem_access.c +1 -2
@@ -29,13 +29,12 @@ int mem_access_domctl(struct domain *d, XEN_GUEST_HANDLE(void) u_domctl) {int rc; -struct p2m_domain *p2m =p2m_get_hostp2m (d);switch ( mec->op ) {case XEN_DOMCTL_MEM_EVENT_OP_ACCESS_RESUME : { -p2m_mem_access_resume (p2m); +p2m_mem_access_resume (d); rc =0 ; }break ;
xen/arch/x86/mm/p2m.c +1 -2
@@ -1194,9 +1194,8 @@ void p2m_mem_access_check(unsigned long/* VCPU paused */ } -void p2m_mem_access_resume (struct p2m_domain *p2m) +void p2m_mem_access_resume (struct domain *d) { -struct domain *d = p2m->domain;mem_event_response_t rsp;/* Pull all responses off the ring */
xen/include/asm-x86/p2m.h +1 -1
@@ -495,7 +495,7 @@ static inline void p2m_mem_paging_populavoid p2m_mem_access_check (unsigned long gpa,bool_t gla_valid,unsigned long gla,bool_t access_r,bool_t access_w,bool_t access_x);/* Resumes the running of the VCPU, restarting the last instruction */ -void p2m_mem_access_resume (struct p2m_domain *p2m); +void p2m_mem_access_resume (struct domain *d);/* Set access type for a region of pfns. * If start_pfn == -1ul, sets the default access type */
Re: [PATCH 4 of 5] Make the prototype of p2m_mem_access_resume consistent
Quoted 9 lines · Andres Lagar-Cavilla
xen/arch/x86/mm/mem_access.c | 3 +-- xen/arch/x86/mm/p2m.c | 3 +-- xen/include/asm-x86/p2m.h | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-)Signed-off-by Adin Scannell <[email protected]>Signed-off-by Keir Fraser <[email protected]>Signed-off-by Andres Lagar-Cavilla <[email protected]>
I've applied this one obvious fixup in isolation; the rest of the series clearly needs some further discussion.
Cheers,
Tim.
Re: [PATCH 4 of 5] Make the prototype of p2m_mem_access_resume consistent
Quoted 7 lines
Quoted 6 lines · Andres Lagar-Cavilla
xen/arch/x86/mm/mem_access.c | 3 +-- xen/arch/x86/mm/p2m.c | 3 +-- xen/include/asm-x86/p2m.h | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-)
Quoted 6 lines
Quoted 3 lines
Signed-off-by Adin Scannell <[email protected]>Signed-off-by Keir Fraser <[email protected]>Signed-off-by Andres Lagar-Cavilla <[email protected]>I've applied this one obvious fixup in isolation; the rest of the series clearly needs some further discussion.
Thanks Tim,
I had to rebase the main patch in the series given the recent commit "mem_event: move mem_event_domain out of struct domain". The rebase is obvious and does not affect the gist of the patch, but I can resend that single patch, the whole series, etc.
Andres
Quoted 5 lines
Cheers,
Tim.
PATCH 1/1 Allow memevent responses to be signaled via the event channel
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/mem_event.c | 26 ++++++++++++++++++++------ 1 files changed, 20 insertions(+), 6 deletions(-)
Don't require a separate domctl to notify the memevent interface that an event has occured. This domctl can be taxing, particularly when you are scaling events and paging to many domains across a single system. Instead, we use the existing event channel to signal when we place something in the ring (as per normal ring operation).
xen/arch/x86/mm/mem_event.c +20 -6
@@ -37,9 +37,11 @@#define mem_event_ring_lock (_med) spin_lock(&(_med)->ring_lock)#define mem_event_ring_unlock (_med) spin_unlock(&(_med)->ring_lock) -static int mem_event_enable (struct domain *d, -xen_domctl_mem_event_op_t *mec, -struct mem_event_domain *med) +static int mem_event_enable ( +struct domain *d, +xen_domctl_mem_event_op_t *mec, +struct mem_event_domain *med, +xen_event_channel_notification_t notification_fn) {int rc;struct domain *dom_mem_event = current->domain;
@@ -97,7 +99,7 @@ static int mem_event_enable(struct domai/* Allocate event channel */ rc =alloc_unbound_xen_event_channel (d->vcpu[0 ], current->domain->domain_id, -NULL ); + notification_fn);if ( rc <0 ) goto err;
@@ -330,6 +332,18 @@ int mem_event_check_ring(struct domain *return 0 ; } +/* Registered with Xen-bound event channel for incoming notifications. */ +static void mem_paging_notification (struct vcpu *v,unsigned int port) +{ +p2m_mem_paging_resume (v->domain); +} + +/* Registered with Xen-bound event channel for incoming notifications. */ +static void mem_access_notification (struct vcpu *v,unsigned int port) +{ +p2m_mem_access_resume (v->domain); +} +int mem_event_domctl (struct domain *d,xen_domctl_mem_event_op_t *mec,XEN_GUEST_HANDLE (void ) u_domctl) {
@@ -391,7 +405,7 @@ int mem_event_domctl(struct domain *d, xif ( p2m->pod.entry_count )break ; - rc =mem_event_enable (d, mec, med); + rc =mem_event_enable (d, mec, med, mem_paging_notification); }break ;
@@ -430,7 +444,7 @@ int mem_event_domctl(struct domain *d, xif ( boot_cpu_data.x86_vendor !=X86_VENDOR_INTEL )break ; - rc =mem_event_enable (d, mec, med); + rc =mem_event_enable (d, mec, med, mem_access_notification); }break ;