Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 03.02.16 at 08:10,  wrote:
> On 2/2/2016 11:21 PM, Jan Beulich wrote:
> On 02.02.16 at 16:00,  wrote:
>>> The limit of 4G is to avoid the data missing from uint64 to uint32
>>> assignment. And I can accept the 8K limit for XenGT in practice.
>>> After all, it is vGPU page tables we are trying to trap and emulate,
>>> not normal page frames.
>>>
>>> And I guess the reason that one domain exhausting Xen's memory can
>>> affect another domain is because rangeset uses Xen heap, instead of the
>>> per-domain memory. So what about we use a 8K limit by now for XenGT,
>>> and in the future, if a per-domain memory allocation solution for
>>> rangeset is ready, we do need to limit the rangeset size. Does this
>>> sounds more acceptable?
>>
>> The lower the limit the better (but no matter how low the limit
>> it won't make this a pretty thing). Anyway I'd still like to wait
>> for what Ian may further say on this.
>>
> Hi Jan, I just had a discussion with my colleague. We believe 8K could
> be the biggest limit for the write-protected ram ranges. If in the
> future, number of vGPU page tables exceeds this limit, we will modify
> our back-end device model to find a trade-off method, instead of
> extending this limit. If you can accept this value as the upper bound
> of rangeset, maybe we do not need to add any tool stack parameters, but
> define a MAX_NR_WR_RAM_RANGES for the write-protected ram rangesset. As
> to other rangesets, we keep their limit as 256. Does this sounds OK? :)

I'm getting the impression that we're moving in circles. A blanket
limit above the 256 one for all domains is _not_ going to be
acceptable; going to 8k will still need host admin consent. With
your rangeset performance improvement patch, each range is
going to be tracked by a 40 byte structure (up from 32), which
already means an overhead increase for all the other ranges. 8k
of wp ranges implies an overhead beyond 448k (including the
xmalloc() overhead), which is not _that_ much, but also not
negligible.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH OSSTEST 5/5 v2] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Campbell]

By looping over @rows looking for buildjobs runvars and adding those
jobs to the output until nothing changes.

The output is resorted by runvar name which is the desired default
behaviour. As usual can be piped to sort(1) to sort by flight+job.

Signed-off-by: Ian Campbell 
---
v2:
 - Use $jobcond,@jobconfparams to avoid SQL injection
 - Only recurse if the option was given
 - Drop synth from ORDER BY
 - Use a Schwatzian transform for the sort, at the same time allowing
   retention of the sorting of synth runvars last.
---
 mg-show-flight-runvars | 44 +---
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index f96539f..faa0ea1 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -46,30 +46,56 @@ for (;;) {
 
 die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 
-
 our @cols = qw(job name val);
 our @rows;
+our %jobs;
+
+sub collect ($;$@) {
+my ($flight,$jobcond,@jobcondparams) = @_;
 
-sub collect ($) {
-my ($flight) = @_;
+$jobcond //= "TRUE";
 
 $flight =~ m/^\d+/ or $flight = "'$flight'";
-my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
+my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond AND 
$jobcond";
 
 my $q = $dbh_tests->prepare
-   ("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, 
job");
-$q->execute();
+   ("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY name, job");
+$q->execute(@jobcondparams);
 
 while (my (@row) = $q->fetchrow_array()) {
my $synth = shift @row;
$row[0] = "$flight.$row[0]" if $recurse;
$row[1] .= $synthsufx if $synth && $synth ne 'f'; # sqlite3 is typeless
push @rows, \@row;
+   $jobs{$row[0]} = 1;
 }
 }
 
 collect($ARGV[0]);
 
+if ($recurse) {
+foreach my $row (@rows) {
+   next unless $row->[1] =~ m/^(?:.*_)?([^_]*)buildjob$/;
+   next if $jobs{$row->[2]};
+
+   # parse this flight and job, which must be in $flight.$job
+   # form if $recurse is true (see collect())
+   my ($tflight, $tjob) = flight_otherjob(undef, $row->[0]);
+   die "$row->[1]" unless $tflight;
+
+   # parse the buildjob reference and recurse. might be a job in
+   # this flight, in which case we still recurse since it might
+   # be a chain from a non-top-level job which hasn't been
+   # included yet. %jobs will prevent us from duplicating or
+   # infinite loops.
+   my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
+   collect($oflight, "job = ?", $ojob);
+
+   # collect() appends to @rows, so we don't need any special
+   # handling to pickup anything which was newly added.
+}
+}
+
 our @colws;
 sub max ($$) { $_[$_[0] < $_[1]] }
 foreach my $row (@rows) {
@@ -77,7 +103,11 @@ foreach my $row (@rows) {
 }
 $colws[1] += length $synthsufx;
 
-foreach my $row (@rows) {
+# Sort by runvar name, then (flight+)job, synth runvars come last.
+foreach my $row (map { $_->[0] }
+sort { $a->[1] cmp $b->[1] }
+map { [ $_, ($_->[1] =~ m/~$/)." $_->[1] $_->[0]" ] }
+@rows) {
 printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
 or die $!;
 }
-- 
2.6.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V8 5/5] x86/hvm: pkeys, add pkeys support for cpuid handling

[Jan Beulich]

>>> On 02.02.16 at 08:35,  wrote:
> This patch adds pkeys support for cpuid handing.
> 
> Pkeys hardware support is CPUID.7.0.ECX[3]:PKU. software support is
> CPUID.7.0.ECX[4]:OSPKE and it reflects the support setting of CR4.PKE.
> 
> X86_FEATURE_OSXSAVE depends on guest X86_FEATURE_XSAVE, but cpu_has_xsave
> function reflects hypervisor X86_FEATURE_XSAVE, it is fixed too.
> 
> Signed-off-by: Huaitong Han 
> ---
> Changes in v7:
> *Rebase in the latest tree.
> *Add a comment for cpu_has_xsave adjustment.
> *Adjust indentation.
> 
>  tools/libxc/xc_cpufeature.h |  3 +++
>  tools/libxc/xc_cpuid_x86.c  |  6 --
>  xen/arch/x86/hvm/hvm.c  | 18 +-
>  3 files changed, 20 insertions(+), 7 deletions(-)

This will need a tools maintainer's ack, so upon re-submission you
should Cc them.

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4572,7 +4572,7 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>  __clear_bit(X86_FEATURE_APIC & 31, edx);
>  
>  /* Fix up OSXSAVE. */
> -if ( cpu_has_xsave )
> +if ( *ecx & cpufeat_mask(X86_FEATURE_XSAVE) )
>  *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ?
>   cpufeat_mask(X86_FEATURE_OSXSAVE) : 0;

First of all I would have wished that since you're already touching it,
you would have brought it into the same shape as you're doing for
PKU below. And then here as well as ...

> @@ -4593,16 +4593,24 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>  if ( !cpu_has_smap )
>  *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>  
> -/* Don't expose MPX to hvm when VMX support is not available */
> +/* Don't expose MPX to hvm when VMX support is not available. */
>  if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>   !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
>  *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>  
> -/* Don't expose INVPCID to non-hap hvm. */
>  if ( !hap_enabled(d) )
> -*ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
> +{
> + /* Don't expose INVPCID to non-hap hvm. */
> + *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
> + /* X86_FEATURE_PKU is not yet implemented for shadow 
> paging. */
> + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU);
> +}
> +
> +if ( (*ecx & cpufeat_mask(X86_FEATURE_PKU)) &&
> + (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) )
> +*ecx |= cpufeat_mask(X86_FEATURE_OSPKE);

... here - shouldn't we also clear the respective flag in the "else"
case?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V8 5/5] x86/hvm: pkeys, add pkeys support for cpuid handling

[Han, Huaitong]

On Wed, 2016-02-03 at 02:50 -0700, Jan Beulich wrote:
> > > > On 02.02.16 at 08:35,  wrote:
> > This patch adds pkeys support for cpuid handing.
> > 
> > Pkeys hardware support is CPUID.7.0.ECX[3]:PKU. software support is
> > CPUID.7.0.ECX[4]:OSPKE and it reflects the support setting of
> > CR4.PKE.
> > 
> > X86_FEATURE_OSXSAVE depends on guest X86_FEATURE_XSAVE, but
> > cpu_has_xsave
> > function reflects hypervisor X86_FEATURE_XSAVE, it is fixed too.
> > 
> > Signed-off-by: Huaitong Han 
> > ---
> > Changes in v7:
> > *Rebase in the latest tree.
> > *Add a comment for cpu_has_xsave adjustment.
> > *Adjust indentation.
> > 
> >  tools/libxc/xc_cpufeature.h |  3 +++
> >  tools/libxc/xc_cpuid_x86.c  |  6 --
> >  xen/arch/x86/hvm/hvm.c  | 18 +-
> >  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> This will need a tools maintainer's ack, so upon re-submission you
> should Cc them.
I will (again) defer this to x86 maintainers. -from wei.l...@citrix.com
> 
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -4572,7 +4572,7 @@ void hvm_cpuid(unsigned int input, unsigned
> > int *eax, unsigned int *ebx,
> >  __clear_bit(X86_FEATURE_APIC & 31, edx);
> >  
> >  /* Fix up OSXSAVE. */
> > -if ( cpu_has_xsave )
> > +if ( *ecx & cpufeat_mask(X86_FEATURE_XSAVE) )
> >  *ecx |= (v->arch.hvm_vcpu.guest_cr[4] &
> > X86_CR4_OSXSAVE) ?
> >   cpufeat_mask(X86_FEATURE_OSXSAVE) : 0;
> 
> First of all I would have wished that since you're already touching
> it,
> you would have brought it into the same shape as you're doing for
> PKU below. And then here as well as ...
It will be updated in V9.
> 
> > @@ -4593,16 +4593,24 @@ void hvm_cpuid(unsigned int input, unsigned
> > int *eax, unsigned int *ebx,
> >  if ( !cpu_has_smap )
> >  *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
> >  
> > -/* Don't expose MPX to hvm when VMX support is not
> > available */
> > +/* Don't expose MPX to hvm when VMX support is not
> > available. */
> >  if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
> >   !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
> >  *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
> >  
> > -/* Don't expose INVPCID to non-hap hvm. */
> >  if ( !hap_enabled(d) )
> > -*ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
> > +{
> > + /* Don't expose INVPCID to non-hap hvm. */
> > + *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
> > + /* X86_FEATURE_PKU is not yet implemented for
> > shadow paging. */
> > + *ecx &= ~cpufeat_mask(X86_FEATURE_PKU);
> > +}
> > +
> > +if ( (*ecx & cpufeat_mask(X86_FEATURE_PKU)) &&
> > + (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PKE) )
> > +*ecx |= cpufeat_mask(X86_FEATURE_OSPKE);
> 
> ... here - shouldn't we also clear the respective flag in the "else"
> case?
X86_FEATURE_OSPKE is not set by xc_cpuid_hvm_policy(tools), it reflects
the setting of CR4.PKE, so it does not need to be cleared like
X86_CR4_OSXSAVE.
> 
> Jan
> 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V8 2/5] x86/hvm: pkeys, add pkeys support for guest_walk_tables

[Han, Huaitong]

On Wed, 2016-02-03 at 02:43 -0700, Jan Beulich wrote:
> > > > On 02.02.16 at 08:35,  wrote:
> > Protection keys define a new 4-bit protection key field(PKEY) in
> > bits 62:59 
> > of
> > leaf entries of the page tables.
> > 
> > PKRU register defines 32 bits, there are 16 domains and 2 attribute
> > bits per
> > domain in pkru, for each i (0 ≤ i ≤ 15), PKRU[2i] is the access
> > -disable bit 
> > for
> > protection key i (ADi); PKRU[2i+1] is the write-disable bit for
> > protection 
> > key
> > i (WDi). PKEY is index to a defined domain.
> > 
> > A fault is considered as a PKU violation if all of the following
> > conditions 
> > are
> > true:
> > 1.CR4_PKE=1.
> > 2.EFER_LMA=1.
> > 3.Page is present with no reserved bit violations.
> > 4.The access is not an instruction fetch.
> > 5.The access is to a user page.
> > 6.PKRU.AD=1
> > or The access is a data write and PKRU.WD=1
> > and either CR0.WP=1 or it is a user access.
> > 
> > Signed-off-by: Huaitong Han 
> 
> Reviewed-by: Jan Beulich 
> albeit ...
> 
> > Changes in v8:
> > *Abstract out _write_cr4.
> 
> ... I'm not happy about the chose name and will try to remember
> to change it to e.g. raw_write_cr4(). Names starting with an
> underscore and a lower case letter are reserved to symbols
> local to a given translation unit, which in my reading doesn't fit
> with them getting placed in header files.
It will be updated in V9.
> 
> Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Ian Campbell]

On Tue, 2016-02-02 at 12:37 +, Wei Liu wrote:
> On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> > From: Roger Pau Monne 
> > 
> > Due to the HVMlite changes there's a chance that the value in rc is
> > checked
> > without being initialised. Fix this by initialising it to 0.
> > 
> > Signed-off-by: Roger Pau Monné 
> > Reported-by: Olaf Hering 
> 
> Acked-by: Wei Liu 

This is CID 1351229, I think?

** CID 1351229:  Uninitialized variables  (UNINIT)
> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> 
> 
> 
> *** CID 1351229:  Uninitialized variables  (UNINIT)
> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> 1437 cur_pages = 0xc0;
> 1438 stat_normal_pages += 0xc0;
> 1439 }
> 1440 else
> 1441 cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
> 1442 
> >>> CID 1351229:  Uninitialized variables  (UNINIT)
> >>> Using uninitialized value "rc".
> 1443 while ( (rc == 0) && (end_pages > cur_pages) )
> 1444 {
> 1445 /* Clip count to maximum 1GB extent. */
> 1446 unsigned long count = end_pages - cur_pages;
> 1447 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
> 1448    

Note that this while loop ends with:
if ( rc != 0 )
break;
and there are no continue statements.

Therefore I wonder if we would be better off removing the rc == 0 part of
the loop condition?

The issue with this patch is the usual one that it will hide other
unintentional uses of rc before it is set to a good value.

This issue was exposed by a prior "rc = xc_domain_populate_physmap_exact"
becoming conditional on device_model. What is also concerning is the lack
of error checking on that call -- is it really ok to just barrel on under
these circumstance?

Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Wei Liu]

On Wed, Feb 03, 2016 at 10:30:54AM +, Ian Campbell wrote:
> On Tue, 2016-02-02 at 12:37 +, Wei Liu wrote:
> > On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> > > From: Roger Pau Monne 
> > > 
> > > Due to the HVMlite changes there's a chance that the value in rc is
> > > checked
> > > without being initialised. Fix this by initialising it to 0.
> > > 
> > > Signed-off-by: Roger Pau Monné 
> > > Reported-by: Olaf Hering 
> > 
> > Acked-by: Wei Liu 
> 
> This is CID 1351229, I think?
> 

Yes, I think so.

> ** CID 1351229:  Uninitialized variables  (UNINIT)
> > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > 
> > 
> > 
> > *** CID 1351229:  Uninitialized variables  (UNINIT)
> > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > 1437 cur_pages = 0xc0;
> > 1438 stat_normal_pages += 0xc0;
> > 1439 }
> > 1440 else
> > 1441 cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
> > 1442 
> > >>> CID 1351229:  Uninitialized variables  (UNINIT)
> > >>> Using uninitialized value "rc".
> > 1443 while ( (rc == 0) && (end_pages > cur_pages) )
> > 1444 {
> > 1445 /* Clip count to maximum 1GB extent. */
> > 1446 unsigned long count = end_pages - cur_pages;
> > 1447 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
> > 1448    
> 
> Note that this while loop ends with:
> if ( rc != 0 )
> break;
> and there are no continue statements.
> 
> Therefore I wonder if we would be better off removing the rc == 0 part of
> the loop condition?
> 

No, there is no if ( rc != 0 )  inside the said while loop.  That "if"
is for the outer for loop.

We could add a test for the while loop, if that looks clearer to you.

> The issue with this patch is the usual one that it will hide other
> unintentional uses of rc before it is set to a good value.
> 
> This issue was exposed by a prior "rc = xc_domain_populate_physmap_exact"
> becoming conditional on device_model. What is also concerning is the lack
> of error checking on that call -- is it really ok to just barrel on under
> these circumstance?
> 

Yeah, that should ideally be fixed, too.

Wei.

> Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] missing lock in percpu_rwlock? (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

On Wed, 2016-02-03 at 10:45 +, Ian Campbell wrote:
> On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> > * CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
> > /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> 
> Coverity seems to think this is new in 41b0aa569adb..9937763265d,
> presumably due to 
> 
> commit f9dd43dddc0a31a4343a58072935c1b5c0cbbee
> Author: Malcolm Crossley 
> Date:   Fri Jan 22 16:04:41 2016 +0100
> 
> rwlock: add per-cpu reader-writer lock infrastructure

It also reports this one, but I suppose this is a false +ve given the name
of the function.

(Also note "simulatenously" should be "simultaneously")

** CID 1351220:  Program hangs  (LOCK)
/xen/include/xen/spinlock.h: 310 in _percpu_read_lock()




*** CID 1351220:  Program hangs  (LOCK)
/xen/include/xen/spinlock.h: 310 in _percpu_read_lock()
304  * Detect using a second percpu_rwlock_t simulatenously and 
fallback
305  * to standard read_lock.
306  */
307 if ( unlikely(this_cpu_ptr(per_cpudata) != NULL ) )
308 {
309 read_lock(_rwlock->rwlock);
>>> CID 1351220:  Program hangs  (LOCK)
>>> Returning without unlocking "percpu_rwlock->rwlock".
310 return;
311 }
312 
313 /* Indicate this cpu is reading. */
314 this_cpu_ptr(per_cpudata) = percpu_rwlock;
315 smp_mb();


> 
> > ___
> > __
> > ___
> > *** CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
> > /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> > 356 percpu_rwlock_t *percpu_rwlock)
> > 357 {
> > 358 /* Validate the correct per_cpudata variable has been
> > provided. */
> > 359 _percpu_rwlock_owner_check(per_cpudata, percpu_rwlock);
> > 360 
> > 361 ASSERT(percpu_rwlock->writer_activating);
> > > > >  CID 1351223:  Concurrent data access violations 
> > > > > (MISSING_LOCK)
> > > > >  Accessing "percpu_rwlock->writer_activating" without holding
> > > > > lock
> > "percpu_rwlock.rwlock". Elsewhere, "percpu_rwlock.writer_activating" is
> > accessed with "percpu_rwlock.rwlock" held 1 out of 2 times (1 of these
> > accesses strongly imply that it is necessary).
> > 362 percpu_rwlock->writer_activating = 0;
> > 363 write_unlock(_rwlock->rwlock);
> > 364 }
> > 365 
> > 366 #define percpu_rw_is_write_locked(l)
> > _rw_is_write_locked(&((l)->rwlock))

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHv3 2/2] spinlock: fair read-write locks

[Jan Beulich]

>>> On 01.02.16 at 12:31,  wrote:
> +void queue_write_lock_slowpath(rwlock_t *lock)
> +{
> +u32 cnts;
> +
> +/* Put the writer into the wait queue. */
> +spin_lock(>lock);
> +
> +/* Try to acquire the lock directly if no reader is present. */
> +if ( !atomic_read(>cnts) &&
> + (atomic_cmpxchg(>cnts, 0, _QW_LOCKED) == 0) )
> +goto unlock;
> +
> +/*
> + * Set the waiting flag to notify readers that a writer is pending,
> + * or wait for a previous writer to go away.
> + */
> +for (;;)

Since everything else here has been nicely converted to Xen style,
strictly speaking these should be

for ( ; ; )

but of course this is no reason to block the patch. Since however,
as said in reply to patch 1, ...

> --- a/xen/include/xen/rwlock.h
> +++ b/xen/include/xen/rwlock.h
> @@ -3,6 +3,188 @@
>  
>  #include 

... this should go away if possible, it would be nice for the cosmetic
thing above to also be fixed up at once.

With at least the latter taken care of, and assuming this won't incur
other changes (apart from adding necessary includes here)
Acked-by: Jan Beulich 

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Error booting Xen

[Dario Faggioli]

Hey Harmandeep,

On Fri, 2016-01-29 at 03:13 -0700, Jan Beulich wrote:
> > > > On 28.01.16 at 20:17,  wrote:
> > Latest set looks good. No boot issues. No resets.
> > Full log at http://paste2.org/NxHNW4vn 
> 
> > Sorry I don't know much about source of last few
> > lines. I was just tracing in xen when these came.
> > I was unable to create them again. I will inform
> > you if I get these again.
> 
> The WRMSR ones are of no concern. What I'm curious about are the
> five instances of
> 
> (XEN) traps.c:3290: GPF (): 82d08019cb87 -> 82d080242e15
> 
> Could you check what these addresses correspond to in source?
> 
I guess you just happened to forgot to follow up on this question from
Jan?

> So far you've said you don't start any, but
> the logs clearly show otherwise. Are there perhaps any that
> get auto-started? In which case I'd be interested in you
> confirming that these come up fine.
> 
And this too?

Can you please do that? :-)

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [distros-debian-squeeze test] 38724: all pass

[Platform Team regression test user]

flight 38724 distros-debian-squeeze real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/38724/

Perfect :-)
All tests in this flight passed
baseline version:
 flight   38707

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-amd64-squeeze-netboot-pygrubpass
 test-amd64-i386-amd64-squeeze-netboot-pygrub pass
 test-amd64-amd64-i386-squeeze-netboot-pygrub pass
 test-amd64-i386-i386-squeeze-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.xs.citrite.net/~osstest/testlogs/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.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC] support more qdisk types

[Ian Campbell]

On Wed, 2016-02-03 at 10:35 +, Wei Liu wrote:
> > Ok. So in your opinion, even if any new disk config is encoded in 'target=',
> > libxlu should split that up into (new) members of libxl_device_disk, not 
> > just
> > plop it into libxl_device_disk.pdev_path?
> > 
> 
> No, not necessarily. I didn't look closely in the code yesterday when
> replying, sorry.
> 
> If  target= has always been shoveled into pdev_path, using that would be
> fine. We already have mechanism to parse target= outside of libxl in
> hotplug script.
> 
> Are you aware of all those hotplug scripts living under tools/hotplug ?
> Does using hotplug script sound plausible to you?
> 
> Currently hotplug script for QEMU is broken and needs fixing though, but
> I'm sure we can figure it out.

How do hotplug scripts factor into this?

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] x86/hvm: Provide list of emulated features in HVM CPUID leaf

[Roger Pau Monné]

El 3/2/16 a les 1:17, Andrew Cooper ha escrit:
> On 02/02/2016 23:30, Boris Ostrovsky wrote:
>> On 02/02/2016 06:22 PM, Andrew Cooper wrote:
>>> On 02/02/2016 23:17, Boris Ostrovsky wrote:
 Hypervisor may choose which features to emulate for HVMlite guests.
 Guest will query the HVM CPUID leaf to find out what is available.

 Signed-off-by: Boris Ostrovsky 
>>> Roger also submitted a patch to do this.  However, it is not
>>> appropriate, so was dropped.
>>>
>>> An HVMLite domain should assume there are no emulated devices.  The very
>>> old legacy devices will never be implemented, and any others we care
>>> about possibly implementing in the future have APCI-based ways of
>>> indicating support.
>>
>> OK, so I wasn't the first one to come up with this ;-)

Hehe, no I think I've posted the exact same patch a week ago:

http://marc.info/?l=xen-devel=145339515912038

>> I think for now I mostly care about APIC and for that I can use HW
>> CPUID bit (which I believe is cleared for HVMlite guests).
> 
> The APIC bit in cpuid is magic and specified as a fast forward of the
> APICBASE_MSR enable bit.
> 
> Therefore, the correct architectural behaviour is for this bit to be
> clear if the local APIC is disabled, or indeed not implemented.
> 
> With my maintainers hat on, I will reject any attempt to introduce
> non-architectural behaviour; at the moment I am dealing with the
> stupidity that is the PV XSAVE interface, where broken bugfix piled on
> top of broken bugfix has resulted in a situation where many Linux PV
> guests crash if provided with architecturally correct behaviour of the
> OSXSAVE cpuid bit (yet another magic one).
> 
>> The trouble is that I need to present Linux as having APIC (boot code
>> doesn't feel good if !cpu_has_apic) so I'll need to keep no-APIC
>> emulation private to Xen-related code. Which is doable.

I have to do the same for FreeBSD, I have to manually switch the APIC
cpuid bit, or else FreeBSD refuses to do SMP initialization. IMHO, what
we currently do (no APIC cpuid bit) is correct, and when a local APIC is
available the bit will indeed be enabled.

> I see two choices.
> 
> 1) Require that Linux DMLite guests require a Local APIC, and we allow
> that to be a configured option.  Exposing APIC definitely makes sense
> longer term, because APICV hardware acceleration outperforms the
> hypercall-based method.

This is what I aim to do long term, that is provide an emulated local
APIC. The plan was to then also provide ACPI tables in order to notify
the presence of the local and IO APICs (we are going to need both if we
plan to do pci-passthrough of devices with PCI interrupts). Of course
the APIC cpuid bit will also be enabled in this case.

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] Leak in xc_dom_load_hvm_kernel() (Was; Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

Roger,

On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> ** CID 1351227:    (RESOURCE_LEAK)
> /tools/libxc/xc_dom_hvmloader.c: 260 in xc_dom_load_hvm_kernel()
> /tools/libxc/xc_dom_hvmloader.c: 270 in xc_dom_load_hvm_kernel()
> /tools/libxc/xc_dom_hvmloader.c: 277 in xc_dom_load_hvm_kernel()

Looks like this came from ad787bafcd2a3058f0f37f2fe84931bd5136bde9?

> 
> *** CID 1351227:    (RESOURCE_LEAK)
> /tools/libxc/xc_dom_hvmloader.c: 260 in xc_dom_load_hvm_kernel()
> 254 elf->dest_size = pages * XC_DOM_PAGE_SIZE(dom);
> 255 
> 256 rc = elf_load_binary(elf);
> 257 if ( rc < 0 )
> 258 {
> 259 DOMPRINTF("%s: failed to load elf binary", __func__);
> >>> CID 1351227:    (RESOURCE_LEAK)
> >>> Variable "entries" going out of scope leaks the storage it points to.
> 260 return rc;
> 261 }
> 262 
> 263 munmap(elf->dest_base, elf->dest_size);
> 264 
> 265 rc = modules_init(dom, dom->total_pages << PAGE_SHIFT, elf, 
> _start,
> /tools/libxc/xc_dom_hvmloader.c: 270 in xc_dom_load_hvm_kernel()
> 264 
> 265 rc = modules_init(dom, dom->total_pages << PAGE_SHIFT, elf, 
> _start,
> 266   _end);
> 267 if ( rc != 0 )
> 268 {
> 269 DOMPRINTF("%s: insufficient space to load modules.", 
> __func__);
> >>> CID 1351227:    (RESOURCE_LEAK)
> >>> Variable "entries" going out of scope leaks the storage it points to.
> 270 return rc;
> 271 }
> 272 
> 273 rc = loadmodules(dom, m_start, m_end, dom->guest_domid);
> 274 if ( rc != 0 )
> 275 {
> /tools/libxc/xc_dom_hvmloader.c: 277 in xc_dom_load_hvm_kernel()
> 271 }
> 272 
> 273 rc = loadmodules(dom, m_start, m_end, dom->guest_domid);
> 274 if ( rc != 0 )
> 275 {
> 276 DOMPRINTF("%s: unable to load modules.", __func__);
> >>> CID 1351227:    (RESOURCE_LEAK)
> >>> Variable "entries" going out of scope leaks the storage it points to.
> 277 return rc;
> 278 }
> 279 
> 280 dom->parms.phys_entry = elf_uval(elf, elf->ehdr, e_entry);
> 281 
> 282 free(entries);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] missing lock in percpu_rwlock? (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

On Wed, 2016-02-03 at 10:50 +, Andrew Cooper wrote:
> On 03/02/16 10:45, Ian Campbell wrote:
> > On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> > > * CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
> > > /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> > Coverity seems to think this is new in 41b0aa569adb..9937763265d,
> > presumably due to 
> > 
> > commit f9dd43dddc0a31a4343a58072935c1b5c0cbbee
> > Author: Malcolm Crossley 
> > Date:   Fri Jan 22 16:04:41 2016 +0100
> > 
> > rwlock: add per-cpu reader-writer lock infrastructure
> 
> Expected behaviour.  writer_activating is expected to only be written
> under lock, but read without lock.

I suppose this is something we should eventually model?

Would you typically mark this as "False positive" or "Intentional"?

I just marked a couple of libxl ones about taking ctx->lock (which is
recursive) twice as "False positive", but perhaps "Intentional" is the
correct designation there?

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] libxc: fix leak in xc_dom_load_hvm_kernel error path

[Roger Pau Monne]

Error path in xc_dom_load_hvm_kernel needs to use the 'error' label instead
of directly returning. This is needed so the entries local variable is
freed.

Coverity-ID: 1351227
Signed-off-by: Roger Pau Monné 
---
Cc: Ian Jackson 
Cc: Ian Campbell 
Cc: Wei Liu 
---
 tools/libxc/xc_dom_hvmloader.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
index 79a3b99..330d5e8 100644
--- a/tools/libxc/xc_dom_hvmloader.c
+++ b/tools/libxc/xc_dom_hvmloader.c
@@ -257,7 +257,7 @@ static elf_errorstatus xc_dom_load_hvm_kernel(struct 
xc_dom_image *dom)
 if ( rc < 0 )
 {
 DOMPRINTF("%s: failed to load elf binary", __func__);
-return rc;
+goto error;
 }
 
 munmap(elf->dest_base, elf->dest_size);
@@ -267,14 +267,14 @@ static elf_errorstatus xc_dom_load_hvm_kernel(struct 
xc_dom_image *dom)
 if ( rc != 0 )
 {
 DOMPRINTF("%s: insufficient space to load modules.", __func__);
-return rc;
+goto error;
 }
 
 rc = loadmodules(dom, m_start, m_end, dom->guest_domid);
 if ( rc != 0 )
 {
 DOMPRINTF("%s: unable to load modules.", __func__);
-return rc;
+goto error;
 }
 
 dom->parms.phys_entry = elf_uval(elf, elf->ehdr, e_entry);
-- 
2.5.4 (Apple Git-61)


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC] support more qdisk types

[Wei Liu]

On Wed, Feb 03, 2016 at 11:05:04AM +, Ian Campbell wrote:
> On Wed, 2016-02-03 at 10:55 +, Wei Liu wrote:
> > On Wed, Feb 03, 2016 at 10:51:27AM +, Ian Campbell wrote:
> > > On Wed, 2016-02-03 at 10:35 +, Wei Liu wrote:
> > > > > Ok. So in your opinion, even if any new disk config is encoded in
> > > > > 'target=',
> > > > > libxlu should split that up into (new) members of
> > > > > libxl_device_disk, not just
> > > > > plop it into libxl_device_disk.pdev_path?
> > > > > 
> > > > 
> > > > No, not necessarily. I didn't look closely in the code yesterday when
> > > > replying, sorry.
> > > > 
> > > > If  target= has always been shoveled into pdev_path, using that would
> > > > be
> > > > fine. We already have mechanism to parse target= outside of libxl in
> > > > hotplug script.
> > > > 
> > > > Are you aware of all those hotplug scripts living under tools/hotplug
> > > > ?
> > > > Does using hotplug script sound plausible to you?
> > > > 
> > > > Currently hotplug script for QEMU is broken and needs fixing though,
> > > > but
> > > > I'm sure we can figure it out.
> > > 
> > > How do hotplug scripts factor into this?
> > > 
> > 
> > If supporting all such block devices  requires presenting a block device
> > to QEMU? If QEMU directly handles them then hotplug script is not in the
> > picture.
> 
> Perhaps I've misunderstood what this thread is about. I thought it was
> about exposing all the various backends which qdisk supports natively, like
> CEPH, sheepdog, iscsi, nbd etc.
> 

Good point. It is me who is confused. Hotplug is not in the picture
then.

Wei.

> Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [linux-4.1 test] 79950: regressions - FAIL

[osstest service owner]

flight 79950 linux-4.1 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/79950/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 66399
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 66399
 test-armhf-armhf-xl-xsm  15 guest-start/debian.repeat fail REGR. vs. 66399
 test-armhf-armhf-xl  15 guest-start/debian.repeat fail REGR. vs. 66399
 build-amd64   5 xen-buildfail in 79642 REGR. vs. 66399
 test-armhf-armhf-xl-credit2 15 guest-start/debian.repeat fail in 79642 REGR. 
vs. 66399
 test-armhf-armhf-xl-cubietruck 15 guest-start/debian.repeat fail in 79817 
REGR. vs. 66399
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat fail in 79817 REGR. 
vs. 66399

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-xsm  11 guest-startfail in 79642 pass in 79950
 test-armhf-armhf-xl  11 guest-startfail in 79817 pass in 79950
 test-armhf-armhf-xl-credit2  11 guest-start fail pass in 79642
 test-armhf-armhf-xl-cubietruck 11 guest-start   fail pass in 79817
 test-armhf-armhf-xl-multivcpu 11 guest-startfail pass in 79817
 test-armhf-armhf-xl-rtds 11 guest-start fail pass in 79817

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat fail in 79642 like 66399
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail in 79817 like 66399
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 66399
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 66399
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   like 66399

Tests which did not succeed, but are not blocking:
 build-amd64-libvirt   1 build-check(1)blocked in 79642 n/a
 build-amd64-rumpuserxen   1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-xl-pvh-amd   1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-xl-pvh-intel  1 build-check(1)   blocked in 79642 n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked in 79642 n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)blocked in 79642 n/a
 test-amd64-i386-xl1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)blocked in 79642 n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-pair 1 build-check(1)blocked in 79642 n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-pygrub   1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked in 79642 n/a
 test-amd64-amd64-qemuu-nested-amd  1 build-check(1)   blocked in 79642 n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked in 79642 n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked in 
79642 n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)  blocked in 79642 n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)blocked in 79642 n/a
 test-amd64-i386-libvirt   1 build-check(1)blocked in 79642 n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1)  blocked in 79642 n/a
 test-amd64-amd64-xl   1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-libvirt  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked in 79642 n/a
 test-amd64-i386-pair  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-xl-qemut-win7-amd64  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-qemuu-nested-intel  1 build-check(1) blocked in 79642 n/a
 test-amd64-i386-xl-raw1 build-check(1)blocked in 79642 n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1) blocked in 79642 n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1) blocked in 79642 n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1)blocked in 79642 n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64 1 build-check(1) blocked in 79642 n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1) blocked in 79642 n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)blocked in 79642 n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1) blocked in 79642 n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked in 79642 n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1)blocked in 79642 n/a
 

Re: [Xen-devel] Xen 4.7 Development Update

[Han, Huaitong]

On Mon, 2016-02-01 at 10:45 +, Wei Liu wrote:
> This email only tracks big items for xen.git tree. Please reply for
> items you
> woulk like to see in 4.7 so that people have an idea what is going on
> and
> prioritise accordingly.
> 
> You're welcome to provide description and use cases of the feature
> you're
> working on.
> 
> = Timeline =
> 
> We now adopt a fixed cut-off date scheme. We will release twice a
> year. The upcoming 4.7 timeline are as followed:
> 
> * Last posting date: March 18, 2016
> * Hard code freeze: April 1, 2016
> * RC1: TBD
> * Release: June 3, 2016
> 
> Note that we don't have freeze exception scheme anymore. All patches
> that wish to go into 4.7 must be posted no later than the last
> posting
> date. All patches posted after that date will be automatically queued
> into next release.
> 
> RCs will be arranged immediately after freeze.
> 
> = Projects =
> *  Memory protection-key support
>   -  Huaitong Han
V8 (7 patches) has been sent out, 2 patches have already been merged, 3
of the remaining 5 patches have been Acked/Reviewed by Andrew/Jan.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v7 9/9] vring: Use the DMA API on Xen

[David Vrabel]

On 03/02/16 05:46, Andy Lutomirski wrote:
> Signed-off-by: Andy Lutomirski 

You forgot the previous Reviewed-by tags.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC] support more qdisk types

[Ian Campbell]

On Tue, 2016-02-02 at 15:06 -0700, Jim Fehlig wrote:
> 
> > And extending
> > the structure seems to be the right thing to do.
> 
> So what do you think of the approach in the RFC patch? It adds discrete knobs 
> in
> the disk config and extends the disk structure similarly. Before I can make
> progress on this we need to agree on extending the config and 
> libxl_device_disk
> structure.

My main concern is that this approach requires us to update libxl for each
new possible backend type.

The intention of the target= in the disk spec is that it consumes the rest
of the line so it can be used to encode pretty much anything. Is it not
possible (modulo bugs) to pass all the necessary information to qdisk in
this form? I thought Dave S had made it possible to use qdisk in this way
back in:

commit a8a1f236a2964506a22d1779648d8e1c8668cb1a
Author: David Scott 
Date:   Tue Apr 23 10:59:26 2013 +0100

libxl: Only call stat() when adding a disk if we expect a device to 
exist.

We consider calling stat() a helpful error check in the following
circumstances only:
 1. the disk backend type must be PHYsical
 2. the disk backend domain must be the same as the running libxl
code (ie LIBXL_TOOLSTACK_DOMID)
 3. there must not be a hotplug script because this would imply that
the device won't be created until after the hotplug script has
run.

With this fix, it is possible to use qemu's built-in block drivers
such as ceph/rbd, with a xl config disk spec like this:

disk=[ 
'backendtype=qdisk,format=raw,vdev=hda,access=rw,target=rbd:rbd/ubuntu1204.img' 
]

Signed-off-by: David Scott 
Acked-by: Ian Campbell 
Acked-by: Roger Pau Monné 

Of course things may have regressed again since then.

Since target is basically passed as is to qdisk does libvirt not already
have some helpers in the QEMU backend for constructing such things?

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] vm_event regression in 4.7

[Andrew Cooper]

On 03/02/16 01:32, Tamas K Lengyel wrote:
>
>
> On Tue, Feb 2, 2016 at 6:00 PM, Andrew Cooper
> > wrote:
>
> On 03/02/2016 00:51, Tamas K Lengyel wrote:
>> Hello all,
>> with the latest master branch of Xen there is a regression
>> enabling vm_event on a domain. If an event listener was
>> previously active on the domain it is now not possible to
>> reenable events as the domctl returns -EINVAL. The problem seems
>> to stem from activating the magic page for vm_event using
>> prepare_ring_for_helper as it returns NULL. Further looking into
>> where things go wrong within that function it seems the page type
>> returned by __get_gfn_type_access is p2m_ram_logdirty with an
>> invalid mfn (0x) and then it hits "Error path:
>> not a suitable GFN at all".
>>
>> Can anyone point me to which change or what may be causing this?
>
> Did the previous event listener replace the page it stole from
> guest physmap for ring purposes when it exited?
>
>
> Ah, here is what seems to be the problem. Previously it was not
> required to do this during teardown. What we had was libxc would check
> if it can map the ring page with xc_map_foreign_pages, and it would
> repopulate the page if it failed before running xc_vm_event_enable.
> However, now it seems xc_map_foreign_pages return non-NULL the second
> time around as well, either though the page is not in the physmap.

This is the bug then.  If there isn't a page in the physmap,
xc_map_foreign_pages() should indicate an error.

> If I enforce libxc to run populate_physmap then I can get vm_event to
> initialize properly again. So the change seems to relate somehow the
> behavior of xc_map_foreign_pages.

This seems likely due to the splitting out of libxenforeignmem from
libxc, which included the the merging of 4? almost identical
map_foreign_$FOO() functions into one.  It is likely that there is a
subtle change in behaviour on an error path.

~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] Leaks in xc_tbuf_get_size() (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

George,

Looks like xentrace is the only maintained component which uses this. so
tag ;-)

On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> * CID 1351228:    (RESOURCE_LEAK)
> /tools/libxc/xc_tbuf.c: 73 in xc_tbuf_get_size()
> /tools/libxc/xc_tbuf.c: 77 in xc_tbuf_get_size()

Coverity is reporting these as new in 41b0aa569adb..9937763265d9 although
the file hasn't changed. However it does look correct that t_info is being
leaked by various paths in this function.

> 
> 
> _
> ___
> *** CID 1351228:    (RESOURCE_LEAK)
> /tools/libxc/xc_tbuf.c: 73 in xc_tbuf_get_size()
> 67 
> 68 t_info = xc_map_foreign_range(xch, DOMID_XEN,
> 69 sysctl.u.tbuf_op.size, PROT_READ | PROT_WRITE,
> 70 sysctl.u.tbuf_op.buffer_mfn);
> 71 
> 72 if ( t_info == NULL || t_info->tbuf_size == 0 )
> >>> CID 1351228:    (RESOURCE_LEAK)
> >>> Variable "t_info" going out of scope leaks the storage it points
> to.
> 73 return -1;
> 74 
> 75 *size = t_info->tbuf_size;
> 76 
> 77 return 0;
> 78 }
> /tools/libxc/xc_tbuf.c: 77 in xc_tbuf_get_size()
> 71 
> 72 if ( t_info == NULL || t_info->tbuf_size == 0 )
> 73 return -1;
> 74 
> 75 *size = t_info->tbuf_size;
> 76 
> >>> CID 1351228:    (RESOURCE_LEAK)
> >>> Variable "t_info" going out of scope leaks the storage it points
> to.
> 77 return 0;
> 78 }
> 79 
> 80 int xc_tbuf_enable(xc_interface *xch, unsigned long pages,
> unsigned long *mfn,
> 81    unsigned long *size)
> 82 {
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC] support more qdisk types

[Wei Liu]

On Tue, Feb 02, 2016 at 03:06:01PM -0700, Jim Fehlig wrote:
> Wei Liu wrote:
> > On Mon, Jan 25, 2016 at 05:25:02PM -0700, Jim Fehlig wrote:
> >> I would like to hear the community's opinion on supporting more qdisk 
> >> types in
> >> xl/libxl, e.g. nbd, rbd, iSCSI, etc. I prefer supporting additional qdisk 
> >> types
> >> in libxl over apps like xl or libvirt doing all the setup, producing a 
> >> block
> >> device, and then passing that to libxl. Each libxl app would have to
> >> re-implement functionality already provided by qdisk. libxl already 
> >> supports
> >> IDE, AHCI, SCSI, and Xen PV qdisks. My suggestion is to extend that to 
> >> initially
> >> include nbd, rbd, and iSCSI. Sheepdog, ssh, etc. could be added in the 
> >> future.
> >>
> > 
> > This is a good idea in general.
> > 
> >> I considered several approaches to supporting additional qdisk types, based
> >> primarily on changes to the disk cfg and interface. At one extreme is to 
> >> change
> >> nothing and use the existing 'target=' to encode all required config for 
> >> the
> >> additional qdisk types. libxl would need to be taught how to turn the blob 
> >> into
> >> an appropriate qdisk. 
> > 
> > I think you mean "libxlu would need to be taught how to turn that blob
> > into an appropriate libxl_device_disk" -- libxl doesn't parse user
> > configuration, it deals with the libxl_device_disk directly to construct
> > arguments for QEMU.
> 
> Right. But if the configuration is all encoded in 'target=', libxlu already
> parses that and puts it in libxl_device_disk.pdev_path.
> 
> > Either it is done by extending target= or adding discrete options, the
> > heavy lifting is going to be in libxlu.  I don't think we want to parse
> > a string inside libxl.
> 
> Ok. So in your opinion, even if any new disk config is encoded in 'target=',
> libxlu should split that up into (new) members of libxl_device_disk, not just
> plop it into libxl_device_disk.pdev_path?
> 

No, not necessarily. I didn't look closely in the code yesterday when
replying, sorry.

If  target= has always been shoveled into pdev_path, using that would be
fine. We already have mechanism to parse target= outside of libxl in
hotplug script.

Are you aware of all those hotplug scripts living under tools/hotplug ?
Does using hotplug script sound plausible to you?

Currently hotplug script for QEMU is broken and needs fixing though, but
I'm sure we can figure it out.

Wei.

> >> At the other extreme is extending xl-disk-configuration
> >> with discrete knobs for each possible config item and making the
> >> libxl_device_disk structure more hierarchical. E.g.
> >>
> > 
> > I don't think the second half (hierarchical structure) is closely tied
> > to whether target= is extended or discrete knobs are used.
> 
> Nod.
> 
> > And extending
> > the structure seems to be the right thing to do.
> 
> So what do you think of the approach in the RFC patch? It adds discrete knobs 
> in
> the disk config and extends the disk structure similarly. Before I can make
> progress on this we need to agree on extending the config and 
> libxl_device_disk
> structure.
> 
> Regards,
> Jim
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC] support more qdisk types

[Wei Liu]

On Wed, Feb 03, 2016 at 10:51:27AM +, Ian Campbell wrote:
> On Wed, 2016-02-03 at 10:35 +, Wei Liu wrote:
> > > Ok. So in your opinion, even if any new disk config is encoded in 
> > > 'target=',
> > > libxlu should split that up into (new) members of libxl_device_disk, not 
> > > just
> > > plop it into libxl_device_disk.pdev_path?
> > > 
> > 
> > No, not necessarily. I didn't look closely in the code yesterday when
> > replying, sorry.
> > 
> > If  target= has always been shoveled into pdev_path, using that would be
> > fine. We already have mechanism to parse target= outside of libxl in
> > hotplug script.
> > 
> > Are you aware of all those hotplug scripts living under tools/hotplug ?
> > Does using hotplug script sound plausible to you?
> > 
> > Currently hotplug script for QEMU is broken and needs fixing though, but
> > I'm sure we can figure it out.
> 
> How do hotplug scripts factor into this?
> 

If supporting all such block devices  requires presenting a block device
to QEMU? If QEMU directly handles them then hotplug script is not in the
picture.

Wei.

> Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Leaks in xc_tbuf_get_size() (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

On Wed, 2016-02-03 at 10:42 +, Andrew Cooper wrote:
> On 03/02/16 10:37, Ian Campbell wrote:
> > George,
> > 
> > Looks like xentrace is the only maintained component which uses this.
> > so
> > tag ;-)
> > 
> > On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> > > * CID 1351228:(RESOURCE_LEAK)
> > > /tools/libxc/xc_tbuf.c: 73 in xc_tbuf_get_size()
> > > /tools/libxc/xc_tbuf.c: 77 in xc_tbuf_get_size()
> > Coverity is reporting these as new in 41b0aa569adb..9937763265d9
> > although
> > the file hasn't changed. However it does look correct that t_info is
> > being
> > leaked by various paths in this function.
> 
> It is "new"ly spotted because xc_map_foreign_range() is now a straight
> function call rather than a function pointer pointing at functions with
> different behaviours.

That makes sense, thanks.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHv3 1/2] spinlock: move rwlock API and per-cpu rwlocks into their own files

[Jan Beulich]

>>> On 01.02.16 at 12:31,  wrote:
> From: Jennifer Herbert 
> 
> In preparation for a replacement read-write lock implementation, move
> the API and the per-cpu read-write locks into their own files.
> 
> Signed-off-by: Jennifer Herbert 
> Signed-off-by: David Vrabel 

I suppose this was meant to also state "no functional change" or
"no change to generated code" or some such?

> --- /dev/null
> +++ b/xen/include/xen/rwlock.h
> @@ -0,0 +1,150 @@
> +#ifndef __RWLOCK_H__
> +#define __RWLOCK_H__
> +
> +#include 

I suppose you need this because ...

> +#define read_lock(l)  _read_lock(l)
> +#define read_lock_irq(l)  _read_lock_irq(l)
> +#define read_lock_irqsave(l, f) \
> +({  \
> +BUILD_BUG_ON(sizeof(f) != sizeof(unsigned long));   \
> +((f) = _read_lock_irqsave(l));  \
> +})
> +
> +#define read_unlock(l)_read_unlock(l)
> +#define read_unlock_irq(l)_read_unlock_irq(l)
> +#define read_unlock_irqrestore(l, f)  _read_unlock_irqrestore(l, f)
> +#define read_trylock(l)   _read_trylock(l)
> +
> +#define write_lock(l) _write_lock(l)
> +#define write_lock_irq(l) _write_lock_irq(l)
> +#define write_lock_irqsave(l, f)\
> +({  \
> +BUILD_BUG_ON(sizeof(f) != sizeof(unsigned long));   \
> +((f) = _write_lock_irqsave(l)); \
> +})
> +#define write_trylock(l)  _write_trylock(l)
> +
> +#define write_unlock(l)   _write_unlock(l)
> +#define write_unlock_irq(l)   _write_unlock_irq(l)
> +#define write_unlock_irqrestore(l, f) _write_unlock_irqrestore(l, f)
> +
> +#define rw_is_locked(l)   _rw_is_locked(l)
> +#define rw_is_write_locked(l) _rw_is_write_locked(l)

... you move all the macro wrappers, but not the underlying
function declarations? Why is that? Apart from appearing
inconsistent, getting rid of the seeming stray include above
would be nice.

Or, looking at the next patch, was this instead done with the
goal of making that other patch more easily reviewable? In
which case this intention should have been stated at least
after the first --- separator above, and the second patch
should then remove the then unnecessary include again (and
likely put some other includes there which tight now get pulled
in via xen/spinlock.h).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/3] Move collectversions from ts-xen-build into Osstest::BuildSupport

[Ian Campbell]

I'm going to have a need for it elsewhere.

Signed-off-by: Ian Campbell 
---
 Osstest/BuildSupport.pm | 12 
 ts-xen-build| 13 +
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/Osstest/BuildSupport.pm b/Osstest/BuildSupport.pm
index 933f6e1..a183546 100644
--- a/Osstest/BuildSupport.pm
+++ b/Osstest/BuildSupport.pm
@@ -42,6 +42,7 @@ BEGIN {
 
   xendist
   $xendist
+  collect_xen_built_versions
 
   submodulefixup submodule_have submodule_find
 
@@ -84,6 +85,17 @@ sub xendist () {
($ho, 'xendist', '', $r{"buildjob"});
 }
 
+sub collect_xen_built_versions () {
+my $tools="$builddir/xen/tools";
+my $extras="$builddir/xen/extras";
+store_revision($ho, 'qemu', "$tools/ioemu-dir", 1);
+store_revision($ho, 'qemu', "$tools/qemu-xen-traditional-dir", 1);
+store_revision($ho, 'qemuu', "$tools/qemu-xen-dir", 1);
+store_revision($ho, 'seabios', "$tools/firmware/seabios-dir", 1);
+store_revision($ho, 'ovmf', "$tools/firmware/ovmf-dir", 1);
+store_revision($ho, 'minios', "$extras/mini-os", 1);
+}
+
 #- submodules -
 
 sub submodulefixup () {
diff --git a/ts-xen-build b/ts-xen-build
index 8f92729..382fe62 100755
--- a/ts-xen-build
+++ b/ts-xen-build
@@ -151,17 +151,6 @@ END
 }
 }
 
-sub collectversions () {
-my $tools="$builddir/xen/tools";
-my $extras="$builddir/xen/extras";
-store_revision($ho, 'qemu', "$tools/ioemu-dir", 1);
-store_revision($ho, 'qemu', "$tools/qemu-xen-traditional-dir", 1);
-store_revision($ho, 'qemuu', "$tools/qemu-xen-dir", 1);
-store_revision($ho, 'seabios', "$tools/firmware/seabios-dir", 1);
-store_revision($ho, 'ovmf', "$tools/firmware/ovmf-dir", 1);
-store_revision($ho, 'minios', "$extras/mini-os", 1);
-}
-
 sub divide () {
 # Only move hv to xeninstall, so that we can have
 # xenpolicy in tools tarball.
@@ -248,7 +237,7 @@ sub trapping ($) {
 checkout();
 
 trapping(\);
-trapping(\);
+trapping(\_xen_built_versions);
 
 die "*** something failed:\n\n".(join "\n\n",@probs)."\n** something failed"
 if @probs;
-- 
2.6.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 3/3] make-coverity-flight: set coverity_upload=true

[Ian Campbell]

Signed-off-by: Ian Campbell 
---
Apply once the previous patch + manual upload lead to us being happy.
---
 make-coverity-flight | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/make-coverity-flight b/make-coverity-flight
index dafb61c..d086762 100755
--- a/make-coverity-flight
+++ b/make-coverity-flight
@@ -41,7 +41,7 @@ 
build_hostflags=share-build-$suite-$arch,arch-$arch,suite-$suite,purpose-build
arch=$arch host_hostflags=$build_hostflags \
tree_xen=$TREE_COVERITY \
revision_xen=$REVISION_COVERITY \
-   coverity_upload=false
+   coverity_upload=true
 
 echo $flight
 
-- 
2.6.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 2/3] Add a weekly coverity flight

[Ian Campbell]

This primarily consists of ts-coverity-scan and make-coverity-flight
which constructs the sole job.

The most recently scanned revision is pushed to a new coverity-scanned
branch in the usual xen.git, tests are run on the master branch.

For the cr-* integration we treat branch=coverity as a special case of
tree=xen. I didn't think tree=coverity made much sense, and would
probably reach tendrils into lots of other places (such as the
invocations of check_tested).

I initially thoughts that $c{CoverityEmail} would need to be an actual
account registered with scan, however a manual experiment using
email=secur...@xen.org was accepted by the service. An "analysis
complete" message was sent to security@ while individual results mails
were sent to each member of the coverity project who was configured to
receive them. I think this is what we want. The "analysis complete"
mail contained no sensitive data, but also no real information other
than "success" (or presumably "failure" if that were to be the case).
I think going to security@ is probably OK.

I have run this in non-uploading mode on the production infra and then
run the curl manually, adjusting the CLI until it works and updated
the script to match. I have not yet run in uploading mode but will do
so once another upload is allowed by the service,

In my experiments the curl command took ~35 minutes to complete (rate
in the 100-200k range). Not sure if this is a problem. Note that curl
is run on the controller (via system_checked) and consequently has no
timeout etc.

Note that the token must be supplied with 
Cc: Andrew Cooper 
---
v2:
 - Split move of collect_xen_built_versions() into separate patch
 - Implemented support for coverity_upload = true (but don't yet set
   it)
 - Add host_hostflags to the job so it can actually run somewhere.
 - Call tsreadconfig() before referencing $r{coverity_upload} so that
   $r is actually populated.
 - use token=http://www.gnu.org/licenses/>.
+
+
+set -e -o posix
+
+branch=$1
+xenbranch=$2
+blessing=$3
+buildflight=$4
+
+flight=`./cs-flight-create $blessing $branch`
+
+. ./cri-common
+. ./ap-common
+. ./mfi-common
+
+defsuite=`getconfig DebianSuite`
+
+arch=amd64
+suite=$defsuite
+
+build_hostflags=share-build-$suite-$arch,arch-$arch,suite-$suite,purpose-build
+
+./cs-job-create $flight coverity-$arch coverity \
+   arch=$arch host_hostflags=$build_hostflags \
+   tree_xen=$TREE_COVERITY \
+   revision_xen=$REVISION_COVERITY \
+   coverity_upload=false
+
+echo $flight
+
+# Local variables:
+# mode: sh
+# sh-basic-offset: 2
+# indent-tabs-mode: nil
+# End:
diff --git a/production-config b/production-config
index f2f0584..e67a253 100644
--- a/production-config
+++ b/production-config
@@ -100,6 +100,10 @@ TftpGrubVersion -XX-XX
 XenUsePath /usr/groups/xencore/systems/bin/xenuse
 XenUseUser osstest
 
+# Results might include potential vulnerabilities.
+CoverityEmail secur...@xen.org
+CoverityTools cov-analysis-linux64-7.7.0.4.tar.gz
+
 # We use the IP address because Citrix can't manage reliable nameservice
 #DebianMirrorHost debian.uk.xensource.com
 #DebianMirrorHost 10.80.16.196
diff --git a/sg-run-job b/sg-run-job
index 20ebb64..7e592dd 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -445,6 +445,11 @@ proc prepare-build-host {} {
 run-ts . host-build-prep ts-xen-build-prep
 }
 
+proc need-hosts/coverity {} { return BUILD }
+proc run-job/coverity {} {
+run-ts . = ts-coverity-scan + host
+}
+
 #-- main program --
 
 jobdb::set-flight
diff --git a/ts-coverity-scan b/ts-coverity-scan
new file mode 100755
index 000..f3cc497
--- /dev/null
+++ b/ts-coverity-scan
@@ -0,0 +1,110 @@
+#!/usr/bin/perl -w
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2015 Citrix Inc.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see .
+
+use strict qw(vars);
+use DBI;
+use Osstest;
+use File::Path;
+use POSIX;
+use Osstest::TestSupport;
+use Osstest::BuildSupport;
+
+tsreadconfig();
+selectbuildhost(\@ARGV);
+# remaining arguments are passed as targets to "make"
+builddirsprops();
+
+# Require explicit opt in from flight construction
+my $coverity_upload = ($r{coverity_upload}//'false') =~ m/true/ ? 1 : 0;
+
+# This must contain exactly and only the token, for example there must
+# be no trailing "\n", otherwise it is included 

Re: [Xen-devel] [PATCH] libxenforeignmemory: handle partial failure correctly

[Wei Liu]

On Wed, Feb 03, 2016 at 10:10:01AM +, Ian Campbell wrote:
> Coverity rightly points out that checking for ret == NULL and then
> calling osdep unmap(ret) is wrong.
> 
> The intention on this code path is to turn partial failure into total
> failure when the err argument is NULL, so we want to take this patch
> whenever ret is _non_ NULL (and err_to_free is set, indicating err was
> NULL).
> 
> CID: 1351219
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
>  tools/libs/foreignmemory/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
> index 6591888..a872b95 100644
> --- a/tools/libs/foreignmemory/core.c
> +++ b/tools/libs/foreignmemory/core.c
> @@ -79,7 +79,7 @@ void *xenforeignmemory_map(xenforeignmemory_handle *fmem,
>  
>  ret = osdep_xenforeignmemory_map(fmem, dom, prot, num, arr, err);
>  
> -if ( ret == 0 && err_to_free )
> +if ( ret && err_to_free )
>  {
>  int i;
>  
> -- 
> 2.1.4
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/3] Add a weekly coverity flight

[Ian Campbell]

On Wed, 2016-02-03 at 09:46 +, Ian Campbell wrote:
> [...]
> +sub build () {
> +my $make = "make $makeflags";
> +
> +# Pre build things we don't want coverity to scan, but which are
> +# normally built by some other command.
> +target_cmd_build($ho, 1000, $builddir, < +cd $builddir/xen
> +./configure
> +$make -C tools/firmware/etherboot all
> +$make mini-os-dir
> +END
> +
> +# Now the stuff we want coverity to look at
> +target_cmd_build($ho, 9000, $builddir, < +cd $builddir/xen
> +export PATH=$builddir/covtools/bin:\$PATH
> +cov-build --dir cov-int $make -C extras/mini-os/
> +cov-build --dir cov-int $make xen tools

This omits building stubdom, which Andy's original script also did.

However stubdom exists as a category in the scan webui and there have
previously been results for stubdoms.

Andy, I presume you deliberately started excluding stubdoms at some point?
I think this is probably the right thing to do, at least for now, since
stubdoms run with guest privileges so aren't hugely interesting, plus they
include an awful lot of third party code which we don't want to be
scanning+triaging (especially given how out of date some of the code is).

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Xen 4.7 Development Update

[Haozhong Zhang]

On 02/01/16 10:45, Wei Liu wrote:
[...] 
> = Projects =
> 
> == Hypervisor == 
[...]
> === x86 === 
[...]
> *  VMX TSC scaling support
>   -  Haozhong Zhang

v4 patch series was sent out. Patch 1 - 3 were merged.

[...]

> == Toolstack == 
> 
> *  vNVDIMM support
>   -  Haozhong Zhang

v1 patch series and a design doc were sent out. Let's see how much I
can get before freeze.

Thanks,
Haozhong

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/3] Move collectversions from ts-xen-build into Osstest::BuildSupport

[Ian Campbell]

On Wed, 2016-02-03 at 09:46 +, Ian Campbell wrote:

These 3 should all have been tagged "OSSTEST v2", sorry for the omission.


> I'm going to have a need for it elsewhere.
> 
> Signed-off-by: Ian Campbell 
> ---
>  Osstest/BuildSupport.pm | 12 
>  ts-xen-build| 13 +
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/Osstest/BuildSupport.pm b/Osstest/BuildSupport.pm
> index 933f6e1..a183546 100644
> --- a/Osstest/BuildSupport.pm
> +++ b/Osstest/BuildSupport.pm
> @@ -42,6 +42,7 @@ BEGIN {
>  
>    xendist
>    $xendist
> +  collect_xen_built_versions
>  
>    submodulefixup submodule_have submodule_find
>  
> @@ -84,6 +85,17 @@ sub xendist () {
>   ($ho, 'xendist', '', $r{"buildjob"});
>  }
>  
> +sub collect_xen_built_versions () {
> +my $tools="$builddir/xen/tools";
> +my $extras="$builddir/xen/extras";
> +store_revision($ho, 'qemu', "$tools/ioemu-dir", 1);
> +store_revision($ho, 'qemu', "$tools/qemu-xen-traditional-dir", 1);
> +store_revision($ho, 'qemuu', "$tools/qemu-xen-dir", 1);
> +store_revision($ho, 'seabios', "$tools/firmware/seabios-dir", 1);
> +store_revision($ho, 'ovmf', "$tools/firmware/ovmf-dir", 1);
> +store_revision($ho, 'minios', "$extras/mini-os", 1);
> +}
> +
>  #- submodules -
>  
>  sub submodulefixup () {
> diff --git a/ts-xen-build b/ts-xen-build
> index 8f92729..382fe62 100755
> --- a/ts-xen-build
> +++ b/ts-xen-build
> @@ -151,17 +151,6 @@ END
>  }
>  }
>  
> -sub collectversions () {
> -my $tools="$builddir/xen/tools";
> -my $extras="$builddir/xen/extras";
> -store_revision($ho, 'qemu', "$tools/ioemu-dir", 1);
> -store_revision($ho, 'qemu', "$tools/qemu-xen-traditional-dir", 1);
> -store_revision($ho, 'qemuu', "$tools/qemu-xen-dir", 1);
> -store_revision($ho, 'seabios', "$tools/firmware/seabios-dir", 1);
> -store_revision($ho, 'ovmf', "$tools/firmware/ovmf-dir", 1);
> -store_revision($ho, 'minios', "$extras/mini-os", 1);
> -}
> -
>  sub divide () {
>  # Only move hv to xeninstall, so that we can have
>  # xenpolicy in tools tarball.
> @@ -248,7 +237,7 @@ sub trapping ($) {
>  checkout();
>  
>  trapping(\);
> -trapping(\);
> +trapping(\_xen_built_versions);
>  
>  die "*** something failed:\n\n".(join "\n\n",@probs)."\n** something
> failed"
>  if @probs;

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] tools: libxencall/foreignmemory: initialise handle->fd

[Wei Liu]

On Wed, Feb 03, 2016 at 10:09:42AM +, Ian Campbell wrote:
> Otherwise the osdep close on the error path touches an uninitialised
> varialble.
> 
> CID: 1351231 (foreignmemory) and 1351230 (call)
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
>  tools/libs/call/core.c  | 2 ++
>  tools/libs/foreignmemory/core.c | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/tools/libs/call/core.c b/tools/libs/call/core.c
> index bbf88de..5ca0372 100644
> --- a/tools/libs/call/core.c
> +++ b/tools/libs/call/core.c
> @@ -24,6 +24,8 @@ xencall_handle *xencall_open(xentoollog_logger *logger, 
> unsigned open_flags)
>  
>  if (!xcall) return NULL;
>  
> +xcall->fd = -1;
> +
>  xcall->flags = open_flags;
>  xcall->buffer_cache_nr = 0;
>  
> diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
> index cfb0a73..6591888 100644
> --- a/tools/libs/foreignmemory/core.c
> +++ b/tools/libs/foreignmemory/core.c
> @@ -27,6 +27,7 @@ xenforeignmemory_handle 
> *xenforeignmemory_open(xentoollog_logger *logger,
>  
>  if (!fmem) return NULL;
>  
> +fmem->fd = -1;
>  fmem->logger = logger;
>  fmem->logger_tofree = NULL;
>  
> -- 
> 2.1.4
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Leaks in xc_tbuf_get_size() (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Andrew Cooper]

On 03/02/16 10:37, Ian Campbell wrote:
> George,
>
> Looks like xentrace is the only maintained component which uses this. so
> tag ;-)
>
> On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
>> * CID 1351228:(RESOURCE_LEAK)
>> /tools/libxc/xc_tbuf.c: 73 in xc_tbuf_get_size()
>> /tools/libxc/xc_tbuf.c: 77 in xc_tbuf_get_size()
> Coverity is reporting these as new in 41b0aa569adb..9937763265d9 although
> the file hasn't changed. However it does look correct that t_info is being
> leaked by various paths in this function.

It is "new"ly spotted because xc_map_foreign_range() is now a straight
function call rather than a function pointer pointing at functions with
different behaviours.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] tools: xenconsole: cleanup when clock_gettime fails.

[Wei Liu]

On Wed, Feb 03, 2016 at 10:43:47AM +, Ian Campbell wrote:
> All other error paths in the infinite loop in handle_io use break, so
> as to free resources.
> 
> CID: 1351226
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

> ---
>  tools/console/daemon/io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
> index e2e7a6b..34666c4 100644
> --- a/tools/console/daemon/io.c
> +++ b/tools/console/daemon/io.c
> @@ -1053,7 +1053,7 @@ void handle_io(void)
>POLLIN|POLLPRI);
>  
>   if (clock_gettime(CLOCK_MONOTONIC, ) < 0)
> - return;
> + break;
>   now = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 100);
>  
>   /* Re-calculate any event counter allowances & unblock
> -- 
> 2.1.4
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] missing lock in percpu_rwlock? (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Andrew Cooper]

On 03/02/16 10:45, Ian Campbell wrote:
> On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
>> * CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
>> /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> Coverity seems to think this is new in 41b0aa569adb..9937763265d,
> presumably due to 
>
> commit f9dd43dddc0a31a4343a58072935c1b5c0cbbee
> Author: Malcolm Crossley 
> Date:   Fri Jan 22 16:04:41 2016 +0100
>
> rwlock: add per-cpu reader-writer lock infrastructure

Expected behaviour.  writer_activating is expected to only be written
under lock, but read without lock.

~Andrew

>
>> _
>> ___
>> *** CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
>> /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
>> 356 percpu_rwlock_t *percpu_rwlock)
>> 357 {
>> 358 /* Validate the correct per_cpudata variable has been
>> provided. */
>> 359 _percpu_rwlock_owner_check(per_cpudata, percpu_rwlock);
>> 360 
>> 361 ASSERT(percpu_rwlock->writer_activating);
>  CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
>  Accessing "percpu_rwlock->writer_activating" without holding lock
>> "percpu_rwlock.rwlock". Elsewhere, "percpu_rwlock.writer_activating" is
>> accessed with "percpu_rwlock.rwlock" held 1 out of 2 times (1 of these
>> accesses strongly imply that it is necessary).
>> 362 percpu_rwlock->writer_activating = 0;
>> 363 write_unlock(_rwlock->rwlock);
>> 364 }
>> 365 
>> 366 #define percpu_rw_is_write_locked(l)
>> _rw_is_write_locked(&((l)->rwlock))
>> 367 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

[Jan Beulich]

>>> On 03.02.16 at 08:00,  wrote:
> On 02/02/16 17:11, Stefano Stabellini wrote:
>> Once upon a time somebody made the decision that ACPI tables
>> on Xen should be static and included in hvmloader. That might have been
>> a bad decision but at least it was coherent. Loading only *some* tables
>> from QEMU, but not others, it feels like an incomplete design to me.
>>
>> For example, QEMU is currently in charge of emulating the PCI bus, why
>> shouldn't it be QEMU that generates the PRT and MCFG?
>>
> 
> To Keir, Jan and Andrew:
> 
> Are there anything related to ACPI that must be done (or are better to
> be done) in hvmloader?

Some of the static tables (FADT and HPET come to mind) likely would
better continue to live in hvmloader. MCFG (for example) coming from
qemu, otoh, would be quite natural (and would finally allow MMCFG
support for guests in the first place). I.e. ...

>> I prefer switching to QEMU building all ACPI tables for devices that it
>> is emulating. However this alternative is good too because it is
>> coherent with the current design.
> 
> I would prefer to this one if the final conclusion is that only one
> agent should be allowed to build guest ACPI. As I said above, it looks
> like a big change to switch to QEMU for all ACPI tables and I'm afraid
> it would break some existing guests. 

... I indeed think that tables should come from qemu for components
living in qemu, and from hvmloader for components coming from Xen.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Nested virtualization off VMware vSphere 6.0 with EL6 guests crashes on Xen 4.6

[Jan Beulich]

>>> On 02.02.16 at 23:05,  wrote:
> This is getting more and more bizzare.
> 
> I realized that this machine has VMCS shadowing so Xen does not trap on
> any vmwrite or vmread. Unless I update the VMCS shadowing bitmap - which
> I did for vmwrite and vmread to get a better view of this. It never
> traps on VIRTUAL_APIC_PAGE_ADDR accesses. It does trap on: 
> VIRTUAL_PROCESSOR_ID,
> VM_EXIT_MSR_LOAD_ADDR and GUEST_[ES,DS,FS,GS,TR]_SELECTORS.
> 
> (It may also trap on IO_BITMAP_A,B but I didn't print that out).
> 
> To confirm that the VMCS that will be given to the L2 guest is correct
> I added some printking of some states that ought to be pretty OK such
> as HOST_RIP or HOST_RSP - which are all 0!

But did you also check what the field of interest starts out as?

> If I let the nvmx_update_virtual_apic_address keep on going without
> modifying the VIRTUAL_APIC_PAGE_ADDR it later on crashes the nested
> guest:
> 
> EN) nvmx_handle_vmwrite: 0   

The missing characters at the beginning may just be a copy-and-
paste mistake, but they could also indicate a truncated log. Can
you clarify which of the two it is?

> (XEN) nvmx_handle_vmwrite: 0 
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 0 
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 0 
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 2008  
> (XEN) nvmx_handle_vmwrite: 800   
> (XEN) nvmx_handle_vmwrite: 804   
> (XEN) nvmx_handle_vmwrite: 806   
> (XEN) nvmx_handle_vmwrite: 80a   
> (XEN) nvmx_handle_vmwrite: 80e   
> (XEN) nvmx_update_virtual_apic_address: vCPU1 0x(vAPIC) 
> 0x0(APIC), 0x0(TPR) ctrl=b5b9effe sec=0 

Assuming the field starts out as other than all ones, could you check
its value on each of the intercepted VMWRITEs, to at least narrow
when it changes.

Kevin, Jun - are there any cases where the hardware would alter
this field's value? Like during some guest side LAPIC manipulations?
(The same monitoring as suggested during VMWRITEs could of
course also be added to LAPIC accesses visible to the hypervisor,
but I guess there won't be too many of those.)

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V8 2/5] x86/hvm: pkeys, add pkeys support for guest_walk_tables

[Jan Beulich]

>>> On 02.02.16 at 08:35,  wrote:
> Protection keys define a new 4-bit protection key field(PKEY) in bits 62:59 
> of
> leaf entries of the page tables.
> 
> PKRU register defines 32 bits, there are 16 domains and 2 attribute bits per
> domain in pkru, for each i (0 ≤ i ≤ 15), PKRU[2i] is the access-disable bit 
> for
> protection key i (ADi); PKRU[2i+1] is the write-disable bit for protection 
> key
> i (WDi). PKEY is index to a defined domain.
> 
> A fault is considered as a PKU violation if all of the following conditions 
> are
> true:
> 1.CR4_PKE=1.
> 2.EFER_LMA=1.
> 3.Page is present with no reserved bit violations.
> 4.The access is not an instruction fetch.
> 5.The access is to a user page.
> 6.PKRU.AD=1
> or The access is a data write and PKRU.WD=1
> and either CR0.WP=1 or it is a user access.
> 
> Signed-off-by: Huaitong Han 

Reviewed-by: Jan Beulich 
albeit ...

> Changes in v8:
> *Abstract out _write_cr4.

... I'm not happy about the chose name and will try to remember
to change it to e.g. raw_write_cr4(). Names starting with an
underscore and a lower case letter are reserved to symbols
local to a given translation unit, which in my reading doesn't fit
with them getting placed in header files.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] libxenforeignmemory: handle partial failure correctly

[Ian Campbell]

Coverity rightly points out that checking for ret == NULL and then
calling osdep unmap(ret) is wrong.

The intention on this code path is to turn partial failure into total
failure when the err argument is NULL, so we want to take this patch
whenever ret is _non_ NULL (and err_to_free is set, indicating err was
NULL).

CID: 1351219

Signed-off-by: Ian Campbell 
---
 tools/libs/foreignmemory/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index 6591888..a872b95 100644
--- a/tools/libs/foreignmemory/core.c
+++ b/tools/libs/foreignmemory/core.c
@@ -79,7 +79,7 @@ void *xenforeignmemory_map(xenforeignmemory_handle *fmem,
 
 ret = osdep_xenforeignmemory_map(fmem, dom, prot, num, arr, err);
 
-if ( ret == 0 && err_to_free )
+if ( ret && err_to_free )
 {
 int i;
 
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxl: Add CPU hotplug support for HVM domains without device model

[Wei Liu]

On Tue, Feb 02, 2016 at 04:02:12PM -0500, Boris Ostrovsky wrote:
> HVMlite domains add/remove VCPUs by toggling "availability" property in
> xenstore.
> 
> Signed-off-by: Boris Ostrovsky 

If this is how it is supposed to work:

Acked-by: Wei Liu 

However, I would like to ask for a follow-up patch to documentation
xen.git hvmlite.markdown (or whichever file you see fit).

> ---
>  tools/libxl/libxl.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 94b5656..ae8bbd8 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -5557,6 +5557,7 @@ int libxl_set_vcpuonline(libxl_ctx *ctx, uint32_t 
> domid, libxl_bitmap *cpumap)
>  case LIBXL_DOMAIN_TYPE_HVM:
>  switch (libxl__device_model_version_running(gc, domid)) {
>  case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> +case LIBXL_DEVICE_MODEL_VERSION_NONE:
>  rc = libxl__set_vcpuonline_xenstore(gc, domid, cpumap, );
>  break;
>  case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> -- 
> 1.7.1
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] tools: libxencall/foreignmemory: initialise handle->fd

[Ian Campbell]

Otherwise the osdep close on the error path touches an uninitialised
varialble.

CID: 1351231 (foreignmemory) and 1351230 (call)

Signed-off-by: Ian Campbell 
---
 tools/libs/call/core.c  | 2 ++
 tools/libs/foreignmemory/core.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/tools/libs/call/core.c b/tools/libs/call/core.c
index bbf88de..5ca0372 100644
--- a/tools/libs/call/core.c
+++ b/tools/libs/call/core.c
@@ -24,6 +24,8 @@ xencall_handle *xencall_open(xentoollog_logger *logger, 
unsigned open_flags)
 
 if (!xcall) return NULL;
 
+xcall->fd = -1;
+
 xcall->flags = open_flags;
 xcall->buffer_cache_nr = 0;
 
diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index cfb0a73..6591888 100644
--- a/tools/libs/foreignmemory/core.c
+++ b/tools/libs/foreignmemory/core.c
@@ -27,6 +27,7 @@ xenforeignmemory_handle 
*xenforeignmemory_open(xentoollog_logger *logger,
 
 if (!fmem) return NULL;
 
+fmem->fd = -1;
 fmem->logger = logger;
 fmem->logger_tofree = NULL;
 
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [libvirt bisection] complete build-i386-libvirt

[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 git://git.sv.gnu.org/gnulib.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:  a70f3b1c77912012905c6c5be3bf37b05592e80f
  Bug not present: 63e15ad5e093d747db99400c269f71e95bda4bbe
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/80184/


  commit a70f3b1c77912012905c6c5be3bf37b05592e80f
  Author: Michal Privoznik 
  Date:   Sat Jan 30 11:55:45 2016 +0100
  
  includes: Install libvirt-common.h
  
  The libvirt-common.h is build time generated file from .in.
  Obviously, it's generated into builddir and not srcdir. Problem
  is, the list of header files to install, virinc_HEADERS contains
  only $(srcdir)/*.h and this misses libvirt-common.h. This problem
  is pretty obvious when doing a VPATH build.
  
  Signed-off-by: Michal Privoznik 


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/80184.bisection-summary --basis-template=79451 
--blessings=real,real-bisect libvirt build-i386-libvirt libvirt-build
Searching for failure / basis pass:
 79891 fail [host=italia0] / 79451 [host=huxelrebe0] 79390 ok.
Failure / basis pass flights: 79891 / 79390
(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 git://git.sv.gnu.org/gnulib.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 041f7c9304788ec5c10ee5794612efa614a6eb61 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
9937763265d9597e5f2439249b16d995842cdf0f
Basis pass 680030c42b7453510f27a41f1778de6d3ade58aa 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
2e46e3f2539d026594ec1618e7df2c2bc8785b42
Generating revisions with ./adhoc-revtuple-generator  
git://libvirt.org/libvirt.git#680030c42b7453510f27a41f1778de6d3ade58aa-041f7c9304788ec5c10ee5794612efa614a6eb61
 
git://git.sv.gnu.org/gnulib.git#6cc32c63e80bc1a30c521b2f07f2b54909b59892-6cc32c63e80bc1a30c521b2f07f2b54909b59892
 
git://xenbits.xen.org/qemu-xen-traditional.git#21f6526d1da331611ac5fe12967549d1a04e149b-21f6526d1da331611ac5fe12967549d1a04e149b
 
git://xenbits.xen.org/qemu-xen.git#2ce1d30ef2858dfed72a281872579e5a26b090dd-2ce1d30ef2858dfed72a281872579e5a26b090dd
 
git://xenbits.xen.org/xen.git#2e46e3f2539d026594ec1618e7df2c2bc8785b42-9937763265d9597e5f2439249b16d995842cdf0f
>From git://cache:9419/git://libvirt.org/libvirt
   1fe6d8b..1794a01  master -> origin/master
Loaded 2001 nodes in revision graph
Searching for test results:
 79451 [host=huxelrebe0]
 79390 pass 680030c42b7453510f27a41f1778de6d3ade58aa 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
2e46e3f2539d026594ec1618e7df2c2bc8785b42
 79891 fail 041f7c9304788ec5c10ee5794612efa614a6eb61 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
9937763265d9597e5f2439249b16d995842cdf0f
 80117 pass 680030c42b7453510f27a41f1778de6d3ade58aa 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
2e46e3f2539d026594ec1618e7df2c2bc8785b42
 80123 fail 041f7c9304788ec5c10ee5794612efa614a6eb61 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
9937763265d9597e5f2439249b16d995842cdf0f
 80136 pass f226ecbfbb1231553f2948d6632acdb0b2266300 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
9937763265d9597e5f2439249b16d995842cdf0f
 80137 fail f73ad5d47e7e9c3d977e72e65b99a3c237a5e1bb 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 
9937763265d9597e5f2439249b16d995842cdf0f
 80169 pass 63e15ad5e093d747db99400c269f71e95bda4bbe 
6cc32c63e80bc1a30c521b2f07f2b54909b59892 
21f6526d1da331611ac5fe12967549d1a04e149b 
2ce1d30ef2858dfed72a281872579e5a26b090dd 

Re: [Xen-devel] [PATCH 2/3] Add a weekly coverity flight

[Andrew Cooper]

On 03/02/16 10:19, Ian Campbell wrote:
> On Wed, 2016-02-03 at 09:46 +, Ian Campbell wrote:
>>  [...]
>> +sub build () {
>> +my $make = "make $makeflags";
>> +
>> +# Pre build things we don't want coverity to scan, but which are
>> +# normally built by some other command.
>> +target_cmd_build($ho, 1000, $builddir, <> +cd $builddir/xen
>> +./configure
>> +$make -C tools/firmware/etherboot all
>> +$make mini-os-dir
>> +END
>> +
>> +# Now the stuff we want coverity to look at
>> +target_cmd_build($ho, 9000, $builddir, <> +cd $builddir/xen
>> +export PATH=$builddir/covtools/bin:\$PATH
>> +cov-build --dir cov-int $make -C extras/mini-os/
>> +cov-build --dir cov-int $make xen tools
> This omits building stubdom, which Andy's original script also did.
>
> However stubdom exists as a category in the scan webui and there have
> previously been results for stubdoms.
>
> Andy, I presume you deliberately started excluding stubdoms at some point?
> I think this is probably the right thing to do, at least for now, since
> stubdoms run with guest privileges so aren't hugely interesting, plus they
> include an awful lot of third party code which we don't want to be
> scanning+triaging (especially given how out of date some of the code is).

Correct.  That is precisely why I prevented stubdom and etherboot from
being scanned.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] missing lock in percpu_rwlock? (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> * CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
> /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()

Coverity seems to think this is new in 41b0aa569adb..9937763265d,
presumably due to 

commit f9dd43dddc0a31a4343a58072935c1b5c0cbbee
Author: Malcolm Crossley 
Date:   Fri Jan 22 16:04:41 2016 +0100

rwlock: add per-cpu reader-writer lock infrastructure

> _
> ___
> *** CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
> /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> 356 percpu_rwlock_t *percpu_rwlock)
> 357 {
> 358 /* Validate the correct per_cpudata variable has been
> provided. */
> 359 _percpu_rwlock_owner_check(per_cpudata, percpu_rwlock);
> 360 
> 361 ASSERT(percpu_rwlock->writer_activating);
> >>> CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
> >>> Accessing "percpu_rwlock->writer_activating" without holding lock
> "percpu_rwlock.rwlock". Elsewhere, "percpu_rwlock.writer_activating" is
> accessed with "percpu_rwlock.rwlock" held 1 out of 2 times (1 of these
> accesses strongly imply that it is necessary).
> 362 percpu_rwlock->writer_activating = 0;
> 363 write_unlock(_rwlock->rwlock);
> 364 }
> 365 
> 366 #define percpu_rw_is_write_locked(l)
> _rw_is_write_locked(&((l)->rwlock))
> 367 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxc: fix leak in xc_dom_load_hvm_kernel error path

[Ian Campbell]

On Wed, 2016-02-03 at 11:59 +0100, Roger Pau Monne wrote:
> Error path in xc_dom_load_hvm_kernel needs to use the 'error' label
> instead
> of directly returning. This is needed so the entries local variable is
> freed.
> 
> Coverity-ID: 1351227
> Signed-off-by: Roger Pau Monné 

Acked + applied, thanks.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 1/2] x86: support 2- and 3-way alternatives

[Andrew Cooper]

On 03/02/16 12:39, Jan Beulich wrote:
> Parts taken from Linux, but implementing the ALTERNATIVE*() macros
> recursively to avoid needless redundancy.
>
> Also make the .discard section non-writable (we might even consider
> dropping its alloc flag too) and limit the pushing and popping of
> sections.
>
> Signed-off-by: Jan Beulich 

There is quite a lot of whitespace damage and mixed tabs/spaces, but the
content looks ok.

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 1/2] x86: support 2- and 3-way alternatives

[Jan Beulich]

>>> On 03.02.16 at 14:15,  wrote:
> On 03/02/16 12:39, Jan Beulich wrote:
>> Parts taken from Linux, but implementing the ALTERNATIVE*() macros
>> recursively to avoid needless redundancy.
>>
>> Also make the .discard section non-writable (we might even consider
>> dropping its alloc flag too) and limit the pushing and popping of
>> sections.
>>
>> Signed-off-by: Jan Beulich 
> 
> There is quite a lot of whitespace damage and mixed tabs/spaces, but the
> content looks ok.

Damage? Mixture? I've tried to make it all Linux style (as the file was
meant to be) for the changes/additions, but I see I failed for some line
of the macro bodies (just fixed).

> Reviewed-by: Andrew Cooper 

Thanks.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] arm: p2m.c bug-fix: hypervisor hang on __p2m_get_mem_access

[Corneliu ZUZU]

When __p2m_get_mem_access gets called, the p2m lock is already taken
by either get_page_from_gva or p2m_get_mem_access.
Possible code paths:
1)  -> get_page_from_gva
-> p2m_mem_access_check_and_get_page
-> __p2m_get_mem_access
2)  -> p2m_get_mem_access
-> __p2m_get_mem_access

In both cases if __p2m_get_mem_access subsequently gets to
call p2m_lookup (happens if !radix_tree_lookup(...)), a hypervisor
hang will occur, since p2m_lookup also spin-locks on the p2m lock.

This bug-fix simply replaces the p2m_lookup call from __p2m_get_mem_access
with a call to __p2m_lookup and also adds an ASSERT to ensure that the p2m lock
is already taken upon __p2m_get_mem_access entry.

Signed-off-by: Corneliu ZUZU 

---
Changed since v1:
  * added p2m-lock ASSERT
---
 xen/arch/arm/p2m.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2190908..e8e6db4 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -468,6 +468,8 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
 #undef ACCESS
 };
 
+ASSERT(spin_is_locked(>lock));
+
 /* If no setting was ever set, just return rwx. */
 if ( !p2m->mem_access_enabled )
 {
@@ -490,7 +492,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
  * No setting was found in the Radix tree. Check if the
  * entry exists in the page-tables.
  */
-paddr_t maddr = p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
+paddr_t maddr = __p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
 if ( INVALID_PADDR == maddr )
 return -ESRCH;
 
-- 
2.5.0


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/2] x86/xstate: also use alternative asm on xsave side

[Jan Beulich]

From: Shuai Ruan 

This patch use alternavtive asm on the xsave side.
As xsaves use modified optimization like xsaveopt, xsaves
may not writing the FPU portion of the save image too.
So xsaves also need some extra tweaks.

Signed-off-by: Shuai Ruan 

Fix XSAVES opcode. Extend the other respective XSAVEOPT conditional to
cover XSAVES as well. Re-wrap comment being adjusted.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -250,27 +250,29 @@ void xsave(struct vcpu *v, uint64_t mask
 uint32_t hmask = mask >> 32;
 uint32_t lmask = mask;
 int word_size = mask & XSTATE_FP ? (cpu_has_fpu_sel ? 8 : 0) : -1;
+#define XSAVE(pfx) \
+alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", \
+ ".byte " pfx "0x0f,0xae,0x37\n", \
+ X86_FEATURE_XSAVEOPT, \
+ ".byte " pfx "0x0f,0xc7,0x27\n", \
+ X86_FEATURE_XSAVEC, \
+ ".byte " pfx "0x0f,0xc7,0x2f\n", \
+ X86_FEATURE_XSAVES, \
+ "=m" (*ptr), \
+ "a" (lmask), "d" (hmask), "D" (ptr))
 
 if ( word_size <= 0 || !is_pv_32bit_vcpu(v) )
 {
 typeof(ptr->fpu_sse.fip.sel) fcs = ptr->fpu_sse.fip.sel;
 typeof(ptr->fpu_sse.fdp.sel) fds = ptr->fpu_sse.fdp.sel;
 
-if ( cpu_has_xsaves )
-asm volatile ( ".byte 0x48,0x0f,0xc7,0x2f"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
-else if ( cpu_has_xsavec )
-asm volatile ( ".byte 0x48,0x0f,0xc7,0x27"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
-else if ( cpu_has_xsaveopt )
+if ( cpu_has_xsaveopt || cpu_has_xsaves )
 {
 /*
- * xsaveopt may not write the FPU portion even when the respective
- * mask bit is set. For the check further down to work we hence
- * need to put the save image back into the state that it was in
- * right after the previous xsaveopt.
+ * XSAVEOPT/XSAVES may not write the FPU portion even when the
+ * respective mask bit is set. For the check further down to work
+ * we hence need to put the save image back into the state that
+ * it was in right after the previous XSAVEOPT.
  */
 if ( word_size > 0 &&
  (ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] == 4 ||
@@ -279,14 +281,9 @@ void xsave(struct vcpu *v, uint64_t mask
 ptr->fpu_sse.fip.sel = 0;
 ptr->fpu_sse.fdp.sel = 0;
 }
-asm volatile ( ".byte 0x48,0x0f,0xae,0x37"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
 }
-else
-asm volatile ( ".byte 0x48,0x0f,0xae,0x27"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
+
+XSAVE("0x48,");
 
 if ( !(mask & ptr->xsave_hdr.xstate_bv & XSTATE_FP) ||
  /*
@@ -296,7 +293,7 @@ void xsave(struct vcpu *v, uint64_t mask
  (!(ptr->fpu_sse.fsw & 0x0080) &&
   boot_cpu_data.x86_vendor == X86_VENDOR_AMD) )
 {
-if ( cpu_has_xsaveopt && word_size > 0 )
+if ( (cpu_has_xsaveopt || cpu_has_xsaves) && word_size > 0 )
 {
 ptr->fpu_sse.fip.sel = fcs;
 ptr->fpu_sse.fdp.sel = fds;
@@ -317,24 +314,10 @@ void xsave(struct vcpu *v, uint64_t mask
 }
 else
 {
-if ( cpu_has_xsaves )
-asm volatile ( ".byte 0x0f,0xc7,0x2f"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
-else if ( cpu_has_xsavec )
-asm volatile ( ".byte 0x0f,0xc7,0x27"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
-else if ( cpu_has_xsaveopt )
-asm volatile ( ".byte 0x0f,0xae,0x37"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
-else
-asm volatile ( ".byte 0x0f,0xae,0x27"
-   : "=m" (*ptr)
-   : "a" (lmask), "d" (hmask), "D" (ptr) );
+XSAVE("");
 word_size = 4;
 }
+#undef XSAVE
 if ( word_size >= 0 )
 ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] = word_size;
 }


x86/xstate: also use alternative asm on xsave side

From: Shuai Ruan 

This patch use alternavtive asm on the xsave side.
As xsaves use modified optimization like xsaveopt, xsaves
may not writing the FPU portion of the save image too.
So xsaves also need some 

Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

[Jan Beulich]

>>> On 03.02.16 at 13:22,  wrote:
> On 02/03/16 02:18, Jan Beulich wrote:
>> >>> On 03.02.16 at 09:28,  wrote:
>> > On 02/02/16 14:15, Konrad Rzeszutek Wilk wrote:
>> >> > 3.1 Guest clwb/clflushopt/pcommit Enabling
>> >> > 
>> >> >  The instruction enabling is simple and we do the same work as in 
> KVM/QEMU.
>> >> >  - All three instructions are exposed to guest via guest cpuid.
>> >> >  - L1 guest pcommit is never intercepted by Xen.
>> >> 
>> >> I wish there was some watermarks like the PLE has.
>> >> 
>> >> My fear is that an unfriendly guest can issue sfence all day long
>> >> flushing out other guests MMC queue (the writes followed by pcommits).
>> >> Which means that an guest may have degraded performance as their
>> >> memory writes are being flushed out immediately as if they were
>> >> being written to UC instead of WB memory. 
>> > 
>> > pcommit takes no parameter and it seems hard to solve this problem
>> > from hardware for now. And the current VMX does not provide mechanism
>> > to limit the commit rate of pcommit like PLE for pause.
>> > 
>> >> In other words - the NVDIMM resource does not provide any resource
>> >> isolation. However this may not be any different than what we had
>> >> nowadays with CPU caches.
>> >>
>> > 
>> > Does Xen have any mechanism to isolate multiple guests' operations on
>> > CPU caches?
>> 
>> No. All it does is disallow wbinvd for guests not controlling any
>> actual hardware. Perhaps pcommit should at least be limited in
>> a similar way?
>>
> 
> But pcommit is a must that makes writes be persistent on pmem. I'll
> look at how guest wbinvd is limited in Xen.

But we could intercept it on guests _not_ supposed to use it, in order
to simply drop in on the floor.

> Any functions suggested, vmx_wbinvd_intercept()?

A good example, yes.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 03.02.16 at 13:50,  wrote:
>>  -Original Message-
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 03 February 2016 12:36
>> To: Paul Durrant
>> Cc: Andrew Cooper; Ian Campbell; Ian Jackson; Stefano Stabellini; Wei Liu;
>> Kevin Tian; zhiyuan...@intel.com; Zhang Yu; xen-devel@lists.xen.org; Keir
>> (Xen.org)
>> Subject: RE: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter
>> max_wp_ram_ranges.
>> 
>> >>> On 03.02.16 at 13:20,  wrote:
>> >>  -Original Message-
>> >> From: Jan Beulich [mailto:jbeul...@suse.com]
>> >> Sent: 03 February 2016 08:33
>> >> To: Zhang Yu
>> >> Cc: Andrew Cooper; Ian Campbell; Paul Durrant; Wei Liu; Ian Jackson;
>> Stefano
>> >> Stabellini; Kevin Tian; zhiyuan...@intel.com; xen-devel@lists.xen.org; 
>> >> Keir
>> >> (Xen.org)
>> >> Subject: Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter
>> >> max_wp_ram_ranges.
>> >>
>> >> >>> On 03.02.16 at 08:10,  wrote:
>> >> > On 2/2/2016 11:21 PM, Jan Beulich wrote:
>> >> > On 02.02.16 at 16:00,  wrote:
>> >> >>> The limit of 4G is to avoid the data missing from uint64 to uint32
>> >> >>> assignment. And I can accept the 8K limit for XenGT in practice.
>> >> >>> After all, it is vGPU page tables we are trying to trap and emulate,
>> >> >>> not normal page frames.
>> >> >>>
>> >> >>> And I guess the reason that one domain exhausting Xen's memory
>> can
>> >> >>> affect another domain is because rangeset uses Xen heap, instead of
>> >> the
>> >> >>> per-domain memory. So what about we use a 8K limit by now for
>> XenGT,
>> >> >>> and in the future, if a per-domain memory allocation solution for
>> >> >>> rangeset is ready, we do need to limit the rangeset size. Does this
>> >> >>> sounds more acceptable?
>> >> >>
>> >> >> The lower the limit the better (but no matter how low the limit
>> >> >> it won't make this a pretty thing). Anyway I'd still like to wait
>> >> >> for what Ian may further say on this.
>> >> >>
>> >> > Hi Jan, I just had a discussion with my colleague. We believe 8K could
>> >> > be the biggest limit for the write-protected ram ranges. If in the
>> >> > future, number of vGPU page tables exceeds this limit, we will modify
>> >> > our back-end device model to find a trade-off method, instead of
>> >> > extending this limit. If you can accept this value as the upper bound
>> >> > of rangeset, maybe we do not need to add any tool stack parameters,
>> but
>> >> > define a MAX_NR_WR_RAM_RANGES for the write-protected ram
>> >> rangesset. As
>> >> > to other rangesets, we keep their limit as 256. Does this sounds OK? :)
>> >>
>> >> I'm getting the impression that we're moving in circles. A blanket
>> >> limit above the 256 one for all domains is _not_ going to be
>> >> acceptable; going to 8k will still need host admin consent. With
>> >> your rangeset performance improvement patch, each range is
>> >> going to be tracked by a 40 byte structure (up from 32), which
>> >> already means an overhead increase for all the other ranges. 8k
>> >> of wp ranges implies an overhead beyond 448k (including the
>> >> xmalloc() overhead), which is not _that_ much, but also not
>> >> negligible.
>> >>
>> >
>> > ... which means we are still going to need a toolstack parameter to set the
>> > limit. We already have a parameter for VRAM size so is having a parameter
>> for
>> > max. GTT shadow ranges such a bad thing?
>> 
>> It's workable, but not nice (see also Ian's earlier response).
>> 
>> > Is the fact that the memory comes
>> > from xenheap rather than domheap the real problem?
>> 
>> Not the primary one, since except on huge memory machines
>> both heaps are identical. To me the primary one is the quite
>> more significant resource consumption in the first place (I'm not
>> going to repeat what I've written in already way too many
>> replies before).
> 
> Ok. Well the only way round tracking specific ranges for emulation (and 
> consequently suffering the overhead) is tracking by type. For XenGT I guess 
> it would be possible to live with a situation where a single ioreq server can 
> register all wp mem emulations for a given VM. I can't say I particularly 
> like that way of doing things but if it's the only way forward then I guess 
> we may have to live with it.

Well, subject to Ian not objecting (still awaiting some follow-up by
him), I didn't mean to say doing it the proposed way is a no-go.
All that I really insist on is that this larger resource consumption
won't go without some form of host admin consent.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter max_wp_ram_ranges.

[Jan Beulich]

>>> On 03.02.16 at 14:07,  wrote:
>>  -Original Message-
> [snip]
>> >> >> I'm getting the impression that we're moving in circles. A blanket
>> >> >> limit above the 256 one for all domains is _not_ going to be
>> >> >> acceptable; going to 8k will still need host admin consent. With
>> >> >> your rangeset performance improvement patch, each range is
>> >> >> going to be tracked by a 40 byte structure (up from 32), which
>> >> >> already means an overhead increase for all the other ranges. 8k
>> >> >> of wp ranges implies an overhead beyond 448k (including the
>> >> >> xmalloc() overhead), which is not _that_ much, but also not
>> >> >> negligible.
>> >> >>
>> >> >
>> >> > ... which means we are still going to need a toolstack parameter to set
>> the
>> >> > limit. We already have a parameter for VRAM size so is having a
>> parameter
>> >> for
>> >> > max. GTT shadow ranges such a bad thing?
>> >>
>> >> It's workable, but not nice (see also Ian's earlier response).
>> >>
>> >> > Is the fact that the memory comes
>> >> > from xenheap rather than domheap the real problem?
>> >>
>> >> Not the primary one, since except on huge memory machines
>> >> both heaps are identical. To me the primary one is the quite
>> >> more significant resource consumption in the first place (I'm not
>> >> going to repeat what I've written in already way too many
>> >> replies before).
>> >
>> > Ok. Well the only way round tracking specific ranges for emulation (and
>> > consequently suffering the overhead) is tracking by type. For XenGT I
>> guess
>> > it would be possible to live with a situation where a single ioreq server 
>> > can
>> > register all wp mem emulations for a given VM. I can't say I particularly
>> > like that way of doing things but if it's the only way forward then I guess
>> > we may have to live with it.
>> 
>> Well, subject to Ian not objecting (still awaiting some follow-up by
>> him), I didn't mean to say doing it the proposed way is a no-go.
>> All that I really insist on is that this larger resource consumption
>> won't go without some form of host admin consent.
> 
> Would you be ok with purely host admin consent e.g. just setting the limit 
> via boot command line?

I wouldn't be happy with that (and I've said so before), since it
would allow all VM this extra resource consumption.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Error booting Xen

[Dario Faggioli]

On Wed, 2016-02-03 at 13:10 +, Andrew Cooper wrote:
> On 03/02/16 12:55, Dario Faggioli wrote:
> > 
> > tbox login: (d1) mapping kernel into physical memory
> > (d1) about to get started...
> > (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c081
> > from 0xe023e008 to 0x00230010.
> > (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c082
> > from 0x82d0bfffe080 to 0x817ef990.
> > (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c083
> > from 0x82d0bfffe0a0 to 0x817f1f60.
> > (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0174
> > from 0xe008 to 0x0010.
> > (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0175
> > from 0x83026d40ffc0 to 0x.
> > (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0176
> > from 0x82d08023eaf0 to 0x817f1d60.
> > (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c084
> > from 0x00074700 to 0x00047700.
> > (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c081
> > from 0xe023e008 to 0x00230010.
> > (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c082
> > from 0x82d0b000 to 0x817ef990.
> > (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c083
> > from 0x82d0b020 to 0x817f1f60.
> > (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0174
> > from 0xe008 to 0x0010.
> > (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0175
> > from 0x8300866fffc0 to 0x.
> > (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0176
> > from 0x82d08023eaf0 to 0x817f1d60.
> > (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c084
> > from 0x00074700 to 0x00047700.
> > 
> > Because this is what you just said above, and that's... well...
> > just
> > impossible?!?! :-O
> 
> This is a Linux PV trying to set up the SYSCALL/SYSENTER MSRs when it
> shouldn't.  There is a fix upstream, but this specifically is
> harmless
> noise.
> 
That's ok... What we're arguing is that is happens as a consequence of
a domain being created, not just "by itself, after boot, without
starting any domain"  :-)

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter max_wp_ram_ranges.

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 03 February 2016 13:18
> To: Paul Durrant
> Cc: Andrew Cooper; Ian Campbell; Ian Jackson; Stefano Stabellini; Wei Liu;
> Kevin Tian; zhiyuan...@intel.com; Zhang Yu; xen-devel@lists.xen.org; Keir
> (Xen.org)
> Subject: RE: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter
> max_wp_ram_ranges.
> 
> >>> On 03.02.16 at 14:07,  wrote:
> >>  -Original Message-
> > [snip]
> >> >> >> I'm getting the impression that we're moving in circles. A blanket
> >> >> >> limit above the 256 one for all domains is _not_ going to be
> >> >> >> acceptable; going to 8k will still need host admin consent. With
> >> >> >> your rangeset performance improvement patch, each range is
> >> >> >> going to be tracked by a 40 byte structure (up from 32), which
> >> >> >> already means an overhead increase for all the other ranges. 8k
> >> >> >> of wp ranges implies an overhead beyond 448k (including the
> >> >> >> xmalloc() overhead), which is not _that_ much, but also not
> >> >> >> negligible.
> >> >> >>
> >> >> >
> >> >> > ... which means we are still going to need a toolstack parameter to
> set
> >> the
> >> >> > limit. We already have a parameter for VRAM size so is having a
> >> parameter
> >> >> for
> >> >> > max. GTT shadow ranges such a bad thing?
> >> >>
> >> >> It's workable, but not nice (see also Ian's earlier response).
> >> >>
> >> >> > Is the fact that the memory comes
> >> >> > from xenheap rather than domheap the real problem?
> >> >>
> >> >> Not the primary one, since except on huge memory machines
> >> >> both heaps are identical. To me the primary one is the quite
> >> >> more significant resource consumption in the first place (I'm not
> >> >> going to repeat what I've written in already way too many
> >> >> replies before).
> >> >
> >> > Ok. Well the only way round tracking specific ranges for emulation (and
> >> > consequently suffering the overhead) is tracking by type. For XenGT I
> >> guess
> >> > it would be possible to live with a situation where a single ioreq server
> can
> >> > register all wp mem emulations for a given VM. I can't say I particularly
> >> > like that way of doing things but if it's the only way forward then I 
> >> > guess
> >> > we may have to live with it.
> >>
> >> Well, subject to Ian not objecting (still awaiting some follow-up by
> >> him), I didn't mean to say doing it the proposed way is a no-go.
> >> All that I really insist on is that this larger resource consumption
> >> won't go without some form of host admin consent.
> >
> > Would you be ok with purely host admin consent e.g. just setting the limit
> > via boot command line?
> 
> I wouldn't be happy with that (and I've said so before), since it
> would allow all VM this extra resource consumption.
> 

The ball is back in Ian's court then.

  Paul

> Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCHv3 2/2] spinlock: fair read-write locks

[David Vrabel]

On 03/02/16 11:28, Jan Beulich wrote:
 On 01.02.16 at 12:31,  wrote:
>> +void queue_write_lock_slowpath(rwlock_t *lock)
>> +{
>> +u32 cnts;
>> +
>> +/* Put the writer into the wait queue. */
>> +spin_lock(>lock);
>> +
>> +/* Try to acquire the lock directly if no reader is present. */
>> +if ( !atomic_read(>cnts) &&
>> + (atomic_cmpxchg(>cnts, 0, _QW_LOCKED) == 0) )
>> +goto unlock;
>> +
>> +/*
>> + * Set the waiting flag to notify readers that a writer is pending,
>> + * or wait for a previous writer to go away.
>> + */
>> +for (;;)
> 
> Since everything else here has been nicely converted to Xen style,
> strictly speaking these should be
> 
> for ( ; ; )
> 
> but of course this is no reason to block the patch. Since however,
> as said in reply to patch 1, ...

TBH, I really think you're pointlessly nit-picking here.  This change
would make zero impact on readability.

>> --- a/xen/include/xen/rwlock.h
>> +++ b/xen/include/xen/rwlock.h
>> @@ -3,6 +3,188 @@
>>  
>>  #include 
> 
> ... this should go away if possible, it would be nice for the cosmetic
> thing above to also be fixed up at once.

The rwlock structure now includes a spinlock, so this #include is
required here.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/3] Add a weekly coverity flight

[Ian Jackson]

Ian Campbell writes ("[PATCH 2/3] Add a weekly coverity flight"):
> In my experiments the curl command took ~35 minutes to complete (rate
> in the 100-200k range). Not sure if this is a problem. Note that curl
> is run on the controller (via system_checked) and consequently has no
> timeout etc.

Can you use curl's builtin timeouts, or wrap it in alarm() ?
Otherwise I think it is possible for this to become wedged
indefinitely and need manual un-wedging.

AFAICT during the curl the only lock or resource which is held is the
build host share.  I think we can live with that.

> Note that the token must be supplied with  @/path/to/token. The latter appears to the server as a file upload
> rather than a text field in a form which doesn't work. In early
> attempts I thought that the trailing \n in /path/to/token might be an
> issue and hence wrote a big comment. However having discovered < vs @
> I am no longer 100% sure that is the case, but I left the comment
> anyway since I can observe on the wire that the \n is included in the
> upload (but each test takes ~35 mins and there is a ratelimit on the
> server side too).

Right.

> Deployment notes:
>  - Put cov-analysis-linux64-7.7.0.4.tar.gz in the Images
>directory. DONE in COLO
>  - Populate $HOME/.xen-osstest/coverity-secret with the token
>DONE in COLO
>  - Populate xen.git#coverity-scanned with an initial baseline, update
>ap-fetch-version-old to refer to it instead of master.
...
> +coverity)
> + #XXX doesn't exist yet, use master for now repo_tree_rev_fetch_git xen 
> $TREE_XEN coverity-scanned $LOCALREV_XEN

This XXX is out of date ?  And so the code should be fixed ?


> + case $branch in
> + coverity)
> + if [ "x$TREE_COVERITY" = x ]; then
> + export TREE_COVERITY=$TREE_XEN
> + fi
> + if [ "x$REVISION_COVERITY" = x ]; then
> + determine_version REVISION_COVERITY coverity COVERITY
> + export REVISION_COVERITY
> + fi
> + NEW_REVISION=$REVISION_COVERITY

I'm not sure why these variables are all REVISION_COVERITY rather than
REVISION_XEN.

But in general this looks good.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 3/3] make-coverity-flight: set coverity_upload=true

[Ian Jackson]

Ian Campbell writes ("[PATCH 3/3] make-coverity-flight: set 
coverity_upload=true"):
> Signed-off-by: Ian Campbell 
> ---
> Apply once the previous patch + manual upload lead to us being happy.

I'll ack this at that time.  Obviously there's nothing to review :-).

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 1/2] x86: support 2- and 3-way alternatives

[Jan Beulich]

Parts taken from Linux, but implementing the ALTERNATIVE*() macros
recursively to avoid needless redundancy.

Also make the .discard section non-writable (we might even consider
dropping its alloc flag too) and limit the pushing and popping of
sections.

Signed-off-by: Jan Beulich 

--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -46,18 +46,28 @@ extern void alternative_instructions(voi
 #define ALTINSTR_REPLACEMENT(newinstr, feature, number) /* replacement */ \
 b_replacement(number)":\n\t" newinstr "\n" e_replacement(number) 
":\n\t"
 
+#define ALTERNATIVE_N(newinstr, feature, number)   \
+".pushsection .altinstructions,\"a\"\n"\
+ALTINSTR_ENTRY(feature, number)\
+".section .discard,\"a\",@progbits\n"  \
+DISCARD_ENTRY(number)  \
+".section .altinstr_replacement, \"ax\"\n" \
+ALTINSTR_REPLACEMENT(newinstr, feature, number)\
+".popsection\n"
+
 /* alternative assembly primitive: */
-#define ALTERNATIVE(oldinstr, newinstr, feature)\
-OLDINSTR(oldinstr)  \
-".pushsection .altinstructions,\"a\"\n" \
-ALTINSTR_ENTRY(feature, 1)  \
-".popsection\n" \
-".pushsection .discard,\"aw\",@progbits\n"  \
-DISCARD_ENTRY(1)\
-".popsection\n" \
-".pushsection .altinstr_replacement, \"ax\"\n"  \
-ALTINSTR_REPLACEMENT(newinstr, feature, 1)  \
-".popsection"
+#define ALTERNATIVE(oldinstr, newinstr, feature) \
+OLDINSTR(oldinstr)   \
+   ALTERNATIVE_N(newinstr, feature, 1)
+
+#define ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, feature2) \
+ALTERNATIVE(oldinstr, newinstr1, feature1)   \
+   ALTERNATIVE_N(newinstr2, feature2, 2)
+
+#define ALTERNATIVE_3(oldinstr, newinstr1, feature1, newinstr2, feature2, \
+ newinstr3, feature3)\
+ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, feature2) \
+   ALTERNATIVE_N(newinstr3, feature3, 3)
 
 /*
  * Alternative instructions for different CPU types or capabilities.
@@ -93,6 +103,37 @@ extern void alternative_instructions(voi
asm volatile (ALTERNATIVE(oldinstr, newinstr, feature)  \
  : output : input)
 
+/*
+ * This is similar to alternative_io. But it has two features and
+ * respective instructions.
+ *
+ * If CPU has feature2, newinstr2 is used.
+ * Otherwise, if CPU has feature1, newinstr1 is used.
+ * Otherwise, oldinstr is used.
+ */
+#define alternative_io_2(oldinstr, newinstr1, feature1, newinstr2, \
+feature2, output, input...)\
+   asm volatile(ALTERNATIVE_2(oldinstr, newinstr1, feature1,   \
+  newinstr2, feature2) \
+: output : input)
+
+/*
+ * This is similar to alternative_io. But it has three features and
+ * respective instructions.
+ *
+ * If CPU has feature3, newinstr3 is used.
+ * Otherwise, if CPU has feature2, newinstr2 is used.
+ * Otherwise, if CPU has feature1, newinstr1 is used.
+ * Otherwise, oldinstr is used.
+ */
+#define alternative_io_3(oldinstr, newinstr1, feature1, newinstr2, \
+feature2, newinstr3, feature3, output, \
+input...)  \
+   asm volatile(ALTERNATIVE_3(oldinstr, newinstr1, feature1,   \
+  newinstr2, feature2, newinstr3,  \
+  feature3)\
+: output : input)
+
 /* Use this macro(s) if you need more than one output parameter. */
 #define ASM_OUTPUT2(a...) a
 



