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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jürgen Groß <jgross@xxxxxxxx>
> Sent: 25 May 2020 08:02
> To: paul@xxxxxxx; 'Jan Beulich' <jbeulich@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>
> Cc: 'Stefano Stabellini' <sstabellini@xxxxxxxxxx>; 'Andrew Cooper' 
> <andrew.cooper3@xxxxxxxxxx>;
> 'George Dunlap' <george.dunlap@xxxxxxxxxx>; 'Anthony PERARD' 
> <anthony.perard@xxxxxxxxxx>; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; '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.20 10:06, Paul Durrant wrote:
> >> -----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 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