Re: [Xen-devel] [PATCH 14/16] SUPPORT.md: Add statement on PCI passthrough
On Mon, Nov 13, 2017 at 03:41:24PM +, George Dunlap wrote: > Signed-off-by: George Dunlap > --- > CC: Ian Jackson > CC: Wei Liu > CC: Andrew Cooper > CC: Jan Beulich > CC: Stefano Stabellini > CC: Konrad Wilk > CC: Tim Deegan > CC: Rich Persaud > CC: Marek Marczykowski-Górecki > CC: Christopher Clark > CC: James McKenzie > --- > 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
On Mon, Oct 02, 2017 at 12:49:24PM +0300, Oleksandr Grytsov wrote: > +=item B > + > +Short name of the virtual sound card. > + > +=item B > + > +Long name of the virtual sound card. > + > +=back Duplicate short-name=, should be name= ? -- 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] python/libxc: extend the call to get/set cap for credit2
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 Acked-by: Marek Marczykowski-Górecki > --- > Cc: George Dunlap > Cc: Dario Faggioli > Cc: Marek Marczykowski-Górecki > > 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, > - &domid, &weight) ) > + &domid, &weight, &cap) ) > return NULL; > > sdom.weight = weight; > +sdom.cap = cap; > > if ( xc_sched_credit2_domain_set(self->xc_handle, domid, &sdom) != 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", &domid) ) > 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, &sdom) != 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()
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 > Reviewed-by: Konrad Rzeszutek Wilk > Reviewed-by: Wei Liu Acked-by: Marek Marczykowski-Górecki > --- > 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()
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 > Reviewed-by: Wei Liu Acked-by: Marek Marczykowski-Górecki > --- > 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()
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 > Reviewed-by: Wei Liu Acked-by: Marek Marczykowski-Górecki > --- > 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 *)&token) != 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 *)&token) != 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()
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 > Reviewed-by: Konrad Rzeszutek Wilk > Reviewed-by: Wei Liu > --- > 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
On Mon, Sep 11, 2017 at 03:06:48AM -0600, Jan Beulich wrote: > >>> On 10.09.17 at 01:53, 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
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
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 > > --- > > Cc: Ian Jackson > > Cc: Wei Liu > > 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 > > --- > > 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) > > { > > int rc; > > xen_pfn_t scratch_gpfn; > > @@ -380,7 +380,7 @@ int xc_dom_gnttab
Re: [Xen-devel] [PATCH v2 1/2] libxl: use xen-blkback for 'vbd' disk types by default
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 > > > > > > > > > --- > > > This is extracted from v1 of "libxl: do not start dom0 qemu for > > > stubdomain when not needed". > > > > > > Signed-off-by: Marek Marczykowski-Górecki > > > > > > --- > > > 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", > > > &disk->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
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 --- 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, &dm_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 = &sdss->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, &dm_config->vfbs[0]); -if (ret) -goto out; -ret = libxl__device_vkb_add(gc, dm_domid, &dm_config->vkbs[0]); -if (ret) -goto out; +if (dm_config->num_vfbs) { +ret = libxl__device_vfb_add(gc, dm_domid, &dm_config->vfbs[0]); +if (ret) +goto out; +} +if (dm_config->num_vkbs) { +ret = libxl__device_vkb_add(gc, dm_domid, &dm_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, &console[i], i == STUBDOM_CONSOLE_LOGGING ? stubdom_state : NULL, &device); @@ -2032,7 +2051,12 @@ static void spawn_stub_launch_dm(libxl__egc *egc, sdss->pvqemu.build_state = &sdss->dm_state; sdss->pvqemu.callback = spawn_stubdom_pvqemu_cb; -libxl__spawn_local_dm(egc, &sdss->pvqemu); +if (!need_qemu) { +/* If dom0 qemu not needed, do not launch it */ +spawn_stubdom_pvqemu_cb(egc, &sdss->pvqemu, 0); +} else { +libxl__spawn_local_dm(egc, &sdss->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
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 --- This is extracted from v1 of "libxl: do not start dom0 qemu for stubdomain when not needed". Signed-off-by: Marek Marczykowski-Górecki --- 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", &disk->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
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
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, &dm_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
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 > > --- > > 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", > > &disk->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, &dm_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 = &sdss->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, &dm_config->vfbs[0]); > > -if (ret) > > -goto out; > > -ret = libxl__device_vkb_add(gc, dm_domid, &dm_config->vkbs[0]); > > -if (ret) > > -goto out; > > +if (dm_config->num_vfbs) { > > +ret = libxl__device_vfb_add(gc, dm_domid, &dm_config->vfbs[0]); > > +if (ret) > > +goto out; > > +} > > +if (dm_config->num_vkbs) { > > +ret = libxl__device_vkb_add(gc, dm_domid, &dm_config->vkbs[0]); > > +if (ret) > > +goto out; > > +} > > > > if (guest_config->b_info.
[Xen-devel] [PATCH] libxl: do not start dom0 qemu for stubdomain when not needed
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 --- 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", &disk->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, &dm_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 = &sdss->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, &dm_config->vfbs[0]); -if (ret) -goto out; -ret = libxl__device_vkb_add(gc, dm_domid, &dm_config->vkbs[0]); -if (ret) -goto out; +if (dm_config->num_vfbs) { +ret = libxl__device_vfb_add(gc, dm_domid, &dm_config->vfbs[0]); +if (ret) +goto out; +} +if (dm_config->num_vkbs) { +ret = libxl__device_vkb_add(gc, dm_domid, &dm_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_
[Xen-devel] [PATCH] libvchan: Fix cleanup when xc_gntshr_open failed
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 --- 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
On Mon, Jul 17, 2017 at 01:54:19PM +, Christian Lindig wrote: > > > On 17. Jul 2017, at 13:38, Andrew Cooper 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
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
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
It's bit 9 not 10 (which is ibs). Signed-off-by: Marek Marczykowski-Górecki --- 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
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 --- 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", 0x0007, 0, CPUID_REG_EBX, 30,
[Xen-devel] [PATCH v3 3/4] libxl: make cpuid_flags array static const
To have it in .rodata, instead of reconstructing each time on stack. Signed-off-by: Marek Marczykowski-Górecki --- 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 0/4] libxl: cpuid bits
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
[Xen-devel] [PATCH v3 4/4] libxl: reformat cpuid_flags
Reverse sorting order, add blank lines at register change. Signed-off-by: Marek Marczykowski-Górecki --- 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, CPUID_REG_ECX, 6, 1}, -{"vmx",
Re: [Xen-devel] [PATCH v2 0/2] libxl: cpuid bits
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 " 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
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 > > 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
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 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
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 --- 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", 0x0007, 0,
[Xen-devel] [PATCH v2 0/2] libxl: cpuid bits
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
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 --- 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
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
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 --- 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}, +{"a
[Xen-devel] [PATCH 2/2] libxl: fix osvm cpuid flag
It's bit 9 not 10 (which is ibs). Signed-off-by: Marek Marczykowski-Górecki --- 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
ext+0xbc73): 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 `TPM_Quote': gdtoa-hexnan.c:(.text+0xc015): undefined 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
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 --- 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(&hmac, (BYTE*)&rsp->auth2->continueAuthSession, 1); + tpm_hmac_final(&hmac, rsp->auth2->auth); ++ /* fall-thru */ + case TPM_TAG_RSP_AUTH1_COMMAND: + tpm_hmac_init(&hmac, rsp->auth1->secret, sizeof(rsp->auth1->secret)); + tpm_hmac_update(&hmac, 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
GCC 7 is too sensitive here. See https://bugs.debian.org/853710 Signed-off-by: Marek Marczykowski-Górecki --- 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
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
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 --- 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
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ß > > Cc: Andrew Cooper ; x...@kernel.org; linux- > > ker...@vger.kernel.org; sta...@vger.kernel.org; xen- > > de...@lists.xenproject.org; Boris Ostrovsky > > 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
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 --- 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" (&hypercall_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
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
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 --- 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" (&hypercall_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
[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
On Tue, Jun 06, 2017 at 04:55:20AM -0600, Jan Beulich wrote: > >>> On 06.06.17 at 12:41, 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 --- 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, .
Re: [Xen-devel] PCI passthrough of USB controllers on Xen 4.8.1, Linux 4.9.29, stubdomain
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
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é 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
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
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 s
[Xen-devel] (pv)?grub and PVHv2
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
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 Acked-by: Marek Marczykowski-Górecki > --- > Cc: Marek Marczykowski-Górecki > Cc: Ian Jackson > > $ get_maintainers.pl -f tools/python > "Marek Marczykowski-Górecki" > Ian Jackson > Wei Liu > 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 > S: Supported > F: xen/include/public/io/ > > +PYTHON BINDINGS > +M: Marek Marczykowski-Górecki > +S: Supported > +F: tools/python > + > QEMU-DM > M: Ian Jackson > 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
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 --- 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
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
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 --- 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, &xsval_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
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 --- 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 7/8] python: adjust module initalization for Python3
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 --- 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(&PyXcType) < 0) -return; +INITERROR; +#if PY_MAJOR_VERSION >= 3 +m = PyModule_Create(&xc_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(&xshandle_type) < 0) -return; +INITERROR; +#if PY_MAJOR_VERSION >= 3 +m = PyModule_Create(&xs_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(&xshandle_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
Py_TYPE works on both Python2 and Python3, while internals of PyObject_HEAD have changed. Signed-off-by: Marek Marczykowski-Górecki --- 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
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 --- 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 */ -xshandle_methods, /* tp_method
[Xen-devel] [PATCH 1/8] python: check return value of PyErr_NewException
Signed-off-by: Marek Marczykowski-Górecki --- 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(&xshandle_type); PyModule_AddObject(m, CLS, (PyObject *)&xshandle_type); -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 6/8] python: use PyLong_* for constructing 'int' type in Python3
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 --- 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 = PyInt_FromLong(meminfo
Re: [Xen-devel] Python 3 bindings
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
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
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
On Mon, Jan 16, 2017 at 05:17:59AM -0700, Jan Beulich wrote: > >>> On 14.01.17 at 03:52, 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
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 libxe
Re: [Xen-devel] xc_evtchn_status fails with EFAULT on HVM, the same on PV works
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" (&hypercall_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
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" (&hypercall_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
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
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(&status, arg, 1) != 0 ) > > ^ > > return -EFAULT; > > rc = evtchn_status(&status); > > if ( !rc && __copy_to_guest(arg, &status, 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(&status, arg, 1) != 0 ) > +unsigned int res = copy_from_guest(&status, 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(&status); > if ( !rc && __copy_to_guest(arg, &status, 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
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(&status, arg, 1) != 0 ) ^ return -EFAULT; rc = evtchn_status(&status); if ( !rc && __copy_to_guest(arg, &status, 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
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
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
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 > > > > > > --- > > > 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
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 > > --- > > 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
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 > > --- > > 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
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 --- 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, &pcidev_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 1/3] libxl: attach xen-pciback only to PV domains
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 --- 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 0/3] Fix PCI passthrough for HVM with stubdomain
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 3/3] libxl: don't try to manipulate json config for stubdomain
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 --- 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, &d_config); -if (rc) goto out; +rc = libxl__get_domain_configuration(gc, domid, &d_config); +if (rc) goto out; +} DEVICE_ADD(pci, pcidevs, domid, &pcidev_saved, COMPARE_PCI, &d_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, &t); if (rc) goto out; -rc = libxl__set_domain_configuration(gc, domid, &d_config); -if (rc) goto out; +if (lock) { +rc = libxl__set_domain_configuration(gc, domid, &d_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
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
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
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
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
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
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?= 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 Signed-off-by: Marek Marczykowski-Górecki --- 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=20Marczykowski-G=C3=B3recki?= Date: Fri, 23 Sep 2016 16:13:37 +0200 Su
[Xen-devel] PCI passthrough with stubdomain
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
Fill backend_domid field based on backend path. Cc: Ian Jackson Cc: Wei Liu Signed-off-by: Marek Marczykowski-Górecki --- 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), &tmp); +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, &nic->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
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 > > Cc: Wei Liu > > Signed-off-by: Marek Marczykowski-Górecki > > Acked-by: Wei Liu > > 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), &tmp); > > +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, &nic->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
Fill backend_domid field based on backend path. Cc: Ian Jackson Cc: Wei Liu Signed-off-by: Marek Marczykowski-Górecki --- 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), &tmp); +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, &nic->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
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
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
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
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
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
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
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 > > Cc: Wei Liu > > Signed-off-by: Marek Marczykowski-Górecki > > --- > > 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
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 Cc: Wei Liu Signed-off-by: Marek Marczykowski-Górecki --- 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
It is no longer required since xl devd use /dev/xen interface. Cc: Ian Jackson Cc: Wei Liu Signed-off-by: Marek Marczykowski-Górecki --- 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)
-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