[Xen-devel] [xen-unstable-smoke test] 126019: regressions - FAIL
flight 126019 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/126019/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl 12 guest-start fail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen 3e4ec07e14bce81f6ae22c31ff1302d1f297a226 baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z1 days Failing since125928 2018-08-15 19:00:49 Z1 days 14 attempts Testing same since 126009 2018-08-16 20:00:24 Z0 days3 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Ian Jackson Jan Beulich Julien Grall Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl fail 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 Not pushing. commit 3e4ec07e14bce81f6ae22c31ff1302d1f297a226 Author: Andrew Cooper Date: Thu Aug 16 16:26:22 2018 +0100 x86/setup: Avoid OoB E820 lookup when calculating the L1TF safe address A number of corner cases (most obviously, no-real-mode and no Multiboot memory map) can end up with e820_raw.nr_map being 0, at which point the L1TF calculation will underflow. Spotted by Coverity. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Reviewed-by: Wei Liu commit afc3f910d3434b540e4e4f51d9fd2723aea22fa2 Author: Jan Beulich Date: Thu Aug 16 00:49:29 2018 -0600 libxl: fix ARM build after 54ed251dc7 Commit "tools: Rework xc_domain_create() to take a full xen_domctl_createdomain" failed to replace one further instance of xc_config in libxl__arch_domain_save_config(). Signed-off-by: Jan Beulich Acked-by: Andrew Cooper Acked-by: Ian Jackson commit 70d8543950ad045fddcb78ae11302e713ef09c76 Author: Zhenzhong Duan Date: Thu Aug 16 09:31:57 2018 +0200 x86/mmcfg: remove redundant code in pci_mmcfg_reject_broken() No functional change. Signed-off-by: Zhenzhong Duan Acked-by: Jan Beulich commit a9e9837f54973ac45488c24e93ed34cbf20e4c66 Author: Jan Beulich Date: Thu Aug 16 09:30:59 2018 +0200 gnttab/ARM: properly implement gnttab_create_status_page() Prevent the "BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames() from triggering. Reported-by: 王磊 Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini commit 7626edeaca972e3e823535dcc44338f6b2f0b21f Author: Paul Durrant Date: Thu Aug 16 09:27:30 2018 +0200 x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries When emulating a rep I/O operation it is possible that the ioreq will describe a single operation that spans multiple GFNs. This is fine as long as all those GFNs fall within an MMIO region covered by a single device model, but unfortunately the higher levels of the emulation code do not guarantee that. This is something that should almost certainly be fixed, but in the meantime this patch makes sure that MMIO is truncated at GFN boundaries and hence the appropriate device model is re-evaluated for each target GFN. NOTE: This patch does not deal with the case of a single MMIO operation spanning a GFN boundary. That is more complex to deal with and is deferred to a subsequent patch. Signed-off-by: Paul Durrant Convert calculations to be 32-bit only. Signed-off-by: Jan Beulich commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into
Re: [Xen-devel] [PATCH v3 13/17] x86/xen: enable Hygon support to Xen
On 08/16/2018 09:29 AM, Pu Wen wrote: > On 2018/8/12 21:26, Boris Ostrovsky wrote: >> On 08/12/2018 04:55 AM, Juergen Gross wrote: >>> On 11/08/18 16:34, Boris Ostrovsky wrote: On 08/11/2018 09:29 AM, Pu Wen wrote: > bool pmu_msr_read(unsigned int msr, uint64_t *val, int *err) > { > - if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { > + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || > + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) { 'if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)' please. >>> Really? Xen supports Centaur, too. >> >> VPMU doesn't --- hypervisor will not initialize it. Besides, the >> existing code will steer non-AMD execution to Intel, which is not right >> either. >> >> I'll add a check to bail if VPMU is not initialized properly, we seem to >> ignore xen_pmu_init() failures. Which, BTW, makes this patch rather >> pointless until there is support for Hygon Xen. > > So should it still need to test vendor Hygon here or wait for your check > done? I'd prefer checking for !Intel, as I suggested above. Centaur will fail either way, but because we use safe versions of MSR access I now think we don't need any extra checks for xen_pmu_init() result. -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] x86/entry/64: Remove %ebx handling from error_entry/exit
commit b3681dd548d06deb2e1573890829dff4b15abf46 upstream. This version applies to v4.9. From Andy Lutomirski, original author: error_entry and error_exit communicate the user vs kernel status of the frame using %ebx. This is unnecessary -- the information is in regs->cs. Just use regs->cs. This makes error_entry simpler and makes error_exit more robust. It also fixes a nasty bug. Before all the Spectre nonsense, The xen_failsafe_callback entry point returned like this: ALLOC_PT_GPREGS_ON_STACK SAVE_C_REGS SAVE_EXTRA_REGS ENCODE_FRAME_POINTER jmp error_exit And it did not go through error_entry. This was bogus: RBX contained garbage, and error_exit expected a flag in RBX. Fortunately, it generally contained *nonzero* garbage, so the correct code path was used. As part of the Spectre fixes, code was added to clear RBX to mitigate certain speculation attacks. Now, depending on kernel configuration, RBX got zeroed and, when running some Wine workloads, the kernel crashes. This was introduced by: commit 3ac6d8c787b8 ("x86/entry/64: Clear registers for exceptions/interrupts, to reduce speculation attack surface") With this patch applied, RBX is no longer needed as a flag, and the problem goes away. I suspect that malicious userspace could use this bug to crash the kernel even without the offending patch applied, though. [Historical note: I wrote this patch as a cleanup before I was aware of the bug it fixed.] [Note to stable maintainers: this should probably get applied to all kernels.] Cc: Brian Gerst Cc: Borislav Petkov Cc: Dominik Brodowski Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: Thomas Gleixner Cc: Boris Ostrovsky Cc: Juergen Gross Cc: xen-devel@lists.xenproject.org Cc: x...@kernel.org Cc: sta...@vger.kernel.org Cc: Andy Lutomirski Fixes: 3ac6d8c787b8 ("x86/entry/64: Clear registers for exceptions/interrupts, to reduce speculation attack surface") Reported-and-tested-by: "M. Vefa Bicakci" Signed-off-by: Andy Lutomirski Signed-off-by: Sarah Newman --- arch/x86/entry/entry_64.S | 20 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S index d58d8dc..76c1d85e 100644 --- a/arch/x86/entry/entry_64.S +++ b/arch/x86/entry/entry_64.S @@ -774,7 +774,7 @@ ENTRY(\sym) call\do_sym - jmp error_exit /* %ebx: no swapgs flag */ + jmp error_exit .endif END(\sym) .endm @@ -1043,7 +1043,6 @@ END(paranoid_exit) /* * Save all registers in pt_regs, and switch gs if needed. - * Return: EBX=0: came from user mode; EBX=1: otherwise */ ENTRY(error_entry) cld @@ -1056,7 +1055,6 @@ ENTRY(error_entry) * the kernel CR3 here. */ SWITCH_KERNEL_CR3 - xorl%ebx, %ebx testb $3, CS+8(%rsp) jz .Lerror_kernelspace @@ -1087,7 +1085,6 @@ ENTRY(error_entry) * for these here too. */ .Lerror_kernelspace: - incl%ebx leaqnative_irq_return_iret(%rip), %rcx cmpq%rcx, RIP+8(%rsp) je .Lerror_bad_iret @@ -1119,28 +1116,19 @@ ENTRY(error_entry) /* * Pretend that the exception came from user mode: set up pt_regs -* as if we faulted immediately after IRET and clear EBX so that -* error_exit knows that we will be returning to user mode. +* as if we faulted immediately after IRET. */ mov %rsp, %rdi callfixup_bad_iret mov %rax, %rsp - decl%ebx jmp .Lerror_entry_from_usermode_after_swapgs END(error_entry) - -/* - * On entry, EBX is a "return to kernel mode" flag: - * 1: already in kernel mode, don't need SWAPGS - * 0: user gsbase is loaded, we need SWAPGS and standard preparation for return to usermode - */ ENTRY(error_exit) - movl%ebx, %eax DISABLE_INTERRUPTS(CLBR_NONE) TRACE_IRQS_OFF - testl %eax, %eax - jnz retint_kernel + testb $3, CS(%rsp) + jz retint_kernel jmp retint_user END(error_exit) -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Future of 32-bit PV support
On 16/08/18 19:34, Stefano Stabellini wrote: > On Thu, 16 Aug 2018, Juergen Gross wrote: >> In the Xen x86 community call we have been discussing whether anyone >> really is depending on 32-bit PV guests. We'd like to evaluate whether >> anyone would see problems with: >> >> - deprecating 32-bit PV guest support in Xen, meaning that we'd >> eventually switch to support 32-bit PV guests only via PV-shim from >> Xen 4.12 or 4.13 >> >> - dropping 32-bit PV support from upstream Linux kernel, resulting in >> current 32-bit PV guests no longer being able to upgrade to the newest >> kernel version any longer >> >> And related to that: >> >> - is there any Linux distribution still shipping 32-bit PV-capable >> systems? >> >> - what about BSD? Is 32-bit PV support important there? > > Hi Juergen, > > Although I can see that deprecating 32-bit PV guest support is > desirable, and it might not cause any problems to Linux and > BSDs, we need to be careful about unikernels. > > There are probably unikernels out there that only support PV 32bit > still. And why not? If you are designing a unikernel today, it would > still make sense to use PV 32bit or PVH. PVH will still work, of course. 32- and 64-bit. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Xen-users] Future of 32-bit PV support
On 17/08/18 00:33, Andy Smith wrote: > Hi Juergen, > > As this was also addressed to -user I'm going to assume that you do > want user response as well. Right. Thanks for responding. > > On Thu, Aug 16, 2018 at 08:17:13AM +0200, Juergen Gross wrote: >> We'd like to evaluate whether anyone would see problems with: >> >> - deprecating 32-bit PV guest support in Xen, meaning that we'd >> eventually switch to support 32-bit PV guests only via PV-shim from >> Xen 4.12 or 4.13 > > Although amd64 has been the default for us for many years, at the > moment we still have 64% of our customers running 32-bit PV. If > there remains a way for us to boot them through PV-shim and then > pvgrub2 with no functional changes and no work inside the guest then > that's fine, we'll adapt. > >> - dropping 32-bit PV support from upstream Linux kernel, resulting in >> current 32-bit PV guests no longer being able to upgrade to the newest >> kernel version any longer > > I doubt there is any technical reason why they can't switch to > 64-bit, it's just that in the majority of cases that involves a > complete reinstall and the users just haven't bothered to. Is something like missing Meltdown mitigation for 32-bit PV guest a technical reason? > If they are forced to switch because an impending kernel update will > leave them with a kernel that doesn't boot, they are going to be > upset that they are forced to reinstall their guest, or switch to a > 64-bit kernel with their existing 32-bit userland. > > It will of course help if they have plenty of warning that they need > to make the switch. But unless we're talking 2+ years of warning I'm > sure there will be some who will be unhappy. > > I was hoping to transition to PVH guests as soon as possible, but > last time I looked into it there was a problem booting the stable > Linux kernel under PVH, and also no support in grub2. Okay, noted. > Will it remain possible to boot a 32-bit Linux guest in PVH mode? Yes. > If so, could the final removal of 32-bit PV in the Linux kernel be > held off until there is: > > 1) a kernel shipping in Debian stable, Ubuntu LTS and CentOS that >boots under PVH, and; > > 2) support in grub2 so I can build a grub image that boots under >PVH? I think this is a reasonable request. > If grub PVH support is not going to happen, what is the roadmap for > user-specified guest kernels under PVH? I have a patch series lying around for grub2 PVH support. It requires some rework and another kernel enhancement. I'll try to resume work on the patches soon. > >> - is there any Linux distribution still shipping 32-bit PV-capable >> systems? > > Debian stable 32-bit kernels still boot under PV, as do Ubuntu 18.04 > LTS ones. Ubuntu LTS releases are supposed to be supported (by > Canonical) for 5 years, and while of course Xen does not fall under > the category of software that they support, there will be people > sticking with 18.04 LTS as long as they can. I guess they will stick to the stable kernel they are using now? Then this will be no problem. > I'm not saying that people running 32-bit PV Ubuntu 18.04 are right > to expect that to continue being supported until 2023. I'm just > saying that human nature dictates that those sorts of expectations > will exist. > > It will help a lot if there is an easy way for us to switch them > from 32-bit PV to PVH, while still letting them install their own > kernels. In the end it should be just a switch of domain type and boot loader (PV -> PVH, grubxen -> grubxen-pvh). The kernel needs to be configured to support PVH, of course. Thanks for the very valuable input! Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 23/25] xen: support console_switching between Dom0 and DomUs on ARM
On Mon, 13 Aug 2018, Julien Grall wrote: > Hi Stefano, > > OOI, on the previous version you said you will explore the CTRL-x N solution > (where N is the domID console to switch too). What was the result here? I meant I'll explore it as a follow-up to this series. I haven't looked into it yet, but it is in my todo. > On 01/08/18 00:28, Stefano Stabellini wrote: > > Today Ctrl-AAA is used to switch between Xen and Dom0. Extend the > > mechanism to allow for switching between Xen, Dom0, and any of the > > initial DomU created from Xen alongside Dom0 out of information provided > > via device tree. > > > > Rename xen_rx to console_rx to match the new behavior. > > > > Clarify existing comment about "notify the guest", making it clear that > > it is only about the hardware domain. > > > > Export a function named console_input_domain() to allow others to know > > which domains has input at a given time. > > > > Signed-off-by: Stefano Stabellini > > CC: andrew.coop...@citrix.com > > CC: george.dun...@eu.citrix.com > > CC: ian.jack...@eu.citrix.com > > CC: jbeul...@suse.com > > CC: konrad.w...@oracle.com > > CC: t...@xen.org > > CC: wei.l...@citrix.com > > --- > > Changes in v3: > > - only call vpl011 functions ifdef CONFIG_SBSA_VUART_CONSOLE > > - add blank line and spaces > > - remove xen_rx from print messages > > - rename xen_rx to console_rx > > - keep switch_serial_input() at boot > > - add better comments > > - fix existing comment > > - add warning if no guests console/uart is available > > - add console_input_domain function > > > > Changes in v2: > > - only call vpl011_rx_char if the vpl011 has been initialized > > --- > > xen/drivers/char/console.c | 71 > > +- > > xen/include/xen/console.h | 2 ++ > > 2 files changed, 60 insertions(+), 13 deletions(-) > > > > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c > > index 0f05369..cd4dfb1 100644 > > --- a/xen/drivers/char/console.c > > +++ b/xen/drivers/char/console.c > > @@ -31,10 +31,13 @@ > > #include > > #include > > #include > > +#include > > #ifdef CONFIG_X86 > > #include > > #include > > +#else > > +#include > > #endif > > /* console: comma-separated list of console outputs. */ > > @@ -389,30 +392,72 @@ static void dump_console_ring_key(unsigned char key) > > free_xenheap_pages(buf, order); > > } > > -/* CTRL- switches input direction between Xen and DOM0. */ > > +/* > > + * CTRL- switches input direction between Xen, Dom0 and > > + * DomUs. > > + */ > > #define switch_code (opt_conswitch[0]-'a'+1) > > -static int __read_mostly xen_rx = 1; /* FALSE => input passed to domain 0. > > */ > > +/* > > + * console_rx=0 => input to xen > > + * console_rx=1 => input to dom0 > > + * console_rx=N => input dom(N-1) > > + */ > > +static int __read_mostly console_rx = 0; > > + > > +struct domain *console_input_domain(void) > > +{ > > +return get_domain_by_id(console_rx - 1); > > Please take care of the case where console_rx == 0. I'll do > > +} > > static void switch_serial_input(void) > > { > > -static char *input_str[2] = { "DOM0", "Xen" }; > > -xen_rx = !xen_rx; > > -printk("*** Serial input -> %s", input_str[xen_rx]); > > +console_rx++; > > +if ( console_rx == max_init_domid + 2 ) > > +console_rx = 0; > > + > > +if ( !console_rx ) > > +printk("*** Serial input to Xen"); > > +else > > +printk("*** Serial input to DOM%d", console_rx - 1); > > + > > if ( switch_code ) > > -printk(" (type 'CTRL-%c' three times to switch input to %s)", > > - opt_conswitch[0], input_str[!xen_rx]); > > +printk(" (type 'CTRL-%c' three times to switch input)", > > + opt_conswitch[0]); > > printk("\n"); > > } > > static void __serial_rx(char c, struct cpu_user_regs *regs) > > { > > -if ( xen_rx ) > > +if ( console_rx == 0 ) > > return handle_keypress(c, regs); > > -/* Deliver input to guest buffer, unless it is already full. */ > > -if ( (serial_rx_prod-serial_rx_cons) != SERIAL_RX_SIZE ) > > -serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c; > > -/* Always notify the guest: prevents receive path from getting stuck. > > */ > > +if ( console_rx == 1 ) > > +{ > > +/* Deliver input to guest buffer, unless it is already full. */ > > +if ( (serial_rx_prod - serial_rx_cons) != SERIAL_RX_SIZE ) > > +serial_rx_ring[SERIAL_RX_MASK(serial_rx_prod++)] = c; > > +} > > +#ifdef CONFIG_SBSA_VUART_CONSOLE > > +else > > +{ > > +struct domain *d = get_domain_by_id(console_rx - 1); > > I don't think this is correct to assume the domain will always be present. > With this series, it would be possible to retire a domain and therefore > get_domain_by_id() would return NULL here. This would result to a data abort > below. Well, spotted! I'll fix. > Also, I
[Xen-devel] [xen-unstable-smoke test] 126015: regressions - FAIL
flight 126015 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/126015/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl 12 guest-start fail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen 3e4ec07e14bce81f6ae22c31ff1302d1f297a226 baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z1 days Failing since125928 2018-08-15 19:00:49 Z1 days 13 attempts Testing same since 126009 2018-08-16 20:00:24 Z0 days2 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Ian Jackson Jan Beulich Julien Grall Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl fail 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 Not pushing. commit 3e4ec07e14bce81f6ae22c31ff1302d1f297a226 Author: Andrew Cooper Date: Thu Aug 16 16:26:22 2018 +0100 x86/setup: Avoid OoB E820 lookup when calculating the L1TF safe address A number of corner cases (most obviously, no-real-mode and no Multiboot memory map) can end up with e820_raw.nr_map being 0, at which point the L1TF calculation will underflow. Spotted by Coverity. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Reviewed-by: Wei Liu commit afc3f910d3434b540e4e4f51d9fd2723aea22fa2 Author: Jan Beulich Date: Thu Aug 16 00:49:29 2018 -0600 libxl: fix ARM build after 54ed251dc7 Commit "tools: Rework xc_domain_create() to take a full xen_domctl_createdomain" failed to replace one further instance of xc_config in libxl__arch_domain_save_config(). Signed-off-by: Jan Beulich Acked-by: Andrew Cooper Acked-by: Ian Jackson commit 70d8543950ad045fddcb78ae11302e713ef09c76 Author: Zhenzhong Duan Date: Thu Aug 16 09:31:57 2018 +0200 x86/mmcfg: remove redundant code in pci_mmcfg_reject_broken() No functional change. Signed-off-by: Zhenzhong Duan Acked-by: Jan Beulich commit a9e9837f54973ac45488c24e93ed34cbf20e4c66 Author: Jan Beulich Date: Thu Aug 16 09:30:59 2018 +0200 gnttab/ARM: properly implement gnttab_create_status_page() Prevent the "BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames() from triggering. Reported-by: 王磊 Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini commit 7626edeaca972e3e823535dcc44338f6b2f0b21f Author: Paul Durrant Date: Thu Aug 16 09:27:30 2018 +0200 x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries When emulating a rep I/O operation it is possible that the ioreq will describe a single operation that spans multiple GFNs. This is fine as long as all those GFNs fall within an MMIO region covered by a single device model, but unfortunately the higher levels of the emulation code do not guarantee that. This is something that should almost certainly be fixed, but in the meantime this patch makes sure that MMIO is truncated at GFN boundaries and hence the appropriate device model is re-evaluated for each target GFN. NOTE: This patch does not deal with the case of a single MMIO operation spanning a GFN boundary. That is more complex to deal with and is deferred to a subsequent patch. Signed-off-by: Paul Durrant Convert calculations to be 32-bit only. Signed-off-by: Jan Beulich commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into
[Xen-devel] [linux-4.9 test] 125913: trouble: broken/fail/pass
flight 125913 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/125913/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict broken test-amd64-i386-xl-qemuu-debianhvm-amd64broken test-amd64-i386-xl-qemuu-ovmf-amd64 broken Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 4 host-install(4) broken pass in 125896 test-amd64-i386-xl-qemuu-ovmf-amd64 4 host-install(4) broken pass in 125896 test-amd64-i386-xl-qemuu-debianhvm-amd64 4 host-install(4) broken pass in 125896 test-amd64-amd64-xl-qemuu-debianhvm-amd64 16 guest-localmigrate/x10 fail in 125896 pass in 125913 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 125896 pass in 125913 test-armhf-armhf-libvirt-raw 7 xen-boot fail pass in 125896 test-amd64-i386-xl-qemut-debianhvm-amd64 10 debian-hvm-install fail pass in 125896 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail pass in 125896 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail in 125896 like 125183 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail in 125896 never pass test-armhf-armhf-libvirt-raw 12 migrate-support-check fail in 125896 never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-check fail in 125896 never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125183 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125183 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125183 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125183 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-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail 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-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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail 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-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linux8f21ecb4249a0914aea08bef1befca9019a3b44b baseline version: linux060744011e93679f03932f050619744be895b772 Last test of basis 125183 2018-07-15 16:46:53 Z 32 days Failing since125271
Re: [Xen-devel] [PATCH] x86/mmcfg/drhd: Move acpi_mmcfg_init() before calling acpi_parse_dmar()
在 2018/8/16 18:37, Jan Beulich 写道: On 16.08.18 at 11:13, wrote: On 2018/8/16 15:10, Jan Beulich wrote: Have you investigated the alternative of deferring acpi_dmar_init() to a later point, or at least the part of it that needs to do PCI config space accesses? I'm not currently convinced the device scope parsing needs to happen that early: Neither iommu_supports_eim() nor iommu_enable_x2apic_IR() look to depend on that information at the first glance, and I think these are the routines that require (part of) the DMAR parsing to happen early. I moved acpi_mmcfg_init() ahead of acpi_boot_init() because below code sequence depending on pci_mmcfg_virt being correctly setup. acpi_dmar_init ->acpi_parse_dmar ->acpi_parse_one_drhd ->acpi_parse_dev_scope ->pci_conf_read8 ->pci_mmcfg_read ->pci_dev_base ->get_virt Have you read my previous response in full? I'm specifically asking whether the device scope parsing (i.e. acpi_parse_dev_scope()) really needs to happen as early as it does now. Without that, the dependency on MMCFG accesses working would go away. I'll move acpi_dmar_init() to later point to have a test as acpi_parse_one_drhd, acpi_parse_one_rmrr and acpi_parse_one_atsr all called acpi_parse_dev_scope. --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1493,6 +1493,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) generic_apic_probe(); +pt_pci_init(); + +acpi_mmcfg_init(); + acpi_boot_init(); With the dependency being _in_ acpi_boot_init(), the invocation of acpi_mmcfg_init() should now be moved there. Yes, I feel better to move pt_pci_init() and acpi_mmcfg_init() in acpi_boot_init() before acpi_mmcfg_init(). I didn't say move both, did I? That said, now having looked at what it actually does, I think you want to rename it if the suggested alternative route can't be used, as in particular the pt_ prefix is quite misleading then. Once that has happened, moving the invocation perhaps even _into_ acpi_mcfg_init() Understood. Personly I like this way more as pt_pci_init() and acpi_mmcfg_init() only initialized some global variable and harmless to move ahead. Also acpi_mcfg_init from its name is suitable to move in acpi_boot_init() Thanks Zhenzhong ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/mmcfg/drhd: Move acpi_mmcfg_init() before calling acpi_parse_dmar()
在 2018/8/16 18:42, Jan Beulich 写道: On 16.08.18 at 11:30, wrote: On 2018/8/16 17:13, Zhenzhong Duan wrote: --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1493,6 +1493,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) generic_apic_probe(); +pt_pci_init(); + +acpi_mmcfg_init(); + acpi_boot_init(); With the dependency being _in_ acpi_boot_init(), the invocation of acpi_mmcfg_init() should now be moved there. Yes, I feel better to move pt_pci_init() and acpi_mmcfg_init() in acpi_boot_init() before acpi_mmcfg_init(). Any more comments? I see acpi_boot_init() is empty func when CONFIG_ACPI_BOOT isn't set. Do we support disabling this config option? If yes, I think above change will break non-acpi case. I'm sorry, but I'm lost: grep produces no single hit on my tree when looking for ACPI_BOOT. Are you looking at some older tree? Even when considering ACPI - that Kconfig option exists only for ARM's purposes right now. If you were to make it user selectable, I think you'd first have to fix a number of build issues in case it got turned off. Sorry, I wrongly looked into oracle internal branch. In upstream, it's CONFIG_ACPI. It looks not an issue as you said CONFIG_ACPI is only for ARM. Thanks Zhenzhong ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Xen-users] Future of 32-bit PV support
Hi Juergen, As this was also addressed to -user I'm going to assume that you do want user response as well. On Thu, Aug 16, 2018 at 08:17:13AM +0200, Juergen Gross wrote: > We'd like to evaluate whether anyone would see problems with: > > - deprecating 32-bit PV guest support in Xen, meaning that we'd > eventually switch to support 32-bit PV guests only via PV-shim from > Xen 4.12 or 4.13 Although amd64 has been the default for us for many years, at the moment we still have 64% of our customers running 32-bit PV. If there remains a way for us to boot them through PV-shim and then pvgrub2 with no functional changes and no work inside the guest then that's fine, we'll adapt. > - dropping 32-bit PV support from upstream Linux kernel, resulting in > current 32-bit PV guests no longer being able to upgrade to the newest > kernel version any longer I doubt there is any technical reason why they can't switch to 64-bit, it's just that in the majority of cases that involves a complete reinstall and the users just haven't bothered to. If they are forced to switch because an impending kernel update will leave them with a kernel that doesn't boot, they are going to be upset that they are forced to reinstall their guest, or switch to a 64-bit kernel with their existing 32-bit userland. It will of course help if they have plenty of warning that they need to make the switch. But unless we're talking 2+ years of warning I'm sure there will be some who will be unhappy. I was hoping to transition to PVH guests as soon as possible, but last time I looked into it there was a problem booting the stable Linux kernel under PVH, and also no support in grub2. Will it remain possible to boot a 32-bit Linux guest in PVH mode? If so, could the final removal of 32-bit PV in the Linux kernel be held off until there is: 1) a kernel shipping in Debian stable, Ubuntu LTS and CentOS that boots under PVH, and; 2) support in grub2 so I can build a grub image that boots under PVH? If grub PVH support is not going to happen, what is the roadmap for user-specified guest kernels under PVH? > - is there any Linux distribution still shipping 32-bit PV-capable > systems? Debian stable 32-bit kernels still boot under PV, as do Ubuntu 18.04 LTS ones. Ubuntu LTS releases are supposed to be supported (by Canonical) for 5 years, and while of course Xen does not fall under the category of software that they support, there will be people sticking with 18.04 LTS as long as they can. I'm not saying that people running 32-bit PV Ubuntu 18.04 are right to expect that to continue being supported until 2023. I'm just saying that human nature dictates that those sorts of expectations will exist. It will help a lot if there is an easy way for us to switch them from 32-bit PV to PVH, while still letting them install their own kernels. Cheers, Andy ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 126009: regressions - FAIL
flight 126009 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/126009/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl 12 guest-start fail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen 3e4ec07e14bce81f6ae22c31ff1302d1f297a226 baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z1 days Failing since125928 2018-08-15 19:00:49 Z1 days 12 attempts Testing same since 126009 2018-08-16 20:00:24 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Ian Jackson Jan Beulich Julien Grall Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl fail 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 Not pushing. commit 3e4ec07e14bce81f6ae22c31ff1302d1f297a226 Author: Andrew Cooper Date: Thu Aug 16 16:26:22 2018 +0100 x86/setup: Avoid OoB E820 lookup when calculating the L1TF safe address A number of corner cases (most obviously, no-real-mode and no Multiboot memory map) can end up with e820_raw.nr_map being 0, at which point the L1TF calculation will underflow. Spotted by Coverity. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Reviewed-by: Wei Liu commit afc3f910d3434b540e4e4f51d9fd2723aea22fa2 Author: Jan Beulich Date: Thu Aug 16 00:49:29 2018 -0600 libxl: fix ARM build after 54ed251dc7 Commit "tools: Rework xc_domain_create() to take a full xen_domctl_createdomain" failed to replace one further instance of xc_config in libxl__arch_domain_save_config(). Signed-off-by: Jan Beulich Acked-by: Andrew Cooper Acked-by: Ian Jackson commit 70d8543950ad045fddcb78ae11302e713ef09c76 Author: Zhenzhong Duan Date: Thu Aug 16 09:31:57 2018 +0200 x86/mmcfg: remove redundant code in pci_mmcfg_reject_broken() No functional change. Signed-off-by: Zhenzhong Duan Acked-by: Jan Beulich commit a9e9837f54973ac45488c24e93ed34cbf20e4c66 Author: Jan Beulich Date: Thu Aug 16 09:30:59 2018 +0200 gnttab/ARM: properly implement gnttab_create_status_page() Prevent the "BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames() from triggering. Reported-by: 王磊 Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini commit 7626edeaca972e3e823535dcc44338f6b2f0b21f Author: Paul Durrant Date: Thu Aug 16 09:27:30 2018 +0200 x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries When emulating a rep I/O operation it is possible that the ioreq will describe a single operation that spans multiple GFNs. This is fine as long as all those GFNs fall within an MMIO region covered by a single device model, but unfortunately the higher levels of the emulation code do not guarantee that. This is something that should almost certainly be fixed, but in the meantime this patch makes sure that MMIO is truncated at GFN boundaries and hence the appropriate device model is re-evaluated for each target GFN. NOTE: This patch does not deal with the case of a single MMIO operation spanning a GFN boundary. That is more complex to deal with and is deferred to a subsequent patch. Signed-off-by: Paul Durrant Convert calculations to be 32-bit only. Signed-off-by: Jan Beulich commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into
[Xen-devel] [xen-unstable test] 125912: regressions - FAIL
flight 125912 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/125912/ 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. 125691 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 15 guest-saverestore.2 fail REGR. vs. 125691 test-amd64-i386-xl-shadow 20 guest-start/debian.repeat fail REGR. vs. 125691 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125691 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125691 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125691 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125691 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125691 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125691 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125691 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125691 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 125691 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-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-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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-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-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-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail 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 aa67b97ed34279c43a43d9ca46727b5746caa92e baseline version: xen 1f7574763cbb2c85825b8cc4d81f386e767a476f Last test of basis 125691 2018-07-30 21:37:12 Z 17 days Failing since125716 2018-08-01 03:36:29 Z 15 days9 attempts Testing same since 125912 2018-08-15 02:23:22 Z1 days1 attempts People who touched revisions under test: Alexandru Isaila Andrew Cooper Christian Lindig Daniel Kiper Doug Goldstein George Dunlap Jan Beulich Juergen Gross Julien Grall Kevin Tian Marek Marczykowski-Górecki Norbert Manthey Paul Durrant Razvan Cojocaru Roger Pau Monné Simon Gaiser Stefano Stabellini Wei Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64-xtf pass build-amd64 pass build-armhf
Re: [Xen-devel] [PATCH] x86: use VMLOAD for PV context switch
On Tue, Jul 10, 2018 at 04:14:11AM -0600, Jan Beulich wrote: > Having noticed that VMLOAD alone is about as fast as a single of the > involved WRMSRs, I thought it might be a reasonable idea to also use it > for PV. Measurements, however, have shown that an actual improvement can > be achieved only with an early prefetch of the VMCB (thanks to Andrew > for suggesting to try this), which I have to admit I can't really > explain. This way on my Fam15 box context switch takes over 100 clocks > less on average (the measured values are heavily varying in all cases, > though). > > This is intentionally not using a new hvm_funcs hook: For one, this is > all about PV, and something similar can hardly be done for VMX. > Furthermore the indirect to direct call patching that is meant to be > applied to most hvm_funcs hooks would be ugly to make work with > functions having more than 6 parameters. > > Signed-off-by: Jan Beulich I have confirmed it with a senior hardware engineer and using vmload in this fashion is safe and recommended for performance. As far as using vmload with PV. Acked-by: Brian Woods -- Brian Woods ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] libxl/arm: Fix build on arm64 + acpi w/ gcc 8.2
Add zero-padding to #defined ACPI table strings that are copied. Provides sufficient characters to satisfy the length required to fully populate the destination and prevent array-bounds warnings. Add BUILD_BUG_ON sizeof checks for compile-time length checking. Signed-off-by: Christopher Clark Reviewed-by: Stefano Stabellini --- v2: add BUILD_BUG_ON length checks, requested by Wei. v1: Please add this patch to the backport list for the next minor 4.11 release. Prior to this: gcc 8.2 objects to memcpy past bounds: | libxl_arm_acpi.c: In function 'make_acpi_header': | libxl_arm_acpi.c:208:5: error: 'memcpy' forming offset [5, 6] is out of the bounds [0, 4] [-Werror=array-bounds] | memcpy(h->oem_id, ACPI_OEM_ID, sizeof(h->oem_id)); | ^ | libxl_arm_acpi.c:209:5: error: 'memcpy' forming offset [5, 8] is out of the bounds [0, 4] [-Werror=array-bounds] | memcpy(h->oem_table_id, ACPI_OEM_TABLE_ID, sizeof(h->oem_table_id)); | ^~~ | libxl_arm_acpi.c:211:5: error: 'memcpy' forming offset 4 is out of the bounds [0, 3] [-Werror=array-bounds] | memcpy(h->asl_compiler_id, ACPI_ASL_COMPILER_ID, | ^~~~ | sizeof(h->asl_compiler_id)); | ~~~ | In function 'make_acpi_rsdp.isra.4', | inlined from 'libxl__prepare_acpi' at libxl_arm_acpi.c:389:5: | libxl_arm_acpi.c:193:5: error: 'memcpy' forming offset [5, 6] is out of the bounds [0, 4] [-Werror=array-bounds] | memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(rsdp->oem_id)); | ^~~ tools/libxl/libxl_arm_acpi.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c index 636f724..ba874c3 100644 --- a/tools/libxl/libxl_arm_acpi.c +++ b/tools/libxl/libxl_arm_acpi.c @@ -48,9 +48,9 @@ extern const unsigned char dsdt_anycpu_arm[]; _hidden extern const int dsdt_anycpu_arm_len; -#define ACPI_OEM_ID "Xen" -#define ACPI_OEM_TABLE_ID "ARM" -#define ACPI_ASL_COMPILER_ID "XL" +#define ACPI_OEM_ID "Xen\0\0" +#define ACPI_OEM_TABLE_ID "ARM\0\0\0\0" +#define ACPI_ASL_COMPILER_ID "XL\0" enum { RSDP, @@ -190,6 +190,7 @@ static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom, struct acpi_table_rsdp *rsdp = (void *)dom->acpi_modules[0].data + offset; memcpy(rsdp->signature, "RSD PTR ", sizeof(rsdp->signature)); +BUILD_BUG_ON(sizeof(ACPI_OEM_ID) != sizeof(rsdp->oem_id)); memcpy(rsdp->oem_id, ACPI_OEM_ID, sizeof(rsdp->oem_id)); rsdp->length = acpitables[RSDP].size; rsdp->revision = 0x02; @@ -205,9 +206,12 @@ static void make_acpi_header(struct acpi_table_header *h, const char *sig, memcpy(h->signature, sig, 4); h->length = len; h->revision = rev; +BUILD_BUG_ON(sizeof(ACPI_OEM_ID) != sizeof(h->oem_id)); memcpy(h->oem_id, ACPI_OEM_ID, sizeof(h->oem_id)); +BUILD_BUG_ON(sizeof(ACPI_OEM_TABLE_ID) != sizeof(h->oem_table_id)); memcpy(h->oem_table_id, ACPI_OEM_TABLE_ID, sizeof(h->oem_table_id)); h->oem_revision = 0; +BUILD_BUG_ON(sizeof(ACPI_ASL_COMPILER_ID) != sizeof(h->asl_compiler_id)); memcpy(h->asl_compiler_id, ACPI_ASL_COMPILER_ID, sizeof(h->asl_compiler_id)); h->asl_compiler_revision = 0; -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2 2/2] x86/spec-ctrl: add support for modifying SSBD VIA LS_CFG MSR
On Wed, Aug 15, 2018 at 10:00:48AM -0600, Jan Beulich wrote: > >>> On 09.08.18 at 21:42, wrote: > > --- a/xen/arch/x86/cpu/amd.c > > +++ b/xen/arch/x86/cpu/amd.c > > @@ -611,14 +611,9 @@ static void init_amd(struct cpuinfo_x86 *c) > > ssbd_amd_ls_cfg_mask = 1ull << bit; > > } > > > > - if (ssbd_amd_ls_cfg_mask && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) { > > + if (ssbd_amd_ls_cfg_mask && !rdmsr_safe(MSR_AMD64_LS_CFG, value)) > > if (!boot_cpu_has(X86_FEATURE_SSBD_AMD_LS_CFG)) > > setup_force_cpu_cap(X86_FEATURE_SSBD_AMD_LS_CFG); > > If the inner if() was not to go away altogether in patch 1, please > fold two successive if()-s like these. > > > --- a/xen/arch/x86/spec_ctrl.c > > +++ b/xen/arch/x86/spec_ctrl.c > > First of all - I'm not convinced some of the AMD specific code here > wouldn't better live in cpu/amd.c. Well, by that logic, a lot of the other logic could go in cpu/intel.c. It has to do with SSBD and when AMD's processors support it via the SPEC_CTRL MSR, the support for SSBD will get merged together in spec_ctrl.c and if that's the case, it makes sense to have all the SSBD code together. IMO > > @@ -50,7 +51,16 @@ bool __initdata bsp_delay_spec_ctrl; > > uint8_t __read_mostly default_xen_spec_ctrl; > > uint8_t __read_mostly default_spec_ctrl_flags; > > > > +/* for SSBD support for AMD via LS_CFG */ > > +#define SSBD_AMD_MAX_SOCKET 2 > > +struct ssbd_amd_ls_cfg_smt_status { > > +spinlock_t lock; > > +uint32_t mask; > > +} __attribute__ ((aligned (64))); > > Where's this literal 64 coming from? Do you perhaps mean > SMP_CACHE_BYTES? And if this is really needed (as said before, > I think the array would better be dynamically allocated than having > compile time determined fixed size), then please don't open-code > __aligned(). It's the cache coherency size. The SMP_CACHE_BYTES is 128 bytes IIRC. I was trying to save space since the struct is so small it would double the size. That can be changed though. > > +bool __read_mostly ssbd_amd_smt_en = false; > > +bool __read_mostly default_xen_ssbd_amd_ls_cfg_en = false; > > uint64_t __read_mostly ssbd_amd_ls_cfg_mask = 0ull; > > +struct ssbd_amd_ls_cfg_smt_status > > *ssbd_amd_smt_status[SSBD_AMD_MAX_SOCKET] = {NULL}; > > Several further pointless initializers. ssbd_amd_ls_cfg_mask -> needs to be initialized, due to how it gets set ssbd_amd_ls_cfg_smt_status -> needs to be initialized, unless xalloc sets it as NULL on failure to alloc default_xen_ssbd_amd_ls_cfg_en -> needs to be initialized of an else added to an if statement ssbd_amd_smt_en -> like the above If you want default_xen_ssbd_amd_ls_cfg_en and ssbd_amd_smt_en to be not be initialized, I can add code to do that. > > +static void ssbd_amd_ls_cfg_set_smt(bool enable_ssbd) > > +{ > > +uint32_t socket, core, thread; > > +uint64_t enable_mask; > > +uint64_t ls_cfg; > > +struct ssbd_amd_ls_cfg_smt_status *status; > > +const struct cpuinfo_x86 *c = _cpu_data; > > + > > +socket = c->phys_proc_id; > > +core = c->cpu_core_id; > > +thread = c->apicid & (c->x86_num_siblings - 1); > > +status = ssbd_amd_smt_status[socket] + core; > > +enable_mask = (1ull << thread); > > + > > +spin_lock(>lock); > > + > > +if ( enable_ssbd ) > > +{ > > +if ( !(status->mask & enable_mask) ) > > So with ->mask being uint32_t, why does enable_mask need to be > uint64_t? Even uint32_t seems way more than needed, but there's > certainly no point going below this. Just that, as expressed before, > you should please use "unsigned int" in favor of uint32_t everywhere, > unless you really need something that's exactly 32-bits wide. Because when changing the variable types from __32 etc, I confused it with the enable mask for the LS_CFG reg. I'll change them. > > +{ > > +status->mask |= enable_mask; > > +rdmsrl(MSR_AMD64_LS_CFG, ls_cfg); > > +if ( !(ls_cfg & ssbd_amd_ls_cfg_mask) ) > > +{ > > +ls_cfg |= ssbd_amd_ls_cfg_mask; > > +wrmsrl(MSR_AMD64_LS_CFG, ls_cfg); > > +} > > +} > > +} > > +else > > +{ > > +if ( status->mask & enable_mask ) > > +{ > > +status->mask &= ~enable_mask; > > +rdmsrl(MSR_AMD64_LS_CFG, ls_cfg); > > +if ( (ls_cfg & ssbd_amd_ls_cfg_mask) && (status->mask == 0) ) > > Please prefer the shorter ! over " == 0". > > > +{ > > +ls_cfg &= ~ssbd_amd_ls_cfg_mask; > > +wrmsrl(MSR_AMD64_LS_CFG, ls_cfg); > > +} > > +} > > +} > > + > > +spin_unlock(>lock); > > +} > > + > > +void ssbd_amd_ls_cfg_set(bool enable_ssbd) > > +{ > > +if ( !ssbd_amd_ls_cfg_mask || > > + !boot_cpu_has(X86_FEATURE_SSBD_AMD_LS_CFG) ) { > > +dprintk(XENLOG_ERR, "SSBD AMD
[Xen-devel] [libvirt bisection] complete build-i386-libvirt
branch xen-unstable xenbranch xen-unstable job build-i386-libvirt testid libvirt-build Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib https://git.savannah.gnu.org/git/gnulib.git/ Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: libvirt git://libvirt.org/libvirt.git Bug introduced: 60d9ad6f1e42618fce10baeb0f02c35e5ebd5b24 Bug not present: 9b837963c54ac50d7faae63184d32a0fb599d1b0 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/126006/ commit 60d9ad6f1e42618fce10baeb0f02c35e5ebd5b24 Author: Michal Privoznik Date: Mon Jun 4 06:51:50 2018 +0200 configure: Require GnuTLS We are building with GnuTLS everywhere because GnuTLS is widely available. Also, it is desirable to prefer cryptographically strong PRNG over "/dev/urandom" which is just a fallback. Signed-off-by: Michal Privoznik Reviewed-by: Daniel P. Berrangé For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/libvirt/build-i386-libvirt.libvirt-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/libvirt/build-i386-libvirt.libvirt-build --summary-out=tmp/126006.bisection-summary --basis-template=123814 --blessings=real,real-bisect libvirt build-i386-libvirt libvirt-build Searching for failure / basis pass: 125903 fail [host=debina1] / 123814 [host=fiano1] 123575 [host=fiano1] 123456 [host=debina0] 123391 ok. Failure / basis pass flights: 125903 / 123391 (tree with no url: minios) (tree with no url: ovmf) (tree with no url: seabios) Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_gnulib https://git.savannah.gnu.org/git/gnulib.git/ Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest b0451117b399df8107340dee8b653cb48e8da1c8 68df637b5f1b5c10370f6981d2a43a5cf74368df 16e5b0787687d8904dad2c026107409eb9bfcb95 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a 1f7574763cbb2c85825b8cc4d81f386e767a476f Basis pass 57d6df39bd7eb8166fee68f4b6da03c0cb0802bf d6397dde2e127e246e3eeb5254a21f42cac783c8 16e5b0787687d8904dad2c026107409eb9bfcb95 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a fc5805daef091240cd5fc06634a8bcdb2f3bb843 Generating revisions with ./adhoc-revtuple-generator git://libvirt.org/libvirt.git#57d6df39bd7eb8166fee68f4b6da03c0cb0802bf-b0451117b399df8107340dee8b653cb48e8da1c8 https://git.savannah.gnu.org/git/gnulib.git/#d6397dde2e127e246e3eeb5254a21f42cac783c8-68df637b5f1b5c10370f6981d2a43a5cf74368df https://gitlab.com/keycodemap/keycodemapdb.git#16e5b0787687d8904dad2c026107409eb9bfcb95-16e5b0787687d8904dad2c026107409eb9bfcb95 git://xenbits.xen.org/qemu-xen-traditional.git#c8ea0457495342c417c3dc033bba25148b279f60-c8ea0457495342c417c3dc033bba25148b279f60 git://xenbits.xen.org/qemu-xen.git#43139135a8938de44f66333831d3a8655d07663a-43139135a8938de44f66333831d3a8655d07663a git://xenbits.xen.org/xen.git#fc5805daef091240cd5fc06634a8bcdb2f3bb843-1f7574763cbb2c85825b8cc4d81f386e767a476f Loaded 3358 nodes in revision graph Searching for test results: 123391 pass 57d6df39bd7eb8166fee68f4b6da03c0cb0802bf d6397dde2e127e246e3eeb5254a21f42cac783c8 16e5b0787687d8904dad2c026107409eb9bfcb95 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a fc5805daef091240cd5fc06634a8bcdb2f3bb843 123373 [host=debina0] 123416 [host=debina0] 123456 [host=debina0] 123575 [host=fiano1] 123840 [host=debina0] 123814 [host=fiano1] 123876 fail irrelevant 123922 [host=debina0] 123904 [host=debina0] 123924 pass 57d6df39bd7eb8166fee68f4b6da03c0cb0802bf d6397dde2e127e246e3eeb5254a21f42cac783c8 16e5b0787687d8904dad2c026107409eb9bfcb95 c8ea0457495342c417c3dc033bba25148b279f60 43139135a8938de44f66333831d3a8655d07663a fc5805daef091240cd5fc06634a8bcdb2f3bb843 123906 [host=debina0] 123934 fail irrelevant 123925 fail irrelevant 123908 [host=debina0] 123916 [host=debina0] 123926 pass irrelevant 123917 [host=debina0] 123927 pass irrelevant 123902 [host=debina0] 123918 [host=debina0] 123938 pass irrelevant 123920 [host=debina0] 123921 [host=debina0] 123928 fail irrelevant 123930 fail irrelevant 123931 fail irrelevant 123935 pass irrelevant 123929 [host=debina0] 123981 [host=debina0] 124034 fail irrelevant 124060 [host=debina0] 124095 fail irrelevant 124159 [host=debina0] 124188 fail irrelevant 124208 [host=debina0] 124239 fail irrelevant
Re: [Xen-devel] [PATCH v3 24/25] xen/vpl011: buffer out chars when the backend is xen
On Mon, 13 Aug 2018, Julien Grall wrote: > Hi, > > On 01/08/18 00:28, Stefano Stabellini wrote: > > To avoid mixing the output of different domains on the console, buffer > > the output chars and print line by line. Unless the domain has input > > from the serial, in which case we want to print char by char for a > > smooth user experience. > > > > The size of SBSA_UART_OUT_BUF_SIZE is arbitrary. 90 should be large > > enough to accommodate the length of most lines of output (typically they > > are limited to 80 characters on Unix systems), plus one extra char for > > the string terminator. > > How about using the same value as vuart (e.g VUART_BUT_SIZE) instead? So we > buffer the same way for the vpl011? Yes, I can do that. > > Signed-off-by: Stefano Stabellini > > --- > > xen/arch/arm/vpl011.c| 21 ++--- > > xen/include/asm-arm/vpl011.h | 3 +++ > > 2 files changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c > > index f206c61..8137371 100644 > > --- a/xen/arch/arm/vpl011.c > > +++ b/xen/arch/arm/vpl011.c > > @@ -28,6 +28,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -85,12 +86,26 @@ static void vpl011_write_data_xen(struct domain *d, > > uint8_t data) > > { > > unsigned long flags; > > struct vpl011 *vpl011 = >arch.vpl011; > > +struct vpl011_xen_backend *intf = vpl011->xen; > > +struct domain *input = console_input_domain(); > > VPL011_LOCK(d, flags); > > -printk("%c", data); > > -if (data == '\n') > > -printk("DOM%u: ", d->domain_id); > > +intf->out[intf->out_prod++] = data; > > +if ( d == input && intf->out_prod == 1 ) > > +{ > > +printk("%c", data); > > +if ( data == '\n' ) > > +printk("DOM%u: ", d->domain_id); > > +intf->out_prod = 0; > > See my remark on the patch implementing vpl011_write_data_xen. With this patch, the missing "DOM" at the beginning cannot happen anymore due to the out buffering. Theoretically it can still happen by switching to DOM1 before DOM1 prints anything, but it is impossible to do in practice. Even in this theoretical scenario, the user would still get the useful "Switching to DOM1" message, that would help clarify what is going on. Thus, my preference is to avoid making the code more complex to fix this issue. > > +} else if ( d == input || > > Coding style: > > } > else if > { I'll fix > > +intf->out_prod == SBSA_UART_OUT_BUF_SIZE - 1 || > > +data == '\n' ) > > +{ > > +intf->out[intf->out_prod++] = '\0'; > > +printk("DOM%u: %s", d->domain_id, intf->out); > > +intf->out_prod = 0; > > +} > > The code is quite difficult to read. It would be easier to differentiate > (domain == input vs domain != input) even if it means a bit of duplication. OK, I can rearrange the code that way. For example: if ( d == input ){ if ( intf->out_prod == 1 ) { printk("%c", data); if ( data == '\n' ) printk("DOM%u: ", d->domain_id); intf->out_prod = 0; } else { if ( data != '\n' ) intf->out[intf->out_prod++] = '\n'; intf->out[intf->out_prod++] = '\0'; printk("DOM%u: %s", d->domain_id, intf->out); intf->out_prod = 0; } } else { if ( intf->out_prod == SBSA_UART_OUT_BUF_SIZE - 2 || data == '\n' ) { if ( data != '\n' ) intf->out[intf->out_prod++] = '\n'; intf->out[intf->out_prod++] = '\0'; printk("DOM%u: %s", d->domain_id, intf->out); intf->out_prod = 0; } } Is it better? > You also don't handle all the cases. > > For the input domain, I don't think you want to print the domain in front. > Instead I would rely on the "Switch to ...". Actually it is very convenient to know at any given time which domain you are talking to. I couldn't find any problems with the prefix, even using VIM, etc. I would rather keep the "DOM" string around. > This would avoid the problem > where DomB needs to have his line printed while it is not the console input. > If this happens in the middle of DomA, then you are loosing track what's going > on. I don't understand the example: if DOM1 has input, and DOM2 prints something, the DOM2 output will be prepended by "DOM2", avoiding any confusion. What am I missing? > Also if you have the buffer full, you will write the line without a newline. > So, the next line is going to be wrong. In that case, you need to add a > newline before printed. I think you want to look at what we did in > vuart_print_char. Yes, this is a problem. I'll do the same as vuart_print_char. > > vpl011->uartris |= TXI; > > vpl011->uartfr &= ~TXFE; > > diff
Re: [Xen-devel] [PATCH v3 22/25] xen/arm: Allow vpl011 to be used by DomU
On Mon, 13 Aug 2018, Julien Grall wrote: > Hi Stefano, > > On 01/08/18 00:28, Stefano Stabellini wrote: > > Make vpl011 being able to be used without a userspace component in Dom0. > > In that case, output is printed to the Xen serial and input is received > > from the Xen serial one character at a time. > > > > Call domain_vpl011_init during construct_domU if vpl011 is enabled. > > > > Introduce a new ring struct with only the ring array to avoid a waste of > > memory. Introduce separate read_data and write_data functions for > > initial domains: vpl011_write_data_xen is very simple and just writes > > to the console, while vpl011_read_data_xen is a duplicate of > > vpl011_read_data. Although textually almost identical, we are forced to > > duplicate the functions because the struct layout is different. > > > > Output characters are printed one by one, potentially leading to > > intermixed output of different domains on the console. A follow-up patch > > will solve the issue by introducing buffering. > > > > Signed-off-by: Stefano Stabellini > > --- > > Changes in v3: > > - add in-code comments > > - improve existing comments > > - remove ifdef around domain_vpl011_init in construct_domU > > - add ASSERT > > - use SBSA_UART_FIFO_SIZE for in buffer size > > - rename ring_enable to backend_in_domain > > - rename struct xencons_in to struct vpl011_xen_backend > > - rename inring field to xen > > - rename helper functions accordingly > > - remove unnecessary stub implementation of vpl011_rx_char > > - move vpl011_rx_char_xen within the file to avoid the need of a forward > >declaration of vpl011_data_avail > > - fix small bug in vpl011_rx_char_xen: increment in_prod before using it > >to check xencons_queued. > > > > Changes in v2: > > - only init if vpl011 > > - rename vpl011_read_char to vpl011_rx_char > > - remove spurious change > > - fix coding style > > - use different ring struct > > - move the write_data changes to their own function > >(vpl011_write_data_noring) > > - duplicate vpl011_read_data > > --- > > xen/arch/arm/domain_build.c | 9 +- > > xen/arch/arm/vpl011.c| 198 > > ++- > > xen/include/asm-arm/vpl011.h | 8 ++ > > 3 files changed, 192 insertions(+), 23 deletions(-) > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > index f9fa484..0888a76 100644 > > --- a/xen/arch/arm/domain_build.c > > +++ b/xen/arch/arm/domain_build.c > > @@ -2638,7 +2638,14 @@ static int __init construct_domU(struct domain *d, > > struct dt_device_node *node) > > if ( rc < 0 ) > > return rc; > > -return __construct_domain(d, ); > > +rc = __construct_domain(d, ); > > +if ( rc < 0 ) > > +return rc; > > + > > +if ( kinfo.vpl011 ) > > +rc = domain_vpl011_init(d, NULL); > > + > > +return rc; > > } > > void __init create_domUs(void) > > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c > > index 725a203..f206c61 100644 > > --- a/xen/arch/arm/vpl011.c > > +++ b/xen/arch/arm/vpl011.c > > @@ -77,6 +77,91 @@ static void vpl011_update_interrupt_status(struct domain > > *d) > > #endif > > } > > +/* > > + * vpl011_write_data_xen writes chars from the vpl011 out buffer to the > > + * console. Only to be used when the backend is Xen. > > + */ > > +static void vpl011_write_data_xen(struct domain *d, uint8_t data) > > +{ > > +unsigned long flags; > > +struct vpl011 *vpl011 = >arch.vpl011; > > + > > +VPL011_LOCK(d, flags); > > + > > +printk("%c", data); > > +if (data == '\n') > > +printk("DOM%u: ", d->domain_id); > > There are a problem in this code. The first line of a domain will always > printed without "DOM%u: " in front. This means you don't really know where it > is coming from until you get the second line. This problem is solved by the follow-up patch that introduces characters buffering. I'll mention it in the commit message. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 126000: regressions - FAIL
flight 126000 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/126000/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl 12 guest-start fail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen afc3f910d3434b540e4e4f51d9fd2723aea22fa2 baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z1 days Failing since125928 2018-08-15 19:00:49 Z1 days 11 attempts Testing same since 125967 2018-08-16 12:00:36 Z0 days3 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Ian Jackson Jan Beulich Julien Grall Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl fail 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 Not pushing. commit afc3f910d3434b540e4e4f51d9fd2723aea22fa2 Author: Jan Beulich Date: Thu Aug 16 00:49:29 2018 -0600 libxl: fix ARM build after 54ed251dc7 Commit "tools: Rework xc_domain_create() to take a full xen_domctl_createdomain" failed to replace one further instance of xc_config in libxl__arch_domain_save_config(). Signed-off-by: Jan Beulich Acked-by: Andrew Cooper Acked-by: Ian Jackson commit 70d8543950ad045fddcb78ae11302e713ef09c76 Author: Zhenzhong Duan Date: Thu Aug 16 09:31:57 2018 +0200 x86/mmcfg: remove redundant code in pci_mmcfg_reject_broken() No functional change. Signed-off-by: Zhenzhong Duan Acked-by: Jan Beulich commit a9e9837f54973ac45488c24e93ed34cbf20e4c66 Author: Jan Beulich Date: Thu Aug 16 09:30:59 2018 +0200 gnttab/ARM: properly implement gnttab_create_status_page() Prevent the "BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames() from triggering. Reported-by: 王磊 Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini commit 7626edeaca972e3e823535dcc44338f6b2f0b21f Author: Paul Durrant Date: Thu Aug 16 09:27:30 2018 +0200 x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries When emulating a rep I/O operation it is possible that the ioreq will describe a single operation that spans multiple GFNs. This is fine as long as all those GFNs fall within an MMIO region covered by a single device model, but unfortunately the higher levels of the emulation code do not guarantee that. This is something that should almost certainly be fixed, but in the meantime this patch makes sure that MMIO is truncated at GFN boundaries and hence the appropriate device model is re-evaluated for each target GFN. NOTE: This patch does not deal with the case of a single MMIO operation spanning a GFN boundary. That is more complex to deal with and is deferred to a subsequent patch. Signed-off-by: Paul Durrant Convert calculations to be 32-bit only. Signed-off-by: Jan Beulich commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into evtchn_init() ... rather than setting it up once domain_create() has completed. This involves constructing a default value for dom0. No practical change in functionality. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Acked-by: Julien Grall commit 4a83497635056d33fe20ef705f35617b1003a276 Author: Andrew Cooper Date: Tue Feb 27 17:39:37 2018 + xen/domctl: Merge set_max_evtchn into createdomain set_max_evtchn is somewhat weird. It was
Re: [Xen-devel] [PATCH 0/6] x86/mm: Minor non-functional cleanup
At 19:34 +0100 on 15 Aug (1534361671), Andrew Cooper wrote: > Minor cleanup which has accumulated while doing other work. No functional > change anywhere. > > Andrew Cooper (6): > x86/mm: Use mfn_eq()/mfn_add() rather than opencoded variations > x86/shadow: Use more appropriate conversion functions > x86/shadow: Switch shadow_domain.has_fast_mmio_entries to bool > x86/shadow: Use MASK_* helpers for the MMIO fastpath PTE manipulation > x86/shadow: Clean up the MMIO fastpath helpers > x86/shadow: Use mfn_t in shadow_track_dirty_vram() Reviewed-by: Tim Deegan (with the one correction that Roger asked for in patch 1/6) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 4.18 06/22] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits
4.18-stable review patch. If anyone has any objections, please let me know. -- From: M. Vefa Bicakci commit 405c018a25fe464dc68057bbc8014a58f2bd4422 upstream. Commit d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption") has moved the query and calculation of the x86_virt_bits and x86_phys_bits fields of the cpuinfo_x86 struct from the get_cpu_cap function to a new function named get_cpu_address_sizes. One of the call sites related to Xen PV VMs was unfortunately missed in the aforementioned commit. This prevents successful boot-up of kernel versions 4.17 and up in Xen PV VMs if CONFIG_DEBUG_VIRTUAL is enabled, due to the following code path: enlighten_pv.c::xen_start_kernel mmu_pv.c::xen_reserve_special_pages page.h::__pa physaddr.c::__phys_addr physaddr.h::phys_addr_valid phys_addr_valid uses boot_cpu_data.x86_phys_bits to validate physical addresses. boot_cpu_data.x86_phys_bits is no longer populated before the call to xen_reserve_special_pages due to the aforementioned commit though, so the validation performed by phys_addr_valid fails, which causes __phys_addr to trigger a BUG, preventing boot-up. Signed-off-by: M. Vefa Bicakci Reviewed-by: Thomas Gleixner Reviewed-by: Boris Ostrovsky Cc: "Kirill A. Shutemov" Cc: Andy Lutomirski Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: Thomas Gleixner Cc: Boris Ostrovsky Cc: Juergen Gross Cc: xen-devel@lists.xenproject.org Cc: x...@kernel.org Cc: sta...@vger.kernel.org # for v4.17 and up Fixes: d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption") Signed-off-by: Boris Ostrovsky Signed-off-by: Greg Kroah-Hartman --- arch/x86/kernel/cpu/common.c |2 +- arch/x86/kernel/cpu/cpu.h|1 + arch/x86/xen/enlighten_pv.c |3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -905,7 +905,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c) apply_forced_caps(c); } -static void get_cpu_address_sizes(struct cpuinfo_x86 *c) +void get_cpu_address_sizes(struct cpuinfo_x86 *c) { u32 eax, ebx, ecx, edx; --- a/arch/x86/kernel/cpu/cpu.h +++ b/arch/x86/kernel/cpu/cpu.h @@ -46,6 +46,7 @@ extern const struct cpu_dev *const __x86 *const __x86_cpu_dev_end[]; extern void get_cpu_cap(struct cpuinfo_x86 *c); +extern void get_cpu_address_sizes(struct cpuinfo_x86 *c); extern void cpu_detect_cache_sizes(struct cpuinfo_x86 *c); extern void init_scattered_cpuid_features(struct cpuinfo_x86 *c); extern u32 get_scattered_cpuid_leaf(unsigned int level, --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -1259,6 +1259,9 @@ asmlinkage __visible void __init xen_sta get_cpu_cap(_cpu_data); x86_configure_nx(); + /* Determine virtual and physical address sizes */ + get_cpu_address_sizes(_cpu_data); + /* Let's presume PV guests always boot on vCPU with id 0. */ per_cpu(xen_vcpu_id, 0) = 0; ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 4.17 05/21] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits
4.17-stable review patch. If anyone has any objections, please let me know. -- From: M. Vefa Bicakci commit 405c018a25fe464dc68057bbc8014a58f2bd4422 upstream. Commit d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption") has moved the query and calculation of the x86_virt_bits and x86_phys_bits fields of the cpuinfo_x86 struct from the get_cpu_cap function to a new function named get_cpu_address_sizes. One of the call sites related to Xen PV VMs was unfortunately missed in the aforementioned commit. This prevents successful boot-up of kernel versions 4.17 and up in Xen PV VMs if CONFIG_DEBUG_VIRTUAL is enabled, due to the following code path: enlighten_pv.c::xen_start_kernel mmu_pv.c::xen_reserve_special_pages page.h::__pa physaddr.c::__phys_addr physaddr.h::phys_addr_valid phys_addr_valid uses boot_cpu_data.x86_phys_bits to validate physical addresses. boot_cpu_data.x86_phys_bits is no longer populated before the call to xen_reserve_special_pages due to the aforementioned commit though, so the validation performed by phys_addr_valid fails, which causes __phys_addr to trigger a BUG, preventing boot-up. Signed-off-by: M. Vefa Bicakci Reviewed-by: Thomas Gleixner Reviewed-by: Boris Ostrovsky Cc: "Kirill A. Shutemov" Cc: Andy Lutomirski Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: Thomas Gleixner Cc: Boris Ostrovsky Cc: Juergen Gross Cc: xen-devel@lists.xenproject.org Cc: x...@kernel.org Cc: sta...@vger.kernel.org # for v4.17 and up Fixes: d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption") Signed-off-by: Boris Ostrovsky Signed-off-by: Greg Kroah-Hartman --- arch/x86/kernel/cpu/common.c |2 +- arch/x86/kernel/cpu/cpu.h|1 + arch/x86/xen/enlighten_pv.c |3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -883,7 +883,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c) apply_forced_caps(c); } -static void get_cpu_address_sizes(struct cpuinfo_x86 *c) +void get_cpu_address_sizes(struct cpuinfo_x86 *c) { u32 eax, ebx, ecx, edx; --- a/arch/x86/kernel/cpu/cpu.h +++ b/arch/x86/kernel/cpu/cpu.h @@ -46,6 +46,7 @@ extern const struct cpu_dev *const __x86 *const __x86_cpu_dev_end[]; extern void get_cpu_cap(struct cpuinfo_x86 *c); +extern void get_cpu_address_sizes(struct cpuinfo_x86 *c); extern void cpu_detect_cache_sizes(struct cpuinfo_x86 *c); extern int detect_extended_topology_early(struct cpuinfo_x86 *c); extern int detect_ht_early(struct cpuinfo_x86 *c); --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -1258,6 +1258,9 @@ asmlinkage __visible void __init xen_sta get_cpu_cap(_cpu_data); x86_configure_nx(); + /* Determine virtual and physical address sizes */ + get_cpu_address_sizes(_cpu_data); + /* Let's presume PV guests always boot on vCPU with id 0. */ per_cpu(xen_vcpu_id, 0) = 0; ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Future of 32-bit PV support
On Thu, Aug 16, 2018 at 12:55 AM, Juergen Gross wrote: > On 16/08/18 08:51, Jan Beulich wrote: > On 16.08.18 at 08:32, wrote: > >> On Wed, Aug 15, 2018 at 11:17 PM, Juergen Gross > wrote: > >> > >>> In the Xen x86 community call we have been discussing whether anyone > >>> really is depending on 32-bit PV guests. We'd like to evaluate whether > >>> anyone would see problems with: > >>> > >>> - deprecating 32-bit PV guest support in Xen, meaning that we'd > >>> eventually switch to support 32-bit PV guests only via PV-shim from > >>> Xen 4.12 or 4.13 > >>> > >>> - dropping 32-bit PV support from upstream Linux kernel, resulting in > >>> current 32-bit PV guests no longer being able to upgrade to the > newest > >>> kernel version any longer > >>> > >>> And related to that: > >>> > >>> - is there any Linux distribution still shipping 32-bit PV-capable > >>> systems? > >>> > >>> - what about BSD? Is 32-bit PV support important there? > >> > >> Juergen - just to be very clear about the scope here: > >> * would this proposal affect the ability to use a 32-bit dom0? > > > > If the Dom0 is to be PV - yes, of course. For the time being there's > > no complete PVH Dom0 support, so if 32-bit is needed here, PV is > > for now indeed the only option. > ack. I asked because it's not necessarily obvious to all that dom0 is included in the term "guest", and it widens the consequences of this change. > > And to be more precise: the first step would be to remove 32-bit PV > support from upstream Linux kernel. This would result in the loss of > the ability to use a _new_ Linux (e.g. >= 4.20 / 5.0) as a 32-bit dom0. > A 32-bit dom0 using a kernel <= 4.19 would still work until we remove > 32-bit PV support from the hypervisor (which we wouldn't do before full > support of PVH dom0, I guess). > That makes sense. > > Is there a special reason you want to use a 32-bit dom0? > In short, no. OpenXT currently uses a 32-bit PV dom0, but work is already under way to migrate to 64-bit. I think your proposal is justified and good. Christopher ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 16/25] xen/arm: rename allocate_memory to allocate_memory_11
On Thu, 16 Aug 2018, Julien Grall wrote: > Hi Stefano, > > On 08/15/2018 09:26 PM, Stefano Stabellini wrote: > > On Mon, 13 Aug 2018, Julien Grall wrote: > > > Hi, > > > > > > On 01/08/18 00:27, Stefano Stabellini wrote: > > > > allocate_memory only deals with directly mapped memory. Rename it to > > > > allocate_memory_11. > > > > > > > > Signed-off-by: Stefano Stabellini > > > > > > > > --- > > > > Changes in v3: > > > > - add patch > > > > --- > > > >xen/arch/arm/domain_build.c | 7 --- > > > >1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > > > > index 066dd75..ab72c36 100644 > > > > --- a/xen/arch/arm/domain_build.c > > > > +++ b/xen/arch/arm/domain_build.c > > > > @@ -244,7 +244,8 @@ fail: > > > > * (as described above) we allow higher allocations and continue > > > > until > > > > * that runs out (or we have allocated sufficient dom0 memory). > > > > */ > > > > -static void __init allocate_memory(struct domain *d, struct kernel_info > > > > *kinfo) > > > > +static void __init allocate_memory_11(struct domain *d, > > > > + struct kernel_info *kinfo) > > > >{ > > > >const unsigned int min_low_order = > > > >get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128))); > > > > @@ -2240,7 +2241,7 @@ static int __init construct_domU(struct domain *d, > > > > struct dt_device_node *node) > > > >/* type must be set before allocate memory */ > > > >d->arch.type = kinfo.type; > > > >#endif > > > > -allocate_memory(d, ); > > > > +allocate_memory_11(d, ); > > > > > > I don't think your patches are correctly ordered. This is adding a lot of > > > confusion in the review because the DomU memory layout is fixed, yet here > > > you > > > rename the function to 1:1 mapping. > > > > > > Most likely you want to do add the new memory function before introducing > > > DomU. > > > > If I do that there will be no callers for the new function and > > compilation fails. Bisectibility is the reason why I had to reorder the > > patches. > > > > I understand but I don't want to give the impression that 1:1 mapping is used > for guests. I can see a couple of solutions: > - Implement allocate_memory in a static inline/#if 0 #endif. > - Provide a dummy call for the memory that will be implemented later > (similar to you do for construct_domU). OK, #if 0 it is ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 13/25] xen/arm: introduce create_domUs
On Thu, 16 Aug 2018, Julien Grall wrote: > Hi Stefano, > > On 08/15/2018 09:04 PM, Stefano Stabellini wrote: > > On Mon, 13 Aug 2018, Julien Grall wrote: > > > > +void __init create_domUs(void) > > > > +{ > > > > +struct dt_device_node *node; > > > > +struct dt_device_node *chosen = dt_find_node_by_name(dt_host, > > > > "chosen"); > > > > + > > > > +if ( chosen != NULL ) > > > > +{ > > > > +dt_for_each_child_node(chosen, node) > > > > +{ > > > > +struct domain *d; > > > > +struct xen_domctl_createdomain d_cfg = { > > > > +.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE, > > > > +.arch.nr_spis = 32, > > > > > > AFAICT, when creating DomU from the toolstack nr_spis will be 0. So why 32 > > > here? > > > > Legacy from debug code. It should be 0, unless vpl011 is enabled, in > > which case it should be 1. > > I would prefer if we use GUEST_VPL011_SPI - 32. This would make the code > bullet-proof for any potential reshuffle of the IRQs. OK, I'll do that. It is actually GUEST_VPL011_SPI - 32 + 1. (GUEST_VPL011_SPI - 32 is 0.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
On Thu, 16 Aug 2018, Julien Grall wrote: > Hi Stefano, > > On 08/14/2018 10:43 PM, Stefano Stabellini wrote: > > On Mon, 16 Jul 2018, Julien Grall wrote: > > > GUEST_BUG_ON may be used in other files doing guest emulation. > > > > > > Signed-off-by: Julien Grall > > > > Given that GUEST_BUG_ON is not actually used in any other files in this > > patch series, I assume you are referring to one of your future series? > > It is going to be used later. However, I don't really refer to any series in > particular. It is just that this macros could be helpful in any emulation code > to catch what we think is a invalid architectural behavior. It is only that a concrete use-case helps. The idea of moving GUEST_BUG_ON is good, but where to? For instance, wouldn't it be better to move it to guest_access.h? I am just trying to point to an existing header which is more widely used across the emulators (vpl011, vgic-v3-its.c, hvm.c, ...) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 25/25] xen/arm: split domain_build.c
On Thu, 16 Aug 2018, Julien Grall wrote: > Hi Stefano, > > On 08/16/2018 01:25 AM, Stefano Stabellini wrote: > > On Mon, 13 Aug 2018, Julien Grall wrote: > > > > + > > > > +#ifndef CONFIG_ACPI > > > > +static inline int prepare_acpi(struct domain *d, struct kernel_info > > > > *kinfo) > > > > +{ > > > > +/* Only booting with ACPI will hit here */ > > > > +BUG(); > > > > +return -EINVAL; > > > > +} > > > > +#else > > > > +int prepare_acpi(struct domain *d, struct kernel_info *kinfo); > > > > +#endif > > > > > > This one should go in asm-arm/acpi.h. > > > > > > So this header is not necessary anymore. > > > > I was unable to add prepare_acpi to asm-arm/acpi.h because it causes a > > #include dependency hell, I am thinking of adding it to > > asm-arm/domain_build.h. > > > > In file included from > > /local/repos/xen-upstream/xen/include/xen/sched.h:11:0, > > from /local/repos/xen-upstream/xen/include/asm/domain.h:5, > > from > > /local/repos/xen-upstream/xen/include/asm/kernel.h:10, > > from /local/repos/xen-upstream/xen/include/asm/acpi.h:27, > > from > > /local/repos/xen-upstream/xen/include/acpi/platform/aclinux.h:58, > > from > > /local/repos/xen-upstream/xen/include/acpi/platform/acenv.h:142, > > from /local/repos/xen-upstream/xen/include/acpi/acpi.h:56, > > from /local/repos/xen-upstream/xen/include/xen/acpi.h:33, > > from pl011.c:307: > > /local/repos/xen-upstream/xen/include/xen/domain.h:59:31: error: ‘struct > > xen_domctl_createdomain’ declared inside parameter list will not be visible > > outside of this definition or declaration [-Werror] > > struct xen_domctl_createdomain *config); > > Xen Arm headers are a bit a mess. :/ > > It looks like create_dom0 lives in setup.h, would it be possible to move the > prototypes there? It doesn't work either :-/ The problem is that the prototype of prepare_acpi needs the definition of struct kernel_info, which triggers the include mess. Leaving the prototypes in asm-arm/domain_build.h work though.___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Future of 32-bit PV support
On Thu, 16 Aug 2018, Juergen Gross wrote: > In the Xen x86 community call we have been discussing whether anyone > really is depending on 32-bit PV guests. We'd like to evaluate whether > anyone would see problems with: > > - deprecating 32-bit PV guest support in Xen, meaning that we'd > eventually switch to support 32-bit PV guests only via PV-shim from > Xen 4.12 or 4.13 > > - dropping 32-bit PV support from upstream Linux kernel, resulting in > current 32-bit PV guests no longer being able to upgrade to the newest > kernel version any longer > > And related to that: > > - is there any Linux distribution still shipping 32-bit PV-capable > systems? > > - what about BSD? Is 32-bit PV support important there? Hi Juergen, Although I can see that deprecating 32-bit PV guest support is desirable, and it might not cause any problems to Linux and BSDs, we need to be careful about unikernels. There are probably unikernels out there that only support PV 32bit still. And why not? If you are designing a unikernel today, it would still make sense to use PV 32bit or PVH. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/setup: Avoid OoB E820 lookup when calculating the L1TF safe address
On Thu, Aug 16, 2018 at 04:27:57PM +0100, Andrew Cooper wrote: > A number of corner cases (most obviously, no-real-mode and no Multiboot memory > map) can end up with e820_raw.nr_map being 0, at which point the L1TF > calculation will underflow. > > Spotted by Coverity. > > 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 v3 0/4] x86/iommu: PVH Dom0 workarounds for missing RMRR entries
On Wed, Aug 15, 2018 at 12:40 AM Jan Beulich wrote: > > >>> On 15.08.18 at 03:00, wrote: > > On Wed, Aug 8, 2018 at 3:54 AM Jan Beulich wrote: > >> > >> >>> On 08.08.18 at 10:25, wrote: > >> > On Tue, Aug 07, 2018 at 10:45:32AM -0600, Tamas K Lengyel wrote: > >> >> On Tue, Aug 7, 2018 at 10:04 AM Tamas K Lengyel > >> >> wrote: > >> >> (XEN) [VT-D]iommu.c:919: iommu_fault_status: Fault Overflow > >> >> (XEN) [VT-D]iommu.c:921: iommu_fault_status: Primary Pending Fault > >> >> (XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr > >> >> 428f926000, iommu reg = 82c000a0c000 > >> >> (XEN) [VT-D]DMAR: reason 06 - PTE Read access is not set > >> >> (XEN) print_vtd_entries: iommu #0 dev :00:02.0 gmfn 428f926 > >> >> (XEN) root_entry[00] = 43aaae001 > >> >> (XEN) context[10] = 2_43cf92001 > >> >> (XEN) l4[000] = 9c043cf91107 > >> >> (XEN) l3[10a] = 8000 > >> >> (XEN) l3[10a] not present > >> >> > >> >> The fault is repeated a million times per second and the system is > >> >> pretty much stalled. > >> > > >> > As Jan says, this page is outside of any range in the memory map. I > >> > wonder however what's in there. > >> > > >> > I think (also seeing the PV issues) you should bring this up with the > >> > driver maintainers, it might actually be a bug that the driver is > >> > trying to access such address. > >> > >> Right, especially considering that the address does not appear to be > >> invariant, I suspect the driver sets up some I/O from (presumably) > >> uninitialized data. This goes unnoticed until DMA translation comes > >> into play. Tamas - did you try enabling DMA translation in Linux > >> when not running on top of Xen? Depending on the > >> CONFIG_INTEL_IOMMU_DEFAULT_ON setting this may not be the > >> default. > > > > I checked and this kernel option is not enabled. Are you suggesting to > > try booting just Linux with this option enabled to see what happens? > > Well, you don't need to rebuild the kernel with the config option > enabled, you can use what you have and add "intel_iommu=on" > to the kernel command line. In case this works without any faults, > changing to "intel_iommu=on,igfx_off" may or may not provide > further insight. Booting just linux with both options works just fine, I grepped the dmesg log for the device that causes the issues with Xen but there are no errors in the log: # dmesg | grep "00\:02\.0" [0.165669] pci :00:02.0: [8086:5916] type 00 class 0x03 [0.165681] pci :00:02.0: reg 0x10: [mem 0xdb00-0xdbff 64bit] [0.165687] pci :00:02.0: reg 0x18: [mem 0x9000-0x9fff 64bit pref] [0.165692] pci :00:02.0: reg 0x20: [io 0xf000-0xf03f] [0.165707] pci :00:02.0: BAR 2: assigned to efifb [0.184223] pci :00:02.0: vgaarb: setting as boot VGA device [0.184223] pci :00:02.0: vgaarb: VGA device added: decodes=io+mem,owns=io+mem,locks=none [0.184223] pci :00:02.0: vgaarb: bridge control possible [0.248925] pci :00:02.0: Video device with shadowed ROM at [mem 0x000c-0x000d] [1.896026] i915 :00:02.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=io+mem:owns=io+mem [1.903881] [drm] Initialized i915 1.6.0 20180514 for :00:02.0 on minor 0 [3.271519] i915 :00:02.0: fb0: inteldrmfb frame buffer device [ 12.519683] snd_hda_intel :00:1f.3: bound :00:02.0 (ops i915_audio_component_bind_ops [i915]) > >> > In the meantime, you can try to add to the command line: > >> > > >> > rmrr=0x428f926=0:0:2.0 > >> > > >> > In order to force an iommu mapping of this address. > >> > >> I suspect this won't help much. > >> > > > > The mfn is not always the same but seems to be at least on some > > occasion. > > I expect that's because many things early at boot go pretty > deterministically. > > > I managed to reboot with the right rmrr= option set but I'm > > still getting the same faults over and over for that mfn I set in the > > rmrr= option. > > Hmm, and the faults still show non-present entries for these? This > options is at risk of getting misspelled - did you check that the > option you've specified actually took effect? I double checked and the option is set properly but I'm still getting the same non-present entry faults as before. At the moment I don't have serial access so not sure how to verify that the option took effect. This is my boot info: [pvh] options=loglvl=all guest_loglvl=all dom0_mem=4096M,max:4096M dom0_max_vcpus=2 sched=null dom0=pvh iommu=required,debug dom0-iommu=relaxed console=vga rmrr=0x5f48385=0:0:2.0,0x216183=0:0:2.0,0x70b6d19=0:0:2.0 kernel=vmlinuz-4.18.0-rc8 root=/dev/mapper/ubuntu--vg-root ro quiet splash vt.handoff=1 ramdisk=initrd.img-4.18.0-rc8 Tamas ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 125990: regressions - FAIL
flight 125990 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125990/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl 12 guest-start fail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen afc3f910d3434b540e4e4f51d9fd2723aea22fa2 baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z1 days Failing since125928 2018-08-15 19:00:49 Z0 days 10 attempts Testing same since 125967 2018-08-16 12:00:36 Z0 days2 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Ian Jackson Jan Beulich Julien Grall Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl fail 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 Not pushing. commit afc3f910d3434b540e4e4f51d9fd2723aea22fa2 Author: Jan Beulich Date: Thu Aug 16 00:49:29 2018 -0600 libxl: fix ARM build after 54ed251dc7 Commit "tools: Rework xc_domain_create() to take a full xen_domctl_createdomain" failed to replace one further instance of xc_config in libxl__arch_domain_save_config(). Signed-off-by: Jan Beulich Acked-by: Andrew Cooper Acked-by: Ian Jackson commit 70d8543950ad045fddcb78ae11302e713ef09c76 Author: Zhenzhong Duan Date: Thu Aug 16 09:31:57 2018 +0200 x86/mmcfg: remove redundant code in pci_mmcfg_reject_broken() No functional change. Signed-off-by: Zhenzhong Duan Acked-by: Jan Beulich commit a9e9837f54973ac45488c24e93ed34cbf20e4c66 Author: Jan Beulich Date: Thu Aug 16 09:30:59 2018 +0200 gnttab/ARM: properly implement gnttab_create_status_page() Prevent the "BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames() from triggering. Reported-by: 王磊 Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini commit 7626edeaca972e3e823535dcc44338f6b2f0b21f Author: Paul Durrant Date: Thu Aug 16 09:27:30 2018 +0200 x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries When emulating a rep I/O operation it is possible that the ioreq will describe a single operation that spans multiple GFNs. This is fine as long as all those GFNs fall within an MMIO region covered by a single device model, but unfortunately the higher levels of the emulation code do not guarantee that. This is something that should almost certainly be fixed, but in the meantime this patch makes sure that MMIO is truncated at GFN boundaries and hence the appropriate device model is re-evaluated for each target GFN. NOTE: This patch does not deal with the case of a single MMIO operation spanning a GFN boundary. That is more complex to deal with and is deferred to a subsequent patch. Signed-off-by: Paul Durrant Convert calculations to be 32-bit only. Signed-off-by: Jan Beulich commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into evtchn_init() ... rather than setting it up once domain_create() has completed. This involves constructing a default value for dom0. No practical change in functionality. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Acked-by: Julien Grall commit 4a83497635056d33fe20ef705f35617b1003a276 Author: Andrew Cooper Date: Tue Feb 27 17:39:37 2018 + xen/domctl: Merge set_max_evtchn into createdomain set_max_evtchn is somewhat weird. It was
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
On Thu, Aug 16, 2018 at 04:56:04PM +0100, Andrew Cooper wrote: > On 16/08/18 15:31, Roger Pau Monné wrote: > > On Thu, Aug 16, 2018 at 11:42:56AM +0100, Andrew Cooper wrote: > >> On 16/08/18 10:55, Roger Pau Monné wrote: > >>> On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk > index ac585a3..c84ed20 100644 > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid > (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) > $(call as-option-add,CFLAGS,CC,\ > ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) > > +# Check to see whether the assmbler supports the .nop directive. > +$(call as-option-add,CFLAGS,CC,\ > +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) > >>> I think I remember commenting on an earlier version of this about the > >>> usage of the CONTROL parameter. I would expect the assembler to > >>> use the most optimized version by default, is that not the case? > >> Again, I don't understand what you're trying to say. > >> > >> This expression is like this, because that's how we actually use it. > > As mentioned in another email, I was wondering why we choose to not > > use nop instructions > 9 bytes. The assembler will by default use > > nop instructions up to 11 bytes for 64bit code. > > Using more than 9 bytes is suboptimal on some hardware. OK. But using the same argument isn't it also suboptimal to use 9 bytes on some hardware then also? What I mean by this is that it would be good to add a comment somewhere that notes why nop instructions are limited to 9 bytes, because that's likely to generate the more optimized code on a wide variety of hardware. At least it would have helped me. > Toolchains use long nops (exclusively?) for basic block alignment, > whereas we use use them for executing through because its still faster > than a branch. > > > > + > CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables > > # Xen doesn't use SSE interally. If the compiler supports it, also > skip the > diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c > index 0ef7a8b..2c844d6 100644 > --- a/xen/arch/x86/alternative.c > +++ b/xen/arch/x86/alternative.c > @@ -84,6 +84,19 @@ static const unsigned char * const > p6_nops[ASM_NOP_MAX+1] init_or_livepatch_cons > > static const unsigned char * const *ideal_nops init_or_livepatch_data = > p6_nops; > > +#ifdef HAVE_AS_NOP_DIRECTIVE > + > +/* Nops in .init.rodata to compare against the runtime ideal nops. */ > +asm ( ".pushsection .init.rodata, \"a\", @progbits\n\t" > + "toolchain_nops: .nops " __stringify(ASM_NOP_MAX) "\n\t" > + ".popsection\n\t"); > +extern char toolchain_nops[ASM_NOP_MAX]; > +static bool __read_mostly toolchain_nops_are_ideal; > + > +#else > +# define toolchain_nops_are_ideal false > +#endif > + > static void __init arch_init_ideal_nops(void) > { > switch ( boot_cpu_data.x86_vendor ) > @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) > ideal_nops = k8_nops; > break; > } > + > +#ifdef HAVE_AS_NOP_DIRECTIVE > +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) > == 0 ) > +toolchain_nops_are_ideal = true; > +#endif > >>> You are only comparing that the biggest nop instruction (9 bytes > >>> AFAICT) generated by the assembler is what Xen believes to be the more > >>> optimized version. What about shorter nops? > >> They are all variations on a theme. > >> > >> For P6 nops, its the 0f 1f root which is important, which takes a modrm > >> byte. Traditionally, its always encoded with eax and uses redundant > >> memory encodings for longer instructions. > >> > >> I can't think of any way of detecting if the optimised nops if the > >> toolchain starts using alternative registers in the encoding, but I > >> expect this case won't happen in practice. > > I would rather do: > > > > toolchain_nops: > > .nops 1 > > .nops 2 > > .nops 3 > > ... > > .nops 9 > > > > And then build an assembler_nops[ASM_NOP_MAX+1] like it's done for the > > other nops. Then you could do: > > > > toolchain_nops_are_ideal = true; > > for ( i = 1; i < ASM_NOP_MAX+1; i++ ) > > if ( memcmp(ideal_nops[i], assembler_nops[i], i) ) > > { > > toolchain_nops_are_ideal = false; > > break; > > } > > > > In order to make sure all the possible nop sizes are using the > > expected optimized version. > > What is the point? Other than the 0f 1f, it really doesn't matter, and > small variations won't invalidate them as ideal nops. > > This check needs to be just good enough to tell
Re: [Xen-devel] [PATCH 2/6] x86/shadow: Use more appropriate conversion functions
On Wed, Aug 15, 2018 at 07:34:33PM +0100, Andrew Cooper wrote: > Replace pfn_to_paddr(mfn_x(...)) with mfn_to_maddr(), and replace an opencoded > gfn_to_gaddr(). > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/6] x86/shadow: Switch shadow_domain.has_fast_mmio_entries to bool
On Wed, Aug 15, 2018 at 07:34:34PM +0100, Andrew Cooper wrote: > Remove an unecessary if(). > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] x86/shadow: Clean up the MMIO fastpath helpers
On Wed, Aug 15, 2018 at 07:34:36PM +0100, Andrew Cooper wrote: > Use bool when appropraite, remove extranious brackets and fix up comment > style. > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Just one nit... > -static inline u32 sh_l1e_mmio_get_flags(shadow_l1e_t sl1e) > +static inline uint32_t sh_l1e_mmio_get_flags(shadow_l1e_t sl1e) Is there any reason to use uint32_t instead of unsigned int? Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/6] x86/shadow: Use mfn_t in shadow_track_dirty_vram()
On Wed, Aug 15, 2018 at 07:34:37PM +0100, Andrew Cooper wrote: > ... as the only user of sl1mfn would prefer it that way. > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/6] x86/shadow: Use MASK_* helpers for the MMIO fastpath PTE manipulation
On Wed, Aug 15, 2018 at 07:34:35PM +0100, Andrew Cooper wrote: > Drop the now-unused SH_L1E_MMIO_GFN_SHIFT definition. > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/6] x86/mm: Use mfn_eq()/mfn_add() rather than opencoded variations
On Wed, Aug 15, 2018 at 07:34:32PM +0100, Andrew Cooper wrote: > Use l1e_get_mfn() in place of l1e_get_pfn() when applicable, and fix up style > on affected lines. > > For sh_remove_shadow_via_pointer(), map_domain_page() is guaranteed to succeed > so there is no need to ASSERT() its success. This allows the pointer > arithmetic to folded into the previous expression, and for vaddr to be > properly typed as l1_pgentry_t, avoiding the cast in l1e_get_mfn(). > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné With one change: > diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c > index 021ae25..0d74c01 100644 > --- a/xen/arch/x86/mm/shadow/multi.c > +++ b/xen/arch/x86/mm/shadow/multi.c > @@ -960,7 +960,8 @@ static int shadow_set_l4e(struct domain *d, > { > /* We lost a reference to an old mfn. */ > mfn_t osl3mfn = shadow_l4e_get_mfn(old_sl4e); > -if ( (mfn_x(osl3mfn) != mfn_x(shadow_l4e_get_mfn(new_sl4e))) > + > +if ( mfn_eq(osl3mfn, shadow_l4e_get_mfn(new_sl4e)) I think this should be !mfn_eq. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
On 16/08/18 15:31, Roger Pau Monné wrote: > On Thu, Aug 16, 2018 at 11:42:56AM +0100, Andrew Cooper wrote: >> On 16/08/18 10:55, Roger Pau Monné wrote: >>> On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk index ac585a3..c84ed20 100644 --- a/xen/arch/x86/Rules.mk +++ b/xen/arch/x86/Rules.mk @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) $(call as-option-add,CFLAGS,CC,\ ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) +# Check to see whether the assmbler supports the .nop directive. +$(call as-option-add,CFLAGS,CC,\ +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) >>> I think I remember commenting on an earlier version of this about the >>> usage of the CONTROL parameter. I would expect the assembler to >>> use the most optimized version by default, is that not the case? >> Again, I don't understand what you're trying to say. >> >> This expression is like this, because that's how we actually use it. > As mentioned in another email, I was wondering why we choose to not > use nop instructions > 9 bytes. The assembler will by default use > nop instructions up to 11 bytes for 64bit code. Using more than 9 bytes is suboptimal on some hardware. Toolchains use long nops (exclusively?) for basic block alignment, whereas we use use them for executing through because its still faster than a branch. > + CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables # Xen doesn't use SSE interally. If the compiler supports it, also skip the diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 0ef7a8b..2c844d6 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -84,6 +84,19 @@ static const unsigned char * const p6_nops[ASM_NOP_MAX+1] init_or_livepatch_cons static const unsigned char * const *ideal_nops init_or_livepatch_data = p6_nops; +#ifdef HAVE_AS_NOP_DIRECTIVE + +/* Nops in .init.rodata to compare against the runtime ideal nops. */ +asm ( ".pushsection .init.rodata, \"a\", @progbits\n\t" + "toolchain_nops: .nops " __stringify(ASM_NOP_MAX) "\n\t" + ".popsection\n\t"); +extern char toolchain_nops[ASM_NOP_MAX]; +static bool __read_mostly toolchain_nops_are_ideal; + +#else +# define toolchain_nops_are_ideal false +#endif + static void __init arch_init_ideal_nops(void) { switch ( boot_cpu_data.x86_vendor ) @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) ideal_nops = k8_nops; break; } + +#ifdef HAVE_AS_NOP_DIRECTIVE +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == 0 ) +toolchain_nops_are_ideal = true; +#endif >>> You are only comparing that the biggest nop instruction (9 bytes >>> AFAICT) generated by the assembler is what Xen believes to be the more >>> optimized version. What about shorter nops? >> They are all variations on a theme. >> >> For P6 nops, its the 0f 1f root which is important, which takes a modrm >> byte. Traditionally, its always encoded with eax and uses redundant >> memory encodings for longer instructions. >> >> I can't think of any way of detecting if the optimised nops if the >> toolchain starts using alternative registers in the encoding, but I >> expect this case won't happen in practice. > I would rather do: > > toolchain_nops: > .nops 1 > .nops 2 > .nops 3 > ... > .nops 9 > > And then build an assembler_nops[ASM_NOP_MAX+1] like it's done for the > other nops. Then you could do: > > toolchain_nops_are_ideal = true; > for ( i = 1; i < ASM_NOP_MAX+1; i++ ) > if ( memcmp(ideal_nops[i], assembler_nops[i], i) ) > { > toolchain_nops_are_ideal = false; > break; > } > > In order to make sure all the possible nop sizes are using the > expected optimized version. What is the point? Other than the 0f 1f, it really doesn't matter, and small variations won't invalidate them as ideal nops. This check needs to be just good enough to tell whether the toolchain used P6 or K8 nops, and unless you explicitly built with -march=k8, the answer is going to be P6. It does not need to check every variation byte for byte. Going back to my original argument for not even doing this basic check, if we happen to be wrong and the toolchain wrote the other kind of long nops, you probably won't be able to measure the difference. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/setup: Avoid OoB E820 lookup when calculating the L1TF safe address
On Thu, Aug 16, 2018 at 04:27:57PM +0100, Andrew Cooper wrote: > A number of corner cases (most obviously, no-real-mode and no Multiboot memory > map) can end up with e820_raw.nr_map being 0, at which point the L1TF > calculation will underflow. > > Spotted by Coverity. > > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Thanks. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/setup: Avoid OoB E820 lookup when calculating the L1TF safe address
>>> On 16.08.18 at 17:27, wrote: > A number of corner cases (most obviously, no-real-mode and no Multiboot memory > map) can end up with e820_raw.nr_map being 0, at which point the L1TF > calculation will underflow. > > Spotted by Coverity. > > 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
Re: [Xen-devel] [PATCH] x86/HVM: correct an inverted check in hvm_load()
On 02/08/18 07:02, Jan Beulich wrote: On 01.08.18 at 18:08, wrote: >> On 01/08/18 16:36, Jan Beulich wrote: >>> Clearly we want to put a vCPU to sleep if it is _not_ already down. >>> >>> Signed-off-by: Jan Beulich >>> --- >>> TBD: Since the flaw apparently never mattered, I imply that the function >>> is never called with any vCPU up. Hence an alternative might be to >>> simply return an error if a vCPU doesn't have VPF_down set. >> With Remus/COLO, we should hit this path with an up vCPU on every >> iteration after the first. Given this bug, I'm struggling to see how it >> works at all. > Well, that would rule out the alternative suggestion, but since > this is neither an ack nor a nak - what do you suggest we do? Apologies for completely missing this. The fix is obvious, so Reviewed-by: Andrew Cooper I don't think we can instead return an error, because of the Remus/COLO usecase, but I expect someone is going to have some debugging to do... ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Ping: [PATCH] x86/HVM: correct an inverted check in hvm_load()
On Thu, Aug 16, 2018 at 02:05:02AM -0600, Jan Beulich wrote: > >>> On 01.08.18 at 17:36, wrote: > > Clearly we want to put a vCPU to sleep if it is _not_ already down. > > > > Signed-off-by: Jan Beulich > > --- > > TBD: Since the flaw apparently never mattered, I imply that the function > > is never called with any vCPU up. Hence an alternative might be to > > simply return an error if a vCPU doesn't have VPF_down set. > > While in an earlier reply you've indicated that the suggested > alternative is not an option, I've not had clear feedback on the > change below. This LGTM and fixes an obvious bug. I don't have an opinion on the above or whether Colo/Remus works at all, but the fix below seems to be less controversial than returning error. Reviewed-by: Roger Pau Monné Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/entry/64: Remove %ebx handling from error_entry/exit
On Thu, Aug 16, 2018 at 8:19 AM, Greg KH wrote: > On Fri, Aug 10, 2018 at 12:23:45AM -0700, Sarah Newman wrote: >> On 08/09/2018 05:41 AM, David Woodhouse wrote: >> > On Wed, 2018-08-08 at 10:35 -0700, Sarah Newman wrote: >> >> commit b3681dd548d06deb2e1573890829dff4b15abf46 upstream. >> >> >> >> This version applies to v4.9. >> > >> > I think you can kill the 'xorl %ebx,%ebx' from error_entry too but yes, >> > this does want to go to 4.9 and earlier because the 'Fixes:' tag is a >> > bit of a lie — the problem existed before that, at least in theory. >> >> The commit 2140a9942 "x86/entry/64: Relax pvops stub clobber >> specifications" was what removed the "movl %ebx, %eax" line later on >> originally, but it was the commit 3ac6d8c787b8 that removed the >> 'xorl %ebx,%ebx'. So these weren't matched. >> >> I don't know if it's a concern, but if someone had gone to the effort of >> backporting the original commit 3ac6d8c787b83, adding the removal of >> 'xorl %ebx,%ebx' to this patch would create merge conflicts. >> For that reason and given the line is harmless, should it be left in? > > I need some kind of agreement here for me to know what to do with this > patch... {hint} > I would remove the xorl. If there's an actual candidate patch, I'd be happy to read it. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/setup: Avoid OoB E820 lookup when calculating the L1TF safe address
A number of corner cases (most obviously, no-real-mode and no Multiboot memory map) can end up with e820_raw.nr_map being 0, at which point the L1TF calculation will underflow. Spotted by Coverity. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné --- xen/arch/x86/setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 727dad4..8d0f6f1 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -913,7 +913,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) /* Sanitise the raw E820 map to produce a final clean version. */ max_page = raw_max_page = init_e820(memmap_type, _raw); -if ( !efi_enabled(EFI_BOOT) ) +if ( !efi_enabled(EFI_BOOT) && e820_raw.nr_map >= 1 ) { /* * Supplement the heuristics in l1tf_calculations() by assuming that -- 2.1.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Future of 32-bit PV support
On Thu, Aug 16, 2018 at 08:17:13AM +0200, Juergen Gross wrote: > In the Xen x86 community call we have been discussing whether anyone > really is depending on 32-bit PV guests. We'd like to evaluate whether > anyone would see problems with: > > - deprecating 32-bit PV guest support in Xen, meaning that we'd > eventually switch to support 32-bit PV guests only via PV-shim from > Xen 4.12 or 4.13 This means 32bit PV support would be switched off by default in the hypervisor build, but distros or individuals could still enable it (like the build system will enable it for the shim). > - dropping 32-bit PV support from upstream Linux kernel, resulting in > current 32-bit PV guests no longer being able to upgrade to the newest > kernel version any longer > > And related to that: > > - is there any Linux distribution still shipping 32-bit PV-capable > systems? > > - what about BSD? Is 32-bit PV support important there? FTR, NetBSD is the only BSD to have PV support, both 32 and 64bits. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/entry/64: Remove %ebx handling from error_entry/exit
On Fri, Aug 10, 2018 at 12:23:45AM -0700, Sarah Newman wrote: > On 08/09/2018 05:41 AM, David Woodhouse wrote: > > On Wed, 2018-08-08 at 10:35 -0700, Sarah Newman wrote: > >> commit b3681dd548d06deb2e1573890829dff4b15abf46 upstream. > >> > >> This version applies to v4.9. > > > > I think you can kill the 'xorl %ebx,%ebx' from error_entry too but yes, > > this does want to go to 4.9 and earlier because the 'Fixes:' tag is a > > bit of a lie — the problem existed before that, at least in theory. > > The commit 2140a9942 "x86/entry/64: Relax pvops stub clobber > specifications" was what removed the "movl %ebx, %eax" line later on > originally, but it was the commit 3ac6d8c787b8 that removed the > 'xorl %ebx,%ebx'. So these weren't matched. > > I don't know if it's a concern, but if someone had gone to the effort of > backporting the original commit 3ac6d8c787b83, adding the removal of > 'xorl %ebx,%ebx' to this patch would create merge conflicts. > For that reason and given the line is harmless, should it be left in? I need some kind of agreement here for me to know what to do with this patch... {hint} thanks, greg k-h ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
On Thu, Aug 16, 2018 at 11:42:56AM +0100, Andrew Cooper wrote: > On 16/08/18 10:55, Roger Pau Monné wrote: > > On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: > >> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk > >> index ac585a3..c84ed20 100644 > >> --- a/xen/arch/x86/Rules.mk > >> +++ b/xen/arch/x86/Rules.mk > >> @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid > >> (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) > >> $(call as-option-add,CFLAGS,CC,\ > >> ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) > >> > >> +# Check to see whether the assmbler supports the .nop directive. > >> +$(call as-option-add,CFLAGS,CC,\ > >> +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) > > I think I remember commenting on an earlier version of this about the > > usage of the CONTROL parameter. I would expect the assembler to > > use the most optimized version by default, is that not the case? > > Again, I don't understand what you're trying to say. > > This expression is like this, because that's how we actually use it. As mentioned in another email, I was wondering why we choose to not use nop instructions > 9 bytes. The assembler will by default use nop instructions up to 11 bytes for 64bit code. > > > >> + > >> CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables > >> > >> # Xen doesn't use SSE interally. If the compiler supports it, also skip > >> the > >> diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c > >> index 0ef7a8b..2c844d6 100644 > >> --- a/xen/arch/x86/alternative.c > >> +++ b/xen/arch/x86/alternative.c > >> @@ -84,6 +84,19 @@ static const unsigned char * const > >> p6_nops[ASM_NOP_MAX+1] init_or_livepatch_cons > >> > >> static const unsigned char * const *ideal_nops init_or_livepatch_data = > >> p6_nops; > >> > >> +#ifdef HAVE_AS_NOP_DIRECTIVE > >> + > >> +/* Nops in .init.rodata to compare against the runtime ideal nops. */ > >> +asm ( ".pushsection .init.rodata, \"a\", @progbits\n\t" > >> + "toolchain_nops: .nops " __stringify(ASM_NOP_MAX) "\n\t" > >> + ".popsection\n\t"); > >> +extern char toolchain_nops[ASM_NOP_MAX]; > >> +static bool __read_mostly toolchain_nops_are_ideal; > >> + > >> +#else > >> +# define toolchain_nops_are_ideal false > >> +#endif > >> + > >> static void __init arch_init_ideal_nops(void) > >> { > >> switch ( boot_cpu_data.x86_vendor ) > >> @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) > >> ideal_nops = k8_nops; > >> break; > >> } > >> + > >> +#ifdef HAVE_AS_NOP_DIRECTIVE > >> +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == > >> 0 ) > >> +toolchain_nops_are_ideal = true; > >> +#endif > > You are only comparing that the biggest nop instruction (9 bytes > > AFAICT) generated by the assembler is what Xen believes to be the more > > optimized version. What about shorter nops? > > They are all variations on a theme. > > For P6 nops, its the 0f 1f root which is important, which takes a modrm > byte. Traditionally, its always encoded with eax and uses redundant > memory encodings for longer instructions. > > I can't think of any way of detecting if the optimised nops if the > toolchain starts using alternative registers in the encoding, but I > expect this case won't happen in practice. I would rather do: toolchain_nops: .nops 1 .nops 2 .nops 3 ... .nops 9 And then build an assembler_nops[ASM_NOP_MAX+1] like it's done for the other nops. Then you could do: toolchain_nops_are_ideal = true; for ( i = 1; i < ASM_NOP_MAX+1; i++ ) if ( memcmp(ideal_nops[i], assembler_nops[i], i) ) { toolchain_nops_are_ideal = false; break; } In order to make sure all the possible nop sizes are using the expected optimized version. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 9/9] x86: move declaration of arch_set_info_hvm_guest and provide stub
On Thu, Aug 16, 2018 at 07:48:43AM -0600, Jan Beulich wrote: > >>> On 16.08.18 at 14:59, wrote: > > On Thu, Aug 16, 2018 at 05:24:15AM -0600, Jan Beulich wrote: > >> >>> On 16.08.18 at 12:42, wrote: > >> > On Tue, Aug 07, 2018 at 05:24:15PM +0100, Wei Liu wrote: > >> > > > >> >> > All uses sit either in HVM-specific code or inside is_hvm_...() > >> >> > conditionals: Why do we need the inline stub? If the declaration > >> >> > was visible independent of CONFIG_HVM, code would compile > >> >> > fine, and references to the function would be removed by the > >> >> > compiler, so linking would also succeed despite there not being > >> >> > any definition of the function. > >> >> > >> >> Last time I tried DCE wasn't working so well. Let me try again and if > >> >> DCE works it would save me a lot of effort to provide stubs. > >> >> > >> > > >> > DCE seems to work better this time. > >> > > >> > The only problem is that some ASSERTs will need to turn into panic or > >> > BUG() if we want to fully utilise DCE. Is that okay? > >> > >> In general yes, I think so. > >> > >> > To give you an example, after making is_hvm_domain evaluate to 0 when > >> > !CONFIG_HVM, vm_event_fill_regs + !CONFIG_HVM compiles fine for debug > >> > build because ASSERT hints the compiler that the rest of the function is > >> > never going to be reachable. But for non-debug build, ASSERT is empty, > >> > so compiler will not eliminate the rest of the function, complaining > >> > hvm_get_segment_register is not available. It is solvable by adding > >> > panic or BUG. > >> > > >> > There is going to be quite a few cases like that. I haven't gone through > >> > all of them. > >> > >> In cases like the example you give I'm not convinced of the > >> suggested conversion though - the entire function should then > >> live inside CONFIG_HVM (or in a file built for HVM only). > >> > > > > This will do, too -- if you don't mind littering CONFIG_HVM in files. > > > > VM event subsystem is entangled with other subsystems, too, so it will > > take some time to clean that up. I haven't got to that part yet. At the > > moment I have accumulated ~25 patches to almost make !CONFIG_HVM work > > for debug build. I will go through all patches later to make them work > > with non-debug build. > > That'll be fine for now, I think. Eventually the HVM pieces should be > moved to hvm/ of course. > Ack. > > One thing I haven't decided what to do is hvm/i8254.c, which is used by > > both PV and HVM. I'm thinking about moving that file under arch/x86 and > > rename it emul-i8254.c. Is that a sensible thing to do? > > Any chance you could leave HVM-only parts where they are? Or > would that make more of a mess than moving the entire file? Basically everything in that file is used by both HVM and PV. I will leave the HVM-only parts under hvm/ if there is any. Wei. > > Jan > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 9/9] x86: move declaration of arch_set_info_hvm_guest and provide stub
>>> On 16.08.18 at 14:59, wrote: > On Thu, Aug 16, 2018 at 05:24:15AM -0600, Jan Beulich wrote: >> >>> On 16.08.18 at 12:42, wrote: >> > On Tue, Aug 07, 2018 at 05:24:15PM +0100, Wei Liu wrote: >> > > >> >> > All uses sit either in HVM-specific code or inside is_hvm_...() >> >> > conditionals: Why do we need the inline stub? If the declaration >> >> > was visible independent of CONFIG_HVM, code would compile >> >> > fine, and references to the function would be removed by the >> >> > compiler, so linking would also succeed despite there not being >> >> > any definition of the function. >> >> >> >> Last time I tried DCE wasn't working so well. Let me try again and if >> >> DCE works it would save me a lot of effort to provide stubs. >> >> >> > >> > DCE seems to work better this time. >> > >> > The only problem is that some ASSERTs will need to turn into panic or >> > BUG() if we want to fully utilise DCE. Is that okay? >> >> In general yes, I think so. >> >> > To give you an example, after making is_hvm_domain evaluate to 0 when >> > !CONFIG_HVM, vm_event_fill_regs + !CONFIG_HVM compiles fine for debug >> > build because ASSERT hints the compiler that the rest of the function is >> > never going to be reachable. But for non-debug build, ASSERT is empty, >> > so compiler will not eliminate the rest of the function, complaining >> > hvm_get_segment_register is not available. It is solvable by adding >> > panic or BUG. >> > >> > There is going to be quite a few cases like that. I haven't gone through >> > all of them. >> >> In cases like the example you give I'm not convinced of the >> suggested conversion though - the entire function should then >> live inside CONFIG_HVM (or in a file built for HVM only). >> > > This will do, too -- if you don't mind littering CONFIG_HVM in files. > > VM event subsystem is entangled with other subsystems, too, so it will > take some time to clean that up. I haven't got to that part yet. At the > moment I have accumulated ~25 patches to almost make !CONFIG_HVM work > for debug build. I will go through all patches later to make them work > with non-debug build. That'll be fine for now, I think. Eventually the HVM pieces should be moved to hvm/ of course. > One thing I haven't decided what to do is hvm/i8254.c, which is used by > both PV and HVM. I'm thinking about moving that file under arch/x86 and > rename it emul-i8254.c. Is that a sensible thing to do? Any chance you could leave HVM-only parts where they are? Or would that make more of a mess than moving the entire file? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] xen/xsm: Cleanup in preparation for XSM SILO mode
>>> On 16.08.18 at 15:18, wrote: > On 16/08/18 13:56, Jan Beulich wrote: > On 16.08.18 at 14:46, wrote: >>> On 26/06/18 12:09, Andrew Cooper wrote: Future changes will introduce a new SILO mode, which is intended to be > useful for cloud and enterprise setups where all domUs are unprivileged and have no buisness communicating directly. This was discussed at XenSummit, but I'll leave further details to the > series which introduces it. However, to begin with, clean up the XSM namespacing > to better separate XSM and FLASK. No functional change. Andrew Cooper (2): xen/xsm: Rename CONFIG_FLASK_* to CONFIG_XSM_FLASK_* xen/xsm: Rename CONIFIG_XSM_POLICY to CONFIG_XSM_FLASK_POLICY >>> Ping "The Rest" in lieu of Daniel. This series is blocking the >>> functional XSM SILO work. >> Iirc I had given some comments, regarding the (too long) names. >> The changes are mechanical enough that I don't think there's >> much else to say. > > And I justified why the current naming is IMO wrong and why it wants to > be suitably namespaced. But I didn't object to the rename (and name spacing) in general, I've merely suggested that shorter (still properly name spaced) names would do as well. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 125967: regressions - FAIL
flight 125967 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125967/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl 12 guest-start fail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen afc3f910d3434b540e4e4f51d9fd2723aea22fa2 baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z0 days Failing since125928 2018-08-15 19:00:49 Z0 days9 attempts Testing same since 125967 2018-08-16 12:00:36 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Ian Jackson Jan Beulich Julien Grall Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl fail 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 Not pushing. commit afc3f910d3434b540e4e4f51d9fd2723aea22fa2 Author: Jan Beulich Date: Thu Aug 16 00:49:29 2018 -0600 libxl: fix ARM build after 54ed251dc7 Commit "tools: Rework xc_domain_create() to take a full xen_domctl_createdomain" failed to replace one further instance of xc_config in libxl__arch_domain_save_config(). Signed-off-by: Jan Beulich Acked-by: Andrew Cooper Acked-by: Ian Jackson commit 70d8543950ad045fddcb78ae11302e713ef09c76 Author: Zhenzhong Duan Date: Thu Aug 16 09:31:57 2018 +0200 x86/mmcfg: remove redundant code in pci_mmcfg_reject_broken() No functional change. Signed-off-by: Zhenzhong Duan Acked-by: Jan Beulich commit a9e9837f54973ac45488c24e93ed34cbf20e4c66 Author: Jan Beulich Date: Thu Aug 16 09:30:59 2018 +0200 gnttab/ARM: properly implement gnttab_create_status_page() Prevent the "BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames() from triggering. Reported-by: 王磊 Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini commit 7626edeaca972e3e823535dcc44338f6b2f0b21f Author: Paul Durrant Date: Thu Aug 16 09:27:30 2018 +0200 x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries When emulating a rep I/O operation it is possible that the ioreq will describe a single operation that spans multiple GFNs. This is fine as long as all those GFNs fall within an MMIO region covered by a single device model, but unfortunately the higher levels of the emulation code do not guarantee that. This is something that should almost certainly be fixed, but in the meantime this patch makes sure that MMIO is truncated at GFN boundaries and hence the appropriate device model is re-evaluated for each target GFN. NOTE: This patch does not deal with the case of a single MMIO operation spanning a GFN boundary. That is more complex to deal with and is deferred to a subsequent patch. Signed-off-by: Paul Durrant Convert calculations to be 32-bit only. Signed-off-by: Jan Beulich commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into evtchn_init() ... rather than setting it up once domain_create() has completed. This involves constructing a default value for dom0. No practical change in functionality. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Acked-by: Julien Grall commit 4a83497635056d33fe20ef705f35617b1003a276 Author: Andrew Cooper Date: Tue Feb 27 17:39:37 2018 + xen/domctl: Merge set_max_evtchn into createdomain set_max_evtchn is somewhat weird. It was
Re: [Xen-devel] [PATCH v3 13/17] x86/xen: enable Hygon support to Xen
On 2018/8/12 21:26, Boris Ostrovsky wrote: On 08/12/2018 04:55 AM, Juergen Gross wrote: On 11/08/18 16:34, Boris Ostrovsky wrote: On 08/11/2018 09:29 AM, Pu Wen wrote: bool pmu_msr_read(unsigned int msr, uint64_t *val, int *err) { - if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { + if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD || + boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) { 'if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)' please. Really? Xen supports Centaur, too. VPMU doesn't --- hypervisor will not initialize it. Besides, the existing code will steer non-AMD execution to Intel, which is not right either. I'll add a check to bail if VPMU is not initialized properly, we seem to ignore xen_pmu_init() failures. Which, BTW, makes this patch rather pointless until there is support for Hygon Xen. So should it still need to test vendor Hygon here or wait for your check done? Thanks, Pu Wen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 13/17] x86/xen: enable Hygon support to Xen
On 2018/8/11 22:34, Boris Ostrovsky wrote: On 08/11/2018 09:29 AM, Pu Wen wrote: To make Xen work correctly on Hygon platforms, reuse AMD's Xen support code path and add vendor check for Hygon along with AMD. Signed-off-by: Pu Wen --- arch/x86/xen/pmu.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/arch/x86/xen/pmu.c b/arch/x86/xen/pmu.c index 7d00d4a..1053dda 100644 --- a/arch/x86/xen/pmu.c +++ b/arch/x86/xen/pmu.c @@ -90,6 +90,12 @@ static void xen_pmu_arch_init(void) k7_counters_mirrored = 0; break; } + } else if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON) { + amd_num_counters = F10H_NUM_COUNTERS; I haven't looked in details at Zen's PMU but the PMC section in the spec starts with "There are six core performance events counters per thread..." There are six core performance events counters per thread, so there are six MSRs for these counters(0-5). Also there are four legacy PMC MSRs, they are alias of the counters(0-3). In this version of kernel Zen use the lagacy version of PMU MSRs for Xen. For safety consideration, Dhyana just fullow this stategy. And it works fine when VPMU enabled in Xen on Hygon platforms by testing with perf. Thanks, Pu Wen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] xen/xsm: Cleanup in preparation for XSM SILO mode
On 16/08/18 13:56, Jan Beulich wrote: On 16.08.18 at 14:46, wrote: >> On 26/06/18 12:09, Andrew Cooper wrote: >>> Future changes will introduce a new SILO mode, which is intended to be >>> useful >>> for cloud and enterprise setups where all domUs are unprivileged and have no >>> buisness communicating directly. >>> >>> This was discussed at XenSummit, but I'll leave further details to the >>> series >>> which introduces it. However, to begin with, clean up the XSM namespacing >>> to >>> better separate XSM and FLASK. >>> >>> No functional change. >>> >>> Andrew Cooper (2): >>> xen/xsm: Rename CONFIG_FLASK_* to CONFIG_XSM_FLASK_* >>> xen/xsm: Rename CONIFIG_XSM_POLICY to CONFIG_XSM_FLASK_POLICY >> Ping "The Rest" in lieu of Daniel. This series is blocking the >> functional XSM SILO work. > Iirc I had given some comments, regarding the (too long) names. > The changes are mechanical enough that I don't think there's > much else to say. And I justified why the current naming is IMO wrong and why it wants to be suitably namespaced. Hence the ping to unblock this series. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 9/9] x86: move declaration of arch_set_info_hvm_guest and provide stub
On Thu, Aug 16, 2018 at 05:24:15AM -0600, Jan Beulich wrote: > >>> On 16.08.18 at 12:42, wrote: > > On Tue, Aug 07, 2018 at 05:24:15PM +0100, Wei Liu wrote: > > > > >> > All uses sit either in HVM-specific code or inside is_hvm_...() > >> > conditionals: Why do we need the inline stub? If the declaration > >> > was visible independent of CONFIG_HVM, code would compile > >> > fine, and references to the function would be removed by the > >> > compiler, so linking would also succeed despite there not being > >> > any definition of the function. > >> > >> Last time I tried DCE wasn't working so well. Let me try again and if > >> DCE works it would save me a lot of effort to provide stubs. > >> > > > > DCE seems to work better this time. > > > > The only problem is that some ASSERTs will need to turn into panic or > > BUG() if we want to fully utilise DCE. Is that okay? > > In general yes, I think so. > > > To give you an example, after making is_hvm_domain evaluate to 0 when > > !CONFIG_HVM, vm_event_fill_regs + !CONFIG_HVM compiles fine for debug > > build because ASSERT hints the compiler that the rest of the function is > > never going to be reachable. But for non-debug build, ASSERT is empty, > > so compiler will not eliminate the rest of the function, complaining > > hvm_get_segment_register is not available. It is solvable by adding > > panic or BUG. > > > > There is going to be quite a few cases like that. I haven't gone through > > all of them. > > In cases like the example you give I'm not convinced of the > suggested conversion though - the entire function should then > live inside CONFIG_HVM (or in a file built for HVM only). > This will do, too -- if you don't mind littering CONFIG_HVM in files. VM event subsystem is entangled with other subsystems, too, so it will take some time to clean that up. I haven't got to that part yet. At the moment I have accumulated ~25 patches to almost make !CONFIG_HVM work for debug build. I will go through all patches later to make them work with non-debug build. One thing I haven't decided what to do is hvm/i8254.c, which is used by both PV and HVM. I'm thinking about moving that file under arch/x86 and rename it emul-i8254.c. Is that a sensible thing to do? Wei. > Jan > > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] xen/xsm: Cleanup in preparation for XSM SILO mode
>>> On 16.08.18 at 14:46, wrote: > On 26/06/18 12:09, Andrew Cooper wrote: >> Future changes will introduce a new SILO mode, which is intended to be useful >> for cloud and enterprise setups where all domUs are unprivileged and have no >> buisness communicating directly. >> >> This was discussed at XenSummit, but I'll leave further details to the series >> which introduces it. However, to begin with, clean up the XSM namespacing to >> better separate XSM and FLASK. >> >> No functional change. >> >> Andrew Cooper (2): >> xen/xsm: Rename CONFIG_FLASK_* to CONFIG_XSM_FLASK_* >> xen/xsm: Rename CONIFIG_XSM_POLICY to CONFIG_XSM_FLASK_POLICY > > Ping "The Rest" in lieu of Daniel. This series is blocking the > functional XSM SILO work. Iirc I had given some comments, regarding the (too long) names. The changes are mechanical enough that I don't think there's much else to say. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
On 16/08/18 13:27, Jan Beulich wrote: On 16.08.18 at 12:56, wrote: >> On 16/08/18 11:29, Jan Beulich wrote: >>> Following some further discussion with Andrew, he looks to be >>> convinced that the issue is to be fixed in the balloon driver, >>> which so far (intentionally afaict) does not remove the direct >>> map entries for ballooned out pages in the HVM case. I'm not >>> convinced of this, but I'd nevertheless like to inquire whether >>> such a change (resulting in shattered super page mappings) >>> would be acceptable in the first place. >> >> We don't tolerate anything else in the directmap pointing to >> invalid/unimplemented frames. Why should ballooning be any different? > > Because ballooning is something virtualization specific, which > doesn't have any equivalent on bare hardware (memory hot > unplug doesn't come quite close enough imo, not the least > because that doesn't work on a page granular basis). Hence > we're to define the exact behavior here, and hence such a > definition could as well include special behavior of accesses > to the involved guest-physical addresses. If I read drivers/virtio/virtio_balloon.c correctly KVM does the same as Xen: ballooned pages are _not_ removed from the direct mappings. Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] libxl: fix ARM build after 54ed251dc7
On Thu, Aug 16, 2018 at 08:41:55AM +0100, Andrew Cooper wrote: > On 16/08/2018 07:49, Jan Beulich wrote: > > Commit "tools: Rework xc_domain_create() to take a full > > xen_domctl_createdomain" failed to replace one further instance of > > xc_config in libxl__arch_domain_save_config(). > > > > Signed-off-by: Jan Beulich > > --- > > I have no environment set up to do cross tool stack builds, so the patch > > is solely based on osstest report and code inspection. > > Nor do I sadly, and we don't yet have this working in Travis/gitlab > which is that testing missed it. > > Acked-by: Andrew Cooper 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 0/2] xen/xsm: Cleanup in preparation for XSM SILO mode
On 26/06/18 12:09, Andrew Cooper wrote: > Future changes will introduce a new SILO mode, which is intended to be useful > for cloud and enterprise setups where all domUs are unprivileged and have no > buisness communicating directly. > > This was discussed at XenSummit, but I'll leave further details to the series > which introduces it. However, to begin with, clean up the XSM namespacing to > better separate XSM and FLASK. > > No functional change. > > Andrew Cooper (2): > xen/xsm: Rename CONFIG_FLASK_* to CONFIG_XSM_FLASK_* > xen/xsm: Rename CONIFIG_XSM_POLICY to CONFIG_XSM_FLASK_POLICY Ping "The Rest" in lieu of Daniel. This series is blocking the functional XSM SILO work. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
>>> On 16.08.18 at 13:48, wrote: > On 16/08/18 12:34, Jan Beulich wrote: > On 16.08.18 at 12:42, wrote: >>> On 16/08/18 10:55, Roger Pau Monné wrote: On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: > @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) > ideal_nops = k8_nops; > break; > } > + > +#ifdef HAVE_AS_NOP_DIRECTIVE > +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == > 0 > ) > +toolchain_nops_are_ideal = true; > +#endif You are only comparing that the biggest nop instruction (9 bytes AFAICT) generated by the assembler is what Xen believes to be the more optimized version. What about shorter nops? >>> They are all variations on a theme. >>> >>> For P6 nops, its the 0f 1f root which is important, which takes a modrm >>> byte. Traditionally, its always encoded with eax and uses redundant >>> memory encodings for longer instructions. >>> >>> I can't think of any way of detecting if the optimised nops if the >>> toolchain starts using alternative registers in the encoding, but I >>> expect this case won't happen in practice. >> It's not just the register encoding, but also the maximum single-insn >> length that gets generated. Recall that until not very long ago we >> had up to 8-byte NOP insns only? The view on the mod (as in ModRM) >> usage may vary over time, as may the view on which or how many >> prefixes are reasonable to have. > > Strictly speaking, the ORM says "encode the least-recently live > register", because all the hint nops are still subject to reg/reg > dependencies. > > However, we definitely can't take advantage of this, nor can the > assembler. Well, _we_ could, at least when tail padding patched in insns: I very much hope we know what we've patched in, and hence at least what registers were used most recently. This is not readily available today, but could be made so. > Compilers can't either, because the exact length of the nop > depends on other relocations. Furthermore, the perf improvement from > doing this would be fractional. True. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
>>> On 16.08.18 at 13:57, wrote: > On Thu, Aug 16, 2018 at 04:18:03AM -0600, Jan Beulich wrote: >> >>> On 16.08.18 at 11:55, wrote: >> > On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: >> >> --- a/xen/arch/x86/Rules.mk >> >> +++ b/xen/arch/x86/Rules.mk >> >> @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid > (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) >> >> $(call as-option-add,CFLAGS,CC,\ >> >> ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) >> >> >> >> +# Check to see whether the assmbler supports the .nop directive. >> >> +$(call as-option-add,CFLAGS,CC,\ >> >> +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) >> > >> > I think I remember commenting on an earlier version of this about the >> > usage of the CONTROL parameter. I would expect the assembler to >> > use the most optimized version by default, is that not the case? >> >> How could it, without knowing what the target hardware is? And even >> if it could, what is considered "most optimized" tends to change every >> once in a while (or else we wouldn't have different NOP flavors to >> begin with). > > I think I haven't explained myself well. By using the CONTROL > parameter we limit the max length of a nop instruction to 9 bytes > instead of using the maximum supported by the assembler (11 bytes IIRC > for 64bit). Is this done because there are issues with using nops > 9 > bytes? There the problems start that I've hinted at towards Andrew: gas supports up to 11-byte NOPs despite the SDM saying otherwise at present. But ORM and SDM disagree as well, for the 3- and 6-byte variants. Otoh AMD recommends up to 11-byte NOPs for Fam15 and earlier, and suggests even using up to 15-byte ones on Fam17. Besides that I also wonder whether in 64-bit mode REX prefixes wouldn't be preferable over operand size override ones. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
On Thu, Aug 16, 2018 at 04:18:03AM -0600, Jan Beulich wrote: > >>> On 16.08.18 at 11:55, wrote: > > On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: > >> --- a/xen/arch/x86/Rules.mk > >> +++ b/xen/arch/x86/Rules.mk > >> @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid > >> (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) > >> $(call as-option-add,CFLAGS,CC,\ > >> ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) > >> > >> +# Check to see whether the assmbler supports the .nop directive. > >> +$(call as-option-add,CFLAGS,CC,\ > >> +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) > > > > I think I remember commenting on an earlier version of this about the > > usage of the CONTROL parameter. I would expect the assembler to > > use the most optimized version by default, is that not the case? > > How could it, without knowing what the target hardware is? And even > if it could, what is considered "most optimized" tends to change every > once in a while (or else we wouldn't have different NOP flavors to > begin with). I think I haven't explained myself well. By using the CONTROL parameter we limit the max length of a nop instruction to 9 bytes instead of using the maximum supported by the assembler (11 bytes IIRC for 64bit). Is this done because there are issues with using nops > 9 bytes? Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] Patch "xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits" has been added to the 4.17-stable tree
This is a note to let you know that I've just added the patch titled xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits to the 4.17-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: xen-pv-call-get_cpu_address_sizes-to-set-x86_virt-phys_bits.patch and it can be found in the queue-4.17 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let know about it. From 405c018a25fe464dc68057bbc8014a58f2bd4422 Mon Sep 17 00:00:00 2001 From: "M. Vefa Bicakci" Date: Tue, 24 Jul 2018 08:45:47 -0400 Subject: xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits From: M. Vefa Bicakci commit 405c018a25fe464dc68057bbc8014a58f2bd4422 upstream. Commit d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption") has moved the query and calculation of the x86_virt_bits and x86_phys_bits fields of the cpuinfo_x86 struct from the get_cpu_cap function to a new function named get_cpu_address_sizes. One of the call sites related to Xen PV VMs was unfortunately missed in the aforementioned commit. This prevents successful boot-up of kernel versions 4.17 and up in Xen PV VMs if CONFIG_DEBUG_VIRTUAL is enabled, due to the following code path: enlighten_pv.c::xen_start_kernel mmu_pv.c::xen_reserve_special_pages page.h::__pa physaddr.c::__phys_addr physaddr.h::phys_addr_valid phys_addr_valid uses boot_cpu_data.x86_phys_bits to validate physical addresses. boot_cpu_data.x86_phys_bits is no longer populated before the call to xen_reserve_special_pages due to the aforementioned commit though, so the validation performed by phys_addr_valid fails, which causes __phys_addr to trigger a BUG, preventing boot-up. Signed-off-by: M. Vefa Bicakci Reviewed-by: Thomas Gleixner Reviewed-by: Boris Ostrovsky Cc: "Kirill A. Shutemov" Cc: Andy Lutomirski Cc: Ingo Molnar Cc: "H. Peter Anvin" Cc: Thomas Gleixner Cc: Boris Ostrovsky Cc: Juergen Gross Cc: xen-devel@lists.xenproject.org Cc: x...@kernel.org Cc: sta...@vger.kernel.org # for v4.17 and up Fixes: d94a155c59c9 ("x86/cpu: Prevent cpuinfo_x86::x86_phys_bits adjustment corruption") Signed-off-by: Boris Ostrovsky Signed-off-by: Greg Kroah-Hartman --- arch/x86/kernel/cpu/common.c |2 +- arch/x86/kernel/cpu/cpu.h|1 + arch/x86/xen/enlighten_pv.c |3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -883,7 +883,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c) apply_forced_caps(c); } -static void get_cpu_address_sizes(struct cpuinfo_x86 *c) +void get_cpu_address_sizes(struct cpuinfo_x86 *c) { u32 eax, ebx, ecx, edx; --- a/arch/x86/kernel/cpu/cpu.h +++ b/arch/x86/kernel/cpu/cpu.h @@ -46,6 +46,7 @@ extern const struct cpu_dev *const __x86 *const __x86_cpu_dev_end[]; extern void get_cpu_cap(struct cpuinfo_x86 *c); +extern void get_cpu_address_sizes(struct cpuinfo_x86 *c); extern void cpu_detect_cache_sizes(struct cpuinfo_x86 *c); extern int detect_extended_topology_early(struct cpuinfo_x86 *c); extern int detect_ht_early(struct cpuinfo_x86 *c); --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -1258,6 +1258,9 @@ asmlinkage __visible void __init xen_sta get_cpu_cap(_cpu_data); x86_configure_nx(); + /* Determine virtual and physical address sizes */ + get_cpu_address_sizes(_cpu_data); + /* Let's presume PV guests always boot on vCPU with id 0. */ per_cpu(xen_vcpu_id, 0) = 0; Patches currently in stable-queue which might be from m@runbox.com are queue-4.17/xen-pv-call-get_cpu_address_sizes-to-set-x86_virt-phys_bits.patch ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
On 16/08/18 12:34, Jan Beulich wrote: On 16.08.18 at 12:42, wrote: >> On 16/08/18 10:55, Roger Pau Monné wrote: >>> On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) ideal_nops = k8_nops; break; } + +#ifdef HAVE_AS_NOP_DIRECTIVE +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == 0 ) +toolchain_nops_are_ideal = true; +#endif >>> You are only comparing that the biggest nop instruction (9 bytes >>> AFAICT) generated by the assembler is what Xen believes to be the more >>> optimized version. What about shorter nops? >> They are all variations on a theme. >> >> For P6 nops, its the 0f 1f root which is important, which takes a modrm >> byte. Traditionally, its always encoded with eax and uses redundant >> memory encodings for longer instructions. >> >> I can't think of any way of detecting if the optimised nops if the >> toolchain starts using alternative registers in the encoding, but I >> expect this case won't happen in practice. > It's not just the register encoding, but also the maximum single-insn > length that gets generated. Recall that until not very long ago we > had up to 8-byte NOP insns only? The view on the mod (as in ModRM) > usage may vary over time, as may the view on which or how many > prefixes are reasonable to have. Strictly speaking, the ORM says "encode the least-recently live register", because all the hint nops are still subject to reg/reg dependencies. However, we definitely can't take advantage of this, nor can the assembler. Compilers can't either, because the exact length of the nop depends on other relocations. Furthermore, the perf improvement from doing this would be fractional. IOW, I don't expect we'll realistically see encodings other than the exact byte layout described in the ORM. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [ovmf test] 125924: all pass - PUSHED
flight 125924 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/125924/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf cb5f4f45ce1fca390b99dae5c42b9c4c8b53deea baseline version: ovmf 22ec06c8aaa1c4023bb7f960746da57f1b648568 Last test of basis 125900 2018-08-14 01:10:52 Z2 days Testing same since 125924 2018-08-15 17:10:53 Z0 days1 attempts People who touched revisions under test: Ming Huang jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 22ec06c8aa..cb5f4f45ce cb5f4f45ce1fca390b99dae5c42b9c4c8b53deea -> xen-tested-master ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86/xsave: prefer eager clearing of state over eager restoring
>>> On 16.08.18 at 13:27, wrote: > On 16/08/18 11:03, Jan Beulich wrote: > On 16.08.18 at 11:07, wrote: >>> On 22/06/2018 11:57, Jan Beulich wrote: --- a/xen/arch/x86/spec_ctrl.c +++ b/xen/arch/x86/spec_ctrl.c @@ -616,7 +616,7 @@ void __init init_speculation_mitigations /* Check whether Eager FPU should be enabled by default. */ if ( opt_eager_fpu == -1 ) -opt_eager_fpu = should_use_eager_fpu(); +opt_eager_fpu = !cpu_has_xsave && should_use_eager_fpu(); >>> I'd not spotted this the first time round. >>> >>> Intel is very clear that, if you're using xsave, you should be using >>> eager FPU. Therefore, this goes specifically against the advice in the >>> ORM, and the advise we were given during the LazyFPU timeframe. >>> >>> Furthermore we (XenServer) and customers have seen a reliable perf >>> improvement from the LazyFPU security fix, up to 8% in places, for >>> normal VDI and server workloads. As I said during the development the >>> LazyFPU fixes, this is almost certainly down to the fact that all code >>> uses the FPU these days. >> Well - as said in the description, observation in my tests (which are >> not a typical server workload) were that about 50% of the context >> switches were no followed by a (lazy) restore, until the vCPU was >> de-scheduled again. > > Counting absolute numbers gives a false impression. > > You've got to account for the relative difference in cycles between an > xrstor and servicing #NM (which includes the xrstor you previously skipped). > > The 50/50 split you see here is definitely going to result in a net perf > hit because servicing #NM is several orders of magnitude more expensive > than xrstor. (For HVM guests, you've got to add another order of > magnitude for the vmexit). > > (At a guess, seeing as its been a little too long since I last did this > kind of stats), you've got to get to somewhere like 85-95% before you're > likely to break even from a performance point of view. That's all understood; hence the post-commit message remark in the patch. >> The change as presented is in fact trying to move to a middle ground, >> in that it doesn't leave stale state in the registers anymore, but >> instead frees the underlying physical ones up for other uses (by >> putting the state components into init state). >> >>> I'm still waiting on a more formal statement from AMD, and don't yet >>> have any perf numbers on their hardware. >>> >>> However, as we will definitely get an extra perf boost from fully >>> deleting the remaining lazy paths (no more clts/stts in the context >>> switch path), my gut feeing is that there is going to have to be some >>> terrible chronic case on AMD for for us to consider not switching to >>> fully eager. >> Yes, eliminating in particular the stts() is certainly going to help >> performance. With ever growing state sizes I'm not convinced though >> that in the long run (and even already with AVX-512, with its well over >> 2k of state) the CR0 access is indeed (going to remain) worse than the >> (perhaps unnecessary) state load. > > You've got to consider what code does in practice, and in practice code > is either number crunching heavily (in which case eager is definitely > the best option), or its using vzeroall/upper/etc in which case you're > not loading 2k of state, and eager is still the better option. You realize that vzeroall / vzeroupper don't touch the high 16 registers (at least as per the doc, I'm yet to verify this on hardware)? Together with the mask registers and other components, that's still way more than 1k then. And as said - the set is only ever growing. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
>>> On 16.08.18 at 12:42, wrote: > On 16/08/18 10:55, Roger Pau Monné wrote: >> On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: >>> @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) >>> ideal_nops = k8_nops; >>> break; >>> } >>> + >>> +#ifdef HAVE_AS_NOP_DIRECTIVE >>> +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == 0 >>> ) >>> +toolchain_nops_are_ideal = true; >>> +#endif >> You are only comparing that the biggest nop instruction (9 bytes >> AFAICT) generated by the assembler is what Xen believes to be the more >> optimized version. What about shorter nops? > > They are all variations on a theme. > > For P6 nops, its the 0f 1f root which is important, which takes a modrm > byte. Traditionally, its always encoded with eax and uses redundant > memory encodings for longer instructions. > > I can't think of any way of detecting if the optimised nops if the > toolchain starts using alternative registers in the encoding, but I > expect this case won't happen in practice. It's not just the register encoding, but also the maximum single-insn length that gets generated. Recall that until not very long ago we had up to 8-byte NOP insns only? The view on the mod (as in ModRM) usage may vary over time, as may the view on which or how many prefixes are reasonable to have. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86/xsave: prefer eager clearing of state over eager restoring
On 16/08/18 11:03, Jan Beulich wrote: On 16.08.18 at 11:07, wrote: >> On 22/06/2018 11:57, Jan Beulich wrote: >>> --- a/xen/arch/x86/spec_ctrl.c >>> +++ b/xen/arch/x86/spec_ctrl.c >>> @@ -616,7 +616,7 @@ void __init init_speculation_mitigations >>> >>> /* Check whether Eager FPU should be enabled by default. */ >>> if ( opt_eager_fpu == -1 ) >>> -opt_eager_fpu = should_use_eager_fpu(); >>> +opt_eager_fpu = !cpu_has_xsave && should_use_eager_fpu(); >> I'd not spotted this the first time round. >> >> Intel is very clear that, if you're using xsave, you should be using >> eager FPU. Therefore, this goes specifically against the advice in the >> ORM, and the advise we were given during the LazyFPU timeframe. >> >> Furthermore we (XenServer) and customers have seen a reliable perf >> improvement from the LazyFPU security fix, up to 8% in places, for >> normal VDI and server workloads. As I said during the development the >> LazyFPU fixes, this is almost certainly down to the fact that all code >> uses the FPU these days. > Well - as said in the description, observation in my tests (which are > not a typical server workload) were that about 50% of the context > switches were no followed by a (lazy) restore, until the vCPU was > de-scheduled again. Counting absolute numbers gives a false impression. You've got to account for the relative difference in cycles between an xrstor and servicing #NM (which includes the xrstor you previously skipped). The 50/50 split you see here is definitely going to result in a net perf hit because servicing #NM is several orders of magnitude more expensive than xrstor. (For HVM guests, you've got to add another order of magnitude for the vmexit). (At a guess, seeing as its been a little too long since I last did this kind of stats), you've got to get to somewhere like 85-95% before you're likely to break even from a performance point of view. > The change as presented is in fact trying to move to a middle ground, > in that it doesn't leave stale state in the registers anymore, but > instead frees the underlying physical ones up for other uses (by > putting the state components into init state). > >> I'm still waiting on a more formal statement from AMD, and don't yet >> have any perf numbers on their hardware. >> >> However, as we will definitely get an extra perf boost from fully >> deleting the remaining lazy paths (no more clts/stts in the context >> switch path), my gut feeing is that there is going to have to be some >> terrible chronic case on AMD for for us to consider not switching to >> fully eager. > Yes, eliminating in particular the stts() is certainly going to help > performance. With ever growing state sizes I'm not convinced though > that in the long run (and even already with AVX-512, with its well over > 2k of state) the CR0 access is indeed (going to remain) worse than the > (perhaps unnecessary) state load. You've got to consider what code does in practice, and in practice code is either number crunching heavily (in which case eager is definitely the best option), or its using vzeroall/upper/etc in which case you're not loading 2k of state, and eager is still the better option. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
>>> On 16.08.18 at 12:56, wrote: > On 16/08/18 11:29, Jan Beulich wrote: >> Following some further discussion with Andrew, he looks to be >> convinced that the issue is to be fixed in the balloon driver, >> which so far (intentionally afaict) does not remove the direct >> map entries for ballooned out pages in the HVM case. I'm not >> convinced of this, but I'd nevertheless like to inquire whether >> such a change (resulting in shattered super page mappings) >> would be acceptable in the first place. > > We don't tolerate anything else in the directmap pointing to > invalid/unimplemented frames. Why should ballooning be any different? Because ballooning is something virtualization specific, which doesn't have any equivalent on bare hardware (memory hot unplug doesn't come quite close enough imo, not the least because that doesn't work on a page granular basis). Hence we're to define the exact behavior here, and hence such a definition could as well include special behavior of accesses to the involved guest-physical addresses. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 9/9] x86: move declaration of arch_set_info_hvm_guest and provide stub
>>> On 16.08.18 at 12:42, wrote: > On Tue, Aug 07, 2018 at 05:24:15PM +0100, Wei Liu wrote: > > >> > All uses sit either in HVM-specific code or inside is_hvm_...() >> > conditionals: Why do we need the inline stub? If the declaration >> > was visible independent of CONFIG_HVM, code would compile >> > fine, and references to the function would be removed by the >> > compiler, so linking would also succeed despite there not being >> > any definition of the function. >> >> Last time I tried DCE wasn't working so well. Let me try again and if >> DCE works it would save me a lot of effort to provide stubs. >> > > DCE seems to work better this time. > > The only problem is that some ASSERTs will need to turn into panic or > BUG() if we want to fully utilise DCE. Is that okay? In general yes, I think so. > To give you an example, after making is_hvm_domain evaluate to 0 when > !CONFIG_HVM, vm_event_fill_regs + !CONFIG_HVM compiles fine for debug > build because ASSERT hints the compiler that the rest of the function is > never going to be reachable. But for non-debug build, ASSERT is empty, > so compiler will not eliminate the rest of the function, complaining > hvm_get_segment_register is not available. It is solvable by adding > panic or BUG. > > There is going to be quite a few cases like that. I haven't gone through > all of them. In cases like the example you give I'm not convinced of the suggested conversion though - the entire function should then live inside CONFIG_HVM (or in a file built for HVM only). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 125950: regressions - FAIL
flight 125950 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125950/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf 6 xen-buildfail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen 70d8543950ad045fddcb78ae11302e713ef09c76 baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z0 days Failing since125928 2018-08-15 19:00:49 Z0 days8 attempts Testing same since 125950 2018-08-16 09:00:31 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Jan Beulich Julien Grall Paul Durrant Wei Liu Zhenzhong Duan jobs: build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked 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 Not pushing. commit 70d8543950ad045fddcb78ae11302e713ef09c76 Author: Zhenzhong Duan Date: Thu Aug 16 09:31:57 2018 +0200 x86/mmcfg: remove redundant code in pci_mmcfg_reject_broken() No functional change. Signed-off-by: Zhenzhong Duan Acked-by: Jan Beulich commit a9e9837f54973ac45488c24e93ed34cbf20e4c66 Author: Jan Beulich Date: Thu Aug 16 09:30:59 2018 +0200 gnttab/ARM: properly implement gnttab_create_status_page() Prevent the "BUG_ON(page_get_owner(pg) != d)" in gnttab_unpopulate_status_frames() from triggering. Reported-by: 王磊 Signed-off-by: Jan Beulich Reviewed-by: Stefano Stabellini commit 7626edeaca972e3e823535dcc44338f6b2f0b21f Author: Paul Durrant Date: Thu Aug 16 09:27:30 2018 +0200 x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries When emulating a rep I/O operation it is possible that the ioreq will describe a single operation that spans multiple GFNs. This is fine as long as all those GFNs fall within an MMIO region covered by a single device model, but unfortunately the higher levels of the emulation code do not guarantee that. This is something that should almost certainly be fixed, but in the meantime this patch makes sure that MMIO is truncated at GFN boundaries and hence the appropriate device model is re-evaluated for each target GFN. NOTE: This patch does not deal with the case of a single MMIO operation spanning a GFN boundary. That is more complex to deal with and is deferred to a subsequent patch. Signed-off-by: Paul Durrant Convert calculations to be 32-bit only. Signed-off-by: Jan Beulich commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into evtchn_init() ... rather than setting it up once domain_create() has completed. This involves constructing a default value for dom0. No practical change in functionality. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Acked-by: Julien Grall commit 4a83497635056d33fe20ef705f35617b1003a276 Author: Andrew Cooper Date: Tue Feb 27 17:39:37 2018 + xen/domctl: Merge set_max_evtchn into createdomain set_max_evtchn is somewhat weird. It was introduced with the event_fifo work, but has never been used. Still, it is a bounding on resources consumed by the event channel infrastructure, and should be part of createdomain, rather than editable after the fact. Drop XEN_DOMCTL_set_max_evtchn completely (including XSM hooks and libxc wrappers), and retain the functionality in
Re: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
On 13/08/18 07:50, Jan Beulich wrote: On 10.08.18 at 18:37, wrote: >> On 10/08/18 17:30, George Dunlap wrote: >>> Sorry, what exactly is the issue here? Linux has a function called >>> load_unaligned_zeropad() which is reading into a ballooned region? > Yes. > >>> Fundamentally, a ballooned page is one which has been allocated to a >>> device driver. I'm having a hard time coming up with a justification >>> for having code which reads memory owned by B in the process of reading >>> memory owned by A. Or is there some weird architectural reason that I'm >>> not aware of? > Well, they do this no matter who owns the successive page (or > perhaps at a smaller granularity also the successive allocation). > I guess their goal is to have just a single MOV in the common > case (with the caller ignoring the uninteresting to it high bytes), > while recovering gracefully from #PF should one occur. > >> The underlying issue is that the emulator can't cope with a single >> misaligned access which crosses RAM and MMIO. It gives up and >> presumably throws #UD back. > We wouldn't have observed any problem if there was #UD in > such a case, as Linux'es fault recovery code doesn't care what > kind of fault has occurred. We're getting back a result of all > ones, even for the part of the read that has actually hit the > last few bytes of the present page. > >> One longstanding Xen bug is that simply ballooning a page out shouldn't >> be able to trigger MMIO emulation to begin with. It is a side effect of >> mixed p2m types, and the fix for this to have Xen understand the guest >> physmap layout. > And hence the consideration of mapping in an all zeros page > instead. This is because of the way __hvmemul_read() / > __hvm_copy() work: The latter doesn't tell its caller how many > bytes it was able to read, and hence the former considers the > entire range MMIO (and forwards the request for emulation). > Of course all of this is an issue only because > hvmemul_virtual_to_linear() sees no need to split the request > at the page boundary, due to the balloon driver having left in > place the mapping of the ballooned out page. Actually, the more I think about this, the more of a bad idea emulating a zero page is. It gives the illusion of a working piece of zeroed ram, except that writes definitely can't take effect. Its going to make bugs even more subtle. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
On 16/08/18 11:29, Jan Beulich wrote: On 13.08.18 at 08:50, wrote: > On 10.08.18 at 18:37, wrote: >>> On 10/08/18 17:30, George Dunlap wrote: Sorry, what exactly is the issue here? Linux has a function called load_unaligned_zeropad() which is reading into a ballooned region? >> Yes. >> Fundamentally, a ballooned page is one which has been allocated to a device driver. I'm having a hard time coming up with a justification for having code which reads memory owned by B in the process of reading memory owned by A. Or is there some weird architectural reason that I'm not aware of? >> Well, they do this no matter who owns the successive page (or >> perhaps at a smaller granularity also the successive allocation). >> I guess their goal is to have just a single MOV in the common >> case (with the caller ignoring the uninteresting to it high bytes), >> while recovering gracefully from #PF should one occur. >> >>> The underlying issue is that the emulator can't cope with a single >>> misaligned access which crosses RAM and MMIO. It gives up and >>> presumably throws #UD back. >> We wouldn't have observed any problem if there was #UD in >> such a case, as Linux'es fault recovery code doesn't care what >> kind of fault has occurred. We're getting back a result of all >> ones, even for the part of the read that has actually hit the >> last few bytes of the present page. >> >>> One longstanding Xen bug is that simply ballooning a page out shouldn't >>> be able to trigger MMIO emulation to begin with. It is a side effect of >>> mixed p2m types, and the fix for this to have Xen understand the guest >>> physmap layout. >> And hence the consideration of mapping in an all zeros page >> instead. This is because of the way __hvmemul_read() / >> __hvm_copy() work: The latter doesn't tell its caller how many >> bytes it was able to read, and hence the former considers the >> entire range MMIO (and forwards the request for emulation). >> Of course all of this is an issue only because >> hvmemul_virtual_to_linear() sees no need to split the request >> at the page boundary, due to the balloon driver having left in >> place the mapping of the ballooned out page. >> >> Obviously the opposite case (access starting in a ballooned >> out page and crossing into an "ordinary" one) would have a >> similar issue, which is presumably even harder to fix without >> going the map-a-zero-page route (or Paul's suggested >> null_handler hack). >> >>> However, the real bug is Linux making such a misaligned access into a >>> ballooned out page in the first place. This is a Linux kernel bug which >>> (presumably) manifests in a very obvious way due to shortcomings in >>> Xen's emulation handling. >> I wouldn't dare to judge whether this is a bug, especially in >> light that they recover gracefully from the #PF that might result in >> the native case. Arguably the caller has to have some knowledge >> about what might live in the following page, as to not inadvertently >> hit an MMIO page rather than a non-present mapping. But I'd >> leave such judgment to them; our business is to get working a case >> that is working without Xen underneath. > Following some further discussion with Andrew, he looks to be > convinced that the issue is to be fixed in the balloon driver, > which so far (intentionally afaict) does not remove the direct > map entries for ballooned out pages in the HVM case. I'm not > convinced of this, but I'd nevertheless like to inquire whether > such a change (resulting in shattered super page mappings) > would be acceptable in the first place. We don't tolerate anything else in the directmap pointing to invalid/unimplemented frames. Why should ballooning be any different? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
On 16/08/18 10:55, Roger Pau Monné wrote: > On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: >> Newer versions of binutils are capable of emitting an exact number bytes >> worth >> of optimised nops, which are P6 nops. Use this in preference to .skip when >> available. >> >> Check at boot time whether the toolchain nops are the correct for the running >> hardware, andskip optimising nops entirely when possible. >^ missing space. > > TBH I'm not sure I see the benefit of using .nops over using .skip. In this case, or in general? In general, so we don't need to self/cross modify the alternatives points which aren't patched. In this case, because it is the .nops directive we're using to insert nops. > Xen needs to do a memcmp in order to check whether the resulting nops > are what Xen considers the more optimized instructions for the CPU > currently running on. Xen can avoid the memcpy by using skip, because > in that case Xen knows exactly the current instructions and there's no > need to memcmp. I'm afraid I don't understand what point you are attempting to make here. > I guess the reason is that the memcmp will be done only once, and > hopefully in most cases the assembler generated nops will be the most > optimized version. The memcmp() is once during init, and you've got to be on very ancient hardware for the toolchain nops to not be the correct ones. I'm going to conservatively estimate that 98% of hardware running Xen will have P6 nops as ideal. >> Signed-off-by: Andrew Cooper >> --- >> CC: Jan Beulich >> CC: Konrad Rzeszutek Wilk >> CC: Roger Pau Monné >> CC: Wei Liu >> --- >> xen/arch/x86/Rules.mk | 4 >> xen/arch/x86/alternative.c| 20 +++- >> xen/include/asm-x86/alternative-asm.h | 12 +++- >> xen/include/asm-x86/alternative.h | 11 +-- >> 4 files changed, 43 insertions(+), 4 deletions(-) >> >> diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk >> index ac585a3..c84ed20 100644 >> --- a/xen/arch/x86/Rules.mk >> +++ b/xen/arch/x86/Rules.mk >> @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid >> (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) >> $(call as-option-add,CFLAGS,CC,\ >> ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) >> >> +# Check to see whether the assmbler supports the .nop directive. >> +$(call as-option-add,CFLAGS,CC,\ >> +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) > I think I remember commenting on an earlier version of this about the > usage of the CONTROL parameter. I would expect the assembler to > use the most optimized version by default, is that not the case? Again, I don't understand what you're trying to say. This expression is like this, because that's how we actually use it. > >> + >> CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables >> >> # Xen doesn't use SSE interally. If the compiler supports it, also skip the >> diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c >> index 0ef7a8b..2c844d6 100644 >> --- a/xen/arch/x86/alternative.c >> +++ b/xen/arch/x86/alternative.c >> @@ -84,6 +84,19 @@ static const unsigned char * const p6_nops[ASM_NOP_MAX+1] >> init_or_livepatch_cons >> >> static const unsigned char * const *ideal_nops init_or_livepatch_data = >> p6_nops; >> >> +#ifdef HAVE_AS_NOP_DIRECTIVE >> + >> +/* Nops in .init.rodata to compare against the runtime ideal nops. */ >> +asm ( ".pushsection .init.rodata, \"a\", @progbits\n\t" >> + "toolchain_nops: .nops " __stringify(ASM_NOP_MAX) "\n\t" >> + ".popsection\n\t"); >> +extern char toolchain_nops[ASM_NOP_MAX]; >> +static bool __read_mostly toolchain_nops_are_ideal; >> + >> +#else >> +# define toolchain_nops_are_ideal false >> +#endif >> + >> static void __init arch_init_ideal_nops(void) >> { >> switch ( boot_cpu_data.x86_vendor ) >> @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) >> ideal_nops = k8_nops; >> break; >> } >> + >> +#ifdef HAVE_AS_NOP_DIRECTIVE >> +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == 0 ) >> +toolchain_nops_are_ideal = true; >> +#endif > You are only comparing that the biggest nop instruction (9 bytes > AFAICT) generated by the assembler is what Xen believes to be the more > optimized version. What about shorter nops? They are all variations on a theme. For P6 nops, its the 0f 1f root which is important, which takes a modrm byte. Traditionally, its always encoded with eax and uses redundant memory encodings for longer instructions. I can't think of any way of detecting if the optimised nops if the toolchain starts using alternative registers in the encoding, but I expect this case won't happen in practice. > I also see a chance that maybe newer assembler versions will at some > point generate more optimized nops, but Xen will replace them with not > so optimized versions if the
[Xen-devel] [xen-4.11-testing test] 125909: tolerable FAIL - PUSHED
flight 125909 xen-4.11-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/125909/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-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-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass 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-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: xen d757c29ffe2e31b15397e43cd58da88b6318b654 baseline version: xen 33ced725e11af4eabd3334d12f53ed807e9e2586 Last test of basis 125674 2018-07-30 09:36:42 Z 17 days Testing same since 125909 2018-08-14 17:06:45 Z1 days1 attempts People who touched revisions under test: Andrew Cooper Christian Lindig George Dunlap Jan Beulich Juergen Gross Kevin Tian Stefano Stabellini 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 pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-prev pass build-i386-prev pass build-amd64-pvopspass build-armhf-pvopspass
Re: [Xen-devel] [PATCH] x86/mmcfg/drhd: Move acpi_mmcfg_init() before calling acpi_parse_dmar()
>>> On 16.08.18 at 11:30, wrote: > On 2018/8/16 17:13, Zhenzhong Duan wrote: --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1493,6 +1493,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) generic_apic_probe(); +pt_pci_init(); + +acpi_mmcfg_init(); + acpi_boot_init(); >>> >>> With the dependency being _in_ acpi_boot_init(), the invocation of >>> acpi_mmcfg_init() should now be moved there. >> Yes, I feel better to move pt_pci_init() and acpi_mmcfg_init() in >> acpi_boot_init() before acpi_mmcfg_init(). Any more comments? > I see acpi_boot_init() is empty func when CONFIG_ACPI_BOOT isn't set. Do > we support disabling this config option? If yes, I think above change > will break non-acpi case. I'm sorry, but I'm lost: grep produces no single hit on my tree when looking for ACPI_BOOT. Are you looking at some older tree? Even when considering ACPI - that Kconfig option exists only for ARM's purposes right now. If you were to make it user selectable, I think you'd first have to fix a number of build issues in case it got turned off. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/mmcfg/drhd: Move acpi_mmcfg_init() before calling acpi_parse_dmar()
>>> On 16.08.18 at 11:13, wrote: > On 2018/8/16 15:10, Jan Beulich wrote: >> Have you investigated the alternative of deferring acpi_dmar_init() >> to a later point, or at least the part of it that needs to do PCI >> config space accesses? I'm not currently convinced the device scope >> parsing needs to happen that early: Neither iommu_supports_eim() >> nor iommu_enable_x2apic_IR() look to depend on that information >> at the first glance, and I think these are the routines that require >> (part of) the DMAR parsing to happen early. > I moved acpi_mmcfg_init() ahead of acpi_boot_init() because below code > sequence depending on pci_mmcfg_virt being correctly setup. > acpi_dmar_init >->acpi_parse_dmar > ->acpi_parse_one_drhd >->acpi_parse_dev_scope > ->pci_conf_read8 >->pci_mmcfg_read > ->pci_dev_base >->get_virt Have you read my previous response in full? I'm specifically asking whether the device scope parsing (i.e. acpi_parse_dev_scope()) really needs to happen as early as it does now. Without that, the dependency on MMCFG accesses working would go away. >>> --- a/xen/arch/x86/setup.c >>> +++ b/xen/arch/x86/setup.c >>> @@ -1493,6 +1493,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) >>> >>> generic_apic_probe(); >>> >>> +pt_pci_init(); >>> + >>> +acpi_mmcfg_init(); >>> + >>> acpi_boot_init(); >> >> With the dependency being _in_ acpi_boot_init(), the invocation of >> acpi_mmcfg_init() should now be moved there. > Yes, I feel better to move pt_pci_init() and acpi_mmcfg_init() in > acpi_boot_init() before acpi_mmcfg_init(). I didn't say move both, did I? That said, now having looked at what it actually does, I think you want to rename it if the suggested alternative route can't be used, as in particular the pt_ prefix is quite misleading then. Once that has happened, moving the invocation perhaps even _into_ acpi_mcfg_init() might be reasonable. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/2] MMIO emulation fixes
>>> On 13.08.18 at 08:50, wrote: On 10.08.18 at 18:37, wrote: > > On 10/08/18 17:30, George Dunlap wrote: > >> Sorry, what exactly is the issue here? Linux has a function called > >> load_unaligned_zeropad() which is reading into a ballooned region? > > Yes. > > >> Fundamentally, a ballooned page is one which has been allocated to a > >> device driver. I'm having a hard time coming up with a justification > >> for having code which reads memory owned by B in the process of reading > >> memory owned by A. Or is there some weird architectural reason that I'm > >> not aware of? > > Well, they do this no matter who owns the successive page (or > perhaps at a smaller granularity also the successive allocation). > I guess their goal is to have just a single MOV in the common > case (with the caller ignoring the uninteresting to it high bytes), > while recovering gracefully from #PF should one occur. > > > The underlying issue is that the emulator can't cope with a single > > misaligned access which crosses RAM and MMIO. It gives up and > > presumably throws #UD back. > > We wouldn't have observed any problem if there was #UD in > such a case, as Linux'es fault recovery code doesn't care what > kind of fault has occurred. We're getting back a result of all > ones, even for the part of the read that has actually hit the > last few bytes of the present page. > > > One longstanding Xen bug is that simply ballooning a page out shouldn't > > be able to trigger MMIO emulation to begin with. It is a side effect of > > mixed p2m types, and the fix for this to have Xen understand the guest > > physmap layout. > > And hence the consideration of mapping in an all zeros page > instead. This is because of the way __hvmemul_read() / > __hvm_copy() work: The latter doesn't tell its caller how many > bytes it was able to read, and hence the former considers the > entire range MMIO (and forwards the request for emulation). > Of course all of this is an issue only because > hvmemul_virtual_to_linear() sees no need to split the request > at the page boundary, due to the balloon driver having left in > place the mapping of the ballooned out page. > > Obviously the opposite case (access starting in a ballooned > out page and crossing into an "ordinary" one) would have a > similar issue, which is presumably even harder to fix without > going the map-a-zero-page route (or Paul's suggested > null_handler hack). > > > However, the real bug is Linux making such a misaligned access into a > > ballooned out page in the first place. This is a Linux kernel bug which > > (presumably) manifests in a very obvious way due to shortcomings in > > Xen's emulation handling. > > I wouldn't dare to judge whether this is a bug, especially in > light that they recover gracefully from the #PF that might result in > the native case. Arguably the caller has to have some knowledge > about what might live in the following page, as to not inadvertently > hit an MMIO page rather than a non-present mapping. But I'd > leave such judgment to them; our business is to get working a case > that is working without Xen underneath. Following some further discussion with Andrew, he looks to be convinced that the issue is to be fixed in the balloon driver, which so far (intentionally afaict) does not remove the direct map entries for ballooned out pages in the HVM case. I'm not convinced of this, but I'd nevertheless like to inquire whether such a change (resulting in shattered super page mappings) would be acceptable in the first place. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
>>> On 16.08.18 at 11:55, wrote: > On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: >> --- a/xen/arch/x86/Rules.mk >> +++ b/xen/arch/x86/Rules.mk >> @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid >> (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) >> $(call as-option-add,CFLAGS,CC,\ >> ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) >> >> +# Check to see whether the assmbler supports the .nop directive. >> +$(call as-option-add,CFLAGS,CC,\ >> +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) > > I think I remember commenting on an earlier version of this about the > usage of the CONTROL parameter. I would expect the assembler to > use the most optimized version by default, is that not the case? How could it, without knowing what the target hardware is? And even if it could, what is considered "most optimized" tends to change every once in a while (or else we wouldn't have different NOP flavors to begin with). Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86/xsave: prefer eager clearing of state over eager restoring
>>> On 16.08.18 at 11:07, wrote: > On 22/06/2018 11:57, Jan Beulich wrote: >> --- a/xen/arch/x86/spec_ctrl.c >> +++ b/xen/arch/x86/spec_ctrl.c >> @@ -616,7 +616,7 @@ void __init init_speculation_mitigations >> >> /* Check whether Eager FPU should be enabled by default. */ >> if ( opt_eager_fpu == -1 ) >> -opt_eager_fpu = should_use_eager_fpu(); >> +opt_eager_fpu = !cpu_has_xsave && should_use_eager_fpu(); > > I'd not spotted this the first time round. > > Intel is very clear that, if you're using xsave, you should be using > eager FPU. Therefore, this goes specifically against the advice in the > ORM, and the advise we were given during the LazyFPU timeframe. > > Furthermore we (XenServer) and customers have seen a reliable perf > improvement from the LazyFPU security fix, up to 8% in places, for > normal VDI and server workloads. As I said during the development the > LazyFPU fixes, this is almost certainly down to the fact that all code > uses the FPU these days. Well - as said in the description, observation in my tests (which are not a typical server workload) were that about 50% of the context switches were no followed by a (lazy) restore, until the vCPU was de-scheduled again. The change as presented is in fact trying to move to a middle ground, in that it doesn't leave stale state in the registers anymore, but instead frees the underlying physical ones up for other uses (by putting the state components into init state). > I'm still waiting on a more formal statement from AMD, and don't yet > have any perf numbers on their hardware. > > However, as we will definitely get an extra perf boost from fully > deleting the remaining lazy paths (no more clts/stts in the context > switch path), my gut feeing is that there is going to have to be some > terrible chronic case on AMD for for us to consider not switching to > fully eager. Yes, eliminating in particular the stts() is certainly going to help performance. With ever growing state sizes I'm not convinced though that in the long run (and even already with AVX-512, with its well over 2k of state) the CR0 access is indeed (going to remain) worse than the (perhaps unnecessary) state load. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/build: Use new .nops directive when available
On Wed, Aug 15, 2018 at 06:57:38PM +0100, Andrew Cooper wrote: > Newer versions of binutils are capable of emitting an exact number bytes worth > of optimised nops, which are P6 nops. Use this in preference to .skip when > available. > > Check at boot time whether the toolchain nops are the correct for the running > hardware, andskip optimising nops entirely when possible. ^ missing space. TBH I'm not sure I see the benefit of using .nops over using .skip. Xen needs to do a memcmp in order to check whether the resulting nops are what Xen considers the more optimized instructions for the CPU currently running on. Xen can avoid the memcpy by using skip, because in that case Xen knows exactly the current instructions and there's no need to memcmp. I guess the reason is that the memcmp will be done only once, and hopefully in most cases the assembler generated nops will be the most optimized version. > Signed-off-by: Andrew Cooper > --- > CC: Jan Beulich > CC: Konrad Rzeszutek Wilk > CC: Roger Pau Monné > CC: Wei Liu > --- > xen/arch/x86/Rules.mk | 4 > xen/arch/x86/alternative.c| 20 +++- > xen/include/asm-x86/alternative-asm.h | 12 +++- > xen/include/asm-x86/alternative.h | 11 +-- > 4 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/x86/Rules.mk b/xen/arch/x86/Rules.mk > index ac585a3..c84ed20 100644 > --- a/xen/arch/x86/Rules.mk > +++ b/xen/arch/x86/Rules.mk > @@ -29,6 +29,10 @@ $(call as-option-add,CFLAGS,CC,"invpcid > (%rax)$$(comma)%rax",-DHAVE_AS_INVPCID) > $(call as-option-add,CFLAGS,CC,\ > ".if ((1 > 0) < 0); .error \"\";.endif",,-DHAVE_AS_NEGATIVE_TRUE) > > +# Check to see whether the assmbler supports the .nop directive. > +$(call as-option-add,CFLAGS,CC,\ > +".L1: .L2: .nops (.L2 - .L1)$$(comma)9",-DHAVE_AS_NOP_DIRECTIVE) I think I remember commenting on an earlier version of this about the usage of the CONTROL parameter. I would expect the assembler to use the most optimized version by default, is that not the case? > + > CFLAGS += -mno-red-zone -fpic -fno-asynchronous-unwind-tables > > # Xen doesn't use SSE interally. If the compiler supports it, also skip the > diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c > index 0ef7a8b..2c844d6 100644 > --- a/xen/arch/x86/alternative.c > +++ b/xen/arch/x86/alternative.c > @@ -84,6 +84,19 @@ static const unsigned char * const p6_nops[ASM_NOP_MAX+1] > init_or_livepatch_cons > > static const unsigned char * const *ideal_nops init_or_livepatch_data = > p6_nops; > > +#ifdef HAVE_AS_NOP_DIRECTIVE > + > +/* Nops in .init.rodata to compare against the runtime ideal nops. */ > +asm ( ".pushsection .init.rodata, \"a\", @progbits\n\t" > + "toolchain_nops: .nops " __stringify(ASM_NOP_MAX) "\n\t" > + ".popsection\n\t"); > +extern char toolchain_nops[ASM_NOP_MAX]; > +static bool __read_mostly toolchain_nops_are_ideal; > + > +#else > +# define toolchain_nops_are_ideal false > +#endif > + > static void __init arch_init_ideal_nops(void) > { > switch ( boot_cpu_data.x86_vendor ) > @@ -112,6 +125,11 @@ static void __init arch_init_ideal_nops(void) > ideal_nops = k8_nops; > break; > } > + > +#ifdef HAVE_AS_NOP_DIRECTIVE > +if ( memcmp(ideal_nops[ASM_NOP_MAX], toolchain_nops, ASM_NOP_MAX) == 0 ) > +toolchain_nops_are_ideal = true; > +#endif You are only comparing that the biggest nop instruction (9 bytes AFAICT) generated by the assembler is what Xen believes to be the more optimized version. What about shorter nops? I also see a chance that maybe newer assembler versions will at some point generate more optimized nops, but Xen will replace them with not so optimized versions if the Xen logic is not so up to date. > } > > /* Use this to add nops to a buffer, then text_poke the whole buffer. */ > @@ -209,7 +227,7 @@ void init_or_livepatch apply_alternatives(struct > alt_instr *start, > base->priv = 1; > > /* Nothing useful to do? */ > -if ( a->pad_len <= 1 ) > +if ( toolchain_nops_are_ideal || a->pad_len <= 1 ) > continue; > > add_nops(buf, a->pad_len); > diff --git a/xen/include/asm-x86/alternative-asm.h > b/xen/include/asm-x86/alternative-asm.h > index 0b61516..0d6fb4b 100644 > --- a/xen/include/asm-x86/alternative-asm.h > +++ b/xen/include/asm-x86/alternative-asm.h > @@ -1,6 +1,8 @@ > #ifndef _ASM_X86_ALTERNATIVE_ASM_H_ > #define _ASM_X86_ALTERNATIVE_ASM_H_ > > +#include > + > #ifdef __ASSEMBLY__ > > /* > @@ -19,6 +21,14 @@ > .byte 0 /* priv */ > .endm > > +.macro mknops nr_bytes > +#ifdef HAVE_AS_NOP_DIRECTIVE > +.nops \nr_bytes, ASM_NOP_MAX > +#else > +.skip \nr_bytes, 0x90 Use P6_NOP1 instead of open coding 0x90? Or have a #define DEFAULT_NOP P6_NOP1 And use it instead. Thanks, Roger.
[Xen-devel] [qemu-mainline test] 125905: trouble: broken/fail/pass
flight 125905 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/125905/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-credit2 broken build-arm64 broken in 125884 build-arm64-xsm broken in 125884 build-arm64-pvopsbroken in 125884 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-credit2 4 host-install(4) broken pass in 125884 Regressions which are regarded as allowable (not blocking): build-arm64-pvops 2 hosts-allocate broken in 125884 REGR. vs. 125640 build-arm64-xsm 2 hosts-allocate broken in 125884 REGR. vs. 125640 build-arm64 2 hosts-allocate broken in 125884 REGR. vs. 125640 Tests which did not succeed, but are not blocking: build-arm64-libvirt 1 build-check(1) blocked in 125884 n/a test-arm64-arm64-xl 1 build-check(1) blocked in 125884 n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked in 125884 n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked in 125884 n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked in 125884 n/a build-arm64-pvops3 capture-logs broken in 125884 blocked in 125640 build-arm64 3 capture-logs broken in 125884 blocked in 125640 build-arm64-xsm 3 capture-logs broken in 125884 blocked in 125640 test-armhf-armhf-xl-credit2 13 migrate-support-check fail in 125884 never pass test-armhf-armhf-xl-credit2 14 saverestore-support-check fail in 125884 never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125640 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125640 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125640 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125640 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125640 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 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-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-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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop 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: qemuu6ad90805383e6d04b3ff49681b8519a48c9f4410 baseline version: qemuu18a398f6a39df4b08ff86ac0d38384193ca5f4cc Last test of basis 125640 2018-07-27 22:16:44 Z 19 days Failing since125675 2018-07-30 09:36:58 Z 17 days 11 attempts Testing same since 125809 2018-08-08 15:11:18 Z7 days5 attempts People who touched revisions under test: Alex Bennée BALATON Zoltan Christian Borntraeger Cornelia Huck David Gibson Dou Liyang Dr. David Alan Gilbert Fam Zheng Geert Uytterhoeven Igor Mammedov Jay Zhou Kevin Wolf KONRAD
Re: [Xen-devel] [PATCH] x86: make arch_set_info_guest() match comments in load_segments()
On 10/07/18 11:13, Jan Beulich wrote: > For both fs_base and gs_base_user, there are comments saying "This can > only be non-zero if selector is NULL." While save_segments() ensures > this, so far arch_set_info_guest() didn't. Make behavior consistent > (attaching comments identical to those in save_segments()). > > Signed-off-by: Jan Beulich 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] x86/hvm/viridian: set shutdown_code in response to CrashNotify
On 10/08/18 16:59, Paul Durrant wrote: >> -Original Message- >> From: Andrew Cooper >> Sent: 10 August 2018 16:57 >> To: Paul Durrant ; xen-devel@lists.xenproject.org >> Cc: Jan Beulich >> Subject: Re: [PATCH] x86/hvm/viridian: set shutdown_code in response to >> CrashNotify >> >> On 10/08/18 16:43, Paul Durrant wrote: >>> When Windows writes the CrashNotify bit in the CRASH_CTL MSR then we >> know >>> it is crashing, so set the domain shutdown code appropriately. >>> >>> Signed-off-by: Paul Durrant >>> --- >>> Cc: Jan Beulich >>> Cc: Andrew Cooper >>> --- >>> xen/arch/x86/hvm/viridian.c | 4 >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c >>> index 486065182c..294cf486cc 100644 >>> --- a/xen/arch/x86/hvm/viridian.c >>> +++ b/xen/arch/x86/hvm/viridian.c >>> @@ -645,6 +645,10 @@ int wrmsr_viridian_regs(uint32_t idx, uint64_t val) >>> if ( !ctl.u.CrashNotify ) >>> break; >>> >>> +spin_lock(>shutdown_lock); >>> +d->shutdown_code = SHUTDOWN_crash; >>> +spin_unlock(>shutdown_lock); >> How does the domain eventually shut down? > I assume it shuts down when the guest writes to the PIIX. > >> It feels slightly odd to have >> a shutdown code before the domain has finished executing code. >> > That's the norm. The PV drivers (if they are installed) set it via a schedop. > This just makes sure it is set even if the PV drivers aren't there. What happens if the user has configured windows to reboot after a crash? ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/mmcfg/drhd: Move acpi_mmcfg_init() before calling acpi_parse_dmar()
On 2018/8/16 17:13, Zhenzhong Duan wrote: --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1493,6 +1493,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) generic_apic_probe(); + pt_pci_init(); + + acpi_mmcfg_init(); + acpi_boot_init(); With the dependency being _in_ acpi_boot_init(), the invocation of acpi_mmcfg_init() should now be moved there. Yes, I feel better to move pt_pci_init() and acpi_mmcfg_init() in acpi_boot_init() before acpi_mmcfg_init(). Any more comments? I see acpi_boot_init() is empty func when CONFIG_ACPI_BOOT isn't set. Do we support disabling this config option? If yes, I think above change will break non-acpi case. Thanks Zhenzhong ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 25/25] xen/arm: split domain_build.c
Hi Stefano, On 08/16/2018 01:25 AM, Stefano Stabellini wrote: On Mon, 13 Aug 2018, Julien Grall wrote: + +#ifndef CONFIG_ACPI +static inline int prepare_acpi(struct domain *d, struct kernel_info *kinfo) +{ +/* Only booting with ACPI will hit here */ +BUG(); +return -EINVAL; +} +#else +int prepare_acpi(struct domain *d, struct kernel_info *kinfo); +#endif This one should go in asm-arm/acpi.h. So this header is not necessary anymore. I was unable to add prepare_acpi to asm-arm/acpi.h because it causes a #include dependency hell, I am thinking of adding it to asm-arm/domain_build.h. In file included from /local/repos/xen-upstream/xen/include/xen/sched.h:11:0, from /local/repos/xen-upstream/xen/include/asm/domain.h:5, from /local/repos/xen-upstream/xen/include/asm/kernel.h:10, from /local/repos/xen-upstream/xen/include/asm/acpi.h:27, from /local/repos/xen-upstream/xen/include/acpi/platform/aclinux.h:58, from /local/repos/xen-upstream/xen/include/acpi/platform/acenv.h:142, from /local/repos/xen-upstream/xen/include/acpi/acpi.h:56, from /local/repos/xen-upstream/xen/include/xen/acpi.h:33, from pl011.c:307: /local/repos/xen-upstream/xen/include/xen/domain.h:59:31: error: ‘struct xen_domctl_createdomain’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror] struct xen_domctl_createdomain *config); Xen Arm headers are a bit a mess. :/ It looks like create_dom0 lives in setup.h, would it be possible to move the prototypes there? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/mmcfg/drhd: Move acpi_mmcfg_init() before calling acpi_parse_dmar()
On 2018/8/16 15:10, Jan Beulich wrote: On 16.08.18 at 07:10, wrote: On a multiple pci segment system such as HPE Superdome-Flex, pci config space from nonzero segment is accessed with mmcfg during acpi parsing DMAR region. First of all - can you please write a little more helpful (to reviewers) patch description. I had to hunt down the config space accesses instead of you clearly saying where they are (and why they are unavoidably there). Sorry, I'll improve it in v2 Furthermore you also move the invocation of pt_pci_init(), with no explanation at all. I did not want to invest the time to understand why that's needed. I'll add it in v2. I moved pt_pci_init() ahead because pci_add_segment() depending on pci_segments radix tree being initialized. acpi_mmcfg_init ->acpi_parse_mcfg ->pci_add_segment We need to setup mmcfg mapping before that or else drhd isn't correctly setup and devices under it fail to load in dom0. That's the improvement side of the change. Mind also saying a word on why the reordering won't break any other dependencies? After all you move up the functions across dozens of other ones, not the least far ahead of the point where IRQs get enabled the first time. pt_pci_init() initialize pci_segments radix tree, acpi_mmcfg_init() initialize pci_mmcfg_virt[] and setup mmcfg mapping in pci_mmcfg_virt[idx].virt. I thought it's ok to just have these structures initialzed earlier. Have you investigated the alternative of deferring acpi_dmar_init() to a later point, or at least the part of it that needs to do PCI config space accesses? I'm not currently convinced the device scope parsing needs to happen that early: Neither iommu_supports_eim() nor iommu_enable_x2apic_IR() look to depend on that information at the first glance, and I think these are the routines that require (part of) the DMAR parsing to happen early. I moved acpi_mmcfg_init() ahead of acpi_boot_init() because below code sequence depending on pci_mmcfg_virt being correctly setup. acpi_dmar_init ->acpi_parse_dmar ->acpi_parse_one_drhd ->acpi_parse_dev_scope ->pci_conf_read8 ->pci_mmcfg_read ->pci_dev_base ->get_virt --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1493,6 +1493,10 @@ void __init noreturn __start_xen(unsigned long mbi_p) generic_apic_probe(); +pt_pci_init(); + +acpi_mmcfg_init(); + acpi_boot_init(); With the dependency being _in_ acpi_boot_init(), the invocation of acpi_mmcfg_init() should now be moved there. Yes, I feel better to move pt_pci_init() and acpi_mmcfg_init() in acpi_boot_init() before acpi_mmcfg_init(). Any more comments? Thanks Zhenzhong ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86/xsave: prefer eager clearing of state over eager restoring
On 16/08/2018 10:07, Andrew Cooper wrote: > On 22/06/2018 11:57, Jan Beulich wrote: >> --- a/xen/arch/x86/spec_ctrl.c >> +++ b/xen/arch/x86/spec_ctrl.c >> @@ -616,7 +616,7 @@ void __init init_speculation_mitigations >> >> /* Check whether Eager FPU should be enabled by default. */ >> if ( opt_eager_fpu == -1 ) >> -opt_eager_fpu = should_use_eager_fpu(); >> +opt_eager_fpu = !cpu_has_xsave && should_use_eager_fpu(); > I'd not spotted this the first time round. > > Intel is very clear that, if you're using xsave, you should be using > eager FPU. Therefore, this goes specifically against the advice in the > ORM, and the advise we were given during the LazyFPU timeframe. > > Furthermore we (XenServer) and customers have seen a reliable perf > improvement from the LazyFPU security fix, up to 8% in places, for > normal VDI and server workloads. As I said during the development the > LazyFPU fixes, this is almost certainly down to the fact that all code > uses the FPU these days. > > I'm still waiting on a more formal statement from AMD, and don't yet > have any perf numbers on their hardware. > > However, as we will definitely get an extra perf boost from fully > deleting the remaining lazy paths (no more clts/stts in the context > switch path), my gut feeing is that there is going to have to be some > terrible chronic case on AMD for for us to consider not switching to > fully eager. > > Irrespective of what we do here, I'd really like Wei to rebase his work > to remove the lazy fpu logic from the nested virt paths, because its a > no-brainer (perf wise) and comes with a massive amount of code > simplification in Xen. Actually, this reminds me of a bug report given during XenSummit in Nanjing. Once Xen has restored lazy state, we drop the interception of #NM, but we still take a vmexit on the clts. This was from Alibaba iirc, and came in at an astounding 70% perf hit to one particular HPC workload. I think this can be fixed by using the host/guest cr0 mask to allow writes of cr0.ts, in exactly the same way as we have recently gained for cr4.pge. Also, AMD has a specific option for virtualisation of cr0.ts writes, and I can't remember if we're using it or not. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 16/25] xen/arm: rename allocate_memory to allocate_memory_11
Hi Stefano, On 08/15/2018 09:26 PM, Stefano Stabellini wrote: On Mon, 13 Aug 2018, Julien Grall wrote: Hi, On 01/08/18 00:27, Stefano Stabellini wrote: allocate_memory only deals with directly mapped memory. Rename it to allocate_memory_11. Signed-off-by: Stefano Stabellini --- Changes in v3: - add patch --- xen/arch/arm/domain_build.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index 066dd75..ab72c36 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -244,7 +244,8 @@ fail: * (as described above) we allow higher allocations and continue until * that runs out (or we have allocated sufficient dom0 memory). */ -static void __init allocate_memory(struct domain *d, struct kernel_info *kinfo) +static void __init allocate_memory_11(struct domain *d, + struct kernel_info *kinfo) { const unsigned int min_low_order = get_order_from_bytes(min_t(paddr_t, dom0_mem, MB(128))); @@ -2240,7 +2241,7 @@ static int __init construct_domU(struct domain *d, struct dt_device_node *node) /* type must be set before allocate memory */ d->arch.type = kinfo.type; #endif -allocate_memory(d, ); +allocate_memory_11(d, ); I don't think your patches are correctly ordered. This is adding a lot of confusion in the review because the DomU memory layout is fixed, yet here you rename the function to 1:1 mapping. Most likely you want to do add the new memory function before introducing DomU. If I do that there will be no callers for the new function and compilation fails. Bisectibility is the reason why I had to reorder the patches. I understand but I don't want to give the impression that 1:1 mapping is used for guests. I can see a couple of solutions: - Implement allocate_memory in a static inline/#if 0 #endif. - Provide a dummy call for the memory that will be implemented later (similar to you do for construct_domU). Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/pv: Use xmemdup() for cpuidmasks, rather than opencoding it
On Wed, Aug 15, 2018 at 10:54:11AM +0100, Andrew Cooper wrote: > Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH RFC] x86/xsave: prefer eager clearing of state over eager restoring
On 22/06/2018 11:57, Jan Beulich wrote: > --- a/xen/arch/x86/spec_ctrl.c > +++ b/xen/arch/x86/spec_ctrl.c > @@ -616,7 +616,7 @@ void __init init_speculation_mitigations > > /* Check whether Eager FPU should be enabled by default. */ > if ( opt_eager_fpu == -1 ) > -opt_eager_fpu = should_use_eager_fpu(); > +opt_eager_fpu = !cpu_has_xsave && should_use_eager_fpu(); I'd not spotted this the first time round. Intel is very clear that, if you're using xsave, you should be using eager FPU. Therefore, this goes specifically against the advice in the ORM, and the advise we were given during the LazyFPU timeframe. Furthermore we (XenServer) and customers have seen a reliable perf improvement from the LazyFPU security fix, up to 8% in places, for normal VDI and server workloads. As I said during the development the LazyFPU fixes, this is almost certainly down to the fact that all code uses the FPU these days. I'm still waiting on a more formal statement from AMD, and don't yet have any perf numbers on their hardware. However, as we will definitely get an extra perf boost from fully deleting the remaining lazy paths (no more clts/stts in the context switch path), my gut feeing is that there is going to have to be some terrible chronic case on AMD for for us to consider not switching to fully eager. Irrespective of what we do here, I'd really like Wei to rebase his work to remove the lazy fpu logic from the nested virt paths, because its a no-brainer (perf wise) and comes with a massive amount of code simplification in Xen. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-wheezy test] 75073: all pass
flight 75073 distros-debian-wheezy real [real] http://osstest.xensource.com/osstest/logs/75073/ Perfect :-) All tests in this flight passed as required baseline version: flight 75056 jobs: build-amd64 pass build-armhf pass build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-wheezy-netboot-pvgrub pass test-amd64-i386-i386-wheezy-netboot-pvgrub pass test-amd64-i386-amd64-wheezy-netboot-pygrub pass test-amd64-amd64-i386-wheezy-netboot-pygrub pass sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 13/25] xen/arm: introduce create_domUs
Hi Stefano, On 08/15/2018 09:04 PM, Stefano Stabellini wrote: On Mon, 13 Aug 2018, Julien Grall wrote: +void __init create_domUs(void) +{ +struct dt_device_node *node; +struct dt_device_node *chosen = dt_find_node_by_name(dt_host, "chosen"); + +if ( chosen != NULL ) +{ +dt_for_each_child_node(chosen, node) +{ +struct domain *d; +struct xen_domctl_createdomain d_cfg = { +.arch.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE, +.arch.nr_spis = 32, AFAICT, when creating DomU from the toolstack nr_spis will be 0. So why 32 here? Legacy from debug code. It should be 0, unless vpl011 is enabled, in which case it should be 1. I would prefer if we use GUEST_VPL011_SPI - 32. This would make the code bullet-proof for any potential reshuffle of the IRQs. 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 06/12] xen/gnttab: Pass max_{grant, maptrack}_frames into grant_table_create()
Hi Andrew, On 08/15/2018 08:03 PM, Andrew Cooper wrote: Actually, I remember now what the problem was. d->grant_table is an opaque type, so .max_grant_frames can't be accessed. One of my indented bits of cleanup here is to remove the gnttab_dom0_frames() function, because it has no business living in the core grant_table.c Would you be happy if I replaced gnttab_dom0_max() in asm-arm with gnttab_dom0_frames() which accounts for the exiting min(), and means that domain_build.c will be ultimately unchanged? I would be happy with such change. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 125943: regressions - FAIL
flight 125943 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125943/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf 6 xen-buildfail REGR. vs. 125923 Tests which did not succeed, but are not blocking: test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass version targeted for testing: xen 4cdb6bfde2300c75725b3e267469bd6c955e baseline version: xen 3dd454c6c694409aaedd4ed075d6aeace2dd8391 Last test of basis 125923 2018-08-15 16:00:41 Z0 days Testing same since 125928 2018-08-15 19:00:49 Z0 days7 attempts People who touched revisions under test: Andrew Cooper Christian Lindig Daniel De Graaf Julien Grall Wei Liu jobs: build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked 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 Not pushing. commit 4cdb6bfde2300c75725b3e267469bd6c955e Author: Andrew Cooper Date: Fri Mar 16 18:27:24 2018 + xen/evtchn: Pass max_evtchn_port into evtchn_init() ... rather than setting it up once domain_create() has completed. This involves constructing a default value for dom0. No practical change in functionality. Signed-off-by: Andrew Cooper Reviewed-by: Roger Pau Monné Reviewed-by: Jan Beulich Acked-by: Julien Grall commit 4a83497635056d33fe20ef705f35617b1003a276 Author: Andrew Cooper Date: Tue Feb 27 17:39:37 2018 + xen/domctl: Merge set_max_evtchn into createdomain set_max_evtchn is somewhat weird. It was introduced with the event_fifo work, but has never been used. Still, it is a bounding on resources consumed by the event channel infrastructure, and should be part of createdomain, rather than editable after the fact. Drop XEN_DOMCTL_set_max_evtchn completely (including XSM hooks and libxc wrappers), and retain the functionality in XEN_DOMCTL_createdomain. Signed-off-by: Andrew Cooper Acked-by: Daniel De Graaf Acked-by: Christian Lindig Acked-by: Wei Liu Reviewed-by: Roger Pau Monné commit 54ed251dc7b85565820019102e533afcea814e16 Author: Andrew Cooper Date: Fri Mar 9 14:38:35 2018 + tools: Rework xc_domain_create() to take a full xen_domctl_createdomain In future patches, the structure will be extended with further information, and this is far cleaner than adding extra parameters. The python stubs are the only user which passes NULL for the existing config option (which is actually the arch substructure). Therefore, the #ifdefary moves to compensate. For libxl, pass the full config object down into libxl__arch_domain_{prepare,save}_config(), as there are in practice arch specific settings in the common part of the structure (flags s3_integrity and oos_off specifically). No practical change in behaviour. Signed-off-by: Andrew Cooper Acked-by: Christian Lindig Reviewed-by: Roger Pau Monné Acked-by: Wei Liu commit acc2a06c780e9e7ffa6373854735503b7d63a1d0 Author: Andrew Cooper Date: Mon Mar 12 10:40:33 2018 + tools/ocaml: Pass a full domctl_create_config into stub_xc_domain_create() The underlying C function is about to make the same change, and the structure is going to gain extra fields. Signed-off-by: Andrew Cooper Acked-by: Christian Lindig (qemu changes not included) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 15/15] xen/arm: traps: Move the implementation of GUEST_BUG_ON in traps.h
Hi Stefano, On 08/14/2018 10:43 PM, Stefano Stabellini wrote: On Mon, 16 Jul 2018, Julien Grall wrote: GUEST_BUG_ON may be used in other files doing guest emulation. Signed-off-by: Julien Grall Given that GUEST_BUG_ON is not actually used in any other files in this patch series, I assume you are referring to one of your future series? It is going to be used later. However, I don't really refer to any series in particular. It is just that this macros could be helpful in any emulation code to catch what we think is a invalid architectural behavior. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 11/15] xen/arm: Allow lpae_is_{table, mapping} helpers to work on invalid entry
Hi, On 08/15/2018 08:13 PM, Stefano Stabellini wrote: On Tue, 14 Aug 2018, Julien Grall wrote: Hi Stefano, On 08/14/2018 10:33 PM, Stefano Stabellini wrote: On Mon, 16 Jul 2018, Julien Grall wrote: Currently, lpae_is_{table, mapping} helpers will always return false on entry with the valid bit unset. However, it would be useful to have them ^entries operating on any entry. For instance to store information in advance but still request a fault. With that change, the p2m is now providing an overlay for *_is_{table, mapping} that will check the valid bit of the entry. Signed-off-by: Julien Grall --- xen/arch/arm/guest_walk.c | 2 +- xen/arch/arm/mm.c | 2 +- xen/arch/arm/p2m.c | 22 ++ xen/include/asm-arm/lpae.h | 11 +++ 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/xen/arch/arm/guest_walk.c b/xen/arch/arm/guest_walk.c index e3e21bdad3..4a1b4cf2c8 100644 --- a/xen/arch/arm/guest_walk.c +++ b/xen/arch/arm/guest_walk.c @@ -566,7 +566,7 @@ static int guest_walk_ld(const struct vcpu *v, * PTE is invalid or holds a reserved entry (PTE<1:0> == x0)) or if the PTE * maps a memory block at level 3 (PTE<1:0> == 01). */ -if ( !lpae_is_mapping(pte, level) ) +if ( !lpae_is_valid(pte) || !lpae_is_mapping(pte, level) ) return -EFAULT; /* Make sure that the lower bits of the PTE's base address are zero. */ diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index e3dafe5fd7..52e57fef2f 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -996,7 +996,7 @@ static int create_xen_entries(enum xenmap_operation op, for(; addr < addr_end; addr += PAGE_SIZE, mfn = mfn_add(mfn, 1)) { entry = _second[second_linear_offset(addr)]; -if ( !lpae_is_table(*entry, 2) ) +if ( !lpae_is_valid(*entry) || !lpae_is_table(*entry, 2) ) { rc = create_xen_table(entry); if ( rc < 0 ) { diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index ec3fdcb554..07925a1be4 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -219,6 +219,20 @@ static p2m_access_t p2m_mem_access_radix_get(struct p2m_domain *p2m, gfn_t gfn) return radix_tree_ptr_to_int(ptr); } +/* + * lpae_is_* helpers don't check whether the valid bit is set in the + * PTE. Provide our own overlay to check the valid bit. + */ +static inline bool p2m_is_mapping(lpae_t pte, unsigned int level) +{ +return lpae_is_valid(pte) && lpae_is_mapping(pte, level); +} + +static inline bool p2m_is_superpage(lpae_t pte, unsigned int level) +{ +return lpae_is_valid(pte) && lpae_is_superpage(pte, level); +} I like the idea, but it would be clearer to me if they were called lpae_is_valid_mapping and lpae_is_valid_superpage respectively. What do you think? > Also, we might as well move them to lpae.h and use them from mm.c and guest_walk.c as appropriate? lpae.h is not suitable because as I said in the commit message each page-table (stage-1, stage-2) may have a different view of what "valid" means. At the moment, that view is the same but it is not going to stay long like that. Hence the reason of prefixing with p2m_. They are specific to the p2m. This is giving us some more liberty in the future while making the lpae code a bit more generic. In guest_walk.c there are only one user, so the introduction of an helper is quite limited there. I see, so by "p2m_is_mapping" you meant specifically "stage2_is_mapping", right? That makes sense now. Yes. We use the term "p2m" everywhere in Xen to refer to stage-2 page-tables (see lpae_p2m_t). So it makes sense to prefix the helpers with "p2m_" here. Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.10-testing test] 125908: regressions - FAIL
flight 125908 xen-4.10-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/125908/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-cubietruck 6 xen-installfail REGR. vs. 125698 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 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-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail 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 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: xen 13e85a6dbc1eeda4f95c0d3afcd205579eab5909 baseline version: xen 87c83af333e0248ada2e6560965aca6096ec7f2b Last test of basis 125698 2018-07-31 12:47:03 Z 15 days Testing same since 125908 2018-08-14 17:06:33 Z1 days1 attempts People who touched revisions under test: Andrew Cooper Christian Lindig George Dunlap Jan Beulich Juergen Gross Kevin Tian Stefano Stabellini 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 pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-prev pass build-i386-prev pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass