Re: [PATCH 05/17] virt: acrn: Introduce ACRN HSM basic driver

[Shuo A Liu]

Hi Dave,

On Sat 29.Aug'20 at  9:12:22 -0700, Dave Hansen wrote:
> On 8/29/20 3:46 AM, Shuo A Liu wrote:
> > On Fri 28.Aug'20 at 12:25:59 +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Aug 25, 2020 at 10:45:05AM +0800, shuo.a.liu@xxxxxxxxx wrote:
> > > > +static long acrn_dev_ioctl(struct file *filp, unsigned int cmd,
> > > > +               unsigned long ioctl_param)
> > > > +{
> > > > +    if (cmd == ACRN_IOCTL_GET_API_VERSION) {
> > > > +        if (copy_to_user((void __user *)ioctl_param,
> > > > +                 &api_version, sizeof(api_version)))
> > > > +            return -EFAULT;
> > >
> > > Why are you versioning your api?  Shouldn't that not be a thing and you
> > > either support an ioctl or you do not?
> >
> > The API version here is more for the hypercalls.
> > The hypercalls might evolve later
>
> They might evolve, but the old ones must always keep working.  Right?

Yes, it's right.

> > and the version indicates which set of interfaces (include the
> > paramters' format) should be used by user space tools. Currently,
> > it's used rarely.
> Why do you need this when the core kernel doesn't?  We add syscalls,
> ioctl()s and prctl()s all the time, but nothing is versioned.

Indeed. It looks a bit odd.

> This sounds like something you need to remove from the series.

OK. I will remove the api version related code.

Thanks
shuo