Re: [Xen-devel] [PATCH V3] xen: Fix BUFIOREQ evtchn init for a stubdom.

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On 02/07/12 16:58, Jan Beulich wrote:
> > > > On 02.07.12 at 17:41, Anthony PERARD <anthony.perard@xxxxxxxxxx> 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 <anthony.perard@xxxxxxxxxx>
> > ---
> > 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



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel