Re: [Xen-devel] [PATCH 14/16] SUPPORT.md: Add statement on PCI passthrough

[Marek Marczykowski-Górecki]

On Mon, Nov 13, 2017 at 03:41:24PM +, George Dunlap wrote:
> Signed-off-by: George Dunlap <george.dun...@citrix.com>
> ---
> CC: Ian Jackson <ian.jack...@citrix.com>
> CC: Wei Liu <wei.l...@citrix.com>
> CC: Andrew Cooper <andrew.coop...@citrix.com>
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Stefano Stabellini <sstabell...@kernel.org>
> CC: Konrad Wilk <konrad.w...@oracle.com>
> CC: Tim Deegan <t...@xen.org>
> CC: Rich Persaud <pers...@gmail.com>
> CC: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> CC: Christopher Clark <christopher.w.cl...@gmail.com>
> CC: James McKenzie <james.mcken...@bromium.com>
> ---
>  SUPPORT.md | 33 -
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index 3e352198ce..a8388f3dc5 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md

(...)

> @@ -522,6 +536,23 @@ Virtual Performance Management Unit for HVM guests
>  Disabled by default (enable with hypervisor command line option).
>  This feature is not security supported: see 
> http://xenbits.xen.org/xsa/advisory-163.html
>  
> +### x86/PCI Device Passthrough
> +
> +Status: Supported, with caveats
> +
> +Only systems using IOMMUs will be supported.

s/will be/are/ ?

> +
> +Not compatible with migration, altp2m, introspection, memory sharing, or 
> memory paging.
> +
> +Because of hardware limitations
> +(affecting any operating system or hypervisor),
> +it is generally not safe to use this feature 
> +to expose a physical device to completely untrusted guests.
> +However, this feature can still confer significant security benefit 
> +when used to remove drivers and backends from domain 0
> +(i.e., Driver Domains).
> +See docs/PCI-IOMMU-bugs.txt for more information.
> +
>  ### ARM/Non-PCI device passthrough
>  
>  Status: Supported

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/5] docs: add PV sound device config

[Marek Marczykowski-Górecki]

On Mon, Oct 02, 2017 at 12:49:24PM +0300, Oleksandr Grytsov wrote:
> +=item 

Re: [Xen-devel] [PATCH 2/2] python/libxc: extend the call to get/set cap for credit2

[Marek Marczykowski-Górecki]

On Wed, Sep 27, 2017 at 12:46:22PM +0100, Wei Liu wrote:
> Commit 68817024 ("xen: credit2: allow to set and get utilization cap")
> added a new parameter. Implement it for the python binding as well.
> 
> Coverity-ID: 1418532
> 
> Signed-off-by: Wei Liu <wei.l...@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

> ---
> Cc: George Dunlap <george.dun...@eu.citrix.com>
> Cc: Dario Faggioli <dario.faggi...@citrix.com>
> Cc: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> 
> Compile-test only.
> ---
>  tools/python/xen/lowlevel/xc/xc.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/python/xen/lowlevel/xc/xc.c 
> b/tools/python/xen/lowlevel/xc/xc.c
> index aa9f8e4d9e..f501764100 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -1348,16 +1348,19 @@ static PyObject 
> *pyxc_sched_credit2_domain_set(XcObject *self,
>  {
>  uint32_t domid;
>  uint16_t weight;
> -static char *kwd_list[] = { "domid", "weight", NULL };
> -static char kwd_type[] = "I|H";
> -struct xen_domctl_sched_credit2 sdom;
> +uint16_t cap;
> +static char *kwd_list[] = { "domid", "weight", "cap", NULL };
> +static char kwd_type[] = "I|HH";
> +struct xen_domctl_sched_credit2 sdom = { };
>  
>  weight = 0;
> +cap = 0;
>  if( !PyArg_ParseTupleAndKeywords(args, kwds, kwd_type, kwd_list,
> - , ) )
> + , , ) )
>  return NULL;
>  
>  sdom.weight = weight;
> +sdom.cap = cap;
>  
>  if ( xc_sched_credit2_domain_set(self->xc_handle, domid, ) != 0 )
>  return pyxc_error_to_exception(self->xc_handle);
> @@ -1369,7 +1372,7 @@ static PyObject *pyxc_sched_credit2_domain_set(XcObject 
> *self,
>  static PyObject *pyxc_sched_credit2_domain_get(XcObject *self, PyObject 
> *args)
>  {
>  uint32_t domid;
> -struct xen_domctl_sched_credit2 sdom;
> +struct xen_domctl_sched_credit2 sdom = { };
>  
>  if( !PyArg_ParseTuple(args, "I", ) )
>  return NULL;
> @@ -1377,8 +1380,8 @@ static PyObject *pyxc_sched_credit2_domain_get(XcObject 
> *self, PyObject *args)
>  if ( xc_sched_credit2_domain_get(self->xc_handle, domid, ) != 0 )
>  return pyxc_error_to_exception(self->xc_handle);
>  
> -    return Py_BuildValue("{s:H}",
> - "weight",  sdom.weight);
> +return Py_BuildValue("{s:HH}",
> + "weight", "cap",  sdom.weight, sdom.cap);
>  }
>  
>  static PyObject *pyxc_domain_setmaxmem(XcObject *self, PyObject *args)

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 1/3] python: Add binding for xs_fileno()

[Marek Marczykowski-Górecki]

On Fri, Sep 22, 2017 at 05:21:12PM +0100, Euan Harris wrote:
> xs_fileno() returns a file descriptor which receives events when Xenstore
> watches fire.   Exposing this in the Python bindings is a prerequisite
> for writing event-driven clients in Python.
> 
> Signed-off-by: Euan Harris <euan.har...@citrix.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> Reviewed-by: Wei Liu <wei.l...@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

> ---
> Changed since v2:
>  * Use PyLong_FromLong instead of PyInt_FromLong
> ---
>  tools/python/xen/lowlevel/xs/xs.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/tools/python/xen/lowlevel/xs/xs.c 
> b/tools/python/xen/lowlevel/xs/xs.c
> index aba5a20..3a827b0 100644
> --- a/tools/python/xen/lowlevel/xs/xs.c
> +++ b/tools/python/xen/lowlevel/xs/xs.c
> @@ -453,6 +453,25 @@ static PyObject *xspy_watch(XsHandle *self, PyObject 
> *args)
>  }
>  
>  
> +#define xspy_fileno_doc "\n"  \
> + "Return the FD to poll for notifications when watches fire.\n"   \
> + "Returns: [int] file descriptor.\n"\
> + "\n"
> +
> +static PyObject *xspy_fileno(XsHandle *self)
> +{
> +struct xs_handle *xh = xshandle(self);
> +int fd;
> +
> +if (!xh)
> +return NULL;
> +
> +fd = xs_fileno(xh);
> +
> +return PyLong_FromLong(fd);
> +}
> +
> +
>  #define xspy_read_watch_doc "\n" \
>   "Read a watch notification.\n"  \
>   "\n"\
> @@ -887,6 +906,7 @@ static PyMethodDef xshandle_methods[] = {
>  XSPY_METH(release_domain,METH_VARARGS),
>  XSPY_METH(close, METH_NOARGS),
>  XSPY_METH(get_domain_path,   METH_VARARGS),
> +XSPY_METH(fileno,METH_NOARGS),
>  { NULL /* Sentinel. */ },
>  };
>  

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 3/3] python: Add binding for non-blocking xs_check_watch()

[Marek Marczykowski-Górecki]

On Thu, Sep 21, 2017 at 05:47:08PM +0100, Euan Harris wrote:
> xs_check_watch() checks for watch notifications without blocking.
> Together with the binding for xs_fileno(), this makes it possible
> to write event-driven clients in Python.
> 
> Signed-off-by: Euan Harris <euan.har...@citrix.com>
> Reviewed-by: Wei Liu <wei.l...@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

> ---
>  tools/python/xen/lowlevel/xs/xs.c | 28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/tools/python/xen/lowlevel/xs/xs.c 
> b/tools/python/xen/lowlevel/xs/xs.c
> index 2af5e07..4710002 100644
> --- a/tools/python/xen/lowlevel/xs/xs.c
> +++ b/tools/python/xen/lowlevel/xs/xs.c
> @@ -474,6 +474,33 @@ static PyObject *xspy_fileno(XsHandle *self)
>  }
>  
>  
> +#define xspy_check_watch_doc "\n"\
> + "Check for watch notifications without blocking.\n" \
> + "\n"\
> + "Returns: [tuple] (path, token).\n" \
> + " None if no watches have fired.\n" \
> + "Raises xen.lowlevel.xs.Error on error.\n"  \
> + "\n"
> +
> +static PyObject *xspy_check_watch(XsHandle *self, PyObject *args)
> +{
> +struct xs_handle *xh = xshandle(self);
> +PyObject *val = NULL;
> +char **xsval;
> +
> +if (!xh)
> +return NULL;
> +
> +xsval = xs_check_watch(xh);
> +if (!xsval) {
> +return none(errno == EAGAIN);
> +}
> +
> +val = match_watch_by_token(self, xsval);
> +free(xsval);
> +return val;
> +}
> +
>  #define xspy_read_watch_doc "\n" \
>   "Read a watch notification.\n"  \
>   "\n"\
> @@ -911,6 +938,7 @@ static PyMethodDef xshandle_methods[] = {
>  XSPY_METH(set_permissions,   METH_VARARGS),
>  XSPY_METH(watch, METH_VARARGS),
>  XSPY_METH(read_watch,    METH_NOARGS),
> +XSPY_METH(check_watch,   METH_NOARGS),
>  XSPY_METH(unwatch,   METH_VARARGS),
>  XSPY_METH(transaction_start, METH_NOARGS),
>  XSPY_METH(transaction_end,   METH_VARARGS | METH_KEYWORDS),

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 2/3] python: Extract registered watch search logic from xspy_read_watch()

[Marek Marczykowski-Górecki]

On Thu, Sep 21, 2017 at 05:47:07PM +0100, Euan Harris wrote:
> When a watch fires, xspy_read_watch() checks whether the client has
> registered interest in the path which changed and, if so, returns the
> path and a client-supplied token.   The binding for xs_check_watch()
> needs to do the same, so this patch extracts the search code into a
> separate function.
> 
> Signed-off-by: Euan Harris <euan.har...@citrix.com>
> Reviewed-by: Wei Liu <wei.l...@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

> ---
> Changed since v1:
>  * Remove stray newline
>  * Fix indentation
> 
>  tools/python/xen/lowlevel/xs/xs.c | 60 
> ---
>  1 file changed, 37 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/python/xen/lowlevel/xs/xs.c 
> b/tools/python/xen/lowlevel/xs/xs.c
> index 9f1b916..2af5e07 100644
> --- a/tools/python/xen/lowlevel/xs/xs.c
> +++ b/tools/python/xen/lowlevel/xs/xs.c
> @@ -77,6 +77,8 @@ static inline struct xs_handle *xshandle(XsHandle *self)
>  
>  static void remove_watch(XsHandle *xsh, PyObject *token);
>  
> +static PyObject *match_watch_by_token(XsHandle *self, char **xsval);
> +
>  static PyObject *none(bool result);
>  
>  static int parse_transaction_path(XsHandle *self, PyObject *args,
> @@ -484,8 +486,6 @@ static PyObject *xspy_read_watch(XsHandle *self, PyObject 
> *args)
>  struct xs_handle *xh = xshandle(self);
>  PyObject *val = NULL;
>  char **xsval;
> -PyObject *token;
> -int i;
>  unsigned int num;
>  
>  if (!xh)
> @@ -497,29 +497,16 @@ again:
>  Py_END_ALLOW_THREADS
>  if (!xsval) {
>  PyErr_SetFromErrno(xs_error);
> -goto exit;
> -}
> -if (sscanf(xsval[XS_WATCH_TOKEN], "%li", (unsigned long *)) != 1) {
> - xs_set_error(EINVAL);
> -goto exit;
> -}
> -for (i = 0; i < PyList_Size(self->watches); i++) {
> -if (token == PyList_GetItem(self->watches, i))
> -break;
> -}
> -if (i == PyList_Size(self->watches)) {
> -  /* We do not have a registered watch for the one that has just fired.
> - Ignore this -- a watch that has been recently deregistered can still
> - have watches in transit.  This is a blocking method, so go back to
> - read again.
> -  */
> -  free(xsval);
> -  goto again;
> +return val;
>  }
> -/* Create tuple (path, token). */
> -val = Py_BuildValue("(sO)", xsval[XS_WATCH_PATH], token);
> - exit:
> +
> +val = match_watch_by_token(self, xsval);
>  free(xsval);
> +
> +if (!val && errno == EAGAIN) {
> +goto again;
> +}
> +
>  return val;
>  }
>  
> @@ -868,6 +855,33 @@ static int parse_transaction_path(XsHandle *self, 
> PyObject *args,
>  }
>  
>  
> +static PyObject *match_watch_by_token(XsHandle *self, char **xsval)
> +{
> +PyObject *token;
> +int i;
> +
> +if (sscanf(xsval[XS_WATCH_TOKEN], "%li", (unsigned long *)) != 1) {
> +xs_set_error(EINVAL);
> +return NULL;
> +}
> +for (i = 0; i < PyList_Size(self->watches); i++) {
> +if (token == PyList_GetItem(self->watches, i))
> +break;
> +}
> +if (i == PyList_Size(self->watches)) {
> +/* We do not have a registered watch for the one that has just fired.
> +   Ignore this -- a watch that has been recently deregistered can 
> still
> +   have watches in transit.
> +*/
> +xs_set_error(EAGAIN);
> +return NULL;
> +}
> +
> +/* Create tuple (path, token). */
> +return Py_BuildValue("(sO)", xsval[XS_WATCH_PATH], token);
> +}
> +
> +
>  static PyObject *none(bool result)
>  {
>  if (result) {

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 1/3] python: Add binding for xs_fileno()

[Marek Marczykowski-Górecki]

On Thu, Sep 21, 2017 at 05:47:06PM +0100, Euan Harris wrote:
> xs_fileno() returns a file descriptor which receives events when Xenstore
> watches fire.   Exposing this in the Python bindings is a prerequisite
> for writing event-driven clients in Python.
> 
> Signed-off-by: Euan Harris <euan.har...@citrix.com>
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> Reviewed-by: Wei Liu <wei.l...@citrix.com>
> ---
>  tools/python/xen/lowlevel/xs/xs.c | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/tools/python/xen/lowlevel/xs/xs.c 
> b/tools/python/xen/lowlevel/xs/xs.c
> index aba5a20..9f1b916 100644
> --- a/tools/python/xen/lowlevel/xs/xs.c
> +++ b/tools/python/xen/lowlevel/xs/xs.c
> @@ -453,6 +453,25 @@ static PyObject *xspy_watch(XsHandle *self, PyObject 
> *args)
>  }
>  
>  
> +#define xspy_fileno_doc "\n"  \
> + "Return the FD to poll for notifications when watches fire.\n"   \
> + "Returns: [int] file descriptor.\n"\
> + "\n"
> +
> +static PyObject *xspy_fileno(XsHandle *self)
> +{
> +struct xs_handle *xh = xshandle(self);
> +int fd;
> +
> +if (!xh)
> +return NULL;
> +
> +fd = xs_fileno(xh);
> +
> +return PyInt_FromLong(fd);

Use PyLong_FromLong. There is no PyInt_* in py3k. But for convenience we
have #define PyLong_FromLong -> PyInt_FromLong for python 2.

> +}
> +
> +
>  #define xspy_read_watch_doc "\n" \
>   "Read a watch notification.\n"  \
>   "\n"\
> @@ -887,6 +906,7 @@ static PyMethodDef xshandle_methods[] = {
>  XSPY_METH(release_domain,METH_VARARGS),
>  XSPY_METH(close, METH_NOARGS),
>  XSPY_METH(get_domain_path,   METH_VARARGS),
> +XSPY_METH(fileno,METH_NOARGS),
>  { NULL /* Sentinel. */ },
>  };
>  

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Xen 4.8.2 released

[Marek Marczykowski-Górecki]

On Mon, Sep 11, 2017 at 03:06:48AM -0600, Jan Beulich wrote:
> >>> On 10.09.17 at 01:53, <marma...@invisiblethingslab.com> wrote:
> > On Wed, Sep 06, 2017 at 08:42:33AM -0600, Jan Beulich wrote:
> >> All,
> >> 
> >> I am pleased to announce the release of Xen 4.8.2. This is
> >> available immediately from its git repository
> >> http://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=refs/heads/stable-4.8
> >>  
> >> (tag RELEASE-4.8.2) or from the XenProject download page
> >> http://www.xenproject.org/downloads/xen-archives/xen-project-48-series/xen-482.html
> >>  
> >> (where a list of changes can also be found).
> >> 
> >> We recommend all users of the 4.8 stable series to update to this
> >> latest point release.
> > 
> > The announcement on the website has wrong link (to 4.8.1 instead of
> > 4.8.2).
> 
> Hmm, thanks for pointing this out, but unless Lars knows without
> further context where this wrong link is, it would have helped if
> you identified the page having that bad link. I can spot such a
> wrong link in the blog post - is that what you're referring to? 

Yes, this one:
https://blog.xenproject.org/2017/09/06/xen-project-4-8-2-is-available/

> Lars,
> could you correct that?
> 
> Jan
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Xen 4.8.2 released

[Marek Marczykowski-Górecki]

On Wed, Sep 06, 2017 at 08:42:33AM -0600, Jan Beulich wrote:
> All,
> 
> I am pleased to announce the release of Xen 4.8.2. This is
> available immediately from its git repository
> http://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=refs/heads/stable-4.8 
> (tag RELEASE-4.8.2) or from the XenProject download page
> http://www.xenproject.org/downloads/xen-archives/xen-project-48-series/xen-482.html
>  
> (where a list of changes can also be found).
> 
> We recommend all users of the 4.8 stable series to update to this
> latest point release.

The announcement on the website has wrong link (to 4.8.1 instead of
4.8.2).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 5/5] tools/libxenctrl: use new xenforeignmemory API to seed grant table

[Marek Marczykowski-Górecki]

On Fri, Aug 04, 2017 at 01:26:21PM +0100, Wei Liu wrote:
> On Wed, Aug 02, 2017 at 10:59:49AM +0100, Paul Durrant wrote:
> > A previous patch added support for priv-mapping guest resources directly
> > (rather than having to foreign-map, which requires P2M modification for
> > HVM guests).
> > 
> > This patch makes use of the new API to seed the guest grant table unless
> > the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> > case the old scheme is used.
> > 
> 
> The code mostly looks fine.
> 
> What's the benefit of doing this?

Also, I see changed signature of xc_dom_gnttab_seed (it got is_hvm
parameter). Wei, what is the policy for backward incompatible libxc API
changes?

> > NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
> >   actually unnecessary, as the grant table has already been seeded
> >   by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> > 
> > Signed-off-by: Paul Durrant <paul.durr...@citrix.com>
> > ---
> > Cc: Ian Jackson <ian.jack...@eu.citrix.com>
> > Cc: Wei Liu <wei.l...@citrix.com>
> 
> BTW Marek needs to be CC on changes to Python bindings. I've done that
> for you.

For Python bits:
Acked-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

