> -----Original Message-----
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 25 November 2016 14:08
> To: Paul Durrant <paul.durr...@citrix.com>
> Cc: Andrew Cooper <andrew.coop...@citrix.com>; Wei Liu
> <wei.l...@citrix.com>; Ian Jackson <ian.jack...@citrix.com>; xen-
> de...@lists.xenproject.org; Daniel De Graaf <dgde...@tycho.nsa.gov>
> 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.durr...@citrix.com> 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@lists.xen.org
https://lists.xen.org/xen-devel
