> -----Original Message-----
> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of Paul
> Durrant
> Sent: 05 September 2019 14:52
> To: Roger Pau Monne <roger....@citrix.com>; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini <sstabell...@kernel.org>; Wei Liu <w...@xen.org>;
> Konrad Rzeszutek Wilk
> <konrad.w...@oracle.com>; Andrew Cooper <andrew.coop...@citrix.com>; Tim
> (Xen.org) <t...@xen.org>;
> George Dunlap <george.dun...@citrix.com>; Julien Grall
> <julien.gr...@arm.com>; Jan Beulich
> <jbeul...@suse.com>; Anthony Perard <anthony.per...@citrix.com>; Ian Jackson
> <ian.jack...@citrix.com>;
> Volodymyr Babchuk <volodymyr_babc...@epam.com>; Roger Pau Monne
> <roger....@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v2 2/2] sysctl/libxl: choose a sane default
> for HAP
>
> > -----Original Message-----
> > From: Roger Pau Monne <roger....@citrix.com>
> > Sent: 05 September 2019 14:27
> > To: xen-devel@lists.xenproject.org
> > Cc: Roger Pau Monne <roger....@citrix.com>; Ian Jackson
> > <ian.jack...@citrix.com>; Wei Liu
> > <w...@xen.org>; Anthony Perard <anthony.per...@citrix.com>; Andrew Cooper
> > <andrew.coop...@citrix.com>;
> > George Dunlap <george.dun...@citrix.com>; Jan Beulich <jbeul...@suse.com>;
> > Julien Grall
> > <julien.gr...@arm.com>; Konrad Rzeszutek Wilk <konrad.w...@oracle.com>;
> > Stefano Stabellini
> > <sstabell...@kernel.org>; Tim (Xen.org) <t...@xen.org>; Volodymyr Babchuk
> <volodymyr_babc...@epam.com>;
> > Paul Durrant <paul.durr...@citrix.com>
> > Subject: [PATCH v2 2/2] sysctl/libxl: choose a sane default for HAP
> >
> > Current libxl code will always enable Hardware Assisted Paging (HAP),
> > expecting that the hypervisor will fallback to shadow if HAP is not
> > available. With the changes to the domain builder that's not the case
> > any longer, and the hypervisor will raise an error if HAP is not
> > available instead of silently falling back to shadow.
> >
> > In order to keep the previous functionality report whether HAP is
> > available or not in XEN_SYSCTL_physinfo, so that the toolstack can
> > select a sane default if there's no explicit user selection of whether
> > HAP should be used.
> >
> > Note that on ARM hardware HAP capability is always reported since it's
> > a required feature in order to run Xen.
> >
> > Fixes: d0c0ba7d3de ('x86/hvm/domain: remove the 'hap_enabled' flag')
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
>
> LGTM
>
> Reviewed-by: Paul Durrant <paul.durr...@citrix.com>
>
> > ---
> > Cc: Paul Durrant <paul.durr...@citrix.com>
> > ---
> > Changes since v1:
> > - Also report HAP capability for ARM.
> > - Print hap capability in xl info.
> > ---
> > tools/libxl/libxl.c | 1 +
> > tools/libxl/libxl_create.c | 8 +++++++-
> > tools/libxl/libxl_types.idl | 1 +
> > tools/xl/xl_info.c | 5 +++--
> > xen/arch/arm/sysctl.c | 2 +-
> > xen/arch/x86/sysctl.c | 2 ++
> > xen/include/public/sysctl.h | 4 ++++
> > 7 files changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> > index ec71574e99..5c0fcf320e 100644
> > --- a/tools/libxl/libxl.c
> > +++ b/tools/libxl/libxl.c
> > @@ -399,6 +399,7 @@ int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo
> > *physinfo)
> > physinfo->cap_pv = !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_pv);
> > physinfo->cap_hvm_directio =
> > !!(xcphysinfo.capabilities & XEN_SYSCTL_PHYSCAP_directio);
> > + physinfo->cap_hap = !!(xcphysinfo.capabilities &
> > XEN_SYSCTL_PHYSCAP_hap);
> >
> > GC_FREE;
> > return 0;
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index 03ce166f4f..6a556dea8f 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -38,7 +38,13 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
> > libxl__arch_domain_create_info_setdefault(gc, c_info);
> >
> > if (c_info->type != LIBXL_DOMAIN_TYPE_PV) {
> > - libxl_defbool_setdefault(&c_info->hap, true);
> > + libxl_physinfo info;
> > + int rc = libxl_get_physinfo(CTX, &info);
> > +
> > + if (rc)
> > + return rc;
> > +
> > + libxl_defbool_setdefault(&c_info->hap, info.cap_hap);
> > libxl_defbool_setdefault(&c_info->oos, true);
> > }
> >
> > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> > index b61399ce36..9e1f8515d3 100644
> > --- a/tools/libxl/libxl_types.idl
> > +++ b/tools/libxl/libxl_types.idl
> > @@ -1025,6 +1025,7 @@ libxl_physinfo = Struct("physinfo", [
> > ("cap_hvm", bool),
> > ("cap_pv", bool),
> > ("cap_hvm_directio", bool), # No longer HVM specific
> > + ("cap_hap", bool),
Actually Julien's review of one of my patches points out that this idl change
should be accompanied by an associated LIBXL_HAVE_CAP_HAP definition.
Paul
> > ], dir=DIR_OUT)
> >
> > libxl_connectorinfo = Struct("connectorinfo", [
> > diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
> > index 46d9c9f712..aa6724bc7f 100644
> > --- a/tools/xl/xl_info.c
> > +++ b/tools/xl/xl_info.c
> > @@ -210,11 +210,12 @@ static void output_physinfo(void)
> > info.hw_cap[4], info.hw_cap[5], info.hw_cap[6], info.hw_cap[7]
> > );
> >
> > - maybe_printf("virt_caps :%s%s%s%s\n",
> > + maybe_printf("virt_caps :%s%s%s%s%s\n",
> > info.cap_pv ? " pv" : "",
> > info.cap_hvm ? " hvm" : "",
> > info.cap_hvm && info.cap_hvm_directio ? " hvm_directio" : "",
> > - info.cap_pv && info.cap_hvm_directio ? " pv_directio" : ""
> > + info.cap_pv && info.cap_hvm_directio ? " pv_directio" : "",
> > + info.cap_hap ? " hap" : ""
> > );
> >
> > vinfo = libxl_get_version_info(ctx);
> > diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> > index 92ac99c928..f87944e847 100644
> > --- a/xen/arch/arm/sysctl.c
> > +++ b/xen/arch/arm/sysctl.c
> > @@ -14,7 +14,7 @@
> >
> > void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> > {
> > - pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
> > + pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm | XEN_SYSCTL_PHYSCAP_hap;
> > }
> >
> > long arch_do_sysctl(struct xen_sysctl *sysctl,
> > diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> > index 7ec6174e6b..5777a05ffc 100644
> > --- a/xen/arch/x86/sysctl.c
> > +++ b/xen/arch/x86/sysctl.c
> > @@ -163,6 +163,8 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_hvm;
> > if ( IS_ENABLED(CONFIG_PV) )
> > pi->capabilities |= XEN_SYSCTL_PHYSCAP_pv;
> > + if ( hvm_hap_supported() )
> > + pi->capabilities |= XEN_SYSCTL_PHYSCAP_hap;
> > }
> >
> > long arch_do_sysctl(
> > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> > index 36b3f8c429..e02d7ce4c6 100644
> > --- a/xen/include/public/sysctl.h
> > +++ b/xen/include/public/sysctl.h
> > @@ -90,6 +90,10 @@ struct xen_sysctl_tbuf_op {
> > /* The platform supports direct access to I/O devices with IOMMU. */
> > #define _XEN_SYSCTL_PHYSCAP_directio 2
> > #define XEN_SYSCTL_PHYSCAP_directio (1u<<_XEN_SYSCTL_PHYSCAP_directio)
> > +/* The platform supports Hardware Assisted Paging. */
> > +#define _XEN_SYSCTL_PHYSCAP_hap 3
> > +#define XEN_SYSCTL_PHYSCAP_hap (1u<<_XEN_SYSCTL_PHYSCAP_hap)
> > +
> > struct xen_sysctl_physinfo {
> > uint32_t threads_per_core;
> > uint32_t cores_per_socket;
> > --
> > 2.22.0
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