> > ---
> >  tools/libxc/include/xc_dom.h|   8 +--
> >  tools/libxc/xc_dom_boot.c   | 102 
> > 
> >  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
> >  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
> >  tools/libxl/libxl_dom.c |   1 -
> >  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
> >  6 files changed, 92 insertions(+), 37 deletions(-)
> > 
> > diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> > index ce47058c41..d6ca0a8680 100644
> > --- a/tools/libxc/include/xc_dom.h
> > +++ b/tools/libxc/include/xc_dom.h
> > @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, 
> > xen_pfn_t pfn,
> >  int xc_dom_boot_image(struct xc_dom_image *dom);
> >  int xc_dom_compat_check(struct xc_dom_image *dom);
> >  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -   xen_pfn_t console_gmfn,
> > -   xen_pfn_t xenstore_gmfn,
> > -   domid_t console_domid,
> > -   domid_t xenstore_domid);
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> > +   bool is_hvm,
> > xen_pfn_t console_gmfn,
> > xen_pfn_t xenstore_gmfn,
> > domid_t console_domid,
> > diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> > index c3b44dd399..fc3174ad7e 100644
> > --- a/tools/libxc/xc_dom_boot.c
> > +++ b/tools/libxc/xc_dom_boot.c
> > @@ -280,11 +280,11 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface 
> > *xch, domid_t domid)
> >  return gmfn;
> >  }
> >  
> > -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> > -   xen_pfn_t console_gmfn,
> > -   xen_pfn_t xenstore_gmfn,
> > -   domid_t console_domid,
> > -   domid_t xenstore_domid)
> > +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> > +  xen_pfn_t console_gmfn,
> > +  xen_pfn_t xenstore_gmfn,
> > +  domid_t console_domid,
> > +  domid_t xenstore_domid)
> >  {
> >  
> >  xen_pfn_t gnttab_gmfn;
> > @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t 
> > domid,
> >  return 0;
> >  }
> >  
> > -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > -   xen_pfn_t console_gpfn,
> > -   xen_pfn_t xenstore_gpfn,
> > -   domid_t console_domid,
> > -   domid_t xenstore_domid)
> > +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> > +  xen_pfn_t console_gpfn,
> > +  xen_pfn_t xenstore_gpfn,
> > +  domid_t console_domid,
> > +  domid_t xenstore_domid)
> >  {

Re: [Xen-devel] [PATCH v2 1/2] libxl: use xen-blkback for 'vbd' disk types by default

[Marek Marczykowski-Górecki]

On Mon, Jul 31, 2017 at 05:01:08PM +0100, Wei Liu wrote:
> On Mon, Jul 31, 2017 at 04:56:04PM +0100, Wei Liu wrote:
> > On Fri, Jul 28, 2017 at 06:42:13PM +0200, Marek Marczykowski-Górecki wrote:
> > > This will allow later to make HVM domain without qemu in dom0 (in
> > > addition to the one in stubdomain).
> > > 
> > > Signed-off-by: Marek Marczykowski-Górecki 
> > > <marma...@invisiblethingslab.com>
> > > 
> > > ---
> > > This is extracted from v1 of "libxl: do not start dom0 qemu for
> > > stubdomain when not needed".
> > > 
> > > Signed-off-by: Marek Marczykowski-Górecki 
> > > <marma...@invisiblethingslab.com>
> > > ---
> > >  tools/libxl/libxl_disk.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
> > > index 63de75c..7842d9b 100644
> > > --- a/tools/libxl/libxl_disk.c
> > > +++ b/tools/libxl/libxl_disk.c
> > > @@ -56,10 +56,12 @@ static void disk_eject_xswatch_callback(libxl__egc 
> > > *egc, libxl__ev_xswatch *w,
> > >  "/local/domain/%d/backend/%" TOSTRING(BACKEND_STRING_SIZE)
> > > "[a-z]/%*d/%*d",
> > > >backend_domid, backend_type);
> > > -if (!strcmp(backend_type, "tap") || !strcmp(backend_type, "vbd")) {
> > > +if (!strcmp(backend_type, "tap")) {
> > >  disk->backend = LIBXL_DISK_BACKEND_TAP;
> > >  } else if (!strcmp(backend_type, "qdisk")) {
> > >  disk->backend = LIBXL_DISK_BACKEND_QDISK;
> > > +} else if (!strcmp(backend_type, "vbd")) {
> > > +disk->backend = LIBXL_DISK_BACKEND_PHY;
> > 
> > Wait, it only occurred to me until now this patch is changing
> > disk_eject_xswatch_callback.
> > 
> > Is this a bug fix? How is it possible for the backend_type to be "vbd"
> > when there isn't such thing in libxl_types.idl?
> 
> Oh, I'm an idiot. That's read from xenstore path. I think this patch is
> correct. But I still tend to think this is a bug fix. How do you
> discover this problem? Has disk eject ever worked for you?

Hmm, I think I've failed with forward-porting this patch. In current Xen
it isn't possible to create cdrom with backend_type!=qdisk...

So, this patch alone isn't sufficient. Please ignore it.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/2] libxl: do not start dom0 qemu for stubdomain when not needed

[Marek Marczykowski-Górecki]

Do not setup vfb+vkb when no access method was configured. Then check if
qemu is really needed.

The only not configurable thing forcing qemu running in dom0 after this
change are consoles used to save/restore. But even in that case, there
is much smaller part of qemu exposed.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_dm.c | 54 --
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 44ebd70..e0e6a99 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1868,13 +1868,17 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
libxl__stub_dm_spawn_state *sdss)
 ret = libxl__domain_build_info_setdefault(gc, _config->b_info);
 if (ret) goto out;
 
-GCNEW(vfb);
-GCNEW(vkb);
-libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
-dm_config->vfbs = vfb;
-dm_config->num_vfbs = 1;
-dm_config->vkbs = vkb;
-dm_config->num_vkbs = 1;
+if (libxl_defbool_val(guest_config->b_info.u.hvm.vnc.enable)
+|| libxl_defbool_val(guest_config->b_info.u.hvm.spice.enable)
+|| libxl_defbool_val(guest_config->b_info.u.hvm.sdl.enable)) {
+GCNEW(vfb);
+GCNEW(vkb);
+libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
+dm_config->vfbs = vfb;
+dm_config->num_vfbs = 1;
+dm_config->vkbs = vkb;
+dm_config->num_vkbs = 1;
+}
 
 stubdom_state->pv_kernel.path
 = libxl__abs_path(gc, "ioemu-stubdom.gz", 
libxl__xenfirmwaredir_path());
@@ -1959,6 +1963,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 libxl__domain_build_state *const d_state = sdss->dm.build_state;
 libxl__domain_build_state *const stubdom_state = >dm_state;
 uint32_t dm_domid = sdss->pvqemu.guest_domid;
+int need_qemu;
 
 if (ret) {
 LOGD(ERROR, guest_domid, "error connecting disk devices");
@@ -1975,12 +1980,16 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 if (ret)
 goto out;
 }
-ret = libxl__device_vfb_add(gc, dm_domid, _config->vfbs[0]);
-if (ret)
-goto out;
-ret = libxl__device_vkb_add(gc, dm_domid, _config->vkbs[0]);
-if (ret)
-goto out;
+if (dm_config->num_vfbs) {
+ret = libxl__device_vfb_add(gc, dm_domid, _config->vfbs[0]);
+if (ret)
+goto out;
+}
+if (dm_config->num_vkbs) {
+ret = libxl__device_vkb_add(gc, dm_domid, _config->vkbs[0]);
+if (ret)
+goto out;
+}
 
 if (guest_config->b_info.u.hvm.serial)
 num_console++;
@@ -1988,7 +1997,6 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 console = libxl__calloc(gc, num_console, sizeof(libxl__device_console));
 
 for (i = 0; i < num_console; i++) {
-libxl__device device;
 console[i].devid = i;
 console[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU;
 /* STUBDOM_CONSOLE_LOGGING (console 0) is for minios logging
@@ -2005,6 +2013,9 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 if (ret) goto out;
 console[i].output = GCSPRINTF("file:%s", filename);
 free(filename);
+/* will be changed back to LIBXL__CONSOLE_BACKEND_IOEMU if qemu
+ * will be in use */
+console[i].consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
 break;
 case STUBDOM_CONSOLE_SAVE:
 console[i].output = GCSPRINTF("file:%s",
@@ -2019,6 +2030,14 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 console[i].output = "pty";
 break;
 }
+}
+
+need_qemu = libxl__need_xenpv_qemu(gc, dm_config);
+
+for (i = 0; i < num_console; i++) {
+libxl__device device;
+if (need_qemu)
+console[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU;
 ret = libxl__device_console_add(gc, dm_domid, [i],
 i == STUBDOM_CONSOLE_LOGGING ? stubdom_state : NULL,
 );
@@ -2032,7 +2051,12 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 sdss->pvqemu.build_state = >dm_state;
 sdss->pvqemu.callback = spawn_stubdom_pvqemu_cb;
 
-libxl__spawn_local_dm(egc, >pvqemu);
+if (!need_qemu) {
+/* If dom0 qemu not needed, do not launch it */
+spawn_stubdom_pvqemu_cb(egc, >pvqemu, 0);
+} else {
+libxl__spawn_local_dm(egc, >pvqemu);
+}
 
 return;
 
-- 
2.7.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 1/2] libxl: use xen-blkback for 'vbd' disk types by default

[Marek Marczykowski-Górecki]

This will allow later to make HVM domain without qemu in dom0 (in
addition to the one in stubdomain).

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

---
This is extracted from v1 of "libxl: do not start dom0 qemu for
stubdomain when not needed".

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_disk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index 63de75c..7842d9b 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -56,10 +56,12 @@ static void disk_eject_xswatch_callback(libxl__egc *egc, 
libxl__ev_xswatch *w,
 "/local/domain/%d/backend/%" TOSTRING(BACKEND_STRING_SIZE)
"[a-z]/%*d/%*d",
>backend_domid, backend_type);
-if (!strcmp(backend_type, "tap") || !strcmp(backend_type, "vbd")) {
+if (!strcmp(backend_type, "tap")) {
 disk->backend = LIBXL_DISK_BACKEND_TAP;
 } else if (!strcmp(backend_type, "qdisk")) {
 disk->backend = LIBXL_DISK_BACKEND_QDISK;
+} else if (!strcmp(backend_type, "vbd")) {
+disk->backend = LIBXL_DISK_BACKEND_PHY;
 } else {
 disk->backend = LIBXL_DISK_BACKEND_UNKNOWN;
 }
-- 
2.7.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxl: do not start dom0 qemu for stubdomain when not needed

[Marek Marczykowski-Górecki]

On Fri, Jul 28, 2017 at 05:17:12PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH] libxl: do not start dom0 qemu for stubdomain 
> when not needed"):
> > On Fri, Jul 28, 2017 at 06:05:13PM +0200, Marek Marczykowski-Górecki wrote:
> > > On Fri, Jul 28, 2017 at 04:17:51PM +0100, Wei Liu wrote:
> > > > On Thu, Jul 27, 2017 at 12:44:25AM +0200, Marek Marczykowski-Górecki 
> > > > wrote:
> > > > > +if (libxl_defbool_val(guest_config->b_info.u.hvm.vnc.enable) ||
> > > > > +
> > > > > libxl_defbool_val(guest_config->b_info.u.hvm.spice.enable) ||
> > > > > +
> > > > > libxl_defbool_val(guest_config->b_info.u.hvm.sdl.enable)) {
> > > > 
> > > > Indentation.
> > > 
> > > Should it really be indented at the same level as the code inside this
> > > block? Looks misleading (you need to look for ") {" to see where
> > > condition ends).
> > 
> > That's how existing code in libxl is like, isn't it?
> 
> Yes.  Sadly.
> 
> If you like we could have a bunfight about changing the indent level.
> I like 2, personally :-P.
> 
> A compromise might be to move the || to the start of the next line, so
> 
>if (cond1
>|| cond2
>|| cond3) {
>code1;
>code2;
> 
> I have done that sometimes in libxl and no-one has objected.

Ok, it would be much better, thanks.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxl: do not start dom0 qemu for stubdomain when not needed

[Marek Marczykowski-Górecki]

On Fri, Jul 28, 2017 at 04:17:51PM +0100, Wei Liu wrote:
> On Thu, Jul 27, 2017 at 12:44:25AM +0200, Marek Marczykowski-Górecki wrote:
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 44ebd70..c9aefe15 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -1868,13 +1868,17 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
> > libxl__stub_dm_spawn_state *sdss)
> >  ret = libxl__domain_build_info_setdefault(gc, _config->b_info);
> >  if (ret) goto out;
> >  
> > -GCNEW(vfb);
> > -GCNEW(vkb);
> > -libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
> > -dm_config->vfbs = vfb;
> > -dm_config->num_vfbs = 1;
> > -dm_config->vkbs = vkb;
> > -dm_config->num_vkbs = 1;
> > +if (libxl_defbool_val(guest_config->b_info.u.hvm.vnc.enable) ||
> > +libxl_defbool_val(guest_config->b_info.u.hvm.spice.enable) ||
> > +libxl_defbool_val(guest_config->b_info.u.hvm.sdl.enable)) {
> 
> Indentation.

Should it really be indented at the same level as the code inside this
block? Looks misleading (you need to look for ") {" to see where
condition ends).

> > +GCNEW(vfb);
> > +GCNEW(vkb);
> > +libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, 
> > vkb);
> > +    dm_config->vfbs = vfb;
> > +dm_config->num_vfbs = 1;
> > +dm_config->vkbs = vkb;
> > +dm_config->num_vkbs = 1;
> > +}

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxl: do not start dom0 qemu for stubdomain when not needed

[Marek Marczykowski-Górecki]

On Fri, Jul 28, 2017 at 04:17:51PM +0100, Wei Liu wrote:
> On Thu, Jul 27, 2017 at 12:44:25AM +0200, Marek Marczykowski-Górecki wrote:
> > Use xen-blkback for 'vbd' disk types by default and do not setup vfb+vkb
> > when no access method was configured. Then check if qemu is really
> > needed.
> > 
> > The only not configurable thing forcing qemu running in dom0 after this
> > change are consoles used to save/restore. But even in that case, there
> > is much smaller part of qemu exposed.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> > ---
> >  tools/libxl/libxl_disk.c |  4 +++-
> >  tools/libxl/libxl_dm.c   | 52 
> > ++--
> >  2 files changed, 40 insertions(+), 16 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
> > index 63de75c..7842d9b 100644
> > --- a/tools/libxl/libxl_disk.c
> > +++ b/tools/libxl/libxl_disk.c
> > @@ -56,10 +56,12 @@ static void disk_eject_xswatch_callback(libxl__egc 
> > *egc, libxl__ev_xswatch *w,
> >  "/local/domain/%d/backend/%" TOSTRING(BACKEND_STRING_SIZE)
> > "[a-z]/%*d/%*d",
> > >backend_domid, backend_type);
> > -if (!strcmp(backend_type, "tap") || !strcmp(backend_type, "vbd")) {
> > +if (!strcmp(backend_type, "tap")) {
> >  disk->backend = LIBXL_DISK_BACKEND_TAP;
> >  } else if (!strcmp(backend_type, "qdisk")) {
> >  disk->backend = LIBXL_DISK_BACKEND_QDISK;
> > +} else if (!strcmp(backend_type, "vbd")) {
> > +disk->backend = LIBXL_DISK_BACKEND_PHY;
> 
> This should be split into a separate patch.
> 
> >  } else {
> >  disk->backend = LIBXL_DISK_BACKEND_UNKNOWN;
> >  }
> > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> > index 44ebd70..c9aefe15 100644
> > --- a/tools/libxl/libxl_dm.c
> > +++ b/tools/libxl/libxl_dm.c
> > @@ -1868,13 +1868,17 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
> > libxl__stub_dm_spawn_state *sdss)
> >  ret = libxl__domain_build_info_setdefault(gc, _config->b_info);
> >  if (ret) goto out;
> >  
> > -GCNEW(vfb);
> > -GCNEW(vkb);
> > -libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
> > -dm_config->vfbs = vfb;
> > -dm_config->num_vfbs = 1;
> > -dm_config->vkbs = vkb;
> > -dm_config->num_vkbs = 1;
> > +if (libxl_defbool_val(guest_config->b_info.u.hvm.vnc.enable) ||
> > +libxl_defbool_val(guest_config->b_info.u.hvm.spice.enable) ||
> > +libxl_defbool_val(guest_config->b_info.u.hvm.sdl.enable)) {
> 
> Indentation.
> 
> > +GCNEW(vfb);
> > +GCNEW(vkb);
> > +libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, 
> > vkb);
> > +dm_config->vfbs = vfb;
> > +dm_config->num_vfbs = 1;
> > +dm_config->vkbs = vkb;
> > +dm_config->num_vkbs = 1;
> > +}
> >  
> >  stubdom_state->pv_kernel.path
> >  = libxl__abs_path(gc, "ioemu-stubdom.gz", 
> > libxl__xenfirmwaredir_path());
> > @@ -1959,6 +1963,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
> >  libxl__domain_build_state *const d_state = sdss->dm.build_state;
> >  libxl__domain_build_state *const stubdom_state = >dm_state;
> >  uint32_t dm_domid = sdss->pvqemu.guest_domid;
> > +int need_qemu;
> >  
> >  if (ret) {
> >  LOGD(ERROR, guest_domid, "error connecting disk devices");
> > @@ -1975,12 +1980,16 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
> >  if (ret)
> >  goto out;
> >  }
> > -ret = libxl__device_vfb_add(gc, dm_domid, _config->vfbs[0]);
> > -if (ret)
> > -goto out;
> > -ret = libxl__device_vkb_add(gc, dm_domid, _config->vkbs[0]);
> > -if (ret)
> > -goto out;
> > +if (dm_config->num_vfbs) {
> > +ret = libxl__device_vfb_add(gc, dm_domid, _config->vfbs[0]);
> > +if (ret)
> > +goto out;
> > +}
> > +if (dm_config->num_vkbs) {
> > +ret = libxl__device_vkb_add(gc, dm_domid, _config->vkbs[0]);
> > +if (ret)
> > +goto out;
> > +}
> >  
> >  if (guest_config->b_info.u.h

[Xen-devel] [PATCH] libxl: do not start dom0 qemu for stubdomain when not needed

[Marek Marczykowski-Górecki]

Use xen-blkback for 'vbd' disk types by default and do not setup vfb+vkb
when no access method was configured. Then check if qemu is really
needed.

The only not configurable thing forcing qemu running in dom0 after this
change are consoles used to save/restore. But even in that case, there
is much smaller part of qemu exposed.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_disk.c |  4 +++-
 tools/libxl/libxl_dm.c   | 52 ++--
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c
index 63de75c..7842d9b 100644
--- a/tools/libxl/libxl_disk.c
+++ b/tools/libxl/libxl_disk.c
@@ -56,10 +56,12 @@ static void disk_eject_xswatch_callback(libxl__egc *egc, 
libxl__ev_xswatch *w,
 "/local/domain/%d/backend/%" TOSTRING(BACKEND_STRING_SIZE)
"[a-z]/%*d/%*d",
>backend_domid, backend_type);
-if (!strcmp(backend_type, "tap") || !strcmp(backend_type, "vbd")) {
+if (!strcmp(backend_type, "tap")) {
 disk->backend = LIBXL_DISK_BACKEND_TAP;
 } else if (!strcmp(backend_type, "qdisk")) {
 disk->backend = LIBXL_DISK_BACKEND_QDISK;
+} else if (!strcmp(backend_type, "vbd")) {
+disk->backend = LIBXL_DISK_BACKEND_PHY;
 } else {
 disk->backend = LIBXL_DISK_BACKEND_UNKNOWN;
 }
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 44ebd70..c9aefe15 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1868,13 +1868,17 @@ void libxl__spawn_stub_dm(libxl__egc *egc, 
libxl__stub_dm_spawn_state *sdss)
 ret = libxl__domain_build_info_setdefault(gc, _config->b_info);
 if (ret) goto out;
 
-GCNEW(vfb);
-GCNEW(vkb);
-libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
-dm_config->vfbs = vfb;
-dm_config->num_vfbs = 1;
-dm_config->vkbs = vkb;
-dm_config->num_vkbs = 1;
+if (libxl_defbool_val(guest_config->b_info.u.hvm.vnc.enable) ||
+libxl_defbool_val(guest_config->b_info.u.hvm.spice.enable) ||
+libxl_defbool_val(guest_config->b_info.u.hvm.sdl.enable)) {
+GCNEW(vfb);
+GCNEW(vkb);
+libxl__vfb_and_vkb_from_hvm_guest_config(gc, guest_config, vfb, vkb);
+dm_config->vfbs = vfb;
+dm_config->num_vfbs = 1;
+dm_config->vkbs = vkb;
+dm_config->num_vkbs = 1;
+}
 
 stubdom_state->pv_kernel.path
 = libxl__abs_path(gc, "ioemu-stubdom.gz", 
libxl__xenfirmwaredir_path());
@@ -1959,6 +1963,7 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 libxl__domain_build_state *const d_state = sdss->dm.build_state;
 libxl__domain_build_state *const stubdom_state = >dm_state;
 uint32_t dm_domid = sdss->pvqemu.guest_domid;
+int need_qemu;
 
 if (ret) {
 LOGD(ERROR, guest_domid, "error connecting disk devices");
@@ -1975,12 +1980,16 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 if (ret)
 goto out;
 }
-ret = libxl__device_vfb_add(gc, dm_domid, _config->vfbs[0]);
-if (ret)
-goto out;
-ret = libxl__device_vkb_add(gc, dm_domid, _config->vkbs[0]);
-if (ret)
-goto out;
+if (dm_config->num_vfbs) {
+ret = libxl__device_vfb_add(gc, dm_domid, _config->vfbs[0]);
+if (ret)
+goto out;
+}
+if (dm_config->num_vkbs) {
+ret = libxl__device_vkb_add(gc, dm_domid, _config->vkbs[0]);
+if (ret)
+goto out;
+}
 
 if (guest_config->b_info.u.hvm.serial)
 num_console++;
@@ -1988,7 +1997,6 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 console = libxl__calloc(gc, num_console, sizeof(libxl__device_console));
 
 for (i = 0; i < num_console; i++) {
-libxl__device device;
 console[i].devid = i;
 console[i].consback = LIBXL__CONSOLE_BACKEND_IOEMU;
 /* STUBDOM_CONSOLE_LOGGING (console 0) is for minios logging
@@ -2005,6 +2013,9 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 if (ret) goto out;
 console[i].output = GCSPRINTF("file:%s", filename);
 free(filename);
+/* will be changed back to LIBXL__CONSOLE_BACKEND_IOEMU if qemu
+ * will be in use */
+console[i].consback = LIBXL__CONSOLE_BACKEND_XENCONSOLED;
 break;
 case STUBDOM_CONSOLE_SAVE:
 console[i].output = GCSPRINTF("file:%s",
@@ -2019,6 +2030,12 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
 console[i].output = "pty";
 break;
 }
+}
+
+need_qemu = libxl__need_xenpv_qemu(gc, dm_config

[Xen-devel] [PATCH] libvchan: Fix cleanup when xc_gntshr_open failed

[Marek Marczykowski-Górecki]

If xc_gntshr_open failed the only thing to cleanup is free allocated
memory. So instead of calling libxenvchan_close (which assume
valid calculated buffers being mmaped already) free memory and return.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libvchan/init.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c
index 83e1dee..e53f3a7 100644
--- a/tools/libvchan/init.c
+++ b/tools/libvchan/init.c
@@ -329,8 +329,10 @@ struct libxenvchan *libxenvchan_server_init(struct 
xentoollog_logger *logger,
}
 
ctrl->gntshr = xengntshr_open(logger, 0);
-   if (!ctrl->gntshr)
-   goto out;
+   if (!ctrl->gntshr) {
+   free(ctrl);
+   return 0;
+   }
 
if (init_evt_srv(ctrl, domain, logger))
goto out;
-- 
2.7.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH RFC] tools: Drop xc_cpuid_check() and bindings

[Marek Marczykowski-Górecki]

On Mon, Jul 17, 2017 at 01:54:19PM +, Christian Lindig wrote:
> 
> > On 17. Jul 2017, at 13:38, Andrew Cooper <andrew.coop...@citrix.com> wrote:
> > 
> > It turns out that Xapi has a library function using it, but that
> > function is dead so can be removed.
> 
> I am fine with the removal of the OCaml bindings and the patch for the OCaml 
> code. If the code is fundamentally broken it should be removed in any case 
> but like you already said, we are not aware of any clients.

Same for python bindings.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] offtopic: handling patches

[Marek Marczykowski-Górecki]

On Fri, Jun 30, 2017 at 06:18:11PM +0100, Andrew Cooper wrote:
> On 30/06/17 17:57, Marek Marczykowski-Górecki wrote:
> > Hi,
> >
> > How you guys handle patches with emails? I know git am and git
> > format-patch/send-email, but those tools are quite limited, especially
> > when handling patch series, subsequent versions etc.
> > What I miss there:
> >  - patch versioning (git notes could be used, but it doesn't survive git
> >commit --amend, nor git rebase)
> >  - keeping/versioning cover email
> >  - collecting Cc: from all patches in series into cover email
> >  - adding Reviewed-by, Acked-by etc tags
> >
> > I can't believe you all do this all manually ;)
> > Is there any commonly available tool I can't find, or everyone have own
> > scripts?
> 
> Manually, I'm afraid.  I've never found anything more automatic which works.
> 
> My general workflow is a single git branch which is always rebased onto
> staging.
> 
> Patch version information lives in the commit message under a --- line,

This is excellent idea! I don't know why never thought of it...

> and I am frequent user of `git commit --fixup/--squash` and `git rebase
> --interactive`.

Me too :)

> I've a separate directory tree where I format patch series including
> cover letters, before using `git send-email --dry-run *.patch` to send
> them.  These get recycled in a lazy fashon, typically once the series
> has been committed, but the old cover letters generally available in an
> adjacent directory when sending a newer series.

I've found git-series[1] tool. From the above list it allow you to diff
between series versions, and more importantly - keep cover letter in
git!

> For collecting and reviewing tags, look at the PatchWork `pwclient`
> utility.  Its `git-am` mode automagically collects tags, which is
> fantastically useful for applying a patch for committing.  (Then again,
> I do always manually check the conversation on list before actually
> committing the series.)

Thanks, indeed looks interesting.
BTW is this[2] the right instance? It doesn't looks to notice applied
patches.


[1] https://github.com/git-series/git-series
[2] https://patchwork.kernel.org/project/xen-devel/list
-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] offtopic: handling patches

[Marek Marczykowski-Górecki]

Hi,

How you guys handle patches with emails? I know git am and git
format-patch/send-email, but those tools are quite limited, especially
when handling patch series, subsequent versions etc.
What I miss there:
 - patch versioning (git notes could be used, but it doesn't survive git
   commit --amend, nor git rebase)
 - keeping/versioning cover email
 - collecting Cc: from all patches in series into cover email
 - adding Reviewed-by, Acked-by etc tags

I can't believe you all do this all manually ;)
Is there any commonly available tool I can't find, or everyone have own
scripts?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 2/4] libxl: fix osvm cpuid flag

[Marek Marczykowski-Górecki]

It's bit 9 not 10 (which is ibs).

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index fda8bf6..98c7c54 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -206,7 +206,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"skinit",   0x8001, NA, CPUID_REG_ECX, 12,  1},
 {"xop",  0x8001, NA, CPUID_REG_ECX, 11,  1},
 {"ibs",  0x8001, NA, CPUID_REG_ECX, 10,  1},
-{"osvw", 0x8001, NA, CPUID_REG_ECX, 10,  1},
+{"osvw", 0x8001, NA, CPUID_REG_ECX,  9,  1},
 {"3dnowprefetch",0x8001, NA, CPUID_REG_ECX,  8,  1},
 {"misalignsse",  0x8001, NA, CPUID_REG_ECX,  7,  1},
 {"sse4a",0x8001, NA, CPUID_REG_ECX,  6,  1},
-- 
git-series 0.9.1

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 1/4] libxl: add more cpuid flags handling

[Marek Marczykowski-Górecki]

This is result of parsing cpu_map.xml from libvirt.
The most important part is handling leaf 0x0007, but while at it add
other bits too.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 docs/man/xl.cfg.pod.5.in  | 21 +
 tools/libxl/libxl_cpuid.c | 40 -
 2 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 38084c7..ff32035 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1464,14 +1464,19 @@ apicidsize brandid clflush family localapicid maxleaf 
maxhvleaf model nc
 proccount procpkg stepping
 
 List of keys taking a character:
-3dnow 3dnowext 3dnowprefetch abm acpi aes altmovcr8 apic avx clfsh cmov
-cmplegacy cmpxchg16 cmpxchg8 cntxid dca de ds dscpl dtes64 est extapic f16c
-ffxsr fma4 fpu fxsr htt hypervisor ia64 ibs lahfsahf lm lwp mca mce misalignsse
-mmx mmxext monitor movbe msr mtrr nodeid nx osvw osxsave pae page1gb pat pbe
-pclmulqdq pdcm pge popcnt pse pse36 psn rdtscp skinit smx ss sse sse2 sse3
-sse4_1 sse4_2 sse4a ssse3 svm svm_decode svm_lbrv svm_npt svm_nrips
-svm_pausefilt svm_tscrate svm_vmcbclean syscall sysenter tbm tm tm2 topoext tsc
-vme vmx wdt x2apic xop xsave xtpr
+3dnow 3dnowext 3dnowprefetch abm acpi adx aes altmovcr8 apic arat avx avx2
+avx512-4fmaps avx512-4vnniw avx512bw avx512cd avx512dq avx512er avx512f
+avx512ifma avx512pf avx512vbmi avx512vl bmi1 bmi2 clflushopt clfsh clwb cmov
+cmplegacy cmpxchg16 cmpxchg8 cmt cntxid dca de ds dscpl dtes64 erms est extapic
+f16c ffxsr fma fma4 fpu fsgsbase fxsr hle htt hypervisor ia64 ibs invpcid
+invtsc lahfsahf lm lwp mca mce misalignsse mmx mmxext monitor movbe mpx msr
+mtrr nodeid nx ospke osvw osxsave pae page1gb pat pbe pcid pclmulqdq pdcm
+perfctr_core perfctr_nb pge pku popcnt pse pse36 psn rdrand rdseed rdtscp rtm
+sha skinit smap smep smx ss sse sse2 sse3 sse4.1 sse4.2 sse4_1 sse4_2 sse4a
+ssse3 svm svm_decode svm_lbrv svm_npt svm_nrips svm_pausefilt svm_tscrate
+svm_vmcbclean syscall sysenter tbm tm tm2 topoext tsc tsc-deadline tsc_adjust
+umip vme vmx wdt x2apic xop xsave xtpr
+
 
 The xend syntax is a list of values in the form of
 'leafnum:register=bitstring,register=bitstring'
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 24591e2..fda8bf6 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -100,11 +100,13 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"clflush",  0x0001, NA, CPUID_REG_EBX,  8,  8},
 {"brandid",  0x0001, NA, CPUID_REG_EBX,  0,  8},
 {"hypervisor",   0x0001, NA, CPUID_REG_ECX, 31,  1},
+{"rdrand",   0x0001, NA, CPUID_REG_ECX, 30,  1},
 {"f16c", 0x0001, NA, CPUID_REG_ECX, 29,  1},
 {"avx",  0x0001, NA, CPUID_REG_ECX, 28,  1},
 {"osxsave",  0x0001, NA, CPUID_REG_ECX, 27,  1},
 {"xsave",0x0001, NA, CPUID_REG_ECX, 26,  1},
 {"aes",  0x0001, NA, CPUID_REG_ECX, 25,  1},
+{"tsc-deadline", 0x0001, NA, CPUID_REG_ECX, 24,  1},
 {"popcnt",   0x0001, NA, CPUID_REG_ECX, 23,  1},
 {"movbe",0x0001, NA, CPUID_REG_ECX, 22,  1},
 {"x2apic",   0x0001, NA, CPUID_REG_ECX, 21,  1},
@@ -114,9 +116,11 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"sse4.1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
 {"sse4_1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
 {"dca",  0x0001, NA, CPUID_REG_ECX, 18,  1},
+{"pcid", 0x0001, NA, CPUID_REG_ECX, 17,  1},
 {"pdcm", 0x0001, NA, CPUID_REG_ECX, 15,  1},
 {"xtpr", 0x0001, NA, CPUID_REG_ECX, 14,  1},
 {"cmpxchg16",0x0001, NA, CPUID_REG_ECX, 13,  1},
+{"fma",  0x0001, NA, CPUID_REG_ECX, 12,  1},
 {"cntxid",   0x0001, NA, CPUID_REG_ECX, 10,  1},
 {"ssse3",0x0001, NA, CPUID_REG_ECX,  9,  1},
 {"tm2",  0x0001, NA, CPUID_REG_ECX,  8,  1},
@@ -158,6 +162,41 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"de",   0x0001, NA, CPUID_REG_EDX,  2,  1},
 {"vme",  0x0001, NA, CPUID_REG_EDX,  1,  1},
 {"fpu",  0x0001, NA, CPUID_REG_EDX,  0,  1},
+{"arat", 0x0006, NA, CPUID_REG_EAX,  2,  1},
+{"avx512vl", 0x0007,  0, CPUID_REG_EBX, 31,  1},
+{"avx512bw",

[Xen-devel] [PATCH v3 3/4] libxl: make cpuid_flags array static const

[Marek Marczykowski-Górecki]

To have it in .rodata, instead of reconstructing each time on stack.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_cpuid.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 98c7c54..3726aa4 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -89,7 +89,7 @@ static libxl_cpuid_policy_list 
cpuid_find_match(libxl_cpuid_policy_list *list,
 int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str)
 {
 #define NA XEN_CPUID_INPUT_UNUSED
-struct cpuid_flags cpuid_flags[] = {
+static const struct cpuid_flags cpuid_flags[] = {
 {"maxleaf",  0x, NA, CPUID_REG_EAX,  0, 32},
   /* the following two entries are subject to tweaking later in the code */
 {"family",   0x0001, NA, CPUID_REG_EAX,  8,  8},
@@ -243,7 +243,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 #undef NA
 char *sep, *val, *endptr;
 int i;
-struct cpuid_flags *flag;
+const struct cpuid_flags *flag;
 struct libxl__cpuid_policy *entry;
 unsigned long num;
 char flags[33], *resstr;
-- 
git-series 0.9.1

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v3 4/4] libxl: reformat cpuid_flags

[Marek Marczykowski-Górecki]

Reverse sorting order, add blank lines at register change.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_cpuid.c | 260 ---
 1 file changed, 137 insertions(+), 123 deletions(-)

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 3726aa4..e692b61 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -92,143 +92,156 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 static const struct cpuid_flags cpuid_flags[] = {
 {"maxleaf",  0x, NA, CPUID_REG_EAX,  0, 32},
   /* the following two entries are subject to tweaking later in the code */
-{"family",   0x0001, NA, CPUID_REG_EAX,  8,  8},
-{"model",0x0001, NA, CPUID_REG_EAX,  4,  8},
 {"stepping", 0x0001, NA, CPUID_REG_EAX,  0,  4},
-{"localapicid",  0x0001, NA, CPUID_REG_EBX, 24,  8},
-{"proccount",0x0001, NA, CPUID_REG_EBX, 16,  8},
-{"clflush",  0x0001, NA, CPUID_REG_EBX,  8,  8},
+{"model",0x0001, NA, CPUID_REG_EAX,  4,  8},
+{"family",   0x0001, NA, CPUID_REG_EAX,  8,  8},
+
 {"brandid",  0x0001, NA, CPUID_REG_EBX,  0,  8},
-{"hypervisor",   0x0001, NA, CPUID_REG_ECX, 31,  1},
-{"rdrand",   0x0001, NA, CPUID_REG_ECX, 30,  1},
-{"f16c", 0x0001, NA, CPUID_REG_ECX, 29,  1},
-{"avx",  0x0001, NA, CPUID_REG_ECX, 28,  1},
-{"osxsave",  0x0001, NA, CPUID_REG_ECX, 27,  1},
-{"xsave",0x0001, NA, CPUID_REG_ECX, 26,  1},
-{"aes",  0x0001, NA, CPUID_REG_ECX, 25,  1},
-{"tsc-deadline", 0x0001, NA, CPUID_REG_ECX, 24,  1},
-{"popcnt",   0x0001, NA, CPUID_REG_ECX, 23,  1},
-{"movbe",0x0001, NA, CPUID_REG_ECX, 22,  1},
-{"x2apic",   0x0001, NA, CPUID_REG_ECX, 21,  1},
+{"clflush",  0x0001, NA, CPUID_REG_EBX,  8,  8},
+{"proccount",0x0001, NA, CPUID_REG_EBX, 16,  8},
+{"localapicid",  0x0001, NA, CPUID_REG_EBX, 24,  8},
+
+{"sse3", 0x0001, NA, CPUID_REG_ECX,  0,  1},
+{"pclmulqdq",0x0001, NA, CPUID_REG_ECX,  1,  1},
+{"dtes64",   0x0001, NA, CPUID_REG_ECX,  2,  1},
+{"monitor",  0x0001, NA, CPUID_REG_ECX,  3,  1},
+{"dscpl",0x0001, NA, CPUID_REG_ECX,  4,  1},
+{"vmx",  0x0001, NA, CPUID_REG_ECX,  5,  1},
+{"smx",  0x0001, NA, CPUID_REG_ECX,  6,  1},
+{"est",  0x0001, NA, CPUID_REG_ECX,  7,  1},
+{"tm2",  0x0001, NA, CPUID_REG_ECX,  8,  1},
+{"ssse3",0x0001, NA, CPUID_REG_ECX,  9,  1},
+{"cntxid",   0x0001, NA, CPUID_REG_ECX, 10,  1},
+{"fma",  0x0001, NA, CPUID_REG_ECX, 12,  1},
+{"cmpxchg16",0x0001, NA, CPUID_REG_ECX, 13,  1},
+{"xtpr", 0x0001, NA, CPUID_REG_ECX, 14,  1},
+{"pdcm", 0x0001, NA, CPUID_REG_ECX, 15,  1},
+{"pcid", 0x0001, NA, CPUID_REG_ECX, 17,  1},
+{"dca",  0x0001, NA, CPUID_REG_ECX, 18,  1},
 /* Linux uses sse4_{1,2}.  Keep sse4.{1,2} for compatibility */
-{"sse4.2",   0x0001, NA, CPUID_REG_ECX, 20,  1},
-{"sse4_2",   0x0001, NA, CPUID_REG_ECX, 20,  1},
-{"sse4.1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
 {"sse4_1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
-{"dca",  0x0001, NA, CPUID_REG_ECX, 18,  1},
-{"pcid", 0x0001, NA, CPUID_REG_ECX, 17,  1},
-{"pdcm", 0x0001, NA, CPUID_REG_ECX, 15,  1},
-{"xtpr", 0x0001, NA, CPUID_REG_ECX, 14,  1},
-{"cmpxchg16",0x0001, NA, CPUID_REG_ECX, 13,  1},
-{"fma",  0x0001, NA, CPUID_REG_ECX, 12,  1},
-{"cntxid",   0x0001, NA, CPUID_REG_ECX, 10,  1},
-{"ssse3",0x0001, NA, CPUID_REG_ECX,  9,  1},
-{"tm2",  0x0001, NA, CPUID_REG_ECX,  8,  1},
-{"est",  0x0001, NA, CPUID_REG_ECX,  7,  1},
-{"smx",  0x0001, NA,

[Xen-devel] [PATCH v3 0/4] libxl: cpuid bits

[Marek Marczykowski-Górecki]

This adds handling more cpuid bits by name. Mostly based on cpu_map.xml from
libvirt.

Changes since v2:
  - added "libxl: make cpuid_flags array static const"
  - added "libxl: reformat cpuid_flags"
  - added few bits to the second patch by Andrew's suggestion
  - reverted the first patch to v1 version

Marek Marczykowski-Górecki (4):
  libxl: add more cpuid flags handling
  libxl: fix osvm cpuid flag
  libxl: make cpuid_flags array static const
  libxl: reformat cpuid_flags

 docs/man/xl.cfg.pod.5.in  |  21 ++--
 tools/libxl/libxl_cpuid.c | 234 ---
 2 files changed, 157 insertions(+), 98 deletions(-)

base-commit: 695bb5f504ab48c1d546446f104c1b6c0ead126d
-- 
git-series 0.9.1

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 0/2] libxl: cpuid bits

[Marek Marczykowski-Górecki]

On Wed, Jun 28, 2017 at 06:55:58PM +0100, Wei Liu wrote:
> On Wed, Jun 28, 2017 at 12:10:20PM +0200, Marek Marczykowski-Górecki wrote:
> > This adds handling more cpuid bits by name. Mostly based on cpu_map.xml from
> > libvirt.
> > 
> > Marek Marczykowski-Górecki (2):
> >   libxl: add more cpuid flags handling
> >   libxl: drop osvw cpuid flag
> 
> Given that I prefer to have v1 of the second patch, please resubmit a
> final version when you collect all the acks. Thanks.

Should I read this as "Acked-by: Wei Liu <wei.l...@citrix.com>" on v1 of
the second patch?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 1/2] libxl: add more cpuid flags handling

[Marek Marczykowski-Górecki]

On Wed, Jun 28, 2017 at 07:16:18PM +0100, Andrew Cooper wrote:
> On 28/06/17 11:10, Marek Marczykowski-Górecki wrote:
> > This is result of parsing cpu_map.xml from libvirt.
> > The most important part is handling leaf 0x0007, but while at it add
> > other bits too.
> >
> > Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> 
> Xen's canonical reference is include/asm-x86/cpufeatures.h from which
> the hypervisor logic is derived.
> 
> > ---
> >  docs/man/xl.cfg.pod.5.in  | 20 
> >  tools/libxl/libxl_cpuid.c | 37 +
> >  2 files changed, 49 insertions(+), 8 deletions(-)
> >
> > Changes since v1:
> >  - set sub-leaf to 0 for leaf 7
> >
> > diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> > index 38084c7..51361c4 100644
> > --- a/docs/man/xl.cfg.pod.5.in
> > +++ b/docs/man/xl.cfg.pod.5.in
> > @@ -1464,14 +1464,18 @@ apicidsize brandid clflush family localapicid 
> > maxleaf maxhvleaf model nc
> >  proccount procpkg stepping
> >  
> >  List of keys taking a character:
> > -3dnow 3dnowext 3dnowprefetch abm acpi aes altmovcr8 apic avx clfsh cmov
> > -cmplegacy cmpxchg16 cmpxchg8 cntxid dca de ds dscpl dtes64 est extapic f16c
> > -ffxsr fma4 fpu fxsr htt hypervisor ia64 ibs lahfsahf lm lwp mca mce 
> > misalignsse
> > -mmx mmxext monitor movbe msr mtrr nodeid nx osvw osxsave pae page1gb pat 
> > pbe
> > -pclmulqdq pdcm pge popcnt pse pse36 psn rdtscp skinit smx ss sse sse2 sse3
> > -sse4_1 sse4_2 sse4a ssse3 svm svm_decode svm_lbrv svm_npt svm_nrips
> > -svm_pausefilt svm_tscrate svm_vmcbclean syscall sysenter tbm tm tm2 
> > topoext tsc
> > -vme vmx wdt x2apic xop xsave xtpr
> > +3dnow 3dnowext 3dnowprefetch abm acpi adx aes altmovcr8 apic arat avx avx2
> > +avx512-4fmaps avx512-4vnniw avx512bw avx512cd avx512dq avx512er avx512f
> > +avx512ifma avx512pf avx512vbmi avx512vl bmi1 bmi2 clflushopt clfsh cmov
> > +cmplegacy cmpxchg16 cmpxchg8 cmt cntxid dca de ds dscpl dtes64 erms est 
> > extapic
> > +f16c ffxsr fma fma4 fpu fsgsbase fxsr hle htt hypervisor ia64 ibs invpcid
> > +invtsc lahfsahf lm lwp mca mce misalignsse mmx mmxext monitor movbe mpx msr
> > +mtrr nodeid nx ospke osvw osxsave pae page1gb pat pbe pcid pclmulqdq pdcm
> > +perfctr_core perfctr_nb pge pku popcnt pse pse36 psn rdrand rdseed rdtscp 
> > rtm
> > +skinit smap smep smx ss sse sse2 sse3 sse4_1 sse4_2 sse4a ssse3 svm 
> > svm_decode
> > +svm_lbrv svm_npt svm_nrips svm_pausefilt svm_tscrate svm_vmcbclean syscall
> > +sysenter tbm tm tm2 topoext tsc tsc-deadline tsc_adjust vme vmx wdt x2apic 
> > xop
> > +xsave xtpr
> >  
> >  The xend syntax is a list of values in the form of
> >  'leafnum:register=bitstring,register=bitstring'
> > diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
> > index 24591e2..8bcdb29 100644
> > --- a/tools/libxl/libxl_cpuid.c
> > +++ b/tools/libxl/libxl_cpuid.c
> > @@ -100,11 +100,13 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
> > *cpuid, const char* str)
> 
> As some general points, it would be helpful to add newlines for clarity
> between the CPUID_REG_* changes.  It is weird being sorted by bit index
> in descending order, although perhaps just me.

It is weird for me too, but I didn't wanted to reformat the whole list.
If there is no real reason for this order, I can change that too.

> The structure really wants to be static const (so living in .rodata). 
> At the moment, it is being reconstructed on the stack by prologue code
> every time the function is called.

Good idea.

> >  {"clflush",  0x0001, NA, CPUID_REG_EBX,  8,  8},
> >  {"brandid",  0x0001, NA, CPUID_REG_EBX,  0,  8},
> >  {"hypervisor",   0x0001, NA, CPUID_REG_ECX, 31,  1},
> > +{"rdrand",   0x0001, NA, CPUID_REG_ECX, 30,  1},
> >  {"f16c", 0x0001, NA, CPUID_REG_ECX, 29,  1},
> >  {"avx",  0x0001, NA, CPUID_REG_ECX, 28,  1},
> >  {"osxsave",  0x0001, NA, CPUID_REG_ECX, 27,  1},
> >  {"xsave",0x0001, NA, CPUID_REG_ECX, 26,  1},
> >  {"aes",  0x0001, NA, CPUID_REG_ECX, 25,  1},
> > +{"tsc-deadline", 0x0001, NA, CPUID_REG_ECX, 24,  1},
> >  {"popcnt",   0x0001, NA, CPUID_REG_ECX, 23,  1},
> >  {"movbe",0x0001, NA, CPUID_REG_ECX, 22,  1},
> >

Re: [Xen-devel] [PATCH 2/2] libxl: fix osvm cpuid flag

[Marek Marczykowski-Górecki]

On Wed, Jun 28, 2017 at 11:16:33AM +0100, Andrew Cooper wrote:
> On 28/06/17 07:09, Jan Beulich wrote:
> >>>> Marek Marczykowski-Górecki <marma...@invisiblethingslab.com> 06/28/17 
> >>>> 3:09 AM >>>
> >> It's bit 9 not 10 (which is ibs).
> > Indeed, but shouldn't it rather be removed? We don't expose it from the
> > hypervisor at all anymore:
> >
> > XEN_CPUFEATURE(OSVW,  3*32+ 9) /*   OS Visible Workaround */
> >
> > (note the absence of any marker character immediately inside the comment).
> 
> I don't believe we have ever actually offered OSVW to guests, despite
> the pretence of being able to.  ISTR it was always clobbered before
> being given to a guest.
> 
> Having said that, we should be advertising OSVW.  It's entire purpose is
> to tell the OS that there is something it can do to manually work round
> a specific erratum.  OTOH, making this migrate safe is liable to be very
> complicated...

I don't have opinion on either approach here, but the current state is
clearly wrong. You've got two versions of the patch, choose one ;)

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 1/2] libxl: add more cpuid flags handling

[Marek Marczykowski-Górecki]

This is result of parsing cpu_map.xml from libvirt.
The most important part is handling leaf 0x0007, but while at it add
other bits too.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 docs/man/xl.cfg.pod.5.in  | 20 
 tools/libxl/libxl_cpuid.c | 37 +
 2 files changed, 49 insertions(+), 8 deletions(-)

Changes since v1:
 - set sub-leaf to 0 for leaf 7

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 38084c7..51361c4 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1464,14 +1464,18 @@ apicidsize brandid clflush family localapicid maxleaf 
maxhvleaf model nc
 proccount procpkg stepping
 
 List of keys taking a character:
-3dnow 3dnowext 3dnowprefetch abm acpi aes altmovcr8 apic avx clfsh cmov
-cmplegacy cmpxchg16 cmpxchg8 cntxid dca de ds dscpl dtes64 est extapic f16c
-ffxsr fma4 fpu fxsr htt hypervisor ia64 ibs lahfsahf lm lwp mca mce misalignsse
-mmx mmxext monitor movbe msr mtrr nodeid nx osvw osxsave pae page1gb pat pbe
-pclmulqdq pdcm pge popcnt pse pse36 psn rdtscp skinit smx ss sse sse2 sse3
-sse4_1 sse4_2 sse4a ssse3 svm svm_decode svm_lbrv svm_npt svm_nrips
-svm_pausefilt svm_tscrate svm_vmcbclean syscall sysenter tbm tm tm2 topoext tsc
-vme vmx wdt x2apic xop xsave xtpr
+3dnow 3dnowext 3dnowprefetch abm acpi adx aes altmovcr8 apic arat avx avx2
+avx512-4fmaps avx512-4vnniw avx512bw avx512cd avx512dq avx512er avx512f
+avx512ifma avx512pf avx512vbmi avx512vl bmi1 bmi2 clflushopt clfsh cmov
+cmplegacy cmpxchg16 cmpxchg8 cmt cntxid dca de ds dscpl dtes64 erms est extapic
+f16c ffxsr fma fma4 fpu fsgsbase fxsr hle htt hypervisor ia64 ibs invpcid
+invtsc lahfsahf lm lwp mca mce misalignsse mmx mmxext monitor movbe mpx msr
+mtrr nodeid nx ospke osvw osxsave pae page1gb pat pbe pcid pclmulqdq pdcm
+perfctr_core perfctr_nb pge pku popcnt pse pse36 psn rdrand rdseed rdtscp rtm
+skinit smap smep smx ss sse sse2 sse3 sse4_1 sse4_2 sse4a ssse3 svm svm_decode
+svm_lbrv svm_npt svm_nrips svm_pausefilt svm_tscrate svm_vmcbclean syscall
+sysenter tbm tm tm2 topoext tsc tsc-deadline tsc_adjust vme vmx wdt x2apic xop
+xsave xtpr
 
 The xend syntax is a list of values in the form of
 'leafnum:register=bitstring,register=bitstring'
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 24591e2..8bcdb29 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -100,11 +100,13 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"clflush",  0x0001, NA, CPUID_REG_EBX,  8,  8},
 {"brandid",  0x0001, NA, CPUID_REG_EBX,  0,  8},
 {"hypervisor",   0x0001, NA, CPUID_REG_ECX, 31,  1},
+{"rdrand",   0x0001, NA, CPUID_REG_ECX, 30,  1},
 {"f16c", 0x0001, NA, CPUID_REG_ECX, 29,  1},
 {"avx",  0x0001, NA, CPUID_REG_ECX, 28,  1},
 {"osxsave",  0x0001, NA, CPUID_REG_ECX, 27,  1},
 {"xsave",0x0001, NA, CPUID_REG_ECX, 26,  1},
 {"aes",  0x0001, NA, CPUID_REG_ECX, 25,  1},
+{"tsc-deadline", 0x0001, NA, CPUID_REG_ECX, 24,  1},
 {"popcnt",   0x0001, NA, CPUID_REG_ECX, 23,  1},
 {"movbe",0x0001, NA, CPUID_REG_ECX, 22,  1},
 {"x2apic",   0x0001, NA, CPUID_REG_ECX, 21,  1},
@@ -114,9 +116,11 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"sse4.1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
 {"sse4_1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
 {"dca",  0x0001, NA, CPUID_REG_ECX, 18,  1},
+{"pcid", 0x0001, NA, CPUID_REG_ECX, 17,  1},
 {"pdcm", 0x0001, NA, CPUID_REG_ECX, 15,  1},
 {"xtpr", 0x0001, NA, CPUID_REG_ECX, 14,  1},
 {"cmpxchg16",0x0001, NA, CPUID_REG_ECX, 13,  1},
+{"fma",  0x0001, NA, CPUID_REG_ECX, 12,  1},
 {"cntxid",   0x0001, NA, CPUID_REG_ECX, 10,  1},
 {"ssse3",0x0001, NA, CPUID_REG_ECX,  9,  1},
 {"tm2",  0x0001, NA, CPUID_REG_ECX,  8,  1},
@@ -158,6 +162,38 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"de",   0x0001, NA, CPUID_REG_EDX,  2,  1},
 {"vme",  0x0001, NA, CPUID_REG_EDX,  1,  1},
 {"fpu",  0x0001, NA, CPUID_REG_EDX,  0,  1},
+{"arat", 0x0006, NA, CPUID_REG_EAX,  2,  1},
+{"avx512vl", 0x0007,  0, CPUID_REG_EBX, 31,  1},
+{"avx512bw&qu

[Xen-devel] [PATCH v2 0/2] libxl: cpuid bits

[Marek Marczykowski-Górecki]

This adds handling more cpuid bits by name. Mostly based on cpu_map.xml from
libvirt.

Marek Marczykowski-Górecki (2):
  libxl: add more cpuid flags handling
  libxl: drop osvw cpuid flag

 docs/man/xl.cfg.pod.5.in  | 20 
 tools/libxl/libxl_cpuid.c | 38 +-
 2 files changed, 49 insertions(+), 9 deletions(-)

-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/2] libxl: drop osvw cpuid flag

[Marek Marczykowski-Górecki]

It was wrong (should be bit 9), but instead of fixing, simply remove it
because it isn't exposed by hypervisor at all.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_cpuid.c | 1 -
 1 file changed, 1 deletion(-)

Changes since v1:
 - drop it instead of fixing it

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 8bcdb29..d95eeb7 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -203,7 +203,6 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"skinit",   0x8001, NA, CPUID_REG_ECX, 12,  1},
 {"xop",  0x8001, NA, CPUID_REG_ECX, 11,  1},
 {"ibs",  0x8001, NA, CPUID_REG_ECX, 10,  1},
-{"osvw", 0x8001, NA, CPUID_REG_ECX, 10,  1},
 {"3dnowprefetch",0x8001, NA, CPUID_REG_ECX,  8,  1},
 {"misalignsse",  0x8001, NA, CPUID_REG_ECX,  7,  1},
 {"sse4a",0x8001, NA, CPUID_REG_ECX,  6,  1},
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 0/2] libxl: cpuid bits

[Marek Marczykowski-Górecki]

This adds handling more cpuid bits by name. Mostly based on cpu_map.xml from
libvirt.
There are also some differences - looks like different naming in Intel and AMD
documentation (and sometimes also Linux). For example "pni" vs "sse3". Is it ok
to add those alternative names too? There is already such case for "sse4.1" / 
"sse4_1".

Marek Marczykowski-Górecki (2):
  libxl: add more cpuid flags handling
  libxl: fix osvm cpuid flag

 docs/man/xl.cfg.pod.5.in  | 20 
 tools/libxl/libxl_cpuid.c | 39 ++-
 2 files changed, 50 insertions(+), 9 deletions(-)

-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/2] libxl: add more cpuid flags handling

[Marek Marczykowski-Górecki]

This is result of parsing cpu_map.xml from libvirt.
The most important part is handling leaf 0x0007, but while at it add
other bits too.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 docs/man/xl.cfg.pod.5.in  | 20 
 tools/libxl/libxl_cpuid.c | 37 +
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
index 38084c7..51361c4 100644
--- a/docs/man/xl.cfg.pod.5.in
+++ b/docs/man/xl.cfg.pod.5.in
@@ -1464,14 +1464,18 @@ apicidsize brandid clflush family localapicid maxleaf 
maxhvleaf model nc
 proccount procpkg stepping
 
 List of keys taking a character:
-3dnow 3dnowext 3dnowprefetch abm acpi aes altmovcr8 apic avx clfsh cmov
-cmplegacy cmpxchg16 cmpxchg8 cntxid dca de ds dscpl dtes64 est extapic f16c
-ffxsr fma4 fpu fxsr htt hypervisor ia64 ibs lahfsahf lm lwp mca mce misalignsse
-mmx mmxext monitor movbe msr mtrr nodeid nx osvw osxsave pae page1gb pat pbe
-pclmulqdq pdcm pge popcnt pse pse36 psn rdtscp skinit smx ss sse sse2 sse3
-sse4_1 sse4_2 sse4a ssse3 svm svm_decode svm_lbrv svm_npt svm_nrips
-svm_pausefilt svm_tscrate svm_vmcbclean syscall sysenter tbm tm tm2 topoext tsc
-vme vmx wdt x2apic xop xsave xtpr
+3dnow 3dnowext 3dnowprefetch abm acpi adx aes altmovcr8 apic arat avx avx2
+avx512-4fmaps avx512-4vnniw avx512bw avx512cd avx512dq avx512er avx512f
+avx512ifma avx512pf avx512vbmi avx512vl bmi1 bmi2 clflushopt clfsh cmov
+cmplegacy cmpxchg16 cmpxchg8 cmt cntxid dca de ds dscpl dtes64 erms est extapic
+f16c ffxsr fma fma4 fpu fsgsbase fxsr hle htt hypervisor ia64 ibs invpcid
+invtsc lahfsahf lm lwp mca mce misalignsse mmx mmxext monitor movbe mpx msr
+mtrr nodeid nx ospke osvw osxsave pae page1gb pat pbe pcid pclmulqdq pdcm
+perfctr_core perfctr_nb pge pku popcnt pse pse36 psn rdrand rdseed rdtscp rtm
+skinit smap smep smx ss sse sse2 sse3 sse4_1 sse4_2 sse4a ssse3 svm svm_decode
+svm_lbrv svm_npt svm_nrips svm_pausefilt svm_tscrate svm_vmcbclean syscall
+sysenter tbm tm tm2 topoext tsc tsc-deadline tsc_adjust vme vmx wdt x2apic xop
+xsave xtpr
 
 The xend syntax is a list of values in the form of
 'leafnum:register=bitstring,register=bitstring'
diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 24591e2..1cf7973 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -100,11 +100,13 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"clflush",  0x0001, NA, CPUID_REG_EBX,  8,  8},
 {"brandid",  0x0001, NA, CPUID_REG_EBX,  0,  8},
 {"hypervisor",   0x0001, NA, CPUID_REG_ECX, 31,  1},
+{"rdrand",   0x0001, NA, CPUID_REG_ECX, 30,  1},
 {"f16c", 0x0001, NA, CPUID_REG_ECX, 29,  1},
 {"avx",  0x0001, NA, CPUID_REG_ECX, 28,  1},
 {"osxsave",  0x0001, NA, CPUID_REG_ECX, 27,  1},
 {"xsave",0x0001, NA, CPUID_REG_ECX, 26,  1},
 {"aes",  0x0001, NA, CPUID_REG_ECX, 25,  1},
+{"tsc-deadline", 0x0001, NA, CPUID_REG_ECX, 24,  1},
 {"popcnt",   0x0001, NA, CPUID_REG_ECX, 23,  1},
 {"movbe",0x0001, NA, CPUID_REG_ECX, 22,  1},
 {"x2apic",   0x0001, NA, CPUID_REG_ECX, 21,  1},
@@ -114,9 +116,11 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"sse4.1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
 {"sse4_1",   0x0001, NA, CPUID_REG_ECX, 19,  1},
 {"dca",  0x0001, NA, CPUID_REG_ECX, 18,  1},
+{"pcid", 0x0001, NA, CPUID_REG_ECX, 17,  1},
 {"pdcm", 0x0001, NA, CPUID_REG_ECX, 15,  1},
 {"xtpr", 0x0001, NA, CPUID_REG_ECX, 14,  1},
 {"cmpxchg16",0x0001, NA, CPUID_REG_ECX, 13,  1},
