All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Nuno Das Neves <nunodasneves@linux.microsoft.com>,
	linux-hyperv@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, mikelley@microsoft.com,
	viremana@linux.microsoft.com, sunilmut@microsoft.com,
	wei.liu@kernel.org, ligrassi@microsoft.com, kys@microsoft.com
Subject: Re: [RFC PATCH 04/18] virt/mshv: request version ioctl
Date: Fri, 05 Mar 2021 10:18:38 +0100	[thread overview]
Message-ID: <87eeguc61d.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <194e0dad-495e-ae94-3f51-d2c95da52139@linux.microsoft.com>

Nuno Das Neves <nunodasneves@linux.microsoft.com> writes:

> On 2/9/2021 5:11 AM, Vitaly Kuznetsov wrote:
>> Nuno Das Neves <nunodasneves@linux.microsoft.com> writes:
>> 
...
>>> +
>>> +3.1 MSHV_REQUEST_VERSION
>>> +------------------------
>>> +:Type: /dev/mshv ioctl
>>> +:Parameters: pointer to a u32
>>> +:Returns: 0 on success
>>> +
>>> +Before issuing any other ioctls, a MSHV_REQUEST_VERSION ioctl must be called to
>>> +establish the interface version with the kernel module.
>>> +
>>> +The caller should pass the MSHV_VERSION as an argument.
>>> +
>>> +The kernel module will check which interface versions it supports and return 0
>>> +if one of them matches.
>>> +
>>> +This /dev/mshv file descriptor will remain 'locked' to that version as long as
>>> +it is open - this ioctl can only be called once per open.
>>> +
>> 
>> KVM used to have KVM_GET_API_VERSION too but this turned out to be not
>> very convenient so we use capabilities (KVM_CHECK_EXTENSION/KVM_ENABLE_CAP)
>> instead.
>> 
>
> The goal of MSHV_REQUEST_VERSION is to support changes to APIs in the core set.
> When we add new features/ioctls beyond the core we can use an extension/capability
> approach like KVM.
>

Driver versions is a very bad idea from distribution/stable kernel point
of view as it presumes that the history is linear. It is not.

Imagine you have the following history upstream:

MSHV_REQUEST_VERSION = 1
<100 commits with features/fixes>
MSHV_REQUEST_VERSION = 2
<another 100 commits with features/fixes>
MSHV_REQUEST_VERSION = 2

Now I'm a linux distribution / stable kernel maintainer. My kernel is at
MSHV_REQUEST_VERSION = 1. Now I want to backport 1 feature from between
VER=1 and VER=2 and another feature from between VER=2 and VER=3. My
history now looks like

MSHV_REQUEST_VERSION = 1
<5 commits from between VER=1 and VER=2>
   Which version should I declare here???? 
<5 commits from between VER=2 and VER=3>
   Which version should I declare here???? 

If I keep VER=1 then userspace will think that I don't have any extra
features added and just won't use them. If I change VER to 2/3, it'll
think I have *all* features from between these versions.

The only reasonable way to manage this is to attach a "capability" to
every ABI change and expose this capability *in the same commit which
introduces the change to the ABI*. This way userspace will now exactly
which ioctls are available and what are their interfaces.

Also, trying to define "core set" is hard but you don't really need
to.

-- 
Vitaly


WARNING: multiple messages have this Message-ID (diff)
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Nuno Das Neves <nunodasneves@linux.microsoft.com>,
	linux-hyperv@vger.kernel.org
Cc: wei.liu@kernel.org, linux-kernel@vger.kernel.org,
	mikelley@microsoft.com, sunilmut@microsoft.com,
	virtualization@lists.linux-foundation.org,
	viremana@linux.microsoft.com, ligrassi@microsoft.com
Subject: Re: [RFC PATCH 04/18] virt/mshv: request version ioctl
Date: Fri, 05 Mar 2021 10:18:38 +0100	[thread overview]
Message-ID: <87eeguc61d.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <194e0dad-495e-ae94-3f51-d2c95da52139@linux.microsoft.com>

