Re: [Xen-devel] [PATCH] pci: clear host_maskall field on assign
On Mon, Oct 07, 2019 at 09:38:48AM +0200, Jan Beulich wrote: >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). Yes. I just checked it on my machine and found you are right. Thanks Chao ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 142398: regressions - FAIL
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
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
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: linuxdc16a7e5f36d65b25a1b66ade1
Re: [Xen-devel] [PATCH for-4.13 v2 0/3] fixes for make_[memory/cpu]_node
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
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
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 pass
[Xen-devel] [PATCH v2 1/3] xen/arm: fix buf size in make_cpus_node
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
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
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, - &bootinfo.reserved_mem); -if ( res ) -return res; +if ( bootinfo.reserved_mem.nr_banks > 0 ) +{ +res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, + &bootinfo.reserved_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
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}
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
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
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
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
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
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
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 = &kinfo->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
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 neve
Re: [Xen-devel] [[PATCH for-4.13]] xen/arm: mm: Allow generic xen page-tables helpers to be called early
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
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 saverestore
[Xen-devel] [xen-unstable-smoke test] 142403: tolerable all pass - PUSHED
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
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
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
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
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
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
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(&xc.{});'.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
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.{}(&xc)\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) 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(&xc) + 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) 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) { C.
[Xen-devel] [PATCH 22/24] golang/xenlight: revise use of Context type
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(&ctx.ctx, C.LIBXL_VERSION, 0, (*C.xentoollog_logger)(unsafe.Pointer(ctx.logger))) + if ret != 0 { + return nil, Error(ret) + } + + return &ctx, 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, &nbPool) @@ -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, &c_cpupool, C.uint32_t(Poolid)) if ret != 0 { - err = Error(-ret) + //err = Error(-ret) return } defer C.libxl_cpupoolinfo_dispose(&c_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 { -
[Xen-devel] [PATCH 19/24] golang/xenlight: begin Go to C type marshaling
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.{}(&xc)\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.{}(&xc)\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) + 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) + 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 er
[Xen-devel] [PATCH 23/24] golang/xenlight: add error return type to Context.Cpupoolinfo
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, &c_cpupool, C.uint32_t(Poolid)) if ret != 0 { - //err = Error(-ret) + err = Error(-ret) return } defer C.libxl_cpupoolinfo_dispose(&c_cpupool) - _ = pool.fromC(&c_cpupool) + err = pool.fromC(&c_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
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
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.{}(&xc)\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.{}(&xc)\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
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 14/24] golang/xenlight: generate structs from the IDL
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 + +# De
[Xen-devel] [PATCH 18/24] golang/xenlight: implement array C to Go marshaling
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(&v); 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(&v); 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(&xc.vcpus); 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(&v); 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(&v); err != nil { + return err + } +
[Xen-devel] [PATCH 15/24] golang/xenlight: remove no-longer used type MemKB
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
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(&chwcap[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(&cphys.hw_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
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
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
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(&c.uuid[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 = &Dominfo{} - di.Uuid = Uuid(cdi.uuid) + di.Uuid.fromC(&cdi.uuid) 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
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
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
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
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, 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
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
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(&cmvg.bytes[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
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 = E
[Xen-devel] [PATCH 06/24] golang/xenlight: re-name Bitmap marshaling functions
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(&cci.cpumap) return } @@ -521,7 +550,10 @@ func (Ctx *Context) CpupoolCreate(Name string, Scheduler Scheduler, Cpumap Bitma var uuid C.libxl_uuid C.libxl_uuid_generate(&uuid) - cbm := Cpumap.toC() + cbm, err := Cpumap.toC() + if err != nil { + return + } defer C.libxl_bitmap_dispose(&cbm) 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(&cbm) ret := C.libxl_cpupool_cpuadd_cpumap(Ctx.ctx, C.uint32_t(Poolid), &cbm) @@ -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(&cbm) ret := C.libxl_cpupool_cpuremove_cpumap(Ctx.ctx, C.uint32_t(Poolid), &cbm) @@ -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(&cbm) - - 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 -} - fun
[Xen-devel] [PATCH 01/24] golang/xenlight: fix calls to libxl_domain_unpause/pause
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
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
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
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: linux5164f0c3740d357ba460b
Re: [Xen-devel] [PATCH] xen/xenbus: fix self-deadlock after killing user process
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
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 f
Re: [Xen-devel] [PATCH v3 08/10] vpci: register as an internal ioreq server
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(&pdev->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, &id, 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
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 n
Re: [Xen-devel] [PATCH 1/4] docs/sphinx: License content with CC-BY-4.0
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 don
Re: [Xen-devel] [PATCH 1/4] docs/sphinx: License content with CC-BY-4.0
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
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(&d->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, &d->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(&d->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, &sbdf); > +reg = p->type == IOREQ_TYPE_PIO ? hvm_pci_decode_addr(cf8, p->addr, > + &sbdf) > +: hvm_mmcfg_decode_addr(mmcfg, > p->addr, > +&sbdf); 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, &sbdf) : hvm_pci_decode_addr(cf8, p->addr, &sbdf); 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 addr
Re: [Xen-devel] [PATCH v2 1/6] Import v1.4 of Contributor Covenant CoC
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
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
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
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
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
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
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
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
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
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 adding
Re: [Xen-devel] [PATCH] read grubenv and set default from saved_entry or next_entry [and 1 more messages]
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] [PATCH for-4.13] xen/arm: fix duplicate memory node in DT
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] [XEN PATCH for-4.13 3/6] libxl: libxl__domain_config_setdefault: New function
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
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
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(&sched_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(&sched_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
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(&sched_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(&sched_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
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]()
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
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
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
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
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