[Xen-devel] [PATCH v11 0/2] libxl: add support for pvscsi, iteration 11

[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:
 - document pvscsi in xen wiki

Changes between v10 and v11:
 - rebase to 'staging' (d887c19..a35dc6c)
 - rebase to 'staging' (829e03c..d887c19)
 - Remove evtch and rref from vscsiinfo
 - Remove COMPARE_VSCSI, use COMPARE_DEVID
 - Remove vscsi_wwn_valid helpers, use sscanf instead

Changes between v9 and v10:
 - rebase to 'staging' (3b971de..829e03c)
http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg02773.html

Changes between v8 and v9:
 - Mention libxl_ctrl_index in vscsiif.h
 - Rename idx to libxl_ctrl_index
 - Fix device assignment in vscsidev_remove
 - Add vscsictrls to multidev callchain
 - Reorder some functions
 - Move and rename vscsidev_rm
 - Remove libxl prefix from static functions
 - Reorder functions in libxl_vscsi.c
 - Add comment to libxl_vscsidev_aodev_cb
 - Move libxl_device_vscsictrl_remove into libxl_vscsi.c
 - Enclose libxl__device instead of aodev in libxl__vscsidev_rm
 - Update typedef of libxl__vscsidev_rm
 - Assign devid earlier in libxl__device_vscsictrl_add
 - Rework xlu_vscsi_get_ctrl and main_vscsiattach
 - Add config idx also to libxl_vscsiinfo
 - Make libxl__vscsi_collect_ctrls static
 - Implement libxl_device_vscsidev_add
 - Move vscsictrl add from libxl.c to libxl_vscsi.c
 - Export libxl__device_nextid for internal use
 - Remove some checks for empty controllers
 - Introduce an idx file to present the config index in empty vscsictrls
 - Use libxl__ev_devstate_wait in libxl__device_vscsictrl_reconfigure_add
 - Rework error handling in libxl__vscsi_fill_ctrl
 - Remove state checking from libxl__vscsi_fill_dev
 - Wrap long lines in libxl__device_vscsictrl_new_backend
 - Use LIBXL_DESTROY_TIMEOUT in libxl__device_vscsidev_rm
 - Remove tab from libxl__vscsi_collect_ctrls
 - Goto out if libxl__device_vscsidev_backend_set_add fails
 - Add fallthrough to libxl__device_vscsidev_backend_set_add
 - Fix typo in xl.cfg.pod: naa.
 - Wrap long line in xl.cfg.pod.5
 - Use memmove in libxl_device_vscsictrl_remove_vscsidev
 - Adjust code in main_vscsilist to fit in 80 chars
 - Align switch and case to gain 4 columns
 - Wrap long line in libxl__device_vscsi_reconfigure_rm
 - Wrap long line in libxl__device_vscsidev_rm_be
 - Adjust flexarray size in libxl__device_vscsi_reconfigure_rm
 - Rework libxl_device_vscsidev_remove
 - Introduce libxl_device_vscsictrl_remove_vscsidev
 - Use stack variables in xlu_vscsi_get_ctrl
 - Rename function names to refer to vscsictrl
 - Rename local variables to refer to vscsictrl
 - Use generic vscsictrl->devid
http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg03196.html

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
http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg01662.html

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

[Xen-devel] [PATCH v11 2/2] 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 v11 1/2] 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|   56 ++
 docs/man/xl.pod.1|   18 +
 tools/libxl/Makefile |2 +
 tools/libxl/libxl.c  |9 +
 tools/libxl/libxl.h  |   42 ++
 tools/libxl/libxl_create.c   |   41 +-
 tools/libxl/libxl_device.c   |2 +
 tools/libxl/libxl_internal.h |8 +
 tools/libxl/libxl_types.idl  |   53 ++
 tools/libxl/libxl_types_internal.idl |1 +
 tools/libxl/libxl_vscsi.c| 1160 ++
 tools/libxl/libxlu_vscsi.c   |  664 +++
 tools/libxl/libxlutil.h  |   18 +
 tools/libxl/xl.h |3 +
 tools/libxl/xl_cmdimpl.c |  225 ++-
 tools/libxl/xl_cmdtable.c|   15 +
 16 files changed, 2313 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index a4cc1b3..be4546b 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -517,6 +517,62 @@ 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 "naa.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 e2bd32d..d3a4a5f 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1416,6 +1416,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 4fc264d..660e6f5 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -108,6 +108,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 
\
@@ -151,6 +152,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 8719f54..60e3803 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4315,6 +4315,7 @@ DEFINE_DEVICE_REMOVE_CUSTOM(usbctrl, destroy, 1)
 /* The following functions are defined:
  * libxl_device_disk_add
  * libxl_device_nic_add
+ * libxl_device_vscsictrl_add
  * libxl_device_vtpm_add
  * libxl_device_usbctrl_add
  * libxl_device_usbdev_add
@@ -4346,6 +4347,9 @@ DE

[Xen-devel] [PATCH] x86/monitor: Disallow setting mem_access_emulate_each_rep when vm_event is NULL

[Razvan Cojocaru]

It is meaningless (and potentially dangerous - see hvmemul_virtual_to_linear())
to set mem_access_emulate_each_rep before xc_monitor_enable() (which allocates
vcpu->arch.vm_event) has been called, so return an error from the
XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case.

Signed-off-by: Razvan Cojocaru 
---
 xen/include/asm-x86/monitor.h | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 0954b59..0544836 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -35,11 +35,22 @@ int arch_monitor_domctl_op(struct domain *d, struct 
xen_domctl_monitor_op *mop)
 switch ( mop->op )
 {
 case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
+{
+bool_t value = !!mop->event;
 domain_pause(d);
-d->arch.mem_access_emulate_each_rep = !!mop->event;
+/*
+ * Enabling emulate_each_rep without a vm_event subscriber
+ * is meaningless.
+ */
+if ( !d->vcpu || !d->vcpu[0]->arch.vm_event )
+{
+domain_unpause(d);
+return -EINVAL;
+}
+d->arch.mem_access_emulate_each_rep = value;
 domain_unpause(d);
 break;
-
+}
 default:
 return -EOPNOTSUPP;
 }
-- 
1.9.1


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

Re: [Xen-devel] [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Juergen Gross]

On 08/04/16 08:56, Luis R. Rodriguez wrote:
> On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross  wrote:
>> On 08/04/16 08:29, Luis R. Rodriguez wrote:
>>> On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross  wrote:
 On 08/04/16 02:32, Luis R. Rodriguez wrote:
> This highlights a semantic gap issue. From a quick cursory review, I think
> we can address this temporarily by just using a check:
>
> void __init x86_early_init_platform_quirks(void)
> {
>   x86_platform.legacy.rtc = 1;
>
>   switch (boot_params.hdr.hardware_subarch) {
>   case X86_SUBARCH_XEN:
>   case X86_SUBARCH_LGUEST:
>   case X86_SUBARCH_INTEL_MID:
> - x86_platform.legacy.rtc = 0;
> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop)
> + x86_platform.legacy.rtc = 0;

 No! Why don't you just use the explicit test xen_initial_domain() ?
>>>
>>> Because we don't want to sprinkle Xen specific code outside of Xen
>>> code. What do you think about the second possibility I listed?
>>> Otherwise, any other ideas?
>>
>> Don't try to guess.
> 
> I can only do that given there is nothing at all to tell me what to
> expect here with regards to RTC on Xen guest, if there is some
> documentation that could help with that please let me know.

Only Xen inernals. :-)

> 
>> In case you don't want to inject Xen internals here, just call a Xen
>> function to either return the correct value, or to set all structure
>> elements correctly.
> 
> I like the later as an option, in case there are further hardware
> subarch specific quirks which require internal logistics. What do
> others think?
> 
>> Thinking more about it: why not do that for all the subarchs?
> 
> I originally had went with that approach, but Ingo made the point that
> it would be best to instead move all quirk settings into one place.
> That lets a reader easily tell what is going on in one place, it also
> compartmentalizes the hardware subarch uses.

Okay. Another idea (not sure whether this is really a good one):

Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't
think the number of subarchs is a scarce resource. :-)

I'd expect other quirks in future might have different settings for
domU and dom0, too.

>> You'd
>> have the specific settings where they belong: in a subarch specific
>> source. Just do the default settings in x86_early_init_platform_quirks()
>> and let the subarch functions set the non-default values.
> 
> This is a rather different approach than what I had originally tried.
> Bike shed thing -- someone just has to decide.
> 
> Left up to me, I kind of really like centralizing the quirk settings
> in one place approach as it means a reader can easily tell what's
> going on regardless of platform in one place for odd settings. I
> prefer this given that we *already* have the semantics over hardware
> subarch in a generalized fashion. We *do not* have semantics for dom0
> Vs domU -- if such a notion is generic to other virtualization

That's not carved in stone - see above. :-)

> environments it deserves consideration to new semantics to deal with
> that, otherwise the callback for handling further quirks is best, but
> I'd also highly discourage such callback to be used.


Juergen


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

Re: [Xen-devel] [PATCH v10 4/5] libxl: add support for vscsi

[Olaf Hering]

On Fri, Apr 01, Ian Jackson wrote:

> > +static int xlu__vscsi_parse_dev(XLU_Config *cfg, char *pdev, 
> > libxl_vscsi_hctl *hctl)
> > +{
> > +struct stat dentry;
> > +char *sysfs = NULL;
> > +const char *type;
> > +int rc, found = 0;
> > +DIR *dirp;
> > +struct dirent *de;
> ...
> > +/* /sys/dev/type/major:minor symlink added in 2.6.27 */
> > +if (asprintf(&sysfs, "/sys/dev/%s/%u:%u/device/scsi_device", type,
> > + major(dentry.st_rdev), minor(dentry.st_rdev)) < 0) {
> > +sysfs = NULL;
> > +rc = ERROR_NOMEM;
> > +goto out;
> > +}
> 
> I'm not sure that this sysfs parsing ought to be in xl rather than
> libxl.  Also, this is Linux-specific code.  So it needs to be made
> conditional somehow.

I think this depends on how libvirt is supposed to interact with libxl
here. Right now libvirt is rather dumb, it supports just the
host:channel:target:lun notation for a backend. My patch for it (which I
have to rebase now) creates a string "pdev,vdev" and calls
xlu_vscsi_get_host.  I will rebase the libvirt patch and resend it as
well.

> It seems to me that the contents of xlu__vscsi_target should be much
> closer to vscsi_pdev_type (unless I have misunderstood).

Well, if its important where that struct is supposed to be in the file,
I can move it down.

> Perhaps the libxl_types.idl API needs to change.  In general, the
> libxl API ought to be close enough in semantics to the xl config API
> that the correspondence is obvious.  I don't think that's the case
> here.

I'm not sure what this paragraph means.


Olaf

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

Re: [Xen-devel] [PATCH] x86/monitor: Disallow setting mem_access_emulate_each_rep when vm_event is NULL

[Razvan Cojocaru]

On 04/08/16 10:10, Razvan Cojocaru wrote:
> It is meaningless (and potentially dangerous - see 
> hvmemul_virtual_to_linear())
> to set mem_access_emulate_each_rep before xc_monitor_enable() (which allocates
> vcpu->arch.vm_event) has been called, so return an error from the
> XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case.
> 
> Signed-off-by: Razvan Cojocaru 
> ---
>  xen/include/asm-x86/monitor.h | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index 0954b59..0544836 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -35,11 +35,22 @@ int arch_monitor_domctl_op(struct domain *d, struct 
> xen_domctl_monitor_op *mop)
>  switch ( mop->op )
>  {
>  case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
> +{
> +bool_t value = !!mop->event;
>  domain_pause(d);
> -d->arch.mem_access_emulate_each_rep = !!mop->event;
> +/*
> + * Enabling emulate_each_rep without a vm_event subscriber
> + * is meaningless.
> + */
> +if ( !d->vcpu || !d->vcpu[0]->arch.vm_event )

Sorry, this is a bit convoluted, and wrong: if d->vcpu != NULL it will
return -EINVAL even if d->vcpu[0]->arch.vm_event is NULL. I'll rework
it. Apologies for the noise.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH v3 08/11] xen: sched: allow for choosing credit2 runqueues configuration at boot

[Dario Faggioli]

On Fri, 2016-04-08 at 06:18 +0200, Juergen Gross wrote:
> On 08/04/16 03:24, Dario Faggioli wrote:
> > 
> > In fact, credit2 uses CPU topology to decide how to arrange
> > its internal runqueues. Before this change, only 'one runqueue
> > per socket' was allowed. However, experiments have shown that,
> > for instance, having one runqueue per physical core improves
> > performance, especially in case hyperthreading is available.
> > 
> > In general, it makes sense to allow users to pick one runqueue
> > arrangement at boot time, so that:
> >  - more experiments can be easily performed to even better
> >    assess and improve performance;
> >  - one can select the best configuration for his specific
> >    use case and/or hardware.
> > 
> > This patch enables the above.
> > 
> > Note that, for correctly arranging runqueues to be per-core,
> > just checking cpu_to_core() on the host CPUs is not enough.
> > In fact, cores (and hyperthreads) on different sockets, can
> > have the same core (and thread) IDs! We, therefore, need to
> > check whether the full topology of two CPUs matches, for
> > them to be put in the same runqueue.
> > 
> > Note also that the default (although not functional) for
> > credit2, since now, has been per-socket runqueue. This patch
> > leaves things that way, to avoid mixing policy and technical
> > changes.
> > 
> > Finally, it would be a nice feature to be able to select
> > a particular runqueue arrangement, even when creating a
> > Credit2 cpupool. This is left as future work.
> > 
> > Signed-off-by: Dario Faggioli 
> > Signed-off-by: Uma Sharma 
>
> Some nits below.
> 
Thanks for the quick review!

A revised version of this patch is provided here (both inlined and
attached), and a branch with the remaining to be committed patches of
this series, and with this patch changed as you suggest, is available
at:

 git://xenbits.xen.org/people/dariof/xen.git 
rel/sched/credit2/fix-runq-and-haff-v4
 
http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/sched/credit2/fix-runq-and-haff-v4

Regards,
Dario
---
commit 7f491488bbff1cc3af021cd29fca7e0fba321e02
Author: Dario Faggioli 
Date:   Tue Sep 29 14:05:09 2015 +0200

xen: sched: allow for choosing credit2 runqueues configuration at boot

In fact, credit2 uses CPU topology to decide how to arrange
its internal runqueues. Before this change, only 'one runqueue
per socket' was allowed. However, experiments have shown that,
for instance, having one runqueue per physical core improves
performance, especially in case hyperthreading is available.

In general, it makes sense to allow users to pick one runqueue
arrangement at boot time, so that:
 - more experiments can be easily performed to even better
   assess and improve performance;
 - one can select the best configuration for his specific
   use case and/or hardware.

This patch enables the above.

Note that, for correctly arranging runqueues to be per-core,
just checking cpu_to_core() on the host CPUs is not enough.
In fact, cores (and hyperthreads) on different sockets, can
have the same core (and thread) IDs! We, therefore, need to
check whether the full topology of two CPUs matches, for
them to be put in the same runqueue.

Note also that the default (although not functional) for
credit2, since now, has been per-socket runqueue. This patch
leaves things that way, to avoid mixing policy and technical
changes.

Finally, it would be a nice feature to be able to select
a particular runqueue arrangement, even when creating a
Credit2 cpupool. This is left as future work.

Signed-off-by: Dario Faggioli 
Signed-off-by: Uma Sharma 
---
Cc: George Dunlap 
Cc: Uma Sharma 
Cc: Juergen Gross 
---
Changes from v3:
 * fix type and other issue in comments;
   use ARRAY_SIZE when iterating the parameter string array.

Changes from v2:
 * valid strings  are now in an array, that we scan during
   parameter parsing, as suggested during review.

Cahnges from v1:
 * fix bug in parameter parsing, and start using strcmp()
   for that, as requested during review.

diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index ca77e3b..0047f94 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -469,6 +469,25 @@ combination with the `low_crashinfo` command line option.
 ### credit2\_load\_window\_shift
 > `= `
 
+### credit2\_runqueue
+> `= core | socket | node | all`
+
+> Default: `socket`
+
+Specify how host CPUs are arranged in runqueues. Runqueues are kept
+balanced with respect to the load generated by the vCPUs running on
+them. Smaller runqueues (as in with `core`) means more accurate load
+balancing (for instance, it will deal better with hyperthreading),
+but also more overhead.
+
+Available alternative

Re: [Xen-devel] [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Luis R. Rodriguez]

On Fri, Apr 8, 2016 at 12:13 AM, Juergen Gross  wrote:
> On 08/04/16 08:56, Luis R. Rodriguez wrote:
>> On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross  wrote:
>>> On 08/04/16 08:29, Luis R. Rodriguez wrote:
 On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross  wrote:
> On 08/04/16 02:32, Luis R. Rodriguez wrote:
>> This highlights a semantic gap issue. From a quick cursory review, I 
>> think
>> we can address this temporarily by just using a check:
>>
>> void __init x86_early_init_platform_quirks(void)
>> {
>>   x86_platform.legacy.rtc = 1;
>>
>>   switch (boot_params.hdr.hardware_subarch) {
>>   case X86_SUBARCH_XEN:
>>   case X86_SUBARCH_LGUEST:
>>   case X86_SUBARCH_INTEL_MID:
>> - x86_platform.legacy.rtc = 0;
>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop)
>> + x86_platform.legacy.rtc = 0;
>
> No! Why don't you just use the explicit test xen_initial_domain() ?

 Because we don't want to sprinkle Xen specific code outside of Xen
 code. What do you think about the second possibility I listed?
 Otherwise, any other ideas?
>>>
>>> Don't try to guess.
>>
>> I can only do that given there is nothing at all to tell me what to
>> expect here with regards to RTC on Xen guest, if there is some
>> documentation that could help with that please let me know.
>
> Only Xen inernals. :-)

Where can I look at this specifically on the Xen sources? No worries
if you don't care -- as I don't really either.

>>> In case you don't want to inject Xen internals here, just call a Xen
>>> function to either return the correct value, or to set all structure
>>> elements correctly.
>>
>> I like the later as an option, in case there are further hardware
>> subarch specific quirks which require internal logistics. What do
>> others think?
>>
>>> Thinking more about it: why not do that for all the subarchs?
>>
>> I originally had went with that approach, but Ingo made the point that
>> it would be best to instead move all quirk settings into one place.
>> That lets a reader easily tell what is going on in one place, it also
>> compartmentalizes the hardware subarch uses.
>
> Okay. Another idea (not sure whether this is really a good one):
>
> Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't
> think the number of subarchs is a scarce resource. :-)

This would mean bumping the x86 boot protocol, we shouldn't take that
lightly, but given that in this case the new subarch would really only
be set by the kernel (or future loaders for perhaps HVMLite) I'd think
this is not such an intrusive alternative.

> I'd expect other quirks in future might have different settings for
> domU and dom0, too.

Can you elaborate a bit more on this. I realize we expect domU and
dom0 on HVMLite in the future, would HVMLite use the subarch ? From
the last discussions on the HVMLite thread Boris noted HVMLite would
use the PC subarch -- how would we do dom0 Vs domU quirk management?

If it goes the EFI route, I gather Xen instead can use the EFI
configuration tables for Xen specific tunings, however we may also
need a generic x86 configuration table for generic quirks I think. We
may need to provide a 1-1 mapping of these quirks there, if the
subarch is not used.

>>> You'd
>>> have the specific settings where they belong: in a subarch specific
>>> source. Just do the default settings in x86_early_init_platform_quirks()
>>> and let the subarch functions set the non-default values.
>>
>> This is a rather different approach than what I had originally tried.
>> Bike shed thing -- someone just has to decide.
>>
>> Left up to me, I kind of really like centralizing the quirk settings
>> in one place approach as it means a reader can easily tell what's
>> going on regardless of platform in one place for odd settings. I
>> prefer this given that we *already* have the semantics over hardware
>> subarch in a generalized fashion. We *do not* have semantics for dom0
>> Vs domU -- if such a notion is generic to other virtualization
>
> That's not carved in stone - see above. :-)

Another subarch for Xen dom0 works well for me as well given the new
subarch would just all set in the kernel. It does mean extending the
x86 boot protocol though, and so for that I yield to hpa.

 Luis

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

Re: [Xen-devel] [PATCH v3 08/11] xen: sched: allow for choosing credit2 runqueues configuration at boot

[Juergen Gross]

On 08/04/16 09:35, Dario Faggioli wrote:
> On Fri, 2016-04-08 at 06:18 +0200, Juergen Gross wrote:
>> On 08/04/16 03:24, Dario Faggioli wrote:
>>>
>>> In fact, credit2 uses CPU topology to decide how to arrange
>>> its internal runqueues. Before this change, only 'one runqueue
>>> per socket' was allowed. However, experiments have shown that,
>>> for instance, having one runqueue per physical core improves
>>> performance, especially in case hyperthreading is available.
>>>
>>> In general, it makes sense to allow users to pick one runqueue
>>> arrangement at boot time, so that:
>>>  - more experiments can be easily performed to even better
>>>assess and improve performance;
>>>  - one can select the best configuration for his specific
>>>use case and/or hardware.
>>>
>>> This patch enables the above.
>>>
>>> Note that, for correctly arranging runqueues to be per-core,
>>> just checking cpu_to_core() on the host CPUs is not enough.
>>> In fact, cores (and hyperthreads) on different sockets, can
>>> have the same core (and thread) IDs! We, therefore, need to
>>> check whether the full topology of two CPUs matches, for
>>> them to be put in the same runqueue.
>>>
>>> Note also that the default (although not functional) for
>>> credit2, since now, has been per-socket runqueue. This patch
>>> leaves things that way, to avoid mixing policy and technical
>>> changes.
>>>
>>> Finally, it would be a nice feature to be able to select
>>> a particular runqueue arrangement, even when creating a
>>> Credit2 cpupool. This is left as future work.
>>>
>>> Signed-off-by: Dario Faggioli 
>>> Signed-off-by: Uma Sharma 
>>
>> Some nits below.
>>
> Thanks for the quick review!
> 
> A revised version of this patch is provided here (both inlined and
> attached), and a branch with the remaining to be committed patches of
> this series, and with this patch changed as you suggest, is available
> at:
> 
>  git://xenbits.xen.org/people/dariof/xen.git 
> rel/sched/credit2/fix-runq-and-haff-v4
>  
> http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/sched/credit2/fix-runq-and-haff-v4

Thanks.

Reviewed-by: Juergen Gross 

> 
> Regards,
> Dario
> ---
> commit 7f491488bbff1cc3af021cd29fca7e0fba321e02
> Author: Dario Faggioli 
> Date:   Tue Sep 29 14:05:09 2015 +0200
> 
> xen: sched: allow for choosing credit2 runqueues configuration at boot
> 
> In fact, credit2 uses CPU topology to decide how to arrange
> its internal runqueues. Before this change, only 'one runqueue
> per socket' was allowed. However, experiments have shown that,
> for instance, having one runqueue per physical core improves
> performance, especially in case hyperthreading is available.
> 
> In general, it makes sense to allow users to pick one runqueue
> arrangement at boot time, so that:
>  - more experiments can be easily performed to even better
>assess and improve performance;
>  - one can select the best configuration for his specific
>use case and/or hardware.
> 
> This patch enables the above.
> 
> Note that, for correctly arranging runqueues to be per-core,
> just checking cpu_to_core() on the host CPUs is not enough.
> In fact, cores (and hyperthreads) on different sockets, can
> have the same core (and thread) IDs! We, therefore, need to
> check whether the full topology of two CPUs matches, for
> them to be put in the same runqueue.
> 
> Note also that the default (although not functional) for
> credit2, since now, has been per-socket runqueue. This patch
> leaves things that way, to avoid mixing policy and technical
> changes.
> 
> Finally, it would be a nice feature to be able to select
> a particular runqueue arrangement, even when creating a
> Credit2 cpupool. This is left as future work.
> 
> Signed-off-by: Dario Faggioli 
> Signed-off-by: Uma Sharma 
> ---
> Cc: George Dunlap 
> Cc: Uma Sharma 
> Cc: Juergen Gross 
> ---
> Changes from v3:
>  * fix type and other issue in comments;
>use ARRAY_SIZE when iterating the parameter string array.
> 
> Changes from v2:
>  * valid strings  are now in an array, that we scan during
>parameter parsing, as suggested during review.
> 
> Cahnges from v1:
>  * fix bug in parameter parsing, and start using strcmp()
>for that, as requested during review.
> 
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index ca77e3b..0047f94 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -469,6 +469,25 @@ combination with the `low_crashinfo` command line option.
>  ### credit2\_load\_window\_shift
>  > `= `
>  
> +### credit2\_runqueue
> +> `= core | socket | node | all`
> +
> +> Default: `socket`
> +
> +Specify how host CPUs are arranged in runqueues. Runqueues are kept
> +bal

Re: [Xen-devel] [PATCH v2 01/10] libxl_pci: improve return codes for more xl commands

[Dario Faggioli]

On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote:
> Return error code instead of always 0. Remove assigned-only ret
> variable.
> 
> Signed-off-by: Paulina Szubarczyk 
> 
> ---
>
I think a better subject would be:

 "libxl: improve return codes for some pci related functions"

or something like that.

And in the changelog, I'd say something like:

"libxl__device_from_pcidev() can be void, 
 while libxl__create_pci_backend() should propagate the success/error, 
 rather than always returning 0."

or something like that. :-)

> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index dc10cb7..9c9cd04 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
>
The code looks ok to me.

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 v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Juergen Gross]

On 08/04/16 09:36, Luis R. Rodriguez wrote:
> On Fri, Apr 8, 2016 at 12:13 AM, Juergen Gross  wrote:
>> On 08/04/16 08:56, Luis R. Rodriguez wrote:
>>> On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross  wrote:
 On 08/04/16 08:29, Luis R. Rodriguez wrote:
> On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross  wrote:
>> On 08/04/16 02:32, Luis R. Rodriguez wrote:
>>> This highlights a semantic gap issue. From a quick cursory review, I 
>>> think
>>> we can address this temporarily by just using a check:
>>>
>>> void __init x86_early_init_platform_quirks(void)
>>> {
>>>   x86_platform.legacy.rtc = 1;
>>>
>>>   switch (boot_params.hdr.hardware_subarch) {
>>>   case X86_SUBARCH_XEN:
>>>   case X86_SUBARCH_LGUEST:
>>>   case X86_SUBARCH_INTEL_MID:
>>> - x86_platform.legacy.rtc = 0;
>>> + if (x86_init.mpparse.get_smp_config != x86_init_uint_noop)
>>> + x86_platform.legacy.rtc = 0;
>>
>> No! Why don't you just use the explicit test xen_initial_domain() ?
>
> Because we don't want to sprinkle Xen specific code outside of Xen
> code. What do you think about the second possibility I listed?
> Otherwise, any other ideas?

 Don't try to guess.
>>>
>>> I can only do that given there is nothing at all to tell me what to
>>> expect here with regards to RTC on Xen guest, if there is some
>>> documentation that could help with that please let me know.
>>
>> Only Xen inernals. :-)
> 
> Where can I look at this specifically on the Xen sources? No worries
> if you don't care -- as I don't really either.

Just look how xen_initial_domain() is defined. :-)

 In case you don't want to inject Xen internals here, just call a Xen
 function to either return the correct value, or to set all structure
 elements correctly.
>>>
>>> I like the later as an option, in case there are further hardware
>>> subarch specific quirks which require internal logistics. What do
>>> others think?
>>>
 Thinking more about it: why not do that for all the subarchs?
>>>
>>> I originally had went with that approach, but Ingo made the point that
>>> it would be best to instead move all quirk settings into one place.
>>> That lets a reader easily tell what is going on in one place, it also
>>> compartmentalizes the hardware subarch uses.
>>
>> Okay. Another idea (not sure whether this is really a good one):
>>
>> Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't
>> think the number of subarchs is a scarce resource. :-)
> 
> This would mean bumping the x86 boot protocol, we shouldn't take that
> lightly, but given that in this case the new subarch would really only
> be set by the kernel (or future loaders for perhaps HVMLite) I'd think
> this is not such an intrusive alternative.

I think adding an own subarch for dom0 isn't that bad. It really is
different from domU as dom0 has per default access to the real hardware
(or at least to most of it).

>> I'd expect other quirks in future might have different settings for
>> domU and dom0, too.
> 
> Can you elaborate a bit more on this.

I guess we might want to add other quirks in order to switch on/off
more features instead of doing this based on #ifdef or environment
tests. I'm thinking of current use, not of HVMlite specific stuff.

> I realize we expect domU and
> dom0 on HVMLite in the future, would HVMLite use the subarch ? From
> the last discussions on the HVMLite thread Boris noted HVMLite would
> use the PC subarch -- how would we do dom0 Vs domU quirk management?

This would have to be settled. I think it might be a good idea to
initialize the quirks to their defaults statically in x86_init.c
and just modify some as needed for HVMlite on early boot (e.g. in
the HVMlite or EFI stub). This will enable us to either make use of
subarch or not for HVMlite, just what fits best.

> If it goes the EFI route, I gather Xen instead can use the EFI
> configuration tables for Xen specific tunings, however we may also
> need a generic x86 configuration table for generic quirks I think. We
> may need to provide a 1-1 mapping of these quirks there, if the
> subarch is not used.

The EFI stub can set the quirks just according to it's needs. Going
this route would _require_ HVMlite is using the PC subarch, though,
in order to avoid overwriting the quirks in
x86_early_init_platform_quirks() later.

> 
 You'd
 have the specific settings where they belong: in a subarch specific
 source. Just do the default settings in x86_early_init_platform_quirks()
 and let the subarch functions set the non-default values.
>>>
>>> This is a rather different approach than what I had originally tried.
>>> Bike shed thing -- someone just has to decide.
>>>
>>> Left up to me, I kind of really like centralizing the quirk settings
>>> in one place approach as it means a reader can easily tell what's
>>> going on regardless of platform

Re: [Xen-devel] Does __KERNEL_DS serve a purpose?

[Andrew Cooper]

On 08/04/2016 01:24, Andy Lutomirski wrote:
> I can't see any reason that we need the __KERNEL_DS segment at all --
> I think that everything that uses __KERNEL_DS could use __USER_DS
> instead.  Am I missing anything?  This has been bugging me for a
> while.
>
> I mulled over this a bit when trying to understand the sysret_ss_attrs
> bug and then forgot about it.

Linux doesn't have a separate __KERNEL_SS.  For the plain data segments,
the dpl is not interesting.

However, %ss is also loaded with __KERNEL_DS, and %ss.dpl is somewhat
important.

~Andrew

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

Re: [Xen-devel] [PATCH v2 02/10] libxl-pci: removing the extra 'out_closedir' cleaning path

[Dario Faggioli]

On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote:
> Signed-off-by: Paulina Szubarczyk 
> 
> ---
> Changed since v1:
>  * Modify the libxl_device_pci_assignable_list() function to use
>    only one 'out' cleaning path.
>
So, the patch does quite a few style cleanups. This is indeed a good
thing, but both subject and changelog need to better reflect it.

I'd go for the following.

Subject: "libxl: style cleanups in libxl_device_pci_assignable_list()

Changelog:
"various coding style compliance cleanups, such as, arranging for 
 using only one path out of the function, whitespaces in loops ad if-s 
 and r instead of rc for storing non-libxl error codes."

Again, the code looks ok to me. Just one suggestion:

> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -395,34 +395,35 @@ libxl_device_pci
> *libxl_device_pci_assignable_list(libxl_ctx *ctx, int *num)
>  libxl_device_pci *pcidevs = NULL, *new, *assigned;
>  struct dirent *de;
>  DIR *dir;
> -int rc, num_assigned;
> +int r, num_assigned;
>  
>  *num = 0;
>  
> -rc = get_all_assigned_devices(gc, &assigned, &num_assigned);
> -if ( rc )
> +r = get_all_assigned_devices(gc, &assigned, &num_assigned);
> +if (r)
>  goto out;
>  
libxl CODING_STYLE allows 'if (r) goto out;', which, since you're
refactoring, you may want to use here.

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] x86/monitor: Disallow setting mem_access_emulate_each_rep when vm_event is NULL

[Andrew Cooper]

On 08/04/2016 08:16, Razvan Cojocaru wrote:
> On 04/08/16 10:10, Razvan Cojocaru wrote:
>> It is meaningless (and potentially dangerous - see 
>> hvmemul_virtual_to_linear())
>> to set mem_access_emulate_each_rep before xc_monitor_enable() (which 
>> allocates
>> vcpu->arch.vm_event) has been called, so return an error from the
>> XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case.
>>
>> Signed-off-by: Razvan Cojocaru 
>> ---
>>  xen/include/asm-x86/monitor.h | 15 +--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
>> index 0954b59..0544836 100644
>> --- a/xen/include/asm-x86/monitor.h
>> +++ b/xen/include/asm-x86/monitor.h
>> @@ -35,11 +35,22 @@ int arch_monitor_domctl_op(struct domain *d, struct 
>> xen_domctl_monitor_op *mop)
>>  switch ( mop->op )
>>  {
>>  case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
>> +{
>> +bool_t value = !!mop->event;

While you are at at, one extra newline here please.

>>  domain_pause(d);
>> -d->arch.mem_access_emulate_each_rep = !!mop->event;
>> +/*
>> + * Enabling emulate_each_rep without a vm_event subscriber
>> + * is meaningless.
>> + */
>> +if ( !d->vcpu || !d->vcpu[0]->arch.vm_event )
> Sorry, this is a bit convoluted, and wrong: if d->vcpu != NULL it will
> return -EINVAL even if d->vcpu[0]->arch.vm_event is NULL. I'll rework
> it. Apologies for the noise.

The issue comes about because d->arch.mem_access_emulate_each_rep is a
per-domain variable, whereas vm_event listeners are per-vcpu.

In principle, it would be legitimate for an introspection engine to only
want to introspect a single vcpu.

Either we need to enforce that every vcpu has vm_event infrastructure
set up (and allow for delivery of events is disabled on a per-vcpu
basis), or settings such as emulate_each_rep needs to be made per-vcpu.

~Andrew

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

Re: [Xen-devel] [PATCH v2 03/10] libxl: fix return value of libxl__device_pci_destroy_all

[Dario Faggioli]

On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote:
> Return rc value instead of always 0.
> 
I'd say "Propagate the error, insetad of always returning 0", but yeah,
this is nitpicking! :-P

Also, not my call, but I'd include this change inside of patch 1: both
are simple enough that they can be made one, IMO (mentioning this new
thing being done in the changelog of the resulting patch, of course).

> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1602,7 +1602,7 @@ int libxl__device_pci_destroy_all(libxl__gc
> *gc, uint32_t domid)
>  }
>  
While you're here, libxl conding style says that the 'int rc' variable
used to host libxl error codes, and if necessary, to return them,
should not be initialized, while this function does it.

Again not my call, but I think you can get rid of the initialization
too (and just assign 0 to it before the loop).

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 05/10] xl_cmdimpl: improve return codes for memset commands

[Dario Faggioli]

On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote:
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3391,15 +3391,15 @@ static int set_memory_max(uint32_t domid,
> const char *mem)
>  memorykb = parse_mem_size_kb(mem);
>  if (memorykb == -1) {
>  fprintf(stderr, "invalid memory size: %s\n", mem);
> -exit(3);
> +return 1;
>
Mmmm.. I see no reason why this can't remain exit(). In fact, it should
be turned int exit(EXIT_FAILURE), and there's Harmandeep's series --
just resubmitted by me tonight-- outstanding that does that [1].

In any case, this patch is probably not necessary any longer, not
because Harmandeep pending series, but because George take care of what
I think you're trying to do in here in
commit 0614c454209ac67016e2296577abfee9e9dcb012 already.

Regards,
Dario

[1] http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg01099.html
-- 
<> (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 06/10] xl_cmdimpl: improve return codes for cd-insert commands

[Dario Faggioli]

On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote:
>  - Use EXIT_{SUCCESS,FAILURE} for main_cd*() function
>  - Use 0/1 as return values of cd_insert function
> 
Done already by commit 04119085f5a2a135e5161535b8821e1aa0d7db8a.

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] x86/monitor: Disallow setting mem_access_emulate_each_rep when vm_event is NULL

[Razvan Cojocaru]

On 04/08/16 11:17, Andrew Cooper wrote:
> On 08/04/2016 08:16, Razvan Cojocaru wrote:
>> On 04/08/16 10:10, Razvan Cojocaru wrote:
>>> It is meaningless (and potentially dangerous - see 
>>> hvmemul_virtual_to_linear())
>>> to set mem_access_emulate_each_rep before xc_monitor_enable() (which 
>>> allocates
>>> vcpu->arch.vm_event) has been called, so return an error from the
>>> XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case.
>>>
>>> Signed-off-by: Razvan Cojocaru 
>>> ---
>>>  xen/include/asm-x86/monitor.h | 15 +--
>>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
>>> index 0954b59..0544836 100644
>>> --- a/xen/include/asm-x86/monitor.h
>>> +++ b/xen/include/asm-x86/monitor.h
>>> @@ -35,11 +35,22 @@ int arch_monitor_domctl_op(struct domain *d, struct 
>>> xen_domctl_monitor_op *mop)
>>>  switch ( mop->op )
>>>  {
>>>  case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
>>> +{
>>> +bool_t value = !!mop->event;
> 
> While you are at at, one extra newline here please.
> 
>>>  domain_pause(d);
>>> -d->arch.mem_access_emulate_each_rep = !!mop->event;
>>> +/*
>>> + * Enabling emulate_each_rep without a vm_event subscriber
>>> + * is meaningless.
>>> + */
>>> +if ( !d->vcpu || !d->vcpu[0]->arch.vm_event )
>> Sorry, this is a bit convoluted, and wrong: if d->vcpu != NULL it will
>> return -EINVAL even if d->vcpu[0]->arch.vm_event is NULL. I'll rework
>> it. Apologies for the noise.
> 
> The issue comes about because d->arch.mem_access_emulate_each_rep is a
> per-domain variable, whereas vm_event listeners are per-vcpu.
> 
> In principle, it would be legitimate for an introspection engine to only
> want to introspect a single vcpu.
> 
> Either we need to enforce that every vcpu has vm_event infrastructure
> set up (and allow for delivery of events is disabled on a per-vcpu
> basis), or settings such as emulate_each_rep needs to be made per-vcpu.

Xc_monitor_enable() does set vm_event up for all of the domain's VCPUs,
so at least for now it is a fair assumption that if it is set for the
first one, it's set for all of them. In other words, your first
requirement is currently being implicitly enforced.

Introspecting a single VCPU is an interesting idea, but not terribly
useful at this point.


Thanks,
Razvan

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

Re: [Xen-devel] [PATCH v2 07/10] xl_cmdimpl - Add return codes for pci-detach, pci-attach, pci-asssignable-add, and pci-assignable-remove.

[Dario Faggioli]

On Wed, 2016-04-06 at 13:46 +0200, Paulina Szubarczyk wrote:
> From: George Dunlap 
> 
> Returning error codes makes it easier for shell scripts to tell if a
> command has failed or succeeded.
> 
> NB this violates the CODING_STYLE preference for not initializing the
> return-value variable at declaration; but in these cases, having a
> "goto out" that jumped over nothing but an "r = 0" seemed
> a bit pointless.
> 
AFAIUI, that rule applies to 'int rc' variables, meant at hosting libxl
error codes and not equally strictly (although it might be a nice to
have) to 'int r' and alike variables meant at hosting return code from
interna functions, syscalls, libxc calls, etc.

Unless I'm missing or misreading something from libxl's CODING_STYLE.
:-)

> Signed-off-by: George Dunlap 
> Signed-off-by: Paulina Szubarczyk 
> 
> ---
> Changed since v1:
> * Changed exit() calls to 'return 1;'
> 
Why do we need to do that? changing exit() in either
exit(EXIT_SUCCESS) or (in most of the cases) exit(EXIT_FAILURE) is much
better, and is the patter we're (slowly :-/) trying to force into xl.

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] [linux-linus test] 89304: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 59254
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 59254
 test-amd64-amd64-xl-credit2  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl-xsm  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-i386-xl-xsm   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl-multivcpu 15 guest-localmigrate   fail REGR. vs. 59254
 test-amd64-i386-xl   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-pair  21 guest-migrate/src_host/dst_host fail REGR. vs. 59254
 test-amd64-i386-pair   22 guest-migrate/dst_host/src_host fail REGR. vs. 59254
 test-amd64-amd64-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 
59254

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 17 guest-localmigrate/x10fail REGR. vs. 59254
 test-armhf-armhf-xl-rtds 11 guest-start   fail REGR. vs. 59254
 test-amd64-amd64-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline 
untested
 test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline 
untested
 test-armhf-armhf-xl-vhd   9 debian-di-install   fail baseline untested
 test-amd64-i386-libvirt-xsm  15 guest-saverestore.2  fail blocked in 59254
 test-amd64-i386-libvirt  15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-libvirt-xsm 15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-libvirt 15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 59254
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 59254

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-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 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-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1 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  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-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-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-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-qemuu-nested-amd 13 xen-boot/l1   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-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   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-vhd 11 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-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass

version targeted for testing:
 linuxc4004b02f8e5b9ce357a0bb1641756cc86962664
baseline version:
 linux45820c294fe1b1a9df495d57

[Xen-devel] Question about the XEN platform pci

[Wu, Bob]

Sorry bother, I read the XEN source code recently, and found the XEN platform 
PCI code under drivers/xen/platform-pci.c,
and I can't fully under this driver's affect, can anybody explain a little for 
me?

Is the platform PCI driver for PV-split-PCI-driver-model such as the 
pci-frontend/pci-backend? or for PCI pass-through model? Or for other purpose?
I saw the xenbus_pcifront_driver/ xenbus_xen_pcibk_driver are registered on 
XENBUS, so I guess the platform-PCI-driver is not for PV PCI driver.

Really thank you for your replay.

Thanks,
Bob

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

Re: [Xen-devel] [PATCH v11 1/2] libxl: add support for vscsi

[Juergen Gross]

On 08/04/16 09:01, Olaf Hering wrote:
> 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 

Reviewed-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] abstract model of IOMMU unmaping/mapping failures

[Xu, Quan]

