Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Juergen Gross]

On 14/04/16 21:44, Luis R. Rodriguez wrote:
> On Thu, Apr 14, 2016 at 10:53:47AM +0100, George Dunlap wrote:
>> On 13/04/16 20:52, Luis R. Rodriguez wrote:
>>> On Wed, Apr 13, 2016 at 04:44:54PM +0100, George Dunlap wrote:
 On Thu, Apr 7, 2016 at 7:51 PM, Luis R. Rodriguez  wrote:
> So more to it, if the EFI entry already provides a way into Linux
> in a more streamlined fashion bringing it closer to the bare metal
> boot entry, why *would* we add another boot entry to x86, even if
> its small and self contained ?

 We would avoid using EFI if:
>>>
>>> And this is what I was looking for, thanks!
>>>
 * Being called both on real hardware and under Xen would make the EFI
 entry point more complicated
>>>
>>> That's on the EFI Linux maintainer to assess. And he seems willing to
>>> consider this.
>>>
 * Adding the necessary EFI support into Xen would be a significant
 chunk of extra work
>>>
>>> This seems to be a good sticking point, but Andi noted another aspect
>>> of this or redundancy as well.
>>>
 * Requiring PVH mode to implement EFI would make it more difficult for
 other kernes (NetBSD, FreeBSD) to act as dom0s.
>>>
>>> What if this is an option only then ?
>>>

 * Requiring PVH mode to use EFI would make it more difficult to
 support unikernel-style workloads for domUs.
>>>
>>> What if this is an option only then ?
>>
>> So first of all, you asked why anyone would oppose EFI, and this is part
>> of the answer to that.
>>
>> Secondly, you mean "What if this is the only thing the Linux maintainers
>> will accept?"  And you already know the answer to that.
> 
> No, I meant to ask, would it be possible to make booting HVMLite using EFI
> be optional ? That way if you already support EFI that can be used on
> your entires with some small modifications.

So you suggest to add two HVMlite modes regarding boot interface
instead of one?

I still have the impression you are suggesting by using the same entry
everything is solved in the OS. You still need the support of HVMlite
especially in the early boot path to make sure the OS won't try to use
the complete EFI standard.

> 
>> How much of a burden it would be on the rest of the open-source
>> ecosystem (Xen, *BSDs, ) is a combination of some as-yet unknown facts
>> (i.e., what a minimal Xen/Linux EFI interface would look like) and a
>> matter of judgement (i.e., given the same interface, reasonable people
>> may come to different conclusions about whether the interface is an
>> undue burden to impose on others or not).
>>
>> But I would hope that the Linux maintainers would at least consider the
>> broader community when weighing their decisions, and not take advantage
>> of their position of dominance to simply ignore the effect of their
>> choices on everybody else.
> 
> This has nothing to do with dominance or anything nefarious, I'm asking
> simply for a full engineering evaluation of all possibilities, with
> the long term in mind. Not for now, but for hardware assumptions which
> are sensible 5 years from now.

No, they are not.

Given how long the EFI standard is available now and how buggy many
vendor's implementations are I don't expect all computers sold in 5
years will have a usable EFI. This will be true especially for
consumer devices where no EFI is available today.


Juergen

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

Re: [Xen-devel] Question about the XEN platform pci

[karim.allah.ah...@gmail.com]

On Wed, Apr 13, 2016 at 11:55 AM, Konrad Rzeszutek Wilk
 wrote:
> On Wed, Apr 13, 2016 at 09:00:18AM +0200, karim.allah.ah...@gmail.com wrote:
>> On Tue, Apr 12, 2016 at 5:45 PM, Konrad Rzeszutek Wilk
>>  wrote:
>> > On Tue, Apr 12, 2016 at 05:33:47PM +0200, karim.allah.ah...@gmail.com 
>> > wrote:
>> >> The INTx interrupt of this platform device can be used by Xen in HVM case 
>> >> to
>> >> notify the guest of pending events in the event channel. However that's 
>> >> usually
>> >> not used in favor of vector callbacks support in Xen where a vector is 
>> >> injected
>> >> directly to the vCPU bypassing LAPIC.
>> >>
>> >> (that said, the platform-pci driver in linux is actually broken when 
>> >> vector
>> >> callbacks are not used anyway)
>> >
>> > Oh? Is there an report/bug somewhere?
>> >
>>
>> I'm not sure if it's reported as a bug somewhere or not.
>>
>> I've always assumed that INTx is deperecated and vector callbacks are the one
>> that's supported that's why I never tried to fix it, in addition I never 
>> tried
>> to reproduce it I just looked at the code and it seemed a little bit off as
>> explained below.
>>
>> Mainly xenbus_init is called during postcore_initcall which will eventually 
>> try
>> to read a value from XenStore and will get stuck on read_reply at xenbus
>> forever since the platform driver is not probed yet and its INTx interrupt
>> handler is not registered yet which basically means that the guest can not be
>> notified at this moment of any pending event channels and none of the 
>> per-event
>> handlers will ever be invoked (including the XenStore one) and the reply will
>> never be picked up by the kernel.
>
> Argh, well that is not good. Also - thanks for reporting this.
>
> No simple ideas come to my mind on how this can be fixed - unless we
> move the xenbus_init and its driver past the platform-pci init?
>
> Or if platform-pci init code ends up being called earlier?
>

Let me test a few alternatives and send a patch to fix that first.

>>
>> The exact stack where things get stuck during xenbus_init:
>>
>> -xenbus_init
>>  -xs_init
>>   -xs_reset_watches
>>-xenbus_scanf
>> -xenbus_read
>>  -xs_single
>>   -xs_single
>>-xs_talkv
>>
>> > Thanks!
>> >>
>> >> I also think that the grant-table lives on this PCI device MMIO BAR (?!)
>> >
>> > The area may be usurped for grant-table as the OS won't touch that memory
>> > area (it after all belongs to the device).
>> >>
>> >> If you looked at hw/i386/xen/xen_platform.c in QEMU source , you will get 
>> >> a
>> >> general idea what this device is supposed todo (like logging to syslog 
>> >> stuff
>> >> for example).
>> >>
>> >> That said the platform device is really not fully utilized anyway in 
>> >> Linux.
>> >>
>> >> On Tue, Apr 12, 2016 at 4:09 PM, Konrad Rzeszutek Wilk
>> >>  wrote:
>> >> > On Tue, Apr 12, 2016 at 02:19:48AM +, Wu, Bob wrote:
>> >> >>
>> >> >> Really thanks for your reply.
>> >> >
>> >> > Hey!
>> >> >
>> >> > CC-ing Xen-devel back on. Please do not drop it and please don't
>> >> > top-post.
>> >> >>
>> >> >> Can you explain a little more?
>> >> >
>> >> > I am not sure what you want me to explain. Perhaps if you
>> >> > read http://xenbits.xen.org/docs/unstable/misc/hvm-emulated-unplug.html
>> >> > it may become clearer?
>> >> >
>> >> >> Is the xen platform pci driver the only purpose for telling QEMU  that 
>> >> >> don’t emulate the IDE driver?
>> >> >
>> >> > And network.
>> >> >> I think it can be done by a simple way, but don't need use this huge 
>> >> >> platform driver.
>> >> >
>> >> > ?
>> >> >>
>> >> >> I guess this is for PCI pass through in XEN HVM mode, but don't sure.
>> >> >
>> >> > No.
>> >> >>
>> >> >> Thanks,
>> >> >> Bob
>> >> >>
>> >> >>
>> >> >> -Original Message-
>> >> >> From: Konrad Rzeszutek Wilk [mailto:konrad.w...@oracle.com]
>> >> >> Sent: 2016年4月11日 22:24
>> >> >> To: Wu, Bob
>> >> >> Cc: xen-devel@lists.xen.org
>> >> >> Subject: Re: [Xen-devel] Question about the XEN platform pci
>> >> >>
>> >> >> On Fri, Apr 08, 2016 at 08:52:08AM +, Wu, Bob wrote:
>> >> >> >
>> >> >> > Sorry bother, I read the XEN source code recently, and found the XEN
>> >> >> > platform PCI code under drivers/xen/platform-pci.c, and I can't 
>> >> >> > fully under this driver's affect, can anybody explain a little for 
>> >> >> > me?
>> >> >> >
>> >> >> > Is the platform PCI driver for PV-split-PCI-driver-model such as the 
>> >> >> > pci-frontend/pci-backend? or for PCI pass-through model? Or for 
>> >> >> > other purpose?
>> >> >> > I saw the xenbus_pcifront_driver/ xenbus_xen_pcibk_driver are 
>> >> >> > registered on XENBUS, so I guess the platform-PCI-driver is not for 
>> >> >> > PV PCI driver.
>> >> >> >
>> >> >>
>> >> >> It is for the QEMU driver. To tell QEMU to stop emulating the 
>> >> >> IDE/network.
>> >> >>
>> >> >> > Really thank you for your replay.
>> >> >> >
>> >> >> > Thanks,

Re: [Xen-devel] [PATCH 3/3] xen: Document XEN_SYSCTL_CPUPOOL_OP_RMCPU anomalous EBUSY result

[Juergen Gross]

On 14/04/16 19:07, Ian Jackson wrote:
> This is my attempt at understanding the situation, from reading
> descriptions provided on list in the context of toolstack patches
> which were attempting to work around the anomaly.
> 
> The multiple `xxx' entries reflect 1. my lack of complete understanding
> 2. API defects which I think I have identified.
> 
> Signed-off-by: Ian Jackson 
> Cc: Wei Liu 
> CC: Dario Faggioli 
> CC: Juergen Gross 
> CC: George Dunlap 
> CC: Jan Beulich 
> CC: Konrad Rzeszutek Wilk 
> ---
>  xen/include/public/sysctl.h |   28 
>  1 file changed, 28 insertions(+)
> 
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 0849908..cfccf38 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -560,6 +560,34 @@ struct xen_sysctl_cpupool_op {
>  typedef struct xen_sysctl_cpupool_op xen_sysctl_cpupool_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpupool_op_t);
>  
> +/*
> + * cpupool operations may return EBUSY if the operation cannot be
> + * executed right now because of another cpupool operation which is
> + * still in progress.  In this case, EBUSY means that the failed
> + * operation had no effect.
> + *
> + * Some operations including at least RMCPU (xxx which others?) may

Only this one.

> + * also return EBUSY because a guest has temporarily pinned one of its
> + * vcpus to the pcpu in question.  It is the pious hope (xxx) of the
> + * author of this comment that this can only occur for domains which
> + * have been granted some kind of hardware privilege (eg passthrough).

Only the hardware domain is allowed to do this.

> + * In this case the operation may have been partially carried out and
> + * the pcpu is left in an anomalous state.  In this state the pcpu may
> + * be used by some not readily predictable subset of the vcpus
> + * (domains) whose vcpus are in the old cpupool.  (xxx is this true?)

Yes.

> + *
> + * This can be detected by seeing whether the pcpu can be added to a
> + * different cpupool.  (xxx this is a silly interface; the situation
> + * should be reported by a different errno value, at least.)  If the

I'm open for suggestions. :-)

> + * pcpu can't be added to a different cpupool for this reason,
> + * attempts to do so will returning (xxx what errno value?)

You might have guessed: EBUSY

> + *
> + * The anomalous situation can be recovered by adding the pcpu back to
> + * the cpupool it came from (xxx this permits a buggy or malicious
> + * guest to prevent the cpu ever being removed from its cpupool).

Only the hardware domain, which has other means to inject problems.


Juergen

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

[Xen-devel] [libvirt test] 91380: tolerable FAIL - PUSHED

[osstest service owner]

flight 91380 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/91380/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 14 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass

version targeted for testing:
 libvirt  fb6ec0ed3dd292670db523896e9dac45f6106fd4
baseline version:
 libvirt  1ee1b58fc39e512924aae9cb010c83df65b71eb2

Last test of basis91230  2016-04-13 05:27:11 Z2 days
Testing same since91380  2016-04-14 09:48:59 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Chunyan Liu 
  Cole Robinson 
  Jim Fehlig 
  Ján Tomko 
  Martin Kletzander 
  Maxim Nestratov 
  Mikhail Feoktistov 
  Nikolay Shirokovskiy 
  Pavel Hrdina 
  Peter Krempa 

jobs:
 build-amd64-xsm  pass
 build-armhf-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-libvirt-xsm pass
 test-armhf-armhf-libvirt-xsm fail
 test-amd64-i386-libvirt-xsm  pass
 test-amd64-amd64-libvirt pass
 test-armhf-armhf-libvirt fail
 test-amd64-i386-libvirt  pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-i386-libvirt-pair pass
 test-armhf-armhf-libvirt-qcow2   fail
 test-armhf-armhf-libvirt-raw fail
 test-amd64-amd64-libvirt-vhd pass



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

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

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

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


Pushing revision :

+ branch=libvirt
+ revision=fb6ec0ed3dd292670db523896e9dac45f6106fd4
+ . ./cri-lock-repos
++ . ./cri-common
+++ . ./cri-getconfig
+++ umask 002
+++ getrepos
 getconfig Repos
 perl -e '
use Osstest;
readglobalconfig();
print $c{"Repos"} or die $!;
'
+++ local repos=/home/osstest/repos
+++ '[' -z 

Re: [Xen-devel] [PATCH 1/3] libxc: Revert "do some retries in xc_cpupool_removecpu() for EBUSY case"

[Juergen Gross]

On 14/04/16 19:07, Ian Jackson wrote:
> libxc may be called from within long-running daemons such as libvirt.
> 
> In such a system this sleep would enable an uncooperative or buggy
> guest to block all toolstack operations for an extended period.
> 
> Sadly, therefore, such a retry loop is not feasible without a lot of
> engineering which is probably not appropriate.

I understand your concerns. OTOH you should consider that libvirt has
no support for cpupool operations today, so it won't run this code.

> This reverts commit 1ef6beea187bca8d11152b6c7d987b2b9450f936
> "libxc: do some retries in xc_cpupool_removecpu() for EBUSY case"
> 
> Signed-off-by: Ian Jackson 
> CC: Dario Faggioli 
> CC: Wei Liu 
> ---
>  tools/libxc/xc_cpupool.c |   20 +---
>  1 file changed, 1 insertion(+), 19 deletions(-)
> 
> diff --git a/tools/libxc/xc_cpupool.c b/tools/libxc/xc_cpupool.c
> index 261b9c9..c42273e 100644
> --- a/tools/libxc/xc_cpupool.c
> +++ b/tools/libxc/xc_cpupool.c
> @@ -20,7 +20,6 @@
>   */
>  
>  #include 
> -#include 
>  #include "xc_private.h"
>  
>  static int do_sysctl_save(xc_interface *xch, struct xen_sysctl *sysctl)
> @@ -138,34 +137,17 @@ int xc_cpupool_addcpu(xc_interface *xch,
>  return do_sysctl_save(xch, );
>  }
>  
> -/*
> - * The hypervisor might return EBUSY when trying to remove a cpu from a
> - * cpupool when a domain running in this cpupool has pinned a vcpu
> - * temporarily. Do some retries in this case, perhaps the situation
> - * cleans up.
> - */
> -#define NUM_RMCPU_BUSY_RETRIES 5
> -
>  int xc_cpupool_removecpu(xc_interface *xch,
>   uint32_t poolid,
>   int cpu)
>  {
> -unsigned retries;
> -int err;
>  DECLARE_SYSCTL;
>  
>  sysctl.cmd = XEN_SYSCTL_cpupool_op;
>  sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_RMCPU;
>  sysctl.u.cpupool_op.cpupool_id = poolid;
>  sysctl.u.cpupool_op.cpu = (cpu < 0) ? XEN_SYSCTL_CPUPOOL_PAR_ANY : cpu;
> -for ( retries = 0; retries < NUM_RMCPU_BUSY_RETRIES; retries++ ) {
> -err = do_sysctl_save(xch, );
> -if ( err < 0 && errno == EBUSY )
> -sleep(1);

I'd rather just remove this sleep() call than the whole retry logic.
EBUSY cases should be very very very very rare and last for some
msecs or usecs only.


Juergen

> -else
> -break;
> -}
> -return err;
> +return do_sysctl_save(xch, );
>  }
>  
>  int xc_cpupool_movedomain(xc_interface *xch,
> 


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

[Xen-devel] [qemu-mainline test] 91359: regressions - FAIL

[osstest service owner]

flight 91359 qemu-mainline real [real]
http://logs.test-lab.xenproject.org/osstest/logs/91359/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-libvirt  9 debian-installfail REGR. vs. 86454
 test-armhf-armhf-xl-credit2  16 guest-start.2 fail REGR. vs. 86454

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds15 guest-start/debian.repeat fail blocked in 86454
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 86454
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 86454

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-xsm  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-rtds 13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-xsm 14 guest-saverestorefail   never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  13 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  12 saverestore-support-checkfail   never pass

version targeted for testing:
 qemuu8b4aaba736e55c8ab6d71350f850a6642f0165b9
baseline version:
 qemuud1f8764099022bc1173f2413331b26d4ff609a0c

Last test of basis86454  2016-03-17 06:01:30 Z   28 days
Failing since 86547  2016-03-18 07:12:41 Z   27 days   27 attempts
Testing same since91359  2016-04-14 05:53:36 Z0 days1 attempts


People who touched revisions under test:
  Alberto Garcia 
  Alex Bennée 
  Alex Bligh 
  Alex Williamson 
  Alexey Kardashevskiy 
  Andrew Baumann 
  Andrey Smetanin 
  Anthony Perard 
  Aurelien Jarno 
  Bandan Das 
  Bastian Koppelmann 
  Benjamin Herrenschmidt 
  Bharata B Rao 
  Bill Paul 
  Bruce Rogers 
  Cao jin 
  Changlong Xie 
  Christophe Fergeau 
  Corey Minyard 
  Cornelia Huck 
  Cédric Le Goater 
  Daniel P. Berrange 
  David Gibson 
  Denis V. Lunev 
  Dr. David Alan Gilbert 
  Ed Maste 
  Edgar E. Iglesias 
  Edgar E. Iglesias 
  Eduardo Habkost 
  

[Xen-devel] [linux-4.1 test] 91350: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-libvirt  6 xen-boot   fail in 91189 pass in 91350
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 13 guest-localmigrate 
fail pass in 91189

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-cubietruck 15 guest-start/debian.repeat fail in 91189 like 
88721
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail in 91189 
like 88832
 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail in 91189 
like 88832
 test-armhf-armhf-xl-cubietruck 11 guest-start  fail like 87395
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeatfail  like 88251
 test-armhf-armhf-xl  15 guest-start/debian.repeatfail   like 88251
 test-armhf-armhf-xl-credit2  15 guest-start/debian.repeatfail   like 88639
 test-armhf-armhf-xl-xsm  11 guest-start  fail   like 88721
 test-armhf-armhf-xl-rtds 11 guest-start  fail   like 88721
 test-armhf-armhf-xl-vhd   9 debian-di-installfail   like 88721
 build-amd64-rumpuserxen   6 xen-buildfail   like 88832
 build-i386-rumpuserxen6 xen-buildfail   like 88832
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 88832
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 88832
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 88832
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 88832

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

version targeted for testing:
 linux206f91a12c5f69c9b4dfd4e0029043794a046933
baseline version:
 linux7f30737678023b5becaf0e2e012665f71b886a7d

Last test of basis88832  2016-04-05 16:02:25 Z9 days
Testing same since89248  2016-04-06 22:18:27 Z8 days6 attempts


People who touched revisions under test:
  Al Viro 
  Alex Deucher 
  Alexander Shishkin 
  Anand Moon 
  Avery Pennarun 
  Catalin Marinas 
  Charles Keepax 
  Chris Bainbridge 
  Dave Gerlach 
  David Engraf 
  Emmanuel 

Re: [Xen-devel] [sh_eth.c] Problem in dma_map_single()

[Wonseok Ko]

​Hi Konrad,​

Finally, I can use ethernet :D. It was my mistake. I thought the
SET_NETDEV_DEV() makes ndev == pdev->dev, but it's not true.

So I changed the dma configuration from:

dma_coerce_mask_and_coherent(*>dev*, DMA_BIT_MASK(32));
SET_NETDEV_DEV(ndev, >dev);


to:

dma_coerce_mask_and_coherent(*>dev*, DMA_BIT_MASK(32));
SET_NETDEV_DEV(ndev, >dev);


and then sh_eth works.

However I wanted to set dma_mask in pdev, I just thought about the original
data have a mask bit.
Do you know how do I set dma_mask in pdev?

Thank you for your help!

Thanks,
Wonseok.


Thanks,
Wonseok.

2016-04-14 22:52 GMT+09:00 Konrad Rzeszutek Wilk :

> On Wed, Apr 13, 2016 at 02:18:09PM +0900, Wonseok Ko wrote:
> > Hi Konrad,
> >
> > In dma_capable(), it checks that mask, size and address of device.
> >
> > Currently, it dosen't pass to first condition code as below:
> >
> > if (!dev->dma_mask)
> >
> > return 0;
> >
> > It shows the device doesn't have dma_mask bit. So I tried to set the
> > dma_mask bit in sh_eth.c
> >
> > My approaches:
> > 1. I tried to set a mask bit(dev->dma_mask) to use
> > dma_coerce_mask_and_coherent()
> > in sh_eth_drv_probe(), but it doesn't work.
>
> Are there any errors? What does dma_mask end up being?
> > 2. forced set dev->dma_mask without kernel api. I passed to dma_capable()
> > but driver cannot work.
>
> Aha, what did you set it to? Do you have a diff?
> >
> > sh_eth driver want to get valid DMA descriptor to set DMA descriptor
> > address for Rx and Tx.
>
> Right, which it would plug in its driver registers so the card
> can pick it up.
> > I tried to set the some bits(such as dma_mask) to get valid dma address
> > forcibly, in this configuration sh_eth cannot work.
>
> Uh, not sure I understand. Or are you repeating what you mentioned
> earlier?
> >
> > My question is if I want to get valid dma address with xen swiotlb(suchas
> > map_page, set_dma_mask and so on)?
>
> Well it all should work just fine - perhaps if you provide some
> diff's and such I can help you along?
>
> >
> >
> >
> >
> > Thanks,
> > Wonseok.
> >
> > 2016-04-13 2:10 GMT+09:00 Konrad Rzeszutek Wilk  >:
> >
> > > On Tue, Apr 12, 2016 at 04:54:55PM +0900, Wonseok Ko wrote:
> > > > Hi,
> > > >
> > > > I'm trying to enable ethernet driver in Domain0 for R-Car H2, but it
> > > > doesn't work. The root cause of the problem is that driver cannot
> satisfy
> > > > the condition of dma_capable(). I found the same problem in the
> mailing
> > >
> > > So what can be done about making it dma_capable() ?
> > >
> > > > list, but the problem is still remaining I guess. previous mail: (
> > > > http://lists.xen.org/archives/html/xen-devel/2014-10/msg03170.html)
> > > >
> > > > Does anyone give me advise?
> > > >
> > > > My environment as below:
> > > > - dom0 linux version: 4.3
> > > > - xen version: 4.6 commit(40d7a7454835c2f7c639c78f6c09e7b6f0e4a4e2)
> > > >
> > > >
> > > > Thanks,
> > > > Wonseok.
> > >
> > > > ___
> > > > Xen-devel mailing list
> > > > Xen-devel@lists.xen.org
> > > > http://lists.xen.org/xen-devel
> > >
> > >
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 2/6] ARM: xen: Register with kernel restart handler

[Guenter Roeck]

Register with kernel restart handler instead of setting arm_pm_restart
directly.

Select a high priority of 192 to ensure that default restart handlers
are replaced if Xen is running.

Acked-by: Arnd Bergmann 
Reviewed-by: Wolfram Sang 
Reviewed-by: Stefano Stabellini 
Signed-off-by: Guenter Roeck 
---
v2: Rebased to v4.6-rc3, added Reviewed/by/Acked-by tags

 arch/arm/xen/enlighten.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd7345c654..76a1d12fd27e 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -193,14 +194,22 @@ after_register_vcpu_info:
put_cpu();
 }
 
-static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
+static int xen_restart(struct notifier_block *nb, unsigned long action,
+  void *data)
 {
struct sched_shutdown r = { .reason = SHUTDOWN_reboot };
int rc;
rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, );
BUG_ON(rc);
+
+   return NOTIFY_DONE;
 }
 
+static struct notifier_block xen_restart_nb = {
+   .notifier_call = xen_restart,
+   .priority = 192,
+};
+
 static void xen_power_off(void)
 {
struct sched_shutdown r = { .reason = SHUTDOWN_poweroff };
@@ -370,7 +379,7 @@ static int __init xen_pm_init(void)
return -ENODEV;
 
pm_power_off = xen_power_off;
-   arm_pm_restart = xen_restart;
+   register_restart_handler(_restart_nb);
if (!xen_initial_domain()) {
struct timespec64 ts;
xen_read_wallclock();
-- 
2.5.0


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

Re: [Xen-devel] [PATCH v8.1 04/27] xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op

[Konrad Rzeszutek Wilk]

On Thu, Apr 14, 2016 at 10:36:46AM -0600, Jan Beulich wrote:
> >>> Konrad Rzeszutek Wilk  04/14/16 12:05 AM >>>
> > @@ -460,6 +461,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
> > u_sysctl)
> > ret = tmem_control(>u.tmem_op);
> > break;
> > 
> > +case XEN_SYSCTL_xsplice_op:
> > +ret = xsplice_op(>u.xsplice);
> > +copyback = (ret == -ENOSYS || ret == -EOPNOTSUPP) ? 0 : 1;
> 
> Why use a conditional expression here when its condition already is a boolean 
> one
> just needing negating?

B/c I thought you would want it this way.

I changed it to

466 if ( ret != -ENOSYS && ret != -EOPNOTSUPP )
467 copyback = 1;

But I don't think this what you meant by 'negating'? As in:

 copyback = !rc ?

But one of the subops returns the number of items and we definitly
want copyback=1 for that.
> 
> > +static int verify_name(const xen_xsplice_name_t *name, char *n)
> > +{
> > +if ( !name->size || name->size > XEN_XSPLICE_NAME_SIZE )
> > +return -EINVAL;
> > +
> > +if ( name->pad[0] || name->pad[1] || name->pad[2] )
> > +return -EINVAL;
> > +
> > +if ( !guest_handle_okay(name->name, name->size) )
> > +return -EINVAL;
> > +
> > +if ( __copy_from_guest(n, name->name, name->size) )
> > +return -EFAULT;
> 
> Is there a particular reason why you open code copy_from_guest() here? And
> considering that you now also read the string here, isn't the function name
> somewhat off?

Yes. In the earlier versions I only verified the name->name and
then later on made the copy. You pointed out that in effect I had
invalidated the earlier checks. Also you asked to make the 
name have a NUL padding - so I put that together - the verification
here checks the name for this. And since to verify I need to copy it in first..

I will change the name of the function to 'get_name'
> 
> > +static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
> > +{
> > +struct payload *data = NULL, *found;
> > +char n[XEN_XSPLICE_NAME_SIZE];
> > +int rc;
> > +
> > +rc = verify_payload(upload, n);
> > +if ( rc )
> > +return rc;
> > +
> > +spin_lock(_lock);
> > +
> > +found = find_payload(n);
> > +if ( IS_ERR(found) )
> > +{
> > +rc = PTR_ERR(found);
> > +goto out;
> > +}
> > +else if ( found )
> > +{
> > +rc = -EEXIST;
> > +goto out;
> > +}
> > +
> > +data = xzalloc(struct payload);
> 
> I generally advocate for not doing allocations with locks held, and I don't 
> think
> it would severely complicate the code here doing so.

I can certainly unlock and then lock again (when adding
it to the list).

That has the positive aspect that in the later patches when
we do vzalloc for the payload we can do it without holding locks.
> 
> Jan
> 

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

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Konrad Rzeszutek Wilk]

On Thu, Apr 14, 2016 at 11:12:01PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 14, 2016 at 04:38:47PM -0400, Konrad Rzeszutek Wilk wrote:
> > > This has nothing to do with dominance or anything nefarious, I'm asking
> > > simply for a full engineering evaluation of all possibilities, with
> > > the long term in mind. Not for now, but for hardware assumptions which
> > > are sensible 5 years from now.
> > 
> > There are two different things in my mind about this conversation:
> > 
> >  1). semantics of low-level code wrapped around pvops. On baremetal
> >it is easy - just look at Intel and AMD SDM.
> >And this is exactly what running in HVM or HVMLite mode will do -
> >all those low-level operations will have the same exact semantic
> >as baremetal.
> 
> Today Linux is KVM stupid for early boot code. I've pointed this out

-EPARSE?
> before, but again, there has been no reason found to need this. Perhaps
> for HVMLite we won't need this...

Are you talking about kvmtools? Which BTW are similar to how HVMLite
would expose the platform.
> 
> >There is no hope for the pv_ops to fix that.
> 
> Actually I beg to differ. See my patches and ongoing work.

I meant in terms of semantics. As in I cannot see some of
those pv-ops to have the same semantics as baremetal. For example
set_pte is simple on x86 (movq $, ).

While on Xen PV it is a potential batching hypercall with
lookup in an P2M table, then perhaps a sidelong look at
the M2P, then maybe the M2P override.

> 
> >And I am pretty sure the HVMLite in 5 years will have no
> >trouble in this as it will be running in VMX mode (HVM).
> 
> HVMLite may still use PV drivers for some things, its not super
> obvious to me that low level semantics will not be needed yet.

PV drivers are very different from low-level semantics.

And it will have to use them.

Maybe it is easier to think of this in terms of kvmtool - it
is pretty much how this would work - but instead of VirtIO
drivers you would be using the Xen PV drivers (thought one
could also use VirtIO ones if you wanted).
> 
> >  2). Boot entry.
> > 
> >The semantics on Linux are well known - they are documented in
> >Documentation/x86/boot.txt.
> > 
> >HVMLite Linux guests have to somehow provide that.
> > 
> >And how it is done seems to be tied around:
> > 
> >a) Use existing boot paths - which means making some
> >   extra stub code to call in those existing boot paths
> >   (for example Xen could bundle with an GRUB2-alike
> >code to be run when booting Linux using that boot-path).
> > 
> >   Or EFI (for a ton more code). Granted not all OSes
> >   support those, so not very OS agnostic.
> 
> What other OSes do is something to consider but if they don't
> do it because they are slacking in one domain should by no means
> be a reason to not evaluate the long term possible gains.
> Specially if we have reasons to believe more architectures will
> consider it and standardize on it.
> 
> It'd be silly not to take this a bit more seriously.

Complexity vs simplicity.
> 
> >Hard part - if the bootparams change then have to
> >   rev up the code in there. May be out of sync
> >   with Linux bootparams.
> 
> If we are going to ultimately standardize on EFI boot for new
> hardware it'd be rather silly to extend the boot params further.

Whoa there... Have you spoken to hpa,tglrx about this?

> 
> >b) Add another simpler boot entry point which has to copy
> >  "some" strings from its format in bootparams.
> > 
> > 
> >So this part of the discussion does not fall in the
> >hardware assumptions. Intel SDM or AMD mention nothing about
> >boot loaders or how to boot an OS - that is all in realms
> >of how software talks to software.
> 
> Right -- so one question to ask here is what other uses are there
> for this outside of say HVMLite. You mentioned Multiboot so far.
> 
> >  3). And there is the discussion on man-power to make this
> >happen.
> 
> Sure.
> 
> >  4). Lastly which one is simpler and involves less code so
> > that there is a less chance of bitrot.
> 
> Indeed.
> 
> You also forgot the tie-in between dead-code and semantics but

Wait, I just spoke about CPU semantics?! Which semantics
are you talking about?
> that clearly is not on your mind. But I'd say this is a good
> summary.

I put 'dead code' in the same realm as device drivers work.
And they seem to always have some issue or another.
Or maybe I getting unlucky and getting copied on those bugs.
> 
>   Luis

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

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Konrad Rzeszutek Wilk]

On Thu, Apr 14, 2016 at 10:56:19PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 14, 2016 at 03:56:53PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Thu, Apr 14, 2016 at 08:40:48PM +0200, Luis R. Rodriguez wrote:
> > > On Wed, Apr 13, 2016 at 09:01:32PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > On Thu, Apr 14, 2016 at 12:23:17AM +0200, Luis R. Rodriguez wrote:
> > > > > VGA code will be dead code for HVMlite for sure as the design doc
> > > > > says it will not run VGA, the ACPI flag will be set but the check
> > > > > for that is not yet on Linux. That means the VGA Linux code will
> > > > > be there but we have no way to ensure it will not run nor that
> > > > > anything will muck with it.
> > > > 
> > > >  The worst it will do is try to read non-existent registers.
> > > 
> > > Really ?
> > > 
> > > Is that your position on all other possible dead code that may have been
> > > possible on old Xen PV guests as well ?
> > 
> > This is not just with Xen - it with other device drivers that are being
> > invoked on baremetal and are not present in hardware anymore.
> 
> Indeed, however virtualization makes this issue much more prominent.

I suppose - as it only exposes a certain type of platform and nothing
else.
> 
> > > As I hinted, after thinking about this for a while I realized that dead 
> > > code is
> > > likely present on bare metal as well even without virtualization, 
> > > specially if
> > 
> > Yes!
> > > you build large single kernels to support a wide array of features which 
> > > only
> > > late at run time can be determined. Virtualization and the pvops design 
> > > just
> > > makes this issue much more prominent. If there are other areas of code 
> > > exposed
> > > that actually may run, but we are not sure may run, I figured some other 
> > > folks
> > > with a bit more security conscience minds might even simply take the 
> > > position
> > > it may be a security risk to leave that code exposed. So to take a 
> > > position
> > > that 'the worst it will do is try to read non-existent registers' -- seems
> > > rather shortsighted here.
> > 
> > Security conscious people trim their CONFIG.
> 
> Not all Linux distributions want to do this, the more binaries the
> higher the cost to test / vet.

OK, but Linux distributions have many goals - and are pulled in
different directions so they cannot always achieve the 'low footprint -
small amount of code to do inspection from security standpoint'

> 
> > > Anyway for more details on thoughts on this refer to the this wiki:
> > > 
> > > http://kernelnewbies.org/KernelProjects/kernel-sandboxing
> > > 
> > > Since this is now getting off topic please send me your feedback on 
> > > another
> > > thread for the non-virtualization aspects of this if that interests you. 
> > > My
> > > point here was rather to highlight the importance of clear semantics due 
> > > to
> > > virtualization in light of possible dead code.
> > 
> > Thank you.
> > > 
> > > > The VGA code should be able to handle failures like that and
> > > > not initialize itself when the hardware is dead (or non-existent).
> > > 
> > > That's right, its through ACPI_FADT_NO_VGA and since its part of the 
> > > HVMLite
> > > design doc we want HVMlite design to address ACPI_FADT_NO_VGA properly.  
> > > I've
> > > paved the way for this to be done cleanly and easily now, but that code 
> > > should
> > > be in place before HVMLite code gets merged.
> > > 
> > > Does domU for old Xen PV also set ACPI_FADT_NO_VGA as well ?  Should it ?
> > 
> > It does not. Not sure - it seems to have worked fine for the last ten
> > years?
> 
> Maybe HVMLite will need it enabled then too, just for bug parity.

 Sure.
> 
> > > > > To be clear -- dead code concerns still exist even without
> > > > > virtualization solutions, its just that with virtualization
> > > > > this stuff comes up more and there has been no proactive
> > > > > measures to address this. The question of semantics here is
> > > > > to see to what extent we need earlier boot code annotations
> > > > > to ensure we address semantics proactively.
> > > > 
> > > > I think what you mean by dead code is another word for
> > > > hardware test coverage?
> > > 
> > > No, no, its very different given that with virtualization the scope of 
> > > possible
> > > dead code is significant and at run time you are certain a huge portion 
> > > of code
> > > should *never ever* run. So for instance we know once we boot bare metal 
> > > none
> > > of the Xen stuff should ever run, likewise on Xen dom0 we know none of 
> > > the KVM
> > > / bare-metal only stuff should never run, when on Xen domU, none of the 
> > > Xen
> > 
> > What is this 'bare metal only stuff' you speak of? On Xen dom0 most of
> > the baremetal code is running.
> 
> A lot, not all. In the past folks added stubs (used to be paravirt_enabled()
> checks) to some code, but we are simply not sure of other possible conflicts.
> This is an known unknown if you will.
> 
> > In fact that is 

Re: [Xen-devel] [PATCH v8.1] xSplice v1 design and implementation.

[Konrad Rzeszutek Wilk]

On Thu, Apr 14, 2016 at 09:17:14AM -0600, Jan Beulich wrote:
> >>> Andrew Cooper  04/14/16 5:14 PM >>>
> >On 14/04/16 15:26, Konrad Rzeszutek Wilk wrote:
> >> @@ -312,8 +307,8 @@ struct xsplice_patch_func {
> >>  };  
> >>  
> >>  
> >> -The size of the structure is 64 bytes or 52 bytes if compiled under 32-bit
> >> -hypervisors.
> >> +The size of the structure is 64 bytes on 64-bit hypervisors. It will be
> >> +52 on 32-bit hypervisors.
> >
> >I would go further an explicitly declare this API unstable, as patches
> >must be built against an exact hypervisor source.  This also gives us
> >leaway in the future.

It will present a problem to the xsplice-tools.git which are outside
the tree. Right now they have their own header file (which mirrors this
one). But if we make it unstable and start mucking around then folks
using the tool would need to pass parameters for 'what version' is
this.

I would rather not have this happen - and now that we do not need
the #ifdef BITS_PER_LONG we can also remove the __XEN__ conditional.

Either way, this is something that can be done later.
> 
> Which it effectively is, now being (iirc) inside a __XEN__ conditional.
> 
> Jan
> 

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

Re: [Xen-devel] [PATCH for-4.7] tools/libxl: Fix legacy migration following COLO backchannel breakage

[Wen Congyang]

On 04/15/2016 03:54 AM, Andrew Cooper wrote:
> c/s f5d947bf1b "tools/libxl: add back channel support to read stream"
> made a bogus adjustment to libxl__stream_read_start(), including
> removing the comment hinting at what was going on, which breaks
> conversion of a legacy migration stream.
> 
> Symptoms look like:
> 
>   root@anonymi:~ # xl migrate domU host
>   migration target: Ready to receive domain.
>   Saving to migration stream new xl format (info 0x1/0x0/2677)
>   xc: error: error polling suspend notification channel: -1: Internal error
>   Loading new save file  (new xl fmt info 
> 0x1/0x0/2677)
>Savefile contains xl domain config in JSON format
>   Parsing config from 
>   libxl: error: libxl_stream_read.c:327:stream_header_done: Invalid ident: 
> expected 0x4c6962786c466d74, got 0x01f00f00
>   libxl: error: libxl_utils.c:430:libxl_read_exactly: file/stream truncated 
> reading ipc msg header from domain 1 save/restore helper stdout pipe
> 
> The adjustment is not required for backchannel support (as there is no
> interaction between back channels and legacy conversion), and caused
> stream->fd to be latched in the datacopier before legacy conversion
> substitutes it for the fd which is the output of the conversion script.
> 
> This causes libxl to consume data from the legacy stream rather than the
> v2 stream, and for the conversion script to encounter an error as the
> legacy stream appears to skip ahead.
> 
> Undo the adjustments to libxl__stream_read_start(), and introduce a
> better description of what is going on.  Introduce some extra assertions
> to try and catch similar breakage in the future.
> 
> Reported-by: Olaf Hering 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Wen Congyang 

> ---
> CC: Ian Jackson 
> CC: Wei Liu 
> CC: Olaf Hering 
> CC: Yang Hongyang 
> CC: Wen Congyang 
> CC: Changlong Xie 
> ---
>  tools/libxl/libxl_stream_read.c | 33 -
>  1 file changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
> index 9659051..89c2f21 100644
> --- a/tools/libxl/libxl_stream_read.c
> +++ b/tools/libxl/libxl_stream_read.c
> @@ -234,16 +234,16 @@ void libxl__stream_read_start(libxl__egc *egc,
>  stream->running = true;
>  stream->phase   = SRS_PHASE_NORMAL;
>  
> -dc->ao   = stream->ao;
> -dc->copywhat = "restore v2 stream";
> -dc->readfd = stream->fd;
> -dc->writefd  = -1;
> -
> -if (stream->back_channel)
> -return;
> -
>  if (stream->legacy) {
> -/* Convert the legacy stream. */
> +/*
> + * Convert the legacy stream.
> + *
> + * This results in a fork()/exec() of conversion helper script.  It 
> is
> + * passed the exiting stream->fd as an input, and returns the
> + * transformed stream via a new pipe.  The fd of this new pipe then
> + * replaces stream->fd, to make the rest of the stream read code
> + * agnostic to whether legacy conversion is happening or not.
> + */
>  libxl__conversion_helper_state *chs = >chs;
>  
>  chs->legacy_fd = stream->fd;
> @@ -258,10 +258,25 @@ void libxl__stream_read_start(libxl__egc *egc,
>  goto err;
>  }
>  
> +/* There should be no interaction of COLO backchannels and legacy
> + * stream conversion. */
> +assert(!stream->back_channel);
> +
> +/* Confirm *dc is still zeroed out, while we shuffle stream->fd. */
> +assert(dc->ao == NULL);
>  assert(stream->chs.v2_carefd);
>  stream->fd = libxl__carefd_fd(stream->chs.v2_carefd);
>  stream->dcs->libxc_fd = stream->fd;
>  }
> +/* stream->fd is now a v2 stream. */
> +
> +dc->ao   = stream->ao;
> +dc->copywhat = "restore v2 stream";
> +dc->readfd   = stream->fd;
> +dc->writefd  = -1;
> +
> +if (stream->back_channel)
> +return;
>  
>  /* Start reading the stream header. */
>  rc = setup_read(stream, "stream header",
> 




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

Re: [Xen-devel] Regarding Outreachy project on Improving CR Dashboard

[Jesus M. Gonzalez-Barahona]

Thanks a lot. I can run it now, with the latest version of Perceval,
and at first glance seems to work. Some improvements could be done,
but it seems to work. Since we're quite close to the evaluation for the
microtask, let's stop here, except for the tests. Please, produce some,
to validate at least a bit the code...

Saludos,

Jesus.

On Thu, 2016-04-14 at 22:41 +0530, Priya wrote:
> Hello Jesus, 
> 
> I had made changes to my code to work with the latest version of
> Perceval, you can see my latest commit [1]. Let me know if come
> across issues?
> 
> I am working on the testing part now, stuck with few issues. Hoping
> to complete by tomorrow or day after. 
> 
> 
> [1]:https://github.com/priya299/Dashboard/commit/150e259c22b36b359f79
> ea711ba4e294d0b0c9ab
> 
> 
> Priya V
> Amrita University
> LinkedIn | GitHub | Bitbucket
> 
> 
> On Mon, Apr 11, 2016 at 1:23 PM, Jesus M. Gonzalez-Barahona  rgia.com> wrote:
> > On Fri, 2016-04-08 at 19:33 +0530, Priya wrote:
> > > Hello,
> > >
> > > I tried running the same command in new version of perceval.  I
> > found
> > > the following missing message id errors in
> > perceval_mbox_parse.log
> > > file. I am working on the testing part and I will be able to
> > finish
> > > it in one or two days.
> > >
> > > You can see the errors here [1]
> > >
> > > [1]:http://imgur.com/yVsIoCT
> > 
> > Hi, Priya. I'm not sure about what exactly is causing your
> > messages,
> > since I cannot reproduce them (see below). But I still suspect that
> > they may happen because in current versions of Perceval the data
> > parsed
> > from an mbox is no longer stored as first level key/data in the
> > dictionary returned by Perceval for each message, but in data for
> > key
> > "data", which is itself a dictionary.
> > 
> > In particular, in the code:
> > 
> > -
> >               for k in msg_json:
> > try:
> > if key == k['Message-ID'].strip('<>'):
> > k['property'] = key
> > -
> > 
> > probably you should be checking for k['data']['Message-ID'] instead
> > of
> > just k['Message-ID'].
> > 
> > Please, have a look at how recent versions of Perceval produce the
> > dictionaries for each message...
> > 
> > But as I said, I cannot reproduce your error. When running your
> > most
> > recent code right now (9a5abc47bbab3b06550) with the most recent
> > Perceval/master code (53efc14001c806f0452) I get:
> > 
> > 
> > (perceval)jgb@expisito:~/src/outreachy/Dashboard/dashboard$ python3
> > createjson.py --mbox advisory-board-2014-02 --output new.json
> > Traceback (most recent call last):
> >   File "createjson.py", line 96, in 
> > main()
> >   File "createjson.py", line 92, in main
> > mparser.create_json(args.mbox,args.output)
> >   File "createjson.py", line 59, in create_json
> > messages = th.message_details(mbox_files)
> >   File
> > "/home/jgb/src/outreachy/Dashboard/dashboard/jwzthreading_r.py",
> > line 338, in message_details
> > urllib.request.urlretrieve(filename, 'mbox')
> >   File "/usr/lib/python3.4/urllib/request.py", line 186, in
> > urlretrieve
> > with contextlib.closing(urlopen(url, data)) as fp:
> >   File "/usr/lib/python3.4/urllib/request.py", line 161, in urlopen
> > return opener.open(url, data, timeout)
> >   File "/usr/lib/python3.4/urllib/request.py", line 449, in open
> > req = Request(fullurl, data)
> >   File "/usr/lib/python3.4/urllib/request.py", line 267, in
> > __init__
> > self.full_url = url
> >   File "/usr/lib/python3.4/urllib/request.py", line 293, in
> > full_url
> > self._parse()
> >   File "/usr/lib/python3.4/urllib/request.py", line 322, in _parse
> > raise ValueError("unknown url type: %r" % self.full_url)
> > ValueError: unknown url type: 'advisory-board-2014-02'
> > -
> > 
> > Could you please try to checkout and install exactly the same
> > version
> > of Perceval I'm using, and see if you get the same error? And if
> > the
> > above problem with the format returned by Perceval persists, maybe
> > you
> > can fix that too.
> > 
> > Saludos,
> > 
> >         Jesus.
> > 
> > --
> > Bitergia: http://bitergia.com
> > /me at Twitter: https://twitter.com/jgbarah
> > 
> > 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
-- 
Bitergia: http://bitergia.com
/me at Twitter: https://twitter.com/jgbarah


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

Re: [Xen-devel] [libvirt] [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Jim Fehlig]

Martin Kletzander wrote:
> On Wed, Apr 13, 2016 at 05:37:05PM -0600, Jim Fehlig wrote:
>> To ensure the libvirt libxl driver will build with future versions
>> of Xen where the libxl API may change in incompatible ways,
>> explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
>> does use new libxl APIs that have been added since Xen 4.2, but
>> currently it does not make use of any changes made to existing
>> APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
>> The version can be bumped if/when the libxl driver consumes the
>> changed APIs.
>>
>> Further details can be found in the following discussion thread
>>
>> https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
>> Signed-off-by: Jim Fehlig 
>> ---
>> src/Makefile.am  |  1 +
>> src/libxl/libxl_conf.h   | 12 
>> src/libxl/libxl_domain.c | 15 ---
>> 3 files changed, 1 insertion(+), 27 deletions(-)
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 08ff301..259a474 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1311,6 +1311,7 @@ endif ! WITH_DRIVER_MODULES
>>
>> libvirt_driver_libxl_impl_la_CFLAGS = \
>> $(LIBXL_CFLAGS)\
>> +-DLIBXL_API_VERSION=0x040200\
> 
> Adding it to LIBXL_CFLAGS in configure.ac would make it show after
> configure, but that's something probably only I would find interesting
> =)

I think that is a good idea. I've sent a V2 that moves the define from
src/Makefile.am to configure.ac

https://www.redhat.com/archives/libvir-list/2016-April/msg00938.html

> I'm no libxl expert and I don't have it on my system, so I can't try
> building it, hence only weak ACK from me.  If it build for you then it's
> fine as nobody objects as I see.

I've built libvirt with the V2 patch against Xen 4.2, 4.3, 4.4, and 4.7/master.

Regards,
Jim

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

Re: [Xen-devel] [PATCH] libxl: use LIBXL_API_VERSION 0x040200

[Jim Fehlig]

George Dunlap wrote:
> On Thu, Apr 14, 2016 at 12:37 AM, Jim Fehlig  wrote:
>> To ensure the libvirt libxl driver will build with future versions
>> of Xen where the libxl API may change in incompatible ways,
>> explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
>> does use new libxl APIs that have been added since Xen 4.2, but
>> currently it does not make use of any changes made to existing
>> APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
>> The version can be bumped if/when the libxl driver consumes the
>> changed APIs.
>>
>> Further details can be found in the following discussion thread
>>
>> https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
>> Signed-off-by: Jim Fehlig 
>> ---
>>  src/Makefile.am  |  1 +
>>  src/libxl/libxl_conf.h   | 12 
>>  src/libxl/libxl_domain.c | 15 ---
>>  3 files changed, 1 insertion(+), 27 deletions(-)
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 08ff301..259a474 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -1311,6 +1311,7 @@ endif ! WITH_DRIVER_MODULES
>>
>>  libvirt_driver_libxl_impl_la_CFLAGS =  \
>> $(LIBXL_CFLAGS) \
>> +   -DLIBXL_API_VERSION=0x040200\
>> -I$(srcdir)/access  \
>> -I$(srcdir)/conf\
>> -I$(srcdir)/secret  \
>> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
>> index 3c0eafb..24e2911 100644
>> --- a/src/libxl/libxl_conf.h
>> +++ b/src/libxl/libxl_conf.h
>> @@ -69,18 +69,6 @@
>>  # endif
>>
>>
>> -/* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
>> - * parameter has been added, representative of 'VCPU soft affinity'. If one
>> - * does not care about it (and that's libvirt case), passing NULL is the
>> - * right thing to do. To mark that change, LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
>> - * is defined. */
>> -# ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
>> -#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
>> -libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
>> -#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
>> -libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map), NULL)
>> -# endif
> 
> Just checking with this -- the LIBXL_API_VERSION is meant for
> *forward* compatibility, but I think the LIBXL_HAVE_* macros are for
> *backwards* compatibility, isn't that right?  If you compile this
> against an older version of Xen without LIBXL_HAVE_VCPU_SOFT_AFFINITY
> (say, 4.4), will it break with this patch?

I've tested the patch (well, actually a variant that moves
'-DLIBXL_API_VERSION=0x040200' to configure.ac as suggested by Martin) by
building against 4.2, 4.3, 4.4, and 4.7/master. I think Dario already did a good
job explaining the removal of the above hunk.

Regards,
Jim

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

[Xen-devel] [PATCH V2] libxl: use LIBXL_API_VERSION 0x040200

[Jim Fehlig]

To ensure the libvirt libxl driver will build with future versions
of Xen where the libxl API may change in incompatible ways,
explicitly use LIBXL_API_VERSION 0x040200. The libxl driver
does use new libxl APIs that have been added since Xen 4.2, but
currently it does not make use of any changes made to existing
APIs such as libxl_domain_create_restore or libxl_set_vcpuaffinity.
The version can be bumped if/when the libxl driver consumes the
changed APIs.

Further details can be found in the following discussion thread

https://www.redhat.com/archives/libvir-list/2016-April/msg00178.html
Signed-off-by: Jim Fehlig 
---
 configure.ac |  2 ++
 src/libxl/libxl_conf.h   | 12 
 src/libxl/libxl_domain.c | 15 ---
 3 files changed, 2 insertions(+), 27 deletions(-)

diff --git a/configure.ac b/configure.ac
index b1500f6..446f2a2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -894,6 +894,7 @@ if test "$with_libxl" != "no" ; then
 PKG_CHECK_MODULES([LIBXL], [xenlight], [
  LIBXL_FIRMWARE_DIR=`$PKG_CONFIG --variable xenfirmwaredir xenlight`
  LIBXL_EXECBIN_DIR=`$PKG_CONFIG --variable libexec_bin xenlight`
+ LIBXL_CFLAGS="$LIBXL_CFLAGS -DLIBXL_API_VERSION=0x040200"
  with_libxl=yes
 ], [LIBXL_FOUND=no])
 if test "$LIBXL_FOUND" = "no"; then
@@ -906,6 +907,7 @@ if test "$with_libxl" != "no" ; then
 LIBS="$LIBS $LIBXL_LIBS"
 AC_CHECK_LIB([xenlight], [libxl_ctx_alloc], [
 with_libxl=yes
+LIBXL_CFLAGS="$LIBXL_CFLAGS -DLIBXL_API_VERSION=0x040200"
 LIBXL_LIBS="$LIBXL_LIBS -lxenlight"
 ],[
 if test "$with_libxl" = "yes"; then
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 3c0eafb..24e2911 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -69,18 +69,6 @@
 # endif
 
 
-/* libxl interface for setting VCPU affinity changed in 4.5. In fact, a new
- * parameter has been added, representative of 'VCPU soft affinity'. If one
- * does not care about it (and that's libvirt case), passing NULL is the
- * right thing to do. To mark that change, LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
- * is defined. */
-# ifdef LIBXL_HAVE_VCPUINFO_SOFT_AFFINITY
-#  define libxl_set_vcpuaffinity(ctx, domid, vcpuid, map) \
-libxl_set_vcpuaffinity((ctx), (domid), (vcpuid), (map), NULL)
-#  define libxl_set_vcpuaffinity_all(ctx, domid, max_vcpus, map) \
-libxl_set_vcpuaffinity_all((ctx), (domid), (max_vcpus), (map), NULL)
-# endif
-
 typedef struct _libxlDriverPrivate libxlDriverPrivate;
 typedef libxlDriverPrivate *libxlDriverPrivatePtr;
 
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 86fb713..14a900c 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -1026,9 +1026,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 int managed_save_fd = -1;
 libxlDomainObjPrivatePtr priv = vm->privateData;
 libxlDriverConfigPtr cfg;
-#ifdef LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS
-libxl_domain_restore_params params;
-#endif
 virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
 libxl_asyncprogress_how aop_console_how;
 
@@ -1118,20 +1115,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
virDomainObjPtr vm,
 ret = libxl_domain_create_new(cfg->ctx, _config,
   , NULL, _console_how);
 } else {
-#if defined(LIBXL_HAVE_DOMAIN_CREATE_RESTORE_SEND_BACK_FD)
-params.checkpointed_stream = 0;
-ret = libxl_domain_create_restore(cfg->ctx, _config, ,
-  restore_fd, -1, , NULL,
-  _console_how);
-#elif defined(LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS)
-params.checkpointed_stream = 0;
-ret = libxl_domain_create_restore(cfg->ctx, _config, ,
-  restore_fd, , NULL,
-  _console_how);
-#else
 ret = libxl_domain_create_restore(cfg->ctx, _config, ,
   restore_fd, NULL, _console_how);
-#endif
 }
 virObjectLock(vm);
 
-- 
2.6.1


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

Re: [Xen-devel] [PATCH] documentation: Add disclaimer

[Paul E. McKenney]

On Wed, Jan 27, 2016 at 02:57:07PM +, David Howells wrote:
> Peter Zijlstra  wrote:
> 
> > +==
> > +DISCLAIMER
> > +==
> > +
> > +This document is not a specification; it is intentionally (for the sake of
> > +brevity) and unintentionally (due to being human) incomplete. This 
> > document is
> > +meant as a guide to using the various memory barriers provided by Linux, 
> > but
> > +in case of any doubt (and there are many) please ask.
> > +
> > +I repeat, this document is not a specification of what Linux expects from
> > +hardware.
> 
> The purpose of this document is twofold:
> 
>  (1) to specify the minimum functionality that one can rely on for any
>  particular barrier, and
> 
>  (2) to provide a guide as to how to use the barriers that are available.
> 
> Note that an architecture can provide more than the minimum requirement for
> any particular barrier, but if the barrier provides less than that, it is
> incorrect.
> 
> Note also that it is possible that a barrier may be a no-op for an
> architecture because the way that arch works renders an explicit barrier
> unnecessary in that case.
> 
> > +
> 
> Can you bung an extra blank line in here if you have to redo this at all?

Done as part of your patch.  Again, apologies for the delay.

Thanx, Paul

> > +
> > +CONTENTS
> > +
> >  
> >   (*) Abstract memory access model.
> >  
> 
> David
> 


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

Re: [Xen-devel] [PATCH] documentation: Add disclaimer

[Paul E. McKenney]

On Wed, Jan 27, 2016 at 09:35:46AM +0100, Peter Zijlstra wrote:
> On Tue, Jan 26, 2016 at 12:11:43PM -0800, Paul E. McKenney wrote:
> > So Peter, would you like to update your patch to include yourself
> > and Will as authors?
> 
> Sure, here goes.
> 
> ---
> Subject: documentation: Add disclaimer
> 
> It appears people are reading this document as a requirements list for
> building hardware. This is not the intent of this document. Nor is it
> particularly suited for this purpose.
> 
> The primary purpose of this document is our collective attempt to define
> a set of primitives that (hopefully) allow us to write correct code on
> the myriad of SMP platforms Linux supports.
> 
> Its a definite work in progress as our understanding of these platforms,
> and memory ordering in general, progresses.
> 
> Nor does being mentioned in this document mean we think its a
> particularly good idea; the data dependency barrier required by Alpha
> being a prime example. Yes we have it, no you're insane to require it
> when building new hardware.
> 
> Signed-off-by: Peter Zijlstra (Intel) 

Rather belatedly queued and pushed to -rcu, apologies for the delay.
One minor edit noted below.

Thanx, Paul

> ---
>  Documentation/memory-barriers.txt | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/memory-barriers.txt 
> b/Documentation/memory-barriers.txt
> index a61be39c7b51..98626125f484 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -4,8 +4,24 @@
> 
>  By: David Howells 
>  Paul E. McKenney 
> +Will Deacon 
> +Peter Zijlstra 
> 
> -Contents:
> +==
> +DISCLAIMER
> +==
> +
> +This document is not a specification; it is intentionally (for the sake of
> +brevity) and unintentionally (due to being human) incomplete. This document 
> is
> +meant as a guide to using the various memory barriers provided by Linux, but
> +in case of any doubt (and there are many) please ask.
> +
> +I repeat, this document is not a specification of what Linux expects from

s/I/To/ because there is more than one author.

> +hardware.
> +
> +
> +CONTENTS
> +
> 
>   (*) Abstract memory access model.
> 
> 


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

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Luis R. Rodriguez]

On Thu, Apr 14, 2016 at 04:38:47PM -0400, Konrad Rzeszutek Wilk wrote:
> > This has nothing to do with dominance or anything nefarious, I'm asking
> > simply for a full engineering evaluation of all possibilities, with
> > the long term in mind. Not for now, but for hardware assumptions which
> > are sensible 5 years from now.
> 
> There are two different things in my mind about this conversation:
> 
>  1). semantics of low-level code wrapped around pvops. On baremetal
>it is easy - just look at Intel and AMD SDM.
>And this is exactly what running in HVM or HVMLite mode will do -
>all those low-level operations will have the same exact semantic
>as baremetal.

Today Linux is KVM stupid for early boot code. I've pointed this out
before, but again, there has been no reason found to need this. Perhaps
for HVMLite we won't need this...

>There is no hope for the pv_ops to fix that.

Actually I beg to differ. See my patches and ongoing work.

>And I am pretty sure the HVMLite in 5 years will have no
>trouble in this as it will be running in VMX mode (HVM).

HVMLite may still use PV drivers for some things, its not super
obvious to me that low level semantics will not be needed yet.

>  2). Boot entry.
> 
>The semantics on Linux are well known - they are documented in
>Documentation/x86/boot.txt.
> 
>HVMLite Linux guests have to somehow provide that.
> 
>And how it is done seems to be tied around:
> 
>a) Use existing boot paths - which means making some
>   extra stub code to call in those existing boot paths
>   (for example Xen could bundle with an GRUB2-alike
>code to be run when booting Linux using that boot-path).
> 
>   Or EFI (for a ton more code). Granted not all OSes
>   support those, so not very OS agnostic.

What other OSes do is something to consider but if they don't
do it because they are slacking in one domain should by no means
be a reason to not evaluate the long term possible gains.
Specially if we have reasons to believe more architectures will
consider it and standardize on it.

It'd be silly not to take this a bit more seriously.

>Hard part - if the bootparams change then have to
>   rev up the code in there. May be out of sync
>   with Linux bootparams.

If we are going to ultimately standardize on EFI boot for new
hardware it'd be rather silly to extend the boot params further.

>b) Add another simpler boot entry point which has to copy
>  "some" strings from its format in bootparams.
> 
> 
>So this part of the discussion does not fall in the
>hardware assumptions. Intel SDM or AMD mention nothing about
>boot loaders or how to boot an OS - that is all in realms
>of how software talks to software.

Right -- so one question to ask here is what other uses are there
for this outside of say HVMLite. You mentioned Multiboot so far.

>  3). And there is the discussion on man-power to make this
>happen.

Sure.

>  4). Lastly which one is simpler and involves less code so
> that there is a less chance of bitrot.

Indeed.

You also forgot the tie-in between dead-code and semantics but
that clearly is not on your mind. But I'd say this is a good
summary.

  Luis

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

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Luis R. Rodriguez]

On Thu, Apr 14, 2016 at 03:56:53PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 14, 2016 at 08:40:48PM +0200, Luis R. Rodriguez wrote:
> > On Wed, Apr 13, 2016 at 09:01:32PM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Thu, Apr 14, 2016 at 12:23:17AM +0200, Luis R. Rodriguez wrote:
> > > > VGA code will be dead code for HVMlite for sure as the design doc
> > > > says it will not run VGA, the ACPI flag will be set but the check
> > > > for that is not yet on Linux. That means the VGA Linux code will
> > > > be there but we have no way to ensure it will not run nor that
> > > > anything will muck with it.
> > > 
> > >  The worst it will do is try to read non-existent registers.
> > 
> > Really ?
> > 
> > Is that your position on all other possible dead code that may have been
> > possible on old Xen PV guests as well ?
> 
> This is not just with Xen - it with other device drivers that are being
> invoked on baremetal and are not present in hardware anymore.

Indeed, however virtualization makes this issue much more prominent.

> > As I hinted, after thinking about this for a while I realized that dead 
> > code is
> > likely present on bare metal as well even without virtualization, specially 
> > if
> 
> Yes!
> > you build large single kernels to support a wide array of features which 
> > only
> > late at run time can be determined. Virtualization and the pvops design just
> > makes this issue much more prominent. If there are other areas of code 
> > exposed
> > that actually may run, but we are not sure may run, I figured some other 
> > folks
> > with a bit more security conscience minds might even simply take the 
> > position
> > it may be a security risk to leave that code exposed. So to take a position
> > that 'the worst it will do is try to read non-existent registers' -- seems
> > rather shortsighted here.
> 
> Security conscious people trim their CONFIG.

Not all Linux distributions want to do this, the more binaries the
higher the cost to test / vet.

> > Anyway for more details on thoughts on this refer to the this wiki:
> > 
> > http://kernelnewbies.org/KernelProjects/kernel-sandboxing
> > 
> > Since this is now getting off topic please send me your feedback on another
> > thread for the non-virtualization aspects of this if that interests you. My
> > point here was rather to highlight the importance of clear semantics due to
> > virtualization in light of possible dead code.
> 
> Thank you.
> > 
> > > The VGA code should be able to handle failures like that and
> > > not initialize itself when the hardware is dead (or non-existent).
> > 
> > That's right, its through ACPI_FADT_NO_VGA and since its part of the HVMLite
> > design doc we want HVMlite design to address ACPI_FADT_NO_VGA properly.  
> > I've
> > paved the way for this to be done cleanly and easily now, but that code 
> > should
> > be in place before HVMLite code gets merged.
> > 
> > Does domU for old Xen PV also set ACPI_FADT_NO_VGA as well ?  Should it ?
> 
> It does not. Not sure - it seems to have worked fine for the last ten
> years?

Maybe HVMLite will need it enabled then too, just for bug parity.

> > > > To be clear -- dead code concerns still exist even without
> > > > virtualization solutions, its just that with virtualization
> > > > this stuff comes up more and there has been no proactive
> > > > measures to address this. The question of semantics here is
> > > > to see to what extent we need earlier boot code annotations
> > > > to ensure we address semantics proactively.
> > > 
> > > I think what you mean by dead code is another word for
> > > hardware test coverage?
> > 
> > No, no, its very different given that with virtualization the scope of 
> > possible
> > dead code is significant and at run time you are certain a huge portion of 
> > code
> > should *never ever* run. So for instance we know once we boot bare metal 
> > none
> > of the Xen stuff should ever run, likewise on Xen dom0 we know none of the 
> > KVM
> > / bare-metal only stuff should never run, when on Xen domU, none of the Xen
> 
> What is this 'bare metal only stuff' you speak of? On Xen dom0 most of
> the baremetal code is running.

A lot, not all. In the past folks added stubs (used to be paravirt_enabled()
checks) to some code, but we are simply not sure of other possible conflicts.
This is an known unknown if you will.

> In fact that is how the device drivers work. Or are you talking about low
> level baremetal code? If so, then PVH/HVMLite does that - it skips pvops so
> that it can run this 'low-level baremetal code'

Are you telling me that HVMLite has no dead code issues ?

> > domU-only stuff should ever run.
> 
> You forgot KVM guest support on baremetal. That shouldn't run either.

Glad you bring that up, yes, that is correct. I'm being just as cautious with
Xen as with KVM on their dead-code possible issues, however their dead code
conerns should be smaller given as you not the boot path.

It doesn't mean dead-cod concerns do 

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Konrad Rzeszutek Wilk]

> This has nothing to do with dominance or anything nefarious, I'm asking
> simply for a full engineering evaluation of all possibilities, with
> the long term in mind. Not for now, but for hardware assumptions which
> are sensible 5 years from now.

There are two different things in my mind about this conversation:

 1). semantics of low-level code wrapped around pvops. On baremetal
   it is easy - just look at Intel and AMD SDM.
   And this is exactly what running in HVM or HVMLite mode will do -
   all those low-level operations will have the same exact semantic
   as baremetal.

   There is no hope for the pv_ops to fix that.

   And I am pretty sure the HVMLite in 5 years will have no
   trouble in this as it will be running in VMX mode (HVM).
   
 2). Boot entry.

   The semantics on Linux are well known - they are documented in
   Documentation/x86/boot.txt.

   HVMLite Linux guests have to somehow provide that.

   And how it is done seems to be tied around:

   a) Use existing boot paths - which means making some
  extra stub code to call in those existing boot paths
  (for example Xen could bundle with an GRUB2-alike
   code to be run when booting Linux using that boot-path).

  Or EFI (for a ton more code). Granted not all OSes
  support those, so not very OS agnostic.

   Hard part - if the bootparams change then have to
  rev up the code in there. May be out of sync
  with Linux bootparams.

   b) Add another simpler boot entry point which has to copy
 "some" strings from its format in bootparams.


   So this part of the discussion does not fall in the
   hardware assumptions. Intel SDM or AMD mention nothing about
   boot loaders or how to boot an OS - that is all in realms
   of how software talks to software.

 3). And there is the discussion on man-power to make this
   happen.

 4). Lastly which one is simpler and involves less code so
that there is a less chance of bitrot.

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

Re: [Xen-devel] [PATCH 2/3] xen: hypercall docs annotations for xen_sysctl_cpupool_op

[Dario Faggioli]

On Thu, 2016-04-14 at 18:07 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson 
> CC: Jan Beulich 
> CC: Tim Deegan 
>
Reviewed-by: Dario Faggioli 

One thing, out of curiosity. This syntax, here:

> -/* XEN_SYSCTL_cpupool_op */
> +/* ` enum XEN_SYSCTL_cpupool_op { */
>  #define XEN_SYSCTL_CPUPOOL_OP_CREATE1  /* C */
>  #define XEN_SYSCTL_CPUPOOL_OP_DESTROY   2  /* D */
>  #define XEN_SYSCTL_CPUPOOL_OP_INFO  3  /* I */
> @@ -546,9 +546,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_numainfo_t);
>  #define XEN_SYSCTL_CPUPOOL_OP_RMCPU 5  /* R */
>  #define XEN_SYSCTL_CPUPOOL_OP_MOVEDOMAIN6  /* M */
>  #define XEN_SYSCTL_CPUPOOL_OP_FREEINFO  7  /* F */
> +/* ` } */
>  #define XEN_SYSCTL_CPUPOOL_PAR_ANY 0x
>  struct xen_sysctl_cpupool_op {
> -uint32_t op;  /* IN */
> +uint32_t op;  /* IN ` enum XEN_SYSCTL_cpupool_op ` */
>
and here, is I guess useful to the hypercall HTML docs generator, as
mentioned in the cover letter?

It is not something we do in many other places (if at all, at least in
this file)... If it is, I'll happily add to my TODO list to convert
more entries to it.

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



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

Re: [Xen-devel] [PATCH 3/3] xen: Document XEN_SYSCTL_CPUPOOL_OP_RMCPU anomalous EBUSY result

[Dario Faggioli]

On Thu, 2016-04-14 at 18:56 +0100, Ian Jackson wrote:
> Andrew Cooper writes ("Re: [Xen-devel] [PATCH 3/3] xen: Document
> XEN_SYSCTL_CPUPOOL_OP_RMCPU anomalous EBUSY result"):
> > 
> > On 14/04/16 18:07, Ian Jackson wrote:
> > > 
> > > +/*
> > > + * cpupool operations may return EBUSY if the operation cannot
> > > be
> > > + * executed right now because of another cpupool operation which
> > > is
> > > + * still in progress.  In this case, EBUSY means that the failed
> > > + * operation had no effect.
> > > + *
> > > + * Some operations including at least RMCPU (xxx which others?)
> > > may
> > > + * also return EBUSY because a guest has temporarily pinned one
> > > of its
> > > + * vcpus to the pcpu in question.  It is the pious hope (xxx) of
> > > the
> > > + * author of this comment that this can only occur for domains
> > > which
> > > + * have been granted some kind of hardware privilege (eg
> > > passthrough).
> > Any VM can be given any arbitrary pinning in its xl configuration
> > file. 
> > Any arbitrary pinning can be applied at runtime via `xl vcpu-pin
> > ...`
> Does that produce EBUSY as well ?
> 
It can, after Juergen series, but I think in this case (setting
affinity), the situation is still acceptable. In fact:

> The reuse of the same error number for all of
> 
>   "the existing configuration (eg toolstack-selected vcpu pinning)
>    means that the operation does not make sense"
> 
This return -EINVAL.

>   "there is some lock contention and trying again may help"
> 
This can't happen in this case (and reason is just that setting the
affinity of a vcpu is different and less problematic than removing a
cpu from a cpupool).

>   "a semantically conflicting, or nearly-semantically-conflicting,
>    operation is currently in progress"
> 
I'm not sure what this means exactly, but I think that --depending on
what it exactly means-- it either can't happen or fall into the -EINVAL
case.

>   "the guest has done a temporary pin which prevents this operation"
> 
This (because of the series) returns -EBUSY.

> is very unfortunate.  How is a toolstack to know what to do ?
> 
Yeah, I agree, but again, I think in this case it's possible for
toolstack to tell.

From a quick check, we do not, in libxl, output any specific error
message in case we get -EBUSY... but I can send a patch to that effect
pretty quickly, if that's deemed necessary.

> > (To the best of my knowledge) A VM cannot choose pinning of its own
> > accord.  (i.e. the host admin has to choose the pinning.)
> AIUI, that is not (now) true.
> 
Yes, now a guest can call the new SCHEDOP_pin_override hypercall (and
Juergen is pushing a series to Linux for it to be able to do that... as
that was the purpose of the while thing!).

However, as said in another email, there's already a check like this in
place, in the implementation of such an hypercall:

        ret = -EPERM;
if ( !is_hardware_domain(current->domain) )
break;

which I think satisfies Ian's (legitimate) concern?

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



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

Re: [Xen-devel] [PATCH v3 1/5] xentrace: Common Support for get_pg_owner/put_pg_owner on ARM and x86

[Ben Sanda]

Jan,

>> +void put_pg_owner(struct domain *pg_owner) {
>> +rcu_unlock_domain(pg_owner);
>> +}

> I cannot see why this then can't just become an inline function.

I investigated this but making put_pg_owner() static inline creates a
circular dependency on rcu_unlock_domain(), which is also a static
inline function. The two functions are in different header files and
this creates a dependency on the one header being included by the
other, which, depending on how C files include them, creates an
implicit definition error by the compiler. For now I will leave the
function as is.

Thanks,
Ben Sanda

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

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Luis R. Rodriguez]

On Thu, Apr 14, 2016 at 10:42:15AM +0100, George Dunlap wrote:
> On 13/04/16 19:54, Luis R. Rodriguez wrote:
> > On Wed, Apr 13, 2016 at 11:05:00AM +0100, George Dunlap wrote:
> >> On Tue, Apr 12, 2016 at 11:12 PM, Luis R. Rodriguez  
> >> wrote:
> >>> Also, x86 does have a history of short DT use. Just pointing that its 
> >>> there as
> >>> an option as well. I'll Cc you on some thread about that.
> >>
> >> I'm not sure how this is relevant to anything.
> > 
> > You brought DT as a reason why ARM was able to use the native point.
> > I'm clarifying DT has nothing to do as a restriction on x86.
> 
> No, DT isn't the reason Xen is able to use the native entry point on
> ARM.  The reason is, to quote myself: "there are no assumptions made
> about what hardware is or is not present on the system -- everything
> that needs to be communicated about what is or is not present can be
> passed in DT."
> 
> So that's three things:
> 1. DT is available to be used
> 2. DT is expected as the main thing that entry point accepts
> 3. There are no assumptions about what hardware is or is not present in
> the system
> 4. Everything that needs to be communicated about what is or is not
> present can be passed in DT.
> 
> Are #2, #3, and #4 true on x86?  If not then #1 is irrelevant.

2) Obviously not, but it can be used.
3) We're getting close to that, see the platform legacy work [0],
   that should help us mesh things into a generic form that we
   didn't have before. There may be others, as is being discussed.
   If you have other ideas now would be great to hear of them.
4) we have ACPI to fill in the gaps these days for not only x86
   but also ARM, as such I think it makes sense to only use DT
   when it makes sense and to standardize on ACPI when possible

[0] http://lkml.kernel.org/r/1460592286-300-1-git-send-email-mcg...@kernel.org

> [snip from another thread]
> 
> > One. CE4100.
> >
> > arch/x86/platform/ce4100/falconfalls.dt
> 
> You CC'd me on some patches related to that.  I don't know anything
> about the code, but it looked like CE4100 is a subarch, and in response
> to that thread Ingo specifically asked you to add a comment saying
> basically "Don't add any more subarches".

Yeap!

> And not only that, but the ugly, nasty legacy PV boot path we're trying
> to get rid of IS ALSO A SUBARCH.  So instead of a quick stub with an
> extra EFI flag, you're proposing we consider add yet another Xen PV subarch?

A little while ago I brought that up as a possibility, given that the
semantics of use of the subarch were also loose... hence the discussion
over that, and now a patch that helps clarify the use as you were
Cc'd on.

What's been decided is that we should not extend the subarch, however
if we need a hypervisor type that's a separate topic and we would need
to address that separately. Its possible. I find it sensible specially if
the goal is to avoid more sporadic entries on Linux and to help with
early boot semantics / addressing dead code prospects.

EFI is another option which already has code and an entry and its
why I've asked us to consider it. So we should probably not really
try to look at adding a hypervisor type until we've really decided
that EFI is a no go at all and makes no sense.

IMHO we should add new entries to x86 linux only as a last resort measure.

> >> What we're talking about is how to get from Xen to a point in the
> >> Linux kernel where everything can Just Work.  The proposed feature is
> >> a mini trampoline that (as I understand it):
> >> 1. Tells Xen where to jump to (via ELF note)
> >> 2. Sets up some basic modes and pagetables and then jumps to the zero
> >> page so Linux can just carry on.
> > 
> > Right, and the my goal is to see to it we do enough homework to
> > ensure we reviewed all possibilities to share as much code as possible
> > already and looked at all options before saying we certainly need yet
> > another entry point. I am not convinced yet this has been done.
> 
> I think we have different ideas about what an appropriate amount of
> homework is. :-)  Everything you've put forward has been given
> consideration and judged unlikely to be promising;

That's fine I'm not afraid of suggestions to be discarded, my goal
is to evaluate all possibilities from an engineering point of
view, and then make decisions.

> and your suggestions for further possibilities (like this one) keep getting
> more and more obviously unsuitable.

Really ? If it wasn't for me looking into the paravirt crap you'd
end up likely with some other semantic mess. If you'd really like
me to stop chiming in let me know and I'll look away form Xen for
good like others have.

> We shouldn't be required to actually post code
> for every single other option just to prove how ugly they are,
> particularly when there's nothing particularly wrong with the code we have.

I'm not asking that. I'm asking for an engineering evaluation. That's very
different. I am going to the Xen 

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Konrad Rzeszutek Wilk]

On Thu, Apr 14, 2016 at 08:40:48PM +0200, Luis R. Rodriguez wrote:
> On Wed, Apr 13, 2016 at 09:01:32PM -0400, Konrad Rzeszutek Wilk wrote:
> > On Thu, Apr 14, 2016 at 12:23:17AM +0200, Luis R. Rodriguez wrote:
> > > On Wed, Apr 13, 2016 at 05:08:01PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > On Wed, Apr 13, 2016 at 10:40:55PM +0200, Luis R. Rodriguez wrote:
> > > > > On Wed, Apr 13, 2016 at 02:56:29PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > > On Wed, Apr 13, 2016 at 08:29:51PM +0200, Luis R. Rodriguez wrote:
> > > > > 
> > > > > and we also want to address dead code issues which pvops simply folded
> > > > > under the rug. The dead code concerns may exist still for hvmlite, so
> > > > > unless someone is willing to make a bold claim there is none, its
> > > > > something to consider.
> > > > 
> > > > What is this dead code you speak of?
> > > 
> > > Kasan is dead code to Xen. If you boot x86 Xen with Kasan enabled
> > 
> > For Xen PV guests,
> 
> That's right. For 5 years this will be a bomb. That went unnoticed
> and I feel I have to pull hair now to try to get folks to fix this.

Sometimes you have to roll up your sleeves and do the work yourself.
> 
> How many other issues will go in which will explode during this 5 year
> time line? How can we proactively address a solution to this now so we
> avoid this in future ?
> 
> Do you believe me now its a real issue?

I never said otherwise. What I was confused was that you grouped
this with HVMLite - which would not have a problem like this.

> 
> Fortunately I have a proactive solution for pvops now in my pipeline that
> should help avoid us having to blow more things up on Xen but also that should
> cause no headaches on behalf of x86 developers. But, reason I have been so
> engaged on HVMLite design review is I want to ensure we take the lessons
> learned from pvops and avoid this for an architecture that will be def-facto
> Xen on Linux 5 years from now.
> 
> Not bringing this up or addressing this now for HVMLite / PVH2 would simply be
> silly, and since it wasn't addressed in pvops I obviously have to ensure I
> convince enough people it was a real issue and ensure that we have enough
> semantics available to address it.

Kasan came after pvops, so of course it was not addressed in pvops.

> 
> Part of the semantics question, which has made my quest hard, was the use of
> semantics for virtualization for code on early boot and later in boot has been
> rather sloppy, so we have recently needed to address some of these gaps. Some
> of these discussions have however been productive, as I'll explain to George
> soon regarding his DT questions.  The discussion is not over though and we 
> need
> to ensure that if we need semantics for HVMLite we'll have them available in
> *clean* way. One of the things where early semantics and design to address
> these issue help in a proactive manner is to address a clean boot entry -- and
> that's also why I've been so pedantic over review of the new HVMlite boot
> entry.

I must have missed your review of the patches. Sorry!
> 
> > > Xen explodes. Quick question, will Kasan not explode with HVMLite ?
> > 
> > .. but for HVMLite of Xen HVM guest Kasan will run.
> 
> Are you sure? Should that mean that Xen HVM should be fine as well.  Does that
> work? Are we sure?

Yes, and yes.
> 
> > > MTRR used to be dead code concern but since we have vetted most of that 
> > > code
> > > now we are pretty certain that code should never run now.
> > > 
> > > KASLR may be -- not sure as I  haven't vetted that, but from
> > > what I have loosely heard maybe.
> > > 
> > > VGA code will be dead code for HVMlite for sure as the design doc
> > > says it will not run VGA, the ACPI flag will be set but the check
> > > for that is not yet on Linux. That means the VGA Linux code will
> > > be there but we have no way to ensure it will not run nor that
> > > anything will muck with it.
> > 
> >  The worst it will do is try to read non-existent registers.
> 
> Really ?
> 
> Is that your position on all other possible dead code that may have been
> possible on old Xen PV guests as well ?

This is not just with Xen - it with other device drivers that are being
invoked on baremetal and are not present in hardware anymore.
> 
> As I hinted, after thinking about this for a while I realized that dead code 
> is
> likely present on bare metal as well even without virtualization, specially if

Yes!
> you build large single kernels to support a wide array of features which only
> late at run time can be determined. Virtualization and the pvops design just
> makes this issue much more prominent. If there are other areas of code exposed
> that actually may run, but we are not sure may run, I figured some other folks
> with a bit more security conscience minds might even simply take the position
> it may be a security risk to leave that code exposed. So to take a position
> that 'the worst it will do is try to read non-existent registers' -- 

Re: [Xen-devel] [PATCH v3 2/5] xentrace: Memory/Page Mapping support for DOMID_XEN on ARM

[Ben Sanda]

Julien, George, Andrew, and Stefano,

> Thank you for the explanation.
> 
> The ARM implementation of share_xen_page_with_guest is nearly the same
> as the x86 one. However, the type is never used so far for the P2M
> code.
> 
> So far, all ARM domains have been auto-translated. DOMID_XEN is the
> first non auto-translated domain.
> 
> We could make DOMID_XEN an auto-translated domain by introducing page
> table for dummy domain. This would make the code cleaner but use more
> memory (allocation of 3 level of page tables).
> 
> Stefano, do you have any opinions on this?

How would you like to me proceed with this issue with regard to the
patch set? It sounds like this is a more far reaching architecture
design question for ARM. I have made the other changes to this version
of the code requested, should I submit v4? Or wait for a further
resolution to this discussion?

Thanks,
Ben Sanda

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

[Xen-devel] [PATCH for-4.7] tools/libxl: Fix legacy migration following COLO backchannel breakage

[Andrew Cooper]

c/s f5d947bf1b "tools/libxl: add back channel support to read stream"
made a bogus adjustment to libxl__stream_read_start(), including
removing the comment hinting at what was going on, which breaks
conversion of a legacy migration stream.

Symptoms look like:

  root@anonymi:~ # xl migrate domU host
  migration target: Ready to receive domain.
  Saving to migration stream new xl format (info 0x1/0x0/2677)
  xc: error: error polling suspend notification channel: -1: Internal error
  Loading new save file  (new xl fmt info 
0x1/0x0/2677)
   Savefile contains xl domain config in JSON format
  Parsing config from 
  libxl: error: libxl_stream_read.c:327:stream_header_done: Invalid ident: 
expected 0x4c6962786c466d74, got 0x01f00f00
  libxl: error: libxl_utils.c:430:libxl_read_exactly: file/stream truncated 
reading ipc msg header from domain 1 save/restore helper stdout pipe

The adjustment is not required for backchannel support (as there is no
interaction between back channels and legacy conversion), and caused
stream->fd to be latched in the datacopier before legacy conversion
substitutes it for the fd which is the output of the conversion script.

This causes libxl to consume data from the legacy stream rather than the
v2 stream, and for the conversion script to encounter an error as the
legacy stream appears to skip ahead.

Undo the adjustments to libxl__stream_read_start(), and introduce a
better description of what is going on.  Introduce some extra assertions
to try and catch similar breakage in the future.

Reported-by: Olaf Hering 
Signed-off-by: Andrew Cooper 
---
CC: Ian Jackson 
CC: Wei Liu 
CC: Olaf Hering 
CC: Yang Hongyang 
CC: Wen Congyang 
CC: Changlong Xie 
---
 tools/libxl/libxl_stream_read.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 9659051..89c2f21 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -234,16 +234,16 @@ void libxl__stream_read_start(libxl__egc *egc,
 stream->running = true;
 stream->phase   = SRS_PHASE_NORMAL;
 
-dc->ao   = stream->ao;
-dc->copywhat = "restore v2 stream";
-dc->readfd = stream->fd;
-dc->writefd  = -1;
-
-if (stream->back_channel)
-return;
-
 if (stream->legacy) {
-/* Convert the legacy stream. */
+/*
+ * Convert the legacy stream.
+ *
+ * This results in a fork()/exec() of conversion helper script.  It is
+ * passed the exiting stream->fd as an input, and returns the
+ * transformed stream via a new pipe.  The fd of this new pipe then
+ * replaces stream->fd, to make the rest of the stream read code
+ * agnostic to whether legacy conversion is happening or not.
+ */
 libxl__conversion_helper_state *chs = >chs;
 
 chs->legacy_fd = stream->fd;
@@ -258,10 +258,25 @@ void libxl__stream_read_start(libxl__egc *egc,
 goto err;
 }
 
+/* There should be no interaction of COLO backchannels and legacy
+ * stream conversion. */
+assert(!stream->back_channel);
+
+/* Confirm *dc is still zeroed out, while we shuffle stream->fd. */
+assert(dc->ao == NULL);
 assert(stream->chs.v2_carefd);
 stream->fd = libxl__carefd_fd(stream->chs.v2_carefd);
 stream->dcs->libxc_fd = stream->fd;
 }
+/* stream->fd is now a v2 stream. */
+
+dc->ao   = stream->ao;
+dc->copywhat = "restore v2 stream";
+dc->readfd   = stream->fd;
+dc->writefd  = -1;
+
+if (stream->back_channel)
+return;
 
 /* Start reading the stream header. */
 rc = setup_read(stream, "stream header",
-- 
2.1.4


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

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Luis R. Rodriguez]

On Thu, Apr 14, 2016 at 10:53:47AM +0100, George Dunlap wrote:
> On 13/04/16 20:52, Luis R. Rodriguez wrote:
> > On Wed, Apr 13, 2016 at 04:44:54PM +0100, George Dunlap wrote:
> >> On Thu, Apr 7, 2016 at 7:51 PM, Luis R. Rodriguez  wrote:
> >>> So more to it, if the EFI entry already provides a way into Linux
> >>> in a more streamlined fashion bringing it closer to the bare metal
> >>> boot entry, why *would* we add another boot entry to x86, even if
> >>> its small and self contained ?
> >>
> >> We would avoid using EFI if:
> > 
> > And this is what I was looking for, thanks!
> > 
> >> * Being called both on real hardware and under Xen would make the EFI
> >> entry point more complicated
> > 
> > That's on the EFI Linux maintainer to assess. And he seems willing to
> > consider this.
> > 
> >> * Adding the necessary EFI support into Xen would be a significant
> >> chunk of extra work
> > 
> > This seems to be a good sticking point, but Andi noted another aspect
> > of this or redundancy as well.
> > 
> >> * Requiring PVH mode to implement EFI would make it more difficult for
> >> other kernes (NetBSD, FreeBSD) to act as dom0s.
> > 
> > What if this is an option only then ?
> > 
> >>
> >> * Requiring PVH mode to use EFI would make it more difficult to
> >> support unikernel-style workloads for domUs.
> > 
> > What if this is an option only then ?
> 
> So first of all, you asked why anyone would oppose EFI, and this is part
> of the answer to that.
> 
> Secondly, you mean "What if this is the only thing the Linux maintainers
> will accept?"  And you already know the answer to that.

No, I meant to ask, would it be possible to make booting HVMLite using EFI
be optional ? That way if you already support EFI that can be used on
your entires with some small modifications.

> How much of a burden it would be on the rest of the open-source
> ecosystem (Xen, *BSDs, ) is a combination of some as-yet unknown facts
> (i.e., what a minimal Xen/Linux EFI interface would look like) and a
> matter of judgement (i.e., given the same interface, reasonable people
> may come to different conclusions about whether the interface is an
> undue burden to impose on others or not).
> 
> But I would hope that the Linux maintainers would at least consider the
> broader community when weighing their decisions, and not take advantage
> of their position of dominance to simply ignore the effect of their
> choices on everybody else.

This has nothing to do with dominance or anything nefarious, I'm asking
simply for a full engineering evaluation of all possibilities, with
the long term in mind. Not for now, but for hardware assumptions which
are sensible 5 years from now.

  Luis

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

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

[osstest service owner]

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

Regressions :-(

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

version targeted for testing:
 ovmf 57b6122f99bc7b7d5c334b45cbdedfa162e15512
baseline version:
 ovmf 5ac96e3a28dd26eabee421919f67fa7c443a47f1

Last test of basis65543  2015-12-08 08:45:15 Z  128 days
Failing since 65593  2015-12-08 23:44:51 Z  127 days  144 attempts
Testing same since91276  2016-04-13 16:28:07 Z1 days1 attempts


People who touched revisions under test:
  "Samer El-Haj-Mahmoud" 
  "Wu, Hao A" 
  "Yao, Jiewen" 
  Alcantara, Paulo 
  Anbazhagan Baraneedharan 
  Andrew Fish 
  Ard Biesheuvel 
  Arthur Crippa Burigo 
  Cecil Sheng 
  Chao Zhang 
  Chao Zhang
  Charles Duffy 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Daocheng Bu 
  Daryl McDaniel 
  David Woodhouse 
  Derek Lin 
  edk2 dev 
  edk2-devel 
  Eric Dong 
  Eric Dong 
  erictian
  Eugene Cohen 
  Evan Lloyd 
  Feng Tian 
  Fu Siyuan 
  Gabriel Somlo 
  Gary Ching-Pang Lin 
  Gary Lin 
  Ghazi Belaam 
  Hao Wu 
  Haojian Zhuang 
  Hess Chen 
  Heyi Guo 
  Hot Tian 
  Jaben Carsey 
  James Bottomley 
  Jeff Fan 
  Jeremy Linton 
  Jiaxin Wu 
  jiewen yao 
  Jim Dailey 
  jim_dai...@dell.com 
  jljusten
  Jordan Justen 
  Juliano Ciocari 
  Justen, Jordan L 
  Karyne Mayer 
  Ken Lu 
  Kun Gui 
  Larry Hauch 
  Laszlo Ersek 
  Leahy, Leroy P
  Leahy, Leroy P 
  Lee Leahy 
  Leekha Shaveta 
  Leendert van Doorn 
  Leif Lindholm 
  Leif Lindholm 
  Leo Duran 
  Leon Li 
  Liming Gao 
  lzeng14
  Mark 
  Mark Doran 
  Mark Rutland 
  Marvin Haeuser 
  Marvin Häuser 
  Michael D Kinney 
  Michael Kinney 
  Michael LeMay 
  Michael Thomas 
  Michał Zegan 
  Ni, Ruiyu 
  Ni, Ruiyu 
  niruiyu
  Olivier Martin 
  Paolo Bonzini 
  Paulo Alcantara 
  Paulo Alcantara Cavalcanti 
  Peter Kirmeier 
  Qin Long 
  Qing Huang 
  Qiu Shumin 
  Qiu, Shumin 
  Rodrigo Dias Correa 
  Ruiyu Ni 
  Ryan Harkin 
  Samer El-Haj-Mahmoud 
  Samer El-Haj-Mahmoud 
  Sami Mujawar 
  Shivamurthy Shastri 
  Shumin Qiu 
  Star Zeng 
  Supreeth Venkatesh 
  Tapan Shah 
  Thomas Palmer 
  Tian, Feng 
  Vladislav Vovchenko 
  Yao Jiewen 
  Yao, Jiewen 
  Ye Ting 
  Yonghong Zhu 
  Zeng, Star 
  Zhang Lubo 
  Zhang, Chao B 
  Zhang, Lubo 
  Zhangfei Gao 

jobs:
 build-amd64-xsm  pass
 

Re: [Xen-devel] REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane. [and 1 more messages]

[Konrad Rzeszutek Wilk]

On Thu, Apr 14, 2016 at 07:11:46PM +0100, Ian Jackson wrote:
> George Dunlap writes ("Re: [Xen-devel] REST MAINTAINERS feedback requested 
> Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring 
> XENVER_ but sane."):
> > On the other hand, I think there's a bit of a faulty interpretation of
> > the procedure here.  Jan reviewed the patch thoroughly and then acked
> > it; on the basis of that, Konrad legitimately checked it in.  After it
> > was checked in Jan said, "I've changed my mind and withdraw my Ack";
> > and the assumption of the subsequent conversation was that an ack
> > *can* be withdrawn after it has been legitimately checked in, and that
> > if no other Ack is supplied, then it must be reverted.
> > 
> > I don't think that's a correct interpretation of the rules.  Reviewers
> > in general, and maintainers in particular, should make reasonably sure
> > that they mean the Ack before they give it; and if they change their
> > mind after it has been legitimately checked in, then it's now up to
> > them to make the change they want to see according to the regular
> > procedure.
> 
> For the record, I agree completely with George here.  I was expecting
> that the next step would be to for Jan to post patches to revert the
> extra hypercall and replace it with something else.
> 
> Jan Beulich writes ("Re: [Xen-devel] REST MAINTAINERS feedback requested Was:
> > And btw., considering that Konrad has already posted a revert patch,
> > and I have ack-ed that one, this could now go in right away (and the
> > discussion could either be settled or start over).
> 
> I don't see that patch you describe in my inbox, but maybe I have
> missed it.

It is part of my series. This is the revert (there are two of them)

http://lists.xen.org/archives/html/xen-devel/2016-04/msg01913.html
http://lists.xen.org/archives/html/xen-devel/2016-04/msg01926.html

And then these two patches add build-id using the XENVER hypercall:

http://lists.xen.org/archives/html/xen-devel/2016-04/msg01923.html
http://lists.xen.org/archives/html/xen-devel/2016-04/msg01902.html
> 
> If that reversion is proposed, following a request for a 2nd/3rd
> opinion from me and George, and given the discussion so far, I think
> that patch ought to have been CC'd to me and George.

Argh, I probably missed you and George on them. My apologies!
> 
> I don't think it would be appropriate to commit a revert except as
> part of a series which introduces an replacement way of providing the
> needed functionality - at least, enough functionality that in practice
> a plausibly long build-id can be retrieved.
> 
> If you want the original reverted, I think it is up to you, Jan, to
> provide (or procure) such a replacement.
> 
> Thanks,
> Ian.

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

Re: [Xen-devel] run xen on hikey board

[Julien Grall]

On 14/04/2016 12:39, Safa Hamza wrote:

hello


Hello,


  to build xen on hikey board ..only should i write

make dist-xen XEN_TARGET_ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu-



there is no CONFIG_EARLY_PRINTK like other board (omap5432 , cubie ..)


Nobody adds earlyprintk support for the hikey board. Feel free to send a 
patch for it.


However, CONFIG_EARLY_PRINTK should not be necessary unless you are 
trying to debug early boot.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v4 0/3] add hypercall option to temporarily pin a vcpu

[Konrad Rzeszutek Wilk]

On Thu, Apr 14, 2016 at 05:11:05PM +0100, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] [PATCH v4 0/3] add hypercall 
> option to temporarily pin a vcpu"):
> > Applied.
> 
> Damn, I see I am too late with my review.
> 
> I will propose to revert some of this. :-/.

It is the season :-)
> 
> Ian.

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

[Xen-devel] [linux-3.18 test] 91272: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-libvirt-pair 21 guest-migrate/src_host/dst_host fail REGR. vs. 
86513
 test-amd64-i386-xl-qemut-win7-amd64 20 leak-check/check fail in 90979 REGR. 
vs. 86513
 test-amd64-amd64-xl-qemuu-win7-amd64 20 leak-check/check fail in 90979 REGR. 
vs. 86513

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-arndale   6 xen-boot   fail in 90979 pass in 91272
 test-amd64-amd64-libvirt-pair 21 guest-migrate/src_host/dst_host fail in 90979 
pass in 91272
 test-armhf-armhf-xl-vhd   9 debian-di-install  fail in 90979 pass in 91272
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat   fail pass in 90979
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop   fail pass in 90979
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop  fail pass in 90979

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

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

version targeted for testing:
 linuxb36eba9b4dd4344ed51b8f644049aeac606ccff2
baseline version:
 linuxd439e869d612dd7a338ac75a4afc3646a5e67370

Last test of basis86513  2016-03-17 21:21:40 Z   27 days
Testing same since89247  2016-04-06 22:15:59 Z7 days6 attempts


People who touched revisions under test:
  Alex Deucher 
  Avery Pennarun 
  Catalin Marinas 
  Chris Bainbridge 
  Emmanuel Grumbach 
  Felix 

Re: [Xen-devel] HVMLite / PVHv2 - using x86 EFI boot entry

[Luis R. Rodriguez]

On Wed, Apr 13, 2016 at 09:01:32PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 14, 2016 at 12:23:17AM +0200, Luis R. Rodriguez wrote:
> > On Wed, Apr 13, 2016 at 05:08:01PM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Wed, Apr 13, 2016 at 10:40:55PM +0200, Luis R. Rodriguez wrote:
> > > > On Wed, Apr 13, 2016 at 02:56:29PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > On Wed, Apr 13, 2016 at 08:29:51PM +0200, Luis R. Rodriguez wrote:
> > > > 
> > > > and we also want to address dead code issues which pvops simply folded
> > > > under the rug. The dead code concerns may exist still for hvmlite, so
> > > > unless someone is willing to make a bold claim there is none, its
> > > > something to consider.
> > > 
> > > What is this dead code you speak of?
> > 
> > Kasan is dead code to Xen. If you boot x86 Xen with Kasan enabled
> 
> For Xen PV guests,

That's right. For 5 years this will be a bomb. That went unnoticed
and I feel I have to pull hair now to try to get folks to fix this.

How many other issues will go in which will explode during this 5 year
time line? How can we proactively address a solution to this now so we
avoid this in future ?

Do you believe me now its a real issue?

Fortunately I have a proactive solution for pvops now in my pipeline that
should help avoid us having to blow more things up on Xen but also that should
cause no headaches on behalf of x86 developers. But, reason I have been so
engaged on HVMLite design review is I want to ensure we take the lessons
learned from pvops and avoid this for an architecture that will be def-facto
Xen on Linux 5 years from now.

Not bringing this up or addressing this now for HVMLite / PVH2 would simply be
silly, and since it wasn't addressed in pvops I obviously have to ensure I
convince enough people it was a real issue and ensure that we have enough
semantics available to address it.

Part of the semantics question, which has made my quest hard, was the use of
semantics for virtualization for code on early boot and later in boot has been
rather sloppy, so we have recently needed to address some of these gaps. Some
of these discussions have however been productive, as I'll explain to George
soon regarding his DT questions.  The discussion is not over though and we need
to ensure that if we need semantics for HVMLite we'll have them available in
*clean* way. One of the things where early semantics and design to address
these issue help in a proactive manner is to address a clean boot entry -- and
that's also why I've been so pedantic over review of the new HVMlite boot
entry.

> > Xen explodes. Quick question, will Kasan not explode with HVMLite ?
> 
> .. but for HVMLite of Xen HVM guest Kasan will run.

Are you sure? Should that mean that Xen HVM should be fine as well.  Does that
work? Are we sure?

> > MTRR used to be dead code concern but since we have vetted most of that code
> > now we are pretty certain that code should never run now.
> > 
> > KASLR may be -- not sure as I  haven't vetted that, but from
> > what I have loosely heard maybe.
> > 
> > VGA code will be dead code for HVMlite for sure as the design doc
> > says it will not run VGA, the ACPI flag will be set but the check
> > for that is not yet on Linux. That means the VGA Linux code will
> > be there but we have no way to ensure it will not run nor that
> > anything will muck with it.
> 
>  The worst it will do is try to read non-existent registers.

Really ?

Is that your position on all other possible dead code that may have been
possible on old Xen PV guests as well ?

As I hinted, after thinking about this for a while I realized that dead code is
likely present on bare metal as well even without virtualization, specially if
you build large single kernels to support a wide array of features which only
late at run time can be determined. Virtualization and the pvops design just
makes this issue much more prominent. If there are other areas of code exposed
that actually may run, but we are not sure may run, I figured some other folks
with a bit more security conscience minds might even simply take the position
it may be a security risk to leave that code exposed. So to take a position
that 'the worst it will do is try to read non-existent registers' -- seems
rather shortsighted here.

Anyway for more details on thoughts on this refer to the this wiki:

http://kernelnewbies.org/KernelProjects/kernel-sandboxing

Since this is now getting off topic please send me your feedback on another
thread for the non-virtualization aspects of this if that interests you. My
point here was rather to highlight the importance of clear semantics due to
virtualization in light of possible dead code.

> The VGA code should be able to handle failures like that and
> not initialize itself when the hardware is dead (or non-existent).

That's right, its through ACPI_FADT_NO_VGA and since its part of the HVMLite
design doc we want HVMlite design to address ACPI_FADT_NO_VGA properly.  I've
paved the 

Re: [Xen-devel] [osstest test] 91154: regressions - FAIL

[Ian Jackson]

osstest service owner writes ("[osstest test] 91154: regressions - FAIL"):
> flight 91154 osstest real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/91154/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-armhf-armhf-xl-credit2  16 guest-start.2   fail REGR. vs. 88006

I'm pretty sure this isn't anything to do with the changes under test
so I have force pushed this:

> version targeted for testing:
>  osstest  6d2e3b614d96d54e35262b1fa20aa9cfe215286b

Ian.

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-i386-rumpuserxen6 xen-build fail REGR. vs. 59254
 build-amd64-rumpuserxen   6 xen-build fail REGR. vs. 59254
 test-amd64-amd64-xl-credit2  17 guest-localmigrate/x10fail REGR. vs. 59254
 test-amd64-amd64-xl-multivcpu 15 guest-localmigrate   fail REGR. vs. 59254
 test-amd64-amd64-xl-xsm  15 guest-localmigratefail REGR. vs. 59254
 test-amd64-i386-xl-xsm   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-i386-xl   15 guest-localmigratefail REGR. vs. 59254
 test-amd64-amd64-pair  22 guest-migrate/dst_host/src_host fail REGR. vs. 59254
 test-amd64-i386-pair   22 guest-migrate/dst_host/src_host fail REGR. vs. 59254
 test-armhf-armhf-xl-multivcpu 16 guest-start.2fail REGR. vs. 59254
 test-amd64-amd64-xl17 guest-localmigrate/x10 fail in 91109 REGR. vs. 59254

Tests which are failing intermittently (not blocking):
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 3 host-install(3) broken in 91109 
pass in 91263
 test-amd64-amd64-xl-credit2  14 guest-saverestore  fail in 91109 pass in 91263
 test-armhf-armhf-xl-multivcpu 15 guest-start/debian.repeat fail in 91109 pass 
in 91263
 test-amd64-amd64-xl-qemut-win7-amd64 15 guest-localmigrate/x10 fail in 91109 
pass in 91263
 test-amd64-amd64-xl  15 guest-localmigrate  fail pass in 91109
 test-amd64-amd64-xl-rtds 15 guest-localmigrate  fail pass in 91109
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 13 guest-localmigrate fail pass 
in 91109

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

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

Re: [Xen-devel] [PATCH for-4.7] hotplug/Linux: fix same_vm check in block script

[Ian Jackson]

Wei Liu writes ("[PATCH for-4.7] hotplug/Linux: fix same_vm check in block 
script"):
> The original same_vm check has two bugs. When stubdom is in use because
> it relies on numeric domid to check if two domains are in fact the same
> one.  Another one is that the check would fail when two stubdoms are
> checked against each other.
> 
> The first bug is fixed by using uuid to identify a domain. The second
> bug is fixed by comparing the domains two stubdoms serve.
> 
> Signed-off-by: Wei Liu 
> ---
> Cc: Ian Jackson 
> 
> This should fix osstest stubdom local migration test. Local migration
> without stubdom is also tested and passed.

This looks plausible to me.

Acked-by: Ian Jackson 

Thanks,
Ian.

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

Re: [Xen-devel] REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane. [and 1 more messages]

[Ian Jackson]

George Dunlap writes ("Re: [Xen-devel] REST MAINTAINERS feedback requested 
Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ 
but sane."):
> On the other hand, I think there's a bit of a faulty interpretation of
> the procedure here.  Jan reviewed the patch thoroughly and then acked
> it; on the basis of that, Konrad legitimately checked it in.  After it
> was checked in Jan said, "I've changed my mind and withdraw my Ack";
> and the assumption of the subsequent conversation was that an ack
> *can* be withdrawn after it has been legitimately checked in, and that
> if no other Ack is supplied, then it must be reverted.
> 
> I don't think that's a correct interpretation of the rules.  Reviewers
> in general, and maintainers in particular, should make reasonably sure
> that they mean the Ack before they give it; and if they change their
> mind after it has been legitimately checked in, then it's now up to
> them to make the change they want to see according to the regular
> procedure.

For the record, I agree completely with George here.  I was expecting
that the next step would be to for Jan to post patches to revert the
extra hypercall and replace it with something else.

Jan Beulich writes ("Re: [Xen-devel] REST MAINTAINERS feedback requested Was:
> And btw., considering that Konrad has already posted a revert patch,
> and I have ack-ed that one, this could now go in right away (and the
> discussion could either be settled or start over).

I don't see that patch you describe in my inbox, but maybe I have
missed it.

If that reversion is proposed, following a request for a 2nd/3rd
opinion from me and George, and given the discussion so far, I think
that patch ought to have been CC'd to me and George.

I don't think it would be appropriate to commit a revert except as
part of a series which introduces an replacement way of providing the
needed functionality - at least, enough functionality that in practice
a plausibly long build-id can be retrieved.

If you want the original reverted, I think it is up to you, Jan, to
provide (or procure) such a replacement.

Thanks,
Ian.

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

Re: [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Dario Faggioli]

On Thu, 2016-04-14 at 18:59 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("Re: [Xen-devel] Fixing libvirt's libxl driver
> breakage -- where to define LIBXL_API_VERSION?"):
> > 
> > And, in those cases, usage should be gated by the appropriate
> > LIBXL_HAVE_FOOBAR symbol, which I see in the sources (e.g.,
> > for LIBXL_HAVE_NO_SUSPEND_RESUME or LIBXL_HAVE_DOMAIN_NODEAFFINITY)
> > to
> > be the case, so good again. :-)
> If libvirt specifies LIBXL_API_VERSION then it does not need to check
> any LIBXL_HAVE_*, since it will actually get an API corresponding to
> the specified API_VERSION.
> 
I concur. And in fact, this patch introduces an LIBXL_API_VERSION and
drops the use of LIBXL_HAVE_*.

And I think the patch is right. :-)

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



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

Re: [Xen-devel] Fixing libvirt's libxl driver breakage -- where to define LIBXL_API_VERSION?

[Ian Jackson]

Dario Faggioli writes ("Re: [Xen-devel] Fixing libvirt's libxl driver breakage 
-- where to define LIBXL_API_VERSION?"):
> And, in those cases, usage should be gated by the appropriate
> LIBXL_HAVE_FOOBAR symbol, which I see in the sources (e.g.,
> for LIBXL_HAVE_NO_SUSPEND_RESUME or LIBXL_HAVE_DOMAIN_NODEAFFINITY) to
> be the case, so good again. :-)

If libvirt specifies LIBXL_API_VERSION then it does not need to check
any LIBXL_HAVE_*, since it will actually get an API corresponding to
the specified API_VERSION.

Ian.

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

Re: [Xen-devel] [PATCH 3/3] xen: Document XEN_SYSCTL_CPUPOOL_OP_RMCPU anomalous EBUSY result

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] [PATCH 3/3] xen: Document 
XEN_SYSCTL_CPUPOOL_OP_RMCPU anomalous EBUSY result"):
> On 14/04/16 18:07, Ian Jackson wrote:
> > +/*
> > + * cpupool operations may return EBUSY if the operation cannot be
> > + * executed right now because of another cpupool operation which is
> > + * still in progress.  In this case, EBUSY means that the failed
> > + * operation had no effect.
> > + *
> > + * Some operations including at least RMCPU (xxx which others?) may
> > + * also return EBUSY because a guest has temporarily pinned one of its
> > + * vcpus to the pcpu in question.  It is the pious hope (xxx) of the
> > + * author of this comment that this can only occur for domains which
> > + * have been granted some kind of hardware privilege (eg passthrough).
> 
> Any VM can be given any arbitrary pinning in its xl configuration file. 
> Any arbitrary pinning can be applied at runtime via `xl vcpu-pin ...`

Does that produce EBUSY as well ?

The reuse of the same error number for all of

  "the existing configuration (eg toolstack-selected vcpu pinning)
   means that the operation does not make sense"

  "there is some lock contention and trying again may help"

  "a semantically conflicting, or nearly-semantically-conflicting,
   operation is currently in progress"

  "the guest has done a temporary pin which prevents this operation"

is very unfortunate.  How is a toolstack to know what to do ?

> (To the best of my knowledge) A VM cannot choose pinning of its own
> accord.  (i.e. the host admin has to choose the pinning.)

AIUI, that is not (now) true.

Ian.

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

Re: [Xen-devel] [for-4.7 0/2] xen/arm: traps: Correctly interpret the content of the register HPFAR_EL2

[Wei Liu]

On Wed, Apr 13, 2016 at 04:55:29PM +0100, Julien Grall wrote:
> Hello,
> 
> This small patch series is a bug fix for Xen 4.7 and should also be backported
> to Xen 4.6.
> 
> Without it, the faulting IPA reported to memaccess may be wrong.
> 
> Regards,
> 
> Cc: wei.l...@citrix.com
> 
> Julien Grall (2):
>   xen/bitops: Introduce macros to generate mask
>   xen/arm: traps: Correctly interpret the content of the register
> HPFAR_EL2

The second patch is a genuine bug fix, so 

Release-acked-by: Wei Liu 

> 
>  xen/arch/arm/traps.c| 11 +--
>  xen/include/asm-arm/processor.h |  7 +++
>  xen/include/xen/bitops.h| 11 +++
>  3 files changed, 27 insertions(+), 2 deletions(-)
> 
> -- 
> 1.9.1
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: [Xen-devel] [PATCH v2] xen: change the sizes of memory fields in the HVM start info to be 64bits

[Wei Liu]

On Wed, Apr 13, 2016 at 10:15:32PM -0600, Jan Beulich wrote:
> >>> Roger Pau Monne  04/12/16 6:12 PM >>>
> >At the moment the only consumer of this structure is x86, but other arches
> >might also use it, so make all the fields 64bits. On x86 Xen will still try
> >to place everything below the 4GiB boundary, but that might not be feasible
> >in other arches.
> >
> >Signed-off-by: Roger Pau Monné 
> >Requested-by: Jan Beulich 
> 
> Acked-by: Jan Beulich 
> 

Release-acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH for-4.7] libxl/save: set domain_suspend_state->domid in do_domain_soft_reset()

[Wei Liu]

On Mon, Apr 11, 2016 at 01:36:14PM +0100, Wei Liu wrote:
> Sorry for breaking it!
> 
> On Mon, Apr 11, 2016 at 02:20:04PM +0200, Vitaly Kuznetsov wrote:
> > c/s d5c693d "libxl/save: Refactor libxl__domain_suspend_state" broke soft
> > reset as libxl__domain_suspend_device_model() now fails when domid in not 
> > set
> > in libxl__domain_suspend_state.
> > 
> > Signed-off-by: Vitaly Kuznetsov 
> 
> Acked-by: Wei Liu 
> 

Release-acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v4 2/3] libxl: print message how to recover from xl cpupool-cpu-remove errors

[Dario Faggioli]

On Thu, 2016-04-14 at 18:22 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("Re: [Xen-devel] [PATCH v4 2/3] libxl: print
> message how to recover from xl cpupool-cpu-remove errors"):
> > 
> > Not easily (and in general not with any patch that I'd consider
> > appropriate for this phase of the release process), as it depends
> > on
> > transient situations in the hypervisor, such as lock contention on
> > scheduling data structures.
> I think it would be best to carry on this conversation in response to
> my fixup mini-series, which contains a docs patch full of `xxx' :-).
> 
Yep, sorry, I've just saw it. I'll reply better tomorrow.

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



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

Re: [Xen-devel] [RFC PATCH v6 00/28] libxl: Deprivilege qemu

[Ian Jackson]

Stefano Stabellini writes ("Re: [RFC PATCH v6 00/28] libxl: Deprivilege qemu"):
> I take that this series is going to miss 4.7 at this stage, right?

I'm afraid so.  We concluded that a crucial piece - arranging for the
necessary access controls on privcmd - was not going to be in place
for 4.7.

Without that, this series does not offer much additional security.  It
is a shame that the code will rot a bit in the meantime as a result of
it missing 4.7.

Ian.

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

Re: [Xen-devel] [PATCH 3/3] xen: Document XEN_SYSCTL_CPUPOOL_OP_RMCPU anomalous EBUSY result

[Andrew Cooper]

On 14/04/16 18:07, Ian Jackson wrote:
> This is my attempt at understanding the situation, from reading
> descriptions provided on list in the context of toolstack patches
> which were attempting to work around the anomaly.
>
> The multiple `xxx' entries reflect 1. my lack of complete understanding
> 2. API defects which I think I have identified.
>
> Signed-off-by: Ian Jackson 
> Cc: Wei Liu 
> CC: Dario Faggioli 
> CC: Juergen Gross 
> CC: George Dunlap 
> CC: Jan Beulich 
> CC: Konrad Rzeszutek Wilk 
> ---
>  xen/include/public/sysctl.h |   28 
>  1 file changed, 28 insertions(+)
>
> diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
> index 0849908..cfccf38 100644
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -560,6 +560,34 @@ struct xen_sysctl_cpupool_op {
>  typedef struct xen_sysctl_cpupool_op xen_sysctl_cpupool_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpupool_op_t);
>  
> +/*
> + * cpupool operations may return EBUSY if the operation cannot be
> + * executed right now because of another cpupool operation which is
> + * still in progress.  In this case, EBUSY means that the failed
> + * operation had no effect.
> + *
> + * Some operations including at least RMCPU (xxx which others?) may
> + * also return EBUSY because a guest has temporarily pinned one of its
> + * vcpus to the pcpu in question.  It is the pious hope (xxx) of the
> + * author of this comment that this can only occur for domains which
> + * have been granted some kind of hardware privilege (eg passthrough).

Any VM can be given any arbitrary pinning in its xl configuration file. 
Any arbitrary pinning can be applied at runtime via `xl vcpu-pin ...`

(To the best of my knowledge) A VM cannot choose pinning of its own
accord.  (i.e. the host admin has to choose the pinning.)

~Andrew

> + *
> + * In this case the operation may have been partially carried out and
> + * the pcpu is left in an anomalous state.  In this state the pcpu may
> + * be used by some not readily predictable subset of the vcpus
> + * (domains) whose vcpus are in the old cpupool.  (xxx is this true?)
> + *
> + * This can be detected by seeing whether the pcpu can be added to a
> + * different cpupool.  (xxx this is a silly interface; the situation
> + * should be reported by a different errno value, at least.)  If the
> + * pcpu can't be added to a different cpupool for this reason,
> + * attempts to do so will returning (xxx what errno value?)
> + *
> + * The anomalous situation can be recovered by adding the pcpu back to
> + * the cpupool it came from (xxx this permits a buggy or malicious
> + * guest to prevent the cpu ever being removed from its cpupool).
> + */ 
> +
>  #define ARINC653_MAX_DOMAINS_PER_SCHEDULE   64
>  /*
>   * This structure is used to pass a new ARINC653 schedule from a


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

Re: [Xen-devel] [PATCH v4 2/3] libxl: print message how to recover from xl cpupool-cpu-remove errors

[Ian Jackson]

Dario Faggioli writes ("Re: [Xen-devel] [PATCH v4 2/3] libxl: print message how 
to recover from xl cpupool-cpu-remove errors"):
> Not easily (and in general not with any patch that I'd consider
> appropriate for this phase of the release process), as it depends on
> transient situations in the hypervisor, such as lock contention on
> scheduling data structures.

I think it would be best to carry on this conversation in response to
my fixup mini-series, which contains a docs patch full of `xxx' :-).

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH v4 3/3] libxl: add force option for xl vcpu-pin

[Dario Faggioli]

On Thu, 2016-04-14 at 17:10 +0100, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH v4 3/3] libxl: add force option for xl
> vcpu-pin"):
> > 
> > 
> > +Specifying I<-f> or I<--force> will remove a temporary pinning
> > done by the
> > +operating system (normally this should be done by the operating
> > system).
> > +In case a temporary pinning is active for a vcpu the affinity of
> > this vcpu
> > +can't be changed without this option.
> This documentation needs to confirm that this can only happen to
> dom0,
> or other domains which have been granted hardware access.
> 
> Assuming that this is actually true.  If not then surely it should
> be.
> 
AFAIUI, it's like that already.

From commit 8fa0fca9f3fdaac1aead9cf61d678a0d8cce02e2:

+case SCHEDOP_pin_override:
+{
+struct sched_pin_override sched_pin_override;
+
+ret = -EPERM;
+if ( !is_hardware_domain(current->domain) )
+break;
+
+ret = -EFAULT;
+if ( copy_from_guest(_pin_override, arg, 1) )
+break;
+
+ret = vcpu_pin_override(current, sched_pin_override.pcpu);
+
+break;
+}

Better saying that in the documentation makes sense, I guess.

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



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

Re: [Xen-devel] REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[George Dunlap]

On Thu, Apr 14, 2016 at 4:59 PM, Jan Beulich  wrote:
 George Dunlap  04/14/16 5:16 PM >>>
>>On the other hand, I think there's a bit of a faulty interpretation of
>>the procedure here.  Jan reviewed the patch thoroughly and then acked
>>it; on the basis of that, Konrad legitimately checked it in.  After it
>>was checked in Jan said, "I've changed my mind and withdraw my Ack";
>>and the assumption of the subsequent conversation was that an ack
>>*can* be withdrawn after it has been legitimately checked in, and that
>>if no other Ack is supplied, then it must be reverted.
>>
>>I don't think that's a correct interpretation of the rules.  Reviewers
>>in general, and maintainers in particular, should make reasonably sure
>>that they mean the Ack before they give it; and if they change their
>>mind after it has been legitimately checked in, then it's now up to
>>them to make the change they want to see according to the regular
>>procedure.  That is, if Jan wants it reverted, he needs to post a
>>patch reverting it and get Acks from the appropriate maintainers; and
>>the discussion needs to be around Jan's reversion being accepted, not
>>about Konrad's original patch continuing to be accepted.  (Obvious
>>exceptions can be made in the case of emergencies like build
>>breakages.)
>
> Fundamentally I agree, but I think there's more to this than just following
> a set of rules. For example, please don't forget the time pressure due to
> the (at that time) rapidly approaching freeze date. And then, mistakes
> happen, and so I made a mistake here by sending the ack a few hours too
> early.

Sure, mistakes happen; but I hope it's not being to controversial to
say that in general, the procedure should be arranged such that the
person who makes the mistake is the one who has to do deal with the
most consequences from the mistake, not the people around him.  I
mean, if you asked a bartender for a bottle of beer, and after he'd
already opened it you said, "Oh sorry, I actually wanted a cider", it
would be fair enough for the bartender to ask you to pay for the beer,
rather than eating it*, wouldn't it? :-)

> What is really hard to understand to me is why it is so difficult to just
> get a refereeing opinion on the actual interface change. IMO we don't
> really need to discuss rules and processes, the question is as simple
> as "Do we want/need this new interface?"

Well Ian and I have already given our opinions -- Ian thinks moving to
a clean interface and deprecating the old one is in general a good
thing, and doesn't look too painful in this case.  I don't really see
a problem with using a large fixed size, but I don't fundamentally see
a problem with moving to a new clean interface either.  Given that
Andy has a strong aversion to the way things are, if you had only a
mild distaste rather than a  strong objection to the new hypercall, it
would probably make sense to go with the new hypercall.

If you do have a strong objection, then maybe we could try to convince
Andy to accept adding subops with different calling semantics to the
existing hypercall.  But I would still think the burden of persuasion
is primarily on you.

 -George

* In this context "eating" something means just accepting the loss of
the thing yourself.  For instance, if you bought two tickets to a
concert but couldn't convince anyone to go with you, you'd say you had
to "eat the other ticket".  Here it means the bartender throws the
beer away and accepts the loss himself.

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

Re: [Xen-devel] [PATCH v4 2/3] libxl: print message how to recover from xl cpupool-cpu-remove errors

[Dario Faggioli]

On Thu, 2016-04-14 at 17:06 +0100, Ian Jackson wrote:
> Juergen Gross writes ("[PATCH v4 2/3] libxl: print message how to
> recover from xl cpupool-cpu-remove errors"):
> > 
> > An error occurring when calling "xl cpupool-cpu-remove" might leave
> > the system in a state where a cpu is neither completely free nor in
> > a cpupool.
> Surely this is a bug.  Can it not be avoided ?
> 
Not easily (and in general not with any patch that I'd consider
appropriate for this phase of the release process), as it depends on
transient situations in the hypervisor, such as lock contention on
scheduling data structures.

> > This can easily be repaired by adding the cpu via
> > "xl cpupool-cpu-add" to the cpupool where it was removed from
> > before.
> > Print a message telling this the user in case of an error.
> ...
> > 
> > -if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, ))
> > -fprintf(stderr, "some cpus may not have been removed from
> > %s\n", pool);
> > +if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, )) {
> > +fprintf(stderr, "Some cpus may have not or only partially
> > been removed from '%s'.\n", pool);
> > +fprintf(stderr, "If a cpu can't be added to another
> > cpupool, add it to '%s' again and retry.\n", pool);
> > +}
> If it can't be avoided then I guess this will have to do but I remain
> to be convinced.
> 
And in fact, it's not something that is introduced by this series,
which is, with this patch, just taking the chance to document things
better (although, this series introduces one more way for the issue to
occur).

Doing some retries at levels lower than this would minimize the chance
of the user actually getting to deal with the problem. For eaxmple,
what's done in libxc... but as you pointed out, that introduces other
problems, so I'm not sure. :-/

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



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

Re: [Xen-devel] Regarding Outreachy project on Improving CR Dashboard

[Priya]

Hello Jesus,

I had made changes to my code to work with the latest version of Perceval,
you can see my latest commit [1]. Let me know if come across issues?

I am working on the testing part now, stuck with few issues. Hoping to
complete by tomorrow or day after.


[1]:
https://github.com/priya299/Dashboard/commit/150e259c22b36b359f79ea711ba4e294d0b0c9ab


*Priya V*
Amrita University
LinkedIn

| GitHub  | Bitbucket



On Mon, Apr 11, 2016 at 1:23 PM, Jesus M. Gonzalez-Barahona <
j...@bitergia.com> wrote:

> On Fri, 2016-04-08 at 19:33 +0530, Priya wrote:
> > Hello,
> >
> > I tried running the same command in new version of perceval.  I found
> > the following missing message id errors in perceval_mbox_parse.log
> > file. I am working on the testing part and I will be able to finish
> > it in one or two days.
> >
> > You can see the errors here [1]
> >
> > [1]:http://imgur.com/yVsIoCT
>
> Hi, Priya. I'm not sure about what exactly is causing your messages,
> since I cannot reproduce them (see below). But I still suspect that
> they may happen because in current versions of Perceval the data parsed
> from an mbox is no longer stored as first level key/data in the
> dictionary returned by Perceval for each message, but in data for key
> "data", which is itself a dictionary.
>
> In particular, in the code:
>
> -
>   for k in msg_json:
> try:
> if key == k['Message-ID'].strip('<>'):
> k['property'] = key
> -
>
> probably you should be checking for k['data']['Message-ID'] instead of
> just k['Message-ID'].
>
> Please, have a look at how recent versions of Perceval produce the
> dictionaries for each message...
>
> But as I said, I cannot reproduce your error. When running your most
> recent code right now (9a5abc47bbab3b06550) with the most recent
> Perceval/master code (53efc14001c806f0452) I get:
>
> 
> (perceval)jgb@expisito:~/src/outreachy/Dashboard/dashboard$ python3
> createjson.py --mbox advisory-board-2014-02 --output new.json
> Traceback (most recent call last):
>   File "createjson.py", line 96, in 
> main()
>   File "createjson.py", line 92, in main
> mparser.create_json(args.mbox,args.output)
>   File "createjson.py", line 59, in create_json
> messages = th.message_details(mbox_files)
>   File "/home/jgb/src/outreachy/Dashboard/dashboard/jwzthreading_r.py",
> line 338, in message_details
> urllib.request.urlretrieve(filename, 'mbox')
>   File "/usr/lib/python3.4/urllib/request.py", line 186, in urlretrieve
> with contextlib.closing(urlopen(url, data)) as fp:
>   File "/usr/lib/python3.4/urllib/request.py", line 161, in urlopen
> return opener.open(url, data, timeout)
>   File "/usr/lib/python3.4/urllib/request.py", line 449, in open
> req = Request(fullurl, data)
>   File "/usr/lib/python3.4/urllib/request.py", line 267, in __init__
> self.full_url = url
>   File "/usr/lib/python3.4/urllib/request.py", line 293, in full_url
> self._parse()
>   File "/usr/lib/python3.4/urllib/request.py", line 322, in _parse
> raise ValueError("unknown url type: %r" % self.full_url)
> ValueError: unknown url type: 'advisory-board-2014-02'
> -
>
> Could you please try to checkout and install exactly the same version
> of Perceval I'm using, and see if you get the same error? And if the
> above problem with the format returned by Perceval persists, maybe you
> can fix that too.
>
> Saludos,
>
> Jesus.
>
> --
> Bitergia: http://bitergia.com
> /me at Twitter: https://twitter.com/jgbarah
>
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 3/3] xen: Document XEN_SYSCTL_CPUPOOL_OP_RMCPU anomalous EBUSY result

[Ian Jackson]

This is my attempt at understanding the situation, from reading
descriptions provided on list in the context of toolstack patches
which were attempting to work around the anomaly.

The multiple `xxx' entries reflect 1. my lack of complete understanding
2. API defects which I think I have identified.

Signed-off-by: Ian Jackson 
Cc: Wei Liu 
CC: Dario Faggioli 
CC: Juergen Gross 
CC: George Dunlap 
CC: Jan Beulich 
CC: Konrad Rzeszutek Wilk 
---
 xen/include/public/sysctl.h |   28 
 1 file changed, 28 insertions(+)

diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 0849908..cfccf38 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -560,6 +560,34 @@ struct xen_sysctl_cpupool_op {
 typedef struct xen_sysctl_cpupool_op xen_sysctl_cpupool_op_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cpupool_op_t);
 
+/*
+ * cpupool operations may return EBUSY if the operation cannot be
+ * executed right now because of another cpupool operation which is
+ * still in progress.  In this case, EBUSY means that the failed
+ * operation had no effect.
+ *
+ * Some operations including at least RMCPU (xxx which others?) may
+ * also return EBUSY because a guest has temporarily pinned one of its
+ * vcpus to the pcpu in question.  It is the pious hope (xxx) of the
+ * author of this comment that this can only occur for domains which
+ * have been granted some kind of hardware privilege (eg passthrough).
+ *
+ * In this case the operation may have been partially carried out and
+ * the pcpu is left in an anomalous state.  In this state the pcpu may
+ * be used by some not readily predictable subset of the vcpus
+ * (domains) whose vcpus are in the old cpupool.  (xxx is this true?)
+ *
+ * This can be detected by seeing whether the pcpu can be added to a
+ * different cpupool.  (xxx this is a silly interface; the situation
+ * should be reported by a different errno value, at least.)  If the
+ * pcpu can't be added to a different cpupool for this reason,
+ * attempts to do so will returning (xxx what errno value?)
+ *
+ * The anomalous situation can be recovered by adding the pcpu back to
+ * the cpupool it came from (xxx this permits a buggy or malicious
+ * guest to prevent the cpu ever being removed from its cpupool).
+ */ 
+
 #define ARINC653_MAX_DOMAINS_PER_SCHEDULE   64
 /*
  * This structure is used to pass a new ARINC653 schedule from a
-- 
1.7.10.4


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

[Xen-devel] [PATCH 1/3] libxc: Revert "do some retries in xc_cpupool_removecpu() for EBUSY case"

[Ian Jackson]

libxc may be called from within long-running daemons such as libvirt.

In such a system this sleep would enable an uncooperative or buggy
guest to block all toolstack operations for an extended period.

Sadly, therefore, such a retry loop is not feasible without a lot of
engineering which is probably not appropriate.

This reverts commit 1ef6beea187bca8d11152b6c7d987b2b9450f936
"libxc: do some retries in xc_cpupool_removecpu() for EBUSY case"

Signed-off-by: Ian Jackson 
CC: Dario Faggioli 
CC: Wei Liu 
---
 tools/libxc/xc_cpupool.c |   20 +---
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/tools/libxc/xc_cpupool.c b/tools/libxc/xc_cpupool.c
index 261b9c9..c42273e 100644
--- a/tools/libxc/xc_cpupool.c
+++ b/tools/libxc/xc_cpupool.c
@@ -20,7 +20,6 @@
  */
 
 #include 
-#include 
 #include "xc_private.h"
 
 static int do_sysctl_save(xc_interface *xch, struct xen_sysctl *sysctl)
@@ -138,34 +137,17 @@ int xc_cpupool_addcpu(xc_interface *xch,
 return do_sysctl_save(xch, );
 }
 
-/*
- * The hypervisor might return EBUSY when trying to remove a cpu from a
- * cpupool when a domain running in this cpupool has pinned a vcpu
- * temporarily. Do some retries in this case, perhaps the situation
- * cleans up.
- */
-#define NUM_RMCPU_BUSY_RETRIES 5
-
 int xc_cpupool_removecpu(xc_interface *xch,
  uint32_t poolid,
  int cpu)
 {
-unsigned retries;
-int err;
 DECLARE_SYSCTL;
 
 sysctl.cmd = XEN_SYSCTL_cpupool_op;
 sysctl.u.cpupool_op.op = XEN_SYSCTL_CPUPOOL_OP_RMCPU;
 sysctl.u.cpupool_op.cpupool_id = poolid;
 sysctl.u.cpupool_op.cpu = (cpu < 0) ? XEN_SYSCTL_CPUPOOL_PAR_ANY : cpu;
-for ( retries = 0; retries < NUM_RMCPU_BUSY_RETRIES; retries++ ) {
-err = do_sysctl_save(xch, );
-if ( err < 0 && errno == EBUSY )
-sleep(1);
-else
-break;
-}
-return err;
+return do_sysctl_save(xch, );
 }
 
 int xc_cpupool_movedomain(xc_interface *xch,
-- 
1.7.10.4


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

[Xen-devel] [PATCH 2/3] xen: hypercall docs annotations for xen_sysctl_cpupool_op

[Ian Jackson]

Signed-off-by: Ian Jackson 
CC: Jan Beulich 
CC: Tim Deegan 
---
 xen/include/public/sysctl.h |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 4596d20..0849908 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -538,7 +538,7 @@ struct xen_sysctl_numainfo {
 typedef struct xen_sysctl_numainfo xen_sysctl_numainfo_t;
 DEFINE_XEN_GUEST_HANDLE(xen_sysctl_numainfo_t);
 
-/* XEN_SYSCTL_cpupool_op */
+/* ` enum XEN_SYSCTL_cpupool_op { */
 #define XEN_SYSCTL_CPUPOOL_OP_CREATE1  /* C */
 #define XEN_SYSCTL_CPUPOOL_OP_DESTROY   2  /* D */
 #define XEN_SYSCTL_CPUPOOL_OP_INFO  3  /* I */
@@ -546,9 +546,10 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_numainfo_t);
 #define XEN_SYSCTL_CPUPOOL_OP_RMCPU 5  /* R */
 #define XEN_SYSCTL_CPUPOOL_OP_MOVEDOMAIN6  /* M */
 #define XEN_SYSCTL_CPUPOOL_OP_FREEINFO  7  /* F */
+/* ` } */
 #define XEN_SYSCTL_CPUPOOL_PAR_ANY 0x
 struct xen_sysctl_cpupool_op {
-uint32_t op;  /* IN */
+uint32_t op;  /* IN ` enum XEN_SYSCTL_cpupool_op ` */
 uint32_t cpupool_id;  /* IN: CDIARM OUT: CI */
 uint32_t sched_id;/* IN: C  OUT: I  */
 uint32_t domid;   /* IN: M  */
-- 
1.7.10.4


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

[Xen-devel] [PATCH 0/3] Revert xc cpupool retries and document anomaly

[Ian Jackson]

This small series is part of some cleanup for the CPUPOOL RMCPU EBUSY
problem.

The first patch in this series, a libxc patch, reverts what I think is
simply a mistake.  The second is trivial adds some annotations for the
benefit of the hypercall API HTML docs generator.  The third patch
provides an attempt at documenting the RMCPU EBUSY problem.


It is disappointing that such a strange and hard-to-use interface
should be committed to the hypervisor.  A contributing factor seem to
be a failure to realise that the behaviour should be documented.  An
proper attempt to document it would probably have resulted in
improvements to the interface.

I think that (unless suppositions not marked with `xxx' are false)
this doc comment is an improvement over leaving the anomaly totally
undocumented.  I therefore think that this patch is appropriate to
commit right away (!)

Followups, or review comments, which provide more accurate or more
precise wording, would of course be welcome.

I also think that there is room for improvement in the presented
hypercall semantics, as I understand them.


I would have liked to submit a fourth patch which made xl print the
unhelpful message about trying to re-add and re-remove the cpu, only
when it was relevant.

Unfortunately because the hypervisor doesn't seem to distinguish the
troublesome cases, it is not possible for libxl to tell the
difference.  So it is not possible for libxl to return a different
error code which xl could use to tell the difference.

I would appreciate suggestions from hypervisor maintainers as to how
this could be improved.


Finally, sorry for not spotting these problems earlier.

Thanks,
Ian.

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

[Xen-devel] [PATCH] docs/arm64: update the documention for loading XSM support

[fu . wei]

From: Fu Wei 

This patch updates the documention for loading XSM by the module which
lacks a specific compatible string. This mechanism has been added by the
commit ca32012341f3de7d3975407fb963e6028f0d0c8b

Signed-off-by: Fu Wei 
---
 docs/misc/arm/device-tree/booting.txt | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/docs/misc/arm/device-tree/booting.txt 
b/docs/misc/arm/device-tree/booting.txt
index ad98bf3..f45a9c4 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -24,10 +24,19 @@ Each node contains the following properties:
string (which must always be present).
 
Xen will assume that the first module which lacks a more
-   specific compatible string is a "multiboot,kernel" and that
-   the second such is a "multiboot,ramdisk". Any subsequent
-   modules which lack a specific compatiblity string will not
-   receive any special treatment.
+   specific compatible string is a "multiboot,kernel". Xen will
+   detect the XSM magic from the second module which lacks of
+   a specific compatiblity string:
+   - if it's XSM, Xen will assume that the second such is a
+   "xen,xsm-policy". and also assume user won't load ramdisk;
+   - if it's not XSM, Xen will assume that the second such is a
+   "multiboot,ramdisk".
+   So if user want to load ramdisk without a specific compatiblity,
+   it must be the 2nd one.
+   Xen will also detect the XSM Magic for the following modules
+   which lack of a specific compatiblity, and assume that the module
+   is a "xen,xsm-policy" or "multiboot,module", according to the
+   result of detection.
 
Xen 4.4 supported a different set of legacy compatible strings
which remain supported such that systems supporting both 4.4
-- 
2.5.5


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

Re: [Xen-devel] [PATCH] monitor: Rename vm_event_monitor_get_capabilities to arch_monitor_get_capabilities

[Tamas K Lengyel]

On Thu, Apr 14, 2016 at 10:36 AM, Andrew Cooper 
wrote:

> On 14/04/16 17:33, Tamas K Lengyel wrote:
> > The monitor_get_capabilities check actually belongs to the monitor
> subsystem so
> > relocating and renaming it to sanitize the code's name and location.
> >
> > Signed-off-by: Tamas K Lengyel 
> > Cc: Razvan Cojocaru 
> > Cc: Keir Fraser 
> > Cc: Jan Beulich 
> > Cc: Andrew Cooper 
> > Cc: Stefano Stabellini 
> > Cc: Julien Grall 
>
> Looks sensible.  Acked-by: Andrew Cooper 
>
> However, this is definitely 4.8 material at this point.
>

Certainly, all my patches at this point are for 4.8.

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

Re: [Xen-devel] REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[Jan Beulich]

>>> George Dunlap  04/14/16 6:20 PM >>>
>On Thu, Apr 14, 2016 at 4:59 PM, Jan Beulich  wrote:
> George Dunlap  04/14/16 5:16 PM >>>
>>>On the other hand, I think there's a bit of a faulty interpretation of
>>>the procedure here.  Jan reviewed the patch thoroughly and then acked
>>>it; on the basis of that, Konrad legitimately checked it in.  After it
>>>was checked in Jan said, "I've changed my mind and withdraw my Ack";
>>>and the assumption of the subsequent conversation was that an ack
>>>*can* be withdrawn after it has been legitimately checked in, and that
>>>if no other Ack is supplied, then it must be reverted.
>>>
>>>I don't think that's a correct interpretation of the rules.  Reviewers
>>>in general, and maintainers in particular, should make reasonably sure
>>>that they mean the Ack before they give it; and if they change their
>>>mind after it has been legitimately checked in, then it's now up to
>>>them to make the change they want to see according to the regular
>>>procedure.  That is, if Jan wants it reverted, he needs to post a
>>>patch reverting it and get Acks from the appropriate maintainers; and
>>>the discussion needs to be around Jan's reversion being accepted, not
>>>about Konrad's original patch continuing to be accepted.  (Obvious
>>>exceptions can be made in the case of emergencies like build
>>>breakages.)
>>
>> Fundamentally I agree, but I think there's more to this than just following
>> a set of rules. For example, please don't forget the time pressure due to
>> the (at that time) rapidly approaching freeze date. And then, mistakes
>> happen, and so I made a mistake here by sending the ack a few hours too
>> early.
>
>Sure, mistakes happen; but I hope it's not being to controversial to
>say that in general, the procedure should be arranged such that the
>person who makes the mistake is the one who has to do deal with the
>most consequences from the mistake, not the people around him.  I
>mean, if you asked a bartender for a bottle of beer, and after he'd
>already opened it you said, "Oh sorry, I actually wanted a cider", it
>would be fair enough for the bartender to ask you to pay for the beer,
>rather than eating it*, wouldn't it? :-)

Sure. And I think that's what I've done. I suggested to revert the thing,
collecting opinions either direction. I just didn't post a revert patch, as I
think that makes little sense - a revert is a mechanical operation, which
doesn't need people looking at the actual patch.

>Well Ian and I have already given our opinions -- Ian thinks moving to
>a clean interface and deprecating the old one is in general a good
>thing, and doesn't look too painful in this case.  I don't really see
>a problem with using a large fixed size, but I don't fundamentally see
>a problem with moving to a new clean interface either.  Given that
>Andy has a strong aversion to the way things are, if you had only a
>mild distaste rather than a  strong objection to the new hypercall, it
>would probably make sense to go with the new hypercall.
>
>If you do have a strong objection, then maybe we could try to convince
>Andy to accept adding subops with different calling semantics to the
>existing hypercall.  But I would still think the burden of persuasion
>is primarily on you.

 I do not have a strong objection, or else I would have converted my ack
into a nak instead of just withdrawing it. I just dislike the duplication, and
hence I'm not happy with me now being the one having approved it to go
in. Hence the request for a replacement ack (or whatever else referee
decision).

And btw., considering that Konrad has already posted a revert patch,
and I have ack-ed that one, this could now go in right away (and the
discussion could either be settled or start over).

Jan


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

[Xen-devel] Update SeaBIOS to include 8a0df3

[Roger Pau Monné]

Hello,

I would like to request an update of the SeaBIOS repository to include the 
latest commits in the 1.9-stable branch. I'm only interested in commit 
8a0df3, which is basically our current version (rel-1.9.1) plus a build fix 
for FreeBSD (or for objcopy implementations that are strict regarding 
alignments).

Would it be possible to push it to our SeaBIOS and update Config.mk, or now 
is too late?

Thanks, Roger.

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

Re: [Xen-devel] [PATCH v8.1 07/27] arm/x86: Use struct virtual_region to do bug, symbol, and (x86) exception tables lookup.

[Jan Beulich]

>>> Konrad Rzeszutek Wilk  04/14/16 12:04 AM >>>
>+/*
>+ * RCU locking. Additions are done either at startup (when there is only
>+ * one CPU) or when all CPUs are running without IRQs.
>+ *
>+ * Deletions are big tricky. We do it when xSplicing (all CPUs running

"big tricky"?

>+ * without IRQs) or during bootup (when clearing the init).
>+ *
>+ * Hence we use list_del_rcu (which sports an memory fence) and a spinlock
>+ * on deletion.
>+ *
>+ * All readers of virtual_region_list MUST use list list_for_each_entry_rcu.
>+ *
>+ */

Stray empty comment line.

>--- /dev/null
>+++ b/xen/include/xen/virtual_region.h
>@@ -0,0 +1,47 @@
>+/*
>+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
>+ *
>+ */
>+
>+#ifndef __XEN_VIRTUAL_REGION__

__XEN_VIRTUAL_REGION_H__

With those minor aspects taken care of,
Acked-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH] monitor: Rename vm_event_monitor_get_capabilities to arch_monitor_get_capabilities

[Razvan Cojocaru]

On 04/14/16 19:33, Tamas K Lengyel wrote:
> The monitor_get_capabilities check actually belongs to the monitor subsystem 
> so
> relocating and renaming it to sanitize the code's name and location.
> 
> Signed-off-by: Tamas K Lengyel 
> Cc: Razvan Cojocaru 
> Cc: Keir Fraser 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Stefano Stabellini 
> Cc: Julien Grall 

Acked-by: Razvan Cojocaru 

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

Re: [Xen-devel] [PATCH v8.1 04/27] xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op

[Jan Beulich]

>>> Konrad Rzeszutek Wilk  04/14/16 12:05 AM >>>
> @@ -460,6 +461,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
> u_sysctl)
> ret = tmem_control(>u.tmem_op);
> break;
> 
> +case XEN_SYSCTL_xsplice_op:
> +ret = xsplice_op(>u.xsplice);
> +copyback = (ret == -ENOSYS || ret == -EOPNOTSUPP) ? 0 : 1;

Why use a conditional expression here when its condition already is a boolean 
one
just needing negating?

> +static int verify_name(const xen_xsplice_name_t *name, char *n)
> +{
> +if ( !name->size || name->size > XEN_XSPLICE_NAME_SIZE )
> +return -EINVAL;
> +
> +if ( name->pad[0] || name->pad[1] || name->pad[2] )
> +return -EINVAL;
> +
> +if ( !guest_handle_okay(name->name, name->size) )
> +return -EINVAL;
> +
> +if ( __copy_from_guest(n, name->name, name->size) )
> +return -EFAULT;

Is there a particular reason why you open code copy_from_guest() here? And
considering that you now also read the string here, isn't the function name
somewhat off?

> +static int xsplice_upload(xen_sysctl_xsplice_upload_t *upload)
> +{
> +struct payload *data = NULL, *found;
> +char n[XEN_XSPLICE_NAME_SIZE];
> +int rc;
> +
> +rc = verify_payload(upload, n);
> +if ( rc )
> +return rc;
> +
> +spin_lock(_lock);
> +
> +found = find_payload(n);
> +if ( IS_ERR(found) )
> +{
> +rc = PTR_ERR(found);
> +goto out;
> +}
> +else if ( found )
> +{
> +rc = -EEXIST;
> +goto out;
> +}
> +
> +data = xzalloc(struct payload);

I generally advocate for not doing allocations with locks held, and I don't 
think
it would severely complicate the code here doing so.

Jan


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

Re: [Xen-devel] [PATCH] monitor: Rename vm_event_monitor_get_capabilities to arch_monitor_get_capabilities

[Andrew Cooper]

On 14/04/16 17:33, Tamas K Lengyel wrote:
> The monitor_get_capabilities check actually belongs to the monitor subsystem 
> so
> relocating and renaming it to sanitize the code's name and location.
>
> Signed-off-by: Tamas K Lengyel 
> Cc: Razvan Cojocaru 
> Cc: Keir Fraser 
> Cc: Jan Beulich 
> Cc: Andrew Cooper 
> Cc: Stefano Stabellini 
> Cc: Julien Grall 

Looks sensible.  Acked-by: Andrew Cooper 

However, this is definitely 4.8 material at this point.

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

[Xen-devel] [PATCH] monitor: Rename vm_event_monitor_get_capabilities to arch_monitor_get_capabilities

[Tamas K Lengyel]

The monitor_get_capabilities check actually belongs to the monitor subsystem so
relocating and renaming it to sanitize the code's name and location.

Signed-off-by: Tamas K Lengyel 
Cc: Razvan Cojocaru 
Cc: Keir Fraser 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
 xen/arch/x86/monitor.c |  2 +-
 xen/common/monitor.c   |  5 ++---
 xen/include/asm-arm/monitor.h  | 11 ++-
 xen/include/asm-arm/vm_event.h |  9 -
 xen/include/asm-x86/monitor.h  | 23 +++
 xen/include/asm-x86/vm_event.h | 23 ---
 6 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 1fec412..621f91a 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -126,7 +126,7 @@ int arch_monitor_domctl_event(struct domain *d,
 
 default:
 /*
- * Should not be reached unless vm_event_monitor_get_capabilities() is
+ * Should not be reached unless arch_monitor_get_capabilities() is
  * not properly implemented.
  */
 ASSERT_UNREACHABLE();
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index d950a7c..7c3d1c8 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 
 int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
@@ -48,12 +47,12 @@ int monitor_domctl(struct domain *d, struct 
xen_domctl_monitor_op *mop)
 if ( unlikely(mop->event > 31) )
 return -EINVAL;
 /* Check if event type is available. */
-if ( unlikely(!(vm_event_monitor_get_capabilities(d) & (1U << 
mop->event))) )
+if ( unlikely(!(arch_monitor_get_capabilities(d) & (1U << 
mop->event))) )
 return -EOPNOTSUPP;
 break;
 
 case XEN_DOMCTL_MONITOR_OP_GET_CAPABILITIES:
-mop->event = vm_event_monitor_get_capabilities(d);
+mop->event = arch_monitor_get_capabilities(d);
 return 0;
 
 default:
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 6e36e99..3fd3c9d 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -39,11 +39,20 @@ int arch_monitor_domctl_event(struct domain *d,
 /*
  * No arch-specific monitor vm-events on ARM.
  *
- * Should not be reached unless vm_event_monitor_get_capabilities() is not
+ * Should not be reached unless arch_monitor_get_capabilities() is not
  * properly implemented.
  */
 ASSERT_UNREACHABLE();
 return -EOPNOTSUPP;
 }
 
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+uint32_t capabilities = 0;
+
+capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+return capabilities;
+}
+
 #endif /* __ASM_ARM_MONITOR_H__ */
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index 014d9ba..a3fc4ce 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -59,13 +59,4 @@ static inline void vm_event_fill_regs(vm_event_request_t 
*req)
 /* Not supported on ARM. */
 }
 
-static inline uint32_t vm_event_monitor_get_capabilities(struct domain *d)
-{
-uint32_t capabilities = 0;
-
-capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
-
-return capabilities;
-}
-
 #endif /* __ASM_ARM_VM_EVENT_H__ */
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 0954b59..446b2af 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -50,4 +50,27 @@ int arch_monitor_domctl_op(struct domain *d, struct 
xen_domctl_monitor_op *mop)
 int arch_monitor_domctl_event(struct domain *d,
   struct xen_domctl_monitor_op *mop);
 
+static inline uint32_t arch_monitor_get_capabilities(struct domain *d)
+{
+uint32_t capabilities = 0;
+
+/*
+ * At the moment only Intel HVM domains are supported. However, event
+ * delivery could be extended to AMD and PV domains.
+ */
+if ( !is_hvm_domain(d) || !cpu_has_vmx )
+return capabilities;
+
+capabilities = (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG) |
+   (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
+   (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
+   (1U << XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST);
+
+/* Since we know this is on VMX, we can just call the hvm func */
+if ( hvm_is_singlestep_supported() )
+capabilities |= (1U << XEN_DOMCTL_MONITOR_EVENT_SINGLESTEP);
+
+return capabilities;
+}
+
 #endif /* __ASM_X86_MONITOR_H__ */
diff --git a/xen/include/asm-x86/vm_event.h b/xen/include/asm-x86/vm_event.h
index 0470240..026f42e 100644
--- a/xen/include/asm-x86/vm_event.h
+++ 

Re: [Xen-devel] [PATCH v8.1 02/27] Revert "HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane."

[Jan Beulich]

>>> Konrad Rzeszutek Wilk  04/14/16 12:04 AM >>>
>This reverts commit 2716d875379d538c1dfccad78a99ca7db2e09f90.
>
>As it was decided that the existing XENVER hypercall - while having
>grown organically over the years can still be expanded.
>
>Signed-off-by: Konrad Rzeszutek Wilk 

Requested-and-acked-by: Jan Beulich 


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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Jan Beulich]

>>> Razvan Cojocaru  04/14/16 6:00 PM >>>
>On 04/14/2016 06:44 PM, Jan Beulich wrote:
>> That's the performance effect on the hypervisor you talk about. But what 
>> about
>> the guest, which all of the sudden gets another domain wide lock applied?
>
>Well, yes, there's bound to be some performance loss - but I think the
>question is: is the added safety worth it? As always, there are
>trade-offs. I am quite possibly missing something, but surely a slightly
>slower, running guest is better than a fast unstable one.
>
>As for the patch, it's definitely perfectible, and I appreciate all
>suggestions to make it the best it can be (or even to scrape it for a
>better solution).

That's the while thing here: If what you have presented comes at a
reasonable price, we may well take it with obvious issues fixed. Yet if the
price is quite high, investing effort into finding a better solution is imo not
just warranted, but necessary.

Jan


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

Re: [Xen-devel] [PATCH v4 0/3] add hypercall option to temporarily pin a vcpu

[Ian Jackson]

Konrad Rzeszutek Wilk writes ("Re: [Xen-devel] [PATCH v4 0/3] add hypercall 
option to temporarily pin a vcpu"):
> Applied.

Damn, I see I am too late with my review.

I will propose to revert some of this. :-/.

Ian.

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

Re: [Xen-devel] [PATCH v4 3/3] libxl: add force option for xl vcpu-pin

[Ian Jackson]

Juergen Gross writes ("[PATCH v4 3/3] libxl: add force option for xl vcpu-pin"):
> In order to be able to undo a vcpu pin override in case of a kernel
> driver error add a flag "-f" to the "xl vcpu-pin" command forcing the
> hypervisor to undo the override.
...
> +Specifying I<-f> or I<--force> will remove a temporary pinning done by the
> +operating system (normally this should be done by the operating system).
> +In case a temporary pinning is active for a vcpu the affinity of this vcpu
> +can't be changed without this option.

This documentation needs to confirm that this can only happen to dom0,
or other domains which have been granted hardware access.

Assuming that this is actually true.  If not then surely it should be.

Ian.

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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Jan Beulich]

>>> Andrew Cooper  04/14/16 5:46 PM >>>
>Short of having the instruction emulator convert any locked instruction
>into a stub, I can't think of a solution which works for both emulated
>and non-emulated instructions.

That's my understanding too.

Jan


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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Jan Beulich]

>>> Razvan Cojocaru  04/14/16 5:45 PM >>>
>On 04/14/2016 06:40 PM, Jan Beulich wrote:
>> To be honest, just having remembered that we do the write back for locked
>> instructions using CMPXCHG, I'd first of all like to see a proper description
>> of "the _whole_ issue".
>
>I believe at least part of the issue has to do with the comment on line
>1013 from xen/arch/x86/hvm/emulate.c:
>
 >994 static int hvmemul_cmpxchg(
 >995 enum x86_segment seg,
 >996 unsigned long offset,
 >997 void *p_old,
 >998 void *p_new,
 >999 unsigned int bytes,
>1000 struct x86_emulate_ctxt *ctxt)
>1001 {
>1002 struct hvm_emulate_ctxt *hvmemul_ctxt =
>1003 container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
>1004
>1005 if ( unlikely(hvmemul_ctxt->set_context) )
>1006 {
>1007 int rc = set_context_data(p_new, bytes);
>1008
>1009 if ( rc != X86EMUL_OKAY )
>1010 return rc;
>1011 }
>1012
>1013 /* Fix this in case the guest is really relying on r-m-w atomicity. */
>1014 return hvmemul_write(seg, offset, p_new, bytes, ctxt);
>1015 }

Ah, so _that's_ where the problem wants to be fixed then (leaving - afaict -
PV emulation paths completely unaffected).

Jan


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

Re: [Xen-devel] [PATCH v4 2/3] libxl: print message how to recover from xl cpupool-cpu-remove errors

[Ian Jackson]

Juergen Gross writes ("[PATCH v4 2/3] libxl: print message how to recover from 
xl cpupool-cpu-remove errors"):
> An error occurring when calling "xl cpupool-cpu-remove" might leave
> the system in a state where a cpu is neither completely free nor in
> a cpupool.

Surely this is a bug.  Can it not be avoided ?

> This can easily be repaired by adding the cpu via
> "xl cpupool-cpu-add" to the cpupool where it was removed from before.
> Print a message telling this the user in case of an error.
...
> -if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, ))
> -fprintf(stderr, "some cpus may not have been removed from %s\n", 
> pool);
> +if (libxl_cpupool_cpuremove_cpumap(ctx, poolid, )) {
> +fprintf(stderr, "Some cpus may have not or only partially been 
> removed from '%s'.\n", pool);
> +fprintf(stderr, "If a cpu can't be added to another cpupool, add it 
> to '%s' again and retry.\n", pool);
> +}

If it can't be avoided then I guess this will have to do but I remain
to be convinced.

In any case, while you're editing this code, can you please wrap your
long lines.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH v4 1/3] libxc: do some retries in xc_cpupool_removecpu() for EBUSY case

[Ian Jackson]

Juergen Gross writes ("[PATCH v4 1/3] libxc: do some retries in 
xc_cpupool_removecpu() for EBUSY case"):
> The hypervisor might return EBUSY when trying to remove a cpu from a
> cpupool when a domain running in this cpupool has pinned a vcpu
> temporarily. Do some retries in this case, perhaps the situation
> cleans up.

Unfortunately this patch is not acceptable because:

> +for ( retries = 0; retries < NUM_RMCPU_BUSY_RETRIES; retries++ ) {
> +err = do_sysctl_save(xch, );
> +if ( err < 0 && errno == EBUSY )
> +sleep(1);

libxc may be called from within long-running daemons such as libvirt.

In such a system this sleep would enable an uncooperative or buggy
guest to block all toolstack operations for an extended period.

Sorry,
Ian.

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

Re: [Xen-devel] REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[Jan Beulich]

>>> George Dunlap  04/14/16 5:16 PM >>>
>On the other hand, I think there's a bit of a faulty interpretation of
>the procedure here.  Jan reviewed the patch thoroughly and then acked
>it; on the basis of that, Konrad legitimately checked it in.  After it
>was checked in Jan said, "I've changed my mind and withdraw my Ack";
>and the assumption of the subsequent conversation was that an ack
>*can* be withdrawn after it has been legitimately checked in, and that
>if no other Ack is supplied, then it must be reverted.
>
>I don't think that's a correct interpretation of the rules.  Reviewers
>in general, and maintainers in particular, should make reasonably sure
>that they mean the Ack before they give it; and if they change their
>mind after it has been legitimately checked in, then it's now up to
>them to make the change they want to see according to the regular
>procedure.  That is, if Jan wants it reverted, he needs to post a
>patch reverting it and get Acks from the appropriate maintainers; and
>the discussion needs to be around Jan's reversion being accepted, not
>about Konrad's original patch continuing to be accepted.  (Obvious
>exceptions can be made in the case of emergencies like build
>breakages.)

Fundamentally I agree, but I think there's more to this than just following
a set of rules. For example, please don't forget the time pressure due to
the (at that time) rapidly approaching freeze date. And then, mistakes
happen, and so I made a mistake here by sending the ack a few hours too
early.

What is really hard to understand to me is why it is so difficult to just
get a refereeing opinion on the actual interface change. IMO we don't
really need to discuss rules and processes, the question is as simple
as "Do we want/need this new interface?"

Jan


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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Razvan Cojocaru]

On 04/14/2016 06:44 PM, Jan Beulich wrote:
> Razvan Cojocaru  04/14/16 7:57 AM >>>
>> On 04/14/16 07:35, Jan Beulich wrote:
>> Razvan Cojocaru  04/13/16 7:53 PM >>>
 @@ -1589,6 +1591,8 @@ x86_emulate(
>>>  >}
>>>   >done_prefixes:
>>>  >
 +ops->smp_lock(lock_prefix);
 +
>>>  >if ( rex_prefix & REX_W )
>>>  >op_bytes = 8;
>>>  
>>> Already from the context it is obvious that the lock can be taken later.
>>
>> I'll take it later.
> 
> And that alone won't suffice: Instructions not touching memory shouldn't take
> any lock at all. As shouldn't instructions that only read or only write 
> memory.
> 
>>> Overall I can see why you want this, but what is the performance impact? 
>>> After
>>> all you're basically making the emulator act like very old systems using a 
>>> bus
>>> lock (i.e. a global one) instead of utilizing cacheline exclusive 
>>> ownership. Plus
>>> you still don't (and cannot, afaict) deal with one LOCKed instruction 
>>> getting
>>> emulated and another in parallel getting executed by the CPU without 
>>> emulation
>>> (granted this shouldn't normally happen, but we also can't exclude that 
>>> case).
>>
>> I was under the impression that for reads, with the new percpu_rwlocks
>> the performance impact is close to zero, with some performance impact
>> for writes - but writes should, for one, be more infrequent, and then
>> the added safety factor should make up for that.
> 
> That's the performance effect on the hypervisor you talk about. But what about
> the guest, which all of the sudden gets another domain wide lock applied?

Well, yes, there's bound to be some performance loss - but I think the
question is: is the added safety worth it? As always, there are
trade-offs. I am quite possibly missing something, but surely a slightly
slower, running guest is better than a fast unstable one.

As for the patch, it's definitely perfectible, and I appreciate all
suggestions to make it the best it can be (or even to scrape it for a
better solution).


Thanks,
Razvan

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

Re: [Xen-devel] Running Xen on Nvidia Jetson-TK1

[Dushyant Behl]

Hi Everyone,

On Fri, Apr 8, 2016 at 5:57 PM, Ian Campbell  wrote:
>> > In your patch for *Hacky* support for Jetsok-TK1 you said that you
>> > were able to run guests on
>> > Jetson-tk1 board with Xen. Can I know which kernel version you used as
>> > dom0 (and possibly domU guests)?
>
> I'm afraid I don't remember. It would probably either have been some
> current-ish upstream kernel from the time, or possibly a Debian kernel
> from around the time. So maybe something 3.16-ish?

@Ian Thanks for the Reply.

@Julien, Thanks a lot, your patch for GIC dtb node worked and the
linux kernel is now able to enable the arch timer.
(Although I needed to modify the code manually as the tree I'm using
is older Xen-4.6, and maybe the patch was against the latest version
of xen).

The kernel is able to initialize itself but it fails while initializing some
nvidia devices in the boot process. I've posted the log here,
Could this also be an issue with the device tree or this is something
related to the kernel?

- UART enabled -
- CPU  booting -
- Xen starting in Hyp mode -
- Zero BSS -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) Checking for initrd in /chosen
(XEN) RAM: 8000 - ffef
(XEN)
(XEN) MODULE[0]: 8200 - 8201 Device Tree
(XEN) MODULE[1]: 8100 - 815443f0 Kernel
console=hvc0 root=/dev/mmcblk0p1 rw rootwait
(XEN)  RESVD[0]: 8200 - 8201
(XEN)
(XEN) Command line: console=dtuart dtuart=serial0 dom0_mem=512M
sync_console log_lvl=all guest_loglvl=all
(XEN) Placing Xen at 0xffc0-0xffe0
(XEN) Update BOOTMOD_XEN from fd00-fd0f9701 =>
ffc0-ffcf9701
(XEN) Xen heap: fa00-fe00 (16384 pages)
(XEN) Dom heap: 507648 pages
(XEN) Domain heap initialised
(XEN) Platform: TEGRA124
(XEN) Looking for dtuart at "s Xen 4.6-unstable
(XEN) Xen version 4.6-unstable (root@) (arm-linux-gnueabihf-gcc
(Ubuntu/Linaro 4.7.3-11ubuntu1) 4.7.3) debug=y Mon Apr 11 09:20:25 UTC
2016
(XEN) Latest ChangeSet:
(XEN) Console output is synchronous.
(XEN) Processor: 413fc0f3: "ARM Limited", variant: 0x3, part 0xc0f, rev 0x3
(XEN) 32-bit Execution:
(XEN)   Processor Features: 1131:00011011
(XEN) Instruction Sets: AArch32 A32 Thumb Thumb-2 ThumbEE Jazelle
(XEN) Extensions: GenericTimer Security
(XEN)   Debug Features: 02010555
(XEN)   Auxiliary Features: 
(XEN)   Memory Model Features: 10201105 4000 0124 02102211
(XEN)  ISA Features: 02101110 13112111 21232041 2131 10011142 
(XEN) Using PSCI-0.1 for SMP bringup
(XEN) Generic Timer IRQ: phys=30 hyp=26 virt=27 Freq: 12000 KHz
(XEN) GICv2 initialization:
(XEN) gic_dist_addr=50041000
(XEN) gic_cpu_addr=50042000
(XEN) gic_hyp_addr=50044000
(XEN) gic_vcpu_addr=50046000
(XEN) gic_maintenance_irq=25
(XEN) GICv2: 192 lines, 4 cpus, secure (IID 043b).
(XEN) Using scheduler: SMP Credit Scheduler (credit)
(XEN) I/O virtualisation disabled
(XEN) Allocated console ring of 32 KiB.
(XEN) VFP implementer 0x41 architecture 4 part 0x30 variant 0xf rev 0x0
(XEN) Bringing up CPU1
- CPU 0001 booting -
- Xen starting in Hyp mode -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) CPU 1 booted.
(XEN) Bringing up CPU2
- CPU 0002 booting -
- Xen starting in Hyp mode -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) CPU 2 booted.
(XEN) Bringing up CPU3
- CPU 0003 booting -
- Xen starting in Hyp mode -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN) CPU 3 booted.
(XEN) Brought up 4 CPUs
(XEN) P2M: 40-bit IPA
(XEN) P2M: 3 levels with order-1 root, VTCR 0x80003558
(XEN) *** LOADING DOMAIN 0 ***
(XEN) Loading kernel from boot module @ 8100
(XEN) zImage: no appended dtb
(XEN) zImage32 Probe successful
(XEN) Allocating 1:1 mappings totalling 512MB for dom0:
(XEN) BANK[0] 0x00a000-0x00c000 (512MB)
(XEN) Additional MMIO 4-40040 (IRAM)
(XEN) Additional MMIO 54200-54240 (Display A)
(XEN) Additional MMIO 54240-54280 (Display B)
(XEN) Additional MMIO 6000f-60010 (EXCEPTION VECTORS)
(XEN) Additional MMIO 6000c-6000d (SYSREG)
(XEN) Additional MMIO 1000-1001 (PCI CFG0)
(XEN) Additional MMIO 1001-1002 (PCI CFG1)
(XEN) Additional MMIO 12000-12010 (PCI IO)
(XEN) Additional MMIO 13000-2 (PCI MEM)
(XEN) Additional MMIO 2-4 (PCI MEM (PREFETCH))
(XEN) Additional IRQ 105 (DISPLAY)
(XEN) TEGRA: Routing IRQ105 to dom0, ICTLR2, mask 0x000200
(XEN) Additional IRQ 106 (DISPLAY B)
(XEN) TEGRA: Routing IRQ106 to dom0, ICTLR2, mask 0x000400
(XEN) Loading zImage from 8100 to a7a0-a7f443f0
(XEN) Allocating PPI 16 for event channel interrupt
(XEN) Loading dom0 DTB to 0xa800-0xa800f1d0
(XEN) Scrubbing Free RAM on 1 nodes using 4 CPUs
(XEN) done.
(XEN) 

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Andrew Cooper]

On 14/04/16 16:40, Jan Beulich wrote:
 Razvan Cojocaru  04/14/16 1:43 PM >>>
>> On 04/14/2016 01:35 PM, David Vrabel wrote:
>>> On 13/04/16 13:26, Razvan Cojocaru wrote:
 LOCK-prefixed instructions are currenly allowed to run in parallel
 in x86_emulate(), which can lead the guest into an undefined state.
 This patch fixes the issue.
>>> Is this sufficient?  What if another VCPU is running on another PCPU and
>>> doing an unemulated LOCK-prefixed instruction to the same memory address?
>>>
>>> This other VCPU could be for another domain (or Xen for that matter).
>> The patch is only sufficient for parallel runs of emulated instructions,
>> as previously stated. It is, however, able to prevent nasty guest lockups.
>>
>> This is what happened in a previous thread where I was hunting down the
>> issue and initially thought that the xen-access.c model was broken when
>> used with emulation, and even proceeded to check that the ring buffer
>> memory accesses are synchronized properly. They were alright, the
>> problem was in fact LOCKed instruction emulation happening in parallel,
>> i.e. a race condition there.
>>
>> This is less obvious if we signal that vm_event responses are available
>> immediately after processing each one (which greatly reduces the chances
>> of a race happening), and more obvious with guests that have 2 (or more)
>> VCPUs where all of them are paused waiting for a vm_event reply, and all
>> of them are woken up at the same time, after processing all of the
>> events, and asked to emulate.
>>
>> I do believe that somewhere in Xen emulating in this manner could occur,
>> so I hope to make emulation generally safer.
>>
>> As for not fixing the _whole_ issue, as Jan has rightly pointed out,
>> that's a rather difficult thing to do.
> To be honest, just having remembered that we do the write back for locked
> instructions using CMPXCHG, I'd first of all like to see a proper description
> of "the _whole_ issue".

All emulated instructions with a lock prefix end up calling into
hvmemul_cmpxchg()

I suspect the issue is to do with the implementation of
hvmemul_cmpxchg(), which contains a TODO from 2008 of

/* Fix this in case the guest is really relying on r-m-w atomicity. */

which, amongst other things, never updates *p_old.


Short of having the instruction emulator convert any locked instruction
into a stub, I can't think of a solution which works for both emulated
and non-emulated instructions.

~Andrew

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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Razvan Cojocaru]

On 04/14/2016 06:40 PM, Jan Beulich wrote:
 Razvan Cojocaru  04/14/16 1:43 PM >>>
>> On 04/14/2016 01:35 PM, David Vrabel wrote:
>>> On 13/04/16 13:26, Razvan Cojocaru wrote:
 LOCK-prefixed instructions are currenly allowed to run in parallel
 in x86_emulate(), which can lead the guest into an undefined state.
 This patch fixes the issue.
>>>
>>> Is this sufficient?  What if another VCPU is running on another PCPU and
>>> doing an unemulated LOCK-prefixed instruction to the same memory address?
>>>
>>> This other VCPU could be for another domain (or Xen for that matter).
>>
>> The patch is only sufficient for parallel runs of emulated instructions,
>> as previously stated. It is, however, able to prevent nasty guest lockups.
>>
>> This is what happened in a previous thread where I was hunting down the
>> issue and initially thought that the xen-access.c model was broken when
>> used with emulation, and even proceeded to check that the ring buffer
>> memory accesses are synchronized properly. They were alright, the
>> problem was in fact LOCKed instruction emulation happening in parallel,
>> i.e. a race condition there.
>>
>> This is less obvious if we signal that vm_event responses are available
>> immediately after processing each one (which greatly reduces the chances
>> of a race happening), and more obvious with guests that have 2 (or more)
>> VCPUs where all of them are paused waiting for a vm_event reply, and all
>> of them are woken up at the same time, after processing all of the
>> events, and asked to emulate.
>>
>> I do believe that somewhere in Xen emulating in this manner could occur,
>> so I hope to make emulation generally safer.
>>
>> As for not fixing the _whole_ issue, as Jan has rightly pointed out,
>> that's a rather difficult thing to do.
> 
> To be honest, just having remembered that we do the write back for locked
> instructions using CMPXCHG, I'd first of all like to see a proper description
> of "the _whole_ issue".

I believe at least part of the issue has to do with the comment on line
1013 from xen/arch/x86/hvm/emulate.c:

 994 static int hvmemul_cmpxchg(
 995 enum x86_segment seg,
 996 unsigned long offset,
 997 void *p_old,
 998 void *p_new,
 999 unsigned int bytes,
1000 struct x86_emulate_ctxt *ctxt)
1001 {
1002 struct hvm_emulate_ctxt *hvmemul_ctxt =
1003 container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
1004
1005 if ( unlikely(hvmemul_ctxt->set_context) )
1006 {
1007 int rc = set_context_data(p_new, bytes);
1008
1009 if ( rc != X86EMUL_OKAY )
1010 return rc;
1011 }
1012
1013 /* Fix this in case the guest is really relying on r-m-w
atomicity. */
1014 return hvmemul_write(seg, offset, p_new, bytes, ctxt);
1015 }


Thanks,
Razvan

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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Jan Beulich]

 Razvan Cojocaru  04/14/16 7:57 AM >>>
>On 04/14/16 07:35, Jan Beulich wrote:
> Razvan Cojocaru  04/13/16 7:53 PM >>>
>>> @@ -1589,6 +1591,8 @@ x86_emulate(
>>  >}
>>   >done_prefixes:
>>  >
>>> +ops->smp_lock(lock_prefix);
>>> +
>>  >if ( rex_prefix & REX_W )
>>  >op_bytes = 8;
>>  
>> Already from the context it is obvious that the lock can be taken later.
>
>I'll take it later.

And that alone won't suffice: Instructions not touching memory shouldn't take
any lock at all. As shouldn't instructions that only read or only write memory.

>> Overall I can see why you want this, but what is the performance impact? 
>> After
>> all you're basically making the emulator act like very old systems using a 
>> bus
>> lock (i.e. a global one) instead of utilizing cacheline exclusive ownership. 
>> Plus
>> you still don't (and cannot, afaict) deal with one LOCKed instruction getting
>> emulated and another in parallel getting executed by the CPU without 
>> emulation
>> (granted this shouldn't normally happen, but we also can't exclude that 
>> case).
>
>I was under the impression that for reads, with the new percpu_rwlocks
>the performance impact is close to zero, with some performance impact
>for writes - but writes should, for one, be more infrequent, and then
>the added safety factor should make up for that.

That's the performance effect on the hypervisor you talk about. But what about
the guest, which all of the sudden gets another domain wide lock applied?

Jan


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

Re: [Xen-devel] Code Review Dashboard (nearly-complete)

[Wei Liu]

On Thu, Apr 14, 2016 at 01:26:29PM +0100, Lars Kurth wrote:
> Hi folks,
> 
> the code review dashboard is nearly complete. The read-only version is 
> accessible via https://kibana.bitergia.com/xen
> 
> We added plenty of documentation in the Documentation menu at the top, which 
> explains how the panels and various use-cases work.
> 
> Items still to do:
> * We have not been able to match code reviews that relate to Linux, QEMU, 
> FreeBSD, NetBSD, etc. which are cross-posted on xen-devel@ ... These will 
> currently show up as un-merged and skew the backlog (aka make it bigger than 
> it is). We will look at these at some later point.
> * The data is nearly up-to-date as per 2 days ago, but we have not yet set up 
> the cron-job to update the underlying data set
> * We have not added time filters for release cycles (Xen 4.2, Xen 4.3, ...) : 
> I just have not had the time to map the labels to 

The last point is particularly good idea.

> 
> But generally it is very good and I believe it should help all members within 
> the Xen community to track their own patches, as well as help reviewers, 
> maintainers and committers.
> 
> Feedback is welcome: I can also easily create new views and dashboards, if 
> needed. I will give a demo at the Hackathon, for those who are interested. I 
> believe, the dashboard will really be able to provide some useful insights, 
> as well as help various members in the community.
> 
> Please have a look!
> 

The upside is that the tool looks very powerful, the downside is that it
is a bit hard to consume information and filter stuff without prior
experience. The way the documentation is arranged is not very
structural and the glossary is missing at the moment. I think that can
be improved.

Wei.


> Best Regards
> Lars 
> ___
> 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] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Razvan Cojocaru]

On 04/14/2016 06:31 PM, Jan Beulich wrote:
 Razvan Cojocaru  04/14/16 10:50 AM >>>
>> On 04/14/2016 07:35 AM, Jan Beulich wrote:
 --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -25,6 +25,8 @@
>>>  >#include 
>>>  >#include 
>>>  >
> +DEFINE_PERCPU_RWLOCK_GLOBAL(emulate_locked_rwlock);
>>> You should try hard to make this static.
>>
>> On second though, this would make the code somewhat more convoluted, as
>> the functions in emulate.c need to access this variable, and so does the
>> code in domain.c that initializes the lock when the domain is created.
>>
>> What I've done is similar to what I found in the current source code with:
>>
>> arch/x86/mm/p2m.c:DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>> common/grant_table.c:DEFINE_PERCPU_RWLOCK_GLOBAL(grant_rwlock);
> 
> Well, that's why I said "you should try hard" instead of "you have to".
> 
>> But I could add a function to emulate.h, for example:
>>
>> void init_emulate_smp_lock();
> 
> Exactly (just that this is perhaps again the wrong header, considering you
> also need this for PV emulation).

I understand, I'll look for a more suitable place for the function
declarations and definitions then (unless a specific place is requested).


Thanks,
Razvan

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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Jan Beulich]

>>> Razvan Cojocaru  04/14/16 1:43 PM >>>
>On 04/14/2016 01:35 PM, David Vrabel wrote:
>> On 13/04/16 13:26, Razvan Cojocaru wrote:
>>> LOCK-prefixed instructions are currenly allowed to run in parallel
>>> in x86_emulate(), which can lead the guest into an undefined state.
>>> This patch fixes the issue.
>> 
>> Is this sufficient?  What if another VCPU is running on another PCPU and
>> doing an unemulated LOCK-prefixed instruction to the same memory address?
>> 
>> This other VCPU could be for another domain (or Xen for that matter).
>
>The patch is only sufficient for parallel runs of emulated instructions,
>as previously stated. It is, however, able to prevent nasty guest lockups.
>
>This is what happened in a previous thread where I was hunting down the
>issue and initially thought that the xen-access.c model was broken when
>used with emulation, and even proceeded to check that the ring buffer
>memory accesses are synchronized properly. They were alright, the
>problem was in fact LOCKed instruction emulation happening in parallel,
>i.e. a race condition there.
>
>This is less obvious if we signal that vm_event responses are available
>immediately after processing each one (which greatly reduces the chances
>of a race happening), and more obvious with guests that have 2 (or more)
>VCPUs where all of them are paused waiting for a vm_event reply, and all
>of them are woken up at the same time, after processing all of the
>events, and asked to emulate.
>
>I do believe that somewhere in Xen emulating in this manner could occur,
>so I hope to make emulation generally safer.
>
>As for not fixing the _whole_ issue, as Jan has rightly pointed out,
>that's a rather difficult thing to do.

To be honest, just having remembered that we do the write back for locked
instructions using CMPXCHG, I'd first of all like to see a proper description
of "the _whole_ issue".

Jan


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

Re: [Xen-devel] xl migrate regression in staging

[Olaf Hering]

On Thu, Apr 14, Wei Liu wrote:

> Maybe go back to 96ae556569b8eaedc0bb242932842c3277b515d8 and try again?
> Then 5cf46a66883ad7a56c5bdee97696373473f80974 and try? So that I can
> know if it is related to COLO series. No, don't try to bisect that
> because it's broken in the middle.

I think I took enough snapshots for my rpm packages to get close to the
above commits. It will take some time to cycle through them.


> Also as Andrew suggested if you can reproduce it with a smaller domain
> or share the guest image with us, that would be helpful.

I have already sent him the link.

Olaf

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

Re: [Xen-devel] [PATCH V2] vm_event: Allow subscribing to write events for specific MSR-s

[Razvan Cojocaru]

On 04/14/2016 06:33 PM, Tamas K Lengyel wrote:
> 
> 
> On Thu, Apr 14, 2016 at 9:20 AM, Jan Beulich  > wrote:
> 
> >>> Razvan Cojocaru  > 04/14/16 11:37 AM >>>
> >On 04/13/2016 06:05 PM, Tamas K Lengyel wrote:
> >>
> >> Yea, well then we need to introduce a new struct with a new subop to
> >> pass the bitmask. I guess its a lesson in ABI design to leave some
> >> wiggle room for future-proofing it (my bad). So I guess we can 
> introduce
> >> XEN_DOMCTL_MONITOR_OP_ENABLE_V2 and struct xen_domctl_monitor_op_v2
> >> where say expand the union to uint64_t just in case?
> >
> >I can do that, but it would seem that this is somewhat at odds with
> >Andrew Cooper's perspective - he has stated that it's within the rules
> >and the domctl can be changed without there being the need for
> >XEN_DOMCTL_MONITOR_OP_ENABLE_V2. So this should be clarified, please,
> >otherwise I'm incurring the risk of changing the code only to have to
> >revert it later.
> 
> You basically have two options - the new sub-op or changing the existing
> one while (if not already done so in a dev cycle) bumping the domctl
> interface version.
> 
> 
> If bumping the domctl version is not too much hassle I think that would
> be the easiest.

Fair enough, I'll look into that option then.


Thanks,
Razvan

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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Jan Beulich]

>>> Razvan Cojocaru  04/14/16 11:08 AM >>>
>On 04/14/2016 08:56 AM, Razvan Cojocaru wrote:
 --- a/xen/arch/x86/mm.c
 >> +++ b/xen/arch/x86/mm.c
 >> @@ -112,6 +112,7 @@
>>> >  >#include 
>>> >  >#include 
>>> >  >#include 
 >> +#include 
>>> >  
>>> > This header shouldn't be included here. You need to move the declarations
>>> > elsewhere for them to be usable here.
>> Understood. Is there any place particularly fitting you could suggest?
>
>Actually, I've removed that #include and the code continues to compile,
>so it would appear that it is somehow #included indirectly, which makes
>the declarations accessible implicitly.

But - see my other reply just sent - it is _still_ the wrong header then. Stuff
shared between PV and HVM doesn't belong in HVM specific files. There are
a few counterexamples, but we shouldn't add more.

Jan


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

Re: [Xen-devel] [PATCH V2] vm_event: Allow subscribing to write events for specific MSR-s

[Tamas K Lengyel]

On Thu, Apr 14, 2016 at 9:20 AM, Jan Beulich  wrote:

> >>> Razvan Cojocaru  04/14/16 11:37 AM >>>
> >On 04/13/2016 06:05 PM, Tamas K Lengyel wrote:
> >>
> >> Yea, well then we need to introduce a new struct with a new subop to
> >> pass the bitmask. I guess its a lesson in ABI design to leave some
> >> wiggle room for future-proofing it (my bad). So I guess we can introduce
> >> XEN_DOMCTL_MONITOR_OP_ENABLE_V2 and struct xen_domctl_monitor_op_v2
> >> where say expand the union to uint64_t just in case?
> >
> >I can do that, but it would seem that this is somewhat at odds with
> >Andrew Cooper's perspective - he has stated that it's within the rules
> >and the domctl can be changed without there being the need for
> >XEN_DOMCTL_MONITOR_OP_ENABLE_V2. So this should be clarified, please,
> >otherwise I'm incurring the risk of changing the code only to have to
> >revert it later.
>
> You basically have two options - the new sub-op or changing the existing
> one while (if not already done so in a dev cycle) bumping the domctl
> interface version.


If bumping the domctl version is not too much hassle I think that would be
the easiest.

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

Re: [Xen-devel] [for-4.7] x86/emulate: synchronize LOCKed instruction emulation

[Jan Beulich]

>>> Razvan Cojocaru  04/14/16 10:50 AM >>>
>On 04/14/2016 07:35 AM, Jan Beulich wrote:
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> >+++ b/xen/arch/x86/hvm/emulate.c
>>> >@@ -25,6 +25,8 @@
>>  >#include 
>>  >#include 
>>  >
>>> >+DEFINE_PERCPU_RWLOCK_GLOBAL(emulate_locked_rwlock);
>> You should try hard to make this static.
>
>On second though, this would make the code somewhat more convoluted, as
>the functions in emulate.c need to access this variable, and so does the
>code in domain.c that initializes the lock when the domain is created.
>
>What I've done is similar to what I found in the current source code with:
>
>arch/x86/mm/p2m.c:DEFINE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
>common/grant_table.c:DEFINE_PERCPU_RWLOCK_GLOBAL(grant_rwlock);

Well, that's why I said "you should try hard" instead of "you have to".

>But I could add a function to emulate.h, for example:
>
>void init_emulate_smp_lock();

Exactly (just that this is perhaps again the wrong header, considering you
also need this for PV emulation).

>and call that from arch/x86/domain.c (as suggested by Andrew Cooper),
>which would allow domain.c to stop referencing emulate_locked_rwlock
>directly.

Right, and to be honest I can't really see how my earlier comment could
have been taken for "add an ARM stub".

Jan


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

Re: [Xen-devel] xl migrate regression in staging

[Wei Liu]

On Thu, Apr 14, 2016 at 03:03:01PM +0200, Olaf Hering wrote:
> Migration from staging-4.5.3f802a5 to staging-4-7.3dac42f fails with a HVM 
> guest:
> 
> 
> root@anonymi:~ # xl migrate domU host
> migration target: Ready to receive domain.
> Saving to migration stream new xl format (info 0x1/0x0/2677)
> xc: error: error polling suspend notification channel: -1: Internal error

Is there anything in xl dmesg regarding this?

> Loading new save file  (new xl fmt info 
> 0x1/0x0/2677)
>  Savefile contains xl domain config in JSON format
> Parsing config from 
> libxl: error: libxl_stream_read.c:327:stream_header_done: Invalid ident: 
> expected 0x4c6962786c466d74, got 0x01f00f00

This is libxl expected libxl stream header (the 0x4cxxx is header
signature) but got something else.

> libxl: error: libxl_utils.c:430:libxl_read_exactly: file/stream truncated 
> reading ipc msg header from domain 1 save/restore helper stdout pipe
> libxl: error: libxl_exec.c:129:libxl_report_child_exitstatus: domain 1 
> save/restore helper [-1] died due to fatal signal Broken pipe
> libxl: error: libxl_dom.c:2036:remus_teardown_done: Remus: failed to teardown 
> device for guest with domid 1, rc -3

I feel quite confused why remus is involved.

> migration sender: libxl_domain_suspend failed (rc=-3)
> libxl: info: libxl_exec.c:118:libxl_report_child_exitstatus: migration 
> transport process [2000] exited with error status 255
> Migration failed, resuming at sender.
> xc: error: Dom 1 not suspended: (shutdown 0, reason 255): Internal error
> libxl: error: libxl.c:515:libxl__domain_resume: xc_domain_resume failed for 
> domain 1: Invalid argument
> 
> 
> I think this regression was introduced in the last 3-4 weeks.
> 

Maybe go back to 96ae556569b8eaedc0bb242932842c3277b515d8 and try again?
Then 5cf46a66883ad7a56c5bdee97696373473f80974 and try? So that I can
know if it is related to COLO series. No, don't try to bisect that
because it's broken in the middle.

Also as Andrew suggested if you can reproduce it with a smaller domain
or share the guest image with us, that would be helpful.

Wei.

> Olaf
> 
> ___
> 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] [for-4.7 1/2] xen/bitops: Introduce macros to generate mask

[Jan Beulich]

>>> Julien Grall  04/14/16 5:08 PM >>>
>On 14/04/16 15:56, Jan Beulich wrote:
> Julien Grall  04/14/16 10:55 AM >>>
>>> On 14/04/2016 05:01, Jan Beulich wrote:
>>> Julien Grall  04/13/16 6:01 PM >>>
> --- a/xen/include/xen/bitops.h
> +++ b/xen/include/xen/bitops.h
> @@ -3,6 +3,17 @@
>#include 
>
>/*
> + * Create a contiguous bitmask starting at bit position @l and ending at
> + * position @h. For example
> + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x00e0.
> + */
> +#define GENMASK(h, l) \
> +(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h
> +
> +#define GENMASK_ULL(h, l) \
> +(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h

 Irrespective of Linux perhaps considering them useful, I'm not sure they
 are (and ISTR these macros having got proposed before).
>>>
>>> This is useful on ARM to generate mask for register. For instance, the
>>> following patch introduces mask for the register HPFAR_EL2. Only the
>>> bits [4:39] are usable, the rest are RES0.
>>>
>>> For ARM, RES0 means the bit is currently read as zero but the software
>>> should not rely on it to preserve forward compatibility. So we want to
>>> mask those bits to avoid breakage with new version of the architecture.
>>
>>   All understood and needed on every kind of architecture. Yet what's wrong
>> with expressing this is as 0xf0, as is being done most everywhere 
>> else?
>
>It is less intuitive to read and it is easier to make a mistake in the mask.

Well, I guess that depends on the person reading/writing the respective piece
of code. To me the macroized form would at least require getting used to.

Jan


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

Re: [Xen-devel] [PATCH V2] vm_event: Allow subscribing to write events for specific MSR-s

[Jan Beulich]

>>> Razvan Cojocaru  04/14/16 11:37 AM >>>
>On 04/13/2016 06:05 PM, Tamas K Lengyel wrote:
>> 
>> Yea, well then we need to introduce a new struct with a new subop to
>> pass the bitmask. I guess its a lesson in ABI design to leave some
>> wiggle room for future-proofing it (my bad). So I guess we can introduce
>> XEN_DOMCTL_MONITOR_OP_ENABLE_V2 and struct xen_domctl_monitor_op_v2
>> where say expand the union to uint64_t just in case?
>
>I can do that, but it would seem that this is somewhat at odds with
>Andrew Cooper's perspective - he has stated that it's within the rules
>and the domctl can be changed without there being the need for
>XEN_DOMCTL_MONITOR_OP_ENABLE_V2. So this should be clarified, please,
>otherwise I'm incurring the risk of changing the code only to have to
>revert it later.

You basically have two options - the new sub-op or changing the existing
one while (if not already done so in a dev cycle) bumping the domctl
interface version.

Jan


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

Re: [Xen-devel] [PATCH v8.1] xSplice v1 design and implementation.

[Jan Beulich]

>>> Andrew Cooper  04/14/16 5:14 PM >>>
>On 14/04/16 15:26, Konrad Rzeszutek Wilk wrote:
>> @@ -312,8 +307,8 @@ struct xsplice_patch_func {
>>  };  
>>  
>>  
>> -The size of the structure is 64 bytes or 52 bytes if compiled under 32-bit
>> -hypervisors.
>> +The size of the structure is 64 bytes on 64-bit hypervisors. It will be
>> +52 on 32-bit hypervisors.
>
>I would go further an explicitly declare this API unstable, as patches
>must be built against an exact hypervisor source.  This also gives us
>leaway in the future.

Which it effectively is, now being (iirc) inside a __XEN__ conditional.

Jan


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

Re: [Xen-devel] REST MAINTAINERS feedback requested Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring XENVER_ but sane.

[George Dunlap]

On Wed, Apr 13, 2016 at 5:07 PM, Ian Jackson  wrote:
> Jan Beulich writes ("Re: [Xen-devel] REST MAINTAINERS feedback requested 
> Was:Re: [PATCH v5 01/28] HYPERCALL_version_op. New hypercall mirroring 
> XENVER_ but sane."):
>> George Dunlap  04/12/16 11:58 AM >>>
>> >2. Use the existing hypercall but wedge in different calling semantics
>> >for the build-id subop.  We could have just that subop pay attention
>> >to an extra argument as a length, for example, and return an error if
>> >the length is too short.  Or we could make essentially a flag that
>> >asks, "How much space if I were to execute this subop for real?"
>>
>> A suitable variant of this is what I've been arguing for.
>
> Earlier I wrote:
>
>   It's clear that there are various options, most of which are
>   tolerable.  Buit if I'm trying to help referee a disagreement between
>   Andrew and Jan I would prefer to be choosing between Andrew's
>   preferred answer and Jan's preferred answer.
>
> So as I see it we have two options actually seriously proposed:
> Andrew's new hypercall, and Jan's additional argument (in the struct,
> seems to be what Jan is mainly suggesting).
>
> The new hypercall is neater but more new code - and involves a
> deprecation plan; the additional argument is more messy but less
> duplication.
>
> I think either of these options is tolerable.  I don't see the need to
> look further.
>
> Frankly, I find the choice difficult.  But the bikeshed has to be
> painted /some/ colour and we should make these decisions in a sensible
> way and that means I and George (who've been called on to help decide)
> need to put forward an opinion.
>
> On balance I think I prefer Jan's suggestion, mostly on the grounds
> that in case of dispute, disagreement, or uncertainty, it is (all
> other things being equal) better to make smaller changes.  And if this
> hypercall becomes _too_ much of a mess we can always replace it later
> along the lines that Andrew suggests.

I'm a bit torn here.  I think on the whole, I agree with Ian's
approach that if there is disagreement, then the more conservative
approach should be taken.  And if we were discussing an uncommitted
patch about which no consensus had been reached, I would second his
suggestion.

On the other hand, I think there's a bit of a faulty interpretation of
the procedure here.  Jan reviewed the patch thoroughly and then acked
it; on the basis of that, Konrad legitimately checked it in.  After it
was checked in Jan said, "I've changed my mind and withdraw my Ack";
and the assumption of the subsequent conversation was that an ack
*can* be withdrawn after it has been legitimately checked in, and that
if no other Ack is supplied, then it must be reverted.

I don't think that's a correct interpretation of the rules.  Reviewers
in general, and maintainers in particular, should make reasonably sure
that they mean the Ack before they give it; and if they change their
mind after it has been legitimately checked in, then it's now up to
them to make the change they want to see according to the regular
procedure.  That is, if Jan wants it reverted, he needs to post a
patch reverting it and get Acks from the appropriate maintainers; and
the discussion needs to be around Jan's reversion being accepted, not
about Konrad's original patch continuing to be accepted.  (Obvious
exceptions can be made in the case of emergencies like build
breakages.)

Normally it's best to try to come to agreement instead of falling back
to rule lawyering; and in any case it's always easier to talk about
the rules when we're not trying to sort out a specific situation.  But
since we've failed to reach a genuine agreement, in this case I think
the rule lawyering is probably a legitimate way to resolve the issue.

Thoughts?

 -George

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

Re: [Xen-devel] [PATCH v8.1] xSplice v1 design and implementation.

[Andrew Cooper]

On 14/04/16 15:26, Konrad Rzeszutek Wilk wrote:
> @@ -312,8 +307,8 @@ struct xsplice_patch_func {
>  };  
>  
>  
> -The size of the structure is 64 bytes or 52 bytes if compiled under 32-bit
> -hypervisors.
> +The size of the structure is 64 bytes on 64-bit hypervisors. It will be
> +52 on 32-bit hypervisors.

I would go further an explicitly declare this API unstable, as patches
must be built against an exact hypervisor source.  This also gives us
leaway in the future.

~Andrew

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

Re: [Xen-devel] [for-4.7 1/2] xen/bitops: Introduce macros to generate mask

[Julien Grall]

Hi Jan,

On 14/04/16 15:56, Jan Beulich wrote:

Julien Grall  04/14/16 10:55 AM >>>

On 14/04/2016 05:01, Jan Beulich wrote:

Julien Grall  04/13/16 6:01 PM >>>

--- a/xen/include/xen/bitops.h
+++ b/xen/include/xen/bitops.h
@@ -3,6 +3,17 @@

   >#include 
   >
   >/*

+ * Create a contiguous bitmask starting at bit position @l and ending at
+ * position @h. For example
+ * GENMASK_ULL(39, 21) gives us the 64bit vector 0x00e0.
+ */
+#define GENMASK(h, l) \
+(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h
+
+#define GENMASK_ULL(h, l) \
+(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h


Irrespective of Linux perhaps considering them useful, I'm not sure they
are (and ISTR these macros having got proposed before).


This is useful on ARM to generate mask for register. For instance, the
following patch introduces mask for the register HPFAR_EL2. Only the
bits [4:39] are usable, the rest are RES0.

For ARM, RES0 means the bit is currently read as zero but the software
should not rely on it to preserve forward compatibility. So we want to
mask those bits to avoid breakage with new version of the architecture.


  All understood and needed on every kind of architecture. Yet what's wrong
with expressing this is as 0xf0, as is being done most everywhere else?


It is less intuitive to read and it is easier to make a mistake in the mask.

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v8.1 22/27] XENVER_build_id/libxc: Provide ld-embedded build-id

[Daniel De Graaf]

On 04/13/2016 06:02 PM, Konrad Rzeszutek Wilk wrote:

If the hypervisor was built with build-ids we can expose the
build-id value to the toolstack (if it is not built with
it will just return -ENODATA). This is a priviligied operation
so only the controlling stack is able to request this.

Signed-off-by: Konrad Rzeszutek Wilk 


Acked-by: Daniel De Graaf 

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

Re: [Xen-devel] [PATCH v8.1 23/27] libxl: info: Display build_id of the hypervisor.

[Wei Liu]

On Wed, Apr 13, 2016 at 06:02:04PM -0400, Konrad Rzeszutek Wilk wrote:
> If the hypervisor is built with we will display it.
> 
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
> CC: Ian Jackson 
> CC: Wei Liu 

Acked-by: Wei Liu 

One nit below.

>  const libxl_version_info* libxl_get_version_info(libxl_ctx *ctx)
>  {
>  GC_INIT(ctx);
> @@ -5363,8 +5395,10 @@ const libxl_version_info* 
> libxl_get_version_info(libxl_ctx *ctx)
>  xen_capabilities_info_t xen_caps;
>  xen_platform_parameters_t p_parms;
>  xen_commandline_t xen_commandline;
> +xen_build_id_t build_id;
>  } u;
>  long xen_version;
> +int r;
>  libxl_version_info *info = >version_info;
>  
>  if (info->xen_version_extra != NULL)
> @@ -5397,6 +5431,17 @@ const libxl_version_info* 
> libxl_get_version_info(libxl_ctx *ctx)
>  xc_version(ctx->xch, XENVER_commandline, _commandline);
>  info->commandline = libxl__strdup(NOGC, u.xen_commandline);
>  
> +u.build_id.len = sizeof(u) - sizeof(u.build_id);
> +r = libxl__xc_version_wrap(gc, info, _id);
> +if (r == -ENOBUFS) {
> +xen_build_id_t *build_id;
> +
> +build_id = libxl__zalloc(gc, info->pagesize);
> +build_id->len = info->pagesize - sizeof(*build_id);
> +r = libxl__xc_version_wrap(gc, info, build_id);
> +if (r)
> +LOGEV(ERROR, r, "getting build_id");

Indentation of this block is wrong.

Wei.

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

Re: [Xen-devel] [PATCH v8.1 22/27] XENVER_build_id/libxc: Provide ld-embedded build-id

[Wei Liu]

On Wed, Apr 13, 2016 at 06:02:03PM -0400, Konrad Rzeszutek Wilk wrote:
> If the hypervisor was built with build-ids we can expose the
> build-id value to the toolstack (if it is not built with
> it will just return -ENODATA). This is a priviligied operation
> so only the controlling stack is able to request this.
> 
> Signed-off-by: Konrad Rzeszutek Wilk 
> ---
> CC: Daniel De Graaf 
> CC: Ian Jackson 
> CC: Wei Liu 

Acked-by: Wei Liu 


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