+{"fma",  0x0001, NA, CPUID_REG_ECX, 12,  1},
 {"cntxid",   0x0001, NA, CPUID_REG_ECX, 10,  1},
 {"ssse3",0x0001, NA, CPUID_REG_ECX,  9,  1},
 {"tm2",  0x0001, NA, CPUID_REG_ECX,  8,  1},
@@ -158,6 +162,38 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"de",   0x0001, NA, CPUID_REG_EDX,  2,  1},
 {"vme",  0x0001, NA, CPUID_REG_EDX,  1,  1},
 {"fpu",  0x0001, NA, CPUID_REG_EDX,  0,  1},
+{"arat", 0x0006, NA, CPUID_REG_EAX,  2,  1},
+{"avx512vl", 0x0007, NA, CPUID_REG_EBX, 31,  1},
+{"avx512bw", 0x0007, NA, CPUID_REG_EBX, 30,  1}

[Xen-devel] [PATCH 2/2] libxl: fix osvm cpuid flag

[Marek Marczykowski-Górecki]

It's bit 9 not 10 (which is ibs).

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index 1cf7973..a4a69af 100644
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -203,7 +203,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 {"skinit",   0x8001, NA, CPUID_REG_ECX, 12,  1},
 {"xop",  0x8001, NA, CPUID_REG_ECX, 11,  1},
 {"ibs",  0x8001, NA, CPUID_REG_ECX, 10,  1},
-{"osvw", 0x8001, NA, CPUID_REG_ECX, 10,  1},
+{"osvw", 0x8001, NA, CPUID_REG_ECX,  9,  1},
 {"3dnowprefetch",0x8001, NA, CPUID_REG_ECX,  8,  1},
 {"misalignsse",  0x8001, NA, CPUID_REG_ECX,  7,  1},
 {"sse4a",0x8001, NA, CPUID_REG_ECX,  6,  1},
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] Building Xen 4.8 with GCC 7

[Marek Marczykowski-Górecki]

 reference to 
`unpack3_TPM_PCR_COMPOSITE'
gdtoa-hexnan.c:(.text+0xc081): undefined reference to `unpack_ALLOC'
gdtoa-hexnan.c:(.text+0xc0da): undefined reference to `unpack3_TPM_AUTH_SESSION'
/home/user/rpmbuild/BUILD/xen-4.8.1/stubdom/mini-os-x86_64-vtpmmgr/mini-os.o: 
In function `unpack_TPMU_PUBLIC_ID':
/home/user/rpmbuild/BUILD/xen-4.8.1/stubdom/vtpmmgr/tpm2_marshal.h:577: 
undefined reference to `unpack_TPMS_ECC_POINT'
/home/user/rpmbuild/BUILD/xen-4.8.1/stubdom/mini-os-x86_64-vtpmmgr/mini-os.o: 
In function `TPM2_CreatePrimary':
gdtoa-hexnan.c:(.text+0xdadc): undefined reference to `unpack_TPMS_ECC_POINT'
make[2]: *** [Makefile:173: 
/home/user/rpmbuild/BUILD/xen-4.8.1/stubdom/mini-os-x86_64-vtpmmgr/mini-os] 
Error 1
make[2]: Leaving directory '/home/user/rpmbuild/BUILD/xen-4.8.1/extras/mini-os'
make[1]: *** [Makefile:576: vtpmmgr-stubdom] Error 2
make[1]: Leaving directory '/home/user/rpmbuild/BUILD/xen-4.8.1/stubdom'
make: *** [Makefile:105: install-stubdom] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.uyhPyK (%build)


-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/3] stubdom: fix vtpm compilation on GCC-7

[Marek Marczykowski-Górecki]

GCC-7 have -Wimplicit-fallthrough enabled with -Wextra. Add appropriate
comment which both mute the warning and improve readibility.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 stubdom/Makefile|  1 +
 stubdom/vtpm-implicit-fallthrough.patch | 10 ++
 2 files changed, 11 insertions(+)
 create mode 100644 stubdom/vtpm-implicit-fallthrough.patch

diff --git a/stubdom/Makefile b/stubdom/Makefile
index db01827..5055e31 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -228,6 +228,7 @@ tpm_emulator-$(XEN_TARGET_ARCH): 
tpm_emulator-$(TPMEMU_VERSION).tar.gz
patch -d $@ -p1 < vtpm-deepquote.patch
patch -d $@ -p1 < vtpm-deepquote-anyloc.patch
patch -d $@ -p1 < vtpm-cmake-Wextra.patch
+   patch -d $@ -p1 < vtpm-implicit-fallthrough.patch
mkdir $@/build
cd $@/build; CC=${CC} $(CMAKE) .. -DCMAKE_C_FLAGS:STRING="-std=c99 
-DTPM_NO_EXTERN $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) 
-Wno-declaration-after-statement"
touch $@
diff --git a/stubdom/vtpm-implicit-fallthrough.patch 
b/stubdom/vtpm-implicit-fallthrough.patch
new file mode 100644
index 000..db97be5
--- /dev/null
+++ b/stubdom/vtpm-implicit-fallthrough.patch
@@ -0,0 +1,10 @@
+--- tpm_emulator-x86_64/tpm/tpm_cmd_handler.c.orig 2017-04-27 
13:37:14.40800 +0200
 tpm_emulator-x86_64/tpm/tpm_cmd_handler.c  2017-04-27 13:39:53.58500 
+0200
+@@ -3397,6 +3397,7 @@
+   sizeof(rsp->auth2->nonceOdd.nonce));
+   tpm_hmac_update(, (BYTE*)>auth2->continueAuthSession, 1);
+   tpm_hmac_final(, rsp->auth2->auth);
++  /* fall-thru */
+ case TPM_TAG_RSP_AUTH1_COMMAND:
+   tpm_hmac_init(, rsp->auth1->secret, sizeof(rsp->auth1->secret));
+   tpm_hmac_update(, rsp->auth1->digest, sizeof(rsp->auth1->digest));
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 3/3] Disable -Wint-in-bool-context

[Marek Marczykowski-Górecki]

GCC 7 is too sensitive here.
See https://bugs.debian.org/853710

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 Config.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Config.mk b/Config.mk
index 1ddcd58..7af5af9 100644
--- a/Config.mk
+++ b/Config.mk
@@ -216,6 +216,7 @@ $(call 
cc-option-add,HOSTCFLAGS,HOSTCC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wdeclaration-after-statement)
 $(call cc-option-add,CFLAGS,CC,-Wno-unused-but-set-variable)
 $(call cc-option-add,CFLAGS,CC,-Wno-unused-local-typedefs)
+$(call cc-option-add,CFLAGS,CC,-Wno-int-in-bool-context)
 
 LDFLAGS += $(foreach i, $(EXTRA_LIB), -L$(i)) 
 CFLAGS += $(foreach i, $(EXTRA_INCLUDES), -I$(i))
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 0/3] Compilation fixes for GCC 7

[Marek Marczykowski-Górecki]

Some compilation fixes for GCC 7, done in Fedora 26 environment.

This isn't complete yet - some compilation issues are still there (more on it
in separate thread), but those patches do solve some problems.

Additionally Xen 4.6 - 4.8 (haven't checked others) require those patches to be
backported:

  796dea3 tools: include sys/sysmacros.h on Linux
  f49fa65 tools:misc:xenlockprof: fix possible format string overflow

Marek Marczykowski-Górecki (3):
  stubdom: fix vtpm compilation on GCC-7
  tools/xenpmd: fix potential string truncation
  Disable -Wint-in-bool-context

 Config.mk   |  1 +
 stubdom/Makefile|  1 +
 stubdom/vtpm-implicit-fallthrough.patch | 10 ++
 tools/xenpmd/xenpmd.c   |  8 
 4 files changed, 16 insertions(+), 4 deletions(-)
 create mode 100644 stubdom/vtpm-implicit-fallthrough.patch

-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/3] tools/xenpmd: fix potential string truncation

[Marek Marczykowski-Górecki]

GCC 7 warns (and thanks to -Werror, fails) about potential string
truncation by snprintf in get_next_battery_file. Since the code already
check if string is no longer than 4 chars, tell gcc about it.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/xenpmd/xenpmd.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/xenpmd/xenpmd.c b/tools/xenpmd/xenpmd.c
index b3a3106..ed6635b 100644
--- a/tools/xenpmd/xenpmd.c
+++ b/tools/xenpmd/xenpmd.c
@@ -87,12 +87,12 @@ static struct xs_handle *xs;
 
 #ifdef RUN_IN_SIMULATE_MODE
 #define BATTERY_DIR_PATH "/tmp/battery"
-#define BATTERY_INFO_FILE_PATH "/tmp/battery/%s/info" 
-#define BATTERY_STATE_FILE_PATH "/tmp/battery/%s/state"
+#define BATTERY_INFO_FILE_PATH "/tmp/battery/%.4s/info"
+#define BATTERY_STATE_FILE_PATH "/tmp/battery/%.4s/state"
 #else
 #define BATTERY_DIR_PATH "/proc/acpi/battery"
-#define BATTERY_INFO_FILE_PATH "/proc/acpi/battery/%s/info"
-#define BATTERY_STATE_FILE_PATH "/proc/acpi/battery/%s/state"
+#define BATTERY_INFO_FILE_PATH "/proc/acpi/battery/%.4s/info"
+#define BATTERY_STATE_FILE_PATH "/proc/acpi/battery/%.4s/state"
 #endif
 
 FILE *get_next_battery_file(DIR *battery_dir, 
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/xen: allow userspace access during hypercalls

['Marek Marczykowski-Górecki']

On Mon, Jun 26, 2017 at 01:09:58PM +, Paul Durrant wrote:
> > -Original Message-
> > From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of
> > Marek Marczykowski-Górecki
> > Sent: 26 June 2017 13:45
> > To: Juergen Groß <jgr...@suse.com>
> > Cc: Andrew Cooper <andrew.coop...@citrix.com>; x...@kernel.org; linux-
> > ker...@vger.kernel.org; sta...@vger.kernel.org; xen-
> > de...@lists.xenproject.org; Boris Ostrovsky <boris.ostrov...@oracle.com>
> > Subject: Re: [Xen-devel] [PATCH] x86/xen: allow userspace access during
> > hypercalls
> > 
> > On Mon, Jun 26, 2017 at 02:05:48PM +0200, Juergen Groß wrote:
> > > On 06/23/2017 02:47 PM, Marek Marczykowski-Górecki wrote:
> > > > Userspace application can do a hypercall through /dev/xen/privcmd, and
> > > > some for some hypercalls argument is a pointers to user-provided
> > > > structure. When SMAP is supported and enabled, hypervisor can't access.
> > > > So, lets allow it.
> > >
> > > What about HYPERVISOR_dm_op?
> > 
> > Indeed, arguments copied to kernel space there are only addresses of
> > buffers. Will send v2 in a moment.
> > But I can't test it right now, as for my understanding this require
> > HVM/PVHv2 dom0 or stubdomain...
> > 
> 
> No, you don't need anything particularly special to use dm_op. Just 
> up-to-date xen, privcmd, and QEMU. QEMU should end up using dm_op by default 
> if all three are in place.

But the issue this patch fixes applies only to hypercalls issued from HVM.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] x86/xen: allow userspace access during hypercalls

[Marek Marczykowski-Górecki]

Userspace application can do a hypercall through /dev/xen/privcmd, and
some for some hypercalls argument is a pointers to user-provided
structure. When SMAP is supported and enabled, hypervisor can't access.
So, lets allow it.

The same applies to HYPERVISOR_dm_op, where additionally privcmd driver
carefully verify buffer addresses.

Cc: sta...@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 arch/x86/include/asm/xen/hypercall.h | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

Changes since v1:
 - add HYPERVISOR_dm_op

diff --git a/arch/x86/include/asm/xen/hypercall.h 
b/arch/x86/include/asm/xen/hypercall.h
index f6d20f6..32b74a8 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -43,6 +43,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -214,10 +215,12 @@ privcmd_call(unsigned call,
__HYPERCALL_DECLS;
__HYPERCALL_5ARG(a1, a2, a3, a4, a5);
 
+   stac();
asm volatile("call *%[call]"
 : __HYPERCALL_5PARAM
 : [call] "a" (_page[call])
 : __HYPERCALL_CLOBBER5);
+   clac();
 
return (long)__res;
 }
@@ -476,7 +479,11 @@ static inline int
 HYPERVISOR_dm_op(
domid_t dom, unsigned int nr_bufs, void *bufs)
 {
-   return _hypercall3(int, dm_op, dom, nr_bufs, bufs);
+   int ret;
+   stac();
+   ret = _hypercall3(int, dm_op, dom, nr_bufs, bufs);
+   clac();
+   return ret;
 }
 
 static inline void
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/xen: allow userspace access during hypercalls

[Marek Marczykowski-Górecki]

On Mon, Jun 26, 2017 at 02:05:48PM +0200, Juergen Groß wrote:
> On 06/23/2017 02:47 PM, Marek Marczykowski-Górecki wrote:
> > Userspace application can do a hypercall through /dev/xen/privcmd, and
> > some for some hypercalls argument is a pointers to user-provided
> > structure. When SMAP is supported and enabled, hypervisor can't access.
> > So, lets allow it.
> 
> What about HYPERVISOR_dm_op?

Indeed, arguments copied to kernel space there are only addresses of
buffers. Will send v2 in a moment.
But I can't test it right now, as for my understanding this require
HVM/PVHv2 dom0 or stubdomain...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] x86/xen: allow userspace access during hypercalls

[Marek Marczykowski-Górecki]

Userspace application can do a hypercall through /dev/xen/privcmd, and
some for some hypercalls argument is a pointers to user-provided
structure. When SMAP is supported and enabled, hypervisor can't access.
So, lets allow it.

Cc: sta...@vger.kernel.org
Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 arch/x86/include/asm/xen/hypercall.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/xen/hypercall.h 
b/arch/x86/include/asm/xen/hypercall.h
index f6d20f6..a1d2c5d 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -43,6 +43,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -214,10 +215,12 @@ privcmd_call(unsigned call,
__HYPERCALL_DECLS;
__HYPERCALL_5ARG(a1, a2, a3, a4, a5);
 
+   stac();
asm volatile("call *%[call]"
 : __HYPERCALL_5PARAM
 : [call] "a" (_page[call])
 : __HYPERCALL_CLOBBER5);
+   clac();
 
return (long)__res;
 }
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

[resurrecting old thread...]

On Mon, Jan 16, 2017 at 11:41:55PM +, Andrew Cooper wrote:
> On 16/01/2017 23:06, Marek Marczykowski-Górecki wrote:
> > On Mon, Jan 16, 2017 at 05:17:59AM -0700, Jan Beulich wrote:
> >> 2) When the guest issues stac()/clac(), it indicates to Xen _its own_
> >> intended view, without affecting Xen's. That is, as soon as hypervisor
> >> context is being entered again, SMAP protection would be in effect
> >> again (albeit as per point 1 guarding only against accessing PV guest
> >> mappings).
> >>
> >> So the driver adjustment suggested by Andrew has an effect on only
> >> page walks done by Xen during copy_{to,from}_guest(), but not on
> >> actual memory accesses.
> > Ok, so indeed the kernel patch makes the most sense here. Is the change
> > in this shape (if works - I'll test it shortly) good to include
> > upstream, or is it "ugly hack"?
> 
> If it works (which I suspect it will), then it will be the correct
> proper upstream fix, and will of course CC stable@.

Should I submit it?

> In the meantime until it percolates into downstream kernels, disabling
> SMAP for affected guests is probably the best stopgap solution.

How to disable SMAP for selected guests only?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough of USB controllers on Xen 4.8.1, Linux 4.9.29, stubdomain

[Marek Marczykowski-Górecki]

On Tue, Jun 06, 2017 at 04:55:20AM -0600, Jan Beulich wrote:
> >>> On 06.06.17 at 12:41, <marma...@invisiblethingslab.com> wrote:
> > [root@dom0 ~]# lspci -s 00:14.0 -vvv
> > 00:14.0 USB controller: Intel Corporation Wildcat Point-LP USB xHCI 
> > Controller (rev 03) (prog-if 30 [XHCI])
> > Subsystem: Intel Corporation Device 7270
> > Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> > Stepping- SERR- FastB2B- DisINTx+
> > Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
> > SERR-  > Latency: 0
> > Interrupt: pin A routed to IRQ 170
> > Region 0: Memory at b220 (64-bit, non-prefetchable) [size=64K]
> > Capabilities: [70] Power Management version 2
> > Flags: PMEClk- DSI- D1- D2- AuxCurrent=375mA 
> > PME(D0-,D1-,D2-,D3hot+,D3cold+)
> > Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
> > Capabilities: [80] MSI: Enable+ Count=1/8 Maskable- 64bit+
> > Address: fee00338  Data: 
> 
> So as I did expect the field accessed is the MSI capability structure.
> Such accesses shouldn't reach pciback, but instead be taken care
> of by qemu. I can't really comment on the qemu side though, but
> this at least makes me think the underlying cause of the problems
> you see with the two controllers is the same.
> 
> Is this a regression of some sort, i.e. did the same setup work in
> earlier Xen versions?

This was working with pure PV, but this is probably irrelevant here.

I can't test if switching only Xen version changes anything (that would
require a lot of recompiling), but very similar setup with mini-os based
stubdom on Xen 4.6.5 and Xen 4.8.1 also doesn't work. Even with a qemu
patch disabling MSI reporting (attached). In that case, I don't get any
message from qemu, but very similar messages from xhci driver.

BTW Other patches:
https://github.com/QubesOS/qubes-vmm-xen/tree/xen-4.8/patches.misc

I've also checked Xen 4.8.1 with qemu-upstream in dom0 (instead of
Linux-based stubdom) and it also doesn't work, but with slightly
different messages:

qemu:
[00:05.0] Write-back to unknown field 0xd8 (partially) inhibited 
(0x)
[00:05.0] If the device doesn't work, try enabling permissive mode
[00:05.0] (unsafe) and if it helps report the problem to xen-devel
[00:06.0] Write-back to unknown field 0x6c (partially) inhibited 
(0x)
[00:06.0] If the device doesn't work, try enabling permissive mode
[00:06.0] (unsafe) and if it helps report the problem to xen-devel

Linux in domU:

[   51.679188] xhci_hcd :00:05.0: Error while assigning device slot ID
[   51.679264] xhci_hcd :00:05.0: Max number of devices this xHCI host 
supports is 32.
    [   51.679359] usb usb1-port2: couldn't allocate usb_device

(and nothing about ehci, no devices connected to it is visible)


-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
MiniOS + QEMU do not appear to work with either MSI or MSI-X.  Some guests
or device drivers do not gracefully handle being told a PCI device has
MSI/MSI-X and then they don't actually get it.  Disable the MSI and MSI-X
capability reporting in PCI config space, making only INTX legacy interrupts
available.

Signed-off-by: Eric Shelton <eshel...@pobox.com>

--- a/tools/qemu-xen-traditional/hw/pass-through.c  2016-10-31 
12:48:42.924026468 -0400
+++ b/tools/qemu-xen-traditional/hw/pass-through.c  2016-10-31 
12:50:52.953026468 -0400
@@ -874,6 +874,10 @@
 .grp_size   = 0x04,
 .size_init  = pt_reg_grp_size_init,
 },
+#ifndef CONFIG_STUBDOM
+/* At present stubdom doesn't support MSI for passthrough, so let's not
+ * expose MSI capability to stubdom HVM guest for now.
+ */
 #ifndef __ia64__
 /* At present IA64 Xen doesn't support MSI for passthrough, so let's not
  * expose MSI capability to IA64 HVM guest for now.
@@ -886,7 +890,8 @@
 .size_init  = pt_msi_size_init,
 .emu_reg_tbl= pt_emu_reg_msi_tbl,
 },
-#endif
+#endif /* __ia64__ */
+#endif /* !CONFIG_STUBDOM */
 /* PCI-X Capabilities List Item reg group */
 {
 .grp_id = PCI_CAP_ID_PCIX,
@@ -931,6 +936,10 @@
 .size_init  = pt_pcie_size_init,
 .emu_reg_tbl= pt_emu_reg_pcie_tbl,
 },
+#ifndef CONFIG_STUBDOM
+/* At present stubdom doesn't support MSI for passthrough, so let's not
+ * expose MSI-X capability to stubdom HVM guest for now.
+ */
 #ifndef __ia64__
 /* At present IA64 Xen doesn't support MSI for passthrough, so let's not
  * expose MSI-X capability to IA64 HVM guest for now.
@@ -943,7 +952,8 @@
 .size_init  = pt_msix_size_init,
 .emu_reg_tbl= pt_emu_reg_msix_tb

Re: [Xen-devel] PCI passthrough of USB controllers on Xen 4.8.1, Linux 4.9.29, stubdomain

[Marek Marczykowski-Górecki]

On Tue, Jun 06, 2017 at 02:37:01AM -0600, Jan Beulich wrote:
> >>> On 02.06.17 at 12:57,  wrote:
> > And in this case, dom0 also prints:
> > 
> > [   49.155606] pciback :00:14.0: Driver tried to write to a 
> > read-only configuration space field at offset 0x82, size 2. This may be 
> > harmless, but if you have problems with your device:
> >1) see permissive attribute in sysfs
> >2) report problems to the xen-devel mailing list along 
> > with details of your device obtained from lspci.
> > [   66.247644] pciback :00:14.0: cache line size of 64 is not 
> > supported
> > [   66.247646] xen_pciback: :00:14.0: cannot enable 
> > memory-write-invalidate (-22)
> > 
> > Enabling permissive mode doesn't change anything.
> 
> I doubt this - the first of the messages won't be logged in permissive
> mode.

Yes, of course. But the other lines and overall effect is the same.

> We'll also need to know what register there is at address 0x82
> (possibly visible from a sufficiently verbose lspci in Dom0).
> 
> As to the latter two - lspci output may also help understand
> what the issue with cache line size here is. A second source of
> information may be lspci output for the device with its normal
> driver loaded and attached in Dom0.

Below is lspci of those two devices, in dom0, with normal driver
attached. Would lspci from domU be useful too?

[root@dom0 ~]# lspci -s 00:1d.0 -vvv
00:1d.0 USB controller: Intel Corporation Wildcat Point-LP USB EHCI Controller 
(rev 03) (prog-if 20 [EHCI])
Subsystem: Intel Corporation Device 7270
Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
SERR- TAbort- 
SERR- 

signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] (pv)?grub and PVHv2

[Marek Marczykowski-Górecki]

On Mon, Jun 05, 2017 at 11:55:24AM +0100, George Dunlap wrote:
> On Fri, Jun 2, 2017 at 10:58 AM, Roger Pau Monné <roger@citrix.com> wrote:
> > On Fri, Jun 02, 2017 at 11:33:50AM +0200, Marek Marczykowski-Górecki wrote:
> >> Hi,
> >>
> >> Is there any method to boot PVHv2 domain using a kernel fetched from
> >> that domain's disk image, _without_ mounting it in dom0? Something like
> >> pvgrub was for PV.
> >
> > Hello,
> >
> > Anthony (Cced) is working on an OVMF port, so it can be used as
> > firmware for PVHv2 guests.
> 
> I think in theory it shouldn't be too hard to port the pvgrub2 code to
> boot into PVH, since it already boots in PV, right?
> 
> Is this something we should try to encourage, or do you think it would
> be better to route everyone through EFI?

For Qubes OS I think EFI is good enough here. Any system supporting
PVHv2 also support EFI (right?), so it shouldn't limit anything.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] (pv)?grub and PVHv2

[Marek Marczykowski-Górecki]

On Fri, Jun 02, 2017 at 12:16:06PM +0100, Anthony PERARD wrote:
> On Fri, Jun 02, 2017 at 10:58:54AM +0100, Roger Pau Monné wrote:
> > On Fri, Jun 02, 2017 at 11:33:50AM +0200, Marek Marczykowski-Górecki wrote:
> > > Hi,
> > > 
> > > Is there any method to boot PVHv2 domain using a kernel fetched from
> > > that domain's disk image, _without_ mounting it in dom0? Something like
> > > pvgrub was for PV.
> > 
> > Hello,
> > 
> > Anthony (Cced) is working on an OVMF port, so it can be used as
> > firmware for PVHv2 guests.
> > 
> > I cannot seem to be able to find the original cover-letter of that
> > patch series, this is the best I could find:
> > 
> > https://lists.01.org/pipermail/edk2-devel/2017-January/006148.html
> 
> Here for the cover-letter:
> https://lists.xenproject.org/archives/html/xen-devel/2016-12/msg00953.html

Thanks!

> But that a UEFI firmware, so I guess the guest would need UEFI support
> backed into the disk image.

That's totally ok. I assume it should point at linux.efi, not grub.efi,
right?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] PCI passthrough of USB controllers on Xen 4.8.1, Linux 4.9.29, stubdomain

[Marek Marczykowski-Górecki]

Hi,

I'm trying to passthrough USB controllers (details below) to a HVM domain
with qemu running in stubdomain.  There are two of them (EHCI and xHCI),
neither of them works. The stubdomain is non-standard Linux
based with Qemu 2.8.0. Both dom0 and guest are running Linux 4.9.29 very
close to vanilla.  I've started debugging with EHCI. On modprobe
ehci-pci I get:

[52453.633768] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
[52453.634496] ehci-pci: EHCI PCI platform driver
[52453.640088] ehci-pci :00:06.0: EHCI Host Controller
[52453.640466] ehci-pci :00:06.0: new USB bus registered, assigned bus 
number 5
[52462.453251] ehci-pci :00:06.0: can't setup: -110
[52462.453315] ehci-pci :00:06.0: USB bus 5 deregistered
[52462.458375] ehci-pci :00:06.0: init :00:06.0 fail, -110
[52462.458421] ehci-pci: probe of :00:06.0 failed with error -110

And at this time, stubdomain print this:

[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004
[00:06.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. 
@0x0004

repeated a lot more times, sometimes with slightly different addresses
and/or for write.

Just to be sure, I've checked with mini-os based stubdomain, but there I
don't even see the EHCI one (only xHCI, which doesn't work the same way
as on updated stubdomain).

Regarding xHCI, the domU messages are (this one is from older kernel,
but the same happens on 4.9 too):

[  347.822393] xhci_hcd :00:05.0: xHCI Host Controller
[  347.822764] xhci_hcd :00:05.0: new USB bus registered, assigned bus 
number 2
[  347.825391] xhci_hcd :00:05.0: hcc params 0x200077c1 hci version 
0x100 quirks 0x0004b810
[  347.832346] usb usb2: New USB device found, idVendor=1d6b, idProduct=0002
[  347.832482] usb usb2: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[  347.832643] usb usb2: Product: xHCI Host Controller
[  347.832779] usb usb2: Manufacturer: Linux 4.8.13-100.fc23.x86_64 xhci-hcd
[  347.832970] usb usb2: SerialNumber: :00:05.0
[  347.833867] hub 2-0:1.0: USB hub found
[  347.834089] hub 2-0:1.0: 11 ports detected
[  347.835127] xhci_hcd :00:05.0: xHCI Host Controller
[  347.836927] xhci_hcd :00:05.0: new USB bus registered, assigned bus 
number 3
[  347.837382] usb usb3: New USB device found, idVendor=1d6b, idProduct=0003
[  347.837582] usb usb3: New USB device strings: Mfr=3, Product=2, 
SerialNumber=1
[  347.837784] usb usb3: Product: xHCI Host Controller
[  347.837921] usb usb3: Manufacturer: Linux 4.8.13-100.fc23.x86_64 xhci-hcd
[  347.838159] usb usb3: SerialNumber: :00:05.0
[  347.838942] hub 3-0:1.0: USB hub found
[  347.839158] hub 3-0:1.0: 4 ports detected
[  363.487226] xhci_hcd :00:05.0: Error while assigning device slot ID
[  363.487368] xhci_hcd :00:05.0: Max number of devices this xHCI host 
supports is 32.
[  363.487508] usb usb2-port1: couldn't allocate usb_device
[  383.967268] xhci_hcd :00:05.0: Error while assigning device slot ID
[  383.967495] xhci_hcd :00:05.0: Max number of devices this xHCI host 
supports is 32.
[  383.967715] usb usb2-port2: couldn't allocate usb_device

And in this case, dom0 also prints:

[   49.155606] pciback :00:14.0: Driver tried to write to a read-only 
configuration space field at offset 0x82, size 2. This may be harmless, but if 
you have problems with your device:
   1) see permissive attribute in sysfs
   2) report problems to the xen-devel mailing list along with 
details of your device obtained from lspci.
[   66.247644] pciback :00:14.0: cache line size of 64 is not supported
[   66.247646] xen_pciback: :00:14.0: cannot enable 
memory-write-invalidate (-22)

Enabling permissive mode doesn't change anything.

Any idea what's wrong?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen

[Xen-devel] (pv)?grub and PVHv2

[Marek Marczykowski-Górecki]

Hi,

Is there any method to boot PVHv2 domain using a kernel fetched from
that domain's disk image, _without_ mounting it in dom0? Something like
pvgrub was for PV.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] MAINTAINERS: add Marek as maintainer of python bindings

[Marek Marczykowski-Górecki]

On Wed, Mar 01, 2017 at 12:32:26PM +, Wei Liu wrote:
> Marek has kindly agreed to step up and co-maintain the python bindings.
> 
> Signed-off-by: Wei Liu <wei.l...@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

> ---
> Cc: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> Cc: Ian Jackson <ian.jack...@eu.citrix.com>
> 
> $ get_maintainers.pl -f tools/python
> "Marek Marczykowski-Górecki" <marma...@invisiblethingslab.com>
> Ian Jackson <ian.jack...@eu.citrix.com>
> Wei Liu <wei.l...@citrix.com>
> xen-devel@lists.xen.org
> ---
>  MAINTAINERS | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4cfd7bcdf0..ff6e99f31b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -306,6 +306,11 @@ M:  Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
>  S:  Supported
>  F:  xen/include/public/io/
>  
> +PYTHON BINDINGS
> +M:   Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> +S:   Supported
> +F:   tools/python
> +
>  QEMU-DM
>  M:   Ian Jackson <ian.jack...@eu.citrix.com>
>  S:   Supported

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 8/8] python: handle long type in scripts

[Marek Marczykowski-Górecki]

In Python3 'long' type have been merged into 'int', '1L' syntax is no
longer valid. Assign 'int' type to a 'long' variable in python3, so
'long(1)' will give correct result in both python2 and python3.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/migration/libxc.py | 32 ++--
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/python/xen/migration/libxc.py 
b/tools/python/xen/migration/libxc.py
index 85a78f4..6fd3f6d 100644
--- a/tools/python/xen/migration/libxc.py
+++ b/tools/python/xen/migration/libxc.py
@@ -14,6 +14,10 @@ from struct import calcsize, unpack
 
 from xen.migration.verify import StreamError, RecordError, VerifyBase
 
+# In Python3 long type have been merged into int, 1L syntax is no longer valid
+if sys.version_info > (3,):
+long = int
+
 # Image Header
 IHDR_FORMAT = "!QIIHHI"
 
@@ -83,23 +87,23 @@ rec_type_to_str = {
 
 # page_data
 PAGE_DATA_FORMAT = "II"
-PAGE_DATA_PFN_MASK   = (1L << 52) - 1
-PAGE_DATA_PFN_RESZ_MASK  = ((1L << 60) - 1) & ~((1L << 52) - 1)
+PAGE_DATA_PFN_MASK   = (long(1) << 52) - 1
+PAGE_DATA_PFN_RESZ_MASK  = ((long(1) << 60) - 1) & ~((long(1) << 52) - 1)
 
 # flags from xen/public/domctl.h: XEN_DOMCTL_PFINFO_* shifted by 32 bits
 PAGE_DATA_TYPE_SHIFT = 60
-PAGE_DATA_TYPE_LTABTYPE_MASK = (0x7L << PAGE_DATA_TYPE_SHIFT)
-PAGE_DATA_TYPE_LTAB_MASK = (0xfL << PAGE_DATA_TYPE_SHIFT)
-PAGE_DATA_TYPE_LPINTAB   = (0x8L << PAGE_DATA_TYPE_SHIFT) # Pinned 
pagetable
-
-PAGE_DATA_TYPE_NOTAB = (0x0L << PAGE_DATA_TYPE_SHIFT) # Regular page
-PAGE_DATA_TYPE_L1TAB = (0x1L << PAGE_DATA_TYPE_SHIFT) # L1 pagetable
-PAGE_DATA_TYPE_L2TAB = (0x2L << PAGE_DATA_TYPE_SHIFT) # L2 pagetable
-PAGE_DATA_TYPE_L3TAB = (0x3L << PAGE_DATA_TYPE_SHIFT) # L3 pagetable
-PAGE_DATA_TYPE_L4TAB = (0x4L << PAGE_DATA_TYPE_SHIFT) # L4 pagetable
-PAGE_DATA_TYPE_BROKEN= (0xdL << PAGE_DATA_TYPE_SHIFT) # Broken
-PAGE_DATA_TYPE_XALLOC= (0xeL << PAGE_DATA_TYPE_SHIFT) # Allocate-only
-PAGE_DATA_TYPE_XTAB  = (0xfL << PAGE_DATA_TYPE_SHIFT) # Invalid
+PAGE_DATA_TYPE_LTABTYPE_MASK = (long(0x7) << PAGE_DATA_TYPE_SHIFT)
+PAGE_DATA_TYPE_LTAB_MASK = (long(0xf) << PAGE_DATA_TYPE_SHIFT)
+PAGE_DATA_TYPE_LPINTAB   = (long(0x8) << PAGE_DATA_TYPE_SHIFT) # Pinned 
pagetable
+
+PAGE_DATA_TYPE_NOTAB = (long(0x0) << PAGE_DATA_TYPE_SHIFT) # Regular 
page
+PAGE_DATA_TYPE_L1TAB = (long(0x1) << PAGE_DATA_TYPE_SHIFT) # L1 
pagetable
+PAGE_DATA_TYPE_L2TAB = (long(0x2) << PAGE_DATA_TYPE_SHIFT) # L2 
pagetable
+PAGE_DATA_TYPE_L3TAB = (long(0x3) << PAGE_DATA_TYPE_SHIFT) # L3 
pagetable
+PAGE_DATA_TYPE_L4TAB = (long(0x4) << PAGE_DATA_TYPE_SHIFT) # L4 
pagetable
+PAGE_DATA_TYPE_BROKEN= (long(0xd) << PAGE_DATA_TYPE_SHIFT) # Broken
+PAGE_DATA_TYPE_XALLOC= (long(0xe) << PAGE_DATA_TYPE_SHIFT) # 
Allocate-only
+PAGE_DATA_TYPE_XTAB  = (long(0xf) << PAGE_DATA_TYPE_SHIFT) # Invalid
 
 # x86_pv_info
 X86_PV_INFO_FORMAT= "BBHI"
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 0/8] Python 3 bindings

[Marek Marczykowski-Górecki]

This patch series add support for Python3 in tools/python. As most python
modules, it looks at $PYTHON variable to what python use, so to use this
PYTHON=python3 needs to be defined in a build environment. If both python2 and
python3 versions are needed, then python bindings needs to be built twice. In
most cases it will be packaging scripts responsibility.

Patches 1-4 are cleanups not really specific to python3, but making it easier.

Marek Marczykowski-Górecki (8):
  python: check return value of PyErr_NewException
  python: drop tp_getattr implementation
  python: use Py_TYPE instead of looking directly into PyObject_HEAD
  python: initialize specific fields of PyTypeObject
  python: use PyBytes/PyUnicode instead of PyString
  python: use PyLong_* for constructing 'int' type in Python3
  python: adjust module initalization for Python3
  python: handle long type in scripts

 tools/python/xen/lowlevel/xc/xc.c   | 148 +++-
 tools/python/xen/lowlevel/xs/xs.c   | 118 
 tools/python/xen/migration/libxc.py |  32 
 3 files changed, 165 insertions(+), 133 deletions(-)

-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 5/8] python: use PyBytes/PyUnicode instead of PyString

[Marek Marczykowski-Górecki]

In Python2 PyBytes is the same as PyString, but in Python3 PyString is
gone and 'str' is really PyUnicode in C-API.
When handling arbitrary data, use PyBytes - which is the right thing to
do in Python3, and pose no API change in Python2. When handling
xenstore paths and transaction ids, which have well defined format, use
PyUnicode - to ease API usage - no need to prefix all xenstore paths
with 'b' when migrating scripts to Python3.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c |  6 +++---
 tools/python/xen/lowlevel/xs/xs.c | 20 
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index bcbb7b0..ce87afb 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -678,7 +678,7 @@ static void pyxc_dom_extract_cpuid(PyObject *config,
 
 for ( i = 0; i < 4; i++ )
 if ( (obj = PyDict_GetItemString(config, regs_extract[i])) != NULL )
-regs[i] = PyString_AS_STRING(obj);
+regs[i] = PyBytes_AS_STRING(obj);
 }
 
 static PyObject *pyxc_create_cpuid_dict(char **regs)
@@ -693,7 +693,7 @@ static PyObject *pyxc_create_cpuid_dict(char **regs)
if ( regs[i] == NULL )
continue;
PyDict_SetItemString(dict, regs_extract[i],
-PyString_FromString(regs[i]));
+PyBytes_FromString(regs[i]));
free(regs[i]);
regs[i] = NULL;
}
@@ -940,7 +940,7 @@ static PyObject *pyxc_readconsolering(XcObject *self,
 str = ptr;
 }
 
-obj = PyString_FromStringAndSize(str, count);
+obj = PyBytes_FromStringAndSize(str, count);
 free(str);
 return obj;
 }
diff --git a/tools/python/xen/lowlevel/xs/xs.c 
b/tools/python/xen/lowlevel/xs/xs.c
index 66ab08d..c2b4d87 100644
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -103,7 +103,7 @@ static PyObject *xspy_read(XsHandle *self, PyObject *args)
 xsval = xs_read(xh, th, path, _n);
 Py_END_ALLOW_THREADS
 if (xsval) {
-PyObject *val = PyString_FromStringAndSize(xsval, xsval_n);
+PyObject *val = PyBytes_FromStringAndSize(xsval, xsval_n);
 free(xsval);
 return val;
 }
@@ -179,7 +179,11 @@ static PyObject *xspy_ls(XsHandle *self, PyObject *args)
 int i;
 PyObject *val = PyList_New(xsval_n);
 for (i = 0; i < xsval_n; i++)
-PyList_SetItem(val, i, PyString_FromString(xsval[i]));
+#if PY_MAJOR_VERSION >= 3
+PyList_SetItem(val, i, PyUnicode_FromString(xsval[i]));
+#else
+PyList_SetItem(val, i, PyBytes_FromString(xsval[i]));
+#endif
 free(xsval);
 return val;
 }
@@ -550,7 +554,11 @@ static PyObject *xspy_transaction_start(XsHandle *self)
 }
 
 snprintf(thstr, sizeof(thstr), "%lX", (unsigned long)th);
-return PyString_FromString(thstr);
+#if PY_MAJOR_VERSION >= 3
+return PyUnicode_FromString(thstr);
+#else
+return PyBytes_FromString(thstr);
+#endif
 }
 
 #define xspy_transaction_end_doc "\n"  \
@@ -773,7 +781,11 @@ static PyObject *xspy_get_domain_path(XsHandle *self, 
PyObject *args)
 Py_END_ALLOW_THREADS
 
 if (xsval) {
-PyObject *val = PyString_FromString(xsval);
+#if PY_MAJOR_VERSION >= 3
+PyObject *val = PyUnicode_FromString(xsval);
+#else
+PyObject *val = PyBytes_FromString(xsval);
+#endif
 free(xsval);
 return val;
 }
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/8] python: drop tp_getattr implementation

[Marek Marczykowski-Górecki]

tp_getattr method of type object is deprecated already in Python2 and
gone in Python3. Default implementation does the same as this custom one.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 7 +--
 tools/python/xen/lowlevel/xs/xs.c | 7 +--
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index 7fbead5..109ef57 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -2640,11 +2640,6 @@ static PyMethodDef pyxc_methods[] = {
 };
 
 
-static PyObject *PyXc_getattr(PyObject *obj, char *name)
-{
-return Py_FindMethod(pyxc_methods, obj, name);
-}
-
 static PyObject *PyXc_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
 XcObject *self = (XcObject *)type->tp_alloc(type, 0);
@@ -2686,7 +2681,7 @@ static PyTypeObject PyXcType = {
 0,
 (destructor)PyXc_dealloc, /* tp_dealloc*/
 NULL, /* tp_print  */
-PyXc_getattr, /* tp_getattr*/
+NULL, /* tp_getattr*/
 NULL, /* tp_setattr*/
 NULL, /* tp_compare*/
 NULL, /* tp_repr   */
diff --git a/tools/python/xen/lowlevel/xs/xs.c 
b/tools/python/xen/lowlevel/xs/xs.c
index 5772f4b..e9eef73 100644
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -870,11 +870,6 @@ static PyMethodDef xshandle_methods[] = {
 { NULL /* Sentinel. */ },
 };
 
-static PyObject *xshandle_getattr(PyObject *self, char *name)
-{
-return Py_FindMethod(xshandle_methods, self, name);
-}
-
 static PyObject *
 xshandle_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
@@ -938,7 +933,7 @@ static PyTypeObject xshandle_type = {
 0,
 (destructor)xshandle_dealloc, /* tp_dealloc*/
 NULL, /* tp_print  */
-xshandle_getattr, /* tp_getattr*/
+NULL, /* tp_getattr*/
 NULL, /* tp_setattr*/
 NULL, /* tp_compare*/
 NULL, /* tp_repr   */
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/8] python: check return value of PyErr_NewException

[Marek Marczykowski-Górecki]

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 4 
 tools/python/xen/lowlevel/xs/xs.c | 4 
 2 files changed, 8 insertions(+)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index 39be1d5..7fbead5 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -2735,6 +2735,10 @@ PyMODINIT_FUNC initxc(void)
   return;
 
 xc_error_obj = PyErr_NewException(PKG ".Error", PyExc_RuntimeError, NULL);
+if (xc_error_obj == NULL) {
+Py_DECREF(m);
+return;
+}
 zero = PyInt_FromLong(0);
 
 /* KAF: This ensures that we get debug output in a timely manner. */
diff --git a/tools/python/xen/lowlevel/xs/xs.c 
b/tools/python/xen/lowlevel/xs/xs.c
index a86edbe..5772f4b 100644
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -987,6 +987,10 @@ PyMODINIT_FUNC initxs(void)
   return;
 
 xs_error = PyErr_NewException(PKG ".Error", PyExc_RuntimeError, NULL);
+if (xs_error == NULL) {
+Py_DECREF(m);
+return;
+}
 
 Py_INCREF(_type);
 PyModule_AddObject(m, CLS, (PyObject *)_type);
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 7/8] python: adjust module initalization for Python3

[Marek Marczykowski-Górecki]

In Python3, PyTypeObject looks slightly different, and also module
initialization is different. Instead of Py_InitModule, PyModule_Create
should be called on already defined PyModuleDef structure. And then
initialization function should return that module.

Additionally initialization function should be named PyInit_,
instead of init.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 35 ---
 tools/python/xen/lowlevel/xs/xs.c | 34 +++---
 2 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index df31a1d..d09c45f 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -2685,7 +2685,11 @@ static void PyXc_dealloc(XcObject *self)
 }
 
 static PyTypeObject PyXcType = {
+#if PY_MAJOR_VERSION >= 3
+.ob_base = { PyObject_HEAD_INIT(NULL) },
+#else
 PyObject_HEAD_INIT(NULL)
+#endif
 .tp_name = PKG "." CLS,
 .tp_basicsize = sizeof(XcObject),
 .tp_itemsize = 0,
@@ -2699,22 +2703,44 @@ static PyTypeObject PyXcType = {
 
 static PyMethodDef xc_methods[] = { { NULL } };
 
+
+#if PY_MAJOR_VERSION >= 3
+static PyModuleDef xc_module = {
+PyModuleDef_HEAD_INIT,
+PKG, /* name */
+NULL,   /* docstring */
+-1, /* size of per-interpreter state, -1 means the module use global
+   variables */
+xc_methods
+};
+#endif
+
+#if PY_MAJOR_VERSION >= 3
+#define INITERROR return NULL
+PyMODINIT_FUNC PyInit_xc(void)
+#else
+#define INITERROR return
 PyMODINIT_FUNC initxc(void)
+#endif
 {
 PyObject *m;
 
 if (PyType_Ready() < 0)
-return;
+INITERROR;
 
+#if PY_MAJOR_VERSION >= 3
+m = PyModule_Create(_module);
+#else
 m = Py_InitModule(PKG, xc_methods);
+#endif
 
 if (m == NULL)
-  return;
+INITERROR;
 
 xc_error_obj = PyErr_NewException(PKG ".Error", PyExc_RuntimeError, NULL);
 if (xc_error_obj == NULL) {
 Py_DECREF(m);
-return;
+INITERROR;
 }
 zero = PyLongOrInt_FromLong(0);
 
@@ -2732,6 +2758,9 @@ PyMODINIT_FUNC initxc(void)
 PyModule_AddIntConstant(m, "XEN_SCHEDULER_CREDIT", XEN_SCHEDULER_CREDIT);
 PyModule_AddIntConstant(m, "XEN_SCHEDULER_CREDIT2", XEN_SCHEDULER_CREDIT2);
 
+#if PY_MAJOR_VERSION >= 3
+return m;
+#endif
 }
 
 
diff --git a/tools/python/xen/lowlevel/xs/xs.c 
b/tools/python/xen/lowlevel/xs/xs.c
index b37daa9..aba5a20 100644
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -946,7 +946,11 @@ static void xshandle_dealloc(XsHandle *self)
 }
 
 static PyTypeObject xshandle_type = {
+#if PY_MAJOR_VERSION >= 3
+.ob_base = { PyObject_HEAD_INIT(NULL) },
+#else
 PyObject_HEAD_INIT(NULL)
+#endif
 .tp_name = PKG "." CLS,
 .tp_basicsize = sizeof(XsHandle),
 .tp_itemsize = 0,
@@ -960,22 +964,43 @@ static PyTypeObject xshandle_type = {
 
 static PyMethodDef xs_methods[] = { { NULL } };
 
+#if PY_MAJOR_VERSION >= 3
+static PyModuleDef xs_module = {
+PyModuleDef_HEAD_INIT,
+PKG, /* name */
+NULL,   /* docstring */
+-1, /* size of per-interpreter state, -1 means the module use global
+   variables */
+xs_methods
+};
+#endif
+
+#if PY_MAJOR_VERSION >= 3
+#define INITERROR return NULL
+PyMODINIT_FUNC PyInit_xs(void)
+#else
+#define INITERROR return
 PyMODINIT_FUNC initxs(void)
+#endif
 {
 PyObject* m;
 
 if (PyType_Ready(_type) < 0)
-return;
+INITERROR;
 
+#if PY_MAJOR_VERSION >= 3
+m = PyModule_Create(_module);
+#else
 m = Py_InitModule(PKG, xs_methods);
+#endif
 
 if (m == NULL)
-  return;
+INITERROR;
 
 xs_error = PyErr_NewException(PKG ".Error", PyExc_RuntimeError, NULL);
 if (xs_error == NULL) {
 Py_DECREF(m);
-return;
+INITERROR;
 }
 
 Py_INCREF(_type);
@@ -983,6 +1008,9 @@ PyMODINIT_FUNC initxs(void)
 
 Py_INCREF(xs_error);
 PyModule_AddObject(m, "Error", xs_error);
+#if PY_MAJOR_VERSION >= 3
+return m;
+#endif
 }
 
 
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 3/8] python: use Py_TYPE instead of looking directly into PyObject_HEAD

[Marek Marczykowski-Górecki]

Py_TYPE works on both Python2 and Python3, while internals of
PyObject_HEAD have changed.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 2 +-
 tools/python/xen/lowlevel/xs/xs.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index 109ef57..75842ef 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -2670,7 +2670,7 @@ static void PyXc_dealloc(XcObject *self)
 self->xc_handle = NULL;
 }
 
-self->ob_type->tp_free((PyObject *)self);
+Py_TYPE(self)->tp_free((PyObject *)self);
 }
 
 static PyTypeObject PyXcType = {
diff --git a/tools/python/xen/lowlevel/xs/xs.c 
b/tools/python/xen/lowlevel/xs/xs.c
index e9eef73..74a80ca 100644
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -922,7 +922,7 @@ static void xshandle_dealloc(XsHandle *self)
 
 Py_XDECREF(self->watches);
 
-self->ob_type->tp_free((PyObject *)self);
+Py_TYPE(self)->tp_free((PyObject *)self);
 }
 
 static PyTypeObject xshandle_type = {
-- 
2.7.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 4/8] python: initialize specific fields of PyTypeObject

[Marek Marczykowski-Górecki]

Fields not named here will be zero-initialized anyway, but using this
way will be much easier to support both Python2 and Python3.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 47 ---
 tools/python/xen/lowlevel/xs/xs.c | 47 ---
 2 files changed, 18 insertions(+), 76 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index 75842ef..bcbb7b0 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -2675,44 +2675,15 @@ static void PyXc_dealloc(XcObject *self)
 
 static PyTypeObject PyXcType = {
 PyObject_HEAD_INIT(NULL)
-0,
-PKG "." CLS,
-sizeof(XcObject),
-0,
-(destructor)PyXc_dealloc, /* tp_dealloc*/
-NULL, /* tp_print  */
-NULL, /* tp_getattr*/
-NULL, /* tp_setattr*/
-NULL, /* tp_compare*/
-NULL, /* tp_repr   */
-NULL, /* tp_as_number  */
-NULL, /* tp_as_sequence*/
-NULL, /* tp_as_mapping */
-NULL, /* tp_hash   */
-NULL, /* tp_call   */
-NULL, /* tp_str*/
-NULL, /* tp_getattro   */
-NULL, /* tp_setattro   */
-NULL, /* tp_as_buffer  */
-Py_TPFLAGS_DEFAULT,   /* tp_flags  */
-"Xen client connections", /* tp_doc*/
-NULL, /* tp_traverse   */
-NULL, /* tp_clear  */
-NULL, /* tp_richcompare*/
-0,/* tp_weaklistoffset */
-NULL, /* tp_iter   */
-NULL, /* tp_iternext   */
-pyxc_methods, /* tp_methods*/
-NULL, /* tp_members*/
-NULL, /* tp_getset */
-NULL, /* tp_base   */
-NULL, /* tp_dict   */
-NULL, /* tp_descr_get  */
-NULL, /* tp_descr_set  */
-0,/* tp_dictoffset */
-(initproc)PyXc_init,  /* tp_init   */
-NULL, /* tp_alloc  */
-PyXc_new, /* tp_new*/
+.tp_name = PKG "." CLS,
+.tp_basicsize = sizeof(XcObject),
+.tp_itemsize = 0,
+.tp_dealloc = (destructor)PyXc_dealloc,
+.tp_flags = Py_TPFLAGS_DEFAULT,
+.tp_doc = "Xen client connections",
+.tp_methods = pyxc_methods,
+.tp_init = (initproc)PyXc_init,
+.tp_new = PyXc_new,
 };
 
 static PyMethodDef xc_methods[] = { { NULL } };
diff --git a/tools/python/xen/lowlevel/xs/xs.c 
b/tools/python/xen/lowlevel/xs/xs.c
index 74a80ca..66ab08d 100644
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -927,44 +927,15 @@ static void xshandle_dealloc(XsHandle *self)
 
 static PyTypeObject xshandle_type = {
 PyObject_HEAD_INIT(NULL)
-0,
-PKG "." CLS,
-sizeof(XsHandle),
-0,
-(destructor)xshandle_dealloc, /* tp_dealloc*/
-NULL, /* tp_print  */
-NULL, /* tp_getattr*/
-NULL, /* tp_setattr*/
-NULL, /* tp_compare*/
-NULL, /* tp_repr   */
-NULL, /* tp_as_number  */
-NULL, /* tp_as_sequence*/
-NULL, /* tp_as_mapping */
-NULL, /* tp_hash   */
-NULL, /* tp_call   */
-NULL, /* tp_str*/
-NULL, /* tp_getattro   */
-NULL, /* tp_setattro   */
-NULL, /* tp_as_buffer  */
-Py_TPFLAGS_DEFAULT,   /* tp_flags  */
-"Xenstore connections",   /* tp_doc*/
-NULL, /* tp_traverse   */
-NULL, /* tp_clear  */
-NULL, /* tp_richcompare*/
-0,/* tp_weaklistoffset */
-NULL, /* tp_iter   */
-NULL, /* tp_iternext   */
- 

[Xen-devel] [PATCH 6/8] python: use PyLong_* for constructing 'int' type in Python3

[Marek Marczykowski-Górecki]

In Python3 'int' and 'long' types are the same, there are no longer
separate PyInt_* functions.  Provide convenient #defines to limit #if in
code.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/python/xen/lowlevel/xc/xc.c | 51 ---
 tools/python/xen/lowlevel/xs/xs.c |  8 ++
 2 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/tools/python/xen/lowlevel/xc/xc.c 
b/tools/python/xen/lowlevel/xc/xc.c
index ce87afb..df31a1d 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -34,6 +34,17 @@
 
 #define FLASK_CTX_LEN 1024
 
+/* Python 2 compatibility */
+#if PY_MAJOR_VERSION >= 3
+#define PyLongOrInt_FromLong PyLong_FromLong
+#define PyLongOrInt_Check PyLong_Check
+#define PyLongOrInt_AsLong PyLong_AsLong
+#else
+#define PyLongOrInt_FromLong PyInt_FromLong
+#define PyLongOrInt_Check PyInt_Check
+#define PyLongOrInt_AsLong PyInt_AsLong
+#endif
+
 static PyObject *xc_error_obj, *zero;
 
 typedef struct {
@@ -127,9 +138,9 @@ static PyObject *pyxc_domain_create(XcObject *self,
 for ( i = 0; i < sizeof(xen_domain_handle_t); i++ )
 {
 PyObject *p = PyList_GetItem(pyhandle, i);
-if ( !PyInt_Check(p) )
+if ( !PyLongOrInt_Check(p) )
 goto out_exception;
-handle[i] = (uint8_t)PyInt_AsLong(p);
+handle[i] = (uint8_t)PyLongOrInt_AsLong(p);
 }
 }
 
@@ -142,7 +153,7 @@ static PyObject *pyxc_domain_create(XcObject *self,
 return pyxc_error_to_exception(self->xc_handle);
 
 
-return PyInt_FromLong(dom);
+return PyLongOrInt_FromLong(dom);
 
 out_exception:
 errno = EINVAL;
@@ -242,7 +253,7 @@ static PyObject *pyxc_vcpu_setaffinity(XcObject *self,
 {
 for ( i = 0; i < PyList_Size(cpulist); i++ ) 
 {
-long cpu = PyInt_AsLong(PyList_GetItem(cpulist, i));
+long cpu = PyLongOrInt_AsLong(PyList_GetItem(cpulist, i));
 if ( cpu < 0 || cpu >= nr_cpus )
 {
 free(cpumap);
@@ -284,9 +295,9 @@ static PyObject *pyxc_domain_sethandle(XcObject *self, 
PyObject *args)
 for ( i = 0; i < sizeof(xen_domain_handle_t); i++ )
 {
 PyObject *p = PyList_GetItem(pyhandle, i);
-if ( !PyInt_Check(p) )
+if ( !PyLongOrInt_Check(p) )
 goto out_exception;
-handle[i] = (uint8_t)PyInt_AsLong(p);
+handle[i] = (uint8_t)PyLongOrInt_AsLong(p);
 }
 
 if (xc_domain_sethandle(self->xc_handle, dom, handle) < 0)
@@ -361,7 +372,7 @@ static PyObject *pyxc_domain_getinfo(XcObject *self,
 return NULL;
 }
 for ( j = 0; j < sizeof(xen_domain_handle_t); j++ )
-PyList_SetItem(pyhandle, j, PyInt_FromLong(info[i].handle[j]));
+PyList_SetItem(pyhandle, j, 
PyLongOrInt_FromLong(info[i].handle[j]));
 PyDict_SetItemString(info_dict, "handle", pyhandle);
 Py_DECREF(pyhandle);
 PyList_SetItem(list, i, info_dict);
@@ -420,7 +431,7 @@ static PyObject *pyxc_vcpu_getinfo(XcObject *self,
 for ( i = 0; i < nr_cpus; i++ )
 {
 if (*(cpumap + i / 8) & 1 ) {
-PyObject *pyint = PyInt_FromLong(i);
+PyObject *pyint = PyLongOrInt_FromLong(i);
 PyList_Append(cpulist, pyint);
 Py_DECREF(pyint);
 }
@@ -836,7 +847,7 @@ static PyObject *pyxc_evtchn_alloc_unbound(XcObject *self,
 if ( (port = xc_evtchn_alloc_unbound(self->xc_handle, dom, remote_dom)) < 
0 )
 return pyxc_error_to_exception(self->xc_handle);
 
-return PyInt_FromLong(port);
+return PyLongOrInt_FromLong(port);
 }
 
 static PyObject *pyxc_evtchn_reset(XcObject *self,
@@ -1063,7 +1074,7 @@ static PyObject *pyxc_topologyinfo(XcObject *self)
 }
 else
 {
-PyObject *pyint = PyInt_FromLong(cputopo[i].core);
+PyObject *pyint = PyLongOrInt_FromLong(cputopo[i].core);
 PyList_Append(cpu_to_core_obj, pyint);
 Py_DECREF(pyint);
 }
@@ -1074,7 +1085,7 @@ static PyObject *pyxc_topologyinfo(XcObject *self)
 }
 else
 {
-PyObject *pyint = PyInt_FromLong(cputopo[i].socket);
+PyObject *pyint = PyLongOrInt_FromLong(cputopo[i].socket);
 PyList_Append(cpu_to_socket_obj, pyint);
 Py_DECREF(pyint);
 }
@@ -1085,7 +1096,7 @@ static PyObject *pyxc_topologyinfo(XcObject *self)
 }
 else
 {
-PyObject *pyint = PyInt_FromLong(cputopo[i].node);
+PyObject *pyint = PyLongOrInt_FromLong(cputopo[i].node);
 PyList_Append(cpu_to_node_obj, pyint);
 Py_DECREF(pyint);
 }
@@ -1139,18 +1150,18 @@ static PyObject *pyxc_numainfo(XcObject *self)
 unsigned invalid_node;
 
 /* Total Memory */
-pyint = PyI

Re: [Xen-devel] Python 3 bindings

[Marek Marczykowski-Górecki]

On Wed, Feb 22, 2017 at 11:34:16AM +, Wei Liu wrote:
> On Tue, Feb 21, 2017 at 02:03:00PM +0100, Marek Marczykowski-Górecki wrote:
> > On Mon, Feb 20, 2017 at 05:18:44PM +, Wei Liu wrote:
> > > On Fri, Feb 17, 2017 at 01:36:01PM +0100, Marek Marczykowski-Górecki 
> > > wrote:
> > > > Hi,
> > > > 
> > > > I'm adjusting python bindings to work on python3 too. This will require
> > > > few #if in the code (to compile for both python2 and python3), but it
> > > > isn't that bad. But there are some major changes in python3, which
> > > > require some decision about the bindings API:
> > > > 
> > > > 1. Python3 has no longer separate 'int' and 'long' type - old 'long'
> > > > type was renamed to 'int' (but on C-API level, it uses PyLong_*). I see
> > > > two options:
> > > >   - switch to PyLong_* everywhere, including python2 bindings - this
> > > > makes the code much cleaner, but it is an API change in python2
> > > >   - switch to PyLong_* only for python3 - this will introduce some
> > > > #ifdefs, but python2 API will be unchanged
> > > 
> > > Could you be more specific? Like, provide a code snippet?
> > 
> > Here is compile tested only version:
> > https://github.com/marmarek/xen/tree/python3
> > 
> > It uses PyLong_* only for python3, here is how it looks in code (I've
> > skipped s/PyInt_/PyLongOrInt_/ for readability):
> > 
> > -8<-
> > --- a/tools/python/xen/lowlevel/xc/xc.c
> > +++ b/tools/python/xen/lowlevel/xc/xc.c
> > @@ -34,6 +34,17 @@
> >  
> >  #define FLASK_CTX_LEN 1024
> >  
> > +/* Python 2 compatibility */
> > +#if PY_VERSION_HEX >= 0x0300
> > +#define PyLongOrInt_FromLong PyLong_FromLong
> > +#define PyLongOrInt_Check PyLong_Check
> > +#define PyLongOrInt_AsLong PyLong_AsLong
> > +#else
> > +#define PyLongOrInt_FromLong PyInt_FromLong
> > +#define PyLongOrInt_Check PyInt_Check
> > +#define PyLongOrInt_AsLong PyInt_AsLong
> > +#endif
> > +
> >  static PyObject *xc_error_obj, *zero;
> >  
> >  typedef struct {
> > --- a/tools/python/xen/lowlevel/xs/xs.c
> > +++ b/tools/python/xen/lowlevel/xs/xs.c
> > @@ -43,6 +43,14 @@
> >  #define PKG "xen.lowlevel.xs"
> >  #define CLS "xs"
> >  
> > +#if PY_VERSION_HEX < 0x0300
> > +/* Python 2 compatibility */
> > +#define PyLong_FromLong PyInt_FromLong
> > +#undef PyLong_Check
> > +#define PyLong_Check PyInt_Check
> > +#define PyLong_AsLong PyInt_AsLong
> > +#endif
> > +
> >  static PyObject *xs_error;
> >  
> 
> If this is the recommended practice, then that's fine. I can't seem to
> find such practice in https://docs.python.org/3/howto/cporting.html but
> I'm no python binding expert.

"In the C-API, PyInt_* functions are replaced by their PyLong_*
equivalents."

> BTW, I went through your python3 branch. It seems that some patches can
> be submitted independently.

Yes, I've tried to separate cleanup commits from actual python3 support.

> >  /** Python wrapper round an xs handle.
> > -8<-
> > 
> > > 
> > > > 
> > > > 2. Python3 has no longer separate 'str' and 'unicode' type, new 'str' is
> > > > the same as 'unicode' (PyUnicode_* at C-API level). For things not
> > > > really unicode-aware, 'bytes' type should be used. On the other hand, in
> > > > python2 'bytes' type was the same as 'str'.
> > > > This affects various places, where in most cases 'bytes' type is
> > > > appropriate (for example cpuid). But I'm not sure about xenstore paths -
> > > > those should also be 'bytes', or maybe 'unicode' (which is implicitly
> > > > using 'utf-8' encoding)? I think the only reason to use 'unicode' is
> > > 
> > > According to docs/txt/misc/xenstore.txt, paths should be ASCII
> > > alphanumerics plus four punctuation characters. Not sure if this is
> > > relevant to what you describe.
> > 
> > It's easy to make function accept both 'bytes' and 'unicode'. The
> > question is what should be return type (read_watch, ls etc) - given
> > limited character set used there, I'm in favor of 'unicode' - easier to
> > handle, but we shouldn't hit any unicode decoding problems.
> > Maybe the same should apply to path arguments (use 'unicode')? Most
> > file-handling methods in python3 use 'unicode' for paths, if that
> > matters.
> > 
> 
> OK. Using unicode makes sense to me. Again, I'm no python expert and I
> trust what you said. :-)

Ok, I'll adjust patches to return unicode paths.

Then test them and actually send here :)

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Python 3 bindings

[Marek Marczykowski-Górecki]

On Mon, Feb 20, 2017 at 05:18:44PM +, Wei Liu wrote:
> On Fri, Feb 17, 2017 at 01:36:01PM +0100, Marek Marczykowski-Górecki wrote:
> > Hi,
> > 
> > I'm adjusting python bindings to work on python3 too. This will require
> > few #if in the code (to compile for both python2 and python3), but it
> > isn't that bad. But there are some major changes in python3, which
> > require some decision about the bindings API:
> > 
> > 1. Python3 has no longer separate 'int' and 'long' type - old 'long'
> > type was renamed to 'int' (but on C-API level, it uses PyLong_*). I see
> > two options:
> >   - switch to PyLong_* everywhere, including python2 bindings - this
> > makes the code much cleaner, but it is an API change in python2
> >   - switch to PyLong_* only for python3 - this will introduce some
> > #ifdefs, but python2 API will be unchanged
> 
> Could you be more specific? Like, provide a code snippet?

Here is compile tested only version:
https://github.com/marmarek/xen/tree/python3

It uses PyLong_* only for python3, here is how it looks in code (I've
skipped s/PyInt_/PyLongOrInt_/ for readability):

-8<-
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -34,6 +34,17 @@
 
 #define FLASK_CTX_LEN 1024
 
+/* Python 2 compatibility */
+#if PY_VERSION_HEX >= 0x0300
+#define PyLongOrInt_FromLong PyLong_FromLong
+#define PyLongOrInt_Check PyLong_Check
+#define PyLongOrInt_AsLong PyLong_AsLong
+#else
+#define PyLongOrInt_FromLong PyInt_FromLong
+#define PyLongOrInt_Check PyInt_Check
+#define PyLongOrInt_AsLong PyInt_AsLong
+#endif
+
 static PyObject *xc_error_obj, *zero;
 
 typedef struct {
--- a/tools/python/xen/lowlevel/xs/xs.c
+++ b/tools/python/xen/lowlevel/xs/xs.c
@@ -43,6 +43,14 @@
 #define PKG "xen.lowlevel.xs"
 #define CLS "xs"
 
+#if PY_VERSION_HEX < 0x0300
+/* Python 2 compatibility */
+#define PyLong_FromLong PyInt_FromLong
+#undef PyLong_Check
+#define PyLong_Check PyInt_Check
+#define PyLong_AsLong PyInt_AsLong
+#endif
+
 static PyObject *xs_error;
 
 /** Python wrapper round an xs handle.
-8<-

> 
> > 
> > 2. Python3 has no longer separate 'str' and 'unicode' type, new 'str' is
> > the same as 'unicode' (PyUnicode_* at C-API level). For things not
> > really unicode-aware, 'bytes' type should be used. On the other hand, in
> > python2 'bytes' type was the same as 'str'.
> > This affects various places, where in most cases 'bytes' type is
> > appropriate (for example cpuid). But I'm not sure about xenstore paths -
> > those should also be 'bytes', or maybe 'unicode' (which is implicitly
> > using 'utf-8' encoding)? I think the only reason to use 'unicode' is
> 
> According to docs/txt/misc/xenstore.txt, paths should be ASCII
> alphanumerics plus four punctuation characters. Not sure if this is
> relevant to what you describe.

It's easy to make function accept both 'bytes' and 'unicode'. The
question is what should be return type (read_watch, ls etc) - given
limited character set used there, I'm in favor of 'unicode' - easier to
handle, but we shouldn't hit any unicode decoding problems.
Maybe the same should apply to path arguments (use 'unicode')? Most
file-handling methods in python3 use 'unicode' for paths, if that
matters.

> > convenience for API users - in python3 if you write 'some string' it
> > will be unicode type, to create bytes data you need to write b'some
> > string'.
> > As for python2, it should definitely be still 'str'/'bytes' type.
> > 
> > There is one more little detail - build process. Here I'm going to
> > follow popular standard - use $(PYTHON) variable - if that points to
> > python3, build for python3. Actually this means no change in the current
> > makefile. If someone want to build for both python2 and python3, will
> > need to call the build twice - at packaging level.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] Python 3 bindings

[Marek Marczykowski-Górecki]

Hi,

I'm adjusting python bindings to work on python3 too. This will require
few #if in the code (to compile for both python2 and python3), but it
isn't that bad. But there are some major changes in python3, which
require some decision about the bindings API:

1. Python3 has no longer separate 'int' and 'long' type - old 'long'
type was renamed to 'int' (but on C-API level, it uses PyLong_*). I see
two options:
  - switch to PyLong_* everywhere, including python2 bindings - this
makes the code much cleaner, but it is an API change in python2
  - switch to PyLong_* only for python3 - this will introduce some
#ifdefs, but python2 API will be unchanged

2. Python3 has no longer separate 'str' and 'unicode' type, new 'str' is
the same as 'unicode' (PyUnicode_* at C-API level). For things not
really unicode-aware, 'bytes' type should be used. On the other hand, in
python2 'bytes' type was the same as 'str'.
This affects various places, where in most cases 'bytes' type is
appropriate (for example cpuid). But I'm not sure about xenstore paths -
those should also be 'bytes', or maybe 'unicode' (which is implicitly
using 'utf-8' encoding)? I think the only reason to use 'unicode' is
convenience for API users - in python3 if you write 'some string' it
will be unicode type, to create bytes data you need to write b'some
string'.
As for python2, it should definitely be still 'str'/'bytes' type.

There is one more little detail - build process. Here I'm going to
follow popular standard - use $(PYTHON) variable - if that points to
python3, build for python3. Actually this means no change in the current
makefile. If someone want to build for both python2 and python3, will
need to call the build twice - at packaging level.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

On Mon, Jan 16, 2017 at 05:17:59AM -0700, Jan Beulich wrote:
> >>> On 14.01.17 at 03:52, <marma...@invisiblethingslab.com> wrote:
> > On Sat, Jan 14, 2017 at 01:47:49AM +, Andrew Cooper wrote:
> >> In a native situation, SMAP raises #PF if hardware tries to follow a
> >> user pointer outside of an area whitelisted by the kernel.  (The
> >> copy_*_guest path suppresses #PF, opting instead to return -EFAULT.)
> >> 
> >> The choice of supervisor vs user in a pagewalk is a single bit, and Xen
> >> (for better or worse, but realistically as a consequence of SMAP being
> >> ~10 years younger than HVM guests) accesses pointers from hypercalls as
> >> a supervisor entity when walking the pagetables.  The key point here is
> >> that Xen must emulate the hardware pagewalker when trying to find the
> >> data being pointed to.
> >> 
> >> If Xen has a bug causing spurious accesses to HVM guests, then all bets
> >> are already off wrt memory corruption, because real hardware isn't used
> >> to make the access.
> >> 
> >> As indicated before, we could deliberately break the Xen pagewalker to
> >> ignore SMAP.  However, that would be identical to "pretend the guest
> >> actually whitelisted this access". 
> > 
> > I think there is important difference between "whitelist all accesses
> > to guest user memory for the hypercall time" vs "whitelist accesses done
> > through copy_from_guest/copy_to_guest only". If I understand correctly
> > above description, modifying privcmd_call would also suppress #PF when
> > Xen would be tricked to access guest memory outside of copy_*_guest. Am
> > I missing something?
> 
> There are two additional things to consider here:
> 
> 1) For HVM guests, Xen can't access guest memory unintentionally,
> as (other than in the PV case) virtual address spaces are distinct.

Good point.

> 2) When the guest issues stac()/clac(), it indicates to Xen _its own_
> intended view, without affecting Xen's. That is, as soon as hypervisor
> context is being entered again, SMAP protection would be in effect
> again (albeit as per point 1 guarding only against accessing PV guest
> mappings).
> 
> So the driver adjustment suggested by Andrew has an effect on only
> page walks done by Xen during copy_{to,from}_guest(), but not on
> actual memory accesses.

Ok, so indeed the kernel patch makes the most sense here. Is the change
in this shape (if works - I'll test it shortly) good to include
upstream, or is it "ugly hack"?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

On Sat, Jan 14, 2017 at 01:47:49AM +, Andrew Cooper wrote:
> On 13/01/2017 20:32, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 07:54:01PM +, Andrew Cooper wrote:
> >> This is a guest kernel bug.  The guest kernel has used SMAP to say
> >> "please disallow all userspace accesses", and Xen is respecting this
> >> when trying to follow the pointer in the hypercall.
> > Hmm, if that's the case, isn't the above patch also effectively breaking
> > SMAP?
> 
> No.
> 
> > I see the purpose of SMAP is to prevent _accidental_ access to
> > arbitrary, guest controlled memory.
> 
> This is correct.  The point of SMAP is to prevent accidental access to
> user memory.
> 
> The ABI of the privcmd hypercall ioctl() is to pass values straight to
> the hypervisor, and it is a fact that most of these hypercalls contain
> pointers to hypercall buffers, which reside in userspace.
> 
> (There is a separate thread ongoing questioning whether the ABI is
> appropriate, but irrespective of those results, this is how the ABI
> currently behaves.  A concerned person might note that anyone with an
> open privcmd fd can issue any hypercall, including the ones only
> intended for kernels, such as set_trap_table, update_va_mapping and iret.)
> 
> Therefore, it is an expected consequence that a hypercall passed blindly
> from a userspace agent contains userspace pointers which Xen must follow
> to complete the hypercall successfully.
> 
> > For example because of some memory
> > corruption bug in the hypervisor. If such a bug could be triggered using
> > a hypercall, then the above patch also makes SMAP useless.  Patching
> > copy_from_guest/copy_to_guest on the other hand would only allow such
> > guest controlled memory access when hypervisor is really supposed to
> > access guest memory.
> 
> I don't follow this argument.
> 
> In a native situation, SMAP raises #PF if hardware tries to follow a
> user pointer outside of an area whitelisted by the kernel.  (The
> copy_*_guest path suppresses #PF, opting instead to return -EFAULT.)
> 
> The choice of supervisor vs user in a pagewalk is a single bit, and Xen
> (for better or worse, but realistically as a consequence of SMAP being
> ~10 years younger than HVM guests) accesses pointers from hypercalls as
> a supervisor entity when walking the pagetables.  The key point here is
> that Xen must emulate the hardware pagewalker when trying to find the
> data being pointed to.
> 
> If Xen has a bug causing spurious accesses to HVM guests, then all bets
> are already off wrt memory corruption, because real hardware isn't used
> to make the access.
> 
> As indicated before, we could deliberately break the Xen pagewalker to
> ignore SMAP.  However, that would be identical to "pretend the guest
> actually whitelisted this access". 

I think there is important difference between "whitelist all accesses
to guest user memory for the hypercall time" vs "whitelist accesses done
through copy_from_guest/copy_to_guest only". If I understand correctly
above description, modifying privcmd_call would also suppress #PF when
Xen would be tricked to access guest memory outside of copy_*_guest. Am
I missing something?

Anyway, if someone can access /dev/xen/privcmd and issue hypercalls,
probably also can whitelist userspace access... So the practical
difference is very small - apart from that I control Xen version, but
not necessary the kernel. And I think this applies to many more Xen
users (from which only I've tripped over this issue...). What is the
point of SMAP if guest can disable it? Is it only about protecting
hypervisor when attacker _does not_ control guest kernel?

> This would fix your symptoms, but
> open up a hole in the case that userspace somehow managed to trick Xen
> into thinking a legitimate hypercall had been made, at which point Xen
> would ignore the last level of protection that SMAP was supposed to offer.
> 
> >> This bug doesn't
> >> manifest for PV guests, and you are probably the first person to do any
> >> serious hypercall work from HVM userspace.
> > That's interesting. I'm just trying to use slightly modified
> > libxenchan...
> 
> And I believe that you are first person (in 6 years of being a part of
> upstream Xen) who I have positively identified as having used
> libxenchan.  (There have been a few comments along the line of "oh yeah
> - there is that libxenchan thing which might be worth looking at".)
> 
> It does raise some questions though.  Linux has (what is supposed to be
> a) perfectly good /dev/xen/evtchn, for interacting with event channels. 
> Why is libxenchan open-coding the hypercalls using a

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

On Fri, Jan 13, 2017 at 07:54:01PM +, Andrew Cooper wrote:
> On 13/01/17 19:40, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 07:27:20PM +, Andrew Cooper wrote:
> >> On 13/01/17 18:59, Marek Marczykowski-Górecki wrote:
> >>> On Fri, Jan 13, 2017 at 06:37:06PM +, Andrew Cooper wrote:
> >>>> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
> >>>>> On Fri, Jan 13, 2017 at 06:15:35PM +, Andrew Cooper wrote:
> >>>>>> Can you get the result of this piece of debugging in the failure case?
> >>>>> I've got this:
> >>>>> ** d4v0 CFG(24, 7f794bd07004, 1) = 24
> >>>> Silly question (and I really hope the answer isn't yes, but I have a
> >>>> sinking feeling it is).
> >>>>
> >>>> Is the guest in question using SMAP? If so, does disabling SMAP fix the
> >>>> problem?
> >>> How can I check that? If looking at 0x20 CR4 bit in `xl debug-keys
> >>> v` output enough, then yes - it's enabled. And booting hypervisor with
> >>> smap=0 "fixed" the problem.
> >> :(, although now I think about it, there might be a quick option.
> >>
> >>> So, what would be the correct solution? I'd prefer not to disable SMAP
> >>> "just" for this reason...
> >> For the quick option, the privcmd driver in Linux needs a stac()/clac()
> >> pair around the actual hypercall invocation, to whitelist userspace
> >> memory accesses as being ok.
> >>
> >> Something like this (completely untested)
> >>
> >> andrewcoop@andrewcoop:/local/linux.git/arch/x86$ git diff
> >> diff --git a/arch/x86/include/asm/xen/hypercall.h
> >> b/arch/x86/include/asm/xen/hypercall.h
> >> index a12a047..e1b2af9e 100644
> >> --- a/arch/x86/include/asm/xen/hypercall.h
> >> +++ b/arch/x86/include/asm/xen/hypercall.h
> >> @@ -214,10 +214,12 @@ privcmd_call(unsigned call,
> >> __HYPERCALL_DECLS;
> >> __HYPERCALL_5ARG(a1, a2, a3, a4, a5);
> >>  
> >> +   stac();
> >> asm volatile("call *%[call]"
> >>  : __HYPERCALL_5PARAM
> >>  : [call] "a" (_page[call])
> >>  : __HYPERCALL_CLOBBER5);
> >> +   clac();
> >>  
> >> return (long)__res;
> >>  }
> > Is there any option to do that from hypervisor side? For example somehow
> > modify copy_from_guest/copy_to_guest? I'm not always controlling the VM
> > kernel (for example sometimes I need to cope with the one from Debian
> > stable).
> 
> Not really.  (I mean - there is, but it involves deliberately breaking
> SMAP support in Xen.)
> 
> This is a guest kernel bug.  The guest kernel has used SMAP to say
> "please disallow all userspace accesses", and Xen is respecting this
> when trying to follow the pointer in the hypercall.

Hmm, if that's the case, isn't the above patch also effectively breaking
SMAP? I see the purpose of SMAP is to prevent _accidental_ access to
arbitrary, guest controlled memory. For example because of some memory
corruption bug in the hypervisor. If such a bug could be triggered using
a hypercall, then the above patch also makes SMAP useless.  Patching
copy_from_guest/copy_to_guest on the other hand would only allow such
guest controlled memory access when hypervisor is really supposed to
access guest memory.

> This bug doesn't
> manifest for PV guests, and you are probably the first person to do any
> serious hypercall work from HVM userspace.

That's interesting. I'm just trying to use slightly modified
libxenchan...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

On Fri, Jan 13, 2017 at 07:27:20PM +, Andrew Cooper wrote:
> On 13/01/17 18:59, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 06:37:06PM +, Andrew Cooper wrote:
> >> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
> >>> On Fri, Jan 13, 2017 at 06:15:35PM +, Andrew Cooper wrote:
> >>>> Can you get the result of this piece of debugging in the failure case?
> >>> I've got this:
> >>> ** d4v0 CFG(24, 7f794bd07004, 1) = 24
> >> Silly question (and I really hope the answer isn't yes, but I have a
> >> sinking feeling it is).
> >>
> >> Is the guest in question using SMAP? If so, does disabling SMAP fix the
> >> problem?
> > How can I check that? If looking at 0x20 CR4 bit in `xl debug-keys
> > v` output enough, then yes - it's enabled. And booting hypervisor with
> > smap=0 "fixed" the problem.
> 
> :(, although now I think about it, there might be a quick option.
> 
> > So, what would be the correct solution? I'd prefer not to disable SMAP
> > "just" for this reason...
> 
> For the quick option, the privcmd driver in Linux needs a stac()/clac()
> pair around the actual hypercall invocation, to whitelist userspace
> memory accesses as being ok.
> 
> Something like this (completely untested)
> 
> andrewcoop@andrewcoop:/local/linux.git/arch/x86$ git diff
> diff --git a/arch/x86/include/asm/xen/hypercall.h
> b/arch/x86/include/asm/xen/hypercall.h
> index a12a047..e1b2af9e 100644
> --- a/arch/x86/include/asm/xen/hypercall.h
> +++ b/arch/x86/include/asm/xen/hypercall.h
> @@ -214,10 +214,12 @@ privcmd_call(unsigned call,
> __HYPERCALL_DECLS;
> __HYPERCALL_5ARG(a1, a2, a3, a4, a5);
>  
> +   stac();
> asm volatile("call *%[call]"
>  : __HYPERCALL_5PARAM
>  : [call] "a" (_page[call])
>  : __HYPERCALL_CLOBBER5);
> +   clac();
>  
> return (long)__res;
>  }

Is there any option to do that from hypervisor side? For example somehow
modify copy_from_guest/copy_to_guest? I'm not always controlling the VM
kernel (for example sometimes I need to cope with the one from Debian
stable).

> For the longer option, introducing a non-virtual ABI for Xen.  This is
> going to become a necessary prerequisite to support AMD's Secure Virtual
> Encryption technology (where the hypervisor deliberately cannot follow
> the pagetables), and would remove the overhead of Xen having to walk the
> guest pagetables.
> 
> Another optimisation would be to alter some of the ops to pass their
> parameters in registers rather than in memory.  There are quite a few
> ops which pass pointers to a single int, which could be completed more
> efficiently by Xen (for both PV and HVM guests) by avoiding the memory
> access entirely.

Yes, but it will not solve all the cases.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

On Fri, Jan 13, 2017 at 06:37:06PM +, Andrew Cooper wrote:
> On 13/01/17 18:32, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 06:15:35PM +, Andrew Cooper wrote:
> >> Can you get the result of this piece of debugging in the failure case?
> > I've got this:
> > ** d4v0 CFG(24, 7f794bd07004, 1) = 24
> 
> Silly question (and I really hope the answer isn't yes, but I have a
> sinking feeling it is).
> 
> Is the guest in question using SMAP? If so, does disabling SMAP fix the
> problem?

How can I check that? If looking at 0x20 CR4 bit in `xl debug-keys
v` output enough, then yes - it's enabled. And booting hypervisor with
smap=0 "fixed" the problem.
So, what would be the correct solution? I'd prefer not to disable SMAP
"just" for this reason...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

On Fri, Jan 13, 2017 at 06:15:35PM +, Andrew Cooper wrote:
> On 13/01/17 18:03, Marek Marczykowski-Górecki wrote:
> > On Fri, Jan 13, 2017 at 05:38:42PM +, Andrew Cooper wrote:
> >> On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
> >>> Hi,
> >>>
> >>> I have a strange problem - xc_evtchn_status fails when running in HVM,
> >>> while exactly the same code (same kernel, same application etc) works
> >>> fine in PV. I've narrowed it down to copy_from_guest call in
> >>> common/event_channel.c, but no idea why it fails there. Xen version is
> >>> 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?
> >> Which specific copy_from_guest() call?
> >>
> >> Copying data out of a PV guest is different to copying out of a HVM
> >> guest, but copy_from_guest() should cope properly with both.
> >>
> >> However, to progress, it would help to know exactly which piece of data
> >> is being requested.
> > This one:
> > https://github.com/xen-project/xen/blob/stable-4.8/xen/common/event_channel.c#L1104
> >
> > case EVTCHNOP_status: {
> > struct evtchn_status status;
> > if ( copy_from_guest(, arg, 1) != 0 )
> > ^
> > return -EFAULT;
> > rc = evtchn_status();
> > if ( !rc && __copy_to_guest(arg, , 1) )
> > rc = -EFAULT;
> > break;
> >
> > The evtchn_status structure in application is on stack (local variable),
> > but I think it shouldn't matter, as libxc copy it to a bounce buffer.
> >
> 
> The intent of bounce buffers is certainly to avoid this problem from
> happening.
> 
> Is this a 32bit HVM guest?  Compat argument translation does make the
> logic a little more complicated.

No, its 64bit guest.

> Can you get the result of this piece of debugging in the failure case?

I've got this:
** d4v0 CFG(24, 7f794bd07004, 1) = 24

> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index 638dc5e..ab5d82a 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -1101,8 +1101,13 @@ long do_event_channel_op(int cmd,
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>  case EVTCHNOP_status: {
>  struct evtchn_status status;
> -if ( copy_from_guest(, arg, 1) != 0 )
> +unsigned int res = copy_from_guest(, arg, 1);
> +    if ( res != 0 )
> +    {
> +printk("** %pv CFG(%zu, %p, 1) = %u\n",
> +   current, sizeof(status), _p(arg.p), res);
>  return -EFAULT;
> +}
>  rc = evtchn_status();
>  if ( !rc && __copy_to_guest(arg, , 1) )
>  rc = -EFAULT;

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

On Fri, Jan 13, 2017 at 05:38:42PM +, Andrew Cooper wrote:
> On 13/01/17 17:31, Marek Marczykowski-Górecki wrote:
> > Hi,
> >
> > I have a strange problem - xc_evtchn_status fails when running in HVM,
> > while exactly the same code (same kernel, same application etc) works
> > fine in PV. I've narrowed it down to copy_from_guest call in
> > common/event_channel.c, but no idea why it fails there. Xen version is
> > 4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?
> 
> Which specific copy_from_guest() call?
> 
> Copying data out of a PV guest is different to copying out of a HVM
> guest, but copy_from_guest() should cope properly with both.
> 
> However, to progress, it would help to know exactly which piece of data
> is being requested.

This one:
https://github.com/xen-project/xen/blob/stable-4.8/xen/common/event_channel.c#L1104

case EVTCHNOP_status: {
struct evtchn_status status;
if ( copy_from_guest(, arg, 1) != 0 )
^
return -EFAULT;
rc = evtchn_status();
if ( !rc && __copy_to_guest(arg, , 1) )
rc = -EFAULT;
break;

The evtchn_status structure in application is on stack (local variable),
but I think it shouldn't matter, as libxc copy it to a bounce buffer.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works

[Marek Marczykowski-Górecki]

Hi,

I have a strange problem - xc_evtchn_status fails when running in HVM,
while exactly the same code (same kernel, same application etc) works
fine in PV. I've narrowed it down to copy_from_guest call in
common/event_channel.c, but no idea why it fails there. Xen version is
4.8.0. kernel is kernel-4.8.13-100.fc23. Any idea?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/3] libxl: attach xen-pciback only to PV domains

[Marek Marczykowski-Górecki]

On Tue, Oct 25, 2016 at 09:10:02AM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Oct 20, 2016 at 12:42:33AM +0200, Marek Marczykowski-Górecki wrote:
> > On Wed, Oct 19, 2016 at 04:46:26PM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Wed, Oct 19, 2016 at 10:37:52AM +0100, Wei Liu wrote:
> > > > On Tue, Oct 18, 2016 at 03:53:31AM +0200, Marek Marczykowski-Górecki 
> > > > wrote:
> > > > > HVM domains use IOMMU and device model assistance for communicating 
> > > > > with
> > > > > PCI devices, xen-pcifront/pciback is used only in PV domains.
> > > > 
> > > > This bit of description is in line with my understanding of how PCI
> > > > passthrough works.
> > > 
> > > Kind of. Pciback is also used to "own" the PCI devices. And in fact
> > > they do an important job of resetting the PCI device when the
> > > device is "bind" to pciback:
> > > 
> > > echo  > bind
> > 
> > This part is still done.
> > 
> > > And .. this is the important part - when device changes ownership.
> > > That is when you disconnect it from one guest and assign to another.
> > > You need to reset the device in between. The code that calls
> > > the pci_reset_function is called by:
> > > 
> > > } 
> > >   
> > >   
> > >   
> > > /*
> > >   
> > >  * Called when:   
> > >   
> > >  *  - XenBus state has been reconfigure (pci unplug). See 
> > > xen_pcibk_remove_device
> > >  *  - XenBus state has been disconnected (guest shutdown). See 
> > > xen_pcibk_xenbus_remove
> > 
> > But this, in case of HVM without stubdomain, is not.
> > 
> > >  *  - 'echo BDF > unbind' on pciback module with no guest attached. See 
> > > pcistub_remove
> > >  *  - 'echo BDF > unbind' with a guest still using it. See pcistub_remove 
> > >   
> > >  *
> > >   
> > >  *  As such we have to be careful.
> > >   
> > >  *
> > >   
> > >  *  To make this easier, the caller has to hold the device lock.  
> > >   
> > >  */   
> > >   
> > > void pcistub_put_pci_dev(struct pci_dev *dev)
> > > 
> > > The first two are done when XenStore 'pci' entries are active - which
> > > this patch will remove and introduce a potential security problem.
> > > 
> > > Unless libxl does an 'unbind' followed by an 'bind'?
> > 
> > What about libxl__device_pci_reset, which is called (at least) before
> > attaching device to some domain, even after my patch and even if the
> > device is already bound to pciback. It tries to reset the device using
> > 'reset' entry in sysfs. I see this isn't available for some devices -
> > can pci_reset_function do any better?
> 
> My vague recollection was that it tried to do it but it aborted
> earlier due to holding locks (dev_lock is held when you do any
> operation on the SysFS). But I may be forgetting the details.
> 
> I need to look in the Linux code to confirm what the tricky part was.

Thanks. This is the last thing holding me from sending v2.

Anyway, if attaching xen-pciback to /something/ is needed, how should it
look? We have 3 cases:
1. PV - without qemu
2. HVM - with qemu in dom0
3. HVM - with qemu in stubdomain
And soon there will be 4th: PVH - without qemu

For 1 and 4 the device should be attached (in terms of xenstore) to the
target domain, as xen-pcifront (or equivalent) running there will be
used. BTW is that true for PVHv2?
For 3 - it should be attached to stubdomain (which is the case).
The question is what about 2 - should it be attached to the target domain,
even though it will not be used?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/3] libxl: attach xen-pciback only to PV domains

[Marek Marczykowski-Górecki]

On Wed, Oct 19, 2016 at 04:46:26PM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 19, 2016 at 10:37:52AM +0100, Wei Liu wrote:
> > On Tue, Oct 18, 2016 at 03:53:31AM +0200, Marek Marczykowski-Górecki wrote:
> > > HVM domains use IOMMU and device model assistance for communicating with
> > > PCI devices, xen-pcifront/pciback is used only in PV domains.
> > 
> > This bit of description is in line with my understanding of how PCI
> > passthrough works.
> 
> Kind of. Pciback is also used to "own" the PCI devices. And in fact
> they do an important job of resetting the PCI device when the
> device is "bind" to pciback:
> 
> echo  > bind

This part is still done.

> And .. this is the important part - when device changes ownership.
> That is when you disconnect it from one guest and assign to another.
> You need to reset the device in between. The code that calls
> the pci_reset_function is called by:
> 
> } 
>   
>   
>   
> /*
>   
>  * Called when:   
>   
>  *  - XenBus state has been reconfigure (pci unplug). See 
> xen_pcibk_remove_device
>  *  - XenBus state has been disconnected (guest shutdown). See 
> xen_pcibk_xenbus_remove

But this, in case of HVM without stubdomain, is not.

>  *  - 'echo BDF > unbind' on pciback module with no guest attached. See 
> pcistub_remove
>  *  - 'echo BDF > unbind' with a guest still using it. See pcistub_remove 
>   
>  *
>   
>  *  As such we have to be careful.
>   
>  *
>   
>  *  To make this easier, the caller has to hold the device lock.  
>   
>  */   
>   
> void pcistub_put_pci_dev(struct pci_dev *dev)
> 
> The first two are done when XenStore 'pci' entries are active - which
> this patch will remove and introduce a potential security problem.
> 
> Unless libxl does an 'unbind' followed by an 'bind'?

What about libxl__device_pci_reset, which is called (at least) before
attaching device to some domain, even after my patch and even if the
device is already bound to pciback. It tries to reset the device using
'reset' entry in sysfs. I see this isn't available for some devices -
can pci_reset_function do any better?


> 
> > 
> > > When HVM domain has device model in stubdomain, attaching xen-pciback to
> > > the target domain itself is not only useless, but also may prevent
> > > attaching xen-pciback to the stubdomain, effectively breaking PCI
> > > passthrough.
> > > 
> > > Signed-off-by: Marek Marczykowski-Górecki 
> > > <marma...@invisiblethingslab.com>
> > > ---
> > >  tools/libxl/libxl_pci.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > > index 6f8f49c..2ae1bc4 100644
> > > --- a/tools/libxl/libxl_pci.c
> > > +++ b/tools/libxl/libxl_pci.c
> > > @@ -,7 +,7 @@ out:
> > >  }
> > >  }
> > >  
> > > -if (!starting)
> > > +if (!starting && !hvm)
> > >  rc = libxl__device_pci_add_xenstore(gc, domid, pcidev, starting);
> > >  else
> > >  rc = 0;
> > > @@ -1306,7 +1306,8 @@ static void libxl__add_pcidevs(libxl__egc *egc, 
> > > libxl__ao *ao, uint32_t domid,
> > >  }
> > >  }
> > >  
> > > -if (d_config->num_pcidevs > 0) {
> > > +if (d_config->num_pcidevs > 0
> > > +&& d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV) {
> > 
> > Please move the indentation forward.
> > 
> > >  rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
> > >  d_config->num_pcidevs);
> > >  if (rc < 0) {
> > > -- 
> > > 2.5.5
> > > 
> > 
> > ___
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > https://lists.xen.org/xen-devel

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 3/3] libxl: don't try to manipulate json config for stubdomain

[Marek Marczykowski-Górecki]

On Wed, Oct 19, 2016 at 10:38:04AM +0100, Wei Liu wrote:
> On Tue, Oct 18, 2016 at 03:53:33AM +0200, Marek Marczykowski-Górecki wrote:
> > Stubdomain do not have it's own config file - its configuration is
> > derived from target domains. Do not try to manipulate it when attaching
> > PCI device.
> > 
> > This bug prevented starting HVM with stubdomain and PCI passthrough
> > device attached.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> > ---
> >  tools/libxl/libxl_pci.c | 24 +++-
> >  1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > index 3805d30..5ad70c5 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -151,14 +151,18 @@ static int libxl__device_pci_add_xenstore(libxl__gc 
> > *gc, uint32_t domid, libxl_d
> >  GCNEW(device);
> >  libxl__device_from_pcidev(gc, domid, pcidev, device);
> >  
> > -lock = libxl__lock_domain_userdata(gc, domid);
> > -if (!lock) {
> > -rc = ERROR_LOCK_FAIL;
> > -goto out;
> > -}
> > +/* Stubdomain config is derived from its target domain, it doesn't have
> > +   its own file */
> > +if (!libxl_is_stubdom(CTX, domid, NULL)) {
> > +lock = libxl__lock_domain_userdata(gc, domid);
> > +if (!lock) {
> > +rc = ERROR_LOCK_FAIL;
> > +goto out;
> > +}
> >  
> 
> What makes PCI devices special with regard to other devices? In other
> words, do we need to make similar changes to other devices as well?

I think PCI is special because libxl doesn't bother to update stubdomain
configuration for other device types at all... If I try dynamic
attach of network device to a HVM (with stubdomain), it gets attached
_only_ to the target domain. So HVM without any PV drivers is out of
luck. Is it supposed to work with qemu in dom0?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/3] libxl: attach xen-pciback only to PV domains

[Marek Marczykowski-Górecki]

On Tue, Oct 18, 2016 at 04:52:29PM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Oct 18, 2016 at 03:53:31AM +0200, Marek Marczykowski-Górecki wrote:
> > HVM domains use IOMMU and device model assistance for communicating with
> > PCI devices, xen-pcifront/pciback is used only in PV domains.
> > When HVM domain has device model in stubdomain, attaching xen-pciback to
> > the target domain itself is not only useless, but also may prevent
> > attaching xen-pciback to the stubdomain, effectively breaking PCI
> > passthrough.
> 
> This has the consequence that the "reset" of the device that
> pciback does will no longer be done.
> 
> That is the FLR functionality will not be exercised anymore.

Are you sure about that? libxl__device_pci_add calls
libxl__device_pci_reset, regardless of my patch.

> > Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> > ---
> >  tools/libxl/libxl_pci.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > index 6f8f49c..2ae1bc4 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -,7 +,7 @@ out:
> >  }
> >  }
> >  
> > -if (!starting)
> > +if (!starting && !hvm)
> >  rc = libxl__device_pci_add_xenstore(gc, domid, pcidev, starting);
> >  else
> >  rc = 0;
> > @@ -1306,7 +1306,8 @@ static void libxl__add_pcidevs(libxl__egc *egc, 
> > libxl__ao *ao, uint32_t domid,
> >  }
> >  }
> >  
> > -if (d_config->num_pcidevs > 0) {
> > +if (d_config->num_pcidevs > 0
> > +&& d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV) {
> >  rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
> >      d_config->num_pcidevs);
> >  if (rc < 0) {
> > -- 
> > 2.5.5
> > 
> > 
> > ___
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > https://lists.xen.org/xen-devel

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/3] libxl: attach PCI device to qemu only after setting pciback/pcifront

[Marek Marczykowski-Górecki]

When qemu is running in stubdomain, handling "pci-ins" command will fail
if pcifront is not initialized already. Fix this by sending such command
only after confirming that pciback/front is running.

It is possible to handle this case in mini-os code, by delaying
"pci-ins" handling until pcifront_thread finishes devices discovery. But
the same problem most likely will apply to any other stubdomain
implementations when they come (Rumprun, Linux) - so better handle this
at libxl level.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_pci.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2ae1bc4..3805d30 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1195,6 +1195,7 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcide
 {
 libxl_ctx *ctx = libxl__gc_owner(gc);
 unsigned int orig_vdev, pfunc_mask;
+char *be_path;
 libxl_device_pci *assigned;
 int num_assigned, i, rc;
 int stubdomid = 0;
@@ -1249,6 +1250,14 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcide
 rc = do_pci_add(gc, stubdomid, _s, 0);
 if ( rc )
 goto out;
+/* wait for the device actually being connected, otherwise device model
+ * running there will fail to find the device */
+be_path = libxl__sprintf(gc, "%s/backend/pci/%d/0",
+libxl__xs_get_dompath(gc, 0), stubdomid);
+rc = libxl__wait_for_backend(gc, be_path,
+GCSPRINTF("%d", XenbusStateConnected));
+if ( rc )
+goto out;
 }
 
 orig_vdev = pcidev->vdevfn & ~7U;
-- 
2.5.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 0/3] Fix PCI passthrough for HVM with stubdomain

[Marek Marczykowski-Górecki]

This series is follow up to previous attempts to fix this. Related threads:
 - http://markmail.org/thread/dwjcdfk3y7s5c5kl "PCI passthrough with stubdomain"
 - http://xen.markmail.org/thread/l7tvqcxbiyc2grvr "Json config and stubdomain"

With those patches applied, HVM finally have PCI devices working. Tested on
Linux 4.4.14 and Window 7 pro guests (both 64bit).


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/3] libxl: attach xen-pciback only to PV domains

[Marek Marczykowski-Górecki]

HVM domains use IOMMU and device model assistance for communicating with
PCI devices, xen-pcifront/pciback is used only in PV domains.
When HVM domain has device model in stubdomain, attaching xen-pciback to
the target domain itself is not only useless, but also may prevent
attaching xen-pciback to the stubdomain, effectively breaking PCI
passthrough.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_pci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 6f8f49c..2ae1bc4 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -,7 +,7 @@ out:
 }
 }
 
-if (!starting)
+if (!starting && !hvm)
 rc = libxl__device_pci_add_xenstore(gc, domid, pcidev, starting);
 else
 rc = 0;
@@ -1306,7 +1306,8 @@ static void libxl__add_pcidevs(libxl__egc *egc, libxl__ao 
*ao, uint32_t domid,
 }
 }
 
-if (d_config->num_pcidevs > 0) {
+if (d_config->num_pcidevs > 0
+&& d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV) {
 rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
 d_config->num_pcidevs);
 if (rc < 0) {
-- 
2.5.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 3/3] libxl: don't try to manipulate json config for stubdomain

[Marek Marczykowski-Górecki]

Stubdomain do not have it's own config file - its configuration is
derived from target domains. Do not try to manipulate it when attaching
PCI device.

This bug prevented starting HVM with stubdomain and PCI passthrough
device attached.

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_pci.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 3805d30..5ad70c5 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -151,14 +151,18 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, 
uint32_t domid, libxl_d
 GCNEW(device);
 libxl__device_from_pcidev(gc, domid, pcidev, device);
 
-lock = libxl__lock_domain_userdata(gc, domid);
-if (!lock) {
-rc = ERROR_LOCK_FAIL;
-goto out;
-}
+/* Stubdomain config is derived from its target domain, it doesn't have
+   its own file */
+if (!libxl_is_stubdom(CTX, domid, NULL)) {
+lock = libxl__lock_domain_userdata(gc, domid);
+if (!lock) {
+rc = ERROR_LOCK_FAIL;
+goto out;
+}
 
-rc = libxl__get_domain_configuration(gc, domid, _config);
-if (rc) goto out;
+rc = libxl__get_domain_configuration(gc, domid, _config);
+if (rc) goto out;
+}
 
 DEVICE_ADD(pci, pcidevs, domid, _saved, COMPARE_PCI, _config);
 
@@ -169,8 +173,10 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc, 
uint32_t domid, libxl_d
 rc = libxl__xs_transaction_start(gc, );
 if (rc) goto out;
 
-rc = libxl__set_domain_configuration(gc, domid, _config);
-if (rc) goto out;
+if (lock) {
+rc = libxl__set_domain_configuration(gc, domid, _config);
+if (rc) goto out;
+}
 
 libxl__xs_writev(gc, t, be_path, libxl__xs_kvs_of_flexarray(gc, back));
 
-- 
2.5.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Json config and stubdomain

[Marek Marczykowski-Górecki]

On Mon, Sep 26, 2016 at 12:01:01PM +0100, Wei Liu wrote:
> On Sat, Sep 24, 2016 at 12:42:45PM +0200, Marek Marczykowski-Górecki wrote:
> > So, the question is, whether json config should be created and properly
> > maintained for stubdomain, or not created at all (assuming all the
> > configuration is already handled in the target domain json config)?
> 
> Stubdom configuration should be derived. It is considered internal
> details.

Ok, I'll adjust PCI handling accordingly.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] Json config and stubdomain

[Marek Marczykowski-Górecki]

Hi,

Debugging stubdomain continuation...
When stubdomain is started, it has no json config created. This results
in multiple problems:
 - impossible to dynamically attach additional devices
 - starting a domain with more than one PCI device fails

The second one is especially interesting because it look so much
inconsistent: libxl__device_pci_add_xenstore when called for the first
device, it immediately calls libxl__create_pci_backend and do not touch
json config at all. But when called for the next device, it loads json
config, adds the device there and save it. So it looks like the first
PCI device will never be saved into json config (when added
dynamically).
In case of stubdomain, libxl__device_pci_add_xenstore is called during
stubdomain startup, so defining two or more PCI devices means domain
startup fail.

So, the question is, whether json config should be created and properly
maintained for stubdomain, or not created at all (assuming all the
configuration is already handled in the target domain json config)?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough with stubdomain

[Marek Marczykowski-Górecki]

On Fri, Sep 23, 2016 at 11:00:42PM +0200, Samuel Thibault wrote:
> Marek Marczykowski-Górecki, on Fri 23 Sep 2016 20:56:43 +0200, wrote:
> > 1. How to do this? ;) I.e. what synchronization primitives are available
> > in mini-os? Just pthread_mutex_lock/unlock?
> 
> pthread_mutex_lock are nops :o) because we don't have pthread_create.
> But for mini-os itself there are synchronization primitives, yes:
> there are also semaphores (./include/semaphore.h) and waitqueues
> (include/wait.h).

Ok, will take a look at this. Thanks.

> > 2. Wouldn't the same problem be with other stubdomain implementations?
> > Like Linux + qemu-xen or rumprun + qemu-xen? At least in Linux case
> > pcifront driver will also needs some time for initialization...
> 
> Possibly, I don't know about them, they didn't exist at the time ;)

Actually they don't exist even now ;) But hopefully will, in the near
future...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough with stubdomain

[Marek Marczykowski-Górecki]

On Fri, Sep 23, 2016 at 04:51:33PM +0200, Samuel Thibault wrote:
> Marek Marczykowski-Górecki, on Fri 23 Sep 2016 16:25:41 +0200, wrote:
> > Toolstack during (stub)domain startup setup two things, mostly
> > asynchronously:
> > 1. pciback/pcifront (through standard xenstore entries)
> > 2. instruct qemu to attach device (by writing "pci-ins" to
> > device-model/xxx/command)
> 
> Ah, since pcifront_watches runs in a separate thread, it may not have
> called init_pcifront before qemu calls libpci, i.e. pcifront_scan.

Yes, exactly.

> I'd say that's where the fix should be: make pcifront_scan wait for
> init_pcifront to be done.  It's however not so simple: we want to make
> pcifront_scan do nothing if no pciback is to be launched. My memory is
> rusty: is there something we are sure will show up in the xenstore when
> a pciback is to be launched?  If so, pcifront_scan could do this: wait
> for pcifront_watches to have checked for the potential presence of
> pciback. If pciback is not to be started, just do nothing; if pciback is
> to be started, wait for init_pcifront to have been called by
> pcifront_watches. Then it will find the devices.

Ok. So, two questions:
1. How to do this? ;) I.e. what synchronization primitives are available
in mini-os? Just pthread_mutex_lock/unlock?

2. Wouldn't the same problem be with other stubdomain implementations?
Like Linux + qemu-xen or rumprun + qemu-xen? At least in Linux case
pcifront driver will also needs some time for initialization...

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough with stubdomain

[Marek Marczykowski-Górecki]

On Fri, Sep 23, 2016 at 11:35:27AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 23, 2016 at 04:25:41PM +0200, Marek Marczykowski-Górecki wrote:
> > On Fri, Sep 23, 2016 at 03:27:07PM +0200, Samuel Thibault wrote:
> > > Marek Marczykowski-Górecki, on Fri 23 Sep 2016 10:48:14 +0200, wrote:
> > > > I'm still trying to get PCI passthrough working with stubdomain and
> > > > without qemu in dom0 (even for just vfb/vkbd backends). How is this
> > > > supposed to work?
> > > 
> > > Just as I recall from my memory:
> > > 
> > > > 1. Should xen-pcifront in the target domain be used (and consequently,
> > > > should xen-pciback be set for it)?
> > > 
> > > I guess that could work.
> > 
> > Could, or should? ;)
> > 
> > In the meantime, I've found this in xen-pcifront driver:
> > 
> > static int __init pcifront_init(void)
> > {
> > if (!xen_pv_domain() || xen_initial_domain())
> >     return -ENODEV;
> > 
> > So, it looks like pcifront is not used in PV domain.
> 
> You mean in HVM domains.

Of course.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] PCI passthrough with stubdomain

[Marek Marczykowski-Górecki]

On Fri, Sep 23, 2016 at 03:27:07PM +0200, Samuel Thibault wrote:
> Marek Marczykowski-Górecki, on Fri 23 Sep 2016 10:48:14 +0200, wrote:
> > I'm still trying to get PCI passthrough working with stubdomain and
> > without qemu in dom0 (even for just vfb/vkbd backends). How is this
> > supposed to work?
> 
> Just as I recall from my memory:
> 
> > 1. Should xen-pcifront in the target domain be used (and consequently,
> > should xen-pciback be set for it)?
> 
> I guess that could work.

Could, or should? ;)

In the meantime, I've found this in xen-pcifront driver:

static int __init pcifront_init(void)
{
if (!xen_pv_domain() || xen_initial_domain())
return -ENODEV;

So, it looks like pcifront is not used in PV domain.

> > Currently xen-pciback is set for both
> > stubdomain and target domain, which presents a race condition and
> > xen-pciback refuses to setup one of them.
> 
> Yes, that's expected, for the reason you say.

In the meantime I've tried to modify libxl to not setup pciback for
target domain. This, along with other modifications (see below) improved
the situation.

> * to summarize
> **
>
> If running PV drivers in the guest, you set the pciback on the guest, be
> it run with stubdom or not. 
> If running plain drivers in the guest,
>   * when not using a stubdom you don't need to set a pciback.
>   * when using a stubdom you need to set a pciback on the stubdom.

Thanks for the explanation!

> So the unfortunate thing is that when using stubdom, you'd have to set
> the pciback either on the guest (to run a PV driver in it), or on the
> stubdom (to run a plain driver in the guest, and let mini-os use PV to
> let qemu pass the board through)

I've tried only on Linux HVM guest and as noted above xen-pcifront
doesn't look to be involved. Is it any different on other OSes?

As for getting PCI passthrough working with stubdomain, for now I hit
those problems:
1. getdomaininfo not allowed from stubdomain (already fixed in master),
2. double setup of pciback (explained above) - for now I've disabled one
of them,
3. race condition between pcifront in mini-os and qemu.

Regarding the last one:
Toolstack during (stub)domain startup setup two things, mostly
asynchronously:
1. pciback/pcifront (through standard xenstore entries)
2. instruct qemu to attach device (by writing "pci-ins" to
device-model/xxx/command)

The later fails if pcifront is not initialized yet. For now I've moved
the second step to be really after the first one, including
synchronization through libxl__wait_for_backend call. 
Is it ok? Or maybe it should be done on stubdomain side (handling
"pci-ins" should wait on pcifront)? I guess the same will apply to
qemu-xen case, or any other stubdomain, so probably better have it in
toolstack, right?

Work-in-progress patches attached. The result is that lspci inside the
guest finally list the device. No idea if it's really working yet.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
From b8a34904411cc6e7a7abdc6296657ff5f3eb92e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marma...@invisiblethingslab.com>
Date: Thu, 22 Sep 2016 16:07:14 +0200
Subject: [PATCH 1/2] libxl: attach xen-pciback only to stubdomain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Organization: Invisible Things Lab
Cc: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>

Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_create.c | 2 +-
 tools/libxl/libxl_pci.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index c6862b8..5625ef2 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1429,7 +1429,7 @@ static void domcreate_attach_pci(libxl__egc *egc, 
libxl__multidev *multidev,
 }
 }
 
-if (d_config->num_pcidevs > 0) {
+if (d_config->num_pcidevs > 0 && d_config->c_info.type == 
LIBXL_DOMAIN_TYPE_PV) {
 ret = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
 d_config->num_pcidevs);
 if (ret < 0) {
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 19c597e..2942772 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1001,7 +1001,7 @@ out:
 }
 }
 
-if (!starting)
+if (!starting && !hvm)
 rc = libxl__device_pci_add_xenstore(gc, domid, pcidev, starting);
 else
 rc = 0;
-- 
2.5.5

From 930398583d5a590c511e58057d000e1ed7defb82 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykows

[Xen-devel] PCI passthrough with stubdomain

[Marek Marczykowski-Górecki]

Hi,

I'm still trying to get PCI passthrough working with stubdomain and
without qemu in dom0 (even for just vfb/vkbd backends). How is this
supposed to work?

1. Should xen-pcifront in the target domain be used (and consequently,
should xen-pciback be set for it)? Currently xen-pciback is set for both
stubdomain and target domain, which presents a race condition and
xen-pciback refuses to setup one of them.
(I have a patch for this, but not sure how it really should work)

1a. How does it look in case of qemu in dom0 (no stubdomain)?

2. What operations are done through stubdomain and what are handled
directly by Xen (itself, or with help of IOMMU)? I'd guess config space
accesses are done through device model. Anything else?

3. What changes (if any) are required in qemu-xen to have it working in
stubdomain in regards to PCI passthrough? Should it just work (assuming
Linux-based stubdomain with xen-pcifront driver)?

PS If anyone interested, here are more details:
https://github.com/QubesOS/qubes-issues/issues/1659

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH for-4.7, 4.6] libxl: do not assume Dom0 backend while getting nic info

[Marek Marczykowski-Górecki]

Fill backend_domid field based on backend path.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index e1ab6ec..9a888a1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3601,6 +3601,18 @@ static int libxl__device_nic_from_xenstore(libxl__gc *gc,
 else
 nic->devid = 0;
 
+rc = libxl__xs_read_checked(gc, XBT_NULL,
+GCSPRINTF("%s/backend", libxl_path), );
+if (rc) goto out;
+
+if (!tmp) {
+LOG(ERROR, "nic %s does not exist (no backend path)", libxl_path);
+rc = ERROR_FAIL;
+goto out;
+}
+rc = libxl__backendpath_parse_domid(gc, tmp, >backend_domid);
+if (rc) goto out;
+
 /* nic->mtu = */
 
 tmp = READ_LIBXLDEV(gc, "mac");
-- 
2.5.5


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxl: do not assume Dom0 backend while getting nic info

[Marek Marczykowski-Górecki]

On Mon, Sep 05, 2016 at 10:39:16AM +0100, Wei Liu wrote:
> On Mon, Sep 05, 2016 at 11:26:04AM +0200, Marek Marczykowski-Górecki wrote:
> > Fill backend_domid field based on backend path.
> > 
> > Cc: Ian Jackson <ian.jack...@eu.citrix.com>
> > Cc: Wei Liu <wei.l...@citrix.com>
> > Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> 
> Acked-by: Wei Liu <wei.l...@citrix.com>
> 
> I think this is a backport candidate?

Yes, certainly. If you want I can send a 4.7 version (function is in
libxl.c there).

In general, should I somehow somehow request a backport in the patch
itself? How?

> > ---
> >  tools/libxl/libxl_nic.c | 12 
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
> > index c34b7ba..d1caa90 100644
> > --- a/tools/libxl/libxl_nic.c
> > +++ b/tools/libxl/libxl_nic.c
> > @@ -309,6 +309,18 @@ static int libxl__device_nic_from_xenstore(libxl__gc 
> > *gc,
> >  else
> >  nic->devid = 0;
> >  
> > +rc = libxl__xs_read_checked(gc, XBT_NULL,
> > +GCSPRINTF("%s/backend", libxl_path), );
> > +if (rc) goto out;
> > +
> > +if (!tmp) {
> > +LOG(ERROR, "nic %s does not exist (no backend path)", libxl_path);
> > +rc = ERROR_FAIL;
> > +goto out;
> > +}
> > +rc = libxl__backendpath_parse_domid(gc, tmp, >backend_domid);
> > +if (rc) goto out;
> > +
> >  /* nic->mtu = */
> >  
> >  rc = libxl__xs_read_checked(gc, XBT_NULL,
> > -- 
> > 2.5.5
> > 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] libxl: do not assume Dom0 backend while getting nic info

[Marek Marczykowski-Górecki]

Fill backend_domid field based on backend path.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/libxl/libxl_nic.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/tools/libxl/libxl_nic.c b/tools/libxl/libxl_nic.c
index c34b7ba..d1caa90 100644
--- a/tools/libxl/libxl_nic.c
+++ b/tools/libxl/libxl_nic.c
@@ -309,6 +309,18 @@ static int libxl__device_nic_from_xenstore(libxl__gc *gc,
 else
 nic->devid = 0;
 
+rc = libxl__xs_read_checked(gc, XBT_NULL,
+GCSPRINTF("%s/backend", libxl_path), );
+if (rc) goto out;
+
+if (!tmp) {
+LOG(ERROR, "nic %s does not exist (no backend path)", libxl_path);
+rc = ERROR_FAIL;
+goto out;
+}
+rc = libxl__backendpath_parse_domid(gc, tmp, >backend_domid);
+if (rc) goto out;
+
 /* nic->mtu = */
 
 rc = libxl__xs_read_checked(gc, XBT_NULL,
-- 
2.5.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH] libxl: do not assume Dom0 backend while getting nic info

[Marek Marczykowski-Górecki]

Fill backend_domid field based on backend path.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] PVH guess crash with memory < maxmem

[Marek Marczykowski-Górecki]

On Mon, Aug 08, 2016 at 05:38:56AM -0600, Jan Beulich wrote:
> Indeed iirc PVHv1 and PoD are incompatible with one another,
> and I don't know any possible workaround. And then I'm not
> sure it's worth your time playing with PVHv1 in the first place.

Thanks.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] PVH hypercall interface

[Marek Marczykowski-Górecki]

On Mon, Aug 08, 2016 at 01:19:43PM +0200, Roger Pau Monne wrote:
> On Mon, Aug 08, 2016 at 11:42:03AM +0100, Andrew Cooper wrote:
> > On 08/08/16 11:21, Marek Marczykowski-Górecki wrote:
> > > Hi,
> > >
> > > What hypercalls are available for PVH guests?
> > 
> > First of all, be aware that there is currently some chaos in the
> > codebase which is soon to be resolved.
> > 
> > What you are looking for is HVMLite, which will likely be renamed to PVH
> > once it is complete.  Original PVH will then be filed in /dev/null,
> > leaving only a set of lessons in git history.
> > 
> > > How is it different for HVM guests?
> > 
> > HVMLite guests are just HVM guests without qemu.
> > 
> > Roger (CC'd) is leading the effort.  You select an HVMLite guest by
> > choosing device-model = None in the xl configuration file, rather than
> > setting pvh=1
> > 
> > > Is it documented somewhere?
> > 
> > There are patches on the list.
> > 
> > Also, docs/misc/hvmlite.markdown
> > 
> > > For example I'd expect that do_mmu_update is available only for PV
> > > guests, but looking at the code I can't find anything preventing other
> > > guest types from using it (no, some obscure conditions deep in execution
> > > path doesn't count).
> > 
> > HVM guests use the hvm_hypercall_table, not the hypercall tables in entry.S
> 
> They have access to the same set of hypercalls as HVM guests, in fact the 
> hypercall table is shared between HVM and HVMlite guests (to become PVH, 
> just using HVMlite here to avoid confusion). Classic PVH guests had their 
> own hypercall table, but as Andrew said you should not look at those.

Thanks for the explanation. What is the current state of HVMlite? I see
"none" is valid value for "device_model_version" already, but if I try
to start such guest, it fails at x86_compat call (tries to boot it as PV
guest). I guess it's because of missing kernel support. Is "[PATCH v2
00/11] HVMlite domU support" sent in Feb the latest version available?
When it is planned to have it in vanilla kernel?


-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] PVH guess crash with memory < maxmem

[Marek Marczykowski-Górecki]

Hi,

I'm having troubles with starting PVH guest when memory < maxmem. Exact
crash message:

(XEN) d2v0 EPT violation 0x182 (-w-/---) gpa 0x003e7ff000 
mfn 0x type 4
(XEN) d2v0 Walking EPT tables for GFN 3e7ff:
(XEN) d2v0  gfn exceeds max_mapped_pfn 19000
(XEN) d2v0  --- GLA 0x88003e7ff000
(XEN) domain_crash called from vmx.c:3022
(XEN) Domain 2 (vcpu#0) crashed on cpu#2:
(XEN) [ Xen-4.7.0  x86_64  debug=n  Not tainted ]
(XEN) CPU:2
(XEN) RIP:0010:[]
(XEN) RFLAGS: 00010016   CONTEXT: hvm guest (d2v0)
(XEN) rax:    rbx: 0001   rcx: 003f
(XEN) rdx: 3e7ff000   rsi: 02b96000   rdi: 88003e7ff000
(XEN) rbp: 81c03b68   rsp: 81c03b40   r8: 0200
(XEN) r9:     r10: 02b96000   r11: 
(XEN) r12: 0001   r13: 0003e7ff   r14: 8800
(XEN) r15: 880001c10828   cr0: 80050033   cr4: 0020
(XEN) cr3: 01c0e000   cr2: 
(XEN) ds:    es:    fs:    gs:    ss:    cs: 0010
(XEN) Guest stack trace from rsp=81c03b40:
(XEN)   Fault while accessing guest memory.

Guest kernel: 4.1.13, guest config attached.

Is there any way around it, or such configuration isn't supported yet?

Or maybe it is possible to increase maxmem later using some trick
(memory hotplug or something)?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
name = "pvhtest"
uuid = "ac823572-853b-4c8c-ba77-47aa515ccd63"
maxmem = 400
memory = 1000
pvh = 1
vcpus = 4
localtime = 0
on_poweroff = "destroy"
on_reboot = "destroy"
on_crash = "destroy"
kernel = "/var/lib/qubes/vm-kernels/4.1.13-6/vmlinuz"
ramdisk = "/var/lib/qubes/vm-kernels/4.1.13-6/initramfs"
extra = "root=/dev/mapper/dmroot ro nomodeset console=hvc0 rd_NO_PLYMOUTH 3 
nopat"
disk = [ 
"/var/lib/qubes/vm-templates/fedora-23-clone/root.img,raw,xvda,r,backendtype=phy",
 "/var/lib/qubes/appvms/pvhtest/private.img,raw,xvdb,w,backendtype=phy",
"/var/lib/qubes/appvms/pvhtest/volatile.img,raw,xvdc,w,backendtype=phy",

"/var/lib/qubes/vm-kernels/4.1.13-6/modules.img,raw,xvdd,r,backendtype=phy" ]



signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] PVH hypercall interface

[Marek Marczykowski-Górecki]

Hi,

What hypercalls are available for PVH guests? How is it different for
HVM guests? Is it documented somewhere?
For example I'd expect that do_mmu_update is available only for PV
guests, but looking at the code I can't find anything preventing other
guest types from using it (no, some obscure conditions deep in execution
path doesn't count).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] systemd: drop dependency on proc-xen.mount in xendriverdomain.service

[Marek Marczykowski-Górecki]

On Mon, Jul 25, 2016 at 10:34:21AM +0100, Wei Liu wrote:
> On Mon, Jul 25, 2016 at 11:30:32AM +0200, Marek Marczykowski-Górecki wrote:
> > On Mon, Jul 25, 2016 at 10:27:47AM +0100, Wei Liu wrote:
> > > On Sun, Jul 24, 2016 at 09:27:13PM +0200, Marek Marczykowski-Górecki 
> > > wrote:
> > > > It is no longer required since xl devd use /dev/xen interface.
> > > > 
> > > 
> > > How would this unit work when there is no /dev/xen interface?
> > 
> > Does it happen in reality? I thought /proc/xen is deprecated for a long
> > time...
> > 
> 
> I can't tell whether it happens in reality or not. I also hope to get
> rid of deprecated interface but they are here to stay for as long as it
> takes.
> 
> I guess the question is, do you have a compelling reason against keeping
> this dependency?

Just cleaning up old stuff, so - no.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] systemd: drop dependency on proc-xen.mount in xendriverdomain.service

[Marek Marczykowski-Górecki]

On Mon, Jul 25, 2016 at 10:27:47AM +0100, Wei Liu wrote:
> On Sun, Jul 24, 2016 at 09:27:13PM +0200, Marek Marczykowski-Górecki wrote:
> > It is no longer required since xl devd use /dev/xen interface.
> > 
> 
> How would this unit work when there is no /dev/xen interface?

Does it happen in reality? I thought /proc/xen is deprecated for a long
time...

> To be precise, we prefer /dev/xen interfaces whenever possible but there
> is a fallback to /proc/xen. Note that a lot of other unit files have
> this dependency on proc-xen.mount.
> 
> I'm inclined to say we should keep this dependency but I'm not sure if I
> missed some obvious things.
> 
> > Cc: Ian Jackson <ian.jack...@eu.citrix.com>
> > Cc: Wei Liu <wei.l...@citrix.com>
> > Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
> > ---
> >  tools/hotplug/Linux/systemd/xendriverdomain.service.in | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/tools/hotplug/Linux/systemd/xendriverdomain.service.in 
> > b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
> > index 0afb54d..a100309 100644
> > --- a/tools/hotplug/Linux/systemd/xendriverdomain.service.in
> > +++ b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
> > @@ -1,7 +1,5 @@
> >  [Unit]
> >  Description=Xen driver domain device daemon
> > -Requires=proc-xen.mount
> > -After=proc-xen.mount
> >  ConditionVirtualization=xen
> >  
> >  [Service]
> > -- 
> > 2.5.5
> 
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/2] systemd: use standard dependencies for xendriverdomain.service

[Marek Marczykowski-Górecki]

Having DefaultDependencies=no means it can be started before / is
remounted read-write, which will result in various failures (to start
with opening the log).
Since "libxl: trigger attach events for devices attached before xl devd
startup" it is no longer important to start it as early as possible,
because it will process devices created before its startup.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/hotplug/Linux/systemd/xendriverdomain.service.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/hotplug/Linux/systemd/xendriverdomain.service.in 
b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
index c0cd454..0afb54d 100644
--- a/tools/hotplug/Linux/systemd/xendriverdomain.service.in
+++ b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
@@ -1,6 +1,5 @@
 [Unit]
 Description=Xen driver domain device daemon
-DefaultDependencies=no
 Requires=proc-xen.mount
 After=proc-xen.mount
 ConditionVirtualization=xen
-- 
2.5.5


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/2] systemd: drop dependency on proc-xen.mount in xendriverdomain.service

[Marek Marczykowski-Górecki]

It is no longer required since xl devd use /dev/xen interface.

Cc: Ian Jackson <ian.jack...@eu.citrix.com>
Cc: Wei Liu <wei.l...@citrix.com>
Signed-off-by: Marek Marczykowski-Górecki <marma...@invisiblethingslab.com>
---
 tools/hotplug/Linux/systemd/xendriverdomain.service.in | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/hotplug/Linux/systemd/xendriverdomain.service.in 
b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
index 0afb54d..a100309 100644
--- a/tools/hotplug/Linux/systemd/xendriverdomain.service.in
+++ b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
@@ -1,7 +1,5 @@
 [Unit]
 Description=Xen driver domain device daemon
-Requires=proc-xen.mount
-After=proc-xen.mount
 ConditionVirtualization=xen
 
 [Service]
-- 
2.5.5


signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] "X86_PV_VCPU_MSRS record truncated" during domain restore (was: Re: [qubes-users] DispVM Doesn't work)

[Marek Marczykowski-Górecki]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On Wed, Jul 20, 2016 at 02:33:20PM +0200, Massimo Colombi wrote:
> I retried (it's not the first time) to regenerate new savefile, but DispVM
> doesn't work.
> I attach the results.

For me it looks like a bug in savefile handling. Moving to xen-devel. Any
idea?
Background info: it's about restoring a domain through libvirt->libxl
(virsh restore equivalent). Xen 4.6.1. Full error:

> 2016-07-20 14:23:01 CEST xc: error: X86_PV_VCPU_MSRS record truncated: length 
> 8, min 9: Internal error
> 2016-07-20 14:23:01 CEST xc: error: Restore failed (0 = Success): Internal 
> error
> 2016-07-20 14:23:01 CEST libxl: error: 
> libxl_stream_read.c:749:libxl__xc_domain_restore_done: restoring domain: 
> Successo
> 2016-07-20 14:23:01 CEST libxl: error: 
> libxl_create.c:1145:domcreate_rebuild_done: cannot (re-)build domain: -3


If relevant, here is fragment of /proc/cpuinfo (just one core):
processor   : 0
vendor_id   : AuthenticAMD
cpu family  : 22
model   : 48
model name  : AMD A8-6410 APU with AMD Radeon R5 Graphics
stepping: 1
microcode   : 0x7030105
cpu MHz : 1996.290
cache size  : 2048 KB
physical id : 0
siblings: 4
core id : 0
cpu cores   : 4
apicid  : 0
initial apicid  : 0
fpu : yes
fpu_exception   : yes
cpuid level : 13
wp  : yes
flags   : fpu de tsc msr pae mce cx8 apic mca cmov pat clflush
mmx fxsr sse sse2 ht
syscall nx mmxext fxsr_opt lm constant_tsc rep_good nopl nonstop_tsc
extd_apicid eagerfpu pni
pclmulqdq ssse3 cx16 sse4_1 sse4_2 movbe popcnt aes xsave avx f16c
rdrand hypervisor lahf_lm
cmp_legacy extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch
perfctr_nb bpext perfctr_l2
arat cpb hw_pstate vmmcall bmi1 xsaveopt
bugs: fxsave_leak
bogomips: 3992.58
TLB size: 1024 4K pages
clflush size: 64
cache_alignment : 64
address sizes   : 40 bits physical, 48 bits virtual
power management: ts ttp tm 100mhzsteps hwpstate cpb [12] [13]


> Best regards,
>  Massimo
> 
> On 07/20/2016 01:37 PM, Marek Marczykowski-Górecki wrote:
> > Try `qvm-create-default-dvm --default-template` to regenerate new savefile.
> > It looks like your savefile is somehow broken.
> 

> [user@dom0 logs]$ qvm-create-default-dvm --default-template
> --> Creating volatile image: 
> /var/lib/qubes/appvms/fedora-23-dvm/volatile.img...
> --> Loading the VM (type = AppVM)...
> --> Starting Qubes DB...
> --> Setting Qubes DB info for the VM...
> --> Updating firewall rules...
> --> Starting the VM...
> --> Starting Qubes GUId...
> Connecting to VM's GUI agent: ..connected
> Waiting for DVM fedora-23-dvm ...
> /qubes-used-mem
> Disco scollegato correttamente
> 
> DVM boot complete, memory used=303380. Saving image...
> 
> 
> 
> Domain fedora-23-dvm saved to /var/lib/qubes/appvms/fedora-23-dvm/dvm-savefile
> 
> DVM savefile created successfully.
> [user@dom0 logs]$ echo xterm | /usr/lib/qubes/qfile-daemon-dvm qubes.VMShell 
> dom0 DEFAULT red
> time=1469017378.68, qfile-daemon-dvm init
> time=1469017378.92, creating DispVM
> time=1469017380.48, collection loaded
> time=1469017380.49, VM created
> time=1469017380.59, VM starting
> time=1469017380.6, creating config file
> time=1469017380.86, calling restore
> Traceback (most recent call last):
>   File "/usr/lib/qubes/qfile-daemon-dvm", line 200, in 
> main()
>   File "/usr/lib/qubes/qfile-daemon-dvm", line 188, in main
> dispvm = qfile.get_dvm()
>   File "/usr/lib/qubes/qfile-daemon-dvm", line 150, in get_dvm
> return self.do_get_dvm()
>   File "/usr/lib/qubes/qfile-daemon-dvm", line 103, in do_get_dvm
> dispvm.start()
>   File 
> "/usr/lib64/python2.7/site-packages/qubes/modules/01QubesDisposableVm.py", 
> line 193, in start
> domain_config, libvirt.VIR_DOMAIN_SAVE_PAUSED)
>   File "/usr/lib64/python2.7/site-packages/libvirt.py", line 4405, in 
> restoreFlags
> if ret == -1: raise libvirtError ('virDomainRestoreFlags() failed', 
> conn=self)
> libvirt.libvirtError: internal error: libxenlight failed to restore domain 
> 'disp1'
> 


- -- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iQEcBAEBCAAGBQJXkActAAoJENuP0xzK19csHZ4IAJF35BwBKbKzNFo0yOcTJxng
2NqVIUmHg3hgVZuNdkoNm09L7f5yIUai+0AqsFvM8BbPmwC9C6EBcu7FICMub/lT
NUbfyf4rW5YRuEVG+OB7+Zge4mz3++kb1cLqobn2vA5Z9ayCDW32Yq5yYFff9zd7
/qMtjuj35oG25fKEs1PIZbDtMkdnq2ef1rg7KDj695SpDSt0g3AadtTjnJVh7f2V
v7K6aUKR6O2xcHWADWhqxLNaKEBA8cd2yHeGYmbQwD9cICqQNVFwH/jIHWsAqGun
d/IKvRetZnLajFm42X7862UIgQWE1qTpqDXV6OCWT/KNh/cL4SLwn+0RW8RbfSQ=
=gdks
-END PGP SIGNATURE-

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel