2021-04-22 15:13:27

by Jürgen Groß

[permalink] [raw]
Subject: [PATCH 0/3] xen: remove some checks for always present Xen features

Some features of Xen can be assumed to be always present, so add a
central check to verify this being true and remove the other checks.

Juergen Gross (3):
xen: check required Xen features
xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests

arch/x86/xen/enlighten_pv.c | 12 ++----------
arch/x86/xen/mmu_pv.c | 4 ++--
drivers/xen/features.c | 18 ++++++++++++++++++
drivers/xen/gntdev.c | 36 ++----------------------------------
4 files changed, 24 insertions(+), 46 deletions(-)

--
2.26.2


2021-04-22 15:18:06

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH 0/3] xen: remove some checks for always present Xen features

On 22.04.2021 17:10, Juergen Gross wrote:
> Some features of Xen can be assumed to be always present, so add a
> central check to verify this being true and remove the other checks.
>
> Juergen Gross (3):
> xen: check required Xen features
> xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
> xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests

I wonder whether it's a good idea to infer feature presence from
version numbers. If (at some point in the past) you had inferred
gnttab v2 being available by version, this would have been broken
by its availability becoming controllable by a command line option
in Xen.

Jan

2021-04-22 15:18:39

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 0/3] xen: remove some checks for always present Xen features

On 22.04.21 17:16, Jan Beulich wrote:
> On 22.04.2021 17:10, Juergen Gross wrote:
>> Some features of Xen can be assumed to be always present, so add a
>> central check to verify this being true and remove the other checks.
>>
>> Juergen Gross (3):
>> xen: check required Xen features
>> xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
>> xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests
>
> I wonder whether it's a good idea to infer feature presence from
> version numbers. If (at some point in the past) you had inferred
> gnttab v2 being available by version, this would have been broken
> by its availability becoming controllable by a command line option
> in Xen.

I'm testing the feature to be really present when booting and issue a
message if it is not there.

Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.06 kB)
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments

2021-04-22 15:25:19

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH 0/3] xen: remove some checks for always present Xen features

On 22.04.2021 17:17, Juergen Gross wrote:
> On 22.04.21 17:16, Jan Beulich wrote:
>> On 22.04.2021 17:10, Juergen Gross wrote:
>>> Some features of Xen can be assumed to be always present, so add a
>>> central check to verify this being true and remove the other checks.
>>>
>>> Juergen Gross (3):
>>> xen: check required Xen features
>>> xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
>>> xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests
>>
>> I wonder whether it's a good idea to infer feature presence from
>> version numbers. If (at some point in the past) you had inferred
>> gnttab v2 being available by version, this would have been broken
>> by its availability becoming controllable by a command line option
>> in Xen.
>
> I'm testing the feature to be really present when booting and issue a
> message if it is not there.

And how does this help if the feature really isn't there yet other code
assumes it is?

Jan

2021-04-22 15:31:19

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 0/3] xen: remove some checks for always present Xen features

On 22.04.21 17:23, Jan Beulich wrote:
> On 22.04.2021 17:17, Juergen Gross wrote:
>> On 22.04.21 17:16, Jan Beulich wrote:
>>> On 22.04.2021 17:10, Juergen Gross wrote:
>>>> Some features of Xen can be assumed to be always present, so add a
>>>> central check to verify this being true and remove the other checks.
>>>>
>>>> Juergen Gross (3):
>>>> xen: check required Xen features
>>>> xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
>>>> xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests
>>>
>>> I wonder whether it's a good idea to infer feature presence from
>>> version numbers. If (at some point in the past) you had inferred
>>> gnttab v2 being available by version, this would have been broken
>>> by its availability becoming controllable by a command line option
>>> in Xen.
>>
>> I'm testing the feature to be really present when booting and issue a
>> message if it is not there.
>
> And how does this help if the feature really isn't there yet other code
> assumes it is?

Did you look at the features I'm testing? Those are really just low
level additions I can't imagine will ever be removed again.


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.06 kB)
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments

2021-04-22 15:44:32

by Jan Beulich

[permalink] [raw]
Subject: Re: [PATCH 0/3] xen: remove some checks for always present Xen features

