Re: [Xen-devel] [PATCH] xen vtd : set msi guest_masked 0 by default

[jzh Chang]

2016-01-26 20:56 GMT+08:00 Jan Beulich :

> >>> On 26.01.16 at 02:34,  wrote:
> > There are some problems when msi guest_masked is set to 1 by default.
> > When guest os is windows 2008 r2 server,
> > the device(eg X540-AT2 vf) is not initialized correctly.
> > Host will always receive message like this :"VF Reset msg received from
> vf".
> > Guest has network connectivity issues,
> > and can not correctly receive/send the packet.
> > So, guest_masked is set to 0 by default.
>
> You describe a problem and half of your change, but there's no
> connection between the two: What is actually wrong with current
> behavior (matching the hardware's - MSI-X mask bits are set when
> coming out of reset).
>
> > --- a/xen/arch/x86/msi.c
> > +++ b/xen/arch/x86/msi.c
> > @@ -512,7 +512,7 @@ void guest_mask_msi_irq(struct irq_desc *desc,
> bool_t mask)
> >
> >  static unsigned int startup_msi_irq(struct irq_desc *desc)
> >  {
> > -if ( unlikely(!msi_set_mask_bit(desc, 0, !!(desc->status &
> IRQ_GUEST))) )
> > +if ( unlikely(!msi_set_mask_bit(desc, 0, 0) ))
> >  WARN();
> >  return 0;
> >  }
>
> Whether this part can go under "set ... by default" is highly
> questionable. Plus, while this affects MSI and MSI-X, ...
>
>  If irq is owned by guest,in function msi_set_mask_bit():
...
bool_t flag = host || guest; //The flag should be true.
...
 writel(flag, entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
...
PCI device can not generate interrrupt.
windows guest can not change vector_ctrl_mask, guest os get abnormal status
of nic.

> > @@ -972,7 +972,7 @@ static int msix_capability_init(struct pci_dev *dev,
> >  entry->msi_attrib.entry_nr = msi->entry_nr;
> >  entry->msi_attrib.maskbit = 1;
> >  entry->msi_attrib.host_masked = 1;
> > -entry->msi_attrib.guest_masked = 1;
> > +entry->msi_attrib.guest_masked = 0;
> >  entry->msi_attrib.pos = pos;
> >  entry->irq = msi->irq;
> >  entry->dev = dev;
>
> ... this change affect MSI-X only, and doing some guessing from
> what you write above I suspect you only really tested one of the
> two cases.
>
> So while the change _may_ be necessary, you'll need to do a
> better job at explaining why you what you do.
>
Msi guest_masked is set to 0 in the original code, only msi-x guest_masked
is modifed in msix_capability_init() function by patch.

>
> Jan
>
>
This issue appears after commited the variable guest_mask.
Initialization operations of pci device may be changed in windows
guest,or Xen need to change the initial state of vtd pci device.
-- 
Jianzhong,Chang
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Fu Wei]

Hi Andrei,

On 28 February 2016 at 00:44, Fu Wei  wrote:
> Hi Andrei
>
> On 28 February 2016 at 01:26, Andrei Borzenkov  wrote:
>> 26.02.2016 14:13, fu@linaro.org пишет:
>>> 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.
>>
>> I think it is better to skip it entirely. It is not really correct -
>> neither multiboot protocol provides any module identification (Xen
>> probes module types), nor is i386 using multiboot2, nor can all modules
>> be probed, so order still matters. To avoid confusion I'd simply
>> replaced the above three lines with
>>
>> Modules should be loaded in the following order:
>>
>>> +The 1st module: dom0 kernel image
>>> +The 2nd module: dom0 ramdisk (optional)
>>
>> This covers both supported platforms without going into too deep
>> details; if you and Vladimir are OK, I'll commit with this change.
>
> Thank you very much!
> Sorry I am not familiar with xen on i386, so maybe I misunderstand this.
> So please commit with your change, Thanks for your correction :-)

I just fetched the mainline GRUB, i would like to know why this
patchset haven't been applied?
Anything I need to do(improve it or post a new patchset according to
your suggestion) for this patchset?

Great thanks :-)

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



-- 
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 0/1] ARM: Implement support for write-ctrlreg vm-events

[Corneliu ZUZU]

On 3/3/2016 4:10 PM, Corneliu ZUZU wrote:

Then,
QUESTIONS (FOR VM-EVENTS & ARM MAINTAINERS ESPECIALLY):

Q1) [...]

Q2) [...]

Q3) [...]

Q4) [...]


Hey all,

I have a question relating to this part of code @ vmx_update_guest_cr:

if ( paging_mode_hap(v->domain) )
{
/* Manage GUEST_CR3 when CR0.PE=0. */
uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
 CPU_BASED_CR3_STORE_EXITING);
v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
v->arch.hvm_vmx.exec_control |= cr3_ctls;

/* Trap CR3 updates if CR3 memory events are enabled. */
if ( v->domain->arch.monitor.write_ctrlreg_enabled &
 monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;

vmx_update_cpu_exec_control(v);
}

While trying to move the check for VM_EVENT_X86_CR3 to the scheduling 
tail, a few questions came to my mind.


1). Tamas, Razvan, maybe you guys could clarify this. I noticed this 
part of code is only executed if paging_mode_hap(v->domain). Is EPT 
mandatory to monitor CR3 writes or is it just that when shadow paging is 
enabled, CR3 r/w are unconditionally trapped? If the former is true, 
shouldn't we do a check like this in vm_event_monitor_get_capabilities 
instead?


2). I was also wondering why CR3 load/stores are trapped if paging is 
disabled for a domain.


Thanks,
Corneliu.

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

Re: [Xen-devel] [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Tamas K Lengyel]

On Mon, Mar 7, 2016 at 9:22 AM, Corneliu ZUZU  wrote:

> On 3/3/2016 4:10 PM, Corneliu ZUZU wrote:
>
>> Then,
>> QUESTIONS (FOR VM-EVENTS & ARM MAINTAINERS ESPECIALLY):
>>
>> Q1) [...]
>>
>> Q2) [...]
>>
>> Q3) [...]
>>
>> Q4) [...]
>>
>
> Hey all,
>
> I have a question relating to this part of code @ vmx_update_guest_cr:
>
> if ( paging_mode_hap(v->domain) )
> {
> /* Manage GUEST_CR3 when CR0.PE=0. */
> uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
>  CPU_BASED_CR3_STORE_EXITING);
> v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
> if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
> v->arch.hvm_vmx.exec_control |= cr3_ctls;
>
> /* Trap CR3 updates if CR3 memory events are enabled. */
> if ( v->domain->arch.monitor.write_ctrlreg_enabled &
>  monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
> v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;
>
> vmx_update_cpu_exec_control(v);
> }
>
> While trying to move the check for VM_EVENT_X86_CR3 to the scheduling
> tail, a few questions came to my mind.
>
> 1). Tamas, Razvan, maybe you guys could clarify this. I noticed this part
> of code is only executed if paging_mode_hap(v->domain). Is EPT mandatory to
> monitor CR3 writes or is it just that when shadow paging is enabled, CR3
> r/w are unconditionally trapped?


EPT is not really required for CR3 monitoring, it just has been the case
that vm_events have been only implemented for hap-enabled domains. AFAIK
for non-hap case CR3 needs to be trapped unconditionally, yes.


> If the former is true, shouldn't we do a check like this in
> vm_event_monitor_get_capabilities instead?
>

Yes, it should now, this code was just written before
vm_event_monitor_get_capabilities was introduced and we haven't gotten
around converting this check to it.


>
> 2). I was also wondering why CR3 load/stores are trapped if paging is
> disabled for a domain.
>

Good question, I was wondering about that myself at some point but I
haven't found an answer to it. Maybe some git archaeology can help
determining when that was added and why ;)

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

Re: [Xen-devel] [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Corneliu ZUZU]

On 3/7/2016 11:12 AM, Tamas K Lengyel wrote:
EPT is not really required for CR3 monitoring, it just has been the 
case that vm_events have been only implemented for hap-enabled domains.


I suppose this is not valid for vm-events in their entirety, right? I 
mean it seems to me that @ least for monitor vm-events VMX is enough.



AFAIK for non-hap case CR3 needs to be trapped unconditionally, yes.

If the former is true, shouldn't we do a check like this in
vm_event_monitor_get_capabilities instead?


Yes, it should now, this code was just written before 
vm_event_monitor_get_capabilities was introduced and we haven't gotten 
around converting this check to it.


Is there any reason why monitor vm-events in their current state 
wouldn't work on non-hap domains?
If they would work, shouldn't we instead simply move the 
monitor.write_ctrlreg_enabled part out of the if ( paging_mode_hap(...) ) ?




2). I was also wondering why CR3 load/stores are trapped if paging
is disabled for a domain.


Good question, I was wondering about that myself at some point but I 
haven't found an answer to it. Maybe some git archaeology can help 
determining when that was added and why ;)


Cheers,
Tamas


Yep, will "blame into it".

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

Re: [Xen-devel] [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Tamas K Lengyel]

On Mon, Mar 7, 2016 at 10:31 AM, Corneliu ZUZU 
wrote:

> On 3/7/2016 11:12 AM, Tamas K Lengyel wrote:
>
> EPT is not really required for CR3 monitoring, it just has been the case
> that vm_events have been only implemented for hap-enabled domains.
>
>
> I suppose this is not valid for vm-events in their entirety, right? I mean
> it seems to me that @ least for monitor vm-events VMX is enough.
>

Yes. OTOH I don't think you can find any CPUs on the market today that
support VMX but have no EPT so this hasn't really caused any issues for
anyone using vm_events, but technically yes VMX is enough for these events.

> AFAIK for non-hap case CR3 needs to be trapped unconditionally, yes.
>
>
>> If the former is true, shouldn't we do a check like this in
>> vm_event_monitor_get_capabilities instead?
>>
>
> Yes, it should now, this code was just written before
> vm_event_monitor_get_capabilities was introduced and we haven't gotten
> around converting this check to it.
>
>
> Is there any reason why monitor vm-events in their current state wouldn't
> work on non-hap domains?
> If they would work, shouldn't we instead simply move the
> monitor.write_ctrlreg_enabled part out of the if ( paging_mode_hap(...) ) ?
>

Yeap, that sounds like the right place to have that check.

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

Re: [Xen-devel] Changes to xenbits login (removing ssh password authentication) - please reply by Friday March 4th

[George Dunlap]

It seems my key is already on xenbits.

 -George

On 02/03/16 17:15, Lars Kurth wrote:
> Hi all,
> 
> due to the denyhosts package having been removed from Jessie, we are planning 
> to disable SSH password authentication from xenbits. The majority of people 
> who have xenbits accounts do use SSH public-key authentication, but there may 
> be some people who don't. 
> 
> I added people who I could identify from their logins and who have been 
> active in the community recently into the BCC list. 
> 
> If you do use password authentication and not SSH public-key authentication, 
> please reply to this mail. We may need to install your SSH key on xenbits.
> 
> Best Regards
> Lars
> 


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

Re: [Xen-devel] [V3] x86/xsaves: calculate the xstate_comp_offsets base on xcomp_bv

[Shuai Ruan]

On Fri, Mar 04, 2016 at 06:56:35AM -0700, Jan Beulich wrote:
> >>> On 04.03.16 at 12:00,  wrote:
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -934,8 +934,14 @@ long arch_do_domctl(
> >  goto vcpuextstate_out;
> >  }
> >  
> > -expand_xsave_states(v, xsave_area,
> > -size - 2 * sizeof(uint64_t));
> > +ret = expand_xsave_states(v, xsave_area,
> > +  size - 2 * sizeof(uint64_t));
> > +if ( ret )
> > +{
> > +xfree(xsave_area);
> > +vcpu_unpause(v);
> > +goto vcpuextstate_out;
> > +}
> 
> Well, while this is one way to deal with the problem, it's certainly
> not the most desirable one: We should try to avoid runtime
> allocations, failures of which then cause other things to fail (in
> perhaps not very graceful ways). And doing so is pretty simple
> here, and you even have two options: Either allocate a per-CPU
> array, or - considering that XCNTXT_MASK has only a limited
> number of bits set - even use an on-stack array of suitable
> (compile time determined from XCNTXT_MASK) size. If you
Thanks.
I will change it to on-stack array.
For "size compile time determined from XCNTXT_MASK", hweight64(XCNTXT_MASK) 
can return the num of bits set. But we need to caculte the highest bit set 
in XCNTXT_MASK at compile time, is there any macro can be used here ?

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

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

[osstest service owner]

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

Regressions :-(

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

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

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-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-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-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 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
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-libvirt-raw  9 debian-di-installfail   never pass

version targeted for testing:
 linuxb9a9cfdbf7254f4a231cc8ddf685cc29d3a9c6e5
baseline version:
 linux07cc49f66973f49a391c91bf4b158fa0f2562ca8

Last test of basis66399  2015-12-15 18:20:39 Z   82 days
Failing since 78925  2016-01-24 13:50:39 Z   42 days   43 attempts
Testing same since85582  2016-03-06 13:53:34 Z0 days1 attempts


431 people touched revisions under test,
not listing them all

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

[Xen-devel] [xen-4.5-testing baseline-only test] 44228: tolerable FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 44228 xen-4.5-testing real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/44228/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-credit2 19 guest-start/debian.repeat fail blocked in 44166
 test-amd64-i386-rumpuserxen-i386 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail blocked in 44166
 test-amd64-amd64-libvirt-vhd 13 guest-saverestorefail blocked in 44166
 test-amd64-i386-xl-qemuu-winxpsp3 15 guest-localmigrate/x10 fail blocked in 
44166

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-rumpuserxen-amd64 15 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt-qcow2 10 guest-start  fail never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 10 guest-start  fail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  10 guest-start  fail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass

version targeted for testing:
 xen  d165c490224da17c5dcaa2964fbcf59cd7dedc56
baseline version:
 xen  fe71162ab965d4a3344bb867f88e967806c80af5

Last test of basis44166  2016-02-26 11:50:23 Z9 days
Testing same since44228  2016-03-07 02:19:25 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Ian Campbell 
  Ian Jackson 
  Jan Beulich 
  Julien Grall 
  Tim Deegan 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-qemuu-nested-amdfail
 test-amd64-amd64-xl-pvh-amd  fail
 test-amd64-i386-qemut-rhel6hvm-amd   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64pass
 test-amd64-i386-xl-qemut-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ov

Re: [Xen-devel] [PATCH v2 1/3] console: allow log level threshold adjustments

[Jan Beulich]

>>> On 04.03.16 at 21:55,  wrote:
>> +case XEN_SYSCTL_LOGLVL_set:
>> +if ( (op->host.lower_thresh >= 0 && op->host.upper_thresh >= 0 &&
>> +  op->host.lower_thresh > op->host.upper_thresh) ||
>> + (op->guest.lower_thresh >= 0 && op->guest.upper_thresh >= 0 &&
>> +  op->guest.lower_thresh > op->guest.upper_thresh) )
>> +return -EINVAL;
>> +
>> +do_loglvl_op(&op->host, &xenlog_lower_thresh,
>> + &xenlog_upper_thresh, "standard");
> 
> 
> The keyboard and the sysctl both allow the user to go beyound the XENLOG_
> values we have. That is you could set the lower and upper threshold to be
> at 9 (or more) say. It will have the same effect as XENLOG_DEBUG (which is 
> 4)
> as printk_prefix_check seems to have a simple < check.
> 
> But perhaps to be correct only accept only proper values? Not allow
> the system admin to set the level to say 31415?

Since there's no bad side effect from doing so I opted for not
adding respective extra checks, keeping the code easier to read.

Jan


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

Re: [Xen-devel] [PATCH v2 00/16] Scheduling related tracing improvements

[Jan Beulich]

>>> On 04.03.16 at 19:25,  wrote:
> Hello committers, George,
> 
> This is basically a ping for this series, as I think most of it can
> actually go in, unless I've missed something.
> 
> So, let me try to recap:
> 
> On Tue, 2016-02-16 at 19:11 +0100, Dario Faggioli wrote:
>> 
>> Dario Faggioli (16):
>>   xen: sched: __runq_tickle takes a useless cpu parameter
>>   xen: sched: move up the trace record for vcpu_wake and
>> vcpu_sleep
>>   xen: sched: improve domain creation tracing
>>   xen: credit2: pack trace data better for xentrace_format
>>   xen: RTDS: pack trace data better for xentrace_format
>>   xen: sched: tracing: enable TSC tracing for all events
>>
> Until here, it's in already.

And that's the part I could reasonably take care of. I generally avoid
committing larger chunks of tools/ stuff, with the expectation that
Ian would take deal with those.

Jan


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

Re: [Xen-devel] [Qemu-devel] RFC: configuring QEMU virtfs for Xen PV(H) guests

[Wei Liu]

On Mon, Mar 07, 2016 at 08:21:46AM +0100, Juergen Gross wrote:
> Hi Wei,
> 
> On 15/02/16 14:44, Wei Liu wrote:
> > On Mon, Feb 15, 2016 at 02:33:05PM +0100, Juergen Gross wrote:
> >> On 15/02/16 14:16, Wei Liu wrote:
> >>> On Mon, Feb 15, 2016 at 09:07:13AM +, Paul Durrant wrote:
> >
> >>> [...]
> > # Option 2: Invent a xen-9p device
> >
> > Another way of doing it is to expose a dummy xen-9p device, so that we
> > can use -fsdev XXX -device xen-9p,YYY.  This simple device should be
> > used to capture the parameters like mount_tag and fsdev_id, and then
> > chained itself to a known location.  Later Xen transport can traverse
> > this known location. This xen-9p device doesn't seem to fit well into
> > the hierarchy. The best I can think of its parent should be
> > TYPE_DEVICE.  In this case:
> >
> > 1. Toolstack arranges some xenstore entries.
> > 2. Toolstack arranges command line options for QEMU:
> >   -fsdev XXX -device xen-9p,XXX
> > 3. QEMU starts up in xen-attach mode, scans xenstore for relevant
> >entries, then traverses the known location.
> >
> > Downside: Inventing a dummy device looks suboptimal to me.
> >>
> >> Sorry, didn't notice this thread before.
> >>
> > 
> > No need to be sorry. I posted this last Friday night. I wouldn't expect
> > many replies on Monady.
> > 
> >> For Xen pvUSB backend in qemu I need a Xen system device acting as
> >> parent for being able to attach/detach virtual USB busses.
> >>
> >> I haven't had time to update my patches for some time, but the patch
> >> for this system device is rather easy. It could be used as a parent
> >> of the xen-9p devices, too.
> >>
> >> I've attached the patch for reference.
> >>
> > 
> > Thanks. I will have a look at your patch.
> 
> Did you have some time to look at the patch? I'm asking because I
> finally found some time to start working on V2 of my qemu based pvUSB
> backend. Stefano asked me to hide the system device in my backend and
> I want to avoid that in case you are needing it, too.
> 

Yes. I need this device. I'm not sure what "hiding this device in
backend" means though.

Wei.

> Juergen

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

Re: [Xen-devel] [PATCH v2 00/16] Scheduling related tracing improvements

[Wei Liu]

On Mon, Mar 07, 2016 at 03:50:20AM -0700, Jan Beulich wrote:
> >>> On 04.03.16 at 19:25,  wrote:
> > Hello committers, George,
> > 
> > This is basically a ping for this series, as I think most of it can
> > actually go in, unless I've missed something.
> > 
> > So, let me try to recap:
> > 
> > On Tue, 2016-02-16 at 19:11 +0100, Dario Faggioli wrote:
> >> 
> >> Dario Faggioli (16):
> >>   xen: sched: __runq_tickle takes a useless cpu parameter
> >>   xen: sched: move up the trace record for vcpu_wake and
> >> vcpu_sleep
> >>   xen: sched: improve domain creation tracing
> >>   xen: credit2: pack trace data better for xentrace_format
> >>   xen: RTDS: pack trace data better for xentrace_format
> >>   xen: sched: tracing: enable TSC tracing for all events
> >>
> > Until here, it's in already.
> 
> And that's the part I could reasonably take care of. I generally avoid
> committing larger chunks of tools/ stuff, with the expectation that
> Ian would take deal with those.
> 

Ian is away this week. To avoid having no tools stuff committed this
whole week, I can prepare a branch for you to pull if you think that's
OK.

Wei.

> Jan
> 

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

Re: [Xen-devel] [Qemu-devel] RFC: configuring QEMU virtfs for Xen PV(H) guests

[Juergen Gross]

On 07/03/16 11:51, Wei Liu wrote:
> On Mon, Mar 07, 2016 at 08:21:46AM +0100, Juergen Gross wrote:
>> Hi Wei,
>>
>> On 15/02/16 14:44, Wei Liu wrote:
>>> On Mon, Feb 15, 2016 at 02:33:05PM +0100, Juergen Gross wrote:
 On 15/02/16 14:16, Wei Liu wrote:
> On Mon, Feb 15, 2016 at 09:07:13AM +, Paul Durrant wrote:
>>>
> [...]
>>> # Option 2: Invent a xen-9p device
>>>
>>> Another way of doing it is to expose a dummy xen-9p device, so that we
>>> can use -fsdev XXX -device xen-9p,YYY.  This simple device should be
>>> used to capture the parameters like mount_tag and fsdev_id, and then
>>> chained itself to a known location.  Later Xen transport can traverse
>>> this known location. This xen-9p device doesn't seem to fit well into
>>> the hierarchy. The best I can think of its parent should be
>>> TYPE_DEVICE.  In this case:
>>>
>>> 1. Toolstack arranges some xenstore entries.
>>> 2. Toolstack arranges command line options for QEMU:
>>>   -fsdev XXX -device xen-9p,XXX
>>> 3. QEMU starts up in xen-attach mode, scans xenstore for relevant
>>>entries, then traverses the known location.
>>>
>>> Downside: Inventing a dummy device looks suboptimal to me.

 Sorry, didn't notice this thread before.

>>>
>>> No need to be sorry. I posted this last Friday night. I wouldn't expect
>>> many replies on Monady.
>>>
 For Xen pvUSB backend in qemu I need a Xen system device acting as
 parent for being able to attach/detach virtual USB busses.

 I haven't had time to update my patches for some time, but the patch
 for this system device is rather easy. It could be used as a parent
 of the xen-9p devices, too.

 I've attached the patch for reference.

>>>
>>> Thanks. I will have a look at your patch.
>>
>> Did you have some time to look at the patch? I'm asking because I
>> finally found some time to start working on V2 of my qemu based pvUSB
>> backend. Stefano asked me to hide the system device in my backend and
>> I want to avoid that in case you are needing it, too.
>>
> 
> Yes. I need this device. I'm not sure what "hiding this device in
> backend" means though.

Stefano wanted it to be pvusb backend private: instead of adding it to
hw/xenpv/xen_machine_pv.c he wanted me to add it to hw/usb/xen-usb.c
where it would be usable by the pvUSB backend only.

With you needing that device I can leave the patch more or less
unmodified (some rebasing to the actual qemu version is needed).

Thanks for looking into the patch,

Juergen


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

Re: [Xen-devel] [PATCH] xen vtd : set msi guest_masked 0 by default

[Jan Beulich]

>>> On 07.03.16 at 09:12,  wrote:
> 2016-01-26 20:56 GMT+08:00 Jan Beulich :
> 
>> >>> On 26.01.16 at 02:34,  wrote:
>> > There are some problems when msi guest_masked is set to 1 by default.
>> > When guest os is windows 2008 r2 server,
>> > the device(eg X540-AT2 vf) is not initialized correctly.
>> > Host will always receive message like this :"VF Reset msg received from
>> vf".
>> > Guest has network connectivity issues,
>> > and can not correctly receive/send the packet.
>> > So, guest_masked is set to 0 by default.
>>
>> You describe a problem and half of your change, but there's no
>> connection between the two: What is actually wrong with current
>> behavior (matching the hardware's - MSI-X mask bits are set when
>> coming out of reset).
>>
>> > --- a/xen/arch/x86/msi.c
>> > +++ b/xen/arch/x86/msi.c
>> > @@ -512,7 +512,7 @@ void guest_mask_msi_irq(struct irq_desc *desc,
>> bool_t mask)
>> >
>> >  static unsigned int startup_msi_irq(struct irq_desc *desc)
>> >  {
>> > -if ( unlikely(!msi_set_mask_bit(desc, 0, !!(desc->status &
>> IRQ_GUEST))) )
>> > +if ( unlikely(!msi_set_mask_bit(desc, 0, 0) ))
>> >  WARN();
>> >  return 0;
>> >  }
>>
>> Whether this part can go under "set ... by default" is highly
>> questionable. Plus, while this affects MSI and MSI-X, ...
>>
>>  If irq is owned by guest,in function msi_set_mask_bit():
> ...
> bool_t flag = host || guest; //The flag should be true.
> ...
>  writel(flag, entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> ...
> PCI device can not generate interrrupt.
> windows guest can not change vector_ctrl_mask, guest os get abnormal status
> of nic.

Here and below - I'm sorry, I do not understand what you're trying
to tell us, or how this is meant to extend beyond the original (too
vague) description of your change. Among unclear things is why
"windows guest can not change vector_ctrl_mask" - you again just
make statements without dealing with any of the why-s.

Please can you try to explain things by matching operations done
by the guest, qemu, and the hypervisor to the effect they have
on the state of the mask bit, and then point out which of those
steps needs changing or doesn't work as intended?

Jan

>> > @@ -972,7 +972,7 @@ static int msix_capability_init(struct pci_dev *dev,
>> >  entry->msi_attrib.entry_nr = msi->entry_nr;
>> >  entry->msi_attrib.maskbit = 1;
>> >  entry->msi_attrib.host_masked = 1;
>> > -entry->msi_attrib.guest_masked = 1;
>> > +entry->msi_attrib.guest_masked = 0;
>> >  entry->msi_attrib.pos = pos;
>> >  entry->irq = msi->irq;
>> >  entry->dev = dev;
>>
>> ... this change affect MSI-X only, and doing some guessing from
>> what you write above I suspect you only really tested one of the
>> two cases.
>>
>> So while the change _may_ be necessary, you'll need to do a
>> better job at explaining why you what you do.
>>
> Msi guest_masked is set to 0 in the original code, only msi-x guest_masked
> is modifed in msix_capability_init() function by patch.
> 
>>
>> Jan
>>
>>
> This issue appears after commited the variable guest_mask.
> Initialization operations of pci device may be changed in windows
> guest,or Xen need to change the initial state of vtd pci device.
> -- 
> Jianzhong,Chang




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

Re: [Xen-devel] [Qemu-devel] RFC: configuring QEMU virtfs for Xen PV(H) guests

[Wei Liu]

On Mon, Mar 07, 2016 at 11:56:15AM +0100, Juergen Gross wrote:
> On 07/03/16 11:51, Wei Liu wrote:
> > On Mon, Mar 07, 2016 at 08:21:46AM +0100, Juergen Gross wrote:
> >> Hi Wei,
> >>
> >> On 15/02/16 14:44, Wei Liu wrote:
> >>> On Mon, Feb 15, 2016 at 02:33:05PM +0100, Juergen Gross wrote:
>  On 15/02/16 14:16, Wei Liu wrote:
> > On Mon, Feb 15, 2016 at 09:07:13AM +, Paul Durrant wrote:
> >>>
> > [...]
> >>> # Option 2: Invent a xen-9p device
> >>>
> >>> Another way of doing it is to expose a dummy xen-9p device, so that we
> >>> can use -fsdev XXX -device xen-9p,YYY.  This simple device should be
> >>> used to capture the parameters like mount_tag and fsdev_id, and then
> >>> chained itself to a known location.  Later Xen transport can traverse
> >>> this known location. This xen-9p device doesn't seem to fit well into
> >>> the hierarchy. The best I can think of its parent should be
> >>> TYPE_DEVICE.  In this case:
> >>>
> >>> 1. Toolstack arranges some xenstore entries.
> >>> 2. Toolstack arranges command line options for QEMU:
> >>>   -fsdev XXX -device xen-9p,XXX
> >>> 3. QEMU starts up in xen-attach mode, scans xenstore for relevant
> >>>entries, then traverses the known location.
> >>>
> >>> Downside: Inventing a dummy device looks suboptimal to me.
> 
>  Sorry, didn't notice this thread before.
> 
> >>>
> >>> No need to be sorry. I posted this last Friday night. I wouldn't expect
> >>> many replies on Monady.
> >>>
>  For Xen pvUSB backend in qemu I need a Xen system device acting as
>  parent for being able to attach/detach virtual USB busses.
> 
>  I haven't had time to update my patches for some time, but the patch
>  for this system device is rather easy. It could be used as a parent
>  of the xen-9p devices, too.
> 
>  I've attached the patch for reference.
> 
> >>>
> >>> Thanks. I will have a look at your patch.
> >>
> >> Did you have some time to look at the patch? I'm asking because I
> >> finally found some time to start working on V2 of my qemu based pvUSB
> >> backend. Stefano asked me to hide the system device in my backend and
> >> I want to avoid that in case you are needing it, too.
> >>
> > 
> > Yes. I need this device. I'm not sure what "hiding this device in
> > backend" means though.
> 
> Stefano wanted it to be pvusb backend private: instead of adding it to
> hw/xenpv/xen_machine_pv.c he wanted me to add it to hw/usb/xen-usb.c
> where it would be usable by the pvUSB backend only.
> 
> With you needing that device I can leave the patch more or less
> unmodified (some rebasing to the actual qemu version is needed).
> 

Yes, please make it available to other PV backends. Someone might want
to graft every device we have to that hierarchy some day later. ;-)

> Thanks for looking into the patch,
> 

Thanks for posting this patch and sorry for the long delay.

Wei.

> Juergen
> 

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

Re: [Xen-devel] [PATCH v6 3/5] IOMMU: Make the pcidevs_lock a recursive one

[Jan Beulich]

>>> On 07.03.16 at 08:05,  wrote:
> I try to fix it with follow:
> patch >>  
> 
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -118,6 +118,11 @@ int pcidevs_is_locked(void)
>  return spin_is_locked(&_pcidevs_lock);
>  }
> 
> +int pcidevs_trylock(void)
> +{
> +return spin_trylock_recursive(&_pcidevs_lock);
> +}
> +
>  void __init pt_pci_init(void)
>  {
>  radix_tree_init(&pci_segments);
> @@ -1365,7 +1370,7 @@ static int assign_device(struct domain *d, u16 seg, u8 
> bus, u8 devfn, u32 flag)
>   p2m_get_hostp2m(d)->global_logdirty)) )
>  return -EXDEV;
> 
> -if ( !spin_trylock(&pcidevs_lock) )
> +if ( !pcidevs_trylock() )
>  return -ERESTART;

