[Xen-devel] OSStest commits and Xen releases
I have found an alarming tendency regarding changes in the OSStest repository: over the last 2 years (or 3 Xen versions) there has been a pattern of OSStest commits being more frequent during the RC phase of a Xen release. On average there were about 4 commits to osstest.git per week. The numbers were significantly higher during RC-phases: Version RC-phase OSStest commits per week 4.12 2019/01/16 - 19 4.11 2018/04/17 - 2018/07/09 10 4.10 2017/10/16 - 2017/12/13 6 I have looked at this as I would have liked to cut 4.12-RC2 this Monday, but OSStests for xen-unstable failed over the weekend. Ian suspected a change in OSStest to be blamed (needs to be verified). As the release manager I don't like RCs being delayed due to changes in our infrastructure. For Xen we have code freeze and patches to go in need the release manager's ack. Shouldn't the same apply to OSStest? I like OSStest very much as it helps catching bugs early. But I believe the main development should not be done in the time when we need it's results to be most reliable. Thoughts? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 test] 132521: tolerable FAIL - PUSHED
flight 132521 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/132521/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 132421 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 132421 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 132421 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 132421 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 132421 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail 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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 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-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 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linux189b75ad3fc2d4a0d40a818ca298526d254ccdc4 baseline version: linuxef50e3059ac91e7b035bce1a89e5b49771ed353a Last test of basis 132421 2019-01-23 07:41:35 Z5 days Testing same since 132494 2019-01-26 09:11:01 Z2 days2 attempts People who touched revisions under test: Aaron Brown Adrian Hunter Anders Roxell Andrew Morton Andy Shevchenko Arnaldo Carvalho de Melo Arnd Bergmann Boris Brezillon Breno Leitao Brian Foster Chuck Lever Corey Minyard Dan Williams Daniel Santos Daniel Vetter Daniel Vetter Dave Airlie David Ahern David Disseldorp David Rientjes David S. Miller Don Brace Greg Kroah-Hartman Ilias Tsitsimpis Jacob Keller Jan Kara Jason Gunthorpe Jeff Kirsher Jiri Olsa Joel Fernandes (Google) Jonas Danielsson Joseph Qi João Paulo Rechi Vita João Paulo Rechi Vita Junxiao Bi Kai-Heng Feng Kees Cook Kevin Barnett Linus Torvalds Lucas Stach Maciej W. Rozycki Martin K. Petersen
Re: [Xen-devel] RT Xen on ARM - R-Car series
Andrii, YEY! I have finally got an error message! The rest of the mail is inline. 2019年1月28日(月) 17:25 Andrii Anisov : > Hello Jairo, > > On 28.01.19 19:20, LOPEZ, FUENTES NACARINO Jairo Eduardo wrote: > > > I was able to compile the Xen image with earlyprintk without issue. > > Cool. > > > It is comforting to get some sort of feedback from the device, even if > it is failing. > > Indeed:) > > > When attempting to boot the Xen image created I did get print messages > but I get the following: > ... > > I see you have freshly built BL2: > > > [0.120591] NOTICE: BL2: v1.4(release):15dba6b > > [0.125081] NOTICE: BL2: Built : 11:34:26, Jan 21 2019 > ... > > - UART enabled - > > - CPU booting - > > - Current EL 0004 - > > - Xen must be entered in NS EL2 mode - > > But something went wrong, and your BL2 did not start u-boot in EL2. > Consequently, hypervisor is not started in EL2 and can not do its work. > Please verify you have your ATF built with ` > RCAR_BL33_EXECUTION_EL=BL33_EL2` as it is stated in the correspondent > recipe [1]. > You can search for it in /build/tmp/work/ name>-poky-linux/v1.4+renesas+/tmp/* . > And if it is compiled with the flag, then take care to reflash all > bootloader images on your board. > > [1] > https://github.com/xen-troops/meta-demo/blob/master/meta-rcar-gen3-xen/recipes-bsp/arm-trusted-firmware/arm-trusted-firmware_git.bbappend > > -- > Sincerely, > Andrii Anisov. > As you mentioned, I checked the ATF build at the following path: /build/tmp/work/-poky-linux/arm-trusted-firmware/v1.4+renesas+/temp Much to my surprise I found no trace of thr RCAR_BL33_EXECUTION_EL=BL33_EL2. It seems that at some point in time I removed the Xen troops meta-demo layer which nullified the ATFW_OPT_append in that layer. I was able to readjust the layers and check that the compilation included the necessary ATF option. After a successful u-boot firmware update, I was met with the following information on the R-Car M3: [0.000191] NOTICE: BL2: R-Car Gen3 Initial Program Loader(CA57) Rev.1.0.21 [0.005753] NOTICE: BL2: PRR is R-Car M3 Ver.1.0 [0.010422] NOTICE: BL2: Board is Starter Kit Rev.1.0 [0.015534] NOTICE: BL2: Boot device is HyperFlash(80MHz) [0.020960] NOTICE: BL2: LCM state is CM [0.025002] NOTICE: BL2: AVS setting succeeded. DVFS_SetVID=0x53 [0.030989] NOTICE: BL2: DDR3200(rev.0.33)NOTICE: [COLD_BOOT]NOTICE: ..0 [0.086113] NOTICE: BL2: DRAM Split is 2ch [0.089998] NOTICE: BL2: QoS is default setting(rev.0.19) [0.095502] NOTICE: BL2: Lossy Decomp areas [0.099675] NOTICE: Entry 0: DCMPAREACRAx:0x8540 DCMPAREACRBx:0x570 [0.106760] NOTICE: Entry 1: DCMPAREACRAx:0x4000 DCMPAREACRBx:0x0 [0.113672] NOTICE: Entry 2: DCMPAREACRAx:0x2000 DCMPAREACRBx:0x0 [0.120586] NOTICE: BL2: v1.4(release):15dba6b [0.125076] NOTICE: BL2: Built : 14:48:12, Jan 29 2019 [0.130264] NOTICE: BL2: Normal boot [0.133909] NOTICE: BL2: dst=0xe6320d00 src=0x818 len=512(0x200) [0.140292] NOTICE: BL2: dst=0x43f0 src=0x8180400 len=6144(0x1800) [0.146913] NOTICE: BL2: dst=0x4400 src=0x81c len=65536(0x1) [0.154144] NOTICE: BL2: dst=0x4410 src=0x820 len=1048576(0x10) [0.168967] NOTICE: BL2: dst=0x5000 src=0x864 len=1048576(0x10) U-Boot 2015.04 (Jan 21 2019 - 20:33:02) CPU: Renesas Electronics R8A7796 rev 1.0 Board: M3ULCB I2C: ready DRAM: 1.9 GiB Bank #0: 0x04800 - 0x07fff, 896 MiB Bank #1: 0x6 - 0x63fff, 1 GiB MMC: sh-sdhi: 0, sh-sdhi: 1 In:serial Out: serial Err: serial Net: ravb First, it is nice to know where to look for the build time for the BL2! After loading earlyprintk enabled Xen and the 4.14 Linux kernel from Yocto 3.9, I was met with the following output: Starting kernel ... - UART enabled - - CPU booting - - Current EL 0008 - - Xen starting at EL2 - - Zero BSS - - Setting up control registers - - Turning on paging - - Ready - (XEN) Checking for initrd in /chosen (XEN) RAM: 4800 - 7fff (XEN) RAM: 0006 - 00063fff (XEN) RAM: 0006 - 00063fff (XEN) (XEN) MODULE[0]: 4800 - 48011000 Device Tree (XEN) MODULE[1]: 7a00 - 7c00 Kernel (XEN) RESVD[0]: 4800 - 48011000 (XEN) (XEN) (XEN) Command line: dom0_mem=752M console=dtuart dtuart=serial0 dom0_max_vcpus=4 (XEN) PFN compression on bits 19...20 (XEN) Xen BUG at page_alloc.c:274 (XEN) [ Xen-4.12.0-rc arm64 debug=y Not tainted ] (XEN) CPU:0 (XEN) PC: 0028b310 page_alloc.c#bootmem_region_add+0x188/0x198 (XEN) LR: 0028b370 (XEN) SP: 002d7d00 (XEN) CPSR: 83c9 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 0060 X1: 0003 X2: 0004 (XEN) X3: 0064 X4: 88011000 X5:
Re: [Xen-devel] [PATCH 0/3] gcc-plugins: Introduce stackinit plugin
On 23.01.2019 14:03, Kees Cook wrote: > This adds a new plugin "stackinit" that attempts to perform unconditional > initialization of all stack variables Hello Kees! Hello everyone! I was curious about the performance impact of the initialization of all stack variables. So I did a very brief test with this plugin on top of 4.20.5. hackbench on Intel Core i7-4770 showed ~0.7% slowdown. hackbench on Kirin 620 (ARM Cortex-A53 Octa-core 1.2GHz) showed ~1.3% slowdown. This test involves the kernel scheduler and allocator. I can't say whether they use stack aggressively. Maybe performance tests of other subsystems (e.g. network subsystem) can show different numbers. Did you try? I've heard a hypothesis that the initialization of all stack variables would pollute CPU caches, which is critical for some types of computations. Maybe some micro-benchmarks can disprove/confirm that? Thanks! Best regards, Alexander ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 3/8] microcode: introduce the global microcode cache
On Mon, Jan 28, 2019 at 06:39:43PM +0100, Roger Pau Monné wrote:
>On Mon, Jan 28, 2019 at 03:06:45PM +0800, Chao Gao wrote:
>> to replace the current per-cpu cache 'uci->mc'.
>>
>> Compared to the current per-cpu cache, the benefits of the global
>> microcode cache are:
>> 1. It reduces the work that need to be done on each CPU. Parsing ucode
>> file can be done once on one CPU. Other CPUs needn't parse ucode file.
>> Instead, they can find out and load a patch with newer revision from
>> the global cache.
>> 2. It reduces the memory consumption on a system with many CPU cores.
>>
>> Two functions, save_patch() and find_patch() are introduced. The
>> former adds one given patch to the global cache. The latter gets
>> a newer and matched ucode patch from the global cache.
>>
>> Note that I deliberately avoid touching 'uci->mc' as I am going to
>> remove it completely in the next patch.
>>
>> Signed-off-by: Chao Gao
>> ---
>> Changes in v5:
>> - reword the commit description
>> - find_patch() and save_patch() are abstracted into common functions
>>with some hooks for AMD and Intel
>> ---
>> xen/arch/x86/microcode.c| 54 +++
>> xen/arch/x86/microcode_amd.c| 94
>> ++---
>> xen/arch/x86/microcode_intel.c | 71 ---
>> xen/include/asm-x86/microcode.h | 13 ++
>> 4 files changed, 219 insertions(+), 13 deletions(-)
>>
>> diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
>> index 4163f50..7d5b769 100644
>> --- a/xen/arch/x86/microcode.c
>> +++ b/xen/arch/x86/microcode.c
>> @@ -61,6 +61,8 @@ static struct ucode_mod_blob __initdata ucode_blob;
>> */
>> static bool_t __initdata ucode_scan;
>>
>> +static LIST_HEAD(microcode_cache);
>> +
>> void __init microcode_set_module(unsigned int idx)
>> {
>> ucode_mod_idx = idx;
>> @@ -208,6 +210,58 @@ static void microcode_fini_cpu(unsigned int cpu)
>> spin_unlock(_mutex);
>> }
>>
>> +/* Save a ucode patch to the global cache list */
>> +bool save_patch(struct microcode_patch *new_patch)
>
>This being a global function likely requires some kind of prefix, I
>would suggest microcode_save_patch, the same applies to the find_patch
>function below.
Will do.
>
>> +{
>> +struct microcode_patch *microcode_patch;
>> +
>> +list_for_each_entry(microcode_patch, _cache, list)
>
>I think I'm missing something here, but given the conversation we had
>in the previous version of the series [0] I assumed there was only a
>single microcode patch that applies to the whole system, and that
>there was no need to keep a list?
>
>Because Xen doesn't support running on such mixed systems anyway?
No. As Jan pointed out in [1], we still want to support mixed systems
which are allowed by Intel and AMD.
[1] https://lists.xenproject.org/archives/html/xen-devel/2018-11/msg03479.html
>
>[0] https://lists.xenproject.org/archives/html/xen-devel/2018-12/msg00381.html
>
>> diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
>> index 1ed573a..fc35c8d 100644
>> --- a/xen/arch/x86/microcode_intel.c
>> +++ b/xen/arch/x86/microcode_intel.c
>> @@ -276,18 +324,24 @@ static int apply_microcode(unsigned int cpu)
>> unsigned int val[2];
>> unsigned int cpu_num = raw_smp_processor_id();
>> struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu_num);
>> +struct microcode_intel *mc_intel;
>> +struct microcode_patch *patch;
>>
>> /* We should bind the task to the CPU */
>> BUG_ON(cpu_num != cpu);
>>
>> -if ( uci->mc.mc_intel == NULL )
>> +patch = find_patch(cpu);
>> +if ( !patch )
>> return -EINVAL;
>>
>> +mc_intel = patch->data;
>> +BUG_ON(!mc_intel);
>> +
>> /* serialize access to the physical write to MSR 0x79 */
>> spin_lock_irqsave(_update_lock, flags);
>>
>> /* write microcode via MSR 0x79 */
>> -wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)uci->mc.mc_intel->bits);
>> +wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits);
>> wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL);
>>
>> /* As documented in the SDM: Do a CPUID 1 here */
>> @@ -298,19 +352,19 @@ static int apply_microcode(unsigned int cpu)
>> val[1] = (uint32_t)(msr_content >> 32);
>>
>> spin_unlock_irqrestore(_update_lock, flags);
>> -if ( val[1] != uci->mc.mc_intel->hdr.rev )
>> +if ( val[1] != mc_intel->hdr.rev )
>> {
>> printk(KERN_ERR "microcode: CPU%d update from revision "
>> "%#x to %#x failed. Resulting revision is %#x.\n", cpu_num,
>> - uci->cpu_sig.rev, uci->mc.mc_intel->hdr.rev, val[1]);
>> + uci->cpu_sig.rev, mc_intel->hdr.rev, val[1]);
>> return -EIO;
>> }
>> printk(KERN_INFO "microcode: CPU%d updated from revision "
>> "%#x to %#x, date = %04x-%02x-%02x \n",
>> cpu_num, uci->cpu_sig.rev, val[1],
>> - uci->mc.mc_intel->hdr.date & 0x,
>>
[Xen-devel] [xen-unstable-smoke test] 132550: tolerable all pass - PUSHED
flight 132550 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/132550/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-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 a18be06acab6ce7d5f035d4df538397a548d46ea baseline version: xen f19a199281a23725beb73bef61eb8964d8e225ce Last test of basis 132538 2019-01-28 17:00:45 Z0 days Testing same since 132550 2019-01-29 01:00:30 Z0 days1 attempts People who touched revisions under test: Julien Grall jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass 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 f19a199281..a18be06aca a18be06acab6ce7d5f035d4df538397a548d46ea -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 132514: regressions - FAIL
flight 132514 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/132514/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 12 guest-start/debianhvm.repeat fail REGR. vs. 131842 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 131842 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 131842 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 131842 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 131842 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 131842 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 131842 test-amd64-i386-xl-pvshim12 guest-start fail 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 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-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-libvirt 13 migrate-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-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 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-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 test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: qemuuad7a21e81231ae64540310384fb0f87ac8758b02 baseline version: qemuu147923b1a901a0370f83a0f4c58ec1baffef22f0 Last test of basis 131842 2019-01-09 00:37:22 Z 20 days Failing since131892 2019-01-09 23:37:00 Z 19 days 17 attempts Testing same since 132488 2019-01-26 03:28:08 Z2 days2 attempts People who touched revisions under test: Aaron Lindsay Aaron Lindsay Aaron Lindsay Aleksandar Markovic Alex Bennée Alex Williamson Alexander Graf Alexander Kanavin Alexandro Sanchez Bach Alexey Kardashevskiy Alistair Francis Andrew Jeffery Anthony PERARD BALATON Zoltan Borislav Petkov Christian Borntraeger Christophe Fergeau Cleber Rosa Collin Walling Cornelia Huck Cédric Le Goater Daniel P. Berrangé David Gibson David Hildenbrand Dongli Zhang Dr. David Alan Gilbert Edgar E. Iglesias Eduardo Habkost Eduardo Otubo Emilio G. Cota Eric Auger Eric Blake Fam Zheng Fei Li Fei Li Frediano Ziglio Fredrik Noring Gerd Hoffmann Greg Kurz Guenter Roeck Igor Mammedov Janosch Frank Jian Wang Joel Stanley John Snow Jon Diekema Juan Quintela Kamal Heib Kashyap Chamarthy Laurent Vivier Laurent Vivier Li Feng Li Qiang Marc-André Lureau Marcel Apfelbaum Mark Cave-Ayland Markus Armbruster Max Filippov Michael Clark Michael Roth Michael S. Tsirkin Nisarg Shah Palmer Dabbelt Paolo Bonzini Paul Durrant Peng Hao Peter Maydell Philippe Mathieu-Daudé Philippe
Re: [Xen-devel] [PATCH] tools/libxl: Fix leaking ssid_label in libxl_name_to_domid
On Mon, Jan 28, 2019 at 5:16 AM Wei Liu wrote:
>
> On Sat, Jan 26, 2019 at 10:45:07PM -0700, Tamas K Lengyel wrote:
> > On systems with XSM enabled libxl_name_to_domid leaks memory
> > allocated for ssid_label:
> >
> > ==2693== 53 bytes in 2 blocks are definitely lost in loss record 4 of 8
> > ==2693==at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
> > ==2693==by 0x6C0A3B9: strdup (strdup.c:42)
> > ==2693==by 0x5108294: libxl_flask_sid_to_context (libxl_flask.c:39)
> > ==2693==by 0x50C2B64: libxl__xcinfo2xlinfo (libxl_domain.c:267)
> > ==2693==by 0x50C2E02: libxl_list_domain (libxl_domain.c:308)
> > ==2693==by 0x508A3C5: libxl_name_to_domid (libxl_utils.c:77)
> >
> > Signed-off-by: Tamas K Lengyel
> > Cc: Ian Jackson
> > Cc: Wei Liu
> > ---
> > tools/libxl/libxl_utils.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> > index e50e094c48..99abfa5497 100644
> > --- a/tools/libxl/libxl_utils.c
> > +++ b/tools/libxl/libxl_utils.c
> > @@ -79,6 +79,8 @@ int libxl_name_to_domid(libxl_ctx *ctx, const char *name,
> > return ERROR_NOMEM;
> >
> > for (i = 0; i < nb_domains; i++) {
> > +if (dominfo[i].ssid_label)
> > +free(dominfo[i].ssid_label);
> > domname = libxl_domid_to_name(ctx, dominfo[i].domid);
> > if (!domname)
> > continue;
>
> Thanks for reporting this issue. I think your patch isn't future-proof.
>
> Can you try the following patch?
Works too.
Thanks,
Tamas
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12 6/8] xen/arm: Implement workaround for Cortex-A76 erratum 1165522
On Mon, 28 Jan 2019, Julien Grall wrote:
> On 1/27/19 9:55 AM, Julien Grall wrote:
> > Hi,
> >
> > On 1/25/19 9:36 PM, Stefano Stabellini wrote:
> > > On Thu, 24 Jan 2019, Julien Grall wrote:
> > > > @James, please correct me if I am wrong below :).
> > > >
> > > > On 24/01/2019 00:52, Stefano Stabellini wrote:
> > > > > On Wed, 28 Nov 2018, Julien Grall wrote:
> > > > ... in the context of the errata, you have to imagine what can happen if
> > > > an AT
> > > > instruction is inserted (via speculation) between each instruction and
> > > > what
> > > > happen if the system registers are re-ordered.
> > > >
> > > > The key of the erratum is VTTBR_EL2. This is what will stop a speculated
> > > > AT
> > > > instruction to allocate a TLBs entry because you are not allowed to
> > > > cache a
> > > > translation that will fault. Without the isb() here, the VTTBR_EL2 may
> > > > be
> > > > synchronized before the rest of the context, so a speculated AT
> > > > instruction
> > > > may use an inconsistent state and allocate a TLB entry with an
> > > > unexpected
> > > > translation against the guest.
> > > >
> > > > So here, we want to ensure the rest of the context is synchronized
> > > > before
> > > > writing to VTTBR_EL2, hence the isb().
> > >
> > > OK. I understand the explanation, thank you.
> > >
> > > I just thought that the CPU would be smart enough to only reorder system
> > > registers writes when appropriate, especially when the CPU is also doing
> > > speculation at the same time. Why would it speculate if it knows that it
> > > is reordering sysreg writes that can badly affect the speculation
> > > itself? Let me say that it doesn't sound like a "sane" behavior to me.
> > > But if it behaves this way, it behaves this way...
> >
> > I hope you are aware we are speaking about an erratum here... Not what the
> > Arm Arm allows.
I know -- we are talking about a specific CPU implementation. That is
why it seems strange to me that a CPU would reorder things that it
should know they cause trouble to speculation. Anyway, no point in
discussing hardware design choices at this stage.
> > Aside the erratum, a processor is allowed to do whatever it wants if it is
> > within the Arm Arm. These registers are described as out-of-context and
> > should not be used by speculation in EL2. If you want to use them in EL2,
> > you need an isb() before any instruction in EL2 using them otherwise you may
> > use an inconsistent context. This is giving enough freedom to the processor
> > while the impact in the software is minimal.
> >
> > [...]
> >
> > > > >
> > > > > > /* Ensure VTTBR_EL2 is synchronized before flushing the
> > > > > > TLBs */
> > > > > > isb();
> > > > > > }
> > > > > > @@ -1504,6 +1545,23 @@ static uint32_t __read_mostly vtcr;
> > > > > > static void setup_virt_paging_one(void *data)
> > > > > > {
> > > > > > WRITE_SYSREG32(vtcr, VTCR_EL2);
> > > > > > +
> > > > > > + /*
> > > > > > + * ARM64_WORKAROUND_AT_SPECULATE: We want to keep the TLBs free
> > > > > > from
> > > > > > + * entries related to EL1/EL0 translation regime until a guest
> > > > > > vCPU
> > > > > > + * is running. For that, we need to set-up VTTBR to point to an
> > > > > > empty
> > > > > > + * page-table and turn on stage-2 translation.
> > > > >
> > > > > I don't understand why this is needed: isn't the lack of HCR_VM (due
> > > > > to
> > > > > your previous patch) supposed to be sufficient? How can there be
> > > > > speculation without HCR_VM?
> > > >
> > > > HCR_EL2.VM unsets means the stage-2 will not be used for the EL1/EL0
> > > > translation regime. In the context of the erratum, the AT can still
> > > > speculate
> > > > except it will not take into account the stage-2. The dependencies on
> > > > VMID
> > > > stills applies when HCR_EL2.VM is unset, so from my understanding, the
> > > > entry
> > > > could get cached to whatever is VTTBR_EL2.VMID.
> > >
> > > Damn! Even if at this point of the boot sequence there is no EL1 / EL0
> > > at all? How can that speculation happen? Shouldn't the first EL1 / EL0
> > > speculation occur after the first leave_hypervisor_tail?
> >
> > How do you know EL1 was not run before hand? Imagine we did a soft reboot or
> > kexec Xen...
> >
> > But the speculation in that context is may be because the processor noticed
> > an AT instruction targeting EL1 in the stream.
This seems to be extremely improbable, borderline impossible to me, but
I can imagine that we might want to be extra-paranoid to make sure all
potential issues are covered.
> > > > > Even if speculation happens without HCR_EL2, why do we need to set it
> > > > > now? Isn't setting empty_root_mfn enough?
> > > >
> > > > The main goal here is to have the TLBs in a known state after the CPU
> > > > has been
> > > > initialized. After the sequence below, we are sure that the TLBs don't
> > > > contain
> > > > entries associated to the EL1/EL0 regime
Re: [Xen-devel] [PATCH for-4.12 v2 5/7] xen/arm: p2m: Only use isb() when it is necessary
On Mon, 28 Jan 2019, Julien Grall wrote:
> The EL1 translation regime is out-of-context when running at EL2. This
> means the processor cannot speculate memory accesses using the registers
> associated to that regime.
>
> An isb() is only needed if Xen is going to use the translation regime
> before returning to the guest (exception returns will synchronize the
> context).
>
> Remove unnecessary isb() and document the ones left.
>
> Signed-off-by: Julien Grall
> Reviewed-by: Andrii Anisov
Reviewed-by: Stefano Stabellini
> ---
> Changes in v2:
> - Remove pointless {}
> - Fix typoes
> - Add Andrii's reviewed-by
> ---
> xen/arch/arm/p2m.c | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 9844bfb936..44391a5f8c 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -106,17 +106,21 @@ void p2m_restore_state(struct vcpu *n)
> return;
>
> WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
> -isb();
> -
> WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
> -isb();
> -
> WRITE_SYSREG(n->arch.hcr_el2, HCR_EL2);
> -isb();
>
> last_vcpu_ran = >last_vcpu_ran[smp_processor_id()];
>
> /*
> + * While we are restoring an out-of-context translation regime
> + * we still need to ensure:
> + * - VTTBR_EL2 is synchronized before flushing the TLBs
> + * - All registers for EL1 are synchronized before executing an AT
> + * instructions targeting S1/S2.
> + */
> +isb();
> +
> +/*
> * Flush local TLB for the domain to prevent wrong TLB translation
> * when running multiple vCPU of the same domain on a single pCPU.
> */
> @@ -147,6 +151,7 @@ static void p2m_force_tlb_flush_sync(struct p2m_domain
> *p2m)
> {
> local_irq_save(flags);
> WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
> +/* Ensure VTTBR_EL2 is synchronized before flushing the TLBs */
> isb();
> }
>
> @@ -155,6 +160,7 @@ static void p2m_force_tlb_flush_sync(struct p2m_domain
> *p2m)
> if ( ovttbr != READ_SYSREG64(VTTBR_EL2) )
> {
> WRITE_SYSREG64(ovttbr, VTTBR_EL2);
> +/* Ensure VTTBR_EL2 is back in place before continuing. */
> isb();
> local_irq_restore(flags);
> }
> @@ -1907,7 +1913,6 @@ static uint32_t __read_mostly vtcr;
> static void setup_virt_paging_one(void *data)
> {
> WRITE_SYSREG32(vtcr, VTCR_EL2);
> -isb();
> }
>
> void __init setup_virt_paging(void)
> --
> 2.11.0
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Backport candidate for Arm
On Mon, 28 Jan 2019, Julien Grall wrote:
> Hi,
>
> On 1/26/19 1:30 AM, Stefano Stabellini wrote:
> > On Mon, 21 Jan 2019, Julien Grall wrote:
> > > Hi,
> > >
> > > Ping?
> > >
> > > Cheers,
> > >
> > > On 30/11/2018 17:25, Julien Grall wrote:
> > > > Hi,
> > > >
> > > > Below a list of backport candidate for Arm.
> > > >
> > > >
> > > > For Xen 4.10+ to handle correctly SMC call parameters and result
> > > >
> > > > 35fc608612 xen/arm: smccc-1.1: Make return values unsigned long
> > > > fa7974f743 xen/arm: smccc-1.1: Handle function result as parameters
> > > >
> > > > For Xen 4.9+ to avoid Dom0 crash when using less vCPUs than pCPUs on
> > > > GICv3
> > > >
> > > > 703d9d5ec1 xen/arm: vgic-v3: Delay the initialization of the domain
> > > > information
> > > > 54ec59f6b0 xen/arm: vgic-v3: Don't create empty re-distributor
> > > > regions
> > > >
> > > > The following patch is required in Xen 4.11 to avoid breaking the new
> > > > vGIC
> > > > after applying the 2 previous patches.
> > > >
> > > > 62aa9e7f1b xen/arm: Don't build GICv3 with the new vGIC
> >
> > For the moment I skipped 54ec59f6b0 and 62aa9e7f1b because 62aa9e7f1b is
> > not a trivial backport. Everything else is done.
>
> Thank you for backporting the patches. I think it was a bit odd to apply patch
> 703d9d5ec1 without the 2 patches. Anyway, below a replacement patch for
> 62aa9e7f1b.
>
> diff --cc xen/arch/arm/Kconfig
> index 8174c0c635,581de67b6b..00
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@@ -12,7 -12,6 +12,7 @@@ config ARM_3
> config ARM_64
> def_bool y
> depends on 64BIT
> - select HAS_GICV3
> ++ select HAS_GICV3 if !NEW_VGIC
>
> config ARM
> def_bool y
>
Thank you, and that is fine for 4.11. I did that. However for 4.9 and
4.10 we also need to make significant changes to 54ec59f6b0, or backport
a lot more. I went with the former approach -- does the backport commit
below look good to you for 4.10?
commit 316e4426a185efefa078dd087c89a694b2149be8
Author: Julien Grall
Date: Mon Jan 28 14:54:52 2019 -0800
xen/arm: vgic-v3: Don't create empty re-distributor regions
At the moment, Xen is assuming the hardware domain will have the same
number of re-distributor regions as the host. However, as the
number of CPUs or the stride (e.g on GICv4) may be different we end up
exposing regions which does not contain any re-distributors.
When booting, Linux will go through all the re-distributor region to
check whether a property (e.g vPLIs) is available accross all the
re-distributors. This will result to a data abort on empty regions
because there are no underlying re-distributor.
So we need to limit the number of regions exposed to the hardware
domain. The code reworked to only expose the minimun number of regions
required by the hardware domain. It is assumed the regions will be
populated starting from the first one.
Lastly, rename vgic_v3_rdist_count to reflect the value return by the
helper.
Reported-by: Shameerali Kolothum Thodi
Signed-off-by: Julien Grall
Tested-by: Shameer Kolothum
Reviewed-by: Stefano Stabellini
(cherry picked from commit 54ec59f6b0b363c34cf1864d5214a05e35ea75ee)
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index a0d290b..f8e5354 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1178,6 +1178,8 @@ static int gicv3_make_hwdom_dt_node(const struct domain
*d,
* GIC has two memory regions: Distributor + rdist regions
* CPU interface and virtual cpu interfaces accessesed as System registers
* So cells are created only for Distributor and rdist regions
+ * The hardware domain may not use all the regions. So only copy
+ * what is necessary.
*/
len = len * (d->arch.vgic.nr_regions + 1);
new_cells = xzalloc_bytes(len);
@@ -1416,6 +1418,10 @@ static int gicv3_make_hwdom_madt(const struct domain *d,
u32 offset)
/* Add Generic Redistributor */
size = sizeof(struct acpi_madt_generic_redistributor);
+/*
+ * The hardware domain may not used all the regions. So only copy
+ * what is necessary.
+ */
for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
{
gicr = (struct acpi_madt_generic_redistributor *)(base_ptr +
table_len);
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 6a30b4a..f76b0b2 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1653,7 +1653,11 @@ static int vgic_v3_vcpu_init(struct vcpu *v)
return 0;
}
-static inline unsigned int vgic_v3_rdist_count(struct domain *d)
+/*
+ * Return the maximum number possible of re-distributor regions for
+ * a given domain.
+ */
+static inline unsigned int vgic_v3_max_rdist_count(struct domain *d)
{
return is_hardware_domain(d) ? vgic_v3_hw.nr_rdist_regions :
GUEST_GICV3_RDIST_REGIONS;
@@ -1665,7
Re: [Xen-devel] [PATCH] arch/arm/xen: Remove duplicate header
On Mon, 28 Jan 2019, Boris Ostrovsky wrote: > On 1/28/19 3:29 AM, Oleksandr Andrushchenko wrote: > > +Boris and Juergen who can also help getting it in > > I can put this in but I'd like to have Stefano's ack, this being ARM. The patch is OK. Sorry for not replying earlier, this thread fell off my radar somehow. Acked-by: Stefano Stabellini > > > > On 1/28/19 8:34 AM, Souptick Joarder wrote: > >> On Mon, Jan 14, 2019 at 4:08 PM Oleksandr Andrushchenko > >> wrote: > >>> On 1/7/19 7:37 PM, Souptick Joarder wrote: > Remove duplicate header which is included twice. > > Signed-off-by: Souptick Joarder > >>> Reviewed-by: Oleksandr Andrushchenko > >> Can we get this patch in queue for 5.1 ? > --- > arch/arm/xen/mm.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c > index cb44aa2..e1d44b9 100644 > --- a/arch/arm/xen/mm.c > +++ b/arch/arm/xen/mm.c > @@ -7,7 +7,6 @@ > #include > #include > #include > -#include > #include > #include > > > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 17/17] libxl: require qemu in dom0 even if stubdomain is in use
Until xenconsoled learns how to handle multiple consoles, this is needed
for save/restore support (qemu state is transferred over secondary
consoles).
Additionally, Linux-based stubdomain waits for all the backends to
initialize during boot. Lack of some console backends results in
stubdomain startup timeout.
This is a temporary patch until xenconsoled will be improved.
Signed-off-by: Marek Marczykowski-Górecki
---
tools/libxl/libxl_dm.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index ce65321..0fdc2f8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2438,7 +2438,11 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
}
}
-need_qemu = libxl__need_xenpv_qemu(gc, dm_config);
+/*
+ * Until xenconsoled learns how to handle multiple consoles, require qemu
+ * in dom0, since the above code always adds at least 2 consoles.
+ */
+need_qemu = 1;
for (i = 0; i < num_console; i++) {
libxl__device device;
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 04/17] libxl: Allow running qemu-xen in stubdomain
Do not prohibit anymore using stubdomain with qemu-xen.
To help distingushing MiniOS and Linux stubdomain, add helper inline
functions libxl__stubdomain_is_linux() and
libxl__stubdomain_is_linux_running(). Those should be used where really
the difference is about MiniOS/Linux, not qemu-xen/qemu-xen-traditional.
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v3:
- new patch, instead of "libxl: Add "stubdomain_version" to
domain_build_info"
- helper functions as suggested by Ian Jackson
---
tools/libxl/libxl_create.c | 9 -
tools/libxl/libxl_internal.h | 17 +
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index a4e74a5..bb62542 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -160,15 +160,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
}
}
-if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
-b_info->device_model_version !=
-LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL &&
-libxl_defbool_val(b_info->device_model_stubdomain)) {
-LOG(ERROR,
-"device model stubdomains require \"qemu-xen-traditional\"");
-return ERROR_INVAL;
-}
-
if (!b_info->max_vcpus)
b_info->max_vcpus = 1;
if (!b_info->avail_vcpus.size) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 459f9bf..b8c698a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -2195,6 +2195,23 @@ _hidden int
libxl__device_model_version_running(libxl__gc *gc, uint32_t domid);
/* Return the system-wide default device model */
_hidden libxl_device_model_version libxl__default_device_model(libxl__gc *gc);
+static inline
+bool libxl__stubdomain_is_linux_running(libxl__gc *gc, uint32_t domid)
+{
+/* same logic as in libxl__stubdomain_is_linux */
+return libxl__device_model_version_running(gc, domid)
+== LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
+
+static inline
+bool libxl__stubdomain_is_linux(libxl_domain_build_info *b_info)
+{
+/* right now qemu-tranditional implies MiniOS stubdomain and qemu-xen
+ * implies Linux stubdomain */
+return libxl_defbool_val(b_info->device_model_stubdomain) &&
+b_info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+}
+
#define DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, fmt, _a...) \
libxl__sprintf(gc, "/local/domain/%u/device-model/%u" fmt, dm_domid, \
domid, ##_a)
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 01/17] Document ioemu MiniOS stubdomain protocol
Add documentation based on reverse-engineered toolstack-ioemu stubdomain protocol. Signed-off-by: Marek Marczykowski-Górecki --- docs/misc/stubdom.txt | 53 - 1 file changed, 53 insertions(+) diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt index de7b6c7..4c524f2 100644 --- a/docs/misc/stubdom.txt +++ b/docs/misc/stubdom.txt @@ -23,6 +23,59 @@ and http://wiki.xen.org/wiki/Device_Model_Stub_Domains for more information on device model stub domains +Toolstack to MiniOS ioemu stubdomain protocol +- + +This section describe communication protocol between toolstack and +qemu-traditional running in MiniOS stubdomain. The protocol include +expectations of both qemu and stubdomain itself. + +Setup (done by toolstack, expected by stubdomain): + - Block devices for target domain are connected as PV disks to stubdomain, + according to configuration order, starting with xvda + - Network devices for target domain are connected as PV nics to stubdomain, + according to configuration order, starting with 0 + - if graphics output is expected, VFB and VKB devices are set for stubdomain + (its backend is responsible for exposing them using appropriate protocol + like VNC or Spice) + - other target domain's devices are not connected at this point to stubdomain + (may be hot-plugged later) + - QEMU command line (space separated arguments) is stored in + /vm//image/dmargs xenstore path + - target domain id is stored in /local/domain//target xenstore path +?? - bios type is stored in /local/domain//hvmloader/bios + - stubdomain's console 0 is connected to qemu log file + - stubdomain's console 1 is connected to qemu save file (for saving state) + - stubdomain's console 2 is connected to qemu save file (for restoring state) + - next consoles are connected according to target guest's serial console configuration + +Startup: +1. PV stubdomain is started with ioemu-stubdom.gz kernel and no initrd +2. stubdomain initialize relevant devices +2. stubdoma signal readiness by writing "running" to /local/domain//device-model//state xenstore path +3. now stubdomain is considered running + +Runtime control (hotplug etc): +Toolstack can issue command through xenstore. The sequence is (from toolstack POV): +1. Write parameter to /local/domain//device-model//parameter. +2. Write command to /local/domain//device-model//command. +3. Wait for command result in /local/domain//device-model//state (command specific value). +4. Write "running" back to /local/domain//device-model//state. + +Defined commands: + - "pci-ins" - PCI hot plug, results: + - "pci-inserted" - success + - "pci-insert-failed" - failure + - "pci-rem" - PCI hot remove, results: + - "pci-removed" - success + - ?? + - "save" - save domain state to console 1, results: + - "paused" - success + - "continue" - resume domain execution, after loading state from console 2 (require -loadvm command argument), results: + - "running" - success + + + PV-GRUB === -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 06/17] libxl: write qemu arguments into separate xenstore keys
This allows using arguments with spaces, like -append, without
nominating any special "separator" character.
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v3:
- previous version of this patch "libxl: use \x1b to separate qemu
arguments for linux stubdomain" used specific non-printable
separator, but it was rejected as xenstore doesn't cope well with
non-printable chars
---
tools/libxl/libxl_dm.c | 39 ++-
1 file changed, 38 insertions(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 1fa4fa8..6cfc256 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2047,6 +2047,40 @@ static int
libxl__vfb_and_vkb_from_hvm_guest_config(libxl__gc *gc,
return 0;
}
+static int libxl__write_stub_linux_dmargs(libxl__gc *gc,
+int dm_domid, int guest_domid,
+char **args)
+{
+libxl_ctx *ctx = libxl__gc_owner(gc);
+int i;
+char *vm_path;
+char *path;
+struct xs_permissions roperm[2];
+xs_transaction_t t;
+
+roperm[0].id = 0;
+roperm[0].perms = XS_PERM_NONE;
+roperm[1].id = dm_domid;
+roperm[1].perms = XS_PERM_READ;
+
+vm_path = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/vm",
guest_domid));
+path = GCSPRINTF("%s/image/dmargs", vm_path);
+
+retry_transaction:
+t = xs_transaction_start(ctx->xsh);
+xs_write(ctx->xsh, t, path, "", 0);
+xs_set_permissions(ctx->xsh, t, path, roperm, ARRAY_SIZE(roperm));
+i = 1;
+for (i=1; args[i] != NULL; i++)
+xs_write(ctx->xsh, t, GCSPRINTF("%s/%03d", path, i), args[i],
strlen(args[i]));
+
+xs_set_permissions(ctx->xsh, t, GCSPRINTF("%s/rtc/timeoffset", vm_path),
roperm, ARRAY_SIZE(roperm));
+if (!xs_transaction_end(ctx->xsh, t, 0))
+if (errno == EAGAIN)
+goto retry_transaction;
+return 0;
+}
+
static int libxl__write_stub_dmargs(libxl__gc *gc,
int dm_domid, int guest_domid,
char **args)
@@ -2249,7 +2283,10 @@ void libxl__spawn_stub_dm(libxl__egc *egc,
libxl__stub_dm_spawn_state *sdss)
libxl__store_libxl_entry(gc, guest_domid, "dm-version",
libxl_device_model_version_to_string(dm_config->b_info.device_model_version));
-libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
+if (libxl__stubdomain_is_linux(_config->b_info))
+libxl__write_stub_linux_dmargs(gc, dm_domid, guest_domid, args);
+else
+libxl__write_stub_dmargs(gc, dm_domid, guest_domid, args);
libxl__xs_printf(gc, XBT_NULL,
GCSPRINTF("%s/image/device-model-domid",
libxl__xs_get_dompath(gc, guest_domid)),
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 12/17] libxl: use vchan for QMP access with Linux stubdomain, libxl__ev_qmp_* version
Access to QMP of QEMU in Linux stubdomain is possible over vchan
connection. Add appropriate handling in libxl__ev_qmp_* API, keeping all the
asynchronous properties.
Since only one client can be connected to vchan server at the same time
and it is not enforced by the libxenvchan itself, additional client-side
locking is needed. Note that qemu supports only one simultaneous client
on a control socket anyway (but in UNIX socket case, it enforce it
server-side), so it doesn't add any extra limitation.
Regarding pipe to self:
Vchan issue notification about incoming data (or space for it)
only once - even if there is more data to read, FD returned by
libxenvchan_fd_for_select() will not be readable. Similar to buffer
space - there is one notification if more data can be written, but FD
isn't considered "writable" after libxenvchan_wait() call, even if in
fact there is a buffer space.
There are two situations where it is problematic:
- some QMP message results in a user callback and processing further
messages must stop (even if more data is already available in vchan
buffer)
- data is scheduled to write after a buffer space notification; this
may result from either delayed libxl__ev_qmp_send() call, or internal
state change
To avoid the above problems, use pipe to self to schedule vchan data
processing, even if vchan would not issue notification itself.
Signed-off-by: Marek Marczykowski-Górecki
---
TODO:
- handle locking for vchan access - a lockfile + flock comes to mind,
but there is no async provisioning for it in libxl
Changes in v3:
- new patch, in place of "libxl: access QMP socket via console for
qemu-in-stubdomain"
---
tools/Rules.mk | 4 +-
tools/libxl/Makefile | 2 +-
tools/libxl/libxl_dm.c | 2 +-
tools/libxl/libxl_internal.h | 7 +-
tools/libxl/libxl_qmp.c | 293 +++-
5 files changed, 301 insertions(+), 7 deletions(-)
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 68f2ed7..12b3129 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -187,8 +187,8 @@ SHLIB_libblktapctl =
PKG_CONFIG_REMOVE += xenblktapctl
endif
-CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl)
$(CFLAGS_xeninclude)
-SHDEPS_libxenlight = $(SHLIB_libxenctrl) $(SHLIB_libxenstore)
$(SHLIB_libblktapctl)
+CFLAGS_libxenlight = -I$(XEN_XENLIGHT) $(CFLAGS_libxenctrl)
$(CFLAGS_xeninclude) $(CFLAGS_libxenvchan)
+SHDEPS_libxenlight = $(SHLIB_libxenctrl) $(SHLIB_libxenstore)
$(SHLIB_libblktapctl) $(SHLIB_libxenvchan)
LDLIBS_libxenlight = $(SHDEPS_libxenlight)
$(XEN_XENLIGHT)/libxenlight$(libextension)
SHLIB_libxenlight = $(SHDEPS_libxenlight) -Wl,-rpath-link=$(XEN_XENLIGHT)
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 6da342e..55b0b63 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -24,6 +24,7 @@ LIBXL_LIBS = $(LDLIBS_libxentoollog) $(LDLIBS_libxenevtchn)
$(LDLIBS_libxenctrl)
ifeq ($(CONFIG_LIBNL),y)
LIBXL_LIBS += $(LIBNL3_LIBS)
endif
+LIBXL_LIBS += $(LDLIBS_libxenvchan)
CFLAGS_LIBXL += $(CFLAGS_libxentoollog)
CFLAGS_LIBXL += $(CFLAGS_libxentoolcore)
@@ -35,6 +36,7 @@ CFLAGS_LIBXL += $(CFLAGS_libblktapctl)
ifeq ($(CONFIG_LIBNL),y)
CFLAGS_LIBXL += $(LIBNL3_CFLAGS)
endif
+CFLAGS_LIBXL += $(CFLAGS_libxenvchan)
CFLAGS_LIBXL += -Wshadow
LIBXL_LIBS-$(CONFIG_ARM) += -lfdt
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 6cfc256..b9a53f3 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1180,7 +1180,7 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
"-xen-domid",
GCSPRINTF("%d", guest_domid), NULL);
-/* There is currently no way to access the QMP socket in the stubdom */
+/* QMP access to qemu running in stubdomain is done over vchan, not local
socket */
if (!is_stubdom) {
flexarray_append(dm_args, "-chardev");
if (state->dm_monitor_fd >= 0) {
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 0095835..9a903a5 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -57,6 +57,7 @@
#include
#include
#include
+#include
#include
@@ -509,6 +510,12 @@ struct libxl__ev_qmp {
libxl__carefd *cfd;
libxl__ev_fd efd;
libxl__qmp_state state;
+/* pipe to wake itself if there is more data in vchan */
+libxl__carefd *pipe_cfd_read;
+libxl__carefd *pipe_cfd_write;
+libxl__ev_fd pipe_efd;
+struct libxenvchan *vchan;
+libxl__xswait_state xswait;
int id;
int next_id;/* next id to use */
/* receive buffer */
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index a235095..45b9f74 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1466,6 +1466,14 @@ static void dm_state_saved(libxl__egc *egc,
libxl__ev_qmp *ev,
/* prototypes */
+static int qmp_ev_connect_vchan(libxl__gc *gc, libxl__ev_qmp *ev);
+static
[Xen-devel] [PATCH v3 16/17] libxl: add locking for libvchan QMP connection
It is not safe for multiple clients to (even try to) connect to the same
vchan server at the same time. Contrary to QMP over local socket,
connection over vchan needs external locking. For now use flock() for
this. This is not ideal for async QMP API, as flock() will block the
whole thread while other thread/process talks to the same QEMU instance.
This may be a problem especially in cause of malicious QEMU, that could
stall the communication. But since libxl doesn't have asynchronous
locking primitives, keep flock() until something better can be used
instead.
Note that qemu will handle only one client at a time anyway, so this
does not introduce artificial limit here.
Signed-off-by: Marek Marczykowski-Górecki
---
tools/libxl/libxl_internal.h | 1 +-
tools/libxl/libxl_qmp.c | 58 +++--
2 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9a903a5..36b38fd 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -515,6 +515,7 @@ struct libxl__ev_qmp {
libxl__carefd *pipe_cfd_write;
libxl__ev_fd pipe_efd;
struct libxenvchan *vchan;
+libxl__carefd *vchan_lock;
libxl__xswait_state xswait;
int id;
int next_id;/* next id to use */
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 7643f15..260dc0a 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -114,6 +114,7 @@ typedef struct callback_id_pair {
struct libxl__qmp_handler {
int qmp_fd;
struct libxenvchan *vchan;
+libxl__carefd *vchan_lock;
bool connected;
time_t timeout;
/* wait_for_id will be used by the synchronous send function */
@@ -497,9 +498,10 @@ static void qmp_close(libxl__qmp_handler *qmp)
callback_id_pair *pp = NULL;
callback_id_pair *tmp = NULL;
-if (qmp->vchan)
+if (qmp->vchan) {
libxenvchan_close(qmp->vchan);
-else
+libxl__carefd_close(qmp->vchan_lock);
+} else
close(qmp->qmp_fd);
LIBXL_STAILQ_FOREACH(pp, >callback_list, next) {
free(tmp);
@@ -805,6 +807,40 @@ static void qmp_parameters_add_integer(libxl__gc *gc,
qmp_parameters_common_add(gc, param, name, obj);
}
+static libxl__carefd *qmp_vchan_lock(libxl__gc *gc, int domid)
+{
+libxl__carefd *cfd;
+char *lock_path;
+int fd, r;
+
+lock_path = GCSPRINTF("%s/qmp-libxl-%d.lock", libxl__run_dir_path(),
domid);
+libxl__carefd_begin();
+fd = open(lock_path, O_RDWR | O_CREAT, 0644);
+cfd = libxl__carefd_opened(CTX, fd);
+if (!cfd) {
+LOGED(ERROR, domid, "QMP lock file open error");
+goto error;
+}
+
+/* Try to lock the file, retrying on EINTR */
+for (;;) {
+r = flock(fd, LOCK_EX);
+if (!r)
+break;
+if (errno != EINTR) {
+/* All other errno: EBADF, EINVAL, ENOLCK, EWOULDBLOCK */
+LOGED(ERROR, domid,
+ "unexpected error while trying to lock %s, fd=%d, errno=%d",
+ lock_path, fd, errno);
+goto error;
+}
+}
+return cfd;
+error:
+libxl__carefd_close(cfd);
+return NULL;
+}
+
#define QMP_PARAMETERS_SPRINTF(args, name, format, ...) \
qmp_parameters_add_string(gc, args, name, GCSPRINTF(format, __VA_ARGS__))
@@ -825,10 +861,15 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc,
uint32_t domid)
dm_domid = libxl_get_stubdom_id(CTX, domid);
if (dm_domid) {
qmp_path = DEVICE_MODEL_XS_PATH(gc, dm_domid, domid, "/qmp-vchan");
-/* TODO: add locking */
+qmp->vchan_lock = qmp_vchan_lock(gc, domid);
+if (!qmp->vchan_lock) {
+qmp_free_handler(qmp);
+return NULL;
+}
qmp->vchan = libxenvchan_client_init(CTX->lg, dm_domid, qmp_path);
if (!qmp->vchan) {
LOGED(ERROR, domid, "QMP vchan connection failed: %s",
strerror(errno));
+libxl__carefd_close(qmp->vchan_lock);
qmp_free_handler(qmp);
return NULL;
}
@@ -1741,6 +1782,12 @@ static void qmp_vchan_watch_callback(libxl__egc *egc,
int pipe_fd[2];
if (!rc) {
+/* FIXME: convert to async locking */
+ev->vchan_lock = qmp_vchan_lock(gc, ev->domid);
+if (!ev->vchan_lock) {
+rc= ERROR_FAIL;
+goto error;
+}
ev->vchan = libxenvchan_client_init(CTX->lg, dm_domid,
ev->xswait.path);
if (ev->vchan) {
/* ok */
@@ -1775,6 +1822,8 @@ static void qmp_vchan_watch_callback(libxl__egc *egc,
if (rc)
goto error;
} else if (errno == ENOENT) {
+libxl__carefd_close(ev->vchan_lock);
+ev->vchan_lock = NULL;
/* not ready yet, wait */
return;
} else {
@@ -2425,6 +2474,7 @@ void libxl__ev_qmp_init(libxl__ev_qmp *ev)
ev->pipe_cfd_read =
[Xen-devel] [PATCH v3 07/17] libxl: create vkb device only for guests with graphics output
The forced vkb device is meant for better performance of qemu access
(at least according to ebbd2561b4cefb299f0f68a88b2788504223de18 "libxl:
Add a vkbd frontend/backend pair for HVM guests"), which isn't used if
there is no configured channel to actually access that keyboard.
One can still add vkb device manually if needed.
This is continuation of b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do
not start dom0 qemu for stubdomain when not needed".
Signed-off-by: Marek Marczykowski-Górecki
Reviewed-by: Jason Andryuk
---
tools/libxl/libxl_create.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 13fc304..736b93c 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1439,9 +1439,13 @@ static void domcreate_launch_dm(libxl__egc *egc,
libxl__multidev *multidev,
libxl__device_console_add(gc, domid, , state, );
libxl__device_console_dispose();
-libxl_device_vkb_init();
-libxl__device_add(gc, domid, __vkb_devtype, );
-libxl_device_vkb_dispose();
+if (libxl_defbool_val(d_config->b_info.u.hvm.vnc.enable)
+|| libxl_defbool_val(d_config->b_info.u.hvm.spice.enable)
+|| libxl_defbool_val(d_config->b_info.u.hvm.sdl.enable)) {
+libxl_device_vkb_init();
+libxl__device_add(gc, domid, __vkb_devtype, );
+libxl_device_vkb_dispose();
+}
dcs->sdss.dm.guest_domid = domid;
if (libxl_defbool_val(d_config->b_info.device_model_stubdomain))
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 10/17] libxl: typo fix in comment
Signed-off-by: Marek Marczykowski-Górecki --- tools/libxl/libxl_qmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c index 42c8ab8..a235095 100644 --- a/tools/libxl/libxl_qmp.c +++ b/tools/libxl/libxl_qmp.c @@ -1452,7 +1452,7 @@ static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev, * * - Allowed internal state transition: * disconnected -> connecting - * connection -> capability_negotiation + * connecting -> capability_negotiation * capability_negotiation/connected -> waiting_reply * waiting_reply-> connected * any -> broken -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 03/17] libxl: fix qemu-trad cmdline for no sdl/vnc case
When qemu is running in stubdomain, any attempt to initialize vnc/sdl
there will crash it (on failed attempt to load a keymap from a file). If
vfb is present, all those cases are skipped. But since
b053f0c4c9e533f3d97837cf897eb920b8355ed3 "libxl: do not start dom0 qemu
for stubdomain when not needed" it is possible to create a stubdomain
without vfb and contrary to the comment -vnc none do trigger VNC
initialization code (just skips exposing it externally).
Change the implicit SDL avoiding method to -nographics option, used when
none of SDL or VNC is enabled.
Signed-off-by: Marek Marczykowski-Górecki
Reviewed-by: Jason Andryuk
Acked-by: Ian Jackson
Acked-by: Wei Liu
---
Changes in v2:
- typo in qemu option
Changes in v3:
- add missing { }
---
tools/libxl/libxl_dm.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b245956..3601b14 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -715,14 +715,15 @@ static int libxl__build_device_model_args_old(libxl__gc
*gc,
if (libxl_defbool_val(vnc->findunused)) {
flexarray_append(dm_args, "-vncunused");
}
-} else
+} else if (!sdl) {
/*
* VNC is not enabled by default by qemu-xen-traditional,
- * however passing -vnc none causes SDL to not be
- * (unexpectedly) enabled by default. This is overridden by
- * explicitly passing -sdl below as required.
+ * however skipping -vnc causes SDL to be
+ * (unexpectedly) enabled by default. If undesired, disable graphics at
+ * all.
*/
-flexarray_append_pair(dm_args, "-vnc", "none");
+flexarray_append(dm_args, "-nographic");
+}
if (sdl) {
flexarray_append(dm_args, "-sdl");
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 02/17] Document ioemu Linux stubdomain protocol
Add documentation for upcoming Linux stubdomain for qemu-upstream. Signed-off-by: Marek Marczykowski-Górecki --- docs/misc/stubdom.txt | 50 - 1 file changed, 50 insertions(+) diff --git a/docs/misc/stubdom.txt b/docs/misc/stubdom.txt index 4c524f2..9c94c6b 100644 --- a/docs/misc/stubdom.txt +++ b/docs/misc/stubdom.txt @@ -75,6 +75,56 @@ Defined commands: - "running" - success +Toolstack to Linux ioemu stubdomain protocol + + +This section describe communication protocol between toolstack and +qemu-upstream running in Linux stubdomain. The protocol include +expectations of both stubdomain, and qemu. + +Setup (done by toolstack, expected by stubdomain): + - Block devices for target domain are connected as PV disks to stubdomain, + according to configuration order, starting with xvda + - Network devices for target domain are connected as PV nics to stubdomain, + according to configuration order, starting with 0 + - [not implemented] if graphics output is expected, VFB and VKB devices are set for stubdomain + (its backend is responsible for exposing them using appropriate protocol + like VNC or Spice) + - other target domain's devices are not connected at this point to stubdomain + (may be hot-plugged later) + - QEMU command line is stored in + /vm//image/dmargs xenstore dir, each argument as separate key + in form /vm//image/dmargs/NNN, where NNN is 0-padded argument + number + - target domain id is stored in /local/domain//target xenstore path +?? - bios type is stored in /local/domain//hvmloader/bios + - stubdomain's console 0 is connected to qemu log file + - stubdomain's console 1 is connected to qemu save file (for saving state) + - stubdomain's console 2 is connected to qemu save file (for restoring state) + - next consoles are connected according to target guest's serial console configuration + +Environment exposed by stubdomain to qemu (needed to construct appropriate qemu command line and later interact with qmp): + - target domain's disks are available as /dev/xvd[a-z] + - console 2 (incoming domain state) is connected with FD 3 + - console 1 (saving domain state) is added over QMP to qemu as "fdset-id 1" (done by stubdomain, toolstack doesn't need to care about it) + - nics are connected to relevant stubdomain PV vifs when available (qemu -netdev should specify ifname= explicitly) + +Startup: +1. toolstack starts PV stubdomain with stubdom-linux-kernel kernel and stubdom-linux-initrd initrd +2. stubdomain initialize relevant devices +3. stubdomain starts qemu with requested command line, plus few stubdomain specific ones - including local qmp access options +4. stubdomain starts vchan server on /local/domain//device-model//qmp-vchan, exposing qmp socket to the toolstack +5. qemu signal readiness by writing "running" to /local/domain//device-model//state xenstore path +6. now device model is considered running + +QEMU can be controlled using QMP over vchan at /local/domain//device-model//qmp-vchan. Only one simultaneous connection is supported and toolstack needs to ensure that. + +Limitations: + - PCI passthrough require permissive mode + - only one nic is supported + - at most 26 emulated disks are supported (more are still available as PV disks) + - graphics output (VNC/SDL/Spice) not supported + PV-GRUB === -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 00/17] Add support for qemu-xen runnning in a Linux-based stubdomain.
General idea is to allow freely set device_model_version and device_model_stubdomain_override and choose the right options based on this choice. Also, allow to specific path to stubdomain kernel/ramdisk, for greater flexibility. First two patches add documentation about expected toolstack-stubdomain-qemu interface, both for MiniOS stubdomain and Linux stubdomain. Initial version has no QMP support - in initial patches it is completely disabled, which means no suspend/restore and no PCI passthrough. Later patches add QMP over libvchan connection support. This means libxenlight will be linked with libxenvchan. Ideally talking to stubdomain would be added only to new libxl__qmp_ev* API, which is written from start with the assumption of untrusted qemu. But unfortunately some parts of libxl public API that calls into QMP, are not async-aware and can't use libxl__qmp_ev* API. Example of such API is libxl_domain_unpause(). Because of this, separate patch add support for QMP over vchan also to the old API. There is also a need for external locking access to vchan connection (one server can handle only one client and libvchan does not verify this). Since I haven't found any asynchronous locking primitives in libxl, for now I've added flock() on a lock file, but this also needs to be converted to async version. The actual stubdomain implementation is here: https://github.com/marmarek/qubes-vmm-xen-stubdom-linux (branch for-upstream, tag for-upstream-v3) See readme there for build instructions. Beware: building on Debian is dangerous, as it require installing "dracut", which will remove initramfs-tools. You may end up with broken initrd on your host. Few comments/questions about the stubdomain code: 1. There are extra patches for qemu that are necessary to run it in stubdomain. While it is desirable to upstream them, I think it can be done after merging libxl part. Stubdomain's qemu build will in most cases be separate anyway, to limit qemu's dependencies (so the stubdomain size). 2. By default Linux hvc-xen console frontend is unreliable for data transfer (qemu state save/restore) - it drops data sent faster than client is reading it. To fix it, console device needs to be switched into raw mode (`stty raw /dev/hvc1`). Especially for restoring qemu state it is tricky, as it would need to be done before opening the device, but stty (obviously) needs to open the device first. To solve this problem, for now the repository contains kernel patch which changes the default for all hvc consoles. Again, this isn't practical problem, as the kernel for stubdomain is built separately. But it would be nice to have something working with vanilla kernel. I see those options: - convert it to kernel cmdline parameter (hvc_console_raw=1 ?) - use channels instead of consoles (and on the kernel side change the default to "raw" only for channels); while in theory better design, libxl part will be more complex, as channels can be connected to sockets but not files, so libxl would need to read/write to it exactly when qemu write/read the data, not before/after as it is done now Remaining parts for eliminating dom0's instance of qemu: - do not force QDISK backend for CDROM - multiple consoles support in xenconsoled Changes in v2: - apply review comments by Jason Andryuk Changes in v3: - rework qemu arguments handling (separate xenstore keys, instead of \x1b separator) - add QMP over libvchan, instead of console - add protocol documentation - a lot of minor changes, see individual patches for full changes list - split xenconsoled patches into separate series Cc: Simon Gaiser Cc: Eric Shelton Cc: Ian Jackson Cc: Wei Liu Eric Shelton (1): libxl: Handle Linux stubdomain specific QEMU options. Marek Marczykowski-Górecki (16): Document ioemu MiniOS stubdomain protocol Document ioemu Linux stubdomain protocol libxl: fix qemu-trad cmdline for no sdl/vnc case libxl: Allow running qemu-xen in stubdomain libxl: write qemu arguments into separate xenstore keys libxl: create vkb device only for guests with graphics output xl: add stubdomain related options to xl config parser tools/libvchan: notify server when client is connected libxl: typo fix in comment libxl: move xswait declaration up in libxl_internal.h libxl: use vchan for QMP access with Linux stubdomain, libxl__ev_qmp_* version libxl: use vchan for QMP access with Linux stubdomain, non-async version libxl: add save/restore support for qemu-xen in stubdomain tools: add missing libxenvchan cflags libxl: add locking for libvchan QMP connection libxl: require qemu in dom0 even if stubdomain is in use docs/man/xl.cfg.5.pod.in | 23 +- docs/misc/stubdom.txt| 103 +- tools/Rules.mk | 6 +- tools/libvchan/init.c| 3 +- tools/libxl/Makefile | 2 +- tools/libxl/libxl_create.c | 51 +++- tools/libxl/libxl_dm.c | 241 ++--
[Xen-devel] [PATCH v3 15/17] tools: add missing libxenvchan cflags
libxenvchan.h include xenevtchn.h and xengnttab.h, so applications built with it needs applicable -I in CFLAGS too. Signed-off-by: Marek Marczykowski-Górecki --- tools/Rules.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/Rules.mk b/tools/Rules.mk index 12b3129..a7c6c21 100644 --- a/tools/Rules.mk +++ b/tools/Rules.mk @@ -160,7 +160,7 @@ SHDEPS_libxenstat = $(SHLIB_libxenctrl) $(SHLIB_libxenstore) LDLIBS_libxenstat = $(SHDEPS_libxenstat) $(XEN_LIBXENSTAT)/libxenstat$(libextension) SHLIB_libxenstat = $(SHDEPS_libxenstat) -Wl,-rpath-link=$(XEN_LIBXENSTAT) -CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN) +CFLAGS_libxenvchan = -I$(XEN_LIBVCHAN) $(CFLAGS_libxengnttab) $(CFLAGS_libxenevtchn) SHDEPS_libxenvchan = $(SHLIB_libxentoollog) $(SHLIB_libxenstore) $(SHLIB_libxenevtchn) $(SHLIB_libxengnttab) LDLIBS_libxenvchan = $(SHDEPS_libxenvchan) $(XEN_LIBVCHAN)/libxenvchan$(libextension) SHLIB_libxenvchan = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN) -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 14/17] libxl: add save/restore support for qemu-xen in stubdomain
Rely on a wrapper script in stubdomain to attach FD 3/4 of qemu to
relevant consoles.
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v3:
- adjust for qmp_ev*
- assume specific fdset id in qemu set in stubdomain
---
tools/libxl/libxl_dm.c | 23 +++
tools/libxl/libxl_qmp.c | 25 -
2 files changed, 35 insertions(+), 13 deletions(-)
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index b9a53f3..ce65321 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1715,10 +1715,17 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
}
if (state->saved_state) {
-/* This file descriptor is meant to be used by QEMU */
-*dm_state_fd = open(state->saved_state, O_RDONLY);
-flexarray_append(dm_args, "-incoming");
-flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
+if (is_stubdom) {
+/* Linux stubdomain connects specific FD to STUBDOM_CONSOLE_RESTORE
+ */
+flexarray_append(dm_args, "-incoming");
+flexarray_append(dm_args, "fd:3");
+} else {
+/* This file descriptor is meant to be used by QEMU */
+*dm_state_fd = open(state->saved_state, O_RDONLY);
+flexarray_append(dm_args, "-incoming");
+flexarray_append(dm_args, GCSPRINTF("fd:%d",*dm_state_fd));
+}
}
for (i = 0; b_info->extra && b_info->extra[i] != NULL; i++)
flexarray_append(dm_args, b_info->extra[i]);
@@ -2192,14 +2199,6 @@ void libxl__spawn_stub_dm(libxl__egc *egc,
libxl__stub_dm_spawn_state *sdss)
assert(libxl_defbool_val(guest_config->b_info.device_model_stubdomain));
-if (libxl__stubdomain_is_linux(_config->b_info)) {
-if (d_state->saved_state) {
-LOG(ERROR, "Save/Restore not supported yet with Linux Stubdom.");
-ret = -1;
-goto out;
-}
-}
-
sdss->pvqemu.guest_domid = 0;
libxl_domain_create_info_init(_config->c_info);
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 19ce3ce..7643f15 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -1328,6 +1328,7 @@ static void dm_stopped(libxl__egc *egc, libxl__ev_qmp *ev,
const libxl__json_object *response, int rc);
static void dm_state_fd_ready(libxl__egc *egc, libxl__ev_qmp *ev,
const libxl__json_object *response, int rc);
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int
fdset);
static void dm_state_saved(libxl__egc *egc, libxl__ev_qmp *ev,
const libxl__json_object *response, int rc);
@@ -1360,10 +1361,17 @@ static void dm_stopped(libxl__egc *egc, libxl__ev_qmp
*ev,
EGC_GC;
libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
const char *const filename = dsps->dm_savefile;
+uint32_t dm_domid = libxl_get_stubdom_id(CTX, dsps->domid);
if (rc)
goto error;
+if (dm_domid) {
+/* see Linux stubdom interface in docs/stubdom.txt */
+dm_state_save_to_fdset(egc, ev, 1);
+return;
+}
+
ev->payload_fd = open(filename, O_WRONLY | O_CREAT, 0600);
if (ev->payload_fd < 0) {
LOGED(ERROR, ev->domid,
@@ -1394,7 +1402,6 @@ static void dm_state_fd_ready(libxl__egc *egc,
libxl__ev_qmp *ev,
EGC_GC;
int fdset;
const libxl__json_object *o;
-libxl__json_object *args = NULL;
libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
close(ev->payload_fd);
@@ -1409,6 +1416,21 @@ static void dm_state_fd_ready(libxl__egc *egc,
libxl__ev_qmp *ev,
goto error;
}
fdset = libxl__json_object_get_integer(o);
+dm_state_save_to_fdset(egc, ev, fdset);
+return;
+
+error:
+assert(rc);
+libxl__remove_file(gc, dsps->dm_savefile);
+dsps->callback_device_model_done(egc, dsps, rc);
+}
+
+static void dm_state_save_to_fdset(libxl__egc *egc, libxl__ev_qmp *ev, int
fdset)
+{
+EGC_GC;
+int rc;
+libxl__json_object *args = NULL;
+libxl__domain_suspend_state *dsps = CONTAINER_OF(ev, *dsps, qmp);
ev->callback = dm_state_saved;
@@ -1426,6 +1448,7 @@ static void dm_state_fd_ready(libxl__egc *egc,
libxl__ev_qmp *ev,
error:
assert(rc);
+/* TODO: only in non-stubdom case */
libxl__remove_file(gc, dsps->dm_savefile);
dsps->callback_device_model_done(egc, dsps, rc);
}
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 09/17] tools/libvchan: notify server when client is connected
Let the server know when the client is connected. Otherwise server will notice only when client send some data. This change does not break existing clients, as libvchan user should handle spurious notifications anyway (for example acknowledge of remote side reading the data). Signed-off-by: Marek Marczykowski-Górecki --- I had this patch in Qubes for a long time and totally forgot it wasn't upstream thing... --- tools/libvchan/init.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/libvchan/init.c b/tools/libvchan/init.c index 180833d..50a64c1 100644 --- a/tools/libvchan/init.c +++ b/tools/libvchan/init.c @@ -447,6 +447,9 @@ struct libxenvchan *libxenvchan_client_init(struct xentoollog_logger *logger, ctrl->ring->cli_live = 1; ctrl->ring->srv_notify = VCHAN_NOTIFY_WRITE; +/* wake up the server */ +xenevtchn_notify(ctrl->event, ctrl->event_port); + out: if (xs) xs_daemon_close(xs); -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 08/17] xl: add stubdomain related options to xl config parser
Signed-off-by: Marek Marczykowski-Górecki Reviewed-by: Jason Andryuk --- docs/man/xl.cfg.5.pod.in | 23 +++ tools/xl/xl_parse.c | 7 +++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 3b92f39..f475196 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -2694,10 +2694,25 @@ model which they were installed with. =item B -Override the path to the binary to be used as the device-model. The -binary provided here MUST be consistent with the -B which you have specified. You should not -normally need to specify this option. +Override the path to the binary to be used as the device-model running in +toolstack domain. The binary provided here MUST be consistent with the +B which you have specified. You should not normally need +to specify this option. + +=item B + +Override the path to the kernel image used as device-model stubdomain. +The binary provided here MUST be consistent with the +B which you have specified. +In case of B it is expected to be MiniOS-based stubdomain +image, in case of B it is expected to be Linux-based stubdomain +kernel. + +=item B + +Override the path to the ramdisk image used as device-model stubdomain. +The binary provided here is to be used by a kernel pointed by B. +It is known to be used only by Linux-based stubdomain kernel. =item B diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 352cd21..77e4cf0 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -2502,6 +2502,13 @@ skip_usbdev: xlu_cfg_replace_string(config, "device_model_user", _info->device_model_user, 0); +xlu_cfg_replace_string (config, "stubdomain_kernel", +_info->stubdomain_kernel, 0); +xlu_cfg_replace_string (config, "stubdomain_ramdisk", +_info->stubdomain_ramdisk, 0); +if (!xlu_cfg_get_long (config, "stubdomain_memory", , 0)) +b_info->stubdomain_memkb = l * 1024; + #define parse_extra_args(type)\ e = xlu_cfg_get_list_as_string_list(config, "device_model_args"#type, \ _info->extra##type, 0);\ -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 05/17] libxl: Handle Linux stubdomain specific QEMU options.
From: Eric Shelton
This patch creates an appropriate command line for the QEMU instance
running in a Linux-based stubdomain.
NOTE: a number of items are not currently implemented for Linux-based
stubdomains, such as:
- save/restore
- QMP socket
- graphics output (e.g., VNC)
Signed-off-by: Eric Shelton
Simon:
* fix disk path
* fix cdrom path and "format"
* pass downscript for network interfaces
Signed-off-by: Simon Gaiser
[drop Qubes-specific parts]
Signed-off-by: Marek Marczykowski-Górecki
---
Changes in v2:
- fix serial specified with serial=[ ... ] syntax
- error out on multiple consoles (incompatible with stubdom)
- drop erroneous chunk about cdrom
Changes in v3:
- change to use libxl__stubdomain_is_linux instead of
b_info->stubdomain_version
- drop libxl__stubdomain_version_running, prefer
libxl__stubdomain_is_linux_running introduced by previous patch
- drop ifup/ifdown script - stubdomain will handle that with qemu
events itself
- slightly simplify -serial argument
- add support for multiple serial consoles, do not ignore
b_info.u.serial(_list)
- add error checking for more than 26 emulated disks ("/dev/xvd%c"
format string)
---
tools/libxl/libxl_create.c | 40 +++-
tools/libxl/libxl_dm.c | 176 +---
tools/libxl/libxl_internal.h | 1 +-
tools/libxl/libxl_mem.c | 6 +-
tools/libxl/libxl_types.idl | 3 +-
5 files changed, 171 insertions(+), 55 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index bb62542..13fc304 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -160,6 +160,31 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
}
}
+if (b_info->type == LIBXL_DOMAIN_TYPE_HVM &&
+libxl_defbool_val(b_info->device_model_stubdomain)) {
+if (!b_info->stubdomain_kernel) {
+switch (b_info->device_model_version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+b_info->stubdomain_kernel =
+libxl__abs_path(NOGC, "ioemu-stubdom.gz",
libxl__xenfirmwaredir_path());
+b_info->stubdomain_ramdisk = NULL;
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+b_info->stubdomain_kernel =
+libxl__abs_path(NOGC,
+"stubdom-linux-kernel",
+libxl__xenfirmwaredir_path());
+b_info->stubdomain_ramdisk =
+libxl__abs_path(NOGC,
+"stubdom-linux-rootfs",
+libxl__xenfirmwaredir_path());
+break;
+default:
+abort();
+}
+}
+}
+
if (!b_info->max_vcpus)
b_info->max_vcpus = 1;
if (!b_info->avail_vcpus.size) {
@@ -195,6 +220,17 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
b_info->target_memkb = b_info->max_memkb;
+if (b_info->stubdomain_memkb == LIBXL_MEMKB_DEFAULT) {
+if (libxl_defbool_val(b_info->device_model_stubdomain)) {
+if (libxl__stubdomain_is_linux(b_info))
+b_info->stubdomain_memkb = LIBXL_LINUX_STUBDOM_MEM * 1024;
+else
+b_info->stubdomain_memkb = 28 * 1024; // MiniOS
+} else {
+b_info->stubdomain_memkb = 0; // no stubdomain
+}
+}
+
libxl_defbool_setdefault(_info->claim_mode, false);
libxl_defbool_setdefault(_info->localtime, false);
@@ -1538,7 +1574,9 @@ static void domcreate_devmodel_started(libxl__egc *egc,
if (dcs->sdss.dm.guest_domid) {
if (d_config->b_info.device_model_version
== LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
-libxl__qmp_initializations(gc, domid, d_config);
+if (!libxl_defbool_val(d_config->b_info.device_model_stubdomain)) {
+libxl__qmp_initializations(gc, domid, d_config);
+}
}
}
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 3601b14..1fa4fa8 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1169,6 +1169,7 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
int i, connection, devid;
uint64_t ram_size;
const char *path, *chardev;
+bool is_stubdom = libxl_defbool_val(b_info->device_model_stubdomain);
dm_args = flexarray_make(gc, 16, 1);
dm_envs = flexarray_make(gc, 16, 1);
@@ -1179,30 +1180,33 @@ static int libxl__build_device_model_args_new(libxl__gc
*gc,
"-xen-domid",
GCSPRINTF("%d", guest_domid), NULL);
-flexarray_append(dm_args, "-chardev");
-if (state->dm_monitor_fd >= 0) {
-flexarray_append(dm_args,
-
[Xen-devel] [PATCH v3 13/17] libxl: use vchan for QMP access with Linux stubdomain, non-async version
Access to QMP of QEMU in Linux stubdomain is possible over vchan
connection. Add appropriate handling to synchronous API.
Since only one client can be connected to vchan server at the same time
and it is not enforced by the libxenvchan itself, additional client-side
locking is needed. Note that qemu supports only one simultaneous client
on a control socket anyway (but in UNIX socket case, it enforce it
server-side, so it doesn't add any extra limitation.
Ideally, I woudn't need (or want) this part, and would communicate with
stubdomain only with async API. But some libxl public functions that are
not async-compatible do need to call qmp commands (for example
libxl_domain_unpause). Alternative to this patch, would be return error,
breaking all such functions, and incrementally convert them to async
API.
Signed-off-by: Marek Marczykowski-Górecki
---
Two TODOs here:
- handle locking, similarly to previous patch
- select() qmp_next() can now be triggered by malicious stubdomain
without actually sending any data, which would evade timeout
enforcing (as select() each time is called with full timeout); the
easiest thing to do, would be to re-use timeout value from previous
select() call, but that works only on Linux; any better idea?
Note that even without using vchan, malicious qemu could cause
qmp_next() to wait almost indefinitely - by writing one byte at a
time and never finishing the message
Changes in v3:
- new patch, in place of "libxl: access QMP socket via console for
qemu-in-stubdomain"
---
tools/libxl/libxl_qmp.c | 61 --
1 file changed, 47 insertions(+), 14 deletions(-)
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 45b9f74..19ce3ce 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -113,6 +113,7 @@ typedef struct callback_id_pair {
struct libxl__qmp_handler {
int qmp_fd;
+struct libxenvchan *vchan;
bool connected;
time_t timeout;
/* wait_for_id will be used by the synchronous send function */
@@ -496,7 +497,10 @@ static void qmp_close(libxl__qmp_handler *qmp)
callback_id_pair *pp = NULL;
callback_id_pair *tmp = NULL;
-close(qmp->qmp_fd);
+if (qmp->vchan)
+libxenvchan_close(qmp->vchan);
+else
+close(qmp->qmp_fd);
LIBXL_STAILQ_FOREACH(pp, >callback_list, next) {
free(tmp);
tmp = pp;
@@ -516,7 +520,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
do {
fd_set rfds;
-int ret = 0;
+int ret = 1; /* used when select() is skipped */
struct timeval timeout = {
.tv_sec = qmp->timeout,
.tv_usec = 0,
@@ -525,7 +529,8 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
FD_ZERO();
FD_SET(qmp->qmp_fd, );
-ret = select(qmp->qmp_fd + 1, , NULL, NULL, );
+if (!(qmp->vchan && libxenvchan_data_ready(qmp->vchan)))
+ret = select(qmp->qmp_fd + 1, , NULL, NULL, );
if (ret == 0) {
LOGD(ERROR, qmp->domid, "timeout");
return -1;
@@ -536,7 +541,14 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
return -1;
}
-rd = read(qmp->qmp_fd, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE);
+if (qmp->vchan) {
+libxenvchan_wait(qmp->vchan);
+if (!libxenvchan_data_ready(qmp->vchan))
+continue;
+rd = libxenvchan_read(qmp->vchan, qmp->buffer,
QMP_RECEIVE_BUFFER_SIZE);
+} else {
+rd = read(qmp->qmp_fd, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE);
+}
if (rd == 0) {
LOGD(ERROR, qmp->domid, "Unexpected end of socket");
return -1;
@@ -684,9 +696,15 @@ static int qmp_send(libxl__qmp_handler *qmp,
goto out;
}
-if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
-"QMP command", "QMP socket"))
-goto out;
+if (qmp->vchan) {
+/* vchan->blocking == 1, so no need to wrap it in a loop */
+if (libxenvchan_write(qmp->vchan, buf, strlen(buf)) == -1)
+goto out;
+} else {
+if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, strlen(buf),
+"QMP command", "QMP socket"))
+goto out;
+}
rc = qmp->last_id_used;
out:
@@ -798,19 +816,34 @@ libxl__qmp_handler *libxl__qmp_initialize(libxl__gc *gc,
uint32_t domid)
{
int ret = 0;
libxl__qmp_handler *qmp = NULL;
-char *qmp_socket;
+int dm_domid;
+char *qmp_path;
qmp = qmp_init_handler(gc, domid);
if (!qmp) return NULL;
-qmp_socket = GCSPRINTF("%s/qmp-libxl-%d", libxl__run_dir_path(), domid);
-if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
-LOGED(ERROR, domid, "Connection error");
-qmp_free_handler(qmp);
-return NULL;
+dm_domid =
[Xen-devel] [xen-unstable test] 132504: regressions - FAIL
flight 132504 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/132504/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 16 guest-localmigrate/x10 fail REGR. vs. 132422 Tests which are failing intermittently (not blocking): test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail in 132478 pass in 132504 test-amd64-amd64-examine 4 memdisk-try-append fail pass in 132478 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 132422 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 132422 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 132422 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 132422 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 132422 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 132422 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 132422 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 132422 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 132422 test-amd64-i386-xl-pvshim12 guest-start fail 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 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: xen 3ec62664bdd67dc0c41ff22198c406729b3c87a4 baseline version: xen 08b908ba63dee8bc313983c5e412852cbcbcda85 Last test of basis 132422 2019-01-23 08:09:34 Z5 days Failing since132457 2019-01-24 14:59:50 Z4 days3 attempts Testing same since 132478 2019-01-25 21:18:24 Z2 days2 attempts People who touched revisions under test: Andrew Cooper Andrii Anisov George Dunlap Jan Beulich Julien Grall Roger Pau Monné Wei Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt
Re: [Xen-devel] [PATCH] arch/arm/xen: Remove duplicate header
On 1/28/19 3:29 AM, Oleksandr Andrushchenko wrote: > +Boris and Juergen who can also help getting it in I can put this in but I'd like to have Stefano's ack, this being ARM. -boris > > On 1/28/19 8:34 AM, Souptick Joarder wrote: >> On Mon, Jan 14, 2019 at 4:08 PM Oleksandr Andrushchenko >> wrote: >>> On 1/7/19 7:37 PM, Souptick Joarder wrote: Remove duplicate header which is included twice. Signed-off-by: Souptick Joarder >>> Reviewed-by: Oleksandr Andrushchenko >> Can we get this patch in queue for 5.1 ? --- arch/arm/xen/mm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c index cb44aa2..e1d44b9 100644 --- a/arch/arm/xen/mm.c +++ b/arch/arm/xen/mm.c @@ -7,7 +7,6 @@ #include #include #include -#include #include #include > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/6] libxl: don't try to manipulate json config for stubdomain
On Mon, Jan 28, 2019 at 02:41:15PM +, Wei Liu wrote:
> On Sat, Jan 26, 2019 at 03:31:14AM +0100, Marek Marczykowski-Górecki wrote:
> > Stubdomain do not have it's own config file - its configuration is
> > derived from target domains. Do not try to manipulate it when attaching
> > PCI device.
> >
>
> So if we add the same configuration to stubdom as well, what would
> happen? I guess libxl will try to attach the PCI devices to both the
> stubdom and DomU?
I'm not sure if I understand you here. You mean adding configuration file for
stubdomain, the one managed by
libxl__get_domain_configuration/libxl__set_domain_configuration? In
theory it would work just fine, but in practice I fear all kind of
desynchronization bugs, like adding device to target domain's config,
but not stubdomain's one in some failure handling case. We'd have 4
things to care about:
- attaching device to target domain
- attaching device to stubdomain
- saving device to target domain's config
- saving device to stubdomain's config
Handling all the failure cases properly will become quite complex,
especially if we add async callbacks into the mix.
Since stubdomain config is deterministically build based on target
domian's config, I don't think adding such complexity is a good idea.
> > This bug prevented starting HVM with stubdomain and PCI passthrough
> > device attached.
> >
> > Signed-off-by: Marek Marczykowski-Górecki
> > ---
> > Changes in v3:
> > - skip libxl__dm_check_start too, as stubdomain is guaranteed to be
> >running at this stage already
> > - do not init d_config at all, as it is used only for json manipulation
> > ---
> > tools/libxl/libxl_pci.c | 48 ++
> > 1 file changed, 30 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> > index 1bde537..8d159cf 100644
> > --- a/tools/libxl/libxl_pci.c
> > +++ b/tools/libxl/libxl_pci.c
> > @@ -120,10 +120,14 @@ static int libxl__device_pci_add_xenstore(libxl__gc
> > *gc, uint32_t domid, libxl_d
> > libxl_domain_config d_config;
> > libxl_device_pci pcidev_saved;
> > libxl__domain_userdata_lock *lock = NULL;
> > +bool is_stubdomain = libxl_is_stubdom(CTX, domid, NULL);
> >
> > -libxl_domain_config_init(_config);
> > -libxl_device_pci_init(_saved);
> > -libxl_device_pci_copy(CTX, _saved, pcidev);
> > +/* Stubdomain doesn't have own config. */
> > +if (!is_stubdomain) {
> > +libxl_domain_config_init(_config);
> > +libxl_device_pci_init(_saved);
> > +libxl_device_pci_copy(CTX, _saved, pcidev);
> > +}
> >
> > be_path = libxl__domain_device_backend_path(gc, 0, domid, 0,
> > LIBXL__DEVICE_KIND_PCI);
> > @@ -152,27 +156,33 @@ static int libxl__device_pci_add_xenstore(libxl__gc
> > *gc, uint32_t domid, libxl_d
> > GCNEW(device);
> > libxl__device_from_pcidev(gc, domid, pcidev, device);
> >
> > -lock = libxl__lock_domain_userdata(gc, domid);
> > -if (!lock) {
> > -rc = ERROR_LOCK_FAIL;
> > -goto out;
> > -}
> > +/* Stubdomain config is derived from its target domain, it doesn't have
> > + its own file */
>
> Although comment style isn't specified in CODING_STYLE, I would like to
> fix this to
>
> /*
> * Stubdom ...
> * ... own file.
> */
Ok.
> > +if (!is_stubdomain) {
> > +lock = libxl__lock_domain_userdata(gc, domid);
> > +if (!lock) {
> > +rc = ERROR_LOCK_FAIL;
> > +goto out;
> > +}
> >
> > -rc = libxl__get_domain_configuration(gc, domid, _config);
> > -if (rc) goto out;
> > +rc = libxl__get_domain_configuration(gc, domid, _config);
> > +if (rc) goto out;
> >
> > -device_add_domain_config(gc, _config, __pcidev_devtype,
> > - _saved);
> > +device_add_domain_config(gc, _config, __pcidev_devtype,
> > + _saved);
> >
> > -rc = libxl__dm_check_start(gc, _config, domid);
> > -if (rc) goto out;
> > +rc = libxl__dm_check_start(gc, _config, domid);
> > +if (rc) goto out;
> > +}
> >
> > for (;;) {
> > rc = libxl__xs_transaction_start(gc, );
> > if (rc) goto out;
> >
> > -rc = libxl__set_domain_configuration(gc, domid, _config);
> > -if (rc) goto out;
> > +if (lock) {
> > +rc = libxl__set_domain_configuration(gc, domid, _config);
> > +if (rc) goto out;
> > +}
> >
> > libxl__xs_writev(gc, t, be_path, libxl__xs_kvs_of_flexarray(gc,
> > back));
> >
> > @@ -184,8 +194,10 @@ static int libxl__device_pci_add_xenstore(libxl__gc
> > *gc, uint32_t domid, libxl_d
> > out:
> > libxl__xs_transaction_abort(gc, );
> > if (lock) libxl__unlock_domain_userdata(lock);
> > -libxl_device_pci_dispose(_saved);
> > -
[Xen-devel] [freebsd-master test] 132527: trouble: blocked/broken
flight 132527 freebsd-master real [real] http://logs.test-lab.xenproject.org/osstest/logs/132527/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-freebsd broken build-amd64-freebsd 5 host-install(5)broken REGR. vs. 132473 Tests which did not succeed, but are not blocking: build-amd64-xen-freebsd 1 build-check(1) blocked n/a build-amd64-freebsd-again 1 build-check(1) blocked n/a version targeted for testing: freebsd 5470c3dc0d4237217774d8c5473f83d527d38ddc baseline version: freebsd bb58c22379e9aff2ac4ef105c371f30069fe61ab Last test of basis 132473 2019-01-25 09:19:00 Z3 days Testing same since 132527 2019-01-28 09:19:07 Z0 days1 attempts People who touched revisions under test: avos bz cy emaste gallatin gonzo hselasky jhb kib lwhsu marius mckusick mmel netchild ngie oshogbo pfg se takawata trasz tuexen jobs: build-amd64-freebsd-againblocked build-amd64-freebsd broken build-amd64-xen-freebsd blocked sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job build-amd64-freebsd broken broken-step build-amd64-freebsd host-install(5) Not pushing. (No revision log; it would be 651 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/6] xen/x86: Allow stubdom access to irq created for msi.
On Mon, Jan 28, 2019 at 02:50:00PM +, Wei Liu wrote:
> On Sat, Jan 26, 2019 at 03:31:15AM +0100, Marek Marczykowski-Górecki wrote:
> > From: Simon Gaiser
> >
> > Stubdomains need to be given sufficient privilege over the guest which it
> > provides emulation for in order for PCI passthrough to work correctly.
> > When a HVM domain try to enable MSI, QEMU in stubdomain calls
> > PHYSDEVOP_map_pirq, but later it needs to call XEN_DOMCTL_bind_pt_irq as
> > part of xc_domain_update_msi_irq. Allow for that as part of
> > PHYSDEVOP_map_pirq.
> >
> > This is not needed for PCI INTx, because IRQ in that case is known
> > beforehand and the stubdomain is given permissions over this IRQ by
> > libxl__device_pci_add (there's a do_pci_add against the stubdomain).
> >
> > Based on
> > https://github.com/OpenXT/xenclient-oe/blob/5e0e7304a5a3c75ef01240a1e3673665b2aaf05e/recipes-extended/xen/files/stubdomain-msi-irq-access.patch
> > by Eric Chanudet .
> >
> > Signed-off-by: Simon Gaiser
> > Signed-off-by: Marek Marczykowski-Górecki
> > ---
> > Changes in v3:
> > - extend commit message
> >
> > With this patch, stubdomain will be able to create and map multiple irq
> > (DoS possibility?), as only target domain is validated in practice. Is
> > that ok? If not, what additional limits could be applied here?
> > In INTx case the problem doesn't apply, because toolstack grant access
> > to particular IRQ and no allocation happen on stubdomain request. But in
> > MSI case, it isn't that easy as IRQ number isn't known before (as
> > explained in the commit message).
> > ---
> > xen/arch/x86/irq.c | 23 +++
> > xen/arch/x86/physdev.c | 9 +
> > 2 files changed, 32 insertions(+)
> >
> > diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> > index 8b44d6c..67c67d4 100644
> > --- a/xen/arch/x86/irq.c
> > +++ b/xen/arch/x86/irq.c
> > @@ -2674,6 +2674,21 @@ int allocate_and_map_msi_pirq(struct domain *d, int
> > index, int *pirq_p,
> > {
> > case MAP_PIRQ_TYPE_MULTI_MSI:
> > irq = create_irq(NUMA_NO_NODE);
> > +if ( !(irq < nr_irqs_gsi || irq >= nr_irqs) &&
> > +current->domain->target == d )
> > +{
> > +ret = irq_permit_access(current->domain, irq);
> > +if ( ret ) {
> > +dprintk(XENLOG_G_ERR,
> > +"dom%d: can't grant it's stubdom (%d) access
> > to "
> > +"irq %d for msi: %d!\n",
> > +d->domain_id,
> > +current->domain->domain_id,
> > +irq,
> > +ret);
> > +return ret;
>
> Don't you need to deallocate the irq before returning?
Yes, indeed.
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
signature.asc
Description: PGP signature
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 132511: tolerable all pass - PUSHED
flight 132511 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/132511/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 132469 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 132469 test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 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-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 7c700108d621a09e4595cd09c8d03277d7d5e88c baseline version: libvirt 429f5454d570b5fca4b40338502bea724e31db99 Last test of basis 132469 2019-01-25 05:11:45 Z3 days Failing since132490 2019-01-26 06:39:12 Z2 days2 attempts Testing same since 132511 2019-01-27 19:54:28 Z0 days1 attempts People who touched revisions under test: Eric Blake Ján Tomko Laine Stump Michal Privoznik Roman Bogorodskiy Thomas Huth jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairpass test-amd64-i386-libvirt-pair 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 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/libvirt.git 429f5454d5..7c700108d6 7c700108d621a09e4595cd09c8d03277d7d5e88c -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 132538: tolerable all pass - PUSHED
flight 132538 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/132538/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-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 f19a199281a23725beb73bef61eb8964d8e225ce baseline version: xen 3ec62664bdd67dc0c41ff22198c406729b3c87a4 Last test of basis 132477 2019-01-25 18:00:45 Z3 days Testing same since 132538 2019-01-28 17:00:45 Z0 days1 attempts People who touched revisions under test: Jan Beulich Norbert Manthey jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-amd64-amd64-xl-qemuu-debianhvm-i386 pass 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 3ec62664bd..f19a199281 f19a199281a23725beb73bef61eb8964d8e225ce -> smoke ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] xen-4.12~rc1 in ub1804: no vfb objects in pv domU, bug or feature?
On Sun, Jan 27, 2019 at 02:15:52PM -0800, Pry Mar wrote: > qemu build config: > http://paste.debian.net/plain/1062777/ > > domU startup trace: > http://paste.debian.net/plain/1062768/ > > This release uses qemu-3.0.0 which has a depends on libxentoolcore. > > In xen-4.11.1 with qemu-2.11.2 vfb objects (VNC) always worked in pv > domU. Only now with qemu-3.x is it failing: the domU stops and closes. VNC with a PV guest still works for me, so something else is going on. The output of `xl -vvv` (domU startup trace) looks fine, beside the error that say qemu didn't start. Could you paste also the output of QEMU: at /var/log/xen/qemu-dm-debsid.log (sorry, logs everywhere) > I see they are patching tools/qemu-xen/configure here: > https://lists.xenproject.org/archives/html/xen-devel/2019-01/msg01696.html That patch isn't important. It shouldn't change anything for the qemu-system-* binaries, but things would changes for others, like qemu-img, it wouldn't be linked to xen's libs anymore. Thanks, -- Anthony PERARD ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 3/8] microcode: introduce the global microcode cache
On Mon, Jan 28, 2019 at 03:06:45PM +0800, Chao Gao wrote:
> to replace the current per-cpu cache 'uci->mc'.
>
> Compared to the current per-cpu cache, the benefits of the global
> microcode cache are:
> 1. It reduces the work that need to be done on each CPU. Parsing ucode
> file can be done once on one CPU. Other CPUs needn't parse ucode file.
> Instead, they can find out and load a patch with newer revision from
> the global cache.
> 2. It reduces the memory consumption on a system with many CPU cores.
>
> Two functions, save_patch() and find_patch() are introduced. The
> former adds one given patch to the global cache. The latter gets
> a newer and matched ucode patch from the global cache.
>
> Note that I deliberately avoid touching 'uci->mc' as I am going to
> remove it completely in the next patch.
>
> Signed-off-by: Chao Gao
> ---
> Changes in v5:
> - reword the commit description
> - find_patch() and save_patch() are abstracted into common functions
>with some hooks for AMD and Intel
> ---
> xen/arch/x86/microcode.c| 54 +++
> xen/arch/x86/microcode_amd.c| 94
> ++---
> xen/arch/x86/microcode_intel.c | 71 ---
> xen/include/asm-x86/microcode.h | 13 ++
> 4 files changed, 219 insertions(+), 13 deletions(-)
>
> diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
> index 4163f50..7d5b769 100644
> --- a/xen/arch/x86/microcode.c
> +++ b/xen/arch/x86/microcode.c
> @@ -61,6 +61,8 @@ static struct ucode_mod_blob __initdata ucode_blob;
> */
> static bool_t __initdata ucode_scan;
>
> +static LIST_HEAD(microcode_cache);
> +
> void __init microcode_set_module(unsigned int idx)
> {
> ucode_mod_idx = idx;
> @@ -208,6 +210,58 @@ static void microcode_fini_cpu(unsigned int cpu)
> spin_unlock(_mutex);
> }
>
> +/* Save a ucode patch to the global cache list */
> +bool save_patch(struct microcode_patch *new_patch)
This being a global function likely requires some kind of prefix, I
would suggest microcode_save_patch, the same applies to the find_patch
function below.
> +{
> +struct microcode_patch *microcode_patch;
> +
> +list_for_each_entry(microcode_patch, _cache, list)
I think I'm missing something here, but given the conversation we had
in the previous version of the series [0] I assumed there was only a
single microcode patch that applies to the whole system, and that
there was no need to keep a list?
Because Xen doesn't support running on such mixed systems anyway?
[0] https://lists.xenproject.org/archives/html/xen-devel/2018-12/msg00381.html
> diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
> index 1ed573a..fc35c8d 100644
> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -276,18 +324,24 @@ static int apply_microcode(unsigned int cpu)
> unsigned int val[2];
> unsigned int cpu_num = raw_smp_processor_id();
> struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu_num);
> +struct microcode_intel *mc_intel;
> +struct microcode_patch *patch;
>
> /* We should bind the task to the CPU */
> BUG_ON(cpu_num != cpu);
>
> -if ( uci->mc.mc_intel == NULL )
> +patch = find_patch(cpu);
> +if ( !patch )
> return -EINVAL;
>
> +mc_intel = patch->data;
> +BUG_ON(!mc_intel);
> +
> /* serialize access to the physical write to MSR 0x79 */
> spin_lock_irqsave(_update_lock, flags);
>
> /* write microcode via MSR 0x79 */
> -wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)uci->mc.mc_intel->bits);
> +wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits);
> wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL);
>
> /* As documented in the SDM: Do a CPUID 1 here */
> @@ -298,19 +352,19 @@ static int apply_microcode(unsigned int cpu)
> val[1] = (uint32_t)(msr_content >> 32);
>
> spin_unlock_irqrestore(_update_lock, flags);
> -if ( val[1] != uci->mc.mc_intel->hdr.rev )
> +if ( val[1] != mc_intel->hdr.rev )
> {
> printk(KERN_ERR "microcode: CPU%d update from revision "
> "%#x to %#x failed. Resulting revision is %#x.\n", cpu_num,
> - uci->cpu_sig.rev, uci->mc.mc_intel->hdr.rev, val[1]);
> + uci->cpu_sig.rev, mc_intel->hdr.rev, val[1]);
> return -EIO;
> }
> printk(KERN_INFO "microcode: CPU%d updated from revision "
> "%#x to %#x, date = %04x-%02x-%02x \n",
> cpu_num, uci->cpu_sig.rev, val[1],
> - uci->mc.mc_intel->hdr.date & 0x,
> - uci->mc.mc_intel->hdr.date >> 24,
> - (uci->mc.mc_intel->hdr.date >> 16) & 0xff);
> + mc_intel->hdr.date & 0x,
> + mc_intel->hdr.date >> 24,
> + (mc_intel->hdr.date >> 16) & 0xff);
Nit: while here could you make an union of the date field with it's
format, so that you can print it without having to perform this
shifting and
Re: [Xen-devel] [PATCH v5 2/8] microcode/intel: extend microcode_update_match()
>>> On 28.01.19 at 17:55, wrote:
> On Mon, Jan 28, 2019 at 03:06:44PM +0800, Chao Gao wrote:
>> to a more generic function. Then, this function can compare two given
>> microcodes' signature/revision as well. Comparing two microcodes is
>> used to update the global microcode cache (introduced by the later
>> patches in this series) when a new microcode is given.
>>
>> Signed-off-by: Chao Gao
>> ---
>> Changes in v5:
>> - constify the extended_signature
>> - use named enum type for the return value of microcode_update_match
>> ---
>> xen/arch/x86/microcode_intel.c | 43
>> ++---
>> xen/include/asm-x86/microcode.h | 6 ++
>> 2 files changed, 25 insertions(+), 24 deletions(-)
>>
>> diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
>> index 4f69f4a..1ed573a 100644
>> --- a/xen/arch/x86/microcode_intel.c
>> +++ b/xen/arch/x86/microcode_intel.c
>> @@ -127,14 +127,24 @@ static int collect_cpu_info(unsigned int cpu_num,
> struct cpu_signature *csig)
>> return 0;
>> }
>>
>> -static inline int microcode_update_match(
>> -unsigned int cpu_num, const struct microcode_header_intel *mc_header,
>> -int sig, int pf)
>> +static enum microcode_match_result microcode_update_match(
>> +const void *mc, unsigned int sig, unsigned int pf, unsigned int rev)
>
> Why are you passing this as a void pointer? The only caller is already
> passing this as a mc_header pointer.
>
>> {
>> -struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu_num);
>> +const struct microcode_header_intel *mc_header = mc;
>> +const struct extended_sigtable *ext_header;
>> +const struct extended_signature *ext_sig;
>> +unsigned int i;
>> +
>> +if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
>> +return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
>
> The code above implies that a microcode blob can only have a single
> version of microcode for each model, I guess this is OK and guaranteed
> by Intel?
The containerization is a little different from AMD's. Multiple
different versions can be present in what commonly named
microcode.bin, but each individual blob holds exactly one
version for one family:model:stepping tuple (plus the further
restriction, named "pf" above).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
On 28.01.19 18:54, Julien Grall wrote: On 1/28/19 4:40 PM, Andrii Anisov wrote: On 28.01.19 18:36, Julien Grall wrote: Hold on, CA57 and CA53 are ARMv8 cores. So are you using 32-bit or 64-bit guests? 64-bit guests. 64-bit guest should not have any Set/Way operations unless you are using a very very old Linux. So what is the version of each guest? All of them derived from 4.14.35. There should be no Set/Way in Linux 4.14. So I am a bit confused how can even reach this code. Can you look if there are a Set/Way executed in Linux? And where? You can add some code in arm64/vsysreg.c to track that down. Yep, I'm still looking into details of that stuff. Yet this patch makes the system functional. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 2/8] microcode/intel: extend microcode_update_match()
On Mon, Jan 28, 2019 at 03:06:44PM +0800, Chao Gao wrote:
> to a more generic function. Then, this function can compare two given
> microcodes' signature/revision as well. Comparing two microcodes is
> used to update the global microcode cache (introduced by the later
> patches in this series) when a new microcode is given.
>
> Signed-off-by: Chao Gao
> ---
> Changes in v5:
> - constify the extended_signature
> - use named enum type for the return value of microcode_update_match
> ---
> xen/arch/x86/microcode_intel.c | 43
> ++---
> xen/include/asm-x86/microcode.h | 6 ++
> 2 files changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
> index 4f69f4a..1ed573a 100644
> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -127,14 +127,24 @@ static int collect_cpu_info(unsigned int cpu_num,
> struct cpu_signature *csig)
> return 0;
> }
>
> -static inline int microcode_update_match(
> -unsigned int cpu_num, const struct microcode_header_intel *mc_header,
> -int sig, int pf)
> +static enum microcode_match_result microcode_update_match(
> +const void *mc, unsigned int sig, unsigned int pf, unsigned int rev)
Why are you passing this as a void pointer? The only caller is already
passing this as a mc_header pointer.
> {
> -struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu_num);
> +const struct microcode_header_intel *mc_header = mc;
> +const struct extended_sigtable *ext_header;
> +const struct extended_signature *ext_sig;
> +unsigned int i;
> +
> +if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
> +return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
The code above implies that a microcode blob can only have a single
version of microcode for each model, I guess this is OK and guaranteed
by Intel?
With at least the first comment fixed:
Reviewed-by: Roger Pau Monné
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.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
On 1/28/19 4:40 PM, Andrii Anisov wrote: On 28.01.19 18:36, Julien Grall wrote: Hold on, CA57 and CA53 are ARMv8 cores. So are you using 32-bit or 64-bit guests? 64-bit guests. 64-bit guest should not have any Set/Way operations unless you are using a very very old Linux. So what is the version of each guest? All of them derived from 4.14.35. There should be no Set/Way in Linux 4.14. So I am a bit confused how can even reach this code. Can you look if there are a Set/Way executed in Linux? And where? You can add some code in arm64/vsysreg.c to track that down. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER
On Mon, Jan 28, 2019 at 04:40:59PM +, Andrew Cooper wrote: > Before the cpuid_policy logic came along, %cr4/EFER auditing on migrate-in was > complicated, because at that point no CPUID information had been set for the > guest. Auditing against the host CPUID was better than nothing, but not > ideal. > > Similarly at the time, PVHv1 lacked the "CPUID passed through from hardware" > behaviour with PV guests had, and PVH dom0 had to be special-cased to be able > to boot. > > Order of information in the migration stream is still an issue (hence we still > need to keep the restore parameter to cope with a nested virt corner case in > the %cr4 case), but since Xen 4.9, all domains start with a suitable CPUID > policy, which is a more appropriate upper bound than host_cpuid_policy. > > Finally, reposition the UMIP logic as it is the only row out of order. > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER
>>> On 28.01.19 at 17:40, wrote: > Before the cpuid_policy logic came along, %cr4/EFER auditing on migrate-in was > complicated, because at that point no CPUID information had been set for the > guest. Auditing against the host CPUID was better than nothing, but not > ideal. > > Similarly at the time, PVHv1 lacked the "CPUID passed through from hardware" > behaviour with PV guests had, and PVH dom0 had to be special-cased to be able > to boot. > > Order of information in the migration stream is still an issue (hence we still > need to keep the restore parameter to cope with a nested virt corner case in > the %cr4 case), but since Xen 4.9, all domains start with a suitable CPUID > policy, which is a more appropriate upper bound than host_cpuid_policy. > > Finally, reposition the UMIP logic as it is the only row out of order. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2 for-4.12] x86/hvm: Fix bit checking for CR4 and MSR_EFER
Before the cpuid_policy logic came along, %cr4/EFER auditing on migrate-in was
complicated, because at that point no CPUID information had been set for the
guest. Auditing against the host CPUID was better than nothing, but not
ideal.
Similarly at the time, PVHv1 lacked the "CPUID passed through from hardware"
behaviour with PV guests had, and PVH dom0 had to be special-cased to be able
to boot.
Order of information in the migration stream is still an issue (hence we still
need to keep the restore parameter to cope with a nested virt corner case in
the %cr4 case), but since Xen 4.9, all domains start with a suitable CPUID
policy, which is a more appropriate upper bound than host_cpuid_policy.
Finally, reposition the UMIP logic as it is the only row out of order.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Juergen Gross
I suspect this isn't an issue in practice because there is no way to get
X86_CR4_SMXE set with the logic in this state, but it is a latent bug.
v2:
* Fix hvm_efer_valid() as well.
* Clarify commit message somewhat.
---
xen/arch/x86/hvm/hvm.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 401c4a9..21944e9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -886,12 +886,7 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t
value,
signed int cr0_pg)
{
const struct domain *d = v->domain;
-const struct cpuid_policy *p;
-
-if ( cr0_pg < 0 && !is_hardware_domain(d) )
-p = d->arch.cpuid;
-else
-p = _cpuid_policy;
+const struct cpuid_policy *p = d->arch.cpuid;
if ( value & ~EFER_KNOWN_MASK )
return "Unknown bits set";
@@ -928,14 +923,9 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t
value,
/* These bits in CR4 can be set by the guest. */
unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore)
{
-const struct cpuid_policy *p;
+const struct cpuid_policy *p = d->arch.cpuid;
bool mce, vmxe;
-if ( !restore && !is_hardware_domain(d) )
-p = d->arch.cpuid;
-else
-p = _cpuid_policy;
-
/* Logic broken out simply to aid readability below. */
mce = p->basic.mce || p->basic.mca;
vmxe = p->basic.vmx && (restore || nestedhvm_enabled(d));
@@ -950,13 +940,13 @@ unsigned long hvm_cr4_guest_valid_bits(const struct
domain *d, bool restore)
X86_CR4_PCE|
(p->basic.fxsr? X86_CR4_OSFXSR: 0) |
(p->basic.sse ? X86_CR4_OSXMMEXCPT: 0) |
+(p->feat.umip ? X86_CR4_UMIP : 0) |
(vmxe ? X86_CR4_VMXE : 0) |
(p->feat.fsgsbase ? X86_CR4_FSGSBASE : 0) |
(p->basic.pcid? X86_CR4_PCIDE : 0) |
(p->basic.xsave ? X86_CR4_OSXSAVE : 0) |
(p->feat.smep ? X86_CR4_SMEP : 0) |
(p->feat.smap ? X86_CR4_SMAP : 0) |
-(p->feat.umip ? X86_CR4_UMIP : 0) |
(p->feat.pku ? X86_CR4_PKE : 0));
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 1/8] microcode/intel: remove redundent check against ucode size
On Mon, Jan 28, 2019 at 03:06:43PM +0800, Chao Gao wrote: > This check has been done in microcode_sanity_check(). Needn't do it > again in get_matching_microcode(). > > Signed-off-by: Chao Gao Reviewed-by: Roger Pau Monné 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.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
On 28.01.19 18:36, Julien Grall wrote: Hold on, CA57 and CA53 are ARMv8 cores. So are you using 32-bit or 64-bit guests? 64-bit guests. 64-bit guest should not have any Set/Way operations unless you are using a very very old Linux. So what is the version of each guest? All of them derived from 4.14.35. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
On 1/28/19 4:32 PM, Andrii Anisov wrote: Hello Julien, Actually I was going to send this patch as RFC, but dropped it at the last moment. On 28.01.19 17:55, Julien Grall wrote: This was missed on purpose. Let me explain why. The call to p2m_invalidate_root() arch_domain_creation_finished is called by *all* the domain at boot to try to optimize the set/way case. The check iommu_use_hap_pt in that context is to prevent guest not using Set/Way to become unusable under the IOMMU use-case. In your case, you seem to have a guest OS using set/way and yet sharing the P2M with the IOMMU. You have the choice between: 1) Crashing on IOMMU fault 2) Become very slow and potentially unusable because you now have to go through the full P2M every time you do a Set/Way. 1) was my favored option because Set/Way should really not be used by the guest. It was implemented by courtesy to the guest OS and I would not rely on everything working. Can you explain what is your use-case (OS used, IOMMU, platform...) Yep, I'm nearly finished moving our development setup to XEN 4.12-rc. The platform is R-Car Gen3 (CA57+CA53), Renesas'es IOMMU named IPMMU is utilized for all peripherals assigned to DomD and GPU shared between DomD and DomA. Three OS'es are running on the HW: HW-less Dom0, driver domain DomD with GPU sharing, Android running on PV drivers and GPU sharing. So on system start (DomD booting) I see few translation faults from DomD only assigned peripherals. But then Android starts and there are lot of translation faults from GPU trying access DomA memory. Then GPU FW dies. Hold on, CA57 and CA53 are ARMv8 cores. So are you using 32-bit or 64-bit guests? 64-bit guest should not have any Set/Way operations unless you are using a very very old Linux. So what is the version of each guest? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
Hello Julien, Actually I was going to send this patch as RFC, but dropped it at the last moment. On 28.01.19 17:55, Julien Grall wrote: This was missed on purpose. Let me explain why. The call to p2m_invalidate_root() arch_domain_creation_finished is called by *all* the domain at boot to try to optimize the set/way case. The check iommu_use_hap_pt in that context is to prevent guest not using Set/Way to become unusable under the IOMMU use-case. In your case, you seem to have a guest OS using set/way and yet sharing the P2M with the IOMMU. You have the choice between: 1) Crashing on IOMMU fault 2) Become very slow and potentially unusable because you now have to go through the full P2M every time you do a Set/Way. 1) was my favored option because Set/Way should really not be used by the guest. It was implemented by courtesy to the guest OS and I would not rely on everything working. Can you explain what is your use-case (OS used, IOMMU, platform...) Yep, I'm nearly finished moving our development setup to XEN 4.12-rc. The platform is R-Car Gen3 (CA57+CA53), Renesas'es IOMMU named IPMMU is utilized for all peripherals assigned to DomD and GPU shared between DomD and DomA. Three OS'es are running on the HW: HW-less Dom0, driver domain DomD with GPU sharing, Android running on PV drivers and GPU sharing. So on system start (DomD booting) I see few translation faults from DomD only assigned peripherals. But then Android starts and there are lot of translation faults from GPU trying access DomA memory. Then GPU FW dies. -- Sincerely, Andrii Anisov. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] pvh/dom0: fix deadlock in GSI mapping
>>> On 28.01.19 at 16:52, wrote: > On Mon, Jan 28, 2019 at 08:30:02AM -0700, Jan Beulich wrote: >> >>> On 28.01.19 at 15:22, wrote: >> > In order to solve it move the vioapic_hwdom_map_gsi outside of the >> > locked region in vioapic_write_redirent. vioapic_hwdom_map_gsi will >> > not access any of the vioapic fields, so there's no need to call the >> > function holding the hvm.irq_lock. >> >> True, but you also move the code across a vioapic_deliver() >> invocation. Is that delivery going to work when >> vioapic_hwdom_map_gsi() gets invoked only afterwards? > > Yes, that vioapic_deliver will only get invoked when there's a pending > gsi (hvm_irq->gsi_assert_count[idx] > 0), and that can only happen > once the hardware gsi is bound to dom0 and the hvm.irq_lock has been > released, so that the dpci softirq can increase the gsi assert count. Oh, I had overlooked the -EEXIST early bail from vioapic_hwdom_map_gsi(), wrongly implying this could be a problem with other than the initial write of an RTE. Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12] xen/arm: irq: End cleanly spurious interrupt
no_irq_type handlers are used when an IRQ does not have action attached.
This is useful to detect misconfiguration between the interrupt
controller and the software.
Currently, all the handlers will do nothing on spurious interrupt. This
means if such interrupt is received, the priority of the interrupt will
not be dropped and the processor will lose the ability to receive any
interrupt lower or equal to the priority.
Spurious interrupt can happen while releasing interrupt assigned to
guest (happen during domain destruction). The interaction is roughly
CPU0CPU1
release_guest_irq(A)
spin_lock(>lock)
gic_remove_irq_from_guest
receive IRQ A
spin_lock(>lock)
desc->handler->shutdown()
set_bit(IRQ_DISABLED)
desc->handler = _irq_type
spin_unlock(>lock)
desc->handler->end();
spin_unlock(>lock)
Because the no_irq_type.end callback is implemented as a NOP, CPU1 will
not drop the priority of the interrupt. So the CPU will not be able to
receive any interrupt route to any guest afterwards.
The problem can be prevented by dropping the priority and deactivating
the interrupt via gic_hw_ops->gic_host_irq->end().
Note that, for now, interrupt used by Xen are safe because it is not
using no_irq_type on release.
Signed-off-by: Julien Grall
---
xen/arch/arm/irq.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 4a02cc1eba..c51cf333ce 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -44,7 +44,14 @@ static void ack_none(struct irq_desc *irq)
printk("unexpected IRQ trap at irq %02x\n", irq->irq);
}
-static void end_none(struct irq_desc *irq) { }
+static void end_none(struct irq_desc *irq)
+{
+/*
+ * Still allow a CPU to end an interrupt if we receive a spurious
+ * interrupt. This will prevent the CPU to lose interrupt forever.
+ */
+gic_hw_ops->gic_host_irq_type->end(irq);
+}
hw_irq_controller no_irq_type = {
.typename = "none",
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-next] xen/arm: irq: Don't use _IRQ_PENDING when handling host interrupt
While SPIs are shared between CPU, it is not possible to receive the
same interrupts on a different CPU while the interrupt is in active
state. The deactivation of the interrupt is done at the end of the
handling.
This means the _IRQ_PENDING logic is unecessary on Arm as a same
interrupt can never come up while in the loop. So remove it to
simplify the interrupt handle code.
Signed-off-by: Julien Grall
---
xen/arch/arm/irq.c | 32 ++--
1 file changed, 10 insertions(+), 22 deletions(-)
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index c51cf333ce..3877657a52 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -199,6 +199,7 @@ int request_irq(unsigned int irq, unsigned int irqflags,
void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
{
struct irq_desc *desc = irq_to_desc(irq);
+struct irqaction *action;
perfc_incr(irqs);
@@ -242,35 +243,22 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq,
int is_fiq)
goto out_no_end;
}
-set_bit(_IRQ_PENDING, >status);
-
-/*
- * Since we set PENDING, if another processor is handling a different
- * instance of this same irq, the other processor will take care of it.
- */
-if ( test_bit(_IRQ_DISABLED, >status) ||
- test_bit(_IRQ_INPROGRESS, >status) )
+if ( test_bit(_IRQ_DISABLED, >status) )
goto out;
set_bit(_IRQ_INPROGRESS, >status);
-while ( test_bit(_IRQ_PENDING, >status) )
-{
-struct irqaction *action;
+action = desc->action;
-clear_bit(_IRQ_PENDING, >status);
-action = desc->action;
+spin_unlock_irq(>lock);
-spin_unlock_irq(>lock);
-
-do
-{
-action->handler(irq, action->dev_id, regs);
-action = action->next;
-} while ( action );
+do
+{
+action->handler(irq, action->dev_id, regs);
+action = action->next;
+} while ( action );
-spin_lock_irq(>lock);
-}
+spin_lock_irq(>lock);
clear_bit(_IRQ_INPROGRESS, >status);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 132499: regressions - FAIL
flight 132499 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/132499/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-examine 4 memdisk-try-append fail REGR. vs. 132451 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 132451 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 132451 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 132451 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 132451 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 132451 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 132451 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 132451 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 132451 test-amd64-i386-xl-pvshim12 guest-start fail 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 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-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-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-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: linuxba606975938179b1e893e44e190d0001de8e5262 baseline version: linux30bac164aca750892b93eef350439a0562a68647 Last test of basis 132451 2019-01-24 01:41:40 Z4 days Failing since132470 2019-01-25 07:37:53 Z3 days2 attempts Testing same since 132499 2019-01-26 14:18:03 Z2 days1 attempts People who touched revisions under test: "Yan, Zheng" Adrian Hunter Ajit Pandey Alban Bedel Alex Deucher Alex Deucher Alexander Usyskin Anders Roxell Andreas Fenkart Andrew F. Davis Anthony Wong Arnd Bergmann Atsushi Nemoto b-ak Bo He Borislav Petkov Charles Yeh Chris Wilson Christian Brauner Christian Brauner Christian Lamparter Christophe JAILLET Colin Ian King Curtis Malainey Dan Carpenter Dave Airlie Dexuan Cui Douglas Anderson Faiz Abbas Felipe Balbi Geert Uytterhoeven Greg Kroah-Hartman Gustavo A. R. Silva He Zhe Ilya Dryomov Jacek Anaszewski Jack Pham Jan Kara Jani Nikula Jerome Brunet Joe Perches Joe Thornber Johan Hovold John Hubbard Jordan Crouse Július Milan Kailang Yang Kangjie Lu Karoly Pados Kenneth Feng Kishon Vijay Abraham I
Re: [Xen-devel] [PATCH for-4.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
Hi,
On 1/28/19 3:34 PM, Andrii Anisov wrote:
From: Andrii Anisov
In case if the p2m table is shared to IOMMU, invalidating it turns IOMMU to
translation faults that could be not repaired.
Fixed patch check for the corresponded condition and has a comment for one
introduced p2m_invalidate_root() call, but miss them for another. So put the
`if` and the comment in place.
This was missed on purpose. Let me explain why. The call to
p2m_invalidate_root() arch_domain_creation_finished is called by *all*
the domain at boot to try to optimize the set/way case.
The check iommu_use_hap_pt in that context is to prevent guest not using
Set/Way to become unusable under the IOMMU use-case.
In your case, you seem to have a guest OS using set/way and yet sharing
the P2M with the IOMMU. You have the choice between:
1) Crashing on IOMMU fault
2) Become very slow and potentially unusable because you now have to go
through the full P2M every time you do a Set/Way.
1) was my favored option because Set/Way should really not be used by
the guest. It was implemented by courtesy to the guest OS and I would
not rely on everything working.
Can you explain what is your use-case (OS used, IOMMU, platform...)?
Cheers,
Fixes: 2148a12 ("xen/arm: Track page accessed between batch of Set/Way
operations")
Signed-off-by: Andrii Anisov
---
xen/arch/arm/p2m.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 059a391..2367e09 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1708,8 +1708,13 @@ void p2m_flush_vm(struct vcpu *v)
/*
* Invalidate the p2m to track which page was modified by the guest
* between call of p2m_flush_vm().
+ *
+ * This is only turned when IOMMU is not used or the page-table are
+ * not shared because bit[0] (e.g valid bit) unset will result
+ * IOMMU fault that could be not fixed-up.
*/
-p2m_invalidate_root(p2m);
+if ( !iommu_use_hap_pt(v->domain) )
+p2m_invalidate_root(p2m);
v->arch.need_flush_to_ram = false;
}
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] x86/hvm: Fix hvm_cr4_guest_valid_bits() for PVH dom0
On 28/01/2019 15:50, Jan Beulich wrote: On 28.01.19 at 16:36, wrote: >> On 28/01/2019 15:22, Jan Beulich wrote: >> On 28.01.19 at 14:56, wrote: Before the cpuid_policy logic came along, %cr4 auditing on migrate-in was complicated, because at that point no CPUID information had been set for the guest. Auditing against the host CPUID was better than nothing, but not ideal. Order of information in the migration stream is still an issue (hence we >> still need to keep the restore parameter to cope with a nested virt corner case), but since Xen 4.9, the domain starts with the applicable max policy, which >> is a more appropriate upper bound than the host cpuid policy. This also makes the fix from c/s 9d2efbafb8 obsolete, as not even dom0 >> starts without a policy. >>> While I agree with the change itself, I'm struggling to make a connection >>> from this description to what was actually wrong for PVH Dom0. You >>> mostly talk about migration, which is not relevant do Dom0 as an object >>> (and I don't see a connection to domains being migrated by PVH Dom0). >> The PVH Dom0 angle is simply that it is wrong to audit against the host >> policy. > So I'd appreciate if you could make the connection a little more > explicit. In any event > Reviewed-by: Jan Beulich As noticed by Wei, hvm_efer_valid() suffers from the same issue, but is in a more problematic state. I'll do a combined v2 patch and see about describing things more clearly. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] pvh/dom0: fix deadlock in GSI mapping
On Mon, Jan 28, 2019 at 08:30:02AM -0700, Jan Beulich wrote: > >>> On 28.01.19 at 15:22, wrote: > > In order to solve it move the vioapic_hwdom_map_gsi outside of the > > locked region in vioapic_write_redirent. vioapic_hwdom_map_gsi will > > not access any of the vioapic fields, so there's no need to call the > > function holding the hvm.irq_lock. > > True, but you also move the code across a vioapic_deliver() > invocation. Is that delivery going to work when > vioapic_hwdom_map_gsi() gets invoked only afterwards? Yes, that vioapic_deliver will only get invoked when there's a pending gsi (hvm_irq->gsi_assert_count[idx] > 0), and that can only happen once the hardware gsi is bound to dom0 and the hvm.irq_lock has been released, so that the dpci softirq can increase the gsi assert count. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/AMD: flush TLB after ucode update
On 1/28/19 8:25 AM, Andrew Cooper wrote: > On 28/01/2019 14:19, Jan Beulich wrote: --- a/xen/arch/x86/microcode_amd.c +++ b/xen/arch/x86/microcode_amd.c @@ -218,6 +218,12 @@ static int apply_microcode(unsigned int spin_unlock_irqrestore(_update_lock, flags); +/* + * Experimentally this helps with performance issues on at least certain + * Fam15 models. >>> This is no longer experimental, now that we understand why. How about: >>> >>> "Some processors leave the ucode blob mapping as UC after the update. >>> Flush the mapping to regain normal cacheability" ? >>> >>> That way, its also slightly less cryptic in the code. >> I did consider re-wording the comment, but decided to leave it unchanged, >> for the way you word it not having public proof anywhere (for now at least). >> I'm fine to change the comment, if I can explicit go-ahead from AMD. Brian, >> Suravee? > > Preferably with the amended wording, (but ultimately, as agreed upon > with AMD), Reviewed-by: Andrew Cooper > Likewise, Reviewed-by: Brian Woods ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] x86/hvm: Fix hvm_cr4_guest_valid_bits() for PVH dom0
>>> On 28.01.19 at 16:36, wrote: > On 28/01/2019 15:22, Jan Beulich wrote: > On 28.01.19 at 14:56, wrote: >>> Before the cpuid_policy logic came along, %cr4 auditing on migrate-in was >>> complicated, because at that point no CPUID information had been set for the >>> guest. Auditing against the host CPUID was better than nothing, but not >>> ideal. >>> >>> Order of information in the migration stream is still an issue (hence we > still >>> need to keep the restore parameter to cope with a nested virt corner case), >>> but since Xen 4.9, the domain starts with the applicable max policy, which > is >>> a more appropriate upper bound than the host cpuid policy. >>> >>> This also makes the fix from c/s 9d2efbafb8 obsolete, as not even dom0 > starts >>> without a policy. >> While I agree with the change itself, I'm struggling to make a connection >> from this description to what was actually wrong for PVH Dom0. You >> mostly talk about migration, which is not relevant do Dom0 as an object >> (and I don't see a connection to domains being migrated by PVH Dom0). > > The PVH Dom0 angle is simply that it is wrong to audit against the host > policy. So I'd appreciate if you could make the connection a little more explicit. In any event Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] x86/hvm: Fix hvm_cr4_guest_valid_bits() for PVH dom0
On 28/01/2019 15:22, Jan Beulich wrote: On 28.01.19 at 14:56, wrote: >> Before the cpuid_policy logic came along, %cr4 auditing on migrate-in was >> complicated, because at that point no CPUID information had been set for the >> guest. Auditing against the host CPUID was better than nothing, but not >> ideal. >> >> Order of information in the migration stream is still an issue (hence we >> still >> need to keep the restore parameter to cope with a nested virt corner case), >> but since Xen 4.9, the domain starts with the applicable max policy, which is >> a more appropriate upper bound than the host cpuid policy. >> >> This also makes the fix from c/s 9d2efbafb8 obsolete, as not even dom0 starts >> without a policy. > While I agree with the change itself, I'm struggling to make a connection > from this description to what was actually wrong for PVH Dom0. You > mostly talk about migration, which is not relevant do Dom0 as an object > (and I don't see a connection to domains being migrated by PVH Dom0). The PVH Dom0 angle is simply that it is wrong to audit against the host policy. The logic from c/s 9d2efbafb8 is a vestigial remnant of PVHv1, because HVM domains never had the "pass through host CPUID values" logic which PV guests used to have. Now that all domains have a cpuid policy, this logic is no longer necessary. > >> This ideally wants backporting to Xen 4.9 and later. I don't think there is >> anything we can reasonably do for 4.8 and earlier. > As (per its title) it affects PVH Dom0 only, is this really something > worth backporting? (But perhaps this clarifies itself once I better > understand the relation between title and description. Hmm - perhaps not. As I said, I'm not aware of any differences between the host max policy and the hvm max policy when it comes to the subset of bits selected by this logic. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12] arm/p2m: do not invalidate p2m root if it is shared with IOMMU
From: Andrii Anisov
In case if the p2m table is shared to IOMMU, invalidating it turns IOMMU to
translation faults that could be not repaired.
Fixed patch check for the corresponded condition and has a comment for one
introduced p2m_invalidate_root() call, but miss them for another. So put the
`if` and the comment in place.
Fixes: 2148a12 ("xen/arm: Track page accessed between batch of Set/Way
operations")
Signed-off-by: Andrii Anisov
---
xen/arch/arm/p2m.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 059a391..2367e09 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1708,8 +1708,13 @@ void p2m_flush_vm(struct vcpu *v)
/*
* Invalidate the p2m to track which page was modified by the guest
* between call of p2m_flush_vm().
+ *
+ * This is only turned when IOMMU is not used or the page-table are
+ * not shared because bit[0] (e.g valid bit) unset will result
+ * IOMMU fault that could be not fixed-up.
*/
-p2m_invalidate_root(p2m);
+if ( !iommu_use_hap_pt(v->domain) )
+p2m_invalidate_root(p2m);
v->arch.need_flush_to_ram = false;
}
--
2.7.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] x86/hvm: Fix hvm_cr4_guest_valid_bits() for PVH dom0
On 28/01/2019 15:22, Wei Liu wrote: > On Mon, Jan 28, 2019 at 01:56:29PM +, Andrew Cooper wrote: >> Before the cpuid_policy logic came along, %cr4 auditing on migrate-in was >> complicated, because at that point no CPUID information had been set for the >> guest. Auditing against the host CPUID was better than nothing, but not >> ideal. >> >> Order of information in the migration stream is still an issue (hence we >> still >> need to keep the restore parameter to cope with a nested virt corner case), >> but since Xen 4.9, the domain starts with the applicable max policy, which is >> a more appropriate upper bound than the host cpuid policy. >> >> This also makes the fix from c/s 9d2efbafb8 obsolete, as not even dom0 starts >> without a policy. >> >> Finally, reposition the UMIP logic as it is the only row out of order. >> >> Signed-off-by: Andrew Cooper > It is indeed the case that all x86 domains will get a cpuid policy, > auditing against that makes more sense. But what about code in > hvm_efer_valid? Why didn't you make it work with arch.cpuid? Frankly, because I hadn't spotted it. hvm_efer_valid() is rather harder to reason about with the cr0_pg parameter, as it doubles as a restore boolean. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] pvh/dom0: fix deadlock in GSI mapping
>>> On 28.01.19 at 15:22, wrote: > In order to solve it move the vioapic_hwdom_map_gsi outside of the > locked region in vioapic_write_redirent. vioapic_hwdom_map_gsi will > not access any of the vioapic fields, so there's no need to call the > function holding the hvm.irq_lock. True, but you also move the code across a vioapic_deliver() invocation. Is that delivery going to work when vioapic_hwdom_map_gsi() gets invoked only afterwards? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] x86/hvm: Fix hvm_cr4_guest_valid_bits() for PVH dom0
On Mon, Jan 28, 2019 at 01:56:29PM +, Andrew Cooper wrote: > Before the cpuid_policy logic came along, %cr4 auditing on migrate-in was > complicated, because at that point no CPUID information had been set for the > guest. Auditing against the host CPUID was better than nothing, but not > ideal. > > Order of information in the migration stream is still an issue (hence we still > need to keep the restore parameter to cope with a nested virt corner case), > but since Xen 4.9, the domain starts with the applicable max policy, which is > a more appropriate upper bound than the host cpuid policy. > > This also makes the fix from c/s 9d2efbafb8 obsolete, as not even dom0 starts > without a policy. > > Finally, reposition the UMIP logic as it is the only row out of order. > > Signed-off-by: Andrew Cooper It is indeed the case that all x86 domains will get a cpuid policy, auditing against that makes more sense. But what about code in hvm_efer_valid? Why didn't you make it work with arch.cpuid? Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] pvh/dom0: fix deadlock in GSI mapping
On Mon, Jan 28, 2019 at 03:22:45PM +0100, Roger Pau Monne wrote: > The current GSI mapping code can cause the following deadlock: > > (XEN) *** Dumping CPU0 host state: *** > (XEN) [ Xen-4.12.0-rc x86_64 debug=y Tainted: C ] > [...] > (XEN) Xen call trace: > (XEN)[] vmac.c#_spin_lock_cb+0x32/0x70 > (XEN)[] vmac.c#hvm_gsi_assert+0x2f/0x60 <- pick > hvm.irq_lock > (XEN)[] io.c#hvm_dirq_assist+0xd9/0x130 <- pick > event_lock > (XEN)[] io.c#dpci_softirq+0xdb/0x120 > (XEN)[] softirq.c#__do_softirq+0x46/0xa0 > (XEN)[] domain.c#idle_loop+0x35/0x90 > (XEN) > [...] > (XEN) *** Dumping CPU3 host state: *** > (XEN) [ Xen-4.12.0-rc x86_64 debug=y Tainted: C ] > [...] > (XEN) Xen call trace: > (XEN)[] vmac.c#_spin_lock_cb+0x3d/0x70 > (XEN)[] vmac.c#allocate_and_map_gsi_pirq+0xc8/0x130 <- > pick event_lock > (XEN)[] vioapic.c#vioapic_hwdom_map_gsi+0x80/0x130 > (XEN)[] vioapic.c#vioapic_write_redirent+0x119/0x1c0 <- > pick hvm.irq_lock > (XEN)[] vioapic.c#vioapic_write+0x35/0x40 > (XEN)[] vmac.c#hvm_process_io_intercept+0xd2/0x230 > (XEN)[] vmac.c#hvm_io_intercept+0x22/0x50 > (XEN)[] emulate.c#hvmemul_do_io+0x21b/0x3c0 > (XEN)[] emulate.c#hvmemul_do_io_buffer+0x32/0x70 > (XEN)[] emulate.c#hvmemul_do_mmio_buffer+0x29/0x30 > (XEN)[] emulate.c#hvmemul_phys_mmio_access+0xf9/0x1b0 > (XEN)[] emulate.c#hvmemul_linear_mmio_access+0xf0/0x180 > (XEN)[] emulate.c#hvmemul_linear_mmio_write+0x21/0x30 > (XEN)[] emulate.c#linear_write+0xa2/0x100 > (XEN)[] emulate.c#hvmemul_write+0xb5/0x120 > (XEN)[] vmac.c#x86_emulate+0x132aa/0x149a0 > (XEN)[] vmac.c#x86_emulate_wrapper+0x29/0x70 > (XEN)[] emulate.c#_hvm_emulate_one+0x50/0x140 > (XEN)[] vmac.c#hvm_emulate_one_insn+0x41/0x100 > (XEN)[] guest_4.o#sh_page_fault__guest_4+0x976/0xd30 > (XEN)[] vmac.c#vmx_vmexit_handler+0x949/0xea0 > (XEN)[] vmac.c#vmx_asm_vmexit_handler+0xfa/0x270 > > In order to solve it move the vioapic_hwdom_map_gsi outside of the > locked region in vioapic_write_redirent. vioapic_hwdom_map_gsi will > not access any of the vioapic fields, so there's no need to call the > function holding the hvm.irq_lock. > > Signed-off-by: Roger Pau Monné Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 05/11] common/grant_table: block speculative out-of-bound accesses
>>> On 28.01.19 at 15:45, wrote:
> On 1/23/19 14:37, Jan Beulich wrote:
> On 23.01.19 at 12:51, wrote:
>>> @@ -2223,7 +2231,8 @@ gnttab_transfer(
>>> okay = gnttab_prepare_for_transfer(e, d, gop.ref);
>>> spin_lock(>page_alloc_lock);
>>>
>>> -if ( unlikely(!okay) || unlikely(e->is_dying) )
>>> +/* Make sure this check is not bypassed speculatively */
>>> +if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) )
>>> {
>>> bool_t drop_dom_ref = !domain_adjust_tot_pages(e, -1);
>> What is it that makes this particular if() different from other
>> surrounding ones? In particular the version dependent code (a few
>> lines down from here as well as elsewhere) look to be easily
>> divertable onto the wrong branch, then causing out of bounds
>> speculative accesses due to the different (version dependent)
>> shared entry sizes.
> This check evaluates the variable okay, which indicates whether the
> value of gop.ref is bounded correctly.
How does gop.ref come into play here? The if() above does not use
or update it.
> The next conditional that uses
> code based on a version should be fine, even when being entered
> speculatively with the wrong version setup, as the value of gop.ref is
> correct (i.e. architecturally visible after this lfence) already. As the
> version dependent macros are used, i.e. shared_entry_v1 and
> shared_entry_v2, I do not see a risk why speculative out-of-bound access
> should happen here.
As said - v2 entries are larger than v1 ones. Therefore, if the
processor wrongly speculates along the v2 path, it may use
indexes valid for v1, but beyond the size when scaled by v2
element size (whereas ->shared_raw[], aliased with
->shared_v1[] and ->shared_v2[], was actually set up with v1
element size).
And please don't forget - this subsequent conditional was just an
easy example. What I'm really after is why you modify the if()
above, without there being any array index involved.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 5/6] xen/x86: add PHYSDEVOP_msi_msix_set_enable
On Sat, Jan 26, 2019 at 03:31:16AM +0100, Marek Marczykowski-Górecki wrote:
> Allow device model running in stubdomain to enable/disable MSI(-X),
> bypassing pciback. While pciback is still used to access config space
> from within stubdomain, it refuse to write to
> PCI_MSI_FLAGS_ENABLE/PCI_MSIX_FLAGS_ENABLE in non-permissive mode. Which
> is the right thing to do for PV domain (the main use case for pciback),
> as PV domain should use XEN_PCI_OP_* commands for that. Unfortunately
> those commands are not good for stubdomain use, as they configure MSI in
> dom0's kernel too, which should not happen for HVM domain.
>
> This new physdevop is allowed only for stubdomain controlling the domain
> which own the device.
>
> Signed-off-by: Marek Marczykowski-Górecki
> ---
> Changes in v3:
> - new patch
>
> This is rather RFC. Any suggestions for shorter name? Also, I'm not sure
> if physdev_msi_msix_set_enable.flag is the best name/idea.
I'm bad at naming things, so I will refrain from commenting on this.
>
> Should it be plugged into XSM? Any suggestions how exactly? New
> function with XSM_DM_PRIV default action? Should it get target domain
> only, or also machine_bdf?
It should be hooked into XSM -- other sub-ops already do that.
> ---
> xen/arch/x86/msi.c | 16
> xen/arch/x86/physdev.c | 24
> xen/include/asm-x86/msi.h| 1 +
> xen/include/public/physdev.h | 13 +
> 4 files changed, 54 insertions(+)
>
> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
> index babc414..9ba934c 100644
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -1474,6 +1474,22 @@ int pci_restore_msi_state(struct pci_dev *pdev)
> return 0;
> }
>
> +int msi_msix_set_enable(struct pci_dev *pdev, int flag, int enable)
> +{
> +if ( !current->domain->target || pdev->domain != current->domain->target
> )
> +return -EPERM;
> +
> +switch ( flag ) {
{ should be on a new line.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/6] xen/x86: Allow stubdom access to irq created for msi.
On Sat, Jan 26, 2019 at 03:31:15AM +0100, Marek Marczykowski-Górecki wrote:
> From: Simon Gaiser
>
> Stubdomains need to be given sufficient privilege over the guest which it
> provides emulation for in order for PCI passthrough to work correctly.
> When a HVM domain try to enable MSI, QEMU in stubdomain calls
> PHYSDEVOP_map_pirq, but later it needs to call XEN_DOMCTL_bind_pt_irq as
> part of xc_domain_update_msi_irq. Allow for that as part of
> PHYSDEVOP_map_pirq.
>
> This is not needed for PCI INTx, because IRQ in that case is known
> beforehand and the stubdomain is given permissions over this IRQ by
> libxl__device_pci_add (there's a do_pci_add against the stubdomain).
>
> Based on
> https://github.com/OpenXT/xenclient-oe/blob/5e0e7304a5a3c75ef01240a1e3673665b2aaf05e/recipes-extended/xen/files/stubdomain-msi-irq-access.patch
> by Eric Chanudet .
>
> Signed-off-by: Simon Gaiser
> Signed-off-by: Marek Marczykowski-Górecki
> ---
> Changes in v3:
> - extend commit message
>
> With this patch, stubdomain will be able to create and map multiple irq
> (DoS possibility?), as only target domain is validated in practice. Is
> that ok? If not, what additional limits could be applied here?
> In INTx case the problem doesn't apply, because toolstack grant access
> to particular IRQ and no allocation happen on stubdomain request. But in
> MSI case, it isn't that easy as IRQ number isn't known before (as
> explained in the commit message).
> ---
> xen/arch/x86/irq.c | 23 +++
> xen/arch/x86/physdev.c | 9 +
> 2 files changed, 32 insertions(+)
>
> diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
> index 8b44d6c..67c67d4 100644
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -2674,6 +2674,21 @@ int allocate_and_map_msi_pirq(struct domain *d, int
> index, int *pirq_p,
> {
> case MAP_PIRQ_TYPE_MULTI_MSI:
> irq = create_irq(NUMA_NO_NODE);
> +if ( !(irq < nr_irqs_gsi || irq >= nr_irqs) &&
> +current->domain->target == d )
> +{
> +ret = irq_permit_access(current->domain, irq);
> +if ( ret ) {
> +dprintk(XENLOG_G_ERR,
> +"dom%d: can't grant it's stubdom (%d) access to "
> +"irq %d for msi: %d!\n",
> +d->domain_id,
> +current->domain->domain_id,
> +irq,
> +ret);
> +return ret;
Don't you need to deallocate the irq before returning?
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 05/11] common/grant_table: block speculative out-of-bound accesses
On 1/23/19 14:37, Jan Beulich wrote:
On 23.01.19 at 12:51, wrote:
>> @@ -1268,7 +1272,8 @@ unmap_common(
>> }
>>
>> smp_rmb();
>> -map = _entry(lgt, op->handle);
>> +map = _entry(lgt, array_index_nospec(op->handle,
>> + lgt->maptrack_limit));
> It might be better to move this into maptrack_entry() itself, or
> make a maptrack_entry_nospec() clone (as several but not all
> uses may indeed not be in need of the extra protection). At
> least the ones in steal_maptrack_handle() and
> put_maptrack_handle() also look potentially suspicious.
I will move the nospec protection into the macro. I would like to avoid
introducing yet another macro.
>
>> @@ -2223,7 +2231,8 @@ gnttab_transfer(
>> okay = gnttab_prepare_for_transfer(e, d, gop.ref);
>> spin_lock(>page_alloc_lock);
>>
>> -if ( unlikely(!okay) || unlikely(e->is_dying) )
>> +/* Make sure this check is not bypassed speculatively */
>> +if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) )
>> {
>> bool_t drop_dom_ref = !domain_adjust_tot_pages(e, -1);
> What is it that makes this particular if() different from other
> surrounding ones? In particular the version dependent code (a few
> lines down from here as well as elsewhere) look to be easily
> divertable onto the wrong branch, then causing out of bounds
> speculative accesses due to the different (version dependent)
> shared entry sizes.
This check evaluates the variable okay, which indicates whether the
value of gop.ref is bounded correctly. The next conditional that uses
code based on a version should be fine, even when being entered
speculatively with the wrong version setup, as the value of gop.ref is
correct (i.e. architecturally visible after this lfence) already. As the
version dependent macros are used, i.e. shared_entry_v1 and
shared_entry_v2, I do not see a risk why speculative out-of-bound access
should happen here.
>
>> @@ -3215,6 +3230,10 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b)
>> if ( ref_a == ref_b )
>> goto out;
>>
>> +/* Make sure the above check is not bypassed speculatively */
>> +ref_a = array_index_nospec(ref_a, nr_grant_entries(d->grant_table));
>> +ref_b = array_index_nospec(ref_b, nr_grant_entries(d->grant_table));
> I think this wants to move up ahead of the if() in context, and the
> comment be changed to plural.
I will move the code above the comparison.
>
>> --- a/xen/include/xen/nospec.h
>> +++ b/xen/include/xen/nospec.h
>> @@ -87,6 +87,15 @@ static inline bool lfence_true(void) { return true; }
>> #define evaluate_nospec(condition) ({ bool res = (condition); rmb(); res;
>> })
>> #endif
>>
>> +/*
>> + * allow to block speculative execution in generic code
>> + */
> Comment style again.
I will fix the comment.
>
>> +#ifdef CONFIG_X86
>> +#define block_speculation() rmb()
>> +#else
>> +#define block_speculation()
>> +#endif
> Why does this not simply resolve to what currently is named lfence_true()
> (perhaps with a cast to void)? And why does this not depend on the
> Kconfig setting?
I will update the definition of this macro to what is called
lfence_true() in this series, and cast it to void. I will furthermore
split the introduction of this macro and this commit.
Best,
Norbert
>
> Jan
>
>
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 6/6] tools/libxc: add wrapper for PHYSDEVOP_msi_msix_set_enable
On Sat, Jan 26, 2019 at 03:31:17AM +0100, Marek Marczykowski-Górecki wrote: > Add libxc wrapper for PHYSDEVOP_msi_msix_set_enable introduced in > previous commit. > > Signed-off-by: Marek Marczykowski-Górecki Assuming the addition of physdev ops is accepted: Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 3/6] libxl: don't try to manipulate json config for stubdomain
On Sat, Jan 26, 2019 at 03:31:14AM +0100, Marek Marczykowski-Górecki wrote:
> Stubdomain do not have it's own config file - its configuration is
> derived from target domains. Do not try to manipulate it when attaching
> PCI device.
>
So if we add the same configuration to stubdom as well, what would
happen? I guess libxl will try to attach the PCI devices to both the
stubdom and DomU?
> This bug prevented starting HVM with stubdomain and PCI passthrough
> device attached.
>
> Signed-off-by: Marek Marczykowski-Górecki
> ---
> Changes in v3:
> - skip libxl__dm_check_start too, as stubdomain is guaranteed to be
>running at this stage already
> - do not init d_config at all, as it is used only for json manipulation
> ---
> tools/libxl/libxl_pci.c | 48 ++
> 1 file changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 1bde537..8d159cf 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -120,10 +120,14 @@ static int libxl__device_pci_add_xenstore(libxl__gc
> *gc, uint32_t domid, libxl_d
> libxl_domain_config d_config;
> libxl_device_pci pcidev_saved;
> libxl__domain_userdata_lock *lock = NULL;
> +bool is_stubdomain = libxl_is_stubdom(CTX, domid, NULL);
>
> -libxl_domain_config_init(_config);
> -libxl_device_pci_init(_saved);
> -libxl_device_pci_copy(CTX, _saved, pcidev);
> +/* Stubdomain doesn't have own config. */
> +if (!is_stubdomain) {
> +libxl_domain_config_init(_config);
> +libxl_device_pci_init(_saved);
> +libxl_device_pci_copy(CTX, _saved, pcidev);
> +}
>
> be_path = libxl__domain_device_backend_path(gc, 0, domid, 0,
> LIBXL__DEVICE_KIND_PCI);
> @@ -152,27 +156,33 @@ static int libxl__device_pci_add_xenstore(libxl__gc
> *gc, uint32_t domid, libxl_d
> GCNEW(device);
> libxl__device_from_pcidev(gc, domid, pcidev, device);
>
> -lock = libxl__lock_domain_userdata(gc, domid);
> -if (!lock) {
> -rc = ERROR_LOCK_FAIL;
> -goto out;
> -}
> +/* Stubdomain config is derived from its target domain, it doesn't have
> + its own file */
Although comment style isn't specified in CODING_STYLE, I would like to
fix this to
/*
* Stubdom ...
* ... own file.
*/
> +if (!is_stubdomain) {
> +lock = libxl__lock_domain_userdata(gc, domid);
> +if (!lock) {
> +rc = ERROR_LOCK_FAIL;
> +goto out;
> +}
>
> -rc = libxl__get_domain_configuration(gc, domid, _config);
> -if (rc) goto out;
> +rc = libxl__get_domain_configuration(gc, domid, _config);
> +if (rc) goto out;
>
> -device_add_domain_config(gc, _config, __pcidev_devtype,
> - _saved);
> +device_add_domain_config(gc, _config, __pcidev_devtype,
> + _saved);
>
> -rc = libxl__dm_check_start(gc, _config, domid);
> -if (rc) goto out;
> +rc = libxl__dm_check_start(gc, _config, domid);
> +if (rc) goto out;
> +}
>
> for (;;) {
> rc = libxl__xs_transaction_start(gc, );
> if (rc) goto out;
>
> -rc = libxl__set_domain_configuration(gc, domid, _config);
> -if (rc) goto out;
> +if (lock) {
> +rc = libxl__set_domain_configuration(gc, domid, _config);
> +if (rc) goto out;
> +}
>
> libxl__xs_writev(gc, t, be_path, libxl__xs_kvs_of_flexarray(gc,
> back));
>
> @@ -184,8 +194,10 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
> uint32_t domid, libxl_d
> out:
> libxl__xs_transaction_abort(gc, );
> if (lock) libxl__unlock_domain_userdata(lock);
> -libxl_device_pci_dispose(_saved);
> -libxl_domain_config_dispose(_config);
> +if (!is_stubdomain) {
> +libxl_device_pci_dispose(_saved);
> +libxl_domain_config_dispose(_config);
> +}
> return rc;
> }
>
> --
> git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/AMD: flush TLB after ucode update
On 28/01/2019 14:19, Jan Beulich wrote: >>> --- a/xen/arch/x86/microcode_amd.c >>> +++ b/xen/arch/x86/microcode_amd.c >>> @@ -218,6 +218,12 @@ static int apply_microcode(unsigned int >>> >>> spin_unlock_irqrestore(_update_lock, flags); >>> >>> +/* >>> + * Experimentally this helps with performance issues on at least >>> certain >>> + * Fam15 models. >> This is no longer experimental, now that we understand why. How about: >> >> "Some processors leave the ucode blob mapping as UC after the update. >> Flush the mapping to regain normal cacheability" ? >> >> That way, its also slightly less cryptic in the code. > I did consider re-wording the comment, but decided to leave it unchanged, > for the way you word it not having public proof anywhere (for now at least). > I'm fine to change the comment, if I can explicit go-ahead from AMD. Brian, > Suravee? Preferably with the amended wording, (but ultimately, as agreed upon with AMD), Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/6] libxl: do not attach xen-pciback to HVM domain, if stubdomain is in use
On Sat, Jan 26, 2019 at 03:31:12AM +0100, Marek Marczykowski-Górecki wrote: > HVM domains use IOMMU and device model assistance for communicating with > PCI devices, xen-pcifront/pciback isn't directly needed by HVM domain. > But pciback serve also second function - it reset the device when it is > deassigned from the guest and for this reason pciback needs to be used > with HVM domain too. > When HVM domain has device model in stubdomain, attaching xen-pciback to > the target domain itself may prevent attaching xen-pciback to the > (PV) stubdomain, effectively breaking PCI passthrough. > > Fix this by attaching pciback only to one domain: if PV stubdomain is in > use, let it be stubdomain (the commit prevents attaching device to target > HVM in this case); otherwise, attach it to the target domain. > > Signed-off-by: Marek Marczykowski-Górecki Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/6] libxl: attach PCI device to qemu only after setting pciback/pcifront
On Sat, Jan 26, 2019 at 03:31:13AM +0100, Marek Marczykowski-Górecki wrote: > When qemu is running in stubdomain, handling "pci-ins" command will fail > if pcifront is not initialized already. Fix this by sending such command > only after confirming that pciback/front is running. > > Signed-off-by: Marek Marczykowski-Górecki Acked-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/AMD: flush TLB after ucode update
>>> On 28.01.19 at 12:40, wrote: > On 28/01/2019 09:51, Jan Beulich wrote: >> The increased number of messages (spec_ctrl.c:print_details()) within a >> certain time window made me notice some slowness of boot time screen >> output. Experimentally I've narrowed the time window to be from >> immediately after the early ucode update on the BSP to the PAT write in >> cpu_init(), which upon further investigation has an effect because of >> the full TLB flush that's implied by that write. >> >> For that reason, as a workaround, flush the TLB of the mapping of the >> page that holds the blob. Note that flushing just a single page is >> sufficient: As per verify_patch_size() patch size can't exceed 4k, and >> the way xmalloc() works the blob can't be crossing a page boundary. >> >> Signed-off-by: Jan Beulich > > I think it is worth at least noting that we are expecting a BKGD/PPR > update to this effect in due course, even if this doesn't end up in the > commit message. To be honest I wasn't sure whether I should say anything like this. >> --- a/xen/arch/x86/microcode_amd.c >> +++ b/xen/arch/x86/microcode_amd.c >> @@ -218,6 +218,12 @@ static int apply_microcode(unsigned int >> >> spin_unlock_irqrestore(_update_lock, flags); >> >> +/* >> + * Experimentally this helps with performance issues on at least certain >> + * Fam15 models. > > This is no longer experimental, now that we understand why. How about: > > "Some processors leave the ucode blob mapping as UC after the update. > Flush the mapping to regain normal cacheability" ? > > That way, its also slightly less cryptic in the code. I did consider re-wording the comment, but decided to leave it unchanged, for the way you word it not having public proof anywhere (for now at least). I'm fine to change the comment, if I can explicit go-ahead from AMD. Brian, Suravee? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] SpectreV1+L1TF Patch Series
On 1/24/19 22:05, Andrew Cooper wrote: > On 23/01/2019 11:51, Norbert Manthey wrote: >> Dear all, >> >> This patch series attempts to mitigate the issue that have been raised in the >> XSA-289 (https://xenbits.xen.org/xsa/advisory-289.html). To block speculative >> execution on Intel hardware, an lfence instruction is required to make sure >> that selected checks are not bypassed. Speculative out-of-bound accesses can >> be prevented by using the array_index_nospec macro. >> >> The lfence instruction should be added on x86 platforms only. To not affect >> platforms that are not affected by the L1TF vulnerability, the lfence >> instruction is patched in via alternative patching on Intel CPUs only. >> Furthermore, the compile time configuration allows to choose how to protect >> the >> evaluation of conditions with the lfence instruction. > Hello, > > First of all, I've dusted off an old patch of mine and made it > speculatively safe. > > https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=9e92acf1b752dfdfb294234b32d1fa9f55bfdc0f > > Using the new domain_vcpu() helper should tidy up quite a few patches in > the series. I will use the introduced function and apply it where I touched code, thanks! > > > Next, to the ordering of patches. > > Please introduce the Kconfig variable(s) first. I'll follow up on that > thread about options. I will drop the Kconfig option and go with "protect both branches" only. > > Next, introduce a new synthetic feature bit to cause patching to occur, > and logic to trigger it in appropriate circumstances. Look through the > history of include/asm-x86/cpufeatures.h to see some examples from the > previous speculative mitigation work. In particular, you'll need a > command line parameter to control the use of this functionality when it > is compiled in. I will introduce a synthesized feature, and a command line option, and add documentation. > > Next, introduce eval_nospec(). To avoid interfering with other > architectures, you probably want something like this: Do you want me to introduce the new macro in a separate commit, and use it in follow up commits? I have been told previously to not split introduced functions from their use cases, but merge them with at least one. Your above commit again only introduces an at this point unused function. Is there a Xen specifc style rule for this? > xen/nospec.h contains: > > /* > * Evaluate a condition in a speculation-safe way. > * Stub implementation for builds which don't care. > */ > #ifndef eval_nospec > #define eval_nospec(x) (x) > #endif > > and something containing x86's implementation. TBH, I personally think > asm/nospec.h is overdue for introducing now. For now, I would like to not introduce new files, as Jan also suggested earlier. Best, Norbert > > ~Andrew Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12] x86/hvm: Fix hvm_cr4_guest_valid_bits() for PVH dom0
Before the cpuid_policy logic came along, %cr4 auditing on migrate-in was
complicated, because at that point no CPUID information had been set for the
guest. Auditing against the host CPUID was better than nothing, but not
ideal.
Order of information in the migration stream is still an issue (hence we still
need to keep the restore parameter to cope with a nested virt corner case),
but since Xen 4.9, the domain starts with the applicable max policy, which is
a more appropriate upper bound than the host cpuid policy.
This also makes the fix from c/s 9d2efbafb8 obsolete, as not even dom0 starts
without a policy.
Finally, reposition the UMIP logic as it is the only row out of order.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
CC: Juergen Gross
I suspect this isn't an issue in practice because there is no way to get
X86_CR4_SMXE set with the logic in this state, but it is a latent bug.
This ideally wants backporting to Xen 4.9 and later. I don't think there is
anything we can reasonably do for 4.8 and earlier.
I should also note that the nested virt corner case probably isn't useful.
Noone ever bothered wiring the VMXON/VMCS pointer registers into the migration
stream, so migrating a VM will cause it to lose all configured state.
---
xen/arch/x86/hvm/hvm.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 401c4a9..1966fd4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -928,14 +928,9 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t
value,
/* These bits in CR4 can be set by the guest. */
unsigned long hvm_cr4_guest_valid_bits(const struct domain *d, bool restore)
{
-const struct cpuid_policy *p;
+const struct cpuid_policy *p = d->arch.cpuid;
bool mce, vmxe;
-if ( !restore && !is_hardware_domain(d) )
-p = d->arch.cpuid;
-else
-p = _cpuid_policy;
-
/* Logic broken out simply to aid readability below. */
mce = p->basic.mce || p->basic.mca;
vmxe = p->basic.vmx && (restore || nestedhvm_enabled(d));
@@ -950,13 +945,13 @@ unsigned long hvm_cr4_guest_valid_bits(const struct
domain *d, bool restore)
X86_CR4_PCE|
(p->basic.fxsr? X86_CR4_OSFXSR: 0) |
(p->basic.sse ? X86_CR4_OSXMMEXCPT: 0) |
+(p->feat.umip ? X86_CR4_UMIP : 0) |
(vmxe ? X86_CR4_VMXE : 0) |
(p->feat.fsgsbase ? X86_CR4_FSGSBASE : 0) |
(p->basic.pcid? X86_CR4_PCIDE : 0) |
(p->basic.xsave ? X86_CR4_OSXSAVE : 0) |
(p->feat.smep ? X86_CR4_SMEP : 0) |
(p->feat.smap ? X86_CR4_SMAP : 0) |
-(p->feat.umip ? X86_CR4_UMIP : 0) |
(p->feat.pku ? X86_CR4_PKE : 0));
}
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2.1 2/2] x86emul: fix test harness and fuzzer build dependencies
On Fri, Jan 25, 2019 at 12:34:18AM -0700, Jan Beulich wrote:
> Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
> userspace test harnesses") didn't account for the dependencies of
> cpuid-autogen.h to potentially change between incremental builds.
> In particular the harness has a "run" goal which is supposed to be
> usable independently of the rest of the tools sub-tree building, and
> both the harness and the fuzzer code are also supposed to be buildable
> independently. Therefore they need to recursively invoke make to re-
> build the generated header if needed, but only when these rules did not
> get invoked recursively themselves.
>
> Finally cpuid.o did not have any dependencies added for it.
>
> Signed-off-by: Jan Beulich
> Acked-by: Andrew Cooper
> ---
> v2.1: Split controversial parts from (hopefully) non-controversial ones.
> v2: Guard $(MAKE) invocations by $(MAKELEVEL) checks.
>
> --- a/tools/fuzz/x86_instruction_emulator/Makefile
> +++ b/tools/fuzz/x86_instruction_emulator/Makefile
> @@ -26,13 +26,15 @@ GCOV_FLAGS := --coverage
> $(CC) -c $(CFLAGS) $(GCOV_FLAGS) $< -o $@
>
> x86.h := $(addprefix $(XEN_ROOT)/tools/include/xen/asm/,\
> - x86-vendors.h x86-defns.h msr-index.h)
> + x86-vendors.h x86-defns.h msr-index.h) \
> + $(addprefix $(XEN_ROOT)/tools/include/xen/lib/x86/, \
> + cpuid.h cpuid-autogen.h)
> x86_emulate.h := x86-emulate.h x86_emulate/x86_emulate.h $(x86.h)
>
> # x86-emulate.c will be implicit for both
> x86-emulate.o x86-emulate-cov.o: x86_emulate/x86_emulate.c $(x86_emulate.h)
>
> -fuzz-emul.o fuzz-emulate-cov.o wrappers.o: $(x86_emulate.h)
> +fuzz-emul.o fuzz-emulate-cov.o cpuid.o wrappers.o: $(x86_emulate.h)
>
> x86-insn-fuzzer.a: fuzz-emul.o x86-emulate.o cpuid.o
> $(AR) rc $@ $^
> @@ -43,6 +45,11 @@ afl-harness: afl-harness.o fuzz-emul.o x
> afl-harness-cov: afl-harness-cov.o fuzz-emul-cov.o x86-emulate-cov.o cpuid.o
> wrappers.o
> $(CC) $(CFLAGS) $(GCOV_FLAGS) $^ -o $@
>
> +ifeq ($(MAKELEVEL),0)
> +$(XEN_ROOT)/tools/include/xen/lib/x86/cpuid-autogen.h: FORCE
> + $(MAKE) -C $(XEN_ROOT)/tools/include build
> +endif
> +
I'm not a big fan of this. But Andrew and you are the maintainers of
this facility so I won't block this patch.
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2.1 1/2] tools: fix build dependency upon generated header(s)
On Fri, Jan 25, 2019 at 12:33:49AM -0700, Jan Beulich wrote:
> Commit fd35f32b4b ("tools/x86emul: Use struct cpuid_policy in the
> userspace test harnesses") didn't account for the dependencies of
> cpuid-autogen.h to potentially change between incremental builds.
> Putting the make invocation to produce the header together with the
> directory tree creation therefore does not work. Introduce a separate
> goal.
>
> Signed-off-by: Jan Beulich
Acked-by: Wei Liu
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH-for-4.10/4.11] libxl: don't set gnttab limits in soft reset case
On Thu, Jan 17, 2019 at 05:40:59PM +0100, Juergen Gross wrote: > In case of soft reset the gnttab limit setting will fail, so omit it. > Setting of max vcpu count is pointless in this case, too, so we can > drop that as well. > > Without this patch soft reset will fail with: > > libxl: error: libxl_dom.c:363:libxl__build_pre: Couldn't set grant table > limits > > Reported-by: Jim Fehlig > Signed-off-by: Juergen Gross > Tested-by: Jim Fehlig Reviewed-by: Wei Liu > --- > 4.12 is not affected due to Andrew's domain creation interface changes, > 4.9 and earlier are not affected due to xc_domain_set_gnttab_limits() > only having been introduced in 4.10. Thanks for checking. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Patch "x86/entry/64/compat: Fix stack switching for XEN PV" has been added to the 4.19-stable tree
This is a note to let you know that I've just added the patch titled
x86/entry/64/compat: Fix stack switching for XEN PV
to the 4.19-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
x86-entry-64-compat-fix-stack-switching-for-xen-pv.patch
and it can be found in the queue-4.19 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
From fc24d75a7f91837d7918e40719575951820b2b8f Mon Sep 17 00:00:00 2001
From: Jan Beulich
Date: Tue, 15 Jan 2019 09:58:16 -0700
Subject: x86/entry/64/compat: Fix stack switching for XEN PV
From: Jan Beulich
commit fc24d75a7f91837d7918e40719575951820b2b8f upstream.
While in the native case entry into the kernel happens on the trampoline
stack, PV Xen kernels get entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path isn't
NMI / #MC safe, as either of these events occurring in the middle of the
stack copying would clobber data on the (source) stack.
There is similar code in interrupt_entry() and nmi(), but there is no fixup
required because those code paths are unreachable in XEN PV guests.
[ tglx: Sanitized subject, changelog, Fixes tag and stable mail address. Sigh ]
Fixes: 7f2590a110b8 ("x86/entry/64: Use a per-CPU trampoline stack for IDT
entries")
Signed-off-by: Jan Beulich
Signed-off-by: Thomas Gleixner
Reviewed-by: Juergen Gross
Acked-by: Andy Lutomirski
Cc: Peter Anvin
Cc: xen-devel@lists.xenproject.org>
Cc: Boris Ostrovsky
Cc: sta...@vger.kernel.org
Link:
https://lkml.kernel.org/r/5c3e112802780020d...@prv1-mh.provo.novell.com
Signed-off-by: Greg Kroah-Hartman
---
arch/x86/entry/entry_64_compat.S |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -356,7 +356,8 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
- movq%rsp, %rdi
+ /* In the Xen PV case we already run on the thread stack. */
+ ALTERNATIVE "movq %rsp, %rdi", "jmp .Lint80_keep_stack",
X86_FEATURE_XENPV
movqPER_CPU_VAR(cpu_current_top_of_stack), %rsp
pushq 6*8(%rdi) /* regs->ss */
@@ -365,8 +366,9 @@ ENTRY(entry_INT80_compat)
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
-
pushq (%rdi) /* pt_regs->di */
+.Lint80_keep_stack:
+
pushq %rsi/* pt_regs->si */
xorl%esi, %esi /* nospec si */
pushq %rdx/* pt_regs->dx */
Patches currently in stable-queue which might be from jbeul...@suse.com are
queue-4.19/x86-entry-64-compat-fix-stack-switching-for-xen-pv.patch
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Patch "x86/entry/64/compat: Fix stack switching for XEN PV" has been added to the 4.20-stable tree
This is a note to let you know that I've just added the patch titled
x86/entry/64/compat: Fix stack switching for XEN PV
to the 4.20-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
x86-entry-64-compat-fix-stack-switching-for-xen-pv.patch
and it can be found in the queue-4.20 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let know about it.
From fc24d75a7f91837d7918e40719575951820b2b8f Mon Sep 17 00:00:00 2001
From: Jan Beulich
Date: Tue, 15 Jan 2019 09:58:16 -0700
Subject: x86/entry/64/compat: Fix stack switching for XEN PV
From: Jan Beulich
commit fc24d75a7f91837d7918e40719575951820b2b8f upstream.
While in the native case entry into the kernel happens on the trampoline
stack, PV Xen kernels get entered with the current thread stack right
away. Hence source and destination stacks are identical in that case,
and special care is needed.
Other than in sync_regs() the copying done on the INT80 path isn't
NMI / #MC safe, as either of these events occurring in the middle of the
stack copying would clobber data on the (source) stack.
There is similar code in interrupt_entry() and nmi(), but there is no fixup
required because those code paths are unreachable in XEN PV guests.
[ tglx: Sanitized subject, changelog, Fixes tag and stable mail address. Sigh ]
Fixes: 7f2590a110b8 ("x86/entry/64: Use a per-CPU trampoline stack for IDT
entries")
Signed-off-by: Jan Beulich
Signed-off-by: Thomas Gleixner
Reviewed-by: Juergen Gross
Acked-by: Andy Lutomirski
Cc: Peter Anvin
Cc: xen-devel@lists.xenproject.org>
Cc: Boris Ostrovsky
Cc: sta...@vger.kernel.org
Link:
https://lkml.kernel.org/r/5c3e112802780020d...@prv1-mh.provo.novell.com
Signed-off-by: Greg Kroah-Hartman
---
arch/x86/entry/entry_64_compat.S |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -361,7 +361,8 @@ ENTRY(entry_INT80_compat)
/* Need to switch before accessing the thread stack. */
SWITCH_TO_KERNEL_CR3 scratch_reg=%rdi
- movq%rsp, %rdi
+ /* In the Xen PV case we already run on the thread stack. */
+ ALTERNATIVE "movq %rsp, %rdi", "jmp .Lint80_keep_stack",
X86_FEATURE_XENPV
movqPER_CPU_VAR(cpu_current_top_of_stack), %rsp
pushq 6*8(%rdi) /* regs->ss */
@@ -370,8 +371,9 @@ ENTRY(entry_INT80_compat)
pushq 3*8(%rdi) /* regs->cs */
pushq 2*8(%rdi) /* regs->ip */
pushq 1*8(%rdi) /* regs->orig_ax */
-
pushq (%rdi) /* pt_regs->di */
+.Lint80_keep_stack:
+
pushq %rsi/* pt_regs->si */
xorl%esi, %esi /* nospec si */
pushq %rdx/* pt_regs->dx */
Patches currently in stable-queue which might be from jbeul...@suse.com are
queue-4.20/x86-entry-64-compat-fix-stack-switching-for-xen-pv.patch
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 09/11] x86/vioapic: block speculative out-of-bound accesses
On 1/28/19 12:12, Jan Beulich wrote: On 28.01.19 at 12:03, wrote: >> On 1/25/19 17:34, Jan Beulich wrote: >> On 23.01.19 at 12:57, wrote: @@ -212,7 +217,12 @@ static void vioapic_write_redirent( struct hvm_irq *hvm_irq = hvm_domain_irq(d); union vioapic_redir_entry *pent, ent; int unmasked = 0; -unsigned int gsi = vioapic->base_gsi + idx; +unsigned int gsi; + +/* Make sure no out-of-bound value for idx can be used */ +idx = array_index_nospec(idx, vioapic->nr_pins); + +gsi = vioapic->base_gsi + idx; >>> I dislike the disconnect from the respective bounds check: There's >>> only one caller, so the construct could be moved there, or >>> otherwise I'd like to see an ASSERT() added documenting that the >>> bounds check is expected to have happened in the caller. >> I agree that the idx value is used as an array index in this function >> only once. However, the gsi value also uses the value of idx, and as >> that is passed to other functions, I want to bound the gsi variable as >> well. Therefore, I chose to have a separate assignment for the idx variable. > I don't mind the separate assignment, and I didn't complain > about idx being used just once. What I said is that there's > only one caller of the function. If the bounds checking was > done there, "gsi" here would be equally "bounded" afaict. > And I did suggest an alternative in case you dislike the > moving of the construct you add. Ah, I understand your previous sentence differently now. Thanks for clarifying. I like to keep the nospec statements close to the problematic use, so that eventual future callers benefit from that as well. Therefore, I'll add an ASSERT statement with the bound check. Best, Norbert > > Jan > > Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] tools/libxl: Fix leaking ssid_label in libxl_name_to_domid
On Sat, Jan 26, 2019 at 10:45:07PM -0700, Tamas K Lengyel wrote:
> On systems with XSM enabled libxl_name_to_domid leaks memory
> allocated for ssid_label:
>
> ==2693== 53 bytes in 2 blocks are definitely lost in loss record 4 of 8
> ==2693==at 0x4C2BE6D: malloc (vg_replace_malloc.c:309)
> ==2693==by 0x6C0A3B9: strdup (strdup.c:42)
> ==2693==by 0x5108294: libxl_flask_sid_to_context (libxl_flask.c:39)
> ==2693==by 0x50C2B64: libxl__xcinfo2xlinfo (libxl_domain.c:267)
> ==2693==by 0x50C2E02: libxl_list_domain (libxl_domain.c:308)
> ==2693==by 0x508A3C5: libxl_name_to_domid (libxl_utils.c:77)
>
> Signed-off-by: Tamas K Lengyel
> Cc: Ian Jackson
> Cc: Wei Liu
> ---
> tools/libxl/libxl_utils.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
> index e50e094c48..99abfa5497 100644
> --- a/tools/libxl/libxl_utils.c
> +++ b/tools/libxl/libxl_utils.c
> @@ -79,6 +79,8 @@ int libxl_name_to_domid(libxl_ctx *ctx, const char *name,
> return ERROR_NOMEM;
>
> for (i = 0; i < nb_domains; i++) {
> +if (dominfo[i].ssid_label)
> +free(dominfo[i].ssid_label);
> domname = libxl_domid_to_name(ctx, dominfo[i].domid);
> if (!domname)
> continue;
Thanks for reporting this issue. I think your patch isn't future-proof.
Can you try the following patch?
---8<---
From fc9f9ad912cb61085a5bfb60aef3643dcd82a496 Mon Sep 17 00:00:00 2001
From: Wei Liu
Date: Mon, 28 Jan 2019 12:10:12 +
Subject: [PATCH] libxl: correctly dispose of dominfo list in
libxl_name_to_domid
Tamas reported ssid_label was leaked. Use the designated function to
free dominfo list to fix the leakage.
Reported-by: Tamas K Lengyel
Signed-off-by: Wei Liu
---
tools/libxl/libxl_utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index e50e094c48..f360f5e228 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -90,7 +90,7 @@ int libxl_name_to_domid(libxl_ctx *ctx, const char *name,
}
free(domname);
}
-free(dominfo);
+libxl_dominfo_list_free(dominfo, nb_domains);
return ret;
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] arm: gic: deactivate sgi immediately after eoi
On 24/01/2019 13:43, Peng Fan wrote: > On i.MX8, we implemented partition reboot which means Cortex-A reboot > will not impact M4 cores and System control Unit core. However GICv3 > is not reset because we also need to support A72 Cluster reboot without > affecting A53 Cluster. > > The gic-v3 controller is configured with EOImode to 1, so during xen > reboot, there is a function call "smp_call_function(halt_this_cpu, NULL, 0);" > ,but halt_this_cpu never return, that means other CPUs have no chance to > deactive the SGI interrupt, because the deactivate_irq operation is at > the end of do_sgi. During xen booting again, CPU0 will issue > GIC_SGI_CALL_FUNCTION to other CPUs. Because GIC_SGI_CALL_FUNCTION of > other CPUs are active during the last reboot, interrupts could not be > triggered unless we deactivate the interrupt first. > > To fix this issue, let's move the deactivate_irq operation just after > eoi_irq, then the SGI interrupt will be in deactive state when > smp_call_function_interrupt. > > Signed-off-by: Peng Fan Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12] libxl: When restricted, start QEMU paused
On 24/01/2019 13:29, Anthony PERARD wrote: > Since libxl later during guest creation run the command "cont", it kind > of expect that QEMU would not do any emulation, use the "-S" command > option to make this effective. Unfortunately, when QEMU is started with > "-S", it won't write QEMU's readiness into xenstore. So only activate > this options when we have a QEMU startup notification via QMP available, > which is when dm_restrict is activated. > > This have the side-effect of rendering ineffective the startup > notification via xenstore, libxl will only have the notification via > QMP. > > It became important to rely only on QMP for notification when we have > it, as cutting short that path may result in the QMP socket been blocked > and have QEMU stop responding to upcoming connection even if none are > active. > > The QEMU bug that this patch works around is: > - libxl connect and hand-check with QEMU, then send the cmd > "query-status". > - QEMU prepare and maybe try send the response, > while also writing "running" into xenstore. > - libxl see via xenstore that QEMU is running and disconnect from the > QMP socket before receiving the response the cmd. > => The QMP socket (monitor) is sometime blocked and will never reply > to commands on new connections. > > This is due to QEMU only responding to one command at a time, and > suspending its monitor (QMP) until the command as been processed and > sent. Disconnecting from the socket doesn't unsuspend the monitor. The > race described here is very likely to happen with QEMU 3.1.50 (during > 3.2 development), but can be reproduced with QEMU 3.1. > > Signed-off-by: Anthony PERARD 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 v3] x86/AMD: flush TLB after ucode update
On 28/01/2019 10:51, Jan Beulich wrote: > The increased number of messages (spec_ctrl.c:print_details()) within a > certain time window made me notice some slowness of boot time screen > output. Experimentally I've narrowed the time window to be from > immediately after the early ucode update on the BSP to the PAT write in > cpu_init(), which upon further investigation has an effect because of > the full TLB flush that's implied by that write. > > For that reason, as a workaround, flush the TLB of the mapping of the > page that holds the blob. Note that flushing just a single page is > sufficient: As per verify_patch_size() patch size can't exceed 4k, and > the way xmalloc() works the blob can't be crossing a page boundary. > > Signed-off-by: Jan Beulich Release-acked-by: Juergen Gross Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH for-4.12 v2 0/7] xen/arm: Workaround for Cortex-A76 erratum 1165522
On 28/01/2019 12:50, Julien Grall wrote: > Hi all, > > Early version of Cortex-A76 can end-up with corrupt TLBs if they > speculate an AT instruction while the S1/S2 system registers are in an > inconsistent state. > > This can happen during guest context switch and when invalidating the > TLBs for other than the current VMID. > > The workaround implemented in Xen will: > - Use an empty stage-2 with a reserved VMID while context > switching between 2 guests > - Use an empty stage-2 with the VMID where TLBs need to > be flushed > > Cheers, > > CC: > > Julien Grall (7): > xen/arm: Only set necessary flags when initializing HCR_EL2 > xen/arm: p2m: Provide an helper to generate the VTTBR > xen/arm: p2m: Introduce an helper to allocate the root page-table > xen/arm: domain_build: Don't switch to the guest P2M when copying data > xen/arm: p2m: Only use isb() when it is necessary > xen/arm: Implement workaround for Cortex-A76 erratum 1165522 > DO NOT APPLY Allow testing the new AT speculate workaround code > > docs/misc/arm/silicon-errata.txt | 1 + > xen/arch/arm/cpuerrata.c | 16 ++ > xen/arch/arm/domain.c| 8 ++- > xen/arch/arm/domain_build.c | 13 - > xen/arch/arm/p2m.c | 118 > +++ > xen/arch/arm/traps.c | 8 ++- > xen/include/asm-arm/cpufeature.h | 3 +- > xen/include/asm-arm/processor.h | 2 + > 8 files changed, 139 insertions(+), 30 deletions(-) > 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 v2 for-4.12] gic-vgic: fix an assert condition
On 25/01/2019 18:06, Andrii Anisov wrote:
> From: Andrii Anisov
>
> Currently, that assert condition does not correspond to a comment above
> and makes assertion failed on HW IRQ disconnection.
> Fix the condition so it corresponds to the comment and allows IRQ
> disconnection on debug builds.
>
> Fixes: ec2a2f1 ("ARM: VGIC: factor out vgic_connect_hw_irq()")
> Signed-off-by: Andrii Anisov
> Suggested-by: Stefan Nuernberger
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 v6 1/3] xen/pt: fix some pass-thru devices don't work across reboot
On Fri, Jan 25, 2019 at 09:13:49AM -0700, Jan Beulich wrote:
On 25.01.19 at 09:26, wrote:
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -732,7 +732,11 @@ long arch_do_domctl(
>> break;
>>
>> ret = -EPERM;
>> -if ( irq <= 0 || !irq_access_permitted(currd, irq) )
>> +/*
>> + * irq < 0 denotes the corresponding pirq has been forcibly unbound.
>> + * For this case, bypass permission check to reap the pirq.
>> + */
>> +if ( !irq || ((irq > 0) && !irq_access_permitted(currd, irq)) )
>> break;
>
>So why would it be correct to continue into pt_irq_destroy_bind()
>with irq < 0? And with an actual XSM policy I'm not sure you'd
>even make it past xsm_unbind_pt_irq(). If the IRQ was forcibly
>unbound before, there shouldn't be anything left to clean up?
But some hints are left to denote that a pirq was forcibly unbound.
See the code snippets below:
''' in unmap_domain_pirq()
...
if ( !forced_unbind )
clear_domain_irq_pirq(d, irq, info);
else
{
info->arch.irq = -irq;
radix_tree_replace_slot(
radix_tree_lookup_slot(>arch.irq_pirq, irq),
radix_tree_int_to_ptr(-pirq));
}
...
'''
and
''' in pirq_guest_unbind()
...
if ( desc == NULL )
{
irq = -pirq->arch.irq;
BUG_ON(irq <= 0);
desc = irq_to_desc(irq);
spin_lock_irq(>lock);
clear_domain_irq_pirq(d, irq, pirq);
}
...
'''
>
>On the whole I think all the extra additions in v6 only serve to
>mask the tool stack not needing to do anymore some of what it
>does, as suggested in a reply to an earlier version. So I guess I
>agree with Roger that v5 came closer, but may need to be
>amended by some tool stack adjustment(s).
Yes. What v6 tries to mask here is irq unbinding and irq unmapping invoked
by qemu and pciback. We need to fix them in pciback and qemu rather than
tool stack. If we want to leave those error messages alone, we can just
take Roger's suggestion. Otherwise, we should try to conditional bypass
irq unbinding and irq unmapping in pciback and qemu and justify it.
Thanks
Chao
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12 v2 4/7] xen/arm: domain_build: Don't switch to the guest P2M when copying data
Until recently, kernel/initrd/dtb were loaded using guest VA and
therefore requiring to restore temporarily the P2M. This was reworked
in a series of commits (up to 9292086 "xen/arm: domain_build: Use
copy_to_guest_phys_flush_dcache in dtb_load") to use a guest PA.
This will also help a follow-up patch which will require
p2m_{save,restore}_state to work in pair to workaround an erratum.
Signed-off-by: Julien Grall
Reviewed-by: Stefano Stabellini
---
Changes in v2:
- Fix typo in commit message
- Add Stefano's reviewed-by
---
xen/arch/arm/domain_build.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index d2c63a89ca..31af989e63 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1923,7 +1923,6 @@ static void __init find_gnttab_region(struct domain *d,
static int __init construct_domain(struct domain *d, struct kernel_info *kinfo)
{
-struct vcpu *saved_current;
int i, cpu;
struct vcpu *v = d->vcpu[0];
struct cpu_user_regs *regs = >arch.cpu_info->guest_cpu_user_regs;
@@ -1945,14 +1944,6 @@ static int __init construct_domain(struct domain *d,
struct kernel_info *kinfo)
#endif
/*
- * The following loads use the domain's p2m and require current to
- * be a vcpu of the domain, temporarily switch
- */
-saved_current = current;
-p2m_restore_state(v);
-set_current(v);
-
-/*
* kernel_load will determine the placement of the kernel as well
* as the initrd & fdt in RAM, so call it first.
*/
@@ -1961,10 +1952,6 @@ static int __init construct_domain(struct domain *d,
struct kernel_info *kinfo)
initrd_load(kinfo);
dtb_load(kinfo);
-/* Now that we are done restore the original p2m and current. */
-set_current(saved_current);
-p2m_restore_state(saved_current);
-
memset(regs, 0, sizeof(*regs));
regs->pc = (register_t)kinfo->entry;
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12 v2 6/7] xen/arm: Implement workaround for Cortex-A76 erratum 1165522
Early version of Cortex-A76 can end-up with corrupt TLBs if they
speculate an AT instruction while the S1/S2 system registers are in an
inconsistent state.
This can happen during guest context switch and when invalidating the
TLBs for other than the current VMID.
The workaround implemented in Xen will:
- Use an empty stage-2 with a reserved VMID while context switching
between 2 guests
- Use an empty stage-2 with the VMID where TLBs need to be flushed
Signed-off-by: Julien Grall
Reviewed-by: Andrii Anisov
---
Changes in v2:
- Fix typo
- Add Andrii's reviewed-by
---
docs/misc/arm/silicon-errata.txt | 1 +
xen/arch/arm/cpuerrata.c | 6
xen/arch/arm/domain.c| 8 +++--
xen/arch/arm/p2m.c | 78 ++--
xen/include/asm-arm/cpufeature.h | 3 +-
xen/include/asm-arm/processor.h | 2 ++
6 files changed, 93 insertions(+), 5 deletions(-)
diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
index 906bf5fd48..6cd1366f15 100644
--- a/docs/misc/arm/silicon-errata.txt
+++ b/docs/misc/arm/silicon-errata.txt
@@ -48,4 +48,5 @@ stable hypervisors.
| ARM| Cortex-A57 | #852523 | N/A
|
| ARM| Cortex-A57 | #832075 | ARM64_ERRATUM_832075
|
| ARM| Cortex-A57 | #834220 | ARM64_ERRATUM_834220
|
+| ARM| Cortex-A76 | #1165522| N/A
|
| ARM| MMU-500 | #842869 | N/A
|
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index f4815cafb4..4431b244fd 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -489,6 +489,12 @@ static const struct arm_cpu_capabilities arm_errata[] = {
.matches = has_ssbd_mitigation,
},
#endif
+{
+/* Cortex-A76 r0p0 - r2p0 */
+.desc = "ARM erratum 116522",
+.capability = ARM64_WORKAROUND_AT_SPECULATE,
+MIDR_RANGE(MIDR_CORTEX_A76, 0, 2 << MIDR_VARIANT_SHIFT),
+},
{},
};
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 41f101746e..6dc633ed50 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -181,8 +181,6 @@ static void ctxt_switch_to(struct vcpu *n)
if ( is_idle_vcpu(n) )
return;
-p2m_restore_state(n);
-
vpidr = READ_SYSREG32(MIDR_EL1);
WRITE_SYSREG32(vpidr, VPIDR_EL2);
WRITE_SYSREG(n->arch.vmpidr, VMPIDR_EL2);
@@ -235,6 +233,12 @@ static void ctxt_switch_to(struct vcpu *n)
#endif
isb();
+/*
+ * ARM64_WORKAROUND_AT_SPECULATE: The P2M should be restored after
+ * the stage-1 MMU sysregs have been restored.
+ */
+p2m_restore_state(n);
+
/* Control Registers */
WRITE_SYSREG(n->arch.cpacr, CPACR_EL1);
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 44391a5f8c..c38bd7e16e 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -5,6 +5,7 @@
#include
#include
+#include
#include
#include
#include
@@ -41,6 +42,8 @@ static const paddr_t level_masks[] =
static const uint8_t level_orders[] =
{ ZEROETH_ORDER, FIRST_ORDER, SECOND_ORDER, THIRD_ORDER };
+static mfn_t __read_mostly empty_root_mfn;
+
static uint64_t generate_vttbr(uint16_t vmid, mfn_t root_mfn)
{
return (mfn_to_maddr(root_mfn) | ((uint64_t)vmid << 48));
@@ -92,9 +95,25 @@ void dump_p2m_lookup(struct domain *d, paddr_t addr)
P2M_ROOT_LEVEL, P2M_ROOT_PAGES);
}
+/*
+ * p2m_save_state and p2m_restore_state work in pair to workaround
+ * ARM64_WORKAROUND_AT_SPECULATE. p2m_save_state will set-up VTTBR to
+ * point to the empty page-tables to stop allocating TLB entries.
+ */
void p2m_save_state(struct vcpu *p)
{
p->arch.sctlr = READ_SYSREG(SCTLR_EL1);
+
+if ( cpus_have_const_cap(ARM64_WORKAROUND_AT_SPECULATE) )
+{
+WRITE_SYSREG64(generate_vttbr(INVALID_VMID, empty_root_mfn),
VTTBR_EL2);
+/*
+ * Ensure VTTBR_EL2 is correctly synchronized so we can restore
+ * the next vCPU context without worrying about AT instruction
+ * speculation.
+ */
+isb();
+}
}
void p2m_restore_state(struct vcpu *n)
@@ -105,10 +124,17 @@ void p2m_restore_state(struct vcpu *n)
if ( is_idle_vcpu(n) )
return;
-WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
WRITE_SYSREG(n->arch.hcr_el2, HCR_EL2);
+/*
+ * ARM64_WORKAROUND_AT_SPECULATE: VTTBR_EL2 should be restored after all
+ * registers associated to EL1/EL0 translations regime have been
+ * synchronized.
+ */
+asm volatile(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_AT_SPECULATE));
+WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+
last_vcpu_ran = >last_vcpu_ran[smp_processor_id()];
/*
@@ -149,8 +175,23 @@ static void p2m_force_tlb_flush_sync(struct p2m_domain
*p2m)
ovttbr =
[Xen-devel] [PATCH for-4.12 v2 0/7] xen/arm: Workaround for Cortex-A76 erratum 1165522
Hi all, Early version of Cortex-A76 can end-up with corrupt TLBs if they speculate an AT instruction while the S1/S2 system registers are in an inconsistent state. This can happen during guest context switch and when invalidating the TLBs for other than the current VMID. The workaround implemented in Xen will: - Use an empty stage-2 with a reserved VMID while context switching between 2 guests - Use an empty stage-2 with the VMID where TLBs need to be flushed Cheers, CC: Julien Grall (7): xen/arm: Only set necessary flags when initializing HCR_EL2 xen/arm: p2m: Provide an helper to generate the VTTBR xen/arm: p2m: Introduce an helper to allocate the root page-table xen/arm: domain_build: Don't switch to the guest P2M when copying data xen/arm: p2m: Only use isb() when it is necessary xen/arm: Implement workaround for Cortex-A76 erratum 1165522 DO NOT APPLY Allow testing the new AT speculate workaround code docs/misc/arm/silicon-errata.txt | 1 + xen/arch/arm/cpuerrata.c | 16 ++ xen/arch/arm/domain.c| 8 ++- xen/arch/arm/domain_build.c | 13 - xen/arch/arm/p2m.c | 118 +++ xen/arch/arm/traps.c | 8 ++- xen/include/asm-arm/cpufeature.h | 3 +- xen/include/asm-arm/processor.h | 2 + 8 files changed, 139 insertions(+), 30 deletions(-) -- 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12 v2 5/7] xen/arm: p2m: Only use isb() when it is necessary
The EL1 translation regime is out-of-context when running at EL2. This
means the processor cannot speculate memory accesses using the registers
associated to that regime.
An isb() is only needed if Xen is going to use the translation regime
before returning to the guest (exception returns will synchronize the
context).
Remove unnecessary isb() and document the ones left.
Signed-off-by: Julien Grall
Reviewed-by: Andrii Anisov
---
Changes in v2:
- Remove pointless {}
- Fix typoes
- Add Andrii's reviewed-by
---
xen/arch/arm/p2m.c | 17 +++--
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 9844bfb936..44391a5f8c 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -106,17 +106,21 @@ void p2m_restore_state(struct vcpu *n)
return;
WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
-isb();
-
WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
-isb();
-
WRITE_SYSREG(n->arch.hcr_el2, HCR_EL2);
-isb();
last_vcpu_ran = >last_vcpu_ran[smp_processor_id()];
/*
+ * While we are restoring an out-of-context translation regime
+ * we still need to ensure:
+ * - VTTBR_EL2 is synchronized before flushing the TLBs
+ * - All registers for EL1 are synchronized before executing an AT
+ * instructions targeting S1/S2.
+ */
+isb();
+
+/*
* Flush local TLB for the domain to prevent wrong TLB translation
* when running multiple vCPU of the same domain on a single pCPU.
*/
@@ -147,6 +151,7 @@ static void p2m_force_tlb_flush_sync(struct p2m_domain *p2m)
{
local_irq_save(flags);
WRITE_SYSREG64(p2m->vttbr, VTTBR_EL2);
+/* Ensure VTTBR_EL2 is synchronized before flushing the TLBs */
isb();
}
@@ -155,6 +160,7 @@ static void p2m_force_tlb_flush_sync(struct p2m_domain *p2m)
if ( ovttbr != READ_SYSREG64(VTTBR_EL2) )
{
WRITE_SYSREG64(ovttbr, VTTBR_EL2);
+/* Ensure VTTBR_EL2 is back in place before continuing. */
isb();
local_irq_restore(flags);
}
@@ -1907,7 +1913,6 @@ static uint32_t __read_mostly vtcr;
static void setup_virt_paging_one(void *data)
{
WRITE_SYSREG32(vtcr, VTCR_EL2);
-isb();
}
void __init setup_virt_paging(void)
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12 v2 1/7] xen/arm: Only set necessary flags when initializing HCR_EL2
Only {A,F,I}MO are necessary to receive interrupts until a guest vCPU is
loaded.
The rest have no effect on Xen and it is better to avoid setting them.
Signed-off-by: Julien Grall
Reviewed-by: Andrii Anisov
Reviewed-by: Stefano Stabellini
---
Changes in v2:
- Fix typo
- Add Andrii's and Stefano's reviewed-by
---
xen/arch/arm/traps.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 221c762ada..64a78d83a5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -157,8 +157,12 @@ void init_traps(void)
WRITE_SYSREG((HCPTR_CP_MASK & ~(HCPTR_CP(10) | HCPTR_CP(11))) | HCPTR_TTA,
CPTR_EL2);
-/* Setup hypervisor traps */
-WRITE_SYSREG(get_default_hcr_flags(), HCR_EL2);
+/*
+ * Configure HCR_EL2 with the bare minimum to run Xen until a guest
+ * is scheduled. {A,I,F}MO bits are set to allow EL2 receiving
+ * interrupts.
+ */
+WRITE_SYSREG(HCR_AMO | HCR_FMO | HCR_IMO, HCR_EL2);
isb();
}
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12 v2 2/7] xen/arm: p2m: Provide an helper to generate the VTTBR
A follow-up patch will need to generate the VTTBR in a few places.
Signed-off-by: Julien Grall
Reviewed-by: Andrii Anisov
Reviewed-by: Stefano Stabellini
---
Changes in v2:
- Add Andrii's and Stefano's reviewed-by
---
xen/arch/arm/p2m.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 059a39157a..38bfa9964f 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -41,6 +41,11 @@ static const paddr_t level_masks[] =
static const uint8_t level_orders[] =
{ ZEROETH_ORDER, FIRST_ORDER, SECOND_ORDER, THIRD_ORDER };
+static uint64_t generate_vttbr(uint16_t vmid, mfn_t root_mfn)
+{
+return (mfn_to_maddr(root_mfn) | ((uint64_t)vmid << 48));
+}
+
/* Unlock the flush and do a P2M TLB flush if necessary */
void p2m_write_unlock(struct p2m_domain *p2m)
{
@@ -1364,7 +1369,7 @@ static int p2m_alloc_table(struct domain *d)
p2m->root = page;
-p2m->vttbr = page_to_maddr(p2m->root) | ((uint64_t)p2m->vmid << 48);
+p2m->vttbr = generate_vttbr(p2m->vmid, page_to_mfn(p2m->root));
/*
* Make sure that all TLBs corresponding to the new VMID are flushed
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12 v2 7/7] DO NOT APPLY Allow testing the new AT speculate workaround code
Signed-off-by: Julien Grall
---
xen/arch/arm/cpuerrata.c | 10 ++
xen/arch/arm/p2m.c | 2 ++
2 files changed, 12 insertions(+)
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 4431b244fd..727c67451d 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -381,6 +381,11 @@ static bool has_ssbd_mitigation(const struct
arm_cpu_capabilities *entry)
}
#endif
+static bool has_at_speculate(const struct arm_cpu_capabilities *entry)
+{
+return true;
+}
+
#define MIDR_RANGE(model, min, max) \
.matches = is_affected_midr_range, \
.midr_model = model,\
@@ -495,6 +500,11 @@ static const struct arm_cpu_capabilities arm_errata[] = {
.capability = ARM64_WORKAROUND_AT_SPECULATE,
MIDR_RANGE(MIDR_CORTEX_A76, 0, 2 << MIDR_VARIANT_SHIFT),
},
+{
+.desc = "AT speculate",
+.capability = ARM64_WORKAROUND_AT_SPECULATE,
+.matches = has_at_speculate,
+},
{},
};
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index c38bd7e16e..2b25706823 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -124,6 +124,8 @@ void p2m_restore_state(struct vcpu *n)
if ( is_idle_vcpu(n) )
return;
+ASSERT(READ_SYSREG64(VTTBR_EL2) == (generate_vttbr(INVALID_VMID,
empty_root_mfn)));
+
WRITE_SYSREG(n->arch.sctlr, SCTLR_EL1);
WRITE_SYSREG(n->arch.hcr_el2, HCR_EL2);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-4.12 v2 3/7] xen/arm: p2m: Introduce an helper to allocate the root page-table
A follow-up patch will require to allocate the root page-table without
having a domain in hand.
Signed-off-by: Julien Grall
Reviewed-by: Andrii Anisov
Reviewed-by: Stefano Stabellini
---
Changes in v2:
- Add Andrii's and Stefano's reviewed-by
---
xen/arch/arm/p2m.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 38bfa9964f..9844bfb936 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1353,21 +1353,29 @@ int guest_physmap_remove_page(struct domain *d, gfn_t
gfn, mfn_t mfn,
return p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
}
-static int p2m_alloc_table(struct domain *d)
+static struct page_info *p2m_allocate_root(void)
{
-struct p2m_domain *p2m = p2m_get_hostp2m(d);
struct page_info *page;
unsigned int i;
page = alloc_domheap_pages(NULL, P2M_ROOT_ORDER, 0);
if ( page == NULL )
-return -ENOMEM;
+return NULL;
/* Clear both first level pages */
for ( i = 0; i < P2M_ROOT_PAGES; i++ )
clear_and_clean_page(page + i);
-p2m->root = page;
+return page;
+}
+
+static int p2m_alloc_table(struct domain *d)
+{
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+
+p2m->root = p2m_allocate_root();
+if ( !p2m->root )
+return -ENOMEM;
p2m->vttbr = generate_vttbr(p2m->vmid, page_to_mfn(p2m->root));
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] xen: add interface for obtaining .config from hypervisor
>>> On 28.01.19 at 12:40, wrote: On 17.01.19 at 15:57, wrote: >> @@ -83,3 +84,9 @@ subdir-$(CONFIG_UBSAN) += ubsan >> >> subdir-$(CONFIG_NEEDS_LIBELF) += libelf >> subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt >> + >> +config_data.c: ../.config >> +( echo "const char xen_config_data[] ="; \ >> + cat $< | gzip | ../tools/bin2c; \ >> + echo ";"; \ >> + echo "unsigned int xen_config_data_sz = sizeof(xen_config_data) - 1;" >> ) > > $@ > > const here as well please. With this adjusted > Reviewed-by: Jan Beulich > (apart from the XSM changes) And I should have added "with Wei's comments also taken care of". Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3] x86/AMD: flush TLB after ucode update
On 28/01/2019 09:51, Jan Beulich wrote:
> The increased number of messages (spec_ctrl.c:print_details()) within a
> certain time window made me notice some slowness of boot time screen
> output. Experimentally I've narrowed the time window to be from
> immediately after the early ucode update on the BSP to the PAT write in
> cpu_init(), which upon further investigation has an effect because of
> the full TLB flush that's implied by that write.
>
> For that reason, as a workaround, flush the TLB of the mapping of the
> page that holds the blob. Note that flushing just a single page is
> sufficient: As per verify_patch_size() patch size can't exceed 4k, and
> the way xmalloc() works the blob can't be crossing a page boundary.
>
> Signed-off-by: Jan Beulich
I think it is worth at least noting that we are expecting a BKGD/PPR
update to this effect in due course, even if this doesn't end up in the
commit message.
> ---
> v3: Use TLB flush instead of PAT write.
> v2: Re-base.
>
> --- a/xen/arch/x86/microcode_amd.c
> +++ b/xen/arch/x86/microcode_amd.c
> @@ -218,6 +218,12 @@ static int apply_microcode(unsigned int
>
> spin_unlock_irqrestore(_update_lock, flags);
>
> +/*
> + * Experimentally this helps with performance issues on at least certain
> + * Fam15 models.
This is no longer experimental, now that we understand why. How about:
"Some processors leave the ucode blob mapping as UC after the update.
Flush the mapping to regain normal cacheability" ?
That way, its also slightly less cryptic in the code.
~Andrew
> + */
> +flush_area_local(hdr, FLUSH_TLB_GLOBAL | FLUSH_ORDER(0));
> +
> /* check current patch id and patch's id for match */
> if ( hw_err || (rev != hdr->patch_id) )
> {
>
>
>
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/2] xen: add interface for obtaining .config from hypervisor
>>> On 17.01.19 at 15:57, wrote: > @@ -83,3 +84,9 @@ subdir-$(CONFIG_UBSAN) += ubsan > > subdir-$(CONFIG_NEEDS_LIBELF) += libelf > subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt > + > +config_data.c: ../.config > + ( echo "const char xen_config_data[] ="; \ > + cat $< | gzip | ../tools/bin2c; \ > + echo ";"; \ > + echo "unsigned int xen_config_data_sz = sizeof(xen_config_data) - 1;" > ) > $@ const here as well please. With this adjusted Reviewed-by: Jan Beulich (apart from the XSM changes) Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
