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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 19 May 2020 08:45
> To: Jürgen Groß <jgross@xxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>; Julien 
> Grall <julien@xxxxxxx>;
> Jun Nakajima <jun.nakajima@xxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Ian Jackson 
> <ian.jackson@xxxxxxxxxxxxx>;
> Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>; Paul Durrant <paul@xxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; George Dunlap <george.dunlap@xxxxxxxxxx>; 
> Anthony PERARD
> <anthony.perard@xxxxxxxxxx>; Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>; 
> Roger Pau Monné
> <roger.pau@xxxxxxxxxx>
> 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 agree that patch #10 really needs a tools maintainer ack but that patch #11 
looks straightforward too so I'd be happy without one for that.

  Paul

> Jan