Re: [Xen-devel] [PATCH 0/9] xen: build fixes with gcc5 and binutils 2.25.0

[Olaf Hering]

On Tue, Feb 09, Luis R. Rodriguez wrote:

> The error log from compiling the libSDL test is:
> /tmp/qemu-conf--5604-.c:1:17: fatal error: SDL.h: No such file or directory

This is just the configure test, it can be ignored.

Olaf

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

[Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()

[Harmandeep Kaur]

Avoid handling issue of the return value of xc_version() in many cases.

Coverity ID 1351217

Signed-off-by: Harmandeep Kaur 
---
 tools/libxl/libxl.c | 39 ++-
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2d18b8d..a939e51 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5267,42 +5267,63 @@ const libxl_version_info* 
libxl_get_version_info(libxl_ctx *ctx)
 xen_platform_parameters_t p_parms;
 xen_commandline_t xen_commandline;
 } u;
+int rc;
 long xen_version;
 libxl_version_info *info = &ctx->version_info;
 
 if (info->xen_version_extra != NULL)
 goto out;
 
-xen_version = xc_version(ctx->xch, XENVER_version, NULL);
+rc = xen_version = xc_version(ctx->xch, XENVER_version, NULL);
+if ( rc < 0 )
+goto out;
+
 info->xen_version_major = xen_version >> 16;
 info->xen_version_minor = xen_version & 0xFF;
+rc = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
+if ( rc < 0 )
+goto out;
 
-xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
 info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra);
+rc = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
+if ( rc < 0 )
+goto out;
 
-xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
 info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler);
 info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by);
 info->compile_domain = libxl__strdup(NOGC, u.xen_cc.compile_domain);
 info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date);
+rc = xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
+if ( rc < 0 )
+goto out;
 
-xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
 info->capabilities = libxl__strdup(NOGC, u.xen_caps);
+rc = xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
+if ( rc < 0 )
+goto out;
 
-xc_version(ctx->xch, XENVER_changeset, &u.xen_chgset);
 info->changeset = libxl__strdup(NOGC, u.xen_chgset);
+rc = xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms);
+if ( rc < 0 )
+goto out;
 
-xc_version(ctx->xch, XENVER_platform_parameters, &u.p_parms);
 info->virt_start = u.p_parms.virt_start;
+rc = info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
+if ( rc < 0 )
+goto out;
 
-info->pagesize = xc_version(ctx->xch, XENVER_pagesize, NULL);
+rc = xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
+if ( rc < 0 )
+goto out;
 
-xc_version(ctx->xch, XENVER_commandline, &u.xen_commandline);
 info->commandline = libxl__strdup(NOGC, u.xen_commandline);
 
  out:
 GC_FREE;
-return info;
+if ( rc < 0 )
+   return NULL;
+else
+   return info;
 }
 
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
-- 
2.5.0


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

[Xen-devel] [PATCH v2] libxc: fix leak of t_info in xc_tbuf_get_size()

[Harmandeep Kaur]

Avoid leaking the memory mapping of the trace buffer

Coverity ID 1351228

Signed-off-by: Harmandeep Kaur 
---
v2: call to unmapping function reduced to one from two
---
 tools/libxc/xc_tbuf.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_tbuf.c b/tools/libxc/xc_tbuf.c
index 695939a..d96cc67 100644
--- a/tools/libxc/xc_tbuf.c
+++ b/tools/libxc/xc_tbuf.c
@@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch, unsigned long *size)
 sysctl.u.tbuf_op.buffer_mfn);
 
 if ( t_info == NULL || t_info->tbuf_size == 0 )
-return -1;
+rc = -1;
+else
+   *size = t_info->tbuf_size;
 
-*size = t_info->tbuf_size;
+xenforeignmemory_unmap(xch->fmem, t_info, *size);
 
-return 0;
+return rc;
 }
 
 int xc_tbuf_enable(xc_interface *xch, unsigned long pages, unsigned long *mfn,
-- 
2.5.0


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

Re: [Xen-devel] [PATCH v2] libxc: fix leak of t_info in xc_tbuf_get_size()

[Dario Faggioli]

On Thu, 2016-02-11 at 14:02 +0530, Harmandeep Kaur wrote:
> Avoid leaking the memory mapping of the trace buffer
> 
> Coverity ID 1351228
> 
> Signed-off-by: Harmandeep Kaur 
>
Reviewed-by: Dario Faggioli 

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [xen-4.6-testing test] 81632: tolerable FAIL - PUSHED

[osstest service owner]

flight 81632 xen-4.6-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/81632/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 78701
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 78701
 test-armhf-armhf-xl-rtds 11 guest-start  fail   like 78701

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  d77bac5c064ffb9dbb5b89b55b89853f1b784ebf
baseline version:
 xen  19fc53a92312876761659e82a1ae5d69b603a4ef

Last test of basis78701  2016-01-21 17:20:37 Z   20 days
Testing same since81632  2016-02-09 15:11:54 Z1 days1 attempts


People who touched revisions under test:
  Jan Beulich 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm pass

[Xen-devel] [PATCH v2] tools: libxl: make it illegal to pass libxl__realloc(gc) a non-gc ptr

[Ian Campbell]

That is, if gc is not NOGC and ptr is not NULL then ptr must be
associated gc.

Currently in this case the new_ptr would not be registered with any
gc, which Coverity rightly points out (in various different places)
would be a memory leak.

It would also be possible to fix this by adding a libxl__ptr_add() at
the same point, however semantically it seems like a programming error
to gc-realloc a pointer which is not associated with the gc in
question, so treat it as such.

Compile tested only, this change could expose latent bugs.

Signed-off-by: Ian Campbell 
---
v2: Check we actually didn't find the ptr in the gc
Correct log message and shorten since LOG will inject the
   function name.
---
 tools/libxl/libxl_internal.c | 4 
 tools/libxl/libxl_internal.h | 4 +++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
index 328046b..fc81130 100644
--- a/tools/libxl/libxl_internal.c
+++ b/tools/libxl/libxl_internal.c
@@ -122,6 +122,10 @@ void *libxl__realloc(libxl__gc *gc, void *ptr, size_t 
new_size)
 break;
 }
 }
+if (i == gc->alloc_maxsize) {
+LOG(CRITICAL, "pointer is not tracked by the given gc");
+abort();
+}
 }
 
 return new_ptr;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index fc1b558..650a958 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -617,7 +617,9 @@ _hidden void *libxl__zalloc(libxl__gc *gc_opt, size_t size) 
NN1;
 _hidden void *libxl__calloc(libxl__gc *gc_opt, size_t nmemb, size_t size) NN1;
 /* change the size of the memory block pointed to by @ptr to @new_size bytes.
  * unlike other allocation functions here any additional space between the
- * oldsize and @new_size is not initialised (similar to a gc'd realloc(3)). */
+ * oldsize and @new_size is not initialised (similar to a gc'd realloc(3)).
+ * if @ptr is non-NULL and @gc_opt is not nogc_gc then @ptr must have been
+ * registered with @gc_opt previously. */
 _hidden void *libxl__realloc(libxl__gc *gc_opt, void *ptr, size_t new_size) 
NN1;
 /* print @fmt into an allocated string large enoughto contain the result.
  * (similar to gc'd asprintf(3)). */
-- 
2.1.4


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

Re: [Xen-devel] [BUG} libxl.c:5947:libxl_send_trigger: Send trigger 'reset' failed: Function not implemented

[Ian Campbell]

On Thu, 2016-02-11 at 09:30 +1100, Alex Braunegg wrote:
> So potentially before trying to make use of libxl__domain_pvcontrol for
> the 'xl vm-list' command to provide feedback if PV control is available,
> I will need to wait for the functionality to be exposed via an applicable
> libxl header.

Yes, exposing an internal function in the public API generally involves:

 * renaming into the libxl_foo rather than libxl__foo namespace and moving
   the prototype to libxl.h. Perhaps giving it a more "public" API friendly
   name and checking that the API is sane enough to be maintained in an API
   compatible manner going forward (this one seems simple enough to be
   sane).
 * Adjusting the function to take a libxl_ctx *ctx instead of a libxl__gc
   *gc, which involves adding GC_INIT and GC_FREE in the function itself.
   This will require ensuring the function has a single exit path which
   calls GC_FREE (see also tools/libxl/CODING_STYLE)
 * Adding a LIBXL_HAVE... to libxl.h to indicate to 3rd party applications
   that this functionality is available (nothing in xen.git should be
   conditional on this #define though)
 * Updating xl to use this new functionality

Patches welcome.

Ian.

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

Re: [Xen-devel] [PATCH v2 1/1] XEN/ARM: Add Odroid-XU3/XU4 support

[Ian Campbell]

On Wed, 2016-02-10 at 17:47 -0800, Suriyan Ramasami wrote:
> 
> 
> On Wed, Feb 10, 2016 at 2:03 AM, Ian Campbell 
> wrote:
> > On Tue, 2016-02-09 at 10:20 -0800, Suriyan Ramasami wrote:
> > >  I agree on the first two paragraphs.
> > > > > For the third paragraph, the rebuttal is that the exynos5800 and
> > > > > exynos5422 based SoCs can have both clusters on at the same time.
> > Hence,
> > > > > the third paragrapah comment will have to be tweaked further.
> > Possibly
> > > > > reading:
> > > > > The exynos5800 and exynos5422 can have both clusters on at the
> > same time.
> > > > > The exynos5800 boots up with cpu0 on cluster0 (A15). The
> > exynos5422 can
> > > > > boot up on either clusters as its pin controlled. In this case
> > the DTS
> > > > > should properly reflect the cpu order.
> > > >
> > > > Does the OS need to be aware of all these combinations though? Is
> > it not
> > > > sufficient to know how to bring up an A15 core and how to bring up
> > an A7
> > > > core and then just do so based on the information in the DTS,
> > without
> > > > needing to worry about which sort of core we happened to have
> > booted on?
> > > >
> > > >
> > > Unfortunately, at least looking at the boot up code for the
> > Exynos5422,
> > > the OS needs to be aware of it. This is what I see in the linux
> > source
> > > code. If it boots up on an A7, then a special reset is needed which
> > is
> > > not needed when booted up otherwise. I do not have much more details
> > on
> > > that other than the Linux code.
> > > Without that reset sequence, I have also verified that the powered on
> > CPU
> > > does not come up.
> > 
> > Are we able to say that if we are booted on cluster 1 (always the A7s)
> > then
> > we always need this magic reset? i.e. is true for all SoCs which have
> > an A7
> > cluster and can boot from it? (it's tautologically true for SocS which
> > either have no A7's or cannot boot from them).
> > 
> I do not have the information to answer the question. I am limited to
> what I know (albeit a little bit) wrt the hardkernel related boards -
> Exynos 5410 (odroid-XU) and the Exynos 5422 (Odoird XU3/XU4). With my
> limited knowledge, I am only aware of Exynos 5410 which is capable of
> booting off of an A7 or an A15.
>  
> >  Maybe I'm looking for similarities between different exynos variants
> > which
> > doesn't exist though. If we are going to talk about specific SoCs in
> > the
> > comments then I would rather that the code was also explicit rather
> > than
> > assuming cluster 1 will only be found on the 5800, that might be as
> > simple
> > as mapping the compatible string to a max_cluster (default 0 for
> > unknown
> > SoC) and warning if pcluster > max_cluster.
> Can you please elaborate on the mapping that you talk about above. I am
> lost here :-(

What I mean is can we say:
    exynos 1234 => Two clusters (max_cluster == 1)
    exynos 5678 => One cluster (max_cluster == 0)
    exynos ABCD => Two clusters (max_cluster == 1) 
    Unknown     => Assume one cluster

and can we also assume that cluster 0 always consists of A15s and cluster 1
(if it exists) always consists of A7s?

If so then we can say:

  max_cluster = look_up_by_compat(compat)
  pcluster = figure out from midr
  pcpu = figure it out

  if (pcluster >= max_cluster)
    error

  do bringup

  if (pluster == 1)
    do special handling for cluster 1 == a7

The difference compared with what you have is that it adds a check that we
expect a second cluster for the SoC before it goes poking at stuff.

What I'm trying to avoid is coming across some other SoC variant which has
2 clusters but has something different to the A7s or which requires some
different handling.

If we were confident that all exynos SoCs always require the same
special handling for cluster 1 then we wouldn't really need this, but I
don't think we know that?

>  
> >  
> > >
> > > >  >  The exynos5410 can have only one cluster on at a time, and it
> > boots
> > > > up
> > > > > with pcluster == 0.
> > > > > Any tweaks and comments on the above is appreciated.
> > > >
> > > > How much of this is down to physical h/w limitations and how much
> > of it
> > > > is
> > > > down to firmware or software limitations? Can you flip the to the
> > other
> > > > cluster somehow?
> > > >   
> > > The 5410 boots up on an A15, and only those 4 A15 CPUs in cluster 0
> > are
> > > brought up. Hence, no flipping is required.
> > 
> > What I meant was, given that the 5410 has a cluster of A15 and a
> > cluster of
> > A7s (right?) and you can only have one on at a time, how does an OS
> > make
> > use of the A7s if it wants to? As you say it boots from the A15, so how
> > can
> > the A7s be used?
> > 
> > 
> The Linux OS has a bL (big - little) switcher module/code which handles
> that. It maps one big core to one little core, and when the load is not
> high, it switches off the big cluster, and turns on the small cluster -
> AFAICT.

So this is an OS limitation, not a h/w one? 

Re: [Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()

[Dario Faggioli]

Hi Harmandeep,

So, I think the code in this patch is ok. Still, a few comments...

On Thu, 2016-02-11 at 14:00 +0530, Harmandeep Kaur wrote:
> Avoid handling issue of the return value of xc_version() in many
> cases.
> 
This can be rephrased to be easier to understand (I'm not sure I can
tell what you mean with "Avoid handling issue of the return value").

Something like "Check the return value of xc_version() and return NULL
if it fails."

In fact, I think the fact that the function can now actually return
NULL should be mentioned here in the changelog.

Another thing that you should check, and probably quickly mention too,
is whether or not the callers of these functions --the ones inside
xen.git of course-- are prepared to deal with this. I mean, returning
NULL on failure of xc_version() is, IMO, the right thing to do here
anyway, but it is something important to know whether more work is
needed to fix the callers as well, or if we're just fine.

To do so, search (e.g., with grep or cscope) for uses of
libxl_get_version_info inside the tools/ dir of xen.git. For instance,
there is one such use in xl_cmdimpl.c, in the output_xeninfo()
function, which looks like it would interact well with this patch...
Can you confirm this is the case also for all the other instances and
note it down here?

> Coverity ID 1351217
> 
> Signed-off-by: Harmandeep Kaur 
> ---
>  tools/libxl/libxl.c | 39 ++-
>  1 file changed, 30 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2d18b8d..a939e51 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5267,42 +5267,63 @@ const libxl_version_info*
> libxl_get_version_info(libxl_ctx *ctx)
>  xen_platform_parameters_t p_parms;
>  xen_commandline_t xen_commandline;
>  } u;
> +int rc;
>
So, according to tools/libxl/CODING_STYLE:

The following local variable names should be used where applicable:

  int rc;/* a libxl error code - and not anything else */
  int r; /* the return value from a system call (or libxc call) */

Given how it's used, this variable you're introducing falls into the
latter category.

And I also think you need to initialize it.

>  long xen_version;
>  libxl_version_info *info = &ctx->version_info;
>  
>  if (info->xen_version_extra != NULL)
>  goto out;
>  
> -xen_version = xc_version(ctx->xch, XENVER_version, NULL);
> +rc = xen_version = xc_version(ctx->xch, XENVER_version, NULL);
> +if ( rc < 0 )
> +goto out;
> +
>  info->xen_version_major = xen_version >> 16;
>  info->xen_version_minor = xen_version & 0xFF;
>
I think you can just get rid of the xen_version variable and, if
xc_version() returns > 0, do the necessary manipulations on r itself
(this, for instance, matches what happens in
xc_core.c:elfnote_fill_xen_version, and is IMO ok to be done here as
well).

> +rc = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
> +if ( rc < 0 )
> +goto out;
>  
> -xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
>  info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra);
> +rc = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
> +if ( rc < 0 )
> +goto out;
>  
> -xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
>  info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler);
>  info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by);
>  info->compile_domain = libxl__strdup(NOGC,
> u.xen_cc.compile_domain);
>  info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date);
>
Although functionally correct, the final look of all these "blocks"
would result rather hard to read.

I think it would be better if you make the patch in such a way that the
final code would look like this:

    r = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
    if ( r < 0 )
        goto out;
    info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra);

    r = xc_version(ctx->xch, XENVER_compile_info, &u.xen_cc);
    if ( r < 0 )
        goto out;  
    info->compiler = libxl__strdup(NOGC, u.xen_cc.compiler);
    info->compile_by = libxl__strdup(NOGC, u.xen_cc.compile_by);
    info->compile_domain = libxl__strdup(NOGC,
u.xen_cc.compile_domain);
    info->compile_date = libxl__strdup(NOGC, u.xen_cc.compile_date);

    r = xc_version(ctx->xch, XENVER_capabilities, &u.xen_caps);
    if ( r < 0 )
        goto out;
    info->capabilities = libxl__strdup(NOGC, u.xen_caps);

etc.

>   out:
>  GC_FREE;
> -return info;
> +if ( rc < 0 )
> + return NULL;
> +else
> + return info;
>
This can become "return r < 0 ? NULL : info;"

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitall

Re: [Xen-devel] [PATCH] public/io/netif.h: fix typos

[Ian Campbell]

On Wed, 2016-02-10 at 16:49 +, Paul Durrant wrote:
> Unfortunately my patch 162a81ab "document control ring and toeplitz
> hashing" contained a couple of typos. This patch fixes them.
> 
> Signed-off-by: Paul Durrant 
> Cc: Ian Campbell 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Keir Fraser 
> Cc: Tim Deegan 


Acked-by: Ian Campbell 



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

Re: [Xen-devel] [PATCH v2] libxc: fix leak of t_info in xc_tbuf_get_size()

[Ian Campbell]

Copying George since he maintains xentrace which this relates to.

On Thu, 2016-02-11 at 14:02 +0530, Harmandeep Kaur wrote:
> Avoid leaking the memory mapping of the trace buffer
> 
> Coverity ID 1351228
> 
> Signed-off-by: Harmandeep Kaur 
> ---
> v2: call to unmapping function reduced to one from two
> ---
>  tools/libxc/xc_tbuf.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxc/xc_tbuf.c b/tools/libxc/xc_tbuf.c
> index 695939a..d96cc67 100644
> --- a/tools/libxc/xc_tbuf.c
> +++ b/tools/libxc/xc_tbuf.c
> @@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch, unsigned long
> *size)
>  sysctl.u.tbuf_op.buffer_mfn);
>  
>  if ( t_info == NULL || t_info->tbuf_size == 0 )
> -return -1;
> +rc = -1;
> +else
> + *size = t_info->tbuf_size;
>  
> -*size = t_info->tbuf_size;
> +xenforeignmemory_unmap(xch->fmem, t_info, *size);

*size could be uninitialised here (in the error path) and even in the
success case I don't think t_info->tbus_size is the right argument here, it
needs to be the size which was passed to the map function, i.e.
sysctl.u.tbuf_op.size.

Ian.

>  
> -return 0;
> +return rc;
>  }
>  
>  int xc_tbuf_enable(xc_interface *xch, unsigned long pages, unsigned long
> *mfn,

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

Re: [Xen-devel] [PATCH 1/3] xen/x86: Drop the uses of invbool_param()

[Ian Campbell]

On Tue, 2016-02-09 at 05:46 -0700, Jan Beulich wrote:
> > > > On 08.02.16 at 18:07,  wrote:
> > There are only four users, and invbool_param() is an unnecessary
> > cognitive
> > overhead to use.
> 
> While this isn't necessarily a bad change, I don't agree to either of
> these arguments. So I'm going to make my ack here dependent on
> seeing some kind of positive feedback on patch 2 by another
> REST maintainer.

FWIW I do have to think a little harder about the double negative in
bool_t foo; /* implicitly 0  init */
invbool_param("foo", foo_disabled);
and what no-foo on the command line actually does in this case than I would
otherwise with:
bool_t foo = true;
boolean_param("foo", foo);

AFAICT the only real benefit of the inversion is that the variable can live
in .bss instead of .data, but it's a total of 4 bool_t's (so 4 bytes? Or
maybe 16 at the most) which hardly seems worth it to me.

Ian.

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

Re: [Xen-devel] [PATCH v2] libxc: fix leak of t_info in xc_tbuf_get_size()

[Dario Faggioli]

On Thu, 2016-02-11 at 09:52 +, Ian Campbell wrote:
> On Thu, 2016-02-11 at 14:02 +0530, Harmandeep Kaur wrote:
> > 
> > diff --git a/tools/libxc/xc_tbuf.c b/tools/libxc/xc_tbuf.c
> > index 695939a..d96cc67 100644
> > --- a/tools/libxc/xc_tbuf.c
> > +++ b/tools/libxc/xc_tbuf.c
> > @@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch,
> > unsigned long
> > *size)
> >  sysctl.u.tbuf_op.buffer_mfn);
> >  
> >  if ( t_info == NULL || t_info->tbuf_size == 0 )
> > -return -1;
> > +rc = -1;
> > +else
> > +   *size = t_info->tbuf_size;
> >  
> > -*size = t_info->tbuf_size;
> > +xenforeignmemory_unmap(xch->fmem, t_info, *size);
> 
> *size could be uninitialised here (in the error path) and even in the
> success case I don't think t_info->tbus_size is the right argument
> here, it
> needs to be the size which was passed to the map function, i.e.
> sysctl.u.tbuf_op.size.
> 
And I think both are issues with the current code, and, more important,
not what Coverity is complaining about in the referenced CID?

To be clear, I'm not arguing that they're not issues we should fix (I
don't know about the tbuf_size vs. tbuf_op.size, but I can check). I'm
genuinely asking whether we should do that here, as compared to in a
pre or follow up patch, and let this one be the one that placates
Coverity on that particular issue.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] libxc: fix leak of t_info in xc_tbuf_get_size()

[Ian Campbell]

On Thu, 2016-02-11 at 11:03 +0100, Dario Faggioli wrote:
> On Thu, 2016-02-11 at 09:52 +, Ian Campbell wrote:
> > On Thu, 2016-02-11 at 14:02 +0530, Harmandeep Kaur wrote:
> > >  
> > > diff --git a/tools/libxc/xc_tbuf.c b/tools/libxc/xc_tbuf.c
> > > index 695939a..d96cc67 100644
> > > --- a/tools/libxc/xc_tbuf.c
> > > +++ b/tools/libxc/xc_tbuf.c
> > > @@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch,
> > > unsigned long
> > > *size)
> > >  sysctl.u.tbuf_op.buffer_mfn);
> > >  
> > >  if ( t_info == NULL || t_info->tbuf_size == 0 )
> > > -return -1;
> > > +rc = -1;
> > > +else
> > > + *size = t_info->tbuf_size;
> > >  
> > > -*size = t_info->tbuf_size;
> > > +xenforeignmemory_unmap(xch->fmem, t_info, *size);
> > 
> > *size could be uninitialised here (in the error path) and even in the
> > success case I don't think t_info->tbus_size is the right argument
> > here, it
> > needs to be the size which was passed to the map function, i.e.
> > sysctl.u.tbuf_op.size.
> > 
> And I think both are issues with the current code,

I don't think so, the xenforeignmemory_unmap using *size as an argument
(where it is either uninitialised or the wrong value) is added by this
patch.

>  and, more important,
> not what Coverity is complaining about in the referenced CID?

> To be clear, I'm not arguing that they're not issues we should fix (I
> don't know about the tbuf_size vs. tbuf_op.size, but I can check). I'm
> genuinely asking whether we should do that here, as compared to in a
> pre or follow up patch, and let this one be the one that placates
> Coverity on that particular issue.

Ian.

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

Re: [Xen-devel] [PATCH] libxl: fix handling returns in libxl_get_version_info()

[Ian Campbell]

On Thu, 2016-02-11 at 10:48 +0100, Dario Faggioli wrote:
> I think it would be better if you make the patch in such a way that the
> final code would look like this:
> 
>     r = xc_version(ctx->xch, XENVER_extraversion, &u.xen_extra);
>     if ( r < 0 )
>         goto out;
>     info->xen_version_extra = libxl__strdup(NOGC, u.xen_extra);

That is, each "paragraph" consists of:

call a variant of xc_version
check for errors in that call
propagate results of that call to info->... as necessary

and I agree. FWIW CODING_STYLE also allows (but doesn't mandate) for
if ( r < 0 ) goto out;
i.e. on a single line, which Harmandeep might prefer here for brevity.

Ian

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

Re: [Xen-devel] [PATCH v2] libxc: fix leak of t_info in xc_tbuf_get_size()

[Dario Faggioli]

On Thu, 2016-02-11 at 10:11 +, Ian Campbell wrote:
> On Thu, 2016-02-11 at 11:03 +0100, Dario Faggioli wrote:
> > On Thu, 2016-02-11 at 09:52 +, Ian Campbell wrote:
> > > On Thu, 2016-02-11 at 14:02 +0530, Harmandeep Kaur wrote:
> > > >  
> > > > diff --git a/tools/libxc/xc_tbuf.c b/tools/libxc/xc_tbuf.c
> > > > index 695939a..d96cc67 100644
> > > > --- a/tools/libxc/xc_tbuf.c
> > > > +++ b/tools/libxc/xc_tbuf.c
> > > > @@ -70,11 +70,13 @@ int xc_tbuf_get_size(xc_interface *xch,
> > > > unsigned long
> > > > *size)
> > > >  sysctl.u.tbuf_op.buffer_mfn);
> > > >  
> > > >  if ( t_info == NULL || t_info->tbuf_size == 0 )
> > > > -return -1;
> > > > +rc = -1;
> > > > +else
> > > > +   *size = t_info->tbuf_size;
> > > >  
> > > > -*size = t_info->tbuf_size;
> > > > +xenforeignmemory_unmap(xch->fmem, t_info, *size);
> > > 
> > > *size could be uninitialised here (in the error path) and even in
> > > the
> > > success case I don't think t_info->tbus_size is the right
> > > argument
> > > here, it
> > > needs to be the size which was passed to the map function, i.e.
> > > sysctl.u.tbuf_op.size.
> > > 
> > And I think both are issues with the current code,
> 
> I don't think so, the xenforeignmemory_unmap using *size as an
> argument
> (where it is either uninitialised or the wrong value) is added by
> this
> patch.
> 
Ah, that one! Yes, you're right, I had overlooked this, and thought you
where referring to something else, sorry. I agree sysctl.u.tbuf_op.size
is what should be used.

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 2/7] xen/x86: merge 2 hvm_event_... functions into 1

[Jan Beulich]

>>> On 10.02.16 at 21:56,  wrote:
> On 10/02/2016 17:11, Jan Beulich wrote:
> On 10.02.16 at 18:04,  wrote:
>>> On 2/10/2016 6:18 PM, Jan Beulich wrote:
>>> On 10.02.16 at 16:50,  wrote:
> --- a/xen/include/asm-x86/hvm/event.h
> +++ b/xen/include/asm-x86/hvm/event.h
> @@ -17,6 +17,12 @@
>   #ifndef __ASM_X86_HVM_EVENT_H__
>   #define __ASM_X86_HVM_EVENT_H__
>   
> +enum hvm_event_breakpoint_type
> +{
> +HVM_EVENT_SOFTWARE_BREAKPOINT,
> +HVM_EVENT_SINGLESTEP_BREAKPOINT,
> +};
 I don't see what good it does to put existing constants into an
 enum.
>>> As Andrew pointed out, an enum was requested in v1 instead of the 
>>> single_step param.
>>> One could use the already existing VM_EVENT_REASON_* constants, but 
>>> conceptually this
>>> function only involves a subset of those (i.e. *breakpoint vm-events*).
>> Re-using existing constants would seem fine to me.
>>
>> I only now realize that I've made a mistake while looking at the
>> above - the capitals made it implicitly "obvious" to me that they're
>> on the right side of an assignment. Please use capitals only for
>> #define-d constants, not enumerated ones.
> 
> Substantially more enums in the Xen codebase use caps than lowercase. 

Well, sadly this indeed seems to be the case.

> Given no specific direction in CODING_STYLE, this is an unreasonable
> request.

Hence I withdraw the request, despite continuing to be convinced
that all-caps enumerators are bad practice.

Jan


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

[Xen-devel] xl dev-detach hangs with missing frontends

[Olaf Hering]

How should libxl__initiate_device_generic_remove deal with devices which
have no frontend driver? Right now it moves "state" from either
XenbusStateInitialising or XenbusStateInitWait to XenbusStateClosing.
Then it expects the backend to move "state" to XenbusStateClosed. This
will never happen, at least for netback and scsiback. The result is a 10
second delay.


# xl - network-detach $domU $mac
2016-02-11 11:33:19 CET [20318] libxl: debug: 
libxl.c:4614:libxl_device_nic_remove: ao 0x19b3870: create: how=(nil) 
callback=(nil) poller=0x19b0200
2016-02-11 11:33:19 CET [20318] libxl: debug: 
libxl_event.c:636:libxl__ev_xswatch_register: watch w=0x19b2c40 
wpath=/local/domain/0/backend/vif/10/1/state token=3/0: register slotnum=3
2016-02-11 11:33:19 CET [20318] libxl: debug: 
libxl.c:4614:libxl_device_nic_remove: ao 0x19b3870: inprogress: 
poller=0x19b0200, flags=i
2016-02-11 11:33:19 CET [20318] libxl: debug: 
libxl_event.c:573:watchfd_callback: watch w=0x19b2c40 
wpath=/local/domain/0/backend/vif/10/1/state token=3/0: event 
epath=/local/domain/0/backend/vif/10/1/state
2016-02-11 11:33:19 CET [20318] libxl: debug: 
libxl_event.c:878:devstate_callback: backend 
/local/domain/0/backend/vif/10/1/state wanted state 6 still waiting state 5
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_aoutils.c:88:xswait_timeout_callback: backend 
/local/domain/0/backend/vif/10/1/state (hoping for state change to 6): xswait 
timeout (path=/local/domain/0/backend/vif/10/1/state)
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:673:libxl__ev_xswatch_deregister: watch w=0x19b2c40 
wpath=/local/domain/0/backend/vif/10/1/state token=3/0: deregister slotnum=3
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:862:devstate_callback: backend 
/local/domain/0/backend/vif/10/1/state wanted state 6  timed out
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2c40: deregister 
unregistered
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_device.c:939:device_backend_callback: calling device_backend_cleanup
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2c40: deregister 
unregistered
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_device.c:945:device_backend_callback: Timeout reached, initiating forced 
remove
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:636:libxl__ev_xswatch_register: watch w=0x19b2c40 
wpath=/local/domain/0/backend/vif/10/1/state token=3/1: register slotnum=3
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:573:watchfd_callback: watch w=0x19b2c40 
wpath=/local/domain/0/backend/vif/10/1/state token=3/1: event 
epath=/local/domain/0/backend/vif/10/1/state
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:874:devstate_callback: backend 
/local/domain/0/backend/vif/10/1/state wanted state 6 ok
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:673:libxl__ev_xswatch_deregister: watch w=0x19b2c40 
wpath=/local/domain/0/backend/vif/10/1/state token=3/1: deregister slotnum=3
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_device.c:939:device_backend_callback: calling device_backend_cleanup
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2c40: deregister 
unregistered
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_device.c:1036:device_hotplug: calling hotplug script: 
/opt/xen/staging-wip/etc/xen/scripts/vif-bridge offline
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_aoutils.c:593:libxl__async_exec_start: forking to execute: 
/opt/xen/staging-wip/etc/xen/scripts/vif-bridge offline
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:542:watchfd_callback: watch 
epath=/local/domain/0/backend/vif/10/1/state token=3/1: empty slot
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2d40: deregister 
unregistered
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_device.c:1023:device_hotplug: No hotplug script to execute
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:686:libxl__ev_xswatch_deregister: watch w=0x19b2d40: deregister 
unregistered
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:1869:libxl__ao_complete: ao 0x19b3870: complete, rc=0
2016-02-11 11:33:29 CET [20318] libxl: debug: 
libxl_event.c:1838:libxl__ao__destroy: ao 0x19b3870: destroy
2016-02-11 11:33:29 CET [20318] xencall:buffer: debug: total allocations:22 
total releases:22
2016-02-11 11:33:29 CET [20318] xencall:buffer: debug: current allocations:0 
maximum allocations:2
2016-02-11 11:33:29 CET [20318] xencall:buffer: debug: cache current size:2
2016-02-11 11:33:29 CET [20318] xencall:buffer: debug: cache hits:14 misses:2 
toobig:6
# echo $?
0

At least ${dev}-detach reports no error in this case.

Olaf

___
Xen-devel mailing list
Xen-devel@lists.xen

Re: [Xen-devel] [PATCH v2] build: specify minimum versions of make and binutils

[Jan Beulich]

>>> On 10.02.16 at 21:36,  wrote:
> On 28/01/2016 13:47, Jan Beulich wrote:
> On 28.01.16 at 14:02,  wrote:
>>> On Thu, 2016-01-28 at 05:49 -0700, Jan Beulich wrote:
>>> On 28.01.16 at 00:12,  wrote:
> To help people avoid having to figure out what versions of make and
> binutils need to be supported document them explicitly. The version of
> binutils that had to be supported was mentioned in
> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg00609.ht 
> ml 
> as 2.17 recently. It was decided that the versions should instead be
> GNU binutils 2.16.1 and GNU Make 3.80 in
> http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg02134.ht 
> ml 
 "decided" is a bit strong. I suggested these values. And while I'm
 pretty certain that even plain make 3.80 will work, I'm in no way
 sure plain 2.16.1 will (what I'm building with once in a while is some
 2.16.9x, and I can't say how many backports it has). So the
 question really is - did you test that things build with these?
>>> Why would he have done, you suggested 2.16.1 with no hint that you thought
>>> it might not be a reasonable version to use.
>>>
>>> TBH having rejected Doug's original proposal I would have said it was up to
>>> you to specify the actual precise versions you think should be used, rather
>>> than making Doug guess and leading him down blind allies by making
>>> apparently authoritative suggestions which you secretly aren't actually
>>> sure about yourself.
>> To be honest it didn't even occur to me that someone might
>> propose such a patch without verifying things actually build
>> (unless using more cautious wording). Also note that in the first
>> reply to the v1 patch I did refer to 2.16.9x (which imo has made
>> clear that that's the lowest one I ever tested with recently), i.e.
>> I don't think I've actively mislead him.
>>
>>> Anyway we could go round and round like this forever. What's wrong with
>>> starting with this as a baseline and bumping it if it turns out to be a
>>> problem in practice?
>> Well, we certainly could (which would be in line with my second
>> reply to v1), just that I'm not sure how much value such a doc
>> addition then has. At the very least it should then say "no
>> lower than 2.16.1, something slightly newer may be needed" or
>> some such.
>>
 Also I'm not sure 2.16.1 is going to be sufficient for ARM (it's
 most definitely too old for ARM64).
>>> I suppose there is an implicit max(version, first version supporting arch).
>>> I don't think we can really go into the level of detail needed for per arch
>>> toolchain requirements.
>> I'm afraid quite frequently "first version supporting arch" isn't
>> good enough. If we know otherwise for ARM64, that's certainly
>> fine.
>>
>>> I certainly don't know which version of either gcc or binutils is needed to
>>> build either ARM variant.
>> Well, again - what's that documentation addition then good for?
> 
> Ping?

It's not really clear at whom this ping was directed, the more that
iirc the patch meanwhile got withdrawn.

> It doesn't matter exactly which version we choose as a minimum, but it
> *is* very important that the version is written down.  Our README file
> is the correct place for this information to live.

I think it is quite relevant which version is to be picked: Anything
older no-one could legitimately report issues against (and I would
very much like to continue to be able to submit build fixes I find
necessary on those two old boxes I keep for a reason), while
stating something too old which even today we don't successfully
build with would be pretty odd.

Jan



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

Re: [Xen-devel] [PATCH 8/8] x86/efi: Generate uefi_call_wrapper() when compiling with clang

[Jan Beulich]

>>> On 10.02.16 at 20:11,  wrote:
> On 10/02/16 13:41, Andrew Cooper wrote:
>> On 10/02/16 13:31, Jan Beulich wrote:
>> On 09.02.16 at 21:01,  wrote:
 Signed-off-by: Andrew Cooper 
 ---
 CC: Jan Beulich 

 What is the GCC version check supposed to be achieving here?  GCC has
 supported this syntax since 3.0
>>> This is best answered by looking at where we've got these headers
>>> from - the gnu-efi package. It has ...
>>>
 --- a/xen/include/asm-x86/x86_64/efibind.h
 +++ b/xen/include/asm-x86/x86_64/efibind.h
 @@ -274,7 +274,7 @@ typedef uint64_t   UINTN;
  #endif
  #endif
  
 -#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4)
 +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4) || __clang__
  #define uefi_call_wrapper(func, va_num, ...)  func(__VA_ARGS__)
  #else
  /* for x86_64, EFI_FUNCTION_WRAPPER must be defined */
>>> /* for x86_64, EFI_FUNCTION_WRAPPER must be defined */
>>> #if defined(HAVE_USE_MS_ABI)
>>> #define uefi_call_wrapper(func, va_num, ...) func(__VA_ARGS__)
>>> #else
>>> UINTN uefi_call_wrapper(void *func, unsigned long va_num, ...);
>>> #endif
>>> #define EFI_FUNCTION __attribute__((ms_abi))
>>>
>>> I think this makes clear that the needed feature here is the
>>> attribute, which certainly wasn't available in older gcc.
>>>
>>> With that the question is whether the Clang case won't also need
>>> a version check, since I can't imagine them having supported this
>>> prior to gcc introducing it.
>> Clang has an substantially more sane way than GCC of checking for
>> individual features.  I will introduce and use the
>> __has_{attribute,feature}() infrastructure to tests like this.
>>
>> Experimentally, Clang 3.5 does have ms_abi support
> 
> Looking at it further, this entire block is unsed.  Nothing in tree
> refers to uefi_call_wrapper() or EFI_FUNCTION_WRAPPER other than this
> small bit; all declarations use EFIABI directly.
> 
> i.e. this:
> 
> diff --git a/xen/include/asm-x86/x86_64/efibind.h
> b/xen/include/asm-x86/x86_64/efibind.h
> index af5f424..b013db1 100644
> --- a/xen/include/asm-x86/x86_64/efibind.h
> +++ b/xen/include/asm-x86/x86_64/efibind.h
> @@ -274,17 +274,6 @@ typedef uint64_t   UINTN;
>  #endif
>  #endif
>  
> -#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 4)
> -#define uefi_call_wrapper(func, va_num, ...)   func(__VA_ARGS__)
> -#else
> -/* for x86_64, EFI_FUNCTION_WRAPPER must be defined */
> -#ifdef  EFI_FUNCTION_WRAPPER
> -UINTN uefi_call_wrapper(void *func, unsigned long va_num, ...);
> -#else
> -#error "EFI_FUNCTION_WRAPPER must be defined for x86_64 architecture"
> -#endif
> -#endif
> -
>  #ifdef _MSC_EXTENSIONS
>  #pragma warning ( disable : 4731 )  // Suppress warnings about
> modification of EBP
>  #endif
> 
> works correctly for GCC and clang.  Would dropping this completely be
> acceptable?

We perhaps might as well; I had mainly kept it to stay as close to
the original header as possible (without leaving around a latent
build breakage).

Jan


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

Re: [Xen-devel] [PATCH 1/3] x86: avoid flush IPI when possible

[Jan Beulich]

>>> On 10.02.16 at 18:51,  wrote:
> On 10/02/16 15:37, Jan Beulich wrote:
> On 10.02.16 at 16:00,  wrote:
>>> On 10/02/16 12:56, Jan Beulich wrote:
 Since CLFLUSH, other than WBINVD, is a coherency domain wide flush,
>>> I can't parse this sentence.
>> Should have been "..., is a cache coherency domain wide flush, ..." -
>> does it read any better then?
> 
> I believe, given the code in the patch, your intent is "if we WBINVD, we
> don't need to IPI other cores cache flushing reasons".

I don't see how this can be read from the sentence. The primary
statement is makes if "CLFLUSH is a cache coherency domain wide
flush". A secondary statement is that this is different from WBINVD.

> However, given your comment below...
> 
>>
>>> CLFUSH states "Invalidates from every level of the cache hierarchy in
>>> the cache coherence domain"
>>>
>>> WBINVD however states "The instruction then issues a special-function
>>> bus cycle that directs external caches to also write back modified data
>>> and another bus cycle to indicate that the external caches should be
>>> invalidated."
>>>
>>> I think we need input from Intel and AMD here as to the behaviour and
>>> terminology here, and in particular, where the coherency domain
>>> boundaries are.  All CPUs, even across multiple sockets, see coherent
>>> caching, but it is unclear whether this qualifies them to be in the same
>>> cache coherency domain per the instruction spec.
>> Linux already doing what this patch switches us to, I'm not sure
>> we need much extra input.
>>
>>> In particular, given the architecture of 8-socket systems and 45MB of
>>> RAM in L3 caches, does wbinvd seriously drain all caches everywhere? 
>> Not everywhere, just on the local socket (assuming there's no external
>> cache).
> 
> If this is true, then it is clearly not safe to omit the IPIs.

When using CLFLUSH it is safe, while when using WBINVD it's not.

>>> Causing 45MB of data to move to remote memory controllers all at once
>>> would cause a massive system stall.
>> That's why it takes (as we know) so long. See the figure in SDM Vol 3
>> section "Invalidating Caches and TLBs".
> 
> I presume you mean Figure 2-10. WBINVD Invalidation of Shared and
> Non-Shared Cache Hierarchy?
> 
> This quite clearly shows that WBINVD will not invalidate or write back
> the L1 caches for other cores in the same processor.
> 
> Have I misunderstood the logic for choosing when to omit the IPIs?

I'm afraid you did, or else I must have introduced a (latent,
because I didn't notice any issues so far) bug.

Jan


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

[Xen-devel] [PATCH v2 2/2] x86/traps: Dump instruction stream in show_execution_state()

[Andrew Cooper]

For first pass triage of crashes, it is useful to have the instruction
stream present, especially now that Xen binary patches itself.

A sample output now looks like:

(XEN) [ Xen-4.7-unstable  x86_64  debug=y  Not tainted ]
(XEN) CPU:0
(XEN) RIP:e008:[] default_idle+0x76/0x7b
(XEN) RFLAGS: 0246   CONTEXT: hypervisor
(XEN) rax: 82d080331030   rbx: 83007fce8000   rcx: 
(XEN) rdx:    rsi: 82d080331b98   rdi: 
(XEN) rbp: 83007fcefef0   rsp: 83007fcefef0   r8:  83007faf8118
(XEN) r9:  0009983e89fd   r10: 0009983e89fd   r11: 0246
(XEN) r12: 83007fd61000   r13:    r14: 83007fad9000
(XEN) r15: 83007fae3000   cr0: 8005003b   cr4: 26e0
(XEN) cr3: 7fc9b000   cr2: 7f70976b3fed
(XEN) ds:    es:    fs:    gs:    ss: e010   cs: e008
(XEN) Xen code around  (default_idle+0x76/0x7b):
(XEN)  83 3c 10 00 75 04 fb f4  01 fb 5d c3 55 48 89 e5 3b 3d 0d 50 12 00 72
(XEN) Xen stack trace from rsp=83007fcefef0:
(XEN)83007fceff10 82d080160e08 82d08012c40a 83007faf9000
(XEN)83007fcefdd8 81a01fd8 88002f07d4c0 81a01fd8
(XEN) 81a01e58 81a01fd8 0246
(XEN)0052   
(XEN)810013aa 0001 deadbeef deadbeef
(XEN)0100 810013aa e033 0246
(XEN)81a01e40 e02b  
(XEN)   83007faf9000
(XEN) 
(XEN) Xen call trace:
(XEN)[] default_idle+0x76/0x7b
(XEN)[] idle_loop+0x51/0x6e
(XEN)

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v2: Deal with %rip wrapping.  In such a case, insert dashes into printed line.
---
 xen/arch/x86/traps.c | 51 +++
 1 file changed, 51 insertions(+)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index ab7deee..5146e69 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -114,6 +114,56 @@ boolean_param("ler", opt_ler);
 #define stack_words_per_line 4
 #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
 
+static void show_code(const struct cpu_user_regs *regs)
+{
+unsigned char insns_before[8], insns_after[16];
+unsigned int i, missing_before, missing_after;
+
+if ( guest_mode(regs) )
+return;
+
+/*
+ * This dance with {insns,missing}_{before,after} is to ensure that, if
+ * %rip -8/+16 wraps around a bounday, we read a non-wrapped regs->rip
+ * pointer, and calculate which bytes were not read so they may be
+ * replaced with dashes in the printed output.
+ */
+missing_before = __copy_from_user(
+insns_before, (void __user *)regs->rip - 8, ARRAY_SIZE(insns_before));
+missing_after = __copy_from_user(
+insns_after, (void __user *)regs->rip, ARRAY_SIZE(insns_after));
+
+printk("Xen code around <%p> (%ps)%s:\n",
+   _p(regs->rip), _p(regs->rip),
+   (missing_before || missing_after) ? " [fault on access]" : "");
+
+/* Print bytes from insns_before[]. */
+for ( i = 0; i < ARRAY_SIZE(insns_before); ++i )
+{
+if ( i < (ARRAY_SIZE(insns_before) - missing_before) )
+printk(" %02x", insns_before[i]);
+else
+printk(" --");
+}
+
+/* Print the byte under %rip. */
+if ( missing_after != ARRAY_SIZE(insns_after) )
+printk(" <%02x>", insns_after[0]);
+else
+printk(" <-->");
+
+/* Print bytes from insns_after[]. */
+for ( i = 1; i < ARRAY_SIZE(insns_after); ++i )
+{
+if ( i < (ARRAY_SIZE(insns_after) - missing_after) )
+printk(" %02x", insns_after[i]);
+else
+printk(" --");
+}
+
+printk("\n");
+}
+
 static void show_guest_stack(struct vcpu *v, const struct cpu_user_regs *regs)
 {
 int i;
@@ -420,6 +470,7 @@ void show_execution_state(const struct cpu_user_regs *regs)
 unsigned long flags = console_lock_recursive_irqsave();
 
 show_registers(regs);
+show_code(regs);
 show_stack(regs);
 
 console_unlock_recursive_irqrestore(flags);
-- 
2.1.4


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

[Xen-devel] [PATCH v2 1/2] x86/traps: Prevent interleaving of concurrent cpu state dumps

[Andrew Cooper]

If two cpus enter show_execution_state() concurrently, the resulting console
output interleaved, and of no help debugging the situation further.

As calls to these locations are rare and usually important, it is acceptable
to serialise them.  These codepaths are also on the terminal error paths, so
the console lock must be the lock used for serialisation, to allow
console_force_unlock() to function properly.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Tim Deegan 
CC: Ian Campbell 

v2: Use the console lock rather than a local lock to allow
console_force_unlock() to function correctly on terminal error paths.
---
 xen/arch/x86/traps.c   | 12 
 xen/drivers/char/console.c | 16 
 xen/include/xen/console.h  |  2 ++
 3 files changed, 30 insertions(+)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index d19250a..ab7deee 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -416,12 +416,19 @@ void show_stack_overflow(unsigned int cpu, const struct 
cpu_user_regs *regs)
 
 void show_execution_state(const struct cpu_user_regs *regs)
 {
+/* Prevent interleaving of output. */
+unsigned long flags = console_lock_recursive_irqsave();
+
 show_registers(regs);
 show_stack(regs);
+
+console_unlock_recursive_irqrestore(flags);
 }
 
 void vcpu_show_execution_state(struct vcpu *v)
 {
+unsigned long flags;
+
 printk("*** Dumping Dom%d vcpu#%d state: ***\n",
v->domain->domain_id, v->vcpu_id);
 
@@ -433,10 +440,15 @@ void vcpu_show_execution_state(struct vcpu *v)
 
 vcpu_pause(v); /* acceptably dangerous */
 
+/* Prevent interleaving of output. */
+flags = console_lock_recursive_irqsave();
+
 vcpu_show_registers(v);
 if ( guest_kernel_mode(v, &v->arch.user_regs) )
 show_guest_stack(v, &v->arch.user_regs);
 
+console_unlock_recursive_irqrestore(flags);
+
 vcpu_unpause(v);
 }
 
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index e0083f1..f4f6141 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -865,6 +865,22 @@ void console_end_log_everything(void)
 atomic_dec(&print_everything);
 }
 
+unsigned long console_lock_recursive_irqsave(void)
+{
+unsigned long flags;
+
+local_irq_save(flags);
+spin_lock_recursive(&console_lock);
+
+return flags;
+}
+
+void console_unlock_recursive_irqrestore(unsigned long flags)
+{
+spin_unlock_recursive(&console_lock);
+local_irq_restore(flags);
+}
+
 void console_force_unlock(void)
 {
 watchdog_disable();
diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index c7fd9ca..ea06fd8 100644
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -21,6 +21,8 @@ int console_has(const char *device);
 
 int fill_console_start_info(struct dom0_vga_console_info *);
 
+unsigned long console_lock_recursive_irqsave(void);
+void console_unlock_recursive_irqrestore(unsigned long flags);
 void console_force_unlock(void);
 
 void console_start_sync(void);
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v2] build: specify minimum versions of make and binutils

[Ian Campbell]

On Thu, 2016-02-11 at 03:42 -0700, Jan Beulich wrote:
> I think it is quite relevant which version is to be picked: Anything
> older no-one could legitimately report issues against (and I would
> very much like to continue to be able to submit build fixes I find
> necessary on those two old boxes I keep for a reason), while
> stating something too old which even today we don't successfully
> build with would be pretty odd.

So lets start with, for x86 whatever the older of the versions on the two
boxes you refer to above and for ARM gcc 4.8 and binutils 2.24, which
corresponds to what I happen to use for both arm32 and arm64 in my
development environment.

Those are IMHO reasonable starting points but obviously they aren't set in
stone and can be easily adjusted later if necessary.

Ian.

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

[Xen-devel] HVMlite ABI specification DRAFT C + implementation outline

[Roger Pau Monné]

Hello,

I've Cced a bunch of people who have expressed interest in the HVMlite 
design/implementation, both from a Xen or OS point of view. If you 
would like to be removed, please say so and I will remove you in 
further iterations. The same applies if you want to be added to the Cc.

This is an initial draft on the HVMlite design and implementation. I've 
mixed certain aspects of the design with the implementation, because I 
think we are quite tied by the implementation possibilities in certain 
aspects, so not speaking about it would make the document incomplete. I 
might be wrong on that, so feel free to comment otherwise if you would 
prefer a different approach.

The document is still not complete. I'm of course not as knowledgeable 
as some people on the Cc, so please correct me if you think there are 
mistakes or simply impossible goals.

I think I've managed to integrate all the comments from DRAFT B. I still
haven't done a s/HVMlite/PVH/, but I plan to do so once the document is
finished and ready to go inside of the Xen tree.

Roger.

---
Xen HVMlite ABI
===

Boot ABI


Since the Xen entry point into the kernel can be different from the
native entry point, a `ELFNOTE` is used in order to tell the domain
builder how to load and jump into the kernel entry point:

ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,  .long,  xen_start32)

The presence of the `XEN_ELFNOTE_PHYS32_ENTRY` note indicates that the
kernel supports the boot ABI described in this document.

The domain builder shall load the kernel into the guest memory space and
jump into the entry point defined at `XEN_ELFNOTE_PHYS32_ENTRY` with the
following machine state:

 * `ebx`: contains the physical memory address where the loader has placed
   the boot start info structure.

 * `cr0`: bit 0 (PE) must be set. All the other writeable bits are cleared.

 * `cr4`: all bits are cleared.

 * `cs`: must be a 32-bit read/execute code segment with a base of ‘0’
   and a limit of ‘0x’. The selector value is unspecified.

 * `ds`, `es`: must be a 32-bit read/write data segment with a base of
   ‘0’ and a limit of ‘0x’. The selector values are all unspecified.

 * `tr`: must be a 32-bit TSS (active) with a base of '0' and a limit of '0x67'.

 * `eflags`: all modifiable bits are clear.

All other processor registers are unspecified. The OS is in charge of setting
up it's own stack, GDT and IDT.

The layout of the boot start info data is the following (pointed to by %ebx):

NOTE: nothing will be loaded at physical address 0, so a 0 value in any of the
address fields should be treated as not present.

 0 ++
   | magic  | Contains the magic value 0x336ec578
   || ("xEn3" with the 0x80 bit of the "E" set).
 4 ++
   | version| Version of this structure. Current version is 0.
   || New versions are guaranteed to be backwards-compatible.
 8 ++
   | flags  | SIF_xxx flags.
12 ++
   | cmdline_paddr  | Physical address of the command line,
   || a zero-terminated ASCII string.
16 ++
   | nr_modules | Number of modules passed to the kernel.
20 ++
   | modlist_paddr  | Physical address of an array of modules
   || (layout of the structure below).
24 ++
   | rsdp_paddr | Physical address of the RSDP ACPI data structure.
28 ++

The layout of each entry in the module structure is the following:

 0 ++
   | paddr  | Physical address of the module.
 8 ++
   | size   | Size of the module in bytes.
16 ++
   | cmdline_paddr  | Physical address of the command line,
   || a zero-terminated ASCII string.
24 ++
   | reserved   |
32 ++

Note that the address and size of the modules is a 64bit unsigned integer.
However Xen will always try to place all modules below the 4GiB boundary.

Other relevant information needed in order to boot a guest kernel
(console page address, xenstore event channel...) can be obtained
using HVMPARAMS, just like it's done on HVM guests.

The setup of the hypercall page is also performed in the same way
as HVM guests, using the hypervisor cpuid leaves and msr ranges.

Hardware description


Hardware description can come from two different sources, just like on (PV)HVM
guests.

Description of PV devices will always come from xenbus, and in fact
xenbus is the only hardware description that is guaranteed to always be
provided to HVMlite guests.

Description of physical hardware devices will always come from ACPI, in the
absence of any physical hardware device ACPI tables may not be provided. The
presence of ACPI tables can be detected by finding the RSDP, just like on
bare metal.

Non-PV devices exposed to the guest
---

The i

Re: [Xen-devel] [PATCH v2 2/2] x86/traps: Dump instruction stream in show_execution_state()

[Jan Beulich]

>>> On 11.02.16 at 11:52,  wrote:
> v2: Deal with %rip wrapping.  In such a case, insert dashes into printed 
> line.

I don't think this deals with wrapping now, since ...

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -114,6 +114,56 @@ boolean_param("ler", opt_ler);
>  #define stack_words_per_line 4
>  #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
>  
> +static void show_code(const struct cpu_user_regs *regs)
> +{
> +unsigned char insns_before[8], insns_after[16];
> +unsigned int i, missing_before, missing_after;
> +
> +if ( guest_mode(regs) )
> +return;
> +
> +/*
> + * This dance with {insns,missing}_{before,after} is to ensure that, if
> + * %rip -8/+16 wraps around a bounday, we read a non-wrapped regs->rip
> + * pointer, and calculate which bytes were not read so they may be
> + * replaced with dashes in the printed output.
> + */
> +missing_before = __copy_from_user(
> +insns_before, (void __user *)regs->rip - 8, 
> ARRAY_SIZE(insns_before));
> +missing_after = __copy_from_user(
> +insns_after, (void __user *)regs->rip, ARRAY_SIZE(insns_after));

... iirc __copy_from_user() doesn't range check the addresses.
Also reading the leading bytes is done in kind of a strange way: It'll
read initial bytes (farther away from RIP) and perhaps not read
later ones (closer to RIP), albeit clearly the ones closer are of more
interest. In the extreme case, where RIP is only a few bytes into a
page following an unmapped one, no leading bytes would be printed
at all despite some actually being readable.

Avoiding actual wrapping could be easily done by extending the
guest_mode() check above to also range check RIP against the
hypervisor image boundaries (post-boot this could even be limited
further, but perhaps using the full XEN_VIRT_{START,END} range is
the better route with xSplice in mind.

And then there's a literal 8 left in the first function invocation
above, which imo would better also be ARRAY_SIZE(insns_before)
(albeit with the comment above this invocation is going to change
anyway).

Jan


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

Re: [Xen-devel] [PATCH 1/2] xenmon: close qos_fd when finished with it in alloc_qos_data

[Wei Liu]

On Wed, Feb 10, 2016 at 04:26:24PM +, Ian Campbell wrote:
> Otherwise the fd is leaked. NB the mmap'd memory in the global
> cpu_qos_data[n] is not affected by closing the underlying fd.
> 
> Compile tested only.
> 
> CID: 1055930
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
>  tools/xenmon/xenbaked.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c
> index eacacb0..782f0c1 100644
> --- a/tools/xenmon/xenbaked.c
> +++ b/tools/xenmon/xenbaked.c
> @@ -689,6 +689,7 @@ static void alloc_qos_data(int ncpu)
>  cpu_qos_data[n] = new_qos;
>  }
>  free(dummy);
> +close(qos_fd);
>  new_qos = NULL;
>  }
>  
> -- 
> 2.1.4
> 

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

Re: [Xen-devel] [PATCH 2/2] xenmon: initialise dummy array

[Wei Liu]

On Wed, Feb 10, 2016 at 04:26:25PM +, Ian Campbell wrote:
> This is just used to expand the shared backing file to the expected
> size (whether this is actually necessary I'm not sure). Rather than
> leaking some small amount of the processes' heap set the array to
> zeroes.
> 
> While at it add a check that the malloc succeeded before using the
> result.
> 
> Compile tested only.
> 
> CID: 1056095 (use of uninitialised data)
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
>  tools/xenmon/xenbaked.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c
> index 782f0c1..3d9e0ed 100644
> --- a/tools/xenmon/xenbaked.c
> +++ b/tools/xenmon/xenbaked.c
> @@ -663,6 +663,11 @@ static void alloc_qos_data(int ncpu)
>  }
>  pgsize = getpagesize();
>  dummy = malloc(pgsize);
> +if (!dummy) {
> +PERROR("malloc");
> +exit(EXIT_FAILURE);
> +}
> +memset(dummy, 0, pgsize);
>  
>  for (n=0; n  
> -- 
> 2.1.4
> 

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

Re: [Xen-devel] [PATCH] loadpolicy: only close polFd if it is valid

[Wei Liu]

On Wed, Feb 10, 2016 at 04:32:39PM +, Ian Campbell wrote:
> It can be -1 at this point.
> 
> CID 1055562
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
>  tools/flask/utils/loadpolicy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/flask/utils/loadpolicy.c b/tools/flask/utils/loadpolicy.c
> index 47b5139..76710a2 100644
> --- a/tools/flask/utils/loadpolicy.c
> +++ b/tools/flask/utils/loadpolicy.c
> @@ -115,7 +115,7 @@ done:
>  if ( ret < 0 )
>  fprintf(stderr, "Unable to unmap policy memory: %s\n", 
> strerror(errno));
>  }
> -if ( polFd )
> +if ( polFd >= 0 )
>  close(polFd);
>  if ( xch )
>  xc_interface_close(xch);
> -- 
> 2.1.4
> 

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

Re: [Xen-devel] [PATCH v2] build: specify minimum versions of make and binutils

[Jan Beulich]

>>> On 11.02.16 at 11:59,  wrote:
> On Thu, 2016-02-11 at 03:42 -0700, Jan Beulich wrote:
>> I think it is quite relevant which version is to be picked: Anything
>> older no-one could legitimately report issues against (and I would
>> very much like to continue to be able to submit build fixes I find
>> necessary on those two old boxes I keep for a reason), while
>> stating something too old which even today we don't successfully
>> build with would be pretty odd.
> 
> So lets start with, for x86 whatever the older of the versions on the two
> boxes you refer to above and for ARM gcc 4.8 and binutils 2.24, which
> corresponds to what I happen to use for both arm32 and arm64 in my
> development environment.
> 
> Those are IMHO reasonable starting points but obviously they aren't set in
> stone and can be easily adjusted later if necessary.

Okay, fine with me then (albeit as said I can't guarantee that the
versions on those boxes [which are identical, it's just that one is a
32-bit one while the other is 64-bit] aren't heavily patched
compared to their upstream originals). This would make

binutils 2.16.91.0.5
gcc 4.1.2_20070115

Jan


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

Re: [Xen-devel] [PATCH] init-xenstore-domain: cleanup all resources on a single exit path

[Wei Liu]

On Wed, Feb 10, 2016 at 04:56:22PM +, Ian Campbell wrote:
> Previously xs_fd would be left open, which is CID 1055993 (previously
> partially fixed by 3bca826aae5eb).
> 
> Instead arrange for both success and error cases to cleanup everything
> on a single exit path instead of doing partial cleanup on the success
> path a few operations higher up.
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
>  tools/helpers/init-xenstore-domain.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/helpers/init-xenstore-domain.c 
> b/tools/helpers/init-xenstore-domain.c
> index 5c5af2d..909542b 100644
> --- a/tools/helpers/init-xenstore-domain.c
> +++ b/tools/helpers/init-xenstore-domain.c
> @@ -172,9 +172,6 @@ static int build(xc_interface *xch)
>  goto err;
>  }
>  
> -xc_dom_release(dom);
> -dom = NULL;
> -
>  rv = xc_domain_set_virq_handler(xch, domid, VIRQ_DOM_EXC);
>  if ( rv )
>  {
> @@ -188,14 +185,18 @@ static int build(xc_interface *xch)
>  goto err;
>  }
>  
> -return 0;
> +rv = 0;
>  
>  err:
>  if ( dom )
>  xc_dom_release(dom);
> -if ( domid != ~0 )
> +if ( xs_fd >= 0 )
> +close(xs_fd);
> +
> +/* if we failed then destroy the domain */
> +if ( rv && domid != ~0 )
>  xc_domain_destroy(xch, domid);
> -close(xs_fd);
> +
>  return rv;
>  }
>  
> -- 
> 2.1.4
> 

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

[Xen-devel] [distros-debian-wheezy test] 38737: all pass

[Platform Team regression test user]

flight 38737 distros-debian-wheezy real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/38737/

Perfect :-)
All tests in this flight passed
baseline version:
 flight   38725

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-wheezy-netboot-pvgrub pass
 test-amd64-i386-i386-wheezy-netboot-pvgrub   pass
 test-amd64-i386-amd64-wheezy-netboot-pygrub  pass
 test-amd64-amd64-i386-wheezy-netboot-pygrub  pass



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary


Push not applicable.


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

Re: [Xen-devel] HVMlite ABI specification DRAFT C + implementation outline

[Jan Beulich]

>>> On 11.02.16 at 12:10,  wrote:
>  * PCI Express MMCFG: no MMCFG areas will be mapped by default into the guest
>memory map. In order to have these areas mapped the hardware domain
>must use the PHYSDEVOP_pci_mmcfg_reserved hypercall. On successful return
>from this hypercall the requested MMCFG areas will be mapped 1:1 into the
>guest memory space.

This is too strict: When the address range is reserved in E820, the
hypervisor will happily use that MMCFG, and hence I see no reason
why it then wouldn't also map it for Dom0.

Jan


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

Re: [Xen-devel] [PATCH v2] tools: libxl: make it illegal to pass libxl__realloc(gc) a non-gc ptr

[Wei Liu]

On Thu, Feb 11, 2016 at 09:23:54AM +, Ian Campbell wrote:
> That is, if gc is not NOGC and ptr is not NULL then ptr must be
> associated gc.
> 

"associated with gc"?

Anyway, I get the idea.

> Currently in this case the new_ptr would not be registered with any
> gc, which Coverity rightly points out (in various different places)
> would be a memory leak.
> 
> It would also be possible to fix this by adding a libxl__ptr_add() at
> the same point, however semantically it seems like a programming error
> to gc-realloc a pointer which is not associated with the gc in
> question, so treat it as such.
> 
> Compile tested only, this change could expose latent bugs.
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
> v2: Check we actually didn't find the ptr in the gc
> Correct log message and shorten since LOG will inject the
>function name.
> ---
>  tools/libxl/libxl_internal.c | 4 
>  tools/libxl/libxl_internal.h | 4 +++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index 328046b..fc81130 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -122,6 +122,10 @@ void *libxl__realloc(libxl__gc *gc, void *ptr, size_t 
> new_size)
>  break;
>  }
>  }
> +if (i == gc->alloc_maxsize) {
> +LOG(CRITICAL, "pointer is not tracked by the given gc");
> +abort();
> +}
>  }
>  
>  return new_ptr;
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index fc1b558..650a958 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -617,7 +617,9 @@ _hidden void *libxl__zalloc(libxl__gc *gc_opt, size_t 
> size) NN1;
>  _hidden void *libxl__calloc(libxl__gc *gc_opt, size_t nmemb, size_t size) 
> NN1;
>  /* change the size of the memory block pointed to by @ptr to @new_size bytes.
>   * unlike other allocation functions here any additional space between the
> - * oldsize and @new_size is not initialised (similar to a gc'd realloc(3)). 
> */
> + * oldsize and @new_size is not initialised (similar to a gc'd realloc(3)).
> + * if @ptr is non-NULL and @gc_opt is not nogc_gc then @ptr must have been
> + * registered with @gc_opt previously. */
>  _hidden void *libxl__realloc(libxl__gc *gc_opt, void *ptr, size_t new_size) 
> NN1;
>  /* print @fmt into an allocated string large enoughto contain the result.
>   * (similar to gc'd asprintf(3)). */
> -- 
> 2.1.4
> 

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

[Xen-devel] [PATCH 2/3] xen: sched: add wakeup flags to the scheduler interface

[Dario Faggioli]

For making it possible to pass to the specific scheduler
code information about the nature of the wakeup, and let
it act accordingly, if necessary.

For now, this will only be useful to Credit1, that needs
to differentiate between 'regular' wakeups (happening,
for instance, because an I/O event), and wakeups
"artificially induced" for migrating a vCPU to another
pCPU.

In this patch, only the WF_wakeup flag is introduced, and
used everything. In fact, instead of changing the prototype
of vcpu_wake(), the _vcpu_wake() helper is introduced. This
can change, in case, at some point, the majority of wakeup
paths will end up wanting to pass a flag.

Signed-off-by: Dario Faggioli 
---
Cc: George Dunlap 
Cc: Josh Whitehead 
Cc: Robert VanVossen 
Cc: Meng Xu 
---
 xen/common/sched_arinc653.c |2 +-
 xen/common/sched_credit.c   |2 +-
 xen/common/sched_credit2.c  |2 +-
 xen/common/sched_rt.c   |2 +-
 xen/common/schedule.c   |9 +++--
 xen/include/xen/sched-if.h  |3 ++-
 xen/include/xen/sched.h |7 +++
 7 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 0606988..b8ea596 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -537,7 +537,7 @@ a653sched_vcpu_sleep(const struct scheduler *ops, struct 
vcpu *vc)
  * @param vcPointer to the VCPU structure for the current domain
  */
 static void
-a653sched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
+a653sched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc, unsigned wf)
 {
 if ( AVCPU(vc) != NULL )
 AVCPU(vc)->awake = 1;
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 5708701..f728ddd 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -983,7 +983,7 @@ csched_vcpu_sleep(const struct scheduler *ops, struct vcpu 
*vc)
 }
 
 static void
-csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
+csched_vcpu_wake(const struct scheduler *ops, struct vcpu *vc, unsigned wf)
 {
 struct csched_vcpu * const svc = CSCHED_VCPU(vc);
 const unsigned int cpu = vc->processor;
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 78220a7..a8ba61d 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -940,7 +940,7 @@ csched2_vcpu_sleep(const struct scheduler *ops, struct vcpu 
*vc)
 }
 
 static void
-csched2_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
+csched2_vcpu_wake(const struct scheduler *ops, struct vcpu *vc, unsigned wf)
 {
 struct csched2_vcpu * const svc = CSCHED2_VCPU(vc);
 s_time_t now = 0;
diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
index 2e5430f..4ee1fce 100644
--- a/xen/common/sched_rt.c
+++ b/xen/common/sched_rt.c
@@ -1022,7 +1022,7 @@ out:
  * TODO: what if these two vcpus belongs to the same domain?
  */
 static void
-rt_vcpu_wake(const struct scheduler *ops, struct vcpu *vc)
+rt_vcpu_wake(const struct scheduler *ops, struct vcpu *vc, unsigned wf)
 {
 struct rt_vcpu * const svc = rt_vcpu(vc);
 s_time_t now = NOW();
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index 7306d71..ea74c96 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -406,7 +406,7 @@ void vcpu_sleep_sync(struct vcpu *v)
 sync_vcpu_execstate(v);
 }
 
-void vcpu_wake(struct vcpu *v)
+static void _vcpu_wake(struct vcpu *v, unsigned wake_flags)
 {
 unsigned long flags;
 spinlock_t *lock = vcpu_schedule_lock_irqsave(v, &flags);
@@ -415,7 +415,7 @@ void vcpu_wake(struct vcpu *v)
 {
 if ( v->runstate.state >= RUNSTATE_blocked )
 vcpu_runstate_change(v, RUNSTATE_runnable, NOW());
-SCHED_OP(VCPU2OP(v), wake, v);
+SCHED_OP(VCPU2OP(v), wake, v, wake_flags);
 }
 else if ( !(v->pause_flags & VPF_blocked) )
 {
@@ -428,6 +428,11 @@ void vcpu_wake(struct vcpu *v)
 TRACE_2D(TRC_SCHED_WAKE, v->domain->domain_id, v->vcpu_id);
 }
 
+void vcpu_wake(struct vcpu *v)
+{
+return _vcpu_wake(v, WF_wakeup);
+}
+
 void vcpu_unblock(struct vcpu *v)
 {
 if ( !test_and_clear_bit(_VPF_blocked, &v->pause_flags) )
diff --git a/xen/include/xen/sched-if.h b/xen/include/xen/sched-if.h
index 66dc9c8..ea1cd4a 100644
--- a/xen/include/xen/sched-if.h
+++ b/xen/include/xen/sched-if.h
@@ -144,7 +144,8 @@ struct scheduler {
 void (*remove_vcpu)(const struct scheduler *, struct vcpu *);
 
 void (*sleep)  (const struct scheduler *, struct vcpu *);
-void (*wake)   (const struct scheduler *, struct vcpu *);
+void (*wake)   (const struct scheduler *, struct vcpu *,
+unsigned int);
 void (*yield)  (const struct scheduler *, struct vcpu *);
 void (*context_saved)  (const struct scheduler *, struct vcpu *);
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index b47a3fe..9fdcfff

[Xen-devel] [PATCH 1/3] xen: credit1: trace vCPU boost/unboost

[Dario Faggioli]

Add tracepoints and a performance counter for
boosting and unboosting in Credit1.

Note that they (the trace points) do not cover
the case of the idle vCPU being boosted to run
a tasklet, as there already is
TRC_CSCHED_SCHED_TASKLET for that.

Signed-off-by:  Dario Faggioli 
---
Cc: George Dunlap 
---
 xen/common/sched_credit.c|8 
 xen/include/xen/perfc_defn.h |1 +
 2 files changed, 9 insertions(+)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 671bbee..5708701 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -126,6 +126,8 @@
 #define TRC_CSCHED_STOLEN_VCPU   TRC_SCHED_CLASS_EVT(CSCHED, 4)
 #define TRC_CSCHED_PICKED_CPUTRC_SCHED_CLASS_EVT(CSCHED, 5)
 #define TRC_CSCHED_TICKLETRC_SCHED_CLASS_EVT(CSCHED, 6)
+#define TRC_CSCHED_BOOST_START   TRC_SCHED_CLASS_EVT(CSCHED, 7)
+#define TRC_CSCHED_BOOST_END TRC_SCHED_CLASS_EVT(CSCHED, 8)
 
 
 /*
@@ -856,7 +858,11 @@ csched_vcpu_acct(struct csched_private *prv, unsigned int 
cpu)
  * amount of CPU resources and should no longer be boosted.
  */
 if ( svc->pri == CSCHED_PRI_TS_BOOST )
+{
 svc->pri = CSCHED_PRI_TS_UNDER;
+TRACE_2D(TRC_CSCHED_BOOST_END, svc->sdom->dom->domain_id,
+ svc->vcpu->vcpu_id);
+}
 
 /*
  * Update credits
@@ -1023,6 +1029,8 @@ csched_vcpu_wake(const struct scheduler *ops, struct vcpu 
*vc)
 if ( svc->pri == CSCHED_PRI_TS_UNDER &&
  !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
 {
+TRACE_2D(TRC_CSCHED_BOOST_START, vc->domain->domain_id, vc->vcpu_id);
+SCHED_STAT_CRANK(vcpu_boost);
 svc->pri = CSCHED_PRI_TS_BOOST;
 }
 
diff --git a/xen/include/xen/perfc_defn.h b/xen/include/xen/perfc_defn.h
index 76ee803..21c1e0b 100644
--- a/xen/include/xen/perfc_defn.h
+++ b/xen/include/xen/perfc_defn.h
@@ -40,6 +40,7 @@ PERFCOUNTER(acct_reorder,   "csched: acct_reorder")
 PERFCOUNTER(acct_min_credit,"csched: acct_min_credit")
 PERFCOUNTER(acct_vcpu_active,   "csched: acct_vcpu_active")
 PERFCOUNTER(acct_vcpu_idle, "csched: acct_vcpu_idle")
+PERFCOUNTER(vcpu_boost, "csched: vcpu_boost")
 PERFCOUNTER(vcpu_park,  "csched: vcpu_park")
 PERFCOUNTER(vcpu_unpark,"csched: vcpu_unpark")
 PERFCOUNTER(load_balance_idle,  "csched: load_balance_idle")


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

[Xen-devel] [PATCH 3/3] xen: credit1: avoid boosting vCPUs being "just" migrated

[Dario Faggioli]

Moving a vCPU to a different pCPU means blocking it and
waking it up (on the new pCPU). Credit1 grants BOOST
priority to vCPUs that wakes up, with the aim of improving
I/O latency. The end result is that vCPUs get boosted when
migrating, which shouldn't happen.

For instance, this causes scheduling anomalies and,
potentially, performance problems, as reported here:
 http://lists.xen.org/archives/html/xen-devel/2015-10/msg02851.html

This patch fixes things by introducing a new wakeup flag,
and using it to tag the wakeups that happens because of
migrations (and avoid boosting, in these cases).

Signed-off-by: Dario Faggioli 
---
Cc: George Dunlap 
---
 xen/common/sched_credit.c |   11 +++
 xen/common/schedule.c |4 ++--
 xen/include/xen/sched.h   |3 +++
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index f728ddd..2a23a0b 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1022,11 +1022,14 @@ csched_vcpu_wake(const struct scheduler *ops, struct 
vcpu *vc, unsigned wf)
  * more CPU resource intensive VCPUs without impacting overall 
  * system fairness.
  *
- * The one exception is for VCPUs of capped domains unpausing
- * after earning credits they had overspent. We don't boost
- * those.
+ * There are a couple of exceptions, when we don't want to boost:
+ *  - VCPUs that are waking up after a migration, rather than
+ *after having block;
+ *  - VCPUs of capped domains unpausing after earning credits
+ *they had overspent.
  */
-if ( svc->pri == CSCHED_PRI_TS_UNDER &&
+if ( !(wf & WF_migrated) &&
+ svc->pri == CSCHED_PRI_TS_UNDER &&
  !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
 {
 TRACE_2D(TRC_CSCHED_BOOST_START, vc->domain->domain_id, vc->vcpu_id);
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index ea74c96..c9a4f52 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -581,8 +581,8 @@ static void vcpu_migrate(struct vcpu *v)
 if ( old_cpu != new_cpu )
 sched_move_irqs(v);
 
-/* Wake on new CPU. */
-vcpu_wake(v);
+/* Wake on new CPU  (and let the scheduler know it's a migration). */
+_vcpu_wake(v, WF_migrated);
 }
 
 /*
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 9fdcfff..5f426ad 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -764,6 +764,9 @@ static inline struct domain *next_domain_in_cpupool(
 /* 'Default' wakeup. */
 #define _WF_wakeup   0
 #define WF_wakeup(1U<<_WF_wakeup)
+/* Post-migration wakeup. */
+#define _WF_migrated 1
+#define WF_migrated  (1U<<_WF_migrated)
 
 static inline int vcpu_runnable(struct vcpu *v)
 {


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

[Xen-devel] [PATCH 0/3] xen: sched: Credit1 shouldn't boost vcpus being migrated.

[Dario Faggioli]

Hi,

This series aims at stopping the Credit1 scheduler from granting BOOST priority
to vcpus that are migrated to a new pcpu.

In fact, BOOST is for vcpus that are waking up after, e.g., having received an
event, to, well, "boost" their wakeup latency and get them quickly to handle
the event itself. However, since vcpu_migrate() basically forces the vcpu into
a sleep+wakeup cycle, vcpus being migrated to a new pcpu, were also being
granted BOOST priority, and that is not correct.

Performance does not seem to be affected much, neither in good or bad way, by
this series. In fact, I've run a few benchmarks:
 - I've considered both a Xen build, and the Iperf network benchmark, running
   either inside 1 or 2 (HVM) VMs;
 - VMs had 12 vcpus, on a 16 pcpus host. Therefore, when only 1 VM is used, the
   host has some "spare" pcpus, while when 2 VMs where used, the host was
   overloaded;
 - each configuration was repeated 10 times, and I'm showing average and
   standard deviation;
 - I've considered the following configurations:
   1) `make xen' run inside 1 VM
   2) `make xen' run, concurrently, inside 2 VMs
   3) Iperf run inside 1 VM
   4) Iperf run, concurretnly, inside 2 VMs
   5) `make xen' and Iperf run, concurrently, each one inside one of the VMs

*** *** ***
1) XEN BUILD INSIDE 1 HVM GUEST (lower is better)
---
baseline :  20.673+/-0.263
patched  :  20.812+/-0.797

2) XEN BUILD INSIDE 2 HVM GUESTS, CONCURRENTLY (lower is better)
--
vm1 vm2 overall
baseline :  35.641+/-1.236  35.104+/-0.392  35.372+/-0.934
patched  :  35.251+/-0.527  35.341+/-0.811  35.296+/-0.667

3) IPERF FROM 1 HVM GUEST TO HOST (higher is better)
-
baseline :  25.25+/-0.63
patched  :  25.56+/-1.90

4) IPERF FROM 2 HVM GUESTS TO HOST, CONCURRENTLY (lower is better)

vm1vm2   overall
baseline :  13.01+/-1.27   14.25+/-0.97  13.63+/-1.27
patched  :  13.29+/-1.87   14.11+/-1.48  13.70+/-1.69

5) MAKEXEN & IPERF IN ONE HVM GUEST EACH, CONCURRENTLY
--
   makexen  iperf
baseline : 30.483+/-0.843   14.92+/-1.59
patched  : 30.885+/-0.882   16.42+/-1.74
*** *** ***

So, basically:
 * 1 and 2 shows that we negligibly regress on a Xen build done in underload
   conditions, as well as we negligibly improve on a Xen build (well multiple
   Xen builds) done in overload.
 * 3 and 4, show negligible to small improvement in Iperf performance, with the
   series applied.
 * 5 shows that, in an heterogeneous workload, including both cpu load and I/O,
   the CPU load is slightly penalized by the series, while the I/O load benefits
   from it. This can probably be explained by the fact that, without this 
series,
   the CPU hungry vcpus were being boosted from time to time, even if they
   shouldn't have been, and this was impacting the vcpus doing I/O.

If anyone can and/or want to run more benchmarks (and/or include this patch in
benchmarks that you routinely run... Marcus, Malcolm, Andrew, maybe?), I'd be
interested in seeing the results.


In any case, I think non-boosting on migration is, conceptually, the right
thing to do, and that's what is done in patch 3.

In order to be able to tell, inside Credit1 code, whether a vcpu is waking up
after being blocked/asleep, or because of a migration, the wake scheduler hook
is augmented (in patch 2) with a flag field. The caller of the hook can
specify, by means of a set of flags, what has been the actual cause of the
wakeup. Right now, after all the series is applied, only two flags exists:
'regular wakeups', and 'migration wakeups'. The mechanism is extensible, and we
can add more i future, it there will come the need for this.

While there, the series introduces (patch 1) also some tracing and a new
performance counter about Credit1 boosting logic, for making it easier to
identify issues like this in future.

Thanks and Regards,
Dario
---
Dario Faggioli (3):
  xen: credit1: trace vCPU boost/unboost
  xen: sched: add wakeup flags to the scheduler interface
  xen: credit1: avoid boosting vCPUs being "just" migrated

 xen/common/sched_arinc653.c  |2 +-
 xen/common/sched_credit.c|   21 -
 xen/common/sched_credit2.c   |2 +-
 xen/common/sched_rt.c|2 +-
 xen/common/schedule.c|   13 +
 xen/include/xen/perfc_defn.h |1 +
 xen/include/xen/sched-if.h   |3 ++-
 xen/include/xen/sched.h  |   10 ++
 8 files changed, 41 insertions(+), 13 deletions(-)
-- 
<> (Raistlin Majere)
---
Dario Faggioli, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
PhD Candidate, ReTiS Lab, Scuola Superiore Sant'Anna, Pisa (Italy)

_

[Xen-devel] [xen-unstable baseline-only test] 38736: tolerable FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 38736 xen-unstable real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/38736/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-amd64-rumpuserxen   6 xen-buildfail   like 38729
 build-i386-rumpuserxen6 xen-buildfail   like 38729
 test-amd64-amd64-xl-credit2  19 guest-start/debian.repeatfail   like 38729
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 38729

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass

version targeted for testing:
 xen  483ad4439f7fc71e12d46dae516f2b9ab2b977ad
baseline version:
 xen  1ac81bb7166b79b6555290547d4e305c74d0

Last test of basis38729  2016-02-07 18:50:01 Z3 days
Testing same since38736  2016-02-11 01:54:56 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  Dario Faggioli 
  George Dunlap 
  Ian Campbell 
  Juergen Gross 
  Razvan Cojocaru 
  Tamas K Lengyel 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-oldkern  pass
 build-i386-oldkern   pass
 build-amd64-prev pass
 

[Xen-devel] [PATCH v2] tools: probe for existence of qemu-xen stderr trace backend.

[Ian Campbell]

QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
log") renamed the "stderr" trace backend to "log", which breaks the
xen build when pointed at a QEMU tree after that point:

./configure of QEMU fail with:
"ERROR: invalid trace backends
Please choose supported trace backends."

Upstream also changed, in baf86d6b3ca0 ("trace: switch default backend
to "log""), to use "log" as the default backend (previously it was
"nop").

Use ./scripts/tracetool.py to check for the presence of the stderr
backend and if it is present then explicitly enable it. If the stderr
backend is not present then assume a newer QEMU which defaults to
"log" and simply accept that default (there is a 1 commit window
upstream where this would result in no trace backend being enabled).

The check is done using the older (deprecated?) --check-backend/--backend
variant of the tracetool.py options rather than the new plural
versions since the singular was supported even by very old versions of
QEMU.  New QEMU has compatibility code but if/when that is removed we
will still do the right thing i.e. no explict configuiration resulting
in the upstream default (currently "log").

If the explicit selection of the "stderr" backend is required then it
is now done unconditionally (not depending on debug=y), which is
simpler to arrange here but also matches the newer upstream's default
to "log" which is not conditional on debug being enabled either.

Tested with current qemu-xen-unstable (e9d8252) and current QEMU
upstream master (88c73d1), both out of tree via
QEMU_UPSTREAM_URL=/path/to/qemu-xen.git.

Signed-off-by: Ian Campbell 
Cc: Paul Durrant 
Cc: Anthony PERARD 
---
v2: Just probe for stderr, otherwise assume the new upstream default
of log is what we will get anyway. Substantially rewrite commit
message.
---
 tools/Makefile | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/Makefile b/tools/Makefile
index cb8652c..3f45fb9 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -228,7 +228,7 @@ qemu-xen-dir-force-update: qemu-xen-dir-find
fi
 
 ifeq ($(debug),y)
-QEMU_XEN_ENABLE_DEBUG := --enable-debug --enable-trace-backend=stderr
+QEMU_XEN_ENABLE_DEBUG := --enable-debug
 else
 QEMU_XEN_ENABLE_DEBUG :=
 endif
@@ -240,8 +240,14 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
source=.; \
fi; \
cd qemu-xen-dir; \
+   if $$source/scripts/tracetool.py --check-backend --backend stderr ; 
then \
+   enable_trace_backend='--enable-trace-backend=stderr'; \
+   else \
+   enable_trace_backend='' ; \
+   fi ; \
$$source/configure --enable-xen --target-list=i386-softmmu \
$(QEMU_XEN_ENABLE_DEBUG) \
+   $$enable_trace_backend \
--prefix=$(LIBEXEC) \
--libdir=$(LIBEXEC_LIB) \
--includedir=$(LIBEXEC_INC) \
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v2 2/2] x86/traps: Dump instruction stream in show_execution_state()

[Andrew Cooper]

On 11/02/16 11:16, Jan Beulich wrote:
 On 11.02.16 at 11:52,  wrote:
>> v2: Deal with %rip wrapping.  In such a case, insert dashes into printed 
>> line.
> I don't think this deals with wrapping now, since ...
>
>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -114,6 +114,56 @@ boolean_param("ler", opt_ler);
>>  #define stack_words_per_line 4
>>  #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
>>  
>> +static void show_code(const struct cpu_user_regs *regs)
>> +{
>> +unsigned char insns_before[8], insns_after[16];
>> +unsigned int i, missing_before, missing_after;
>> +
>> +if ( guest_mode(regs) )
>> +return;
>> +
>> +/*
>> + * This dance with {insns,missing}_{before,after} is to ensure that, if
>> + * %rip -8/+16 wraps around a bounday, we read a non-wrapped regs->rip
>> + * pointer, and calculate which bytes were not read so they may be
>> + * replaced with dashes in the printed output.
>> + */
>> +missing_before = __copy_from_user(
>> +insns_before, (void __user *)regs->rip - 8, 
>> ARRAY_SIZE(insns_before));
>> +missing_after = __copy_from_user(
>> +insns_after, (void __user *)regs->rip, ARRAY_SIZE(insns_after));
> ... iirc __copy_from_user() doesn't range check the addresses.
> Also reading the leading bytes is done in kind of a strange way: It'll
> read initial bytes (farther away from RIP) and perhaps not read
> later ones (closer to RIP), albeit clearly the ones closer are of more
> interest. In the extreme case, where RIP is only a few bytes into a
> page following an unmapped one, no leading bytes would be printed
> at all despite some actually being readable.

I think in this specific case, it might be best to hand roll some asm
using rep movs and the direction flag, with manual fault handling.

>
> Avoiding actual wrapping could be easily done by extending the
> guest_mode() check above to also range check RIP against the
> hypervisor image boundaries (post-boot this could even be limited
> further, but perhaps using the full XEN_VIRT_{START,END} range is
> the better route with xSplice in mind.

I would like, where possible, to avoid omitting the instruction stream
if Xen is outside of its expected boundaries.

~Andrew

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

Re: [Xen-devel] [PATCH v2 1/6] xen: factor out p2m list allocation into separate function

[Daniel Kiper]

On Thu, Feb 11, 2016 at 08:53:21AM +0100, Juergen Gross wrote:
> Do the p2m list allocation of the to be loaded kernel in a separate
> function. This will allow doing the p2m list allocation at different
> times of the boot preparations depending on the features the kernel
> is supporting.
>
> While at this remove superfluous setting of first_p2m_pfn and
> nr_p2m_frames as those are needed only in case of the p2m list not
> being mapped by the initial kernel mapping.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/loader/i386/xen.c | 70 
> ++---
>  1 file changed, 40 insertions(+), 30 deletions(-)
>
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index c4d9689..42ed7c7 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -52,6 +52,8 @@ static struct grub_xen_file_info xen_inf;
>  static struct xen_multiboot_mod_list *xen_module_info_page;
>  static grub_uint64_t modules_target_start;
>  static grub_size_t n_modules;
> +static struct grub_relocator_xen_state state;
> +static grub_xen_mfn_t *virt_mfn_list;

Do we strongly need this as globals? I suppose that
both of them could be local to grub_xen_boot.

>  #define PAGE_SIZE 4096
>  #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
> @@ -166,7 +168,7 @@ generate_page_table (grub_uint64_t *where, grub_uint64_t 
> paging_start,
>  }
>
>  static grub_err_t
> -set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t pfn)
> +set_mfns (grub_xen_mfn_t pfn)
>  {
>grub_xen_mfn_t i, t;
>grub_xen_mfn_t cn_pfn = -1, st_pfn = -1;
> @@ -175,32 +177,32 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t 
> pfn)
>
>for (i = 0; i < grub_xen_start_page_addr->nr_pages; i++)
>  {
> -  if (new_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn)
> +  if (virt_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn)
>   cn_pfn = i;
> -  if (new_mfn_list[i] == grub_xen_start_page_addr->store_mfn)
> +  if (virt_mfn_list[i] == grub_xen_start_page_addr->store_mfn)
>   st_pfn = i;
>  }
>if (cn_pfn == (grub_xen_mfn_t)-1)
>  return grub_error (GRUB_ERR_BUG, "no console");
>if (st_pfn == (grub_xen_mfn_t)-1)
>  return grub_error (GRUB_ERR_BUG, "no store");
> -  t = new_mfn_list[pfn];
> -  new_mfn_list[pfn] = new_mfn_list[cn_pfn];
> -  new_mfn_list[cn_pfn] = t;
> -  t = new_mfn_list[pfn + 1];
> -  new_mfn_list[pfn + 1] = new_mfn_list[st_pfn];
> -  new_mfn_list[st_pfn] = t;
> -
> -  m2p_updates[0].ptr = page2offset (new_mfn_list[pfn]) | MMU_MACHPHYS_UPDATE;
> +  t = virt_mfn_list[pfn];
> +  virt_mfn_list[pfn] = virt_mfn_list[cn_pfn];
> +  virt_mfn_list[cn_pfn] = t;
> +  t = virt_mfn_list[pfn + 1];
> +  virt_mfn_list[pfn + 1] = virt_mfn_list[st_pfn];
> +  virt_mfn_list[st_pfn] = t;
> +
> +  m2p_updates[0].ptr = page2offset (virt_mfn_list[pfn]) | 
> MMU_MACHPHYS_UPDATE;
>m2p_updates[0].val = pfn;
>m2p_updates[1].ptr =
> -page2offset (new_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE;
> +page2offset (virt_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE;
>m2p_updates[1].val = pfn + 1;
>m2p_updates[2].ptr =
> -page2offset (new_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE;
> +page2offset (virt_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE;
>m2p_updates[2].val = cn_pfn;
>m2p_updates[3].ptr =
> -page2offset (new_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE;
> +page2offset (virt_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE;
>m2p_updates[3].val = st_pfn;
>
>grub_xen_mmu_update (m2p_updates, 4, NULL, DOMID_SELF);
> @@ -209,34 +211,43 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t 
> pfn)
>  }
>
>  static grub_err_t
> +grub_xen_p2m_alloc (void)
> +{
> +  grub_relocator_chunk_t ch;
> +  grub_size_t p2msize;
> +  grub_err_t err;
> +
> +  state.mfn_list = max_addr;
> +  next_start.mfn_list = max_addr + xen_inf.virt_base;
> +  p2msize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages;
> +  err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, p2msize);

Hmmm Where relocator is defined?

> +  if (err)
> +return err;
> +  virt_mfn_list = get_virtual_current_address (ch);
> +  grub_memcpy (virt_mfn_list,
> +(void *) grub_xen_start_page_addr->mfn_list, p2msize);
> +  max_addr = ALIGN_UP (max_addr + p2msize, PAGE_SIZE);
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
>  grub_xen_boot (void)
>  {
> -  struct grub_relocator_xen_state state;
>grub_relocator_chunk_t ch;
>grub_err_t err;
> -  grub_size_t pgtsize;
>struct start_info *nst;
>grub_uint64_t nr_info_pages;
>grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
>struct gnttab_set_version gnttab_setver;
> -  grub_xen_mfn_t *new_mfn_list;
>grub_size_t i;
>
>if (grub_xen_n_allocated_shared_pages)
>  return grub_error (GRUB_ERR_BUG, "active grants");
>
> -  state.mfn_list = max_addr;
> -  next_start.mfn_list = max_addr + xen_inf.virt_base;

[Xen-devel] [PATCH v3 1/2] build: specify minimum versions of gcc and binutils

[Ian Campbell]

From: Doug Goldstein 

To help people avoid having to figure out what versions of gcc and
binutils need to be supported document them explicitly.

Signed-off-by: Doug Goldstein 
Signed-off-by: Ian Campbell 
---
v3: [ijc] Updated version for binutils and include gcc (separately for
x86 and ARM in both cases), deferred make to second patch
---
 README | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/README b/README
index 1324c7c..356a350 100644
--- a/README
+++ b/README
@@ -35,9 +35,13 @@ Second, there are a number of prerequisites for building a 
Xen source
 release. Make sure you have all the following installed, either by
 visiting the project webpage or installing a pre-built package
 provided by your OS distributor:
-* GCC v4.1 or later
+* GCC
+  - For x86 4.1.2_20070115 or later
+  - For ARM 4.8 or later
 * GNU Make
-* GNU Binutils
+* GNU Binutils:
+  - For x86 2.16.91.0.5 or later
+  - For ARM 2.24 or later
 * Development install of zlib (e.g., zlib-dev)
 * Development install of Python v2.3 or later (e.g., python-dev)
 * Development install of curses (e.g., libncurses-dev)
-- 
2.1.4


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

[Xen-devel] [PATCH v3 2/2] build: specify minimum versions of make

[Ian Campbell]

From: Doug Goldstein 

To help people avoid having to figure out what versions of make
needs to be supported document it explicitly.

Signed-off-by: Doug Goldstein 
Signed-off-by: Ian Campbell 
---
v3: [ijc] Put make into a separate patch to reduce risk to
gcc+binutils patch #1 (since I'm not sure if v3.80 was really agreed)
---
 README | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README b/README
index 356a350..dd36ec8 100644
--- a/README
+++ b/README
@@ -38,7 +38,7 @@ provided by your OS distributor:
 * GCC
   - For x86 4.1.2_20070115 or later
   - For ARM 4.8 or later
-* GNU Make
+* GNU Make v3.80 or later
 * GNU Binutils:
   - For x86 2.16.91.0.5 or later
   - For ARM 2.24 or later
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v2 2/6] xen: factor out allocation of special pages into separate function

[Daniel Kiper]

On Thu, Feb 11, 2016 at 08:53:22AM +0100, Juergen Gross wrote:
> Do the allocation of special pages (start info, console and xenbus
> ring buffers) in a separate function. This will allow to do the
> allocation at different times of the boot preparations depending on
> the features the kernel is supporting.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/loader/i386/xen.c | 50 
> +
>  1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index 42ed7c7..e48cc3f 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -54,6 +54,8 @@ static grub_uint64_t modules_target_start;
>  static grub_size_t n_modules;
>  static struct grub_relocator_xen_state state;
>  static grub_xen_mfn_t *virt_mfn_list;
> +static struct start_info *virt_start_info;
> +static grub_xen_mfn_t console_pfn;

Same as in patch #1.

Daniel

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

Re: [Xen-devel] [PATCH v2 3/6] xen: factor out allocation of page tables into separate function

[Daniel Kiper]

On Thu, Feb 11, 2016 at 08:53:23AM +0100, Juergen Gross wrote:
> Do the allocation of page tables in a separate function. This will
> allow to do the allocation at different times of the boot preparations
> depending on the features the kernel is supporting.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/loader/i386/xen.c | 82 
> -
>  1 file changed, 51 insertions(+), 31 deletions(-)
>
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index e48cc3f..65cec27 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -56,6 +56,9 @@ static struct grub_relocator_xen_state state;
>  static grub_xen_mfn_t *virt_mfn_list;
>  static struct start_info *virt_start_info;
>  static grub_xen_mfn_t console_pfn;
> +static grub_uint64_t *virt_pgtable;
> +static grub_uint64_t pgtbl_start;
> +static grub_uint64_t pgtbl_end;

Same as in patches #1 and #2.

>  #define PAGE_SIZE 4096
>  #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
> @@ -106,17 +109,17 @@ get_pgtable_size (grub_uint64_t total_pages, 
> grub_uint64_t virt_base)
>
>  static void
>  generate_page_table (grub_uint64_t *where, grub_uint64_t paging_start,
> -  grub_uint64_t total_pages, grub_uint64_t virt_base,
> -  grub_xen_mfn_t *mfn_list)
> +  grub_uint64_t paging_end, grub_uint64_t total_pages,
> +  grub_uint64_t virt_base, grub_xen_mfn_t *mfn_list)
>  {
>if (!virt_base)
> -total_pages++;
> +paging_end++;
>
>grub_uint64_t lx[NUMBER_OF_LEVELS], lxs[NUMBER_OF_LEVELS];
>grub_uint64_t nlx, nls, sz = 0;
>int l;
>
> -  nlx = total_pages;
> +  nlx = paging_end;
>nls = virt_base >> PAGE_SHIFT;
>for (l = 0; l < NUMBER_OF_LEVELS; l++)
>  {
> @@ -160,7 +163,7 @@ generate_page_table (grub_uint64_t *where, grub_uint64_t 
> paging_start,
>if (pr)
>  pg += POINTERS_PER_PAGE;
>
> -  for (j = 0; j < total_pages; j++)
> +  for (j = 0; j < paging_end; j++)
>  {
>if (j >= paging_start && j < lp)
>   pg[j + lxs[0]] = page2offset (mfn_list[j]) | 5;
> @@ -261,24 +264,12 @@ grub_xen_special_alloc (void)
>  }
>
>  static grub_err_t
> -grub_xen_boot (void)
> +grub_xen_pt_alloc (void)
>  {
>grub_relocator_chunk_t ch;
>grub_err_t err;
>grub_uint64_t nr_info_pages;
>grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
> -  struct gnttab_set_version gnttab_setver;
> -  grub_size_t i;
> -
> -  if (grub_xen_n_allocated_shared_pages)
> -return grub_error (GRUB_ERR_BUG, "active grants");
> -
> -  err = grub_xen_p2m_alloc ();
> -  if (err)
> -return err;
> -  err = grub_xen_special_alloc ();
> -  if (err)
> -return err;
>
>next_start.pt_base = max_addr + xen_inf.virt_base;
>state.paging_start = max_addr >> PAGE_SHIFT;
> @@ -298,30 +289,59 @@ grub_xen_boot (void)
>nr_pages = nr_need_pages;
>  }
>
> -  grub_dprintf ("xen", "bootstrap domain %llx+%llx\n",
> - (unsigned long long) xen_inf.virt_base,
> - (unsigned long long) page2offset (nr_pages));
> -
>err = grub_relocator_alloc_chunk_addr (relocator, &ch,
>max_addr, page2offset (nr_pt_pages));
>if (err)
>  return err;
>
> +  virt_pgtable = get_virtual_current_address (ch);
> +  pgtbl_start = max_addr >> PAGE_SHIFT;
> +  max_addr += page2offset (nr_pt_pages);
> +  state.stack = max_addr + STACK_SIZE + xen_inf.virt_base;
> +  state.paging_size = nr_pt_pages;
> +  next_start.nr_pt_frames = nr_pt_pages;
> +  max_addr = page2offset (nr_pages);
> +  pgtbl_end = nr_pages;
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_xen_boot (void)
> +{
> +  grub_err_t err;
> +  grub_uint64_t nr_pages;
> +  struct gnttab_set_version gnttab_setver;
> +  grub_size_t i;
> +
> +  if (grub_xen_n_allocated_shared_pages)
> +return grub_error (GRUB_ERR_BUG, "active grants");
> +
> +  err = grub_xen_p2m_alloc ();
> +  if (err)
> +return err;
> +  err = grub_xen_special_alloc ();
> +  if (err)
> +return err;
> +  err = grub_xen_pt_alloc ();
> +  if (err)
> +return err;

Should not we free memory allocated by grub_xen_p2m_alloc() and
grub_xen_special_alloc() if grub_xen_pt_alloc() failed?

Daniel

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

Re: [Xen-devel] [PATCH v3 2/2] build: specify minimum versions of make

[Andrew Cooper]

On 11/02/16 12:23, Ian Campbell wrote:
> From: Doug Goldstein 
>
> To help people avoid having to figure out what versions of make
> needs to be supported document it explicitly.
>
> Signed-off-by: Doug Goldstein 
> Signed-off-by: Ian Campbell 

Reviewed-by: Andrew Cooper 

In particular, I have had changes rejected before for using v3.81'isms,
whereas we have plenty of v3.80'isms

> ---
> v3: [ijc] Put make into a separate patch to reduce risk to
> gcc+binutils patch #1 (since I'm not sure if v3.80 was really agreed)
> ---
>  README | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/README b/README
> index 356a350..dd36ec8 100644
> --- a/README
> +++ b/README
> @@ -38,7 +38,7 @@ provided by your OS distributor:
>  * GCC
>- For x86 4.1.2_20070115 or later
>- For ARM 4.8 or later
> -* GNU Make
> +* GNU Make v3.80 or later
>  * GNU Binutils:
>- For x86 2.16.91.0.5 or later
>- For ARM 2.24 or later


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

Re: [Xen-devel] [PATCH v3 1/2] build: specify minimum versions of gcc and binutils

[Andrew Cooper]

On 11/02/16 12:23, Ian Campbell wrote:
> From: Doug Goldstein 
>
> To help people avoid having to figure out what versions of gcc and
> binutils need to be supported document them explicitly.
>
> Signed-off-by: Doug Goldstein 
> Signed-off-by: Ian Campbell 

I would be tempted to abbreviate this to GCC 4.1 and Binutls 2.16,
unless we specifically know of issues between the release and point
releases.

Either way, Reviewed-by: Andrew Cooper 

> ---
> v3: [ijc] Updated version for binutils and include gcc (separately for
> x86 and ARM in both cases), deferred make to second patch
> ---
>  README | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/README b/README
> index 1324c7c..356a350 100644
> --- a/README
> +++ b/README
> @@ -35,9 +35,13 @@ Second, there are a number of prerequisites for building a 
> Xen source
>  release. Make sure you have all the following installed, either by
>  visiting the project webpage or installing a pre-built package
>  provided by your OS distributor:
> -* GCC v4.1 or later
> +* GCC
> +  - For x86 4.1.2_20070115 or later
> +  - For ARM 4.8 or later
>  * GNU Make
> -* GNU Binutils
> +* GNU Binutils:
> +  - For x86 2.16.91.0.5 or later
> +  - For ARM 2.24 or later
>  * Development install of zlib (e.g., zlib-dev)
>  * Development install of Python v2.3 or later (e.g., python-dev)
>  * Development install of curses (e.g., libncurses-dev)


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

Re: [Xen-devel] [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping

[Daniel Kiper]

On Thu, Feb 11, 2016 at 08:53:24AM +0100, Juergen Gross wrote:
> Modern pvops linux kernels support an initrd not covered by the initial
> mapping. This capability is flagged by an elf-note.
>
> In case the elf-note is set by the kernel don't place the initrd into
> the initial mapping. This will allow to load larger initrds and/or
> support domains with larger memory, as the initial mapping is limited
> to 2GB and it is containing the p2m list.
>
> Signed-off-by: Juergen Gross 
> ---
>  grub-core/loader/i386/xen.c| 56 
> ++
>  grub-core/loader/i386/xen_fileXX.c |  3 ++
>  include/grub/xen_file.h|  1 +
>  3 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> index 65cec27..0f41048 100644
> --- a/grub-core/loader/i386/xen.c
> +++ b/grub-core/loader/i386/xen.c
> @@ -307,15 +307,14 @@ grub_xen_pt_alloc (void)
>  }
>
>  static grub_err_t
> -grub_xen_boot (void)
> +grub_xen_alloc_end (void)
>  {
>grub_err_t err;
> -  grub_uint64_t nr_pages;
> -  struct gnttab_set_version gnttab_setver;
> -  grub_size_t i;
> +  static int called = 0;
>
> -  if (grub_xen_n_allocated_shared_pages)
> -return grub_error (GRUB_ERR_BUG, "active grants");
> +  if (called)
> +return GRUB_ERR_NONE;

Why?

> +  called = 1;
>
>err = grub_xen_p2m_alloc ();
>if (err)
> @@ -327,6 +326,24 @@ grub_xen_boot (void)
>if (err)
>  return err;
>
> +  return GRUB_ERR_NONE;
> +}
> +
> +static grub_err_t
> +grub_xen_boot (void)
> +{
> +  grub_err_t err;
> +  grub_uint64_t nr_pages;
> +  struct gnttab_set_version gnttab_setver;
> +  grub_size_t i;
> +
> +  if (grub_xen_n_allocated_shared_pages)
> +return grub_error (GRUB_ERR_BUG, "active grants");

Why?

> +  err = grub_xen_alloc_end ();
> +  if (err)
> +return err;
> +
>err = set_mfns (console_pfn);
>if (err)
>  return err;
> @@ -587,6 +604,13 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> ((unused)),
>goto fail;
>  }
>
> +  if (xen_inf.unmapped_initrd)
> +{
> +  err = grub_xen_alloc_end ();
> +  if (err)
> +goto fail;
> +}
> +
>if (grub_initrd_init (argc, argv, &initrd_ctx))
>  goto fail;
>
> @@ -603,13 +627,22 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> ((unused)),
>   goto fail;
>  }
>
> -  next_start.mod_start = max_addr + xen_inf.virt_base;
> -  next_start.mod_len = size;
> -
> -  max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE);
> +  if (xen_inf.unmapped_initrd)
> +{
> +  next_start.flags |= SIF_MOD_START_PFN;
> +  next_start.mod_start = max_addr >> PAGE_SHIFT;
> +  next_start.mod_len = size;
> +}
> +  else
> +{
> +  next_start.mod_start = max_addr + xen_inf.virt_base;
> +  next_start.mod_len = size;
> +}
>
>grub_dprintf ("xen", "Initrd, addr=0x%x, size=0x%x\n",
> - (unsigned) next_start.mod_start, (unsigned) size);
> + (unsigned) (max_addr + xen_inf.virt_base), (unsigned) size);
> +
> +  max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE);
>
>  fail:
>grub_initrd_close (&initrd_ctx);
> @@ -660,6 +693,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ 
> ((unused)),
>
>if (!xen_module_info_page)
>  {
> +  xen_inf.unmapped_initrd = 0;
>n_modules = 0;
>max_addr = ALIGN_UP (max_addr, PAGE_SIZE);
>modules_target_start = max_addr;
> diff --git a/grub-core/loader/i386/xen_fileXX.c 
> b/grub-core/loader/i386/xen_fileXX.c
> index 1ba5649..69fccd2 100644
> --- a/grub-core/loader/i386/xen_fileXX.c
> +++ b/grub-core/loader/i386/xen_fileXX.c
> @@ -253,6 +253,9 @@ parse_note (grub_elf_t elf, struct grub_xen_file_info *xi,
> descsz == 2 ? 2 : 3) == 0)
>   xi->arch = GRUB_XEN_FILE_I386;
> break;
> + case 16:

Could you define this a constant and use it here?

Daniel

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

Re: [Xen-devel] [PATCH v2 1/6] xen: factor out p2m list allocation into separate function

[Juergen Gross]

On 11/02/16 13:19, Daniel Kiper wrote:
> On Thu, Feb 11, 2016 at 08:53:21AM +0100, Juergen Gross wrote:
>> Do the p2m list allocation of the to be loaded kernel in a separate
>> function. This will allow doing the p2m list allocation at different
>> times of the boot preparations depending on the features the kernel
>> is supporting.
>>
>> While at this remove superfluous setting of first_p2m_pfn and
>> nr_p2m_frames as those are needed only in case of the p2m list not
>> being mapped by the initial kernel mapping.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  grub-core/loader/i386/xen.c | 70 
>> ++---
>>  1 file changed, 40 insertions(+), 30 deletions(-)
>>
>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>> index c4d9689..42ed7c7 100644
>> --- a/grub-core/loader/i386/xen.c
>> +++ b/grub-core/loader/i386/xen.c
>> @@ -52,6 +52,8 @@ static struct grub_xen_file_info xen_inf;
>>  static struct xen_multiboot_mod_list *xen_module_info_page;
>>  static grub_uint64_t modules_target_start;
>>  static grub_size_t n_modules;
>> +static struct grub_relocator_xen_state state;
>> +static grub_xen_mfn_t *virt_mfn_list;
> 
> Do we strongly need this as globals? I suppose that
> both of them could be local to grub_xen_boot.

This would require passing the state pointer to many other functions.
Same applies to virt_mfn_list.

I just followed the style used in the source already: variables used in
multiple functions are mostly defined globally (there are even some
which are used in one function only).

> 
>>  #define PAGE_SIZE 4096
>>  #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
>> @@ -166,7 +168,7 @@ generate_page_table (grub_uint64_t *where, grub_uint64_t 
>> paging_start,
>>  }
>>
>>  static grub_err_t
>> -set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t pfn)
>> +set_mfns (grub_xen_mfn_t pfn)
>>  {
>>grub_xen_mfn_t i, t;
>>grub_xen_mfn_t cn_pfn = -1, st_pfn = -1;
>> @@ -175,32 +177,32 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, 
>> grub_xen_mfn_t pfn)
>>
>>for (i = 0; i < grub_xen_start_page_addr->nr_pages; i++)
>>  {
>> -  if (new_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn)
>> +  if (virt_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn)
>>  cn_pfn = i;
>> -  if (new_mfn_list[i] == grub_xen_start_page_addr->store_mfn)
>> +  if (virt_mfn_list[i] == grub_xen_start_page_addr->store_mfn)
>>  st_pfn = i;
>>  }
>>if (cn_pfn == (grub_xen_mfn_t)-1)
>>  return grub_error (GRUB_ERR_BUG, "no console");
>>if (st_pfn == (grub_xen_mfn_t)-1)
>>  return grub_error (GRUB_ERR_BUG, "no store");
>> -  t = new_mfn_list[pfn];
>> -  new_mfn_list[pfn] = new_mfn_list[cn_pfn];
>> -  new_mfn_list[cn_pfn] = t;
>> -  t = new_mfn_list[pfn + 1];
>> -  new_mfn_list[pfn + 1] = new_mfn_list[st_pfn];
>> -  new_mfn_list[st_pfn] = t;
>> -
>> -  m2p_updates[0].ptr = page2offset (new_mfn_list[pfn]) | 
>> MMU_MACHPHYS_UPDATE;
>> +  t = virt_mfn_list[pfn];
>> +  virt_mfn_list[pfn] = virt_mfn_list[cn_pfn];
>> +  virt_mfn_list[cn_pfn] = t;
>> +  t = virt_mfn_list[pfn + 1];
>> +  virt_mfn_list[pfn + 1] = virt_mfn_list[st_pfn];
>> +  virt_mfn_list[st_pfn] = t;
>> +
>> +  m2p_updates[0].ptr = page2offset (virt_mfn_list[pfn]) | 
>> MMU_MACHPHYS_UPDATE;
>>m2p_updates[0].val = pfn;
>>m2p_updates[1].ptr =
>> -page2offset (new_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE;
>> +page2offset (virt_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE;
>>m2p_updates[1].val = pfn + 1;
>>m2p_updates[2].ptr =
>> -page2offset (new_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE;
>> +page2offset (virt_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE;
>>m2p_updates[2].val = cn_pfn;
>>m2p_updates[3].ptr =
>> -page2offset (new_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE;
>> +page2offset (virt_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE;
>>m2p_updates[3].val = st_pfn;
>>
>>grub_xen_mmu_update (m2p_updates, 4, NULL, DOMID_SELF);
>> @@ -209,34 +211,43 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, 
>> grub_xen_mfn_t pfn)
>>  }
>>
>>  static grub_err_t
>> +grub_xen_p2m_alloc (void)
>> +{
>> +  grub_relocator_chunk_t ch;
>> +  grub_size_t p2msize;
>> +  grub_err_t err;
>> +
>> +  state.mfn_list = max_addr;
>> +  next_start.mfn_list = max_addr + xen_inf.virt_base;
>> +  p2msize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages;
>> +  err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, p2msize);
> 
> Hmmm Where relocator is defined?

First global variable in this source.

> 
>> +  if (err)
>> +return err;
>> +  virt_mfn_list = get_virtual_current_address (ch);
>> +  grub_memcpy (virt_mfn_list,
>> +   (void *) grub_xen_start_page_addr->mfn_list, p2msize);
>> +  max_addr = ALIGN_UP (max_addr + p2msize, PAGE_SIZE);
>> +
>> +  return GRUB_ERR_NONE;
>> +}
>> +
>> +static grub_err_t
>>  grub_xen_boot (void)
>>  {
>> -  struct grub_relocator_xen_state state;
>> 

Re: [Xen-devel] [PATCH v2 2/6] xen: factor out allocation of special pages into separate function

[Juergen Gross]

On 11/02/16 13:21, Daniel Kiper wrote:
> On Thu, Feb 11, 2016 at 08:53:22AM +0100, Juergen Gross wrote:
>> Do the allocation of special pages (start info, console and xenbus
>> ring buffers) in a separate function. This will allow to do the
>> allocation at different times of the boot preparations depending on
>> the features the kernel is supporting.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  grub-core/loader/i386/xen.c | 50 
>> +
>>  1 file changed, 32 insertions(+), 18 deletions(-)
>>
>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>> index 42ed7c7..e48cc3f 100644
>> --- a/grub-core/loader/i386/xen.c
>> +++ b/grub-core/loader/i386/xen.c
>> @@ -54,6 +54,8 @@ static grub_uint64_t modules_target_start;
>>  static grub_size_t n_modules;
>>  static struct grub_relocator_xen_state state;
>>  static grub_xen_mfn_t *virt_mfn_list;
>> +static struct start_info *virt_start_info;
>> +static grub_xen_mfn_t console_pfn;
> 
> Same as in patch #1.

Same answer. :-)


Juergen


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

Re: [Xen-devel] [PATCH v2 5/6] xen: modify page table construction

[Daniel Kiper]

On Thu, Feb 11, 2016 at 08:53:25AM +0100, Juergen Gross wrote:
> Modify the page table construction to allow multiple virtual regions
> to be mapped. This is done as preparation for removing the p2m list
> from the initial kernel mapping in order to support huge pv domains.
>
> This allows a cleaner approach for mapping the relocator page by
> using this capability.
>
> The interface to the assembler level of the relocator has to be changed
> in order to be able to process multiple page table areas.

I hope that older kernels could be loaded as is and everything works as 
expected.

> Signed-off-by: Juergen Gross 
> ---
>  grub-core/lib/i386/xen/relocator.S   |  47 +++---
>  grub-core/lib/x86_64/xen/relocator.S |  44 +++--
>  grub-core/lib/xen/relocator.c|  22 ++-
>  grub-core/loader/i386/xen.c  | 313 
> +++
>  include/grub/xen/relocator.h |   6 +-
>  5 files changed, 277 insertions(+), 155 deletions(-)
>
> diff --git a/grub-core/lib/i386/xen/relocator.S 
> b/grub-core/lib/i386/xen/relocator.S
> index 694a54c..c23b405 100644
> --- a/grub-core/lib/i386/xen/relocator.S
> +++ b/grub-core/lib/i386/xen/relocator.S
> @@ -50,41 +50,45 @@ VARIABLE(grub_relocator_xen_remapper_map_high)
>   jmp *%ebx
>
>  LOCAL(cont):
> - xorl%eax, %eax
> - movl%eax, %ebp
> + /* mov imm32, %eax */
> + .byte   0xb8
> +VARIABLE(grub_relocator_xen_paging_areas_addr)
> + .long   0
> + movl%eax, %ebx
>  1:
> -
> + movl0(%ebx), %ebp
> + movl4(%ebx), %ecx

Oh... No, please use constants not plain numbers and
explain in comments what is going on. Otherwise it
is unreadable.

> + testl   %ecx, %ecx
> + jz  3f
> + addl$8, %ebx
> + movl%ebx, %esp
> +
> +2:
> + movl%ecx, %edi
>   /* mov imm32, %eax */
>   .byte   0xb8
>  VARIABLE(grub_relocator_xen_mfn_list)
>   .long   0
> - movl%eax, %edi
> - movl%ebp, %eax
> - movl0(%edi, %eax, 4), %ecx
> -
> - /* mov imm32, %ebx */
> - .byte   0xbb
> -VARIABLE(grub_relocator_xen_paging_start)
> - .long   0
> - shll$12, %eax
> - addl%eax, %ebx
> + movl0(%eax, %ebp, 4), %ecx
> + movl%ebp, %ebx
> + shll$12, %ebx

Ditto.

>   movl%ecx, %edx
>   shll$12,  %ecx
>   shrl$20,  %edx
>   orl $5, %ecx
>   movl$2, %esi
>   movl$__HYPERVISOR_update_va_mapping, %eax
> - int $0x82
> + int $0x82   /* parameters: eax, ebx, ecx, edx, esi */

Please use more verbose comments.

>
>   incl%ebp
> - /* mov imm32, %ecx */
> - .byte   0xb9
> -VARIABLE(grub_relocator_xen_paging_size)
> - .long   0
> - cmpl%ebp, %ecx
> + movl%edi, %ecx
> +
> + loop2b
>
> - ja  1b
> + mov %esp, %ebx
> + jmp 1b
>
> +3:
>   /* mov imm32, %ebx */
>   .byte   0xbb
>  VARIABLE(grub_relocator_xen_mmu_op_addr)
> @@ -102,6 +106,9 @@ VARIABLE(grub_relocator_xen_remap_continue)
>
>   jmp *%eax
>
> +VARIABLE(grub_relocator_xen_paging_areas)
> + .long   0, 0, 0, 0, 0, 0, 0, 0
> +
>  VARIABLE(grub_relocator_xen_mmu_op)
>   .space 256
>
> diff --git a/grub-core/lib/x86_64/xen/relocator.S 
> b/grub-core/lib/x86_64/xen/relocator.S
> index 92e9e72..dbb90c7 100644
> --- a/grub-core/lib/x86_64/xen/relocator.S
> +++ b/grub-core/lib/x86_64/xen/relocator.S

Is to possible to split this patch to i386 and x86-64 stuff patches?

> @@ -50,31 +50,24 @@ VARIABLE(grub_relocator_xen_remapper_map)
>
>  LOCAL(cont):
>
> - /* mov imm64, %rcx */
> - .byte   0x48
> - .byte   0xb9
> -VARIABLE(grub_relocator_xen_paging_size)
> - .quad   0
> -
> - /* mov imm64, %rax */
> - .byte   0x48
> - .byte   0xb8
> -VARIABLE(grub_relocator_xen_paging_start)
> - .quad   0
> -
> - movq%rax, %r12
> -
>   /* mov imm64, %rax */
>   .byte   0x48
>   .byte   0xb8
>  VARIABLE(grub_relocator_xen_mfn_list)
>   .quad   0
>
> - movq%rax, %rsi
> + movq%rax, %rbx
> + leaqEXT_C(grub_relocator_xen_paging_areas) (%rip), %r8
> +
>  1:
> + movq0(%r8), %r12
> + movq8(%r8), %rcx

Ditto.

> + testq   %rcx, %rcx
> + jz  3f
> +2:
>   movq%r12, %rdi
> - movq%rsi, %rbx
> - movq0(%rsi), %rsi
> + shlq$12, %rdi
> + movq(%rbx, %r12, 8), %rsi

Ditto.

>   shlq$12,  %rsi
>   orq $5, %rsi
>   movq$2, %rdx
> @@ -83,13 +76,15 @@ VARIABLE(grub_relocator_xen_mfn_list)
>   syscall
>
>   movq%r9, %rcx
> - addq$8, %rbx
> - addq$4096, %r12
> - movq%rbx, %rsi
> + incq%r12
> +
> + loop 2b
>
> - loop 1b
> + addq$16, %r8

Ditto.

> + jmp 1b
>
> - leaq   LOCAL(mmu_op) (%rip), %rdi
> +3:
> + leaq   EXT_C(grub_relocator_xen_mmu_op) (%rip), %rdi
>   movq   $3, %rsi
>   movq   $0, %rdx
>   movq   $0x7FF0, %r10

U

Re: [Xen-devel] [PATCH v2 2/2] x86/traps: Dump instruction stream in show_execution_state()

[Jan Beulich]

>>> On 11.02.16 at 13:12,  wrote:
> On 11/02/16 11:16, Jan Beulich wrote:
> On 11.02.16 at 11:52,  wrote:
>>> --- a/xen/arch/x86/traps.c
>>> +++ b/xen/arch/x86/traps.c
>>> @@ -114,6 +114,56 @@ boolean_param("ler", opt_ler);
>>>  #define stack_words_per_line 4
>>>  #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
>>>  
>>> +static void show_code(const struct cpu_user_regs *regs)
>>> +{
>>> +unsigned char insns_before[8], insns_after[16];
>>> +unsigned int i, missing_before, missing_after;
>>> +
>>> +if ( guest_mode(regs) )
>>> +return;
>>> +
>>> +/*
>>> + * This dance with {insns,missing}_{before,after} is to ensure that, if
>>> + * %rip -8/+16 wraps around a bounday, we read a non-wrapped regs->rip
>>> + * pointer, and calculate which bytes were not read so they may be
>>> + * replaced with dashes in the printed output.
>>> + */
>>> +missing_before = __copy_from_user(
>>> +insns_before, (void __user *)regs->rip - 8, 
>>> ARRAY_SIZE(insns_before));
>>> +missing_after = __copy_from_user(
>>> +insns_after, (void __user *)regs->rip, ARRAY_SIZE(insns_after));
>> ... iirc __copy_from_user() doesn't range check the addresses.
>> Also reading the leading bytes is done in kind of a strange way: It'll
>> read initial bytes (farther away from RIP) and perhaps not read
>> later ones (closer to RIP), albeit clearly the ones closer are of more
>> interest. In the extreme case, where RIP is only a few bytes into a
>> page following an unmapped one, no leading bytes would be printed
>> at all despite some actually being readable.
> 
> I think in this specific case, it might be best to hand roll some asm
> using rep movs and the direction flag, with manual fault handling.

That's certainly an option.

>> Avoiding actual wrapping could be easily done by extending the
>> guest_mode() check above to also range check RIP against the
>> hypervisor image boundaries (post-boot this could even be limited
>> further, but perhaps using the full XEN_VIRT_{START,END} range is
>> the better route with xSplice in mind.
> 
> I would like, where possible, to avoid omitting the instruction stream
> if Xen is outside of its expected boundaries.

Which is because of ...? What useful information do you think can
be gained from the actual instruction when the mere fact of being
outside the boundaries is a bug?

Jan


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

[Xen-devel] [libvirt test] 81645: regressions - FAIL

[osstest service owner]

flight 81645 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/81645/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt-qcow2  4 host-ping-check-native  fail REGR. vs. 80121

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  73b70b403d3550a471373a1a65b868583d4b4e1a
baseline version:
 libvirt  6ec319b84f67d72bf59fe7e0fd41d88ee9c393c7

Last test of basis80121  2016-02-03 04:26:28 Z8 days
Failing since 80382  2016-02-04 04:29:24 Z7 days6 attempts
Testing same since81645  2016-02-09 16:31:00 Z1 days1 attempts


People who touched revisions under test:
  Cole Robinson 
  Daniel P. Berrange 
  Dmitry Andreev 
  Erik Skultety 
  Joao Martins 
  John Ferlan 
  Ján Tomko 
  Martin Kletzander 
  Michal Privoznik 
  Nikolay Shirokovskiy 
  Peter Krempa 
  Roman Bogorodskiy 
  Wido den Hollander 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt fail
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-qcow2   fail
 test-armhf-armhf-libvirt-raw fail
 test-amd64-amd64-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 1136 lines long.)

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

Re: [Xen-devel] [PATCH v2 3/6] xen: factor out allocation of page tables into separate function

[Juergen Gross]

On 11/02/16 13:27, Daniel Kiper wrote:
> On Thu, Feb 11, 2016 at 08:53:23AM +0100, Juergen Gross wrote:
>> Do the allocation of page tables in a separate function. This will
>> allow to do the allocation at different times of the boot preparations
>> depending on the features the kernel is supporting.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  grub-core/loader/i386/xen.c | 82 
>> -
>>  1 file changed, 51 insertions(+), 31 deletions(-)
>>
>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>> index e48cc3f..65cec27 100644
>> --- a/grub-core/loader/i386/xen.c
>> +++ b/grub-core/loader/i386/xen.c
>> @@ -56,6 +56,9 @@ static struct grub_relocator_xen_state state;
>>  static grub_xen_mfn_t *virt_mfn_list;
>>  static struct start_info *virt_start_info;
>>  static grub_xen_mfn_t console_pfn;
>> +static grub_uint64_t *virt_pgtable;
>> +static grub_uint64_t pgtbl_start;
>> +static grub_uint64_t pgtbl_end;
> 
> Same as in patches #1 and #2.

Yep.

> 
>>  #define PAGE_SIZE 4096
>>  #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
>> @@ -106,17 +109,17 @@ get_pgtable_size (grub_uint64_t total_pages, 
>> grub_uint64_t virt_base)
>>
>>  static void
>>  generate_page_table (grub_uint64_t *where, grub_uint64_t paging_start,
>> - grub_uint64_t total_pages, grub_uint64_t virt_base,
>> - grub_xen_mfn_t *mfn_list)
>> + grub_uint64_t paging_end, grub_uint64_t total_pages,
>> + grub_uint64_t virt_base, grub_xen_mfn_t *mfn_list)
>>  {
>>if (!virt_base)
>> -total_pages++;
>> +paging_end++;
>>
>>grub_uint64_t lx[NUMBER_OF_LEVELS], lxs[NUMBER_OF_LEVELS];
>>grub_uint64_t nlx, nls, sz = 0;
>>int l;
>>
>> -  nlx = total_pages;
>> +  nlx = paging_end;
>>nls = virt_base >> PAGE_SHIFT;
>>for (l = 0; l < NUMBER_OF_LEVELS; l++)
>>  {
>> @@ -160,7 +163,7 @@ generate_page_table (grub_uint64_t *where, grub_uint64_t 
>> paging_start,
>>if (pr)
>>  pg += POINTERS_PER_PAGE;
>>
>> -  for (j = 0; j < total_pages; j++)
>> +  for (j = 0; j < paging_end; j++)
>>  {
>>if (j >= paging_start && j < lp)
>>  pg[j + lxs[0]] = page2offset (mfn_list[j]) | 5;
>> @@ -261,24 +264,12 @@ grub_xen_special_alloc (void)
>>  }
>>
>>  static grub_err_t
>> -grub_xen_boot (void)
>> +grub_xen_pt_alloc (void)
>>  {
>>grub_relocator_chunk_t ch;
>>grub_err_t err;
>>grub_uint64_t nr_info_pages;
>>grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
>> -  struct gnttab_set_version gnttab_setver;
>> -  grub_size_t i;
>> -
>> -  if (grub_xen_n_allocated_shared_pages)
>> -return grub_error (GRUB_ERR_BUG, "active grants");
>> -
>> -  err = grub_xen_p2m_alloc ();
>> -  if (err)
>> -return err;
>> -  err = grub_xen_special_alloc ();
>> -  if (err)
>> -return err;
>>
>>next_start.pt_base = max_addr + xen_inf.virt_base;
>>state.paging_start = max_addr >> PAGE_SHIFT;
>> @@ -298,30 +289,59 @@ grub_xen_boot (void)
>>nr_pages = nr_need_pages;
>>  }
>>
>> -  grub_dprintf ("xen", "bootstrap domain %llx+%llx\n",
>> -(unsigned long long) xen_inf.virt_base,
>> -(unsigned long long) page2offset (nr_pages));
>> -
>>err = grub_relocator_alloc_chunk_addr (relocator, &ch,
>>   max_addr, page2offset (nr_pt_pages));
>>if (err)
>>  return err;
>>
>> +  virt_pgtable = get_virtual_current_address (ch);
>> +  pgtbl_start = max_addr >> PAGE_SHIFT;
>> +  max_addr += page2offset (nr_pt_pages);
>> +  state.stack = max_addr + STACK_SIZE + xen_inf.virt_base;
>> +  state.paging_size = nr_pt_pages;
>> +  next_start.nr_pt_frames = nr_pt_pages;
>> +  max_addr = page2offset (nr_pages);
>> +  pgtbl_end = nr_pages;
>> +
>> +  return GRUB_ERR_NONE;
>> +}
>> +
>> +static grub_err_t
>> +grub_xen_boot (void)
>> +{
>> +  grub_err_t err;
>> +  grub_uint64_t nr_pages;
>> +  struct gnttab_set_version gnttab_setver;
>> +  grub_size_t i;
>> +
>> +  if (grub_xen_n_allocated_shared_pages)
>> +return grub_error (GRUB_ERR_BUG, "active grants");
>> +
>> +  err = grub_xen_p2m_alloc ();
>> +  if (err)
>> +return err;
>> +  err = grub_xen_special_alloc ();
>> +  if (err)
>> +return err;
>> +  err = grub_xen_pt_alloc ();
>> +  if (err)
>> +return err;
> 
> Should not we free memory allocated by grub_xen_p2m_alloc() and
> grub_xen_special_alloc() if grub_xen_pt_alloc() failed?

Hmm, why? If I don't miss anything freeing memory in case of error isn't
done anywhere (at least not in this source file).

Juergen


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

Re: [Xen-devel] [PATCH v2] tools: probe for existence of qemu-xen stderr trace backend.

[Paul Durrant]

> -Original Message-
> From: Ian Campbell [mailto:ian.campb...@citrix.com]
> Sent: 11 February 2016 12:11
> To: Ian Jackson; Wei Liu; xen-devel@lists.xen.org; Stefano Stabellini
> Cc: Ian Campbell; Paul Durrant; Anthony Perard
> Subject: [PATCH v2] tools: probe for existence of qemu-xen stderr trace
> backend.
> 
> QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
> log") renamed the "stderr" trace backend to "log", which breaks the
> xen build when pointed at a QEMU tree after that point:
> 
> ./configure of QEMU fail with:
> "ERROR: invalid trace backends
> Please choose supported trace backends."
> 
> Upstream also changed, in baf86d6b3ca0 ("trace: switch default backend
> to "log""), to use "log" as the default backend (previously it was
> "nop").
> 
> Use ./scripts/tracetool.py to check for the presence of the stderr
> backend and if it is present then explicitly enable it. If the stderr
> backend is not present then assume a newer QEMU which defaults to
> "log" and simply accept that default (there is a 1 commit window
> upstream where this would result in no trace backend being enabled).
> 
> The check is done using the older (deprecated?) --check-backend/--backend
> variant of the tracetool.py options rather than the new plural
> versions since the singular was supported even by very old versions of
> QEMU.  New QEMU has compatibility code but if/when that is removed we
> will still do the right thing i.e. no explict configuiration resulting
> in the upstream default (currently "log").
> 
> If the explicit selection of the "stderr" backend is required then it
> is now done unconditionally (not depending on debug=y), which is
> simpler to arrange here but also matches the newer upstream's default
> to "log" which is not conditional on debug being enabled either.
> 
> Tested with current qemu-xen-unstable (e9d8252) and current QEMU
> upstream master (88c73d1), both out of tree via
> QEMU_UPSTREAM_URL=/path/to/qemu-xen.git.
> 
> Signed-off-by: Ian Campbell 
> Cc: Paul Durrant 

LGTM.

Reviewed-by: Paul Durrant 

> Cc: Anthony PERARD 
> ---
> v2: Just probe for stderr, otherwise assume the new upstream default
> of log is what we will get anyway. Substantially rewrite commit
> message.
> ---
>  tools/Makefile | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index cb8652c..3f45fb9 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -228,7 +228,7 @@ qemu-xen-dir-force-update: qemu-xen-dir-find
>   fi
> 
>  ifeq ($(debug),y)
> -QEMU_XEN_ENABLE_DEBUG := --enable-debug --enable-trace-
> backend=stderr
> +QEMU_XEN_ENABLE_DEBUG := --enable-debug
>  else
>  QEMU_XEN_ENABLE_DEBUG :=
>  endif
> @@ -240,8 +240,14 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>   source=.; \
>   fi; \
>   cd qemu-xen-dir; \
> + if $$source/scripts/tracetool.py --check-backend --backend stderr ;
> then \
> + enable_trace_backend='--enable-trace-backend=stderr'; \
> + else \
> + enable_trace_backend='' ; \
> + fi ; \
>   $$source/configure --enable-xen --target-list=i386-softmmu \
>   $(QEMU_XEN_ENABLE_DEBUG) \
> + $$enable_trace_backend \
>   --prefix=$(LIBEXEC) \
>   --libdir=$(LIBEXEC_LIB) \
>   --includedir=$(LIBEXEC_INC) \
> --
> 2.1.4


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

Re: [Xen-devel] [PATCH v3 1/2] build: specify minimum versions of gcc and binutils

[Ian Campbell]

On Thu, 2016-02-11 at 12:31 +, Andrew Cooper wrote:
> On 11/02/16 12:23, Ian Campbell wrote:
> > From: Doug Goldstein 
> > 
> > To help people avoid having to figure out what versions of gcc and
> > binutils need to be supported document them explicitly.
> > 
> > Signed-off-by: Doug Goldstein 
> > Signed-off-by: Ian Campbell 
> 
> I would be tempted to abbreviate this to GCC 4.1 and Binutls 2.16,
> unless we specifically know of issues between the release and point
> releases.

I don't mind doing this but if there is any quibbling at all about it then
I would simply go with what is below.

> 
> Either way, Reviewed-by: Andrew Cooper 
> 
> > ---
> > v3: [ijc] Updated version for binutils and include gcc (separately for
> > x86 and ARM in both cases), deferred make to second patch
> > ---
> >  README | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/README b/README
> > index 1324c7c..356a350 100644
> > --- a/README
> > +++ b/README
> > @@ -35,9 +35,13 @@ Second, there are a number of prerequisites for
> > building a Xen source
> >  release. Make sure you have all the following installed, either by
> >  visiting the project webpage or installing a pre-built package
> >  provided by your OS distributor:
> > -* GCC v4.1 or later
> > +* GCC
> > +  - For x86 4.1.2_20070115 or later
> > +  - For ARM 4.8 or later
> >  * GNU Make
> > -* GNU Binutils
> > +* GNU Binutils:
> > +  - For x86 2.16.91.0.5 or later
> > +  - For ARM 2.24 or later
> >  * Development install of zlib (e.g., zlib-dev)
> >  * Development install of Python v2.3 or later (e.g., python-dev)
> >  * Development install of curses (e.g., libncurses-dev)
> 

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

Re: [Xen-devel] [PATCH 2/3] xen: sched: add wakeup flags to the scheduler interface

[Jan Beulich]

>>> On 11.02.16 at 12:38,  wrote:
> --- a/xen/include/xen/sched-if.h
> +++ b/xen/include/xen/sched-if.h
> @@ -144,7 +144,8 @@ struct scheduler {
>  void (*remove_vcpu)(const struct scheduler *, struct vcpu *);
>  
>  void (*sleep)  (const struct scheduler *, struct vcpu *);
> -void (*wake)   (const struct scheduler *, struct vcpu *);
> +void (*wake)   (const struct scheduler *, struct vcpu *,
> +unsigned int);

Just one cosmetic comment: You properly use "unsigned int" here,
but just "unsigned" everywhere else in this patch. May I ask that
you use the canonical full form everywhere?

Jan


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

Re: [Xen-devel] [PATCH 3/3] xen: credit1: avoid boosting vCPUs being "just" migrated

[Jan Beulich]

>>> On 11.02.16 at 12:39,  wrote:
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -1022,11 +1022,14 @@ csched_vcpu_wake(const struct scheduler *ops, struct 
> vcpu *vc, unsigned wf)
>   * more CPU resource intensive VCPUs without impacting overall 
>   * system fairness.
>   *
> - * The one exception is for VCPUs of capped domains unpausing
> - * after earning credits they had overspent. We don't boost
> - * those.
> + * There are a couple of exceptions, when we don't want to boost:
> + *  - VCPUs that are waking up after a migration, rather than
> + *after having block;
> + *  - VCPUs of capped domains unpausing after earning credits
> + *they had overspent.
>   */
> -if ( svc->pri == CSCHED_PRI_TS_UNDER &&
> +if ( !(wf & WF_migrated) &&
> + svc->pri == CSCHED_PRI_TS_UNDER &&
>   !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
>  {

Considering the other svc->flags check done here, wouldn't it be
possible to achieve the same effect without patch 2, by having
csched_cpu_pick() set a newly defined flag, and check for it here?

Jan


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

Re: [Xen-devel] [PATCH v3 1/2] build: specify minimum versions of gcc and binutils

[Jan Beulich]

>>> On 11.02.16 at 14:21,  wrote:
> On Thu, 2016-02-11 at 12:31 +, Andrew Cooper wrote:
>> On 11/02/16 12:23, Ian Campbell wrote:
>> > From: Doug Goldstein 
>> > 
>> > To help people avoid having to figure out what versions of gcc and
>> > binutils need to be supported document them explicitly.
>> > 
>> > Signed-off-by: Doug Goldstein 
>> > Signed-off-by: Ian Campbell 
>> 
>> I would be tempted to abbreviate this to GCC 4.1 and Binutls 2.16,
>> unless we specifically know of issues between the release and point
>> releases.
> 
> I don't mind doing this but if there is any quibbling at all about it then
> I would simply go with what is below.

Yeah, I'd prefer the patches to go in as they're now: Both
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v2 2/2] x86/traps: Dump instruction stream in show_execution_state()

[Andrew Cooper]

On 11/02/16 12:52, Jan Beulich wrote:
 On 11.02.16 at 13:12,  wrote:
>> On 11/02/16 11:16, Jan Beulich wrote:
>> On 11.02.16 at 11:52,  wrote:
 --- a/xen/arch/x86/traps.c
 +++ b/xen/arch/x86/traps.c
 @@ -114,6 +114,56 @@ boolean_param("ler", opt_ler);
  #define stack_words_per_line 4
  #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
  
 +static void show_code(const struct cpu_user_regs *regs)
 +{
 +unsigned char insns_before[8], insns_after[16];
 +unsigned int i, missing_before, missing_after;
 +
 +if ( guest_mode(regs) )
 +return;
 +
 +/*
 + * This dance with {insns,missing}_{before,after} is to ensure that, 
 if
 + * %rip -8/+16 wraps around a bounday, we read a non-wrapped regs->rip
 + * pointer, and calculate which bytes were not read so they may be
 + * replaced with dashes in the printed output.
 + */
 +missing_before = __copy_from_user(
 +insns_before, (void __user *)regs->rip - 8, 
 ARRAY_SIZE(insns_before));
 +missing_after = __copy_from_user(
 +insns_after, (void __user *)regs->rip, ARRAY_SIZE(insns_after));
>>> ... iirc __copy_from_user() doesn't range check the addresses.
>>> Also reading the leading bytes is done in kind of a strange way: It'll
>>> read initial bytes (farther away from RIP) and perhaps not read
>>> later ones (closer to RIP), albeit clearly the ones closer are of more
>>> interest. In the extreme case, where RIP is only a few bytes into a
>>> page following an unmapped one, no leading bytes would be printed
>>> at all despite some actually being readable.
>> I think in this specific case, it might be best to hand roll some asm
>> using rep movs and the direction flag, with manual fault handling.
> That's certainly an option.

It is turning out to be much nicer.

>
>>> Avoiding actual wrapping could be easily done by extending the
>>> guest_mode() check above to also range check RIP against the
>>> hypervisor image boundaries (post-boot this could even be limited
>>> further, but perhaps using the full XEN_VIRT_{START,END} range is
>>> the better route with xSplice in mind.
>> I would like, where possible, to avoid omitting the instruction stream
>> if Xen is outside of its expected boundaries.
> Which is because of ...? What useful information do you think can
> be gained from the actual instruction when the mere fact of being
> outside the boundaries is a bug?

Wherever %rip is pointing, the code under %rip is directly relevant to
the exact values of the registers and stack dump printed.

It will be obvious from the numeric value of %rip whether it is bad
(also, whether symbol information is found), and making it work for all
cases is easier than restricting to the Xen-only case.

~Andrew

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

[Xen-devel] [linux-4.1 test] 81641: regressions - FAIL

[osstest service owner]

flight 81641 linux-4.1 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/81641/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvops3 host-install(3) broken in 81339 REGR. vs. 66399
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 66399
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 66399
 test-armhf-armhf-xl-credit2  15 guest-start/debian.repeat fail REGR. vs. 66399
 test-armhf-armhf-xl  15 guest-start/debian.repeat fail REGR. vs. 66399
 test-armhf-armhf-xl-xsm  16 guest-start.2fail in 80172 REGR. vs. 66399

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-xsm  11 guest-startfail in 80370 pass in 81641
 test-armhf-armhf-xl  11 guest-startfail in 80752 pass in 81641
 test-armhf-armhf-xl-xsm  15 guest-start/debian.repeat   fail pass in 80172
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat  fail pass in 80370
 test-armhf-armhf-xl-cubietruck 15 guest-start/debian.repeat fail pass in 80752
 test-armhf-armhf-xl-rtds 11 guest-start fail pass in 80752
 test-amd64-amd64-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail pass in 
81339

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat fail in 80752 like 66399
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail in 80752 like 66399
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 66399
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 66399
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 66399
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   like 66399

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm  1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)  blocked in 81339 n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)  blocked in 81339 n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-xl   1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-xl-xsm   1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked in 81339 n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-libvirt  1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)blocked in 81339 n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)blocked in 81339 n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds 13 saverestore-support-check fail in 80752 never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-check fail in 80752 never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-sup

Re: [Xen-devel] [PATCH v2 2/2] x86/traps: Dump instruction stream in show_execution_state()

[Jan Beulich]

>>> On 11.02.16 at 14:41,  wrote:
> Wherever %rip is pointing, the code under %rip is directly relevant to
> the exact values of the registers and stack dump printed.
> 
> It will be obvious from the numeric value of %rip whether it is bad
> (also, whether symbol information is found), and making it work for all
> cases is easier than restricting to the Xen-only case.

Let's see (at the example of v3) whether this "easier" actually holds...

Jan


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

Re: [Xen-devel] [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping

[Juergen Gross]

On 11/02/16 13:33, Daniel Kiper wrote:
> On Thu, Feb 11, 2016 at 08:53:24AM +0100, Juergen Gross wrote:
>> Modern pvops linux kernels support an initrd not covered by the initial
>> mapping. This capability is flagged by an elf-note.
>>
>> In case the elf-note is set by the kernel don't place the initrd into
>> the initial mapping. This will allow to load larger initrds and/or
>> support domains with larger memory, as the initial mapping is limited
>> to 2GB and it is containing the p2m list.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  grub-core/loader/i386/xen.c| 56 
>> ++
>>  grub-core/loader/i386/xen_fileXX.c |  3 ++
>>  include/grub/xen_file.h|  1 +
>>  3 files changed, 49 insertions(+), 11 deletions(-)
>>
>> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
>> index 65cec27..0f41048 100644
>> --- a/grub-core/loader/i386/xen.c
>> +++ b/grub-core/loader/i386/xen.c
>> @@ -307,15 +307,14 @@ grub_xen_pt_alloc (void)
>>  }
>>
>>  static grub_err_t
>> -grub_xen_boot (void)
>> +grub_xen_alloc_end (void)
>>  {
>>grub_err_t err;
>> -  grub_uint64_t nr_pages;
>> -  struct gnttab_set_version gnttab_setver;
>> -  grub_size_t i;
>> +  static int called = 0;
>>
>> -  if (grub_xen_n_allocated_shared_pages)
>> -return grub_error (GRUB_ERR_BUG, "active grants");
>> +  if (called)
>> +return GRUB_ERR_NONE;
> 
> Why?

Did you look at the function? grub_xen_alloc_end() (some parts of the
original grub_xen_boot()) is new and may be called multiple times. It
was much easier to just call it and let it do what must be done only
at the first time called.

> 
>> +  called = 1;
>>
>>err = grub_xen_p2m_alloc ();
>>if (err)
>> @@ -327,6 +326,24 @@ grub_xen_boot (void)
>>if (err)
>>  return err;
>>
>> +  return GRUB_ERR_NONE;
>> +}
>> +
>> +static grub_err_t
>> +grub_xen_boot (void)
>> +{
>> +  grub_err_t err;
>> +  grub_uint64_t nr_pages;
>> +  struct gnttab_set_version gnttab_setver;
>> +  grub_size_t i;
>> +
>> +  if (grub_xen_n_allocated_shared_pages)
>> +return grub_error (GRUB_ERR_BUG, "active grants");
> 
> Why?

That's how it has been before. git just decided to generate the diff
that way.

> 
>> +  err = grub_xen_alloc_end ();
>> +  if (err)
>> +return err;
>> +
>>err = set_mfns (console_pfn);
>>if (err)
>>  return err;
>> @@ -587,6 +604,13 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
>> ((unused)),
>>goto fail;
>>  }
>>
>> +  if (xen_inf.unmapped_initrd)
>> +{
>> +  err = grub_xen_alloc_end ();
>> +  if (err)
>> +goto fail;
>> +}
>> +
>>if (grub_initrd_init (argc, argv, &initrd_ctx))
>>  goto fail;
>>
>> @@ -603,13 +627,22 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
>> ((unused)),
>>  goto fail;
>>  }
>>
>> -  next_start.mod_start = max_addr + xen_inf.virt_base;
>> -  next_start.mod_len = size;
>> -
>> -  max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE);
>> +  if (xen_inf.unmapped_initrd)
>> +{
>> +  next_start.flags |= SIF_MOD_START_PFN;
>> +  next_start.mod_start = max_addr >> PAGE_SHIFT;
>> +  next_start.mod_len = size;
>> +}
>> +  else
>> +{
>> +  next_start.mod_start = max_addr + xen_inf.virt_base;
>> +  next_start.mod_len = size;
>> +}
>>
>>grub_dprintf ("xen", "Initrd, addr=0x%x, size=0x%x\n",
>> -(unsigned) next_start.mod_start, (unsigned) size);
>> +(unsigned) (max_addr + xen_inf.virt_base), (unsigned) size);
>> +
>> +  max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE);
>>
>>  fail:
>>grub_initrd_close (&initrd_ctx);
>> @@ -660,6 +693,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ 
>> ((unused)),
>>
>>if (!xen_module_info_page)
>>  {
>> +  xen_inf.unmapped_initrd = 0;
>>n_modules = 0;
>>max_addr = ALIGN_UP (max_addr, PAGE_SIZE);
>>modules_target_start = max_addr;
>> diff --git a/grub-core/loader/i386/xen_fileXX.c 
>> b/grub-core/loader/i386/xen_fileXX.c
>> index 1ba5649..69fccd2 100644
>> --- a/grub-core/loader/i386/xen_fileXX.c
>> +++ b/grub-core/loader/i386/xen_fileXX.c
>> @@ -253,6 +253,9 @@ parse_note (grub_elf_t elf, struct grub_xen_file_info 
>> *xi,
>>descsz == 2 ? 2 : 3) == 0)
>>  xi->arch = GRUB_XEN_FILE_I386;
>>break;
>> +case 16:
> 
> Could you define this a constant and use it here?

This would be the only case with a constant. All others are numeric
as well.


Juergen


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

[Xen-devel] [PATCH v3 2/2] x86/traps: Dump instruction stream in show_execution_state()

[Andrew Cooper]

For first pass triage of crashes, it is useful to have the instruction
stream present, especially now that Xen binary patches itself.

A sample output now looks like:

(XEN) [ Xen-4.7-unstable  x86_64  debug=y  Not tainted ]
(XEN) CPU:0
(XEN) RIP:e008:[] default_idle+0x76/0x7b
(XEN) RFLAGS: 0246   CONTEXT: hypervisor
(XEN) rax: 82d080331030   rbx: 83007fce8000   rcx: 
(XEN) rdx:    rsi: 82d080331b98   rdi: 
(XEN) rbp: 83007fcefef0   rsp: 83007fcefef0   r8:  83007faf8118
(XEN) r9:  0009983e89fd   r10: 0009983e89fd   r11: 0246
(XEN) r12: 83007fd61000   r13:    r14: 83007fad9000
(XEN) r15: 83007fae3000   cr0: 8005003b   cr4: 26e0
(XEN) cr3: 7fc9b000   cr2: 7f70976b3fed
(XEN) ds:    es:    fs:    gs:    ss: e010   cs: e008
(XEN) Xen code around  (default_idle+0x76/0x7b):
(XEN)  83 3c 10 00 75 04 fb f4  01 fb 5d c3 55 48 89 e5 3b 3d 0d 50 12 00 72
(XEN) Xen stack trace from rsp=83007fcefef0:
(XEN)83007fceff10 82d080160e08 82d08012c40a 83007faf9000
(XEN)83007fcefdd8 81a01fd8 88002f07d4c0 81a01fd8
(XEN) 81a01e58 81a01fd8 0246
(XEN)0052   
(XEN)810013aa 0001 deadbeef deadbeef
(XEN)0100 810013aa e033 0246
(XEN)81a01e40 e02b  
(XEN)   83007faf9000
(XEN) 
(XEN) Xen call trace:
(XEN)[] default_idle+0x76/0x7b
(XEN)[] idle_loop+0x51/0x6e
(XEN)

A sample with a partial access looks like:

(XEN) Xen code around  (8300ac0fe002) [fault on access]:
(XEN)  -- -- -- -- -- -- 00 00 <00> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 

v3:
 * Read backwards from %rip, to get the instructions closer to %rip in the
   case that there is a fault somewhere on the access.  Use handrolled asm
   with custom fault handling.
v2:
 * Deal with %rip wrapping.  In such a case, insert dashes into printed line.
---
 xen/arch/x86/traps.c | 69 
 1 file changed, 69 insertions(+)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index ab7deee..26a5026 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -114,6 +114,74 @@ boolean_param("ler", opt_ler);
 #define stack_words_per_line 4
 #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
 
+static void show_code(const struct cpu_user_regs *regs)
+{
+unsigned char insns_before[8] = {}, insns_after[16] = {};
+unsigned int i, tmp, missing_before, missing_after;
+
+if ( guest_mode(regs) )
+return;
+
+stac();
+
+/*
+ * Copy forward from regs->rip.  In the case of a fault, %ecx contains the
+ * number of bytes remaining to copy.
+ */
+asm volatile ("1: rep movsb; 2:"
+  _ASM_EXTABLE(1b, 2b)
+  : "=&c" (missing_after),
+"=&D" (tmp), "=&S" (tmp)
+  : "0" (ARRAY_SIZE(insns_after)),
+"1" (insns_after),
+"2" (regs->rip));
+
+/*
+ * Copy backwards from regs->rip - 1.  In the case of a fault, %ecx
+ * contains the number of bytes remaining to copy.
+ */
+asm volatile ("std;"
+  "1: rep movsb;"
+  "2: cld;"
+  _ASM_EXTABLE(1b, 2b)
+  : "=&c" (missing_before),
+"=&D" (tmp), "=&S" (tmp)
+  : "0" (ARRAY_SIZE(insns_before)),
+"1" (insns_before + ARRAY_SIZE(insns_before)),
+"2" (regs->rip - 1));
+clac();
+
+printk("Xen code around <%p> (%ps)%s:\n",
+   _p(regs->rip), _p(regs->rip),
+   (missing_before || missing_after) ? " [fault on access]" : "");
+
+/* Print bytes from insns_before[]. */
+for ( i = 0; i < ARRAY_SIZE(insns_before); ++i )
+{
+if ( i < missing_before )
+printk(" --");
+else
+printk(" %02x", insns_before[i]);
+}
+
+/* Print the byte under %rip. */
+if ( missing_after != ARRAY_SIZE(insns_after) )
+printk(" <%02x>", insns_after[0]);
+else
+printk(" <-->");
+
+/* Print bytes from insns_after[]. */
+for ( i = 1; i < ARRAY_SIZE(insns_after); ++i )
+{
+if ( i < (ARRAY_SIZE(insns_after) - missing_after) )
+printk(" %02x", insns_after[i]);
+else
+printk(" --");
+}
+
+printk("\n");
+}
+
 static void show_guest_stack(struct vcpu *v, const struct cpu_user_regs *regs)
 {
 int i;
@@ -420,6 +488,7 @@ void show_execut

Re: [Xen-devel] [PATCH v2 5/6] xen: modify page table construction

[Juergen Gross]

On 11/02/16 13:47, Daniel Kiper wrote:
> On Thu, Feb 11, 2016 at 08:53:25AM +0100, Juergen Gross wrote:
>> Modify the page table construction to allow multiple virtual regions
>> to be mapped. This is done as preparation for removing the p2m list
>> from the initial kernel mapping in order to support huge pv domains.
>>
>> This allows a cleaner approach for mapping the relocator page by
>> using this capability.
>>
>> The interface to the assembler level of the relocator has to be changed
>> in order to be able to process multiple page table areas.
> 
> I hope that older kernels could be loaded as is and everything works as 
> expected.

See Patch 0/6. I have tested old kernels, too.

> 
>> Signed-off-by: Juergen Gross 
>> ---
>>  grub-core/lib/i386/xen/relocator.S   |  47 +++---
>>  grub-core/lib/x86_64/xen/relocator.S |  44 +++--
>>  grub-core/lib/xen/relocator.c|  22 ++-
>>  grub-core/loader/i386/xen.c  | 313 
>> +++
>>  include/grub/xen/relocator.h |   6 +-
>>  5 files changed, 277 insertions(+), 155 deletions(-)
>>
>> diff --git a/grub-core/lib/i386/xen/relocator.S 
>> b/grub-core/lib/i386/xen/relocator.S
>> index 694a54c..c23b405 100644
>> --- a/grub-core/lib/i386/xen/relocator.S
>> +++ b/grub-core/lib/i386/xen/relocator.S
>> @@ -50,41 +50,45 @@ VARIABLE(grub_relocator_xen_remapper_map_high)
>>  jmp *%ebx
>>
>>  LOCAL(cont):
>> -xorl%eax, %eax
>> -movl%eax, %ebp
>> +/* mov imm32, %eax */
>> +.byte   0xb8
>> +VARIABLE(grub_relocator_xen_paging_areas_addr)
>> +.long   0
>> +movl%eax, %ebx
>>  1:
>> -
>> +movl0(%ebx), %ebp
>> +movl4(%ebx), %ecx
> 
> Oh... No, please use constants not plain numbers and
> explain in comments what is going on. Otherwise it
> is unreadable.

Hmm, yes, some comments wouldn't hurt. :-)
Regarding constants: do you really think I should introduce their
first usage in this file with my patch?

> 
>> +testl   %ecx, %ecx
>> +jz  3f
>> +addl$8, %ebx
>> +movl%ebx, %esp
>> +
>> +2:
>> +movl%ecx, %edi
>>  /* mov imm32, %eax */
>>  .byte   0xb8
>>  VARIABLE(grub_relocator_xen_mfn_list)
>>  .long   0
>> -movl%eax, %edi
>> -movl%ebp, %eax
>> -movl0(%edi, %eax, 4), %ecx
>> -
>> -/* mov imm32, %ebx */
>> -.byte   0xbb
>> -VARIABLE(grub_relocator_xen_paging_start)
>> -.long   0
>> -shll$12, %eax
>> -addl%eax, %ebx
>> +movl0(%eax, %ebp, 4), %ecx
>> +movl%ebp, %ebx
>> +shll$12, %ebx
> 
> Ditto.

Look at the removed line above: I just switched register usage.

> 
>>  movl%ecx, %edx
>>  shll$12,  %ecx
>>  shrl$20,  %edx
>>  orl $5, %ecx
>>  movl$2, %esi
>>  movl$__HYPERVISOR_update_va_mapping, %eax
>> -int $0x82
>> +int $0x82   /* parameters: eax, ebx, ecx, edx, esi */
> 
> Please use more verbose comments.

:-)

> 
>>
>>  incl%ebp
>> -/* mov imm32, %ecx */
>> -.byte   0xb9
>> -VARIABLE(grub_relocator_xen_paging_size)
>> -.long   0
>> -cmpl%ebp, %ecx
>> +movl%edi, %ecx
>> +
>> +loop2b
>>
>> -ja  1b
>> +mov %esp, %ebx
>> +jmp 1b
>>
>> +3:
>>  /* mov imm32, %ebx */
>>  .byte   0xbb
>>  VARIABLE(grub_relocator_xen_mmu_op_addr)
>> @@ -102,6 +106,9 @@ VARIABLE(grub_relocator_xen_remap_continue)
>>
>>  jmp *%eax
>>
>> +VARIABLE(grub_relocator_xen_paging_areas)
>> +.long   0, 0, 0, 0, 0, 0, 0, 0
>> +
>>  VARIABLE(grub_relocator_xen_mmu_op)
>>  .space 256
>>
>> diff --git a/grub-core/lib/x86_64/xen/relocator.S 
>> b/grub-core/lib/x86_64/xen/relocator.S
>> index 92e9e72..dbb90c7 100644
>> --- a/grub-core/lib/x86_64/xen/relocator.S
>> +++ b/grub-core/lib/x86_64/xen/relocator.S
> 
> Is to possible to split this patch to i386 and x86-64 stuff patches?

I don't think so. The C-part is common and assembler sources must both
match.

> 
>> @@ -50,31 +50,24 @@ VARIABLE(grub_relocator_xen_remapper_map)
>>
>>  LOCAL(cont):
>>
>> -/* mov imm64, %rcx */
>> -.byte   0x48
>> -.byte   0xb9
>> -VARIABLE(grub_relocator_xen_paging_size)
>> -.quad   0
>> -
>> -/* mov imm64, %rax */
>> -.byte   0x48
>> -.byte   0xb8
>> -VARIABLE(grub_relocator_xen_paging_start)
>> -.quad   0
>> -
>> -movq%rax, %r12
>> -
>>  /* mov imm64, %rax */
>>  .byte   0x48
>>  .byte   0xb8
>>  VARIABLE(grub_relocator_xen_mfn_list)
>>  .quad   0
>>
>> -movq%rax, %rsi
>> +movq%rax, %rbx
>> +leaqEXT_C(grub_relocator_xen_paging_areas) (%rip), %r8
>> +
>>  1:
>> +movq0(%r8), %r12
>> +movq8(%r8), %rcx
> 
> Ditto.
> 
>> +testq   %rcx, %rcx
>> +jz  3f
>> +2:
>>  movq%r12, %rdi
>> -movq%rsi, %rbx
>> -movq0(%rsi), %rsi
>> +shlq$12, %rdi
>> +movq(%rbx, %r12, 8), %rsi
> 
> Ditto.
> 
>>  shlq$12,  %rsi
>>  orq $5, %rsi
>>

Re: [Xen-devel] [PATCH v7 3/5] libxl: add support for vscsi

[Olaf Hering]

On Fri, Feb 05, Olaf Hering wrote:

> +static int libxl__vscsictrl_next_vscsidev_id(libxl__gc *gc, uint32_t domid,
> + libxl_device_vscsictrl *vscsi,
> + libxl_devid *vscsidev_id)
> +{

> +path = GCSPRINTF("%s/vscsi/%u/next_vscsidev_id",
> + libxl__xs_libxl_path(gc, domid),
> + vscsi->devid);

This creates /libxl//vscsi//counter. Where is the place to
remove /libxl//vscsi/ during scsi-detach? As a workaround
I'm in the process to move the counter to the backend path.

Olaf

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

Re: [Xen-devel] [PATCH v7 3/5] libxl: add support for vscsi

[Olaf Hering]

On Thu, Feb 11, Olaf Hering wrote:

> I'm in the process to move the counter to the backend path.

... which fails due to libxl__device_exists(). So, back to /libxl

Olaf

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

[Xen-devel] [PATCH] x86, microcode: Remove unnecessary paravirt_enabled check

[Boris Ostrovsky]

Commit a18a0f6850d4 ("x86, microcode: Don't initialize microcode code on
paravirt") added a paravirt test in microcode_init(), primarily to avoid
making mc_bp_resume()->load_ucode_ap()->check_loader_disabled_ap() calls
On 32-bit kernels this callchain ends up using __pa_nodebug() macro
which is invalid for Xen PV guests.

A subsequent commit, fbae4ba8c4a3 ("x86, microcode: Reload microcode on
resume"), eliminated this callchain thus making a18a0f6850d4
unnecessary.

Signed-off-by: Boris Ostrovsky 
---
 arch/x86/kernel/cpu/microcode/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/microcode/core.c 
b/arch/x86/kernel/cpu/microcode/core.c
index cea8552..ac360bf 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -623,7 +623,7 @@ int __init microcode_init(void)
struct cpuinfo_x86 *c = &boot_cpu_data;
int error;
 
-   if (paravirt_enabled() || dis_ucode_ldr)
+   if (dis_ucode_ldr)
return -EINVAL;
 
if (c->x86_vendor == X86_VENDOR_INTEL)
-- 
2.1.0


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

Re: [Xen-devel] [PATCH v2] libxc: fix leak in xc_offline_page error path

[Ian Campbell]

On Wed, 2016-02-10 at 11:51 +, Ian Jackson wrote:
> Harmandeep Kaur writes ("[PATCH v2] libxc: fix leak in xc_offline_page
> error path"):
> > Avoid leaking the mapping of the m2p in one of the possible failure
> > cases.
> > 
> > Coverity CID 1351225
> > 
> > Signed-off-by: Harmandeep Kaur 
> 
> Thanks.
> 
> Acked-by: Ian Jackson 

applied, including...

>   Reviewed-by: Dario Faggioli 
> 
> You would put that right after the Reviewed-by.

...this.

FTR I was unsure if xc_unmap_domain_meminfo would be safe to call after a
failed xc_map_domain_meminfo (which it should be, but with libxc you never
can tell) and I think it is indeed safe, although the failure paths in
xc_map_domain_meminfo are pretty complicated they do appear to always reset
fields in the struct to the "free" state.

> 
> 
> However, there is no need to resend your patch for this.  It is normal
> for a committer to search the thread for acks to the patch which
> arrive after it was posted, and fold them in.
> 
> In this case since the Subject has changed, someone needs to make sure
> that that search turns up the review, but this message of mine will
> serve that purpose.
> 
> 
> So, having said all that, thanks for your contribution :-).
> 
> Regards,
> Ian.

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

Re: [Xen-devel] [PATCH] init-xenstore-domain: cleanup all resources on a single exit path

[Ian Campbell]

On Thu, 2016-02-11 at 11:27 +, Wei Liu wrote:
> On Wed, Feb 10, 2016 at 04:56:22PM +, Ian Campbell wrote:
> > Previously xs_fd would be left open, which is CID 1055993 (previously
> > partially fixed by 3bca826aae5eb).
> > 
> > Instead arrange for both success and error cases to cleanup everything
> > on a single exit path instead of doing partial cleanup on the success
> > path a few operations higher up.
> > 
> > Signed-off-by: Ian Campbell 
> 
> Acked-by: Wei Liu 

Thanks, applied.


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

[Xen-devel] [PATCH v8 2/5] docs: add vscsi to xenstore-paths.markdown

[Olaf Hering]

Signed-off-by: Olaf Hering 
Acked-by: Ian Campbell 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 
---
 docs/misc/xenstore-paths.markdown | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/misc/xenstore-paths.markdown 
b/docs/misc/xenstore-paths.markdown
index 6a6fda9..76f67b1 100644
--- a/docs/misc/xenstore-paths.markdown
+++ b/docs/misc/xenstore-paths.markdown
@@ -265,6 +265,11 @@ A virtual keyboard device frontend. Described by
 A virtual network device frontend. Described by
 [xen/include/public/io/netif.h][NETIF]
 
+ ~/device/vscsi/$DEVID/* []
+
+A virtual scsi device frontend. Described by
+[xen/include/public/io/vscsiif.h][SCSIIF]
+
  ~/console/* []
 
 The primary PV console device. Described in [console.txt](console.txt)
@@ -335,6 +340,10 @@ A virtual keyboard device backend. Described by
 A virtual network device backend. Described by
 [xen/include/public/io/netif.h][NETIF]
 
+ ~/backend/vscsi/$DOMID/$DEVID/* []
+
+A PV SCSI backend.
+
  ~/backend/console/$DOMID/$DEVID/* []
 
 A PV console backend. Described in [console.txt](console.txt)
@@ -525,6 +534,7 @@ domain instead of a daemon in dom0.
 [KBDIF]: 
http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,kbdif.h.html
 [LIBXLMEM]: http://xenbits.xen.org/docs/unstable/misc/libxl_memory.txt
 [NETIF]: 
http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,netif.h.html
+[SCSIIF]: 
http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,vscsiif.h.html
 [SI]: 
http://xenbits.xen.org/docs/unstable/hypercall/include,public,xen.h.html#Struct_start_info
 [VCPU]: 
http://xenbits.xen.org/docs/unstable/hypercall/include,public,vcpu.h.html
 [XSWIRE]: 
http://xenbits.xen.org/docs/unstable/hypercall/include,public,io,xs_wire.h.html

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

[Xen-devel] [PATCH v8 5/5] Scripts to create and delete xen-scsiback nodes in Linux target framework

[Olaf Hering]

Just to make them public, not meant for merging:
The scripts used during development to create a bunch of SCSI devices in
dom0 using the Linux target framework. targetcli3 and rtslib3 is used.

A patch is required for python-rtslib:
http://article.gmane.org/gmane.linux.scsi.target.devel/8146

Signed-off-by: Olaf Hering 
---
 tools/misc/Makefile  |   4 +
 tools/misc/target-create-xen-scsiback.sh | 135 +++
 tools/misc/target-delete-xen-scsiback.sh |  41 ++
 3 files changed, 180 insertions(+)

diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index a2ef0ec..180c9f5 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -35,6 +35,8 @@ INSTALL_SBIN += $(INSTALL_SBIN-y)
 
 # Everything to be installed in a private bin/
 INSTALL_PRIVBIN+= xenpvnetboot
+INSTALL_PRIVBIN+= target-create-xen-scsiback.sh
+INSTALL_PRIVBIN+= target-delete-xen-scsiback.sh
 
 # Everything to be installed
 TARGETS_ALL := $(INSTALL_BIN) $(INSTALL_SBIN) $(INSTALL_PRIVBIN)
@@ -45,6 +47,8 @@ TARGETS_COPY += xen-ringwatch
 TARGETS_COPY += xencons
 TARGETS_COPY += xencov_split
 TARGETS_COPY += xenpvnetboot
+TARGETS_COPY += target-create-xen-scsiback.sh
+TARGETS_COPY += target-delete-xen-scsiback.sh
 
 # Everything which needs to be built
 TARGETS_BUILD := $(filter-out $(TARGETS_COPY),$(TARGETS_ALL))
diff --git a/tools/misc/target-create-xen-scsiback.sh 
b/tools/misc/target-create-xen-scsiback.sh
new file mode 100755
index 000..d014a0a
--- /dev/null
+++ b/tools/misc/target-create-xen-scsiback.sh
@@ -0,0 +1,135 @@
+#!/usr/bin/env bash
+unset LANG
+unset ${!LC_*}
+set -x
+set -e
+
+modprobe --version
+targetcli --version
+udevadm --version
+blockdev --version
+parted --version
+sfdisk --version
+mkswap --version
+
+configfs=/sys/kernel/config
+target_path=$configfs/target
+
+num_luns=4
+num_hosts=4
+
+case "$1" in
+   -p)
+   backend="pvops"
+   ;;
+   -x)
+   backend="xenlinux"
+   ;;
+   *)
+   : "usage: $0 [-p|-x]"
+   if grep -qw xenfs$ /proc/filesystems
+   then
+   backend="pvops"
+   else
+   backend="xenlinux"
+   fi
+   ;;
+esac
+
+get_wwn() {
+   sed '
+   s@-@@g
+   s@^\(.\{16\}\)\(.*\)@\1@
+   ' /proc/sys/kernel/random/uuid
+}
+
+if test ! -d "${target_path}"
+then
+   modprobe -v configfs
+   mount -vt configfs configfs $configfs
+   modprobe -v target_core_mod
+fi
+if test "${backend}" = "pvops"
+then
+   modprobe -v xen-scsiback
+fi
+
+host=0
+while test $host -lt $num_hosts
+do
+   host=$(( $host + 1 ))
+   lun=0
+   loopback_wwn="naa.`get_wwn`"
+   pvscsi_wwn="naa.`get_wwn`"
+   targetcli /loopback create ${loopback_wwn}
+   if test "${backend}" = "pvops"
+   then
+   targetcli /xen-pvscsi create ${pvscsi_wwn}
+   fi
+   while test $lun -lt $num_luns
+   do
+   : h $host l $lun
+   f_file=/dev/shm/Fileio.${host}.${lun}.file
+   f_uuid=/dev/shm/Fileio.${host}.${lun}.uuid
+   f_link=/dev/shm/Fileio.${host}.${lun}.link
+   fileio_name="fio_${host}.${lun}"
+   pscsi_name="ps_${host}.${lun}"
+
+   targetcli /backstores/fileio create name=${fileio_name} 
"file_or_dev=${f_file}" size=$((1024*1024 * 8 )) sparse=true
+   targetcli /loopback/${loopback_wwn}/luns create 
/backstores/fileio/${fileio_name} $lun
+
+   vpd_uuid="`sed -n '/^T10 VPD Unit Serial 
Number:/s@^[^:]\+:[[:blank:]]\+@@p' 
/sys/kernel/config/target/core/fileio_*/${fileio_name}/wwn/vpd_unit_serial`"
+   if test -z "${vpd_uuid}"
+   then
+   exit 1
+   fi
+   echo "${vpd_uuid}" > "${f_uuid}"
+   by_id="`echo ${vpd_uuid} | sed 
's@-@@g;s@^\(.\{25\}\)\(.*\)@scsi-36001405\1@'`"
+   udevadm settle --exit-if-exists="/dev/disk/by-id/${by_id}"
+   ln -sfvbn "/dev/disk/by-id/${by_id}" "${f_link}"
+
+   f_major=$((`stat --dereference --format=0x%t "${f_link}"`))
+   f_minor=$((`stat --dereference --format=0x%T "${f_link}"`))
+   if test -z "${f_major}" || test -z "${f_minor}"
+   then
+   exit 1
+   fi
+   f_alias=`ls -d 
/sys/dev/block/${f_major}:${f_minor}/device/scsi_device/*:*:*:*`
+   if test -z "${f_alias}"
+   then
+   exit 1
+   fi
+   f_alias=${f_alias##*/}
+
+   blockdev --rereadpt "${f_link}"
+   udevadm settle
+   echo 1,96,S | sfdisk "${f_link}"
+   udevadm settle
+   blockdev --rereadpt "${f_link}"
+   udevadm settle
+   parted -s "${f_link}" unit s print
+
+   d_link="`readlink \"${f_link}\"`"
+   if test -n "${d_link}"
+  

[Xen-devel] [PATCH v8 1/5] vscsiif.h: fix WWN notation for p-dev property

[Olaf Hering]

The pvops kernel expects either "naa.WWN:LUN" or "h:c:t:l" in the p-dev
property. Add the missing :LUN part to the comment.

Signed-off-by: Olaf Hering 
Acked-by: Ian Campbell 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 
---
 xen/include/public/io/vscsiif.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h
index 7a1db05..e8e38a9 100644
--- a/xen/include/public/io/vscsiif.h
+++ b/xen/include/public/io/vscsiif.h
@@ -60,7 +60,7 @@
  *
  *  A string specifying the backend device: either a 4-tuple "h:c:t:l"
  *  (host, controller, target, lun, all integers), or a WWN (e.g.
- *  "naa.60014054ac780582").
+ *  "naa.60014054ac780582:0").
  *
  * v-dev
  *  Values: string

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

[Xen-devel] [PATCH v8 3/5] libxl: add support for vscsi

[Olaf Hering]

Port pvscsi support from xend to libxl:

 vscsi=['pdev,vdev{,options}']
 xl scsi-attach
 xl scsi-detach
 xl scsi-list

Signed-off-by: Olaf Hering 
Cc: Ian Jackson 
Cc: Stefano Stabellini 
Cc: Ian Campbell 
Cc: Wei Liu 
---
 docs/man/xl.cfg.pod.5|  55 +++
 docs/man/xl.pod.1|  18 +
 tools/libxl/Makefile |   2 +
 tools/libxl/libxl.c  | 420 +
 tools/libxl/libxl.h  |  35 ++
 tools/libxl/libxl_create.c   |   1 +
 tools/libxl/libxl_device.c   |   2 +
 tools/libxl/libxl_internal.h |  19 +
 tools/libxl/libxl_types.idl  |  53 +++
 tools/libxl/libxl_types_internal.idl |   1 +
 tools/libxl/libxl_vscsi.c| 615 +++
 tools/libxl/libxlu_vscsi.c   | 697 +++
 tools/libxl/libxlutil.h  |  18 +
 tools/libxl/xl.h |   3 +
 tools/libxl/xl_cmdimpl.c | 208 ++-
 tools/libxl/xl_cmdtable.c|  15 +
 16 files changed, 2161 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8899f75..3b5b1c5 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -517,6 +517,61 @@ value is optional if this is a guest domain.
 
 =back
 
+=item B
+
+Specifies the PVSCSI devices to be provided to the guest. PVSCSI passes
+SCSI devices from the backend domain to the guest.
+
+Each VSCSI_SPEC_STRING consists of "pdev,vdev[,options]".
+'pdev' describes the physical device, preferable in a persistent format such 
as /dev/disk/by-*/*.
+'vdev' is the domU device in vHOST:CHANNEL:TARGET:LUN notation, all integers.
+'options' lists additional flags which a backend may recognize.
+
+The supported values for "pdev" and "options" depends on the backend driver 
used:
+
+=over 4
+
+=item B
+
+=over 4
+
+=item C
+
+The backend driver in the pvops kernel is part of the Linux-IO Target framework
+(LIO). As such the SCSI devices have to be configured first with the tools
+provided by this framework, such as a xen-scsiback aware targetcli. The "pdev"
+in domU.cfg has to refer to a config item in that framework instead of the raw
+device. Usually this is a WWN in the form of "na.WWN:LUN".
+
+=item C
+
+No options recognized.
+
+=back
+
+=item B
+
+=over 4
+
+=item C
+
+The dom0 device in either /dev/scsidev or pHOST:CHANNEL:TARGET:LUN notation.
+
+It's recommended to use persistent names "/dev/disk/by-*/*" to refer to a 
"pdev".
+The toolstack will translate this internally to "h:c:t:l" notation, which is 
how
+the backend driver will access the device. Using the "h:c:t:l" notation for
+"pdev" in domU.cfg is discouraged because this value will change across 
reboots,
+depending on the detection order in the OS.
+
+=item C
+
+Currently only the option value "feature-host" is recognized. SCSI command
+emulation in backend driver is bypassed when "feature-host" is specified.
+
+=back
+
+=back
+
 =item B
 
 Specifies the paravirtual framebuffer devices which should be supplied
diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 4279c7c..0674149 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1293,6 +1293,24 @@ List virtual trusted platform modules for a domain.
 
 =back
 
+=head2 PVSCSI DEVICES
+
+=over 4
+
+=item B I I I,I<[feature-host]>
+
+Creates a new vscsi device in the domain specified by I.
+
+=item B I I
+
+Removes the vscsi device from domain specified by I.
+
+=item B I I<[domain-id] ...>
+
+List vscsi devices for the domain specified by I.
+
+=back
+
 =head1 PCI PASS-THROUGH
 
 =over 4
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 620720e..a71abf2 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -99,6 +99,7 @@ endif
 LIBXL_LIBS += -lyajl
 
 LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
+   libxl_vscsi.o \
libxl_dom.o libxl_exec.o libxl_xshelp.o libxl_device.o \
libxl_internal.o libxl_utils.o libxl_uuid.o \
libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o 
\
@@ -141,6 +142,7 @@ AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h 
_paths.h \
 AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
 AUTOSRCS += _libxl_save_msgs_callout.c _libxl_save_msgs_helper.c
 LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
+   libxlu_vscsi.o \
libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o
 $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2bde0f5..7583b4b 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2059,6 +2059,417 @@ static int libxl__resolve_domid(libxl__gc *gc, const 
char *name,
 }
 
 
/**/
+
+static int libxl__device_vscsidev_backend_set_add(libxl__gc *gc,
+

[Xen-devel] [PATCH v8 0/5] libbxl: add support for pvscsi, iteration 8

[Olaf Hering]

Port vscsi=[] and scsi-{attach,detach,list} commands from xend to libxl.

libvirt uses its existing SCSI support:
http://lists.xenproject.org/archives/html/xen-devel/2015-04/msg02963.html

targetcli/rtslib has to be aware of xen-scsiback (upstream unresponsive):
http://article.gmane.org/gmane.linux.scsi.target.devel/8146

TODO:
 - comply with tools/libxl/CODING_STYLE everywhere
 - document pvscsi in xen wiki

Changes between v7 and v8:
 - Make be_path const in libxl__device_vscsictrl_add
 - Adjust years in Copyright
 - Whitespace in libxl_device_vscsictrl idl
 - Implement libxl_device_vscsictrl_destroy
 - Set backend_devid
 - Reorder assignments in libxl__device_vscsictrl_add
 - Whitespace changes for libxl__device_vscsi_reconfigure_add
 - Register watch unconditionally in libxl__device_vscsidev_rm
 - Wait for StateClosing for unconnected frontends in detach
 - Adjust logic in libxl__vscsi_fill_dev
 - Rework scsi-detach
 - Increase swap partition from 12 to 96 blocks for mkswap
 - Update MERGE macro for vscsictrls

Changes between v6 and v7:
 - rebase to 'staging' (3b971de)
 - Introduce libxl_device_vscsictrl_append_vscsidev
 - Add libxl__vscsi_collect_ctrls and used it in libxl_device_vscsictrl_list
 - Convert type of lun from u32 to u64 per SCSI spec
 - Use vscsi_saved in libxl__device_vscsictrl_add
 - Assign unique vscsidev_id per vscsictrl
 - Rename vscsi_dev to vscsidev in function names
 - Rename variables in libxl_vscsiinfo
 - Rename locals to refer to ctrl/dev
 - Rename various strings from host to ctrl
 - Rename local variables vscsi_ctrl to vscsictrl
 - Rename libxl_device_vscsictrl->vscsi_devs to vscsidevs
 - Remove libxl_device_vscsidev->remove, rework detach
 - Rename libxl_device_vscsidev->vscsi_dev_id to vscsidev_id
 - Rename local variables vscsi_host to vscsi_ctrl
 - Rename local variables v_hst to v_ctrl
 - Remove libxl_device_vscsictrl->v_hst
 - Rename libxl_vscsi_dev to libxl_device_vscsidev
 - Rename libxl_device_vscsi to libxl_device_vscsictrl
http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg00884.html

Changes between v5 and v6:
 - rebase to 'staging' (a7b39c8 and b7e7ad8)
 - Fix off-by-one in xlu__vscsi_compare_udev
 - xl.cfg: use options instead of option
 - xl.cfg: fix grammar for pdev/options
 - xl.cfg: fix Usually typo
 - Remove next_vscsi_dev_id from libxl_device_vscsi
 - Use XLU_WWN_LEN also in libxl_vscsi.c
http://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01446.html

Changes between v4 and v5:
 - vscsiif.h: refer to backend_domid
 - Set update_json in libxl_device_vscsi_remove
 - Remove comment from libxl__device_vscsi_add
 - Remove debug LOG from libxl__device_vscsi_reconfigure
 - Move local nb variable in libxl__device_vscsi_reconfigure
 - Make be_path const in libxl__device_vscsi_reconfigure
 - Adjust libxl__device_vscsi_dev_backend_set to avoid long lines
 - Adjust libxl__device_vscsi_dev_backend_rm to avoid long lines
 - Use CTX in libxl__device_vscsi_dev_backend_rm
 - Make be_path const in libxl__device_vscsi_dev_backend_rm
 - xl.cfg: Its typo
 - xl.cfg: Use persistent instead of persistant
 - Rename feature_host to scsi_raw_cmds
 - target-create-xen-scsiback.sh: detect pvops and xenlinux
 - Wrap long lines in main_vscsilist
 - Call libxl_vscsiinfo_dispose unconditional
 - Let scsi-list print p-dev instead of p-devname
 - Handle broken vscsi device entry in xenstore
 - Split libxl__vscsi_fill_host from libxl_device_vscsi_list
 - Make xlu_vscsi_append_dev static
 - Remove reference to pvscsi.txt from xenstore-paths.markdown
 - xl.cfg: update Linux and xenlinux
 - xl.cfg: refer to backend domain instead of dom0
 - xl.cfg: be more verbose what persistant format is
 - return if libxl__device_vscsi_dev_backend_set fails in 
libxl__device_vscsi_new_backend
 - target-create-xen-scsiback.sh: set also alias for libvirt
http://lists.xenproject.org/archives/html/xen-devel/2015-05/msg00523.html

Changes between v3 and v4:
 - Use libxl__device_nextid in libxl__device_vscsi_add
 - Remove check for duplicate pdev assignment from libxl_device_vscsi_get_host
 - Caller provides libxl_device_vscsi to libxl_device_vscsi_get_host
 - Define LIBXL_HAVE_VSCSI
 - Remove init_val from libxl_vscsi_pdev_type
 - Move some functions from libxl to libxlu
 - Introduce libxl_device_vscsi->next_vscsi_dev_id to handle holes
 - Use Struct in KeyedUnion for ocaml idl
 - docs: Mention pvscsi in xl and xl.cfg
 - Turn feature_host into a defbool and add checking
 - Support pvops and /dev/ nodes in config
 - Wrap entire libxlu_vscsi.c with ifdef linux
 - Set remove flag in libxl_device_vscsi_list
 - Fix vscsiif path in xenstore-paths.markdown
 - vscsiif.h: add some notes about xenstore layout
 - Add copyright to libxlu_vscsi.c and libxl_vscsi.c
 - Scripts to create and delete xen-scsiback nodes in Linux target framework
 - Remove pvscsi.txt
http://lists.xenproject.org/archives/html/xen-devel/2015-04/msg01949.html

Changes between v2 and v3:
 - Adjust change for vscsiif.h
 - Support 

[Xen-devel] [PATCH v8 4/5] vscsiif.h: add some notes about xenstore layout

[Olaf Hering]

Signed-off-by: Olaf Hering 
Acked-by: Ian Campbell 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 
---
 xen/include/public/io/vscsiif.h | 68 +
 1 file changed, 68 insertions(+)

diff --git a/xen/include/public/io/vscsiif.h b/xen/include/public/io/vscsiif.h
index e8e38a9..2c5f04a 100644
--- a/xen/include/public/io/vscsiif.h
+++ b/xen/include/public/io/vscsiif.h
@@ -104,6 +104,74 @@
  *  response structures.
  */
 
+/*
+ * Xenstore format in practice
+ * ===
+ * 
+ * The backend driver uses a single_host:many_devices notation to manage domU
+ * devices. Everything is stored in 
/local/domain//backend/vscsi/.
+ * The xenstore layout looks like this (dom0 is assumed to be the 
backend_domid):
+ * 
+ * //feature-host = "0"
+ * //frontend = "/local/domain//device/vscsi/0"
+ * //frontend-id = ""
+ * //online = "1"
+ * //state = "4"
+ * //vscsi-devs/dev-0/p-dev = "8:0:2:1" or "naa.wwn:lun"
+ * //vscsi-devs/dev-0/state = "4"
+ * //vscsi-devs/dev-0/v-dev = "0:0:0:0"
+ * //vscsi-devs/dev-1/p-dev = "8:0:2:2"
+ * //vscsi-devs/dev-1/state = "4"
+ * //vscsi-devs/dev-1/v-dev = "0:0:1:0"
+ * 
+ * The frontend driver maintains its state in
+ * /local/domain//device/vscsi/.
+ * 
+ * /backend = "/local/domain/0/backend/vscsi//"
+ * /backend-id = "0"
+ * /event-channel = "20"
+ * /ring-ref = "43"
+ * /state = "4"
+ * /vscsi-devs/dev-0/state = "4"
+ * /vscsi-devs/dev-1/state = "4"
+ * 
+ * In addition to the entries for backend and frontend these flags are stored
+ * for the toolstack:
+ * 
+ * //vscsi-devs/dev-1/p-devname = "/dev/$device"
+ * 
+ * 
+ * Backend/frontend protocol
+ * =
+ * 
+ * To create a vhost along with a device:
+ * //feature-host = "0"
+ * //frontend = "/local/domain//device/vscsi/0"
+ * //frontend-id = ""
+ * //online = "1"
+ * //state = "1"
+ * //vscsi-devs/dev-0/p-dev = "8:0:2:1"
+ * //vscsi-devs/dev-0/state = "1"
+ * //vscsi-devs/dev-0/v-dev = "0:0:0:0"
+ * Wait for //state + //vscsi-devs/dev-0/state 
become 4
+ * 
+ * To add another device to a vhost:
+ * //state = "7"
+ * //vscsi-devs/dev-1/p-dev = "8:0:2:2"
+ * //vscsi-devs/dev-1/state = "1"
+ * //vscsi-devs/dev-1/v-dev = "0:0:1:0"
+ * Wait for //state + //vscsi-devs/dev-1/state 
become 4
+ * 
+ * To remove a device from a vhost:
+ * //state = "7"
+ * //vscsi-devs/dev-1/state = "5"
+ * Wait for //state to become 4
+ * Wait for //vscsi-devs/dev-1/state become 6
+ * Remove //vscsi-devs/dev-1/{state,p-dev,v-dev,p-devname}
+ * Remove //vscsi-devs/dev-1/
+ *
+ */
+
 /* Requests from the frontend to the backend */
 
 /*

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

Re: [Xen-devel] [PATCH v2] tools: libxl: make it illegal to pass libxl__realloc(gc) a non-gc ptr

[Ian Campbell]

On Thu, 2016-02-11 at 11:37 +, Wei Liu wrote:
> On Thu, Feb 11, 2016 at 09:23:54AM +, Ian Campbell wrote:
> > That is, if gc is not NOGC and ptr is not NULL then ptr must be
> > associated gc.
> > 
> 
> "associated with gc"?
> 
> Anyway, I get the idea.

Yeah, apparently I was having language issues yesterday (v1 had a different
mangling elsewhere)

> > Currently in this case the new_ptr would not be registered with any
> > gc, which Coverity rightly points out (in various different places)
> > would be a memory leak.
> > 
> > It would also be possible to fix this by adding a libxl__ptr_add() at
> > the same point, however semantically it seems like a programming error
> > to gc-realloc a pointer which is not associated with the gc in
> > question, so treat it as such.
> > 
> > Compile tested only, this change could expose latent bugs.
> > 
> > Signed-off-by: Ian Campbell 
> 
> Acked-by: Wei Liu 

Thanks. I corrected the above to "with a gc" and applied.


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

[Xen-devel] [PATCH] tools/console: correct make dependencies for _paths.h

[Olaf Hering]

Correct dependencies for _paths.h to avoid build failure with make -j.
Only main.c requires _paths.h. This fixes commit 8398ec70 ("xenconsole:
Ensure exclusive access to console using locks")

Signed-off-by: Olaf Hering 
Cc: Ian Jackson 
Cc: Stefano Stabellini 
Cc: Ian Campbell 
Cc: Wei Liu 
---

Should be backported to 4.6


 tools/console/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/console/Makefile b/tools/console/Makefile
index 6eeac8f..a7bec75 100644
--- a/tools/console/Makefile
+++ b/tools/console/Makefile
@@ -30,7 +30,8 @@ daemon/io.o: CFLAGS += $(CFLAGS_libxenevtchn) 
$(CFLAGS_libxengnttab)
 xenconsoled: $(patsubst %.c,%.o,$(wildcard daemon/*.c))
$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS) $(LDLIBS_libxenevtchn) 
$(LDLIBS_libxengnttab) $(LDLIBS_xenconsoled) $(APPEND_LDFLAGS)
 
-xenconsole: client/_paths.h $(patsubst %.c,%.o,$(wildcard client/*.c))
+client/main.o: client/_paths.h
+xenconsole: $(patsubst %.c,%.o,$(wildcard client/*.c))
$(CC) $(LDFLAGS) $^ -o $@ $(LDLIBS) $(LDLIBS_xenconsole) 
$(APPEND_LDFLAGS)
 
 genpath-target = $(call buildmakevars2header,client/_paths.h)

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

Re: [Xen-devel] [PATCH] public/io/netif.h: fix typos

[Ian Campbell]

On Thu, 2016-02-11 at 09:53 +, Ian Campbell wrote:
> On Wed, 2016-02-10 at 16:49 +, Paul Durrant wrote:
> > Unfortunately my patch 162a81ab "document control ring and toeplitz
> > hashing" contained a couple of typos. This patch fixes them.
> > 
> > Signed-off-by: Paul Durrant 
> > Cc: Ian Campbell 
> > Cc: Ian Jackson 
> > Cc: Jan Beulich 
> > Cc: Keir Fraser 
> > Cc: Tim Deegan 
> 
> 
> Acked-by: Ian Campbell 

and applied.


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

Re: [Xen-devel] [PATCH v3 1/2] build: specify minimum versions of gcc and binutils

[Ian Campbell]

On Thu, 2016-02-11 at 06:32 -0700, Jan Beulich wrote:
> > > > On 11.02.16 at 14:21,  wrote:
> > On Thu, 2016-02-11 at 12:31 +, Andrew Cooper wrote:
> > > On 11/02/16 12:23, Ian Campbell wrote:
> > > > From: Doug Goldstein 
> > > > 
> > > > To help people avoid having to figure out what versions of gcc and
> > > > binutils need to be supported document them explicitly.
> > > > 
> > > > Signed-off-by: Doug Goldstein 
> > > > Signed-off-by: Ian Campbell 
> > > 
> > > I would be tempted to abbreviate this to GCC 4.1 and Binutls 2.16,
> > > unless we specifically know of issues between the release and point
> > > releases.
> > 
> > I don't mind doing this but if there is any quibbling at all about it
> > then
> > I would simply go with what is below.
> 
> Yeah, I'd prefer the patches to go in as they're now: Both
> Acked-by: Jan Beulich 

I have applied them both with you ack and Andy's R-by since he said "Either
way".

Thanks everyone.

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

Re: [Xen-devel] [PATCH v2 3/7] xen/vm-events: Move monitor_domctl to common-side.

[Tamas K Lengyel]

>
> * the #ifdefs make it possible for that code to be put in common => that
> makes it *clear* that those code parts are NOT
> architecture specific and their implementation can be safely used for all
> architectures.
>

The current practice has been to put empty static inline functions into
architecture-specific headers if the part is not handled/implemented for
the specific architecture. This has already been the case for
monitor_domctl (there is already separate asm-arm/monitor.h and
asm-x86/monitor.h) so it should be followed as more of the code moves into
common.

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

Re: [Xen-devel] [PATCH 1/2] xenmon: close qos_fd when finished with it in alloc_qos_data

[Ian Campbell]

On Thu, 2016-02-11 at 11:18 +, Wei Liu wrote:
> On Wed, Feb 10, 2016 at 04:26:24PM +, Ian Campbell wrote:
> > Otherwise the fd is leaked. NB the mmap'd memory in the global
> > cpu_qos_data[n] is not affected by closing the underlying fd.
> > 
> > Compile tested only.
> > 
> > CID: 1055930
> > 
> > Signed-off-by: Ian Campbell 
> 
> Acked-by: Wei Liu 

Applied both, thanks.

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

Re: [Xen-devel] [PATCH v4 01/17] Xen: ACPI: Hide UART used by Xen

[Stefano Stabellini]

On Wed, 10 Feb 2016, Rafael J. Wysocki wrote:
> On Tuesday, February 09, 2016 11:19:02 AM Stefano Stabellini wrote:
> > On Mon, 8 Feb 2016, Rafael J. Wysocki wrote:
> > > On Monday, February 08, 2016 10:57:01 AM Stefano Stabellini wrote:
> > > > On Sat, 6 Feb 2016, Rafael J. Wysocki wrote:
> > > > > On Fri, Feb 5, 2016 at 4:05 AM, Shannon Zhao 
> > > > >  wrote:
> > > > > > From: Shannon Zhao 
> > > > > >
> > > > > > ACPI 6.0 introduces a new table STAO to list the devices which are 
> > > > > > used
> > > > > > by Xen and can't be used by Dom0. On Xen virtual platforms, the 
> > > > > > physical
> > > > > > UART is used by Xen. So here it hides UART from Dom0.
> > > > > >
> > > > > > Signed-off-by: Shannon Zhao 
> > > > > > Reviewed-by: Stefano Stabellini 
> > > > > 
> > > > > Well, this doesn't look right to me.
> > > > > 
> > > > > We need to find a nicer way to achieve what you want.
> > > > 
> > > > I take that you are talking about how to honor the STAO table in Linux.
> > > > Do you have any concrete suggestions?
> > > 
> > > I do.
> > > 
> > > The last hunk of the patch is likely what it needs to be, although I'm
> > > not sure if the place it is added to is the right one.  That's a minor 
> > > thing,
> > > though.
> > > 
> > > The other part is problematic.  Not that as it doesn't work, but because 
> > > of
> > > how it works.  With these changes the device will be visible to the OS (in
> > > fact to user space even), but will never be "present".  I'm not sure if
> > > that's what you want?
> > > 
> > > It might be better to add a check to acpi_bus_type_and_status() that will
> > > evaluate the "should ignore?" thing and return -ENODEV if this is true.  
> > > This
> > > way the device won't be visible at all.
> > 
> > Something like below?  Actually your suggestion is better, thank you!
> > 
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 78d5f02..4778c51 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -1455,6 +1455,9 @@ static int acpi_bus_type_and_status(acpi_handle 
> > handle, int *type,
> > if (ACPI_FAILURE(status))
> > return -ENODEV;
> >  
> > +   if (acpi_check_device_is_ignored(handle))
> > +   return -ENODEV;
> > +
> > switch (acpi_type) {
> > case ACPI_TYPE_ANY: /* for ACPI_ROOT_OBJECT */
> > case ACPI_TYPE_DEVICE:
> > 
> 
> I thought about doing that under ACPI_TYPE_DEVICE, because it shouldn't be
> applicable to the other types.  But generally, yes.

I was pondering about it myself. Maybe an ACPI_TYPE_PROCESSOR object
could theoretically be hidden with the STAO? I added the check before
the switch because I thought that there would be no harm in being
caution about it.


> Plus I'd move the table checks to acpi_scan_init(), so the UART address can
> be a static variable in scan.c.
>
> Also maybe rename acpi_check_device_is_ignored() to something like
> acpi_device_should_be_hidden().

Both make sense. Shannon, are you happy to make these changes?

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

Re: [Xen-devel] [PATCH] tools/console: correct make dependencies for _paths.h

[Ian Campbell]

On Thu, 2016-02-11 at 15:38 +, Olaf Hering wrote:
> Correct dependencies for _paths.h to avoid build failure with make -j.
> Only main.c requires _paths.h. This fixes commit 8398ec70 ("xenconsole:
> Ensure exclusive access to console using locks")
> 
> Signed-off-by: Olaf Hering 
> Cc: Ian Jackson 
> Cc: Stefano Stabellini 
> Cc: Ian Campbell 
> Cc: Wei Liu 

Acked-by: Ian Campbell 


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

Re: [Xen-devel] [PATCH v2] tools: probe for existence of qemu-xen stderr trace backend.

[Stefano Stabellini]

On Thu, 11 Feb 2016, Ian Campbell wrote:
> QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
> log") renamed the "stderr" trace backend to "log", which breaks the
> xen build when pointed at a QEMU tree after that point:
> 
> ./configure of QEMU fail with:
> "ERROR: invalid trace backends
> Please choose supported trace backends."
> 
> Upstream also changed, in baf86d6b3ca0 ("trace: switch default backend
> to "log""), to use "log" as the default backend (previously it was
> "nop").
> 
> Use ./scripts/tracetool.py to check for the presence of the stderr
> backend and if it is present then explicitly enable it. If the stderr
> backend is not present then assume a newer QEMU which defaults to
> "log" and simply accept that default (there is a 1 commit window
> upstream where this would result in no trace backend being enabled).
> 
> The check is done using the older (deprecated?) --check-backend/--backend
> variant of the tracetool.py options rather than the new plural
> versions since the singular was supported even by very old versions of
> QEMU.  New QEMU has compatibility code but if/when that is removed we
> will still do the right thing i.e. no explict configuiration resulting
> in the upstream default (currently "log").
> 
> If the explicit selection of the "stderr" backend is required then it
> is now done unconditionally (not depending on debug=y), which is
> simpler to arrange here but also matches the newer upstream's default
> to "log" which is not conditional on debug being enabled either.
> 
> Tested with current qemu-xen-unstable (e9d8252) and current QEMU
> upstream master (88c73d1), both out of tree via
> QEMU_UPSTREAM_URL=/path/to/qemu-xen.git.
> 
> Signed-off-by: Ian Campbell 
> Cc: Paul Durrant 
> Cc: Anthony PERARD 

Reviewed-by: Stefano Stabellini 


> v2: Just probe for stderr, otherwise assume the new upstream default
> of log is what we will get anyway. Substantially rewrite commit
> message.
> ---
>  tools/Makefile | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index cb8652c..3f45fb9 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -228,7 +228,7 @@ qemu-xen-dir-force-update: qemu-xen-dir-find
>   fi
>  
>  ifeq ($(debug),y)
> -QEMU_XEN_ENABLE_DEBUG := --enable-debug --enable-trace-backend=stderr
> +QEMU_XEN_ENABLE_DEBUG := --enable-debug
>  else
>  QEMU_XEN_ENABLE_DEBUG :=
>  endif
> @@ -240,8 +240,14 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>   source=.; \
>   fi; \
>   cd qemu-xen-dir; \
> + if $$source/scripts/tracetool.py --check-backend --backend stderr ; 
> then \
> + enable_trace_backend='--enable-trace-backend=stderr'; \
> + else \
> + enable_trace_backend='' ; \
> + fi ; \
>   $$source/configure --enable-xen --target-list=i386-softmmu \
>   $(QEMU_XEN_ENABLE_DEBUG) \
> + $$enable_trace_backend \
>   --prefix=$(LIBEXEC) \
>   --libdir=$(LIBEXEC_LIB) \
>   --includedir=$(LIBEXEC_INC) \
> -- 
> 2.1.4
> 

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

[Xen-devel] [BUG] gdbsx crashes Xen

[Carl Patenaude Poulin]

Hey folks,

With gdbsx running, Running `gdb -ex "target remote localhost:"
crashes Xen and reboots the server.

Example session: http://i.imgur.com/UXh3RCy.png

Minimal reproduction: https://github.com/lilred/xen-crash-repro

Top line of `xl dmesg`:

Xen version 4.6.0 (r...@cs.mcgill.ca) (gcc (Ubuntu 5.2.1-22ubuntu2)
5.2.1 20151010) debug=y Mon Dec 28 21:34:16 EST 2015

Output of `lsb_release -a`:

No LSB modules are available.
Distributor ID: Ubuntu
Description:Ubuntu 15.10
Release:15.10
Codename:   wily

I can provide more information if you tell me what to look for. Also,
I would LOVE a work-around.

Best

Carl Patenaude Poulin
B. Eng. Software Engineering student
McGill University
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] tools: probe for existence of qemu-xen stderr trace backend.

[Wei Liu]

On Thu, Feb 11, 2016 at 12:11:21PM +, Ian Campbell wrote:
> QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
> log") renamed the "stderr" trace backend to "log", which breaks the
> xen build when pointed at a QEMU tree after that point:
> 
> ./configure of QEMU fail with:
> "ERROR: invalid trace backends
> Please choose supported trace backends."
> 
> Upstream also changed, in baf86d6b3ca0 ("trace: switch default backend
> to "log""), to use "log" as the default backend (previously it was
> "nop").
> 
> Use ./scripts/tracetool.py to check for the presence of the stderr
> backend and if it is present then explicitly enable it. If the stderr
> backend is not present then assume a newer QEMU which defaults to
> "log" and simply accept that default (there is a 1 commit window
> upstream where this would result in no trace backend being enabled).
> 
> The check is done using the older (deprecated?) --check-backend/--backend
> variant of the tracetool.py options rather than the new plural
> versions since the singular was supported even by very old versions of
> QEMU.  New QEMU has compatibility code but if/when that is removed we
> will still do the right thing i.e. no explict configuiration resulting
> in the upstream default (currently "log").
> 
> If the explicit selection of the "stderr" backend is required then it
> is now done unconditionally (not depending on debug=y), which is
> simpler to arrange here but also matches the newer upstream's default
> to "log" which is not conditional on debug being enabled either.
> 
> Tested with current qemu-xen-unstable (e9d8252) and current QEMU
> upstream master (88c73d1), both out of tree via
> QEMU_UPSTREAM_URL=/path/to/qemu-xen.git.
> 
> Signed-off-by: Ian Campbell 
> Cc: Paul Durrant 
> Cc: Anthony PERARD 

Acked-by: Wei Liu 

> ---
> v2: Just probe for stderr, otherwise assume the new upstream default
> of log is what we will get anyway. Substantially rewrite commit
> message.
> ---
>  tools/Makefile | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/Makefile b/tools/Makefile
> index cb8652c..3f45fb9 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -228,7 +228,7 @@ qemu-xen-dir-force-update: qemu-xen-dir-find
>   fi
>  
>  ifeq ($(debug),y)
> -QEMU_XEN_ENABLE_DEBUG := --enable-debug --enable-trace-backend=stderr
> +QEMU_XEN_ENABLE_DEBUG := --enable-debug
>  else
>  QEMU_XEN_ENABLE_DEBUG :=
>  endif
> @@ -240,8 +240,14 @@ subdir-all-qemu-xen-dir: qemu-xen-dir-find
>   source=.; \
>   fi; \
>   cd qemu-xen-dir; \
> + if $$source/scripts/tracetool.py --check-backend --backend stderr ; 
> then \
> + enable_trace_backend='--enable-trace-backend=stderr'; \
> + else \
> + enable_trace_backend='' ; \
> + fi ; \
>   $$source/configure --enable-xen --target-list=i386-softmmu \
>   $(QEMU_XEN_ENABLE_DEBUG) \
> + $$enable_trace_backend \
>   --prefix=$(LIBEXEC) \
>   --libdir=$(LIBEXEC_LIB) \
>   --includedir=$(LIBEXEC_INC) \
> -- 
> 2.1.4
> 

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

Re: [Xen-devel] [BUG] gdbsx crashes Xen

[Andrew Cooper]

On 11/02/16 16:25, Carl Patenaude Poulin wrote:
> Hey folks,
>
> With gdbsx running, Running `gdb -ex "target remote localhost:"
> crashes Xen and reboots the server.
>
> Example session: http://i.imgur.com/UXh3RCy.png
>
> Minimal reproduction: https://github.com/lilred/xen-crash-repro
>
> Top line of `xl dmesg`:
>
> Xen version 4.6.0 (r...@cs.mcgill.ca )
> (gcc (Ubuntu 5.2.1-22ubuntu2) 5.2.1 20151010) debug=y Mon Dec 28
> 21:34:16 EST 2015
>
> Output of `lsb_release -a`:
>
> No LSB modules are available.
> Distributor ID: Ubuntu
> Description:Ubuntu 15.10
> Release:15.10
> Codename:   wily
>
> I can provide more information if you tell me what to look for. Also,
> I would LOVE a work-around.

At the very minimum, you need to set up a serial console and capture the
panic log.

Are you on Broadwell hardware by any chance?  I fixed a SMAP violation
in that hypercall a little while ago.

~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] tools/console: correct make dependencies for _paths.h

[Ian Campbell]

On Thu, 2016-02-11 at 16:03 +, Ian Campbell wrote:
> On Thu, 2016-02-11 at 15:38 +, Olaf Hering wrote:
> > Correct dependencies for _paths.h to avoid build failure with make -j.
> > Only main.c requires _paths.h. This fixes commit 8398ec70 ("xenconsole:
> > Ensure exclusive access to console using locks")
> > 
> > Signed-off-by: Olaf Hering 
> > Cc: Ian Jackson 
> > Cc: Stefano Stabellini 
> > Cc: Ian Campbell 
> > Cc: Wei Liu 
> 
> Acked-by: Ian Campbell 

and applied, thanks.


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

Re: [Xen-devel] [PATCH v2 1/6] xen: factor out p2m list allocation into separate function

[Daniel Kiper]

On Thu, Feb 11, 2016 at 01:38:10PM +0100, Juergen Gross wrote:
> On 11/02/16 13:19, Daniel Kiper wrote:
> > On Thu, Feb 11, 2016 at 08:53:21AM +0100, Juergen Gross wrote:
> >> Do the p2m list allocation of the to be loaded kernel in a separate
> >> function. This will allow doing the p2m list allocation at different
> >> times of the boot preparations depending on the features the kernel
> >> is supporting.
> >>
> >> While at this remove superfluous setting of first_p2m_pfn and
> >> nr_p2m_frames as those are needed only in case of the p2m list not
> >> being mapped by the initial kernel mapping.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/loader/i386/xen.c | 70 
> >> ++---
> >>  1 file changed, 40 insertions(+), 30 deletions(-)
> >>
> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> >> index c4d9689..42ed7c7 100644
> >> --- a/grub-core/loader/i386/xen.c
> >> +++ b/grub-core/loader/i386/xen.c
> >> @@ -52,6 +52,8 @@ static struct grub_xen_file_info xen_inf;
> >>  static struct xen_multiboot_mod_list *xen_module_info_page;
> >>  static grub_uint64_t modules_target_start;
> >>  static grub_size_t n_modules;
> >> +static struct grub_relocator_xen_state state;
> >> +static grub_xen_mfn_t *virt_mfn_list;
> >
> > Do we strongly need this as globals? I suppose that
> > both of them could be local to grub_xen_boot.
>
> This would require passing the state pointer to many other functions.
> Same applies to virt_mfn_list.
>
> I just followed the style used in the source already: variables used in
> multiple functions are mostly defined globally (there are even some
> which are used in one function only).

Well, I do not like that style but if maintainer do not object I will
do not complain more here about that.

> >>  #define PAGE_SIZE 4096
> >>  #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
> >> @@ -166,7 +168,7 @@ generate_page_table (grub_uint64_t *where, 
> >> grub_uint64_t paging_start,
> >>  }
> >>
> >>  static grub_err_t
> >> -set_mfns (grub_xen_mfn_t * new_mfn_list, grub_xen_mfn_t pfn)
> >> +set_mfns (grub_xen_mfn_t pfn)
> >>  {
> >>grub_xen_mfn_t i, t;
> >>grub_xen_mfn_t cn_pfn = -1, st_pfn = -1;
> >> @@ -175,32 +177,32 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, 
> >> grub_xen_mfn_t pfn)
> >>
> >>for (i = 0; i < grub_xen_start_page_addr->nr_pages; i++)
> >>  {
> >> -  if (new_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn)
> >> +  if (virt_mfn_list[i] == grub_xen_start_page_addr->console.domU.mfn)
> >>cn_pfn = i;
> >> -  if (new_mfn_list[i] == grub_xen_start_page_addr->store_mfn)
> >> +  if (virt_mfn_list[i] == grub_xen_start_page_addr->store_mfn)
> >>st_pfn = i;
> >>  }
> >>if (cn_pfn == (grub_xen_mfn_t)-1)
> >>  return grub_error (GRUB_ERR_BUG, "no console");
> >>if (st_pfn == (grub_xen_mfn_t)-1)
> >>  return grub_error (GRUB_ERR_BUG, "no store");
> >> -  t = new_mfn_list[pfn];
> >> -  new_mfn_list[pfn] = new_mfn_list[cn_pfn];
> >> -  new_mfn_list[cn_pfn] = t;
> >> -  t = new_mfn_list[pfn + 1];
> >> -  new_mfn_list[pfn + 1] = new_mfn_list[st_pfn];
> >> -  new_mfn_list[st_pfn] = t;
> >> -
> >> -  m2p_updates[0].ptr = page2offset (new_mfn_list[pfn]) | 
> >> MMU_MACHPHYS_UPDATE;
> >> +  t = virt_mfn_list[pfn];
> >> +  virt_mfn_list[pfn] = virt_mfn_list[cn_pfn];
> >> +  virt_mfn_list[cn_pfn] = t;
> >> +  t = virt_mfn_list[pfn + 1];
> >> +  virt_mfn_list[pfn + 1] = virt_mfn_list[st_pfn];
> >> +  virt_mfn_list[st_pfn] = t;
> >> +
> >> +  m2p_updates[0].ptr = page2offset (virt_mfn_list[pfn]) | 
> >> MMU_MACHPHYS_UPDATE;
> >>m2p_updates[0].val = pfn;
> >>m2p_updates[1].ptr =
> >> -page2offset (new_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE;
> >> +page2offset (virt_mfn_list[pfn + 1]) | MMU_MACHPHYS_UPDATE;
> >>m2p_updates[1].val = pfn + 1;
> >>m2p_updates[2].ptr =
> >> -page2offset (new_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE;
> >> +page2offset (virt_mfn_list[cn_pfn]) | MMU_MACHPHYS_UPDATE;
> >>m2p_updates[2].val = cn_pfn;
> >>m2p_updates[3].ptr =
> >> -page2offset (new_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE;
> >> +page2offset (virt_mfn_list[st_pfn]) | MMU_MACHPHYS_UPDATE;
> >>m2p_updates[3].val = st_pfn;
> >>
> >>grub_xen_mmu_update (m2p_updates, 4, NULL, DOMID_SELF);
> >> @@ -209,34 +211,43 @@ set_mfns (grub_xen_mfn_t * new_mfn_list, 
> >> grub_xen_mfn_t pfn)
> >>  }
> >>
> >>  static grub_err_t
> >> +grub_xen_p2m_alloc (void)
> >> +{
> >> +  grub_relocator_chunk_t ch;
> >> +  grub_size_t p2msize;
> >> +  grub_err_t err;
> >> +
> >> +  state.mfn_list = max_addr;
> >> +  next_start.mfn_list = max_addr + xen_inf.virt_base;
> >> +  p2msize = sizeof (grub_xen_mfn_t) * grub_xen_start_page_addr->nr_pages;
> >> +  err = grub_relocator_alloc_chunk_addr (relocator, &ch, max_addr, 
> >> p2msize);
> >
> > Hmmm Where relocator is defined?
>
> First global variable in this source.
>
>

Re: [Xen-devel] [PATCH v2] tools: probe for existence of qemu-xen stderr trace backend.

[Ian Campbell]

On Thu, 2016-02-11 at 16:30 +, Wei Liu wrote:
> On Thu, Feb 11, 2016 at 12:11:21PM +, Ian Campbell wrote:
> > QEMU upstream commit ed7f5f1d8db0 ("trace: convert stderr backend to
> > log") renamed the "stderr" trace backend to "log", which breaks the
> > xen build when pointed at a QEMU tree after that point:
> > 
> > ./configure of QEMU fail with:
> > "ERROR: invalid trace backends
> > Please choose supported trace backends."
> > 
> > Upstream also changed, in baf86d6b3ca0 ("trace: switch default backend
> > to "log""), to use "log" as the default backend (previously it was
> > "nop").
> > 
> > Use ./scripts/tracetool.py to check for the presence of the stderr
> > backend and if it is present then explicitly enable it. If the stderr
> > backend is not present then assume a newer QEMU which defaults to
> > "log" and simply accept that default (there is a 1 commit window
> > upstream where this would result in no trace backend being enabled).
> > 
> > The check is done using the older (deprecated?) --check-backend/
> > --backend
> > variant of the tracetool.py options rather than the new plural
> > versions since the singular was supported even by very old versions of
> > QEMU.  New QEMU has compatibility code but if/when that is removed we
> > will still do the right thing i.e. no explict configuiration resulting
> > in the upstream default (currently "log").
> > 
> > If the explicit selection of the "stderr" backend is required then it
> > is now done unconditionally (not depending on debug=y), which is
> > simpler to arrange here but also matches the newer upstream's default
> > to "log" which is not conditional on debug being enabled either.
> > 
> > Tested with current qemu-xen-unstable (e9d8252) and current QEMU
> > upstream master (88c73d1), both out of tree via
> > QEMU_UPSTREAM_URL=/path/to/qemu-xen.git.
> > 
> > Signed-off-by: Ian Campbell 
> > Cc: Paul Durrant 
> > Cc: Anthony PERARD 
> 
> Acked-by: Wei Liu 

Thanks, applied.


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

Re: [Xen-devel] [PATCH v2 3/6] xen: factor out allocation of page tables into separate function

[Daniel Kiper]

On Thu, Feb 11, 2016 at 01:53:51PM +0100, Juergen Gross wrote:
> On 11/02/16 13:27, Daniel Kiper wrote:
> > On Thu, Feb 11, 2016 at 08:53:23AM +0100, Juergen Gross wrote:
> >> Do the allocation of page tables in a separate function. This will
> >> allow to do the allocation at different times of the boot preparations
> >> depending on the features the kernel is supporting.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/loader/i386/xen.c | 82 
> >> -
> >>  1 file changed, 51 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> >> index e48cc3f..65cec27 100644
> >> --- a/grub-core/loader/i386/xen.c
> >> +++ b/grub-core/loader/i386/xen.c
> >> @@ -56,6 +56,9 @@ static struct grub_relocator_xen_state state;
> >>  static grub_xen_mfn_t *virt_mfn_list;
> >>  static struct start_info *virt_start_info;
> >>  static grub_xen_mfn_t console_pfn;
> >> +static grub_uint64_t *virt_pgtable;
> >> +static grub_uint64_t pgtbl_start;
> >> +static grub_uint64_t pgtbl_end;
> >
> > Same as in patches #1 and #2.
>
> Yep.
>
> >
> >>  #define PAGE_SIZE 4096
> >>  #define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
> >> @@ -106,17 +109,17 @@ get_pgtable_size (grub_uint64_t total_pages, 
> >> grub_uint64_t virt_base)
> >>
> >>  static void
> >>  generate_page_table (grub_uint64_t *where, grub_uint64_t paging_start,
> >> -   grub_uint64_t total_pages, grub_uint64_t virt_base,
> >> -   grub_xen_mfn_t *mfn_list)
> >> +   grub_uint64_t paging_end, grub_uint64_t total_pages,
> >> +   grub_uint64_t virt_base, grub_xen_mfn_t *mfn_list)
> >>  {
> >>if (!virt_base)
> >> -total_pages++;
> >> +paging_end++;
> >>
> >>grub_uint64_t lx[NUMBER_OF_LEVELS], lxs[NUMBER_OF_LEVELS];
> >>grub_uint64_t nlx, nls, sz = 0;
> >>int l;
> >>
> >> -  nlx = total_pages;
> >> +  nlx = paging_end;
> >>nls = virt_base >> PAGE_SHIFT;
> >>for (l = 0; l < NUMBER_OF_LEVELS; l++)
> >>  {
> >> @@ -160,7 +163,7 @@ generate_page_table (grub_uint64_t *where, 
> >> grub_uint64_t paging_start,
> >>if (pr)
> >>  pg += POINTERS_PER_PAGE;
> >>
> >> -  for (j = 0; j < total_pages; j++)
> >> +  for (j = 0; j < paging_end; j++)
> >>  {
> >>if (j >= paging_start && j < lp)
> >>pg[j + lxs[0]] = page2offset (mfn_list[j]) | 5;
> >> @@ -261,24 +264,12 @@ grub_xen_special_alloc (void)
> >>  }
> >>
> >>  static grub_err_t
> >> -grub_xen_boot (void)
> >> +grub_xen_pt_alloc (void)
> >>  {
> >>grub_relocator_chunk_t ch;
> >>grub_err_t err;
> >>grub_uint64_t nr_info_pages;
> >>grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
> >> -  struct gnttab_set_version gnttab_setver;
> >> -  grub_size_t i;
> >> -
> >> -  if (grub_xen_n_allocated_shared_pages)
> >> -return grub_error (GRUB_ERR_BUG, "active grants");
> >> -
> >> -  err = grub_xen_p2m_alloc ();
> >> -  if (err)
> >> -return err;
> >> -  err = grub_xen_special_alloc ();
> >> -  if (err)
> >> -return err;
> >>
> >>next_start.pt_base = max_addr + xen_inf.virt_base;
> >>state.paging_start = max_addr >> PAGE_SHIFT;
> >> @@ -298,30 +289,59 @@ grub_xen_boot (void)
> >>nr_pages = nr_need_pages;
> >>  }
> >>
> >> -  grub_dprintf ("xen", "bootstrap domain %llx+%llx\n",
> >> -  (unsigned long long) xen_inf.virt_base,
> >> -  (unsigned long long) page2offset (nr_pages));
> >> -
> >>err = grub_relocator_alloc_chunk_addr (relocator, &ch,
> >> max_addr, page2offset (nr_pt_pages));
> >>if (err)
> >>  return err;
> >>
> >> +  virt_pgtable = get_virtual_current_address (ch);
> >> +  pgtbl_start = max_addr >> PAGE_SHIFT;
> >> +  max_addr += page2offset (nr_pt_pages);
> >> +  state.stack = max_addr + STACK_SIZE + xen_inf.virt_base;
> >> +  state.paging_size = nr_pt_pages;
> >> +  next_start.nr_pt_frames = nr_pt_pages;
> >> +  max_addr = page2offset (nr_pages);
> >> +  pgtbl_end = nr_pages;
> >> +
> >> +  return GRUB_ERR_NONE;
> >> +}
> >> +
> >> +static grub_err_t
> >> +grub_xen_boot (void)
> >> +{
> >> +  grub_err_t err;
> >> +  grub_uint64_t nr_pages;
> >> +  struct gnttab_set_version gnttab_setver;
> >> +  grub_size_t i;
> >> +
> >> +  if (grub_xen_n_allocated_shared_pages)
> >> +return grub_error (GRUB_ERR_BUG, "active grants");
> >> +
> >> +  err = grub_xen_p2m_alloc ();
> >> +  if (err)
> >> +return err;
> >> +  err = grub_xen_special_alloc ();
> >> +  if (err)
> >> +return err;
> >> +  err = grub_xen_pt_alloc ();
> >> +  if (err)
> >> +return err;
> >
> > Should not we free memory allocated by grub_xen_p2m_alloc() and
> > grub_xen_special_alloc() if grub_xen_pt_alloc() failed?
>
> Hmm, why? If I don't miss anything freeing memory in case of error isn't
> done anywhere (at least not in this source file).

It seems that memory is freed by higher level functions when you
try to us

Re: [Xen-devel] [PATCH v2 4/6] xen: add capability to load initrd outside of initial mapping

[Daniel Kiper]

On Thu, Feb 11, 2016 at 03:13:40PM +0100, Juergen Gross wrote:
> On 11/02/16 13:33, Daniel Kiper wrote:
> > On Thu, Feb 11, 2016 at 08:53:24AM +0100, Juergen Gross wrote:
> >> Modern pvops linux kernels support an initrd not covered by the initial
> >> mapping. This capability is flagged by an elf-note.
> >>
> >> In case the elf-note is set by the kernel don't place the initrd into
> >> the initial mapping. This will allow to load larger initrds and/or
> >> support domains with larger memory, as the initial mapping is limited
> >> to 2GB and it is containing the p2m list.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/loader/i386/xen.c| 56 
> >> ++
> >>  grub-core/loader/i386/xen_fileXX.c |  3 ++
> >>  include/grub/xen_file.h|  1 +
> >>  3 files changed, 49 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> >> index 65cec27..0f41048 100644
> >> --- a/grub-core/loader/i386/xen.c
> >> +++ b/grub-core/loader/i386/xen.c
> >> @@ -307,15 +307,14 @@ grub_xen_pt_alloc (void)
> >>  }
> >>
> >>  static grub_err_t
> >> -grub_xen_boot (void)
> >> +grub_xen_alloc_end (void)
> >>  {
> >>grub_err_t err;
> >> -  grub_uint64_t nr_pages;
> >> -  struct gnttab_set_version gnttab_setver;
> >> -  grub_size_t i;
> >> +  static int called = 0;
> >>
> >> -  if (grub_xen_n_allocated_shared_pages)
> >> -return grub_error (GRUB_ERR_BUG, "active grants");
> >> +  if (called)
> >> +return GRUB_ERR_NONE;
> >
> > Why?
>
> Did you look at the function? grub_xen_alloc_end() (some parts of the
> original grub_xen_boot()) is new and may be called multiple times. It
> was much easier to just call it and let it do what must be done only
> at the first time called.

OK, but this means that you can use this loader only once. If you make
a mistake in boot command line then you cannot fix it by calling this
loader again because grub_xen_alloc_end() will fail. Am I right?

> >> +  called = 1;
> >>
> >>err = grub_xen_p2m_alloc ();
> >>if (err)
> >> @@ -327,6 +326,24 @@ grub_xen_boot (void)
> >>if (err)
> >>  return err;
> >>
> >> +  return GRUB_ERR_NONE;
> >> +}
> >> +
> >> +static grub_err_t
> >> +grub_xen_boot (void)
> >> +{
> >> +  grub_err_t err;
> >> +  grub_uint64_t nr_pages;
> >> +  struct gnttab_set_version gnttab_setver;
> >> +  grub_size_t i;
> >> +
> >> +  if (grub_xen_n_allocated_shared_pages)
> >> +return grub_error (GRUB_ERR_BUG, "active grants");
> >
> > Why?
>
> That's how it has been before. git just decided to generate the diff
> that way.

Could you try --patience git option?

> >> +  err = grub_xen_alloc_end ();
> >> +  if (err)
> >> +return err;
> >> +
> >>err = set_mfns (console_pfn);
> >>if (err)
> >>  return err;
> >> @@ -587,6 +604,13 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>goto fail;
> >>  }
> >>
> >> +  if (xen_inf.unmapped_initrd)
> >> +{
> >> +  err = grub_xen_alloc_end ();
> >> +  if (err)
> >> +goto fail;
> >> +}
> >> +
> >>if (grub_initrd_init (argc, argv, &initrd_ctx))
> >>  goto fail;
> >>
> >> @@ -603,13 +627,22 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>goto fail;
> >>  }
> >>
> >> -  next_start.mod_start = max_addr + xen_inf.virt_base;
> >> -  next_start.mod_len = size;
> >> -
> >> -  max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE);
> >> +  if (xen_inf.unmapped_initrd)
> >> +{
> >> +  next_start.flags |= SIF_MOD_START_PFN;
> >> +  next_start.mod_start = max_addr >> PAGE_SHIFT;
> >> +  next_start.mod_len = size;
> >> +}
> >> +  else
> >> +{
> >> +  next_start.mod_start = max_addr + xen_inf.virt_base;
> >> +  next_start.mod_len = size;
> >> +}
> >>
> >>grub_dprintf ("xen", "Initrd, addr=0x%x, size=0x%x\n",
> >> -  (unsigned) next_start.mod_start, (unsigned) size);
> >> +  (unsigned) (max_addr + xen_inf.virt_base), (unsigned) size);
> >> +
> >> +  max_addr = ALIGN_UP (max_addr + size, PAGE_SIZE);
> >>
> >>  fail:
> >>grub_initrd_close (&initrd_ctx);
> >> @@ -660,6 +693,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>
> >>if (!xen_module_info_page)
> >>  {
> >> +  xen_inf.unmapped_initrd = 0;
> >>n_modules = 0;
> >>max_addr = ALIGN_UP (max_addr, PAGE_SIZE);
> >>modules_target_start = max_addr;
> >> diff --git a/grub-core/loader/i386/xen_fileXX.c 
> >> b/grub-core/loader/i386/xen_fileXX.c
> >> index 1ba5649..69fccd2 100644
> >> --- a/grub-core/loader/i386/xen_fileXX.c
> >> +++ b/grub-core/loader/i386/xen_fileXX.c
> >> @@ -253,6 +253,9 @@ parse_note (grub_elf_t elf, struct grub_xen_file_info 
> >> *xi,
> >>  descsz == 2 ? 2 : 3) == 0)
> >>xi->arch = GRUB_XEN_FILE_I386;
> >>  break;
> >> +  case 16:
> >
> > Could you define this a constant and use it

Re: [Xen-devel] [PATCH v3] travis: add initial Travis CI script to do builds

[Doug Goldstein]

On 2/9/16 6:33 PM, Dario Faggioli wrote:
> On Tue, 2016-02-09 at 17:09 +, Andrew Cooper wrote:
>> On 08/02/16 02:45, Doug Goldstein wrote:
>>> This is just suppose to do a simple compile test on Travis CI.
>>> Currently
>>> due to linux86 (bcc/bin86/dev86) not being whitelisted the tools
>>> cannot
>>> be built.
>>>
>>> Signed-off-by: Doug Goldstein 
>>
>> Reviewed-by: Andrew Cooper 
>>
>> This will be a massive help for contributes to sanity check their
>> patch
>> series.
>>
> BTW, when all the bits will be in place, a piece of doc (e.g., on the
> wiki) explaining how to take advantage of this "massive help" would be
> great... E.g., for those people that have managed to survive until now
> barely knowing what Travis even is! :-P
> 
> Regards,
> Dario
> 

Gladly. I just got write permissions to the wiki so I'll get started soon.

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 3/3] xen: credit1: avoid boosting vCPUs being "just" migrated

[Dario Faggioli]

On Thu, 2016-02-11 at 06:30 -0700, Jan Beulich wrote:
> > > > On 11.02.16 at 12:39,  wrote:
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -1022,11 +1022,14 @@ csched_vcpu_wake(const struct scheduler
> > *ops, struct vcpu *vc, unsigned wf)
> >   * more CPU resource intensive VCPUs without impacting
> > overall 
> >   * system fairness.
> >   *
> > - * The one exception is for VCPUs of capped domains unpausing
> > - * after earning credits they had overspent. We don't boost
> > - * those.
> > + * There are a couple of exceptions, when we don't want to
> > boost:
> > + *  - VCPUs that are waking up after a migration, rather than
> > + *after having block;
> > + *  - VCPUs of capped domains unpausing after earning credits
> > + *they had overspent.
> >   */
> > -if ( svc->pri == CSCHED_PRI_TS_UNDER &&
> > +if ( !(wf & WF_migrated) &&
> > + svc->pri == CSCHED_PRI_TS_UNDER &&
> >   !test_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) )
> >  {
> 
> Considering the other svc->flags check done here, wouldn't it be
> possible to achieve the same effect without patch 2, by having
> csched_cpu_pick() set a newly defined flag, and check for it here?
> 
It can indeed. I've coded it up, and I like the way it came out better.

I'm rerunning the benchmarks right now (just in case! :-)). I'll send
v2 out as soon as they finish.

I did like the idea of "wakeup flags", and I think they may actually
turn out useful, but they're not necessary for this specific use case,
as it appears. Well, next time. ;-)

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/3] xen: sched: add wakeup flags to the scheduler interface

[Dario Faggioli]

On Thu, 2016-02-11 at 06:24 -0700, Jan Beulich wrote:
> > > > On 11.02.16 at 12:38,  wrote:
> > --- a/xen/include/xen/sched-if.h
> > +++ b/xen/include/xen/sched-if.h
> > @@ -144,7 +144,8 @@ struct scheduler {
> >  void (*remove_vcpu)(const struct scheduler *,
> > struct vcpu *);
> >  
> >  void (*sleep)  (const struct scheduler *,
> > struct vcpu *);
> > -void (*wake)   (const struct scheduler *,
> > struct vcpu *);
> > +void (*wake)   (const struct scheduler *,
> > struct vcpu *,
> > +unsigned int);
> 
> Just one cosmetic comment: You properly use "unsigned int" here,
> but just "unsigned" everywhere else in this patch. May I ask that
> you use the canonical full form everywhere?
> 
Of course you can :-), and I was already down to change this... But, as
a matter of fact, this patch is not going to be present in v2.

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3] travis: add initial Travis CI script to do builds

[Doug Goldstein]

On 2/7/16 8:45 PM, Doug Goldstein wrote:
> This is just suppose to do a simple compile test on Travis CI. Currently
> due to linux86 (bcc/bin86/dev86) not being whitelisted the tools cannot
> be built.
> 
> Signed-off-by: Doug Goldstein 

ping?

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 5/6] xen: modify page table construction

[Daniel Kiper]

On Thu, Feb 11, 2016 at 03:35:45PM +0100, Juergen Gross wrote:
> On 11/02/16 13:47, Daniel Kiper wrote:
> > On Thu, Feb 11, 2016 at 08:53:25AM +0100, Juergen Gross wrote:
> >> Modify the page table construction to allow multiple virtual regions
> >> to be mapped. This is done as preparation for removing the p2m list
> >> from the initial kernel mapping in order to support huge pv domains.
> >>
> >> This allows a cleaner approach for mapping the relocator page by
> >> using this capability.
> >>
> >> The interface to the assembler level of the relocator has to be changed
> >> in order to be able to process multiple page table areas.
> >
> > I hope that older kernels could be loaded as is and everything works as 
> > expected.
>
> See Patch 0/6. I have tested old kernels, too.

That is great! I just wanted to be sure.

> >> Signed-off-by: Juergen Gross 
> >> ---
> >>  grub-core/lib/i386/xen/relocator.S   |  47 +++---
> >>  grub-core/lib/x86_64/xen/relocator.S |  44 +++--
> >>  grub-core/lib/xen/relocator.c|  22 ++-
> >>  grub-core/loader/i386/xen.c  | 313 
> >> +++
> >>  include/grub/xen/relocator.h |   6 +-
> >>  5 files changed, 277 insertions(+), 155 deletions(-)
> >>
> >> diff --git a/grub-core/lib/i386/xen/relocator.S 
> >> b/grub-core/lib/i386/xen/relocator.S
> >> index 694a54c..c23b405 100644
> >> --- a/grub-core/lib/i386/xen/relocator.S
> >> +++ b/grub-core/lib/i386/xen/relocator.S
> >> @@ -50,41 +50,45 @@ VARIABLE(grub_relocator_xen_remapper_map_high)
> >>jmp *%ebx
> >>
> >>  LOCAL(cont):
> >> -  xorl%eax, %eax
> >> -  movl%eax, %ebp
> >> +  /* mov imm32, %eax */
> >> +  .byte   0xb8
> >> +VARIABLE(grub_relocator_xen_paging_areas_addr)
> >> +  .long   0
> >> +  movl%eax, %ebx
> >>  1:
> >> -
> >> +  movl0(%ebx), %ebp
> >> +  movl4(%ebx), %ecx
> >
> > Oh... No, please use constants not plain numbers and
> > explain in comments what is going on. Otherwise it
> > is unreadable.
>
> Hmm, yes, some comments wouldn't hurt. :-)
> Regarding constants: do you really think I should introduce their
> first usage in this file with my patch?

I would not afraid that. At least in code you change/touch. If you
are not sure please ask maintainer about that.

> >> +  testl   %ecx, %ecx
> >> +  jz  3f
> >> +  addl$8, %ebx
> >> +  movl%ebx, %esp
> >> +
> >> +2:
> >> +  movl%ecx, %edi
> >>/* mov imm32, %eax */
> >>.byte   0xb8
> >>  VARIABLE(grub_relocator_xen_mfn_list)
> >>.long   0
> >> -  movl%eax, %edi
> >> -  movl%ebp, %eax
> >> -  movl0(%edi, %eax, 4), %ecx
> >> -
> >> -  /* mov imm32, %ebx */
> >> -  .byte   0xbb
> >> -VARIABLE(grub_relocator_xen_paging_start)
> >> -  .long   0
> >> -  shll$12, %eax
> >> -  addl%eax, %ebx
> >> +  movl0(%eax, %ebp, 4), %ecx
> >> +  movl%ebp, %ebx
> >> +  shll$12, %ebx
> >
> > Ditto.
>
> Look at the removed line above: I just switched register usage.

It does not hurt to change number to constant here too.

> >>movl%ecx, %edx
> >>shll$12,  %ecx
> >>shrl$20,  %edx
> >>orl $5, %ecx
> >>movl$2, %esi
> >>movl$__HYPERVISOR_update_va_mapping, %eax
> >> -  int $0x82
> >> +  int $0x82   /* parameters: eax, ebx, ecx, edx, esi */
> >
> > Please use more verbose comments.
>
> :-)
>
> >
> >>
> >>incl%ebp
> >> -  /* mov imm32, %ecx */
> >> -  .byte   0xb9
> >> -VARIABLE(grub_relocator_xen_paging_size)
> >> -  .long   0
> >> -  cmpl%ebp, %ecx
> >> +  movl%edi, %ecx
> >> +
> >> +  loop2b
> >>
> >> -  ja  1b
> >> +  mov %esp, %ebx
> >> +  jmp 1b
> >>
> >> +3:
> >>/* mov imm32, %ebx */
> >>.byte   0xbb
> >>  VARIABLE(grub_relocator_xen_mmu_op_addr)
> >> @@ -102,6 +106,9 @@ VARIABLE(grub_relocator_xen_remap_continue)
> >>
> >>jmp *%eax
> >>
> >> +VARIABLE(grub_relocator_xen_paging_areas)
> >> +  .long   0, 0, 0, 0, 0, 0, 0, 0
> >> +
> >>  VARIABLE(grub_relocator_xen_mmu_op)
> >>.space 256
> >>
> >> diff --git a/grub-core/lib/x86_64/xen/relocator.S 
> >> b/grub-core/lib/x86_64/xen/relocator.S
> >> index 92e9e72..dbb90c7 100644
> >> --- a/grub-core/lib/x86_64/xen/relocator.S
> >> +++ b/grub-core/lib/x86_64/xen/relocator.S
> >
> > Is to possible to split this patch to i386 and x86-64 stuff patches?
>
> I don't think so. The C-part is common and assembler sources must both
> match.

Yep, I am aware of that. However, I was able to that in my patch series.
So, maybe it is possible here. Could you try it?

> >> @@ -50,31 +50,24 @@ VARIABLE(grub_relocator_xen_remapper_map)
> >>
> >>  LOCAL(cont):
> >>
> >> -  /* mov imm64, %rcx */
> >> -  .byte   0x48
> >> -  .byte   0xb9
> >> -VARIABLE(grub_relocator_xen_paging_size)
> >> -  .quad   0
> >> -
> >> -  /* mov imm64, %rax */
> >> -  .byte   0x48
> >> -  .byte   0xb8
> >> -VARIABLE(grub_relocator_xen_paging_start)
> >> -  .quad   0
> >> -
> >> -  movq%rax, %r12
> >> -
> >>/* mov imm64, %rax */
> >>.byte   0x4

[Xen-devel] [xen-unstable-smoke test] 81996: regressions - trouble: blocked/broken/pass

[osstest service owner]

flight 81996 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/81996/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf   4 host-build-prep   fail REGR. vs. 81826

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  2052a51f9749b79973cba54bd4306cad7726e763
baseline version:
 xen  c26b88bf644011396b4e4f6f15901a66b87d8c19

Last test of basis81826  2016-02-10 17:24:44 Z1 days
Testing same since81996  2016-02-11 16:03:06 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Doug Goldstein 
  George Dunlap 
  Harmandeep Kaur 
  Ian Campbell 
  Ian Jackson 
  Jan Beulich 
  Kevin Tian 
  Paul Durrant 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  broken  
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  blocked 
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Not pushing.

(No revision log; it would be 326 lines long.)

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