From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony PERARD Subject: Re: [PATCH V3] xen: Fix BUFIOREQ evtchn init for a stubdom. Date: Mon, 2 Jul 2012 17:05:19 +0100 Message-ID: <4FF1C6BF.2010500@citrix.com> References: <1341243676-2369-1-git-send-email-anthony.perard@citrix.com> <4FF1E130020000780008D27E@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4FF1E130020000780008D27E@nat28.tlf.novell.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: Jan Beulich Cc: Stefano Stabellini , "Keir (Xen.org)" , Ian Campbell , Xen Devel List-Id: xen-devel@lists.xenproject.org On 02/07/12 16:58, Jan Beulich wrote: >>>> On 02.07.12 at 17:41, Anthony PERARD wrote: >> This is a missing part from the previous patch that add the BUFIOREQ_EVTCHN >> parameter. This patch changes the ownership of the buifioreq event channel >> to >> the stubdom (when HVM_PARAM_DM_DOMAIN is set within the stubdom). >> >> This patch introduces an helper to replace a xen port. >> >> This fix the initialization of QEMU inside the stubdomain. >> >> Signed-off-by: Anthony PERARD >> --- >> Change v3: >> For the bufioreq replacement: >> - the code is now out of the vcpu loop >> - the code does not take a lock anymore >> - rename int *port to int *p_port >> Change v2: >> - an helper >> - the replacement of the buferioreq evtchn is inside iorp->lock now. >> >> xen/arch/x86/hvm/hvm.c | 32 ++++++++++++++++++++++---------- >> 1 files changed, 22 insertions(+), 10 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index e0d495d..c2dfa73 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -3664,6 +3664,21 @@ static int hvmop_flush_tlb_all(void) >> return 0; >> } >> >> +static int hvm_replace_event_channel(struct vcpu *v, uint64_t remote_domid, >> + int *p_port) >> +{ >> + int old_port, new_port; >> + >> + new_port = alloc_unbound_xen_event_channel(v, remote_domid, NULL); >> + if ( new_port < 0 ) >> + return new_port; >> + >> + /* xchg() ensures that only we free_xen_event_channel() */ >> + old_port = xchg(p_port, new_port); >> + free_xen_event_channel(v, old_port); >> + return 0; >> +} >> + >> long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg) >> >> { >> @@ -3775,19 +3790,16 @@ long do_hvm_op(unsigned long op, >> XEN_GUEST_HANDLE(void) arg) >> rc = 0; >> domain_pause(d); /* safe to change per-vcpu xen_port */ >> iorp = &d->arch.hvm_domain.ioreq; >> + if (d->vcpu[0]) >> + hvm_replace_event_channel(d->vcpu[0], a.value, >> + (int*)&d->vcpu[0]->domain->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_EVTCHN]); > > Did I overlook this in v2? You clearly need to handle the error > case here (it is being handled, albeit - but that's not your patch's > fault - only in a rudimentary way, inside the loop). Well, if there is an error with the replace, it just break the for_each loop and do domain_unpause. So my guest was to leave it fail a second time :). But here, how should I handle the error case? If there is an error, should I not go into the for_each and go strait to domain_unpause (with the rc value set)? >> for_each_vcpu ( d, v ) >> { >> - int old_port, new_port; >> - new_port = alloc_unbound_xen_event_channel( >> - v, a.value, NULL); >> - if ( new_port < 0 ) >> - { >> - rc = new_port; >> + rc = hvm_replace_event_channel(v, a.value, >> + >> &v->arch.hvm_vcpu.xen_port); >> + if ( rc ) >> break; >> - } >> - /* xchg() ensures that only we free_xen_event_channel() >> */ >> - old_port = xchg(&v->arch.hvm_vcpu.xen_port, new_port); >> - free_xen_event_channel(v, old_port); >> + >> spin_lock(&iorp->lock); >> if ( iorp->va != NULL ) >> get_ioreq(v)->vp_eport = v->arch.hvm_vcpu.xen_port; >> -- >> Anthony PERARD > > > -- Anthony PERARD