* [PATCH] xen: add option to disable GNTTABOP_transfer
@ 2022-02-01 9:02 Juergen Gross
2022-02-03 9:10 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Juergen Gross @ 2022-02-01 9:02 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
The grant table operation GNTTABOP_transfer is meant to be used in
PV device backends, and it hasn't been used in Linux since the old
Xen-o-Linux days.
Add a command line sub-option to the "gnttab" option for disabling the
GNTTABOP_transfer functionality.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
docs/misc/xen-command-line.pandoc | 7 +++++--
xen/common/grant_table.c | 6 ++++++
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 6b3da6ddc1..97ddcfa523 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1167,9 +1167,9 @@ does not provide `VM_ENTRY_LOAD_GUEST_PAT`.
Specify which console gdbstub should use. See **console**.
### gnttab
-> `= List of [ max-ver:<integer>, transitive=<bool> ]`
+> `= List of [ max-ver:<integer>, transitive=<bool>, transfer=<bool> ]`
-> Default: `gnttab=max-ver:2,transitive`
+> Default: `gnttab=max-ver:2,transitive,transfer`
Control various aspects of the grant table behaviour available to guests.
@@ -1178,6 +1178,9 @@ version are 1 and 2.
* `transitive` Permit or disallow the use of transitive grants. Note that the
use of grant table v2 without transitive grants is an ABI breakage from the
guests point of view.
+* `transfer` Permit or disallow the GNTTABOP_transfer operation of the
+grant table hypercall. Note that disallowing GNTTABOP_transfer is an ABI
+breakage from the guests point of view.
The usage of gnttab v2 is not security supported on ARM platforms.
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index ed1e2fabce..d1c225e927 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -181,6 +181,7 @@ static int parse_gnttab_max_maptrack_frames(const char *arg)
unsigned int __read_mostly opt_gnttab_max_version = GNTTAB_MAX_VERSION;
static bool __read_mostly opt_transitive_grants = true;
+static bool __read_mostly opt_grant_transfer = true;
static int __init parse_gnttab(const char *s)
{
@@ -204,6 +205,8 @@ static int __init parse_gnttab(const char *s)
}
else if ( (val = parse_boolean("transitive", s, ss)) >= 0 )
opt_transitive_grants = val;
+ else if ( (val = parse_boolean("transfer", s, ss)) >= 0 )
+ opt_grant_transfer = val;
else
rc = -EINVAL;
@@ -2233,6 +2236,9 @@ gnttab_transfer(
unsigned int max_bitsize;
struct active_grant_entry *act;
+ if ( !opt_grant_transfer )
+ return -ENOSYS;
+
for ( i = 0; i < count; i++ )
{
bool_t okay;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xen: add option to disable GNTTABOP_transfer
2022-02-01 9:02 [PATCH] xen: add option to disable GNTTABOP_transfer Juergen Gross
@ 2022-02-03 9:10 ` Jan Beulich
2022-02-03 10:55 ` Juergen Gross
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2022-02-03 9:10 UTC (permalink / raw)
To: Juergen Gross
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On 01.02.2022 10:02, Juergen Gross wrote:
> The grant table operation GNTTABOP_transfer is meant to be used in
> PV device backends, and it hasn't been used in Linux since the old
> Xen-o-Linux days.
Kind of unusual spelling of XenoLinux ;-)
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -181,6 +181,7 @@ static int parse_gnttab_max_maptrack_frames(const char *arg)
>
> unsigned int __read_mostly opt_gnttab_max_version = GNTTAB_MAX_VERSION;
> static bool __read_mostly opt_transitive_grants = true;
> +static bool __read_mostly opt_grant_transfer = true;
If this was conditional upon PV (with a #define to false in the
opposite case), it could be __ro_after_init right away, while at
the same time allowing the compiler to eliminate gnttab_transfer().
> @@ -204,6 +205,8 @@ static int __init parse_gnttab(const char *s)
> }
> else if ( (val = parse_boolean("transitive", s, ss)) >= 0 )
> opt_transitive_grants = val;
> + else if ( (val = parse_boolean("transfer", s, ss)) >= 0 )
> + opt_grant_transfer = val;
> else
> rc = -EINVAL;
To possibly save a further roundtrip: If the PV dependency was added
above, I'd like to ask to follow the model of parse_iommu_param()
here and use "#ifndef opt_grant_transfer" around the added code in
favor of "#ifdef CONFIG_PV".
> @@ -2233,6 +2236,9 @@ gnttab_transfer(
> unsigned int max_bitsize;
> struct active_grant_entry *act;
>
> + if ( !opt_grant_transfer )
> + return -ENOSYS;
-EOPNOTSUPP please.
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xen: add option to disable GNTTABOP_transfer
2022-02-03 9:10 ` Jan Beulich
@ 2022-02-03 10:55 ` Juergen Gross
2022-02-03 12:04 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Juergen Gross @ 2022-02-03 10:55 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 1945 bytes --]
On 03.02.22 10:10, Jan Beulich wrote:
> On 01.02.2022 10:02, Juergen Gross wrote:
>> The grant table operation GNTTABOP_transfer is meant to be used in
>> PV device backends, and it hasn't been used in Linux since the old
>> Xen-o-Linux days.
>
> Kind of unusual spelling of XenoLinux ;-)
>
>> --- a/xen/common/grant_table.c
>> +++ b/xen/common/grant_table.c
>> @@ -181,6 +181,7 @@ static int parse_gnttab_max_maptrack_frames(const char *arg)
>>
>> unsigned int __read_mostly opt_gnttab_max_version = GNTTAB_MAX_VERSION;
>> static bool __read_mostly opt_transitive_grants = true;
>> +static bool __read_mostly opt_grant_transfer = true;
>
> If this was conditional upon PV (with a #define to false in the
> opposite case), it could be __ro_after_init right away, while at
> the same time allowing the compiler to eliminate gnttab_transfer().
Nice idea. The other option would be to put all (or most) of
gnttab_transfer() in a "#ifdef CONFIG_PV" section, allowing to
remove the "#ifdef CONFIG_X86" parts in it, too.
>
>> @@ -204,6 +205,8 @@ static int __init parse_gnttab(const char *s)
>> }
>> else if ( (val = parse_boolean("transitive", s, ss)) >= 0 )
>> opt_transitive_grants = val;
>> + else if ( (val = parse_boolean("transfer", s, ss)) >= 0 )
>> + opt_grant_transfer = val;
>> else
>> rc = -EINVAL;
>
> To possibly save a further roundtrip: If the PV dependency was added
> above, I'd like to ask to follow the model of parse_iommu_param()
> here and use "#ifndef opt_grant_transfer" around the added code in
> favor of "#ifdef CONFIG_PV".
Okay.
>
>> @@ -2233,6 +2236,9 @@ gnttab_transfer(
>> unsigned int max_bitsize;
>> struct active_grant_entry *act;
>>
>> + if ( !opt_grant_transfer )
>> + return -ENOSYS;
>
> -EOPNOTSUPP please.
Yes, that's better.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xen: add option to disable GNTTABOP_transfer
2022-02-03 10:55 ` Juergen Gross
@ 2022-02-03 12:04 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2022-02-03 12:04 UTC (permalink / raw)
To: Juergen Gross
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On 03.02.2022 11:55, Juergen Gross wrote:
> On 03.02.22 10:10, Jan Beulich wrote:
>> On 01.02.2022 10:02, Juergen Gross wrote:
>>> The grant table operation GNTTABOP_transfer is meant to be used in
>>> PV device backends, and it hasn't been used in Linux since the old
>>> Xen-o-Linux days.
>>
>> Kind of unusual spelling of XenoLinux ;-)
>>
>>> --- a/xen/common/grant_table.c
>>> +++ b/xen/common/grant_table.c
>>> @@ -181,6 +181,7 @@ static int parse_gnttab_max_maptrack_frames(const char *arg)
>>>
>>> unsigned int __read_mostly opt_gnttab_max_version = GNTTAB_MAX_VERSION;
>>> static bool __read_mostly opt_transitive_grants = true;
>>> +static bool __read_mostly opt_grant_transfer = true;
>>
>> If this was conditional upon PV (with a #define to false in the
>> opposite case), it could be __ro_after_init right away, while at
>> the same time allowing the compiler to eliminate gnttab_transfer().
>
> Nice idea. The other option would be to put all (or most) of
> gnttab_transfer() in a "#ifdef CONFIG_PV" section, allowing to
> remove the "#ifdef CONFIG_X86" parts in it, too.
Yes, sure. The downside being that then this code won't be compile-
tested anymore in !PV builds. Yet keeping code visible to compilers
is what we aim for elsewhere by preferring if(IS_ENABLED(...)) over
#ifdef where possible.
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-02-03 12:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 9:02 [PATCH] xen: add option to disable GNTTABOP_transfer Juergen Gross
2022-02-03 9:10 ` Jan Beulich
2022-02-03 10:55 ` Juergen Gross
2022-02-03 12:04 ` Jan Beulich
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.