Re: [Xen-devel] [PATCH-for-4.9 v1 7/8] dm_op: convert HVMOP_inject_trap and HVMOP_inject_msi

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 25 November 2016 14:08
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Wei Liu
> <wei.liu2@xxxxxxxxxx>; Ian Jackson <Ian.Jackson@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH-for-4.9 v1 7/8] dm_op: convert
> HVMOP_inject_trap and HVMOP_inject_msi
> 
> >>> On 18.11.16 at 18:14, <paul.durrant@xxxxxxxxxx> wrote:
> > +static int dm_op_inject_trap(struct domain *d, unsigned int vcpuid,
> > +                             uint16_t vector, uint8_t type,
> > +                             uint8_t insn_len, uint32_t error_code,
> > +                             unsigned long cr2)
> > +{
> > +    struct vcpu *v;
> > +
> > +    if ( vector > INT16_MAX )
> > +        return -EINVAL;
> 
> Please limit vector to uint8_t and delete this strange (architecturally
> wrong) check.

This check is only meant to check that the field assignment below it doesn't 
overflow. IIRC the old code didn't even do that. I'll limit to uint8_t as you 
suggest though.

> 
> > +    if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
> > +        return -EINVAL;
> 
> ENOENT (to make error reasons distinguishable for the caller)?
> 

Ok.

> > +    case DMOP_inject_msi:
> > +    {
> > +        struct xen_dm_op_inject_msi *data =
> > +            &op.u.inject_msi;
> > +
> > +        rc = hvm_inject_msi(d, data->addr, data->data);
> 
> Line length clearly is not an issue here, but if you want to keep
> the helper variable, then please constify it (which I guess would
> apply to some of the earlier patches too).

Ok. I'll try to const everything that can't be subject to a continuation.

  Paul

> 
> Jan


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