[Xen-devel] [xen-4.11-testing test] 144753: tolerable FAIL - PUSHED

2019-12-12 Thread osstest service owner
flight 144753 xen-4.11-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144753/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass

version targeted for testing:
 xen  14b62ab3e5a79816edfc6dd3afce1bb68c106ac5
baseline version:
 xen  f562c6bb93a284033bf6f5af06287a71bc40a110

Last test of basis   144716  2019-12-11 14:36:36 Z1 days
Testing same since   144753  2019-12-12 05:26:41 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Jeremi Piotrowski 
  Krzysztof Kolasa 
  Mark Pryor 

jobs:
 

Re: [Xen-devel] [PATCH v2 0/2] [PATCH-for-4.13] Work towards removing brctl

2019-12-12 Thread Jürgen Groß

On 13.12.19 05:08, Steven Haigh wrote:

Start updating scripts for network functionality

(Resending as the patch emails seem to have been eaten somewhere)

The scripts for networking in Xen have a mixture of formatting,
tab spacing, space spacing inconsistencies.

We also have issues where CentOS 8 does not have brctl - being
replaced with ip / bridge commands.

This series starts cleaning up whitespace and formatting, as well
as starts adding conditionals for using brctl (if present) but using
ip if /usr/sbin/brctl is not installed.

Changes since v1
   * Fixed reference to /usr/bin/brctl instead of /usr/sbin/brctl

Steven Haigh (2):
   Tidy up whitespace and formatting in file to be consistent.
   Use ip for bridge related functions where brctl is not present

  tools/hotplug/Linux/colo-proxy-setup  |  30 +++--
  tools/hotplug/Linux/vif-bridge|  19 ++-
  tools/hotplug/Linux/vif2  |  12 +-
  tools/hotplug/Linux/xen-network-common.sh | 151 +++---
  4 files changed, 121 insertions(+), 91 deletions(-)



As this is no 4.13 regression its too late for the patches to make it
into 4.13, sorry.

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

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

2019-12-12 Thread Jürgen Groß

On 12.12.19 20:05, David Miller wrote:

From: Paul Durrant 
Date: Thu, 12 Dec 2019 13:54:06 +


In the past it used to be the case that the Xen toolstack relied upon
udev to execute backend hotplug scripts. However this has not been the
case for many releases now and removal of the associated code in
xen-netback shortens the source by more than 100 lines, and removes much
complexity in the interaction with the xenstore backend state.

NOTE: xen-netback is the only xenbus driver to have a functional uevent()
   method. The only other driver to have a method at all is
   pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
   Hence this patch also facilitates further cleanup.

Signed-off-by: Paul Durrant 


If userspace ever used this stuff, I seriously doubt you can remove this
even if it hasn't been used in 5+ years.


Hmm, depends.

This has been used by Xen tools in dom0 only. If the last usage has been
in a Xen version which is no longer able to run with current Linux in
dom0 it could be removed. But I guess this would have to be a rather old
version of Xen (like 3.x?).

Paul, can you give a hint since which Xen version the toolstack no
longer relies on udev to start the hotplug scripts?


Juergen

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

[Xen-devel] [PATCH 1/2] Tidy up whitespace and formatting in file to be consistent.

2019-12-12 Thread Steven Haigh
Signed-off-by: Steven Haigh 
---
 tools/hotplug/Linux/xen-network-common.sh | 144 +++---
 1 file changed, 70 insertions(+), 74 deletions(-)

diff --git a/tools/hotplug/Linux/xen-network-common.sh 
b/tools/hotplug/Linux/xen-network-common.sh
index 92ffa603f7..ab76827a64 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -26,118 +26,114 @@
 #   that the virtual device will take once the physical device has
 #   been renamed.
 
-if ! which ifup >/dev/null 2>/dev/null
-then
-  preiftransfer()
-  {
-true
-  }
-  ifup()
-  {
-false
-  }
-  ifdown()
-  {
-false
-  }
+if ! which ifup >/dev/null 2>/dev/null; then
+   preiftransfer()
+   {
+   true
+   }
+   ifup()
+   {
+   false
+   }
+   ifdown()
+   {
+   false
+   }
 else
-  preiftransfer()
-  {
-true
-  }
+   preiftransfer()
+   {
+   true
+   }
 fi
 
 
 first_file()
 {
-  t="$1"
-  shift
-  for file in $@
-  do
-if [ "$t" "$file" ]
-then
-  echo "$file"
-  return
-fi
-  done
+   t="$1"
+   shift
+   for file in $@; do
+   if [ "$t" "$file" ]; then
+   echo "$file"
+   return
+   fi
+   done
 }
 
 find_dhcpd_conf_file()
 {
-  first_file -f /etc/dhcp3/dhcpd.conf /etc/dhcpd.conf
+   first_file -f /etc/dhcp3/dhcpd.conf /etc/dhcpd.conf
 }
 
 
 find_dhcpd_init_file()
 {
-  first_file -x /etc/init.d/{dhcp3-server,dhcp,dhcpd}
+   first_file -x /etc/init.d/{dhcp3-server,dhcp,dhcpd}
 }
 
 find_dhcpd_arg_file()
 {
-  first_file -f /etc/sysconfig/dhcpd /etc/defaults/dhcp 
/etc/default/dhcp3-server
+   first_file -f /etc/sysconfig/dhcpd /etc/defaults/dhcp 
/etc/default/dhcp3-server
 }
 
 # configure interfaces which act as pure bridge ports:
 _setup_bridge_port() {
-local dev="$1"
-local virtual="$2"
-
-# take interface down ...
-ip link set dev ${dev} down
-
-if [ $virtual -ne 0 ] ; then
-# Initialise a dummy MAC address. We choose the numerically
-# largest non-broadcast address to prevent the address getting
-# stolen by an Ethernet bridge for STP purposes.
-# (FE:FF:FF:FF:FF:FF)
-ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true
-fi
-
-# ... and configure it
-ip address flush dev ${dev}
+   local dev="$1"
+   local virtual="$2"
+
+   # take interface down ...
+   ip link set dev ${dev} down
+
+   if [ $virtual -ne 0 ]; then
+   # Initialise a dummy MAC address. We choose the numerically
+   # largest non-broadcast address to prevent the address getting
+   # stolen by an Ethernet bridge for STP purposes.
+   # (FE:FF:FF:FF:FF:FF)
+   ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true
+   fi
+
+   # ... and configure it
+   ip address flush dev ${dev}
 }
 
 setup_physical_bridge_port() {
-_setup_bridge_port $1 0
+   _setup_bridge_port $1 0
 }
 setup_virtual_bridge_port() {
-_setup_bridge_port $1 1
+   _setup_bridge_port $1 1
 }
 
 # Usage: create_bridge bridge
 create_bridge () {
-local bridge=$1
-
-# Don't create the bridge if it already exists.
-if [ ! -e "/sys/class/net/${bridge}/bridge" ]; then
-   brctl addbr ${bridge}
-   brctl stp ${bridge} off
-   brctl setfd ${bridge} 0
-fi
+   local bridge=$1
+
+   # Don't create the bridge if it already exists.
+   if [ ! -e "/sys/class/net/${bridge}/bridge" ]; then
+   brctl addbr ${bridge}
+   brctl stp ${bridge} off
+   brctl setfd ${bridge} 0
+   fi
 }
 
 # Usage: add_to_bridge bridge dev
 add_to_bridge () {
-local bridge=$1
-local dev=$2
-
-# Don't add $dev to $bridge if it's already on a bridge.
-if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then
-   ip link set dev ${dev} up || true
-   return
-fi
-brctl addif ${bridge} ${dev}
-ip link set dev ${dev} up
+   local bridge=$1
+   local dev=$2
+
+   # Don't add $dev to $bridge if it's already on a bridge.
+   if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then
+   ip link set dev ${dev} up || true
+   return
+   fi
+   brctl addif ${bridge} ${dev}
+   ip link set dev ${dev} up
 }
 
 # Usage: set_mtu bridge dev
 set_mtu () {
-local bridge=$1
-local dev=$2
-mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`"
-if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]
-then
-ip link set dev ${dev} mtu $mtu || :
-fi
+   local bridge=$1
+   local dev=$2
+   mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`"
+   if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]; then
+   ip link set dev ${dev} mtu $mtu || :
+   fi
 }
-- 
2.24.1



[Xen-devel] [PATCH 2/2] Use ip for bridge related functions where brctl is not present

2019-12-12 Thread Steven Haigh
Signed-off-by: Steven Haigh 
---
 tools/hotplug/Linux/colo-proxy-setup  | 30 +--
 tools/hotplug/Linux/vif-bridge| 19 --
 tools/hotplug/Linux/vif2  | 12 +++--
 tools/hotplug/Linux/xen-network-common.sh | 15 +---
 4 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/tools/hotplug/Linux/colo-proxy-setup 
b/tools/hotplug/Linux/colo-proxy-setup
index 94e2034452..cbd5b773c6 100755
--- a/tools/hotplug/Linux/colo-proxy-setup
+++ b/tools/hotplug/Linux/colo-proxy-setup
@@ -76,10 +76,17 @@ function teardown_primary()
 
 function setup_secondary()
 {
-do_without_error brctl delif $bridge $vifname
-do_without_error brctl addbr $forwardbr
-do_without_error brctl addif $forwardbr $vifname
-do_without_error brctl addif $forwardbr $forwarddev
+if [ -x "/usr/sbin/brctl" ]; then
+do_without_error brctl delif $bridge $vifname
+do_without_error brctl addbr $forwardbr
+do_without_error brctl addif $forwardbr $vifname
+do_without_error brctl addif $forwardbr $forwarddev
+else
+do_without_error ip link set $vifname nomaster
+do_without_error ip link add name $forwardbr type bridge
+do_without_error ip link set $vifname master $forwardbr
+do_without_error ip link set $forwarddev master $forwardbr
+fi
 do_without_error ip link set dev $forwardbr up
 do_without_error modprobe xt_SECCOLO
 
@@ -91,10 +98,17 @@ function setup_secondary()
 
 function teardown_secondary()
 {
-do_without_error brctl delif $forwardbr $forwarddev
-do_without_error brctl delif $forwardbr $vifname
-do_without_error brctl delbr $forwardbr
-do_without_error brctl addif $bridge $vifname
+if [ -x "/usr/sbin/brctl" ]; then
+do_without_error brctl delif $forwardbr $forwarddev
+do_without_error brctl delif $forwardbr $vifname
+do_without_error brctl delbr $forwardbr
+do_without_error brctl addif $bridge $vifname
+else
+do_without_error ip link set $forwarddev nomaster
+do_without_error ip link set $vifname nomaster
+do_without_error ip link delete $forwardbr type bridge
+do_without_error ip link set $vifname master $bridge
+fi
 
 do_without_error iptables -t mangle -D PREROUTING -m physdev --physdev-in \
 $vifname -j SECCOLO --index $index
diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index 6956dea66a..e035411934 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -31,12 +31,13 @@ dir=$(dirname "$0")
 bridge=${bridge:-}
 bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
 
-if [ -z "$bridge" ]
-then
-  bridge=$(brctl show | awk 'NR==2{print$1}')
-
-  if [ -z "$bridge" ]
-  then
+if [ -z "$bridge" ]; then
+if [ -x "/usr/sbin/brctl" ]; then
+bridge=$(brctl show | awk 'NR==2{print$1}')
+else
+bridge=$(bridge link | cut -d" " -f7)
+fi
+  if [ -z "$bridge" ]; then
  fatal "Could not find bridge, and none was specified"
   fi
 else
@@ -82,7 +83,11 @@ case "$command" in
 ;;
 
 offline)
-do_without_error brctl delif "$bridge" "$dev"
+if [ -x "/usr/sbin/brctl"]; then
+do_without_error brctl delif "$bridge" "$dev"
+else
+do_without_error ip link set "$dev" nomaster
+fi
 do_without_error ifconfig "$dev" down
 ;;
 
diff --git a/tools/hotplug/Linux/vif2 b/tools/hotplug/Linux/vif2
index 2c155be68c..e36070cbbb 100644
--- a/tools/hotplug/Linux/vif2
+++ b/tools/hotplug/Linux/vif2
@@ -7,13 +7,21 @@ dir=$(dirname "$0")
 bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
 if [ -z "$bridge" ]
 then
-nr_bridges=$(($(brctl show | cut -f 1 | grep -v "^$" | wc -l) - 1))
+if [ -x "/usr/sbin/brctl" ]; then
+nr_bridges=$(($(brctl show | cut -f 1 | grep -v "^$" | wc -l) - 1))
+else
+nr_bridges=$(bridge link | wc -l)
+fi
 if [ "$nr_bridges" != 1 ]
then
fatal "no bridge specified, and don't know which one to use 
($nr_bridges found)"
 fi
-bridge=$(brctl show | cut -d "
+if [ -x "/usr/sbin/brctl" ]; then
+bridge=$(brctl show | cut -d "
 " -f 2 | cut -f 1)
+else
+bridge=$(bridge link | cut -d" " -f6)
+fi
 fi
 
 command="$1"
diff --git a/tools/hotplug/Linux/xen-network-common.sh 
b/tools/hotplug/Linux/xen-network-common.sh
index ab76827a64..7833deac6c 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -108,9 +108,12 @@ create_bridge () {
 
# Don't create the bridge if it already exists.
if [ ! -e "/sys/class/net/${bridge}/bridge" ]; then
-   brctl addbr ${bridge}
-   brctl stp ${bridge} off
-   brctl setfd ${bridge} 0
+   if [ -x "/usr/sbin/brctl" ]; then
+   brctl addbr ${bridge}
+   

[Xen-devel] [PATCH v2 0/2] [PATCH-for-4.13] Work towards removing brctl

2019-12-12 Thread Steven Haigh
Start updating scripts for network functionality

(Resending as the patch emails seem to have been eaten somewhere)

The scripts for networking in Xen have a mixture of formatting,
tab spacing, space spacing inconsistencies.

We also have issues where CentOS 8 does not have brctl - being
replaced with ip / bridge commands.

This series starts cleaning up whitespace and formatting, as well
as starts adding conditionals for using brctl (if present) but using
ip if /usr/sbin/brctl is not installed.

Changes since v1
  * Fixed reference to /usr/bin/brctl instead of /usr/sbin/brctl

Steven Haigh (2):
  Tidy up whitespace and formatting in file to be consistent.
  Use ip for bridge related functions where brctl is not present

 tools/hotplug/Linux/colo-proxy-setup  |  30 +++--
 tools/hotplug/Linux/vif-bridge|  19 ++-
 tools/hotplug/Linux/vif2  |  12 +-
 tools/hotplug/Linux/xen-network-common.sh | 151 +++---
 4 files changed, 121 insertions(+), 91 deletions(-)

-- 
2.24.1

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

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

2019-12-12 Thread osstest service owner
flight 144747 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144747/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-xl-credit1 16 guest-start/debian.repeat fail REGR. vs. 144712

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail  like 144712
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144712
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 144712
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 144712
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144712
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 144712
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 144712
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 144712
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144712
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 144712
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 144712
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass

version targeted for testing:
 xen  b4f042236ae0bb6725b3e8dd40af5a2466a6f971
baseline version:
 xen  272c18435e93cbf749c096a9552ab5ef0d79a4ca

Last test of basis   144712  2019-12-11 12:06:14 Z1 days
Testing same since   144747  2019-12-12 01:07:03 Z1 days1 attempts


People who touched 

[Xen-devel] [PATCH v2 0/2] [PATCH-for-4.13] Work towards removing brctl

2019-12-12 Thread Steven Haigh
Start updating scripts for network functionality

The scripts for networking in Xen have a mixture of formatting,
tab spacing, space spacing inconsistencies.

We also have issues where CentOS 8 does not have brctl - being
replaced with ip / bridge commands.

This series starts cleaning up whitespace and formatting, as well
as starts adding conditionals for using brctl (if present) but using
ip if /usr/sbin/brctl is not installed.

Changes since v1
  * Fixed reference to /usr/bin/brctl instead of /usr/sbin/brctl

Steven Haigh (2):
  Tidy up whitespace and formatting in file to be consistent.
  Use ip for bridge related functions where brctl is not present

 tools/hotplug/Linux/colo-proxy-setup  |  30 +++--
 tools/hotplug/Linux/vif-bridge|  19 ++-
 tools/hotplug/Linux/vif2  |  12 +-
 tools/hotplug/Linux/xen-network-common.sh | 151 +++---
 4 files changed, 121 insertions(+), 91 deletions(-)

-- 
2.24.1


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

[Xen-devel] [PATCH 0/2] [PATCH-for-4.13] Work towards removing brctl

2019-12-12 Thread Steven Haigh
Start updating scripts for network functionality

The scripts for networking in Xen have a mixture of formatting,
tab spacing, space spacing inconsistencies.

We also have issues where CentOS 8 does not have brctl - being
replaced with ip / bridge commands.

This series starts cleaning up whitespace and formatting, as well
as starts adding conditionals for using brctl (if present) but using
ip if /usr/sbin/brctl is not installed.

Steven Haigh (2):
  Tidy up whitespace and formatting in file to be consistent.
  Use ip for bridge related functions where brctl is not present

 tools/hotplug/Linux/colo-proxy-setup  |  30 +++--
 tools/hotplug/Linux/vif-bridge|  19 ++-
 tools/hotplug/Linux/vif2  |  12 +-
 tools/hotplug/Linux/xen-network-common.sh | 151 +++---
 4 files changed, 121 insertions(+), 91 deletions(-)

-- 
2.24.1


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

[Xen-devel] [PATCH 1/2] Tidy up whitespace and formatting in file to be consistent.

2019-12-12 Thread Steven Haigh
Signed-off-by: Steven Haigh 
---
 tools/hotplug/Linux/xen-network-common.sh | 144 +++---
 1 file changed, 70 insertions(+), 74 deletions(-)

diff --git a/tools/hotplug/Linux/xen-network-common.sh 
b/tools/hotplug/Linux/xen-network-common.sh
index 92ffa603f7..ab76827a64 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -26,118 +26,114 @@
 #   that the virtual device will take once the physical device has
 #   been renamed.
 
-if ! which ifup >/dev/null 2>/dev/null
-then
-  preiftransfer()
-  {
-true
-  }
-  ifup()
-  {
-false
-  }
-  ifdown()
-  {
-false
-  }
+if ! which ifup >/dev/null 2>/dev/null; then
+   preiftransfer()
+   {
+   true
+   }
+   ifup()
+   {
+   false
+   }
+   ifdown()
+   {
+   false
+   }
 else
-  preiftransfer()
-  {
-true
-  }
+   preiftransfer()
+   {
+   true
+   }
 fi
 
 
 first_file()
 {
-  t="$1"
-  shift
-  for file in $@
-  do
-if [ "$t" "$file" ]
-then
-  echo "$file"
-  return
-fi
-  done
+   t="$1"
+   shift
+   for file in $@; do
+   if [ "$t" "$file" ]; then
+   echo "$file"
+   return
+   fi
+   done
 }
 
 find_dhcpd_conf_file()
 {
-  first_file -f /etc/dhcp3/dhcpd.conf /etc/dhcpd.conf
+   first_file -f /etc/dhcp3/dhcpd.conf /etc/dhcpd.conf
 }
 
 
 find_dhcpd_init_file()
 {
-  first_file -x /etc/init.d/{dhcp3-server,dhcp,dhcpd}
+   first_file -x /etc/init.d/{dhcp3-server,dhcp,dhcpd}
 }
 
 find_dhcpd_arg_file()
 {
-  first_file -f /etc/sysconfig/dhcpd /etc/defaults/dhcp 
/etc/default/dhcp3-server
+   first_file -f /etc/sysconfig/dhcpd /etc/defaults/dhcp 
/etc/default/dhcp3-server
 }
 
 # configure interfaces which act as pure bridge ports:
 _setup_bridge_port() {
-local dev="$1"
-local virtual="$2"
-
-# take interface down ...
-ip link set dev ${dev} down
-
-if [ $virtual -ne 0 ] ; then
-# Initialise a dummy MAC address. We choose the numerically
-# largest non-broadcast address to prevent the address getting
-# stolen by an Ethernet bridge for STP purposes.
-# (FE:FF:FF:FF:FF:FF)
-ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true
-fi
-
-# ... and configure it
-ip address flush dev ${dev}
+   local dev="$1"
+   local virtual="$2"
+
+   # take interface down ...
+   ip link set dev ${dev} down
+
+   if [ $virtual -ne 0 ]; then
+   # Initialise a dummy MAC address. We choose the numerically
+   # largest non-broadcast address to prevent the address getting
+   # stolen by an Ethernet bridge for STP purposes.
+   # (FE:FF:FF:FF:FF:FF)
+   ip link set dev ${dev} address fe:ff:ff:ff:ff:ff || true
+   fi
+
+   # ... and configure it
+   ip address flush dev ${dev}
 }
 
 setup_physical_bridge_port() {
-_setup_bridge_port $1 0
+   _setup_bridge_port $1 0
 }
 setup_virtual_bridge_port() {
-_setup_bridge_port $1 1
+   _setup_bridge_port $1 1
 }
 
 # Usage: create_bridge bridge
 create_bridge () {
-local bridge=$1
-
-# Don't create the bridge if it already exists.
-if [ ! -e "/sys/class/net/${bridge}/bridge" ]; then
-   brctl addbr ${bridge}
-   brctl stp ${bridge} off
-   brctl setfd ${bridge} 0
-fi
+   local bridge=$1
+
+   # Don't create the bridge if it already exists.
+   if [ ! -e "/sys/class/net/${bridge}/bridge" ]; then
+   brctl addbr ${bridge}
+   brctl stp ${bridge} off
+   brctl setfd ${bridge} 0
+   fi
 }
 
 # Usage: add_to_bridge bridge dev
 add_to_bridge () {
-local bridge=$1
-local dev=$2
-
-# Don't add $dev to $bridge if it's already on a bridge.
-if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then
-   ip link set dev ${dev} up || true
-   return
-fi
-brctl addif ${bridge} ${dev}
-ip link set dev ${dev} up
+   local bridge=$1
+   local dev=$2
+
+   # Don't add $dev to $bridge if it's already on a bridge.
+   if [ -e "/sys/class/net/${bridge}/brif/${dev}" ]; then
+   ip link set dev ${dev} up || true
+   return
+   fi
+   brctl addif ${bridge} ${dev}
+   ip link set dev ${dev} up
 }
 
 # Usage: set_mtu bridge dev
 set_mtu () {
-local bridge=$1
-local dev=$2
-mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`"
-if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]
-then
-ip link set dev ${dev} mtu $mtu || :
-fi
+   local bridge=$1
+   local dev=$2
+   mtu="`ip link show dev ${bridge}| awk '/mtu/ { print $5 }'`"
+   if [ -n "$mtu" ] && [ "$mtu" -gt 0 ]; then
+   ip link set dev ${dev} mtu $mtu || :
+   fi
 }
-- 
2.24.1



[Xen-devel] [PATCH 2/2] Use ip for bridge related functions where brctl is not present

2019-12-12 Thread Steven Haigh
Signed-off-by: Steven Haigh 
---
 tools/hotplug/Linux/colo-proxy-setup  | 30 +--
 tools/hotplug/Linux/vif-bridge| 19 --
 tools/hotplug/Linux/vif2  | 12 +++--
 tools/hotplug/Linux/xen-network-common.sh | 15 +---
 4 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/tools/hotplug/Linux/colo-proxy-setup 
b/tools/hotplug/Linux/colo-proxy-setup
index 94e2034452..690021d10a 100755
--- a/tools/hotplug/Linux/colo-proxy-setup
+++ b/tools/hotplug/Linux/colo-proxy-setup
@@ -76,10 +76,17 @@ function teardown_primary()
 
 function setup_secondary()
 {
-do_without_error brctl delif $bridge $vifname
-do_without_error brctl addbr $forwardbr
-do_without_error brctl addif $forwardbr $vifname
-do_without_error brctl addif $forwardbr $forwarddev
+if [ -x "/usr/bin/brctl" ]; then
+do_without_error brctl delif $bridge $vifname
+do_without_error brctl addbr $forwardbr
+do_without_error brctl addif $forwardbr $vifname
+do_without_error brctl addif $forwardbr $forwarddev
+else
+do_without_error ip link set $vifname nomaster
+do_without_error ip link add name $forwardbr type bridge
+do_without_error ip link set $vifname master $forwardbr
+do_without_error ip link set $forwarddev master $forwardbr
+fi
 do_without_error ip link set dev $forwardbr up
 do_without_error modprobe xt_SECCOLO
 
@@ -91,10 +98,17 @@ function setup_secondary()
 
 function teardown_secondary()
 {
-do_without_error brctl delif $forwardbr $forwarddev
-do_without_error brctl delif $forwardbr $vifname
-do_without_error brctl delbr $forwardbr
-do_without_error brctl addif $bridge $vifname
+if [ -x "/usr/bin/brctl" ]; then
+do_without_error brctl delif $forwardbr $forwarddev
+do_without_error brctl delif $forwardbr $vifname
+do_without_error brctl delbr $forwardbr
+do_without_error brctl addif $bridge $vifname
+else
+do_without_error ip link set $forwarddev nomaster
+do_without_error ip link set $vifname nomaster
+do_without_error ip link delete $forwardbr type bridge
+do_without_error ip link set $vifname master $bridge
+fi
 
 do_without_error iptables -t mangle -D PREROUTING -m physdev --physdev-in \
 $vifname -j SECCOLO --index $index
diff --git a/tools/hotplug/Linux/vif-bridge b/tools/hotplug/Linux/vif-bridge
index 6956dea66a..e035411934 100644
--- a/tools/hotplug/Linux/vif-bridge
+++ b/tools/hotplug/Linux/vif-bridge
@@ -31,12 +31,13 @@ dir=$(dirname "$0")
 bridge=${bridge:-}
 bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
 
-if [ -z "$bridge" ]
-then
-  bridge=$(brctl show | awk 'NR==2{print$1}')
-
-  if [ -z "$bridge" ]
-  then
+if [ -z "$bridge" ]; then
+if [ -x "/usr/sbin/brctl" ]; then
+bridge=$(brctl show | awk 'NR==2{print$1}')
+else
+bridge=$(bridge link | cut -d" " -f7)
+fi
+  if [ -z "$bridge" ]; then
  fatal "Could not find bridge, and none was specified"
   fi
 else
@@ -82,7 +83,11 @@ case "$command" in
 ;;
 
 offline)
-do_without_error brctl delif "$bridge" "$dev"
+if [ -x "/usr/sbin/brctl"]; then
+do_without_error brctl delif "$bridge" "$dev"
+else
+do_without_error ip link set "$dev" nomaster
+fi
 do_without_error ifconfig "$dev" down
 ;;
 
diff --git a/tools/hotplug/Linux/vif2 b/tools/hotplug/Linux/vif2
index 2c155be68c..e36070cbbb 100644
--- a/tools/hotplug/Linux/vif2
+++ b/tools/hotplug/Linux/vif2
@@ -7,13 +7,21 @@ dir=$(dirname "$0")
 bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
 if [ -z "$bridge" ]
 then
-nr_bridges=$(($(brctl show | cut -f 1 | grep -v "^$" | wc -l) - 1))
+if [ -x "/usr/sbin/brctl" ]; then
+nr_bridges=$(($(brctl show | cut -f 1 | grep -v "^$" | wc -l) - 1))
+else
+nr_bridges=$(bridge link | wc -l)
+fi
 if [ "$nr_bridges" != 1 ]
then
fatal "no bridge specified, and don't know which one to use 
($nr_bridges found)"
 fi
-bridge=$(brctl show | cut -d "
+if [ -x "/usr/sbin/brctl" ]; then
+bridge=$(brctl show | cut -d "
 " -f 2 | cut -f 1)
+else
+bridge=$(bridge link | cut -d" " -f6)
+fi
 fi
 
 command="$1"
diff --git a/tools/hotplug/Linux/xen-network-common.sh 
b/tools/hotplug/Linux/xen-network-common.sh
index ab76827a64..7833deac6c 100644
--- a/tools/hotplug/Linux/xen-network-common.sh
+++ b/tools/hotplug/Linux/xen-network-common.sh
@@ -108,9 +108,12 @@ create_bridge () {
 
# Don't create the bridge if it already exists.
if [ ! -e "/sys/class/net/${bridge}/bridge" ]; then
-   brctl addbr ${bridge}
-   brctl stp ${bridge} off
-   brctl setfd ${bridge} 0
+   if [ -x "/usr/sbin/brctl" ]; then
+   brctl addbr ${bridge}
+ 

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

2019-12-12 Thread osstest service owner
flight 144751 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144751/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-libvirt   6 libvirt-buildfail REGR. vs. 144517
 build-i386-libvirt6 libvirt-buildfail REGR. vs. 144517
 build-arm64-libvirt   6 libvirt-buildfail REGR. vs. 144517
 build-armhf-libvirt   6 libvirt-buildfail REGR. vs. 144517

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-arm64-arm64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a

version targeted for testing:
 libvirt  4bccb9965d5616be04a45a8b891c45f2320c157b
baseline version:
 libvirt  d0d728c7c00fd3a62731e50c7bc646df323c0622

Last test of basis   144517  2019-12-04 04:18:55 Z8 days
Failing since144526  2019-12-05 04:19:27 Z7 days8 attempts
Testing same since   144751  2019-12-12 04:18:45 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Cole Robinson 
  Daniel Berrange 
  Daniel P. Berrangé 
  Eric Blake 
  Fabiano Fidêncio 
  Han Han 
  Jidong Xia 
  Jiri Denemark 
  Ján Tomko 
  Marc-André Lureau 
  Michal Privoznik 
  Pavel Hrdina 
  Pavel Mores 
  Peter Krempa 

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



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

[Xen-devel] [xen-4.13-testing test] 144736: regressions - FAIL

2019-12-12 Thread osstest service owner
flight 144736 xen-4.13-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144736/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-xl-credit1   7 xen-boot fail REGR. vs. 144673
 test-armhf-armhf-xl-vhd  18 leak-check/check fail REGR. vs. 144673

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10   fail  like 144673
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail  like 144673
 test-amd64-i386-xl-pvshim12 guest-start  fail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass

version targeted for testing:
 xen  ecd3e34ff88b4a8130e7bc6dc18b09682ac3da2b
baseline version:
 xen  b0f0bbca95bd532212fb1956f3e23d1ab13a53cf

Last test of basis   144673  2019-12-10 19:07:50 Z2 days
Failing since144708  2019-12-11 11:38:22 Z1 days2 attempts
Testing same since   144736  2019-12-11 19:06:02 Z1 days1 attempts


People who touched 

Re: [Xen-devel] [PATCH] x86/microcode: Support builtin CPU microcode

2019-12-12 Thread Eslam Elnikety

On 11.12.19 10:54, Jan Beulich wrote:

On 11.12.2019 00:18, Eslam Elnikety wrote:

On 10.12.19 10:37, Jan Beulich wrote:

On 09.12.2019 09:41, Eslam Elnikety wrote:

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2113,7 +2113,7 @@ logic applies:
  active by default.
   
   ### ucode (x86)

-> `= List of [  | scan=, nmi= ]`
+> `= List of [  | scan= | builtin=, nmi= ]`


Despite my other question regarding the usefulness of this as a
whole a few comments.

Do "scan" and "builtin" really need to exclude each other? I.e.
don't you mean , instead of | ?

The useful case here would be specifying ucode=scan,builtin which would
translate to fallback onto the builtin microcode if a scan fails. In
fact, this is already the case given the implementation in v1. It will
be better to clarify this semantic by allowing scan,builtin.

On that note, I really see the "" and "scan=" to be
equal. Following the logic earlier we should probably also allow
ucode=,builtin. This translates to fallback to builtin if there
are no modules at index .


Almost - if the builtin one is newer than the separate blob, then
either of the cmdline options you name should still cause the
builtin one to be loaded. IOW you want to honor both options, not
prefer the earlier over a later one.



On the "newest of everything": That's not what I intend to propose. The 
microcode provided via a scan (or  for that matter) will always 
override the builtin microcode. The common case would be that the 
microcode provided via a scan (or ) is newer than the builtin. 
Yet, an administrator will have the option, if needed, to use an older 
microcode via a scan (or ) to override a newer builtin microcode.



-- Eslam

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

Re: [Xen-devel] [PATCH] x86/microcode: Support builtin CPU microcode

2019-12-12 Thread Eslam Elnikety

On 11.12.19 10:47, Jan Beulich wrote:

On 10.12.2019 23:40, Eslam Elnikety wrote:

On 10.12.19 10:21, Jan Beulich wrote:

On 09.12.2019 22:49, Eslam Elnikety wrote:

On 09.12.19 16:19, Andrew Cooper wrote:

On 09/12/2019 08:41, Eslam Elnikety wrote:

--- /dev/null
+++ b/xen/arch/x86/microcode/Makefile
@@ -0,0 +1,40 @@
+# Copyright (C) 2019 Amazon.com, Inc. or its affiliates.
+# Author: Eslam Elnikety 
+#
+# 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.
+
+obj-y += builtin_ucode.o
+
+# Directory holding the microcode updates.
+UCODE_DIR=$(patsubst "%",%,$(CONFIG_BUILTIN_UCODE_DIR))
+amd-blobs := $(wildcard $(UCODE_DIR)/amd-ucode/*)
+intel-blobs := $(wildcard $(UCODE_DIR)/intel-ucode/*)


This is a little dangerous.  I can see why you want to do it like this,
and I can't provide any obvious suggestions, but if this glob picks up
anything which isn't a microcode file, it will break the logic to search
for the right blob.



We can limit the amd-blobs and intel-blob to binaries following the
naming convention AuthenticAMD.bin and GenuineIntel.bin for amd and
intel, respectively. Yet, this would be imposing an unnecessary
restriction on administrators who may want to be innovative with naming
(or want to use the naming microcode_amd_*.bin or FF-MM-SS instead).

Alternatively, we can introduce CONFIG_BUILTIN_UCODE_INTEL and
CONFIG_BUILTIN_UCODE_AMD. Both default to empty strings. Then, an
administrator can specify exactly the microcodes to include relative to
the CONFIG_BUILTIN_UCODE_DIR. For example:
CONFIG_BUILTIN_UCODE_INTEL="intel-ucode/06-3a-09"
CONFIG_BUILTIN_UCODE_AMD="amd-ucode/microcode_amd_fam15h.bin"


This would make the feature even less generic - I already meant to


I do not follow the point about being less generic. (I hope my example
did not give the false impression that CONFIG_BUILTIN_UCODE_{AMD,INTEL}
allow for only a single microcode blob for a single signature).


Well, the example indeed has given this impression to me. I'm
having a hard time seeing how, beyond very narrow special cases,
either of the examples could be useful to anyone. Yet I think
examples should be generally useful.



Andrew's earlier response hinted at "what can we do to avoid picking 
random bits in the builtin microcode?" My response was outlining 
alternatives, and the examples there were not meant to be useful beyond 
explaining those alternatives.



ask whether building ucode into binaries is really a useful thing
when we already have more flexible ways. I could see this being
useful if there was no other way to make ucode available at boot
time.


It is useful in addition to the existing ways to do early microcode
updates. First, when operating many hosts, using boot modules (either a
distinct microcode module or within an initrd) becomes involved. For
instance, tools to update boot entries (e.g.,
https://linux.die.net/man/8/grubby) do not support adding arbitrary
(microcode) modules.


I.e. you suggest to work around tools shortcomings by extending
Xen? Wouldn't the more appropriate way to deal with this be to
make the tools more capable?


Second, there is often need to couple a Xen build with a minimum
microcode patch level. Having the microcode built within the Xen image
itself is a streamlined, natural way of achieving that.


Okay, I can accept this as a reason, to some degree at least. Yet
as said elsewhere, I don't think you want then to override a
possible "external" ucode module with the builtin blobs. Instead
the newest of everything that's available should then be loaded.


Extending Xen to work around tools shortcomings is absolutely not what I 
have in mind. I should have started with the second reason. Read this 
as: Xen relies on a minimum microcode feature set, and it makes sense to 
couple both in one binary. This coupling just happens to provide an 
added benefit in the face of tools shortcoming.


Thanks,
Eslam

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

[Xen-devel] [PATCH v3 7/7] Added Resolving Disagreement

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

This guide provides Best Practice on identifying and resolving
common classes of disagreement

Changes since v2 (added in v2)
* Fix typos
* Add section: "Issue: Multiple ways to solve a problem"
* Changed line wrapping to 80 characters
* Replaced inline style links with reference style links

Signed-off-by: Lars Kurth 
--
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 resolving-disagreement.md | 188 ++
 1 file changed, 188 insertions(+)
 create mode 100644 resolving-disagreement.md

diff --git a/resolving-disagreement.md b/resolving-disagreement.md
new file mode 100644
index 000..97bca7b
--- /dev/null
+++ b/resolving-disagreement.md
@@ -0,0 +1,188 @@
+# Resolving Disagreement
+
+This guide provides Best Practice on resolving disagreement, such as
+* Gracefully accept constructive criticism
+* Focus on what is best for the community
+* Resolve differences in opinion effectively
+
+## Theory: Paul Graham's hierarchy of disagreement
+
+Paul Graham proposed a **disagreement hierarchy** in a 2008 essay
+**[How to Disagree][1]**, putting types of arguments into a seven-point
+hierarchy and observing that *moving up the disagreement hierarchy makes people
+less mean, and will make most of them happier*. Graham also suggested that the
+hierarchy can be thought of as a pyramid, as the highest forms of disagreement
+are rarer.
+
+| ![Graham's Hierarchy of Disagreement][2]|
+| *A representation of Graham's hierarchy of disagreement from [Loudacris][3]
+  modified by [Rocket000][4]* |
+
+In the context of the Xen Project we strive to **only use the top half** of the
+hierarchy. **Name-calling** and **Ad hominem** arguments are not acceptable
+within the Xen Project.
+
+## Issue: Scope creep
+
+One thing which occasionally happens during code review is that a code reviewer
+asks or appears to ask the author of a patch to implement additional
+functionalities.
+
+This could take for example the form of
+> Do you think it would be useful for the code to do XXX?
+> I can imagine a user wanting to do YYY (and XXX would enable this)
+
+That potentially adds additional work for the code author, which they may not
+have the time to perform. It is good practice for authors to consider such a
+request in terms of
+* Usefulness to the user
+* Code churn, complexity or impact on other system properties
+* Extra time to implement and report back to the reviewer
+
+If you believe that the impact/cost is too high, report back to the reviewer.
+To resolve this, it is advisable to
+* Report your findings
+* And then check whether this was merely an interesting suggestion, or 
something
+  the reviewer feels more strongly about
+
+In the latter case, there are typically several common outcomes
+* The **author and reviewer agree** that the suggestion should be implemented
+* The **author and reviewer agree** that it may make sense to defer
+  implementation
+* The **author and reviewer agree** that it makes no sense to implement the
+  suggestion
+
+The author of a patch would typically suggest their preferred outcome, for
+example
+> I am not sure it is worth to implement XXX
+> Do you think this could be done as a separate patch in future?
+
+In cases, where no agreement can be found, the best approach would be to get an
+independent opinion from another maintainer or the project's leadership team.
+
+## Issue: [Bikeshedding][5]
+
+Occasionally discussions about unimportant but easy-to-grasp issues can lead to
+prolonged and unproductive discussions. The best way to approach this is to
+try and **anticipate** bikeshedding and highlight it as such upfront. However,
+the format of a code review does not always lend itself well to this approach,
+except for highlighting it in the cover letter of a patch series.
+
+However, typically Bikeshedding issues are fairly easy to recognize in a code
+review, as you will very quickly get different reviewers providing differing
+opinions. In this case it is best for the author or a reviewer to call out the
+potential bikeshedding issue using something like
+
+> Looks we have a bikeshedding issue here
+> I think we should call a quick vote to settle the issue
+
+Our governance provides the mechanisms of [informal votes][6] or
+[lazy voting][7] which lend themselves well to resolve such issues.
+
+## Issue: Small functional issues
+
+The most common area of disagreements which happen in code reviews, are
+differing opinions on whether small functional issues in a patch series have to
+be resolved or not before the code is ready to be submitted. Such disagreements
+are typically caused by different expectations related to the level of
+perfection a patch series needs to fulfil before it can be considered ready to
+be 

[Xen-devel] [PATCH v3 6/7] Add guide on Communication Best Practice

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

This guide covers the bulk on Best Practice related to code review
It primarily focusses on code review interactions
It also covers how to deal with Misunderstandings and Cultural
Differences

Changes since v2 (added in v2)
* Fix typos
* Extended "Verbose vs. terse"
* Added "Clarity over Verbosity"
* Broke "Identify the severity of an issue or disagreement" into two chapters
  - "Identify the severity and optionality of review comments" and made
clarifications
  - "Identify the severity of a disagreement"
  - Expanded "Prioritize significant flaws"
* Added "Reviewers: Take account of previous reviewer(s) comments"
* Added prefixes such as "Reviewers:" where appropriate
* Fixed lien wrapping to 80 characters
* Replaced inline links with reference links

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 communication-practice.md | 504 ++
 1 file changed, 504 insertions(+)
 create mode 100644 communication-practice.md

diff --git a/communication-practice.md b/communication-practice.md
new file mode 100644
index 000..0ae2426
--- /dev/null
+++ b/communication-practice.md
@@ -0,0 +1,504 @@
+# Communication Best Practice
+
+This guide provides communication Best Practice that helps you in
+* Using welcoming and inclusive language
+* Keeping discussions technical and actionable
+* Being respectful of differing viewpoints and experiences
+* Being aware of your own and counterpart???s communication style and culture
+* Show empathy towards other community members
+
+## Code reviews for **reviewers** and **patch authors**
+
+Before embarking on a code review, it is important to remember that
+* A poorly executed code review can hurt the contributors feeling, even when a
+  reviewer did not intend to do so. Feeling defensive is a normal reaction to
+  a critique or feedback. A reviewer should be aware of how the pitch, tone,
+  or sentiment of their comments could be interpreted by the contributor. The
+  same applies to responses of an author to the reviewer.
+* When reviewing someone's code, you are ultimately looking for issues. A good
+  code reviewer is able to mentally separate finding issues from articulating
+  code review comments in a constructive and positive manner: depending on your
+  personality this can be **difficult** and you may need to develop a technique
+  that works for you.
+* As software engineers we like to be proud of the solutions we came up with.
+  This can make it easy to take another people???s criticism personally. Always
+  remember that it is the code that is being reviewed, not you as a person.
+* When you receive code review feedback, please be aware that we have reviewers
+  from different backgrounds, communication styles and cultures. Although we
+  all trying to create a productive, welcoming and agile environment, we do
+  not always succeed.
+
+### Express appreciation
+
+As the nature of code review to find bugs and possible issues, it is very easy
+for reviewers to get into a mode of operation where the patch review ends up
+being a list of issues, not mentioning what is right and well done. This can
+lead to the code submitter interpreting your feedback in a negative way.
+
+The opening of a code review provides an opportunity to address this and also
+sets the tone for the rest of the code review. Starting **every** review on a
+positive note, helps set the tone for the rest of the review.
+
+For an initial patch, you can use phrases such as
+> Thanks for the patch
+> Thanks for doing this
+
+For further revisions within a review, phrases such as
+> Thank you for addressing the last set of changes
+
+If you believe the code was good, it is good practice to highlight this by
+using phrases such as
+> Looks good, just a few comments
+> The changes you have made since the last version look good
+
+If you think there were issues too many with the code to use one of the
+phrases, you can still start on a positive note, by for example saying
+> I think this is a good change
+> I think this is a good feature proposal
+
+It is also entirely fine to highlight specific changes as good. The best place
+to do this, is at the top of a patch, as addressing code review comments
+typically requires a contributor to go through the list of things to address
+and an in-lined positive comment is likely to break that workflow.
+
+You should also consider, that if you review a patch of an experienced
+contributor phrases such as *Thanks for the patch* could come across as
+patronizing, while using *Thanks for doing this* is less likely to be
+interpreted as such.
+
+Appreciation should also be expressed by patch authors when asking for
+clarifications to a review or responding to questions. A simple
+> Thank you for your feedback
+> Thank you for your reply
+> 

[Xen-devel] [PATCH v3 1/7] Import v1.4 of Contributor Covenant CoC

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-of-conduct.md | 76 ++
 1 file changed, 76 insertions(+)
 create mode 100644 code-of-conduct.md

diff --git a/code-of-conduct.md b/code-of-conduct.md
new file mode 100644
index 000..81b217c
--- /dev/null
+++ b/code-of-conduct.md
@@ -0,0 +1,76 @@
+# Contributor Covenant Code of Conduct
+
+## Our Pledge
+
+In the interest of fostering an open and welcoming environment, we as
+contributors and maintainers pledge to make participation in our project and
+our community a harassment-free experience for everyone, regardless of age, 
body
+size, disability, ethnicity, sex characteristics, gender identity and 
expression,
+level of experience, education, socio-economic status, nationality, personal
+appearance, race, religion, or sexual identity and orientation.
+
+## Our Standards
+
+Examples of behavior that contributes to creating a positive environment
+include:
+
+* Using welcoming and inclusive language
+* Being respectful of differing viewpoints and experiences
+* Gracefully accepting constructive criticism
+* Focusing on what is best for the community
+* Showing empathy towards other community members
+
+Examples of unacceptable behavior by participants include:
+
+* The use of sexualized language or imagery and unwelcome sexual attention or
+  advances
+* Trolling, insulting/derogatory comments, and personal or political attacks
+* Public or private harassment
+* Publishing others' private information, such as a physical or electronic
+  address, without explicit permission
+* Other conduct which could reasonably be considered inappropriate in a
+  professional setting
+
+## Our Responsibilities
+
+Project maintainers are responsible for clarifying the standards of acceptable
+behavior and are expected to take appropriate and fair corrective action in
+response to any instances of unacceptable behavior.
+
+Project maintainers have the right and responsibility to remove, edit, or
+reject comments, commits, code, wiki edits, issues, and other contributions
+that are not aligned to this Code of Conduct, or to ban temporarily or
+permanently any contributor for other behaviors that they deem inappropriate,
+threatening, offensive, or harmful.
+
+## Scope
+
+This Code of Conduct applies within all project spaces, and it also applies 
when
+an individual is representing the project or its community in public spaces.
+Examples of representing a project or community include using an official
+project e-mail address, posting via an official social media account, or acting
+as an appointed representative at an online or offline event. Representation of
+a project may be further defined and clarified by project maintainers.
+
+## Enforcement
+
+Instances of abusive, harassing, or otherwise unacceptable behavior may be
+reported by contacting the project team at [INSERT EMAIL ADDRESS]. All
+complaints will be reviewed and investigated and will result in a response that
+is deemed necessary and appropriate to the circumstances. The project team is
+obligated to maintain confidentiality with regard to the reporter of an 
incident.
+Further details of specific enforcement policies may be posted separately.
+
+Project maintainers who do not follow or enforce the Code of Conduct in good
+faith may face temporary or permanent repercussions as determined by other
+members of the project's leadership.
+
+## Attribution
+
+This Code of Conduct is adapted from the [Contributor Covenant][homepage], 
version 1.4,
+available at 
https://www.contributor-covenant.org/version/1/4/code-of-conduct.html
+
+[homepage]: https://www.contributor-covenant.org
+
+For answers to common questions about this code of conduct, see
+https://www.contributor-covenant.org/faq
-- 
2.13.0


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

[Xen-devel] [PATCH v3 0/7] Code of Conduct + Extra Guides and Best Practices

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

This series proposes a concrete version of the Xen Project
CoC based on v1.4 of the Contributor Covenant. See [1]

It tries to address all elements in the v2 review, which raised
a number of hard questions. One of the main outstanding items
were good examples for cover letters and well structured large
patch series (see TODO in "Add Code Review Guide")

For convenience of review and in line with other policy documents
I created a git repository at [2]. This series can be found at [3].

I also reformatted the series to 80 characters and replaced
inline syile links with reference style links to make it easier
to stick to a character limit. To just see text but not formatting changes,
have a look at [4]. This is why patch 3 "Reformat Xen Project CoC to fit
into 80 character limit" was added.

[1] https://www.contributor-covenant.org/version/1/4/code-of-conduct.md
[2] http://xenbits.xen.org/gitweb/?p=people/larsk/code-of-conduct.git;a=summary
[3] 
http://xenbits.xen.org/gitweb/?p=people/larsk/code-of-conduct.git;a=shortlog;h=refs/heads/CoC-v3
[4] 
http://xenbits.xen.org/gitweb/?p=people/larsk/code-of-conduct.git;a=commitdiff;h=ee96578446eb41320b3e8a3cada441bb891ad0df;hp=2e4b36afaa73277d246d7e84037db1532a136ec7

Changes since v2
  * Reformatted all text to 80 characters and replaced link style

  code-review-guide.md
  * Extend introduction
  * Add "Code Review Workflow" covering
- "Workflow from a Reviewer's Perspective"
- "Workflow from an Author's Perspective"
- "Problematic Patch Reviews"

  TODO: find suitable examples on how to structure/describe good patch series

  communication-practice.md
  * Fix typos
  * Extended "Verbose vs. terse"
  * Added "Clarity over Verbosity"
  * Broke "Identify the severity of an issue or disagreement" into two chapters
- "Identify the severity and optionality of review comments" and made
  clarifications
- "Identify the severity of a disagreement"
- Expanded "Prioritize significant flaws"
  * Added "Reviewers: Take account of previous reviewer(s) comments"
  * Added prefixes such as "Reviewers:" where appropriate

  resolving-disagreement.md
  * Fix typos
  * Add section: "Issue: Multiple ways to solve a problem"

Changes since v1
* Code of Conduct
  Only whitespace changes

* Added Communication Guide
  Contains values and a process based on advice and mediation in case of issues
  This is the primary portal for

* Added Code Review Guide
  Which is based on [4] with some additions for completeness
  It primarily sets expectations and anything communication related is removed

* Added guide on Communication Best Practice
  Takes the communication section from [4] and expands on it with more examples
  and cases. This is probably where we may need some discussion

* Added document on Resolving Disagreement
  A tiny bit of theory to set the scene
  It covers some common cases of disagreements and how we may approach them
  Again, this probably needs some discussion

Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org

Lars Kurth (7):
  Import v1.4 of Contributor Covenant CoC
  Xen Project Code of Conduct
  Reformat Xen Project CoC to fit into 80 character limit
  Add Communication Guide
  Add Code Review Guide
  Add guide on Communication Best Practice
  Added Resolving Disagreement

--
2.13.0


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

[Xen-devel] [PATCH v3 2/7] Xen Project Code of Conduct

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

Specific changes to the baseline:
* Replace list of positive behaviors with link to separate process
* Replace maintainers with project leadership
  (except in our pledge where maintainers is more appropriate)
* Add 'of all sub-projects' to clarify scope of CoC
* Rename Enforcement
* Replace "project team" with "Conduct Team members"
* Add e-mail alias
* Add section on contacting individual Conduct Team members
* Add section on Conduct Team members

Signed-off-by: Lars Kurth 
---
Chagges since v1:
* Addressed newline changes

Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-of-conduct.md | 45 -
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/code-of-conduct.md b/code-of-conduct.md
index 81b217c..5d6d1d5 100644
--- a/code-of-conduct.md
+++ b/code-of-conduct.md
@@ -1,4 +1,4 @@
-# Contributor Covenant Code of Conduct
+# Xen Project Code of Conduct
 
 ## Our Pledge
 
@@ -11,14 +11,10 @@ appearance, race, religion, or sexual identity and 
orientation.
 
 ## Our Standards
 
-Examples of behavior that contributes to creating a positive environment
-include:
-
-* Using welcoming and inclusive language
-* Being respectful of differing viewpoints and experiences
-* Gracefully accepting constructive criticism
-* Focusing on what is best for the community
-* Showing empathy towards other community members
+We believe that a Code of Conduct can help create a harassment-free 
environment,
+but is not sufficient to create a welcoming environment on its own: guidance on
+creating a welcoming environment, how to communicate in an effective and 
friendly
+way, etc. can be found [here](communication-guide.md).
 
 Examples of unacceptable behavior by participants include:
 
@@ -33,11 +29,11 @@ Examples of unacceptable behavior by participants include:
 
 ## Our Responsibilities
 
-Project maintainers are responsible for clarifying the standards of acceptable
+Project leadership team members are responsible for clarifying the standards 
of acceptable
 behavior and are expected to take appropriate and fair corrective action in
 response to any instances of unacceptable behavior.
 
-Project maintainers have the right and responsibility to remove, edit, or
+Project leadership team members have the right and responsibility to remove, 
edit, or
 reject comments, commits, code, wiki edits, issues, and other contributions
 that are not aligned to this Code of Conduct, or to ban temporarily or
 permanently any contributor for other behaviors that they deem inappropriate,
@@ -45,26 +41,41 @@ threatening, offensive, or harmful.
 
 ## Scope
 
-This Code of Conduct applies within all project spaces, and it also applies 
when
+This Code of Conduct applies within all project spaces of all sub-projects, 
and it also applies when
 an individual is representing the project or its community in public spaces.
 Examples of representing a project or community include using an official
 project e-mail address, posting via an official social media account, or acting
 as an appointed representative at an online or offline event. Representation of
-a project may be further defined and clarified by project maintainers.
+a project may be further defined and clarified by the project leadership.
 
-## Enforcement
+## What to do if you witness or are subject to unacceptable behavior
 
 Instances of abusive, harassing, or otherwise unacceptable behavior may be
-reported by contacting the project team at [INSERT EMAIL ADDRESS]. All
+reported by contacting Conduct Team members at cond...@xenproject.org. All
 complaints will be reviewed and investigated and will result in a response that
-is deemed necessary and appropriate to the circumstances. The project team is
+is deemed necessary and appropriate to the circumstances. Conduct Team members 
are
 obligated to maintain confidentiality with regard to the reporter of an 
incident.
 Further details of specific enforcement policies may be posted separately.
 
-Project maintainers who do not follow or enforce the Code of Conduct in good
+If you have concerns about any of the members of the conduct@ alias,
+you are welcome to contact precisely the Conduct Team member(s) of
+your choice.
+
+Project leadership team members who do not follow or enforce the Code of 
Conduct in good
 faith may face temporary or permanent repercussions as determined by other
 members of the project's leadership.
 
+## Conduct Team members
+Conduct Team members are project leadership team members from any
+sub-project. The current list of Conduct Team members is:
+* Lars Kurth 
+* George Dunlap 
+* Ian Jackson 
+
+Conduct Team members are changed by proposing a change to this document,
+posted on all sub-project lists, followed by a formal global vote as outlined
+[here]: 

[Xen-devel] [PATCH v3 5/7] Add Code Review Guide

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

This document highlights what reviewers such as maintainers and committers look
for when reviewing code. It sets expectations for code authors and provides
a framework for code reviewers.

Changes since v2 (introduced in v2)
* Extend introduction
* Add "Code Review Workflow" covering
  - "Workflow from a Reviewer's Perspective"
  - "Workflow from an Author's Perspective"
  - "Problematic Patch Reviews"
* Wrap to 80 characters
* Replace inline links with reference links to make
  wrapping easier

TODO: find suitable examples on how to structure/describe good patch series

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-review-guide.md | 309 +++
 1 file changed, 309 insertions(+)
 create mode 100644 code-review-guide.md

diff --git a/code-review-guide.md b/code-review-guide.md
new file mode 100644
index 000..5ffbbac
--- /dev/null
+++ b/code-review-guide.md
@@ -0,0 +1,309 @@
+# Code Review Guide
+
+This document highlights what reviewers such as maintainers and committers look
+for when reviewing your code. It sets expectations for code authors and 
provides
+a framework for code reviewers.
+
+Before we start, it is important to remember that the primary purpose of a
+a code review is to indentify any bugs or potential bugs in the code. Most code
+reviews are relatively straight-forward and do not require re-writing the
+submitted code substantially.
+
+The document provides advice on how to structure larger patch series and
+provides  pointers on code author's and reviewer's workflows.
+
+Sometimes it happens that a submitted patch series made wrong assumptions or 
has
+a flawed design or architecture. This can be frustrating for contributors and
+code  reviewers. Note that this document does contain [a section](#problems)
+that provides  suggestions on how to minimize the impact for most stake-holders
+and also on how to avoid such situations.
+
+This document does **not cover** the following topics:
+* [Communication Best Practice][1]
+* [Resolving Disagreement][2]
+* [Patch Submission Workflow][3]
+* [Managing Patch Submission with Git][4]
+
+## What we look for in Code Reviews
+
+When performing a code review, reviewers typically look for the following 
things
+
+### Is the change necessary to accomplish the goals?
+
+* Is it clear what the goals are?
+* Do we need to make a change, or can the goals be met with existing
+  functionality?
+
+### Architecture / Interface
+
+* Is this the best way to solve the problem?
+* Is this the right part of the code to modify?
+* Is this the right level of abstraction?
+* Is the interface general enough? Too general? Forward compatible?
+
+### Functionality
+
+* Does it do what it???s trying to do?
+* Is it doing it in the most ef???cient way?
+* Does it handle all the corner / error cases correctly?
+
+### Maintainability / Robustness
+
+* Is the code clear? Appropriately commented?
+* Does it duplicate another piece of code?
+* Does the code make hidden assumptions?
+* Does it introduce sections which need to be kept **in sync** with other
+  sections?
+* Are there other **traps** someone modifying this code might fall into?
+
+**Note:** Sometimes you will work in areas which have identified 
maintainability
+and/or robustness issues. In such cases, maintainers may ask you to make
+additional changes, such that your submitted code does not make things worse or
+point you to other patches are already being worked on.
+
+### System properties
+
+In some areas of the code, system properties such as
+* Code size
+* Performance
+* Scalability
+* Latency
+* Complexity
+* 
+are also important during code reviews.
+
+### Style
+
+* Comments, carriage returns, **snuggly braces**, 
+* See [CODING_STYLE][5] and [tools/libxl/CODING_STYLE][6]
+* No extraneous whitespace changes
+
+### Documentation and testing
+
+* If there is pre-existing documentation in the tree, such as man pages, design
+  documents, etc. a contributor may be asked to update the documentation
+  alongside the change. Documentation is typically present in the [docs][7]
+  folder.
+* When adding new features that have an impact on the end-user,
+  a contributor should include an update to the [SUPPORT.md][8] file.
+  Typically, more complex features require several patch series before it is
+  ready to be advertised in SUPPORT.md
+* When adding new features, a contributor may be asked to provide tests or
+  ensure that existing tests pass
+
+ Testing for the Xen Project Hypervisor
+
+Tests are typically located in one of the following directories
+* **Unit tests**: [tools/tests][9] or [xen/test][A]
+  Unit testing is hard for a system like Xen and typically requires building a
+  subsystem of your tree. If your change can be easily unit tested, you should
+  

[Xen-devel] [PATCH v3 4/7] Add Communication Guide

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

This document is a portal page that lays out our gold standard,
best practices for some common situations and mechanisms to help
resolve issues that can have a negative effect on our community.

Detail is covered in subsequent documents

Changes since v2 (introduced in v2)
- Make lines break at 80 characters

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 communication-guide.md | 67 ++
 1 file changed, 67 insertions(+)
 create mode 100644 communication-guide.md

diff --git a/communication-guide.md b/communication-guide.md
new file mode 100644
index 000..3c412d9
--- /dev/null
+++ b/communication-guide.md
@@ -0,0 +1,67 @@
+# Communication Guide
+
+We believe that our [Code of Conduct](code-of-conduct.md) can help create a
+harassment-free environment, but is not sufficient to create a welcoming
+environment on its own. We can all make mistakes: when we do, we take
+responsibility for them and try to improve.
+
+This document lays out our gold standard, best practices for some common
+situations and mechanisms to help resolve issues that can have a
+negative effect on our community.
+
+## Goal
+
+We want a productive, welcoming and agile community that can welcome new
+ideas in a complex technical field which is able to reflect on and improve how
+we work.
+
+## Communication & Handling Differences in Opinions
+
+Examples of behavior that contributes to creating a positive environment
+include:
+* Use welcoming and inclusive language
+* Keep discussions technical and actionable
+* Be respectful of differing viewpoints and experiences
+* Be aware of your own and counterpart???s communication style and culture
+* Gracefully accept constructive criticism
+* Focus on what is best for the community
+* Show empathy towards other community members
+* Resolve differences in opinion effectively
+
+## Getting Help
+
+When developing code collaboratively, technical discussion and disagreements
+are unavoidable. Our contributors come from different countries and cultures,
+are driven by different goals and take pride in their work and in their point
+of view. This invariably can lead to lengthy and unproductive debate,
+followed by indecision, sometimes this can impact working relationships
+or lead to other issues that can have a negative effect on our community.
+
+To minimize such issue, we provide a 3-stage process
+* Self-help as outlined in this document
+* Ability to ask for an independent opinion or help in private
+* Mediation between parties which disagree. In this case a neutral community
+  member assists the disputing parties resolve the issues or will work with the
+  parties such that they can improve future interactions.
+
+If you need and independent opinion or help, feel free to contact
+mediat...@xenproject.org. The team behind mediation@ is made up of the
+same community members as those listed in the Conduct Team: see
+[Code of Conduct](code-of-conduct.md). In addition, team members are obligated
+to maintain confidentiality with regard discussions that take place. If you
+have concerns about any of the members of the mediation@ alias, you are
+welcome to contact precisely the team member(s) of your choice. In this case,
+please make certain that you highlight the nature of a request by making sure
+that either help or mediation is mentioned in the e-mail subject or body.
+
+## Specific Topics and Best Practice
+
+* [Code Review Guide](code-review-guide.md):
+  Essential reading for code reviewers and contributors
+* [Communication Best Practice](communication-practice.md):
+  This guide covers communication guidelines for code reviewers and reviewees.
+  It should help you create self-awareness, anticipate, avoid  and help resolve
+  communication issues.
+* [Resolving Disagreement](resolving-disagreement.md):
+  This guide lays out common situations that can lead to dead-lock and shows
+  common patterns on how to avoid and resolve issues.
-- 
2.13.0


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

[Xen-devel] [PATCH v3 3/7] Reformat Xen Project CoC to fit into 80 character limit

2019-12-12 Thread Lars Kurth
From: Lars Kurth 

No content changes

Signed-off-by: Lars Kurth 
---
Cc: minios-de...@lists.xenproject.org
Cc: xen-...@lists.xenproject.org
Cc: win-pv-de...@lists.xenproject.org
Cc: mirageos-de...@lists.xenproject.org
Cc: committ...@xenproject.org
---
 code-of-conduct.md | 56 --
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/code-of-conduct.md b/code-of-conduct.md
index 5d6d1d5..4912f47 100644
--- a/code-of-conduct.md
+++ b/code-of-conduct.md
@@ -5,16 +5,16 @@
 In the interest of fostering an open and welcoming environment, we as
 contributors and maintainers pledge to make participation in our project and
 our community a harassment-free experience for everyone, regardless of age, 
body
-size, disability, ethnicity, sex characteristics, gender identity and 
expression,
-level of experience, education, socio-economic status, nationality, personal
-appearance, race, religion, or sexual identity and orientation.
+size, disability, ethnicity, sex characteristics, gender identity and
+expression, level of experience, education, socio-economic status, nationality,
+personal appearance, race, religion, or sexual identity and orientation.
 
 ## Our Standards
 
 We believe that a Code of Conduct can help create a harassment-free 
environment,
 but is not sufficient to create a welcoming environment on its own: guidance on
-creating a welcoming environment, how to communicate in an effective and 
friendly
-way, etc. can be found [here](communication-guide.md).
+creating a welcoming environment, how to communicate in an effective and
+friendly way, etc. can be found [here](communication-guide.md).
 
 Examples of unacceptable behavior by participants include:
 
@@ -29,41 +29,43 @@ Examples of unacceptable behavior by participants include:
 
 ## Our Responsibilities
 
-Project leadership team members are responsible for clarifying the standards 
of acceptable
-behavior and are expected to take appropriate and fair corrective action in
-response to any instances of unacceptable behavior.
+Project leadership team members are responsible for clarifying the standards of
+acceptable behavior and are expected to take appropriate and fair corrective
+action in response to any instances of unacceptable behavior.
 
-Project leadership team members have the right and responsibility to remove, 
edit, or
-reject comments, commits, code, wiki edits, issues, and other contributions
-that are not aligned to this Code of Conduct, or to ban temporarily or
-permanently any contributor for other behaviors that they deem inappropriate,
-threatening, offensive, or harmful.
+Project leadership team members have the right and responsibility to remove,
+edit, or reject comments, commits, code, wiki edits, issues, and other
+contributions that are not aligned to this Code of Conduct, or to ban
+temporarily or permanently any contributor for other behaviors that they deem
+inappropriate, threatening, offensive, or harmful.
 
 ## Scope
 
-This Code of Conduct applies within all project spaces of all sub-projects, 
and it also applies when
-an individual is representing the project or its community in public spaces.
-Examples of representing a project or community include using an official
-project e-mail address, posting via an official social media account, or acting
-as an appointed representative at an online or offline event. Representation of
-a project may be further defined and clarified by the project leadership.
+This Code of Conduct applies within all project spaces of all sub-projects,
+and it also applies when an individual is representing the project or its
+community in public spaces. Examples of representing a project or community
+include using an official project e-mail address, posting via an official 
social
+media account, or acting as an appointed representative at an online or offline
+event. Representation of a project may be further defined and clarified by the
+project leadership.
 
 ## What to do if you witness or are subject to unacceptable behavior
 
 Instances of abusive, harassing, or otherwise unacceptable behavior may be
 reported by contacting Conduct Team members at cond...@xenproject.org. All
 complaints will be reviewed and investigated and will result in a response that
-is deemed necessary and appropriate to the circumstances. Conduct Team members 
are
-obligated to maintain confidentiality with regard to the reporter of an 
incident.
-Further details of specific enforcement policies may be posted separately.
+is deemed necessary and appropriate to the circumstances. Conduct Team members
+are obligated to maintain confidentiality with regard to the reporter of an
+incident. Further details of specific enforcement policies may be posted
+separately.
 
 If you have concerns about any of the members of the conduct@ alias,
 you are welcome to contact precisely the Conduct Team member(s) of
 your choice.
 
-Project leadership team members 

[Xen-devel] [xtf test] 144743: all pass - PUSHED

2019-12-12 Thread osstest service owner
flight 144743 xtf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144743/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 xtf  27c415ad6e4a48eb3aac6e9d0adc6d0ef04d40cf
baseline version:
 xtf  08a19af3c78e8a03f83bc354b50545136c03edd2

Last test of basis   143721  2019-11-04 13:24:42 Z   38 days
Failing since144355  2019-11-28 23:38:54 Z   13 days4 attempts
Testing same since   144743  2019-12-11 22:10:07 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 

jobs:
 build-amd64-xtf  pass
 build-amd64  pass
 build-amd64-pvopspass
 test-xtf-amd64-amd64-1   pass
 test-xtf-amd64-amd64-2   pass
 test-xtf-amd64-amd64-3   pass
 test-xtf-amd64-amd64-4   pass
 test-xtf-amd64-amd64-5   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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xtf.git
   08a19af..27c415a  27c415ad6e4a48eb3aac6e9d0adc6d0ef04d40cf -> 
xen-tested-master

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

Re: [Xen-devel] [PATCH 4/4] x86/mm: More discriptive names for page de/validation functions

2019-12-12 Thread Andrew Cooper
On 12/12/2019 17:32, George Dunlap wrote:
> The functions alloc_page_type(), alloc_lN_table(), free_page_type()
> and free_lN_table() are confusingly named:

There is alloc_segdesc_page() which should be adjusted for consistency.

> nothing is being allocated or freed.

Well - strictly speaking the type reference is being obtained/dropped,
and this is a kind of alloc/free, although I agree that the names are
not great.

However, I'm not entirely sure that {de,}validate are great either,
because it isn't obviously tied to obtaining/dropping a type reference.

That said, I don't have a better suggestion right now.

~Andrew

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

[Xen-devel] [xen-4.10-testing test] 144728: regressions - trouble: fail/pass/starved

2019-12-12 Thread osstest service owner
flight 144728 xen-4.10-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144728/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 13 guest-saverestore fail REGR. 
vs. 144324
 test-armhf-armhf-xl-vhd  10 debian-di-installfail REGR. vs. 144324

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 144324
 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass
 test-amd64-amd64-qemuu-nested-amd 22 leak-check/check  fail never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop  fail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass
 test-arm64-arm64-xl-thunderx  2 hosts-allocate   starved  n/a

version targeted for testing:
 xen  6cb1cb9c63e91b71ce639e7b7cf08ca85d44266f
baseline version:
 xen  e4899550ff7834e1ea5dfbbfb1c618f64e247761

Last test of basis   144324  2019-11-27 12:48:37 Z   15 days
Testing same since   144728  2019-12-11 15:10:47 Z1 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  George Dunlap 
  Jan Beulich 
  Julien Grall 
  Kevin Tian 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm 

Re: [Xen-devel] [PATCH 3/4] x86/mm: Use a more descriptive name for pagetable mfns

2019-12-12 Thread Andrew Cooper
On 12/12/2019 17:32, George Dunlap wrote:
> In many places, a PTE being modified is accompanied by the pagetable
> mfn which contains the PTE (primarily in order to be able to maintain
> linear mapping counts).  In many cases, this mfn is stored in the
> non-descript variable (or argement) "pfn".
>
> Replace these names with lNmfn, to indicate that 1) this is a
> pagetable mfn, and 2) that it is the same level as the PTE in
> question.  This should be enough to remind readers that it's the mfn
> containing the PTE.
>
> No functional change.
>
> Signed-off-by: George Dunlap 

Acked-by: Andrew Cooper 

Any chance mfn_t can find its way in?

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

Re: [Xen-devel] [PATCH 2/4] x86/mm: Implement common put_data_pages for put_page_from_l[23]e

2019-12-12 Thread Andrew Cooper
On 12/12/2019 17:32, George Dunlap wrote:
> Both put_page_from_l2e and put_page_from_l3e handle having superpage
> entries by looping over each page and "put"-ing each one individually.
> As with putting page table entries, this code is functionally
> identical, but for some reason different.  Moreover, there is already
> a common function, put_data_page(), to handle automatically swapping
> between put_page() (for read-only pages) or put_page_and_type() (for
> read-write pages).
>
> Replace this with put_data_pages() (plural), which does the entire
> loop, as well as the put_page / put_page_and_type switch.
>
> Signed-off-by: George Dunlap 
> ---
> NB that I've used the "simple for loop" version to make it easy to see
> what's going on, rather than the "do { } while()" version which uses &
> and compare to zero rather than comparing to the max.

So while I think the change is an improvement, and are fine as
presented, I'm -1 towards it overall.

I am going to once again express my firm opinion that the remaining use
of PV superpages do far more harm than good, and should be removed
completely.

We construct dom0 with some superpages for its p2m and/or initrd.

This turned out to be the issue behind pv-l1tf breaking for dom0 (c/s
96f6ee15ad7c), and why we had to ship XSA-273 in an insecure
configuration (and I'd like to point out that Xen is still in an
insecure configuration by default.)

There is a still-outstanding issue with grant map by linear address over
a superpage where things malfunction, and the only reason this doesn't
have an XSA is that it is believed to be restricted to dom0 only.

Finally, an L3_SHIFT loop is a long running operation which we can't put
a continuation into the middle of, and I bet there are fun things which
can be done there in the general case.

Seeing as PV guests decompress and free the initrd, and repositions the
p2m, none of these superpages tend to survive post boot, so I am
currently unconvinced that a perf improvement would be anything but
marginal.

I certainly don't think it is worth the complexity and corner cases it
causes is Xen.

~Andrew

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

Re: [Xen-devel] [PATCH 1/4] x86/mm: Refactor put_page_from_l*e to reduce code duplication

2019-12-12 Thread Andrew Cooper
On 12/12/2019 17:32, George Dunlap wrote:
> put_page_from_l[234]e have identical functionality for devalidating an
> N-1 entry which is a pagetable.  But mystifyingly, they duplicate the
> code in slightly different arrangements that make it hard to tell that
> it's the same.

ITYM "this code has grown rather more organically than most" :)

> Create a new function, put_pt_page(), which handles the common
> functionality; and refactor all the functions to be symmetric,
> differing only in the level of pagetable expected (and in whether they
> handle superpages).
>
> No functional change intended.
>
> Signed-off-by: George Dunlap 

Reviewed-by: Andrew Cooper 

The only thing I can see is that the l2 case gains an assertion, but
this looks to be correct.

~Andrew

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

Re: [Xen-devel] [PATCH 4/4] x86/mm: More discriptive names for page de/validation functions

2019-12-12 Thread Andrew Cooper
On 12/12/2019 17:32, George Dunlap wrote:
> @@ -3016,7 +3016,7 @@ static int _get_page_type(struct page_info *page, 
> unsigned long type,
>  page->partial_flags = 0;
>  }
>  page->linear_pt_count = 0;
> -rc = alloc_page_type(page, type, preemptible);
> +rc = validate_page_type(page, type, preemptible);
>  }
>  
>   out:

FYI this has a conflict vs XSA-309, and needs rebasing onto staging. 
Luckily, this one isn't hard to fix up.

~Andrew

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

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

2019-12-12 Thread David Miller
From: Paul Durrant 
Date: Thu, 12 Dec 2019 13:54:06 +

> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
> 
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>   method. The only other driver to have a method at all is
>   pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>   Hence this patch also facilitates further cleanup.
> 
> Signed-off-by: Paul Durrant 

If userspace ever used this stuff, I seriously doubt you can remove this
even if it hasn't been used in 5+ years.

Sorry.

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

Re: [Xen-devel] [XEN PATCH 8/8] xen: Move GCC_HAS_VISIBILITY_ATTRIBUTE to Kconfig and common

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index ff6c0f5cdd18..8c846261d241 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -78,7 +78,7 @@
>  #define __must_be_array(a) \
>BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof([0])))
>  
> -#ifdef GCC_HAS_VISIBILITY_ATTRIBUTE
> +#ifdef CONFIG_CC_HAS_VISIBILITY_ATTRIBUTE
>  /* Results in more efficient PIC code (no indirections through GOT or PLT). 
> */
>  #pragma GCC visibility push(hidden)

(I realise we are getting into archaeology, but) Why do we have this as
a pragma gcc?

Surely it would be simpler to just feed -fvisibility=hidden into CFLAGS?

Ack for the overall change, but I expect we can clean this up a bit more
as well.

~Andrew

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

Re: [Xen-devel] [XEN PATCH 7/8] xen: Use $(CONFIG_CC_IS_CLANG) instead of $(clang) in Makefile

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> Kconfig can check if $(CC) is clang or not, if it is
> CONFIG_CC_IS_CLANG will be set.
>
> With that patch, the hypervisor can be built using clang by running
> `make CC=clang CXX=clang++` without needed to provide an extra clang
> parameter.
>
> `make clang=y` still works as Config.mk will set CC and CXX.
>
> Signed-off-by: Anthony PERARD 

Acked-by: Andrew Cooper 

This is a massive improvement on the status quo.

Do we perhaps want to tweak the relevant CI scripts and other build
instructions now that clang=y is obsolete?

~Andrew

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

Re: [Xen-devel] [PATCH net] xen-netback: avoid race that can lead to NULL pointer dereference

2019-12-12 Thread David Miller
From: Paul Durrant 
Date: Thu, 12 Dec 2019 12:37:23 +

> Commit 2ac061ce97f4 ("xen/netback: cleanup init and deinit code")
> introduced a problem. In function xenvif_disconnect_queue(), the value of
> queue->rx_irq is zeroed *before* queue->task is stopped. Unfortunately that
> task may call notify_remote_via_irq(queue->rx_irq) and calling that
> function with a zero value results in a NULL pointer dereference in
> evtchn_from_irq().
> 
> This patch simply re-orders things, stopping all tasks before zero-ing the
> irq values, thereby avoiding the possibility of the race.
> 
> Signed-off-by: Paul Durrant 

Please repost this with an appropriate Fixes: tag.

And then you can removed the explicit commit reference from the log message
and simply say "The commit mentioned in the Fixes tag introduced a problen ..."

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

Re: [Xen-devel] [XEN PATCH 6/8] xen: Move CONFIG_INDIRECT_THUNK to Kconfig

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> Now that Kconfig has the capability to run shell command when
> generating CONFIG_* we can use it in some cases to test CFLAGS.
>
> CONFIG_INDIRECT_THUNK is a good example that wants to exist both in
> Makefile and as a C macro, which Kconfig do. So use Kconfig to
> generate CONFIG_INDIRECT_THUNK and have the CFLAGS depends on that.
>
> Signed-off-by: Anthony PERARD 

Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [XEN PATCH 5/8] xen: Import cc-ifversion from Kbuild

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> This is in preparation of importing Kbuild to build Xen. We won't be
> able to include Config.mk so we will need a replacement for the macro
> `cc-ifversion'.
>
> This patch imports parts of "scripts/Kbuild.include" from Linux v5.4,
> the macro cc-ifversion. It makes use of CONFIG_GCC_VERSION that
> Kconfig now provides.
>
> Since they are no other use of Xen's `cc-ifversion' macro, we can
> remove it.
>
> Signed-off-by: Anthony PERARD 

Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [XEN PATCH 4/8] xen: Have Kconfig check $(CC)'s version

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> diff --git a/xen/Makefile b/xen/Makefile
> index efbe9605e52b..0cf4ded9d9d4 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -267,6 +267,7 @@ $(foreach base,arch/x86/mm/guest_walk_% \
> arch/x86/mm/shadow/guest_%, \
>  $(foreach ext,o i s,$(call build-intermediate,$(base).$(ext
>  
> +export CC LD

This probably wants to be higher up the file, where we export all the
other variables.  Perhaps also include CXX.

Otherwise, Acked-by: Andrew Cooper 

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

Re: [Xen-devel] [XEN PATCH 3/8] xen: Update Kconfig to Linux v5.4

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> This patch updates Kconfig to a more recent version of Kconfig, found
> in Linux v5.4.0, 219d54332a09 ("Linux 5.4").
>
> With the updated version of Kconfig, other changes are necessary to
> avoid breaking the build.
>
> Kconfig files:
> - fix Kconfig files that where using option env=*:
>   Since Linux commit 104daea149c4 ("kconfig: reference environment
>   variables directly and remove 'option env='"), we can access the
>   environment directly via $() and "option env=" as been removed.
> - CONFIG_EXPERT='y' will now appear in .config file if
>   XEN_CONFIG_EXPERT=y in the environment. The alternative is to change
>   "EXPERT" to "$(XEN_CONFIG_EXPERT)" in all Kconfig files.
>
> Makefile:
> - silentoldconfig target as been removed from Kconfig. To update
>   include/generated/autoconf.h, we need to use syncconfig target
>   instead.
>
> Makefile.kconfig:
> - Import newer needed code from Linux's Makefile.lib and
>   Kbuild.include and Makefile.build.
> - Set Q to empty, Xen build system doesn't silence commands. Having Q
>   empty mean we can import stuff from Linux without having to remove the
>   leading $(Q) from build commands. And quiet='' means commands will be
>   echoed.
> - Add $(PHONY) to .PHONY. Like it is intended by Kbuild.
>
> Makefile.host is also updated and copied from Linux.
>
> Dependency change:
> - Now depends on flex/bison, maybe we could _shipped those files like
>   before. Linux doesn't do that anymore.

Content like that should not be checked in to being with, and I think it
is entirely reasonable to require flex/bison in a build environment.

Indeed, README lists them as mandatory requirements.

> Signed-off-by: Anthony PERARD 

Acked-by: Andrew Cooper 

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

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

2019-12-12 Thread osstest service owner
flight 144737 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144737/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64   6 xen-buildfail REGR. vs. 144637
 build-i386-xsm6 xen-buildfail REGR. vs. 144637
 build-amd64-xsm   6 xen-buildfail REGR. vs. 144637
 build-i3866 xen-buildfail REGR. vs. 144637

Tests which did not succeed, but are not blocking:
 build-i386-libvirt1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 build-amd64-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a

version targeted for testing:
 ovmf 2fe25a74d6fee3c2ac0b930f7f3596cb432e766e
baseline version:
 ovmf 804666c86e7b6f04fe5c5cfdb13199c19e0e99b0

Last test of basis   144637  2019-12-09 09:09:49 Z3 days
Failing since144646  2019-12-10 01:39:53 Z2 days   11 attempts
Testing same since   144713  2019-12-11 12:09:19 Z1 days3 attempts


People who touched revisions under test:
  Antoine Coeur 
  Ard Biesheuvel 
  Bob Feng 
  Jiewen Yao 
  Michael Kubacki 
  Philippe Mathieu-Daude 
  Steven Shi 

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



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 2fe25a74d6fee3c2ac0b930f7f3596cb432e766e
Author: Ard Biesheuvel 
Date:   Tue Mar 5 14:32:48 2019 +0100

ArmPkg/MmCommunicationDxe: relay architected PI events to MM context

PI defines a few architected events that have significance in the MM
context as well as in the non-secure DXE context. So register notify
handlers for these events, and relay them into the standalone MM world.

Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jiewen Yao 
Reviewed-by: Achin Gupta 

commit d3add11e87dace180387562d6f1951f2bffbd3d9
Author: Michael Kubacki 
Date:   Wed Nov 20 17:31:24 2019 -0800

MdeModulePkg PeiCore: Improve comment semantics

This patch clarifies wording in several PeiCore comments to improve
reading comprehension.

Cc: Dandan Bi 
Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Hao A Wu 
Signed-off-by: Michael Kubacki 
Reviewed-by: Liming Gao 
Reviewed-by: Jian J Wang 

commit d39d1260c615b716675f67f5c4e1f4f52df01dad
Author: Michael Kubacki 
Date:   Wed Nov 20 17:10:48 2019 -0800

MdeModulePkg PeiCore: Fix typos

Cc: Dandan Bi 
Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Hao A Wu 
Signed-off-by: Michael Kubacki 
Reviewed-by: Liming Gao 
Reviewed-by: Philippe Mathieu-Daude 
Reviewed-by: Jian J Wang 

commit 97eedf5dfbaffde33210fd88066247cf0b7d3325
Author: Antoine Coeur 
Date:   Wed Dec 4 12:14:53 2019 +0800

IntelFsp2WrapperPkg: Fix various typos

Fix various typos in comments and documentation.

Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Star Zeng 
Reviewed-by: Philippe Mathieu-Daude 
Signed-off-by: Philippe Mathieu-Daude 
Reviewed-by: Nate DeSimone 
Reviewed-by: Chasel Chiu 
Reviewed-by: Star Zeng 

commit 7e55cf6b48dcd43de46d008b2f12caaad2554503
Author: Jiewen Yao 
Date:   Sat Dec 7 21:41:10 2019 +0800

SecurityPkg/Tcg2Smm: Measure the table before patch.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1940

According to TCG PFP specification: the ACPI table must be
measured prior to any modification, and the measurement
must 

Re: [Xen-devel] [XEN PATCH 2/8] Config.mk: Remove stray comment

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> This comment isn't about CONFIG_TESTS, but about SEABIOS_DIR that has
> been removed.
>
> Originally, the comment was added by 5f82d0858de1 ("tools: support
> SeaBIOS. Use by default when upstream qemu is configured."), then
> later the SEABIOS_DIR was removed by 14ee3c05f3ef ("Clone and build
> Seabios by default") but that comment about the pain was left behind.
> The commit that made CONFIG_TESTS painful was 85896a7c4dc7 ("build:
> add autoconf to replace custom checks in tools/check").
>
> Let the comment rest, event if CONFIG_TESTS=y can be painful. It's
> enabled by default anyway and doesn't cause pain for most
> configuration.

Do you mean "rest, even if"?  Even if so, I'm afraid I still can't parse
this paragraph.

The change itself looks fine and reasonable.

~Andrew

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

Re: [Xen-devel] [PATCH] xen/balloon: fix ballooned page accounting without hotplug enabled

2019-12-12 Thread Boris Ostrovsky



On 12/12/19 9:17 AM, Juergen Gross wrote:

When CONFIG_XEN_BALLOON_MEMORY_HOTPLUG is not defined
reserve_additional_memory() will set balloon_stats.target_pages to a
wrong value in case there are still some ballooned pages allocated via
alloc_xenballooned_pages().

This will result in balloon_process() no longer be triggered when
ballooned pages are freed in batches.

Reported-by: Nicholas Tsirakis 
Signed-off-by: Juergen Gross 


Reviewed-by: Boris Ostrovsky 



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

Re: [Xen-devel] [XEN PATCH 1/8] Config.mk: Remove unused setvar_dir macro

2019-12-12 Thread Andrew Cooper
On 12/12/2019 18:27, Anthony PERARD wrote:
> And remove all mention of it in docs. It hasn't been used since
> 9ead9afcb935 ("Add configure --with-sysconfig-leaf-dir=SUBDIR to set
> CONFIG_LEAF_DIR").
>
> Signed-off-by: Anthony PERARD 

Acked-by: Andrew Cooper 

> ---
>  Config.mk| 11 ---
>  docs/misc/distro_mapping.txt |  5 ++---
>  2 files changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/Config.mk b/Config.mk
> index 54e4b7091bfc..8768398d5ece 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -68,17 +68,6 @@ DEPS_RM = $(DEPS) $(DEPS_INCLUDE)
>  include $(XEN_ROOT)/config/$(XEN_OS).mk
>  include $(XEN_ROOT)/config/$(XEN_TARGET_ARCH).mk
>  
> -# arguments: variable, common path part, path to test, if yes, if no
> -define setvar_dir
> -  ifndef $(1)
> -ifneq (,$(wildcard $(2)$(3)))
> -  $(1) ?= $(2)$(4)
> -else
> -  $(1) ?= $(2)$(5)
> -endif
> -  endif
> -endef
> -
>  ifneq ($(EXTRA_PREFIX),)
>  EXTRA_INCLUDES += $(EXTRA_PREFIX)/include
>  EXTRA_LIB += $(EXTRA_PREFIX)/lib
> diff --git a/docs/misc/distro_mapping.txt b/docs/misc/distro_mapping.txt
> index 2e46592728e3..599b6fd1e912 100644
> --- a/docs/misc/distro_mapping.txt
> +++ b/docs/misc/distro_mapping.txt

It looks like this is entirely obsolete since we switched to using
./configure.

Mind if we expand the patch to kill this file fully?  (Can be sorted on
commit if you want.)

~Andrew

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

[Xen-devel] [XEN PATCH 0/8] xen: Kconfig update with few extra

2019-12-12 Thread Anthony PERARD
Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git 
br.build-system-xen-kconfig-v1

Hi,

This is a update of Kconfig as used to build the hypervisor. This is also in
preparation of using Kbuild. The first version of the series, with a POC of
using Kbuild to build xen can be found here:
https://lists.xenproject.org/archives/html/xen-devel/2019-10/msg01609.html

There's also some cleanup patches.

It has been suggested to me that Kconfig's new capability could be used to
generate CFLAGS. I'm not sure how, but I've already added some patches where
having CONFIG_* was already used.

New features of Kconfig:
- Can run shell commands!
This could be use to generate the CFLAGS and cleanup the Kconfig files
to not have to depends on some env var (like XEN_HAS_BUILD_ID).
- Update to the graphical menuconfig, xconfig. It's now built with Qt4/Qt5.
- Probably others that I forgot.

This whole series should be "no functionality changes", I think.

Cheers,

Anthony PERARD (8):
  Config.mk: Remove unused setvar_dir macro
  Config.mk: Remove stray comment
  xen: Update Kconfig to Linux v5.4
  xen: Have Kconfig check $(CC)'s version
  xen: Import cc-ifversion from Kbuild
  xen: Move CONFIG_INDIRECT_THUNK to Kconfig
  xen: Use $(CONFIG_CC_IS_CLANG) instead of $(clang) in Makefile
  xen: Move GCC_HAS_VISIBILITY_ATTRIBUTE to Kconfig and common

 Config.mk |   19 -
 docs/misc/distro_mapping.txt  |5 +-
 docs/misc/kconfig-language.rst|  701 +
 docs/misc/kconfig-language.txt|  395 ---
 docs/misc/kconfig-macro-language.rst  |  247 ++
 docs/misc/{kconfig.txt => kconfig.rst}|  185 +-
 xen/.gitignore|2 +
 xen/Kconfig   |   35 +-
 xen/Makefile  |5 +-
 xen/Rules.mk  |9 +-
 xen/arch/arm/Kconfig  |2 +-
 xen/arch/arm/Rules.mk |4 -
 xen/arch/x86/Kconfig  |3 +
 xen/arch/x86/Rules.mk |   11 +-
 xen/common/Kconfig|   12 +-
 xen/common/coverage/Makefile  |   10 +-
 xen/include/Makefile  |2 +-
 xen/include/xen/compiler.h|2 +-
 xen/scripts/Kbuild.include|7 +
 xen/scripts/Kconfig.include   |   39 +
 xen/scripts/clang-version.sh  |   19 +
 xen/scripts/gcc-version.sh|   20 +
 xen/tools/kconfig/.gitignore  |6 +-
 xen/tools/kconfig/Makefile|  268 +-
 xen/tools/kconfig/Makefile.host   |  121 +-
 xen/tools/kconfig/Makefile.kconfig|   52 +-
 xen/tools/kconfig/conf.c  |  191 +-
 xen/tools/kconfig/confdata.c  |  491 ++--
 xen/tools/kconfig/expr.c  |  213 +-
 xen/tools/kconfig/expr.h  |  108 +-
 xen/tools/kconfig/gconf-cfg.sh|   30 +
 xen/tools/kconfig/gconf.c |   39 +-
 xen/tools/kconfig/images.c|   34 +-
 xen/tools/kconfig/images.h|   33 +
 xen/tools/kconfig/lexer.l |  471 +++
 xen/tools/kconfig/list.h  |1 +
 xen/tools/kconfig/lkc.h   |   38 +-
 xen/tools/kconfig/lkc_proto.h |   21 +-
 xen/tools/kconfig/lxdialog/.gitignore |4 -
 xen/tools/kconfig/lxdialog/BIG.FAT.WARNING|2 +-
 xen/tools/kconfig/lxdialog/check-lxdialog.sh  |   91 -
 xen/tools/kconfig/lxdialog/checklist.c|   15 +-
 xen/tools/kconfig/lxdialog/dialog.h   |   17 +-
 xen/tools/kconfig/lxdialog/inputbox.c |   18 +-
 xen/tools/kconfig/lxdialog/menubox.c  |   15 +-
 xen/tools/kconfig/lxdialog/textbox.c  |   15 +-
 xen/tools/kconfig/lxdialog/util.c |   15 +-
 xen/tools/kconfig/lxdialog/yesno.c|   15 +-
 xen/tools/kconfig/mconf-cfg.sh|   47 +
 xen/tools/kconfig/mconf.c |   27 +-
 xen/tools/kconfig/menu.c  |  288 +-
 xen/tools/kconfig/merge_config.sh |   87 +-
 xen/tools/kconfig/nconf-cfg.sh|   47 +
 xen/tools/kconfig/nconf.c |   42 +-
 xen/tools/kconfig/nconf.gui.c |   30 +-
 xen/tools/kconfig/nconf.h |9 +-
 xen/tools/kconfig/{zconf.y => parser.y}   |  409 ++-
 xen/tools/kconfig/preprocess.c|  574 
 xen/tools/kconfig/qconf-cfg.sh|   32 +
 xen/tools/kconfig/qconf.cc|  750 +++--
 xen/tools/kconfig/qconf.h |  153 +-
 xen/tools/kconfig/streamline_config.pl|   53 +-
 xen/tools/kconfig/symbol.c  

[Xen-devel] [XEN PATCH 6/8] xen: Move CONFIG_INDIRECT_THUNK to Kconfig

2019-12-12 Thread Anthony PERARD
Now that Kconfig has the capability to run shell command when
generating CONFIG_* we can use it in some cases to test CFLAGS.

CONFIG_INDIRECT_THUNK is a good example that wants to exist both in
Makefile and as a C macro, which Kconfig do. So use Kconfig to
generate CONFIG_INDIRECT_THUNK and have the CFLAGS depends on that.

Signed-off-by: Anthony PERARD 
---
 xen/arch/x86/Kconfig  | 3 +++
 xen/arch/x86/Rules.mk | 4 +---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 02bb05f42ef1..ac0fbe3e1aa1 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -32,6 +32,9 @@ config ARCH_DEFCONFIG
string
default "arch/x86/configs/x86_64_defconfig"
 
+config INDIRECT_THUNK
+   def_bool $(cc-option,-mindirect-branch-register)
+
 menu "Architecture Features"
 
 source "arch/Kconfig"
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index 92fdbe9d6822..a2c257fb95b2 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -71,11 +71,9 @@ CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
 endif
 
 # Compile with thunk-extern, indirect-branch-register if avaiable.
-ifneq ($(call cc-option,$(CC),-mindirect-branch-register,n),n)
+ifeq ($(CONFIG_INDIRECT_THUNK),y)
 CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
-CFLAGS += -DCONFIG_INDIRECT_THUNK
 CFLAGS += -fno-jump-tables
-export CONFIG_INDIRECT_THUNK=y
 endif
 
 # If supported by the compiler, reduce stack alignment to 8 bytes. But allow
-- 
Anthony PERARD


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

[Xen-devel] [XEN PATCH 2/8] Config.mk: Remove stray comment

2019-12-12 Thread Anthony PERARD
This comment isn't about CONFIG_TESTS, but about SEABIOS_DIR that has
been removed.

Originally, the comment was added by 5f82d0858de1 ("tools: support
SeaBIOS. Use by default when upstream qemu is configured."), then
later the SEABIOS_DIR was removed by 14ee3c05f3ef ("Clone and build
Seabios by default") but that comment about the pain was left behind.
The commit that made CONFIG_TESTS painful was 85896a7c4dc7 ("build:
add autoconf to replace custom checks in tools/check").

Let the comment rest, event if CONFIG_TESTS=y can be painful. It's
enabled by default anyway and doesn't cause pain for most
configuration.

Signed-off-by: Anthony PERARD 
---
 Config.mk | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/Config.mk b/Config.mk
index 8768398d5ece..35d66e5e121a 100644
--- a/Config.mk
+++ b/Config.mk
@@ -289,7 +289,4 @@ QEMU_TRADITIONAL_LOC ?= $(call or,$(wildcard 
$(QEMU_TRADITIONAL_INTREE)),\
 QEMU_UPSTREAM_LOC ?= $(call or,$(wildcard $(QEMU_UPSTREAM_INTREE)),\
$(QEMU_UPSTREAM_URL))
 
-# Short answer -- do not enable this unless you know what you are
-# doing and are prepared for some pain.
-
 CONFIG_TESTS   ?= y
-- 
Anthony PERARD


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

[Xen-devel] [XEN PATCH 4/8] xen: Have Kconfig check $(CC)'s version

2019-12-12 Thread Anthony PERARD
This import several files from Linux v5.3
 - scripts/Kconfig.include
 - scripts/clang-version.sh
 - scripts/gcc-version.sh
 and several config values from from Linux's init/Kconfig file.

Files are copied into scripts/ directory because that's were the files
are found in Linux tree, and also because we are going to import more
of Kbuild from Linux which is located in scripts/.

CONFIG_GCC_VERSION and CONFIG_CC_IS_CLANG are going to be use in
follow-up patches.

Signed-off-by: Anthony PERARD 
---
 xen/Kconfig  | 17 
 xen/Makefile |  1 +
 xen/scripts/Kconfig.include  | 39 
 xen/scripts/clang-version.sh | 19 ++
 xen/scripts/gcc-version.sh   | 20 ++
 5 files changed, 96 insertions(+)
 create mode 100644 xen/scripts/Kconfig.include
 create mode 100755 xen/scripts/clang-version.sh
 create mode 100755 xen/scripts/gcc-version.sh

diff --git a/xen/Kconfig b/xen/Kconfig
index 01067326b4e7..9f6512d65b08 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -4,9 +4,26 @@
 #
 mainmenu "Xen/$(SRCARCH) $(XEN_FULLVERSION) Configuration"
 
+source "scripts/Kconfig.include"
+
 config BROKEN
bool
 
+config CC_IS_GCC
+   def_bool $(success,$(CC) --version | head -n 1 | grep -q gcc)
+
+config GCC_VERSION
+   int
+   default $(shell,$(BASEDIR)/scripts/gcc-version.sh $(CC)) if CC_IS_GCC
+   default 0
+
+config CC_IS_CLANG
+   def_bool $(success,$(CC) --version | head -n 1 | grep -q clang)
+
+config CLANG_VERSION
+   int
+   default $(shell,$(BASEDIR)/scripts/clang-version.sh $(CC))
+
 source "arch/$(SRCARCH)/Kconfig"
 
 config DEFCONFIG_LIST
diff --git a/xen/Makefile b/xen/Makefile
index efbe9605e52b..0cf4ded9d9d4 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -267,6 +267,7 @@ $(foreach base,arch/x86/mm/guest_walk_% \
arch/x86/mm/shadow/guest_%, \
 $(foreach ext,o i s,$(call build-intermediate,$(base).$(ext
 
+export CC LD
 kconfig := oldconfig config menuconfig defconfig \
nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig \
randconfig $(notdir $(wildcard arch/$(SRCARCH)/configs/*_defconfig))
diff --git a/xen/scripts/Kconfig.include b/xen/scripts/Kconfig.include
new file mode 100644
index ..8221095ca34b
--- /dev/null
+++ b/xen/scripts/Kconfig.include
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Kconfig helper macros
+
+# Convenient variables
+comma   := ,
+quote   := "
+squote  := '
+empty   :=
+space   := $(empty) $(empty)
+dollar  := $
+right_paren := )
+left_paren  := (
+
+# $(if-success,,,)
+# Return  if  exits with 0,  otherwise.
+if-success = $(shell,{ $(1); } >/dev/null 2>&1 && echo "$(2)" || echo "$(3)")
+
+# $(success,)
+# Return y if  exits with 0, n otherwise
+success = $(if-success,$(1),y,n)
+
+# $(failure,)
+# Return n if  exits with 0, y otherwise
+failure = $(if-success,$(1),n,y)
+
+# $(cc-option,)
+# Return y if the compiler supports , n otherwise
+cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -E -x c /dev/null -o 
/dev/null)
+
+# $(ld-option,)
+# Return y if the linker supports , n otherwise
+ld-option = $(success,$(LD) -v $(1))
+
+# check if $(CC) and $(LD) exist
+$(error-if,$(failure,command -v $(CC)),compiler '$(CC)' not found)
+$(error-if,$(failure,command -v $(LD)),linker '$(LD)' not found)
+
+# gcc version including patch level
+gcc-version := $(shell,$(BASEDIR)/scripts/gcc-version.sh $(CC))
diff --git a/xen/scripts/clang-version.sh b/xen/scripts/clang-version.sh
new file mode 100755
index ..6fabf0695761
--- /dev/null
+++ b/xen/scripts/clang-version.sh
@@ -0,0 +1,19 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# clang-version clang-command
+#
+# Print the compiler version of `clang-command' in a 5 or 6-digit form
+# such as `50001' for clang-5.0.1 etc.
+
+compiler="$*"
+
+if ! ( $compiler --version | grep -q clang) ; then
+   echo 0
+   exit 1
+fi
+
+MAJOR=$(echo __clang_major__ | $compiler -E -x c - | tail -n 1)
+MINOR=$(echo __clang_minor__ | $compiler -E -x c - | tail -n 1)
+PATCHLEVEL=$(echo __clang_patchlevel__ | $compiler -E -x c - | tail -n 1)
+printf "%d%02d%02d\\n" $MAJOR $MINOR $PATCHLEVEL
diff --git a/xen/scripts/gcc-version.sh b/xen/scripts/gcc-version.sh
new file mode 100755
index ..ae353432539b
--- /dev/null
+++ b/xen/scripts/gcc-version.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# gcc-version gcc-command
+#
+# Print the gcc version of `gcc-command' in a 5 or 6-digit form
+# such as `29503' for gcc-2.95.3, `30301' for gcc-3.3.1, etc.
+
+compiler="$*"
+
+if [ ${#compiler} -eq 0 ]; then
+   echo "Error: No compiler specified." >&2
+   printf "Usage:\n\t$0 \n" >&2
+   exit 1
+fi
+
+MAJOR=$(echo __GNUC__ | $compiler -E -x c - | tail -n 1)
+MINOR=$(echo __GNUC_MINOR__ | $compiler -E -x c - | tail -n 1)
+PATCHLEVEL=$(echo __GNUC_PATCHLEVEL__ | $compiler -E -x c 

[Xen-devel] [XEN PATCH 7/8] xen: Use $(CONFIG_CC_IS_CLANG) instead of $(clang) in Makefile

2019-12-12 Thread Anthony PERARD
Kconfig can check if $(CC) is clang or not, if it is
CONFIG_CC_IS_CLANG will be set.

With that patch, the hypervisor can be built using clang by running
`make CC=clang CXX=clang++` without needed to provide an extra clang
parameter.

`make clang=y` still works as Config.mk will set CC and CXX.

Signed-off-by: Anthony PERARD 
---
 xen/Rules.mk | 8 
 xen/arch/x86/Rules.mk| 2 +-
 xen/common/coverage/Makefile | 2 +-
 xen/include/Makefile | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index d053dbd26526..fcdafd029342 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -64,7 +64,7 @@ CFLAGS += -pipe -D__XEN__ -include 
$(BASEDIR)/include/xen/config.h
 CFLAGS-$(CONFIG_DEBUG_INFO) += -g
 CFLAGS += '-D__OBJECT_FILE__="$@"'
 
-ifneq ($(clang),y)
+ifneq ($(CONFIG_CC_IS_CLANG),y)
 # Clang doesn't understand this command line argument, and doesn't appear to
 # have an suitable alternative.  The resulting compiled binary does function,
 # but has an excessively large symbol table.
@@ -126,7 +126,7 @@ subdir-all := $(subdir-y) $(subdir-n)
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += 
-DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_COVERAGE),y)
-ifeq ($(clang),y)
+ifeq ($(CONFIG_CC_IS_CLANG),y)
 COV_FLAGS := -fprofile-instr-generate -fcoverage-mapping
 else
 COV_FLAGS := -fprofile-arcs -ftest-coverage
@@ -143,7 +143,7 @@ endif
 
 ifeq ($(CONFIG_LTO),y)
 CFLAGS += -flto
-LDFLAGS-$(clang) += -plugin LLVMgold.so
+LDFLAGS-$(CONFIG_CC_IS_CLANG) += -plugin LLVMgold.so
 # Would like to handle all object files as bitcode, but objects made from
 # pure asm are in a different format and have to be collected separately.
 # Mirror the directory tree, collecting them as built_in_bin.o.
@@ -197,7 +197,7 @@ SRCPATH := $(patsubst $(BASEDIR)/%,%,$(CURDIR))
 %.o: %.c Makefile
 ifeq ($(CONFIG_ENFORCE_UNIQUE_SYMBOLS),y)
$(CC) $(CFLAGS) -c $< -o $(@D)/.$(@F).tmp
-ifeq ($(clang),y)
+ifeq ($(CONFIG_CC_IS_CLANG),y)
$(OBJCOPY) --redefine-sym $<=$(SRCPATH)/$< $(@D)/.$(@F).tmp $@
 else
$(OBJCOPY) --redefine-sym $(https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [XEN PATCH 8/8] xen: Move GCC_HAS_VISIBILITY_ATTRIBUTE to Kconfig and common

2019-12-12 Thread Anthony PERARD
The check for $(CC) -fvisibility=hidden is done by both arm and x86,
so the patch also move the check to the common area.

The check doesn't check if $(CC) is gcc, and clang can accept that
option as well, so s/GCC/CC/ is done to the define name.

Signed-off-by: Anthony PERARD 
---
 xen/Kconfig| 4 
 xen/arch/arm/Rules.mk  | 4 
 xen/arch/x86/Rules.mk  | 5 -
 xen/include/xen/compiler.h | 2 +-
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/xen/Kconfig b/xen/Kconfig
index 9f6512d65b08..fc49f4c30a29 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -24,6 +24,10 @@ config CLANG_VERSION
int
default $(shell,$(BASEDIR)/scripts/clang-version.sh $(CC))
 
+# -fvisibility=hidden reduces -fpic cost, if it's available
+config CC_HAS_VISIBILITY_ATTRIBUTE
+   def_bool $(cc-option,-fvisibility=hidden)
+
 source "arch/$(SRCARCH)/Kconfig"
 
 config DEFCONFIG_LIST
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 3d9a0ed357bc..022a3a6f82ba 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -18,10 +18,6 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
 CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
 CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
 
-ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
-CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
-endif
-
 EARLY_PRINTK := n
 
 ifeq ($(CONFIG_DEBUG),y)
diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk
index b98e14e28c5a..e69b8e697cc0 100644
--- a/xen/arch/x86/Rules.mk
+++ b/xen/arch/x86/Rules.mk
@@ -65,11 +65,6 @@ CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables
 # SSE setup for variadic function calls.
 CFLAGS += -mno-sse $(call cc-option,$(CC),-mskip-rax-setup)
 
-# -fvisibility=hidden reduces -fpic cost, if it's available
-ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
-CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
-endif
-
 # Compile with thunk-extern, indirect-branch-register if avaiable.
 ifeq ($(CONFIG_INDIRECT_THUNK),y)
 CFLAGS += -mindirect-branch=thunk-extern -mindirect-branch-register
diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index ff6c0f5cdd18..8c846261d241 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -78,7 +78,7 @@
 #define __must_be_array(a) \
   BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof([0])))
 
-#ifdef GCC_HAS_VISIBILITY_ATTRIBUTE
+#ifdef CONFIG_CC_HAS_VISIBILITY_ATTRIBUTE
 /* Results in more efficient PIC code (no indirections through GOT or PLT). */
 #pragma GCC visibility push(hidden)
 #endif
-- 
Anthony PERARD


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

[Xen-devel] [XEN PATCH 1/8] Config.mk: Remove unused setvar_dir macro

2019-12-12 Thread Anthony PERARD
And remove all mention of it in docs. It hasn't been used since
9ead9afcb935 ("Add configure --with-sysconfig-leaf-dir=SUBDIR to set
CONFIG_LEAF_DIR").

Signed-off-by: Anthony PERARD 
---
 Config.mk| 11 ---
 docs/misc/distro_mapping.txt |  5 ++---
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/Config.mk b/Config.mk
index 54e4b7091bfc..8768398d5ece 100644
--- a/Config.mk
+++ b/Config.mk
@@ -68,17 +68,6 @@ DEPS_RM = $(DEPS) $(DEPS_INCLUDE)
 include $(XEN_ROOT)/config/$(XEN_OS).mk
 include $(XEN_ROOT)/config/$(XEN_TARGET_ARCH).mk
 
-# arguments: variable, common path part, path to test, if yes, if no
-define setvar_dir
-  ifndef $(1)
-ifneq (,$(wildcard $(2)$(3)))
-  $(1) ?= $(2)$(4)
-else
-  $(1) ?= $(2)$(5)
-endif
-  endif
-endef
-
 ifneq ($(EXTRA_PREFIX),)
 EXTRA_INCLUDES += $(EXTRA_PREFIX)/include
 EXTRA_LIB += $(EXTRA_PREFIX)/lib
diff --git a/docs/misc/distro_mapping.txt b/docs/misc/distro_mapping.txt
index 2e46592728e3..599b6fd1e912 100644
--- a/docs/misc/distro_mapping.txt
+++ b/docs/misc/distro_mapping.txt
@@ -9,9 +9,8 @@ INITD_DIR| /etc/rc.d/init.d | /etc/init.d   | 
/etc/init.d|
 -+--+---++
 
 The existence of these directories are tested at build-time (on the
-build host, via the "setvar_dir" macro in Config.mk) and for some
-scripts at run-time.  If the Red Hat directory exists, it is used;
-otherwise the Debian one is used.
+build host) and for some scripts at run-time.  If the Red Hat
+directory exists, it is used; otherwise the Debian one is used.
 
 The INITD_DIR path can be changed with configure --with-initddir=DIR.
 The CONFIG_LEAF_DIR name can be changed with configure
-- 
Anthony PERARD


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

[Xen-devel] [XEN PATCH 5/8] xen: Import cc-ifversion from Kbuild

2019-12-12 Thread Anthony PERARD
This is in preparation of importing Kbuild to build Xen. We won't be
able to include Config.mk so we will need a replacement for the macro
`cc-ifversion'.

This patch imports parts of "scripts/Kbuild.include" from Linux v5.4,
the macro cc-ifversion. It makes use of CONFIG_GCC_VERSION that
Kconfig now provides.

Since they are no other use of Xen's `cc-ifversion' macro, we can
remove it.

Signed-off-by: Anthony PERARD 
---
 Config.mk| 5 -
 xen/Rules.mk | 1 +
 xen/common/coverage/Makefile | 8 
 xen/scripts/Kbuild.include   | 7 +++
 4 files changed, 12 insertions(+), 9 deletions(-)
 create mode 100644 xen/scripts/Kbuild.include

diff --git a/Config.mk b/Config.mk
index 35d66e5e121a..65649d6122d1 100644
--- a/Config.mk
+++ b/Config.mk
@@ -121,11 +121,6 @@ define cc-ver-check-closure
 endif
 endef
 
-# cc-ifversion: Check compiler version and take branch accordingly
-# Usage $(call cc-ifversion,lt,0x040700,string_if_y,string_if_n)
-cc-ifversion = $(shell [ $(call cc-ver,$(CC),$(1),$(2)) = "y" ] \
-   && echo $(3) || echo $(4))
-
 # Require GCC v4.1+
 check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1")
 $(eval $(check-y))
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 5aba841b0a95..d053dbd26526 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -2,6 +2,7 @@
 -include $(BASEDIR)/include/config/auto.conf
 
 include $(XEN_ROOT)/Config.mk
+include $(BASEDIR)/scripts/Kbuild.include
 
 
 ifneq ($(origin crash_debug),undefined)
diff --git a/xen/common/coverage/Makefile b/xen/common/coverage/Makefile
index 46c78d1086d6..b509e51f960b 100644
--- a/xen/common/coverage/Makefile
+++ b/xen/common/coverage/Makefile
@@ -1,10 +1,10 @@
 obj-y += coverage.o
 ifneq ($(clang),y)
 obj-y += gcov_base.o gcov.o
-obj-y += $(call cc-ifversion,lt,0x040700, \
-   gcc_3_4.o, $(call cc-ifversion,lt,0x040900, \
-   gcc_4_7.o, $(call cc-ifversion,lt,0x05, \
-   gcc_4_9.o, $(call cc-ifversion,lt,0x07, \
+obj-y += $(call cc-ifversion,-lt,0407, \
+   gcc_3_4.o, $(call cc-ifversion,-lt,0409, \
+   gcc_4_7.o, $(call cc-ifversion,-lt,0500, \
+   gcc_4_9.o, $(call cc-ifversion,-lt,0700, \
gcc_5.o, gcc_7.o
 else
 obj-y += llvm.o
diff --git a/xen/scripts/Kbuild.include b/xen/scripts/Kbuild.include
new file mode 100644
index ..a5c462fd9777
--- /dev/null
+++ b/xen/scripts/Kbuild.include
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# kbuild: Generic definitions
+
+# cc-ifversion
+# Usage:  EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
+cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] && echo $(3) || 
echo $(4))
-- 
Anthony PERARD


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

Re: [Xen-devel] [PATCH net] xen-netback: avoid race that can lead to NULL pointer dereference

2019-12-12 Thread Wei Liu
On Thu, Dec 12, 2019 at 12:37:23PM +, Paul Durrant wrote:
> Commit 2ac061ce97f4 ("xen/netback: cleanup init and deinit code")
> introduced a problem. In function xenvif_disconnect_queue(), the value of
> queue->rx_irq is zeroed *before* queue->task is stopped. Unfortunately that
> task may call notify_remote_via_irq(queue->rx_irq) and calling that
> function with a zero value results in a NULL pointer dereference in
> evtchn_from_irq().
> 
> This patch simply re-orders things, stopping all tasks before zero-ing the
> irq values, thereby avoiding the possibility of the race.
> 
> Signed-off-by: Paul Durrant 

Acked-by: Wei Liu 

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

[Xen-devel] [PATCH 4/4] x86/mm: More discriptive names for page de/validation functions

2019-12-12 Thread George Dunlap
The functions alloc_page_type(), alloc_lN_table(), free_page_type()
and free_lN_table() are confusingly named: nothing is being allocated
or freed.  Rather, the page being passed in is being either validated
or devalidated for use as the specific type.

Rename these functions to "validate" and "devalidate" respectively to
be more clear.

No functional change.

Signed-off-by: George Dunlap 
---
CC: Andrew Cooper 
CC: Jan Beulich 
---
 xen/arch/x86/domain.c|  2 +-
 xen/arch/x86/mm.c| 62 
 xen/include/asm-x86/mm.h |  4 +--
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index f5c0c378ef..ee20de6493 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2032,7 +2032,7 @@ static int relinquish_memory(
 if ( likely(y == x) )
 {
 /* No need for atomic update of type_info here: noone else 
updates it. */
-switch ( ret = free_page_type(page, x, 1) )
+switch ( ret = devalidate_page_type(page, x, 1) )
 {
 case 0:
 break;
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 54b4100d55..dc1463123c 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1366,7 +1366,7 @@ static int put_page_from_l4e(l4_pgentry_t l4e, unsigned 
long pfn,
 return put_pt_page(l4e_get_page(l4e), mfn_to_page(_mfn(pfn)), flags);
 }
 
-static int alloc_l1_table(struct page_info *page)
+static int validate_l1_table(struct page_info *page)
 {
 struct domain *d = page_get_owner(page);
 l1_pgentry_t  *pl1e;
@@ -1408,7 +1408,7 @@ static int alloc_l1_table(struct page_info *page)
 
  fail:
 gdprintk(XENLOG_WARNING,
- "Failure %d in alloc_l1_table: slot %#x\n", ret, i);
+ "Failure %d in validate_l1_table: slot %#x\n", ret, i);
  out:
 while ( i-- > 0 )
 put_page_from_l1e(pl1e[i], d);
@@ -1441,7 +1441,7 @@ static int create_pae_xen_mappings(struct domain *d, 
l3_pgentry_t *pl3e)
  *  1. Cannot appear in slots != 3 because get_page_type() checks the
  * PGT_pae_xen_l2 flag, which is asserted iff the L2 appears in slot 3
  *  2. Cannot appear in another page table's L3:
- * a. alloc_l3_table() calls this function and this check will fail
+ * a. validate_l3_table() calls this function and this check will fail
  * b. mod_l3_entry() disallows updates to slot 3 in an existing table
  */
 page = l3e_get_page(l3e3);
@@ -1457,7 +1457,7 @@ static int create_pae_xen_mappings(struct domain *d, 
l3_pgentry_t *pl3e)
 return 1;
 }
 
-static int alloc_l2_table(struct page_info *page, unsigned long type)
+static int validate_l2_table(struct page_info *page, unsigned long type)
 {
 struct domain *d = page_get_owner(page);
 unsigned long  l2mfn = mfn_x(page_to_mfn(page));
@@ -1469,8 +1469,8 @@ static int alloc_l2_table(struct page_info *page, 
unsigned long type)
 pl2e = map_domain_page(_mfn(l2mfn));
 
 /*
- * NB that alloc_l2_table will never set partial_pte on an l2; but
- * free_l2_table might if a linear_pagetable entry is interrupted
+ * NB that validate_l2_table will never set partial_pte on an l2; but
+ * devalidate_l2_table might if a linear_pagetable entry is interrupted
  * partway through de-validation.  In that circumstance,
  * get_page_from_l2e() will always return -EINVAL; and we must
  * retain the type ref by doing the normal partial_flags tracking.
@@ -1497,7 +1497,7 @@ static int alloc_l2_table(struct page_info *page, 
unsigned long type)
 /*
  * It shouldn't be possible for get_page_from_l2e to return
  * -ERESTART, since we never call this with PTF_preemptible.
- * (alloc_l1_table may return -EINTR on an L1TF-vulnerable
+ * (validate_l1_table may return -EINTR on an L1TF-vulnerable
  * entry.)
  *
  * NB that while on a "clean" promotion, we can never get
@@ -1518,12 +1518,12 @@ static int alloc_l2_table(struct page_info *page, 
unsigned long type)
 else if ( rc < 0 && rc != -EINTR )
 {
 gdprintk(XENLOG_WARNING,
- "Failure %d in alloc_l2_table: slot %#x\n", rc, i);
+ "Failure %d in validate_l2_table: slot %#x\n", rc, i);
 ASSERT(current->arch.old_guest_table == NULL);
 if ( i )
 {
 /*
- * alloc_l1_table() doesn't set old_guest_table; it does
+ * validate_l1_table() doesn't set old_guest_table; it does
  * its own tear-down immediately on failure.  If it
  * did we'd need to check it and set partial_flags as we
  * do in alloc_l[34]_table().
@@ -1556,7 +1556,7 @@ static int alloc_l2_table(struct page_info *page, 
unsigned long type)
 return rc;
 }
 
-static int alloc_l3_table(struct 

[Xen-devel] [PATCH 0/4] Post-299 cleanups

2019-12-12 Thread George Dunlap
This series implements a number of cleanups to make the code simpler
and easier to follow.  No functional changes intended.

George Dunlap (4):
  x86/mm: Refactor put_page_from_l*e to reduce code duplication
  x86/mm: Implement common put_data_pages for put_page_from_l[23]e
  x86/mm: Use a more descriptive name for pagetable mfns
  x86/mm: More discriptive names for page de/validation functions

 xen/arch/x86/domain.c|   2 +-
 xen/arch/x86/mm.c| 243 ---
 xen/include/asm-x86/mm.h |   4 +-
 3 files changed, 102 insertions(+), 147 deletions(-)

--
CC: Andrew Cooper 
CC: Jan Beulich 

2.24.0

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

[Xen-devel] [PATCH 2/4] x86/mm: Implement common put_data_pages for put_page_from_l[23]e

2019-12-12 Thread George Dunlap
Both put_page_from_l2e and put_page_from_l3e handle having superpage
entries by looping over each page and "put"-ing each one individually.
As with putting page table entries, this code is functionally
identical, but for some reason different.  Moreover, there is already
a common function, put_data_page(), to handle automatically swapping
between put_page() (for read-only pages) or put_page_and_type() (for
read-write pages).

Replace this with put_data_pages() (plural), which does the entire
loop, as well as the put_page / put_page_and_type switch.

Signed-off-by: George Dunlap 
---
NB that I've used the "simple for loop" version to make it easy to see
what's going on, rather than the "do { } while()" version which uses &
and compare to zero rather than comparing to the max.

CC: Andrew Cooper 
CC: Jan Beulich 
---
 xen/arch/x86/mm.c | 52 ++-
 1 file changed, 20 insertions(+), 32 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index d8a0eb2aa5..c05039ab21 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1289,14 +1289,6 @@ void put_page_from_l1e(l1_pgentry_t l1e, struct domain 
*l1e_owner)
 }
 
 #ifdef CONFIG_PV
-static void put_data_page(struct page_info *page, bool writeable)
-{
-if ( writeable )
-put_page_and_type(page);
-else
-put_page(page);
-}
-
 static int put_pt_page(struct page_info *pg, struct page_info *ptpg,
unsigned int flags)
 {
@@ -1319,6 +1311,20 @@ static int put_pt_page(struct page_info *pg, struct 
page_info *ptpg,
 return rc;
 }
 
+static int put_data_pages(struct page_info *page, bool writeable, int pt_shift)
+{
+int i, count = 1 << (pt_shift - PAGE_SHIFT);
+
+ASSERT(!(mfn_x(page_to_mfn(page)) & (count - 1)));
+for ( i = 0; i < count ; i++, page++ )
+if ( writeable )
+put_page_and_type(page);
+else
+put_page(page);
+
+return 0;
+}
+
 /*
  * NB. Virtual address 'l2e' maps to a machine address within frame 'pfn'.
  * Note also that this automatically deals correctly with linear p.t.'s.
@@ -1330,18 +1336,9 @@ static int put_page_from_l2e(l2_pgentry_t l2e, unsigned 
long pfn,
 return 1;
 
 if ( l2e_get_flags(l2e) & _PAGE_PSE )
-{
-struct page_info *page = l2e_get_page(l2e);
-bool writeable = l2e_get_flags(l2e) & _PAGE_RW;
-unsigned int i;
-
-ASSERT(!(mfn_x(page_to_mfn(page)) &
- ((1UL << (L2_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1)));
-for ( i = 0; i < (1u << PAGETABLE_ORDER); i++, page++ )
-put_data_page(page, writeable);
-
-return 0;
-}
+return put_data_pages(l2e_get_page(l2e),
+  l2e_get_flags(l2e) & _PAGE_RW,
+  L2_PAGETABLE_SHIFT);
 
 return put_pt_page(l2e_get_page(l2e), mfn_to_page(_mfn(pfn)), flags);
 }
@@ -1353,18 +1350,9 @@ static int put_page_from_l3e(l3_pgentry_t l3e, unsigned 
long pfn,
 return 1;
 
 if ( unlikely(l3e_get_flags(l3e) & _PAGE_PSE) )
-{
-unsigned long mfn = l3e_get_pfn(l3e);
-bool writeable = l3e_get_flags(l3e) & _PAGE_RW;
-
-ASSERT(!(flags & PTF_partial_set));
-ASSERT(!(mfn & ((1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1)));
-do {
-put_data_page(mfn_to_page(_mfn(mfn)), writeable);
-} while ( ++mfn & ((1UL << (L3_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1) );
-
-return 0;
-}
+return put_data_pages(l3e_get_page(l3e),
+  l3e_get_flags(l3e) & _PAGE_RW,
+  L3_PAGETABLE_SHIFT);
 
 return put_pt_page(l3e_get_page(l3e), mfn_to_page(_mfn(pfn)), flags);
 }
-- 
2.24.0


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

[Xen-devel] [PATCH 1/4] x86/mm: Refactor put_page_from_l*e to reduce code duplication

2019-12-12 Thread George Dunlap
put_page_from_l[234]e have identical functionality for devalidating an
N-1 entry which is a pagetable.  But mystifyingly, they duplicate the
code in slightly different arrangements that make it hard to tell that
it's the same.

Create a new function, put_pt_page(), which handles the common
functionality; and refactor all the functions to be symmetric,
differing only in the level of pagetable expected (and in whether they
handle superpages).

No functional change intended.

Signed-off-by: George Dunlap 
---
CC: Andrew Cooper 
CC: Jan Beulich 
---
 xen/arch/x86/mm.c | 89 +++
 1 file changed, 28 insertions(+), 61 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7d4dd80a85..d8a0eb2aa5 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1297,6 +1297,28 @@ static void put_data_page(struct page_info *page, bool 
writeable)
 put_page(page);
 }
 
+static int put_pt_page(struct page_info *pg, struct page_info *ptpg,
+   unsigned int flags)
+{
+int rc = 0;
+
+if ( flags & PTF_defer )
+{
+ASSERT(!(flags & PTF_partial_set));
+current->arch.old_guest_ptpg = ptpg;
+current->arch.old_guest_table = pg;
+current->arch.old_guest_table_partial = false;
+}
+else
+{
+rc = _put_page_type(pg, flags | PTF_preemptible, ptpg);
+if ( likely(!rc) )
+put_page(pg);
+}
+
+return rc;
+}
+
 /*
  * NB. Virtual address 'l2e' maps to a machine address within frame 'pfn'.
  * Note also that this automatically deals correctly with linear p.t.'s.
@@ -1304,8 +1326,6 @@ static void put_data_page(struct page_info *page, bool 
writeable)
 static int put_page_from_l2e(l2_pgentry_t l2e, unsigned long pfn,
  unsigned int flags)
 {
-int rc = 0;
-
 if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) || (l2e_get_pfn(l2e) == pfn) )
 return 1;
 
@@ -1319,35 +1339,16 @@ static int put_page_from_l2e(l2_pgentry_t l2e, unsigned 
long pfn,
  ((1UL << (L2_PAGETABLE_SHIFT - PAGE_SHIFT)) - 1)));
 for ( i = 0; i < (1u << PAGETABLE_ORDER); i++, page++ )
 put_data_page(page, writeable);
-}
-else
-{
-struct page_info *pg = l2e_get_page(l2e);
-struct page_info *ptpg = mfn_to_page(_mfn(pfn));
 
-if ( flags & PTF_defer )
-{
-current->arch.old_guest_ptpg = ptpg;
-current->arch.old_guest_table = pg;
-current->arch.old_guest_table_partial = false;
-}
-else
-{
-rc = _put_page_type(pg, flags | PTF_preemptible, ptpg);
-if ( likely(!rc) )
-put_page(pg);
-}
+return 0;
 }
 
-return rc;
+return put_pt_page(l2e_get_page(l2e), mfn_to_page(_mfn(pfn)), flags);
 }
 
 static int put_page_from_l3e(l3_pgentry_t l3e, unsigned long pfn,
  unsigned int flags)
 {
-struct page_info *pg;
-int rc;
-
 if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) || (l3e_get_pfn(l3e) == pfn) )
 return 1;
 
@@ -1365,50 +1366,16 @@ static int put_page_from_l3e(l3_pgentry_t l3e, unsigned 
long pfn,
 return 0;
 }
 
-pg = l3e_get_page(l3e);
-
-if ( flags & PTF_defer )
-{
-ASSERT(!(flags & PTF_partial_set));
-current->arch.old_guest_ptpg = mfn_to_page(_mfn(pfn));
-current->arch.old_guest_table = pg;
-current->arch.old_guest_table_partial = false;
-return 0;
-}
-
-rc = _put_page_type(pg, flags | PTF_preemptible, mfn_to_page(_mfn(pfn)));
-if ( likely(!rc) )
-put_page(pg);
-
-return rc;
+return put_pt_page(l3e_get_page(l3e), mfn_to_page(_mfn(pfn)), flags);
 }
 
 static int put_page_from_l4e(l4_pgentry_t l4e, unsigned long pfn,
  unsigned int flags)
 {
-int rc = 1;
-
-if ( (l4e_get_flags(l4e) & _PAGE_PRESENT) &&
- (l4e_get_pfn(l4e) != pfn) )
-{
-struct page_info *pg = l4e_get_page(l4e);
-
-if ( flags & PTF_defer )
-{
-ASSERT(!(flags & PTF_partial_set));
-current->arch.old_guest_ptpg = mfn_to_page(_mfn(pfn));
-current->arch.old_guest_table = pg;
-current->arch.old_guest_table_partial = false;
-return 0;
-}
-
-rc = _put_page_type(pg, flags | PTF_preemptible,
-mfn_to_page(_mfn(pfn)));
-if ( likely(!rc) )
-put_page(pg);
-}
+if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) || (l4e_get_pfn(l4e) == pfn) )
+return 1;
 
-return rc;
+return put_pt_page(l4e_get_page(l4e), mfn_to_page(_mfn(pfn)), flags);
 }
 
 static int alloc_l1_table(struct page_info *page)
-- 
2.24.0


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

[Xen-devel] [PATCH 3/4] x86/mm: Use a more descriptive name for pagetable mfns

2019-12-12 Thread George Dunlap
In many places, a PTE being modified is accompanied by the pagetable
mfn which contains the PTE (primarily in order to be able to maintain
linear mapping counts).  In many cases, this mfn is stored in the
non-descript variable (or argement) "pfn".

Replace these names with lNmfn, to indicate that 1) this is a
pagetable mfn, and 2) that it is the same level as the PTE in
question.  This should be enough to remind readers that it's the mfn
containing the PTE.

No functional change.

Signed-off-by: George Dunlap 
---
CC: Andrew Cooper 
CC: Jan Beulich 
---
 xen/arch/x86/mm.c | 50 +++
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index c05039ab21..54b4100d55 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -1141,7 +1141,7 @@ static int get_page_and_type_from_mfn(
 define_get_linear_pagetable(l2);
 static int
 get_page_from_l2e(
-l2_pgentry_t l2e, unsigned long pfn, struct domain *d, unsigned int flags)
+l2_pgentry_t l2e, unsigned long l2mfn, struct domain *d, unsigned int 
flags)
 {
 unsigned long mfn = l2e_get_pfn(l2e);
 int rc;
@@ -1156,7 +1156,7 @@ get_page_from_l2e(
 ASSERT(!(flags & PTF_preemptible));
 
 rc = get_page_and_type_from_mfn(_mfn(mfn), PGT_l1_page_table, d, flags);
-if ( unlikely(rc == -EINVAL) && get_l2_linear_pagetable(l2e, pfn, d) )
+if ( unlikely(rc == -EINVAL) && get_l2_linear_pagetable(l2e, l2mfn, d) )
 rc = 0;
 
 return rc;
@@ -1165,7 +1165,7 @@ get_page_from_l2e(
 define_get_linear_pagetable(l3);
 static int
 get_page_from_l3e(
-l3_pgentry_t l3e, unsigned long pfn, struct domain *d, unsigned int flags)
+l3_pgentry_t l3e, unsigned long l3mfn, struct domain *d, unsigned int 
flags)
 {
 int rc;
 
@@ -1180,7 +1180,7 @@ get_page_from_l3e(
 l3e_get_mfn(l3e), PGT_l2_page_table, d, flags | PTF_preemptible);
 if ( unlikely(rc == -EINVAL) &&
  !is_pv_32bit_domain(d) &&
- get_l3_linear_pagetable(l3e, pfn, d) )
+ get_l3_linear_pagetable(l3e, l3mfn, d) )
 rc = 0;
 
 return rc;
@@ -1189,7 +1189,7 @@ get_page_from_l3e(
 define_get_linear_pagetable(l4);
 static int
 get_page_from_l4e(
-l4_pgentry_t l4e, unsigned long pfn, struct domain *d, unsigned int flags)
+l4_pgentry_t l4e, unsigned long l4mfn, struct domain *d, unsigned int 
flags)
 {
 int rc;
 
@@ -1202,7 +1202,7 @@ get_page_from_l4e(
 
 rc = get_page_and_type_from_mfn(
 l4e_get_mfn(l4e), PGT_l3_page_table, d, flags | PTF_preemptible);
-if ( unlikely(rc == -EINVAL) && get_l4_linear_pagetable(l4e, pfn, d) )
+if ( unlikely(rc == -EINVAL) && get_l4_linear_pagetable(l4e, l4mfn, d) )
 rc = 0;
 
 return rc;
@@ -1460,13 +1460,13 @@ static int create_pae_xen_mappings(struct domain *d, 
l3_pgentry_t *pl3e)
 static int alloc_l2_table(struct page_info *page, unsigned long type)
 {
 struct domain *d = page_get_owner(page);
-unsigned long  pfn = mfn_x(page_to_mfn(page));
+unsigned long  l2mfn = mfn_x(page_to_mfn(page));
 l2_pgentry_t  *pl2e;
 unsigned int   i;
 intrc = 0;
 unsigned int   partial_flags = page->partial_flags;
 
-pl2e = map_domain_page(_mfn(pfn));
+pl2e = map_domain_page(_mfn(l2mfn));
 
 /*
  * NB that alloc_l2_table will never set partial_pte on an l2; but
@@ -1492,7 +1492,7 @@ static int alloc_l2_table(struct page_info *page, 
unsigned long type)
 rc = -EINTR;
 }
 else
-rc = get_page_from_l2e(l2e, pfn, d, partial_flags);
+rc = get_page_from_l2e(l2e, l2mfn, d, partial_flags);
 
 /*
  * It shouldn't be possible for get_page_from_l2e to return
@@ -1559,14 +1559,14 @@ static int alloc_l2_table(struct page_info *page, 
unsigned long type)
 static int alloc_l3_table(struct page_info *page)
 {
 struct domain *d = page_get_owner(page);
-unsigned long  pfn = mfn_x(page_to_mfn(page));
+unsigned long  l3mfn = mfn_x(page_to_mfn(page));
 l3_pgentry_t  *pl3e;
 unsigned int   i;
 intrc = 0;
 unsigned int   partial_flags = page->partial_flags;
 l3_pgentry_t   l3e = l3e_empty();
 
-pl3e = map_domain_page(_mfn(pfn));
+pl3e = map_domain_page(_mfn(l3mfn));
 
 /*
  * PAE guests allocate full pages, but aren't required to initialize
@@ -1603,7 +1603,7 @@ static int alloc_l3_table(struct page_info *page)
 rc = -EINTR;
 }
 else
-rc = get_page_from_l3e(l3e, pfn, d,
+rc = get_page_from_l3e(l3e, l3mfn, d,
partial_flags | PTF_retain_ref_on_restart);
 
 if ( rc == -ERESTART )
@@ -1786,8 +1786,8 @@ void zap_ro_mpt(mfn_t mfn)
 static int alloc_l4_table(struct page_info *page)
 {
 struct domain *d = page_get_owner(page);
-unsigned long  pfn = mfn_x(page_to_mfn(page));
-l4_pgentry_t  *pl4e = map_domain_page(_mfn(pfn));
+unsigned 

[Xen-devel] [xen-4.8-testing test] 144761: trouble: pass/preparing/queued

2019-12-12 Thread osstest service owner
flight 144761 xen-4.8-testing running [real]
http://logs.test-lab.xenproject.org/osstest/logs/144761/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qcow2 queued
 test-amd64-amd64-xl   queued
 test-amd64-i386-qemuu-rhel6hvm-intel  queued
 test-amd64-amd64-migrupgrade  queued
 test-amd64-amd64-xl-xsm   queued
 test-armhf-armhf-xl-arndale   queued
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow queued
 test-amd64-amd64-libvirt  queued
 test-armhf-armhf-xl-cubietruck  queued
 test-amd64-amd64-xl-qemut-debianhvm-i386-xsm queued
 test-arm64-arm64-xl-seattle   queued
 test-xtf-amd64-amd64-1queued
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmqueued
 test-arm64-arm64-xl-xsm   queued
 test-amd64-i386-xl-qemuu-ws16-amd64  queued
 test-amd64-i386-freebsd10-amd64  queued
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  queued
 test-arm64-arm64-xl-credit1   queued
 test-amd64-i386-pair  queued
 test-amd64-i386-migrupgrade   queued
 test-amd64-amd64-xl-credit1   queued
 test-amd64-i386-xlqueued
 test-amd64-i386-xl-xsmqueued
 test-amd64-amd64-libvirt-xsm  queued
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  queued
 test-armhf-armhf-xl-vhd   queued
 test-amd64-i386-xl-shadow queued
 test-amd64-i386-xl-qemuu-debianhvm-amd64 queued
 test-amd64-amd64-libvirt-vhd  queued
 test-amd64-amd64-i386-pvgrub  queued
 test-amd64-amd64-xl-multivcpu  queued
 test-amd64-i386-xl-qemut-debianhvm-amd64 queued
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  queued
 test-amd64-i386-qemut-rhel6hvm-amd  queued
 test-amd64-i386-xl-qemut-ws16-amd64  queued
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm queued
 test-amd64-amd64-pair queued
 test-xtf-amd64-amd64-5queued
 test-amd64-amd64-qemuu-nested-amd  queued
 test-amd64-i386-xl-qemut-win7-amd64  queued
 test-amd64-amd64-xl-qemut-ws16-amd64  queued
 test-armhf-armhf-xl   queued
 test-amd64-i386-livepatch queued
 test-amd64-amd64-amd64-pvgrub  queued
 test-amd64-i386-libvirt-pair  queued
 test-xtf-amd64-amd64-2queued
 test-amd64-amd64-xl-shadowqueued
 test-amd64-amd64-xl-qemut-debianhvm-amd64queued
 test-armhf-armhf-libvirt  queued
 test-amd64-amd64-xl-qemuu-win7-amd64  queued
 test-amd64-i386-qemuu-rhel6hvm-amd  queued
 build-arm64-libvirt   queued
 test-xtf-amd64-amd64-4queued
 test-amd64-amd64-xl-qemut-win7-amd64  queued
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   queued
 test-amd64-i386-xl-rawqueued
 build-i386-libvirtqueued
 test-amd64-i386-freebsd10-i386  queued
 test-armhf-armhf-libvirt-raw  queued
 test-amd64-i386-xl-qemuu-win7-amd64  queued
 test-armhf-armhf-xl-credit1   queued
 test-amd64-amd64-xl-qemuu-debianhvm-amd64queued
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmqueued
 test-arm64-arm64-xl   queued
 test-armhf-armhf-xl-multivcpu  queued
 test-amd64-i386-qemut-rhel6hvm-intel  queued
 test-amd64-amd64-qemuu-nested-intel  queued
 test-amd64-amd64-xl-credit2   queued
 test-xtf-amd64-amd64-3queued
 test-amd64-amd64-xl-qemuu-ovmf-amd64  queued
 test-arm64-arm64-libvirt-xsm  queued
 test-amd64-i386-libvirt   queued
 test-amd64-amd64-libvirt-pair  queued
 test-amd64-amd64-pygrub   queued
 test-amd64-amd64-livepatchqueued
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm queued
 test-amd64-i386-xl-qemuu-ovmf-amd64  queued
 test-arm64-arm64-xl-credit2   queued
 test-amd64-i386-libvirt-xsm   queued
 test-amd64-amd64-xl-rtds  queued
 test-amd64-amd64-xl-qemuu-ws16-amd64  queued
 test-arm64-arm64-xl-thunderx  queued
 test-armhf-armhf-xl-rtds  queued
 

Re: [Xen-devel] [xen-4.9-testing test] 144723: regressions - trouble: fail/pass/starved

2019-12-12 Thread Ian Jackson
osstest service owner writes ("[xen-4.9-testing test] 144723: regressions - 
trouble: fail/pass/starved"):
> flight 144723 xen-4.9-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/144723/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-i386-xl-qemut-win7-amd64 15 guest-saverestore.2 fail REGR. vs. 
> 144545
>  test-amd64-amd64-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail REGR. 
> vs. 144545

These tests have been flaky for a long time.  Given the 4.13 release
should get priority I propose to force push this rather than waiting
for the retest to complete.  I will then kill the retest flight, since
stable-4.9 will then be == staging-4.9.

Ian.

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

[Xen-devel] [xen-unstable-smoke test] 144766: tolerable all pass - PUSHED

2019-12-12 Thread osstest service owner
flight 144766 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144766/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  5852ca48526316918cd82fba1033a6a5379fbc4c
baseline version:
 xen  b4f042236ae0bb6725b3e8dd40af5a2466a6f971

Last test of basis   144719  2019-12-11 15:00:35 Z1 days
Testing same since   144766  2019-12-12 15:00:35 Z0 days1 attempts


People who touched revisions under test:
  Juergen Gross 
  Wei Liu 

jobs:
 build-arm64-xsm  pass
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-arm64-arm64-xl-xsm  pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-amd64-libvirt pass



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

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

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

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


Pushing revision :

To xenbits.xen.org:/home/xen/git/xen.git
   b4f042236a..5852ca4852  5852ca48526316918cd82fba1033a6a5379fbc4c -> smoke

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

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

2019-12-12 Thread Durrant, Paul
> -Original Message-
> From: jandr...@gmail.com 
> Sent: 12 December 2019 16:32
> To: Durrant, Paul 
> Cc: xen-devel ; net...@vger.kernel.org;
> open list ; Wei Liu ;
> David S. Miller 
> Subject: Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev
> related code
> 
> On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant  wrote:
> >
> > In the past it used to be the case that the Xen toolstack relied upon
> > udev to execute backend hotplug scripts. However this has not been the
> > case for many releases now and removal of the associated code in
> > xen-netback shortens the source by more than 100 lines, and removes much
> > complexity in the interaction with the xenstore backend state.
> >
> > NOTE: xen-netback is the only xenbus driver to have a functional
> uevent()
> >   method. The only other driver to have a method at all is
> >   pvcalls-back, and currently pvcalls_back_uevent() simply returns
> 0.
> >   Hence this patch also facilitates further cleanup.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> > Cc: Wei Liu 
> > Cc: "David S. Miller" 
> > ---
> >  drivers/net/xen-netback/common.h |  11 ---
> >  drivers/net/xen-netback/xenbus.c | 125 ---
> >  2 files changed, 14 insertions(+), 122 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-
> netback/common.h
> > index 05847eb91a1b..e48da004c1a3 100644
> 
> 
> 
> > -static inline void backend_switch_state(struct backend_info *be,
> > -   enum xenbus_state state)
> > -{
> > -   struct xenbus_device *dev = be->dev;
> > -
> > -   pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> > -   be->state = state;
> > -
> > -   /* If we are waiting for a hotplug script then defer the
> > -* actual xenbus state change.
> > -*/
> > -   if (!be->have_hotplug_status_watch)
> > -   xenbus_switch_state(dev, state);
> 
> have_hotplug_status_watch prevents xen-netback from switching to
> connected state unless the the backend scripts have written
> "hotplug-status" "success".  I had always thought that was intentional
> so the frontend doesn't connect when the backend is unconnected.  i.e.
> if the backend scripts fails, it writes "hotplug-status" "error" and
> the frontend doesn't connect.
> 
> That behavior is independent of using udev to run the scripts.  I'm
> not opposed to removing it, but I think it at least warrants
> mentioning in the commit message.

True, but it's probably related. The netback probe would previously kick udev, 
the hotplug script would then run, and then the state would go connected. I 
think, because the hotplug is invoked directly by the toolstack now, these 
things really ought not to be tied together. TBH I can't see any harm in the 
frontend seeing the network connection before the backend plumbing is done... 
If the frontend should have any sort of indication of whether the backend is 
plumbed or not then IMO it ought to be as a virtual carrier/link status, 
because unplumbing and re-plumbing could be done at any time really without any 
need for the shared ring to go away (and in fact I will be following up at some 
point with a patch to allow unbind and re-bind of netback).

I'll elaborate in the commit message as you suggest :-)

Cheers,

  Paul

> 
> Regards,
> Jason
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

2019-12-12 Thread Jason Andryuk
On Thu, Dec 12, 2019 at 8:56 AM Paul Durrant  wrote:
>
> In the past it used to be the case that the Xen toolstack relied upon
> udev to execute backend hotplug scripts. However this has not been the
> case for many releases now and removal of the associated code in
> xen-netback shortens the source by more than 100 lines, and removes much
> complexity in the interaction with the xenstore backend state.
>
> NOTE: xen-netback is the only xenbus driver to have a functional uevent()
>   method. The only other driver to have a method at all is
>   pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
>   Hence this patch also facilitates further cleanup.
>
> Signed-off-by: Paul Durrant 
> ---
> Cc: Wei Liu 
> Cc: "David S. Miller" 
> ---
>  drivers/net/xen-netback/common.h |  11 ---
>  drivers/net/xen-netback/xenbus.c | 125 ---
>  2 files changed, 14 insertions(+), 122 deletions(-)
>
> diff --git a/drivers/net/xen-netback/common.h 
> b/drivers/net/xen-netback/common.h
> index 05847eb91a1b..e48da004c1a3 100644



> -static inline void backend_switch_state(struct backend_info *be,
> -   enum xenbus_state state)
> -{
> -   struct xenbus_device *dev = be->dev;
> -
> -   pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> -   be->state = state;
> -
> -   /* If we are waiting for a hotplug script then defer the
> -* actual xenbus state change.
> -*/
> -   if (!be->have_hotplug_status_watch)
> -   xenbus_switch_state(dev, state);

have_hotplug_status_watch prevents xen-netback from switching to
connected state unless the the backend scripts have written
"hotplug-status" "success".  I had always thought that was intentional
so the frontend doesn't connect when the backend is unconnected.  i.e.
if the backend scripts fails, it writes "hotplug-status" "error" and
the frontend doesn't connect.

That behavior is independent of using udev to run the scripts.  I'm
not opposed to removing it, but I think it at least warrants
mentioning in the commit message.

Regards,
Jason

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

Re: [Xen-devel] [PATCH v7 2/3] xen/blkback: Squeeze page pools if a memory pressure is detected

2019-12-12 Thread SeongJae Park
On Thu, 12 Dec 2019 16:23:17 +0100 "Roger Pau Monné"  
wrote:

> > On Thu, 12 Dec 2019 12:42:47 +0100 "Roger Pau Monné"  
> > wrote:
> > > > On the slow block device
> > > > 
> > > > 
> > > > max_pgs   Min   Max   Median AvgStddev
> > > > 0 38.7  45.8  38.7   40.12  3.1752165
> > > > 1024  38.7  45.8  38.7   40.12  3.1752165
> > > > No difference proven at 95.0% confidence
> > > > 
> > > > On the fast block device
> > > > 
> > > > 
> > > > max_pgs   Min   Max   Median AvgStddev
> > > > 0 417   423   420419.4  2.5099801
> > > > 1024  414   425   416417.8  4.4384682
> > > > No difference proven at 95.0% confidence
> > > 
> > > This is intriguing, as it seems to prove that the usage of a cache of
> > > free pages is irrelevant performance wise.
> > > 
> > > The pool of free pages was introduced long ago, and it's possible that
> > > recent improvements to the balloon driver had made such pool useless,
> > > at which point it could be removed instead of worked around.
> > 
> > I guess the grant page allocation overhead in this test scenario is really
> > small.  In an absence of memory pressure, fragmentation, and NUMA imbalance,
> > the latency of the page allocation ('get_page()') is very short, as it will
> > success in the fast path.
> 
> The allocation of the pool of free pages involves more than get_page,
> it uses gnttab_alloc_pages which in the worse case will allocate a
> page and balloon it out issuing one hypercall.
> 
> > Few years ago, I once measured the page allocation latency on my machine.
> > Roughly speaking, it was about 1us in best case, 100us in worst case, and 
> > 5us
> > in average.  Please keep in mind that the measurement was not designed and
> > performed in serious way.  Thus the results could have profile overhead in 
> > it,
> > though.  While keeping that in mind, let's simply believe the number and 
> > ignore
> > the latency of the block layer, blkback itself (including the grant
> > mapping), and anything else including context switch, cache miss, but the
> > allocation.  In other words, suppose that the grant page allocation is only 
> > one
> > source of the overhead.  It will be able to achieve 1 million IOPS (4KB *
> > 1MIOPS = 4 GB/s) in the best case, 200 thousand IOPS (800 MB/s) in average, 
> > and
> > 10 thousand IOPS (40 MB/s) in worst case.  Based on this coarse 
> > calculation, I
> > think the test results is reasonable.
> > 
> > This also means that the effect of the blkback's free pages pool might be
> > visible under page allocation fast path failure situation.  Nevertheless, it
> > would be also hard to measure that in micro level unless the measurement is
> > well designed and controlled.
> > 
> > > 
> > > Do you think you could perform some more tests (as pointed out above
> > > against the block device to skip the fs overhead) and report back the
> > > results?
> > 
> > To be honest, I'm not sure whether additional tests are really necessary,
> > because I think the `dd` test and the results explanation already makes some
> > sense and provide the minimal proof of the concept.  Also, this change is a
> > fallback for the memory pressure situation, which is an error path in some
> > point of view.  Such errorneous situation might not happen frequently and if
> > the situation is not solved in short time, something much worse (e.g., OOM 
> > kill
> > of the user space xen control processes) than temporal I/O performance
> > degradation could happen.  Thus, I'm not sure whether such detailed 
> > performance
> > measurement is necessary for this rare error handling change.
> 
> Right, my main concern is that we seem to be adding duck tape so
> things don't fall apart, but if such cache is really not beneficial
> from a performance PoV I would rather see it go away than adding more
> stuff to it in order to workaround corner cases like memory
> starvation.

Right, if the cache is really giving no benefit, it would be much better to
simply remove it.  However, as mentioned before, I'm not sure whether it is
useless at all.  Maybe we could do some more detailed test to know that, but it
would be an out of scope of this patch.

> 
> Anyway, I guess we can take such change, but long term we need to look
> into fixing grants to not use ballooned pages, and figure out if the
> blkback free page cache is really useful or not.

Totally agreed.


Thanks,
SeongJae Park

> 
> Thanks, Roger.
> 

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

Re: [Xen-devel] [xen-4.8-testing test] 144726: regressions - trouble: fail/pass/starved [and 1 more messages]

2019-12-12 Thread Jan Beulich
On 12.12.2019 17:01, Ian Jackson wrote:
> Jan Beulich writes ("Re: [xen-4.8-testing test] 144726: regressions - 
> trouble: fail/pass/starved"):
>> On 12.12.2019 16:40, Ian Jackson wrote:
>>> From Juergen I would like a release-ack for the osstest commit to
>>> "allow" it for the future.
>>
>> Is this really worth it? The 4.8 tree is dead now; strictly speaking
>> even the last batch of XSAs shouldn't have gone there anymore, but
>> we did so to be friendly to certain distros, as it was just barely
>> past the expiry date.
> 
> This is pattern likely to occur again in the future with newer
> unsupported but security-supported branches.
> 
> Having gone to the trouble of figuring out what is going on I thought
> I would write it up and produce a commit in osstest.git that serves as
> an example of how to fix it.

Makes sense.

Jan

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

Re: [Xen-devel] [PATCH v7 2/3] xen/blkback: Squeeze page pools if a memory pressure is detected

2019-12-12 Thread SeongJae Park
On Thu, 12 Dec 2019 16:27:57 +0100 "Roger Pau Monné"  
wrote:

> > diff --git a/drivers/block/xen-blkback/blkback.c 
> > b/drivers/block/xen-blkback/blkback.c
> > index fd1e19f1a49f..98823d150905 100644
> > --- a/drivers/block/xen-blkback/blkback.c
> > +++ b/drivers/block/xen-blkback/blkback.c
> > @@ -142,6 +142,21 @@ static inline bool persistent_gnt_timeout(struct 
> > persistent_gnt *persistent_gnt)
> > HZ * xen_blkif_pgrant_timeout);
> >  }
> >  
> > +/* Once a memory pressure is detected, squeeze free page pools for a 
> > while. */
> > +static unsigned int buffer_squeeze_duration_ms = 10;
> > +module_param_named(buffer_squeeze_duration_ms,
> > +   buffer_squeeze_duration_ms, int, 0644);
> > +MODULE_PARM_DESC(buffer_squeeze_duration_ms,
> > +"Duration in ms to squeeze pages buffer when a memory pressure is 
> > detected");
> > +
> > +static unsigned long buffer_squeeze_end;
> > +
> > +void xen_blkbk_reclaim_memory(struct xenbus_device *dev)
> > +{
> > +   buffer_squeeze_end = jiffies +
> > +   msecs_to_jiffies(buffer_squeeze_duration_ms);
> 
> I'm not sure this is fully correct. This function will be called for
> each blkback instance, but the timeout is stored in a global variable
> that's shared between all blkback instances. Shouldn't this timeout be
> stored in xen_blkif so each instance has it's own local variable?
> 
> Or else in the case you have 1k blkback instances the timeout is
> certainly going to be longer than expected, because each call to
> xen_blkbk_reclaim_memory will move it forward.

Agreed that.  I think the extended timeout would not make a visible
performance, though, because the time that 1k-loop take would be short enough
to be ignored compared to the millisecond-scope duration.

I took this way because I wanted to minimize such structural changes as far as
I can, as this is just a point-fix rather than ultimate solution.  That said,
it is not fully correct and very confusing.  My another colleague also pointed
out it in internal review.  Correct solution would be to adding a variable in
the struct as you suggested or avoiding duplicated update of the variable by
initializing the variable once the squeezing duration passes.  I would prefer
the later way, as it is more straightforward and still not introducing
structural change.  For example, it might be like below:

diff --git a/drivers/block/xen-blkback/blkback.c 
b/drivers/block/xen-blkback/blkback.c
index f41c698dd854..6856c8ef88de 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -152,8 +152,9 @@ static unsigned long buffer_squeeze_end;
 
 void xen_blkbk_reclaim_memory(struct xenbus_device *dev)
 {
-   buffer_squeeze_end = jiffies +
-   msecs_to_jiffies(buffer_squeeze_duration_ms);
+   if (!buffer_squeeze_end)
+   buffer_squeeze_end = jiffies +
+   msecs_to_jiffies(buffer_squeeze_duration_ms);
 }
 
 static inline int get_free_page(struct xen_blkif_ring *ring, struct page 
**page)
@@ -669,10 +670,13 @@ int xen_blkif_schedule(void *arg)
}
 
/* Shrink the free pages pool if it is too large. */
-   if (time_before(jiffies, buffer_squeeze_end))
+   if (time_before(jiffies, buffer_squeeze_end)) {
shrink_free_pagepool(ring, 0);
-   else
+   } else {
+   if (unlikely(buffer_squeeze_end))
+   buffer_squeeze_end = 0;
shrink_free_pagepool(ring, max_buffer_pages);
+   }
 
if (log_stats && time_after(jiffies, ring->st_print))
print_stats(ring);

May I ask you what way would you prefer?


Thanks,
SeongJae Park

> 
> Thanks, Roger.
> 

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

Re: [Xen-devel] [xen-4.8-testing test] 144726: regressions - trouble: fail/pass/starved [and 1 more messages]

2019-12-12 Thread Ian Jackson
Jan Beulich writes ("Re: [xen-4.8-testing test] 144726: regressions - trouble: 
fail/pass/starved"):
> On 12.12.2019 16:40, Ian Jackson wrote:
> > If Jan approves will force push 4.8-testing.
> 
> I do. Thanks.

Done.

> > From Juergen I would like a release-ack for the osstest commit to
> > "allow" it for the future.
> 
> Is this really worth it? The 4.8 tree is dead now; strictly speaking
> even the last batch of XSAs shouldn't have gone there anymore, but
> we did so to be friendly to certain distros, as it was just barely
> past the expiry date.

This is pattern likely to occur again in the future with newer
unsupported but security-supported branches.

Having gone to the trouble of figuring out what is going on I thought
I would write it up and produce a commit in osstest.git that serves as
an example of how to fix it.

Jürgen Groß writes ("Re: [xen-4.8-testing test] 144726: regressions - trouble: 
fail/pass/starved"):
> At least I really don't mind, as I can't see any risk for 4.13.
> 
> Release-acked-by: Juergen Gross 

Thanks.

Ian.

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

Re: [Xen-devel] [xen-4.8-testing test] 144726: regressions - trouble: fail/pass/starved

2019-12-12 Thread Andrew Cooper
On 12/12/2019 15:54, Jürgen Groß wrote:
> On 12.12.19 16:51, Jan Beulich wrote:
>> On 12.12.2019 16:40, Ian Jackson wrote:
>>> osstest service owner writes ("[xen-4.8-testing test] 144726:
>>> regressions - trouble: fail/pass/starved"):
 flight 144726 xen-4.8-testing real [real]
 http://logs.test-lab.xenproject.org/osstest/logs/144726/

 Regressions :-(

 Tests which did not succeed and are blocking,
 including tests which could not be run:
   test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail
 REGR. vs. 144558
>>>
>>> See patch below for analysis.  Andrew, would you please take a look
>>> and check that what I said is true.
>>>
>>> If Jan approves will force push 4.8-testing.
>>
>> I do. Thanks.
>>
>>>  From Juergen I would like a release-ack for the osstest commit to
>>> "allow" it for the future.
>>
>> Is this really worth it? The 4.8 tree is dead now; strictly speaking
>> even the last batch of XSAs shouldn't have gone there anymore, but
>> we did so to be friendly to certain distros, as it was just barely
>> past the expiry date.
>
> At least I really don't mind, as I can't see any risk for 4.13.
>
> Release-acked-by: Juergen Gross 

LGTM.  Not fussed whether we take this patch, or kill 4.8 testing entirely.

~Andrew

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

Re: [Xen-devel] [PATCH] x86emul: correct LFS et al handling for 64-bit mode

2019-12-12 Thread Jan Beulich
On 12.12.2019 15:20, Andrew Cooper wrote:
> On 12/12/2019 13:05, Jan Beulich wrote:
>> On 12.12.2019 12:37, Andrew Cooper wrote:
>>> On 12/12/2019 10:11, Jan Beulich wrote:
 On 11.12.2019 21:57, Andrew Cooper wrote:
> On 11/12/2019 09:28, Jan Beulich wrote:
>> AMD and friends explicitly specify that 64-bit operands aren't possible
>> for these insns. Nevertheless REX.W isn't fully ignored: It still
>> cancels a possible operand size override (0x66). Intel otoh explicitly
>> provides for 64-bit operands on the respective insn page of the SDM.
>>
>> Signed-off-by: Jan Beulich 
> It is definitely more than just these.  Near jumps have per-vendor
> behaviour on how long the instruction is, whereas far jump/calls are in
> the same category as these by the looks of things.
 But you don't expect me to fold all of these into one patch, do
 you?
>>> short jmp certainly not, but far jmp/call is just two extra case
>>> statements in this new code block, no?
>> Not exactly (the other change would need to be in
>> x86_decode_onebyte() and depend on ModRM.reg), but yes, I can
>> do this. Yet then it would feel odd to not also deal with the
>> near counterparts at the same time. Which in turn would make
>> is desirable to also deal with near RET as well. At which
>> point we're about to also discuss CALL/JMP with displacement
>> operands and Jcc.
>>
 We have _some_ vendor dependent behavior already, and I'm
 slowly adding to it. Our far call/jmp support is rather
 incomplete in other ways anyway.
>>> There is different truncation behaviour for %rip which needs altering,
>>> but that is a separate area of code.  Anything else?
>> protmode_load_seg() and MOVSXD already have vendor dependent
>> code, if that was your question.
> 
> I was actually just asking about far jmp/call.
> 
> If you're sure that far jmp/call is more complicated than just tweaking
> this patch, then fine.  Acked-by: Andrew Cooper 

Thanks much. I'll see to put together the far branch counterpart
soon. Perhaps I can also do the near branch parts then.

>> For things needing doing see
>> above plus LOOP, J[ER]CXZ, SYSENTER, and SYSEXIT as far as I'm
>> currently aware.
> 
> SYSCALL and SYSRET as well.  The way they handle MSR_STAR is vendor
> specific, as well as #UD conditions.
> 
> I've just noticed that I've still got an XSA-204 followup patch still
> outstanding from 2016...

Oh. Looking forward to see it.

Jan

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

Re: [Xen-devel] [xen-4.8-testing test] 144726: regressions - trouble: fail/pass/starved

2019-12-12 Thread Jürgen Groß

On 12.12.19 16:51, Jan Beulich wrote:

On 12.12.2019 16:40, Ian Jackson wrote:

osstest service owner writes ("[xen-4.8-testing test] 144726: regressions - trouble: 
fail/pass/starved"):

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
  test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 144558


See patch below for analysis.  Andrew, would you please take a look
and check that what I said is true.

If Jan approves will force push 4.8-testing.


I do. Thanks.


 From Juergen I would like a release-ack for the osstest commit to
"allow" it for the future.


Is this really worth it? The 4.8 tree is dead now; strictly speaking
even the last batch of XSAs shouldn't have gone there anymore, but
we did so to be friendly to certain distros, as it was just barely
past the expiry date.


At least I really don't mind, as I can't see any risk for 4.13.

Release-acked-by: Juergen Gross 


Juergen

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

Re: [Xen-devel] [xen-4.8-testing test] 144726: regressions - trouble: fail/pass/starved

2019-12-12 Thread Jan Beulich
On 12.12.2019 16:40, Ian Jackson wrote:
> osstest service owner writes ("[xen-4.8-testing test] 144726: regressions - 
> trouble: fail/pass/starved"):
>> flight 144726 xen-4.8-testing real [real]
>> http://logs.test-lab.xenproject.org/osstest/logs/144726/
>>
>> Regressions :-(
>>
>> Tests which did not succeed and are blocking,
>> including tests which could not be run:
>>  test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 
>> 144558
> 
> See patch below for analysis.  Andrew, would you please take a look
> and check that what I said is true.
> 
> If Jan approves will force push 4.8-testing.

I do. Thanks.

> From Juergen I would like a release-ack for the osstest commit to
> "allow" it for the future.

Is this really worth it? The 4.8 tree is dead now; strictly speaking
even the last batch of XSAs shouldn't have gone there anymore, but
we did so to be friendly to certain distros, as it was just barely
past the expiry date.

Jan

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

Re: [Xen-devel] [xen-4.8-testing test] 144726: regressions - trouble: fail/pass/starved

2019-12-12 Thread Ian Jackson
osstest service owner writes ("[xen-4.8-testing test] 144726: regressions - 
trouble: fail/pass/starved"):
> flight 144726 xen-4.8-testing real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/144726/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 
> 144558

See patch below for analysis.  Andrew, would you please take a look
and check that what I said is true.

If Jan approves will force push 4.8-testing.

From Juergen I would like a release-ack for the osstest commit to
"allow" it for the future.

Thanks,
Ian.

From 93a4162b6d85bd1e78d822f1e807517c3e207ce7 Mon Sep 17 00:00:00 2001
From: Ian Jackson 
Date: Thu, 12 Dec 2019 14:37:11 +
Subject: [OSSTEST PATCH] allow: xen-4.8-testing xtf/test-hvm64-lbr-tsx-vmentry

Xen 4.8 lacks
  20f1976b44199d1e7a15fe5d2c8c1a4375b74997
  x86/vmx: Fix vmentry failure because of invalid LER on Broadwell
and
  d6e9f8d4f35d938417f9dc2ea50f6e8004e26725
  x86/vmx: fix vmentry failure with TSX bits in LBR
and given its support lifetime, we do not intend to backport them.

These bugs are checked for by xtf/test-hvm64-lbr-tsx-vmentry.  So
those tests fail in xen-4.8-testing but only on applicable hardware.
Because we don't pin these tests to individual hosts (because that
would involve running the XTF tests on each host pair) this can show
up as a "regression".  Force pushing it makes it go away for a bit,
until for some reason the test runs on a different host.

Instead, treat these "regressions" as allowable but only in 4.8.

I have tested this with
  ./sg-report-flight --that-flight=144558 144726
and diff'd before and after.  The difference is as expected, that
  test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 144558
is now
  Regressions which are regarded as allowable (not blocking):

CC: Andrew Cooper 
CC: Juergen Gross 
Signed-off-by: Ian Jackson 
---
 allow.xen-4.8-testing | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 allow.xen-4.8-testing

diff --git a/allow.xen-4.8-testing b/allow.xen-4.8-testing
new file mode 100644
index ..0aff6917
--- /dev/null
+++ b/allow.xen-4.8-testing
@@ -0,0 +1 @@
+test-xtf-amd64-amd64-@@xtf/test-hvm64-lbr-tsx-vmentry
-- 
2.11.0


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

Re: [Xen-devel] [PATCH v7 2/3] xen/blkback: Squeeze page pools if a memory pressure is detected

2019-12-12 Thread Roger Pau Monné
On Wed, Dec 11, 2019 at 06:10:15PM +, SeongJae Park wrote:
> diff --git a/drivers/block/xen-blkback/blkback.c 
> b/drivers/block/xen-blkback/blkback.c
> index fd1e19f1a49f..98823d150905 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -142,6 +142,21 @@ static inline bool persistent_gnt_timeout(struct 
> persistent_gnt *persistent_gnt)
>   HZ * xen_blkif_pgrant_timeout);
>  }
>  
> +/* Once a memory pressure is detected, squeeze free page pools for a while. 
> */
> +static unsigned int buffer_squeeze_duration_ms = 10;
> +module_param_named(buffer_squeeze_duration_ms,
> + buffer_squeeze_duration_ms, int, 0644);
> +MODULE_PARM_DESC(buffer_squeeze_duration_ms,
> +"Duration in ms to squeeze pages buffer when a memory pressure is detected");
> +
> +static unsigned long buffer_squeeze_end;
> +
> +void xen_blkbk_reclaim_memory(struct xenbus_device *dev)
> +{
> + buffer_squeeze_end = jiffies +
> + msecs_to_jiffies(buffer_squeeze_duration_ms);

I'm not sure this is fully correct. This function will be called for
each blkback instance, but the timeout is stored in a global variable
that's shared between all blkback instances. Shouldn't this timeout be
stored in xen_blkif so each instance has it's own local variable?

Or else in the case you have 1k blkback instances the timeout is
certainly going to be longer than expected, because each call to
xen_blkbk_reclaim_memory will move it forward.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v7 2/3] xen/blkback: Squeeze page pools if a memory pressure is detected

2019-12-12 Thread Roger Pau Monné
On Thu, Dec 12, 2019 at 02:39:05PM +0100, SeongJae Park wrote:
> On Thu, 12 Dec 2019 12:42:47 +0100 "Roger Pau Monné"  
> wrote:
> > > On the slow block device
> > > 
> > > 
> > > max_pgs   Min   Max   Median AvgStddev
> > > 0 38.7  45.8  38.7   40.12  3.1752165
> > > 1024  38.7  45.8  38.7   40.12  3.1752165
> > > No difference proven at 95.0% confidence
> > > 
> > > On the fast block device
> > > 
> > > 
> > > max_pgs   Min   Max   Median AvgStddev
> > > 0 417   423   420419.4  2.5099801
> > > 1024  414   425   416417.8  4.4384682
> > > No difference proven at 95.0% confidence
> > 
> > This is intriguing, as it seems to prove that the usage of a cache of
> > free pages is irrelevant performance wise.
> > 
> > The pool of free pages was introduced long ago, and it's possible that
> > recent improvements to the balloon driver had made such pool useless,
> > at which point it could be removed instead of worked around.
> 
> I guess the grant page allocation overhead in this test scenario is really
> small.  In an absence of memory pressure, fragmentation, and NUMA imbalance,
> the latency of the page allocation ('get_page()') is very short, as it will
> success in the fast path.

The allocation of the pool of free pages involves more than get_page,
it uses gnttab_alloc_pages which in the worse case will allocate a
page and balloon it out issuing one hypercall.

> Few years ago, I once measured the page allocation latency on my machine.
> Roughly speaking, it was about 1us in best case, 100us in worst case, and 5us
> in average.  Please keep in mind that the measurement was not designed and
> performed in serious way.  Thus the results could have profile overhead in it,
> though.  While keeping that in mind, let's simply believe the number and 
> ignore
> the latency of the block layer, blkback itself (including the grant
> mapping), and anything else including context switch, cache miss, but the
> allocation.  In other words, suppose that the grant page allocation is only 
> one
> source of the overhead.  It will be able to achieve 1 million IOPS (4KB *
> 1MIOPS = 4 GB/s) in the best case, 200 thousand IOPS (800 MB/s) in average, 
> and
> 10 thousand IOPS (40 MB/s) in worst case.  Based on this coarse calculation, I
> think the test results is reasonable.
> 
> This also means that the effect of the blkback's free pages pool might be
> visible under page allocation fast path failure situation.  Nevertheless, it
> would be also hard to measure that in micro level unless the measurement is
> well designed and controlled.
> 
> > 
> > Do you think you could perform some more tests (as pointed out above
> > against the block device to skip the fs overhead) and report back the
> > results?
> 
> To be honest, I'm not sure whether additional tests are really necessary,
> because I think the `dd` test and the results explanation already makes some
> sense and provide the minimal proof of the concept.  Also, this change is a
> fallback for the memory pressure situation, which is an error path in some
> point of view.  Such errorneous situation might not happen frequently and if
> the situation is not solved in short time, something much worse (e.g., OOM 
> kill
> of the user space xen control processes) than temporal I/O performance
> degradation could happen.  Thus, I'm not sure whether such detailed 
> performance
> measurement is necessary for this rare error handling change.

Right, my main concern is that we seem to be adding duck tape so
things don't fall apart, but if such cache is really not beneficial
from a performance PoV I would rather see it go away than adding more
stuff to it in order to workaround corner cases like memory
starvation.

Anyway, I guess we can take such change, but long term we need to look
into fixing grants to not use ballooned pages, and figure out if the
blkback free page cache is really useful or not.

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v4 4/9] x86/mm: introduce l{1, 2}t local variables to modify_xen_mappings

2019-12-12 Thread Julien Grall

Hi,

On 04/12/2019 17:10, Hongyan Xia wrote:

From: Wei Liu 

The pl2e and pl1e variables are heavily (ab)used in that function.  It
is fine at the moment because all page tables are always mapped so
there is no need to track the life time of each variable.

We will soon have the requirement to map and unmap page tables. We
need to track the life time of each variable to avoid leakage.

Introduce some l{1,2}t variables with limited scope so that we can
track life time of pointers to xen page tables more easily.

No functional change.

Signed-off-by: Wei Liu 
Reviewed-by: Jan Beulich 
---
  xen/arch/x86/mm.c | 68 ++-
  1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 790578d2b3..303bc35549 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5601,6 +5601,8 @@ int modify_xen_mappings(unsigned long s, unsigned long e, 
unsigned int nf)
  
  if ( l3e_get_flags(*pl3e) & _PAGE_PSE )

  {
+l2_pgentry_t *l2t;
+
  if ( l2_table_offset(v) == 0 &&
   l1_table_offset(v) == 0 &&
   ((e - v) >= (1UL << L3_PAGETABLE_SHIFT)) )
@@ -5616,11 +5618,11 @@ int modify_xen_mappings(unsigned long s, unsigned long 
e, unsigned int nf)
  }
  
  /* PAGE1GB: shatter the superpage and fall through. */

-pl2e = alloc_xen_pagetable();
-if ( !pl2e )
+l2t = alloc_xen_pagetable();
+if ( !l2t )
  return -ENOMEM;


Indirectly related to this patch, it looks like TLBs will not be flushed 
even part of the mapping is not removed.


Another problem I have spotted is most of the callers of 
map_pages_to_xen() & modify_xen_mappings() will never check the return 
value.


If we plan to use destroy_xen_mappings() for unmapping xenheap page, 
then we will need to ensure that destroy_xen_mappings() will never fail. 
Otherwise we will end up to keep part of the mappings and therefore 
defeating the purpose of secret hiding.


This may mean that shattering/merging should be prevented for xenheap 
region.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [BUG] Xen-ballooned memory never returned to domain after partial-free

2019-12-12 Thread Nicholas Tsirakis
>> Do you happen to know the answer to my second question? It's not as 
>> important,
>> but it does confuse me as I wouldn't expect the total memory to be
>> balloon-able at
>> all with the hotplugging configs disabled.

> Ballooning != hotplugging memory
>
> With memory hotplug you can add (or - in theory - remove) memory to the
> kernel it didn't know about before.
>
> With ballooning you just give some memory back to the hypervisor, but
> kernel still has some knowledge about it (e.g. keeps struct page for
> each ballooned memory page).

Got it, thanks for that clarification and for all your help!

--Niko

On Thu, Dec 12, 2019 at 9:21 AM Jürgen Groß  wrote:
>
> On 12.12.19 15:10, Nicholas Tsirakis wrote:
> >> And I think this is the problem. We want here:
> >>
> >>  balloon_stats.target_pages = balloon_stats.current_pages +
> >>   balloon_stats.target_unpopulated;
> >
> > Ahh I knew I was missing something. Tested the patch, works great! 
> > "Reported by"
> > is fine with me.
>
> Thanks.
>
> >
> > Do you happen to know the answer to my second question? It's not as 
> > important,
> > but it does confuse me as I wouldn't expect the total memory to be
> > balloon-able at
> > all with the hotplugging configs disabled.
>
> Ballooning != hotplugging memory
>
> With memory hotplug you can add (or - in theory - remove) memory to the
> kernel it didn't know about before.
>
> With ballooning you just give some memory back to the hypervisor, but
> kernel still has some knowledge about it (e.g. keeps struct page for
> each ballooned memory page).
>
> HTH, Juergen

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

Re: [Xen-devel] [BUG] Xen-ballooned memory never returned to domain after partial-free

2019-12-12 Thread Jürgen Groß

On 12.12.19 15:10, Nicholas Tsirakis wrote:

And I think this is the problem. We want here:

 balloon_stats.target_pages = balloon_stats.current_pages +
  balloon_stats.target_unpopulated;


Ahh I knew I was missing something. Tested the patch, works great! "Reported by"
is fine with me.


Thanks.



Do you happen to know the answer to my second question? It's not as important,
but it does confuse me as I wouldn't expect the total memory to be
balloon-able at
all with the hotplugging configs disabled.


Ballooning != hotplugging memory

With memory hotplug you can add (or - in theory - remove) memory to the
kernel it didn't know about before.

With ballooning you just give some memory back to the hypervisor, but
kernel still has some knowledge about it (e.g. keeps struct page for
each ballooned memory page).

HTH, Juergen

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

Re: [Xen-devel] [PATCH] x86emul: correct LFS et al handling for 64-bit mode

2019-12-12 Thread Andrew Cooper
On 12/12/2019 13:05, Jan Beulich wrote:
> On 12.12.2019 12:37, Andrew Cooper wrote:
>> On 12/12/2019 10:11, Jan Beulich wrote:
>>> On 11.12.2019 21:57, Andrew Cooper wrote:
 On 11/12/2019 09:28, Jan Beulich wrote:
> AMD and friends explicitly specify that 64-bit operands aren't possible
> for these insns. Nevertheless REX.W isn't fully ignored: It still
> cancels a possible operand size override (0x66). Intel otoh explicitly
> provides for 64-bit operands on the respective insn page of the SDM.
>
> Signed-off-by: Jan Beulich 
 It is definitely more than just these.  Near jumps have per-vendor
 behaviour on how long the instruction is, whereas far jump/calls are in
 the same category as these by the looks of things.
>>> But you don't expect me to fold all of these into one patch, do
>>> you?
>> short jmp certainly not, but far jmp/call is just two extra case
>> statements in this new code block, no?
> Not exactly (the other change would need to be in
> x86_decode_onebyte() and depend on ModRM.reg), but yes, I can
> do this. Yet then it would feel odd to not also deal with the
> near counterparts at the same time. Which in turn would make
> is desirable to also deal with near RET as well. At which
> point we're about to also discuss CALL/JMP with displacement
> operands and Jcc.
>
>>> We have _some_ vendor dependent behavior already, and I'm
>>> slowly adding to it. Our far call/jmp support is rather
>>> incomplete in other ways anyway.
>> There is different truncation behaviour for %rip which needs altering,
>> but that is a separate area of code.  Anything else?
> protmode_load_seg() and MOVSXD already have vendor dependent
> code, if that was your question.

I was actually just asking about far jmp/call.

If you're sure that far jmp/call is more complicated than just tweaking
this patch, then fine.  Acked-by: Andrew Cooper 

> For things needing doing see
> above plus LOOP, J[ER]CXZ, SYSENTER, and SYSEXIT as far as I'm
> currently aware.

SYSCALL and SYSRET as well.  The way they handle MSR_STAR is vendor
specific, as well as #UD conditions.

I've just noticed that I've still got an XSA-204 followup patch still
outstanding from 2016...

~Andrew

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

[Xen-devel] [PATCH] xen/balloon: fix ballooned page accounting without hotplug enabled

2019-12-12 Thread Juergen Gross
When CONFIG_XEN_BALLOON_MEMORY_HOTPLUG is not defined
reserve_additional_memory() will set balloon_stats.target_pages to a
wrong value in case there are still some ballooned pages allocated via
alloc_xenballooned_pages().

This will result in balloon_process() no longer be triggered when
ballooned pages are freed in batches.

Reported-by: Nicholas Tsirakis 
Signed-off-by: Juergen Gross 
---
 drivers/xen/balloon.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 4f2e78a5e4db..0c142bcab79d 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -394,7 +394,8 @@ static struct notifier_block xen_memory_nb = {
 #else
 static enum bp_state reserve_additional_memory(void)
 {
-   balloon_stats.target_pages = balloon_stats.current_pages;
+   balloon_stats.target_pages = balloon_stats.current_pages +
+balloon_stats.target_unpopulated;
return BP_ECANCELED;
 }
 #endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
-- 
2.16.4


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

Re: [Xen-devel] [PATCH-for-4.13] build: fix tools/configure in case only python3 exists

2019-12-12 Thread Wei Liu
On Thu, Dec 12, 2019 at 02:11:32PM +, Ian Jackson wrote:
> Thanks for tidying this up.
> 
> Juergen Gross writes ("[PATCH-for-4.13] build: fix tools/configure in case 
> only python3 exists"):
> > -AS_IF([test -z "$PYTHON"], [PYTHON="python"])
> > -AS_IF([echo "$PYTHON" | grep -q "^/"], [], [PYTHON=`type -p "$PYTHON"`])
> > +AS_IF([test -z "$PYTHON"], [AC_CHECK_PROGS([PYTHON], [python python3 
> > python2], err)])
> > +AS_IF([test "$PYTHON" = "err"], [AC_MSG_ERROR([No python interpreter 
> > found])])
> 
> I think this use of `err' is a bit odd.  According to the FM you could
> say simply:
> 
>   +AS_IF([test -z "$PYTHON"], [AC_CHECK_PROGS([PYTHON], [python python3 
> python2])])
>   +AS_IF([test -z "$PYTHON"], [AC_MSG_ERROR([No python interpreter found])])
> 
> But this is a style nit I think since no-one will call their python
> interpreter `err' :-).  And you will have tested your version and at
> this stage of 4.13 it would be better to have fewer iterations of this
> patch, so I think it should go in as it is.
> 
> > +AS_IF([echo "$PYTHON" | grep -q "^/"], [], [AC_PATH_PROG([PYTHON], 
> > [$PYTHON])])
> 
> Thanks.
> 
> Reviewed-by: Ian Jackson 

Thanks. I will push this to staging and staging-4.13 shortly.

Wei.

> 
> Ian.

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

Re: [Xen-devel] [PATCH-for-4.13] build: fix tools/configure in case only python3 exists

2019-12-12 Thread Ian Jackson
Thanks for tidying this up.

Juergen Gross writes ("[PATCH-for-4.13] build: fix tools/configure in case only 
python3 exists"):
> -AS_IF([test -z "$PYTHON"], [PYTHON="python"])
> -AS_IF([echo "$PYTHON" | grep -q "^/"], [], [PYTHON=`type -p "$PYTHON"`])
> +AS_IF([test -z "$PYTHON"], [AC_CHECK_PROGS([PYTHON], [python python3 
> python2], err)])
> +AS_IF([test "$PYTHON" = "err"], [AC_MSG_ERROR([No python interpreter 
> found])])

I think this use of `err' is a bit odd.  According to the FM you could
say simply:

  +AS_IF([test -z "$PYTHON"], [AC_CHECK_PROGS([PYTHON], [python python3 
python2])])
  +AS_IF([test -z "$PYTHON"], [AC_MSG_ERROR([No python interpreter found])])

But this is a style nit I think since no-one will call their python
interpreter `err' :-).  And you will have tested your version and at
this stage of 4.13 it would be better to have fewer iterations of this
patch, so I think it should go in as it is.

> +AS_IF([echo "$PYTHON" | grep -q "^/"], [], [AC_PATH_PROG([PYTHON], 
> [$PYTHON])])

Thanks.

Reviewed-by: Ian Jackson 

Ian.

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

Re: [Xen-devel] [BUG] Xen-ballooned memory never returned to domain after partial-free

2019-12-12 Thread Nicholas Tsirakis
> And I think this is the problem. We want here:
>
> balloon_stats.target_pages = balloon_stats.current_pages +
>  balloon_stats.target_unpopulated;

Ahh I knew I was missing something. Tested the patch, works great! "Reported by"
is fine with me.

Do you happen to know the answer to my second question? It's not as important,
but it does confuse me as I wouldn't expect the total memory to be
balloon-able at
all with the hotplugging configs disabled.

--Niko

On Thu, Dec 12, 2019 at 2:18 AM Jürgen Groß  wrote:
>
> On 11.12.19 23:08, Nicholas Tsirakis wrote:
> > Hello,
> >
> > The issue I'm seeing is that pages of previously-xenballooned memory are 
> > getting
> > trapped in the balloon on free, specifically when they are free'd in batches
> > (i.e. not all at once). The first batch is restored to the domain properly, 
> > but
> > subsequent frees are not.
> >
> > Truthfully I'm not sure if this is a bug or not, but the behavior I'm seeing
> > doesn't seem to make sense. Note that this "bug" is in the balloon driver, 
> > but
> > the behavior is seen when using the gnttab API, which utilizes the balloon 
> > in
> > the background.
> >
> > --
> >
> > This issue is better illustrated as an example, seen below. Note that the 
> > file
> > in question is drivers/xen/balloon.c:
> >
> > Kernel version: 4.19.*, code seems consistent on master as well
> > Relevant configs:
> >  - CONFIG_MEMORY_HOTPLUG not set
> >  - CONFIG_XEN_BALLOON_MEMORY_HOTPLUG not set
> >
> > * current_pages = # of pages assigned to domain
> > * target_pages = # of pages we want assigned to domain
> > * credit = target - current
> >
> > Start with current_pages/target_pages = 20 pages
> >
> > 1. alloc 5 pages with gnttab_alloc_pages(). current_pages = 15, credit = 5.
> > 2. alloc 3 pages with gnttab_alloc_pages(). current_pages = 12, credit = 8.
> > 3. some time later, free the last 3 pages with gnttab_free_pages().
> > 4. 3 pages go back to balloon and balloon worker is scheduled since credit 
> > > 0.
> >  * Relevant part of balloon worker shown below:
> >
> >  do {
> >  ...
> >
> >  credit = current_credit();
> >
> >  if (credit > 0) {
> >  if (balloon_is_inflated())
> >  state = increase_reservation(credit);
> >  else
> >  state = reserve_additional_memory();
> >  }
> >
> >  ...
> >
> >  } while (credit && state == BP_DONE);
> >
> > 5. credit > 0 and the balloon contains 3 pages, so run 
> > increase_reservation. 3
> > pages are restored to domain, correctly. current_pages = 15, credit = 5.
> > 6. at this point credit is still > 0, so we loop again.
> > 7. this time, the balloon has 0 pages, so we call reserve_additional_memory,
> > seen below. note that CONFIG_XEN_BALLOON_MEMORY_HOTPLUG is disabled, so 
> > this
> > funciton is very sparse.
> >
> >  static enum bp_state reserve_additional_memory(void)
> >  {
> >  balloon_stats.target_pages = balloon_stats.current_pages;
> >  return BP_ECANCELED;
> >  }
> >
> > 8. now target = current = 15, which drops our credit down to 0.
>
> And I think this is the problem. We want here:
>
>  balloon_stats.target_pages = balloon_stats.current_pages +
>   balloon_stats.target_unpopulated;
>
> This should fix it. Thanks for the detailed analysis!
>
> Does the attached patch work for you?
>
> And are you fine with the "Reported-by:" added?
>
>
> Juergen

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

Re: [Xen-devel] [PATCH] IOMMU: make DMA containment of quarantined devices optional

2019-12-12 Thread Jan Beulich
On 12.12.2019 14:15, Roger Pau Monné wrote:
> On Thu, Dec 12, 2019 at 10:28:26AM +0100, Jan Beulich wrote:
>> On 11.12.2019 16:54, Roger Pau Monné wrote:
>>> On Wed, Dec 11, 2019 at 01:53:00PM +0100, Jan Beulich wrote:
 --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
 +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
 @@ -85,18 +85,36 @@ int get_dma_requestor_id(uint16_t seg, u
  return req_id;
  }
  
 -static void amd_iommu_setup_domain_device(
 +static int __must_check allocate_domain_resources(struct domain_iommu *hd)
 +{
 +int rc;
 +
 +spin_lock(>arch.mapping_lock);
 +rc = amd_iommu_alloc_root(hd);
 +spin_unlock(>arch.mapping_lock);
 +
 +return rc;
 +}
 +
 +static int __must_check amd_iommu_setup_domain_device(
  struct domain *domain, struct amd_iommu *iommu,
  uint8_t devfn, struct pci_dev *pdev)
  {
  struct amd_iommu_dte *table, *dte;
  unsigned long flags;
 -int req_id, valid = 1;
 +int req_id, valid = 1, rc;
  u8 bus = pdev->bus;
 -const struct domain_iommu *hd = dom_iommu(domain);
 +struct domain_iommu *hd = dom_iommu(domain);
 +
 +/* dom_io is used as a sentinel for quarantined devices */
 +if ( domain == dom_io && !hd->arch.root_table )
>>>
>>> This condition (and it's Intel counterpart) would be better in a macro
>>> IMO, so that vendor code regardless of the implementation can use the
>>> same macro (and to avoid having to add the same comment in all
>>> instances), ie: IS_DEVICE_QUARANTINED or some such would be fine IMO.
>>
>> The "device" in the name suggested is inapplicable, as there's no
>> device involved here. The conditional also isn't about
>> "quarantined", but about the extended for thereof.
> 
> Maybe IS_QUARANTINE_FULL or IS_FULLY_QUARANTINED or something similar
> in order to match the command line option then?

And IS_*() or is_*() ought to have an object to pass to. What would
the object be here? The domain isn't applicable. Maybe something
like FULL_QUARANTINE_MODE() (without any parameters) ...

>> I further don't
>> understand "vendor code" in your remark: Different macros would be
>> needed for either vendor anyway.
> 
> Yes, but both macros would have the same name, hence you wouldn't need
> to think whether you are in AMD or Intel code as the macro would
> always have the same name.
> 
>> (I did actually consider having
>> some kind of predicate helper, but I couldn't come up with a
>> sufficiently good name. I also think such an abstraction should
>> then have been introduced when these conditionals were first added
>> in their then still vendor independent form.)
> 
> I would prefer some kind of macro, as I think there's quite a lot of
> replication of those two checks, and IMO it's easy to by mistake use
> the wrong one when moving between Intel and AMD code (the more that
> it would build fine but lead to runtime issues).

Makes sense, as long as we can find a half way suitable name.

 --- a/xen/include/xen/iommu.h
 +++ b/xen/include/xen/iommu.h
 @@ -53,8 +53,9 @@ static inline bool_t dfn_eq(dfn_t x, dfn
  }
  
  extern bool_t iommu_enable, iommu_enabled;
 -extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
 +extern bool force_iommu, iommu_verbose, iommu_igfx;
  extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
 +extern uint8_t iommu_quarantine;
>>>
>>> Exporting this variable without the paired defines seems pointless,
>>> how are external callers supposed to figure out the quarantine mode
>>> without the IOMMU_quarantine_* defines?
>>
>> Why pointless? Outside of the file knowing the IOMMU_quarantine_*
>> defines the variable continues to have boolean meaning.
> 
> Do you think you could add a comment to that effect?

Will do.

Jan

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

[Xen-devel] [xen-4.8-testing test] 144726: regressions - trouble: fail/pass/starved

2019-12-12 Thread osstest service owner
flight 144726 xen-4.8-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/144726/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail REGR. vs. 144558

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 144558
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 144558
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144558
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144558
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 144558
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 144558
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144558
 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 144558
 test-amd64-i386-libvirt  13 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  13 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-seattle  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx  2 hosts-allocate   starved  n/a

version targeted for testing:
 xen  8db85532cbb80c6396e5dab8809feb7b7b0d5c45
baseline version:
 xen  a260e93db794f560502e89859aaf111d178e80e4

Last test of basis   144558  2019-12-05 17:36:17 Z6 days
Testing same since   144726  2019-12-11 15:10:46 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  George Dunlap 
  Jan Beulich 
  Julien Grall 
  Kevin Tian 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 

[Xen-devel] [PATCH net-next] xen-netback: get rid of old udev related code

2019-12-12 Thread Paul Durrant
In the past it used to be the case that the Xen toolstack relied upon
udev to execute backend hotplug scripts. However this has not been the
case for many releases now and removal of the associated code in
xen-netback shortens the source by more than 100 lines, and removes much
complexity in the interaction with the xenstore backend state.

NOTE: xen-netback is the only xenbus driver to have a functional uevent()
  method. The only other driver to have a method at all is
  pvcalls-back, and currently pvcalls_back_uevent() simply returns 0.
  Hence this patch also facilitates further cleanup.

Signed-off-by: Paul Durrant 
---
Cc: Wei Liu 
Cc: "David S. Miller" 
---
 drivers/net/xen-netback/common.h |  11 ---
 drivers/net/xen-netback/xenbus.c | 125 ---
 2 files changed, 14 insertions(+), 122 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 05847eb91a1b..e48da004c1a3 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -251,17 +251,6 @@ struct xenvif_hash {
 struct backend_info {
struct xenbus_device *dev;
struct xenvif *vif;
-
-   /* This is the state that will be reflected in xenstore when any
-* active hotplug script completes.
-*/
-   enum xenbus_state state;
-
-   enum xenbus_state frontend_state;
-   struct xenbus_watch hotplug_status_watch;
-   u8 have_hotplug_status_watch:1;
-
-   const char *hotplug_script;
 };
 
 struct xenvif {
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index f533b7372d59..4e89393d5dd8 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -15,7 +15,6 @@ static int connect_data_rings(struct backend_info *be,
 static void connect(struct backend_info *be);
 static int read_xenbus_vif_flags(struct backend_info *be);
 static int backend_create_xenvif(struct backend_info *be);
-static void unregister_hotplug_status_watch(struct backend_info *be);
 static void xen_unregister_watchers(struct xenvif *vif);
 static void set_backend_state(struct backend_info *be,
  enum xenbus_state state);
@@ -199,17 +198,11 @@ static int netback_remove(struct xenbus_device *dev)
 {
struct backend_info *be = dev_get_drvdata(>dev);
 
-   set_backend_state(be, XenbusStateClosed);
-
-   unregister_hotplug_status_watch(be);
if (be->vif) {
-   kobject_uevent(>dev.kobj, KOBJ_OFFLINE);
xen_unregister_watchers(be->vif);
-   xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
xenvif_free(be->vif);
be->vif = NULL;
}
-   kfree(be->hotplug_script);
kfree(be);
dev_set_drvdata(>dev, NULL);
return 0;
@@ -227,7 +220,6 @@ static int netback_probe(struct xenbus_device *dev,
struct xenbus_transaction xbt;
int err;
int sg;
-   const char *script;
struct backend_info *be = kzalloc(sizeof(struct backend_info),
  GFP_KERNEL);
if (!be) {
@@ -239,7 +231,6 @@ static int netback_probe(struct xenbus_device *dev,
be->dev = dev;
dev_set_drvdata(>dev, be);
 
-   be->state = XenbusStateInitialising;
err = xenbus_switch_state(dev, XenbusStateInitialising);
if (err)
goto fail;
@@ -347,21 +338,12 @@ static int netback_probe(struct xenbus_device *dev,
if (err)
pr_debug("Error writing feature-ctrl-ring\n");
 
-   script = xenbus_read(XBT_NIL, dev->nodename, "script", NULL);
-   if (IS_ERR(script)) {
-   err = PTR_ERR(script);
-   xenbus_dev_fatal(dev, err, "reading script");
-   goto fail;
-   }
-
-   be->hotplug_script = script;
-
-
-   /* This kicks hotplug scripts, so do it immediately. */
err = backend_create_xenvif(be);
if (err)
goto fail;
 
+   set_backend_state(be, XenbusStateInitWait);
+
return 0;
 
 abort_transaction:
@@ -374,29 +356,6 @@ static int netback_probe(struct xenbus_device *dev,
 }
 
 
-/*
- * Handle the creation of the hotplug script environment.  We add the script
- * and vif variables to the environment, for the benefit of the vif-* hotplug
- * scripts.
- */
-static int netback_uevent(struct xenbus_device *xdev,
- struct kobj_uevent_env *env)
-{
-   struct backend_info *be = dev_get_drvdata(>dev);
-
-   if (!be)
-   return 0;
-
-   if (add_uevent_var(env, "script=%s", be->hotplug_script))
-   return -ENOMEM;
-
-   if (!be->vif)
-   return 0;
-
-   return add_uevent_var(env, "vif=%s", be->vif->dev->name);
-}
-
-
 static int backend_create_xenvif(struct backend_info *be)
 {
int err;
@@ -422,7 +381,6 @@ static int backend_create_xenvif(struct backend_info *be)

Re: [Xen-devel] [PATCH v7 2/3] xen/blkback: Squeeze page pools if a memory pressure is detected

2019-12-12 Thread SeongJae Park
On Thu, 12 Dec 2019 12:42:47 +0100 "Roger Pau Monné"  
wrote:

> 
> Please make sure you Cc me in blkback related patches.

Sorry for forgotting you!  I will never forget again.

> 
> On Wed, Dec 11, 2019 at 06:10:15PM +, SeongJae Park wrote:
> > Each `blkif` has a free pages pool for the grant mapping.  The size of
> > the pool starts from zero and be increased on demand while processing
> ^ is
> > the I/O requests.  If current I/O requests handling is finished or 100
> > milliseconds has passed since last I/O requests handling, it checks and
> > shrinks the pool to not exceed the size limit, `max_buffer_pages`.
> > 
> > Therefore, host administrators can cause memory pressure in blkback by
> > attaching a large number of block devices and inducing I/O.  Such
> > problematic situations can be avoided by limiting the maximum number of
> > devices that can be attached, but finding the optimal limit is not so
> > easy.  Improper set of the limit can results in the memory pressure or a
>   ^ s/the//
> > resource underutilization.  This commit avoids such problematic
> > situations by squeezing the pools (returns every free page in the pool
> > to the system) for a while (users can set this duration via a module
> > parameter) if a memory pressure is detected.
> ^ s/a//
> > 
> > Discussions
> > ===
> > 
> > The `blkback`'s original shrinking mechanism returns only pages in the
> > pool, which are not currently be used by `blkback`, to the system.  In
> 
> I think you can remove both comas in the above sentence.
> 
> > other words, the pages that are not mapped with granted pages.  Because
> > this commit is changing only the shrink limit but still uses the same
> > freeing mechanism it does not touch pages which are currently mapping
> > grants.
> > 
> > Once a memory pressure is detected, this commit keeps the squeezing
>^ s/a//

Thank you for corrections, will apply!

> > limit for a user-specified time duration.  The duration should be
> > neither too long nor too short.  If it is too long, the squeezing
> > incurring overhead can reduce the I/O performance.  If it is too short,
> > `blkback` will not free enough pages to reduce the memory pressure.
> > This commit sets the value as `10 milliseconds` by default because it is
> > a short time in terms of I/O while it is a long time in terms of memory
> > operations.  Also, as the original shrinking mechanism works for at
> > least every 100 milliseconds, this could be a somewhat reasonable
> > choice.  I also tested other durations (refer to the below section for
> > more details) and confirmed that 10 milliseconds is the one that works
> > best with the test.  That said, the proper duration depends on actual
> > configurations and workloads.  That's why this commit allows users to
> > set the duration as a module parameter.
> > 
> > Memory Pressure Test
> > 
> > 
> > To show how this commit fixes the memory pressure situation well, I
> > configured a test environment on a xen-running virtualization system.
> > On the `blkfront` running guest instances, I attach a large number of
> > network-backed volume devices and induce I/O to those.  Meanwhile, I
> > measure the number of pages that swapped in (pswpin) and out (pswpout)
> > on the `blkback` running guest.  The test ran twice, once for the
> > `blkback` before this commit and once for that after this commit.  As
> > shown below, this commit has dramatically reduced the memory pressure:
> > 
> > pswpin  pswpout
> > before  76,672  185,799
> > after  2123,325
> > 
> > Optimal Aggressive Shrinking Duration
> > -
> > 
> > To find a best squeezing duration, I repeated the test with three
> > different durations (1ms, 10ms, and 100ms).  The results are as below:
> > 
> > durationpswpin  pswpout
> > 1   852 6,424
> > 10  212 3,325
> > 100 203 3,340
> > 
> > As expected, the memory pressure has decreased as the duration is
> > increased, but the reduction stopped from the `10ms`.  Based on this
> > results, I chose the default duration as 10ms.
> > 
> > Performance Overhead Test
> > =
> > 
> > This commit could incur I/O performance degradation under severe memory
> > pressure because the squeezing will require more page allocations per
> > I/O.  To show the overhead, I artificially made a worst-case squeezing
> > situation and measured the I/O performance of a `blkfront` running
> > guest.
> > 
> > For the artificial squeezing, I set the `blkback.max_buffer_pages` using
> > the `/sys/module/xen_blkback/parameters/max_buffer_pages` file.  In this
> > test, I set the value to `1024` and `0`.  The `1024` is the default
> > value.  Setting the value as `0` is same to a situation doing the
> > squeezing always (worst-case).
> > 

Re: [Xen-devel] [PATCH v4 0/2] Refactor super page shattering

2019-12-12 Thread Jan Beulich
On 12.12.2019 13:46, Hongyan Xia wrote:
> map_pages_to_xen and modify_xen_mappings use almost exactly the same
> page shattering logic, and the code is mingled with other PTE
> manipulations so it is less obvious that the intention is page
> shattering. Factor out the functions to make them reusable and to make
> the intention more obvious.
> 
> Of course, there is not much difference between the shattering logic of
> each level, so we could further turn the per-level functions into a
> single macro, although this is not that simple since we have per-level
> functions and macros all over the place and there are slight differences
> between levels. Keep it per-level for now.

FWIW these look okay to me now, and I would give them my R-b without
if there wasn't the type safety issue. Andrew?

Jan

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

Re: [Xen-devel] [PATCH] IOMMU: make DMA containment of quarantined devices optional

2019-12-12 Thread Roger Pau Monné
On Thu, Dec 12, 2019 at 10:28:26AM +0100, Jan Beulich wrote:
> On 11.12.2019 16:54, Roger Pau Monné wrote:
> > On Wed, Dec 11, 2019 at 01:53:00PM +0100, Jan Beulich wrote:
> >> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> >> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> >> @@ -85,18 +85,36 @@ int get_dma_requestor_id(uint16_t seg, u
> >>  return req_id;
> >>  }
> >>  
> >> -static void amd_iommu_setup_domain_device(
> >> +static int __must_check allocate_domain_resources(struct domain_iommu *hd)
> >> +{
> >> +int rc;
> >> +
> >> +spin_lock(>arch.mapping_lock);
> >> +rc = amd_iommu_alloc_root(hd);
> >> +spin_unlock(>arch.mapping_lock);
> >> +
> >> +return rc;
> >> +}
> >> +
> >> +static int __must_check amd_iommu_setup_domain_device(
> >>  struct domain *domain, struct amd_iommu *iommu,
> >>  uint8_t devfn, struct pci_dev *pdev)
> >>  {
> >>  struct amd_iommu_dte *table, *dte;
> >>  unsigned long flags;
> >> -int req_id, valid = 1;
> >> +int req_id, valid = 1, rc;
> >>  u8 bus = pdev->bus;
> >> -const struct domain_iommu *hd = dom_iommu(domain);
> >> +struct domain_iommu *hd = dom_iommu(domain);
> >> +
> >> +/* dom_io is used as a sentinel for quarantined devices */
> >> +if ( domain == dom_io && !hd->arch.root_table )
> > 
> > This condition (and it's Intel counterpart) would be better in a macro
> > IMO, so that vendor code regardless of the implementation can use the
> > same macro (and to avoid having to add the same comment in all
> > instances), ie: IS_DEVICE_QUARANTINED or some such would be fine IMO.
> 
> The "device" in the name suggested is inapplicable, as there's no
> device involved here. The conditional also isn't about
> "quarantined", but about the extended for thereof.

Maybe IS_QUARANTINE_FULL or IS_FULLY_QUARANTINED or something similar
in order to match the command line option then?

The comment above explicitly mentions that dom_io is used as a
sentinel for quarantined devices, hence the DEVICE in the name didn't
seem that far off.

> I further don't
> understand "vendor code" in your remark: Different macros would be
> needed for either vendor anyway.

Yes, but both macros would have the same name, hence you wouldn't need
to think whether you are in AMD or Intel code as the macro would
always have the same name.

> (I did actually consider having
> some kind of predicate helper, but I couldn't come up with a
> sufficiently good name. I also think such an abstraction should
> then have been introduced when these conditionals were first added
> in their then still vendor independent form.)

I would prefer some kind of macro, as I think there's quite a lot of
replication of those two checks, and IMO it's easy to by mistake use
the wrong one when moving between Intel and AMD code (the more that
it would build fine but lead to runtime issues).

> 
> >> --- a/xen/drivers/passthrough/iommu.c
> >> +++ b/xen/drivers/passthrough/iommu.c
> >> @@ -30,13 +30,17 @@ bool_t __initdata iommu_enable = 1;
> >>  bool_t __read_mostly iommu_enabled;
> >>  bool_t __read_mostly force_iommu;
> >>  bool_t __read_mostly iommu_verbose;
> >> -bool __read_mostly iommu_quarantine = true;
> >>  bool_t __read_mostly iommu_igfx = 1;
> >>  bool_t __read_mostly iommu_snoop = 1;
> >>  bool_t __read_mostly iommu_qinval = 1;
> >>  bool_t __read_mostly iommu_intremap = 1;
> >>  bool_t __read_mostly iommu_crash_disable;
> >>  
> >> +#define IOMMU_quarantine_none  false
> >> +#define IOMMU_quarantine_basic true
> >> +#define IOMMU_quarantine_full  2
> >> +uint8_t __read_mostly iommu_quarantine = IOMMU_quarantine_basic;
> > 
> > I don't really like to use booleans with non-boolean variables.
> > Wouldn't it be better to just use plain numbers, or even better an
> > enum?
> 
> No option is really good here, I think. I did consider using an
> enum, but I wanted to restrict the variable to 8 bits.

IMO I wouldn't be that worried about using 8 vs 32 bits.

> If I was
> to use an enum, of course I'd also want to have the variable this
> (correct) type. And I'd also like to avoid the packed attribute
> here. The above seemed to least bad option; I could be convinced
> to use 0/1 instead of false/true, though.

Yes please, 0/1 would be fine for me.

> 
> >> --- a/xen/include/xen/iommu.h
> >> +++ b/xen/include/xen/iommu.h
> >> @@ -53,8 +53,9 @@ static inline bool_t dfn_eq(dfn_t x, dfn
> >>  }
> >>  
> >>  extern bool_t iommu_enable, iommu_enabled;
> >> -extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
> >> +extern bool force_iommu, iommu_verbose, iommu_igfx;
> >>  extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
> >> +extern uint8_t iommu_quarantine;
> > 
> > Exporting this variable without the paired defines seems pointless,
> > how are external callers supposed to figure out the quarantine mode
> > without the IOMMU_quarantine_* defines?
> 
> Why pointless? Outside of the file knowing the IOMMU_quarantine_*
> 

Re: [Xen-devel] [PATCH] x86emul: correct LFS et al handling for 64-bit mode

2019-12-12 Thread Jan Beulich
On 12.12.2019 12:37, Andrew Cooper wrote:
> On 12/12/2019 10:11, Jan Beulich wrote:
>> On 11.12.2019 21:57, Andrew Cooper wrote:
>>> On 11/12/2019 09:28, Jan Beulich wrote:
 AMD and friends explicitly specify that 64-bit operands aren't possible
 for these insns. Nevertheless REX.W isn't fully ignored: It still
 cancels a possible operand size override (0x66). Intel otoh explicitly
 provides for 64-bit operands on the respective insn page of the SDM.

 Signed-off-by: Jan Beulich 
>>> It is definitely more than just these.  Near jumps have per-vendor
>>> behaviour on how long the instruction is, whereas far jump/calls are in
>>> the same category as these by the looks of things.
>> But you don't expect me to fold all of these into one patch, do
>> you?
> 
> short jmp certainly not, but far jmp/call is just two extra case
> statements in this new code block, no?

Not exactly (the other change would need to be in
x86_decode_onebyte() and depend on ModRM.reg), but yes, I can
do this. Yet then it would feel odd to not also deal with the
near counterparts at the same time. Which in turn would make
is desirable to also deal with near RET as well. At which
point we're about to also discuss CALL/JMP with displacement
operands and Jcc.

>> We have _some_ vendor dependent behavior already, and I'm
>> slowly adding to it. Our far call/jmp support is rather
>> incomplete in other ways anyway.
> 
> There is different truncation behaviour for %rip which needs altering,
> but that is a separate area of code.  Anything else?

protmode_load_seg() and MOVSXD already have vendor dependent
code, if that was your question. For things needing doing see
above plus LOOP, J[ER]CXZ, SYSENTER, and SYSEXIT as far as I'm
currently aware.

Jan

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

Re: [Xen-devel] [PATCH V3 1/2] x86/altp2m: Add hypercall to set a range of sve bits

2019-12-12 Thread Alexandru Stefan ISAILA


On 12.12.2019 13:26, George Dunlap wrote:
> On 12/12/19 9:37 AM, Alexandru Stefan ISAILA wrote:
>>
>>
>> On 06.12.2019 17:29, George Dunlap wrote:
>>> On 11/21/19 3:02 PM, Alexandru Stefan ISAILA wrote:
 By default the sve bits are not set.
 This patch adds a new hypercall, xc_altp2m_set_supress_ve_multi(),
 to set a range of sve bits.
 The core function, p2m_set_suppress_ve_multi(), does not brake in case
 of a error and it is doing a best effort for setting the bits in the
 given range. A check for continuation is made in order to have
 preemption on big ranges.

 Signed-off-by: Alexandru Isaila 
>>>
>>> There's something strangely deformed in your mail that makes it hard for
>>> me to apply the patches to my tree, and I'm not sure why.
>>>
>>> It seems the core mail is base64-encrypted; and that *inside* that
>>> base64 encryption is a bunch of Windows-style linefeeds.  The result is
>>> that when I try to download your series and apply it with git-am, I get
>>> loads of rejected hunks with "^M" at the end of them.
>>>
>>> Sometimes I've been able to work around this by going on patchew.org/Xen
>>> and getting an mbox from there; but it doesn't seem to have your series
>>> (perhaps because it doesn't have a cover letter).
>>>
>>> Looking at the headers, it seems this is coming from git itself.  Do you
>>> perhaps have "transferEncoding" set to "base64"?  If so, chance you
>>> could try setting it to 'auto', and setting 'assume8bitEncoding = true"?
>>
>> I didn't have anything set for transferEncoding in .gitconfig but I can set
>>   assume8bitEncoding = yes
>>   transferEncoding = 8bit
>>
>> for the future.
>>
>> Sorry for the inconvenience.
> 
> Well, I'm also sorry that I'm having trouble on my end.  :-)  You'd
> think that you doing "git send-email" and me doing "git am" would Just
> Work(tm), and it's frustrating that it doesn't.  *Hopefully* those
> changes will make it work; otherwise we'll have to figure out something
> else.
> 

We have to solve this somehow so on the next ver. please let me know if 
everything is ok.

Alex
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v4 0/2] Refactor super page shattering

2019-12-12 Thread Hongyan Xia
map_pages_to_xen and modify_xen_mappings use almost exactly the same
page shattering logic, and the code is mingled with other PTE
manipulations so it is less obvious that the intention is page
shattering. Factor out the functions to make them reusable and to make
the intention more obvious.

Of course, there is not much difference between the shattering logic of
each level, so we could further turn the per-level functions into a
single macro, although this is not that simple since we have per-level
functions and macros all over the place and there are slight differences
between levels. Keep it per-level for now.

tree:
https://xenbits.xen.org/git-http/people/hx242/xen.git
shatter-refactor_v4

---
Changes in v4:
- helper functions now return bool instead of a random value.
- rebase

Changes in v3:
- style and indentation fixes.

Changes in v2:
- rebase.
- improve asm code.
- avoid stale values when taking the lock.
- move allocation of PTE tables inside the shatter function.

Hongyan Xia (2):
  x86/mm: factor out the code for shattering an l3 PTE
  x86/mm: factor out the code for shattering an l2 PTE

 xen/arch/x86/mm.c | 194 +++---
 1 file changed, 98 insertions(+), 96 deletions(-)

-- 
2.17.1


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

[Xen-devel] [PATCH v4 1/2] x86/mm: factor out the code for shattering an l3 PTE

2019-12-12 Thread Hongyan Xia
map_pages_to_xen and modify_xen_mappings are performing almost exactly
the same operations when shattering an l3 PTE, the only difference
being whether we want to flush.

Signed-off-by: Hongyan Xia 

---
Changes in v4:
- use false/true instead of -1/0 to indicate failure/success.
- remove unnecessary cast.

Changes in v3:
- style and indentation changes.
- return -ENOMEM instead of -1.

Changes in v2:
- improve asm.
- re-read pl3e from memory when taking the lock.
- move the allocation of l2t inside the shatter function.
---
 xen/arch/x86/mm.c | 98 +++
 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 7d4dd80a85..8def4fb8d8 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5151,6 +5151,52 @@ l1_pgentry_t *virt_to_xen_l1e(unsigned long v)
  flush_area_local((const void *)v, f) : \
  flush_area_all((const void *)v, f))
 
+/* Shatter an l3 entry and populate l2. If virt is passed in, also do flush. */
+static bool shatter_l3e(l3_pgentry_t *pl3e, unsigned long virt, bool locking)
+{
+unsigned int i;
+l3_pgentry_t ol3e = *pl3e;
+l2_pgentry_t l2e = l2e_from_intpte(l3e_get_intpte(ol3e));
+l2_pgentry_t *l2t = alloc_xen_pagetable();
+
+if ( !l2t )
+return false;
+
+for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
+{
+l2e_write(l2t + i, l2e);
+l2e = l2e_from_intpte(
+  l2e_get_intpte(l2e) + (PAGE_SIZE << PAGETABLE_ORDER));
+}
+
+if ( locking )
+spin_lock(_pgdir_lock);
+if ( (l3e_get_flags(*pl3e) & _PAGE_PRESENT) &&
+ (l3e_get_flags(*pl3e) & _PAGE_PSE) )
+{
+l3e_write_atomic(pl3e,
+l3e_from_paddr(virt_to_maddr(l2t), __PAGE_HYPERVISOR));
+l2t = NULL;
+}
+if ( locking )
+spin_unlock(_pgdir_lock);
+
+if ( virt )
+{
+unsigned int flush_flags =
+FLUSH_TLB | FLUSH_ORDER(2 * PAGETABLE_ORDER);
+
+if ( l3e_get_flags(ol3e) & _PAGE_GLOBAL )
+flush_flags |= FLUSH_TLB_GLOBAL;
+flush_area(virt, flush_flags);
+}
+
+if ( l2t )
+free_xen_pagetable(l2t);
+
+return true;
+}
+
 int map_pages_to_xen(
 unsigned long virt,
 mfn_t mfn,
@@ -5244,9 +5290,6 @@ int map_pages_to_xen(
 if ( (l3e_get_flags(ol3e) & _PAGE_PRESENT) &&
  (l3e_get_flags(ol3e) & _PAGE_PSE) )
 {
-unsigned int flush_flags =
-FLUSH_TLB | FLUSH_ORDER(2 * PAGETABLE_ORDER);
-
 /* Skip this PTE if there is no change. */
 if ( ((l3e_get_pfn(ol3e) & ~(L2_PAGETABLE_ENTRIES *
  L1_PAGETABLE_ENTRIES - 1)) +
@@ -5267,33 +5310,9 @@ int map_pages_to_xen(
 continue;
 }
 
-pl2e = alloc_xen_pagetable();
-if ( pl2e == NULL )
+/* Pass virt to indicate we need to flush. */
+if ( !shatter_l3e(pl3e, virt, locking) )
 return -ENOMEM;
-
-for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
-l2e_write(pl2e + i,
-  l2e_from_pfn(l3e_get_pfn(ol3e) +
-   (i << PAGETABLE_ORDER),
-   l3e_get_flags(ol3e)));
-
-if ( l3e_get_flags(ol3e) & _PAGE_GLOBAL )
-flush_flags |= FLUSH_TLB_GLOBAL;
-
-if ( locking )
-spin_lock(_pgdir_lock);
-if ( (l3e_get_flags(*pl3e) & _PAGE_PRESENT) &&
- (l3e_get_flags(*pl3e) & _PAGE_PSE) )
-{
-l3e_write_atomic(pl3e, l3e_from_mfn(virt_to_mfn(pl2e),
-__PAGE_HYPERVISOR));
-pl2e = NULL;
-}
-if ( locking )
-spin_unlock(_pgdir_lock);
-flush_area(virt, flush_flags);
-if ( pl2e )
-free_xen_pagetable(pl2e);
 }
 
 pl2e = virt_to_xen_l2e(virt);
@@ -5578,27 +5597,8 @@ int modify_xen_mappings(unsigned long s, unsigned long 
e, unsigned int nf)
 }
 
 /* PAGE1GB: shatter the superpage and fall through. */
-pl2e = alloc_xen_pagetable();
-if ( !pl2e )
+if ( !shatter_l3e(pl3e, 0, locking) )
 return -ENOMEM;
-for ( i = 0; i < L2_PAGETABLE_ENTRIES; i++ )
-l2e_write(pl2e + i,
-  l2e_from_pfn(l3e_get_pfn(*pl3e) +
-   (i << PAGETABLE_ORDER),
-   l3e_get_flags(*pl3e)));
-if ( locking )
-spin_lock(_pgdir_lock);
-if ( (l3e_get_flags(*pl3e) & _PAGE_PRESENT) &&
- (l3e_get_flags(*pl3e) & _PAGE_PSE) )
-{
-l3e_write_atomic(pl3e, l3e_from_mfn(virt_to_mfn(pl2e),
- 

[Xen-devel] [PATCH v4 2/2] x86/mm: factor out the code for shattering an l2 PTE

2019-12-12 Thread Hongyan Xia
map_pages_to_xen and modify_xen_mappings are performing almost exactly
the same operations when shattering an l2 PTE, the only difference
being whether we want to flush.

Signed-off-by: Hongyan Xia 

---
Changes in v4:
- use false/true instead of -1/0 to indicate failure/success.
- remove unnecessary cast.

Changes in v3:
- style and indentation changes.
- return -ENOMEM instead of -1.

Changes in v2:
- improve asm.
- re-read pl2e from memory when taking the lock.
- move the allocation of l1t inside the shatter function.
---
 xen/arch/x86/mm.c | 96 ---
 1 file changed, 49 insertions(+), 47 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 8def4fb8d8..4daf0ff0f0 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5151,6 +5151,52 @@ l1_pgentry_t *virt_to_xen_l1e(unsigned long v)
  flush_area_local((const void *)v, f) : \
  flush_area_all((const void *)v, f))
 
+/* Shatter an l2 entry and populate l1. If virt is passed in, also do flush. */
+static bool shatter_l2e(l2_pgentry_t *pl2e, unsigned long virt, bool locking)
+{
+unsigned int i;
+l2_pgentry_t ol2e = *pl2e;
+l1_pgentry_t l1e = l1e_from_paddr(l2e_get_paddr(ol2e),
+  lNf_to_l1f(l2e_get_flags(ol2e)));
+l1_pgentry_t *l1t = alloc_xen_pagetable();
+
+if ( !l1t )
+return false;
+
+for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
+{
+l1e_write(l1t + i, l1e);
+l1e = l1e_from_intpte(l1e_get_intpte(l1e) + PAGE_SIZE);
+}
+
+if ( locking )
+spin_lock(_pgdir_lock);
+if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) &&
+ (l2e_get_flags(*pl2e) & _PAGE_PSE) )
+{
+l2e_write_atomic(pl2e,
+l2e_from_paddr(virt_to_maddr(l1t), __PAGE_HYPERVISOR));
+l1t = NULL;
+}
+if ( locking )
+spin_unlock(_pgdir_lock);
+
+if ( virt )
+{
+unsigned int flush_flags =
+FLUSH_TLB | FLUSH_ORDER(PAGETABLE_ORDER);
+
+if ( l2e_get_flags(ol2e) & _PAGE_GLOBAL )
+flush_flags |= FLUSH_TLB_GLOBAL;
+flush_area(virt, flush_flags);
+}
+
+if ( l1t )
+free_xen_pagetable(l1t);
+
+return true;
+}
+
 /* Shatter an l3 entry and populate l2. If virt is passed in, also do flush. */
 static bool shatter_l3e(l3_pgentry_t *pl3e, unsigned long virt, bool locking)
 {
@@ -5364,9 +5410,6 @@ int map_pages_to_xen(
 }
 else if ( l2e_get_flags(*pl2e) & _PAGE_PSE )
 {
-unsigned int flush_flags =
-FLUSH_TLB | FLUSH_ORDER(PAGETABLE_ORDER);
-
 /* Skip this PTE if there is no change. */
 if ( (((l2e_get_pfn(*pl2e) & ~(L1_PAGETABLE_ENTRIES - 1)) +
l1_table_offset(virt)) == mfn_x(mfn)) &&
@@ -5385,32 +5428,9 @@ int map_pages_to_xen(
 goto check_l3;
 }
 
-pl1e = alloc_xen_pagetable();
-if ( pl1e == NULL )
+/* Pass virt to indicate we need to flush. */
+if ( !shatter_l2e(pl2e, virt, locking) )
 return -ENOMEM;
-
-for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
-l1e_write([i],
-  l1e_from_pfn(l2e_get_pfn(*pl2e) + i,
-   lNf_to_l1f(l2e_get_flags(*pl2e;
-
-if ( l2e_get_flags(*pl2e) & _PAGE_GLOBAL )
-flush_flags |= FLUSH_TLB_GLOBAL;
-
-if ( locking )
-spin_lock(_pgdir_lock);
-if ( (l2e_get_flags(*pl2e) & _PAGE_PRESENT) &&
- (l2e_get_flags(*pl2e) & _PAGE_PSE) )
-{
-l2e_write_atomic(pl2e, l2e_from_mfn(virt_to_mfn(pl1e),
-__PAGE_HYPERVISOR));
-pl1e = NULL;
-}
-if ( locking )
-spin_unlock(_pgdir_lock);
-flush_area(virt, flush_flags);
-if ( pl1e )
-free_xen_pagetable(pl1e);
 }
 
 pl1e  = l2e_to_l1e(*pl2e) + l1_table_offset(virt);
@@ -5633,26 +5653,8 @@ int modify_xen_mappings(unsigned long s, unsigned long 
e, unsigned int nf)
 else
 {
 /* PSE: shatter the superpage and try again. */
-pl1e = alloc_xen_pagetable();
-if ( !pl1e )
+if ( !shatter_l2e(pl2e, 0, locking) )
 return -ENOMEM;
-for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ )
-l1e_write([i],
-  l1e_from_pfn(l2e_get_pfn(*pl2e) + i,
-   l2e_get_flags(*pl2e) & ~_PAGE_PSE));
-if ( locking )
-spin_lock(_pgdir_lock);
-  

[Xen-devel] [PATCH net] xen-netback: avoid race that can lead to NULL pointer dereference

2019-12-12 Thread Paul Durrant
Commit 2ac061ce97f4 ("xen/netback: cleanup init and deinit code")
introduced a problem. In function xenvif_disconnect_queue(), the value of
queue->rx_irq is zeroed *before* queue->task is stopped. Unfortunately that
task may call notify_remote_via_irq(queue->rx_irq) and calling that
function with a zero value results in a NULL pointer dereference in
evtchn_from_irq().

This patch simply re-orders things, stopping all tasks before zero-ing the
irq values, thereby avoiding the possibility of the race.

Signed-off-by: Paul Durrant 
---
Cc: Juergen Gross 
Cc: Jakub Kicinski 
Cc: Wei Liu 
Cc: "David S. Miller" 
---
 drivers/net/xen-netback/interface.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c 
b/drivers/net/xen-netback/interface.c
index 68dd7bb07ca6..f15ba3de6195 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -628,18 +628,6 @@ int xenvif_connect_ctrl(struct xenvif *vif, grant_ref_t 
ring_ref,
 
 static void xenvif_disconnect_queue(struct xenvif_queue *queue)
 {
-   if (queue->tx_irq) {
-   unbind_from_irqhandler(queue->tx_irq, queue);
-   if (queue->tx_irq == queue->rx_irq)
-   queue->rx_irq = 0;
-   queue->tx_irq = 0;
-   }
-
-   if (queue->rx_irq) {
-   unbind_from_irqhandler(queue->rx_irq, queue);
-   queue->rx_irq = 0;
-   }
-
if (queue->task) {
kthread_stop(queue->task);
queue->task = NULL;
@@ -655,6 +643,18 @@ static void xenvif_disconnect_queue(struct xenvif_queue 
*queue)
queue->napi.poll = NULL;
}
 
+   if (queue->tx_irq) {
+   unbind_from_irqhandler(queue->tx_irq, queue);
+   if (queue->tx_irq == queue->rx_irq)
+   queue->rx_irq = 0;
+   queue->tx_irq = 0;
+   }
+
+   if (queue->rx_irq) {
+   unbind_from_irqhandler(queue->rx_irq, queue);
+   queue->rx_irq = 0;
+   }
+
xenvif_unmap_frontend_data_rings(queue);
 }
 
-- 
2.20.1


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

Re: [Xen-devel] [PATCH] x86: store cr4 during suspend/resume

2019-12-12 Thread Andrew Cooper
On 12/12/2019 09:52, Jan Beulich wrote:
> On 11.12.2019 20:36, Andrew Cooper wrote:
>> On 10/12/2019 10:07, Jan Beulich wrote:
>>> On 10.12.2019 10:59, Andrew Cooper wrote:
 On 09/12/2019 18:06, Roger Pau Monne wrote:
> Currently cr4 is not cached before suspension, and mmu_cr4_features is
> used in order to restore the expected cr4 value. This is correct so
> far because the tasklet that executes the suspend/resume code is
> running in the idle vCPU context.
>
> In order to make the code less fragile, explicitly save the current
> cr4 value before suspension, so that it can be restored afterwards.
> This ensures that the cr4 value cached in the cpu_info doesn't get out
> of sync after resume from suspension.
>
> Suggested-by: Jan Beulich 
> Signed-off-by: Roger Pau Monné 
 Why?  There is nothing fragile here.  Suspend/resume is always in idle
 context and loads of other logic already depends on this.

 I've been slowly stripping out redundant saved state like this.
>>> Where it's clearly redundant, this is fine. But I don't think it's
>>> sufficiently clear here
>> There is a reason I made it explicitly crystal clear with c/s 87e7b4d5b
> Well, this makes clear we're in idle context, yes. But there's
> still a disconnect between this and the use of mmu_cr4_features
> (up to and including the somewhat misleading comment saying
> "mmu_cr4_features contains latest cr4 setting" without it really
> being clear what "latest" means, now that we run with varying
> CR4 values. Yes, write_ptbase() does use the variable when
> switching to idle, but the variable name lack any connection to
> this fact.

The name of the variable should probably be improved - I'm not a fan of
its current name.

Perhaps this seems more obvious to me than others because I did all the
work for XSA-293, but the commit message of c/s b2dd00574a4 is relevant:

> First of all, modify write_ptbase() to always use mmu_cr4_features for
> IDLE
> and HVM contexts.  mmu_cr4_features *is* the correct value to use, and
> makes
> the ASSERT() obviously redundant.

and the same applies to S3 path.

>
>>> , and going back to what was there before
>>> is imo generally less error prone than going to some fixed state.
>> It is demonstrably more error prone, which is why I'm slowly killing it.
>>
>> Stashing state wastes unnecessary space, and adds an error case where
>> state is either stashed incorrectly, or gets modified before restore,
>> and we'll blindly use.
> The waste of space is entirely secondary here, I think. A value
> getting modified before restore is no different from a value
> going out of sync with the variable we reload from. It's a blind
> use in either case.
>
>> Two examples of real bugs caused by this are c/s 0c30171cb and 4ee0ad72d
> I see your point for the former, but the latter seems to be unrelated.

Oh - quite right.  I wasn't paying quite enough attention when doing
archaeology.

>
>> Absolutely nothing remaining in suspend.c should be spilled.  It can all
>> be (re)caluclated from the same information source as the AP boot path,
>> and the result will be strictly smaller in RAM, and more robust.
> Robustness to me would imply using the same code for doing the
> calculations, not re-calculating from the same information source.
> This could be as simple as an idle_cr4() wrapper around the read
> of mmu_cr4_features for the case at hand (suitably used wherever
> applicable).
>
> Anyway - together with your subsequent mail I accept your objections.
> Once the code changes proposed there have gone in, I think it'll
> become more clear to readers that indeed state saving/restoring is to
> be the exception, not the rule (current code doesn't give this
> impression, I think).

It was all inherited from Linux, and is slowly being stripped out there
as well.

I'll try and do some more cleanup in some free time.

~Andrew

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

Re: [Xen-devel] [PATCH] x86emul: correct segment override decode for 64-bit mode

2019-12-12 Thread Andrew Cooper
On 12/12/2019 10:04, Jan Beulich wrote:
> On 11.12.2019 21:51, Andrew Cooper wrote:
>> On 11/12/2019 09:27, Jan Beulich wrote:
>>> The legacy / compatibility mode ES, CS, SS, and DS overrides are null
>>> prefixes in 64-bit mode, i.e. they in particular don't cancel an
>>> earlier FS or GS one.
>>>
>>> Signed-off-by: Jan Beulich 
>> null is a very overloaded term.  What you mean here is simply "ignored".
> The AMD PM has "Instead, they are treated as null prefixes." This is
> what I've taken to use here. I'm happy to take whatever other
> sensible wording you like better (including "ignored"). But I'd like
> you to explicitly clarify that you're not okay with me using a term
> from vendor documentation here.

"Ignored" is the more descriptive term, matches 2 different parts of the
APM, and most importantly, more obviously matches the code.

I can't even spot mention of this behaviour in the SDM.

>
>> In attempting to confirm/test this, I've found yet another curiosity
>> with instruction length calculations when reordering a rex prefix and
>> legacy prefix.  Objdump gets it wrong, but the instruction boundaries
>> according to singlestep are weird.
> Objdump getting it wrong is no surprise at all to me (which is one
> of the reasons why I prefer to use my own disassembler wherever
> possible). Yet without you spelling out what specific anomalies
> you've observed (or what weirdness there is with single stepping)
> I won't know whether I may want to make an attempt at fixing
> objdump. Nor can I see what this comment's implication is on the
> patch here, i.e. what changes you mean me to make.

The sequence in question is:

1048a1:    48       rex.W
1048a2:    2e 8b 32     mov    %cs:(%rdx),%esi

which was deliberately permuting the rex and %cs prefix to see what
happened.

The instruction boundary issue was a mistake in my code and with it
fixed, both Intel and AMD processors agree that the above 4 bytes is a
single instruction with 32bit operand size.  x86_emulate() also agrees,
which was the point of the test.

As I've resolved the instruction length ambiguity, Acked/Tested-by:
Andrew Cooper 

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

Re: [Xen-devel] [PATCH v2] xen/arm: remove physical timer offset

2019-12-12 Thread Julien Grall

Hi,

On 11/12/2019 20:00, Jeff Kubascik wrote:

On 12/5/2019 3:28 PM, Julien Grall wrote:

However, set_timer expects a signed 64 bit value in ns. The conversion of cval
(unsigned 64 bit) from ticks to ns is going to overflow this. I'm not sure what
would be the best way to work around this limitation. At the very least, I think
we should print a warning message.


A warning message in emulation is definitely not the right solution. If
a user asks something that is valid from the spec PoV then we should
implement it correctly. The more that I don't think boot_count store
what you expect (see above).

But we definitely can't allow the caller of ticks_to_ns() to pass a
negative value as argument because (cval - boot_count) may be over 2^63
for instance if the user requests a timer to be set in a million of year
(I didn't do the math!).


Assuming 100MHz timer frequency, the math works out to be about 5,850 years,
give or take. I'm assuming we don't need to worry about rollover conditions?


Do you mean for the timer exposed to the guest? If so, I think this is 
up to the guest to take care of the roll-over.


In Xen, we only have to be careful if this will roll-over our counter. 
Looking at the code, I don't think we are taking care of this.


From the Arm Arm mandates the timer to have roll-over time of not less 
than 40 years. So anything running Xen more than 40 years continuously 
may hit the problem.


The major hurdle to handle rollover is that the size of the counter is 
at least 56 bits on Armv8. But you have no way to detect the number of 
bits. In short, for roll-over, we may need to use TVAL rather than CVAL 
in Xen.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH-for-4.13] build: fix tools/configure in case only python3 exists

2019-12-12 Thread Wei Liu
On Wed, Dec 11, 2019 at 05:56:59PM +0100, Juergen Gross wrote:
> Calling ./configure with python3 being there but no python,
> tools/configure will fail. Fix that by defaulting to python and
> falling back to python3 or python2.
> 
> While at it fix the use of non portable "type -p" by replacing it by
> AC_PATH_PROG().
> 
> Signed-off-by: Juergen Gross 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v3 4/4] xen-blkback: support dynamic unbind/bind

2019-12-12 Thread Jürgen Groß

On 12.12.19 12:46, Roger Pau Monné wrote:

On Wed, Dec 11, 2019 at 03:29:56PM +, Paul Durrant wrote:

By simply re-attaching to shared rings during connect_ring() rather than
assuming they are freshly allocated (i.e assuming the counters are zero)
it is possible for vbd instances to be unbound and re-bound from and to
(respectively) a running guest.

This has been tested by running:

while true;
   do fio --name=randwrite --ioengine=libaio --iodepth=16 \
   --rw=randwrite --bs=4k --direct=1 --size=1G --verify=crc32;
   done

in a PV guest whilst running:

while true;
   do echo vbd-$DOMID-$VBD >unbind;
   echo unbound;
   sleep 5;
   echo vbd-$DOMID-$VBD >bind;
   echo bound;
   sleep 3;
   done

in dom0 from /sys/bus/xen-backend/drivers/vbd to continuously unbind and
re-bind its system disk image.

This is a highly useful feature for a backend module as it allows it to be
unloaded and re-loaded (i.e. updated) without requiring domUs to be halted.
This was also tested by running:

while true;
   do echo vbd-$DOMID-$VBD >unbind;
   echo unbound;
   sleep 5;
   rmmod xen-blkback;
   echo unloaded;
   sleep 1;
   modprobe xen-blkback;
   echo bound;
   cd $(pwd);
   sleep 3;
   done

in dom0 whilst running the same loop as above in the (single) PV guest.

Some (less stressful) testing has also been done using a Windows HVM guest
with the latest 9.0 PV drivers installed.

Signed-off-by: Paul Durrant 


Reviewed-by: Roger Pau Monné 

Thanks!

Juergen: I guess you will also pick this series and merge it from the
Xen tree instead of the block one?


Yes.


Juergen

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

Re: [Xen-devel] [PATCH v3 4/4] xen-blkback: support dynamic unbind/bind

2019-12-12 Thread Roger Pau Monné
On Wed, Dec 11, 2019 at 03:29:56PM +, Paul Durrant wrote:
> By simply re-attaching to shared rings during connect_ring() rather than
> assuming they are freshly allocated (i.e assuming the counters are zero)
> it is possible for vbd instances to be unbound and re-bound from and to
> (respectively) a running guest.
> 
> This has been tested by running:
> 
> while true;
>   do fio --name=randwrite --ioengine=libaio --iodepth=16 \
>   --rw=randwrite --bs=4k --direct=1 --size=1G --verify=crc32;
>   done
> 
> in a PV guest whilst running:
> 
> while true;
>   do echo vbd-$DOMID-$VBD >unbind;
>   echo unbound;
>   sleep 5;
>   echo vbd-$DOMID-$VBD >bind;
>   echo bound;
>   sleep 3;
>   done
> 
> in dom0 from /sys/bus/xen-backend/drivers/vbd to continuously unbind and
> re-bind its system disk image.
> 
> This is a highly useful feature for a backend module as it allows it to be
> unloaded and re-loaded (i.e. updated) without requiring domUs to be halted.
> This was also tested by running:
> 
> while true;
>   do echo vbd-$DOMID-$VBD >unbind;
>   echo unbound;
>   sleep 5;
>   rmmod xen-blkback;
>   echo unloaded;
>   sleep 1;
>   modprobe xen-blkback;
>   echo bound;
>   cd $(pwd);
>   sleep 3;
>   done
> 
> in dom0 whilst running the same loop as above in the (single) PV guest.
> 
> Some (less stressful) testing has also been done using a Windows HVM guest
> with the latest 9.0 PV drivers installed.
> 
> Signed-off-by: Paul Durrant 

Reviewed-by: Roger Pau Monné 

Thanks!

Juergen: I guess you will also pick this series and merge it from the
Xen tree instead of the block one?

Roger.

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

Re: [Xen-devel] [PATCH v7 2/3] xen/blkback: Squeeze page pools if a memory pressure is detected

2019-12-12 Thread Roger Pau Monné
Hello,

Please make sure you Cc me in blkback related patches.

On Wed, Dec 11, 2019 at 06:10:15PM +, SeongJae Park wrote:
> Each `blkif` has a free pages pool for the grant mapping.  The size of
> the pool starts from zero and be increased on demand while processing
^ is
> the I/O requests.  If current I/O requests handling is finished or 100
> milliseconds has passed since last I/O requests handling, it checks and
> shrinks the pool to not exceed the size limit, `max_buffer_pages`.
> 
> Therefore, host administrators can cause memory pressure in blkback by
> attaching a large number of block devices and inducing I/O.  Such
> problematic situations can be avoided by limiting the maximum number of
> devices that can be attached, but finding the optimal limit is not so
> easy.  Improper set of the limit can results in the memory pressure or a
  ^ s/the//
> resource underutilization.  This commit avoids such problematic
> situations by squeezing the pools (returns every free page in the pool
> to the system) for a while (users can set this duration via a module
> parameter) if a memory pressure is detected.
^ s/a//
> 
> Discussions
> ===
> 
> The `blkback`'s original shrinking mechanism returns only pages in the
> pool, which are not currently be used by `blkback`, to the system.  In

I think you can remove both comas in the above sentence.

> other words, the pages that are not mapped with granted pages.  Because
> this commit is changing only the shrink limit but still uses the same
> freeing mechanism it does not touch pages which are currently mapping
> grants.
> 
> Once a memory pressure is detected, this commit keeps the squeezing
   ^ s/a//
> limit for a user-specified time duration.  The duration should be
> neither too long nor too short.  If it is too long, the squeezing
> incurring overhead can reduce the I/O performance.  If it is too short,
> `blkback` will not free enough pages to reduce the memory pressure.
> This commit sets the value as `10 milliseconds` by default because it is
> a short time in terms of I/O while it is a long time in terms of memory
> operations.  Also, as the original shrinking mechanism works for at
> least every 100 milliseconds, this could be a somewhat reasonable
> choice.  I also tested other durations (refer to the below section for
> more details) and confirmed that 10 milliseconds is the one that works
> best with the test.  That said, the proper duration depends on actual
> configurations and workloads.  That's why this commit allows users to
> set the duration as a module parameter.
> 
> Memory Pressure Test
> 
> 
> To show how this commit fixes the memory pressure situation well, I
> configured a test environment on a xen-running virtualization system.
> On the `blkfront` running guest instances, I attach a large number of
> network-backed volume devices and induce I/O to those.  Meanwhile, I
> measure the number of pages that swapped in (pswpin) and out (pswpout)
> on the `blkback` running guest.  The test ran twice, once for the
> `blkback` before this commit and once for that after this commit.  As
> shown below, this commit has dramatically reduced the memory pressure:
> 
> pswpin  pswpout
> before  76,672  185,799
> after  2123,325
> 
> Optimal Aggressive Shrinking Duration
> -
> 
> To find a best squeezing duration, I repeated the test with three
> different durations (1ms, 10ms, and 100ms).  The results are as below:
> 
> durationpswpin  pswpout
> 1   852 6,424
> 10  212 3,325
> 100 203 3,340
> 
> As expected, the memory pressure has decreased as the duration is
> increased, but the reduction stopped from the `10ms`.  Based on this
> results, I chose the default duration as 10ms.
> 
> Performance Overhead Test
> =
> 
> This commit could incur I/O performance degradation under severe memory
> pressure because the squeezing will require more page allocations per
> I/O.  To show the overhead, I artificially made a worst-case squeezing
> situation and measured the I/O performance of a `blkfront` running
> guest.
> 
> For the artificial squeezing, I set the `blkback.max_buffer_pages` using
> the `/sys/module/xen_blkback/parameters/max_buffer_pages` file.  In this
> test, I set the value to `1024` and `0`.  The `1024` is the default
> value.  Setting the value as `0` is same to a situation doing the
> squeezing always (worst-case).
> 
> For the I/O performance measurement, I run a simple `dd` command 5 times
> as below and collect the 'MB/s' results.
> 
> $ for i in {1..5}; do dd if=/dev/zero of=file \
>  bs=4k count=$((256*512)); sync; done

I think it would be better if you could skip the filesystem overhead
by writing directly to a block 

Re: [Xen-devel] [PATCH] x86emul: correct LFS et al handling for 64-bit mode

2019-12-12 Thread Andrew Cooper
On 12/12/2019 10:11, Jan Beulich wrote:
> On 11.12.2019 21:57, Andrew Cooper wrote:
>> On 11/12/2019 09:28, Jan Beulich wrote:
>>> AMD and friends explicitly specify that 64-bit operands aren't possible
>>> for these insns. Nevertheless REX.W isn't fully ignored: It still
>>> cancels a possible operand size override (0x66). Intel otoh explicitly
>>> provides for 64-bit operands on the respective insn page of the SDM.
>>>
>>> Signed-off-by: Jan Beulich 
>> It is definitely more than just these.  Near jumps have per-vendor
>> behaviour on how long the instruction is, whereas far jump/calls are in
>> the same category as these by the looks of things.
> But you don't expect me to fold all of these into one patch, do
> you?

short jmp certainly not, but far jmp/call is just two extra case
statements in this new code block, no?

> We have _some_ vendor dependent behavior already, and I'm
> slowly adding to it. Our far call/jmp support is rather
> incomplete in other ways anyway.

There is different truncation behaviour for %rip which needs altering,
but that is a separate area of code.  Anything else?

~Andrew

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

  1   2   >