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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-libvirt-xsm   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 133580
 test-amd64-amd64-xl-pvshim  20 guest-start/debian.repeat fail REGR. vs. 133580
 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail REGR. vs. 133580

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 133580
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 133580
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-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-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-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   

Re: [Xen-devel] Latest development (master) Xen fails to boot on HP ProLiant DL20 GEN10

[Roman Shaposhnik]

Sorry -- was traveling last week, but I'm still very curious to get to
the bottom of this:

On Tue, Oct 1, 2019 at 1:25 AM Jan Beulich  wrote:
>
> On 01.10.2019 00:38, Roman Shaposhnik wrote:
> > Btw, forgot to attach the patch with maxcpus=2 -- interestingly enough
> > Xen seems to hang much further down than before (basically after
> > attempting to build out Dom0)
>
> All 3 logs contain
>
> (XEN) TSC_DEADLINE disabled due to Errata; please update microcode to version 
> 0x52 (or later)

Ok. This makes some sense. Btw, in a situation like this, do we expect
Xen to cope with a broken microcode or we always expect microcode to
be updated?

> Please load up-to-date microcode on the system and,

I couldn't find any kind of HP guide on how to do this on this box.
Any chance someone here may have a pointer for me?

> preferably with Andrew's suggestions also applied, re-post the logs. I notice 
> that
> even logs 1 and 2 have "Brought up 4 CPUs", other than you've
> indicated in your initial report. This suggests something's broken
> _after_ bringup of secondary CPUs, not while bringing them up. Log
> 3 effectively seems to confirm this.
>
> Seeing that "max_cstate=1" did help, as another next step could you
> try whether "mwait-idle=0" makes enough of a difference (it'll
> likely make a difference initially, as it makes the system
> effectively stay in a "max_cstate=1"-like mode until Dom0 has booted
> up far enough; the question this is going to be whether a hang still
> occurs one Dom0 has uploaded C-state data)?

Ok. Will do all these experiments tomorrow.

My plan is to use latest Xen master + build it with extra debug info
as was suggested earlier in this thread.

Thanks,
Roman.

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

[Xen-devel] [linux-4.4 test] 142401: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-pvshim   18 guest-localmigrate/x10   fail REGR. vs. 139698

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-raw7 xen-boot fail in 142381 pass in 142401
 test-armhf-armhf-xl  16 guest-start/debian.repeat  fail pass in 142381

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-pvshim12 guest-start  fail   never pass
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install 
fail never pass
 test-amd64-amd64-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-arm64-arm64-xl-credit1   7 xen-boot fail   never pass
 test-arm64-arm64-xl-xsm   7 xen-boot fail   never pass
 test-arm64-arm64-xl-credit2   7 xen-boot fail   never pass
 test-arm64-arm64-libvirt-xsm  7 xen-boot fail   never pass
 test-arm64-arm64-xl-seattle   7 xen-boot fail   never pass
 test-arm64-arm64-xl   7 xen-boot fail   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-amd64-xl-pvhv2-intel 12 guest-start fail 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-thunderx  7 xen-boot 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-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-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail 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-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-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-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop  fail never pass
 test-arm64-arm64-examine  8 reboot   fail   never pass
 test-amd64-amd64-xl-qemut-ws16-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-amd64-xl-qemuu-ws16-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-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-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass

version targeted for testing:
 linux5164f0c3740d357ba460b44222bedfa2475ca794
baseline version:
 linux

Re: [Xen-devel] [PATCH for-4.13 v2 0/3] fixes for make_[memory/cpu]_node

[Jürgen Groß]

On 08.10.19 03:14, Stefano Stabellini wrote:

Hi all,

This is a small collection of fixes for make_memory_node and
make_cpus_node for 4.13.

Cheers,

Stefano


Stefano Stabellini (3):
   xen/arm: fix buf size in make_cpus_node
   xen/arm: make_memory_node return error on nr_banks == 0
   xen/arm: fix duplicate memory node in DT

  xen/arch/arm/domain_build.c | 22 +++---
  1 file changed, 15 insertions(+), 7 deletions(-)



For the series:

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] [PATCH 01/24] golang/xenlight: fix calls to libxl_domain_unpause/pause

[Jürgen Groß]

On 07.10.19 18:39, George Dunlap wrote:

On 10/7/19 4:12 PM, Nick Rosbrook wrote:

From: Nick Rosbrook 

These functions require a third argument of type const *libxl_asyncop_how.

Pass nil to fix compilation errors. This will have the effect of
performing these operations synchronously.

Signed-off-by: Nick Rosbrook 


Reviewed-by: George Dunlap 

Juergen, this is actually a bug fix (these lines didn't get updated when
the API changed), so I'm going to check this in later this week if you
don't object.


Release-acked-by: Juergen Gross 


Juergen

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

[Xen-devel] [linux-next test] 142391: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-i386-examine   8 reboot   fail  like 142258
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail  like 142258
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail  like 142258
 test-amd64-i386-libvirt   7 xen-boot fail  like 142258
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot  fail like 142258
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-bootfail like 142258
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot   fail like 142258
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail like 
142258
 test-amd64-i386-xl-shadow 7 xen-boot fail  like 142258
 test-amd64-i386-xl-raw7 xen-boot fail  like 142258
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot   fail like 142258
 test-amd64-i386-libvirt-xsm   7 xen-boot fail  like 142258
 test-amd64-i386-xl7 xen-boot fail  like 142258
 test-amd64-i386-xl-xsm7 xen-boot fail  like 142258
 test-amd64-i386-freebsd10-i386  7 xen-bootfail like 142258
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail like 
142258
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-bootfail like 142258
 test-amd64-i386-pair 10 xen-boot/src_hostfail  like 142258
 test-amd64-i386-pair 11 xen-boot/dst_hostfail  like 142258
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot  fail like 142258
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow  7 xen-boot   fail like 142258
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot   fail like 142258
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm  7 xen-boot fail like 142258
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot   fail like 142258
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot  fail like 142258
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot   fail like 142258
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot  fail like 142258
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot   fail like 142258
 test-amd64-i386-xl-pvshim 7 xen-boot fail  like 142258
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot   fail like 142258
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot   fail like 142258
 test-amd64-i386-freebsd10-amd64  7 xen-boot   fail like 142258
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot   fail like 142258
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 142258
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 142258
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 142258
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 142258
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 142258
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 142258
 test-amd64-amd64-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-amd64-amd64-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-libvirt-xsm 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 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-xl  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  14 saverestore-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-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-libvirt-vhd 12 migrate-support-checkfail   never 

[Xen-devel] [PATCH v2 1/3] xen/arm: fix buf size in make_cpus_node

[Stefano Stabellini]

The size of buf is calculated wrongly: the number is 64bit, not 32bit.
Also the number is printed as a hexadecimal number, so we need 8 bytes
for 32bit, not 10 bytes.

As a result, it should be sizeof("cpu@") + 16 bytes for a 64-bit number
+ 1 byte for \0. Total = 21.

Fixes: fafd682c3e (xen/arm: Create a fake cpus node in dom0 device tree)
Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- patch added
---
 xen/arch/arm/domain_build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 921b054520..60923a7051 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -788,8 +788,8 @@ static int __init make_cpus_node(const struct domain *d, 
void *fdt)
 unsigned int cpu;
 const void *compatible = NULL;
 u32 len;
-/* Placeholder for cpu@ + a 32-bit number + \0 */
-char buf[15];
+/* Placeholder for cpu@ + a 64-bit number + \0 */
+char buf[21];
 u32 clock_frequency;
 bool clock_valid;
 uint64_t mpidr_aff;
-- 
2.17.1


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

[Xen-devel] [PATCH v2 3/3] xen/arm: fix duplicate memory node in DT

[Stefano Stabellini]

When reserved-memory regions are present in the host device tree, dom0
is started with multiple memory nodes. Each memory node should have a
unique name, but today they are all called "memory" leading to Linux
printing the following warning at boot:

  OF: Duplicate name in base, renamed to "memory#1"

This patch fixes the problem by appending a "@" to the
name, as per the Device Tree specification, where  matches
the base of address of the first region.

Fixes: 248faa637d2 (xen/arm: add reserved-memory regions to the dom0 memory 
node)
Reported-by: Oleksandr Tyshchenko 
Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- fix buf size calculation: the number is 64bit and printed as
  hexadecimal
- move check on nr_banks to a separate patch
---
 xen/arch/arm/domain_build.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index ea01aada0b..3de4dafaed 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -646,6 +646,8 @@ static int __init make_memory_node(const struct domain *d,
 int res, i;
 int reg_size = addrcells + sizecells;
 int nr_cells = reg_size * mem->nr_banks;
+/* Placeholder for memory@ + a 64-bit number + \0 */
+char buf[24];
 __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
 __be32 *cells;
 
@@ -657,7 +659,8 @@ static int __init make_memory_node(const struct domain *d,
reg_size, nr_cells);
 
 /* ePAPR 3.4 */
-res = fdt_begin_node(fdt, "memory");
+snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
+res = fdt_begin_node(fdt, buf);
 if ( res )
 return res;
 
-- 
2.17.1


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

[Xen-devel] [PATCH v2 2/3] xen/arm: make_memory_node return error on nr_banks == 0

[Stefano Stabellini]

Call make_memory_node for reserved_memory only if we actually have any
reserved_memory regions to handle.

Add a check in make_memory_node to return an error if it has been called
with no memory banks as argument.

Fixes: 248faa637d2 (xen/arm: add reserved-memory regions to the dom0 memory 
node)
Signed-off-by: Stefano Stabellini 
---
Changes in v2:
- patch added
---
 xen/arch/arm/domain_build.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 60923a7051..ea01aada0b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -650,6 +650,8 @@ static int __init make_memory_node(const struct domain *d,
 __be32 *cells;
 
 BUG_ON(nr_cells >= ARRAY_SIZE(reg));
+if ( mem->nr_banks == 0 )
+return -ENOENT;
 
 dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
reg_size, nr_cells);
@@ -1540,10 +1542,13 @@ static int __init handle_node(struct domain *d, struct 
kernel_info *kinfo,
  * Create a second memory node to store the ranges covering
  * reserved-memory regions.
  */
-res = make_memory_node(d, kinfo->fdt, addrcells, sizecells,
-   _mem);
-if ( res )
-return res;
+if ( bootinfo.reserved_mem.nr_banks > 0 )
+{
+res = make_memory_node(d, kinfo->fdt, addrcells, sizecells,
+   _mem);
+if ( res )
+return res;
+}
 }
 
 res = fdt_end_node(kinfo->fdt);
-- 
2.17.1


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

[Xen-devel] [PATCH for-4.13 v2 0/3] fixes for make_[memory/cpu]_node

[Stefano Stabellini]

Hi all,

This is a small collection of fixes for make_memory_node and
make_cpus_node for 4.13.

Cheers,

Stefano


Stefano Stabellini (3):
  xen/arm: fix buf size in make_cpus_node
  xen/arm: make_memory_node return error on nr_banks == 0
  xen/arm: fix duplicate memory node in DT

 xen/arch/arm/domain_build.c | 22 +++---
 1 file changed, 15 insertions(+), 7 deletions(-)

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

[Xen-devel] On unions usage, specifically arch.{hvm,pv}

[Marek Marczykowski-Górecki]

Hi all,

To be honest, I think unions are very scary from security point of view.
It's quite easy to use a field that in given context have very different
meaning and easily results in security issue. In the most cases,
compiler can't help you here. And seeing "IOMMU: add missing HVM check"
patch recently, fixing a but that was there for a year, I think my point
is valid.
There are multiple unions in the Xen code base, but I'd start with the
one in x86's struct arch_domain: {pv,hvm}. In some cases (like the above
_patched_ one), it is obvious that using arch.pv or arch.hvm in given
context is valid. But in some it is very much not obvious, like usage of
d->arch.hvm.dirty_vram in _sh_propagate()
(xen/arch/x86/mm/shadow/multi.c) - at least one caller seems to deal
also with PV vcpus
(sh_page_fault->shadow_get_and_create_l1e->l2e_propagate_from_guest).
Maybe I'm missing something, or maybe I've just found a bug, I don't
know, that's my point. And this is after casual grep for arch.hvm and
picking random file (took like 1 minute).

I propose to implement some measures to make similar bugs less likely.
Some ideas:
1. Add asserts for guest type, if the check isn't visible in obvious
place, near arch.pv / arch.hvm usage.

or maybe even better:

2. Add wrappers (inline, #define, whatever) that perform the check
before accessing those fields. And forbid accessing those (and maybe
later others too?) unions directly, so it would be trivial to verify.
There could be multiple wrappers for most commons code patterns. For
example one for combined is_hvm_domain(d) &&
some-check-on-arch.hvm-field. Or another one just adding an ASSERT() /
BUG_ON().

Ideally such check should be part of a release build (IMO it's better to
crash early with clear message, instead of crashing later in mysterious
way or having privilege escalation bug - if that's the alternative). But
having those checks just in debug build would be an improvement already.

Thoughts?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?


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

Re: [Xen-devel] [[PATCH for-4.13]] xen/arm: mm: Allow generic xen page-tables helpers to be called early

[Stefano Stabellini]

On Mon, 7 Oct 2019, Julien Grall wrote:
> Hi,
> 
> On 03/10/2019 02:02, Stefano Stabellini wrote:
> > On Fri, 20 Sep 2019, Julien Grall wrote:
> >> That's not correct. alloc_boot_pages() is actually here to allow dynamic
> >> allocation before the memory subsystem (and therefore the runtime 
> >> allocator)
> >> is initialized.
> > 
> > Let me change the question then: is the system_state ==
> > SYS_STATE_early_boot check strictly necessary? It looks like it is not:
> > the patch would work even if it was just:
> 
> I had a few thoughts about it. On Arm32, this only really works for 
> 32-bits machine address (it can go up to 40-bits). I haven't really 
> fully investigated what could go wrong, but it would be best to keep it 
> only for early boot.
> 
> Also, I don't really want to rely on this "workaround" after boot. Maybe 
> we would want to keep them unmapped in the future.

Yes, no problems, we agree on that. I am not asking in regards to the
check system_state == SYS_STATE_early_boot with the goal of asking you
to get rid of it. I am fine with keeping the check. (Maybe we want to add
an `unlikely()' around the check.)

I am trying to understand whether the code actually relies on
system_state == SYS_STATE_early_boot, and, if so, why. The goal is to
make sure that if there are some limitations that they are documented,
or just to double-check that there are no limitations.

In regards to your comment about only working for 32-bit addresses on
Arm32, you have a point. At least we should be careful with the mfn to
vaddr conversion because mfn_to_maddr returns a paddr_t which is 64-bit
and vaddr_t is 32-bit. I imagine that theoretically, even with
system_state == SYS_STATE_early_boot, it could get truncated with the
wrong combination of mfn and phys_offset.

If nothing else, maybe we should add a truncation check for safety?
Something like the following but that ideally would be applicable to
arm64 too without having to add an #ifdef:

paddr_t pa = mfn_to_maddr(mfn) - phys_offset;

if ( pa < _end && is_kernel((vaddr_t)pa) )
return (lpae_t *)(vaddr_t)pa;

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

Re: [Xen-devel] [PATCH for-4.13] xen/arm: fix duplicate memory node in DT

[Stefano Stabellini]

On Mon, 7 Oct 2019, Julien Grall wrote:
> Hi,
> 
> On 07/10/2019 22:30, Stefano Stabellini wrote:
> > On Mon, 7 Oct 2019, Julien Grall wrote:
> >> On 05/10/2019 00:09, Stefano Stabellini wrote:
> >>> When reserved-memory regions are present in the host device tree, dom0
> >>> is started with multiple memory nodes. Each memory node should have a
> >>> unique name, but today they are all called "memory" leading to Linux
> >>> printing the following warning at boot:
> >>>
> >>> OF: Duplicate name in base, renamed to "memory#1"
> >>>
> >>> This patch fixes the problem by appending a "@" to the
> >>> name, as per the Device Tree specification, where  matches
> >>> the base of address of the first region.
> >>>
> >>> Reported-by: Oleksandr Tyshchenko 
> >>> Signed-off-by: Stefano Stabellini 
> >>>
> >>> ---
> >>>
> >>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> >>> index 921b054520..a4c07db383 100644
> >>> --- a/xen/arch/arm/domain_build.c
> >>> +++ b/xen/arch/arm/domain_build.c
> >>> @@ -646,16 +646,22 @@ static int __init make_memory_node(const struct 
> >>> domain
> >>> *d,
> >>>int res, i;
> >>>int reg_size = addrcells + sizecells;
> >>>int nr_cells = reg_size * mem->nr_banks;
> >>> +/* Placeholder for memory@ + a 32-bit number + \0 */
> >>> +char buf[18];
> >>>__be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells 
> >>> */];
> >>>__be32 *cells;
> >>>  BUG_ON(nr_cells >= ARRAY_SIZE(reg));
> >>> +/* Nothing to do */
> >>
> >> This a departure from the current solution where a node will be created 
> >> with
> >> no "reg" property. I think this change of behavior should at least be
> >> described in the commit message if not implemented in a separate patch. 
> >> But...
> >>
> >>> +if ( mem->nr_banks == 0 )
> >>> +return 0;
> >>
> >> ... I don't think we want to ignore it. The caller most likely messed up 
> >> the
> >> banks and we should instead report an error.
> > 
> > I admit it wasn't my intention to change the current behavior. As I was
> > looking through the code I noticed that we call make_memory_node for
> > both normal memory and reserved_memory. Of course, reserved_memory could
> > have no banks. So I thought it would be good to check whether there are
> > any banks before continuing because now we are going to access
> > mem->bank[0].start, which would be a mistake if there are no banks.
> 
> Ok, so this not theoritical bug as I first thought but a real bug on 
> platform where DT does not have reserved-regions node.
> 
> In this case, this should be in a separate patch as this is now 2 
> different bugs solved in one patch.

OK


> > In regards to your comment about returning error, we could return ENOENT,
> > however we would also have to handle ENOENT especially at the caller
> > side (handle_node). Or we would have to add a check if ( mem->nr_banks >
> > 0) to avoid calling make_memory_node when nr_banks is zero.
> 
> I would much prefer if we check mem->nr_banks > 0 for reserved-regions 
> before hand.

All right


> Both will need a "Fixes:" to keep track of the original patch.

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

Re: [Xen-devel] [PATCH for-4.13] xen/arm: fix duplicate memory node in DT

[Julien Grall]

Hi,

On 07/10/2019 22:30, Stefano Stabellini wrote:
> On Mon, 7 Oct 2019, Julien Grall wrote:
>> On 05/10/2019 00:09, Stefano Stabellini wrote:
>>> When reserved-memory regions are present in the host device tree, dom0
>>> is started with multiple memory nodes. Each memory node should have a
>>> unique name, but today they are all called "memory" leading to Linux
>>> printing the following warning at boot:
>>>
>>> OF: Duplicate name in base, renamed to "memory#1"
>>>
>>> This patch fixes the problem by appending a "@" to the
>>> name, as per the Device Tree specification, where  matches
>>> the base of address of the first region.
>>>
>>> Reported-by: Oleksandr Tyshchenko 
>>> Signed-off-by: Stefano Stabellini 
>>>
>>> ---
>>>
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index 921b054520..a4c07db383 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -646,16 +646,22 @@ static int __init make_memory_node(const struct domain
>>> *d,
>>>int res, i;
>>>int reg_size = addrcells + sizecells;
>>>int nr_cells = reg_size * mem->nr_banks;
>>> +/* Placeholder for memory@ + a 32-bit number + \0 */
>>> +char buf[18];
>>>__be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
>>>__be32 *cells;
>>>  BUG_ON(nr_cells >= ARRAY_SIZE(reg));
>>> +/* Nothing to do */
>>
>> This a departure from the current solution where a node will be created with
>> no "reg" property. I think this change of behavior should at least be
>> described in the commit message if not implemented in a separate patch. 
>> But...
>>
>>> +if ( mem->nr_banks == 0 )
>>> +return 0;
>>
>> ... I don't think we want to ignore it. The caller most likely messed up the
>> banks and we should instead report an error.
> 
> I admit it wasn't my intention to change the current behavior. As I was
> looking through the code I noticed that we call make_memory_node for
> both normal memory and reserved_memory. Of course, reserved_memory could
> have no banks. So I thought it would be good to check whether there are
> any banks before continuing because now we are going to access
> mem->bank[0].start, which would be a mistake if there are no banks.

Ok, so this not theoritical bug as I first thought but a real bug on 
platform where DT does not have reserved-regions node.

In this case, this should be in a separate patch as this is now 2 
different bugs solved in one patch.

> 
> In regards to your comment about returning error, we could return ENOENT,
> however we would also have to handle ENOENT especially at the caller
> side (handle_node). Or we would have to add a check if ( mem->nr_banks >
> 0) to avoid calling make_memory_node when nr_banks is zero.

I would much prefer if we check mem->nr_banks > 0 for reserved-regions 
before hand.

Both will need a "Fixes:" to keep track of the original patch.

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] xen/arm: fix duplicate memory node in DT

[Stefano Stabellini]

On Mon, 7 Oct 2019, Jürgen Groß wrote:
> On 05.10.19 01:09, Stefano Stabellini wrote:
> > When reserved-memory regions are present in the host device tree, dom0
> > is started with multiple memory nodes. Each memory node should have a
> > unique name, but today they are all called "memory" leading to Linux
> > printing the following warning at boot:
> > 
> >OF: Duplicate name in base, renamed to "memory#1"
> > 
> > This patch fixes the problem by appending a "@" to the
> > name, as per the Device Tree specification, where  matches
> > the base of address of the first region.
> > 
> > Reported-by: Oleksandr Tyshchenko 
> > Signed-off-by: Stefano Stabellini 
> > 
> > ---
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 921b054520..a4c07db383 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -646,16 +646,22 @@ static int __init make_memory_node(const struct domain
> > *d,
> >   int res, i;
> >   int reg_size = addrcells + sizecells;
> >   int nr_cells = reg_size * mem->nr_banks;
> > +/* Placeholder for memory@ + a 32-bit number + \0 */
> > +char buf[18];
> 
> You are using PRIx64 for printing the number, so I guess you should
> enlarge buf by 8 bytes and adjust the comment (s/32/64/).

Well spotted! In fact, there is a similar error in make_cpus_node. I'll
fix that one too in a separate patch.


> >   __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
> >   __be32 *cells;
> > BUG_ON(nr_cells >= ARRAY_SIZE(reg));
> > +/* Nothing to do */
> > +if ( mem->nr_banks == 0 )
> > +return 0;
> > dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> >  reg_size, nr_cells);
> > /* ePAPR 3.4 */
> > -res = fdt_begin_node(fdt, "memory");
> > +snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
> > +res = fdt_begin_node(fdt, buf);
> >   if ( res )
> >   return res;
> 
> 
> Juergen
> ___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH for-4.13] xen/arm: fix duplicate memory node in DT

[Stefano Stabellini]

On Mon, 7 Oct 2019, Julien Grall wrote:
> On 05/10/2019 00:09, Stefano Stabellini wrote:
> > When reserved-memory regions are present in the host device tree, dom0
> > is started with multiple memory nodes. Each memory node should have a
> > unique name, but today they are all called "memory" leading to Linux
> > printing the following warning at boot:
> > 
> >OF: Duplicate name in base, renamed to "memory#1"
> > 
> > This patch fixes the problem by appending a "@" to the
> > name, as per the Device Tree specification, where  matches
> > the base of address of the first region.
> > 
> > Reported-by: Oleksandr Tyshchenko 
> > Signed-off-by: Stefano Stabellini 
> > 
> > ---
> > 
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 921b054520..a4c07db383 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -646,16 +646,22 @@ static int __init make_memory_node(const struct domain
> > *d,
> >   int res, i;
> >   int reg_size = addrcells + sizecells;
> >   int nr_cells = reg_size * mem->nr_banks;
> > +/* Placeholder for memory@ + a 32-bit number + \0 */
> > +char buf[18];
> >   __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
> >   __be32 *cells;
> > BUG_ON(nr_cells >= ARRAY_SIZE(reg));
> > +/* Nothing to do */
> 
> This a departure from the current solution where a node will be created with
> no "reg" property. I think this change of behavior should at least be
> described in the commit message if not implemented in a separate patch. But...
> 
> > +if ( mem->nr_banks == 0 )
> > +return 0;
> 
> ... I don't think we want to ignore it. The caller most likely messed up the
> banks and we should instead report an error.

I admit it wasn't my intention to change the current behavior. As I was
looking through the code I noticed that we call make_memory_node for
both normal memory and reserved_memory. Of course, reserved_memory could
have no banks. So I thought it would be good to check whether there are
any banks before continuing because now we are going to access
mem->bank[0].start, which would be a mistake if there are no banks.

In regards to your comment about returning error, we could return ENOENT,
however we would also have to handle ENOENT especially at the caller
side (handle_node). Or we would have to add a check if ( mem->nr_banks >
0) to avoid calling make_memory_node when nr_banks is zero.


> > dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",
> >  reg_size, nr_cells);
> > /* ePAPR 3.4 */
> > -res = fdt_begin_node(fdt, "memory");
> > +snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
> > +res = fdt_begin_node(fdt, buf);
> >   if ( res )
> >   return res;
> >   
> 
> Cheers,
> 
> -- 
> Julien Grall
> 

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

Re: [Xen-devel] [PATCH 2/2] xen/arm: domain_build: Don't expose IOMMU specific properties to the guest

[Julien Grall]

Hi,

On 03/10/2019 13:18, Oleksandr wrote:
> 
> On 01.10.19 22:07, Julien Grall wrote:
>> On 10/1/19 5:07 PM, Oleksandr wrote:
>>>
>>> On 01.10.19 18:36, Julien Grall wrote:
 On 01/10/2019 16:25, Oleksandr wrote:
>
> On 01.10.19 18:04, Julien Grall wrote:
>>> > 1. Giving the IOMMU to Dom0 is a bad idea.
>>
>> Please to try expand your thoughts in the same e-mail when you say 
>> "this is a bad idea".
> 
> Well, this was a conclusion I had got from the discussion [1]. Sorry for 
> not being clear here.
> 
> 
>>
>> But, this is clearly what happen in current Xen setup if the driver is 
>> not enabled. What I want to avoid is exposing an half complete 
>> bindings to the guest (you don't know how it will behave).
>>
>> So we either remove all the properties and node related to the IOMMUs 
>> or nothing.
> I think, I got it. Our target is *not* to add a way for Dom0 to use 
> IOMMU, but to be consistent in removing IOMMU node/master device 
> properties. Now, we remove the IOMMU node if Xen identifies it (the 
> IOMMU driver is present in Xen),
> so looks like we have to remove master device properties only if this 
> master device is behind the IOMMU which node is removed. This, I hope, 
> will avoid exposing an half complete bindings to guest. Am I right?
> 
> 
> [1] 
> https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00858.html
> 
> 
> --
> 
> If you happy with that logic I will craft a proper patch.

The logic looks alright to me. One comment below, I will look at the 
rest once this is formally sent.

> 
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 67021d9..6d45e55 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -480,10 +480,26 @@ static int __init write_properties(struct domain 
> *d, struct kernel_info *kinfo,
>   const struct dt_property *prop, *status = NULL;
>   int res = 0;
>   int had_dom0_bootargs = 0;
> +    struct dt_device_node *iommu_node;
> 
>   if ( kinfo->cmdline && kinfo->cmdline[0] )
>   bootargs = >cmdline[0];
> 
> +    /*
> + * If we skip the IOMMU device when creating DT for Dom0 (even if

I would prefer if we use "hwdom" over "Dom0". They are both the same on 
Arm, but this may change in the future (we may actually drop Dom0 ;)).

> + * the IOMMU device is not used by Xen), we should also skip the IOMMU
> + * specific properties of the master device behind it in order to 
> avoid
> + * exposing an half complete IOMMU bindings to Dom0.
> + * Use "iommu_node" as an indicator of the master device which 
> properties
> + * should be skipped.
> + */
> +    iommu_node = dt_parse_phandle(node, "iommus", 0);
> +    if ( iommu_node )
> +    {
> +    if ( device_get_class(iommu_node) != DEVICE_IOMMU )
> +    iommu_node = NULL;
> +    }
> +
>   dt_for_each_property_node (node, prop)
>   {
>   const void *prop_data = prop->value;
> @@ -540,6 +556,19 @@ static int __init write_properties(struct domain 
> *d, struct kernel_info *kinfo,
>   continue;
>   }
> 
> +    if ( iommu_node )
> +    {
> +    /* Don't expose IOMMU specific properties to Dom0 */
> +    if ( dt_property_name_is_equal(prop, "iommus") )
> +    continue;
> +
> +    if ( dt_property_name_is_equal(prop, "iommu-map") )
> +    continue;
> +
> +    if ( dt_property_name_is_equal(prop, "iommu-map-mask") )
> +    continue;
> +    }
> +
>   res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);
> 
>   if ( res )
> 
> 

Cheers,

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. 
vs. 141822
 test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 
141822
 test-amd64-amd64-xl-pvshim  20 guest-start/debian.repeat fail REGR. vs. 141822

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 7 xen-boot fail in 142327 pass in 
142383
 test-arm64-arm64-examine 11 examine-serial/bootloader fail in 142359 pass in 
142383
 test-amd64-amd64-xl-pvshim 16 guest-localmigrate fail in 142359 pass in 142383
 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail pass in 142327
 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat  fail pass in 142327
 test-armhf-armhf-libvirt-raw 18 leak-check/check   fail pass in 142359

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 141822
 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 141822
 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 141822
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 141822
 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 141822
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 141822
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 141822
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 141822
 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 141822
 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-xsm 13 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim12 guest-start  fail   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-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-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-credit2  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-libvirt-vhd 12 migrate-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-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-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  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-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 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-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 migrate-support-checkfail   

Re: [Xen-devel] [[PATCH for-4.13]] xen/arm: mm: Allow generic xen page-tables helpers to be called early

[Julien Grall]

Hi,

On 03/10/2019 02:02, Stefano Stabellini wrote:
> On Fri, 20 Sep 2019, Julien Grall wrote:
>> That's not correct. alloc_boot_pages() is actually here to allow dynamic
>> allocation before the memory subsystem (and therefore the runtime allocator)
>> is initialized.
> 
> Let me change the question then: is the system_state ==
> SYS_STATE_early_boot check strictly necessary? It looks like it is not:
> the patch would work even if it was just:

I had a few thoughts about it. On Arm32, this only really works for 
32-bits machine address (it can go up to 40-bits). I haven't really 
fully investigated what could go wrong, but it would be best to keep it 
only for early boot.

Also, I don't really want to rely on this "workaround" after boot. Maybe 
we would want to keep them unmapped in the future.

Cheers,

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

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

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 140282
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 140282
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail 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-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-amd64-libvirt-xsm 13 migrate-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-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-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-rtds 13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 14 saverestore-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 

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

[osstest service owner]

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

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  f8abe4fe3c247b069daa59d84d479e42822d93de
baseline version:
 xen  4a647ad128a6e8ea91e9df140708d80548bf47f7

Last test of basis   142395  2019-10-07 10:01:06 Z0 days
Testing same since   142403  2019-10-07 15:01:16 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Juergen Gross 
  Lars Kurth 
  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
   4a647ad128..f8abe4fe3c  f8abe4fe3c247b069daa59d84d479e42822d93de -> smoke

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

[Xen-devel] [freebsd-master test] 142392: regressions - trouble: blocked/fail

[osstest service owner]

flight 142392 freebsd-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/142392/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-freebsd   7 freebsd-buildfail REGR. vs. 141501

Tests which did not succeed, but are not blocking:
 build-amd64-freebsd-again 1 build-check(1)   blocked  n/a
 build-amd64-xen-freebsd   1 build-check(1)   blocked  n/a

version targeted for testing:
 freebsd  bd8e9d12b18187962234407c3e44e9eb34e855d1
baseline version:
 freebsd  14aef6dfca96006e52b8fb920bde7c612ba58b79

Last test of basis   141501  2019-09-20 09:19:51 Z   17 days
Failing since141701  2019-09-23 09:19:41 Z   14 days6 attempts
Testing same since   142392  2019-10-07 09:20:10 Z0 days1 attempts


People who touched revisions under test:
  0mp <0...@freebsd.org>
  alc 
  allanjude 
  andrew 
  asomers 
  avg 
  bapt 
  brooks 
  cem 
  cperciva 
  cy 
  dab 
  daichi 
  dim 
  emaste 
  erj 
  gallatin 
  gjb 
  glebius 
  gonzo 
  grembo 
  hrs 
  hselasky 
  ian 
  imp 
  jhb 
  jhibbits 
  jilles 
  jkim 
  jtl 
  kaktus 
  kan 
  karels 
  kevans 
  kib 
  lwhsu 
  manu 
  markj 
  mav 
  mckusick 
  mhorne 
  mjg 
  mm 
  mmacy 
  olivier 
  oshogbo 
  Piotr Pietruszewski 
  ray 
  rmacklem 
  royger 
  rrs 
  rstone 
  schweikh 
  sef 
  sjg 
  tijl 
  trasz 
  tsoome 
  tuexen 
  vangyzen 
  vmaffione 
  yuripv 

jobs:
 build-amd64-freebsd-againblocked 
 build-amd64-freebsd  fail
 build-amd64-xen-freebsd  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.

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

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-arm64-arm64-libvirt-qcow2 16 guest-start.2 fail in 142345 REGR. vs. 142252

Tests which are failing intermittently (not blocking):
 test-arm64-arm64-libvirt-qcow2 15 guest-start/debian.repeat fail pass in 142345

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 142252
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 142252
 test-amd64-amd64-libvirt 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-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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 
fail 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-libvirt 13 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 14 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass
 test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 13 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  d20983ff63e9f4566d3733cf011aa7a137dd65c8
baseline version:
 libvirt  2346b2f6564ae9f7ba35bc863cb0fab39cadeb12

Last test of basis   142252  2019-10-04 04:23:56 Z3 days
Testing same since   142345  2019-10-06 04:19:12 Z1 days2 attempts


People who touched revisions under test:
  Daniel Veillard 

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  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-arm64-arm64-libvirt-qcow2   fail
 test-armhf-armhf-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass



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

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

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

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


Not pushing.

Re: [Xen-devel] [PATCH 01/24] golang/xenlight: fix calls to libxl_domain_unpause/pause

[George Dunlap]

On 10/7/19 4:12 PM, Nick Rosbrook wrote:
> From: Nick Rosbrook 
> 
> These functions require a third argument of type const *libxl_asyncop_how.
> 
> Pass nil to fix compilation errors. This will have the effect of
> performing these operations synchronously.
> 
> Signed-off-by: Nick Rosbrook 

Reviewed-by: George Dunlap 

Juergen, this is actually a bug fix (these lines didn't get updated when
the API changed), so I'm going to check this in later this week if you
don't object.

> ---
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Wei Liu 
> 
>  tools/golang/xenlight/xenlight.go | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/golang/xenlight/xenlight.go 
> b/tools/golang/xenlight/xenlight.go
> index f5d171c2d5..59b8186a64 100644
> --- a/tools/golang/xenlight/xenlight.go
> +++ b/tools/golang/xenlight/xenlight.go
> @@ -1011,7 +1011,7 @@ func (Ctx *Context) DomainUnpause(Id Domid) (err error) 
> {
>   return
>   }
>  
> - ret := C.libxl_domain_unpause(Ctx.ctx, C.uint32_t(Id))
> + ret := C.libxl_domain_unpause(Ctx.ctx, C.uint32_t(Id), nil)
>  
>   if ret != 0 {
>   err = Error(-ret)
> @@ -1026,7 +1026,7 @@ func (Ctx *Context) DomainPause(id Domid) (err error) {
>   return
>   }
>  
> - ret := C.libxl_domain_pause(Ctx.ctx, C.uint32_t(id))
> + ret := C.libxl_domain_pause(Ctx.ctx, C.uint32_t(id), nil)
>  
>   if ret != 0 {
>   err = Error(-ret)
> 


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

Re: [Xen-devel] [PATCH v2 5/6] Add guide on Communication Best Practice

[George Dunlap]

On 9/27/19 10:14 AM, Jan Beulich wrote:
> On 26.09.2019 21:39, Lars Kurth wrote:
>> +### Verbose vs. terse
>> +Due to the time it takes to review and compose code reviewer, reviewers 
>> often adopt a
>> +terse style. It is not unusual to see review comments such as
>> +> typo
>> +> s/resions/regions/
>> +> coding style
>> +> coding style: brackets not needed
>> +etc.
>> +
>> +Terse code review style has its place and can be productive for both the 
>> reviewer and
>> +the author. However, overuse can come across as unfriendly, lacking empathy 
>> and
>> +can thus create a negative impression with the author of a patch. This is 
>> in particular
>> +true, when you do not know the author or the author is a newcomer. Terse
>> +communication styles can also be perceived as rude in some cultures.
> 
> And another remark here: Not being terse in situations like the ones
> enumerated as examples above is a double waste of the reviewer's time:

FWIW I don't think this document prohibits terse replies.  It points out
that they can come across as unfriendly, and they can be perceived as
rude in some cultures; both of which are true.  It then *recommends*
that reviewers compensate for it in a review opening (i.e., once per
patch / series) which expresses appreciation; which is both helpful and
relatively low cost.

The point of the opening is to set the tone.  If you start out with
something positive, and ends with "thanks", then a long series of terse
comments is more likely to be read as simply being efficient.  If the
entire review consists of nothing but criticism or terse comments, it's
more likely to be read as annoyance on the part of the reviewer.

> They shouldn't even need to make such comments, especially not many
> times for a single patch (see your mention of "overuse").  I realize
> we still have no automated mechanism to check style aspects, but
> anybody can easily look over their patches before submitting them.
> And for an occasional issue I think a terse reply is quite reasonable
> to have.

This sort of sounds like you are *intending* to express annoyance?

If so, that's a slightly different question than what this section is
addressing. :-)

 -George

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

Re: [Xen-devel] [PATCH v2 5/6] Add guide on Communication Best Practice

[George Dunlap]

On 9/26/19 8:39 PM, Lars Kurth wrote:
> +investigate the practice foot-binding, it is hard to disagree with the 
> dictionart entry.

Typo: dictionary

 -George

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

[Xen-devel] [PATCH 16/24] golang/xenlight: begin C to Go type marshaling

[Nick Rosbrook]

From: Nick Rosbrook 

Implement basic type conversion in fromC functions such as strings and
integer types. Also, remove existing toGo functions from xenlight.go in
favor of the new generated functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py   | 120 +++
 tools/golang/xenlight/xenlight.go | 111 +--
 tools/golang/xenlight/xenlight_helpers.go | 967 ++
 3 files changed, 1098 insertions(+), 100 deletions(-)
 create mode 100644 tools/golang/xenlight/xenlight_helpers.go

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index c8513b79e0..75dcd01724 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -18,6 +18,12 @@ builtin_type_names = {
 idl.uint64.typename: 'uint64',
 }
 
+# Some go keywords that conflict with field names in libxl structs.
+go_keywords = ['type', 'func']
+
+go_builtin_types = ['bool', 'string', 'int', 'byte',
+'uint16', 'uint32', 'uint64']
+
 # List of strings that need to be written to a file
 # after a struct definition.
 type_extras = []
@@ -168,6 +174,118 @@ def xenlight_golang_define_union(ty = None, structname = 
''):
 
 return s
 
+def xenlight_golang_generate_helpers(path = None, types = None, comment = 
None):
+"""
+Generate a .go file (xenlight_helpers.go by default)
+that contains helper functions for marshaling between
+C and Go types.
+"""
+if path is None:
+path = 'xenlight_helpers.go'
+
+with open(path, 'w') as f:
+if comment is not None:
+f.write(comment)
+f.write('package xenlight\n')
+
+# Cgo preamble
+f.write('/*\n')
+f.write('#cgo LDFLAGS: -lxenlight\n')
+f.write('#include \n')
+f.write('#include \n')
+f.write('\n')
+
+f.write('*/\nimport "C"\n')
+
+for ty in types:
+if not isinstance(ty, idl.Struct):
+continue
+
+f.write(xenlight_golang_define_from_C(ty))
+f.write('\n')
+
+go_fmt(path)
+
+def xenlight_golang_define_from_C(ty = None, typename = None, nested = False):
+s = ''
+
+gotypename = ctypename = ''
+
+if typename is not None:
+gotypename = xenlight_golang_fmt_name(typename)
+ctypename  = typename
+else:
+gotypename = xenlight_golang_fmt_name(ty.typename)
+ctypename  = ty.typename
+
+if not nested:
+s += 'func (x *{}) fromC(xc *C.{}) error 
{{\n'.format(gotypename,ctypename)
+
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+# TODO
+continue
+
+gotypename = xenlight_golang_fmt_name(f.type.typename)
+gofname= xenlight_golang_fmt_name(f.name)
+cfname = f.name
+
+# In cgo, C names that conflict with Go keywords can be
+# accessed by prepending an underscore to the name.
+if cfname in go_keywords:
+cfname = '_' + cfname
+
+# If this is nested, we need the outer name too.
+if nested and typename is not None:
+goname = xenlight_golang_fmt_name(typename)
+goname = '{}.{}'.format(goname, gofname)
+cname  = '{}.{}'.format(typename, cfname)
+
+else:
+goname = gofname
+cname  = cfname
+
+# Types that satisfy this condition can be easily casted or
+# converted to a Go builtin type.
+is_castable = (f.type.json_parse_type == 'JSON_INTEGER' or
+   isinstance(f.type, idl.Enumeration) or
+   gotypename in go_builtin_types)
+
+if is_castable:
+# Use the cgo helper for converting C strings.
+if gotypename == 'string':
+s += 'x.{} = C.GoString(xc.{})\n'.format(goname, cname)
+continue
+
+s += 'x.{} = {}(xc.{})\n'.format(goname, gotypename, cname)
+
+else:
+# If the type is not castable, we need to call its fromC
+# function.
+varname = '{}_{}'.format(f.type.typename,f.name)
+varname = xenlight_golang_fmt_name(varname, exported=False)
+
+s += 'var {} {}\n'.format(varname, gotypename)
+s += 'if err := {}.fromC({});'.format(varname, cname)
+s += 'err != nil {\n return err\n}\n'
+s += 'x.{} = {}\n'.format(goname, varname)
+
+elif isinstance(f.type, idl.Struct):
+s += xenlight_golang_define_from_C(f.type, typename=f.name, 
nested=True)
+
+elif isinstance(f.type, idl.KeyedUnion):
+pass
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+   

[Xen-devel] [PATCH 21/24] golang/xenlight: implement array Go to C marshaling

[Nick Rosbrook]

From: Nick Rosbrook 

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py   |  44 ++-
 tools/golang/xenlight/xenlight_helpers.go | 359 ++
 2 files changed, 402 insertions(+), 1 deletion(-)

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index 35d9dfea40..d8a6120e5c 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -479,7 +479,7 @@ def xenlight_golang_define_to_C(ty = None, typename = None, 
nested = False):
 for f in ty.fields:
 if f.type.typename is not None:
 if isinstance(f.type, idl.Array):
-# TODO
+s += xenlight_golang_array_to_C(f, ty.dispose_fn)
 continue
 
 gotypename = xenlight_golang_fmt_name(f.type.typename)
@@ -610,6 +610,48 @@ def xenlight_golang_union_to_C(ty = None, union_name = '',
 
 return s
 
+def xenlight_golang_array_to_C(ty = None, dispose_fn = ''):
+s = ''
+
+gotypename = xenlight_golang_fmt_name(ty.type.elem_type.typename)
+goname = xenlight_golang_fmt_name(ty.name)
+ctypename  = ty.type.elem_type.typename
+cname  = ty.name
+clenvar= ty.type.lenvar.name
+golenvar   = xenlight_golang_fmt_name(clenvar,exported=False)
+
+is_enum = isinstance(ty.type.elem_type,idl.Enumeration)
+if gotypename in go_builtin_types or is_enum:
+s += '{} := len(x.{})\n'.format(golenvar,goname)
+s += 'xc.{} = 
(*C.{})(C.malloc(C.size_t({}*{})))\n'.format(cname,ctypename,
+   
golenvar,golenvar)
+s += 'xc.{} = C.int({})\n'.format(clenvar,golenvar)
+s += 'c{} := 
(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(goname,
+  
ctypename,cname,
+  
golenvar,golenvar)
+s += 'for i,v := range x.{} {{\n'.format(goname)
+s += 'c{}[i] = C.{}(v)\n'.format(goname,ctypename)
+s += '}\n'
+
+return s
+
+s += '{} := len(x.{})\n'.format(golenvar,goname)
+s += 'xc.{} = 
(*C.{})(C.malloc(C.ulong({})*C.sizeof_{}))\n'.format(cname,ctypename,
+   
golenvar,ctypename)
+s += 'xc.{} = C.int({})\n'.format(clenvar,golenvar)
+s += 'c{} := 
(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(goname,
+ 
ctypename,cname,
+ 
golenvar,golenvar)
+s += 'for i,v := range x.{} {{\n'.format(goname)
+s += 'tmp, err := v.toC()\n'
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,err\n}\n'
+s += 'c{}[i] = tmp\n'.format(goname)
+s += '}\n'
+
+return s
+
 def xenlight_golang_fmt_name(name, exported = True):
 """
 Take a given type name and return an
diff --git a/tools/golang/xenlight/xenlight_helpers.go 
b/tools/golang/xenlight/xenlight_helpers.go
index 2cb5adaec5..7940c209a2 100644
--- a/tools/golang/xenlight/xenlight_helpers.go
+++ b/tools/golang/xenlight/xenlight_helpers.go
@@ -662,6 +662,18 @@ func (x *VcpuSchedParams) fromC(xc 
*C.libxl_vcpu_sched_params) error {
 func (x *VcpuSchedParams) toC() (xc C.libxl_vcpu_sched_params, err error) {
C.libxl_vcpu_sched_params_init()
xc.sched = C.libxl_scheduler(x.Sched)
+   numVcpus := len(x.Vcpus)
+   xc.vcpus = (*C.libxl_sched_params)(C.malloc(C.ulong(numVcpus) * 
C.sizeof_libxl_sched_params))
+   xc.num_vcpus = C.int(numVcpus)
+   cVcpus := (*[1 << 
28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
+   for i, v := range x.Vcpus {
+   tmp, err := v.toC()
+   if err != nil {
+   C.libxl_vcpu_sched_params_dispose()
+   return xc, err
+   }
+   cVcpus[i] = tmp
+   }
return xc, nil
 }
 
@@ -710,6 +722,13 @@ func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
 func (x *VnodeInfo) toC() (xc C.libxl_vnode_info, err error) {
C.libxl_vnode_info_init()
xc.memkb = C.uint64_t(x.Memkb)
+   numDistances := len(x.Distances)
+   xc.distances = (*C.uint32_t)(C.malloc(C.size_t(numDistances * 
numDistances)))
+   xc.num_distances = C.int(numDistances)
+   cDistances := (*[1 << 
28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
+   for i, v := range x.Distances {
+   cDistances[i] = C.uint32_t(v)
+   }
xc.pnode = C.uint32_t(x.Pnode)
xc.vcpus, err = x.Vcpus.toC()
if err != nil {
@@ -1096,6 +1115,30 @@ func (x *DomainBuildInfo) toC() (xc 
C.libxl_domain_build_info, err error) {

[Xen-devel] [PATCH 22/24] golang/xenlight: revise use of Context type

[Nick Rosbrook]

From: Nick Rosbrook 

Remove the exported global context variable, 'Ctx.' Generally, it is
better to not export global variables for use through a Go package.
However, there are some exceptions that can be found in the standard
library.

Add a NewContext function instead, and remove the Open, IsOpen, and
CheckOpen functions as a result.

Also, comment-out an ineffectual assignment to 'err' inside the function
Context.CpupoolInfo so that compilation does not fail.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 219 +-
 1 file changed, 34 insertions(+), 185 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index a8f933c75f..e540b5413d 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -74,6 +74,39 @@ func (e Error) Error() string {
return fmt.Sprintf("libxl error: %d", -e)
 }
 
+// Context represents a libxl_ctx.
+type Context struct {
+   ctx*C.libxl_ctx
+   logger *C.xentoollog_logger_stdiostream
+}
+
+// NewContext returns a new Context.
+func NewContext() (*Context, error) {
+   var ctx Context
+
+   ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
+
+   ret := C.libxl_ctx_alloc(, C.LIBXL_VERSION, 0, 
(*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+   if ret != 0 {
+   return nil, Error(ret)
+   }
+
+   return , nil
+}
+
+// Close closes the Context.
+func (ctx *Context) Close() error {
+   ret := C.libxl_ctx_free(ctx.ctx)
+   ctx.ctx = nil
+   C.xtl_logger_destroy((*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+
+   if ret != 0 {
+   return Error(ret)
+   }
+
+   return nil
+}
+
 /*
  * Types: Builtins
  */
@@ -299,11 +332,6 @@ func (cpl *CpuidPolicyList) toC() 
(C.libxl_cpuid_policy_list, error) {
return *ccpl, nil
 }
 
-type Context struct {
-   ctx*C.libxl_ctx
-   logger *C.xentoollog_logger_stdiostream
-}
-
 // Hwcap represents a libxl_hwcap.
 type Hwcap [8]uint32
 
@@ -480,11 +508,6 @@ func SchedulerFromString(name string) (s Scheduler, err 
error) {
 // libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool_out);
 // void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nb_pool);
 func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
-   err := Ctx.CheckOpen()
-   if err != nil {
-   return
-   }
-
var nbPool C.int
 
c_cpupool_list := C.libxl_list_cpupool(Ctx.ctx, )
@@ -508,16 +531,11 @@ func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
 
 // int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t 
poolid);
 func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo) {
-   err := Ctx.CheckOpen()
-   if err != nil {
-   return
-   }
-
var c_cpupool C.libxl_cpupoolinfo
 
ret := C.libxl_cpupool_info(Ctx.ctx, _cpupool, C.uint32_t(Poolid))
if ret != 0 {
-   err = Error(-ret)
+   //err = Error(-ret)
return
}
defer C.libxl_cpupoolinfo_dispose(_cpupool)
@@ -534,11 +552,6 @@ func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool 
Cpupoolinfo) {
 // FIXME: uuid
 // FIXME: Setting poolid
 func (Ctx *Context) CpupoolCreate(Name string, Scheduler Scheduler, Cpumap 
Bitmap) (err error, Poolid uint32) {
-   err = Ctx.CheckOpen()
-   if err != nil {
-   return
-   }
-
poolid := C.uint32_t(C.LIBXL_CPUPOOL_POOLID_ANY)
name := C.CString(Name)
defer C.free(unsafe.Pointer(name))
@@ -567,11 +580,6 @@ func (Ctx *Context) CpupoolCreate(Name string, Scheduler 
Scheduler, Cpumap Bitma
 
 // int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
 func (Ctx *Context) CpupoolDestroy(Poolid uint32) (err error) {
-   err = Ctx.CheckOpen()
-   if err != nil {
-   return
-   }
-
ret := C.libxl_cpupool_destroy(Ctx.ctx, C.uint32_t(Poolid))
if ret != 0 {
err = Error(-ret)
@@ -583,11 +591,6 @@ func (Ctx *Context) CpupoolDestroy(Poolid uint32) (err 
error) {
 
 // int libxl_cpupool_cpuadd(libxl_ctx *ctx, uint32_t poolid, int cpu);
 func (Ctx *Context) CpupoolCpuadd(Poolid uint32, Cpu int) (err error) {
-   err = Ctx.CheckOpen()
-   if err != nil {
-   return
-   }
-
ret := C.libxl_cpupool_cpuadd(Ctx.ctx, C.uint32_t(Poolid), C.int(Cpu))
if ret != 0 {
err = Error(-ret)
@@ -600,11 +603,6 @@ func (Ctx *Context) CpupoolCpuadd(Poolid uint32, Cpu int) 
(err error) {
 // int libxl_cpupool_cpuadd_cpumap(libxl_ctx *ctx, uint32_t poolid,
 // const libxl_bitmap *cpumap);
 func (Ctx *Context) CpupoolCpuaddCpumap(Poolid uint32, Cpumap Bitmap) (err 
error) {
-   err = Ctx.CheckOpen()
-   if err != nil {
-   return
-   

[Xen-devel] [PATCH 19/24] golang/xenlight: begin Go to C type marshaling

[Nick Rosbrook]

From: Nick Rosbrook 

Implement conversion of basic type conversions such as strings
and integer types in toC functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py   |   80 ++
 tools/golang/xenlight/xenlight_helpers.go | 1013 +
 2 files changed, 1093 insertions(+)

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index 2b620f0ae9..eccc334b41 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -237,6 +237,9 @@ def xenlight_golang_generate_helpers(path = None, types = 
None, comment = None):
 
 del helper_extras[:]
 
+f.write(xenlight_golang_define_to_C(ty))
+f.write('\n')
+
 go_fmt(path)
 
 def xenlight_golang_define_from_C(ty = None, typename = None, nested = False):
@@ -457,6 +460,83 @@ def xenlight_golang_array_from_C(ty = None):
 
 return s
 
+def xenlight_golang_define_to_C(ty = None, typename = None, nested = False):
+s = ''
+
+gotypename = ctypename = ''
+
+if typename is not None:
+gotypename = xenlight_golang_fmt_name(typename)
+ctypename  = typename
+else:
+gotypename = xenlight_golang_fmt_name(ty.typename)
+ctypename  = ty.typename
+
+if not nested:
+s += 'func (x *{}) toC() (xc C.{},err error) 
{{\n'.format(gotypename,ctypename)
+s += 'C.{}()\n'.format(ty.init_fn)
+
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+# TODO
+continue
+
+gotypename = xenlight_golang_fmt_name(f.type.typename)
+ctypename  = f.type.typename
+gofname= xenlight_golang_fmt_name(f.name)
+cfname = f.name
+
+# In cgo, C names that conflict with Go keywords can be
+# accessed by prepending an underscore to the name.
+if cfname in go_keywords:
+cfname = '_' + cfname
+
+# If this is nested, we need the outer name too.
+if nested and typename is not None:
+goname = xenlight_golang_fmt_name(typename)
+goname = '{}.{}'.format(goname, gofname)
+cname  = '{}.{}'.format(typename, cfname)
+
+else:
+goname = gofname
+cname  = cfname
+
+is_castable = (f.type.json_parse_type == 'JSON_INTEGER' or
+   isinstance(f.type, idl.Enumeration) or
+   gotypename in go_builtin_types)
+
+if is_castable:
+# Use the cgo helper for converting C strings.
+if gotypename == 'string':
+s += 'xc.{} = C.CString(x.{})\n'.format(cname,goname)
+continue
+
+s += 'xc.{} = C.{}(x.{})\n'.format(cname,ctypename,goname)
+
+else:
+s += 'xc.{}, err = x.{}.toC()\n'.format(cname,goname)
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(ty.dispose_fn)
+s += 'return xc, err\n'
+s += '}\n'
+
+elif isinstance(f.type, idl.Struct):
+s += xenlight_golang_define_to_C(f.type, typename=f.name, 
nested=True)
+
+elif isinstance(f.type, idl.KeyedUnion):
+# TODO
+pass
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+if not nested:
+s += 'return xc, nil'
+s += '}\n'
+
+return s
+
 def xenlight_golang_fmt_name(name, exported = True):
 """
 Take a given type name and return an
diff --git a/tools/golang/xenlight/xenlight_helpers.go 
b/tools/golang/xenlight/xenlight_helpers.go
index 2cdc1bbdc9..92e6afcd10 100644
--- a/tools/golang/xenlight/xenlight_helpers.go
+++ b/tools/golang/xenlight/xenlight_helpers.go
@@ -130,6 +130,13 @@ func (x *IoportRange) fromC(xc *C.libxl_ioport_range) 
error {
return nil
 }
 
+func (x *IoportRange) toC() (xc C.libxl_ioport_range, err error) {
+   C.libxl_ioport_range_init()
+   xc.first = C.uint32_t(x.First)
+   xc.number = C.uint32_t(x.Number)
+   return xc, nil
+}
+
 func (x *IomemRange) fromC(xc *C.libxl_iomem_range) error {
x.Start = uint64(xc.start)
x.Number = uint64(xc.number)
@@ -137,11 +144,25 @@ func (x *IomemRange) fromC(xc *C.libxl_iomem_range) error 
{
return nil
 }
 
+func (x *IomemRange) toC() (xc C.libxl_iomem_range, err error) {
+   C.libxl_iomem_range_init()
+   xc.start = C.uint64_t(x.Start)
+   xc.number = C.uint64_t(x.Number)
+   xc.gfn = C.uint64_t(x.Gfn)
+   return xc, nil
+}
+
 func (x *VgaInterfaceInfo) fromC(xc *C.libxl_vga_interface_info) error {
x.Kind = VgaInterfaceType(xc.kind)
return nil
 }
 
+func (x *VgaInterfaceInfo) toC() (xc C.libxl_vga_interface_info, err error) {
+

[Xen-devel] [PATCH 23/24] golang/xenlight: add error return type to Context.Cpupoolinfo

[Nick Rosbrook]

From: Nick Rosbrook 

A previous commit that removed Context.CheckOpen revealed
an ineffectual assignent to err in Context.Cpupoolinfo, as
there is no error return type.

Since it appears that the intent is to return an error here,
add an error return value to the function signature.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index e540b5413d..0a4f476451 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -530,17 +530,17 @@ func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
 }
 
 // int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t 
poolid);
-func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo) {
+func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo, err error) {
var c_cpupool C.libxl_cpupoolinfo
 
ret := C.libxl_cpupool_info(Ctx.ctx, _cpupool, C.uint32_t(Poolid))
if ret != 0 {
-   //err = Error(-ret)
+   err = Error(-ret)
return
}
defer C.libxl_cpupoolinfo_dispose(_cpupool)
 
-   _ = pool.fromC(_cpupool)
+   err = pool.fromC(_cpupool)
 
return
 }
-- 
2.19.1


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

[Xen-devel] [PATCH 24/24] golang/xenlight: add make target for generated files

[Nick Rosbrook]

From: Nick Rosbrook 

Remove the PKGSOURCES variable since adding xenlight_types.go
and xenlight_helpers.go to this list breaks the rest of the
Makefile.

Add xenlight_%.go target for generated files, and use full
file names within install, uninstall and $(XEN_GOPATH)$(GOXL_PKG_DIR)
rule.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/Makefile | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index 0987305224..821a5d48fa 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -7,20 +7,22 @@ GOCODE_DIR ?= $(prefix)/share/gocode/
 GOXL_PKG_DIR = /src/$(XEN_GOCODE_URL)/xenlight/
 GOXL_INSTALL_DIR = $(GOCODE_DIR)$(GOXL_PKG_DIR)
 
-# PKGSOURCES: Files which comprise the distributed source package
-PKGSOURCES = xenlight.go
-
 GO ?= go
 
 .PHONY: all
 all: build
 
 .PHONY: package
-package: $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES)
+package: $(XEN_GOPATH)$(GOXL_PKG_DIR)
 
-$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/$(PKGSOURCES): $(PKGSOURCES)
+$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: xenlight_%.go
$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
-   $(INSTALL_DATA) $(PKGSOURCES) $(XEN_GOPATH)$(GOXL_PKG_DIR)
+   $(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+   $(INSTALL_DATA) xenlight_types.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+   $(INSTALL_DATA) xenlight_helpers.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+
+xenlight_%.go: gengotypes.py $(XEN_ROOT)/tools/libxl/libxl_types.idl 
$(XEN_ROOT)/tools/libxl/idl.py
+   XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py ../../libxl/libxl_types.idl
 
 # Go will do its own dependency checking, and not actuall go through
 # with the build if none of the input files have changed.
@@ -36,10 +38,14 @@ build: package
 .PHONY: install
 install: build
$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
-   $(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)$(PKGSOURCES) 
$(DESTDIR)$(GOXL_INSTALL_DIR)
+   $(install_data) $(xen_gopath)$(goxl_pkg_dir)xenlight.go 
$(destdir)$(goxl_install_dir)
+   $(install_data) $(xen_gopath)$(goxl_pkg_dir)xenlight_types.go 
$(destdir)$(goxl_install_dir)
+   $(install_data) $(xen_gopath)$(goxl_pkg_dir)xenlight_helpers.go 
$(destdir)$(goxl_install_dir)
 
 .PHONY: uninstall
-   rm -f $(addprefix $(DESTDIR)$(GOXL_INSTALL_DIR)/, $(PKGSOURCES))
+   rm -f $(addprefix $(DESTDIR)$(GOXL_INSTALL_DIR)/, xenlight.go)
+   rm -f $(addprefix $(DESTDIR)$(GOXL_INSTALL_DIR)/, xenlight_types.go)
+   rm -f $(addprefix $(DESTDIR)$(GOXL_INSTALL_DIR)/, xenlight_helpers.go)
 
 .PHONY: clean
 clean:
-- 
2.19.1


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

[Xen-devel] [PATCH 20/24] golang/xenlight: implement keyed union Go to C marshaling

[Nick Rosbrook]

From: Nick Rosbrook 

Since the C union cannot be directly populated, populate the fields of the
corresponding C struct defined in the cgo preamble, and then copy that
struct as bytes into the byte slice that Go uses as the union.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py   |  77 -
 tools/golang/xenlight/xenlight_helpers.go | 325 ++
 2 files changed, 400 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index eccc334b41..35d9dfea40 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -525,8 +525,7 @@ def xenlight_golang_define_to_C(ty = None, typename = None, 
nested = False):
 s += xenlight_golang_define_to_C(f.type, typename=f.name, 
nested=True)
 
 elif isinstance(f.type, idl.KeyedUnion):
-# TODO
-pass
+s += xenlight_golang_union_to_C(f.type, f.name, ty.typename, 
ty.dispose_fn)
 
 else:
 raise Exception('type {} not supported'.format(f.type))
@@ -537,6 +536,80 @@ def xenlight_golang_define_to_C(ty = None, typename = 
None, nested = False):
 
 return s
 
+def xenlight_golang_union_to_C(ty = None, union_name = '',
+   struct_name = '', dispose_fn = ''):
+keyname   = ty.keyvar.name
+gokeyname = xenlight_golang_fmt_name(keyname)
+keytype   = ty.keyvar.type.typename
+gokeytype = xenlight_golang_fmt_name(keytype)
+
+interface_name = '{}_{}_union'.format(struct_name, keyname)
+interface_name = xenlight_golang_fmt_name(interface_name, exported=False)
+
+cgo_keyname = keyname
+if cgo_keyname in go_keywords:
+cgo_keyname = '_' + cgo_keyname
+
+
+s = 'xc.{} = C.{}(x.{})\n'.format(cgo_keyname,keytype,gokeyname)
+s += 'switch x.{}{{\n'.format(gokeyname)
+
+# Create switch statement to determine how to populate the C union.
+for f in ty.fields:
+key_val = '{}_{}'.format(keytype, f.name)
+key_val = xenlight_golang_fmt_name(key_val)
+if f.type is None:
+continue
+
+s += 'case {}:\n'.format(key_val)
+cgotype = '{}_{}_union_{}'.format(struct_name,keyname,f.name)
+gotype  = xenlight_golang_fmt_name(cgotype)
+goname  = '{}_{}'.format(keyname,f.name)
+goname  = xenlight_golang_fmt_name(goname,exported=False)
+
+field_name = xenlight_golang_fmt_name('{}_union'.format(keyname))
+s += 'tmp, ok := x.{}.({})\n'.format(field_name,gotype)
+s += 'if !ok {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,errors.New("wrong type for union key 
{}")\n'.format(keyname)
+s += '}\n'
+
+s += 'var {} C.{}\n'.format(f.name,cgotype)
+for uf in f.type.fields:
+gotypename = xenlight_golang_fmt_name(uf.type.typename)
+ctypename  = uf.type.typename
+gofname= xenlight_golang_fmt_name(uf.name)
+
+is_castable = (uf.type.json_parse_type == 'JSON_INTEGER' or
+   isinstance(uf.type, idl.Enumeration) or
+   gotypename in go_builtin_types)
+
+if not is_castable:
+s += '{}.{}, err = 
tmp.{}.toC()\n'.format(f.name,uf.name,gofname)
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,err \n}\n'
+
+elif gotypename == 'string':
+s += '{}.{} = 
C.CString(tmp.{})\n'.format(f.name,uf.name,gofname)
+
+else:
+s += '{}.{} = 
C.{}(tmp.{})\n'.format(f.name,uf.name,ctypename,gofname)
+
+# The union is still represented as Go []byte.
+s += '{}Bytes := 
C.GoBytes(unsafe.Pointer(&{}),C.sizeof_{})\n'.format(f.name,
+  
f.name,
+  
cgotype)
+s += 'copy(xc.{}[:],{}Bytes)\n'.format(union_name,f.name)
+
+# End switch statement
+s += 'default:\n'
+err_string = '"invalid union key \'%v\'", x.{}'.format(gokeyname)
+s += 'return xc, fmt.Errorf({})'.format(err_string)
+s += '}\n'
+
+return s
+
 def xenlight_golang_fmt_name(name, exported = True):
 """
 Take a given type name and return an
diff --git a/tools/golang/xenlight/xenlight_helpers.go 
b/tools/golang/xenlight/xenlight_helpers.go
index 92e6afcd10..2cb5adaec5 100644
--- a/tools/golang/xenlight/xenlight_helpers.go
+++ b/tools/golang/xenlight/xenlight_helpers.go
@@ -433,6 +433,21 @@ func (x *Channelinfo) toC() (xc C.libxl_channelinfo, err 
error) {
xc.state = C.int(x.State)
xc.evtch = C.int(x.Evtch)
xc.rref = C.int(x.Rref)
+   xc.connection = C.libxl_channel_connection(x.Connection)
+   switch x.Connection {
+   

[Xen-devel] [PATCH 17/24] golang/xenlight: implement keyed union C to Go marshaling

[Nick Rosbrook]

From: Nick Rosbrook 

Switch over union key to determine how to populate 'union' in Go struct.

Since the unions of C types cannot be directly accessed, add C structs in
cgo preamble to assist in marshaling keyed unions. This allows the C
type defined in the preamble to be populated first, and then accessed
directly to populate the Go struct.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py   | 136 ++-
 tools/golang/xenlight/xenlight_helpers.go | 440 ++
 2 files changed, 575 insertions(+), 1 deletion(-)

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index 75dcd01724..ececaafd72 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -28,6 +28,14 @@ go_builtin_types = ['bool', 'string', 'int', 'byte',
 # after a struct definition.
 type_extras = []
 
+# cgo preamble for xenlight_helpers.go, created during type generation and
+# written later.
+cgo_helpers_preamble = []
+
+# List of strings that need to be written to a file
+# after a helper func definition.
+helper_extras = []
+
 def xenlight_golang_generate_types(path = None, types = None, comment = None):
 """
 Generate a .go file (xenlight_types.go by default)
@@ -159,6 +167,8 @@ def xenlight_golang_define_union(ty = None, structname = 
''):
 s = xenlight_golang_define_struct(f.type, typename=name)
 type_extras.append(s)
 
+xenlight_golang_union_cgo_preamble(f.type, name=name)
+
 # Define function to implement 'union' interface
 name = xenlight_golang_fmt_name(name)
 s = 'func (x {}) is{}(){{}}\n'.format(name, interface_name)
@@ -174,6 +184,18 @@ def xenlight_golang_define_union(ty = None, structname = 
''):
 
 return s
 
+def xenlight_golang_union_cgo_preamble(ty = None, name = ''):
+s = ''
+
+s += 'typedef struct {} {{\n'.format(name)
+
+for f in ty.fields:
+s += '\t{} {};\n'.format(f.type.typename, f.name)
+
+s += '}} {};\n'.format(name)
+
+cgo_helpers_preamble.append(s)
+
 def xenlight_golang_generate_helpers(path = None, types = None, comment = 
None):
 """
 Generate a .go file (xenlight_helpers.go by default)
@@ -187,6 +209,7 @@ def xenlight_golang_generate_helpers(path = None, types = 
None, comment = None):
 if comment is not None:
 f.write(comment)
 f.write('package xenlight\n')
+f.write('import (\n"unsafe"\n"errors"\n"fmt"\n)\n')
 
 # Cgo preamble
 f.write('/*\n')
@@ -195,6 +218,10 @@ def xenlight_golang_generate_helpers(path = None, types = 
None, comment = None):
 f.write('#include \n')
 f.write('\n')
 
+for s in cgo_helpers_preamble:
+f.write(s)
+f.write('\n')
+
 f.write('*/\nimport "C"\n')
 
 for ty in types:
@@ -204,6 +231,12 @@ def xenlight_golang_generate_helpers(path = None, types = 
None, comment = None):
 f.write(xenlight_golang_define_from_C(ty))
 f.write('\n')
 
+for extra in helper_extras:
+f.write(extra)
+f.write('\n')
+
+del helper_extras[:]
+
 go_fmt(path)
 
 def xenlight_golang_define_from_C(ty = None, typename = None, nested = False):
@@ -275,7 +308,7 @@ def xenlight_golang_define_from_C(ty = None, typename = 
None, nested = False):
 s += xenlight_golang_define_from_C(f.type, typename=f.name, 
nested=True)
 
 elif isinstance(f.type, idl.KeyedUnion):
-pass
+s += xenlight_golang_union_from_C(f.type, f.name, ty.typename)
 
 else:
 raise Exception('type {} not supported'.format(f.type))
@@ -286,6 +319,107 @@ def xenlight_golang_define_from_C(ty = None, typename = 
None, nested = False):
 
 return s
 
+def xenlight_golang_union_from_C(ty = None, union_name = '', struct_name = ''):
+keyname   = ty.keyvar.name
+gokeyname = xenlight_golang_fmt_name(keyname)
+keytype   = ty.keyvar.type.typename
+gokeytype = xenlight_golang_fmt_name(keytype)
+
+interface_name = '{}_{}_union'.format(struct_name, keyname)
+interface_name = xenlight_golang_fmt_name(interface_name, exported=False)
+
+cgo_keyname = keyname
+if cgo_keyname in go_keywords:
+cgo_keyname = '_' + cgo_keyname
+
+cases = {}
+
+for f in ty.fields:
+val = '{}_{}'.format(keytype, f.name)
+val = xenlight_golang_fmt_name(val)
+
+# Add to list of cases to make for the switch
+# statement below.
+if f.type is None:
+continue
+
+cases[f.name] = val
+
+# Define fromC func for 'union' struct.
+typename   = '{}_{}_union_{}'.format(struct_name,keyname,f.name)
+gotypename = xenlight_golang_fmt_name(typename)
+
+# Define the function here. The cases for keyed unions are a little
+# different.
+s = 'func (x 

[Xen-devel] [PATCH 18/24] golang/xenlight: implement array C to Go marshaling

[Nick Rosbrook]

From: Nick Rosbrook 

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py   |  39 ++-
 tools/golang/xenlight/xenlight_helpers.go | 300 ++
 2 files changed, 338 insertions(+), 1 deletion(-)

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index ececaafd72..2b620f0ae9 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -257,7 +257,7 @@ def xenlight_golang_define_from_C(ty = None, typename = 
None, nested = False):
 for f in ty.fields:
 if f.type.typename is not None:
 if isinstance(f.type, idl.Array):
-# TODO
+s += xenlight_golang_array_from_C(f)
 continue
 
 gotypename = xenlight_golang_fmt_name(f.type.typename)
@@ -420,6 +420,43 @@ def xenlight_golang_union_fields_from_C(ty = None):
 
 return s
 
+def xenlight_golang_array_from_C(ty = None):
+"""
+Convert C array to Go slice using the method
+described here:
+
+https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
+"""
+s = ''
+
+gotypename = xenlight_golang_fmt_name(ty.type.elem_type.typename)
+goname = xenlight_golang_fmt_name(ty.name)
+ctypename  = ty.type.elem_type.typename
+cname  = ty.name
+cslice = 'c{}'.format(goname)
+clenvar= ty.type.lenvar.name
+golenvar   = xenlight_golang_fmt_name(clenvar,exported=False)
+
+s += '{} := int(xc.{})\n'.format(golenvar, clenvar)
+s += '{} := '.format(cslice)
+s +='(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(ctypename, 
cname,
+golenvar, 
golenvar)
+s += 'x.{} = make([]{}, {})\n'.format(goname, gotypename, golenvar)
+s += 'for i, v := range {} {{\n'.format(cslice)
+
+is_enum = isinstance(ty.type.elem_type,idl.Enumeration)
+if gotypename in go_builtin_types or is_enum:
+s += 'x.{}[i] = {}(v)\n'.format(goname, gotypename)
+else:
+s += 'var e {}\n'.format(gotypename)
+s += 'if err := e.fromC(); err != nil {\n'
+s += 'return err }\n'
+s += 'x.{}[i] = e\n'.format(goname)
+
+s += '}\n'
+
+return s
+
 def xenlight_golang_fmt_name(name, exported = True):
 """
 Take a given type name and return an
diff --git a/tools/golang/xenlight/xenlight_helpers.go 
b/tools/golang/xenlight/xenlight_helpers.go
index b8abef8068..2cdc1bbdc9 100644
--- a/tools/golang/xenlight/xenlight_helpers.go
+++ b/tools/golang/xenlight/xenlight_helpers.go
@@ -382,6 +382,16 @@ func (x *SchedParams) fromC(xc *C.libxl_sched_params) 
error {
 
 func (x *VcpuSchedParams) fromC(xc *C.libxl_vcpu_sched_params) error {
x.Sched = Scheduler(xc.sched)
+   numVcpus := int(xc.num_vcpus)
+   cVcpus := (*[1 << 
28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
+   x.Vcpus = make([]SchedParams, numVcpus)
+   for i, v := range cVcpus {
+   var e SchedParams
+   if err := e.fromC(); err != nil {
+   return err
+   }
+   x.Vcpus[i] = e
+   }
return nil
 }
 
@@ -399,6 +409,12 @@ func (x *DomainSchedParams) fromC(xc 
*C.libxl_domain_sched_params) error {
 
 func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
x.Memkb = uint64(xc.memkb)
+   numDistances := int(xc.num_distances)
+   cDistances := (*[1 << 
28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
+   x.Distances = make([]uint32, numDistances)
+   for i, v := range cDistances {
+   x.Distances[i] = uint32(v)
+   }
x.Pnode = uint32(xc.pnode)
var bitmapVcpus Bitmap
if err := bitmapVcpus.fromC(); err != nil {
@@ -431,6 +447,26 @@ func (x *DomainBuildInfo) fromC(xc 
*C.libxl_domain_build_info) error {
return err
}
x.Nodemap = bitmapNodemap
+   numVcpuHardAffinity := int(xc.num_vcpu_hard_affinity)
+   cVcpuHardAffinity := (*[1 << 
28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_hard_affinity))[:numVcpuHardAffinity:numVcpuHardAffinity]
+   x.VcpuHardAffinity = make([]Bitmap, numVcpuHardAffinity)
+   for i, v := range cVcpuHardAffinity {
+   var e Bitmap
+   if err := e.fromC(); err != nil {
+   return err
+   }
+   x.VcpuHardAffinity[i] = e
+   }
+   numVcpuSoftAffinity := int(xc.num_vcpu_soft_affinity)
+   cVcpuSoftAffinity := (*[1 << 
28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_soft_affinity))[:numVcpuSoftAffinity:numVcpuSoftAffinity]
+   x.VcpuSoftAffinity = make([]Bitmap, numVcpuSoftAffinity)
+   for i, v := range cVcpuSoftAffinity {
+   var e Bitmap
+   if err := e.fromC(); err != nil {
+   return err
+   }
+   

[Xen-devel] [PATCH 14/24] golang/xenlight: generate structs from the IDL

[Nick Rosbrook]

From: Nick Rosbrook 

Add struct and keyed union generation to gengotypes.py. For keyed unions,
use a method similar to gRPC's oneof to interpret C unions as Go types.
Meaning, for a given struct with a union field, generate a struct for
each sub-struct defined in the union. Then, define an interface of one
method which is implemented by each of the defined sub-structs. For
example:

  type domainBuildInfoTypeUnion interface {
  isdomainBuildInfoTypeUnion()
  }

  type DomainBuildInfoTypeUnionHvm struct {
  // HVM-specific fields...
  }

  func (x DomainBuildInfoTypeUnionHvm) isdomainBuildInfoTypeUnion() {}

  type DomainBuildInfoTypeUnionPv struct {
  // PV-specific fields...
  }

  func (x DomainBuildInfoTypeUnionPv) isdomainBuildInfoTypeUnion() {}

  type DomainBuildInfoTypeUnionPvh struct {
  // PVH-specific fields...
  }

  func (x DomainBuildInfoTypeUnionPvh) isdomainBuildInfoTypeUnion() {}

Then, remove existing struct definitions in xenlight.go that conflict
with the generated types, and modify existing marshaling functions to
align with the new type definitions. Notably, drop "time" package since
fields of type time.Duration are now of type uint64.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py | 103 +++
 tools/golang/xenlight/xenlight.go   | 123 +---
 tools/golang/xenlight/xenlight_types.go | 834 
 3 files changed, 952 insertions(+), 108 deletions(-)

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
index 59307492cb..c8513b79e0 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -18,6 +18,10 @@ builtin_type_names = {
 idl.uint64.typename: 'uint64',
 }
 
+# List of strings that need to be written to a file
+# after a struct definition.
+type_extras = []
+
 def xenlight_golang_generate_types(path = None, types = None, comment = None):
 """
 Generate a .go file (xenlight_types.go by default)
@@ -35,6 +39,13 @@ def xenlight_golang_generate_types(path = None, types = 
None, comment = None):
 f.write(xenlight_golang_type_define(ty))
 f.write('\n')
 
+# Append extra types
+for extra in type_extras:
+f.write(extra)
+f.write('\n')
+
+del type_extras[:]
+
 go_fmt(path)
 
 def xenlight_golang_type_define(ty = None):
@@ -43,6 +54,9 @@ def xenlight_golang_type_define(ty = None):
 if isinstance(ty, idl.Enumeration):
 s += xenlight_golang_define_enum(ty)
 
+elif isinstance(ty, idl.Aggregate):
+s += xenlight_golang_define_struct(ty)
+
 return s
 
 def xenlight_golang_define_enum(ty = None):
@@ -65,6 +79,95 @@ def xenlight_golang_define_enum(ty = None):
 
 return s
 
+def xenlight_golang_define_struct(ty = None, typename = None, nested = False):
+s = ''
+name = ''
+
+if typename is not None:
+name = xenlight_golang_fmt_name(typename)
+else:
+name = xenlight_golang_fmt_name(ty.typename)
+
+# Begin struct definition
+if nested:
+s += '{} struct {{\n'.format(name)
+else:
+s += 'type {} struct {{\n'.format(name)
+
+# Write struct fields
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+typename = f.type.elem_type.typename
+typename = xenlight_golang_fmt_name(typename)
+name = xenlight_golang_fmt_name(f.name)
+
+s += '{} []{}\n'.format(name, typename)
+else:
+typename = f.type.typename
+typename = xenlight_golang_fmt_name(typename)
+name = xenlight_golang_fmt_name(f.name)
+
+s += '{} {}\n'.format(name, typename)
+
+elif isinstance(f.type, idl.Struct):
+s += xenlight_golang_define_struct(f.type, typename=f.name, 
nested=True)
+
+elif isinstance(f.type, idl.KeyedUnion):
+s += xenlight_golang_define_union(f.type, ty.typename)
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+# End struct definition
+s += '}\n'
+
+return s
+
+def xenlight_golang_define_union(ty = None, structname = ''):
+"""
+Generate the Go translation of a KeyedUnion.
+
+Define an unexported interface to be used as
+the type of the union. Then, define a struct
+for each field of the union which implements
+that interface.
+"""
+s = ''
+
+interface_name = '{}_{}_union'.format(structname, ty.keyvar.name)
+interface_name = xenlight_golang_fmt_name(interface_name, exported=False)
+
+s += 'type {} interface {{\n'.format(interface_name)
+s += 'is{}()\n'.format(interface_name)
+s += '}\n'
+
+type_extras.append(s)
+
+for f in ty.fields:
+if f.type is None:
+continue
+
+# 

[Xen-devel] [PATCH 15/24] golang/xenlight: remove no-longer used type MemKB

[Nick Rosbrook]

From: Nick Rosbrook 

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 0adb12d1bf..f91c0d2be2 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -83,8 +83,6 @@ type Domid uint32
 // Devid is a device ID.
 type Devid int
 
-type MemKB uint64
-
 // Uuid is a domain UUID.
 type Uuid [16]byte
 
-- 
2.19.1


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

[Xen-devel] [PATCH 13/24] golang/xenlight: re-factor Hwcap type implementation

[Nick Rosbrook]

From: Nick Rosbrook 

Re-define Hwcap as [8]uint32, and implement toC function. Also, re-name and
modify signature of toGo function to fromC.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 29 +++--
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 3e3753f92e..8d520bbd98 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -307,20 +307,29 @@ type Context struct {
logger *C.xentoollog_logger_stdiostream
 }
 
-type Hwcap []C.uint32_t
-
-func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
-   // Alloc a Go slice for the bytes
-   size := 8
-   ghwcap = make([]C.uint32_t, size)
+// Hwcap represents a libxl_hwcap.
+type Hwcap [8]uint32
 
+func (hwcap *Hwcap) fromC(chwcap *C.libxl_hwcap) error {
// Make a slice pointing to the C array
-   mapslice := (*[1 << 
30]C.uint32_t)(unsafe.Pointer([0]))[:size:size]
+   mapslice := (*[8]C.uint32_t)(unsafe.Pointer(chwcap))
 
// And copy the C array into the Go array
-   copy(ghwcap, mapslice)
+   for i, v := range mapslice {
+   hwcap[i] = uint32(v)
+   }
 
-   return
+   return nil
+}
+
+func (hwcap *Hwcap) toC() (C.libxl_hwcap, error) {
+   var chwcap C.libxl_hwcap
+
+   for i, v := range hwcap {
+   chwcap[i] = C.uint32_t(v)
+   }
+
+   return chwcap, nil
 }
 
 // KeyValueList represents a libxl_key_value_list.
@@ -465,7 +474,7 @@ func (cphys *C.libxl_physinfo) toGo() (physinfo *Physinfo) {
physinfo.SharingFreedPages = uint64(cphys.sharing_freed_pages)
physinfo.SharingUsedFrames = uint64(cphys.sharing_used_frames)
physinfo.NrNodes = uint32(cphys.nr_nodes)
-   physinfo.HwCap = cphys.hw_cap.toGo()
+   physinfo.HwCap.fromC(_cap)
physinfo.CapHvm = bool(cphys.cap_hvm)
physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)
 
-- 
2.19.1


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

[Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type

[Nick Rosbrook]

From: Nick Rosbrook 

Define CpuidPolicyList as a wrapper struct with field val of type
*C.libxl_cpuid_policy_list and implement fromC and toC functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 20 
 1 file changed, 20 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index d41de253f3..9c384485e1 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -249,6 +249,26 @@ type EvLink struct{}
 func (el *EvLink) fromC(cel *C.libxl_ev_link) error  { return nil }
 func (el *EvLink) toC() (cel C.libxl_ev_link, err error) { return }
 
+// CpuidPolicyList represents a libxl_cpuid_policy_list.
+type CpuidPolicyList struct {
+   val *C.libxl_cpuid_policy_list
+}
+
+func (cpl *CpuidPolicyList) fromC(ccpl *C.libxl_cpuid_policy_list) error {
+   cpl.val = ccpl
+   return nil
+}
+
+func (cpl *CpuidPolicyList) toC() (C.libxl_cpuid_policy_list, error) {
+   if cpl.val == nil {
+   var c C.libxl_cpuid_policy_list
+   return c, nil
+   }
+   ccpl := (*C.libxl_cpuid_policy_list)(unsafe.Pointer(cpl.val))
+
+   return *ccpl, nil
+}
+
 type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
-- 
2.19.1


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

[Xen-devel] [PATCH 08/24] golang/xenlight: define Mac builtin type

[Nick Rosbrook]

From: Nick Rosbrook 

Define Mac as [6]byte and implement fromC, toC, and String functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index a3a1836d31..3b7824b284 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -181,6 +181,41 @@ func (d *Defbool) toC() (C.libxl_defbool, error) {
return c, nil
 }
 
+// Mac represents a libxl_mac, or simply a MAC address.
+type Mac [6]byte
+
+// String formats a Mac address to string representation.
+func (mac Mac) String() string {
+   s := "%x:%x:%x:%x:%x:%x"
+   opts := make([]interface{}, 6)
+
+   for i, v := range mac {
+   opts[i] = v
+   }
+
+   return fmt.Sprintf(s, opts...)
+}
+
+func (mac *Mac) fromC(cmac *C.libxl_mac) error {
+   b := (*[6]C.uint8_t)(unsafe.Pointer(cmac))
+
+   for i, v := range b {
+   mac[i] = byte(v)
+   }
+
+   return nil
+}
+
+func (mac *Mac) toC() (C.libxl_mac, error) {
+   var cmac C.libxl_mac
+
+   for i, v := range mac {
+   cmac[i] = C.uint8_t(v)
+   }
+
+   return cmac, nil
+}
+
 type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
-- 
2.19.1


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

[Xen-devel] [PATCH 12/24] golang/xenlight: re-factor Uuid type implementation

[Nick Rosbrook]

From: Nick Rosbrook 

Re-define Uuid as [16]byte and implement fromC, toC, and String functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 37 +--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 9c384485e1..3e3753f92e 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -86,7 +86,40 @@ type Devid int
 
 type MemKB uint64
 
-type Uuid C.libxl_uuid
+// Uuid is a domain UUID.
+type Uuid [16]byte
+
+// String formats a Uuid in the form "-xx-xx-xx-xx".
+func (u Uuid) String() string {
+   s := "%x%x%x%x-%x%x-%x%x-%x%x-%x%x%x%x%x%x"
+   opts := make([]interface{}, 16)
+
+   for i, v := range u {
+   opts[i] = v
+   }
+
+   return fmt.Sprintf(s, opts...)
+}
+
+func (u *Uuid) fromC(c *C.libxl_uuid) error {
+   b := (*[16]C.uint8_t)(unsafe.Pointer([0]))
+
+   for i, v := range b {
+   u[i] = byte(v)
+   }
+
+   return nil
+}
+
+func (u *Uuid) toC() (C.libxl_uuid, error) {
+   var c C.libxl_uuid
+
+   for i, v := range u {
+   c.uuid[i] = C.uint8_t(v)
+   }
+
+   return c, nil
+}
 
 // defboolVal represents a defbool value.
 type defboolVal int
@@ -516,7 +549,7 @@ type Dominfo struct {
 func (cdi *C.libxl_dominfo) toGo() (di *Dominfo) {
 
di = {}
-   di.Uuid = Uuid(cdi.uuid)
+   di.Uuid.fromC()
di.Domid = Domid(cdi.domid)
di.Ssidref = uint32(cdi.ssidref)
di.SsidLabel = C.GoString(cdi.ssid_label)
-- 
2.19.1


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

[Xen-devel] [PATCH 10/24] golang/xenlight: define EvLink builtin as empty struct

[Nick Rosbrook]

From: Nick Rosbrook 

Define EvLink as empty struct as there is currently no reason the internal of
this type should be used in Go.

Implement fromC and toC functions as no-ops.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 6aca5b89c0..d41de253f3 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -239,6 +239,16 @@ func (mvg *MsVmGenid) toC() (C.libxl_ms_vm_genid, error) {
return cmvg, nil
 }
 
+// EvLink represents a libxl_ev_link.
+//
+// Represented as an empty struct for now, as there is no
+// apparent need for the internals of this type to be exposed
+// through the Go package.
+type EvLink struct{}
+
+func (el *EvLink) fromC(cel *C.libxl_ev_link) error  { return nil }
+func (el *EvLink) toC() (cel C.libxl_ev_link, err error) { return }
+
 type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
-- 
2.19.1


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

[Xen-devel] [PATCH 07/24] golang/xenlight: define StringList builtin type

[Nick Rosbrook]

From: Nick Rosbrook 

Define StringList as []string an implement fromC and toC functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 29 +
 1 file changed, 29 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 09fcdca5d1..a3a1836d31 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -234,6 +234,35 @@ func (kvl KeyValueList) toC() (C.libxl_key_value_list, 
error) {
return ckvl, nil
 }
 
+// StringList represents a libxl_string_list.
+type StringList []string
+
+func (sl StringList) fromC(csl *C.libxl_string_list) error {
+   size := int(C.libxl_string_list_length(csl))
+   list := (*[1 << 30]*C.char)(unsafe.Pointer(csl))[:size:size]
+
+   sl = make([]string, size)
+
+   for i, v := range list {
+   sl[i] = C.GoString(v)
+   }
+
+   return nil
+}
+
+func (sl StringList) toC() (C.libxl_string_list, error) {
+   var char *C.char
+   size := len(sl)
+   csl := (C.libxl_string_list)(C.malloc(C.ulong(size) * 
C.ulong(unsafe.Sizeof(char
+   clist := (*[1 << 30]*C.char)(unsafe.Pointer(csl))[:size:size]
+
+   for i, v := range sl {
+   clist[i] = C.CString(v)
+   }
+
+   return csl, nil
+}
+
 // Bitmap represents a libxl_bitmap.
 //
 // Implement the Go bitmap type such that the underlying data can
-- 
2.19.1


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

[Xen-devel] [PATCH 05/24] golang/xenlight: define KeyValueList builtin type

[Nick Rosbrook]

From: Nick Rosbrook 

Define KeyValueList builtin type, analagous to libxl_key_value_list as
map[string]string, and implement its fromC and toC functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 33 ++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 4d4fad2a9d..8196a42855 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -202,11 +202,42 @@ func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
return
 }
 
+// KeyValueList represents a libxl_key_value_list.
+type KeyValueList map[string]string
+
+func (kvl KeyValueList) fromC(ckvl *C.libxl_key_value_list) error {
+   size := int(C.libxl_key_value_list_length(ckvl))
+   list := (*[1 << 30]*C.char)(unsafe.Pointer(ckvl))[:size:size]
+
+   for i := 0; i < size*2; i += 2 {
+   kvl[C.GoString(list[i])] = C.GoString(list[i+1])
+   }
+
+   return nil
+}
+
+func (kvl KeyValueList) toC() (C.libxl_key_value_list, error) {
+   // Add extra slot for sentinel
+   var char *C.char
+   csize := 2*len(kvl) + 1
+   ckvl := (C.libxl_key_value_list)(C.malloc(C.ulong(csize) * 
C.ulong(unsafe.Sizeof(char
+   clist := (*[1 << 31]*C.char)(unsafe.Pointer(ckvl))[:csize:csize]
+
+   i := 0
+   for k, v := range kvl {
+   clist[i] = C.CString(k)
+   clist[i+1] = C.CString(v)
+   i += 2
+   }
+   clist[len(clist)-1] = nil
+
+   return ckvl, nil
+}
+
 // typedef struct {
 // uint32_t size;  /* number of bytes in map */
 // uint8_t *map;
 // } libxl_bitmap;
-
 // Implement the Go bitmap type such that the underlying data can
 // easily be copied in and out.  NB that we still have to do copies
 // both directions, because cgo runtime restrictions forbid passing to
-- 
2.19.1


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

[Xen-devel] [PATCH 03/24] golang/xenlight: define Defbool builtin type

[Nick Rosbrook]

From: Nick Rosbrook 

Define Defbool as struct analagous to the C type, and define the type
'defboolVal' that represent true, false, and default defbool values.

Implement Set, Unset, SetIfDefault, IsDefault, Val, and String functions
on Defbool so that the type can be used in Go analagously to how its
used in C.

Finally, implement fromC and toC functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 93 +++
 1 file changed, 93 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index e617f22fcf..7bf16dc03b 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -85,6 +85,99 @@ type MemKB uint64
 
 type Uuid C.libxl_uuid
 
+// defboolVal represents a defbool value.
+type defboolVal int
+
+const (
+   defboolDefault defboolVal = 0
+   defboolFalse   defboolVal = -1
+   defboolTruedefboolVal = 1
+)
+
+// Defbool represents a libxl_defbool.
+type Defbool struct {
+   val defboolVal
+}
+
+func (d Defbool) String() string {
+   switch d.val {
+   case defboolDefault:
+   return ""
+   case defboolFalse:
+   return "False"
+   case defboolTrue:
+   return "True"
+   }
+
+   return ""
+}
+
+// Set sets the value of the Defbool.
+func (d *Defbool) Set(b bool) {
+   if b {
+   d.val = defboolTrue
+   return
+   }
+   d.val = defboolFalse
+}
+
+// Unset resets the Defbool to default value.
+func (d *Defbool) Unset() {
+   d.val = defboolDefault
+}
+
+// SetIfDefault sets the value of Defbool only if
+// its current value is default.
+func (d *Defbool) SetIfDefault(b bool) {
+   if d.IsDefault() {
+   d.Set(b)
+   }
+}
+
+// IsDefault returns true if the value of Defbool
+// is default, returns false otherwise.
+func (d *Defbool) IsDefault() bool {
+   return d.val == defboolDefault
+}
+
+// Val returns the boolean value associated with the
+// Defbool value. An error is returned if the value
+// is default.
+func (d *Defbool) Val() (bool, error) {
+   if d.IsDefault() {
+   return false, fmt.Errorf("%v: cannot take value of default 
defbool", ErrorInval)
+   }
+
+   return (d.val > 0), nil
+}
+
+func (d *Defbool) fromC(c *C.libxl_defbool) error {
+   if C.libxl_defbool_is_default(*c) {
+   d.val = defboolDefault
+   return nil
+   }
+
+   if C.libxl_defbool_val(*c) {
+   d.val = defboolTrue
+   return nil
+   }
+
+   d.val = defboolFalse
+
+   return nil
+}
+
+func (d *Defbool) toC() (C.libxl_defbool, error) {
+   var c C.libxl_defbool
+
+   if !d.IsDefault() {
+   val, _ := d.Val()
+   C.libxl_defbool_set(, C.bool(val))
+   }
+
+   return c, nil
+}
+
 type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
-- 
2.19.1


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

[Xen-devel] [PATCH 00/24] generated Go libxl bindings using IDL

[Nick Rosbrook]

From: Nick Rosbrook 

After Xen summit, we started the discussion in this[1] RFC thread
to figure out how to generate Go bindings for libxl. This series
implements that Go code generation using the existing IDL, and updates
the existing hand-written code in xenlight.go to use the generated
code.

The goal of this series is to provide a good foundation for continued
development of the Go package.

These patches can also be found on my GitHub branch[2].

[1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02259.html
[2] https://github.com/enr0n/xen/tree/golang-patches-v1

Nick Rosbrook (24):
  golang/xenlight: fix calls to libxl_domain_unpause/pause
  golang/xenlight: generate enum types from IDL
  golang/xenlight: define Defbool builtin type
  golang/xenlight: define Devid type as int
  golang/xenlight: define KeyValueList builtin type
  golang/xenlight: re-name Bitmap marshaling functions
  golang/xenlight: define StringList builtin type
  golang/xenlight: define Mac builtin type
  golang/xenlight: define MsVmGenid builtin type
  golang/xenlight: define EvLink builtin as empty struct
  golang/xenlight: define CpuidPolicyList builtin type
  golang/xenlight: re-factor Uuid type implementation
  golang/xenlight: re-factor Hwcap type implementation
  golang/xenlight: generate structs from the IDL
  golang/xenlight: remove no-longer used type MemKB
  golang/xenlight: begin C to Go type marshaling
  golang/xenlight: implement keyed union C to Go marshaling
  golang/xenlight: implement array C to Go marshaling
  golang/xenlight: begin Go to C type marshaling
  golang/xenlight: implement keyed union Go to C marshaling
  golang/xenlight: implement array Go to C marshaling
  golang/xenlight: revise use of Context type
  golang/xenlight: add error return type to Context.Cpupoolinfo
  golang/xenlight: add make target for generated files

 tools/golang/xenlight/Makefile|   22 +-
 tools/golang/xenlight/gengotypes.py   |  698 +
 tools/golang/xenlight/xenlight.go |  928 +++---
 tools/golang/xenlight/xenlight_helpers.go | 3404 +
 tools/golang/xenlight/xenlight_types.go   | 1212 
 5 files changed, 5727 insertions(+), 537 deletions(-)
 create mode 100644 tools/golang/xenlight/gengotypes.py
 create mode 100644 tools/golang/xenlight/xenlight_helpers.go
 create mode 100644 tools/golang/xenlight/xenlight_types.go

Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 
-- 
2.19.1


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

[Xen-devel] [PATCH 09/24] golang/xenlight: define MsVmGenid builtin type

[Nick Rosbrook]

From: Nick Rosbrook 

Define MsVmGenid as [int(C.LIBXL_MS_VM_GENID_LEN)]byte and implement fromC and 
toC functions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 3b7824b284..6aca5b89c0 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -216,6 +216,29 @@ func (mac *Mac) toC() (C.libxl_mac, error) {
return cmac, nil
 }
 
+// MsVmGenid represents a libxl_ms_vm_genid.
+type MsVmGenid [int(C.LIBXL_MS_VM_GENID_LEN)]byte
+
+func (mvg *MsVmGenid) fromC(cmvg *C.libxl_ms_vm_genid) error {
+   b := 
(*[int(C.LIBXL_MS_VM_GENID_LEN)]C.uint8_t)(unsafe.Pointer([0]))
+
+   for i, v := range b {
+   mvg[i] = byte(v)
+   }
+
+   return nil
+}
+
+func (mvg *MsVmGenid) toC() (C.libxl_ms_vm_genid, error) {
+   var cmvg C.libxl_ms_vm_genid
+
+   for i, v := range mvg {
+   cmvg.bytes[i] = C.uint8_t(v)
+   }
+
+   return cmvg, nil
+}
+
 type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
-- 
2.19.1


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

[Xen-devel] [PATCH 02/24] golang/xenlight: generate enum types from IDL

[Nick Rosbrook]

From: Nick Rosbrook 

Introduce gengotypes.py to generate Go code the from IDL. As a first step,
implement 'enum' type generation.

As a result of the newly-generated code, remove the existing, and now
conflicting definitions in xenlight.go. In the case of the Error type,
rename the slice 'errors' to 'libxlErrors' so that it does not conflict
with the standard library package 'errors.' And, negate the values used
in 'libxlErrors' since the generated error values are negative.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/gengotypes.py | 109 +++
 tools/golang/xenlight/xenlight.go   | 140 ++---
 tools/golang/xenlight/xenlight_types.go | 378 
 3 files changed, 515 insertions(+), 112 deletions(-)
 create mode 100644 tools/golang/xenlight/gengotypes.py
 create mode 100644 tools/golang/xenlight/xenlight_types.go

diff --git a/tools/golang/xenlight/gengotypes.py 
b/tools/golang/xenlight/gengotypes.py
new file mode 100644
index 00..59307492cb
--- /dev/null
+++ b/tools/golang/xenlight/gengotypes.py
@@ -0,0 +1,109 @@
+#!/usr/bin/python
+
+import os
+import sys
+
+sys.path.append('{}/tools/libxl'.format(os.environ['XEN_ROOT']))
+import idl
+
+# Go versions of some builtin types.
+# Append the libxl-defined builtins after IDL parsing.
+builtin_type_names = {
+idl.bool.typename: 'bool',
+idl.string.typename: 'string',
+idl.integer.typename: 'int',
+idl.uint8.typename: 'byte',
+idl.uint16.typename: 'uint16',
+idl.uint32.typename: 'uint32',
+idl.uint64.typename: 'uint64',
+}
+
+def xenlight_golang_generate_types(path = None, types = None, comment = None):
+"""
+Generate a .go file (xenlight_types.go by default)
+that contains a Go type for each type in types.
+"""
+if path is None:
+path = 'xenlight_types.go'
+
+with open(path, 'w') as f:
+if comment is not None:
+f.write(comment)
+f.write('package xenlight\n')
+
+for ty in types:
+f.write(xenlight_golang_type_define(ty))
+f.write('\n')
+
+go_fmt(path)
+
+def xenlight_golang_type_define(ty = None):
+s = ''
+
+if isinstance(ty, idl.Enumeration):
+s += xenlight_golang_define_enum(ty)
+
+return s
+
+def xenlight_golang_define_enum(ty = None):
+s = ''
+typename = ''
+
+if ty.typename is not None:
+typename = xenlight_golang_fmt_name(ty.typename)
+s += 'type {} int\n'.format(typename)
+
+# Start const block
+s += 'const(\n'
+
+for v in ty.values:
+name = xenlight_golang_fmt_name(v.name)
+s += '{} {} = {}\n'.format(name, typename, v.value)
+
+# End const block
+s += ')\n'
+
+return s
+
+def xenlight_golang_fmt_name(name, exported = True):
+"""
+Take a given type name and return an
+appropriate Go type name.
+"""
+if name in builtin_type_names.keys():
+return builtin_type_names[name]
+
+# Name is not a builtin, format it for Go.
+words = name.split('_')
+
+# Remove 'libxl' prefix
+if words[0].lower() == 'libxl':
+words.remove(words[0])
+
+if exported:
+return ''.join(x.title() for x in words)
+
+return words[0] + ''.join(x.title() for x in words[1:])
+
+def go_fmt(path):
+""" Call go fmt on the given path. """
+os.system('go fmt {}'.format(path))
+
+if __name__ == '__main__':
+idlname = sys.argv[1]
+
+(builtins, types) = idl.parse(idlname)
+
+for b in builtins:
+name = b.typename
+builtin_type_names[name] = xenlight_golang_fmt_name(name)
+
+header_comment="""// DO NOT EDIT.
+//
+// This file is generated by:
+// {}
+//
+""".format(' '.join(sys.argv))
+
+xenlight_golang_generate_types(types=types,
+   comment=header_comment)
diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 59b8186a64..e617f22fcf 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -37,77 +37,42 @@ import (
"unsafe"
 )
 
-/*
- * Errors
- */
-
-type Error int
-
-const (
-   ErrorNonspecific  = Error(-C.ERROR_NONSPECIFIC)
-   ErrorVersion  = Error(-C.ERROR_VERSION)
-   ErrorFail = Error(-C.ERROR_FAIL)
-   ErrorNi   = Error(-C.ERROR_NI)
-   ErrorNomem= Error(-C.ERROR_NOMEM)
-   ErrorInval= Error(-C.ERROR_INVAL)
-   ErrorBadfail  = Error(-C.ERROR_BADFAIL)
-   ErrorGuestTimedout= Error(-C.ERROR_GUEST_TIMEDOUT)
-   ErrorTimedout = Error(-C.ERROR_TIMEDOUT)
-   ErrorNoparavirt   = Error(-C.ERROR_NOPARAVIRT)
-   ErrorNotReady = Error(-C.ERROR_NOT_READY)
-   ErrorOseventRegFail   = 

[Xen-devel] [PATCH 06/24] golang/xenlight: re-name Bitmap marshaling functions

[Nick Rosbrook]

From: Nick Rosbrook 

Re-name and modify signature of toGo function to fromC. The reason for
using 'fromC' rather than 'toGo' is that it is not a good idea to define
methods on the C types. Also, add error return type to Bitmap's toC function.

Finally, as code-cleanup, re-organize the Bitmap type's comments as per
Go conventions.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 93 ---
 1 file changed, 48 insertions(+), 45 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 8196a42855..09fcdca5d1 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -234,19 +234,48 @@ func (kvl KeyValueList) toC() (C.libxl_key_value_list, 
error) {
return ckvl, nil
 }
 
-// typedef struct {
-// uint32_t size;  /* number of bytes in map */
-// uint8_t *map;
-// } libxl_bitmap;
+// Bitmap represents a libxl_bitmap.
+//
 // Implement the Go bitmap type such that the underlying data can
 // easily be copied in and out.  NB that we still have to do copies
 // both directions, because cgo runtime restrictions forbid passing to
 // a C function a pointer to a Go-allocated structure which contains a
 // pointer.
 type Bitmap struct {
+   // typedef struct {
+   // uint32_t size;  /* number of bytes in map */
+   // uint8_t *map;
+   // } libxl_bitmap;
bitmap []C.uint8_t
 }
 
+func (bm *Bitmap) fromC(cbm *C.libxl_bitmap) error {
+   // Alloc a Go slice for the bytes
+   size := int(cbm.size)
+   bm.bitmap = make([]C.uint8_t, size)
+
+   // Make a slice pointing to the C array
+   mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+   // And copy the C array into the Go array
+   copy(bm.bitmap, mapslice)
+
+   return nil
+}
+
+func (bm *Bitmap) toC() (C.libxl_bitmap, error) {
+   var cbm C.libxl_bitmap
+
+   size := len(bm.bitmap)
+   cbm.size = C.uint32_t(size)
+   cbm._map = (*C.uint8_t)(C.malloc(C.ulong(cbm.size) * C.sizeof_uint8_t))
+   cslice := (*[1 << 31]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+   copy(cslice, bm.bitmap)
+
+   return cbm, nil
+}
+
 /*
  * Types: IDL
  *
@@ -447,7 +476,7 @@ func (cci C.libxl_cpupoolinfo) toGo() (gci CpupoolInfo) {
gci.PoolName = C.GoString(cci.pool_name)
gci.Scheduler = Scheduler(cci.sched)
gci.DomainCount = int(cci.n_dom)
-   gci.Cpumap = cci.cpumap.toGo()
+   gci.Cpumap.fromC()
 
return
 }
@@ -521,7 +550,10 @@ func (Ctx *Context) CpupoolCreate(Name string, Scheduler 
Scheduler, Cpumap Bitma
var uuid C.libxl_uuid
C.libxl_uuid_generate()
 
-   cbm := Cpumap.toC()
+   cbm, err := Cpumap.toC()
+   if err != nil {
+   return
+   }
defer C.libxl_bitmap_dispose()
 
ret := C.libxl_cpupool_create(Ctx.ctx, name, 
C.libxl_scheduler(Scheduler),
@@ -576,7 +608,10 @@ func (Ctx *Context) CpupoolCpuaddCpumap(Poolid uint32, 
Cpumap Bitmap) (err error
return
}
 
-   cbm := Cpumap.toC()
+   cbm, err := Cpumap.toC()
+   if err != nil {
+   return
+   }
defer C.libxl_bitmap_dispose()
 
ret := C.libxl_cpupool_cpuadd_cpumap(Ctx.ctx, C.uint32_t(Poolid), )
@@ -612,7 +647,10 @@ func (Ctx *Context) CpupoolCpuremoveCpumap(Poolid uint32, 
Cpumap Bitmap) (err er
return
}
 
-   cbm := Cpumap.toC()
+   cbm, err := Cpumap.toC()
+   if err != nil {
+   return
+   }
defer C.libxl_bitmap_dispose()
 
ret := C.libxl_cpupool_cpuremove_cpumap(Ctx.ctx, C.uint32_t(Poolid), 
)
@@ -735,41 +773,6 @@ func (Ctx *Context) CpupoolMakeFree(Cpumap Bitmap) (err 
error) {
  * Bitmap operations
  */
 
-// Return a Go bitmap which is a copy of the referred C bitmap.
-func (cbm C.libxl_bitmap) toGo() (gbm Bitmap) {
-   // Alloc a Go slice for the bytes
-   size := int(cbm.size)
-   gbm.bitmap = make([]C.uint8_t, size)
-
-   // Make a slice pointing to the C array
-   mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
-
-   // And copy the C array into the Go array
-   copy(gbm.bitmap, mapslice)
-
-   return
-}
-
-// Must be C.libxl_bitmap_dispose'd of afterwards
-func (gbm Bitmap) toC() (cbm C.libxl_bitmap) {
-   C.libxl_bitmap_init()
-
-   size := len(gbm.bitmap)
-   cbm._map = (*C.uint8_t)(C.malloc(C.size_t(size)))
-   cbm.size = C.uint32_t(size)
-   if cbm._map == nil {
-   panic("C.calloc failed!")
-   }
-
-   // Make a slice pointing to the C array
-   mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
-
-   // And copy the Go array into the C array
-   copy(mapslice, gbm.bitmap)
-
-   return
-}
-
 func (bm *Bitmap) Test(bit int) bool {

[Xen-devel] [PATCH 01/24] golang/xenlight: fix calls to libxl_domain_unpause/pause

[Nick Rosbrook]

From: Nick Rosbrook 

These functions require a third argument of type const *libxl_asyncop_how.

Pass nil to fix compilation errors. This will have the effect of
performing these operations synchronously.

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index f5d171c2d5..59b8186a64 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -1011,7 +1011,7 @@ func (Ctx *Context) DomainUnpause(Id Domid) (err error) {
return
}
 
-   ret := C.libxl_domain_unpause(Ctx.ctx, C.uint32_t(Id))
+   ret := C.libxl_domain_unpause(Ctx.ctx, C.uint32_t(Id), nil)
 
if ret != 0 {
err = Error(-ret)
@@ -1026,7 +1026,7 @@ func (Ctx *Context) DomainPause(id Domid) (err error) {
return
}
 
-   ret := C.libxl_domain_pause(Ctx.ctx, C.uint32_t(id))
+   ret := C.libxl_domain_pause(Ctx.ctx, C.uint32_t(id), nil)
 
if ret != 0 {
err = Error(-ret)
-- 
2.19.1


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

[Xen-devel] [PATCH 04/24] golang/xenlight: define Devid type as int

[Nick Rosbrook]

From: Nick Rosbrook 

Signed-off-by: Nick Rosbrook 
---
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Wei Liu 

 tools/golang/xenlight/xenlight.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/golang/xenlight/xenlight.go 
b/tools/golang/xenlight/xenlight.go
index 7bf16dc03b..4d4fad2a9d 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -81,6 +81,9 @@ func (e Error) Error() string {
 
 type Domid uint32
 
+// Devid is a device ID.
+type Devid int
+
 type MemKB uint64
 
 type Uuid C.libxl_uuid
-- 
2.19.1


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

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

[osstest service owner]

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

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  4a647ad128a6e8ea91e9df140708d80548bf47f7
baseline version:
 xen  f4049b2a9850c847b06ec6ad1cec1c7c2c303b94

Last test of basis   142284  2019-10-04 19:00:47 Z2 days
Testing same since   142395  2019-10-07 10:01:06 Z0 days1 attempts


People who touched revisions under test:
  Daniel De Graaf 
  Julien Grall 

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
   f4049b2a98..4a647ad128  4a647ad128a6e8ea91e9df140708d80548bf47f7 -> smoke

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

[Xen-devel] [linux-4.4 test] 142381: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-pvshim   18 guest-localmigrate/x10   fail REGR. vs. 139698

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-pvshim   12 guest-start  fail in 142363 pass in 142381
 test-armhf-armhf-libvirt 16 guest-start/debian.repeat fail in 142363 pass in 
142381
 test-amd64-i386-xl-raw7 xen-boot   fail pass in 142363

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-xl-pvhv2-amd 12 guest-start  fail  never pass
 test-arm64-arm64-xl-xsm   7 xen-boot fail   never pass
 test-arm64-arm64-xl-credit1   7 xen-boot fail   never pass
 test-arm64-arm64-xl-credit2   7 xen-boot fail   never pass
 test-arm64-arm64-libvirt-xsm  7 xen-boot fail   never pass
 test-arm64-arm64-xl-seattle   7 xen-boot fail   never pass
 test-arm64-arm64-xl   7 xen-boot fail   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-amd64-xl-pvhv2-intel 12 guest-start fail 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-thunderx  7 xen-boot 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-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-amd64-amd64-xl-qemut-win7-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-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-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-qemut-win7-amd64 17 guest-stop  fail never pass
 test-arm64-arm64-examine  8 reboot   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-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-ws16-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-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-i386-xl-qemut-ws16-amd64 17 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass

version targeted for testing:
 linux

Re: [Xen-devel] [PATCH] xen/xenbus: fix self-deadlock after killing user process

[James Dingwall]

On Tue, Oct 01, 2019 at 01:37:24PM -0400, Boris Ostrovsky wrote:
> On 10/1/19 11:03 AM, Juergen Gross wrote:
> > In case a user process using xenbus has open transactions and is killed
> > e.g. via ctrl-C the following cleanup of the allocated resources might
> > result in a deadlock due to trying to end a transaction in the xenbus
> > worker thread:
> >
> > [ 2551.474706] INFO: task xenbus:37 blocked for more than 120 seconds.
> > [ 2551.492215]   Tainted: P   OE 5.0.0-29-generic #5
> > [ 2551.510263] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables 
> > this message.
> > [ 2551.528585] xenbus  D037  2 0x8080
> > [ 2551.528590] Call Trace:
> > [ 2551.528603]  __schedule+0x2c0/0x870
> > [ 2551.528606]  ? _cond_resched+0x19/0x40
> > [ 2551.528632]  schedule+0x2c/0x70
> > [ 2551.528637]  xs_talkv+0x1ec/0x2b0
> > [ 2551.528642]  ? wait_woken+0x80/0x80
> > [ 2551.528645]  xs_single+0x53/0x80
> > [ 2551.528648]  xenbus_transaction_end+0x3b/0x70
> > [ 2551.528651]  xenbus_file_free+0x5a/0x160
> > [ 2551.528654]  xenbus_dev_queue_reply+0xc4/0x220
> > [ 2551.528657]  xenbus_thread+0x7de/0x880
> > [ 2551.528660]  ? wait_woken+0x80/0x80
> > [ 2551.528665]  kthread+0x121/0x140
> > [ 2551.528667]  ? xb_read+0x1d0/0x1d0
> > [ 2551.528670]  ? kthread_park+0x90/0x90
> > [ 2551.528673]  ret_from_fork+0x35/0x40
> >
> > Fix this by doing the cleanup via a workqueue instead.
> >
> > Reported-by: James Dingwall 
> > Fixes: fd8aa9095a95c ("xen: optimize xenbus driver for multiple concurrent 
> > xenstore accesses")
> > Cc:  # 4.11
> > Signed-off-by: Juergen Gross 
> 
> Reviewed-by: Boris Ostrovsky 
> 

Tested-by: James Dingwall 

This patch does resolve the observed issue although for my (extreme and 
not representative of our normal workload) test case the worker still 
gets blocked for some time if the xenstore-rm is interrupted and no 
concurrent xenstore commands can run.  I assume that the worker 
completes the rm and then does a rollback in the background rather than 
being interrupted early as a result of the userspace program being 
terminated.

Thanks,
James

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

Re: [Xen-devel] [PATCH 1/4] docs/sphinx: License content with CC-BY-4.0

[Lars Kurth]

On 07/10/2019, 13:29, "Andrew Cooper"  wrote:

On 07/10/2019 13:01, Lars Kurth wrote:
>
> On 03/10/2019, 21:56, "Andrew Cooper"  wrote:
>
> Creative Commons is a more common license than GPL for documentation 
purposes.
> Switch to using CC-BY-4.0 to explicitly permit re-purposing and 
remixing of
> the content.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Lars Kurth 
> CC: George Dunlap 
> CC: Ian Jackson 
> CC: Jan Beulich 
> CC: Konrad Rzeszutek Wilk 
> CC: Stefano Stabellini 
> CC: Tim Deegan 
> CC: Wei Liu 
> CC: Julien Grall 
> CC: Rich Persaud 
> CC: Juergen Gross 
> ---
>  COPYING |  3 +++
>  docs/README.source  | 32 

>  docs/admin-guide/index.rst  |  2 ++
>  docs/admin-guide/microcode-loading.rst  |  2 ++
>  docs/conf.py|  1 +
>  docs/guest-guide/index.rst  |  2 ++
>  docs/guest-guide/x86/hypercall-abi.rst  |  2 ++
>  docs/guest-guide/x86/index.rst  |  2 ++
>  docs/hypervisor-guide/code-coverage.rst |  2 ++
>  docs/hypervisor-guide/index.rst |  2 ++
>  docs/index.rst  |  2 ++
>  11 files changed, 52 insertions(+)
>  create mode 100644 docs/README.source
> 
> diff --git a/COPYING b/COPYING
> index 310fd52c27..80fac091d3 100644
> --- a/COPYING
> +++ b/COPYING
> @@ -47,6 +47,9 @@ various drivers, support functions and header files 
within Xen-aware
>  Linux source trees. In all such cases, license terms are stated at 
the
>  top of the file or in a COPYING file in the same directory.
>  
> +Sphinx documentation is licensed under CC-BY 4.0.  See
> +docs/README.source for more specific information.
> +
>  In some cases, compatible 3rd party code has been imported into the
>  Xen tree, retaining the original license, such as
>- AES-128 3.0
> diff --git a/docs/README.source b/docs/README.source
> new file mode 100644
> index 00..f20fa92c28
> --- /dev/null
> +++ b/docs/README.source
> @@ -0,0 +1,32 @@
> +Sphinx documentation:
> +
> +All source rendered by Sphinx is licensed under CC-BY-4.0.
>
> Sorry for opening this can of worms. 
>
> Although I had seen the discussion between Rich and you about this, I had 
> not actually done any groundwork on the licensing. 
>
> So, we have to look at two things:
>
> * Compatibility:
>See 
https://creativecommons.org/2015/10/08/cc-by-sa-4-0-now-one-way-compatible-with-gplv3/
 
>This makes CC-BY-4.0 inbound compatible with GPLv3
>It's not clear to me whether GPLv2 is compatible with CC-BY 4.0: lack 
of publicly
>available information implies this is not the case 
>
> * Output License
>But even if it is, the produced sphinx output would be GPLv2, not 
CC-BY 4.0
>This would even be true if none of the older GPLv2 docs portions were 
included, as
>the API docs generated from source are GPLv2
>
> As such the statement "All source rendered by Sphinx is licensed under
> CC-BY-4.0" is wrong.

At the moment, I (and therefore Citrix in practice) holds the copyright
on all rst content which exists in Xen.

The point of this patch is to get it licensed sensibly (and in
particular, not falling back to the GPL default) before 4.13 goes out of
the door.

The result therefore is uniformly CC-BY-4.0, with no GPL in sight.

Having looked at this again, you are correct. I was assuming that 
https://andrewcoop-xen.readthedocs.io/en/docs-devel/guest-guide/x86/hypercall-abi.html
was linking to the doxygen generated ABI docs, which it seems not to do.

> Although it is probably correct to say "All CC-BY 4.0 source rendered by
> Sphinx is licensed under CC-BY-4.0", because Sphinx retains the source 
file
> to html mapping and linkage in docs generation works differently
> to linkage in code. 
>
> I am wondering whether anyone else has come across this. This question in
> particular goes back to Rich who made a very strong case for CC-BY-4.0 
based
> documentation. I don't think we would have an issue if the entire sphinx 
doc-set
> is GPLv2 if most content is licensed under CC-BY-4.0, except that such an
> approach would make re-using the entire sphinx generated docset messy.
>
> We probably also want to maintain the capability to copy text from some
> documentation freely into the source tree and vice versa, if needed. This 
is
> particularly true 

Re: [Xen-devel] [PATCH v3 08/10] vpci: register as an internal ioreq server

[Jan Beulich]

On 30.09.2019 15:32, Roger Pau Monne wrote:
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -29,6 +29,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 

This is the only change to this file, and there's no addition to
asm/hvm/ioreq.h - how come this #include is needed?

> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -20,6 +20,8 @@
>  #include 
>  #include 
>  
> +#include 

This of course means a step away from the code here being generic
enough such that Arm could eventually use it. Independent of this
aspect perhaps the #include would better move ...

> @@ -478,6 +480,61 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, 
> unsigned int size,
>  spin_unlock(>vpci->lock);
>  }
>  
> +#ifdef __XEN__

... here?

> +static int ioreq_handler(ioreq_t *req, void *data)
> +{
> +pci_sbdf_t sbdf;
> +
> +/*
> + * NB: certain requests of type different than PCI are broadcasted to all
> + * registered ioreq servers, ignored those.
> + */
> +if ( req->type != IOREQ_TYPE_PCI_CONFIG || req->data_is_ptr )
> +return X86EMUL_UNHANDLEABLE;

I understand the left side of the || , but why the right side? There
shouldn't be any IOREQ_TYPE_PCI_CONFIG requests with data_is_ptr set,
should there? I also didn't think requests with data_is_ptr set would
get broadcast?

> +int vpci_register_ioreq(struct domain *d)
> +{
> +ioservid_t id;
> +int rc;
> +
> +if ( !has_vpci(d) )
> +return 0;
> +
> +rc = hvm_create_ioreq_server(d, HVM_IOREQSRV_BUFIOREQ_OFF, , true);
> +if ( rc )
> +return rc;
> +
> +rc = hvm_set_ioreq_handler(d, id, ioreq_handler, NULL);
> +if ( rc )
> +return rc;
> +
> +if ( is_hardware_domain(d) )
> +{
> +/* Handle all devices in vpci. */
> +rc = hvm_map_io_range_to_ioreq_server(d, id, XEN_DMOP_IO_RANGE_PCI,
> +  0, ~(uint64_t)0);
> +if ( rc )
> +return rc;
> +}
> +
> +rc = hvm_set_ioreq_server_state(d, id, true);
> +if ( rc )
> +return rc;
> +
> +return rc;

This last sequence of statements looks a little odd - is this in
anticipation of further additions to the function?

Furthermore the only caller expects the function to clean up after
itself (i.e. partially completed setup be undone), which doesn't
look to be the case here. I can't seem to be able to convince
myself that all of this gets cleaned up.

Jan

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-freebsd10-i386  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-raw7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-examine   8 reboot   fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-libvirt   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-intel  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-freebsd10-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ovmf-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-qemut-rhel6hvm-amd  7 xen-boot   fail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-libvirt-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-xl-xsm7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl-qemut-debianhvm-amd64  7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-boot fail REGR. vs. 
133580
 test-amd64-i386-libvirt-xsm   7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-pvshim 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-ws16-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-xl7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win7-amd64  7 xen-boot  fail REGR. vs. 133580
 test-amd64-i386-pair 10 xen-boot/src_hostfail REGR. vs. 133580
 test-amd64-i386-pair 11 xen-boot/dst_hostfail REGR. vs. 133580
 test-amd64-i386-xl-shadow 7 xen-boot fail REGR. vs. 133580
 test-amd64-i386-xl-qemuu-win10-i386  7 xen-boot  fail REGR. vs. 133580
 test-arm64-arm64-examine11 examine-serial/bootloader fail REGR. vs. 133580
 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 133580
 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install  fail REGR. vs. 133580

Tests which did not succeed, but are not blocking:
 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-i386-xl-qemut-debianhvm-i386-xsm  7 xen-boot fail baseline untested
 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 133580
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 133580
 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580
 test-amd64-amd64-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-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-xl-thunderx 13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2  fail never pass
 test-arm64-arm64-xl-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  14 saverestore-support-checkfail   

Re: [Xen-devel] [PATCH 1/4] docs/sphinx: License content with CC-BY-4.0

[Andrew Cooper]

On 07/10/2019 13:01, Lars Kurth wrote:
>
> On 03/10/2019, 21:56, "Andrew Cooper"  wrote:
>
> Creative Commons is a more common license than GPL for documentation 
> purposes.
> Switch to using CC-BY-4.0 to explicitly permit re-purposing and remixing 
> of
> the content.
> 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Lars Kurth 
> CC: George Dunlap 
> CC: Ian Jackson 
> CC: Jan Beulich 
> CC: Konrad Rzeszutek Wilk 
> CC: Stefano Stabellini 
> CC: Tim Deegan 
> CC: Wei Liu 
> CC: Julien Grall 
> CC: Rich Persaud 
> CC: Juergen Gross 
> ---
>  COPYING |  3 +++
>  docs/README.source  | 32 
> 
>  docs/admin-guide/index.rst  |  2 ++
>  docs/admin-guide/microcode-loading.rst  |  2 ++
>  docs/conf.py|  1 +
>  docs/guest-guide/index.rst  |  2 ++
>  docs/guest-guide/x86/hypercall-abi.rst  |  2 ++
>  docs/guest-guide/x86/index.rst  |  2 ++
>  docs/hypervisor-guide/code-coverage.rst |  2 ++
>  docs/hypervisor-guide/index.rst |  2 ++
>  docs/index.rst  |  2 ++
>  11 files changed, 52 insertions(+)
>  create mode 100644 docs/README.source
> 
> diff --git a/COPYING b/COPYING
> index 310fd52c27..80fac091d3 100644
> --- a/COPYING
> +++ b/COPYING
> @@ -47,6 +47,9 @@ various drivers, support functions and header files 
> within Xen-aware
>  Linux source trees. In all such cases, license terms are stated at the
>  top of the file or in a COPYING file in the same directory.
>  
> +Sphinx documentation is licensed under CC-BY 4.0.  See
> +docs/README.source for more specific information.
> +
>  In some cases, compatible 3rd party code has been imported into the
>  Xen tree, retaining the original license, such as
>- AES-128 3.0
> diff --git a/docs/README.source b/docs/README.source
> new file mode 100644
> index 00..f20fa92c28
> --- /dev/null
> +++ b/docs/README.source
> @@ -0,0 +1,32 @@
> +Sphinx documentation:
> +
> +All source rendered by Sphinx is licensed under CC-BY-4.0.
>
> Sorry for opening this can of worms. 
>
> Although I had seen the discussion between Rich and you about this, I had 
> not actually done any groundwork on the licensing. 
>
> So, we have to look at two things:
>
> * Compatibility:
>See 
> https://creativecommons.org/2015/10/08/cc-by-sa-4-0-now-one-way-compatible-with-gplv3/
>  
>This makes CC-BY-4.0 inbound compatible with GPLv3
>It's not clear to me whether GPLv2 is compatible with CC-BY 4.0: lack of 
> publicly
>available information implies this is not the case 
>
> * Output License
>But even if it is, the produced sphinx output would be GPLv2, not CC-BY 4.0
>This would even be true if none of the older GPLv2 docs portions were 
> included, as
>the API docs generated from source are GPLv2
>
> As such the statement "All source rendered by Sphinx is licensed under
> CC-BY-4.0" is wrong.

At the moment, I (and therefore Citrix in practice) holds the copyright
on all rst content which exists in Xen.

The point of this patch is to get it licensed sensibly (and in
particular, not falling back to the GPL default) before 4.13 goes out of
the door.

The result therefore is uniformly CC-BY-4.0, with no GPL in sight.

> Although it is probably correct to say "All CC-BY 4.0 source rendered by
> Sphinx is licensed under CC-BY-4.0", because Sphinx retains the source file
> to html mapping and linkage in docs generation works differently
> to linkage in code. 
>
> I am wondering whether anyone else has come across this. This question in
> particular goes back to Rich who made a very strong case for CC-BY-4.0 based
> documentation. I don't think we would have an issue if the entire sphinx 
> doc-set
> is GPLv2 if most content is licensed under CC-BY-4.0, except that such an
> approach would make re-using the entire sphinx generated docset messy.
>
> We probably also want to maintain the capability to copy text from some
> documentation freely into the source tree and vice versa, if needed. This is
> particularly true for content in Technical Debt, user content (may end up in
> man pages), etc.

I disagree, insofar as blindly copying notes out of source code and into
the sphinx documentation is liable to get a -1 from me.

The types of style/language which are appropriate for these two cases
are a disjoint set.

>
> Maybe the right approach would be to dually license the documentation
> files using both GPLv2 and CC-BY 4.0 and quantifying this in the COPYING
> file of the docs directory (starting from a specific date). We could 
> eventually
> re-license all the other stuff over time, which should be relatively 
> straightforward
> and/or exclude specific problematic directories.

I 

Re: [Xen-devel] [PATCH 1/4] docs/sphinx: License content with CC-BY-4.0

[Lars Kurth]

On 03/10/2019, 21:56, "Andrew Cooper"  wrote:

Creative Commons is a more common license than GPL for documentation 
purposes.
Switch to using CC-BY-4.0 to explicitly permit re-purposing and remixing of
the content.

Signed-off-by: Andrew Cooper 
---
CC: Lars Kurth 
CC: George Dunlap 
CC: Ian Jackson 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 
CC: Julien Grall 
CC: Rich Persaud 
CC: Juergen Gross 
---
 COPYING |  3 +++
 docs/README.source  | 32 

 docs/admin-guide/index.rst  |  2 ++
 docs/admin-guide/microcode-loading.rst  |  2 ++
 docs/conf.py|  1 +
 docs/guest-guide/index.rst  |  2 ++
 docs/guest-guide/x86/hypercall-abi.rst  |  2 ++
 docs/guest-guide/x86/index.rst  |  2 ++
 docs/hypervisor-guide/code-coverage.rst |  2 ++
 docs/hypervisor-guide/index.rst |  2 ++
 docs/index.rst  |  2 ++
 11 files changed, 52 insertions(+)
 create mode 100644 docs/README.source

diff --git a/COPYING b/COPYING
index 310fd52c27..80fac091d3 100644
--- a/COPYING
+++ b/COPYING
@@ -47,6 +47,9 @@ various drivers, support functions and header files 
within Xen-aware
 Linux source trees. In all such cases, license terms are stated at the
 top of the file or in a COPYING file in the same directory.
 
+Sphinx documentation is licensed under CC-BY 4.0.  See
+docs/README.source for more specific information.
+
 In some cases, compatible 3rd party code has been imported into the
 Xen tree, retaining the original license, such as
   - AES-128 3.0
diff --git a/docs/README.source b/docs/README.source
new file mode 100644
index 00..f20fa92c28
--- /dev/null
+++ b/docs/README.source
@@ -0,0 +1,32 @@
+Sphinx documentation:
+
+All source rendered by Sphinx is licensed under CC-BY-4.0.

Sorry for opening this can of worms. 

Although I had seen the discussion between Rich and you about this, I had 
not actually done any groundwork on the licensing. 

So, we have to look at two things:

* Compatibility:
   See 
https://creativecommons.org/2015/10/08/cc-by-sa-4-0-now-one-way-compatible-with-gplv3/
 
   This makes CC-BY-4.0 inbound compatible with GPLv3
   It's not clear to me whether GPLv2 is compatible with CC-BY 4.0: lack of 
publicly
   available information implies this is not the case 

* Output License
   But even if it is, the produced sphinx output would be GPLv2, not CC-BY 4.0
   This would even be true if none of the older GPLv2 docs portions were 
included, as
   the API docs generated from source are GPLv2

As such the statement "All source rendered by Sphinx is licensed under
CC-BY-4.0" is wrong. 

Although it is probably correct to say "All CC-BY 4.0 source rendered by
Sphinx is licensed under CC-BY-4.0", because Sphinx retains the source file
to html mapping and linkage in docs generation works differently
to linkage in code. 

I am wondering whether anyone else has come across this. This question in
particular goes back to Rich who made a very strong case for CC-BY-4.0 based
documentation. I don't think we would have an issue if the entire sphinx doc-set
is GPLv2 if most content is licensed under CC-BY-4.0, except that such an
approach would make re-using the entire sphinx generated docset messy.

We probably also want to maintain the capability to copy text from some
documentation freely into the source tree and vice versa, if needed. This is
particularly true for content in Technical Debt, user content (may end up in
man pages), etc.

Maybe the right approach would be to dually license the documentation
files using both GPLv2 and CC-BY 4.0 and quantifying this in the COPYING
file of the docs directory (starting from a specific date). We could eventually
re-license all the other stuff over time, which should be relatively 
straightforward
and/or exclude specific problematic directories.

Things like standardising say man pages to rst, would potentially also
create complexities with this patch, because of 
+This includes:
+  * All ReStructured Text files:  docs/*/*.rst

I don't want this to become a long-winded conversation during the 4.13 freeze.
Please keep this in mind when responding.

It may mean though, that we can't resolve this before 4.13 is released

Regards
Lars

 

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

Re: [Xen-devel] [PATCH v3 07/10] ioreq: allow decoding accesses to MMCFG regions

[Jan Beulich]

On 30.09.2019 15:32, Roger Pau Monne wrote:
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -279,6 +279,18 @@ unsigned int hvm_pci_decode_addr(unsigned int cf8, 
> unsigned int addr,
>  return CF8_ADDR_LO(cf8) | (addr & 3);
>  }
>  
> +unsigned int hvm_mmcfg_decode_addr(const struct hvm_mmcfg *mmcfg,
> +   paddr_t addr, pci_sbdf_t *sbdf)
> +{
> +addr -= mmcfg->addr;

As the function isn't even static, wouldn't it be helpful to at least
assert that addr >= mmcfg->addr ahead of this, and addr < mmcfg->size
afterwards?

> @@ -574,6 +550,17 @@ void destroy_vpci_mmcfg(struct domain *d)
>  write_unlock(>arch.hvm.mmcfg_lock);
>  }
>  
> +const struct hvm_mmcfg *hvm_mmcfg_find(const struct domain *d, paddr_t addr)
> +{
> +const struct hvm_mmcfg *mmcfg;
> +
> +list_for_each_entry ( mmcfg, >arch.hvm.mmcfg_regions, next )
> +if ( addr >= mmcfg->addr && addr < mmcfg->addr + mmcfg->size )

As a minor remark, this could be coded without even a theoretical risk
of overflow as

if ( addr >= mmcfg->addr && addr - mmcfg->addr < mmcfg->size )

> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -1326,27 +1326,34 @@ ioservid_t hvm_select_ioreq_server(struct domain *d, 
> ioreq_t *p)
>  uint8_t type;
>  uint64_t addr;
>  unsigned int id;
> +const struct hvm_mmcfg *mmcfg;
>  
>  if ( p->type != IOREQ_TYPE_COPY && p->type != IOREQ_TYPE_PIO )
>  return XEN_INVALID_IOSERVID;
>  
>  cf8 = d->arch.hvm.pci_cf8;
>  
> -if ( p->type == IOREQ_TYPE_PIO &&
> - (p->addr & ~3) == 0xcfc &&
> - CF8_ENABLED(cf8) )
> +read_lock(>arch.hvm.mmcfg_lock);
> +if ( (p->type == IOREQ_TYPE_PIO &&
> +  (p->addr & ~3) == 0xcfc &&
> +  CF8_ENABLED(cf8)) ||
> + (p->type == IOREQ_TYPE_COPY &&
> +  (mmcfg = hvm_mmcfg_find(d, p->addr)) != NULL) )
>  {
>  uint32_t x86_fam;
>  pci_sbdf_t sbdf;
>  unsigned int reg;
>  
> -reg = hvm_pci_decode_addr(cf8, p->addr, );
> +reg = p->type == IOREQ_TYPE_PIO ? hvm_pci_decode_addr(cf8, p->addr,
> +  )
> +: hvm_mmcfg_decode_addr(mmcfg, 
> p->addr,
> +);

I'm afraid old gcc will consider this use of mmcfg "potentially
uninitialized". I.e. I think the variable wants to gain a NULL
initializer, at which point you can slightly shorten the above
to

reg = mmcfg ? hvm_mmcfg_decode_addr(mmcfg, p->addr, )
: hvm_pci_decode_addr(cf8, p->addr, );


also eliminating a pointer deref (i.e. likely producing minimally
more efficient code).

>  /* PCI config data cycle */
>  type = XEN_DMOP_IO_RANGE_PCI;
>  addr = ((uint64_t)sbdf.sbdf << 32) | reg;
>  /* AMD extended configuration space access? */
> -if ( CF8_ADDR_HI(cf8) &&
> +if ( p->type == IOREQ_TYPE_PIO && CF8_ADDR_HI(cf8) &&

Would be "!mmcfg && ..." then here.

> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -165,9 +165,19 @@ void stdvga_deinit(struct domain *d);
>  
>  extern void hvm_dpci_msi_eoi(struct domain *d, int vector);
>  
> -/* Decode a PCI port IO access into a bus/slot/func/reg. */
> +struct hvm_mmcfg {
> +struct list_head next;
> +paddr_t addr;
> +unsigned int size;
> +uint16_t segment;
> +uint8_t start_bus;
> +};
> +
> +/* Decode a PCI port IO or MMCFG access into a bus/slot/func/reg. */
>  unsigned int hvm_pci_decode_addr(unsigned int cf8, unsigned int addr,
>   pci_sbdf_t *sbdf);
> +unsigned int hvm_mmcfg_decode_addr(const struct hvm_mmcfg *mmcfg,
> +   paddr_t addr, pci_sbdf_t *sbdf);

The latest now the comment also wants to include "seg/", I think.

> @@ -178,15 +188,18 @@ void register_g2m_portio_handler(struct domain *d);
>  /* HVM port IO handler for vPCI accesses. */
>  void register_vpci_portio_handler(struct domain *d);
>  
> -/* HVM MMIO handler for PCI MMCFG accesses. */
> -int register_vpci_mmcfg_handler(struct domain *d, paddr_t addr,
> -unsigned int start_bus, unsigned int end_bus,
> -unsigned int seg);
> -/* Destroy tracked MMCFG areas. */
> -void destroy_vpci_mmcfg(struct domain *d);
> +/* HVM PCI MMCFG regions registration. */
> +int hvm_register_mmcfg(struct domain *d, paddr_t addr,
> +   unsigned int start_bus, unsigned int end_bus,
> +   unsigned int seg);

As - from a hierarchy perspective - the segment is more significant, can
you put it ahead of the start/end bus numbers please?

> +void hvm_free_mmcfg(struct domain *d);
> +const struct hvm_mmcfg *hvm_mmcfg_find(const struct domain *d, paddr_t addr);
>  
>  /* Check if an address is between a MMCFG region for 

Re: [Xen-devel] [PATCH v2 1/6] Import v1.4 of Contributor Covenant CoC

[Lars Kurth]

On 07/10/2019, 12:06, "George Dunlap"  wrote:

On 9/26/19 8:39 PM, Lars Kurth wrote:
> 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.

This is relatively minor, but I don't feel quite comfortable with the
wording.  "pledge to make it a harassment-free experience" to me implies
that we pledge that *nobody will ever experience harassment*.  I don't
think that's something we can deliver, any more than a government can
promise there will be zero crime.  I think we could promise to
*maintain* a harassment-free experience, which implies things to a
restoring harassment-free state after it's been broken.

Everything else looks good.

Could you come up with an alternative concrete text proposal? 

My take-away is that you say we should use
* "pledge to strive to make ..." or 
* "pledge to try their best to make ..." or
* "strive to make ..." 
in which case we may also need to change "The Pledge".

On the other hand: the rest of the document does clearly lay out that
what we promise is to deal with incidents in an effective manner.
And by the mere inclusion of mechanism to do this, it is actually clear
that we can't guarantee that *nobody will ever experience harassment*.

I guess it comes down to subtleties of how pledge, promise, strive, ...
differ

Regards
Lars
 



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

Re: [Xen-devel] [PATCH for-4.13] docs: update all URLs in man pages

[Wei Liu]

On Mon, Oct 07, 2019 at 11:07:03AM +, Lars Kurth wrote:
> 
> 
> On 07/10/2019, 10:01, "Wei Liu"  wrote:
> 
> On Mon, 7 Oct 2019 at 09:13, Lars Kurth  wrote:
> >
> >
> >
> > On 04/10/2019, 09:57, "Wei Liu"  wrote:
> >
> > On Thu, Oct 03, 2019 at 04:12:30PM +, Lars Kurth wrote:
> > > Specifically
> > > * xen.org to xenproject.org
> > > * http to https
> > > * Replaced pages where page has moved
> > >   (including on xen pages as well as external pages)
> > > * Removed some URLs (e.g. downloads for Linux PV drivers)
> > >
> > > Tested-by: Lars Kurth 
> > > Signed-off-by: Lars Kurth 
> >
> > Do you have a branch for this patch?
> >
> > Unfortunately, not: I never created a personal copy of xen.git on 
> xenbits
> > Really should do this
> 
> Please do. I couldn't reply this patch cleanly. Not sure why git
> wasn't happy about it.
> 
> Wei.
> 
> Hi Wei,
> 
> I fixed the missing http link and rebased to staging
> See 
> http://xenbits.xen.org/gitweb/?p=people/larsk/xen.git;a=commit;h=088c2781795c5924cd1fc7f11f3d31154d866799
>  & 
> http://xenbits.xen.org/gitweb/?p=people/larsk/xen.git;a=shortlog;h=refs/heads/docs-changes-4.13-v2
>  

Thanks. I have pushed your patch.

> 
> When rebasing there was no conflict, so not sure why it didn't apply for you
> 

I found that git sometimes doesn't handle our doc changes well. Not sure
why. Never got to the bottom of it.

Wei.

> Regards
> Lars
> 

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

Re: [Xen-devel] [PATCH v7 1/3] AMD/IOMMU: allocate one device table per PCI segment

[Jürgen Groß]

On 07.10.19 12:49, Jan Beulich wrote:

On 07.10.2019 12:19, Jürgen Groß  wrote:

On 07.10.19 12:03, Jan Beulich wrote:

I appreciate the ack, but I think I'd prefer to not make use of it
if at all possible under these conditions. Instead I'd like us to
reach some common ground here. Seeing that we're past the deadline
already, Jürgen's release ack will now be needed anyway. Jürgen -
would you be fine with settling on this taking a few more days,
and then still allow in this series? Or is trying to soon find a
resolution here pointless as you'd rather not see this go in
anymore?


As it isn't a completely trivial patch series I'd like to know what
the gain would be: is it just a "nice to have", does it solve a
theoretical (not to be expected) problem, or do you think it will
be needed to be backported if I say "no"?


The 3rd patch in this series is what is really wanted, to close
a previously just theoretical but (I think) now in principle
possible gap with device table initialization, potentially
allowing untranslated DMA or interrupt requests to make it
through when not so intended. If this was to be backported, I
think I'd try re-basing patches 2 and 3 onto a tree without
patch 1, but doing so for master doesn't look (to me) to be a
very reasonable step, seeing that patch 1 had been there first.


Okay, thanks for the explanation.

Needing some more days is fine for me, so trying to find a solution soon
absolutely makes sense. :-)


Juergen

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

Re: [Xen-devel] [PATCH for-4.13] docs: update all URLs in man pages

[Jürgen Groß]

On 07.10.19 13:07, Lars Kurth wrote:



On 07/10/2019, 10:01, "Wei Liu"  wrote:

 On Mon, 7 Oct 2019 at 09:13, Lars Kurth  wrote:
 >
 >
 >
 > On 04/10/2019, 09:57, "Wei Liu"  wrote:
 >
 > On Thu, Oct 03, 2019 at 04:12:30PM +, Lars Kurth wrote:
 > > Specifically
 > > * xen.org to xenproject.org
 > > * http to https
 > > * Replaced pages where page has moved
 > >   (including on xen pages as well as external pages)
 > > * Removed some URLs (e.g. downloads for Linux PV drivers)
 > >
 > > Tested-by: Lars Kurth 
 > > Signed-off-by: Lars Kurth 
 >
 > Do you have a branch for this patch?
 >
 > Unfortunately, not: I never created a personal copy of xen.git on xenbits
 > Really should do this
 
 Please do. I couldn't reply this patch cleanly. Not sure why git

 wasn't happy about it.
 
 Wei.
 
Hi Wei,


I fixed the missing http link and rebased to staging
See 
http://xenbits.xen.org/gitweb/?p=people/larsk/xen.git;a=commit;h=088c2781795c5924cd1fc7f11f3d31154d866799
 & 
http://xenbits.xen.org/gitweb/?p=people/larsk/xen.git;a=shortlog;h=refs/heads/docs-changes-4.13-v2

When rebasing there was no conflict, so not sure why it didn't apply for you


My Release-acked-by: still stands.


Juergen


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

Re: [Xen-devel] [PATCH for-4.13] docs: update all URLs in man pages

[Lars Kurth]

On 07/10/2019, 10:01, "Wei Liu"  wrote:

On Mon, 7 Oct 2019 at 09:13, Lars Kurth  wrote:
>
>
>
> On 04/10/2019, 09:57, "Wei Liu"  wrote:
>
> On Thu, Oct 03, 2019 at 04:12:30PM +, Lars Kurth wrote:
> > Specifically
> > * xen.org to xenproject.org
> > * http to https
> > * Replaced pages where page has moved
> >   (including on xen pages as well as external pages)
> > * Removed some URLs (e.g. downloads for Linux PV drivers)
> >
> > Tested-by: Lars Kurth 
> > Signed-off-by: Lars Kurth 
>
> Do you have a branch for this patch?
>
> Unfortunately, not: I never created a personal copy of xen.git on xenbits
> Really should do this

Please do. I couldn't reply this patch cleanly. Not sure why git
wasn't happy about it.

Wei.

Hi Wei,

I fixed the missing http link and rebased to staging
See 
http://xenbits.xen.org/gitweb/?p=people/larsk/xen.git;a=commit;h=088c2781795c5924cd1fc7f11f3d31154d866799
 & 
http://xenbits.xen.org/gitweb/?p=people/larsk/xen.git;a=shortlog;h=refs/heads/docs-changes-4.13-v2
 

When rebasing there was no conflict, so not sure why it didn't apply for you

Regards
Lars

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

Re: [Xen-devel] [PATCH v2 1/6] Import v1.4 of Contributor Covenant CoC

[George Dunlap]

On 9/26/19 8:39 PM, Lars Kurth wrote:
> 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.

This is relatively minor, but I don't feel quite comfortable with the
wording.  "pledge to make it a harassment-free experience" to me implies
that we pledge that *nobody will ever experience harassment*.  I don't
think that's something we can deliver, any more than a government can
promise there will be zero crime.  I think we could promise to
*maintain* a harassment-free experience, which implies things to a
restoring harassment-free state after it's been broken.

Everything else looks good.

 -George

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

Re: [Xen-devel] [PATCH v7 1/3] AMD/IOMMU: allocate one device table per PCI segment

[Jan Beulich]

On 07.10.2019 12:19, Jürgen Groß  wrote:
> On 07.10.19 12:03, Jan Beulich wrote:
>> I appreciate the ack, but I think I'd prefer to not make use of it
>> if at all possible under these conditions. Instead I'd like us to
>> reach some common ground here. Seeing that we're past the deadline
>> already, Jürgen's release ack will now be needed anyway. Jürgen -
>> would you be fine with settling on this taking a few more days,
>> and then still allow in this series? Or is trying to soon find a
>> resolution here pointless as you'd rather not see this go in
>> anymore?
> 
> As it isn't a completely trivial patch series I'd like to know what
> the gain would be: is it just a "nice to have", does it solve a
> theoretical (not to be expected) problem, or do you think it will
> be needed to be backported if I say "no"?

The 3rd patch in this series is what is really wanted, to close
a previously just theoretical but (I think) now in principle
possible gap with device table initialization, potentially
allowing untranslated DMA or interrupt requests to make it
through when not so intended. If this was to be backported, I
think I'd try re-basing patches 2 and 3 onto a tree without
patch 1, but doing so for master doesn't look (to me) to be a
very reasonable step, seeing that patch 1 had been there first.

Jan

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

Re: [Xen-devel] [PATCH v2] xen/sched: let credit scheduler control its timer all alone

[George Dunlap]

On 10/7/19 7:35 AM, Juergen Gross wrote:
> The credit scheduler is the only scheduler with tick_suspend and
> tick_resume callbacks. Today those callbacks are invoked without being
> guarded by the scheduler lock which is critical when at the same the
> cpu those callbacks are active is being moved to or from a cpupool.
> 
> Crashes like the following are possible due to that race:
> 
> (XEN) [ Xen-4.13.0-8.0.12-d  x86_64  debug=y   Not tainted ]
> (XEN) CPU:79
> (XEN) RIP:e008:[] set_timer+0x39/0x1f7
> (XEN) RFLAGS: 00010002   CONTEXT: hypervisor
> 
> (XEN) Xen call trace:
> (XEN)[] set_timer+0x39/0x1f7
> (XEN)[]
> sched_credit.c#csched_tick_resume+0x54/0x59
> (XEN)[] sched_tick_resume+0x67/0x86
> (XEN)[] mwait-idle.c#mwait_idle+0x32b/0x357
> (XEN)[] domain.c#idle_loop+0xa6/0xc2
> (XEN)
> (XEN) Pagetable walk from 0048:
> (XEN)  L4[0x000] = 0082cfb9c063 
> (XEN)  L3[0x000] = 0082cfb9b063 
> (XEN)  L2[0x000] = 0082cfb9a063 
> (XEN)  L1[0x000] =  
> (XEN)
> (XEN) 
> (XEN) Panic on CPU 79:
> (XEN) FATAL PAGE FAULT
> (XEN) [error_code=]
> (XEN) Faulting linear address: 0048
> (XEN) 
> 
> The callbacks are used when the cpu is going to or coming from idle in
> order to allow higher C-states.
> 
> The credit scheduler knows when it is going to schedule an idle
> scheduling unit or another one after idle, so it can easily stop or
> resume the timer itself removing the need to do so via the callback.
> As this timer handling is done in the main scheduling function the
> scheduler lock is still held, so the race with cpupool operations can
> no longer occur. Note that calling the callbacks from schedule_cpu_rm()
> and schedule_cpu_add() is no longer needed, as the transitions to and
> from idle in the cpupool with credit active will automatically occur
> and do the right thing.
> 
> With the last user of the callbacks gone those can be removed.
> 
> Suggested-by: George Dunlap 
> Signed-off-by: Juergen Gross 
> ---
> V2:
> - instead of locking handle timer in credit only (George Dunlap)
> ---
>  xen/common/sched_arinc653.c |  3 ---
>  xen/common/sched_credit.c   | 34 --
>  xen/common/schedule.c   | 26 ++
>  xen/include/xen/sched-if.h  | 15 ---
>  4 files changed, 10 insertions(+), 68 deletions(-)

Like those diffstats. :-)

Pushed, thanks.

 -George

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

Re: [Xen-devel] [PATCH v7 1/3] AMD/IOMMU: allocate one device table per PCI segment

[Jürgen Groß]

On 07.10.19 12:03, Jan Beulich wrote:

I appreciate the ack, but I think I'd prefer to not make use of it
if at all possible under these conditions. Instead I'd like us to
reach some common ground here. Seeing that we're past the deadline
already, Jürgen's release ack will now be needed anyway. Jürgen -
would you be fine with settling on this taking a few more days,
and then still allow in this series? Or is trying to soon find a
resolution here pointless as you'd rather not see this go in
anymore?


As it isn't a completely trivial patch series I'd like to know what
the gain would be: is it just a "nice to have", does it solve a
theoretical (not to be expected) problem, or do you think it will
be needed to be backported if I say "no"?


Juergen

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

Re: [Xen-devel] [PATCH v7 1/3] AMD/IOMMU: allocate one device table per PCI segment

[Jan Beulich]

On 04.10.2019 19:28, Andrew Cooper wrote:
> On 04/10/2019 14:30, Jan Beulich wrote:
>> On 04.10.2019 15:18, Andrew Cooper wrote:
>>> On 26/09/2019 15:28, Jan Beulich wrote:
 @@ -1068,8 +1067,29 @@ static void * __init allocate_ppr_log(st
  IOMMU_PPR_LOG_DEFAULT_ENTRIES, "PPR Log");
  }
  
 +/*
 + * Within ivrs_mappings[] we allocate an extra array element to store
 + * - segment number,
 + * - device table.
 + */
 +#define IVRS_MAPPINGS_SEG(m) (m)[ivrs_bdf_entries].dte_requestor_id
 +#define IVRS_MAPPINGS_DEVTAB(m) (m)[ivrs_bdf_entries].intremap_table
 +
 +static void __init free_ivrs_mapping(void *ptr)
 +{
 +const struct ivrs_mappings *ivrs_mappings = ptr;
>>> How absolutely certain are we that ptr will never be NULL?
>> As certain as we can be by never installing a NULL pointer into the
>> radix tree, and by observing that neither radix_tree_destroy() nor
>> radix_tree_node_destroy() would ever call the callback for a NULL
>> node.
>>
>>> It might be better to rename this to radix_tree_free_ivrs_mappings() to
>>> make it clear who calls it, and also provide a hint as to why the
>>> parameter is void.
>> I'm not happy to add a radix_tree_ prefix; I'd be fine with adding
>> e.g. do_ instead, in case this provides enough of a hint for your
>> taste that this is actually a callback function.
> 
> How about a _callback() suffix?  I'm looking to make it obvious that you
> code shouldn't simply call it directly.

Well, okay, done.

> A "do_" prefix, in particular, provides no useful information to the reader.

Depends, I guess: There are a couple of places where we already use
such naming. People aware of this may make this implication.

 @@ -1082,13 +1102,15 @@ static int __init amd_iommu_init_one(str
  if ( intr && !set_iommu_interrupt_handler(iommu) )
  goto error_out;
  
 -/* To make sure that device_table.buffer has been successfully 
 allocated */
 -if ( device_table.buffer == NULL )
 +/* Make sure that the device table has been successfully allocated. */
 +ivrs_mappings = get_ivrs_mappings(iommu->seg);
 +if ( !IVRS_MAPPINGS_DEVTAB(ivrs_mappings) )
>>> This is still going to crash with a NULL pointer deference in the case
>>> described by the comment.  (Then again, it may not crash, and hit
>>> userspace at the 64M mark.)
>>>
>>> You absolutely need to check ivrs_mappings being non NULL before using
>>> IVRS_MAPPINGS_DEVTAB(), or perhaps roll the check into the macro.
>> I can only repeat what I've said in reply to your respective v6 remark:
>> We won't come here for an IOMMU which didn't have its ivrs_mappings
>> successfully allocated.
> 
> Right, but to a first approximation, I don't care.  I can picture
> exactly what Coverity will say about this, in that radix_tree_lookup()
> may return NULL, and it is used here unconditionally where in most other
> contexts, the pointer gets checked before use.

Except that, as per your stats below, it's not anywhere near "most".

>> You also seem to be mixing up this and the
>> device table allocation - the comment refers to the latter, while your
>> NULL deref concern is about the former. (If you go through the code
>> you'll find that we have numerous other places utilizing the fact that
>> get_ivrs_mappings() can't fail in cases like the one above.)
> 
> The existing code being terrible isn't a reasonable justification for
> adding to the mess.
> 
> It appears we have:
> 
> 1x assert not null
> 14x blind use
> 3x check
> 
> which isn't a great statement about the quality of the code.

If any of the "blind" uses were indeed on a path where this could
in theory be NULL, I'd agree. The patch we're discussing here
definitely doesn't fall into this category.

> Seeing as we are pushed to the deadline for 4.13, begrudgingly A-by
> (preferably with the _callback() suffix), but I'm still not happy with
> the overall quality of the code.  At least it isn't getting
> substantially worse as a consequence here.

I appreciate the ack, but I think I'd prefer to not make use of it
if at all possible under these conditions. Instead I'd like us to
reach some common ground here. Seeing that we're past the deadline
already, Jürgen's release ack will now be needed anyway. Jürgen -
would you be fine with settling on this taking a few more days,
and then still allow in this series? Or is trying to soon find a
resolution here pointless as you'd rather not see this go in
anymore?

As to what (if anything) to change - I'd be fine with adding an
assertion, but I don't think that would buy us much (considering
non-debug builds). What I'm not happy about is adding checks just
for the sake of doing so. Applying the underlying thinking of
"don't trust ourselves" to the entire code base would imo result
in severe crippling of the sources (nevertheless I agree that
there are cases, when connections are less obvious, where 

Re: [Xen-devel] [PATCH for-4.13] xen/arm: fix duplicate memory node in DT

[Julien Grall]

Hi,

On 05/10/2019 00:09, Stefano Stabellini wrote:

When reserved-memory regions are present in the host device tree, dom0
is started with multiple memory nodes. Each memory node should have a
unique name, but today they are all called "memory" leading to Linux
printing the following warning at boot:

   OF: Duplicate name in base, renamed to "memory#1"

This patch fixes the problem by appending a "@" to the
name, as per the Device Tree specification, where  matches
the base of address of the first region.

Reported-by: Oleksandr Tyshchenko 
Signed-off-by: Stefano Stabellini 

---

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 921b054520..a4c07db383 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -646,16 +646,22 @@ static int __init make_memory_node(const struct domain *d,
  int res, i;
  int reg_size = addrcells + sizecells;
  int nr_cells = reg_size * mem->nr_banks;
+/* Placeholder for memory@ + a 32-bit number + \0 */
+char buf[18];
  __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
  __be32 *cells;
  
  BUG_ON(nr_cells >= ARRAY_SIZE(reg));

+/* Nothing to do */


This a departure from the current solution where a node will be created with no 
"reg" property. I think this change of behavior should at least be described in 
the commit message if not implemented in a separate patch. But...



+if ( mem->nr_banks == 0 )
+return 0;


... I don't think we want to ignore it. The caller most likely messed up the 
banks and we should instead report an error.


  
  dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",

 reg_size, nr_cells);
  
  /* ePAPR 3.4 */

-res = fdt_begin_node(fdt, "memory");
+snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
+res = fdt_begin_node(fdt, buf);
  if ( res )
  return res;
  



Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] read grubenv and set default from saved_entry or next_entry [and 1 more messages]

[Ian Jackson]

Hi.  Thanks for the message.

Just one thing I wanted to reply to in your mail:

YOUNG, MICHAEL A. writes ("Re: [Xen-devel] [PATCH] read grubenv and set default 
from saved_entry or next_entry [and 1 more messages]"):
> On Wed, 11 Sep 2019, Ian Jackson wrote:
> > I find this filename hackery rather unprincipled.  I'm not entirely
> > sure I can see a better way, given the way cfg_list is constructed.
> > Can you think of a less hacky approach ?
...
> One option would be to do a fresh search for grubenv in the same places
> we looked for grub.cfg.
> 
> Alternatively, it should be possible to do a more precise edit using
> f.rpartition("/").

I don't feel I fully understand the implications, but either of these
seems like they might be good strategies to me.  I guess the former
risks finding an unrelated leftover grubenv somewhere which is
probably not desirable.

Anyway, I will leave this to your judgement.

Thanks for the rest of your comments.  I'll look forward to a revised
set of patches - thanks.

Regards,
Ian.

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

Re: [Xen-devel] [XEN PATCH for-4.13 3/6] libxl: libxl__domain_config_setdefault: New function

[Ian Jackson]

Anthony PERARD writes ("Re: [XEN PATCH for-4.13 3/6] libxl: 
libxl__domain_config_setdefault: New function"):
> On Fri, Oct 04, 2019 at 05:04:27PM +0100, Ian Jackson wrote:
> > Anthony PERARD writes ("Re: [XEN PATCH for-4.13 3/6] libxl: 
> > libxl__domain_config_setdefault: New function"):
> > > Shouldn't you check for error ?
> > 
> > Blimey, yes!  Thanks!
> 
> With that fixed:
> Reviewed-by: Anthony PERARD 

Thanks.

I think the things you spotted in this review means I should go
through this series again, and try to test it somehow.  I will do that
and then send a v2.

Ian.

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

Re: [Xen-devel] [PATCH] xen/typesafe: Force helpers to be always_inline

[Jan Beulich]

On 04.10.2019 19:02, George Dunlap wrote:
> On 10/2/19 9:11 AM, Jan Beulich wrote:
>> On 01.10.2019 22:59, Andrew Cooper wrote:
>>> On 01/10/2019 09:38, Jan Beulich wrote:
 On 30.09.2019 21:16, Andrew Cooper wrote:
> Clang in particular has a habit of out-of-lining these and creating 
> multiple
> local copies of _mfn() and mfn_x(), etc.  Override this behaviour.
 Is special casing the typesafe helpers then the right approach? The
 fundamental idea after all is to let the compiler decide. I certainly
 agree that not inlining such trivial functions despite the inline
 keyword looks far from optimal, but if there's such a general issue
 with clang, shouldn't we make "inline" expand to "always_inline"
 uniformly?
>>>
>>> Inline handing is a mess.
>>>
>>> We currently define inline to __inline__.  Undoing this results in build
>>> failures.
>>>
>>> Linux currently defines inline to always_inline and they are desperately
>>> trying to undo this (mis)behaviour.
>>>
>>> There are a few uses of always_inline for safety purposes (the
>>> speculative helpers).  Most uses of always_inline look to be workarounds
>>> for the size-of-asm bug/(mis)feature.
>>>
>>> In an ideal world, we wouldn't need it at all, but I definitely don't
>>> think that taking the Linux approach is a clever move.  We definitely
>>> have some static inlines which would better not being inline.
>>
>> IOW your suggested approach (at least for the foreseeable future) is to
>> do what you do here and convert inline to always_inline as we see fit?
>> If so, we should at least settle on some sufficiently firm criteria by
>> which such a conversion would be justifiable.
>>
>> Seeing that this is primarily to help clang - did you consider
>> introducing something like clang_inline, expanding to just inline for
>> gcc, but always_inline for clang? This would at least provide a
>> sufficiently easy way to undo this if a better clang-side approach can
>> be found down the road.
> 
> What would be the point of this?  The only reason always_inline isn't
> necessary for gcc (if I'm following the argument) is because it so far
> has always inlined these functions.  If it stopped inlining them, we'd
> need to change it to always_inline anyway; so why not just say so to
> begin with?

The point of this would be to _avoid_ using always_inline as much as
possible. We really shouldn't fight compiler decisions more than
absolutely necessary. Hence also my request for sufficiently firm
criteria when to switch in the first place. Or else would could, as
mentioned as an option elsewhere, make inline expand to always_inline
uniformly. (Or course, even always_inline isn't a guarantee for the
compiler to actually inline a function.)

Jan

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

Re: [Xen-devel] [PATCH v2] xen/sched: let credit scheduler control its timer all alone

[Jürgen Groß]

On 07.10.19 11:05, Dario Faggioli wrote:

On Mon, 2019-10-07 at 08:35 +0200, Juergen Gross wrote:

The credit scheduler is the only scheduler with tick_suspend and
tick_resume callbacks. Today those callbacks are invoked without
being
guarded by the scheduler lock which is critical when at the same the
cpu those callbacks are active is being moved to or from a cpupool.

Crashes like the following are possible due to that race:

(XEN) [ Xen-4.13.0-8.0.12-d  x86_64  debug=y   Not tainted ]
(XEN) CPU:79
(XEN) RIP:e008:[] set_timer+0x39/0x1f7
(XEN) RFLAGS: 00010002   CONTEXT: hypervisor

(XEN) Xen call trace:
(XEN)[] set_timer+0x39/0x1f7
(XEN)[]
sched_credit.c#csched_tick_resume+0x54/0x59
(XEN)[] sched_tick_resume+0x67/0x86
(XEN)[] mwait-idle.c#mwait_idle+0x32b/0x357
(XEN)[] domain.c#idle_loop+0xa6/0xc2
(XEN)
(XEN) Pagetable walk from 0048:
(XEN)  L4[0x000] = 0082cfb9c063 
(XEN)  L3[0x000] = 0082cfb9b063 
(XEN)  L2[0x000] = 0082cfb9a063 
(XEN)  L1[0x000] =  
(XEN)
(XEN) 
(XEN) Panic on CPU 79:
(XEN) FATAL PAGE FAULT
(XEN) [error_code=]
(XEN) Faulting linear address: 0048
(XEN) 

The callbacks are used when the cpu is going to or coming from idle
in
order to allow higher C-states.

The credit scheduler knows when it is going to schedule an idle
scheduling unit or another one after idle, so it can easily stop or
resume the timer itself removing the need to do so via the callback.
As this timer handling is done in the main scheduling function the
scheduler lock is still held, so the race with cpupool operations can
no longer occur. Note that calling the callbacks from
schedule_cpu_rm()
and schedule_cpu_add() is no longer needed, as the transitions to and
from idle in the cpupool with credit active will automatically occur
and do the right thing.

With the last user of the callbacks gone those can be removed.


Which is great! :-0


Suggested-by: George Dunlap 
Signed-off-by: Juergen Gross 


Well, unless I'm missing something, I guess that, at this point:


--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -3082,32 +3078,14 @@ void schedule_dump(struct cpupool *c)
  
  void sched_tick_suspend(void)

  {
-struct scheduler *sched;
-unsigned int cpu = smp_processor_id();
-
-rcu_read_lock(_res_rculock);
-
-sched = get_sched_res(cpu)->scheduler;
-sched_do_tick_suspend(sched, cpu);
-rcu_idle_enter(cpu);
+rcu_idle_enter(smp_processor_id());
  rcu_idle_timer_start();
-
-rcu_read_unlock(_res_rculock);
  }


sched_tick_suspend() could go away and rcu_idle_enter() be called
directly (with rcu_idle_timer_start() becoming static, and called
directly by rcu_idle_timer_enter() itself)

And the same for sched_tick_resume(), rcu_idle_timer_stop() and
rcu_idle_exit().

I'll give my:

Reviewed-by: Dario Faggioli 

To this patch, though, as I appreciate we want it in to be able to
continue testing core-scheduling during 4.13 rc-phase.

It'd be cool if the adjustments described above (if agreed upon), could
come as a follow-up.


Noted on my "scheduling cleanup" todo list.


Juergen


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

Re: [Xen-devel] [PATCH v2] xen/sched: let credit scheduler control its timer all alone

[Dario Faggioli]

On Mon, 2019-10-07 at 08:35 +0200, Juergen Gross wrote:
> The credit scheduler is the only scheduler with tick_suspend and
> tick_resume callbacks. Today those callbacks are invoked without
> being
> guarded by the scheduler lock which is critical when at the same the
> cpu those callbacks are active is being moved to or from a cpupool.
> 
> Crashes like the following are possible due to that race:
> 
> (XEN) [ Xen-4.13.0-8.0.12-d  x86_64  debug=y   Not tainted ]
> (XEN) CPU:79
> (XEN) RIP:e008:[] set_timer+0x39/0x1f7
> (XEN) RFLAGS: 00010002   CONTEXT: hypervisor
> 
> (XEN) Xen call trace:
> (XEN)[] set_timer+0x39/0x1f7
> (XEN)[]
> sched_credit.c#csched_tick_resume+0x54/0x59
> (XEN)[] sched_tick_resume+0x67/0x86
> (XEN)[] mwait-idle.c#mwait_idle+0x32b/0x357
> (XEN)[] domain.c#idle_loop+0xa6/0xc2
> (XEN)
> (XEN) Pagetable walk from 0048:
> (XEN)  L4[0x000] = 0082cfb9c063 
> (XEN)  L3[0x000] = 0082cfb9b063 
> (XEN)  L2[0x000] = 0082cfb9a063 
> (XEN)  L1[0x000] =  
> (XEN)
> (XEN) 
> (XEN) Panic on CPU 79:
> (XEN) FATAL PAGE FAULT
> (XEN) [error_code=]
> (XEN) Faulting linear address: 0048
> (XEN) 
> 
> The callbacks are used when the cpu is going to or coming from idle
> in
> order to allow higher C-states.
> 
> The credit scheduler knows when it is going to schedule an idle
> scheduling unit or another one after idle, so it can easily stop or
> resume the timer itself removing the need to do so via the callback.
> As this timer handling is done in the main scheduling function the
> scheduler lock is still held, so the race with cpupool operations can
> no longer occur. Note that calling the callbacks from
> schedule_cpu_rm()
> and schedule_cpu_add() is no longer needed, as the transitions to and
> from idle in the cpupool with credit active will automatically occur
> and do the right thing.
> 
> With the last user of the callbacks gone those can be removed.
> 
Which is great! :-0

> Suggested-by: George Dunlap 
> Signed-off-by: Juergen Gross 
>
Well, unless I'm missing something, I guess that, at this point:

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -3082,32 +3078,14 @@ void schedule_dump(struct cpupool *c)
>  
>  void sched_tick_suspend(void)
>  {
> -struct scheduler *sched;
> -unsigned int cpu = smp_processor_id();
> -
> -rcu_read_lock(_res_rculock);
> -
> -sched = get_sched_res(cpu)->scheduler;
> -sched_do_tick_suspend(sched, cpu);
> -rcu_idle_enter(cpu);
> +rcu_idle_enter(smp_processor_id());
>  rcu_idle_timer_start();
> -
> -rcu_read_unlock(_res_rculock);
>  }
> 
sched_tick_suspend() could go away and rcu_idle_enter() be called
directly (with rcu_idle_timer_start() becoming static, and called
directly by rcu_idle_timer_enter() itself)

And the same for sched_tick_resume(), rcu_idle_timer_stop() and
rcu_idle_exit().

I'll give my:

Reviewed-by: Dario Faggioli 

To this patch, though, as I appreciate we want it in to be able to
continue testing core-scheduling during 4.13 rc-phase.

It'd be cool if the adjustments described above (if agreed upon), could
come as a follow-up.

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



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

Re: [Xen-devel] [PATCH for-4.13] docs: update all URLs in man pages

[Wei Liu]

On Mon, 7 Oct 2019 at 09:13, Lars Kurth  wrote:
>
>
>
> On 04/10/2019, 09:57, "Wei Liu"  wrote:
>
> On Thu, Oct 03, 2019 at 04:12:30PM +, Lars Kurth wrote:
> > Specifically
> > * xen.org to xenproject.org
> > * http to https
> > * Replaced pages where page has moved
> >   (including on xen pages as well as external pages)
> > * Removed some URLs (e.g. downloads for Linux PV drivers)
> >
> > Tested-by: Lars Kurth 
> > Signed-off-by: Lars Kurth 
>
> Do you have a branch for this patch?
>
> Unfortunately, not: I never created a personal copy of xen.git on xenbits
> Really should do this

Please do. I couldn't reply this patch cleanly. Not sure why git
wasn't happy about it.

Wei.

> Lars
>
>
>

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

Re: [Xen-devel] [PATCH] xen/sched: fix locking in sched_tick_[suspend|resume]()

[Dario Faggioli]

On Sun, 2019-10-06 at 20:05 +0200, Jürgen Groß wrote:
> On 04.10.19 18:09, George Dunlap wrote:
> > 
> > I can think of a couple of options:
> > 
> > 1. Have schedule.c call s->tick_* when switching to / from idle
> > 
> > 2. Get rid of s->tick_*, and have sched_credit.c suspend / resume
> > ticks
> > when switching to / from idle in csched_schedule()
> > 
> > 3. Have schedule.c suspend / resume ticks, and have an interface
> > that
> > allows schedulers to enable / disable them.
> > 
> > 4. Rework sched_credit to be tickless.
> 
> I'm going with 2., as it will have multiple advantages:
> 
Good choice! For these reasons:

> - not very intrusive
> - keeps credit specifics in credit
>
And also because, if you'd go for 4, I'm sure that reviewing something
like that would would cause me nightmares! :-O

Thanks and Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)



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

Re: [Xen-devel] [RESEND TRIVIAL 3/3] treewide: arch: Fix Kconfig indentation

[Geert Uytterhoeven]

On Fri, Oct 4, 2019 at 4:57 PM Krzysztof Kozlowski  wrote:
> Adjust indentation from spaces to tab (+optional two spaces) as in
> coding style with command like:
> $ sed -e 's/^/\t/' -i */Kconfig
>
> Signed-off-by: Krzysztof Kozlowski 

>  arch/m68k/Kconfig.bus  |  2 +-
>  arch/m68k/Kconfig.debug| 16 
>  arch/m68k/Kconfig.machine  |  8 

For m68k:
Acked-by: Geert Uytterhoeven 

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

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

Re: [Xen-devel] [PATCH for-4.13] docs: update all URLs in man pages

[Lars Kurth]

On 04/10/2019, 09:57, "Wei Liu"  wrote:

On Thu, Oct 03, 2019 at 04:12:30PM +, Lars Kurth wrote:
> Specifically
> * xen.org to xenproject.org
> * http to https
> * Replaced pages where page has moved
>   (including on xen pages as well as external pages)
> * Removed some URLs (e.g. downloads for Linux PV drivers)
> 
> Tested-by: Lars Kurth 
> Signed-off-by: Lars Kurth 

Do you have a branch for this patch?

Unfortunately, not: I never created a personal copy of xen.git on xenbits
Really should do this
Lars



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

Re: [Xen-devel] [PATCH] pci: clear host_maskall field on assign

[Jan Beulich]

On 05.10.2019 01:58, Chao Gao wrote:
> On Wed, Oct 02, 2019 at 12:49:35PM +0200, Roger Pau Monne wrote:
>> The current implementation of host_maskall makes it sticky across
>> assign and deassign calls, which means that once a guest forces Xen to
>> set host_maskall the maskall bit is not going to be cleared until a
>> call to PHYSDEVOP_prepare_msix is performed. Such call however
>> shouldn't be part of the normal flow when doing PCI passthrough, and
>> hence the flag needs to be cleared when assigning in order to prevent
>> host_maskall being carried over from previous assignations.
>>
>> Note that other mask fields, like guest_masked or the entry maskbit
>> are already reset when the msix capability is initialized. Also note
>> that doing the reset of host_maskall there would allow the guest to
>> reset such field by enabling and disabling MSIX, which is not
>> intended.
>>
>> Signed-off-by: Roger Pau Monné 
>> ---
>> Cc: Chao Gao 
>> Cc: "Spassov, Stanislav" 
>> Cc: Pasi Kärkkäinen 
>> ---
>> Chao, Stanislav, can you please check if this patch fixes your
>> issues?
> 
> I am glad to. For your testing, you can just kill qemu and destroy the
> guest. Then maskall bit of a pass-thru device will be set. And in this
> case, try to recreate the guest and check whether the maskall bit is
> cleared in guest.
> 
> The solution is similar to my v1 [1]. One question IMO (IIRC, it is why
> I changed to another approach) is: why not do such reset at deivce
> deassignment such that dom0 can use a clean device. Otherwise, the
> device won't work after being unbound from pciback. But I am not so
> sure, I can check it next Tuesday.

I too did think about this, but aiui pciback needs to issue
PHYSDEVOP_release_msix anyway, and Dom0 would then re-setup MSI-X
"from scratch", i.e. we'd clear the flag anyway in
msix_capability_init() due to msix->used_entries being zero at
the first (of possibly several) invocation(s).

Jan

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

Re: [Xen-devel] [Xen-users] xenstat_domain_cpu_ns() occasionally returns a huge value

[Jan Beulich]

On 06.10.2019 11:01, Jürgen Groß wrote:
> On 06.10.19 07:19, Andy Smith wrote:
>> Hi,
>>
>> I was writing a little utility to dump out domain CPU times and I
>> noticed that occasionally xenstat_domain_cpu_ns() returns an
>> erroneous huge value like 9223488034477457013.
>>
>> Attached is a small test program that just requests every domain's
>> CPU time in a tight loop; it received such a result after less than
>> 3 minutes running in dom0 of a host with only dom0 and two other PV
>> domains running:
>>
>> $ make cpu_time_test
>> cc -Wall  -lxenstat -lyajl -Wl,-rpath,/usr/lib/xen-4.12/lib 
>> -L/usr/lib/xen-4.12/lib  cpu_time_test.c   -o cpu_time_test
>> $ sudo time ./cpu_time_test
>> Got a weird CPU time 9223488108.867305 >100 years 
>> (cpu_ns=9223488108867304818)
>> Command exited with non-zero status 1
>> 84.07user 41.90system 2:40.20elapsed 78%CPU (0avgtext+0avgdata 
>> 39780maxresident)k
>> 0inputs+0outputs (0major+9541minor)pagefaults 0swaps
>>
>> The erroneous results are always somewhere above 922
>> nanoseconds (some 285 years of CPU time if it were genuine!). Then
>> the next reading will be normal. Very occasionally I've seen two in
>> a row. I see this on both 4.12 and 4.10.
>>
>> My C is very rusty so I've probably made a simple error and don't
>> want to bother xen-devel with it; can someone familiar with using
>> the xenstat interface please tell me what I've done wrong here?
> 
> I believe chances are rather high this is the bug which was corrected
> recently with Xen commit f28c4c4c10bdacb.
> 
> Andy, you can easily avoid that problem by removing the highest bit
> of the runtime value, e.g.
> 
> correct_value = reported_runtime & ~(1ULL << 63);
> 
> Jan, I think that patch should be included in stable versions.

I have it queued already; I've merely been waiting for it to pass the
pus gate to master.

Jan

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

Re: [Xen-devel] [PATCH for-4.13] xen/xsm: flask: Prevent NULL deference in flask_assign_{, dt}device()

[Jürgen Groß]

On 04.10.19 18:42, Julien Grall wrote:

flask_assign_{, dt}device() may be used to check whether you can test if
a device is assigned. In this case, the domain will be NULL.

However, flask_iommu_resource_use_perm() will be called and may end up
to deference a NULL pointer. This can be prevented by moving the call
after we check the validity for the domain pointer.

Coverity-ID: 1486741
Fixes: 71e617a6b8 ('use is_iommu_enabled() where appropriate...')
Signed-off-by: Julien Grall 


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] [PATCH for-4.13] xen/arm: fix duplicate memory node in DT

[Jürgen Groß]

On 05.10.19 01:09, Stefano Stabellini wrote:

When reserved-memory regions are present in the host device tree, dom0
is started with multiple memory nodes. Each memory node should have a
unique name, but today they are all called "memory" leading to Linux
printing the following warning at boot:

   OF: Duplicate name in base, renamed to "memory#1"

This patch fixes the problem by appending a "@" to the
name, as per the Device Tree specification, where  matches
the base of address of the first region.

Reported-by: Oleksandr Tyshchenko 
Signed-off-by: Stefano Stabellini 

---

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 921b054520..a4c07db383 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -646,16 +646,22 @@ static int __init make_memory_node(const struct domain *d,
  int res, i;
  int reg_size = addrcells + sizecells;
  int nr_cells = reg_size * mem->nr_banks;
+/* Placeholder for memory@ + a 32-bit number + \0 */
+char buf[18];


You are using PRIx64 for printing the number, so I guess you should
enlarge buf by 8 bytes and adjust the comment (s/32/64/).


  __be32 reg[NR_MEM_BANKS * 4 /* Worst case addrcells + sizecells */];
  __be32 *cells;
  
  BUG_ON(nr_cells >= ARRAY_SIZE(reg));

+/* Nothing to do */
+if ( mem->nr_banks == 0 )
+return 0;
  
  dt_dprintk("Create memory node (reg size %d, nr cells %d)\n",

 reg_size, nr_cells);
  
  /* ePAPR 3.4 */

-res = fdt_begin_node(fdt, "memory");
+snprintf(buf, sizeof(buf), "memory@%"PRIx64, mem->bank[0].start);
+res = fdt_begin_node(fdt, buf);
  if ( res )
  return res;



Juergen


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

Re: [Xen-devel] [XEN PATCH for-4.13 0/6] Drop/deprecate libxl_get_required_*_memory

[Jürgen Groß]

On 04.10.19 17:17, Ian Jackson wrote:

libxl_get_required_shadow_memory has always been anomalous.  libxl
ought to default these things itself.  Recently, another analogous
setting, iommu_memkb, was introduced, along with another function
along the same lines.

This API is not very good.  Fixing it was not entirely trivial.
(Thanks to Paul for explaining some of the difficulties and Anthony
for in-principle review of my proposal.)

Ian Jackson (6):
   libxl: Offer API versions 0x040700 and 0x040800
   xl: Pass libxl_domain_config to freemem(), instead of b_info
   libxl: libxl__domain_config_setdefault: New function
   libxl: libxl_domain_need_memory: Make it take a domain_config
   libxl: Move shadow_memkb and iommu_memkb defaulting into libxl
   libxl: Remove/deprecate libxl_get_required_*_memory from the API

  tools/libxl/libxl.h  | 24 ++-
  tools/libxl/libxl_create.c   | 95 ++--
  tools/libxl/libxl_dom.c  |  7 ++--
  tools/libxl/libxl_internal.h | 10 +
  tools/libxl/libxl_mem.c  | 70 ++--
  tools/libxl/libxl_utils.c| 15 ---
  tools/libxl/libxl_utils.h|  2 +-
  tools/xl/xl_parse.c  | 15 +--
  tools/xl/xl_vmcontrol.c  |  6 +--
  9 files changed, 183 insertions(+), 61 deletions(-)



For the series:

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] [PATCH for-4.13] xen/xsm: flask: Check xmalloc_array() return in security_sid_to_context()

[Jürgen Groß]

On 04.10.19 18:56, Julien Grall wrote:

xmalloc_array() may return NULL if there are memory. Rather than trying
to deference it directly, we should check the return value first.

Coverity-ID: 1381852
Signed-off-by: Julien Grall 


Release-acked-by: Juergen Gross 


Juergen

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

[Xen-devel] [PATCH v2] xen/sched: let credit scheduler control its timer all alone

[Juergen Gross]

The credit scheduler is the only scheduler with tick_suspend and
tick_resume callbacks. Today those callbacks are invoked without being
guarded by the scheduler lock which is critical when at the same the
cpu those callbacks are active is being moved to or from a cpupool.

Crashes like the following are possible due to that race:

(XEN) [ Xen-4.13.0-8.0.12-d  x86_64  debug=y   Not tainted ]
(XEN) CPU:79
(XEN) RIP:e008:[] set_timer+0x39/0x1f7
(XEN) RFLAGS: 00010002   CONTEXT: hypervisor

(XEN) Xen call trace:
(XEN)[] set_timer+0x39/0x1f7
(XEN)[]
sched_credit.c#csched_tick_resume+0x54/0x59
(XEN)[] sched_tick_resume+0x67/0x86
(XEN)[] mwait-idle.c#mwait_idle+0x32b/0x357
(XEN)[] domain.c#idle_loop+0xa6/0xc2
(XEN)
(XEN) Pagetable walk from 0048:
(XEN)  L4[0x000] = 0082cfb9c063 
(XEN)  L3[0x000] = 0082cfb9b063 
(XEN)  L2[0x000] = 0082cfb9a063 
(XEN)  L1[0x000] =  
(XEN)
(XEN) 
(XEN) Panic on CPU 79:
(XEN) FATAL PAGE FAULT
(XEN) [error_code=]
(XEN) Faulting linear address: 0048
(XEN) 

The callbacks are used when the cpu is going to or coming from idle in
order to allow higher C-states.

The credit scheduler knows when it is going to schedule an idle
scheduling unit or another one after idle, so it can easily stop or
resume the timer itself removing the need to do so via the callback.
As this timer handling is done in the main scheduling function the
scheduler lock is still held, so the race with cpupool operations can
no longer occur. Note that calling the callbacks from schedule_cpu_rm()
and schedule_cpu_add() is no longer needed, as the transitions to and
from idle in the cpupool with credit active will automatically occur
and do the right thing.

With the last user of the callbacks gone those can be removed.

Suggested-by: George Dunlap 
Signed-off-by: Juergen Gross 
---
V2:
- instead of locking handle timer in credit only (George Dunlap)
---
 xen/common/sched_arinc653.c |  3 ---
 xen/common/sched_credit.c   | 34 --
 xen/common/schedule.c   | 26 ++
 xen/include/xen/sched-if.h  | 15 ---
 4 files changed, 10 insertions(+), 68 deletions(-)

diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c
index 45c05c6cd9..565575c326 100644
--- a/xen/common/sched_arinc653.c
+++ b/xen/common/sched_arinc653.c
@@ -724,9 +724,6 @@ static const struct scheduler sched_arinc653_def = {
 
 .dump_settings  = NULL,
 .dump_cpu_state = NULL,
-
-.tick_suspend   = NULL,
-.tick_resume= NULL,
 };
 
 REGISTER_SCHEDULER(sched_arinc653_def);
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 31fdcd6a2f..fbffcf3996 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -1831,6 +1831,7 @@ static void csched_schedule(
 {
 const unsigned int cur_cpu = smp_processor_id();
 const unsigned int sched_cpu = sched_get_resource_cpu(cur_cpu);
+struct csched_pcpu *spc = CSCHED_PCPU(cur_cpu);
 struct list_head * const runq = RUNQ(sched_cpu);
 struct csched_unit * const scurr = CSCHED_UNIT(unit);
 struct csched_private *prv = CSCHED_PRIV(ops);
@@ -2000,6 +2001,13 @@ out:
 unit->next_task = snext->unit;
 snext->unit->migrated = migrated;
 
+/* Stop credit tick when going to idle, restart it when coming from idle. 
*/
+if ( !is_idle_unit(unit) && is_idle_unit(unit->next_task) )
+stop_timer(>ticker);
+if ( is_idle_unit(unit) && !is_idle_unit(unit->next_task) )
+set_timer(>ticker, now + MICROSECS(prv->tick_period_us)
+- now % MICROSECS(prv->tick_period_us) );
+
 CSCHED_UNIT_CHECK(unit->next_task);
 }
 
@@ -2237,29 +2245,6 @@ csched_deinit(struct scheduler *ops)
 }
 }
 
-static void csched_tick_suspend(const struct scheduler *ops, unsigned int cpu)
-{
-struct csched_pcpu *spc;
-
-spc = CSCHED_PCPU(cpu);
-
-stop_timer(>ticker);
-}
-
-static void csched_tick_resume(const struct scheduler *ops, unsigned int cpu)
-{
-struct csched_private *prv;
-struct csched_pcpu *spc;
-uint64_t now = NOW();
-
-spc = CSCHED_PCPU(cpu);
-
-prv = CSCHED_PRIV(ops);
-
-set_timer(>ticker, now + MICROSECS(prv->tick_period_us)
-- now % MICROSECS(prv->tick_period_us) );
-}
-
 static const struct scheduler sched_credit_def = {
 .name   = "SMP Credit Scheduler",
 .opt_name   = "credit",
@@ -2295,9 +2280,6 @@ static const struct scheduler sched_credit_def = {
 .switch_sched   = csched_switch_sched,
 .alloc_domdata  = csched_alloc_domdata,
 .free_domdata   = csched_free_domdata,
-
-.tick_suspend   = csched_tick_suspend,
-.tick_resume= csched_tick_resume,
 };
 
 REGISTER_SCHEDULER(sched_credit_def);
diff --git 

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

[osstest service owner]

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

Regressions :-(

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 saverestore-support-checkfail  like 140282
 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail  like 140282
 test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass
 test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail 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-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-amd64-libvirt-xsm 13 migrate-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-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-credit1  13 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-cubietruck 13 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 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-rtds 14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  14 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  13 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  14 

Re: [Xen-devel] [PATCH] xen/sched: fix locking in sched_tick_[suspend|resume]()

[Jürgen Groß]

On 06.10.19 20:05, Jürgen Groß wrote:

On 04.10.19 18:09, George Dunlap wrote:

On 10/4/19 4:40 PM, Jürgen Groß wrote:

On 04.10.19 17:37, George Dunlap wrote:

On 10/4/19 4:03 PM, Jürgen Groß wrote:

On 04.10.19 16:56, George Dunlap wrote:

On 10/4/19 3:43 PM, Jürgen Groß wrote:

On 04.10.19 16:34, George Dunlap wrote:

On 10/4/19 3:24 PM, Jürgen Groß wrote:

On 04.10.19 16:08, George Dunlap wrote:

On 10/4/19 7:40 AM, Juergen Gross wrote:

sched_tick_suspend() and sched_tick_resume() should not call the
scheduler specific timer handlers in case the cpu they are
running on
is just being moved to or from a cpupool.

Use a new percpu lock for that purpose.


Is there a reason we can't use the pcpu_schedule_lock() 
instead of

introducing a new one?  Sorry if this is obvious, but it's been a
while
since I poked around this code.


Lock contention would be higher especially with credit2 which is
using a
per-core or even per-socket lock. We don't care about other
scheduling
activity here, all we need is a guard against our per-cpu 
scheduler

data being changed beneath our feet.


Is this code really being called so often that we need to worry 
about

this level of contention?


Its called each time idle is entered and left again.

Especially with core scheduling there is a high probability of
multiple
cpus leaving idle at the same time and the per-scheduler lock being
used
in parallel already.


Hrm, that does sound pretty bad.


We already have a *lot* of locks; and in this case you're adding a
second lock which interacts with the per-scheduler cpu lock.  This
just
seems like asking for trouble.


In which way does it interact with the per-scheduler cpu lock?


I won't Nack the patch, but I don't think I would ack it without
clear
evidence that the extra lock has a performance improvement that's
worth
the cost of the extra complexity.


I think complexity is lower this way. Especially considering the 
per-

scheduler lock changing with moving a cpu to or from a cpupool.


The key aspect of the per-scheduler lock is that once you hold it, 
the

pointer to the lock can't change.

After this patch, the fact remains that sometimes you need to grab 
one

lock, sometimes the other, and sometimes both.

And, tick_suspend() lives in the per-scheduler code.  Each scheduler
has
to remember that tick_suspend and tick_resume hold a completely
different lock to the rest of the scheduling functions.


Is that really so critical? Today only credit1 has tick_suspend and
tick_resume hooks, and both are really very simple. I can add a
comment in sched-if.h if you like.

And up to now there was no lock at all involved when calling them...

If you think using the normal scheduler lock is to be preferred I'd
be happy to change the patch. But I should mention I was already
planning to revisit usage of the scheduler lock and replace it by the
new per-cpu lock where appropriate (not sure I'd find any appropriate
path for replacement).


Well the really annoying thing here is that all the other schedulers --
in particular, credit2, which as you say, is designed to have multiple
runqueues share the same lock -- have to grab & release the lock 
just to

find out that there's nothing to do.

And even credit1 doesn't do anything particularly clever -- all it does
is stop and start a timer based on a scheduler-global configuration. 
And

the scheduling lock is grabbed to switch to idle anyway.  It seems like
we should be able to do something more sensible.


Yeah, I thought the same.


I can think of a couple of options:

1. Have schedule.c call s->tick_* when switching to / from idle

2. Get rid of s->tick_*, and have sched_credit.c suspend / resume ticks
when switching to / from idle in csched_schedule()

3. Have schedule.c suspend / resume ticks, and have an interface that
allows schedulers to enable / disable them.

4. Rework sched_credit to be tickless.


I'm going with 2., as it will have multiple advantages:

- not very intrusive
- keeps credit specifics in credit
- allows us to fix an existing bug in credit: if we have only one domain
   running in a cpupool with credit scheduler and this domain is capped
   today's handling will disable the credit tick in idle and there will
   be nobody unpausing the vcpus of the capped domain.


This is not an issue, I managed to mix up timer and master_timer of the
credit scheduler.


Juergen

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