Re: [Xen-devel] [PATCH v5 5/9] ioreq-server: on-demand creation of ioreq server

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 06.05.14 at 16:24, <Paul.Durrant@xxxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >>> On 01.05.14 at 14:08, <paul.durrant@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/hvm/hvm.c
>> > +++ b/xen/arch/x86/hvm/hvm.c
>> > @@ -389,40 +389,38 @@ void hvm_do_resume(struct vcpu *v)
>> >  {
>> >      struct domain *d = v->domain;
>> >      struct hvm_ioreq_server *s = d->arch.hvm_domain.ioreq_server;
>> > -    ioreq_t *p;
>> >
>> >      check_wakeup_from_wait();
>> >
>> >      if ( is_hvm_vcpu(v) )
>> >          pt_restore_timer(v);
>> >
>> > -    if ( !s )
>> > -        goto check_inject_trap;
>> > -
>> > -    /* NB. Optimised for common case (p->state == STATE_IOREQ_NONE).
>> */
>> > -    p = get_ioreq(s, v);
>> > -    while ( p->state != STATE_IOREQ_NONE )
>> > +    if ( s )
>> >      {
>> > -        switch ( p->state )
>> > +        ioreq_t *p = get_ioreq(s, v);
>> > +
>> > +        while ( p->state != STATE_IOREQ_NONE )
>> >          {
>> > -        case STATE_IORESP_READY: /* IORESP_READY -> NONE */
>> > -            rmb(); /* see IORESP_READY /then/ read contents of ioreq */
>> > -            hvm_io_assist(p);
>> > -            break;
>> > -        case STATE_IOREQ_READY:  /* IOREQ_{READY,INPROCESS} ->
>> IORESP_READY */
>> > -        case STATE_IOREQ_INPROCESS:
>> > -            wait_on_xen_event_channel(p->vp_eport,
>> > -                                      (p->state != STATE_IOREQ_READY) &&
>> > -                                      (p->state != 
>> > STATE_IOREQ_INPROCESS));
>> > -            break;
>> > -        default:
>> > -            gdprintk(XENLOG_ERR, "Weird HVM iorequest state %d.\n", p-
>> >state);
>> > -            domain_crash(v->domain);
>> > -            return; /* bail */
>> > +            switch ( p->state )
>> > +            {
>> > +            case STATE_IORESP_READY: /* IORESP_READY -> NONE */
>> > +                rmb(); /* see IORESP_READY /then/ read contents of ioreq 
>> > */
>> > +                hvm_io_assist(p);
>> > +                break;
>> > +            case STATE_IOREQ_READY:  /* IOREQ_{READY,INPROCESS} ->
>> IORESP_READY */
>> > +            case STATE_IOREQ_INPROCESS:
>> > +                wait_on_xen_event_channel(p->vp_eport,
>> > +                                          (p->state != STATE_IOREQ_READY) 
>> > &&
>> > +                                          (p->state != 
>> > STATE_IOREQ_INPROCESS));
>> > +                break;
>> > +            default:
>> > +                gdprintk(XENLOG_ERR, "Weird HVM iorequest state %d.\n", p-
>> >state);
>> > +                domain_crash(d);
>> > +                return; /* bail */
>> > +            }
>> >          }
>> >      }
>> >
>> > - check_inject_trap:
>> >      /* Inject pending hw/sw trap */
>> >      if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
>> >      {
>> 
>> Isn't this entire hunk just a stylistic change from using goto to the
>> alternative if() representation? I would strongly suggest leaving out
>> such reformatting from an already large patch.
>> 
> 
> Ok. I'll pull it into pre-series tidy up.

If you really think it's worthwhile...

Jan


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