Exactly.

> A quick question, is it '-ERESTART', instead of '-EBUSY' ?

No idea what this question is about in this context.

Jan


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

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

[George Dunlap]

On Fri, Mar 4, 2016 at 10:00 PM, Konrad Rzeszutek Wilk
 wrote:
>> +/* Handle VT-d posted-interrupt when VCPU is blocked. */
>> +static void pi_wakeup_interrupt(struct cpu_user_regs *regs)
>> +{
>> +struct arch_vmx_struct *vmx, *tmp;
>> +spinlock_t *lock = &per_cpu(vmx_pi_blocking, smp_processor_id()).lock;
>> +struct list_head *blocked_vcpus =
>> + &per_cpu(vmx_pi_blocking, smp_processor_id()).list;
>> +
>> +ack_APIC_irq();
>> +this_cpu(irq_count)++;
>> +
>> +spin_lock(lock);
>> +
>> +/*
>> + * XXX: The length of the list depends on how many vCPU is current
>> + * blocked on this specific pCPU. This may hurt the interrupt latency
>> + * if the list grows to too many entries.
>> + */
>> +list_for_each_entry_safe(vmx, tmp, blocked_vcpus, pi_blocking.list)
>> +{
>
>
> My recollection of the 'most-horrible' case of this being really bad is when
> the scheduler puts the vCPU0 and VCPU1 of the guest on the same pCPU (as an 
> example)
> and they round-robin all the time.
>
> 
> Would it be perhaps possible to have an anti-affinity flag to deter the
> scheduler from this? That is whichever struct vcpu has 'anti-affinity' flag
> set - the scheduler will try as much as it can _to not_ schedule the 'struct 
> vcpu'
> if the previous 'struct vcpu' had this flag as well on this pCPU?

Well having vcpus from the same guest on the same pcpu is problematic
for a number of reasons -- spinlocks first and foremost.  So in
general trying to avoid that would be useful for most guests.

The thing with scheduling is that it's a bit like economics: it seems
simple but it's actually not at all obvious what the emergent behavior
will be from adding a simple rule. :-)

On the whole it seems unlikely that having two vcpus on a single pcpu
is a "stable" situation -- it's likely to be pretty transient, and
thus not have a major impact on performance.

That said, the load balancing code from credit2 *should*, in theory,
make it easier to implement this sort of thing; it has the concept of
a "cost" that it's trying to minimize; so you could in theory add a
"cost" to configurations where vcpus from the same processor share the
same pcpu.  Then it's not a hard-and-fast rule: if you have more vcpus
than pcpus, the scheduler will just deal. :-)

But I think some profiling is in order before anyone does serious work on this.

 -George

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

Re: [Xen-devel] [PATCH v2 00/16] Scheduling related tracing improvements

[Jan Beulich]

>>> On 07.03.16 at 11:54,  wrote:
> On Mon, Mar 07, 2016 at 03:50:20AM -0700, Jan Beulich wrote:
>> >>> On 04.03.16 at 19:25,  wrote:
>> > Hello committers, George,
>> > 
>> > This is basically a ping for this series, as I think most of it can
>> > actually go in, unless I've missed something.
>> > 
>> > So, let me try to recap:
>> > 
>> > On Tue, 2016-02-16 at 19:11 +0100, Dario Faggioli wrote:
>> >> 
>> >> Dario Faggioli (16):
>> >>   xen: sched: __runq_tickle takes a useless cpu parameter
>> >>   xen: sched: move up the trace record for vcpu_wake and
>> >> vcpu_sleep
>> >>   xen: sched: improve domain creation tracing
>> >>   xen: credit2: pack trace data better for xentrace_format
>> >>   xen: RTDS: pack trace data better for xentrace_format
>> >>   xen: sched: tracing: enable TSC tracing for all events
>> >>
>> > Until here, it's in already.
>> 
>> And that's the part I could reasonably take care of. I generally avoid
>> committing larger chunks of tools/ stuff, with the expectation that
>> Ian would take deal with those.
>> 
> 
> Ian is away this week. To avoid having no tools stuff committed this
> whole week, I can prepare a branch for you to pull if you think that's
> OK.

Well, if these were urgent I'd say yes. But I don't think they are,
so I'd leave it to either Konrad (who has basically asked for what
you offer) if he wants to deal with it, of wait for Ian's return.

Jan


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

Re: [Xen-devel] [PATCH v6 3/5] IOMMU: Make the pcidevs_lock a recursive one

[Xu, Quan]

On March 07, 2016 7:14pm,  wrote:
> >>> On 07.03.16 at 08:05,  wrote:


> > A quick question, is it '-ERESTART', instead of '-EBUSY' ?
> 
> No idea what this question is about in this context.
> 

it is in xen/drivers/passthrough/pci.c, assign_device().

static int assign_device()
{
   
if ( !spin_trylock(&pcidevs_lock) )
return -ERESTART;
   
}

Quan

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

Re: [Xen-devel] [PATCH v2 00/16] Scheduling related tracing improvements

[Wei Liu]

On Mon, Mar 07, 2016 at 04:21:22AM -0700, Jan Beulich wrote:
> >>> On 07.03.16 at 11:54,  wrote:
> > On Mon, Mar 07, 2016 at 03:50:20AM -0700, Jan Beulich wrote:
> >> >>> On 04.03.16 at 19:25,  wrote:
> >> > Hello committers, George,
> >> > 
> >> > This is basically a ping for this series, as I think most of it can
> >> > actually go in, unless I've missed something.
> >> > 
> >> > So, let me try to recap:
> >> > 
> >> > On Tue, 2016-02-16 at 19:11 +0100, Dario Faggioli wrote:
> >> >> 
> >> >> Dario Faggioli (16):
> >> >>   xen: sched: __runq_tickle takes a useless cpu parameter
> >> >>   xen: sched: move up the trace record for vcpu_wake and
> >> >> vcpu_sleep
> >> >>   xen: sched: improve domain creation tracing
> >> >>   xen: credit2: pack trace data better for xentrace_format
> >> >>   xen: RTDS: pack trace data better for xentrace_format
> >> >>   xen: sched: tracing: enable TSC tracing for all events
> >> >>
> >> > Until here, it's in already.
> >> 
> >> And that's the part I could reasonably take care of. I generally avoid
> >> committing larger chunks of tools/ stuff, with the expectation that
> >> Ian would take deal with those.
> >> 
> > 
> > Ian is away this week. To avoid having no tools stuff committed this
> > whole week, I can prepare a branch for you to pull if you think that's
> > OK.
> 
> Well, if these were urgent I'd say yes. But I don't think they are,
> so I'd leave it to either Konrad (who has basically asked for what
> you offer) if he wants to deal with it, of wait for Ian's return.
> 

It's not urgent, but I would like to avoid wasting any test cycle. No
matter how innocent a patch looks, there is always risk that it breaks
something.

Konrad, your call.

Wei.

> Jan
> 

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

Re: [Xen-devel] [PATCH v6 3/5] IOMMU: Make the pcidevs_lock a recursive one

[Jan Beulich]

>>> On 07.03.16 at 12:23,  wrote:
> On March 07, 2016 7:14pm,  wrote:
>> >>> On 07.03.16 at 08:05,  wrote:
> 
> 
>> > A quick question, is it '-ERESTART', instead of '-EBUSY' ?
>> 
>> No idea what this question is about in this context.
>> 
> 
> it is in xen/drivers/passthrough/pci.c, assign_device().
> 
> static int assign_device()
> {
>
> if ( !spin_trylock(&pcidevs_lock) )
> return -ERESTART;
>
> }

But I still don't understand what you're trying to find out or point
out.

Jan


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

Re: [Xen-devel] [PATCH v6 3/5] IOMMU: Make the pcidevs_lock a recursive one

[Xu, Quan]

On March 07, 2016 7:36pm,  wrote:
> >>> On 07.03.16 at 12:23,  wrote:
> > On March 07, 2016 7:14pm,  wrote:
> >> >>> On 07.03.16 at 08:05,  wrote:
> >
> >
> >> > A quick question, is it '-ERESTART', instead of '-EBUSY' ?
> >>
> >> No idea what this question is about in this context.
> >>
> >
> > it is in xen/drivers/passthrough/pci.c, assign_device().
> >
> > static int assign_device()
> > {
> >
> > if ( !spin_trylock(&pcidevs_lock) )
> > return -ERESTART;
> >
> > }
> 
> But I still don't understand what you're trying to find out or point out.
> 

Jan, sorry.
Now the return error code is '-ERESTART' for ' if ( 
!spin_trylock(&pcidevs_lock) ', in assign_device(), in 
xen/drivers/passthrough/pci.c.
I think it would be '-EBUSY'.
Quan

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

Re: [Xen-devel] [PATCH v2 3/3] xl: new "loglvl" command

[Jan Beulich]

>>> On 04.03.16 at 19:45,  wrote:
> On Fri, 2016-03-04 at 09:48 -0700, Jan Beulich wrote:
>> This is pretty simplistic for now, but I'd rather have someone better
>> friends with the tools improve it (if desired).
>> 
>> Signed-off-by: Jan Beulich 
>> 
>> --- a/tools/libxl/libxl.c
>> +++ b/tools/libxl/libxl.c
>> @@ -5958,6 +5958,26 @@ int libxl_send_debug_keys(libxl_ctx *ctx
>>  return 0;
>>  }
>>  
>> +int libxl_log_level(libxl_ctx *ctx, bool set, bool guest,
>> +int *lower_thresh, int *upper_thresh)
>> +{
>> +int ret;
>>
> As per libxl coding style, this wants to be 'r'.

This and everything else below look to be valid comments, but
it's rather frustrating that simply cloning an existing function (I
user the debug key ones as basis) doesn't give me valid code,
the more that I did scroll up and down a few pages to see
whether I just happened to pick a particularly bad example.
(This adds to the reasons why I've continue to push out getting
the tool stack side done for a patch the hypervisor side of which
has been done a couple of months back.)

Jan

>> +GC_INIT(ctx);
>>
> I don't seem to find it in CODING_STYLE, but I'd say there should be an
> empty line here.
> 
>> +if (set) {
>> +ret = xc_set_log_level(ctx->xch, guest, *lower_thresh,
>> *upper_thresh);
>> +} else {
>> +ret = xc_get_log_level(ctx->xch, guest, lower_thresh,
>> upper_thresh);
>> +}
>> +if ( ret < 0 ) {
>> +LOGE(ERROR, "%s %slog level",
>> + set ? "setting" : "getting", guest ? "guest " : "");
>> +GC_FREE;
>> +return ERROR_FAIL;
>>
> Libxl wants only one error/cleanup path out of the function, and
> recommends using a variable called rc for hosting the libxl error code
> to be returned, and goto, if necessary.
> 
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
> 
>> +int main_loglvl(int argc, char **argv)
>> +{
>> +static const struct option opts[] = {
>> +{"guest", 0, 0, 'g'},
>> +{"set", 0, 0, 's'},
>> +COMMON_LONG_OPTS
>> +};
>> +int opt, lower_thresh = -1, upper_thresh = -1;
>> +bool guest = false, set = false;
>> +
>> +SWITCH_FOREACH_OPT(opt, "gs:", opts, "loglvl", 0) {
>> +case 'g':
>> +guest = true;
>> +break;
>> +case 's':
>> +if (*optarg != '/')
>> +lower_thresh = parse_loglvl(&optarg);
>> +if (*optarg == '/') {
>> +++optarg;
>> +upper_thresh = parse_loglvl(&optarg);
>> +}
>> +set = true;
>> +break;
>> +}
>> +
>> +if (libxl_log_level(ctx, set, guest, &lower_thresh,
>> &upper_thresh)) {
>> +fprintf(stderr, "cannot %s %s log level\n",
>> +set ? "set" : "get", guest ? "guest" : "host");
>> +return 1;
>>
> This is indeed super-inconsistent in xl. But we're trying to improve it
>  (it's half done and there are patches) and using EXIT_FAILURE and
> EXIT_SUCCESS for program exit codes, and this return can be classified
> as such.
> 
>> +}
>> +
>> +if (!set)
>> +printf("%s log levels: %s/%s\n", guest ? "guest" : "host",
>> +   format_loglvl(lower_thresh),
>> format_loglvl(upper_thresh));
>> +
>> +return 0;
>>
> And this as well, of course. :-)
> 
> 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)




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

Re: [Xen-devel] [PATCH v6 3/5] IOMMU: Make the pcidevs_lock a recursive one

[Jan Beulich]

>>> On 07.03.16 at 12:42,  wrote:
> On March 07, 2016 7:36pm,  wrote:
>> >>> On 07.03.16 at 12:23,  wrote:
>> > On March 07, 2016 7:14pm,  wrote:
>> >> >>> On 07.03.16 at 08:05,  wrote:
>> >
>> >
>> >> > A quick question, is it '-ERESTART', instead of '-EBUSY' ?
>> >>
>> >> No idea what this question is about in this context.
>> >>
>> >
>> > it is in xen/drivers/passthrough/pci.c, assign_device().
>> >
>> > static int assign_device()
>> > {
>> >
>> > if ( !spin_trylock(&pcidevs_lock) )
>> > return -ERESTART;
>> >
>> > }
>> 
>> But I still don't understand what you're trying to find out or point out.
> 
> Jan, sorry.
> Now the return error code is '-ERESTART' for ' if ( 
> !spin_trylock(&pcidevs_lock) ', in assign_device(), in 
> xen/drivers/passthrough/pci.c.
> I think it would be '-EBUSY'.

Oh - definitely not. Just follow the call chain back up, and you
should find that this gets taken as an indication to create a
continuation, whereas -EBUSY would bubble back up to the
original (user space) caller (which is _not_ what we want here).

Jan


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

Re: [Xen-devel] [PATCH v6 3/5] IOMMU: Make the pcidevs_lock a recursive one

[Xu, Quan]

On March 07, 2016 7:49pm,  wrote:
> >>> On 07.03.16 at 12:42,  wrote:
> > On March 07, 2016 7:36pm,  wrote:
> >> >>> On 07.03.16 at 12:23,  wrote:
> >> > On March 07, 2016 7:14pm,  wrote:
> >> >> >>> On 07.03.16 at 08:05,  wrote:
> >> >
> >> >
> >> >> > A quick question, is it '-ERESTART', instead of '-EBUSY' ?
> >> >>
> >> >> No idea what this question is about in this context.
> >> >>
> >> >
> >> > it is in xen/drivers/passthrough/pci.c, assign_device().
> >> >
> >> > static int assign_device()
> >> > {
> >> >
> >> > if ( !spin_trylock(&pcidevs_lock) )
> >> > return -ERESTART;
> >> >
> >> > }
> >>
> >> But I still don't understand what you're trying to find out or point out.
> >
> > Jan, sorry.
> > Now the return error code is '-ERESTART' for ' if (
> > !spin_trylock(&pcidevs_lock) ', in assign_device(), in
> > xen/drivers/passthrough/pci.c.
> > I think it would be '-EBUSY'.
> 
> Oh - definitely not. Just follow the call chain back up, and you should find 
> that
> this gets taken as an indication to create a continuation, whereas -EBUSY 
> would
> bubble back up to the original (user space) caller (which is _not_ what we 
> want
> here).
> 

Got it. thanks.
Quan

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

Re: [Xen-devel] [V3] x86/xsaves: calculate the xstate_comp_offsets base on xcomp_bv

[Jan Beulich]

>>> On 07.03.16 at 11:19,  wrote:
> On Fri, Mar 04, 2016 at 06:56:35AM -0700, Jan Beulich wrote:
>> >>> On 04.03.16 at 12:00,  wrote:
>> > --- a/xen/arch/x86/domctl.c
>> > +++ b/xen/arch/x86/domctl.c
>> > @@ -934,8 +934,14 @@ long arch_do_domctl(
>> >  goto vcpuextstate_out;
>> >  }
>> >  
>> > -expand_xsave_states(v, xsave_area,
>> > -size - 2 * sizeof(uint64_t));
>> > +ret = expand_xsave_states(v, xsave_area,
>> > +  size - 2 * sizeof(uint64_t));
>> > +if ( ret )
>> > +{
>> > +xfree(xsave_area);
>> > +vcpu_unpause(v);
>> > +goto vcpuextstate_out;
>> > +}
>> 
>> Well, while this is one way to deal with the problem, it's certainly
>> not the most desirable one: We should try to avoid runtime
>> allocations, failures of which then cause other things to fail (in
>> perhaps not very graceful ways). And doing so is pretty simple
>> here, and you even have two options: Either allocate a per-CPU
>> array, or - considering that XCNTXT_MASK has only a limited
>> number of bits set - even use an on-stack array of suitable
>> (compile time determined from XCNTXT_MASK) size. If you
> Thanks.
> I will change it to on-stack array.
> For "size compile time determined from XCNTXT_MASK", hweight64(XCNTXT_MASK) 
> can return the num of bits set. But we need to caculte the highest bit set 
> in XCNTXT_MASK at compile time, is there any macro can be used here ?

You may want to pull in Linux'es ilog2().

Jan


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

Re: [Xen-devel] [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Corneliu ZUZU]

On 3/7/2016 11:45 AM, Tamas K Lengyel wrote:



On Mon, Mar 7, 2016 at 10:31 AM, Corneliu ZUZU > wrote:


On 3/7/2016 11:12 AM, Tamas K Lengyel wrote:

EPT is not really required for CR3 monitoring, it just has been
the case that vm_events have been only implemented for
hap-enabled domains.


I suppose this is not valid for vm-events in their entirety,
right? I mean it seems to me that @ least for monitor vm-events
VMX is enough.


Yes. OTOH I don't think you can find any CPUs on the market today that 
support VMX but have no EPT so this hasn't really caused any issues 
for anyone using vm_events, but technically yes VMX is enough for 
these events.



AFAIK for non-hap case CR3 needs to be trapped unconditionally, yes.

If the former is true, shouldn't we do a check like this in
vm_event_monitor_get_capabilities instead?


Yes, it should now, this code was just written before
vm_event_monitor_get_capabilities was introduced and we haven't
gotten around converting this check to it.


Is there any reason why monitor vm-events in their current state
wouldn't work on non-hap domains?
If they would work, shouldn't we instead simply move the
monitor.write_ctrlreg_enabled part out of the if (
paging_mode_hap(...) ) ?


Yeap, that sounds like the right place to have that check.

Tamas


Good, with that out of the way, one more issue to solve. What I'm 
actually trying to do is to move that part of the code to the scheduling 
tail - i.e. enabling/disabling CPU_BASED_CR3_LOAD_EXITING only when we 
actually enter the vcpu.
To do this I also need to know exactly in what cases 
CPU_BASED_CR3_LOAD_EXITING can/is enabled, besides the already mentioned 
case when a domain's paging is disabled.


I'm searching through the codebase right now but it's a bit dizzying, 
can someone provide some feedback on this matter?


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

Re: [Xen-devel] [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Corneliu ZUZU]

On 3/7/2016 2:07 PM, Corneliu ZUZU wrote:

On 3/7/2016 11:45 AM, Tamas K Lengyel wrote:



On Mon, Mar 7, 2016 at 10:31 AM, Corneliu ZUZU > wrote:


On 3/7/2016 11:12 AM, Tamas K Lengyel wrote:

EPT is not really required for CR3 monitoring, it just has been
the case that vm_events have been only implemented for
hap-enabled domains.


I suppose this is not valid for vm-events in their entirety,
right? I mean it seems to me that @ least for monitor vm-events
VMX is enough.


Yes. OTOH I don't think you can find any CPUs on the market today 
that support VMX but have no EPT so this hasn't really caused any 
issues for anyone using vm_events, but technically yes VMX is enough 
for these events.



AFAIK for non-hap case CR3 needs to be trapped unconditionally, yes.

If the former is true, shouldn't we do a check like this in
vm_event_monitor_get_capabilities instead?


Yes, it should now, this code was just written before
vm_event_monitor_get_capabilities was introduced and we haven't
gotten around converting this check to it.


Is there any reason why monitor vm-events in their current state
wouldn't work on non-hap domains?
If they would work, shouldn't we instead simply move the
monitor.write_ctrlreg_enabled part out of the if (
paging_mode_hap(...) ) ?


Yeap, that sounds like the right place to have that check.

Tamas


Good, with that out of the way, one more issue to solve. What I'm 
actually trying to do is to move that part of the code to the 
scheduling tail - i.e. enabling/disabling CPU_BASED_CR3_LOAD_EXITING 
only when we actually enter the vcpu.
To do this I also need to know exactly in what cases 
CPU_BASED_CR3_LOAD_EXITING can/is enabled, besides the already 
mentioned case when a domain's paging is disabled.


I'm searching through the codebase right now but it's a bit dizzying, 
can someone provide some feedback on this matter?


Thanks,
Corneliu.


Ok, by searching for places v->arch.hvm_vmx.exec_control is set, it 
seems that vmx_update_guest_cr is the only place where CR3 load-exiting 
is set/unset.


It also seems that in non-hap case CPU_BASED_CR3_LOAD_EXITING is indeed 
unconditionally enabled, i.e. @ vmx_vcpu_initialise -> vmx_create_vmcs 
-> construct_vmcs:


v->arch.hvm_vmx.secondary_exec_control = vmx_secondary_exec_control;

/* Disable VPID for now: we decide when to enable it on VMENTER. */
v->arch.hvm_vmx.secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;

if ( paging_mode_hap(d) )
{
v->arch.hvm_vmx.exec_control &= ~(CPU_BASED_INVLPG_EXITING |
  CPU_BASED_CR3_LOAD_EXITING |
CPU_BASED_CR3_STORE_EXITING);
}

Can somebody else confirm this, just to be sure?

Tamas, if this were true, that would mean that we can move that part to 
the scheduling tail, and we could write there smth like this pseudocode:


/* if ! hap => CR3 writes unconditionally trap */
if (paging_mode_hap) return;
if  (monitor.write_ctrlreg_enabled for CR3) and 
(CPU_BASED_CR3_LOAD_EXITING currently disabled)

enable CPU_BASED_CR3_LOAD_EXITING;
   else if (NOT monitor.write_ctrlreg_enabled for CR3) and 
(CPU_BASED_CR3_LOAD_EXITING currently enabled) and (paging is enabled)

disable CPU_BASED_CR3_LOAD_EXITING;

Would that be suitable?

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

[Xen-devel] [xen-4.3-testing test] 85586: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

flight 85586 xen-4.3-testing real [real]
http://logs.test-lab.xenproject.org/osstest/logs/85586/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf-pvops 3 host-install(3) broken REGR. vs. 83004
 build-armhf   3 host-install(3) broken REGR. vs. 83004
 test-amd64-amd64-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
83004
 test-amd64-amd64-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
83004
 test-amd64-i386-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
83004
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
83004

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 13 guest-localmigrate fail in 85479 
pass in 85586
 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-localmigrate/x10 fail pass in 
85479

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stopfail in 85479 like 83004
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 83004
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 83004

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-amd64-amd64-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail never pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install  fail never pass
 build-amd64-rumpuserxen   6 xen-buildfail   never pass
 build-i386-rumpuserxen6 xen-buildfail   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-vhd 11 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail never pass
 test-amd64-i386-xend-qemut-winxpsp3 20 leak-check/checkfail never pass

version targeted for testing:
 xen  404e83e055cb419efccbcb0c5c89476307a9ae46
baseline version:
 xen  ccc7adf9cff5d5f93720afcc1d0f7227d50feab2

Last test of basis83004  2016-02-18 14:47:44 Z   17 days
Testing same since84923  2016-03-01 13:41:07 Z5 days6 attempts


People who touched revisions under test:
  Ian Campbell 
  Ian Jackson 
  Wei Liu 

jobs:
 build-amd64  pass
 build-armhf  broken  
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  blocked 
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopsbroken  
 build-i386-pvops pass
 build-amd64-rumpuserxen  fail
 build-i386-rumpuserxen   fail
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  blocked 
 test-amd64-i386-xl   pass
 test-amd64-i386-qemut-rhel6hvm-amd   pass
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemut-debianhvm-amd64fail
 test-amd64-i386-xl-qemut-debianhvm-amd64 fail
 test-amd64-amd64-xl-qemuu-debianhvm-amd64fail
 test-amd64-i386-xl-qemuu-debianhvm-amd64 fail
 test-amd64-i386-freebsd10-amd64  pass
 test-amd64-amd64-xl-qemuu-ovmf-amd64 fail
 test-amd64-i386-xl-qemuu-ovmf-amd64  

Re: [Xen-devel] [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Andrew Cooper]

On 07/03/16 09:12, Tamas K Lengyel wrote:
>
>
> On Mon, Mar 7, 2016 at 9:22 AM, Corneliu ZUZU  > wrote:
>
> On 3/3/2016 4:10 PM, Corneliu ZUZU wrote:
>
> Then,
> QUESTIONS (FOR VM-EVENTS & ARM MAINTAINERS ESPECIALLY):
>
> Q1) [...]
>
> Q2) [...]
>
> Q3) [...]
>
> Q4) [...]
>
>
> Hey all,
>
> I have a question relating to this part of code @ vmx_update_guest_cr:
>
> if ( paging_mode_hap(v->domain) )
> {
> /* Manage GUEST_CR3 when CR0.PE =0. */
> uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
>  CPU_BASED_CR3_STORE_EXITING);
> v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
> if ( !hvm_paging_enabled(v) &&
> !vmx_unrestricted_guest(v) )
> v->arch.hvm_vmx.exec_control |= cr3_ctls;
>
> /* Trap CR3 updates if CR3 memory events are enabled. */
> if ( v->domain->arch.monitor.write_ctrlreg_enabled &
>  monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
> v->arch.hvm_vmx.exec_control |=
> CPU_BASED_CR3_LOAD_EXITING;
>
> vmx_update_cpu_exec_control(v);
> }
>
> While trying to move the check for VM_EVENT_X86_CR3 to the
> scheduling tail, a few questions came to my mind.
>
> 1). Tamas, Razvan, maybe you guys could clarify this. I noticed
> this part of code is only executed if paging_mode_hap(v->domain).
> Is EPT mandatory to monitor CR3 writes or is it just that when
> shadow paging is enabled, CR3 r/w are unconditionally trapped?
>
>
> EPT is not really required for CR3 monitoring, it just has been the
> case that vm_events have been only implemented for hap-enabled
> domains. AFAIK for non-hap case CR3 needs to be trapped
> unconditionally, yes.

Specifically, the shadow pagetable code needs to swap shadows when the
guest switches cr3.

>  
>
> If the former is true, shouldn't we do a check like this in
> vm_event_monitor_get_capabilities instead?
>
>
> Yes, it should now, this code was just written before
> vm_event_monitor_get_capabilities was introduced and we haven't gotten
> around converting this check to it.
>  
>
>
> 2). I was also wondering why CR3 load/stores are trapped if paging
> is disabled for a domain.
>
>
> Good question, I was wondering about that myself at some point but I
> haven't found an answer to it. Maybe some git archaeology can help
> determining when that was added and why ;)

Gen1 VT-x didn't support running a guest in non-paged mode.  Gen2
introduced "unrestricted-guest" which works as intended, but Gen1 has to
fake non-pagad mode using identity paging.  As a result, CR3 cannot be
used as scratch space like it can in non-paged mode, and the guest must
be prevented from moving CR3 away from the gfn set up by the domain
builder in HVM_PARAM_IDENT_PT.

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

Re: [Xen-devel] [Patch V1 3/3] xen: add pvUSB backend

[Juergen Gross]

Sorry, I just found time now to continue with this series.

On 27/10/15 19:54, Konrad Rzeszutek Wilk wrote:
> On Thu, Sep 03, 2015 at 12:45:13PM +0200, Juergen Gross wrote:
>> Add a backend for para-virtualized USB devices for xen domains.
>>
>> The backend is using host-libusb to forward USB requests from a
>> domain via libusb to the real device(s) passed through.
>>
>> Signed-off-by: Juergen Gross 
>> ---
>>  hw/usb/Makefile.objs |4 +
>>  hw/usb/xen-usb.c | 1120 
>> ++
>>  hw/xenpv/xen_machine_pv.c|3 +
>>  include/hw/xen/xen_backend.h |   13 +-
>>  4 files changed, 1137 insertions(+), 3 deletions(-)
>>  create mode 100644 hw/usb/xen-usb.c
>>
>> diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
>> index 3fe4dff..0253184 100644
>> --- a/hw/usb/Makefile.objs
>> +++ b/hw/usb/Makefile.objs
>> @@ -36,3 +36,7 @@ common-obj-$(CONFIG_USB_REDIR) += redirect.o quirks.o
>>  
>>  # usb pass-through
>>  common-obj-y += $(patsubst %,host-%.o,$(HOST_USB))
>> +
>> +ifeq ($(CONFIG_USB_LIBUSB),y)
>> +common-obj-$(CONFIG_XEN_BACKEND) += xen-usb.o
>> +endif
>> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
>> new file mode 100644
>> index 000..2570bd7
>> --- /dev/null
>> +++ b/hw/usb/xen-usb.c
>> @@ -0,0 +1,1120 @@
>> +/*
>> + *  xen paravirt usb device backend
>> + *
>> + *  (c) Juergen Gross 
>> + *
>> + *  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; under version 2 of the License.
>> + *
>> + *  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, see .
>> + *
>> + *  Contributions after 2012-01-13 are licensed under the terms of the
>> + *  GNU GPL, version 2 or (at your option) any later version.
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "qemu-common.h"
>> +#include "qemu/config-file.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/usb.h"
>> +#include "hw/xen/xen_backend.h"
>> +#include "monitor/qdev.h"
>> +#include "qapi/qmp/qbool.h"
>> +#include "qapi/qmp/qint.h"
>> +#include "qapi/qmp/qstring.h"
>> +#include "sys/user.h"
>> +
>> +#include 
>> +#include 
>> +
>> +#define TR(fmt, args...)\
>> +{   \
>> +struct timeval tv;  \
>> +\
>> +gettimeofday(&tv, NULL);\
>> +fprintf(stderr, "%8ld.%06ld xen-usb(%s):" fmt, tv.tv_sec,   \
>> +tv.tv_usec, __func__, ##args);  \
>> +}
>> +#define TR_REQ(fmt, args...) { if (tr_debug & 1) TR(fmt, ##args) }
>> +#define TR_BUS(fmt, args...) { if (tr_debug & 2) TR(fmt, ##args) }
>> +
>> +#define USBBACK_MAXPORTSUSBIF_PIPE_PORT_MASK
>> +#define USBBACK_DEVNAME_SIZE32
> 
> That does not seem to be used
>> +#define USB_DEV_ADDR_SIZE   128
>> +
>> +struct usbif_ctrlrequest {
>> +uint8_tbRequestType;
>> +uint8_tbRequest;
>> +uint16_t   wValue;
>> +uint16_t   wIndex;
>> +uint16_t   wLength;
>> +};
> 
> Would it make sense to mention that this is part of the ABI?

It is part of the USB hardware interface. I'll add a comment.

> And if so perhaps a pointer where this in the Xen code base?
> 
>> +
>> +struct usbif_isoc_descriptor {
>> +uint32_t   offset;
>> +uint32_t   length;
>> +uint32_t   actual_length;
>> +int32_tstatus;
>> +};
> 
> Ditto?

ISOC code is remove from this patch, so I'll address this later.

> 
>> +
>> +struct usbback_info;
>> +struct usbback_req;
>> +
>> +struct usbback_stub {
>> +USBDevice  *dev;
>> +USBPortport;
>> +unsigned   speed;
> 
> unsigned int?

Okay.

>> +bool   attached;
>> +QTAILQ_HEAD(submit_q_head, usbback_req) submit_q;
>> +};
>> +
>> +struct usbback_req {
>> +struct usbback_info  *usbif;
>> +struct usbback_stub  *stub;
>> +struct usbif_urb_request req;
>> +USBPacketpacket;
>> +
>> +unsigned int nr_buffer_segs; /* # of transfer_buffer 
>> segments */
>> +unsigned int nr_extra_segs;  /* # of iso_frame_desc 
>> segments  */
>> +
>> +QTAILQ_ENTRY(usbback_req) q;
>> +
>> +void *buffer;
>> +void *isoc_buffer;
>> +struct libusb_transfer   *xfer;
>> +};
>> +
>> +struct usbback_info {
>> +struct XenDevice xendev;  /* must

Re: [Xen-devel] [PATCH 0/1] ARM: Implement support for write-ctrlreg vm-events

[Corneliu ZUZU]

On 3/7/2016 2:38 PM, Andrew Cooper wrote:

On 07/03/16 09:12, Tamas K Lengyel wrote:



On Mon, Mar 7, 2016 at 9:22 AM, Corneliu ZUZU > wrote:


On 3/3/2016 4:10 PM, Corneliu ZUZU wrote:

Then,
QUESTIONS (FOR VM-EVENTS & ARM MAINTAINERS ESPECIALLY):

Q1) [...]

Q2) [...]

Q3) [...]

Q4) [...]


Hey all,

I have a question relating to this part of code @
vmx_update_guest_cr:

if ( paging_mode_hap(v->domain) )
{
/* Manage GUEST_CR3 when CR0.PE =0. */
uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
 CPU_BASED_CR3_STORE_EXITING);
v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
if ( !hvm_paging_enabled(v) &&
!vmx_unrestricted_guest(v) )
v->arch.hvm_vmx.exec_control |= cr3_ctls;

/* Trap CR3 updates if CR3 memory events are enabled. */
if ( v->domain->arch.monitor.write_ctrlreg_enabled &
 monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
v->arch.hvm_vmx.exec_control |=
CPU_BASED_CR3_LOAD_EXITING;

vmx_update_cpu_exec_control(v);
}

While trying to move the check for VM_EVENT_X86_CR3 to the
scheduling tail, a few questions came to my mind.

1). Tamas, Razvan, maybe you guys could clarify this. I noticed
this part of code is only executed if paging_mode_hap(v->domain).
Is EPT mandatory to monitor CR3 writes or is it just that when
shadow paging is enabled, CR3 r/w are unconditionally trapped?


EPT is not really required for CR3 monitoring, it just has been the 
case that vm_events have been only implemented for hap-enabled 
domains. AFAIK for non-hap case CR3 needs to be trapped 
unconditionally, yes.


Specifically, the shadow pagetable code needs to swap shadows when the 
guest switches cr3.



If the former is true, shouldn't we do a check like this in
vm_event_monitor_get_capabilities instead?


Yes, it should now, this code was just written before 
vm_event_monitor_get_capabilities was introduced and we haven't 
gotten around converting this check to it.



2). I was also wondering why CR3 load/stores are trapped if
paging is disabled for a domain.


Good question, I was wondering about that myself at some point but I 
haven't found an answer to it. Maybe some git archaeology can help 
determining when that was added and why ;)


Gen1 VT-x didn't support running a guest in non-paged mode.  Gen2 
introduced "unrestricted-guest" which works as intended, but Gen1 has 
to fake non-pagad mode using identity paging.  As a result, CR3 cannot 
be used as scratch space like it can in non-paged mode, and the guest 
must be prevented from moving CR3 away from the gfn set up by the 
domain builder in HVM_PARAM_IDENT_PT.


~Andrew


Nice, thanks a bunch.

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

Re: [Xen-devel] [PATCH v6 for Xen 4.7 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Jan Beulich]

