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

From: Dave Hansen
Date: Sat Aug 29 2020 - 12:14:16 EST


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?

> 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.

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