[Xen-devel] Likely regression in efi=no-rs option
Hi! as I've reported earlier -- part of my testing of Xen 4.13 RC2 failed in a massive way with Dom0 never coming up. I've traced that problem to the option that we're using to boot Xen: efi=no-rs We've been using this option for quite sometime and Xen 4.13 RC2 is the first one that seems to make Dom0 boot fail with this option present (note that RC1 was fine). I was wondering whether there were any changes in the areas related to UEFI in Xen that may have triggered this. Here's the boot line that works with RC2: dom0_mem=1024M,max:1024M dom0_max_vcpus=1 dom0_vcpus_pin smt=false adding efi=no-rs make Dom0 boot process fail: efi=no-rs dom0_mem=1024M,max:1024M dom0_max_vcpus=1 dom0_vcpus_pin smt=false Attaching xl info and dmesg just in case Thanks, Roman. Xen 4.13.0-rc (XEN) Xen version 4.13.0-rc (@) (gcc (Alpine 6.4.0) 6.4.0) debug=y Thu Nov 14 06:43:01 UTC 2019 (XEN) Latest ChangeSet: (XEN) build-id: 3cf9f737e7ebc3c92024f33583302bdacd36b883 (XEN) Bootloader: GRUB 2.03 (XEN) Command line: dom0_mem=1024M,max:1024M dom0_max_vcpus=1 dom0_vcpus_pin smt=false (XEN) Xen image load base address: 0x8800 (XEN) Video information: (XEN) VGA is graphics mode 1680x1050, 32 bpp (XEN) Disc information: (XEN) Found 0 MBR signatures (XEN) Found 1 EDD information structures (XEN) EFI RAM map: (XEN) - 00058000 (usable) (XEN) 00058000 - 00059000 (reserved) (XEN) 00059000 - 0009f000 (usable) (XEN) 0009f000 - 000a (reserved) (XEN) 0010 - 8648a000 (usable) (XEN) 8648a000 - 8648b000 (ACPI NVS) (XEN) 8648b000 - 864b5000 (reserved) (XEN) 864b5000 - 8c224000 (usable) (XEN) 8c224000 - 8c528000 (reserved) (XEN) 8c528000 - 8c736000 (usable) (XEN) 8c736000 - 8cea7000 (ACPI NVS) (XEN) 8cea7000 - 8d2ff000 (reserved) (XEN) 8d2ff000 - 8d30 (usable) (XEN) 8d30 - 8d40 (reserved) (XEN) e000 - f000 (reserved) (XEN) fe00 - fe011000 (reserved) (XEN) fec0 - fec01000 (reserved) (XEN) fee0 - fee01000 (reserved) (XEN) ff00 - 0001 (reserved) (XEN) 0001 - 00016e00 (usable) (XEN) ACPI: RSDP 8CE49000, 0024 (r2 ALASKA) (XEN) ACPI: XSDT 8CE490A8, 00CC (r1 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: FACP 8CE6C370, 010C (r5 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: DSDT 8CE49208, 23167 (r2 ALASKA A M I 1072009 INTL 20120913) (XEN) ACPI: FACS 8CE8EF80, 0040 (XEN) ACPI: APIC 8CE6C480, 0084 (r3 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: FPDT 8CE6C508, 0044 (r1 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: FIDT 8CE6C550, 009C (r1 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: MCFG 8CE6C5F0, 003C (r1 ALASKA A M I 1072009 MSFT 97) (XEN) ACPI: HPET 8CE6C630, 0038 (r1 ALASKA A M I 1072009 AMI.5000B) (XEN) ACPI: LPIT 8CE6C668, 0094 (r1 INTEL SKL-ULT0 MSFT 5F) (XEN) ACPI: SSDT 8CE6C700, 0248 (r2 INTEL sensrhub0 INTL 20120913) (XEN) ACPI: SSDT 8CE6C948, 2BAE (r2 INTEL PtidDevc 1000 INTL 20120913) (XEN) ACPI: SSDT 8CE6F4F8, 0BE3 (r2 INTEL Ther_Rvp 1000 INTL 20120913) (XEN) ACPI: SSDT 8CE700E0, 04A3 (r2 INTEL zpodd 1000 INTL 20120913) (XEN) ACPI: DBGP 8CE70588, 0034 (r1 INTEL 0 MSFT 5F) (XEN) ACPI: DBG2 8CE705C0, 0054 (r0 INTEL 0 MSFT 5F) (XEN) ACPI: SSDT 8CE70618, 06E9 (r2 INTEL xh_rvp070 INTL 20120913) (XEN) ACPI: SSDT 8CE70D08, 547E (r2 SaSsdt SaSsdt 3000 INTL 20120913) (XEN) ACPI: UEFI 8CE76188, 0042 (r10 0) (XEN) ACPI: SSDT 8CE761D0, 0E73 (r2 CpuRef CpuSsdt 3000 INTL 20120913) (XEN) ACPI: BGRT 8CE77048, 0038 (r1 ALASKA A M I 1072009 AMI 10013) (XEN) ACPI: DMAR 8CE77080, 00A8 (r1 INTEL SKL 1 INTL1) (XEN) ACPI: TPM2 8CE77128, 0034 (r3Tpm2Tabl1 AMI 0) (XEN) ACPI: ASF! 8CE77160, 00A5 (r32 INTEL HCG1 TFSMF4240) (XEN) System RAM: 4003MB (4099736kB) (XEN) No NUMA configuration found (XEN) Faking a node at -00016e00 (XEN) Domain heap initialised (XEN) vesafb: framebuffer at 0xc000, mapped to 0x82c000201000, using 6912k, total 6912k (XEN) vesafb: mode is 1680x1050x32, linelength=6720, font 8x16 (XEN) vesafb: Truecolor: size=8:8:8:8, shift=24:16:8:0 (XEN) CPU Vendor: Intel, Family 6 (0x6), Model 78 (0x4e), Stepping 3 (raw 000406e3) (XEN) SMBIOS 3.0 present. (XEN) Using APIC driver default (XEN) ACPI: PM-Timer IO Port: 0x1808 (32 bits) (XEN) ACPI: v5 SLEEP INFO: control[0:0], status[0:0] (XEN) ACPI: SLEEP INFO: pm1x_cnt[1:1804,1:0], pm1x_evt[1:1800,1:0] (XEN) ACPI: 32/64X FACS address mismatch in FADT - 8ce8ef80/, using 32
[Xen-devel] [qemu-mainline test] 144154: tolerable FAIL - PUSHED
flight 144154 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/144154/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-rtds 16 guest-localmigrate fail like 144120 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 144120 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 144120 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144120 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 144120 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 144120 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144120 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 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-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-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-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 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 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-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 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 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass version targeted for testing: qemuu19bef037fe096b17edda103fd513ce6451da23c8 baseline version: qemuue10bf1fe00eceb2dbff973f5939036ef3f3c77a4 Last test of basis 144120 2019-11-14 14:36:14 Z1 days Testing same since 144154 2019-11-15 12:36:16 Z0 days1 attempts People who touched revisions under test: Alistair Francis Hiroyuki Obinata hiroyuki.obinata Palmer Dabbelt Palmer Dabbelt Peter Maydell jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64
Re: [Xen-devel] [PATCH V2] kdd.c: Add support for initial handshake in KD protocol for Win 7, 8 and 10 (64 bit)
CC Wei's correct e-mail address.
On Sat, 16 Nov 2019, 05:44 Julien Grall, wrote:
> Hi,
>
> I am not commenting on the code itself but the process.
>
> On Thu, 14 Nov 2019, 07:59 Julian Tuminaro,
> wrote:
>
>> From: Julian Tuminaro and Jenish Rakholiya > and rakholiyajenish...@gmail.com>
>>
>
> AFAICT this is the first time we have such format for "From".
>
> We usually have one person listed per tag and I think we should stick with
> it.
>
> Otherwise this is possibly going to break tools like get_maintainers.pl
> that tends to also output the list of contributors (depending on the
> option) and stat tools.
>
> Although, I am not entirely sure how to encode 2 authors here. Maybe 2
> From tag?
>
>
>> Current implementation of find_os is based on the hard-coded values for
>> different Windows version. It uses the value for get the address to
>> start looking for DOS header in the given specified range. However, this
>> is not scalable to all version of Windows as it will require us to keep
>> adding new entries and also due to KASLR, chances of not hitting the PE
>> header is significant. We implement a way for 64-bit systems to use IDT
>> entry to get a valid exception/interrupt handler and then move back into
>> the memory to find the valid DOS header. Since IDT entries are protected
>> by PatchGuard, we think our assumption that IDT entries will not be
>> corrupted is valid for our purpose. Once we have the image base, we
>> search for the DBGKD_GET_VERSION64 structure type in .data section to
>> get information required for handshake.
>>
>> Currently, this is a work in progress feature and current patch only
>> supports the handshake and memory read/write on 64-bit systems.
>>
>> NOTE: This is the Updated version of the previous patch submitted
>
>
> This paragraph is not useful after committing. We tend to add them after
> "---" so it get stripped by git am.
>
> NOTE: This has currently been only tested when debugging was not enabled
>> on the guest Windows.
>
>
> This one is arguable, I think someone should have done the testing in most
> of the configurations before committing. So it can be put after "---" to
> inform the reviewer the state if the patch.
>
> Cheers,
>
>
>> Signed-off-by: Jenish Rakholiya
>> Signed-off-by: Julian Tuminaro
>> ---
>> tools/debugger/kdd/kdd.c | 392 ---
>> 1 file changed, 366 insertions(+), 26 deletions(-)
>>
>> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
>> index fb8c645355..6d3febefda 100644
>> --- a/tools/debugger/kdd/kdd.c
>> +++ b/tools/debugger/kdd/kdd.c
>> @@ -41,6 +41,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>> #include
>> @@ -51,6 +52,16 @@
>>
>> #include "kdd.h"
>>
>> +/*
>> + * TODO: kdd_os is a type which is used to represent os array. Adding a
>> + * variable here would result in adding a new field to each element in
>> array.
>> + * However, since most of the fields are part of the same struct that we
>> are
>> + * trying to read from memory, we have added kddl to this structure. If
>> + * required, we can possibly separate the kddl value to someplace else
>> + *
>> + * We also use kddl of size uint32_t which is actually used to represent
>> the
>> + * offset from image base rather than actual address
>> + */
>> /* Windows version details */
>> typedef struct {
>> uint32_t build;
>> @@ -62,6 +73,7 @@ typedef struct {
>> uint32_t version; /* +-> NtBuildNumber */
>> uint32_t modules; /* +-> PsLoadedModuleList */
>> uint32_t prcbs; /* +-> KiProcessorBlock */
>> +uint32_t kddl; /* +-> KdDebuggerList */
>> } kdd_os;
>>
>> /* State of the debugger stub */
>> @@ -85,6 +97,117 @@ typedef struct {
>> kdd_os os; /* OS-specific magic
>> numbers */
>> } kdd_state;
>>
>> +/**
>> + * @brief Structure to represent DBGKD_GET_VERSION64
>> + *
>> + * reference:
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdbgexts/ns-wdbgexts-_dbgkd_get_version64
>> + */
>> +typedef struct {
>> +uint16_t MajorVersion; /* usually 0xf for free
>> build */
>> +uint16_t MinorVersion; /* build number of
>> target OS */
>> +uint8_t ProtocolVersion; /* version of the debugger
>> protocol */
>> +uint8_t KdSecondaryVersion; /* secondary version
>> number */
>> +uint16_t Flags;/* set of bit flags for the current debugging
>> session */
>> +uint16_t MachineType; /* type of the target's
>> processor */
>> +uint8_t MaxPacketType; /* one plus the highest number for a
>> debugger */
>> + /* packet type recognized by the
>> target */
>> +uint8_t MaxStateChagne; /* one plus the highest number for a
>> state */
>> + /* change generated by the
>> target */
>> +uint8_t
Re: [Xen-devel] [PATCH V2] kdd.c: Add support for initial handshake in KD protocol for Win 7, 8 and 10 (64 bit)
Hi,
I am not commenting on the code itself but the process.
On Thu, 14 Nov 2019, 07:59 Julian Tuminaro,
wrote:
> From: Julian Tuminaro and Jenish Rakholiya rakholiyajenish...@gmail.com>
>
AFAICT this is the first time we have such format for "From".
We usually have one person listed per tag and I think we should stick with
it.
Otherwise this is possibly going to break tools like get_maintainers.pl
that tends to also output the list of contributors (depending on the
option) and stat tools.
Although, I am not entirely sure how to encode 2 authors here. Maybe 2 From
tag?
> Current implementation of find_os is based on the hard-coded values for
> different Windows version. It uses the value for get the address to
> start looking for DOS header in the given specified range. However, this
> is not scalable to all version of Windows as it will require us to keep
> adding new entries and also due to KASLR, chances of not hitting the PE
> header is significant. We implement a way for 64-bit systems to use IDT
> entry to get a valid exception/interrupt handler and then move back into
> the memory to find the valid DOS header. Since IDT entries are protected
> by PatchGuard, we think our assumption that IDT entries will not be
> corrupted is valid for our purpose. Once we have the image base, we
> search for the DBGKD_GET_VERSION64 structure type in .data section to
> get information required for handshake.
>
> Currently, this is a work in progress feature and current patch only
> supports the handshake and memory read/write on 64-bit systems.
>
> NOTE: This is the Updated version of the previous patch submitted
This paragraph is not useful after committing. We tend to add them after
"---" so it get stripped by git am.
NOTE: This has currently been only tested when debugging was not enabled
> on the guest Windows.
This one is arguable, I think someone should have done the testing in most
of the configurations before committing. So it can be put after "---" to
inform the reviewer the state if the patch.
Cheers,
> Signed-off-by: Jenish Rakholiya
> Signed-off-by: Julian Tuminaro
> ---
> tools/debugger/kdd/kdd.c | 392 ---
> 1 file changed, 366 insertions(+), 26 deletions(-)
>
> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> index fb8c645355..6d3febefda 100644
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -41,6 +41,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -51,6 +52,16 @@
>
> #include "kdd.h"
>
> +/*
> + * TODO: kdd_os is a type which is used to represent os array. Adding a
> + * variable here would result in adding a new field to each element in
> array.
> + * However, since most of the fields are part of the same struct that we
> are
> + * trying to read from memory, we have added kddl to this structure. If
> + * required, we can possibly separate the kddl value to someplace else
> + *
> + * We also use kddl of size uint32_t which is actually used to represent
> the
> + * offset from image base rather than actual address
> + */
> /* Windows version details */
> typedef struct {
> uint32_t build;
> @@ -62,6 +73,7 @@ typedef struct {
> uint32_t version; /* +-> NtBuildNumber */
> uint32_t modules; /* +-> PsLoadedModuleList */
> uint32_t prcbs; /* +-> KiProcessorBlock */
> +uint32_t kddl; /* +-> KdDebuggerList */
> } kdd_os;
>
> /* State of the debugger stub */
> @@ -85,6 +97,117 @@ typedef struct {
> kdd_os os; /* OS-specific magic
> numbers */
> } kdd_state;
>
> +/**
> + * @brief Structure to represent DBGKD_GET_VERSION64
> + *
> + * reference:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdbgexts/ns-wdbgexts-_dbgkd_get_version64
> + */
> +typedef struct {
> +uint16_t MajorVersion; /* usually 0xf for free
> build */
> +uint16_t MinorVersion; /* build number of target
> OS */
> +uint8_t ProtocolVersion; /* version of the debugger
> protocol */
> +uint8_t KdSecondaryVersion; /* secondary version
> number */
> +uint16_t Flags;/* set of bit flags for the current debugging
> session */
> +uint16_t MachineType; /* type of the target's
> processor */
> +uint8_t MaxPacketType; /* one plus the highest number for a
> debugger */
> + /* packet type recognized by the
> target */
> +uint8_t MaxStateChagne; /* one plus the highest number for a
> state */
> + /* change generated by the
> target */
> +uint8_t MaxManipulate; /* one more that the highest number,
> recognized */
> +/* by the target, for a command to manipulate the
> target */
> +uint8_t Simulation;/* indication if target is in simulated
> execution */
> +
[Xen-devel] [xen-4.11-testing test] 144152: regressions - FAIL
flight 144152 xen-4.11-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/144152/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 144025 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 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-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 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-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-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-qemuu-ws16-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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen 74507046dbd2c5d2991eeabd1af39af0d6b29d70 baseline version: xen 006b2041242129896fbd30135b3dc6f575894a07 Last test of basis 144025 2019-11-11 17:36:00 Z4 days Testing same since 144058 2019-11-12 18:05:56 Z3 days5 attempts
Re: [Xen-devel] [PATCH for-4.13 v3] x86/passthrough: fix migration of MSI when using posted interrupts
On 11/15/19 6:06 AM, Roger Pau Monné wrote: > On Fri, Nov 15, 2019 at 05:23:51AM +, Tian, Kevin wrote: >>> From: Roger Pau Monne [mailto:roger@citrix.com] >>> Sent: Friday, November 8, 2019 9:34 PM >>> >>> When using posted interrupts and the guest migrates MSI from vCPUs Xen >>> needs to flush any pending PIRR vectors on the previous vCPU, or else >>> those vectors could get wrongly injected at a later point when the MSI >>> fields are already updated. >> I may overlook but isn't it the guest's responsibility of handling such >> case? Even on bare metal, an in-fly interrupt may be delivered to >> wrong CPU when MSI is being migrated? > According to Joe from Oracle Linux already takes care of that by > checking IRR when migrating interrupts between CPUs, but it seems like > the vector is not pending in IRR (my hypothesis is that it's pending > in PIR but lacking a sync into IRR). > > After digging more into the posted interrupt code, I think there's an > issue somewhere else, and the sync on MSI reconfiguration done by this > patch is just covering that up. > > There shouldn't be any interrupts pending in the PIR when the vCPU is > running, and any pending vectors in the PIR should be synced into IRR > before executing the vCPU. > > AFAICT there's an issue with how PIR is synced into IRR, it relies on > vlapic_find_highest_irr being called from vlapic_has_pending_irq, but > depending on which interrupts are pending it's possible that > vlapic_has_pending_irq is not called by hvm_vcpu_has_pending_irq, thus > leaving IRR stale. > > The patch below should solve that and also simplify > __vmx_deliver_posted_interrupt, there's no reason to raise a softirq > in __vmx_deliver_posted_interrupt: if the vCPU is the one currently > running or if it's not running at all the sync of PIR to IRR will > happen on vmentry, without the need of any softirq being set. Also > note the raise_softirq in __vmx_deliver_posted_interrupt should have > been a cpu_raise_softirq(cpu, VCPU_KICK_SOFTIRQ) instead. > > Joe, can you give a try to the patch below? This patch fixed my issue as well. Thanks, Joe ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable test] 144149: regressions - FAIL
flight 144149 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/144149/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 144042 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 144042 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail REGR. vs. 144042 test-armhf-armhf-xl-rtds 17 guest-start.2fail REGR. vs. 144042 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 144042 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 144042 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 144042 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 144042 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 144042 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 144042 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 144042 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 144042 test-amd64-amd64-libvirt 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-xsm 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-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-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail 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-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 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-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen 09242da55b32e2d1c3728c23cd43d0377b74bed6 baseline version: xen a458d3bd0d2585275c128556ec0cbd818c6a7b0d Last test of basis 144042 2019-11-12 09:07:51 Z3 days Failing since144067 2019-11-13 02:19:05 Z2 days4 attempts Testing same since
[Xen-devel] [TESTDAY] Test report
NOTE: this may or may not be a hair on fire problem, reporting it anyway since I'd hate to pass on something that maybe a serious issue. I haven't had time to debug this just yet -- so just reporting it here pretty raw. Software: Xen 4.13 RC2 Linux kernel 4.19.5 Hardware: Supermicro E300 https://www.supermicro.com/en/products/system/Mini-ITX/SYS-E300-8D.cfm Supermicro E100 https://www.supermicro.com/en/products/system/Box_PC/SYS-E100-9S.cfm Supermicro E50 https://www.supermicro.com/en/products/system/Box_PC/SYS-E50-9AP.cfm Functionality tested: trying to boot Dom0 Comments: Xen boots completely and then seems like it either dies right after saying Xen relinquishing a console or Dom0 dies (without printing a single line of output) FWIW, this started happening after upgrade to RC2. IOW, if I take my previous RC1 binary and stick it into the very same setup -- everything boots fine. The issue doesn't seem to be reproducible on Dell boxes (and in my virtual QEmu setup) that I've got. Thanks, Roman. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 144151: all pass - PUSHED
flight 144151 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/144151/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 0b9ad0bc030bbd79073a26fc9b3527ff9128b9da baseline version: ovmf 6fe77f347ed820c5924f2ac6ddc43aa869cdbd5e Last test of basis 144138 2019-11-15 01:39:03 Z0 days Testing same since 144151 2019-11-15 11:24:37 Z0 days1 attempts People who touched revisions under test: Liming Gao Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 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/osstest/ovmf.git 6fe77f347e..0b9ad0bc03 0b9ad0bc030bbd79073a26fc9b3527ff9128b9da -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [HACK XEN PATCH v3 11/11] HACK: Force virt timer to PPI0 (IRQ16)
From: Ian Campbell Just for testing so the guest vtimer ppi it isn't the same as the physical virt timer PPI on my platform, and therefore allows to exercise the non-1:1 bits of the code. --- xen/include/public/arch-arm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h index c365b1b39e..e7ab984a3b 100644 --- a/xen/include/public/arch-arm.h +++ b/xen/include/public/arch-arm.h @@ -451,7 +451,7 @@ typedef uint64_t xen_callback_t; #define GUEST_MAX_VCPUS 128 /* Interrupts */ -#define GUEST_TIMER_VIRT_PPI27 +#define GUEST_TIMER_VIRT_PPI16 #define GUEST_TIMER_PHYS_S_PPI 29 #define GUEST_TIMER_PHYS_NS_PPI 30 #define GUEST_EVTCHN_PPI31 -- 2.24.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH v3 09/11] xen: arm: gic: supporting routing a PPI to the current vcpu.
From: Ian Campbell
That is whichever vcpu is resident when the interrupt fires. An
interrupt is in this state when both IRQ_GUEST and IRQ_PER_CPU are set
in the descriptor status. Only PPIs can be in this mode.
This requires some peripheral specific code to make use of the
previously introduced functionality to save and restore the PPI state.
The vtimer driver will do so shortly.
Signed-off-by: Ian Campbell
Signed-off-by: Stewart Hildebrand
---
v3:
* Change calls to gic_set_irq_properties() to gic_set_irq_type() and
gic_set_irq_priority() due to following commits:
16580cde5a xen/arm: gic: Do not configure affinity during routing
23e8118b8e xen/arm: gic: split set_irq_properties
* Partially address feedback from v2 [1]:
* Clarify a comment.
* Switch loglevel back to XENLOG_G_ERR and bump a parameter to the
next line to comply with line length coding style.
* Call vgic_get_hw_irq_desc from gic_save_and_mask_hwppi
* Call vgic_connect_hw_irq from gic_restore_hwppi
---
Note: I have not yet addressed feedback from [1] regarding
differentiating between CPU0/CPU1 in the error message.
I also have not yet given much thought to Julien's comment in [1] "Why
do you set the parameter virq to irq?"
I hope to investigate further if time allows, but if anyone has any
input I'd like to hear it.
[1] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01064.html
---
xen/arch/arm/gic.c| 33 ++--
xen/arch/arm/irq.c| 80 +++
xen/include/asm-arm/gic.h | 2 +
xen/include/asm-arm/irq.h | 1 +
4 files changed, 104 insertions(+), 12 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 75921724dd..982afaadbd 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -92,8 +92,7 @@ void gic_save_state(struct vcpu *v)
void gic_save_and_mask_hwppi(struct vcpu *v, const unsigned virq,
struct hwppi_state *s)
{
-struct pending_irq *p = irq_to_pending(v, virq);
-struct irq_desc *desc = p->desc;
+struct irq_desc *desc = vgic_get_hw_irq_desc(NULL, v, virq);
spin_lock(>lock);
@@ -123,7 +122,6 @@ void gic_restore_hwppi(struct vcpu *v,
const unsigned virq,
const struct hwppi_state *s)
{
-struct pending_irq *p = irq_to_pending(v, virq);
struct irq_desc *desc = irq_to_desc(s->irq);
spin_lock(>lock);
@@ -131,7 +129,8 @@ void gic_restore_hwppi(struct vcpu *v,
ASSERT(virq >= 16 && virq < 32);
ASSERT(!is_idle_vcpu(v));
-p->desc = desc; /* Migrate to new physical processor */
+/* Migrate to new physical processor */
+vgic_connect_hw_irq(v->domain, v, virq, desc, true);
irq_set_virq(desc, virq);
@@ -178,6 +177,32 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned
int priority)
gic_set_irq_priority(desc, priority);
}
+/*
+ * Program the GIC to route an interrupt to the current guest.
+ *
+ * That is, the IRQ is delivered to whichever VCPU happens to be
+ * resident on the PCPU when the interrupt arrives.
+ *
+ * Currently the interrupt *must* be a PPI and the code responsible
+ * for the related hardware must save and restore the PPI with
+ * gic_save_and_mask_hwppi/gic_restore_hwppi.
+ */
+int gic_route_irq_to_current_guest(struct irq_desc *desc,
+ unsigned int priority)
+{
+ASSERT(spin_is_locked(>lock));
+ASSERT(desc->irq >= 16 && desc->irq < 32);
+
+desc->handler = gic_hw_ops->gic_guest_irq_type;
+set_bit(_IRQ_GUEST, >status);
+set_bit(_IRQ_PER_CPU, >status);
+
+gic_set_irq_type(desc, desc->arch.type);
+gic_set_irq_priority(desc, GIC_PRI_IRQ);
+
+return 0;
+}
+
/* Program the GIC to route an interrupt to a guest
* - desc.lock must be held
*/
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 1a8e599c2e..17dec64203 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -236,6 +236,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq,
int is_fiq)
if ( test_bit(_IRQ_GUEST, >status) )
{
struct irq_guest *info = irq_get_guest_info(desc);
+struct vcpu *v;
perfc_incr(guest_irqs);
desc->handler->end(desc);
@@ -243,10 +244,15 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq,
int is_fiq)
set_bit(_IRQ_INPROGRESS, >status);
/*
- * The irq cannot be a PPI, we only support delivery of SPIs to
- * guests.
+ * A PPI exposed to a guest must always be in IRQ_GUEST|IRQ_PER_CPU
+ * mode ("route to active VCPU"), so we use current.
+ *
+ * For SPI, we use NULL. In this case, vgic_inject_irq() will look up
+ * the required target for delivery to a specific guest.
*/
-vgic_inject_irq(info->d, NULL, info->virq, true);
+v = test_bit(_IRQ_PER_CPU, >status) ? current : NULL;
+vgic_inject_irq(info->d, v,
[Xen-devel] [RFC XEN PATCH v3 10/11] xen: arm: context switch vtimer PPI state.
From: Ian Campbell
... instead of artificially masking the timer interrupt in the timer
state and relying on the guest to unmask (which it isn't required to
do per the h/w spec, although Linux does).
By using the newly added hwppi save/restore functionality we make use
of the GICD I[SC]ACTIVER registers to save and restore the active
state of the interrupt, which prevents the nested invocations which
the current masking works around.
Signed-off-by: Ian Campbell
Signed-off-by: Stewart Hildebrand
---
v2: Rebased, in particular over Julien's passthrough stuff which
reworked a bunch of IRQ related stuff.
Also largely rewritten since precursor patches now lay very
different groundwork.
v3: Address feedback from v2 [1]:
* Remove virt_timer_irqs performance counter since it is now unused.
* Add caveat to comment about not using I*ACTIVER register.
* HACK: don't initialize pending_irq->irq in vtimer for new vGIC to
allows us to build with CONFIG_NEW_VGIC=y
[1] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01065.html
---
Note: Regarding Stefano's comment in [2], I did test it with the call
to gic_hwppi_set_pending removed, and I was able to boot dom0.
[2] https://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02683.html
---
xen/arch/arm/time.c | 26 ++
xen/arch/arm/vtimer.c| 45 +---
xen/include/asm-arm/domain.h | 1 +
xen/include/asm-arm/perfc_defn.h | 1 -
4 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 739bcf186c..e3a23b8e16 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -243,28 +243,6 @@ static void timer_interrupt(int irq, void *dev_id, struct
cpu_user_regs *regs)
}
}
-static void vtimer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
-{
-/*
- * Edge-triggered interrupts can be used for the virtual timer. Even
- * if the timer output signal is masked in the context switch, the
- * GIC will keep track that of any interrupts raised while IRQS are
- * disabled. As soon as IRQs are re-enabled, the virtual interrupt
- * will be injected to Xen.
- *
- * If an IDLE vCPU was scheduled next then we should ignore the
- * interrupt.
- */
-if ( unlikely(is_idle_vcpu(current)) )
-return;
-
-perfc_incr(virt_timer_irqs);
-
-current->arch.virt_timer.ctl = READ_SYSREG32(CNTV_CTL_EL0);
-WRITE_SYSREG32(current->arch.virt_timer.ctl | CNTx_CTL_MASK, CNTV_CTL_EL0);
-vgic_inject_irq(current->domain, current, current->arch.virt_timer.irq,
true);
-}
-
/*
* Arch timer interrupt really ought to be level triggered, since the
* design of the timer/comparator mechanism is based around that
@@ -304,8 +282,8 @@ void init_timer_interrupt(void)
request_irq(timer_irq[TIMER_HYP_PPI], 0, timer_interrupt,
"hyptimer", NULL);
-request_irq(timer_irq[TIMER_VIRT_PPI], 0, vtimer_interrupt,
- "virtimer", NULL);
+route_hwppi_to_current_vcpu(timer_irq[TIMER_VIRT_PPI], "virtimer");
+
request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], 0, timer_interrupt,
"phytimer", NULL);
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index e6aebdac9e..6e3498952d 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -55,9 +55,19 @@ static void phys_timer_expired(void *data)
static void virt_timer_expired(void *data)
{
struct vtimer *t = data;
-t->ctl |= CNTx_CTL_MASK;
-vgic_inject_irq(t->v->domain, t->v, t->irq, true);
-perfc_incr(vtimer_virt_inject);
+t->ctl |= CNTx_CTL_PENDING;
+if ( !(t->ctl & CNTx_CTL_MASK) )
+{
+/*
+ * An edge triggered interrupt should now be pending. Since
+ * this timer can never expire while the domain is scheduled
+ * we know that the gic_restore_hwppi in virt_timer_restore
+ * will cause the real hwppi to occur and be routed.
+ */
+gic_hwppi_set_pending(>ppi_state);
+vcpu_unblock(t->v);
+perfc_incr(vtimer_virt_inject);
+}
}
int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
@@ -98,9 +108,14 @@ int domain_vtimer_init(struct domain *d, struct
xen_arch_domainconfig *config)
int vcpu_vtimer_init(struct vcpu *v)
{
+#ifndef CONFIG_NEW_VGIC
+struct pending_irq *p;
+#endif
struct vtimer *t = >arch.phys_timer;
bool d0 = is_hardware_domain(v->domain);
+const unsigned host_vtimer_irq_ppi = timer_get_irq(TIMER_VIRT_PPI);
+
/*
* Hardware domain uses the hardware interrupts, guests get the virtual
* platform.
@@ -118,10 +133,18 @@ int vcpu_vtimer_init(struct vcpu *v)
init_timer(>timer, virt_timer_expired, t, v->processor);
t->ctl = 0;
t->irq = d0
-? timer_get_irq(TIMER_VIRT_PPI)
+? host_vtimer_irq_ppi
: GUEST_TIMER_VIRT_PPI;
t->v = v;
[Xen-devel] [XEN PATCH v3 05/11] xen: arm: add interfaces to save/restore the state of a PPI.
From: Ian Campbell
Make use of the GICD I[SC]ACTIVER registers to save and
restore the active state of the interrupt.
For edge triggered interrupts we also need to context switch the
pending bit via I[SC]PENDR. Note that for level triggered interrupts
SPENDR sets a latch which is only cleared by ICPENDR (and not by h/w
state changes), therefore we do not want to context switch the pending
state for level PPIs -- instead we rely on the context switch of the
peripheral to restore the correct level.
Unused as yet, will be used by the vtimer code shortly.
Signed-off-by: Ian Campbell
Signed-off-by: Stewart Hildebrand
---
v3: Address feedback from v2 [1]:
* Add a comment to explain that PPI are always below 31.
* Use uint32_t for pendingr, activer, enabler
* Fixup register names in gic-v3.c
* Add newlines for clarity
* Make gicv3_irq_enable/disable declarations static
* Use readl_relaxed (not readl_gicd) in gic-v3.c
* Add note to comment explaining devices using PPI being quiet during
save/restore. Suggested by Julien.
* Test on QEMU's model of GICv3
Note: I have not given any thought to the comments in [2] regarding
disabling IRQ or enable/disable state.
[1] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01049.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01051.html
---
xen/arch/arm/gic-v2.c| 69
xen/arch/arm/gic-v3.c| 69
xen/arch/arm/gic.c | 54
xen/arch/arm/irq.c | 7
xen/include/asm-arm/domain.h | 11 ++
xen/include/asm-arm/gic.h| 22
xen/include/asm-arm/irq.h| 2 ++
7 files changed, 234 insertions(+)
diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index 256988c665..13f106cb61 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -123,6 +123,9 @@ static DEFINE_PER_CPU(u8, gic_cpu_id);
/* Maximum cpu interface per GIC */
#define NR_GIC_CPU_IF 8
+static void gicv2_irq_enable(struct irq_desc *desc);
+static void gicv2_irq_disable(struct irq_desc *desc);
+
static inline void writeb_gicd(uint8_t val, unsigned int offset)
{
writeb_relaxed(val, gicv2.map_dbase + offset);
@@ -191,6 +194,38 @@ static void gicv2_save_state(struct vcpu *v)
writel_gich(0, GICH_HCR);
}
+static void gicv2_save_and_mask_hwppi(struct irq_desc *desc,
+ struct hwppi_state *s)
+{
+const uint32_t mask = (1u << desc->irq); /* PPIs are IRQ# 16-31 */
+const uint32_t pendingr = readl_gicd(GICD_ISPENDR);
+const uint32_t activer = readl_gicd(GICD_ISACTIVER);
+const uint32_t enabler = readl_gicd(GICD_ISENABLER);
+const bool is_edge = !!(desc->arch.type & DT_IRQ_TYPE_EDGE_BOTH);
+
+s->active = !!(activer & mask);
+s->enabled = !!(enabler & mask);
+s->pending = !!(pendingr & mask);
+
+/* Write a 1 to IC...R to clear the corresponding bit of state */
+if ( s->active )
+writel_gicd(mask, GICD_ICACTIVER);
+
+/*
+ * For an edge interrupt clear the pending state, for a level interrupt
+ * this clears the latch there is no need since saving the peripheral state
+ * (and/or restoring the next VCPU) will cause the correct action.
+ */
+if ( is_edge && s->pending )
+writel_gicd(mask, GICD_ICPENDR);
+
+if ( s->enabled )
+gicv2_irq_disable(desc);
+
+ASSERT(!(readl_gicd(GICD_ISACTIVER) & mask));
+ASSERT(!(readl_gicd(GICD_ISENABLER) & mask));
+}
+
static void gicv2_restore_state(const struct vcpu *v)
{
int i;
@@ -203,6 +238,38 @@ static void gicv2_restore_state(const struct vcpu *v)
writel_gich(GICH_HCR_EN, GICH_HCR);
}
+static void gicv2_restore_hwppi(struct irq_desc *desc,
+const struct hwppi_state *s)
+{
+const uint32_t mask = (1u << desc->irq); /* PPIs are IRQ# 16-31 */
+const bool is_edge = !!(desc->arch.type & DT_IRQ_TYPE_EDGE_BOTH);
+
+/*
+ * The IRQ must always have been set inactive and masked etc by
+ * the saving of the previous state via save_and_mask_hwppi.
+ */
+ASSERT(!(readl_gicd(GICD_ISACTIVER) & mask));
+ASSERT(!(readl_gicd(GICD_ISENABLER) & mask));
+
+if ( s->active )
+writel_gicd(mask, GICD_ICACTIVER);
+
+/*
+ * Restore pending state for edge triggered interrupts only. For
+ * level triggered interrupts the level will be restored as
+ * necessary by restoring the state of the relevant peripheral.
+ *
+ * For a level triggered interrupt ISPENDR acts as a *latch* which
+ * is only cleared by ICPENDR (i.e. the input level is no longer
+ * relevant). We certainly do not want that here.
+ */
+if ( is_edge && s->pending )
+writel_gicd(mask, GICD_ISPENDR);
+
+if ( s->enabled )
+gicv2_irq_enable(desc);
+}
+
static void gicv2_dump_state(const struct vcpu *v)
{
int i;
@@
[Xen-devel] [XEN PATCH v3 06/11] Add NR_SGIS and NR_PPIS definitions to irq.h
These will be used in a follow-up patch.
Signed-off-by: Stewart Hildebrand
---
v3: new patch
---
xen/include/asm-arm/irq.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 3b37a21c06..367fe6269c 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -33,7 +33,9 @@ struct arch_irq_desc {
unsigned int type;
};
-#define NR_LOCAL_IRQS 32
+#define NR_SGIS 16
+#define NR_PPIS 16
+#define NR_LOCAL_IRQS (NR_SGIS + NR_PPIS)
/*
* This only covers the interrupts that Xen cares about, so SGIs, PPIs and
--
2.24.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH v3 07/11] xen: arm: vgic: allow delivery of PPIs to guests
Allow vgic_get_hw_irq_desc to be called with a vcpu argument.
Use vcpu argument in vgic_connect_hw_irq.
vgic_connect_hw_irq is called for PPIs and SPIs, not SGIs. Enforce with
ASSERTs.
Signed-off-by: Stewart Hildebrand
---
v3: new patch
---
Note: I have only modified the old vgic to allow delivery of PPIs.
---
xen/arch/arm/gic-vgic.c | 24
xen/arch/arm/vgic.c | 6 +++---
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index 98c021f1a8..2c66a8fa92 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -418,7 +418,7 @@ struct irq_desc *vgic_get_hw_irq_desc(struct domain *d,
struct vcpu *v,
{
struct pending_irq *p;
-ASSERT(!v && virq >= 32);
+ASSERT((!v && (virq >= 32)) || (!d && v && (virq >= 16) && (virq < 32)));
if ( !v )
v = d->vcpu[0];
@@ -434,15 +434,23 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu *v,
unsigned int virq,
struct irq_desc *desc, bool connect)
{
unsigned long flags;
-/*
- * Use vcpu0 to retrieve the pending_irq struct. Given that we only
- * route SPIs to guests, it doesn't make any difference.
- */
-struct vcpu *v_target = vgic_get_target_vcpu(d->vcpu[0], virq);
-struct vgic_irq_rank *rank = vgic_rank_irq(v_target, virq);
-struct pending_irq *p = irq_to_pending(v_target, virq);
+struct vcpu *v_target;
+struct vgic_irq_rank *rank;
+struct pending_irq *p;
int ret = 0;
+if (v)
+v_target = v;
+else
+/* Use vcpu0 to retrieve the pending_irq struct. */
+v_target = vgic_get_target_vcpu(d->vcpu[0], virq);
+
+rank = vgic_rank_irq(v_target, virq);
+p = irq_to_pending(v_target, virq);
+
+ASSERT(virq >= NR_SGIS);
+ASSERT(p->irq >= NR_SGIS);
+
/* "desc" is optional when we disconnect an IRQ. */
ASSERT(!connect || desc);
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 82f524a35c..c3933c2687 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -410,10 +410,10 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
irq_set_affinity(p->desc, cpumask_of(v_target->processor));
spin_lock_irqsave(>desc->lock, flags);
/*
- * The irq cannot be a PPI, we only support delivery of SPIs
- * to guests.
+ * The irq cannot be a SGI, we only support delivery of SPIs
+ * and PPIs to guests.
*/
-ASSERT(irq >= 32);
+ASSERT(irq >= NR_SGIS);
if ( irq_type_set_by_domain(d) )
gic_set_irq_type(p->desc, vgic_get_virq_type(v, n, i));
p->desc->handler->enable(p->desc);
--
2.24.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [RFC XEN PATCH v3 08/11] xen: arm: vgic: don't fail if IRQ is already connected
There are some IRQs that happen to have multiple "interrupts = < ... >;"
properties with the same IRQ in the device tree. For example:
interrupts = <0 123 4>,
<0 123 4>,
<0 123 4>,
<0 123 4>,
<0 123 4>;
In this case it seems that we are invoking vgic_connect_hw_irq multiple
times for the same IRQ.
Rework the checks to allow booting in this scenario.
I have not seen any cases where the pre-existing p->desc is any different from
the new desc, so BUG() out if they're different for now.
Signed-off-by: Stewart Hildebrand
---
v3: new patch
I tested on Xilinx Zynq UltraScale+ with the old vGIC. I have not fully
tested with CONFIG_NEW_VGIC. This hack only became necessary after
introducing the PPI series, and I'm not entirely sure what the reason
is for that.
I'm also unsure if BUG()ing out is the right thing to do in case of
desc != p->desc, or what conditions would even trigger this? Is this
function exposed to guests?
---
xen/arch/arm/gic-vgic.c | 9 +++--
xen/arch/arm/vgic/vgic.c | 4
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c
index 2c66a8fa92..5c16e66b32 100644
--- a/xen/arch/arm/gic-vgic.c
+++ b/xen/arch/arm/gic-vgic.c
@@ -460,9 +460,14 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu *v,
unsigned int virq,
if ( connect )
{
/* The VIRQ should not be already enabled by the guest */
-if ( !p->desc &&
- !test_bit(GIC_IRQ_GUEST_ENABLED, >status) )
+if ( !test_bit(GIC_IRQ_GUEST_ENABLED, >status) )
+{
+if (p->desc && p->desc != desc)
+{
+BUG();
+}
p->desc = desc;
+}
else
ret = -EBUSY;
}
diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
index f0f2ea5021..aa775f7668 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -882,6 +882,10 @@ int vgic_connect_hw_irq(struct domain *d, struct vcpu
*vcpu,
irq->hw = true;
irq->hwintid = desc->irq;
}
+else if ( irq->hw && !irq->enabled && irq->hwintid == desc->irq )
+{
+/* The IRQ was already connected. No action is necessary. */
+}
else
ret = -EBUSY;
}
--
2.24.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH v3 04/11] xen: arm: remove is_assignable_irq
It only had 1 caller.
Reverse the condition for readability.
Signed-off-by: Stewart Hildebrand
---
v3: new patch
---
xen/arch/arm/irq.c| 9 ++---
xen/include/asm-arm/irq.h | 2 --
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 9cc0a54867..c80782026f 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -390,12 +390,6 @@ err:
return rc;
}
-bool is_assignable_irq(unsigned int irq)
-{
-/* For now, we can only route SPIs to the guest */
-return (irq >= NR_LOCAL_IRQS) && (irq < gic_number_lines());
-}
-
/*
* Only the hardware domain is allowed to set the configure the
* interrupt type for now.
@@ -509,7 +503,8 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
return -EINVAL;
}
-if ( !is_assignable_irq(irq) )
+/* For now, we can only route SPIs to the guest */
+if ( (irq < NR_LOCAL_IRQS) || (irq >= gic_number_lines()) )
{
printk(XENLOG_G_ERR "the IRQ%u is not routable\n", irq);
return -EINVAL;
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index e45d574598..e14001b5c6 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -70,8 +70,6 @@ static inline bool is_lpi(unsigned int irq)
#define domain_pirq_to_irq(d, pirq) (pirq)
-bool is_assignable_irq(unsigned int irq);
-
void init_IRQ(void);
void init_secondary_IRQ(void);
--
2.24.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH v3 01/11] xen: arm: fix indentation of struct vtimer
From: Ian Campbell
Signed-off-by: Ian Campbell
Reviewed-by: Julien Grall [1]
Acked-by: Stefano Stabellini [2]
[1] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg00985.html
[2] https://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02646.html
---
v3:
* Rebase (no conflicts)
* Add Reviewed-by and Acked-by from a few years ago
---
xen/include/asm-arm/domain.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 86ebdd2bcf..f3f3fb7d7f 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -35,11 +35,11 @@ enum domain_type {
#define is_domain_direct_mapped(d) ((d) == hardware_domain)
struct vtimer {
-struct vcpu *v;
-int irq;
-struct timer timer;
-uint32_t ctl;
-uint64_t cval;
+struct vcpu *v;
+int irq;
+struct timer timer;
+uint32_t ctl;
+uint64_t cval;
};
struct arch_domain
--
2.24.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH v3 03/11] xen: arm: Refactor route_irq_to_guest
From: Ian Campbell
Split out the bit which allocates the struct irqaction and calls
__setup_irq into a new function (setup_guest_irq). I'm going to want
to call this a second time in a subsequent patch.
Note that the action is now allocated and initialised with the desc
lock held (since it is taken by the caller). I don't think this is an
issue (and avoiding this would make things more complex)
Signed-off-by: Ian Campbell
Signed-off-by: Stewart Hildebrand
---
v2: New patch (maybe, it's been a while...)
v3: Rebase + trivial fixups
---
Note: I have not given much thought regarding Julien's comment in [1]
[1] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg01041.html
---
xen/arch/arm/irq.c | 108 +++--
1 file changed, 64 insertions(+), 44 deletions(-)
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 3877657a52..9cc0a54867 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -407,61 +407,25 @@ bool irq_type_set_by_domain(const struct domain *d)
return (d == hardware_domain);
}
-/*
- * Route an IRQ to a specific guest.
- * For now only SPIs are assignable to the guest.
- */
-int route_irq_to_guest(struct domain *d, unsigned int virq,
- unsigned int irq, const char * devname)
+static int setup_guest_irq(struct irq_desc *desc, unsigned int virq,
+ unsigned int irqflags,
+ struct irq_guest *info, const char *devname)
{
+const unsigned irq = desc->irq;
struct irqaction *action;
-struct irq_guest *info;
-struct irq_desc *desc;
-unsigned long flags;
-int retval = 0;
-
-if ( virq >= vgic_num_irqs(d) )
-{
-printk(XENLOG_G_ERR
- "the vIRQ number %u is too high for domain %u (max = %u)\n",
- irq, d->domain_id, vgic_num_irqs(d));
-return -EINVAL;
-}
-
-/* Only routing to virtual SPIs is supported */
-if ( virq < NR_LOCAL_IRQS )
-{
-printk(XENLOG_G_ERR "IRQ can only be routed to an SPI\n");
-return -EINVAL;
-}
+int retval;
+struct domain *d = info->d;
-if ( !is_assignable_irq(irq) )
-{
-printk(XENLOG_G_ERR "the IRQ%u is not routable\n", irq);
-return -EINVAL;
-}
-desc = irq_to_desc(irq);
+ASSERT(spin_is_locked(>lock));
action = xmalloc(struct irqaction);
if ( !action )
return -ENOMEM;
-info = xmalloc(struct irq_guest);
-if ( !info )
-{
-xfree(action);
-return -ENOMEM;
-}
-
-info->d = d;
-info->virq = virq;
-
action->dev_id = info;
action->name = devname;
action->free_on_release = 1;
-spin_lock_irqsave(>lock, flags);
-
if ( !irq_type_set_by_domain(d) && desc->arch.type == IRQ_TYPE_INVALID )
{
printk(XENLOG_G_ERR "IRQ %u has not been configured\n", irq);
@@ -496,6 +460,8 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
d->domain_id, irq, irq_get_guest_info(desc)->virq);
retval = -EBUSY;
}
+else
+retval = 0;
}
else
{
@@ -509,6 +475,61 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
if ( retval )
goto out;
+return 0;
+
+out:
+xfree(action);
+return retval;
+}
+
+/*
+ * Route an IRQ to a specific guest.
+ * For now only SPIs are assignable to the guest.
+ */
+int route_irq_to_guest(struct domain *d, unsigned int virq,
+ unsigned int irq, const char * devname)
+{
+struct irq_guest *info;
+struct irq_desc *desc;
+unsigned long flags;
+int retval;
+
+if ( virq >= vgic_num_irqs(d) )
+{
+printk(XENLOG_G_ERR
+ "the vIRQ number %u is too high for domain %u (max = %u)\n",
+ irq, d->domain_id, vgic_num_irqs(d));
+return -EINVAL;
+}
+
+/* Only routing to virtual SPIs is supported */
+if ( virq < NR_LOCAL_IRQS )
+{
+printk(XENLOG_G_ERR "IRQ can only be routed to an SPI\n");
+return -EINVAL;
+}
+
+if ( !is_assignable_irq(irq) )
+{
+printk(XENLOG_G_ERR "the IRQ%u is not routable\n", irq);
+return -EINVAL;
+}
+
+desc = irq_to_desc(irq);
+
+info = xmalloc(struct irq_guest);
+if ( !info )
+return -ENOMEM;
+
+info->d = d;
+info->virq = virq;
+
+spin_lock_irqsave(>lock, flags);
+
+retval = setup_guest_irq(desc, virq, flags, info, devname);
+if ( retval )
+goto out;
+
retval = gic_route_irq_to_guest(d, virq, desc, GIC_PRI_IRQ);
spin_unlock_irqrestore(>lock, flags);
@@ -523,7 +544,6 @@ int route_irq_to_guest(struct domain *d, unsigned int virq,
out:
spin_unlock_irqrestore(>lock, flags);
-xfree(action);
free_info:
xfree(info);
--
2.24.0
___
Xen-devel mailing list
[Xen-devel] [XEN PATCH v3 02/11] xen: arm: fix typo in the description of struct pending_irq->desc
From: Ian Campbell s/it/if/ makes more sense. Signed-off-by: Ian Campbell Reviewed-by: Julien Grall [1] Acked-by: Stefano Stabellini [2] [1] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg00986.html [2] https://lists.xenproject.org/archives/html/xen-devel/2015-12/msg02645.html --- v3: * Rebase (no conflicts) * Add Reviewed-by and Acked-by from a few years ago --- xen/include/asm-arm/vgic.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h index 447d24ea59..ce1e3c4bbd 100644 --- a/xen/include/asm-arm/vgic.h +++ b/xen/include/asm-arm/vgic.h @@ -77,7 +77,7 @@ struct pending_irq #define GIC_IRQ_GUEST_MIGRATING 4 #define GIC_IRQ_GUEST_PRISTINE_LPI 5 unsigned long status; -struct irq_desc *desc; /* only set it the irq corresponds to a physical irq */ +struct irq_desc *desc; /* only set if the irq corresponds to a physical irq */ unsigned int irq; #define GIC_INVALID_LR (uint8_t)~0 uint8_t lr; -- 2.24.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH v3 00/11] xen: arm: context switch vtimer PPI state
This is an update to Ian Campbell's work to route timer PPIs to guests [1]. I attempted to address most of the feedback on v2 of the series. There are a couple of comments I was unsure about - instances of this are noted in the individual patches. Highlights in v3: * Rebase * Tested on QEMU with GICv3 * Tested on Xilinx Zynq UltraScale+ with GICv2 While I build-tested with CONFIG_NEW_VGIC=y, I only did a quick runtime test with the new vGIC and I encountered an ASSERT failure: Assertion 'irq->hwintid >= VGIC_NR_PRIVATE_IRQS' failed at vgic-mmio.c:96 Because of this, and because there is still some feedback outstanding from v2, portions of this series may be considered RFC-ish (especially the last patch "context switch vtimer PPI state"). [1] https://lists.xenproject.org/archives/html/xen-devel/2015-11/msg00921.html Ian Campbell (7): xen: arm: fix indentation of struct vtimer xen: arm: fix typo in the description of struct pending_irq->desc xen: arm: Refactor route_irq_to_guest xen: arm: add interfaces to save/restore the state of a PPI. xen: arm: gic: supporting routing a PPI to the current vcpu. xen: arm: context switch vtimer PPI state. HACK: Force virt timer to PPI0 (IRQ16) Stewart Hildebrand (4): xen: arm: remove is_assignable_irq Add NR_SGIS and NR_PPIS definitions to irq.h xen: arm: vgic: allow delivery of PPIs to guests xen: arm: vgic: don't fail if IRQ is already connected xen/arch/arm/gic-v2.c| 69 +++ xen/arch/arm/gic-v3.c| 69 +++ xen/arch/arm/gic-vgic.c | 33 +++-- xen/arch/arm/gic.c | 79 xen/arch/arm/irq.c | 202 ++- xen/arch/arm/time.c | 26 +--- xen/arch/arm/vgic.c | 6 +- xen/arch/arm/vgic/vgic.c | 4 + xen/arch/arm/vtimer.c| 45 ++- xen/include/asm-arm/domain.h | 22 +++- xen/include/asm-arm/gic.h| 24 xen/include/asm-arm/irq.h| 9 +- xen/include/asm-arm/perfc_defn.h | 1 - xen/include/asm-arm/vgic.h | 2 +- xen/include/public/arch-arm.h| 2 +- 15 files changed, 483 insertions(+), 110 deletions(-) -- 2.24.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ping: [PATCH v2] x86/stackframe/32: repair 32-bit Xen PV
On Fri, Nov 15, 2019 at 6:30 AM Jan Beulich wrote:
>
> Andy,
>
> On 29.10.2019 10:28, Jan Beulich wrote:
> > Once again RPL checks have been introduced which don't account for a
> > 32-bit kernel living in ring 1 when running in a PV Xen domain. The
> > case in FIXUP_FRAME has been preventing boot; adjust BUG_IF_WRONG_CR3
> > as well just in case.
> >
> > Fixes: 3c88c692c287 ("x86/stackframe/32: Provide consistent pt_regs")
> > Signed-off-by: Jan Beulich
>
> would you mind clarifying whether I should follow Thomas' request,
> overriding what you had asked for an I did carry out for v2? I don't
> think this regression should be left unfixed for much longer (as
> much as the other part of it, addressed by a later 2-patch series).
>
I'm fine with doing it Thomas' way.
--Andy
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 21/22] 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
---
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 210a418c02..4e12cebac1 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -74,6 +74,39 @@ func (e Error) Error() string {
return fmt.Sprintf("libxl error: %d", -e)
}
+// Context represents a libxl_ctx.
+type Context struct {
+ ctx*C.libxl_ctx
+ logger *C.xentoollog_logger_stdiostream
+}
+
+// NewContext returns a new Context.
+func NewContext() (*Context, error) {
+ var ctx Context
+
+ ctx.logger = C.xtl_createlogger_stdiostream(C.stderr, C.XTL_ERROR, 0)
+
+ ret := C.libxl_ctx_alloc(, C.LIBXL_VERSION, 0,
(*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+ if ret != 0 {
+ return nil, Error(ret)
+ }
+
+ return , nil
+}
+
+// Close closes the Context.
+func (ctx *Context) Close() error {
+ ret := C.libxl_ctx_free(ctx.ctx)
+ ctx.ctx = nil
+ C.xtl_logger_destroy((*C.xentoollog_logger)(unsafe.Pointer(ctx.logger)))
+
+ if ret != 0 {
+ return Error(ret)
+ }
+
+ return nil
+}
+
/*
* Types: Builtins
*/
@@ -304,11 +337,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
@@ -463,11 +491,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, )
@@ -491,16 +514,11 @@ func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
// int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t
poolid);
func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo) {
- err := Ctx.CheckOpen()
- if err != nil {
- return
- }
-
var c_cpupool C.libxl_cpupoolinfo
ret := C.libxl_cpupool_info(Ctx.ctx, _cpupool, C.uint32_t(Poolid))
if ret != 0 {
- err = Error(-ret)
+ //err = Error(-ret)
return
}
defer C.libxl_cpupoolinfo_dispose(_cpupool)
@@ -517,11 +535,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))
@@ -550,11 +563,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)
@@ -566,11 +574,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)
@@ -583,11 +586,6 @@ func (Ctx *Context) CpupoolCpuadd(Poolid uint32, Cpu int)
(err error) {
// int libxl_cpupool_cpuadd_cpumap(libxl_ctx *ctx, uint32_t poolid,
// const libxl_bitmap *cpumap);
func (Ctx *Context) CpupoolCpuaddCpumap(Poolid uint32, Cpumap Bitmap) (err
error) {
- err = Ctx.CheckOpen()
- if err != nil {
- return
- }
-
cbm, err := Cpumap.toC()
if err
[Xen-devel] [PATCH v2 20/22] golang/xenlight: implement array Go to C marshaling
From: Nick Rosbrook
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/gengotypes.py | 44 +++-
tools/golang/xenlight/helpers.gen.go | 359 +++
2 files changed, 402 insertions(+), 1 deletion(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 5fa070b320..d67e21b47b 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -500,7 +500,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)
@@ -631,6 +631,48 @@ def xenlight_golang_union_to_C(ty = None, union_name = '',
return s
+def xenlight_golang_array_to_C(ty = None, dispose_fn = ''):
+s = ''
+
+gotypename = xenlight_golang_fmt_name(ty.type.elem_type.typename)
+goname = xenlight_golang_fmt_name(ty.name)
+ctypename = ty.type.elem_type.typename
+cname = ty.name
+clenvar= ty.type.lenvar.name
+golenvar = xenlight_golang_fmt_name(clenvar,exported=False)
+
+is_enum = isinstance(ty.type.elem_type,idl.Enumeration)
+if gotypename in go_builtin_types or is_enum:
+s += '{} := len(x.{})\n'.format(golenvar,goname)
+s += 'xc.{} =
(*C.{})(C.malloc(C.size_t({}*{})))\n'.format(cname,ctypename,
+
golenvar,golenvar)
+s += 'xc.{} = C.int({})\n'.format(clenvar,golenvar)
+s += 'c{} :=
(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(goname,
+
ctypename,cname,
+
golenvar,golenvar)
+s += 'for i,v := range x.{} {{\n'.format(goname)
+s += 'c{}[i] = C.{}(v)\n'.format(goname,ctypename)
+s += '}\n'
+
+return s
+
+s += '{} := len(x.{})\n'.format(golenvar,goname)
+s += 'xc.{} =
(*C.{})(C.malloc(C.ulong({})*C.sizeof_{}))\n'.format(cname,ctypename,
+
golenvar,ctypename)
+s += 'xc.{} = C.int({})\n'.format(clenvar,golenvar)
+s += 'c{} :=
(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(goname,
+
ctypename,cname,
+
golenvar,golenvar)
+s += 'for i,v := range x.{} {{\n'.format(goname)
+s += 'tmp, err := v.toC()\n'
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,err\n}\n'
+s += 'c{}[i] = tmp\n'.format(goname)
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index bc94e13d70..3d4bff1e7b 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -664,6 +664,18 @@ func (x *VcpuSchedParams) fromC(xc
*C.libxl_vcpu_sched_params) error {
func (x *VcpuSchedParams) toC() (xc C.libxl_vcpu_sched_params, err error) {
C.libxl_vcpu_sched_params_init()
xc.sched = C.libxl_scheduler(x.Sched)
+ numVcpus := len(x.Vcpus)
+ xc.vcpus = (*C.libxl_sched_params)(C.malloc(C.ulong(numVcpus) *
C.sizeof_libxl_sched_params))
+ xc.num_vcpus = C.int(numVcpus)
+ cVcpus := (*[1 <<
28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
+ for i, v := range x.Vcpus {
+ tmp, err := v.toC()
+ if err != nil {
+ C.libxl_vcpu_sched_params_dispose()
+ return xc, err
+ }
+ cVcpus[i] = tmp
+ }
return xc, nil
}
@@ -712,6 +724,13 @@ func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
func (x *VnodeInfo) toC() (xc C.libxl_vnode_info, err error) {
C.libxl_vnode_info_init()
xc.memkb = C.uint64_t(x.Memkb)
+ numDistances := len(x.Distances)
+ xc.distances = (*C.uint32_t)(C.malloc(C.size_t(numDistances *
numDistances)))
+ xc.num_distances = C.int(numDistances)
+ cDistances := (*[1 <<
28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
+ for i, v := range x.Distances {
+ cDistances[i] = C.uint32_t(v)
+ }
xc.pnode = C.uint32_t(x.Pnode)
xc.vcpus, err = x.Vcpus.toC()
if err != nil {
@@ -1099,6 +1118,30 @@ func (x *DomainBuildInfo) toC() (xc
C.libxl_domain_build_info, err error) {
C.libxl_domain_build_info_dispose()
return xc, err
}
+
[Xen-devel] [PATCH v2 17/22] golang/xenlight: implement array C to Go marshaling
From: Nick Rosbrook
Signed-off-by: Nick Rosbrook
---
tools/golang/xenlight/gengotypes.py | 39 +++-
tools/golang/xenlight/helpers.gen.go | 300 +++
2 files changed, 338 insertions(+), 1 deletion(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 57cecd5989..7083ccc871 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -270,7 +270,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)
@@ -441,6 +441,43 @@ def xenlight_golang_union_fields_from_C(ty = None):
return s
+def xenlight_golang_array_from_C(ty = None):
+"""
+Convert C array to Go slice using the method
+described here:
+
+https://github.com/golang/go/wiki/cgo#turning-c-arrays-into-go-slices
+"""
+s = ''
+
+gotypename = xenlight_golang_fmt_name(ty.type.elem_type.typename)
+goname = xenlight_golang_fmt_name(ty.name)
+ctypename = ty.type.elem_type.typename
+cname = ty.name
+cslice = 'c{}'.format(goname)
+clenvar= ty.type.lenvar.name
+golenvar = xenlight_golang_fmt_name(clenvar,exported=False)
+
+s += '{} := int(xc.{})\n'.format(golenvar, clenvar)
+s += '{} := '.format(cslice)
+s +='(*[1<<28]C.{})(unsafe.Pointer(xc.{}))[:{}:{}]\n'.format(ctypename,
cname,
+golenvar,
golenvar)
+s += 'x.{} = make([]{}, {})\n'.format(goname, gotypename, golenvar)
+s += 'for i, v := range {} {{\n'.format(cslice)
+
+is_enum = isinstance(ty.type.elem_type,idl.Enumeration)
+if gotypename in go_builtin_types or is_enum:
+s += 'x.{}[i] = {}(v)\n'.format(goname, gotypename)
+else:
+s += 'var e {}\n'.format(gotypename)
+s += 'if err := e.fromC(); err != nil {\n'
+s += 'return err }\n'
+s += 'x.{}[i] = e\n'.format(goname)
+
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index d2dd9f2507..abdbae1dc7 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -383,6 +383,16 @@ func (x *SchedParams) fromC(xc *C.libxl_sched_params)
error {
func (x *VcpuSchedParams) fromC(xc *C.libxl_vcpu_sched_params) error {
x.Sched = Scheduler(xc.sched)
+ numVcpus := int(xc.num_vcpus)
+ cVcpus := (*[1 <<
28]C.libxl_sched_params)(unsafe.Pointer(xc.vcpus))[:numVcpus:numVcpus]
+ x.Vcpus = make([]SchedParams, numVcpus)
+ for i, v := range cVcpus {
+ var e SchedParams
+ if err := e.fromC(); err != nil {
+ return err
+ }
+ x.Vcpus[i] = e
+ }
return nil
}
@@ -400,6 +410,12 @@ func (x *DomainSchedParams) fromC(xc
*C.libxl_domain_sched_params) error {
func (x *VnodeInfo) fromC(xc *C.libxl_vnode_info) error {
x.Memkb = uint64(xc.memkb)
+ numDistances := int(xc.num_distances)
+ cDistances := (*[1 <<
28]C.uint32_t)(unsafe.Pointer(xc.distances))[:numDistances:numDistances]
+ x.Distances = make([]uint32, numDistances)
+ for i, v := range cDistances {
+ x.Distances[i] = uint32(v)
+ }
x.Pnode = uint32(xc.pnode)
var bitmapVcpus Bitmap
if err := bitmapVcpus.fromC(); err != nil {
@@ -432,6 +448,26 @@ func (x *DomainBuildInfo) fromC(xc
*C.libxl_domain_build_info) error {
return err
}
x.Nodemap = bitmapNodemap
+ numVcpuHardAffinity := int(xc.num_vcpu_hard_affinity)
+ cVcpuHardAffinity := (*[1 <<
28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_hard_affinity))[:numVcpuHardAffinity:numVcpuHardAffinity]
+ x.VcpuHardAffinity = make([]Bitmap, numVcpuHardAffinity)
+ for i, v := range cVcpuHardAffinity {
+ var e Bitmap
+ if err := e.fromC(); err != nil {
+ return err
+ }
+ x.VcpuHardAffinity[i] = e
+ }
+ numVcpuSoftAffinity := int(xc.num_vcpu_soft_affinity)
+ cVcpuSoftAffinity := (*[1 <<
28]C.libxl_bitmap)(unsafe.Pointer(xc.vcpu_soft_affinity))[:numVcpuSoftAffinity:numVcpuSoftAffinity]
+ x.VcpuSoftAffinity = make([]Bitmap, numVcpuSoftAffinity)
+ for i, v := range cVcpuSoftAffinity {
+ var e Bitmap
+ if err := e.fromC(); err != nil {
+ return err
+ }
+ x.VcpuSoftAffinity[i] = e
+ }
var defboolNumaPlacement Defbool
[Xen-devel] [PATCH v2 16/22] 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
---
Changes in v2:
- Do not use global variable for extra helper function definitions.
Instead, return a tuple which contains a list of extra helper
functions associated with the original.
tools/golang/xenlight/gengotypes.py | 153 +-
tools/golang/xenlight/helpers.gen.go | 440 +++
2 files changed, 590 insertions(+), 3 deletions(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 0c8a1327a1..57cecd5989 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -24,6 +24,10 @@ go_keywords = ['type', 'func']
go_builtin_types = ['bool', 'string', 'int', 'byte',
'uint16', 'uint32', 'uint64']
+# cgo preamble for xenlight_helpers.go, created during type generation and
+# written later.
+cgo_helpers_preamble = []
+
def xenlight_golang_generate_types(path = None, types = None, comment = None):
"""
Generate a .go file (types.gen.go by default)
@@ -168,6 +172,8 @@ def xenlight_golang_define_union(ty = None, structname =
''):
extras.append(r[0])
extras.extend(r[1])
+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)
@@ -182,6 +188,18 @@ def xenlight_golang_define_union(ty = None, structname =
''):
return (s,extras)
+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 (helpers.gen.go by default)
@@ -195,6 +213,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')
@@ -203,19 +222,38 @@ 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:
if not isinstance(ty, idl.Struct):
continue
-f.write(xenlight_golang_define_from_C(ty))
+(fdef, extras) = xenlight_golang_define_from_C(ty)
+
+f.write(fdef)
f.write('\n')
+for extra in extras:
+f.write(extra)
+f.write('\n')
+
go_fmt(path)
def xenlight_golang_define_from_C(ty = None, typename = None, nested = False):
+"""
+Define the fromC helper function for ty.
+
+Return a tuple that contains a string with the
+function definition, and a (potentially empty) list
+of extra definitions that are associated with
+this helper function.
+"""
s = ''
+extras = []
gotypename = ctypename = ''
@@ -280,10 +318,16 @@ def xenlight_golang_define_from_C(ty = None, typename =
None, nested = False):
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)
+r = xenlight_golang_define_from_C(f.type, typename=f.name,
nested=True)
+
+s += r[0]
+extras.extend(r[1])
elif isinstance(f.type, idl.KeyedUnion):
-pass
+r = xenlight_golang_union_from_C(f.type, f.name, ty.typename)
+
+s += r[0]
+extras.extend(r[1])
else:
raise Exception('type {} not supported'.format(f.type))
@@ -292,6 +336,109 @@ def xenlight_golang_define_from_C(ty = None, typename =
None, nested = False):
s += 'return nil'
s += '}\n'
+return (s,extras)
+
+def xenlight_golang_union_from_C(ty = None, union_name = '', struct_name = ''):
+extras = []
+
+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 =
[Xen-devel] [PATCH v2 18/22] 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
---
tools/golang/xenlight/gengotypes.py | 80 ++
tools/golang/xenlight/helpers.gen.go | 1015 ++
2 files changed, 1095 insertions(+)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 7083ccc871..1522633f83 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -241,6 +241,9 @@ def xenlight_golang_generate_helpers(path = None, types =
None, comment = None):
f.write(extra)
f.write('\n')
+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):
@@ -478,6 +481,83 @@ def xenlight_golang_array_from_C(ty = None):
return s
+def xenlight_golang_define_to_C(ty = None, typename = None, nested = False):
+s = ''
+
+gotypename = ctypename = ''
+
+if typename is not None:
+gotypename = xenlight_golang_fmt_name(typename)
+ctypename = typename
+else:
+gotypename = xenlight_golang_fmt_name(ty.typename)
+ctypename = ty.typename
+
+if not nested:
+s += 'func (x *{}) toC() (xc C.{},err error)
{{\n'.format(gotypename,ctypename)
+s += 'C.{}()\n'.format(ty.init_fn)
+
+for f in ty.fields:
+if f.type.typename is not None:
+if isinstance(f.type, idl.Array):
+# TODO
+continue
+
+gotypename = xenlight_golang_fmt_name(f.type.typename)
+ctypename = f.type.typename
+gofname= xenlight_golang_fmt_name(f.name)
+cfname = f.name
+
+# In cgo, C names that conflict with Go keywords can be
+# accessed by prepending an underscore to the name.
+if cfname in go_keywords:
+cfname = '_' + cfname
+
+# If this is nested, we need the outer name too.
+if nested and typename is not None:
+goname = xenlight_golang_fmt_name(typename)
+goname = '{}.{}'.format(goname, gofname)
+cname = '{}.{}'.format(typename, cfname)
+
+else:
+goname = gofname
+cname = cfname
+
+is_castable = (f.type.json_parse_type == 'JSON_INTEGER' or
+ isinstance(f.type, idl.Enumeration) or
+ gotypename in go_builtin_types)
+
+if is_castable:
+# Use the cgo helper for converting C strings.
+if gotypename == 'string':
+s += 'xc.{} = C.CString(x.{})\n'.format(cname,goname)
+continue
+
+s += 'xc.{} = C.{}(x.{})\n'.format(cname,ctypename,goname)
+
+else:
+s += 'xc.{}, err = x.{}.toC()\n'.format(cname,goname)
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(ty.dispose_fn)
+s += 'return xc, err\n'
+s += '}\n'
+
+elif isinstance(f.type, idl.Struct):
+s += xenlight_golang_define_to_C(f.type, typename=f.name,
nested=True)
+
+elif isinstance(f.type, idl.KeyedUnion):
+# TODO
+pass
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+if not nested:
+s += 'return xc, nil'
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index abdbae1dc7..495601f738 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -130,6 +130,13 @@ func (x *IoportRange) fromC(xc *C.libxl_ioport_range)
error {
return nil
}
+func (x *IoportRange) toC() (xc C.libxl_ioport_range, err error) {
+ C.libxl_ioport_range_init()
+ xc.first = C.uint32_t(x.First)
+ xc.number = C.uint32_t(x.Number)
+ return xc, nil
+}
+
func (x *IomemRange) fromC(xc *C.libxl_iomem_range) error {
x.Start = uint64(xc.start)
x.Number = uint64(xc.number)
@@ -137,11 +144,25 @@ func (x *IomemRange) fromC(xc *C.libxl_iomem_range) error
{
return nil
}
+func (x *IomemRange) toC() (xc C.libxl_iomem_range, err error) {
+ C.libxl_iomem_range_init()
+ xc.start = C.uint64_t(x.Start)
+ xc.number = C.uint64_t(x.Number)
+ xc.gfn = C.uint64_t(x.Gfn)
+ return xc, nil
+}
+
func (x *VgaInterfaceInfo) fromC(xc *C.libxl_vga_interface_info) error {
x.Kind = VgaInterfaceType(xc.kind)
return nil
}
+func (x *VgaInterfaceInfo) toC() (xc C.libxl_vga_interface_info, err error) {
+ C.libxl_vga_interface_info_init()
+
[Xen-devel] [PATCH v2 15/22] 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
---
Changes in v2:
- Add Makefile changes for helpers.gen.go.
- Re-generate helpers.gen.go to include libxl changes after rebase.
tools/golang/xenlight/Makefile | 2 +
tools/golang/xenlight/gengotypes.py | 120
tools/golang/xenlight/helpers.gen.go | 969 +++
tools/golang/xenlight/xenlight.go| 111 +--
4 files changed, 1102 insertions(+), 100 deletions(-)
create mode 100644 tools/golang/xenlight/helpers.gen.go
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index 681f32c234..07b8896e5b 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -19,6 +19,7 @@ $(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: %.gen.go
$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
$(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
$(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+ $(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
%.gen.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
@@ -39,6 +40,7 @@ install: build
$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)xenlight.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)types.gen.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
+ $(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)helpers.gen.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
.PHONY: uninstall
rm -rf $(DESTDIR)$(GOXL_INSTALL_DIR)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 8963b14eee..0c8a1327a1 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']
+
def xenlight_golang_generate_types(path = None, types = None, comment = None):
"""
Generate a .go file (types.gen.go by default)
@@ -176,6 +182,118 @@ def xenlight_golang_define_union(ty = None, structname =
''):
return (s,extras)
+def xenlight_golang_generate_helpers(path = None, types = None, comment =
None):
+"""
+Generate a .go file (helpers.gen.go by default)
+that contains helper functions for marshaling between
+C and Go types.
+"""
+if path is None:
+path = 'helpers.gen.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 ==
[Xen-devel] [PATCH v2 22/22] 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
---
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 4e12cebac1..becd07b0b6 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -513,17 +513,17 @@ func (Ctx *Context) ListCpupool() (list []Cpupoolinfo) {
}
// int libxl_cpupool_info(libxl_ctx *ctx, libxl_cpupoolinfo *info, uint32_t
poolid);
-func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo) {
+func (Ctx *Context) CpupoolInfo(Poolid uint32) (pool Cpupoolinfo, err error) {
var c_cpupool C.libxl_cpupoolinfo
ret := C.libxl_cpupool_info(Ctx.ctx, _cpupool, C.uint32_t(Poolid))
if ret != 0 {
- //err = Error(-ret)
+ err = Error(-ret)
return
}
defer C.libxl_cpupoolinfo_dispose(_cpupool)
- _ = pool.fromC(_cpupool)
+ err = pool.fromC(_cpupool)
return
}
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 19/22] 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
---
tools/golang/xenlight/gengotypes.py | 77 ++-
tools/golang/xenlight/helpers.gen.go | 325 +++
2 files changed, 400 insertions(+), 2 deletions(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 1522633f83..5fa070b320 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -546,8 +546,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))
@@ -558,6 +557,80 @@ def xenlight_golang_define_to_C(ty = None, typename =
None, nested = False):
return s
+def xenlight_golang_union_to_C(ty = None, union_name = '',
+ struct_name = '', dispose_fn = ''):
+keyname = ty.keyvar.name
+gokeyname = xenlight_golang_fmt_name(keyname)
+keytype = ty.keyvar.type.typename
+gokeytype = xenlight_golang_fmt_name(keytype)
+
+interface_name = '{}_{}_union'.format(struct_name, keyname)
+interface_name = xenlight_golang_fmt_name(interface_name, exported=False)
+
+cgo_keyname = keyname
+if cgo_keyname in go_keywords:
+cgo_keyname = '_' + cgo_keyname
+
+
+s = 'xc.{} = C.{}(x.{})\n'.format(cgo_keyname,keytype,gokeyname)
+s += 'switch x.{}{{\n'.format(gokeyname)
+
+# Create switch statement to determine how to populate the C union.
+for f in ty.fields:
+key_val = '{}_{}'.format(keytype, f.name)
+key_val = xenlight_golang_fmt_name(key_val)
+if f.type is None:
+continue
+
+s += 'case {}:\n'.format(key_val)
+cgotype = '{}_{}_union_{}'.format(struct_name,keyname,f.name)
+gotype = xenlight_golang_fmt_name(cgotype)
+goname = '{}_{}'.format(keyname,f.name)
+goname = xenlight_golang_fmt_name(goname,exported=False)
+
+field_name = xenlight_golang_fmt_name('{}_union'.format(keyname))
+s += 'tmp, ok := x.{}.({})\n'.format(field_name,gotype)
+s += 'if !ok {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,errors.New("wrong type for union key
{}")\n'.format(keyname)
+s += '}\n'
+
+s += 'var {} C.{}\n'.format(f.name,cgotype)
+for uf in f.type.fields:
+gotypename = xenlight_golang_fmt_name(uf.type.typename)
+ctypename = uf.type.typename
+gofname= xenlight_golang_fmt_name(uf.name)
+
+is_castable = (uf.type.json_parse_type == 'JSON_INTEGER' or
+ isinstance(uf.type, idl.Enumeration) or
+ gotypename in go_builtin_types)
+
+if not is_castable:
+s += '{}.{}, err =
tmp.{}.toC()\n'.format(f.name,uf.name,gofname)
+s += 'if err != nil {\n'
+s += 'C.{}()\n'.format(dispose_fn)
+s += 'return xc,err \n}\n'
+
+elif gotypename == 'string':
+s += '{}.{} =
C.CString(tmp.{})\n'.format(f.name,uf.name,gofname)
+
+else:
+s += '{}.{} =
C.{}(tmp.{})\n'.format(f.name,uf.name,ctypename,gofname)
+
+# The union is still represented as Go []byte.
+s += '{}Bytes :=
C.GoBytes(unsafe.Pointer(&{}),C.sizeof_{})\n'.format(f.name,
+
f.name,
+
cgotype)
+s += 'copy(xc.{}[:],{}Bytes)\n'.format(union_name,f.name)
+
+# End switch statement
+s += 'default:\n'
+err_string = '"invalid union key \'%v\'", x.{}'.format(gokeyname)
+s += 'return xc, fmt.Errorf({})'.format(err_string)
+s += '}\n'
+
+return s
+
def xenlight_golang_fmt_name(name, exported = True):
"""
Take a given type name and return an
diff --git a/tools/golang/xenlight/helpers.gen.go
b/tools/golang/xenlight/helpers.gen.go
index 495601f738..bc94e13d70 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.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 {
+ case ChannelConnectionPty:
+ tmp, ok :=
[Xen-devel] [PATCH v2 13/22] 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
---
Changes in v2:
- Do not use global variables for extra type definitions. Instead,
return a tuple which includes a list of extra type definitions
associated with the original type.
- Re-generate types.gen.go to include changes to libxl after rebase.
tools/golang/xenlight/gengotypes.py | 119 +++-
tools/golang/xenlight/types.gen.go | 836
tools/golang/xenlight/xenlight.go | 123 +---
3 files changed, 966 insertions(+), 112 deletions(-)
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
index 2211541547..8963b14eee 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -32,18 +32,32 @@ def xenlight_golang_generate_types(path = None, types =
None, comment = None):
f.write('package xenlight\n')
for ty in types:
-f.write(xenlight_golang_type_define(ty))
+(tdef, extras) = xenlight_golang_type_define(ty)
+
+f.write(tdef)
f.write('\n')
+# Append extra types
+for extra in extras:
+f.write(extra)
+f.write('\n')
+
go_fmt(path)
def xenlight_golang_type_define(ty = None):
-s = ''
+"""
+Generate the Go type definition of ty.
+Return a tuple that contains a string with the
+type definition, and a (potentially empty) list
+of extra definitions that are associated with
+this type.
+"""
if isinstance(ty, idl.Enumeration):
-s += xenlight_golang_define_enum(ty)
+return (xenlight_golang_define_enum(ty), [])
-return s
+elif isinstance(ty, idl.Aggregate):
+return xenlight_golang_define_struct(ty)
def xenlight_golang_define_enum(ty = None):
s = ''
@@ -65,6 +79,103 @@ def xenlight_golang_define_enum(ty = None):
return s
+def xenlight_golang_define_struct(ty = None, typename = None, nested = False):
+s = ''
+extras = []
+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):
+r = xenlight_golang_define_struct(f.type, typename=f.name,
nested=True)
+
+s += r[0]
+extras.extend(r[1])
+
+elif isinstance(f.type, idl.KeyedUnion):
+r = xenlight_golang_define_union(f.type, ty.typename)
+
+s += r[0]
+extras.extend(r[1])
+
+else:
+raise Exception('type {} not supported'.format(f.type))
+
+# End struct definition
+s += '}\n'
+
+return (s,extras)
+
+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 = ''
+extras
[Xen-devel] [PATCH v2 07/22] 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
---
Changes in v2:
- Fix the format string in String function to use %02x.
- Use a value reciever for the toC function.
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 72afc3cf14..eb0d309543 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 := "%02x:%02x:%02x:%02x:%02x:%02x"
+ 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 v2 04/22] golang/xenlight: define KeyValueList as empty struct
From: Nick Rosbrook
Define KeyValueList as empty struct as there is currently no reason for
this type to be available in the Go package.
Implement fromC and toC functions as no-ops.
Signed-off-by: Nick Rosbrook
---
Changes in v2:
- Re-define KeyValueList as empty struct, as it was decided this type
probably shouldn't be exposed in the Go package.
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 8ac26e63f0..3edff18471 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -202,6 +202,16 @@ func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
return
}
+// KeyValueList represents a libxl_key_value_list.
+//
+// Represented as an empty struct for now, as there is no
+// apparent need for this type to be exposed through the
+// Go package.
+type KeyValueList struct{}
+
+func (kvl KeyValueList) fromC(ckvl *C.libxl_key_value_list) error {
return nil }
+func (kvl KeyValueList) toC() (ckvl C.libxl_key_value_list, err error) {
return }
+
// typedef struct {
// uint32_t size; /* number of bytes in map */
// uint8_t *map;
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 02/22] 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
Reviewed-by: George Dunlap
---
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 89ed439fd0..640d82f35f 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -85,6 +85,99 @@ type MemKB uint64
type Uuid C.libxl_uuid
+// defboolVal represents a defbool value.
+type defboolVal int
+
+const (
+ defboolDefault defboolVal = 0
+ defboolFalse defboolVal = -1
+ defboolTruedefboolVal = 1
+)
+
+// Defbool represents a libxl_defbool.
+type Defbool struct {
+ val defboolVal
+}
+
+func (d Defbool) String() string {
+ switch d.val {
+ case defboolDefault:
+ return ""
+ case defboolFalse:
+ return "False"
+ case defboolTrue:
+ return "True"
+ }
+
+ return ""
+}
+
+// Set sets the value of the Defbool.
+func (d *Defbool) Set(b bool) {
+ if b {
+ d.val = defboolTrue
+ return
+ }
+ d.val = defboolFalse
+}
+
+// Unset resets the Defbool to default value.
+func (d *Defbool) Unset() {
+ d.val = defboolDefault
+}
+
+// SetIfDefault sets the value of Defbool only if
+// its current value is default.
+func (d *Defbool) SetIfDefault(b bool) {
+ if d.IsDefault() {
+ d.Set(b)
+ }
+}
+
+// IsDefault returns true if the value of Defbool
+// is default, returns false otherwise.
+func (d *Defbool) IsDefault() bool {
+ return d.val == defboolDefault
+}
+
+// Val returns the boolean value associated with the
+// Defbool value. An error is returned if the value
+// is default.
+func (d *Defbool) Val() (bool, error) {
+ if d.IsDefault() {
+ return false, fmt.Errorf("%v: cannot take value of default
defbool", ErrorInval)
+ }
+
+ return (d.val > 0), nil
+}
+
+func (d *Defbool) fromC(c *C.libxl_defbool) error {
+ if C.libxl_defbool_is_default(*c) {
+ d.val = defboolDefault
+ return nil
+ }
+
+ if C.libxl_defbool_val(*c) {
+ d.val = defboolTrue
+ return nil
+ }
+
+ d.val = defboolFalse
+
+ return nil
+}
+
+func (d *Defbool) toC() (C.libxl_defbool, error) {
+ var c C.libxl_defbool
+
+ if !d.IsDefault() {
+ val, _ := d.Val()
+ C.libxl_defbool_set(, C.bool(val))
+ }
+
+ return c, nil
+}
+
type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 12/22] 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
Reviewed-by: George Dunlap
---
Changes in v2:
- Fix comment in fromC since an array is being used now, not a slice.
- Use a concise variable name instead of mapslice for the C array.
tools/golang/xenlight/xenlight.go | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 67c1bb1225..d6d912a037 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -312,20 +312,29 @@ type Context struct {
logger *C.xentoollog_logger_stdiostream
}
-type Hwcap []C.uint32_t
+// Hwcap represents a libxl_hwcap.
+type Hwcap [8]uint32
-func (chwcap C.libxl_hwcap) toGo() (ghwcap Hwcap) {
- // Alloc a Go slice for the bytes
- size := 8
- ghwcap = make([]C.uint32_t, size)
-
- // Make a slice pointing to the C array
- mapslice := (*[1 <<
30]C.uint32_t)(unsafe.Pointer([0]))[:size:size]
+func (hwcap *Hwcap) fromC(chwcap *C.libxl_hwcap) error {
+ // Make an array pointing to the C array
+ a := (*[8]C.uint32_t)(unsafe.Pointer(chwcap))
// And copy the C array into the Go array
- copy(ghwcap, mapslice)
+ for i, v := range a {
+ 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.
@@ -448,7 +457,7 @@ func (cphys *C.libxl_physinfo) toGo() (physinfo *Physinfo) {
physinfo.SharingFreedPages = uint64(cphys.sharing_freed_pages)
physinfo.SharingUsedFrames = uint64(cphys.sharing_used_frames)
physinfo.NrNodes = uint32(cphys.nr_nodes)
- physinfo.HwCap = cphys.hw_cap.toGo()
+ physinfo.HwCap.fromC(_cap)
physinfo.CapHvm = bool(cphys.cap_hvm)
physinfo.CapHvmDirectio = bool(cphys.cap_hvm_directio)
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 09/22] 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
Reviewed-by: George Dunlap
---
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 108b50124a..d57f780116 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 v2 14/22] golang/xenlight: remove no-longer used type MemKB
From: Nick Rosbrook Signed-off-by: Nick Rosbrook Acked-by: George Dunlap --- 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 c2764af277..9420197bfb 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 v2 01/22] 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
---
Changes in v2:
- Introduce Makefile targets for code generation
- Re-generate Go code (includes new libxl_passtrhough enum).
- Use *.gen.go naming convention for generated Go files.
tools/golang/xenlight/Makefile | 18 +-
tools/golang/xenlight/gengotypes.py | 109
tools/golang/xenlight/types.gen.go | 388
tools/golang/xenlight/xenlight.go | 140 ++
4 files changed, 535 insertions(+), 120 deletions(-)
create mode 100644 tools/golang/xenlight/gengotypes.py
create mode 100644 tools/golang/xenlight/types.gen.go
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index 0987305224..681f32c234 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -7,20 +7,21 @@ 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/: %.gen.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) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
+
+%.gen.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 +37,11 @@ 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)types.gen.go
$(DESTDIR)$(GOXL_INSTALL_DIR)
.PHONY: uninstall
- rm -f $(addprefix $(DESTDIR)$(GOXL_INSTALL_DIR)/, $(PKGSOURCES))
+ rm -rf $(DESTDIR)$(GOXL_INSTALL_DIR)
.PHONY: clean
clean:
diff --git a/tools/golang/xenlight/gengotypes.py
b/tools/golang/xenlight/gengotypes.py
new file mode 100644
index 00..2211541547
--- /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 (types.gen.go by default)
+that contains a Go type for each type in types.
+"""
+if path is None:
+path = 'types.gen.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
[Xen-devel] [PATCH v2 11/22] 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
---
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 6f0a9278ad..67c1bb1225 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -86,7 +86,40 @@ type Devid int
type MemKB uint64
-type Uuid C.libxl_uuid
+// Uuid is a domain UUID.
+type Uuid [16]byte
+
+// String formats a Uuid in the form "-xx-xx-xx-xx".
+func (u Uuid) String() string {
+ s := "%x%x%x%x-%x%x-%x%x-%x%x-%x%x%x%x%x%x"
+ opts := make([]interface{}, 16)
+
+ for i, v := range u {
+ opts[i] = v
+ }
+
+ return fmt.Sprintf(s, opts...)
+}
+
+func (u *Uuid) fromC(c *C.libxl_uuid) error {
+ b := (*[16]C.uint8_t)(unsafe.Pointer([0]))
+
+ for i, v := range b {
+ u[i] = byte(v)
+ }
+
+ return nil
+}
+
+func (u *Uuid) toC() (C.libxl_uuid, error) {
+ var c C.libxl_uuid
+
+ for i, v := range u {
+ c.uuid[i] = C.uint8_t(v)
+ }
+
+ return c, nil
+}
// defboolVal represents a defbool value.
type defboolVal int
@@ -499,7 +532,7 @@ type Dominfo struct {
func (cdi *C.libxl_dominfo) toGo() (di *Dominfo) {
di = {}
- di.Uuid = Uuid(cdi.uuid)
+ di.Uuid.fromC()
di.Domid = Domid(cdi.domid)
di.Ssidref = uint32(cdi.ssidref)
di.SsidLabel = C.GoString(cdi.ssid_label)
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 06/22] golang/xenlight: define StringList builtin type
From: Nick Rosbrook
Define StringList as []string an implement fromC and toC functions.
Signed-off-by: Nick Rosbrook
---
Changes in v2:
- Define fromC with a pointer receiver since a newly-allocated slice
is being assigned to the StringList.
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 1c5e3c0cc7..72afc3cf14 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -212,6 +212,35 @@ type KeyValueList struct{}
func (kvl KeyValueList) fromC(ckvl *C.libxl_key_value_list) error {
return nil }
func (kvl KeyValueList) toC() (ckvl C.libxl_key_value_list, err error) {
return }
+// 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 v2 08/22] 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
Reviewed-by: George Dunlap
---
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 eb0d309543..108b50124a 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -216,6 +216,29 @@ func (mac Mac) toC() (C.libxl_mac, error) {
return cmac, nil
}
+// MsVmGenid represents a libxl_ms_vm_genid.
+type MsVmGenid [int(C.LIBXL_MS_VM_GENID_LEN)]byte
+
+func (mvg *MsVmGenid) fromC(cmvg *C.libxl_ms_vm_genid) error {
+ b :=
(*[int(C.LIBXL_MS_VM_GENID_LEN)]C.uint8_t)(unsafe.Pointer([0]))
+
+ for i, v := range b {
+ mvg[i] = byte(v)
+ }
+
+ return nil
+}
+
+func (mvg *MsVmGenid) toC() (C.libxl_ms_vm_genid, error) {
+ var cmvg C.libxl_ms_vm_genid
+
+ for i, v := range mvg {
+ cmvg.bytes[i] = C.uint8_t(v)
+ }
+
+ return cmvg, nil
+}
+
type Context struct {
ctx*C.libxl_ctx
logger *C.xentoollog_logger_stdiostream
--
2.19.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 03/22] golang/xenlight: define Devid type as int
From: Nick Rosbrook
Signed-off-by: Nick Rosbrook
Reviewed-by: George Dunlap
---
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 640d82f35f..8ac26e63f0 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] [PATCH v2 05/22] 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
Acked-by: George Dunlap
---
Changes in v2:
- Use consistent variable naming for slice created from
libxl_bitmap.
tools/golang/xenlight/xenlight.go | 94 ---
1 file changed, 48 insertions(+), 46 deletions(-)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index 3edff18471..1c5e3c0cc7 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -212,20 +212,48 @@ type KeyValueList struct{}
func (kvl KeyValueList) fromC(ckvl *C.libxl_key_value_list) error {
return nil }
func (kvl KeyValueList) toC() (ckvl C.libxl_key_value_list, err error) {
return }
-// 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
+ cs := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+ // And copy the C array into the Go array
+ copy(bm.bitmap, cs)
+
+ 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))
+ cs := (*[1 << 31]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
+
+ copy(cs, bm.bitmap)
+
+ return cbm, nil
+}
+
/*
* Types: IDL
*
@@ -426,7 +454,7 @@ func (cci C.libxl_cpupoolinfo) toGo() (gci CpupoolInfo) {
gci.PoolName = C.GoString(cci.pool_name)
gci.Scheduler = Scheduler(cci.sched)
gci.DomainCount = int(cci.n_dom)
- gci.Cpumap = cci.cpumap.toGo()
+ gci.Cpumap.fromC()
return
}
@@ -500,7 +528,10 @@ func (Ctx *Context) CpupoolCreate(Name string, Scheduler
Scheduler, Cpumap Bitma
var uuid C.libxl_uuid
C.libxl_uuid_generate()
- cbm := Cpumap.toC()
+ cbm, err := Cpumap.toC()
+ if err != nil {
+ return
+ }
defer C.libxl_bitmap_dispose()
ret := C.libxl_cpupool_create(Ctx.ctx, name,
C.libxl_scheduler(Scheduler),
@@ -555,7 +586,10 @@ func (Ctx *Context) CpupoolCpuaddCpumap(Poolid uint32,
Cpumap Bitmap) (err error
return
}
- cbm := Cpumap.toC()
+ cbm, err := Cpumap.toC()
+ if err != nil {
+ return
+ }
defer C.libxl_bitmap_dispose()
ret := C.libxl_cpupool_cpuadd_cpumap(Ctx.ctx, C.uint32_t(Poolid), )
@@ -591,7 +625,10 @@ func (Ctx *Context) CpupoolCpuremoveCpumap(Poolid uint32,
Cpumap Bitmap) (err er
return
}
- cbm := Cpumap.toC()
+ cbm, err := Cpumap.toC()
+ if err != nil {
+ return
+ }
defer C.libxl_bitmap_dispose()
ret := C.libxl_cpupool_cpuremove_cpumap(Ctx.ctx, C.uint32_t(Poolid),
)
@@ -714,41 +751,6 @@ func (Ctx *Context) CpupoolMakeFree(Cpumap Bitmap) (err
error) {
* Bitmap operations
*/
-// Return a Go bitmap which is a copy of the referred C bitmap.
-func (cbm C.libxl_bitmap) toGo() (gbm Bitmap) {
- // Alloc a Go slice for the bytes
- size := int(cbm.size)
- gbm.bitmap = make([]C.uint8_t, size)
-
- // Make a slice pointing to the C array
- mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
-
- // And copy the C array into the Go array
- copy(gbm.bitmap, mapslice)
-
- return
-}
-
-// Must be C.libxl_bitmap_dispose'd of afterwards
-func (gbm Bitmap) toC() (cbm C.libxl_bitmap) {
- C.libxl_bitmap_init()
-
- size := len(gbm.bitmap)
- cbm._map = (*C.uint8_t)(C.malloc(C.size_t(size)))
- cbm.size = C.uint32_t(size)
- if cbm._map == nil {
- panic("C.calloc failed!")
- }
-
- // Make a slice pointing to the C array
- mapslice := (*[1 << 30]C.uint8_t)(unsafe.Pointer(cbm._map))[:size:size]
-
[Xen-devel] [PATCH v2 10/22] golang/xenlight: define CpuidPolicyList builtin type
From: Nick Rosbrook
Define CpuidPolicyList as a string so that libxl_cpuid_parse_config can
be used in the toC function.
For now, fromC is a no-op since libxl does not support a way to read a
policy, modify it,and then give it back to libxl.
Signed-off-by: Nick Rosbrook
---
Changes in v2:
- Re-define CpuidPolicyList as string.
- Make fromC a no-op.
- Use libxl_cpuid_parse_config in toC function.
tools/golang/xenlight/xenlight.go | 25 +
1 file changed, 25 insertions(+)
diff --git a/tools/golang/xenlight/xenlight.go
b/tools/golang/xenlight/xenlight.go
index d57f780116..6f0a9278ad 100644
--- a/tools/golang/xenlight/xenlight.go
+++ b/tools/golang/xenlight/xenlight.go
@@ -249,6 +249,31 @@ 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.
+//
+// The value of CpuidPolicyList is honored when used as input to libxl. If
+// a struct contains a field of type CpuidPolicyList, that field will be left
+// empty when it is returned from libxl.
+type CpuidPolicyList string
+
+func (cpl CpuidPolicyList) fromC(ccpl *C.libxl_cpuid_policy_list) error {
return nil }
+
+func (cpl CpuidPolicyList) toC() (C.libxl_cpuid_policy_list, error) {
+ var ccpl C.libxl_cpuid_policy_list
+
+ s := C.CString(string(cpl))
+ defer C.free(unsafe.Pointer(s))
+
+ ret := C.libxl_cpuid_parse_config(, s)
+ if ret != 0 {
+ C.libxl_cpuid_dispose()
+
+ return ccpl, Error(-ret)
+ }
+
+ 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 v2 00/22] 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. The v1 series can be found on my GitHub branch[2]. Changes in v2: - GitHub branch for v2 [3]. - Drop patch 01/24 from v1 since was committed as a bug fix for 4.13. - The Makefile changes in 24/24 from v1 have been moved to the patches where the build changes are introduced. [1] https://lists.xenproject.org/archives/html/xen-devel/2019-07/msg02259.html [2] https://github.com/enr0n/xen/tree/golang-patches-v1 [3] https://github.com/enr0n/xen/tree/golang-patches-v2 Nick Rosbrook (22): golang/xenlight: generate enum types from IDL golang/xenlight: define Defbool builtin type golang/xenlight: define Devid type as int golang/xenlight: define KeyValueList as empty struct 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 tools/golang/xenlight/Makefile | 20 +- tools/golang/xenlight/gengotypes.py | 719 ++ tools/golang/xenlight/helpers.gen.go | 3408 ++ tools/golang/xenlight/types.gen.go | 1224 + tools/golang/xenlight/xenlight.go| 909 +++ 5 files changed, 5744 insertions(+), 536 deletions(-) create mode 100644 tools/golang/xenlight/gengotypes.py create mode 100644 tools/golang/xenlight/helpers.gen.go create mode 100644 tools/golang/xenlight/types.gen.go -- 2.19.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 1/2] libxl: introduce new backend type VINPUT
Oleksandr Grytsov writes ("Re: [PATCH v1 1/2] libxl: introduce new backend type
VINPUT"):
> 1. Move QEMU_BACKEND macro to libxl__device_type structure as function
> and let the device to decide it has QEMU backend:
>
> struct libxl__device_type {
> ...
> device_qemu_backend_fn_t qemu_backend
> }
>
> In this case, introducing new device type for VKBD is not needed. The VKBD
> device will check backend type XS entry to define which backend is running.
Sorry for the delay replying. In your earlier mails I had trouble
figuring out what you meant but this little vignette makes it clear to
me.
I think the problem you are trying to solve is this: in your case
QEMU_BACKEND needs to depend on the visible vkb_backend field, but the
device->backend_kind is set unconditionally to just VKB ?
Could you solve this problem by inventing a new backend_kind, and
writing your own function libxl__device_from_vkb, and putting
*different* values into backend_kind ? I think that is what
backend_kind is for. See for example various console functions and
also libxl__device_from_disk.
> 2. Use string type for VKBD backend_type field instead of enum. It will be
> empty for QEMU and generic for "user space" backends.
This seems worse.
> On Mon, Oct 28, 2019 at 4:06 PM Oleksandr Grytsov wrote:
> > On Wed, Oct 16, 2019 at 4:26 PM Oleksandr Grytsov wrote:
> > > [Ian:]
> > > > [Oleksandr:]
> > > > > [Ian:]
> > > > > > I also don't understand why the "user space" kbd backend seems to be
> > > > > > "linux" in the enum.
> > > > >
> > > > > I agree this is not so good name. But I don't know how to call
> > > > > backends which are not running
> > > > > inside QEMU in general.
> > > >
> > > > I think this would be useable on freebsd ? "linux" definitely seems
> > > > wrong. I see it hasn't been in a release so it is not too late to
> > > > rename it, subject to discussion with Juergen as RM.
...
> > > > Maybe "linux" should be "troops"...
> > >
> > > It doesn't look as generic solution. If some user implements own backend
> > > it should add new entry into backend type enum.
Would you be prepared to change it to *something* else ?
AFAICT from the code it just uses what would the `usual' xenstore pv
control plane path for a device called "vkb" ?
So maybe we could call it "pv" ? Is there a protocol doc I should be
looking at that defines this vkb interface ?
Sorry still to be so confused.
Regards,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen hiding thermal capabilities from Dom0
On Thu, Nov 14, 2019 at 10:05 PM Jan Beulich wrote: > > On 14.11.2019 17:07, Rishi wrote: > > In some of our hosts, Xen is not correctly exposing processor thermal > > capabilities to Dom0. > > Please refer: https://bugzilla.kernel.org/show_bug.cgi?id=205347 > > > > The flag > > /* Thermal and Power Management Leaf, CPUID level 0x0006 (EAX), word 14 > > */ > > X86_FEATURE_DTHERM (14*32+ 0) > > > > is returned 0 via PVOP_VCALL4 vs returns 1 via native_cpuid() call from > > Dom0. > > > > Sample output via PVCALL vs Native call. > > [63291.688755] cpuid_eax 6 : a 0 > > [63291.688759] native_cpuid : a 77 > > > > Is this a bug or needs some special feature to be enabled from Xen command > > line? > > Exposing this to guests (including Dom0) would imply properly > virtualizing the respective behavior. In > xen/arch/x86/cpuid.c:recalculate_misc() we specifically have > > p->basic.raw[0x6] = EMPTY_LEAF; /* Therm/Power not exposed to guests. */ > > If you wanted this exposed, you'd first of all need to come up > with a sane virtualization model of this functionality. Simply > exposing the CPUID bits is not going to be an option. > > Jan Is it due to any security risks? If so, are there any advisories around it? Should it be allowed that Xen is hiding CPU flags (in this case Therm), yet a modification to Dom0 kernel allows them to be brought back? ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 144157: tolerable all pass - PUSHED
flight 144157 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/144157/ 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 b92a286cfb72eacbc988b500f4bb04dbe6bedc0c baseline version: xen f43afb079031d90a7810dce380ad0d224b895ea3 Last test of basis 144155 2019-11-15 13:00:30 Z0 days Testing same since 144157 2019-11-15 16:09:06 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Anthony PERARD Igor Druzhinin Jan Beulich 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 f43afb0790..b92a286cfb b92a286cfb72eacbc988b500f4bb04dbe6bedc0c -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.13 v3] passthrough: simplify locking and logging
From: Paul Durrant
Dropping the pcidevs lock between calling device_assigned() and
assign_device() means that the latter has to do the same check as the
former for no obvious gain. Also, since long running operations under
pcidevs lock already drop the lock and return -ERESTART periodically there
is little point in immediately failing an assignment operation with
-ERESTART just because the pcidevs lock could not be acquired (for the
second time, having already blocked on acquiring the lock in
device_assigned()).
This patch instead acquires the lock once for assignment (or test assign)
operations directly in iommu_do_pci_domctl() and thus can remove the
duplicate domain ownership check in assign_device(). Whilst in the
neighbourhood, the patch also removes some debug logging from
assign_device() and deassign_device() and replaces it with proper error
logging, which allows error logging in iommu_do_pci_domctl() to be
removed.
Signed-off-by: Paul Durrant
Signed-off-by: Igor Druzhinin
---
Since Paul doesn't mind and kindly agreed - I'm taking ownership of this patch
review process from now on.
Changes in v3:
- Dropped controversial hunk with error code processing of device_assigned().
Readability is worse with it and I don't think we can safely stop converting
the error code to avoid userspace breakage.
- Addressed other minor comments.
- Fixed Paul's email again to reflect that the code was made in Citrix.
---
xen/drivers/passthrough/pci.c | 78 ---
1 file changed, 22 insertions(+), 56 deletions(-)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 18a7dc7..8a25d4f 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -932,30 +932,26 @@ static int deassign_device(struct domain *d, uint16_t
seg, uint8_t bus,
break;
ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
-if ( !ret )
-continue;
-
-printk(XENLOG_G_ERR "%pd: deassign %04x:%02x:%02x.%u failed (%d)\n",
- d, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret);
-return ret;
+if ( ret )
+goto out;
}
devfn = pdev->devfn;
ret = hd->platform_ops->reassign_device(d, target, devfn,
pci_to_dev(pdev));
if ( ret )
-{
-dprintk(XENLOG_G_ERR,
-"%pd: deassign device (%04x:%02x:%02x.%u) failed\n",
-d, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
-return ret;
-}
+goto out;
if ( pdev->domain == hardware_domain )
pdev->quarantine = false;
pdev->fault.count = 0;
+out:
+if ( ret )
+printk(XENLOG_G_ERR "%pd: deassign (%04x:%02x:%02x.%u) failed (%d)\n",
+ d, seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn), ret);
+
return ret;
}
@@ -976,10 +972,7 @@ int pci_release_devices(struct domain *d)
{
bus = pdev->bus;
devfn = pdev->devfn;
-if ( deassign_device(d, pdev->seg, bus, devfn) )
-printk("domain %d: deassign device (%04x:%02x:%02x.%u) failed!\n",
- d->domain_id, pdev->seg, bus,
- PCI_SLOT(devfn), PCI_FUNC(devfn));
+deassign_device(d, pdev->seg, bus, devfn);
}
pcidevs_unlock();
@@ -1475,8 +1468,7 @@ static int device_assigned(u16 seg, u8 bus, u8 devfn)
struct pci_dev *pdev;
int rc = 0;
-pcidevs_lock();
-
+ASSERT(pcidevs_locked());
pdev = pci_get_pdev(seg, bus, devfn);
if ( !pdev )
@@ -1490,11 +1482,10 @@ static int device_assigned(u16 seg, u8 bus, u8 devfn)
pdev->domain != dom_io )
rc = -EBUSY;
-pcidevs_unlock();
-
return rc;
}
+/* Caller should hold the pcidevs_lock */
static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
{
const struct domain_iommu *hd = dom_iommu(d);
@@ -1513,23 +1504,11 @@ static int assign_device(struct domain *d, u16 seg, u8
bus, u8 devfn, u32 flag)
p2m_get_hostp2m(d)->global_logdirty) )
return -EXDEV;
-if ( !pcidevs_trylock() )
-return -ERESTART;
-
+/* device_assigned() should already have cleared the device for assignment
*/
+ASSERT(pcidevs_locked());
pdev = pci_get_pdev(seg, bus, devfn);
-
-rc = -ENODEV;
-if ( !pdev )
-goto done;
-
-rc = 0;
-if ( d == pdev->domain )
-goto done;
-
-rc = -EBUSY;
-if ( pdev->domain != hardware_domain &&
- pdev->domain != dom_io )
-goto done;
+ASSERT(pdev && (pdev->domain == hardware_domain ||
+pdev->domain == dom_io));
if ( pdev->msix )
{
@@ -1550,19 +1529,16 @@ static int assign_device(struct domain *d, u16 seg, u8
bus, u8 devfn, u32 flag)
if ( PCI_SLOT(devfn) != PCI_SLOT(pdev->devfn) )
break;
[Xen-devel] [xen-4.12-testing test] 144141: regressions - FAIL
flight 144141 xen-4.12-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/144141/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 144035 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qcow217 guest-localmigrate/x10 fail like 144007 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-xsm 13 migrate-support-checkfail 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-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 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-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-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-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-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-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail 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-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-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 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-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass version targeted for testing: xen 0138da196c8c334589a25144d4d69bf6553e2658 baseline version: xen 278e46ae8f99485915ae662e7905c8333a55048a Last test of basis 144035 2019-11-12 00:36:50 Z3 days Testing same since 144059 2019-11-12 19:10:11 Z2 days4 attempts People who touched revisions under test: Andrew Cooper George Dunlap jobs:
Re: [Xen-devel] [XEN PATCH for-4.13] libxl_pci: Don't hold QMP connection while waiting
Anthony PERARD writes ("[XEN PATCH for-4.13] libxl_pci: Don't hold QMP
connection while waiting"):
> After sending the 'device_del' command for a PCI passthrough device,
> we wait until QEMU has effectively deleted the device, this involves
> executing more QMP commands. While waiting, libxl hold the connection.
I just read the code here. It seems to poll on a timer. How ugly.
> It isn't necessary to hold the connection and it prevents others from
> making progress, so this patch releases the QMP connection.
Right.
> if (rc) goto out;
>
> +libxl__ev_qmp_dispose(gc, qmp);
> +
> asked_id = GCSPRINTF(PCI_PT_QDEV_ID,
It's not it entirely clear to me why you dispose this before the error
exit, but I think it doesn't matter. If it does matter then please
explain :-).
Acked-by: Ian Jackson
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ryzen 3xxx works with Windows
Can add weight to these findings. Tested with Xen 4.12.1 and the cpuid line suggested and it looks like my Windows VM has come up with 4 vCPUS. I can't RDP in to make sure its 100% booted, but it certainly isn't doing the crash dump cycle - and CPU usage is consistent with being successfully booted. Steven Haigh net...@crc.id.au https://www.crc.id.au +613 9001 6090 +614 1293 5897 On Fri, Nov 15, 2019 at 18:06, Andreas Kinzler wrote: Hello All, I compared the CPUID listings from Ryzen 2700X (attached as tar.xz) to 3700X and found only very few differences. I added cpuid = [ "0x8008:ecx=0100" ] to xl.cfg and then Windows runs great with 16 vCPUs. Cinebench R15 score is >2050 which is more or less the bare metal value. Regards Andreas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/9] x86: introduce a new set of APIs to manage Xen page tables
On Fri, Nov 15, 2019 at 04:23:30PM +0100, Jan Beulich wrote:
> On 02.10.2019 19:16, Hongyan Xia wrote:
> > @@ -4847,22 +4848,50 @@ int mmcfg_intercept_write(
> > }
> >
> > void *alloc_xen_pagetable(void)
> > +{
> > +mfn_t mfn;
> > +
> > +mfn = alloc_xen_pagetable_new();
> > +ASSERT(!mfn_eq(mfn, INVALID_MFN));
> > +
> > +return map_xen_pagetable_new(mfn);
> > +}
> > +
> > +void free_xen_pagetable(void *v)
> > +{
> > +if ( system_state != SYS_STATE_early_boot )
> > +free_xen_pagetable_new(virt_to_mfn(v));
> > +}
> > +
> > +mfn_t alloc_xen_pagetable_new(void)
> > {
> > if ( system_state != SYS_STATE_early_boot )
> > {
> > void *ptr = alloc_xenheap_page();
> >
> > BUG_ON(!hardware_domain && !ptr);
> > -return ptr;
> > +return virt_to_mfn(ptr);
> > }
> >
> > -return mfn_to_virt(mfn_x(alloc_boot_pages(1, 1)));
> > +return alloc_boot_pages(1, 1);
> > }
> >
> > -void free_xen_pagetable(void *v)
> > +void *map_xen_pagetable_new(mfn_t mfn)
>
> There's no need for the map/unmap functions to have a _new
> suffix, is there?
>
It is more consistent.
> > {
> > -if ( system_state != SYS_STATE_early_boot )
> > -free_xenheap_page(v);
> > +return mfn_to_virt(mfn_x(mfn));
> > +}
> > +
[...]
>
> > +{
> > +/* XXX still using xenheap page, no need to do anything. */
>
> I wonder if it wouldn't be a good idea to at least have some
> leak detection here temporarily, such that we have a chance of
> noticing paths not properly doing the necessary unmapping.
>
> The again a question is why you introduce such a map/unmap pair
> in the first place. This is going to be a thin wrapper around
> {,un}map_domain_page() in the end anyway, and hence callers
> could as well be switched to calling those function directly,
> as they're no-ops on Xen heap pages.
All roads lead to Rome, but some roads are easier than others. Having a
set of APIs to deal with page tables make the code easier to follow IMO.
And we can potentially do more stuff in this function, for example, make
the unmap function test if the page is really a page table to avoid
misuse; or like you said, have some leak detection check there.
Also, please consider there are dozens of patches that are built on top
of this set of new APIs. Having to rewrite them just for mechanical
changes is not fun for Hongyan. I would suggest we be more pragmatic
here.
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ryzen 3xxx works with Windows
On 11/15/19 5:06 PM, Andreas Kinzler wrote: > Hello All, > > I compared the CPUID listings from Ryzen 2700X (attached as tar.xz) to > 3700X and found only very few differences. I added > > cpuid = [ "0x8008:ecx=0100" ] > > to xl.cfg and then Windows runs great with 16 vCPUs. Cinebench R15 score > is >2050 which is more or less the bare metal value. Awesome. Any idea what those bits do? -George ___ 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 v2 6/6] libxl_qmp: Have a lock for QMP socket access
Anthony PERARD writes ("[XEN PATCH for-4.13 v2 6/6] libxl_qmp: Have a lock for
QMP socket access"):
> Background:
> This happens when attempting to create a guest with multiple
> pci devices passthrough, libxl creates one connection per device to
> attach and execute connect() on all at once before any single
> connection has finished.
>
> To work around this, we use a new lock.
Thanks again for tackling this.
> Error handling of connect() and lock() is a bit awkward as
> libxl__ev_qmp_send() doesn't allow to call the callback synchronously.
> So we setup a timer to have a callback that has been called
> asynchronously. We use the _abs variant it does strictly less than
> _rel, thus avoiding unnecessary code that could return an error
> (unnecessary because we only need to have the callback been called
> ASAP).
I have some problems with the approach here, I'm afraid.
> This patch workaround the fact that it's not possible to connect
> multiple time to a single QMP socket. QEMU listen on the socket with
> a backlog value of 1, which mean that on Linux when concurrent thread
> call connect() on the socket, they get EAGAIN.
...
> * qmp_state External cfdefd id rx_buf* tx_buf* msg*
> * disconnected Idle NULL Idlereset freefreefree
> + * waiting_lock Active open Idlereset usedfreeset
> * connecting Active open IN reset usedfreeset
> * cap.negActive open IN|OUT sent usedcap_neg set
> * cap.negActive open IN sent usedfreeset
Don't `lock' and `time' need to be added to this table ?
The table may become rather wide :-/. Maybe it could be
compressed/abbreviated some more or maybe we'll just live with it
becoming wider.
> out:
> -return rc;
> +/* An error occurred and we need to let the caller know. At this
> + * point, we can only do so via the callback. Unfortunately, the
> + * callback of libxl__ev_slowlock_lock() might be called synchronously,
> + * but libxl__ev_qmp_send() promise that it will not call the callback
> + * synchronously. So we have to arrange to call the callback
> + * asynchronously. */
> +ev->rc = rc;
> +struct timeval now = { 0 };
> +rc = libxl__ev_time_register_abs(ev->ao, >etime,
> + lock_error_callback, now);
> +/* If setting up the timer failed, there is no way to tell the caller
> + * of libxl__ev_qmp_send() that the connection to the QMP socket
> + * failed. But they are supposed to have a timer of their own. */
> +if (rc)
> +LOGD(ERROR, ev->domid,
> + "Failed to setup a callback call. rc=%d", rc);
I don't think this is right. I think this callback has to be set up
in a way that can't fail. But I think this is possible. We're free
to allocate memory (although this may not be needed) and the egc
(which we have) can contain callback pointers.
I can see two approaches:
1. Invent a new libxl_ev_immediate_but_not_reentrant_callback (can't
think of a good name right now). Add a new list head to the egc,
and when you want to register a callback, put it on that list.
Add the callback execution to egc_run_callbacks, probably at the
top in a loop (since immediate callbacks may add more immediate
callbacks).
2. Use libxl__ev_time; provide libxl__ev_time_register_now which
cannot fail: it sets the abs time to 0 and skips the abortable
stuff, and puts the libxl__ev_time on the front of the CTX etimes
list. Have egc_run_callbacks run immediate timeouts: ie, if the
head of the etimes list has abs time of 0, strip it, and call it.
I think this involves some special handling of this case because
you want to avoid messing about with the OSEVENT_HOOKs, so
you probably need a new bit in libxl__ev_time. Otherwise
time_deregister wouldn't know what to do.
I think 1 is probably less confusing and less risky. It doesn't
disturb, or get entangled in, the existing ev_time code; it doesn't
involve pointlessly allocating an unused abortable. And it doesn't
involve confusion over which error returns are possible where and
which rc values must be kept and which discarded.
Sorry,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Ryzen 3xxx works with Windows
Hello All, I compared the CPUID listings from Ryzen 2700X (attached as tar.xz) to 3700X and found only very few differences. I added cpuid = [ "0x8008:ecx=0100" ] to xl.cfg and then Windows runs great with 16 vCPUs. Cinebench R15 score is >2050 which is more or less the bare metal value. Regards Andreas cpuid-2700X.tar.xz Description: Binary data ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 144144: tolerable all pass - PUSHED
flight 144144 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/144144/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 143023 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 143023 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check 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-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 14 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 95f5ac9ae52455e9da47afc95fa31c9456ac27ae baseline version: libvirt 2cff65e4c60ed7b3c0c6a97d526d1f8d52c0e919 Last test of basis 143023 2019-10-22 04:19:26 Z 24 days Failing since143051 2019-10-23 04:18:57 Z 23 days 20 attempts Testing same since 144144 2019-11-15 04:18:42 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Andrew Jones Bjoern Walk Daniel Henrique Barboza Daniel P. Berrangé Daniel Veillard Eric Blake Jidong Xia Jim Fehlig Jiri Denemark John Ferlan Jonathon Jongsma Julio Faracco Ján Tomko Laine Stump Laine Stump Mao Zhongyi Maya Rashish Michal Privoznik Pavel Hrdina Peter Krempa Pino Toscano Wang Yechao Yi Li Zhang Shengju 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 pass 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
Re: [Xen-devel] [XEN PATCH for-4.13] configure: Fix test for python 3.8
On Fri, Nov 15, 2019 at 04:15:32PM +, Anthony PERARD wrote: > https://docs.python.org/3.8/whatsnew/3.8.html#debug-build-uses-the-same-abi-as-release-build > > > To embed Python into an application, a new --embed option must be > > passed to python3-config --libs --embed to get -lpython3.8 (link the > > application to libpython). To support both 3.8 and older, try > > python3-config --libs --embed first and fallback to python3-config > > --libs (without --embed) if the previous command fails. > > Signed-off-by: Anthony PERARD > --- > > Notes: > You may want to rerun ./autogen.sh on commit. Indeed. This patch introduces a lot of unrelated changes, presumably due to the difference in autoconf. > diff --git a/m4/python_devel.m4 b/m4/python_devel.m4 > index e365cd658e0e..bbf1e0354b2b 100644 > --- a/m4/python_devel.m4 > +++ b/m4/python_devel.m4 > @@ -23,8 +23,15 @@ AS_IF([test x"$pyconfig" = x"no"], [ > ], [ > dnl If python-config is found use it > CPPFLAGS="$CFLAGS `$PYTHON-config --cflags`" > -LDFLAGS="$LDFLAGS `$PYTHON-config --ldflags`" > -LIBS="$LIBS `$PYTHON-config --libs`" > +dnl We need to use --embed with python 3.8 but not with earlier version > so > +dnl check if it is recognized. > +python_devel_embed="" > +if $PYTHON-config --embed >/dev/null 2>/dev/null; then > + python_devel_embed="--embed" > +fi > +LDFLAGS="$LDFLAGS `$PYTHON-config --ldflags $python_devel_embed`" > +LIBS="$LIBS `$PYTHON-config --libs $python_devel_embed`" > +unset python_devel_embed > ]) Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [XEN PATCH for-4.13] configure: Fix test for python 3.8
https://docs.python.org/3.8/whatsnew/3.8.html#debug-build-uses-the-same-abi-as-release-build
> To embed Python into an application, a new --embed option must be
> passed to python3-config --libs --embed to get -lpython3.8 (link the
> application to libpython). To support both 3.8 and older, try
> python3-config --libs --embed first and fallback to python3-config
> --libs (without --embed) if the previous command fails.
Signed-off-by: Anthony PERARD
---
Notes:
You may want to rerun ./autogen.sh on commit.
configure | 14 +-
docs/configure | 14 +-
m4/python_devel.m4 | 11 +--
stubdom/configure | 14 +-
tools/configure| 33 +
5 files changed, 25 insertions(+), 61 deletions(-)
diff --git a/configure b/configure
index 4d258eb61534..6ddcf52d9c1b 100755
--- a/configure
+++ b/configure
@@ -644,7 +644,6 @@ infodir
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -723,7 +722,6 @@ datadir='${datarootdir}'
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -976,15 +974,6 @@ do
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
- -runstatedir | --runstatedir | --runstatedi | --runstated \
- | --runstate | --runstat | --runsta | --runst | --runs \
- | --run | --ru | --r)
-ac_prev=runstatedir ;;
- -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
- | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
- | --run=* | --ru=* | --r=*)
-runstatedir=$ac_optarg ;;
-
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1122,7 +,7 @@ fi
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
- libdir localedir mandir runstatedir
+ libdir localedir mandir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
@@ -1275,7 +1264,6 @@ Fine tuning of the installation directories:
--sysconfdir=DIRread-only single-machine data [PREFIX/etc]
--sharedstatedir=DIRmodifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
- --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIRobject code libraries [EPREFIX/lib]
--includedir=DIRC header files [PREFIX/include]
--oldincludedir=DIR C header files for non-gcc [/usr/include]
diff --git a/docs/configure b/docs/configure
index e81644752f43..ca5e28264634 100755
--- a/docs/configure
+++ b/docs/configure
@@ -634,7 +634,6 @@ infodir
docdir
oldincludedir
includedir
-runstatedir
localstatedir
sharedstatedir
sysconfdir
@@ -711,7 +710,6 @@ datadir='${datarootdir}'
sysconfdir='${prefix}/etc'
sharedstatedir='${prefix}/com'
localstatedir='${prefix}/var'
-runstatedir='${localstatedir}/run'
includedir='${prefix}/include'
oldincludedir='/usr/include'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
@@ -964,15 +962,6 @@ do
| -silent | --silent | --silen | --sile | --sil)
silent=yes ;;
- -runstatedir | --runstatedir | --runstatedi | --runstated \
- | --runstate | --runstat | --runsta | --runst | --runs \
- | --run | --ru | --r)
-ac_prev=runstatedir ;;
- -runstatedir=* | --runstatedir=* | --runstatedi=* | --runstated=* \
- | --runstate=* | --runstat=* | --runsta=* | --runst=* | --runs=* \
- | --run=* | --ru=* | --r=*)
-runstatedir=$ac_optarg ;;
-
-sbindir | --sbindir | --sbindi | --sbind | --sbin | --sbi | --sb)
ac_prev=sbindir ;;
-sbindir=* | --sbindir=* | --sbindi=* | --sbind=* | --sbin=* \
@@ -1110,7 +1099,7 @@ fi
for ac_var in exec_prefix prefix bindir sbindir libexecdir datarootdir \
datadir sysconfdir sharedstatedir localstatedir includedir \
oldincludedir docdir infodir htmldir dvidir pdfdir psdir \
- libdir localedir mandir runstatedir
+ libdir localedir mandir
do
eval ac_val=\$$ac_var
# Remove trailing slashes.
@@ -1263,7 +1252,6 @@ Fine tuning of the installation directories:
--sysconfdir=DIRread-only single-machine data [PREFIX/etc]
--sharedstatedir=DIRmodifiable architecture-independent data [PREFIX/com]
--localstatedir=DIR modifiable single-machine data [PREFIX/var]
- --runstatedir=DIR modifiable per-process data [LOCALSTATEDIR/run]
--libdir=DIRobject code libraries [EPREFIX/lib]
--includedir=DIRC header files [PREFIX/include]
--oldincludedir=DIR C header files for
Re: [Xen-devel] [XEN PATCH for-4.13 v2 4/6] libxl: Introduce libxl__ev_slowlock_dispose
Anthony PERARD writes ("[XEN PATCH for-4.13 v2 4/6] libxl: Introduce
libxl__ev_slowlock_dispose"):
> Which allow to cancel the lock operation while it is in Active state.
>
> Signed-off-by: Anthony PERARD
Acked-by: Ian Jackson
___
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 v2 3/6] libxl: Rename ev_devlock to ev_slowlock
Anthony PERARD writes ("[XEN PATCH for-4.13 v2 3/6] libxl: Rename ev_devlock to
ev_slowlock"):
> We are going to introduce a different lock based on the same
> implementation as the ev_devlock but with a different path. The
> different slowlock will be differentiated by calling different _init()
> functions.
>
> So we rename libxl__ev_devlock to lib__ev_slowlock, but keep
> libxl__ev_devlock_init().
>
> Some log messages produced ev_slowlock are changed to print the
> name of the lock file (userdata_userid).
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type
> On the whole I think sending v2 earlier is better, since I'll have the > discussions more recently in my head, and so will (hopefully) be able to > get an Ack or R-b more quickly. > > When the development window is open, stuff can be checked in as it's > reviewed, making the whole thing easier. > > To be clear, this is for times when the review of the whole series is > taking longer than a few days. If I review 3 patches of a 6-patch > series one day, probably better to give me a chance to finish the next > day before sending vN+1. :-) But if I stall for a few days, go ahead > and resend. Okay thanks for the clarification. I'll plan on sending v2 once I make these changes to CpuidPolicyList. -NR ___ 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 v2 1/6] libxl: Introduce libxl__ev_child_kill_deregister
Anthony PERARD writes ("[XEN PATCH for-4.13 v2 1/6] libxl: Introduce
libxl__ev_child_kill_deregister"):
> Allow to deregister the callback associated with a child death event.
>
> The death isn't immediate will need to be collected later, so the
> ev_child machinery register its own callback.
>
> libxl__ev_child_kill_deregister() might be called by an AO operation
> that is finishing/cleaning up without a chance for libxl to be
> notified of the child death (via SIGCHLD). So it is possible that the
> application calls libxl_ctx_free() while there are still child around.
> To avoid the application getting unexpected SIGCHLD, the libxl__ao
> responsible for killing a child will have to wait until it has been
> properly reaped.
Very good.
Acked-by: Ian Jackson
Thanks,
Ian.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [XEN PATCH for-4.13 v2 2/6] libxl: Move libxl__ev_devlock declaration
Anthony PERARD writes ("[XEN PATCH for-4.13 v2 2/6] libxl: Move
libxl__ev_devlock declaration"):
> We are going to want to include libxl__ev_devlock into libxl__ev_qmp.
>
> No functional changes.
Acked-by: Ian Jackson
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type
On 11/15/19 3:51 PM, Nick Rosbrook wrote: >> Yes, let's do that. > > Okay, will do. > > As a point of clarification, should I be waiting until you've reviewed > all patches in v1 before I send v2 of this series? Or do you prefer > that I send a v2 that addresses your review so far? On the whole I think sending v2 earlier is better, since I'll have the discussions more recently in my head, and so will (hopefully) be able to get an Ack or R-b more quickly. When the development window is open, stuff can be checked in as it's reviewed, making the whole thing easier. To be clear, this is for times when the review of the whole series is taking longer than a few days. If I review 3 patches of a 6-patch series one day, probably better to give me a chance to finish the next day before sending vN+1. :-) But if I stall for a few days, go ahead and resend. Thanks, -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type
> Yes, let's do that. Okay, will do. As a point of clarification, should I be waiting until you've reviewed all patches in v1 before I send v2 of this series? Or do you prefer that I send a v2 that addresses your review so far? Thanks, -NR ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type
On 11/15/19 3:26 PM, Nick Rosbrook wrote: >> If we do have to keep the C pointer around for some reason, I think >> using SetFinalizer is a necessary backstop to keep the library from >> leaking. It's all well and good to say, "Make sure you call Dispose()", >> but I think for a GC'd language that's just going to be too easy to >> forget; and it will be a huge pain for long-running processes. > > I understand your motivation for wanting to make this fool-proof, but > there are plenty of common examples in Go where it's well-understood > that if I call `NewFoo` then I need to `foo.Close()` (defer'd or > otherwise). I don't think that alone is a good enough argument for > turning to SetFinalizer. But, I'm certainly not advocating for the > Dispose option either - as I said I think that would be unfortunate > from an API perspective. > >> If we didn't have this type as a type, we'd have to avoid somehow >> exposing the user to the functions which take and use it. The main >> place it's used ATM is in DomainBuildInfo. We could explore whether it >> would be practical to "implement" CpuidPolicyList as a string, and then >> have toC() call libxl_cpuid_parse_config(). Obviously that means >> fromC() would fail; but I'm not sure DomainBuildInfo is really a >> structure passed "out" of libxl anyway. > > It's sounding more and more like we need a way to give types an > "exported/unexported" attribute in the IDL. > > Why exactly would fromC be doomed to fail? Just because there is no > `libxl_cpuid_to_string` or otherwise? Sorry, I was typing this in a bit of a rush at the end of the day yesterday. :-) Yes, that's what I meant: There's basically no way to read a policy from libxl and then pass it back to libxl (since there's no way to convert libxl_cpuid_policy_list => CpuidPolicyList => libxl_cpuid_policy_list again). But at the moment, a string is the "preferred form of modification" as it were; so if such a read-modify-write feature were implemented, libxl would need to add libxl_cpuid_to_string anyway. (Or give some other modifiable form.) > In any case, I think defining it > as a string may be a good intermediate option for now (even if it > means fromC has to be a no-op). That way we can ensure calls to > `libxl_cpuid_dipose` as usual. Yes, let's do that. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 11/15/19 3:34 PM, Jan Beulich wrote: > On 15.11.2019 16:30, George Dunlap wrote: >> On 11/15/19 3:27 PM, Jan Beulich wrote: >>> On 15.11.2019 16:05, George Dunlap wrote: FTR, please avoid top-posting. :-) On 11/15/19 2:31 PM, Steven Haigh wrote: > Just regarding the use of a system environment variable to turn this > feature / bugfix / hack on and off - this would probably break starting > the VM via the xendomains script. > > If the VM definition is in /etc/xen/auto/, then there would be nothing > to set the environment variable before the VM is launched - hence it > would not be applied and a guest crash would occur... > > Depending on the VM's settings, this would either continue to start & > crash - or just stop again until it could be started with the ENV > variable. Right. So a couple of options: 1. Users of xendomains could set the environment variable in their xendomains script 2. We could add a xl.cfg option. Unknown xl.cfg entries are ignored (for better or for worse); in the future, when the "fake ht" thing is replaced, we can either continue ignoring it, or give a useful error message saying how it should be changed. 2a. We could have the config option *replace* the environment variable; in which case we'd leave libvirt users high and dry 2b. We could have the config option cause xl to *set* the environment variable, which should continue to allow other toolstacks (even those not using libxl) to potentially work around the issue. >>> >>> And how would any of these allow to deal with heterogeneous sets of >>> guests? >> >> Are you perhaps confusing 'xl.cfg' (which is the per-domain >> configuration file) with 'xl.conf' (which is the system-wide >> configuration file)? > > Oh, indeed I was. I'm not used to any suffixes on domain config > files. I'm sorry. FYI I'm using the names of the respective man pages: `man xl.cfg` gives you the man page for the per-domain config, `man xl.conf` gives you the global config. It's far from obvious, but at least it's something. :-) -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 15/11/2019 15:23, George Dunlap wrote: > On 11/15/19 2:59 PM, Andrew Cooper wrote: >> On 15/11/2019 14:55, George Dunlap wrote: > +p->basic.htt = false; I think everything below here indeed simply undoes what said old commit did, but I can't match up this one. And together with the question of whether instead leaving it alone, cmp_legacy then would have the same question raised. >>> This is based on a XenServer patch which reverts the entire commit, and >>> has been maintained in the patchqueue since the commit made it upstream, >>> AFAICT. So I'll let someone from that team comment on the wherefores; >>> but as I said, it's by far the best tested option we're going to get. >> Yes. It is a revert of c/s ca2eee92df44 (Xen 3.4, and maintained >> forwards until now) because it broke migration across that changeset. >> >> It is also this exact version of the revert which has been tested and >> confirmed to fix the Ryzen 3xxx fixes. >> >> A separate experiment tried playing with only the flags, via >> cpuid="host:htt=0,cmp_legacy=1" and this did not resolve the crashes. > Is that because the "revert" still clears cmp_legacy, rather than > setting it to 1? > > I think what Jan was getting at was that ca2eee92df44 *sets* htt and > *clears* cmp_legacy, but previous to that commit, htt and cmp_legacy > weren't changed, they were simply left alone. When reverting this > patch, why do we not simply leave it alone, as was done before, rather > than actively clearing them? You also need to account for the accumulated bugfixes of the code since ca2eee92df44. > I think it's a good question to ask, but unless there is a known issue > with what the patch does, I think it's far less risky just to take the > version which has been tested. In short, yes I believe the behaviour is deliberate, although I don't have the bug tickets to hand to remember exactly what went wrong. The only other possibility (and perhaps is better, now that it is possible) is to foward those two bits from the host policy. They are set in the guest policy due to (still) not having a split between default and max (another issue in the queue to be fixed). ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 144155: tolerable all pass - PUSHED
flight 144155 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/144155/ 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 f43afb079031d90a7810dce380ad0d224b895ea3 baseline version: xen 7b4c3d0443b59a0c9d74b5a97467a8d3236f Last test of basis 144150 2019-11-15 10:00:35 Z0 days Testing same since 144155 2019-11-15 13:00:30 Z0 days1 attempts People who touched revisions under test: Juergen Gross Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 7b4c3d0443..f43afb0790 f43afb079031d90a7810dce380ad0d224b895ea3 -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 15.11.2019 16:30, George Dunlap wrote: > On 11/15/19 3:27 PM, Jan Beulich wrote: >> On 15.11.2019 16:05, George Dunlap wrote: >>> FTR, please avoid top-posting. :-) >>> >>> On 11/15/19 2:31 PM, Steven Haigh wrote: Just regarding the use of a system environment variable to turn this feature / bugfix / hack on and off - this would probably break starting the VM via the xendomains script. If the VM definition is in /etc/xen/auto/, then there would be nothing to set the environment variable before the VM is launched - hence it would not be applied and a guest crash would occur... Depending on the VM's settings, this would either continue to start & crash - or just stop again until it could be started with the ENV variable. >>> >>> Right. So a couple of options: >>> >>> 1. Users of xendomains could set the environment variable in their >>> xendomains script >>> >>> 2. We could add a xl.cfg option. Unknown xl.cfg entries are ignored >>> (for better or for worse); in the future, when the "fake ht" thing is >>> replaced, we can either continue ignoring it, or give a useful error >>> message saying how it should be changed. >>> >>> 2a. We could have the config option *replace* the environment variable; >>> in which case we'd leave libvirt users high and dry >>> >>> 2b. We could have the config option cause xl to *set* the environment >>> variable, which should continue to allow other toolstacks (even those >>> not using libxl) to potentially work around the issue. >> >> And how would any of these allow to deal with heterogeneous sets of >> guests? > > Are you perhaps confusing 'xl.cfg' (which is the per-domain > configuration file) with 'xl.conf' (which is the system-wide > configuration file)? Oh, indeed I was. I'm not used to any suffixes on domain config files. I'm sorry. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [TESTDAY] Test report
On 15.11.19 16:19, Tamas K Lengyel wrote: On Fri, Nov 15, 2019 at 4:56 AM Andrew Cooper wrote: On 14/11/2019 22:36, Tamas K Lengyel wrote: On Thu, Nov 14, 2019 at 11:39 AM Andrew Cooper wrote: On 14/11/2019 18:34, Tamas K Lengyel wrote: * Comments: All works, altp2m+introspection requires the ept=pml=0 boot flag specified to workaround a deadlock in Xen Is this separate from the general problem with EPT A/D and write-protecting pagetables? It sounds like it is, it happens without write-protecting in-guest pagetables. I didn't have time to investigate where the deadlock happens and since the workaround is fine for the usecase it wasn't a priority to figure out. Thinking about it, PML will do the wrong thing (deadlocks aside) as soon as any altp2m gfn translations are used. I'd be tempted to work around the deadlock by disabling pml the moment altp2m is touched. That would give a sightly less bad user experience, and should be easy to sort for 4.13. Thoughts, (inc. Juergen as RM) ? That sounds like a good idea to me, that way you can keep pml for guests where it doesn't cause an issue instead of disabling it system wide. Sounds like decent way to handle it. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 15.11.2019 16:23, George Dunlap wrote: > On 11/15/19 2:59 PM, Andrew Cooper wrote: >> On 15/11/2019 14:55, George Dunlap wrote: > +p->basic.htt = false; I think everything below here indeed simply undoes what said old commit did, but I can't match up this one. And together with the question of whether instead leaving it alone, cmp_legacy then would have the same question raised. >>> This is based on a XenServer patch which reverts the entire commit, and >>> has been maintained in the patchqueue since the commit made it upstream, >>> AFAICT. So I'll let someone from that team comment on the wherefores; >>> but as I said, it's by far the best tested option we're going to get. >> >> Yes. It is a revert of c/s ca2eee92df44 (Xen 3.4, and maintained >> forwards until now) because it broke migration across that changeset. >> >> It is also this exact version of the revert which has been tested and >> confirmed to fix the Ryzen 3xxx fixes. >> >> A separate experiment tried playing with only the flags, via >> cpuid="host:htt=0,cmp_legacy=1" and this did not resolve the crashes. > > Is that because the "revert" still clears cmp_legacy, rather than > setting it to 1? > > I think what Jan was getting at was that ca2eee92df44 *sets* htt and > *clears* cmp_legacy, but previous to that commit, htt and cmp_legacy > weren't changed, they were simply left alone. When reverting this > patch, why do we not simply leave it alone, as was done before, rather > than actively clearing them? Actually no, I wasn't looking properly - HTT used to be cleared as much as CMP_LEGACY before that change. Somehow I didn't spot the former when putting together my earlier reply (maybe I looked for HTT when its only HT there). So I'm sorry for the extra noise. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 11/15/19 3:27 PM, Jan Beulich wrote: > On 15.11.2019 16:05, George Dunlap wrote: >> FTR, please avoid top-posting. :-) >> >> On 11/15/19 2:31 PM, Steven Haigh wrote: >>> Just regarding the use of a system environment variable to turn this >>> feature / bugfix / hack on and off - this would probably break starting >>> the VM via the xendomains script. >>> >>> If the VM definition is in /etc/xen/auto/, then there would be nothing >>> to set the environment variable before the VM is launched - hence it >>> would not be applied and a guest crash would occur... >>> >>> Depending on the VM's settings, this would either continue to start & >>> crash - or just stop again until it could be started with the ENV variable. >> >> Right. So a couple of options: >> >> 1. Users of xendomains could set the environment variable in their >> xendomains script >> >> 2. We could add a xl.cfg option. Unknown xl.cfg entries are ignored >> (for better or for worse); in the future, when the "fake ht" thing is >> replaced, we can either continue ignoring it, or give a useful error >> message saying how it should be changed. >> >> 2a. We could have the config option *replace* the environment variable; >> in which case we'd leave libvirt users high and dry >> >> 2b. We could have the config option cause xl to *set* the environment >> variable, which should continue to allow other toolstacks (even those >> not using libxl) to potentially work around the issue. > > And how would any of these allow to deal with heterogeneous sets of > guests? Are you perhaps confusing 'xl.cfg' (which is the per-domain configuration file) with 'xl.conf' (which is the system-wide configuration file)? #1 would obviously require arranging for *all* xendomain-enabled guests to be started with the config enabled. #2 would allow heterogeneous guests if the admin went through and added the workaround to the guests she wanted. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 15.11.2019 16:05, George Dunlap wrote: > FTR, please avoid top-posting. :-) > > On 11/15/19 2:31 PM, Steven Haigh wrote: >> Just regarding the use of a system environment variable to turn this >> feature / bugfix / hack on and off - this would probably break starting >> the VM via the xendomains script. >> >> If the VM definition is in /etc/xen/auto/, then there would be nothing >> to set the environment variable before the VM is launched - hence it >> would not be applied and a guest crash would occur... >> >> Depending on the VM's settings, this would either continue to start & >> crash - or just stop again until it could be started with the ENV variable. > > Right. So a couple of options: > > 1. Users of xendomains could set the environment variable in their > xendomains script > > 2. We could add a xl.cfg option. Unknown xl.cfg entries are ignored > (for better or for worse); in the future, when the "fake ht" thing is > replaced, we can either continue ignoring it, or give a useful error > message saying how it should be changed. > > 2a. We could have the config option *replace* the environment variable; > in which case we'd leave libvirt users high and dry > > 2b. We could have the config option cause xl to *set* the environment > variable, which should continue to allow other toolstacks (even those > not using libxl) to potentially work around the issue. And how would any of these allow to deal with heterogeneous sets of guests? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 11/24] golang/xenlight: define CpuidPolicyList builtin type
> If we do have to keep the C pointer around for some reason, I think > using SetFinalizer is a necessary backstop to keep the library from > leaking. It's all well and good to say, "Make sure you call Dispose()", > but I think for a GC'd language that's just going to be too easy to > forget; and it will be a huge pain for long-running processes. I understand your motivation for wanting to make this fool-proof, but there are plenty of common examples in Go where it's well-understood that if I call `NewFoo` then I need to `foo.Close()` (defer'd or otherwise). I don't think that alone is a good enough argument for turning to SetFinalizer. But, I'm certainly not advocating for the Dispose option either - as I said I think that would be unfortunate from an API perspective. > If we didn't have this type as a type, we'd have to avoid somehow > exposing the user to the functions which take and use it. The main > place it's used ATM is in DomainBuildInfo. We could explore whether it > would be practical to "implement" CpuidPolicyList as a string, and then > have toC() call libxl_cpuid_parse_config(). Obviously that means > fromC() would fail; but I'm not sure DomainBuildInfo is really a > structure passed "out" of libxl anyway. It's sounding more and more like we need a way to give types an "exported/unexported" attribute in the IDL. Why exactly would fromC be doomed to fail? Just because there is no `libxl_cpuid_to_string` or otherwise? In any case, I think defining it as a string may be a good intermediate option for now (even if it means fromC has to be a no-op). That way we can ensure calls to `libxl_cpuid_dipose` as usual. -NR ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/vcpu: Sanitise VCPUOP_initialise call hierachy
On Fri, 15 Nov 2019, 18:13 Andrew Cooper, wrote: > On 31/10/2019 21:25, Julien Grall wrote: > > Hi, > > > > On 31/10/2019 19:28, Andrew Cooper wrote: > >> This code is especially tangled. VCPUOP_initialise calls into > >> arch_initialise_vcpu() which calls back into default_initialise_vcpu() > which > >> is common code. > >> > >> This path is actually dead code on ARM, because VCPUOP_initialise is > filtered > >> out by do_arm_vcpu_op(). > >> > >> The only valid way to start a secondary CPU on ARM is via the PSCI > interface. > >> The same could in principle be said about INIT-SIPI-SIPI for x86 HVM, > if HVM > >> guests hadn't already interited a paravirt way of starting CPUs. > >> > >> Either way, it is quite likely that no future architectures implemented > in Xen > >> are going to want to use a PV interface, as some standardised (v)CPU > bringup > >> mechanism will already exist. > > I am not sure I agree here. Looking at Linux RISCv code (see [1] and > > [2]), it looks like the kernel has to deal with selecting one "lucky" > > CPU/hart to deal with the boot and park all the others. > > > > So it looks like to me there are nothing at the moment on RISCv to do > > (v)CPU bring-up. We might be able to use PSCI (although this is an ARM > > specific way), but would rather wait and see what RISCv folks come up > > with before deciding PV is never going to be used. > > Nothing here prohibits other architectures from using a PV interface if > they wish. > Well, your commit message and the code movement implies that nobody will ever use it. > However, your examples prove my point. There is an already-agreed way > to start RISCv CPUs which is not a PV interface, and therefore is very > unlikely to adopted to run differently under Xen. I would not call that a way to start CPUs because AFAICT all CPUs have to be brought up together and you can't offline them. This is fairly restrictive for a guest so I don't think reusing it would sustainable long term. FWIW, this is exactly what Arm used to have before PSCI. Cheers, ~Andrew > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 11/15/19 2:59 PM, Andrew Cooper wrote: > On 15/11/2019 14:55, George Dunlap wrote: +p->basic.htt = false; >>> I think everything below here indeed simply undoes what said old >>> commit did, but I can't match up this one. And together with the >>> question of whether instead leaving it alone, cmp_legacy then >>> would have the same question raised. >> This is based on a XenServer patch which reverts the entire commit, and >> has been maintained in the patchqueue since the commit made it upstream, >> AFAICT. So I'll let someone from that team comment on the wherefores; >> but as I said, it's by far the best tested option we're going to get. > > Yes. It is a revert of c/s ca2eee92df44 (Xen 3.4, and maintained > forwards until now) because it broke migration across that changeset. > > It is also this exact version of the revert which has been tested and > confirmed to fix the Ryzen 3xxx fixes. > > A separate experiment tried playing with only the flags, via > cpuid="host:htt=0,cmp_legacy=1" and this did not resolve the crashes. Is that because the "revert" still clears cmp_legacy, rather than setting it to 1? I think what Jan was getting at was that ca2eee92df44 *sets* htt and *clears* cmp_legacy, but previous to that commit, htt and cmp_legacy weren't changed, they were simply left alone. When reverting this patch, why do we not simply leave it alone, as was done before, rather than actively clearing them? I think it's a good question to ask, but unless there is a known issue with what the patch does, I think it's far less risky just to take the version which has been tested. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/9] x86: introduce a new set of APIs to manage Xen page tables
On 02.10.2019 19:16, Hongyan Xia wrote:
> @@ -4847,22 +4848,50 @@ int mmcfg_intercept_write(
> }
>
> void *alloc_xen_pagetable(void)
> +{
> +mfn_t mfn;
> +
> +mfn = alloc_xen_pagetable_new();
> +ASSERT(!mfn_eq(mfn, INVALID_MFN));
> +
> +return map_xen_pagetable_new(mfn);
> +}
> +
> +void free_xen_pagetable(void *v)
> +{
> +if ( system_state != SYS_STATE_early_boot )
> +free_xen_pagetable_new(virt_to_mfn(v));
> +}
> +
> +mfn_t alloc_xen_pagetable_new(void)
> {
> if ( system_state != SYS_STATE_early_boot )
> {
> void *ptr = alloc_xenheap_page();
>
> BUG_ON(!hardware_domain && !ptr);
> -return ptr;
> +return virt_to_mfn(ptr);
> }
>
> -return mfn_to_virt(mfn_x(alloc_boot_pages(1, 1)));
> +return alloc_boot_pages(1, 1);
> }
>
> -void free_xen_pagetable(void *v)
> +void *map_xen_pagetable_new(mfn_t mfn)
There's no need for the map/unmap functions to have a _new
suffix, is there?
> {
> -if ( system_state != SYS_STATE_early_boot )
> -free_xenheap_page(v);
> +return mfn_to_virt(mfn_x(mfn));
> +}
> +
> +/* v can point to an entry within a table or be NULL */
> +void unmap_xen_pagetable_new(void *v)
Can this please take const void *, such that callers needing
mappings just for read purposes can have their pointer const-
qualified as well?
> +{
> +/* XXX still using xenheap page, no need to do anything. */
I wonder if it wouldn't be a good idea to at least have some
leak detection here temporarily, such that we have a chance of
noticing paths not properly doing the necessary unmapping.
The again a question is why you introduce such a map/unmap pair
in the first place. This is going to be a thin wrapper around
{,un}map_domain_page() in the end anyway, and hence callers
could as well be switched to calling those function directly,
as they're no-ops on Xen heap pages.
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -633,6 +633,17 @@ int arch_acquire_resource(struct domain *d, unsigned int
> type,
> /* Allocator functions for Xen pagetables. */
> void *alloc_xen_pagetable(void);
> void free_xen_pagetable(void *v);
> +mfn_t alloc_xen_pagetable_new(void);
> +void *map_xen_pagetable_new(mfn_t mfn);
> +void unmap_xen_pagetable_new(void *v);
> +void free_xen_pagetable_new(mfn_t mfn);
> +
> +#define UNMAP_XEN_PAGETABLE_NEW(ptr)\
> +do {\
> +unmap_xen_pagetable_new((ptr)); \
Stray double parentheses.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [TESTDAY] Test report
On Fri, Nov 15, 2019 at 4:56 AM Andrew Cooper wrote: > > On 14/11/2019 22:36, Tamas K Lengyel wrote: > > On Thu, Nov 14, 2019 at 11:39 AM Andrew Cooper > > wrote: > >> On 14/11/2019 18:34, Tamas K Lengyel wrote: > >>> * Comments: All works, altp2m+introspection requires the ept=pml=0 > >>> boot flag specified to workaround a deadlock in Xen > >> Is this separate from the general problem with EPT A/D and > >> write-protecting pagetables? > >> > > It sounds like it is, it happens without write-protecting in-guest > > pagetables. I didn't have time to investigate where the deadlock > > happens and since the workaround is fine for the usecase it wasn't a > > priority to figure out. > > Thinking about it, PML will do the wrong thing (deadlocks aside) as soon > as any altp2m gfn translations are used. > > I'd be tempted to work around the deadlock by disabling pml the moment > altp2m is touched. That would give a sightly less bad user experience, > and should be easy to sort for 4.13. > > Thoughts, (inc. Juergen as RM) ? That sounds like a good idea to me, that way you can keep pml for guests where it doesn't cause an issue instead of disabling it system wide. Tamas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] x86: clang build check adjustments
On Fri, Nov 15, 2019 at 04:00:42PM +0100, Jan Beulich wrote: > On 15.11.2019 15:45, Roger Pau Monné wrote: > > Also, if > > possible, could both patches have the same prefix? (x86/clang) > > I did notice the prefix difference before sending the series. > I wouldn't mind making both just x86: (moving "clang" elsewhere > in the title of yours), but I don't want to make mine have > x86/clang, because I think these should represent subsystems or > alike. Oh, that's fine then. Feel free to move the 'clang' part somewhere else. Ie: 'x86: move and fix .skip clang check' LGTM for example. Thanks. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 11/15/19 3:10 PM, Jürgen Groß wrote: > On 15.11.19 16:05, George Dunlap wrote: >> FTR, please avoid top-posting. :-) >> >> On 11/15/19 2:31 PM, Steven Haigh wrote: >>> Just regarding the use of a system environment variable to turn this >>> feature / bugfix / hack on and off - this would probably break starting >>> the VM via the xendomains script. >>> >>> If the VM definition is in /etc/xen/auto/, then there would be nothing >>> to set the environment variable before the VM is launched - hence it >>> would not be applied and a guest crash would occur... >>> >>> Depending on the VM's settings, this would either continue to start & >>> crash - or just stop again until it could be started with the ENV >>> variable. >> >> Right. So a couple of options: >> >> 1. Users of xendomains could set the environment variable in their >> xendomains script >> >> 2. We could add a xl.cfg option. Unknown xl.cfg entries are ignored >> (for better or for worse); in the future, when the "fake ht" thing is >> replaced, we can either continue ignoring it, or give a useful error >> message saying how it should be changed. >> >> 2a. We could have the config option *replace* the environment variable; >> in which case we'd leave libvirt users high and dry >> >> 2b. We could have the config option cause xl to *set* the environment >> variable, which should continue to allow other toolstacks (even those >> not using libxl) to potentially work around the issue. >> >> Right now I'm leaning towards 2b, and having it be in a separate patch. > > In which case we should consider having a way to set arbitrary > environment variables from the config file in order to avoid this kind > of discussion in future similar cases. Right, I was thinking about a good / useful interface; e.g.: workarounds = [ AMD_RYZEN_TOPOLOGY_FIX=ignore ] And then have 'ignore' mean, "Ignore this if you haven't heard of it, or if the option has gone away; 'auto' mean, "Do the most reasonable thing", 'strict' to mean, "Fail domain build if this workaround is obsolete", or something like that. Then users can dial their own "just make it work" vs "tell me it's gone" as they like. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen/vcpu: Sanitise VCPUOP_initialise call hierachy
On 31/10/2019 21:25, Julien Grall wrote: > Hi, > > On 31/10/2019 19:28, Andrew Cooper wrote: >> This code is especially tangled. VCPUOP_initialise calls into >> arch_initialise_vcpu() which calls back into default_initialise_vcpu() which >> is common code. >> >> This path is actually dead code on ARM, because VCPUOP_initialise is filtered >> out by do_arm_vcpu_op(). >> >> The only valid way to start a secondary CPU on ARM is via the PSCI interface. >> The same could in principle be said about INIT-SIPI-SIPI for x86 HVM, if HVM >> guests hadn't already interited a paravirt way of starting CPUs. >> >> Either way, it is quite likely that no future architectures implemented in >> Xen >> are going to want to use a PV interface, as some standardised (v)CPU bringup >> mechanism will already exist. > I am not sure I agree here. Looking at Linux RISCv code (see [1] and > [2]), it looks like the kernel has to deal with selecting one "lucky" > CPU/hart to deal with the boot and park all the others. > > So it looks like to me there are nothing at the moment on RISCv to do > (v)CPU bring-up. We might be able to use PSCI (although this is an ARM > specific way), but would rather wait and see what RISCv folks come up > with before deciding PV is never going to be used. Nothing here prohibits other architectures from using a PV interface if they wish. However, your examples prove my point. There is an already-agreed way to start RISCv CPUs which is not a PV interface, and therefore is very unlikely to adopted to run differently under Xen. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/9] x86: move some xen mm function declarations
On 02.10.2019 19:16, Hongyan Xia wrote: > From: Wei Liu > > They were put into page.h but mm.h is more appropriate. > > The real reason is that I will be adding some new functions which > takes mfn_t. It turns out it is a bit difficult to do in page.h. > > No functional change. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich with one further request: > --- a/xen/include/asm-x86/mm.h > +++ b/xen/include/asm-x86/mm.h > @@ -630,4 +630,9 @@ int arch_acquire_resource(struct domain *d, unsigned int > type, >unsigned int id, unsigned long frame, >unsigned int nr_frames, xen_pfn_t mfn_list[]); > > +/* Allocator functions for Xen pagetables. */ > +void *alloc_xen_pagetable(void); > +void free_xen_pagetable(void *v); > +l1_pgentry_t *virt_to_xen_l1e(unsigned long v); Can these please be put next to e.g. do_page_walk()? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 15.11.19 16:05, George Dunlap wrote: FTR, please avoid top-posting. :-) On 11/15/19 2:31 PM, Steven Haigh wrote: Just regarding the use of a system environment variable to turn this feature / bugfix / hack on and off - this would probably break starting the VM via the xendomains script. If the VM definition is in /etc/xen/auto/, then there would be nothing to set the environment variable before the VM is launched - hence it would not be applied and a guest crash would occur... Depending on the VM's settings, this would either continue to start & crash - or just stop again until it could be started with the ENV variable. Right. So a couple of options: 1. Users of xendomains could set the environment variable in their xendomains script 2. We could add a xl.cfg option. Unknown xl.cfg entries are ignored (for better or for worse); in the future, when the "fake ht" thing is replaced, we can either continue ignoring it, or give a useful error message saying how it should be changed. 2a. We could have the config option *replace* the environment variable; in which case we'd leave libvirt users high and dry 2b. We could have the config option cause xl to *set* the environment variable, which should continue to allow other toolstacks (even those not using libxl) to potentially work around the issue. Right now I'm leaning towards 2b, and having it be in a separate patch. In which case we should consider having a way to set arbitrary environment variables from the config file in order to avoid this kind of discussion in future similar cases. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
FTR, please avoid top-posting. :-) On 11/15/19 2:31 PM, Steven Haigh wrote: > Just regarding the use of a system environment variable to turn this > feature / bugfix / hack on and off - this would probably break starting > the VM via the xendomains script. > > If the VM definition is in /etc/xen/auto/, then there would be nothing > to set the environment variable before the VM is launched - hence it > would not be applied and a guest crash would occur... > > Depending on the VM's settings, this would either continue to start & > crash - or just stop again until it could be started with the ENV variable. Right. So a couple of options: 1. Users of xendomains could set the environment variable in their xendomains script 2. We could add a xl.cfg option. Unknown xl.cfg entries are ignored (for better or for worse); in the future, when the "fake ht" thing is replaced, we can either continue ignoring it, or give a useful error message saying how it should be changed. 2a. We could have the config option *replace* the environment variable; in which case we'd leave libvirt users high and dry 2b. We could have the config option cause xl to *set* the environment variable, which should continue to allow other toolstacks (even those not using libxl) to potentially work around the issue. Right now I'm leaning towards 2b, and having it be in a separate patch. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 15/11/2019 14:55, George Dunlap wrote: >>> +p->basic.htt = false; >> I think everything below here indeed simply undoes what said old >> commit did, but I can't match up this one. And together with the >> question of whether instead leaving it alone, cmp_legacy then >> would have the same question raised. > This is based on a XenServer patch which reverts the entire commit, and > has been maintained in the patchqueue since the commit made it upstream, > AFAICT. So I'll let someone from that team comment on the wherefores; > but as I said, it's by far the best tested option we're going to get. Yes. It is a revert of c/s ca2eee92df44 (Xen 3.4, and maintained forwards until now) because it broke migration across that changeset. It is also this exact version of the revert which has been tested and confirmed to fix the Ryzen 3xxx fixes. A separate experiment tried playing with only the flags, via cpuid="host:htt=0,cmp_legacy=1" and this did not resolve the crashes. Given that both the before and after logic here is broken in the eyes of the APM, I'm not overly fussed about working about exactly how. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] x86: clang build check adjustments
On 15.11.2019 15:45, Roger Pau Monné wrote: > On Fri, Nov 15, 2019 at 11:43:21AM +0100, Jan Beulich wrote: >> 1: fix clang .macro retention check >> 2: clang: move and fix .skip check > > For both: > > Tested-by: Roger Pau Monné > [On FreeBSD and Debian 9.5] Thanks much. I'll take the liberty and drop the seemingly stray 's' from your email address here. > Reviewed-by: Roger Pau Monné > > Note there's a typo in this email's subject (clank v clang). Yeah, I had noticed this right after sending. Fingers and brain must have had a disconnect. > Also, if > possible, could both patches have the same prefix? (x86/clang) I did notice the prefix difference before sending the series. I wouldn't mind making both just x86: (moving "clang" elsewhere in the title of yours), but I don't want to make mine have x86/clang, because I think these should represent subsystems or alike. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 11/15/19 2:42 PM, Jan Beulich wrote:
> On 15.11.2019 15:29, George Dunlap wrote:
>> On 11/15/19 2:18 PM, Jan Beulich wrote:
>>> On 15.11.2019 15:10, George Dunlap wrote:
On 11/15/19 2:06 PM, Andrew Cooper wrote:
> On 15/11/2019 14:04, George Dunlap wrote:
It's not entirely uncommon to (also) consider how the resulting
diff would look like when putting together a change. And besides
the review aspect, there's also the archeology one - "git blame"
yields much more helpful results when code doesn't get moved
around more than necessary. But yes, there's no very clear "this
is the better option" here. I've taken another look at the code
before your change though - everything is already nicely in one
place with Andrew's most recent code reorg. So I'm now having an
even harder time seeing why you want to move things around again.
>>> We don't. I've recommend twice now to have a single "else if" hunk
>>> which is nearly empty, and much more obviously a gross "make it work for
>>> 4.13" bodge.
>> The results are a tiny bit better, but not much really (see attached).
>
> What I meant was:
>
>> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
>> index 312c481f1e..bc088e45f0 100644
>> --- a/tools/libxc/xc_cpuid_x86.c
>> +++ b/tools/libxc/xc_cpuid_x86.c
>> @@ -579,52 +579,71 @@ int xc_cpuid_apply_policy(xc_interface *xch,
>> uint32_t domid,
>> }
>
> else if ( getenv() )
> {
> ...
> }
>
>> else
>> {
>
> With no delta to this block at all.
Then we have to duplicate this code in both blocks:
/*
* These settings are necessary to cause earlier
HVM_PARAM_NESTEDHVM /
* XEN_DOMCTL_disable_migrate settings to be reflected correctly in
* CPUID. Xen will discard these bits if configuration hasn't been
* set for the domain.
*/
p->extd.itsc = true;
p->basic.vmx = true;
p->extd.svm = true;
I won't object if that's what you guys really want.
>>>
>>> Personally I think the duplication is less bad than the far
>>> heavier original code churn, but to be honest, especially with
>>> this intended to go away again soon anyway, I'd not be opposed
>>> at all to
>>>
>>> ...
>>> else if ( getenv() )
>>> goto no_fake_ht;
>>
>> This isn't correct, because you do need to clear htt and cmp_legacy, as
>> well as zeroing out cores_per_package and threads_per_cache on Intel.
>> (At least, that's what XenServer's patch does, and it's the best tested.)
>>
>>> else
>>> {
>>> ...
>>> no_fake_ht:
>>> /*
>>> * These settings are necessary to cause earlier
>>> HVM_PARAM_NESTEDHVM /
>>> * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
>>> * CPUID. Xen will discard these bits if configuration hasn't been
>>> * set for the domain.
>>> */
>>> p->extd.itsc = true;
>>> p->basic.vmx = true;
>>> p->extd.svm = true;
>>> }
>>>
>>> (despite my general dislike of goto).
>>
>> Well I think gotos into other blocks is even worse. :-)
>>
>> I think the result is a lot nicer to review for sure.
>
> Trying to comment despite this having been an attachment:
>
>> --- a/tools/libxc/xc_cpuid_x86.c
>> +++ b/tools/libxc/xc_cpuid_x86.c
>> @@ -579,6 +579,26 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t
>> domid,
>> }
>> else
>> {
>> +if ( getenv("XEN_LIBXC_DISABLE_FAKEHT") ) {
>
> The brace wants to move onto its own line.
Ack
>
>> +p->basic.htt = false;
>
> I think everything below here indeed simply undoes what said old
> commit did, but I can't match up this one. And together with the
> question of whether instead leaving it alone, cmp_legacy then
> would have the same question raised.
This is based on a XenServer patch which reverts the entire commit, and
has been maintained in the patchqueue since the commit made it upstream,
AFAICT. So I'll let someone from that team comment on the wherefores;
but as I said, it's by far the best tested option we're going to get.
-George
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] x86: clank build check adjustments
On 15/11/2019 14:45, Roger Pau Monné wrote: > On Fri, Nov 15, 2019 at 11:43:21AM +0100, Jan Beulich wrote: >> 1: fix clang .macro retention check >> 2: clang: move and fix .skip check > For both: > > Tested-by: Roger Pau Monné > [On FreeBSD and Debian 9.5] > Reviewed-by: Roger Pau Monné > > Note there's a typo in this email's subject (clank v clang). Also, if > possible, could both patches have the same prefix? (x86/clang) Acked-by: Andrew Cooper ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 0/2] x86: clank build check adjustments
On Fri, Nov 15, 2019 at 11:43:21AM +0100, Jan Beulich wrote: > 1: fix clang .macro retention check > 2: clang: move and fix .skip check For both: Tested-by: Roger Pau Monné [On FreeBSD and Debian 9.5] Reviewed-by: Roger Pau Monné Note there's a typo in this email's subject (clank v clang). Also, if possible, could both patches have the same prefix? (x86/clang) Thanks, Roger. ___ 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: adjust xen release cycle text
On 15.11.2019 15:27, Wei Liu wrote: > Fix text about release cycle. Drop the conjured up example that's no > longer applicable. > > Signed-off-by: Wei Liu FWIW Acked-by: Jan Beulich ___ 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: adjust xen release cycle text
On 15.11.19 15:27, Wei Liu wrote: Fix text about release cycle. Drop the conjured up example that's no longer applicable. Signed-off-by: Wei Liu Reviewed-by: Juergen Gross 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 RFC] x86: Add hack to disable "Fake HT" mode
On 15.11.2019 15:29, George Dunlap wrote:
> On 11/15/19 2:18 PM, Jan Beulich wrote:
>> On 15.11.2019 15:10, George Dunlap wrote:
>>> On 11/15/19 2:06 PM, Andrew Cooper wrote:
On 15/11/2019 14:04, George Dunlap wrote:
>>> It's not entirely uncommon to (also) consider how the resulting
>>> diff would look like when putting together a change. And besides
>>> the review aspect, there's also the archeology one - "git blame"
>>> yields much more helpful results when code doesn't get moved
>>> around more than necessary. But yes, there's no very clear "this
>>> is the better option" here. I've taken another look at the code
>>> before your change though - everything is already nicely in one
>>> place with Andrew's most recent code reorg. So I'm now having an
>>> even harder time seeing why you want to move things around again.
>> We don't. I've recommend twice now to have a single "else if" hunk
>> which is nearly empty, and much more obviously a gross "make it work for
>> 4.13" bodge.
> The results are a tiny bit better, but not much really (see attached).
What I meant was:
> diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
> index 312c481f1e..bc088e45f0 100644
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -579,52 +579,71 @@ int xc_cpuid_apply_policy(xc_interface *xch,
> uint32_t domid,
> }
else if ( getenv() )
{
...
}
> else
> {
With no delta to this block at all.
>>>
>>> Then we have to duplicate this code in both blocks:
>>>
>>> /*
>>> * These settings are necessary to cause earlier
>>> HVM_PARAM_NESTEDHVM /
>>> * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
>>> * CPUID. Xen will discard these bits if configuration hasn't been
>>> * set for the domain.
>>> */
>>> p->extd.itsc = true;
>>> p->basic.vmx = true;
>>> p->extd.svm = true;
>>>
>>> I won't object if that's what you guys really want.
>>
>> Personally I think the duplication is less bad than the far
>> heavier original code churn, but to be honest, especially with
>> this intended to go away again soon anyway, I'd not be opposed
>> at all to
>>
>> ...
>> else if ( getenv() )
>> goto no_fake_ht;
>
> This isn't correct, because you do need to clear htt and cmp_legacy, as
> well as zeroing out cores_per_package and threads_per_cache on Intel.
> (At least, that's what XenServer's patch does, and it's the best tested.)
>
>> else
>> {
>> ...
>> no_fake_ht:
>> /*
>> * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM
>> /
>> * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
>> * CPUID. Xen will discard these bits if configuration hasn't been
>> * set for the domain.
>> */
>> p->extd.itsc = true;
>> p->basic.vmx = true;
>> p->extd.svm = true;
>> }
>>
>> (despite my general dislike of goto).
>
> Well I think gotos into other blocks is even worse. :-)
>
> I think the result is a lot nicer to review for sure.
Trying to comment despite this having been an attachment:
>--- a/tools/libxc/xc_cpuid_x86.c
>+++ b/tools/libxc/xc_cpuid_x86.c
>@@ -579,6 +579,26 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t
>domid,
> }
> else
> {
>+if ( getenv("XEN_LIBXC_DISABLE_FAKEHT") ) {
The brace wants to move onto its own line.
>+p->basic.htt = false;
I think everything below here indeed simply undoes what said old
commit did, but I can't match up this one. And together with the
question of whether instead leaving it alone, cmp_legacy then
would have the same question raised.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] golang/xenlight: add missing arguments to libxl_domain_shutdown/reboot
> Actually this has already been submitted and Release-acked here: > > https://patchew.org/Xen/20191023162358.7222-1-george.dun...@citrix.com/ Ah thanks, I remember seeing that now but I confused it with the *very* similar patch for libxl_domain_pause/unpause. -NR ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH V2] kdd.c: Add support for initial handshake in KD protocol for Win 7, 8 and 10 (64 bit)
Hi Julian and Jenish I have queued this patch to my for-next branch based on Paul and Tim's review. Note that Xen is currently frozen. This patch will get committed once the tree is open for new features. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
Just regarding the use of a system environment variable to turn this
feature / bugfix / hack on and off - this would probably break starting
the VM via the xendomains script.
If the VM definition is in /etc/xen/auto/, then there would be nothing
to set the environment variable before the VM is launched - hence it
would not be applied and a guest crash would occur...
Depending on the VM's settings, this would either continue to start &
crash - or just stop again until it could be started with the ENV
variable.
Steven Haigh
net...@crc.id.au https://www.crc.id.au
+613 9001 6090 +614 1293 5897
On Fri, Nov 15, 2019 at 10:57, George Dunlap
wrote:
Changeset ca2eee92df44 ("x86, hvm: Expose host core/HT topology to HVM
guests") attempted to "fake up" a topology which would induce guest
operating systems to not treat vcpus as sibling hyperthreads. This
involved (among other things) actually reporting hyperthreading as
available, but giving vcpus every other APICID. The resulting cpu
featureset is invalid, but most operating systems on most hardware
managed to cope with it.
Unfortunately, Windows running on modern AMD hardware -- including
Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus -- gets
confused by the resulting contradictory feature bits and crashes
during installation. (Linux guests have so far continued to cope.)
A "proper" fix is complicated and it's too late to fix it either for
4.13, or to backport to supported branches. As a short-term fix,
implement an option to disable this "Fake HT" mode. The resulting
topology reported will not be canonical, but experimentally continues
to work with Windows guests.
However, disabling this "Fake HT" mode has not been widely tested, and
will almost certainly break migration if applied inconsistently.
To minimize impact while allowing administrators to disable "Fake HT"
only on guests which are known not to work without it (i.e., Windows
guests) on affected hardware, add an environment variable which can be
set to disable the "Fake HT" mode on such hardware.
Reported-by: Steven Haigh
Reported-by: Andreas Kinzler
Signed-off-by: George Dunlap
---
This has been compile-tested only; I'm posting it early to get
feedback on the approach.
TODO: Prevent such guests from being migrated
Open questions:
- Is this the right place to put the `getenv` check?
- Is there any way we can make migration work, at least in some cases?
- Can we check for known-problematic models, and at least report a
more useful error?
CC: Andrew Cooper
CC: Jan Beulich
CC: Ian Jackson
CC: Anthony Perard
---
tools/libxc/xc_cpuid_x86.c | 74
+++---
1 file changed, 45 insertions(+), 29 deletions(-)
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 312c481f1e..70c85e1467 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -579,52 +579,68 @@ int xc_cpuid_apply_policy(xc_interface *xch,
uint32_t domid,
}
else
{
-/*
- * Topology for HVM guests is entirely controlled by Xen.
For now, we
- * hardcode APIC_ID = vcpu_id * 2 to give the illusion of no
SMT.
- */
-p->basic.htt = true;
+p->basic.htt = false;
p->extd.cmp_legacy = false;
-/*
- * Leaf 1 EBX[23:16] is Maximum Logical Processors Per
Package.
- * Update to reflect vLAPIC_ID = vCPU_ID * 2, but make sure
to avoid
- * overflow.
- */
-if ( !(p->basic.lppp & 0x80) )
-p->basic.lppp *= 2;
-
switch ( p->x86_vendor )
{
case X86_VENDOR_INTEL:
for ( i = 0; (p->cache.subleaf[i].type &&
i < ARRAY_SIZE(p->cache.raw)); ++i )
{
-p->cache.subleaf[i].cores_per_package =
-(p->cache.subleaf[i].cores_per_package << 1) | 1;
+p->cache.subleaf[i].cores_per_package = 0;
p->cache.subleaf[i].threads_per_cache = 0;
}
break;
+}
-case X86_VENDOR_AMD:
-case X86_VENDOR_HYGON:
+if ( !getenv("XEN_LIBXC_DISABLE_FAKEHT") ) {
/*
- * Leaf 0x8008 ECX[15:12] is ApicIdCoreSize.
- * Leaf 0x8008 ECX[7:0] is NumberOfCores (minus one).
- * Update to reflect vLAPIC_ID = vCPU_ID * 2. But avoid
- * - overflow,
- * - going out of sync with leaf 1 EBX[23:16],
- * - incrementing ApicIdCoreSize when it's zero (which
changes the
- * meaning of bits 7:0).
+ * Topology for HVM guests is entirely controlled by
Xen. For now, we
+ * hardcode APIC_ID = vcpu_id * 2 to give the illusion
of no SMT.
*/
-if ( p->extd.nc < 0x7f )
+p->basic.htt = true;
+
+/*
+ * Leaf 1 EBX[23:16] is Maximum Logical Processors Per
Package.
+
[Xen-devel] Ping: [PATCH v2] x86/stackframe/32: repair 32-bit Xen PV
Andy,
On 29.10.2019 10:28, Jan Beulich wrote:
> Once again RPL checks have been introduced which don't account for a
> 32-bit kernel living in ring 1 when running in a PV Xen domain. The
> case in FIXUP_FRAME has been preventing boot; adjust BUG_IF_WRONG_CR3
> as well just in case.
>
> Fixes: 3c88c692c287 ("x86/stackframe/32: Provide consistent pt_regs")
> Signed-off-by: Jan Beulich
would you mind clarifying whether I should follow Thomas' request,
overriding what you had asked for an I did carry out for v2? I don't
think this regression should be left unfixed for much longer (as
much as the other part of it, addressed by a later 2-patch series).
Thanks, Jan
> ---
> v2: Avoid #ifdef and alter comment along the lines of Andy's suggestion.
>
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -48,6 +48,13 @@
>
> #include "calling.h"
>
> +/*
> + * When running on Xen PV, the actual %cs register's RPL in the kernel is 1,
> + * not 0. If we need to distinguish between a %cs from kernel mode and a %cs
> + * from user mode, we can do test $2 instead of test $3.
> + */
> +#define USER_SEGMENT_RPL_MASK 2
> +
> .section .entry.text, "ax"
>
> /*
> @@ -172,7 +179,7 @@
> ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI
> .if \no_user_check == 0
> /* coming from usermode? */
> - testl $SEGMENT_RPL_MASK, PT_CS(%esp)
> + testl $USER_SEGMENT_RPL_MASK, PT_CS(%esp)
> jz .Lend_\@
> .endif
> /* On user-cr3? */
> @@ -217,7 +224,7 @@
> testl $X86_EFLAGS_VM, 4*4(%esp)
> jnz .Lfrom_usermode_no_fixup_\@
> #endif
> - testl $SEGMENT_RPL_MASK, 3*4(%esp)
> + testl $USER_SEGMENT_RPL_MASK, 3*4(%esp)
> jnz .Lfrom_usermode_no_fixup_\@
>
> orl $CS_FROM_KERNEL, 3*4(%esp)
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86: Add hack to disable "Fake HT" mode
On 11/15/19 2:18 PM, Jan Beulich wrote:
> On 15.11.2019 15:10, George Dunlap wrote:
>> On 11/15/19 2:06 PM, Andrew Cooper wrote:
>>> On 15/11/2019 14:04, George Dunlap wrote:
>> It's not entirely uncommon to (also) consider how the resulting
>> diff would look like when putting together a change. And besides
>> the review aspect, there's also the archeology one - "git blame"
>> yields much more helpful results when code doesn't get moved
>> around more than necessary. But yes, there's no very clear "this
>> is the better option" here. I've taken another look at the code
>> before your change though - everything is already nicely in one
>> place with Andrew's most recent code reorg. So I'm now having an
>> even harder time seeing why you want to move things around again.
> We don't. I've recommend twice now to have a single "else if" hunk
> which is nearly empty, and much more obviously a gross "make it work for
> 4.13" bodge.
The results are a tiny bit better, but not much really (see attached).
>>>
>>> What I meant was:
>>>
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 312c481f1e..bc088e45f0 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -579,52 +579,71 @@ int xc_cpuid_apply_policy(xc_interface *xch,
uint32_t domid,
}
>>>
>>> else if ( getenv() )
>>> {
>>> ...
>>> }
>>>
else
{
>>>
>>> With no delta to this block at all.
>>
>> Then we have to duplicate this code in both blocks:
>>
>> /*
>> * These settings are necessary to cause earlier
>> HVM_PARAM_NESTEDHVM /
>> * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
>> * CPUID. Xen will discard these bits if configuration hasn't been
>> * set for the domain.
>> */
>> p->extd.itsc = true;
>> p->basic.vmx = true;
>> p->extd.svm = true;
>>
>> I won't object if that's what you guys really want.
>
> Personally I think the duplication is less bad than the far
> heavier original code churn, but to be honest, especially with
> this intended to go away again soon anyway, I'd not be opposed
> at all to
>
> ...
> else if ( getenv() )
> goto no_fake_ht;
This isn't correct, because you do need to clear htt and cmp_legacy, as
well as zeroing out cores_per_package and threads_per_cache on Intel.
(At least, that's what XenServer's patch does, and it's the best tested.)
> else
> {
> ...
> no_fake_ht:
> /*
> * These settings are necessary to cause earlier HVM_PARAM_NESTEDHVM /
> * XEN_DOMCTL_disable_migrate settings to be reflected correctly in
> * CPUID. Xen will discard these bits if configuration hasn't been
> * set for the domain.
> */
> p->extd.itsc = true;
> p->basic.vmx = true;
> p->extd.svm = true;
> }
>
> (despite my general dislike of goto).
Well I think gotos into other blocks is even worse. :-)
I think the result is a lot nicer to review for sure.
-George
>From b83b7b6db04b0705878798cded2f4c6904cf6fb5 Mon Sep 17 00:00:00 2001
From: George Dunlap
Date: Thu, 14 Nov 2019 16:58:34 +
Subject: [PATCH] x86: Add hack to disable "Fake HT" mode
Changeset ca2eee92df44 ("x86, hvm: Expose host core/HT topology to HVM
guests") attempted to "fake up" a topology which would induce guest
operating systems to not treat vcpus as sibling hyperthreads. This
involved (among other things) actually reporting hyperthreading as
available, but giving vcpus every other APICID. The resulting cpu
featureset is invalid, but most operating systems on most hardware
managed to cope with it.
Unfortunately, Windows running on modern AMD hardware -- including
Ryzen 3xxx series processors, and reportedly EPYC "Rome" cpus -- gets
confused by the resulting contradictory feature bits and crashes
during installation. (Linux guests have so far continued to cope.)
A "proper" fix is complicated and it's too late to fix it either for
4.13, or to backport to supported branches. As a short-term fix,
implement an option to disable this "Fake HT" mode. The resulting
topology reported will not be canonical, but experimentally continues
to work with Windows guests.
However, disabling this "Fake HT" mode has not been widely tested, and
will almost certainly break migration if applied inconsistently.
To minimize impact while allowing administrators to disable "Fake HT"
only on guests which are known not to work without it (i.e., Windows
guests) on affected hardware, add an environment variable which can be
set to disable the "Fake HT" mode on such hardware.
Reported-by: Steven Haigh
Reported-by: Andreas Kinzler
Signed-off-by: George Dunlap
---
This has been compile-tested only; I'm posting it early to get
feedback on the approach.
TODO: Prevent such guests from being migrated
Open questions:
- Is this the right
[Xen-devel] [PATCH for-4.13] docs: adjust xen release cycle text
Fix text about release cycle. Drop the conjured up example that's no longer applicable. Signed-off-by: Wei Liu --- docs/process/xen-release-management.pandoc | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/docs/process/xen-release-management.pandoc b/docs/process/xen-release-management.pandoc index d6abc90a02..e1aa1eda8f 100644 --- a/docs/process/xen-release-management.pandoc +++ b/docs/process/xen-release-management.pandoc @@ -15,12 +15,11 @@ that they can have an idea what to expect from the Release Manager. # Xen release cycle -The Xen hypervisor project now releases twice a year, at the beginning of -June and the beginning of December. The actual release date depends on a lot -of factors. +The Xen hypervisor project now releases every 8 months. The actual release date +depends on a lot of factors. We can roughly divide one release into two periods. The development period -and the freeze period. The former is 4 months long and the latter is about 2 +and the freeze period. The former is 6 months long and the latter is about 2 months long. During development period, contributors submit patches to be reviewed and @@ -34,14 +33,6 @@ During freeze period, the tree is closed for new features. Only bug fixes are accepted. This period can be shorter or longer than 2 months. If it ends up longer than 2 months, it eats into the next development period. -Here is a conjured up example (use ```cal 2017``` to get an idea): - -* Development period: 2017 June 11 - 2017 September 29 -* the "cut-off date" is 2017 September 29 -* the "last posting date" is 2017 September 15 -* Freeze period: 2017 October 2 - 2017 December 7 -* the anticipated release date is 2017 December 7 - # The different roles in a Xen release ## Release Manager -- 2.20.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