On April 08, 2016 6:44am,  wrote:
> >>> On 06.04.16 at 09:38,  wrote:
> > On April 01, 2016 7:57pm,  wrote:
> >> >>> On 31.03.16 at 11:06,  wrote:
> >> > 4. __gnttab_unmap_common():rollback (no change)
> >> >
> >> > (Existing code)
> >> >>>...
> >> >  if ( !kind )
> >> > err = iommu_unmap_page(ld, op->frame);
> >> > else if ( !(kind & MAPKIND_WRITE) )
> >> > err = iommu_map_page(ld, op->frame, op->frame,
> >> > IOMMUF_readable);
> >> >
> >> > double_gt_unlock(lgt, rgt);
> >> >
> >> > if ( err )
> >> > rc = GNTST_general_error;
> >> ><<...
> >> >
> >> > Similarly no change required, as error has been correctly handled.
> >>
> >> I wouldn't call this "correctly handled", but for the hardware domain
> >> it should be good enough, and by crashing DomU-s simply reporting the
> >> error up the call tree is sufficient. One question though is whether
> >> the loops higher up the call tree in grant_table.c shouldn't be
> >> exited when noticing the domain has crashed, both to avoid unnecessary
> work and to reduce the risk of secondary problems.
> >>
> >
> > For this point, I suppose that the domain structure is not destructed
> > (we are safe to call domain's element, e.g. ld->grant_table) in
> > do_grant_table_op hypercall cycle, even the domain is crashed down. I
> > am not quite sure, whether it is correct or not, could you explain
> > more?
> 
> Explain what more? 
> Sure, struct domain stays around until the domain gets
> actually cleaned up, 

Explain why it is sure. Now i have got it.

> so accesses to its grant table (and alike) remain valid while
> execution didn't leave the context of that guest (vCPU) yet.
> 

Thanks for your reminding.

> >> > 8. p2m_remove_page():rollback one level(change required).
> >> >
> >> > (Existing code)
> >> >   >>...
> >> > if ( !paging_mode_translate(p2m->domain) )
> >> > {
> >> > if ( need_iommu(p2m->domain) )
> >> > for ( i = 0; i < (1 << page_order); i++ )
> >> > iommu_unmap_page(p2m->domain, mfn + i);
> >> > return 0;
> >> > }
> >> >   <<...
> >> >
> >> > There is no error checking of iommu_unmap_page. We need to add.
> >> > However, there are several callers of this function which don't
> >> > handle error at all. I'm not sure whether hardware domain will use this
> function.
> >> > Since it is in core p2m logic (which I don't have much knowledge),
> >> > I hope we can print a warning and simply return error here (given
> >> > the fact that non-hardware domains are all crashed immediately
> >> > within unmap call)
> >>
> >> Yes, at least error propagation needs to be added here. I don't think
> >> more is required. (Be careful, btw, with adding warnings - you must
> >> not spam the log with such.)
> >>
> >
> >
> >  agreed. A quick question about error propagation:
> > ...
> > for ( i = 0; i < (1UL << page_order); i++ )
> > iommu_unmap_page(p2m->domain, gfn + i); ...
> >
> > As you mentioned, as a special case, unmapping should perhaps continue
> > despite an error, in an attempt to do best effort cleanup.
> > Then i could modify the code as:
> >
> > ...
> > for ( i = 0; i < (1UL << page_order); i++ ) {
> > rc = iommu_unmap_page(p2m->domain, gfn + i);
> > if ( rc )
> >   ret = rc;
> > }
> > ..
> >return ret;
> > ...
> >
> > It looks cumbersome to me, any suggestion?
> 
> What's cumbersome here?
> 

Sorry, I don't like to introduce 'rc'/'ret' in the same function. Ignore me, if 
it is working.

> >> > 11. __get_page_type(): open (propose no-rollback).
> >> > The following is in detail:
> >> > >>...
> >> >   if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
> >> > {
> >> > if ( (x & PGT_type_mask) == PGT_writable_page )
> >> > iommu_unmap_page(d, mfn_to_gmfn(d,
> >> page_to_mfn(page)));
> >> > else if ( type == PGT_writable_page )
> >> > iommu_map_page(d, mfn_to_gmfn(d,
> >> page_to_mfn(page)),
> >> >page_to_mfn(page),
> >> >
> >> IOMMUF_readable|IOMMUF_writable);
> >> > }
> >> > <<...
> >> >
> >> > It's almost in the end of the func.
> >> > As the limited set of cases where __get_page_type() calls
> >> > iommu_{,un}map_page(), and a get_page_type() invocation that
> >> > previously was known to never fail (or at least so we hope, based
> >> > on the existing code) [I got it from Jan's reply.],
> >>
> >> Please don't mis-quote me: I said this for a specific caller of the
> >> function, not about the function itself.
> >>
> >
> > Sorry for that.
> >
> >> > we can classify it as normal PV domain and be treated as a fatal error.
> >> > So for I am inclined to leave it as today.
> >>
> >
> > I think one level propagation, only propagated in __get_page_type(),
> > is enough.
> 
> Not sure what you mean with "one level propagation" here.
> 

one level propagation is propagating error only in __get_

Re: [Xen-devel] [PATCH v2 10/10] libxl: libxl_tmem functions improving coding style

[Dario Faggioli]

On Wed, 2016-04-06 at 13:46 +0200, Paulina Szubarczyk wrote:
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -6238,14 +6238,14 @@ uint32_t libxl_vm_get_start_time(libxl_ctx
> *ctx, uint32_t domid)
>  
>  char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
>  {
> -int rc;
> +int r;
>  char _buf[32768];
>  GC_INIT(ctx);
>  
> -rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST,
> domid, 32768, use_long,
> +r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST,
> domid, 32768, use_long,
>
This is ok, but...
>   _buf);
>
...this now have the wrong indentation: it should be aligned with the
opening '(' in the line above (so, basically, you have to kill one
white space and move it back by one column).

> -if (rc < 0) {
> -LOGEV(ERROR, rc, "Can not get tmem list");
> +if (r < 0) {
> +LOGEV(ERROR, r, "Can not get tmem list");
>
libxc functions are supposed to, on failure, set errno and alwas return
-1. Using LOGEV and passing r to it means that you're always logging -1
as error code.

I think 'LOG(ERROR, "blabla")' is what should be used here.

And these same comments apply to all the other hunks of the patch.

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] [linux-3.16 test] 89328: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 
85048

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit2 15 guest-start/debian.repeat fail in 89242 pass in 
89328
 test-armhf-armhf-xl  16 guest-start.2   fail pass in 89242
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail pass in 
89242

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds15 guest-start/debian.repeat fail blocked in 85048
 test-armhf-armhf-xl-rtds 11 guest-start   fail in 89242 like 85048
 build-i386-rumpuserxen6 xen-buildfail   like 85048
 build-amd64-rumpuserxen   6 xen-buildfail   like 85048
 test-amd64-amd64-xl-credit2  17 guest-localmigrate/x10   fail   like 85048
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 85048
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 85048
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 85048
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 85048

Tests which did not succeed, but are not blocking:
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 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-amd64-amd64-xl-multivcpu 17 guest-localmigrate/x10   fail  never pass
 test-amd64-amd64-xl-rtds 17 guest-localmigrate/x10   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-amd64-amd64-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-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-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  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-amd64-amd64-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-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  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-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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-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-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail 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-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 linux3a96c6601b6fc47baa6d296f9111ba7be4dad6fc
baseline version:
 linux7f2a8840d127c8d5c59a5d79235e1205aba2e102

Last test of basis85048  2016-03-02 10:56:10 Z   36 days
Testing same since87897  2016-03-29 14:28:05 Z9 days   10 attempts


People who touched revisions under test:
  Akshay Bhat 
  Al Viro 
  Alex Deucher 
  Alex Williamson 
  Alexander Deucher 
  Amir Vad

Re: [Xen-devel] [PATCH v2 08/10] libxl: improve main_tmem_* return codes

[Dario Faggioli]

On Wed, 2016-04-06 at 13:46 +0200, Paulina Szubarczyk wrote:
> Functions libxl_tmem_freeze(), libxl_tmem_thaw(), libxl_tmem_set()
> and
> libxl_tmem_shared_auth() located in libxl.c file return
> ERROR_FAIL/ERROR_INVAL or internal error codes from libxc library
> improve main_tmem_* return codes by returning EXIT_{SUCCESS/FAILURE}
> accordingly to return codes of those functions.
> 
This is a patch to xl, so the subject should be:
 "xl: improve main_tmem_* return codes"

> Signed-off-by: Paulina Szubarczyk 
> Reviewed-by: Konrad Rzeszutek Wilk 
> Acked-by: Roger Pau Monné 
>
With the above done:

Reviewed-by: Dario Faggioli 

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 05/10] xl_cmdimpl: improve return codes for memset commands

[Paulina Szubarczyk]

On Fri, 2016-04-08 at 10:26 +0200, Dario Faggioli wrote:
> On Wed, 2016-04-06 at 13:45 +0200, Paulina Szubarczyk wrote:
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -3391,15 +3391,15 @@ static int set_memory_max(uint32_t domid,
> > const char *mem)
> >  memorykb = parse_mem_size_kb(mem);
> >  if (memorykb == -1) {
> >  fprintf(stderr, "invalid memory size: %s\n", mem);
> > -exit(3);
> > +return 1;
> >
> Mmmm.. I see no reason why this can't remain exit(). In fact, it should
> be turned int exit(EXIT_FAILURE), and there's Harmandeep's series --
> just resubmitted by me tonight-- outstanding that does that [1].

There was a discussion [2] that the exit() could be replaced by return 1
in the v1 of this patch, that is way I did here and in the following
commits. Should it be rather reverted? 

> 
> In any case, this patch is probably not necessary any longer, not
> because Harmandeep pending series, but because George take care of what
> I think you're trying to do in here in
> commit 0614c454209ac67016e2296577abfee9e9dcb012 already.

In that George's commit the set_memory_* functions return
EXIT_{FAILURE/SUCCESS}. Following the patches from Harmandeep's series I
was going to changed them to return 1/0 and return
EXIT_{FAILURE/SUCCESS} in main_* functions, after asking about that in
that thread [3], though, you said there are exceptions too that in [1].

> 
> Regards,
> Dario
> 
> [1] http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg01099.html
[2]
http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg00384.html
[3]
http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg03422.html

Paulina



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

[Xen-devel] [PATCH for-4.7 v3 2/2] xen/x86: Introduce a new VMASSIST for architectural behaviour of iopl

[Andrew Cooper]

The existing vIOPL interface is hard to use, and need not be.

Introduce a VMASSIST with which a guest can opt-in to having vIOPL behaviour
consistenly with native hardware.

Specifically:
 - virtual iopl updated from do_iret() hypercalls.
 - virtual iopl reported in bounce frames.
 - guest kernels assumed to be level 0 for the purpose of iopl checks.

v->arch.pv_vcpu.iopl is altered to store IOPL shifted as it would exist
eflags, for the benefit of the assembly code.

Signed-off-by: Andrew Cooper 
Reviewed-by: Jan Beulich 
---
v2:
 * Shift vcpu.iopl to match eflags.
 * Factor out iopl_ok().
 * Use CMOVcc in assembly code.
v3:
 * Don't clear AC for 32bit failsafe callback.

Along with this, I have functional tests for both vIOPL interfaces in PV
guests.
---
 xen/arch/x86/domain.c  | 15 ++-
 xen/arch/x86/physdev.c |  2 +-
 xen/arch/x86/traps.c   | 15 +--
 xen/arch/x86/x86_64/asm-offsets.c  |  3 +++
 xen/arch/x86/x86_64/compat/entry.S |  7 ++-
 xen/arch/x86/x86_64/compat/traps.c |  4 
 xen/arch/x86/x86_64/entry.S|  7 ++-
 xen/arch/x86/x86_64/traps.c|  3 +++
 xen/include/asm-x86/config.h   |  1 +
 xen/include/asm-x86/domain.h   |  4 +++-
 xen/include/public/xen.h   |  8 
 11 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index a4f6db2..9e436ef 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -994,7 +994,7 @@ int arch_set_info_guest(
 init_int80_direct_trap(v);
 
 /* IOPL privileges are virtualised. */
-v->arch.pv_vcpu.iopl = (v->arch.user_regs.eflags >> 12) & 3;
+v->arch.pv_vcpu.iopl = v->arch.user_regs.eflags & X86_EFLAGS_IOPL;
 v->arch.user_regs.eflags &= ~X86_EFLAGS_IOPL;
 
 /* Ensure real hardware interrupts are enabled. */
@@ -1735,8 +1735,10 @@ static void load_segments(struct vcpu *n)
 cs_and_mask = (unsigned short)regs->cs |
 ((unsigned int)vcpu_info(n, evtchn_upcall_mask) << 16);
 /* Fold upcall mask into RFLAGS.IF. */
-eflags  = regs->_eflags & ~X86_EFLAGS_IF;
+eflags  = regs->_eflags & ~(X86_EFLAGS_IF|X86_EFLAGS_IOPL);
 eflags |= !vcpu_info(n, evtchn_upcall_mask) << 9;
+if ( VM_ASSIST(n->domain, architectural_iopl) )
+eflags |= n->arch.pv_vcpu.iopl;
 
 if ( !ring_1(regs) )
 {
@@ -1763,7 +1765,8 @@ static void load_segments(struct vcpu *n)
 vcpu_info(n, evtchn_upcall_mask) = 1;
 
 regs->entry_vector |= TRAP_syscall;
-regs->_eflags  &= 0xFFFCBEFFUL;
+regs->_eflags  &= ~(X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|
+X86_EFLAGS_IOPL|X86_EFLAGS_TF);
 regs->ss= FLAT_COMPAT_KERNEL_SS;
 regs->_esp  = (unsigned long)(esp-7);
 regs->cs= FLAT_COMPAT_KERNEL_CS;
@@ -1781,8 +1784,10 @@ static void load_segments(struct vcpu *n)
 ((unsigned long)vcpu_info(n, evtchn_upcall_mask) << 32);
 
 /* Fold upcall mask into RFLAGS.IF. */
-rflags  = regs->rflags & ~X86_EFLAGS_IF;
+rflags  = regs->rflags & ~(X86_EFLAGS_IF|X86_EFLAGS_IOPL);
 rflags |= !vcpu_info(n, evtchn_upcall_mask) << 9;
+if ( VM_ASSIST(n->domain, architectural_iopl) )
+rflags |= n->arch.pv_vcpu.iopl;
 
 if ( put_user(regs->ss,rsp- 1) |
  put_user(regs->rsp,   rsp- 2) |
@@ -1806,7 +1811,7 @@ static void load_segments(struct vcpu *n)
 
 regs->entry_vector |= TRAP_syscall;
 regs->rflags   &= ~(X86_EFLAGS_AC|X86_EFLAGS_VM|X86_EFLAGS_RF|
-X86_EFLAGS_NT|X86_EFLAGS_TF);
+X86_EFLAGS_NT|X86_EFLAGS_IOPL|X86_EFLAGS_TF);
 regs->ss= FLAT_KERNEL_SS;
 regs->rsp   = (unsigned long)(rsp-11);
 regs->cs= FLAT_KERNEL_CS;
diff --git a/xen/arch/x86/physdev.c b/xen/arch/x86/physdev.c
index 1cb9b58..5a49796 100644
--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -529,7 +529,7 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
arg)
 if ( set_iopl.iopl > 3 )
 break;
 ret = 0;
-curr->arch.pv_vcpu.iopl = set_iopl.iopl;
+curr->arch.pv_vcpu.iopl = MASK_INSR(set_iopl.iopl, X86_EFLAGS_IOPL);
 break;
 }
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 8125c53..7861a3c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1779,6 +1779,17 @@ static int read_gate_descriptor(unsigned int gate_sel,
 return 1;
 }
 
+/* Perform IOPL check between the vcpu's shadowed IOPL, and the assumed cpl. */
+static bool_t iopl_ok(const struct vcpu *v, const struct cpu_user_regs *regs)
+{
+unsigned int cpl = guest_kernel_mode(v, regs) ?
+(VM_ASSIST(v->dom

Re: [Xen-devel] [PATCH v2 05/10] xl_cmdimpl: improve return codes for memset commands

[Dario Faggioli]

On Fri, 2016-04-08 at 11:04 +0200, Paulina Szubarczyk wrote:
> On Fri, 2016-04-08 at 10:26 +0200, Dario Faggioli wrote:
> > Mmmm.. I see no reason why this can't remain exit(). In fact, it
> > should
> > be turned int exit(EXIT_FAILURE), and there's Harmandeep's series
> > --
> > just resubmitted by me tonight-- outstanding that does that [1].
> There was a discussion [2] that the exit() could be replaced by
> return 1
> in the v1 of this patch, that is way I did here and in the following
> commits. 
>
Yeah, and in fact, sorry for not participating in that discussion... I
wanted to, but was otherwise engaged at the time.

> Should it be rather reverted? 
> 
Well, this is something Wei and Ian have the final say on.

I'm actually fine with either approaches but, considering that:
 1. calling exit() in situations similar to this is, AFAICT, what xl
    does pretty much always;
 2. converting exit()-s to 'return 1' may (but that's on a case by
    case basis) require a bigger patch;

Right now, given the status this is in xl, I would just focus on making
sure that the program terminates with either EXIT_SUCCESS or
EXIT_FAILURE; if that comes from an exit() being called in a non-top
level functions, so be it... And, in fact, in this case, the quickest
and cleanest way to make that happen, is doing what George's patch
does, which is neither exit() nor 'return 1'. :-/

> > In any case, this patch is probably not necessary any longer, not
> > because Harmandeep pending series, but because George take care of
> > what
> > I think you're trying to do in here in
> > commit 0614c454209ac67016e2296577abfee9e9dcb012 already.
> In that George's commit the set_memory_* functions return
> EXIT_{FAILURE/SUCCESS}. Following the patches from Harmandeep's
> series I
> was going to changed them to return 1/0 and return
> EXIT_{FAILURE/SUCCESS} in main_* functions, after asking about that
> in
> that thread [3], though, you said there are exceptions too that in
> [1].
> 
Yep, it's again, at least up to a cartain extent, a matter of taste and
something that is hard to carve in stone, leaving no room for
exceptions.

For example, in principle, what you say you're planning to do is
correct: internal functions should return 0/1, top level function (and
main_foo() are top level functions) should either exit() or return
EXIT_SUCCESS/EXIT_FAILURE.

But.

Although set_memory_max() certainly is an internal function, it is
_only_ used from main_memmax() like this:

  return set_memory_max(domid, mem);

and since main_memmax() is a top level function, if it were my call,
I'd be more than ok with bending the above stated rule and allow
set_memory_max() to return EXIT_SUCCESS/EXIT_FAILURE.

IOW, since it's return code is always directly returned by a top level
function, I think we can allow set_memory_max() (and functions used in
similar ways) to return just like top level functions do.

I know, it's tricky, because it's not just a matter of "blindly"
applying a set of rules, you have to consider all the implication --and 
there are many of them-- on a case by case basis. It's what (among
other things), IMO, makes Open Source challenging, but also beautiful
and rewarding. :-D

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] arm: Fix asynchronous aborts (SError exceptions) due to bogus PTEs

[Julien Grall]

On 06/04/16 19:57, Shanker Donthineni wrote:

Hi Julien/Stefano,


Hi Shanker,



Any other comments to be addressed? Please propose an alternative
solution to fix the problem if this patch changes are not appropriate.


All the comments have been addressed.



On 03/28/2016 11:46 PM, Shanker Donthineni wrote:

From: Vikram Sethi 

ARMv8 architecture allows performing prefetch data/instructions
from memory locations marked as normal memory. Prefetch does not
mean that the data/instruction has to be used/executed in code
flow. All PTEs that appear to be valid to MMU must contain valid
physical address with proper attributes otherwise MMU table walk
might cause imprecise asynchronous aborts.

The way current XEN code is preparing page tables for frametable
and xenheap memory can create bogus PTEs. This patch fixes the
issue by clearing page table memory before populating EL2 L0/L1
PTEs. Without this patch XEN crashes on Qualcomm Technologies
server chips due to asynchronous aborts.

The speculative/prefetch feature explanation is scattered everywhere
in ARM specification but below two sections have useful information.

E2.8 Memory types and attributes (ver DDI0487A_h)
G4.12.6 External abort on a translation table walk (ver DDI0487A_h)

Signed-off-by: Vikram Sethi 
Signed-off-by: Shanker Donthineni 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH] Data integrity extension support for xen-block

[Roger Pau Monné]

On Fri, 8 Apr 2016, Bob Liu wrote:
> 
> On 04/07/2016 11:55 PM, Juergen Gross wrote:
> > On 07/04/16 12:00, Bob Liu wrote:
> >> * What's data integrity extension and why?
> >> Modern filesystems feature checksumming of data and metadata to protect 
> >> against
> >> data corruption.  However, the detection of the corruption is done at read 
> >> time
> >> which could potentially be months after the data was written.  At that 
> >> point the
> >> original data that the application tried to write is most likely lost.
> >>
> >> The solution in Linux is the data integrity framework which enables 
> >> protection
> >> information to be pinned to I/Os and sent to/received from controllers that
> >> support it. struct bio has been extended with a pointer to a struct bip 
> >> which
> >> in turn contains the integrity metadata. The bip is essentially a trimmed 
> >> down
> >> bio with a bio_vec and some housekeeping.
> >>
> >> * Issues when xen-block get involved.
> >> xen-blkfront only transmits the normal data of struct bio while the 
> >> integrity
> >> metadata buffer(struct bio_integrity_payload in each bio) is ignored.
> >>
> >> * Proposal of transmitting bio integrity payload.
> >> Adding an extra request following the normal data request, this extra 
> >> request
> >> contains the integrity payload.
> >> The xen-blkback will reconstruct an new bio with both received normal data 
> >> and
> >> integrity metadata.
> >>
> >> Welcome any better ideas, thank you!
> >>
> >> [1] http://lwn.net/Articles/280023/
> >> [2] https://www.kernel.org/doc/Documentation/block/data-integrity.txt
> >>
> >> Signed-off-by: Bob Liu 
> >> ---
> >>  xen/include/public/io/blkif.h |   50 
> >> +
> >>  1 file changed, 50 insertions(+)
> >>
> >> diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
> >> index 99f0326..3d8d39f 100644
> >> --- a/xen/include/public/io/blkif.h
> >> +++ b/xen/include/public/io/blkif.h
> >> @@ -635,6 +635,28 @@
> >>  #define BLKIF_OP_INDIRECT  6
> >>  
> >>  /*
> >> + * Recognized only if "feature-extra-request" is present in backend 
> >> xenbus info.
> >> + * A request with BLKIF_OP_EXTRA_FLAG indicates an extra request is 
> >> followed
> >> + * in the shared ring buffer.
> >> + *
> >> + * By this way, extra data like bio integrity payload can be transmitted 
> >> from
> >> + * frontend to backend.
> >> + *
> >> + * The 'wire' format is like:
> >> + *  Request 1: xen_blkif_request
> >> + * [Request 2: xen_blkif_extra_request](only if request 1 has 
> >> BLKIF_OP_EXTRA_FLAG)
> >> + *  Request 3: xen_blkif_request
> >> + *  Request 4: xen_blkif_request
> >> + * [Request 5: xen_blkif_extra_request](only if request 4 has 
> >> BLKIF_OP_EXTRA_FLAG)
> >> + *  ...
> >> + *  Request N: xen_blkif_request
> >> + *
> >> + * If a backend does not recognize BLKIF_OP_EXTRA_FLAG, it should *not* 
> >> create the
> >> + * "feature-extra-request" node!
> >> + */
> >> +#define BLKIF_OP_EXTRA_FLAG (0x80)
> >> +
> >> +/*
> >>   * Maximum scatter/gather segments per request.
> >>   * This is carefully chosen so that sizeof(blkif_ring_t) <= PAGE_SIZE.
> >>   * NB. This could be 12 if the ring indexes weren't stored in the same 
> >> page.
> >> @@ -703,6 +725,34 @@ struct blkif_request_indirect {
> >>  };
> >>  typedef struct blkif_request_indirect blkif_request_indirect_t;
> >>  
> >> +enum blkif_extra_request_type {
> >> +  BLKIF_EXTRA_TYPE_DIX = 1,   /* Data integrity extension 
> >> payload.  */
> >> +};
> >> +
> >> +struct bio_integrity_req {
> >> +  /*
> >> +   * Grant mapping for transmitting bio integrity payload to backend.
> >> +   */
> >> +  grant_ref_t *gref;
> >> +  unsigned int nr_grefs;
> >> +  unsigned int len;
> >> +};
> > 
> > How does the payload look like? It's structure should be defined here
> > or a reference to it's definition in case it is a standard should be
> > given.
> > 
> 
> The payload is also described using struct bio_vec(the same as bio).
> 
> /*
>  * bio integrity payload
>  */
> struct bio_integrity_payload {
>   struct bio  *bip_bio;   /* parent bio */
> 
>   struct bvec_iterbip_iter;
> 
>   bio_end_io_t*bip_end_io;/* saved I/O completion fn */
> 
>   unsigned short  bip_slab;   /* slab the bip came from */
>   unsigned short  bip_vcnt;   /* # of integrity bio_vecs */
>   unsigned short  bip_max_vcnt;   /* integrity bio_vec slots */
>   unsigned short  bip_flags;  /* control flags */
> 
>   struct work_struct  bip_work;   /* I/O completion */
> 
>   struct bio_vec  *bip_vec;
>   struct bio_vec  bip_inline_vecs[0];/* embedded bvec array */
> };

There's no way we are going to embed such a Linux specific payload into 
the PV block protocol. Also, I have the feeling there are a lot of fields 
in this struct that make no sense to transmit on the ring (work_stru

[Xen-devel] [ovmf test] 89340: regressions - FAIL

[osstest service owner]

flight 89340 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/89340/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail REGR. vs. 65543
 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 65543

version targeted for testing:
 ovmf 2777ed7068f0ce186761f5cfef8a0c2252e3e2a5
baseline version:
 ovmf 5ac96e3a28dd26eabee421919f67fa7c443a47f1

Last test of basis65543  2015-12-08 08:45:15 Z  122 days
Failing since 65593  2015-12-08 23:44:51 Z  121 days  140 attempts
Testing same since89340  2016-04-07 10:43:56 Z0 days1 attempts


People who touched revisions under test:
  "Samer El-Haj-Mahmoud" 
  "Wu, Hao A" 
  "Yao, Jiewen" 
  Alcantara, Paulo 
  Anbazhagan Baraneedharan 
  Andrew Fish 
  Ard Biesheuvel 
  Arthur Crippa Burigo 
  Cecil Sheng 
  Chao Zhang 
  Chao Zhang
  Charles Duffy 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Daocheng Bu 
  Daryl McDaniel 
  David Woodhouse 
  Derek Lin 
  edk2 dev 
  edk2-devel 
  Eric Dong 
  Eric Dong 
  erictian
  Eugene Cohen 
  Evan Lloyd 
  Feng Tian 
  Fu Siyuan 
  Gabriel Somlo 
  Gary Ching-Pang Lin 
  Gary Lin 
  Ghazi Belaam 
  Hao Wu 
  Haojian Zhuang 
  Hess Chen 
  Heyi Guo 
  Hot Tian 
  Jaben Carsey 
  James Bottomley 
  Jeff Fan 
  Jeremy Linton 
  Jiaxin Wu 
  jiewen yao 
  Jim Dailey 
  jim_dai...@dell.com 
  jljusten
  Jordan Justen 
  Juliano Ciocari 
  Karyne Mayer 
  Ken Lu 
  Kun Gui 
  Larry Hauch 
  Laszlo Ersek 
  Leahy, Leroy P
  Leahy, Leroy P 
  Lee Leahy 
  Leekha Shaveta 
  Leendert van Doorn 
  Leif Lindholm 
  Leif Lindholm 
  Leo Duran 
  Leon Li 
  Liming Gao 
  lzeng14
  Mark Doran 
  Mark Rutland 
  Marvin Haeuser 
  Marvin Häuser 
  Michael D Kinney 
  Michael Kinney 
  Michael LeMay 
  Michael Thomas 
  Michał Zegan 
  Ni, Ruiyu 
  Ni, Ruiyu 
  niruiyu
  Olivier Martin 
  Paolo Bonzini 
  Paulo Alcantara 
  Paulo Alcantara Cavalcanti 
  Peter Kirmeier 
  Qin Long 
  Qing Huang 
  Qiu Shumin 
  Qiu, Shumin 
  Rodrigo Dias Correa 
  Ruiyu Ni 
  Ryan Harkin 
  Samer El-Haj-Mahmoud 
  Samer El-Haj-Mahmoud 
  Sami Mujawar 
  Shivamurthy Shastri 
  Shumin Qiu 
  Star Zeng 
  Supreeth Venkatesh 
  Tapan Shah 
  Thomas Palmer 
  Tian, Feng 
  Vladislav Vovchenko 
  Yao Jiewen 
  Yao, Jiewen 
  Ye Ting 
  Yonghong Zhu 
  Zeng, Star 
  Zhang Lubo 
  Zhang, Chao B 
  Zhang, Lubo 
  Zhangfei Gao 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail



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 17938 lines long.)

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

Re: [Xen-devel] [PATCH v3 1/6] xl: improve return and exit codes of memory related functions

[Dario Faggioli]

On Fri, 2016-04-08 at 04:23 +0200, Dario Faggioli wrote:
> 
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const
> char *mem)
>  memorykb = parse_mem_size_kb(mem);
>  if (memorykb == -1) {
>  fprintf(stderr, "invalid memory size: %s\n", mem);
> -exit(3);
> +exit(EXIT_FAILURE);
>
Actually, after George's 0614c4542, it's better if this is turned into
a return (and likewise in set_memory_target()).

The inlined (and attached) patch does that.

And there's an updated branch (labelled v4, and of course I can
resubmit the whole series from there, if necessary), with this version
of the patch in it, at:
 git://xenbits.xen.org/people/dariof/xen.git rel/xl/exit-code-cleanup-v4
 
http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/xl/exit-code-cleanup-v4

Regards,
Dario
---
commit 43586222084638ec98b693702132f3412a63ddda
Author: Harmandeep Kaur 
Date:   Thu Apr 7 12:38:09 2016 +0200

xl: improve return and exit codes of memory related functions

by making them more consistent with other examples in xl.

While there, make freemem() of boolean return type, which
looks more natural, and add comment explaining why
parse_mem_size_kb() needs to diverge from the pattern.

Signed-off-by: Harmandeep Kaur 
Signed-off-by: Dario Faggioli 
---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: George Dunlap 
---
v4: make set_memory_max() and set_memory_target() more
consistent.

v3: Shorten changelog.
Make freemem() boolean return type.

v2: Add comment to explain return vaule of parse_mem_size_kb().
Add freemem() and main_sharing().
Remove find_domain().

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2ee6c74..eab2abd 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2680,40 +2680,45 @@ static int preserve_domain(uint32_t *r_domid, 
libxl_event *event,
 return rc == 0 ? 1 : 0;
 }
 
-static int freemem(uint32_t domid, libxl_domain_build_info *b_info)
+/*
+ * Returns false if memory can't be freed, but also if we encounter errors.
+ * Returns true in case there is already, or we manage to free it, enough
+ * memory, but also if autoballoon is false.
+ */
+static bool freemem(uint32_t domid, libxl_domain_build_info *b_info)
 {
 int rc, retries = 3;
 uint32_t need_memkb, free_memkb;
 
 if (!autoballoon)
-return 0;
+return true;
 
 rc = libxl_domain_need_memory(ctx, b_info, &need_memkb);
 if (rc < 0)
-return rc;
+return false;
 
 do {
 rc = libxl_get_free_memory(ctx, &free_memkb);
 if (rc < 0)
-return rc;
+return false;
 
 if (free_memkb >= need_memkb)
-return 0;
+return true;
 
 rc = libxl_set_memory_target(ctx, 0, free_memkb - need_memkb, 1, 0);
 if (rc < 0)
-return rc;
+return false;
 
 /* wait until dom0 reaches its target, as long as we are making
  * progress */
 rc = libxl_wait_for_memory_target(ctx, 0, 10);
 if (rc < 0)
-return rc;
+return false;
 
 retries--;
 } while (retries > 0);
 
-return ERROR_NOMEM;
+return false;
 }
 
 static void autoconnect_console(libxl_ctx *ctx_ignored,
@@ -2980,8 +2985,7 @@ start:
 goto error_out;
 
 if (domid_soft_reset == INVALID_DOMID) {
-ret = freemem(domid, &d_config.b_info);
-if (ret < 0) {
+if (!freemem(domid, &d_config.b_info)) {
 fprintf(stderr, "failed to free memory for the domain\n");
 ret = ERROR_FAIL;
 goto error_out;
@@ -3245,6 +3249,7 @@ void help(const char *command)
 }
 }
 
+/* Returns -1 on failure; the amount of memory on success. */
 static int64_t parse_mem_size_kb(const char *mem)
 {
 char *endptr;
@@ -3391,7 +3396,7 @@ static int set_memory_max(uint32_t domid, const char *mem)
 memorykb = parse_mem_size_kb(mem);
 if (memorykb == -1) {
 fprintf(stderr, "invalid memory size: %s\n", mem);
-exit(3);
+return EXIT_FAILURE;
 }
 
 if (libxl_domain_setmaxmem(ctx, domid, memorykb)) {
@@ -3425,7 +3430,7 @@ static int set_memory_target(uint32_t domid, const char 
*mem)
 memorykb = parse_mem_size_kb(mem);
 if (memorykb == -1)  {
 fprintf(stderr, "invalid memory size: %s\n", mem);
-exit(3);
+return EXIT_FAILURE;
 }
 
 if (libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1)) {
@@ -6132,7 +6137,7 @@ int main_sharing(int argc, char **argv)
 info = libxl_list_domain(ctx, &nb_domain);
 if (!info) {
 fprintf(stderr, "libxl_list_domain failed.\n");
-return 1;
+return EXIT_FAILURE;
 }
 info_free = info;
 } else if (optind ==

Re: [Xen-devel] [PATCH v3 08/11] xen: sched: allow for choosing credit2 runqueues configuration at boot

[Dario Faggioli]

On Fri, 2016-04-08 at 09:39 +0200, Juergen Gross wrote:
> On 08/04/16 09:35, Dario Faggioli wrote:
> > 
> > A revised version of this patch is provided here (both inlined and
> > attached), and a branch with the remaining to be committed patches
> > of
> > this series, and with this patch changed as you suggest, is
> > available
> > at:
> > 
> >  git://xenbits.xen.org/people/dariof/xen.git rel/sched/credit2/fix-
> > runq-and-haff-v4
> >  http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;
> > h=refs/heads/rel/sched/credit2/fix-runq-and-haff-v4
>
> Thanks.
> 
Well, thanks to you. :-)

> Reviewed-by: Juergen Gross 
> 
I've updated the branch at:
 git://xenbits.xen.org/people/dariof/xen.git 
rel/sched/credit2/fix-runq-and-haff-v4
 
http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/sched/credit2/fix-runq-and-haff-v4

so that this patch, in there, now has this tag.

I should say that I force-pushed to it, but I don't really expect
anyone to be tracking it. :-P

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] Running Xen on Nvidia Jetson-TK1

[Julien Grall]

On 07/04/16 08:48, Dushyant Behl wrote:

Hello,


Hi Dushyant,


On Fri, Apr 1, 2016 at 3:34 PM, Julien Grall  wrote:

Hello Dushyant,

On 29/03/16 21:56, Dushyant Behl wrote:


On Wed, Mar 30, 2016 at 12:31 AM, Julien Grall 
wrote:


On 24/03/16 11:05, Dushyant Behl wrote:




(XEN) DOM0: [0.00] irq: no irq domain found for
/interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for
/interrupt-controller !
(XEN) DOM0: [0.00] irq: no irq domain found for
/interrupt-controller !
(XEN) DOM0: [0.00] arch_timer: No interrupt available, giving up



It looks like to me that Xen is not recreating the device-tree correctly. I
would look into the kernel to find what is expected.


This looks like a possible bug (or some missing feature) in Xen's
device tree creation which could
take some time to handle, so if I could be of any more help to you
with this issue please let me know.


There was a conversation on #xen-arm few days ago about this problem.
Xen doesn't correctly recreate the GIC node which result in a loop 
between the interrupt controller. Can you try the below patch?


http://dev.ktemkin.com/misc/xenarm-gic-parents.patch



[I've cc'ed Ian Campbell in this mail (Sorry for cc'ing you explicitly)]


Ian's citrix e-mail is not valid anymore. I have CC'ed the new one.


Ian,

Actually, I want to run Xen on the Tegra Jetson board for some project
of mine but currently Linux-4.1 is
failing as dom0 because its not able to receive interrupts from the arch_timer.
This link contains the dom0 failure boot log -
http://lists.xenproject.org/archives/html/xen-devel/2016-03/msg03715.html

In your patch for *Hacky* support for Jetsok-TK1 you said that you
were able to run guests on
Jetson-tk1 board with Xen. Can I know which kernel version you used as
dom0 (and possibly domU guests)?


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC PATCH] Data integrity extension support for xen-block

[Bob Liu]

On 04/08/2016 05:44 PM, Roger Pau Monné wrote:
> On Fri, 8 Apr 2016, Bob Liu wrote:
>>
>> On 04/07/2016 11:55 PM, Juergen Gross wrote:
>>> On 07/04/16 12:00, Bob Liu wrote:
 * What's data integrity extension and why?
 Modern filesystems feature checksumming of data and metadata to protect 
 against
 data corruption.  However, the detection of the corruption is done at read 
 time
 which could potentially be months after the data was written.  At that 
 point the
 original data that the application tried to write is most likely lost.

 The solution in Linux is the data integrity framework which enables 
 protection
 information to be pinned to I/Os and sent to/received from controllers that
 support it. struct bio has been extended with a pointer to a struct bip 
 which
 in turn contains the integrity metadata. The bip is essentially a trimmed 
 down
 bio with a bio_vec and some housekeeping.

 * Issues when xen-block get involved.
 xen-blkfront only transmits the normal data of struct bio while the 
 integrity
 metadata buffer(struct bio_integrity_payload in each bio) is ignored.

 * Proposal of transmitting bio integrity payload.
 Adding an extra request following the normal data request, this extra 
 request
 contains the integrity payload.
 The xen-blkback will reconstruct an new bio with both received normal data 
 and
 integrity metadata.

 Welcome any better ideas, thank you!

 [1] http://lwn.net/Articles/280023/
 [2] https://www.kernel.org/doc/Documentation/block/data-integrity.txt

 Signed-off-by: Bob Liu 
 ---
  xen/include/public/io/blkif.h |   50 
 +
  1 file changed, 50 insertions(+)

 diff --git a/xen/include/public/io/blkif.h b/xen/include/public/io/blkif.h
 index 99f0326..3d8d39f 100644
 --- a/xen/include/public/io/blkif.h
 +++ b/xen/include/public/io/blkif.h
 @@ -635,6 +635,28 @@
  #define BLKIF_OP_INDIRECT  6
  
  /*
 + * Recognized only if "feature-extra-request" is present in backend 
 xenbus info.
 + * A request with BLKIF_OP_EXTRA_FLAG indicates an extra request is 
 followed
 + * in the shared ring buffer.
 + *
 + * By this way, extra data like bio integrity payload can be transmitted 
 from
 + * frontend to backend.
 + *
 + * The 'wire' format is like:
 + *  Request 1: xen_blkif_request
 + * [Request 2: xen_blkif_extra_request](only if request 1 has 
 BLKIF_OP_EXTRA_FLAG)
 + *  Request 3: xen_blkif_request
 + *  Request 4: xen_blkif_request
 + * [Request 5: xen_blkif_extra_request](only if request 4 has 
 BLKIF_OP_EXTRA_FLAG)
 + *  ...
 + *  Request N: xen_blkif_request
 + *
 + * If a backend does not recognize BLKIF_OP_EXTRA_FLAG, it should *not* 
 create the
 + * "feature-extra-request" node!
 + */
 +#define BLKIF_OP_EXTRA_FLAG (0x80)
 +
 +/*
   * Maximum scatter/gather segments per request.
   * This is carefully chosen so that sizeof(blkif_ring_t) <= PAGE_SIZE.
   * NB. This could be 12 if the ring indexes weren't stored in the same 
 page.
 @@ -703,6 +725,34 @@ struct blkif_request_indirect {
  };
  typedef struct blkif_request_indirect blkif_request_indirect_t;
  
 +enum blkif_extra_request_type {
 +  BLKIF_EXTRA_TYPE_DIX = 1,   /* Data integrity extension 
 payload.  */
 +};
 +
 +struct bio_integrity_req {
 +  /*
 +   * Grant mapping for transmitting bio integrity payload to backend.
 +   */
 +  grant_ref_t *gref;
 +  unsigned int nr_grefs;
 +  unsigned int len;
 +};
>>>
>>> How does the payload look like? It's structure should be defined here
>>> or a reference to it's definition in case it is a standard should be
>>> given.
>>>
>>
>> The payload is also described using struct bio_vec(the same as bio).
>>
>> /*
>>  * bio integrity payload
>>  */
>> struct bio_integrity_payload {
>>  struct bio  *bip_bio;   /* parent bio */
>>
>>  struct bvec_iterbip_iter;
>>
>>  bio_end_io_t*bip_end_io;/* saved I/O completion fn */
>>
>>  unsigned short  bip_slab;   /* slab the bip came from */
>>  unsigned short  bip_vcnt;   /* # of integrity bio_vecs */
>>  unsigned short  bip_max_vcnt;   /* integrity bio_vec slots */
>>  unsigned short  bip_flags;  /* control flags */
>>
>>  struct work_struct  bip_work;   /* I/O completion */
>>
>>  struct bio_vec  *bip_vec;
>>  struct bio_vec  bip_inline_vecs[0];/* embedded bvec array */
>> };
> 
> There's no way we are going to embed such a Linux specific payload into 
> the PV block protocol. Also, I have the feeling there are a lot of fields 
> in th

Re: [Xen-devel] [PATCH v11 1/2] libxl: add support for vscsi

[Olaf Hering]

On Fri, Apr 08, Olaf Hering wrote:

> +int xlu_vscsi_get_ctrl(XLU_Config *cfg, libxl_ctx *ctx, uint32_t domid,
> +   const char *str,
> +   libxl_device_vscsictrl *ctrl,
> +   libxl_device_vscsidev *dev,
> +   libxl_device_vscsictrl **existing)

> +if (found_ctrl == -1) {
> +*existing = NULL;

> +int main_vscsiattach(int argc, char **argv)

> +libxl_device_vscsictrl ctrl, *existing = NULL;

> +existing = xmalloc(sizeof(*existing));
> +libxl_device_vscsictrl_init(existing);

This handling of 'existing' is bogus, I will rework it. Not sure what I
had in mind here...

Olaf

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

Re: [Xen-devel] [PATCH v4 00/14] x86: remove paravirt_enabled

[Borislav Petkov]

On Fri, Apr 08, 2016 at 03:14:43AM +0200, Luis R. Rodriguez wrote:
> On Wed, Apr 06, 2016 at 05:06:20PM -0700, Luis R. Rodriguez wrote:
> > Now that Andy's ASM paravirt_enabled() use is merged 
> 
> Sorry I should have provided more context, I meant that now
> that Andy's ASM paravirt_enabled() removal is merged:
> 
> This is the ASM hack that Andy removed:
> https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/fsgsbase&id=58a5aac5331388a175a42b6ed2154f0559cefb21
> 
> This puts a nail on coffin for the ASM hack:
> https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/fsgsbase&id=0dd0036f6e07f741a1356b424b84a3164b6e59cf

Ah right, that.

Thanks!

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.

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

Re: [Xen-devel] [PATCH v2] libxl: replace the usage of uuid_t with a char array

[Roger Pau Monné]

On Thu, 7 Apr 2016, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH v2] libxl: replace the usage of uuid_t with a 
> char array"):
> > A thought: maybe it is worth to have a #define LIBXL_HAVE_UNIFIED_UUID
> > in libxl.h?
> 
> This is a good idea.
> 
> > /* If this is defined, libxl_uuid is big endian 16-octet stream on all
> >  * platform. The libxl_uuid API family will handle transformation
> >  * between libxl_uuid format and OS specific format.
> >  */
> > #define LBIXL_HAVE_UNIFIED_UUID 1
> 
> But the wording here isn't, quite.  The "transformation between
> libxl_uuid format and OS specific format" is entirely hidden within
> libxl and exists only to implement the functions provided by libxl.

What about:

/*
 * LIBXL_HAVE_BYTEARRAY_UUID
 *
 * If this is defined, the internal member of libxl_uuid is defined
 * as a 16 byte array that contains the UUID in big endian format.
 * Also, the same structure layout is used across all OSes.
 */
#define LIBXL_HAVE_BYTEARRAY_UUID 1

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

Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server

[George Dunlap]

Oops -- forgot to CC the list...

On Thu, Apr 7, 2016 at 10:55 AM, George Dunlap
 wrote:
> On Thu, Apr 7, 2016 at 8:01 AM, Yu, Zhang  wrote:
 +/* For now, only support for HAP enabled hvm */
 +if ( !hap_enabled(d) )
 +goto out;
>>>
>>>
>>> So before I suggested that this be restricted to HAP because you were
>>> using p2m_memory_type_changed(), which was only implemented on EPT.
>>> But since then you've switched that code to use
>>> p2m_change_entry_type_global() instead, which is implemented by both;
>>> and you implement the type in p2m_type_to_flags().  Is there any
>>> reason to keep this restriction?
>>>
>>
>> Yes. And this is a change which was not explained clearly. Sorry.
>>
>> Reason I've chosen p2m_change_entry_type_global() instead:
>> p2m_memory_type_changed() will only trigger the resynchronization for
>> the ept memory types in resolve_misconfig(). Yet it is the p2m type we
>> wanna to be recalculated, so here comes p2m_change_entry_type_global().
>>
>> Reasons I restricting the code in HAP mode:
>> Well, I guess p2m_change_entry_type_global() was only called by HAP code
>> like hap_[en|dis]able_log_dirty() etc, which were registered during
>> hap_domain_init(). As to shadow mode, it is sh_[en|dis]able_log_dirty(),
>> which do not use p2m_change_entry_type_global().
>
> Oh, right -- yes, there's an ASSERT(hap_enabled()) right at the top of
> p2m_pt_change_entry_type_global().
>
> Yes, if that functionality is not already implemented for shadow,
> there's no need for you to implement it; and restricting it to
> HAP-only is the obvious solution.
>
 +/*
 + * Each time we map/unmap an ioreq server to/from p2m_ioreq_server,
 + * we mark the p2m table to be recalculated, so that gfns which were
 + * previously marked with p2m_ioreq_server can be resynced.
 + */
 +p2m_change_entry_type_global(d, p2m_ioreq_server, p2m_ram_rw);
>>>
>>>
>>> This comment doesn't seem to be accurate (or if it is it's a bit
>>> confusing).  Would it be more accurate to say something like the
>>> following:
>>>
>>> "Each time we map / unmap in ioreq server to/from p2m_ioreq_server, we
>>> reset all memory currently marked p2m_ioreq_server to p2m_ram_rw."
>>>
>> Well, I agree this comment is not quite accurate. Like you said in your
>> comment, the purpose here, calling p2m_change_entry_type_global() is to
>> "reset all memory currently marked p2m_ioreq_server to p2m_ram_rw". But
>> the recalculation is asynchronous. So how about:
>>
>> "Each time we map/unmap an ioreq server to/from p2m_ioreq_server, we
>> mark the p2m table to be recalculated, so all memory currently marked
>> p2m_ioreq_server can be reset to p2m_ram_rw later."?
>
> I think we're emphasizing different things -- I'm emphasizing what the
> change will be, and you're emphasizing when the change will happen.
> :-)
>
> I think from the point of view of someone reading this code, it
> doesn't matter when the physical p2m entries get updated; *logically*
> they are updated now, since from this call onward anything that reads
> the p2m table will get the new type.  The fact that we do it lazily is
> an implementation detail -- we could change the function behind the
> scenes to do it all at once, and the semantics would be the same (it
> would just cause a long change all at once).
>
> If you really want to include when the change will happen, what about this:
>
> "Each time we map / unmap in ioreq server to/from p2m_ioreq_server, we
> reset all memory currently marked p2m_ioreq_server to p2m_ram_rw.
> (This happens lazily as the p2m entries are accessed.)"
>
> BTW, I think this also means that for the interface at the moment, you
> can't change the kinds of accesses you want to intercept very easily
> -- if you want to change from intercepting only writes to intercepting
> both reads and writes, you have to detach from the ioreq_server type
> completely (which will make all your currently-marked ioreq_server
> pages go back to ram_rw), then attach again, and re-mark all the pages
> you were watching.
>
> Which is perhaps not perfect, but I suppose it will do.  It should
> even be possible to change this in the future -- ioreq servers that
> want to change the access mode can try just changing it directly, and
> if they get -EBUSY, do it the hard way.
>
> (Just to be clear, I'm thinking out loud here in the last two
> paragraphs -- no action required unless you feel like it.)
>
>>> But of course that raises another question: is there ever any risk
>>> that an ioreq server will change some other ram type (say, p2m_ram_ro)
>>> to p2m_ioreq_server, and then have it changed back to p2m_ram_rw when
>>> it detaches?
>>>
>> Good question. And unfortunately, yes there is. :)
>> Maybe we should insist only p2m_ram_rw pages can be changed to
>> p2m_ioreq_server, and vice versa.
>
> Well I think if you only allow p2m_ram_rw pages to be changed to
> p2m_ioreq_server, that should be sufficient.  If t

Re: [Xen-devel] [PATCH v3 2/5] xentrace: Memory/Page Mapping support for DOMID_XEN on ARM

[Julien Grall]

(CC Stefano's new e-mail address)

Hello Benjamin,

On 04/04/16 19:48, Benjamin Sanda wrote:

  xen/arch/arm/mm.c  |  3 ++-
  xen/arch/arm/p2m.c | 35 +++
  2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 81f9e2e..19d6c2c 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1099,7 +1099,8 @@ int xenmem_add_to_physmap_one(
  {
  struct domain *od;
  p2m_type_t p2mt;
-od = rcu_lock_domain_by_any_id(foreign_domid);
+od = get_pg_owner(foreign_domid);
+


Please also replace the call to rcu_unlock_domain by put_pg_owner to 
stay consistent.



  if ( od == NULL )
  return -ESRCH;

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index a2a9c4b..a99b670 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -227,11 +227,38 @@ paddr_t p2m_lookup(struct domain *d, paddr_t paddr, 
p2m_type_t *t)
  {
  paddr_t ret;
  struct p2m_domain *p2m = &d->arch.p2m;
+struct page_info *page;
+unsigned long mfn;

-spin_lock(&p2m->lock);
-ret = __p2m_lookup(d, paddr, t);
-spin_unlock(&p2m->lock);
-
+/*
+* DOMID_XEN is considered a PV guest on x86 (i.e MFN == GFN), but
+* on ARM there is no such concept. Thus requests to DOMID_XEN
+* on ARM use a MFN address directly and do not need translation
+* from PFN.
+*/


The coding style for the comment should be:

/*
 * FOo
 * Bar
 */


+if(DOMID_XEN != d->domain_id)


if ( ... )


+{
+spin_lock(&p2m->lock);
+ret = __p2m_lookup(d, paddr, t);
+spin_unlock(&p2m->lock);
+}
+else
+{
+/* retrieve the page to determine read/write or read only mapping */
+mfn = paddr >> PAGE_SHIFT;
+if (mfn_valid(mfn))
+{
+page = mfn_to_page(mfn);
+*t = (page->u.inuse.type_info == PGT_writable_page ?
+p2m_ram_rw : p2m_ram_ro);


Unfortunately, xenmem_add_to_physmap_one will ignore the return type and 
will always map using the type p2m_map_foreign. I would introduce

a new type p2m_map_foreign_ro to allow read-only foreign mapping.

I've looked at the x86 code (p2m_add_foreign) and I haven't been able to 
find how the page will be mapped read-only in the guest P2M. 
get_page_from_gfn will always return p2m_raw_rw for DOMID_XEN as it's a 
non translated domain.


Andrew and Jan, do you know how this is supposed to work when xentrace 
is used in a HVM domain? Does x86 Xen always mapped Read-Write the page?



+}
+else
+{
+*t = p2m_invalid;
+}


The brackets are not necessary for a single statement.


+ret = paddr;
+}
+
  return ret;
  }




Regards,

--
Julien Grall

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

[Xen-devel] [distros-debian-squeeze test] 44277: trouble: blocked/broken

[Platform Team regression test user]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-pvops 3 host-install(3) broken REGR. vs. 44250
 build-armhf-pvops 3 host-install(3) broken REGR. vs. 44250
 build-i386-pvops  3 host-install(3) broken REGR. vs. 44250
 build-i3863 host-install(3) broken REGR. vs. 44250
 build-amd64   3 host-install(3) broken REGR. vs. 44250
 build-armhf   3 host-install(3) broken REGR. vs. 44250

Tests which did not succeed, but are not blocking:
 test-amd64-i386-amd64-squeeze-netboot-pygrub  1 build-check(1) blocked n/a
 test-amd64-amd64-amd64-squeeze-netboot-pygrub  1 build-check(1)blocked n/a
 test-amd64-i386-i386-squeeze-netboot-pygrub  1 build-check(1)  blocked n/a
 test-amd64-amd64-i386-squeeze-netboot-pygrub  1 build-check(1) blocked n/a

baseline version:
 flight   44250

jobs:
 build-amd64  broken  
 build-armhf  broken  
 build-i386   broken  
 build-amd64-pvopsbroken  
 build-armhf-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-amd64-squeeze-netboot-pygrubblocked 
 test-amd64-i386-amd64-squeeze-netboot-pygrub blocked 
 test-amd64-amd64-i386-squeeze-netboot-pygrub blocked 
 test-amd64-i386-i386-squeeze-netboot-pygrub  blocked 



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] [PATCH v3 3/5] xentrace: Timestamp support for ARM platform

[Julien Grall]

Hello Benjamin,

On 04/04/16 19:48, Benjamin Sanda wrote:

Moved get_cycles() to time.c and modified to return the core timestamp
tick count for use by the trace buffer timestamping routines in
xentrace. get_cycles() was moved to the C file to avoid including the
register specific header file in time.h and to commonize it with the
get_s_time() function. Also defined cycles_t as uint64_t to simplify
casting.


I'm not sure what you mean by "simplify casting".

The type cycles_t is not correctly defined for ARM32 because "unsigned 
long" is always 32-bits. However, the physical count register (CNTPCT) 
is always 64-bits. So the number of cycles would have been truncated.


The rest of the patch looks good to me.


get_s_time() was also modified to now use the updated get_cycles() to
retrieve the tick count instead of directly reading it.

Signed-off-by: Benjamin Sanda 

---
Changed since v2:
   * Combined v2 patches 7 and 6 into one patch in v3. No code change.

---
Changed since v1:
   * Moved get_cycles() to time.c
   * Added function prototype for get_cycles()
---
  xen/arch/arm/time.c|  9 -
  xen/include/asm-arm/time.h | 11 +--
  2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 7dae28b..9aface3 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -192,10 +192,17 @@ int __init init_xen_time(void)
  /* Return number of nanoseconds since boot */
  s_time_t get_s_time(void)
  {
-uint64_t ticks = READ_SYSREG64(CNTPCT_EL0) - boot_count;
+cycles_t ticks = get_cycles();
  return ticks_to_ns(ticks);
  }

+/* Return the number of ticks since boot */
+cycles_t get_cycles(void)
+{
+/* return raw tick count of main timer */
+return READ_SYSREG64(CNTPCT_EL0) - boot_count;
+}
+
  /* Set the timer to wake us up at a particular time.
   * Timeout is a Xen system time (nanoseconds since boot); 0 disables the 
timer.
   * Returns 1 on success; 0 if the timeout is too soon or is in the past. */
diff --git a/xen/include/asm-arm/time.h b/xen/include/asm-arm/time.h
index 5b9a31d..b57f4c1 100644
--- a/xen/include/asm-arm/time.h
+++ b/xen/include/asm-arm/time.h
@@ -5,12 +5,8 @@
  DT_MATCH_COMPATIBLE("arm,armv7-timer"), \
  DT_MATCH_COMPATIBLE("arm,armv8-timer")

-typedef unsigned long cycles_t;
-
-static inline cycles_t get_cycles (void)
-{
-return 0;
-}
+/* Tick count type */
+typedef uint64_t cycles_t;

  /* List of timer's IRQ */
  enum timer_ppi
@@ -37,6 +33,9 @@ extern void init_timer_interrupt(void);
  /* Counter value at boot time */
  extern uint64_t boot_count;

+/* Get raw system tick count */
+cycles_t get_cycles(void);
+
  extern s_time_t ticks_to_ns(uint64_t ticks);
  extern uint64_t ns_to_ticks(s_time_t ns);




Regards,

--
Julien Grall

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

[Xen-devel] [distros-debian-wheezy test] 44278: tolerable trouble: blocked/broken

[Platform Team regression test user]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 build-amd64-pvops 3 host-install(3)  broken like 44254
 build-armhf-pvops 3 host-install(3)  broken like 44254
 build-i386-pvops  3 host-install(3)  broken like 44254
 build-i3863 host-install(3)  broken like 44254
 build-amd64   3 host-install(3)  broken like 44254
 build-armhf   3 host-install(3)  broken like 44254

Tests which did not succeed, but are not blocking:
 test-amd64-i386-i386-wheezy-netboot-pvgrub  1 build-check(1)   blocked n/a
 test-amd64-amd64-i386-wheezy-netboot-pygrub  1 build-check(1)  blocked n/a
 test-amd64-amd64-amd64-wheezy-netboot-pvgrub  1 build-check(1) blocked n/a
 test-amd64-i386-amd64-wheezy-netboot-pygrub  1 build-check(1)  blocked n/a

baseline version:
 flight   44254

jobs:
 build-amd64  broken  
 build-armhf  broken  
 build-i386   broken  
 build-amd64-pvopsbroken  
 build-armhf-pvopsbroken  
 build-i386-pvops broken  
 test-amd64-amd64-amd64-wheezy-netboot-pvgrub blocked 
 test-amd64-i386-i386-wheezy-netboot-pvgrub   blocked 
 test-amd64-i386-amd64-wheezy-netboot-pygrub  blocked 
 test-amd64-amd64-i386-wheezy-netboot-pygrub  blocked 



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] [PATCH] tools: handle xl migrate --debug in legacy stream

[Wei Liu]

On Thu, Apr 07, 2016 at 06:31:01PM +0200, Olaf Hering wrote:
> Doing a 'xl migrate --debug domU host' on xen-4.5 adds a
> XC_SAVE_ID_ENABLE_VERIFY_MODE marker, which is not handled.
> Since using --debug is valid usage, handle it by logging the fact
> instead of aborting the migration.
> 
> Signed-off-by: Olaf Hering 
> Cc: Ian Jackson 
> Cc: Wei Liu 

Acked-by: Wei Liu 

> ---
>  tools/python/scripts/convert-legacy-stream | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/python/scripts/convert-legacy-stream 
> b/tools/python/scripts/convert-legacy-stream
> index 41fee10..5f80f13 100755
> --- a/tools/python/scripts/convert-legacy-stream
> +++ b/tools/python/scripts/convert-legacy-stream
> @@ -389,8 +389,7 @@ def read_chunks(vm):
>  write_page_data(pfns, pages)
>  
>  elif marker == legacy.CHUNK_enable_verify_mode:
> -# For debugging purposes only.  Will not be seen in real 
> migration
> -raise RuntimeError("Unable to convert a debug stream")
> +info("This is a debug stream")
>  
>  elif marker == legacy.CHUNK_vcpu_info:
>  max_id, = unpack_exact("i")

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

Re: [Xen-devel] [PATCH v3 4/5] xentrace: Trace Buffer Initialization on ARM

[Julien Grall]

Hello Benjamin,

On 04/04/16 19:48, Benjamin Sanda wrote:

Added call to init_trace_bufs() to initialize the trace buffers for
use by xentrace.

Signed-off-by: Benjamin Sanda 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server

[George Dunlap]

On 08/04/16 11:10, Yu, Zhang wrote:
[snip]
> BTW, I noticed your reply has not be CCed to mailing list, and I also
> wonder if we should raise this last question in community?

Oops -- that was a mistake on my part.  :-)  I appreciate the
discretion; just so you know in the future, if I'm purposely changing
the CC list (removing xen-devel and/or adding extra people), I'll almost
always say so at the top of the mail.

>> And then of course there's the p2m_ioreq_server -> p2m_ram_logdirty
>> transition -- I assume that live migration is incompatible with this
>> functionality?  Is there anything that prevents a live migration from
>> being started when there are outstanding p2m_ioreq_server entries?
>>
> 
> Another good question, and the answer is unfortunately yes. :-)
> 
> If live migration happens during the normal emulation process, entries
> marked with p2m_ioreq_server will be changed to p2m_log_dirty in
> resolve_misconfig(), and later write operations will change them to
> p2m_ram_rw, thereafter these pages can not be forwarded to device model.
> From this point of view, this functionality is incompatible with live
> migration.
> 
> But for XenGT, I think this is acceptable, because, if live migration
> is to be supported in the future, intervention from backend device
> model will be necessary. At that time, we can guarantee from the device
> model side that there's no outdated p2m_ioreq_server entries, hence no
> need to reset the p2m type back to p2m_ram_rw(and do not include
> p2m_ioreq_server in the P2M_CHANGEABLE_TYPES). By "outdated", I mean
> when an ioreq server is detached from p2m_ioreq_server, or before an
> ioreq server is attached to this type, entries marked with
> p2m_ioreq_server should be regarded as outdated.
> 
> Is this acceptible to you? Any suggestions?

So the question is, as of this series, what happens if someone tries to
initiate a live migration while there are outstanding p2m_ioreq_server
entries?

If the answer is "the ioreq server suddenly loses all control of the
memory", that's something that needs to be changed.

If the answer is, "everything just works", that's perfect.

If the answer is, "Before logdirty mode is set, the ioreq server has the
opportunity to detach, removing the p2m_ioreq_server entries, and
operating without that functionality", that's good too.

If the answer is, "the live migration request fails and the guest
continues to run", that's also acceptable.  If you want this series to
be checked in today (the last day for 4.7), this is probably your best bet.

 -George



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

Re: [Xen-devel] [PATCH v11 2/2] Scripts to create and delete xen-scsiback nodes in Linux target framework

[Wei Liu]

On Fri, Apr 08, 2016 at 07:01:31AM +, Olaf Hering wrote:
> 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 

I think it is the right thing to do to upstream all management scripts,
so:

Acked-by: Wei Liu 

Though I'm still not quite sure how users are supposed to use these
scripts because your patch to libxl doesn't seem to use them directly.

A follow-up patch to provide a document on setting up vscsi with libxl
would be much appreciated. The document can be placed under docs/misc
directory.

Wei.

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

Re: [Xen-devel] [PATCH v11 2/2] Scripts to create and delete xen-scsiback nodes in Linux target framework

[Olaf Hering]

On Fri, Apr 08, Wei Liu wrote:

> On Fri, Apr 08, 2016 at 07:01:31AM +, Olaf Hering wrote:
> > 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 
> 
> I think it is the right thing to do to upstream all management scripts,
> so:
> 
> Acked-by: Wei Liu 

Thanks, but:

> Though I'm still not quite sure how users are supposed to use these
> scripts because your patch to libxl doesn't seem to use them directly.
> 
> A follow-up patch to provide a document on setting up vscsi with libxl
> would be much appreciated. The document can be placed under docs/misc
> directory.

These two scripts create just a bunch of scsi devices in tmpfs so my
libxl code has something to work with. I should document all this in the
wiki, soon. There is already an outdated PVSCSI page.

Olaf

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

[Xen-devel] [linux-3.18 test] 89339: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 
86513
 test-amd64-amd64-xl-qemuu-win7-amd64 20 leak-check/check  fail REGR. vs. 86513
 test-armhf-armhf-libvirt-qcow2  9 debian-di-install   fail REGR. vs. 86513
 build-armhf   4 host-build-prep  fail in 89247 REGR. vs. 86513
 test-amd64-i386-xl-qemut-win7-amd64 20 leak-check/check fail in 89247 REGR. 
vs. 86513

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 15 guest-localmigrate/x10 fail 
in 89247 pass in 89339
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
in 89247 pass in 89339
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail in 89247 pass in 89339
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop   fail pass in 89247

Regressions which are regarded as allowable (not blocking):
 build-amd64-rumpuserxen   6 xen-buildfail   like 86513
 build-i386-rumpuserxen6 xen-buildfail   like 86513
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 86513
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 86513

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

Re: [Xen-devel] [PATCH v11 1/2] libxl: add support for vscsi

[Wei Liu]

On Fri, Apr 08, 2016 at 07:01:30AM +, Olaf Hering wrote:
> 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 

This patch looks quite close now.

I have one comment below, which needs to be addressed properly.

Also there are some higher level questions that are not settled in the
last iteration, I will copy and paste it here:

> > I'm not sure that this sysfs parsing ought to be in xl rather than
> > libxl.  Also, this is Linux-specific code.  So it needs to be made
> > conditional somehow.
> 
> I think this depends on how libvirt is supposed to interact with libxl
> here. Right now libvirt is rather dumb, it supports just the
> host:channel:target:lun notation for a backend. My patch for it (which I
> have to rebase now) creates a string "pdev,vdev" and calls
> xlu_vscsi_get_host.  I will rebase the libvirt patch and resend it as
> well.
> 

I guess the answer is that we should leave the parsing inside libxl(u)
so that libvirt can benefit from it.

The bottom line is that client can opt out of this set of APIs if they
want to.

> > It seems to me that the contents of xlu__vscsi_target should be much
> > closer to vscsi_pdev_type (unless I have misunderstood).
> 
> Well, if its important where that struct is supposed to be in the file,
> I can move it down.
> 

I don't think the location matters that much. You might have
misunderstood.

> > Perhaps the libxl_types.idl API needs to change.  In general, the
> > libxl API ought to be close enough in semantics to the xl config API
> > that the correspondence is obvious.  I don't think that's the case
> > here.
> 
> I'm not sure what this paragraph means.

What Ian wanted was that we need clear correlation of xl configuration
syntax with libxl API fields. For example, for a FOO device

   xl.cfg: FOO = [ "bar=baz" ]

   libxl_types.idl:

libxl_device_FOO = Struct("device_foo", [
  ("bar", some_type)
  ...
  ])

However, I don't think that can be easily achieved in the case of pvscsi
because we need to support both legacy Xen-classic kernel and new PVOPS
kernel. Olaf, please correct me if I'm wrong.


[...]
> +
> +static bool vscsi_fill_ctrl(libxl__gc *gc,
> +xs_transaction_t t,
> +const char *fe_path,
> +const char *dir,
> +libxl_device_vscsictrl *ctrl)
> +{
> +libxl_device_vscsidev dev;
> +char *tmp, *be_path, *devs_path;
> +char **dev_dirs;
> +unsigned int ndev_dirs, dev_dir;
> +bool ok;
> +
> +ctrl->devid = atoi(dir);
> +
> +be_path = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend", fe_path, 
> dir));
> +if (!be_path)
> +goto out;
> +
> +tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend-id", fe_path, dir));
> +if (!tmp)
> +goto out;
> +ctrl->backend_domid = atoi(tmp);

Please sanitise input coming from frontend.  You need to check if the
backend domid and frontend domid make sense.

Wei.

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

[Xen-devel] x86 patch ping

[Jan Beulich]

Andrew,

could I please get acks or otherwise on

http://lists.xen.org/archives/html/xen-devel/2016-03/msg01469.html

http://lists.xen.org/archives/html/xen-devel/2016-03/msg02167.html
(with the 1st patch having gone in already)

http://lists.xen.org/archives/html/xen-devel/2016-04/msg00040.html

There are also
http://lists.xen.org/archives/html/xen-devel/2016-03/msg03746.html
http://lists.xen.org/archives/html/xen-devel/2016-03/msg03747.html
but I guess I'll put them in without further acks, considering they
are simply ports from Linux.

Thanks, Jan


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

Re: [Xen-devel] x86 patch ping

[Andrew Cooper]

On 08/04/16 13:10, Jan Beulich wrote:
> Andrew,
>
> could I please get acks or otherwise on
>
> http://lists.xen.org/archives/html/xen-devel/2016-03/msg01469.html
>
> http://lists.xen.org/archives/html/xen-devel/2016-03/msg02167.html
> (with the 1st patch having gone in already)
>
> http://lists.xen.org/archives/html/xen-devel/2016-04/msg00040.html
>
> There are also
> http://lists.xen.org/archives/html/xen-devel/2016-03/msg03746.html
> http://lists.xen.org/archives/html/xen-devel/2016-03/msg03747.html
> but I guess I'll put them in without further acks, considering they
> are simply ports from Linux.

These last 3 links ("x86/vMSI-X: fix qword write" and the two from
linux) are straightforward.

Acked-by: Andrew Cooper 

The first I will need to do a closer review of.

The SMEP/SMAP series is still very concerning.  I need to follow up on
the performance testing, but it currently looks like no real improvement
on the 40-70% performance hit for 32bit PV guests.

~Andrew


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

[Xen-devel] Xen.git freezes today

[Wei Liu]

Hi all

Committers, please commit all pending feature patch series today. We
shall look into stabilising the tree next week and arrange RCs as soon
as possible.

To committers and contributors, please CC me on patches that wish to go
into staging, starting next week.

Thanks
Wei.

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

Re: [Xen-devel] [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Boris Ostrovsky]

On 04/08/2016 02:29 AM, Luis R. Rodriguez wrote:

On Thu, Apr 7, 2016 at 10:18 PM, Juergen Gross  wrote:

On 08/04/16 02:32, Luis R. Rodriguez wrote:

On Thu, Apr 07, 2016 at 08:55:54AM -0400, Boris Ostrovsky wrote:

On 04/06/2016 08:06 PM, Luis R. Rodriguez wrote:

We have 4 types of x86 platforms that disable RTC:

   * Intel MID
   * Lguest - uses paravirt
   * Xen dom-U - uses paravirt
   * x86 on legacy systems annotated with an ACPI legacy flag

We can consolidate all of these into a platform specific legacy
quirk set early in boot through i386_start_kernel() and through
x86_64_start_reservations(). This deals with the RTC quirks which
we can rely on through the hardware subarch, the ACPI check can
be dealt with separately.

v2: split the subarch check from the ACPI check, clarify
 on the ACPI change commit log why ordering works

Suggested-by: Ingo Molnar 
Signed-off-by: Luis R. Rodriguez 

<-- snip -->


diff --git a/arch/x86/kernel/platform-quirks.c 
b/arch/x86/kernel/platform-quirks.c
new file mode 100644
index ..1b114ac5996f
--- /dev/null
+++ b/arch/x86/kernel/platform-quirks.c
@@ -0,0 +1,18 @@
+#include 
+#include 
+
+#include 
+#include 
+
+void __init x86_early_init_platform_quirks(void)
+{
+   x86_platform.legacy.rtc = 1;
+
+   switch (boot_params.hdr.hardware_subarch) {
+   case X86_SUBARCH_XEN:
+   case X86_SUBARCH_LGUEST:
+   case X86_SUBARCH_INTEL_MID:
+   x86_platform.legacy.rtc = 0;
+   break;
+   }
+}

What about Xen dom0 (aka initial domain)?

Indeed, thanks for catching this, the hunk below removes the re-enablement of
the the RTC for dom0:


--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1192,7 +1192,6 @@ static const struct pv_info xen_info __initconst = {
  #ifdef CONFIG_X86_64
 .extra_user_64bit_cs = FLAT_USER_CS64,
  #endif
-   .features = 0,
 .name = "Xen",
  };
@@ -1525,8 +1524,6 @@ asmlinkage __visible void __init xen_start_kernel(void)
 /* Install Xen paravirt ops */
 pv_info = xen_info;
-   if (xen_initial_domain())
-   pv_info.features |= PV_SUPPORTED_RTC;
 pv_init_ops = xen_init_ops;
 if (!xen_pvh_domain()) {
 pv_cpu_ops = xen_cpu_ops;

This should then break dom0 unless of course you have the respective next
patch applied and that disabled the RTC due to an ACPI setting on your
platform. Juergen, can you check to see if that was the case for your
testing platform on dom0 ?

Are you sure it would break?

No, suspected that it should though.


Wouldn't it just fall back to another
clock source, e.g. hpet?

I suppose so.


I looked into my test system: seems as if add_rtc_cmos() is returning
before the .legacy.rtc test.

OK thanks...


It works because the clock must have been discovered by ACPI prior to 
add_rtc_cmos() call. It's PNP0b00 object, I believe. The rest of the 
routine is to handle the case when RTC is not found in ACPI tables for 
whatever reasons (I think).


That's why we added paravirt_has(RTC) --- dom0 should be able to handle 
such cases, just like bare metal.


-boris

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

Re: [Xen-devel] Running Xen on Nvidia Jetson-TK1

[Ian Campbell]

On Fri, 2016-04-08 at 11:10 +0100, Julien Grall wrote:

> > In your patch for *Hacky* support for Jetsok-TK1 you said that you
> > were able to run guests on
> > Jetson-tk1 board with Xen. Can I know which kernel version you used as
> > dom0 (and possibly domU guests)?

I'm afraid I don't remember. It would probably either have been some
current-ish upstream kernel from the time, or possibly a Debian kernel
from around the time. So maybe something 3.16-ish?

Ian.

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

[Xen-devel] [PATCH V2] x86/monitor: Disallow setting mem_access_emulate_each_rep when vm_event is NULL

[Razvan Cojocaru]

It is meaningless (and potentially dangerous - see hvmemul_virtual_to_linear())
to set mem_access_emulate_each_rep before xc_monitor_enable() (which allocates
vcpu->arch.vm_event) has been called, so return an error from the
XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case.

Signed-off-by: Razvan Cojocaru 

---
Changes since V1:
 - Fixed the if() condition.
 - Introduced an rc return variable to simplify the code.
---
 xen/include/asm-x86/monitor.h | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 0954b59..a66760d 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -32,19 +32,29 @@
 static inline
 int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
+int rc = 0;
+
 switch ( mop->op )
 {
 case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
 domain_pause(d);
-d->arch.mem_access_emulate_each_rep = !!mop->event;
+/*
+ * Enabling mem_access_emulate_each_rep without a vm_event subscriber
+ * is meaningless.
+ */
+if ( d->vcpu && d->vcpu[0]->arch.vm_event )
+d->arch.mem_access_emulate_each_rep = !!mop->event;
+else
+rc = -EINVAL;
+
 domain_unpause(d);
 break;
 
 default:
-return -EOPNOTSUPP;
+rc = -EOPNOTSUPP;
 }
 
-return 0;
+return rc;
 }
 
 int arch_monitor_domctl_event(struct domain *d,
-- 
1.9.1


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

Re: [Xen-devel] [PATCH v4 04/14] x86/rtc: replace paravirt rtc check with platform legacy quirk

[Boris Ostrovsky]

On 04/08/2016 03:59 AM, Juergen Gross wrote:

On 08/04/16 09:36, Luis R. Rodriguez wrote:

On Fri, Apr 8, 2016 at 12:13 AM, Juergen Gross  wrote:

On 08/04/16 08:56, Luis R. Rodriguez wrote:

On Thu, Apr 7, 2016 at 11:38 PM, Juergen Gross  wrote:

Okay. Another idea (not sure whether this is really a good one):

Add X86_SUBARCH_XEN_DOM0. As hardware_subarch is 32 bits wide I don't
think the number of subarchs is a scarce resource. :-)

This would mean bumping the x86 boot protocol, we shouldn't take that
lightly, but given that in this case the new subarch would really only
be set by the kernel (or future loaders for perhaps HVMLite) I'd think
this is not such an intrusive alternative.

I think adding an own subarch for dom0 isn't that bad. It really is
different from domU as dom0 has per default access to the real hardware
(or at least to most of it).


Can we do this (overwrite quirks) in x86_init_ops.arch_setup? I'd really 
like to avoid adding a what essentially is a sub-subarch.


-boris

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

Re: [Xen-devel] [PATCH v3 2/6] xl: improve exit codes of save/restore and migration functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:23:39AM +0200, Dario Faggioli wrote:
> From: Harmandeep Kaur 
> 
> by making them more consistent with other examples in xl.
> 
> Macros CHK_ERRNOVAL, CHK_SYSCALL, MUST are also updated.
> 
> Signed-off-by: Harmandeep Kaur 
> Signed-off-by: Dario Faggioli 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v3 5/6] xl: make return type of create_domain() more consistent.

[Wei Liu]

On Fri, Apr 08, 2016 at 04:24:07AM +0200, Dario Faggioli wrote:
> create_domain() is of uint32_t return type, because on
> success it returns the domid of the new domain, and
> uint32_t is what we typically use for domid-s.
> 
> However, on failure, it returns ERROR_FAIL or ERROR_INVAL,
> which are -3 and -6. Callers assign the return value to an
> 'int rc' variable and then check for '(rc < 0)'.
> 
> Although things work, and no tool (compiler, Coverity, ecc.)
> is complaining, using 'int' as return type seems better.
> 
> Signed-off-by: Dario Faggioli 

Acked-by: Wei Liu 

> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: Andrew Cooper 
> ---
>  tools/libxl/xl_cmdimpl.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 823cb46..d2ad8c8 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2777,7 +2777,7 @@ static void 
> evdisable_disk_ejects(libxl_evgen_disk_eject **diskws,
>  }
>  }
>  
> -static uint32_t create_domain(struct domain_create *dom_info)
> +static int create_domain(struct domain_create *dom_info)
>  {
>  uint32_t domid = INVALID_DOMID;
>  
> 

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

Re: [Xen-devel] [PATCH v3 6/6] xl: improve exit codes of domain creation related functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:24:17AM +0200, Dario Faggioli wrote:
> by making them more consistent with other examples in xl.
> 
> Signed-off-by: Dario Faggioli 
> Signed-off-by: Harmandeep Kaur 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> ---
> v3: In create_domain(), only deal with exit codes, internal returns
> need more work than what was being done in the original patch.
> Shorten changelog.
> 
> v2: Add create_domain()
> Remove main_sharing()
> ---
>  tools/libxl/xl_cmdimpl.c |   32 
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index d2ad8c8..31e2f30 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2964,7 +2964,7 @@ static int create_domain(struct domain_create *dom_info)
>  if (!json) {
>  fprintf(stderr,
>  "Failed to convert domain configuration to JSON\n");
> -exit(1);
> +exit(EXIT_FAILURE);
>  }
>  fputs(json, cfg_print_fh);
>  free(json);
> @@ -3212,7 +3212,7 @@ out:
>   * already happened in the parent.
>   */
>  if ( daemonize && !need_daemon )
> -exit(ret);
> +exit(EXIT_SUCCESS);

This is not entirely correct: ret can be non-zero here if I'm not
mistaken. Whether it is useful to exit with that value, I don't know.

Wei.

 

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

Re: [Xen-devel] [PATCH v3 1/6] xl: improve return and exit codes of memory related functions

[Wei Liu]

On Fri, Apr 08, 2016 at 11:58:02AM +0200, Dario Faggioli wrote:
[...]
> commit 43586222084638ec98b693702132f3412a63ddda
> Author: Harmandeep Kaur 
> Date:   Thu Apr 7 12:38:09 2016 +0200
> 
> xl: improve return and exit codes of memory related functions
> 
> by making them more consistent with other examples in xl.
> 
> While there, make freemem() of boolean return type, which
> looks more natural, and add comment explaining why
> parse_mem_size_kb() needs to diverge from the pattern.
> 
> Signed-off-by: Harmandeep Kaur 
> Signed-off-by: Dario Faggioli 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> Cc: George Dunlap 
> ---

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v3 4/6] xl : improve exit codes of debug related functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:23:59AM +0200, Dario Faggioli wrote:
> From: Harmandeep Kaur 
> 
> by making them more consistent with other examples in xl.
> 
> Signed-off-by: Harmandeep Kaur 
> Signed-off-by: Dario Faggioli 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v3 3/6] xl: improve exit codes of some of the domain handling functions

[Wei Liu]

On Fri, Apr 08, 2016 at 04:23:50AM +0200, Dario Faggioli wrote:
> From: Harmandeep Kaur 
> 
> by making them more consistent with other examples in xl.
> 
> Affected functions are the ones related to console, vnc,
> dump, destroy, shutdown, list, domid and domname.
> 
> Signed-off-by: Harmandeep Kaur 
> Signed-off-by: Dario Fagggioli 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v3 04/11] xen: sched: close potential races when switching scheduler to CPUs

[George Dunlap]

On 08/04/16 02:23, Dario Faggioli wrote:
> In short, the point is making sure that the actual switch
> of scheduler and the remapping of the scheduler's runqueue
> lock occur in the same critical section, protected by the
> "old" scheduler's lock (and not, e.g., in the free_pdata
> hook, as it is now for Credit2 and RTDS).
> 
> Not doing  so, is (at least) racy. In fact, for instance,
> if we switch cpu X from, Credit2 to Credit, we do:
> 
>  schedule_cpu_switch(x, csched2 --> csched):
>//scheduler[x] is csched2
>//schedule_lock[x] is csched2_lock
>csched_alloc_pdata(x)
>csched_init_pdata(x)
>pcpu_schedule_lock(x) > takes csched2_lock
>scheduler[X] = csched
>pcpu_schedule_unlock(x) --> unlocks csched2_lock
>[1]
>csched2_free_pdata(x)
>  pcpu_schedule_lock(x) --> takes csched2_lock
>  schedule_lock[x] = csched_lock
>  spin_unlock(csched2_lock)
> 
> While, if we switch cpu X from, Credit to Credit2, we do:
> 
>  schedule_cpu_switch(X, csched --> csched2):
>//scheduler[x] is csched
>//schedule_lock[x] is csched_lock
>csched2_alloc_pdata(x)
>csched2_init_pdata(x)
>  pcpu_schedule_lock(x) --> takes csched_lock
>  schedule_lock[x] = csched2_lock
>  spin_unlock(csched_lock)
>[2]
>pcpu_schedule_lock(x) > takes csched2_lock
>scheduler[X] = csched2
>pcpu_schedule_unlock(x) --> unlocks csched2_lock
>csched_free_pdata(x)
> 
> And if we switch cpu X from RTDS to Credit2, we do:
> 
>  schedule_cpu_switch(X, RTDS --> csched2):
>//scheduler[x] is rtds
>//schedule_lock[x] is rtds_lock
>csched2_alloc_pdata(x)
>csched2_init_pdata(x)
>  pcpu_schedule_lock(x) --> takes rtds_lock
>  schedule_lock[x] = csched2_lock
>  spin_unlock(rtds_lock)
>pcpu_schedule_lock(x) > takes csched2_lock
>scheduler[x] = csched2
>pcpu_schedule_unlock(x) --> unlocks csched2_lock
>rtds_free_pdata(x)
>  spin_lock(rtds_lock)
>  ASSERT(schedule_lock[x] == rtds_lock) [3]
>  schedule_lock[x] = DEFAULT_SCHEDULE_LOCK [4]
>  spin_unlock(rtds_lock)
> 
> So, the first problem is that, if anything related to
> scheduling, and involving CPU, happens at [1] or [2], we:
>  - take csched2_lock,
>  - operate on Credit1 functions and data structures,
> which is no good!
> 
> The second problem is that the ASSERT at [3] triggers, and
> the third that at [4], we screw up the lock remapping we've
> done for ourself in csched2_init_pdata()!
> 
> The first problem arises because there is a window during
> which the lock is already the new one, but the scheduler is
> still the old one. The other two, becase we let schedulers
> mess with the lock (re)mapping done by others.
> 
> This patch, therefore, introduces a new hook in the scheduler
> interface, called switch_sched, meant at being used when
> switching scheduler on a CPU, and implements it for the
> various schedulers, so that things are done in the proper
> order and under the protection of the best suited (set of)
> lock(s). It is necessary to add the hook (as compared to
> keep doing things in generic code), because different
> schedulers may have different locking schemes.
> 
> Signed-off-by: Dario Faggioli 

Thanks!

Reviewed-by: George Dunlap 


> ---
> Cc: George Dunlap 
> Cc: Josh Whitehead 
> Cc: Robert VanVossen 
> Cc: Meng Xu 
> Cc: Tianyang Chen 
> ---
> Changes from v2:
>  - the hook is implemented in ARINC653 too, as necessary and
>as requested during review.
> 
> Changes from v1:
> 
> new patch, basically, coming from squashing what were
> 4 patches in v1. In any case, with respect to those 4
> patches:
>  - runqueue lock is back being taken in schedule_cpu_switch(),
>as suggested during review;
>  - add barriers for making sure all initialization is done
>when the new lock is assigned, as sugested during review;
>  - add comments and ASSERT-s about how and why the adopted
>locking scheme is safe, as suggested during review.
> ---
>  xen/common/sched_arinc653.c |   34 ++
>  xen/common/sched_credit.c   |   44 +++
>  xen/common/sched_credit2.c  |   81 
> ---
>  xen/common/sched_rt.c   |   45 +---
>  xen/common/schedule.c   |   41 +-
>  xen/include/xen/sched-if.h  |3 ++
>  6 files changed, 206 insertions(+), 42 deletions(-)
> 
> diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
> index b79fcdf..ebd2090 100644
> --- a/xen/common/sched_arinc653.c
> +++ b/xen/common/sched_arinc653.c
> @@ -652,6 +652,38 @@ a653sched_pick_cpu(const struct scheduler *ops, struct 
> vcpu *vc)
>  }
>  
>  /**
> + * Xen scheduler callback to change the scheduler of a cpu
> + *
> + * @param new_ops   Pointer to this instance of the scheduler structure
> + * @param cpu   The cpu that is changing scheduler
> + * @param pdata scheduler specific PCPU data (we don't have any)
> + * @param vdata sched

[Xen-devel] [PATCH 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

Register with kernel restart handler instead of setting arm_pm_restart
directly.

Select a high priority of 192 to ensure that default restart handlers
are replaced if Xen is running.

Signed-off-by: Guenter Roeck 
---
 arch/arm/xen/enlighten.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd7345c654..76a1d12fd27e 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -193,14 +194,22 @@ after_register_vcpu_info:
put_cpu();
 }
 
-static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
+static int xen_restart(struct notifier_block *nb, unsigned long action,
+  void *data)
 {
struct sched_shutdown r = { .reason = SHUTDOWN_reboot };
int rc;
rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
BUG_ON(rc);
+
+   return NOTIFY_DONE;
 }
 
+static struct notifier_block xen_restart_nb = {
+   .notifier_call = xen_restart,
+   .priority = 192,
+};
+
 static void xen_power_off(void)
 {
struct sched_shutdown r = { .reason = SHUTDOWN_poweroff };
@@ -370,7 +379,7 @@ static int __init xen_pm_init(void)
return -ENODEV;
 
pm_power_off = xen_power_off;
-   arm_pm_restart = xen_restart;
+   register_restart_handler(&xen_restart_nb);
if (!xen_initial_domain()) {
struct timespec64 ts;
xen_read_wallclock(&ts);
-- 
2.5.0


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

Re: [Xen-devel] [PATCH v3 04/11] xen: sched: close potential races when switching scheduler to CPUs

[George Dunlap]

On 08/04/16 13:52, George Dunlap wrote:
> On 08/04/16 02:23, Dario Faggioli wrote:
>> In short, the point is making sure that the actual switch
>> of scheduler and the remapping of the scheduler's runqueue
>> lock occur in the same critical section, protected by the
>> "old" scheduler's lock (and not, e.g., in the free_pdata
>> hook, as it is now for Credit2 and RTDS).
>>
>> Not doing  so, is (at least) racy. In fact, for instance,
>> if we switch cpu X from, Credit2 to Credit, we do:
>>
>>  schedule_cpu_switch(x, csched2 --> csched):
>>//scheduler[x] is csched2
>>//schedule_lock[x] is csched2_lock
>>csched_alloc_pdata(x)
>>csched_init_pdata(x)
>>pcpu_schedule_lock(x) > takes csched2_lock
>>scheduler[X] = csched
>>pcpu_schedule_unlock(x) --> unlocks csched2_lock
>>[1]
>>csched2_free_pdata(x)
>>  pcpu_schedule_lock(x) --> takes csched2_lock
>>  schedule_lock[x] = csched_lock
>>  spin_unlock(csched2_lock)
>>
>> While, if we switch cpu X from, Credit to Credit2, we do:
>>
>>  schedule_cpu_switch(X, csched --> csched2):
>>//scheduler[x] is csched
>>//schedule_lock[x] is csched_lock
>>csched2_alloc_pdata(x)
>>csched2_init_pdata(x)
>>  pcpu_schedule_lock(x) --> takes csched_lock
>>  schedule_lock[x] = csched2_lock
>>  spin_unlock(csched_lock)
>>[2]
>>pcpu_schedule_lock(x) > takes csched2_lock
>>scheduler[X] = csched2
>>pcpu_schedule_unlock(x) --> unlocks csched2_lock
>>csched_free_pdata(x)
>>
>> And if we switch cpu X from RTDS to Credit2, we do:
>>
>>  schedule_cpu_switch(X, RTDS --> csched2):
>>//scheduler[x] is rtds
>>//schedule_lock[x] is rtds_lock
>>csched2_alloc_pdata(x)
>>csched2_init_pdata(x)
>>  pcpu_schedule_lock(x) --> takes rtds_lock
>>  schedule_lock[x] = csched2_lock
>>  spin_unlock(rtds_lock)
>>pcpu_schedule_lock(x) > takes csched2_lock
>>scheduler[x] = csched2
>>pcpu_schedule_unlock(x) --> unlocks csched2_lock
>>rtds_free_pdata(x)
>>  spin_lock(rtds_lock)
>>  ASSERT(schedule_lock[x] == rtds_lock) [3]
>>  schedule_lock[x] = DEFAULT_SCHEDULE_LOCK [4]
>>  spin_unlock(rtds_lock)
>>
>> So, the first problem is that, if anything related to
>> scheduling, and involving CPU, happens at [1] or [2], we:
>>  - take csched2_lock,
>>  - operate on Credit1 functions and data structures,
>> which is no good!
>>
>> The second problem is that the ASSERT at [3] triggers, and
>> the third that at [4], we screw up the lock remapping we've
>> done for ourself in csched2_init_pdata()!
>>
>> The first problem arises because there is a window during
>> which the lock is already the new one, but the scheduler is
>> still the old one. The other two, becase we let schedulers
>> mess with the lock (re)mapping done by others.
>>
>> This patch, therefore, introduces a new hook in the scheduler
>> interface, called switch_sched, meant at being used when
>> switching scheduler on a CPU, and implements it for the
>> various schedulers, so that things are done in the proper
>> order and under the protection of the best suited (set of)
>> lock(s). It is necessary to add the hook (as compared to
>> keep doing things in generic code), because different
>> schedulers may have different locking schemes.
>>
>> Signed-off-by: Dario Faggioli 
> 
> Thanks!
> 
> Reviewed-by: George Dunlap 

Committers:

Hopefully the arinc653 maintainers will get an opportunity to take a
look at this before the hard freeze today.  But if not, given the
timing, and the fact that the patch is really more to do with the
interface to the scheduling system as a whole rather than internal
algorithms of the arinc scheduler, I think it's probably OK to take the
liberty of checking it in even without an Ack (as long as there's no
Nack).  We can always revert / amend it later if there are objections.

 -George

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

Re: [Xen-devel] [PATCH V2] x86/monitor: Disallow setting mem_access_emulate_each_rep when vm_event is NULL

[Andrew Cooper]

On 08/04/16 13:35, Razvan Cojocaru wrote:
> It is meaningless (and potentially dangerous - see 
> hvmemul_virtual_to_linear())
> to set mem_access_emulate_each_rep before xc_monitor_enable() (which allocates
> vcpu->arch.vm_event) has been called, so return an error from the
> XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP hypercall when that is the case.
>
> Signed-off-by: Razvan Cojocaru 
>
> ---
> Changes since V1:
>  - Fixed the if() condition.
>  - Introduced an rc return variable to simplify the code.
> ---
>  xen/include/asm-x86/monitor.h | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index 0954b59..a66760d 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -32,19 +32,29 @@
>  static inline
>  int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op 
> *mop)
>  {
> +int rc = 0;
> +
>  switch ( mop->op )
>  {
>  case XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP:
>  domain_pause(d);
> -d->arch.mem_access_emulate_each_rep = !!mop->event;
> +/*
> + * Enabling mem_access_emulate_each_rep without a vm_event subscriber
> + * is meaningless.
> + */
> +if ( d->vcpu && d->vcpu[0]->arch.vm_event )

Sorry - I forgot to mention this before, but this check is slightly
buggy.  You want

if ( d->max_vcpus && d->vcpu[0] && d->vcpu[0]->arch.vm_event )

d->max_vcpus being non-zero guarentees that d->vcpu has been allocated,
but you still need to check that d->vcpu[0] has been allocated before
following the pointer.

With that fixed, Reviewed-by: Andrew Cooper 

> +d->arch.mem_access_emulate_each_rep = !!mop->event;
> +else
> +rc = -EINVAL;
> +
>  domain_unpause(d);
>  break;
>  
>  default:
> -return -EOPNOTSUPP;
> +rc = -EOPNOTSUPP;
>  }
>  
> -return 0;
> +return rc;
>  }
>  
>  int arch_monitor_domctl_event(struct domain *d,


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

Re: [Xen-devel] [PATCH v3 04/11] xen: sched: close potential races when switching scheduler to CPUs

[Dario Faggioli]

On Fri, 2016-04-08 at 14:00 +0100, George Dunlap wrote:
> On 08/04/16 13:52, George Dunlap wrote:
> > On 08/04/16 02:23, Dario Faggioli wrote:
> > > Signed-off-by: Dario Faggioli 
> > Thanks!
> > 
> > Reviewed-by: George Dunlap 
> Committers:
> 
> Hopefully the arinc653 maintainers will get an opportunity to take a
> look at this before the hard freeze today.  But if not, given the
> timing, and the fact that the patch is really more to do with the
> interface to the scheduling system as a whole rather than internal
> algorithms of the arinc scheduler, I think it's probably OK to take
> the
> liberty of checking it in even without an Ack (as long as there's no
> Nack).  We can always revert / amend it later if there are
> objections.
> 
Thanks George,

FWIW, I do agree, and I'm up for fixing any issue that could be raised,
or any bug that could surface in ARINC code because of this, super
quickly, during the rc period.

Now, a much easier but I guess technically relevant question: assuming
that you (George) also like and Ack patch 8, should I (and this is for
committers) resend the series, or do you guys can fetch it from the git
branch and/or list (with the correct version of patch 8 being the one
attached to the reply to Juergen's further comments)?

Thanks again 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 1/4] libxl: set the device model version earlier in xenstore

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:26PM +0200, Roger Pau Monne wrote:
> So libxl doesn't have to pass the build info around just to get the device
> model used by the guest. This allows to simplify
> libxl__device_nic_setdefault.
> 
> Signed-off-by: Roger Pau Monné 
> Cc: Ian Jackson 
> Cc: Wei Liu 

I have some reservation on this approach. I would rather passing around
a struct than accessing xenstore. The latter is much more expensive.

Wei.

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

Re: [Xen-devel] [PATCH v3 08/11] xen: sched: allow for choosing credit2 runqueues configuration at boot

[George Dunlap]

On 08/04/16 08:35, Dario Faggioli wrote:
> On Fri, 2016-04-08 at 06:18 +0200, Juergen Gross wrote:
>> On 08/04/16 03:24, Dario Faggioli wrote:
>>>
>>> In fact, credit2 uses CPU topology to decide how to arrange
>>> its internal runqueues. Before this change, only 'one runqueue
>>> per socket' was allowed. However, experiments have shown that,
>>> for instance, having one runqueue per physical core improves
>>> performance, especially in case hyperthreading is available.
>>>
>>> In general, it makes sense to allow users to pick one runqueue
>>> arrangement at boot time, so that:
>>>  - more experiments can be easily performed to even better
>>>assess and improve performance;
>>>  - one can select the best configuration for his specific
>>>use case and/or hardware.
>>>
>>> This patch enables the above.
>>>
>>> Note that, for correctly arranging runqueues to be per-core,
>>> just checking cpu_to_core() on the host CPUs is not enough.
>>> In fact, cores (and hyperthreads) on different sockets, can
>>> have the same core (and thread) IDs! We, therefore, need to
>>> check whether the full topology of two CPUs matches, for
>>> them to be put in the same runqueue.
>>>
>>> Note also that the default (although not functional) for
>>> credit2, since now, has been per-socket runqueue. This patch
>>> leaves things that way, to avoid mixing policy and technical
>>> changes.
>>>
>>> Finally, it would be a nice feature to be able to select
>>> a particular runqueue arrangement, even when creating a
>>> Credit2 cpupool. This is left as future work.
>>>
>>> Signed-off-by: Dario Faggioli 
>>> Signed-off-by: Uma Sharma 
>>
>> Some nits below.
>>
> Thanks for the quick review!
> 
> A revised version of this patch is provided here (both inlined and
> attached), and a branch with the remaining to be committed patches of
> this series, and with this patch changed as you suggest, is available
> at:
> 
>  git://xenbits.xen.org/people/dariof/xen.git 
> rel/sched/credit2/fix-runq-and-haff-v4
>  
> http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/sched/credit2/fix-runq-and-haff-v4
> 
> Regards,
> Dario
> ---
> commit 7f491488bbff1cc3af021cd29fca7e0fba321e02
> Author: Dario Faggioli 
> Date:   Tue Sep 29 14:05:09 2015 +0200
> 
> xen: sched: allow for choosing credit2 runqueues configuration at boot
> 
> In fact, credit2 uses CPU topology to decide how to arrange
> its internal runqueues. Before this change, only 'one runqueue
> per socket' was allowed. However, experiments have shown that,
> for instance, having one runqueue per physical core improves
> performance, especially in case hyperthreading is available.
> 
> In general, it makes sense to allow users to pick one runqueue
> arrangement at boot time, so that:
>  - more experiments can be easily performed to even better
>assess and improve performance;
>  - one can select the best configuration for his specific
>use case and/or hardware.
> 
> This patch enables the above.
> 
> Note that, for correctly arranging runqueues to be per-core,
> just checking cpu_to_core() on the host CPUs is not enough.
> In fact, cores (and hyperthreads) on different sockets, can
> have the same core (and thread) IDs! We, therefore, need to
> check whether the full topology of two CPUs matches, for
> them to be put in the same runqueue.
> 
> Note also that the default (although not functional) for
> credit2, since now, has been per-socket runqueue. This patch
> leaves things that way, to avoid mixing policy and technical
> changes.
> 
> Finally, it would be a nice feature to be able to select
> a particular runqueue arrangement, even when creating a
> Credit2 cpupool. This is left as future work.
> 
> Signed-off-by: Dario Faggioli 
> Signed-off-by: Uma Sharma 

Reviewed-by: George Dunlap 

> ---
> Cc: George Dunlap 
> Cc: Uma Sharma 
> Cc: Juergen Gross 
> ---
> Changes from v3:
>  * fix type and other issue in comments;
>use ARRAY_SIZE when iterating the parameter string array.
> 
> Changes from v2:
>  * valid strings  are now in an array, that we scan during
>parameter parsing, as suggested during review.
> 
> Cahnges from v1:
>  * fix bug in parameter parsing, and start using strcmp()
>for that, as requested during review.
> 
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index ca77e3b..0047f94 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -469,6 +469,25 @@ combination with the `low_crashinfo` command line option.
>  ### credit2\_load\_window\_shift
>  > `= `
>  
> +### credit2\_runqueue
> +> `= core | socket | node | all`
> +
> +> Default: `socket`
> +
> +Specify how host CPUs are arranged in runqueues. Runqueues are kept
> +balanced wit

Re: [Xen-devel] [PATCH 3/4] libxl: only allow guests with a device model to use cd-{eject/insert}

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:28PM +0200, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné 
> Cc: Ian Jackson 
> Cc: Wei Liu 
> ---
>  tools/libxl/libxl.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 9d785a4..232e2c1 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -2909,6 +2909,12 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, 
> libxl_device_disk *disk,
>  goto out;
>  }
>  
> +if (dm_ver == LIBXL_DEVICE_MODEL_VERSION_NONE) {
> +LOG(ERROR, "HVM guests without a device model cannot use cd-insert");

It's not specific to HVM, right?

Wei.

> +rc = ERROR_FAIL;
> +goto out;
> +}
> +
>  disks = libxl_device_disk_list(ctx, domid, &num);
>  for (i = 0; i < num; i++) {
>  if (disks[i].is_cdrom && !strcmp(disk->vdev, disks[i].vdev))
> -- 
> 2.6.4 (Apple Git-63)
> 

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

Re: [Xen-devel] [PATCH v2 1/3] x86/ioreq server: Add new functions to get/set memory types.

[Andrew Cooper]

On 31/03/16 11:53, Yu Zhang wrote:
> For clarity this patch breaks the code to set/get memory types out
> of do_hvm_op() into dedicated functions: hvmop_set/get_mem_type().
> Also, for clarity, checks for whether a memory type change is allowed
> are broken out into a separate function called by hvmop_set_mem_type().
>
> There is no intentional functional change in this patch.
>
> Signed-off-by: Paul Durrant 
> Signed-off-by: Yu Zhang 
> Cc: Keir Fraser 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 

Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v2 2/3] x86/ioreq server: Rename p2m_mmio_write_dm to p2m_ioreq_server

[Andrew Cooper]

On 31/03/16 11:53, Yu Zhang wrote:
> Previously p2m type p2m_mmio_write_dm was introduced for write-
> protected memory pages whose write operations are supposed to be
> forwarded to and emulated by an ioreq server. Yet limitations of
> rangeset restrict the number of guest pages to be write-protected.
>
> This patch replaces the p2m type p2m_mmio_write_dm with a new name:
> p2m_ioreq_server, which means this p2m type can be claimed by one
> ioreq server, instead of being tracked inside the rangeset of ioreq
> server. Patches following up will add the related hvmop handling
> code which map/unmap type p2m_ioreq_server to/from an ioreq server.
>
> Signed-off-by: Paul Durrant 
> Signed-off-by: Yu Zhang 

Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Ian Jackson]

Chun Yan Liu writes ("Re: [PATCH 1/4] a fix in libxl_device_usbdev_list"):
> On 4/8/2016 at 12:45 AM, in message
> <22278.36492.245114.295...@mariner.uk.xensource.com>, Ian Jackson
>  wrote: 
> > If libxl__device_usbdev_list_for_usbctrl fails, shouldn't 
> > libxl_device_usbdev_list fail too ? 
> 
> Following the similar definitions of other device types, the return value of
> this function is "libxl_device_usbdev *", to treat the above case as failure,
> it cannot be reflected through return value, we can only set return value
> to NULL, but that will be confusing with a real no-device case.

Urk.  Yes, you're right.  *sigh*

We won't fix that bad API pattern for 4.7.  So

Acked-by: Ian Jackson 

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH 2/4] libxl: set the backend type to Qdisk for CDROM devices on DM HVM guests

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:27PM +0200, Roger Pau Monne wrote:
> This is needed because the cd-{insert/eject} functions are not prepared to
> deal with blkback, which would be used by default if no backend was
> specified.
> 
> Signed-off-by: Roger Pau Monné 
> Cc: Ian Jackson 
> Cc: Wei Liu 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v6 08/24] xsplice: Add helper elf routines

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH v6 08/24] xsplice: Add helper elf routines"):
> On 07/04/16 17:19, Ian Jackson wrote:
> > My understanding of this is that the purpose of this machinery is to
> > supply binary runtime patches to the hypervisor.  So I think someone
> > who can inject malicious xsplice payloads can already control the
> > host.  Is that right ?
> 
> Correct.

OK, good, then from my point of view:

Acked-by: Ian Jackson 

> > It might be worth mentioning somewhere that this loader must not be
> > used for xsplice payloads for guest kernels.
> 
> I don't see how this is related.  If the host admin wanted to patch
> guest kernels without using the kernels internal self-patching
> mechanism, it would be infinitely easier to do the patching from dom0,
> using toolstack mapping powers.

Well, maybe.  I was worried about someone trying to make this ELF
xsplice code dynamically patch a guest kernel at load time.  That
might seem like a convenient idea to them.  But if you think it's not
likely, then fine.

Ian.

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

Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

[Ian Jackson]

Ian Jackson writes ("Re: [PATCH 1/4] a fix in libxl_device_usbdev_list"):
> We won't fix that bad API pattern for 4.7.  So
> 
> Acked-by: Ian Jackson 

Queued.

Ian.

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

Re: [Xen-devel] [PATCH 4/4] libxl: remove code added to use the 'phy' backend with CDROM devices

[Wei Liu]

On Thu, Apr 07, 2016 at 07:45:29PM +0200, Roger Pau Monne wrote:
> Signed-off-by: Roger Pau Monné 
> Cc: Ian Jackson 
> Cc: Wei Liu 

Acked-by: Wei Liu 

> ---
>  tools/libxl/libxl_device.c | 9 +
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index ce53520..82405a7 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -191,20 +191,13 @@ static int disk_try_backend(disk_try_backend_args *a,
>  
>  switch (backend) {
>  case LIBXL_DISK_BACKEND_PHY:
> -if (!(a->disk->format == LIBXL_DISK_FORMAT_RAW ||
> -  a->disk->format == LIBXL_DISK_FORMAT_EMPTY)) {
> +if (a->disk->format != LIBXL_DISK_FORMAT_RAW) {
>  goto bad_format;
>  }
>  
>  if (libxl_defbool_val(a->disk->colo_enable))
>  goto bad_colo;
>  
> -if (a->disk->format == LIBXL_DISK_FORMAT_EMPTY) {
> -LOG(DEBUG, "Disk vdev=%s is empty, skipping physical device 
> check",
> -a->disk->vdev);
> -return backend;
> -}
> -
>  if (a->disk->backend_domid != LIBXL_TOOLSTACK_DOMID) {
>  LOG(DEBUG, "Disk vdev=%s, is using a storage driver domain, "
> "skipping physical device check", a->disk->vdev);
> -- 
> 2.6.4 (Apple Git-63)
> 

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

Re: [Xen-devel] [PATCH v2 3/3] x86/ioreq server: Add HVMOP to map guest ram with p2m_ioreq_server to an ioreq server

[Andrew Cooper]

On 31/03/16 11:53, Yu Zhang wrote:
> A new HVMOP - HVMOP_map_mem_type_to_ioreq_server, is added to
> let one ioreq server claim/disclaim its responsibility for the
> handling of guest pages with p2m type p2m_ioreq_server. Users
> of this HVMOP can specify whether the p2m_ioreq_server is supposed
> to handle write accesses or read ones or both in a parameter named
> flags. For now, we only support one ioreq server for this p2m type,
> so once an ioreq server has claimed its ownership, subsequent calls
> of the HVMOP_map_mem_type_to_ioreq_server will fail. Users can also
> disclaim the ownership of guest ram pages with this p2m type, by
> triggering this new HVMOP, with ioreq server id set to the current
> owner's and flags parameter set to 0.
>
> For now, both HVMOP_map_mem_type_to_ioreq_server and p2m_ioreq_server
> are only supported for HVMs with HAP enabled.
>
> Note that flags parameter(if not 0) of this HVMOP only indicates
> which kind of memory accesses are to be forwarded to an ioreq server,
> it has impact on the access rights of guest ram pages, but are not
> the same. Due to hardware limitations, if only write operations are
> to be forwarded, read ones will be performed at full speed, with
> no hypervisor intervention. But if read ones are to be forwarded to
> an ioreq server, writes will inevitably be trapped into hypervisor,
> which means significant performance impact.
>
> Also note that this HVMOP_map_mem_type_to_ioreq_server will not
> change the p2m type of any guest ram page, until HVMOP_set_mem_type
> is triggered. So normally the steps should be the backend driver
> first claims its ownership of guest ram pages with p2m_ioreq_server
> type, and then sets the memory type to p2m_ioreq_server for specified
> guest ram pages.
>
> Signed-off-by: Paul Durrant 
> Signed-off-by: Yu Zhang 
> Cc: Keir Fraser 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Jun Nakajima 
> Cc: Kevin Tian 
> Cc: Tim Deegan 
> ---
>  xen/arch/x86/hvm/emulate.c   | 125 
> +--
>  xen/arch/x86/hvm/hvm.c   |  95 +++--
>  xen/arch/x86/mm/hap/nested_hap.c |   2 +-
>  xen/arch/x86/mm/p2m-ept.c|  14 -
>  xen/arch/x86/mm/p2m-pt.c |  25 +---
>  xen/arch/x86/mm/p2m.c|  82 +
>  xen/arch/x86/mm/shadow/multi.c   |   3 +-
>  xen/include/asm-x86/p2m.h|  36 +--
>  xen/include/public/hvm/hvm_op.h  |  37 
>  9 files changed, 395 insertions(+), 24 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index ddc8007..77a4793 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -94,11 +94,69 @@ static const struct hvm_io_handler null_handler = {
>  .ops = &null_ops
>  };
>  
> +static int mem_read(const struct hvm_io_handler *io_handler,
> +uint64_t addr,
> +uint32_t size,
> +uint64_t *data)
> +{
> +struct domain *currd = current->domain;
> +unsigned long gmfn = paddr_to_pfn(addr);
> +unsigned long offset = addr & ~PAGE_MASK;
> +struct page_info *page = get_page_from_gfn(currd, gmfn, NULL, 
> P2M_UNSHARE);
> +uint8_t *p;
> +
> +if ( !page )
> +return X86EMUL_UNHANDLEABLE;
> +
> +p = __map_domain_page(page);
> +p += offset;
> +memcpy(data, p, size);

What happens when offset + size crosses the page boundary?


> diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
> index a1eae52..d46f186 100644
> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -489,6 +489,43 @@ struct xen_hvm_altp2m_op {
>  typedef struct xen_hvm_altp2m_op xen_hvm_altp2m_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_op_t);
>  
> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
> +
> +/*
> + * HVMOP_map_mem_type_to_ioreq_server : map or unmap the IOREQ Server 
> + *  to specific memroy type 
> + *  for specific accesses 
> + *
> + * Note that if only write operations are to be forwarded to an ioreq server,
> + * read operations will be performed with no hypervisor intervention. But if
> + * flags indicates that read operations are to be forwarded to an ioreq 
> server,
> + * write operations will inevitably be trapped into hypervisor, whether they
> + * are emulated by hypervisor or forwarded to ioreq server depends on the 
> flags
> + * setting. This situation means significant performance impact.
> + */
> +#define HVMOP_map_mem_type_to_ioreq_server 26
> +struct xen_hvm_map_mem_type_to_ioreq_server {
> +domid_t domid;  /* IN - domain to be serviced */
> +ioservid_t id;  /* IN - ioreq server id */
> +hvmmem_type_t type; /* IN - memory type */

hvmmem_type_t is an enum and doesn't have a fixed width.  It can't be
used in the public API.

You also have some implicit padding holes

Re: [Xen-devel] [PATCH 1/2] libxl: Set rc on failure of usbdev_busaddr_to_busid

[Ian Jackson]

Chun Yan Liu writes ("Re: [PATCH 1/2] libxl: Set rc on failure of 
usbdev_busaddr_to_busid"):
> On 4/8/2016 at 01:04 AM, in message
> <22278.37677.975595.101...@mariner.uk.xensource.com>, Ian Jackson
>  wrote: 
> > Chun Yan Liu writes ("Re: [PATCH 1/2] libxl: Set rc on failure of  
> > usbdev_busaddr_to_busid"): 
> > > Thanks, Ian! 
> >  
> > Should I take that as a Reviewed-by ? 
> 
> Ah, yes. Acked. :-)

Thanks, queued.

Ian.

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

Re: [Xen-devel] [PATCH V2] bind_usbintf: do not reuse 'path'

[Ian Jackson]

Chunyan Liu writes ("[PATCH V2] bind_usbintf: do not reuse 'path'"):
> To avoid confusion, use "intf_path" to indicate driver/interface path,
> and "bind_path" indicate driver/bind path.
> 
> Signed-off-by: Chunyan Liu 
> CC: Simon Cao 
> CC: George Dunlap 
> CC: Ian Jackson 

Acked-by: Ian Jackson 

And queued, thanks.

Again: I added `libxl: ' to the start of the patch title.  I also
added the CID.

Ian.

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

Re: [Xen-devel] [PATCH v4 14/14] configure: do not depend on SEABIOS_PATH or OVMF_PATH ...

[Wei Liu]

On Mon, Mar 14, 2016 at 05:55:49PM +, Anthony PERARD wrote:
> ... to compile SeaBIOS and OVMF. Only depends on CONFIG_*.
> 
> If --with-system-* configure option is used, then set *_CONFIG=n to not
> compile SEABIOS and OVMF.
> 
> Signed-off-by: Anthony PERARD 
> 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v4 03/14] configure: #define SEABIOS_PATH and OVMF_PATH

[Wei Liu]

On Mon, Mar 14, 2016 at 05:55:38PM +, Anthony PERARD wrote:
> Those paths are to be used by libxl, in order to load the firmware in
> memory. If a system path is not define via --with-system-seabios or
> --with-system-ovmf, then this default to the Xen firmware directory.
> 
> Signed-off-by: Anthony PERARD 
> 

Acked-by: Wei Liu 

> ---
> Please, run ./autogen.sh on this patch.
> ---
>  tools/configure.ac | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 5b5dda4..7e5452e 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -218,6 +218,9 @@ AC_ARG_WITH([system-seabios],
>  esac
>  ],[])
>  AC_SUBST(seabios_path)
> +AC_DEFINE_UNQUOTED([SEABIOS_PATH],
> +   ["${seabios_path:-$XENFIRMWAREDIR/seabios.bin}"],
> +   [SeaBIOS path])
>  
>  AC_ARG_WITH([system-ovmf],
>  AS_HELP_STRING([--with-system-ovmf@<:@=PATH@:>@],
> @@ -229,6 +232,9 @@ AC_ARG_WITH([system-ovmf],
>  esac
>  ],[])
>  AC_SUBST(ovmf_path)
> +AC_DEFINE_UNQUOTED([OVMF_PATH],
> +   ["${ovmf_path:-$XENFIRMWAREDIR/ovmf.bin}"],
> +   [OVMF path])
>  
>  AC_ARG_WITH([extra-qemuu-configure-args],
>  AS_HELP_STRING([--with-extra-qemuu-configure-args@<:@="--ARG1 ..."@:>@],
> -- 
> Anthony PERARD
> 

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

Re: [Xen-devel] [RFC PATCH] Data integrity extension support for xen-block

[Ian Jackson]

FYI I have things I want to say in this conversation but today I am
concentrating on committing for the 4.7 freeze.

Ian.

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

Re: [Xen-devel] [PATCH v8 00/28] Add ITS support

[Steve Capper]

Hello,
We are going to re-examine the design document behind GICv3 ITS within
ARM to see if any simplifications can be made. This should, in turn,
help us simplify this series somewhat.

The revised design document will then be sent to xen-devel for review
before we re-examine this series.

Cheers,
-- 
Steve Capper

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

Re: [Xen-devel] [PATCH v2] libxl: replace the usage of uuid_t with a char array

[Ian Jackson]

Roger Pau Monné writes ("Re: [PATCH v2] libxl: replace the usage of uuid_t with 
a char array"):
> What about:
> 
> /*
>  * LIBXL_HAVE_BYTEARRAY_UUID
>  *
>  * If this is defined, the internal member of libxl_uuid is defined
>  * as a 16 byte array that contains the UUID in big endian format.
>  * Also, the same structure layout is used across all OSes.
>  */
> #define LIBXL_HAVE_BYTEARRAY_UUID 1

Yes.

Ian.

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

Re: [Xen-devel] [PATCH] tools: handle xl migrate --debug in legacy stream

[Ian Jackson]

Wei Liu writes ("Re: [PATCH] tools: handle xl migrate --debug in legacy 
stream"):
> On Thu, Apr 07, 2016 at 06:31:01PM +0200, Olaf Hering wrote:
> > Doing a 'xl migrate --debug domU host' on xen-4.5 adds a
> > XC_SAVE_ID_ENABLE_VERIFY_MODE marker, which is not handled.
> > Since using --debug is valid usage, handle it by logging the fact
> > instead of aborting the migration.
> > 
> > Signed-off-by: Olaf Hering 
> > Cc: Ian Jackson 
> > Cc: Wei Liu 
> 
> Acked-by: Wei Liu 

Thanks, and thanks to Andrew for the review.  Queued for 4.7.

Ian.

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

Re: [Xen-devel] [PATCH v11 1/2] libxl: add support for vscsi

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v11 1/2] libxl: add support for vscsi"):
> What Ian wanted was that we need clear correlation of xl configuration
> syntax with libxl API fields. For example, for a FOO device

Thanks for explaining, yes.

> However, I don't think that can be easily achieved in the case of pvscsi
> because we need to support both legacy Xen-classic kernel and new PVOPS
> kernel. Olaf, please correct me if I'm wrong.

That is unfortunate.  Also I am confused, then: if it is not possible
to support a sensible semantics (with an appropriate syntax) at the
libxl level, how is it possible to do so at the xl level ?

(I confess I haven't reread the patch in detail today...)

Thanks,
Ian.

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

[Xen-devel] [PATCH v3] libxl: replace the usage of uuid_t with a char array

[Roger Pau Monne]

The internals of the uuid_t struct don't match a big endian octet stream on
*BSD systems, which means that it cannot be directly casted to a
uint8_t[16].

In order to solve that change the type to be an unsigned char[16], which
doesn't imply any other change on Linux. On *BSDs change the helpers so that
the uuid is always stored as a big endian byte stream.

NB: tested on FreeBSD and Linux only.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Wei Liu 
Discussed-with: Ian Jackson 
Discussed-with: Wei Liu 
---
Cc: Ian Jackson 
Cc: Wei Liu 
---
NB2: AFAICT the NetBSD version of libxl_uuid_from_string *could* be switched
to the FreeBSD one (because NetBSD also has uuid_from_string), but I don't
have a NetBSD box in order to test it.
---
Changes since v2:
 - Add LIBXL_HAVE_BYTEARRAY_UUID define.

Changes since v1:
 - Readd a line that was deleted by error.
---
 tools/libxl/libxl.h|  9 +
 tools/libxl/libxl_osdeps.h |  3 +++
 tools/libxl/libxl_uuid.c   | 28 
 tools/libxl/libxl_uuid.h   | 21 -
 4 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 677e060..8935a8f 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -932,6 +932,15 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, 
libxl_mac *src);
  */
 #define LIBXL_HAVE_VGA_INTERFACE_TYPE_UNKNOWN 1
 
+/*
+ * LIBXL_HAVE_BYTEARRAY_UUID
+ *
+ * If this is defined, the internal member of libxl_uuid is defined
+ * as a 16 byte array that contains the UUID in big endian format.
+ * Also, the same structure layout is used across all OSes.
+ */
+#define LIBXL_HAVE_BYTEARRAY_UUID 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_osdeps.h b/tools/libxl/libxl_osdeps.h
index 802c762..10ce703 100644
--- a/tools/libxl/libxl_osdeps.h
+++ b/tools/libxl/libxl_osdeps.h
@@ -30,6 +30,7 @@
 #define SYSFS_PCIBACK_DRIVER   "/kern/xen/pci"
 #define NETBACK_NIC_NAME   "xvif%ui%d"
 #include 
+#include 
 #elif defined(__OpenBSD__)
 #include 
 #elif defined(__linux__)
@@ -39,6 +40,7 @@
 #define SYSFS_PCIBACK_DRIVER   "/sys/bus/pci/drivers/pciback"
 #define NETBACK_NIC_NAME   "vif%u.%d"
 #include 
+#include 
 #elif defined(__sun__)
 #include 
 #elif defined(__FreeBSD__)
@@ -49,6 +51,7 @@
 #define NETBACK_NIC_NAME   "xnb%u.%d"
 #include 
 #include 
+#include 
 #endif
 
 #ifndef SYSFS_USBBACK_DRIVER
diff --git a/tools/libxl/libxl_uuid.c b/tools/libxl/libxl_uuid.c
index 7d4a032..dadb79b 100644
--- a/tools/libxl/libxl_uuid.c
+++ b/tools/libxl/libxl_uuid.c
@@ -64,27 +64,35 @@ uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid)
 int libxl_uuid_is_nil(const libxl_uuid *uuid)
 {
 uint32_t status;
+uuid_t nat_uuid;
 
-return uuid_is_nil(&uuid->uuid, &status);
+uuid_dec_be(uuid->uuid, &nat_uuid);
+
+return uuid_is_nil(&nat_uuid, &status);
 }
 
 void libxl_uuid_generate(libxl_uuid *uuid)
 {
 uint32_t status;
+uuid_t nat_uuid;
 
-BUILD_BUG_ON(sizeof(libxl_uuid) != sizeof(uuid_t));
-uuid_create(&uuid->uuid, &status);
+uuid_create(&nat_uuid, &status);
 assert(status == uuid_s_ok);
+
+uuid_enc_be(uuid->uuid, &nat_uuid);
 }
 
 #ifdef __FreeBSD__
 int libxl_uuid_from_string(libxl_uuid *uuid, const char *in)
 {
 uint32_t status;
+uuid_t nat_uuid;
 
-uuid_from_string(in, &uuid->uuid, &status);
+uuid_from_string(in, &nat_uuid, &status);
 if (status != uuid_s_ok)
-return -1;
+return ERROR_FAIL;
+uuid_enc_be(uuid->uuid, &nat_uuid);
+
 return 0;
 }
 #else
@@ -115,8 +123,12 @@ void libxl_uuid_clear(libxl_uuid *uuid)
 #ifdef __FreeBSD__
 int libxl_uuid_compare(const libxl_uuid *uuid1, const libxl_uuid *uuid2)
 {
+uuid_t nat_uuid1, nat_uuid2;
 
-return uuid_compare(&uuid1->uuid, &uuid2->uuid, NULL);
+uuid_dec_be(uuid1->uuid, &nat_uuid1);
+uuid_dec_be(uuid2->uuid, &nat_uuid2);
+
+return uuid_compare(&nat_uuid1, &nat_uuid2, NULL);
 }
 #else
 int libxl_uuid_compare(const libxl_uuid *uuid1, const libxl_uuid *uuid2)
@@ -128,13 +140,13 @@ int libxl_uuid_compare(const libxl_uuid *uuid1, const 
libxl_uuid *uuid2)
 const uint8_t *libxl_uuid_bytearray_const(const libxl_uuid *uuid)
 {
 
-return uuid->uuid_raw;
+return uuid->uuid;
 }
 
 uint8_t *libxl_uuid_bytearray(libxl_uuid *uuid)
 {
 
-return uuid->uuid_raw;
+return uuid->uuid;
 }
 #else
 
diff --git a/tools/libxl/libxl_uuid.h b/tools/libxl/libxl_uuid.h
index c5041c7..994320d 100644
--- a/tools/libxl/libxl_uuid.h
+++ b/tools/libxl/libxl_uuid.h
@@ -21,18 +21,19 @@
 uuid[4], uuid[5], uuid[6], uuid[7], \
 uuid[8], uuid[9], uuid[10], uuid[11], \
 uuid[12], uuid[13], uuid[14], uuid[15]
+#define LIBXL_UUID_BYTES(arg) LIBXL__UUID_BYTES((arg).uuid)
 
+typedef struct {
+/* UUID as an o

Re: [Xen-devel] [PATCH v3 04/11] xen: sched: close potential races when switching scheduler to CPUs

[Robert VanVossen]

On 4/8/2016 9:11 AM, Dario Faggioli wrote:
> On Fri, 2016-04-08 at 14:00 +0100, George Dunlap wrote:
>> On 08/04/16 13:52, George Dunlap wrote:
>>> On 08/04/16 02:23, Dario Faggioli wrote:
 Signed-off-by: Dario Faggioli 
>>> Thanks!
>>>
>>> Reviewed-by: George Dunlap 
>> Committers:
>>
>> Hopefully the arinc653 maintainers will get an opportunity to take a
>> look at this before the hard freeze today.  But if not, given the
>> timing, and the fact that the patch is really more to do with the
>> interface to the scheduling system as a whole rather than internal
>> algorithms of the arinc scheduler, I think it's probably OK to take
>> the
>> liberty of checking it in even without an Ack (as long as there's no
>> Nack).  We can always revert / amend it later if there are
>> objections.

Acked-by: Robert VanVossen 

Luckily, I had some time today.

Thanks,
Robbie VanVossen

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

Re: [Xen-devel] [PATCH v6 1/6] libxl: handle error from libxl__need_xenpv_qemu() correctly

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v6 1/6] libxl: handle error from 
libxl__need_xenpv_qemu() correctly"):
> On Thu, Mar 31, 2016 at 07:49:01AM +0200, Juergen Gross wrote:
> > In case libxl__need_xenpv_qemu() returns an error let domain creation
> > fail.
> > 
> > Signed-off-by: Juergen Gross 
> 
> Acked-by: Wei Liu 
> 
> Ian, this is a  backport candidate.

Queued for backport.

Ian.

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

Re: [Xen-devel] Regarding Outreachy project on Improving CR Dashboard

[Priya]

Hello,

I tried running the same command in new version of perceval.  I found the
following missing message id errors in perceval_mbox_parse.log file. I am
working on the testing part and I will be able to finish it in one or two
days.

You can see the errors here [1]

[1]:http://imgur.com/yVsIoCT




*Priya V*
Amrita University
LinkedIn

| GitHub  | Bitbucket

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

Re: [Xen-devel] [PATCH v3 01/11] xen: sched: make implementing .alloc_pdata optional

[Robert VanVossen]

On 4/7/2016 9:23 PM, Dario Faggioli wrote:
> The .alloc_pdata scheduler hook must, before this change,
> be implemented by all schedulers --even those ones that
> don't need to allocate anything.
> 
> Make it possible to just use the SCHED_OP(), like for
> the other hooks, by using ERR_PTR() and IS_ERR() for
> error reporting. This:
>  - makes NULL a variant of success;
>  - allows for errors other than ENOMEM to be properly
>communicated (if ever necessary).
> 
> This, in turn, means that schedulers not needing to
> allocate any per-pCPU data, can avoid implementing the
> hook. In fact, the artificial implementation of
> .alloc_pdata in the ARINC653 is removed (and, while there,
> nuke .free_pdata too, as it is equally useless).
> 
> Signed-off-by: Dario Faggioli 
> Reviewed-by: Meng Xu 
> Reviewed-by: Juergen Gross 
> Acked-by: George Dunlap 

Acked-by: Robert VanVossen 

> ---
> Cc: Robert VanVossen 
> Cc: Josh Whitehead 
> Cc: Jan Beulich 
> ---
> Changes from v1:
>  * only update sd->sched_priv if alloc_pdata does not return
>IS_ERR, so that xfree() can always be safely called on
>sd->sched_priv itself, as requested during review;
>  * xen/err.h included in .c files that actually need it,
>instead than in sched-if.h.
> ---
>  xen/common/sched_arinc653.c |   31 ---
>  xen/common/sched_credit.c   |5 +++--
>  xen/common/sched_credit2.c  |2 +-
>  xen/common/sched_rt.c   |8 
>  xen/common/schedule.c   |   27 +--
>  5 files changed, 25 insertions(+), 48 deletions(-)
> 
> diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
> index 8a11a2f..b79fcdf 100644
> --- a/xen/common/sched_arinc653.c
> +++ b/xen/common/sched_arinc653.c
> @@ -456,34 +456,6 @@ a653sched_free_vdata(const struct scheduler *ops, void 
> *priv)
>  }
>  
>  /**
> - * This function allocates scheduler-specific data for a physical CPU
> - *
> - * We do not actually make use of any per-CPU data but the hypervisor expects
> - * a non-NULL return value
> - *
> - * @param ops   Pointer to this instance of the scheduler structure
> - *
> - * @return  Pointer to the allocated data
> - */
> -static void *
> -a653sched_alloc_pdata(const struct scheduler *ops, int cpu)
> -{
> -/* return a non-NULL value to keep schedule.c happy */
> -return SCHED_PRIV(ops);
> -}
> -
> -/**
> - * This function frees scheduler-specific data for a physical CPU
> - *
> - * @param ops   Pointer to this instance of the scheduler structure
> - */
> -static void
> -a653sched_free_pdata(const struct scheduler *ops, void *pcpu, int cpu)
> -{
> -/* nop */
> -}
> -
> -/**
>   * This function allocates scheduler-specific data for a domain
>   *
>   * We do not actually make use of any per-domain data but the hypervisor
> @@ -737,9 +709,6 @@ static const struct scheduler sched_arinc653_def = {
>  .free_vdata = a653sched_free_vdata,
>  .alloc_vdata= a653sched_alloc_vdata,
>  
> -.free_pdata = a653sched_free_pdata,
> -.alloc_pdata= a653sched_alloc_pdata,
> -
>  .free_domdata   = a653sched_free_domdata,
>  .alloc_domdata  = a653sched_alloc_domdata,
>  
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index 4c4927f..63a4a63 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  
>  /*
> @@ -532,12 +533,12 @@ csched_alloc_pdata(const struct scheduler *ops, int cpu)
>  /* Allocate per-PCPU info */
>  spc = xzalloc(struct csched_pcpu);
>  if ( spc == NULL )
> -return NULL;
> +return ERR_PTR(-ENOMEM);
>  
>  if ( !alloc_cpumask_var(&spc->balance_mask) )
>  {
>  xfree(spc);
> -return NULL;
> +return ERR_PTR(-ENOMEM);
>  }
>  
>  spin_lock_irqsave(&prv->lock, flags);
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index b8c8e40..e97d8be 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -2047,7 +2047,7 @@ csched2_alloc_pdata(const struct scheduler *ops, int 
> cpu)
>  printk("%s: cpu %d not online yet, deferring initializatgion\n",
> __func__, cpu);
>  
> -return (void *)1;
> +return NULL;
>  }
>  
>  static void
> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> index 321b0a5..aece318 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /*
> @@ -681,7 +682,7 @@ rt_alloc_pdata(const struct scheduler *ops, int cpu)
>  spin_unlock_irqrestore(old_lock, flags);
>  
>  if ( !alloc_cpumask_var(&_cpumask_scratch[cpu]) )
> -return NULL;
> +return ERR_PTR(-ENOMEM);
>  
>  if ( prv->repl_timer == NULL )
>  {
> @@ -689,13 +690,12 @@ rt_alloc_pdata(const struct scheduler *ops, int cpu)
>  prv->rep

Re: [Xen-devel] [PATCH v3] libxl: replace the usage of uuid_t with a char array

[Ian Jackson]

Roger Pau Monne writes ("[PATCH v3] libxl: replace the usage of uuid_t with a 
char array"):
> The internals of the uuid_t struct don't match a big endian octet stream on
> *BSD systems, which means that it cannot be directly casted to a
> uint8_t[16].
> 
> In order to solve that change the type to be an unsigned char[16], which
> doesn't imply any other change on Linux. On *BSDs change the helpers so that
> the uuid is always stored as a big endian byte stream.

Queued, thanks.

Ian.

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

Re: [Xen-devel] [PATCH] tools: handle xl migrate --debug in legacy stream

[Ian Jackson]

Andrew Cooper writes ("Re: [PATCH] tools: handle xl migrate --debug in legacy 
stream"):
> On 07/04/16 17:31, Olaf Hering wrote:
> > Doing a 'xl migrate --debug domU host' on xen-4.5 adds a
> > XC_SAVE_ID_ENABLE_VERIFY_MODE marker, which is not handled.
> > Since using --debug is valid usage, handle it by logging the fact
> > instead of aborting the migration.
> >
> > Signed-off-by: Olaf Hering 
> > Cc: Ian Jackson 
> > Cc: Wei Liu 
> 
> Hmm - I had never considered someone doing that.  Debug mode for legacy
> migration was sufficiently broken (in a falling over wild pointers kind
> of way) that I assumed noone ever used it.
> 
> But yes - not needlessly killing the migration is an improvement.
> 
> Reviewed-by: Andrew Cooper 
> 
> Ian: This is also a backport candidate for 4.6

Queued for backport.

Ian.

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

Re: [Xen-devel] [PATCH v3 0/6] xl: convert exit codes related to domain subcommands to EXIT_[SUCCESS|FAILURE]

[Ian Jackson]

Dario Faggioli writes ("[PATCH v3 0/6] xl: convert exit codes related to domain 
subcommands to EXIT_[SUCCESS|FAILURE]"):
> This is about using EXIT_SUCCESS or EXIT_FAILURE when calling
> exit(), in xl.  main_foo() functions are treated as they were
> main(), and so they also return EXIT_SUCCESS or
> EXIT_FAILURE. Internal functions are also refactored. The idea was
> to have them return either 0 or 1, but there needs to be exceptions
> (that are documented). TBF, this series mostly deals with exit
> codes, and the amount of internal function refactoring is rather
> limited.

I have queued patches 1-5.  Thanks to Dario for the git branch.

Ian.

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

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[George Dunlap]

On 07/04/16 19:51, Luis R. Rodriguez wrote:
> While Andrew's position is right in that perhaps only Xen tools have to deal
> with the HVMLite specific entry, it would also still mean diverging from ARM's
> own EFI entry only position, which I'd like to clarify that ARM has no custom
> Xen entry, we should strive to match that. Anything far from that to me really
> deserves an explanation, specially if we are going to argue that HVMLite is
> the best that x86 Xen can do.
> 
> Ultimately unifying entry approaches for Xen in a streamlined fashion seems
> like a sensible thing to strive for. Anything we push in the other direction,
> as small as it can be, should deserve at least a 'hey, wait a minute'...

Quick factual correction here.

"Since ARM guests only use the EFI entry point, x86 guests should also
only use the EFI entry point" is certainly a reasonable argument to make.

However, dom0 on ARM does not use the EFI entry point.  When starting
dom0, Xen uses the native entry point (the one that UBoot uses) and
hands dom0 a device-tree node.  The reason this is possible on ARM is
that there are no assumptions made about what hardware is or is not
present on the system -- everything that needs to be communicated about
what is or is not present can be passed in DT.

So it is incorrect to say that ARM has an "EFI entry only" position.

(On ACPI systems, it does apparently generate some UEFI informational
tables, which it passes to the dom0 kernel via DT; and the kernel
unpacks and puts in the right place.  Normal Xen ARM guests can use EFI,
but that's because we start OVMF in the guest context to provide the EFI
services.  These may be where the idea that ARM guests use only the UEFI
entry point came from.)

Obviously it would be nice if we could use the native entry point on x86
as well, but there's decades of legacy hardware and backwards
compatibility to deal with there.

(Julien is a Xen ARM maintainer, he can correct me if I've said
something incorrect.)

 -George

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

Re: [Xen-devel] [PATCH v11 1/2] libxl: add support for vscsi

[Olaf Hering]

On Fri, Apr 08, Wei Liu wrote:

> What Ian wanted was that we need clear correlation of xl configuration
> syntax with libxl API fields. For example, for a FOO device
> 
>xl.cfg: FOO = [ "bar=baz" ]
> 
>libxl_types.idl:
> 
> libxl_device_FOO = Struct("device_foo", [
>   ("bar", some_type)
>   ...
>   ])
> 
> However, I don't think that can be easily achieved in the case of pvscsi
> because we need to support both legacy Xen-classic kernel and new PVOPS
> kernel. Olaf, please correct me if I'm wrong.

I think that has nothing todo with the backend kernel, the domU.cfg has
always "vscsi=[ 'pdev,vdev' ]". So in this case libxl_device_vscsictrl
should become libxl_device_vscsi.

> > +
> > +static bool vscsi_fill_ctrl(libxl__gc *gc,
> > +xs_transaction_t t,
> > +const char *fe_path,
> > +const char *dir,
> > +libxl_device_vscsictrl *ctrl)
> > +{
> > +libxl_device_vscsidev dev;
> > +char *tmp, *be_path, *devs_path;
> > +char **dev_dirs;
> > +unsigned int ndev_dirs, dev_dir;
> > +bool ok;
> > +
> > +ctrl->devid = atoi(dir);
> > +
> > +be_path = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend", fe_path, 
> > dir));
> > +if (!be_path)
> > +goto out;
> > +
> > +tmp = libxl__xs_read(gc, t, GCSPRINTF("%s/%s/backend-id", fe_path, 
> > dir));
> > +if (!tmp)
> > +goto out;
> > +ctrl->backend_domid = atoi(tmp);
> 
> Please sanitise input coming from frontend.  You need to check if the
> backend domid and frontend domid make sense.

What do you have in mind? Something like in vusb_be_from_xs_fe?

Olaf

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