On Thu  5.Nov'20 at  7:29:07 +0100, Greg Kroah-Hartman wrote:
On Thu, Nov 05, 2020 at 11:10:29AM +0800, Shuo A Liu wrote:
On Wed  4.Nov'20 at 20:02:35 +0100, Greg Kroah-Hartman wrote:
> On Mon, Oct 19, 2020 at 02:17:52PM +0800, shuo.a....@intel.com wrote:
> > --- /dev/null
> > +++ b/include/uapi/linux/acrn.h
> > @@ -0,0 +1,56 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +/*
> > + * Userspace interface for /dev/acrn_hsm - ACRN Hypervisor Service Module
> > + *
> > + * This file can be used by applications that need to communicate with the 
HSM
> > + * via the ioctl interface.
> > + */
> > +
> > +#ifndef _UAPI_ACRN_H
> > +#define _UAPI_ACRN_H
> > +
> > +#include <linux/types.h>
> > +
> > +/**
> > + * struct acrn_vm_creation - Info to create a User VM
> > + * @vmid:            User VM ID returned from the hypervisor
> > + * @reserved0:               Reserved
> > + * @vcpu_num:                Number of vCPU in the VM. Return from 
hypervisor.
> > + * @reserved1:               Reserved
> > + * @uuid:            UUID of the VM. Pass to hypervisor directly.
> > + * @vm_flag:         Flag of the VM creating. Pass to hypervisor directly.
> > + * @ioreq_buf:               Service VM GPA of I/O request buffer. Pass to
> > + *                   hypervisor directly.
> > + * @cpu_affinity:    CPU affinity of the VM. Pass to hypervisor directly.
> > + * @reserved2:               Reserved
>
> Reserved and must be 0?

Not a must.

That's guaranteed to come back and bite you in the end.

OK. I can fill them with zero before passing them to hypervisor.

You all have read the "how to write a good api" document, right?

Is it Documentation/driver-api/ioctl.rst? Or i missed..


> What are they reserved for?
>
> Same for all of the reserved fields, why?

Some reserved fields are to map layout in the hypervisor side, others
are for future use.

ioctls should not have these, again, please read the documentation.  If
you need something new in the future, just make a new ioctl.

OK. I will remove some reserved fields for scalability. Though i can
keep some reserved fields for alignment (and to keep same data structure
layout with the hypervisor), right?
Documentation/driver-api/ioctl.rst says that explicit reserved fields
could be used.


> > + */
> > +struct acrn_vm_creation {
> > +     __u16   vmid;
> > +     __u16   reserved0;
> > +     __u16   vcpu_num;
> > +     __u16   reserved1;
> > +     __u8    uuid[16];
>
> We have a userspace-visable uid structure in include/uapi/uuid.h, why
> not use that?

we just pass the uuid data from user space to hypervisor. So, we can
remove a header dependeny with using raw data format.

I do not understand this, please use the built-in kernel types we have.

OK. I will use that.

Thanks
shuo

Reply via email to