Re: [Xen-devel] [PATCH 15/18] PVH xen: add hypercall support for PVH

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.06.13 at 05:09, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> On Tue, 25 Jun 2013 11:12:25 +0100
> "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> 
>> > @@ -3777,7 +3815,7 @@ long do_hvm_op(unsigned long op,
>> > XEN_GUEST_HANDLE_PARAM(void) arg) return -ESRCH;
>> >  
>> >          rc = -EINVAL;
>> > -        if ( !is_hvm_domain(d) )
>> > +        if ( is_pv_domain(d) )
>> >              goto param_fail;
>> >  
>> >          rc = xsm_hvm_param(XSM_TARGET, d, op);
>> > @@ -3949,7 +3987,7 @@ long do_hvm_op(unsigned long op,
>> > XEN_GUEST_HANDLE_PARAM(void) arg) break;
>> >              }
>> >  
>> > -            if ( rc == 0 ) 
>> > +            if ( rc == 0 && !is_pvh_domain(d) )
>> >              {
>> >                  d->arch.hvm_domain.params[a.index] = a.value;
>> >  
>> 
>> This last check I think you do because params[] points nowhere for
> Correct.
> 
>> PVH guests. If so - why don't you just drop this and the earlier
>> hunk? Or otherwise some of the case statements between need to
> 
> I don't understand, drop from where? You mean a totally separate function
> for PVH (I had that in very earlier patches).

No, just drop the two patch hunks. The first check then allows to
only get into all the parameter handling code for HVM guests, and
hence no further checks are necessary further down in any of the
parameter handling.

>> also guard against accessing the unset pointer.
> 
> Correct, I'd need to do that. Originally, I had a white list of case
> operations prohibited for PVH, but removed it.

This would only be necessary if _some_ of the parameters can
validly be set of PVH. In which case params[] can't be NULL
anymore, so the whole logic would need to change.

Jan


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