Re: [Xen-devel] [PATCH 2/9] vm_event: Define VM_EVENT type

[Petre Ovidiu PIRCALABU <ppircalabu@xxxxxxxxxxxxxxx>]

On Fri, 2019-05-31 at 16:26 -0700, Andrew Cooper wrote:
> On 30/05/2019 07:18, Petre Pircalabu wrote:
> > 
> 
> This is an internal helper, so can reasonably be expected to not be
> called with junk, and can do away with the param pointer.
> 
> Something like
> 
> static int xc_vm_event_ring_pfn_param(unsigned int type)
> {
>     switch ( type )
>     {
>         case XEN_VM_EVENT_TYPE_PAGING:
>             return HVM_PARAM_PAGING_RING_PFN;
> ...
>         default:
>             return -EINVAL;
>     }
> }
> 
> will work fine because HVM_PARAM_* are all tiny unsigned integers in
> practice.  It also has a more sensible API for the caller.

I think in the end it's best just to move the helper to
xc_vm_event_enable (inline the switch) as it is not use outside it.

> 
> > index 19486d5..19281fa 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -769,80 +769,18 @@ struct xen_domctl_gdbsx_domstatus {
> >   * VM event operations
> >   */
> >  
> > 
> > -
> > -/* Use for teardown/setup of helper<->hypervisor interface for
> > paging,
> > - * access and sharing.*/
> >  struct xen_domctl_vm_event_op {
> > -    uint32_t       op;           /* XEN_VM_EVENT_* */
> > -    uint32_t       mode;         /* XEN_DOMCTL_VM_EVENT_OP_* */
> > +    uint32_t        op;           /* XEN_VM_EVENT_* */
> > +    uint32_t        type;         /* XEN_VM_EVENT_TYPE_* */
> 
> Why did the vertical alignment change?

The initial vertical alignment was not 4 spaces (I will revert it back
to the way it was in order to simplify the review  )
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel