[Xen-devel] [qemu-mainline test] 86813: regressions - FAIL

[osstest service owner]

flight 86813 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/86813/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 86454
 test-amd64-i386-qemuu-rhel6hvm-intel  9 redhat-installfail REGR. vs. 86454
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
86454
 test-amd64-i386-qemuu-rhel6hvm-amd  9 redhat-install  fail REGR. vs. 86454
 test-amd64-i386-freebsd10-amd64 10 guest-startfail REGR. vs. 86454
 test-amd64-amd64-qemuu-nested-amd  9 debian-hvm-install   fail REGR. vs. 86454
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail REGR. vs. 86454
 test-amd64-amd64-qemuu-nested-intel  9 debian-hvm-install fail REGR. vs. 86454
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 86454
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
86454
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 86454
 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 86454
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 86454
 test-amd64-i386-freebsd10-i386 10 guest-start fail REGR. vs. 86454

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds15 guest-start/debian.repeat fail blocked in 86454
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 86454
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 86454

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  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-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-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-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-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-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 qemuu4829e0378dfb91d55af9dfd741bd09e8f2c4f91a
baseline version:
 qemuud1f8764099022bc1173f2413331b26d4ff609a0c

Last test of basis86454  2016-03-17 06:01:30 Z5 days
Failing since 86547  2016-03-18 07:12:41 Z3 days4 attempts
Testing same since86628  2016-03-19 04:51:17 Z3 days3 attempts


People who touched revisions under test:
  Alberto Garcia 
  Daniel P. Berrange 
  David Gibson 
  Eduardo Habkost 
  Eric Blake 
  Jeff Cody 
  Kevin Wolf 
  Marcel Apfelbaum 
  Markus Armbruster 
  Matthew Fortune 
  Max Reitz 
  Paolo Bonzini 
  Peter Maydell 
  Stefan Hajnoczi 

jobs:
 build-amd64-xsm 

Re: [Xen-devel] Outreachy 2016

[Paulina Szubarczyk]

Hi Roger,

I installed Xen from the source yesterday.

​Thanks and regards
,
Paulina Szubarczyk

On 21 March 2016 at 17:28, Roger Pau Monné  wrote:

> On Mon, 21 Mar 2016, Paulina Szubarczyk wrote:
>
> > Hi ​Lars,
> >
> > ​Thank you for your answer. I am looking forward to further information
> from you.
>
> Hello Paulina,
>
> Sorry for the delay, I was away last week and I'm just trying to catch up
> with all my email.
>
> As a first step you should look into installing latest Xen from source on
> your prefered Linux distribution, we have a wiki page that should help
> you in doing this:
> http://wiki.xenproject.org/wiki/Compiling_Xen_From_Source.
>
> You should use the code from the git repository, either the staging or
> master branches should be fine (staging might be a bit unstable, so I
> would recommend starting with master).
>
> In the meantime I will try to search for some bit-sized contributions.
>
> Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v11 25/27] setup and control colo proxy on primary side

[Changlong Xie]

On 03/05/2016 02:05 AM, Ian Jackson wrote:

