diff mbox

[2/4] xen: limit grant v2 interface to the v1 functionality

Message ID 20170908144849.2958-3-jgross@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jürgen Groß Sept. 8, 2017, 2:48 p.m. UTC
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.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/grant-table.c | 138 ----------------------------------------------
 include/xen/grant_table.h |  25 ---------
 2 files changed, 163 deletions(-)

Comments

Boris Ostrovsky Sept. 12, 2017, 3:44 p.m. UTC | #1
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)

-boris
Jürgen Groß Sept. 12, 2017, 3:50 p.m. UTC | #2
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?


Juergen
Boris Ostrovsky Sept. 12, 2017, 4:05 p.m. UTC | #3
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.

-boris
Jürgen Groß Sept. 12, 2017, 4:09 p.m. UTC | #4
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?


Juergen
Boris Ostrovsky Sept. 12, 2017, 4:21 p.m. UTC | #5
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.

-boris
Jürgen Groß Sept. 12, 2017, 6:18 p.m. UTC | #6
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. So it won't
hit them on a random system by not working, but they would not be able
to load such a driver (same as today without V2 support).

In case they really want it they can send patches for support of subpage
or transient grants. Like they would have to do for complete V2 support
today.


Juergen
Boris Ostrovsky Sept. 13, 2017, 1:22 p.m. UTC | #7
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?

-boris

>  So it won't
> hit them on a random system by not working, but they would not be able
> to load such a driver (same as today without V2 support).
>
> In case they really want it they can send patches for support of subpage
> or transient grants. Like they would have to do for complete V2 support
> today.
>
>
> Juergen
>
Jürgen Groß Sept. 13, 2017, 1:38 p.m. UTC | #8
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).


Juergen
Boris Ostrovsky Sept. 13, 2017, 1:50 p.m. UTC | #9
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.

-boris
Jürgen Groß Sept. 13, 2017, 2:45 p.m. UTC | #10
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
Boris Ostrovsky Sept. 13, 2017, 4:20 p.m. UTC | #11
On 09/13/2017 10:45 AM, Juergen Gross wrote:
> 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.

I don't think partially supported interface is a supported interface.
It's just something that has been working until now.

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

OK. That I haven't considered that.

BTW, why are you not removing (*update_trans_entry) definition from
gnttab_ops? You are taking (*update_subpage_entry) out.


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

I never said that ;-)

-boris
Jürgen Groß Sept. 14, 2017, 8:13 a.m. UTC | #12
On 13/09/17 18:20, Boris Ostrovsky wrote:
> On 09/13/2017 10:45 AM, Juergen Gross wrote:
>> 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.
> 
> I don't think partially supported interface is a supported interface.
> It's just something that has been working until now.
> 
>> 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. 
> 
> OK. That I haven't considered that.
> 
> BTW, why are you not removing (*update_trans_entry) definition from
> gnttab_ops? You are taking (*update_subpage_entry) out.

Just for having a reason to send V2. ;-)

Thanks for catching it.


Juergen
diff mbox

