Re: [PATCH] x86: Update HVM_PARAM_CALLBACK_IRQ

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Mar 01, 2016 at 03:25:55AM -0700, Jan Beulich wrote:
> >>> On 29.02.16 at 21:39, <konrad.wilk@oracle.com> wrote:
> > @@ -50,11 +52,13 @@ struct hvm_irq {
> >      /* Virtual interrupt and via-link for paravirtual platform driver. */
> >      uint32_t callback_via_asserted;
> >      union {
> > +        /* These MUST match with HVM_PARAM_CALLBACK_IRQ types. */
> >          enum {
> > -            HVMIRQ_callback_none,
> > -            HVMIRQ_callback_gsi,
> > -            HVMIRQ_callback_pci_intx,
> > -            HVMIRQ_callback_vector
> > +            HVMIRQ_callback_gsi = HVM_PARAM_CALLBACK_TYPE_GSI,
> > +            HVMIRQ_callback_pci_intx = HVM_PARAM_CALLBACK_TYPE_PCI_INTX,
> > +            HVMIRQ_callback_vector = HVM_PARAM_CALLBACK_TYPE_VECTOR,
> > +            /* Will change if we add more types. */
> > +            HVMIRQ_callback_none = HVM_PARAM_CALLBACK_TYPE_NUM,
> >          } callback_via_type;
> >      };
> 
> I.e. a domain will now start in HVMIRQ_callback_gsi mode (due to
> HVM_PARAM_CALLBACK_TYPE_GSI = 0) instead of
> HVMIRQ_callback_none? That can't be right.

Argh. I was looking for all the cases for callback_via_type being set but
of course missed the most obvious one!

> 
> > --- a/xen/include/public/hvm/params.h
> > +++ b/xen/include/public/hvm/params.h
> > @@ -56,6 +56,21 @@
> >   */
> >  
> >  /*
> > + * In the future this may change.
> > + */
> > +#define HVM_PARAM_CALLBACK_TYPE_NUM      HVM_PARAM_CALLBACK_TYPE_VECTOR + 1
> 
> Missing parentheses.
> 
> > +/*
> > + * The val[63:56] convenience shift.
> > + */
> > +#define HVM_PARAM_CALLBACK_TYPE_SHIFT 56
> > +
> > +/*
> > + * Wrapper around for HVM_PARAM_CALLBACK_TYPE_VECTOR.
> 
> Looks like either "around" or "for" wants to be dropped. I also
> think this would better live right next to that constant's definition.

OK.
> 
> Also note that all comments you add are single line ones, and
> hence don't conform to our coding style (yes, there are other
> [bad] examples of such in this file).

/me nods. Will make it conform to the single line one type comment.

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