+static void colo_proxy_async_call(libxl__egc *egc,
+  libxl__colo_save_state *css,
+  void func(libxl__colo_save_state *),
+  libxl__ev_child_callback callback)
+{


This is a straight clone-and-hack of drbd_async_call.  Please make a
common function, or something.


Ok




+static void colo_proxy_async_wait_for_checkpoint(libxl__colo_save_state *css)
+{
+int req;
+
+req = colo_proxy_checkpoint(&css->cps, 500);


What is this magic number ?  This seems quite odd...



It's an empirical timeout value(5s) for waiting a checkpoint.

Thanks
-Xie


Thanks,
Ian.


.





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

Re: [Xen-devel] [PATCH v11 22/27] COLO proxy: implement setup/teardown of COLO proxy module

[Changlong Xie]

CC: lizhij...@cn.fujitsu.com

On 03/22/2016 01:44 PM, Changlong Xie wrote:

On 03/05/2016 01:59 AM, Ian Jackson wrote:

Changlong Xie writes ("[PATCH v11 22/27] COLO proxy: implement
setup/teardown of COLO proxy module"):

From: Wen Congyang 

setup/teardown of COLO proxy module.
we use netlink to communicate with proxy module.
About colo-proxy module:
https://lkml.org/lkml/2015/6/18/32


I just notice that, the url on lkml is out of operation.


How to use:
http://wiki.xen.org/wiki/COLO_-_Coarse_Grain_Lock_Stepping


This seems pleasingly self-contained, and to be done in the right
place, but:

I don't understand how discrepant packets are detected, in order to
cause a resync.  I mean, I don't see any code in this patch which
receives a notification of a discrepancy and causes a resync.  What am
I missing ?


Please refer:

http://www.spinics.net/lists/netdev/msg333520.html

I think it's detail enough.

Thanks
 -Xie



Thanks,
Ian.


.





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




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

Re: [Xen-devel] [PATCH v11 22/27] COLO proxy: implement setup/teardown of COLO proxy module

[Changlong Xie]

On 03/05/2016 01:59 AM, Ian Jackson wrote:

Changlong Xie writes ("[PATCH v11 22/27] COLO proxy: implement setup/teardown of 
COLO proxy module"):

From: Wen Congyang 

setup/teardown of COLO proxy module.
we use netlink to communicate with proxy module.
About colo-proxy module:
https://lkml.org/lkml/2015/6/18/32


I just notice that, the url on lkml is out of operation.


How to use:
http://wiki.xen.org/wiki/COLO_-_Coarse_Grain_Lock_Stepping


This seems pleasingly self-contained, and to be done in the right
place, but:

I don't understand how discrepant packets are detected, in order to
cause a resync.  I mean, I don't see any code in this patch which
receives a notification of a discrepancy and causes a resync.  What am
I missing ?


Please refer:

http://www.spinics.net/lists/netdev/msg333520.html

I think it's detail enough.

Thanks
-Xie



Thanks,
Ian.


.





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

Re: [Xen-devel] [PATCH v11 27/27] cmdline switches and config vars to control colo-proxy

[Changlong Xie]

On 03/05/2016 02:09 AM, Ian Jackson wrote:

Changlong Xie writes ("[PATCH v11 27/27] cmdline switches and config vars to control 
colo-proxy"):

From: Wen Congyang 

Add cmdline switches to 'xl migrate-receive' command to specify
a domain-specific hotplug script to setup COLO proxy.

Add a new config var 'colo.default.agentscript' to xl.conf, that
allows the user to override the default global script used to
setup COLO proxy.

...


+(b) An example for COLO network configuration: vif =[ '...,forwarddev=xxx,...']
+
+=item B :Forward devices for primary and secondary, there are
+directly connected.


What should the user specify as a `forwarddev' ?


a physical NIC(forwarddev=eth0) or a virtual NIC(forwarddev=vlan0) could 
be fine.





diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bff08b0..63e1a0d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1072,6 +1072,11 @@ static void domcreate_bootloader_done(libxl__egc *egc,
  crs->recv_fd = restore_fd;
  crs->hvm = (info->type == LIBXL_DOMAIN_TYPE_HVM);
  crs->callback = libxl__colo_restore_setup_done;
+if (dcs->colo_proxy_script)
+crs->colo_proxy_script = libxl__strdup(gc, 
dcs->colo_proxy_script);
+else
+crs->colo_proxy_script = GCSPRINTF("%s/colo-proxy-setup",
+   
libxl__xen_script_dir_path());


Would it be possible to separate out this setting and plubming of
colo_proxy_script into its own patch ?  It is quite formulaic and
makes the substance of the patch harder to read.


Will try that.




@@ -4660,8 +4666,9 @@ int main_migrate_receive(int argc, char **argv)
  int debug = 0, daemonize = 1, monitor = 1;
  libxl_checkpointed_stream checkpointed = LIBXL_CHECKPOINTED_STREAM_NONE;
  int opt;
+char *script = NULL;

-SWITCH_FOREACH_OPT(opt, "Fedrc", NULL, "migrate-receive", 0) {
+SWITCH_FOREACH_OPT(opt, "Fedrcn:", NULL, "migrate-receive", 0) {


I don't think we want to allocate `-n' for this.  I think needing to
specify this will be rare.  Can we use a long option instead ?



Ok

Thanks
-Xie


Thanks,
Ian.


.





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

Re: [Xen-devel] [PATCH v10 26/31] COLO proxy: implement setup/teardown of COLO proxy module

[Changlong Xie]

On 03/14/2016 05:13 PM, Wen Congyang wrote:

On 03/12/2016 06:25 AM, Konrad Rzeszutek Wilk wrote:

+extern int colo_proxy_setup(libxl__colo_proxy_state *cps);
+extern void colo_proxy_teardown(libxl__colo_proxy_state *cps);
  #endif
diff --git a/tools/libxl/libxl_colo_proxy.c b/tools/libxl/libxl_colo_proxy.c
new file mode 100644
index 000..e07e640
--- /dev/null
+++ b/tools/libxl/libxl_colo_proxy.c
@@ -0,0 +1,230 @@
+/*
+ * Copyright (C) 2015 FUJITSU LIMITED


2016?


Yes, I will check all new files.


+ * Author: Yang Hongyang 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+#include "libxl_colo.h"
+#include 
+
+#define NETLINK_COLO 28


Can you include a comment why 28? Why not 31415?


OK, will add a comment to describe it.




+
+enum colo_netlink_op {
+COLO_QUERY_CHECKPOINT = (NLMSG_MIN_TYPE + 1),
+COLO_CHECKPOINT,
+COLO_FAILOVER,
+COLO_PROXY_INIT,
+COLO_PROXY_RESET, /* UNUSED, will be used for continuous FT */
+};
+
+/* = colo-proxy: helper functions == */
+
+static int colo_proxy_send(libxl__colo_proxy_state *cps, uint8_t *buff,
+   uint64_t size, int type)
+{
+struct sockaddr_nl sa;
+struct nlmsghdr msg;
+struct iovec iov;
+struct msghdr mh;
+int ret;
+
+STATE_AO_GC(cps->ao);
+
+memset(&sa, 0, sizeof(sa));
+sa.nl_family = AF_NETLINK;
+sa.nl_pid = 0;
+sa.nl_groups = 0;
+
+msg.nlmsg_len = NLMSG_SPACE(0);
+msg.nlmsg_flags = NLM_F_REQUEST;
+if (type == COLO_PROXY_INIT) {
+msg.nlmsg_flags |= NLM_F_ACK;
+}


I don't think you need the { }?


Yes, will fix it in the next version.



Ah, yup:

5. Block structure

Every indented statement is braced apart from blocks that contain just
one statement.


+msg.nlmsg_seq = 0;
+/* This is untrusty */


Umm, can you be more specific pls?


+msg.nlmsg_pid = cps->index;
+msg.nlmsg_type = type;
+
+iov.iov_base = &msg;
+iov.iov_len = msg.nlmsg_len;
+
+mh.msg_name = &sa;
+mh.msg_namelen = sizeof(sa);
+mh.msg_iov = &iov;
+mh.msg_iovlen = 1;
+mh.msg_control = NULL;
+mh.msg_controllen = 0;
+mh.msg_flags = 0;
+
+ret = sendmsg(cps->sock_fd, &mh, 0);
+if (ret <= 0) {
+LOG(ERROR, "can't send msg to kernel by netlink: %s",
+strerror(errno));
+}
+
+return ret;
+}
+
+/* error: return -1, otherwise return 0 */
+static int64_t colo_proxy_recv(libxl__colo_proxy_state *cps, uint8_t **buff,
+   unsigned int timeout_us)
+{
+struct sockaddr_nl sa;
+struct iovec iov;
+struct msghdr mh = {
+.msg_name = &sa,
+.msg_namelen = sizeof(sa),
+.msg_iov = &iov,
+.msg_iovlen = 1,
+};
+struct timeval tv;
+uint32_t size = 16384;
+int64_t len = 0;
+int ret;
+
+STATE_AO_GC(cps->ao);
+uint8_t *tmp = libxl__malloc(NOGC, size);
+
+if (timeout_us) {
+tv.tv_sec = timeout_us / 100;
+tv.tv_usec = timeout_us % 100;
+setsockopt(cps->sock_fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
+}
+
+iov.iov_base = tmp;
+iov.iov_len = size;
+next:
+ret = recvmsg(cps->sock_fd, &mh, 0);
+if (ret <= 0) {
+if (errno != EAGAIN && errno != EWOULDBLOCK)


-EINTR ?


IIRC, WAGAIN and EWOULDBLOCK may have different value in some system.
EINTR is not handled here.




+LOGE(ERROR, "can't recv msg from kernel by netlink");
+goto err;
+}
+
+len += ret;
+if (mh.msg_flags & MSG_TRUNC) {
+size += 16384;
+tmp = libxl__realloc(NOGC, tmp, size);


You really should check 'tmp'.

If this loop continues on for some time the 'size' may be
in milions and this realloc will fail.


As i see from the code, libxl_realloc will invoke _exit(-1) if malloc 
failed, so we'll keep it.


Thanks
-Xie


OK, will fix it in the next version.




+iov.iov_base = tmp + len;
+iov.iov_len = size - len;
+goto next;



+}
+
+*buff = tmp;
+ret = len;
+goto out;
+
+err:
+free(tmp);
+*buff = NULL;
+
+out:
+if (timeout_us) {
+tv.tv_sec = 0;
+tv.tv_usec = 0;
+setsockopt(cps->sock_fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
+}
+return ret;
+}
+
+/* = colo-proxy: setup and teardown == */
+
+int colo_proxy_setup(libxl__colo_proxy_state *cps)
+{
+int skfd

Re: [Xen-devel] [PATCH v6 14/22] arm/acpi: Create min DT stub for Dom0

[Julien Grall]

On 17/03/2016 09:41, Shannon Zhao wrote:

From: Shannon Zhao 

Create a DT for Dom0 for ACPI-case only. DT contains minimal required
informations such as Dom0 bootargs, initrd, efi description table and
address of uefi memory table.

Also port the document of this device tree bindings from Linux.


Porting means the document already exist in Linux and therefore there is 
a commit associated.


In this case, the document didn't reach Linux and this series will 
likely reach upstream and the Linux one.


So I think this should be the other way around.



Signed-off-by: Naresh Bhat 
Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  docs/misc/arm/device-tree/uefi.txt |  58 +++
  xen/arch/arm/domain_build.c| 143 +
  xen/arch/arm/efi/efi-dom0.c|  48 +
  xen/include/asm-arm/setup.h|   2 +
  4 files changed, 251 insertions(+)
  create mode 100644 docs/misc/arm/device-tree/uefi.txt

diff --git a/docs/misc/arm/device-tree/uefi.txt 
b/docs/misc/arm/device-tree/uefi.txt
new file mode 100644
index 000..41a8be0
--- /dev/null
+++ b/docs/misc/arm/device-tree/uefi.txt


Looking at the content, this file describe the bindings to notify the 
kernel that it runs on Xen. This is not UEFI specific.


So guest.txt would be a better name.


@@ -0,0 +1,58 @@
+* Xen hypervisor device tree bindings
+
+Xen ARM virtual platforms shall have a top-level "hypervisor" node with
+the following properties:
+
+- compatible:
+   compatible = "xen,xen-", "xen,xen";
+  where  is the version of the Xen ABI of the platform.
+
+- reg: specifies the base physical address and size of a region in
+  memory where the grant table should be mapped to, using an
+  HYPERVISOR_memory_op hypercall. The memory region is large enough to map
+  the whole grant table (it is larger or equal to gnttab_max_grant_frames()).
+
+- interrupts: the interrupt used by Xen to inject event notifications.
+  A GIC node is also required.


The properties "reg" and "interrupts" are not created by Xen when using 
ACPI. However, based on the bindings description they are mandatory.


Please update the description to reflect that the properties doesn't 
exist when booting using ACPI.



+
+To support UEFI on Xen ARM virtual platforms, Xen populates the FDT "uefi" node
+under /hypervisor with following parameters:
+
+
+Name  | Size   | Description
+
+xen,uefi-system-table | 64-bit | Guest physical address of the UEFI System
+  || Table.
+
+xen,uefi-mmap-start   | 64-bit | Guest physical address of the UEFI memory
+  || map.
+
+xen,uefi-mmap-size| 32-bit | Size in bytes of the UEFI memory map
+  || pointed to in previous entry.
+
+xen,uefi-mmap-desc-size   | 32-bit | Size in bytes of each entry in the UEFI
+  || memory map.
+
+xen,uefi-mmap-desc-ver| 32-bit | Version of the mmap descriptor format.
+
+
+Example (assuming #address-cells = <2> and #size-cells = <2>):
+
+hypervisor {
+   compatible = "xen,xen-4.3", "xen,xen";
+   reg = <0 0xb000 0 0x2>;
+   interrupts = <1 15 0xf08>;
+   uefi {
+   xen,uefi-system-table = <0x>;
+   xen,uefi-mmap-start = <0x>;
+   xen,uefi-mmap-size = <0x>;
+   xen,uefi-mmap-desc-size = <0x>;
+   xen,uefi-mmap-desc-ver = <0x>;
+};
+};
+
+The format and meaning of the "xen,uefi-*" parameters are similar to those in
+Documentation/arm/uefi.txt in Linux, which are provided by the regular Linux
+UEFI stub. However they differ because they are provided by the Xen hypervisor,
+together with a set of UEFI runtime services implemented via hypercalls, see
+xen/include/public/platform.h.
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index e036887..6726e45 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,145 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+#define ACPI_DOM0_FDT_MIN_SIZE 4096
+
+static int make_chosen_node(const struct kernel_info *kinfo,


Please prefix it with acpi_ to make clear it's ACPI specific.


+struct membank tbl

Re: [Xen-devel] [PATCH v6 13/22] arm/acpi: Map the new created EFI and ACPI tables to Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Map the UEFI and ACPI tables which we created to non-RAM space in Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/domain_build.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 008fc76..e036887 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1691,6 +1691,21 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
 d->arch.efi_acpi_table, &kinfo->mem, tbl_add);

+/* Map the EFI and ACPI tables to Dom0 */
+rc = map_regions_rw(d,
+paddr_to_pfn(d->arch.efi_acpi_gpa),
+PFN_UP(d->arch.efi_acpi_len),
+paddr_to_pfn(virt_to_maddr(d->arch.efi_acpi_table)));


The ACPI/EFI tables could potentially have data in the cache but are not 
written into the memory (because Xen is mapping the RAM with caching 
enabled). However, DOM0 may decide to map it with cache disabled. 
Therefore it would be possible for the domain to see wrong data.


So I think you need to clean the cache for this region.


+if ( rc != 0 )
+{
+printk(XENLOG_ERR "Unable to map 0x%"PRIx64


"Unable to map EFI/ACPI table ..." to differentiate with a similar error 
message in map_range_to_domain



+   " - 0x%"PRIx64" in domain %d\n",
+   d->arch.efi_acpi_gpa & PAGE_MASK,
+   PAGE_ALIGN(d->arch.efi_acpi_gpa + d->arch.efi_acpi_len) - 1,
+   d->domain_id);
+return rc;
+}
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v6 01/22] arm/acpi: Estimate memory required for acpi/efi tables

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

[...]

+static int prepare_acpi(struct domain *d, struct kernel_info *kinfo)
+{
+int rc = 0;
+int order;
+
+rc = estimate_acpi_efi_size(d, kinfo);
+if ( rc != 0 )
+return rc;
+
+order = get_order_from_bytes(d->arch.efi_acpi_len);
+d->arch.efi_acpi_table = alloc_xenheap_pages(order, 0);
+if ( d->arch.efi_acpi_table == NULL )
+{
+printk("unable to allocate memory!\n");
+return -ENOMEM;
+}
+memset(d->arch.efi_acpi_table, 0, d->arch.efi_acpi_len);
+
+/* For ACPI, Dom0 doesn't use kinfo->gnttab_start to get the grant table
+ * region. So we use it as the ACPI table mapped address. */
+d->arch.efi_acpi_gpa = kinfo->gnttab_start;


I forgot to mention it on the last mail. If you re-use the gnttab 
region, you need to make sure that the region size will be enough to fit 
the ACPI tables.


It will help for debugging if we reach the limit in the future.


+
+return 0;


[...]

Regards,

--
Julien Grall

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

[Xen-devel] [linux-linus test] 86811: regressions - FAIL

[osstest service owner]

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

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
 test-amd64-amd64-xl-credit2  15 guest-localmigratefail REGR. vs. 59254
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 59254
 test-amd64-amd64-xl  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-i386-xl-xsm   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-i386-xl   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl-multivcpu 15 guest-localmigrate   fail REGR. vs. 59254
 test-amd64-amd64-pair  22 guest-migrate/dst_host/src_host fail REGR. vs. 59254
 test-armhf-armhf-xl   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-cubietruck  6 xen-bootfail REGR. vs. 59254
 test-armhf-armhf-xl-xsm   6 xen-boot  fail REGR. vs. 59254
 test-amd64-i386-pair   22 guest-migrate/dst_host/src_host fail REGR. vs. 59254
 test-amd64-amd64-xl-xsm  14 guest-saverestore fail REGR. vs. 59254
 test-armhf-armhf-xl-credit2   6 xen-boot  fail REGR. vs. 59254
 test-armhf-armhf-xl-multivcpu  6 xen-boot fail REGR. vs. 59254

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 15 guest-localmigratefail REGR. vs. 59254
 test-armhf-armhf-xl-rtds  6 xen-boot  fail REGR. vs. 59254
 test-amd64-amd64-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline 
untested
 test-armhf-armhf-xl-vhd   6 xen-bootfail baseline untested
 test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline 
untested
 test-amd64-amd64-libvirt 15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-libvirt-xsm 15 guest-saverestore.2  fail blocked in 59254
 test-amd64-i386-libvirt  15 guest-saverestore.2  fail blocked in 59254
 test-amd64-i386-libvirt-xsm  15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 59254
 test-amd64-amd64-xl-qemut-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-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  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-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  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
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1 fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 13 xen-boot/l1   fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass

version targeted for testing:
 linux643ad15d47410d37d43daf3ef1c8ac52c281efa5
baseline version:
 linux45820c294fe1b1a9df495d57f40585ef2d069a39

Last test of basis59254  2015-07-09 04:20:48 Z  256 days
Failing since 59348  2015-07-10 04:24:05 Z  255 days  184 attempts
Testing same since86811  2016-03-21 10:26:16 Z0 days1 attempts


4700 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm 

Re: [Xen-devel] [PATCH v2] arm: Fix asynchronous aborts (SError exceptions) due to bogus PTEs

[Shanker Donthineni]

Hi Julien,

Do you have any other comments to be addressed?

On 03/16/2016 02:08 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
> G4.12.6 External abort on a translation table walk
>
> Signed-off-by: Vikram Sethi 
> Signed-off-by: Shanker Donthineni 
> ---
> Changes since v1:
> Replace memset() with clear_page()
> Edit commit description 
>
>  xen/arch/arm/mm.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 81f9e2e..3fda8f3 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -730,6 +730,8 @@ void __init setup_xenheap_mappings(unsigned long base_mfn,
>  else
>  {
>  unsigned long first_mfn = alloc_boot_pages(1, 1);
> +
> +clear_page(mfn_to_virt(first_mfn));
>  pte = mfn_to_xen_entry(first_mfn, WRITEALLOC);
>  pte.pt.table = 1;
>  write_pte(p, pte);
> @@ -773,6 +775,7 @@ void __init setup_frametable_mappings(paddr_t ps, paddr_t 
> pe)
>  second = mfn_to_virt(second_base);
>  for ( i = 0; i < nr_second; i++ )
>  {
> +clear_page(mfn_to_virt(second_base + i));
>  pte = mfn_to_xen_entry(second_base + i, WRITEALLOC);
>  pte.pt.table = 1;
>  write_pte(&xen_first[first_table_offset(FRAMETABLE_VIRT_START)+i], 
> pte);

-- 
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


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

Re: [Xen-devel] [libvirt test] 86625: regressions - FAIL

[Jim Fehlig]

On 03/19/2016 05:55 PM, osstest service owner wrote:
> flight 86625 libvirt real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/86625/

There was a fair bit of turmoil after the libvirt NSS module got merged. AFAIK,
the various build issues and 'make check' failures have been resolved.

Regards,
Jim


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

[Xen-devel] i don't get any eth0 while running xen on omap5

[Safa Hamza]

i get xen running on omap5 board with linux dom0 and ubuntu 14.04 lts file
system

i Get the linux kernel source from

git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git


now i'm trying to install some guests but the problem i don't have internet
connexion


when typing "ifconfig" i don't get any "eth0" only" lo losetup" despite in
/etc/network/interfaces there is eth0 .

i think the problem is in my kernel configuration but i don't know to fix
it ..

i'll apprecite your help

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

Re: [Xen-devel] identify a Xen PV domU to fix devmem_is_allowed

[Boris Ostrovsky]

On 03/15/2016 12:57 PM, Olaf Hering wrote:

On Tue, Mar 01, Boris Ostrovsky wrote:


on domU:

[root@dhcp-burlington7-2nd-B-east-10-152-55-140 ~]# od -N 1 -j 4096 /dev/mem
od: /dev/mem: read error: Bad address
001
[root@dhcp-burlington7-2nd-B-east-10-152-55-140 ~]#

with

(XEN) mm.c:1767:d14v0 Bad L1 flags 10

How should we proceed with this bug?



I can't see any way to avoid calling xen_pv_domain() so what you 
suggested should work. The only problem is that this will now cause 
reserved areas to also return 0:


# head /proc/iomem
-0fff : reserved <
1000-0009 : System RAM
000a-000f : reserved   <=
  000f-000f : System ROM
0010-7fff : System RAM
  0100-0172a065 : Kernel code
  0172a066-01d32b3f : Kernel data
  01ec5000-02026fff : Kernel bss
fee0-fee00fff : Local APIC

which I don't think they really should.

How about this:

if (pagenr < 256 && !xen_pv_domain())
return 1;
if (iomem_is_exclusive(pagenr << PAGE_SHIFT))
return 0;
if (!page_is_ram(pagenr))
return 1;


Also, while looking into this I noticed that pat_x_mtrr_type() will make 
us switch from _PAGE_CACHE_MODE_WB to _PAGE_CACHE_MODE_UC_MINUS when 
trying to mmap and this is what causes the hypervisor error message and 
the splat in Linux. We make this switch despite the fact that MTRR is 
disabled and therefore mtrr_type_lookup() returns MTRR_TYPE_INVALID.


Should we return req_type is MTRR is disabled?

-boris





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

[Xen-devel] [linux-mingo-tip-master test] 86788: regressions - FAIL

[osstest service owner]

flight 86788 linux-mingo-tip-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/86788/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 60684
 test-amd64-amd64-libvirt 14 guest-saverestore fail REGR. vs. 60684
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 60684
 test-amd64-amd64-xl-xsm  15 guest-localmigratefail REGR. vs. 60684
 test-amd64-amd64-xl  15 guest-localmigratefail REGR. vs. 60684
 test-amd64-amd64-xl-multivcpu 15 guest-localmigrate   fail REGR. vs. 60684
 test-amd64-amd64-libvirt-xsm 15 guest-saverestore.2   fail REGR. vs. 60684
 test-amd64-amd64-xl-credit2  15 guest-localmigratefail REGR. vs. 60684

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 17 guest-localmigrate/x10fail REGR. vs. 60684
 test-amd64-i386-libvirt-xsm  15 guest-saverestore.2  fail blocked in 60684
 test-amd64-i386-xl-xsm   15 guest-localmigrate   fail blocked in 60684
 test-amd64-i386-xl   15 guest-localmigrate   fail blocked in 60684
 test-amd64-i386-libvirt  15 guest-saverestore.2  fail blocked in 60684
 test-amd64-amd64-libvirt-pair 22 guest-migrate/dst_host/src_host fail blocked 
in 60684
 test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail blocked 
in 60684
 test-amd64-i386-pair  22 guest-migrate/dst_host/src_host fail blocked in 60684
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop   fail blocked in 60684
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 60684
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 60684
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 60684

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

version targeted for testing:
 linux37e94a21ee548b60e53710c72ae20673b8668bdf
baseline version:
 linux69f75ebe3b1d1e636c4ce0a0ee248edacc69cbe0

Last test of basis60684  2015-08-13 04:21:46 Z  221 days
Failing since 60712  2015-08-15 18:33:48 Z  218 days  164 attempts
Testing same since86788  2016-03-21 05:19:09 Z0 days1 attempts

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
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   fail
 test-amd64-amd64-xl  fail
 test-amd64-i386-xl   fail
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm pass
 test-amd64-amd64-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  fail
 test-amd64-amd64-xl-xsm   

[Xen-devel] [PATCH v2 7/9] libxl: Allow local access for block devices with hotplug scripts

[George Dunlap]

From: George Dunlap 

pygrub and qemuu need to be able to access a VM's disks locally in
order to be able to pull out the kernel and provide emulated disk
access, respectively.  This can be done either by accessing the local
disk directly, or by plugging the target disk into dom0 to allow
access.

Unfortunately, while the plugging machinery works for pygrub, it does
not yet work for qemuu; this means that at the moment, disks with
hotplug scripts or disks with non-dom0 backends cannot be provided as
emulated devices to HVM domains.

Fortunately, disks using hotplug scripts created in dom0 do create a
block device as part of set-up, which can be accessed locally; and if
they use block-common.sh:write_dev, this path will be written to
physical-device-path.

Modify libxl__device_disk_setdefault() to be able to fish this path
out of xenstore and pass it to the caller.

Unfortunately, at the time pygrub runs, the devices have not yet been
set up.  Rather than try to stash the domid somewhere to pass, we just
pass INVALID_DOMID.

This allows qemuu to emulate block devices created with custom hotplug
scripts.

Signed-off-by: George Dunlap 
---
Changes since v1:
- Ported on top of previous changed patches
- Removed vestigal paragraph from changelog
- Change domid argument to guest_domid
- Added a missing space after an if ()
- Trimmed some long lines

CC: Ian Jackson 
CC: Wei Liu 
CC: Roger Pau Monne 
---
 tools/libxl/libxl.c  | 43 ++-
 tools/libxl/libxl_dm.c   |  4 ++--
 tools/libxl/libxl_internal.h |  3 ++-
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 602bec2..944f4d1 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2295,7 +2295,7 @@ int libxl__device_disk_setdefault(libxl__gc *gc, 
libxl_device_disk *disk)
 }
 
 int libxl__device_from_disk(libxl__gc *gc, uint32_t domid,
-   libxl_device_disk *disk,
+   const libxl_device_disk *disk,
libxl__device *device)
 {
 int devid;
@@ -3001,8 +3001,9 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void 
*get_vdev_user,
 /* Callbacks */
 
 char *libxl__device_disk_find_local_path(libxl__gc *gc, 
- const libxl_device_disk *disk,
- bool qdisk_direct)
+  libxl_domid guest_domid,
+  const libxl_device_disk *disk,
+  bool qdisk_direct)
 {
 char *path = NULL;
 
@@ -3031,6 +3032,38 @@ char *libxl__device_disk_find_local_path(libxl__gc *gc,
 path = libxl__strdup(gc, disk->pdev_path);
 LOG(DEBUG, "Directly accessing local QDISK target %s", path);
 goto out;
+} 
+
+/* 
+ * If the format isn't raw and / or we're using a script, then see
+ * if the script has written a path to the "cooked" node
+ */
+if (disk->script && guest_domid != INVALID_DOMID) {
+libxl__device device;
+char *be_path, *pdpath;
+int rc;
+
+LOG(DEBUG,
+"Run from a script; checking for physical-device-path (vdev %s)",
+disk->vdev);
+
+rc = libxl__device_from_disk(gc, guest_domid, disk, &device);
+if (rc < 0)
+goto out;
+
+be_path = libxl__device_backend_path(gc, &device);
+
+pdpath = libxl__sprintf(gc, "%s/physical-device-path", be_path);
+
+LOG(DEBUG, "Attempting to read node %s", pdpath);
+path = libxl__xs_read(gc, XBT_NULL, pdpath);
+
+if (path)
+LOG(DEBUG, "Accessing cooked block device %s", path);
+else
+LOG(DEBUG, "No physical-device-path, can't access locally.");
+
+goto out;
 }
 
  out:
@@ -3057,7 +3090,8 @@ void libxl__device_disk_local_initiate_attach(libxl__egc 
*egc,
 
 LOG(DEBUG, "Trying to find local path");
 
-dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk, false);
+dls->diskpath = libxl__device_disk_find_local_path(gc, INVALID_DOMID,
+   in_disk, false);
 if (dls->diskpath) {
 LOG(DEBUG, "Local path found, executing callback.");
 dls->callback(egc, dls, 0);
@@ -3073,7 +3107,6 @@ void libxl__device_disk_local_initiate_attach(libxl__egc 
*egc,
 rc = libxl__device_disk_setdefault(gc, disk);
 if (rc) goto out;
 
-/* If we can't find a local path, attach it */
 libxl__prepare_ao_device(ao, &dls->aodev);
 dls->aodev.callback = local_device_attach_cb;
 device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev,
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1493b84..b581657 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1208,8 +1208,8 @@ static int libxl__build_device_mod

[Xen-devel] [PATCH v2 9/9] DO NOT APPLY libxl: Change hotplug script interface to use physical-device-path

[George Dunlap]

From: Ian Jackson 

DO NOT APPLY.  This is provided for future reference, as a starting
point to clean up the disk path.  A simple fix will make it "work",
but will introduce a subtle race condition.

* Change block-common.sh on Linux to only write physical-device-path
  with the path of the device node, rather than also writing
  physical-device with its major:minor numbers.

* Have the libxl Linux hotplug script scheduler convert this, by
  reading physical-device-path and writing physical-device.  This
  happens just when we have decided that there is no more script to
  run.

(As a reminder: Many hotplug scripts are called multiple times; so
libxl__get_hotplug_script_info() is called repeatedly until it returns
'0'.  Block scripts are only called once; but this gives us the
opportunity to do the translation at any point when
libxl__get_hotplug_script_info() *would* return zero.)

* Add an xxx about the fact that the sharing check in
  tools/hotplug/Linux/block needs adjusting.  Note that WITHOUT THIS
  OTHER FIX THE CHANGE TO BLOCK-COMMON.SH IS BROKEN.

* This has been tested (with the xxx commented out) and works.

The reason the block script is broken with this change is that
block:check_sharing() reads physical-device to check the major:minor
for duplicates rather than checking the path.  But since this is (now)
written by libxl without the block script lock held, it's possible
that a duplicate instance will run the check_sharing() check before
libxl gets a chance to write that node.

It should be sufficient to modify check_sharing() to read that node if
it's avialable, and if it's not available, to instead read the
major/minor from physical-device-path.

Signed-off-by: Ian Jackson 
Signed-off-by: George Dunlap 
---
Chances since rfc v1:
- Fixed two bugs in the patch
 - Use backend xenstore node rather than frondend
 - Correctly interpret return value for libxl__device_physdisk_major_minor()
- Remove erroneous comment about libxl__device_physdisk_major_minor()'s return 
value
- Port to libxl script

CC: Roger Pau Monne 
CC: Wei Liu 
---
 docs/misc/block-scripts.txt | 38 ++--
 tools/hotplug/Linux/block-common.sh | 15 ++--
 tools/libxl/libxl_linux.c   | 70 +++--
 3 files changed, 82 insertions(+), 41 deletions(-)

diff --git a/docs/misc/block-scripts.txt b/docs/misc/block-scripts.txt
index 6dd5d48..58c6955 100644
--- a/docs/misc/block-scripts.txt
+++ b/docs/misc/block-scripts.txt
@@ -48,18 +48,18 @@ Output
 Block scripts are responsible for making sure that if a file is
 provided to a VM read/write, that it is not provided to any other VM.
 
-FreeBSD block hotplug scripts must write
-"$XENBUS_PATH/physical-device-path" with the path to the physical
-device or file.  Linux and NetBSD block hotplug scripts *should* also
-write this node.
+Block hotplug scripts must write "$XENBUS_PATH/physical-device-path"
+with the path to the physical device or file.
 
-For the time being, Linux and NetBSD block hotplug scripts must write
-"$XENBUS_PATH/physical-device" with the device's major and minor
-numbers, written in hex, and separated by a colon.
+Linux scripts used to write "$XENBUS_PATH/physical-device" with the
+major and minor number of a block device, separated by a colon,
+instead of physical-device-path.  This interface style is deprecated
+but still supported.  However, some functionality, such as emulated
+devices for HVM guests, may not work.
 
 Scripts which include block-common.sh can simply call write_dev "$dev"
 with a path to the device, and write_dev will do the right thing, now
-and going forward.  (See the discussion below.)
+and going forward.
 
 Rationale and future work
 -
@@ -74,25 +74,9 @@ major/minor numbers, and can give direct access to a file 
without
 going through loopback; so its driver will consume
 physical-device-path.
 
-On Linux, the device model (qemu) needs access to a file it can
-interpret to provide emulated disks before paravirtualized drivers are
-marked as up.  The easiest way to accomplish this is to allow qemu to
-consume physical-device-path (rather than, say, having dom0 act as
-both a frontend and a backend).
-
-Going forward, the plan is at some point to have all block scripts
-simply write "physical-device-path", and then have libxl write the
-other nodes.  The reason we haven't done this yet is that the main
-block script wants to check to make sure the *major/minor* number
-hasn't been re-used, rather than just checking that the *specific
-device node* isn't re-used.  To do this it currently uses
-physical-device; and to do this *safely* it needs physical-device to
-be written with the lock held.
-
-The simplest solution for sorting this out would be to have the block
-script use physical-device if it's present, but if not, to directly
-stat physical-device-path.  But there's not time before the 4.7
-release to make sure all that works.
+Rather than have different inte

[Xen-devel] [PATCH v2 1/9] tools/hotplug: Add a "dummy" hotplug script for testing

[George Dunlap]

From: George Dunlap 

Testing the hotplug external script path at the moment involves
actually setting up one of the alternate datapaths (blktap, iscsi,
&c).  Simplify testing by making a script which does a simple loopback,
but still has a target that can't be used directly.

To use:

script=block-dummy,vdev=xvda,target=dummy:

Signed-off-by: George Dunlap 
Acked-by: Ian Jackson 
---
CC: Ian Jackson 
CC: Wei Liu 
CC: Roger Pau Monne 
---
 tools/hotplug/Linux/Makefile|   1 +
 tools/hotplug/Linux/block-dummy | 107 
 2 files changed, 108 insertions(+)

diff --git a/tools/hotplug/Linux/Makefile b/tools/hotplug/Linux/Makefile
index 6e10118..dcfd58a 100644
--- a/tools/hotplug/Linux/Makefile
+++ b/tools/hotplug/Linux/Makefile
@@ -27,6 +27,7 @@ XEN_SCRIPTS += vscsi
 XEN_SCRIPTS += block-iscsi
 XEN_SCRIPTS += block-tap
 XEN_SCRIPTS += block-drbd-probe
+XEN_SCRIPTS += block-dummy
 XEN_SCRIPTS += $(XEN_SCRIPTS-y)
 
 SUBDIRS-$(CONFIG_SYSTEMD) += systemd
diff --git a/tools/hotplug/Linux/block-dummy b/tools/hotplug/Linux/block-dummy
new file mode 100644
index 000..57d40b5
--- /dev/null
+++ b/tools/hotplug/Linux/block-dummy
@@ -0,0 +1,107 @@
+#!/bin/bash -e
+#
+# dummy Xen block device hotplug script
+#
+# Author George Dunlap 
+#
+# Based on block-iscsi by Roger Pau Monné 
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published
+# by the Free Software Foundation; version 2.1 only. with the special
+# exception on linking described in file LICENSE.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Lesser General Public License for more details.
+#
+# Usage:
+#
+# Target should be specified using the following syntax:
+#
+# script=block-dummy,vdev=xvda,target=dummy:
+
+dir=$(dirname "$0")
+. "$dir/block-common.sh"
+
+check_tools()
+{
+if ! command -v losetup > /dev/null 2>&1; then
+fatal "Unable to find losetup"
+fi
+}
+
+# Sets the following global variables based on the params field passed in as
+# a parameter: type file
+parse_target()
+{
+params=($(echo "$1" | tr ":" "\n"))
+
+type=${params[0]}
+file=${params[1]}
+if [ -z "$type" ] || [ -z "$file" ]; then
+fatal "Cannot parse required parameters"
+fi
+
+if [ "$type" != "dummy" ] ; then
+   fatal "Invalid type: $type"
+fi
+}
+
+# Attaches the device and writes xenstore backend entries to connect
+# the device
+add()
+{
+test -f "$file" || fatal "$file does not exist."
+
+loopdev=$(losetup -f 2>/dev/null || find_free_loopback_dev)
+if [ "$loopdev" = '' ]
+then
+fatal 'Failed to find an unused loop device'
+fi
+
+if LANG=C losetup -h 2>&1 | grep read-only >/dev/null
+then
+roflag="-$mode"; roflag="${roflag#-w}"; roflag="${roflag#-!}"
+else
+roflag=''
+fi
+
+do_or_die losetup $roflag "$loopdev" "$file"
+# FIXME Is this OK?
+xenstore_write "$XENBUS_PATH/node" "$loopdev"
+write_dev "$loopdev"
+}
+
+# Disconnects the device
+remove()
+{
+node=$(xenstore_read "$XENBUS_PATH/node")
+losetup -d "$node"
+}
+
+command=$1
+target=$(xenstore-read $XENBUS_PATH/params || true)
+if [ -z "$target" ]; then
+fatal "No information about the target"
+fi
+
+parse_target "$target"
+
+mode=$(xenstore_read "$XENBUS_PATH/mode")
+mode=$(canonicalise_mode "$mode")
+
+check_tools || exit 1
+
+case $command in
+add)
+add
+;;
+remove)
+remove
+;;
+*)
+exit 1
+;;
+esac
-- 
2.1.4


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

[Xen-devel] [PATCH v2 8/9] docs: Document block-script protocol

[George Dunlap]

Signed-off-by: George Dunlap 
---
Changes since v1:
- Attempt to make a clear distinction between custom hotplug scripts
and the script called for raw physical devices and files

CC: Ian Jackson 
CC: Wei Liu 
CC: Roger Pau Monne 
---
 docs/misc/block-scripts.txt | 101 
 1 file changed, 101 insertions(+)

diff --git a/docs/misc/block-scripts.txt b/docs/misc/block-scripts.txt
new file mode 100644
index 000..6dd5d48
--- /dev/null
+++ b/docs/misc/block-scripts.txt
@@ -0,0 +1,101 @@
+Block scripts
+=
+
+Block scripts are called at the moment anytime blkback is directly
+involved in providing access to a backend.  There are three general
+cases this happens:
+
+1. When a user passes a block device in the 'target' field of the disk
+specification
+
+2. When a user passes a file in the 'target' field of the disk
+specification
+
+3. When a user specifies a custom hotplug script.
+
+Setup
+-
+
+It is highly recommended that custom hotplug scripts as much as
+possible include and use the common Xen functionality.  If the script
+is run from the normal block script location (/etc/xen/scripts by
+default), then this can be done by adding the following to the top of
+the script:
+
+dir=$(dirname "$0")
+. "$dir/block-common.sh"
+
+
+Inputs
+--
+
+In all cases, the scripts are called with either "add" or "remove" as
+the command.  For custom scripts, the command will be the first
+argument of the script (i.e. $1).
+
+The environment variable XENBUS_PATH will be set to the
+path for the block device to be created.
+
+When the script is run, the following nodes shall already have been
+written into xenstore:
+
+ $XENBUS/paramsThe contents of the 'target' section of the disk 
specification verbatim.
+ $XENBUS/mode  'r' (for readonly) or 'w' (for read-write)
+
+Output
+---
+
+Block scripts are responsible for making sure that if a file is
+provided to a VM read/write, that it is not provided to any other VM.
+
+FreeBSD block hotplug scripts must write
+"$XENBUS_PATH/physical-device-path" with the path to the physical
+device or file.  Linux and NetBSD block hotplug scripts *should* also
+write this node.
+
+For the time being, Linux and NetBSD block hotplug scripts must write
+"$XENBUS_PATH/physical-device" with the device's major and minor
+numbers, written in hex, and separated by a colon.
+
+Scripts which include block-common.sh can simply call write_dev "$dev"
+with a path to the device, and write_dev will do the right thing, now
+and going forward.  (See the discussion below.)
+
+Rationale and future work
+-
+
+Historically, the block scripts wrote a node called "physical-device",
+which contains the major and minor numbers, written in hex, and
+separated by a colon (e.g., "1a:2").  This is required by the Linux
+blkback driver.
+
+FreeBSD blkback, on the other hand, does not have the concept of
+major/minor numbers, and can give direct access to a file without
+going through loopback; so its driver will consume
+physical-device-path.
+
+On Linux, the device model (qemu) needs access to a file it can
+interpret to provide emulated disks before paravirtualized drivers are
+marked as up.  The easiest way to accomplish this is to allow qemu to
+consume physical-device-path (rather than, say, having dom0 act as
+both a frontend and a backend).
+
+Going forward, the plan is at some point to have all block scripts
+simply write "physical-device-path", and then have libxl write the
+other nodes.  The reason we haven't done this yet is that the main
+block script wants to check to make sure the *major/minor* number
+hasn't been re-used, rather than just checking that the *specific
+device node* isn't re-used.  To do this it currently uses
+physical-device; and to do this *safely* it needs physical-device to
+be written with the lock held.
+
+The simplest solution for sorting this out would be to have the block
+script use physical-device if it's present, but if not, to directly
+stat physical-device-path.  But there's not time before the 4.7
+release to make sure all that works.
+
+Another possibility would be to only call out to scripts when using
+custom hotplug scripts; and when doing files or physical devices, to
+do the duplicate checking inside of libxl instead.  The rationale for
+doing this in block scripts rather than in libxl isn't clear at thes
+point.
-- 
2.1.4


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

[Xen-devel] [PATCH v2 5/9] libxl: Rearrange qemu upstream disk argument code

[George Dunlap]

Reorganize the qemuu disk argument code to make a clean separation
between finding a file to use, and constructing the parameters:

* Rename pdev_path to target_path

* Only use qemu_disk_format_string() in circumstances where qemu may
be interpreting the disk (i.e., backend==QDISK).  In all other cases,
it should use RAW.

* Share as much as possible between the is_cdrom path and the normal
path.

This is mainly prep for sharing the local path finder with the
bootloader; but it does allow cdroms to use any backend that a normal
disk can use. Previously this was limited to RAW files or things that
qemu could handle directly; as of this changeset, it now includes tap
disks; and in future changesets it will include backends with custom
block scripts.

NB that this retains an existing bug, that disks with custom block
scripts or non-dom0 backends will have the bogus pdev_path passed in
to qemu, most likely resulting in qemu exiting with an error.  This
will be fixed in follow-up patches.

Signed-off-by: George Dunlap 
---
Changes since v1:
- Move qemuu disk argument refactoring into a separate patch

CC: Ian Jackson 
CC: Wei Liu 
CC: Stefano Stabellini 
CC: Anthony Perard 
---
 tools/libxl/libxl_dm.c | 61 +-
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 4aca38e..dfcf141 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1161,9 +1161,9 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
 int disk, part;
 int dev_number =
 libxl__device_disk_dev_number(disks[i].vdev, &disk, &part);
-const char *format = qemu_disk_format_string(disks[i].format);
+const char *format;
 char *drive;
-const char *pdev_path;
+const char *target_path;
 
 if (dev_number == -1) {
 LOG(WARN, "unable to determine"" disk number for %s",
@@ -1171,22 +1171,22 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
 continue;
 }
 
-if (disks[i].is_cdrom) {
-if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY)
-drive = libxl__sprintf
-(gc, 
"if=ide,index=%d,readonly=%s,media=cdrom,cache=writeback,id=ide-%i",
- disk, disks[i].readwrite ? "off" : "on", dev_number);
-else
-drive = libxl__sprintf
-(gc, 
"file=%s,if=ide,index=%d,readonly=%s,media=cdrom,format=%s,cache=writeback,id=ide-%i",
- disks[i].pdev_path, disk, disks[i].readwrite ? "off" 
: "on", format, dev_number);
-} else {
-if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
+/* 
+ * If qemu isn't doing the interpreting, the parameter is
+ * always raw
+ */
+if (disks[i].backend == LIBXL_DISK_BACKEND_QDISK)
+format = qemu_disk_format_string(disks[i].format);
+else
+format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
+
+if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
+if (!disks[i].is_cdrom) {
 LOG(WARN, "cannot support"" empty disk format for %s",
 disks[i].vdev);
 continue;
 }
-
+} else {
 if (format == NULL) {
 LOG(WARN,
 "unable to determine"" disk image format %s",
@@ -1194,14 +1194,22 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
 continue;
 }
 
-if (disks[i].backend == LIBXL_DISK_BACKEND_TAP) {
-format = qemu_disk_format_string(LIBXL_DISK_FORMAT_RAW);
-pdev_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
-  disks[i].format);
-} else {
-pdev_path = disks[i].pdev_path;
-}
+if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
+target_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
+disks[i].format);
+else 
+target_path = disks[i].pdev_path;
+}
+
+if (disks[i].is_cdrom) {
+drive = libxl__sprintf(gc,
+ 
"if=ide,index=%d,readonly=%s,media=cdrom,cache=writeback,id=ide-%i",
+ disk, disks[i].readwrite ? "off" : "on", dev_number);
 
+if (target_path)
+drive = libxl__sprintf(gc, "%s,file=%s,format=%s",
+   drive, target_path, format);
+} else {
 /*
  

[Xen-devel] [PATCH v2 2/9] libxl: Remove redundant setting of phyical-device

[George Dunlap]

Regardless of whether we're running a custom hotplug script or using
normal phy: or file:, the "block" script will be run, which will set
all the necessary xenstore nodes.

In fact, writing this value here prevents the block script from
accomplishing its only purpose: to detect duplicate physical block
devices used in different virtual devices.  The first thing the block
script does is check to see if this node is written; and if it is, it
silently exits.

Remove this, and let the block script perform its duplicate checking
function.

Signed-off-by: George Dunlap 
Acked-by: Ian Jackson 
---
NOTE: It's likely that the duplicate checking for physical devices has
never been run under libxl (at least since this bug was introduced);
this may shake out some issues.

CC: Ian Jackson 
CC: Wei Liu 
CC: Roger Pau Monne 
---
 tools/libxl/libxl.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 3471c4c..93f50d3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -2440,21 +2440,6 @@ static void device_disk_add(libxl__egc *egc, uint32_t 
domid,
  libxl__xen_script_dir_path());
 flexarray_append_pair(back, "script", script);
 
-/* If the user did not supply a block script then we
- * write the physical-device node ourselves.
- *
- * If the user did supply a script then that script is
- * responsible for this since the block device may not
- * exist yet.
- */
-if (!disk->script &&
-disk->backend_domid == LIBXL_TOOLSTACK_DOMID) {
-int major, minor;
-if (!libxl__device_physdisk_major_minor(dev, &major, 
&minor))
-flexarray_append_pair(back, "physical-device",
-  GCSPRINTF("%x:%x", major, 
minor));
-}
-
 assert(device->backend_kind == LIBXL__DEVICE_KIND_VBD);
 break;
 
-- 
2.1.4


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

[Xen-devel] [PATCH v2 3/9] tools/hotplug: Write physical-device-path in addition to physical-device

[George Dunlap]

Change block-common.sh on Linux to write physical-device-path with the
path of the device node, in addition to physical-device with its
major:minor numbers.

Signed-off-by: George Dunlap 
---
NB that a later patch in this series introduces some documentation for
the block script protocol.  If it's wanted I could fold that into this
patch; or if we wanted to be really strict, I could check it in before
this patch (modifying it so that it's accurate at that point in time)
and then have this patch update it.

CC: Ian Jackson 
CC: Roger Pau Monne 
CC: Wei Liu 
---
 tools/hotplug/Linux/block-common.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/hotplug/Linux/block-common.sh 
b/tools/hotplug/Linux/block-common.sh
index ee95009..35110b4 100644
--- a/tools/hotplug/Linux/block-common.sh
+++ b/tools/hotplug/Linux/block-common.sh
@@ -67,7 +67,8 @@ write_dev() {
   fi
  
   xenstore_write "$XENBUS_PATH/physical-device" "$mm"
-
+  xenstore_write "$XENBUS_PATH/physical-device-path" "$1"
+  
   success
 }
 
-- 
2.1.4


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

[Xen-devel] [PATCH v2 0/9] tools: Allow HVM domains emulated access to disks provided by hotplug scripts

[George Dunlap]

In order for HVM domains to provide emulated access to disks provided
by hotplug scripts, qemu needs access to a "cooked" version of the
disk.  In the case of hotplug scripts, this "cooked" version is
available in the form of a block device passed to blkback.  Make this
"cooked" version available to qemu.

This series also starts to work towards a rationalized interface to
the block hotplug scripts, on which hotplug scripts for FreeBSD can be
added.

Changes since v1:
- Split one of the patches into two


George Dunlap (8):
  tools/hotplug: Add a "dummy" hotplug script for testing
  libxl: Remove redundant setting of phyical-device
  tools/hotplug: Write physical-device-path in addition to
physical-device
  libxl: Move check for local access to a funciton
  libxl: Rearrange qemu upstream disk argument code
  libxl: Share logic for finding path between qemuu and pygrub
  libxl: Allow local access for block devices with hotplug scripts
  docs: Document block-script protocol

Ian Jackson (1):
  DO NOT APPLY libxl: Change hotplug script interface to use
physical-device-path

 docs/misc/block-scripts.txt |  85 
 tools/hotplug/Linux/Makefile|   1 +
 tools/hotplug/Linux/block-common.sh |  16 ++---
 tools/hotplug/Linux/block-dummy | 107 ++
 tools/libxl/libxl.c | 128 ++--
 tools/libxl/libxl_dm.c  |  82 +++
 tools/libxl/libxl_internal.h|  11 +++-
 tools/libxl/libxl_linux.c   |  70 +++-
 8 files changed, 422 insertions(+), 78 deletions(-)
 create mode 100644 docs/misc/block-scripts.txt
 create mode 100644 tools/hotplug/Linux/block-dummy

-- 
CC: Ian Jackson 
CC: Wei Liu 
CC: Roger Pau Monne 
CC: Anthony Perard 


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

[Xen-devel] [PATCH v2 4/9] libxl: Move check for local access to a funciton

[George Dunlap]

From: George Dunlap 

Move pygrub checks for local access ability into a separate function.

Also reorganize libxl__device_disk_local_initiate_attach so that we
don't initialize dls->disk unless we actually end up doing a local
attach.

Signed-off-by: George Dunlap 
---
Changes since v1:
- Stylistic updates
 - No space between * and function name in libxl__device_disk_find_local_path
 - { on start of newline for function
 - Long line now under 80 characters
 - Remove redundant check for pdev_path
 - Space between if and condition
 - Avoid if ((x=...)) construction

CC: Ian Jackson 
CC: Wei Liu 
CC: Roger Pau Monne 
---
 tools/libxl/libxl.c | 67 +
 1 file changed, 47 insertions(+), 20 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 93f50d3..2d7a154 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3000,13 +3000,38 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void 
*get_vdev_user,
 
 /* Callbacks */
 
+static char *libxl__device_disk_find_local_path(libxl__gc *gc, 
+const libxl_device_disk *disk)
+{
+char *path = NULL;
+
+/* No local paths for driver domains */
+if (disk->backend_domname != NULL) {
+LOG(DEBUG, "Non-local backend, can't access locally.\n");
+goto out;
+}
+
+/*
+ * If this is in raw format, and we're not using a script or a
+ * driver domain, we can access the target path directly.
+ */
+if (disk->format == LIBXL_DISK_FORMAT_RAW
+&& disk->script == NULL) {
+path = libxl__strdup(gc, disk->pdev_path);
+LOG(DEBUG, "Directly accessing local RAW disk %s", path);
+goto out;
+} 
+
+ out:
+return path;
+}
+
 static void local_device_attach_cb(libxl__egc *egc, libxl__ao_device *aodev);
 
 void libxl__device_disk_local_initiate_attach(libxl__egc *egc,
  libxl__disk_local_state *dls)
 {
 STATE_AO_GC(dls->ao);
-char *dev = NULL;
 int rc;
 const libxl_device_disk *in_disk = dls->in_disk;
 libxl_device_disk *disk = &dls->disk;
@@ -3014,34 +3039,36 @@ void 
libxl__device_disk_local_initiate_attach(libxl__egc *egc,
 
 assert(in_disk->pdev_path);
 
-memcpy(disk, in_disk, sizeof(libxl_device_disk));
-disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
-if (in_disk->script != NULL)
-disk->script = libxl__strdup(gc, in_disk->script);
 disk->vdev = NULL;
 
-rc = libxl__device_disk_setdefault(gc, disk);
-if (rc) goto out;
+if (dls->diskpath)
+LOG(DEBUG, "Strange, dls->diskpath already set: %s", dls->diskpath);
 
-/* If this is in a driver domain, or it's not a raw format, or it involves
- * running a script, we have to do a local attach. */
-if (disk->backend_domname != NULL
-|| disk->format != LIBXL_DISK_FORMAT_RAW
-|| disk->script != NULL) {
+LOG(DEBUG, "Trying to find local path");
+
+dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk);
+if (dls->diskpath) {
+LOG(DEBUG, "Local path found, executing callback.");
+dls->callback(egc, dls, 0);
+} else {
+LOG(DEBUG, "Local path not found, initiating attach.");
+
+memcpy(disk, in_disk, sizeof(libxl_device_disk));
+disk->pdev_path = libxl__strdup(gc, in_disk->pdev_path);
+if (in_disk->script != NULL)
+disk->script = libxl__strdup(gc, in_disk->script);
+disk->vdev = NULL;
+
+rc = libxl__device_disk_setdefault(gc, disk);
+if (rc) goto out;
+
+/* If we can't find a local path, attach it */
 libxl__prepare_ao_device(ao, &dls->aodev);
 dls->aodev.callback = local_device_attach_cb;
 device_disk_add(egc, LIBXL_TOOLSTACK_DOMID, disk, &dls->aodev,
 libxl__alloc_vdev, (void *) blkdev_start);
-return;
 }
 
-LOG(DEBUG, "locally attaching RAW disk %s", disk->pdev_path);
-dev = disk->pdev_path;
-
-if (dev != NULL)
-dls->diskpath = libxl__strdup(gc, dev);
-
-dls->callback(egc, dls, 0);
 return;
 
  out:
-- 
2.1.4


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

[Xen-devel] [PATCH v2 6/9] libxl: Share logic for finding path between qemuu and pygrub

[George Dunlap]

From: George Dunlap 

qemu can also access disks which will be provided with a qdisk backend
directly; add a flag to libxl__device_disk_find_local_path to indicate
whether to check for qdisk direct access.

Call libxl__device_disk_find_local_path() for most paths.  If we can't
find a local path, print an error and skip the disk, rather than using
a bogus path.

Now if there is no local access to the disk (i.e., because the disk
has a non-local backend, or relies on a custom hotplug script), libxl
will now print a warning and not provide the emulated disk, rather
than providing bogus parameters to qemu which cause it to error out.
(Such disks will still be available via the PV backend.)

I left the libxl__blktap_devpath in the qemuu-specific code rather
than sharing it with the pyrgub code because:

1) When the pygrub path runs the guest disks have not yet been set up

2) libxl__blktap_devpath() will give you the existing devpath if it
already exists, but will set one up for you if you don't.  So on the
pygrub path, this would end up setting up a new tap device.

3) There is no tap-specific teardown code on the pygrub path, and I
don't want to add any (particularly since I'm hoping to remove tapdisk
altogether soon).

Signed-off-by: George Dunlap 
---
Changes since v1:
- Re-port on top of revisions to previous patches
- Address line length issues
- Remove stray space at the end of an if (xxx )
- Break out devicemodel argument rearrangement into a separate patch
- Spell out implications of warning messages which skip emulated disks

CC: Ian Jackson 
CC: Wei Liu 
CC: Stefano Stabellini 
CC: Anthony Perard 
CC: Roger Pau Monne 
---
 tools/libxl/libxl.c  | 17 ++---
 tools/libxl/libxl_dm.c   | 27 +++
 tools/libxl/libxl_internal.h |  8 
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2d7a154..602bec2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3000,8 +3000,9 @@ static char * libxl__alloc_vdev(libxl__gc *gc, void 
*get_vdev_user,
 
 /* Callbacks */
 
-static char *libxl__device_disk_find_local_path(libxl__gc *gc, 
-const libxl_device_disk *disk)
+char *libxl__device_disk_find_local_path(libxl__gc *gc, 
+ const libxl_device_disk *disk,
+ bool qdisk_direct)
 {
 char *path = NULL;
 
@@ -3022,6 +3023,16 @@ static char 
*libxl__device_disk_find_local_path(libxl__gc *gc,
 goto out;
 } 
 
+/*
+ * If we're being called for a qemu path, we can pass the target
+ * string directly as well
+ */
+if (qdisk_direct && disk->backend == LIBXL_DISK_BACKEND_QDISK) {
+path = libxl__strdup(gc, disk->pdev_path);
+LOG(DEBUG, "Directly accessing local QDISK target %s", path);
+goto out;
+}
+
  out:
 return path;
 }
@@ -3046,7 +3057,7 @@ void libxl__device_disk_local_initiate_attach(libxl__egc 
*egc,
 
 LOG(DEBUG, "Trying to find local path");
 
-dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk);
+dls->diskpath = libxl__device_disk_find_local_path(gc, in_disk, false);
 if (dls->diskpath) {
 LOG(DEBUG, "Local path found, executing callback.");
 dls->callback(egc, dls, 0);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index dfcf141..1493b84 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1182,23 +1182,42 @@ static int libxl__build_device_model_args_new(libxl__gc 
*gc,
 
 if (disks[i].format == LIBXL_DISK_FORMAT_EMPTY) {
 if (!disks[i].is_cdrom) {
-LOG(WARN, "cannot support"" empty disk format for %s",
+LOG(WARN, "Cannot support empty disk format for %s",
 disks[i].vdev);
 continue;
 }
 } else {
 if (format == NULL) {
 LOG(WARN,
-"unable to determine"" disk image format %s",
+"Unable to determine disk image format: %s\n",
+"Disk will be available via PV drivers but not as an"
+"emulated disk.",
 disks[i].vdev);
 continue;
 }
 
+/* 
+ * We can't call libxl__blktap_devpath from
+ * libxl__device_disk_find_local_path for now because
+ * the bootloader is called before the disks are set
+ * up, so this function would set up a blktap node,
+ * but there's no TAP tear-down on error conditions in
+ * the bootloader path.
+ */
 if (disks[i].backend == LIBXL_DISK_BACKEND_TAP)
 target_path = libxl__blktap_devpath(gc, disks[i].pdev_path,
  

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

[osstest service owner]

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

Regressions :-(

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

version targeted for testing:
 ovmf 6fe9d9c15b184ff0790b7ba0338c27fec58f7ebf
baseline version:
 ovmf 5ac96e3a28dd26eabee421919f67fa7c443a47f1

Last test of basis65543  2015-12-08 08:45:15 Z  104 days
Failing since 65593  2015-12-08 23:44:51 Z  103 days  114 attempts
Testing same since86792  2016-03-21 05:48:24 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 
  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 
  Jaben Carsey 
  Jeff Fan 
  Jiaxin Wu 
  jiewen yao 
  Jim Dailey 
  jim_dai...@dell.com 
  Jordan Justen 
  Karyne Mayer 
  Larry Hauch 
  Laszlo Ersek 
  Leahy, Leroy P
  Leahy, Leroy P 
  Lee Leahy 
  Leekha Shaveta 
  Leif Lindholm 
  Liming Gao 
  Mark Rutland 
  Marvin Haeuser 
  Michael Kinney 
  Michael LeMay 
  Michael Thomas 
  Michał Zegan 
  Ni, Ruiyu 
  Paolo Bonzini 
  Paulo Alcantara 
  Paulo Alcantara Cavalcanti 
  Peter Kirmeier 
  Qin Long 
  Qiu Shumin 
  Rodrigo Dias Correa 
  Ruiyu Ni 
  Ryan Harkin 
  Samer El-Haj-Mahmoud 
  Samer El-Haj-Mahmoud 
  Star Zeng 
  Supreeth Venkatesh 
  Tapan Shah 
  Tian, Feng 
  Vladislav Vovchenko 
  Yao Jiewen 
  Yao, Jiewen 
  Ye Ting 
  Yonghong Zhu 
  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 13692 lines long.)

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

Re: [Xen-devel] [PATCH v4 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Jan Beulich]

>>> On 21.03.16 at 18:04,  wrote:
> On Tue, 15 Mar 2016, Jan Beulich wrote:
>> >>> On 14.03.16 at 18:55,  wrote:
>> > --- a/xen/include/public/xen.h
>> > +++ b/xen/include/public/xen.h
>> > @@ -841,6 +841,37 @@ typedef struct start_info start_info_t;
>> >   */
>> >  #define XEN_HVM_START_MAGIC_VALUE 0x336ec578
>> >  
>> > +#if defined(__i386__) || defined(__x86_64__)
>> > +/* C representation of the x86/HVM start info layout.
>> > + *
>> > + * The canonical definition of this layout is abrove, this is just a way 
>> > to
>> > + * represent the layout described there using C types.
>> > + *
>> > + * NB: the packed attribute is not really needed, but it helps us enforce
>> > + * the fact this this is just a representation, and it might indeed
>> > + * be required in the future if there are alignment changes.
>> > + */
> 
> ^ Rationale on why the packed attribute was added.

Well, I admit to have overlooked this comment, but I don't see
how the packed attribute helps enforce anything. Hence, Anthony,
I think together with the attribute that part of the comment should
be removed.

> I would really like to avoid placing this in public headers, or else 
> people will think this is the definition of the payload and will forget 
> that this is just a C representation of it, but the definition is in a 
> comment just above. I want to avoid the issues we have already seen by the 
> usage of C structures as definitions of the placement of payloads in 
> memory.
> 
> If this really has to be there, please guard it with:
> 
> #if defined(__XEN__) || defined(__XEN_TOOLS__)
> 
> So only the Xen kernel/tools can use it.

And hvmloader can't. Not a good idea imo.

Jan


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

Re: [Xen-devel] [PATCH v3 20/28] x86/domctl: Update PV domain cpumasks when setting cpuid policy

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> @@ -87,6 +88,129 @@ static void update_domain_cpuid_info(struct domain *d,
>  d->arch.x86_model = (ctl->eax >> 4) & 0xf;
>  if ( d->arch.x86 >= 0x6 )
>  d->arch.x86_model |= (ctl->eax >> 12) & 0xf0;
> +
> +if ( is_pv_domain(d) && ((levelling_caps & LCAP_1cd) == LCAP_1cd) )
> +{
> +uint64_t mask = cpuidmask_defaults._1cd;
> +uint32_t ecx = ctl->ecx & pv_featureset[FEATURESET_1c];
> +uint32_t edx = ctl->edx & pv_featureset[FEATURESET_1d];
> +
> +/*
> + * Must expose hosts HTT value so a guest using native CPU can

DYM "native CPUID" here?

> + * correctly interpret other leaves which cannot be masked.
> + */
> +edx &= ~cpufeat_mask(X86_FEATURE_HTT);
> +if ( cpu_has_htt )
> +edx |= cpufeat_mask(X86_FEATURE_HTT);
> +
> +switch ( boot_cpu_data.x86_vendor )
> +{
> +case X86_VENDOR_INTEL:
> +mask &= ((uint64_t)edx << 32) | ecx;
> +break;
> +
> +case X86_VENDOR_AMD:
> +mask &= ((uint64_t)ecx << 32) | edx;
> +
> +/* Fast-forward bits - Must be set. */
> +if (ecx & cpufeat_mask(X86_FEATURE_XSAVE))

Missing blanks inside parens. Also I think the comment should be
expanded to include the fact that the need to do this here but
not in the Intel case was empirically(?) determined - just in case
something isn't quite right with this on some future hardware,
and readers (possibly including ourselves) wonder.

> +case X86_VENDOR_AMD:
> +/*
> + * Must expose hosts CMP_LEGACY value so a guest using native
> + * CPU can correctly interpret other leaves which cannot be

CPUID again?

> + * masked.
> + */
> +ecx &= ~cpufeat_mask(X86_FEATURE_CMP_LEGACY);
> +if ( cpu_has_cmp_legacy )
> +ecx |= cpufeat_mask(X86_FEATURE_CMP_LEGACY);
> +
> +mask &= ((uint64_t)ecx << 32) | edx;
> +
> +/* Fast-forward bits - Must be set. */
> +ecx = 0;
> +edx = cpufeat_mask(X86_FEATURE_APIC);
> +
> +mask |= ((uint64_t)ecx << 32) | edx;

This certainly looks strange (as in more complicated than it needs to
be) - why not just mask |= cpufeat_mask(X86_FEATURE_APIC)?

Jan


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

Re: [Xen-devel] [PATCH v4 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Roger Pau Monné]

Hello,

On Tue, 15 Mar 2016, Jan Beulich wrote:

> >>> On 14.03.16 at 18:55,  wrote:
> > --- a/xen/include/public/xen.h
> > +++ b/xen/include/public/xen.h
> > @@ -841,6 +841,37 @@ typedef struct start_info start_info_t;
> >   */
> >  #define XEN_HVM_START_MAGIC_VALUE 0x336ec578
> >  
> > +#if defined(__i386__) || defined(__x86_64__)
> > +/* C representation of the x86/HVM start info layout.
> > + *
> > + * The canonical definition of this layout is abrove, this is just a way to
> > + * represent the layout described there using C types.
> > + *
> > + * NB: the packed attribute is not really needed, but it helps us enforce
> > + * the fact this this is just a representation, and it might indeed
> > + * be required in the future if there are alignment changes.
> > + */

^ Rationale on why the packed attribute was added.

> > +struct hvm_start_info {
> > +uint32_t magic; /* Contains the magic value 0x336ec578 
> >   */
> > +/* ("xEn3" with the 0x80 bit of the "E" 
> > set).*/
> > +uint32_t version;   /* Version of this structure.  
> >   */
> > +uint32_t flags; /* SIF_xxx flags.  
> >   */
> > +uint32_t cmdline_paddr; /* Physical address of the command line.   
> >   */
> > +uint32_t nr_modules;/* Number of modules passed to the kernel. 
> >   */
> > +uint32_t modlist_paddr; /* Physical address of an array of 
> >   */
> > +/* hvm_modlist_entry.  
> >   */
> > +uint32_t rsdp_paddr;/* Physical address of the RSDP ACPI data  
> >   */
> > +/* structure.  
> >   */
> > +} __attribute__((packed));
> > +
> > +struct hvm_modlist_entry {
> > +uint64_t paddr; /* Physical address of the module. 
> >   */
> > +uint64_t size;  /* Size of the module in bytes.
> >   */
> > +uint64_t cmdline_paddr; /* Physical address of the command line.   
> >   */
> > +uint64_t reserved;
> > +} __attribute__((packed));
> > +#endif /* x86 */
> 
> This needs extra care: Either the packed attributes need to be
> dropped (Roger - why did they get put there in the first place?),
> or their use needs to be shielded from compilers not understanding
> them.

Hello,

The rationale behind the packed attribute is in the highlighted comment 
above.

I would really like to avoid placing this in public headers, or else 
people will think this is the definition of the payload and will forget 
that this is just a C representation of it, but the definition is in a 
comment just above. I want to avoid the issues we have already seen by the 
usage of C structures as definitions of the placement of payloads in 
memory.

If this really has to be there, please guard it with:

#if defined(__XEN__) || defined(__XEN_TOOLS__)

So only the Xen kernel/tools can use it.

Roger.

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

Re: [Xen-devel] [PATCH v3 19/28] x86/pv: Provide custom cpumasks for PV domains

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> And use them in preference to cpumask_defaults on context switch.  HVM 
> domains
> must not be masked (to avoid interfering with cpuid calls within the guest),
> so always lazily context switch to the host default.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v3 16/28] x86/cpu: Rework AMD masking MSR setup

[Andrew Cooper]

On 21/03/16 16:51, Jan Beulich wrote:
 On 15.03.16 at 16:35,  wrote:
>> +/* Fast-forward bits - Must be set. */
>> +if (ecx & cpufeat_mask(X86_FEATURE_XSAVE))
>> +ecx |= cpufeat_mask(X86_FEATURE_OSXSAVE);
> Wouldn't you think it would be better to also handle PKU/OSPKE
> here, just in case AMD decides to implement this? Because if they
> do, we will expose the feature no matter that this code is not
> ready for it.
>
> Either way,
> Reviewed-by: Jan Beulich 

OSPKE is in ecx.

The AMD mask registers cover eax and ebx.

~Andrew

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

Re: [Xen-devel] [PATCH v6 12/22] arm/acpi: Prepare EFI memory descriptor for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Create a few EFI memory descriptors to tell Dom0 the RAM region


s/a few//


information, ACPI table regions and EFI tables reserved resions.


s/resions/regions/



Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
---
v6: remove acpi_diabled check
---
  xen/arch/arm/domain_build.c |  2 ++
  xen/arch/arm/efi/efi-dom0.c | 40 
  xen/include/asm-arm/setup.h |  5 +
  3 files changed, 47 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 613551c..008fc76 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1688,6 +1688,8 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  acpi_map_other_tables(d);
  acpi_create_efi_system_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_table,
   tbl_add);
+acpi_create_efi_mmap_table(d->arch.efi_acpi_gpa, d->arch.efi_acpi_len,
+   d->arch.efi_acpi_table, &kinfo->mem, tbl_add);

  return 0;
  }
diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index b8a062c..3ffde94 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -23,6 +23,7 @@

  #include "efi.h"
  #include "efi-dom0.h"
+#include 
  #include 
  #include 

@@ -92,3 +93,42 @@ void __init acpi_create_efi_system_table(paddr_t paddr, void 
*efi_acpi_table,
  tbl_add[TBL_EFIT].start = table_addr;
  tbl_add[TBL_EFIT].size = table_size;
  }
+
+void __init acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,


Please rename paddr and size to a more meaningful name. Like: acpi_gpa 
and acpi_len.


Actually, you could directly pass the domain. So you will pass one 
argument rather 3 (paddr, size and efi_acpi_table).



+   void *efi_acpi_table,
+   const struct meminfo *mem,


Please pass kinfo instead.


+   struct membank tbl_add[])
+{
+EFI_MEMORY_DESCRIPTOR *memory_map;
+unsigned int i, offset;
+u8 *base_ptr;
+
+base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_MMAP);
+memory_map = (EFI_MEMORY_DESCRIPTOR *)(base_ptr);


NIT: the bracket around base_ptr are not necessary.


+
+offset = 0;
+for( i = 0; i < mem->nr_banks; i++, offset++ )
+{
+memory_map[offset].Type = EfiConventionalMemory;
+memory_map[offset].PhysicalStart = mem->bank[i].start;
+memory_map[offset].NumberOfPages = PFN_UP(mem->bank[i].size);


The page size use by Xen and UEFI may be different. Please use 
EFI_SIZE_TO_PAGES here.



+memory_map[offset].Attribute = EFI_MEMORY_WB;
+}
+
+for( i = 0; i < acpi_mem.nr_banks; i++, offset++ )
+{
+memory_map[offset].Type = EfiACPIReclaimMemory;
+memory_map[offset].PhysicalStart = acpi_mem.bank[i].start;
+memory_map[offset].NumberOfPages = PFN_UP(acpi_mem.bank[i].size);


Ditto

You are also assuming that acpi_mem.bank[i].size will always be aligned 
to 4KB. If so, we may expose unwanted data to the guest.


Based on how the field is set, I would add a BUG_ON to ensure this 
condition.



+memory_map[offset].Attribute = EFI_MEMORY_WB;
+}
+
+memory_map[offset].Type = EfiACPIReclaimMemory;
+memory_map[offset].PhysicalStart = paddr;
+memory_map[offset].NumberOfPages = PFN_UP(size);
+memory_map[offset].Attribute = EFI_MEMORY_WB;
+
+tbl_add[TBL_MMAP].start = paddr + acpi_get_table_offset(tbl_add, TBL_MMAP);
+tbl_add[TBL_MMAP].size = sizeof(EFI_MEMORY_DESCRIPTOR)
+ * (mem->nr_banks + acpi_mem.nr_banks + 1);
+}
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index e423b15..b2899f2 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -56,6 +56,11 @@ size_t estimate_efi_size(int mem_nr_banks);
  void acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
struct membank tbl_add[]);

+void acpi_create_efi_mmap_table(paddr_t paddr, paddr_t size,
+void *efi_acpi_table,
+const struct meminfo *mem,
+struct membank tbl_add[]);
+
  int construct_dom0(struct domain *d);

  void discard_initial_modules(void);



Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 16/28] x86/cpu: Rework AMD masking MSR setup

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> + /* Fast-forward bits - Must be set. */
> + if (ecx & cpufeat_mask(X86_FEATURE_XSAVE))
> + ecx |= cpufeat_mask(X86_FEATURE_OSXSAVE);

Wouldn't you think it would be better to also handle PKU/OSPKE
here, just in case AMD decides to implement this? Because if they
do, we will expose the feature no matter that this code is not
ready for it.

Either way,
Reviewed-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v3 17/28] x86/cpu: Rework Intel masking/faulting setup

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> This patch is best reviewed as its end result rather than as a diff, as it
> rewrites almost all of the setup.
> 
> On the BSP, cpuid information is used to evaluate the potential available 
> set
> of masking MSRs, and they are unconditionally probed, filling in the
> availability information and hardware defaults.  A side effect of this is 
> that
> probe_intel_cpuid_faulting() can move to being __init.
> 
> The command line parameters are then combined with the hardware defaults to
> further restrict the Xen default masking level.  Each cpu is then context
> switched into the default levelling state.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [PATCH v8 for Xen 4.7 1/4] xen: enable per-VCPU parameter for RTDS

[Jan Beulich]

>>> On 21.03.16 at 17:03,  wrote:
> On Mon, Mar 21, 2016 at 10:49 AM, Jan Beulich  wrote:
> On 21.03.16 at 16:18,  wrote:
>>> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich  wrote:
>>> On 18.03.16 at 22:26,  wrote:
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
>  #define XEN_SCHEDULER_ARINC653 7
>  #define XEN_SCHEDULER_RTDS 8
>
> -/* Set or get info? */
> +typedef struct xen_domctl_sched_credit {
> +uint16_t weight;
> +uint16_t cap;
> +} xen_domctl_sched_credit_t;
> +
> +typedef struct xen_domctl_sched_credit2 {
> +uint16_t weight;
> +} xen_domctl_sched_credit2_t;
> +
> +typedef struct xen_domctl_sched_rtds {
> +uint32_t period;
> +uint32_t budget;
> +} xen_domctl_sched_rtds_t;
> +
> +typedef struct xen_domctl_schedparam_vcpu {
> +union {
> +xen_domctl_sched_credit_t credit;
> +xen_domctl_sched_credit2_t credit2;
> +xen_domctl_sched_rtds_t rtds;
> +} u;
> +uint32_t vcpuid;
> +uint16_t padding[2];

 So why uint16_t[2] instead of just uint32_t? And what's the
 padding needed for in the first place?
>>>
>>> You're right. It's better to use uint32_t, which pads (the struct) to
>>> the 64-bit boundary.
>>
>> Which doesn't answer the "why in the first place" part - I don't
>> see why structure size needs to be a multiple of 64 bits.
>>
> In this patch post,
> 
> http://lists.xen.org/archives/html/xen-devel/2015-07/msg02334.html 
> 
> you had a comment about the structure size, which I think you mean
> the struct size should be a multiple of 64 bits.

Looks like I had got mislead by there being struct
xen_domctl_sched_sedf, but it not being part of the union. I'm
sorry for that.

Jan


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

Re: [Xen-devel] Outreachy 2016

[Roger Pau Monné]

On Mon, 21 Mar 2016, Paulina Szubarczyk wrote:

> Hi ​Lars, 
> 
> ​Thank you for your answer. I am looking forward to further information from 
> you.

Hello Paulina,

Sorry for the delay, I was away last week and I'm just trying to catch up 
with all my email.

As a first step you should look into installing latest Xen from source on
your prefered Linux distribution, we have a wiki page that should help 
you in doing this: http://wiki.xenproject.org/wiki/Compiling_Xen_From_Source.

You should use the code from the git repository, either the staging or 
master branches should be fine (staging might be a bit unstable, so I 
would recommend starting with master).

In the meantime I will try to search for some bit-sized contributions.

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

Re: [Xen-devel] [PATCH v3 15/28] x86/cpu: Sysctl and common infrastructure for levelling context switching

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> --- a/xen/arch/x86/cpu/common.c
> +++ b/xen/arch/x86/cpu/common.c
> @@ -36,6 +36,12 @@ integer_param("cpuid_mask_ext_ecx", 
> opt_cpuid_mask_ext_ecx);
>  unsigned int opt_cpuid_mask_ext_edx = ~0u;
>  integer_param("cpuid_mask_ext_edx", opt_cpuid_mask_ext_edx);
>  
> +unsigned int __initdata expected_levelling_cap;

Especially for an __initdata item to be non-static, its use(s) should
be visible in a patch adding such a variable.

> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -618,6 +618,37 @@ void microcode_set_module(unsigned int);
>  int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
>  int microcode_resume_cpu(unsigned int cpu);
>  
> +#define LCAP_faulting XEN_SYSCTL_CPU_LEVELCAP_faulting
> +#define LCAP_1cd  (XEN_SYSCTL_CPU_LEVELCAP_ecx |\
> +   XEN_SYSCTL_CPU_LEVELCAP_edx)
> +#define LCAP_e1cd (XEN_SYSCTL_CPU_LEVELCAP_extd_ecx |   \
> +   XEN_SYSCTL_CPU_LEVELCAP_extd_edx)
> +#define LCAP_Da1  XEN_SYSCTL_CPU_LEVELCAP_xsave_eax
> +#define LCAP_6c   XEN_SYSCTL_CPU_LEVELCAP_themal_ecx
> +#define LCAP_7ab0 (XEN_SYSCTL_CPU_LEVELCAP_l7s0_eax |   \
> +   XEN_SYSCTL_CPU_LEVELCAP_17s0_ebx)
> +
> +/*
> + * Expected levelling capabilities (given cpuid vendor/family information),
> + * and levelling capabilities actually available (given MSR probing).
> + */
> +extern unsigned int expected_levelling_cap, levelling_caps;
> +
> +struct cpuidmasks
> +{
> +uint64_t _1cd;
> +uint64_t e1cd;
> +uint64_t Da1;
> +uint64_t _6c;
> +uint64_t _7ab0;
> +};
> +
> +/* Per CPU shadows of masking MSR values, for lazy context switching. */
> +DECLARE_PER_CPU(struct cpuidmasks, cpuidmasks);
> +
> +/* Default masking MSR values, calculated at boot. */
> +extern struct cpuidmasks cpuidmask_defaults;

Would much/all of this not better go into cpuid.h?

> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -766,6 +766,27 @@ struct xen_sysctl_tmem_op {
>  typedef struct xen_sysctl_tmem_op xen_sysctl_tmem_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_tmem_op_t);
>  
> +/*
> + * XEN_SYSCTL_get_cpu_levelling_caps (x86 specific)
> + *
> + * Return hardware capabilities concerning masking or faulting of the cpuid
> + * instruction for PV guests.
> + */
> +struct xen_sysctl_cpu_levelling_caps {
> +#define XEN_SYSCTL_CPU_LEVELCAP_faulting   (1ul <<  0) /* CPUID faulting
> */
> +#define XEN_SYSCTL_CPU_LEVELCAP_ecx(1ul <<  1) /* 0x0001.ecx
> */
> +#define XEN_SYSCTL_CPU_LEVELCAP_edx(1ul <<  2) /* 0x0001.edx
> */
> +#define XEN_SYSCTL_CPU_LEVELCAP_extd_ecx   (1ul <<  3) /* 0x8001.ecx
> */
> +#define XEN_SYSCTL_CPU_LEVELCAP_extd_edx   (1ul <<  4) /* 0x8001.edx
> */
> +#define XEN_SYSCTL_CPU_LEVELCAP_xsave_eax  (1ul <<  5) /* 0x000D:1.eax  
> */
> +#define XEN_SYSCTL_CPU_LEVELCAP_themal_ecx (1ul <<  6) /* 0x0006.ecx
> */

thermal

> +#define XEN_SYSCTL_CPU_LEVELCAP_l7s0_eax   (1ul <<  7) /* 0x0007:0.eax  
> */
> +#define XEN_SYSCTL_CPU_LEVELCAP_17s0_ebx   (1ul <<  8) /* 0x0007:0.ebx  
> */

There appears to be a 1 here when I think an l is meant.

Jan


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

Re: [Xen-devel] [PATCH v3 14/28] x86/cpu: Move set_cpumask() calls into c_early_init()

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> Before c/s 44e24f8567 "x86: don't call generic_identify() redundantly", the
> commandline-provided masks would take effect in Xen's view of the features.
> 
> As the masks got applied after the query for features, the redundant call to
> generic_identify() would clobber the pre-masking feature information with the
> post-masking information.
> 
> Move the set_cpumask() calls into c_early_init() so their effects take place
> before the main query for features in generic_identify().
> 
> The cpuid_mask_* command line parameters now limit the entire system, a
> feature XenServer was relying on for testing purposes.  Subsequent changes
> will cause the mask MSRs to be context switched per-domain, removing the need
> to use the command line parameters for heterogeneous levelling purposes.
> 
> Signed-off-by: Andrew Cooper 

As said before, I'm not in agreement with this change at this point in
the series. If you keep it here, I will make it a requirement to not go
in independently ahead of enough of the other patches of the series.

Jan


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

Re: [Xen-devel] [PATCH v3 13/28] xen/x86: Improvements to in-hypervisor cpuid sanity checks

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
>  case 0x8001:
> -/* We expose RDTSCP feature to guest only when
> -   tsc_mode == TSC_MODE_DEFAULT and host_tsc_is_safe() returns 1 */
> -if ( d->arch.tsc_mode != TSC_MODE_DEFAULT ||
> - !host_tsc_is_safe() )
> -*edx &= ~cpufeat_mask(X86_FEATURE_RDTSCP);
> -/* Hide 1GB-superpage feature if we can't emulate it. */
> -if (!hvm_pse1gb_supported(d))
> -*edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);
> -/* Only provide PSE36 when guest runs in 32bit PAE or in long mode */
> -if ( !(hvm_pae_enabled(v) || hvm_long_mode_enabled(v)) )
> -*edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
> -/* Hide data breakpoint extensions if the hardware has no support. */
> -if ( !boot_cpu_has(X86_FEATURE_DBEXT) )
> -*ecx &= ~cpufeat_mask(X86_FEATURE_DBEXT);
> +*ecx &= hvm_featureset[FEATURESET_e1c];
> +*edx &= hvm_featureset[FEATURESET_e1d];
> +
> +/* If emulating Intel, clear the duplicated features in e1d. */
> +if ( d->arch.x86_vendor == X86_VENDOR_INTEL )
> +*edx &= ~CPUID_COMMON_1D_FEATURES;

I think this would better be != X86_VENDOR_AMD, to also cover
VIA.

> +/* OSXSAVE cleared by pv_featureset.  Fast-forward CR4 back in. */
> +if ( (is_pv_domain(currd) && guest_kernel_mode(curr, regs) &&
> +  (this_cpu(cr4) & X86_CR4_OSXSAVE)) ||
> + (curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_OSXSAVE) )
> +c |= cpufeat_mask(X86_FEATURE_OSXSAVE);
> +
> +c |= cpufeat_mask(X86_FEATURE_HYPERVISOR);
>  break;

Is this correct for PVH (which calls here out of vmx.c)? At least
the ->arch.pv_vcpu use unlikely is.

> +/* OSPKE cleared by pv_featureset.  Fast-forward CR4 back in. */
> +if ( curr->arch.pv_vcpu.ctrlreg[4] & X86_CR4_PKE )
> +c |= cpufeat_mask(X86_FEATURE_OSPKE);

That's kind of pointless for PV, and similarly to the above one likely
wrong for PVH.

Jan


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

Re: [Xen-devel] [PATCH v6 11/22] arm/acpi: Prepare EFI system table for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

[...]


diff --git a/xen/arch/arm/efi/efi-dom0.c b/xen/arch/arm/efi/efi-dom0.c
index 90a7699..b8a062c 100644
--- a/xen/arch/arm/efi/efi-dom0.c
+++ b/xen/arch/arm/efi/efi-dom0.c
@@ -48,3 +48,47 @@ size_t __init estimate_efi_size(int mem_nr_banks)

  return size;
  }
+
+#include "../../../common/decompress.h"
+#define XZ_EXTERN STATIC
+#include "../../../common/xz/crc32.c"


All the includes should be at the beginning of the file.


+
+void __init acpi_create_efi_system_table(paddr_t paddr, void *efi_acpi_table,
+ struct membank tbl_add[])
+{
+u64 table_addr, table_size, offset = 0;
+u8 *base_ptr;
+EFI_CONFIGURATION_TABLE *efi_conf_tbl;
+EFI_SYSTEM_TABLE *efi_sys_tbl;
+
+table_addr = paddr + acpi_get_table_offset(tbl_add, TBL_EFIT);
+table_size = sizeof(EFI_SYSTEM_TABLE) + sizeof(EFI_CONFIGURATION_TABLE)
+ + sizeof(xen_efi_fw_vendor);
+base_ptr = efi_acpi_table + acpi_get_table_offset(tbl_add, TBL_EFIT);
+efi_sys_tbl = (EFI_SYSTEM_TABLE *)base_ptr;
+
+efi_sys_tbl->Hdr.Signature = EFI_SYSTEM_TABLE_SIGNATURE;
+/* Specify the revision as 2.5 */
+efi_sys_tbl->Hdr.Revision = (2 << 16 | 50);
+efi_sys_tbl->Hdr.HeaderSize = table_size;
+
+efi_sys_tbl->FirmwareRevision = 1;
+efi_sys_tbl->NumberOfTableEntries = 1;
+offset += sizeof(EFI_SYSTEM_TABLE);
+memcpy((CHAR16 *)(base_ptr + offset), xen_efi_fw_vendor,


Why the cast to CHAR16?


+   sizeof(xen_efi_fw_vendor));
+efi_sys_tbl->FirmwareVendor = (CHAR16 *)(table_addr + offset);
+
+offset += sizeof(xen_efi_fw_vendor);
+efi_conf_tbl = (EFI_CONFIGURATION_TABLE *)(base_ptr + offset);
+efi_conf_tbl->VendorGuid = (EFI_GUID)ACPI_20_TABLE_GUID;
+efi_conf_tbl->VendorTable = (VOID *)tbl_add[TBL_RSDP].start;
+efi_sys_tbl->ConfigurationTable = (EFI_CONFIGURATION_TABLE *)(table_addr
+  + offset);
+xz_crc32_init();
+efi_sys_tbl->Hdr.CRC32 = xz_crc32((uint8_t *)efi_sys_tbl,
+  efi_sys_tbl->Hdr.HeaderSize, 0);
+
+tbl_add[TBL_EFIT].start = table_addr;
+tbl_add[TBL_EFIT].size = table_size;
+}


[...]

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v6 08/22] arm/acpi: Prepare RSDP table for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy RSDP table and replace rsdp->xsdt_physical_address with new address

  ^ the

of XSDT table, so it can point to the right XSDT table.

Signed-off-by: Shannon Zhao 
Acked-by: Stefano Stabellini 


With the 2 changes mentioned:

Acked-by: Julien Grall 


---
  xen/arch/arm/domain_build.c | 36 
  1 file changed, 36 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index f9fe289..ea7d6a5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,38 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static int acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
+{
+
+struct acpi_table_rsdp *rsdp = NULL;
+u64 addr;
+u64 table_size = sizeof(struct acpi_table_rsdp);
+u8 *base_ptr;
+u8 checksum;
+
+addr = acpi_os_get_root_pointer();
+if( !addr )
+panic("Unable to get acpi root pointer\n");


Please avoid panic when it's possible.


+
+rsdp = acpi_os_map_memory(addr, table_size);
+base_ptr = d->arch.efi_acpi_table
+   + acpi_get_table_offset(tbl_add, TBL_RSDP);
+ACPI_MEMCPY(base_ptr, rsdp, table_size);
+acpi_os_unmap_memory(rsdp, table_size);
+
+rsdp = (struct acpi_table_rsdp *)base_ptr;
+/* Replace xsdt_physical_address */
+rsdp->xsdt_physical_address = tbl_add[TBL_XSDT].start;
+checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, rsdp), table_size);
+rsdp->checksum = rsdp->checksum - checksum;
+
+tbl_add[TBL_RSDP].start = d->arch.efi_acpi_gpa
+  + acpi_get_table_offset(tbl_add, TBL_RSDP);
+tbl_add[TBL_RSDP].size = table_size;
+
+return 0;
+}
+
  static void acpi_xsdt_modify_entry(u64 entry[], unsigned long entry_count,
 char *signature, u64 addr)
  {
@@ -1625,6 +1657,10 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  if ( rc != 0 )
  return rc;

+rc = acpi_create_rsdp(d, tbl_add);
+if ( rc != 0 )
+return rc;
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v8 for Xen 4.7 1/4] xen: enable per-VCPU parameter for RTDS

[Chong Li]

On Mon, Mar 21, 2016 at 10:49 AM, Jan Beulich  wrote:
 On 21.03.16 at 16:18,  wrote:
>> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich  wrote:
>> On 18.03.16 at 22:26,  wrote:
 --- a/xen/include/public/domctl.h
 +++ b/xen/include/public/domctl.h
 @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
  #define XEN_SCHEDULER_ARINC653 7
  #define XEN_SCHEDULER_RTDS 8

 -/* Set or get info? */
 +typedef struct xen_domctl_sched_credit {
 +uint16_t weight;
 +uint16_t cap;
 +} xen_domctl_sched_credit_t;
 +
 +typedef struct xen_domctl_sched_credit2 {
 +uint16_t weight;
 +} xen_domctl_sched_credit2_t;
 +
 +typedef struct xen_domctl_sched_rtds {
 +uint32_t period;
 +uint32_t budget;
 +} xen_domctl_sched_rtds_t;
 +
 +typedef struct xen_domctl_schedparam_vcpu {
 +union {
 +xen_domctl_sched_credit_t credit;
 +xen_domctl_sched_credit2_t credit2;
 +xen_domctl_sched_rtds_t rtds;
 +} u;
 +uint32_t vcpuid;
 +uint16_t padding[2];
>>>
>>> So why uint16_t[2] instead of just uint32_t? And what's the
>>> padding needed for in the first place?
>>
>> You're right. It's better to use uint32_t, which pads (the struct) to
>> the 64-bit boundary.
>
> Which doesn't answer the "why in the first place" part - I don't
> see why structure size needs to be a multiple of 64 bits.
>
In this patch post,

http://lists.xen.org/archives/html/xen-devel/2015-07/msg02334.html

you had a comment about the structure size, which I think you mean
the struct size should be a multiple of 64 bits.

Chong


-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

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

Re: [Xen-devel] [PATCH v6 10/22] arm/acpi: Map all other tables for Dom0

[Julien Grall]

Title: Map all other tables into DOM0 memory.

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Map all other tables to Dom0 using 1:1 mappings.


s/to/into/



Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/domain_build.c | 26 ++
  1 file changed, 26 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ea7d6a5..c71976c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,30 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static void acpi_map_other_tables(struct domain *d)
+{
+int i;
+unsigned long res;
+u64 addr, size;
+
+/* Map all other tables to Dom0 using 1:1 mappings. */


That's not really true. AFAICT, you will map all the ACPI tables, 
included the ones that have been discarded.



+for( i = 0; i < acpi_gbl_root_table_list.count; i++ )
+{
+addr = acpi_gbl_root_table_list.tables[i].address;
+size = acpi_gbl_root_table_list.tables[i].length;
+res = map_regions_rw(d,
+ paddr_to_pfn(addr & PAGE_MASK),
+ DIV_ROUND_UP(size, PAGE_SIZE),
+ paddr_to_pfn(addr & PAGE_MASK));
+if ( res )
+{
+ panic(XENLOG_ERR "Unable to map 0x%"PRIx64
+   " - 0x%"PRIx64" in domain \n",
+   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);


"Unable to map ACPI region ..." to differentiate with the error message 
in map_range_to_domain.



+}
+}
+}
+
  static int acpi_create_rsdp(struct domain *d, struct membank tbl_add[])
  {

@@ -1661,6 +1685,8 @@ static int prepare_acpi(struct domain *d, struct 
kernel_info *kinfo)
  if ( rc != 0 )
  return rc;

+acpi_map_other_tables(d);
+
  return 0;
  }
  #else



Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v6 09/22] arm/p2m: Add helper functions to map memory regions

[Julien Grall]

Title: to map/unmap

On 17/03/2016 09:40, Shannon Zhao wrote:

diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index 433952a..17be6ad 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -144,6 +144,16 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, 
xen_pfn_t end_mfn);
  /* Setup p2m RAM mapping for domain d from start-end. */
  int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);

+int map_regions_rw(struct domain *d,
+unsigned long start_gfn,
+unsigned long nr_mfns,
+unsigned long mfn);


From the commit message, this function will map the region read-write 
and cacheable.


But it's not clear from the name. Please either rename the function or 
document it in the code.



+
+int unmap_regions_rw(struct domain *d,
+unsigned long start_gfn,
+unsigned long nr_mfns,
+unsigned long mfn);
+
  int guest_physmap_add_entry(struct domain *d,
  unsigned long gfn,
  unsigned long mfn,



Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v6 05/22] arm/acpi: Prepare MADT table for Dom0

[Jan Beulich]

>>> On 21.03.16 at 16:26,  wrote:
> On 17/03/2016 09:40, Shannon Zhao wrote:
>> +gicd = container_of(header, struct acpi_madt_generic_distributor, 
>> header);
>> +ACPI_MEMCPY(base_ptr + table_size, gicd,
>> +sizeof(struct acpi_madt_generic_distributor));
>> +table_size += sizeof(struct acpi_madt_generic_distributor);
>> +
>> +/* Add other subtables*/
> 
> NIT: Missing space before */

And a full stop.

Jan


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

Re: [Xen-devel] [PATCH v8 for Xen 4.7 1/4] xen: enable per-VCPU parameter for RTDS

[Jan Beulich]

>>> On 21.03.16 at 16:18,  wrote:
> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich  wrote:
> On 18.03.16 at 22:26,  wrote:
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
>>>  #define XEN_SCHEDULER_ARINC653 7
>>>  #define XEN_SCHEDULER_RTDS 8
>>>
>>> -/* Set or get info? */
>>> +typedef struct xen_domctl_sched_credit {
>>> +uint16_t weight;
>>> +uint16_t cap;
>>> +} xen_domctl_sched_credit_t;
>>> +
>>> +typedef struct xen_domctl_sched_credit2 {
>>> +uint16_t weight;
>>> +} xen_domctl_sched_credit2_t;
>>> +
>>> +typedef struct xen_domctl_sched_rtds {
>>> +uint32_t period;
>>> +uint32_t budget;
>>> +} xen_domctl_sched_rtds_t;
>>> +
>>> +typedef struct xen_domctl_schedparam_vcpu {
>>> +union {
>>> +xen_domctl_sched_credit_t credit;
>>> +xen_domctl_sched_credit2_t credit2;
>>> +xen_domctl_sched_rtds_t rtds;
>>> +} u;
>>> +uint32_t vcpuid;
>>> +uint16_t padding[2];
>>
>> So why uint16_t[2] instead of just uint32_t? And what's the
>> padding needed for in the first place?
> 
> You're right. It's better to use uint32_t, which pads (the struct) to
> the 64-bit boundary.

Which doesn't answer the "why in the first place" part - I don't
see why structure size needs to be a multiple of 64 bits.

>> Also, while for a domctl it's
>> not as strictly needed as for other hypercalls, checking that all
>> padding fields are zero would still seem to be rather desirable.
>>
> 
> Do you mean:
> 
> +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
> +case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
> +if ( op->u.v.padding !=0 )
> +{
> +rc = -EINVAL;
> +break;
> +}
> +while ( index < op->u.v.nr_vcpus )
> +{
> +if ( copy_from_guest_offset(&local_sched,
> +op->u.v.vcpus, index, 1) )
> +{
> +rc = -EFAULT;
> +break;
> +}
> +if ( local_sched.padding != 0 )
> +{
> +rc = -EINVAL;
> +break;
> +}
> +if ( local_sched.vcpuid >= d->max_vcpus ||
> + d->vcpu[local_sched.vcpuid] == NULL )
> +{
> +rc = -EINVAL;
> +break;
> +}
> 
> ?

Yes, something along those lines.

Jan


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

Re: [Xen-devel] [PATCH v8 for Xen 4.7 1/4] xen: enable per-VCPU parameter for RTDS

[George Dunlap]

On 21/03/16 15:18, Chong Li wrote:
> On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich  wrote:
> On 18.03.16 at 22:26,  wrote:
>>> Add XEN_DOMCTL_SCHEDOP_getvcpuinfo and _putvcpuinfo hypercalls
>>> to independently get and set the scheduling parameters of each
>>> vCPU of a domain
>>>
>>> Signed-off-by: Chong Li 
>>> Signed-off-by: Meng Xu 
>>> Signed-off-by: Sisu Xi 
>>>
>>> ---
>>> Changes on PATCH v7:
>>> 1) A bug in case XEN_DOMCTL_SCHEDOP_getinfo (in Xen 4.6) is fixed:
>>> The default PERIOD or BUDGET should be divided by MICROSECS(1),
>>> before returned to upper caller.
>>
>> Seems like there's still some misunderstanding here: Anything
>> past the first --- won't make it into the repo, yet the description
>> of what bug you fix should end up there.
> 
> I see. Do I put the "bug fix" before "Signed-off-by ..." or after it?

You need to put everything important for understanding this changeset in
the changset description.  So in this case, you'd probably add a
paragraph like this:

"Also fix a bug in XEN_DOMCTL_SCHEDOP_getinfo, where PERIOD and BUDGET
are not divided by MICROSECS(1) before being returned to the caller."

(And that would be before the SoB.)

 -George



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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 66399
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 66399
 test-armhf-armhf-xl-cubietruck 15 guest-start/debian.repeat fail REGR. vs. 
66399
 test-armhf-armhf-xl  15 guest-start/debian.repeat fail REGR. vs. 66399
 test-armhf-armhf-xl-credit2 15 guest-start/debian.repeat fail in 86654 REGR. 
vs. 66399

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl  11 guest-startfail in 86587 pass in 86761
 test-armhf-armhf-xl-rtds 11 guest-start fail pass in 86587
 test-armhf-armhf-xl-credit2  11 guest-start fail pass in 86654
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 15 guest-localmigrate/x10 fail 
pass in 86654
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat  fail pass in 86654

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

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

version targeted for testing:
 linux7f30737678023b5becaf0e2e012665f71b886a7d
baseline version:
 linux07cc49f66973f49a391c91bf4b158fa0f2562ca8

Last test of basis66399  2015-12-15 18:20:39 Z   96 days
Failing since 78925  2016-01-24 13:50:39 Z   57 days   61 attempts
Testing same since86587  2016-03-18 16:11:01 Z2 days3 attempts


494 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass

Re: [Xen-devel] [PATCH v3 11/28] xen/x86: Clear dependent features when clearing a cpu cap

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> When clearing a cpu cap, clear all dependent features.  This avoids having a
> featureset with intermediate features disabled, but leaf features enabled.
> 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 


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

Re: [Xen-devel] [libvirt] [PATCH V2 0/4] Extend to a tristate

[Jim Fehlig]

On 03/21/2016 06:49 AM, Ján Tomko wrote:
> On Mon, Feb 29, 2016 at 09:00:44PM -0700, Jim Fehlig wrote:
>> An expanded V2 of
>>
>> https://www.redhat.com/archives/libvir-list/2016-February/msg00140.html
>>
>> In V2, the  feature is extended with a state='on|off' attribute to
>> allow differentiating the 'on' and 'off' states with not set (or hypervisor
>> default).
>>
>> Obviously post 1.3.2 material. See individual patches for details.
>>
>> Jim Fehlig (4):
>>   conf: add 'state' attribute to  feature
>>   xenconfig: change 'hap' setting to align with Xen behavior
>>   Xen drivers: show hap enabled by default in capabilities
>>   libxl: support enabling and disabling  feature
>>
>>  docs/formatdomain.html.in  |  6 ++-
>>  docs/schemas/domaincommon.rng  |  6 ++-
>>  src/conf/domain_conf.c |  4 +-
>>  src/libxl/libxl_conf.c | 20 ++--
>>  src/xen/xen_hypervisor.c   |  2 +-
>>  src/xenconfig/xen_common.c | 14 ++---
>>  .../test-disk-positional-parms-full.cfg|  1 -
>>  .../test-disk-positional-parms-partial.cfg |  1 -
>>  ...est-fullvirt-direct-kernel-boot-bogus-extra.cfg |  1 -
>>  .../test-fullvirt-direct-kernel-boot-extra.cfg |  1 -
>>  .../test-fullvirt-direct-kernel-boot.cfg   |  1 -
>>  tests/xlconfigdata/test-fullvirt-multiusb.cfg  |  1 -
>>  tests/xlconfigdata/test-fullvirt-nohap.cfg | 26 ++
>>  tests/xlconfigdata/test-fullvirt-nohap.xml | 59 
>> ++
>>  tests/xlconfigdata/test-new-disk.cfg   |  1 -
>>  tests/xlconfigdata/test-rbd-multihost-noauth.cfg   |  1 -
>>  tests/xlconfigdata/test-spice-features.cfg |  1 -
>>  tests/xlconfigdata/test-spice.cfg  |  1 -
>>  tests/xlconfigdata/test-vif-rate.cfg   |  1 -
>>  tests/xlconfigtest.c   |  1 +
>>  tests/xmconfigdata/test-escape-paths.cfg   |  1 -
>>  .../xmconfigdata/test-fullvirt-default-feature.cfg |  1 -
>>  tests/xmconfigdata/test-fullvirt-force-hpet.cfg|  1 -
>>  tests/xmconfigdata/test-fullvirt-force-nohpet.cfg  |  1 -
>>  tests/xmconfigdata/test-fullvirt-localtime.cfg |  1 -
>>  tests/xmconfigdata/test-fullvirt-net-netfront.cfg  |  1 -
>>  tests/xmconfigdata/test-fullvirt-new-cdrom.cfg |  1 -
>>  tests/xmconfigdata/test-fullvirt-nohap.cfg | 28 ++
>>  tests/xmconfigdata/test-fullvirt-nohap.xml | 51 +++
>>  tests/xmconfigdata/test-fullvirt-parallel-tcp.cfg  |  1 -
>>  .../test-fullvirt-serial-dev-2-ports.cfg   |  1 -
>>  .../test-fullvirt-serial-dev-2nd-port.cfg  |  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-file.cfg   |  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-null.cfg   |  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-pipe.cfg   |  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-pty.cfg|  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-stdio.cfg  |  1 -
>>  .../test-fullvirt-serial-tcp-telnet.cfg|  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-tcp.cfg|  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-udp.cfg|  1 -
>>  tests/xmconfigdata/test-fullvirt-serial-unix.cfg   |  1 -
>>  tests/xmconfigdata/test-fullvirt-sound.cfg |  1 -
>>  tests/xmconfigdata/test-fullvirt-usbmouse.cfg  |  1 -
>>  tests/xmconfigdata/test-fullvirt-usbtablet.cfg |  1 -
>>  tests/xmconfigdata/test-fullvirt-utc.cfg   |  1 -
>>  tests/xmconfigdata/test-no-source-cdrom.cfg|  1 -
>>  tests/xmconfigdata/test-pci-devs.cfg   |  1 -
>>  tests/xmconfigtest.c   |  1 +
>>  48 files changed, 202 insertions(+), 52 deletions(-)
>>  create mode 100644 tests/xlconfigdata/test-fullvirt-nohap.cfg
>>  create mode 100644 tests/xlconfigdata/test-fullvirt-nohap.xml
>>  create mode 100755 tests/xmconfigdata/test-fullvirt-nohap.cfg
>>  create mode 100644 tests/xmconfigdata/test-fullvirt-nohap.xml
>>
> ACK series

Thanks a lot for taking a look, much appreciated! Pushed now.

Regards,
Jim


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

Re: [Xen-devel] [PATCH v6 07/22] arm/acpi: Prepare XSDT table for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy and modify XSDT table before passing it to Dom0. Repalce the entry


s/Repalce/Replace/


value of the copied table. Add a new entry for STAO table as well. And
keep entry value of other reused tables unchanged.

Signed-off-by: Shannon Zhao 
Acked-by: Stefano Stabellini 


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 v3 10/28] xen/x86: Generate deep dependencies of features

[Jan Beulich]

>>> On 15.03.16 at 16:35,  wrote:
> @@ -18,12 +19,34 @@ uint32_t __read_mostly hvm_featureset[FSCAPINTS];
>  
>  static void __init sanitise_featureset(uint32_t *fs)
>  {
> +uint32_t disabled_features[FSCAPINTS];
>  unsigned int i;
>  
>  for ( i = 0; i < FSCAPINTS; ++i )
>  {
>  /* Clamp to known mask. */
>  fs[i] &= known_features[i];
> +
> +/*
> + * Identify which features with deep dependencies have been
> + * disabled.
> + */
> +disabled_features[i] = ~fs[i] & deep_features[i];
> +}
> +
> +for_each_set_bit(i, (void *)disabled_features,

I'm afraid the cast here is not really valid: If FSCAPINTS is odd,
there would be an (admittedly benign) out of bounds access as
a result. For fully defined behavior I think you need to kind of
open code for_each_set_bit() here (and, if there are any, in
similar constructs).

> +deps = {
> +# FPU is taken to mean support for the x87 regisers as well as the
> +# instructions.  MMX is documented to alias the %MM registers over 
> the
> +# x87 %ST registers in hardware.
> +FPU: [MMX],
> +
> +# The PSE36 feature indicates that reserved bits in a PSE superpage
> +# may be used as extra physical address bits.
> +PSE: [PSE36],
> +
> +# Entering Long Mode requires that %CR4.PAE is set.  The NX pagetable
> +# bit is only representable in the 64bit PTE format offered by PAE.
> +PAE: [LM, NX],

Also PKU?

> +# APIC is special, but X2APIC does depend on APIC being available in
> +# the first place.
> +APIC: [X2APIC],
> +
> +# AMD built MMXExtentions and 3DNow as extentions to MMX.
> +MMX: [MMXEXT, _3DNOW],
> +
> +# The FXSAVE/FXRSTOR instructions were introduced into hardware 
> before
> +# SSE, which is why they behave differently based on %CR4.OSFXSAVE 
> and
> +# have their own feature bit.  AMD however introduce the Fast FXSR
> +# feature as an optimisation.
> +FXSR: [FFXSR],

Also SSE.

> +# SSE is taken to mean support for the %XMM registers as well as the
> +# instructions.  The SSE extentions were re-specified as core for
> +# 64bit support.
> +SSE: [SSE2, LM],

I think listing LM here is pointless when it's also listed with SSE2.

> +# SSE2 was also re-specified as core for 64bit.  The AESNI and SHA
> +# instruction groups are documented to require checking for SSE2
> +# support as a prerequisite.
> +SSE2: [SSE3, LM, AESNI, SHA],
> +
> +# AMD K10 processors has SSE3 and SSE4A.  Bobcat/Barcelona processors
> +# subsequently included SSSE3, and Bulldozer subsequently included
> +# SSE4_1.  Intel have never shipped SSE4A.
> +SSE3: [SSSE3, SSE4_1, SSE4A],
> +SSE4_1: [SSE4_2],

Numerically these dependencies make sense, but do they really build
on top of one another? The last ones (SSE4.1 and SSE4.2) are
particularly examples of things that look more like siblings than
ancestors, as do AESNI and SHA wrt SSE2. Otherwise BMI2 would
likely need to be considered dependent on BMI1...

> +# XSAVE is an extra set of instructions for state management, but
> +# doesn't constitue new state itself.  Some of the dependent features
> +# are instructions built on top of base XSAVE, while others are new
> +# instruction groups which are specified to require XSAVE for state
> +# management.
> +XSAVE: [XSAVEOPT, XSAVEC, XGETBV1, XSAVES, AVX, MPX],

Also PKU again? And LWP?

Jan


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

Re: [Xen-devel] [PATCH 1/5] public/xen.h: add flags field to vcpu_time_info

[Joao Martins]

On 03/21/2016 03:27 PM, Andrew Cooper wrote:
> On 21/03/16 15:10, Jan Beulich wrote:
> On 17.03.16 at 17:12,  wrote:
>>> --- a/xen/include/public/xen.h
>>> +++ b/xen/include/public/xen.h
>>> @@ -614,10 +614,14 @@ struct vcpu_time_info {
>>>   */
>>>  uint32_t tsc_to_system_mul;
>>>  int8_t   tsc_shift;
>>> -int8_t   pad1[3];
>>> +int8_t   flags;
>> For use as flags I'm sure this would better be uint8_t.
> 
> Sadly not possible.  Linux have already made the above adjustment
> without (CC'ing xen-devel), so that ABI is set.
> 
I was double checking again and it's my mistake. Both flags and pad fields are
uint8_t. I will fix both in v2, to make sure it's all kept the same on both
header files. My apologies.

Joao

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

Re: [Xen-devel] [PATCH v6 06/22] arm/acpi: Prepare STAO table for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Create STAO table for Dom0. This table is used to tell Dom0 whether it
should ignore UART defined in SPCR table or the ACPI namespace names.

Look at below url for details:
http://wiki.xenproject.org/mediawiki/images/0/02/Status-override-table.pdf

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


With the change mentioned below:

Acked-by: Julien Grall 


---
  xen/arch/arm/domain_build.c | 41 +
  1 file changed, 41 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ed257e0..b369f2e 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,43 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static int acpi_create_stao(struct domain *d, struct membank tbl_add[])
+{
+struct acpi_table_header *table = NULL;
+struct acpi_table_stao *stao = NULL;
+u32 table_size = sizeof(struct acpi_table_stao);
+u32 offset = acpi_get_table_offset(tbl_add, TBL_STAO);
+acpi_status status;
+u8 *base_ptr, checksum;
+
+/* Copy OEM and ASL compiler fields from another table, use MADT */
+status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+if ( ACPI_FAILURE(status) )
+{
+const char *msg = acpi_format_exception(status);
+
+printk("Failed to get MADT table, %s\n", msg);


NIT: You use the same error message in 2 different place. Can you add a 
prefix to help finding the error quicker?



+return -EINVAL;
+}
+
+base_ptr = d->arch.efi_acpi_table + offset;
+ACPI_MEMCPY(base_ptr, table, sizeof(struct acpi_table_header));
+
+stao = (struct acpi_table_stao *)base_ptr;
+ACPI_MEMCPY(stao->header.signature, ACPI_SIG_STAO, 4);
+stao->header.revision = 1;
+stao->header.length = table_size;
+stao->ignore_uart = 1;
+checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, stao), table_size);
+stao->header.checksum -= checksum;
+
+tbl_add[TBL_STAO].start = d->arch.efi_acpi_gpa + offset;
+tbl_add[TBL_STAO].size = table_size;
+
+return 0;
+}
+
  static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
  {
  struct acpi_table_header *table = NULL;


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 4/8] libxl: Move check for local access to a funciton

[George Dunlap]

On 17/03/16 18:11, Ian Jackson wrote:
> George Dunlap writes ("[PATCH 4/8] libxl: Move check for local access to a 
> funciton"):
>> +static char * libxl__device_disk_find_local_path(libxl__gc *gc, 
>> + const libxl_device_disk 
>> *disk) {
> ...
>> +if (disk->format == LIBXL_DISK_FORMAT_RAW
>> +&& disk->script == NULL
>> +&& disk->pdev_path) {
> 
> libxl__device_disk_local_initiate_attach asserts pdev_path.  Do you
> foresee it maybe being NULL and if so how ?

I think I must have been copying the existing code, which checks for
pdev_path of disk (rather than in_disk, which is what's being passed to
this function).

In the next patch this will also be called from the devicemodel code,
but that code dereferences disk[i].pdev_path (implicitly assuming that
it will never be null), so I think we can drop this check.

Ack on the other two comments.

 -George


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

Re: [Xen-devel] [PATCH 1/5] public/xen.h: add flags field to vcpu_time_info

[Andrew Cooper]

On 21/03/16 15:10, Jan Beulich wrote:
 On 17.03.16 at 17:12,  wrote:
>> --- a/xen/include/public/xen.h
>> +++ b/xen/include/public/xen.h
>> @@ -614,10 +614,14 @@ struct vcpu_time_info {
>>   */
>>  uint32_t tsc_to_system_mul;
>>  int8_t   tsc_shift;
>> -int8_t   pad1[3];
>> +int8_t   flags;
> For use as flags I'm sure this would better be uint8_t.

Sadly not possible.  Linux have already made the above adjustment
without (CC'ing xen-devel), so that ABI is set.

~Andrew

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

Re: [Xen-devel] [PATCH v6 05/22] arm/acpi: Prepare MADT table for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy main MADT table contents and distributor subtable from physical
ACPI MADT table. Make other subtables through the callback of
gic_hw_ops.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


With the 2 changes mentioned below:

Acked-by: Julien Grall 


---
  xen/arch/arm/domain_build.c | 50 +
  1 file changed, 50 insertions(+)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d9b7213..ed257e0 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1357,6 +1357,52 @@ static int prepare_dtb(struct domain *d, struct 
kernel_info *kinfo)
  }

  #ifdef CONFIG_ACPI
+static int acpi_create_madt(struct domain *d, struct membank tbl_add[])
+{
+struct acpi_table_header *table = NULL;
+struct acpi_table_madt *madt = NULL;
+struct acpi_subtable_header *header;
+struct acpi_madt_generic_distributor *gicd;
+u32 table_size = sizeof(struct acpi_table_madt);
+u32 offset = acpi_get_table_offset(tbl_add, TBL_MADT);
+acpi_status status;
+u8 *base_ptr, checksum;
+
+status = acpi_get_table(ACPI_SIG_MADT, 0, &table);
+
+if ( ACPI_FAILURE(status) )
+{
+const char *msg = acpi_format_exception(status);
+
+printk("Failed to get MADT table, %s\n", msg);
+return -EINVAL;
+}
+
+base_ptr = d->arch.efi_acpi_table + offset;
+ACPI_MEMCPY(base_ptr, table, table_size);
+
+/* Add Generic Distributor */
+header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR, 0);
+if ( !header )
+panic("Can't get GICD entry");


Please avoid panic when it's possible to return an error.


+gicd = container_of(header, struct acpi_madt_generic_distributor, header);
+ACPI_MEMCPY(base_ptr + table_size, gicd,
+sizeof(struct acpi_madt_generic_distributor));
+table_size += sizeof(struct acpi_madt_generic_distributor);
+
+/* Add other subtables*/


NIT: Missing space before */


+table_size += gic_make_hwdom_madt(d, offset + table_size);
+
+madt = (struct acpi_table_madt *)base_ptr;
+madt->header.length = table_size;
+checksum = acpi_tb_checksum(ACPI_CAST_PTR(u8, madt), table_size);
+madt->header.checksum -= checksum;
+
+tbl_add[TBL_MADT].start = d->arch.efi_acpi_gpa + offset;
+tbl_add[TBL_MADT].size = table_size;
+
+return 0;
+}

  static int acpi_create_fadt(struct domain *d, struct membank tbl_add[])
  {


[...]

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v8 for Xen 4.7 1/4] xen: enable per-VCPU parameter for RTDS

[Chong Li]

On Mon, Mar 21, 2016 at 8:35 AM, Jan Beulich  wrote:
 On 18.03.16 at 22:26,  wrote:
>> Add XEN_DOMCTL_SCHEDOP_getvcpuinfo and _putvcpuinfo hypercalls
>> to independently get and set the scheduling parameters of each
>> vCPU of a domain
>>
>> Signed-off-by: Chong Li 
>> Signed-off-by: Meng Xu 
>> Signed-off-by: Sisu Xi 
>>
>> ---
>> Changes on PATCH v7:
>> 1) A bug in case XEN_DOMCTL_SCHEDOP_getinfo (in Xen 4.6) is fixed:
>> The default PERIOD or BUDGET should be divided by MICROSECS(1),
>> before returned to upper caller.
>
> Seems like there's still some misunderstanding here: Anything
> past the first --- won't make it into the repo, yet the description
> of what bug you fix should end up there.

I see. Do I put the "bug fix" before "Signed-off-by ..." or after it?

>
>> --- a/xen/include/public/domctl.h
>> +++ b/xen/include/public/domctl.h
>> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
>>  #define XEN_SCHEDULER_ARINC653 7
>>  #define XEN_SCHEDULER_RTDS 8
>>
>> -/* Set or get info? */
>> +typedef struct xen_domctl_sched_credit {
>> +uint16_t weight;
>> +uint16_t cap;
>> +} xen_domctl_sched_credit_t;
>> +
>> +typedef struct xen_domctl_sched_credit2 {
>> +uint16_t weight;
>> +} xen_domctl_sched_credit2_t;
>> +
>> +typedef struct xen_domctl_sched_rtds {
>> +uint32_t period;
>> +uint32_t budget;
>> +} xen_domctl_sched_rtds_t;
>> +
>> +typedef struct xen_domctl_schedparam_vcpu {
>> +union {
>> +xen_domctl_sched_credit_t credit;
>> +xen_domctl_sched_credit2_t credit2;
>> +xen_domctl_sched_rtds_t rtds;
>> +} u;
>> +uint32_t vcpuid;
>> +uint16_t padding[2];
>
> So why uint16_t[2] instead of just uint32_t? And what's the
> padding needed for in the first place?

You're right. It's better to use uint32_t, which pads (the struct) to
the 64-bit boundary.

> Also, while for a domctl it's
> not as strictly needed as for other hypercalls, checking that all
> padding fields are zero would still seem to be rather desirable.
>

Do you mean:

+case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
+case XEN_DOMCTL_SCHEDOP_putvcpuinfo:
+if ( op->u.v.padding !=0 )
+{
+rc = -EINVAL;
+break;
+}
+while ( index < op->u.v.nr_vcpus )
+{
+if ( copy_from_guest_offset(&local_sched,
+op->u.v.vcpus, index, 1) )
+{
+rc = -EFAULT;
+break;
+}
+if ( local_sched.padding != 0 )
+{
+rc = -EINVAL;
+break;
+}
+if ( local_sched.vcpuid >= d->max_vcpus ||
+ d->vcpu[local_sched.vcpuid] == NULL )
+{
+rc = -EINVAL;
+break;
+}

?

Thanks,
Chong


-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

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

Re: [Xen-devel] [PATCH v6 04/22] arm/gic: Add a new callback for creating MADT table for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Add a new member in gic_hw_operations which is used to creat MADT table


s/create/create/


for Dom0.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 
---
  xen/arch/arm/gic-v2.c | 34 ++
  xen/arch/arm/gic-v3.c | 47 +++
  xen/arch/arm/gic.c|  5 +
  xen/include/asm-arm/gic.h |  3 +++
  4 files changed, 89 insertions(+)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 0fcb894..02db5f2 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -685,6 +685,35 @@ static void __init gicv2_dt_init(void)
  }

  #ifdef CONFIG_ACPI
+static u32 gicv2_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+struct acpi_subtable_header *header;
+struct acpi_madt_generic_interrupt *host_gicc, *gicc;
+u32 i, size, table_len = 0;
+u8 *base_ptr = d->arch.efi_acpi_table + offset;
+
+header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+if ( !header )
+panic("Can't get GICC entry");


I would prefer if you return an error here. In the future we may want to 
support hardware domain creation later (see the xen option hardware_dom).



+host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
+ header);
+
+size = sizeof(struct acpi_madt_generic_interrupt);
+/* Add Generic Interrupt */
+for ( i = 0; i < d->max_vcpus; i++ )
+{
+gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
+ACPI_MEMCPY(gicc, host_gicc, size);
+gicc->cpu_interface_number = i;
+gicc->uid = i;
+gicc->flags = ACPI_MADT_ENABLED;
+gicc->arm_mpidr = vcpuid_to_vaffinity(i);


What about the other fields?

* parking_version: DOM0 will always use PSCI => should be set to 0
* performance_interrupt: There is currently PMU support for DOM0
* gic{v,h}_base_address/vgic_interrupt: they are used by Xen => should 
not be exposed to the guest



+table_len += size;
+}
+
+return table_len;
+}
+
  static int __init
  gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
  const unsigned long end)


[...]


diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index f83fd88..d9fce4b 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1236,6 +1236,48 @@ static void __init gicv3_dt_init(void)
  }

  #ifdef CONFIG_ACPI
+static u32 gicv3_make_hwdom_madt(const struct domain *d, u32 offset)
+{
+struct acpi_subtable_header *header;
+struct acpi_madt_generic_interrupt *host_gicc, *gicc;
+struct acpi_madt_generic_redistributor *gicr;
+u8 *base_ptr = d->arch.efi_acpi_table + offset;
+u32 i, table_len = 0, size;
+
+/* Add Generic Interrupt */
+header = acpi_table_get_entry_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT, 0);
+if ( !header )
+panic("Can't get GICC entry");
+host_gicc = container_of(header, struct acpi_madt_generic_interrupt,
+ header);
+
+size = sizeof(struct acpi_madt_generic_interrupt);
+for ( i = 0; i < d->max_vcpus; i++ )
+{
+gicc = (struct acpi_madt_generic_interrupt *)(base_ptr + table_len);
+ACPI_MEMCPY(gicc, host_gicc, size);
+gicc->cpu_interface_number = i;
+gicc->uid = i;
+gicc->flags = ACPI_MADT_ENABLED;
+gicc->arm_mpidr = vcpuid_to_vaffinity(i);
+table_len += size;


Ditto for the fields.

Furthermore, you use the Generic Redistributor table to describe the 
redistributor regions. So the field gicr_base_address should be set to 0.



+}
+
+/* Add Generic Redistributor */
+size = sizeof(struct acpi_madt_generic_redistributor);
+for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
+{
+gicr = (struct acpi_madt_generic_redistributor *)(base_ptr + 
table_len);
+gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
+gicr->header.length = size;
+gicr->base_address = d->arch.vgic.rdist_regions[i].base;
+gicr->length = d->arch.vgic.rdist_regions[i].size;
+table_len += size;
+}
+
+return table_len;
+}
+
  static int __init
  gic_acpi_parse_madt_cpu(struct acpi_subtable_header *header,
  const unsigned long end)


[...]

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2 0/5] xen: avoid module usage in non-modular code

[David Vrabel]

On 20/03/16 00:23, Paul Gortmaker wrote:
> [[PATCH v2 0/5] xen: avoid module usage in non-modular code] On 21/02/2016 
> (Sun 19:06) Paul Gortmaker wrote:
> 
>> This series of commits is a part of a larger project to ensure
>> people don't reference modular support functions in non-modular
>> code.  Overall there was roughly 5k lines of dead code in the
>> kernel due to this.  So far we've fixed several areas, like tty,
>> x86, net, ... and we continue to work on other areas.
> 
> Just wondering if this is still pending for this merge window; Stefano
> had reviewed the two commits he wanted changed vs. v1 and this v2 was
> sent approximately a month ago w/o any further change requests.

Sorry. While I was checking for pending series for 4.6 I accidentally
looked at v1 and saw outstanding comments and skipped it.

I've now applied this series, thanks.

David

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

Re: [Xen-devel] [PATCH 1/5] public/xen.h: add flags field to vcpu_time_info

[Jan Beulich]

>>> On 17.03.16 at 17:12,  wrote:
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -614,10 +614,14 @@ struct vcpu_time_info {
>   */
>  uint32_t tsc_to_system_mul;
>  int8_t   tsc_shift;
> -int8_t   pad1[3];
> +int8_t   flags;

For use as flags I'm sure this would better be uint8_t.

Jan


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

Re: [Xen-devel] [PATCH 03/16] xen: sched: make implementing .alloc_pdata optional

[Jan Beulich]

>>> On 21.03.16 at 15:48,  wrote:
> On 18/03/16 20:04, Dario Faggioli wrote:
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -1491,9 +1491,9 @@ static int cpu_schedule_up(unsigned int cpu)
>>  if ( idle_vcpu[cpu] == NULL )
>>  return -ENOMEM;
>>  
>> -if ( (ops.alloc_pdata != NULL) &&
>> - ((sd->sched_priv = ops.alloc_pdata(&ops, cpu)) == NULL) )
>> -return -ENOMEM;
>> +sd->sched_priv = SCHED_OP(&ops, alloc_pdata, cpu);
>> +if ( IS_ERR(sd->sched_priv) )
>> +return PTR_ERR(sd->sched_priv);
> 
> Calling xfree() with an IS_ERR() value might be a bad idea.
> Either you need to set sd->sched_priv to NULL in error case or you
> modify xfree() to return immediately not only in the NULL case, but
> in the IS_ERR() case as well.

The latter option is a no-go imo.

Jan


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

Re: [Xen-devel] [PATCH v4 08/34] vmap: Make the while loop less fishy.

[Jan Beulich]

>>> On 21.03.16 at 15:22,  wrote:
> Or to take a different tack: I understand that you don't think there's
> no particular benefit to adding a comment in cases like this; could
> you explain to me why you think it would have a significant cost?

There's no significant cost here. Yet I do think that commenting the
obvious is not really helpful (or else we end up with more comments
than there is actual code; some may consider this a good thing, but
I'm of the opinion that this would only serve obfuscating the code).
Plus - as said in various other contexts - I'm in favor of consistency,
i.e. I would think that if these constructs warrant a comment, all
of them should get one. The fall through example you gave for
switch statements is actually a good one: There the comments
silence Coverity. If comments here had the same effect, I think I'd
have no reservations against adding such, yet I don't think they
do.

In the end it boils down to: I'm not going to ack any such change
myself, but I'm also not going to stand in the way of other
maintainers ack-ing such comments getting added. What I would
object to though is calling such constructs "fishy" in commit titles,
commit messages, or the comments being added.

Jan


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

Re: [Xen-devel] List of projects for 4.7

[Wei Liu]

On Mon, Mar 21, 2016 at 06:21:20AM +0100, Juergen Gross wrote:
[...]
> > 4. Load bios via toolstack
> 
> What about my pending "pin override" tools related patches? Hypervisor
> parts are already committed.
> 

That was omitted, because as far as I can tell all toolstack patches
were already acked or reviewed -- you probably should prod Ian to commit
it.

Wei.

> 
> Juergen

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

Re: [Xen-devel] [PATCH 10/16] xen: sched: improve credit2 bootparams' scope, placement and signedness

[Juergen Gross]

On 18/03/16 20:05, Dario Faggioli wrote:
> From: Uma Sharma 
> 
> and, while we are adjusting signedness of opt_load_window_shift,
> make also prv->load_window_shift unsigned, as approapriate.
> 
> Signed-off-by: Uma Sharma 
> Signed-off-by: Dario Faggioli 
> ---
> Cc: George Dunlap 
> Cc: Juergen Gross 
> Cc: Jan Beulich 
> ---
> Patch has changed, so I'm not sticking any tag v1 received
> (namely, Juergen's Reviewed-by:).

Adding it again:

Reviewed-by: Juergen Gross 

> ---
> Changes from v1:
>  * improve signedness as well, as requested during review.
> ---
>  xen/common/sched_credit2.c |   10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index 64fb028..2fd4175 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -162,7 +162,7 @@
>  #define CSFLAG_runq_migrate_request (1<<__CSFLAG_runq_migrate_request)
>  
>  
> -int opt_migrate_resist=500;
> +static unsigned int __read_mostly opt_migrate_resist = 500;
>  integer_param("sched_credit2_migrate_resist", opt_migrate_resist);
>  
>  /*
> @@ -185,12 +185,12 @@ integer_param("sched_credit2_migrate_resist", 
> opt_migrate_resist);
>   *   to a load of 1.
>   */
>  #define LOADAVG_GRANULARITY_SHIFT (10)
> -int opt_load_window_shift=18;
> +static unsigned int __read_mostly opt_load_window_shift = 18;
>  #define  LOADAVG_WINDOW_SHIFT_MIN 4
>  integer_param("credit2_load_window_shift", opt_load_window_shift);
> -int opt_underload_balance_tolerance=0;
> +static int __read_mostly opt_underload_balance_tolerance = 0;
>  integer_param("credit2_balance_under", opt_underload_balance_tolerance);
> -int opt_overload_balance_tolerance=-3;
> +static int __read_mostly opt_overload_balance_tolerance = -3;
>  integer_param("credit2_balance_over", opt_overload_balance_tolerance);
>  
>  /*
> @@ -227,7 +227,7 @@ struct csched2_private {
>  cpumask_t active_queues; /* Queues which may have active cpus */
>  struct csched2_runqueue_data rqd[NR_CPUS];
>  
> -int load_window_shift;
> +unsigned int load_window_shift;
>  };
>  
>  /*
> 
> 


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

Re: [Xen-devel] [PATCH 03/16] xen: sched: make implementing .alloc_pdata optional

[Juergen Gross]

On 18/03/16 20:04, 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 
> ---
> Cc: George Dunlap 
> Cc: Robert VanVossen 
> Cc: Josh Whitehead 
> Cc: Meng Xu 
> Cc: Jan Beulich 
> Cc: Juergen Gross 
> ---
> Changes from v1:
>  * use IS_ERR() and friends to deal with the return value
>of alloc_pdata, as suggested during review.
> ---
>  xen/common/sched_arinc653.c |   31 ---
>  xen/common/sched_credit.c   |4 ++--
>  xen/common/sched_credit2.c  |2 +-
>  xen/common/sched_rt.c   |7 +++
>  xen/common/schedule.c   |   18 --
>  xen/include/xen/sched-if.h  |1 +
>  6 files changed, 15 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 305889a..288749f 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -532,12 +532,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 7ddad38..36dc9ee 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -2044,7 +2044,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 d98bfb6..ac8019f 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -665,7 +665,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 )
>  {
> @@ -673,13 +673,12 @@ rt_alloc_pdata(const struct scheduler *ops, int cpu)
>  prv->repl_timer = xzalloc(struct timer);
>  
>  if ( prv->repl_timer == NULL )
> -return NULL;
> +return ERR_PTR(-ENOMEM);
>  
>  init_timer(prv->repl_timer, repl_timer_handler, (void *)ops, cpu);
>  }
>  
> -/* 1 indicates alloc. succeed in schedule.c */
> -return (void *)1;
> +return NULL;
>  }
>  
>  static void
> diff 

Re: [Xen-devel] [PATCH 2/3] libxl: add domain config parameter to force start of qemu

[Juergen Gross]

On 21/03/16 15:28, George Dunlap wrote:
> On 18/03/16 08:11, Juergen Gross wrote:
>> On 17/03/16 17:06, George Dunlap wrote:
>>> On Thu, Mar 10, 2016 at 3:00 PM, Juergen Gross  wrote:
 Today the device model (qemu) is started for a pv domain only in case
 a device requiring qemu is specified in the domain configuration
 (qdisk, vfb, channel). If there is no such device the device model
 isn't started and hence it is possible to add such a device to the
 domain later.

 Add a domain configuration parameter to specify the device model is
 to be started in any case. This will enable adding devices with a
 qemu based backend later.

 While the optimal solution would be to start the device model
 automatically when needed this would require some major rework of
 libxl at multiple places.

 Signed-off-by: Juergen Gross 
>>>
>>> So wait -- what happens now if you try to attach a disk with a qdisk
>>> backend to a PV guest that didn't start with qemu running?
>>
>> It won't work (that was my test case for the patch).
>>
>>> I'd really like to see patch 3 get in, but I'm not really in favor of
>>> this sort of a user-visible hack, particularly as we have to support
>>> it in libxl more or less indefinitely.
>>
>> Hmm, really? We can add a smarter variant later which will start the
>> device model in case it isn't started yet. Then the new config parameter
>> could be just ignored.
> 
> Sure; but it will (probably) only actually be useful for one release
> cycle (now 6 months), and then it will sit around cluttering up the
> interface for years to come.  The situation wrt hotplug has been this
> way for years now, and nobody has complained; I don't think an extra 6
> months will be a big deal.
> 
> Ultimately it's the tools maintainers' call; I wouldn't argue against it
> if one of them think it's a good idea.
> 
>> I didn't do the smart variant as I'm not sure I could set it up in time
>> for 4.7. I'd be happy to do it with some assistance regarding the async
>> framework of libxl I'm not at all familiar with.
> 
> I'm sure help could be arranged; but it might be difficult to do by 4.7.

Okay, let's see what can be arranged.

BTW: This patch is not strictly required for patch 3. Patch 3 is just
adding another case where no device model is available when needed.


Juergen

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

Re: [Xen-devel] [PATCH 2/3] libxl: add domain config parameter to force start of qemu

[George Dunlap]

On 18/03/16 08:11, Juergen Gross wrote:
> On 17/03/16 17:06, George Dunlap wrote:
>> On Thu, Mar 10, 2016 at 3:00 PM, Juergen Gross  wrote:
>>> Today the device model (qemu) is started for a pv domain only in case
>>> a device requiring qemu is specified in the domain configuration
>>> (qdisk, vfb, channel). If there is no such device the device model
>>> isn't started and hence it is possible to add such a device to the
>>> domain later.
>>>
>>> Add a domain configuration parameter to specify the device model is
>>> to be started in any case. This will enable adding devices with a
>>> qemu based backend later.
>>>
>>> While the optimal solution would be to start the device model
>>> automatically when needed this would require some major rework of
>>> libxl at multiple places.
>>>
>>> Signed-off-by: Juergen Gross 
>>
>> So wait -- what happens now if you try to attach a disk with a qdisk
>> backend to a PV guest that didn't start with qemu running?
> 
> It won't work (that was my test case for the patch).
> 
>> I'd really like to see patch 3 get in, but I'm not really in favor of
>> this sort of a user-visible hack, particularly as we have to support
>> it in libxl more or less indefinitely.
> 
> Hmm, really? We can add a smarter variant later which will start the
> device model in case it isn't started yet. Then the new config parameter
> could be just ignored.

Sure; but it will (probably) only actually be useful for one release
cycle (now 6 months), and then it will sit around cluttering up the
interface for years to come.  The situation wrt hotplug has been this
way for years now, and nobody has complained; I don't think an extra 6
months will be a big deal.

Ultimately it's the tools maintainers' call; I wouldn't argue against it
if one of them think it's a good idea.

> I didn't do the smart variant as I'm not sure I could set it up in time
> for 4.7. I'd be happy to do it with some assistance regarding the async
> framework of libxl I'm not at all familiar with.

I'm sure help could be arranged; but it might be difficult to do by 4.7.

 -George


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

Re: [Xen-devel] Xen components.

[Jason Long]

It is not a good diagram. Looking at my OS diagram.
For example, Xen have Dom0, DomU and



On Monday, March 21, 2016 6:34 AM, Andrew Cooper  
wrote:
On 21/03/16 13:28, Jason Long wrote:

Don't top post.

> Thank you but I need a diagram like below. It is an diagram about OS 
> components :
>
> http://www.c-jump.com/CIS24/Slides/Booting/images/os_components.png
>
> Can you show me similar diagram for Xen?

http://lmgtfy.com/?q=diagram+of+xen+components


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

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

Re: [Xen-devel] [PATCH 08/16] xen: sched: prepare a .switch_sched hook for Credit2

[Jan Beulich]

>>> On 18.03.16 at 20:04,  wrote:
> RTDS is basically identical to Credit2, as far as scheduler
> lock (re)mapping is concerned. Therefore, the same analisys
> and considerations expressed for the previous patch ("xen:
> sched: prepare a .switch_sched hook for Credit2"), applies
> to it to.

Yet the title should still say RTDS ...

Jan


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

Re: [Xen-devel] Outreachy Application

[Lasya Venneti]

Hello,

My initial contribution can be found at:
http://lists.xen.org/archives/html/xen-devel/2015-10/msg02921.html


On Mon, Mar 21, 2016 at 4:56 PM, Lars Kurth 
wrote:

> Hi Lasya, (adding xen-devel@)
>
> On 21 Mar 2016, at 11:17, Lasya Venneti  wrote:
>
> Hi Lars, Jesus!
>
> This is Lasya from India, if you recall. I had worked with Xen before the
> last Outreachy round, however I could not get selected for the same as the
> program rules had changed and they wanted fulltime participants.
>
> This time, however, I could not contribute much as I have a heavy semester
> going on, but I will be completely free during the duration of May-July as
> we have summer vacations, hence I will be able to do the project justice. I
> have submitted an outreachy proposal for the *Xen Code Review Dashboard. *I
> request you to have a look at it, and give me valuable feedback. It's
> rudimentary as of now, but I assure you I can improve on it as I start
> working on it. Looking forward to getting shortlisted, in case the
> participants have not yet been finalized, and you don't have someone else
> in mind..
>
>
> I will have a look. But the Outreachy program does require an initial
> contribution. You may want to search
> http://lists.xenproject.org/archives/html/xen-devel/2016-03/threads.html for
> Outreachy
> Lars
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4 08/34] vmap: Make the while loop less fishy.

[George Dunlap]

On Mon, Mar 21, 2016 at 1:26 PM, Jan Beulich  wrote:
 On 21.03.16 at 13:04,  wrote:
>> On Thu, Mar 17, 2016 at 4:08 PM, Ian Jackson  
>> wrote:
>>> Konrad Rzeszutek Wilk writes ("[PATCH v4 08/34] vmap: Make the while loop 
>>> less fishy."):
   error:
 -while ( i-- )
 -free_domheap_page(mfn_to_page(mfn_x(mfn[i])));
 +while ( i )
 +free_domheap_page(mfn_to_page(mfn_x(mfn[--i])));
>>>
>>> I quite strongly dislike this.  It is good practice to keep the loop
>>> control code together where this is reasonably convenient.
>>>
>>> I wouldn't quibble on such a stylistic matter (particularly outside my
>>> bailiwick) but (a) I would like to reinforce Jan's position and
>>> (b) it seems worth writing an email as there will be many occurrences.
>>
>> Since we're taking about general principle (and I've been referred to
>> here from a similar discussion elsewhere [1]), let me weigh in as
>> well.
>>
>> I can see the point of not wanting the decrement to be in the middle
>> of the expression here.  But I also entirely agree with Konrad's
>> assessment that this code is likely to be confusing; and the fact that
>> a computer program following a list of rules *developed by
>> professional bug-finders* is confused by this kind of semantics I
>> think supports this assessment.  At very least it has the potential to
>> waste a lot of mental energy figuring out why code that looks wrong
>> isn't wrong; and at worst there's a risk that at some point someone
>> will "fix" it incorrectly.
>>
>> The fact that there are already many instances of this pattern in the
>> source tree would be relevant if we expect nobody but people currently
>> familiar with the code to every try to read or modify it.  But since
>> on the contrary we hope that others will contribute to the codebase,
>> and even that they may eventually become maintainers, I think there is
>> sense in addressing them, at least as they come up.
>
> Well, if talk was about something really complex here, I might
> agree. But unary prefix and postfix operators are an integral
> part of the C language, and while code reading indeed shouldn't
> require overly much mental energy, I think we should be
> permitted to make full knowledge of the base programming
> language a prereq to reading our code. Otherwise - where do
> you want to draw the boundary of what is permitted and what
> is not? (Yes, I know I'm guilty in occasionally writing rather
> complex expressions, with at times not immediately obvious side
> effects, and I'm trying to do better irrespective of all such also
> falling in the above "basic language" features category. That's
> because I can see doing so being past the boundary of
> reasonably understandable code.)

And I'm not saying that we can't take advantage of postfix operators
in this case; I'm just saying that if we are going to, it would be
better to point it out in a comment.

There are lots of "features" of C that standard practice still demands
that we add a comment for; the default fall-through for switch
statements comes to mind.  Of course programmers should be required to
know that switch statements default to fallthrough in C; but it's
still a common mistake to forget that, and so we point them in the
right direction by adding comments.

Similarly, of course programmers should know the difference between
prefix and postfix operators.  But this is a case where there's a risk
of tripping over something, so it makes sense to point them in the
right direction by adding comments.

Or to take a different tack: I understand that you don't think there's
no particular benefit to adding a comment in cases like this; could
you explain to me why you think it would have a significant cost?

>> do {
>>  i--;
>>  [cleanup]
>> } while ( i > 0 );
>>
>
> But you realize that the first (but not the second) one is wrong for
> the i == 0 case?

So it is.  I think I was thinking of a case where it was known that at
least one iteration had succeeded; but obviously the second is more
safe in general.

 -George

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

Re: [Xen-devel] [PATCH 03/16] xen: sched: make implementing .alloc_pdata optional

[Jan Beulich]

>>> On 18.03.16 at 20:04,  wrote:
> --- a/xen/include/xen/sched-if.h
> +++ b/xen/include/xen/sched-if.h
> @@ -9,6 +9,7 @@
>  #define __XEN_SCHED_IF_H__
>  
>  #include 
> +#include 
>  
>  /* A global pointer to the initial cpupool (POOL0). */
>  extern struct cpupool *cpupool0;

There is no visible use in this header of what err.h defines - why
does it get included all of the sudden?

Jan


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

Re: [Xen-devel] [PATCH v2 5/6] virt, sched: add cpu pinning to smp_call_sync_on_phys_cpu()

[Juergen Gross]

On 21/03/16 14:42, Jan Beulich wrote:
 On 21.03.16 at 13:24,  wrote:
>> @@ -758,9 +759,14 @@ struct smp_sync_call_struct {
>>  static void smp_call_sync_callback(struct work_struct *work)
>>  {
>>  struct smp_sync_call_struct *sscs;
>> +unsigned int cpu = smp_processor_id();
> 
> So this obtains the vCPU number, yet ...
> 
>>  sscs = container_of(work, struct smp_sync_call_struct, work);
>> +preempt_disable();
>> +hypervisor_pin_vcpu(cpu);
> 
> ... here you're supposed to pass a pCPU number.
> 
> Also don't you need to call smp_processor_id() after preempt_disable()?

No, I'm running on the workqueue bound to the specific (v)cpu and I'm
expecting this vcpu to be pinned to the same numbered pcpu.

preempt_disable() is just called to avoid scheduling of another thread
while the override pinning is active.


Juergen

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

Re: [Xen-devel] [PATCH v2 5/6] virt, sched: add cpu pinning to smp_call_sync_on_phys_cpu()

[David Vrabel]

On 21/03/16 13:42, Jan Beulich wrote:
>
> Also don't you need to call smp_processor_id() after preempt_disable()?

I suggest using get_cpu()/put_cpu() here.

David

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-libvirt   5 libvirt-build fail REGR. vs. 86536

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  9a423d1826838839f06a4f9551ce1c8b30f3d118
baseline version:
 libvirt  9a0c7f5f834185db9017c34aabc03ad99cf37bed

Last test of basis86536  2016-03-18 04:25:19 Z3 days
Failing since 86625  2016-03-19 04:23:22 Z2 days3 attempts
Testing same since86783  2016-03-21 04:21:26 Z0 days1 attempts


People who touched revisions under test:
  Cole Robinson 
  Jim Fehlig 
  John Ferlan 
  Martin Kletzander 
  Michal Privoznik 
  Roman Bogorodskiy 

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



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

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

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

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


Not pushing.


commit 9a423d1826838839f06a4f9551ce1c8b30f3d118
Author: Cole Robinson 
Date:   Thu Jan 7 22:49:58 2016 -0500

domain: Add virDomainDefAddImplicitDevices

It's just a combination of AddImplicitControllers, and AddConsoleCompat.
Every caller that wants ImplicitControllers also wants the ConsoleCompat
AFAICT, so lump them together. We also need it for future patches.

commit ae33a7b3369e4fe71000ea5f87ad64fe990619ec
Author: Roman Bogorodskiy 
Date:   Sat Mar 19 19:56:02 2016 +0300

nss: don't try to build nss plugin when disabled

Even if nss is disabled, the build system tries to build some
targets like libnss_libvirt_impl.la and nsstest. Hide those
under the "if WITH_NSS" block like the rest of NSS plugin bit

Re: [Xen-devel] [PATCH v2 5/6] virt, sched: add cpu pinning to smp_call_sync_on_phys_cpu()

[Jan Beulich]

>>> On 21.03.16 at 13:24,  wrote:
> @@ -758,9 +759,14 @@ struct smp_sync_call_struct {
>  static void smp_call_sync_callback(struct work_struct *work)
>  {
>   struct smp_sync_call_struct *sscs;
> + unsigned int cpu = smp_processor_id();

So this obtains the vCPU number, yet ...

>   sscs = container_of(work, struct smp_sync_call_struct, work);
> + preempt_disable();
> + hypervisor_pin_vcpu(cpu);

... here you're supposed to pass a pCPU number.

Also don't you need to call smp_processor_id() after preempt_disable()?

Jan


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

Re: [Xen-devel] [PATCH v3 08/28] xen/x86: Annotate VM applicability in featureset

[Andrew Cooper]

On 21/03/16 11:53, Jan Beulich wrote:

 +XEN_CPUFEATURE(X2APIC,1*32+21) /*A  Extended xAPIC */
>>> Does this make sense for PV?
>> It is currently visible, and we already have to leak APIC through to PV
>> guests.
> Having to leak on piece of state doesn't mean when need to leak
> more, when that's clearly impossibly to be meaningful to a guest.

PV guests can already real real APIC IDs out cpu leaves in the masking case.

Now I think about it, this is special just like HTT and CMP_LEGACY, and
must have the host value leaked through to PV guests when masking is in
effect.

 +XEN_CPUFEATURE(HYPERVISOR,1*32+31) /*A  Running under some hypervisor 
 */
>>> Wouldn't this better be one of the special ones?
>> Why? It doesn't need any special handling in Xen.  For all intents and
>> purposes, it is just like a regular feature bit.
> Except that you don't pass through its host value, but instead
> override it to 1. Which makes it kind of special, I would say.

HVM guests go straight from the toolstack settings.  PV guests have it
unilaterally set.

I suppose this does qualify for special under the guidelines I was
using.  I suspect it is only unilaterally set so it appears as set to
dom0.  DomUs also have it unilaterally set by the toolstack.

I suspect it makes very little difference for a PV guest, which knows
for certain that it is virtualised.


 +XEN_CPUFEATURE(LM,2*32+29) /*A  Long Mode (x86-64) */
>>> I think I had asked before, but doesn't the customization needed
>>> for 32-bit PV guests also rather make this a special one?
>> Why would it?  It is a simple feature which isn't present for 32bit guests.
> Together with the above the question really is: What exactly makes
> a feature special? The need for run time overrides, or something
> more sophisticated?

"Things which Xen has to deal specially with", which includes
conditionally leaking host state through.

~Andrew

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

Re: [Xen-devel] [PATCH v8 for Xen 4.7 1/4] xen: enable per-VCPU parameter for RTDS

[Jan Beulich]

>>> On 18.03.16 at 22:26,  wrote:
> Add XEN_DOMCTL_SCHEDOP_getvcpuinfo and _putvcpuinfo hypercalls
> to independently get and set the scheduling parameters of each
> vCPU of a domain
> 
> Signed-off-by: Chong Li 
> Signed-off-by: Meng Xu 
> Signed-off-by: Sisu Xi 
> 
> ---
> Changes on PATCH v7:
> 1) A bug in case XEN_DOMCTL_SCHEDOP_getinfo (in Xen 4.6) is fixed:
> The default PERIOD or BUDGET should be divided by MICROSECS(1),
> before returned to upper caller.

Seems like there's still some misunderstanding here: Anything
past the first --- won't make it into the repo, yet the description
of what bug you fix should end up there.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -338,24 +338,64 @@ DEFINE_XEN_GUEST_HANDLE(xen_domctl_max_vcpus_t);
>  #define XEN_SCHEDULER_ARINC653 7
>  #define XEN_SCHEDULER_RTDS 8
>  
> -/* Set or get info? */
> +typedef struct xen_domctl_sched_credit {
> +uint16_t weight;
> +uint16_t cap;
> +} xen_domctl_sched_credit_t;
> +
> +typedef struct xen_domctl_sched_credit2 {
> +uint16_t weight;
> +} xen_domctl_sched_credit2_t;
> +
> +typedef struct xen_domctl_sched_rtds {
> +uint32_t period;
> +uint32_t budget;
> +} xen_domctl_sched_rtds_t;
> +
> +typedef struct xen_domctl_schedparam_vcpu {
> +union {
> +xen_domctl_sched_credit_t credit;
> +xen_domctl_sched_credit2_t credit2;
> +xen_domctl_sched_rtds_t rtds;
> +} u;
> +uint32_t vcpuid;
> +uint16_t padding[2];

So why uint16_t[2] instead of just uint32_t? And what's the
padding needed for in the first place? Also, while for a domctl it's
not as strictly needed as for other hypercalls, checking that all
padding fields are zero would still seem to be rather desirable.

Jan


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

Re: [Xen-devel] Xen components.

[Andrew Cooper]

On 21/03/16 13:28, Jason Long wrote:

Don't top post.

> Thank you but I need a diagram like below. It is an diagram about OS 
> components :
>
> http://www.c-jump.com/CIS24/Slides/Booting/images/os_components.png
>
> Can you show me similar diagram for Xen?

http://lmgtfy.com/?q=diagram+of+xen+components

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

Re: [Xen-devel] Xen components.

[Jason Long]

Thank you but I need a diagram like below. It is an diagram about OS components 
:

http://www.c-jump.com/CIS24/Slides/Booting/images/os_components.png

Can you show me similar diagram for Xen?



On Monday, March 21, 2016 3:42 AM, Lars Kurth  wrote:


> On 20 Mar 2016, at 21:04, Daniele Palumbo  wrote:
> 
> Il giorno 20/mar/2016, alle ore 20:11, Jason Long  ha 
> scritto:
>> any idea?
> 
> FAQ?

See
http://wiki.xenproject.org/wiki/Xen_Project_Software_Overview
as well as several presentations on slideshare ... e.g. 
http://www.slideshare.net/xen_com_mgr/xen-hypervisor-for-the-cloud-frontier-meetup
 (there are lots of them; so it depends on exactly what you are looking for)
Lars
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

Re: [Xen-devel] [PATCH v4 08/34] vmap: Make the while loop less fishy.

[Jan Beulich]

>>> On 21.03.16 at 13:04,  wrote:
> On Thu, Mar 17, 2016 at 4:08 PM, Ian Jackson  
> wrote:
>> Konrad Rzeszutek Wilk writes ("[PATCH v4 08/34] vmap: Make the while loop 
>> less fishy."):
>>>   error:
>>> -while ( i-- )
>>> -free_domheap_page(mfn_to_page(mfn_x(mfn[i])));
>>> +while ( i )
>>> +free_domheap_page(mfn_to_page(mfn_x(mfn[--i])));
>>
>> I quite strongly dislike this.  It is good practice to keep the loop
>> control code together where this is reasonably convenient.
>>
>> I wouldn't quibble on such a stylistic matter (particularly outside my
>> bailiwick) but (a) I would like to reinforce Jan's position and
>> (b) it seems worth writing an email as there will be many occurrences.
> 
> Since we're taking about general principle (and I've been referred to
> here from a similar discussion elsewhere [1]), let me weigh in as
> well.
> 
> I can see the point of not wanting the decrement to be in the middle
> of the expression here.  But I also entirely agree with Konrad's
> assessment that this code is likely to be confusing; and the fact that
> a computer program following a list of rules *developed by
> professional bug-finders* is confused by this kind of semantics I
> think supports this assessment.  At very least it has the potential to
> waste a lot of mental energy figuring out why code that looks wrong
> isn't wrong; and at worst there's a risk that at some point someone
> will "fix" it incorrectly.
> 
> The fact that there are already many instances of this pattern in the
> source tree would be relevant if we expect nobody but people currently
> familiar with the code to every try to read or modify it.  But since
> on the contrary we hope that others will contribute to the codebase,
> and even that they may eventually become maintainers, I think there is
> sense in addressing them, at least as they come up.

Well, if talk was about something really complex here, I might
agree. But unary prefix and postfix operators are an integral
part of the C language, and while code reading indeed shouldn't
require overly much mental energy, I think we should be
permitted to make full knowledge of the base programming
language a prereq to reading our code. Otherwise - where do
you want to draw the boundary of what is permitted and what
is not? (Yes, I know I'm guilty in occasionally writing rather
complex expressions, with at times not immediately obvious side
effects, and I'm trying to do better irrespective of all such also
falling in the above "basic language" features category. That's
because I can see doing so being past the boundary of
reasonably understandable code.)

> In my case I've suggested adding a comment to clue people into the
> fact that the postfix semantics are in operation; I think that
> balances "reducing cognitive load" with "avoids unnecessarily verbose
> code".
> 
> Other options would be things like this:
> 
> do {
>  i--;
>  [cleanup]
> } while ( i > 0 );
> 
> or
> 
> while ( i > 0 ) {
>  i--;
>  [cleanup]
> }
> 
> The first one I think is the clearest, but neither one are very concise.

But you realize that the first (but not the second) one is wrong for
the i == 0 case?

Jan


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

Re: [Xen-devel] [PATCH 4/5] x86/time: refactor read_platform_stime()

[Andrew Cooper]

On 21/03/16 11:45, Joao Martins wrote:
>
> All fixed, though I found one change missing in this series, specifically on
> time_calibration_std_rendezvous. Otherwise this commit would break 
> compilation.
> See chunk below for the change I am adding:
>
> @@ -1377,7 +1380,7 @@ static void time_calibration_std_rendezvous(void *_r)
>  {
>  while ( atomic_read(&r->semaphore) != (total_cpus - 1) )
>  cpu_relax();
> -r->master_stime = read_platform_stime();
> +r->master_stime = read_platform_stime(NULL);
>  mb(); /* write r->master_stime /then/ signal */
>  atomic_inc(&r->semaphore);
>  }
>
> Having this fixed, could I still keep your Reviewed-by?

Yes - that's fine.

~Andrew

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

Re: [Xen-devel] [PATCH v6 01/22] arm/acpi: Estimate memory required for acpi/efi tables

[Jan Beulich]

>>> On 18.03.16 at 19:44,  wrote:
> On 17/03/16 09:40, Shannon Zhao wrote:
>> --- /dev/null
>> +++ b/xen/arch/arm/efi/efi-dom0.c
>> @@ -0,0 +1,50 @@
>> +/*
>> + *  efi-dom0.c - Domain0 EFI Boot Support
>> + *
>> + *  Copyright (C) 2016 Shannon Zhao 
>> + *
>> + * 
>> ~~
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + *
>> + *  This program is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + *  GNU General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU General Public License
>> + *  along with this program; If not, see .
>> + *
>> + * 
>> ~~
>> + */
>> +
>> +#include "efi.h"
>> +#include "efi-dom0.h"
>> +#include 
>> +#include 
>> +
>> +struct meminfo __initdata acpi_mem;
> 
> I'm worry about sharing data between the EFI stub and Xen. On ARM, the 
> EFI stub is completely independent. Once it has finished to execute, it 
> will call xen as if it runs without UEFI.
> 
> This means that BSS will be zeroed. I would have expect to see acpi_mem 
> residing in BSS. So I'm wondering how this can work.

The __initdata annotation makes it a member of .init.data instead
of the expected .bss.

Jan


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

[Xen-devel] [xen-unstable test] 86747: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail REGR. vs. 
86491
 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail in 86645 
REGR. vs. 86491

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-arndale   3 host-install(3)  broken in 86645 pass in 86747
 test-amd64-amd64-xl-qemut-win7-amd64 12 guest-saverestore fail in 86645 pass 
in 86747
 test-amd64-i386-xl-qemuu-win7-amd64 12 guest-saverestorefail pass in 86645
 test-armhf-armhf-xl-credit2  15 guest-start/debian.repeat   fail pass in 86645

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

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-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 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-amd64-amd64-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-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-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-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass

version targeted for testing:
 xen  829e03ca0ef757350546df8546a6575ca3d0e8da
baseline version:
 xen  a6f2cdb633bf519244a16674031b8034b581ba7f

Last test of basis86491  2016-03-17 15:24:59 Z3 days
Failing since 86560  2016-03-18 10:56:34 Z3 days3 attempts
Testing same since86645  2016-03-19 12:55:56 Z1 days2 attempts


People who touched revisions under test:
  Andrew Cooper 
  Chunyan Liu 
  Dagaen Golomb 
  David Vrabel 
  George Dunlap 
  Ian Jackson 
  Jan Beulich 
  Konrad Rzeszutek Wilk 
  Meng Xu 
  Paul Durrant 
  Shannon Zhao 
  Simon Cao 
  Tianyang Chen 
  Wei Liu 
  Wen Congyang 

jobs:
 build-amd64-xsm  pass
 build-armhf-

Re: [Xen-devel] [PATCH v4 04/34] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[Jan Beulich]

>>> On 18.03.16 at 20:22,  wrote:
>> > + * return the number of bytes requested for the operation. Or an
>> > + * negative value if an error is encountered.
>> > + */
>> > +
>> > +typedef uint64_t xen_version_op_val_t;
>> > +DEFINE_XEN_GUEST_HANDLE(xen_version_op_val_t);
>> > +
>> > +typedef void xen_version_op_buf_t;
>> > +DEFINE_XEN_GUEST_HANDLE(xen_version_op_buf_t);
>> 
>> Are these actually useful for anything? And for the various strings,
> 
> The xen_version_op_val_t is definitly used by the toolstack.
> 
>> wouldn't a "char" handle be more natural?
> 
> Heh. It was char[] before but Andrew liked it as void.

But that was because you used it for non string types too,
wasn't it?

> @@ -380,6 +388,133 @@ DO(xen_version)(int cmd, XEN_GUEST_HANDLE_PARAM(void) 
> arg)
>  return -ENOSYS;
>  }
>  
> +static const char *capabilities_info(unsigned int *len)
> +{
> +static xen_capabilities_info_t __read_mostly cached_cap;
> +static unsigned int __read_mostly cached_cap_len;
> +static bool_t cached;
> +
> +if ( unlikely(!cached) )
> +{
> +arch_get_xen_caps(&cached_cap);
> +cached_cap_len = strlen(cached_cap) + 1;
> +cached = 1;
> +}

I'm sorry for noticing this only now, but without any locking this is
unsafe: x86's arch_get_xen_caps() using safe_strcat() to fill the
buffer, simultaneous invocations would possibly produce garbled
output to all (i.e. also subsequently started) guests. Either use a
real lock here, or make the guard a tristate one, which gets
transitioned e.g. from 0 to -1 by the first one coming here (doing
the initialization), with everyone else waiting for it to become +1
(to which the initializing party sets it once it is done).

> +DO(version_op)(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) arg,
> +   unsigned int len)
> +{
> +union {
> +xen_version_op_val_t val;
> +xen_feature_info_t fi;
> +} u = {};
> +unsigned int sz = 0;
> +const void *ptr = NULL;
> +int rc = xsm_version_op(XSM_OTHER, cmd);
> +
> +/* We can safely return -EPERM! */
> +if ( rc )
> +return rc;
> +
> +/*
> + * The HYPERVISOR_xen_version differs in that some return the value,
> + * and some copy it on back on argument. We follow the same rule for all
> + * sub-ops: return 0 on success, positive value of bytes returned, and
> + * always copy the result in arg. Yeey sanity!
> + */
> +switch ( cmd )
> +{
> +case XEN_VERSION_version:
> +sz = sizeof(xen_version_op_val_t);
> +u.val = (xen_major_version() << 16) | xen_minor_version();
> +break;
> +
> +case XEN_VERSION_extraversion:
> +sz = strlen(xen_extra_version()) + 1;
> +ptr = xen_extra_version();
> +break;
> +
> +case XEN_VERSION_capabilities:
> +ptr = capabilities_info(&sz);
> +break;
> +
> +case XEN_VERSION_changeset:
> +sz = strlen(xen_changeset()) + 1;
> +ptr = xen_changeset();
> +break;
> +
> +case XEN_VERSION_platform_parameters:
> +sz = sizeof(xen_version_op_val_t);
> +u.val = HYPERVISOR_VIRT_START;
> +break;
> +
> +case XEN_VERSION_get_features:
> +if ( copy_from_guest(&u.fi, arg, 1) )

Afaict this is incompatible with the null handle check further down (i.e.
you also need to check for a null handle here).

> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -128,6 +128,9 @@
>   ** VCPUOP_register_vcpu_info
>   ** VCPUOP_register_runstate_memory_area
>   *
> + *  HYPERVISOR_version_op
> + *   All generic sub-operations
> + *
>   *
>   * Other notes on the ARM ABI:

I don't think the extra almost blank line is warranted here.

> --- a/xen/include/public/version.h
> +++ b/xen/include/public/version.h
> @@ -30,7 +30,15 @@
>  
>  #include "xen.h"
>  
> -/* NB. All ops return zero on success, except XENVER_{version,pagesize} */
> +/*
> + * There are two hypercalls mentioned in here. The XENVER_ are for
> + * HYPERCALL_xen_version (17), while VERSION_ are for the
> + * HYPERCALL_version_op (41).
> + *
> + * The subops are very similar except that the later hypercall has a
> + * sane interface.
> + */
> +
>  
>  /* arg == NULL; returns major:minor (16:16). */

Nor is the extra blank one here.

> @@ -87,6 +95,66 @@ typedef struct xen_feature_info xen_feature_info_t;
>  #define XENVER_commandline 9
>  typedef char xen_commandline_t[1024];
>  
> +
> +
> +/*
> + * The HYPERCALL_version_op has a set of sub-ops which mirror the

And three consecutive blank lines are too much in any event. (If
for no other reason that because that provides extremely bad
patch context if a later change happened right next to these three
lines.)

> +/*
> + * arg == char.
> + *
> + * The toolstack fills it out for guest consumption. It is intended to hold
> + * the UUID of the guest.
> + */
> +#define XEN_VERSION_guest_handle8

So this is the place where I agree with An

Re: [Xen-devel] [libvirt] [PATCH V2 0/4] Extend to a tristate

[Ján Tomko]

On Mon, Feb 29, 2016 at 09:00:44PM -0700, Jim Fehlig wrote:
> An expanded V2 of
> 
> https://www.redhat.com/archives/libvir-list/2016-February/msg00140.html
> 
> In V2, the  feature is extended with a state='on|off' attribute to
> allow differentiating the 'on' and 'off' states with not set (or hypervisor
> default).
> 
> Obviously post 1.3.2 material. See individual patches for details.
> 
> Jim Fehlig (4):
>   conf: add 'state' attribute to  feature
>   xenconfig: change 'hap' setting to align with Xen behavior
>   Xen drivers: show hap enabled by default in capabilities
>   libxl: support enabling and disabling  feature
> 
>  docs/formatdomain.html.in  |  6 ++-
>  docs/schemas/domaincommon.rng  |  6 ++-
>  src/conf/domain_conf.c |  4 +-
>  src/libxl/libxl_conf.c | 20 ++--
>  src/xen/xen_hypervisor.c   |  2 +-
>  src/xenconfig/xen_common.c | 14 ++---
>  .../test-disk-positional-parms-full.cfg|  1 -
>  .../test-disk-positional-parms-partial.cfg |  1 -
>  ...est-fullvirt-direct-kernel-boot-bogus-extra.cfg |  1 -
>  .../test-fullvirt-direct-kernel-boot-extra.cfg |  1 -
>  .../test-fullvirt-direct-kernel-boot.cfg   |  1 -
>  tests/xlconfigdata/test-fullvirt-multiusb.cfg  |  1 -
>  tests/xlconfigdata/test-fullvirt-nohap.cfg | 26 ++
>  tests/xlconfigdata/test-fullvirt-nohap.xml | 59 
> ++
>  tests/xlconfigdata/test-new-disk.cfg   |  1 -
>  tests/xlconfigdata/test-rbd-multihost-noauth.cfg   |  1 -
>  tests/xlconfigdata/test-spice-features.cfg |  1 -
>  tests/xlconfigdata/test-spice.cfg  |  1 -
>  tests/xlconfigdata/test-vif-rate.cfg   |  1 -
>  tests/xlconfigtest.c   |  1 +
>  tests/xmconfigdata/test-escape-paths.cfg   |  1 -
>  .../xmconfigdata/test-fullvirt-default-feature.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-force-hpet.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-force-nohpet.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-localtime.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-net-netfront.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-new-cdrom.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-nohap.cfg | 28 ++
>  tests/xmconfigdata/test-fullvirt-nohap.xml | 51 +++
>  tests/xmconfigdata/test-fullvirt-parallel-tcp.cfg  |  1 -
>  .../test-fullvirt-serial-dev-2-ports.cfg   |  1 -
>  .../test-fullvirt-serial-dev-2nd-port.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-file.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-null.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-pipe.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-serial-pty.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-stdio.cfg  |  1 -
>  .../test-fullvirt-serial-tcp-telnet.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-tcp.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-udp.cfg|  1 -
>  tests/xmconfigdata/test-fullvirt-serial-unix.cfg   |  1 -
>  tests/xmconfigdata/test-fullvirt-sound.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-usbmouse.cfg  |  1 -
>  tests/xmconfigdata/test-fullvirt-usbtablet.cfg |  1 -
>  tests/xmconfigdata/test-fullvirt-utc.cfg   |  1 -
>  tests/xmconfigdata/test-no-source-cdrom.cfg|  1 -
>  tests/xmconfigdata/test-pci-devs.cfg   |  1 -
>  tests/xmconfigtest.c   |  1 +
>  48 files changed, 202 insertions(+), 52 deletions(-)
>  create mode 100644 tests/xlconfigdata/test-fullvirt-nohap.cfg
>  create mode 100644 tests/xlconfigdata/test-fullvirt-nohap.xml
>  create mode 100755 tests/xmconfigdata/test-fullvirt-nohap.cfg
>  create mode 100644 tests/xmconfigdata/test-fullvirt-nohap.xml
> 

ACK series

Jan

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

Re: [Xen-devel] [PATCH v4 06/34] x86/arm: Add BUGFRAME_NR define and BUILD checks.

[Jan Beulich]

>>> On 18.03.16 at 20:59,  wrote:
> I know I copied and pasted it and I must have done something uncanny.
> 
> Anyhow this is what the change looks like now (I've retained the Reviewed
> and Ack as I think this change is mostly cosmetical in nature?)

I think that's okay.

> v5: Add Acks, make BUILD_BUG_ON checks look correct. Position the
> BUGFRAME_NR properly.

Almost, that is.

> --- a/xen/include/asm-x86/bug.h
> +++ b/xen/include/asm-x86/bug.h
> @@ -10,6 +10,7 @@
>  #define BUGFRAME_bug2
>  #define BUGFRAME_assert 3
>  
> +#define BUGFRAME_NR 4
>  #ifndef __ASSEMBLY__

The insertion wants to go _before_ the blank line. (And in the
ARM case you then may consider removing the preceding blank
line too; in any event the ARM and x86 ones should look similar
in the end.)

Jan


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

[Xen-devel] [PATCH v2 3/6] dcdbas: make use of smp_call_sync_on_phys_cpu()

[Juergen Gross]

Use smp_call_sync_on_phys_cpu() to raise SMI on cpu 0.

Signed-off-by: Juergen Gross 
---
 drivers/firmware/dcdbas.c | 46 --
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/firmware/dcdbas.c b/drivers/firmware/dcdbas.c
index 829eec8..d15ad0b 100644
--- a/drivers/firmware/dcdbas.c
+++ b/drivers/firmware/dcdbas.c
@@ -238,33 +238,14 @@ static ssize_t host_control_on_shutdown_store(struct 
device *dev,
return count;
 }
 
-/**
- * dcdbas_smi_request: generate SMI request
- *
- * Called with smi_data_lock.
- */
-int dcdbas_smi_request(struct smi_cmd *smi_cmd)
+static int raise_smi(void *par)
 {
-   cpumask_var_t old_mask;
-   int ret = 0;
+   struct smi_cmd *smi_cmd = par;
 
-   if (smi_cmd->magic != SMI_CMD_MAGIC) {
-   dev_info(&dcdbas_pdev->dev, "%s: invalid magic value\n",
-__func__);
-   return -EBADR;
-   }
-
-   /* SMI requires CPU 0 */
-   if (!alloc_cpumask_var(&old_mask, GFP_KERNEL))
-   return -ENOMEM;
-
-   cpumask_copy(old_mask, ¤t->cpus_allowed);
-   set_cpus_allowed_ptr(current, cpumask_of(0));
if (smp_processor_id() != 0) {
dev_dbg(&dcdbas_pdev->dev, "%s: failed to get CPU 0\n",
__func__);
-   ret = -EBUSY;
-   goto out;
+   return -EBUSY;
}
 
/* generate SMI */
@@ -280,10 +261,23 @@ int dcdbas_smi_request(struct smi_cmd *smi_cmd)
: "memory"
);
 
-out:
-   set_cpus_allowed_ptr(current, old_mask);
-   free_cpumask_var(old_mask);
-   return ret;
+   return 0;
+}
+/**
+ * dcdbas_smi_request: generate SMI request
+ *
+ * Called with smi_data_lock.
+ */
+int dcdbas_smi_request(struct smi_cmd *smi_cmd)
+{
+   if (smi_cmd->magic != SMI_CMD_MAGIC) {
+   dev_info(&dcdbas_pdev->dev, "%s: invalid magic value\n",
+__func__);
+   return -EBADR;
+   }
+
+   /* SMI requires CPU 0 */
+   return smp_call_sync_on_phys_cpu(0, raise_smi, smi_cmd);
 }
 
 /**
-- 
2.6.2


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

[Xen-devel] [PATCH v2 5/6] virt, sched: add cpu pinning to smp_call_sync_on_phys_cpu()

[Juergen Gross]

Add generic virtualization support for pinning the current vcpu to a
specified physical cpu. As this operation isn't performance critical
(a very limited set of operations like BIOS calls and SMIs is expected
to need this) just add a hypervisor specific indirection.

Such a pinning should last as short as possible as it might block
sensible vcpu scheduling and maybe other hypervisor functions like
suspending the system which rely on scheduling. To ensure this don't
let the current thread be preempted while the vcpu is pinned in
smp_call_sync_on_phys_cpu().

Signed-off-by: Juergen Gross 
---
V2: adapt to using workqueues
add include/linux/hypervisor.h to hide architecture specific stuff
from generic kernel code

In case paravirt maintainers don't want to be responsible for
include/linux/hypervisor.h I could take it.
---
 MAINTAINERS   |  1 +
 arch/x86/include/asm/hypervisor.h |  9 +
 include/linux/hypervisor.h| 17 +
 kernel/smp.c  |  6 ++
 kernel/up.c   | 11 ++-
 5 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/hypervisor.h

diff --git a/MAINTAINERS b/MAINTAINERS
index da636ea..62c2371 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8271,6 +8271,7 @@ S:Supported
 F: Documentation/virtual/paravirt_ops.txt
 F: arch/*/kernel/paravirt*
 F: arch/*/include/asm/paravirt.h
+F: include/linux/hypervisor.h
 
 PARIDE DRIVERS FOR PARALLEL PORT IDE DEVICES
 M: Tim Waugh 
diff --git a/arch/x86/include/asm/hypervisor.h 
b/arch/x86/include/asm/hypervisor.h
index 055ea99..13f80a2 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -43,6 +43,9 @@ struct hypervisor_x86 {
 
/* X2APIC detection (run once per boot) */
bool(*x2apic_available)(void);
+
+   /* pin current vcpu to specified physical cpu (run rarely) */
+   void(*pin_vcpu)(int);
 };
 
 extern const struct hypervisor_x86 *x86_hyper;
@@ -56,6 +59,12 @@ extern const struct hypervisor_x86 x86_hyper_kvm;
 extern void init_hypervisor(struct cpuinfo_x86 *c);
 extern void init_hypervisor_platform(void);
 extern bool hypervisor_x2apic_available(void);
+
+static inline void hypervisor_pin_vcpu(int cpu)
+{
+   if (x86_hyper->pin_vcpu)
+   x86_hyper->pin_vcpu(cpu);
+}
 #else
 static inline void init_hypervisor(struct cpuinfo_x86 *c) { }
 static inline void init_hypervisor_platform(void) { }
diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h
new file mode 100644
index 000..3fa5ef2
--- /dev/null
+++ b/include/linux/hypervisor.h
@@ -0,0 +1,17 @@
+#ifndef __LINUX_HYPEVISOR_H
+#define __LINUX_HYPEVISOR_H
+
+/*
+ * Generic Hypervisor support
+ * Juergen Gross 
+ */
+
+#ifdef CONFIG_HYPERVISOR_GUEST
+#include 
+#else
+static inline void hypervisor_pin_vcpu(int cpu)
+{
+}
+#endif
+
+#endif /* __LINUX_HYPEVISOR_H */
diff --git a/kernel/smp.c b/kernel/smp.c
index 62da74b..673242b 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "smpboot.h"
 
@@ -758,9 +759,14 @@ struct smp_sync_call_struct {
 static void smp_call_sync_callback(struct work_struct *work)
 {
struct smp_sync_call_struct *sscs;
+   unsigned int cpu = smp_processor_id();
 
sscs = container_of(work, struct smp_sync_call_struct, work);
+   preempt_disable();
+   hypervisor_pin_vcpu(cpu);
sscs->ret = sscs->func(sscs->data);
+   hypervisor_pin_vcpu(-1);
+   preempt_enable();
 
complete(&sscs->done);
 }
diff --git a/kernel/up.c b/kernel/up.c
index afd395c..725ec44 100644
--- a/kernel/up.c
+++ b/kernel/up.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 
 int smp_call_function_single(int cpu, void (*func) (void *info), void *info,
int wait)
@@ -85,9 +86,17 @@ EXPORT_SYMBOL(on_each_cpu_cond);
 
 int smp_call_sync_on_phys_cpu(unsigned int cpu, int (*func)(void *), void *par)
 {
+   int ret;
+
if (cpu != 0)
return -EINVAL;
 
-   return func(par);
+   preempt_disable();
+   hypervisor_pin_vcpu(0);
+   ret = func(par);
+   hypervisor_pin_vcpu(-1);
+   preempt_enable();
+
+   return ret;
 }
 EXPORT_SYMBOL_GPL(smp_call_sync_on_phys_cpu);
-- 
2.6.2


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

Re: [Xen-devel] [PATCH v6 03/22] arm/acpi: Prepare FADT table for Dom0

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

Copy and modify FADT table before passing it to Dom0. Set PSCI_COMPLIANT
and PSCI_USE_HVC.

Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 


Acked-by: Julien Grall 

Regards,

--
Julien Grall

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

[Xen-devel] [PATCH v2 0/6] Support calling functions on dedicated physical cpu

[Juergen Gross]

Some hardware (e.g. Dell Studio laptops) require special functions to
be called on physical cpu 0 in order to avoid occasional hangs. When
running as dom0 under Xen this could be achieved only via special boot
parameters (vcpu pinning) limiting the hypervisor in it's scheduling
decisions.

This patch series is adding a generic function to be able to temporarily
pin a (virtual) cpu to a dedicated physical cpu for executing above
mentioned functions on that specific cpu. The drivers (dcdbas and i8k)
requiring this functionality are modified accordingly.

Changes in V2:
- instead of manipulating the allowed set of cpus use cpu specific
  workqueue as requested by Peter Zijlstra
- add include/linux/hypervisor.h to hide architecture specific stuff
  from generic kernel cod


Juergen Gross (6):
  xen: sync xen header
  smp: add function to execute a function synchronously on a physical
cpu
  dcdbas: make use of smp_call_sync_on_phys_cpu()
  hwmon: use smp_call_sync_on_phys_cpu() for dell-smm i8k
  virt, sched: add cpu pinning to smp_call_sync_on_phys_cpu()
  xen: add xen_pin_vcpu() to support calling functions on a dedicated
pcpu

 MAINTAINERS   |   1 +
 arch/x86/include/asm/hypervisor.h |   9 
 arch/x86/xen/enlighten.c  |  40 +++
 drivers/firmware/dcdbas.c |  46 --
 drivers/hwmon/dell-smm-hwmon.c|  27 +-
 include/linux/hypervisor.h|  17 +++
 include/linux/smp.h   |   2 +
 include/xen/interface/sched.h | 100 +++---
 kernel/smp.c  |  50 +++
 kernel/up.c   |  18 +++
 10 files changed, 251 insertions(+), 59 deletions(-)
 create mode 100644 include/linux/hypervisor.h

-- 
2.6.2


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

[Xen-devel] [PATCH v2 1/6] xen: sync xen header

[Juergen Gross]

Import the actual version of include/xen/interface/sched.h from Xen.

Signed-off-by: Juergen Gross 
---
 include/xen/interface/sched.h | 100 ++
 1 file changed, 82 insertions(+), 18 deletions(-)

diff --git a/include/xen/interface/sched.h b/include/xen/interface/sched.h
index f184909..a4c4d73 100644
--- a/include/xen/interface/sched.h
+++ b/include/xen/interface/sched.h
@@ -3,6 +3,24 @@
  *
  * Scheduler state interactions
  *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
  * Copyright (c) 2005, Keir Fraser 
  */
 
@@ -12,18 +30,30 @@
 #include 
 
 /*
+ * Guest Scheduler Operations
+ *
+ * The SCHEDOP interface provides mechanisms for a guest to interact
+ * with the scheduler, including yield, blocking and shutting itself
+ * down.
+ */
+
+/*
  * The prototype for this hypercall is:
- *  long sched_op_new(int cmd, void *arg)
+ * long HYPERVISOR_sched_op(enum sched_op cmd, void *arg, ...)
+ *
  * @cmd == SCHEDOP_??? (scheduler operation).
  * @arg == Operation-specific extra argument(s), as described below.
+ * ...  == Additional Operation-specific extra arguments, described below.
  *
- * **NOTE**:
- * Versions of Xen prior to 3.0.2 provide only the following legacy version
+ * Versions of Xen prior to 3.0.2 provided only the following legacy version
  * of this hypercall, supporting only the commands yield, block and shutdown:
  *  long sched_op(int cmd, unsigned long arg)
  * @cmd == SCHEDOP_??? (scheduler operation).
  * @arg == 0   (SCHEDOP_yield and SCHEDOP_block)
  *  == SHUTDOWN_* code (SCHEDOP_shutdown)
+ *
+ * This legacy version is available to new guests as:
+ * long HYPERVISOR_sched_op_compat(enum sched_op cmd, unsigned long arg)
  */
 
 /*
@@ -44,12 +74,17 @@
 /*
  * Halt execution of this domain (all VCPUs) and notify the system controller.
  * @arg == pointer to sched_shutdown structure.
+ *
+ * If the sched_shutdown_t reason is SHUTDOWN_suspend then
+ * x86 PV guests must also set RDX (EDX for 32-bit guests) to the MFN
+ * of the guest's start info page.  RDX/EDX is the third hypercall
+ * argument.
+ *
+ * In addition, which reason is SHUTDOWN_suspend this hypercall
+ * returns 1 if suspend was cancelled or the domain was merely
+ * checkpointed, and 0 if it is resuming in a new domain.
  */
 #define SCHEDOP_shutdown2
-struct sched_shutdown {
-unsigned int reason; /* SHUTDOWN_* */
-};
-DEFINE_GUEST_HANDLE_STRUCT(sched_shutdown);
 
 /*
  * Poll a set of event-channel ports. Return when one or more are pending. An
@@ -57,12 +92,6 @@ DEFINE_GUEST_HANDLE_STRUCT(sched_shutdown);
  * @arg == pointer to sched_poll structure.
  */
 #define SCHEDOP_poll3
-struct sched_poll {
-GUEST_HANDLE(evtchn_port_t) ports;
-unsigned int nr_ports;
-uint64_t timeout;
-};
-DEFINE_GUEST_HANDLE_STRUCT(sched_poll);
 
 /*
  * Declare a shutdown for another domain. The main use of this function is
@@ -71,15 +100,11 @@ DEFINE_GUEST_HANDLE_STRUCT(sched_poll);
  * @arg == pointer to sched_remote_shutdown structure.
  */
 #define SCHEDOP_remote_shutdown4
-struct sched_remote_shutdown {
-domid_t domain_id; /* Remote domain ID */
-unsigned int reason;   /* SHUTDOWN_xxx reason */
-};
 
 /*
  * Latch a shutdown code, so that when the domain later shuts down it
  * reports this code to the control tools.
- * @arg == as for SCHEDOP_shutdown.
+ * @arg == sched_shutdown, as for SCHEDOP_shutdown.
  */
 #define SCHEDOP_shutdown_code 5
 
@@ -92,10 +117,47 @@ struct sched_remote_shutdown {
  * With id != 0 and timeout != 0, poke watchdog timer and set new timeout.
  */
 #define SCHEDOP_watchdog6
+
+/*
+ * Override the current vcpu affinity by pinning it to one physical cpu or
+ * undo this override restoring the previous affinity.
+ * @arg == pointer to sched_pin_override structure.
+ *
+ * A negative pcpu value will undo a previous pin override and restore the
+ * previous cpu affinity.
+ * T

[Xen-devel] [PATCH v2 6/6] xen: add xen_pin_vcpu() to support calling functions on a dedicated pcpu

[Juergen Gross]

Some hardware models (e.g. Dell Studio 1555 laptops) require calls to
the firmware to be issued on cpu 0 only. As Dom0 might have to use
these calls, add xen_pin_vcpu() to achieve this functionality.

In case either the domain doesn't have the privilege to make the
related hypercall or the hypervisor isn't supporting it, issue a
warning once and disable further pinning attempts.

Signed-off-by: Juergen Gross 
---
 arch/x86/xen/enlighten.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 2379a5a..157b3d9 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1885,6 +1885,45 @@ static void xen_set_cpu_features(struct cpuinfo_x86 *c)
}
 }
 
+static void xen_pin_vcpu(int cpu)
+{
+   static bool disable_pinning;
+   struct sched_pin_override pin_override;
+   int ret;
+
+   if (disable_pinning)
+   return;
+
+   pin_override.pcpu = cpu;
+   ret = HYPERVISOR_sched_op(SCHEDOP_pin_override, &pin_override);
+   if (cpu < 0)
+   return;
+
+   switch (ret) {
+   case -ENOSYS:
+   pr_warn("The kernel tried to call a function on physical cpu 
%d, but Xen isn't\n"
+   "supporting this. In case of problems you might 
consider vcpu pinning.\n",
+   cpu);
+   disable_pinning = true;
+   break;
+   case -EPERM:
+   WARN(1, "Trying to pin vcpu without having privilege to do 
so\n");
+   disable_pinning = true;
+   break;
+   case -EINVAL:
+   case -EBUSY:
+   pr_warn("The kernel tried to call a function on physical cpu 
%d, but this cpu\n"
+   "seems not to be available. Please check your Xen cpu 
configuration.\n",
+   cpu);
+   break;
+   case 0:
+   break;
+   default:
+   WARN(1, "rc %d while trying to pin vcpu\n", ret);
+   disable_pinning = true;
+   }
+}
+
 const struct hypervisor_x86 x86_hyper_xen = {
.name   = "Xen",
.detect = xen_platform,
@@ -1893,6 +1932,7 @@ const struct hypervisor_x86 x86_hyper_xen = {
 #endif
.x2apic_available   = xen_x2apic_para_available,
.set_cpu_features   = xen_set_cpu_features,
+   .pin_vcpu   = xen_pin_vcpu,
 };
 EXPORT_SYMBOL(x86_hyper_xen);
 
-- 
2.6.2


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

[Xen-devel] [PATCH v2 4/6] hwmon: use smp_call_sync_on_phys_cpu() for dell-smm i8k

[Juergen Gross]

Use the smp_call_sync_on_phys_cpu() function to call system management
mode on cpu 0.

Signed-off-by: Juergen Gross 
---
 drivers/hwmon/dell-smm-hwmon.c | 27 ---
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index c43318d..4875462 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -130,23 +130,15 @@ static inline const char *i8k_get_dmi_data(int field)
 /*
  * Call the System Management Mode BIOS. Code provided by Jonathan Buzzard.
  */
-static int i8k_smm(struct smm_regs *regs)
+static int i8k_smm_func(void *par)
 {
int rc;
+   struct smm_regs *regs = par;
int eax = regs->eax;
-   cpumask_var_t old_mask;
 
/* SMM requires CPU 0 */
-   if (!alloc_cpumask_var(&old_mask, GFP_KERNEL))
-   return -ENOMEM;
-   cpumask_copy(old_mask, ¤t->cpus_allowed);
-   rc = set_cpus_allowed_ptr(current, cpumask_of(0));
-   if (rc)
-   goto out;
-   if (smp_processor_id() != 0) {
-   rc = -EBUSY;
-   goto out;
-   }
+   if (smp_processor_id() != 0)
+   return -EBUSY;
 
 #if defined(CONFIG_X86_64)
asm volatile("pushq %%rax\n\t"
@@ -204,13 +196,18 @@ static int i8k_smm(struct smm_regs *regs)
if (rc != 0 || (regs->eax & 0x) == 0x || regs->eax == eax)
rc = -EINVAL;
 
-out:
-   set_cpus_allowed_ptr(current, old_mask);
-   free_cpumask_var(old_mask);
return rc;
 }
 
 /*
+ * Call the System Management Mode BIOS.
+ */
+static int i8k_smm(struct smm_regs *regs)
+{
+   return smp_call_sync_on_phys_cpu(0, i8k_smm_func, regs);
+}
+
+/*
  * Read the fan status.
  */
 static int i8k_get_fan_status(int fan)
-- 
2.6.2


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

[Xen-devel] [PATCH v2 2/6] smp: add function to execute a function synchronously on a physical cpu

[Juergen Gross]

On some hardware models (e.g. Dell Studio 1555 laptop) some hardware
related functions (e.g. SMIs) are to be executed on physical cpu 0
only. Instead of open coding such a functionality multiple times in
the kernel add a service function for this purpose. This will enable
the possibility to take special measures in virtualized environments
like Xen, too.

Signed-off-by: Juergen Gross 
---
V2: instead of manipulating the allowed set of cpus use cpu specific
workqueue as requested by Peter Zijlstra
---
 include/linux/smp.h |  2 ++
 kernel/smp.c| 44 
 kernel/up.c |  9 +
 3 files changed, 55 insertions(+)

diff --git a/include/linux/smp.h b/include/linux/smp.h
index c441407..fc9d21b 100644
--- a/include/linux/smp.h
+++ b/include/linux/smp.h
@@ -196,4 +196,6 @@ extern void arch_enable_nonboot_cpus_end(void);
 
 void smp_setup_processor_id(void);
 
+int smp_call_sync_on_phys_cpu(unsigned int cpu, int (*func)(void *), void 
*par);
+
 #endif /* __LINUX_SMP_H */
diff --git a/kernel/smp.c b/kernel/smp.c
index 7416544..62da74b 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -739,3 +739,47 @@ void wake_up_all_idle_cpus(void)
preempt_enable();
 }
 EXPORT_SYMBOL_GPL(wake_up_all_idle_cpus);
+
+/**
+ * smp_call_sync_on_phys_cpu - Call a function on a specific physical cpu
+ *
+ * Used to call a function on a specific physical cpu. Even if the specified
+ * cpu isn't the current one, return only after the called function has
+ * returned.
+ */
+struct smp_sync_call_struct {
+   struct work_struct  work;
+   struct completion   done;
+   int (*func)(void *);
+   void*data;
+   int ret;
+};
+
+static void smp_call_sync_callback(struct work_struct *work)
+{
+   struct smp_sync_call_struct *sscs;
+
+   sscs = container_of(work, struct smp_sync_call_struct, work);
+   sscs->ret = sscs->func(sscs->data);
+
+   complete(&sscs->done);
+}
+
+int smp_call_sync_on_phys_cpu(unsigned int cpu, int (*func)(void *), void *par)
+{
+   struct smp_sync_call_struct sscs = {
+   .work = __WORK_INITIALIZER(sscs.work, smp_call_sync_callback),
+   .done = COMPLETION_INITIALIZER_ONSTACK(sscs.done),
+   .func = func,
+   .data = par,
+   };
+
+   if (cpu >= nr_cpu_ids)
+   return -EINVAL;
+
+   queue_work_on(cpu, system_wq, &sscs.work);
+   wait_for_completion(&sscs.done);
+
+   return sscs.ret;
+}
+EXPORT_SYMBOL_GPL(smp_call_sync_on_phys_cpu);
diff --git a/kernel/up.c b/kernel/up.c
index 1760bf3..afd395c 100644
--- a/kernel/up.c
+++ b/kernel/up.c
@@ -82,3 +82,12 @@ void on_each_cpu_cond(bool (*cond_func)(int cpu, void *info),
preempt_enable();
 }
 EXPORT_SYMBOL(on_each_cpu_cond);
+
+int smp_call_sync_on_phys_cpu(unsigned int cpu, int (*func)(void *), void *par)
+{
+   if (cpu != 0)
+   return -EINVAL;
+
+   return func(par);
+}
+EXPORT_SYMBOL_GPL(smp_call_sync_on_phys_cpu);
-- 
2.6.2


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

Re: [Xen-devel] [PATCH 1/2] IOMMU/MMU: Adjust top level functions for VT-d Device-TLB flush error.

[Jan Beulich]

>>> On 21.03.16 at 07:18,  wrote:
>>  From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Friday, March 18, 2016 5:49 PM
>> 
>> >>> On 18.03.16 at 10:38,  wrote:
>> > On Fri, 2016-03-18 at 03:29 -0600, Jan Beulich wrote:
>> >> >
>> >> Not sure what exactly you're asking for: As said, we first need to
>> >> settle on an abstract model. Do we want IOMMU mapping failures
>> >> to be fatal to the domain (perhaps with the exception of the
>> >> hardware one)? I think we do, and for the hardware domain we'd
>> >> do things on a best effort basis (always erring on the side of
>> >> unmapping). Which would probably mean crashing the domain
>> >> could be centralized in iommu_{,un}map_page(). How much roll
>> >> back would then still be needed in callers of these functions for
>> >> the hardware domain's sake would need to be seen.
>> >>
>> >> So before you start coing, give others (namely but not limited to
>> >> VT-d, AMD IOMMU, other x86, and x86/mm maintainers) a chance
>> >> to voice differing opinions.
>> >>
>> > I'm nothing of the above but,
>> 
>> Don't you fall under "but not limited to"  ;-) ?
>> 
>> > FWIW, the behavior Jan described
>> > (crashing the domain for all domains but the hardware domain) was
>> > indeed the intended plan for this series, as far as I understood from
>> > talking to people and looking at previous email conversations and
>> > submissions.
>> 
>> That was taking only the flush timeout as an error source into account.
>> Now that we see that the lack of error handling pre-exists, we can't
>> just extend that intended model to also cover those other error
>> reasons without at least having given people a chance to object.
>> 
> 
> It makes sense so I'm OK with this behavior change. 
> 
> Then regarding to Quan's next version (if nobody disagrees), is it better
> to first describe related callers and then reach agreement which caller
> still needs error handling for hardware domain, before Quan starts to
> change actual code (at least based on current discussion Quan didn't
> have thorough understanding of such necessity yet)?

Well, ideally we'd indeed reach agreement before starting to write
or change code. However, in a case like this where error handling
was just ignored in pre-existing code, I think the easiest way to get
a complete picture is to see what places / paths need changing. I.e.
here it may indeed be better to start from the patches. Whether that
means posting the patches in patch form, or - prior to posting them -
extracting what was learned into a textual description is a different
aspect (and I'd leave it to Quan to pick the route more suitable to him).

Jan


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

Re: [Xen-devel] [PATCH v6 02/22] arm/acpi: Add a helper function to get the acpi table offset

[Julien Grall]

Hi Shannon,

On 17/03/2016 09:40, Shannon Zhao wrote:

From: Shannon Zhao 

These tables are aligned with 64bit.

Signed-off-by: Shannon Zhao 
---
  xen/arch/arm/acpi/lib.c| 15 +++
  xen/include/asm-arm/acpi.h |  2 ++
  2 files changed, 17 insertions(+)

diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
index db5c4d8..79f7edd 100644
--- a/xen/arch/arm/acpi/lib.c
+++ b/xen/arch/arm/acpi/lib.c
@@ -60,3 +60,18 @@ bool_t __init acpi_psci_hvc_present(void)
  {
  return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
  }
+
+paddr_t __init acpi_get_table_offset(struct membank tbl_add[],
+ EFI_MEM_RES index)


Without looking at the callers, the usage of this function is very 
unclear. For instance what does mean the offset in this case?


Furthermore, you are assuming that all the tables before the given index 
have all been created and will never be modified. So all the code flow 
to generate the ACPI tables is inflexible.


I'm fine for now with this kind of assumption, but this need to be spell 
out in the header.



+{
+int i;
+paddr_t offset = 0;
+
+for ( i = 0; i < index; i++ )
+{
+/* Aligned with 64bit (8 bytes) */
+offset += ROUNDUP(tbl_add[i].size, 8);
+}
+
+return offset;
+}
diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
index 7f59761..569fc31 100644
--- a/xen/include/asm-arm/acpi.h
+++ b/xen/include/asm-arm/acpi.h
@@ -25,6 +25,7 @@

  #include 
  #include 
+#include 

  #define COMPILER_DEPENDENT_INT64   long long
  #define COMPILER_DEPENDENT_UINT64  unsigned long long
@@ -45,6 +46,7 @@ typedef enum {
  bool_t __init acpi_psci_present(void);
  bool_t __init acpi_psci_hvc_present(void);
  void __init acpi_smp_init_cpus(void);
+paddr_t acpi_get_table_offset(struct membank tbl_add[], EFI_MEM_RES index);

  #ifdef CONFIG_ACPI
  extern bool_t acpi_disabled;



Regards,

--
Julien Grall

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

Re: [Xen-devel] Interested to participate in Outreachy Program

[sabiya kazi]

Hi Doug,
Can you please help on questions/problems where I am stuck?
Deadline for applying for outreachy is coming closer.
Regards,
Sabiya

Hi Doug,
 I am done with building of xen source.Now, I have started looking
at source files and identifying changes required for given task.

   As you suggested, I went through virsh command source to get idea how
escape sequence char option is implemented. Based on that,I came up with
following findings:
*To complete this task, Steps will be*


   1. Do help related change in libxl/xl_cmdtable.c
   2. Add handling of one more option for command 'xl console' in file
   xl_cmdimpl.c in method  main_console.  From this method, call is delegated
   to libxl_console_exec() in libxl.c
   3. libxl_console_exec delegates to execl(), Do some changes in this
   method.


*Question/Doubts:*

   -  Where I need to use this escape char?
   -  libxl_console_exec() is called by many methods Will it affect other
   flows as well?
   - Could not find execl method implementation where it actually prints
   data
   - Changes for handling escape char are not cleared

Can you please help me on this?
Regards,
-Sabiya



On Sat, Mar 19, 2016 at 11:31 PM, sabiya kazi  wrote:

> Hi Doug,
> I have started building xen source.
> Can you help me on issue of creating guest domain?
>
>
> Regards,
> -Sabiya
>
> On Fri, Mar 18, 2016 at 1:18 AM, sabiya kazi  wrote:
>
>> Hi Doug,
>>
>> I have proceeded on xen installation. I have configured grub to use xen,
>> I could see xl command and tried it's few options.
>>
>> However, I am getting failure while creating a Debian PV Guest using
>>
>> xen-create-image --hostname=tutorial-pv-guest \
>>   --memory=512mb \
>>   --vcpus=2 \
>>   --lvm=vg0 \
>>   --dhcp \
>>   --pygrub \
>>   --dist=wheezy
>>
>> I have attached logs for reference.
>>  Before this step, I could not setup Linux Bridge Network for Guest
>> networking.
>> I am using my router for internet connection.Can this will be problem?
>> Can you please have a look and let me know.
>>Now, One more question to make changes for "Xl command" I need to
>> check it's source, I have checked out source from xen repository.
>> Can you share path of xl and virsh command source, I can start looking at
>> source..
>>
>> Rega
>>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel