Re: [Xen-devel] [Qemu-devel] Question about scsi emulation

2015-03-24 Thread Yaoli Zheng
Thank you for the advice!
It will be grateful if someone can provide any guide how to config  MegaSAS HBA 
or how to use xen pvscsi in XEN.
There seems no option in XEN for these two driver emulation and no document 
found online.

Thanks!

-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com] 
Sent: Monday, March 23, 2015 11:18 AM
To: Stefan Hajnoczi
Cc: Yaoli Zheng; qemu-de...@nongnu.org; xen-devel@lists.xen.org
Subject: Re: [Xen-devel] [Qemu-devel] Question about scsi emulation

On Mon, 23 Mar 2015, Stefan Hajnoczi wrote:
> On Wed, Mar 18, 2015 at 02:16:47PM -0700, Yaoli Zheng wrote:
> > We have problem using qemu emulated scsi driver(the old lsi). Wonder if any 
> > of other device model we can try for emulating scsi, and how we can get and 
> > config it in Xen? Having been told virtio-scsi is alternative one, but have 
> > no idea how to get it work in Xen.
> 
> The MegaSAS HBA might be another option.  Not sure whether MegaSAS or 
> virtio-scsi are supported under Xen though.
 
Making MegaSAS HBA work on Xen should be easy.

Rather than virtio-scsi I would consider the new Xen pvscsi frontend/backend 
drivers, now upstream in Linux (drivers/scsi/xen-scsifront.c and 
drivers/xen/xen-scsiback.c). Usually Xen PV drivers perform much better than 
virtio devices on Xen.

That said, probably most things would be better than the old lsi.

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


Re: [Xen-devel] [PATCH] x86: don't change affinity with interrupt unmasked

2015-03-24 Thread Jan Beulich
>>> On 23.03.15 at 19:46,  wrote:
> On Fri, Mar 20, 2015 at 03:40:02PM +, Jan Beulich wrote:
>> With ->startup unmasking the IRQ, setting the affinity afterwards
>> without masking the IRQ again is invalid namely for MSI (which can't
>> have their affinity updated atomically).
> 
> That took a bit of verification :-)
> 
> Could you include this in the commit please:
> 
> Per 6.8.3.5. Per-vector Masking and Function Masking from
> https://www.pcisig.com/specifications/conventional/msi-x_ecn.pdf 
> ".. anytime software unmasks a currently masked MSI-X
> Table entry either by clearing its Mask bit or by clearing the Function Mask 
> bit, the
> function must update any Address or Data values that it cached from that 
> entry. If
> software changes the Address or Data value of an entry while the entry is 
> unmasked, the 
> result is undefined."

I'd rather not, as that may result in the wrong impression that only
MSI-X is affected, while MSI is as much. Apart from the potential
caching is only one part of the problem, the other is (as said in the
description) the non-atomic nature of the address/data updates
for both variants of MSI. I guess I'll simply extend what is in
parentheses at the end to include the caching issue.

Jan


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


Re: [Xen-devel] [PATCH 2/2] x86/vMSI-X: add valid bits for read acceleration

2015-03-24 Thread Jan Beulich
>>> On 23.03.15 at 20:11,  wrote:
> On Fri, Mar 20, 2015 at 04:27:57PM +, Jan Beulich wrote:
>> Again because Xen doesn't get to see all guest writes, it shouldn't
>> serve reads from its cache before having seen a write to the respective
>> address.
>> 
>> Signed-off-by: Jan Beulich 
> 
> Reviewed-by: Konrad Rzeszutek Wilk 
> 
>> 
>> --- a/xen/arch/x86/hvm/vmsi.c
>> +++ b/xen/arch/x86/hvm/vmsi.c
>> @@ -153,12 +153,15 @@ struct msixtbl_entry
>>  /* TODO: resolve the potential race by destruction of pdev */
>>  struct pci_dev *pdev;
>>  unsigned long gtable;   /* gpa of msix table */
>> -unsigned long table_flags[BITS_TO_LONGS(MAX_MSIX_TABLE_ENTRIES)];
>> +DECLARE_BITMAP(table_flags, MAX_MSIX_TABLE_ENTRIES);
> 
> That seems unrelated to this patch? Perhaps mention the cleanup
> part in the commit.

Ah, yes, I meant to but forgot.

Jan


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


Re: [Xen-devel] PML (Page Modification Logging) design for Xen

2015-03-24 Thread Jan Beulich
>>> On 24.03.15 at 07:42,  wrote:

> 
> On 02/17/2015 06:19 PM, Jan Beulich wrote:
> On 12.02.15 at 03:39,  wrote:
>>> On 02/11/2015 07:52 PM, Andrew Cooper wrote:
 On 11/02/15 08:28, Kai Huang wrote:
> Design
> ==
>
> - PML feature is used globally
>
> A new Xen boot parameter, say 'opt_enable_pml', will be introduced to
> control PML feature detection, and PML feature will only be detected
> if opt_enable_pml = 1. Once PML feature is detected, it will be used
> for dirty logging for all domains globally. Currently we don't support
> to use PML on basis of per-domain as it will require additional
> control from XL tool.
 Rather than adding in a new top level command line option for an ept
 subfeature, it would be preferable to add an "ept=" option which has
 "pml" as a sub boolean.
>>> Which is good to me, if Jan agrees.
>>>
>>> Jan, which do you prefer here?
>> A single "ept=" option as Andrew suggested.
> Hi Andrew, Jan, Tim,
> 
> Sorry to bring this thread back.
> 
> Regarding to the parameter to control PML, I plan to enable PML by 
> default, in which case would a "ept=no-pml" be more reasonable to 
> disable it manually?

Imo the default should be off at least initially. The command line
option parsing is (and should be) independent of the chosen
default anyway, i.e. overrides in either direction should be
possible.

> Actually by referring to "iommu=" parameter, I would like to do below 
> changes. Is it good to you?

Looks okay.

Jan


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


Re: [Xen-devel] PML (Page Modification Logging) design for Xen

2015-03-24 Thread Kai Huang



On 03/24/2015 03:53 PM, Jan Beulich wrote:

On 24.03.15 at 07:42,  wrote:

On 02/17/2015 06:19 PM, Jan Beulich wrote:

On 12.02.15 at 03:39,  wrote:

On 02/11/2015 07:52 PM, Andrew Cooper wrote:

On 11/02/15 08:28, Kai Huang wrote:

Design
==

- PML feature is used globally

A new Xen boot parameter, say 'opt_enable_pml', will be introduced to
control PML feature detection, and PML feature will only be detected
if opt_enable_pml = 1. Once PML feature is detected, it will be used
for dirty logging for all domains globally. Currently we don't support
to use PML on basis of per-domain as it will require additional
control from XL tool.

Rather than adding in a new top level command line option for an ept
subfeature, it would be preferable to add an "ept=" option which has
"pml" as a sub boolean.

Which is good to me, if Jan agrees.

Jan, which do you prefer here?

A single "ept=" option as Andrew suggested.

Hi Andrew, Jan, Tim,

Sorry to bring this thread back.

Regarding to the parameter to control PML, I plan to enable PML by
default, in which case would a "ept=no-pml" be more reasonable to
disable it manually?

Imo the default should be off at least initially.

OK.

  The command line
option parsing is (and should be) independent of the chosen
default anyway, i.e. overrides in either direction should be
possible.
While the parse_ept_param function does support "ept=pml" and 
"ept=no-pml" both, I think in the comments of the function we should 
explicitly tell whether to use "ept=pml" (in case PML is off by 
default), or "ept=no-pml" (in case PML is on by default), otherwise 
"ept=pml,no-pml" is legal but obviously it doesn't make any sense (and 
looks this issue also exists in parse_iommu_param?).


Thanks,
-Kai



Actually by referring to "iommu=" parameter, I would like to do below
changes. Is it good to you?

Looks okay.

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 2/2] x86: Don't use BAD_APICID for non-APICID fields

2015-03-24 Thread Jan Beulich
>>> On 23.03.15 at 20:54,  wrote:
> BAD_APICID is used by cpuinfo_x86's phys_proc_id, cpu_core_id
> and compute_unit_id even though these fields don't hold an APIC ID
> itself but rather its derivative.
> 
> Provide appropriate macros for each of those three (and make them
> unsigned).
> 
> This also fixes regression introduced by commit 2090f14c5cbd ("sysctl:
> make XEN_SYSCTL_topologyinfo sysctl a little more efficient") which
> leaked BAD_APICID into common code, breaking ARM.
> 
> Signed-off-by: Boris Ostrovsky 
> Reported-by: Julien Grall 
> ---
> 
> I left sysctl with
> 
> cputopo.core = cpu_to_core(i);
> if ( cputopo.core == INVALID_COREID )
> cputopo.core = XEN_INVALID_CORE_ID;
> cputopo.socket = cpu_to_socket(i);
> if ( cputopo.socket == INVALID_SOCKETID )
> cputopo.socket = XEN_INVALID_SOCKET_ID;
> 
> since this is the only place that would use suggested inlines for external
> (public) calls.

But again - why did you not avoid the new #defines and use the
XEN_-prefixed ones right away?

> --- a/xen/include/asm-x86/smp.h
> +++ b/xen/include/asm-x86/smp.h
> @@ -16,7 +16,8 @@
>  #include 
>  #endif
>  
> -#define BAD_APICID -1U
> +#define BAD_APICID   -1U
> +#define INVALID_CUID (~0U)   /* AMD Compute Unit ID */

Touching the BAD_APICID line should have resulted in also adding
parentheses there.

Jan


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


Re: [Xen-devel] PML (Page Modification Logging) design for Xen

2015-03-24 Thread Jan Beulich
>>> On 24.03.15 at 09:06,  wrote:
> On 03/24/2015 03:53 PM, Jan Beulich wrote:
>>   The command line
>> option parsing is (and should be) independent of the chosen
>> default anyway, i.e. overrides in either direction should be
>> possible.
> While the parse_ept_param function does support "ept=pml" and 
> "ept=no-pml" both, I think in the comments of the function we should 
> explicitly tell whether to use "ept=pml" (in case PML is off by 
> default), or "ept=no-pml" (in case PML is on by default), otherwise 
> "ept=pml,no-pml" is legal but obviously it doesn't make any sense (and 
> looks this issue also exists in parse_iommu_param?).

While "ept=pml,no-pml" makes little sense, there's nothing wrong
with allowing it. "ept=pml ept=no-pml" may in fact make sense,
when wanting to override a setting e.g. in an EFI config file on
the command (or grub) line. IOW don't lose time on preventing
non-sense option combinations if the resulting settings
nevertheless are valid / meaningful.

Jan


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


Re: [Xen-devel] PML (Page Modification Logging) design for Xen

2015-03-24 Thread Kai Huang



On 03/24/2015 04:14 PM, Jan Beulich wrote:

On 24.03.15 at 09:06,  wrote:

On 03/24/2015 03:53 PM, Jan Beulich wrote:

   The command line
option parsing is (and should be) independent of the chosen
default anyway, i.e. overrides in either direction should be
possible.

While the parse_ept_param function does support "ept=pml" and
"ept=no-pml" both, I think in the comments of the function we should
explicitly tell whether to use "ept=pml" (in case PML is off by
default), or "ept=no-pml" (in case PML is on by default), otherwise
"ept=pml,no-pml" is legal but obviously it doesn't make any sense (and
looks this issue also exists in parse_iommu_param?).

While "ept=pml,no-pml" makes little sense, there's nothing wrong
with allowing it. "ept=pml ept=no-pml" may in fact make sense,
when wanting to override a setting e.g. in an EFI config file on
the command (or grub) line. IOW don't lose time on preventing
non-sense option combinations if the resulting settings
nevertheless are valid / meaningful.

Hmm. Reasonable indeed. Thanks.

Thanks,
-Kai


Jan




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


Re: [Xen-devel] [libvirt] libvirt/libxl implemetation of get_online_cpu / virNodeGetCPUMap?

2015-03-24 Thread Manish Jaggi


On Tuesday 24 February 2015 08:30 PM, Anthony PERARD wrote:

On Tue, Feb 24, 2015 at 01:22:19PM +, Daniel P. Berrange wrote:

On Tue, Feb 24, 2015 at 01:15:57PM +, Anthony PERARD wrote:

On Tue, Feb 24, 2015 at 12:46:44PM +, Daniel P. Berrange wrote:

On Tue, Feb 24, 2015 at 12:41:01PM +, Anthony PERARD wrote:

Hi,

A recent OpenStack nova commit make use of virNodeGetCPUMap to get the list
of online cpu of a host. But this API is not implemented for the libvirt
xen driver.

The commit:
   Add handling for offlined CPUs to the nova libvirt driver.
https://review.openstack.org/gitweb?p=openstack/nova.git;a=commitdiff;h=0696a5cd5f0fdc08951a074961bb8ce0c3310086

FWIW, this should not impact Xen based on my understanding. The code
path in question should only be used when Nova is setup todo NUMA
pinning support, and that is not supported with Xen in OpenStack,
only KVM.  Did it actually cause failures for you, or are you simply
keeping track of all used APIs in Nova as a sanity check ?

It prevent nova from starting. I do the setup with DevStack.

The error:
libvirtError: this function is not supported by the connection driver: 
virNodeGetCPUMap

And a part of the traceback:
   File "/opt/stack/nova/nova/openstack/common/service.py", line 491, in 
run_service
 service.start()
   File "/opt/stack/nova/nova/service.py", line 181, in start
 self.manager.pre_start_hook()
   File "/opt/stack/nova/nova/compute/manager.py", line 1188, in pre_start_hook
 self.update_available_resource(nova.context.get_admin_context())
   File "/opt/stack/nova/nova/compute/manager.py", line 6062, in 
update_available_resource
 rt.update_available_resource(context)
   File "/opt/stack/nova/nova/compute/resource_tracker.py", line 315, in 
update_available_resource
 resources = self.driver.get_available_resource(self.nodename)
   File "/opt/stack/nova/nova/virt/libvirt/driver.py", line 4896, in 
get_available_resource
 numa_topology = self._get_host_numa_topology()
   File "/opt/stack/nova/nova/virt/libvirt/driver.py", line 4749, in 
_get_host_numa_topology
 online_cpus = self._host.get_online_cpus()
   File "/opt/stack/nova/nova/virt/libvirt/host.py", line 599, in 
get_online_cpus
 (cpus, cpu_map, online) = self.get_connection().getCPUMap()

I'll look into why nova is going through NUMA code paths then.

Oh damn, yes, I understand why now. Please file a bug against Nova for
this, as we must fix it as a high pripority. It was certainly not my
intention to break Xen when I approved this change
I applied the patch, not getting this python libvirtError, but libvirt 
deamon is throwing error.
2015-03-24 08:46:31.169+: 1030: error : virNodeGetCPUMap:1342 : this 
function is not supported by the connection driver: virNodeGetCPUMap



Here is the bug report: https://bugs.launchpad.net/nova/+bug/1425115

Regards,




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


[Xen-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

2015-03-24 Thread Chen, Tiejun

All guys,

Sorry to bother you.

I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and 
tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like 
pyxc_methods[] and pyxl_methods[]? And how should we call these approaches?


In my specific case, I'm trying to introduce a new flag to each a device 
while assigning device. So this means I have to add a parameter, 'flag', 
into


int xc_assign_device(
xc_interface *xch,
uint32_t domid,
uint32_t machine_sbdf)

Then this is extended as

int xc_assign_device(
xc_interface *xch,
uint32_t domid,
uint32_t machine_sbdf,
uint32_t flag)

After this introduction, obviously I should cover all cases using 
xc_assign_device(). And also I found this fallout goes into these two 
files. For example, here pyxc_assign_device() is involved. Currently it 
has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str' 
should represent all pci devices with SBDF format, right?


But I don't know exactly what rule should be complied to construct this 
sort of flag into 'pci_str', or any reasonable idea to achieve my goal?


Thanks
Tiejun

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


Re: [Xen-devel] Xen crash with mem-sharing and cloning

2015-03-24 Thread Tamas K Lengyel
On Tue, Mar 24, 2015 at 4:54 AM, Andres Lagar Cavilla <
and...@lagarcavilla.org> wrote:

>
>
> On Mon, Mar 23, 2015 at 11:25 AM, Tamas K Lengyel  > wrote:
>
>> On Mon, Mar 23, 2015 at 6:59 PM, Andres Lagar Cavilla <
>> and...@lagarcavilla.org> wrote:
>>
>>> On Mon, Mar 23, 2015 at 9:10 AM, Tamas K Lengyel <
>>> tkleng...@sec.in.tum.de> wrote:
>>>
 Hello everyone,
 I'm trying to chase down a bug that reproducibly crashes Xen (tested
 with 4.4.1). The problem is somewhere within the mem-sharing subsystem and
 how that interacts with domains that are being actively saved. In my setup
 I use the xl toolstack to rapidly create clones of HVM domains by piping
 "xl save -c" into xl restore with a modified domain config which updates
 the name/disk/vif. However, during such an operation Xen crashes with the
 following log if there are already active clones.

 IMHO there should be no conflict between saving the domain and
 memsharing, as long as the domain is actually just being checkpointed "-c"
 - it's memory should remain as is. This is however clearly not the case.
 Any ideas?

>>>
>>> Tamas, I'm not clear on the use of memsharing in this workflow. As
>>> described, you pipe save into restore, but the internal magic is lost on
>>> me. Are you fanning out to multiple restores? That would seem to be the
>>> case, given the need to update name/disk/vif.
>>>
>>> Anyway, I'm inferring. Instead, could you elaborate?
>>>
>>> Thanks
>>> Andre
>>>
>>
>> Hi Andre,
>> thanks for getting back on this issue. The script I'm using is at
>> https://github.com/tklengyel/drakvuf/blob/master/tools/clone.pl. The
>> script simply creates a FIFO pipe (mkfifo) and saves the domain into that
>> pipe which is immediately read by xl restore with the updated configuration
>> file. This mainly just to eliminate having to read the memory dump from
>> disk. That part of the system works as expected and multiple save/restores
>> running at the same time don't cause any side-effects. Once the domain has
>> thus been cloned, I run memshare on every page which also works as
>> expected. This problem only occurs when the cloning procedure runs when a
>> page unshare operation kicks in on a already active clone (as you see in
>> the log).
>>
>
> Sorry Tamas, I'm a bit slow here, I looked at your script -- looks
> allright, no mention of memsharing in there.
>
> Re-reading ... memsharing? memshare? Is this memshrtool in tools/testing?
> How are you running it?
>


Hi Andre,
the memsharing happens here
https://github.com/tklengyel/drakvuf/blob/master/src/main.c#L144 after the
clone script finished. This is effectively the same approach as in
tools/testing, just automatically looping from 0 to max_gpfn. Afterwards
all unsharing happens automatically either induced by the guest itself, or
when I map pages into the my app with xc_map_foreign_range PROT_WRITE.


>
> Certainly no xen crash should happen with user-space input. I'm just
> trying to understand what you're doing. The unshare code is not, uhmm,
> brief, so a NULL deref could happen in half a dozen places at first glance.
>

Well let me know what I could do help tracing it down. I don't think
(potentially buggy) userspace tools should crash Xen either =)

Tamas


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


Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

2015-03-24 Thread Wei Liu
On Tue, Mar 24, 2015 at 05:13:31AM +, Pang, LongtaoX wrote:
> 
> 
> > -Original Message-
> > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > Sent: Tuesday, March 24, 2015 1:37 AM
> > To: Wei Liu
> > Cc: Pang, LongtaoX; Hu, Robert; ian.jack...@eu.citrix.com;
> > xen-devel@lists.xen.org
> > Subject: Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some
> > testsupport APIs
> > 
> > On Mon, 2015-03-23 at 17:29 +, Wei Liu wrote:
> > > On Mon, Mar 23, 2015 at 04:45:55PM +, Ian Campbell wrote:
> > > > On Mon, 2015-03-23 at 16:20 +, Pang, LongtaoX wrote:
> > > > > >
> > > > > > > > > The editconfig_cd thing -- yet another thing which Ian
> > > > > > > > > questioned and which it was agreed you would change but you
> > haven't.
> > > > > > > > >
> > > > > > > > For this question, I have sent a mail about it.(2015-03-04)
> > > > > > > > After finishing L1 guest VM installation, we need to change
> > > > > > > > L1 guest boot sequence from ISO image to hard disk, we need
> > > > > > > > modify the "boot=cd" ,
> > > > > > >
> > > > > > > Do you? As Ian asked before, why is guest_editconfig_nocd  not
> > > > > > > sufficient? It removes the CD from the virtual drive, meaning
> > > > > > > that "boot=dc" will fail to boot from d and fallthru to c.
> > > > > > >
> > > > > > > >  also need to enable 'nestedhvm' feature in hvm configure
> > > > > > > > file,
> > > > > > >
> > > > > > > This certainly doesn't belong in a function called
> > > > > > > guest_editconfig_cd, since it has nothing to do with cds at all.
> > > > > > >
> > > > > > > Anyway, it's not clear why you need to edit this into the
> > > > > > > nestedhvm configuration, instead of adding it when the
> > > > > > > configuration is created via more_prepareguest_hvm. What harm
> > > > > > > is there in enabling this during guest install?
> > > > > > >
> > > > > > I will try it.
> > > > > >
> > > > > Re-use 'guest_ediconfig_nocd', after finishing L1 installation, it
> > > > > could boot into L1 OS, but failed to install packages( such as
> > > > > lvm2, rsync, bridge-utils ) via Debian repo in L1, as below msg:
> > > >
> > > > Oh dear. Things really ought to be tailored on install to use the
> > > > network repositories for the apt sources, not the cdrom.
> > >
> > > When I wrote ts-debian-hvm-install, one of the problems (if I remember
> > > correctly) was that our network infrastructure didn't support booting
> > > EFI from PXE boot. I ended up making that disk image to sort of work
> > > around this.
> > >
> > > >
> > > > Installing from netboot rather than netinst media ought to achieve
> > > > that, I'm not sure with ts-debian-hvm-install uses though or how to
> > > > achieve it via preseeding if it isn't the default for the given media.
> > > >
> > >
> > > Per  https://www.debian.org/releases/stable/example-preseed.txt,
> > > these runes look interesting.
> > >
> > > # Additional repositories, local[0-9] available #d-i
> > > apt-setup/local0/repository string \
> > > #   http://local.server/debian stable main
> > > #d-i apt-setup/local0/comment string local server # Enable deb-src
> > > lines #d-i apt-setup/local0/source boolean true # URL to the public
> > > key of the local repository; you must provide a key # or # apt will
> > > complain about the unauthenticated repository and so the #
> > > sources.list line will be left commented out #d-i apt-setup/local0/key
> > > string http://local.server/key
> > >
> > > Not sure if they will really end up in source.list though.
> > 
> > My expectation is that the existing preseed will have resulted in both http 
> > and
> > cdrom entries, and all that is needed is to comment out the cdrom ones so 
> > the
> > network ones take precedence.
> > 
> > Lets wait for an answer to my question about what is in sources.list on 
> > these
> > VMs before speculating further on how to fix this though.
> > 
> > Ian.
> I have checked the sources.list file in L1 guest, it contains both CDROM repo 
> entry and URL entry(Debian repository mirror location), 
> Such as below:
> deb cdrom:[Debian GNU/Linux 7.6.0 _Wheezy_ - Official amd64 DVD Binary-1 
> 20140712-14:11]/ wheezy contrib main
> deb http://linux-ftp.sh.intel.com//pub/mirrors/debian wheezy main
> deb-src http://linux-ftp.sh.intel.com//pub/mirrors/debian wheezy main
> 
> It seems that CDROM repo entry take effect, but it definitely unavailable, 
> because ISO image is removed.
> If I comment out the CDROM repo entry manually, and then try to 'apt-get 
> install', it works fine.
> For wei's first solution that change boot sequence from cd_disc to HDD, it 
> does works and I have created a 'guest_ediconfig_nocd' function about that in 
> previously patchs, maybe it's not preferred according to Ian Campbell's 
> opinion.

FWIW I think Ian's approach is better. Using same source to install
packages is better than using different sources.

> So, maybe I should write some code in 'ts-nested-setup' script to implement 
> ssh int

Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 05:13 +, Pang, LongtaoX wrote:
> So, maybe I should write some code in 'ts-nested-setup' script to
> implement ssh into L1, edit sources.list and comment out the CDROM
> repo entry.

Please do it in ts-debian-hvm-install, either as a postinst step or,
better (IMHO), by arranging for it to happen during preseeding using the
late_command infrastructure.

Ian.


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


Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 03:25 +, Hu, Robert wrote:

> Though no harm as far as I see now, I cannot forcast any harm in the
> future. Anyway, this 'nestedhvm' is for a nested usage. Better to enable
> it when we are actually going enable a nested environment.
> Practically, we can do it in either way. and the first is actually easier to 
> do.

One good thing about just enabling it from the start is that we get some
testing of normal guest installation while nestedhvm happens to be
installed, which might help catch regressions.

Ian.



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


Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 03:34 +, Hu, Robert wrote:

> > Make it an option to the function which creates the configuration in the
> > first place. Quoting myself from earlier in this very thread:

> Then we still need to designate to use e1000 device somewhere; say, in
> ts-nested-setup, we store this designation in runvar.
> Then when creating the config, according to your proposal, we plumbing
> such designation through $xopts, since we see it in runvar. Are you OK
> with such implementation?

I'm not sure if this should be a runvar or just something which
ts-nested-setup knows. That's one for Ian J to decide.

> But if like above, I would prefer to altering/manipulating guest configation 
> directly in ts-nested-setup after normal guest is setup. Since this is more
> simple and constrained to nested case only.

IMHO, this should be properly integrated into the configuration
creation, not fixed up later as an adhoc step.

Ian.


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


Re: [Xen-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:
> All guys,
> 
> Sorry to bother you.
> 
> I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and 
> tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like 
> pyxc_methods[] and pyxl_methods[]?

They are registered with the Python runtime, so they are called from
Python code. The first member of the struct is the pythonic function
name, e.g. from xc.c:
{ "domain_create", 
  (PyCFunction)pyxc_domain_create, 
  METH_VARARGS | METH_KEYWORDS, "\n"
  "Create a new domain.\n"
  " dom[int, 0]:Domain identifier to use (allocated if zero).\n"
  "Returns: [int] new domain identifier; -1 on error.\n" },
defines a method called domain_create, in the xen.lowlevel.xc namespace.

>  And how should we call these approaches?

I'm not sure what you are asking here.

> In my specific case, I'm trying to introduce a new flag to each a device 
> while assigning device. So this means I have to add a parameter, 'flag', 
> into
> 
> int xc_assign_device(
>  xc_interface *xch,
>  uint32_t domid,
>  uint32_t machine_sbdf)
> 
> Then this is extended as
> 
> int xc_assign_device(
>  xc_interface *xch,
>  uint32_t domid,
>  uint32_t machine_sbdf,
>  uint32_t flag)
> 
> After this introduction, obviously I should cover all cases using 
> xc_assign_device(). And also I found this fallout goes into these two 
> files. For example, here pyxc_assign_device() is involved. Currently it 
> has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str' 
> should represent all pci devices with SBDF format, right?

It appears so, yes.

> But I don't know exactly what rule should be complied to construct this 
> sort of flag into 'pci_str', or any reasonable idea to achieve my goal?

If it is non-trivial to fix them IMHO it is acceptable for the new
parameter to not be plumbed up to the Python bindings until someone
comes along with a requirement to use it from Python. IOW you can just
pass whatever the nop value is for the new argument.

Ian.


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


[Xen-devel] [ovmf test] 36590: tolerable FAIL - PUSHED

2015-03-24 Thread xen . org
flight 36590 ovmf real [real]
http://www.chiark.greenend.org.uk/~xensrcts/logs/36590/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-pair17 guest-migrate/src_host/dst_host fail like 36525

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel  9 guest-start  fail  never pass
 test-amd64-amd64-xl-pvh-amd   9 guest-start  fail   never pass
 test-amd64-i386-libvirt  10 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 10 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pcipt-intel  9 guest-start fail never pass
 test-amd64-i386-xl-qemut-win7-amd64 14 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-win7-amd64 14 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 14 guest-stop fail never pass
 test-amd64-i386-xl-winxpsp3  14 guest-stop   fail   never pass
 test-amd64-i386-xl-win7-amd64 14 guest-stop   fail  never pass
 test-amd64-i386-xl-qemuu-win7-amd64 14 guest-stop  fail never pass
 test-amd64-amd64-xl-win7-amd64 14 guest-stop   fail never pass
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 14 guest-stop fail never pass
 test-amd64-amd64-xl-winxpsp3 14 guest-stop   fail   never pass
 test-amd64-i386-xl-qemuu-winxpsp3 14 guest-stopfail never pass
 test-amd64-i386-xl-winxpsp3-vcpus1 14 guest-stop   fail never pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 14 guest-stop fail never pass
 test-amd64-i386-xl-qemut-winxpsp3 14 guest-stopfail never pass
 test-amd64-amd64-xl-qemut-winxpsp3 14 guest-stop   fail never pass
 test-amd64-amd64-xl-qemuu-winxpsp3  7 windows-install  fail never pass

version targeted for testing:
 ovmf f61762d0c86b42c7922ed2d4326c9647252752ae
baseline version:
 ovmf 0ac71f158b2da40493288342ad2c978c8a795a2f


People who touched revisions under test:
  Jeff Fan 
  Tyler Smith 


jobs:
 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  pass
 test-amd64-i386-xl   pass
 test-amd64-amd64-xl-pvh-amd  fail
 test-amd64-i386-rhel6hvm-amd pass
 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-ovmf-amd64 pass
 test-amd64-i386-xl-qemuu-ovmf-amd64  pass
 test-amd64-amd64-xl-qemut-win7-amd64 fail
 test-amd64-i386-xl-qemut-win7-amd64  fail
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  fail
 test-amd64-amd64-xl-win7-amd64   fail
 test-amd64-i386-xl-win7-amd64fail
 test-amd64-amd64-xl-credit2  pass
 test-amd64-i386-freebsd10-i386   pass
 test-amd64-amd64-xl-pcipt-intel  fail
 test-amd64-amd64-xl-pvh-intelfail
 test-amd64-i386-rhel6hvm-intel   pass
 test-amd64-i386-qemut-rhel6hvm-intel pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-amd64-libvirt pass
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-xl-multivcpupass
 test-amd64-amd64-pairpass
 test-amd64-i386-pair fail
 test-amd64-amd64-xl-sedf-pin pass
 test-amd64-amd

Re: [Xen-devel] [Qemu-devel] Question about scsi emulation

2015-03-24 Thread Juergen Gross

On 03/24/2015 01:20 AM, Yaoli Zheng wrote:

Thank you for the advice!
It will be grateful if someone can provide any guide how to config  MegaSAS HBA 
or how to use xen pvscsi in XEN.
There seems no option in XEN for these two driver emulation and no document 
found online.


The pvscsi driver is rather new in the Linux kernel. Support for pvscsi
in Xen tools (xl) is just being worked on:

http://lists.xen.org/archives/html/xen-devel/2015-03/msg00030.html

Juergen



Thanks!

-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
Sent: Monday, March 23, 2015 11:18 AM
To: Stefan Hajnoczi
Cc: Yaoli Zheng; qemu-de...@nongnu.org; xen-devel@lists.xen.org
Subject: Re: [Xen-devel] [Qemu-devel] Question about scsi emulation

On Mon, 23 Mar 2015, Stefan Hajnoczi wrote:

On Wed, Mar 18, 2015 at 02:16:47PM -0700, Yaoli Zheng wrote:

We have problem using qemu emulated scsi driver(the old lsi). Wonder if any of 
other device model we can try for emulating scsi, and how we can get and config 
it in Xen? Having been told virtio-scsi is alternative one, but have no idea 
how to get it work in Xen.


The MegaSAS HBA might be another option.  Not sure whether MegaSAS or
virtio-scsi are supported under Xen though.


Making MegaSAS HBA work on Xen should be easy.

Rather than virtio-scsi I would consider the new Xen pvscsi frontend/backend 
drivers, now upstream in Linux (drivers/scsi/xen-scsifront.c and 
drivers/xen/xen-scsiback.c). Usually Xen PV drivers perform much better than 
virtio devices on Xen.

That said, probably most things would be better than the old lsi.

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




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


Re: [Xen-devel] [PATCH 1/2] pci: Include asm/numa.h in pci.h

2015-03-24 Thread Ian Campbell
On Mon, 2015-03-23 at 15:54 -0400, Boris Ostrovsky wrote:
> Commit 4fa6b0bacf9c ("pci: stash device's PXM information in struct
> pci_dev") added node field to xen/include/xen/pci.h. Its type,
> nodeid_t, is defined in asm/numa.h and we should include this file
> explicitly in pci.h
> 
> Signed-off-by: Boris Ostrovsky 
> Reported-by: Julien Grall 

Acked-by: Ian Campbell 

> ---
>  xen/include/xen/pci.h |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index b281790..4377f3e 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /*



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


Re: [Xen-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

2015-03-24 Thread Chen, Tiejun

On 2015/3/24 17:51, Ian Campbell wrote:

On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:

All guys,



Thanks for your reply.


Sorry to bother you.

I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and
tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like
pyxc_methods[] and pyxl_methods[]?


They are registered with the Python runtime, so they are called from
Python code. The first member of the struct is the pythonic function


Sorry I don't understanding this. So seems you mean instead of xl, this 
is called by the third party user with python?



name, e.g. from xc.c:
 { "domain_create",


Otherwise, often we always perform `xl create xxx' to create a VM. So I 
think this should go into this flow like this,


xl_cmdtable.c:main_create()
|
+ create_domain()
|
+ libxl_domain_create_new()
|
+ do_domain_create()
|
+ 
Right?


   (PyCFunction)pyxc_domain_create,


So I don't see 'pyxc_domain_create' is called. Or I'm missing something...


   METH_VARARGS | METH_KEYWORDS, "\n"
   "Create a new domain.\n"
   " dom[int, 0]:Domain identifier to use (allocated if 
zero).\n"
   "Returns: [int] new domain identifier; -1 on error.\n" },
defines a method called domain_create, in the xen.lowlevel.xc namespace.


  And how should we call these approaches?


I'm not sure what you are asking here.


If you can give a real case to call this, things couldn't be better :)




In my specific case, I'm trying to introduce a new flag to each a device
while assigning device. So this means I have to add a parameter, 'flag',
into

int xc_assign_device(
  xc_interface *xch,
  uint32_t domid,
  uint32_t machine_sbdf)

Then this is extended as

int xc_assign_device(
  xc_interface *xch,
  uint32_t domid,
  uint32_t machine_sbdf,
  uint32_t flag)

After this introduction, obviously I should cover all cases using
xc_assign_device(). And also I found this fallout goes into these two
files. For example, here pyxc_assign_device() is involved. Currently it
has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str'
should represent all pci devices with SBDF format, right?


It appears so, yes.


But I don't know exactly what rule should be complied to construct this
sort of flag into 'pci_str', or any reasonable idea to achieve my goal?


If it is non-trivial to fix them IMHO it is acceptable for the new
parameter to not be plumbed up to the Python bindings until someone
comes along with a requirement to use it from Python. IOW you can just
pass whatever the nop value is for the new argument.



Should I extend this 'pci_str' like "Seg,bus,device,function:flag"? But 
I'm not sure if I'm breaking the existing usage since like I said, I 
don't know what scenarios are using these methods.


Thanks
Tiejun

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


Re: [Xen-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 18:15 +0800, Chen, Tiejun wrote:
> On 2015/3/24 17:51, Ian Campbell wrote:
> > On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:
> >> All guys,
> >>
> 
> Thanks for your reply.
> 
> >> Sorry to bother you.
> >>
> >> I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and
> >> tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like
> >> pyxc_methods[] and pyxl_methods[]?
> >
> > They are registered with the Python runtime, so they are called from
> > Python code. The first member of the struct is the pythonic function
> 
> Sorry I don't understanding this. So seems you mean instead of xl, this 
> is called by the third party user with python?

Yes, tools/python/xen is the python bindings for various C libraries
supported by Xen.

NB, the libxl ones are broken and not even compiled right now, you can
ignore them.

> 
> > name, e.g. from xc.c:
> >  { "domain_create",
> 
> Otherwise, often we always perform `xl create xxx' to create a VM. So I 
> think this should go into this flow like this,
> 
> xl_cmdtable.c:main_create()
>   |
>   + create_domain()
>   |
>   + libxl_domain_create_new()
>   |
>   + do_domain_create()
>   |
>   + 
> Right?

Yes, xl is written in C not python so tools/python doesn't enter the
picture.

> 
> >(PyCFunction)pyxc_domain_create,
> 
> So I don't see 'pyxc_domain_create' is called. Or I'm missing something...

Chances are that there are no intree users of this code any more, xend
would have used it at one time with something like:
import xen.lowlevel.xc
xc = xen.lowlevel.xc.xc()
xc.domain_create()
etc.

> >
> >> In my specific case, I'm trying to introduce a new flag to each a device
> >> while assigning device. So this means I have to add a parameter, 'flag',
> >> into
> >>
> >> int xc_assign_device(
> >>   xc_interface *xch,
> >>   uint32_t domid,
> >>   uint32_t machine_sbdf)
> >>
> >> Then this is extended as
> >>
> >> int xc_assign_device(
> >>   xc_interface *xch,
> >>   uint32_t domid,
> >>   uint32_t machine_sbdf,
> >>   uint32_t flag)
> >>
> >> After this introduction, obviously I should cover all cases using
> >> xc_assign_device(). And also I found this fallout goes into these two
> >> files. For example, here pyxc_assign_device() is involved. Currently it
> >> has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str'
> >> should represent all pci devices with SBDF format, right?
> >
> > It appears so, yes.
> >
> >> But I don't know exactly what rule should be complied to construct this
> >> sort of flag into 'pci_str', or any reasonable idea to achieve my goal?
> >
> > If it is non-trivial to fix them IMHO it is acceptable for the new
> > parameter to not be plumbed up to the Python bindings until someone
> > comes along with a requirement to use it from Python. IOW you can just
> > pass whatever the nop value is for the new argument.
> >
> 
> Should I extend this 'pci_str' like "Seg,bus,device,function:flag"? But 
> I'm not sure if I'm breaking the existing usage since like I said, I 
> don't know what scenarios are using these methods.

Like I said in the paragraph above, if it is complicated then it is fine
to ignore this new parameter from Python.

I don't know what the semantics of flag is, if it is per SBDF then I
suppose if you really wanted to expose this here then you would need to
invent some syntax for doing so.

Ian.


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


Re: [Xen-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

2015-03-24 Thread Chen, Tiejun

On 2015/3/24 18:20, Ian Campbell wrote:

On Tue, 2015-03-24 at 18:15 +0800, Chen, Tiejun wrote:

On 2015/3/24 17:51, Ian Campbell wrote:

On Tue, 2015-03-24 at 16:47 +0800, Chen, Tiejun wrote:

All guys,



Thanks for your reply.


Sorry to bother you.

I have a question to two files, tools/python/xen/lowlevel/xc/xc.c and
tools/python/xen/lowlevel/xl/xl.c. Who is a caller to those methods like
pyxc_methods[] and pyxl_methods[]?


They are registered with the Python runtime, so they are called from
Python code. The first member of the struct is the pythonic function


Sorry I don't understanding this. So seems you mean instead of xl, this
is called by the third party user with python?


Yes, tools/python/xen is the python bindings for various C libraries
supported by Xen.


Thanks for your explanation.



NB, the libxl ones are broken and not even compiled right now, you can
ignore them.


Looks this is still compiled now.






name, e.g. from xc.c:
  { "domain_create",


Otherwise, often we always perform `xl create xxx' to create a VM. So I
think this should go into this flow like this,

xl_cmdtable.c:main_create()
|
+ create_domain()
|
+ libxl_domain_create_new()
|
+ do_domain_create()
|
+ 
Right?


Yes, xl is written in C not python so tools/python doesn't enter the
picture.


Yeah.






(PyCFunction)pyxc_domain_create,


So I don't see 'pyxc_domain_create' is called. Or I'm missing something...


Chances are that there are no intree users of this code any more, xend
would have used it at one time with something like:
import xen.lowlevel.xc
 xc = xen.lowlevel.xc.xc()
 xc.domain_create()
etc.




In my specific case, I'm trying to introduce a new flag to each a device
while assigning device. So this means I have to add a parameter, 'flag',
into

int xc_assign_device(
   xc_interface *xch,
   uint32_t domid,
   uint32_t machine_sbdf)

Then this is extended as

int xc_assign_device(
   xc_interface *xch,
   uint32_t domid,
   uint32_t machine_sbdf,
   uint32_t flag)

After this introduction, obviously I should cover all cases using
xc_assign_device(). And also I found this fallout goes into these two
files. For example, here pyxc_assign_device() is involved. Currently it
has two parameters, 'dom' and 'pci_str', and as I understand 'pci_str'
should represent all pci devices with SBDF format, right?


It appears so, yes.


But I don't know exactly what rule should be complied to construct this
sort of flag into 'pci_str', or any reasonable idea to achieve my goal?


If it is non-trivial to fix them IMHO it is acceptable for the new
parameter to not be plumbed up to the Python bindings until someone
comes along with a requirement to use it from Python. IOW you can just
pass whatever the nop value is for the new argument.



Should I extend this 'pci_str' like "Seg,bus,device,function:flag"? But
I'm not sure if I'm breaking the existing usage since like I said, I
don't know what scenarios are using these methods.


Like I said in the paragraph above, if it is complicated then it is fine
to ignore this new parameter from Python.

I don't know what the semantics of flag is, if it is per SBDF then I


Yes, this should be a flag specific to a SBDF.

You know, I'm working to fix RMRR completely. Based on some discussion 
about that design ( I assume you may read that thread previously :) ), 
now we probably need to pass a flag to introduce our policy.



suppose if you really wanted to expose this here then you would need to
invent some syntax for doing so.



Definitely.

When I finish this I will send you to review technically.

Again, really appreciate your clarification to me.

Thanks
Tiejun

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


Re: [Xen-devel] One question to lowlevel/xl/xl.c and lowlevel/xc/xc.c

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 18:31 +0800, Chen, Tiejun wrote:
> > NB, the libxl ones are broken and not even compiled right now, you can
> > ignore them.
> 
> Looks this is still compiled now.

xc is, xl is not, I am sure of that.

> > I don't know what the semantics of flag is, if it is per SBDF then I
> 
> Yes, this should be a flag specific to a SBDF.
> 
> You know, I'm working to fix RMRR completely. Based on some discussion 
> about that design ( I assume you may read that thread previously :) ), 
> now we probably need to pass a flag to introduce our policy.

Unless you have a concrete requirement to expose RMRR via the Python
bindings to libxc (i.e. you know somebody is using them) then I think
you should not bother.

Making RMRR work via the (C) interface to libxl used by xl and libvirt
is sufficient for a new in tree feature.

Ian.
> > suppose if you really wanted to expose this here then you would need to
> > invent some syntax for doing so.
> >
> 
> Definitely.
> 
> When I finish this I will send you to review technically.
> 
> Again, really appreciate your clarification to me.
> 
> Thanks
> Tiejun



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


Re: [Xen-devel] [PATCH 01/29] libxl: Further fix exit paths from libxl_device_events_handler

2015-03-24 Thread Ian Campbell
On Wed, 2015-02-18 at 17:23 +0100, Roger Pau Monné wrote:
> El 10/02/15 a les 21.09, Ian Jackson ha escrit:
> > On the success path, do not call GC_FREE explicitly.  Instead, call
> > AO_INPROGRESS.
> > 
> > GC_FREE will free the gc underlying the long-term ao, which is then
> > subsequently referenced in backend_watch_callback's call to
> > libxl__nested_ao_create.  It is a miracle that this ever works at all.
> > 
> > Also, add an `if (rc) goto out;' after the xswatch registration.
> > 
> > After this, libxl_device_events_handler has the conventional and
> > correct ao initiation pattern.
> > 
> > Signed-off-by: Ian Jackson 
> 
> Acked-by: Roger Pau Monné 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 02/29] libxl: Comment cleanups

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> * Add two comments in libxl_remus_disk_drbd documenting buggy handling
>   of the hotplug script exit status.
> 
> * Add a section heading for async exec in libxl_aoutils.c
> 
> * Mention the right function name (libxl__ev_child_fork, not
>   libxl__ev_fork) in libxl_internal.h
> 
> Signed-off-by: Ian Jackson 
> CC: Yang Hongyang 
> CC: Wen Congyang 
> CC: Lai Jiangshan 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 03/29] libxl: suspend: switch_logdirty_done takes rc

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> +int broke;
> +if (rc) {
> +broke = -1;
> +} else {
> +broke = 0;
> +}

int broke = rc ? -1 : 0;

?

But it looks like perhaps you are preparing to add other code in one or
the other case, or maybe you just prefer this for some reason. Either
way:

Acked-by: Ian Campbell 

>  libxl__xc_domain_saverestore_async_callback_done(egc, &dss->shs, broke);
>  }
>  



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


Re: [Xen-devel] [PATCH 04/29] libxl: suspend: common suspend callbacks take rc

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> Change the following functions to take a libxl error code rather than
> a boolean "ok" value, and translate that value to the boolean expected
> by libxc at the last moment:
>   domain_suspend_callback_common_done} dss->callback_common_done
>   remus_domain_suspend_callback_common_done  }
>   domain_suspend_common_done
> 
> Also, abolish domain_suspend_common_failed as
> domain_suspend_common_done can easily do its job and the call sites
> now have to supply the right rc value anyway.
> 
> In domain_suspend_common_guest_suspended, change "ret" to "rc"
> as it contains a libxl error code.
> 
> There is no functional change in this patch: the proper rc value now
> propagates further, but is still eventually smashed to a boolean.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

There are a few new ERROR_FAILs which we might like to consider making
more specific either now or later.

Ian.


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


Re: [Xen-devel] [PATCH 05/29] libxl: suspend: Return correct error from callbacks

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> If a suspend callback fails, it has a libxl error code in its hand.
> However we must return to libxc the values that libxc expects.  So we
> stash the libxl error code in dss->rc and fish it out again after
> libxc returns from the suspend call.
> 
> While we're here, abolish the now-redundant `ok' variable in
> remus_devices_postsuspend_cb.
> 
> The overall functional change is that libxl_domain_save now completes
> with the correct error code as determined when the underlying failure
> happened.  (Usually this is, still, ERROR_FAIL.)
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


[Xen-devel] March Advisory Board Meeting Minutes

2015-03-24 Thread Lars Kurth
See http://wiki.xenproject.org/wiki/AB_Meeting/March_2015_Minutes 
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH 06/29] libxl: Use libxl__xswait* in libxl__ao_device

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> @@ -1164,7 +1136,7 @@ static void device_hotplug_clean(libxl__gc *gc, 
> libxl__ao_device *aodev)
>  {
>  /* Clean events and check reentrancy */
>  libxl__ev_time_deregister(gc, &aodev->timeout);

You seem to have removed the initialisation of this in a previous hunk
but not this deregistration or the field itself.

Was that deliberate, perhaps it was serving dual purpose somewhere?

> -libxl__ev_xswatch_deregister(gc, &aodev->xs_watch);
> +libxl__xswait_stop(gc, &aodev->xswait);
>  assert(!libxl__async_exec_inuse(&aodev->aes));
>  }
>  
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 2862c69..5a76d51 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -2152,7 +2152,7 @@ struct libxl__ao_device {
>  /* Bodge for Qemu devices */
>  libxl__ev_time timeout;
>  /* xenstore watch for backend path of driver domains */
> -libxl__ev_xswatch xs_watch;
> +libxl__xswait_state xswait;
>  int num_exec;
>  /* for calling hotplug scripts */
>  libxl__async_exec_state aes;



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


Re: [Xen-devel] [PATCH 07/29] libxl: xswait/devstate: Move xswait to before devstate

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> Pure code motion.  We are going to make devstate use xswait.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH v2] xen/passthrough: Support a single iommu_domain per xen domain per SMMU

2015-03-24 Thread Manish Jaggi


On Monday 23 March 2015 07:16 PM, Robbie VanVossen wrote:

If multiple devices are being passed through to the same domain and they
share a single SMMU, then they only require a single iommu_domain.

In arm_smmu_assign_dev, before a new iommu_domain is created, the
xen_domain->contexts is checked for any iommu_domains that are already
assigned to device that uses the same SMMU as the current device. If one
is found, attach the device to that iommu_domain. If a new one isn't
found, create a new iommu_domain just like before.

The arm_smmu_deassign_dev function assumes that there is a single
device per iommu_domain. This meant that when the first device was
deassigned, the iommu_domain was freed and when another device was
deassigned a crash occured in xen.

To fix this, a reference counter was added to the iommu_domain struct.
When an arm_smmu_xen_device references an iommu_domain, the
iommu_domains ref is incremented. When that reference is removed, the
iommu_domains ref is decremented. The iommu_domain will only be freed
when the ref is 0.

Signed-off-by: Robbie VanVossen 

Hi,
Are you adding a PCI passthrough support to Xen ?. I am in process of 
sending smmu driver patches based on juliens latest code.



---
Changed since v1:
   * Fixed coding style for comments
   * Move increment/decrement outside of attach/detach functions
   * Expanded xen_domain->lock to protect more of the assign/deassign
 functions
   * Removed iommu_domain add/remove_device functions
---
  xen/drivers/passthrough/arm/smmu.c |   98 +++-
  1 file changed, 74 insertions(+), 24 deletions(-)

diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index a7a7da9..1a3de00 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -223,6 +223,7 @@ struct iommu_domain
/* Runtime SMMU configuration for this iommu_domain */
struct arm_smmu_domain  *priv;
  
+	atomic_t ref;

/* Used to link iommu_domain contexts for a same domain.
 * There is at least one per-SMMU to used by the domain.
 * */
@@ -2569,7 +2570,9 @@ static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
  {
struct iommu_domain *domain;
struct arm_smmu_xen_domain *xen_domain;
-   int ret;
+   struct arm_smmu_device *smmu;
+   int ret = 0;
+   int existing_ctxt_fnd = 0;
  
  	xen_domain = domain_hvm_iommu(d)->arch.priv;
  
@@ -2585,36 +2588,77 @@ static int arm_smmu_assign_dev(struct domain *d, u8 devfn,

return ret;
}
  
+	spin_lock(&xen_domain->lock);

+
/*
-* TODO: Share the context bank (i.e iommu_domain) when the device is
-* under the same SMMU as another device assigned to this domain.
-* Would it useful for PCI
+* Check to see if a context bank (iommu_domain) already exists for
+* this xen domain under the same SMMU
 */
-   domain = xzalloc(struct iommu_domain);
-   if (!domain)
-   return -ENOMEM;
+   if (!list_empty(&xen_domain->contexts)) {
+   smmu = find_smmu_for_device(dev);
+   if (!smmu) {
+   dev_err(dev, "cannot find SMMU\n");
+   ret = -ENXIO;
+   goto out;
+   }
  
-	ret = arm_smmu_domain_init(domain);

-   if (ret)
-   goto err_dom_init;
+   /*
+* Loop through the &xen_domain->contexts to locate a context
+* assigned to this SMMU
+*/
+   list_for_each_entry(domain, &xen_domain->contexts, list) {
+   if(domain->priv->smmu == smmu)
+   {
+   /*
+* We have found a context already associated 
with the
+* same xen domain and SMMU
+*/
+   ret = arm_smmu_attach_dev(domain, dev);
+   if (ret) {
+   dev_err(dev,
+   "cannot attach device to already 
existing iommu_domain\n");
+   goto out;
+   }
+   atomic_inc(&domain->ref);
+
+   existing_ctxt_fnd = 1;
+   break;
+   }
+   }
+   }
  
-	domain->priv->cfg.domain = d;

+   if(!existing_ctxt_fnd){
  
-	ret = arm_smmu_attach_dev(domain, dev);

-   if (ret)
-   goto err_attach_dev;
+   domain = xzalloc(struct iommu_domain);
+   if (!domain) {
+   ret = -ENOMEM;
+   goto out;
+   }
  
-	spin_lock(&xen_domain->lock);

-   /* Chain the new context to the domain */
-   list_a

Re: [Xen-devel] [PATCH 09/29] libxl: New error codes CANCELLED etc.

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> We introduce ERROR_CANCELLED now, so that we can write code to handle
> it, and decreee that functions might return it, even though currently
> there is nowhere where this error is generated.
> 
> While we're here, provide ERROR_NOTFOUND and ERROR_NOTIMPLEMENTED,
> which will also be used later, but only as part of the public API.
> 
> Signed-off-by: Ian Jackson 



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


Re: [Xen-devel] [PATCH 08/29] libxl: devstate: Use libxl__xswait*

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 10/29] libxl: events: Make timeout and async exec setup take an ao, not a gc

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> Change the timeout setup functions to take a libxl__ao, not a
> libxl__gc.  This is going to be needed for ao cancellation, because
> timeouts are going to be a main hook for ao cancellation - so the
> timeouts need to be associated with an ao.
> 
> This means that timeouts can only occur as part of a long-running
> libxl function (but this is of course correct, as libxl shouldn't have
> any global timeouts, and indeed all the call sites have an ao).
> 
> Also remove the gc parameter from libxl__async_exec_start.  It can
> just use the gc from the ao supplied in the aes.
> 
> All the callers follow the obvious patterns and therefore supply the
> ao's gc to libxl__async_exec_start and the timeout setup functions.
> There is therefore no functional change in this patch.
> 
> Signed-off-by: Ian Jackson 
> CC: Yang Hongyang 
> CC: Wen Congyang 
> CC: Lai Jiangshan 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 11/29] libxl: events: Make libxl__async_exec_* pass caller an rc

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> The internal user of libxl__async_exec_start et al now gets an rc as
> well as the process's exit status.
> 
> For now this is always either 0 or ERROR_FAIL, but with ao
> cancellation this will possibly be CANCELLED or TIMEDOUT too.
> 
> Signed-off-by: Ian Jackson 
> ---
> v2: New patch due to rebause; v1 had changes to device_hotplug_*
>  scripts instead.
> Callback now gets unambiguous information about error situation:
>  previously, if only thing that went wrong was that child died
>  badly, rc would be FAILED, which was unambigously; now rc=0.
> Add a comment document the meaning of the rc and status parameters
>  to the callback.
> ---
>  tools/libxl/libxl_aoutils.c |9 ++---
>  tools/libxl/libxl_device.c  |   13 +
>  tools/libxl/libxl_internal.h|   11 ++-
>  tools/libxl/libxl_netbuffer.c   |   19 ++-
>  tools/libxl/libxl_remus_disk_drbd.c |8 +---
>  5 files changed, 40 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index 754e2d1..891cdb8 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -483,11 +483,12 @@ static void async_exec_done(libxl__egc *egc,
>  libxl__ev_time_deregister(gc, &aes->time);
>  
>  if (status) {
> -libxl_report_child_exitstatus(CTX, LIBXL__LOG_ERROR,
> -  aes->what, pid, status);
> +if (!aes->rc)

Could be one "if (status && !aes->rc)", unless perhaps there is more
code to come in this block?

> +libxl__async_exec_state *aes, int rc, int status);
> +/*
> + * Meaning of status and rc:
> + *  rc==0, status==0all went well
> + *  rc==0, status!=0everything OK except child exited nonzero (logged)
> + *  rc!=0   something else went wrong (status is real
> + *   exit status, maybe reflecting SIGKILL if aes
> + *   code killed the child).  Logged unless CANCELLED.

I'm unclear on whether status is valid in this third case or not. I
think you are saying that it is (probably?) valid but if rc!=0 the
caller likely doesn't actually care what it is?

Everything else looks fine to me.

Ian.


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


Re: [Xen-devel] [PATCH v2 7/7] tools: add tools support for Intel CAT

2015-03-24 Thread Chao Peng
On Thu, Mar 19, 2015 at 12:51:19PM +, Ian Campbell wrote:
> On Thu, 2015-03-19 at 18:41 +0800, Chao Peng wrote:
> > This is the xc/xl changes to support Intel Cache Allocation
> > Technology(CAT). Two commands are introduced:
> > - xl psr-cat-cbm-set [-s socket]  
> >   Set cache capacity bitmasks(CBM) for a domain.
> > - xl psr-cat-show 
> >   Show Cache Allocation Technology information.
> > 
> > Signed-off-by: Chao Peng 
> > ---
> >  tools/libxc/include/xenctrl.h |  15 +++
> >  tools/libxc/xc_psr.c  |  74 ++
> 
> At the libxc level this looks like a reasonable wrapping of the
> hypercall interface, so if the hypervisor folks are happy with that then
> I'm happy with this.
> 
> >  tools/libxl/libxl.h   |  18 
> 
> At the libxl level things seem to be rather opaque to me, i.e. what is
> domain_data? what does it contain? does it have any more semantics than
> a 64-bit number?

The 64-bit number now holds the cache bitmask (CBM) for the domain. In
the future, the number may represent the memory bandwidth throttling for
the domain. So use a neutral name here.

> 
> What future new values might there be for libxl_psr_cat_type?

As far as I can see, besides L3_CBM, libxl_psr_cat_type may be L2_CBM or
MEM_BANDWIDTH_THROTTLING in the future.

> 
> >  tools/libxl/libxl_psr.c   | 107 ++--
> >  tools/libxl/libxl_types.idl   |   4 +
> >  tools/libxl/xl.h  |   4 +
> >  tools/libxl/xl_cmdimpl.c  | 225 
> > --
> >  tools/libxl/xl_cmdtable.c |  13 +++
> 
> You've missed updating the manpage.

Yep, thanks.

> 
> >  8 files changed, 445 insertions(+), 15 deletions(-)
> > 
> 
> > +#ifdef LIBXL_HAVE_PSR_CAT
> > +int libxl_psr_cat_set_domain_data(libxl_ctx *ctx, uint32_t domid,
> > +  libxl_psr_cat_type type, uint32_t target,
> > +  uint64_t data);
> > +int libxl_psr_cat_get_domain_data(libxl_ctx *ctx, uint32_t domid,
> > +  libxl_psr_cat_type type, uint32_t target,
> > +  uint64_t *data_r);
> > +int libxl_psr_cat_get_l3_info(libxl_ctx *ctx, uint32_t socket,
> > +  uint32_t *cos_max_r, uint32_t *cbm_len_r);
> > +#endif
> > +
> 
> 
> > [...]
> > +static uint32_t get_phy_socket_num(void)
> 
> I think this would be better named "count_physical_sockets" or
> something, otherwise I might think it takes a lock.
> 
> > +{
> > +int rc;
> > +uint32_t nr_sockets;
> 
>uint32_t nr_sockets = 0;
> 
> > +libxl_physinfo info;
> > +libxl_physinfo_init(&info);
> > +rc = libxl_get_physinfo(ctx, &info);
> 
> Then:
>if (!rc)
>nr_sockets = info.nr_cpus / info.threads_per_core / 
> info.cores_per_socket;
>   
>libxl_physinfo_dispose(&info);
>return nr_sockets;
> 
> means you only have one return path to do clean up on.

Thanks, I will adopt your suggestion.

> 
> Also, does this function need to be in an ifdef, since it isn't called
> unless LIBXL_HAVE_... is set? (IOW: does this compile for ARM)

libxl_get_physinfo is for all archs, so I think it compiles for ARM. 

> 
> 
> > @@ -8057,6 +8070,202 @@ int main_psr_cmt_show(int argc, char **argv)
> >  }
> >  #endif
> >  
> > +#ifdef LIBXL_HAVE_PSR_CAT
> > +static int psr_cat_l3_cbm_set(uint32_t domid, uint32_t socket, uint64_t 
> > cbm)
> > +{
> > +int rc;
> > +uint32_t i, nr_sockets;
> > +
> > +if (socket != ALL_SOCKETS) {
> > +return libxl_psr_cat_set_domain_data(ctx, domid,
> > + LIBXL_PSR_CAT_TYPE_L3_CBM,
> > + socket, cbm);
> > +} else {
> > +nr_sockets = get_phy_socket_num();
> 
> I wonder if the libxl API ought to allow for an "all sockets" argument
> and then do all this internally instead of making the callers do it?

I can of course. But I just think of the API semantic consistence for
future, the "target" may be cpu but not socket if we support L2_CBM, so
will that value still represent all cpus? If this is not the problem,
then I will do it.

> 
> > +if (nr_sockets == 0) {
> > +fprintf(stderr, "Failed getting physinfo\n");
> > +return -1;
> > +}
> > +for (i = 0; i < nr_sockets; i++) {
> > +rc = libxl_psr_cat_set_domain_data(ctx, domid,
> > +   LIBXL_PSR_CAT_TYPE_L3_CBM,
> > +   i, cbm);
> > +/* Total L3 cache size */
> > +printf("%-46s", "Total L3 Cache Size");
> 
> How wide are these lines going to be? Can we try and keep it to <80
> columns? Perhaps you could include an example of the output of each of
> the show functions in the commit message?

As socket number is variable, it's hard to strictly ensure <80 if all
the sockets displayed in one line. Perhaps socket by socket displa

Re: [Xen-devel] [PATCH 12/29] libxl: events: Permit timeouts to signal cancellation

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:09 +, Ian Jackson wrote:
> The callback functions provided by users must take an rc value.  This
> rc value can be ERROR_TIMEDOUT or ERROR_CANCELLED.
> 
> Users of xswait are now expected to deal correctly with
> ERROR_CANCELLED.  If they experience this, it hasn't been logged.
> And the caller won't log it either since it's not TIMEDOUT.
> Luckily this is correct, so we can just change the doc comment.
> 
> Currently nothing generates ERROR_CANCELLED; in particular the
> timeouts cannot in fact signal cancellation.
> 
> There should be no publicly visible change except that some error
> returns from libxl will change from ERROR_FAIL to ERROR_TIMEDOUT, and
> some changes to debugging messages.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 13/29] libxl: domain create: Do not destroy on cancellation

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> If we cancelled the domain creation, do not try to tear it down again
> Document this.
> 
> This is a backwards-compatible API change since old libxl users will
> never cancel any operations.
> 
> In the current code, there is no functional change, because
> ERROR_CANCELLED is never generated anywhere yet.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

I presume at some later stage in the series a suitable
LIBXL_HAVE_CANCELLATION will materialise? I mention it here because it
is on my mind.



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


Re: [Xen-devel] [PATCH 14/29] libxl: ao: Record ultimate parent of a nested ao

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> This will be used by the cancellation machinery.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 15/29] libxl: ao: Count the nested progeny of an ao

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> This will detect any "escaped" nested aos.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 16/29] libxl: ao: Provide manip_refcnt

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> +/*
> + * A "manip" is a libxl public function manipulating this ao, which
> + * has a pointer to it.  We have to not destroy it while that's the
> + * case, obviously.

It might be nice to the reader to make a reference to the "An ao and its
gc may be accessed only with the ctx lock held." sentence in the overall
ao docs?
[...]
> +libxl__ao__destroy(ctx,ao);

Nit: missing space after ",".

But other than those and including if you disagree about extending the
comment, since it's not a big deal:
Acked-by: Ian Campbell 

Ian.


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


Re: [Xen-devel] [OSSTEST Nested PATCH 2/6] Add and expose some testsupport APIs

2015-03-24 Thread Ian Jackson
Ian Campbell writes ("Re: [OSSTEST Nested PATCH 2/6] Add and expose some 
testsupport APIs"):
> On Tue, 2015-03-24 at 03:34 +, Hu, Robert wrote:
> > > Make it an option to the function which creates the configuration in the
> > > first place. Quoting myself from earlier in this very thread:
> 
> > Then we still need to designate to use e1000 device somewhere; say, in
> > ts-nested-setup, we store this designation in runvar.
> > Then when creating the config, according to your proposal, we plumbing
> > such designation through $xopts, since we see it in runvar. Are you OK
> > with such implementation?
> 
> I'm not sure if this should be a runvar or just something which
> ts-nested-setup knows. That's one for Ian J to decide.

I think this should be in a runvar named after the guest, and should
be set by make-flight.  In principle that would allow us to have jobs
with and without this setting, for example.

I think the needed moving parts for this are:
 * something in prepareguest_part_xencfg which looks up
   the runvar (eg guest_var($gho,'vifmodel') and if not undef
   adds something appropriate to the vif setting
 * corresponding setting in the new jobs in make-flight/mfi-common

Thanks,
Ian.

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


Re: [Xen-devel] [PATCH 17/29] libxl: cancellation: Provide public ao cancellation API

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> +/*
> + * It is sometimes possible to cancel an asynchronous operation.
> + *
> + * libxl_ao_cancel searches for an ongoing asynchronous operation whose
> + * ao_how is identical to *how, and tries to cancel it.

I can see that you have arranged for the pointer not to be required to
match, just the contents of the struct, which may be convenient for some
callers who haven't remembered the ao_how somewhere convenient, but is
it permissible to use the same pointer if it is convenient?

Other than wondering about that this patch looks good,
Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 18/29] libxl: cancellation: Provide explicit internal cancel check API

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> Some places in libxl which can't handle cancellation via a
> libxl__ao_cancellable callback might nevertheless benefit from being
> able to explicitly check for cancellation.
> 
> Provide the (fairly trivial) internal API function to do this.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


[Xen-devel] [PATCH v4] xen/arm: Add RESERVE option to reserve non-leaf level table entries

2015-03-24 Thread vijay . kilari
From: Vijaya Kumar K 

On x86, for the pages mapped with PAGE_HYPERVISOR attribute
non-leaf page tables are allocated with valid pte entries.
and with MAP_SMALL_PAGES attribute only non-leaf page tables are
allocated with invalid (valid bit set to 0) pte entries.
However on arm this is not the case. On arm for the pages
mapped with PAGE_HYPERVISOR and MAP_SMALL_PAGES both
non-leaf and leaf level page table are allocated with valid bit
in pte entries.

This behaviour in arm makes common vmap code fail to
allocate memory beyond 128MB as described below.

In vm_init, map_pages_to_xen() is called for mapping
vm_bitmap. Initially one page of vm_bitmap is allocated
and mapped using PAGE_HYPERVISOR attribute.
For the rest of vm_bitmap pages, MAP_SMALL_PAGES attribute
is used to map.

In ARM for both PAGE_HYPERVISOR and MAP_SMALL_PAGES, valid bit
is set to 1 in pte entry for these mapping.

In vm_alloc(), map_pages_to_xen() is failing for >128MB because
for this next vm_bitmap page the mapping is already set in vm_init()
with valid bit set in pte entry. So map_pages_to_xen() in
ARM returns error.

With this patch, MAP_SMALL_PAGES is dropped and arch specific
populate_pt_range() api is introduced to populate non-leaf
page table entries for the requested pages. Added RESERVE option
to map non-leaf page table entries.

Signed-off-by: Vijaya Kumar K
Acked-by: Jan Beulich 
---
v4:
 - Update commit title
 - Dropped all changes to page.h except dropping MAP_SMALL_PAGES
 - Dropped get_pte_flags() and is_pte_preset()
v3:
 - Fix typos in commit message
 - Introduce arch specific api populate_pt_range
v2:
 - Rename PTE_INVALID to PAGE_PRESENT
 - Re-define PAGE_* macros with PAGE_PRESENT
 - Rename parameter ai to flags
 - Introduce macro to check present flag and extract attribute
   index values
---
 xen/arch/arm/mm.c  |   13 -
 xen/arch/x86/mm.c  |6 ++
 xen/common/vmap.c  |2 +-
 xen/include/asm-arm/page.h |1 -
 xen/include/xen/mm.h   |7 ++-
 5 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7d4ba0c..d103f8f 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -827,7 +827,8 @@ static int create_xen_table(lpae_t *entry)
 
 enum xenmap_operation {
 INSERT,
-REMOVE
+REMOVE,
+RESERVE
 };
 
 static int create_xen_entries(enum xenmap_operation op,
@@ -859,12 +860,15 @@ static int create_xen_entries(enum xenmap_operation op,
 
 switch ( op ) {
 case INSERT:
+case RESERVE:
 if ( third[third_table_offset(addr)].pt.valid )
 {
 printk("create_xen_entries: trying to replace an existing 
mapping addr=%lx mfn=%lx\n",
addr, mfn);
 return -EINVAL;
 }
+if ( op == RESERVE )
+break;
 pte = mfn_to_xen_entry(mfn, ai);
 pte.pt.table = 1;
 write_pte(&third[third_table_offset(addr)], pte);
@@ -898,6 +902,13 @@ int map_pages_to_xen(unsigned long virt,
 {
 return create_xen_entries(INSERT, virt, mfn, nr_mfns, flags);
 }
+
+int populate_pt_range(unsigned long virt, unsigned long mfn,
+  unsigned long nr_mfns)
+{
+return create_xen_entries(RESERVE, virt, mfn, nr_mfns, 0);
+}
+
 void destroy_xen_mappings(unsigned long v, unsigned long e)
 {
 create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0);
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 45acfb5..2387cc0 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5701,6 +5701,12 @@ int map_pages_to_xen(
 return 0;
 }
 
+int populate_pt_range(unsigned long virt, unsigned long mfn,
+  unsigned long nr_mfns)
+{
+return map_pages_to_xen(virt, mfn, nr_mfns, MAP_SMALL_PAGES);
+}
+
 void destroy_xen_mappings(unsigned long s, unsigned long e)
 {
 bool_t locking = system_state > SYS_STATE_boot;
diff --git a/xen/common/vmap.c b/xen/common/vmap.c
index 783cea3..739d468 100644
--- a/xen/common/vmap.c
+++ b/xen/common/vmap.c
@@ -40,7 +40,7 @@ void __init vm_init(void)
 bitmap_fill(vm_bitmap, vm_low);
 
 /* Populate page tables for the bitmap if necessary. */
-map_pages_to_xen(va, 0, vm_low - nr, MAP_SMALL_PAGES);
+populate_pt_range(va, 0, vm_low - nr);
 }
 
 void *vm_alloc(unsigned int nr, unsigned int align)
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 3e7b0ae..8de5e26 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -64,7 +64,6 @@
 #define PAGE_HYPERVISOR (WRITEALLOC)
 #define PAGE_HYPERVISOR_NOCACHE (DEV_SHARED)
 #define PAGE_HYPERVISOR_WC  (DEV_WC)
-#define MAP_SMALL_PAGES PAGE_HYPERVISOR
 
 /*
  * Stage 2 Memory Type.
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index fd60c9c..4bc3d8e 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -55,7 +55,12

[Xen-devel] [PATCH OSSTEST 0/2] Uboot patches for XSM test case

2015-03-24 Thread Wei Liu
Wei Liu (2):
  uboot: use "readlink -f"
  uboot: make flask loading address host property

 Osstest/Debian.pm | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

-- 
1.9.1


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


[Xen-devel] [PATCH OSSTEST 2/2] uboot: make flask loading address host property

2015-03-24 Thread Wei Liu
Signed-off-by: Wei Liu 
---
Please set host property 'UBootSetFlaskAddrR' to 0x120.
---
 Osstest/Debian.pm | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index b829878..9f7705e 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -158,10 +158,16 @@ sub setupboot_uboot () {
# Use the flaskpolicy from tools build job because we might
# want to test cross releases policy compatibility.
my $flaskpolicy = get_runvar('flaskpolicy',$r{buildjob});
+   my $flask_policy_addr_r =
+   get_host_property($ho, 'UBootSetFlaskAddrR', undef);
+   my $set_flask_addr_r =
+   $flask_policy_addr_r ?
+   "setenv flask_policy_addr_r $flask_policy_addr_r" : "";
+
$xenhopt .= " flask=enforcing";
$flask_commands = 

[Xen-devel] [PATCH OSSTEST 1/2] uboot: use "readlink -f"

2015-03-24 Thread Wei Liu
If the path is not a symlink, readlink by default returns empty string.
Use "-f" to always return canonical path. This fixes the problem that
xenpolicy file not getting loaded (because it is not a symlink).

Also change another spot that calls readlink to get xen binary path in
case in the future we decide to not use symlink.

Signed-off-by: Wei Liu 
---
 Osstest/Debian.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Osstest/Debian.pm b/Osstest/Debian.pm
index 6784024..b829878 100644
--- a/Osstest/Debian.pm
+++ b/Osstest/Debian.pm
@@ -162,7 +162,7 @@ sub setupboot_uboot () {
$flask_commands = 

Re: [Xen-devel] [RFC PATCH v2 13/22] xen/arm: its: Add virtual ITS command support

2015-03-24 Thread Julien Grall

Hello Vijay,

More questions/remarks about command processing.

On 19/03/2015 14:38, vijay.kil...@gmail.com wrote:

+int vgic_its_process_cmd(struct vcpu *v, struct vgic_its *vits)
+{
+struct its_cmd_block virt_cmd;
+
+/* XXX: Currently we are processing one cmd at a time */
+ASSERT(spin_is_locked(&vits->lock));
+
+do {
+if ( vgic_its_read_virt_cmd(v, vits, &virt_cmd) )
+goto err;
+if ( vgic_its_parse_its_command(v, vits, &virt_cmd) )
+goto err;
+} while ( vits->cmd_write != vits->cmd_write_save );
+
+vits->cmd_write_save = vits->cmd_write;
+DPRINTK("vITS: write_save 0x%lx write 0x%lx \n",
+vits->cmd_write_save,
+vits->cmd_write);
+/* XXX: Currently we are processing one cmd at a time */
+vgic_its_update_read_ptr(v, vits);


From the spec the GITS_CREADR should be updated at every command 
processing. That would make cmd_write_save pointless.


Also, you are taking the VITS lock for the whole process. This process 
can be very long. How will it affect the other vCPUs of the domain?


Finally, in environment with multiple guests using ITS, the ITS command 
send to the physical ITS may be interleaved (i.e DOM1 cmd, DOM2 cmd, 
DOM1 cmd ...). Is there any possible side-effect?


Regards,

--
Julien Grall

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


Re: [Xen-devel] [PATCH 17/29] libxl: cancellation: Provide public ao cancellation API

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> +/*
> + * For nested aos:
> + *  Semantically, cancellation affects the whole tree of aos,
> + *not just the parent.
> + *  libxl__ao_cancellable.ao refers to the child, so
> + *that the child callback sees the right ao.  (After all,
> + *it was code dealing with the child that set .ao.)
> + *  But, the cancellable is recorded on the "cancellables" list
> + *for the ultimate root ao, so that every possible child
> + *cancellation occurs as a result of the cancellation of the
> + *parent.
> + *  We set ao->cancelling only in the root.
> + */

WRT this, given a tree of ao's, which ones need to be cancellable for a
cancellation to succeed? I would assume all of them do, or else the
cancellation can only occur if/when the non-cancellable ones happen to
end?

Do we(/are we going to) take steps to stop new non-cancellable ao's to
the tree once the root is cancelled?

Ian.


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


Re: [Xen-devel] [PATCH 19/29] libxl: cancellation: Make timeouts cancellable

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> Make libxl__ev_time* register with the cancellation machinery, so that
> libxl_ao_cancel can cancel any operation which has a timeout.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 20/29] libxl: cancellation: Note that driver domain task cannot be usefully cancelled

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> In practice, cancelling this task will cause all subsequent actual
> backend operations to fail, but will not actually cause the
> libxl_device_events_handler operation to complete.
> 
> Signed-off-by: Ian Jackson 
> CC: Roger Pau Monne 

Acked-by: Ian Campbell 

This is an unfortunate short-coming though, and I presume one which
could be fixed by updates to the toolstack<->driver domain protocol?

Ian.


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


Re: [Xen-devel] [PATCH 21/29] libxl: cancellation: Make spawns cancellable

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> The libxl__spawn_spawn internal API permits the caller to specify
> .timeout_ms==-1, meaning to wait forever.  Provide an explicit
> cancellable to allow spawns to be cancelled.

AIUI this also lets spawns with timeout_ms > -1 to be cancelled, which I
think is desirable.

> I think there are not currently any internal callers which do use
> spawn with an infinite timeout, but this should not be left as a
> lacuna for later.
> 
> Also, this change means that in practice anything which is cancelled
> while spawning ought to be queued for two cancellation notifications:
> one from its timeout, and one from the explicit cancellation
> registration.  Hopefully this will mean that more exciting paths are
> exercised during testing.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] Design and Question: Eliminate Xen (RTDS) scheduler overhead on dedicated CPU

2015-03-24 Thread George Dunlap
On Tue, Mar 24, 2015 at 3:50 AM, Meng Xu  wrote:
> Hi Dario and George,
>
> I'm exploring the design choice of eliminating the Xen scheduler overhead on
> the dedicated CPU. A dedicated CPU is a PCPU that has a full capacity VCPU
> pinned onto it and no other VCPUs will run on that PCPU.

Hey Meng!  This sounds awesome, thanks for looking into it.


> [Problems]
> The issue I'm encountering is as follows:
> After I implemented the dedicated cpu feature, I compared the latency of a
> cpu-intensive task in domU on dedicated CPU (denoted as R_dedcpu) and the
> latency on non-dedicated CPU (denoted as R_nodedcpu). The expected result
> should be R_dedcpu < R_nodedcpu since we avoid the scheduler overhead.
> However, the actual result is R_dedcpu > R_nodedcpu, and R_dedcpu -
> R_nodedcpu ~= 1000 cycles.
>
> After adding some trace to every function that may raise the
> SCHEDULE_SOFTIRQ, I found:
> When a cpu is not marked as dedicated cpu and the scheduler on it is not
> disabled, the vcpu_block() is triggered 2896 times during 58280322928ns
> (i.e., triggered once every 20,124,421ns in average) on the dedicated cpu.
> However,
> When I disable the scheduler on a dedicated cpu, the function
> vcpu_block(void) @schedule.c will be triggered very frequently; the
> vcpu_block(void) is triggered 644824 times during 8,918,636,761ns (i.e.,
> once every 13831ns in average) on the dedicated cpu.
>
> To sum up the problem I'm facing, the vcpu_block(void) is trigger much
> faster and more frequently when the scheduler is disabled on a cpu than when
> the scheduled is enabled.
>
> [My question]
> I'm very confused at the reason why vcpu_block(void) is triggered so
> frequently when the scheduler is disabled.  The vcpu_block(void) is called
> by the SCHEDOP_block hypercall, but why this hypercall will be triggered so
> frequently?
>
> It will be great if you know the answer directly. (This is just a pure hope
> and I cannot really expect it. :-) )
> But I really appreciate it if you could give me some directions on how I
> should figure it out. I grepped vcpu_block(void) and SCHEDOP_block  in the
> xen code base, but didn't found much call to them.
>
> What confused me most is that  the dedicated VCPU should be blocked less
> frequently instead of more frequently when the scheduler is disabled on the
> dedicated CPU, because the dedicated VCPU is always running on the CPU now
> without the hypervisor scheduler's interference.

So if I had to guess, I would guess that you're not actually blocking
when the guest tries to block.  Normally if the guest blocks, it
blocks in a loop like this:

do {
  enable_irqs();
  hlt;
  disable_irqs;
} while (!interrup_pending);

For a PV guest, the hlt() would be replaced with a PV block() hypercall.

Normally, when a guest calls block(), then it's taken off the
runqueue; and if there's nothing on the runqueue, then the scheduler
will run the idle domain; it's the idle domain that actually does the
blocking.

If you've hardwired it always to return the vcpu in question rather
than the idle domain, then it will never block -- it will busy-wait,
calling block millions of times.

The simplest way to get your prototype working, in that case, would be
to return the idle vcpu for that pcpu if the guest is blocked.

But a brief comment on your design:

Looking at your design at the moment, you will get rid of the overhead
of the scheduler-related interrupts, and any pluggable-cpu accounting
that needs to happen (e.g., calculating credits burned, &c).  And
that's certainly not nothing.  But it's not really accurate to say
that you're avoiding the scheduler entirely.  At the moment, as far as
I can tell, you're still going through all the normal schedule.c
machinery between wake-up and actually running the vm; and the normal
machinery for interrupt delivery.

I'm wondering -- are people really going to want to just pin a single
vcpu from a domain like this?  Or are they going to want to pin all
vcpus from a given domain?

For the first to be useful, the guest OS would need to understand
somehow that this cpu has better properties than the other vcpus on
its system.  Which I suppose could be handled manually (e.g., by the
guest admin pinning processes to that cpu or something).

The reason I'm asking is because another option that would avoid the
need for special per-cpu flags would to make a "sched_place" scheduler
(sched_partition?), which would essentially do what you've done here
-- when you add a vcpu to the scheduler, it simply chooses one of its
free cpus and dedicates it to that vcpu.  If no such cpus are
available, it returns an error.  In that case, you could use the
normal cpupool machinery to assign cpus to that scheduler, without
needing to introduce these extra flags, and to make each of the
pluggable schedulers need to deal with the complexity of implementing
the "dedicated" scheduling.

The only downside is that at the moment you can't have a domain cross
cpupools; so either all vcpus 

Re: [Xen-devel] [PATCH 22/29] libxl: Introduce DOMAIN_DESTROYED error code

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> This is currently reported only by the bootloader code, if the domain
> is destroyed while the bootloader is running.
> 
> In the future it would be nice to return it for other circumstances
> where the domain existed when the operation started but subsequently
> vanished.

Konrad has a semantically similar error code which he is adding, I think
in his recent libxl series to do with vcpu-set.

AIUI Konrad's semantics are simply "domain does not exist", which seems
to be usefully distinct from your "did exist but doesn't any more".

I just wanted to mention it in case I'd misunderstood one or both error
codes. As it stands this patch seems fine to me:

> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

I do wonder though if we ought to be better about documenting in the
code|headers|idl what error codes mean and where they should be used
(some are global, others specific to a subset of calls etc).

Ian.

> ---
> v2: New in this version of the series.
> ---
>  tools/libxl/libxl_bootloader.c |2 +-
>  tools/libxl/libxl_types.idl|1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/libxl_bootloader.c b/tools/libxl/libxl_bootloader.c
> index 79947d4..c3f3a1f 100644
> --- a/tools/libxl/libxl_bootloader.c
> +++ b/tools/libxl/libxl_bootloader.c
> @@ -611,7 +611,7 @@ static void bootloader_display_copyfail(libxl__egc *egc,
>  static void bootloader_domaindeath(libxl__egc *egc, libxl__domaindeathcheck 
> *dc)
>  {
>  libxl__bootloader_state *bl = CONTAINER_OF(dc, *bl, deathcheck);
> -bootloader_stop(egc, bl, ERROR_FAIL);
> +bootloader_stop(egc, bl, ERROR_DOMAIN_DESTROYED);
>  }
>  
>  static void bootloader_finished(libxl__egc *egc, libxl__ev_child *child,
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 193f22a..d91b70d 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -66,6 +66,7 @@ libxl_error = Enumeration("error", [
>  (-20, "CANCELLED"),
>  (-21, "NOTFOUND"),
>  (-22, "NOTIMPLEMENTED"),
> +(-23, "DOMAIN_DESTROYED"),
>  ], value_namespace = "")
>  
>  libxl_domain_type = Enumeration("domain_type", [



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


Re: [Xen-devel] [PATCH 23/29] libxl: cancellation: Support cancellation where we spot domain death

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> Make an active libxl__domaindeathcheck contain an active
> libxl__ao_cancellable.
> 
> Consequential changes are:
>  * domaindeath callbacks now take an rc value.
>  * libxl__domaindeathcheck_start takes an ao, not a gc.
>  * bootloader_domaindeath plumbs the rc through to its caller.
>  * libxl__domaindeathcheck_init and _stop are not quite trivial any
>more so are moved from (inline functions) in libxl_internal.h, to
>ordinary functions defined in libxl_event.c.
>  * libxl__domaindeathcheck_start is not trivial any more, and now has
>the standard error-handling pattern.
> 
> The only current user of libxl__domaindeathcheck is the bootloader.
> So the result is that now it is possible to effectively cancel domain
> creation while the bootloader is running.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 




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


Re: [Xen-devel] [PATCH 24/29] libxl: Introduce FILLZERO

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> FILLZERO is a macro for memset(&foo,0,sizeof(foo)).  It eliminates the
> possiblity to make the error memset(&foo,0,sizeof(&foo)).

but not:
foo *p = allocate_a_foo()
 memset(p, 0, sizeof(p))
although that's probably less likely to go wrong and I don't think it
can be avoided by the sorts of tricks used here.

> No callers yet, but document it in CODING_STYLE.  (In accordance with
> existing libxl policy, I haven't gone through all existing possible
> call sites.)

We don't usually expose such helpers in the public API, but I suppose
you have a good reason to do so here, could you mention it in the commit
log please.

>  
> +
> +#define LIBXL_FILLZERO(object) (memset(&(object), 0, sizeof((object

Evaluates object twice, so LIBXL_FILEZERO(*(p++)), would behave
surprisingly. I'm not sure if this can be resolved though, so this might
be a Don't Do That Then situation.

Only answer I can think of is to require the type be passed to the macro
and validating it matches the object via a pointer comparison.

Ian.


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


Re: [Xen-devel] [PATCH 25/29] libxl: cancellation: Preparations for save/restore cancellation

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> Two unrelated non-functional changes, broken out into a pre-patch for
> easier review:
> 
> Break out a function sendsig() in libxl_save_callout.c.
> 
> Move io_fd to be a global variable in libxl_save_helper.c.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 26/29] libxl: cancellation: Handle SIGTERM in save/restore helper

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> During startup of the save/restore helper, set the disposition of
> SIGTERM appropriately.
> 
> For restore, we can simply die immediately - there is no point trying
> to do any kind of cleanup on what is now going to be a trashed domain.
> 
> For save, we want to arrange that libxc's cleanup code (eg turning off
> logdirty) takes place.  So our signal handler replaces the fd with one
> on which writes will fail, causing libxc's own loop to fail next time
> it actually tries to do a write.
> 
> Currently this has only a minor beneficial effect: we don't send the
> helper a SIGTERM ourselves, and if someone else contrives to send our
> helper a SIGTERM they have probably sent one to libxl too in which
> case things are going to be a bit messy anyway.
> 
> But in the next patch libxl is going to use SIGTERM itself on ao
> cancellation.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 27/29] libxl: cancellation: Cancel libxc save/restore

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> Register the the save/restore helper interface with the cancellation
> machinery.  When we are informed that save/restore should be
> cancelled, we make a note of the that in our rc variable, and send the
> helper a SIGTERM.  It will die in due course.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 




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


Re: [Xen-devel] [PATCH 28/29] libxl: ao: datacopier callback gets an rc

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> libxl__datacopier_* now provides its caller's callback function with
> an rc value.  This relieves the caller of the need to figure out an
> appropriate rc value.
> 
> Arrange that the `other internal failure' cases now get a valid
> positive errno value (EIO).
> 
> In a few places, assert that errno is nonzero before passing it to our
> caller.
> 
> Extend the datacopier callback API to permit the dc to signal
> CANCELLED.  (It doesn't actually do this yet, though.)
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 

Might be worth CC-ing the Migration-v2 folks on a 3rd version of this if
one happens.



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


Re: [Xen-devel] [PATCH 29/29] libxl: cancellation: Make datacopiers cancellable

2015-03-24 Thread Ian Campbell
On Tue, 2015-02-10 at 20:10 +, Ian Jackson wrote:
> libxl__datacopier_* can now actually generate a callback with
> rc==CANCELLED.
> 
> This provides cancellation during some corner cases, including (at
> least) copying the device model data during the end of domain save.
> 
> Signed-off-by: Ian Jackson 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 1/5] tools/mkrpm: improve version.release handling

2015-03-24 Thread Olaf Hering
On Mon, Mar 23, George Dunlap wrote:

> On 03/23/2015 06:49 PM, Olaf Hering wrote:
> > release will be anything after a dash, or 0. And an optional ".$PKG_RELEASE"
> > will be appended.
> Just trying to decode this: So if I were using PKG_RELEASE=$(date ...),
> then I'd get something like
> 
> xen-4.6-unstable.20150323160547.x86_64.rpm
> xen-4.4.2-0.20150323160547.x86_64.rpm
> 
> For non-release states and release states, respectively; and if I hadn't
> set PKG_RELEASE, those would come out like this:
> 
> xen-4.6-unstable.x86_64.rpm
> xen-4.4.2-0.x86_64.rpm
> 
> Is that about right?  That all looks good to me.

Ok, so I will send an updated patch.

Olaf

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


Re: [Xen-devel] ocaml libxl bindings and KeyedUnion

2015-03-24 Thread Olaf Hering
On Fri, Mar 20, Ian Campbell wrote:

> On Fri, 2015-03-20 at 17:44 +0100, Olaf Hering wrote:
> > + ("dev", Struct(None, [("m", libxl_vscsi_hctl)])),
> > + ("wwn", Struct(None, [("m", string)])),
> > + ("hctl", Struct(None, [("m", libxl_vscsi_hctl)])),
> 
> > > Aside: What is the difference between dev and hctl in this context?
> > 
> > Its supposed to represent either "/dev/something" and "h:ct:l".
> 
> So shouldn't dev by a string then?

No, the result will be written to the "p-dev" property.

> >  The
> > result in the "p-dev" property, which is used by the backend, is the
> > same.  But translating "p-dev" back into the config string for the
> > scsi-list command needs some way to represent that. I'm not fully happy
> > with the current way. Perhaps the code should just reuse the "p-devname"
> > property to tell what was in the config file.
> 
> Perhaps the list command should just list the canonical name for the
> device (i.e p-dev? a h:c:t:l tuple) and not worry about matching the
> config file?

I will think about this suggestion. Right now the scsilist command does
just what xm did.

> I'd even go as far as suggesting that the libxl API ought only to deal
> with the canonical name and that the toolstack can parse whatever forms
> it likes into that (perhaps using a helper from libxlu). That would make
> the libxl interface a lot simpler, wouldn't it?

Olaf

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


Re: [Xen-devel] [PATCH v2 7/7] tools: add tools support for Intel CAT

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 19:20 +0800, Chao Peng wrote:
> On Thu, Mar 19, 2015 at 12:51:19PM +, Ian Campbell wrote:
> > On Thu, 2015-03-19 at 18:41 +0800, Chao Peng wrote:
> > > This is the xc/xl changes to support Intel Cache Allocation
> > > Technology(CAT). Two commands are introduced:
> > > - xl psr-cat-cbm-set [-s socket]  
> > >   Set cache capacity bitmasks(CBM) for a domain.
> > > - xl psr-cat-show 
> > >   Show Cache Allocation Technology information.
> > > 
> > > Signed-off-by: Chao Peng 
> > > ---
> > >  tools/libxc/include/xenctrl.h |  15 +++
> > >  tools/libxc/xc_psr.c  |  74 ++
> > 
> > At the libxc level this looks like a reasonable wrapping of the
> > hypercall interface, so if the hypervisor folks are happy with that then
> > I'm happy with this.
> > 
> > >  tools/libxl/libxl.h   |  18 
> > 
> > At the libxl level things seem to be rather opaque to me, i.e. what is
> > domain_data? what does it contain? does it have any more semantics than
> > a 64-bit number?
> 
> The 64-bit number now holds the cache bitmask (CBM) for the domain. In
> the future, the number may represent the memory bandwidth throttling for
> the domain. So use a neutral name here.

How is the memory bandwidth throttling for a domain represented?

Is it necessary and/or desirable to funnel such distinct operations
through a single libxl API call? Why not add more semantically
meaningful APIs for each case or set of related cases?

> > What future new values might there be for libxl_psr_cat_type?
> 
> As far as I can see, besides L3_CBM, libxl_psr_cat_type may be L2_CBM or
> MEM_BANDWIDTH_THROTTLING in the future.

> > 
> > Also, does this function need to be in an ifdef, since it isn't called
> > unless LIBXL_HAVE_... is set? (IOW: does this compile for ARM)
> 
> libxl_get_physinfo is for all archs, so I think it compiles for ARM.

I was meaning hte other way around, the callers of get_phy_socket_num
are, I think, all within LIBXL_HAVE_ So on architectures which don't
set those defines this static function will be unused, which will cause
the compiler to complain and therefore the build will fail.

> 
> > 
> > 
> > > @@ -8057,6 +8070,202 @@ int main_psr_cmt_show(int argc, char **argv)
> > >  }
> > >  #endif
> > >  
> > > +#ifdef LIBXL_HAVE_PSR_CAT
> > > +static int psr_cat_l3_cbm_set(uint32_t domid, uint32_t socket, uint64_t 
> > > cbm)
> > > +{
> > > +int rc;
> > > +uint32_t i, nr_sockets;
> > > +
> > > +if (socket != ALL_SOCKETS) {
> > > +return libxl_psr_cat_set_domain_data(ctx, domid,
> > > + LIBXL_PSR_CAT_TYPE_L3_CBM,
> > > + socket, cbm);
> > > +} else {
> > > +nr_sockets = get_phy_socket_num();
> > 
> > I wonder if the libxl API ought to allow for an "all sockets" argument
> > and then do all this internally instead of making the callers do it?
> 
> I can of course. But I just think of the API semantic consistence for
> future, the "target" may be cpu but not socket if we support L2_CBM, so
> will that value still represent all cpus?

By extension having a way to say all cpus (or indeed "all whatevers")
seems likely to be useful too.

> > > +if (nr_sockets == 0) {
> > > +fprintf(stderr, "Failed getting physinfo\n");
> > > +return -1;
> > > +}
> > > +for (i = 0; i < nr_sockets; i++) {
> > > +rc = libxl_psr_cat_set_domain_data(ctx, domid,
> > > +   LIBXL_PSR_CAT_TYPE_L3_CBM,
> > > +   i, cbm);
> > > +/* Total L3 cache size */
> > > +printf("%-46s", "Total L3 Cache Size");
> > 
> > How wide are these lines going to be? Can we try and keep it to <80
> > columns? Perhaps you could include an example of the output of each of
> > the show functions in the commit message?
> 
> As socket number is variable, it's hard to strictly ensure <80 if all
> the sockets displayed in one line. Perhaps socket by socket display is
> the right direction.

Each socket is a column? I had assumed each was a row (which highlights
why examples of the output is useful!).

Ian


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


Re: [Xen-devel] [PATCH OSSTEST 1/2] uboot: use "readlink -f"

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 11:45 +, Wei Liu wrote:
> If the path is not a symlink, readlink by default returns empty string.
> Use "-f" to always return canonical path. This fixes the problem that
> xenpolicy file not getting loaded (because it is not a symlink).
> 
> Also change another spot that calls readlink to get xen binary path in
> case in the future we decide to not use symlink.
> 
> Signed-off-by: Wei Liu 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH OSSTEST 2/2] uboot: make flask loading address host property

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 11:45 +, Wei Liu wrote:
> Signed-off-by: Wei Liu 

Acked-by: Ian Campbell 




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


Re: [Xen-devel] [PATCH OSSTEST] README.dev: Document steps after restarting ms-ownerdaemon

2015-03-24 Thread Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST] README.dev: Document steps after 
restarting ms-ownerdaemon"):
> +After restarting ms-ownerdaemon
> +===
> +
> +... including after an (unexpected) restart of the controller VM.

Acked-by: Ian Jackson 

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


Re: [Xen-devel] [PATCH OSSTEST] start/stop the guest 10 times in the standard test jobs

2015-03-24 Thread Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST] start/stop the guest 10 times in the 
standard test jobs"):
> We already arrange by more adhoc means to repeat a localhost migration
> 10 times.

Assuming you have tested this and observed that it generates the right
testid,

Acked-by: Ian Jackson 

Ian.

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


Re: [Xen-devel] [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

2015-03-24 Thread Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST] Arrange for core dumps to be placed in 
/var/core and collect them"):
> Refactor the $kvp_replace helper in ts-xen-install into a generic
> helper (which requires using ::EO and ::EI for namespacing) for use
> with target_editfile [etc.]

Acked-by: Ian Jackson 

> I did debate making the presence of cores in /var/core fail the test
> (somehow), but I decided that would be annoying for standalone mode or
> shared host scenarios where the core files might be stale or related
> to another job.

I think you should add /var/core to the leak check step.

Ian.

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


Re: [Xen-devel] [PATCH OSSTEST] start/stop the guest 10 times in the standard test jobs

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 12:37 +, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST] start/stop the guest 10 times in the 
> standard test jobs"):
> > We already arrange by more adhoc means to repeat a localhost migration
> > 10 times.
> 
> Assuming you have tested this and observed that it generates the right
> testid,

Now you mention it, I have not, I should and will though.

> Acked-by: Ian Jackson 
> 
> Ian.



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


Re: [Xen-devel] [PATCH OSSTEST] Arrange for core dumps to be placed in /var/core and collect them

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 12:40 +, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST] Arrange for core dumps to be placed in 
> /var/core and collect them"):
> > Refactor the $kvp_replace helper in ts-xen-install into a generic
> > helper (which requires using ::EO and ::EI for namespacing) for use
> > with target_editfile [etc.]
> 
> Acked-by: Ian Jackson 
> 
> > I did debate making the presence of cores in /var/core fail the test
> > (somehow), but I decided that would be annoying for standalone mode or
> > shared host scenarios where the core files might be stale or related
> > to another job.
> 
> I think you should add /var/core to the leak check step.

Which in practice just means listing it in the arugment to
inventory_files?



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


Re: [Xen-devel] [PATCH OSSTEST] start/stop the guest 10 times in the standard test jobs

2015-03-24 Thread Ian Campbell
On Tue, 2015-03-24 at 12:42 +, Ian Campbell wrote:
> On Tue, 2015-03-24 at 12:37 +, Ian Jackson wrote:
> > Ian Campbell writes ("[PATCH OSSTEST] start/stop the guest 10 times in the 
> > standard test jobs"):
> > > We already arrange by more adhoc means to repeat a localhost migration
> > > 10 times.
> > 
> > Assuming you have tested this and observed that it generates the right
> > testid,
> 
> Now you mention it, I have not, I should and will though.

$ OSSTEST_SIMULATE=1 ./standalone run-job -h cam-st10 test-amd64-amd64-xl
$ grep testid logs/standalone/test-amd64-amd64-xl.log
[...]
2015-03-24 12:50:13 Z standalone.test-amd64-amd64-xl == 18 testid 
guest-stop/host/debian/;/host.repeat ==
[...]

So clearly not right! Adding:

diff --git a/sg-run-job b/sg-run-job
index be7b648..f2c70cf 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -316,7 +316,7 @@ proc test-guest-nomigr {g} {
 run-ts . =   ts-guest-stop+ host $g
 run-ts . =.2 ts-guest-start   + host $g
 
-repeat-ts 10 =.repeat + \
+repeat-ts 10 =.repeat \
+ ts-guest-stophost   $g \; \
+ ts-guest-start + host + $g +
 

results in "18 testid guest-start/debian.repeat" which I think is more
like it. See below.

Ian.

8<--

>From aae447a6b7b1c4e8d583375da4ed19451d844e75 Mon Sep 17 00:00:00 2001
From: Ian Campbell 
Date: Wed, 11 Mar 2015 10:08:57 +
Subject: [PATCH] start/stop the guest 10 times in the standard test jobs

The recent libvirt failures seemed to happen after a couple of
stop/start pairs, so arrange to start/stop each guest 10 times.

By cribbing from the existing use of repeat-ts I hope I've arranged
for a new test with testid ts-guest-start/debian.repeat.

We already arrange by more adhoc means to repeat a localhost migration
10 times.

Signed-off-by: Ian Campbell 
Acked-by: Ian Jackson 
---
v2: Correct placement of + to generate testid
guest-start/debian.repeat and not some rubbish.
---
 sg-run-job | 5 +
 1 file changed, 5 insertions(+)

diff --git a/sg-run-job b/sg-run-job
index a1ff24b..f2c70cf 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -315,6 +315,11 @@ proc test-guest {g} {
 proc test-guest-nomigr {g} {
 run-ts . =   ts-guest-stop+ host $g
 run-ts . =.2 ts-guest-start   + host $g
+
+repeat-ts 10 =.repeat \
+   + ts-guest-stophost   $g \; \
+   + ts-guest-start + host + $g +
+
 run-ts . =   ts-guest-destroy + host $g
 }
 
-- 
2.1.4




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


Re: [Xen-devel] [PATCH OSSTEST] start/stop the guest 10 times in the standard test jobs

2015-03-24 Thread Ian Jackson
Ian Campbell writes ("Re: [Xen-devel] [PATCH OSSTEST] start/stop the guest 10 
times in the standard test jobs"):
> From: Ian Campbell 
> Date: Wed, 11 Mar 2015 10:08:57 +
> Subject: [PATCH] start/stop the guest 10 times in the standard test jobs

Acked-by: Ian Jackson 

Thanks,
Ian.

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


[Xen-devel] [PATCH OSSTEST] standalone: Add --dry-run option for run-job.

2015-03-24 Thread Ian Campbell
Signed-off-by: Ian Campbell 
---
 standalone | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/standalone b/standalone
index caf3fd5..d8113e4 100755
--- a/standalone
+++ b/standalone
@@ -17,7 +17,7 @@ Operations:
   Setup dist path runvars for JOB from existing build results. Useful
   after recreating a flight to avoid the need to rebuild.
 
-* run-job [cfhrR] [JOB]
+* run-job [cfhrRs] [JOB]
 
   Run the named JOB.
 
@@ -42,6 +42,7 @@ Options:
 -h HOST, --host=HOST  Test host
 -r, --reuse   Do not wipe test host (default)
 -R, --noreuse, --noreinstall  Wipe the test host (if job or test does so)
+-s, --simulate, --dry-run Dry run, printing what would be run
 --lvextendmax=GB  Limit the size of the build logical volume
 --baselineCreate a baseline flight instead of a tip flight
 
@@ -63,7 +64,7 @@ if [ x$op = x--help ] ; then
 exit 0
 fi
 
-TEMP=$(getopt -o c:f:h:rR --long 
config:,flight:,host:,reuse,noreuse,reinstall,lvextendmax:,baseline,help -- 
"$@")
+TEMP=$(getopt -o c:f:h:rRs --long 
config:,flight:,host:,reuse,noreuse,reinstall,simulate,dry-run,lvextendmax:,baseline,help
 -- "$@")
 
 eval set -- "$TEMP"
 
@@ -71,6 +72,7 @@ config=${OSSTEST_CONFIG-$HOME/.xen-osstest/config}
 flight="standalone"
 host=
 reuse=1 # Don't blow away machines by default
+dryrun=0
 lvextendmax=50 # Leave some LVM space free for running tests
 nobaseline=y
 
@@ -81,6 +83,7 @@ while true ; do
-h|--host)   host=$2;   shift 2;;
-r|--reuse)  reuse=1;   shift 1;;
-R|--noreuse|--reinstall)reuse=0;shift 1;;
+   -s|--simulate|--dry-run)dryrun=1;shift 1;;
--lvextendmax)lvextendmax=$2; shift 2;;
--baseline)nobaseline=n; shift 1;;
--help)  usage; exit 0;;
@@ -241,6 +244,7 @@ case $op in
OSSTEST_FLIGHT=$flight \
OSSTEST_HOST_HOST=$host \
OSSTEST_HOST_REUSE=$reuse \
+   OSSTEST_SIMULATE=$dryrun \
with_logging logs/$flight/$job.log ./sg-run-job $job
;;
 run-test)
-- 
2.1.4


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


Re: [Xen-devel] [PATCH OSSTEST] standalone: Add --dry-run option for run-job.

2015-03-24 Thread Ian Jackson
Ian Campbell writes ("[PATCH OSSTEST] standalone: Add --dry-run option for 
run-job."):
> Signed-off-by: Ian Campbell 

Acked-by: Ian Jackson 

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


Re: [Xen-devel] [PATCH V13 5/7] xen/arm: Instruction prefetch abort (X) mem_event handling

2015-03-24 Thread Tamas K Lengyel
On Mon, Mar 23, 2015 at 5:47 PM, Tamas K Lengyel  wrote:

> On Mon, Mar 23, 2015 at 5:22 PM, Ian Campbell 
> wrote:
> > On Mon, 2015-03-23 at 16:47 +0100, Tamas K Lengyel wrote:
> >>
> >>
> >> On Mon, Mar 23, 2015 at 4:18 PM, Ian Campbell
> >>  wrote:
> >> On Mon, 2015-03-23 at 15:15 +, Ian Campbell wrote:
> >> > _But_ I suppose you are not really worried about the guest
> >> doing a PT
> >> > update, but rather xenaccess playing games with the stage 2
> >> behind the
> >> > guest's back, which might require us to do some TLB
> >> shootdowns, and we'd
> >> > have to assume both I-TLB and D-TLB since we don't know what
> >> the guest
> >> > has in those pages.
> >>
> >> When we change the p2m we do a tlbi vmalle1is, so regardless
> >> of whether
> >> split tlb is a thing we are flushing everything, so we should
> >> be good
> >> from a stage 2 PoV.
> >>
> >>
> >> When we apply p2m changes we do indeed flush the TLB. I do actually
> >> worry about a potential malicious in-guest kernel playing tricks with
> >> its pagetables which may have happened after the p2m changes were
> >> applied. The xen-access permissions are applied based on IPAs. Say I
> >> want to be notified when a specific API is being called, so I walk the
> >> guest-pagestables and set a trap at the page the IPA is one. If the
> >> malicious guest kernel wants to avoid triggering the xen-access
> >> notifications, it could theoretically prime its tables and perform the
> >> right accesses so that the iTLB still has the mapping I wanted to trap
> >> with xen-access, but the dTLB has a different mapping. The instruction
> >> abort trap will happen but in Xen I will see the mapping according to
> >> the dTLB, thus the radix-tree lookup fails and the trap is injected
> >> back into the guest.
> >
> > But in that case the guest will take a trap, so what has it gained other
> > than a spurious trap?
>
> Bypassing the notification being sent to the subscriber application
> about the trap.
>
> >
> > I can't see anything in the TLB chapter of the ARMv8 ARM which suggests
> > split TLBs are possible in AArch64 mode, and the AArch32 refs look like
> > backwards compat stuff to me.
> >
> > The ARMv7 ARM does mention the split, but only to say that the
> > distinction between the ITLB and DTLB maintenance operations is
> > deprecated and that modern TLB ops do not support the distinction. It's
> > not 100% unambiguous about whether split TLBs themselves are allowed in
> > ARMv7 though, and in particular I'm not sure what impact the addition of
> > the LPAE and virtualisation extensions will have had on these
> > requirements. I also can't quite tell if split TLB is allowed on VMSA
> > systems, or if it is PMSA only.
> >
> > As you might imagine I'm not keen on adding TLB flushes "just in case",
> > so I would really like to see some good references to the architecture
> > specs before adding a flush on this path.
> >
> > Ian.
>
> IMHO a TLB flush in this path is low impact for systems that don't use
> xen-access. As far as which hardware supports it, I see the following
> for A53 (
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0500f/BIIJFHJD.html
> ):
>
> 10-entry fully-associative instruction micro TLB.
> 10-entry fully-associative data micro TLB.
>
> For A57 (
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0500f/BIIJFHJD.html
> ):
>
> 48-entry fully-associative L1 instruction TLB.
> 32-entry fully-associative L1 data TLB for data load and store pipelines.
>
> Tamas
>

Ian,
according to the ARM ARM v8 split TLB is possible, see section TLB matching
(page 1826): "In some cases, the TLB can hold two mappings for the same
address". In fact, it seems like some hardware can even detect such cases
and cause a Data-abort or Prefetch abort with error code TLB Conflict (see
section TLB conflict aborts, page 1827). They did indeed deprecate the
separate maintenance operations but that doesn't really effect the problem
I'm describing.

It's unfortunate we can't make the MMU get us the translation as if doing a
prefetch, only as a data access. It's also unfortunate that during a
Permission fault we don't get the IPA associated with the fault, only the
GVA. I looked at the KVM code and they seem to query HPFAR_EL2 if the fault
is during s1ptw, but otherwise they do exactly what we do here.

So, IMHO doing a TLB flush here would prevent a primed DTLB becoming a
problem. Not sure if that helps if the ITLB is primed however as we would
have no way to replicate the translation that is cached..

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


[Xen-devel] [PATCH 0/3] Automatically derive soft affinity from vnuma information

2015-03-24 Thread Dario Faggioli
Hi,

This small series is meant at achieving the following two goals:
 - when building a vnuma enabled guest, if any vcpu hard or soft affinity is
   specified, and that does not match with the vnuma configuration supplied
   (e.g., vcpu X, belonging to vnode Y on pnode W, has affinity with pcpus
   outside of pnode W), we want to warn the user about it;
 - when building a vnuma enabled guest, if no _soft_ affinity is specified, we
   want to automatically build it up, basing on the vnuma configuration supplied
   (e.g., vcpu X, belonging to vnode Y on pnode W will have soft affinity with
   pcpus of pnode W).

IOW, with the following in the config file:

 vcpus  = '4'
 memory = '2000'
 vnuma  = [ [ "pnode=0","size=1000","vcpus=0-1","vdistances=10,20"  ],
[ "pnode=1","size=1000","vcpus=2-3","vdistances=20,10"  ] ]

We'll, automatically, end up with this:

 root@Zhaman:~# xl vcpu-list 1
 NameID  VCPU   CPU State   Time(s) Affinity 
(Hard / Soft)
 test-pv  1 07   -b-   8.1  all / 0-7
 test-pv  1 13   -b-   6.2  all / 0-7
 test-pv  1 2   10   -b-   0.1  all / 8-15
 test-pv  1 38   -b-   0.1  all / 8-15

While at it, I'm taking the chance (in patch #3) for doing some trivial
cleanups.

Thanks and Regards,
Dario

---
Dario Faggioli (3):
  libxl: check whether vcpu affinity and vnuma info match
  libxl: automatically set soft affinity after vnuma info
  libxl: cleanup some misuse of 'cpumap' as parameter

 tools/libxl/libxl_dom.c   |   46 ++
 tools/libxl/libxl_utils.h |6 +++--
 tools/libxl/libxl_vnuma.c |   54 +
 3 files changed, 102 insertions(+), 4 deletions(-)

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


[Xen-devel] [PATCH 1/3] libxl: check whether vcpu affinity and vnuma info match

2015-03-24 Thread Dario Faggioli
More specifically, vcpus are assigned to a vnode, which in
turn is associated with a pnode. If a vcpu also has, in its
(hard or soft) affinity, some pcpus that are not part of the
said pnode, print a warning to the user.

Signed-off-by: Dario Faggioli 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Stefano Stabellini 
Cc: Wei Liu 
---
 tools/libxl/libxl_vnuma.c |   54 +
 1 file changed, 54 insertions(+)

diff --git a/tools/libxl/libxl_vnuma.c b/tools/libxl/libxl_vnuma.c
index aad297e..3383724 100644
--- a/tools/libxl/libxl_vnuma.c
+++ b/tools/libxl/libxl_vnuma.c
@@ -33,6 +33,43 @@ static int compare_vmemrange(const void *a, const void *b)
 return 0;
 }
 
+/* Check if a vcpu has an hard (or soft) affinity set in such
+ * a way that it does not match the pnode to which the vcpu itself
+ * is assigned to.
+ */
+static int check_vnuma_affinity(libxl__gc *gc,
+ unsigned int vcpu,
+ unsigned int pnode,
+ unsigned int num_affinity,
+ const libxl_bitmap *affinity,
+ const char *kind)
+{
+libxl_bitmap nodemap;
+int rc = 0;
+
+libxl_bitmap_init(&nodemap);
+
+rc = libxl_node_bitmap_alloc(CTX, &nodemap, 0);
+if (rc) {
+LOG(ERROR, "Can't allocate nodemap");
+goto out;
+}
+
+rc = libxl_cpumap_to_nodemap(CTX, affinity, &nodemap);
+if (rc) {
+LOG(ERROR, "Can't convert Vcpu %d affinity to nodemap", vcpu);
+goto out;
+}
+
+if (libxl_bitmap_count_set(&nodemap) != 1 ||
+!libxl_bitmap_test(&nodemap, pnode))
+LOG(WARN, "Vcpu %d %s affinity and vnuma info mismatch", vcpu, kind);
+
+out:
+libxl_bitmap_dispose(&nodemap);
+return rc;
+}
+
 /* Check if vNUMA configuration is valid:
  *  1. all pnodes inside vnode_to_pnode array are valid
  *  2. each vcpu belongs to one and only one vnode
@@ -101,6 +138,23 @@ int libxl__vnuma_config_check(libxl__gc *gc,
 }
 }
 
+/* Check whether vcpu affinity (if any) matches vnuma configuration */
+for (i = 0; i < b_info->num_vnuma_nodes; i++) {
+v = &b_info->vnuma_nodes[i];
+libxl_for_each_set_bit(j, v->vcpus) {
+if (b_info->num_vcpu_hard_affinity > j)
+check_vnuma_affinity(gc, j, v->pnode,
+ b_info->num_vcpu_hard_affinity,
+ &b_info->vcpu_hard_affinity[j],
+ "hard");
+if (b_info->num_vcpu_soft_affinity > j)
+check_vnuma_affinity(gc, j, v->pnode,
+ b_info->num_vcpu_soft_affinity,
+ &b_info->vcpu_soft_affinity[j],
+ "soft");
+}
+}
+
 /* Check vmemranges */
 qsort(state->vmemranges, state->num_vmemranges, sizeof(xen_vmemrange_t),
   compare_vmemrange);


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


[Xen-devel] [PATCH 2/3] libxl: automatically set soft affinity after vnuma info

2015-03-24 Thread Dario Faggioli
More specifically, vcpus are assigned to a vnode, which in
turn is associated with a pnode. If a vcpu does not have any
soft affinity, automatically build up one, matching the pcpus
of the said pnode.

Signed-off-by: Dario Faggioli 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Stefano Stabellini 
Cc: Wei Liu 
---
 tools/libxl/libxl_dom.c |   44 
 1 file changed, 44 insertions(+)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index ace8a66..554ea68 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -439,6 +439,44 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
 return rc;
 }
 
+static int set_vnuma_affinity(libxl__gc *gc, uint32_t domid,
+  libxl_domain_build_info *info)
+{
+libxl_bitmap cpumap;
+libxl_vnode_info *v;
+unsigned int i, j;
+int rc = 0;
+
+libxl_bitmap_init(&cpumap);
+
+rc = libxl_cpu_bitmap_alloc(CTX, &cpumap, 0);
+if (rc) {
+LOG(ERROR, "Can't allocate nodemap");
+goto out;
+}
+
+/*
+ * For each vcpu in each vnode, set its soft affinity to
+ * the pcpus belonging to the pnode the vnode is on
+ */
+for (i = 0; i < info->num_vnuma_nodes; i++) {
+v = &info->vnuma_nodes[i];
+
+rc = libxl_node_to_cpumap(CTX, v->pnode, &cpumap);
+if (rc) {
+LOG(ERROR, "Can't get cpumap for vnode %d", i);
+goto out;
+}
+
+libxl_for_each_set_bit(j, v->vcpus)
+libxl_set_vcpuaffinity(CTX, domid, j, NULL, &cpumap);
+}
+
+out:
+libxl_bitmap_dispose(&cpumap);
+return rc;
+}
+
 int libxl__build_post(libxl__gc *gc, uint32_t domid,
   libxl_domain_build_info *info,
   libxl__domain_build_state *state,
@@ -450,6 +488,12 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
 char **ents;
 int i, rc;
 
+if (info->num_vnuma_nodes && !info->num_vcpu_soft_affinity) {
+rc = set_vnuma_affinity(gc, domid, info);
+if (rc)
+return rc;
+}
+
 rc = libxl_domain_sched_params_set(CTX, domid, &info->sched_params);
 if (rc)
 return rc;


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


[Xen-devel] [PATCH 3/3] libxl: cleanup some misuse of 'cpumap' as parameter

2015-03-24 Thread Dario Faggioli
in favour of the more generic 'bitmap', which is better
since these are generic libxl_bitmap_* functions.

Also fix a typo, and remove a stale (and wrong) comment.

No functional change intended.

Signed-off-by: Dario Faggioli 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Stefano Stabellini 
Cc: Wei Liu 
---
 tools/libxl/libxl_dom.c   |2 +-
 tools/libxl/libxl_utils.h |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 554ea68..26a0382 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -376,7 +376,7 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
 }
 if (info->nodemap.size)
 libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap);
-/* As mentioned in libxl.h, vcpu_hard_array takes precedence */
+
 if (info->num_vcpu_hard_affinity || info->num_vcpu_soft_affinity) {
 libxl_bitmap *hard_affinity, *soft_affinity;
 int i, n_vcpus;
diff --git a/tools/libxl/libxl_utils.h b/tools/libxl/libxl_utils.h
index acacdd9..68b5580 100644
--- a/tools/libxl/libxl_utils.h
+++ b/tools/libxl/libxl_utils.h
@@ -89,8 +89,8 @@ int libxl_bitmap_is_empty(const libxl_bitmap *bitmap);
 int libxl_bitmap_test(const libxl_bitmap *bitmap, int bit);
 void libxl_bitmap_set(libxl_bitmap *bitmap, int bit);
 void libxl_bitmap_reset(libxl_bitmap *bitmap, int bit);
-int libxl_bitmap_count_set(const libxl_bitmap *cpumap);
-char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *cpumap);
+int libxl_bitmap_count_set(const libxl_bitmap *bitmap);
+char *libxl_bitmap_to_hex_string(libxl_ctx *ctx, const libxl_bitmap *bitmap);
 static inline void libxl_bitmap_set_any(libxl_bitmap *bitmap)
 {
 memset(bitmap->map, -1, bitmap->size);
@@ -145,7 +145,7 @@ int libxl_node_to_cpumap(libxl_ctx *ctx, int node,
  libxl_bitmap *cpumap);
 /* Populate nodemap with the nodes of the cpus in cpumap */
 int libxl_cpumap_to_nodemap(libxl_ctx *ctx,
-const libxl_bitmap *cpuemap,
+const libxl_bitmap *cpumap,
 libxl_bitmap *nodemap);
 
  static inline uint32_t libxl__sizekb_to_mb(uint32_t s) {


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


Re: [Xen-devel] [PATCH 2/2] x86: Don't use BAD_APICID for non-APICID fields

2015-03-24 Thread Boris Ostrovsky

On 03/24/2015 04:07 AM, Jan Beulich wrote:

On 23.03.15 at 20:54,  wrote:

BAD_APICID is used by cpuinfo_x86's phys_proc_id, cpu_core_id
and compute_unit_id even though these fields don't hold an APIC ID
itself but rather its derivative.

Provide appropriate macros for each of those three (and make them
unsigned).

This also fixes regression introduced by commit 2090f14c5cbd ("sysctl:
make XEN_SYSCTL_topologyinfo sysctl a little more efficient") which
leaked BAD_APICID into common code, breaking ARM.

Signed-off-by: Boris Ostrovsky 
Reported-by: Julien Grall 
---

I left sysctl with

 cputopo.core = cpu_to_core(i);
 if ( cputopo.core == INVALID_COREID )
 cputopo.core = XEN_INVALID_CORE_ID;
 cputopo.socket = cpu_to_socket(i);
 if ( cputopo.socket == INVALID_SOCKETID )
 cputopo.socket = XEN_INVALID_SOCKET_ID;

since this is the only place that would use suggested inlines for external
(public) calls.

But again - why did you not avoid the new #defines and use the
XEN_-prefixed ones right away?


I did want to use XEN_* macros but then I realized that I need a new 
#define for cpuinfo_x86.compute_unit_id as well. This #define does not 
need to be public so for consistency I thought that having 
hypervisor-internal INVALID_* macros would be better.



-boris


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


Re: [Xen-devel] Xen unstability on HP Moonshot m400

2015-03-24 Thread Mark Salter
On Mon, 2015-03-23 at 23:58 +, Stefano Stabellini wrote:
> On Mon, 23 Mar 2015, Christoffer Dall wrote:
> > On Mon, Mar 23, 2015 at 1:36 PM, Ian Campbell  
> > wrote:
> >   On Sat, 2015-03-21 at 13:34 +0100, Christoffer Dall wrote:
> >   > Hi,
> >   >
> >   > I have been experiencing a problematic crash running Xen on m400 
> > over
> >   > the last few days.  I already spoke to Ian and Stefano about this, 
> > but
> >   > thought I'd summarize what I've seen so far and loop in a wider
> >   > audience.
> >   >
> >   > The basic setup is this:
> >   >  - Two m400 nodes, one running Linux bare-metal, the other running
> >   > Xen.
> >   >  - The Xen node runs Dom0 and 1 DomU
> >   >  - The m400 has a Mellanox Connectx-3 PCIe 10G ethernet card with 
> > two
> >   > parts on it
> >   >  - Dom0 uses NAT forwarding from Dom0's eth0 (which is connected to
> >   > the internet) and regular bridging to eth1 which is connected to a
> >   > private VLAN to the bare-metal node
> >   >  - Dom0 and DomU are configured with 14GB of ram, 4 cpus each
> >   >  - DomU runs apache2 serving the GCC manual (see
> >   > 
> > https://github.com/chazy/kvmperf/blob/master/cmdline_tests/apache_install.sh)
> >   >
> >   > The bare-metal node runs apache bench, like this: "ab -n 10 -c 
> > 100
> >   
> > >http://secure-web.cisco.com/1r5tZ8-7RF8gHRANwFdizEZzgeMsjxVO0yKbYiV4zy7LeiUfYBXMkFq7FGW_SZ1x-VxdzyK-ErDsOUiQ9z2x-N
> > y7XkL_loHP8ene_BuNFscGyWmQ3r6CtXAYaZCY4xRmmPT1uJOsZDLMu7j-LfCOGmQDSdBwgW7QYukI2bCtTrXM/http%3A%2F%2F10.10.1.120%2F
> >   gcc%2Findex.html"
> >   >
> >   > (10.10.1.120 is the DomU IP address of the bridged interface to 
> > eth1)
> >   >
> >   > What happens now is that the entire Xen node goes down.  I see 
> > various
> >   > errors in the kernel log, some examples:
> >   > http://pastebin.ubuntu.com/10642148/
> >   > http://pastebin.ubuntu.com/10642177/
> >   > http://pastebin.ubuntu.com/10642181/
> >   > http://pastebin.ubuntu.com/10635573/
> >   >
> >   >
> >   > All Linux kernels are 3.18 plus some tweaks for the m400 cartridge:
> >   > https://github.com/columbia/linux-kvm-arm/tree/columbia-armvirt-3.18
> > 
> >   Is it worth adding
> >   
> > https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/commit/?id=285994a62c80f1d72c6924282bcb59608098d5ec
> >   to your kernel? It isn't Xen specific but it's perhaps possible that 
> > Xen opens the window wider.

You definitely want that one. Without it, the page table walker could
end up using a stale pointer to a page being used for something other
than page tables.

> > 
> >   How confident are you in
> >   
> > https://github.com/columbia/linux-kvm-arm/commit/5e29cb0478f3d90e4f568d6bea6840960331bcbb
> >  ?
> >   (although I suppose you aren't running in ACPI mode if you are running
> >   Xen?)
> > 
> > 
> > I'm not confident at all, but Linux (last I checked was v3.19) doesn't boot 
> > without it, so not sure if there's an
> > alternative?  Mark?
> 
> This patch is key: it doesn't look like it is setting
> dev->archdata.dma_coherent appropriately, see the implementation of
> set_arch_dma_coherent_ops.

You'd want this if booting with ACPI. You might also need it for
enumerated PCI devices even if booting with devicetree.

> 
> 
> > 
> >   If we think the issue might be to do with coherency of foreign 
> > mappings
> >   undergoing i/o from dom0 and we've already ruled out disk (by using a
> >   loopback mounted rootfs) then it might be worth bodging netback to
> >   always copy too.
> > 
> >   Adding a call to skb_orphan_frags right before the netif_receive_skb 
> > in
> >   drivers/net/xen-netback/netback.c:xenvif_tx_submit is a simple but
> >   rather inefficient way of doing that (so I hope it doesn't perturb the
> >   issue).
> > 
> > 
> > I'll be happy to try this.
> 
> If we are right and the problem is due to the commit above not setting
> dma_coherent to true (the kernel will think that actually the network
> card is not coherent), then Ian's workaround should hide the problem.
> 
> 
> > 
> >   Stefano (who is more familiar with the Linux swiotlb side of things 
> > than
> >   me) is travelling this week so he'll be on West coast time, not sure
> >   when he gets off a plane nor if he's on email anyway (he's at ELC + 
> > this
> >   ARM ACPI thing)
> > 
> > 
> > ok, we'll see what happens.
> > 
> > -Christoffer
> > 
> > 



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


Re: [Xen-devel] [PATCH v19 05/14] x86/VPMU: Interface for setting PMU mode and flags

2015-03-24 Thread Jan Beulich
>>> On 17.03.15 at 15:54,  wrote:
> Add runtime interface for setting PMU mode and flags. Three main modes are
> provided:
> * XENPMU_MODE_OFF:  PMU is not virtualized
> * XENPMU_MODE_SELF: Guests can access PMU MSRs and receive PMU interrupts.
> * XENPMU_MODE_HV: Same as XENPMU_MODE_SELF for non-proviledged guests, dom0
>   can profile itself and the hypervisor.
> 
> Note that PMU modes are different from what can be provided at Xen's boot 
> line
> with 'vpmu' argument. An 'off' (or '0') value is equivalent to 
> XENPMU_MODE_OFF.
> Any other value, on the other hand, will cause VPMU mode to be set to
> XENPMU_MODE_SELF during boot.
> 
> For feature flags only Intel's BTS is currently supported.
> 
> Mode and flags are set via HYPERVISOR_xenpmu_op hypercall.
> 
> Signed-off-by: Boris Ostrovsky 
> Acked-by: Daniel De Graaf 

Acked-by: Jan Beulich 


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


Re: [Xen-devel] Xen unstability on HP Moonshot m400

2015-03-24 Thread Mark Salter
On Tue, 2015-03-24 at 09:54 -0400, Mark Salter wrote:
> On Mon, 2015-03-23 at 23:58 +, Stefano Stabellini wrote:
> > On Mon, 23 Mar 2015, Christoffer Dall wrote:
> > > On Mon, Mar 23, 2015 at 1:36 PM, Ian Campbell  
> > > wrote:
> > >   On Sat, 2015-03-21 at 13:34 +0100, Christoffer Dall wrote:
> > >   > Hi,
> > >   >
> > >   > I have been experiencing a problematic crash running Xen on m400 
> > > over
> > >   > the last few days.  I already spoke to Ian and Stefano about 
> > > this, but
> > >   > thought I'd summarize what I've seen so far and loop in a wider
> > >   > audience.
> > >   >
> > >   > The basic setup is this:
> > >   >  - Two m400 nodes, one running Linux bare-metal, the other running
> > >   > Xen.
> > >   >  - The Xen node runs Dom0 and 1 DomU
> > >   >  - The m400 has a Mellanox Connectx-3 PCIe 10G ethernet card with 
> > > two
> > >   > parts on it
> > >   >  - Dom0 uses NAT forwarding from Dom0's eth0 (which is connected 
> > > to
> > >   > the internet) and regular bridging to eth1 which is connected to a
> > >   > private VLAN to the bare-metal node
> > >   >  - Dom0 and DomU are configured with 14GB of ram, 4 cpus each
> > >   >  - DomU runs apache2 serving the GCC manual (see
> > >   > 
> > > https://github.com/chazy/kvmperf/blob/master/cmdline_tests/apache_install.sh)
> > >   >
> > >   > The bare-metal node runs apache bench, like this: "ab -n 10 
> > > -c 100
> > >   
> > > >http://secure-web.cisco.com/1r5tZ8-7RF8gHRANwFdizEZzgeMsjxVO0yKbYiV4zy7LeiUfYBXMkFq7FGW_SZ1x-VxdzyK-ErDsOUiQ9z2x-N
> > > y7XkL_loHP8ene_BuNFscGyWmQ3r6CtXAYaZCY4xRmmPT1uJOsZDLMu7j-LfCOGmQDSdBwgW7QYukI2bCtTrXM/http%3A%2F%2F10.10.1.120%2F
> > >   gcc%2Findex.html"
> > >   >
> > >   > (10.10.1.120 is the DomU IP address of the bridged interface to 
> > > eth1)
> > >   >
> > >   > What happens now is that the entire Xen node goes down.  I see 
> > > various
> > >   > errors in the kernel log, some examples:
> > >   > http://pastebin.ubuntu.com/10642148/
> > >   > http://pastebin.ubuntu.com/10642177/
> > >   > http://pastebin.ubuntu.com/10642181/
> > >   > http://pastebin.ubuntu.com/10635573/
> > >   >
> > >   >
> > >   > All Linux kernels are 3.18 plus some tweaks for the m400 
> > > cartridge:
> > >   > 
> > > https://github.com/columbia/linux-kvm-arm/tree/columbia-armvirt-3.18
> > > 
> > >   Is it worth adding
> > >   
> > > https://git.kernel.org/cgit/linux/kernel/git/arm64/linux.git/commit/?id=285994a62c80f1d72c6924282bcb59608098d5ec
> > >   to your kernel? It isn't Xen specific but it's perhaps possible 
> > > that Xen opens the window wider.
> 
> You definitely want that one. Without it, the page table walker could
> end up using a stale pointer to a page being used for something other
> than page tables.
> 
> > > 
> > >   How confident are you in
> > >   
> > > https://github.com/columbia/linux-kvm-arm/commit/5e29cb0478f3d90e4f568d6bea6840960331bcbb
> > >  ?
> > >   (although I suppose you aren't running in ACPI mode if you are 
> > > running
> > >   Xen?)
> > > 
> > > 
> > > I'm not confident at all, but Linux (last I checked was v3.19) doesn't 
> > > boot without it, so not sure if there's an
> > > alternative?  Mark?
> > 
> > This patch is key: it doesn't look like it is setting
> > dev->archdata.dma_coherent appropriately, see the implementation of
> > set_arch_dma_coherent_ops.
> 
> You'd want this if booting with ACPI. You might also need it for
> enumerated PCI devices even if booting with devicetree.

There's an updated version of this patch for newer kernels in the
devel branch of git.fedorahosted.org/git/kernel-arm64.git

There is also this one in Linus' tree which may be of interest to you:

commit 7132813c384515c9dede1ae20e56f3895feb7f1e
Author: Suzuki K. Poulose 
Date:   Thu Mar 19 18:17:09 2015 +

arm64: Honor __GFP_ZERO in dma allocations

> > 
> > 
> > > 
> > >   If we think the issue might be to do with coherency of foreign 
> > > mappings
> > >   undergoing i/o from dom0 and we've already ruled out disk (by using 
> > > a
> > >   loopback mounted rootfs) then it might be worth bodging netback to
> > >   always copy too.
> > > 
> > >   Adding a call to skb_orphan_frags right before the 
> > > netif_receive_skb in
> > >   drivers/net/xen-netback/netback.c:xenvif_tx_submit is a simple but
> > >   rather inefficient way of doing that (so I hope it doesn't perturb 
> > > the
> > >   issue).
> > > 
> > > 
> > > I'll be happy to try this.
> > 
> > If we are right and the problem is due to the commit above not setting
> > dma_coherent to true (the kernel will think that actually the network
> > card is not coherent), then Ian's workaround should hide the problem.
> > 
> > 
> > > 
> > >   Stefano (who is more familiar with the Linux swiotlb side of things 
> > > than
> > > 

Re: [Xen-devel] [PATCH v2] xen/passthrough: Support a single iommu_domain per xen domain per SMMU

2015-03-24 Thread Robert VanVossen


On 3/24/2015 7:07 AM, Manish Jaggi wrote:
> 
> On Monday 23 March 2015 07:16 PM, Robbie VanVossen wrote:
>> If multiple devices are being passed through to the same domain and they
>> share a single SMMU, then they only require a single iommu_domain.
>>
>> In arm_smmu_assign_dev, before a new iommu_domain is created, the
>> xen_domain->contexts is checked for any iommu_domains that are already
>> assigned to device that uses the same SMMU as the current device. If one
>> is found, attach the device to that iommu_domain. If a new one isn't
>> found, create a new iommu_domain just like before.
>>
>> The arm_smmu_deassign_dev function assumes that there is a single
>> device per iommu_domain. This meant that when the first device was
>> deassigned, the iommu_domain was freed and when another device was
>> deassigned a crash occured in xen.
>>
>> To fix this, a reference counter was added to the iommu_domain struct.
>> When an arm_smmu_xen_device references an iommu_domain, the
>> iommu_domains ref is incremented. When that reference is removed, the
>> iommu_domains ref is decremented. The iommu_domain will only be freed
>> when the ref is 0.
>>
>> Signed-off-by: Robbie VanVossen 
> Hi,
> Are you adding a PCI passthrough support to Xen ?. I am in process of 
> sending smmu driver patches based on juliens latest code.
>

Nope, I am just working on what this patch describes

Thanks,
Robbie VanVossen
DornerWorks


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


Re: [Xen-devel] [PATCH v19 07/14] x86/VPMU: Initialize PMU for PV(H) guests

2015-03-24 Thread Jan Beulich
>>> On 17.03.15 at 15:54,  wrote:
> @@ -263,14 +264,14 @@ void vpmu_initialise(struct vcpu *v)
>  BUILD_BUG_ON(sizeof(struct xen_pmu_intel_ctxt) > XENPMU_CTXT_PAD_SZ);
>  BUILD_BUG_ON(sizeof(struct xen_pmu_amd_ctxt) > XENPMU_CTXT_PAD_SZ);
>  
> -if ( is_pvh_vcpu(v) )
> -return;
> -
>  ASSERT(!vpmu->flags && !vpmu->context);
>  
> -spin_lock(&vpmu_lock);
> -vpmu_count++; /* Prevent vpmu_mode from changing until we are done */
> -spin_unlock(&vpmu_lock);
> +if ( v->domain != hardware_domain )
> +{
> +spin_lock(&vpmu_lock);
> +vpmu_count++; /* Prevent vpmu_mode from changing until we are done */
> +spin_unlock(&vpmu_lock);
> +}

So why is this being made conditional here? A patch this size would
certainly benefit from a slightly more verbose description...

And then if the conditional is indeed needed - why don't you use
is_hardware_domain()?

> +static int pvpmu_init(struct domain *d, xen_pmu_params_t *params)
> +{
> +struct vcpu *v;
> +struct vpmu_struct *vpmu;
> +struct page_info *page;
> +uint64_t gfn = params->val;
> +
> +if ( vpmu_mode == XENPMU_MODE_OFF )
> +return -EINVAL;
> +
> +if ( (params->vcpu >= d->max_vcpus) || (d->vcpu == NULL) ||
> + (d->vcpu[params->vcpu] == NULL) )
> +return -EINVAL;
> +
> +page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
> +if ( !page )
> +return -EINVAL;
> +
> +if ( !get_page_type(page, PGT_writable_page) )
> +{
> +put_page(page);
> +return -EINVAL;
> +}
> +
> +v = d->vcpu[params->vcpu];
> +vpmu = vcpu_vpmu(v);
> +
> +spin_lock(&vpmu->vpmu_lock);
> +
> +if ( v->arch.vpmu.xenpmu_data )
> +{
> +put_page_and_type(page);
> +spin_unlock(&vpmu->vpmu_lock);

These two lines should be switched.

> +return -EEXIST;
> +}
> +
> +v->arch.vpmu.xenpmu_data = __map_domain_page_global(page);
> +if ( !v->arch.vpmu.xenpmu_data )
> +{
> +put_page_and_type(page);
> +spin_unlock(&vpmu->vpmu_lock);

Same here.

> +static void pvpmu_finish(struct domain *d, xen_pmu_params_t *params)
> +{
> +struct vcpu *v;
> +struct vpmu_struct *vpmu;
> +uint64_t mfn;
> +
> +if ( (params->vcpu >= d->max_vcpus) || (d->vcpu == NULL) ||
> + (d->vcpu[params->vcpu] == NULL) )
> +return;
> +
> +v = d->vcpu[params->vcpu];
> +if ( v != current )
> +vcpu_pause(v);
> +
> +vpmu = vcpu_vpmu(v);
> +spin_lock(&vpmu->vpmu_lock);
> +
> +vpmu_destroy(v);
> +
> +if ( v->arch.vpmu.xenpmu_data )
> +{
> +mfn = domain_page_map_to_mfn(v->arch.vpmu.xenpmu_data);
> +ASSERT(mfn != 0);
> +unmap_domain_page_global(v->arch.vpmu.xenpmu_data);
> +put_page_and_type(mfn_to_page(mfn));
> +v->arch.vpmu.xenpmu_data = NULL;
> +}
> +
> +spin_unlock(&vpmu->vpmu_lock);

Similarly here the put should be moved out of the locked region.

Jan


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


Re: [Xen-devel] [PATCH v19 08/14] x86/VPMU: Save VPMU state for PV guests during context switch

2015-03-24 Thread Jan Beulich
>>> On 17.03.15 at 15:54,  wrote:
> Save VPMU state during context switch for both HVM and PV(H) guests.
> 
> A subsequent patch ("x86/VPMU: NMI-based VPMU support") will make it possible
> for vpmu_switch_to() to call vmx_vmcs_try_enter()->vcpu_pause() which needs
> is_running to be correctly set/cleared. To prepare for that, call 
> context_saved()
> before vpmu_switch_to() is executed. (Note that while this change could have
> been dalayed until that later patch, the changes are harmless to existing 
> code
> and so we do it here)
> 
> Signed-off-by: Boris Ostrovsky 

Reviewed-by: Jan Beulich 


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


Re: [Xen-devel] [PATCH 2/2] make {,g}dprintk() a no‑op in non‑debug builds

2015-03-24 Thread Ian Campbell
On Fri, 2015-03-20 at 15:13 +, Jan Beulich wrote:
> As discussed before, code absolutely wanting to get something into the
> log should use {,g}printk() instead.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Ian Campbell 



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


Re: [Xen-devel] [PATCH 1/2] introduce gprintk()

2015-03-24 Thread Ian Campbell
On Fri, 2015-03-20 at 15:12 +, Jan Beulich wrote:
> ... and convert several gdprintk()-s to it, as the next patch will make
> them no-ops in non-debug builds.
> 
> Note that as a non-debug facility this does not print file name and
> line number of the origin, to people are expected to use meaningful and
> easily distinguishable messages (i.e. just like with plain printk()).
> 
> Signed-off-by: Jan Beulich 

Acked-by: Ian Campbell 

> ---
> Note: ARM code was not touched.

I've done a pass, this goes after this series.

8<-


>From 6a979279a489d60d29d12ff8639689e224e0bfdf Mon Sep 17 00:00:00 2001
From: Ian Campbell 
Date: Tue, 24 Mar 2015 14:17:40 +
Subject: [PATCH] xen: arm: use gprintk as appropriate

gdprintk is now only included with debug=y builds. Therefore:
 - switch some uses to gprintk
 - remove some now redundant #ifndef NDEBUG surrounding existing
   gdprintk uses.

Signed-off-by: Ian Campbell 
---
 xen/arch/arm/decode.c  |6 +++---
 xen/arch/arm/traps.c   |   20 +++-
 xen/arch/arm/vgic-v3.c |4 ++--
 xen/arch/arm/vgic.c|   10 +-
 xen/arch/arm/vtimer.c  |2 +-
 5 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/xen/arch/arm/decode.c b/xen/arch/arm/decode.c
index 9d237f8..c6f49a5 100644
--- a/xen/arch/arm/decode.c
+++ b/xen/arch/arm/decode.c
@@ -78,7 +78,7 @@ static int decode_thumb2(register_t pc, struct hsr_dabt 
*dabt, uint16_t hw1)
 return 0;
 
 bad_thumb2:
-gdprintk(XENLOG_ERR, "unhandled THUMB2 instruction 0x%x%x\n", hw1, hw2);
+gprintk(XENLOG_ERR, "unhandled THUMB2 instruction 0x%x%x\n", hw1, hw2);
 
 return 1;
 }
@@ -145,7 +145,7 @@ static int decode_thumb(register_t pc, struct hsr_dabt 
*dabt)
 return 0;
 
 bad_thumb:
-gdprintk(XENLOG_ERR, "unhandled THUMB instruction 0x%x\n", instr);
+gprintk(XENLOG_ERR, "unhandled THUMB instruction 0x%x\n", instr);
 return 1;
 }
 
@@ -155,7 +155,7 @@ int decode_instruction(const struct cpu_user_regs *regs, 
struct hsr_dabt *dabt)
 return decode_thumb(regs->pc, dabt);
 
 /* TODO: Handle ARM instruction */
-gdprintk(XENLOG_ERR, "unhandled ARM instruction\n");
+gprintk(XENLOG_ERR, "unhandled ARM instruction\n");
 
 return 1;
 }
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 89cbde6..7216daf 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1674,14 +1674,12 @@ static void do_cp15_32(struct cpu_user_regs *regs,
 break;
 
 default:
-#ifndef NDEBUG
 gdprintk(XENLOG_ERR,
  "%s p15, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
  cp32.read ? "mrc" : "mcr",
  cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
 gdprintk(XENLOG_ERR, "unhandled 32-bit CP15 access %#x\n",
  hsr.bits & HSR_CP32_REGS_MASK);
-#endif
 inject_undef_exception(regs, hsr.len);
 return;
 }
@@ -1709,7 +1707,6 @@ static void do_cp15_64(struct cpu_user_regs *regs,
 break;
 default:
 {
-#ifndef NDEBUG
 struct hsr_cp64 cp64 = hsr.cp64;
 
 gdprintk(XENLOG_ERR,
@@ -1718,7 +1715,6 @@ static void do_cp15_64(struct cpu_user_regs *regs,
  cp64.op1, cp64.reg1, cp64.reg2, cp64.crm, regs->pc);
 gdprintk(XENLOG_ERR, "unhandled 64-bit CP15 access %#x\n",
  hsr.bits & HSR_CP64_REGS_MASK);
-#endif
 inject_undef_exception(regs, hsr.len);
 return;
 }
@@ -1780,14 +1776,12 @@ static void do_cp14_32(struct cpu_user_regs *regs, 
union hsr hsr)
 
 default:
 bad_cp:
-#ifndef NDEBUG
 gdprintk(XENLOG_ERR,
  "%s p14, %d, r%d, cr%d, cr%d, %d @ 0x%"PRIregister"\n",
   cp32.read ? "mrc" : "mcr",
   cp32.op1, cp32.reg, cp32.crn, cp32.crm, cp32.op2, regs->pc);
 gdprintk(XENLOG_ERR, "unhandled 32-bit cp14 access %#x\n",
  hsr.bits & HSR_CP32_REGS_MASK);
-#endif
 inject_undef_exception(regs, hsr.len);
 return;
 }
@@ -1797,9 +1791,7 @@ bad_cp:
 
 static void do_cp14_dbg(struct cpu_user_regs *regs, union hsr hsr)
 {
-#ifndef NDEBUG
 struct hsr_cp64 cp64 = hsr.cp64;
-#endif
 
 if ( !check_conditional_instr(regs, hsr) )
 {
@@ -1807,22 +1799,19 @@ static void do_cp14_dbg(struct cpu_user_regs *regs, 
union hsr hsr)
 return;
 }
 
-#ifndef NDEBUG
 gdprintk(XENLOG_ERR,
  "%s p14, %d, r%d, r%d, cr%d @ 0x%"PRIregister"\n",
  cp64.read ? "mrrc" : "mcrr",
  cp64.op1, cp64.reg1, cp64.reg2, cp64.crm, regs->pc);
 gdprintk(XENLOG_ERR, "unhandled 64-bit CP14 access %#x\n",
  hsr.bits & HSR_CP64_REGS_MASK);
-#endif
+
 inject_undef_exception(regs, hsr.len);
 }
 
 static void do_cp(struct cpu_user_regs *regs, union hsr hsr)
 {
-#ifndef NDEBUG
 struct hsr_cp cp = hsr.cp;
-#endif
 
 if ( !check_conditional_instr(regs, hsr) )
 {
@@ -1830,10 +1819,8 @@ static vo

[Xen-devel] [PATCH OSSTEST v2] Arrange for core dumps to be placed in /var/core and collect them

2015-03-24 Thread Ian Campbell
Refactor the $kvp_replace helper in ts-xen-install into a generic
helper (which requires using ::EO and ::EI for namespacing) for use
with target_editfile and use it to edit /etc/sysctl.conf to set
kernel.core_pattern on boot.

Tested in standalone mode by installing and running a C program
containing "*(int *)0 = 1;" which, after running "ulimit -c unlimited"
produces the expected core file. ts-logs-capture when run in
standalone mode then picks them up.

I've not yet figured out how to make the desired rlimit take affect
for all processes (including e.g. daemons spawned on boot). Likely
this will involve some combination of pam_limits.so PAM module and
adding explicit ulimit calls to the initscripts which we care about
(primarily xencommons and libvirt initscripts).

Signed-off-by: Ian Campbell 
Acked-by: Ian Jackson 
---
v2: Add /var/core to ts-leak-check
---
 Osstest/TestSupport.pm | 22 ++
 ts-host-install|  8 
 ts-leak-check  |  2 +-
 ts-logs-capture|  2 ++
 ts-xen-install | 19 ++-
 5 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 8754e22..ece2282 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -57,6 +57,7 @@ BEGIN {
   target_put_guest_image target_editfile
   target_editfile_cancel
   target_editfile_root target_file_exists
+  target_editfile_kvp_replace
   target_run_apt
   target_install_packages target_install_packages_norec
   target_jobdir target_extract_jobdistpath_subdir
@@ -542,6 +543,27 @@ sub teditfileex {
if $install;
 }
 
+# Replace a Key=Value style line in a config file.
+#
+# To be used as 3rd argument to target_editfile(_root) as:
+#target_editfile_root($ho, "/path/to/a/file",
+#   sub { target_editfile_kvp_replace($key, $value) });
+sub target_editfile_kvp_replace ($$)
+{
+my ($key,$value) = @_;
+my $prnow;
+$prnow= sub {
+   print ::EO "$key=$value\n" or die $!;
+   $prnow= sub { };
+};
+while (<::EI>) {
+   print ::EO or die $! unless m/^$key\b/;
+   $prnow->() if m/^#$key/;
+}
+print ::EO "\n" or die $!;
+$prnow->();
+};
+
 sub target_editfile_root ($$$;$$) { teditfileex('root',@_); }
 sub target_editfile  ($$$;$$) { teditfileex('osstest',@_); }
 # my $code= pop @_;
diff --git a/ts-host-install b/ts-host-install
index 9656079..2e3e84b 100755
--- a/ts-host-install
+++ b/ts-host-install
@@ -139,6 +139,14 @@ END
});
 }
 
+target_cmd_root($ho, 'mkdir -p /var/core');
+target_editfile_root($ho, '/etc/sysctl.conf',
+   sub { target_editfile_kvp_replace(
+ "kernel.core_pattern",
+ # %p==pid,%e==executable name,%t==timestamp
+ "/var/core/%t.%p.%e.core") });
+target_cmd_root($ho, "sysctl --load /etc/sysctl.conf");
+
 target_cmd_root($ho, "update-rc.d osstest-confirm-booted start 99 2 .");
 
 logm('OK: install completed');
diff --git a/ts-leak-check b/ts-leak-check
index ec40435..fdade36 100755
--- a/ts-leak-check
+++ b/ts-leak-check
@@ -157,7 +157,7 @@ sub inventory () {
 inventory_domains();
 inventory_processes();
 inventory_xenstore();
-inventory_files('/tmp /var/run /var/tmp /var/lib/xen');
+inventory_files('/tmp /var/run /var/tmp /var/lib/xen /var/core');
 }
 
 if (!eval {
diff --git a/ts-logs-capture b/ts-logs-capture
index 453b03d..45b0a38 100755
--- a/ts-logs-capture
+++ b/ts-logs-capture
@@ -136,6 +136,8 @@ sub fetch_logs_host_guests () {
 
   /home/osstest/osstest-confirm-booted.log
 
+  /var/core/*.core
+
   )];
 if (!try_fetch_logs($ho, $logs)) {
 logm("log fetching failed, trying hard host reboot...");
diff --git a/ts-xen-install b/ts-xen-install
index b3f4387..2db0b29 100755
--- a/ts-xen-install
+++ b/ts-xen-install
@@ -116,26 +116,11 @@ sub adjustconfig () {
 }
 die unless defined $trace_config_file;
 
-my $kvp_replace = sub($$) {
-   my ($key,$value) = @_;
-my $prnow;
-$prnow= sub {
-print EO "$key=$value\n" or die $!;
-$prnow= sub { };
-};
-while () {
-print EO or die $! unless m/^$key\b/;
-$prnow->() if m/^#$key/;
-}
-print EO "\n" or die $!;
-$prnow->();
-};
-
 target_editfile_root($ho, $trace_config_file,
-sub { $kvp_replace->("XENCONSOLED_TRACE", "guest") });
+   sub { target_editfile_kvp_replace("XENCONSOLED_TRACE", "guest") });
 
 target_editfile_root($ho, '/etc/libvirt/libvirtd.conf',
-sub { $kvp_replace->("log_level", "1") })
+   sub { target_editfile_kvp_replace("log_level", "1") })
if toolstack($ho)->{Name} eq "libv

Re: [Xen-devel] [PATCH] hvmloader: don't treat ROM BAR like other BARs

2015-03-24 Thread Ian Campbell
On Fri, 2015-03-20 at 15:20 +, Jan Beulich wrote:
> Its low 11 bits have different meaning.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Ian Campbell 



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


[Xen-devel] [linux-3.16 test] 36613: regressions - trouble: broken/fail/pass

2015-03-24 Thread xen . org
flight 36613 linux-3.16 real [real]
http://www.chiark.greenend.org.uk/~xensrcts/logs/36613/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-credit2  15 guest-localmigrate/x10fail REGR. vs. 34167
 test-amd64-i386-pair   17 guest-migrate/src_host/dst_host fail REGR. vs. 34167
 test-amd64-amd64-xl-winxpsp3  7 windows-install   fail REGR. vs. 34167

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-sedf  3 host-install(3)   broken pass in 36531
 test-armhf-armhf-xl   3 host-install(3)   broken pass in 36531
 test-amd64-i386-rhel6hvm-intel 3 host-install(3) broken in 36531 pass in 36613
 test-amd64-amd64-xl-pcipt-intel 3 host-install(3) broken in 36531 pass in 36613
 test-amd64-i386-rhel6hvm-amd  3 host-install(3)  broken in 36531 pass in 36613
 test-amd64-amd64-xl-qemuu-win7-amd64 3 host-install(3) broken in 36531 pass in 
36613
 test-amd64-amd64-xl-winxpsp3  3 host-install(3)  broken in 36531 pass in 36613

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-intel  9 guest-start  fail  never pass
 test-amd64-amd64-rumpuserxen-amd64 13 
rumpuserxen-demo-xenstorels/xenstorels.repeat fail never pass
 test-armhf-armhf-xl-multivcpu 10 migrate-support-checkfail  never pass
 test-armhf-armhf-libvirt 10 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  10 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-sedf-pin 10 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 10 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-midway   10 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-pvh-amd   9 guest-start  fail   never pass
 test-amd64-amd64-xl-pcipt-intel  9 guest-start fail never pass
 test-amd64-i386-libvirt  10 migrate-support-checkfail   never pass
 test-amd64-amd64-xl-multivcpu 15 guest-localmigrate/x10   fail  never pass
 test-amd64-i386-freebsd10-i386  7 freebsd-install  fail never pass
 test-amd64-amd64-xl-sedf 15 guest-localmigrate/x10   fail   never pass
 test-amd64-i386-freebsd10-amd64  7 freebsd-install fail never pass
 test-amd64-i386-xl-qemut-winxpsp3 14 guest-stopfail never pass
 test-amd64-i386-xl-qemut-win7-amd64 14 guest-stop  fail never pass
 test-amd64-amd64-xl-qemut-winxpsp3 14 guest-stop   fail never pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 14 guest-stop fail never pass
 test-amd64-amd64-xl-win7-amd64 14 guest-stop   fail never pass
 test-amd64-i386-xl-win7-amd64 14 guest-stop   fail  never pass
 test-amd64-i386-xl-qemuu-winxpsp3 14 guest-stopfail never pass
 test-amd64-amd64-xl-qemuu-win7-amd64 14 guest-stop fail never pass
 test-amd64-i386-xl-qemuu-win7-amd64 14 guest-stop  fail never pass
 test-amd64-amd64-xl-sedf-pin 15 guest-localmigrate/x10   fail   never pass
 test-amd64-amd64-xl-qemut-win7-amd64 14 guest-stop fail never pass
 test-amd64-i386-xl-winxpsp3  14 guest-stop   fail   never pass
 test-amd64-i386-xl-winxpsp3-vcpus1 14 guest-stop   fail never pass
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 14 guest-stop fail never pass
 test-amd64-amd64-xl-qemuu-winxpsp3  7 windows-install  fail never pass
 test-armhf-armhf-xl-sedf 10 migrate-support-check fail in 36531 never pass
 test-armhf-armhf-xl  10 migrate-support-check fail in 36531 never pass

version targeted for testing:
 linux4923505b93e073f70380557cb360997e5b84b4f9
baseline version:
 linux19583ca584d6f574384e17fe7613dfaeadcdc4a6


1129 people touched revisions under test,
not listing them all


jobs:
 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
 build-amd64-rumpuserxen  pass
 build-i386-rumpuserxen   pass
 test-amd64-amd64-xl  pass
 test-armhf-armhf-xl  broken  
 test-amd64-i3

Re: [Xen-devel] [PATCH v19 11/14] x86/VPMU: Handle PMU interrupts for PV guests

2015-03-24 Thread Jan Beulich
>>> On 17.03.15 at 15:54,  wrote:
> Changes in v19:
> * Adjusted for new ops interfaces (passing vcpu vs. vpmu)
> * Test for domain->max_cpu in choose_hwdom_vcpu() instead of 
> 'domain->vcpu!=NULL'

I suppose that's something that then should also be done in patch 7?

> --- a/xen/arch/x86/hvm/vpmu.c
> +++ b/xen/arch/x86/hvm/vpmu.c
> @@ -87,31 +87,57 @@ static void __init parse_vpmu_param(char *s)
>  void vpmu_lvtpc_update(uint32_t val)
>  {
>  struct vpmu_struct *vpmu;
> +struct vcpu *curr;
>  
>  if ( vpmu_mode == XENPMU_MODE_OFF )
>  return;
>  
> -vpmu = vcpu_vpmu(current);
> +curr = current;

Please make this the initializer of the variable, to be consistent with
other code (including yours a few lines down).

> +vpmu = vcpu_vpmu(curr);
>  
>  vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | (val & APIC_LVT_MASKED);
> -apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
> +
> +/* Postpone APIC updates for PV(H) guests if PMU interrupt is pending */
> +if ( is_hvm_vcpu(curr) || !vpmu->xenpmu_data ||

Here and below you use is_hvm_vcpu() - is the title wrongly saying
just PV when PV/PVH is meant?

> @@ -548,6 +719,24 @@ long do_xenpmu_op(unsigned int op, 
> XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
>  pvpmu_finish(current->domain, &pmu_params);
>  break;
>  
> +case XENPMU_lvtpc_set:
> +curr = current;
> +xenpmu_data = curr->arch.vpmu.xenpmu_data;
> +if ( xenpmu_data == NULL )
> +return -EINVAL;
> +vpmu_lvtpc_update(xenpmu_data->pmu.l.lapic_lvtpc);
> +break;

You don't use curr more than once here, hence I don't see the point
of latching current into a local variable.

These aside,
Acked-by: Jan Beulich 

Jan


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


Re: [Xen-devel] [PATCH v3 1/5] libxl: introduce libxl__device_model_xs_path

2015-03-24 Thread Ian Campbell
On Fri, 2015-03-20 at 16:19 +, Wei Liu wrote:
> +_hidden char *libxl__device_model_xs_path(libxl__gc *gc, uint32_t dm_domid,
> +  uint32_t domid,
> +  const char *format,  ...);

This should have the PRINTF_ATTRIBUTE thing and not the double space
before "...".

With that fixed: Acked-by: Ian Campbell 

Since this is pretty minor if you want to resend just this one patch, or
even send an incremental fixup to be folded in I can take care of it on
commit.

Ian.



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


Re: [Xen-devel] [PATCH 2/3] libxl: automatically set soft affinity after vnuma info

2015-03-24 Thread Wei Liu
On Tue, Mar 24, 2015 at 02:42:15PM +0100, Dario Faggioli wrote:
> More specifically, vcpus are assigned to a vnode, which in
> turn is associated with a pnode. If a vcpu does not have any
> soft affinity, automatically build up one, matching the pcpus
> of the said pnode.
> 

What about hard affinity? Do we need to generate hard affinity map as
well? What's your thought on this?

The code itself looks good to me.

Wei.

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


[Xen-devel] [PATCH v2] tools/mkrpm: improve version.release handling

2015-03-24 Thread Olaf Hering
An increasing version and/or release number helps to update existing
packages without --force as in "rpm Uvh --force xen.rpm". Instead its
possible to do "rpm -Fvh *.rpm" to update only already installed
packages.

The usage of --force disables essentials checks such as file conflict
detection. As a result the new xen.rpm may overwrite files owned by
other packages.

With the current way of calculating version-release it is difficult to
get an increasing release number into the spec file. The release is
always zero unless "make make XEN_VENDORVERSION=`date +.%s`" is used,
which has the bad side effect that xen.gz always gets a different
filename every time.

Update mkrpm to recognize PKG_RELEASE=. Its value will be appended to
the Release string. It can be filled with a time stamp, like:
 make rpmball PKG_RELEASE="`date +%Y%m%d%H%M%S`"

Signed-off-by: Olaf Hering 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Stefano Stabellini 
Cc: Wei Liu 
Cc: George Dunlap 
---
 INSTALL  | 4 +++-
 tools/misc/mkrpm | 4 +---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/INSTALL b/INSTALL
index 1b67c36..8735bf5 100644
--- a/INSTALL
+++ b/INSTALL
@@ -197,8 +197,10 @@ BOOT_DIR=
 EFI_DIR=
 
 The make target 'rpmball' will build a xen.rpm. This variable can be
-used to append a custom string to the name.
+used to append a custom string to the name. In addition a string can be
+appended to the rpm Release: tag.
 PKG_SUFFIX=
+PKG_RELEASE=
 
 The hypervisor will report a certain version string. This variable can
 be used to append a custom string to the version.
diff --git a/tools/misc/mkrpm b/tools/misc/mkrpm
index 9b8c6d9..f9363a1 100644
--- a/tools/misc/mkrpm
+++ b/tools/misc/mkrpm
@@ -17,9 +17,7 @@ xenroot="$1"
 # version and release.  Default to "0" if there isn't a release.
 v=(${2/-/ })
 version=${v[0]}
-release=${v[1]}
-
-[[ -n "$release" ]] || release="0"
+release="${v[1]:-0}${PKG_RELEASE:+.$PKG_RELEASE}"
 
 cd $xenroot
 

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


Re: [Xen-devel] [PATCH 1/3] libxl: check whether vcpu affinity and vnuma info match

2015-03-24 Thread Wei Liu
On Tue, Mar 24, 2015 at 02:41:48PM +0100, Dario Faggioli wrote:
> More specifically, vcpus are assigned to a vnode, which in
> turn is associated with a pnode. If a vcpu also has, in its
> (hard or soft) affinity, some pcpus that are not part of the
> said pnode, print a warning to the user.
> 

Currently all the checks that returns error are all guest visible
mis-configurations. I'm trying to reason whether we should return an
error or just print a warning.

What is the outcome if you have conflicting setting in vNUMA and vcpu
affinity? I guess it's just performance penalty but nothing guest
visible could happen?

Wei.

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


Re: [Xen-devel] [PATCH 3/3] libxl: cleanup some misuse of 'cpumap' as parameter

2015-03-24 Thread Wei Liu
On Tue, Mar 24, 2015 at 02:42:27PM +0100, Dario Faggioli wrote:
> in favour of the more generic 'bitmap', which is better
> since these are generic libxl_bitmap_* functions.
> 
> Also fix a typo, and remove a stale (and wrong) comment.
> 
> No functional change intended.
> 
> Signed-off-by: Dario Faggioli 
> Cc: Ian Campbell 
> Cc: Ian Jackson 
> Cc: Stefano Stabellini 
> Cc: Wei Liu 

Acked-by: Wei Liu 

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


  1   2   3   >