Nuno Das Neves <nunodasneves@linux.microsoft.com> writes:

> On 2/9/2021 5:11 AM, Vitaly Kuznetsov wrote:
>> Nuno Das Neves <nunodasneves@linux.microsoft.com> writes:
>> 
...
>>> +
>>> +3.1 MSHV_REQUEST_VERSION
>>> +------------------------
>>> +:Type: /dev/mshv ioctl
>>> +:Parameters: pointer to a u32
>>> +:Returns: 0 on success
>>> +
>>> +Before issuing any other ioctls, a MSHV_REQUEST_VERSION ioctl must be called to
>>> +establish the interface version with the kernel module.
>>> +
>>> +The caller should pass the MSHV_VERSION as an argument.
>>> +
>>> +The kernel module will check which interface versions it supports and return 0
>>> +if one of them matches.
>>> +
>>> +This /dev/mshv file descriptor will remain 'locked' to that version as long as
>>> +it is open - this ioctl can only be called once per open.
>>> +
>> 
>> KVM used to have KVM_GET_API_VERSION too but this turned out to be not
>> very convenient so we use capabilities (KVM_CHECK_EXTENSION/KVM_ENABLE_CAP)
>> instead.
>> 
>
> The goal of MSHV_REQUEST_VERSION is to support changes to APIs in the core set.
> When we add new features/ioctls beyond the core we can use an extension/capability
> approach like KVM.
>

Driver versions is a very bad idea from distribution/stable kernel point
of view as it presumes that the history is linear. It is not.

Imagine you have the following history upstream:

MSHV_REQUEST_VERSION = 1
<100 commits with features/fixes>
MSHV_REQUEST_VERSION = 2
<another 100 commits with features/fixes>
MSHV_REQUEST_VERSION = 2

Now I'm a linux distribution / stable kernel maintainer. My kernel is at
MSHV_REQUEST_VERSION = 1. Now I want to backport 1 feature from between
VER=1 and VER=2 and another feature from between VER=2 and VER=3. My
history now looks like

MSHV_REQUEST_VERSION = 1
<5 commits from between VER=1 and VER=2>
   Which version should I declare here???? 
<5 commits from between VER=2 and VER=3>
   Which version should I declare here???? 

If I keep VER=1 then userspace will think that I don't have any extra
features added and just won't use them. If I change VER to 2/3, it'll
think I have *all* features from between these versions.

The only reasonable way to manage this is to attach a "capability" to
every ABI change and expose this capability *in the same commit which
introduces the change to the ABI*. This way userspace will now exactly
which ioctls are available and what are their interfaces.

Also, trying to define "core set" is hard but you don't really need
to.

-- 
Vitaly

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

  reply	other threads:[~2021-03-05  9:19 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-21  0:30 [RFC PATCH 00/18] Microsoft Hypervisor root partition ioctl interface Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 01/18] x86/hyperv: convert hyperv statuses to linux error codes Nuno Das Neves
2021-02-09 13:04   ` Vitaly Kuznetsov
2021-02-09 13:04     ` Vitaly Kuznetsov
2021-03-04 18:24     ` Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 02/18] asm-generic/hyperv: convert hyperv statuses to strings Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 03/18] virt/mshv: minimal mshv module (/dev/mshv/) Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 04/18] virt/mshv: request version ioctl Nuno Das Neves
2021-02-08 19:41   ` Michael Kelley
2021-02-08 19:41     ` Michael Kelley via Virtualization
2021-03-04 21:35     ` Nuno Das Neves
2021-02-09 13:11   ` Vitaly Kuznetsov
2021-03-04 18:43     ` Nuno Das Neves
2021-03-05  9:18       ` Vitaly Kuznetsov [this message]
2021-03-05  9:18         ` Vitaly Kuznetsov
2021-04-07  0:21         ` Nuno Das Neves
2021-04-07  7:38           ` Vitaly Kuznetsov
2021-04-07  7:38             ` Vitaly Kuznetsov
2021-04-07 13:43             ` Wei Liu
2021-04-07 14:02               ` Vitaly Kuznetsov
2021-04-07 14:02                 ` Vitaly Kuznetsov
2021-04-07 14:19                 ` Wei Liu
2020-11-21  0:30 ` [RFC PATCH 05/18] virt/mshv: create partition ioctl Nuno Das Neves
2021-02-09 13:15   ` Vitaly Kuznetsov
2021-02-09 13:15     ` Vitaly Kuznetsov
2021-03-04 18:44     ` Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 06/18] virt/mshv: create, initialize, finalize, delete partition hypercalls Nuno Das Neves
2021-02-08 19:42   ` Michael Kelley
2021-02-08 19:42     ` Michael Kelley via Virtualization
2021-03-04 23:49     ` Nuno Das Neves
2021-03-04 23:58       ` Michael Kelley
2021-03-04 23:58         ` Michael Kelley via Virtualization
2020-11-21  0:30 ` [RFC PATCH 07/18] virt/mshv: withdraw memory hypercall Nuno Das Neves
2021-02-08 19:44   ` Michael Kelley
2021-02-08 19:44     ` Michael Kelley via Virtualization
2021-03-05 21:01     ` Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 08/18] virt/mshv: map and unmap guest memory Nuno Das Neves
2021-02-08 19:45   ` Michael Kelley
2021-02-08 19:45     ` Michael Kelley via Virtualization
2021-03-08 19:14     ` Nuno Das Neves
2021-03-08 19:30       ` Michael Kelley
2021-03-08 19:30         ` Michael Kelley via Virtualization
2020-11-21  0:30 ` [RFC PATCH 09/18] virt/mshv: create vcpu ioctl Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 10/18] virt/mshv: get and set vcpu registers ioctls Nuno Das Neves
2021-02-08 19:47   ` Michael Kelley
2021-02-08 19:47     ` Michael Kelley via Virtualization
2021-03-09  1:39     ` Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 11/18] virt/mshv: set up synic pages for intercept messages Nuno Das Neves
2021-02-08 19:47   ` Michael Kelley
2021-02-08 19:47     ` Michael Kelley via Virtualization
2021-03-11 19:37     ` Nuno Das Neves
2021-03-11 20:45       ` Michael Kelley
2021-03-11 20:45         ` Michael Kelley via Virtualization
2020-11-21  0:30 ` [RFC PATCH 12/18] virt/mshv: run vp ioctl and isr Nuno Das Neves
2020-11-24 16:15   ` Wei Liu
2020-11-21  0:30 ` [RFC PATCH 13/18] virt/mshv: install intercept ioctl Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 14/18] virt/mshv: assert interrupt ioctl Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 15/18] virt/mshv: get and set vp state ioctls Nuno Das Neves
2021-02-08 19:48   ` Michael Kelley
2021-02-08 19:48     ` Michael Kelley via Virtualization
2021-03-11 23:38     ` Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 16/18] virt/mshv: mmap vp register page Nuno Das Neves
2021-02-08 19:49   ` Michael Kelley
2021-02-08 19:49     ` Michael Kelley via Virtualization
2021-03-25 17:36     ` Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 17/18] virt/mshv: get and set partition property ioctls Nuno Das Neves
2020-11-21  0:30 ` [RFC PATCH 18/18] virt/mshv: Add enlightenment bits to create partition ioctl Nuno Das Neves
2020-11-24 16:18 ` [RFC PATCH 00/18] Microsoft Hypervisor root partition ioctl interface Wei Liu
2021-02-08 19:40 ` Michael Kelley
2021-02-08 19:40   ` Michael Kelley via Virtualization

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87eeguc61d.fsf@vitty.brq.redhat.com \
    --to=vkuznets@redhat.com \
    --cc=kys@microsoft.com \
    --cc=ligrassi@microsoft.com \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikelley@microsoft.com \
    --cc=nunodasneves@linux.microsoft.com \
    --cc=sunilmut@microsoft.com \
    --cc=viremana@linux.microsoft.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=wei.liu@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.