[Xen-devel] [xen-unstable-smoke test] 126019: regressions - FAIL

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Boris Ostrovsky
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

2018-08-16 Thread Sarah Newman
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

2018-08-16 Thread Juergen Gross
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

2018-08-16 Thread Juergen Gross
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread osstest service owner
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-08-16 Thread Zhenzhong Duan

在 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-08-16 Thread Zhenzhong Duan

在 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

2018-08-16 Thread Andy Smith
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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Brian Woods
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

2018-08-16 Thread Christopher Clark
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

2018-08-16 Thread Brian Woods
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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Tim Deegan
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

2018-08-16 Thread Greg Kroah-Hartman
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

2018-08-16 Thread Greg Kroah-Hartman
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

2018-08-16 Thread Christopher Clark
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread Stefano Stabellini
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

2018-08-16 Thread Wei Liu
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

2018-08-16 Thread Tamas K Lengyel
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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Roger Pau Monné
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()

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Jan Beulich
>>> 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()

2018-08-16 Thread Andrew Cooper
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()

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Andy Lutomirski
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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Greg KH
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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Wei Liu
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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Pu Wen

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

2018-08-16 Thread Pu Wen

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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Wei Liu
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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Juergen Gross
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

2018-08-16 Thread Wei Liu
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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread gregkh

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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread osstest service owner
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()

2018-08-16 Thread 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.

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()

2018-08-16 Thread 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.

>>> --- 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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Jan Beulich
>>> 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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread osstest service owner
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()

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Andrew Cooper
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()

2018-08-16 Thread Zhenzhong Duan


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

2018-08-16 Thread Julien Grall

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()

2018-08-16 Thread Zhenzhong Duan

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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Julien Grall

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

2018-08-16 Thread Roger Pau Monné
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

2018-08-16 Thread Andrew Cooper
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

2018-08-16 Thread Platform Team regression test user
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

2018-08-16 Thread Julien Grall

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()

2018-08-16 Thread Julien Grall

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

2018-08-16 Thread osstest service owner
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

2018-08-16 Thread Julien Grall

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

2018-08-16 Thread Julien Grall

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

2018-08-16 Thread osstest service owner
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
 

  1   2   >