RE: [PATCH v10 00/12] Add hypervisor sysfs-like support

From: Paul Durrant <xadimgnik@gmail.com>
To: "'Jürgen Groß'" <jgross@suse.com>,
	"'Jan Beulich'" <jbeulich@suse.com>,
	"'Kevin Tian'" <kevin.tian@intel.com>,
	"'Julien Grall'" <julien@xen.org>,
	"'Jun Nakajima'" <jun.nakajima@intel.com>,
	"'Wei Liu'" <wl@xen.org>,
	"'Ian Jackson'" <ian.jackson@eu.citrix.com>,
	"'Daniel De Graaf'" <dgdegra@tycho.nsa.gov>
Cc: "'Stefano Stabellini'" <sstabellini@kernel.org>,
	"'Andrew Cooper'" <andrew.cooper3@citrix.com>,
	"'George Dunlap'" <george.dunlap@citrix.com>,
	"'Anthony PERARD'" <anthony.perard@citrix.com>,
	xen-devel@lists.xenproject.org,
	"'Volodymyr Babchuk'" <Volodymyr_Babchuk@epam.com>,
	"'Roger Pau Monné'" <roger.pau@citrix.com>
Subject: RE: [PATCH v10 00/12] Add hypervisor sysfs-like support
Date: Tue, 26 May 2020 09:00:02 +0100	[thread overview]
Message-ID: <003f01d63333$aa399f20$feacdd60$@xen.org> (raw)
In-Reply-To: <fc55f4dc-c802-2153-cd6a-736a29e8a396@suse.com>

> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 25 May 2020 08:02
> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; 'Kevin Tian' <kevin.tian@intel.com>; 'Julien
> Grall' <julien@xen.org>; 'Jun Nakajima' <jun.nakajima@intel.com>; 'Wei Liu' <wl@xen.org>; 'Ian
> Jackson' <ian.jackson@eu.citrix.com>; 'Daniel De Graaf' <dgdegra@tycho.nsa.gov>
> Cc: 'Stefano Stabellini' <sstabellini@kernel.org>; 'Andrew Cooper' <andrew.cooper3@citrix.com>;
> 'George Dunlap' <george.dunlap@citrix.com>; 'Anthony PERARD' <anthony.perard@citrix.com>; xen-
> devel@lists.xenproject.org; 'Volodymyr Babchuk' <Volodymyr_Babchuk@epam.com>; 'Roger Pau Monné'
> <roger.pau@citrix.com>
> Subject: Re: [PATCH v10 00/12] Add hypervisor sysfs-like support
> 
> On 19.05.20 10:06, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 19 May 2020 08:45
> >> To: Jürgen Groß <jgross@suse.com>; Kevin Tian <kevin.tian@intel.com>; Julien Grall
> <julien@xen.org>;
> >> Jun Nakajima <jun.nakajima@intel.com>; Wei Liu <wl@xen.org>; Ian Jackson
> <ian.jackson@eu.citrix.com>;
> >> Daniel De Graaf <dgdegra@tycho.nsa.gov>; Paul Durrant <paul@xen.org>
> >> Cc: xen-devel@lists.xenproject.org; Stefano Stabellini <sstabellini@kernel.org>; Andrew Cooper
> >> <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Anthony PERARD
> >> <anthony.perard@citrix.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Roger Pau Monné
> >> <roger.pau@citrix.com>
> >> Subject: Re: [PATCH v10 00/12] Add hypervisor sysfs-like support
> >>
> >> On 19.05.2020 09:30, Jürgen Groß wrote:
> >>> On 19.05.20 09:20, Juergen Gross wrote:
> >>>>
> >>>> Juergen Gross (12):
> >>>>     xen/vmx: let opt_ept_ad always reflect the current setting
> >>>>     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
> >>>>     tools/libxl: use libxenhypfs for setting xen runtime parameters
> >>>>     tools/libxc: remove xc_set_parameters()
> >>>>     xen: remove XEN_SYSCTL_set_parameter support
> >>>>
> >>>>    .gitignore                          |   6 +
> >>>>    docs/features/hypervisorfs.pandoc   |  92 +++++
> >>>>    docs/man/xenhypfs.1.pod             |  61 ++++
> >>>>    docs/misc/hypfs-paths.pandoc        | 165 +++++++++
> >>>>    tools/Rules.mk                      |   8 +-
> >>>>    tools/flask/policy/modules/dom0.te  |   4 +-
> >>>>    tools/libs/Makefile                 |   1 +
> >>>>    tools/libs/hypfs/Makefile           |  16 +
> >>>>    tools/libs/hypfs/core.c             | 536 ++++++++++++++++++++++++++++
> >>>>    tools/libs/hypfs/include/xenhypfs.h |  90 +++++
> >>>>    tools/libs/hypfs/libxenhypfs.map    |  10 +
> >>>>    tools/libs/hypfs/xenhypfs.pc.in     |  10 +
> >>>>    tools/libxc/include/xenctrl.h       |   1 -
> >>>>    tools/libxc/xc_misc.c               |  21 --
> >>>>    tools/libxl/Makefile                |   3 +-
> >>>>    tools/libxl/libxl.c                 |  53 ++-
> >>>>    tools/libxl/libxl_internal.h        |   1 +
> >>>>    tools/libxl/xenlight.pc.in          |   2 +-
> >>>>    tools/misc/Makefile                 |   6 +
> >>>>    tools/misc/xenhypfs.c               | 192 ++++++++++
> >>>>    tools/xl/xl_misc.c                  |   1 -
> >>>>    xen/arch/arm/traps.c                |   3 +
> >>>>    xen/arch/arm/xen.lds.S              |  13 +-
> >>>>    xen/arch/x86/hvm/hypercall.c        |   3 +
> >>>>    xen/arch/x86/hvm/vmx/vmcs.c         |  47 ++-
> >>>>    xen/arch/x86/hvm/vmx/vmx.c          |   4 +-
> >>>>    xen/arch/x86/hypercall.c            |   3 +
> >>>>    xen/arch/x86/pv/domain.c            |  21 +-
> >>>>    xen/arch/x86/pv/hypercall.c         |   3 +
> >>>>    xen/arch/x86/xen.lds.S              |  12 +-
> >>>>    xen/common/Kconfig                  |  23 ++
> >>>>    xen/common/Makefile                 |  13 +
> >>>>    xen/common/grant_table.c            |  62 +++-
> >>>>    xen/common/hypfs.c                  | 452 +++++++++++++++++++++++
> >>>>    xen/common/kernel.c                 |  84 ++++-
> >>>>    xen/common/sysctl.c                 |  36 --
> >>>>    xen/drivers/char/console.c          |  72 +++-
> >>>>    xen/include/Makefile                |   1 +
> >>>>    xen/include/asm-x86/hvm/vmx/vmcs.h  |   3 +-
> >>>>    xen/include/public/hypfs.h          | 129 +++++++
> >>>>    xen/include/public/sysctl.h         |  19 +-
> >>>>    xen/include/public/xen.h            |   1 +
> >>>>    xen/include/xen/hypercall.h         |  10 +
> >>>>    xen/include/xen/hypfs.h             | 123 +++++++
> >>>>    xen/include/xen/kernel.h            |   3 +
> >>>>    xen/include/xen/lib.h               |   1 -
> >>>>    xen/include/xen/param.h             | 126 +++++--
> >>>>    xen/include/xlat.lst                |   2 +
> >>>>    xen/include/xsm/dummy.h             |   6 +
> >>>>    xen/include/xsm/xsm.h               |   6 +
> >>>>    xen/tools/binfile                   |  43 +++
> >>>>    xen/xsm/dummy.c                     |   1 +
> >>>>    xen/xsm/flask/Makefile              |   5 +-
> >>>>    xen/xsm/flask/flask-policy.S        |  16 -
> >>>>    xen/xsm/flask/hooks.c               |   9 +-
> >>>>    xen/xsm/flask/policy/access_vectors |   4 +-
> >>>>    56 files changed, 2445 insertions(+), 193 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
> >>>>
> >>>
> >>> There are some Acks missing on this series, so please have a look at the
> >>> patches!
> >>>
> >>> There are missing especially:
> >>>
> >>> - Patch 1: VMX maintainers
> >>> - Patch 2 + 4: XSM maintainer
> >>> - Patch 4 + 9: Arm maintainer
> >>> - Patch 10 + 11: tools maintainers
> >>>
> >>> I'd really like the series to go into 4.14 (deadline this Friday).
> >>
> >
> > I would also like to see this in 4.14.
> >
> >> FTR I'm intending to waive the need for the first three of the named
> >> sets if they don't arrive by Friday (and there I don't mean last
> >> minute on Friday) - they're not overly intrusive (maybe with the
> >> exception of the XSM parts in #4) and the series has been pending
> >> for long enough. I don't feel comfortable to do so for patch 10,
> >> though; patch 11 looks to be simple enough again.
> >>
> >> Paul, as the release manager, please let me know if you disagree.
> >>
> >
> > Looking at patch #4, I'm not confident that the XSM parts are complete (e.g. does xen.if need
> updating?). Also I'd put the new access vector in xen2, since that's where set_parameter currently is
> (and will be removed from in a later patch), but the xen class does appear to have space so that's
> really just my taste.
> 
> I don't think xen.if needs updating, as it contains only macros for
> groups of operations.
> 

Ok.

> As the new hypercall isn't only replacing set_parameter, but has much
> wider semantics, I don't think it should go to xen2. There will be
> probably more interfaces being replaced and/or added after all.
> 

If you're happy with it then, in the absence of a response from Daniel, then I think patch #4 can go in. Patch #10 and #11 have acks now, so it looks like the series is good to go. Could you send a patch for CHANGELOG.md as I think we'd consider this a significant feature :-)

  Paul

> 
> Juergen