x86: support 2- and 3-way alternatives

Parts taken from Linux, but implementing the ALTERNATIVE*() macros
recursively to avoid needless redundancy.

Also make the .discard section non-writable (we might even consider
dropping its alloc flag too) and limit the pushing and popping of
sections.

Signed-off-by: Jan Beulich 

--- a/xen/include/asm-x86/alternative.h
+++ b/xen/include/asm-x86/alternative.h
@@ -46,18 +46,28 @@ extern void alternative_instructions(voi
 #define ALTINSTR_REPLACEMENT(newinstr, feature, number) /* replacement */ \
 b_replacement(number)":\n\t" newinstr "\n" e_replacement(number) 
":\n\t"
 
+#define ALTERNATIVE_N(newinstr, feature, number)   \
+

Re: [Xen-devel] [PATCH v4] xen: sched: convert RTDS from time to event driven model

[Dario Faggioli]

On Tue, 2016-02-02 at 13:09 -0500, Tianyang Chen wrote:
> On 2/2/2016 10:08 AM, Dario Faggioli wrote:
> > On Sun, 2016-01-31 at 23:32 -0500, Tianyang Chen wrote:
> > > 
> > > +struct rt_private *prv = rt_priv(ops);
> > > +struct list_head *replq = rt_replq(ops);
> > > +struct list_head *iter;
> > > +
> > > +ASSERT( spin_is_locked(>lock) );
> > > +
> > Is this really useful? Considering where this is called, I don't
> > think
> > it is.
> > 
> 
> This basically comes from __q_insert(). 
>
I see. Well it's still not necessary, IMO, so don't add it. At some
point, we may want to remove it from __runq_insert() too.

The bottom line is: prv->lock is, currently, the runqueue lock (it's
the lock for everything! :-D). It is pretty obvious that you need the
runq lock to manipulate the runqueue.

It is not the runqueue that you are manipulating here, it is the
replenishment queue, but as a matter of fact, prv->lock serializes that
too. So, if anything, it would be more useful to add a comment
somewhere, noting (reinforcing, I should probably say) the point that
also this new queue for replenishment is being serialized by prv->lock, 
than an assert like this one.

> I have been searching for the 
> definitions of vcpu_schedule_lock_irq() and pcpu_schedule_lock_irq().
> Do 
> you know where they are defined? I did some name search in the
> entire 
> xen directory but still nothing.
> 
They are auto-generated. Look in xen/include/xen/sched-if.h.

> > > +ASSERT( !__vcpu_on_p(svc) );
> > > +
> > > +list_for_each(iter, replq)
> > > +{
> > > +struct rt_vcpu * iter_svc = __p_elem(iter);
> > > +if ( svc->cur_deadline <= iter_svc->cur_deadline )
> > > +break;
> > > +}
> > > +
> > > +list_add_tail(>p_elem, iter);
> > > +}
> > This looks ok. The list_for_each()+list_ad_tail() part would be
> > basically identical to what we have inside __runq_insert(),
> > thgough,
> > wouldn't it?
> > 
> > It may make sense to define an _ordered_queue_insert() (or
> > _deadline_queue_insert(), since it's alwas the deadline that is
> > compared) utility function that we can then call in both cases.
> > 
> > > @@ -449,6 +507,7 @@ rt_init(struct scheduler *ops)
> > >   INIT_LIST_HEAD(>sdom);
> > >   INIT_LIST_HEAD(>runq);
> > >   INIT_LIST_HEAD(>depletedq);
> > > +INIT_LIST_HEAD(>replq);
> > > 
> > Is there a reason why you don't do at least the allocation of the
> > timer
> > here? Because, to me, this looks the best place for that.
> > 
> > >   cpumask_clear(>tickled);
> > > 
> > > @@ -473,6 +532,9 @@ rt_deinit(const struct scheduler *ops)
> > >   xfree(_cpumask_scratch);
> > >   _cpumask_scratch = NULL;
> > >   }
> > > +
> > > +kill_timer(prv->repl_timer);
> > > +
> > >   xfree(prv);
> > > 
> > And here, you're freeing prv, without having freed prv->timer,
> > which is
> > therefore leaked.
> > 
> > > @@ -632,6 +699,17 @@ rt_vcpu_insert(const struct scheduler *ops,
> > > struct vcpu *vc)
> > >   vcpu_schedule_unlock_irq(lock, vc);
> > > 
> > >   SCHED_STAT_CRANK(vcpu_insert);
> > > +
> > > +if( prv->repl_timer == NULL )
> > > +{
> > > +/* vc->processor has been set in schedule.c */
> > > +cpu = vc->processor;
> > > +
> > > +repl_timer = xzalloc(struct timer);
> > > +
> > > +prv->repl_timer = repl_timer;
> > > +init_timer(repl_timer, repl_handler, (void *)ops, cpu);
> > > +}
> > >   }
> > > 
> > This belong to rt_init, IMO.
> > 
> 
> When a new pool is created, the corresponding scheduler is also 
> initialized. But there is no pcpu assigned to that pool. 
>
Sure, and in fact, above, when commenting on rt_init() I said "Is there
a reason why you don't do at least the allocation of the timer here?"

But you're right, here I put it like all should belong to rt_init(), I
should have been more clear.

> If init_timer() 
> happens in rt_init, how does it know which cpu to pick?
> When move_domain() is called, there must be some pcpus in the 
> destination pool and then vcpu_insert happens.
> 
Allocating memory for the rt_priv copy of this particular scheduler
instance, definitely belong in rt_init().

Then, in this case, we could do the allocation in rt_init() and do the
initialization later, perhaps here, or the first time that the timer
needs to be started, or when the first pCPU is assigned to this
scheduler (by being assigned to the cpupool this scheduler services).

I honestly prefer the latter. That is to say, in rt_alloc_pdata(), you
check whether prv->repl_timer is NULL and you allocate and initialize
it for the pCPU being brought up. I also would put an explicit 'prv-
>repl_timer=NULL', with a comment that proper allocation and
initialization will happen later, and the reason why that is the case,
in rt_init().

What do you think?

> > > @@ -841,7 +881,6 @@ rt_schedule(const struct scheduler *ops,
> > > s_time_t
> > > now, bool_t tasklet_work_sched
> > >   /* 

Re: [Xen-devel] [PATCH 2/3] Add a weekly coverity flight

[Ian Campbell]

On Wed, 2016-02-03 at 12:12 +, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH 2/3] Add a weekly coverity flight"):
> > In my experiments the curl command took ~35 minutes to complete (rate
> > in the 100-200k range). Not sure if this is a problem. Note that curl
> > is run on the controller (via system_checked) and consequently has no
> > timeout etc.
> 
> Can you use curl's builtin timeouts, or wrap it in alarm() ?
> Otherwise I think it is possible for this to become wedged
> indefinitely and need manual un-wedging.

I added --max-time 3600.

> AFAICT during the curl the only lock or resource which is held is the
> build host share.  I think we can live with that.

There's the per-branch lock too, FWIW.

> > Deployment notes:
> >  - Put cov-analysis-linux64-7.7.0.4.tar.gz in the Images
> >    directory. DONE in COLO
> >  - Populate $HOME/.xen-osstest/coverity-secret with the token
> >    DONE in COLO
> >  - Populate xen.git#coverity-scanned with an initial baseline, update
> >    ap-fetch-version-old to refer to it instead of master.
> ...
> > +coverity)
> > +   #XXX doesn't exist yet, use master for now
> > repo_tree_rev_fetch_git xen $TREE_XEN coverity-scanned $LOCALREV_XEN
> 
> This XXX is out of date ?  And so the code should be fixed ?

coverity-scanned doesn't exist in xen.git yet, so for now this is still
correct, its the subject of the 3rd deployment note.

I'd be happy to push 9937763265d9597e5f2439249b16d995842cdf0f (the subject
of my recent adhoc test) to that new branch in xen.git right away though if
you agree and then update this code.

> > +   case $branch in
> > +   coverity)
> > +   if [ "x$TREE_COVERITY" = x ]; then
> > +   export TREE_COVERITY=$TREE_XEN
> > +   fi
> > +   if [ "x$REVISION_COVERITY" = x ]; then
> > +   determine_version REVISION_COVERITY coverity
> > COVERITY
> > +   export REVISION_COVERITY
> > +   fi
> > +   NEW_REVISION=$REVISION_COVERITY
> 
> I'm not sure why these variables are all REVISION_COVERITY rather than
> REVISION_XEN.

IIRC I had trouble getting cr-daily-branch's determine_version to populate
REVISION_XEN using values from "ap-fetch-version* coverity", maybe due to
the [ "x$tbranch" = "x$branch" ] ? 

or else there was some other wrinkle around that area.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Error booting Xen

[Dario Faggioli]

On Wed, 2016-02-03 at 17:05 +0530, Harmandeep Kaur wrote:
> On Wed, Feb 3, 2016 at 1:53 PM, Dario Faggioli
>  wrote:
> > 
> Maybe I failed to shutdown a guest which was showing up
> on next boot. But there are no auto-starting guests.
> Following link goes to latest serial log
> http://paste2.org/5PDz9bP1 
>
No, sorry, I probably am not getting. Are you saying that, just booting
Xen and *not* doing anything else --either from SSH, from the keyboard
of that box, from serial connection... anything at all-- and without
any guest configured to auto start itself, results in these lines to
appear on your serial console?

tbox login: (d1) mapping kernel into physical memory
(d1) about to get started...
(XEN) traps.c:2684:d1v0 Domain attempted WRMSR c081 from 
0xe023e008 to 0x00230010.
(XEN) traps.c:2684:d1v0 Domain attempted WRMSR c082 from 
0x82d0bfffe080 to 0x817ef990.
(XEN) traps.c:2684:d1v0 Domain attempted WRMSR c083 from 
0x82d0bfffe0a0 to 0x817f1f60.
(XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0174 from 
0xe008 to 0x0010.
(XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0175 from 
0x83026d40ffc0 to 0x.
(XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0176 from 
0x82d08023eaf0 to 0x817f1d60.
(XEN) traps.c:2684:d1v0 Domain attempted WRMSR c084 from 
0x00074700 to 0x00047700.
(XEN) traps.c:2684:d1v1 Domain attempted WRMSR c081 from 
0xe023e008 to 0x00230010.
(XEN) traps.c:2684:d1v1 Domain attempted WRMSR c082 from 
0x82d0b000 to 0x817ef990.
(XEN) traps.c:2684:d1v1 Domain attempted WRMSR c083 from 
0x82d0b020 to 0x817f1f60.
(XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0174 from 
0xe008 to 0x0010.
(XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0175 from 
0x8300866fffc0 to 0x.
(XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0176 from 
0x82d08023eaf0 to 0x817f1d60.
(XEN) traps.c:2684:d1v1 Domain attempted WRMSR c084 from 
0x00074700 to 0x00047700.

Because this is what you just said above, and that's... well... just
impossible?!?! :-O

What's the output then, if you login (either via serial, ssh or regular
keyboad+monitor) and run `xl list' and `xl vcpu-list' as the very first
thing?  What's inside `ls -l /var/log/xen/' (or `ls -l
/var/local/log/xen')?

> and then I created a guest with
> config (http://paste2.org/azvj25Hg) and
> http://paste2.org/cgKna0j1 log was presented at terminal.
> 
Well, it looks like the guest boot did not indeed go well, but that is
probably because of other thing... can you try uncommenting the line
'extra = "root=/dev/xvda1"' from the config?

Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)



signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter max_wp_ram_ranges.

[Paul Durrant]

> -Original Message-
[snip]
> >> >> I'm getting the impression that we're moving in circles. A blanket
> >> >> limit above the 256 one for all domains is _not_ going to be
> >> >> acceptable; going to 8k will still need host admin consent. With
> >> >> your rangeset performance improvement patch, each range is
> >> >> going to be tracked by a 40 byte structure (up from 32), which
> >> >> already means an overhead increase for all the other ranges. 8k
> >> >> of wp ranges implies an overhead beyond 448k (including the
> >> >> xmalloc() overhead), which is not _that_ much, but also not
> >> >> negligible.
> >> >>
> >> >
> >> > ... which means we are still going to need a toolstack parameter to set
> the
> >> > limit. We already have a parameter for VRAM size so is having a
> parameter
> >> for
> >> > max. GTT shadow ranges such a bad thing?
> >>
> >> It's workable, but not nice (see also Ian's earlier response).
> >>
> >> > Is the fact that the memory comes
> >> > from xenheap rather than domheap the real problem?
> >>
> >> Not the primary one, since except on huge memory machines
> >> both heaps are identical. To me the primary one is the quite
> >> more significant resource consumption in the first place (I'm not
> >> going to repeat what I've written in already way too many
> >> replies before).
> >
> > Ok. Well the only way round tracking specific ranges for emulation (and
> > consequently suffering the overhead) is tracking by type. For XenGT I
> guess
> > it would be possible to live with a situation where a single ioreq server 
> > can
> > register all wp mem emulations for a given VM. I can't say I particularly
> > like that way of doing things but if it's the only way forward then I guess
> > we may have to live with it.
> 
> Well, subject to Ian not objecting (still awaiting some follow-up by
> him), I didn't mean to say doing it the proposed way is a no-go.
> All that I really insist on is that this larger resource consumption
> won't go without some form of host admin consent.
> 

Would you be ok with purely host admin consent e.g. just setting the limit via 
boot command line?

  Paul

> Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

[Haozhong Zhang]

On 02/03/16 12:02, Stefano Stabellini wrote:
> On Wed, 3 Feb 2016, Haozhong Zhang wrote:
> > On 02/02/16 17:11, Stefano Stabellini wrote:
> > > On Mon, 1 Feb 2016, Haozhong Zhang wrote:
[...]
> > > >  This design treats host NVDIMM devices as ordinary MMIO devices:
> > > >  (1) Dom0 Linux NVDIMM driver is responsible to detect (through NFIT)
> > > >  and drive host NVDIMM devices (implementing block device
> > > >  interface). Namespaces and file systems on host NVDIMM devices
> > > >  are handled by Dom0 Linux as well.
> > > > 
> > > >  (2) QEMU mmap(2) the pmem NVDIMM devices (/dev/pmem0) into its
> > > >  virtual address space (buf).
> > > > 
> > > >  (3) QEMU gets the host physical address of buf, i.e. the host system
> > > >  physical address that is occupied by /dev/pmem0, and calls Xen
> > > >  hypercall XEN_DOMCTL_memory_mapping to map it to a DomU.
> > > 
> > > How is this going to work from a security perspective? Is it going to
> > > require running QEMU as root in Dom0, which will prevent NVDIMM from
> > > working by default on Xen? If so, what's the plan?
> > >
> > 
> > Oh, I forgot to address the non-root qemu issues in this design ...
> > 
> > The default user:group of /dev/pmem0 is root:disk, and its permission
> > is rw-rw. We could lift the others permission to rw, so that
> > non-root QEMU can mmap /dev/pmem0. But it looks too risky.
> 
> Yep, too risky.
> 
> 
> > Or, we can make a file system on /dev/pmem0, create files on it, set
> > the owner of those files to xen-qemuuser-domid$domid, and then pass
> > those files to QEMU. In this way, non-root QEMU should be able to
> > mmap those files.
> 
> Maybe that would work. Worth adding it to the design, I would like to
> read more details on it.
> 
> Also note that QEMU initially runs as root but drops privileges to
> xen-qemuuser-domid$domid before the guest is started. Initially QEMU
> *could* mmap /dev/pmem0 while is still running as root, but then it
> wouldn't work for any devices that need to be mmap'ed at run time
> (hotplug scenario).
>

Thanks for this information. I'll test some experimental code and then
post a design to address the non-root qemu issue.

> 
> > > >  (ACPI part is described in Section 3.3 later)
> > > > 
> > > >  Above (1)(2) have already been done in current QEMU. Only (3) is
> > > >  needed to implement in QEMU. No change is needed in Xen for address
> > > >  mapping in this design.
> > > > 
> > > >  Open: It seems no system call/ioctl is provided by Linux kernel to
> > > >get the physical address from a virtual address.
> > > >/proc//pagemap provides information of mapping from
> > > >VA to PA. Is it an acceptable solution to let QEMU parse this
> > > >file to get the physical address?
> > > 
> > > Does it work in a non-root scenario?
> > >
> > 
> > Seemingly no, according to Documentation/vm/pagemap.txt in Linux kernel:
> > | Since Linux 4.0 only users with the CAP_SYS_ADMIN capability can get PFNs.
> > | In 4.0 and 4.1 opens by unprivileged fail with -EPERM.  Starting from
> > | 4.2 the PFN field is zeroed if the user does not have CAP_SYS_ADMIN.
> > | Reason: information about PFNs helps in exploiting Rowhammer 
> > vulnerability.
> >
> > A possible alternative is to add a new hypercall similar to
> > XEN_DOMCTL_memory_mapping but receiving virtual address as the address
> > parameter and translating to machine address in the hypervisor.
> 
> That might work.
> 
> 
> > > >  Open: For a large pmem, mmap(2) is very possible to not map all SPA
> > > >occupied by pmem at the beginning, i.e. QEMU may not be able to
> > > >get all SPA of pmem from buf (in virtual address space) when
> > > >calling XEN_DOMCTL_memory_mapping.
> > > >Can mmap flag MAP_LOCKED or mlock(2) be used to enforce the
> > > >entire pmem being mmaped?
> > > 
> > > Ditto
> > >
> > 
> > No. If I take the above alternative for the first open, maybe the new
> > hypercall above can inject page faults into dom0 for the unmapped
> > virtual address so as to enforce dom0 Linux to create the page
> > mapping.
> 
> Otherwise you need to use something like the mapcache in QEMU
> (xen-mapcache.c), which admittedly, given its complexity, would be best
> to avoid.
>

Definitely not mapcache like things. What I want is something similar to
what emulate_gva_to_mfn() in Xen does.

[...]
> > > If we start asking QEMU to build ACPI tables, why should we stop at NFIT
> > > and SSDT?
> > 
> > for easing my development of supporting vNVDIMM in Xen ... I mean
> > NFIT and SSDT are the only two tables needed for this purpose and I'm
> > afraid to break exiting guests if I completely switch to QEMU for
> > guest ACPI tables.
> 
> I realize that my words have been a bit confusing. Not /all/ ACPI
> tables, just all the tables regarding devices for which QEMU is in
> charge (the PCI bus and all devices behind it). Anything related to cpus
> and memory (FADT, MADT, etc) 

[Xen-devel] [xen-unstable-smoke test] 80205: tolerable all pass - PUSHED

[osstest service owner]

flight 80205 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/80205/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  0f2b207d464f6b2a3508934a86b79557d3d0f0fd
baseline version:
 xen  ffc342fbb060cd753fc3a5f6fb6f550dd29a2637

Last test of basis79987  2016-02-02 16:03:19 Z0 days
Testing same since80205  2016-02-03 12:03:08 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Boris Ostrovsky 
  Corneliu ZUZU 
  Ian Campbell 
  Ian Jackson 
  Konrad Rzeszutek Wilk 
  Roger Pau Monne 
  Roger Pau Monné 
  Vitaly Kuznetsov 
  Wei Liu 
  Zoltan Kiss 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 pass
 test-amd64-amd64-libvirt pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=0f2b207d464f6b2a3508934a86b79557d3d0f0fd
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x '!=' x/home/osstest/repos/lock ']'
++ OSSTEST_REPOS_LOCK_LOCKED=/home/osstest/repos/lock
++ exec with-lock-ex -w /home/osstest/repos/lock ./ap-push xen-unstable-smoke 
0f2b207d464f6b2a3508934a86b79557d3d0f0fd
+ branch=xen-unstable-smoke
+ revision=0f2b207d464f6b2a3508934a86b79557d3d0f0fd
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z /home/osstest/repos ']'
+++ '[' '!' -d /home/osstest/repos ']'
+++ echo /home/osstest/repos
++ repos=/home/osstest/repos
++ repos_lock=/home/osstest/repos/lock
++ '[' x/home/osstest/repos/lock '!=' x/home/osstest/repos/lock ']'
+ . ./cri-common
++ . ./cri-getconfig
++ umask 002
+ select_xenbranch
+ case "$branch" in
+ tree=xen
+ xenbranch=xen-unstable-smoke
+ qemuubranch=qemu-upstream-unstable
+ '[' xxen = xlinux ']'
+ linuxbranch=
+ '[' xqemu-upstream-unstable = x ']'
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable-smoke
+ prevxenbranch=xen-unstable
+ '[' x0f2b207d464f6b2a3508934a86b79557d3d0f0fd = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo 

Re: [Xen-devel] [PATCH v2 2/2] x86/xstate: also use alternative asm on xsave side

[Jan Beulich]

>>> On 03.02.16 at 14:26,  wrote:
> On 03/02/16 12:39, Jan Beulich wrote:
>> --- a/xen/arch/x86/xstate.c
>> +++ b/xen/arch/x86/xstate.c
>> @@ -250,27 +250,29 @@ void xsave(struct vcpu *v, uint64_t mask
>>  uint32_t hmask = mask >> 32;
>>  uint32_t lmask = mask;
>>  int word_size = mask & XSTATE_FP ? (cpu_has_fpu_sel ? 8 : 0) : -1;
>> +#define XSAVE(pfx) \
>> +alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", \
>> + ".byte " pfx "0x0f,0xae,0x37\n", \
>> + X86_FEATURE_XSAVEOPT, \
>> + ".byte " pfx "0x0f,0xc7,0x27\n", \
>> + X86_FEATURE_XSAVEC, \
>> + ".byte " pfx "0x0f,0xc7,0x2f\n", \
>> + X86_FEATURE_XSAVES, \
> 
> Given that the options are a little out of order and using raw bytes,
> would you mind annotating the lines with the operations. e.g.
> 
> +alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", /* xsave */ \
> + ".byte " pfx "0x0f,0xae,0x37\n", /* xsaveopt */ \
> + X86_FEATURE_XSAVEOPT, \
> + ".byte " pfx "0x0f,0xc7,0x27\n", /* xsavec */ \
> + X86_FEATURE_XSAVEC, \
> + ".byte " pfx "0x0f,0xc7,0x2f\n", /* xsaves */ \
> + X86_FEATURE_XSAVES, \
> 
> IMO, this is somewhat clearer to read.

Okay, since I had been considering this too, I've just done so.

> Otherwise,
> 
> Reviewed-by: Andrew Cooper 

Thanks, Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] xen/arm: drop hip04 support

[Ian Campbell]

On Tue, 2016-02-02 at 13:13 +, Zoltan Kiss wrote:
> This platform is no longer actively used, but it makes GICv2 development
> harder.
> 
> Signed-off-by: Zoltan Kiss 

Thanks, I appreciate the proactive removal of no-longer supported
platforms.

I was going to ask you to confirm that you were still part of Huawei while
on secondment to Linaro, but I see you have CCd that other hat of yours as
well, which is good enough for me. Hence:

Acked-by: Ian Campbell 

and applied.

I don't see anything obvious in the wiki / docs which would need
updating/deleting, if you know of anything please could you either update
or let us know.

Thanks,
Ian.

> ---
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7c1bf82..12f147c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -177,11 +177,6 @@ S:   Supported
>  F:   xen/arch/x86/debug.c
>  F:   tools/debugger/gdbsx/
>  
> -HISILICON HIP04 SUPPORT
> -M:   Zoltan Kiss 
> -S:   Supported
> -F:   xen/arch/arm/gic-hip04.c
> -
>  INTEL(R) TRUSTED EXECUTION TECHNOLOGY (TXT)
>  M:   Gang Wei 
>  M:   Shane Wang 
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-
> printk.txt
> index 7e03955..41b528b 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -37,7 +37,6 @@ the name of the machine:
>    - dra7: printk with 8250 on DRA7 platform
>    - exynos5250: printk with the second UART
>    - fastmodel: printk on ARM Fastmodel software emulators
> -  - hip04-d01: printk with 8250 on HiSilicon Hip-04 D01
>    - juno: printk with pl011 on Juno platform
>    - lager: printk with SCIF0 on Renesas R-Car H2 processors
>    - midway: printk with the pl011 on Calxeda Midway processors
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 2f050f5..6a58a41 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -12,7 +12,6 @@ obj-y += domctl.o
>  obj-y += sysctl.o
>  obj-y += domain_build.o
>  obj-y += gic.o gic-v2.o
> -obj-$(CONFIG_ARM_32) += gic-hip04.o
>  obj-$(CONFIG_HAS_GICV3) += gic-v3.o
>  obj-y += io.o
>  obj-y += irq.o
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 4947e64..d6bbe7c 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -42,7 +42,6 @@ EARLY_PRINTK_brcm   := 8250,0xF040AB00,2
>  EARLY_PRINTK_dra7   := 8250,0x4806A000,2
>  EARLY_PRINTK_fastmodel  := pl011,0x1c09,115200
>  EARLY_PRINTK_exynos5250 := exynos4210,0x12c2
> -EARLY_PRINTK_hip04-d01  := 8250,0xE4007000,2
>  EARLY_PRINTK_juno   := pl011,0x7ff8
>  EARLY_PRINTK_lager  := scif,0xe6e6
>  EARLY_PRINTK_midway := pl011,0xfff36000
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> deleted file mode 100644
> index a42cf24..000
> --- a/xen/arch/arm/gic-hip04.c
> +++ /dev/null
> @@ -1,757 +0,0 @@
> -/*
> - * xen/arch/arm/gic-hip04.c
> - *
> - * Generic Interrupt Controller for HiSilicon Hip04 platform
> - * Based heavily on gic-v2.c (id
> 3bcf563fec26378f7f4cf1e2ad0d4d5b3f341919)
> - *
> - * Tim Deegan 
> - * Copyright (c) 2011 Citrix Systems.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - * GNU General Public License for more details.
> - */
> -
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -#include 
> -
> -/*
> - * LR register definitions are GIC v2 specific.
> - * Moved these definitions from header file to here
> - */
> -#define GICH_V2_LR_VIRTUAL_MASK0x3ff
> -#define GICH_V2_LR_VIRTUAL_SHIFT   0
> -#define GICH_V2_LR_PHYSICAL_MASK   0x3ff
> -#define GICH_V2_LR_PHYSICAL_SHIFT  10
> -#define GICH_V2_LR_STATE_MASK  0x3
> -#define GICH_V2_LR_STATE_SHIFT 28
> -#define GICH_V2_LR_PRIORITY_SHIFT  23
> -#define GICH_V2_LR_PRIORITY_MASK   0x1f
> -#define GICH_V2_LR_HW_SHIFT31
> -#define GICH_V2_LR_HW_MASK 0x1
> -#define GICH_V2_LR_GRP_SHIFT   30
> -#define GICH_V2_LR_GRP_MASK0x1
> -#define GICH_V2_LR_MAINTENANCE_IRQ (1<<19)
> -#define GICH_V2_LR_GRP1(1<<30)
> -#define GICH_V2_LR_HW  (1<<31)
> -#define GICH_V2_LR_CPUID_SHIFT 9
> -#define GICH_V2_VTR_NRLRGS 0x3f
> -
> -#define GICH_V2_VMCR_PRIORITY_MASK   0x1f
> -#define GICH_V2_VMCR_PRIORITY_SHIFT  27
> -
> -/* Global state */
> -static struct {

Re: [Xen-devel] [PATCH 1/4] libxl: Use libxl_strdup instead of strdup on libxl_version_info

[Ian Campbell]

On Mon, 2016-02-01 at 12:13 +, Wei Liu wrote:
> On Tue, Jan 26, 2016 at 04:30:57PM -0500, Konrad Rzeszutek Wilk wrote:
> > The change is simple replace of raw strdup with a libxl variant.
> > The benefit of that is the libxl variant has the extra
> > behaviour of abort-on-alloc-fail - and will improve error handling.
> > 
> > libxl_version_info is a bit odd - it is a public function and as
> > libxl.h
> > mentions - the callers of libxl_ public function needs to call the
> > appropiate
> > _dispose() function.
> > 
> > "However libxl_get_version_info() is special and returns a cached
> > result from the ctx which cannot and should not be freed (as evidenced
> > by it returning a const struct). This data is freed in libxl_ctx_free()
> > by calling libxl_version_info_dispose(). This is why none of the
> > callers
> > remember to free -- they shouldn't be doing so." (Ian Campbell)
> > 
> > So the patch makes sure to use the NOGC.
> > 
> > Suggested-by: Wei Liu 
> > Signed-off-by: Konrad Rzeszutek Wilk 
> 
> Acked-by: Wei Liu 

Applied.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

[Haozhong Zhang]

On 02/03/16 02:18, Jan Beulich wrote:
> >>> On 03.02.16 at 09:28,  wrote:
> > On 02/02/16 14:15, Konrad Rzeszutek Wilk wrote:
> >> > 3.1 Guest clwb/clflushopt/pcommit Enabling
> >> > 
> >> >  The instruction enabling is simple and we do the same work as in 
> >> > KVM/QEMU.
> >> >  - All three instructions are exposed to guest via guest cpuid.
> >> >  - L1 guest pcommit is never intercepted by Xen.
> >> 
> >> I wish there was some watermarks like the PLE has.
> >> 
> >> My fear is that an unfriendly guest can issue sfence all day long
> >> flushing out other guests MMC queue (the writes followed by pcommits).
> >> Which means that an guest may have degraded performance as their
> >> memory writes are being flushed out immediately as if they were
> >> being written to UC instead of WB memory. 
> > 
> > pcommit takes no parameter and it seems hard to solve this problem
> > from hardware for now. And the current VMX does not provide mechanism
> > to limit the commit rate of pcommit like PLE for pause.
> > 
> >> In other words - the NVDIMM resource does not provide any resource
> >> isolation. However this may not be any different than what we had
> >> nowadays with CPU caches.
> >>
> > 
> > Does Xen have any mechanism to isolate multiple guests' operations on
> > CPU caches?
> 
> No. All it does is disallow wbinvd for guests not controlling any
> actual hardware. Perhaps pcommit should at least be limited in
> a similar way?
>

But pcommit is a must that makes writes be persistent on pmem. I'll
look at how guest wbinvd is limited in Xen. Any functions suggested,
vmx_wbinvd_intercept()?

Thanks,
Haozhong

> >> >  - L1 hypervisor is allowed to intercept L2 guest pcommit.
> >> 
> >> clwb?
> > 
> > VMX is not capable to intercept clwb. Any reason to intercept it?
> 
> I don't think so - otherwise normal memory writes might also need
> intercepting. Bus bandwidth simply is shared (and CLWB operates
> on a guest virtual address, so can only cause bus traffic for cache
> lines the guest has managed to dirty).
> 
> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter max_wp_ram_ranges.

[Paul Durrant]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 03 February 2016 08:33
> To: Zhang Yu
> Cc: Andrew Cooper; Ian Campbell; Paul Durrant; Wei Liu; Ian Jackson; Stefano
> Stabellini; Kevin Tian; zhiyuan...@intel.com; xen-devel@lists.xen.org; Keir
> (Xen.org)
> Subject: Re: [Xen-devel] [PATCH v3 3/3] tools: introduce parameter
> max_wp_ram_ranges.
> 
> >>> On 03.02.16 at 08:10,  wrote:
> > On 2/2/2016 11:21 PM, Jan Beulich wrote:
> > On 02.02.16 at 16:00,  wrote:
> >>> The limit of 4G is to avoid the data missing from uint64 to uint32
> >>> assignment. And I can accept the 8K limit for XenGT in practice.
> >>> After all, it is vGPU page tables we are trying to trap and emulate,
> >>> not normal page frames.
> >>>
> >>> And I guess the reason that one domain exhausting Xen's memory can
> >>> affect another domain is because rangeset uses Xen heap, instead of
> the
> >>> per-domain memory. So what about we use a 8K limit by now for XenGT,
> >>> and in the future, if a per-domain memory allocation solution for
> >>> rangeset is ready, we do need to limit the rangeset size. Does this
> >>> sounds more acceptable?
> >>
> >> The lower the limit the better (but no matter how low the limit
> >> it won't make this a pretty thing). Anyway I'd still like to wait
> >> for what Ian may further say on this.
> >>
> > Hi Jan, I just had a discussion with my colleague. We believe 8K could
> > be the biggest limit for the write-protected ram ranges. If in the
> > future, number of vGPU page tables exceeds this limit, we will modify
> > our back-end device model to find a trade-off method, instead of
> > extending this limit. If you can accept this value as the upper bound
> > of rangeset, maybe we do not need to add any tool stack parameters, but
> > define a MAX_NR_WR_RAM_RANGES for the write-protected ram
> rangesset. As
> > to other rangesets, we keep their limit as 256. Does this sounds OK? :)
> 
> I'm getting the impression that we're moving in circles. A blanket
> limit above the 256 one for all domains is _not_ going to be
> acceptable; going to 8k will still need host admin consent. With
> your rangeset performance improvement patch, each range is
> going to be tracked by a 40 byte structure (up from 32), which
> already means an overhead increase for all the other ranges. 8k
> of wp ranges implies an overhead beyond 448k (including the
> xmalloc() overhead), which is not _that_ much, but also not
> negligible.
> 

... which means we are still going to need a toolstack parameter to set the 
limit. We already have a parameter for VRAM size so is having a parameter for 
max. GTT shadow ranges such a bad thing? Is the fact that the memory comes from 
xenheap rather than domheap the real problem?

  Paul

> Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Xen 4.7 Development Update

[Olaf Hering]

On Mon, Feb 01, Wei Liu wrote:

> == Toolstack == 

> *  Libxl PVSCSI support
>   -  Olaf Hering


I will send a new variant later this week. Getting pvscsi into 4.7 will
most likely require much of your time for review, I hope you guys find
the time and energy to do that.

Olaf

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2] arm: p2m.c bug-fix: hypervisor hang on __p2m_get_mem_access

[Corneliu ZUZU]

On 2/3/2016 1:52 PM, Ian Campbell wrote:

On Wed, 2016-02-03 at 13:37 +0200, Corneliu ZUZU wrote:

I just now applied a previous v2 which was already in my queue. Was this
just an accidental resend of v2 or is there some important change and this
is really a v3?


When __p2m_get_mem_access gets called, the p2m lock is already taken
by either get_page_from_gva or p2m_get_mem_access.
Possible code paths:
1)  -> get_page_from_gva
-> p2m_mem_access_check_and_get_page
-> __p2m_get_mem_access
2)  -> p2m_get_mem_access
-> __p2m_get_mem_access

In both cases if __p2m_get_mem_access subsequently gets to
call p2m_lookup (happens if !radix_tree_lookup(...)), a hypervisor
hang will occur, since p2m_lookup also spin-locks on the p2m lock.

This bug-fix simply replaces the p2m_lookup call from
__p2m_get_mem_access
with a call to __p2m_lookup and also adds an ASSERT to ensure that the
p2m lock
is already taken upon __p2m_get_mem_access entry.

Signed-off-by: Corneliu ZUZU 

---
Changed since v1:
   * added p2m-lock ASSERT
---
  xen/arch/arm/p2m.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 2190908..e8e6db4 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -468,6 +468,8 @@ static int __p2m_get_mem_access(struct domain *d,
gfn_t gfn,
  #undef ACCESS
  };
  
+ASSERT(spin_is_locked(>lock));

+
  /* If no setting was ever set, just return rwx. */
  if ( !p2m->mem_access_enabled )
  {
@@ -490,7 +492,7 @@ static int __p2m_get_mem_access(struct domain *d,
gfn_t gfn,
   * No setting was found in the Radix tree. Check if the
   * entry exists in the page-tables.
   */
-paddr_t maddr = p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
+paddr_t maddr = __p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
  if ( INVALID_PADDR == maddr )
  return -ESRCH;
  

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
No, sorry, this is just a duplicate of the 1st v2, I thought the first 
one was not sent properly (after waiting a few days and noticing

I was no longer finding it on the web).
Ignore this one. And thanks.

Corneliu.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 0/2] x86: also use alternative asm on xsave side

[Jan Beulich]

1: support 2- and 3-way alternatives
2: xstate: also use alternative asm on xsave side

Signed-off-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH OSSTEST 5/5 v3] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Campbell]

By looping over @rows looking for buildjobs runvars and adding those
jobs to the output until nothing changes.

The output is resorted by runvar name which is the desired default
behaviour. As usual can be piped to sort(1) to sort by flight+job.

Signed-off-by: Ian Campbell 
---
v2:
 - Use $jobcond,@jobconfparams to avoid SQL injection
 - Only recurse if the option was given
 - Drop synth from ORDER BY
 - Use a Schwatzian transform for the sort, at the same time allowing
   retention of the sorting of synth runvars last.
v3:
 - Fixup detection of already handle jobs to cope properly with jobs
   with now rows.
 - Iterate over @rows using an index var