Patch

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 65c4bdb0b463..02451a696cc5 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -123,17 +123,6 @@  struct gnttab_ops {
 	 */
 	int (*query_foreign_access)(grant_ref_t ref);
 	/*
-	 * Grant a domain to access a range of bytes within the page referred by
-	 * an available grant entry. Ref parameter is reference of a grant entry
-	 * which will be sub-page accessed, domid is id of grantee domain, frame
-	 * is frame address of subpage grant, flags is grant type and flag
-	 * information, page_off is offset of the range of bytes, and length is
-	 * length of bytes to be accessed.
-	 */
-	void (*update_subpage_entry)(grant_ref_t ref, domid_t domid,
-				     unsigned long frame, int flags,
-				     unsigned page_off, unsigned length);
-	/*
 	 * Redirect an available grant entry on domain A to another grant
 	 * reference of domain B, then allow domain C to use grant reference
 	 * of domain B transitively. Ref parameter is an available grant entry
@@ -292,122 +281,6 @@  int gnttab_grant_foreign_access(domid_t domid, unsigned long frame,
 }
 EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
 
-static void gnttab_update_subpage_entry_v2(grant_ref_t ref, domid_t domid,
-					   unsigned long frame, int flags,
-					   unsigned page_off, unsigned length)
-{
-	gnttab_shared.v2[ref].sub_page.frame = frame;
-	gnttab_shared.v2[ref].sub_page.page_off = page_off;
-	gnttab_shared.v2[ref].sub_page.length = length;
-	gnttab_shared.v2[ref].hdr.domid = domid;
-	wmb();
-	gnttab_shared.v2[ref].hdr.flags =
-				GTF_permit_access | GTF_sub_page | flags;
-}
-
-int gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref, domid_t domid,
-					    unsigned long frame, int flags,
-					    unsigned page_off,
-					    unsigned length)
-{
-	if (flags & (GTF_accept_transfer | GTF_reading |
-		     GTF_writing | GTF_transitive))
-		return -EPERM;
-
-	if (gnttab_interface->update_subpage_entry == NULL)
-		return -ENOSYS;
-
-	gnttab_interface->update_subpage_entry(ref, domid, frame, flags,
-					       page_off, length);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage_ref);
-
-int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long frame,
-					int flags, unsigned page_off,
-					unsigned length)
-{
-	int ref, rc;
-
-	ref = get_free_entries(1);
-	if (unlikely(ref < 0))
-		return -ENOSPC;
-
-	rc = gnttab_grant_foreign_access_subpage_ref(ref, domid, frame, flags,
-						     page_off, length);
-	if (rc < 0) {
-		put_free_entry(ref);
-		return rc;
-	}
-
-	return ref;
-}
-EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_subpage);
-
-bool gnttab_subpage_grants_available(void)
-{
-	return gnttab_interface->update_subpage_entry != NULL;
-}
-EXPORT_SYMBOL_GPL(gnttab_subpage_grants_available);
-
-static void gnttab_update_trans_entry_v2(grant_ref_t ref, domid_t domid,
-					 int flags, domid_t trans_domid,
-					 grant_ref_t trans_gref)
-{
-	gnttab_shared.v2[ref].transitive.trans_domid = trans_domid;
-	gnttab_shared.v2[ref].transitive.gref = trans_gref;
-	gnttab_shared.v2[ref].hdr.domid = domid;
-	wmb();
-	gnttab_shared.v2[ref].hdr.flags =
-				GTF_permit_access | GTF_transitive | flags;
-}
-
-int gnttab_grant_foreign_access_trans_ref(grant_ref_t ref, domid_t domid,
-					  int flags, domid_t trans_domid,
-					  grant_ref_t trans_gref)
-{
-	if (flags & (GTF_accept_transfer | GTF_reading |
-		     GTF_writing | GTF_sub_page))
-		return -EPERM;
-
-	if (gnttab_interface->update_trans_entry == NULL)
-		return -ENOSYS;
-
-	gnttab_interface->update_trans_entry(ref, domid, flags, trans_domid,
-					     trans_gref);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_trans_ref);
-
-int gnttab_grant_foreign_access_trans(domid_t domid, int flags,
-				      domid_t trans_domid,
-				      grant_ref_t trans_gref)
-{
-	int ref, rc;
-
-	ref = get_free_entries(1);
-	if (unlikely(ref < 0))
-		return -ENOSPC;
-
-	rc = gnttab_grant_foreign_access_trans_ref(ref, domid, flags,
-						   trans_domid, trans_gref);
-	if (rc < 0) {
-		put_free_entry(ref);
-		return rc;
-	}
-
-	return ref;
-}
-EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_trans);
-
-bool gnttab_trans_grants_available(void)
-{
-	return gnttab_interface->update_trans_entry != NULL;
-}
-EXPORT_SYMBOL_GPL(gnttab_trans_grants_available);
-
 static int gnttab_query_foreign_access_v1(grant_ref_t ref)
 {
 	return gnttab_shared.v1[ref].flags & (GTF_reading|GTF_writing);
@@ -1296,8 +1169,6 @@  static const struct gnttab_ops gnttab_v2_ops = {
 	.end_foreign_access_ref		= gnttab_end_foreign_access_ref_v2,
 	.end_foreign_transfer_ref	= gnttab_end_foreign_transfer_ref_v2,
 	.query_foreign_access		= gnttab_query_foreign_access_v2,
-	.update_subpage_entry		= gnttab_update_subpage_entry_v2,
-	.update_trans_entry		= gnttab_update_trans_entry_v2,
 };
 
 static void gnttab_request_version(void)
@@ -1313,15 +1184,6 @@  static void gnttab_request_version(void)
 		grefs_per_grant_frame = XEN_PAGE_SIZE /
 					sizeof(union grant_entry_v2);
 		gnttab_interface = &gnttab_v2_ops;
-	} else if (grant_table_version == 2) {
-		/*
-		 * If we've already used version 2 features,
-		 * but then suddenly discover that they're not
-		 * available (e.g. migrating to an older
-		 * version of Xen), almost unbounded badness
-		 * can happen.
-		 */
-		panic("we need grant tables version 2, but only version 1 is available");
 	} else {
 		grant_table_version = 1;
 		grefs_per_grant_frame = XEN_PAGE_SIZE /
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index dd6c7a32ee32..2e37741f6b8d 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -84,24 +84,6 @@  int gnttab_resume(void);
 
 int gnttab_grant_foreign_access(domid_t domid, unsigned long frame,
 				int readonly);
-int gnttab_grant_foreign_access_subpage(domid_t domid, unsigned long frame,
-					int flags, unsigned page_off,
-					unsigned length);
-int gnttab_grant_foreign_access_trans(domid_t domid, int flags,
-				      domid_t trans_domid,
-				      grant_ref_t trans_gref);
-
-/*
- * Are sub-page grants available on this version of Xen?  Returns true if they
- * are, and false if they're not.
- */
-bool gnttab_subpage_grants_available(void);
-
-/*
- * Are transitive grants available on this version of Xen?  Returns true if they
- * are, and false if they're not.
- */
-bool gnttab_trans_grants_available(void);
 
 /*
  * End access through the given grant reference, iff the grant entry is no
@@ -148,13 +130,6 @@  void gnttab_cancel_free_callback(struct gnttab_free_callback *callback);
 
 void gnttab_grant_foreign_access_ref(grant_ref_t ref, domid_t domid,
 				     unsigned long frame, int readonly);
-int gnttab_grant_foreign_access_subpage_ref(grant_ref_t ref, domid_t domid,
-					    unsigned long frame, int flags,
-					    unsigned page_off,
-					    unsigned length);
-int gnttab_grant_foreign_access_trans_ref(grant_ref_t ref, domid_t domid,
-					  int flags, domid_t trans_domid,
-					  grant_ref_t trans_gref);
 
 /* Give access to the first 4K of the page */
 static inline void gnttab_page_grant_foreign_access_ref_one(