On 22.04.2021 17:28, Juergen Gross wrote:
> On 22.04.21 17:23, Jan Beulich wrote:
>> On 22.04.2021 17:17, Juergen Gross wrote:
>>> On 22.04.21 17:16, Jan Beulich wrote:
>>>> On 22.04.2021 17:10, Juergen Gross wrote:
>>>>> Some features of Xen can be assumed to be always present, so add a
>>>>> central check to verify this being true and remove the other checks.
>>>>>
>>>>> Juergen Gross (3):
>>>>> xen: check required Xen features
>>>>> xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
>>>>> xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests
>>>>
>>>> I wonder whether it's a good idea to infer feature presence from
>>>> version numbers. If (at some point in the past) you had inferred
>>>> gnttab v2 being available by version, this would have been broken
>>>> by its availability becoming controllable by a command line option
>>>> in Xen.
>>>
>>> I'm testing the feature to be really present when booting and issue a
>>> message if it is not there.
>>
>> And how does this help if the feature really isn't there yet other code
>> assumes it is?
>
> Did you look at the features I'm testing?

I did, yes.

> Those are really just low
> level additions I can't imagine will ever be removed again.

I don't expect them to be removed. But I don't think the people having
contributed gnttab v2 expected any such for it, either.

Jan

2021-04-22 15:53:27

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 0/3] xen: remove some checks for always present Xen features

On 22.04.21 17:42, Jan Beulich wrote:
> On 22.04.2021 17:28, Juergen Gross wrote:
>> On 22.04.21 17:23, Jan Beulich wrote:
>>> On 22.04.2021 17:17, Juergen Gross wrote:
>>>> On 22.04.21 17:16, Jan Beulich wrote:
>>>>> On 22.04.2021 17:10, Juergen Gross wrote:
>>>>>> Some features of Xen can be assumed to be always present, so add a
>>>>>> central check to verify this being true and remove the other checks.
>>>>>>
>>>>>> Juergen Gross (3):
>>>>>> xen: check required Xen features
>>>>>> xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
>>>>>> xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests
>>>>>
>>>>> I wonder whether it's a good idea to infer feature presence from
>>>>> version numbers. If (at some point in the past) you had inferred
>>>>> gnttab v2 being available by version, this would have been broken
>>>>> by its availability becoming controllable by a command line option
>>>>> in Xen.
>>>>
>>>> I'm testing the feature to be really present when booting and issue a
>>>> message if it is not there.
>>>
>>> And how does this help if the feature really isn't there yet other code
>>> assumes it is?
>>
>> Did you look at the features I'm testing?
>
> I did, yes.
>
>> Those are really just low
>> level additions I can't imagine will ever be removed again.
>
> I don't expect them to be removed. But I don't think the people having
> contributed gnttab v2 expected any such for it, either.

There is a major difference here.

gnttab v2 was replacing an existing functionality by a more scalable,
but more complex solution.

The features I'm assuming to be present are basically repairing issues
which have been present due to omissions in the initial implementation.

Especially the XENFEAT_gnttab_map_avail_bits causes a racy workaround in
the kernel when not present. The race is only avoided in case the user
code is well-behaved. It is dom0 user code, yes, but nevertheless such
issues are never nice.


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.06 kB)
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments

2021-05-10 07:35:50

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 0/3] xen: remove some checks for always present Xen features

On 22.04.21 17:10, Juergen Gross wrote:
> Some features of Xen can be assumed to be always present, so add a
> central check to verify this being true and remove the other checks.
>
> Juergen Gross (3):
> xen: check required Xen features
> xen: assume XENFEAT_mmu_pt_update_preserve_ad being set for pv guests
> xen: assume XENFEAT_gnttab_map_avail_bits being set for pv guests
>
> arch/x86/xen/enlighten_pv.c | 12 ++----------
> arch/x86/xen/mmu_pv.c | 4 ++--
> drivers/xen/features.c | 18 ++++++++++++++++++
> drivers/xen/gntdev.c | 36 ++----------------------------------
> 4 files changed, 24 insertions(+), 46 deletions(-)
>

Could I please get some feedback on this series?


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.06 kB)
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments