Subject: Re: [PATCH 2/4] xen: limit grant v2 interface to the v1 functionality
To: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org
References: <20170908144849.2958-1-jgross@suse.com>
 <20170908144849.2958-3-jgross@suse.com>
 <f9cc6652-c524-bf4f-73f0-494d16deedf0@oracle.com>
 <8c87dc06-bbc8-5279-d135-d3351032913d@suse.com>
 <2777c0f8-7555-fbf3-6e18-81cea2fd1a39@oracle.com>
 <7dd99241-f771-9fa2-8a6a-ce019dbe08b6@suse.com>
 <46a17191-916a-e039-7e2f-a197e6783fe4@oracle.com>
 <258bc8dd-c475-bd5d-dffc-8c77fbe3cca3@suse.com>
 <9e8e497a-49d1-6299-8101-dd1123f81fee@oracle.com>
 <ccea56f5-bfbe-6003-f7bd-d2477831b9e3@suse.com>
 <0ea7c145-9a86-f002-75d0-bc6b8be72867@oracle.com>
From: Juergen Gross <jgross@suse.com>
Message-ID: <ebdfb0b3-599e-e92c-87e3-09fff2d7b523@suse.com>
Date: Wed, 13 Sep 2017 16:45:50 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <0ea7c145-9a86-f002-75d0-bc6b8be72867@oracle.com>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3446
Lines: 74

On 13/09/17 15:50, Boris Ostrovsky wrote:
> On 09/13/2017 09:38 AM, Juergen Gross wrote:
>> On 13/09/17 15:22, Boris Ostrovsky wrote:
>>> On 09/12/2017 02:18 PM, Juergen Gross wrote:
>>>> On 12/09/17 18:21, Boris Ostrovsky wrote:
>>>>> On 09/12/2017 12:09 PM, Juergen Gross wrote:
>>>>>> On 12/09/17 18:05, Boris Ostrovsky wrote:
>>>>>>> On 09/12/2017 11:50 AM, Juergen Gross wrote:
>>>>>>>> On 12/09/17 17:44, Boris Ostrovsky wrote:
>>>>>>>>> On 09/08/2017 10:48 AM, Juergen Gross wrote:
>>>>>>>>>> As there is currently no user for sub-page grants or transient grants
>>>>>>>>>> remove that functionality. This at once makes it possible to switch
>>>>>>>>>> from grant v2 to grant v1 without restrictions, as there is no loss of
>>>>>>>>>> functionality other than the limited frame number width related to
>>>>>>>>>> the switch.
>>>>>>>>> But isn't that ABI violation? v2 is expected to support this (XSAs
>>>>>>>>> notwithstanding)
>>>>>>>> No, I don't think so.
>>>>>>>>
>>>>>>>> The hypervisor still supports it, but the domU (or dom0) isn't required
>>>>>>>> to make use of all the features IMHO. Or are you aware of any backend
>>>>>>>> querying the grant version of a frontend and acting in another way if v2
>>>>>>>> is detected?
>>>>>>> I am not aware of any but that doesn't mean that they don't (or won't)
>>>>>>> exist.
>>>>>> But isn't the frontend the one which is defining what is granted in
>>>>>> which way? How should there be an ABI breakage when the frontend just
>>>>>> isn't using sub-page or transitive grants?
>>>>> People may provide both front and backend drivers and frontends, knowing
>>>>> that v2 is available, could decide to use those features.
>>>> No, without the functions to use them it will be impossible.
>>> I don't follow this. Which functions? The ones this patch is removing?
>> Yes, just after having been added one patch earlier.
>>
>> Right now the Linux kernel doesn't support grant V2 at all. So there
>> surely is no driver making use of V2 features right now.
>>
>> Ican merge patches 1 and 2 in case you want. I just thought a pure
>> revert of the former V2 remove patch would be easier to review,
>> taking into account that the former V2 support was working in
>> production environments (and even back then there was no user of
>> sub-page or transient grants).
> 
> No, I don't have problems with *how* you are doing this (revert fully
> first and then remove).
> 
> I am just not sure that removing these functions is the way to go
> because we are ending up with partial implementation of v2. The fact
> that noone is/has been using these features is IMO not particularly
> relevant.
> 
> If these two were optional features then it would have been reasonable
> to drop them.

Why does the kernel need to support all features of an interface?

I'm quite sure there are lots of interfaces supported only partially in
the kernel.

Having support for functionality in the kernel not being used at all is
just adding dead code with a high potential to bitrot. I can't even test
this functionality right now, as there are no users of it. So I'd risk
adding something which is broken from the beginning. So currently my V2
support is regarding the exported kernel interfaces the same as V1, but
without the limitation to 32 bit frame numbers.

And looking at

https://lists.xen.org/archives/html/xen-devel/2017-08/msg03194.html

I believe my partial V2 support isn't the worst idea.


Juergen