>>> On 06.03.16 at 18:55,  wrote:
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -1054,6 +1054,10 @@ csched_dom_cntl(
>   * lock. Runq lock not needed anywhere in here. */
>  spin_lock_irqsave(&prv->lock, flags);
>  
> +if ( op->cmd == XEN_DOMCTL_SCHEDOP_putvcpuinfo ||
> + op->cmd == XEN_DOMCTL_SCHEDOP_getvcpuinfo )
> +return -EINVAL;
> +
>  if ( op->cmd == XEN_DOMCTL_SCHEDOP_getinfo )
>  {
>  op->u.credit.weight = sdom->weight;

Considering the rest of the code following where, I would - albeit
I'm not maintainer of this code - strongly suggest moving to
switch() in such cases, with the default case returning -EINVAL (or
maybe better -EOPNOTSUPP).

> @@ -1130,23 +1146,17 @@ rt_dom_cntl(
>  unsigned long flags;
>  int rc = 0;
>  
> +xen_domctl_schedparam_vcpu_t local_sched;
> +s_time_t period, budget;
> +uint32_t index = 0;
> +

There's a stray blank line left ahead of this addition.

>  switch ( op->cmd )
>  {
> -case XEN_DOMCTL_SCHEDOP_getinfo:
> -if ( d->max_vcpus > 0 )
> -{
> -spin_lock_irqsave(&prv->lock, flags);
> -svc = rt_vcpu(d->vcpu[0]);
> -op->u.rtds.period = svc->period / MICROSECS(1);
> -op->u.rtds.budget = svc->budget / MICROSECS(1);
> -spin_unlock_irqrestore(&prv->lock, flags);
> -}
> -else
> -{
> -/* If we don't have vcpus yet, let's just return the defaults. */
> -op->u.rtds.period = RTDS_DEFAULT_PERIOD;
> -op->u.rtds.budget = RTDS_DEFAULT_BUDGET;
> -}
> +case XEN_DOMCTL_SCHEDOP_getinfo: /* return the default parameters */
> +spin_lock_irqsave(&prv->lock, flags);
> +op->u.rtds.period = RTDS_DEFAULT_PERIOD / MICROSECS(1);
> +op->u.rtds.budget = RTDS_DEFAULT_BUDGET / MICROSECS(1);
> +spin_unlock_irqrestore(&prv->lock, flags);
>  break;

This alters the values returned when d->max_vcpus == 0 - while
this looks to be intentional, I think calling out such a bug fix in the
description is a must.

> @@ -1163,6 +1173,96 @@ rt_dom_cntl(
>  }
>  spin_unlock_irqrestore(&prv->lock, flags);
>  break;
> +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
> +if ( guest_handle_is_null(op->u.v.vcpus) )
> +{
> +rc = -EINVAL;

Perhaps rather -EFAULT? But then again - what is this check good for
(considering that it doesn't cover other obviously bad handle values)?

> +break;
> +}
> +while ( index < op->u.v.nr_vcpus )
> +{
> +if ( copy_from_guest_offset(&local_sched,
> +  op->u.v.vcpus, index, 1) )

Indentation.

> +{
> +rc = -EFAULT;
> +break;
> +}
> +if ( local_sched.vcpuid >= d->max_vcpus ||
> +  d->vcpu[local_sched.vcpuid] == NULL )

Again. And more below.

> +{
> +rc = -EINVAL;
> +break;
> +}
> +
> +spin_lock_irqsave(&prv->lock, flags);
> +svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> +local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
> +local_sched.s.rtds.period = svc->period / MICROSECS(1);
> +spin_unlock_irqrestore(&prv->lock, flags);
> +
> +if ( __copy_to_guest_offset(op->u.v.vcpus, index,
> +&local_sched, 1) )
> +{
> +rc = -EFAULT;
> +break;
> +}
> +if ( (++index > 0x3f) && hypercall_preempt_check() )
> +break;

So how is the caller going to be able to reliably read all vCPU-s'
information for a guest with more than 64 vCPU-s?

> +}
> +
> +if ( !rc && (op->u.v.nr_vcpus != index) )
> +op->u.v.nr_vcpus = index;

I don't think the right side of the && is really necessary / useful.

> +break;
> +case XEN_DOMCTL_SCHEDOP_putvcpuinfo:

When switch statements get large, please put blank lines between
individual case blocks.

> +if ( guest_handle_is_null(op->u.v.vcpus) )
> +{
> +rc = -EINVAL;
> +break;
> +}
> +while ( index < op->u.v.nr_vcpus )
> +{
> +if ( copy_from_guest_offset(&local_sched,
> +  op->u.v.vcpus, index, 1) )
> +{
> +rc = -EFAULT;
> +break;
> +}
> +if ( local_sched.vcpuid >= d->max_vcpus ||
> +  d->vcpu[local_sched.vcpuid] == NULL )
> +{
> +rc = -EINVAL;
> +break;
> +}
> +
> +period = MICROSECS(local_sched.s.rtds.period);
> +budget = MICROSECS(local_sched.s.rtds.budget);
> +if ( period > RTDS_MAX_PERIOD || budget < RTDS_MIN_BUDGET ||
> +  

Re: [Xen-devel] Behaviour when setting CPU_BASED_MONITOR_TRAP_FLAG in hvm_do_resume()

[Andrew Cooper]

On 06/03/16 13:35, Razvan Cojocaru wrote:
> Hello,
>
> Assuming I set v->arch.hvm_vmx.exec_control |=
> CPU_BASED_MONITOR_TRAP_FLAG; in hvm_do_resume(), would that cause a
> VMEXIT with EXIT_REASON_MONITOR_TRAP_FLAG _before_ the instruction at he
> current rIP runs, or _after_ it?
>
> A few tests I've ran suggest that the VMEXIT occurs _before_, i.e. the
> instruction is not running between setting the flag and the VMEXIT, but
> the actual code is a bit more involved and I might have just come across
> a corner case, so I thought it would be best to have official
> confirmation on the list.

Wow the SDM is opaque in its description of the monitor trap flag.

My reading of section 25.5.2 is that you will get a MTF exit on every
new instruction boundary, other than the rip pending at the vmentry,
which would give it fault semantics.

In the case of interacting with interrupts or traps, the trap/interrupt
action will occur before the MTF exit, and the exit will be on the
boundary starting the exception handler.


This would make it consistent with the other intercept semantics, where
even interception of software traps behave like faults.  (e.g. c/s 0747bc8)

~Andrew

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

Re: [Xen-devel] [PATCH] xen/mm: Fix page_list_* helpers to evaluate all their arguments

[Jan Beulich]

>>> On 05.03.16 at 17:52,  wrote:
> Doing this reveals that const-correctness of page_list_{next,prev}() is
> suspect, taking a const pointer and returning a non-const one.  It is left
> functioning as it did before, with an explicit typecast to remove constness.

I don't see anything suspect here: Retrieving the next or prev list
element doesn't alter the current one, so the input pointer can
legitimately be const, while the output one obviously shouldn't be.
Or else why don't you similarly consider page_list_{first,last}()
bogus in that same respect?

> +static inline bool_t
> +page_list_empty(const struct page_list_head *head)
> +{
> +return list_empty(head);
> +}

While I appreciate the conversion to bool_t, to be fully correct you
either need to use !! here, or switch list_empty() to bool_t too.

> +static inline struct page_info *
> +page_list_first(const struct page_list_head *head)
> +{
> +return list_first_entry(head, struct page_info, list);
> +}
> +static inline struct page_info *
> +page_list_last(const struct page_list_head *head)
> +{
> +return list_last_entry(head, struct page_info, list);
> +}
> +static inline struct page_info *
> +page_list_next(const struct page_info *page,
> +   const struct page_list_head *head)
> +{
> +return list_next_entry((struct page_info *)page, list);
> +}
> +static inline struct page_info *
> +page_list_prev(const struct page_info *page,
> +   const struct page_list_head *head)
> +{
> +return list_prev_entry((struct page_info *)page, list);
> +}

I'd suggest to avoid the explicit casts, by using
list_entry(page->list.next, struct page_info, list) and
list_entry(page->list.prev, struct page_info, list) respectively.

Jan


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

Re: [Xen-devel] [PATCH v2 3/3] xl: new "loglvl" command

[Fabio Fantoni]

Il 04/03/2016 17:48, Jan Beulich ha scritto:

This is pretty simplistic for now, but I'd rather have someone better
friends with the tools improve it (if desired).

Signed-off-by: Jan Beulich 

--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5958,6 +5958,26 @@ int libxl_send_debug_keys(libxl_ctx *ctx
  return 0;
  }
  
+int libxl_log_level(libxl_ctx *ctx, bool set, bool guest,

+int *lower_thresh, int *upper_thresh)
+{
+int ret;
+GC_INIT(ctx);
+if (set) {
+ret = xc_set_log_level(ctx->xch, guest, *lower_thresh, *upper_thresh);
+} else {
+ret = xc_get_log_level(ctx->xch, guest, lower_thresh, upper_thresh);
+}
+if ( ret < 0 ) {
+LOGE(ERROR, "%s %slog level",
+ set ? "setting" : "getting", guest ? "guest " : "");
+GC_FREE;
+return ERROR_FAIL;
+}
+GC_FREE;
+return 0;
+}
+
  libxl_xen_console_reader *
  libxl_xen_console_read_start(libxl_ctx *ctx, int clear)
  {
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -1765,6 +1765,8 @@ int libxl_send_trigger(libxl_ctx *ctx, u
 libxl_trigger trigger, uint32_t vcpuid);
  int libxl_send_sysrq(libxl_ctx *ctx, uint32_t domid, char sysrq);
  int libxl_send_debug_keys(libxl_ctx *ctx, char *keys);
+int libxl_log_level(libxl_ctx *ctx, bool set, bool guest,
+int *lower_thresh, int *upper_thresh);
  
  typedef struct libxl__xen_console_reader libxl_xen_console_reader;
  
--- a/tools/libxl/xl.h

+++ b/tools/libxl/xl.h
@@ -81,6 +81,7 @@ int main_trigger(int argc, char **argv);
  int main_sysrq(int argc, char **argv);
  int main_debug_keys(int argc, char **argv);
  int main_dmesg(int argc, char **argv);
+int main_loglvl(int argc, char **argv);
  int main_top(int argc, char **argv);
  int main_networkattach(int argc, char **argv);
  int main_networklist(int argc, char **argv);
@@ -209,6 +210,8 @@ extern void printf_info_sexp(int domid,
  #define XL_GLOBAL_CONFIG XEN_CONFIG_DIR "/xl.conf"
  #define XL_LOCK_FILE XEN_LOCK_DIR "/xl"
  
+#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))

+
  #endif /* XL_H */
  
  /*

--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -6469,6 +6469,84 @@ int main_debug_keys(int argc, char **arg
  return 0;
  }
  
+static const struct {

+int level;
+char string[8];
+} loglvls[] = {
+{ 0, "none" },
+{ 1, "error" },
+{ 2, "warning" },
+{ 3, "info" },
+{ 4, "all" },
+{ 4, "debug" },
+};


double "4" for both all and debug seems strange to me, is it right?


+
+static int parse_loglvl(char **parg)
+{
+unsigned int i;
+
+for (i = 0; i < ARRAY_SIZE(loglvls); ++i) {
+size_t l = strlen(loglvls[i].string);
+
+if (!strncmp(*parg, loglvls[i].string, l)) {
+*parg += l;
+return loglvls[i].level;
+}
+}
+
+return -1;
+}
+
+static const char *format_loglvl(int loglvl)
+{
+unsigned int i;
+
+for (i = 0; i < ARRAY_SIZE(loglvls); ++i) {
+if (loglvl == loglvls[i].level)
+return loglvls[i].string;
+}
+
+return "";
+}
+
+int main_loglvl(int argc, char **argv)
+{
+static const struct option opts[] = {
+{"guest", 0, 0, 'g'},
+{"set", 0, 0, 's'},
+COMMON_LONG_OPTS
+};
+int opt, lower_thresh = -1, upper_thresh = -1;
+bool guest = false, set = false;
+
+SWITCH_FOREACH_OPT(opt, "gs:", opts, "loglvl", 0) {
+case 'g':
+guest = true;
+break;
+case 's':
+if (*optarg != '/')
+lower_thresh = parse_loglvl(&optarg);
+if (*optarg == '/') {
+++optarg;
+upper_thresh = parse_loglvl(&optarg);
+}
+set = true;
+break;
+}
+
+if (libxl_log_level(ctx, set, guest, &lower_thresh, &upper_thresh)) {
+fprintf(stderr, "cannot %s %s log level\n",
+set ? "set" : "get", guest ? "guest" : "host");
+return 1;
+}
+
+if (!set)
+printf("%s log levels: %s/%s\n", guest ? "guest" : "host",
+   format_loglvl(lower_thresh), format_loglvl(upper_thresh));
+
+return 0;
+}
+
  int main_dmesg(int argc, char **argv)
  {
  unsigned int clear = 0;
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -309,6 +309,13 @@ struct cmd_spec cmd_table[] = {
"[-c]",
"  -cClear dmesg buffer as well as printing it",
  },
+{ "loglvl",
+  &main_loglvl, 0, 1,
+  "Manage Xen log levels",
+  "[-g] [-s=[LOWER][/UPPER]]",
+  "-g, --guest act on guest log level\n"
+  "-s [LOWER][/UPPER], --set=[LOWER][/UPPER]   set new log level\n"
+},
  { "top",
&main_top, 0, 0,
"Monitor a host and the domains in real time",






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

Re: [Xen-devel] [PATCH v2 3/3] xl: new "loglvl" command

[Jan Beulich]

>>> On 07.03.16 at 14:20,  wrote:
> Il 04/03/2016 17:48, Jan Beulich ha scritto:
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -6469,6 +6469,84 @@ int main_debug_keys(int argc, char **arg
>>   return 0;
>>   }
>>   
>> +static const struct {
>> +int level;
>> +char string[8];
>> +} loglvls[] = {
>> +{ 0, "none" },
>> +{ 1, "error" },
>> +{ 2, "warning" },
>> +{ 3, "info" },
>> +{ 4, "all" },
>> +{ 4, "debug" },
>> +};
> 
> double "4" for both all and debug seems strange to me, is it right?

Yes, it is both right and intentional.

Jan


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

Re: [Xen-devel] [BUG] Xen BUG at irq.c:1705 after [VT-D]d1:PCIe: unmap

[Jan Beulich]

>>> On 05.03.16 at 13:22,  wrote:
> This has happened in 2 out of four recent shutdowns of a specific domU
> "garbo.hvm", domid=1.
> --- domU specs (no tmem in cmdline arg to domU): -
> Linux garbo 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt20-1+deb8u4
> (2016-02-29) x86_64
> garbo:~# cat /proc/cmdline
> BOOT_IMAGE=/boot/vmlinuz-3.16.0-4-amd64
> root=UUID=042054cf-2a70-46de-bd90-c712fbedbf81 ro intel_iommu=on 3
> console=ttyS0 console=vga
> 
> serial log:
> (XEN) [2016-03-04 23:38:48] tmem: allocating persistent-private tmem
> pool for domid=3...<2>pool_id=0
> (XEN) [2016-03-04 23:39:12] grant_table.c:1491:d2v1 Expanding dom (2)
> grant table from (5) to (6) frames.
> (XEN) [2016-03-04 23:40:28] [VT-D]d1:PCIe: unmap :08:00.0
> (XEN) [2016-03-04 23:40:28] [VT-D]d0:PCIe: map :08:00.0
> (XEN) [2016-03-04 23:40:30] tmem: flushing tmem pools for domid=1
> (XEN) [2016-03-04 23:40:30] Destroying persistent-private tmem pool
> domid=1 pool_id=0
> (XEN) [2016-03-04 23:40:30] Xen BUG at irq.c:1705
> (XEN) [2016-03-04 23:40:30] [ Xen-4.6.1  x86_64  debug=y 
> Tainted:C ]

Plain Xen 4.6.1 doesn't have any BUG() or BUG_ON() at that line,
so you will need to tell us which nearby one it is. And of course
the question then also arises whether plain 4.6.1 (or even better
4.7-unstable) would also surface this problem.

> I do have qemu-dm logs for the domain, but I'm not certain which log
> goes with which run. Attaching what I THINK are correct. My timezone is
> GMT+1, xen seems to be at GMT, so qemu-dm with file-system timestamp Mar
> 5 00:40 ought to be right.

Not how the domain ID is being logged at the very top of the file,
to aid making such association. (But yes, from the looks of it you
picked the right one.)

Jan


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

Re: [Xen-devel] [PATCH v2 00/16] Scheduling related tracing improvements

[Dario Faggioli]

On Mon, 2016-03-07 at 11:27 +, Wei Liu wrote:
> On Mon, Mar 07, 2016 at 04:21:22AM -0700, Jan Beulich wrote:
> > > > > On 07.03.16 at 11:54,  wrote:
> > > 
> > Well, if these were urgent I'd say yes. But I don't think they are,
> > so I'd leave it to either Konrad (who has basically asked for what
> > you offer) if he wants to deal with it, of wait for Ian's return.
> > 
> It's not urgent, but I would like to avoid wasting any test cycle. No
> matter how innocent a patch looks, there is always risk that it
> breaks
> something.
> 
> Konrad, your call.
> 
In any case, and as far as these patches are concerned, here it is:

 git://xenbits.xen.org/people/dariof/xen.git  tracing/sched-events-improvements

 
http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/tracing/sched-events-improvements

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

[Xen-devel] [qemu-mainline baseline-only test] 44229: tolerable FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 44229 qemu-mainline real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/44229/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl  19 guest-start/debian.repeatfail   like 44226
 test-amd64-amd64-amd64-pvgrub 10 guest-start  fail  like 44226
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1 fail like 44226

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-midway   12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  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:
 qemuu1464ad45cd6cdeb0b5c1a54d3d3791396e47e52f
baseline version:
 qemuu3c0f12df65da872d5fbccae469f2cb21ed1c03b7

Last test of basis44226  2016-03-06 07:25:59 Z1 days
Testing same since44229  2016-03-07 06:25:10 Z0 days1 attempts


People who touched revisions under test:
  Daniel P. Berrange 
  Eric Blake 
  Kashyap Chamarthy 
  Markus Armbruster 
  Peter Maydell 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  fail
 test-armhf-armhf-xl  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpa

[Xen-devel] [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE

[Andrew Cooper]

The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS and PEBS
UNAVAIL bits.

Some 64bit Windows include IA32_MISC_ENABLE in the set of items checked by
PatchGuard, and will suffer a BSOD 0x109 CRITICAL_STRUCTURE_CORRUPTION if the
contents change on migrate.

The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Boris Ostrovsky 

This appears to have been broken since the vPMU code was first introduced.  It
appears to have lurked this log due to a hole (now fixed) in XenServers
upgrade testing.  The BSODs occur ~80% of the time on Win 8 thru 10, but
appear very hard to provoke on Windows 7.

This MSR still leaks mostly host state through into the guest.  Therefore
migration of windows is still liable to crash if moving between two
non-identical servers.  I need to get proper MSR levelling sorted before this
issue can be resolved fully.
---
 xen/arch/x86/cpu/vpmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 237b5ff..2f9ddf6 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
 return ret;
 
  nop:
-if ( !is_write )
+if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
 *msr_content = 0;
 
 return 0;
-- 
2.1.4


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

Re: [Xen-devel] [Xen-users] Garbled screen after RAM Scrub on boot

[George Dunlap]

Jan / Konrad,

Have you guys ever seen something like this before?  Any ideas how to proceed?

Thanks
 -George

On Tue, Feb 23, 2016 at 1:37 PM, Francis Greaves  wrote:
> Dear All
> I am using Centos 7 with Xen 4.6 on a Dell Poweredge T430 with (from lspci)
> Matrox Electronics Systems Ltd. G200eR2 (rev 01) VGA
>
> When the machine boots, after the 'Scrubbing Free RAM' message, I get a
> screen filled with little white squares until the login prompt, so I cannot
> see what is happening as the machine boots. Also there is nothing on the
> screen when I reboot or shutdown.
>
> My /etc/default/grub is
>
> GRUB_DISTRIBUTOR="$(sed 's, release .*$,,g' /etc/system-release)"
> GRUB_DEFAULT=saved
> GRUB_DISABLE_SUBMENU=true
> GRUB_CMDLINE_LINUX="crashkernel=auto rhgb intremap=no_x2apic_optout"
> GRUB_CMDLINE_XEN_DEFAULT="dom0_mem=13312M,max:14336M dom0_max_vcpus=6
> dom0_vcpus_pin"
> GRUB_GFXMODE=1024x768
> GRUB_GFXPAYLOAD_LINUX=keep
> GRUB_CMDLINE_LINUX_XEN_REPLACE_DEFAULT="console=hvc0 earlyprintk=xen
> nomodeset"
>
> I have tried setting (for a 1024x768 resolution) vga=792 in the
> GRUB_CMDLINE_LINUX and commenting out GRUB_GFXMODE and
> GRUB_GFXPAYLOAD_LINUX, but this makes no difference.
> I have also tried with no settings for vga at all, and with and without
> 'rhgb'
> I have even tried bootscrub=false as suggested by the CentOS-virt mailing
> list, but that does not work either. They suggested I asked you folk.
>
> What am I doing wrong?
>
> Regards
> Francis
>
>
> ___
> Xen-users mailing list
> xen-us...@lists.xen.org
> http://lists.xen.org/xen-users

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

[Xen-devel] [distros-debian-sid test] 44230: trouble: blocked/broken/pass

[Platform Team regression test user]

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-armhf   3 host-install(3) broken REGR. vs. 44198
 build-armhf-pvops 3 host-install(3) broken REGR. vs. 44198
 test-amd64-i386-amd64-sid-netboot-pygrub 3 host-install(3) broken REGR. vs. 
44198
 test-amd64-i386-i386-sid-netboot-pvgrub 3 host-install(3) broken REGR. vs. 
44198
 test-amd64-amd64-i386-sid-netboot-pygrub 3 host-install(3) broken REGR. vs. 
44198
 test-amd64-amd64-amd64-sid-netboot-pvgrub 3 host-install(3) broken REGR. vs. 
44198

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-armhf-sid-netboot-pygrub  1 build-check(1)blocked n/a

baseline version:
 flight   44198

jobs:
 build-amd64  pass
 build-armhf  broken  
 build-i386   pass
 build-amd64-pvopspass
 build-armhf-pvopsbroken  
 build-i386-pvops pass
 test-amd64-amd64-amd64-sid-netboot-pvgrubbroken  
 test-amd64-i386-i386-sid-netboot-pvgrub  broken  
 test-amd64-i386-amd64-sid-netboot-pygrub broken  
 test-armhf-armhf-armhf-sid-netboot-pygrubblocked 
 test-amd64-amd64-i386-sid-netboot-pygrub broken  



sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images

Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs

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


Push not applicable.


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

Re: [Xen-devel] [Xen-users] Garbled screen after RAM Scrub on boot

[Andrew Cooper]

On 07/03/16 14:33, George Dunlap wrote:
> Jan / Konrad,
>
> Have you guys ever seen something like this before?  Any ideas how to proceed?
>
> Thanks
>  -George

Sounds like the scrub is writing zeroes over the graphics framebuffer. 
This probably means that the firmware is using a region for the
framebuffer which isn't marked as reserved in the E820 region.

~Andrew

>
> On Tue, Feb 23, 2016 at 1:37 PM, Francis Greaves  wrote:
>> Dear All
>> I am using Centos 7 with Xen 4.6 on a Dell Poweredge T430 with (from lspci)
>> Matrox Electronics Systems Ltd. G200eR2 (rev 01) VGA
>>
>> When the machine boots, after the 'Scrubbing Free RAM' message, I get a
>> screen filled with little white squares until the login prompt, so I cannot
>> see what is happening as the machine boots. Also there is nothing on the
>> screen when I reboot or shutdown.
>>
>> My /etc/default/grub is
>>
>> GRUB_DISTRIBUTOR="$(sed 's, release .*$,,g' /etc/system-release)"
>> GRUB_DEFAULT=saved
>> GRUB_DISABLE_SUBMENU=true
>> GRUB_CMDLINE_LINUX="crashkernel=auto rhgb intremap=no_x2apic_optout"
>> GRUB_CMDLINE_XEN_DEFAULT="dom0_mem=13312M,max:14336M dom0_max_vcpus=6
>> dom0_vcpus_pin"
>> GRUB_GFXMODE=1024x768
>> GRUB_GFXPAYLOAD_LINUX=keep
>> GRUB_CMDLINE_LINUX_XEN_REPLACE_DEFAULT="console=hvc0 earlyprintk=xen
>> nomodeset"
>>
>> I have tried setting (for a 1024x768 resolution) vga=792 in the
>> GRUB_CMDLINE_LINUX and commenting out GRUB_GFXMODE and
>> GRUB_GFXPAYLOAD_LINUX, but this makes no difference.
>> I have also tried with no settings for vga at all, and with and without
>> 'rhgb'
>> I have even tried bootscrub=false as suggested by the CentOS-virt mailing
>> list, but that does not work either. They suggested I asked you folk.
>>
>> What am I doing wrong?
>>
>> Regards
>> Francis
>>
>>
>> ___
>> Xen-users mailing list
>> xen-us...@lists.xen.org
>> http://lists.xen.org/xen-users
> ___
> 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 v2 1/3] console: allow log level threshold adjustments

[Konrad Rzeszutek Wilk]

On Mon, Mar 7, 2016 at 5:44 AM, Jan Beulich  wrote:
 On 04.03.16 at 21:55,  wrote:
>>> +case XEN_SYSCTL_LOGLVL_set:
>>> +if ( (op->host.lower_thresh >= 0 && op->host.upper_thresh >= 0 &&
>>> +  op->host.lower_thresh > op->host.upper_thresh) ||
>>> + (op->guest.lower_thresh >= 0 && op->guest.upper_thresh >= 0 &&
>>> +  op->guest.lower_thresh > op->guest.upper_thresh) )
>>> +return -EINVAL;
>>> +
>>> +do_loglvl_op(&op->host, &xenlog_lower_thresh,
>>> + &xenlog_upper_thresh, "standard");
>>
>>
>> The keyboard and the sysctl both allow the user to go beyound the XENLOG_
>> values we have. That is you could set the lower and upper threshold to be
>> at 9 (or more) say. It will have the same effect as XENLOG_DEBUG (which is
>> 4)
>> as printk_prefix_check seems to have a simple < check.
>>
>> But perhaps to be correct only accept only proper values? Not allow
>> the system admin to set the level to say 31415?
>
> Since there's no bad side effect from doing so I opted for not
> adding respective extra checks, keeping the code easier to read.
>

Fair enough. Could you perhaps just add that in the commit description?

Also I noticed that this patch is missing an XSM check in flask_sysctl
- could that be added please?
> Jan
>
>
> ___
> 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] xen/vPMU: Do not clobber IA32_MISC_ENABLE

[Boris Ostrovsky]

On 03/07/2016 09:27 AM, Andrew Cooper wrote:

The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS and PEBS
UNAVAIL bits.

Some 64bit Windows include IA32_MISC_ENABLE in the set of items checked by
PatchGuard, and will suffer a BSOD 0x109 CRITICAL_STRUCTURE_CORRUPTION if the
contents change on migrate.

The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Boris Ostrovsky 

This appears to have been broken since the vPMU code was first introduced.  It
appears to have lurked this log due to a hole (now fixed) in XenServers
upgrade testing.  The BSODs occur ~80% of the time on Win 8 thru 10, but
appear very hard to provoke on Windows 7.

This MSR still leaks mostly host state through into the guest.  Therefore
migration of windows is still liable to crash if moving between two
non-identical servers.  I need to get proper MSR levelling sorted before this
issue can be resolved fully.
---
  xen/arch/x86/cpu/vpmu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 237b5ff..2f9ddf6 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
  return ret;
  
   nop:

-if ( !is_write )
+if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
  *msr_content = 0;
  
  return 0;



This is Intel-specific register so the test should really be happening 
in vpmu_intel.c. Of course then you'd need to always dereference 
vcpu_vpmu() and possibly add more checks to read/write ops (to mirror 
the one at the top of vpmu_do_msr()).


So maybe at least have the vendor check too??

-boris

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

Re: [Xen-devel] Behaviour when setting CPU_BASED_MONITOR_TRAP_FLAG in hvm_do_resume()

[Razvan Cojocaru]

On 03/07/2016 03:13 PM, Andrew Cooper wrote:
> On 06/03/16 13:35, Razvan Cojocaru wrote:
>> Hello,
>>
>> Assuming I set v->arch.hvm_vmx.exec_control |=
>> CPU_BASED_MONITOR_TRAP_FLAG; in hvm_do_resume(), would that cause a
>> VMEXIT with EXIT_REASON_MONITOR_TRAP_FLAG _before_ the instruction at he
>> current rIP runs, or _after_ it?
>>
>> A few tests I've ran suggest that the VMEXIT occurs _before_, i.e. the
>> instruction is not running between setting the flag and the VMEXIT, but
>> the actual code is a bit more involved and I might have just come across
>> a corner case, so I thought it would be best to have official
>> confirmation on the list.
> 
> Wow the SDM is opaque in its description of the monitor trap flag.
> 
> My reading of section 25.5.2 is that you will get a MTF exit on every
> new instruction boundary, other than the rip pending at the vmentry,
> which would give it fault semantics.
> 
> In the case of interacting with interrupts or traps, the trap/interrupt
> action will occur before the MTF exit, and the exit will be on the
> boundary starting the exception handler.
> 
> 
> This would make it consistent with the other intercept semantics, where
> even interception of software traps behave like faults.  (e.g. c/s 0747bc8)

The issue turned out to be that if _only_ the MTF is set but not
v->arch.hvm_vcpu.single_step, vmx_intr_assist() doesn't return early:

221 void vmx_intr_assist(void)
222 {
223 struct hvm_intack intack;
224 struct vcpu *v = current;
225 unsigned int tpr_threshold = 0;
226 enum hvm_intblk intblk;
227 int pt_vector = -1;
228
229 /* Block event injection when single step with MTF. */
230 if ( unlikely(v->arch.hvm_vcpu.single_step) )
231 {
232 v->arch.hvm_vmx.exec_control |= CPU_BASED_MONITOR_TRAP_FLAG;
233 vmx_update_cpu_exec_control(v);
234 return;
235 }

i.e. even if MTF is already set, only v->arch.hvm_vcpu.single_step counts.


Thanks,
Razvan

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

Re: [Xen-devel] Xen domUs seem unable to use one qxl memory bar

[Fabio Fantoni]

Il 24/02/2016 16:44, Fabio Fantoni ha scritto:
Today I was trying a newer version of qxl driver on windows 10 domU 
(on dom0 with xen 4.6) and I got a windows blue screen about the 
updated driver.

On latest xl dmesg line I found this error which I suppose is related:
(XEN) memory.c:161:d0v0 Could not allocate order=18 extent: id=53 
memflags=0 (0 of 1)

Checking the few commits added in the latest update I found this one:
https://cgit.freedesktop.org/~teuf/qxl-wddm-dod/commit/?id=5df8c4f318d808a8381a948da3ce15b5d4aa6682 

Here it starts to use the second qxl memory bar which was unused 
before in windows qxl drivers FWIK.
This additional memory bar is not present in stdvga and if I remember 
correctly when I started to try qxl vga some years ago I was not able 
to boot the domUs at all for one or more problem about memory, solved 
by Jan Beulich and/or Anthony Perard.
After such fixes qxl was usable, now I use it even in production 
windows domUs with monitor resolution up to 1920x1080.


Can someone tell me if the second qxl memory bar is really "unusable" 
or something, please?

If yes how can it be solved?


Ping...can someone help me please?
I did other test in a testing system with xen 4.6 from git (based on 
commit 842e19d951c04c99c27a0fa2bca3d1e677a3) and qemu from qemu-xen 
master (based on commit 316a862e5534249a6e6d876b4e203342d3fb870e).
In this case gave me different xl dmesg output (I suppose for debug 
feature of new win pv driver), log in attachments.


If you need more information/tests tell me and I'll post them.

Thanks for any reply and sorry for my bad english.

48D4830245C8B48
(d6) XEN|BUGCHECK: - Parameter[4282] = 894838246C8B482E
(d6) XEN|BUGCHECK: - Parameter[4283] = 8B480200D7B4
(d6) XEN|BUGCHECK: - Parameter[4284] = 08D7848948402474
(d6) XEN|BUGCHECK: - Parameter[4285] = 5F20C4834802
(d6) XEN|BUGCHECK: - Parameter[4286] = 245C8948CCC3
(d6) XEN|BUGCHECK: - Parameter[4287] = 8B4830EC83485708
(d6) XEN|BUGCHECK: - Parameter[4288] = 48DA8B480A8B48C1
(d6) XEN|BUGCHECK: - Parameter[4289] = 010842C60675C985
(d6) XEN|BUGCHECK: - Parameter[4290] = 8D4C000842C66BEB
(d6) XEN|BUGCHECK: - Parameter[4291] = 0001BA502444
(d6) XEN|BUGCHECK: - Parameter[4292] = C085009890FF
(d6) XEN|BUGCHECK: - Parameter[4293] = 485024448B485578
(d6) XEN|BUGCHECK: - Parameter[4294] = 482024448D4C0B8B
(d6) XEN|BUGCHECK: - Parameter[4295] = 850850FF5824548D
(d6) XEN|BUGCHECK: - Parameter[4296] = 448B48FF333C78C0
(d6) XEN|BUGCHECK: - Parameter[4297] = 4858244C8B482024
(d6) XEN|BUGCHECK: - Parameter[4298] = 448D4C0048246483
(d6) XEN|BUGCHECK: - Parameter[4299] = 3D0850FFD78B4824
(d6) XEN|BUGCHECK: - Parameter[4300] = C0850C74C01E0339
(d6) XEN|BUGCHECK: - Parameter[4301] = 0148247C83481578
(d6) XEN|BUGCHECK: - Parameter[4302] = 7201FF83C7FF0B77
(d6) XEN|BUGCHECK: - Parameter[4303] = 48C033010843C6CC
(d6) XEN|BUGCHECK: - Parameter[4304] = 30C4834840245C8B
(d6) XEN|BUGCHECK: - Parameter[4305] = 72013A83C35F
(d6) XEN|BUGCHECK: - Parameter[4306] = 83C3C01E0304B806
(d6) XEN|BUGCHECK: - Parameter[4307] = 0305B8067201047A
(d6) XEN|BUGCHECK: - Parameter[4308] = 000150BA83C3C01E
(d6) XEN|BUGCHECK: - Parameter[4309] = 1E0348B806740100
(d6) XEN|BUGCHECK: - Parameter[4310] = 00FFB90C428BC3C0
(d6) XEN|BUGCHECK: - Parameter[4311] = 3B0A7602F883
(d6) XEN|BUGCHECK: - Parameter[4312] = C01E0306B80674C1
(d6) XEN|BUGCHECK: - Parameter[4313] = 7602F88314428BC3
(d6) XEN|BUGCHECK: - Parameter[4314] = 2C428BEE75C13B04
(d6) XEN|BUGCHECK: - Parameter[4315] = 74C0850A7403F883
(d6) XEN|BUGCHECK: - Parameter[4316] = 33C3C01E0310B806
(d6) XEN|BUGCHECK: - Parameter[4317] = 01047A83C3C0
(d6) XEN|BUGCHECK: - Parameter[4318] = C3C01E0310B80674
(d6) XEN|BUGCHECK: - Parameter[4319] = 047403F88320428B
(d6) XEN|BUGCHECK: - Parameter[4320] = 01247A83EE75C085
(d6) XEN|BUGCHECK: - Parameter[4321] = 15E8831C428BE875
(d6) XEN|BUGCHECK: - Parameter[4322] = 1E031025C01BD8F7
(d6) XEN|BUGCHECK: - Parameter[4323] = 48C48B48C3C0
(d6) XEN|BUGCHECK: - Parameter[4324] = 4810688948085889
(d6) XEN|BUGCHECK: - Parameter[4325] = 4120788948187089
(d6) XEN|BUGCHECK: - Parameter[4326] = FA8B4860EC834856
(d6) XEN|BUGCHECK: - Parameter[4327] = 107F83D98B48D233
(d6) XEN|BUGCHECK: - Parameter[4328] = C00DB80A7304
(d6) XEN|BUGCHECK: - Parameter[4329] = 0001BD0121E9
(d6) XEN|BUGCHECK: - Parameter[4330] = 0160A939
(d6) XEN|BUGCHECK: - Parameter[4331] = 078B010E8F0F
(d6) XEN|BUGCHECK: - Parameter[4332] = 8BC90348800C8D48
(d6) XEN|BUGCHECK: - Parameter[4333] = 844001A0CB84
(d6) XEN|BUGCHECK: - Parameter[4334] = A800F5850FC5
(d6) XEN|BUGCHECK: - Parameter[4335] = 4800E9840F04
(d6) XEN|BUGCHECK: - Parameter[4336] = 8B01A8CB9489
(d6) XEN|BUGCHECK: - Parameter[4337] = C90348800C8D4807
(d6) XEN|BUGCHECK: - Parameter[4338] = 01B0CB948948
(d6) XEN|BUGCHECK: - Parameter[4339] = 1074C5844018478B
(d6) XEN|BUGCHECK: - Parameter[4340] = 8D4805C08348078B
(d6) XEN|BUGCHECK: - Parameter[4341] = C32C8BC003488004
(d6) XEN

[Xen-devel] [PATCH v2] xen/mm: Fix page_list_* helpers to evaluate all their arguments

[Andrew Cooper]

If an architecture does not provide a custom page_list_entry, default
page_list_* helpers are provided, wrapping list_head as an underlying type for
page_list_head.

The two declarations of the page_list_* helpers differ between defines and
static inline functions, where the defines discard some of their parameters.

This causes a compilation failure if CONFIG_BIGMEM and debug=n in p2m-pod.c:

  p2m-pod.c: In function ‘p2m_pod_cache_add’:
  p2m-pod.c:72:20: error: unused variable ‘d’ [-Werror=unused-variable]
   struct domain *d = p2m->domain;
  ^
  cc1: all warnings being treated as errors

because the use of d outside of the !NDEBUG section doesn't get evaluated as a
parameter by page_list_del().

Fix this by turning all #defines into static inline functions, so all
parameters are evaluated even if they are not used.

While editing this area, correct the return type of page_list_empty from int
to bool_t.

Reported-by: Doug Goldstein 
Signed-off-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
---
CC: Jan Beulich 
CC: Tim Deegan 
CC: George Dunlap 

v2: Remove explicit casts, and missing !! in page_list_empty()
---
 xen/include/xen/mm.h | 95 
 1 file changed, 74 insertions(+), 21 deletions(-)

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index a795dd6..8600cf6 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -220,7 +220,7 @@ struct page_list_head
 # define INIT_PAGE_LIST_HEAD(head) ((head)->tail = (head)->next = NULL)
 # define INIT_PAGE_LIST_ENTRY(ent) ((ent)->prev = (ent)->next = PAGE_LIST_NULL)
 
-static inline int
+static inline bool_t
 page_list_empty(const struct page_list_head *head)
 {
 return !head->next;
@@ -392,31 +392,84 @@ page_list_splice(struct page_list_head *list, struct 
page_list_head *head)
 # define PAGE_LIST_HEAD  LIST_HEAD
 # define INIT_PAGE_LIST_HEAD INIT_LIST_HEAD
 # define INIT_PAGE_LIST_ENTRYINIT_LIST_HEAD
-# define page_list_empty list_empty
-# define page_list_first(hd) \
-list_first_entry(hd, struct page_info, list)
-# define page_list_last(hd)  \
-list_last_entry(hd, struct page_info, list)
-# define page_list_next(pg, hd)  list_next_entry(pg, list)
-# define page_list_prev(pg, hd)  list_prev_entry(pg, list)
-# define page_list_add(pg, hd)   list_add(&(pg)->list, hd)
-# define page_list_add_tail(pg, hd)  list_add_tail(&(pg)->list, hd)
-# define page_list_del(pg, hd)   list_del(&(pg)->list)
-# define page_list_del2(pg, hd1, hd2)list_del(&(pg)->list)
-# define page_list_remove_head(hd)   (!page_list_empty(hd) ? \
-({ \
-struct page_info *__pg = page_list_first(hd); \
-list_del(&__pg->list); \
-__pg; \
-}) : NULL)
-# define page_list_move(dst, src)(!list_empty(src) ? \
-list_replace_init(src, dst) : (void)0)
+
+static inline bool_t
+page_list_empty(const struct page_list_head *head)
+{
+return !!list_empty(head);
+}
+static inline struct page_info *
+page_list_first(const struct page_list_head *head)
+{
+return list_first_entry(head, struct page_info, list);
+}
+static inline struct page_info *
+page_list_last(const struct page_list_head *head)
+{
+return list_last_entry(head, struct page_info, list);
+}
+static inline struct page_info *
+page_list_next(const struct page_info *page,
+   const struct page_list_head *head)
+{
+return list_entry(page->list.next, struct page_info, list);
+}
+static inline struct page_info *
+page_list_prev(const struct page_info *page,
+   const struct page_list_head *head)
+{
+return list_entry(page->list.prev, struct page_info, list);
+}
+static inline void
+page_list_add(struct page_info *page, struct page_list_head *head)
+{
+list_add(&page->list, head);
+}
+static inline void
+page_list_add_tail(struct page_info *page, struct page_list_head *head)
+{
+list_add_tail(&page->list, head);
+}
+static inline void
+page_list_del(struct page_info *page, struct page_list_head *head)
+{
+list_del(&page->list);
+}
+static inline void
+page_list_del2(struct page_info *page, struct page_list_head *head1,
+   struct page_list_head *head2)
+{
+list_del(&page->list);
+}
+static inline struct page_info *
+page_list_remove_head(struct page_list_head *head)
+{
+struct page_info *pg;
+
+if ( page_list_empty(head) )
+return NULL;
+
+pg = page_list_first(head);
+list_del(&pg->list);
+return pg;
+}
+static inline void
+page_list_move(struct page_list_head *dst, struct page_list_head *src)
+{
+if ( !list_empty(src) )
+list_replace_init(src, dst);
+}
+static inline void
+page_list_splice(struct page_list_head *list, struct page_list_head *head)
+{
+list_splice(list, head);
+}
+
 # define page_list_for_each(pos, head)   list_for_each_entry(pos, head, list)
 # define page_list_f

Re: [Xen-devel] [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE

[Andrew Cooper]

On 07/03/16 14:45, Boris Ostrovsky wrote:
> On 03/07/2016 09:27 AM, Andrew Cooper wrote:
>> The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
>> vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS
>> and PEBS
>> UNAVAIL bits.
>>
>> Some 64bit Windows include IA32_MISC_ENABLE in the set of items
>> checked by
>> PatchGuard, and will suffer a BSOD 0x109
>> CRITICAL_STRUCTURE_CORRUPTION if the
>> contents change on migrate.
>>
>> The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.
>>
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Jan Beulich 
>> CC: Boris Ostrovsky 
>>
>> This appears to have been broken since the vPMU code was first
>> introduced.  It
>> appears to have lurked this log due to a hole (now fixed) in XenServers
>> upgrade testing.  The BSODs occur ~80% of the time on Win 8 thru 10, but
>> appear very hard to provoke on Windows 7.
>>
>> This MSR still leaks mostly host state through into the guest. 
>> Therefore
>> migration of windows is still liable to crash if moving between two
>> non-identical servers.  I need to get proper MSR levelling sorted
>> before this
>> issue can be resolved fully.
>> ---
>>   xen/arch/x86/cpu/vpmu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
>> index 237b5ff..2f9ddf6 100644
>> --- a/xen/arch/x86/cpu/vpmu.c
>> +++ b/xen/arch/x86/cpu/vpmu.c
>> @@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t
>> *msr_content,
>>   return ret;
>>  nop:
>> -if ( !is_write )
>> +if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
>>   *msr_content = 0;
>> return 0;
>
>
> This is Intel-specific register so the test should really be happening
> in vpmu_intel.c. Of course then you'd need to always dereference
> vcpu_vpmu() and possibly add more checks to read/write ops (to mirror
> the one at the top of vpmu_do_msr()).
>
> So maybe at least have the vendor check too??

Strictly speaking, if we were to do a vendor check, it should be a guest
vendor check, not a host vendor check.

OTOH, we won't get here on a non-Intel host system, and emulating a
cross-vendor vPMU is going to end in disaster.  I personally don't think
its worth it.

~Andrew

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

[Xen-devel] [PATCH v4] xen/errno: Reduce complexity of inclusion

[Andrew Cooper]

The inclusion rules conditions for errno.h were unnecesserily complicated, and
required the includer to jump through hoops if they wished to avoid getting
multiple namespaces worth of constants.

Simply the logic, and document what is going on.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Tim Deegan 
CC: Doug Goldstein 

v3:
 * Reinstate magic documentation comments
 * Provide assembly-suitable defaults if appropriate

v4:
 * Reintroduce broken #undef logic to be bug-compatible with the previous
   version.
---
 xen/include/public/errno.h | 41 ++---
 xen/include/xen/errno.h|  6 ++
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/xen/include/public/errno.h b/xen/include/public/errno.h
index dbac396..a0dd0cf 100644
--- a/xen/include/public/errno.h
+++ b/xen/include/public/errno.h
@@ -1,4 +1,31 @@
+/*
+ * There are two expected ways of including this header.
+ *
+ * 1) The "default" case (expected from tools etc).
+ *
+ * Simply #include 
+ *
+ * In this circumstance, normal header guards apply and the includer shall get
+ * an enumeration in the XEN_xxx namespace, appropriate for C or assembly.
+ *
+ * 2) The special case where the includer provides a XEN_ERRNO() in scope.
+ *
+ * In this case, no inclusion guards apply and the caller is responsible for
+ * their XEN_ERRNO() being appropriate in the included context.  The header
+ * will unilaterally #undef XEN_ERRNO().
+ */
+
+#ifndef XEN_ERRNO
+
+/*
+ * Includer has not provided a custom XEN_ERRNO().  Arrange for normal header
+ * guards, an automatic enum (for C code) and constants in the XEN_xxx
+ * namespace.
+ */
 #ifndef __XEN_PUBLIC_ERRNO_H__
+#define __XEN_PUBLIC_ERRNO_H__
+
+#define XEN_ERRNO_DEFAULT_INCLUDE
 
 #ifndef __ASSEMBLY__
 
@@ -11,11 +38,12 @@ enum xen_errno {
 
 #endif /* __ASSEMBLY__ */
 
+#endif /* __XEN_PUBLIC_ERRNO_H__ */
+#endif /* !XEN_ERRNO */
+
 /* ` enum neg_errnoval {  [ -Efoo for each Efoo in the list below ]  } */
 /* ` enum errnoval { */
 
-#endif /* __XEN_PUBLIC_ERRNO_H__ */
-
 #ifdef XEN_ERRNO
 
 /*
@@ -84,14 +112,13 @@ XEN_ERRNO(ETIMEDOUT,   110)/* Connection timed out 
*/
 
 #undef XEN_ERRNO
 #endif /* XEN_ERRNO */
-
-#ifndef __XEN_PUBLIC_ERRNO_H__
-#define __XEN_PUBLIC_ERRNO_H__
-
 /* ` } */
 
+/* Clean up from a default include.  Close the enum (for C). */
+#ifdef XEN_ERRNO_DEFAULT_INCLUDE
+#undef XEN_ERRNO_DEFAULT_INCLUDE
 #ifndef __ASSEMBLY__
 };
 #endif
 
-#endif /*  __XEN_PUBLIC_ERRNO_H__ */
+#endif /* XEN_ERRNO_DEFAULT_INCLUDE */
diff --git a/xen/include/xen/errno.h b/xen/include/xen/errno.h
index 3178466..69b28dd 100644
--- a/xen/include/xen/errno.h
+++ b/xen/include/xen/errno.h
@@ -1,18 +1,16 @@
 #ifndef __XEN_ERRNO_H__
 #define __XEN_ERRNO_H__
 
-#include 
-
 #ifndef __ASSEMBLY__
 
-#define XEN_ERRNO(name, value) name = XEN_##name,
+#define XEN_ERRNO(name, value) name = value,
 enum {
 #include 
 };
 
 #else /* !__ASSEMBLY__ */
 
-#define XEN_ERRNO(name, value) .equ name, XEN_##name
+#define XEN_ERRNO(name, value) .equ name, value
 #include 
 
 #endif /* __ASSEMBLY__ */
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] xen/vPMU: Do not clobber IA32_MISC_ENABLE

[Boris Ostrovsky]

On 03/07/2016 09:58 AM, Andrew Cooper wrote:

On 07/03/16 14:45, Boris Ostrovsky wrote:

On 03/07/2016 09:27 AM, Andrew Cooper wrote:

The VMX RDMSR intercept for MSR_IA32_MISC_ENABLE falls through into
vpmu_do_rdmsr(), so that core2_vpmu_do_rdmsr() may play with the PTS
and PEBS
UNAVAIL bits.

Some 64bit Windows include IA32_MISC_ENABLE in the set of items
checked by
PatchGuard, and will suffer a BSOD 0x109
CRITICAL_STRUCTURE_CORRUPTION if the
contents change on migrate.

The vPMU infrastructure should not clobber IA32_MISC_ENABLE at all.

Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Boris Ostrovsky 

This appears to have been broken since the vPMU code was first
introduced.  It
appears to have lurked this log due to a hole (now fixed) in XenServers
upgrade testing.  The BSODs occur ~80% of the time on Win 8 thru 10, but
appear very hard to provoke on Windows 7.

This MSR still leaks mostly host state through into the guest.
Therefore
migration of windows is still liable to crash if moving between two
non-identical servers.  I need to get proper MSR levelling sorted
before this
issue can be resolved fully.
---
   xen/arch/x86/cpu/vpmu.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index 237b5ff..2f9ddf6 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -169,7 +169,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t
*msr_content,
   return ret;
  nop:
-if ( !is_write )
+if ( !is_write && (msr != MSR_IA32_MISC_ENABLE) )
   *msr_content = 0;
 return 0;


This is Intel-specific register so the test should really be happening
in vpmu_intel.c. Of course then you'd need to always dereference
vcpu_vpmu() and possibly add more checks to read/write ops (to mirror
the one at the top of vpmu_do_msr()).

So maybe at least have the vendor check too??

Strictly speaking, if we were to do a vendor check, it should be a guest
vendor check, not a host vendor check.

OTOH, we won't get here on a non-Intel host system, and emulating a
cross-vendor vPMU is going to end in disaster.  I personally don't think
its worth it.


I wasn't thinking about cross-vendor cases. But I forgot that we now go 
to VPMU code only for VPMU registers.


Reviewed-by: Boris Ostrovsky 


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

Re: [Xen-devel] [PATCH v2 1/3] travis: skip building coverity, smoke, and master

[Konrad Rzeszutek Wilk]

On Fri, Mar 04, 2016 at 02:09:46PM -0600, Doug Goldstein wrote:
> Skip building of the coverity, smoke, stable, and master branches since
> they just fast forward from staging.
> 
> Suggested-by: Andrew Cooper 
> Signed-off-by: Doug Goldstein 

Reviewed-by: Konrad Rzeszutek Wilk 

and applied.
> ---
> CC: Ian Jackson 
> CC: Jan Beulich 
> CC: Keir Fraser 
> CC: Tim Deegan 
> CC: Andrew Cooper 
> 
> change since v1:
> - ignore all coverity tested branches
> - ignore stable branches as well
> ---
>  .travis.yml | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 6693af2..6803e2f 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,6 +1,13 @@
>  language: c
>  dist: trusty
>  sudo: required
> +# don't test master, smoke and coverity branches
> +branches:
> +except:
> +- master
> +- smoke
> +- /^coverity-tested\/.*/
> +- /^stable-.*/
>  matrix:
>  include:
>  - compiler: gcc
> -- 
> 2.4.10
> 
> 
> ___
> 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 v2 1/3] console: allow log level threshold adjustments

[Jan Beulich]

>>> On 07.03.16 at 15:41,  wrote:
> On Mon, Mar 7, 2016 at 5:44 AM, Jan Beulich  wrote:
> On 04.03.16 at 21:55,  wrote:
 +case XEN_SYSCTL_LOGLVL_set:
 +if ( (op->host.lower_thresh >= 0 && op->host.upper_thresh >= 0 &&
 +  op->host.lower_thresh > op->host.upper_thresh) ||
 + (op->guest.lower_thresh >= 0 && op->guest.upper_thresh >= 0 
 &&
 +  op->guest.lower_thresh > op->guest.upper_thresh) )
 +return -EINVAL;
 +
 +do_loglvl_op(&op->host, &xenlog_lower_thresh,
 + &xenlog_upper_thresh, "standard");
>>>
>>>
>>> The keyboard and the sysctl both allow the user to go beyound the XENLOG_
>>> values we have. That is you could set the lower and upper threshold to be
>>> at 9 (or more) say. It will have the same effect as XENLOG_DEBUG (which is
>>> 4)
>>> as printk_prefix_check seems to have a simple < check.
>>>
>>> But perhaps to be correct only accept only proper values? Not allow
>>> the system admin to set the level to say 31415?
>>
>> Since there's no bad side effect from doing so I opted for not
>> adding respective extra checks, keeping the code easier to read.
>>
> 
> Fair enough. Could you perhaps just add that in the commit description?

Sure.

> Also I noticed that this patch is missing an XSM check in flask_sysctl
> - could that be added please?

Of course; it's pretty ugly that one doesn't notice the lack thereof
via a build failure.

Jan


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

Re: [Xen-devel] [PATCH v2 3/3] travis: actually disable debug for non-debug

[Konrad Rzeszutek Wilk]

On Fri, Mar 04, 2016 at 02:09:48PM -0600, Doug Goldstein wrote:
> Non-debug builds need to explicitly disable debug due to debug being
> defaulted to y in Config.mk

Which is because by default staging has debug=y (this changes once
the rcX candidates become available).

Reviewed-by: Konrad Rzeszutek Wilk 
> 
> Signed-off-by: Doug Goldstein 
> ---
> CC: Ian Jackson 
> CC: Jan Beulich 
> CC: Keir Fraser 
> CC: Tim Deegan 
> CC: Andrew Cooper 
> 
> change since v1:
> - none
> 
> tested at: https://travis-ci.org/cardoe/xen/builds/113700670
> this run shows a failure with BIGMEM=y and debug=n which was only exposed
> by this change.
> ---
>  .travis.yml | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 4bcd5a0..741a8ab 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -11,33 +11,33 @@ branches:
>  matrix:
>  include:
>  - compiler: gcc
> -  env: XEN_TARGET_ARCH=x86_64
> +  env: XEN_TARGET_ARCH=x86_64 debug=n
>  - compiler: gcc
> -  env: XEN_TARGET_ARCH=x86_64 XEN_CONFIG_EXPERT=y RANDCONFIG=y
> +  env: XEN_TARGET_ARCH=x86_64 XEN_CONFIG_EXPERT=y RANDCONFIG=y 
> debug=n
>  - compiler: gcc-5
> -  env: XEN_TARGET_ARCH=x86_64
> +  env: XEN_TARGET_ARCH=x86_64 debug=n
>  - compiler: gcc
>env: XEN_TARGET_ARCH=x86_64 debug=y
>  - compiler: gcc-5
>env: XEN_TARGET_ARCH=x86_64 debug=y
>  - compiler: clang
> -  env: XEN_TARGET_ARCH=x86_64 clang=y
> +  env: XEN_TARGET_ARCH=x86_64 clang=y debug=n
>  - compiler: clang-3.8
> -  env: XEN_TARGET_ARCH=x86_64 clang=y
> +  env: XEN_TARGET_ARCH=x86_64 clang=y debug=n
>  - compiler: clang
>env: XEN_TARGET_ARCH=x86_64 clang=y debug=y
>  - compiler: clang-3.8
>env: XEN_TARGET_ARCH=x86_64 clang=y debug=y
>  - compiler: gcc
> -  env: XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf-
> +  env: XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- 
> debug=n
>  - compiler: gcc
> -  env: XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- 
> XEN_CONFIG_EXPERT=y RANDCONFIG=y
> +  env: XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- 
> XEN_CONFIG_EXPERT=y RANDCONFIG=y debug=n
>  - compiler: gcc
>env: XEN_TARGET_ARCH=arm32 CROSS_COMPILE=arm-linux-gnueabihf- 
> debug=y
>  - compiler: gcc
> -  env: XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-
> +  env: XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- debug=n
>  - compiler: gcc
> -  env: XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- 
> XEN_CONFIG_EXPERT=y RANDCONFIG=y
> +  env: XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- 
> XEN_CONFIG_EXPERT=y RANDCONFIG=y debug=n
>  - compiler: gcc
>env: XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- debug=y
>  addons:
> -- 
> 2.4.10
> 
> 
> ___
> 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 v2 00/16] Scheduling related tracing improvements

[Konrad Rzeszutek Wilk]

On Mon, Mar 07, 2016 at 03:21:46PM +0100, Dario Faggioli wrote:
> On Mon, 2016-03-07 at 11:27 +, Wei Liu wrote:
> > On Mon, Mar 07, 2016 at 04:21:22AM -0700, Jan Beulich wrote:
> > > > > > On 07.03.16 at 11:54,  wrote:
> > > > 
> > > Well, if these were urgent I'd say yes. But I don't think they are,
> > > so I'd leave it to either Konrad (who has basically asked for what
> > > you offer) if he wants to deal with it, of wait for Ian's return.
> > > 
> > It's not urgent, but I would like to avoid wasting any test cycle. No
> > matter how innocent a patch looks, there is always risk that it
> > breaks
> > something.
> > 
> > Konrad, your call.
> > 
> In any case, and as far as these patches are concerned, here it is:
> 
>  git://xenbits.xen.org/people/dariof/xen.git  
> tracing/sched-events-improvements
> 
>  
> http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/tracing/sched-events-improvements

Let me put them in staging.
> 
> 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)
> 



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

Re: [Xen-devel] [PATCH 1/4] x86/alternatives: correct near branch check

[Andrew Cooper]

On 04/03/16 11:27, Jan Beulich wrote:
> Make sure the near JMP/CALL check doesn't consume uninitialized
> data, not even in a benign way. And relax the length check at once.
>
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -174,7 +174,7 @@ static void __init apply_alternatives(st
>  memcpy(insnbuf, replacement, a->replacementlen);
>  
>  /* 0xe8/0xe9 are relative branches; fix the offset. */
> -if ( (*insnbuf & 0xfe) == 0xe8 && a->replacementlen == 5 )
> +if ( a->replacementlen >= 5 && (*insnbuf & 0xfe) == 0xe8 )
>  *(s32 *)(insnbuf + 1) += replacement - instr;
>  
>  add_nops(insnbuf + a->replacementlen,
>
>
>

Swapping the order is definitely a good thing.

However, relaxing the length check seems less so.  `E8 rel32` or `E9
rel32` encodings are strictly 5 bytes long.

There are complications with the `67 E{8,9} rel16` encodings, but those
are not catered for anyway, and the manual warns about undefined
behaviour if used in long mode.

What is your usecase for relaxing the check?  IMO, if it isn't exactly 5
bytes long, there is some corruption somewhere and the relocation
should't happen.

~Andrew

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

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

[Konrad Rzeszutek Wilk]

On Mon, Mar 07, 2016 at 11:21:33AM +, George Dunlap wrote:
> On Fri, Mar 4, 2016 at 10:00 PM, Konrad Rzeszutek Wilk
>  wrote:
> >> +/* Handle VT-d posted-interrupt when VCPU is blocked. */
> >> +static void pi_wakeup_interrupt(struct cpu_user_regs *regs)
> >> +{
> >> +struct arch_vmx_struct *vmx, *tmp;
> >> +spinlock_t *lock = &per_cpu(vmx_pi_blocking, smp_processor_id()).lock;
> >> +struct list_head *blocked_vcpus =
> >> + &per_cpu(vmx_pi_blocking, smp_processor_id()).list;
> >> +
> >> +ack_APIC_irq();
> >> +this_cpu(irq_count)++;
> >> +
> >> +spin_lock(lock);
> >> +
> >> +/*
> >> + * XXX: The length of the list depends on how many vCPU is current
> >> + * blocked on this specific pCPU. This may hurt the interrupt latency
> >> + * if the list grows to too many entries.
> >> + */
> >> +list_for_each_entry_safe(vmx, tmp, blocked_vcpus, pi_blocking.list)
> >> +{
> >
> >
> > My recollection of the 'most-horrible' case of this being really bad is when
> > the scheduler puts the vCPU0 and VCPU1 of the guest on the same pCPU (as an 
> > example)
> > and they round-robin all the time.
> >
> > 
> > Would it be perhaps possible to have an anti-affinity flag to deter the
> > scheduler from this? That is whichever struct vcpu has 'anti-affinity' flag
> > set - the scheduler will try as much as it can _to not_ schedule the 
> > 'struct vcpu'
> > if the previous 'struct vcpu' had this flag as well on this pCPU?
> 
> Well having vcpus from the same guest on the same pcpu is problematic
> for a number of reasons -- spinlocks first and foremost.  So in
> general trying to avoid that would be useful for most guests.

PV ticketlocks in HVM and PV guests make this "manageable".

> 
> The thing with scheduling is that it's a bit like economics: it seems
> simple but it's actually not at all obvious what the emergent behavior
> will be from adding a simple rule. :-)


> 
> On the whole it seems unlikely that having two vcpus on a single pcpu
> is a "stable" situation -- it's likely to be pretty transient, and
> thus not have a major impact on performance.

Except that we are concerned with it - in fact we are disabling this
feature because it may happen. How do we make sure it does not happen
all the time? Or at least do some back-off if things do get
in this situation.
> 
> That said, the load balancing code from credit2 *should*, in theory,
> make it easier to implement this sort of thing; it has the concept of
> a "cost" that it's trying to minimize; so you could in theory add a
> "cost" to configurations where vcpus from the same processor share the
> same pcpu.  Then it's not a hard-and-fast rule: if you have more vcpus
> than pcpus, the scheduler will just deal. :-)
> 
> But I think some profiling is in order before anyone does serious work on 
> this.

I appreciate your response being 'profiling' instead of 'Are you
NUTS!?' :-)

> 
>  -George

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

Re: [Xen-devel] [PATCH 1/4] x86/alternatives: correct near branch check

[Jan Beulich]

>>> On 07.03.16 at 16:43,  wrote:
> On 04/03/16 11:27, Jan Beulich wrote:
>> Make sure the near JMP/CALL check doesn't consume uninitialized
>> data, not even in a benign way. And relax the length check at once.
>>
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/alternative.c
>> +++ b/xen/arch/x86/alternative.c
>> @@ -174,7 +174,7 @@ static void __init apply_alternatives(st
>>  memcpy(insnbuf, replacement, a->replacementlen);
>>  
>>  /* 0xe8/0xe9 are relative branches; fix the offset. */
>> -if ( (*insnbuf & 0xfe) == 0xe8 && a->replacementlen == 5 )
>> +if ( a->replacementlen >= 5 && (*insnbuf & 0xfe) == 0xe8 )
>>  *(s32 *)(insnbuf + 1) += replacement - instr;
>>  
>>  add_nops(insnbuf + a->replacementlen,
>>
>>
>>
> 
> Swapping the order is definitely a good thing.
> 
> However, relaxing the length check seems less so.  `E8 rel32` or `E9
> rel32` encodings are strictly 5 bytes long.
> 
> There are complications with the `67 E{8,9} rel16` encodings, but those
> are not catered for anyway, and the manual warns about undefined
> behaviour if used in long mode.
> 
> What is your usecase for relaxing the check?  IMO, if it isn't exactly 5
> bytes long, there is some corruption somewhere and the relocation
> should't happen.

The relaxation is solely because at least CALL could validly
be followed by further instructions.

Jan


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

Re: [Xen-devel] [PATCH] libxc: simplify error handling in meminit_hvm

[Doug Goldstein]

On 3/3/16 10:09 AM, Wei Liu wrote:
> The hunk that prints out error message can only be reached from the loop
> that allocates memory. Move that hunk into the loop to avoid confusing
> gcc.
> 
> Reported-by: Doug Goldstein 
> Signed-off-by: Wei Liu 
> ---

Reviewed-by: Doug Goldstein 

-- 
Doug Goldstein



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 1/4] x86/alternatives: correct near branch check

[Andrew Cooper]

On 07/03/16 15:56, Jan Beulich wrote:
 On 07.03.16 at 16:43,  wrote:
>> On 04/03/16 11:27, Jan Beulich wrote:
>>> Make sure the near JMP/CALL check doesn't consume uninitialized
>>> data, not even in a benign way. And relax the length check at once.
>>>
>>> Signed-off-by: Jan Beulich 
>>>
>>> --- a/xen/arch/x86/alternative.c
>>> +++ b/xen/arch/x86/alternative.c
>>> @@ -174,7 +174,7 @@ static void __init apply_alternatives(st
>>>  memcpy(insnbuf, replacement, a->replacementlen);
>>>  
>>>  /* 0xe8/0xe9 are relative branches; fix the offset. */
>>> -if ( (*insnbuf & 0xfe) == 0xe8 && a->replacementlen == 5 )
>>> +if ( a->replacementlen >= 5 && (*insnbuf & 0xfe) == 0xe8 )
>>>  *(s32 *)(insnbuf + 1) += replacement - instr;
>>>  
>>>  add_nops(insnbuf + a->replacementlen,
>>>
>>>
>>>
>> Swapping the order is definitely a good thing.
>>
>> However, relaxing the length check seems less so.  `E8 rel32` or `E9
>> rel32` encodings are strictly 5 bytes long.
>>
>> There are complications with the `67 E{8,9} rel16` encodings, but those
>> are not catered for anyway, and the manual warns about undefined
>> behaviour if used in long mode.
>>
>> What is your usecase for relaxing the check?  IMO, if it isn't exactly 5
>> bytes long, there is some corruption somewhere and the relocation
>> should't happen.
> The relaxation is solely because at least CALL could validly
> be followed by further instructions.

But without scanning the entire replacement buffer, there might be other
relocations needing to happen.

That would require decoding the instructions, which is an extreme faff. 
It would be better to leave it currently as-is to effectively disallow
mixing a jmp/call replacement with other code, to avoid the subtle
failure of a second relocation not taking effect

~Andrew

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

Re: [Xen-devel] [Xen-users] Garbled screen after RAM Scrub on boot

[Jan Beulich]

>>> On 07.03.16 at 15:33,  wrote:
> Have you guys ever seen something like this before?  Any ideas how to 
> proceed?

What Andrew said is one possibility. There could be others, depending
on when _exactly_ these odd little squares show up. However, as to
what Andrew suggests - afaict this ought to lead to a black screen,
since character zero is a blank one in all our fonts, not a little box
(which is usually used to represent unknown characters), and even
the character used in debug build (0xc2) isn't represented by a little
box in any of our fonts. I therefore suspect the problem to be
introduced by the Dom0 kernel, not the hypervisor.

Jan

> On Tue, Feb 23, 2016 at 1:37 PM, Francis Greaves  wrote:
>> Dear All
>> I am using Centos 7 with Xen 4.6 on a Dell Poweredge T430 with (from lspci)
>> Matrox Electronics Systems Ltd. G200eR2 (rev 01) VGA
>>
>> When the machine boots, after the 'Scrubbing Free RAM' message, I get a
>> screen filled with little white squares until the login prompt, so I cannot
>> see what is happening as the machine boots. Also there is nothing on the
>> screen when I reboot or shutdown.
>>
>> My /etc/default/grub is
>>
>> GRUB_DISTRIBUTOR="$(sed 's, release .*$,,g' /etc/system-release)"
>> GRUB_DEFAULT=saved
>> GRUB_DISABLE_SUBMENU=true
>> GRUB_CMDLINE_LINUX="crashkernel=auto rhgb intremap=no_x2apic_optout"
>> GRUB_CMDLINE_XEN_DEFAULT="dom0_mem=13312M,max:14336M dom0_max_vcpus=6
>> dom0_vcpus_pin"
>> GRUB_GFXMODE=1024x768
>> GRUB_GFXPAYLOAD_LINUX=keep
>> GRUB_CMDLINE_LINUX_XEN_REPLACE_DEFAULT="console=hvc0 earlyprintk=xen
>> nomodeset"
>>
>> I have tried setting (for a 1024x768 resolution) vga=792 in the
>> GRUB_CMDLINE_LINUX and commenting out GRUB_GFXMODE and
>> GRUB_GFXPAYLOAD_LINUX, but this makes no difference.
>> I have also tried with no settings for vga at all, and with and without
>> 'rhgb'
>> I have even tried bootscrub=false as suggested by the CentOS-virt mailing
>> list, but that does not work either. They suggested I asked you folk.
>>
>> What am I doing wrong?
>>
>> Regards
>> Francis
>>
>>
>> ___
>> Xen-users mailing list
>> xen-us...@lists.xen.org 
>> http://lists.xen.org/xen-users 




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

Re: [Xen-devel] [PATCH] libxc: simplify error handling in meminit_hvm

[Roger Pau Monné]

On Thu, 3 Mar 2016, Wei Liu wrote:

> The hunk that prints out error message can only be reached from the loop
> that allocates memory. Move that hunk into the loop to avoid confusing
> gcc.
> 
> Reported-by: Doug Goldstein 
> Signed-off-by: Wei Liu 

Acked-by: Roger Pau Monné ___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Dario Faggioli]

On Mon, 2016-03-07 at 10:53 -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Mar 07, 2016 at 11:21:33AM +, George Dunlap wrote:
> > 
> > > 
> > > Would it be perhaps possible to have an anti-affinity flag to
> > > deter the
> > > scheduler from this? That is whichever struct vcpu has 'anti-
> > > affinity' flag
> > > set - the scheduler will try as much as it can _to not_ schedule
> > > the 'struct vcpu'
> > > if the previous 'struct vcpu' had this flag as well on this pCPU?
>
That can also be seen as step in the direction of (supporting) gang
scheduling, which we've said already it would be something interesting
to look at, although difficult to implement and even more difficult to
figure out whether it is actually a good thing for most workloads.

In any case, I see from where this comes, and am up for thinking about
it, although my fear is that it would complicate the code by quite a
bit, so I agree with George that profiling work is necessary to try to
assess whether it could be really useful (as well as, once
implemented/drafted, whether it is really good and does not cause perf
regressions).

> > On the whole it seems unlikely that having two vcpus on a single
> > pcpu
> > is a "stable" situation -- it's likely to be pretty transient, and
> > thus not have a major impact on performance.
> Except that we are concerned with it - in fact we are disabling this
> feature because it may happen. 
>
I'm sorry, I'm not getting, what feature are you disabling?

> > But I think some profiling is in order before anyone does serious
> > work on this.
> I appreciate your response being 'profiling' instead of 'Are you
> NUTS!?' :-)
> 
That's only because everyone knows you're nuts, there's no need to
state it all the times! :-P :-P

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 1/4] x86/alternatives: correct near branch check

[Jan Beulich]

>>> On 07.03.16 at 17:11,  wrote:
> On 07/03/16 15:56, Jan Beulich wrote:
> On 07.03.16 at 16:43,  wrote:
>>> On 04/03/16 11:27, Jan Beulich wrote:
 Make sure the near JMP/CALL check doesn't consume uninitialized
 data, not even in a benign way. And relax the length check at once.

 Signed-off-by: Jan Beulich 

 --- a/xen/arch/x86/alternative.c
 +++ b/xen/arch/x86/alternative.c
 @@ -174,7 +174,7 @@ static void __init apply_alternatives(st
  memcpy(insnbuf, replacement, a->replacementlen);
  
  /* 0xe8/0xe9 are relative branches; fix the offset. */
 -if ( (*insnbuf & 0xfe) == 0xe8 && a->replacementlen == 5 )
 +if ( a->replacementlen >= 5 && (*insnbuf & 0xfe) == 0xe8 )
  *(s32 *)(insnbuf + 1) += replacement - instr;
  
  add_nops(insnbuf + a->replacementlen,



>>> Swapping the order is definitely a good thing.
>>>
>>> However, relaxing the length check seems less so.  `E8 rel32` or `E9
>>> rel32` encodings are strictly 5 bytes long.
>>>
>>> There are complications with the `67 E{8,9} rel16` encodings, but those
>>> are not catered for anyway, and the manual warns about undefined
>>> behaviour if used in long mode.
>>>
>>> What is your usecase for relaxing the check?  IMO, if it isn't exactly 5
>>> bytes long, there is some corruption somewhere and the relocation
>>> should't happen.
>> The relaxation is solely because at least CALL could validly
>> be followed by further instructions.
> 
> But without scanning the entire replacement buffer, there might be other
> relocations needing to happen.
> 
> That would require decoding the instructions, which is an extreme faff. 
> It would be better to leave it currently as-is to effectively disallow
> mixing a jmp/call replacement with other code, to avoid the subtle
> failure of a second relocation not taking effect

Well, such missing further fixup would be noticed immediately by
someone trying (unless the patch code path never gets executed).
Whereas a simply adjustment to register state would seem quite
reasonable to follow a call. While right now the subsequent
patches don't depend on this being >= or ==, I think it was wrong
to be == from the beginning.

Plus - there are endless other possibilities of instructions needing
fixups (most notably such with RIP-relative memory operands),
none of which are even remotely reasonable to deal with here.
I.e. namely in the absence of a CALL/JMP the same issue would
exist anyway, which is why I'm not overly concerned of those.
All we want is a specific special case to be treated correctly.

Jan


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

Re: [Xen-devel] [PATCH v6 for Xen 4.7 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Chong Li]

On Mon, Mar 7, 2016 at 6:59 AM, Jan Beulich  wrote:
 On 06.03.16 at 18:55,  wrote:

>
>> @@ -1130,23 +1146,17 @@ rt_dom_cntl(
>>  unsigned long flags;
>>  int rc = 0;
>>
>> +xen_domctl_schedparam_vcpu_t local_sched;
>> +s_time_t period, budget;
>> +uint32_t index = 0;
>> +
>
> There's a stray blank line left ahead of this addition.
>
>>  switch ( op->cmd )
>>  {
>> -case XEN_DOMCTL_SCHEDOP_getinfo:
>> -if ( d->max_vcpus > 0 )
>> -{
>> -spin_lock_irqsave(&prv->lock, flags);
>> -svc = rt_vcpu(d->vcpu[0]);
>> -op->u.rtds.period = svc->period / MICROSECS(1);
>> -op->u.rtds.budget = svc->budget / MICROSECS(1);
>> -spin_unlock_irqrestore(&prv->lock, flags);
>> -}
>> -else
>> -{
>> -/* If we don't have vcpus yet, let's just return the defaults. 
>> */
>> -op->u.rtds.period = RTDS_DEFAULT_PERIOD;
>> -op->u.rtds.budget = RTDS_DEFAULT_BUDGET;
>> -}
>> +case XEN_DOMCTL_SCHEDOP_getinfo: /* return the default parameters */
>> +spin_lock_irqsave(&prv->lock, flags);
>> +op->u.rtds.period = RTDS_DEFAULT_PERIOD / MICROSECS(1);
>> +op->u.rtds.budget = RTDS_DEFAULT_BUDGET / MICROSECS(1);
>> +spin_unlock_irqrestore(&prv->lock, flags);
>>  break;
>
> This alters the values returned when d->max_vcpus == 0 - while
> this looks to be intentional, I think calling out such a bug fix in the
> description is a must.

Based on previous discussion, XEN_DOMCTL_SCHEDOP_getinfo only returns
the default parameters,
no matter whether vcpu is created yet or not. But I can absolutely
explain this in the description.
>
>> @@ -1163,6 +1173,96 @@ rt_dom_cntl(
>>  }
>>  spin_unlock_irqrestore(&prv->lock, flags);
>>  break;
>> +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
>> +if ( guest_handle_is_null(op->u.v.vcpus) )
>> +{
>> +rc = -EINVAL;
>
> Perhaps rather -EFAULT? But then again - what is this check good for
> (considering that it doesn't cover other obviously bad handle values)?

Dario suggested this in the last post, because vcpus is a handle and
needs to be validated.

>> +{
>> +rc = -EINVAL;
>> +break;
>> +}
>> +
>> +spin_lock_irqsave(&prv->lock, flags);
>> +svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>> +local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
>> +local_sched.s.rtds.period = svc->period / MICROSECS(1);
>> +spin_unlock_irqrestore(&prv->lock, flags);
>> +
>> +if ( __copy_to_guest_offset(op->u.v.vcpus, index,
>> +&local_sched, 1) )
>> +{
>> +rc = -EFAULT;
>> +break;
>> +}
>> +if ( (++index > 0x3f) && hypercall_preempt_check() )
>> +break;
>
> So how is the caller going to be able to reliably read all vCPU-s'
> information for a guest with more than 64 vCPU-s?

In libxc, we re-issue hypercall if the current one is preempted.

>
>> +}
>> +
>> +if ( !rc && (op->u.v.nr_vcpus != index) )
>> +op->u.v.nr_vcpus = index;
>
> I don't think the right side of the && is really necessary / useful.

The right side is to check whether the vcpus array is fully processed.
When it is true and no error occurs (rc == 0), we
update op->u.v.nr_vcpus, which is returned to libxc, and helps xc
function figuring out how many un-processed vcpus should
be taken care of in the next hypercall.


>
>> +typedef struct xen_domctl_schedparam_vcpu {
>> +union {
>> +xen_domctl_sched_credit_t credit;
>> +xen_domctl_sched_credit2_t credit2;
>> +xen_domctl_sched_rtds_t rtds;
>> +} s;
>
> Please call such unions "u", as done everywhere else.
>
>> +uint16_t vcpuid;
>
> Any particular reason to limit this to 16 bits, when elsewhere
> we commonly use 32 bits for vCPU IDs?

I'll change it.

Thanks for your comments.
Chong



-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

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

Re: [Xen-devel] [PATCH v6 for Xen 4.7 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Jan Beulich]

>>> On 07.03.16 at 17:28,  wrote:
> On Mon, Mar 7, 2016 at 6:59 AM, Jan Beulich  wrote:
> On 06.03.16 at 18:55,  wrote:
>>>  switch ( op->cmd )
>>>  {
>>> -case XEN_DOMCTL_SCHEDOP_getinfo:
>>> -if ( d->max_vcpus > 0 )
>>> -{
>>> -spin_lock_irqsave(&prv->lock, flags);
>>> -svc = rt_vcpu(d->vcpu[0]);
>>> -op->u.rtds.period = svc->period / MICROSECS(1);
>>> -op->u.rtds.budget = svc->budget / MICROSECS(1);
>>> -spin_unlock_irqrestore(&prv->lock, flags);
>>> -}
>>> -else
>>> -{
>>> -/* If we don't have vcpus yet, let's just return the defaults. 
>>> */
>>> -op->u.rtds.period = RTDS_DEFAULT_PERIOD;
>>> -op->u.rtds.budget = RTDS_DEFAULT_BUDGET;
>>> -}
>>> +case XEN_DOMCTL_SCHEDOP_getinfo: /* return the default parameters */
>>> +spin_lock_irqsave(&prv->lock, flags);
>>> +op->u.rtds.period = RTDS_DEFAULT_PERIOD / MICROSECS(1);
>>> +op->u.rtds.budget = RTDS_DEFAULT_BUDGET / MICROSECS(1);
>>> +spin_unlock_irqrestore(&prv->lock, flags);
>>>  break;
>>
>> This alters the values returned when d->max_vcpus == 0 - while
>> this looks to be intentional, I think calling out such a bug fix in the
>> description is a must.
> 
> Based on previous discussion, XEN_DOMCTL_SCHEDOP_getinfo only returns
> the default parameters,
> no matter whether vcpu is created yet or not. But I can absolutely
> explain this in the description.

That wasn't the point of the comment. Instead the change (fix) to
divide by MICROSECS(1) is what otherwise would go in silently.

>>> @@ -1163,6 +1173,96 @@ rt_dom_cntl(
>>>  }
>>>  spin_unlock_irqrestore(&prv->lock, flags);
>>>  break;
>>> +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
>>> +if ( guest_handle_is_null(op->u.v.vcpus) )
>>> +{
>>> +rc = -EINVAL;
>>
>> Perhaps rather -EFAULT? But then again - what is this check good for
>> (considering that it doesn't cover other obviously bad handle values)?
> 
> Dario suggested this in the last post, because vcpus is a handle and
> needs to be validated.

Well, as said - the handle being non-null doesn't make it a valid
handle. Any validation can be left to copy_{to,from}_guest*()
unless you mean to give a null handle some special meaning.

>>> +{
>>> +rc = -EINVAL;
>>> +break;
>>> +}
>>> +
>>> +spin_lock_irqsave(&prv->lock, flags);
>>> +svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>>> +local_sched.s.rtds.budget = svc->budget / MICROSECS(1);
>>> +local_sched.s.rtds.period = svc->period / MICROSECS(1);
>>> +spin_unlock_irqrestore(&prv->lock, flags);
>>> +
>>> +if ( __copy_to_guest_offset(op->u.v.vcpus, index,
>>> +&local_sched, 1) )
>>> +{
>>> +rc = -EFAULT;
>>> +break;
>>> +}
>>> +if ( (++index > 0x3f) && hypercall_preempt_check() )
>>> +break;
>>
>> So how is the caller going to be able to reliably read all vCPU-s'
>> information for a guest with more than 64 vCPU-s?
> 
> In libxc, we re-issue hypercall if the current one is preempted.

And with the current code - how does libxc know? (And anyway,
this should only be a last resort, if the hypervisor can't by itself
arrange for a continuation. If done this way, having a code
comment referring to the required caller behavior would seem to
be an absolute must.)

>>> +}
>>> +
>>> +if ( !rc && (op->u.v.nr_vcpus != index) )
>>> +op->u.v.nr_vcpus = index;
>>
>> I don't think the right side of the && is really necessary / useful.
> 
> The right side is to check whether the vcpus array is fully processed.
> When it is true and no error occurs (rc == 0), we
> update op->u.v.nr_vcpus, which is returned to libxc, and helps xc
> function figuring out how many un-processed vcpus should
> be taken care of in the next hypercall.

Just consider what the contents of op->u.v.nr_vcpus is after
this piece of code was executed, once with the full conditional,
and another time with the right side of the && omitted.

Jan

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 60684
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 60684
 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-xsm  15 guest-localmigratefail REGR. vs. 60684
 test-amd64-amd64-xl  15 guest-localmigratefail REGR. vs. 60684
 test-amd64-amd64-libvirt-xsm 15 guest-saverestore.2   fail REGR. vs. 60684
 test-amd64-amd64-pair23 guest-stop/src_host   fail REGR. vs. 60684
 test-amd64-amd64-xl-credit2  15 guest-localmigratefail REGR. vs. 60684
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install 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   15 guest-localmigrate   fail blocked in 60684
 test-amd64-i386-libvirt  15 guest-saverestore.2  fail blocked in 60684
 test-amd64-i386-xl-xsm   15 guest-localmigrate   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-libvirt-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
 test-amd64-amd64-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-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-intel 14 guest-saverestorefail  never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   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-amd64-qemuu-nested-intel 13 xen-boot/l1 fail never pass
 test-amd64-amd64-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-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:
 linux5a13582d3ff5299e1176a6a1d625020f3affb92d
baseline version:
 linux69f75ebe3b1d1e636c4ce0a0ee248edacc69cbe0

Last test of basis60684  2015-08-13 04:21:46 Z  207 days
Failing since 60712  2015-08-15 18:33:48 Z  204 days  149 attempts
Testing same since85596  2016-03-06 17:04: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   fail
 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-xsmfail
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm pass
 test-amd64

Re: [Xen-devel] [PATCH v2] xen/mm: Fix page_list_* helpers to evaluate all their arguments

[Jan Beulich]

>>> On 07.03.16 at 16:01,  wrote:
> +static inline void
> +page_list_del2(struct page_info *page, struct page_list_head *head1,
> +   struct page_list_head *head2)
> +{
> +list_del(&page->list);
> +}

The conversion of this in particular causes a build failure on ARM
(which doesn't d->arch.relmem_list, used as the second argument
to the above in page_alloc.c), which required me to remove that
commit again before pushing.

Jan


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

Re: [Xen-devel] [PATCH 2/4] x86: suppress SMAP and SMEP while running 32-bit PV guest code

[Andrew Cooper]

On 04/03/16 11:27, Jan Beulich wrote:
> Since such guests' kernel code runs in ring 1, their memory accesses,
> at the paging layer, are supervisor mode ones, and hence subject to
> SMAP/SMEP checks. Such guests cannot be expected to be aware of those
> two features though (and so far we also don't expose the respective
> feature flags), and hence may suffer page faults they cannot deal with.
>
> While the placement of the re-enabling slightly weakens the intended
> protection, it was selected such that 64-bit paths would remain
> unaffected where possible. At the expense of a further performance hit
> the re-enabling could be put right next to the CLACs.
>
> Note that this introduces a number of extra TLB flushes - CR4.SMEP
> transitioning from 0 to 1 always causes a flush, and it transitioning
> from 1 to 0 may also do.
>
> Signed-off-by: Jan Beulich 
>
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -67,6 +67,8 @@ boolean_param("smep", opt_smep);
>  static bool_t __initdata opt_smap = 1;
>  boolean_param("smap", opt_smap);
>  
> +unsigned long __read_mostly cr4_smep_smap_mask;

Are we liable to gain any other cr4 features which would want to be
included in this?  Might it be wise to chose a slightly more generic
name such as cr4_pv32_mask ?

>  #define SHADOW_BYTES 16 /* Shadow EIP + shadow hypercall # */
>  #else
>  /* Relocate argument registers and zero-extend to 64 bits. */
> -movl  %eax,%eax  /* Hypercall #  */
>  xchgl %ecx,%esi  /* Arg 2, Arg 4 */
>  movl  %edx,%edx  /* Arg 3*/
>  movl  %edi,%r8d  /* Arg 5*/
> @@ -174,10 +174,43 @@ compat_bad_hypercall:
>  /* %rbx: struct vcpu, interrupts disabled */
>  ENTRY(compat_restore_all_guest)
>  ASSERT_INTERRUPTS_DISABLED
> +.Lcr4_orig:
> +ASM_NOP3 /* mov   %cr4, %rax */
> +ASM_NOP6 /* and   $..., %rax */
> +ASM_NOP3 /* mov   %rax, %cr4 */
> +.pushsection .altinstr_replacement, "ax"
> +.Lcr4_alt:
> +mov   %cr4, %rax
> +and   $~(X86_CR4_SMEP|X86_CR4_SMAP), %rax
> +mov   %rax, %cr4
> +.Lcr4_alt_end:
> +.section .altinstructions, "a"
> +altinstruction_entry .Lcr4_orig, .Lcr4_alt, X86_FEATURE_SMEP, 12, \
> + (.Lcr4_alt_end - .Lcr4_alt)
> +altinstruction_entry .Lcr4_orig, .Lcr4_alt, X86_FEATURE_SMAP, 12, \
> + (.Lcr4_alt_end - .Lcr4_alt)

These 12's look as if they should be (.Lcr4_alt - .Lcr4_orig).

> +.popsection
>  RESTORE_ALL adj=8 compat=1
>  .Lft0:  iretq
>  _ASM_PRE_EXTABLE(.Lft0, handle_exception)
>  
> +/* This mustn't modify registers other than %rax. */
> +ENTRY(cr4_smep_smap_restore)
> +mov   %cr4, %rax
> +test  $X86_CR4_SMEP|X86_CR4_SMAP,%eax
> +jnz   0f
> +orcr4_smep_smap_mask(%rip), %rax
> +mov   %rax, %cr4
> +ret
> +0:
> +and   cr4_smep_smap_mask(%rip), %eax
> +cmp   cr4_smep_smap_mask(%rip), %eax
> +je1f
> +BUG

What is the purpose of this bugcheck? It looks like it is catching a
mismatch of masked options, but I am not completely sure.

For all other ASM level BUG's, I put a short comment on the same line,
to aid people who hit the bug.

> +1:
> +xor   %eax, %eax
> +ret
> +
>  /* %rdx: trap_bounce, %rbx: struct vcpu */
>  ENTRY(compat_post_handle_exception)
>  testb $TBF_EXCEPTION,TRAPBOUNCE_flags(%rdx)
> @@ -190,6 +223,7 @@ ENTRY(compat_post_handle_exception)
>  /* See lstar_enter for entry register state. */
>  ENTRY(cstar_enter)
>  sti
> +SMEP_SMAP_RESTORE
>  movq  8(%rsp),%rax /* Restore %rax. */
>  movq  $FLAT_KERNEL_SS,8(%rsp)
>  pushq %r11
> @@ -225,6 +259,7 @@ UNLIKELY_END(compat_syscall_gpf)
>  jmp   .Lcompat_bounce_exception
>  
>  ENTRY(compat_sysenter)
> +SMEP_SMAP_RESTORE
>  movq  VCPU_trap_ctxt(%rbx),%rcx
>  cmpb  $TRAP_gp_fault,UREGS_entry_vector(%rsp)
>  movzwl VCPU_sysenter_sel(%rbx),%eax
> @@ -238,6 +273,7 @@ ENTRY(compat_sysenter)
>  jmp   compat_test_all_events
>  
>  ENTRY(compat_int80_direct_trap)
> +SMEP_SMAP_RESTORE
>  call  compat_create_bounce_frame
>  jmp   compat_test_all_events
>  
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -434,6 +434,7 @@ ENTRY(dom_crash_sync_extable)
>  
>  ENTRY(common_interrupt)
>  SAVE_ALL CLAC
> +SMEP_SMAP_RESTORE
>  movq %rsp,%rdi
>  callq do_IRQ
>  jmp ret_from_intr
> @@ -454,13 +455,64 @@ ENTRY(page_fault)
>  GLOBAL(handle_exception)
>  SAVE_ALL CLAC
>  handle_exception_saved:
> +GET_CURRENT(%rbx)
>  testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp)
>  jzexception_with_ints_disabled
> -sti
> +
> +.Lsmep_smap_orig:
> +jmp   0f
> +.if 0 // GAS bug (affecting a

Re: [Xen-devel] Prototype Code Review Dashboards (input required)

[Lars Kurth]

> On 4 Mar 2016, at 09:21, Jan Beulich  wrote:
> 
 On 04.03.16 at 10:05,  wrote:
>>> On 4 Mar 2016, at 08:42, Jan Beulich  wrote:
>> On 03.03.16 at 19:55,  wrote:
> On 2 Mar 2016, at 22:45, Daniel Izquierdo  wrote:
> On 01/03/16 18:04, Lars Kurth wrote:
>> It may be better to use the following definition (although, others may 
>> disagree)
>> A reviewer is someone who did one of the following for a patch or series:
>> - Added a reviewed-by flag
>> - Added an acked-by flag (maintainers tend to use acked-by)
>> - Made a comment, but is NOT the author
> 
...
>> 
>> @Jan, the use-case to measure real review contributions was primarily added 
>> on your request. Do you think the proposed definition above, is good enough?
> 
> Yes, the last bullet point should be what mostly addresses my
> original concern. Some differentiation between Acked-by and
> Reviewed-by may also help - remember that in the case of
> maintainers we generally mean the latter to imply the former,
> and that in the case of non-maintainers the former doesn't
> really mean much.

Sounds as if an approach similar to the one taken for the commit vs. review 
balance may make sense
Regards
Lars
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 3/4] x86: use optimal NOPs to fill the SMAP/SMEP placeholders

[Andrew Cooper]

On 04/03/16 11:28, Jan Beulich wrote:
> Alternatives patching code picks the most suitable NOPs for the
> running system, so simply use it to replace the pre-populated ones.
>
> Use an arbitrary, always available feature to key off from.

I would be tempted to introduce X86_FEATURE_ALWAYS as an alias of
X86_FEATURE_LM, or even a new synthetic feature.  The choice of LM is
explained in the commit message, but will be non-obvious to people
reading the code.

~Andrew

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  6a5f34a262e0370164a0cd1162fa110a681dcfdd
baseline version:
 xen  1bd52e1fd66c47af690124d74d11ccb271c96f6b

Last test of basis85354  2016-03-04 17:01:50 Z3 days
Testing same since85654  2016-03-07 16:02:42 Z0 days1 attempts


People who touched revisions under test:
  Dario Faggioli 
  Doug Goldstein 
  George Dunlap 
  Konrad Rzeszutek Wilk 

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



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

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

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

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


Pushing revision :

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

Re: [Xen-devel] [PATCH 4/4] x86: use 32-bit loads for 32-bit PV guest state reload

[Andrew Cooper]

On 04/03/16 11:29, Jan Beulich wrote:
> This is slightly more efficient than loading 64-bit quantities.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

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

Re: [Xen-devel] [PATCH v6 for Xen 4.7 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Dario Faggioli]

On Mon, 2016-03-07 at 09:40 -0700, Jan Beulich wrote:
> > > > On 07.03.16 at 17:28,  wrote:
> > On Mon, Mar 7, 2016 at 6:59 AM, Jan Beulich 
> > wrote:
> > > 
> > > > @@ -1163,6 +1173,96 @@ rt_dom_cntl(
> > > > 
> > > > +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
> > > > +if ( guest_handle_is_null(op->u.v.vcpus) )
> > > > +{
> > > > +rc = -EINVAL;
> > > Perhaps rather -EFAULT? But then again - what is this check good
> > > for
> > > (considering that it doesn't cover other obviously bad handle
> > > values)?
> > Dario suggested this in the last post, because vcpus is a handle
> > and
> > needs to be validated.
>
> Well, as said - the handle being non-null doesn't make it a valid
> handle. Any validation can be left to copy_{to,from}_guest*()
> unless you mean to give a null handle some special meaning.
> 
IIRC, I was looking at how XEN_SYSCTL_pcitopoinfo is handled, for
reference, and that has some guest_handle_is_null()==>EINVAL sainity
checking (in xen/common/sysctl.c), which, when I thought about it, made
sense to me.

My reasoning was, sort of:
 1. if the handle is NULL, no point getting into the somewhat 
    complicated logic of the while,
 2. more accurate error reporting: as being passed a NULL handler 
    looked something we could identify and call invalid, rather than 
    waiting for the copy to fault.

In any event, I've no problem at all with this being dropped.

> > > > +{
> > > > +rc = -EINVAL;
> > > > +break;
> > > > +}
> > > > +
> > > > +spin_lock_irqsave(&prv->lock, flags);
> > > > +svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
> > > > +local_sched.s.rtds.budget = svc->budget /
> > > > MICROSECS(1);
> > > > +local_sched.s.rtds.period = svc->period /
> > > > MICROSECS(1);
> > > > +spin_unlock_irqrestore(&prv->lock, flags);
> > > > +
> > > > +if ( __copy_to_guest_offset(op->u.v.vcpus, index,
> > > > +&local_sched, 1) )
> > > > +{
> > > > +rc = -EFAULT;
> > > > +break;
> > > > +}
> > > > +if ( (++index > 0x3f) && hypercall_preempt_check()
> > > > )
> > > > +break;
> > > So how is the caller going to be able to reliably read all vCPU-
> > > s'
> > > information for a guest with more than 64 vCPU-s?
> > In libxc, we re-issue hypercall if the current one is preempted.
> And with the current code - how does libxc know? (And anyway,
> this should only be a last resort, if the hypervisor can't by itself
> arrange for a continuation. If done this way, having a code
> comment referring to the required caller behavior would seem to
> be an absolute must.)
> 
I definitely agree on commenting.

About the structure of the code, as said above, I do like
how XEN_SYSCTL_pcitopoinfo ended up being handled, I think it is a
great fit for this specific case and, comparing at both this and
previous version, I do think this one is (bugs apart) looking better.

I'm sure I said this --long ago-- when discussing v4 (and maybe even
previous versions), as well as more recently, when reviewing v5, and
that's why Chong (finally! :-D) did it.

So, with the comment in place (and with bugs fixed :-)), are you (Jan)
ok with this being done this way?

> > > > +}
> > > > +
> > > > +if ( !rc && (op->u.v.nr_vcpus != index) )
> > > > +op->u.v.nr_vcpus = index;
> > > I don't think the right side of the && is really necessary /
> > > useful.
> > The right side is to check whether the vcpus array is fully
> > processed.
> > When it is true and no error occurs (rc == 0), we
> > update op->u.v.nr_vcpus, which is returned to libxc, and helps xc
> > function figuring out how many un-processed vcpus should
> > be taken care of in the next hypercall.
> Just consider what the contents of op->u.v.nr_vcpus is after
> this piece of code was executed, once with the full conditional,
> and another time with the right side of the && omitted.
> 
BTW, Chong, I'm not sure this has to do with what Jan is saying, but
looking again at XEN_SYSCTL_pcitopoinfo, it looks to me you're missing
copying nr_vcpus back up to the guest (which is actually what makes
libxc knows whether all vcpus have been processed or now).

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 v2 3/3] xl: new "loglvl" command

[Dario Faggioli]

On Mon, 2016-03-07 at 04:46 -0700, Jan Beulich wrote:
> > > > On 04.03.16 at 19:45,  wrote:
> > On Fri, 2016-03-04 at 09:48 -0700, Jan Beulich wrote:

> > > --- a/tools/libxl/libxl.c
> > > +++ b/tools/libxl/libxl.c
> > > @@ -5958,6 +5958,26 @@ int libxl_send_debug_keys(libxl_ctx *ctx
> > >  return 0;
> > >  }
> > >  
> > > +int libxl_log_level(libxl_ctx *ctx, bool set, bool guest,
> > > +int *lower_thresh, int *upper_thresh)
> > > +{
> > > +int ret;
> > > 
> > As per libxl coding style, this wants to be 'r'.
> This and everything else below look to be valid comments, but
> it's rather frustrating that simply cloning an existing function (I
> user the debug key ones as basis) doesn't give me valid code,
> the more that I did scroll up and down a few pages to see
> whether I just happened to pick a particularly bad example.
>
Hehe, but do you understand that, saying this, you're making it very
likely that people will ask *you* to fix libxl_send_debug_keys() --and
perhaps more tool side code? :-P :-P

No, jokes apart, I agree that inconsistency is a real bad thing... but
it's an hard fight, and we do have examples spread all around the
source code (both Xen and tools), AFAICT.

I run into the patch, decided to have a look, and thought I better say
what I found, with the aim of fighting exactly that (inconsistency in
the code). If there is anything else I can do for help, feel free to
ask (e.g., I guess I can send a patch to fix style of
libxl_send_debug_keys() myself :-)).

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

[Xen-devel] [PATCH v3] xen/mm: Fix page_list_* helpers to evaluate all their arguments

[Andrew Cooper]

If an architecture does not provide a custom page_list_entry, default
page_list_* helpers are provided, wrapping list_head as an underlying type for
page_list_head.

The two declarations of the page_list_* helpers differ between defines and
static inline functions, where the defines discard some of their parameters.

This causes a compilation failure if CONFIG_BIGMEM and debug=n in p2m-pod.c:

  p2m-pod.c: In function ‘p2m_pod_cache_add’:
  p2m-pod.c:72:20: error: unused variable ‘d’ [-Werror=unused-variable]
   struct domain *d = p2m->domain;
  ^
  cc1: all warnings being treated as errors

because the use of d outside of the !NDEBUG section doesn't get evaluated as a
parameter by page_list_del().

Fix this by turning all #defines into static inline functions, so all
parameters are evaluated even if they are not used.

This reveals a build issue on ARM.  page_alloc.c references
d->arch.relmem_list in the previously-discarded parameter.  Fix this by
introducing relmem_list for ARM (currently unused).

While editing this area, correct the return type of page_list_empty from int
to bool_t.

Reported-by: Doug Goldstein 
Signed-off-by: Andrew Cooper 
Reviewed-by: Doug Goldstein 
---
CC: Jan Beulich 
CC: Tim Deegan 
CC: George Dunlap 
CC: Stefano Stabellini 
CC: Julien Grall 

v2:
 * Remove explicit casts, and missing !! in page_list_empty()

v3:
 * Fix build on ARM.  As it is referenced from common, relmem_list shouldn't
   be an .arch variable.  However, moving it would split it away from the
   relmem enumeration, which is specific to the arch.  (Basically - I lack
   sufficient TUITs to disentagle this properly, and this is the most simple 
fix.)
---
 xen/include/asm-arm/domain.h |  1 +
 xen/include/xen/mm.h | 95 ++--
 2 files changed, 75 insertions(+), 21 deletions(-)

diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index c35ed40..c274547 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -65,6 +65,7 @@ struct arch_domain
 RELMEM_mapping,
 RELMEM_done,
 } relmem;
+struct page_list_head relmem_list;
 
 /* Virtual CPUID */
 uint32_t vpidr;
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index a795dd6..8600cf6 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -220,7 +220,7 @@ struct page_list_head
 # define INIT_PAGE_LIST_HEAD(head) ((head)->tail = (head)->next = NULL)
 # define INIT_PAGE_LIST_ENTRY(ent) ((ent)->prev = (ent)->next = PAGE_LIST_NULL)
 
-static inline int
+static inline bool_t
 page_list_empty(const struct page_list_head *head)
 {
 return !head->next;
@@ -392,31 +392,84 @@ page_list_splice(struct page_list_head *list, struct 
page_list_head *head)
 # define PAGE_LIST_HEAD  LIST_HEAD
 # define INIT_PAGE_LIST_HEAD INIT_LIST_HEAD
 # define INIT_PAGE_LIST_ENTRYINIT_LIST_HEAD
-# define page_list_empty list_empty
-# define page_list_first(hd) \
-list_first_entry(hd, struct page_info, list)
-# define page_list_last(hd)  \
-list_last_entry(hd, struct page_info, list)
-# define page_list_next(pg, hd)  list_next_entry(pg, list)
-# define page_list_prev(pg, hd)  list_prev_entry(pg, list)
-# define page_list_add(pg, hd)   list_add(&(pg)->list, hd)
-# define page_list_add_tail(pg, hd)  list_add_tail(&(pg)->list, hd)
-# define page_list_del(pg, hd)   list_del(&(pg)->list)
-# define page_list_del2(pg, hd1, hd2)list_del(&(pg)->list)
-# define page_list_remove_head(hd)   (!page_list_empty(hd) ? \
-({ \
-struct page_info *__pg = page_list_first(hd); \
-list_del(&__pg->list); \
-__pg; \
-}) : NULL)
-# define page_list_move(dst, src)(!list_empty(src) ? \
-list_replace_init(src, dst) : (void)0)
+
+static inline bool_t
+page_list_empty(const struct page_list_head *head)
+{
+return !!list_empty(head);
+}
+static inline struct page_info *
+page_list_first(const struct page_list_head *head)
+{
+return list_first_entry(head, struct page_info, list);
+}
+static inline struct page_info *
+page_list_last(const struct page_list_head *head)
+{
+return list_last_entry(head, struct page_info, list);
+}
+static inline struct page_info *
+page_list_next(const struct page_info *page,
+   const struct page_list_head *head)
+{
+return list_entry(page->list.next, struct page_info, list);
+}
+static inline struct page_info *
+page_list_prev(const struct page_info *page,
+   const struct page_list_head *head)
+{
+return list_entry(page->list.prev, struct page_info, list);
+}
+static inline void
+page_list_add(struct page_info *page, struct page_list_head *head)
+{
+list_add(&page->list, head);
+}
+static inline void
+page_list_add_tail(struct page_info *page, struct page_list_head *head)
+{
+list_add_tail(&page->list, head);
+}
+s

[Xen-devel] [PATCH] tools/foreign: Avoid using alignment directives when not appropriate

[Andrew Cooper]

The foreign header generation blindly replaces 'uint64_t' with '__align8__
uint64_t', to get correct alignment when built as 32bit.  This is correct in
most circumstances, but Clang objects to two specific uses.

 * Inside a sizeof() expression
 * As part of a typecast

An example error looks like:

/local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:204:44:
error: 'aligned' attribute ignored when parsing type 
[-Werror,-Wignored-attributes]
__align8__ uint64_t evtchn_mask[sizeof(__align8__ uint64_t) * 8];
   ^~
/local/xen.git/tools/libxc/../../tools/include/xen/foreign/x86_64.h:13:36:
note: expanded from macro '__align8__'
# define __align8__ __attribute__((aligned (8)))
   ^~~

This sedary is sufficient to fix all the bad examples without touching any of
the legitimate uses, and is more simple than teaching mkheader.py how to parse
C.

Signed-off-by: Andrew Cooper 
---
CC: Ian Jackson 
CC: Wei Liu 
CC: Stefano Stabellini 
CC: Julien Grall 
---
 tools/include/xen-foreign/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/include/xen-foreign/Makefile 
b/tools/include/xen-foreign/Makefile
index 80a446a..b25bfa8 100644
--- a/tools/include/xen-foreign/Makefile
+++ b/tools/include/xen-foreign/Makefile
@@ -35,6 +35,8 @@ x86_32.h: mkheader.py structs.py 
$(ROOT)/arch-x86/xen-x86_32.h $(ROOT)/arch-x86/
 
 x86_64.h: mkheader.py structs.py $(ROOT)/arch-x86/xen-x86_64.h 
$(ROOT)/arch-x86/xen.h $(ROOT)/xen.h
$(PYTHON) $< $* $@ $(filter %.h,$^)
+   #Avoid mixing an alignment directive with a uint64_t cast or sizeof 
expression
+   sed 's/(__align8__ uint64_t)/(uint64_t)/g' -i $@
 
 checker.c: mkchecker.py structs.py
$(PYTHON) $< $@ $(architectures)
-- 
2.1.4


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

[Xen-devel] [PATCH 1/2] xsm: move the XSM_MAGIC value to Kconfig

[Doug Goldstein]

Let Kconfig set the XSM_MAGIC value for us.

Signed-off-by: Doug Goldstein 
---
CC: Daniel De Graaf 
---
 xen/common/Kconfig   | 8 
 xen/include/xen/config.h | 1 -
 xen/include/xsm/xsm.h| 5 +
 xen/xsm/xsm_core.c   | 4 ++--
 xen/xsm/xsm_policy.c | 6 +++---
 5 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 8fbc46d..d661da3 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -113,6 +113,14 @@ config XSM
 
  If unsure, say N.
 
+# XSM magic for policy detection
+config XSM_MAGIC
+hex
+default 0xf97cff8c if FLASK
+default 0 if !FLASK
+---help---
+  Identifies a FLASK XSM policy start point
+
 # Enable schedulers
 menu "Schedulers"
visible if EXPERT = "y"
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 96f5539..3f8c53d 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -79,7 +79,6 @@
 #define STR(...) __STR(__VA_ARGS__)
 
 #ifdef CONFIG_FLASK
-#define XSM_MAGIC 0xf97cff8c
 /* Maintain statistics on the access vector cache */
 #define FLASK_AVC_STATS 1
 #endif
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 3afed70..7f313ad 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -21,11 +21,8 @@
 typedef void xsm_op_t;
 DEFINE_XEN_GUEST_HANDLE(xsm_op_t);
 
-/* policy magic number (defined by XSM_MAGIC) */
+/* policy magic number (defined by CONFIG_XSM_MAGIC) */
 typedef u32 xsm_magic_t;
-#ifndef XSM_MAGIC
-#define XSM_MAGIC 0x
-#endif
 
 /* These annotations are used by callers and in dummy.h to document the
  * default actions of XSM hooks. They should be compiled out otherwise.
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 5e432de..d6965ba 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -67,7 +67,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
 
 printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-if ( XSM_MAGIC )
+if ( CONFIG_XSM_MAGIC )
 {
 ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
 if ( ret )
@@ -92,7 +92,7 @@ int __init xsm_dt_init(void)
 
 printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
-if ( XSM_MAGIC )
+if ( CONFIG_XSM_MAGIC )
 {
 ret = xsm_dt_policy_init();
 if ( ret )
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index b60d822..52aa4a9 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -54,7 +54,7 @@ int __init xsm_multiboot_policy_init(unsigned long 
*module_map,
 _policy_start = bootstrap_map(mod + i);
 _policy_len   = mod[i].mod_end;
 
-if ( (xsm_magic_t)(*_policy_start) == XSM_MAGIC )
+if ( (xsm_magic_t)(*_policy_start) == CONFIG_XSM_MAGIC )
 {
 policy_buffer = (char *)_policy_start;
 policy_size = _policy_len;
@@ -89,10 +89,10 @@ int __init xsm_dt_policy_init(void)
 
 copy_from_paddr(&magic, paddr, sizeof(magic));
 
-if ( magic != XSM_MAGIC )
+if ( magic != CONFIG_XSM_MAGIC )
 {
 printk(XENLOG_ERR "xsm: Invalid magic for XSM blob got 0x%x "
-   "expected 0x%x\n", magic, XSM_MAGIC);
+   "expected 0x%x\n", magic, CONFIG_XSM_MAGIC);
 return -EINVAL;
 }
 
-- 
2.4.10


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

[Xen-devel] [PATCH 2/2] xsm: move FLASK_AVC_STATS to Kconfig

[Doug Goldstein]

Have Kconfig set CONFIG_FLASK_AVC_STATS and prefix all uses with CONFIG_
to use the Kconfig variable.

Signed-off-by: Doug Goldstein 
---
CC: Daniel De Graaf 
---
 xen/common/Kconfig  | 8 +++-
 xen/include/xen/config.h| 5 -
 xen/xsm/flask/avc.c | 4 ++--
 xen/xsm/flask/flask_op.c| 4 ++--
 xen/xsm/flask/include/avc.h | 2 +-
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index d661da3..db23edc 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -23,6 +23,12 @@ config FLASK
 
  If unsure, say N.
 
+config FLASK_AVC_STATS
+def_bool y if FLASK
+depends on FLASK
+---help---
+  Maintain statistics on the access vector cache
+
 # Select HAS_DEVICE_TREE if device tree is supported
 config HAS_DEVICE_TREE
bool
@@ -117,7 +123,7 @@ config XSM
 config XSM_MAGIC
 hex
 default 0xf97cff8c if FLASK
-default 0 if !FLASK
+default 0
 ---help---
   Identifies a FLASK XSM policy start point
 
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index 3f8c53d..ef6e5ee 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -78,11 +78,6 @@
 #define __STR(...) #__VA_ARGS__
 #define STR(...) __STR(__VA_ARGS__)
 
-#ifdef CONFIG_FLASK
-/* Maintain statistics on the access vector cache */
-#define FLASK_AVC_STATS 1
-#endif
-
 /* allow existing code to work with Kconfig variable */
 #define NR_CPUS CONFIG_NR_CPUS
 
diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index 31bc702..7764379 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -56,7 +56,7 @@ const struct selinux_class_perm selinux_class_perm = {
 #define AVC_DEF_CACHE_THRESHOLD512
 #define AVC_CACHE_RECLAIM16
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 #define avc_cache_stats_incr(field) \
 do {\
 __get_cpu_var(avc_cache_stats).field++;\
@@ -101,7 +101,7 @@ struct avc_callback_node {
 /* Exported via Flask hypercall */
 unsigned int avc_cache_threshold = AVC_DEF_CACHE_THRESHOLD;
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 DEFINE_PER_CPU(struct avc_cache_stats, avc_cache_stats);
 #endif
 
diff --git a/xen/xsm/flask/flask_op.c b/xen/xsm/flask/flask_op.c
index f4f5dd1..3c9c99e 100644
--- a/xen/xsm/flask/flask_op.c
+++ b/xen/xsm/flask/flask_op.c
@@ -469,7 +469,7 @@ static int flask_security_make_bools(void)
 return ret;
 }
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 
 static int flask_security_avc_cachestats(struct xen_flask_cache_stats *arg)
 {
@@ -761,7 +761,7 @@ ret_t do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) 
u_flask_op)
 rv = avc_get_hash_stats(&op.u.hash_stats);
 break;
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 case FLASK_AVC_CACHESTATS:
 rv = flask_security_avc_cachestats(&op.u.cache_stats);
 break;
diff --git a/xen/xsm/flask/include/avc.h b/xen/xsm/flask/include/avc.h
index 4283562..729856e 100644
--- a/xen/xsm/flask/include/avc.h
+++ b/xen/xsm/flask/include/avc.h
@@ -108,7 +108,7 @@ struct xen_flask_hash_stats;
 int avc_get_hash_stats(struct xen_flask_hash_stats *arg);
 extern unsigned int avc_cache_threshold;
 
-#ifdef FLASK_AVC_STATS
+#ifdef CONFIG_FLASK_AVC_STATS
 DECLARE_PER_CPU(struct avc_cache_stats, avc_cache_stats);
 #endif
 
-- 
2.4.10


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

Re: [Xen-devel] [PATCH v3 21/23] xsplice: Add support for shadow variables

[Martin Pohlack]

On 12.02.2016 19:05, Konrad Rzeszutek Wilk wrote:
> From: Ross Lagerwall 
> 
> Shadow variables are a piece of infrastructure to be used by xsplice
> modules. They are used to attach a new piece of data to an existing
> structure in memory.
> 
> Signed-off-by: Ross Lagerwall 
> ---
>  xen/common/Makefile |   1 +
>  xen/common/xsplice_shadow.c | 105 
> 
>  xen/include/xen/xsplice_patch.h |  39 +++
>  3 files changed, 145 insertions(+)
>  create mode 100644 xen/common/xsplice_shadow.c
>  create mode 100644 xen/include/xen/xsplice_patch.h
> 
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index a8ceaff..f4d54ad 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -75,3 +75,4 @@ subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
>  
>  obj-$(CONFIG_XSPLICE) += xsplice.o
>  obj-$(CONFIG_XSPLICE) += xsplice_elf.o
> +obj-$(CONFIG_XSPLICE) += xsplice_shadow.o
> diff --git a/xen/common/xsplice_shadow.c b/xen/common/xsplice_shadow.c
> new file mode 100644
> index 000..619cdee
> --- /dev/null
> +++ b/xen/common/xsplice_shadow.c
> @@ -0,0 +1,105 @@
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define SHADOW_SLOTS 256
> +struct hlist_head shadow_tbl[SHADOW_SLOTS];

Thinking about this more, how would a module using this global hash ever
be unloadable again without leaking memory?

For unloading you would need some iterator that walks all the
dynamically created shadow elements and frees them.  The simplest
approach would be if each hotpatch would bring its own instance of the
hash table (if it needs it).  That would allow it to fully walk and
release the hash content on its unload path.

Martin

Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B


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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 65543
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail REGR. vs. 65543

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

Last test of basis65543  2015-12-08 08:45:15 Z   90 days
Failing since 65593  2015-12-08 23:44:51 Z   89 days   94 attempts
Testing same since85629  2016-03-07 05:35:42 Z0 days1 attempts


People who touched revisions under test:
  "Samer El-Haj-Mahmoud" 
  "Yao, Jiewen" 
  Alcantara, Paulo 
  Anbazhagan Baraneedharan 
  Andrew Fish 
  Ard Biesheuvel 
  Arthur Crippa Burigo 
  Cecil Sheng 
  Chao Zhang 
  Charles Duffy 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Daocheng Bu 
  Daryl McDaniel 
  David Woodhouse 
  edk2 dev 
  edk2-devel 
  Eric Dong 
  Eric Dong 
  Eugene Cohen 
  Evan Lloyd 
  Feng Tian 
  Fu Siyuan 
  Hao Wu 
  Haojian Zhuang 
  Hess Chen 
  Heyi Guo 
  Jaben Carsey 
  Jeff Fan 
  Jiaxin Wu 
  jiewen yao 
  Jim Dailey 
  jim_dai...@dell.com 
  Jordan Justen 
  Karyne Mayer 
  Larry Hauch 
  Laszlo Ersek 
  Leahy, Leroy P 
  Lee Leahy 
  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 
  Tian, Feng 
  Vladislav Vovchenko 
  Yao Jiewen 
  Yao, Jiewen 
  Ye Ting 
  Yonghong Zhu 
  Zhang Lubo 
  Zhang, Chao B 
  Zhang, Lubo 
  Zhangfei Gao 

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 11991 lines long.)

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  0aa1330aac92fd75f185c9b354396014178fe95d
baseline version:
 xen  6a5f34a262e0370164a0cd1162fa110a681dcfdd

Last test of basis85654  2016-03-07 16:02:42 Z0 days
Testing same since85661  2016-03-07 18:02:23 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 

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



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

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

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

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


Pushing revision :

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

[Xen-devel] [linux-linus test] 85614: regressions - FAIL

[osstest service owner]

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

Regressions :-(

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

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

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-rumpuserxen-amd64  1 build-check(1)   blocked n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   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-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  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-qemuu-nested-intel 13 xen-boot/l1 fail never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 13 xen-boot/l1   fail never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-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-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass

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

[Konrad Rzeszutek Wilk]

On Mon, Mar 07, 2016 at 05:19:59PM +0100, Dario Faggioli wrote:
> On Mon, 2016-03-07 at 10:53 -0500, Konrad Rzeszutek Wilk wrote:
> > On Mon, Mar 07, 2016 at 11:21:33AM +, George Dunlap wrote:
> > > 
> > > > 
> > > > Would it be perhaps possible to have an anti-affinity flag to
> > > > deter the
> > > > scheduler from this? That is whichever struct vcpu has 'anti-
> > > > affinity' flag
> > > > set - the scheduler will try as much as it can _to not_ schedule
> > > > the 'struct vcpu'
> > > > if the previous 'struct vcpu' had this flag as well on this pCPU?
> >
> That can also be seen as step in the direction of (supporting) gang
> scheduling, which we've said already it would be something interesting
> to look at, although difficult to implement and even more difficult to
> figure out whether it is actually a good thing for most workloads.
> 
> In any case, I see from where this comes, and am up for thinking about
> it, although my fear is that it would complicate the code by quite a
> bit, so I agree with George that profiling work is necessary to try to
> assess whether it could be really useful (as well as, once
> implemented/drafted, whether it is really good and does not cause perf
> regressions).
> 
> > > On the whole it seems unlikely that having two vcpus on a single
> > > pcpu
> > > is a "stable" situation -- it's likely to be pretty transient, and
> > > thus not have a major impact on performance.
> > Except that we are concerned with it - in fact we are disabling this
> > feature because it may happen. 
> >
> I'm sorry, I'm not getting, what feature are you disabling?

It is already disabled in the code:

 62 /*  

 63  * In the current implementation of VT-d posted interrupts, in some extreme 

 64  * cases, the per cpu list which saves the blocked vCPU will be very long,  

 65  * and this will affect the interrupt latency, so let this feature off by   

 66  * default until we find a good solution to resolve it. 

 67  */ 

 68 bool_t __read_mostly iommu_intpost; 


> 
> > > But I think some profiling is in order before anyone does serious
> > > work on this.
> > I appreciate your response being 'profiling' instead of 'Are you
> > NUTS!?' :-)
> > 
> That's only because everyone knows you're nuts, there's no need to
> state it all the times! :-P :-P



Glad that you have the _right_ expectations of me :) 

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

Re: [Xen-devel] Xentrace on Xilinx ARM

[Paul Sujkov]

Hi Ben,

sorry for the delayed answer. There is xenalyze fork repo made by RT-Xen
author, Meng Xu:

https://github.com/PennPanda/xen-analyze

apart from some comments and logs it's quite the same tool as it was of Xen
4.5, you can use it while you have troubles with the code in the Xen repo.
Do you still need any help with Xen traces on arm?

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

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

[Konrad Rzeszutek Wilk]

On Wed, Mar 02, 2016 at 03:14:52PM +0800, Haozhong Zhang wrote:
> On 03/01/16 13:49, Konrad Rzeszutek Wilk wrote:
> > On Tue, Mar 01, 2016 at 06:33:32PM +, Ian Jackson wrote:
> > > Haozhong Zhang writes ("Re: [Xen-devel] [RFC Design Doc] Add vNVDIMM 
> > > support for Xen"):
> > > > On 02/18/16 21:14, Konrad Rzeszutek Wilk wrote:
> > > > > [someone:]
> > > > > > (2) For XENMAPSPACE_gmfn, _gmfn_range and _gmfn_foreign,
> > > > > >(a) never map idx in them to GFNs occupied by vNVDIMM, and
> > > > > >(b) never map idx corresponding to GFNs occupied by vNVDIMM
> > > > > 
> > > > > Would that mean that guest xen-blkback or xen-netback wouldn't
> > > > > be able to fetch data from the GFNs? As in, what if the HVM guest
> > > > > that has the NVDIMM also serves as a device domain - that is it
> > > > > has xen-blkback running to service other guests?
> > > > 
> > > > I'm not familiar with xen-blkback and xen-netback, so following
> > > > statements maybe wrong.
> > > > 
> > > > In my understanding, xen-blkback/-netback in a device domain maps the
> > > > pages from other domains into its own domain, and copies data between
> > > > those pages and vNVDIMM. The access to vNVDIMM is performed by NVDIMM
> > > > driver in device domain. In which steps of this procedure that
> > > > xen-blkback/-netback needs to map into GFNs of vNVDIMM?
> > > 
> > > I think I agree with what you are saying.  I don't understand exactly
> > > what you are proposing above in XENMAPSPACE_gmfn but I don't see how
> > > anything about this would interfere with blkback.
> > > 
> > > blkback when talking to an nvdimm will just go through the block layer
> > > front door, and do a copy, I presume.
> > 
> > I believe you are right. The block layer, and then the fs would copy in.
> > > 
> > > I don't see how netback comes into it at all.
> > > 
> > > But maybe I am just confused or ignorant!  Please do explain :-).
> > 
> > s/back/frontend/  
> > 
> > My fear was refcounting.
> > 
> > Specifically where we do not do copying. For example, you could
> > be sending data from the NVDIMM GFNs (scp?) to some other location
> > (another host?). It would go over the xen-netback (in the dom0)
> > - which would then grant map it (dom0 would).
> >
> 
> Thanks for the explanation!
> 
> It means NVDIMM is very possibly mapped in page granularity, and
> hypervisor needs per-page data structures like page_info (rather than the
> range set style nvdimm_pages) to manage those mappings.

I do not know. I figured you need some accounting in the hypervisor
as the pages can be grant mapped but I don't know the intricate details
of the P2M code to tell you for certain.

[edit: Your later email seems to imply that you do not need all this
information? Just ranges?]
> 
> Then we will face the problem that the potentially huge number of
> per-page data structures may not fit in the normal ram. Linux kernel
> developers came across the same problem, and their solution is to
> reserve an area of NVDIMM and put the page structures in the reserved
> area (https://lwn.net/Articles/672457/). I think we may take the similar
> solution:
> (1) Dom0 Linux kernel reserves an area on each NVDIMM for Xen usage
> (besides the one used by Linux kernel itself) and reports the address
> and size to Xen hypervisor.
> 
> Reasons to choose Linux kernel to make the reservation include:
> (a) only Dom0 Linux kernel has the NVDIMM driver,
> (b) make it flexible for Dom0 Linux kernel to handle all
> reservations (for itself and Xen).
> 
> (2) Then Xen hypervisor builds the page structures for NVDIMM pages and
> stores them in above reserved areas.
> 
> (3) The reserved area is used as volatile, i.e. above two steps must be
> done for every host boot.
> 
> > In effect Xen there are two guests (dom0 and domU) pointing in the
> > P2M to the same GPFN. And that would mean:
> > 
> > > > > >(b) never map idx corresponding to GFNs occupied by vNVDIMM
> > 
> > Granted the XENMAPSPACE_gmfn happens _before_ the grant mapping is done
> > so perhaps this is not an issue?
> > 
> > The other situation I was envisioning - where the driver domain has
> > the NVDIMM passed in, and as well SR-IOV network card and functions
> > as an iSCSI target. That should work OK as we just need the IOMMU
> > to have the NVDIMM GPFNs programmed in.
> >
> 
> For this IOMMU usage example and above granted pages example, there
> remains one question: who is responsible to perform NVDIMM flush
> (clwb/clflushopt/pcommit)?


> 
> For the granted page example, if a NVDIMM page is granted to
> xen-netback, does the hypervisor need to tell xen-netback it's a NVDIMM
> page so that xen-netback can perform proper flush when it writes to that
> page? Or we may keep the NVDIMM transparent to xen-netback, and let Xen
> perform the flush when xen-netback gives up the granted NVDIMM page?
> 
> For the IOMMU example, my understanding is that there is a piece of
> software in the driver doma

Re: [Xen-devel] [PATCH 1/4] ns16550: store pointer to config parameters for PCI

[Konrad Rzeszutek Wilk]

On Tue, Feb 23, 2016 at 04:28:18AM -0700, Jan Beulich wrote:
> Subsequent changes will want to use this pointer.
> 
> This makes the enable_ro structure member redundant, so it gets dropped
> at once.
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Konrad Rzeszutek Wilk 

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

Re: [Xen-devel] [PATCH 2/4] ns16550: enable Pericom controller support

[Konrad Rzeszutek Wilk]

> +[param_pericom_4port] = {
> +.base_baud = 921600,
> +.uart_offset = 8,
> +.reg_width = 1,
> +.fifo_size = 16,
> +.lsr_mask = UART_LSR_THRE,
> +.bar0 = 1,
> +.max_ports = 4,
> +},
> +[param_pericom_8port] = {
> +.base_baud = 921600,
> +.uart_offset = 8,
> +.reg_width = 1,
> +.fifo_size = 16,
> +.lsr_mask = UART_LSR_THRE,
> +.bar0 = 1,
> +.max_ports = 8,

Perhaps document that Xen can only access two of the ports? Unless we
expand the ns16550_com array of course.

> @@ -830,12 +899,11 @@ static int __init check_existence(struct
>  
>  #ifdef CONFIG_HAS_PCI
>  static int __init
> -pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int bar_idx)
> +pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>  {
>  u64 orig_base = uart->io_base;
>  unsigned int b, d, f, nextf, i;
>  
> -uart->io_base = 0;
>  /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
>  for ( b = skip_amt ? 1 : 0; b < 0x100; b++ )
>  {
> @@ -843,8 +911,10 @@ pci_uart_config(struct ns16550 *uart, bo
>  {
>  for ( f = 0; f < 8; f = nextf )
>  {
> +unsigned int bar_idx = 0, port_idx = idx;

s/port_idx/port/? or port_nr /?

>  uint32_t bar, bar_64 = 0, len, len_64;
> -u64 size;
> +u64 size = 0;
> +const struct ns16550_config_param *param = uart_param;
>  
>  nextf = (f || (pci_conf_read16(0, b, d, f, PCI_HEADER_TYPE) &
> 0x80)) ? f + 1 : 8;
> @@ -863,15 +933,38 @@ pci_uart_config(struct ns16550 *uart, bo
>  continue;
>  }
>  
> +/* Check for params in uart_config lookup table */
> +for ( i = 0; i < ARRAY_SIZE(uart_config); i++)

I am pretty sure I wrote this piece of code - could you fix the
Style on it please? The i++) please?
> +{
> +u16 vendor = pci_conf_read16(0, b, d, f, PCI_VENDOR_ID);
> +u16 device = pci_conf_read16(0, b, d, f, PCI_DEVICE_ID);
> +
> +if ( uart_config[i].vendor_id == vendor &&
> + uart_config[i].dev_id == device )
> +{
> +param += uart_config[i].param;
> +if ( !param->bar0 )
> +{
> +bar_idx = idx;
> +port_idx = 0;
> +}
> +break;
> +}
> +}
> +
> +if ( port_idx >= param->max_ports )
> +{
> +idx -= param->max_ports;
> +continue;

Could you add a comment about this? I understand it can detect if we are
using an AMT device with the 'com2=115200,8n1,amt' (which would be
invalid - AMT devices only have one IO PORT and there is only one of
them on the machine) we would skip over the found device and continue on..
Thought I don't understand why we want to decrease the idx value from one to 
zero?

Hmm, if it was some other PCI based serial card like:

01:05.0 Serial controller: NetMos Technology PCI 9835 Multi-I/O
Controller (rev 01) (prog-if 02 [16550])
Subsystem: LSI Logic / Symbios Logic Device 0001
Flags: medium devsel, IRQ 20
I/O ports at e050 [size=8]
I/O ports at e040 [size=8]
I/O ports at e030 [size=8]
I/O ports at e020 [size=8]
I/O ports at e010 [size=8]
I/O ports at e000 [size=16]

With 'com1=115200,8n1,pci' and 'com2=115200,8n1,pci' then the first loop
would find the device. The second loop would decrement idx (1) by 1 and
continue.. which would make it go search for another device.

I hadn't tested this patch on the above device but I believe it used
to work with the com1 and com2 going throught it - while with the new code
it won't?


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

Re: [Xen-devel] [PATCH 3/4] console: adjust IRQ initialization

[Konrad Rzeszutek Wilk]

On Tue, Feb 23, 2016 at 04:30:01AM -0700, Jan Beulich wrote:
> In order for a Xen internal PCI device driver to enable MSI on the
> device, we need another hook which the driver can use to create the IRQ
> (doing this in the init_preirq hook is too early, since IRQ code hasn't
> got initialized at that time yet, and doing it in init_postirq is too
> late because at least on x86 smp_intr_init() needs to know the IRQ
> number).
> 
> On x86 this additionally requires a slight ordering change to IRQ
> initialization, to facilitate calling the new hook between basic
> initialization and the call path leading to smp_intr_init().
> 
> Signed-off-by: Jan Beulich 

Reviewed-by: Konrad Rzeszutek Wilk 

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

Re: [Xen-devel] [PATCH v6 for Xen 4.7 1/4] xen: enable per-VCPU parameter settings for RTDS scheduler

[Chong Li]

On Mon, Mar 7, 2016 at 11:53 AM, Dario Faggioli
 wrote:
> On Mon, 2016-03-07 at 09:40 -0700, Jan Beulich wrote:
>> > > > On 07.03.16 at 17:28,  wrote:
>> > On Mon, Mar 7, 2016 at 6:59 AM, Jan Beulich 
>> > wrote:
>> > >
>> > > > @@ -1163,6 +1173,96 @@ rt_dom_cntl(
>> > > >
>> > > > +case XEN_DOMCTL_SCHEDOP_getvcpuinfo:
>> > > > +if ( guest_handle_is_null(op->u.v.vcpus) )
>> > > > +{
>> > > > +rc = -EINVAL;

>
>> > > > +{
>> > > > +rc = -EINVAL;
>> > > > +break;
>> > > > +}
>> > > > +
>> > > > +spin_lock_irqsave(&prv->lock, flags);
>> > > > +svc = rt_vcpu(d->vcpu[local_sched.vcpuid]);
>> > > > +local_sched.s.rtds.budget = svc->budget /
>> > > > MICROSECS(1);
>> > > > +local_sched.s.rtds.period = svc->period /
>> > > > MICROSECS(1);
>> > > > +spin_unlock_irqrestore(&prv->lock, flags);
>> > > > +
>> > > > +if ( __copy_to_guest_offset(op->u.v.vcpus, index,
>> > > > +&local_sched, 1) )
>> > > > +{
>> > > > +rc = -EFAULT;
>> > > > +break;
>> > > > +}
>> > > > +if ( (++index > 0x3f) && hypercall_preempt_check()
>> > > > )
>> > > > +break;

>
>> > > > +}
>> > > > +
>> > > > +if ( !rc && (op->u.v.nr_vcpus != index) )
>> > > > +op->u.v.nr_vcpus = index;
>> > > I don't think the right side of the && is really necessary /
>> > > useful.
>> > The right side is to check whether the vcpus array is fully
>> > processed.
>> > When it is true and no error occurs (rc == 0), we
>> > update op->u.v.nr_vcpus, which is returned to libxc, and helps xc
>> > function figuring out how many un-processed vcpus should
>> > be taken care of in the next hypercall.
>> Just consider what the contents of op->u.v.nr_vcpus is after
>> this piece of code was executed, once with the full conditional,
>> and another time with the right side of the && omitted.
>>
> BTW, Chong, I'm not sure this has to do with what Jan is saying, but
> looking again at XEN_SYSCTL_pcitopoinfo, it looks to me you're missing
> copying nr_vcpus back up to the guest (which is actually what makes
> libxc knows whether all vcpus have been processed or now).

I think by "op->u.v.nr_vcpus = index", we already make the new nr_vcpus
seen by the guest (I've verified it).

In the case XEN_DOMCTL_scheduler_op of do_domctl(),
we make "copyback = 1" after calling sched_adjust(), which means all
fields in op (including
the new nr_vcpus) will be copied to u_domctl (at the end of
do_domctl()). This operation
ensures the new nr_vcpus is copied back up to the guest.

Please correct me if my understanding is wrong.

Chong

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



-- 
Chong Li
Department of Computer Science and Engineering
Washington University in St.louis

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

[Xen-devel] [xen-unstable test] 85628: tolerable FAIL

[osstest service owner]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds15 guest-start/debian.repeat fail blocked in 85533
 build-i386-rumpuserxen6 xen-buildfail   like 85533
 build-amd64-rumpuserxen   6 xen-buildfail   like 85533
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 85533
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 85533
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 85533

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-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   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-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass

version targeted for testing:
 xen  1bd52e1fd66c47af690124d74d11ccb271c96f6b
baseline version:
 xen  1bd52e1fd66c47af690124d74d11ccb271c96f6b

Last test of basis85628  2016-03-07 05:37:15 Z0 days
Testing same since0  1970-01-01 00:00:00 Z 16867 days0 attempts

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-oldkern  pass
 build-i386-oldkern   pass
 build-amd64-prev pass
 build-i386-prev  pass
 build-amd64-pvopspass
 build-armhf-pvops

[Xen-devel] [RESEND][PATCH V16 1/6] libxl: export some functions for pvusb use

[Chunyan Liu]

Signed-off-by: Chunyan Liu 
Signed-off-by: Simon Cao 
Reviewed-by: Wei Liu 
Acked-by: Ian Jackson 
---
 tools/libxl/libxl.c  | 5 ++---
 tools/libxl/libxl_internal.h | 3 +++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 833fd40..2ac9c0f 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1966,7 +1966,7 @@ out:
 }
 
 /* common function to get next device id */
-static int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device)
+int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device)
 {
 char *dompath, **l;
 unsigned int nb;
@@ -1985,8 +1985,7 @@ static int libxl__device_nextid(libxl__gc *gc, uint32_t 
domid, char *device)
 return nextid;
 }
 
-static int libxl__resolve_domid(libxl__gc *gc, const char *name,
-uint32_t *domid)
+int libxl__resolve_domid(libxl__gc *gc, const char *name, uint32_t *domid)
 {
 if (!name)
 return 0;
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index cb9790b..9c8519a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1180,6 +1180,9 @@ _hidden int libxl__init_console_from_channel(libxl__gc 
*gc,
  libxl__device_console *console,
  int dev_num,
  libxl_device_channel *channel);
+_hidden int libxl__device_nextid(libxl__gc *gc, uint32_t domid, char *device);
+_hidden int libxl__resolve_domid(libxl__gc *gc, const char *name,
+ uint32_t *domid);
 
 /*
  * For each aggregate type which can be used as an input we provide:
-- 
2.1.4


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

[Xen-devel] [RESEND][PATCH V16 3/6] refactor DEFINE_DEVICE_REMOVE to fit for more device types

[Chunyan Liu]

For some device type, device removal operation needs to be
handled specially, like usbctrl, it needs to remove all usb
devices under it first, then remove usbctrl. Extend
DEFINE_DEVICE_REMOVE to support generic and custom way
For those need to be handled specially, call
DEFINE_DEVICE_REMOVE_CUSTOM, it requires user defined
libxl__initiate_device_##type##_remove. Otherwise, just
call DEFINE_DEVICE_REMOVE as before.

Signed-off-by: George Dunlap 
Signed-off-by: Chunyan Liu 
---
 tools/libxl/libxl.c  | 18 +-
 tools/libxl/libxl_device.c   | 10 +-
 tools/libxl/libxl_internal.h |  4 ++--
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 2ac9c0f..2ab5ad3 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3134,7 +3134,7 @@ void libxl__device_disk_local_initiate_detach(libxl__egc 
*egc,
 aodev->dev = device;
 aodev->callback = local_device_detach_cb;
 aodev->force = 0;
-libxl__initiate_device_remove(egc, aodev);
+libxl__initiate_device_generic_remove(egc, aodev);
 return;
 }
 
@@ -4103,7 +4103,7 @@ out:
  * libxl_device_vfb_remove
  * libxl_device_vfb_destroy
  */
-#define DEFINE_DEVICE_REMOVE(type, removedestroy, f)\
+#define DEFINE_DEVICE_REMOVE_EXT(type, remtype, removedestroy, f)\
 int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,   \
 uint32_t domid, libxl_device_##type *type,  \
 const libxl_asyncop_how *ao_how)\
@@ -4123,13 +4123,19 @@ out:
 aodev->dev = device;\
 aodev->callback = device_addrm_aocomplete;  \
 aodev->force = f;   \
-libxl__initiate_device_remove(egc, aodev);  \
+libxl__initiate_device_##remtype##_remove(egc, aodev);  \
 \
 out:\
-if (rc) return AO_CREATE_FAIL(rc);\
+if (rc) return AO_CREATE_FAIL(rc);  \
 return AO_INPROGRESS;   \
 }
 
+#define DEFINE_DEVICE_REMOVE(type, removedestroy, f) \
+DEFINE_DEVICE_REMOVE_EXT(type, generic, removedestroy, f)
+
+#define DEFINE_DEVICE_REMOVE_CUSTOM(type, removedestroy, f)  \
+DEFINE_DEVICE_REMOVE_EXT(type, type, removedestroy, f)
+
 /* Define all remove/destroy functions and undef the macro */
 
 /* disk */
@@ -4158,6 +4164,8 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
  * 2. dynamically add/remove qemu chardevs via qmp messages. */
 
 #undef DEFINE_DEVICE_REMOVE
+#undef DEFINE_DEVICE_REMOVE_CUSTOM
+#undef DEFINE_DEVICE_REMOVE_EXT
 
 
/**/
 
@@ -4362,7 +4370,7 @@ static int remove_device(libxl__egc *egc, libxl__ao *ao,
 aodev->dev = dev;
 aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
 aodev->callback = device_complete;
-libxl__initiate_device_remove(egc, aodev);
+libxl__initiate_device_generic_remove(egc, aodev);
 break;
 case LIBXL__DEVICE_KIND_QDISK:
 if (--dguest->num_qdisks == 0) {
diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index 8bb5e93..a356e2a 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -676,7 +676,7 @@ void libxl__devices_destroy(libxl__egc *egc, 
libxl__devices_remove_state *drs)
 aodev->action = LIBXL__DEVICE_ACTION_REMOVE;
 aodev->dev = dev;
 aodev->force = drs->force;
-libxl__initiate_device_remove(egc, aodev);
+libxl__initiate_device_generic_remove(egc, aodev);
 }
 }
 }
@@ -775,8 +775,8 @@ out:
 return;
 }
 
-void libxl__initiate_device_remove(libxl__egc *egc,
-   libxl__ao_device *aodev)
+void libxl__initiate_device_generic_remove(libxl__egc *egc,
+   libxl__ao_device *aodev)
 {
 STATE_AO_GC(aodev->ao);
 xs_transaction_t t = 0;
@@ -806,7 +806,7 @@ void libxl__initiate_device_remove(libxl__egc *egc,
 (info.paused || info.dying || info.shutdown)) {
 /*
  * TODO: 4.2 Bodge due to QEMU, see comment on top of
- * libxl__initiate_device_remove in libxl_internal.h
+ * libxl__initiate_device_generic_remove in libxl_internal.h
  */
 rc = libxl__ev_time_register_rel(ao, &aodev->timeout,
  device_qemu_timeout,
@@ -942,7 +942,7 @@ static void device_backend_callback(libxl__egc *egc, 
libxl__ev_devstate *ds,
 !aodev->force) {

[Xen-devel] [RESEND][PATCH V16 6/6] xl: add pvusb commands

[Chunyan Liu]

Add pvusb commands: usbctrl-attach, usbctrl-detach, usb-list,
usbdev-attach and usbdev-detach.

To attach a usb device to guest through pvusb, one could follow
following example:

 #xl usbctrl-attach test_vm version=1 ports=8

 #xl usb-list test_vm
 will show the usb controllers and port usage under the domain.

 #xl usbdev-attach test_vm hostbus=1 hostaddr=2
 will find the first usable controller:port, and attach usb
 device whose busnum is 1 and devnum is 6.
 One could also specify which  and which .

 #xl usbdev-detach test_vm 0 1
 will detach USB device under controller 0 port 1.

 #xl usbctrl-detach test_vm dev_id
 will destroy the controller with specified dev_id. Dev_id
 can be traced in usb-list info.

Signed-off-by: Chunyan Liu 
Signed-off-by: Simon Cao 
Reviewed-by: George Dunlap 
---
 docs/man/xl.pod.1 |  37 +
 tools/libxl/xl.h  |   5 ++
 tools/libxl/xl_cmdimpl.c  | 190 ++
 tools/libxl/xl_cmdtable.c |  25 ++
 4 files changed, 257 insertions(+)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 4279c7c..dc6213e 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1345,6 +1345,43 @@ List pass-through pci devices for a domain.
 
 =back
 
+=head1 USB PASS-THROUGH
+
+=over 4
+
+=item B I I
+
+Create a new USB controller in the domain specified by I,
+I describes the device to attach, using form
+C where B has the same
+meaning as the B description in the domain config file.
+See L for more information.
+
+=item B I I
+
+Destroy a USB controller from the specified domain.
+B is devid of the USB controller.
+
+=item B I I
+
+Hot-plug a new pass-through USB device to the domain specified by
+I, I describes the device to attach, using
+form C where B has the same
+meaning as the B description in the domain config file.
+See L for more information.
+
+=item B I I I
+
+Hot-unplug a previously assigned USB device from a domain.
+B and B is USB controller:port in guest
+where the USB device is attached to.
+
+=item B I
+
+List pass-through usb devices for a domain.
+
+=back
+
 =head1 TMEM
 
 =over 4
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index bdab125..309627a 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -92,6 +92,11 @@ int main_blockdetach(int argc, char **argv);
 int main_vtpmattach(int argc, char **argv);
 int main_vtpmlist(int argc, char **argv);
 int main_vtpmdetach(int argc, char **argv);
+int main_usbctrl_attach(int argc, char **argv);
+int main_usbctrl_detach(int argc, char **argv);
+int main_usbdev_attach(int argc, char **argv);
+int main_usbdev_detach(int argc, char **argv);
+int main_usblist(int argc, char **argv);
 int main_uptime(int argc, char **argv);
 int main_claims(int argc, char **argv);
 int main_tmem_list(int argc, char **argv);
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 3137918..a3610fc 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -3517,6 +3517,196 @@ int main_cd_insert(int argc, char **argv)
 return 0;
 }
 
+int main_usbctrl_attach(int argc, char **argv)
+{
+uint32_t domid;
+int opt, rc = 0;
+libxl_device_usbctrl usbctrl;
+
+SWITCH_FOREACH_OPT(opt, "", NULL, "usbctrl-attach", 1) {
+/* No options */
+}
+
+domid = find_domain(argv[optind++]);
+
+libxl_device_usbctrl_init(&usbctrl);
+
+for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
+if (parse_usbctrl_config(&usbctrl, *argv))
+return 1;
+}
+
+rc = libxl_device_usbctrl_add(ctx, domid, &usbctrl, 0);
+if (rc) {
+fprintf(stderr, "libxl_device_usbctrl_add failed.\n");
+rc = 1;
+}
+
+libxl_device_usbctrl_dispose(&usbctrl);
+return rc;
+}
+
+int main_usbctrl_detach(int argc, char **argv)
+{
+uint32_t domid;
+int opt, devid, rc;
+libxl_device_usbctrl usbctrl;
+
+SWITCH_FOREACH_OPT(opt, "", NULL, "usbctrl-detach", 2) {
+/* No options */
+}
+
+domid = find_domain(argv[optind]);
+devid = atoi(argv[optind+1]);
+
+libxl_device_usbctrl_init(&usbctrl);
+if (libxl_devid_to_device_usbctrl(ctx, domid, devid, &usbctrl)) {
+fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
+return 1;
+}
+
+rc = libxl_device_usbctrl_remove(ctx, domid, &usbctrl, 0);
+if (rc) {
+fprintf(stderr, "libxl_device_usbctrl_remove failed.\n");
+rc = 1;
+}
+
+libxl_device_usbctrl_dispose(&usbctrl);
+return rc;
+
+}
+
+int main_usbdev_attach(int argc, char **argv)
+{
+uint32_t domid;
+int opt, rc;
+libxl_device_usbdev usbdev;
+
+SWITCH_FOREACH_OPT(opt, "", NULL, "usbdev-attach", 2) {
+/* No options */
+}
+
+libxl_device_usbdev_init(&usbdev);
+
+domid = find_domain(argv[optind++]);
+
+for (argv += optind, argc -= optind; argc > 0; ++argv, --argc) {
+if (parse_usbdev_config(&usbdev, *argv))
+return 1;
+}
+
+rc = libxl_device_usbdev_add(ctx, 

[Xen-devel] [RESEND][PATCH V16 2/6] libxl_utils: add internal function to read sysfs file contents

[Chunyan Liu]

Add a new function libxl_read_sysfs_file_contents to handle sysfs file
specially. It would be used in later pvusb work.

Signed-off-by: Chunyan Liu 
Acked-by: Ian Jackson 
---
 tools/libxl/libxl_internal.h |  4 +++
 tools/libxl/libxl_utils.c| 74 
 2 files changed, 78 insertions(+)

diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 9c8519a..429ea32 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -4078,6 +4078,10 @@ void libxl__bitmap_copy_best_effort(libxl__gc *gc, 
libxl_bitmap *dptr,
 
 int libxl__count_physical_sockets(libxl__gc *gc, int *sockets);
 
+_hidden int libxl__read_sysfs_file_contents(libxl__gc *gc,
+const char *filename,
+void **data_r,
+int *datalen_r);
 
 #define LIBXL_QEMU_USER_PREFIX "xen-qemuuser"
 #define LIBXL_QEMU_USER_BASE   LIBXL_QEMU_USER_PREFIX"-domid"
diff --git a/tools/libxl/libxl_utils.c b/tools/libxl/libxl_utils.c
index 672d3f8..b0cb9e1 100644
--- a/tools/libxl/libxl_utils.c
+++ b/tools/libxl/libxl_utils.c
@@ -396,6 +396,80 @@ int libxl_read_file_contents(libxl_ctx *ctx, const char 
*filename,
 return e;
 }
 
+int libxl__read_sysfs_file_contents(libxl__gc *gc, const char *filename,
+void **data_r, int *datalen_r)
+{
+FILE *f = 0;
+uint8_t *data = 0;
+int datalen = 0;
+int e;
+struct stat stab;
+ssize_t rs;
+
+f = fopen(filename, "r");
+if (!f) {
+if (errno == ENOENT) return ENOENT;
+LOGE(ERROR, "failed to open %s", filename);
+goto xe;
+}
+
+if (fstat(fileno(f), &stab)) {
+LOGE(ERROR, "failed to fstat %s", filename);
+goto xe;
+}
+
+if (!S_ISREG(stab.st_mode)) {
+LOGE(ERROR, "%s is not a plain file", filename);
+errno = ENOTTY;
+goto xe;
+}
+
+if (stab.st_size > INT_MAX) {
+LOG(ERROR, "file %s is far too large", filename);
+errno = EFBIG;
+goto xe;
+}
+
+datalen = stab.st_size;
+
+if (stab.st_size && data_r) {
+data = libxl__malloc(gc, datalen);
+
+/* For sysfs file, datalen is always PAGE_SIZE. 'read'
+ * will return the number of bytes of the actual content,
+ * rs <= datalen is expected.
+ */
+rs = fread(data, 1, datalen, f);
+if (rs < datalen) {
+if (ferror(f)) {
+LOGE(ERROR, "failed to read %s", filename);
+goto xe;
+}
+
+datalen = rs;
+data = libxl__realloc(gc, data, datalen);
+}
+}
+
+if (fclose(f)) {
+f = 0;
+LOGE(ERROR, "failed to close %s", filename);
+goto xe;
+}
+
+if (data_r) *data_r = data;
+if (datalen_r) *datalen_r = datalen;
+
+return 0;
+
+ xe:
+e = errno;
+assert(e != ENOENT);
+if (f) fclose(f);
+return e;
+}
+
+
 #define READ_WRITE_EXACTLY(rw, zero_is_eof, constdata)\
   \
   int libxl_##rw##_exactly(libxl_ctx *ctx, int fd, \
-- 
2.1.4


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

[Xen-devel] [RESEND][PATCH V16 0/6] xen pvusb toolstack work

[Chunyan Liu]

This patch series is to add pvusb toolstack work, supporting hot add|remove
USB device to|from guest and specify USB device in domain configuration file.

RESEND to remove a incorrect rc in 4/6:
+out:
+path = GCSPRINTF(USBBACK_INFO_PATH "/%s", usbdev_encode);
+rc = libxl__xs_rm_checked(gc, XBT_NULL, path);
'rc' should be removed here.
+return rc;

Sorry for trouble you.

Changes to V15:
* address George's comments (patch 4/6)

V15:
http://lists.xen.org/archives/html/xen-devel/2016-03/msg00040.html

V14:
http://lists.xenproject.org/archives/html/xen-devel/2016-02/msg02745.html

V13:
http://lists.xenproject.org/archives/html/xen-devel/2016-01/msg02125.html

V12:
http://lists.xen.org/archives/html/xen-devel/2015-12/msg02697.html

V11:
http://lists.xen.org/archives/html/xen-devel/2015-12/msg01626.html

V10:
http://lists.xen.org/archives/html/xen-devel/2015-12/msg01172.html

V9:
http://lists.xen.org/archives/html/xen-devel/2015-11/msg02744.html

V8:
http://lists.xen.org/archives/html/xen-devel/2015-10/msg02178.html

V7:
http://lists.xen.org/archives/html/xen-devel/2015-09/msg03115.html

V6:
http://lists.xen.org/archives/html/xen-devel/2015-08/msg00750.html

V5:
http://lists.xen.org/archives/html/xen-devel/2015-06/msg04052.html

V4:
http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg01327.html

Related Discussion Threads:
http://www.redhat.com/archives/libvir-list/2014-June/msg00038.html
http://lists.xen.org/archives/html/xen-devel/2014-06/msg00086.html

  <<< pvusb work introduction >>>

1. Overview

There are two general methods for passing through individual host
devices to a guest. The first is via an emulated USB device
controller; the second is PVUSB.

Additionally, there are two ways to add USB devices to a guest: via
the config file at domain creation time, and via hot-plug while the VM
is running.

* Emulated USB

In emulated USB, the device model (qemu) presents an emulated USB
controller to the guest. The device model process then grabs control
of the device from domain 0 and and passes the USB commands between
the guest OS and the host USB device.

This method is only available to HVM domains, and is not available for
domains running with device model stubdomains.

* PVUSB

PVUSB uses a paravirtialized front-end/back-end interface, similar to
the traditional Xen PV network and disk protocols. In order to use
PVUSB, you need usbfront in your guest OS, and usbback in dom0 (or
your USB driver domain).

2. Specifying a host USB device

QEMU qmp commands allows USB devices to be specified either by their
bus address (in the form bus.device) or their device tag (in the form
vendorid:deviceid).

Each way of specifying has its advantages:

Specifying by device tag will always get the same device,
regardless of where the device ends up in the USB bus topology.
However, if there are two identical devices, it will not allow you to
specify which one.

Specifying by bus address will always allow you to choose a
specific device, even if you have duplicates. However, the bus address
may change depending on which port you plugged the device into, and
possibly also after a reboot.

To avoid duplication of vendorid:deviceid, we'll use bus address to
specify host USB device in xl toolstack.

You can use lsusb to list the USB devices on the system:

Bus 001 Device 003: ID 0424:2514 Standard Microsystems Corp. USB 2.0
Hub
Bus 003 Device 002: ID f617:0905
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
Bus 001 Device 004: ID 0424:2640 Standard Microsystems Corp. USB 2.0
Hub
Bus 001 Device 005: ID 0424:4060 Standard Microsystems Corp. Ultra
Fast Media Reader
Bus 001 Device 006: ID 046d:c016 Logitech, Inc. Optical Wheel Mouse

To pass through the Logitec mouse, for instance, you could specify
1.6 (remove leading zeroes).

Note: USB hubs can not be assigned to guest.

3. PVUSB toolstack

* Specify USB device in xl config file

You can just specify usb devices, like:
usbdev=['hostbus=1, hostaddr=6']

Then it will create a USB controller automatically and attach the USB
device to the first available USB controller:port.

or, you can explicitly specify usb controllers and usb devices, like:
usbctrl=['verison=1, ports=4', 'version=2, ports=8', ]
usbdev=['hostbus=1, hostaddr=6, controller=0, port=1']

Then it will create two USB controllers as you specified.
And if controller and port are specified in usb config, then it will
attach the USB device to that controller:port. About the controller
and port value:
Each USB controller has a index (or called devid) based on 0. The 1st
controller has index 0, the 2nd controller has index 1, ...
Under controller, each port has a port number based on 1. In above
configuration, the 1st controller will have port 1,2,3,4.

* Hot-Plug USB device

To attach a USB device, you should first create a USB controller.
e.g.
xl usbctrl-attach domain [version=1|2] [ports=value]
By default, it will create a USB2.0 controller with 8 ports.

Then yo

[Xen-devel] [RESEND][PATCH V16 5/6] domcreate: support pvusb in configuration file

[Chunyan Liu]

Add code to support pvusb in domain config file. One could specify
usbctrl and usb in domain's configuration file and create domain,
then usb controllers will be created and usb device would be attached
to guest automatically.

One could specify usb controllers and usb devices in config file
like this:
usbctrl=['version=2,ports=4', 'version=1, ports=4', ]
usbdev=['hostbus=2, hostaddr=1, controller=0,port=1', ]

Signed-off-by: Chunyan Liu 
Signed-off-by: Simon Cao 
Reviewed-by: George Dunlap 
Acked-by: Ian Jackson 
---
 docs/man/xl.cfg.pod.5|  84 +
 tools/libxl/libxl_create.c   |  73 +++--
 tools/libxl/libxl_device.c   |   4 ++
 tools/libxl/libxl_internal.h |   8 
 tools/libxl/xl_cmdimpl.c | 107 ++-
 5 files changed, 272 insertions(+), 4 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 56b1117..b156caa 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -722,6 +722,90 @@ Note this may be overridden by rdm_policy option in PCI 
device configuration.
 
 =back
 
+=item B
+
+Specifies the USB controllers created for this guest. Each
+B has the form C where:
+
+=over 4
+
+=item B
+
+Possible Bs are:
+
+=over 4
+
+=item B
+
+Specifies the usb controller type.  Currently only 'pv' and 'auto'
+are supported.
+
+=item B
+
+Specifies the usb controller version.  Possible values include
+1 (USB1.1) and 2 (USB2.0). Default is 2 (USB2.0).
+
+=item B
+
+Specifies the total ports of the usb controller. The maximum
+number is 31. Default is 8.
+
+USB controler ids start from 0.  In line with the USB spec, however,
+ports on a controller start from 1.
+
+E.g.
+usbctrl=["version=1,ports=4", "version=2,ports=8",]
+The first controller has:
+controller id = 0, and port 1,2,3,4.
+The second controller has:
+controller id = 1, and port 1,2,3,4,5,6,7,8.
+
+=back
+
+=back
+
+=item B
+
+Specifies the USB devices to be attached to the guest at boot. Each
+B has the form C where:
+
+=over 4
+
+=item B
+
+Possible Bs are:
+
+=over 4
+
+=item B
+
+Specifies USB device type. Currently only support 'hostdev'.
+
+=item B
+
+Specifies busnum of the USB device from the host perspective.
+
+=item B
+
+Specifies devnum of the USB device from the host perspective.
+
+=item B
+
+Specifies USB controller id, to which controller the USB device is attached.
+
+=item B
+
+Specifies USB port, to which port the USB device is attached. B
+is valid only when B is specified.
+
+=back
+
+If no controller is specified, an available controller:port combination
+will be used.  If there are no available controller:port options,
+a new controller will be created.
+
+=back
+
 =item B
 
 Specifies the host PCI devices to passthrough to this guest. Each 
B
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index f1028bc..61b5c01 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -733,6 +733,10 @@ static void domcreate_launch_dm(libxl__egc *egc, 
libxl__multidev *aodevs,
 
 static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev,
int ret);
+static void domcreate_attach_usbctrls(libxl__egc *egc,
+  libxl__multidev *multidev, int ret);
+static void domcreate_attach_usbdevs(libxl__egc *egc, libxl__multidev 
*multidev,
+ int ret);
 static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
  int ret);
 static void domcreate_attach_dtdev(libxl__egc *egc,
@@ -1401,13 +1405,13 @@ static void domcreate_attach_vtpms(libxl__egc *egc,
if (d_config->num_vtpms > 0) {
/* Attach vtpms */
libxl__multidev_begin(ao, &dcs->multidev);
-   dcs->multidev.callback = domcreate_attach_pci;
+   dcs->multidev.callback = domcreate_attach_usbctrls;
libxl__add_vtpms(egc, ao, domid, d_config, &dcs->multidev);
libxl__multidev_prepared(egc, &dcs->multidev, 0);
return;
}
 
-   domcreate_attach_pci(egc, multidev, 0);
+   domcreate_attach_usbctrls(egc, multidev, 0);
return;
 
 error_out:
@@ -1415,6 +1419,69 @@ error_out:
domcreate_complete(egc, dcs, ret);
 }
 
+static void domcreate_attach_usbctrls(libxl__egc *egc,
+  libxl__multidev *multidev, int ret)
+{
+libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
+STATE_AO_GC(dcs->ao);
+int domid = dcs->guest_domid;
+
+libxl_domain_config *const d_config = dcs->guest_config;
+
+if (ret) {
+LOG(ERROR, "unable to add vtpm devices");
+goto error_out;
+}
+
+if (d_config->num_usbctrls > 0) {
+/* Attach usbctrls */
+libxl__multidev_begin(ao, &dcs->multidev);
+dcs->multidev.callback = domcreate_attach_usbdevs;
+libxl__add_usbctrls(egc, ao, domid, d_config, &dcs->multidev);
+libxl__multide