diff mbox series

[v1,1/6] docs: add feature document for Xen hypervisor sysfs-like support

Message ID 20190927090048.28872-2-jgross@suse.com (mailing list archive)
State Superseded
Headers show
Series Add hypervisor sysfs-like support | expand

Commit Message

Jürgen Groß Sept. 27, 2019, 9 a.m. UTC
On the 2019 Xen developer summit there was agreement that the Xen
hypervisor should gain support for a hierarchical name-value store
similar to the Linux kernel's sysfs.

In the beginning there should only be basic support: entries can be
added from the hypervisor itself only, there is a simple hypercall
interface to read the data.

Add a feature document for setting the base of a discussion regarding
the desired functionality and the entries to add.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
V1:
- remove the "--" prefixes of the sub-commands of the user tool
  (Jan Beulich)
- rename xenfs to xenhypfs (Jan Beulich)
- add "tree" and "write" options to user tool
---
 docs/features/hypervisorfs.pandoc | 119 ++++++++++++++++++++++++++++++++++++++
 docs/misc/hypfs-paths.pandoc      |  55 ++++++++++++++++++
 2 files changed, 174 insertions(+)
 create mode 100644 docs/features/hypervisorfs.pandoc
 create mode 100644 docs/misc/hypfs-paths.pandoc

Comments

Ian Jackson Sept. 27, 2019, 10:37 a.m. UTC | #1
Juergen Gross writes ("[PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
> On the 2019 Xen developer summit there was agreement that the Xen
> hypervisor should gain support for a hierarchical name-value store
> similar to the Linux kernel's sysfs.
> 
> In the beginning there should only be basic support: entries can be
> added from the hypervisor itself only, there is a simple hypercall
> interface to read the data.
> 
> Add a feature document for setting the base of a discussion regarding
> the desired functionality and the entries to add.

Thanks for this work.  Like others, I approve of the basic idea.

Reading your spec document here, I think there is a key part missing:
please could you specify the allowable syntax for keys, and for
values.

I guess that keys will be chosen from some limited safe character
set ?  What about values ?  Might we create a key whose value contains
binary data ?

Depend on the answer to this question, I may want to suggest changes
or enhancements to your proposed command-line tool.

Also, your top-level document has a list of paths in it, which is
presumably prospective.  Maybe that information should be a non-parsed
header section in the paths document ?

Would it be possible to add a script to xen.git which lists the
xenhypfs and checks that all the paths are documented ?  We could add
a few calls to that to osstest...

Thanks,
Ian.
Jürgen Groß Sept. 27, 2019, 11:41 a.m. UTC | #2
On 27.09.19 12:37, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
>> On the 2019 Xen developer summit there was agreement that the Xen
>> hypervisor should gain support for a hierarchical name-value store
>> similar to the Linux kernel's sysfs.
>>
>> In the beginning there should only be basic support: entries can be
>> added from the hypervisor itself only, there is a simple hypercall
>> interface to read the data.
>>
>> Add a feature document for setting the base of a discussion regarding
>> the desired functionality and the entries to add.
> 
> Thanks for this work.  Like others, I approve of the basic idea.
> 
> Reading your spec document here, I think there is a key part missing:
> please could you specify the allowable syntax for keys, and for
> values.

Yes, that should be done.

> 
> I guess that keys will be chosen from some limited safe character
> set ?  What about values ?  Might we create a key whose value contains
> binary data ?

I'd go with "[-A-Za-z0-9@_.:()\[\]#,;]*" for keys and ASCII for values.

> Depend on the answer to this question, I may want to suggest changes
> or enhancements to your proposed command-line tool.
> 
> Also, your top-level document has a list of paths in it, which is
> presumably prospective.  Maybe that information should be a non-parsed
> header section in the paths document ?

Yes, that's better.

> Would it be possible to add a script to xen.git which lists the
> xenhypfs and checks that all the paths are documented ?  We could add
> a few calls to that to osstest...

I'd expect some parts to be described rather generically (as can be seen
in patch 6 for the runtime parameters). Maybe I should add the entries
with wildcards there.

OTOH adding such a script can easily be done later, maybe with some
tweaks to the path documentation.


Juergen
Ian Jackson Sept. 27, 2019, 1:37 p.m. UTC | #3
Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
> On 27.09.19 12:37, Ian Jackson wrote:
> > I guess that keys will be chosen from some limited safe character
> > set ?  What about values ?  Might we create a key whose value contains
> > binary data ?
> 
> I'd go with "[-A-Za-z0-9@_.:()\[\]#,;]*" for keys

I think this is ASCII printing characters with the exception of
  ! " ` $ % ^ & * = + { } ' ~ < > / \ |

I struggle to find a principled explanation for this particular
exclusion set (apart from /), considering that following are
included:
  - @ _ . : ( ) [ ] # , ;

Could we borrow some existing permitted character set ?  If we are
permitting shell metacharacters why not just permit all printable
ASCII except / ?

> and ASCII for values.

Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
inclusive, or something else ?

> > Would it be possible to add a script to xen.git which lists the
> > xenhypfs and checks that all the paths are documented ?  We could add
> > a few calls to that to osstest...
> 
> I'd expect some parts to be described rather generically (as can be seen
> in patch 6 for the runtime parameters). Maybe I should add the entries
> with wildcards there.

That would be nice.

> OTOH adding such a script can easily be done later, maybe with some
> tweaks to the path documentation.

Sure.  Having at least some unit tests ought to be on the roadmap for
supported status, but it doesn't need to happen now.

Ian.
Jürgen Groß Sept. 27, 2019, 2:58 p.m. UTC | #4
On 27.09.19 15:37, Ian Jackson wrote:
> Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
>> On 27.09.19 12:37, Ian Jackson wrote:
>>> I guess that keys will be chosen from some limited safe character
>>> set ?  What about values ?  Might we create a key whose value contains
>>> binary data ?
>>
>> I'd go with "[-A-Za-z0-9@_.:()\[\]#,;]*" for keys
> 
> I think this is ASCII printing characters with the exception of
>    ! " ` $ % ^ & * = + { } ' ~ < > / \ |
> 
> I struggle to find a principled explanation for this particular
> exclusion set (apart from /), considering that following are
> included:
>    - @ _ . : ( ) [ ] # , ;
> 
> Could we borrow some existing permitted character set ?  If we are
> permitting shell metacharacters why not just permit all printable
> ASCII except / ?

Hmm, maybe we should allow just the "Posix portable file name character
set"? That would be [-._0-9A-Za-z]. And we should explicitly not allow
the key names "." and "..".

> 
>> and ASCII for values.
> 
> Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
> inclusive, or something else ?

:-)

As I'd like to support e.g. the .config file contents of the hypervisor
build I guess I need (0x01-0xff) inclusive, right?

> 
>>> Would it be possible to add a script to xen.git which lists the
>>> xenhypfs and checks that all the paths are documented ?  We could add
>>> a few calls to that to osstest...
>>
>> I'd expect some parts to be described rather generically (as can be seen
>> in patch 6 for the runtime parameters). Maybe I should add the entries
>> with wildcards there.
> 
> That would be nice.

Okay, will do.


Juergen
Ian Jackson Sept. 27, 2019, 3:19 p.m. UTC | #5
Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
> On 27.09.19 15:37, Ian Jackson wrote:
> > I think this is ASCII printing characters with the exception of
> >    ! " ` $ % ^ & * = + { } ' ~ < > / \ |
> > 
> > I struggle to find a principled explanation for this particular
> > exclusion set (apart from /), considering that following are
> > included:
> >    - @ _ . : ( ) [ ] # , ;
> > 
> > Could we borrow some existing permitted character set ?  If we are
> > permitting shell metacharacters why not just permit all printable
> > ASCII except / ?
> 
> Hmm, maybe we should allow just the "Posix portable file name character
> set"? That would be [-._0-9A-Za-z]. And we should explicitly not allow
> the key names "." and "..".

I agree about "." and "..".

I'm not sure the "Posix portable file name character set" is a very
good guide.  Posix will be pretty restricted there.  What is the legal
set in a Linux sysfs filename ?

> > Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
> > inclusive, or something else ?
> 
> :-)
> 
> As I'd like to support e.g. the .config file contents of the hypervisor
> build I guess I need (0x01-0xff) inclusive, right?

The .config file I have here does not seem to contain any control
characters.  If it did surely that would be a bug?  If you want this
you obviously do need to permit newlines and spaces and perhaps tabs
too.

Ian.
Jürgen Groß Sept. 30, 2019, 8:17 a.m. UTC | #6
On 27.09.19 17:19, Ian Jackson wrote:
> Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
>> On 27.09.19 15:37, Ian Jackson wrote:
>>> I think this is ASCII printing characters with the exception of
>>>     ! " ` $ % ^ & * = + { } ' ~ < > / \ |
>>>
>>> I struggle to find a principled explanation for this particular
>>> exclusion set (apart from /), considering that following are
>>> included:
>>>     - @ _ . : ( ) [ ] # , ;
>>>
>>> Could we borrow some existing permitted character set ?  If we are
>>> permitting shell metacharacters why not just permit all printable
>>> ASCII except / ?
>>
>> Hmm, maybe we should allow just the "Posix portable file name character
>> set"? That would be [-._0-9A-Za-z]. And we should explicitly not allow
>> the key names "." and "..".
> 
> I agree about "." and "..".
> 
> I'm not sure the "Posix portable file name character set" is a very
> good guide.  Posix will be pretty restricted there.  What is the legal
> set in a Linux sysfs filename ?