---
 mg-show-flight-runvars | 48 +---
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index f96539f..c847456 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -46,19 +46,21 @@ for (;;) {
 
 die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 
-
 our @cols = qw(job name val);
 our @rows;
+our %jobs;
+
+sub collect ($;$@) {
+my ($flight,$jobcond,@jobcondparams) = @_;
 
-sub collect ($) {
-my ($flight) = @_;
+$jobcond //= "TRUE";
 
 $flight =~ m/^\d+/ or $flight = "'$flight'";
-my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
+my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond AND 
$jobcond";
 
 my $q = $dbh_tests->prepare
-   ("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, 
job");
-$q->execute();
+   ("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY name, job");
+$q->execute(@jobcondparams);
 
 while (my (@row) = $q->fetchrow_array()) {
my $synth = shift @row;
@@ -70,6 +72,34 @@ sub collect ($) {
 
 collect($ARGV[0]);
 
+if ($recurse) {
+# collect() appends to @rows, avoiding the need to recurse
+# there. However this means we can't use foreach (since that is
+# not guaranteed to work if the array is mutated under its feet),
+# instead use an index.
+for ( my $rowidx = 0; $rowidx < @rows ; $rowidx++ ) {
+   my $row = $rows[$rowidx];
+
+   next unless $row->[1] =~ m/^(?:.*_)?([^_]*)buildjob$/;
+
+   # parse this flight and job, which must be in $flight.$job
+   # form if $recurse is true (see collect())
+   my ($tflight, $tjob) = flight_otherjob(undef, $row->[0]);
+   die "$row->[1]" unless $tflight;
+
+   # parse the buildjob reference and recurse. might be a job in
+   # this flight, in which case we still recurse since it might
+   # be a chain from a non-top-level job which hasn't been
+   # included yet. %jobs will prevent us from duplicating or
+   # infinite loops.
+   my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
+
+   next if $jobs{$oflight,$ojob}++;
+
+   collect($oflight, "job = ?", $ojob);
+}
+}
+
 our @colws;
 sub max ($$) { $_[$_[0] < $_[1]] }
 foreach my $row (@rows) {
@@ -77,7 +107,11 @@ foreach my $row (@rows) {
 }
 $colws[1] += length $synthsufx;
 
-foreach my $row (@rows) {
+# Sort by runvar name, then (flight+)job, synth runvars come last.
+foreach my $row (map { $_->[0] }
+sort { $a->[1] cmp $b->[1] }
+map { [ $_, ($_->[1] =~ m/~$/)." $_->[1] $_->[0]" ] }
+@rows) {
 printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
 or die $!;
 }
-- 
2.6.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Error booting Xen

[Harmandeep Kaur]

On Wed, Feb 3, 2016 at 6:25 PM, Dario Faggioli
 wrote:
> On Wed, 2016-02-03 at 17:05 +0530, Harmandeep Kaur wrote:
>> On Wed, Feb 3, 2016 at 1:53 PM, Dario Faggioli
>>  wrote:
>> >
>> Maybe I failed to shutdown a guest which was showing up
>> on next boot. But there are no auto-starting guests.
>> Following link goes to latest serial log
>> http://paste2.org/5PDz9bP1
>>
> No, sorry, I probably am not getting. Are you saying that, just booting
> Xen and *not* doing anything else --either from SSH, from the keyboard
> of that box, from serial connection... anything at all-- and without
> any guest configured to auto start itself, results in these lines to
> appear on your serial console?
>
> tbox login: (d1) mapping kernel into physical memory
> (d1) about to get started...
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c081 from 
> 0xe023e008 to 0x00230010.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c082 from 
> 0x82d0bfffe080 to 0x817ef990.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c083 from 
> 0x82d0bfffe0a0 to 0x817f1f60.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0174 from 
> 0xe008 to 0x0010.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0175 from 
> 0x83026d40ffc0 to 0x.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0176 from 
> 0x82d08023eaf0 to 0x817f1d60.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c084 from 
> 0x00074700 to 0x00047700.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c081 from 
> 0xe023e008 to 0x00230010.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c082 from 
> 0x82d0b000 to 0x817ef990.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c083 from 
> 0x82d0b020 to 0x817f1f60.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0174 from 
> 0xe008 to 0x0010.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0175 from 
> 0x8300866fffc0 to 0x.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0176 from 
> 0x82d08023eaf0 to 0x817f1d60.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c084 from 
> 0x00074700 to 0x00047700.
>
> Because this is what you just said above, and that's... well... just
> impossible?!?! :-O
>
> What's the output then, if you login (either via serial, ssh or regular
> keyboad+monitor) and run `xl list'

$ sudo xl list
NameID   Mem VCPUsStateTime(s)
Domain-0 0  4096 4 r-  37.6

> and `xl vcpu-list' as the very first
> thing?

$ sudo xl vcpu-list
NameID  VCPU   CPU State   Time(s)
Affinity (Hard / Soft)
Domain-0 0 03   r--  13.3  all / all
Domain-0 0 10   -b-   7.0  all / all
Domain-0 0 21   -b-   5.4  all / all
Domain-0 0 32   r--  13.5  all / all


> What's inside `ls -l /var/log/xen/' (or `ls -l
> /var/local/log/xen')?

$ ls -l /var/log/xen/
total 48
-rw-r--r-- 1 root adm  28 Jan 23 00:14 xen-hotplug.log
-rw-r--r-- 1 root adm  91 Feb  3 17:14 xl-ubuntu.pvlinux.log
-rw-r--r-- 1 root adm  91 Jan 29 00:54 xl-ubuntu.pvlinux.log.1
-rw-r--r-- 1 root adm 144 Jan 26 23:50 xl-ubuntu.pvlinux.log.10
-rw-r--r-- 1 root adm  91 Jan 29 00:41 xl-ubuntu.pvlinux.log.2
-rw-r--r-- 1 root adm  91 Jan 29 00:32 xl-ubuntu.pvlinux.log.3
-rw-r--r-- 1 root adm  91 Jan 29 00:24 xl-ubuntu.pvlinux.log.4
-rw-r--r-- 1 root adm  91 Jan 28 22:22 xl-ubuntu.pvlinux.log.5
-rw-r--r-- 1 root adm  62 Jan 27 19:09 xl-ubuntu.pvlinux.log.6
-rw-r--r-- 1 root adm  91 Jan 27 19:07 xl-ubuntu.pvlinux.log.7
-rw-r--r-- 1 root adm 222 Jan 27 18:41 xl-ubuntu.pvlinux.log.8
-rw-r--r-- 1 root adm  62 Jan 27 15:03 xl-ubuntu.pvlinux.log.9

>
>> and then I created a guest with
>> config (http://paste2.org/azvj25Hg) and
>> http://paste2.org/cgKna0j1 log was presented at terminal.
>>
> Well, it looks like the guest boot did not indeed go well, but that is
> probably because of other thing... can you try uncommenting the line
> 'extra = "root=/dev/xvda1"' from the config?

Trying this suggestion.

>
> Regards,
> Dario
> --
> <> (Raistlin Majere)
> -
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)
>

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/2] x86/xsave: use alternative asm on xsave side.

[Jan Beulich]

>>> On 02.02.16 at 08:11,  wrote:
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -248,24 +248,26 @@ void xsave(struct vcpu *v, uint64_t mask)
>  uint32_t hmask = mask >> 32;
>  uint32_t lmask = mask;
>  int word_size = mask & XSTATE_FP ? (cpu_has_fpu_sel ? 8 : 0) : -1;
> +#define XSAVE(pfx) \
> +alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", \
> + ".byte " pfx "0x0f,0xae,0x37\n", \
> + X86_FEATURE_XSAVEOPT, \
> + ".byte " pfx "0x0f,0xc7,0x27\n", \
> + X86_FEATURE_XSAVEC, \
> + ".byte " pfx "0x0f,0xc7,0x37\n", \
> + X86_FEATURE_XSAVES, \
> + "=m" (*ptr), \
> + "a" (lmask), "d" (hmask), "D" (ptr))
>  
>  if ( word_size <= 0 || !is_pv_32bit_vcpu(v) )
>  {
>  typeof(ptr->fpu_sse.fip.sel) fcs = ptr->fpu_sse.fip.sel;
>  typeof(ptr->fpu_sse.fdp.sel) fds = ptr->fpu_sse.fdp.sel;
>  
> -if ( cpu_has_xsaves )
> -asm volatile ( ".byte 0x48,0x0f,0xc7,0x2f"
> -   : "=m" (*ptr)
> -   : "a" (lmask), "d" (hmask), "D" (ptr) );
> -else if ( cpu_has_xsavec )
> -asm volatile ( ".byte 0x48,0x0f,0xc7,0x27"
> -   : "=m" (*ptr)
> -   : "a" (lmask), "d" (hmask), "D" (ptr) );
> -else if ( cpu_has_xsaveopt )
> +if ( cpu_has_xsaveopt || cpu_has_xsaves )
>  {
>  /*
> - * xsaveopt may not write the FPU portion even when the 
> respective
> + * xsaveopt/xsaves may not write the FPU portion even when the 
> respective

Apart from this line now being too long and hence the entire
comment needing re-formatting
Reviewed-by: Jan Beulich 

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 2/4] libxc/xc_domain_resume: Update comment.

[Ian Campbell]

On Mon, 2016-02-01 at 12:14 +, Wei Liu wrote:
> On Tue, Jan 26, 2016 at 04:30:58PM -0500, Konrad Rzeszutek Wilk wrote:
> > To hopefully clarify what it meant. Also point out that mechanism
> > by which the return 1 value is done is via an intimate knowledge of the
> > hypercall ABI (i.e. which register - eax - is the return value).
> > 
> > Signed-off-by: Konrad Rzeszutek Wilk 
> 
> As far as I can tell all concerns in previous versions have been
> addressed.
> 
> Acked-by: Wei Liu 

Applied.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Error booting Xen

[Andrew Cooper]

On 03/02/16 12:55, Dario Faggioli wrote:
> On Wed, 2016-02-03 at 17:05 +0530, Harmandeep Kaur wrote:
>> On Wed, Feb 3, 2016 at 1:53 PM, Dario Faggioli
>>  wrote:
>>>  
>> Maybe I failed to shutdown a guest which was showing up
>> on next boot. But there are no auto-starting guests.
>> Following link goes to latest serial log
>> http://paste2.org/5PDz9bP1 
>>
> No, sorry, I probably am not getting. Are you saying that, just booting
> Xen and *not* doing anything else --either from SSH, from the keyboard
> of that box, from serial connection... anything at all-- and without
> any guest configured to auto start itself, results in these lines to
> appear on your serial console?
>
> tbox login: (d1) mapping kernel into physical memory
> (d1) about to get started...
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c081 from 
> 0xe023e008 to 0x00230010.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c082 from 
> 0x82d0bfffe080 to 0x817ef990.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c083 from 
> 0x82d0bfffe0a0 to 0x817f1f60.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0174 from 
> 0xe008 to 0x0010.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0175 from 
> 0x83026d40ffc0 to 0x.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR 0176 from 
> 0x82d08023eaf0 to 0x817f1d60.
> (XEN) traps.c:2684:d1v0 Domain attempted WRMSR c084 from 
> 0x00074700 to 0x00047700.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c081 from 
> 0xe023e008 to 0x00230010.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c082 from 
> 0x82d0b000 to 0x817ef990.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c083 from 
> 0x82d0b020 to 0x817f1f60.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0174 from 
> 0xe008 to 0x0010.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0175 from 
> 0x8300866fffc0 to 0x.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR 0176 from 
> 0x82d08023eaf0 to 0x817f1d60.
> (XEN) traps.c:2684:d1v1 Domain attempted WRMSR c084 from 
> 0x00074700 to 0x00047700.
>
> Because this is what you just said above, and that's... well... just
> impossible?!?! :-O

This is a Linux PV trying to set up the SYSCALL/SYSENTER MSRs when it
shouldn't.  There is a fix upstream, but this specifically is harmless
noise.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] libxc: fix uninitialised usage of rc in meminit_hvm

[Roger Pau Monne]

From: Roger Pau Monne 

Due to the HVMlite changes there's a chance that the value in rc is checked
without being initialised. Fix this by initialising it to 0 prior to the
while loop. Also add a specific error check to a previous populate_physmap
call, this prevents us from overwriting this error.

Signed-off-by: Roger Pau Monné 
Reported-by: Olaf Hering 
---
Cc: Ian Jackson 
Cc: Ian Campbell 
Cc: Wei Liu 
---
Changes since v1:
 - Only set rc = 0 prior to the while loop.
 - Add an error check to the previous populate_physmap call.
---
 tools/libxc/xc_dom_x86.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index ef474a8..2a3f64b 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -1412,8 +1412,15 @@ static int meminit_hvm(struct xc_dom_image *dom)
  * ensure that we can be preempted and hence dom0 remains responsive.
  */
 if ( dom->device_model )
+{
 rc = xc_domain_populate_physmap_exact(
 xch, domid, 0xa0, 0, memflags, >p2m_host[0x00]);
+if ( rc != 0 )
+{
+DOMPRINTF("Could not populate low memory (< 0xA0).\n");
+goto error_out;
+}
+}
 
 stat_normal_pages = 0;
 for ( vmemid = 0; vmemid < nr_vmemranges; vmemid++ )
@@ -1440,6 +1447,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
 else
 cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
 
+rc = 0;
 while ( (rc == 0) && (end_pages > cur_pages) )
 {
 /* Clip count to maximum 1GB extent. */
-- 
2.5.4 (Apple Git-61)


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v7 5/9] virtio_ring: Support DMA APIs

[Michael S. Tsirkin]

On Tue, Feb 02, 2016 at 09:46:36PM -0800, Andy Lutomirski wrote:
> virtio_ring currently sends the device (usually a hypervisor)
> physical addresses of its I/O buffers.  This is okay when DMA
> addresses and physical addresses are the same thing, but this isn't
> always the case.  For example, this never works on Xen guests, and
> it is likely to fail if a physical "virtio" device ever ends up
> behind an IOMMU or swiotlb.
> 
> The immediate use case for me is to enable virtio on Xen guests.
> For that to work, we need vring to support DMA address translation
> as well as a corresponding change to virtio_pci or to another
> driver.
> 
> Signed-off-by: Andy Lutomirski 
> ---
>  drivers/virtio/Kconfig   |   2 +-
>  drivers/virtio/virtio_ring.c | 200 
> ---
>  tools/virtio/linux/dma-mapping.h |  17 
>  3 files changed, 183 insertions(+), 36 deletions(-)
>  create mode 100644 tools/virtio/linux/dma-mapping.h
> 
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index cab9f3f63a38..77590320d44c 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -60,7 +60,7 @@ config VIRTIO_INPUT
>  
>   config VIRTIO_MMIO
>   tristate "Platform bus driver for memory mapped virtio devices"
> - depends on HAS_IOMEM
> + depends on HAS_IOMEM && HAS_DMA
>   select VIRTIO
>   ---help---
>This drivers provides support for memory mapped virtio

What's this chunk doing here btw? Should be part of the mmio patch?

> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index ab0be6c084f6..9abc008ff7ea 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #ifdef DEBUG
>  /* For development, we want to crash whenever the ring is screwed. */
> @@ -54,6 +55,11 @@
>  #define END_USE(vq)
>  #endif
>  
> +struct vring_desc_state {
> + void *data; /* Data for callback. */
> + struct vring_desc *indir_desc;  /* Indirect descriptor, if any. */
> +};
> +
>  struct vring_virtqueue {
>   struct virtqueue vq;
>  
> @@ -98,8 +104,8 @@ struct vring_virtqueue {
>   ktime_t last_add_time;
>  #endif
>  
> - /* Tokens for callbacks. */
> - void *data[];
> + /* Per-descriptor state. */
> + struct vring_desc_state desc_state[];
>  };
>  
>  #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq)
> @@ -128,6 +134,79 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
>   return false;
>  }
>  
> +/*
> + * The DMA ops on various arches are rather gnarly right now, and
> + * making all of the arch DMA ops work on the vring device itself
> + * is a mess.  For now, we use the parent device for DMA ops.
> + */
> +struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> +{
> + return vq->vq.vdev->dev.parent;
> +}
> +
> +/* Map one sg entry. */
> +static dma_addr_t vring_map_one_sg(const struct vring_virtqueue *vq,
> +struct scatterlist *sg,
> +enum dma_data_direction direction)
> +{
> + if (!vring_use_dma_api(vq->vq.vdev))
> + return (dma_addr_t)sg_phys(sg);
> +
> + /*
> +  * We can't use dma_map_sg, because we don't use scatterlists in
> +  * the way it expects (we don't guarantee that the scatterlist
> +  * will exist for the lifetime of the mapping).
> +  */
> + return dma_map_page(vring_dma_dev(vq),
> + sg_page(sg), sg->offset, sg->length,
> + direction);
> +}
> +
> +static dma_addr_t vring_map_single(const struct vring_virtqueue *vq,
> +void *cpu_addr, size_t size,
> +enum dma_data_direction direction)
> +{
> + if (!vring_use_dma_api(vq->vq.vdev))
> + return (dma_addr_t)virt_to_phys(cpu_addr);
> +
> + return dma_map_single(vring_dma_dev(vq),
> +   cpu_addr, size, direction);
> +}
> +
> +static void vring_unmap_one(const struct vring_virtqueue *vq,
> + struct vring_desc *desc)
> +{
> + u16 flags;
> +
> + if (!vring_use_dma_api(vq->vq.vdev))
> + return;
> +
> + flags = virtio16_to_cpu(vq->vq.vdev, desc->flags);
> +
> + if (flags & VRING_DESC_F_INDIRECT) {
> + dma_unmap_single(vring_dma_dev(vq),
> +  virtio64_to_cpu(vq->vq.vdev, desc->addr),
> +  virtio32_to_cpu(vq->vq.vdev, desc->len),
> +  (flags & VRING_DESC_F_WRITE) ?
> +  DMA_FROM_DEVICE : DMA_TO_DEVICE);
> + } else {
> + dma_unmap_page(vring_dma_dev(vq),
> +virtio64_to_cpu(vq->vq.vdev, desc->addr),
> +virtio32_to_cpu(vq->vq.vdev, desc->len),
> +   

Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

[Haozhong Zhang]

On 02/02/16 14:15, Konrad Rzeszutek Wilk wrote:
> > 3. Design of vNVDIMM in Xen
> 
> Thank you for this design!
> 
> > 
> >  Similarly to that in KVM/QEMU, enabling vNVDIMM in Xen is composed of
> >  three parts:
> >  (1) Guest clwb/clflushopt/pcommit enabling,
> >  (2) Memory mapping, and
> >  (3) Guest ACPI emulation.
> 
> 
> .. MCE? and vMCE?
>

Specifications on my hand seem not mention much about MCE for NVDIMM,
but I remember that NVDIMM driver in Linux kernel does have MCE
code. I'll have a look at that code and add this part later.

> > 
> >  The rest of this section present the design of each part
> >  respectively. The basic design principle to reuse existing code in
> >  Linux NVDIMM driver and QEMU as much as possible. As recent
> >  discussions in the both Xen and QEMU mailing lists for the v1 patch
> >  series, alternative designs are also listed below.
> > 
> > 
> > 3.1 Guest clwb/clflushopt/pcommit Enabling
> > 
> >  The instruction enabling is simple and we do the same work as in KVM/QEMU.
> >  - All three instructions are exposed to guest via guest cpuid.
> >  - L1 guest pcommit is never intercepted by Xen.
> 
> I wish there was some watermarks like the PLE has.
> 
> My fear is that an unfriendly guest can issue sfence all day long
> flushing out other guests MMC queue (the writes followed by pcommits).
> Which means that an guest may have degraded performance as their
> memory writes are being flushed out immediately as if they were
> being written to UC instead of WB memory. 
>

pcommit takes no parameter and it seems hard to solve this problem
from hardware for now. And the current VMX does not provide mechanism
to limit the commit rate of pcommit like PLE for pause.

> In other words - the NVDIMM resource does not provide any resource
> isolation. However this may not be any different than what we had
> nowadays with CPU caches.
>

Does Xen have any mechanism to isolate multiple guests' operations on
CPU caches?

> 
> >  - L1 hypervisor is allowed to intercept L2 guest pcommit.
> 
> clwb?
>

VMX is not capable to intercept clwb. Any reason to intercept it?

> > 
> > 
> > 3.2 Address Mapping
> > 
> > 3.2.1 My Design
> > 
> >  The overview of this design is shown in the following figure.
> > 
> >  Dom0 |   DomU
> >   |
> >   |
> >  QEMU |
> >  +...++...+-+ |
> >   VA |   | Label Storage Area |   | buf | |
> >  +...++...+-+ |
> >  ^^ ^ |
> >  || | |
> >  V| | |
> >  +---+   +---+mmap(2) |
> >  | vACPI |   | v_DSM || | |+++
> >  +---+   +---+| | |   SPA  || /dev/pmem0 |
> >  ^   ^ +--+ | |+++
> >  |---|-||--   | ^^
> >  |   | || | ||
> >  |+--+ +~-~-+|
> >  |||| |
> > XEN_DOMCTL_memory_mapping
> >  |||+-~--+
> >  |||| |
> >  ||   +++ |
> >  Linux   ||   SPA || /dev/pmem0 | | +--+   +--+
> >  ||   +++ | | ACPI |   | _DSM |
> >  ||   ^   | +--+   +--+
> >  ||   |   | |  |
> >  ||   Dom0 Driver |   hvmloader/xl |
> >  
> > ||---|-|--|---
> >  |+---~-~--+
> >  Xen || |
> >  +~-+
> >  
> > -|
> >   ++
> >|
> > +-+
> >  HW |NVDIMM   |
> > +-+
> > 
> > 
> >  This design treats host NVDIMM devices as ordinary MMIO devices:
> 
> Nice.
> 
> But it also means you need Xen to 'share' the ranges of an MMIO device.
> 
> That is you may need dom0 _DSM method to access certain ranges
> (the AML code may need to poke there) - and the guest may want to access
> those as well.
>

Currently, we 

Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM support for Xen

[Jan Beulich]

>>> On 03.02.16 at 09:28,  wrote:
> On 02/02/16 14:15, Konrad Rzeszutek Wilk wrote:
>> > 3.1 Guest clwb/clflushopt/pcommit Enabling
>> > 
>> >  The instruction enabling is simple and we do the same work as in KVM/QEMU.
>> >  - All three instructions are exposed to guest via guest cpuid.
>> >  - L1 guest pcommit is never intercepted by Xen.
>> 
>> I wish there was some watermarks like the PLE has.
>> 
>> My fear is that an unfriendly guest can issue sfence all day long
>> flushing out other guests MMC queue (the writes followed by pcommits).
>> Which means that an guest may have degraded performance as their
>> memory writes are being flushed out immediately as if they were
>> being written to UC instead of WB memory. 
> 
> pcommit takes no parameter and it seems hard to solve this problem
> from hardware for now. And the current VMX does not provide mechanism
> to limit the commit rate of pcommit like PLE for pause.
> 
>> In other words - the NVDIMM resource does not provide any resource
>> isolation. However this may not be any different than what we had
>> nowadays with CPU caches.
>>
> 
> Does Xen have any mechanism to isolate multiple guests' operations on
> CPU caches?

No. All it does is disallow wbinvd for guests not controlling any
actual hardware. Perhaps pcommit should at least be limited in
a similar way?

>> >  - L1 hypervisor is allowed to intercept L2 guest pcommit.
>> 
>> clwb?
> 
> VMX is not capable to intercept clwb. Any reason to intercept it?

I don't think so - otherwise normal memory writes might also need
intercepting. Bus bandwidth simply is shared (and CLWB operates
on a guest virtual address, so can only cause bus traffic for cache
lines the guest has managed to dirty).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] tools: xenconsole: cleanup when clock_gettime fails.

[Ian Campbell]

All other error paths in the infinite loop in handle_io use break, so
as to free resources.

CID: 1351226

Signed-off-by: Ian Campbell 
---
 tools/console/daemon/io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index e2e7a6b..34666c4 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -1053,7 +1053,7 @@ void handle_io(void)
 POLLIN|POLLPRI);
 
if (clock_gettime(CLOCK_MONOTONIC, ) < 0)
-   return;
+   break;
now = ((long long)ts.tv_sec * 1000) + (ts.tv_nsec / 100);
 
/* Re-calculate any event counter allowances & unblock
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Roger Pau Monné]

El 3/2/16 a les 11:30, Ian Campbell ha escrit:
> On Tue, 2016-02-02 at 12:37 +, Wei Liu wrote:
>> On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
>>> From: Roger Pau Monne 
>>>
>>> Due to the HVMlite changes there's a chance that the value in rc is
>>> checked
>>> without being initialised. Fix this by initialising it to 0.
>>>
>>> Signed-off-by: Roger Pau Monné 
>>> Reported-by: Olaf Hering 
>>
>> Acked-by: Wei Liu 
> 
> This is CID 1351229, I think?

Looks like, according the the description below.

> 
> ** CID 1351229:  Uninitialized variables  (UNINIT)
>> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
>>  
>>  
>> 
>> *** CID 1351229:  Uninitialized variables  (UNINIT)
>> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
>> 1437 cur_pages = 0xc0;
>> 1438 stat_normal_pages += 0xc0;
>> 1439 }
>> 1440 else
>> 1441 cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
>> 1442 
>  CID 1351229:  Uninitialized variables  (UNINIT)
>  Using uninitialized value "rc".
>> 1443 while ( (rc == 0) && (end_pages > cur_pages) )
>> 1444 {
>> 1445 /* Clip count to maximum 1GB extent. */
>> 1446 unsigned long count = end_pages - cur_pages;
>> 1447 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
>> 1448
> 
> Note that this while loop ends with:
> if ( rc != 0 )
> break;
> and there are no continue statements.
> 
> Therefore I wonder if we would be better off removing the rc == 0 part of
> the loop condition?

We could, but I think we would still have the same issue with the "if (
rc != 0 )" at the end of the loop, AFAICT rc is never unconditionally
set inside of the loop itself, so gcc and coverity would still complain
about uninitialized usage.

> The issue with this patch is the usual one that it will hide other
> unintentional uses of rc before it is set to a good value.
> 
> This issue was exposed by a prior "rc = xc_domain_populate_physmap_exact"
> becoming conditional on device_model. What is also concerning is the lack
> of error checking on that call -- is it really ok to just barrel on under
> these circumstance?

Hm, I guess we then rely on the rc == 0 at the start of the while loop
in order to bail out. IMHO the logic in this function is overly complicated.

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Ian Campbell]

On Wed, 2016-02-03 at 11:44 +0100, Roger Pau Monné wrote:
> El 3/2/16 a les 11:30, Ian Campbell ha escrit:
> > On Tue, 2016-02-02 at 12:37 +, Wei Liu wrote:
> > > On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> > > > From: Roger Pau Monne 
> > > > 
> > > > Due to the HVMlite changes there's a chance that the value in rc is
> > > > checked
> > > > without being initialised. Fix this by initialising it to 0.
> > > > 
> > > > Signed-off-by: Roger Pau Monné 
> > > > Reported-by: Olaf Hering 
> > > 
> > > Acked-by: Wei Liu 
> > 
> > This is CID 1351229, I think?
> 
> Looks like, according the the description below.
> 
> > 
> > ** CID 1351229:  Uninitialized variables  (UNINIT)
> > > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > >  
> > >  
> > > _
> > > ___
> > > *** CID 1351229:  Uninitialized variables  (UNINIT)
> > > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > > 1437 cur_pages = 0xc0;
> > > 1438 stat_normal_pages += 0xc0;
> > > 1439 }
> > > 1440 else
> > > 1441 cur_pages = vmemranges[vmemid].start >>
> > > PAGE_SHIFT;
> > > 1442 
> > > > > >  CID 1351229:  Uninitialized variables  (UNINIT)
> > > > > >  Using uninitialized value "rc".
> > > 1443 while ( (rc == 0) && (end_pages > cur_pages) )
> > > 1444 {
> > > 1445 /* Clip count to maximum 1GB extent. */
> > > 1446 unsigned long count = end_pages - cur_pages;
> > > 1447 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
> > > 1448
> > 
> > Note that this while loop ends with:
> > if ( rc != 0 )
> > break;
> > and there are no continue statements.
> > 
> > Therefore I wonder if we would be better off removing the rc == 0 part
> > of
> > the loop condition?
> 
> We could, but I think we would still have the same issue with the "if (
> rc != 0 )" at the end of the loop, AFAICT rc is never unconditionally
> set inside of the loop itself, so gcc and coverity would still complain
> about uninitialized usage.

Right, I was looking at the wrong loop as Wei pointed out.

I think "rc = 0" before the while might be a reasonable option.

> > The issue with this patch is the usual one that it will hide other
> > unintentional uses of rc before it is set to a good value.
> > 
> > This issue was exposed by a prior "rc =
> > xc_domain_populate_physmap_exact"
> > becoming conditional on device_model. What is also concerning is the
> > lack
> > of error checking on that call -- is it really ok to just barrel on
> > under
> > these circumstance?
> 
> Hm, I guess we then rely on the rc == 0 at the start of the while loop
> in order to bail out. IMHO the logic in this function is overly
> complicated.

Indeed, although we do some other (I suppose pointless) work first in that
case too.

Moving some of it into separate helpers would be a nice further cleanup.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Status of multiboot2 support on EFI?

[Wei Liu]

On Tue, Feb 02, 2016 at 01:02:56PM -0800, PGNet Dev wrote:
> Here http://wiki.xenproject.org/wiki/Xen_EFI#Xen_as_gz_binary refers to
> original discussion in 2013 (work has been deferred to Xen 4.6. )
> 
> and says "work has been deferred to Xen 4.6."
> 
> >I think that this should be Xen 4.8 target.
> 
> That's roughly another year+ from now for EFI support?
> 
> Per, http://wiki.xenproject.org/wiki/Xen_Roadmap, there's no 4.8 info at
> all.
> 
> But 8months plus 4.7 release =~ Feb 2017?
> 

We now have 6 months cycle.

The release after 4.7 will freeze in October and hopefully release in
December.

Wei.

> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v6 6/8] arm/gic-v3: Refactor gicv3_init into generic and dt specific parts

[Ian Campbell]

On Sat, 2016-01-30 at 17:03 +0800, Shannon Zhao wrote:
> 
> On 2016/1/28 18:27, Stefano Stabellini wrote:
> > On Thu, 28 Jan 2016, Shannon Zhao wrote:
> > > > From: Shannon Zhao 
> > > > 
> > > > Refactor gic-v3 related functions into dt and generic parts. This
> > > > will be
> > > > helpful when adding acpi support for gic-v3.
> > > > 
> > > > Signed-off-by: Shannon Zhao 
> > Reviewed-by: Stefano Stabellini 
> > 
> Thanks a lot!
> 
> Hi Ian, Would you please pick the last five patches(4-8)?

Done. I nearly missed the hidden v6 but Stefano pointed it out to me.

0f5f9d8 pl011: Refactor pl011 driver to dt and common initialization parts
57c5953 arm/uart: Rename dt-uart.c to arm-uart.c
b3aeac4 arm/gic-v3: Refactor gicv3_init into generic and dt specific parts
57ab13c arm/gic-v2: Refactor gicv2_init into generic and dt specific parts
eaf1de3 arm/smpboot: Move dt specific code in smp to seperate functions

There was one minor conflict with a MAINTAINERS change, but I trivially
resolved it.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V2] arm: p2m.c bug-fix: hypervisor hang on __p2m_get_mem_access

[Ian Campbell]

On Wed, 2016-02-03 at 13:54 +0200, Corneliu ZUZU wrote:
> On 2/3/2016 1:48 PM, Ian Campbell wrote:
> > On Wed, 2016-01-27 at 14:24 +0200, Corneliu ZUZU wrote:
> > > When __p2m_get_mem_access gets called, the p2m lock is already taken
> > > by either get_page_from_gva or p2m_get_mem_access.
> > > 
> > > Possible code paths:
> > > 1)-> get_page_from_gva
> > >   -> p2m_mem_access_check_and_get_page
> > >   -> __p2m_get_mem_access
> > > 2)-> p2m_get_mem_access
> > >   -> __p2m_get_mem_access
> > > 
> > > In both cases if __p2m_get_mem_access subsequently gets to
> > > call p2m_lookup (happens if !radix_tree_lookup(...)), a hypervisor
> > > hang will occur, since p2m_lookup also spin-locks on the p2m lock.
> > > 
> > > This bug-fix simply replaces the p2m_lookup call from
> > > __p2m_get_mem_access
> > > with a call to __p2m_lookup.
> > > 
> > > Following Ian's suggestion, we also add an ASSERT to ensure that
> > > the p2m lock is taken upon __p2m_get_mem_access entry.
> > > 
> > > Signed-off-by: Corneliu ZUZU 
> > Acked + applied, thanks.
> > 
> > 
> I thought this mail was not sent properly (didn't find it any longer on 
> the web (?)) and I resent it just earlier.
> I figured it must've been the fact that I forgot to put a "Changed since 
> v1" section & that I didn't include an
> "--in-reply-to" option. Apparently it was actually sent correctly.
> Sorry, ignore the last one (which contains a "Changed since v1" section).

OK, please check that what is currently in xen.git#staging is what you
think should be there.

Ian.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] missing lock in percpu_rwlock? (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Andrew Cooper]

On 03/02/16 12:21, Andrew Cooper wrote:
> On 03/02/16 11:00, Ian Campbell wrote:
>> On Wed, 2016-02-03 at 10:50 +, Andrew Cooper wrote:
>>> On 03/02/16 10:45, Ian Campbell wrote:
 On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> * CID 1351223:  Concurrent data access violations  (MISSING_LOCK)
> /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
 Coverity seems to think this is new in 41b0aa569adb..9937763265d,
 presumably due to 

 commit f9dd43dddc0a31a4343a58072935c1b5c0cbbee
 Author: Malcolm Crossley 
 Date:   Fri Jan 22 16:04:41 2016 +0100

 rwlock: add per-cpu reader-writer lock infrastructure
>>> Expected behaviour.  writer_activating is expected to only be written
>>> under lock, but read without lock.
>> I suppose this is something we should eventually model?
> Short of annotating the source code with Coverity comments (which has
> already been objected to), I don't see a way.
>
> This issue is Coverity (correctly) observing the behaviour of the
> function, and coming to the wrong conclusion.  The modelling file is
> used to correct the interpretation of the behaviour of the function.
>
>> Would you typically mark this as "False positive" or "Intentional"?
> I would err on the side of Intentional.
>
> The analysis of the issue was correct; that data was accessed both with
> and without the lock, and that this usually means a data race condition.
>
> In this case, it is a deliberate algorithm decision to have the data
> access like this.
>
>> I just marked a couple of libxl ones about taking ctx->lock (which is
>> recursive) twice as "False positive", but perhaps "Intentional" is the
>> correct designation there?
> There is an attempt to model this in the model file, but it clearly
> isn't taking.

(I meant to say as well)

This I would err in the side of false positive, with "modelling
required" as a reason.  The lock is a recursive lock and Coverity should
be able to spot this fact, but can't for some reason.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] missing lock in percpu_rwlock? (Was: Re: New Defects reported by Coverity Scan for XenProject)

[Ian Campbell]

On Wed, 2016-02-03 at 12:24 +, Andrew Cooper wrote:
> On 03/02/16 12:21, Andrew Cooper wrote:
> > On 03/02/16 11:00, Ian Campbell wrote:
> > > On Wed, 2016-02-03 at 10:50 +, Andrew Cooper wrote:
> > > > On 03/02/16 10:45, Ian Campbell wrote:
> > > > > On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
> > > > > > * CID 1351223:  Concurrent data access
> > > > > > violations  (MISSING_LOCK)
> > > > > > /xen/include/xen/spinlock.h: 362 in _percpu_write_unlock()
> > > > > Coverity seems to think this is new in 41b0aa569adb..9937763265d,
> > > > > presumably due to 
> > > > > 
> > > > > commit f9dd43dddc0a31a4343a58072935c1b5c0cbbee
> > > > > Author: Malcolm Crossley 
> > > > > Date:   Fri Jan 22 16:04:41 2016 +0100
> > > > > 
> > > > > rwlock: add per-cpu reader-writer lock infrastructure
> > > > Expected behaviour.  writer_activating is expected to only be
> > > > written
> > > > under lock, but read without lock.
> > > I suppose this is something we should eventually model?
> > Short of annotating the source code with Coverity comments (which has
> > already been objected to), I don't see a way.
> > 
> > This issue is Coverity (correctly) observing the behaviour of the
> > function, and coming to the wrong conclusion.  The modelling file is
> > used to correct the interpretation of the behaviour of the function.
> > 
> > > Would you typically mark this as "False positive" or "Intentional"?
> > I would err on the side of Intentional.
> > 
> > The analysis of the issue was correct; that data was accessed both with
> > and without the lock, and that this usually means a data race
> > condition.
> > 
> > In this case, it is a deliberate algorithm decision to have the data
> > access like this.

Done. I linked to your explanation in the comments.

> > 
> > > I just marked a couple of libxl ones about taking ctx->lock (which is
> > > recursive) twice as "False positive", but perhaps "Intentional" is
> > > the
> > > correct designation there?
> > There is an attempt to model this in the model file, but it clearly
> > isn't taking.
> 
> (I meant to say as well)
> 
> This I would err in the side of false positive, with "modelling
> required" as a reason.  The lock is a recursive lock and Coverity should
> be able to spot this fact, but can't for some reason.

Good idea, I'll update those.

Ian

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [linux-mingo-tip-master test] 80120: regressions - FAIL

[osstest service owner]

flight 80120 linux-mingo-tip-master real [real]
http://logs.test-lab.xenproject.org/osstest/logs/80120/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 60684
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 60684
 build-i386-pvops  5 kernel-build  fail REGR. vs. 60684
 build-amd64-pvops 5 kernel-build  fail REGR. vs. 60684

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-pvh-amd   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-credit2   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-xsm   1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-rtds  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-amd64-amd64-pair 1 build-check(1)   blocked  n/a
 test-amd64-amd64-pygrub   1 build-check(1)   blocked  n/a
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-amd64-i386-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-amd64-xl-qcow2 1 build-check(1)   blocked  n/a
 test-amd64-amd64-amd64-pvgrub  1 build-check(1)   blocked  n/a
 test-amd64-amd64-qemuu-nested-intel  1 build-check(1)  blocked n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-qemuu-nested-amd  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-amd64-amd64-libvirt-vhd  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64  1 build-check(1)blocked n/a
 test-amd64-amd64-xl-qemut-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  1 build-check(1)blocked n/a
 test-amd64-amd64-xl-qemuu-win7-amd64  1 build-check(1) blocked n/a
 test-amd64-amd64-xl-qemuu-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked 
n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-ovmf-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  1 build-check(1)  blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-winxpsp3  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-winxpsp3  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemut-winxpsp3  1 build-check(1)   blocked n/a
 test-amd64-i386-pair  1 

Re: [Xen-devel] [PATCH v2 02/11] xen/hvmlite: Bootstrap HVMlite guest

[Andrew Cooper]

On 03/02/16 18:55, Luis R. Rodriguez wrote:
> We add new hypervisor type to close the semantic gap for hypervisor types, and
> much like subarch enable also a subarch_data to let you pass and use your
> hvmlite_start_info. This would not only help with the semantics but also help
> avoid yet-another-entry point and force us to provide a well define structure
> for considering code that should not run by pegging it as required or 
> supported
> for different early x86 code stubs.

Was I unclear last time?  Xen *will not* be introducing Linux-specifics
into the HVMLite starting ABI.

Your perceived problem with multiple entry points is not a problem with
multiple entry points; It is a problem with multiple different paths
performing the same initialisation.

The Linux entry for PV guests is indeed completely horrible.  I am not
trying to defend it in the slightest.

However, the HVMLite entry which is a very short stub that sets up a
zeropage and hands off to the native start routine is fine.  There is
still just routine performing native x86 startup.

If you still desperately want to avoid multiple entry points, then just
insist on using grub for the VM.  I expect that that is how most people
will end up using HVMLite VMs anyway.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [linux-linus test] 80122: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 80122 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/80122/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 59254
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 59254
 test-armhf-armhf-xl-cubietruck  8 leak-check/basis(8) fail REGR. vs. 59254
 test-armhf-armhf-xl-xsm  15 guest-start/debian.repeat fail REGR. vs. 59254
 test-armhf-armhf-xl  15 guest-start/debian.repeat fail REGR. vs. 59254
 test-amd64-i386-xl   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl-xsm  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-i386-xl-xsm   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl-credit2  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-xl-multivcpu 15 guest-localmigrate   fail REGR. vs. 59254
 test-amd64-i386-pair   22 guest-migrate/dst_host/src_host fail REGR. vs. 59254
 test-amd64-amd64-pair  22 guest-migrate/dst_host/src_host fail REGR. vs. 59254

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-multivcpu 11 guest-start   fail in 79918 pass in 80122
 test-armhf-armhf-xl-credit2   8 leak-check/basis(8) fail pass in 79918
 test-armhf-armhf-xl-arndale  15 guest-start/debian.repeat   fail pass in 79918
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 20 leak-check/check fail pass in 
79918

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 11 guest-start   fail REGR. vs. 59254
 test-amd64-amd64-xl-rtds 15 guest-localmigratefail REGR. vs. 59254
 test-armhf-armhf-xl-vhd   8 leak-check/basis(8) fail baseline untested
 test-amd64-amd64-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline 
untested
 test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail baseline 
untested
 test-amd64-i386-libvirt  15 guest-saverestore.2  fail blocked in 59254
 test-amd64-i386-libvirt-xsm  15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-libvirt-xsm 15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-libvirt 15 guest-saverestore.2  fail blocked in 59254
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 59254
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 59254
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 59254

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2 13 saverestore-support-check fail in 79918 never 
pass
 test-armhf-armhf-xl-credit2  12 migrate-support-check fail in 79918 never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1 fail never pass
 

Re: [Xen-devel] [BUG] pci-passthrough generates "xen:events: Failed to obtain physical IRQ" for some devices

[Konrad Rzeszutek Wilk]

> > And I wonder if the XEN_DOMCTL_irq_permission aka, xc_domain_irq_permission
> > had been called. I remember that at some point we missed it for Xend..
> >

False alarm.

It was all in Linux. Attached are four patches (the first two
are for your issue you discovered).
I was wondering if there is a way for you to test them?

>From 9b8c92f9535ca9af672facd05360570730a33e05 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk 
Date: Wed, 3 Feb 2016 10:18:18 -0500
Subject: [PATCH 1/4] xen/pciback: Check PF instead of VF for
 PCI_COMMAND_MEMORY

c/s 408fb0e5aa7fda0059db282ff58c3b2a4278baa0
"xen/pciback: Don't allow MSI-X ops if PCI_COMMAND_MEMORY is not set."
would check the device for PCI_COMMAND_MEMORY which is great.
Except that VF devices are unique - for example they have no
legacy interrupts, and also any writes to PCI_COMMAND_MEMORY
are silently ignored (by the hardware).

CC: sta...@vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk 
---
 drivers/xen/xen-pciback/pciback_ops.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/xen-pciback/pciback_ops.c 
b/drivers/xen/xen-pciback/pciback_ops.c
index 73dafdc..8c86a53 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -213,6 +213,7 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
int i, result;
struct msix_entry *entries;
u16 cmd;
+   struct pci_dev *phys_dev;
 
if (unlikely(verbose_request))
printk(KERN_DEBUG DRV_NAME ": %s: enable MSI-X\n",
@@ -227,8 +228,10 @@ int xen_pcibk_enable_msix(struct xen_pcibk_device *pdev,
/*
 * PCI_COMMAND_MEMORY must be enabled, otherwise we may not be able
 * to access the BARs where the MSI-X entries reside.
+* But VF devices are unique in which the PF needs to be checked.
 */
-   pci_read_config_word(dev, PCI_COMMAND, );
+   phys_dev = pci_physfn(dev);
+   pci_read_config_word(phys_dev, PCI_COMMAND, );
if (dev->msi_enabled || !(cmd & PCI_COMMAND_MEMORY))
return -ENXIO;
 
-- 
2.1.0

>From 0f8901117800fc2f1a87cc5468f1ab7e4288cc5f Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk 
Date: Wed, 3 Feb 2016 10:22:02 -0500
Subject: [PATCH 2/4] xen/pciback: Save the number of MSI-X entries to be
 copied later.

c/s  8135cf8b092723dbfcc611fe6fdcb3a36c9951c5
"xen/pciback: Save xen_pci_op commands before processing it"
would copyback the processed values - which was great.

However it missed the case that xen_pcibk_enable_msix - when
completing would overwrite op->value (which had the number
of MSI-X vectors requested) with the return value (which for
success was zero). Hence the copy-back routine (which would use
op->value) would copy exactly zero MSI-X vectors back.

Signed-off-by: Konrad Rzeszutek Wilk 
---
 drivers/xen/xen-pciback/pciback_ops.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/xen-pciback/pciback_ops.c 
b/drivers/xen/xen-pciback/pciback_ops.c
index 8c86a53..2fc5880 100644
--- a/drivers/xen/xen-pciback/pciback_ops.c
+++ b/drivers/xen/xen-pciback/pciback_ops.c
@@ -335,7 +335,9 @@ void xen_pcibk_do_op(struct work_struct *data)
struct xen_pcibk_dev_data *dev_data = NULL;
struct xen_pci_op *op = >op;
int test_intx = 0;
-
+#ifdef CONFIG_PCI_MSI
+   unsigned int nr = 0;
+#endif
*op = pdev->sh_info->op;
barrier();
dev = xen_pcibk_get_pci_dev(pdev, op->domain, op->bus, op->devfn);
@@ -363,6 +365,7 @@ void xen_pcibk_do_op(struct work_struct *data)
op->err = xen_pcibk_disable_msi(pdev, dev, op);
break;
case XEN_PCI_OP_enable_msix:
+   nr = op->value;
op->err = xen_pcibk_enable_msix(pdev, dev, op);
break;
case XEN_PCI_OP_disable_msix:
@@ -385,7 +388,7 @@ void xen_pcibk_do_op(struct work_struct *data)
if (op->cmd == XEN_PCI_OP_enable_msix && op->err == 0) {
unsigned int i;
 
-   for (i = 0; i < op->value; i++)
+   for (i = 0; i < nr; i++)
pdev->sh_info->op.msix_entries[i].vector =
op->msix_entries[i].vector;
}
-- 
2.1.0

>From 3a0d57b60a61eb461504f8ed1845afd5084b7889 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk 
Date: Wed, 3 Feb 2016 16:39:21 -0500
Subject: [PATCH 3/4] xen/pcifront: Report the errors better.

The messages should be different depending on the type of error.

Signed-off-by: Konrad Rzeszutek Wilk 
---
 arch/x86/include/asm/xen/pci.h | 4 ++--
 arch/x86/pci/xen.c | 5 -
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/xen/pci.h 

[Xen-devel] [BUG} libxl.c:5947:libxl_send_trigger: Send trigger 'reset' failed: Function not implemented

[Alex Braunegg]

Hi all,

I have been testing the Xen 4.6.0 packages - however I have found the
following issue with attempting to reboot a Windows VM:

--
[root@mynas-s5000xvn services]# /usr/sbin/xl reboot Windows_2008_R2   
Rebooting domain 5
PV control interface not available: external graceful reboot not possible.
Use "-F" to fallback to ACPI reset event.
reboot failed (rc=-10)

[root@mynas-s5000xvn services]# /usr/sbin/xl -F reboot Windows_2008_R2
/usr/sbin/xl: invalid option -- 'F'
unknown global option

[root@mynas-s5000xvn services]# /usr/sbin/xl reboot -F Windows_2008_R2
Rebooting domain 5
PV control interface not available: sending ACPI reset button event.
libxl: error: libxl.c:5947:libxl_send_trigger: Send trigger 'reset' failed:
Function not implemented
reboot failed (rc=-3)

[root@mynas-s5000xvn services]# /usr/sbin/xl reboot Windows_2008_R2 -F
Rebooting domain 5
PV control interface not available: external graceful reboot not possible.
Use "-F" to fallback to ACPI reset event.
reboot failed (rc=-10)

[root@mynas-s5000xvn services]#
--

The VM in question is a clean install of Windows 2008 R2. The same issue
occurs when attempting to use the 'shutdown' command.

If the PV Windows Drivers are installed - then this is not an issue - the
reboot / shutdown works flawlessly.

The use case here is that not all users will install the PV Windows drivers
- especially since the easy install & signed drivers (msi package) from
'ejbdigital' are no longer available


Best regards,

Alex


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] libxl/libxl_dm.c: Enable websocket functionality

[Alex Braunegg]

Hi all,

I have been testing the Xen 4.6.0 packages - however there is no way that I
have found to enable websocket support in any of the xen / qemu
configuration files. 

The only way that I have been able to do this is with the following patch -
which works perfectly for my use case on a CentOS 6.x derived system:

-
diff -uNr xen-4.6.0.original/tools/libxl/libxl_dm.c
xen-4.6.0.modified/tools/libxl/libxl_dm.c
--- xen-4.6.0.original/tools/libxl/libxl_dm.c   2015-10-06
01:33:39.0 +1100
+++ xen-4.6.0.modified/tools/libxl/libxl_dm.c   2016-01-29
08:50:18.860371704 +1100
@@ -440,11 +440,11 @@
 }
 vncarg = vnc->listen;
 } else {
-vncarg = libxl__sprintf(gc, "%s:%d", vnc->listen,
+vncarg = libxl__sprintf(gc,
"%s:%d,websocket,x509=/etc/pki/xen", vnc->listen,
 vnc->display);
 }
 } else
-vncarg = libxl__sprintf(gc, "127.0.0.1:%d", vnc->display);
+vncarg = libxl__sprintf(gc,
"127.0.0.1:%d,websocket,x509=/etc/pki/xen", vnc->display);

 if (vnc->passwd && vnc->passwd[0]) {
 vncarg = libxl__sprintf(gc, "%s,password", vncarg);
@@ -806,11 +806,11 @@
 }
 vncarg = vnc->listen;
 } else {
-vncarg = libxl__sprintf(gc, "%s:%d", vnc->listen,
+vncarg = libxl__sprintf(gc,
"%s:%d,websocket,x509=/etc/pki/xen", vnc->listen,
 vnc->display);
 }
 } else
-vncarg = libxl__sprintf(gc, "127.0.0.1:%d", vnc->display);
+vncarg = libxl__sprintf(gc,
"127.0.0.1:%d,websocket,x509=/etc/pki/xen", vnc->display);

 if (vnc->passwd && vnc->passwd[0]) {
 vncarg = libxl__sprintf(gc, "%s,password", vncarg);
-

This however introduces 2 hard-coded items:
- Websocket will be enabled each time VNC is enabled.
- x509 certificate path

By default, the websocket port mirrors the vnc port = so vnc port = 5900,
then websocket port = 5700 and so on. 

A better solution however in my mind would be to store the websocket & x509
path in the .cfg file for each virtual machine:

-
vnc=1
vncconsole=1
vnclisten='0.0.0.0'
vncpasswd='abcd1234'
vncdisplay=0
websocket=1 <- where 0 = disabled, 1 = enabled
x509={path} <- File system path to x509 files to cater for OS
system variance or user preference, would also allow for different
certificates per websocket connection if required
-

Best regards,

Alex


















___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] Leaks in xc_tbuf_get_size() (Was: Re: New Defects reported by Coverity Scan for XenProject)

[George Dunlap]

On Wed, Feb 3, 2016 at 10:37 AM, Ian Campbell  wrote:
> George,
>
> Looks like xentrace is the only maintained component which uses this. so
> tag ;-)

OK -- I think this will only be called once, so it's only leaking a
page or two of virtual address space until the process dies (which in
the case of an error will be immediate).  But certainly worth fixing.
:-)

I'll put this on a list of clean-ups; maybe we can give it to the next
OPW / GSoC candidate who shows up (if I don't get to it earlier).

 -George

>
> On Tue, 2016-02-02 at 20:23 -0800, scan-ad...@coverity.com wrote:
>> * CID 1351228:(RESOURCE_LEAK)
>> /tools/libxc/xc_tbuf.c: 73 in xc_tbuf_get_size()
>> /tools/libxc/xc_tbuf.c: 77 in xc_tbuf_get_size()
>
> Coverity is reporting these as new in 41b0aa569adb..9937763265d9 although
> the file hasn't changed. However it does look correct that t_info is being
> leaked by various paths in this function.
>
>>
>>
>> _
>> ___
>> *** CID 1351228:(RESOURCE_LEAK)
>> /tools/libxc/xc_tbuf.c: 73 in xc_tbuf_get_size()
>> 67
>> 68 t_info = xc_map_foreign_range(xch, DOMID_XEN,
>> 69 sysctl.u.tbuf_op.size, PROT_READ | PROT_WRITE,
>> 70 sysctl.u.tbuf_op.buffer_mfn);
>> 71
>> 72 if ( t_info == NULL || t_info->tbuf_size == 0 )
>> >>> CID 1351228:(RESOURCE_LEAK)
>> >>> Variable "t_info" going out of scope leaks the storage it points
>> to.
>> 73 return -1;
>> 74
>> 75 *size = t_info->tbuf_size;
>> 76
>> 77 return 0;
>> 78 }
>> /tools/libxc/xc_tbuf.c: 77 in xc_tbuf_get_size()
>> 71
>> 72 if ( t_info == NULL || t_info->tbuf_size == 0 )
>> 73 return -1;
>> 74
>> 75 *size = t_info->tbuf_size;
>> 76
>> >>> CID 1351228:(RESOURCE_LEAK)
>> >>> Variable "t_info" going out of scope leaks the storage it points
>> to.
>> 77 return 0;
>> 78 }
>> 79
>> 80 int xc_tbuf_enable(xc_interface *xch, unsigned long pages,
>> unsigned long *mfn,
>> 81unsigned long *size)
>> 82 {
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH V13 3/5] libxl: add pvusb API

[George Dunlap]

On 03/02/16 07:34, Chun Yan Liu wrote:
> 
> 
 On 2/3/2016 at 02:11 AM, in message
> <22192.61775.427189.268...@mariner.uk.xensource.com>, Ian Jackson
>  wrote: 
>> George Dunlap writes ("Re: [Xen-devel] [PATCH V13 3/5] libxl: add pvusb 
>> API"): 
>>> There are effectively four states a device can be in, from the 
>>> 'assignment' point of view: 
>>>  
>>> 1. Assigned to the normal Linux device driver(s) for the USB device 
>>>  
>>> 2. Assigned to no driver 
>>>  
>>> 3. Assigned to usbback, but not yet assigned to any guest 
>>>  
>>> 4. Assigned to a guest 
>>  
>> Thanks for your clear explanation (of which I will snip much). 
>>  
>>> Additionally, each USB "device" has one or more "interfaces".  To 
>>> assign a "device" to usbback in the sysfs case means assigning all of 
>>> the "interfaces".  The code seems to assume that different interfaces 
>>> from the same device can be assigned to different drivers. 
>>  
>> It is indeed the case that in principle a single USB device with 
>> multiple interfaces can be assigned to multiple different drivers. 
>>  
>>> Regarding Ian's comments: 
>>>  
>>> Since "assigned to the guest" and "listed under the pvusb xenstore 
>>> node" are the same thing, is it even *possible* to (safely) unassign 
>>> the device from usbback before removing the xenstore nodes? 
>>  
>> It might be possible to remove some of the xenstore nodes but leave 
>> others present, so that usbback detaches, but enough information 
>> remains for libxl to know that Xen still `owns' the device. 
> 
> Indeed "unassign from usbback, but listed under pvusb xenstore" is
> a confusing state. usb-list can list it but guest can not see it. 
> What user can do under that state is: reattempt usbdev_remove, if it 
> succeeds, everything is cleaned up, that's the best result; but 
> possibly it still fails (for example, in my testing, always cannot 
> rebind to original driver), in this case, the confusing state will 
> be lasting, and the device could not be removed, this is worse.

As I said in my other mail, I think removing the pvusb nodes should be
done once it's successfully un-bound from usbback, *even if* the re-bind
to the original driver fails.  (That is, once it reaches state 2,
usb-list should no longer list it.)

>>> Perhaps the best approach code-wise is to change the "goto out" on 
>>> failure of unbind_usbintf() into a "continue".  That way: 
>>>  
>>> 1. All interfaces which can be re-assigned are re-assigned (and work 
>>> as much as possible) 
>>>  
>>> 2. All interfaces which can be unbound but not re-assigned are at 
>>> least unbound (so that reloading the original driver might pick them 
>>> up) 
>>  
>> I certainly don't mind the software trying to do as much of its task 
>> as possible.
> 
> Could I understand that this way is acceptable? That means: removing 
> xenstore, and as much as we could (on failure of "unbind from usbback"
> or "bind to original driver", don't "goto out", just "continue").

I think part of it depends on what is *possible*.  If it's possible to
safely unbind the device from usbback while retaining its place in the
pvusb-related xenstore nodes, then I think we should (so that the user
can re-try removing it).  If it's not possible, then of course we have
to remove the pvusb xenstore nodes first, and then we'll just have to
deal as gracefully as possible with failure unbinding from usbback.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel