Re: [Xen-devel] [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt

[Jan Beulich]

>>> On 26.02.16 at 05:37,  wrote:
>>  From: Zhang, Haozhong
>> Sent: Tuesday, February 23, 2016 10:05 AM
>> 
>> Signed-off-by: Haozhong Zhang 
> 
> Reviewed-by: Kevin Tian , except:
> 
>> +
>> +Hardware TSC Scaling
>> +
>> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
>> +by guest rdtsc/p increasing in a different frequency than the host
>> +TSC frequency.
>> +
>> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
> 
> 'HVM container' means something different. We usually use "HVM domain"
> as you may see in other places in this doc.

But I think this is specifically meant to refer to both HVM and PVH
domains.

Jan


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

Re: [Xen-devel] [PATCH v5 6/6] docs: Add descriptions of TSC scaling in xl.cfg and tscmode.txt

[Haozhong Zhang]

On 02/26/16 01:01, Jan Beulich wrote:
> >>> On 26.02.16 at 05:37,  wrote:
> >>  From: Zhang, Haozhong
> >> Sent: Tuesday, February 23, 2016 10:05 AM
> >> 
> >> Signed-off-by: Haozhong Zhang 
> > 
> > Reviewed-by: Kevin Tian , except:
> > 
> >> +
> >> +Hardware TSC Scaling
> >> +
> >> +Intel VMX TSC scaling and AMD SVM TSC ratio allow the guest TSC read
> >> +by guest rdtsc/p increasing in a different frequency than the host
> >> +TSC frequency.
> >> +
> >> +If a HVM container in default TSC mode (tsc_mode=0) or PVRDTSCP mode
> > 
> > 'HVM container' means something different. We usually use "HVM domain"
> > as you may see in other places in this doc.
> 
> But I think this is specifically meant to refer to both HVM and PVH
> domains.
>

Yes, that is what I mean. So 'HVM container' is the correct name?

Haozhong

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

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Jan Beulich]

>>> On 26.02.16 at 08:37,  wrote:
> On February 25, 2016 8:24pm,  wrote:
>> >>> On 25.02.16 at 13:14,  wrote:
>> > On February 25, 2016 4:59pm,  wrote:
> 
>> >> However, the same effect could be achieved by making the lock a
>> >> recursive one, which would then seem to more conventional approach
>> >> (but requiring as much code to be touched).
> 
> IMO, for v6, I'd better:
>   (1). replace all of 'spin_lock(&pcidevs_lock)' with 
> 'spin_lock_recursive(&pcidevs_lock)',
>   (2). replace all of 'spin_unlock(&pcidevs_lock)' with 
> 'spin_unlock_recursive(&pcidevs_lock)', 
>   (3). _do_not_ touch 'spin_is_locked(&pcidevs_lock)'.
> Then, ignore v5 [patch4/7], [patch5/7], [patch6/7],  *_locked() related 
> code,  and add a new patch for the above (1). (2). (3). Right?

Yes.

> BTW, .e.g. 'd->page_alloc_lock', I found the 'd->page_alloc_lock' is called 
> by 
> both spin_lock_recursive() and 'spin_lock()'.
> _If_  both spin_lock_recursive(&d->page_alloc_lock) and 
> 'spin_lock(&d->page_alloc_lock)' are recursively called in same call tree as 
> below, it might be a bug. Right?
> 
> 
> {
> ...
> spin_lock()
> ...
>spin_lock_recursive()
>...
>spin_unlock_recursive()  <--- (lock might be now free)
> ...
> spin_unlock()
> ...
> }

Well, such a use of course would be a bug. But using plain
spin_lock() on a path where recursive acquire can't occur is
fine.

Nevertheless I'd recommend not mixing things for the pcidevs
one, as its uses are scattered around too much for it to be
reasonably possible to prove correctness if done that way.
Instead please make the lock a static variable in e.g.
xen/drivers/pci/pci.c or xen/drivers/passthrough/pci.c, and
force acquire/release to go through helper functions. That
way we can ensure instance gets left. The only safe alternative
to this would be to rename the lock at once, or to make it
read/write one (but then recursion would be allowed only for
the read cases, and aiui you'd need the write variant for your
use).

Jan


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

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Jan Beulich]

>>> On 26.02.16 at 09:14,  wrote:
> Nevertheless I'd recommend not mixing things for the pcidevs
> one, as its uses are scattered around too much for it to be
> reasonably possible to prove correctness if done that way.
> Instead please make the lock a static variable in e.g.
> xen/drivers/pci/pci.c or xen/drivers/passthrough/pci.c, and
> force acquire/release to go through helper functions. That
> way we can ensure instance gets left. The only safe alternative
> to this would be to rename the lock at once, or to make it
> read/write one (but then recursion would be allowed only for
> the read cases, and aiui you'd need the write variant for your
> use).

Actually, not even that - as of the XSA-114 fix we don't allow
reader recursion anymore. (I wonder whether we have any
latent issue in the tree due to that, since I don't recall anyone
having audited all r/w lock uses back then.)

Jan


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

Re: [Xen-devel] [PATCH v13 1/2] vmx: VT-d posted-interrupt core logic handling

[Jan Beulich]

>>> On 26.02.16 at 02:30,  wrote:

> 
>> -Original Message-
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: Wednesday, February 24, 2016 8:50 PM
>> To: George Dunlap ; Wu, Feng
>> 
>> Cc: Doug Goldstein ; Andrew Cooper
>> ; Dario Faggioli ;
>> GeorgeDunlap ; Tian, Kevin
>> ; xen-devel@lists.xen.org; Keir Fraser 
>> Subject: Re: [Xen-devel] [PATCH v13 1/2] vmx: VT-d posted-interrupt core 
>> logic
>> handling
>> 
>> >>> On 24.02.16 at 13:09,  wrote:
>> > Another reason for such a comment is that it actually raises awareness
>> > that the hook isn't properly structured: if you get such a compile
>> > error, then it's either not defined in the right place, or it's not
>> > using the right calling convention.
>> 
>> Well, why I generally agree with you here, I'm afraid there are
>> many pre-existing instances in our headers. Cleaning that up is
>> likely going to be a major work item.
>> 
>> > It also makes me realize that this code will no longer build on ARM,
>> > since arch_do_block() is only defined in asm-x86 (and not asm-arm).
>> 
>> The patch has
>> 
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -310,6 +310,8 @@ static inline void free_vcpu_guest_context(struct
>> vcpu_guest_context *vgc)
>>  xfree(vgc);
>>  }
>> 
>> +static inline void arch_vcpu_block(struct vcpu *v) {}
>> +
>>  #endif /* __ASM_DOMAIN_H__ */
>> 
>>  /*
>> 
>> (and for the avoidance of doubt there's no arch_do_block() afaics).
>> 
>> > It seems like to do the callback properly, we should do something like
>> > the attached.  Jan, what do you think?
>> 
>> Well, as per above that would address the particular issue here
>> without addressing the many other existing ones, and it would
>> cause out of line functions _plus_ another indirect call when the
>> idea is to have such hooks inlined as far as possible.
>> 
>> But you indeed point out one important problem - the hook as it
>> is right now lacks a has_hvm_container_vcpu(), and hence would
>> break for PV guests.
> 
> Do you mean I need to add has_hvm_container_vcpu() check in
> macro arch_vcpu_block()? But .vcpu_block is assigned in
> vmx_pi_hooks_assign(). I am not clear how this hooks can affect
> PV guests, could you please elaborate a bit more? Thanks a lot!

Quoting you patch (v12, because it looks slightly better, but
the difference doesn't matter for this discussion):

#define arch_vcpu_block(v) ({   \
if ( (v)->domain->arch.hvm_domain.vmx.vcpu_block )  \
(v)->domain->arch.hvm_domain.vmx.vcpu_block((v));   \
})

and quoting asm-x86/domain.h:

struct arch_domain
{
...
union {
struct pv_domain pv_domain;
struct hvm_domain hvm_domain;
};
...
};

Hence accessing the field for PV domains is invalid.

Jan


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

Re: [Xen-devel] [PATCH 2/3] x86/xsaves: fix overwriting between non-lazy/lazy xsave[sc]

[Jan Beulich]

>>> On 26.02.16 at 08:41,  wrote:
> On Wed, Feb 24, 2016 at 02:16:38AM -0700, Jan Beulich wrote:
>> >> > --- a/xen/arch/x86/i387.c
>> >> > +++ b/xen/arch/x86/i387.c
>> >> > @@ -118,7 +118,7 @@ static inline uint64_t vcpu_xsave_mask(const struct 
>> >> > vcpu *v)
>> >> >  if ( v->fpu_dirtied )
>> >> >  return v->arch.nonlazy_xstate_used ? XSTATE_ALL : XSTATE_LAZY;
>> >> >  
>> >> > -return v->arch.nonlazy_xstate_used ? XSTATE_NONLAZY : 0;
>> >> > +return ( cpu_has_xsaves || cpu_has_xsavec ) ? XSTATE_ALL : 
>> >> > XSTATE_NONLAZY;
>> >> >  }
>> >> 
>> >> The description lacks any mention of the performance impact,
>> >> and what investigation was done to find ways to perhaps
>> >> overcome this. For example, regardless of cpu_has_xsaves,
>> >> do we really always need to _use_ XSAVES?
>> >> 
>> > Currently no supervisor xstates is enabled in xen or even in
>> > guest os. Using xsaves is a little ahead (xsavec may used).  
>> > xsavec may also cause overwriting problem like xsaves.
>> > I will add performance impact in the description. 
>> > I am still thinking of a better way to overcome the overhead xsave
>> > (But have not get a better solution yet).
>> 
>> I was thinking that taking into consideration the features a guest
>> has ever used (i.e. v->arch.xcr0_accum) to decide which variant
>> to use may be a worthwhile avenue to explore.
>> 
> Oh, Thanks for your suggestion.
> You mean when (v->arch.xcr0_accum & (XSTATE_LAZY & ~XSTATE_FP_SSE)) return 
> false,
> we can return XSTATE_NONLAZY in vcpu_xsave_mask when using xsave[cs]
> otherwise return XSTATE_ALL.
> It means we can save the area safely, if the guest has ever use 
> XSTATE_NONLAZY | XSTATE_FP_SSE only. 

That's one step in the right direction. But the main difference
between XSAVE/XSAVEOPT and XSAVEC/XSAVES is that the former
can be used incrementally, while the latter can't. And I highly
doubt the modified optimization the latter two support will kick in
very often, since there's quite good a chance that the guest
itself executed another one of these between two of our instances.
Which to me means it should at least be investigated whether using
XSAVEOPT in favor of XSAVEC wouldn't produce better performance,
and whether XSAVES wouldn't better be used only if the guest uses
a component which can only be saved that way.

Jan


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

[Xen-devel] [linux-mingo-tip-master test] 83992: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-rumpuserxen3 host-install(3) broken REGR. vs. 60684
 test-amd64-amd64-i386-pvgrub  3 host-install(3) broken REGR. vs. 60684
 test-amd64-i386-qemuu-rhel6hvm-intel  3 host-install(3) broken REGR. vs. 60684
 test-amd64-amd64-xl-qemut-win7-amd64  3 host-install(3) broken REGR. vs. 60684
 test-amd64-i386-xl-qemut-winxpsp3  3 host-install(3)broken REGR. vs. 60684
 test-amd64-amd64-xl-xsm  15 guest-localmigratefail REGR. vs. 60684
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 60684
 test-amd64-amd64-xl-multivcpu 15 guest-localmigrate   fail REGR. vs. 60684
 test-amd64-amd64-libvirt 15 guest-saverestore.2   fail REGR. vs. 60684
 test-amd64-amd64-xl  15 guest-localmigratefail REGR. vs. 60684
 test-amd64-amd64-xl-credit2  15 guest-localmigratefail REGR. vs. 60684
 test-amd64-amd64-libvirt-xsm 15 guest-saverestore.2   fail REGR. vs. 60684
 test-amd64-amd64-pair  22 guest-migrate/dst_host/src_host fail REGR. vs. 60684

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds 15 guest-localmigratefail REGR. vs. 60684
 test-amd64-i386-libvirt-xsm  15 guest-saverestore.2  fail blocked in 60684
 test-amd64-i386-xl-xsm   15 guest-localmigrate   fail blocked in 60684
 test-amd64-i386-libvirt  15 guest-saverestore.2  fail blocked in 60684
 test-amd64-i386-xl   15 guest-localmigrate   fail blocked in 60684
 test-amd64-i386-libvirt-pair 22 guest-migrate/dst_host/src_host fail blocked 
in 60684
 test-amd64-amd64-libvirt-pair 22 guest-migrate/dst_host/src_host fail blocked 
in 60684
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop   fail blocked in 60684
 test-amd64-i386-pair  22 guest-migrate/dst_host/src_host fail blocked in 60684
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 60684
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 60684

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-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-amd 13 xen-boot/l1   fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1 fail never pass

version targeted for testing:
 linuxc0d4dca97aa5990b277f77c5d8c50bd3718f1189
baseline version:
 linux69f75ebe3b1d1e636c4ce0a0ee248edacc69cbe0

Last test of basis60684  2015-08-13 04:21:46 Z  197 days
Failing since 60712  2015-08-15 18:33:48 Z  194 days  139 attempts
Testing same since83992  2016-02-25 16:52:20 Z0 days1 attempts

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   broken  
 test-amd64-amd64-xl  fail
 test-amd64-i386-xl   fail
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 te

[Xen-devel] [ovmf test] 83998: regressions - FAIL

[osstest service owner]

flight 83998 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/83998/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail 
REGR. vs. 65543
 test-amd64-i386-xl-qemuu-ovmf-amd64 17 guest-start/debianhvm.repeat fail REGR. 
vs. 65543

version targeted for testing:
 ovmf e89d672110aaf1c5a85404375ca6767b099d3b50
baseline version:
 ovmf 5ac96e3a28dd26eabee421919f67fa7c443a47f1

Last test of basis65543  2015-12-08 08:45:15 Z   80 days
Failing since 65593  2015-12-08 23:44:51 Z   79 days   81 attempts
Testing same since83998  2016-02-25 17:22:47 Z0 days1 attempts


People who touched revisions under test:
  "Samer El-Haj-Mahmoud" 
  "Yao, Jiewen" 
  Alcantara, Paulo 
  Andrew Fish 
  Ard Biesheuvel 
  Arthur Crippa Burigo 
  Cecil Sheng 
  Chao Zhang 
  Charles Duffy 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Daocheng Bu 
  Daryl McDaniel 
  edk2 dev 
  edk2-devel 
  Eric Dong 
  Eric Dong 
  Eugene Cohen 
  Evan Lloyd 
  Feng Tian 
  Fu Siyuan 
  Hao Wu 
  Hess Chen 
  Heyi Guo 
  Jaben Carsey 
  Jeff Fan 
  Jiaxin Wu 
  jiewen yao 
  Jim Dailey 
  Jordan Justen 
  Karyne Mayer 
  Larry Hauch 
  Laszlo Ersek 
  Leekha Shaveta 
  Leif Lindholm 
  Liming Gao 
  Mark Rutland 
  Marvin Haeuser 
  Michael Kinney 
  Michael LeMay 
  Michael Thomas 
  Ni, Ruiyu 
  Paolo Bonzini 
  Paulo Alcantara 
  Paulo Alcantara Cavalcanti 
  Qin Long 
  Qiu Shumin 
  Rodrigo Dias Correa 
  Ruiyu Ni 
  Ryan Harkin 
  Samer El-Haj-Mahmoud 
  Samer El-Haj-Mahmoud 
  Star Zeng 
  Supreeth Venkatesh 
  Tapan Shah 
  Vladislav Vovchenko 
  Yao Jiewen 
  Yao, Jiewen 
  Ye Ting 
  Yonghong Zhu 
  Zhang Lubo 
  Zhang, Chao B 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  fail



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

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

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

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


Not pushing.

(No revision log; it would be 9934 lines long.)

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

Re: [Xen-devel] [PATCH v13 1/2] vmx: VT-d posted-interrupt core logic handling

[Wu, Feng]

> Quoting you patch (v12, because it looks slightly better, but
> the difference doesn't matter for this discussion):
> 
> #define arch_vcpu_block(v) ({   \
> if ( (v)->domain->arch.hvm_domain.vmx.vcpu_block )  \
> (v)->domain->arch.hvm_domain.vmx.vcpu_block((v));   \
> })
> 
> and quoting asm-x86/domain.h:
> 
> struct arch_domain
> {
> ...
> union {
> struct pv_domain pv_domain;
> struct hvm_domain hvm_domain;
> };
> ...
> };
> 
> Hence accessing the field for PV domains is invalid.

Oh, that is right! Accessing 'hvm_domain' itself needs to be gated by
has_hvm_container_vcpu(), Thanks for pointing this out!

Thanks,
Feng

> 
> Jan


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

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

[Dario Faggioli]

On Fri, 2016-02-26 at 00:15 -0500, Tianyang Chen wrote:
> On 2/25/2016 6:31 PM, Dario Faggioli wrote:
> > As far as my opinion goes, this patch is not ready to
> > go in
> > right now (as I said, I've got questions and comments), but its
> > status
> > is way past RFC.
> > 
> Oh OK, I had the impression that RFC means request for comments and
> it 
> should always be used because indeed, I'm asking for comments.
> 
Exactly. Everyone is asking for comments when sending out a patch
series, and that's why it's not necessary to state it with the tag...
it's always the case, so no need to tell it all the times!

And, for the same reason, this means that, when the tag is there,
you're not only asking for comments and/or, if everything is ok,
inclusion, but you're asking for "just some feedback on a draft", and
as I said, we're beyond that phase. :-)

> > Wait, maybe you misunderstood and/or I did not make myself clear
> > enough
> > (in which case, sorry). I never meant to say "always stop the
> > timer".
> > Atually, in one of my last email I said the opposite, and I think
> > that
> > would be the best thing to do.
> > 
> > Do you think there is any specific reason why we need to always
> > stop
> > and restart it? If not, I think we can:
> >   - have deadline_queue_remove() also return whether the element
> > removed was the first one (i.e., the one the timer was
> > programmed
> > after);
> >   - if it was, re-program the timer after the new front of the
> > queue;
> >   - if it wasn't, nothing to do.
> > 
> > Thoughts?
> > 
> It was my thought originally that the timer needs to be re-
> programmed 
> only when the top vcpu is taken off. So did you mean when I
> manipulated 
> repl_timer->expires manually, the timer should be stopped using
> proper 
> timer API? The manipulation is gone now.
>
I know... This is mostly due to my fault commenting on this in two
different emails.

So, basically, yes, I meant that, if you want to fiddle with the timer
--like you where doing with those 'repl_timer->expires = 0'-- you need
to do it properly, with the proper API, locking, etc.

Then, in a subsequent email, I said that you just only need to do that
in a subset of the cases when this function is called.

Of course, the desired result is the combination of the two above
considerations, i.e.:
 - only stop/restart the timer when necessary,
 - if necessary, do it properly.

>  Also, set_timer internally 
> disables the timer so I assume it's safe just to call set_timer() 
> directly, right?
> 
Yes, it is.

> > > @@ -405,22 +500,38 @@ __runq_insert(const struct scheduler *ops,
> > > struct rt_vcpu *svc)
> > > 
> > >   /* add svc to runq if svc still has budget */
> > >   if ( svc->cur_budget > 0 )
> > > -{
> > > -list_for_each(iter, runq)
> > > -{
> > > -struct rt_vcpu * iter_svc = __q_elem(iter);
> > > -if ( svc->cur_deadline <= iter_svc->cur_deadline )
> > > -break;
> > > - }
> > > -list_add_tail(&svc->q_elem, iter);
> > > -}
> > > +deadline_queue_insert(&__q_elem, svc, &svc->q_elem,
> > > runq);
> > >   else
> > >   {
> > >   list_add(&svc->q_elem, &prv->depletedq);
> > > +ASSERT( __vcpu_on_replq(svc) );
> > > 
> > So, by doing this, you're telling me that, if the vcpu is added to
> > the
> > depleted queue, there must be a replenishment planned for it (or
> > the
> > ASSERT() would fail).
> > 
> > But if we are adding it to the runq, there may or may not be a
> > replenishment planned for it.
> > 
> > Is this what we want? Why there must not be a replenishment planned
> > already for a vcpu going into the runq (to happen at its next
> > deadline)?
> > 
> It looks like the current code doesn't add a vcpu to the
> replenishment 
> queue when vcpu_insert() is called. When the scheduler is
> initialized, 
> all the vcpus are added to the replenishment queue after waking up
> from 
> sleep. This needs to be changed (add vcpu to replq in vcpu_insert())
> to 
> make it consistent in a sense that when rt_vcpu_insert() is called,
> it 
> needs to have a corresponding replenishment event queued.
> 
> This way the ASSERT() is for both cases in __runq_insert() to
> enforce 
> the fact that "when a vcpu is inserted to runq/depletedq, a 
> replenishment event is waiting for it".
> 
I am ok with this (calling replq_insert() in rt_vcpu_insert()). Not
just doing that unconditionally though, as a vcpu can, e.g., be paused
when the insert_vcpu hook is called, and in that case, I don't think we
want to enqueue the replenishment event, do we?

> > static void
> > rt_context_saved(const struct scheduler *ops, struct vcpu *vc)
> > {
> >  struct rt_vcpu *svc = rt_vcpu(vc);
> >  spinlock_t *lock = vcpu_schedule_lock_irq(vc);
> > 
> >  clear_bit(__RTDS_scheduled, &svc->flags);
> >  /* not insert idle vcpu to runq */
> >  if ( is_idle_vcpu(vc) )
> >  goto out;
> > 
> >  if ( test_and

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Paul Durrant]

> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 26 February 2016 04:25
> To: Paul Durrant; xen-de...@lists.xenproject.org
> Subject: RE: [Xen-devel] [PATCH] docs/design: introduce
> HVMMEM_ioreq_serverX types
> 
> > From: Paul Durrant
> > Sent: Thursday, February 25, 2016 11:49 PM
> >
> > This patch adds a new 'designs' subdirectory under docs as a repository
> > for this and future design proposals.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> >
> > For convenience this document can also be viewed in PDF at:
> >
> > http://xenbits.xen.org/people/pauldu/hvmmem_ioreq_server.pdf
> > ---
> >  docs/designs/hvmmem_ioreq_server.md | 63
> > +
> >  1 file changed, 63 insertions(+)
> >  create mode 100755 docs/designs/hvmmem_ioreq_server.md
> >
> > diff --git a/docs/designs/hvmmem_ioreq_server.md
> > b/docs/designs/hvmmem_ioreq_server.md
> > new file mode 100755
> > index 000..47fa715
> > --- /dev/null
> > +++ b/docs/designs/hvmmem_ioreq_server.md
> > @@ -0,0 +1,63 @@
> > +HVMMEM\_ioreq\_serverX
> > +--
> > +
> > +Background
> > +==
> > +
> > +The concept of the IOREQ server was introduced to allow multiple distinct
> > +device emulators to a single VM. The XenGT project uses an IOREQ server
> to
> > +provide mediated pass-through of Intel GPUs to guests and, as part of the
> > +mediation, needs to intercept accesses to GPU page-tables (or GTTs) that
> > +reside in guest RAM.
> > +
> > +The current implementation of this sets the type of GTT pages to type
> > +HVMMEM\_mmio\_write\_dm, which causes Xen to emulate writes to
> such pages,
> > +and then maps the guest physical addresses of those pages to the XenGT
> > +IOREQ server using the HVMOP\_map\_io\_range\_to\_ioreq\_server
> hypercall.
> > +However, because the number of GTTs is potentially large, using this
> > +approach does not scale well.
> > +
> > +Proposal
> > +
> > +
> > +Because the number of spare types available in the P2M type-space is
> > +currently very limited it is proposed that HVMMEM\_mmio\_write\_dm
> be
> > +replaced by a single new type HVMMEM\_ioreq\_server. In future, if the
> > +P2M type-space is increased, this can be renamed to
> HVMMEM\_ioreq\_server0
> > +and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc.
> types
> > +can be added.
> > +
> > +Accesses to a page of type HVMMEM\_ioreq\_serverX should be the
> same as
> > +HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server.
> Furthermore
> > +it should only be possible to set the type of a page to
> > +HVMMEM\_ioreq\_serverX if that page is currently of type
> HVMMEM\_ram\_rw.
> 
> Is there similar assumption on the opposite change, i.e. from ioreq_serverX
> only to ram_rw?
> 

Yes, I will call that out.

> > +
> > +To allow an IOREQ server to claim or release a claim to a type a new pair
> > +of hypercalls will be introduced:
> > +
> > +- HVMOP\_map\_mem\_type\_to\_ioreq\_server
> > +- HVMOP\_unmap\_mem\_type\_from\_ioreq\_server
> > +
> > +and an associated argument structure:
> > +
> > +   struct hvm_ioreq_mem_type {
> > +   domid_t domid;  /* IN - domain to be serviced */
> > +   ioservid_t id;  /* IN - server id */
> > +   hvmmem_type_t type; /* IN - memory type */
> > +   uint32_t flags; /* IN - types of access to be
> > +   intercepted */
> > +
> > +   #define _HVMOP_IOREQ_MEM_ACCESS_READ 0
> > +   #define HVMOP_IOREQ_MEM_ACCESS_READ \
> > +   (1 << _HVMOP_IOREQ_MEM_ACCESS_READ)
> > +
> > +   #define _HVMOP_IOREQ_MEM_ACCESS_WRITE 1
> > +   #define HVMOP_IOREQ_MEM_ACCESS_WRITE \
> > +   (1 << _HVMOP_IOREQ_MEM_ACCESS_WRITE)
> > +
> > +   };
> > +
> > +
> > +Once the type has been claimed then the requested types of access to
> any
> > +page of the claimed type will be passed to the IOREQ server for handling.
> > +Only HVMMEM\_ioreq\_serverX types may be claimed.
> > --
> 
> It'd good to also add how to handle multiple ioreq servers claiming
> one same type for a given domain.
> 

That would clearly be an error so I imagine -EBUSY would be an appropriate 
return value from the map hypercall.

  Paul

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

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Jan Beulich]

>>> On 26.02.16 at 05:24,  wrote:
>> +Proposal
>> +
>> +
>> +Because the number of spare types available in the P2M type-space is
>> +currently very limited it is proposed that HVMMEM\_mmio\_write\_dm be
>> +replaced by a single new type HVMMEM\_ioreq\_server. In future, if the
>> +P2M type-space is increased, this can be renamed to HVMMEM\_ioreq\_server0
>> +and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc. types
>> +can be added.
>> +
>> +Accesses to a page of type HVMMEM\_ioreq\_serverX should be the same as
>> +HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server. Furthermore
>> +it should only be possible to set the type of a page to
>> +HVMMEM\_ioreq\_serverX if that page is currently of type HVMMEM\_ram\_rw.
> 
> Is there similar assumption on the opposite change, i.e. from ioreq_serverX
> only to ram_rw?

Yes. The above should be re-worded to make clear this is a
bi-directional requirement.

>> +To allow an IOREQ server to claim or release a claim to a type a new pair
>> +of hypercalls will be introduced:
>> +
>> +- HVMOP\_map\_mem\_type\_to\_ioreq\_server
>> +- HVMOP\_unmap\_mem\_type\_from\_ioreq\_server
>> +
>> +and an associated argument structure:
>> +
>> +struct hvm_ioreq_mem_type {
>> +domid_t domid;  /* IN - domain to be serviced */
>> +ioservid_t id;  /* IN - server id */
>> +hvmmem_type_t type; /* IN - memory type */
>> +uint32_t flags; /* IN - types of access to be
>> +intercepted */
>> +
>> +#define _HVMOP_IOREQ_MEM_ACCESS_READ 0
>> +#define HVMOP_IOREQ_MEM_ACCESS_READ \
>> +(1 << _HVMOP_IOREQ_MEM_ACCESS_READ)
>> +
>> +#define _HVMOP_IOREQ_MEM_ACCESS_WRITE 1
>> +#define HVMOP_IOREQ_MEM_ACCESS_WRITE \
>> +(1 << _HVMOP_IOREQ_MEM_ACCESS_WRITE)
>> +
>> +};
>> +
>> +
>> +Once the type has been claimed then the requested types of access to any
>> +page of the claimed type will be passed to the IOREQ server for handling.
>> +Only HVMMEM\_ioreq\_serverX types may be claimed.
>> --
> 
> It'd good to also add how to handle multiple ioreq servers claiming
> one same type for a given domain.

It's going to fail, but I agree that perhaps this would better be
spelled out.

Jan


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

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Jan Beulich]

>>> On 26.02.16 at 07:59,  wrote:
>> +Proposal
>> +
>> +
>> +Because the number of spare types available in the P2M type-space is
>> +currently very limited it is proposed that HVMMEM\_mmio\_write\_dm be
>> +replaced by a single new type HVMMEM\_ioreq\_server. In future, if the
>> +P2M type-space is increased, this can be renamed to HVMMEM\_ioreq\_server0
>> +and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc. types
>> +can be added.
>> +
>> +Accesses to a page of type HVMMEM\_ioreq\_serverX should be the same as
>> +HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server. Furthermore
> 
> Sorry, do you mean even when a gfn is set to HVMMEM_ioreq_serverX type,
> its access rights in P2M still remains unchanged? So the new hypercall
> pair, HVMOP_[un]map_mem_type_to_ioreq_server, are also responsible for
> the PTE updates on the access bits?
> 
> If it is true, I'm afraid this would be time consuming, because the
> map/unmap will have to traverse all P2M structures to detect the PTEs
> with HVMMEM_ioreq_serverX flag set. Yet in XenGT, setting this flag is
> triggered dynamically with the construction/destruction of shadow PPGTT.
> But I'm not sure to which degree the performance casualties will be,
> with frequent EPT table walk and EPT tlb flush.

No walking of EPT trees will be necessary in that case, just like it
already has been made unnecessary for other changes resulting
in various PTE attributes needing re-calculation. We'll only need
to extend the p2m_memory_type_changed() mechanism to cover
changes like this one.

Jan


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

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Paul Durrant]

> -Original Message-
> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
> Sent: 26 February 2016 06:59
> To: Paul Durrant; xen-de...@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH] docs/design: introduce
> HVMMEM_ioreq_serverX types
> 
> Hi Paul,
> 
>Thanks a lot for your help on this! And below are my questions.
> 
> On 2/25/2016 11:49 PM, Paul Durrant wrote:
> > This patch adds a new 'designs' subdirectory under docs as a repository
> > for this and future design proposals.
> >
> > Signed-off-by: Paul Durrant 
> > ---
> >
> > For convenience this document can also be viewed in PDF at:
> >
> > http://xenbits.xen.org/people/pauldu/hvmmem_ioreq_server.pdf
> > ---
> >   docs/designs/hvmmem_ioreq_server.md | 63
> +
> >   1 file changed, 63 insertions(+)
> >   create mode 100755 docs/designs/hvmmem_ioreq_server.md
> >
> > diff --git a/docs/designs/hvmmem_ioreq_server.md
> b/docs/designs/hvmmem_ioreq_server.md
> > new file mode 100755
> > index 000..47fa715
> > --- /dev/null
> > +++ b/docs/designs/hvmmem_ioreq_server.md
> > @@ -0,0 +1,63 @@
> > +HVMMEM\_ioreq\_serverX
> > +--
> > +
> > +Background
> > +==
> > +
> > +The concept of the IOREQ server was introduced to allow multiple distinct
> > +device emulators to a single VM. The XenGT project uses an IOREQ server
> to
> > +provide mediated pass-through of Intel GPUs to guests and, as part of the
> > +mediation, needs to intercept accesses to GPU page-tables (or GTTs) that
> > +reside in guest RAM.
> > +
> > +The current implementation of this sets the type of GTT pages to type
> > +HVMMEM\_mmio\_write\_dm, which causes Xen to emulate writes to
> such pages,
> > +and then maps the guest physical addresses of those pages to the XenGT
> > +IOREQ server using the HVMOP\_map\_io\_range\_to\_ioreq\_server
> hypercall.
> > +However, because the number of GTTs is potentially large, using this
> > +approach does not scale well.
> > +
> > +Proposal
> > +
> > +
> > +Because the number of spare types available in the P2M type-space is
> > +currently very limited it is proposed that HVMMEM\_mmio\_write\_dm
> be
> > +replaced by a single new type HVMMEM\_ioreq\_server. In future, if the
> > +P2M type-space is increased, this can be renamed to
> HVMMEM\_ioreq\_server0
> > +and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc.
> types
> > +can be added.
> > +
> > +Accesses to a page of type HVMMEM\_ioreq\_serverX should be the
> same as
> > +HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server.
> Furthermore
> 
> Sorry, do you mean even when a gfn is set to HVMMEM_ioreq_serverX
> type,
> its access rights in P2M still remains unchanged? So the new hypercall
> pair, HVMOP_[un]map_mem_type_to_ioreq_server, are also responsible
> for
> the PTE updates on the access bits?

Yes, the access bits would not change *on existing* pages of this type until 
the type is claimed.

> 
> If it is true, I'm afraid this would be time consuming, because the
> map/unmap will have to traverse all P2M structures to detect the PTEs
> with HVMMEM_ioreq_serverX flag set. Yet in XenGT, setting this flag is
> triggered dynamically with the construction/destruction of shadow PPGTT.
> But I'm not sure to which degree the performance casualties will be,
> with frequent EPT table walk and EPT tlb flush.

I don't see the concern. I am assuming XenGT would claim the type at start of 
day and then just to HVMOP_set_mem_type operations on PPGTT pages as necessary, 
which would not require p2m traversal (since that hypercall takes the gfn as an 
arg) and the EPT flush requirements are no different to setting the 
mmio_write_dm type in the current implementation.

> 
> If it is not, I guess we can(e.g. when trying to WP a gfn):
> 1> use HVMOP_set_mem_type to set the HVMMEM_ioreq_serverX flag,
> which
> for the write protected case works the same as HVMMEM_mmio_write_dm;
> If successful, accesses to a page of type HVMMEM_ioreq_serverX should
> trigger the ioreq server selection path, but will be discarded.
> 2> after HVMOP_map_mem_type_to_ioreq_server is called, all accesses to
> this pages of type HVMMEM_ioreq_serverX would be forwarded to a
> specified ioreq server.
> 
> As to XenGT backend device model, we only need to use the map hypercall
> once when trying to contruct the first shadow PPGTT, and use the unmap
> hypercall when a VM is going to be torn down.
> 

Yes, that's correct. A single claim at start of day and a single 'unclaim' at 
end of day. In between the current HVMOP_set_mem_type is pretty much as before 
(just with the new type name) and there is no longer a need for the hypercall 
to map the range to the ioreq server.

  Paul

> Any suggestions? :)
> 
> Thanks
> Yu
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Xu, Quan]

On February 26, 2016 4:15pm,  wrote:
> >>> On 26.02.16 at 08:37,  wrote:
> > On February 25, 2016 8:24pm,  wrote:
> >> >>> On 25.02.16 at 13:14,  wrote:
> >> > On February 25, 2016 4:59pm,  wrote:
> >
> >> >> However, the same effect could be achieved by making the lock a
> >> >> recursive one, which would then seem to more conventional approach
> >> >> (but requiring as much code to be touched).
> >
> > IMO, for v6, I'd better:
> >   (1). replace all of 'spin_lock(&pcidevs_lock)' with
> > 'spin_lock_recursive(&pcidevs_lock)',
> >   (2). replace all of 'spin_unlock(&pcidevs_lock)' with
> > 'spin_unlock_recursive(&pcidevs_lock)',
> >   (3). _do_not_ touch 'spin_is_locked(&pcidevs_lock)'.
> > Then, ignore v5 [patch4/7], [patch5/7], [patch6/7],  *_locked()
> > related code,  and add a new patch for the above (1). (2). (3). Right?
> 
> Yes.
> 
> > BTW, .e.g. 'd->page_alloc_lock', I found the 'd->page_alloc_lock' is
> > called by both spin_lock_recursive() and 'spin_lock()'.
> > _If_  both spin_lock_recursive(&d->page_alloc_lock) and
> > 'spin_lock(&d->page_alloc_lock)' are recursively called in same call
> > tree as below, it might be a bug. Right?
> >
> >
> > {
> > ...
> > spin_lock()
> > ...
> >spin_lock_recursive()
> >...
> >spin_unlock_recursive()  <--- (lock might be now free)
> > ...
> > spin_unlock()
> > ...
> > }
> 
> Well, such a use of course would be a bug. But using plain
> spin_lock() on a path where recursive acquire can't occur is fine.
> 
> Nevertheless I'd recommend not mixing things for the pcidevs one, as its uses
> are scattered around too much for it to be reasonably possible to prove
> correctness if done that way.


But I do _not_ mix things for the pcidevs one. As 
  (1). replace ___all___ of 'spin_lock(&pcidevs_lock)' with 
'spin_lock_recursive(&pcidevs_lock)',
  (2). replace ___all___ of 'spin_unlock(&pcidevs_lock)' with 
'spin_unlock_recursive(&pcidevs_lock)',



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

Re: [Xen-devel] [PATCH 0/4] handful of Kconfig changes

[Jan Beulich]

>>> On 26.02.16 at 04:39,  wrote:
> Mostly just simplification, consolidation, and more conversion. The
> last two were entries added while the original series was in the last
> steps of review so they just needed to be finished up.
> 
> Doug Goldstein (4):
>   build: consolidate CONFIG_HAS_VIDEO and CONFIG_VIDEO
>   build: consolidate CONFIG_HAS_ACPI and CONFIG_ACPI
>   build: convert HAS_NUMA to Kconfig
>   build: convert HAS_CORE_PARKING to Kconfig

Patches 3 and 4 have indentation issues, which could be fixed up
while committing, and with them adjusted
Reviewed-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Yu, Zhang]

Thanks, Paul.

On 2/26/2016 5:21 PM, Paul Durrant wrote:

-Original Message-
From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
Sent: 26 February 2016 06:59
To: Paul Durrant; xen-de...@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH] docs/design: introduce
HVMMEM_ioreq_serverX types

Hi Paul,

Thanks a lot for your help on this! And below are my questions.

On 2/25/2016 11:49 PM, Paul Durrant wrote:

This patch adds a new 'designs' subdirectory under docs as a repository
for this and future design proposals.

Signed-off-by: Paul Durrant 
---

For convenience this document can also be viewed in PDF at:

http://xenbits.xen.org/people/pauldu/hvmmem_ioreq_server.pdf
---
   docs/designs/hvmmem_ioreq_server.md | 63

+

   1 file changed, 63 insertions(+)
   create mode 100755 docs/designs/hvmmem_ioreq_server.md

diff --git a/docs/designs/hvmmem_ioreq_server.md

b/docs/designs/hvmmem_ioreq_server.md

new file mode 100755
index 000..47fa715
--- /dev/null
+++ b/docs/designs/hvmmem_ioreq_server.md
@@ -0,0 +1,63 @@
+HVMMEM\_ioreq\_serverX
+--
+
+Background
+==
+
+The concept of the IOREQ server was introduced to allow multiple distinct
+device emulators to a single VM. The XenGT project uses an IOREQ server

to

+provide mediated pass-through of Intel GPUs to guests and, as part of the
+mediation, needs to intercept accesses to GPU page-tables (or GTTs) that
+reside in guest RAM.
+
+The current implementation of this sets the type of GTT pages to type
+HVMMEM\_mmio\_write\_dm, which causes Xen to emulate writes to

such pages,

+and then maps the guest physical addresses of those pages to the XenGT
+IOREQ server using the HVMOP\_map\_io\_range\_to\_ioreq\_server

hypercall.

+However, because the number of GTTs is potentially large, using this
+approach does not scale well.
+
+Proposal
+
+
+Because the number of spare types available in the P2M type-space is
+currently very limited it is proposed that HVMMEM\_mmio\_write\_dm

be

+replaced by a single new type HVMMEM\_ioreq\_server. In future, if the
+P2M type-space is increased, this can be renamed to

HVMMEM\_ioreq\_server0

+and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc.

types

+can be added.
+
+Accesses to a page of type HVMMEM\_ioreq\_serverX should be the

same as

+HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server.

Furthermore

Sorry, do you mean even when a gfn is set to HVMMEM_ioreq_serverX
type,
its access rights in P2M still remains unchanged? So the new hypercall
pair, HVMOP_[un]map_mem_type_to_ioreq_server, are also responsible
for
the PTE updates on the access bits?


Yes, the access bits would not change *on existing* pages of this type until 
the type is claimed.



If it is true, I'm afraid this would be time consuming, because the
map/unmap will have to traverse all P2M structures to detect the PTEs
with HVMMEM_ioreq_serverX flag set. Yet in XenGT, setting this flag is
triggered dynamically with the construction/destruction of shadow PPGTT.
But I'm not sure to which degree the performance casualties will be,
with frequent EPT table walk and EPT tlb flush.


I don't see the concern. I am assuming XenGT would claim the type at start of 
day and then just to HVMOP_set_mem_type operations on PPGTT pages as necessary, 
which would not require p2m traversal (since that hypercall takes the gfn as an 
arg) and the EPT flush requirements are no different to setting the 
mmio_write_dm type in the current implementation.


So this looks like my second interpretation, with step 1> and step 2>
changed?





If it is not, I guess we can(e.g. when trying to WP a gfn):
1> use HVMOP_set_mem_type to set the HVMMEM_ioreq_serverX flag,
which
for the write protected case works the same as HVMMEM_mmio_write_dm;
If successful, accesses to a page of type HVMMEM_ioreq_serverX should
trigger the ioreq server selection path, but will be discarded.
2> after HVMOP_map_mem_type_to_ioreq_server is called, all accesses to
this pages of type HVMMEM_ioreq_serverX would be forwarded to a
specified ioreq server.

As to XenGT backend device model, we only need to use the map hypercall
once when trying to contruct the first shadow PPGTT, and use the unmap
hypercall when a VM is going to be torn down.



Yes, that's correct. A single claim at start of day and a single 'unclaim' at 
end of day. In between the current HVMOP_set_mem_type is pretty much as before 
(just with the new type name) and there is no longer a need for the hypercall 
to map the range to the ioreq server.

   Paul



B.R.
Yu

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

Re: [Xen-devel] [PATCH v5 07/22] ACPI / table: Print GIC information when MADT is parsed

[Jan Beulich]

>>> On 26.02.16 at 07:22,  wrote:
> From: Hanjun Guo 
> 
> When MADT is parsed, print GIC information as debug message:
> 
> ACPI: GICC (acpi_id[0x] address[e112f000] MPIDR[0x0] enabled)
> ACPI: GICC (acpi_id[0x0001] address[e112f000] MPIDR[0x1] enabled)
> ...
> ACPI: GICC (acpi_id[0x0201] address[e112f000] MPIDR[0x201] enabled)
> 
> This debug information will be very helpful to bring up early systems to
> see if acpi_id and MPIDR are matched or not as spec defined.
> 
> CC: Rafael J. Wysocki 
> Tested-by: Suravee Suthikulpanit 
> Tested-by: Yijing Wang 
> Tested-by: Mark Langsdorf 
> Tested-by: Jon Masters 
> Tested-by: Timur Tabi 
> Tested-by: Robert Richter 
> Acked-by: Robert Richter 
> Acked-by: Sudeep Holla 
> Acked-by: Rafael J. Wysocki 
> Acked-by: Lorenzo Pieralisi 
> Acked-by: Grant Likely 
> Signed-off-by: Hanjun Guo 
> Signed-off-by: Tomasz Nowicki 
> Signed-off-by: Will Deacon 
> [Linux commit 4c1c8d7a7ebc8b909493a14b21b233e5377b69aa]
> [Use container_of instead of cast and PRIx64 instead of %llx]
> Signed-off-by: Shannon Zhao 

A note on this bazillion of tags: I think Tested-by should
unconditionally be dropped when coming from a Linux commit. I
also think retaining Acked-by doesn't make much sense (and may
be actively misleading if it came from a person who is maintainer
of parts of our code). Same would go for Reviewed-by, which we
don't have here. And for the Signed-off-by I think only the original
author's and yours are relevant, unless the others mean actual
changes were done (i.e. they didn't just get added because the
patch passed their hands). This may not always be easy to tell, so
I don't take this last aspect very strictly, but when pulling in Linux
changes myself, I usually try to guess.

No reason to re-submit, though, I'll take care of stripping what I
think should be stripped when committing.

> V5: port this change from Linux

I much appreciate that while doing so you converted to container_of().

Thanks, Jan


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

Re: [Xen-devel] Ping: [PATCH 2/5] x86emul: limit-check branch targets

[Tim Deegan]

At 07:52 -0700 on 25 Feb (1456386721), Jan Beulich wrote:
> >>> On 17.02.16 at 17:35,  wrote:
> > All branches need to #GP when their target violates the segment limit
> > (in 16- and 32-bit modes) or is non-canonical (in 64-bit mode). For
> > near branches facilitate this via a zero-byte instruction fetch from
> > the target address (resulting in address translation and validation
> > without an actual read from memory), while far branches get dealt with
> > by breaking up the segment register loading into a read-and-validate
> > part and a write one. The latter at once allows correcting some
> > ordering issues in how the individual emulation steps get carried out:
> > Before updating machine state, all exceptions unrelated to that state
> > updating should have got raised (i.e. the only ones possibly resulting
> > in partly updated state are faulting memory writes [pushes]).
> > 
> > Note that while not immediately needed here, write and distinct read
> > emulation routines get updated to deal with zero byte accesses too, for
> > overall consistency.
> > 
> > Reported-by: å??令 
> > Signed-off-by: Jan Beulich 

Sorry, hadn't spotted the shadow change.

Acked-by: Tim Deegan 

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

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Yu, Zhang]

On 2/26/2016 5:18 PM, Jan Beulich wrote:

On 26.02.16 at 07:59,  wrote:

+Proposal
+
+
+Because the number of spare types available in the P2M type-space is
+currently very limited it is proposed that HVMMEM\_mmio\_write\_dm be
+replaced by a single new type HVMMEM\_ioreq\_server. In future, if the
+P2M type-space is increased, this can be renamed to HVMMEM\_ioreq\_server0
+and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc. types
+can be added.
+
+Accesses to a page of type HVMMEM\_ioreq\_serverX should be the same as
+HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server. Furthermore


Sorry, do you mean even when a gfn is set to HVMMEM_ioreq_serverX type,
its access rights in P2M still remains unchanged? So the new hypercall
pair, HVMOP_[un]map_mem_type_to_ioreq_server, are also responsible for
the PTE updates on the access bits?

If it is true, I'm afraid this would be time consuming, because the
map/unmap will have to traverse all P2M structures to detect the PTEs
with HVMMEM_ioreq_serverX flag set. Yet in XenGT, setting this flag is
triggered dynamically with the construction/destruction of shadow PPGTT.
But I'm not sure to which degree the performance casualties will be,
with frequent EPT table walk and EPT tlb flush.


No walking of EPT trees will be necessary in that case, just like it
already has been made unnecessary for other changes resulting
in various PTE attributes needing re-calculation. We'll only need
to extend the p2m_memory_type_changed() mechanism to cover
changes like this one.


So you mean when the access bits are to be updated, we can leverage
something like p2m_memory_type_changed(which I guess only deals with
memory types, not access bits) to avoid the walking of EPT trees? I'll
need to study this part.
Anyway, thanks for your advice. :)

B.R.
Yu

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

Re: [Xen-devel] [PATCH v5 08/22] arm/acpi: Parse MADT to map logical cpu to MPIDR and get cpu_possible_map

[Jan Beulich]

>>> On 26.02.16 at 07:22,  wrote:
> --- a/xen/include/xen/acpi.h
> +++ b/xen/include/xen/acpi.h
> @@ -39,6 +39,10 @@
>  #define ACPI_MADT_GET_POLARITY(inti) ACPI_MADT_GET_(POLARITY, inti)
>  #define ACPI_MADT_GET_TRIGGER(inti)  ACPI_MADT_GET_(TRIGGER, inti)
>  
> +#define BAD_MADT_ENTRY(entry, end) (\
> +(!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > 
> (end) ||  \
> +((struct acpi_subtable_header *)(entry))->length < 
> sizeof(*(entry)))

Would it be too much to ask for you to eliminate the bogus cast
here, in favor of properly using (entry)->header.length? Afaics
this should be fine for all users of the macro.

Jan


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

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Paul Durrant]

> -Original Message-
> From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
> Sent: 26 February 2016 09:37
> To: Jan Beulich
> Cc: Paul Durrant; xen-de...@lists.xenproject.org
> Subject: Re: [Xen-devel] [PATCH] docs/design: introduce
> HVMMEM_ioreq_serverX types
> 
> On 2/26/2016 5:18 PM, Jan Beulich wrote:
>  On 26.02.16 at 07:59,  wrote:
> >>> +Proposal
> >>> +
> >>> +
> >>> +Because the number of spare types available in the P2M type-space is
> >>> +currently very limited it is proposed that
> HVMMEM\_mmio\_write\_dm be
> >>> +replaced by a single new type HVMMEM\_ioreq\_server. In future, if
> the
> >>> +P2M type-space is increased, this can be renamed to
> HVMMEM\_ioreq\_server0
> >>> +and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc.
> types
> >>> +can be added.
> >>> +
> >>> +Accesses to a page of type HVMMEM\_ioreq\_serverX should be the
> same as
> >>> +HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server.
> Furthermore
> >>
> >> Sorry, do you mean even when a gfn is set to HVMMEM_ioreq_serverX
> type,
> >> its access rights in P2M still remains unchanged? So the new hypercall
> >> pair, HVMOP_[un]map_mem_type_to_ioreq_server, are also
> responsible for
> >> the PTE updates on the access bits?
> >>
> >> If it is true, I'm afraid this would be time consuming, because the
> >> map/unmap will have to traverse all P2M structures to detect the PTEs
> >> with HVMMEM_ioreq_serverX flag set. Yet in XenGT, setting this flag is
> >> triggered dynamically with the construction/destruction of shadow
> PPGTT.
> >> But I'm not sure to which degree the performance casualties will be,
> >> with frequent EPT table walk and EPT tlb flush.
> >
> > No walking of EPT trees will be necessary in that case, just like it
> > already has been made unnecessary for other changes resulting
> > in various PTE attributes needing re-calculation. We'll only need
> > to extend the p2m_memory_type_changed() mechanism to cover
> > changes like this one.
> 
> So you mean when the access bits are to be updated, we can leverage
> something like p2m_memory_type_changed(which I guess only deals with
> memory types, not access bits) to avoid the walking of EPT trees? I'll
> need to study this part.

No, the P2M is walked when the map/unmap hypercall is issued but, in the XenGT 
use-case, that hypercall is issued once at start of day and - if everything is 
working as it I believe it should - there won't actually be any pages of type 
HVMMEM_ioreq_server at that point, so no EPT flush is required.

> Anyway, thanks for your advice. :)

I will post an implementation hopefully in the next few days once I've proved 
it works in my XenGT rig.

  Paul

> 
> B.R.
> Yu

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

Re: [Xen-devel] [PATCH v5 10/22] acpi/table: Introduce acpi_table_get_entry_madt to get specified entry

[Jan Beulich]

>>> On 26.02.16 at 07:22,  wrote:
> From: Shannon Zhao 
> 
> This function could get the specified index entry of MADT table. This
> would be useful when it needs to get the contens of the entry.
> 
> Cc: Jan Beulich 
> Signed-off-by: Shannon Zhao 
> ---
> V5: address Jan's comments
> V4: Fix coding style and make the function only for MADT table
> ---
>  xen/drivers/acpi/tables.c | 62 
> +++
>  xen/include/xen/acpi.h|  2 ++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/xen/drivers/acpi/tables.c b/xen/drivers/acpi/tables.c
> index f81c369..fa4e666 100644
> --- a/xen/drivers/acpi/tables.c
> +++ b/xen/drivers/acpi/tables.c
> @@ -221,6 +221,68 @@ void __init acpi_table_print_madt_entry(struct 
> acpi_subtable_header *header)
>   }
>  }
>  
> +static struct acpi_subtable_header * __init
> +acpi_get_entry(const char *id, unsigned long table_size,
> +const struct acpi_table_header *table_header,
> +enum acpi_madt_type entry_id, unsigned int entry_index)
> +{
> + struct acpi_subtable_header *entry;
> + int count = 0;
> + unsigned long table_end;
> +
> + if (!table_size)
> + return NULL;
> +
> + if (!table_header) {
> + printk(KERN_WARNING PREFIX "%4.4s not present\n", id);
> + return NULL;
> + }
> +
> + table_end = (unsigned long)table_header + table_header->length;
> +
> + /* Parse all entries looking for a match. */
> + entry = (struct acpi_subtable_header *)
> + ((unsigned long)table_header + table_size);
> +
> + while (((unsigned long)entry) + sizeof(struct acpi_subtable_header) <

Isn't this simply (unsigned long)(entry + 1)?

> +table_end) {
> + if (entry->length < sizeof(*entry)) {
> + printk(KERN_ERR PREFIX "[%4.4s:%#x] Invalid length\n",
> +id, entry_id);
> + return NULL;
> + }
> +
> + if (entry->type == entry_id) {
> + if (count == entry_index)
> + return entry;
> + count++;
> + }
> +
> + entry = (struct acpi_subtable_header *)
> + ((unsigned long)entry + entry->length);

While a cast indeed is unavoidable here, this could also be simplified to

entry = (void *)entry + entry->length;

afaict. Same, as I see only now, goes for the calculation right
ahead of the loop.

> +struct acpi_subtable_header * __init
> +acpi_table_get_entry_madt(enum acpi_madt_type entry_id,
> +   unsigned int entry_index)
> +{
> + struct acpi_table_header *table_header = NULL;

Afaict the initializer is not needed, since ...

> + acpi_status status;
> +
> + status = acpi_get_table(ACPI_SIG_MADT, acpi_apic_instance,
> + &table_header);
> + if (ACPI_FAILURE(status)) {

... you properly check the status returned.

Jan


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

Re: [Xen-devel] [PATCH v5 20/22] arm/acpi: Initialize serial port from ACPI SPCR table

[Jan Beulich]

>>> On 26.02.16 at 07:22,  wrote:
> From: Shannon Zhao 
> 
> Parse ACPI SPCR (Serial Port Console Redirection table) table and
> initialize the serial port pl011.
> 
> Signed-off-by: Parth Dixit 
> Signed-off-by: Shannon Zhao 
> Reviewed-by: Stefano Stabellini 

Acked-by: Jan Beulich 

It's not obvious to me whether this could go in ahead of most of
the rest of the series, together with patch 19 (which clearly can
go in right away).

Jan


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

Re: [Xen-devel] [PATCH] docs/design: introduce HVMMEM_ioreq_serverX types

[Yu, Zhang]

On 2/26/2016 5:50 PM, Paul Durrant wrote:

-Original Message-
From: Yu, Zhang [mailto:yu.c.zh...@linux.intel.com]
Sent: 26 February 2016 09:37
To: Jan Beulich
Cc: Paul Durrant; xen-de...@lists.xenproject.org
Subject: Re: [Xen-devel] [PATCH] docs/design: introduce
HVMMEM_ioreq_serverX types

On 2/26/2016 5:18 PM, Jan Beulich wrote:

On 26.02.16 at 07:59,  wrote:

+Proposal
+
+
+Because the number of spare types available in the P2M type-space is
+currently very limited it is proposed that

HVMMEM\_mmio\_write\_dm be

+replaced by a single new type HVMMEM\_ioreq\_server. In future, if

the

+P2M type-space is increased, this can be renamed to

HVMMEM\_ioreq\_server0

+and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc.

types

+can be added.
+
+Accesses to a page of type HVMMEM\_ioreq\_serverX should be the

same as

+HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server.

Furthermore


Sorry, do you mean even when a gfn is set to HVMMEM_ioreq_serverX

type,

its access rights in P2M still remains unchanged? So the new hypercall
pair, HVMOP_[un]map_mem_type_to_ioreq_server, are also

responsible for

the PTE updates on the access bits?

If it is true, I'm afraid this would be time consuming, because the
map/unmap will have to traverse all P2M structures to detect the PTEs
with HVMMEM_ioreq_serverX flag set. Yet in XenGT, setting this flag is
triggered dynamically with the construction/destruction of shadow

PPGTT.

But I'm not sure to which degree the performance casualties will be,
with frequent EPT table walk and EPT tlb flush.


No walking of EPT trees will be necessary in that case, just like it
already has been made unnecessary for other changes resulting
in various PTE attributes needing re-calculation. We'll only need
to extend the p2m_memory_type_changed() mechanism to cover
changes like this one.


So you mean when the access bits are to be updated, we can leverage
something like p2m_memory_type_changed(which I guess only deals with
memory types, not access bits) to avoid the walking of EPT trees? I'll
need to study this part.


No, the P2M is walked when the map/unmap hypercall is issued but, in the XenGT 
use-case, that hypercall is issued once at start of day and - if everything is 
working as it I believe it should - there won't actually be any pages of type 
HVMMEM_ioreq_server at that point, so no EPT flush is required.


Anyway, thanks for your advice. :)


I will post an implementation hopefully in the next few days once I've proved 
it works in my XenGT rig.


Great. Looking forward to this implementation, and thanks for your
help. :)

B.R.
Yu


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

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Xu, Quan]

> On February 26, 2016 4:15pm,   wrote:
> >>> On 26.02.16 at 08:37,  wrote:
> > On February 25, 2016 8:24pm,  wrote:
> >> >>> On 25.02.16 at 13:14,  wrote:
> >> > On February 25, 2016 4:59pm,  wrote:


> Nevertheless I'd recommend not mixing things for the pcidevs one, as its uses
> are scattered around too much for it to be reasonably possible to prove
> correctness if done that way.


> Instead please make the lock a static variable in e.g.
> xen/drivers/pci/pci.c or xen/drivers/passthrough/pci.c, and force
> acquire/release to go through helper functions. That way we can ensure
> instance gets left. The only safe alternative to this would be to rename the 
> lock
> at once, or to make it read/write one (but then recursion would be allowed 
> only
> for the read cases, and aiui you'd need the write variant for your use).
> 

Jan, sorry, I don't follow this comment. Is it necessary for v6 patch set?

Quan

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

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Jan Beulich]

>>> On 26.02.16 at 10:24,  wrote:
> On February 26, 2016 4:15pm,  wrote:
>> >>> On 26.02.16 at 08:37,  wrote:
>> > On February 25, 2016 8:24pm,  wrote:
>> >> >>> On 25.02.16 at 13:14,  wrote:
>> >> > On February 25, 2016 4:59pm,  wrote:
>> >
>> >> >> However, the same effect could be achieved by making the lock a
>> >> >> recursive one, which would then seem to more conventional approach
>> >> >> (but requiring as much code to be touched).
>> >
>> > IMO, for v6, I'd better:
>> >   (1). replace all of 'spin_lock(&pcidevs_lock)' with
>> > 'spin_lock_recursive(&pcidevs_lock)',
>> >   (2). replace all of 'spin_unlock(&pcidevs_lock)' with
>> > 'spin_unlock_recursive(&pcidevs_lock)',
>> >   (3). _do_not_ touch 'spin_is_locked(&pcidevs_lock)'.
>> > Then, ignore v5 [patch4/7], [patch5/7], [patch6/7],  *_locked()
>> > related code,  and add a new patch for the above (1). (2). (3). Right?
>> 
>> Yes.
>> 
>> > BTW, .e.g. 'd->page_alloc_lock', I found the 'd->page_alloc_lock' is
>> > called by both spin_lock_recursive() and 'spin_lock()'.
>> > _If_  both spin_lock_recursive(&d->page_alloc_lock) and
>> > 'spin_lock(&d->page_alloc_lock)' are recursively called in same call
>> > tree as below, it might be a bug. Right?
>> >
>> >
>> > {
>> > ...
>> > spin_lock()
>> > ...
>> >spin_lock_recursive()
>> >...
>> >spin_unlock_recursive()  <--- (lock might be now free)
>> > ...
>> > spin_unlock()
>> > ...
>> > }
>> 
>> Well, such a use of course would be a bug. But using plain
>> spin_lock() on a path where recursive acquire can't occur is fine.
>> 
>> Nevertheless I'd recommend not mixing things for the pcidevs one, as its uses
>> are scattered around too much for it to be reasonably possible to prove
>> correctness if done that way.
> 
> 
> But I do _not_ mix things for the pcidevs one. As 
>   (1). replace ___all___ of 'spin_lock(&pcidevs_lock)' with 
> 'spin_lock_recursive(&pcidevs_lock)',
>   (2). replace ___all___ of 'spin_unlock(&pcidevs_lock)' with 
> 'spin_unlock_recursive(&pcidevs_lock)',

I am not saying you mix things, I'm just saying there is a risk you
do perhaps not even knowingly, e.g. when another patch goes in
about the same time as yours which adds another instance. Plus
doing what you state above is going to be close to unreviewable,
since the reviewer would have to check that indeed you caught
all instances. By removing global visibility of the lock variable both
issues are easily avoided, since without catching all cases a build
error would result.

Jan


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

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Jan Beulich]

>>> On 26.02.16 at 11:08,  wrote:
>>  On February 26, 2016 4:15pm,   wrote:
>> >>> On 26.02.16 at 08:37,  wrote:
>> > On February 25, 2016 8:24pm,  wrote:
>> >> >>> On 25.02.16 at 13:14,  wrote:
>> >> > On February 25, 2016 4:59pm,  wrote:
>> Nevertheless I'd recommend not mixing things for the pcidevs one, as its uses
>> are scattered around too much for it to be reasonably possible to prove
>> correctness if done that way.
> 
> 
>> Instead please make the lock a static variable in e.g.
>> xen/drivers/pci/pci.c or xen/drivers/passthrough/pci.c, and force
>> acquire/release to go through helper functions. That way we can ensure
>> instance gets left. The only safe alternative to this would be to rename the 
>> lock
>> at once, or to make it read/write one (but then recursion would be allowed 
>> only
>> for the read cases, and aiui you'd need the write variant for your use).
> 
> Jan, sorry, I don't follow this comment. Is it necessary for v6 patch set?

See my other reply just sent.

Jan


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

Re: [Xen-devel] [PATCH v5 08/22] arm/acpi: Parse MADT to map logical cpu to MPIDR and get cpu_possible_map

[Shannon Zhao]

On 2016/2/26 17:49, Jan Beulich wrote:
 On 26.02.16 at 07:22,  wrote:
>> > --- a/xen/include/xen/acpi.h
>> > +++ b/xen/include/xen/acpi.h
>> > @@ -39,6 +39,10 @@
>> >  #define ACPI_MADT_GET_POLARITY(inti)  ACPI_MADT_GET_(POLARITY, inti)
>> >  #define ACPI_MADT_GET_TRIGGER(inti)   ACPI_MADT_GET_(TRIGGER, inti)
>> >  
>> > +#define BAD_MADT_ENTRY(entry, end) (  
>> >   \
>> > +(!(entry)) || (unsigned long)(entry) + sizeof(*(entry)) > 
>> > (end) ||  \
>> > +((struct acpi_subtable_header *)(entry))->length < 
>> > sizeof(*(entry)))
> Would it be too much to ask for you to eliminate the bogus cast
> here, in favor of properly using (entry)->header.length? Afaics
> this should be fine for all users of the macro.
Oh, sorry, will update this.

Thanks,
-- 
Shannon


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

Re: [Xen-devel] [PATCH v5 20/22] arm/acpi: Initialize serial port from ACPI SPCR table

[Shannon Zhao]

On 2016/2/26 18:04, Jan Beulich wrote:
 On 26.02.16 at 07:22,  wrote:
>> > From: Shannon Zhao 
>> > 
>> > Parse ACPI SPCR (Serial Port Console Redirection table) table and
>> > initialize the serial port pl011.
>> > 
>> > Signed-off-by: Parth Dixit 
>> > Signed-off-by: Shannon Zhao 
>> > Reviewed-by: Stefano Stabellini 
> Acked-by: Jan Beulich 
> 
Thanks a lot!

> It's not obvious to me whether this could go in ahead of most of
> the rest of the series, together with patch 19 (which clearly can
> go in right away).

I think this one could go in ahead with patch 19 since it's independent.

Thanks,
-- 
Shannon


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

[Xen-devel] [PATCH v2] docs/design: introduce HVMMEM_ioreq_serverX types

[Paul Durrant]

This patch adds a new 'designs' subdirectory under docs as a repository
for this and future design proposals. It also adjusts the docs
Makefile to look in the new subdirectory.

Signed-off-by: Paul Durrant 
---

For convenience this document can also be viewed in PDF at:

http://xenbits.xen.org/people/pauldu/hvmmem_ioreq_server.pdf

v2:
 - Added the Makefile modification
 - Addressed various review comments on v1 of the design
 - Small amount of re-formatting
---
 docs/Makefile |  8 ++--
 docs/designs/hvmmem_ioreq_server.markdown | 66 +++
 2 files changed, 71 insertions(+), 3 deletions(-)
 create mode 100755 docs/designs/hvmmem_ioreq_server.markdown

diff --git a/docs/Makefile b/docs/Makefile
index b9da605..c365fdc 100644
--- a/docs/Makefile
+++ b/docs/Makefile
@@ -11,11 +11,13 @@ MAN1SRC-y := $(sort $(shell find man/ -name '*.pod.1' 
-print))
 MAN5SRC-y := $(sort $(shell find man/ -name '*.pod.5' -print))
 MAN8SRC-y := $(sort $(shell find man/ -name '*.pod.8' -print))
 
-MARKDOWNSRC-y := $(sort $(shell find misc -name '*.markdown' -print))
+SUBDIRS := features/ misc/ specs/ designs/
 
-TXTSRC-y := $(sort $(shell find misc -name '*.txt' -print))
+MARKDOWNSRC-y := $(sort $(shell find $(SUBDIRS) -name '*.markdown' -print))
 
-PANDOCSRC-y := $(sort $(shell find features/ misc/ specs/ -name '*.pandoc' 
-print))
+TXTSRC-y := $(sort $(shell find $(SUBDIRS) -name '*.txt' -print))
+
+PANDOCSRC-y := $(sort $(shell find $(SUBDIRS) -name '*.pandoc' -print))
 
 # Documentation targets
 DOC_MAN1 := $(patsubst man/%.pod.1,man1/%.1,$(MAN1SRC-y))
diff --git a/docs/designs/hvmmem_ioreq_server.markdown 
b/docs/designs/hvmmem_ioreq_server.markdown
new file mode 100755
index 000..0223b5d
--- /dev/null
+++ b/docs/designs/hvmmem_ioreq_server.markdown
@@ -0,0 +1,66 @@
+# HVMMEM\_ioreq\_serverX
+
+
+## Background
+
+The concept of the IOREQ server was introduced to allow multiple distinct
+device emulators to a single VM. The XenGT project uses an IOREQ server to
+provide mediated pass-through of Intel GPUs to guests and, as part of the
+mediation, needs to intercept accesses to GPU page-tables (or GTTs) that
+reside in guest RAM.
+
+The current implementation of this sets the type of GTT pages to type
+HVMMEM\_mmio\_write\_dm, which causes Xen to emulate writes to such pages,
+and then maps the guest physical addresses of those pages to the XenGT
+IOREQ server using the HVMOP\_map\_io\_range\_to\_ioreq\_server hypercall.
+However, because the number of GTTs is potentially large, using this
+approach does not scale well.
+
+##Proposal
+
+Because the number of spare types available in the P2M type-space is
+currently very limited it is proposed that HVMMEM\_mmio\_write\_dm be
+replaced by a single new type HVMMEM\_ioreq\_server. In future, if the
+P2M type-space is increased, this can be renamed to HVMMEM\_ioreq\_server0
+and new HVMMEM\_ioreq\_server1, HVMMEM\_ioreq\_server2, etc. types
+can be added.
+
+Accesses to a page of type HVMMEM\_ioreq\_serverX should be the same as
+HVMMEM\_ram\_rw until the type is _claimed_ by an IOREQ server. Furthermore
+it should only be possible to change the type of a page to
+HVMMEM\_ioreq\_serverX if that page is currently of type HVMMEM\_ram\_rw, and
+if the page is of type HVMMEM\_ioreq\_serverX it should only be possible to
+change the type to HVMMEM\_ram\_rw.
+
+To allow an IOREQ server to claim or release a claim to a type a hypercall
+will be introduced:
+
+- HVMOP\_map\_mem\_type\_to\_ioreq\_server
+
+and an associated argument structure:
+
+   struct hvm_ioreq_mem_type {
+   domid_t domid;  /* IN - domain to be serviced */
+   ioservid_t id;  /* IN - server id */
+   hvmmem_type_t type; /* IN - memory type */
+   uint32_t flags; /* IN - types of access to be
+   intercepted */
+
+   #define _HVMOP_IOREQ_MEM_ACCESS_READ 0
+   #define HVMOP_IOREQ_MEM_ACCESS_READ \
+   (1u << _HVMOP_IOREQ_MEM_ACCESS_READ)
+
+   #define _HVMOP_IOREQ_MEM_ACCESS_WRITE 1
+   #define HVMOP_IOREQ_MEM_ACCESS_WRITE \
+   (1u << _HVMOP_IOREQ_MEM_ACCESS_WRITE)
+
+   };
+
+
+Once the type has been claimed then the requested types of access to any
+page of the claimed type will be passed to the IOREQ server for handling.
+Only HVMMEM\_ioreq\_serverX types may be claimed and only one IOREQ
+server may have a claim on a type (`-EBUSY` will be returned if an attempt
+is made to claim a type that is already claimed).
+To release a claim the hypercall is issued with the `flags` field set to
+zero.
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Jan Beulich]

>>> On 25.02.16 at 17:50,  wrote:
> @@ -670,7 +676,13 @@ int cpu_disable_scheduler(unsigned int cpu)
>  if ( cpumask_empty(&online_affinity) &&
>   cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
>  {
> -printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v);
> +if ( v->affinity_broken )
> +{
> +/* The vcpu is temporarily pinned, can't move it. */
> +vcpu_schedule_unlock_irqrestore(lock, flags, v);
> +ret = -EBUSY;
> +continue;
> +}

So far the function can only return 0 or -EAGAIN. By using "continue"
here you will make it impossible for the caller to reliably determine
whether possibly both things failed. Despite -EBUSY being a logical
choice here, I think you'd better use -EAGAIN here too. And it needs
to be determined whether continuing the loop in this as well as the
pre-existing cases is actually the right thing to do.

> @@ -679,6 +691,8 @@ int cpu_disable_scheduler(unsigned int cpu)
>  v->affinity_broken = 1;
>  }
>  
> +printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v);

Wouldn't it be even better to make this the "else" to the
preceding if(), since in the suspend case this is otherwise going
to be printed for every vCPU not currently running on pCPU0?

> @@ -753,14 +767,22 @@ static int vcpu_set_affinity(
>  struct vcpu *v, const cpumask_t *affinity, cpumask_t *which)
>  {
>  spinlock_t *lock;
> +int ret = 0;
>  
>  lock = vcpu_schedule_lock_irq(v);
>  
> -cpumask_copy(which, affinity);
> +if ( v->affinity_broken )
> +{
> +ret = -EBUSY;
> +}

Unnecessary braces.

> @@ -979,6 +1001,53 @@ void watchdog_domain_destroy(struct domain *d)
>  kill_timer(&d->watchdog_timer[i]);
>  }
>  
> +static long do_pin_temp(int cpu)
> +{
> +struct vcpu *v = current;
> +spinlock_t *lock;
> +long ret = -EINVAL;
> +
> +lock = vcpu_schedule_lock_irq(v);
> +
> +if ( cpu == -1 )
> +{
> +if ( v->affinity_broken )
> +{
> +cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved);
> +v->affinity_broken = 0;
> +set_bit(_VPF_migrating, &v->pause_flags);
> +ret = 0;
> +}
> +}
> +else if ( cpu < nr_cpu_ids && cpu >= 0 )

Perhaps easier to simply use "cpu < 0" in the first if()?

> +{
> +if ( v->affinity_broken )
> +{
> +ret = -EBUSY;
> +}
> +else if ( cpumask_test_cpu(cpu, VCPU2ONLINE(v)) )
> +{

This is a rather ugly restriction: How would a caller fulfill its job
when this is not the case?

> @@ -1088,6 +1157,23 @@ ret_t do_sched_op(int cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  break;
>  }
>  
> +case SCHEDOP_pin_temp:
> +{
> +struct sched_pin_temp sched_pin_temp;
> +
> +ret = -EFAULT;
> +if ( copy_from_guest(&sched_pin_temp, arg, 1) )
> +break;
> +
> +ret = xsm_schedop_pin_temp(XSM_PRIV);
> +if ( ret )
> +break;
> +
> +ret = do_pin_temp(sched_pin_temp.pcpu);
> +
> +break;
> +}

So having come here I still don't see why this is called "temp":
Nothing enforces this to be a temporary state, and hence the
sub-op name currently is actively misleading.

> --- a/xen/include/public/sched.h
> +++ b/xen/include/public/sched.h
> @@ -118,6 +118,15 @@
>   * With id != 0 and timeout != 0, poke watchdog timer and set new timeout.
>   */
>  #define SCHEDOP_watchdog6
> +
> +/*
> + * Temporarily pin the current vcpu to one physical cpu or undo that pinning.
> + * @arg == pointer to sched_pin_temp_t structure.
> + *
> + * Setting pcpu to -1 will undo a previous temporary pinning.
> + * This call is allowed for domains with domain control privilege only.
> + */

Why domain control privilege? I'd actually suggest limiting the
ability to the hardware domain, at once eliminating the need
for the XSM check.

> +struct sched_pin_temp {
> +int pcpu;

Fixed width types only please in the public interface. Also this needs
an entry in xen/include/xlat.lst, and a consumer of the resulting
check macro.

Jan

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

Re: [Xen-devel] [PATCH] docs: spell out limits of security support for qemu-xen

[Lars Kurth]

On 26/02/2016 02:52, "Doug Goldstein"  wrote:

>On 2/25/16 9:43 AM, Stefano Stabellini wrote:
>
>> +++ b/docs/misc/qemu-xen-security
>> @@ -0,0 +1,20 @@
>> +qemu-xen (git://xenbits.xen.org/qemu-xen.git) is only supported for
>> +security fixes when used together with the Xen hypervisor and only with
>> +a subset of all the possible QEMU emulators. Specifically:
>
>So I'll get my comments on paper here rather than something just
>mentioned on IRC. This is exactly why the Xen team should be pushing to
>remove as many "in-tree" items as possible.

I am wondering, whether making dependencies explicit would also make the
life of Xen based distributions easier. That is obviously something we
should verify.

>The security surface area of
>Xen is huge and statements like this help the CYA factor they don't
>completely eliminate the problems of manpower of having to check against
>different upstreams if a vulnerability affects you or downstreams doing
>something bad causing a security issue for users which ultimately gets
>blamed on Xen. 

I agree, we are seeing more and more of this. Having clearer boundaries
and changing the model, would clearly help managing the increasingly bad
press coverage we are getting. For example, if you look at XSA's the rate
of XSA's we are discovering per year has been relatively stable per year,
but I have a hunch that it is actually decreasing if you remove QEMU and
other 3rd party XSA's we are handling.

>There are then further complications where sometimes the
>version shipped by Xen isn't an upstream release and so there may be
>other vulnerabilities above and beyond what upstream announces.
>
>I urge the Xen maintainers to make it a goal to remove external
>libraries and applications (like qemu-xen) from the tree entirely and
>recommend the use of the upstream release. I know the concern is testing
>but it involves calling out your dependencies just like you do any other
>dependency. (e.g. Xen X.Y requires QEMU A.B.C, no guarantees are made
>about the compatibility of other versions)
>
>I know Stefano is making an effort with this with Project Raisin and
>really that should become the embraced way to stand up a "full" Xen
>system from source rather than a hodge podge collection of packages that
>are fetched by the Xen build system. This will bring the how developers
>use the source packages closer with how many users of distros use Xen
>(e.g. a number of distros use upstream QEMU releases instead of qemu-xen).

I guess there is a debate to be have. Maybe this is something we can
discuss on here and next steps at the Hackathon. Konrad already put a
discussion in at 
http://wiki.xenproject.org/wiki/Hackathon/April2016#Security

Regards
Lars

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

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

[Andrei Borzenkov]

On Fri, Feb 26, 2016 at 8:59 AM, Fu Wei  wrote:
> +@subsection xen_module
>
> -@deffn Command xen_linux file [arguments]
> -Load a dom0 kernel image for xen hypervisor at the booting process of 
> xen.
> +@deffn Command xen_module [--nounzip] file [arguments]
> +Load a module for xen hypervisor at the booting process of xen.
> The rest of the line is passed verbatim as the module command line.
> +Each module will be identified by the order in which the modules are 
> added.
> +The 1st module: dom0 kernel image
> +The 2nd module: dom0 ramdisk
> +All subsequent modules: UNKNOW
> @end deffn

 Hmm ... from previous discussion I gathered that Xen can detect module
 type. What if there is no initrd for dom0? How can subsequent modules be
>>>
>>> Now , Xen detect module type by the order. (at least on ARM64).
>>> I think i386 is using Multiboot(2) protocol, so maybe this order is
>>> nothing to do with i386.
>>>
>>
>> Then we have obvious problem with your XSM patch 
>> (http://savannah.gnu.org/bugs/?43420) - XSM may land as the first module. 
>> That's actually something to solve on Xen side I think. It's just that so 
>> far we had just kernel and initrd, so that was non issue.
>
> Oh, did you mean Wei Liu's patch?
>
> I guess XSM may land as the third module (or the module after linux
> kernel, if you don't have initrd)
>
> Yes, agree. (That's actually something to solve on Xen side)
>
> I guess xen can get xsm from a special initrd. so for now there is not
> big problem on xsm.
>
> Please correct me if I misunderstand something. :-)
>
> Thanks!
>
> Back to this patch, is that OK for you, or any suggestion?  Thanks !
>

Yes, as this is dedicated Xen loader we should document this mandatory
order - first module must be kernel image, second module must be
initrd. I do not think we need to mention possibility to load more
than two modules until there is clear understanding how it can be done
without initrd.

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

Re: [Xen-devel] [PATCH v3 0/2] Clear .bss for VP guests

[Roger Pau Monné]

El 25/2/16 a les 16:16, Boris Ostrovsky ha escrit:
> PV guests need to have their .bss zeroed out since it is not guaranteed
> to be cleared by Xen's domain builder

I guess I'm missing something, but elf_load_image (in libelf-loader.c)
seems to be able to clear segments (it will zero the memory between
p_paddr + p_filesz and p_paddr + p_memsz) while loading the ELF into
memory, so if the program headers are correctly setup the .bss should be
zeroed out AFAICT.

Roger.



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

[Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Corneliu ZUZU]

This patch adds ARM support for guest-request monitor vm-events.
Note: on ARM hypercall instruction skipping must be done manually
by the caller. This will probably be changed in a future patch.

Summary of changes:
== Moved to common-side:
  * XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST handling (moved from X86
  arch_monitor_domctl_event to common monitor_domctl)
  * hvm_event_guest_request->vm_event_monitor_guest_request
  * hvm_event_traps->vm_event_monitor_traps (also added target vcpu as param)
  * guest-request bits from X86 'struct arch_domain' (to common 'struct domain')
== ARM implementations:
  * do_hvm_op now handling of HVMOP_guest_request_vm_event => calls
  vm_event_monitor_guest_request (as on X86)
  * arch_monitor_get_capabilities->vm_event_monitor_get_capabilities,
updated to reflect support for XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST
  * vm_event_init_domain (does nothing), vm_event_cleanup_domain
== Misc:
  * vm_event_fill_regs, no longer X86-specific. ARM-side implementation of this
  function currently does nothing, that will be added in a separate patch.

Signed-off-by: Corneliu ZUZU 
---
Changed since v1:
  * hvm_event_traps, hvm_event_guest_request moved to common/vm_event.c and
  and renamed to vm_event_monitor_traps and vm_event_monitor_guest_request
  * arch_monitor_get_capabilities moved to vm_event.h and renamed to
  vm_event_monitor_get_capabilities
  * arch_hvm_event_fill_regs replaced w/ existing vm_event_fill_regs
  (see commit adc75eba8b15c7103a010f736fe62e3fb2383964)
  * defined altp2m_active for ARM and altp2m_vcpu_idx to remove #ifdef in
  vm_event_monitor_traps
  * change bitfield members type to unsigned int
---
 xen/arch/arm/hvm.c  |  8 
 xen/arch/x86/hvm/event.c| 82 -
 xen/arch/x86/hvm/hvm.c  |  3 +-
 xen/arch/x86/monitor.c  | 18 +
 xen/arch/x86/vm_event.c |  1 +
 xen/common/monitor.c| 36 +++---
 xen/common/vm_event.c   | 60 ++
 xen/include/asm-arm/altp2m.h| 39 
 xen/include/asm-arm/monitor.h   |  8 +---
 xen/include/asm-arm/vm_event.h  | 19 +-
 xen/include/asm-x86/altp2m.h| 10 +++--
 xen/include/asm-x86/domain.h|  4 +-
 xen/include/asm-x86/hvm/event.h | 13 +--
 xen/include/asm-x86/monitor.h   | 23 
 xen/include/asm-x86/vm_event.h  | 23 
 xen/include/xen/sched.h |  6 +++
 xen/include/xen/vm_event.h  |  9 +
 17 files changed, 232 insertions(+), 130 deletions(-)
 create mode 100644 xen/include/asm-arm/altp2m.h

diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
index 056db1a..c01123a 100644
--- a/xen/arch/arm/hvm.c
+++ b/xen/arch/arm/hvm.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -72,6 +73,13 @@ long do_hvm_op(unsigned long op, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 break;
 }
 
+case HVMOP_guest_request_vm_event:
+if ( guest_handle_is_null(arg) )
+vm_event_monitor_guest_request();
+else
+rc = -EINVAL;
+break;
+
 default:
 {
 gdprintk(XENLOG_DEBUG, "HVMOP op=%lu: not implemented\n", op);
diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
index d0b7d90..56c5514 100644
--- a/xen/arch/x86/hvm/event.c
+++ b/xen/arch/x86/hvm/event.c
@@ -6,6 +6,7 @@
  * Copyright (c) 2004, Intel Corporation.
  * Copyright (c) 2005, International Business Machines Corporation.
  * Copyright (c) 2008, Citrix Systems, Inc.
+ * Copyright (c) 2016, Bitdefender S.R.L.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -21,71 +22,32 @@
  */
 
 #include 
-#include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
-static int hvm_event_traps(uint8_t sync, vm_event_request_t *req)
-{
-int rc;
-struct vcpu *curr = current;
-struct domain *currd = curr->domain;
-
-rc = vm_event_claim_slot(currd, &currd->vm_event->monitor);
-switch ( rc )
-{
-case 0:
-break;
-case -ENOSYS:
-/*
- * If there was no ring to handle the event, then
- * simply continue executing normally.
- */
-return 1;
-default:
-return rc;
-};
-
-if ( sync )
-{
-req->flags |= VM_EVENT_FLAG_VCPU_PAUSED;
-vm_event_vcpu_pause(curr);
-}
-
-if ( altp2m_active(currd) )
-{
-req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
-req->altp2m_idx = vcpu_altp2m(curr).p2midx;
-}
-
-vm_event_fill_regs(req);
-vm_event_put_request(currd, &currd->vm_event->monitor, req);
-
-return 1;
-}
-
 bool_t hvm_event_cr(unsigned int index, unsigned long value, unsigned long old)
 {
-struct arch_domain *currad = ¤t->domain->arch;
+struct vcpu *curr = current;
+struct arch_domain *ad = &curr->domain->arch;
 

[Xen-devel] [PATCH v3 1/4] i386, xen: Add xen_hypervisor and xen_module aliases for i386

[fu . wei]

From: Fu Wei 

Signed-off-by: Fu Wei 
---
 grub-core/loader/i386/xen.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index c4d9689..15b0727 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -689,6 +689,7 @@ fail:
 }
 
 static grub_command_t cmd_xen, cmd_initrd, cmd_module, cmd_multiboot;
+static grub_command_t cmd_xen_hypervisor, cmd_xen_module;
 
 GRUB_MOD_INIT (xen)
 {
@@ -696,10 +697,14 @@ GRUB_MOD_INIT (xen)
   0, N_("Load Linux."));
   cmd_multiboot = grub_register_command ("multiboot", grub_cmd_xen,
 0, N_("Load Linux."));
+  cmd_xen_hypervisor = grub_register_command ("xen_hypervisor", grub_cmd_xen,
+ 0, N_("Load Linux."));
   cmd_initrd = grub_register_command ("initrd", grub_cmd_initrd,
  0, N_("Load initrd."));
   cmd_module = grub_register_command ("module", grub_cmd_module,
  0, N_("Load module."));
+  cmd_xen_module = grub_register_command ("xen_module", grub_cmd_module,
+ 0, N_("Load module."));
   my_mod = mod;
 }
 
@@ -709,4 +714,6 @@ GRUB_MOD_FINI (xen)
   grub_unregister_command (cmd_initrd);
   grub_unregister_command (cmd_multiboot);
   grub_unregister_command (cmd_module);
+  grub_unregister_command (cmd_xen_module);
+  grub_unregister_command (cmd_xen_hypervisor);
 }
-- 
2.5.0


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

[Xen-devel] [PATCH v3 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

[fu . wei]

From: Fu Wei 

delete: xen_linux, xen_initrd, xen_xsm
add: xen_module

This update bases on
commit 0edd750e50698854068358ea53528100a9192902
Author: Vladimir Serbinenko 
Date:   Fri Jan 22 10:18:47 2016 +0100

xen_boot: Remove obsolete module type distinctions.

Signed-off-by: Fu Wei 
---
 docs/grub.texi | 33 ++---
 1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index 82f6fa4..3fbdd99 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3861,9 +3861,7 @@ you forget a command, you can run the command 
@command{help}
 * videoinfo::   List available video modes
 @comment * xen_*::  Xen boot commands
 * xen_hypervisor::  Load xen hypervisor binary
-* xen_linux::   Load dom0 kernel for xen hypervisor
-* xen_initrd::  Load dom0 initrd for dom0 kernel
-* xen_xsm:: Load xen security module for xen hypervisor
+* xen_module::  Load xen modules for xen hypervisor
 @end menu
 
 
@@ -5141,30 +5139,19 @@ verbatim as the @dfn{kernel command-line}. Any other 
binaries must be
 reloaded after using this command.
 @end deffn
 
-@node xen_linux
-@subsection xen_linux
+@node xen_module
+@subsection xen_module
 
-@deffn Command xen_linux file [arguments]
-Load a dom0 kernel image for xen hypervisor at the booting process of xen.
+@deffn Command xen_module [--nounzip] file [arguments]
+Load a module for xen hypervisor at the booting process of xen.
 The rest of the line is passed verbatim as the module command line.
+On i386,  the modules will be identified by Multiboot(2) protocol.
+On arm64, each module will be identified by the order in which the
+modules are added.
+The 1st module: dom0 kernel image
+The 2nd module: dom0 ramdisk (optional)
 @end deffn
 
-@node xen_initrd
-@subsection xen_initrd
-
-@deffn Command xen_initrd file
-Load a initrd image for dom0 kernel at the booting process of xen.
-@end deffn
-
-@node xen_xsm
-@subsection xen_xsm
-
-@deffn Command xen_xsm file
-Load a xen security module for xen hypervisor at the booting process of xen.
-See @uref{http://wiki.xen.org/wiki/XSM} for more detail.
-@end deffn
-
-
 @node Networking commands
 @section The list of networking commands
 
-- 
2.5.0


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

[Xen-devel] [PATCH v3 0/4] arm64, xen: add xen_boot support into grup-mkconfig

[fu . wei]

From: Fu Wei 

This patchset add xen_boot support into grup-mkconfig for
generating xen boot entrances automatically

Also update the docs/grub.texi for new xen_boot commands.

This patchset has been tested on Foudation model with a bug fix:
http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00205.html

ChangeLog:
v3: reorder the patches
update the introduction of xen_module commands in docs/grub.texi

v2: http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00282.html
add "--nounzip" option support in xen_module
use "feature_xen_boot" instead of "grub_xen_boot"
update the introduction of xen boot commands in docs/grub.texi

v1 :first upstream patchset:
http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00264.html

Fu Wei (4):
  i386,xen: Add xen_hypervisor and xen_module aliases for i386
  arm64: add "--nounzip" option support in xen_module command
  * util/grub.d/20_linux_xen.in: Add xen_boot command support
  arm64: update the introduction of xen boot commands in 
docs/grub.texi

 docs/grub.texi| 33 ++---
 grub-core/loader/arm64/xen_boot.c | 17 +
 grub-core/loader/i386/xen.c   |  7 +++
 grub-core/normal/main.c   |  2 +-
 util/grub.d/20_linux_xen.in   | 13 ++---
 5 files changed, 45 insertions(+), 27 deletions(-)

-- 
2.5.0


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

Re: [Xen-devel] [PATCH v5 20/22] arm/acpi: Initialize serial port from ACPI SPCR table

[Shannon Zhao]

On 2016/2/26 18:20, Shannon Zhao wrote:
> 
> On 2016/2/26 18:04, Jan Beulich wrote:
>  On 26.02.16 at 07:22,  wrote:
 >> > From: Shannon Zhao 
 >> > 
 >> > Parse ACPI SPCR (Serial Port Console Redirection table) table and
 >> > initialize the serial port pl011.
 >> > 
 >> > Signed-off-by: Parth Dixit 
 >> > Signed-off-by: Shannon Zhao 
 >> > Reviewed-by: Stefano Stabellini 
>> > Acked-by: Jan Beulich 
>> > 
> Thanks a lot!
> 
>> > It's not obvious to me whether this could go in ahead of most of
>> > the rest of the series, together with patch 19 (which clearly can
>> > go in right away).
> I think this one could go in ahead with patch 19 since it's independent.
Actually it's not that independent since it relies on patch 11 to
compile. So this one could be picked up by Ian if you don't mind.

Thanks,
-- 
Shannon


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

[Xen-devel] [PATCH v3 3/4] * util/grub.d/20_linux_xen.in: Add xen_boot command support

[fu . wei]

From: Fu Wei 

This patch adds the support of xen_boot command:
xen_hypervisor
xen_module

Also add a new "feature_xen_boot" to indicate this grub support
xen_boot command.

Signed-off-by: Fu Wei 
---
 grub-core/normal/main.c |  2 +-
 util/grub.d/20_linux_xen.in | 13 ++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
index 78a70a8..3402a05 100644
--- a/grub-core/normal/main.c
+++ b/grub-core/normal/main.c
@@ -488,7 +488,7 @@ static const char *features[] = {
   "feature_chainloader_bpb", "feature_ntldr", "feature_platform_search_hint",
   "feature_default_font_path", "feature_all_video_module",
   "feature_menuentry_id", "feature_menuentry_options", "feature_200_final",
-  "feature_nativedisk_cmd", "feature_timeout_style"
+  "feature_nativedisk_cmd", "feature_timeout_style", "feature_xen_boot"
 };
 
 GRUB_MOD_INIT(normal)
diff --git a/util/grub.d/20_linux_xen.in b/util/grub.d/20_linux_xen.in
index 46045db..998ac21 100644
--- a/util/grub.d/20_linux_xen.in
+++ b/util/grub.d/20_linux_xen.in
@@ -122,16 +122,23 @@ linux_entry ()
 else
 xen_rm_opts="no-real-mode edd=off"
 fi
-   multiboot   ${rel_xen_dirname}/${xen_basename} placeholder 
${xen_args} \${xen_rm_opts}
+if [ "x\$feature_xen_boot" != xy ]; then
+xen_loader="multiboot"
+module_loader="module"
+else
+xen_loader="xen_hypervisor"
+module_loader="xen_module"
+fi
+   \${xen_loader}  ${rel_xen_dirname}/${xen_basename} placeholder 
${xen_args} \${xen_rm_opts}
echo'$(echo "$lmessage" | grub_quote)'
-   module  ${rel_dirname}/${basename} placeholder 
root=${linux_root_device_thisversion} ro ${args}
+   \${module_loader}   ${rel_dirname}/${basename} placeholder 
root=${linux_root_device_thisversion} ro ${args}
 EOF
   if test -n "${initrd}" ; then
 # TRANSLATORS: ramdisk isn't identifier. Should be translated.
 message="$(gettext_printf "Loading initial ramdisk ...")"
 sed "s/^/$submenu_indentation/" << EOF
echo'$(echo "$message" | grub_quote)'
-   module  --nounzip   ${rel_dirname}/${initrd}
+   \${module_loader} --nounzip ${rel_dirname}/${initrd}
 EOF
   fi
   sed "s/^/$submenu_indentation/" << EOF
-- 
2.5.0


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

[Xen-devel] [PATCH v3 2/4] arm64: add "--nounzip" option support in xen_module command

[fu . wei]

From: Fu Wei 

This patch adds "--nounzip" option support in order to
be compatible with the module command of i386, then we can
simplify grub-mkconfig support code.

Signed-off-by: Fu Wei 
---
 grub-core/loader/arm64/xen_boot.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/grub-core/loader/arm64/xen_boot.c 
b/grub-core/loader/arm64/xen_boot.c
index 8ae43d7..d63e631 100644
--- a/grub-core/loader/arm64/xen_boot.c
+++ b/grub-core/loader/arm64/xen_boot.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -379,6 +380,20 @@ grub_cmd_xen_module (grub_command_t cmd 
__attribute__((unused)),
 
   struct xen_boot_binary *module = NULL;
   grub_file_t file = 0;
+  int nounzip = 0;
+
+  if (!argc)
+{
+  grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
+  goto fail;
+}
+
+  if (grub_strcmp (argv[0], "--nounzip") == 0)
+{
+  argv++;
+  argc--;
+  nounzip = 1;
+}
 
   if (!argc)
 {
@@ -403,6 +418,8 @@ grub_cmd_xen_module (grub_command_t cmd 
__attribute__((unused)),
 
   grub_dprintf ("xen_loader", "Init module and node info\n");
 
+  if (nounzip)
+grub_file_filter_disable_compression ();
   file = grub_file_open (argv[0]);
   if (!file)
 goto fail;
-- 
2.5.0


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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Juergen Gross]

On 26/02/16 11:39, Jan Beulich wrote:
 On 25.02.16 at 17:50,  wrote:
>> @@ -670,7 +676,13 @@ int cpu_disable_scheduler(unsigned int cpu)
>>  if ( cpumask_empty(&online_affinity) &&
>>   cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
>>  {
>> -printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v);
>> +if ( v->affinity_broken )
>> +{
>> +/* The vcpu is temporarily pinned, can't move it. */
>> +vcpu_schedule_unlock_irqrestore(lock, flags, v);
>> +ret = -EBUSY;
>> +continue;
>> +}
> 
> So far the function can only return 0 or -EAGAIN. By using "continue"
> here you will make it impossible for the caller to reliably determine
> whether possibly both things failed. Despite -EBUSY being a logical
> choice here, I think you'd better use -EAGAIN here too. And it needs
> to be determined whether continuing the loop in this as well as the
> pre-existing cases is actually the right thing to do.

EBUSY vs. EAGAIN: by returning EAGAIN I would signal to Xen tools that
the hypervisor is currently not able to do the desired operation
(especially removing a cpu from a cpupool), but the situation will
change automatically via scheduling. EBUSY will stop retries in Xen
tools and this is want I want here: I can't be sure the situation
will change soon.

Regarding continuation of the loop: I think you are right in the
EBUSY case: I should break out of the loop. I should not do so in the
EAGAIN case as I want to remove as many vcpus from the physical cpu as
possible without returning to the Xen tools in between.

> 
>> @@ -679,6 +691,8 @@ int cpu_disable_scheduler(unsigned int cpu)
>>  v->affinity_broken = 1;
>>  }
>>  
>> +printk(XENLOG_DEBUG "Breaking affinity for %pv\n", v);
> 
> Wouldn't it be even better to make this the "else" to the
> preceding if(), since in the suspend case this is otherwise going
> to be printed for every vCPU not currently running on pCPU0?

Yes, I'll change it.

> 
>> @@ -753,14 +767,22 @@ static int vcpu_set_affinity(
>>  struct vcpu *v, const cpumask_t *affinity, cpumask_t *which)
>>  {
>>  spinlock_t *lock;
>> +int ret = 0;
>>  
>>  lock = vcpu_schedule_lock_irq(v);
>>  
>> -cpumask_copy(which, affinity);
>> +if ( v->affinity_broken )
>> +{
>> +ret = -EBUSY;
>> +}
> 
> Unnecessary braces.

Will remove.

> 
>> @@ -979,6 +1001,53 @@ void watchdog_domain_destroy(struct domain *d)
>>  kill_timer(&d->watchdog_timer[i]);
>>  }
>>  
>> +static long do_pin_temp(int cpu)
>> +{
>> +struct vcpu *v = current;
>> +spinlock_t *lock;
>> +long ret = -EINVAL;
>> +
>> +lock = vcpu_schedule_lock_irq(v);
>> +
>> +if ( cpu == -1 )
>> +{
>> +if ( v->affinity_broken )
>> +{
>> +cpumask_copy(v->cpu_hard_affinity, v->cpu_hard_affinity_saved);
>> +v->affinity_broken = 0;
>> +set_bit(_VPF_migrating, &v->pause_flags);
>> +ret = 0;
>> +}
>> +}
>> +else if ( cpu < nr_cpu_ids && cpu >= 0 )
> 
> Perhaps easier to simply use "cpu < 0" in the first if()?

Okay.

> 
>> +{
>> +if ( v->affinity_broken )
>> +{
>> +ret = -EBUSY;
>> +}
>> +else if ( cpumask_test_cpu(cpu, VCPU2ONLINE(v)) )
>> +{
> 
> This is a rather ugly restriction: How would a caller fulfill its job
> when this is not the case?

He can't. We should document that at least on hardware requiring this
functionality it is a bad idea to remove cpu 0 from the cpupool with the
hardware domain.

> 
>> @@ -1088,6 +1157,23 @@ ret_t do_sched_op(int cmd, 
>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>  break;
>>  }
>>  
>> +case SCHEDOP_pin_temp:
>> +{
>> +struct sched_pin_temp sched_pin_temp;
>> +
>> +ret = -EFAULT;
>> +if ( copy_from_guest(&sched_pin_temp, arg, 1) )
>> +break;
>> +
>> +ret = xsm_schedop_pin_temp(XSM_PRIV);
>> +if ( ret )
>> +break;
>> +
>> +ret = do_pin_temp(sched_pin_temp.pcpu);
>> +
>> +break;
>> +}
> 
> So having come here I still don't see why this is called "temp":
> Nothing enforces this to be a temporary state, and hence the
> sub-op name currently is actively misleading.

I've chosen this name as the old affinity is saved and can (and should)
be recovered later. So it is intended to be temporary.

>> --- a/xen/include/public/sched.h
>> +++ b/xen/include/public/sched.h
>> @@ -118,6 +118,15 @@
>>   * With id != 0 and timeout != 0, poke watchdog timer and set new timeout.
>>   */
>>  #define SCHEDOP_watchdog6
>> +
>> +/*
>> + * Temporarily pin the current vcpu to one physical cpu or undo that 
>> pinning.
>> + * @arg == pointer to sched_pin_temp_t structure.
>> + *
>> + * Setting pcpu to -1 will undo a previous temporar

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

[Fu Wei]

Hi Andrei,

On 26 February 2016 at 18:50, Andrei Borzenkov  wrote:
> On Fri, Feb 26, 2016 at 8:59 AM, Fu Wei  wrote:
>> +@subsection xen_module
>>
>> -@deffn Command xen_linux file [arguments]
>> -Load a dom0 kernel image for xen hypervisor at the booting process of 
>> xen.
>> +@deffn Command xen_module [--nounzip] file [arguments]
>> +Load a module for xen hypervisor at the booting process of xen.
>> The rest of the line is passed verbatim as the module command line.
>> +Each module will be identified by the order in which the modules are 
>> added.
>> +The 1st module: dom0 kernel image
>> +The 2nd module: dom0 ramdisk
>> +All subsequent modules: UNKNOW
>> @end deffn
>
> Hmm ... from previous discussion I gathered that Xen can detect module
> type. What if there is no initrd for dom0? How can subsequent modules be

 Now , Xen detect module type by the order. (at least on ARM64).
 I think i386 is using Multiboot(2) protocol, so maybe this order is
 nothing to do with i386.

>>>
>>> Then we have obvious problem with your XSM patch 
>>> (http://savannah.gnu.org/bugs/?43420) - XSM may land as the first module. 
>>> That's actually something to solve on Xen side I think. It's just that so 
>>> far we had just kernel and initrd, so that was non issue.
>>
>> Oh, did you mean Wei Liu's patch?
>>
>> I guess XSM may land as the third module (or the module after linux
>> kernel, if you don't have initrd)
>>
>> Yes, agree. (That's actually something to solve on Xen side)
>>
>> I guess xen can get xsm from a special initrd. so for now there is not
>> big problem on xsm.
>>
>> Please correct me if I misunderstand something. :-)
>>
>> Thanks!
>>
>> Back to this patch, is that OK for you, or any suggestion?  Thanks !
>>
>
> Yes, as this is dedicated Xen loader we should document this mandatory
> order - first module must be kernel image, second module must be
> initrd. I do not think we need to mention possibility to load more
> than two modules until there is clear understanding how it can be done
> without initrd.

Great thanks for your review, I have updated and sent the v3 patchset,
Hope I understood your suggestion correctly, Please check.  :-)



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Dario Faggioli]

On Fri, 2016-02-26 at 12:14 +0100, Juergen Gross wrote:
> On 26/02/16 11:39, Jan Beulich wrote:
> > 
> > > @@ -670,7 +676,13 @@ int cpu_disable_scheduler(unsigned int cpu)
> > >  if ( cpumask_empty(&online_affinity) &&
> > >   cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
> > >  {
> > > -printk(XENLOG_DEBUG "Breaking affinity for
> > > %pv\n", v);
> > > +if ( v->affinity_broken )
> > > +{
> > > +/* The vcpu is temporarily pinned, can't
> > > move it. */
> > > +vcpu_schedule_unlock_irqrestore(lock, flags,
> > > v);
> > > +ret = -EBUSY;
> > > +continue;
> > > +}
> > So far the function can only return 0 or -EAGAIN. By using
> > "continue"
> > here you will make it impossible for the caller to reliably
> > determine
> > whether possibly both things failed. Despite -EBUSY being a logical
> > choice here, I think you'd better use -EAGAIN here too. And it
> > needs
> > to be determined whether continuing the loop in this as well as the
> > pre-existing cases is actually the right thing to do.
> EBUSY vs. EAGAIN: by returning EAGAIN I would signal to Xen tools
> that
> the hypervisor is currently not able to do the desired operation
> (especially removing a cpu from a cpupool), but the situation will
> change automatically via scheduling. EBUSY will stop retries in Xen
> tools and this is want I want here: I can't be sure the situation
> will change soon.
> 
I agree with this.

> Regarding continuation of the loop: I think you are right in the
> EBUSY case: I should break out of the loop. I should not do so in the
> EAGAIN case as I want to remove as many vcpus from the physical cpu
> as
> possible without returning to the Xen tools in between.
> 
And with this too.

And I think that, if we indeed break out of the loop on EBUSY, that
will also make it possible to figure out properly what actually went
wrong, so it should be fine from that point of view as well.

> > > @@ -679,6 +691,8 @@ int cpu_disable_scheduler(unsigned int cpu)
> > >  v->affinity_broken = 1;
> > >  }
> > >  
> > > +printk(XENLOG_DEBUG "Breaking affinity for
> > > %pv\n", v);
> > Wouldn't it be even better to make this the "else" to the
> > preceding if(), since in the suspend case this is otherwise going
> > to be printed for every vCPU not currently running on pCPU0?
> Yes, I'll change it.
> 
On this, can (either of) you elaborate a bit more? I don't think I'm
following...

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D 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] Xen 4.4, xenstored and WATCH requests

[Wei Liu]

Hello

On Thu, Feb 25, 2016 at 03:54:14PM +0300, Sergei Lebedev wrote:
> Hello list,
> 
> I’m working on a Python client library for XenStore [1]. The library
> implements two ways to access XenStore: via Unix socket and via /dev.
> The /dev interface turned out to be a bit problematic, because it
> ignores the req_id field for WATCH requests.
> 

So the unix socket interface worked fine for you?

> The spec [2] requires all responses (except WATCH_EVENT) to copy
> req_id from request. So I wonder if it’s possible that the behaviour I
> observe is in fact a bug in xenstored?
> 
> Below is a Python script demonstrating the issue:
> 
> import os
> import struct
> 
> WATCH = 4
> WRITE = 11
> 
> def send_packet(fd, op, rq_id, tx_id, payload):
> data = struct.pack(b"", op, rq_id, tx_id, len(payload)) + payload
> os.write(fd, data)
> 
> 
> def print_reply(fd):
> op, rq_id, tx_id, size = struct.unpack(b"", os.read(fd, 16))
> payload = os.read(fd, size) if size else b""
> print(op, rq_id, tx_id, payload)
> 
> 
> fd = os.open("/dev/xen/xenbus", os.O_RDWR)
> try:
> send_packet(fd, WRITE, 24, 0, b"/foo\x00bar\x00")
> print_reply(fd)  # ACK for WRITE with rq_id = 24.
> send_packet(fd, WATCH, 42, 0, b"/foo\x00token\x00")
> print_reply(fd)  # Spurious (?) WATCH_EVENT.
> print_reply(fd)  # ACK for WATCH with rq_id = 0.
> finally:
> os.close(fd)
> 

> Example output:
> 
> (11, 24, 0, 'OK\x00')
> (15, 4294967295, 892960384, '/foo\x00token\x00')
> (4, 0, 0, 'OK\x00')
> 
> I’m running Xen 4.4 on Ubuntu 14.04 inside VirtualBox.
> 
> $ uname -a
> Linux xen-devel 3.19.0-25-generic #26~14.04.1-Ubuntu SMP Fri Jul 24 21:16:20 
> UTC 2015 x86_64 x86_64 x86_64 GNU/Linux
> 

FWIW I tried you program in my own testbox running xen-unstable with
Debian's stock 3.16 kernel, the result is the same.

And in xen.git

  $ git diff origin/stable-4.4..origin/staging -- docs/misc/xenstore.txt

shows nothing.

So this is not a problem in 4.4 only.

Wei.

> Regards,
> Sergei
> 
> [1]: http://github.com/selectel/pyxs
> [2]: http://xenbits.xen.org/docs/4.4-testing/misc/xenstore.txt
> 
> 
> ___
> 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 1/4] build: consolidate CONFIG_HAS_VIDEO and CONFIG_VIDEO

[Jan Beulich]

>>> On 26.02.16 at 04:39,  wrote:
> --- a/xen/drivers/video/Kconfig
> +++ b/xen/drivers/video/Kconfig
> @@ -1,12 +1,12 @@
>  
> -# Select HAS_VIDEO if video is supported
> -config HAS_VIDEO
> +# Select VIDEO if video is supported
> +config VIDEO
>   bool
>  
>  # Select VGA if VGA is supported
>  config VGA
>   bool
> - select HAS_VIDEO
> + select VIDEO

Even if the needed ARM ack was in place, this wouldn't apply. I
suppose there is another patch missing ahead of this one,
converting HAS_VGA -> VGA.

Jan


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

Re: [Xen-devel] [PATCHv4 1/3] x86/fpu: Add a per-domain field to set the width of FIP/FDP

[Jan Beulich]

>>> On 25.02.16 at 16:10,  wrote:
> --- a/xen/arch/x86/xstate.c
> +++ b/xen/arch/x86/xstate.c
> @@ -249,7 +249,7 @@ void xsave(struct vcpu *v, uint64_t mask)
>  struct xsave_struct *ptr = v->arch.xsave_area;
>  uint32_t hmask = mask >> 32;
>  uint32_t lmask = mask;
> -int word_size = mask & XSTATE_FP ? (cpu_has_fpu_sel ? 8 : 0) : -1;
> +unsigned int fip_width = v->domain->arch.x87_fip_width;
>  #define XSAVE(pfx) \
>  alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", /* xsave */ \
>   ".byte " pfx "0x0f,0xae,0x37\n", /* xsaveopt */ \
> @@ -261,7 +261,15 @@ void xsave(struct vcpu *v, uint64_t mask)
>   "=m" (*ptr), \
>   "a" (lmask), "d" (hmask), "D" (ptr))
>  
> -if ( word_size <= 0 || !is_pv_32bit_vcpu(v) )
> +if ( fip_width == 8 || !(mask & XSTATE_FP) )
> +{
> +XSAVE("0x48,");
> +}
> +else if ( fip_width == 4 )
> +{
> +XSAVE("");
> +}
> +else
>  {
>  typeof(ptr->fpu_sse.fip.sel) fcs = ptr->fpu_sse.fip.sel;
>  typeof(ptr->fpu_sse.fdp.sel) fds = ptr->fpu_sse.fdp.sel;
> @@ -301,25 +309,25 @@ void xsave(struct vcpu *v, uint64_t mask)
>  return;
>  }
>  
> -if ( word_size > 0 &&
> - !((ptr->fpu_sse.fip.addr | ptr->fpu_sse.fdp.addr) >> 32) )
> +/*
> + * If the FIP/FDP[63:32] are both zero, it is safe to use the
> + * 32-bit restore to also restore the selectors.
> + */
> +if ( !((ptr->fpu_sse.fip.addr | ptr->fpu_sse.fdp.addr) >> 32) )
>  {
>  struct ix87_env fpu_env;
>  
>  asm volatile ( "fnstenv %0" : "=m" (fpu_env) );
>  ptr->fpu_sse.fip.sel = fpu_env.fcs;
>  ptr->fpu_sse.fdp.sel = fpu_env.fds;
> -word_size = 4;
> +fip_width = 4;
>  }
> -}
> -else
> -{
> -XSAVE("");
> -word_size = 4;
> +else
> +fip_width = 8;
>  }
>  #undef XSAVE
> -if ( word_size >= 0 )
> -ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] = word_size;
> +if ( mask & XSTATE_FP )
> +ptr->fpu_sse.x[FPU_WORD_SIZE_OFFSET] = fip_width;
>  }

This needed further fixing up, as two references to word_size
had been left in. Which means that patch 3 now will need to be
re-based. And please double check the adjustments.

Jan


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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Juergen Gross]

On 26/02/16 12:20, Dario Faggioli wrote:
> On Fri, 2016-02-26 at 12:14 +0100, Juergen Gross wrote:
>> On 26/02/16 11:39, Jan Beulich wrote:
 @@ -679,6 +691,8 @@ int cpu_disable_scheduler(unsigned int cpu)
  v->affinity_broken = 1;
  }
  
 +printk(XENLOG_DEBUG "Breaking affinity for
 %pv\n", v);
>>> Wouldn't it be even better to make this the "else" to the
>>> preceding if(), since in the suspend case this is otherwise going
>>> to be printed for every vCPU not currently running on pCPU0?
>> Yes, I'll change it.
>>
> On this, can (either of) you elaborate a bit more? I don't think I'm
> following...

In the suspend case the affinity will be broken only temporarily, so
there is no need to print the debug message.


juergen


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

Re: [Xen-devel] [PATCH] docs: spell out limits of security support for qemu-xen

[Stefano Stabellini]

On Thu, 25 Feb 2016, Doug Goldstein wrote:
> On 2/25/16 9:43 AM, Stefano Stabellini wrote:
> 
> > +++ b/docs/misc/qemu-xen-security
> > @@ -0,0 +1,20 @@
> > +qemu-xen (git://xenbits.xen.org/qemu-xen.git) is only supported for
> > +security fixes when used together with the Xen hypervisor and only with
> > +a subset of all the possible QEMU emulators. Specifically:
> 
> So I'll get my comments on paper here rather than something just
> mentioned on IRC. This is exactly why the Xen team should be pushing to
> remove as many "in-tree" items as possible. The security surface area of
> Xen is huge and statements like this help the CYA factor they don't
> completely eliminate the problems of manpower of having to check against
> different upstreams if a vulnerability affects you or downstreams doing
> something bad causing a security issue for users which ultimately gets
> blamed on Xen. There are then further complications where sometimes the
> version shipped by Xen isn't an upstream release and so there may be
> other vulnerabilities above and beyond what upstream announces.
> 
> I urge the Xen maintainers to make it a goal to remove external
> libraries and applications (like qemu-xen) from the tree entirely and
> recommend the use of the upstream release. I know the concern is testing
> but it involves calling out your dependencies just like you do any other
> dependency. (e.g. Xen X.Y requires QEMU A.B.C, no guarantees are made
> about the compatibility of other versions)
> 
> I know Stefano is making an effort with this with Project Raisin and
> really that should become the embraced way to stand up a "full" Xen
> system from source rather than a hodge podge collection of packages that
> are fetched by the Xen build system. This will bring the how developers
> use the source packages closer with how many users of distros use Xen
> (e.g. a number of distros use upstream QEMU releases instead of qemu-xen).

Thanks Doug, I fully agree with you.

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

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Xu, Quan]

On February 26, 2016 6:11pm,  wrote:
> >>> On 26.02.16 at 10:24,  wrote:
> > On February 26, 2016 4:15pm,  wrote:
> >> >>> On 26.02.16 at 08:37,  wrote:
> >> > On February 25, 2016 8:24pm,  wrote:
> >> >> >>> On 25.02.16 at 13:14,  wrote:
> >> >> > On February 25, 2016 4:59pm,  wrote:
> >> >
> >> >> >> However, the same effect could be achieved by making the lock a
> >> >> >> recursive one, which would then seem to more conventional
> >> >> >> approach (but requiring as much code to be touched).
> >> >
> >> > IMO, for v6, I'd better:
> >> >   (1). replace all of 'spin_lock(&pcidevs_lock)' with
> >> > 'spin_lock_recursive(&pcidevs_lock)',
> >> >   (2). replace all of 'spin_unlock(&pcidevs_lock)' with
> >> > 'spin_unlock_recursive(&pcidevs_lock)',
> >> >   (3). _do_not_ touch 'spin_is_locked(&pcidevs_lock)'.
> >> > Then, ignore v5 [patch4/7], [patch5/7], [patch6/7],  *_locked()
> >> > related code,  and add a new patch for the above (1). (2). (3). Right?
> >>
> >> Yes.
> >>
> >> > BTW, .e.g. 'd->page_alloc_lock', I found the 'd->page_alloc_lock'
> >> > is called by both spin_lock_recursive() and 'spin_lock()'.
> >> > _If_  both spin_lock_recursive(&d->page_alloc_lock) and
> >> > 'spin_lock(&d->page_alloc_lock)' are recursively called in same
> >> > call tree as below, it might be a bug. Right?
> >> >
> >> >
> >> > {
> >> > ...
> >> > spin_lock()
> >> > ...
> >> >spin_lock_recursive()
> >> >...
> >> >spin_unlock_recursive()  <--- (lock might be now free)
> >> > ...
> >> > spin_unlock()
> >> > ...
> >> > }
> >>
> >> Well, such a use of course would be a bug. But using plain
> >> spin_lock() on a path where recursive acquire can't occur is fine.
> >>
> >> Nevertheless I'd recommend not mixing things for the pcidevs one, as
> >> its uses are scattered around too much for it to be reasonably
> >> possible to prove correctness if done that way.
> >
> >
> > But I do _not_ mix things for the pcidevs one. As
> >   (1). replace ___all___ of 'spin_lock(&pcidevs_lock)' with
> > 'spin_lock_recursive(&pcidevs_lock)',
> >   (2). replace ___all___ of 'spin_unlock(&pcidevs_lock)' with
> > 'spin_unlock_recursive(&pcidevs_lock)',
> 
> I am not saying you mix things, I'm just saying there is a risk you do 
> perhaps not
> even knowingly, 

I actually don't.

> e.g. when another patch goes in about the same time as yours
> which adds another instance. Plus doing what you state above is going to be
> close to unreviewable, since the reviewer would have to check that indeed you
> caught all instances. By removing global visibility of the lock variable both 
> issues
> are easily avoided, since without catching all cases a build error would 
> result.



I would summarize as follows and make sure we are on the same page.

 1) make pcidevs_lock only visible inside xen/drivers/passthrough/pci.c, 
   turning the definition of the variable into a static one, and getting rid of 
its declaration from xen/include/xen/pci.h

 2) define 3 helpers in 'include/xen/pci.h':
*pcidevs_lock()
*pcidevs_unlock()
*pcidevs_is_locked()
  Then, I'd implement these helpers in xen/drivers/passthrough/pci.c. 
  Of course, the helpers will do spin_lock_recursive() and 
spin_unlock_recursive(). It is quite similar to what is done for domain_lock().

3) use these helpers. Replace
 *spin_lock(&pcidevs_lock) with pcidevs_lock(),
 *spin_unlock(&pcidevs_lock) with pcidevs_unlock(),
 *spin_is_locked(&pcidevs_lock) with pcidevs_is_locked().

Right?

Quan

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

Re: [Xen-devel] [PATCH v1 1/1] xen/arm: Re-add the Xilinx ZynqMP platform

[Stefano Stabellini]

On Fri, 26 Feb 2016, Edgar E. Iglesias wrote:
> From: "Edgar E. Iglesias" 
> 
> Re-add the Xilinx ZynqMP platform. This time include a
> blacklisted zynqmp-pm (Power Management) device that does
> not yet play nicely with Xen.
> 
> Signed-off-by: Edgar E. Iglesias 

Acked-by: Stefano Stabellini 


>  xen/arch/arm/platforms/Makefile|  1 +
>  xen/arch/arm/platforms/xilinx-zynqmp.c | 47 
> ++
>  2 files changed, 48 insertions(+)
>  create mode 100644 xen/arch/arm/platforms/xilinx-zynqmp.c
> 
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index e173fec..3689eec 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_ARM_32) += sunxi.o
>  obj-$(CONFIG_ARM_32) += rcar2.o
>  obj-$(CONFIG_ARM_64) += seattle.o
>  obj-$(CONFIG_ARM_64) += xgene-storm.o
> +obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> diff --git a/xen/arch/arm/platforms/xilinx-zynqmp.c 
> b/xen/arch/arm/platforms/xilinx-zynqmp.c
> new file mode 100644
> index 000..2adee91
> --- /dev/null
> +++ b/xen/arch/arm/platforms/xilinx-zynqmp.c
> @@ -0,0 +1,47 @@
> +/*
> + * xen/arch/arm/platforms/xilinx-zynqmp.c
> + *
> + * Xilinx ZynqMP setup
> + *
> + * Copyright (c) 2016 Xilinx Inc.
> + * Written by Edgar E. Iglesias 
> + *
> + * 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 
> +
> +static const char * const zynqmp_dt_compat[] __initconst =
> +{
> +"xlnx,zynqmp",
> +NULL
> +};
> +
> +static const struct dt_device_match zynqmp_blacklist_dev[] __initconst =
> +{
> +/* Power management is not yet supported.  */
> +DT_MATCH_COMPATIBLE("xlnx,zynqmp-pm"),
> +{ /* sentinel */ },
> +};
> +
> +PLATFORM_START(xgene_storm, "Xilinx ZynqMP")
> +.compatible = zynqmp_dt_compat,
> +.blacklist_dev = zynqmp_blacklist_dev,
> +PLATFORM_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> -- 
> 2.5.0
> 

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Jan Beulich]

>>> On 26.02.16 at 12:07,  wrote:
> --- a/xen/include/asm-x86/altp2m.h
> +++ b/xen/include/asm-x86/altp2m.h
> @@ -15,8 +15,8 @@
>   * this program; If not, see .
>   */
>  
> -#ifndef _X86_ALTP2M_H
> -#define _X86_ALTP2M_H
> +#ifndef __ASM_X86_ALTP2M_H
> +#define __ASM_X86_ALTP2M_H

Unrelated change? (No need to undo, but please don't mix such
into patches especially when they are quite large already anyway.)

> @@ -33,5 +33,9 @@ void altp2m_vcpu_initialise(struct vcpu *v);
>  void altp2m_vcpu_destroy(struct vcpu *v);
>  void altp2m_vcpu_reset(struct vcpu *v);
>  
> -#endif /* _X86_ALTP2M_H */
> +static inline uint16_t altp2m_vcpu_idx(struct vcpu *v)

const

With that, non-event x86 source changes
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH v5 01/22] arm/acpi: Emulate io ports for arm

[Stefano Stabellini]

On Fri, 26 Feb 2016, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Add macros to emulate x86 style ports for arm. This avoids modification in
> common code for acpi. Here just print a warning on ARM.
> 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Stefano Stabellini 


> V5: not write to the address, just print warning
> V4: print warning
> ---
>  xen/include/asm-arm/arm64/io.h | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/xen/include/asm-arm/arm64/io.h b/xen/include/asm-arm/arm64/io.h
> index 37abc47..f80156f 100644
> --- a/xen/include/asm-arm/arm64/io.h
> +++ b/xen/include/asm-arm/arm64/io.h
> @@ -20,6 +20,7 @@
>  #ifndef _ARM_ARM64_IO_H
>  #define _ARM_ARM64_IO_H
>  
> +#include 
>  #include 
>  
>  /*
> @@ -109,4 +110,26 @@ static inline u64 __raw_readq(const volatile void 
> __iomem *addr)
>  #define writel(v,c) ({ __iowmb(); writel_relaxed((v),(c)); })
>  #define writeq(v,c) ({ __iowmb(); writeq_relaxed((v),(c)); })
>  
> +/*
> + * Emulate x86 io ports for ARM.
> + */
> +static inline int emulate_read(u64 addr)
> +{
> +printk(XENLOG_G_WARNING "Can't access IO %lx\n", addr);
> +return 0;
> +}
> +
> +static inline void emulate_write(u64 addr)
> +{
> +printk(XENLOG_G_WARNING "Can't access IO %lx\n", addr);
> +}
> +
> +#define inb(c) ( emulate_read(c) )
> +#define inw(c) ( emulate_read(c) )
> +#define inl(c) ( emulate_read(c) )
> +
> +#define outb(v, c) ( emulate_write(c) )
> +#define outw(v, c) ( emulate_write(c) )
> +#define outl(v, c) ( emulate_write(c) )
> +
>  #endif /* _ARM_ARM64_IO_H */

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Corneliu ZUZU]

On 2/26/2016 1:56 PM, Jan Beulich wrote:

On 26.02.16 at 12:07,  wrote:

--- a/xen/include/asm-x86/altp2m.h
+++ b/xen/include/asm-x86/altp2m.h
@@ -15,8 +15,8 @@
   * this program; If not, see .
   */
  
-#ifndef _X86_ALTP2M_H

-#define _X86_ALTP2M_H
+#ifndef __ASM_X86_ALTP2M_H
+#define __ASM_X86_ALTP2M_H

Unrelated change? (No need to undo, but please don't mix such
into patches especially when they are quite large already anyway.)


Noted.


@@ -33,5 +33,9 @@ void altp2m_vcpu_initialise(struct vcpu *v);
  void altp2m_vcpu_destroy(struct vcpu *v);
  void altp2m_vcpu_reset(struct vcpu *v);
  
-#endif /* _X86_ALTP2M_H */

+static inline uint16_t altp2m_vcpu_idx(struct vcpu *v)

const


'const', as in:

+static inline const uint16_t altp2m_vcpu_idx(struct vcpu *v)

?



With that, non-event x86 source changes
Acked-by: Jan Beulich 

Jan




Thanks,
Corneliu.

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

Re: [Xen-devel] Xen 4.4, xenstored and WATCH requests

[Wei Liu]

On Fri, Feb 26, 2016 at 11:26:50AM +, Wei Liu wrote:
> Hello
> 
> On Thu, Feb 25, 2016 at 03:54:14PM +0300, Sergei Lebedev wrote:
> > Hello list,
> > 
> > I’m working on a Python client library for XenStore [1]. The library
> > implements two ways to access XenStore: via Unix socket and via /dev.
> > The /dev interface turned out to be a bit problematic, because it
> > ignores the req_id field for WATCH requests.
> > 
> 
> So the unix socket interface worked fine for you?
> 

BTW can you paste in the result of

  $ ps aux | grep xenstored

here?

I need to know which variant of xenstored you're running.

Wei.

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

Re: [Xen-devel] [PATCH v5 05/22] arm/acpi: Move end_boot_allocator after acpi_boot_table_init

[Stefano Stabellini]

On Fri, 26 Feb 2016, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> To support ACPI NUMA, it needs to make the ACPI initialization done
> before boot_end_allocator. Also, x86 does this by the same way.
> 
> Signed-off-by: Parth Dixit 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Stefano Stabellini 


> V5: fix the commit message
> ---
>  xen/arch/arm/setup.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index fee5385..d4261e8 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -615,8 +615,6 @@ static void __init setup_mm(unsigned long dtb_paddr, 
> size_t dtb_size)
> allocator. */
>  init_xenheap_pages(pfn_to_paddr(xenheap_mfn_start),
> pfn_to_paddr(boot_mfn_start));
> -
> -end_boot_allocator();
>  }
>  #else /* CONFIG_ARM_64 */
>  static void __init setup_mm(unsigned long dtb_paddr, size_t dtb_size)
> @@ -684,8 +682,6 @@ static void __init setup_mm(unsigned long dtb_paddr, 
> size_t dtb_size)
>  
>  setup_frametable_mappings(ram_start, ram_end);
>  max_page = PFN_DOWN(ram_end);
> -
> -end_boot_allocator();
>  }
>  #endif
>  
> @@ -759,6 +755,8 @@ void __init start_xen(unsigned long boot_phys_offset,
>  /* Parse the ACPI tables for possible boot-time configuration */
>  acpi_boot_table_init();
>  
> +end_boot_allocator();
> +
>  vm_init();
>  dt_unflatten_host_device_tree();
>  
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] [PATCH V14 4/6] libxl: add pvusb API

[Ian Jackson]

George Dunlap writes ("Re: [Xen-devel] [PATCH V14 4/6] libxl: add pvusb API"):
> On Fri, Feb 19, 2016 at 10:39 AM, Chunyan Liu  wrote:
> > +  [...]
> 
> So I see below that you're calling this before removing things from
> xenstore, so that if any of these fail, the user can still call "xl
> usb-remove" to retry.
> 
> Unfortunately, since you reassign interfaces to the original driver
> before all interfaces are de-assigned from usbback, you can end up in
> a situation where the device is partially re-plugged into the original
> drivers, partially still assigned to pciback.
> 
> I think this whole process should be three steps:
> 
> 1. Unassign all interfaces from usbback, stopping and returning an
> error as soon as one attempt fails
> 
> 2. Remove the pvusb xenstore nodes (stopping and returning an error if it 
> fails)
> 
> 3. Attempt to re-assign all interfaces to the original drivers,
> stopping and returning an error as soon as one attempt fails.

This seems like a good plan to me.

(Making 3 after 2 re-attemptable would mean that the original driver
information needs to be saved in a different location in xenstore to
the pvusb control nodes, but that is not a problem.)

> * If one of the re-assignments fail, then the user will have to reload
> the drivers, reboot, or mess around with sysfs to fix things.
> *However*, this avoids a scenario where a user is completely unable to
> remove a device from a domain because of a buggy driver.

Right.

> I realize this falls short of the "crash-only" design IanJ suggested
> we try to do, but I think that in this case the work required to do
> such a design would be a lot more work than the benefit it gives us.

I think what you have above is indeed crash-only.  You can tell by all
the "if any error occurs, stop immediately".

The only wrinkle is that the obvious implementation reads the old
bindings from xenstore between 1 and 2, deletes the information from
xenstore in 2, and uses that information in step 3, which is cheating
(and leads to the sysfs-wrangling-required state).  But that is quite
easy to avoid:
  - record the old driver bindings somewhere in xenstore (keyed by
the physical host device, not the guest domain)
  - provide a libxl call and corresponding xl command which attempts
to rebind

But, FAOD, I do not want to block this patch until such a thing is
implemented.  I think it is sufficient to document the existence of
the awkward state, and the likely workarounds.

Ian.

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

Re: [Xen-devel] [PATCH v5 06/22] arm/acpi: Parse FADT table and get PSCI flags

[Stefano Stabellini]

On Fri, 26 Feb 2016, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> There are two flags: PSCI_COMPLIANT and PSCI_USE_HVC. When set, the
> former signals to the OS that the hardware is PSCI compliant. The latter
> selects the appropriate conduit for PSCI calls by toggling between
> Hypervisor Calls (HVC) and Secure Monitor Calls (SMC). FADT table
> contains such information, parse FADT to get the flags for furture
> usage.
> 
> Since STAO table and the GIC version are introduced by ACPI 6.0, we will
> check the version and only parse FADT table with version >= 6.0. If
> firmware provides ACPI tables with ACPI version less than 6.0, OS will
> be messed up with those information, so disable ACPI if we get an FADT
> table with version less than 6.0.
> 
> Signed-off-by: Hanjun Guo 
> Signed-off-by: Naresh Bhat 
> Signed-off-by: Parth Dixit 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Stefano Stabellini 


> V5: only define acpi_psci_present and acpi_psci_hvc_present once 
> V4: drop disable_acpi in acpi_parse_fadt
> ---
>  xen/arch/arm/acpi/boot.c   | 30 ++
>  xen/arch/arm/acpi/lib.c| 12 
>  xen/include/asm-arm/acpi.h |  3 +++
>  3 files changed, 45 insertions(+)
> 
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 1570f7e..6b33fbe 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -27,9 +27,32 @@
>  
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  
>  #include 
>  
> +static int __init acpi_parse_fadt(struct acpi_table_header *table)
> +{
> +struct acpi_table_fadt *fadt = (struct acpi_table_fadt *)table;
> +
> +/*
> + * Revision in table header is the FADT Major revision, and there
> + * is a minor revision of FADT which was introduced by ACPI 6.0,
> + * we only deal with ACPI 6.0 or newer revision to get GIC and SMP
> + * boot protocol configuration data, or we will disable ACPI.
> + */
> +if ( table->revision > 6
> + || (table->revision == 6 && fadt->minor_revision >= 0) )
> +return 0;
> +
> +printk("Unsupported FADT revision %d.%d, should be 6.0+, will disable 
> ACPI\n",
> +table->revision, fadt->minor_revision);
> +
> +return -EINVAL;
> +}
> +
>  /*
>   * acpi_boot_table_init() called from setup_arch(), always.
>   *  1. find RSDP and get its address, and then find XSDT
> @@ -54,5 +77,12 @@ int __init acpi_boot_table_init(void)
>  return error;
>  }
>  
> +if ( acpi_table_parse(ACPI_SIG_FADT, acpi_parse_fadt) )
> +{
> +/* disable ACPI if no FADT is found */
> +disable_acpi();
> +printk("Can't find FADT\n");
> +}
> +
>  return 0;
>  }
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> index f817fe6..a30e4e6 100644
> --- a/xen/arch/arm/acpi/lib.c
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -50,3 +50,15 @@ char *__acpi_map_table(paddr_t phys, unsigned long size)
>  
>   return ((char *) base + offset);
>  }
> +
> +/* 1 to indicate PSCI 0.2+ is implemented */
> +bool_t __init acpi_psci_present(void)
> +{
> +return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_COMPLIANT;
> +}
> +
> +/* 1 to indicate HVC is present instead of SMC as the PSCI conduit */
> +bool_t __init acpi_psci_hvc_present(void)
> +{
> +return acpi_gbl_FADT.arm_boot_flags & ACPI_FADT_PSCI_USE_HVC;
> +}
> diff --git a/xen/include/asm-arm/acpi.h b/xen/include/asm-arm/acpi.h
> index 10e02bd..f13072f 100644
> --- a/xen/include/asm-arm/acpi.h
> +++ b/xen/include/asm-arm/acpi.h
> @@ -30,6 +30,9 @@
>  #define COMPILER_DEPENDENT_UINT64  unsigned long long
>  #define ACPI_MAP_MEM_ATTR  PAGE_HYPERVISOR
>  
> +bool_t __init acpi_psci_present(void);
> +bool_t __init acpi_psci_hvc_present(void);
> +
>  #ifdef CONFIG_ACPI
>  extern bool_t acpi_disabled;
>  /* Basic configuration for ACPI */
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Razvan Cojocaru]

On 02/26/2016 02:05 PM, Corneliu ZUZU wrote:
> On 2/26/2016 1:56 PM, Jan Beulich wrote:
> On 26.02.16 at 12:07,  wrote:
>>> --- a/xen/include/asm-x86/altp2m.h
>>> +++ b/xen/include/asm-x86/altp2m.h
>>> @@ -15,8 +15,8 @@
>>>* this program; If not, see .
>>>*/
>>>   -#ifndef _X86_ALTP2M_H
>>> -#define _X86_ALTP2M_H
>>> +#ifndef __ASM_X86_ALTP2M_H
>>> +#define __ASM_X86_ALTP2M_H
>> Unrelated change? (No need to undo, but please don't mix such
>> into patches especially when they are quite large already anyway.)
> 
> Noted.
> 
>>> @@ -33,5 +33,9 @@ void altp2m_vcpu_initialise(struct vcpu *v);
>>>   void altp2m_vcpu_destroy(struct vcpu *v);
>>>   void altp2m_vcpu_reset(struct vcpu *v);
>>>   -#endif /* _X86_ALTP2M_H */
>>> +static inline uint16_t altp2m_vcpu_idx(struct vcpu *v)
>> const
> 
> 'const', as in:
> 
> +static inline const uint16_t altp2m_vcpu_idx(struct vcpu *v)

Since there's no functional difference between returning const uint6_t
and plain uint16_t, I assume that Jan meant "const struct vcpu *v".


Cheers,
Razvan

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

Re: [Xen-devel] [PATCH v5 08/22] arm/acpi: Parse MADT to map logical cpu to MPIDR and get cpu_possible_map

[Stefano Stabellini]

On Fri, 26 Feb 2016, Shannon Zhao wrote:
> From: Parth Dixit 
> 
> MADT contains the information for MPIDR which is essential for SMP
> initialization, parse the GIC cpu interface structures to get the MPIDR
> value and map it to cpu_logical_map(), and add enabled cpu with valid
> MPIDR into cpu_possible_map.
> 
> Move BAD_MADT_ENTRY to common place and parenthesize its parameters.
> 
> Cc: Jan Beulich 
> Signed-off-by: Hanjun Guo 
> Signed-off-by: Tomasz Nowicki 
> Signed-off-by: Naresh Bhat 
> Signed-off-by: Parth Dixit 
> Signed-off-by: Shannon Zhao 
> ---
> V5: drop bootcpu_valid and parenthesize parameters of BAD_MADT_ENTRY
> V4: fix coding style and address some comments
> ---
>  xen/arch/arm/acpi/boot.c   | 121 
> +
>  xen/arch/x86/acpi/boot.c   |   4 --
>  xen/include/asm-arm/acpi.h |   1 +
>  xen/include/xen/acpi.h |   4 ++
>  4 files changed, 126 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 6b33fbe..066b590 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -32,6 +32,127 @@
>  #include 
>  
>  #include 
> +#include 
> +
> +/* Processors with enabled flag and sane MPIDR */
> +static unsigned int enabled_cpus;
> +
> +/* total number of cpus in this system */
> +static unsigned int __initdata total_cpus;
> +
> +/*
> + * acpi_map_gic_cpu_interface - generates a logical cpu number
> + * and map to MPIDR represented by GICC structure
> + */
> +static void __init
> +acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor)
> +{
> +int i;
> +u64 mpidr = processor->arm_mpidr & MPIDR_HWID_MASK;
> +bool_t enabled = !!(processor->flags & ACPI_MADT_ENABLED);
> +
> +if ( mpidr == MPIDR_INVALID )
> +{
> +printk("Skip MADT cpu entry with invalid MPIDR\n");
> +return;
> +}
> +
> +total_cpus++;
> +if ( !enabled )
> +return;
> +
> +if ( enabled_cpus >=  NR_CPUS )
> +{
> +printk("NR_CPUS limit of %d reached, Processor %d/0x%"PRIx64" 
> ignored.\n",
> +   NR_CPUS, total_cpus, mpidr);
> +return;
> +}
> +
> +/* Check if GICC structure of boot CPU is available in the MADT */
> +if ( (enabled_cpus == 0) && (cpu_logical_map(0) != mpidr) )
> +{
> +printk("Firmware bug, duplicate CPU MPIDR: 0x%"PRIx64" in MADT\n",
> +   mpidr);

Please change this error message into something more appropriate, like:

printk("Firmware bug, invalid CPU MPIDR for cpu0: 0x%"PRIx64" in 
MADT\n",
mpidr);

with this change:

Reviewed-by: Stefano Stabellini 


> +return;
> +}
> +
> +/*
> + * Duplicate MPIDRs are a recipe for disaster. Scan
> + * all initialized entries and check for
> + * duplicates. If any is found just ignore the CPU.
> + */
> +for ( i = 0; i < enabled_cpus; i++ )
> +{
> +if ( cpu_logical_map(i) == mpidr )
> +{
> +printk("Firmware bug, duplicate CPU MPIDR: 0x%"PRIx64" in 
> MADT\n",
> +   mpidr);
> +return;
> +}
> +}
> +
> +if ( !acpi_psci_present() )
> +return;
> +
> +/* CPU 0 was already initialized */
> +if ( enabled_cpus )
> +{
> +if ( arch_cpu_init(enabled_cpus, NULL) < 0 )
> +return;
> +
> +/* map the logical cpu id to cpu MPIDR */
> +cpu_logical_map(enabled_cpus) = mpidr;
> +}
> +
> +enabled_cpus++;
> +}
> +
> +static int __init
> +acpi_parse_gic_cpu_interface(struct acpi_subtable_header *header,
> + const unsigned long end)
> +{
> +struct acpi_madt_generic_interrupt *processor =
> +   container_of(header, struct acpi_madt_generic_interrupt, 
> header);
> +
> +if ( BAD_MADT_ENTRY(processor, end) )
> +return -EINVAL;
> +
> +acpi_table_print_madt_entry(header);
> +acpi_map_gic_cpu_interface(processor);
> +return 0;
> +}
> +
> +/* Parse GIC cpu interface entries in MADT for SMP init */
> +void __init acpi_smp_init_cpus(void)
> +{
> +int count, i;
> +
> +/*
> + * do a partial walk of MADT to determine how many CPUs
> + * we have including disabled CPUs, and get information
> + * we need for SMP init
> + */
> +count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
> +acpi_parse_gic_cpu_interface, 0);
> +
> +if ( count <= 0 )
> +{
> +printk("Error parsing GIC CPU interface entry\n");
> +return;
> +}
> +
> +if ( enabled_cpus > 1 )
> +{
> +printk("MADT missing boot CPU MPIDR, not enabling secondaries\n");
> +return;
> +}
> +
> +for ( i = 0; i < enabled_cpus; i++ )
> +cpumask_set_cpu(i, &cpu_possible_map);
> +
> +/* Make boot-up look pretty */
> +printk("%d CPUs enabled, %d CPUs total\n", enabled_cpus, total_cpus);
> +}
>  
>  static int __init acpi_parse_fadt(s

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Corneliu ZUZU]

On 2/26/2016 2:14 PM, Razvan Cojocaru wrote:

On 02/26/2016 02:05 PM, Corneliu ZUZU wrote:

On 2/26/2016 1:56 PM, Jan Beulich wrote:

On 26.02.16 at 12:07,  wrote:

--- a/xen/include/asm-x86/altp2m.h
+++ b/xen/include/asm-x86/altp2m.h
@@ -15,8 +15,8 @@
* this program; If not, see .
*/
   -#ifndef _X86_ALTP2M_H
-#define _X86_ALTP2M_H
+#ifndef __ASM_X86_ALTP2M_H
+#define __ASM_X86_ALTP2M_H

Unrelated change? (No need to undo, but please don't mix such
into patches especially when they are quite large already anyway.)

Noted.


@@ -33,5 +33,9 @@ void altp2m_vcpu_initialise(struct vcpu *v);
   void altp2m_vcpu_destroy(struct vcpu *v);
   void altp2m_vcpu_reset(struct vcpu *v);
   -#endif /* _X86_ALTP2M_H */
+static inline uint16_t altp2m_vcpu_idx(struct vcpu *v)

const

'const', as in:

+static inline const uint16_t altp2m_vcpu_idx(struct vcpu *v)

Since there's no functional difference between returning const uint6_t
and plain uint16_t, I assume that Jan meant "const struct vcpu *v".


I thought the functional difference would be when calling:

uint16_t idx = altp2m_vcpu_idx(v); // => can subsequently modify idx
const uint16_t idx = altp2m_vcpu_idx(v); // => cannot subsequently 
modify idx (unless const is casted to non-const)





Cheers,
Razvan



Corneliu.

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

Re: [Xen-devel] [PATCH 1/8] tools/xenalyze: Close symbol_file after reading it

[Ian Jackson]

George Dunlap writes ("[PATCH 1/8] tools/xenalyze: Close symbol_file after 
reading it"):
> ...to avoid leaking the FD and associated memory.
> 
> CID 1306872
> 
> Signed-off-by: George Dunlap 

Acked-by: Ian Jackson 

> Not sure if this will actually make coverity happy, or if we need to
> actually set symbol_file to NULL.

I don't see why you'd need to assign to the symbol_file local (which
is about to go out of scope).

Ian.

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

Re: [Xen-devel] [PATCH 2/8] tools/xenalyze: Avoid redundant check

[Ian Jackson]

George Dunlap writes ("[PATCH 2/8] tools/xenalyze: Avoid redundant check"):
> Coverity notices that if !head is true, that !N can never be true.
> 
> Don't bother checking N, since we know it has to be at least one.
> 
> CID 1354243
> 
> Signed-off-by: George Dunlap 

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH 3/8] tools/xenalyze: Handle fstat errors properly

[Ian Jackson]

George Dunlap writes ("[PATCH 3/8] tools/xenalyze: Handle fstat errors 
properly"):
> They're pretty unlikely to fail, but doesn't hurt to check.
...
> -fstat(fd, &s);
> +if ( fstat(fd, &s) ) {
> +perror("fstat");
> +free(h);
> +h = NULL;
> +goto out;
> +}

Is there some reason why you're not calling exit ?  mread64 already
does so on mmap failure.

For that matter, is error() not available in this file ?

>  struct stat s;
> -fstat(G.fd, &s);
> +if ( fstat(G.fd, &s) ) {
> +perror("fstat");
> +error(ERR_SYSTEM, NULL);
> +}
>  G.file_size = s.st_size;

This hunk LGTM.

Ian.

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

Re: [Xen-devel] [PATCH 4/8] tools/xenalyze: Mark unreachable code as unreachable

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-devel] [PATCH 4/8] tools/xenalyze: Mark 
unreachable code as unreachable"):
> On Thu, 2016-02-25 at 15:43 +, George Dunlap wrote:
> > Right -- well basically error(ASSERT,...) is a custom abort().  But in
> > the current case it isn't actually doing anything more than an abort()
> > would, so perhaps I should use that instead (since coverity knows about
> > abort() and assert() but not my custom function).
> 
> Personally this is what I would do in this case.

Indeed.  I would replace the fprintf/error with

   assert(!"logic hole in this function");

This is a standard idiom for `abort()' when you don't like to just
call abort because it doesn't produce a message.

Ian.

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

Re: [Xen-devel] [PATCH 5/8] tools/xenalyze: Fix check for error return value

[Ian Jackson]

George Dunlap writes ("[PATCH 5/8] tools/xenalyze: Fix check for error return 
value"):
> fdopen returns NULL on failure, not a negative integer.
> 
> CID 1306863
> CID 1306858
> 
> Signed-off-by: George Dunlap 
> ---
> Strangely, coverity counted this as two bugs: one for comparing a
> pointer with a negative integer, one for the comparison never possibly
> being true (thus making the if condition dead code).

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Razvan Cojocaru]

On 02/26/2016 02:20 PM, Corneliu ZUZU wrote:
> On 2/26/2016 2:14 PM, Razvan Cojocaru wrote:
>> On 02/26/2016 02:05 PM, Corneliu ZUZU wrote:
>>> On 2/26/2016 1:56 PM, Jan Beulich wrote:
>>> On 26.02.16 at 12:07,  wrote:
> --- a/xen/include/asm-x86/altp2m.h
> +++ b/xen/include/asm-x86/altp2m.h
> @@ -15,8 +15,8 @@
> * this program; If not, see .
> */
>-#ifndef _X86_ALTP2M_H
> -#define _X86_ALTP2M_H
> +#ifndef __ASM_X86_ALTP2M_H
> +#define __ASM_X86_ALTP2M_H
 Unrelated change? (No need to undo, but please don't mix such
 into patches especially when they are quite large already anyway.)
>>> Noted.
>>>
> @@ -33,5 +33,9 @@ void altp2m_vcpu_initialise(struct vcpu *v);
>void altp2m_vcpu_destroy(struct vcpu *v);
>void altp2m_vcpu_reset(struct vcpu *v);
>-#endif /* _X86_ALTP2M_H */
> +static inline uint16_t altp2m_vcpu_idx(struct vcpu *v)
 const
>>> 'const', as in:
>>>
>>> +static inline const uint16_t altp2m_vcpu_idx(struct vcpu *v)
>> Since there's no functional difference between returning const uint6_t
>> and plain uint16_t, I assume that Jan meant "const struct vcpu *v".
> 
> I thought the functional difference would be when calling:
> 
> uint16_t idx = altp2m_vcpu_idx(v); // => can subsequently modify idx
> const uint16_t idx = altp2m_vcpu_idx(v); // => cannot subsequently
> modify idx (unless const is casted to non-const)

True, but you can write that code regardless of whether
altp2m_vcpu_idx(v) returns const or non-const uint16_t, so saying "const
uint16_t altp2m_vcpu_idx(struct vcpu *v)" instead of "uint16_t
altp2m_vcpu_idx(struct vcpu *v)" only add syntactic noise with no actual
benefit.


Cheers,
Razvan

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

Re: [Xen-devel] [PATCH 6/8] tools/xenalyze: Fix off-by-one in MAX_CPUS range checks

[Ian Jackson]

George Dunlap writes ("[PATCH 6/8] tools/xenalyze: Fix off-by-one in MAX_CPUS 
range checks"):
> Skip action / throw error if cpu/vcpu >= MAX_CPUS  rather than >.
> 
> Also add an assertion to vcpu_find, to make future errors of this kind
> not out-of-bounds.
...
> +/* "Graceful" handling of vid >= MAX_CPUS should be handled elsewhere */
> +if ( vid >= MAX_CPUS ) {
> +fprintf(stderr, "%s: vcpu %d exceeds MAX_CPUS %d!\n",
> +__func__, vid, MAX_CPUS);
> +error(ERR_ASSERT, NULL);
> +}

I'm not convinced by the existence of error(ERR_ASSERT,...).  What is
wrong with assert() ?

If you agree that ERR_ASSERT should be got rid of, then you could
start here...

But:

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Jan Beulich]

>>> On 26.02.16 at 13:20,  wrote:
> On 2/26/2016 2:14 PM, Razvan Cojocaru wrote:
>> On 02/26/2016 02:05 PM, Corneliu ZUZU wrote:
>>> On 2/26/2016 1:56 PM, Jan Beulich wrote:
>>> On 26.02.16 at 12:07,  wrote:
> --- a/xen/include/asm-x86/altp2m.h
> +++ b/xen/include/asm-x86/altp2m.h
> @@ -15,8 +15,8 @@
> * this program; If not, see .
> */
>-#ifndef _X86_ALTP2M_H
> -#define _X86_ALTP2M_H
> +#ifndef __ASM_X86_ALTP2M_H
> +#define __ASM_X86_ALTP2M_H
 Unrelated change? (No need to undo, but please don't mix such
 into patches especially when they are quite large already anyway.)
>>> Noted.
>>>
> @@ -33,5 +33,9 @@ void altp2m_vcpu_initialise(struct vcpu *v);
>void altp2m_vcpu_destroy(struct vcpu *v);
>void altp2m_vcpu_reset(struct vcpu *v);
>-#endif /* _X86_ALTP2M_H */
> +static inline uint16_t altp2m_vcpu_idx(struct vcpu *v)
 const
>>> 'const', as in:
>>>
>>> +static inline const uint16_t altp2m_vcpu_idx(struct vcpu *v)
>> Since there's no functional difference between returning const uint6_t
>> and plain uint16_t, I assume that Jan meant "const struct vcpu *v".
> 
> I thought the functional difference would be when calling:
> 
> uint16_t idx = altp2m_vcpu_idx(v); // => can subsequently modify idx
> const uint16_t idx = altp2m_vcpu_idx(v); // => cannot subsequently 
> modify idx (unless const is casted to non-const)

That's correct, but for this the return type of the function doesn't
matter. In fact I'd expect the compiler to warn about a meaningless
modifier placed on a function return type.

Jan


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

Re: [Xen-devel] [PATCH v5 20/22] arm/acpi: Initialize serial port from ACPI SPCR table

[Jan Beulich]

>>> On 26.02.16 at 12:10,  wrote:

> 
> On 2016/2/26 18:20, Shannon Zhao wrote:
>> 
>> On 2016/2/26 18:04, Jan Beulich wrote:
>>  On 26.02.16 at 07:22,  wrote:
> >> > From: Shannon Zhao 
> >> > 
> >> > Parse ACPI SPCR (Serial Port Console Redirection table) table and
> >> > initialize the serial port pl011.
> >> > 
> >> > Signed-off-by: Parth Dixit 
> >> > Signed-off-by: Shannon Zhao 
> >> > Reviewed-by: Stefano Stabellini 
>>> > Acked-by: Jan Beulich 
>>> > 
>> Thanks a lot!
>> 
>>> > It's not obvious to me whether this could go in ahead of most of
>>> > the rest of the series, together with patch 19 (which clearly can
>>> > go in right away).
>> I think this one could go in ahead with patch 19 since it's independent.
> Actually it's not that independent since it relies on patch 11 to
> compile. So this one could be picked up by Ian if you don't mind.

I don't think Ian's going to pick up any of these patches
anymore. I've applied it already, fixing the build issue.

Jan


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

Re: [Xen-devel] [PATCH 7/8] tools/xenalyze: Fix multiple instances of *HYPERCALL_MAX

[Ian Jackson]

George Dunlap writes ("[PATCH 7/8] tools/xenalyze: Fix multiple instances of 
*HYPERCALL_MAX"):
> We HYPERCALL_MAX defined as the maximum enumerated hypercall, and we
^ missing word `have' ?

> have PV_HYPERCALL_MAX defined as some other number (presumably based
> on experience with actual hypercalls).  Both are used to size arrays
> (hypercall_name[] and pv_data.hypercall_count[], respectively).
> 
> Rename PV_HYPERCALL_MAX to HYPERCALL_MAX, and use HYPERCALL_MAX to
> size (and iterate over) all arrays.
...
> diff --git a/tools/xentrace/xenalyze.c b/tools/xentrace/xenalyze.c
> index 3e26a4c..4ae50b8 100644
> --- a/tools/xentrace/xenalyze.c
> +++ b/tools/xentrace/xenalyze.c
> @@ -1068,9 +1068,10 @@ enum {
>  HYPERCALL_sysctl,
>  HYPERCALL_domctl,
>  HYPERCALL_kexec_op,
> -HYPERCALL_MAX
>  };
>  
> +#define HYPERCALL_MAX 38
> +
>  char *hypercall_name[HYPERCALL_MAX] = {
>  [HYPERCALL_set_trap_table]="set_trap_table",
>  [HYPERCALL_mmu_update]="mmu_update",
> @@ -1509,13 +1510,12 @@ char *pv_name[PV_MAX] = {
>  [PV_HYPERCALL_SUBCALL]="hypercall (subcall)",

Does this produce a build error if HYPERCALL_MAX is too small ?

Ian.

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

Re: [Xen-devel] [PATCH v5 5/7] VT-d: Refactor iommu_ops .map_page() and unmap_page()

[Jan Beulich]

>>> On 26.02.16 at 12:48,  wrote:
> I would summarize as follows and make sure we are on the same page.
> 
>  1) make pcidevs_lock only visible inside xen/drivers/passthrough/pci.c, 
>turning the definition of the variable into a static one, and getting rid 
> of its declaration from xen/include/xen/pci.h
> 
>  2) define 3 helpers in 'include/xen/pci.h':
> *pcidevs_lock()
> *pcidevs_unlock()
> *pcidevs_is_locked()
>   Then, I'd implement these helpers in xen/drivers/passthrough/pci.c. 
>   Of course, the helpers will do spin_lock_recursive() and 
> spin_unlock_recursive(). It is quite similar to what is done for 
> domain_lock().
> 
> 3) use these helpers. Replace
>  *spin_lock(&pcidevs_lock) with pcidevs_lock(),
>  *spin_unlock(&pcidevs_lock) with pcidevs_unlock(),
>  *spin_is_locked(&pcidevs_lock) with pcidevs_is_locked().

Yes.

Jan


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

Re: [Xen-devel] [PATCH 8/8] tools/xenalyze: Actually handle case where number of ipi vectors exceeds static max

[Ian Jackson]

George Dunlap writes ("[PATCH 8/8] tools/xenalyze: Actually handle case where 
number of ipi vectors exceeds static max"):
> find_vec() is supposed to find the vector in the list if it exists,
> choose an empty slot if it doesn't exist, and return null if all slots
> are full.
> 
> However, coverity noticed that although the callers of find_vec() handle
> the last condition, find_vec() itself didn't.
> 
> Check to see if we actually found an empty slot before attempting to
> initialize it.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH] docs: Update README to include Clang

[Ian Jackson]

Andrew Cooper writes ("[PATCH] docs: Update README to include Clang"):
> Xen now builds on x86 with Clang 3.5 and 3.8.  Update README to reflect this.
> 
> Mark Clang as no longer a permitted failure in Travis, to prevent future
> regressions slipping back in.
> 
> Signed-off-by: Andrew Cooper 

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Jan Beulich]

>>> On 26.02.16 at 12:20,  wrote:
> On Fri, 2016-02-26 at 12:14 +0100, Juergen Gross wrote:
>> On 26/02/16 11:39, Jan Beulich wrote:
>> > 
>> > > @@ -670,7 +676,13 @@ int cpu_disable_scheduler(unsigned int cpu)
>> > >  if ( cpumask_empty(&online_affinity) &&
>> > >   cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
>> > >  {
>> > > -printk(XENLOG_DEBUG "Breaking affinity for
>> > > %pv\n", v);
>> > > +if ( v->affinity_broken )
>> > > +{
>> > > +/* The vcpu is temporarily pinned, can't
>> > > move it. */
>> > > +vcpu_schedule_unlock_irqrestore(lock, flags,
>> > > v);
>> > > +ret = -EBUSY;
>> > > +continue;
>> > > +}
>> > So far the function can only return 0 or -EAGAIN. By using
>> > "continue"
>> > here you will make it impossible for the caller to reliably
>> > determine
>> > whether possibly both things failed. Despite -EBUSY being a logical
>> > choice here, I think you'd better use -EAGAIN here too. And it
>> > needs
>> > to be determined whether continuing the loop in this as well as the
>> > pre-existing cases is actually the right thing to do.
>> EBUSY vs. EAGAIN: by returning EAGAIN I would signal to Xen tools
>> that
>> the hypervisor is currently not able to do the desired operation
>> (especially removing a cpu from a cpupool), but the situation will
>> change automatically via scheduling. EBUSY will stop retries in Xen
>> tools and this is want I want here: I can't be sure the situation
>> will change soon.
>> 
> I agree with this.

I'm of two minds here: I can see your viewpoint, but considering
this is called "temporarily pin a vcpu" the condition is supposed to
be going away again soon.

>> Regarding continuation of the loop: I think you are right in the
>> EBUSY case: I should break out of the loop. I should not do so in the
>> EAGAIN case as I want to remove as many vcpus from the physical cpu
>> as
>> possible without returning to the Xen tools in between.
>> 
> And with this too.
> 
> And I think that, if we indeed break out of the loop on EBUSY, that
> will also make it possible to figure out properly what actually went
> wrong, so it should be fine from that point of view as well.

Yes indeed.

>> > > @@ -679,6 +691,8 @@ int cpu_disable_scheduler(unsigned int cpu)
>> > >  v->affinity_broken = 1;
>> > >  }
>> > >  
>> > > +printk(XENLOG_DEBUG "Breaking affinity for
>> > > %pv\n", v);
>> > Wouldn't it be even better to make this the "else" to the
>> > preceding if(), since in the suspend case this is otherwise going
>> > to be printed for every vCPU not currently running on pCPU0?
>> Yes, I'll change it.
>> 
> On this, can (either of) you elaborate a bit more? I don't think I'm
> following...

In addition to Jürgen's reply: My main concern here is that on
a bug system this message would get printed for almost every
vCPU in the system, which could end up being a lot of noise.

And there's a similar message on the resume side I think -
perhaps that one should be silenced too.

Jan

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

Re: [Xen-devel] Xen 4.4, xenstored and WATCH requests

[Wei Liu]

(CC'ing Linux maintainers)

OK. Thanks for all the information.

This seems to be a bug in Linux xenbus driver.

Relevant code snippet in
drivers/xen/xenbus/xenbus_dev_frontend.c:xenbus_write_watch()


  /* Success.  Synthesize a reply to say all is OK. */
  {
  struct {
  struct xsd_sockmsg hdr;
  char body[3];
  } __packed reply = {
  {
  .type = msg_type,
  .len = sizeof(reply.body)
  },
  "OK"
  };
  
  mutex_lock(&u->reply_mutex);
  rc = queue_reply(&u->read_buffers, &reply, sizeof(reply));
  wake_up(&u->read_waitq);
  mutex_unlock(&u->reply_mutex);
  }

The synthesised reply doesn't echo the req_id back to user.

Wei.

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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Juergen Gross]

On 26/02/16 13:39, Jan Beulich wrote:
 On 26.02.16 at 12:20,  wrote:
>> On Fri, 2016-02-26 at 12:14 +0100, Juergen Gross wrote:
>>> On 26/02/16 11:39, Jan Beulich wrote:

> @@ -670,7 +676,13 @@ int cpu_disable_scheduler(unsigned int cpu)
>  if ( cpumask_empty(&online_affinity) &&
>   cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
>  {
> -printk(XENLOG_DEBUG "Breaking affinity for
> %pv\n", v);
> +if ( v->affinity_broken )
> +{
> +/* The vcpu is temporarily pinned, can't
> move it. */
> +vcpu_schedule_unlock_irqrestore(lock, flags,
> v);
> +ret = -EBUSY;
> +continue;
> +}
 So far the function can only return 0 or -EAGAIN. By using
 "continue"
 here you will make it impossible for the caller to reliably
 determine
 whether possibly both things failed. Despite -EBUSY being a logical
 choice here, I think you'd better use -EAGAIN here too. And it
 needs
 to be determined whether continuing the loop in this as well as the
 pre-existing cases is actually the right thing to do.
>>> EBUSY vs. EAGAIN: by returning EAGAIN I would signal to Xen tools
>>> that
>>> the hypervisor is currently not able to do the desired operation
>>> (especially removing a cpu from a cpupool), but the situation will
>>> change automatically via scheduling. EBUSY will stop retries in Xen
>>> tools and this is want I want here: I can't be sure the situation
>>> will change soon.
>>>
>> I agree with this.
> 
> I'm of two minds here: I can see your viewpoint, but considering
> this is called "temporarily pin a vcpu" the condition is supposed to
> be going away again soon.

It is supposed to do so, yes. But the hypervisor can't make sure it
will, as it requires an appropriate hypercall by the hardware domain.
In the cpupool case no domain is capable to make the situation
persist.

Would you be fine with adding a tools patch doing a limited number
of retries in the EBUSY case (maybe with sleeping 1 second in that
case)?

> 
>>> Regarding continuation of the loop: I think you are right in the
>>> EBUSY case: I should break out of the loop. I should not do so in the
>>> EAGAIN case as I want to remove as many vcpus from the physical cpu
>>> as
>>> possible without returning to the Xen tools in between.
>>>
>> And with this too.
>>
>> And I think that, if we indeed break out of the loop on EBUSY, that
>> will also make it possible to figure out properly what actually went
>> wrong, so it should be fine from that point of view as well.
> 
> Yes indeed.
> 
> @@ -679,6 +691,8 @@ int cpu_disable_scheduler(unsigned int cpu)
>  v->affinity_broken = 1;
>  }
>  
> +printk(XENLOG_DEBUG "Breaking affinity for
> %pv\n", v);
 Wouldn't it be even better to make this the "else" to the
 preceding if(), since in the suspend case this is otherwise going
 to be printed for every vCPU not currently running on pCPU0?
>>> Yes, I'll change it.
>>>
>> On this, can (either of) you elaborate a bit more? I don't think I'm
>> following...
> 
> In addition to Jürgen's reply: My main concern here is that on
> a bug system this message would get printed for almost every
> vCPU in the system, which could end up being a lot of noise.
> 
> And there's a similar message on the resume side I think -
> perhaps that one should be silenced too.

Okay. I'll do the silencing (both cases) in an extra patch.


Juergen

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Corneliu ZUZU]

On 2/26/2016 2:31 PM, Jan Beulich wrote:

On 26.02.16 at 13:20,  wrote:

On 2/26/2016 2:14 PM, Razvan Cojocaru wrote:

On 02/26/2016 02:05 PM, Corneliu ZUZU wrote:

On 2/26/2016 1:56 PM, Jan Beulich wrote:

On 26.02.16 at 12:07,  wrote:

--- a/xen/include/asm-x86/altp2m.h
+++ b/xen/include/asm-x86/altp2m.h
@@ -15,8 +15,8 @@
 * this program; If not, see .
 */
-#ifndef _X86_ALTP2M_H
-#define _X86_ALTP2M_H
+#ifndef __ASM_X86_ALTP2M_H
+#define __ASM_X86_ALTP2M_H

Unrelated change? (No need to undo, but please don't mix such
into patches especially when they are quite large already anyway.)

Noted.


@@ -33,5 +33,9 @@ void altp2m_vcpu_initialise(struct vcpu *v);
void altp2m_vcpu_destroy(struct vcpu *v);
void altp2m_vcpu_reset(struct vcpu *v);
-#endif /* _X86_ALTP2M_H */
+static inline uint16_t altp2m_vcpu_idx(struct vcpu *v)

const

'const', as in:

+static inline const uint16_t altp2m_vcpu_idx(struct vcpu *v)

Since there's no functional difference between returning const uint6_t
and plain uint16_t, I assume that Jan meant "const struct vcpu *v".

I thought the functional difference would be when calling:

uint16_t idx = altp2m_vcpu_idx(v); // => can subsequently modify idx
const uint16_t idx = altp2m_vcpu_idx(v); // => cannot subsequently
modify idx (unless const is casted to non-const)

That's correct, but for this the return type of the function doesn't
matter. In fact I'd expect the compiler to warn about a meaningless
modifier placed on a function return type.

Jan




I find having

static inline const uint16_t altp2m_vcpu_idx(struct vcpu *v)

and subsequently writing

uint16_t idx = altp2m_vcpu_idx(v);

instead of

const uint16_t idx = altp2m_vcpu_idx(v);

without the compiler throwing at least a warning counter-intuitive.

Reminds me of something Linus said in an email: "it would be *stupid* 
for a C compiler to do anything but what we assume it does".


Noted, will change to 'const struct vcpu *v'.

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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Dario Faggioli]

On Fri, 2016-02-26 at 05:39 -0700, Jan Beulich wrote:
> > 
> > > > On 26.02.16 at 12:20,  wrote:
> > On Fri, 2016-02-26 at 12:14 +0100, Juergen Gross wrote:
> > > 
> > > EBUSY vs. EAGAIN: by returning EAGAIN I would signal to Xen tools
> > > that
> > > the hypervisor is currently not able to do the desired operation
> > > (especially removing a cpu from a cpupool), but the situation
> > > will
> > > change automatically via scheduling. EBUSY will stop retries in
> > > Xen
> > > tools and this is want I want here: I can't be sure the situation
> > > will change soon.
> > > 
> > I agree with this.
> I'm of two minds here: I can see your viewpoint, but considering
> this is called "temporarily pin a vcpu" the condition is supposed to
> be going away again soon.
> 
Maybe one difference is that it won't go away "by itself". I.e., just
retrying in a while, especially if from inside Xen, and without anyone
explicitly calling the hypercall again with proper argument, nothing
will change.

> > > > Wouldn't it be even better to make this the "else" to the
> > > > preceding if(), since in the suspend case this is otherwise
> > > > going
> > > > to be printed for every vCPU not currently running on pCPU0?
> > > Yes, I'll change it.
> > > 
> > On this, can (either of) you elaborate a bit more? I don't think
> > I'm
> > following...
> In addition to Jürgen's reply: My main concern here is that on
> a bug system this message would get printed for almost every
> vCPU in the system, which could end up being a lot of noise.
> 
> And there's a similar message on the resume side I think -
> perhaps that one should be silenced too.
> 
What I don't understand is this part of your first comment "in the
suspend case this is otherwise going to be printed for every vCPU not
currently running on pCPU0".

First, do you mean with Juergen's patch, or even right now?

And anyway, this is going to be printed for all the vCPUs that does not
have, in their hard affinity, any of the pCPUs that are going to remain
online (or to remain in the domain's cpupool).

In shutdown and suspend, when we try to move everything to pCPU 0, it
will get printed for all the vCPUs that does not have pCPU 0 in their
hard affinity.

We can argue about that being useful or not, and about it being
(potentially) too noisy or not. I personally think it could be useful
(it's XENLOG_DEBUG, after all), but I won't oppose getting rid of it...
I am just not getting why you're saying "not currently running on
pCPU0".

Thanks and Regards,
Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D 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 v4 3/4] libelf: rewrite symtab/strtab loading

[Jan Beulich]

>>> On 16.02.16 at 18:37,  wrote:
> --- a/xen/common/libelf/libelf-loader.c
> +++ b/xen/common/libelf/libelf-loader.c
> @@ -164,20 +164,33 @@ void elf_parse_bsdsyms(struct elf_binary *elf, uint64_t 
> pstart)
>  sz = sizeof(uint32_t);
>  
>  /* Space for the elf and elf section headers */
> -sz += (elf_uval(elf, elf->ehdr, e_ehsize) +
> -   elf_shdr_count(elf) * elf_uval(elf, elf->ehdr, e_shentsize));
> +sz += elf_uval(elf, elf->ehdr, e_ehsize) +
> +  3 * elf_uval(elf, elf->ehdr, e_shentsize);

I think a literal 3 like this either needs a #define (at once serving
as documentation) or a comment. Perhaps rather the former,
considering that the (same?) 3 appears again further down. Plus -
what guarantees there are 3 section headers in the image?

>  sz = elf_round_up(elf, sz);
>  
> -/* Space for the symbol and string tables. */
> +/* Space for the symbol and string table. */
>  for ( i = 0; i < elf_shdr_count(elf); i++ )
>  {
>  shdr = elf_shdr_by_index(elf, i);
>  if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
>  /* input has an insane section header count field */
>  break;
> -type = elf_uval(elf, shdr, sh_type);
> -if ( (type == SHT_STRTAB) || (type == SHT_SYMTAB) )
> -sz = elf_round_up(elf, sz + elf_uval(elf, shdr, sh_size));
> +
> +if ( elf_uval(elf, shdr, sh_type) != SHT_SYMTAB )
> +continue;
> +
> +sz = elf_round_up(elf, sz + elf_uval(elf, shdr, sh_size));
> +shdr = elf_shdr_by_index(elf, elf_uval(elf, shdr, sh_link));
> +
> +if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) )
> +/* input has an insane section header count field */
> +break;
> +
> +if ( elf_uval(elf, shdr, sh_type) != SHT_STRTAB )
> +/* Invalid symtab -> strtab link */
> +break;

This is not sufficient - what if sh_link is out of bounds, but the
bogusly accessed field happens to hold SHT_STRTAB? (Oddly
enough you have at least an SHN_UNDEF check in the second
loop below.)

Also even if (I assume) elf_load_image() would refuse to read
outside the bounds of the image, wouldn't you better check here
that both symtab and strtab are within image bounds? The more
that you ignore errors from elf_load_image() (and
elf_mem{cpy,set}_safe() don't even return any)?

> @@ -186,79 +199,169 @@ void elf_parse_bsdsyms(struct elf_binary *elf, 
> uint64_t pstart)
>  
>  static void elf_load_bsdsyms(struct elf_binary *elf)
>  {
> -ELF_HANDLE_DECL(elf_ehdr) sym_ehdr;
> -unsigned long sz;
> -elf_ptrval maxva;
> -elf_ptrval symbase;
> -elf_ptrval symtab_addr;
> -ELF_HANDLE_DECL(elf_shdr) shdr;
> -unsigned i, type;
> +/*
> + * Header that is placed at the end of the kernel and allows
> + * the OS to find where the symtab and strtab have been loaded.
> + * It mimics a valid ELF file header, although it only contains
> + * a symtab and a strtab section.
> + *
> + * NB: according to the ELF spec there's only ONE symtab per ELF
> + * file, and accordingly we will only load the corresponding
> + * strtab, so we only need three section headers in our fake ELF
> + * header (first section header is always a dummy).
> + */
> +struct {
> +uint32_t size;
> +struct {
> +elf_ehdr header;
> +elf_shdr section[3];
> +} __attribute__((packed)) elf_header;
> +} __attribute__((packed)) header;

If this is placed at the end, how can the OS reasonably use it
without knowing that there are exactly 3 section header
entries? I.e. how would it find the base of this structure?

> +ELF_HANDLE_DECL(elf_ehdr) header_handle;
> +unsigned long shdr_size;
> +ELF_HANDLE_DECL(elf_shdr) section_handle;
> +ELF_HANDLE_DECL(elf_shdr) image_handle;
> +unsigned int i, link;
> +elf_ptrval header_base;
> +elf_ptrval elf_header_base;
> +elf_ptrval symtab_base;
> +elf_ptrval strtab_base;
>  
>  if ( !elf->bsd_symtab_pstart )
>  return;
>  
> -#define elf_hdr_elm(_elf, _hdr, _elm, _val) \
> -do {\
> -if ( elf_64bit(_elf) )  \
> -elf_store_field(_elf, _hdr, e64._elm, _val);  \
> -else\
> -elf_store_field(_elf, _hdr, e32._elm, _val);  \
> +#define elf_store_field_bitness(_elf, _hdr, _elm, _val) \
> +do {\
> +if ( elf_64bit(_elf) )  \
> +elf_store_field(_elf, _hdr, e64._elm, _val);\
> +else\
> +elf_store_field(_elf, _hdr, e32._elm, _val);\
>  } while ( 0 )
>  
> -symbase = elf_get_ptr(elf, elf->bsd_symtab_pstart);
> -symtab_addr = maxva = sym

[Xen-devel] [linux-3.14 test] 84019: trouble: blocked/broken/fail/pass

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-pair 3 host-install/src_host(3) broken REGR. vs. 83009
 test-amd64-amd64-pair3 host-install/src_host(3) broken REGR. vs. 83009
 test-amd64-i386-freebsd10-amd64  3 host-install(3)  broken REGR. vs. 83009
 test-amd64-i386-freebsd10-i386  3 host-install(3)   broken REGR. vs. 83009
 test-amd64-amd64-xl-qemut-winxpsp3  3 host-install(3)   broken REGR. vs. 83009

Regressions which are regarded as allowable (not blocking):
 build-amd64-rumpuserxen   6 xen-buildfail   like 83009
 build-i386-rumpuserxen6 xen-buildfail   like 83009
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 83009
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 83009
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 83009
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 83009

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-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass

version targeted for testing:
 linux0b2737d1affeecfcf2f5ceeadeecfcbecba21e35
baseline version:
 linux462982e3b5c5a9802bab17f75e8405201891e04d

Last test of basis83009  2016-02-18 14:52:37 Z7 days
Testing same since84019  2016-02-25 20:28:07 Z0 days1 attempts


People who touched revisions under test:
  Alan Stern 
  Alex Thorlton 
  Andrew Banman 
  Andrew Elble 
  Andrew Gabbasov 
  Andrew Lunn 
  Andrew Morton 
  Anson Huang 
  Anton Protopopov 
  Arnaldo Carvalho de Melo 
  Arnaldo Carvalho de Melo 
  Arnd Bergmann 
  Aurélien Francillon 
  Bart Van Assche 
  Benjamin LaHaise 
  Benjamin Tissoires 
  Borislav Petkov 
  Chris Mason 
  Christoph Hellwig 
  Cong Wang 
  CQ Tang 
  Dan Carpenter 
  Dan Williams 
  Daniel Mentz 
  Darren Hart 
  David S. Miller 
  David Sterba 
  David Woodhouse 
  Dmitry Torokhov 
  Dmitry Vyukov 
  Eric Dumazet 
  Erich Schubert 
  Ewan D. Milne 
  Filipe Manana 
  Greg Kroah-Hartman 
  Gregory CLEMENT 
  Hannes Reinecke 
  Helmut Klein 
  Herton R. Krzesinski 
  Holger Hoffstätte 
  Imre Deak 
  Ingo Molnar 
  Insu Yun 
  James Bottomley 
  James Bottomley 
  Jan Kara 
  Jani Nikula 
  Jann Horn 
  Jens Axboe 
  Joe Lawrence 
  Johannes Thumshirn 
  Jonathan Cameron 
  Kees Cook 
  Ken Xue 
  Kirill A. Shutemov 
  Kishon Vijay Abraham I 
  Konstantin Khlebnikov 
  Lars-Peter Clausen 
  Laura Abbott 
  Linus Torvalds 
  Linus Walleij 
  Martijn Coenen 
  Martin K. Petersen 
  Mathias Nyman 
  Matt Fleming 
  Matthew Wilcox 
  Michael Hennerich 
  Michal Hocko 
  Mika Westerberg 
  Miklos Szeredi 
  Namhyung Kim 
  Naoya Horiguchi 
  Nicholas Bellinger 
  Nicolas Pitre 
  Paul Mackerras 
  Paul Menzel 
  Peter Feiner 
  Peter Hurley 
  Peter Oberparleiter 
  Peter Zijlstra (Intel) 
  Peter Zijlstra 
  Roman Gushchin 
  Russell King 
  Rusty Russell 
  Sebastian Herbszt 
  Sergey Senozhatsky 
  Sergey Senozhatsky 
  Soeren Grunewald 
  Steve French 
  Steve French 
  Steven Rostedt 
  Sudip Mukherjee 
  Sudip Mukherjee 
  Takashi Iwai 
  Theodore Ts'o 
  Thomas Gleixner 
  Thomas Huth 
  Tony Lindgren 
  Trond Myklebust 
  Vasily Averin 
  Vegard Nossum 
  Vladimir Zapolskiy 
  Vlastimil Babka 
  WANG Cong 
  Willy Tarreau 
  Yong Li 

jobs:
 build-amd64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvops

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Jan Beulich]

>>> On 26.02.16 at 14:07,  wrote:
> On Fri, 2016-02-26 at 05:39 -0700, Jan Beulich wrote:
>> > > > On 26.02.16 at 12:20,  wrote:
>> > On Fri, 2016-02-26 at 12:14 +0100, Juergen Gross wrote:
>> > > > Wouldn't it be even better to make this the "else" to the
>> > > > preceding if(), since in the suspend case this is otherwise
>> > > > going
>> > > > to be printed for every vCPU not currently running on pCPU0?
>> > > Yes, I'll change it.
>> > > 
>> > On this, can (either of) you elaborate a bit more? I don't think
>> > I'm
>> > following...
>> In addition to Jürgen's reply: My main concern here is that on
>> a bug system this message would get printed for almost every
>> vCPU in the system, which could end up being a lot of noise.
>> 
>> And there's a similar message on the resume side I think -
>> perhaps that one should be silenced too.
>> 
> What I don't understand is this part of your first comment "in the
> suspend case this is otherwise going to be printed for every vCPU not
> currently running on pCPU0".
> 
> First, do you mean with Juergen's patch, or even right now?

Even right now. Just that his patch made this pretty obvious.

> And anyway, this is going to be printed for all the vCPUs that does not
> have, in their hard affinity, any of the pCPUs that are going to remain
> online (or to remain in the domain's cpupool).
> 
> In shutdown and suspend, when we try to move everything to pCPU 0, it
> will get printed for all the vCPUs that does not have pCPU 0 in their
> hard affinity.
> 
> We can argue about that being useful or not, and about it being
> (potentially) too noisy or not. I personally think it could be useful
> (it's XENLOG_DEBUG, after all), but I won't oppose getting rid of it...
> I am just not getting why you're saying "not currently running on
> pCPU0".

Oh, you're right, that was too strict - "not being allowed to run
on CPU0" would be the right description. And indeed that makes
it look not as noisy (but too much, since during suspend this is
what one has to expect would happen).

Jan

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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Jan Beulich]

>>> On 26.02.16 at 13:49,  wrote:
> On 26/02/16 13:39, Jan Beulich wrote:
> On 26.02.16 at 12:20,  wrote:
>>> On Fri, 2016-02-26 at 12:14 +0100, Juergen Gross wrote:
 On 26/02/16 11:39, Jan Beulich wrote:
>
>> @@ -670,7 +676,13 @@ int cpu_disable_scheduler(unsigned int cpu)
>>  if ( cpumask_empty(&online_affinity) &&
>>   cpumask_test_cpu(cpu, v->cpu_hard_affinity) )
>>  {
>> -printk(XENLOG_DEBUG "Breaking affinity for
>> %pv\n", v);
>> +if ( v->affinity_broken )
>> +{
>> +/* The vcpu is temporarily pinned, can't
>> move it. */
>> +vcpu_schedule_unlock_irqrestore(lock, flags,
>> v);
>> +ret = -EBUSY;
>> +continue;
>> +}
> So far the function can only return 0 or -EAGAIN. By using
> "continue"
> here you will make it impossible for the caller to reliably
> determine
> whether possibly both things failed. Despite -EBUSY being a logical
> choice here, I think you'd better use -EAGAIN here too. And it
> needs
> to be determined whether continuing the loop in this as well as the
> pre-existing cases is actually the right thing to do.
 EBUSY vs. EAGAIN: by returning EAGAIN I would signal to Xen tools
 that
 the hypervisor is currently not able to do the desired operation
 (especially removing a cpu from a cpupool), but the situation will
 change automatically via scheduling. EBUSY will stop retries in Xen
 tools and this is want I want here: I can't be sure the situation
 will change soon.

>>> I agree with this.
>> 
>> I'm of two minds here: I can see your viewpoint, but considering
>> this is called "temporarily pin a vcpu" the condition is supposed to
>> be going away again soon.
> 
> It is supposed to do so, yes. But the hypervisor can't make sure it
> will, as it requires an appropriate hypercall by the hardware domain.
> In the cpupool case no domain is capable to make the situation
> persist.
> 
> Would you be fine with adding a tools patch doing a limited number
> of retries in the EBUSY case (maybe with sleeping 1 second in that
> case)?

That would make me worry less, yes.

Jan


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

Re: [Xen-devel] [PATCH] xen: add hypercall option to temporarily pin a vcpu

[Dario Faggioli]

On Fri, 2016-02-26 at 06:32 -0700, Jan Beulich wrote:
> > > > On 26.02.16 at 14:07,  wrote:
> > We can argue about that being useful or not, and about it being
> > (potentially) too noisy or not. I personally think it could be
> > useful
> > (it's XENLOG_DEBUG, after all), but I won't oppose getting rid of
> > it...
> > I am just not getting why you're saying "not currently running on
> > pCPU0".
> Oh, you're right, that was too strict - "not being allowed to run
> on CPU0" would be the right description. And indeed that makes
> it look not as noisy (but too much, since during suspend this is
> what one has to expect would happen).
>
Yes, mostly on the ground that it's the intended behavior and (in that
case really) temporary, I'm indeed ok silencing this on suspend.

Since it's pretty independent, I'd prefer this to be done in a separate
patch, together with the resume side, as (AFAIUI) Juergen is planning
to do already.

Dario
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D 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 v5 04/22] arm/acpi: Add basic ACPI initialization

[Stefano Stabellini]

On Fri, 26 Feb 2016, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> acpi_boot_table_init() will be called in start_xen to get the RSDP and
> all the table pointers. With this patch, we can get ACPI boot-time
> tables from firmware on ARM64.
> 
> Signed-off-by: Naresh Bhat 
> Signed-off-by: Parth Dixit 
> Signed-off-by: Shannon Zhao 

Acked-by: Stefano Stabellini 


>  xen/arch/arm/acpi/Makefile |  1 +
>  xen/arch/arm/acpi/boot.c   | 58 
> ++
>  xen/arch/arm/setup.c   |  4 
>  3 files changed, 63 insertions(+)
>  create mode 100644 xen/arch/arm/acpi/boot.c
> 
> diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
> index b5be22d..196c40a 100644
> --- a/xen/arch/arm/acpi/Makefile
> +++ b/xen/arch/arm/acpi/Makefile
> @@ -1 +1,2 @@
>  obj-y += lib.o
> +obj-y += boot.o
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> new file mode 100644
> index 000..1570f7e
> --- /dev/null
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -0,0 +1,58 @@
> +/*
> + *  ARM Specific Low-Level ACPI Boot Support
> + *
> + *  Copyright (C) 2001, 2002 Paul Diefenbaugh 
> + *  Copyright (C) 2001 Jun Nakajima 
> + *  Copyright (C) 2014, Naresh Bhat 
> + *  Copyright (C) 2015, Shannon Zhao 
> + *
> + * ~~
> + *
> + *  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.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + * ~~
> + */
> +
> +#include 
> +#include 
> +
> +#include 
> +
> +/*
> + * acpi_boot_table_init() called from setup_arch(), always.
> + *  1. find RSDP and get its address, and then find XSDT
> + *  2. extract all tables and checksums them all
> + *
> + * return value: (currently ignored)
> + *   0: success
> + *   !0: failure
> + *
> + * We can parse ACPI boot-time tables such as FADT, MADT after
> + * this function is called.
> + */
> +int __init acpi_boot_table_init(void)
> +{
> +int error;
> +
> +/* Initialize the ACPI boot-time table parser. */
> +error = acpi_table_init();
> +if ( error )
> +{
> +disable_acpi();
> +return error;
> +}
> +
> +return 0;
> +}
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index e6b689e..fee5385 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -36,6 +36,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -755,6 +756,9 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>  setup_mm(fdt_paddr, fdt_size);
>  
> +/* Parse the ACPI tables for possible boot-time configuration */
> +acpi_boot_table_init();
> +
>  vm_init();
>  dt_unflatten_host_device_tree();
>  
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Tamas K Lengyel]

On Fri, Feb 26, 2016 at 6:07 AM, Corneliu ZUZU 
wrote:

> This patch adds ARM support for guest-request monitor vm-events.
> Note: on ARM hypercall instruction skipping must be done manually
> by the caller. This will probably be changed in a future patch.
>
> Summary of changes:
> == Moved to common-side:
>   * XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST handling (moved from X86
>   arch_monitor_domctl_event to common monitor_domctl)
>   * hvm_event_guest_request->vm_event_monitor_guest_request
>   * hvm_event_traps->vm_event_monitor_traps (also added target vcpu as
> param)
>   * guest-request bits from X86 'struct arch_domain' (to common 'struct
> domain')
> == ARM implementations:
>   * do_hvm_op now handling of HVMOP_guest_request_vm_event => calls
>   vm_event_monitor_guest_request (as on X86)
>   * arch_monitor_get_capabilities->vm_event_monitor_get_capabilities,
> updated to reflect support for XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST
>   * vm_event_init_domain (does nothing), vm_event_cleanup_domain
> == Misc:
>   * vm_event_fill_regs, no longer X86-specific. ARM-side implementation of
> this
>   function currently does nothing, that will be added in a separate
> patch.
>
> Signed-off-by: Corneliu ZUZU 
>

Looks good to me, thanks!

Acked-by: Tamas K Lengyel 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] MAINTAINERS: Remove myself from ARM (incl DT), seabios, tools and The Rest

[Wei Liu]

On Wed, Feb 24, 2016 at 11:37:00AM +, Ian Campbell wrote:
[...]
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cd4da04..b320152 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -127,7 +127,6 @@ F:xen/common/sched_arinc653.c
[...]
>  
>  SEABIOS UPSTREAM
> -M:   Ian Campbell 
>  M:   Wei Liu 
>  S:   Supported
>  T:   git git://xenbits.xen.org/seabios.git

Acked-by: Wei Liu 

> @@ -314,7 +311,6 @@ F:stubdom/
>  TOOLSTACK
>  M:   Ian Jackson 
>  M:   Stefano Stabellini 
> -M:   Ian Campbell 
>  M:   Wei Liu 
>  S:   Supported
>  F:   autogen.sh

Acked-by: Wei Liu 

Now you're officially off the hook. :-D

Ian, thanks for all your contribution to the project and I wish you all
the best with your new venture!

Wei.

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

Re: [Xen-devel] [PATCH, RESEND] xen: allocate gntdev_copy_batch dynamically

[Boris Ostrovsky]

On 02/25/2016 04:25 PM, Arnd Bergmann wrote:

struct gntdev_copy_batch is arguably too large to fit on the kernel stack,
and we get a warning about the stack usage in gntdev_ioctl_grant_copy:

drivers/xen/gntdev.c:949:1: error: the frame size of 1240 bytes is larger than 
1024 bytes

This changes the code to us a dynamic allocation instead.

Signed-off-by: Arnd Bergmann 
Fixes: a4cdb556cae0 ("xen/gntdev: add ioctl for grant copy")
---
  drivers/xen/gntdev.c | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)

I sent this in January, Boris sent an almost identical patch
as http://www.gossamer-threads.com/lists/xen/devel/414056
but the bug remains present in mainline and linux-next as of
Feb 25.

Could you apply one of the patches before the bug makes it
into v4.5?


David wanted to shrink the structure size instead:
http://www.gossamer-threads.com/lists/xen/devel/414535

-boris


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

Re: [Xen-devel] [PATCH v5 18/22] arm/acpi: Add a new ACPI initialized function for UART

[Stefano Stabellini]

On Fri, 26 Feb 2016, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> This adds a new function to initialize UART for ACPI on ARM.
> 
> Signed-off-by: Shannon Zhao 

Reviewed-by: Stefano Stabellini 


> V5: fix coding style
> ---
>  xen/arch/arm/setup.c|  2 +-
>  xen/drivers/char/arm-uart.c | 37 +++--
>  xen/include/xen/serial.h|  2 +-
>  3 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index d4261e8..6d205a9 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -768,7 +768,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>  gic_preinit();
>  
> -dt_uart_init();
> +arm_uart_init();
>  console_init_preirq();
>  console_init_ring();
>  
> diff --git a/xen/drivers/char/arm-uart.c b/xen/drivers/char/arm-uart.c
> index 883e615..627746b 100644
> --- a/xen/drivers/char/arm-uart.c
> +++ b/xen/drivers/char/arm-uart.c
> @@ -1,7 +1,7 @@
>  /*
>   * xen/drivers/char/arm-uart.c
>   *
> - * Generic uart retrieved via the device tree
> + * Generic uart retrieved via the device tree or ACPI
>   *
>   * Julien Grall 
>   * Copyright (c) 2013 Linaro Limited.
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   * Configure UART port with a string:
> @@ -35,7 +36,7 @@
>  static char __initdata opt_dtuart[256] = "";
>  string_param("dtuart", opt_dtuart);
>  
> -void __init dt_uart_init(void)
> +static void __init dt_uart_init(void)
>  {
>  struct dt_device_node *dev;
>  int ret;
> @@ -96,6 +97,38 @@ void __init dt_uart_init(void)
>  printk("Unable to initialize dtuart: %d\n", ret);
>  }
>  
> +#ifdef CONFIG_ACPI
> +static void __init acpi_uart_init(void)
> +{
> +struct acpi_table_spcr *spcr = NULL;
> +int ret;
> +
> +acpi_get_table(ACPI_SIG_SPCR, 0, (struct acpi_table_header **)&spcr);
> +
> +if ( spcr == NULL )
> +{
> +printk("Unable to get spcr table\n");
> +}
> +else
> +{
> +ret = acpi_device_init(DEVICE_SERIAL, NULL, spcr->interface_type);
> +
> +if ( ret )
> +printk("Unable to initialize acpi uart: %d\n", ret);
> +}
> +}
> +#else
> +static void __init acpi_uart_init(void) { }
> +#endif
> +
> +void __init arm_uart_init(void)
> +{
> +if ( acpi_disabled )
> +dt_uart_init();
> +else
> +acpi_uart_init();
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 71e6ade..1212a12 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -170,7 +170,7 @@ struct ns16550_defaults {
>  void ns16550_init(int index, struct ns16550_defaults *defaults);
>  void ehci_dbgp_init(void);
>  
> -void __init dt_uart_init(void);
> +void arm_uart_init(void);
>  
>  struct physdev_dbgp_op;
>  int dbgp_op(const struct physdev_dbgp_op *);
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] [PATCH v4] public/io/netif.h: make control ring hash protocol more general

[Paul Durrant]

Ping + 1?

> -Original Message-
> From: Paul Durrant
> Sent: 22 February 2016 13:06
> To: Paul Durrant; xen-de...@lists.xenproject.org
> Cc: Ian Campbell; Ian Jackson; Jan Beulich; Keir (Xen.org); Tim (Xen.org)
> Subject: RE: [PATCH v4] public/io/netif.h: make control ring hash protocol
> more general
> 
> Ping?
> 
> > -Original Message-
> > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > Sent: 17 February 2016 15:05
> > To: xen-de...@lists.xenproject.org
> > Cc: Paul Durrant; Ian Campbell; Ian Jackson; Jan Beulich; Keir (Xen.org); 
> > Tim
> > (Xen.org)
> > Subject: [PATCH v4] public/io/netif.h: make control ring hash protocol more
> > general
> >
> > This patch modified the control ring protocol (of which there is
> > not yet an implementation) to make it more general. Most of the
> > concepts are not limited to toeplitz hashing so it's best not to
> > make them unnecessarily specific.
> >
> > Apart from changing the names of various definitions and modifying
> > comments, this patch:
> >
> > - Adds a new control message type to select a hash algorithm.
> > - Adds a reference implementation of the toeplitz hash.
> > - Changes the 'toeplitz' extra info fragment into a 'hash' extra
> >   info fragment and replaces the octet of padding with the index of
> >   the algorithm that was used to create the hash value.
> > - Relaxes the restriction that the mapping table has to be
> >   power-of-2 sized.
> >
> > The patch also fixes a few spelling typos noticed along the way.
> >
> > Signed-off-by: Paul Durrant 
> > Cc: Ian Campbell 
> > Cc: Ian Jackson 
> > Cc: Jan Beulich 
> > Cc: Keir Fraser 
> > Cc: Tim Deegan 
> > ---
> >
> > v2:
> >   - Remove 'inline' from reference hash definition and make the
> > definition optional
> >
> > v3:
> >   - Use XEN_NETIF_ prefix instead of NETIF_
> >   - Make hash mapping table optional
> >   - Allow a hash mapping table that is not power-of-two sized
> >
> > v4:
> >   - More NETIF -> XEN_NETIF substitution
> >   - Also defined ctrl message structs with xen_ prefix and get rid
> > of typedefs
> > ---
> >  xen/include/public/io/netif.h | 350 +++
> ---
> > 
> >  1 file changed, 226 insertions(+), 124 deletions(-)
> >
> > diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
> > index 8816e0f..ca00614 100644
> > --- a/xen/include/public/io/netif.h
> > +++ b/xen/include/public/io/netif.h
> > @@ -164,7 +164,7 @@
> >   * Control ring
> >   * 
> >   *
> > - * Some features, such as toeplitz hashing (detailed below), require a
> > + * Some features, such as hashing (detailed below), require a
> >   * significant amount of out-of-band data to be passed from frontend to
> >   * backend. Use of xenstore is not suitable for large quantities of data
> >   * because of quota limitations and so a dedicated 'control ring' is used.
> > @@ -191,8 +191,8 @@
> >   */
> >
> >  /*
> > - * Toeplitz hash types
> > - * ===
> > + * Hash types
> > + * ==
> >   *
> >   * For the purposes of the definitions below, 'Packet[]' is an array of
> >   * octets containing an IP packet without options, 'Array[X..Y]' means a
> > @@ -206,10 +206,11 @@
> >   * Buffer[0..8] = Packet[12..15] (source address) +
> >   *Packet[16..19] (destination address)
> >   *
> > - * Result = ToeplitzHash(Buffer, 8)
> > + * Result = Hash(Buffer, 8)
> >   */
> > -#define _NETIF_CTRL_TOEPLITZ_HASH_IPV4 0
> > -#define NETIF_CTRL_TOEPLITZ_HASH_IPV4  (1 <<
> > _NETIF_CTRL_TOEPLITZ_HASH_IPV4)
> > +#define _XEN_NETIF_CTRL_HASH_TYPE_IPV4 0
> > +#define XEN_NETIF_CTRL_HASH_TYPE_IPV4 \
> > +(1 << _XEN_NETIF_CTRL_HASH_TYPE_IPV4)
> >
> >  /*
> >   * A hash calculated over an IP version 4 header and TCP header as
> > @@ -220,10 +221,11 @@
> >   * Packet[20..21] (source port) +
> >   * Packet[22..23] (destination port)
> >   *
> > - * Result = ToeplitzHash(Buffer, 12)
> > + * Result = Hash(Buffer, 12)
> >   */
> > -#define _NETIF_CTRL_TOEPLITZ_HASH_IPV4_TCP 1
> > -#define NETIF_CTRL_TOEPLITZ_HASH_IPV4_TCP  (1 <<
> > _NETIF_CTRL_TOEPLITZ_HASH_IPV4_TCP)
> > +#define _XEN_NETIF_CTRL_HASH_TYPE_IPV4_TCP 1
> > +#define XEN_NETIF_CTRL_HASH_TYPE_IPV4_TCP \
> > +(1 << _XEN_NETIF_CTRL_HASH_TYPE_IPV4_TCP)
> >
> >  /*
> >   * A hash calculated over an IP version 6 header as follows:
> > @@ -231,10 +233,11 @@
> >   * Buffer[0..32] = Packet[8..23]  (source address ) +
> >   * Packet[24..39] (destination address)
> >   *
> > - * Result = ToeplitzHash(Buffer, 32)
> > + * Result = Hash(Buffer, 32)
> >   */
> > -#define _NETIF_CTRL_TOEPLITZ_HASH_IPV6 2
> > -#define NETIF_CTRL_TOEPLITZ_HASH_IPV6  (1 <<
> > _NETIF_CTRL_TOEPLITZ_HASH_IPV4)
> > +#define _XEN_NETIF_CTRL_HASH_TYPE_IPV6 2
> > +#define XEN_NETIF_CTRL_HASH_TYPE_IPV6 \
> > +(1 << _XEN_NETIF_CTRL_HASH_TYPE_IPV6)
> >
> >  /*
> >   * A hash calculated over an IP version 6 header and TCP header as

Re: [Xen-devel] [PATCH v3 0/2] Clear .bss for VP guests

[Boris Ostrovsky]

On 02/26/2016 05:53 AM, Roger Pau Monné wrote:

El 25/2/16 a les 16:16, Boris Ostrovsky ha escrit:

PV guests need to have their .bss zeroed out since it is not guaranteed
to be cleared by Xen's domain builder

I guess I'm missing something, but elf_load_image (in libelf-loader.c)
seems to be able to clear segments (it will zero the memory between
p_paddr + p_filesz and p_paddr + p_memsz) while loading the ELF into
memory, so if the program headers are correctly setup the .bss should be
zeroed out AFAICT.


Right, but I don't think this is guaranteed. It's uninitialized data so 
in principle it can be anything.


The ELF spec says "the system initializes the data with zero when the 
program begins to run" which I read as it's up to runtime and not the 
loader to do so.


And since kernel does it explicitly on baremetal path I think it's a 
good idea for PV to do the same.


-boris

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

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Razvan Cojocaru]

On 02/26/2016 01:07 PM, Corneliu ZUZU wrote:
> This patch adds ARM support for guest-request monitor vm-events.
> Note: on ARM hypercall instruction skipping must be done manually
> by the caller. This will probably be changed in a future patch.
> 
> Summary of changes:
> == Moved to common-side:
>   * XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST handling (moved from X86
>   arch_monitor_domctl_event to common monitor_domctl)
>   * hvm_event_guest_request->vm_event_monitor_guest_request
>   * hvm_event_traps->vm_event_monitor_traps (also added target vcpu as param)
>   * guest-request bits from X86 'struct arch_domain' (to common 'struct 
> domain')
> == ARM implementations:
>   * do_hvm_op now handling of HVMOP_guest_request_vm_event => calls
>   vm_event_monitor_guest_request (as on X86)
>   * arch_monitor_get_capabilities->vm_event_monitor_get_capabilities,
> updated to reflect support for XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST
>   * vm_event_init_domain (does nothing), vm_event_cleanup_domain
> == Misc:
>   * vm_event_fill_regs, no longer X86-specific. ARM-side implementation of 
> this
>   function currently does nothing, that will be added in a separate patch.
> 
> Signed-off-by: Corneliu ZUZU 
> ---
> Changed since v1:
>   * hvm_event_traps, hvm_event_guest_request moved to common/vm_event.c and
>   and renamed to vm_event_monitor_traps and vm_event_monitor_guest_request
>   * arch_monitor_get_capabilities moved to vm_event.h and renamed to
>   vm_event_monitor_get_capabilities
>   * arch_hvm_event_fill_regs replaced w/ existing vm_event_fill_regs
>   (see commit adc75eba8b15c7103a010f736fe62e3fb2383964)
>   * defined altp2m_active for ARM and altp2m_vcpu_idx to remove #ifdef in
>   vm_event_monitor_traps
>   * change bitfield members type to unsigned int
> ---
>  xen/arch/arm/hvm.c  |  8 
>  xen/arch/x86/hvm/event.c| 82 
> -
>  xen/arch/x86/hvm/hvm.c  |  3 +-
>  xen/arch/x86/monitor.c  | 18 +
>  xen/arch/x86/vm_event.c |  1 +
>  xen/common/monitor.c| 36 +++---
>  xen/common/vm_event.c   | 60 ++
>  xen/include/asm-arm/altp2m.h| 39 
>  xen/include/asm-arm/monitor.h   |  8 +---
>  xen/include/asm-arm/vm_event.h  | 19 +-
>  xen/include/asm-x86/altp2m.h| 10 +++--
>  xen/include/asm-x86/domain.h|  4 +-
>  xen/include/asm-x86/hvm/event.h | 13 +--
>  xen/include/asm-x86/monitor.h   | 23 
>  xen/include/asm-x86/vm_event.h  | 23 
>  xen/include/xen/sched.h |  6 +++
>  xen/include/xen/vm_event.h  |  9 +
>  17 files changed, 232 insertions(+), 130 deletions(-)
>  create mode 100644 xen/include/asm-arm/altp2m.h

Acked-by: Razvan Cojocaru 


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

Re: [Xen-devel] [PATCH v5 09/11] xen: add capability to load initrd outside of initial mapping

[Daniel Kiper]

On Thu, Feb 25, 2016 at 04:33:46PM +0100, Juergen Gross wrote:
> On 25/02/16 13:47, Daniel Kiper wrote:
> > On Thu, Feb 25, 2016 at 12:33:35PM +0100, Juergen Gross wrote:
> >> Modern pvops linux kernels support an initrd not covered by the initial
> >> mapping. This capability is flagged by an elf-note.
> >>
> >> In case the elf-note is set by the kernel don't place the initrd into
> >> the initial mapping. This will allow to load larger initrds and/or
> >> support domains with larger memory, as the initial mapping is limited
> >> to 2GB and it is containing the p2m list.
> >>
> >> Signed-off-by: Juergen Gross 
> >> ---
> >> V5: let call grub_xen_alloc_final() all subfunctions unconditionally
> >> and let them decide whether they need to do anything
> >> V4: rename grub_xen_alloc_end() to grub_xen_alloc_final()
> >> ---
> >>  grub-core/loader/i386/xen.c| 65 
> >> ++
> >>  grub-core/loader/i386/xen_fileXX.c |  3 ++
> >>  include/grub/xen_file.h|  1 +
> >>  3 files changed, 56 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
> >> index 2e12763..466f0c0 100644
> >> --- a/grub-core/loader/i386/xen.c
> >> +++ b/grub-core/loader/i386/xen.c
> >> @@ -228,6 +228,9 @@ grub_xen_p2m_alloc (void)
> >>grub_size_t p2msize;
> >>grub_err_t err;
> >>
> >> +  if (xen_state.virt_mfn_list)
> >> +return GRUB_ERR_NONE;
> >> +
> >>xen_state.state.mfn_list = xen_state.max_addr;
> >>xen_state.next_start.mfn_list =
> >>  xen_state.max_addr + xen_state.xen_inf.virt_base;
> >> @@ -250,6 +253,9 @@ grub_xen_special_alloc (void)
> >>grub_relocator_chunk_t ch;
> >>grub_err_t err;
> >>
> >> +  if (xen_state.virt_start_info)
> >> +return GRUB_ERR_NONE;
> >> +
> >>err = grub_relocator_alloc_chunk_addr (xen_state.relocator, &ch,
> >> xen_state.max_addr,
> >> sizeof (xen_state.next_start));
> >> @@ -281,6 +287,9 @@ grub_xen_pt_alloc (void)
> >>grub_uint64_t nr_info_pages;
> >>grub_uint64_t nr_pages, nr_pt_pages, nr_need_pages;
> >>
> >> +  if (xen_state.virt_pgtable)
> >> +return GRUB_ERR_NONE;
> >> +
> >>xen_state.next_start.pt_base =
> >>  xen_state.max_addr + xen_state.xen_inf.virt_base;
> >>xen_state.state.paging_start = xen_state.max_addr >> PAGE_SHIFT;
> >> @@ -320,6 +329,24 @@ grub_xen_pt_alloc (void)
> >>  }
> >>
> >>  static grub_err_t
> >> +grub_xen_alloc_final (void)
> >> +{
> >> +  grub_err_t err;
> >> +
> >> +  err = grub_xen_p2m_alloc ();
> >> +  if (err)
> >> +return err;
> >> +  err = grub_xen_special_alloc ();
> >> +  if (err)
> >> +return err;
> >> +  err = grub_xen_pt_alloc ();
> >> +  if (err)
> >> +return err;
> >
> > Could you move grub_xen_p2m_alloc() here? This way grub_xen_alloc_final()
> > will be real final in patch 11 and you do not need an extra condition
> > around grub_xen_p2m_alloc().
>
> No. Page tables must be allocated after p2m unless p2m is outside of
> initial kernel mapping (patch 11).

OK, I was afraid of that. However, then grub_xen_alloc_final() is not final.
So, maybe it should be called grub_xen_alloc_boot_data() or something like that.

> >> +  return GRUB_ERR_NONE;
> >> +}
> >> +
> >> +static grub_err_t
> >>  grub_xen_boot (void)
> >>  {
> >>grub_err_t err;
> >> @@ -330,13 +357,7 @@ grub_xen_boot (void)
> >>if (grub_xen_n_allocated_shared_pages)
> >>  return grub_error (GRUB_ERR_BUG, "active grants");
> >>
> >> -  err = grub_xen_p2m_alloc ();
> >> -  if (err)
> >> -return err;
> >> -  err = grub_xen_special_alloc ();
> >> -  if (err)
> >> -return err;
> >> -  err = grub_xen_pt_alloc ();
> >> +  err = grub_xen_alloc_final ();
> >
> > if (!xen_state.xen_inf.unmapped_initrd)
> >   {
> > err = grub_xen_alloc_final ();
> > if (err)
> >   goto fail;
> >   }
> >
> > This way you avoid checks in grub_xen_p2m_alloc(),
> > grub_xen_special_alloc() and grub_xen_pt_alloc().
>
> No. In case there is no initrd, but the kernel does support an unmapped
> initrd, I would omit allocating special pages, p2m and page tables.

Ups... I have missed that.

> >>if (err)
> >>  return err;
> >>
> >> @@ -610,6 +631,13 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>goto fail;
> >>  }
> >>
> >> +  if (xen_state.xen_inf.unmapped_initrd)
> >> +{
> >> +  err = grub_xen_alloc_final ();
> >> +  if (err)
> >> +  goto fail;
> >> +}
> >> +
> >>if (grub_initrd_init (argc, argv, &initrd_ctx))
> >>  goto fail;
> >>
> >> @@ -627,14 +655,24 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ 
> >> ((unused)),
> >>goto fail;
> >>  }
> >>
> >> -  xen_state.next_start.mod_start =
> >> -xen_state.max_addr + xen_state.xen_inf.virt_base;
> >> -  xen_state.next_start.mod_len = size;
> >
> > Leave "xen_state.next_start.mod_len = size;" as is here and...
> >
> >> -
> 

[Xen-devel] [xen-unstable-smoke test] 84118: trouble: broken/pass

[osstest service owner]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-debianhvm-i386 3 host-install(3) broken REGR. vs. 
83972

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  2753e07f5e8440cd5751369f5a037a7824e5a29e
baseline version:
 xen  abf8824fe530bcf060c757596f68663c87546a6a

Last test of basis83972  2016-02-25 13:01:35 Z1 days
Testing same since84118  2016-02-26 12:03:38 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Bob Moore 
  David Vrabel 
  Doug Goldstein 
  Hanjun Guo 
  Jan Beulich 
  Kevin Tian 
  Liang Z Li 
  Parth Dixit 
  Shannon Zhao 
  Tim Deegan 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-amd64-libvirt  pass
 test-armhf-armhf-xl  pass
 test-amd64-amd64-xl-qemuu-debianhvm-i386 broken  
 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

broken-step test-amd64-amd64-xl-qemuu-debianhvm-i386 host-install(3)

Not pushing.


commit 2753e07f5e8440cd5751369f5a037a7824e5a29e
Author: Shannon Zhao 
Date:   Fri Feb 26 12:37:50 2016 +0100

arm/acpi: Initialize serial port from ACPI SPCR table

Parse ACPI SPCR (Serial Port Console Redirection table) table and
initialize the serial port pl011.

Signed-off-by: Parth Dixit 
Signed-off-by: Shannon Zhao 
Reviewed-by: Stefano Stabellini 

Fix build.

Acked-by: Jan Beulich 

commit d5e8732bcad85b4166fba6e54724740a9e973691
Author: Bob Moore 
Date:   Fri Feb 26 12:37:18 2016 +0100

ACPICA / Headers: Add support for CSRT and DBG2 ACPI tables

These tables are defined outside of the ACPI specification.

Signed-off-by: Bob Moore 
[Linux commit 4e2f9c278ad84196991fcf6f6646a3e15967fe90]
[only port the DBG2 changes]
Signed-off-by: Shannon Zhao 
Acked-by: Jan Beulich 

commit 89b5aa9bf74ebd4fcec06be05024e127361903a6
Author: Hanjun Guo 
Date:   Fri Feb 26 12:36:46 2016 +0100

ACPI / table: Print GIC information when MADT is parsed

When MADT is parsed, print GIC information as debug message:

ACPI: GICC (acpi_id[0x] address[e112f000] MPIDR[0x0] enabled)
ACPI: GICC (acpi_id[0x0001] address[e112f000] MPIDR[0x1] enabled)
...
ACPI: GICC (acpi_id[0x0201] address[e112f000] MPIDR[0x201] enabled)

This debug information will be very helpful to bring up early systems to
see if acpi_id and MPIDR are matched or not as spec defined.

Signed-off-by: Hanjun Guo 
[Linux commit 4c1c8d7a7ebc8b909493a14b21b233e5377b69aa]
[Use container_of instead of cast and PRIx64 instead of %llx]
Signed-off-by: Shannon Zhao 
Acked-by: Jan Beulich 

commit 763076cf5d29b41c98171ac2489e1726f95667ae
Author: Doug Goldstein 
Date:   Fri Feb 26 12:35:46 2016 +0100

build: convert HAS_CORE_PARKING to Kconfig

Convert HAS_CORE_PARKING to Kconfig as CONFIG_CORE_PARKING. While
removing HAS_CORE_PARKING, removed a trailing whitespace on a near by
line.

Signed-off-by: Doug Goldstein 
Reviewed-by: Jan Beulich 

commit 1afea8d3a9b947912e6be587b462a159fc7cd5d2
Author: Doug Goldstein 
Date:   Fri Feb 26 12:33:14 2016 +0100

build: convert HAS_NUMA to Kconfig

Convert HAS_NUMA to Kconfig as CONFIG_NUMA and let CONFIG_NUMA be
defined by Kconfig.

Signed-off-by: Doug Goldstein 
Reviewed-by: Jan Beulich 

commit fb1e853c53580c4d9be3519e552141805b75b19c
Author: Doug Goldstein 
Date:   Fri Feb 26 12:31:47 2016 +0100

build: consolidate CONFIG_HAS_ACPI and CONFIG_ACPI

No real advantage to keeping these separat

Re: [Xen-devel] [PATCH v2 4/4] arm64: update the introduction of xen boot commands in docs/grub.texi

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 26.02.2016 12:15, Fu Wei wrote:
> Hi Andrei,
> 
> On 26 February 2016 at 18:50, Andrei Borzenkov  wrote:
>> On Fri, Feb 26, 2016 at 8:59 AM, Fu Wei  wrote:
>>> +@subsection xen_module
>>>
>>> -@deffn Command xen_linux file [arguments]
>>> -Load a dom0 kernel image for xen hypervisor at the booting process of 
>>> xen.
>>> +@deffn Command xen_module [--nounzip] file [arguments]
>>> +Load a module for xen hypervisor at the booting process of xen.
>>> The rest of the line is passed verbatim as the module command line.
>>> +Each module will be identified by the order in which the modules are 
>>> added.
>>> +The 1st module: dom0 kernel image
>>> +The 2nd module: dom0 ramdisk
>>> +All subsequent modules: UNKNOW
>>> @end deffn
>>
>> Hmm ... from previous discussion I gathered that Xen can detect module
>> type. What if there is no initrd for dom0? How can subsequent modules be
>
> Now , Xen detect module type by the order. (at least on ARM64).
> I think i386 is using Multiboot(2) protocol, so maybe this order is
> nothing to do with i386.
>

 Then we have obvious problem with your XSM patch 
 (http://savannah.gnu.org/bugs/?43420) - XSM may land as the first module. 
 That's actually something to solve on Xen side I think. It's just that so 
 far we had just kernel and initrd, so that was non issue.
>>>
>>> Oh, did you mean Wei Liu's patch?
>>>
>>> I guess XSM may land as the third module (or the module after linux
>>> kernel, if you don't have initrd)
>>>
>>> Yes, agree. (That's actually something to solve on Xen side)
>>>
>>> I guess xen can get xsm from a special initrd. so for now there is not
>>> big problem on xsm.
>>>
>>> Please correct me if I misunderstand something. :-)
>>>
>>> Thanks!
>>>
>>> Back to this patch, is that OK for you, or any suggestion?  Thanks !
>>>
>>
>> Yes, as this is dedicated Xen loader we should document this mandatory
>> order - first module must be kernel image, second module must be
>> initrd. I do not think we need to mention possibility to load more
>> than two modules until there is clear understanding how it can be done
>> without initrd.
> 
> Great thanks for your review, I have updated and sent the v3 patchset,
> Hope I understood your suggestion correctly, Please check.  :-)
> 
Your patches look fine. Let's wait for Andrei's opinion but I'm leaning
to commit them
> 
> 




signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v5 03/22] arm/acpi: Add __acpi_map_table function for ARM

[Stefano Stabellini]

On Fri, 26 Feb 2016, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Implement __acpi_map_table function for ARM.
> 
> Signed-off-by: Shannon Zhao 
> ---
> V4: add __acpi_map_table function
> ---
>  xen/arch/arm/Makefile|  1 +
>  xen/arch/arm/acpi/Makefile   |  1 +
>  xen/arch/arm/acpi/lib.c  | 52 
> 
>  xen/include/asm-arm/config.h |  2 ++
>  4 files changed, 56 insertions(+)
>  create mode 100644 xen/arch/arm/acpi/Makefile
>  create mode 100644 xen/arch/arm/acpi/lib.c
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 1783912..6d51094 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -2,6 +2,7 @@ subdir-$(CONFIG_ARM_32) += arm32
>  subdir-$(CONFIG_ARM_64) += arm64
>  subdir-y += platforms
>  subdir-$(CONFIG_ARM_64) += efi
> +subdir-$(CONFIG_HAS_ACPI) += acpi
>  
>  obj-$(EARLY_PRINTK) += early_printk.o
>  obj-y += cpu.o
> diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
> new file mode 100644
> index 000..b5be22d
> --- /dev/null
> +++ b/xen/arch/arm/acpi/Makefile
> @@ -0,0 +1 @@
> +obj-y += lib.o
> diff --git a/xen/arch/arm/acpi/lib.c b/xen/arch/arm/acpi/lib.c
> new file mode 100644
> index 000..f817fe6
> --- /dev/null
> +++ b/xen/arch/arm/acpi/lib.c
> @@ -0,0 +1,52 @@
> +/*
> + *  lib.c - Architecture-Specific Low-Level ACPI Support
> + *
> + *  Copyright (C) 2015, Shannon Zhao 
> + *
> + * ~~
> + *
> + *  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.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + * ~~
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +char *__acpi_map_table(paddr_t phys, unsigned long size)
> +{
> + unsigned long base, offset, mapped_size;
> + int idx;
> +
> + offset = phys & (PAGE_SIZE - 1);
> + mapped_size = PAGE_SIZE - offset;
> + set_fixmap(FIX_ACPI_BEGIN, phys >> PAGE_SHIFT, PAGE_HYPERVISOR);
> + base = FIXMAP_ADDR(FIX_ACPI_BEGIN);
> +
> + /*
> +  * Most cases can be covered by the below.
> +  */
> + idx = FIX_ACPI_BEGIN;
> + while (mapped_size < size) {
> + if (++idx > FIX_ACPI_END)
> + return NULL;/* cannot handle this */
> + phys += PAGE_SIZE;
> + set_fixmap(idx, phys >> PAGE_SHIFT, PAGE_HYPERVISOR);
> + mapped_size += PAGE_SIZE;
> + }
> +
> + return ((char *) base + offset);
> +}
> diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
> index a1b968d..41dd860 100644
> --- a/xen/include/asm-arm/config.h
> +++ b/xen/include/asm-arm/config.h
> @@ -180,6 +180,8 @@
>  #define FIXMAP_GICC14  /* Interrupt controller: CPU registers (first 
> page) */
>  #define FIXMAP_GICC25  /* Interrupt controller: CPU registers (second 
> page) */
>  #define FIXMAP_GICH 6  /* Interrupt controller: virtual interface 
> control registers */
> +#define FIX_ACPI_BEGIN  7  /* Start mappings of ACPI tables */
> +#define FIX_ACPI_END10 /* End mappings of ACPI tables */
>  
>  #define PAGE_SHIFT  12

Please rename FIX_ACPI_BEGIN and FIX_ACPI_END to FIXMAP_ACPI_BEGIN and
FIXMAP_ACPI_END.

Also please move the declaration of FIX_ACPI_PAGES from asm-x86/acpi.h
to a common header, and maybe rename it to NUM_ACPI_PAGES. Then you can
define FIXMAP_ACPI_END as (FIXMAP_ACPI_BEGIN + NUM_ACPI_PAGES - 1).

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

[Xen-devel] [qemu-upstream-4.4-testing baseline-only test] 44171: trouble: broken/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 44171 qemu-upstream-4.4-testing real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/44171/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-libvirt   broken
 test-amd64-amd64-xl-qemuu-ovmf-amd64  broken
 test-amd64-amd64-pv   broken
 test-amd64-i386-pair  broken
 test-amd64-i386-qemuu-rhel6hvm-intel  broken
 test-amd64-i386-xl-qemuu-ovmf-amd64  broken
 test-amd64-i386-pvbroken
 test-amd64-amd64-xl-qemuu-winxpsp3  broken
 test-amd64-amd64-amd64-pvgrub  broken
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 broken
 test-amd64-i386-freebsd10-amd64  broken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64broken
 test-amd64-amd64-xl-credit2   broken
 test-amd64-amd64-pygrub   broken
 test-amd64-amd64-libvirt-pair  broken
 test-amd64-i386-xl-rawbroken
 test-amd64-amd64-qemuu-nested-amd  broken
 test-amd64-amd64-xl-qcow2 broken
 test-amd64-i386-libvirt   broken
 test-amd64-amd64-xl-multivcpu  broken
 test-amd64-amd64-pair broken
 test-amd64-i386-freebsd10-i386  broken
 build-i386-libvirtbroken
 test-amd64-amd64-xl   broken
 test-amd64-i386-libvirt-pair  broken
 test-amd64-amd64-xl-qemuu-win7-amd64  broken
 test-amd64-amd64-libvirt  broken
 test-amd64-i386-xl-qemuu-win7-amd64  broken
 test-amd64-amd64-libvirt-vhd  broken
 test-amd64-amd64-qemuu-nested-intel  broken
 test-amd64-amd64-i386-pvgrub  broken
 test-amd64-i386-qemuu-rhel6hvm-amd  broken
 test-amd64-i386-xl-qemuu-debianhvm-amd64 broken
 test-amd64-i386-xlbroken

version targeted for testing:
 qemuu16169ab825a03262cd66382dc0b02caa0dbd636a
baseline version:
 qemuu2264b0c66075cc34c252a1386f019f8be6240890

Last test of basis38625  2016-01-12 16:28:40 Z   44 days
Testing same since44171  2016-02-26 11:55:40 Z0 days1 attempts


People who touched revisions under test:
  Don Slutz 
  Gerd Hoffmann 
  Jason Wang 
  John Snow 
  Laszlo Ersek 
  Michael S. Tsirkin 
  P J P 
  Paolo Bonzini 
  Peter Maydell 
  Prasad J Pandit 
  Roger Pau Monne 
  Roger Pau Monné 
  Stefano Stabellini 

jobs:
 build-amd64-xend pass
 build-i386-xend  pass
 build-amd64  pass
 build-i386   pass
 build-amd64-libvirt  broken  
 build-i386-libvirt   broken  
 build-amd64-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  broken  
 test-amd64-i386-xl   broken  
 test-amd64-amd64-qemuu-nested-amdbroken  
 test-amd64-i386-qemuu-rhel6hvm-amd   broken  
 test-amd64-amd64-xl-qemuu-debianhvm-amd64broken  
 test-amd64-i386-xl-qemuu-debianhvm-amd64 broken  
 test-amd64-i386-freebsd10-amd64  broken  
 test-amd64-amd64-xl-qemuu-ovmf-amd64 broken  
 test-amd64-i386-xl-qemuu-ovmf-amd64  broken  
 test-amd64-amd64-xl-qemuu-win7-amd64 broken  
 test-amd64-i386-xl-qemuu-win7-amd64  broken  
 test-amd64-amd64-xl-credit2  broken  
 test-amd64-i386-freebsd10-i386   broken  
 test-amd64-amd64-qemuu-nested-intel  broken  
 test-amd64-i386-qemuu-rhel6hvm-intel broken  
 test-amd64-amd64-libvirt broken  
 test-amd64-i386-libvirt  broken  
 test-amd64-amd64-xl-multivcpubroken  
 test-amd64-amd64-pairbroken  
 test-amd64-i386-pair broken  
 test-amd64-amd64-libvirt-pairbroken  
 test-amd64-i386-libvirt-pair broken  
 test

Re: [Xen-devel] [PATCH v2] arm/monitor vm-events: Implement guest-request support

[Stefano Stabellini]

On Fri, 26 Feb 2016, Corneliu ZUZU wrote:
> This patch adds ARM support for guest-request monitor vm-events.
> Note: on ARM hypercall instruction skipping must be done manually
> by the caller. This will probably be changed in a future patch.
> 
> Summary of changes:
> == Moved to common-side:
>   * XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST handling (moved from X86
>   arch_monitor_domctl_event to common monitor_domctl)
>   * hvm_event_guest_request->vm_event_monitor_guest_request
>   * hvm_event_traps->vm_event_monitor_traps (also added target vcpu as param)
>   * guest-request bits from X86 'struct arch_domain' (to common 'struct 
> domain')
> == ARM implementations:
>   * do_hvm_op now handling of HVMOP_guest_request_vm_event => calls
>   vm_event_monitor_guest_request (as on X86)
>   * arch_monitor_get_capabilities->vm_event_monitor_get_capabilities,
> updated to reflect support for XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST
>   * vm_event_init_domain (does nothing), vm_event_cleanup_domain
> == Misc:
>   * vm_event_fill_regs, no longer X86-specific. ARM-side implementation of 
> this
>   function currently does nothing, that will be added in a separate patch.
> 
> Signed-off-by: Corneliu ZUZU 

ARM and common bits:

Acked-by: Stefano Stabellini 


> Changed since v1:
>   * hvm_event_traps, hvm_event_guest_request moved to common/vm_event.c and
>   and renamed to vm_event_monitor_traps and vm_event_monitor_guest_request
>   * arch_monitor_get_capabilities moved to vm_event.h and renamed to
>   vm_event_monitor_get_capabilities
>   * arch_hvm_event_fill_regs replaced w/ existing vm_event_fill_regs
>   (see commit adc75eba8b15c7103a010f736fe62e3fb2383964)
>   * defined altp2m_active for ARM and altp2m_vcpu_idx to remove #ifdef in
>   vm_event_monitor_traps
>   * change bitfield members type to unsigned int
> ---
>  xen/arch/arm/hvm.c  |  8 
>  xen/arch/x86/hvm/event.c| 82 
> -
>  xen/arch/x86/hvm/hvm.c  |  3 +-
>  xen/arch/x86/monitor.c  | 18 +
>  xen/arch/x86/vm_event.c |  1 +
>  xen/common/monitor.c| 36 +++---
>  xen/common/vm_event.c   | 60 ++
>  xen/include/asm-arm/altp2m.h| 39 
>  xen/include/asm-arm/monitor.h   |  8 +---
>  xen/include/asm-arm/vm_event.h  | 19 +-
>  xen/include/asm-x86/altp2m.h| 10 +++--
>  xen/include/asm-x86/domain.h|  4 +-
>  xen/include/asm-x86/hvm/event.h | 13 +--
>  xen/include/asm-x86/monitor.h   | 23 
>  xen/include/asm-x86/vm_event.h  | 23 
>  xen/include/xen/sched.h |  6 +++
>  xen/include/xen/vm_event.h  |  9 +
>  17 files changed, 232 insertions(+), 130 deletions(-)
>  create mode 100644 xen/include/asm-arm/altp2m.h
> 
> diff --git a/xen/arch/arm/hvm.c b/xen/arch/arm/hvm.c
> index 056db1a..c01123a 100644
> --- a/xen/arch/arm/hvm.c
> +++ b/xen/arch/arm/hvm.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -72,6 +73,13 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  break;
>  }
>  
> +case HVMOP_guest_request_vm_event:
> +if ( guest_handle_is_null(arg) )
> +vm_event_monitor_guest_request();
> +else
> +rc = -EINVAL;
> +break;
> +
>  default:
>  {
>  gdprintk(XENLOG_DEBUG, "HVMOP op=%lu: not implemented\n", op);
> diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c
> index d0b7d90..56c5514 100644
> --- a/xen/arch/x86/hvm/event.c
> +++ b/xen/arch/x86/hvm/event.c
> @@ -6,6 +6,7 @@
>   * Copyright (c) 2004, Intel Corporation.
>   * Copyright (c) 2005, International Business Machines Corporation.
>   * Copyright (c) 2008, Citrix Systems, Inc.
> + * Copyright (c) 2016, Bitdefender S.R.L.
>   *
>   * This program is free software; you can redistribute it and/or modify it
>   * under the terms and conditions of the GNU General Public License,
> @@ -21,71 +22,32 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  
> -static int hvm_event_traps(uint8_t sync, vm_event_request_t *req)
> -{
> -int rc;
> -struct vcpu *curr = current;
> -struct domain *currd = curr->domain;
> -
> -rc = vm_event_claim_slot(currd, &currd->vm_event->monitor);
> -switch ( rc )
> -{
> -case 0:
> -break;
> -case -ENOSYS:
> -/*
> - * If there was no ring to handle the event, then
> - * simply continue executing normally.
> - */
> -return 1;
> -default:
> -return rc;
> -};
> -
> -if ( sync )
> -{
> -req->flags |= VM_EVENT_FLAG_VCPU_PAUSED;
> -vm_event_vcpu_pause(curr);
> -}
> -
> -if ( altp2m_active(currd) )
> -{
> -req->flags |= VM_EVENT_FLAG_ALTERNATE_P2M;
> -req->altp2m_idx = vcpu_

Re: [Xen-devel] [PATCH v2 0/3] hvc_xen: fix xenboot on x86 and export to ARM

[Stefano Stabellini]

On Thu, 25 Feb 2016, Stefano Stabellini wrote:
> Hi all,
> 
> this series exports xenboot on ARM and ARM64 as earlycon, and gets
> xenboot fully working again for PV DomUs on x86 (currently the xenboot
> output only goes to the hypervisor via hypercall and not to the DomU
> console).
> 
> Cheers,
> 
> Stefano
> 
> 
> Stefano Stabellini (3):
>   hvc_xen: add earlycon support
>   hvc_xen: fix xenboot for DomUs
>   hvc_xen: make early_printk work with HVM guests

Boris has reviewed all the patches in the series. Are you all happy if I
go ahead and  commit the whole series to xentip/for-linus-4.6 ?

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