* [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support
@ 2020-02-19 8:11 Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 1/8] xen: add a generic way to include binary files as variables Juergen Gross
` (7 more replies)
0 siblings, 8 replies; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Kevin Tian, Stefano Stabellini, Julien Grall,
Jun Nakajima, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap,
Andrew Cooper, Ian Jackson, Jan Beulich, Daniel De Graaf,
Volodymyr Babchuk, Roger Pau Monné
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.
This is a first implementation of that idea adding the basic
functionality to hypervisor and tools side. The interface to any
user program making use of that "xen-hypfs" is a new library
"libxenhypfs" with a stable interface.
As a followup switching libxl to use the hypervisor file system for
runtime parameter modifications is possible. This would enable
removal of the XEN_SYSCTL_set_parameter sysctl operation.
Changes in V5:
- switched to xsm for privilege check
Changes in V4:
- former patch 2 removed as already committed
- addressed review comments
Changes in V3:
- major rework, especially by supporting binary contents of entries
- added several new patches (1, 2, 7)
- full support of all runtime parameters
- support of writing entries (especially runtime parameters)
Changes in V2:
- all comments to V1 addressed
- added man-page for xenhypfs tool
- added runtime parameter read access for string parameters
Changes in V1:
- renamed xenfs ->xenhypfs
- added writable entries support at the interface level and in the
xenhypfs tool
- added runtime parameter read access (integer type only for now)
- added docs/misc/hypfs-paths.pandoc for path descriptions
Juergen Gross (8):
xen: add a generic way to include binary files as variables
docs: add feature document for Xen hypervisor sysfs-like support
xen: add basic hypervisor filesystem support
libs: add libxenhypfs
tools: add xenfs tool
xen: provide version information in hypfs
xen: add /buildinfo/config entry to hypervisor filesystem
xen: add runtime parameter access support to hypfs
.gitignore | 6 +
docs/features/hypervisorfs.pandoc | 92 +++++++
docs/man/xenhypfs.1.pod | 61 +++++
docs/misc/hypfs-paths.pandoc | 163 +++++++++++
tools/Rules.mk | 6 +
tools/flask/policy/modules/dom0.te | 2 +-
tools/libs/Makefile | 1 +
tools/libs/hypfs/Makefile | 16 ++
tools/libs/hypfs/core.c | 533 ++++++++++++++++++++++++++++++++++++
tools/libs/hypfs/include/xenhypfs.h | 75 +++++
tools/libs/hypfs/libxenhypfs.map | 10 +
tools/libs/hypfs/xenhypfs.pc.in | 10 +
tools/misc/Makefile | 6 +
tools/misc/xenhypfs.c | 189 +++++++++++++
xen/arch/arm/traps.c | 1 +
xen/arch/arm/xen.lds.S | 5 +
xen/arch/x86/hvm/hypercall.c | 1 +
xen/arch/x86/hvm/vmx/vmcs.c | 17 +-
xen/arch/x86/hypercall.c | 1 +
xen/arch/x86/pv/domain.c | 7 +-
xen/arch/x86/pv/hypercall.c | 1 +
xen/arch/x86/xen.lds.S | 5 +
xen/common/Makefile | 13 +
xen/common/grant_table.c | 17 +-
xen/common/hypfs.c | 395 ++++++++++++++++++++++++++
xen/common/kernel.c | 78 +++++-
xen/drivers/char/console.c | 49 +++-
xen/include/public/hypfs.h | 127 +++++++++
xen/include/public/xen.h | 1 +
xen/include/xen/hypercall.h | 8 +
xen/include/xen/hypfs.h | 112 ++++++++
xen/include/xen/kernel.h | 3 +
xen/include/xen/param.h | 74 ++++-
xen/include/xlat.lst | 2 +
xen/include/xsm/dummy.h | 6 +
xen/include/xsm/xsm.h | 6 +
xen/tools/binfile | 41 +++
xen/xsm/flask/Makefile | 5 +-
xen/xsm/flask/flask-policy.S | 16 --
xen/xsm/flask/policy/access_vectors | 2 +
40 files changed, 2123 insertions(+), 40 deletions(-)
create mode 100644 docs/features/hypervisorfs.pandoc
create mode 100644 docs/man/xenhypfs.1.pod
create mode 100644 docs/misc/hypfs-paths.pandoc
create mode 100644 tools/libs/hypfs/Makefile
create mode 100644 tools/libs/hypfs/core.c
create mode 100644 tools/libs/hypfs/include/xenhypfs.h
create mode 100644 tools/libs/hypfs/libxenhypfs.map
create mode 100644 tools/libs/hypfs/xenhypfs.pc.in
create mode 100644 tools/misc/xenhypfs.c
create mode 100644 xen/common/hypfs.c
create mode 100644 xen/include/public/hypfs.h
create mode 100644 xen/include/xen/hypfs.h
create mode 100755 xen/tools/binfile
delete mode 100644 xen/xsm/flask/flask-policy.S
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 1/8] xen: add a generic way to include binary files as variables
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 12:05 ` Wei Liu
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 2/8] docs: add feature document for Xen hypervisor sysfs-like support Juergen Gross
` (6 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jan Beulich, Daniel De Graaf
Add a new script xen/tools/binfile for including a binary file at build
time being usable via a pointer and a size variable in the hypervisor.
Make use of that generic tool in xsm.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V3:
- new patch
V4:
- add alignment parameter (Jan Beulich)
- use .Lend instead of . (Jan Beulich)
---
.gitignore | 1 +
xen/tools/binfile | 41 +++++++++++++++++++++++++++++++++++++++++
xen/xsm/flask/Makefile | 5 ++++-
xen/xsm/flask/flask-policy.S | 16 ----------------
4 files changed, 46 insertions(+), 17 deletions(-)
create mode 100755 xen/tools/binfile
delete mode 100644 xen/xsm/flask/flask-policy.S
diff --git a/.gitignore b/.gitignore
index 4ca679ddbc..b2624df79a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -313,6 +313,7 @@ xen/test/livepatch/*.livepatch
xen/tools/kconfig/.tmp_gtkcheck
xen/tools/kconfig/.tmp_qtcheck
xen/tools/symbols
+xen/xsm/flask/flask-policy.S
xen/xsm/flask/include/av_perm_to_string.h
xen/xsm/flask/include/av_permissions.h
xen/xsm/flask/include/class_to_string.h
diff --git a/xen/tools/binfile b/xen/tools/binfile
new file mode 100755
index 0000000000..7bb35a5178
--- /dev/null
+++ b/xen/tools/binfile
@@ -0,0 +1,41 @@
+#!/bin/sh
+# usage: binfile [-i] [-a <align>] <target-src.S> <binary-file> <varname>
+# -a <align> align data at 2^<align> boundary (default: byte alignment)
+# -i add to .init.rodata (default: .rodata) section
+
+section=""
+align=0
+
+OPTIND=1
+while getopts "ia:" opt; do
+ case "$opt" in
+ i)
+ section=".init"
+ ;;
+ a)
+ align=$OPTARG
+ ;;
+ esac
+done
+
+target=$1
+binsource=$2
+varname=$3
+
+cat <<EOF >$target
+#include <asm/asm_defns.h>
+
+ .section $section.rodata, "a", %progbits
+
+ .p2align $align
+ .global $varname
+$varname:
+ .incbin "$binsource"
+.Lend:
+
+ .type $varname, %object
+ .size $varname, .Lend - $varname
+
+ .global ${varname}_size
+ ASM_INT(${varname}_size, .Lend - $varname)
+EOF
diff --git a/xen/xsm/flask/Makefile b/xen/xsm/flask/Makefile
index 7c3f381287..a807521235 100644
--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -30,6 +30,9 @@ $(AV_H_FILES): $(AV_H_DEPEND)
obj-bin-$(CONFIG_XSM_FLASK_POLICY) += flask-policy.o
flask-policy.o: policy.bin
+flask-policy.S: $(XEN_ROOT)/xen/tools/binfile
+ $(XEN_ROOT)/xen/tools/binfile -i $@ policy.bin xsm_flask_init_policy
+
FLASK_BUILD_DIR := $(CURDIR)
POLICY_SRC := $(FLASK_BUILD_DIR)/xenpolicy-$(XEN_FULLVERSION)
@@ -39,4 +42,4 @@ policy.bin: FORCE
.PHONY: clean
clean::
- rm -f $(ALL_H_FILES) *.o $(DEPS_RM) policy.* $(POLICY_SRC)
+ rm -f $(ALL_H_FILES) *.o $(DEPS_RM) policy.* $(POLICY_SRC) flask-policy.S
diff --git a/xen/xsm/flask/flask-policy.S b/xen/xsm/flask/flask-policy.S
deleted file mode 100644
index d38aa39964..0000000000
--- a/xen/xsm/flask/flask-policy.S
+++ /dev/null
@@ -1,16 +0,0 @@
-#include <asm/asm_defns.h>
-
- .section .init.rodata, "a", %progbits
-
-/* const unsigned char xsm_flask_init_policy[] __initconst */
- .global xsm_flask_init_policy
-xsm_flask_init_policy:
- .incbin "policy.bin"
-.Lend:
-
- .type xsm_flask_init_policy, %object
- .size xsm_flask_init_policy, . - xsm_flask_init_policy
-
-/* const unsigned int __initconst xsm_flask_init_policy_size */
- .global xsm_flask_init_policy_size
- ASM_INT(xsm_flask_init_policy_size, .Lend - xsm_flask_init_policy)
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 2/8] docs: add feature document for Xen hypervisor sysfs-like support
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 1/8] xen: add a generic way to include binary files as variables Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support Juergen Gross
` (5 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jan Beulich
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
V2:
- move example tree to the paths description (Ian Jackson)
- specify allowed characters for keys and values (Ian Jackson)
V3:
- correct introduction (writable entries)
V4:
- add list specification
- add entry example (Julien Grall)
- correct date and Xen version (Julien Grall)
- add ARM64 as possible architecture (Julien Grall)
- add version description to the feature doc (Jan Beulich)
---
docs/features/hypervisorfs.pandoc | 92 +++++++++++++++++++++++++++++++++
docs/misc/hypfs-paths.pandoc | 105 ++++++++++++++++++++++++++++++++++++++
2 files changed, 197 insertions(+)
create mode 100644 docs/features/hypervisorfs.pandoc
create mode 100644 docs/misc/hypfs-paths.pandoc
diff --git a/docs/features/hypervisorfs.pandoc b/docs/features/hypervisorfs.pandoc
new file mode 100644
index 0000000000..a0a0ead057
--- /dev/null
+++ b/docs/features/hypervisorfs.pandoc
@@ -0,0 +1,92 @@
+% Hypervisor FS
+% Revision 1
+
+\clearpage
+
+# Basics
+---------------- ---------------------
+ Status: **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. Entries and directories are created by the hypervisor,
+while the toolstack is able to use a hypercall to query the entry
+values or (if allowed by the hypervisor) to modify them.
+
+# 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 paths are documented in `docs/misc/hypfs-paths.pandoc`.
+
+# Technical details
+
+Access to the hypervisor filesystem is done via the stable new hypercall
+__HYPERVISOR_filesystem_op. This hypercall supports a sub-command
+XEN_HYPFS_OP_get_version which will return the highest version of the
+interface supported by the hypervisor. Additions to the interface need
+to bump the interface version. The hypervisor is required to support the
+previous interface versions, too (this implies that additions will always
+require new sub-commands in order to allow the hypervisor to decide which
+version of the interface to use).
+
+* hypercall interface specification
+ * `xen/include/public/hypfs.h`
+* hypervisor internal files
+ * `xen/include/xen/hypfs.h`
+ * `xen/common/hypfs.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
+---------- -------- -------- -------------------------------------------
+2020-01-23 1 Xen 4.14 Document written
+---------- -------- -------- -------------------------------------------
diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
new file mode 100644
index 0000000000..b9f50f6998
--- /dev/null
+++ b/docs/misc/hypfs-paths.pandoc
@@ -0,0 +1,105 @@
+# 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).
+
+A hypervisor file system entry name can be any 0-delimited byte string
+not containing any '/' character. The names "." and ".." are reserved
+for file system internal use.
+
+VALUES are strings and can take the following forms:
+
+* STRING -- an arbitrary 0-delimited byte 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
+ ")".
+* {VALUE, VALUE, ... } -- a list of possible values separated by "," and
+ 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 | ARM64 | 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.
+
+So an entry could look like this:
+
+ /cpu-bugs/active-pv/xpti = ("No"|{"dom0", "domU", "PCID on"}) [w,X86,PV]
+
+Possible values would be "No" or a list of "dom0", "domU", and "PCID on".
+The entry would be writable and it would exist on X86 only and only if the
+hypervisor is configured to support PV guests.
+
+## Example
+
+A populated Xen hypervisor file system might look like the following example:
+
+ /
+ 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"
+ params/ directory with hypervisor parameter values
+ (boot/runtime parameters)
+
+## General Paths
+
+#### /
+
+The root of the hypervisor file system.
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 1/8] xen: add a generic way to include binary files as variables Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 2/8] docs: add feature document for Xen hypervisor sysfs-like support Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 15:49 ` Jan Beulich
2020-02-19 16:26 ` Julien Grall
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 4/8] libs: add libxenhypfs Juergen Gross
` (4 subsequent siblings)
7 siblings, 2 replies; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jan Beulich, Daniel De Graaf, Volodymyr Babchuk,
Roger Pau Monné
Add the infrastructure for the hypervisor filesystem.
This includes the hypercall interface and the base functions for
entry creation, deletion and modification.
In order not to have to repeat the same pattern multiple times in case
adding a new node should BUG_ON() failure, the helpers for adding a
node (hypfs_add_dir() and hypfs_add_leaf()) get a nofault parameter
causing the BUG() in case of a failure.
When supporting writable leafs the entry's write pointer will need to
be set to the function performing the write to the variable holding the
content. In case there are no special constraints this will be
hypfs_write_bool() for type XEN_HYPFS_TYPE_BOOL and hypfs_write_leaf()
for the other entry types.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V1:
- rename files from filesystem.* to hypfs.*
- add dummy write entry support
- rename hypercall filesystem_op to hypfs_op
- add support for unsigned integer entries
V2:
- test new entry name to be valid
V3:
- major rework, especially by supporting binary contents of entries
- addressed all comments
V4:
- sort #includes alphabetically (Wei Liu)
- add public interface structures to xlat.lst (Jan Beulich)
- let DIRENTRY_SIZE() add 1 for trailing nul byte (Jan Beulich)
- remove hypfs_add_entry() (Jan Beulich)
- len -> ulen (Jan Beulich)
- switch sequence of tests in hypfs_get_entry_rel() (Jan Beulich)
- add const qualifier (Jan Beulich)
- return -ENOBUFS if only direntry but no entry contents are returned
(Jan Beulich)
- use xmalloc() instead of xzalloc() (Jan Beulich)
- better error handling in hypfs_write_leaf() (Jan Beulich)
- return -EOPNOTSUPP for unknown sub-command (Jan Beulich)
- use plain integers for enum-like constants in public interface
(Jan Beulich)
- rename XEN_HYPFS_OP_read_contents to XEN_HYPFS_OP_read (Jan Beulich)
- add some comments in include/public/hypfs.h (Jan Beulich)
- use const_char for user parameter path (Jan Beulich)
- add helpers for XEN_HYPFS_TYPE_BOOL and XEN_HYPFS_TYPE_INT entry
definitions (Jan Beulich)
- make statically defined entries __read_mostly (Jan Beulich)
V5:
- switch to xsm for privilege check
---
tools/flask/policy/modules/dom0.te | 2 +-
xen/arch/arm/traps.c | 1 +
xen/arch/x86/hvm/hypercall.c | 1 +
xen/arch/x86/hypercall.c | 1 +
xen/arch/x86/pv/hypercall.c | 1 +
xen/common/Makefile | 1 +
xen/common/hypfs.c | 357 ++++++++++++++++++++++++++++++++++++
xen/include/public/hypfs.h | 127 +++++++++++++
xen/include/public/xen.h | 1 +
xen/include/xen/hypercall.h | 8 +
xen/include/xen/hypfs.h | 108 +++++++++++
xen/include/xlat.lst | 2 +
xen/include/xsm/dummy.h | 6 +
xen/include/xsm/xsm.h | 6 +
xen/xsm/flask/policy/access_vectors | 2 +
15 files changed, 623 insertions(+), 1 deletion(-)
create mode 100644 xen/common/hypfs.c
create mode 100644 xen/include/public/hypfs.h
create mode 100644 xen/include/xen/hypfs.h
diff --git a/tools/flask/policy/modules/dom0.te b/tools/flask/policy/modules/dom0.te
index 272f6a4f75..20925e38a2 100644
--- a/tools/flask/policy/modules/dom0.te
+++ b/tools/flask/policy/modules/dom0.te
@@ -11,7 +11,7 @@ allow dom0_t xen_t:xen {
mtrr_del mtrr_read microcode physinfo quirk writeconsole readapic
writeapic privprofile nonprivprofile kexec firmware sleep frequency
getidle debug getcpuinfo heap pm_op mca_op lockprof cpupool_op
- getscheduler setscheduler
+ getscheduler setscheduler hypfs_op
};
allow dom0_t xen_t:xen2 {
resource_op psr_cmt_op psr_alloc pmu_ctrl get_symbol
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 6f9bec22d3..87af810667 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1382,6 +1382,7 @@ static arm_hypercall_t arm_hypercall_table[] = {
#ifdef CONFIG_ARGO
HYPERCALL(argo_op, 5),
#endif
+ HYPERCALL(hypfs_op, 5),
};
#ifndef NDEBUG
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 33dd2d99d2..210dda4f38 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -144,6 +144,7 @@ static const hypercall_table_t hvm_hypercall_table[] = {
#endif
HYPERCALL(xenpmu_op),
COMPAT_CALL(dm_op),
+ HYPERCALL(hypfs_op),
HYPERCALL(arch_1)
};
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 7f299d45c6..05a3f5e25b 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -73,6 +73,7 @@ const hypercall_args_t hypercall_args_table[NR_hypercalls] =
ARGS(hvm_op, 2),
ARGS(dm_op, 3),
#endif
+ ARGS(hypfs_op, 5),
ARGS(mca, 1),
ARGS(arch_1, 1),
};
diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c
index 17ddf9ea1f..83907d4f00 100644
--- a/xen/arch/x86/pv/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -85,6 +85,7 @@ const hypercall_table_t pv_hypercall_table[] = {
HYPERCALL(hvm_op),
COMPAT_CALL(dm_op),
#endif
+ HYPERCALL(hypfs_op),
HYPERCALL(mca),
HYPERCALL(arch_1),
};
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 2abb8250b0..3a2c1ae690 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -10,6 +10,7 @@ obj-y += domain.o
obj-y += event_2l.o
obj-y += event_channel.o
obj-y += event_fifo.o
+obj-y += hypfs.o
obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
obj-$(CONFIG_GRANT_TABLE) += grant_table.o
obj-y += guestcopy.o
diff --git a/xen/common/hypfs.c b/xen/common/hypfs.c
new file mode 100644
index 0000000000..0a32d45979
--- /dev/null
+++ b/xen/common/hypfs.c
@@ -0,0 +1,357 @@
+/******************************************************************************
+ *
+ * hypfs.c
+ *
+ * Simple sysfs-like file system for the hypervisor.
+ */
+
+#include <xen/err.h>
+#include <xen/guest_access.h>
+#include <xen/hypercall.h>
+#include <xen/hypfs.h>
+#include <xen/lib.h>
+#include <xen/rwlock.h>
+#include <public/hypfs.h>
+
+#define DIRENTRY_NAME_OFF offsetof(struct xen_hypfs_dirlistentry, name)
+#define DIRENTRY_SIZE(name_len) \
+ (DIRENTRY_NAME_OFF + \
+ ROUNDUP((name_len) + 1, alignof(struct xen_hypfs_direntry)))
+
+static DEFINE_RWLOCK(hypfs_lock);
+
+HYPFS_DIR_INIT(hypfs_root, "");
+
+static int add_entry(struct hypfs_entry_dir *parent, struct hypfs_entry *new)
+{
+ int ret = -ENOENT;
+ struct hypfs_entry *e;
+
+ write_lock(&hypfs_lock);
+
+ list_for_each_entry ( e, &parent->dirlist, list )
+ {
+ int cmp = strcmp(e->name, new->name);
+
+ if ( cmp > 0 )
+ {
+ ret = 0;
+ list_add_tail(&new->list, &e->list);
+ break;
+ }
+ if ( cmp == 0 )
+ {
+ ret = -EEXIST;
+ break;
+ }
+ }
+
+ if ( ret == -ENOENT )
+ {
+ ret = 0;
+ list_add_tail(&new->list, &parent->dirlist);
+ }
+
+ if ( !ret )
+ {
+ unsigned int sz = strlen(new->name);
+
+ parent->e.size += DIRENTRY_SIZE(sz);
+ }
+
+ write_unlock(&hypfs_lock);
+
+ return ret;
+}
+
+int hypfs_add_dir(struct hypfs_entry_dir *parent,
+ struct hypfs_entry_dir *dir, bool nofault)
+{
+ int ret;
+
+ ret = add_entry(parent, &dir->e);
+ BUG_ON(nofault && ret);
+
+ return ret;
+}
+
+int hypfs_add_leaf(struct hypfs_entry_dir *parent,
+ struct hypfs_entry_leaf *leaf, bool nofault)
+{
+ int ret;
+
+ if ( !leaf->content )
+ ret = -EINVAL;
+ else
+ ret = add_entry(parent, &leaf->e);
+ BUG_ON(nofault && ret);
+
+ return ret;
+}
+
+static int hypfs_get_path_user(char *buf,
+ XEN_GUEST_HANDLE_PARAM(const_char) uaddr,
+ unsigned long ulen)
+{
+ if ( ulen > XEN_HYPFS_MAX_PATHLEN )
+ return -EINVAL;
+
+ if ( copy_from_guest(buf, uaddr, ulen) )
+ return -EFAULT;
+
+ if ( buf[ulen - 1] )
+ return -EINVAL;
+
+ return 0;
+}
+
+static struct hypfs_entry *hypfs_get_entry_rel(struct hypfs_entry_dir *dir,
+ const char *path)
+{
+ const char *end;
+ struct hypfs_entry *entry;
+ unsigned int name_len;
+
+ if ( dir->e.type != XEN_HYPFS_TYPE_DIR )
+ return NULL;
+
+ if ( !*path )
+ return &dir->e;
+
+ end = strchr(path, '/');
+ if ( !end )
+ end = strchr(path, '\0');
+ name_len = end - path;
+
+ list_for_each_entry ( entry, &dir->dirlist, list )
+ {
+ int cmp = strncmp(path, entry->name, name_len);
+ struct hypfs_entry_dir *d = container_of(entry,
+ struct hypfs_entry_dir, e);
+
+ if ( cmp < 0 )
+ return NULL;
+ if ( !cmp && strlen(entry->name) == name_len )
+ return *end ? hypfs_get_entry_rel(d, end + 1) : entry;
+ }
+
+ return NULL;
+}
+
+struct hypfs_entry *hypfs_get_entry(const char *path)
+{
+ if ( path[0] != '/' )
+ return NULL;
+
+ return hypfs_get_entry_rel(&hypfs_root, path + 1);
+}
+
+int hypfs_read_dir(const struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr)
+{
+ const struct hypfs_entry_dir *d;
+ const struct hypfs_entry *e;
+ unsigned int size = entry->size;
+
+ d = container_of(entry, const struct hypfs_entry_dir, e);
+
+ list_for_each_entry ( e, &d->dirlist, list )
+ {
+ struct xen_hypfs_dirlistentry direntry;
+ unsigned int e_namelen = strlen(e->name);
+ unsigned int e_len = DIRENTRY_SIZE(e_namelen);
+
+ direntry.e.flags = e->write ? XEN_HYPFS_WRITEABLE : 0;
+ direntry.e.type = e->type;
+ direntry.e.encoding = e->encoding;
+ direntry.e.content_len = e->size;
+ direntry.off_next = list_is_last(&e->list, &d->dirlist) ? 0 : e_len;
+ if ( copy_to_guest(uaddr, &direntry, 1) )
+ return -EFAULT;
+
+ if ( copy_to_guest_offset(uaddr, DIRENTRY_NAME_OFF,
+ e->name, e_namelen + 1) )
+ return -EFAULT;
+
+ guest_handle_add_offset(uaddr, e_len);
+
+ ASSERT(e_len <= size);
+ size -= e_len;
+ }
+
+ return 0;
+}
+
+int hypfs_read_leaf(const struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr)
+{
+ const struct hypfs_entry_leaf *l;
+
+ l = container_of(entry, const struct hypfs_entry_leaf, e);
+
+ return copy_to_guest(uaddr, l->content, entry->size) ? -EFAULT: 0;
+}
+
+static int hypfs_read(const struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ struct xen_hypfs_direntry e;
+ long ret = -EINVAL;
+
+ if ( ulen < sizeof(e) )
+ goto out;
+
+ e.flags = entry->write ? XEN_HYPFS_WRITEABLE : 0;
+ e.type = entry->type;
+ e.encoding = entry->encoding;
+ e.content_len = entry->size;
+
+ ret = -EFAULT;
+ if ( copy_to_guest(uaddr, &e, 1) )
+ goto out;
+
+ ret = -ENOBUFS;
+ if ( ulen < entry->size + sizeof(e) )
+ goto out;
+
+ guest_handle_add_offset(uaddr, sizeof(e));
+
+ ret = entry->read(entry, uaddr);
+
+ out:
+ return ret;
+}
+
+int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ char *buf;
+ int ret;
+
+ if ( ulen > leaf->e.size )
+ return -ENOSPC;
+
+ buf = xmalloc_array(char, ulen);
+ if ( !buf )
+ return -ENOMEM;
+
+ ret = -EFAULT;
+ if ( copy_from_guest(buf, uaddr, ulen) )
+ goto out;
+
+ ret = -EINVAL;
+ if ( leaf->e.type == XEN_HYPFS_TYPE_STRING && buf[ulen] )
+ goto out;
+
+ ret = 0;
+ memcpy(leaf->write_ptr, buf, ulen);
+
+ out:
+ xfree(buf);
+ return ret;
+}
+
+int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ union {
+ char buf[8];
+ uint8_t u8;
+ uint16_t u16;
+ uint32_t u32;
+ uint64_t u64;
+ } u;
+
+ ASSERT(leaf->e.type == XEN_HYPFS_TYPE_UINT && leaf->e.size <= 8);
+
+ if ( ulen != leaf->e.size )
+ return -EDOM;
+
+ if ( copy_from_guest(u.buf, uaddr, ulen) )
+ return -EFAULT;
+
+ switch ( leaf->e.size )
+ {
+ case 1:
+ *(uint8_t *)leaf->write_ptr = !!u.u8;
+ break;
+ case 2:
+ *(uint16_t *)leaf->write_ptr = !!u.u16;
+ break;
+ case 4:
+ *(uint32_t *)leaf->write_ptr = !!u.u32;
+ break;
+ case 8:
+ *(uint64_t *)leaf->write_ptr = !!u.u64;
+ break;
+ }
+
+ return 0;
+}
+
+static int hypfs_write(struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ struct hypfs_entry_leaf *l;
+
+ if ( !entry->write )
+ return -EACCES;
+
+ l = container_of(entry, struct hypfs_entry_leaf, e);
+
+ return entry->write(l, uaddr, ulen);
+}
+
+long do_hypfs_op(unsigned int cmd,
+ XEN_GUEST_HANDLE_PARAM(const_char) arg1, unsigned long arg2,
+ XEN_GUEST_HANDLE_PARAM(void) arg3, unsigned long arg4)
+{
+ int ret;
+ struct hypfs_entry *entry;
+ static char path[XEN_HYPFS_MAX_PATHLEN];
+
+ if ( xsm_hypfs_op(XSM_PRIV) )
+ return -EPERM;
+
+ if ( cmd == XEN_HYPFS_OP_get_version )
+ return XEN_HYPFS_VERSION;
+
+ if ( cmd == XEN_HYPFS_OP_write_contents )
+ write_lock(&hypfs_lock);
+ else
+ read_lock(&hypfs_lock);
+
+ ret = hypfs_get_path_user(path, arg1, arg2);
+ if ( ret )
+ goto out;
+
+ entry = hypfs_get_entry(path);
+ if ( !entry )
+ {
+ ret = -ENOENT;
+ goto out;
+ }
+
+ switch ( cmd )
+ {
+ case XEN_HYPFS_OP_read:
+ ret = hypfs_read(entry, arg3, arg4);
+ break;
+
+ case XEN_HYPFS_OP_write_contents:
+ ret = hypfs_write(entry, arg3, arg4);
+ break;
+
+ default:
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ out:
+ if ( cmd == XEN_HYPFS_OP_write_contents )
+ write_unlock(&hypfs_lock);
+ else
+ read_unlock(&hypfs_lock);
+
+ return ret;
+}
diff --git a/xen/include/public/hypfs.h b/xen/include/public/hypfs.h
new file mode 100644
index 0000000000..4aa2e9449a
--- /dev/null
+++ b/xen/include/public/hypfs.h
@@ -0,0 +1,127 @@
+/******************************************************************************
+ * Xen Hypervisor Filesystem
+ *
+ * Copyright (c) 2019, SUSE Software Solutions Germany GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __XEN_PUBLIC_HYPFS_H__
+#define __XEN_PUBLIC_HYPFS_H__
+
+#include "xen.h"
+
+/*
+ * Definitions for the __HYPERVISOR_hypfs_op hypercall.
+ */
+
+/* Highest version number of the hypfs interface currently defined. */
+#define XEN_HYPFS_VERSION 1
+
+/* Maximum length of a path in the filesystem. */
+#define XEN_HYPFS_MAX_PATHLEN 1024
+
+struct xen_hypfs_direntry {
+ uint16_t flags;
+#define XEN_HYPFS_WRITEABLE 0x0001
+ uint8_t type;
+#define XEN_HYPFS_TYPE_DIR 0
+#define XEN_HYPFS_TYPE_BLOB 1
+#define XEN_HYPFS_TYPE_STRING 2
+#define XEN_HYPFS_TYPE_UINT 3
+#define XEN_HYPFS_TYPE_INT 4
+#define XEN_HYPFS_TYPE_BOOL 5
+ uint8_t encoding;
+#define XEN_HYPFS_ENC_PLAIN 0
+#define XEN_HYPFS_ENC_GZIP 1
+ uint32_t content_len;
+};
+
+struct xen_hypfs_dirlistentry {
+ struct xen_hypfs_direntry e;
+ /* Offset in bytes to next entry (0 == this is the last entry). */
+ uint16_t off_next;
+ /* Zero terminated entry name, possibly with some padding for alignment. */
+ char name[XEN_FLEX_ARRAY_DIM];
+};
+
+/*
+ * Hypercall operations.
+ */
+
+/*
+ * XEN_HYPFS_OP_get_version
+ *
+ * Read highest interface version supported by the hypervisor.
+ *
+ * Possible return values:
+ * >0: highest supported interface version
+ * <0: negative Xen errno value
+ */
+#define XEN_HYPFS_OP_get_version 0
+
+/*
+ * XEN_HYPFS_OP_read
+ *
+ * Read a filesystem entry.
+ *
+ * Returns the direntry and contents of an entry in the buffer supplied by the
+ * caller (struct xen_hypfs_direntry with the contents following directly
+ * after it).
+ * The data buffer must be at least the size of the direntry returned in order
+ * to have success. If the data buffer was not large enough for all the data
+ * -ENOBUFS and no entry data is returned, but the direntry will contain the
+ * needed size for the returned data.
+ * The format of the contents is according to its entry type and encoding.
+ * The contents of a directory are multiple struct xen_hypfs_dirlistentry
+ * items.
+ *
+ * arg1: XEN_GUEST_HANDLE(path name)
+ * arg2: length of path name (including trailing zero byte)
+ * arg3: XEN_GUEST_HANDLE(data buffer written by hypervisor)
+ * arg4: data buffer size
+ *
+ * Possible return values:
+ * 0: success (at least the direntry was returned)
+ * <0 : negative Xen errno value
+ */
+#define XEN_HYPFS_OP_read 1
+
+/*
+ * XEN_HYPFS_OP_write_contents
+ *
+ * Write contents of a filesystem entry.
+ *
+ * Writes an entry with the contents of a buffer supplied by the caller.
+ * The data type and encoding can't be changed. The size can be changed only
+ * for blobs and strings.
+ *
+ * arg1: XEN_GUEST_HANDLE(path name)
+ * arg2: length of path name (including trailing zero byte)
+ * arg3: XEN_GUEST_HANDLE(content buffer read by hypervisor)
+ * arg4: content buffer size
+ *
+ * Possible return values:
+ * 0: success
+ * <0 : negative Xen errno value
+ */
+#define XEN_HYPFS_OP_write_contents 2
+
+#endif /* __XEN_PUBLIC_HYPFS_H__ */
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index d2198dffad..bf80f1da8c 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -130,6 +130,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_ulong_t);
#define __HYPERVISOR_argo_op 39
#define __HYPERVISOR_xenpmu_op 40
#define __HYPERVISOR_dm_op 41
+#define __HYPERVISOR_hypfs_op 42
/* Architecture-specific hypercall definitions. */
#define __HYPERVISOR_arch_0 48
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index ad8ad27b23..836a8b1ba8 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -150,6 +150,14 @@ do_dm_op(
unsigned int nr_bufs,
XEN_GUEST_HANDLE_PARAM(xen_dm_op_buf_t) bufs);
+extern long
+do_hypfs_op(
+ unsigned int cmd,
+ XEN_GUEST_HANDLE_PARAM(const_char) arg1,
+ unsigned long arg2,
+ XEN_GUEST_HANDLE_PARAM(void) arg3,
+ unsigned long arg4);
+
#ifdef CONFIG_COMPAT
extern int
diff --git a/xen/include/xen/hypfs.h b/xen/include/xen/hypfs.h
new file mode 100644
index 0000000000..5b26b2e611
--- /dev/null
+++ b/xen/include/xen/hypfs.h
@@ -0,0 +1,108 @@
+#ifndef __XEN_HYPFS_H__
+#define __XEN_HYPFS_H__
+
+#include <xen/list.h>
+#include <xen/string.h>
+#include <public/hypfs.h>
+
+struct hypfs_entry_leaf;
+
+struct hypfs_entry {
+ unsigned short type;
+ unsigned short encoding;
+ unsigned int size;
+ const char *name;
+ struct list_head list;
+ int (*read)(const struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr);
+ int (*write)(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen);
+};
+
+struct hypfs_entry_leaf {
+ struct hypfs_entry e;
+ union {
+ const void *content;
+ void *write_ptr;
+ };
+};
+
+struct hypfs_entry_dir {
+ struct hypfs_entry e;
+ struct list_head dirlist;
+};
+
+#define HYPFS_DIR_INIT(var, nam) \
+ struct hypfs_entry_dir __read_mostly var = { \
+ .e.type = XEN_HYPFS_TYPE_DIR, \
+ .e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .e.name = nam, \
+ .e.size = 0, \
+ .e.list = LIST_HEAD_INIT(var.e.list), \
+ .e.read = hypfs_read_dir, \
+ .dirlist = LIST_HEAD_INIT(var.dirlist), \
+ }
+
+/* Content and size need to be set via hypfs_string_set(). */
+#define HYPFS_STRING_INIT(var, nam) \
+ struct hypfs_entry_leaf __read_mostly var = { \
+ .e.type = XEN_HYPFS_TYPE_STRING, \
+ .e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .e.name = nam, \
+ .e.read = hypfs_read_leaf, \
+ }
+
+static inline void hypfs_string_set(struct hypfs_entry_leaf *leaf,
+ const char *str)
+{
+ leaf->content = str;
+ leaf->e.size = strlen(str) + 1;
+}
+
+#define HYPFS_UINT_INIT(var, nam, contvar) \
+ struct hypfs_entry_leaf __read_mostly var = { \
+ .e.type = XEN_HYPFS_TYPE_UINT, \
+ .e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .e.name = nam, \
+ .e.size = sizeof(contvar), \
+ .e.read = hypfs_read_leaf, \
+ .content = &contvar, \
+ }
+
+#define HYPFS_INT_INIT(var, nam, contvar) \
+ struct hypfs_entry_leaf __read_mostly var = { \
+ .e.type = XEN_HYPFS_TYPE_INT, \
+ .e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .e.name = nam, \
+ .e.size = sizeof(contvar), \
+ .e.read = hypfs_read_leaf, \
+ .content = &contvar, \
+ }
+
+#define HYPFS_BOOL_INIT(var, nam, contvar) \
+ struct hypfs_entry_leaf __read_mostly var = { \
+ .e.type = XEN_HYPFS_TYPE_BOOL, \
+ .e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .e.name = nam, \
+ .e.size = sizeof(contvar), \
+ .e.read = hypfs_read_leaf, \
+ .content = &contvar, \
+ }
+
+extern struct hypfs_entry_dir hypfs_root;
+
+struct hypfs_entry *hypfs_get_entry(const char *path);
+int hypfs_add_dir(struct hypfs_entry_dir *parent,
+ struct hypfs_entry_dir *dir, bool nofault);
+int hypfs_add_leaf(struct hypfs_entry_dir *parent,
+ struct hypfs_entry_leaf *leaf, bool nofault);
+int hypfs_read_dir(const struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr);
+int hypfs_read_leaf(const struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr);
+int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen);
+int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen);
+
+#endif /* __XEN_HYPFS_H__ */
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 95f5e5592b..228728ba25 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -86,6 +86,8 @@
? vcpu_hvm_context hvm/hvm_vcpu.h
? vcpu_hvm_x86_32 hvm/hvm_vcpu.h
? vcpu_hvm_x86_64 hvm/hvm_vcpu.h
+? xen_hypfs_direntry hypfs.h
+? xen_hypfs_dirlistentry hypfs.h
? kexec_exec kexec.h
! kexec_image kexec.h
! kexec_range kexec.h
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index b8e185e6fa..68b5bf8f8e 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -433,6 +433,12 @@ static XSM_INLINE int xsm_page_offline(XSM_DEFAULT_ARG uint32_t cmd)
return xsm_default_action(action, current->domain, NULL);
}
+static XSM_INLINE int xsm_hypfs_op(XSM_DEFAULT_VOID)
+{
+ XSM_ASSERT_ACTION(XSM_PRIV);
+ return xsm_default_action(action, current->domain, NULL);
+}
+
static XSM_INLINE long xsm_do_xsm_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return -ENOSYS;
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index e22d6160b5..a80bcf3e42 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -127,6 +127,7 @@ struct xsm_operations {
int (*resource_setup_misc) (void);
int (*page_offline)(uint32_t cmd);
+ int (*hypfs_op)(void);
long (*do_xsm_op) (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op);
#ifdef CONFIG_COMPAT
@@ -536,6 +537,11 @@ static inline int xsm_page_offline(xsm_default_t def, uint32_t cmd)
return xsm_ops->page_offline(cmd);
}
+static inline int xsm_hypfs_op(xsm_default_t def)
+{
+ return xsm_ops->hypfs_op();
+}
+
static inline long xsm_do_xsm_op (XEN_GUEST_HANDLE_PARAM(xsm_op_t) op)
{
return xsm_ops->do_xsm_op(op);
diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors
index c055c14c26..c9e385fb9b 100644
--- a/xen/xsm/flask/policy/access_vectors
+++ b/xen/xsm/flask/policy/access_vectors
@@ -67,6 +67,8 @@ class xen
lockprof
# XEN_SYSCTL_cpupool_op
cpupool_op
+# hypfs hypercall
+ hypfs_op
# XEN_SYSCTL_scheduler_op with XEN_DOMCTL_SCHEDOP_getinfo, XEN_SYSCTL_sched_id, XEN_DOMCTL_SCHEDOP_getvcpuinfo
getscheduler
# XEN_SYSCTL_scheduler_op with XEN_DOMCTL_SCHEDOP_putinfo, XEN_DOMCTL_SCHEDOP_putvcpuinfo
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 4/8] libs: add libxenhypfs
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
` (2 preceding siblings ...)
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 5/8] tools: add xenfs tool Juergen Gross
` (3 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jan Beulich
Add the new library libxenhypfs for access to the hypervisor filesystem.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
---
V1:
- rename to libxenhypfs
- add xenhypfs_write()
V3:
- major rework due to new hypervisor interface
- add decompression capability
V4:
- add dependency to libz in pkgconfig file (Wei Liu)
---
.gitignore | 2 +
tools/Rules.mk | 6 +
tools/libs/Makefile | 1 +
tools/libs/hypfs/Makefile | 16 ++
tools/libs/hypfs/core.c | 533 ++++++++++++++++++++++++++++++++++++
tools/libs/hypfs/include/xenhypfs.h | 75 +++++
tools/libs/hypfs/libxenhypfs.map | 10 +
tools/libs/hypfs/xenhypfs.pc.in | 10 +
8 files changed, 653 insertions(+)
create mode 100644 tools/libs/hypfs/Makefile
create mode 100644 tools/libs/hypfs/core.c
create mode 100644 tools/libs/hypfs/include/xenhypfs.h
create mode 100644 tools/libs/hypfs/libxenhypfs.map
create mode 100644 tools/libs/hypfs/xenhypfs.pc.in
diff --git a/.gitignore b/.gitignore
index b2624df79a..e98c3f056d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -109,6 +109,8 @@ tools/libs/evtchn/headers.chk
tools/libs/evtchn/xenevtchn.pc
tools/libs/gnttab/headers.chk
tools/libs/gnttab/xengnttab.pc
+tools/libs/hypfs/headers.chk
+tools/libs/hypfs/xenhypfs.pc
tools/libs/call/headers.chk
tools/libs/call/xencall.pc
tools/libs/foreignmemory/headers.chk
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 52f47be3f8..a04697a33c 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -19,6 +19,7 @@ XEN_LIBXENGNTTAB = $(XEN_ROOT)/tools/libs/gnttab
XEN_LIBXENCALL = $(XEN_ROOT)/tools/libs/call
XEN_LIBXENFOREIGNMEMORY = $(XEN_ROOT)/tools/libs/foreignmemory
XEN_LIBXENDEVICEMODEL = $(XEN_ROOT)/tools/libs/devicemodel
+XEN_LIBXENHYPFS = $(XEN_ROOT)/tools/libs/hypfs
XEN_LIBXC = $(XEN_ROOT)/tools/libxc
XEN_XENLIGHT = $(XEN_ROOT)/tools/libxl
# Currently libxlutil lives in the same directory as libxenlight
@@ -134,6 +135,11 @@ SHDEPS_libxendevicemodel = $(SHLIB_libxentoollog) $(SHLIB_libxentoolcore) $(SHLI
LDLIBS_libxendevicemodel = $(SHDEPS_libxendevicemodel) $(XEN_LIBXENDEVICEMODEL)/libxendevicemodel$(libextension)
SHLIB_libxendevicemodel = $(SHDEPS_libxendevicemodel) -Wl,-rpath-link=$(XEN_LIBXENDEVICEMODEL)
+CFLAGS_libxenhypfs = -I$(XEN_LIBXENHYPFS)/include $(CFLAGS_xeninclude)
+SHDEPS_libxenhypfs = $(SHLIB_libxentoollog) $(SHLIB_libxentoolcore) $(SHLIB_xencall)
+LDLIBS_libxenhypfs = $(SHDEPS_libxenhypfs) $(XEN_LIBXENHYPFS)/libxenhypfs$(libextension)
+SHLIB_libxenhypfs = $(SHDEPS_libxenhypfs) -Wl,-rpath-link=$(XEN_LIBXENHYPFS)
+
# code which compiles against libxenctrl get __XEN_TOOLS__ and
# therefore sees the unstable hypercall interfaces.
CFLAGS_libxenctrl = -I$(XEN_LIBXC)/include $(CFLAGS_libxentoollog) $(CFLAGS_libxenforeignmemory) $(CFLAGS_libxendevicemodel) $(CFLAGS_xeninclude) -D__XEN_TOOLS__
diff --git a/tools/libs/Makefile b/tools/libs/Makefile
index 88901e7341..69cdfb5975 100644
--- a/tools/libs/Makefile
+++ b/tools/libs/Makefile
@@ -9,6 +9,7 @@ SUBDIRS-y += gnttab
SUBDIRS-y += call
SUBDIRS-y += foreignmemory
SUBDIRS-y += devicemodel
+SUBDIRS-y += hypfs
ifeq ($(CONFIG_RUMP),y)
SUBDIRS-y := toolcore
diff --git a/tools/libs/hypfs/Makefile b/tools/libs/hypfs/Makefile
new file mode 100644
index 0000000000..06dd449929
--- /dev/null
+++ b/tools/libs/hypfs/Makefile
@@ -0,0 +1,16 @@
+XEN_ROOT = $(CURDIR)/../../..
+include $(XEN_ROOT)/tools/Rules.mk
+
+MAJOR = 1
+MINOR = 0
+LIBNAME := hypfs
+USELIBS := toollog toolcore call
+
+APPEND_LDFLAGS += -lz
+
+SRCS-y += core.c
+
+include ../libs.mk
+
+$(PKG_CONFIG_LOCAL): PKG_CONFIG_INCDIR = $(XEN_LIBXENHYPFS)/include
+$(PKG_CONFIG_LOCAL): PKG_CONFIG_CFLAGS_LOCAL = $(CFLAGS_xeninclude)
diff --git a/tools/libs/hypfs/core.c b/tools/libs/hypfs/core.c
new file mode 100644
index 0000000000..ced616c879
--- /dev/null
+++ b/tools/libs/hypfs/core.c
@@ -0,0 +1,533 @@
+/*
+ * Copyright (c) 2019 SUSE Software Solutions Germany GmbH
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define __XEN_TOOLS__ 1
+
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <inttypes.h>
+#include <stdlib.h>
+#include <string.h>
+#include <zlib.h>
+
+#include <xentoollog.h>
+#include <xenhypfs.h>
+#include <xencall.h>
+#include <xentoolcore_internal.h>
+
+#include <xen/xen.h>
+#include <xen/hypfs.h>
+
+#define BUF_SIZE 4096
+
+struct xenhypfs_handle {
+ xentoollog_logger *logger, *logger_tofree;
+ unsigned int flags;
+ xencall_handle *xcall;
+};
+
+xenhypfs_handle *xenhypfs_open(xentoollog_logger *logger,
+ unsigned open_flags)
+{
+ xenhypfs_handle *fshdl = calloc(1, sizeof(*fshdl));
+
+ if (!fshdl)
+ return NULL;
+
+ fshdl->flags = open_flags;
+ fshdl->logger = logger;
+ fshdl->logger_tofree = NULL;
+
+ if (!fshdl->logger) {
+ fshdl->logger = fshdl->logger_tofree =
+ (xentoollog_logger*)
+ xtl_createlogger_stdiostream(stderr, XTL_PROGRESS, 0);
+ if (!fshdl->logger)
+ goto err;
+ }
+
+ fshdl->xcall = xencall_open(fshdl->logger, 0);
+ if (!fshdl->xcall)
+ goto err;
+
+ /* No need to remember supported version, we only support V1. */
+ if (xencall1(fshdl->xcall, __HYPERVISOR_hypfs_op,
+ XEN_HYPFS_OP_get_version) < 0)
+ goto err;
+
+ return fshdl;
+
+err:
+ xtl_logger_destroy(fshdl->logger_tofree);
+ xencall_close(fshdl->xcall);
+ free(fshdl);
+ return NULL;
+}
+
+int xenhypfs_close(xenhypfs_handle *fshdl)
+{
+ if (!fshdl)
+ return 0;
+
+ xencall_close(fshdl->xcall);
+ xtl_logger_destroy(fshdl->logger_tofree);
+ free(fshdl);
+ return 0;
+}
+
+static int xenhypfs_get_pathbuf(xenhypfs_handle *fshdl, const char *path,
+ char **path_buf)
+{
+ int ret = -1;
+ int path_sz;
+
+ if (!fshdl) {
+ errno = EBADF;
+ goto out;
+ }
+
+ path_sz = strlen(path) + 1;
+ if (path_sz > XEN_HYPFS_MAX_PATHLEN)
+ {
+ errno = ENAMETOOLONG;
+ goto out;
+ }
+
+ *path_buf = xencall_alloc_buffer(fshdl->xcall, path_sz);
+ if (!*path_buf) {
+ errno = ENOMEM;
+ goto out;
+ }
+ strcpy(*path_buf, path);
+
+ ret = path_sz;
+
+ out:
+ return ret;
+}
+
+static void *xenhypfs_inflate(void *in_data, size_t *sz)
+{
+ unsigned char *workbuf;
+ void *content = NULL;
+ unsigned int out_sz;
+ z_stream z = { .opaque = NULL };
+ int ret;
+
+ workbuf = malloc(BUF_SIZE);
+ if (!workbuf)
+ return NULL;
+
+ z.next_in = in_data;
+ z.avail_in = *sz;
+ ret = inflateInit2(&z, MAX_WBITS + 32); /* 32 == gzip */
+
+ for (*sz = 0; ret == Z_OK; *sz += out_sz) {
+ z.next_out = workbuf;
+ z.avail_out = BUF_SIZE;
+ ret = inflate(&z, Z_SYNC_FLUSH);
+ if (ret != Z_OK && ret != Z_STREAM_END)
+ break;
+
+ out_sz = z.next_out - workbuf;
+ content = realloc(content, *sz + out_sz);
+ if (!content) {
+ ret = Z_MEM_ERROR;
+ break;
+ }
+ memcpy(content + *sz, workbuf, out_sz);
+ }
+
+ inflateEnd(&z);
+ if (ret != Z_STREAM_END) {
+ free(content);
+ content = NULL;
+ errno = EIO;
+ }
+ free(workbuf);
+ return content;
+}
+
+static void xenhypfs_set_attrs(struct xen_hypfs_direntry *entry,
+ struct xenhypfs_dirent *dirent)
+{
+ dirent->size = entry->content_len;
+
+ switch(entry->type) {
+ case XEN_HYPFS_TYPE_DIR:
+ dirent->type = xenhypfs_type_dir;
+ break;
+ case XEN_HYPFS_TYPE_BLOB:
+ dirent->type = xenhypfs_type_blob;
+ break;
+ case XEN_HYPFS_TYPE_STRING:
+ dirent->type = xenhypfs_type_string;
+ break;
+ case XEN_HYPFS_TYPE_UINT:
+ dirent->type = xenhypfs_type_uint;
+ break;
+ case XEN_HYPFS_TYPE_INT:
+ dirent->type = xenhypfs_type_int;
+ break;
+ case XEN_HYPFS_TYPE_BOOL:
+ dirent->type = xenhypfs_type_bool;
+ break;
+ default:
+ dirent->type = xenhypfs_type_blob;
+ }
+
+ switch (entry->encoding) {
+ case XEN_HYPFS_ENC_PLAIN:
+ dirent->encoding = xenhypfs_enc_plain;
+ break;
+ case XEN_HYPFS_ENC_GZIP:
+ dirent->encoding = xenhypfs_enc_gzip;
+ break;
+ default:
+ dirent->encoding = xenhypfs_enc_plain;
+ dirent->type = xenhypfs_type_blob;
+ }
+
+ dirent->is_writable = entry->flags & XEN_HYPFS_WRITEABLE;
+}
+
+void *xenhypfs_read_raw(xenhypfs_handle *fshdl, const char *path,
+ struct xenhypfs_dirent **dirent)
+{
+ void *retbuf = NULL, *content = NULL;
+ char *path_buf = NULL;
+ const char *name;
+ struct xen_hypfs_direntry *entry;
+ int ret;
+ int sz, path_sz;
+
+ *dirent = NULL;
+ ret = xenhypfs_get_pathbuf(fshdl, path, &path_buf);
+ if (ret < 0)
+ goto out;
+
+ path_sz = ret;
+
+ for (sz = BUF_SIZE;; sz = sizeof(*entry) + entry->content_len) {
+ if (retbuf)
+ xencall_free_buffer(fshdl->xcall, retbuf);
+
+ retbuf = xencall_alloc_buffer(fshdl->xcall, sz);
+ if (!retbuf) {
+ errno = ENOMEM;
+ goto out;
+ }
+ entry = retbuf;
+
+ ret = xencall5(fshdl->xcall, __HYPERVISOR_hypfs_op, XEN_HYPFS_OP_read,
+ (unsigned long)path_buf, path_sz,
+ (unsigned long)retbuf, sz);
+ if (!ret)
+ break;
+
+ if (ret != ENOBUFS) {
+ errno = -ret;
+ goto out;
+ }
+ }
+
+ content = malloc(entry->content_len);
+ if (!content)
+ goto out;
+ memcpy(content, entry + 1, entry->content_len);
+
+ name = strrchr(path, '/');
+ if (!name)
+ name = path;
+ else {
+ name++;
+ if (!*name)
+ name--;
+ }
+ *dirent = calloc(1, sizeof(struct xenhypfs_dirent) + strlen(name) + 1);
+ if (!*dirent) {
+ free(content);
+ content = NULL;
+ errno = ENOMEM;
+ goto out;
+ }
+ (*dirent)->name = (char *)(*dirent + 1);
+ strcpy((*dirent)->name, name);
+ xenhypfs_set_attrs(entry, *dirent);
+
+ out:
+ ret = errno;
+ xencall_free_buffer(fshdl->xcall, path_buf);
+ xencall_free_buffer(fshdl->xcall, retbuf);
+ errno = ret;
+
+ return content;
+}
+
+char *xenhypfs_read(xenhypfs_handle *fshdl, const char *path)
+{
+ char *buf, *ret_buf = NULL;
+ struct xenhypfs_dirent *dirent;
+ int ret;
+
+ buf = xenhypfs_read_raw(fshdl, path, &dirent);
+ if (!buf)
+ goto out;
+
+ switch (dirent->encoding) {
+ case xenhypfs_enc_plain:
+ break;
+ case xenhypfs_enc_gzip:
+ ret_buf = xenhypfs_inflate(buf, &dirent->size);
+ if (!ret_buf)
+ goto out;
+ free(buf);
+ buf = ret_buf;
+ ret_buf = NULL;
+ break;
+ }
+
+ switch (dirent->type) {
+ case xenhypfs_type_dir:
+ errno = EISDIR;
+ break;
+ case xenhypfs_type_blob:
+ errno = EDOM;
+ break;
+ case xenhypfs_type_string:
+ ret_buf = buf;
+ buf = NULL;
+ break;
+ case xenhypfs_type_uint:
+ case xenhypfs_type_bool:
+ switch (dirent->size) {
+ case 1:
+ ret = asprintf(&ret_buf, "%"PRIu8, *(uint8_t *)buf);
+ break;
+ case 2:
+ ret = asprintf(&ret_buf, "%"PRIu16, *(uint16_t *)buf);
+ break;
+ case 4:
+ ret = asprintf(&ret_buf, "%"PRIu32, *(uint32_t *)buf);
+ break;
+ case 8:
+ ret = asprintf(&ret_buf, "%"PRIu64, *(uint64_t *)buf);
+ break;
+ default:
+ ret = -1;
+ errno = EDOM;
+ }
+ if (ret < 0)
+ ret_buf = NULL;
+ break;
+ case xenhypfs_type_int:
+ switch (dirent->size) {
+ case 1:
+ ret = asprintf(&ret_buf, "%"PRId8, *(int8_t *)buf);
+ break;
+ case 2:
+ ret = asprintf(&ret_buf, "%"PRId16, *(int16_t *)buf);
+ break;
+ case 4:
+ ret = asprintf(&ret_buf, "%"PRId32, *(int32_t *)buf);
+ break;
+ case 8:
+ ret = asprintf(&ret_buf, "%"PRId64, *(int64_t *)buf);
+ break;
+ default:
+ ret = -1;
+ errno = EDOM;
+ }
+ if (ret < 0)
+ ret_buf = NULL;
+ break;
+ }
+
+ out:
+ ret = errno;
+ free(buf);
+ free(dirent);
+ errno = ret;
+
+ return ret_buf;
+}
+
+struct xenhypfs_dirent *xenhypfs_readdir(xenhypfs_handle *fshdl,
+ const char *path,
+ unsigned int *num_entries)
+{
+ void *buf, *curr;
+ int ret;
+ char *names;
+ struct xenhypfs_dirent *ret_buf = NULL, *dirent;
+ unsigned int n = 0, name_sz = 0;
+ struct xen_hypfs_dirlistentry *entry;
+
+ buf = xenhypfs_read_raw(fshdl, path, &dirent);
+ if (!buf)
+ goto out;
+
+ if (dirent->type != xenhypfs_type_dir ||
+ dirent->encoding != xenhypfs_enc_plain) {
+ errno = ENOTDIR;
+ goto out;
+ }
+
+ if (dirent->size) {
+ curr = buf;
+ for (n = 1;; n++) {
+ entry = curr;
+ name_sz += strlen(entry->name) + 1;
+ if (!entry->off_next)
+ break;
+
+ curr += entry->off_next;
+ }
+ }
+
+ ret_buf = malloc(n * sizeof(*ret_buf) + name_sz);
+ if (!ret_buf)
+ goto out;
+
+ *num_entries = n;
+ names = (char *)(ret_buf + n);
+ curr = buf;
+ for (n = 0; n < *num_entries; n++) {
+ entry = curr;
+ xenhypfs_set_attrs(&entry->e, ret_buf + n);
+ ret_buf[n].name = names;
+ strcpy(names, entry->name);
+ names += strlen(entry->name) + 1;
+ curr += entry->off_next;
+ }
+
+ out:
+ ret = errno;
+ free(buf);
+ free(dirent);
+ errno = ret;
+
+ return ret_buf;
+}
+
+int xenhypfs_write(xenhypfs_handle *fshdl, const char *path, const char *val)
+{
+ void *buf = NULL;
+ char *path_buf = NULL, *val_end;
+ int ret, saved_errno;
+ int sz, path_sz;
+ struct xen_hypfs_direntry *entry;
+ uint64_t mask;
+
+ ret = xenhypfs_get_pathbuf(fshdl, path, &path_buf);
+ if (ret < 0)
+ goto out;
+
+ path_sz = ret;
+ ret = -1;
+
+ sz = BUF_SIZE;
+ buf = xencall_alloc_buffer(fshdl->xcall, sz);
+ if (!buf) {
+ errno = ENOMEM;
+ goto out;
+ }
+
+ ret = xencall5(fshdl->xcall, __HYPERVISOR_hypfs_op, XEN_HYPFS_OP_read,
+ (unsigned long)path_buf, path_sz,
+ (unsigned long)buf, sizeof(*entry));
+ if (ret)
+ goto out;
+ ret = -1;
+ entry = buf;
+ if (!(entry->flags & XEN_HYPFS_WRITEABLE)) {
+ errno = EACCES;
+ goto out;
+ }
+ if (entry->encoding != XEN_HYPFS_ENC_PLAIN) {
+ /* Writing compressed data currently not supported. */
+ errno = EDOM;
+ goto out;
+ }
+
+ switch (entry->type) {
+ case XEN_HYPFS_TYPE_STRING:
+ if (sz < strlen(val) + 1) {
+ sz = strlen(val) + 1;
+ xencall_free_buffer(fshdl->xcall, buf);
+ buf = xencall_alloc_buffer(fshdl->xcall, sz);
+ if (!buf) {
+ errno = ENOMEM;
+ goto out;
+ }
+ }
+ strcpy(buf, val);
+ break;
+ case XEN_HYPFS_TYPE_UINT:
+ sz = entry->content_len;
+ errno = 0;
+ *(unsigned long long *)buf = strtoull(val, &val_end, 0);
+ if (errno || !*val || *val_end)
+ goto out;
+ mask = ~0ULL << (8 * sz);
+ if ((*(uint64_t *)buf & mask) && ((*(uint64_t *)buf & mask) != mask)) {
+ errno = ERANGE;
+ goto out;
+ }
+ break;
+ case XEN_HYPFS_TYPE_INT:
+ sz = entry->content_len;
+ errno = 0;
+ *(unsigned long long *)buf = strtoll(val, &val_end, 0);
+ if (errno || !*val || *val_end)
+ goto out;
+ mask = (sz == 8) ? 0 : ~0ULL << (8 * sz);
+ if ((*(uint64_t *)buf & mask) && ((*(uint64_t *)buf & mask) != mask)) {
+ errno = ERANGE;
+ goto out;
+ }
+ break;
+ case XEN_HYPFS_TYPE_BOOL:
+ *(unsigned long long *)buf = 0;
+ if (!strcmp(val, "1") || !strcmp(val, "on") || !strcmp(val, "yes") ||
+ !strcmp(val, "true") || !strcmp(val, "enable"))
+ *(unsigned long long *)buf = 1;
+ else if (strcmp(val, "0") && strcmp(val, "no") && strcmp(val, "off") &&
+ strcmp(val, "false") && strcmp(val, "disable")) {
+ errno = EDOM;
+ goto out;
+ }
+ break;
+ default:
+ /* No support for other types (yet). */
+ errno = EDOM;
+ goto out;
+ }
+
+ ret = xencall5(fshdl->xcall, __HYPERVISOR_hypfs_op,
+ XEN_HYPFS_OP_write_contents,
+ (unsigned long)path_buf, path_sz,
+ (unsigned long)buf, sz);
+
+ out:
+ saved_errno = errno;
+ xencall_free_buffer(fshdl->xcall, path_buf);
+ xencall_free_buffer(fshdl->xcall, buf);
+ errno = saved_errno;
+ return ret;
+}
diff --git a/tools/libs/hypfs/include/xenhypfs.h b/tools/libs/hypfs/include/xenhypfs.h
new file mode 100644
index 0000000000..29c69712ce
--- /dev/null
+++ b/tools/libs/hypfs/include/xenhypfs.h
@@ -0,0 +1,75 @@
+/*
+ * Copyright (c) 2019 SUSE Software Solutions Germany GmbH
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation;
+ * version 2.1 of the License.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef XENHYPFS_H
+#define XENHYPFS_H
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <sys/types.h>
+
+/* Callers who don't care don't need to #include <xentoollog.h> */
+struct xentoollog_logger;
+
+typedef struct xenhypfs_handle xenhypfs_handle;
+
+struct xenhypfs_dirent {
+ char *name;
+ size_t size;
+ enum {
+ xenhypfs_type_dir,
+ xenhypfs_type_blob,
+ xenhypfs_type_string,
+ xenhypfs_type_uint,
+ xenhypfs_type_int,
+ xenhypfs_type_bool
+ } type;
+ enum {
+ xenhypfs_enc_plain,
+ xenhypfs_enc_gzip
+ } encoding;
+ bool is_writable;
+};
+
+xenhypfs_handle *xenhypfs_open(struct xentoollog_logger *logger,
+ unsigned int open_flags);
+int xenhypfs_close(xenhypfs_handle *fshdl);
+
+/* Returned buffer and dirent should be freed via free(). */
+void *xenhypfs_read_raw(xenhypfs_handle *fshdl, const char *path,
+ struct xenhypfs_dirent **dirent);
+
+/* Returned buffer should be freed via free(). */
+char *xenhypfs_read(xenhypfs_handle *fshdl, const char *path);
+
+/* Returned buffer should be freed via free(). */
+struct xenhypfs_dirent *xenhypfs_readdir(xenhypfs_handle *fshdl,
+ const char *path,
+ unsigned int *num_entries);
+
+int xenhypfs_write(xenhypfs_handle *fshdl, const char *path, const char *val);
+
+#endif /* XENHYPFS_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libs/hypfs/libxenhypfs.map b/tools/libs/hypfs/libxenhypfs.map
new file mode 100644
index 0000000000..47f1edda3e
--- /dev/null
+++ b/tools/libs/hypfs/libxenhypfs.map
@@ -0,0 +1,10 @@
+VERS_1.0 {
+ global:
+ xenhypfs_open;
+ xenhypfs_close;
+ xenhypfs_read_raw;
+ xenhypfs_read;
+ xenhypfs_readdir;
+ xenhypfs_write;
+ local: *; /* Do not expose anything by default */
+};
diff --git a/tools/libs/hypfs/xenhypfs.pc.in b/tools/libs/hypfs/xenhypfs.pc.in
new file mode 100644
index 0000000000..92a262c7a2
--- /dev/null
+++ b/tools/libs/hypfs/xenhypfs.pc.in
@@ -0,0 +1,10 @@
+prefix=@@prefix@@
+includedir=@@incdir@@
+libdir=@@libdir@@
+
+Name: Xenhypfs
+Description: The Xenhypfs library for Xen hypervisor
+Version: @@version@@
+Cflags: -I${includedir} @@cflagslocal@@
+Libs: @@libsflag@@${libdir} -lxenhypfs
+Requires.private: xentoolcore,xentoollog,xencall,z
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 5/8] tools: add xenfs tool
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
` (3 preceding siblings ...)
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 4/8] libs: add libxenhypfs Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 6/8] xen: provide version information in hypfs Juergen Gross
` (2 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jan Beulich
Add the xenfs tool for accessing the hypervisor filesystem.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
---
V1:
- rename to xenhypfs
- don't use "--" for subcommands
- add write support
V2:
- escape non-printable characters per default with cat subcommand
(Ian Jackson)
- add -b option to cat subcommand (Ian Jackson)
- add man page
V3:
- adapt to new hypfs interface
---
.gitignore | 1 +
docs/man/xenhypfs.1.pod | 61 ++++++++++++++++
tools/misc/Makefile | 6 ++
tools/misc/xenhypfs.c | 189 ++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 257 insertions(+)
create mode 100644 docs/man/xenhypfs.1.pod
create mode 100644 tools/misc/xenhypfs.c
diff --git a/.gitignore b/.gitignore
index e98c3f056d..fd5610718d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -367,6 +367,7 @@ tools/libxl/test_timedereg
tools/libxl/test_fdderegrace
tools/firmware/etherboot/eb-roms.h
tools/firmware/etherboot/gpxe-git-snapshot.tar.gz
+tools/misc/xenhypfs
tools/misc/xenwatchdogd
tools/misc/xen-hvmcrash
tools/misc/xen-lowmemd
diff --git a/docs/man/xenhypfs.1.pod b/docs/man/xenhypfs.1.pod
new file mode 100644
index 0000000000..37aa488fcc
--- /dev/null
+++ b/docs/man/xenhypfs.1.pod
@@ -0,0 +1,61 @@
+=head1 NAME
+
+xenhypfs - Xen tool to access Xen hypervisor file system
+
+=head1 SYNOPSIS
+
+B<xenhypfs> I<subcommand> [I<options>] [I<args>]
+
+=head1 DESCRIPTION
+
+The B<xenhypfs> program is used to access the Xen hypervisor file system.
+It can be used to show the available entries, to show their contents and
+(if allowed) to modify their contents.
+
+=head1 SUBCOMMANDS
+
+=over 4
+
+=item B<ls> I<path>
+
+List the available entries below I<path>.
+
+=item B<cat> [I<-b>] I<path>
+
+Show the contents of the entry specified by I<path>. Non-printable characters
+other than white space characters (like tab, new line) will be shown as
+B<\xnn> (B<nn> being a two digit hex number) unless the option B<-b> is
+specified.
+
+=item B<write> I<path> I<value>
+
+Set the contents of the entry specified by I<path> to I<value>.
+
+=item B<tree>
+
+Show all the entries of the file system as a tree.
+
+=back
+
+=head1 RETURN CODES
+
+=over 4
+
+=item B<0>
+
+Success
+
+=item B<1>
+
+Invalid usage (e.g. unknown subcommand, unknown option, missing parameter).
+
+=item B<2>
+
+Entry not found while traversing the tree.
+
+=item B<3>
+
+Access right violation.
+
+=back
+
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 63947bfadc..9fdb13597f 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -24,6 +24,7 @@ INSTALL_SBIN-$(CONFIG_X86) += xen-lowmemd
INSTALL_SBIN-$(CONFIG_X86) += xen-mfndump
INSTALL_SBIN-$(CONFIG_X86) += xen-ucode
INSTALL_SBIN += xencov
+INSTALL_SBIN += xenhypfs
INSTALL_SBIN += xenlockprof
INSTALL_SBIN += xenperf
INSTALL_SBIN += xenpm
@@ -86,6 +87,9 @@ xenperf: xenperf.o
xenpm: xenpm.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+xenhypfs: xenhypfs.o
+ $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenhypfs) $(APPEND_LDFLAGS)
+
xenlockprof: xenlockprof.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
@@ -94,6 +98,8 @@ xen-hptool.o: CFLAGS += -I$(XEN_ROOT)/tools/libxc $(CFLAGS_libxencall)
xen-hptool: xen-hptool.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
+xenhypfs.o: CFLAGS += $(CFLAGS_libxenhypfs)
+
# xen-mfndump incorrectly uses libxc internals
xen-mfndump.o: CFLAGS += -I$(XEN_ROOT)/tools/libxc $(CFLAGS_libxencall)
xen-mfndump: xen-mfndump.o
diff --git a/tools/misc/xenhypfs.c b/tools/misc/xenhypfs.c
new file mode 100644
index 0000000000..0b834bf4fa
--- /dev/null
+++ b/tools/misc/xenhypfs.c
@@ -0,0 +1,189 @@
+#define _GNU_SOURCE
+#include <ctype.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <xenhypfs.h>
+
+static struct xenhypfs_handle *hdl;
+
+static int usage(void)
+{
+ fprintf(stderr, "usage: xenhypfs ls <path>\n");
+ fprintf(stderr, " xenhypfs cat [-b] <path>\n");
+ fprintf(stderr, " xenhypfs write <path> <val>\n");
+ fprintf(stderr, " xenhypfs tree\n");
+
+ return 1;
+}
+
+static void xenhypfs_print_escaped(char *string)
+{
+ char *c;
+
+ for (c = string; *c; c++) {
+ if (isgraph(*c) || isspace(*c))
+ printf("%c", *c);
+ else
+ printf("\\x%02x", *c);
+ }
+ printf("\n");
+}
+
+static int xenhypfs_cat(int argc, char *argv[])
+{
+ int ret = 0;
+ char *result;
+ char *path;
+ bool bin = false;
+
+ switch (argc) {
+ case 1:
+ path = argv[0];
+ break;
+
+ case 2:
+ if (strcmp(argv[0], "-b"))
+ return usage();
+ bin = true;
+ path = argv[1];
+ break;
+
+ default:
+ return usage();
+ }
+
+ result = xenhypfs_read(hdl, path);
+ if (!result) {
+ perror("could not read");
+ ret = 3;
+ } else {
+ if (!bin)
+ printf("%s\n", result);
+ else
+ xenhypfs_print_escaped(result);
+ free(result);
+ }
+
+ return ret;
+}
+
+static int xenhypfs_wr(char *path, char *val)
+{
+ int ret;
+
+ ret = xenhypfs_write(hdl, path, val);
+ if (ret) {
+ perror("could not write");
+ ret = 3;
+ }
+
+ return ret;
+}
+
+static char *xenhypfs_type(struct xenhypfs_dirent *ent)
+{
+ char *res;
+
+ switch (ent->type) {
+ case xenhypfs_type_dir:
+ res = "<dir> ";
+ break;
+ case xenhypfs_type_blob:
+ res = "<blob> ";
+ break;
+ case xenhypfs_type_string:
+ res = "<string>";
+ break;
+ case xenhypfs_type_uint:
+ res = "<uint> ";
+ break;
+ case xenhypfs_type_int:
+ res = "<int> ";
+ break;
+ default:
+ res = "<\?\?\?> ";
+ break;
+ }
+
+ return res;
+}
+
+static int xenhypfs_ls(char *path)
+{
+ struct xenhypfs_dirent *ent;
+ unsigned int n, i;
+ int ret = 0;
+
+ ent = xenhypfs_readdir(hdl, path, &n);
+ if (!ent) {
+ perror("could not read dir");
+ ret = 3;
+ } else {
+ for (i = 0; i < n; i++)
+ printf("%s r%c %s\n", xenhypfs_type(ent + i),
+ ent[i].is_writable ? 'w' : '-', ent[i].name);
+
+ free(ent);
+ }
+
+ return ret;
+}
+
+static int xenhypfs_tree_sub(char *path, unsigned int depth)
+{
+ struct xenhypfs_dirent *ent;
+ unsigned int n, i;
+ int ret = 0;
+ char *p;
+
+ ent = xenhypfs_readdir(hdl, path, &n);
+ if (!ent)
+ return 2;
+
+ for (i = 0; i < n; i++) {
+ printf("%*s%s%s\n", depth * 2, "", ent[i].name,
+ ent[i].type == xenhypfs_type_dir ? "/" : "");
+ if (ent[i].type == xenhypfs_type_dir) {
+ asprintf(&p, "%s%s%s", path, (depth == 1) ? "" : "/", ent[i].name);
+ if (xenhypfs_tree_sub(p, depth + 1))
+ ret = 2;
+ }
+ }
+
+ free(ent);
+
+ return ret;
+}
+
+static int xenhypfs_tree(void)
+{
+ printf("/\n");
+
+ return xenhypfs_tree_sub("/", 1);
+}
+
+int main(int argc, char *argv[])
+{
+ int ret;
+
+ hdl = xenhypfs_open(NULL, 0);
+
+ if (!hdl) {
+ fprintf(stderr, "Could not open libxenhypfs\n");
+ ret = 2;
+ } else if (argc >= 3 && !strcmp(argv[1], "cat"))
+ ret = xenhypfs_cat(argc - 2, argv + 2);
+ else if (argc == 3 && !strcmp(argv[1], "ls"))
+ ret = xenhypfs_ls(argv[2]);
+ else if (argc == 4 && !strcmp(argv[1], "write"))
+ ret = xenhypfs_wr(argv[2], argv[3]);
+ else if (argc == 2 && !strcmp(argv[1], "tree"))
+ ret = xenhypfs_tree();
+ else
+ ret = usage();
+
+ xenhypfs_close(hdl);
+
+ return ret;
+}
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 6/8] xen: provide version information in hypfs
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
` (4 preceding siblings ...)
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 5/8] tools: add xenfs tool Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 7/8] xen: add /buildinfo/config entry to hypervisor filesystem Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs Juergen Gross
7 siblings, 0 replies; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jan Beulich
Provide version and compile information in /buildinfo/ node of the
Xen hypervisor file system. As this information is accessible by dom0
only no additional security problem arises.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V3:
- new patch
V4:
- add __read_mostly annotations (Jan Beulich)
---
docs/misc/hypfs-paths.pandoc | 45 ++++++++++++++++++++++++++++++++++++++++++++
xen/common/kernel.c | 45 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 90 insertions(+)
diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
index b9f50f6998..e392feff27 100644
--- a/docs/misc/hypfs-paths.pandoc
+++ b/docs/misc/hypfs-paths.pandoc
@@ -103,3 +103,48 @@ A populated Xen hypervisor file system might look like the following example:
#### /
The root of the hypervisor file system.
+
+#### /buildinfo/
+
+A directory containing static information generated while building the
+hypervisor.
+
+#### /buildinfo/changeset = STRING
+
+Git commit of the hypervisor.
+
+#### /buildinfo/compileinfo/
+
+A directory containing information about compilation of Xen.
+
+#### /buildinfo/compileinfo/compile_by = STRING
+
+Information who compiled the hypervisor.
+
+#### /buildinfo/compileinfo/compile_date = STRING
+
+Date of the hypervisor compilation.
+
+#### /buildinfo/compileinfo/compile_domain = STRING
+
+Information about the compile domain.
+
+#### /buildinfo/compileinfo/compiler = STRING
+
+The compiler used to build Xen.
+
+#### /buildinfo/version/
+
+A directory containing version information of the hypervisor.
+
+#### /buildinfo/version/extra = STRING
+
+Extra version information.
+
+#### /buildinfo/version/major = INTEGER
+
+The major version of Xen.
+
+#### /buildinfo/version/minor = INTEGER
+
+The minor version of Xen.
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 22941cec94..6ee0e64404 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -13,6 +13,7 @@
#include <xen/paging.h>
#include <xen/guest_access.h>
#include <xen/hypercall.h>
+#include <xen/hypfs.h>
#include <xsm/xsm.h>
#include <asm/current.h>
#include <public/version.h>
@@ -373,6 +374,50 @@ void __init do_initcalls(void)
(*call)();
}
+static unsigned int __read_mostly major_version;
+static unsigned int __read_mostly minor_version;
+
+static HYPFS_DIR_INIT(buildinfo, "buildinfo");
+static HYPFS_DIR_INIT(compileinfo, "compileinfo");
+static HYPFS_DIR_INIT(version, "version");
+static HYPFS_UINT_INIT(major, "major", major_version);
+static HYPFS_UINT_INIT(minor, "minor", minor_version);
+static HYPFS_STRING_INIT(changeset, "changeset");
+static HYPFS_STRING_INIT(compiler, "compiler");
+static HYPFS_STRING_INIT(compile_by, "compile_by");
+static HYPFS_STRING_INIT(compile_date, "compile_date");
+static HYPFS_STRING_INIT(compile_domain, "compile_domain");
+static HYPFS_STRING_INIT(extra, "extra");
+
+static int __init buildinfo_init(void)
+{
+ hypfs_add_dir(&hypfs_root, &buildinfo, true);
+
+ hypfs_string_set(&changeset, xen_changeset());
+ hypfs_add_leaf(&buildinfo, &changeset, true);
+
+ hypfs_add_dir(&buildinfo, &compileinfo, true);
+ hypfs_string_set(&compiler, xen_compiler());
+ hypfs_string_set(&compile_by, xen_compile_by());
+ hypfs_string_set(&compile_date, xen_compile_date());
+ hypfs_string_set(&compile_domain, xen_compile_domain());
+ hypfs_add_leaf(&compileinfo, &compiler, true);
+ hypfs_add_leaf(&compileinfo, &compile_by, true);
+ hypfs_add_leaf(&compileinfo, &compile_date, true);
+ hypfs_add_leaf(&compileinfo, &compile_domain, true);
+
+ major_version = xen_major_version();
+ minor_version = xen_minor_version();
+ hypfs_add_dir(&buildinfo, &version, true);
+ hypfs_string_set(&extra, xen_extra_version());
+ hypfs_add_leaf(&version, &extra, true);
+ hypfs_add_leaf(&version, &major, true);
+ hypfs_add_leaf(&version, &minor, true);
+
+ return 0;
+}
+__initcall(buildinfo_init);
+
# define DO(fn) long do_##fn
#endif
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 7/8] xen: add /buildinfo/config entry to hypervisor filesystem
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
` (5 preceding siblings ...)
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 6/8] xen: provide version information in hypfs Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 15:57 ` Jan Beulich
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs Juergen Gross
7 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jan Beulich
Add the /buildinfo/config entry to the hypervisor filesystem. This
entry contains the .config file used to build the hypervisor.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- store data in gzip format
- use binfile mechanism to create data file
- move code to kernel.c
---
.gitignore | 2 ++
docs/misc/hypfs-paths.pandoc | 4 ++++
xen/common/Makefile | 12 ++++++++++++
xen/common/kernel.c | 10 ++++++++++
xen/include/xen/kernel.h | 3 +++
5 files changed, 31 insertions(+)
diff --git a/.gitignore b/.gitignore
index fd5610718d..bc8e053ccb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -297,6 +297,8 @@ xen/arch/*/efi/boot.c
xen/arch/*/efi/compat.c
xen/arch/*/efi/efi.h
xen/arch/*/efi/runtime.c
+xen/common/config_data.S
+xen/common/config.gz
xen/include/headers*.chk
xen/include/asm
xen/include/asm-*/asm-offsets.h
diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
index e392feff27..1faebcccbc 100644
--- a/docs/misc/hypfs-paths.pandoc
+++ b/docs/misc/hypfs-paths.pandoc
@@ -133,6 +133,10 @@ Information about the compile domain.
The compiler used to build Xen.
+#### /buildinfo/config = STRING
+
+The contents of the `xen/.config` file at the time of the hypervisor build.
+
#### /buildinfo/version/
A directory containing version information of the hypervisor.
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 3a2c1ae690..72fb790c57 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_ARGO) += argo.o
obj-y += bitmap.o
obj-y += bsearch.o
+obj-y += config_data.o
obj-$(CONFIG_CORE_PARKING) += core_parking.o
obj-y += cpu.o
obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
@@ -73,3 +74,14 @@ subdir-$(CONFIG_UBSAN) += ubsan
subdir-$(CONFIG_NEEDS_LIBELF) += libelf
subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
+
+config.gz: ../.config
+ gzip -c $< >$@
+
+config_data.o: config.gz
+
+config_data.S: $(XEN_ROOT)/xen/tools/binfile
+ $(XEN_ROOT)/xen/tools/binfile $@ config.gz xen_config_data
+
+clean::
+ rm config_data.S config.gz 2>/dev/null || true
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 6ee0e64404..3d496bb9e6 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -388,6 +388,13 @@ static HYPFS_STRING_INIT(compile_by, "compile_by");
static HYPFS_STRING_INIT(compile_date, "compile_date");
static HYPFS_STRING_INIT(compile_domain, "compile_domain");
static HYPFS_STRING_INIT(extra, "extra");
+static struct hypfs_entry_leaf config = {
+ .e.type = XEN_HYPFS_TYPE_STRING,
+ .e.encoding = XEN_HYPFS_ENC_GZIP,
+ .e.name = "config",
+ .e.read = hypfs_read_leaf,
+ .content = &xen_config_data
+};
static int __init buildinfo_init(void)
{
@@ -414,6 +421,9 @@ static int __init buildinfo_init(void)
hypfs_add_leaf(&version, &major, true);
hypfs_add_leaf(&version, &minor, true);
+ config.e.size = xen_config_data_size;
+ hypfs_add_leaf(&buildinfo, &config, true);
+
return 0;
}
__initcall(buildinfo_init);
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64da9f..2f883031f9 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -100,5 +100,8 @@ extern enum system_state {
bool_t is_active_kernel_text(unsigned long addr);
+extern char xen_config_data;
+extern unsigned int xen_config_data_size;
+
#endif /* _LINUX_KERNEL_H */
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
` (6 preceding siblings ...)
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 7/8] xen: add /buildinfo/config entry to hypervisor filesystem Juergen Gross
@ 2020-02-19 8:11 ` Juergen Gross
2020-02-19 16:44 ` Jan Beulich
7 siblings, 1 reply; 24+ messages in thread
From: Juergen Gross @ 2020-02-19 8:11 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Kevin Tian, Stefano Stabellini, Julien Grall,
Jun Nakajima, Wei Liu, Konrad Rzeszutek Wilk, George Dunlap,
Andrew Cooper, Ian Jackson, Jan Beulich, Volodymyr Babchuk,
Roger Pau Monné
Add support to read and modify values of hypervisor runtime parameters
via the hypervisor file system.
As runtime parameters can be modified via a sysctl, too, this path has
to take the hypfs rw_lock as writer.
For custom runtime parameters the resulting parameter value needs to
be stored in a string buffer for being consumable by the file system.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- complete rework
- support custom parameters, too
- support parameter writing
---
docs/misc/hypfs-paths.pandoc | 9 ++++++
xen/arch/arm/xen.lds.S | 5 +++
xen/arch/x86/hvm/vmx/vmcs.c | 17 +++++++++-
xen/arch/x86/pv/domain.c | 7 ++++-
xen/arch/x86/xen.lds.S | 5 +++
xen/common/grant_table.c | 17 +++++++---
xen/common/hypfs.c | 38 +++++++++++++++++++++++
xen/common/kernel.c | 23 +++++++++++++-
xen/drivers/char/console.c | 49 +++++++++++++++++++++++++----
xen/include/xen/hypfs.h | 4 +++
xen/include/xen/param.h | 74 +++++++++++++++++++++++++++++++++++++++-----
11 files changed, 226 insertions(+), 22 deletions(-)
diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc
index 1faebcccbc..7d03852ccd 100644
--- a/docs/misc/hypfs-paths.pandoc
+++ b/docs/misc/hypfs-paths.pandoc
@@ -152,3 +152,12 @@ The major version of Xen.
#### /buildinfo/version/minor = INTEGER
The minor version of Xen.
+
+#### /params/
+
+A directory of runtime parameters.
+
+#### /params/*
+
+The single parameters. The description of the different parameters can be
+found in `docs/misc/xen-command-line.pandoc`.
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index a497f6a48d..0061a8dfea 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -89,6 +89,11 @@ SECTIONS
__start_schedulers_array = .;
*(.data.schedulers)
__end_schedulers_array = .;
+
+ . = ALIGN(8);
+ __paramhypfs_start = .;
+ *(.data.paramhypfs)
+ __paramhypfs_end = .;
*(.data.rel)
*(.data.rel.*)
CONSTRUCTORS
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 65445afeb0..36ddccfb73 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -70,6 +70,17 @@ integer_param("ple_window", ple_window);
static bool __read_mostly opt_ept_pml = true;
static s8 __read_mostly opt_ept_ad = -1;
int8_t __read_mostly opt_ept_exec_sp = -1;
+static char opt_ept_setting[16] = "pml=1";
+
+
+static void update_ept_param(void)
+{
+ snprintf(opt_ept_setting, sizeof(opt_ept_setting), "pml=%d", opt_ept_pml);
+ if ( opt_ept_ad >= 0 )
+ param_append_str(opt_ept_setting, ",ad=%d", opt_ept_ad);
+ if ( opt_ept_exec_sp >= 0 )
+ param_append_str(opt_ept_setting, ",exec-sp=%d", opt_ept_exec_sp);
+}
static int __init parse_ept_param(const char *s)
{
@@ -93,6 +104,8 @@ static int __init parse_ept_param(const char *s)
s = ss + 1;
} while ( *ss );
+ update_ept_param();
+
return rc;
}
custom_param("ept", parse_ept_param);
@@ -115,6 +128,8 @@ static int parse_ept_param_runtime(const char *s)
opt_ept_exec_sp = val;
+ update_ept_param();
+
rcu_read_lock(&domlist_read_lock);
for_each_domain ( d )
{
@@ -144,7 +159,7 @@ static int parse_ept_param_runtime(const char *s)
return 0;
}
-custom_runtime_only_param("ept", parse_ept_param_runtime);
+custom_runtime_only_param("ept", parse_ept_param_runtime, opt_ept_setting);
/* Dynamic (run-time adjusted) execution control flags. */
u32 vmx_pin_based_exec_control __read_mostly;
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 0b37653b12..71e1f69ff8 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -22,6 +22,7 @@ static __read_mostly enum {
PCID_XPTI,
PCID_NOXPTI
} opt_pcid = PCID_XPTI;
+static char opt_pcid_val[7] = "xpti";
static int parse_pcid(const char *s)
{
@@ -31,10 +32,12 @@ static int parse_pcid(const char *s)
{
case 0:
opt_pcid = PCID_OFF;
+ snprintf(opt_pcid_val, sizeof(opt_pcid_val), "off");
break;
case 1:
opt_pcid = PCID_ALL;
+ snprintf(opt_pcid_val, sizeof(opt_pcid_val), "on");
break;
default:
@@ -42,10 +45,12 @@ static int parse_pcid(const char *s)
{
case 0:
opt_pcid = PCID_NOXPTI;
+ snprintf(opt_pcid_val, sizeof(opt_pcid_val), "noxpti");
break;
case 1:
opt_pcid = PCID_XPTI;
+ snprintf(opt_pcid_val, sizeof(opt_pcid_val), "xpti");
break;
default:
@@ -57,7 +62,7 @@ static int parse_pcid(const char *s)
return rc;
}
-custom_runtime_param("pcid", parse_pcid);
+custom_runtime_param("pcid", parse_pcid, opt_pcid_val);
static void noreturn continue_nonidle_domain(struct vcpu *v)
{
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 7f9459d683..21a37f0f57 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -279,6 +279,11 @@ SECTIONS
__start_schedulers_array = .;
*(.data.schedulers)
__end_schedulers_array = .;
+
+ . = ALIGN(8);
+ __paramhypfs_start = .;
+ *(.data.paramhypfs)
+ __paramhypfs_end = .;
} :text
DECL_SECTION(.data) {
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index bc37acae0e..e8dcc9e568 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -85,8 +85,10 @@ struct grant_table {
struct grant_table_arch arch;
};
+#define PAR_VAL_SZ 12
+
static int parse_gnttab_limit(const char *param, const char *arg,
- unsigned int *valp)
+ unsigned int *valp, char *par_val)
{
const char *e;
unsigned long val;
@@ -99,28 +101,33 @@ static int parse_gnttab_limit(const char *param, const char *arg,
return -ERANGE;
*valp = val;
+ snprintf(par_val, PAR_VAL_SZ, "%lu", val);
return 0;
}
unsigned int __read_mostly opt_max_grant_frames = 64;
+static char gnttab_max_frames_val[PAR_VAL_SZ] = "64";
static int parse_gnttab_max_frames(const char *arg)
{
return parse_gnttab_limit("gnttab_max_frames", arg,
- &opt_max_grant_frames);
+ &opt_max_grant_frames, gnttab_max_frames_val);
}
-custom_runtime_param("gnttab_max_frames", parse_gnttab_max_frames);
+custom_runtime_param("gnttab_max_frames", parse_gnttab_max_frames,
+ gnttab_max_frames_val);
static unsigned int __read_mostly opt_max_maptrack_frames = 1024;
+static char max_maptrack_frames_val[PAR_VAL_SZ] = "1024";
static int parse_gnttab_max_maptrack_frames(const char *arg)
{
return parse_gnttab_limit("gnttab_max_maptrack_frames", arg,
- &opt_max_maptrack_frames);
+ &opt_max_maptrack_frames,
+ max_maptrack_frames_val);
}
custom_runtime_param("gnttab_max_maptrack_frames",
- parse_gnttab_max_maptrack_frames);
+ parse_gnttab_max_maptrack_frames, max_maptrack_frames_val);
#ifndef GNTTAB_MAX_VERSION
#define GNTTAB_MAX_VERSION 2
diff --git a/xen/common/hypfs.c b/xen/common/hypfs.c
index 0a32d45979..e568338002 100644
--- a/xen/common/hypfs.c
+++ b/xen/common/hypfs.c
@@ -10,6 +10,7 @@
#include <xen/hypercall.h>
#include <xen/hypfs.h>
#include <xen/lib.h>
+#include <xen/param.h>
#include <xen/rwlock.h>
#include <public/hypfs.h>
@@ -289,6 +290,33 @@ int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
return 0;
}
+int hypfs_write_custom(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ struct param_hypfs *p;
+ char *buf;
+ int ret;
+
+ buf = xzalloc_array(char, ulen);
+ if ( !buf )
+ return -ENOMEM;
+
+ ret = -EFAULT;
+ if ( copy_from_guest(buf, uaddr, ulen) )
+ goto out;
+
+ ret = -EDOM;
+ if ( buf[ulen - 1] )
+ goto out;
+
+ p = container_of(leaf, struct param_hypfs, hypfs);
+ ret = p->param->par.func(buf);
+
+ out:
+ xfree(buf);
+ return ret;
+}
+
static int hypfs_write(struct hypfs_entry *entry,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
{
@@ -355,3 +383,13 @@ long do_hypfs_op(unsigned int cmd,
return ret;
}
+
+void hypfs_write_lock(void)
+{
+ write_lock(&hypfs_lock);
+}
+
+void hypfs_write_unlock(void)
+{
+ write_unlock(&hypfs_lock);
+}
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 3d496bb9e6..e62a1680c7 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -198,7 +198,13 @@ static void __init _cmdline_parse(const char *cmdline)
int runtime_parse(const char *line)
{
- return parse_params(line, __param_start, __param_end);
+ int ret;
+
+ hypfs_write_lock();
+ ret = parse_params(line, __param_start, __param_end);
+ hypfs_write_unlock();
+
+ return ret;
}
/**
@@ -428,6 +434,21 @@ static int __init buildinfo_init(void)
}
__initcall(buildinfo_init);
+static HYPFS_DIR_INIT(params, "params");
+
+static int __init param_init(void)
+{
+ struct param_hypfs *param;
+
+ hypfs_add_dir(&hypfs_root, ¶ms, true);
+
+ for ( param = __paramhypfs_start; param < __paramhypfs_end; param++ )
+ hypfs_add_leaf(¶ms, ¶m->hypfs, true);
+
+ return 0;
+}
+__initcall(param_init);
+
# define DO(fn) long do_##fn
#endif
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 913ae1b66a..5440145549 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -78,9 +78,11 @@ enum con_timestamp_mode
};
static enum con_timestamp_mode __read_mostly opt_con_timestamp_mode = TSM_NONE;
+static char con_timestamp_mode_val[7] = "none";
static int parse_console_timestamps(const char *s);
-custom_runtime_param("console_timestamps", parse_console_timestamps);
+custom_runtime_param("console_timestamps", parse_console_timestamps,
+ con_timestamp_mode_val);
/* conring_size: allows a large console ring than default (16kB). */
static uint32_t __initdata opt_conring_size;
@@ -118,13 +120,17 @@ static DEFINE_SPINLOCK(console_lock);
#ifdef NDEBUG
#define XENLOG_UPPER_THRESHOLD 2 /* Do not print INFO and DEBUG */
#define XENLOG_LOWER_THRESHOLD 2 /* Always print ERR and WARNING */
+#define XENLOG_DEFAULT_VAL "warning/warning"
#define XENLOG_GUEST_UPPER_THRESHOLD 2 /* Do not print INFO and DEBUG */
#define XENLOG_GUEST_LOWER_THRESHOLD 0 /* Rate-limit ERR and WARNING */
+#define XENLOG_GUEST_DEFAULT_VAL "none/warning"
#else
#define XENLOG_UPPER_THRESHOLD 4 /* Do not discard anything */
#define XENLOG_LOWER_THRESHOLD 4 /* Print everything */
+#define XENLOG_DEFAULT_VAL "all/all"
#define XENLOG_GUEST_UPPER_THRESHOLD 4 /* Do not discard anything */
#define XENLOG_GUEST_LOWER_THRESHOLD 4 /* Print everything */
+#define XENLOG_GUEST_DEFAULT_VAL "all/all"
#endif
/*
* The XENLOG_DEFAULT is the default given to printks that
@@ -133,16 +139,20 @@ static DEFINE_SPINLOCK(console_lock);
#define XENLOG_DEFAULT 1 /* XENLOG_WARNING */
#define XENLOG_GUEST_DEFAULT 1 /* XENLOG_WARNING */
+#define LOGLVL_VAL_SZ 16
static int __read_mostly xenlog_upper_thresh = XENLOG_UPPER_THRESHOLD;
static int __read_mostly xenlog_lower_thresh = XENLOG_LOWER_THRESHOLD;
+static char xenlog_val[LOGLVL_VAL_SZ] = XENLOG_DEFAULT_VAL;
static int __read_mostly xenlog_guest_upper_thresh =
XENLOG_GUEST_UPPER_THRESHOLD;
static int __read_mostly xenlog_guest_lower_thresh =
XENLOG_GUEST_LOWER_THRESHOLD;
+static char xenlog_guest_val[LOGLVL_VAL_SZ] = XENLOG_GUEST_DEFAULT_VAL;
static int parse_loglvl(const char *s);
static int parse_guest_loglvl(const char *s);
+static char *lvl2opt[] = { "none", "error", "warning", "info", "all" };
/*
* <lvl> := none|error|warning|info|debug|all
* loglvl=<lvl_print_always>[/<lvl_print_ratelimit>]
@@ -151,8 +161,8 @@ static int parse_guest_loglvl(const char *s);
* Similar definitions for guest_loglvl, but applies to guest tracing.
* Defaults: loglvl=warning ; guest_loglvl=none/warning
*/
-custom_runtime_param("loglvl", parse_loglvl);
-custom_runtime_param("guest_loglvl", parse_guest_loglvl);
+custom_runtime_param("loglvl", parse_loglvl, xenlog_val);
+custom_runtime_param("guest_loglvl", parse_guest_loglvl, xenlog_guest_val);
static atomic_t print_everything = ATOMIC_INIT(0);
@@ -173,7 +183,7 @@ static int __parse_loglvl(const char *s, const char **ps)
return 2; /* sane fallback */
}
-static int _parse_loglvl(const char *s, int *lower, int *upper)
+static int _parse_loglvl(const char *s, int *lower, int *upper, char *val)
{
*lower = *upper = __parse_loglvl(s, &s);
if ( *s == '/' )
@@ -181,18 +191,21 @@ static int _parse_loglvl(const char *s, int *lower, int *upper)
if ( *upper < *lower )
*upper = *lower;
+ snprintf(val, LOGLVL_VAL_SZ, "%s/%s", lvl2opt[*lower], lvl2opt[*upper]);
+
return *s ? -EINVAL : 0;
}
static int parse_loglvl(const char *s)
{
- return _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh);
+ return _parse_loglvl(s, &xenlog_lower_thresh, &xenlog_upper_thresh,
+ xenlog_val);
}
static int parse_guest_loglvl(const char *s)
{
return _parse_loglvl(s, &xenlog_guest_lower_thresh,
- &xenlog_guest_upper_thresh);
+ &xenlog_guest_upper_thresh, xenlog_guest_val);
}
static char *loglvl_str(int lvl)
@@ -731,22 +744,46 @@ static int parse_console_timestamps(const char *s)
{
case 0:
opt_con_timestamp_mode = TSM_NONE;
+ snprintf(con_timestamp_mode_val, sizeof(con_timestamp_mode_val),
+ "none");
return 0;
case 1:
opt_con_timestamp_mode = TSM_DATE;
+ snprintf(con_timestamp_mode_val, sizeof(con_timestamp_mode_val),
+ "date");
return 0;
}
if ( *s == '\0' || /* Compat for old booleanparam() */
!strcmp(s, "date") )
+ {
opt_con_timestamp_mode = TSM_DATE;
+ snprintf(con_timestamp_mode_val, sizeof(con_timestamp_mode_val),
+ "date");
+ }
else if ( !strcmp(s, "datems") )
+ {
opt_con_timestamp_mode = TSM_DATE_MS;
+ snprintf(con_timestamp_mode_val, sizeof(con_timestamp_mode_val),
+ "datems");
+ }
else if ( !strcmp(s, "boot") )
+ {
opt_con_timestamp_mode = TSM_BOOT;
+ snprintf(con_timestamp_mode_val, sizeof(con_timestamp_mode_val),
+ "boot");
+ }
else if ( !strcmp(s, "raw") )
+ {
opt_con_timestamp_mode = TSM_RAW;
+ snprintf(con_timestamp_mode_val, sizeof(con_timestamp_mode_val),
+ "raw");
+ }
else if ( !strcmp(s, "none") )
+ {
opt_con_timestamp_mode = TSM_NONE;
+ snprintf(con_timestamp_mode_val, sizeof(con_timestamp_mode_val),
+ "none");
+ }
else
return -EINVAL;
diff --git a/xen/include/xen/hypfs.h b/xen/include/xen/hypfs.h
index 5b26b2e611..2b6ed8ae23 100644
--- a/xen/include/xen/hypfs.h
+++ b/xen/include/xen/hypfs.h
@@ -104,5 +104,9 @@ int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen);
int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen);
+int hypfs_write_custom(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen);
+void hypfs_write_lock(void);
+void hypfs_write_unlock(void);
#endif /* __XEN_HYPFS_H__ */
diff --git a/xen/include/xen/param.h b/xen/include/xen/param.h
index 75471eb4ad..b4ef3e7a22 100644
--- a/xen/include/xen/param.h
+++ b/xen/include/xen/param.h
@@ -1,6 +1,7 @@
#ifndef _XEN_PARAM_H
#define _XEN_PARAM_H
+#include <xen/hypfs.h>
#include <xen/init.h>
/*
@@ -23,10 +24,17 @@ struct kernel_param {
} par;
};
+struct param_hypfs {
+ const struct kernel_param *param;
+ struct hypfs_entry_leaf hypfs;
+};
+
extern const struct kernel_param __setup_start[], __setup_end[];
extern const struct kernel_param __param_start[], __param_end[];
+extern struct param_hypfs __paramhypfs_start[], __paramhypfs_end[];
#define __dataparam __used_section(".data.param")
+#define __paramhypfs __used_section(".data.paramhypfs")
#define __param(att) static const att \
__attribute__((__aligned__(sizeof(void *)))) struct kernel_param
@@ -76,40 +84,87 @@ extern const struct kernel_param __param_start[], __param_end[];
.type = OPT_IGNORE }
#define __rtparam __param(__dataparam)
+#define __paramfs static __paramhypfs \
+ __attribute__((__aligned__(sizeof(void *)))) struct param_hypfs
-#define custom_runtime_only_param(_name, _var) \
+#define custom_runtime_only_param(_name, _var, contvar) \
__rtparam __rtpar_##_var = \
{ .name = _name, \
.type = OPT_CUSTOM, \
- .par.func = _var }
+ .par.func = _var }; \
+ __paramfs __parfs_##_var = \
+ { .param = &__rtpar_##_var, \
+ .hypfs.e.type = XEN_HYPFS_TYPE_STRING, \
+ .hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .hypfs.e.name = _name, \
+ .hypfs.e.size = sizeof(contvar), \
+ .hypfs.e.read = hypfs_read_leaf, \
+ .hypfs.e.write = hypfs_write_custom, \
+ .hypfs.content = &contvar }
#define boolean_runtime_only_param(_name, _var) \
__rtparam __rtpar_##_var = \
{ .name = _name, \
.type = OPT_BOOL, \
.len = sizeof(_var), \
- .par.var = &_var }
+ .par.var = &_var }; \
+ __paramfs __parfs_##_var = \
+ { .param = &__rtpar_##_var, \
+ .hypfs.e.type = XEN_HYPFS_TYPE_UINT, \
+ .hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .hypfs.e.name = _name, \
+ .hypfs.e.size = sizeof(_var), \
+ .hypfs.e.read = hypfs_read_leaf, \
+ .hypfs.e.write = hypfs_write_bool, \
+ .hypfs.content = &_var }
#define integer_runtime_only_param(_name, _var) \
__rtparam __rtpar_##_var = \
{ .name = _name, \
.type = OPT_UINT, \
.len = sizeof(_var), \
- .par.var = &_var }
+ .par.var = &_var }; \
+ __paramfs __parfs_##_var = \
+ { .param = &__rtpar_##_var, \
+ .hypfs.e.type = XEN_HYPFS_TYPE_UINT, \
+ .hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .hypfs.e.name = _name, \
+ .hypfs.e.size = sizeof(_var), \
+ .hypfs.e.read = hypfs_read_leaf, \
+ .hypfs.e.write = hypfs_write_leaf, \
+ .hypfs.content = &_var }
#define size_runtime_only_param(_name, _var) \
__rtparam __rtpar_##_var = \
{ .name = _name, \
.type = OPT_SIZE, \
.len = sizeof(_var), \
- .par.var = &_var }
+ .par.var = &_var }; \
+ __paramfs __parfs_##_var = \
+ { .param = &__rtpar_##_var, \
+ .hypfs.e.type = XEN_HYPFS_TYPE_UINT, \
+ .hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .hypfs.e.name = _name, \
+ .hypfs.e.size = sizeof(_var), \
+ .hypfs.e.read = hypfs_read_leaf, \
+ .hypfs.e.write = hypfs_write_leaf, \
+ .hypfs.content = &_var }
#define string_runtime_only_param(_name, _var) \
__rtparam __rtpar_##_var = \
{ .name = _name, \
.type = OPT_STR, \
.len = sizeof(_var), \
- .par.var = &_var }
+ .par.var = &_var }; \
+ __paramfs __parfs_##_var = \
+ { .param = &__rtpar_##_var, \
+ .hypfs.e.type = XEN_HYPFS_TYPE_STRING, \
+ .hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
+ .hypfs.e.name = _name, \
+ .hypfs.e.size = sizeof(_var), \
+ .hypfs.e.read = hypfs_read_leaf, \
+ .hypfs.e.write = hypfs_write_leaf, \
+ .hypfs.content = &_var }
-#define custom_runtime_param(_name, _var) \
+#define custom_runtime_param(_name, _var, contvar) \
custom_param(_name, _var); \
- custom_runtime_only_param(_name, _var)
+ custom_runtime_only_param(_name, _var, contvar)
#define boolean_runtime_param(_name, _var) \
boolean_param(_name, _var); \
boolean_runtime_only_param(_name, _var)
@@ -123,4 +178,7 @@ extern const struct kernel_param __param_start[], __param_end[];
string_param(_name, _var); \
string_runtime_only_param(_name, _var)
+#define param_append_str(var, fmt, val) \
+ snprintf(var + strlen(var), sizeof(var) - strlen(var), fmt, val)
+
#endif /* _XEN_PARAM_H */
--
2.16.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 1/8] xen: add a generic way to include binary files as variables
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 1/8] xen: add a generic way to include binary files as variables Juergen Gross
@ 2020-02-19 12:05 ` Wei Liu
0 siblings, 0 replies; 24+ messages in thread
From: Wei Liu @ 2020-02-19 12:05 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
xen-devel, Daniel De Graaf
On Wed, Feb 19, 2020 at 09:11:19AM +0100, Juergen Gross wrote:
> Add a new script xen/tools/binfile for including a binary file at build
> time being usable via a pointer and a size variable in the hypervisor.
>
> Make use of that generic tool in xsm.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support Juergen Gross
@ 2020-02-19 15:49 ` Jan Beulich
2020-02-19 16:19 ` Julien Grall
2020-02-20 7:27 ` Jürgen Groß
2020-02-19 16:26 ` Julien Grall
1 sibling, 2 replies; 24+ messages in thread
From: Jan Beulich @ 2020-02-19 15:49 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
On 19.02.2020 09:11, Juergen Gross wrote:
> +static int hypfs_get_path_user(char *buf,
> + XEN_GUEST_HANDLE_PARAM(const_char) uaddr,
> + unsigned long ulen)
> +{
> + if ( ulen > XEN_HYPFS_MAX_PATHLEN )
> + return -EINVAL;
> +
> + if ( copy_from_guest(buf, uaddr, ulen) )
> + return -EFAULT;
> +
> + if ( buf[ulen - 1] )
> + return -EINVAL;
I (still, but I may not have said so before) wonder whether
memchr(buf, 0, ulen) != buf + ulen - 1 wouldn't be better here.
> +int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
> +{
> + char *buf;
> + int ret;
> +
> + if ( ulen > leaf->e.size )
> + return -ENOSPC;
Okay, this makes sure you don't overrun the internal buffer.
What about the opposite mismatch (ulen < leaf->e.size)? The
result, except perhaps for (nul-terminated) strings, is not
going to be very useful, at the very least.
> + buf = xmalloc_array(char, ulen);
> + if ( !buf )
> + return -ENOMEM;
> +
> + ret = -EFAULT;
> + if ( copy_from_guest(buf, uaddr, ulen) )
> + goto out;
> +
> + ret = -EINVAL;
> + if ( leaf->e.type == XEN_HYPFS_TYPE_STRING && buf[ulen] )
buf[ulen - 1] I guess? Or, as above, memchr() again?
> +int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
> +{
> + union {
> + char buf[8];
> + uint8_t u8;
> + uint16_t u16;
> + uint32_t u32;
> + uint64_t u64;
> + } u;
> +
> + ASSERT(leaf->e.type == XEN_HYPFS_TYPE_UINT && leaf->e.size <= 8);
> +
> + if ( ulen != leaf->e.size )
> + return -EDOM;
> +
> + if ( copy_from_guest(u.buf, uaddr, ulen) )
> + return -EFAULT;
> +
> + switch ( leaf->e.size )
> + {
> + case 1:
> + *(uint8_t *)leaf->write_ptr = !!u.u8;
> + break;
> + case 2:
> + *(uint16_t *)leaf->write_ptr = !!u.u16;
> + break;
> + case 4:
> + *(uint32_t *)leaf->write_ptr = !!u.u32;
> + break;
> + case 8:
> + *(uint64_t *)leaf->write_ptr = !!u.u64;
> + break;
> + }
Looking at this again, is there really a need for uint64_t support
here? I.e. can't you cap at unsigned long (or even unsigned int),
and perhaps additionally avoid use of fixed width types here
altogether (some trickery may be needed for 32-bit's
sizeof(long) == sizeof(int))?
> --- /dev/null
> +++ b/xen/include/public/hypfs.h
> @@ -0,0 +1,127 @@
> +/******************************************************************************
> + * Xen Hypervisor Filesystem
> + *
> + * Copyright (c) 2019, SUSE Software Solutions Germany GmbH
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __XEN_PUBLIC_HYPFS_H__
> +#define __XEN_PUBLIC_HYPFS_H__
> +
> +#include "xen.h"
> +
> +/*
> + * Definitions for the __HYPERVISOR_hypfs_op hypercall.
> + */
> +
> +/* Highest version number of the hypfs interface currently defined. */
> +#define XEN_HYPFS_VERSION 1
> +
> +/* Maximum length of a path in the filesystem. */
> +#define XEN_HYPFS_MAX_PATHLEN 1024
I think it would be nice if the blank padding here matched that
of the other #define-s up and down from here.
> +/*
> + * XEN_HYPFS_OP_get_version
> + *
> + * Read highest interface version supported by the hypervisor.
> + *
> + * Possible return values:
> + * >0: highest supported interface version
> + * <0: negative Xen errno value
> + */
> +#define XEN_HYPFS_OP_get_version 0
> +
> +/*
> + * XEN_HYPFS_OP_read
> + *
> + * Read a filesystem entry.
> + *
> + * Returns the direntry and contents of an entry in the buffer supplied by the
> + * caller (struct xen_hypfs_direntry with the contents following directly
> + * after it).
> + * The data buffer must be at least the size of the direntry returned in order
> + * to have success. If the data buffer was not large enough for all the data
Looks like the "to have success" is stale now?
> + * -ENOBUFS and no entry data is returned, but the direntry will contain the
> + * needed size for the returned data.
> + * The format of the contents is according to its entry type and encoding.
> + * The contents of a directory are multiple struct xen_hypfs_dirlistentry
> + * items.
> + *
> + * arg1: XEN_GUEST_HANDLE(path name)
> + * arg2: length of path name (including trailing zero byte)
> + * arg3: XEN_GUEST_HANDLE(data buffer written by hypervisor)
> + * arg4: data buffer size
> + *
> + * Possible return values:
> + * 0: success (at least the direntry was returned)
As is this?
> +static inline void hypfs_string_set(struct hypfs_entry_leaf *leaf,
> + const char *str)
> +{
> + leaf->content = str;
> + leaf->e.size = strlen(str) + 1;
> +}
This looks at least risky to me, as the function name does in
no way indicate that no copy of the string will be made. Hence
its use with e.g. .init.rodata contents or a stack variable
will not produce the intended result.
> +#define HYPFS_UINT_INIT(var, nam, contvar) \
> + struct hypfs_entry_leaf __read_mostly var = { \
> + .e.type = XEN_HYPFS_TYPE_UINT, \
> + .e.encoding = XEN_HYPFS_ENC_PLAIN, \
> + .e.name = nam, \
> + .e.size = sizeof(contvar), \
> + .e.read = hypfs_read_leaf, \
> + .content = &contvar, \
> + }
> +
> +#define HYPFS_INT_INIT(var, nam, contvar) \
> + struct hypfs_entry_leaf __read_mostly var = { \
> + .e.type = XEN_HYPFS_TYPE_INT, \
> + .e.encoding = XEN_HYPFS_ENC_PLAIN, \
> + .e.name = nam, \
> + .e.size = sizeof(contvar), \
> + .e.read = hypfs_read_leaf, \
> + .content = &contvar, \
> + }
> +
> +#define HYPFS_BOOL_INIT(var, nam, contvar) \
> + struct hypfs_entry_leaf __read_mostly var = { \
> + .e.type = XEN_HYPFS_TYPE_BOOL, \
> + .e.encoding = XEN_HYPFS_ENC_PLAIN, \
> + .e.name = nam, \
> + .e.size = sizeof(contvar), \
> + .e.read = hypfs_read_leaf, \
> + .content = &contvar, \
> + }
Quite a lot of redundancy for just a single line of difference
between the instance. Perhaps have another helper macro?
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -86,6 +86,8 @@
> ? vcpu_hvm_context hvm/hvm_vcpu.h
> ? vcpu_hvm_x86_32 hvm/hvm_vcpu.h
> ? vcpu_hvm_x86_64 hvm/hvm_vcpu.h
> +? xen_hypfs_direntry hypfs.h
> +? xen_hypfs_dirlistentry hypfs.h
Where are the checking macros used that these produce?
> --- a/xen/xsm/flask/policy/access_vectors
> +++ b/xen/xsm/flask/policy/access_vectors
> @@ -67,6 +67,8 @@ class xen
> lockprof
> # XEN_SYSCTL_cpupool_op
> cpupool_op
> +# hypfs hypercall
> + hypfs_op
> # XEN_SYSCTL_scheduler_op with XEN_DOMCTL_SCHEDOP_getinfo, XEN_SYSCTL_sched_id, XEN_DOMCTL_SCHEDOP_getvcpuinfo
> getscheduler
> # XEN_SYSCTL_scheduler_op with XEN_DOMCTL_SCHEDOP_putinfo, XEN_DOMCTL_SCHEDOP_putvcpuinfo
I may not know enough about XSM to see why you can get away without
also modifying flask/hooks.c.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 7/8] xen: add /buildinfo/config entry to hypervisor filesystem
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 7/8] xen: add /buildinfo/config entry to hypervisor filesystem Juergen Gross
@ 2020-02-19 15:57 ` Jan Beulich
2020-02-20 6:13 ` Jürgen Groß
0 siblings, 1 reply; 24+ messages in thread
From: Jan Beulich @ 2020-02-19 15:57 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, xen-devel
On 19.02.2020 09:11, Juergen Gross wrote:
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -1,6 +1,7 @@
> obj-$(CONFIG_ARGO) += argo.o
> obj-y += bitmap.o
> obj-y += bsearch.o
> +obj-y += config_data.o
In particular with embedded uses in mind, I think this wants to
have a Kconfig control.
> @@ -414,6 +421,9 @@ static int __init buildinfo_init(void)
> hypfs_add_leaf(&version, &major, true);
> hypfs_add_leaf(&version, &minor, true);
>
> + config.e.size = xen_config_data_size;
This being the only use of xen_config_data_size, it suggests that
it could in principle live in .init.rodata. I realize this may
mean more customization to the binfile script than is warranted
by these 4 bytes of data, but I wanted to at least point out the
aspect.
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -100,5 +100,8 @@ extern enum system_state {
>
> bool_t is_active_kernel_text(unsigned long addr);
>
> +extern char xen_config_data;
> +extern unsigned int xen_config_data_size;
const for both?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-19 15:49 ` Jan Beulich
@ 2020-02-19 16:19 ` Julien Grall
2020-02-20 7:27 ` Jürgen Groß
1 sibling, 0 replies; 24+ messages in thread
From: Julien Grall @ 2020-02-19 16:19 UTC (permalink / raw)
To: Jan Beulich, Juergen Gross
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
Hi,
On 19/02/2020 15:49, Jan Beulich wrote:
> On 19.02.2020 09:11, Juergen Gross wrote:
>> +int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
>> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
>> +{
>> + union {
>> + char buf[8];
>> + uint8_t u8;
>> + uint16_t u16;
>> + uint32_t u32;
>> + uint64_t u64;
>> + } u;
>> +
>> + ASSERT(leaf->e.type == XEN_HYPFS_TYPE_UINT && leaf->e.size <= 8);
>> +
>> + if ( ulen != leaf->e.size )
>> + return -EDOM;
>> +
>> + if ( copy_from_guest(u.buf, uaddr, ulen) )
>> + return -EFAULT;
>> +
>> + switch ( leaf->e.size )
>> + {
>> + case 1:
>> + *(uint8_t *)leaf->write_ptr = !!u.u8;
>> + break;
>> + case 2:
>> + *(uint16_t *)leaf->write_ptr = !!u.u16;
>> + break;
>> + case 4:
>> + *(uint32_t *)leaf->write_ptr = !!u.u32;
>> + break;
>> + case 8:
>> + *(uint64_t *)leaf->write_ptr = !!u.u64;
>> + break;
>> + }
>
> Looking at this again, is there really a need for uint64_t support
> here? I.e. can't you cap at unsigned long (or even unsigned int),
> and perhaps additionally avoid use of fixed width types here
> altogether (some trickery may be needed for 32-bit's
> sizeof(long) == sizeof(int))?
On Arm, we want to have the interface exactly the same for 64-bit and
32-bit. If you plan to not support 8-byte here, this would effectively
mean that 8-byte could not be used on Arm.
However, I can't find any use of hypfs_write_bool, so I am not entirely
sure why we would want to cater size other than 1. Juerge, do you mind
expanding your thoughts?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support Juergen Gross
2020-02-19 15:49 ` Jan Beulich
@ 2020-02-19 16:26 ` Julien Grall
2020-02-19 16:47 ` Julien Grall
2020-02-20 7:29 ` Jürgen Groß
1 sibling, 2 replies; 24+ messages in thread
From: Julien Grall @ 2020-02-19 16:26 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
On 19/02/2020 08:11, Juergen Gross wrote:
> +int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
> +{
> + union {
> + char buf[8];
> + uint8_t u8;
> + uint16_t u16;
> + uint32_t u32;
> + uint64_t u64;
> + } u;
> +
> + ASSERT(leaf->e.type == XEN_HYPFS_TYPE_UINT && leaf->e.size <= 8);
Shouldn't this be XEN_HYPFSW_TYPE_BOOL? But then, why are we allowing a
boolean parameter to be held in a 64-bit value?
IHMO, a boolean should really be hold in a bool and nothing else.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs Juergen Gross
@ 2020-02-19 16:44 ` Jan Beulich
2020-02-20 8:22 ` Jürgen Groß
0 siblings, 1 reply; 24+ messages in thread
From: Jan Beulich @ 2020-02-19 16:44 UTC (permalink / raw)
To: Juergen Gross
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel, Volodymyr Babchuk, Roger Pau Monné
On 19.02.2020 09:11, Juergen Gross wrote:
> --- a/docs/misc/hypfs-paths.pandoc
> +++ b/docs/misc/hypfs-paths.pandoc
> @@ -152,3 +152,12 @@ The major version of Xen.
> #### /buildinfo/version/minor = INTEGER
>
> The minor version of Xen.
> +
> +#### /params/
> +
> +A directory of runtime parameters.
> +
> +#### /params/*
> +
> +The single parameters. The description of the different parameters can be
s/single/individual/?
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -89,6 +89,11 @@ SECTIONS
> __start_schedulers_array = .;
> *(.data.schedulers)
> __end_schedulers_array = .;
> +
> + . = ALIGN(8);
> + __paramhypfs_start = .;
> + *(.data.paramhypfs)
> + __paramhypfs_end = .;
Do you really need 8-byte alignment even on 32-bit Arm?
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -70,6 +70,17 @@ integer_param("ple_window", ple_window);
> static bool __read_mostly opt_ept_pml = true;
> static s8 __read_mostly opt_ept_ad = -1;
> int8_t __read_mostly opt_ept_exec_sp = -1;
> +static char opt_ept_setting[16] = "pml=1";
This is dangerous imo - such strings would better be populated
during boot by invoking the same function that also does so
after updating. Otherwise it won't take long until reported
and actual settings will be out of sync, until first modified
via this new interface.
> +
> +
> +static void update_ept_param(void)
No double blank lines please.
> @@ -31,10 +32,12 @@ static int parse_pcid(const char *s)
> {
> case 0:
> opt_pcid = PCID_OFF;
> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "off");
> break;
>
> case 1:
> opt_pcid = PCID_ALL;
> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "on");
> break;
>
> default:
> @@ -42,10 +45,12 @@ static int parse_pcid(const char *s)
> {
> case 0:
> opt_pcid = PCID_NOXPTI;
> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "noxpti");
> break;
>
> case 1:
> opt_pcid = PCID_XPTI;
> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "xpti");
> break;
Pretty expensive to use snprintf() here - how about strlcpy()?
> @@ -99,28 +101,33 @@ static int parse_gnttab_limit(const char *param, const char *arg,
> return -ERANGE;
>
> *valp = val;
> + snprintf(par_val, PAR_VAL_SZ, "%lu", val);
>
> return 0;
> }
>
> unsigned int __read_mostly opt_max_grant_frames = 64;
> +static char gnttab_max_frames_val[PAR_VAL_SZ] = "64";
This and the other option are plain integer ones from a presentation
pov, so it would be nice to get away here without the extra buffers.
> @@ -289,6 +290,33 @@ int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
> return 0;
> }
>
> +int hypfs_write_custom(struct hypfs_entry_leaf *leaf,
> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
> +{
> + struct param_hypfs *p;
> + char *buf;
> + int ret;
> +
> + buf = xzalloc_array(char, ulen);
> + if ( !buf )
> + return -ENOMEM;
> +
> + ret = -EFAULT;
> + if ( copy_from_guest(buf, uaddr, ulen) )
> + goto out;
> +
> + ret = -EDOM;
> + if ( buf[ulen - 1] )
Perhaps memchr() again.
> @@ -23,10 +24,17 @@ struct kernel_param {
> } par;
> };
>
> +struct param_hypfs {
> + const struct kernel_param *param;
As long as this is here, I don't think ...
> @@ -76,40 +84,87 @@ extern const struct kernel_param __param_start[], __param_end[];
> .type = OPT_IGNORE }
>
> #define __rtparam __param(__dataparam)
> +#define __paramfs static __paramhypfs \
> + __attribute__((__aligned__(sizeof(void *)))) struct param_hypfs
... you need the alignment attribute here. But see below.
> -#define custom_runtime_only_param(_name, _var) \
> +#define custom_runtime_only_param(_name, _var, contvar) \
> __rtparam __rtpar_##_var = \
> { .name = _name, \
> .type = OPT_CUSTOM, \
> - .par.func = _var }
> + .par.func = _var }; \
> + __paramfs __parfs_##_var = \
> + { .param = &__rtpar_##_var, \
Instead of a pointer, can't the param struct be part of this
bigger structure?
> + .hypfs.e.type = XEN_HYPFS_TYPE_STRING, \
> + .hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
> + .hypfs.e.name = _name, \
> + .hypfs.e.size = sizeof(contvar), \
This will go wrong if contvar is not an array. I guess you want
ARRAY_SIZE(contvar) * sizeof(*(convar)) here, and perhaps also
...
> + .hypfs.e.read = hypfs_read_leaf, \
> + .hypfs.e.write = hypfs_write_custom, \
> + .hypfs.content = &contvar }
... omit the & here.
> @@ -123,4 +178,7 @@ extern const struct kernel_param __param_start[], __param_end[];
> string_param(_name, _var); \
> string_runtime_only_param(_name, _var)
>
> +#define param_append_str(var, fmt, val) \
> + snprintf(var + strlen(var), sizeof(var) - strlen(var), fmt, val)
The sizeof() here again isn't safe against var not being of array
type. Also again perhaps cheaper to use strlcat()?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-19 16:26 ` Julien Grall
@ 2020-02-19 16:47 ` Julien Grall
2020-02-20 7:29 ` Jürgen Groß
1 sibling, 0 replies; 24+ messages in thread
From: Julien Grall @ 2020-02-19 16:47 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
On 19/02/2020 16:26, Julien Grall wrote:
>
>
> On 19/02/2020 08:11, Juergen Gross wrote:
>> +int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
>> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned
>> long ulen)
>> +{
>> + union {
>> + char buf[8];
>> + uint8_t u8;
>> + uint16_t u16;
>> + uint32_t u32;
>> + uint64_t u64;
>> + } u;
>> +
>> + ASSERT(leaf->e.type == XEN_HYPFS_TYPE_UINT && leaf->e.size <= 8);
>
> Shouldn't this be XEN_HYPFSW_TYPE_BOOL? But then, why are we allowing a
> boolean parameter to be held in a 64-bit value?
>
> IHMO, a boolean should really be hold in a bool and nothing else.
After sending my e-mail, I realized the size of the bool is
implementation defined. However, I think my point stands, it would be
better to have a fixed size for bool (i.e 1) and deny all the other use.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 7/8] xen: add /buildinfo/config entry to hypervisor filesystem
2020-02-19 15:57 ` Jan Beulich
@ 2020-02-20 6:13 ` Jürgen Groß
0 siblings, 0 replies; 24+ messages in thread
From: Jürgen Groß @ 2020-02-20 6:13 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, xen-devel
On 19.02.20 16:57, Jan Beulich wrote:
> On 19.02.2020 09:11, Juergen Gross wrote:
>> --- a/xen/common/Makefile
>> +++ b/xen/common/Makefile
>> @@ -1,6 +1,7 @@
>> obj-$(CONFIG_ARGO) += argo.o
>> obj-y += bitmap.o
>> obj-y += bsearch.o
>> +obj-y += config_data.o
>
> In particular with embedded uses in mind, I think this wants to
> have a Kconfig control.
Okay.
>
>> @@ -414,6 +421,9 @@ static int __init buildinfo_init(void)
>> hypfs_add_leaf(&version, &major, true);
>> hypfs_add_leaf(&version, &minor, true);
>>
>> + config.e.size = xen_config_data_size;
>
> This being the only use of xen_config_data_size, it suggests that
> it could in principle live in .init.rodata. I realize this may
> mean more customization to the binfile script than is warranted
> by these 4 bytes of data, but I wanted to at least point out the
> aspect.
In case this pattern is coming up again we might want to enhance
the script, but right now I don't think this is needed.
>
>> --- a/xen/include/xen/kernel.h
>> +++ b/xen/include/xen/kernel.h
>> @@ -100,5 +100,8 @@ extern enum system_state {
>>
>> bool_t is_active_kernel_text(unsigned long addr);
>>
>> +extern char xen_config_data;
>> +extern unsigned int xen_config_data_size;
>
> const for both?
Yes.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-19 15:49 ` Jan Beulich
2020-02-19 16:19 ` Julien Grall
@ 2020-02-20 7:27 ` Jürgen Groß
2020-02-20 8:43 ` Jan Beulich
1 sibling, 1 reply; 24+ messages in thread
From: Jürgen Groß @ 2020-02-20 7:27 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
On 19.02.20 16:49, Jan Beulich wrote:
> On 19.02.2020 09:11, Juergen Gross wrote:
>> +static int hypfs_get_path_user(char *buf,
>> + XEN_GUEST_HANDLE_PARAM(const_char) uaddr,
>> + unsigned long ulen)
>> +{
>> + if ( ulen > XEN_HYPFS_MAX_PATHLEN )
>> + return -EINVAL;
>> +
>> + if ( copy_from_guest(buf, uaddr, ulen) )
>> + return -EFAULT;
>> +
>> + if ( buf[ulen - 1] )
>> + return -EINVAL;
>
> I (still, but I may not have said so before) wonder whether
> memchr(buf, 0, ulen) != buf + ulen - 1 wouldn't be better here.
I like the idea.
>
>> +int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
>> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
>> +{
>> + char *buf;
>> + int ret;
>> +
>> + if ( ulen > leaf->e.size )
>> + return -ENOSPC;
>
> Okay, this makes sure you don't overrun the internal buffer.
> What about the opposite mismatch (ulen < leaf->e.size)? The
> result, except perhaps for (nul-terminated) strings, is not
> going to be very useful, at the very least.
Yes, will tighten the test.
>
>> + buf = xmalloc_array(char, ulen);
>> + if ( !buf )
>> + return -ENOMEM;
>> +
>> + ret = -EFAULT;
>> + if ( copy_from_guest(buf, uaddr, ulen) )
>> + goto out;
>> +
>> + ret = -EINVAL;
>> + if ( leaf->e.type == XEN_HYPFS_TYPE_STRING && buf[ulen] )
>
> buf[ulen - 1] I guess? Or, as above, memchr() again?
I'll go the memchr() way.
>
>> +int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
>> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
>> +{
>> + union {
>> + char buf[8];
>> + uint8_t u8;
>> + uint16_t u16;
>> + uint32_t u32;
>> + uint64_t u64;
>> + } u;
>> +
>> + ASSERT(leaf->e.type == XEN_HYPFS_TYPE_UINT && leaf->e.size <= 8);
>> +
>> + if ( ulen != leaf->e.size )
>> + return -EDOM;
>> +
>> + if ( copy_from_guest(u.buf, uaddr, ulen) )
>> + return -EFAULT;
>> +
>> + switch ( leaf->e.size )
>> + {
>> + case 1:
>> + *(uint8_t *)leaf->write_ptr = !!u.u8;
>> + break;
>> + case 2:
>> + *(uint16_t *)leaf->write_ptr = !!u.u16;
>> + break;
>> + case 4:
>> + *(uint32_t *)leaf->write_ptr = !!u.u32;
>> + break;
>> + case 8:
>> + *(uint64_t *)leaf->write_ptr = !!u.u64;
>> + break;
>> + }
>
> Looking at this again, is there really a need for uint64_t support
> here? I.e. can't you cap at unsigned long (or even unsigned int),
> and perhaps additionally avoid use of fixed width types here
> altogether (some trickery may be needed for 32-bit's
> sizeof(long) == sizeof(int))?
The reason for different lengths for bool is that boolean_param() is
falling back to the general integer param handling with all the natural
integer sizes.
I think I'll modify the macros in param.h to accept only sizeof(bool)
variables for boolean parameters and then I can switch to this fixed
size here.
>
>> --- /dev/null
>> +++ b/xen/include/public/hypfs.h
>> @@ -0,0 +1,127 @@
>> +/******************************************************************************
>> + * Xen Hypervisor Filesystem
>> + *
>> + * Copyright (c) 2019, SUSE Software Solutions Germany GmbH
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a copy
>> + * of this software and associated documentation files (the "Software"), to
>> + * deal in the Software without restriction, including without limitation the
>> + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
>> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#ifndef __XEN_PUBLIC_HYPFS_H__
>> +#define __XEN_PUBLIC_HYPFS_H__
>> +
>> +#include "xen.h"
>> +
>> +/*
>> + * Definitions for the __HYPERVISOR_hypfs_op hypercall.
>> + */
>> +
>> +/* Highest version number of the hypfs interface currently defined. */
>> +#define XEN_HYPFS_VERSION 1
>> +
>> +/* Maximum length of a path in the filesystem. */
>> +#define XEN_HYPFS_MAX_PATHLEN 1024
>
> I think it would be nice if the blank padding here matched that
> of the other #define-s up and down from here.
Okay.
>
>> +/*
>> + * XEN_HYPFS_OP_get_version
>> + *
>> + * Read highest interface version supported by the hypervisor.
>> + *
>> + * Possible return values:
>> + * >0: highest supported interface version
>> + * <0: negative Xen errno value
>> + */
>> +#define XEN_HYPFS_OP_get_version 0
>> +
>> +/*
>> + * XEN_HYPFS_OP_read
>> + *
>> + * Read a filesystem entry.
>> + *
>> + * Returns the direntry and contents of an entry in the buffer supplied by the
>> + * caller (struct xen_hypfs_direntry with the contents following directly
>> + * after it).
>> + * The data buffer must be at least the size of the direntry returned in order
>> + * to have success. If the data buffer was not large enough for all the data
>
> Looks like the "to have success" is stale now?
Yes.
>
>> + * -ENOBUFS and no entry data is returned, but the direntry will contain the
>> + * needed size for the returned data.
>> + * The format of the contents is according to its entry type and encoding.
>> + * The contents of a directory are multiple struct xen_hypfs_dirlistentry
>> + * items.
>> + *
>> + * arg1: XEN_GUEST_HANDLE(path name)
>> + * arg2: length of path name (including trailing zero byte)
>> + * arg3: XEN_GUEST_HANDLE(data buffer written by hypervisor)
>> + * arg4: data buffer size
>> + *
>> + * Possible return values:
>> + * 0: success (at least the direntry was returned)
>
> As is this?
Yes.
>
>> +static inline void hypfs_string_set(struct hypfs_entry_leaf *leaf,
>> + const char *str)
>> +{
>> + leaf->content = str;
>> + leaf->e.size = strlen(str) + 1;
>> +}
>
> This looks at least risky to me, as the function name does in
> no way indicate that no copy of the string will be made. Hence
> its use with e.g. .init.rodata contents or a stack variable
> will not produce the intended result.
Okay, what about naming it hypfs_string_set_reference() ?
>
>> +#define HYPFS_UINT_INIT(var, nam, contvar) \
>> + struct hypfs_entry_leaf __read_mostly var = { \
>> + .e.type = XEN_HYPFS_TYPE_UINT, \
>> + .e.encoding = XEN_HYPFS_ENC_PLAIN, \
>> + .e.name = nam, \
>> + .e.size = sizeof(contvar), \
>> + .e.read = hypfs_read_leaf, \
>> + .content = &contvar, \
>> + }
>> +
>> +#define HYPFS_INT_INIT(var, nam, contvar) \
>> + struct hypfs_entry_leaf __read_mostly var = { \
>> + .e.type = XEN_HYPFS_TYPE_INT, \
>> + .e.encoding = XEN_HYPFS_ENC_PLAIN, \
>> + .e.name = nam, \
>> + .e.size = sizeof(contvar), \
>> + .e.read = hypfs_read_leaf, \
>> + .content = &contvar, \
>> + }
>> +
>> +#define HYPFS_BOOL_INIT(var, nam, contvar) \
>> + struct hypfs_entry_leaf __read_mostly var = { \
>> + .e.type = XEN_HYPFS_TYPE_BOOL, \
>> + .e.encoding = XEN_HYPFS_ENC_PLAIN, \
>> + .e.name = nam, \
>> + .e.size = sizeof(contvar), \
>> + .e.read = hypfs_read_leaf, \
>> + .content = &contvar, \
>> + }
>
> Quite a lot of redundancy for just a single line of difference
> between the instance. Perhaps have another helper macro?
Fine with me.
>
>> --- a/xen/include/xlat.lst
>> +++ b/xen/include/xlat.lst
>> @@ -86,6 +86,8 @@
>> ? vcpu_hvm_context hvm/hvm_vcpu.h
>> ? vcpu_hvm_x86_32 hvm/hvm_vcpu.h
>> ? vcpu_hvm_x86_64 hvm/hvm_vcpu.h
>> +? xen_hypfs_direntry hypfs.h
>> +? xen_hypfs_dirlistentry hypfs.h
>
> Where are the checking macros used that these produce?
Ah, sorry. Will add them.
>
>> --- a/xen/xsm/flask/policy/access_vectors
>> +++ b/xen/xsm/flask/policy/access_vectors
>> @@ -67,6 +67,8 @@ class xen
>> lockprof
>> # XEN_SYSCTL_cpupool_op
>> cpupool_op
>> +# hypfs hypercall
>> + hypfs_op
>> # XEN_SYSCTL_scheduler_op with XEN_DOMCTL_SCHEDOP_getinfo, XEN_SYSCTL_sched_id, XEN_DOMCTL_SCHEDOP_getvcpuinfo
>> getscheduler
>> # XEN_SYSCTL_scheduler_op with XEN_DOMCTL_SCHEDOP_putinfo, XEN_DOMCTL_SCHEDOP_putvcpuinfo
>
> I may not know enough about XSM to see why you can get away without
> also modifying flask/hooks.c.
Hmm, strange, I was sure to have added it.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-19 16:26 ` Julien Grall
2020-02-19 16:47 ` Julien Grall
@ 2020-02-20 7:29 ` Jürgen Groß
1 sibling, 0 replies; 24+ messages in thread
From: Jürgen Groß @ 2020-02-20 7:29 UTC (permalink / raw)
To: Julien Grall, xen-devel
Cc: Stefano Stabellini, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, Jan Beulich,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
On 19.02.20 17:26, Julien Grall wrote:
>
>
> On 19/02/2020 08:11, Juergen Gross wrote:
>> +int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
>> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned
>> long ulen)
>> +{
>> + union {
>> + char buf[8];
>> + uint8_t u8;
>> + uint16_t u16;
>> + uint32_t u32;
>> + uint64_t u64;
>> + } u;
>> +
>> + ASSERT(leaf->e.type == XEN_HYPFS_TYPE_UINT && leaf->e.size <= 8);
>
> Shouldn't this be XEN_HYPFSW_TYPE_BOOL? But then, why are we allowing a
> boolean parameter to be held in a 64-bit value?
>
> IHMO, a boolean should really be hold in a bool and nothing else.
Yes, as already replied to Jan I'll modify the param handling for bool
which allowed larger variables and which was the reason I added this
possibility here, too.
And yes, the test should use XEN_HYPFSW_TYPE_BOOL.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs
2020-02-19 16:44 ` Jan Beulich
@ 2020-02-20 8:22 ` Jürgen Groß
2020-02-20 8:49 ` Jan Beulich
0 siblings, 1 reply; 24+ messages in thread
From: Jürgen Groß @ 2020-02-20 8:22 UTC (permalink / raw)
To: Jan Beulich
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel, Volodymyr Babchuk, Roger Pau Monné
On 19.02.20 17:44, Jan Beulich wrote:
> On 19.02.2020 09:11, Juergen Gross wrote:
>> --- a/docs/misc/hypfs-paths.pandoc
>> +++ b/docs/misc/hypfs-paths.pandoc
>> @@ -152,3 +152,12 @@ The major version of Xen.
>> #### /buildinfo/version/minor = INTEGER
>>
>> The minor version of Xen.
>> +
>> +#### /params/
>> +
>> +A directory of runtime parameters.
>> +
>> +#### /params/*
>> +
>> +The single parameters. The description of the different parameters can be
>
> s/single/individual/?
Yes, this is better.
>
>> --- a/xen/arch/arm/xen.lds.S
>> +++ b/xen/arch/arm/xen.lds.S
>> @@ -89,6 +89,11 @@ SECTIONS
>> __start_schedulers_array = .;
>> *(.data.schedulers)
>> __end_schedulers_array = .;
>> +
>> + . = ALIGN(8);
>> + __paramhypfs_start = .;
>> + *(.data.paramhypfs)
>> + __paramhypfs_end = .;
>
> Do you really need 8-byte alignment even on 32-bit Arm?
I just followed the general pattern in this file.
>
>> --- a/xen/arch/x86/hvm/vmx/vmcs.c
>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
>> @@ -70,6 +70,17 @@ integer_param("ple_window", ple_window);
>> static bool __read_mostly opt_ept_pml = true;
>> static s8 __read_mostly opt_ept_ad = -1;
>> int8_t __read_mostly opt_ept_exec_sp = -1;
>> +static char opt_ept_setting[16] = "pml=1";
>
> This is dangerous imo - such strings would better be populated
> during boot by invoking the same function that also does so
> after updating. Otherwise it won't take long until reported
> and actual settings will be out of sync, until first modified
> via this new interface.
Hmm, this would require a general update_value() function for each
custom runtime parameter.
I can see how this would look like.
>
>> +
>> +
>> +static void update_ept_param(void)
>
> No double blank lines please.
Oh, sorry.
>
>> @@ -31,10 +32,12 @@ static int parse_pcid(const char *s)
>> {
>> case 0:
>> opt_pcid = PCID_OFF;
>> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "off");
>> break;
>>
>> case 1:
>> opt_pcid = PCID_ALL;
>> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "on");
>> break;
>>
>> default:
>> @@ -42,10 +45,12 @@ static int parse_pcid(const char *s)
>> {
>> case 0:
>> opt_pcid = PCID_NOXPTI;
>> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "noxpti");
>> break;
>>
>> case 1:
>> opt_pcid = PCID_XPTI;
>> + snprintf(opt_pcid_val, sizeof(opt_pcid_val), "xpti");
>> break;
>
> Pretty expensive to use snprintf() here - how about strlcpy()?
Oh, of course.
>
>> @@ -99,28 +101,33 @@ static int parse_gnttab_limit(const char *param, const char *arg,
>> return -ERANGE;
>>
>> *valp = val;
>> + snprintf(par_val, PAR_VAL_SZ, "%lu", val);
>>
>> return 0;
>> }
>>
>> unsigned int __read_mostly opt_max_grant_frames = 64;
>> +static char gnttab_max_frames_val[PAR_VAL_SZ] = "64";
>
> This and the other option are plain integer ones from a presentation
> pov, so it would be nice to get away here without the extra buffers.
There is more than pure integer semantics here: the value is limited,
so I can't just use the normal integer assignment.
Its not as if those functions are performance critical, and special
casing for sparing the buffer memory will probably waste more memory
due to larger code size.
>
>> @@ -289,6 +290,33 @@ int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
>> return 0;
>> }
>>
>> +int hypfs_write_custom(struct hypfs_entry_leaf *leaf,
>> + XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
>> +{
>> + struct param_hypfs *p;
>> + char *buf;
>> + int ret;
>> +
>> + buf = xzalloc_array(char, ulen);
>> + if ( !buf )
>> + return -ENOMEM;
>> +
>> + ret = -EFAULT;
>> + if ( copy_from_guest(buf, uaddr, ulen) )
>> + goto out;
>> +
>> + ret = -EDOM;
>> + if ( buf[ulen - 1] )
>
> Perhaps memchr() again.
Okay.
>
>> @@ -23,10 +24,17 @@ struct kernel_param {
>> } par;
>> };
>>
>> +struct param_hypfs {
>> + const struct kernel_param *param;
>
> As long as this is here, I don't think ...
>
>> @@ -76,40 +84,87 @@ extern const struct kernel_param __param_start[], __param_end[];
>> .type = OPT_IGNORE }
>>
>> #define __rtparam __param(__dataparam)
>> +#define __paramfs static __paramhypfs \
>> + __attribute__((__aligned__(sizeof(void *)))) struct param_hypfs
>
> ... you need the alignment attribute here. But see below.
>
>> -#define custom_runtime_only_param(_name, _var) \
>> +#define custom_runtime_only_param(_name, _var, contvar) \
>> __rtparam __rtpar_##_var = \
>> { .name = _name, \
>> .type = OPT_CUSTOM, \
>> - .par.func = _var }
>> + .par.func = _var }; \
>> + __paramfs __parfs_##_var = \
>> + { .param = &__rtpar_##_var, \
>
> Instead of a pointer, can't the param struct be part of this
> bigger structure?
I have some further patches in my pipeline which will do even more,
by removing the sysctl for setting parameters and using hypfs for
that purpose in libxl. This will remove the need for the runtime
copy of struct kernel_param completely.
But r
>
>> + .hypfs.e.type = XEN_HYPFS_TYPE_STRING, \
>> + .hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
>> + .hypfs.e.name = _name, \
>> + .hypfs.e.size = sizeof(contvar), \
>
> This will go wrong if contvar is not an array. I guess you want
> ARRAY_SIZE(contvar) * sizeof(*(convar)) here, and perhaps also
Ah, nice idea.
> ...
>
>> + .hypfs.e.read = hypfs_read_leaf, \
>> + .hypfs.e.write = hypfs_write_custom, \
>> + .hypfs.content = &contvar }
>
> ... omit the & here.
Okay.
>
>> @@ -123,4 +178,7 @@ extern const struct kernel_param __param_start[], __param_end[];
>> string_param(_name, _var); \
>> string_runtime_only_param(_name, _var)
>>
>> +#define param_append_str(var, fmt, val) \
>> + snprintf(var + strlen(var), sizeof(var) - strlen(var), fmt, val)
>
> The sizeof() here again isn't safe against var not being of array
> type. Also again perhaps cheaper to use strlcat()?
Yes.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-20 7:27 ` Jürgen Groß
@ 2020-02-20 8:43 ` Jan Beulich
2020-02-20 9:07 ` Jürgen Groß
0 siblings, 1 reply; 24+ messages in thread
From: Jan Beulich @ 2020-02-20 8:43 UTC (permalink / raw)
To: Jürgen Groß
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
On 20.02.2020 08:27, Jürgen Groß wrote:
> On 19.02.20 16:49, Jan Beulich wrote:
>> On 19.02.2020 09:11, Juergen Gross wrote:
>>> +static inline void hypfs_string_set(struct hypfs_entry_leaf *leaf,
>>> + const char *str)
>>> +{
>>> + leaf->content = str;
>>> + leaf->e.size = strlen(str) + 1;
>>> +}
>>
>> This looks at least risky to me, as the function name does in
>> no way indicate that no copy of the string will be made. Hence
>> its use with e.g. .init.rodata contents or a stack variable
>> will not produce the intended result.
>
> Okay, what about naming it hypfs_string_set_reference() ?
Hmm, a little better because of drawing more attention to it
due to the longish name. I'd be curious what others think here,
in particular towards the alternative of actually allocating
space and copying the string. If it is to remain as it is,
while the function body is small I still think you also want to
attach a comment to call out this behavior.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs
2020-02-20 8:22 ` Jürgen Groß
@ 2020-02-20 8:49 ` Jan Beulich
2020-02-20 9:11 ` Jürgen Groß
0 siblings, 1 reply; 24+ messages in thread
From: Jan Beulich @ 2020-02-20 8:49 UTC (permalink / raw)
To: Jürgen Groß
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel, Volodymyr Babchuk, Roger Pau Monné
On 20.02.2020 09:22, Jürgen Groß wrote:
> On 19.02.20 17:44, Jan Beulich wrote:
>> On 19.02.2020 09:11, Juergen Gross wrote:
>>> --- a/xen/arch/arm/xen.lds.S
>>> +++ b/xen/arch/arm/xen.lds.S
>>> @@ -89,6 +89,11 @@ SECTIONS
>>> __start_schedulers_array = .;
>>> *(.data.schedulers)
>>> __end_schedulers_array = .;
>>> +
>>> + . = ALIGN(8);
>>> + __paramhypfs_start = .;
>>> + *(.data.paramhypfs)
>>> + __paramhypfs_end = .;
>>
>> Do you really need 8-byte alignment even on 32-bit Arm?
>
> I just followed the general pattern in this file.
Well, it'll be the Arm maintainers to judge anyway. It merely
caught my eye.
>>> @@ -99,28 +101,33 @@ static int parse_gnttab_limit(const char *param, const char *arg,
>>> return -ERANGE;
>>>
>>> *valp = val;
>>> + snprintf(par_val, PAR_VAL_SZ, "%lu", val);
>>>
>>> return 0;
>>> }
>>>
>>> unsigned int __read_mostly opt_max_grant_frames = 64;
>>> +static char gnttab_max_frames_val[PAR_VAL_SZ] = "64";
>>
>> This and the other option are plain integer ones from a presentation
>> pov, so it would be nice to get away here without the extra buffers.
>
> There is more than pure integer semantics here: the value is limited,
> so I can't just use the normal integer assignment.
Which is why I've said "from a presentation pov", i.e. when consuming
the value for passing back as a string.
> Its not as if those functions are performance critical, and special
> casing for sparing the buffer memory will probably waste more memory
> due to larger code size.
Well, it's not so much the memory savings, but the reduction of
redundant information being kept. But if it's not sufficiently simple
to generalize, so be it then.
>>> @@ -23,10 +24,17 @@ struct kernel_param {
>>> } par;
>>> };
>>>
>>> +struct param_hypfs {
>>> + const struct kernel_param *param;
>>
>> As long as this is here, I don't think ...
>>
>>> @@ -76,40 +84,87 @@ extern const struct kernel_param __param_start[], __param_end[];
>>> .type = OPT_IGNORE }
>>>
>>> #define __rtparam __param(__dataparam)
>>> +#define __paramfs static __paramhypfs \
>>> + __attribute__((__aligned__(sizeof(void *)))) struct param_hypfs
>>
>> ... you need the alignment attribute here. But see below.
>>
>>> -#define custom_runtime_only_param(_name, _var) \
>>> +#define custom_runtime_only_param(_name, _var, contvar) \
>>> __rtparam __rtpar_##_var = \
>>> { .name = _name, \
>>> .type = OPT_CUSTOM, \
>>> - .par.func = _var }
>>> + .par.func = _var }; \
>>> + __paramfs __parfs_##_var = \
>>> + { .param = &__rtpar_##_var, \
>>
>> Instead of a pointer, can't the param struct be part of this
>> bigger structure?
>
> I have some further patches in my pipeline which will do even more,
> by removing the sysctl for setting parameters and using hypfs for
> that purpose in libxl. This will remove the need for the runtime
> copy of struct kernel_param completely.
Ah, good to know. While reviewing this I was wondering about exactly
this.
> But r
???
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support
2020-02-20 8:43 ` Jan Beulich
@ 2020-02-20 9:07 ` Jürgen Groß
0 siblings, 0 replies; 24+ messages in thread
From: Jürgen Groß @ 2020-02-20 9:07 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Konrad Rzeszutek Wilk,
George Dunlap, Andrew Cooper, Ian Jackson, xen-devel,
Daniel De Graaf, Volodymyr Babchuk, Roger Pau Monné
On 20.02.20 09:43, Jan Beulich wrote:
> On 20.02.2020 08:27, Jürgen Groß wrote:
>> On 19.02.20 16:49, Jan Beulich wrote:
>>> On 19.02.2020 09:11, Juergen Gross wrote:
>>>> +static inline void hypfs_string_set(struct hypfs_entry_leaf *leaf,
>>>> + const char *str)
>>>> +{
>>>> + leaf->content = str;
>>>> + leaf->e.size = strlen(str) + 1;
>>>> +}
>>>
>>> This looks at least risky to me, as the function name does in
>>> no way indicate that no copy of the string will be made. Hence
>>> its use with e.g. .init.rodata contents or a stack variable
>>> will not produce the intended result.
>>
>> Okay, what about naming it hypfs_string_set_reference() ?
>
> Hmm, a little better because of drawing more attention to it
> due to the longish name. I'd be curious what others think here,
> in particular towards the alternative of actually allocating
> space and copying the string. If it is to remain as it is,
> while the function body is small I still think you also want to
> attach a comment to call out this behavior.
Okay.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs
2020-02-20 8:49 ` Jan Beulich
@ 2020-02-20 9:11 ` Jürgen Groß
0 siblings, 0 replies; 24+ messages in thread
From: Jürgen Groß @ 2020-02-20 9:11 UTC (permalink / raw)
To: Jan Beulich
Cc: Kevin Tian, Stefano Stabellini, Julien Grall, Wei Liu,
Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper, Ian Jackson,
Jun Nakajima, xen-devel, Volodymyr Babchuk, Roger Pau Monné
On 20.02.20 09:49, Jan Beulich wrote:
> On 20.02.2020 09:22, Jürgen Groß wrote:
>> On 19.02.20 17:44, Jan Beulich wrote:
>>> On 19.02.2020 09:11, Juergen Gross wrote:
>>>> --- a/xen/arch/arm/xen.lds.S
>>>> +++ b/xen/arch/arm/xen.lds.S
>>>> @@ -89,6 +89,11 @@ SECTIONS
>>>> __start_schedulers_array = .;
>>>> *(.data.schedulers)
>>>> __end_schedulers_array = .;
>>>> +
>>>> + . = ALIGN(8);
>>>> + __paramhypfs_start = .;
>>>> + *(.data.paramhypfs)
>>>> + __paramhypfs_end = .;
>>>
>>> Do you really need 8-byte alignment even on 32-bit Arm?
>>
>> I just followed the general pattern in this file.
>
> Well, it'll be the Arm maintainers to judge anyway. It merely
> caught my eye.
>
>>>> @@ -99,28 +101,33 @@ static int parse_gnttab_limit(const char *param, const char *arg,
>>>> return -ERANGE;
>>>>
>>>> *valp = val;
>>>> + snprintf(par_val, PAR_VAL_SZ, "%lu", val);
>>>>
>>>> return 0;
>>>> }
>>>>
>>>> unsigned int __read_mostly opt_max_grant_frames = 64;
>>>> +static char gnttab_max_frames_val[PAR_VAL_SZ] = "64";
>>>
>>> This and the other option are plain integer ones from a presentation
>>> pov, so it would be nice to get away here without the extra buffers.
>>
>> There is more than pure integer semantics here: the value is limited,
>> so I can't just use the normal integer assignment.
>
> Which is why I've said "from a presentation pov", i.e. when consuming
> the value for passing back as a string.
Hmm, this might even be possible via not using the common macro.
I'll have a look into it.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-02-20 9:12 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19 8:11 [Xen-devel] [PATCH v5 0/8] Add hypervisor sysfs-like support Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 1/8] xen: add a generic way to include binary files as variables Juergen Gross
2020-02-19 12:05 ` Wei Liu
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 2/8] docs: add feature document for Xen hypervisor sysfs-like support Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 3/8] xen: add basic hypervisor filesystem support Juergen Gross
2020-02-19 15:49 ` Jan Beulich
2020-02-19 16:19 ` Julien Grall
2020-02-20 7:27 ` Jürgen Groß
2020-02-20 8:43 ` Jan Beulich
2020-02-20 9:07 ` Jürgen Groß
2020-02-19 16:26 ` Julien Grall
2020-02-19 16:47 ` Julien Grall
2020-02-20 7:29 ` Jürgen Groß
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 4/8] libs: add libxenhypfs Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 5/8] tools: add xenfs tool Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 6/8] xen: provide version information in hypfs Juergen Gross
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 7/8] xen: add /buildinfo/config entry to hypervisor filesystem Juergen Gross
2020-02-19 15:57 ` Jan Beulich
2020-02-20 6:13 ` Jürgen Groß
2020-02-19 8:11 ` [Xen-devel] [PATCH v5 8/8] xen: add runtime parameter access support to hypfs Juergen Gross
2020-02-19 16:44 ` Jan Beulich
2020-02-20 8:22 ` Jürgen Groß
2020-02-20 8:49 ` Jan Beulich
2020-02-20 9:11 ` Jürgen Groß
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.