Everything but "/" and "\0".

> 
>>> Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
>>> inclusive, or something else ?
>>
>> :-)
>>
>> As I'd like to support e.g. the .config file contents of the hypervisor
>> build I guess I need (0x01-0xff) inclusive, right?
> 
> The .config file I have here does not seem to contain any control
> characters.  If it did surely that would be a bug?  If you want this
> you obviously do need to permit newlines and spaces and perhaps tabs
> too.

.config can contain user supplied strings. While not making much sense
to have unprintable characters in there I don't see how to prevent them.
And I guess we'd want to see them in case such a weird .config was used
(and probably resulted in strange behavior).


Juergen
Jan Beulich Sept. 30, 2019, 8:57 a.m. UTC | #7
On 30.09.2019 10:17, Jürgen Groß wrote:
> On 27.09.19 17:19, Ian Jackson wrote:
>> Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
>>> On 27.09.19 15:37, Ian Jackson wrote:
>>>> I think this is ASCII printing characters with the exception of
>>>>     ! " ` $ % ^ & * = + { } ' ~ < > / \ |
>>>>
>>>> I struggle to find a principled explanation for this particular
>>>> exclusion set (apart from /), considering that following are
>>>> included:
>>>>     - @ _ . : ( ) [ ] # , ;
>>>>
>>>> Could we borrow some existing permitted character set ?  If we are
>>>> permitting shell metacharacters why not just permit all printable
>>>> ASCII except / ?
>>>
>>> Hmm, maybe we should allow just the "Posix portable file name character
>>> set"? That would be [-._0-9A-Za-z]. And we should explicitly not allow
>>> the key names "." and "..".
>>
>> I agree about "." and "..".
>>
>> I'm not sure the "Posix portable file name character set" is a very
>> good guide.  Posix will be pretty restricted there.  What is the legal
>> set in a Linux sysfs filename ?
> 
> Everything but "/" and "\0".
> 
>>
>>>> Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
>>>> inclusive, or something else ?
>>>
>>> :-)
>>>
>>> As I'd like to support e.g. the .config file contents of the hypervisor
>>> build I guess I need (0x01-0xff) inclusive, right?
>>
>> The .config file I have here does not seem to contain any control
>> characters.  If it did surely that would be a bug?  If you want this
>> you obviously do need to permit newlines and spaces and perhaps tabs
>> too.
> 
> .config can contain user supplied strings. While not making much sense
> to have unprintable characters in there I don't see how to prevent them.

Does / can't kconfig reject them?

Jan
Jürgen Groß Sept. 30, 2019, 9:09 a.m. UTC | #8
On 30.09.19 10:57, Jan Beulich wrote:
> On 30.09.2019 10:17, Jürgen Groß wrote:
>> On 27.09.19 17:19, Ian Jackson wrote:
>>> Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
>>>> On 27.09.19 15:37, Ian Jackson wrote:
>>>>> I think this is ASCII printing characters with the exception of
>>>>>      ! " ` $ % ^ & * = + { } ' ~ < > / \ |
>>>>>
>>>>> I struggle to find a principled explanation for this particular
>>>>> exclusion set (apart from /), considering that following are
>>>>> included:
>>>>>      - @ _ . : ( ) [ ] # , ;
>>>>>
>>>>> Could we borrow some existing permitted character set ?  If we are
>>>>> permitting shell metacharacters why not just permit all printable
>>>>> ASCII except / ?
>>>>
>>>> Hmm, maybe we should allow just the "Posix portable file name character
>>>> set"? That would be [-._0-9A-Za-z]. And we should explicitly not allow
>>>> the key names "." and "..".
>>>
>>> I agree about "." and "..".
>>>
>>> I'm not sure the "Posix portable file name character set" is a very
>>> good guide.  Posix will be pretty restricted there.  What is the legal
>>> set in a Linux sysfs filename ?
>>
>> Everything but "/" and "\0".
>>
>>>
>>>>> Do you mean "any 7-bit byte", or octet values 32-126 (0x20-0x7e)
>>>>> inclusive, or something else ?
>>>>
>>>> :-)
>>>>
>>>> As I'd like to support e.g. the .config file contents of the hypervisor
>>>> build I guess I need (0x01-0xff) inclusive, right?
>>>
>>> The .config file I have here does not seem to contain any control
>>> characters.  If it did surely that would be a bug?  If you want this
>>> you obviously do need to permit newlines and spaces and perhaps tabs
>>> too.
>>
>> .config can contain user supplied strings. While not making much sense
>> to have unprintable characters in there I don't see how to prevent them.
> 
> Does / can't kconfig reject them?

Right now it doesn't.

I tested that by manually modifying my .config and then doing a make.
The (wrong) .config was accepted and the hypervisor built with it.


Juergen
Ian Jackson Sept. 30, 2019, 10:07 a.m. UTC | #9
Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
> On 30.09.19 10:57, Jan Beulich wrote:
> > On 30.09.2019 10:17, Jürgen Groß wrote:
> >> .config can contain user supplied strings. While not making much sense
> >> to have unprintable characters in there I don't see how to prevent them.
> > 
> > Does / can't kconfig reject them?
> 
> Right now it doesn't.
> 
> I tested that by manually modifying my .config and then doing a make.
> The (wrong) .config was accepted and the hypervisor built with it.

Urgh.

I think that either

(A) this needs to be prevented somehow, or

(B) your userland tools, at the very least, need souping up so that
they (i) by default do not risk spamming the user's terminal with
binary gibberish which someone stored in xenhypfs (ii) can be used to
reliably extract any binary objects which have been stored.

I think (B)(i) necessarily involves some kind of quoting scheme :-/.
(B)(ii) means the quoting needs to be turn-off-able.

Ian.
Jürgen Groß Sept. 30, 2019, 10:16 a.m. UTC | #10
On 30.09.19 12:07, Ian Jackson wrote:
> Jürgen Groß writes ("Re: [PATCH v1 1/6] docs: add feature document for Xen hypervisor sysfs-like support"):
>> On 30.09.19 10:57, Jan Beulich wrote:
>>> On 30.09.2019 10:17, Jürgen Groß wrote:
>>>> .config can contain user supplied strings. While not making much sense
>>>> to have unprintable characters in there I don't see how to prevent them.
>>>
>>> Does / can't kconfig reject them?
>>
>> Right now it doesn't.
>>
>> I tested that by manually modifying my .config and then doing a make.
>> The (wrong) .config was accepted and the hypervisor built with it.
> 
> Urgh.
> 
> I think that either
> 
> (A) this needs to be prevented somehow, or

Difficult, as we inherit kconfig from the Linux kernel.

> (B) your userland tools, at the very least, need souping up so that
> they (i) by default do not risk spamming the user's terminal with
> binary gibberish which someone stored in xenhypfs (ii) can be used to
> reliably extract any binary objects which have been stored.
> 
> I think (B)(i) necessarily involves some kind of quoting scheme :-/.
> (B)(ii) means the quoting needs to be turn-off-able.

Yes, this seems to be a sensible thing to do.


Juergen
diff mbox series

Patch

diff --git a/docs/features/hypervisorfs.pandoc b/docs/features/hypervisorfs.pandoc
new file mode 100644
index 0000000000..3155cee7d3
--- /dev/null
+++ b/docs/features/hypervisorfs.pandoc
@@ -0,0 +1,119 @@ 
+% Hypervisor FS
+% Revision 1
+
+\clearpage
+
+# Basics
+---------------- ---------------------
+         Status: e.g. **Supported**
+
+  Architectures: all
+
+     Components: Hypervisor, toolstack
+---------------- ---------------------
+
+# Overview
+
+The Hypervisor FS is a hierarchical name-value store for reporting
+information to guests, especially dom0.  It is similar to the Linux
+kernel's sysfs, but without the functionality to directly alter
+entries values. Entries and directories are created by the hypervisor,
+while the toolstack is able to use a hypercall to query the entry
+values.
+
+# User details
+
+With:
+
+    xenhypfs ls <path>
+
+the user can list the entries of a specific path of the FS. Using:
+
+    xenhypfs cat <path>
+
+the content of an entry can be retrieved. Using:
+
+    xenhypfs write <path> <string>
+
+a writable entry can be modified. With:
+
+    xenhypfs tree
+
+the complete Hypervisor FS entry tree can be printed.
+
+The FS structure is:
+
+    /
+        buildinfo/           directory containing build-time data
+            config           contents of .config file used to build Xen
+        cpu-bugs/            x86: directory of cpu bug information
+            l1tf             "Vulnerable" or "Not vulnerable"
+            mds              "Vulnerable" or "Not vulnerable"
+            meltdown         "Vulnerable" or "Not vulnerable"
+            spec-store-bypass "Vulnerable" or "Not vulnerable"
+            spectre-v1       "Vulnerable" or "Not vulnerable"
+            spectre-v2       "Vulnerable" or "Not vulnerable"
+            mitigations/     directory of mitigation settings
+                bti-thunk    "N/A", "RETPOLINE", "LFENCE" or "JMP"
+                spec-ctrl    "No", "IBRS+" or IBRS-"
+                ibpb         "No" or "Yes"
+                l1d-flush    "No" or "Yes"
+                md-clear     "No" or "VERW"
+                l1tf-barrier "No" or "Yes"
+            active-hvm/      directory for mitigations active in hvm doamins
+                msr-spec-ctrl "No" or "Yes"
+                rsb          "No" or "Yes"
+                eager-fpu    "No" or "Yes"
+                md-clear     "No" or "Yes"
+            active-pv/       directory for mitigations active in pv doamins
+                msr-spec-ctrl "No" or "Yes"
+                rsb          "No" or "Yes"
+                eager-fpu    "No" or "Yes"
+                md-clear     "No" or "Yes"
+                xpti         "No" or list of "dom0", "domU", "PCID on"
+                l1tf-shadow  "No" or list of "dom0", "domU"
+        parameters/          directory with hypervisor parameter values
+                             (boot/runtime parameters)
+
+# Technical details
+
+Access to the hypervisor filesystem is done via the stable new hypercall
+__HYPERVISOR_filesystem_op.
+
+* hypercall interface specification
+    * `xen/include/public/filesystem.h`
+* hypervisor internal files
+    * `xen/include/xen/filesystem.h`
+    * `xen/common/filesystem.c`
+* `libxenhypfs`
+    * `tools/libs/libxenhypfs/*`
+* `xenhypfs`
+    * `tools/misc/xenhypfs.c`
+* path documentation
+    * `docs/misc/hypfs-paths.pandoc`
+ 
+# Testing
+
+Any new parameters or hardware mitigations should be verified to show up
+correctly in the filesystem.
+
+# Areas for improvement
+
+* More detailed access rights
+* Entries per domain and/or per cpupool
+
+# Known issues
+
+* None
+
+# References
+
+* None
+
+# History
+
+------------------------------------------------------------------------
+Date       Revision Version  Notes
+---------- -------- -------- -------------------------------------------
+2019-09-18 1        Xen 4.13 Document written
+---------- -------- -------- -------------------------------------------
diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
new file mode 100644
index 0000000000..2fe5455e6f
--- /dev/null
+++ b/docs/misc/hypfs-paths.pandoc
@@ -0,0 +1,55 @@ 
+# Xenhypfs Paths
+
+This document attempts to define all the paths which are available
+in the Xen hypervisor file system (hypfs).
+
+The hypervisor file system can be accessed via the xenhypfs tool.
+
+## Notation
+
+The hypervisor file system is similar to the Linux kernel's sysfs.
+In this document directories are always specified with a trailing "/".
+
+The following notation conventions apply:
+
+        DIRECTORY/
+
+        PATH = VALUES [TAGS]
+
+The first syntax defines a directory. It normally contains related
+entries and the general scope of the directory is described.
+
+The second syntax defines a file entry containing values which are
+either set by the hypervisor or, if the file is writable, can be set
+by the user.
+
+PATH can contain simple regex constructs following the Perl compatible
+regexp syntax described in pcre(3) or perlre(1).
+
+VALUES are strings and can take the following forms:
+
+* STRING -- an arbitrary string.
+* INTEGER -- An integer, in decimal representation unless otherwise
+  noted.
+* "a literal string" -- literal strings are contained within quotes.
+* (VALUE | VALUE | ... ) -- a set of alternatives. Alternatives are
+  separated by a "|" and all the alternatives are enclosed in "(" and
+  ")".
+
+Additional TAGS may follow as a comma separated set of the following
+tags enclosed in square brackets.
+
+* w -- Path is writable by the user. This capability is usually
+  limited to the control domain (e.g. dom0).
+* ARM | ARM32 | X86: the path is available for the respective architecture
+  only.
+* PV --  Path is valid for PV capable hypervisors only.
+* HVM -- Path is valid for HVM capable hypervisors only.
+* CONFIG_* -- Path is valid only in case the hypervisor was built with
+  the respective config option.
+
+## General Paths
+
+#### /
+
+The root of the hypervisor file system.