Re: [Xen-devel] [PATCH v5 2/4] x86/ldt: Make modify_ldt synchronous

[Borislav Petkov]

On Mon, Jul 27, 2015 at 10:29:39PM -0700, Andy Lutomirski wrote:
> modify_ldt has questionable locking and does not synchronize
> threads.  Improve it: redesign the locking and synchronize all
> threads' LDTs using an IPI on all modifications.
> 
> This will dramatically slow down modify_ldt in multithreaded
> programs, but there shouldn't be any multithreaded programs that
> care about modify_ldt's performance in the first place.
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Andy Lutomirski 

I've stared a lot at this one these days, I guess a

Reviewed-by: Borislav Petkov 

is in order.

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

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

Re: [Xen-devel] [xen-unstable test] 60076: regressions - FAIL

[Dario Faggioli]

On Wed, 2015-07-29 at 15:15 +0100, Julien Grall wrote:
> On 29/07/15 15:10, Julien Grall wrote:

> > osstest is waiting 40s to get the network ready in the guest. When the
> > test pass, the osstest is likely waiting ~20s to pass it. I took the
> > time between
> > 
> > guest debian.guest.osstest 5a:36:0e:06:00:20 22 link/ip/tcp: waiting 40s...
> > 
> > and the first
> > 
> > executing ssh ... root@172.16.146.149 echo guest debian.guest.osstest: ok
> > guest debian.guest.osstest: ok
> 
> > For instance see
> > http://logs.test-lab.xenproject.org/osstest/logs/59910/test-armhf-armhf-xl-multivcpu/14.ts-guest-start.log
> 
> FWIW, there is also worth case where the waiting time very close to 40s
> (exactly 38s):
> 
> http://logs.test-lab.xenproject.org/osstest/logs/59721/test-armhf-armhf-xl-multivcpu/14.ts-guest-start.log
> 
Exactly my point, together with this:

http://logs.test-lab.xenproject.org/osstest/logs/60076/test-armhf-armhf-xl-multivcpu/arndale-metrocentre---var-log-xen-console-guest-debian.guest.osstest.log

It show two instances of full guest boot, which makes sense as it is the
second attempt that "fails".

Look at the second one and note:
 - that it actually boots fine
 - for some reason, we have:

[1.196443] udevd[69]: starting version 175
Begin: Loading essential drivers ... done.
Begin: Running /scripts/init-premount ... done.
Begin: Mounting root file system ... Begin: 
Running /scripts/local-top ... done.
Begin: Running /scripts/local-premount ... done.
Begin: Running /scripts/local-premount ... done.
[   20.741128] EXT4-fs (xvda2): mounting ext3 file system using the ext4 
subsystem
[   20.755723] EXT4-fs (xvda2): mounted filesystem with ordered data mode. 
Opts: (null)
... ... ... ...
[   47.329342] EXT4-fs (xvda2): re-mounted. Opts: (null)
[] Checking root file system...fsck from util-linux 2.20.1
/dev/xvda2: clean, 14689/262144 files, 124109/1048576 blocks
... ... ... ...
[   47.803550] EXT4-fs (xvda2): re-mounted. Opts: errors=remount-ro

   so it looks like it did take quite a bit to start. Yes, that's in 
   guest time, but stil...

In first instance, we have this:

[1.221159] udevd[69]: starting version 175
Begin: Loading essential drivers ... done.
Begin: Running /scripts/init-premount ... done.
Begin: Mounting root file system ... Begin: Running /scripts/local-top ... done.
Begin: Running /scripts/local-premount ... done.
[2.275805] EXT4-fs (xvda2): mounting ext3 file system using the ext4 
subsystem
[2.300418] EXT4-fs (xvda2): mounted filesystem with ordered data mode. 
Opts: (null)
... ... ... ...
[5.958201] EXT4-fs (xvda2): re-mounted. Opts: (null)
[] Checking root file system...fsck from util-linux 2.20.1
... ... ... ...
[6.424911] EXT4-fs (xvda2): re-mounted. Opts: errors=remount-ro

Then, no, I don't think I see why the pre-mount activities (I don't even
know what those are, although, I don't think it matters) already is ~10x
slower, and then the mounting and the fsck check ~6x...

The host is certainly overloaded, in terms of number of vcpus vs. number
of pcpus, but it's not that all those vcpus should be super busy at this
point... Perhaps, the host being practically UP matters (I don't think
I've actually ever run Xen on an UP system! :-P)

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


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

Re: [Xen-devel] Second regression due to libxl: Remove linux udev rules (2ba368d13893402b2f1fb3c283ddcc714659dd9b)

[Ian Campbell]

On Wed, 2015-07-29 at 11:45 -0400, Konrad Rzeszutek Wilk wrote:
> relying on the (stale) 4.5 rules file having the UDEV_CALL=1 in them.
> 
> I don't exactly understand how the hotplug scripts are invoked via 'xl'.

They are called when the backend gets to (or passes through)
XenbusStateInitWait (attach) or XenbusStateClosed (detach).

> With udev it was pretty clear and easy to me.

It was also, unfortunately, racy.

In particular on tear down there was no interlock between the scripts
(executed asynchronously by udev) and the toolstack. Some backends have
interlock between the backend and the script, but that's not the
same/sufficient.

This race means that the xenstore dir could be removed before the script
runs, and the script may need information from xenstore in order to do the
tear down.

This was a particular problem for detaching a vif on a vswitch system,
since vswitch (unlike Linux bridge) does not automatically remove a port
when the device disappears, so we need xenstore info (specifically the
bridge node) to clean up.

I believe there were also similar issues with block-iscsi (to logout of the
target) and even regular block devices where loopback was in use (to see
the type and know whether to losetup -d or not, this was the reason why we
didn't do loopback for file:// devices with libxl for quite a while).

It was also completely different for each backend platform (Linux,
BSD,etc), which was problematic from a support PoV.

> Note that I see this problem regardless of me having 'xl devd' running or 
> not.

You definitely do _not_ want to run xl devd in dom0 (or more precisely in
your toolstack domain). Having both xl and devd doing this operations will
not result in anything you want.

Might be worth having some interlock on that, if we don't already.

> > Another option would be to install an empty xen-backend.rules for the
> > 4.6 release, and then remove it for 4.7.
> 
> Or trim down the udev rules ?

The udev scripts should have been unused since 4.5.0-rc1, where they were
by default gated from running in dom0 in favour of the libxl version. In
the default configuration the scripts detected when they were called via
udev and exited immediately without doing anything, leaving them to do the
real work when called directly from the toolstack.

Have you been seeing this issue since then and "fixing" it by manually
reverting to the udev behaviour in /etc/xen/xl.conf (or elsewhere for other
libxl clients)?

If not then there is some unintentional change in 2ba368d138934 as well as
the unintentional removal of the udev scripts. There really should have
been no semantic change compared with the default behaviour from 4.5.0-rc1.

Putting back the udev rules (even a trimmed down version, whatever that
means) is just papering over the underlying issue, whatever that is. Only
once we have understood the underlying issue can we consider whether the
appropriate remedial action for 4.6 is to put udev back (i.e. if the real
fix is too intrusive etc)

I think the next thing to try should be to revert only the tools/libxl
portion of 2ba368d138934, i.e. return to the old toolstack code without
putting the udev scripts back (being careful to clear up any remnants of
the previous larger revert from the installed system). 

That should also be a change with no functional difference. So it will, I
think, help rule in/out any unintentional change in behaviour in (lib)xl as
opposed to some weird interaction with the inactive udev scripts.

Ian.

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

Re: [Xen-devel] Second regression due to libxl: Remove linux udev rules (2ba368d13893402b2f1fb3c283ddcc714659dd9b)

[Roger Pau Monné]

El 30/07/15 a les 10.17, Ian Campbell ha escrit:
[...]
> The udev scripts should have been unused since 4.5.0-rc1, where they were

udev scripts have been unused since 4.2 on Dom0, not 4.5. See:

b24dc64ef34437c958b40a71f510f404e0c4bbe4
57ad6afe2a08a03c40bcd336bfb27e008e1d3e53

Roger.

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

Re: [Xen-devel] [xen-unstable test] 60107: regressions - FAIL

[Dario Faggioli]

On Wed, 2015-07-29 at 21:01 +, osstest service owner wrote:
> flight 60107 xen-unstable real [real]
> http://logs.test-lab.xenproject.org/osstest/logs/60107/
> 
> Regressions :-(
> 
> Tests which did not succeed and are blocking,
> including tests which could not be run:
>  test-amd64-amd64-xl-multivcpu  6 xen-boot fail REGR. vs. 
> 59817
>
This is on merlot0... Can't that thing even boot on time now?!?! :-O

Looking at the serial log here:
http://logs.test-lab.xenproject.org/osstest/logs/60107/test-amd64-amd64-xl-multivcpu/serial-merlot0.log

The biggest 'skip' in the timestamps I've found is this:

  Jul 29 18:39:21.753156 Starting NTP server: ntpd.
  Jul 29 18:39:49.681201 Starting /usr/local/sbin/xenstored...

That did not happen during the previous (baremetal) boot:

  Jul 29 18:32:40.521077 Starting NTP server: ntpd.
  Jul 29 18:32:40.745125 Starting periodic command scheduler: cron.

It looks like, in general, such thing is slower under Xen, but by how
much, it varies quite a bit (also, some of the lines are mangled):

  Jul 29 07:06:46.497115 Starting NTP server: ntpdStarting 
/usr/local/sbin/xenstored...
  Jul 29 07:07:05.145127 Setting domain 0 name, domid and JSON config...

  Jul 29 08:13:28.549079 Starting NTP server: ntpd.
  Jul 29 08:13:31.917273 Starting /usr/local/sbin/xenstored...

  Jul 29 08:59:05.309143 Starting NTP server: ntpd.
  Jul 29 08:59:07.757168 Starting /usr/local/sbin/xenstored...

Infra/transient issues?

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


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

Re: [Xen-devel] Second regression due to libxl: Remove linux udev rules (2ba368d13893402b2f1fb3c283ddcc714659dd9b)

[Roger Pau Monné]

El 28/07/15 a les 21.47, Konrad Rzeszutek Wilk ha escrit:
> Hey,
> 
> I launch a bunch of guests at the same time or in parallel and 
> the scripts end up timing out with:
> 
> 
> Parsing config from //g-vm8.cfg
> WARNING: you seem to be using "kernel" directive to override HVM guest 
> firmware. Ignore that. Use "firmware_override" instead if you really want a 
> non-default firmware
> Jul 28 19:20:53 tst036 logger: /etc/xen/scripts/block: add 
> XENBUS_PATH=backend/vbd/13/5632
> libxl: error: libxl_aoutils.c:539:async_exec_timeout: killing execution of 
> /etc/xen/scripts/block add because of timeout
> libxl: error: libxl_create.c:1157:domcreate_launch_dm: unable to add disk 
> devices
> libxl: error: libxl_dm.c:1955:kill_device_model: unable to find device model 
> pid in /local/domain/13/image/device-model-pid
> libxl: error: libxl.c:1606:libxl__destroy_domid: libxl__destroy_device_model 
> failed for 13
> Jul 28 19:21:03 tst036 logger: /etc/xen/scripts/block: remove 
> XENBUS_PATH=backend/vbd/13/5632
> Jul 28 19:21:04 tst036 logger: /etc/xen/scripts/block: Writing 
> backend/vbd/13/5632/hotplug-error xenstore-read backend/vbd/13/5632/node 
> failed. backend/vbd/13/5632/hotplug-status error to xenstore.
> Jul 28 19:21:04 tst036 logger: /etc/xen/scripts/block: xenstore-read 
> backend/vbd/13/5632/node failed.
> Jul 28 19:21:05 tst036 logger: /etc/xen/scripts/block: Writing 
> backend/vbd/13/5632/hotplug-error /etc/xen/scripts/block failed; error 
> detected. backend/vbd/13/5632/hotplug-status error to xenstore.
> Jul 28 19:21:05 tst036 logger: /etc/xen/scripts/block: /etc/xen/scripts/block 
> failed; error detected.
> libxl: error: libxl_exec.c:118:libxl_report_child_exitstatus: 
> /etc/xen/scripts/block remove [10344] exited with error status 1
> libxl: error: libxl_device.c:1085:device_hotplug_child_death_cb: script: 
> /etc/xen/scripts/block failed; error detected.
> libxl: error: libxl.c:1569:libxl__destroy_domid: non-existant domain 13
> libxl: error: libxl.c:1527:domain_destroy_callback: unable to destroy guest 
> with domid 13
> libxl: error: libxl.c:1454:domain_destroy_cb: destruction of domain 13 failed
> 
> And I cannot start the guest.
> 
> While if I revert the mentioned commit everything works peachy.
> 
> What is interesting is that if I have the revert I can see that the
> 
> Jul 28 19:39:03 tst036 logger: /etc/xen/scripts/block: Writing 
> backend/vbd/14/5632/physical-device 7:d to xenstore.
> Jul 28 19:39:03 tst036 logger: /etc/xen/scripts/block: Writing 
> backend/vbd/14/5632/hotplug-status connected to xenstore.
> 
> or often done much much later after xl create has started.
> 
> Attached is the bad log and the good log.

Can you do the same test with xl -vvv and the following patch applied 
(with and without 2ba368 reverted):

diff --git a/tools/hotplug/Linux/block b/tools/hotplug/Linux/block
index 8d2ee9d..d6f1b58 100644
--- a/tools/hotplug/Linux/block
+++ b/tools/hotplug/Linux/block
@@ -283,13 +283,19 @@ mount it read-write in a guest domain."
 
   shared_list=$(losetup -a |
 sed -n -e 
"s@^\([^:]\+\)\(:[[:blank:]]\[0*${dev}\]:${inode}[[:blank:]](.*)\)@\1@p" )
+  echo "Starting sharing checks"
+  start=`date +%s`
   for dev in $shared_list
   do
 if [ -n "$dev" ]
 then
+  echo "Checking sharing of $file $dev $mode"
   check_file_sharing "$file" "$dev" "$mode"
 fi
   done
+  end=`date +%s`
+  runtime=$((end-start))
+  echo "Checks took: $runtime"
 fi
 
 loopdev=$(losetup -f 2>/dev/null || find_free_loopback_dev)

Roger. 


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

Re: [Xen-devel] Second regression due to libxl: Remove linux udev rules (2ba368d13893402b2f1fb3c283ddcc714659dd9b)

[Ian Campbell]

On Thu, 2015-07-30 at 10:43 +0200, Roger Pau Monné wrote:
> El 30/07/15 a les 10.17, Ian Campbell ha escrit:
> [...]
> > The udev scripts should have been unused since 4.5.0-rc1, where they 
> > were
> 
> udev scripts have been unused since 4.2 on Dom0, not 4.5. See:
> 
> b24dc64ef34437c958b40a71f510f404e0c4bbe4
> 57ad6afe2a08a03c40bcd336bfb27e008e1d3e53

My mistake.

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

Re: [Xen-devel] Arndale secondary CPU boot issue Was Re: [xen-unstable test] 60076: regressions - FAIL

[Ian Campbell]

On Wed, 2015-07-29 at 19:18 +0100, Julien Grall wrote:

As an aside from the issue you are seeing:

> The old implementation of spinlock is sending an event (via the assembly
> instruction SEV) to the other physical CPUs. This will wake up the
> others CPUs waiting on the assembly instruction WFE (Wait For Event).

Uh, I didn't notice this about the new implementation, sorry I should have
done.

IMHO we should investigate (probably with some urgency) inserting a WFE and
SEV pair into the lock/unlock paths, else power consumption will suck.

I think that probably means using something new to replace the cpu_relax()
calls in the spinlocks with a WFE on ARM (we don't just want to change
relax) and to add a arch specific hook for the SEV on the release path.

If it is too late for 4.6 (which would depend on the eventual complexity of
the actual fix) then we should fix this ASAP in 4.7 and backport for 4.6.1.

> It appears to be required on the Arndale to boot secondaries CPUs.
> Although, depending on where I put the sev I don't have the same 
> behavior:
>   - sev in smp_init callback: the CPU is not coming up
>   - sev before or after arch_cpu_up: the CPU is booting but not in 
> HYP
> mode [2]
> 
> I haven't yet figured out where the "sev" should be placed in order to
> get the CPU boot correctly.

Does the arndale end up using 
.cpu_up = cpu_up_send_sgi,
or
.cpu_up = exynos5_cpu_up,
?

> What I don't understand is how the placement of "sev" would affect the
> secondary processor to boot in HYP mode or Kernel mode or nothing at all
> 
> This platform seems very picky and I don't remember having a
> documentation about how the SMP boot works for this platform. Linux
> seems to avoid the SEV for this platform.

32-bit Linux has some in common code paths IIRC, which are not always
apparent at first glance.

Ian.

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

Re: [Xen-devel] [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction

[Ian Campbell]

On Wed, 2015-07-29 at 17:44 +0100, Andrew Cooper wrote:
> On 29/07/15 17:39, Ravi Sahita wrote:
> > As reported by Wei Lu on July 27 2015
> 
> Reported-by: Wei Liu 

Which leaves an effectively empty commit message:

x86/hvm.c: fix regression in guest destruction

Reported-by: Wei Liu 
Signed-off-by: Ravi Sahita 
Reviewed-by: Andrew Cooper 
Tested-by: Wei Liu 

If I were maintainer of this code I'd be very grumpy about the lack of a
commit log (What regression? What is the fix? Why is it correct?) for this
patch. I asked on IRC and Andy suggested instead:

x86/hvm.c: Don't tear down altp2m state if it was never set up

which I have applied with. I still don't think the commit message is very
satisfactory, but I'm not maintainer of any of this code so meh.

Ian.



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

Re: [Xen-devel] [PATCH for 4.6] x86/p2m.c: fix missed off-by-one in altp2m commit

[Ian Campbell]

On Wed, 2015-07-29 at 17:46 +0100, Andrew Cooper wrote:
> On 29/07/15 17:40, Ravi Sahita wrote:
> > Signed-off-by: Ravi Sahita 
> 
> Reviewed-by: Andrew Cooper 

Applied.



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

Re: [Xen-devel] [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction

[Ian Campbell]

On Thu, 2015-07-30 at 10:21 +0100, Ian Campbell wrote:

> which I have applied with. I still don't think the commit message is very
> satisfactory, but I'm not maintainer of any of this code so meh.

For the benefit of the archives perhaps someone could explain why gating a
per-vcpu teardown on a host level feature setting is correct?

In particular what ensures that altp2m_vcpu_initialise has been called,
given that this is only called from HVMOP_altp2m_set_domain_state. What
happens if that HVMOP is never touched?

Do things work both for altp2m disabled on the Xen command line and
disabled/enabled in the guest config? If so how?

Also how come HVMOP_altp2m_set_domain_state does not have a
hvm_altp2m_supported check?

Ian.

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

Re: [Xen-devel] [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction

[George Dunlap]

On 07/30/2015 10:32 AM, Ian Campbell wrote:
> On Thu, 2015-07-30 at 10:21 +0100, Ian Campbell wrote:
> 
>> which I have applied with. I still don't think the commit message is very
>> satisfactory, but I'm not maintainer of any of this code so meh.
> 
> For the benefit of the archives perhaps someone could explain why gating a
> per-vcpu teardown on a host level feature setting is correct?
> 
> In particular what ensures that altp2m_vcpu_initialise has been called,
> given that this is only called from HVMOP_altp2m_set_domain_state. What
> happens if that HVMOP is never touched?
> 
> Do things work both for altp2m disabled on the Xen command line and
> disabled/enabled in the guest config? If so how?
> 
> Also how come HVMOP_altp2m_set_domain_state does not have a
> hvm_altp2m_supported check?

So this was all acked & stuff before I had much of a chance to comment
on it, but on my to-do list for 4.7 is to rework a lot of the
initialization / teardown stuff.  In particular:

- Always and only check for whether something has been initialized
(e.g., non-NULL, non-INVALID_MFN) before tearing it down

- Do *all* of the initialization for both altp2m and nestedhvm when
they're actually enabled for the domain, rather than doing a bunch of
the initialization unconditionally up front.

This is all part of the "technical debt" we were talking about when we
considered giving it a freeze exception.

 -George


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

Re: [Xen-devel] [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction

[Ian Campbell]

On Thu, 2015-07-30 at 10:38 +0100, George Dunlap wrote:
> On 07/30/2015 10:32 AM, Ian Campbell wrote:
> > On Thu, 2015-07-30 at 10:21 +0100, Ian Campbell wrote:
> > 
> > > which I have applied with. I still don't think the commit message is 
> > > very
> > > satisfactory, but I'm not maintainer of any of this code so meh.
> > 
> > For the benefit of the archives perhaps someone could explain why 
> > gating a
> > per-vcpu teardown on a host level feature setting is correct?
> > 
> > In particular what ensures that altp2m_vcpu_initialise has been called,
> > given that this is only called from HVMOP_altp2m_set_domain_state. What
> > happens if that HVMOP is never touched?
> > 
> > Do things work both for altp2m disabled on the Xen command line and
> > disabled/enabled in the guest config? If so how?
> > 
> > Also how come HVMOP_altp2m_set_domain_state does not have a
> > hvm_altp2m_supported check?
> 
> So this was all acked & stuff before I had much of a chance to comment
> on it, but on my to-do list for 4.7 is to rework a lot of the
> initialization / teardown stuff.  In particular:
> 
> - Always and only check for whether something has been initialized
> (e.g., non-NULL, non-INVALID_MFN) before tearing it down
> 
> - Do *all* of the initialization for both altp2m and nestedhvm when
> they're actually enabled for the domain, rather than doing a bunch of
> the initialization unconditionally up front.
> 
> This is all part of the "technical debt" we were talking about when we
> considered giving it a freeze exception.

Thanks, I'm inferring that everything I asked about in the second from last
paragraph is somehow fine, just confusingly achieved in the current code...

I'm done grumping now...

Ian.

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

Re: [Xen-devel] PCI Pass-through in Xen ARM - Draft 2.

[Ian Campbell]

On Wed, 2015-07-29 at 15:07 +0530, Manish Jaggi wrote:
> 
> On Monday 06 July 2015 03:50 PM, Ian Campbell wrote:
> > On Mon, 2015-07-06 at 15:36 +0530, Manish Jaggi wrote:
> > > On Monday 06 July 2015 02:41 PM, Ian Campbell wrote:
> > > > On Sun, 2015-07-05 at 11:25 +0530, Manish Jaggi wrote:
> > > > > On Monday 29 June 2015 04:01 PM, Julien Grall wrote:
> > > > > > Hi Manish,
> > > > > > 
> > > > > > On 28/06/15 19:38, Manish Jaggi wrote:
> > > > > > > 4.1 Holes in guest memory space
> > > > > > > 
> > > > > > > Holes are added in the guest memory space for mapping pci 
> > > > > > > device's BAR
> > > > > > > regions.
> > > > > > > These are defined in arch-arm.h
> > > > > > > 
> > > > > > > /* For 32bit */
> > > > > > > GUEST_MMIO_HOLE0_BASE, GUEST_MMIO_HOLE0_SIZE
> > > > > > > 
> > > > > > > /* For 64bit */
> > > > > > > GUEST_MMIO_HOLE1_BASE , GUEST_MMIO_HOLE1_SIZE
> > > > > > The memory layout for 32bit and 64bit are exactly the same. Why 
> > > > > > do you
> > > > > > need to differ here?
> > > > > I think Ian has already replied. I will change the name of macro
> > > > > > > 4.2 New entries in xenstore for device BARs
> > > > > > > 
> > > > > > > toolkit also updates the xenstore information for the device
> > > > > > > (virtualbar:physical bar).
> > > > > > > This information is read by xenpciback and returned to the 
> > > > > > > pcifront
> > > > > > > driver configuration
> > > > > > > space accesses.
> > > > > > Can you details what do you plan to put in xenstore and how?
> > > > > It is implementation . But I plan to put under domU / device / 
> > > > > heirarchy
> > > > Actually, xenstore is an API of sorts which needs to be maintained 
> > > > going
> > > > forward (since front and backend can evolve separately, so it does 
> > > > need
> > > > some level of design and documentation.
> > > > 
> > > > > > What about the expansion ROM?
> > > > > Do you want to put some restriction on not using expansion ROM as 
> > > > > a
> > > > > passthrough device.
> > > > "expansion ROM as a passthrough device" doesn't make sense to me,
> > > > passthrough devices may _have_ an expansion ROM.
> > > > 
> > > > The expansion ROM is just another BAR. I don't know how 
> > > > pcifront/back
> > > > deal with those today on PV x86, but I see no reason for ARM to 
> > > > deviate.
> > > > 
> > > > 
> > > > > > > 4.3 Hypercall for bdf mapping notification to xen
> > > > > > > ---
> > > > > > > #define PHYSDEVOP_map_sbdf  43
> > > > > > > typedef struct {
> > > > > > >u32 s;
> > > > > > >u8 b;
> > > > > > >u8 df;
> > > > > > >u16 res;
> > > > > > > } sbdf_t;
> > > > > > > struct physdev_map_sbdf {
> > > > > > >int domain_id;
> > > > > > >sbdf_tsbdf;
> > > > > > >sbdf_tgsbdf;
> > > > > > > };
> > > > > > > 
> > > > > > > Each domain has a pdev list, which contains the list of all 
> > > > > > > pci devices.
> > > > > > > The
> > > > > > > pdev structure already has a sbdf information. The 
> > > > > > > arch_pci_dev is
> > > > > > > updated to
> > > > > > > contain the gsbdf information. (gs- guest segment id)
> > > > > > > 
> > > > > > > Whenever there is trap from guest or an interrupt has to be 
> > > > > > > injected,
> > > > > > > the pdev
> > > > > > > list is iterated to find the gsbdf.
> > > > > > Can you give more background for this section? i.e:
> > > > > > - Why do you need this?
> > > > > > - How xen will translate the gbdf to a vDeviceID?
> > > > > In the context of the hypercall processing.
> > > > > > - Who will call this hypercall?
> > > > > > - Why not setting the gsbdf when the device is 
> > > > > > assigned?
> > > > > Can the maintainer of the pciback suggest an alternate.
> > > > That's not me, but I don't think this belongs here, I think it can 
> > > > be
> > > > done from the toolstack. If you think not then please explain what
> > > > information the toolstack doesn't have in its possession which 
> > > > prevents
> > > > this mapping from being done there.
> > > The toolstack does not have the guest sbdf information. I could only
> > > find it in xenpciback.
> > Are you sure? The sbdf relates to the physical device, correct? If so
> > then surely the toolstack knows it -- it's written in the config file
> > and is the primary parameter to all of the related libxl passthrough
> > APIs. The toolstack wouldn't be able to do anything about passing
> > through a given device without knowing which device it should be 
> > passing
> > through.
> > 
> > Perhaps this info needs plumbing through to some new bit of the
> > toolstack, but it is surely available somewhere.
> > 
> > If you meant the virtual SBDF then that is in libxl_device_pci.vdevfn.
> I added prints in libxl__device_pci_add. vdevfn is always 0 so this may 
> not be the right variable to use.
> Can you please recheck.
> 
>

Re: [Xen-devel] [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction

[Andrew Cooper]

On 30/07/15 10:32, Ian Campbell wrote:
> Also how come HVMOP_altp2m_set_domain_state does not have a
> hvm_altp2m_supported check?

(For the benefit of not leaving this question unanswered)

There is an early exit at the top of do_altp2m_op(), ahead of even
reading the hypercall body from the guest.

~Andrew

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

Re: [Xen-devel] [PATCH for-4.6 0/2] Replace FSF street address with canonical URL

[Wei Liu]

These are only mechanical changes and I deem this fall into
"keep documentation up to date" category, so with my RM hat on.

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.6 0/2] Replace FSF street address with canonical URL

[Andrew Cooper]

On 29/07/15 11:35, Ian Campbell wrote:
> We currently have numerous variations on the FSF's addresses, old, new and
> plain incorrect (there was a buggy version in circulation at one time).
>
> The current recommendation in http://www.gnu.org/licenses/gpl-howto.en.html
>  is to use "If not, see ."
>
> I care about this because in the process of splitting up libxenctrl I found
> myself copying a lot of incorrect boiler plate to new files, which is just
> silly.
>
> Patch #2 is completely automated (see description in patch) but it is still
> a pain to keep regenerating. Wei, would you be willing to accept this into
> 4.6? It is heavily automated and mechanical and so I think it is low risk.
> On the flip side doing this during the freeze means I'm not constantly
> playing catchup with new files.
>
> The second patch is large (260K), which I suspect it may not make it to the
> list, hence what I will send here will be an abridged version. The full
> patch is at the URL below.
>
> Since this is mechanical I'm only CCing THE REST maintainers and Wei (RM),
> I think collecting all the acks from all the maintainers is unnecessary in
> this case.
>
> Ian.
>
>
> The following changes since commit 44313ab77f3e3c5b566ea4f23b0e32bfd5eafa29:
>
>   tools/libxl: Do not fire the stream callback multiple times (2015-07-28 
> 14:02:18 +0100)
>
> are available in the git repository at:
>
>   git://xenbits.xen.org/people/ianc/xen.git fsf-address-v1
>
> for you to fetch changes up to 443701ef0c7ff30872e27419cf4356fb6bdb4059:
>
>   Replace FSF street address with canonical URL (2015-07-29 11:14:07 +0100)

Looks good.  I don't think this warrants acks from every maintainer on
the list of touched files.  This is a mechanical change unrelated to the
code content of the files.

~Andrew

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

Re: [Xen-devel] HVM guest max memory allocation

[Wei Liu]

On Thu, Jul 30, 2015 at 06:21:06AM +, Hao, Xudong wrote:
> > -Original Message-
> > From: Wei Liu [mailto:wei.l...@citrix.com]
> > Sent: Friday, July 24, 2015 3:29 PM
> > To: Hao, Xudong
> > Cc: xen-devel@lists.xen.org; Wu, Feng; wei.l...@citrix.com
> > Subject: Re: [Xen-devel] HVM guest max memory allocation
> > 
> > On Fri, Jul 24, 2015 at 06:04:53AM +, Hao, Xudong wrote:
> > > Hi,
> > >
> > > When creating HVM guest(no balloon driver), what's the max memory we
> > > could set? We can get the current system free memory by "xl info", but
> > > when configure the free memory to a HVM guest, it fail to boot up.
> > > Does Xen allocate additional memory when do VM creating? How many the
> > > additional memory burning per HVM by Xen?
> > 
> > Historically libxl will add some slack on top of the memory you ask for.
> > 
> > The extra memory for HVM at the moment is 2 MB. That means if you ask for
> > 1024 MB for guest, libxl bumps that to 1026.
> > 
> 
> Wei, 
> 
> Thanks for your quick response, sorry I reply later as this mail missed by my 
> outlook... 
> OK, 2MB for tool stack per HVM.
> 
> > You also need to spare some memory for Xen itself to allocate internal
> > structures. I would say try to reduce the number for a few MBs.
> > 
> 
> Does different HVM configure get different memory consume of Xen itself? Can 
> we calculate an accurate boundary memory for HVM?

That would be hard I think, because when you have different features
enabled, Xen allocates different structures to keep track of guest
state.  

It is like a OS keeping track of the state of a process. It's hard to
tell in Linux how many kernel objects are allocated for a particular
process, no? It's also hard to tell how many tricks (which consumes
memory) Linux has done to make things run fast.

The closely thing I can think of is the debug key H and m. With those you
know how much actual memory is used. And, *if* Xen almost allocates all
structures up front you can have a relatively close approximation of
memory consumption.

George (CC'ed) might have some more ideas.

Wei.

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

Re: [Xen-devel] [PATCH for 4.6] x86/hvm.c: fix regression in guest destruction

[Ian Campbell]

On Thu, 2015-07-30 at 10:55 +0100, Andrew Cooper wrote:
> On 30/07/15 10:32, Ian Campbell wrote:
> > Also how come HVMOP_altp2m_set_domain_state does not have a
> > hvm_altp2m_supported check?
> 
> (For the benefit of not leaving this question unanswered)
> 
> There is an early exit at the top of do_altp2m_op(), ahead of even
> reading the hypercall body from the guest.

Thanks.

Ian.


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

Re: [Xen-devel] [BUG] Emulation issues

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monné [mailto:roger@citrix.com]
> Sent: 29 July 2015 14:54
> To: Paul Durrant; xen-devel; Andrew Cooper
> Subject: Re: [BUG] Emulation issues
> 
> El 29/07/15 a les 14.41, Paul Durrant ha escrit:
> >> -Original Message-
> >> From: Roger Pau Monné [mailto:roger@citrix.com]
> >> Sent: 29 July 2015 11:37
> >> To: Paul Durrant; xen-devel; Andrew Cooper
> >> Subject: Re: [BUG] Emulation issues
> >>
> >> El 29/07/15 a les 12.27, Paul Durrant ha escrit:
>  -Original Message-
>  From: Roger Pau Monné [mailto:roger@citrix.com]
>  Sent: 29 July 2015 11:17
>  To: xen-devel; Andrew Cooper; Paul Durrant
>  Subject: [BUG] Emulation issues
> 
>  Hello,
> 
>  While trying to debug a hotplug scripts issue, I came across what seems
>  to be an emulation bug inside of Xen. The result of this is a bunch of
>  repeated messages on the serial console:
> 
> >>>
> >>> Was there anything of interest before this? You got an 'unhandleable'
> >> emulation which generally should not happen, but I guess there may be a
> >> shutdown race in tearing down the ioreq server list and sending
> emulation
> >> requests which may cause hvm_send_ioreq() to return
> >> X86EMUL_UNHANDLEABLE. It would be good to better understand the
> >> sequence of events.
> >>
> >> I don't think there's anything relevant before the messages I've posted,
> >> here is a more complete log:
> >>
> >> (XEN) irq.c:386: Dom91 callback via changed to Direct Vector 0x93
> >> (XEN) irq.c:386: Dom92 callback via changed to Direct Vector 0x93
> >> (XEN) irq.c:276: Dom91 PCI link 0 changed 5 -> 0
> >> (XEN) irq.c:276: Dom91 PCI link 1 changed 10 -> 0
> >> (XEN) irq.c:276: Dom91 PCI link 2 changed 11 -> 0
> >> (XEN) irq.c:276: Dom91 PCI link 3 changed 5 -> 0
> >> (XEN) irq.c:276: Dom92 PCI link 0 changed 5 -> 0
> >> (XEN) irq.c:276: Dom92 PCI link 1 changed 10 -> 0
> >> (XEN) irq.c:276: Dom92 PCI link 2 changed 11 -> 0
> >> (XEN) irq.c:276: Dom92 PCI link 3 changed 5 -> 0
> >> INIT: Id "T0" respawning too fast: disabled for 5 minutes
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) io.c:165:d83v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >>
> >> If you can provide a debug/trace patch I can run the same workload with
> >> it in order to trace the sequence of events.
> >>
> >
> > Could you try this?
> >
> > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> > index 30acb78..1bc3cc9 100644
> > --- a/xen/arch/x86/hvm/emulate.c
> > +++ b/xen/arch/x86/hvm/emulate.c
> > @@ -145,6 +145,8 @@ static int hvmemul_do_io(
> >  return X86EMUL_UNHANDLEABLE;
> >  goto finish_access;
> >  default:
> > +gprintk(XENLOG_ERR, "weird emulation state %u\n",
> > +vio->io_req.state);
> >  return X86EMUL_UNHANDLEABLE;
> >  }
> >
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > index ec1d797..38d6d99 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -2747,6 +2747,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s,
> ioreq_t *pr
> >  }
> >  }
> >
> > +gprintk(XENLOG_ERR, "unable to contact device model\n");
> >  return X86EMUL_UNHANDLEABLE;
> >  }
> 
> I've applied your patch and the one from Andrew, so my current diff is:
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 30acb78..1bc3cc9 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -145,6 +145,8 @@ static int hvmemul_do_io(
>  return X86EMUL_UNHANDLEABLE;
>  goto finish_access;
>  default:
> +gprintk(XENLOG_ERR, "weird emulation state %u\n",
> +vio->io_req.state);
>  return X86EMUL_UNHANDLEABLE;
>  }
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ec1d797..38d6d99 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2747,6 +2747,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s,
> ioreq_t *proto_p,
>   

Re: [Xen-devel] [BUG] Emulation issues

[Roger Pau Monné]

El 30/07/15 a les 12.12, Paul Durrant ha escrit:
>> -Original Message-
>> From: Roger Pau Monné [mailto:roger@citrix.com]
>> Sent: 29 July 2015 14:54
>> To: Paul Durrant; xen-devel; Andrew Cooper
>> Subject: Re: [BUG] Emulation issues
>> I've applied your patch and the one from Andrew, so my current diff is:
>>
>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
>> index 30acb78..1bc3cc9 100644
>> --- a/xen/arch/x86/hvm/emulate.c
>> +++ b/xen/arch/x86/hvm/emulate.c
>> @@ -145,6 +145,8 @@ static int hvmemul_do_io(
>>  return X86EMUL_UNHANDLEABLE;
>>  goto finish_access;
>>  default:
>> +gprintk(XENLOG_ERR, "weird emulation state %u\n",
>> +vio->io_req.state);
>>  return X86EMUL_UNHANDLEABLE;
>>  }
>>
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index ec1d797..38d6d99 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -2747,6 +2747,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s,
>> ioreq_t *proto_p,
>>  }
>>  }
>>
>> +gprintk(XENLOG_ERR, "unable to contact device model\n");
>>  return X86EMUL_UNHANDLEABLE;
>>  }
>>
>> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
>> index d3b9cae..12d50c2 100644
>> --- a/xen/arch/x86/hvm/io.c
>> +++ b/xen/arch/x86/hvm/io.c
>> @@ -163,7 +163,9 @@ int handle_pio(uint16_t port, unsigned int size, int dir)
>>  break;
>>  default:
>>  gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
>> -domain_crash(curr->domain);
>> +show_execution_state(&curr->arch.user_regs);
>> +dump_execution_state();
>> +domain_crash_synchronous();
>>  break;
>>  }
>>
>> And got the following panic while doing a `xl shutdown -w -a` of 20 HVM
>> guests:
>>
>> (XEN) irq.c:386: Dom19 callback via changed to Direct Vector 0x93
>> (XEN) irq.c:276: Dom19 PCI link 0 changed 5 -> 0
>> (XEN) irq.c:276: Dom19 PCI link 1 changed 10 -> 0
>> (XEN) irq.c:276: Dom19 PCI link 2 changed 11 -> 0
>> (XEN) irq.c:276: Dom19 PCI link 3 changed 5 -> 0
>> (XEN) d10v0 weird emulation state 1
>> (XEN) io.c:165:d10v0 Weird HVM ioemulation status 1.
>> (XEN) Assertion 'diff < STACK_SIZE' failed at traps.c:91
>> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Tainted:C ]
>> (XEN) CPU:0
>> (XEN) RIP:e008:[] show_registers+0x60/0x32f
>> (XEN) RFLAGS: 00010212   CONTEXT: hypervisor (d10v0)
>> (XEN) rax: 1348fc88   rbx: 8300cc668290   rcx: 
>> (XEN) rdx: 8300dfaf   rsi: 8300cc668358   rdi: 8300dfaf7bb8
>> (XEN) rbp: 8300dfaf7bd8   rsp: 8300dfaf7a98   r8:  83019d27
>> (XEN) r9:  0004   r10: 0004   r11: 0001
>> (XEN) r12: 8300cc668000   r13:    r14: 82c00026c000
>> (XEN) r15: 830198bf9000   cr0: 8005003b   cr4: 26e0
>> (XEN) cr3: cc77b000   cr2: 880002762df8
>> (XEN) ds:    es:    fs:    gs:    ss: e010   cs: e008
>> (XEN) Xen stack trace from rsp=8300dfaf7a98:
>> (XEN)8300dfaf7ac8 82d080144b11 0046
>> 8300dfaf7ac8
>> (XEN)0046 0092 8300dfaf7ae0
>> 82d08012cfd3
>> (XEN)82d0802a1bc0 8300dfaf7af8 0046
>> 2001
>> (XEN)2001 f80002089e28 0001
>> fe3829c0
>> (XEN)b004  0014
>> 0002
>> (XEN)b004 2001 b005
>> b004
>> (XEN)2001 b004
>> beefbeef<0>d15v0 weird emulation state 1
>> (XEN)  8036fa45<0>io.c:165:d15v0 Weird HVM ioemulation status
>> 1.
>> (XEN)
>> (XEN)   Assertion 'diff < STACK_SIZE' failed at traps.c:91
>> (XEN)  00bfbeef[ Xen-4.6-unstable  x86_64  debug=y  Tainted:
>> C ]
>> (XEN)  0046CPU:6
>> (XEN)  fe3829c0RIP:e008:[] beef
>> show_registers+0x60/0x32f
>> (XEN)
>> (XEN) RFLAGS: 00010212CONTEXT: hypervisor
>>  (d15v0) 
>> (XEN) rax: 000121dd3c88   rbx: 83007b4c4290   rcx: 
>> (XEN)  rdx: 83019d29   rsi: 83007b4c4358   rdi:
>> 83019d297bb8
>> (XEN)
>> (XEN)   rbp: 83019d297bd8   rsp: 83019d297a98   r8:  83019d27
>> (XEN)  8300cc668290r9:  0001   r10: 0001   r11:
>> 0001
>> (XEN)  8300cc668000r12: 83007b4c4000   r13:    r14:
>> 82c000299000
>> (XEN)  r15: 830198bf9000   cr0: 8005003b   cr4:
>> 26e0
>> (XEN)  82c00026c000cr3: 7b5d7000   cr2: 8800026b14d8
>> (XEN)
>> (XEN)   ds: 002b   es: 002b   fs:    gs:    ss: e010   cs: e008
>> (XEN)  8300dfaf7bf8Xen stack trace from rsp=83019d297a

Re: [Xen-devel] [BUG] Emulation issues

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monné [mailto:roger@citrix.com]
> Sent: 30 July 2015 11:17
> To: Paul Durrant; xen-devel; Andrew Cooper
> Subject: Re: [BUG] Emulation issues
> 
> El 30/07/15 a les 12.12, Paul Durrant ha escrit:
> >> -Original Message-
> >> From: Roger Pau Monné [mailto:roger@citrix.com]
> >> Sent: 29 July 2015 14:54
> >> To: Paul Durrant; xen-devel; Andrew Cooper
> >> Subject: Re: [BUG] Emulation issues
> >> I've applied your patch and the one from Andrew, so my current diff is:
> >>
> >> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> >> index 30acb78..1bc3cc9 100644
> >> --- a/xen/arch/x86/hvm/emulate.c
> >> +++ b/xen/arch/x86/hvm/emulate.c
> >> @@ -145,6 +145,8 @@ static int hvmemul_do_io(
> >>  return X86EMUL_UNHANDLEABLE;
> >>  goto finish_access;
> >>  default:
> >> +gprintk(XENLOG_ERR, "weird emulation state %u\n",
> >> +vio->io_req.state);
> >>  return X86EMUL_UNHANDLEABLE;
> >>  }
> >>
> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >> index ec1d797..38d6d99 100644
> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -2747,6 +2747,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server
> *s,
> >> ioreq_t *proto_p,
> >>  }
> >>  }
> >>
> >> +gprintk(XENLOG_ERR, "unable to contact device model\n");
> >>  return X86EMUL_UNHANDLEABLE;
> >>  }
> >>
> >> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> >> index d3b9cae..12d50c2 100644
> >> --- a/xen/arch/x86/hvm/io.c
> >> +++ b/xen/arch/x86/hvm/io.c
> >> @@ -163,7 +163,9 @@ int handle_pio(uint16_t port, unsigned int size, int
> dir)
> >>  break;
> >>  default:
> >>  gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
> >> -domain_crash(curr->domain);
> >> +show_execution_state(&curr->arch.user_regs);
> >> +dump_execution_state();
> >> +domain_crash_synchronous();
> >>  break;
> >>  }
> >>
> >> And got the following panic while doing a `xl shutdown -w -a` of 20 HVM
> >> guests:
> >>
> >> (XEN) irq.c:386: Dom19 callback via changed to Direct Vector 0x93
> >> (XEN) irq.c:276: Dom19 PCI link 0 changed 5 -> 0
> >> (XEN) irq.c:276: Dom19 PCI link 1 changed 10 -> 0
> >> (XEN) irq.c:276: Dom19 PCI link 2 changed 11 -> 0
> >> (XEN) irq.c:276: Dom19 PCI link 3 changed 5 -> 0
> >> (XEN) d10v0 weird emulation state 1
> >> (XEN) io.c:165:d10v0 Weird HVM ioemulation status 1.
> >> (XEN) Assertion 'diff < STACK_SIZE' failed at traps.c:91
> >> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Tainted:C ]
> >> (XEN) CPU:0
> >> (XEN) RIP:e008:[] show_registers+0x60/0x32f
> >> (XEN) RFLAGS: 00010212   CONTEXT: hypervisor (d10v0)
> >> (XEN) rax: 1348fc88   rbx: 8300cc668290   rcx:
> 
> >> (XEN) rdx: 8300dfaf   rsi: 8300cc668358   rdi: 8300dfaf7bb8
> >> (XEN) rbp: 8300dfaf7bd8   rsp: 8300dfaf7a98   r8:  83019d27
> >> (XEN) r9:  0004   r10: 0004   r11:
> 0001
> >> (XEN) r12: 8300cc668000   r13:    r14: 82c00026c000
> >> (XEN) r15: 830198bf9000   cr0: 8005003b   cr4:
> 26e0
> >> (XEN) cr3: cc77b000   cr2: 880002762df8
> >> (XEN) ds:    es:    fs:    gs:    ss: e010   cs: e008
> >> (XEN) Xen stack trace from rsp=8300dfaf7a98:
> >> (XEN)8300dfaf7ac8 82d080144b11 0046
> >> 8300dfaf7ac8
> >> (XEN)0046 0092 8300dfaf7ae0
> >> 82d08012cfd3
> >> (XEN)82d0802a1bc0 8300dfaf7af8 0046
> >> 2001
> >> (XEN)2001 f80002089e28 0001
> >> fe3829c0
> >> (XEN)b004  0014
> >> 0002
> >> (XEN)b004 2001 b005
> >> b004
> >> (XEN)2001 b004
> >> beefbeef<0>d15v0 weird emulation state 1
> >> (XEN)  8036fa45<0>io.c:165:d15v0 Weird HVM ioemulation
> status
> >> 1.
> >> (XEN)
> >> (XEN)   Assertion 'diff < STACK_SIZE' failed at traps.c:91
> >> (XEN)  00bfbeef[ Xen-4.6-unstable  x86_64  debug=y  Tainted:
> >> C ]
> >> (XEN)  0046CPU:6
> >> (XEN)  fe3829c0RIP:e008:[]
> beef
> >> show_registers+0x60/0x32f
> >> (XEN)
> >> (XEN) RFLAGS: 00010212CONTEXT:
> hypervisor
> >>  (d15v0) 
> >> (XEN) rax: 000121dd3c88   rbx: 83007b4c4290   rcx:
> 
> >> (XEN)  rdx: 83019d29   rsi: 83007b4c4358   rdi:
> >> 83019d297bb8
> >> (XEN)
> >> (XEN)   rbp: 83019d297bd8   rsp: 83019d297a98   r8:
> 83019d27
> >> (XEN)  8300cc668290r9:  0001   r10: 0001
> r11:
> >> 0

Re: [Xen-devel] [PATCH v2] build: use correct qemu path in systemd service file and init script

[Ian Campbell]

On Thu, 2015-07-30 at 14:51 +0800, Ting-Wei Lan wrote:
> When --with-system-qemu is used, it is possible that we cannot find
> qemu-system-i386 in LIBEXEC_BIN, which can cause error in xencommons
> init script and xen-qemu-dom0-disk-backend.service systemd service.
> 
> Signed-off-by: Ting-Wei Lan 

Personally I would have omitted the distinction between @qemu_xen_path@ and
@qemu_xen_systemd@ and just put the env invocation in the service file as
"/usr/bin/env @qemu_xen_path@" but I suppose that is just bike shedding,
so:

Acked-by: Ian Campbell 

Wei Lui, what do you think about this for 4.6? It fixes a real issue where 
--with-system-qemu is used without an explicit path, which is supposed to
search for "qemu" in $PATH but fails to do so for the initscripts and unit
files, where it uses the old hardcoded default value instead, which
probably doesn't exist if you are using this option (and if it did isn't
the thing the user asked for).

The fix looks pretty straight forward to me.

Mostly unrelated, is "qemu" a sensible default here? No binary package on
Debian actually provides a "qemu" binary, they are all qemu-system-foo or
variants. I'm not sure if that's just a Debian packaging issue though. I've
added the Qemu-xen maintainers for input...

Ian.


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

Re: [Xen-devel] [BUG] Emulation issues

[Andrew Cooper]

On 30/07/15 11:24, Andrew Cooper wrote:
> On 30/07/15 11:16, Roger Pau Monné wrote:
>> El 30/07/15 a les 12.12, Paul Durrant ha escrit:
 -Original Message-
 From: Roger Pau Monné [mailto:roger@citrix.com]
 Sent: 29 July 2015 14:54
 To: Paul Durrant; xen-devel; Andrew Cooper
 Subject: Re: [BUG] Emulation issues
 I've applied your patch and the one from Andrew, so my current diff is:

 diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
 index 30acb78..1bc3cc9 100644
 --- a/xen/arch/x86/hvm/emulate.c
 +++ b/xen/arch/x86/hvm/emulate.c
 @@ -145,6 +145,8 @@ static int hvmemul_do_io(
  return X86EMUL_UNHANDLEABLE;
  goto finish_access;
  default:
 +gprintk(XENLOG_ERR, "weird emulation state %u\n",
 +vio->io_req.state);
  return X86EMUL_UNHANDLEABLE;
  }

 diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
 index ec1d797..38d6d99 100644
 --- a/xen/arch/x86/hvm/hvm.c
 +++ b/xen/arch/x86/hvm/hvm.c
 @@ -2747,6 +2747,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s,
 ioreq_t *proto_p,
  }
  }

 +gprintk(XENLOG_ERR, "unable to contact device model\n");
  return X86EMUL_UNHANDLEABLE;
  }

 diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
 index d3b9cae..12d50c2 100644
 --- a/xen/arch/x86/hvm/io.c
 +++ b/xen/arch/x86/hvm/io.c
 @@ -163,7 +163,9 @@ int handle_pio(uint16_t port, unsigned int size, int 
 dir)
  break;
  default:
  gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
 -domain_crash(curr->domain);
 +show_execution_state(&curr->arch.user_regs);
 +dump_execution_state();
 +domain_crash_synchronous();
  break;
  }

 And got the following panic while doing a `xl shutdown -w -a` of 20 HVM
 guests:

 (XEN) irq.c:386: Dom19 callback via changed to Direct Vector 0x93
 (XEN) irq.c:276: Dom19 PCI link 0 changed 5 -> 0
 (XEN) irq.c:276: Dom19 PCI link 1 changed 10 -> 0
 (XEN) irq.c:276: Dom19 PCI link 2 changed 11 -> 0
 (XEN) irq.c:276: Dom19 PCI link 3 changed 5 -> 0
 (XEN) d10v0 weird emulation state 1
 (XEN) io.c:165:d10v0 Weird HVM ioemulation status 1.
 (XEN) Assertion 'diff < STACK_SIZE' failed at traps.c:91
 (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Tainted:C ]
 (XEN) CPU:0
 (XEN) RIP:e008:[] show_registers+0x60/0x32f
 (XEN) RFLAGS: 00010212   CONTEXT: hypervisor (d10v0)
 (XEN) rax: 1348fc88   rbx: 8300cc668290   rcx: 
 (XEN) rdx: 8300dfaf   rsi: 8300cc668358   rdi: 8300dfaf7bb8
 (XEN) rbp: 8300dfaf7bd8   rsp: 8300dfaf7a98   r8:  83019d27
 (XEN) r9:  0004   r10: 0004   r11: 0001
 (XEN) r12: 8300cc668000   r13:    r14: 82c00026c000
 (XEN) r15: 830198bf9000   cr0: 8005003b   cr4: 26e0
 (XEN) cr3: cc77b000   cr2: 880002762df8
 (XEN) ds:    es:    fs:    gs:    ss: e010   cs: e008
 (XEN) Xen stack trace from rsp=8300dfaf7a98:
 (XEN)8300dfaf7ac8 82d080144b11 0046
 8300dfaf7ac8
 (XEN)0046 0092 8300dfaf7ae0
 82d08012cfd3
 (XEN)82d0802a1bc0 8300dfaf7af8 0046
 2001
 (XEN)2001 f80002089e28 0001
 fe3829c0
 (XEN)b004  0014
 0002
 (XEN)b004 2001 b005
 b004
 (XEN)2001 b004
 beefbeef<0>d15v0 weird emulation state 1
 (XEN)  8036fa45<0>io.c:165:d15v0 Weird HVM ioemulation status
 1.
 (XEN)
 (XEN)   Assertion 'diff < STACK_SIZE' failed at traps.c:91
 (XEN)  00bfbeef[ Xen-4.6-unstable  x86_64  debug=y  Tainted:
 C ]
 (XEN)  0046CPU:6
 (XEN)  fe3829c0RIP:e008:[] beef
 show_registers+0x60/0x32f
 (XEN)
 (XEN) RFLAGS: 00010212CONTEXT: hypervisor
  (d15v0) 
 (XEN) rax: 000121dd3c88   rbx: 83007b4c4290   rcx: 
 (XEN)  rdx: 83019d29   rsi: 83007b4c4358   rdi:
 83019d297bb8
 (XEN)
 (XEN)   rbp: 83019d297bd8   rsp: 83019d297a98   r8:  
 83019d27
 (XEN)  8300cc668290r9:  0001   r10: 0001   r11:
 0001
 (XEN)  8300cc668000r12: 83007b4c4000   r13:    r14:
 82c000299000
 (X

Re: [Xen-devel] [BUG] Emulation issues

[Andrew Cooper]

On 30/07/15 11:16, Roger Pau Monné wrote:
> El 30/07/15 a les 12.12, Paul Durrant ha escrit:
>>> -Original Message-
>>> From: Roger Pau Monné [mailto:roger@citrix.com]
>>> Sent: 29 July 2015 14:54
>>> To: Paul Durrant; xen-devel; Andrew Cooper
>>> Subject: Re: [BUG] Emulation issues
>>> I've applied your patch and the one from Andrew, so my current diff is:
>>>
>>> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
>>> index 30acb78..1bc3cc9 100644
>>> --- a/xen/arch/x86/hvm/emulate.c
>>> +++ b/xen/arch/x86/hvm/emulate.c
>>> @@ -145,6 +145,8 @@ static int hvmemul_do_io(
>>>  return X86EMUL_UNHANDLEABLE;
>>>  goto finish_access;
>>>  default:
>>> +gprintk(XENLOG_ERR, "weird emulation state %u\n",
>>> +vio->io_req.state);
>>>  return X86EMUL_UNHANDLEABLE;
>>>  }
>>>
>>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>>> index ec1d797..38d6d99 100644
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -2747,6 +2747,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s,
>>> ioreq_t *proto_p,
>>>  }
>>>  }
>>>
>>> +gprintk(XENLOG_ERR, "unable to contact device model\n");
>>>  return X86EMUL_UNHANDLEABLE;
>>>  }
>>>
>>> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
>>> index d3b9cae..12d50c2 100644
>>> --- a/xen/arch/x86/hvm/io.c
>>> +++ b/xen/arch/x86/hvm/io.c
>>> @@ -163,7 +163,9 @@ int handle_pio(uint16_t port, unsigned int size, int 
>>> dir)
>>>  break;
>>>  default:
>>>  gdprintk(XENLOG_ERR, "Weird HVM ioemulation status %d.\n", rc);
>>> -domain_crash(curr->domain);
>>> +show_execution_state(&curr->arch.user_regs);
>>> +dump_execution_state();
>>> +domain_crash_synchronous();
>>>  break;
>>>  }
>>>
>>> And got the following panic while doing a `xl shutdown -w -a` of 20 HVM
>>> guests:
>>>
>>> (XEN) irq.c:386: Dom19 callback via changed to Direct Vector 0x93
>>> (XEN) irq.c:276: Dom19 PCI link 0 changed 5 -> 0
>>> (XEN) irq.c:276: Dom19 PCI link 1 changed 10 -> 0
>>> (XEN) irq.c:276: Dom19 PCI link 2 changed 11 -> 0
>>> (XEN) irq.c:276: Dom19 PCI link 3 changed 5 -> 0
>>> (XEN) d10v0 weird emulation state 1
>>> (XEN) io.c:165:d10v0 Weird HVM ioemulation status 1.
>>> (XEN) Assertion 'diff < STACK_SIZE' failed at traps.c:91
>>> (XEN) [ Xen-4.6-unstable  x86_64  debug=y  Tainted:C ]
>>> (XEN) CPU:0
>>> (XEN) RIP:e008:[] show_registers+0x60/0x32f
>>> (XEN) RFLAGS: 00010212   CONTEXT: hypervisor (d10v0)
>>> (XEN) rax: 1348fc88   rbx: 8300cc668290   rcx: 
>>> (XEN) rdx: 8300dfaf   rsi: 8300cc668358   rdi: 8300dfaf7bb8
>>> (XEN) rbp: 8300dfaf7bd8   rsp: 8300dfaf7a98   r8:  83019d27
>>> (XEN) r9:  0004   r10: 0004   r11: 0001
>>> (XEN) r12: 8300cc668000   r13:    r14: 82c00026c000
>>> (XEN) r15: 830198bf9000   cr0: 8005003b   cr4: 26e0
>>> (XEN) cr3: cc77b000   cr2: 880002762df8
>>> (XEN) ds:    es:    fs:    gs:    ss: e010   cs: e008
>>> (XEN) Xen stack trace from rsp=8300dfaf7a98:
>>> (XEN)8300dfaf7ac8 82d080144b11 0046
>>> 8300dfaf7ac8
>>> (XEN)0046 0092 8300dfaf7ae0
>>> 82d08012cfd3
>>> (XEN)82d0802a1bc0 8300dfaf7af8 0046
>>> 2001
>>> (XEN)2001 f80002089e28 0001
>>> fe3829c0
>>> (XEN)b004  0014
>>> 0002
>>> (XEN)b004 2001 b005
>>> b004
>>> (XEN)2001 b004
>>> beefbeef<0>d15v0 weird emulation state 1
>>> (XEN)  8036fa45<0>io.c:165:d15v0 Weird HVM ioemulation status
>>> 1.
>>> (XEN)
>>> (XEN)   Assertion 'diff < STACK_SIZE' failed at traps.c:91
>>> (XEN)  00bfbeef[ Xen-4.6-unstable  x86_64  debug=y  Tainted:
>>> C ]
>>> (XEN)  0046CPU:6
>>> (XEN)  fe3829c0RIP:e008:[] beef
>>> show_registers+0x60/0x32f
>>> (XEN)
>>> (XEN) RFLAGS: 00010212CONTEXT: hypervisor
>>>  (d15v0) 
>>> (XEN) rax: 000121dd3c88   rbx: 83007b4c4290   rcx: 
>>> (XEN)  rdx: 83019d29   rsi: 83007b4c4358   rdi:
>>> 83019d297bb8
>>> (XEN)
>>> (XEN)   rbp: 83019d297bd8   rsp: 83019d297a98   r8:  
>>> 83019d27
>>> (XEN)  8300cc668290r9:  0001   r10: 0001   r11:
>>> 0001
>>> (XEN)  8300cc668000r12: 83007b4c4000   r13:    r14:
>>> 82c000299000
>>> (XEN)  r15: 830198bf9000   cr0: 8005003b   cr4:
>>> 26e0
>>> (XEN)  82c00026c000cr3: 7b5d7000   cr2: ff

[Xen-devel] Vote for Xen talks at OpenStack summit (deadline today)

[Lars Kurth]

So far, I am aware of one talk. Note that the OpenStack summit agenda's are 
determined by popular vote (which ends today).

If there are others Xen related talks, feel free to add by replying to this mail
https://www.openstack.org/summit/tokyo-2015/vote-for-speakers/Presentation/6150 


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

Re: [Xen-devel] [PATCH v2] build: use correct qemu path in systemd service file and init script

[Wei Liu]

On Thu, Jul 30, 2015 at 11:24:47AM +0100, Ian Campbell wrote:
> On Thu, 2015-07-30 at 14:51 +0800, Ting-Wei Lan wrote:
> > When --with-system-qemu is used, it is possible that we cannot find
> > qemu-system-i386 in LIBEXEC_BIN, which can cause error in xencommons
> > init script and xen-qemu-dom0-disk-backend.service systemd service.
> > 
> > Signed-off-by: Ting-Wei Lan 
> 
> Personally I would have omitted the distinction between @qemu_xen_path@ and
> @qemu_xen_systemd@ and just put the env invocation in the service file as
> "/usr/bin/env @qemu_xen_path@" but I suppose that is just bike shedding,
> so:
> 
> Acked-by: Ian Campbell 
> 
> Wei Lui, what do you think about this for 4.6? It fixes a real issue where 
> --with-system-qemu is used without an explicit path, which is supposed to
> search for "qemu" in $PATH but fails to do so for the initscripts and unit
> files, where it uses the old hardcoded default value instead, which
> probably doesn't exist if you are using this option (and if it did isn't
> the thing the user asked for).
> 
> The fix looks pretty straight forward to me.
> 

I agree with you. It should be applied for 4.6.

> Mostly unrelated, is "qemu" a sensible default here? No binary package on
> Debian actually provides a "qemu" binary, they are all qemu-system-foo or
> variants. I'm not sure if that's just a Debian packaging issue though. I've
> added the Qemu-xen maintainers for input...
> 
> Ian.

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

Re: [Xen-devel] Arndale secondary CPU boot issue Was Re: [xen-unstable test] 60076: regressions - FAIL

[Andrew Cooper]

On 29/07/15 19:18, Julien Grall wrote:
> On 29/07/15 15:15, Julien Grall wrote:
 Can it be that things are "just" slow, since we're creating a 4 vcpus
 guest on a 1 pcpu (not so powerful, I guess) host?
>>> The arndale board has a 2 physical CPUs. Although it looks like that the
>>> secondary cpu is never coming up:
>>>
>>> Jul 28 01:35:39.057076 (XEN) Adding cpu 1 to runqueue 0
>>> Jul 28 01:35:39.057104 (XEN) Bringing up CPU1
>>> Jul 28 01:35:39.064998 (XEN) CPU1 never came online
>>> Jul 28 01:35:40.065133 (XEN) Removing cpu 1 from runqueue 0
>>> Jul 28 01:35:40.065176 (XEN) Failed to bring up CPU 1 (error -5)
>>>
>>> This has been broken at some point in Xen 4.6. Xen 4.5 is booting with
>>> the right number of physical on the Arndale.
> I figured out what's going on. The problem interestingly came after the
> commit which added the support of the ticket lock [1] in Xen.
>
> While the problem is solved by reverting this patch, the source of the
> issue is not because of a ticket lock issue with ARM (thanks god!).

As an aside, why is failing to bring up a cpu not fatal under ARM?

I admit that x86 isn't much better in this regard - it will spin in an
infinite loop waiting for the upcoming cpu to call in, but it least it
doesn't proceed booting with some cpus unexpectedly missing.

~Andrew

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

Re: [Xen-devel] Arndale secondary CPU boot issue Was Re: [xen-unstable test] 60076: regressions - FAIL

[Stefano Stabellini]

On Thu, 30 Jul 2015, Ian Campbell wrote:
> On Wed, 2015-07-29 at 19:18 +0100, Julien Grall wrote:
> 
> As an aside from the issue you are seeing:
> 
> > The old implementation of spinlock is sending an event (via the assembly
> > instruction SEV) to the other physical CPUs. This will wake up the
> > others CPUs waiting on the assembly instruction WFE (Wait For Event).
> 
> Uh, I didn't notice this about the new implementation, sorry I should have
> done.
> 
> IMHO we should investigate (probably with some urgency) inserting a WFE and
> SEV pair into the lock/unlock paths, else power consumption will suck.
> 
> I think that probably means using something new to replace the cpu_relax()
> calls in the spinlocks with a WFE on ARM (we don't just want to change
> relax) and to add a arch specific hook for the SEV on the release path.

I agree: adding a WFE in cpu_relax() is too risky at this point.


> If it is too late for 4.6 (which would depend on the eventual complexity of
> the actual fix) then we should fix this ASAP in 4.7 and backport for 4.6.1.

I don't think we can release 4.6 without a WFE in the locks. We might
want to consider reverting to spin_locks on ARM (although I am aware
that the code is common at the moment).

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

Re: [Xen-devel] [Xen-Devel] Enabling IRQ Crossbar (Secondary Interrupt Controller) Support

[Ian Campbell]

On Wed, 2015-07-29 at 16:36 -0400, Brandon Perez wrote:
> On 07/21/2015 11:12 AM, Ian Campbell wrote:
> > On Tue, 2015-07-21 at 10:07 -0400, Brandon Perez wrote:
> > > 
> > >   I'm not sure that these patches are quite ready yet to be put
> > > into
> > > the Xen repo.
> > 
> > That's ok, but even for an RFC (Request For Comments) please post them
> > one patch per email in the manner of git send-email. You can use -
> > -subject-prefix='PATCH RFC' to tag them as such.
> > 
> > Thanks,
> > Ian.
> > 
> 
> All,
> 
>  Apologies for the delay, I've been pretty busy the last week. Also, 
> sorry for the confusion with sending the patches, this is my first time 
> working with an open-source dev group.
> 
>  I've sent the patches, one per email, via git.

Thanks. For next time please can you send a batch of related patches all
with the same git send-email invocation such that they arrive as a set of
threaded mails (i.e. with a References header on the previous). That keeps
the related patches together in peoples in box, and also labels them 1/N,
2/N etc so we know which order they are applicable in.

Invoking send-email with --dry-run will print out all the headers it would
send, so you can check it will do as expected.

http://wiki.xen.org/wiki/Submitting_Xen_Patches#Git_send-email talks about
this stuff a bit.

Thanks,

Ian.

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

Re: [Xen-devel] [BUG] Emulation issues

[Paul Durrant]

> -Original Message-
[big snip]
> Sorry, missed that in the noise. So, the problem is that there is no 
> in-flight I/O
> even though pio completion is being attempted. Something has got out of
> sync.
> 

I think I understand what may be happening... The code in hvmemul_do_io() 
basically expects to be called either to issue an I/O or to extract info from a 
completed one. However it is being called unconditionally (in the PIO case) out 
of hvm_do_resume, rather than only if the in-flight I/O state has been updated 
to STATE_IORESP_READY.

Can you try this patch (also containing my previous debug patch)?

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 30acb78..1bc3cc9 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -145,6 +145,8 @@ static int hvmemul_do_io(
 return X86EMUL_UNHANDLEABLE;
 goto finish_access;
 default:
+gprintk(XENLOG_ERR, "weird emulation state %u\n",
+vio->io_req.state);
 return X86EMUL_UNHANDLEABLE;
 }

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ec1d797..a476271 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -472,7 +472,6 @@ void hvm_do_resume(struct vcpu *v)
 struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io;
 struct domain *d = v->domain;
 struct hvm_ioreq_server *s;
-enum hvm_io_completion io_completion;

 check_wakeup_from_wait();

@@ -499,33 +498,38 @@ void hvm_do_resume(struct vcpu *v)
 }
 }

-io_completion = vio->io_completion;
-vio->io_completion = HVMIO_no_completion;
-
-switch ( io_completion )
-{
-case HVMIO_no_completion:
-break;
-case HVMIO_mmio_completion:
-handle_mmio();
-break;
-case HVMIO_pio_completion:
-(void)handle_pio(vio->io_req.addr, vio->io_req.size,
- vio->io_req.dir);
-break;
-case HVMIO_realmode_completion:
+if ( vio->io_req.state == STATE_IORESP_READY )
 {
-struct hvm_emulate_ctxt ctxt;
+enum hvm_io_completion io_completion;

-hvm_emulate_prepare(&ctxt, guest_cpu_user_regs());
-vmx_realmode_emulate_one(&ctxt);
-hvm_emulate_writeback(&ctxt);
+io_completion = vio->io_completion;
+vio->io_completion = HVMIO_no_completion;

-break;
-}
-default:
-ASSERT_UNREACHABLE();
-break;
+switch ( io_completion )
+{
+case HVMIO_no_completion:
+break;
+case HVMIO_mmio_completion:
+handle_mmio();
+break;
+case HVMIO_pio_completion:
+(void)handle_pio(vio->io_req.addr, vio->io_req.size,
+ vio->io_req.dir);
+break;
+case HVMIO_realmode_completion:
+{
+struct hvm_emulate_ctxt ctxt;
+
+hvm_emulate_prepare(&ctxt, guest_cpu_user_regs());
+vmx_realmode_emulate_one(&ctxt);
+hvm_emulate_writeback(&ctxt);
+
+break;
+}
+default:
+ASSERT_UNREACHABLE();
+break;
+}
 }

 if ( unlikely(d->arch.event_write_data) )
@@ -2747,6 +2751,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s, ioreq_t 
*proto_p,
 }
 }

+gprintk(XENLOG_ERR, "unable to contact device model\n");
 return X86EMUL_UNHANDLEABLE;
 }

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

Re: [Xen-devel] [PATCH] tools: Update libxl flex outputs to Flex from Jessie.

[Wei Liu]

On Wed, Jul 15, 2015 at 11:21:17AM +0100, Ian Campbell wrote:
> This is the result of
>   $ find -name \*.l -exec touch {} \;
>   $ find -name \*.y -exec touch {} \;
> 
> and then rebuilding.
> 
> This avoids churn on the machine I use for committing which has been
> updated, causing flex to go from 2.5.35-10.1 to 2.5.39-8.
> 
> Signed-off-by: Ian Campbell 

Acked-by: Wei Liu 

It has missed the boat so please wait for next version.

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

Re: [Xen-devel] [PATCH for-2.4 0/3] Migration regressions with Xen.

[Juan Quintela]

Anthony PERARD  wrote:
> This is a critical issue for Xen as migration either with the same version
> of QEMU, or from a previous version of QEMU is broken.
>
> Any suggestion on how to move forward?

Will send a pull requset tomorrow.

Thinking about creating a single function that is called for all needed
places, just to avoid this problem in the future.

Thanks, Juan.

>
> On Tue, Jul 28, 2015 at 04:54:42PM +0100, Anthony PERARD wrote:
>> Hi,
>> 
>> We've spotted several regression which prevent migration with Xen using the
>> same version of QEMU or from a previous version of QEMU (tryied with 2.2).
>> 
>> Regression have been introduce by at least:
>> - df4b1024526cae3479da3492d6371fd4a7324a03
>>   migration: create new section to store global state
>> - 61964c23e5ddd5a33f15699e45ce126f879e3e33
>>   migration: Add configuration section
>> 
>> The first patch would fix the default case when we use the machine 'xenfv'.
>> 
>> But we can also use the machine 'pc' and I don't know how to fix this case.
>> The second and third patches are a step in the direction of fixing
>> migration with the machine 'pc'.
>> 
>> I have not look at save/restore for PV guest yet.
>> 
>> Thanks,
>> 
>> Anthony PERARD (3):
>>   migration: Fix regretion for xenfv machine.
>>   migration: Fix global state with Xen.
>>   migration: Add configuration section to vmstate with xen.
>> 
>>  hw/i386/pc_piix.c | 3 +++
>>  include/migration/migration.h | 1 +
>>  migration/migration.c | 7 +++
>>  migration/savevm.c| 6 ++
>>  4 files changed, 17 insertions(+)

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

Re: [Xen-devel] [PATCH RFC] DRA7: Add specific mappings for devices/regions not in the device tree.

[Ian Campbell]

On Wed, 2015-07-29 at 16:53 -0400, Brandon Perez wrote:
> The DRA7 chip, which is similar to the OMAP5 chip, also requires specific
> mappings. These are MMIO mappings which are not explicitly stated in the 
> device
> tree, so Xen does not know to map them. This patch adds these regions required
> by the DRA7 to be mapped.

I suppose the chances of getting this fixed in the DTS is slim?

> Signed-off-by: Brandon Perez 
> ---
>  xen/arch/arm/platforms/omap5.c|   27 +++
>  xen/include/asm-arm/platforms/omap5.h |3 +++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/xen/arch/arm/platforms/omap5.c 
> b/xen/arch/arm/platforms/omap5.c
> index e7bf30d..3c6495a 100644
> --- a/xen/arch/arm/platforms/omap5.c
> +++ b/xen/arch/arm/platforms/omap5.c
> @@ -120,6 +120,32 @@ static int omap5_specific_mapping(struct domain *d)
>  return 0;
>  }
> 
> +/* Additional mappings for dom0 (not in the DTS) */
> +static int dra7_specific_mapping(struct domain *d)
> +{
> +/* Map the PRM module */
> +map_mmio_regions(d, paddr_to_pfn(OMAP5_PRM_BASE), 2,
> + paddr_to_pfn(OMAP5_PRM_BASE));
> +
> +/* Map the PRM_MPU */
> +map_mmio_regions(d, paddr_to_pfn(OMAP5_PRCM_MPU_BASE), 1,
> + paddr_to_pfn(OMAP5_PRCM_MPU_BASE));
> +
> +/* Map the Wakeup Gen */
> +map_mmio_regions(d, paddr_to_pfn(OMAP5_WKUPGEN_BASE), 1,
> + paddr_to_pfn(OMAP5_WKUPGEN_BASE));
> +
> +/* Map the on-chip SRAM */
> +map_mmio_regions(d, paddr_to_pfn(OMAP5_SRAM_PA), 32,
> + paddr_to_pfn(OMAP5_SRAM_PA));
> +
> +/* Map GPMC address space for NAND flash. */
> +map_mmio_regions(d, paddr_to_pfn(OMAP5_GPMC_PA), 65536,
> + paddr_to_pfn(OMAP5_GPMC_PA));

This is basically the same as the omap5 one, plus an extra GPMC mapping.

I'm unsure about the relationship between omap5 and the DRA7, is it the
case that there is some common core which both omap5 and dra7 derive from?
Or maybe dra7 derives from omap5?

If there is some commonality I'd prefer to see a common function, used from
both.

So depending on the "lineage" either omap5_common_specific_mapping, called
by both omap5_specific_mapping and dra7_specific_mapping (if they have a
common core) or dri7_specific_mapping just calling omap5_specific_mapping
(if dra7 is a derivative of omap5).

Of course if the GPMC is also present on omap5 then we should just add the
specific mapping there and call the existing function for dra7 too.

> +
> +return 0;
> +}
> +
>  static int __init omap5_smp_init(void)
>  {
>  void __iomem *wugen_base;
> @@ -171,6 +197,7 @@ PLATFORM_START(dra7, "TI DRA7")
>  .init_time = omap5_init_time,
>  .cpu_up = cpu_up_send_sgi,
>  .smp_init = omap5_smp_init,
> +.specific_mapping = dra7_specific_mapping,
> 
>  .dom0_gnttab_start = 0x4b00,
>  .dom0_gnttab_size = 0x2,
> diff --git a/xen/include/asm-arm/platforms/omap5.h b/xen/include/asm
> -arm/platforms/omap5.h
> index c559c84..d87e7d2 100644
> --- a/xen/include/asm-arm/platforms/omap5.h
> +++ b/xen/include/asm-arm/platforms/omap5.h
> @@ -20,6 +20,9 @@
>  #define OMAP_AUX_CORE_BOOT_0_OFFSET 0x800
>  #define OMAP_AUX_CORE_BOOT_1_OFFSET 0x804
> 
> +#define OMAP5_GPMC_PA   0x0100
> +#define OMAP5_TILER_PA  0x6000

TILER_PA is unused, did you miss it or is this #define unneeded?

I'd really like to see us moving away from xen/include/asm
-arm/platforms/*.h to adding the definitions locally to
xen/arch/arm/platforms/*.c, since the way things are since Xen 4.5 nothing
outside the .c file should need those definitions any longer.

I don't suppose you fancy tackling that for the omap case as a precursor to
this series do you?

> +
>  #endif /* __ASM_ARM_PLATFORMS_OMAP5_H */
> 
>  /*
> --
> 1.7.9.5
> 

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

Re: [Xen-devel] [PATCH RFC] Traps: Enable pass-through SMC call support for guest OS's.

[Ian Campbell]

On Wed, 2015-07-29 at 16:53 -0400, Brandon Perez wrote:
> Originally, Xen did not allow for guests to make SMC calls. However, on the 
> DRA7
> chips, the kernel needs to make several SMC calls to interact with the secure
> ROM code.
> 
> There are two solutions for solving this in the patch. The selected method is 
> to
> simply allow the kernel to make the call, without going through the 
> hypervisor.
> The other is to trap and emulate the call in the hypervisor.

I think we shouldn't be letting even dom0 make unfettered calls to the
secure world.

I think that means we need to continue to trap but also to call a platform
specific hook which filters which calls are allowed through (looking at the
immediate as well as the register contents). This will also give us an
opportunity to translate anything which needs to be translated (e.g. from
IPA to PA perhaps).

A platform with no hook provided should default to deny.

The actually call to smc should be via smc.S, you will need to refactor
call_smc into call_smc0 and call_smc1, and you may want to add a
convenience wrapper in the platform code.

> 
> Signed-off-by: Brandon Perez 
> ---
>  xen/arch/arm/traps.c |   15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 258d4c5..9b9de7b 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -123,8 +123,9 @@ void __cpuinit init_traps(void)
>   CPTR_EL2);
> 
>  /* Setup hypervisor traps */
> +// TODO: Choose method
>  WRITE_SYSREG(HCR_PTW|HCR_BSU_INNER|HCR_AMO|HCR_IMO|HCR_FMO|HCR_VM|
> - HCR_TWE|HCR_TWI|HCR_TSC|HCR_TAC|HCR_SWIO|HCR_TIDCP, 
> HCR_EL2);
> + HCR_TWE|HCR_TWI/*|HCR_TSC*/|HCR_TAC|HCR_SWIO|HCR_TIDCP, 
> HCR_EL2);
>  isb();
>  }
> 
> @@ -2494,6 +2495,18 @@ asmlinkage void do_trap_hypervisor(struct 
> cpu_user_regs *regs)
>  GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
>  perfc_incr(trap_smc32);
>  inject_undef32_exception(regs);
> +// TODO: Choose method
> +/*#define omap5_smc(func_id, arg1) \
> +asm volatile ("push {r1-r12, lr}\n\t" \
> +  "mov r12,%0\n\t" \
> +  "mov r0,%1\n\t" \
> +  "smc #1\n\t" \
> +  "pop {r1-r12, lr}" \
> +  : \
> +  : "r" (func_id), "r" (arg1))
> +
> +omap5_smc(regs->r12, regs->r0);
> +advance_pc(regs, hsr); */
>  break;
>  case HSR_EC_HVC32:
>  GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
> --
> 1.7.9.5
> 

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

Re: [Xen-devel] [PATCH v2] build: use correct qemu path in systemd service file and init script

[Anthony PERARD]

On Thu, Jul 30, 2015 at 11:24:47AM +0100, Ian Campbell wrote:
> On Thu, 2015-07-30 at 14:51 +0800, Ting-Wei Lan wrote:
> > When --with-system-qemu is used, it is possible that we cannot find
> > qemu-system-i386 in LIBEXEC_BIN, which can cause error in xencommons
> > init script and xen-qemu-dom0-disk-backend.service systemd service.
> > 
> > Signed-off-by: Ting-Wei Lan 
> 
> Personally I would have omitted the distinction between @qemu_xen_path@ and
> @qemu_xen_systemd@ and just put the env invocation in the service file as
> "/usr/bin/env @qemu_xen_path@" but I suppose that is just bike shedding,
> so:
> 
> Acked-by: Ian Campbell 
> 
> Wei Lui, what do you think about this for 4.6? It fixes a real issue where 
> --with-system-qemu is used without an explicit path, which is supposed to
> search for "qemu" in $PATH but fails to do so for the initscripts and unit
> files, where it uses the old hardcoded default value instead, which
> probably doesn't exist if you are using this option (and if it did isn't
> the thing the user asked for).
> 
> The fix looks pretty straight forward to me.
> 
> Mostly unrelated, is "qemu" a sensible default here? No binary package on
> Debian actually provides a "qemu" binary, they are all qemu-system-foo or
> variants. I'm not sure if that's just a Debian packaging issue though. I've
> added the Qemu-xen maintainers for input...

QEMU does not build a binary called 'qemu', so this would be package
specific. So I think a default should be either 'qemu-system-i386' or fail
if no path is provided.

-- 
Anthony PERARD

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

Re: [Xen-devel] [Xen-Devel] Enabling IRQ Crossbar (Secondary Interrupt Controller) Support

[Ian Campbell]

On Wed, 2015-07-29 at 16:36 -0400, Brandon Perez wrote:
> On 07/21/2015 11:12 AM, Ian Campbell wrote:
> > On Tue, 2015-07-21 at 10:07 -0400, Brandon Perez wrote:
> > > 
> > >   I'm not sure that these patches are quite ready yet to be put
> > > into
> > > the Xen repo.
> > 
> > That's ok, but even for an RFC (Request For Comments) please post them
> > one patch per email in the manner of git send-email. You can use -
> > -subject-prefix='PATCH RFC' to tag them as such.
> > 
> > Thanks,
> > Ian.
> > 
> 
> All,
> 
>  Apologies for the delay, I've been pretty busy the last week. Also, 
> sorry for the confusion with sending the patches, this is my first time 
> working with an open-source dev group.
> 
>  I've sent the patches, one per email, via git.

They were "From: bpere...@ti.com", which bounces my replies. I can resend
if you are not subscribed to the list or you can fish them out of the
archives I guess.

Ian.

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

Re: [Xen-devel] [PATCH for-2.4 0/3] Migration regressions with Xen.

[Stefano Stabellini]

On Thu, 30 Jul 2015, Juan Quintela wrote:
> Anthony PERARD  wrote:
> > This is a critical issue for Xen as migration either with the same version
> > of QEMU, or from a previous version of QEMU is broken.
> >
> > Any suggestion on how to move forward?
> 
> Will send a pull requset tomorrow.
> 
> Thinking about creating a single function that is called for all needed
> places, just to avoid this problem in the future.
> 

Hi Juan,

thanks for looking into this!
Do you have patches already we can look at and help you test to make
sure they fix the issue?

Thanks,

Stefano

> 
> >
> > On Tue, Jul 28, 2015 at 04:54:42PM +0100, Anthony PERARD wrote:
> >> Hi,
> >> 
> >> We've spotted several regression which prevent migration with Xen using the
> >> same version of QEMU or from a previous version of QEMU (tryied with 2.2).
> >> 
> >> Regression have been introduce by at least:
> >> - df4b1024526cae3479da3492d6371fd4a7324a03
> >>   migration: create new section to store global state
> >> - 61964c23e5ddd5a33f15699e45ce126f879e3e33
> >>   migration: Add configuration section
> >> 
> >> The first patch would fix the default case when we use the machine 'xenfv'.
> >> 
> >> But we can also use the machine 'pc' and I don't know how to fix this case.
> >> The second and third patches are a step in the direction of fixing
> >> migration with the machine 'pc'.
> >> 
> >> I have not look at save/restore for PV guest yet.
> >> 
> >> Thanks,
> >> 
> >> Anthony PERARD (3):
> >>   migration: Fix regretion for xenfv machine.
> >>   migration: Fix global state with Xen.
> >>   migration: Add configuration section to vmstate with xen.
> >> 
> >>  hw/i386/pc_piix.c | 3 +++
> >>  include/migration/migration.h | 1 +
> >>  migration/migration.c | 7 +++
> >>  migration/savevm.c| 6 ++
> >>  4 files changed, 17 insertions(+)
> 

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

Re: [Xen-devel] Arndale secondary CPU boot issue Was Re: [xen-unstable test] 60076: regressions - FAIL

[Ian Campbell]

On Thu, 2015-07-30 at 11:54 +0100, Stefano Stabellini wrote:
> On Thu, 30 Jul 2015, Ian Campbell wrote:
> > On Wed, 2015-07-29 at 19:18 +0100, Julien Grall wrote:
> > 
> > As an aside from the issue you are seeing:
> > 
> > > The old implementation of spinlock is sending an event (via the 
> > > assembly
> > > instruction SEV) to the other physical CPUs. This will wake up the
> > > others CPUs waiting on the assembly instruction WFE (Wait For Event).
> > 
> > Uh, I didn't notice this about the new implementation, sorry I should 
> > have
> > done.
> > 
> > IMHO we should investigate (probably with some urgency) inserting a WFE 
> > and
> > SEV pair into the lock/unlock paths, else power consumption will suck.
> > 
> > I think that probably means using something new to replace the 
> > cpu_relax()
> > calls in the spinlocks with a WFE on ARM (we don't just want to change
> > relax) and to add a arch specific hook for the SEV on the release path.
> 
> I agree: adding a WFE in cpu_relax() is too risky at this point.
> 
> 
> > If it is too late for 4.6 (which would depend on the eventual 
> > complexity of
> > the actual fix) then we should fix this ASAP in 4.7 and backport for 
> > 4.6.1.
> 
> I don't think we can release 4.6 without a WFE in the locks. We might
> want to consider reverting to spin_locks on ARM (although I am aware
> that the code is common at the moment).

It turns out we were missing the WFE even in the old code (I vaguely recall
having to refactor the Linux original to fit in with our arch/common split
and leaving myself a TODO item).

So I don't think we can justify changing this was 4.6. Investigating for
4.7 would be nice. Needs some careful though about races of the evt bit vs
the tickets changing though.

Ian.


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

Re: [Xen-devel] Arndale secondary CPU boot issue Was Re: [xen-unstable test] 60076: regressions - FAIL

[David Vrabel]

On 30/07/15 11:54, Stefano Stabellini wrote:
> On Thu, 30 Jul 2015, Ian Campbell wrote:
>> On Wed, 2015-07-29 at 19:18 +0100, Julien Grall wrote:
>>
>> As an aside from the issue you are seeing:
>>
>>> The old implementation of spinlock is sending an event (via the assembly
>>> instruction SEV) to the other physical CPUs. This will wake up the
>>> others CPUs waiting on the assembly instruction WFE (Wait For Event).
>>
>> Uh, I didn't notice this about the new implementation, sorry I should have
>> done.
>>
>> IMHO we should investigate (probably with some urgency) inserting a WFE and
>> SEV pair into the lock/unlock paths, else power consumption will suck.
>>
>> I think that probably means using something new to replace the cpu_relax()
>> calls in the spinlocks with a WFE on ARM (we don't just want to change
>> relax) and to add a arch specific hook for the SEV on the release path.
> 
> I agree: adding a WFE in cpu_relax() is too risky at this point.

WFE in cpu_relax() would be broken.

However, adding two hooks for spin_relax() (using this instead of
cpu_relax()) and spin_signal() that do the WFE/SEV seems low risk to me.

For x86 use:

#define spin_relax() cpu_relax()
#define spin_signal()

>> If it is too late for 4.6 (which would depend on the eventual complexity of
>> the actual fix) then we should fix this ASAP in 4.7 and backport for 4.6.1.
> 
> I don't think we can release 4.6 without a WFE in the locks. We might
> want to consider reverting to spin_locks on ARM (although I am aware
> that the code is common at the moment).

You can't revert the ticket locks for one architecture only.

David

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

Re: [Xen-devel] [PATCH RFC] ARM/IRQ-Crossbar: Make Xen aware of devices being statically mapped in the IRQ crossbar.

[Ian Campbell]

On Wed, 2015-07-29 at 16:53 -0400, Brandon Perez wrote:
> In general, Xen needs to own very few interrupts to run. Most of these are 
> timer
> PPIs, which do not involve the crossbar. The notable exception to this is the
> serial device, which is an SPI. On the DRA7 chips, this involves going through
> the interrupt crossbar.
> 
> As the device tree entry will contain the crossbar input number, and not the
> IRQ line, Xen must be given the SPI input line number. This is achieved with
> the "default-mapping" property, which contains the SPI number that the device
> will correspond to in the mapping setup by the bootloader.

Is the default-mapping property defined by a generic standard? I don't find
it in ePAPR for example. If it isn't a generic part of device tree then it
shouldn't be handled in device_tree.c, it would need to be handled in a
device specific place which has matched on the compatible string (probably
the one for your cross-bar) to determine the semantics of the properties.

I also don't any such property for any device in the device-tree bindings
docs[0], which we would want to see before committing to using an interface
.

Thanks,
Ian.

[0] in linux.git as Documentation/devicetree/bindings or separately in http
s://git.kernel.org/cgit/linux/kernel/git/devicetree/devicetree
-rebasing.git/

> 
> Signed-off-by: Brandon Perez 
> ---
>  xen/common/device_tree.c |   11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 31f169b..fc82eb4 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -1036,10 +1036,13 @@ int dt_device_get_raw_irq(const struct 
> dt_device_node *device,
>  dt_dprintk("dt_device_get_raw_irq: dev=%s, index=%u\n",
> device->full_name, index);
> 
> -/* Get the interrupts property */
> -intspec = dt_get_property(device, "interrupts", &intlen);
> -if ( intspec == NULL )
> -return -EINVAL;
> +/* Get the appropiate interrupts property */
> +intspec = dt_get_property(device, "default-mapping", &intlen);
> +if (intspec == NULL) {
> +intspec = dt_get_property(device, "interrupts", &intlen);
> +if (intspec == NULL)
> +return -EINVAL;
> +}
>  intlen /= sizeof(*intspec);
> 
>  dt_dprintk(" intspec=%d intlen=%d\n", be32_to_cpup(intspec), 
> intlen);
> --
> 1.7.9.5
> 

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

Re: [Xen-devel] [PATCH v2] build: use correct qemu path in systemd service file and init script

[Ian Campbell]

On Thu, 2015-07-30 at 12:23 +0100, Anthony PERARD wrote:
> On Thu, Jul 30, 2015 at 11:24:47AM +0100, Ian Campbell wrote:
> > On Thu, 2015-07-30 at 14:51 +0800, Ting-Wei Lan wrote:
> > > When --with-system-qemu is used, it is possible that we cannot find
> > > qemu-system-i386 in LIBEXEC_BIN, which can cause error in xencommons
> > > init script and xen-qemu-dom0-disk-backend.service systemd service.
> > > 
> > > Signed-off-by: Ting-Wei Lan 
> > 
> > Personally I would have omitted the distinction between @qemu_xen_path@ 
> > and
> > @qemu_xen_systemd@ and just put the env invocation in the service file 
> > as
> > "/usr/bin/env @qemu_xen_path@" but I suppose that is just bike 
> > shedding,
> > so:
> > 
> > Acked-by: Ian Campbell 
> > 
> > Wei Lui, what do you think about this for 4.6? It fixes a real issue 
> > where 
> > --with-system-qemu is used without an explicit path, which is supposed 
> > to
> > search for "qemu" in $PATH but fails to do so for the initscripts and 
> > unit
> > files, where it uses the old hardcoded default value instead, which
> > probably doesn't exist if you are using this option (and if it did 
> > isn't
> > the thing the user asked for).
> > 
> > The fix looks pretty straight forward to me.
> > 
> > Mostly unrelated, is "qemu" a sensible default here? No binary package 
> > on
> > Debian actually provides a "qemu" binary, they are all qemu-system-foo 
> > or
> > variants. I'm not sure if that's just a Debian packaging issue though. 
> > I've
> > added the Qemu-xen maintainers for input...
> 
> QEMU does not build a binary called 'qemu', so this would be package
> specific. So I think a default should be either 'qemu-system-i386' or 
> fail if no path is provided.

Thanks. I think we should take Ting-Wei's patch as is for 4.6 and fixup the
default to be qemu-system-i386 in 4.7.

Ian.

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

Re: [Xen-devel] Arndale secondary CPU boot issue Was Re: [xen-unstable test] 60076: regressions - FAIL

[Julien Grall]

Hi Ian,

On 30/07/15 09:55, Ian Campbell wrote:
> On Wed, 2015-07-29 at 19:18 +0100, Julien Grall wrote:
>> It appears to be required on the Arndale to boot secondaries CPUs.
>> Although, depending on where I put the sev I don't have the same 
>> behavior:
>>  - sev in smp_init callback: the CPU is not coming up
>>  - sev before or after arch_cpu_up: the CPU is booting but not in 
>> HYP
>> mode [2]
>>
>> I haven't yet figured out where the "sev" should be placed in order to
>> get the CPU boot correctly.
> 
> Does the arndale end up using 
> .cpu_up = cpu_up_send_sgi,
> or
> .cpu_up = exynos5_cpu_up,
> ?

The first one given that the arndale is an exynos5250. I'm not sure why
we didn't use exynos5_cpu_up. Although it doesn't seem to fix the problem.

>> What I don't understand is how the placement of "sev" would affect the
>> secondary processor to boot in HYP mode or Kernel mode or nothing at all
>>
>> This platform seems very picky and I don't remember having a
>> documentation about how the SMP boot works for this platform. Linux
>> seems to avoid the SEV for this platform.
> 
> 32-bit Linux has some in common code paths IIRC, which are not always
> apparent at first glance.

Good to know.

Regards,

-- 
Julien Grall

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

Re: [Xen-devel] Vote for Xen talks at OpenStack summit (deadline today)

[Juergen Gross]

On 07/30/2015 12:29 PM, Lars Kurth wrote:

So far, I am aware of one talk. Note that the OpenStack summit agenda's
are determined by popular vote (which ends today).

If there are others Xen related talks, feel free to add by replying to
this mail
https://www.openstack.org/summit/tokyo-2015/vote-for-speakers/Presentation/6150


https://www.openstack.org/summit/tokyo-2015/vote-for-speakers/Presentation/5743


Juergen

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

Re: [Xen-devel] [PATCH v2 32/41] arm : acpi dynamically map mmio regions

[Shannon Zhao]

On 2015/7/5 21:30, Parth Dixit wrote:
> +shannon
> 
> On 15 June 2015 at 06:49, Julien Grall  wrote:
>> Hi Parth,
>>
>> On 14/06/2015 11:27, Parth Dixit wrote:
>>>
>>> On 8 June 2015 at 22:20, Julien Grall  wrote:

 Hi Parth,

 On 17/05/2015 21:03, Parth Dixit wrote:
>
>
> In ACPI mmio regions are described in DSDT which requires
> AML interpreter. Defer the mapping of mmio until DSDT is parsed in
> DOM0 and map mmio's dynamically at the time of request.



 I'm against a such solution. Even though it's DOM0 we should avoid to
 allow
 him to map anything (RAM,...) on data abort.
>>>
>>> I think we agreed to this solution
>>> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg02059.html
>>
>>
>> Firstly, this kind of link should have been put in the changelog of the
>> patch (after ---). It helps the reviewer to know what was decided (or not)
>> on the previous discussion. It's more true with a series of more than 40
>> patches...
>>
>> Secondly, the thread you pointed as some discussion on it but no formal
>> agreement about what to do. If there is no answer, it's better to do a
>> resume and ask if anyone are agree.
>>
>> Finally, what you've implemented and what was suggested by Ian is different.
>> You are allowing any region to be mapped in DOM0 via this way. Only MMIO
>> should be allowed.
>>
>> Concerning the mapping attribute used. Based on Ard answer "The UEFI spec
>> mandates that the memory map describes all memory in the system, so if dom0
>> accesses any ranges outside of that, it makes sense
>> to just use device mappings for stage 2.". We should use by default Device
>> Stage 2, it's safer. If it doesn't work later (because some PCI BAR may be
>> memory, which if I wasn't able to prove), then we can think differently.
>>
>> For the mapping of the MMIO to DOM0, I believe we can map any non-RAM (and
>> any non-RAM not used by Xen) regions to DOM0 at boot time (I think x86 does
>> that). It would keep the ACPI code contained at boot time and no difference
>> during runtime.
>>

But How do we get the MMIO information of the devices in DSDT table? If
we want to parse DSDT table, we must introduce AML interpreter, IIUC.

Julien, do you have any other ideas?

Thanks,
-- 
Shannon

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

Re: [Xen-devel] PCI Pass-through in Xen ARM - Draft 2.

[Manish Jaggi]

On Thursday 30 July 2015 03:24 PM, Ian Campbell wrote:

On Wed, 2015-07-29 at 15:07 +0530, Manish Jaggi wrote:

On Monday 06 July 2015 03:50 PM, Ian Campbell wrote:

On Mon, 2015-07-06 at 15:36 +0530, Manish Jaggi wrote:

On Monday 06 July 2015 02:41 PM, Ian Campbell wrote:

On Sun, 2015-07-05 at 11:25 +0530, Manish Jaggi wrote:

On Monday 29 June 2015 04:01 PM, Julien Grall wrote:

Hi Manish,

On 28/06/15 19:38, Manish Jaggi wrote:

4.1 Holes in guest memory space

Holes are added in the guest memory space for mapping pci
device's BAR
regions.
These are defined in arch-arm.h

/* For 32bit */
GUEST_MMIO_HOLE0_BASE, GUEST_MMIO_HOLE0_SIZE
 
/* For 64bit */

GUEST_MMIO_HOLE1_BASE , GUEST_MMIO_HOLE1_SIZE

The memory layout for 32bit and 64bit are exactly the same. Why
do you
need to differ here?

I think Ian has already replied. I will change the name of macro

4.2 New entries in xenstore for device BARs

toolkit also updates the xenstore information for the device
(virtualbar:physical bar).
This information is read by xenpciback and returned to the
pcifront
driver configuration
space accesses.

Can you details what do you plan to put in xenstore and how?

It is implementation . But I plan to put under domU / device /
heirarchy

Actually, xenstore is an API of sorts which needs to be maintained
going
forward (since front and backend can evolve separately, so it does
need
some level of design and documentation.


What about the expansion ROM?

Do you want to put some restriction on not using expansion ROM as
a
passthrough device.

"expansion ROM as a passthrough device" doesn't make sense to me,
passthrough devices may _have_ an expansion ROM.

The expansion ROM is just another BAR. I don't know how
pcifront/back
deal with those today on PV x86, but I see no reason for ARM to
deviate.



4.3 Hypercall for bdf mapping notification to xen
---
#define PHYSDEVOP_map_sbdf  43
typedef struct {
u32 s;
u8 b;
u8 df;
u16 res;
} sbdf_t;
struct physdev_map_sbdf {
int domain_id;
sbdf_tsbdf;
sbdf_tgsbdf;
};

Each domain has a pdev list, which contains the list of all
pci devices.
The
pdev structure already has a sbdf information. The
arch_pci_dev is
updated to
contain the gsbdf information. (gs- guest segment id)

Whenever there is trap from guest or an interrupt has to be
injected,
the pdev
list is iterated to find the gsbdf.

Can you give more background for this section? i.e:
- Why do you need this?
- How xen will translate the gbdf to a vDeviceID?

In the context of the hypercall processing.

- Who will call this hypercall?
- Why not setting the gsbdf when the device is
assigned?

Can the maintainer of the pciback suggest an alternate.

That's not me, but I don't think this belongs here, I think it can
be
done from the toolstack. If you think not then please explain what
information the toolstack doesn't have in its possession which
prevents
this mapping from being done there.

The toolstack does not have the guest sbdf information. I could only
find it in xenpciback.

Are you sure? The sbdf relates to the physical device, correct? If so
then surely the toolstack knows it -- it's written in the config file
and is the primary parameter to all of the related libxl passthrough
APIs. The toolstack wouldn't be able to do anything about passing
through a given device without knowing which device it should be
passing
through.

Perhaps this info needs plumbing through to some new bit of the
toolstack, but it is surely available somewhere.

If you meant the virtual SBDF then that is in libxl_device_pci.vdevfn.

I added prints in libxl__device_pci_add. vdevfn is always 0 so this may
not be the right variable to use.
Can you please recheck.

Also the vdev-X entry in xenstore appears to be created from pciback
code and not from xl.
Check function xen_pcibk_publish_pci_dev.

So I have to send a hypercall from pciback only.

I don't think the necessarily follows.

You could have the tools read the vdev-X node back on plug.
I have been trying to get the flow of caller of libxl__device_pci_add 
during pci device assignemnt from cfg file(cold boot).
It should be called form xl create flow. Is it called from C code or 
Python code.


libxl__device_pci_add calls xc_assign_device


Secondly, the vdev-X entry is created async by dom0 watching on event. 
So how the tools could read back and call assign device again.


static void xen_pcibk_be_watch(struct xenbus_watch *watch,
 const char **vec, unsigned int len)
{
 ...
switch (xenbus_read_driver_state(pdev->xdev->nodename)) {
case XenbusStateInitWait:
xen_pcibk_setup_backend(pdev);
break;
}


Or you could change things such that vdevfn is always chosen by the
toolstack for ARM, not optio

Re: [Xen-devel] [BUG] Emulation issues

[Roger Pau Monné]

El 30/07/15 a les 12.59, Paul Durrant ha escrit:
>> -Original Message-
> [big snip]
>> Sorry, missed that in the noise. So, the problem is that there is no 
>> in-flight I/O
>> even though pio completion is being attempted. Something has got out of
>> sync.
>>
> 
> I think I understand what may be happening... The code in hvmemul_do_io() 
> basically expects to be called either to issue an I/O or to extract info from 
> a completed one. However it is being called unconditionally (in the PIO case) 
> out of hvm_do_resume, rather than only if the in-flight I/O state has been 
> updated to STATE_IORESP_READY.
> 
> Can you try this patch (also containing my previous debug patch)?
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 30acb78..1bc3cc9 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -145,6 +145,8 @@ static int hvmemul_do_io(
>  return X86EMUL_UNHANDLEABLE;
>  goto finish_access;
>  default:
> +gprintk(XENLOG_ERR, "weird emulation state %u\n",
> +vio->io_req.state);
>  return X86EMUL_UNHANDLEABLE;
>  }
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index ec1d797..a476271 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -472,7 +472,6 @@ void hvm_do_resume(struct vcpu *v)
>  struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io;
>  struct domain *d = v->domain;
>  struct hvm_ioreq_server *s;
> -enum hvm_io_completion io_completion;
> 
>  check_wakeup_from_wait();
> 
> @@ -499,33 +498,38 @@ void hvm_do_resume(struct vcpu *v)
>  }
>  }
> 
> -io_completion = vio->io_completion;
> -vio->io_completion = HVMIO_no_completion;
> -
> -switch ( io_completion )
> -{
> -case HVMIO_no_completion:
> -break;
> -case HVMIO_mmio_completion:
> -handle_mmio();
> -break;
> -case HVMIO_pio_completion:
> -(void)handle_pio(vio->io_req.addr, vio->io_req.size,
> - vio->io_req.dir);
> -break;
> -case HVMIO_realmode_completion:
> +if ( vio->io_req.state == STATE_IORESP_READY )
>  {
> -struct hvm_emulate_ctxt ctxt;
> +enum hvm_io_completion io_completion;
> 
> -hvm_emulate_prepare(&ctxt, guest_cpu_user_regs());
> -vmx_realmode_emulate_one(&ctxt);
> -hvm_emulate_writeback(&ctxt);
> +io_completion = vio->io_completion;
> +vio->io_completion = HVMIO_no_completion;
> 
> -break;
> -}
> -default:
> -ASSERT_UNREACHABLE();
> -break;
> +switch ( io_completion )
> +{
> +case HVMIO_no_completion:
> +break;
> +case HVMIO_mmio_completion:
> +handle_mmio();
> +break;
> +case HVMIO_pio_completion:
> +(void)handle_pio(vio->io_req.addr, vio->io_req.size,
> + vio->io_req.dir);
> +break;
> +case HVMIO_realmode_completion:meet
> +{
> +struct hvm_emulate_ctxt ctxt;
> +
> +hvm_emulate_prepare(&ctxt, guest_cpu_user_regs());
> +vmx_realmode_emulate_one(&ctxt);
> +hvm_emulate_writeback(&ctxt);
> +
> +break;
> +}
> +default:
> +ASSERT_UNREACHABLE();
> +break;
> +}
>  }
> 
>  if ( unlikely(d->arch.event_write_data) )
> @@ -2747,6 +2751,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s, ioreq_t 
> *proto_p,
>  }
>  }
> 
> +gprintk(XENLOG_ERR, "unable to contact device model\n");
>  return X86EMUL_UNHANDLEABLE;
>  }

With this (and only this) patch applied I get the following output:

(XEN) irq.c:276: Dom18 PCI link 2 changed 11 -> 0
(XEN) irq.c:276: Dom18 PCI link 3 changed 5 -> 0
(XEN) irq.c:276: Dom19 PCI link 0 changed 5 -> 0
(XEN) irq.c:276: Dom19 PCI link 1 changed 10 -> 0
(XEN) irq.c:276: Dom19 PCI link 2 changed 11 -> 0
(XEN) irq.c:276: Dom19 PCI link 3 changed 5 -> 0
(XEN) d19v0 weird emulation state 1
(XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
(XEN) domain_crash called from io.c:166
(XEN) d19v0 weird emulation state 1
(XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
(XEN) domain_crash called from io.c:166
(XEN) d19v0 weird emulation state 1
(XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
(XEN) domain_crash called from io.c:166
(XEN) d19v0 weird emulation state 1
(XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
(XEN) domain_crash called from io.c:166
(XEN) d19v0 weird emulation state 1
(XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
(XEN) domain_crash called from io.c:166
(XEN) d19v0 weird emulation state 1
(XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
(XEN) domain_crash called from io.c:166
(XEN) d19v0 weird emulation state 1
(XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
(XEN) domain_crash called from io.c:166
(XEN) d19v0 weird emulation s

Re: [Xen-devel] [BUG] Emulation issues

[Paul Durrant]

> -Original Message-
> From: Roger Pau Monné [mailto:roger@citrix.com]
> Sent: 30 July 2015 14:06
> To: Paul Durrant; xen-devel; Andrew Cooper
> Subject: Re: [BUG] Emulation issues
> 
> El 30/07/15 a les 12.59, Paul Durrant ha escrit:
> >> -Original Message-
> > [big snip]
> >> Sorry, missed that in the noise. So, the problem is that there is no 
> >> in-flight
> I/O
> >> even though pio completion is being attempted. Something has got out of
> >> sync.
> >>
> >
> > I think I understand what may be happening... The code in
> hvmemul_do_io() basically expects to be called either to issue an I/O or to
> extract info from a completed one. However it is being called unconditionally
> (in the PIO case) out of hvm_do_resume, rather than only if the in-flight I/O
> state has been updated to STATE_IORESP_READY.
> >
> > Can you try this patch (also containing my previous debug patch)?
> >
> > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> > index 30acb78..1bc3cc9 100644
> > --- a/xen/arch/x86/hvm/emulate.c
> > +++ b/xen/arch/x86/hvm/emulate.c
> > @@ -145,6 +145,8 @@ static int hvmemul_do_io(
> >  return X86EMUL_UNHANDLEABLE;
> >  goto finish_access;
> >  default:
> > +gprintk(XENLOG_ERR, "weird emulation state %u\n",
> > +vio->io_req.state);
> >  return X86EMUL_UNHANDLEABLE;
> >  }
> >
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > index ec1d797..a476271 100644
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -472,7 +472,6 @@ void hvm_do_resume(struct vcpu *v)
> >  struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io;
> >  struct domain *d = v->domain;
> >  struct hvm_ioreq_server *s;
> > -enum hvm_io_completion io_completion;
> >
> >  check_wakeup_from_wait();
> >
> > @@ -499,33 +498,38 @@ void hvm_do_resume(struct vcpu *v)
> >  }
> >  }
> >
> > -io_completion = vio->io_completion;
> > -vio->io_completion = HVMIO_no_completion;
> > -
> > -switch ( io_completion )
> > -{
> > -case HVMIO_no_completion:
> > -break;
> > -case HVMIO_mmio_completion:
> > -handle_mmio();
> > -break;
> > -case HVMIO_pio_completion:
> > -(void)handle_pio(vio->io_req.addr, vio->io_req.size,
> > - vio->io_req.dir);
> > -break;
> > -case HVMIO_realmode_completion:
> > +if ( vio->io_req.state == STATE_IORESP_READY )
> >  {
> > -struct hvm_emulate_ctxt ctxt;
> > +enum hvm_io_completion io_completion;
> >
> > -hvm_emulate_prepare(&ctxt, guest_cpu_user_regs());
> > -vmx_realmode_emulate_one(&ctxt);
> > -hvm_emulate_writeback(&ctxt);
> > +io_completion = vio->io_completion;
> > +vio->io_completion = HVMIO_no_completion;
> >
> > -break;
> > -}
> > -default:
> > -ASSERT_UNREACHABLE();
> > -break;
> > +switch ( io_completion )
> > +{
> > +case HVMIO_no_completion:
> > +break;
> > +case HVMIO_mmio_completion:
> > +handle_mmio();
> > +break;
> > +case HVMIO_pio_completion:
> > +(void)handle_pio(vio->io_req.addr, vio->io_req.size,
> > + vio->io_req.dir);
> > +break;
> > +case HVMIO_realmode_completion:meet
> > +{
> > +struct hvm_emulate_ctxt ctxt;
> > +
> > +hvm_emulate_prepare(&ctxt, guest_cpu_user_regs());
> > +vmx_realmode_emulate_one(&ctxt);
> > +hvm_emulate_writeback(&ctxt);
> > +
> > +break;
> > +}
> > +default:
> > +ASSERT_UNREACHABLE();
> > +break;
> > +}
> >  }
> >
> >  if ( unlikely(d->arch.event_write_data) )
> > @@ -2747,6 +2751,7 @@ int hvm_send_ioreq(struct hvm_ioreq_server *s,
> ioreq_t *proto_p,
> >  }
> >  }
> >
> > +gprintk(XENLOG_ERR, "unable to contact device model\n");
> >  return X86EMUL_UNHANDLEABLE;
> >  }
> 
> With this (and only this) patch applied I get the following output:
> 
> (XEN) irq.c:276: Dom18 PCI link 2 changed 11 -> 0
> (XEN) irq.c:276: Dom18 PCI link 3 changed 5 -> 0
> (XEN) irq.c:276: Dom19 PCI link 0 changed 5 -> 0
> (XEN) irq.c:276: Dom19 PCI link 1 changed 10 -> 0
> (XEN) irq.c:276: Dom19 PCI link 2 changed 11 -> 0
> (XEN) irq.c:276: Dom19 PCI link 3 changed 5 -> 0
> (XEN) d19v0 weird emulation state 1
> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
> (XEN) domain_crash called from io.c:166
> (XEN) d19v0 weird emulation state 1
> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
> (XEN) domain_crash called from io.c:166
> (XEN) d19v0 weird emulation state 1
> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
> (XEN) domain_crash called from io.c:166
> (XEN) d19v0 weird emulation state 1
> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
> (XEN) domain_cra

Re: [Xen-devel] [BUG] Emulation issues

[Andrew Cooper]

On 30/07/15 14:12, Paul Durrant wrote:
>> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
>> (XEN) domain_crash called from io.c:166
>> (XEN) d19v0 weird emulation state 1
>> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
>> (XEN) domain_crash called from io.c:166
>> (XEN) d19v0 weird emulation state 1
>> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
>> (XEN) domain_crash called from io.c:166
>>
> Hmm. Can't understand how that's happening... handle_pio() shouldn't be 
> called unless the state is STATE_IORESP_READY and yet the inner function is 
> hitting the default case in the switch.

Sounds like something is changing the state between the two checks.  Is
this shared memory writeable by qemu?

~Andrew

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

Re: [Xen-devel] [BUG] Emulation issues

[Paul Durrant]

> -Original Message-
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com]
> Sent: 30 July 2015 14:19
> To: Paul Durrant; Roger Pau Monne; xen-devel
> Subject: Re: [BUG] Emulation issues
> 
> On 30/07/15 14:12, Paul Durrant wrote:
> >> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) d19v0 weird emulation state 1
> >> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >> (XEN) d19v0 weird emulation state 1
> >> (XEN) io.c:165:d19v0 Weird HVM ioemulation status 1.
> >> (XEN) domain_crash called from io.c:166
> >>
> > Hmm. Can't understand how that's happening... handle_pio() shouldn't be
> called unless the state is STATE_IORESP_READY and yet the inner function is
> hitting the default case in the switch.
> 
> Sounds like something is changing the state between the two checks.  Is
> this shared memory writeable by qemu?
> 

No, this is the internal state. I really can't see how it's being changed.

  Paul

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

Re: [Xen-devel] [Xen-Devel] Enabling IRQ Crossbar (Secondary Interrupt Controller) Support

[Brandon Perez]

On 07/30/2015 07:23 AM, Ian Campbell wrote:

On Wed, 2015-07-29 at 16:36 -0400, Brandon Perez wrote:

On 07/21/2015 11:12 AM, Ian Campbell wrote:

On Tue, 2015-07-21 at 10:07 -0400, Brandon Perez wrote:


   I'm not sure that these patches are quite ready yet to be put
into
the Xen repo.


That's ok, but even for an RFC (Request For Comments) please post them
one patch per email in the manner of git send-email. You can use -
-subject-prefix='PATCH RFC' to tag them as such.

Thanks,
Ian.



All,

  Apologies for the delay, I've been pretty busy the last week. Also,
sorry for the confusion with sending the patches, this is my first time
working with an open-source dev group.

  I've sent the patches, one per email, via git.


They were "From: bpere...@ti.com", which bounces my replies. I can resend
if you are not subscribed to the list or you can fish them out of the
archives I guess.

Ian.



Apologies, I mistyped my email address. It should be "b-per...@ti.com".

Brandon

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

Re: [Xen-devel] [Xen-Devel] Enabling IRQ Crossbar (Secondary Interrupt Controller) Support

[Brandon Perez]

On 07/30/2015 06:57 AM, Ian Campbell wrote:

On Wed, 2015-07-29 at 16:36 -0400, Brandon Perez wrote:

On 07/21/2015 11:12 AM, Ian Campbell wrote:

On Tue, 2015-07-21 at 10:07 -0400, Brandon Perez wrote:


   I'm not sure that these patches are quite ready yet to be put
into
the Xen repo.


That's ok, but even for an RFC (Request For Comments) please post them
one patch per email in the manner of git send-email. You can use -
-subject-prefix='PATCH RFC' to tag them as such.

Thanks,
Ian.



All,

  Apologies for the delay, I've been pretty busy the last week. Also,
sorry for the confusion with sending the patches, this is my first time
working with an open-source dev group.

  I've sent the patches, one per email, via git.


Thanks. For next time please can you send a batch of related patches all
with the same git send-email invocation such that they arrive as a set of
threaded mails (i.e. with a References header on the previous). That keeps
the related patches together in peoples in box, and also labels them 1/N,
2/N etc so we know which order they are applicable in.

Invoking send-email with --dry-run will print out all the headers it would
send, so you can check it will do as expected.

http://wiki.xen.org/wiki/Submitting_Xen_Patches#Git_send-email talks about
this stuff a bit.

Thanks,

Ian.



Hi Ian,

I see that makes sense, I'll make sure to batch the patches next time.

Brandon

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

[Xen-devel] [PATCH OSSTEST] Executive: Change Cambridge specific default

[Ian Campbell]

ControlDaemonHost is unused in production-config* since they both set
{Owner,Queue}DaemonHost explicitly.

Some sort of default seems useful, so switch it to just
"control-daemons" in the DNS domain of the controller.

Signed-off-by: Ian Campbell 
---
 Osstest/Executive.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 79d433a..97723be 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -93,7 +93,7 @@ our (@all_lock_tables) = qw(flights resources);
 #  Transactional reads must take out locks as if they were modifying
 
 augmentconfigdefaults(
-ControlDaemonHost => 'control-daemons.osstest.cam.xci-test.com',
+ControlDaemonHost => 'control-daemons',
 OwnerDaemonPort => 4031,
 QueueDaemonPort => 4032,
 QueueDaemonRetry => 120, # seconds
-- 
2.1.4


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

[Xen-devel] [PATCH OSSTEST] crontab-cambridge: Add a commented out adhoc bisect line

[Ian Campbell]

This is handy to have, editing it in locally just means one cannot
simply use "crontab contab-cambridge" to load a new one without
remembering the content of the line for next time.

Signed-off-by: Ian Campbell 
---
 crontab-cambridge | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crontab-cambridge b/crontab-cambridge
index 3d510ac..37e5527 100644
--- a/crontab-cambridge
+++ b/crontab-cambridge
@@ -7,5 +7,6 @@ MAILTO=ian.jack...@citrix.com,ian.campb...@eu.citrix.com
 46 7   * * 4   cd testing.git && 
BRANCHES='distros-debian-jessie'  ./cr-for-branches branches -w 
"./cr-daily-branch --real"
 46 7   * * 3   cd testing.git && 
BRANCHES='distros-debian-wheezy'  ./cr-for-branches branches -w 
"./cr-daily-branch --real"
 46 7   * * 2   cd testing.git && 
BRANCHES='distros-debian-squeeze' ./cr-for-branches branches -w 
"./cr-daily-branch --real"
+#8-59/5*   * * *   cd bisects/adhoc.git && 
with-lock-ex -q data-tree-lock bash -c "./cr-try-bisect-adhoc; exit $?"
 22 8   * * *   cd testing.git && BRANCHES=maintjobs
./cr-for-branches . -w ./cr-all-branch-statuses ''
 3  4   * * *   savelog -c28 
testing.git/tmp/cr-for-branches.log >/dev/null
-- 
2.1.4


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

[Xen-devel] [RFC for-4.7] Switching to a single qemu tree each per qemu-xen and qemu-trad

[Ian Campbell]

(CC-ing 2x QEMU maintainers and stable release manager)

The separate trees are a holdover from mercurial, which didn't (at the
time) have a good in-repo branching model.

I propose that Xen 4.6 should be the last release which uses these split
trees and that instead we combine them into just qemu-xen.git (upstream)
and qemu-xen-traditional.git (our old fork), with branches in those repos
to match our releases. I picked those names to match the libxl interface
names for these.

This will result in one place to get all the qemu stable branches for a
given qemu type, without having to remember the naming scheme (which is
counter-intuitive). It will allow easier cherry-picking and produce less
places for users to look for our code etc.

We could put all branches for both in the same tree but I think we want to
just let qemu-xen-trad sit quietly in its own corner. Keeping it separate
reinforces that it is almost completely frozen, also it saves me having to
think up a branch naming scheme within a single repo.

A proposed mapping for the two tree scheme which broadly follows the
xen.git scheme is (paths relative to xenbits git root):

qemu-xen-traditional:

/staging/qemu-xen-unstable.git#master => /qemu-xen-traditional.git#staging
/staging/qemu-xen-X.Y-testing.git#master => 
/qemu-xen-traditional.git#staging-X.Y
/qemu-xen-unstable.git#master => /qemu-xen-traditional.git#master
/qemu-xen-X.Y-testing.git#master => /qemu-xen-traditional.git#stable-X.Y

NB the revision to use is still referenced in xen.git/Config.mk for
each branch, no change here.

XXX why do staging/* exist, and what pushes from staging to the other?
Should we ditch one or the other?

qemu-xen:

/staging/qemu-upstream-unstable.git#master => /qemu-xen.git#staging
/staging/qemu-upstream-X.Y-testing.git#master => /qemu-xen.git#staging-X.Y
/qemu-upstream-unstable.git#master => /qemu-xen.git#master
/qemu-upstream-X.Y-testing.git#master => /qemu-xen.git#stable-X.Y

These are push gated by their respective qemu-upstream-* osstest flights.

qemu-mainline:

Upstream remains git://git.qemu.org/qemu.git. 

/osstest/qemu.git#mainline/xen-tested-master => 
/qemu-xen.git#upstream-tested

/osstest/qemu.git should be removed.

NB this is the same qemu-xen.git as above.

Below is an osstest patch which implements this proposed scheme (I've not
actually created any trees).

The main open question is what to do about the existing split repos for
existing stable branches. We could:

  * Teach osstest (ap-push) to push to the old tree as well as the new
for existing (<= 4.6) branches only.
  * Push a Config.mk update to every stable branch and retire the
existing trees on the next relevant point release, if there is one.

In both cases perhaps combined with tweaking gitweb to omit the old names
from the project list (http://xenbits.xen.org/gitweb/).

Ian.

>From c87f628a7b505d46f2823252c0c3e3ae0fcb736b Mon Sep 17 00:00:00 2001
From: Ian Campbell 
Date: Thu, 30 Jul 2015 13:42:03 +0100
Subject: [PATCH] Switch to merged qemu-xen{,-traditional}.git trees

Signed-off-by: Ian Campbell 
---
 ap-common|  7 +++
 ap-fetch-version | 10 --
 ap-fetch-version-old | 12 
 ap-push  | 18 ++
 4 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/ap-common b/ap-common
index dfeab9e..eb0624e 100644
--- a/ap-common
+++ b/ap-common
@@ -26,9 +26,7 @@
 : ${TREE_XEN:=git://xenbits.xen.org/xen.git}
 : ${PUSH_TREE_XEN:=$XENBITS:/home/xen/git/xen.git}
 
-#: ${TREE_QEMU:=git://mariner.uk.xensource.com/qemu-$xenbranch.git}
-: ${TREE_QEMU:=git://xenbits.xen.org/staging/qemu-$xenbranch.git}
-
+: ${TREE_QEMU:=git://xenbits.xen.org/qemu-xen-traditional.git}
 
 : ${GIT_KERNEL_ORG:=git://git.kernel.org}
 : ${KERNEL_SCM:=${GIT_KERNEL_ORG}/pub/scm/linux/kernel/git}
@@ -87,7 +85,8 @@ fi
 
 : ${TREEBASE_LINUX_XCP:=http://hg.uk.xensource.com/carbon/trunk/linux-2.6.27}
 
-: 
${TREE_QEMU_UPSTREAM:=git://xenbits.xen.org/staging/qemu-upstream-${xenbranch#xen-}.git}
+: ${TREE_QEMU_UPSTREAM:=git://xenbits.xen.org/qemu-xen.git}
+: ${PUSH_TREE_QEMU_UPSTREAM=$XENBITS:/home/xen/git/qemu-xen.git
 : ${LOCALREV_QEMU_UPSTREAM:=daily-cron.$branch}
 
 : ${TREE_QEMU_MAINLINE:=git://git.qemu.org/qemu.git}
diff --git a/ap-fetch-version b/ap-fetch-version
index 8b41acf..754a398 100755
--- a/ap-fetch-version
+++ b/ap-fetch-version
@@ -53,9 +53,15 @@ qemu-mainline)
repo_tree_rev_fetch_git $branch \
$TREE_QEMU_MAINLINE master $LOCALREV_QEMU_UPSTREAM
;;
-qemu-upstream-*)
+qemu-upstream-unstable)
 repo_tree_rev_fetch_git $branch \
-   $TREE_QEMU_UPSTREAM master $LOCALREV_QEMU_UPSTREAM
+   $TREE_QEMU_UPSTREAM staging $LOCALREV_QEMU_UPSTREAM
+;;
+qemu-upstream-*-testing)
+   branchcore=${branch#qemu-upstream-}
+   branchcore=${branchcore%-testing}
+repo_tree_rev_fetch_git $branch

Re: [Xen-devel] [RFC for-4.7] Switching to a single qemu tree each per qemu-xen and qemu-trad

[Ian Jackson]

Ian Campbell writes ("[RFC for-4.7] Switching to a single qemu tree each per 
qemu-xen and qemu-trad"):
...
> qemu-xen-traditional:
...
> XXX why do staging/* exist, and what pushes from staging to the other?
> Should we ditch one or the other?

Once upon a time, some of these had their own push gate.  They are
nowadays always pushed together.  The staging branches should be
abolished.

> The main open question is what to do about the existing split repos for
> existing stable branches. We could:
> 
>   * Teach osstest (ap-push) to push to the old tree as well as the new
> for existing (<= 4.6) branches only.
>   * Push a Config.mk update to every stable branch and retire the
> existing trees on the next relevant point release, if there is one.

We could do both of these, so we have a transitional period.

Ian.

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

Re: [Xen-devel] PCI Pass-through in Xen ARM - Draft 2.

[Ian Campbell]

On Thu, 2015-07-30 at 18:21 +0530, Manish Jaggi wrote:
> 
> On Thursday 30 July 2015 03:24 PM, Ian Campbell wrote:
> > On Wed, 2015-07-29 at 15:07 +0530, Manish Jaggi wrote:
> > > On Monday 06 July 2015 03:50 PM, Ian Campbell wrote:
> > > > On Mon, 2015-07-06 at 15:36 +0530, Manish Jaggi wrote:
> > > > > On Monday 06 July 2015 02:41 PM, Ian Campbell wrote:
> > > > > > On Sun, 2015-07-05 at 11:25 +0530, Manish Jaggi wrote:
> > > > > > > On Monday 29 June 2015 04:01 PM, Julien Grall wrote:
> > > > > > > > Hi Manish,
> > > > > > > > 
> > > > > > > > On 28/06/15 19:38, Manish Jaggi wrote:
> > > > > > > > > 4.1 Holes in guest memory space
> > > > > > > > > 
> > > > > > > > > Holes are added in the guest memory space for mapping pci
> > > > > > > > > device's BAR
> > > > > > > > > regions.
> > > > > > > > > These are defined in arch-arm.h
> > > > > > > > > 
> > > > > > > > > /* For 32bit */
> > > > > > > > > GUEST_MMIO_HOLE0_BASE, GUEST_MMIO_HOLE0_SIZE
> > > > > > > > >  
> > > > > > > > > /* For 64bit */
> > > > > > > > > GUEST_MMIO_HOLE1_BASE , GUEST_MMIO_HOLE1_SIZE
> > > > > > > > The memory layout for 32bit and 64bit are exactly the same. 
> > > > > > > > Why
> > > > > > > > do you
> > > > > > > > need to differ here?
> > > > > > > I think Ian has already replied. I will change the name of 
> > > > > > > macro
> > > > > > > > > 4.2 New entries in xenstore for device BARs
> > > > > > > > > 
> > > > > > > > > toolkit also updates the xenstore information for the 
> > > > > > > > > device
> > > > > > > > > (virtualbar:physical bar).
> > > > > > > > > This information is read by xenpciback and returned to 
> > > > > > > > > the
> > > > > > > > > pcifront
> > > > > > > > > driver configuration
> > > > > > > > > space accesses.
> > > > > > > > Can you details what do you plan to put in xenstore and 
> > > > > > > > how?
> > > > > > > It is implementation . But I plan to put under domU / device 
> > > > > > > /
> > > > > > > heirarchy
> > > > > > Actually, xenstore is an API of sorts which needs to be 
> > > > > > maintained
> > > > > > going
> > > > > > forward (since front and backend can evolve separately, so it 
> > > > > > does
> > > > > > need
> > > > > > some level of design and documentation.
> > > > > > 
> > > > > > > > What about the expansion ROM?
> > > > > > > Do you want to put some restriction on not using expansion 
> > > > > > > ROM as
> > > > > > > a
> > > > > > > passthrough device.
> > > > > > "expansion ROM as a passthrough device" doesn't make sense to 
> > > > > > me,
> > > > > > passthrough devices may _have_ an expansion ROM.
> > > > > > 
> > > > > > The expansion ROM is just another BAR. I don't know how
> > > > > > pcifront/back
> > > > > > deal with those today on PV x86, but I see no reason for ARM to
> > > > > > deviate.
> > > > > > 
> > > > > > 
> > > > > > > > > 4.3 Hypercall for bdf mapping notification to xen
> > > > > > > > > ---
> > > > > > > > > #define PHYSDEVOP_map_sbdf  43
> > > > > > > > > typedef struct {
> > > > > > > > > u32 s;
> > > > > > > > > u8 b;
> > > > > > > > > u8 df;
> > > > > > > > > u16 res;
> > > > > > > > > } sbdf_t;
> > > > > > > > > struct physdev_map_sbdf {
> > > > > > > > > int domain_id;
> > > > > > > > > sbdf_tsbdf;
> > > > > > > > > sbdf_tgsbdf;
> > > > > > > > > };
> > > > > > > > > 
> > > > > > > > > Each domain has a pdev list, which contains the list of 
> > > > > > > > > all
> > > > > > > > > pci devices.
> > > > > > > > > The
> > > > > > > > > pdev structure already has a sbdf information. The
> > > > > > > > > arch_pci_dev is
> > > > > > > > > updated to
> > > > > > > > > contain the gsbdf information. (gs- guest segment id)
> > > > > > > > > 
> > > > > > > > > Whenever there is trap from guest or an interrupt has to 
> > > > > > > > > be
> > > > > > > > > injected,
> > > > > > > > > the pdev
> > > > > > > > > list is iterated to find the gsbdf.
> > > > > > > > Can you give more background for this section? i.e:
> > > > > > > > - Why do you need this?
> > > > > > > > - How xen will translate the gbdf to a vDeviceID?
> > > > > > > In the context of the hypercall processing.
> > > > > > > > - Who will call this hypercall?
> > > > > > > > - Why not setting the gsbdf when the device is
> > > > > > > > assigned?
> > > > > > > Can the maintainer of the pciback suggest an alternate.
> > > > > > That's not me, but I don't think this belongs here, I think it 
> > > > > > can
> > > > > > be
> > > > > > done from the toolstack. If you think not then please explain 
> > > > > > what
> > > > > > information the toolstack doesn't have in its possession which
> > > > > > prevents
> > > > > > this mapping from being done there.
> > > > > The toolstack does not have the guest sbdf information. I could 
> > > > > only
> > > > > find it in x

Re: [Xen-devel] [RFC for-4.7] Switching to a single qemu tree each per qemu-xen and qemu-trad

[Andrew Cooper]

On 30/07/15 15:22, Ian Campbell wrote:
> (CC-ing 2x QEMU maintainers and stable release manager)
>
> The separate trees are a holdover from mercurial, which didn't (at the
> time) have a good in-repo branching model.
>
> I propose that Xen 4.6 should be the last release which uses these split
> trees and that instead we combine them into just qemu-xen.git (upstream)
> and qemu-xen-traditional.git (our old fork), with branches in those repos
> to match our releases. I picked those names to match the libxl interface
> names for these.

Very much +1 with my XenServer hat on.

>
> This will result in one place to get all the qemu stable branches for a
> given qemu type, without having to remember the naming scheme (which is
> counter-intuitive). It will allow easier cherry-picking and produce less
> places for users to look for our code etc.
>
> We could put all branches for both in the same tree but I think we want to
> just let qemu-xen-trad sit quietly in its own corner. Keeping it separate
> reinforces that it is almost completely frozen, also it saves me having to
> think up a branch naming scheme within a single repo.
>
> A proposed mapping for the two tree scheme which broadly follows the
> xen.git scheme is (paths relative to xenbits git root):
>
> qemu-xen-traditional:
>
> /staging/qemu-xen-unstable.git#master => /qemu-xen-traditional.git#staging
> /staging/qemu-xen-X.Y-testing.git#master => 
> /qemu-xen-traditional.git#staging-X.Y
> /qemu-xen-unstable.git#master => /qemu-xen-traditional.git#master
> /qemu-xen-X.Y-testing.git#master => /qemu-xen-traditional.git#stable-X.Y

And take the change to drop the other random branches in the repository 
By my observations:

$ git remote show upstream
* remote upstream
  ...
  Remote branches:
ide-write-cache
iwj.block-extendable-flag
origin
qemu
upstream
vga-reverse-merge
xen.netscriptenv


>
> NB the revision to use is still referenced in xen.git/Config.mk for
> each branch, no change here.
>
> XXX why do staging/* exist, and what pushes from staging to the other?
> Should we ditch one or the other?

This is how staging used to work for the mecurial xen trees.  I presume
therefore that whatever used to be done for the Xen trees are still
being done for the current qemu trees.

>
> qemu-xen:
>
> /staging/qemu-upstream-unstable.git#master => /qemu-xen.git#staging
> /staging/qemu-upstream-X.Y-testing.git#master => /qemu-xen.git#staging-X.Y
> /qemu-upstream-unstable.git#master => /qemu-xen.git#master
> /qemu-upstream-X.Y-testing.git#master => /qemu-xen.git#stable-X.Y
>
> These are push gated by their respective qemu-upstream-* osstest flights.
>
> qemu-mainline:
>
> Upstream remains git://git.qemu.org/qemu.git. 
>
> /osstest/qemu.git#mainline/xen-tested-master => 
> /qemu-xen.git#upstream-tested
>
> /osstest/qemu.git should be removed.
>
> NB this is the same qemu-xen.git as above.
>
> Below is an osstest patch which implements this proposed scheme (I've not
> actually created any trees).

LGTM.

Also suitable adjustments to the landing page at http://xenbits.xen.org/

~Andrew

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

Re: [Xen-devel] [RFC for-4.7] Switching to a single qemu tree each per qemu-xen and qemu-trad

[Ian Campbell]

On Thu, 2015-07-30 at 15:33 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[RFC for-4.7] Switching to a single qemu tree each 
> per qemu-xen and qemu-trad"):
> ...
> > qemu-xen-traditional:
> ...
> > XXX why do staging/* exist, and what pushes from staging to the 
> > other?
> > Should we ditch one or the other?
> 
> Once upon a time, some of these had their own push gate.  They are
> nowadays always pushed together.  The staging branches should be
> abolished.

Thanks, I suspect as much.

> > The main open question is what to do about the existing split repos for
> > existing stable branches. We could:
> > 
> >   * Teach osstest (ap-push) to push to the old tree as well as the 
> > new
> > for existing (<= 4.6) branches only.
> >   * Push a Config.mk update to every stable branch and retire the
> > existing trees on the next relevant point release, if there is 
> > one.
> 
> We could do both of these, so we have a transitional period.

That could work, yes.

e.g. perhaps do the dual push until the corresponding point release
happens?

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

Re: [Xen-devel] [RFC for-4.7] Switching to a single qemu tree each per qemu-xen and qemu-trad

[Vitaly Kuznetsov]

Ian Campbell  writes:

> (CC-ing 2x QEMU maintainers and stable release manager)
>
> The separate trees are a holdover from mercurial, which didn't (at the
> time) have a good in-repo branching model.
>
> I propose that Xen 4.6 should be the last release which uses these split
> trees and that instead we combine them into just qemu-xen.git (upstream)
> and qemu-xen-traditional.git (our old fork)

(sorry my question is a bit unrelated)

is there a proposed time frame for the retirement of
qemu-xen-traditional? http://wiki.xenproject.org/wiki/QEMU_Upstream only
mentions VGA passthrough as a missing feature but maybe it can be fixed
for 4.7?

-- 
  Vitaly

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

Re: [Xen-devel] [RFC for-4.7] Switching to a single qemu tree each per qemu-xen and qemu-trad

[Ian Campbell]

On Thu, 2015-07-30 at 16:51 +0200, Vitaly Kuznetsov wrote:
> Ian Campbell  writes:
> 
> > (CC-ing 2x QEMU maintainers and stable release manager)
> > 
> > The separate trees are a holdover from mercurial, which didn't (at the
> > time) have a good in-repo branching model.
> > 
> > I propose that Xen 4.6 should be the last release which uses these 
> > split
> > trees and that instead we combine them into just qemu-xen.git 
> > (upstream)
> > and qemu-xen-traditional.git (our old fork)
> 
> (sorry my question is a bit unrelated)
> 
> is there a proposed time frame for the retirement of
> qemu-xen-traditional? http://wiki.xenproject.org/wiki/QEMU_Upstream only
> mentions VGA passthrough as a missing feature but maybe it can be fixed
> for 4.7?

AIUI people (from Intel?) are working on that.

Due to Windows activation issues which are triggered by changing the h/w
platform (aka QEMU) under the guest we are taking a long tail approach here
and not actively looking to remove qemu-xen-traditional so that people with
VMs installed under old versions can choose the stick with that h/w
platform even on newer Xen.

Eventually, some long time down the line, we may decide that the chances
that anyone is still running such a VM is negligible and we can remove it.

In the meantime qemu-xen-traditional is essentially frozen, no new features
and only adjustments to e.g. keep up with changes to the control libraries
etc are made.

Ian.

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

Re: [Xen-devel] [PATCH v2] build: use correct qemu path in systemd service file and init script

[Ian Campbell]

On Thu, 2015-07-30 at 11:30 +0100, Wei Liu wrote:
> On Thu, Jul 30, 2015 at 11:24:47AM +0100, Ian Campbell wrote:
> > On Thu, 2015-07-30 at 14:51 +0800, Ting-Wei Lan wrote:
> > > When --with-system-qemu is used, it is possible that we cannot find
> > > qemu-system-i386 in LIBEXEC_BIN, which can cause error in xencommons
> > > init script and xen-qemu-dom0-disk-backend.service systemd service.
> > > 
> > > Signed-off-by: Ting-Wei Lan 
> > 
> > Personally I would have omitted the distinction between @qemu_xen_path@ 
> > and
> > @qemu_xen_systemd@ and just put the env invocation in the service file 
> > as
> > "/usr/bin/env @qemu_xen_path@" but I suppose that is just bike 
> > shedding,
> > so:
> > 
> > Acked-by: Ian Campbell 
> > 
> > Wei Lui, what do you think about this for 4.6? It fixes a real issue 
> > where 
> > --with-system-qemu is used without an explicit path, which is supposed 
> > to
> > search for "qemu" in $PATH but fails to do so for the initscripts and 
> > unit
> > files, where it uses the old hardcoded default value instead, which
> > probably doesn't exist if you are using this option (and if it did 
> > isn't
> > the thing the user asked for).
> > 
> > The fix looks pretty straight forward to me.
> > 
> 
> I agree with you. It should be applied for 4.6.

Thanks, applied.

Ting-Wei: I got a reject in xencommons.in because your tree apparently
lacks 8e986e5a61ef from May. Please check I've resolved it correctly, and
please use a more up to date baseline for future patches.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH for-4.6 0/2] Replace FSF street address with canonical URL

[Ian Campbell]

On Thu, 2015-07-30 at 10:56 +0100, Wei Liu wrote:
> These are only mechanical changes and I deem this fall into
> "keep documentation up to date" category, so with my RM hat on.
> 
> Release-acked-by: Wei Liu 

Thanks, I also ran this by Ian J and he saw no reason not to go ahead.

So I have merged from
git://xenbits.xen.org/people/ianc/xen.git fsf-address-v1

Thanks,
Ian.

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

[Xen-devel] 4.6 release bug lists

[Wei Liu]

Hi all

Here are some bugs that I'm aware of. The name after a item is the
person who is working on it.

# Blockers

1. Toolstack record breaks migration v2 (Andrew Cooper)
2. Block hotplug scripts invoked multiple times (Roger Pau Monne)


# Non-blockers

1. QEMU block script doesn't work (George Dunlap)

If you think I miss other bugs (very likely because I haven't gone
through my xen-devel archive), please reply to this email.

We would start cutting RCs after all blocker bugs are fixed.

Wei.

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

Re: [Xen-devel] 4.6 release bug lists

[Roger Pau Monné]

El 30/07/15 a les 17.04, Wei Liu ha escrit:
> Hi all
> 
> Here are some bugs that I'm aware of. The name after a item is the
> person who is working on it.
> 
> # Blockers
> 
> 1. Toolstack record breaks migration v2 (Andrew Cooper)
> 2. Block hotplug scripts invoked multiple times (Roger Pau Monne)

This "issue" is due to stale udev rules from previous installations. A
clean install or a update from a packaged Xen from a distro should work
fine. We could consider installing an empty udev rules file to replace
the previous one, but IMHO this is kind of crappy.

Then Konrad was also seeing timeout errors from hotplug scripts
execution, and we are still looking into the cause of this.

> 
> # Non-blockers
> 
> 1. QEMU block script doesn't work (George Dunlap)
> 
> If you think I miss other bugs (very likely because I haven't gone
> through my xen-devel archive), please reply to this email.

I'm quite sure this should be a blocker:

http://marc.info/?l=xen-devel&m=143816508010982

Roger.


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

Re: [Xen-devel] [RFC for-4.7] Switching to a single qemu tree each per qemu-xen and qemu-trad

[Ian Jackson]

Andrew Cooper writes ("Re: [Xen-devel] [RFC for-4.7] Switching to a single qemu 
tree each per qemu-xen and qemu-trad"):
> And take the change to drop the other random branches in the repository 
> By my observations:
> 
> $ git remote show upstream
> * remote upstream
>   ...
>   Remote branches:
> ide-write-cache
> iwj.block-extendable-flag
> origin
> qemu
> upstream
> vga-reverse-merge
> xen.netscriptenv

Yes, all of that can be got rid of.  I'm not even sure how much of
that got there.

Ian.

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

[Xen-devel] [PATCH for-4.6] xl/libxl: disable PV vNUMA

[Wei Liu]

Update xl manual and disable PV vNUMA in libxl.

Signed-off-by: Wei Liu 
---
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Dario Faggioli 
---
 docs/man/xl.cfg.pod.5  | 4 
 tools/libxl/libxl_create.c | 9 +
 2 files changed, 13 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index c78c3ba..782106c 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -261,6 +261,10 @@ Specify virtual NUMA configuration with positional 
arguments. The
 nth B in the list specifies the configuration of nth
 virtual node.
 
+Note that virtual NUMA for PV guest is not yet supported, because
+there is issue with regard to cpuid handling that affects PV virtual
+NUMA.
+
 Each B is a list, which has a form of
 "[VNODE_CONFIG_OPTION,VNODE_CONFIG_OPTION, ... ]"  (without quotes).
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4f2f50b..4f4273d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -860,6 +860,15 @@ static void initiate_domain_create(libxl__egc *egc,
 goto error_out;
 }
 
+/* PV vNUMA is not yet supported because there is issue with
+ * regard to cpuid handling.
+ */
+if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&
+d_config->b_info.num_vnuma_nodes) {
+LOG(ERROR, "PV vNUMA not yet supported");
+goto error_out;
+}
+
 ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
 if (ret) {
 LOG(ERROR, "Unable to set domain create info defaults");
-- 
2.1.4


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

Re: [Xen-devel] [PATCH for-4.6] xl/libxl: disable PV vNUMA

[Roger Pau Monné]

El 30/07/15 a les 17.12, Wei Liu ha escrit:
> Update xl manual and disable PV vNUMA in libxl.
> 
> Signed-off-by: Wei Liu 
> ---
> Cc: Ian Campbell 
> Cc: Ian Jackson 
> Cc: Dario Faggioli 
> ---
>  docs/man/xl.cfg.pod.5  | 4 
>  tools/libxl/libxl_create.c | 9 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index c78c3ba..782106c 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -261,6 +261,10 @@ Specify virtual NUMA configuration with positional 
> arguments. The
>  nth B in the list specifies the configuration of nth
>  virtual node.
>  
> +Note that virtual NUMA for PV guest is not yet supported, because
> +there is issue with regard to cpuid handling that affects PV virtual
   ^ an   ^regarding
> +NUMA.
> +
>  Each B is a list, which has a form of
>  "[VNODE_CONFIG_OPTION,VNODE_CONFIG_OPTION, ... ]"  (without quotes).
>  
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 4f2f50b..4f4273d 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -860,6 +860,15 @@ static void initiate_domain_create(libxl__egc *egc,
>  goto error_out;
>  }
>  
> +/* PV vNUMA is not yet supported because there is issue with
> + * regard to cpuid handling.

Same.

> + */
> +if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&
> +d_config->b_info.num_vnuma_nodes) {
> +LOG(ERROR, "PV vNUMA not yet supported");
> +goto error_out;
> +}
> +
>  ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
>  if (ret) {
>  LOG(ERROR, "Unable to set domain create info defaults");
> 


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

Re: [Xen-devel] [PATCH v2] build: use correct qemu path in systemd service file and init script

[Doug Goldstein]

On Thu, Jul 30, 2015 at 6:35 AM, Ian Campbell  wrote:
> On Thu, 2015-07-30 at 12:23 +0100, Anthony PERARD wrote:
>> On Thu, Jul 30, 2015 at 11:24:47AM +0100, Ian Campbell wrote:
>> > On Thu, 2015-07-30 at 14:51 +0800, Ting-Wei Lan wrote:
>> > > When --with-system-qemu is used, it is possible that we cannot find
>> > > qemu-system-i386 in LIBEXEC_BIN, which can cause error in xencommons
>> > > init script and xen-qemu-dom0-disk-backend.service systemd service.
>> > >
>> > > Signed-off-by: Ting-Wei Lan 
>> >
>> > Personally I would have omitted the distinction between @qemu_xen_path@
>> > and
>> > @qemu_xen_systemd@ and just put the env invocation in the service file
>> > as
>> > "/usr/bin/env @qemu_xen_path@" but I suppose that is just bike
>> > shedding,
>> > so:
>> >
>> > Acked-by: Ian Campbell 
>> >
>> > Wei Lui, what do you think about this for 4.6? It fixes a real issue
>> > where
>> > --with-system-qemu is used without an explicit path, which is supposed
>> > to
>> > search for "qemu" in $PATH but fails to do so for the initscripts and
>> > unit
>> > files, where it uses the old hardcoded default value instead, which
>> > probably doesn't exist if you are using this option (and if it did
>> > isn't
>> > the thing the user asked for).
>> >
>> > The fix looks pretty straight forward to me.
>> >
>> > Mostly unrelated, is "qemu" a sensible default here? No binary package
>> > on
>> > Debian actually provides a "qemu" binary, they are all qemu-system-foo
>> > or
>> > variants. I'm not sure if that's just a Debian packaging issue though.
>> > I've
>> > added the Qemu-xen maintainers for input...
>>
>> QEMU does not build a binary called 'qemu', so this would be package
>> specific. So I think a default should be either 'qemu-system-i386' or
>> fail if no path is provided.
>
> Thanks. I think we should take Ting-Wei's patch as is for 4.6 and fixup the
> default to be qemu-system-i386 in 4.7.
>
> Ian.
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Ian,

I would actually advise against using the name "qemu" because upstream
explicitly asked distros not to use that since 1.0.

http://wiki.qemu.org/ChangeLog/1.0

You should always use "qemu-system-i386".

-- 
Doug Goldstein

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

Re: [Xen-devel] [PATCH for-4.6] xl/libxl: disable PV vNUMA

[Wei Liu]

On Thu, Jul 30, 2015 at 05:30:58PM +0200, Roger Pau Monné wrote:
> El 30/07/15 a les 17.12, Wei Liu ha escrit:
> > Update xl manual and disable PV vNUMA in libxl.
> > 
> > Signed-off-by: Wei Liu 
> > ---
> > Cc: Ian Campbell 
> > Cc: Ian Jackson 
> > Cc: Dario Faggioli 
> > ---
> >  docs/man/xl.cfg.pod.5  | 4 
> >  tools/libxl/libxl_create.c | 9 +
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > index c78c3ba..782106c 100644
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -261,6 +261,10 @@ Specify virtual NUMA configuration with positional 
> > arguments. The
> >  nth B in the list specifies the configuration of nth
> >  virtual node.
> >  
> > +Note that virtual NUMA for PV guest is not yet supported, because
> > +there is issue with regard to cpuid handling that affects PV virtual
>^ an   ^regarding

Added "an", but "with regard to" is a commonly used phrase.

Did I miss anything?

Wei.

> > +NUMA.
> > +
> >  Each B is a list, which has a form of
> >  "[VNODE_CONFIG_OPTION,VNODE_CONFIG_OPTION, ... ]"  (without quotes).
> >  
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index 4f2f50b..4f4273d 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -860,6 +860,15 @@ static void initiate_domain_create(libxl__egc *egc,
> >  goto error_out;
> >  }
> >  
> > +/* PV vNUMA is not yet supported because there is issue with
> > + * regard to cpuid handling.
> 
> Same.
> 
> > + */
> > +if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&
> > +d_config->b_info.num_vnuma_nodes) {
> > +LOG(ERROR, "PV vNUMA not yet supported");
> > +goto error_out;
> > +}
> > +
> >  ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
> >  if (ret) {
> >  LOG(ERROR, "Unable to set domain create info defaults");
> > 

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

Re: [Xen-devel] 4.6 release bug lists

[Wei Liu]

On Thu, Jul 30, 2015 at 05:10:10PM +0200, Roger Pau Monné wrote:
> El 30/07/15 a les 17.04, Wei Liu ha escrit:
> > Hi all
> > 
> > Here are some bugs that I'm aware of. The name after a item is the
> > person who is working on it.
> > 
> > # Blockers
> > 
> > 1. Toolstack record breaks migration v2 (Andrew Cooper)
> > 2. Block hotplug scripts invoked multiple times (Roger Pau Monne)
> 
> This "issue" is due to stale udev rules from previous installations. A
> clean install or a update from a packaged Xen from a distro should work
> fine. We could consider installing an empty udev rules file to replace
> the previous one, but IMHO this is kind of crappy.
> 
> Then Konrad was also seeing timeout errors from hotplug scripts
> execution, and we are still looking into the cause of this.
> 
> > 
> > # Non-blockers
> > 
> > 1. QEMU block script doesn't work (George Dunlap)
> > 
> > If you think I miss other bugs (very likely because I haven't gone
> > through my xen-devel archive), please reply to this email.
> 
> I'm quite sure this should be a blocker:
> 
> http://marc.info/?l=xen-devel&m=143816508010982
> 

Yes, that is pretty bad. It should be a blocker.

I'm going to put your name and Paul's in that item.

Wei.

> Roger.

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

Re: [Xen-devel] [PATCH v5 0/4] x86: modify_ldt improvement, test, and config option

[Boris Ostrovsky]

On 07/28/2015 01:29 AM, Andy Lutomirski wrote:

This is intended for x86/urgent.  Sorry for taking so long, but it
seemed nice to avoid breaking Xen.

This fixes the "dazed and confused" issue which was exposed by the
CVE-2015-5157 fix.  It's also probably a good general attack surface
reduction, and it replaces some scary code with IMO less scary code.

Also, servers and embedded systems should probably turn off modify_ldt.
This makes that possible.

Boris, could I get a Tested-by, assuming this works for you?


As far as Xen guests are concerned,

Tested-by: Boris Ostrovsky 

But ldt_gdt_32 test segfaults on 64-bit kernels. Baremetal and virt. I 
thought it worked for me before but can't reproduce this with older 
patches. Does it work for you?



-boris




Willy and Kees: I left the config option alone.  The -tiny people will
like it, and we can always add a sysctl of some sort later.

Changes from v4:
  - Fix Xen even better (patch 1 is new).
  - Reorder the patches to make a little more sense.

Changes from v3:
  - Hopefully fixed Xen.
  - Fixed 32-bit test case on 32-bit native kernel.
  - Fix bogus vumnap for some LDT sizes.
  - Strengthen test case to check all LDT sizes (catches bogus vunmap).
  - Lots of cleanups, mostly from Borislav.
  - Simplify IPI code using on_each_cpu_mask.

Changes from v2:
  - Allocate ldt_struct and the LDT entries separately.  This should fix Xen.
  - Stop using write_ldt_entry, since I'm pretty sure it's unnecessary now
that we no longer mutate an in-use LDT.  (Xen people, can you check?)

Changes from v1:
  - The config option is new.
  - The test case is new.
  - Fixed a missing allocation failure check.
  - Fixed a use-after-free on fork().

Andy Lutomirski (4):
   x86/xen: Unmap aliases in xen_alloc_ldt and xen_free_ldt
   x86/ldt: Make modify_ldt synchronous
   selftests/x86, x86/ldt: Add a selftest for modify_ldt
   x86/ldt: Make modify_ldt optional

  arch/x86/Kconfig  |  17 ++
  arch/x86/include/asm/desc.h   |  15 -
  arch/x86/include/asm/mmu.h|   5 +-
  arch/x86/include/asm/mmu_context.h|  68 -
  arch/x86/kernel/Makefile  |   3 +-
  arch/x86/kernel/cpu/common.c  |   4 +-
  arch/x86/kernel/cpu/perf_event.c  |  16 +-
  arch/x86/kernel/ldt.c | 262 +
  arch/x86/kernel/process_64.c  |   6 +-
  arch/x86/kernel/step.c|   8 +-
  arch/x86/power/cpu.c  |   3 +-
  arch/x86/xen/enlighten.c  |  12 +
  kernel/sys_ni.c   |   1 +
  tools/testing/selftests/x86/Makefile  |   2 +-
  tools/testing/selftests/x86/ldt_gdt.c | 520 ++
  15 files changed, 787 insertions(+), 155 deletions(-)
  create mode 100644 tools/testing/selftests/x86/ldt_gdt.c




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

Re: [Xen-devel] [PATCH for-4.6] xl/libxl: disable PV vNUMA

[Andrew Cooper]

On 30/07/15 16:45, Wei Liu wrote:
> On Thu, Jul 30, 2015 at 05:30:58PM +0200, Roger Pau Monné wrote:
>> El 30/07/15 a les 17.12, Wei Liu ha escrit:
>>> Update xl manual and disable PV vNUMA in libxl.
>>>
>>> Signed-off-by: Wei Liu 
>>> ---
>>> Cc: Ian Campbell 
>>> Cc: Ian Jackson 
>>> Cc: Dario Faggioli 
>>> ---
>>>  docs/man/xl.cfg.pod.5  | 4 
>>>  tools/libxl/libxl_create.c | 9 +
>>>  2 files changed, 13 insertions(+)
>>>
>>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>>> index c78c3ba..782106c 100644
>>> --- a/docs/man/xl.cfg.pod.5
>>> +++ b/docs/man/xl.cfg.pod.5
>>> @@ -261,6 +261,10 @@ Specify virtual NUMA configuration with positional 
>>> arguments. The
>>>  nth B in the list specifies the configuration of nth
>>>  virtual node.
>>>  
>>> +Note that virtual NUMA for PV guest is not yet supported, because
>>> +there is issue with regard to cpuid handling that affects PV virtual
>>^ an   ^regarding
> Added "an", but "with regard to" is a commonly used phrase.

"with regards to" or "regarding" are both fine here, but the singular of
"regard" is not.

~Andrew

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

[Xen-devel] [linux-3.4 test] 60108: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 30511

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-sedf-pin  6 xen-boot   fail in 58831 pass in 58798
 test-amd64-i386-xl-qemuu-win7-amd64 9 windows-install fail in 58831 pass in 
60108
 test-amd64-i386-pair 10 xen-boot/dst_host   fail pass in 58831
 test-amd64-i386-pair  9 xen-boot/src_host   fail pass in 58831
 test-amd64-amd64-xl   6 xen-bootfail pass in 59576
 test-amd64-amd64-pair10 xen-boot/dst_host   fail pass in 59961
 test-amd64-amd64-libvirt  6 xen-bootfail pass in 60064
 test-amd64-amd64-pair 9 xen-boot/src_host   fail pass in 60064

Regressions which are regarded as allowable (not blocking):
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-i386-xl-xsm6 xen-bootfail baseline untested
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-i386-libvirt-xsm   6 xen-bootfail baseline untested
 test-amd64-amd64-xl-multivcpu  6 xen-boot   fail baseline untested
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail baseline 
untested
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-amd64-xl-xsm   6 xen-bootfail baseline untested
 test-amd64-amd64-xl-credit2   6 xen-bootfail baseline untested
 test-amd64-amd64-xl-rtds  6 xen-bootfail baseline untested
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm 6 xen-boot fail baseline untested
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 11 guest-saverestore fail 
baseline untested
 test-amd64-amd64-xl-sedf  6 xen-boot  fail in 58831 like 30406
 test-amd64-amd64-libvirt-xsm  6 xen-boot   fail in 59576 baseline untested
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 12 guest-localmigrate 
fail in 59576 baseline untested
 test-amd64-i386-libvirt  11 guest-start   fail in 59576 like 30511
 test-amd64-amd64-libvirt 11 guest-start   fail in 59576 like 30511
 test-amd64-i386-qemut-rhel6hvm-amd 12 guest-start/redhat.repeat fail in 60064 
blocked in 30511
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop fail in 60064 like 30511
 test-amd64-i386-xl-qemut-win7-amd64 15 guest-localmigrate/x10  fail like 30394
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 30511
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-bootfail like 53709-bisect
 test-amd64-i386-xl6 xen-bootfail like 53725-bisect
 test-amd64-i386-freebsd10-amd64  6 xen-boot fail like 58780-bisect
 test-amd64-i386-xl-qemuu-winxpsp3  6 xen-boot   fail like 58786-bisect
 test-amd64-i386-qemut-rhel6hvm-intel  6 xen-bootfail like 58788-bisect
 test-amd64-i386-rumpuserxen-i386  6 xen-bootfail like 58799-bisect
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1  6 xen-bootfail like 58801-bisect
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  6 xen-boot   fail like 58803-bisect
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-boot  fail like 58804-bisect
 test-amd64-i386-freebsd10-i386  6 xen-boot  fail like 58805-bisect
 test-amd64-i386-xl-qemuu-ovmf-amd64  6 xen-boot fail like 58806-bisect
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-boot  fail like 58807-bisect
 test-amd64-i386-xl-qemut-winxpsp3  6 xen-boot   fail like 58808-bisect
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1  6 xen-bootfail like 58809-bisect
 test-amd64-amd64-rumpuserxen-amd64  6 xen-boot  fail like 58810-bisect
 test-amd64-i386-xl-qemuu-debianhvm-amd64  6 xen-bootfail like 58811-bisect
 test-amd64-amd64-xl-qemut-debianhvm-amd64  6 xen-boot   fail like 58813-bisect
 test-amd64-i386-qemuu-rhel6hvm-intel  6 xen-bootfail like 58814-bisect
 test-amd64-i386-xl-qemut-debianhvm-amd64  6 xen-bootfail like 58815-bisect

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-libvirt 12 migrate-support-check fail in 60064 never pass
 test-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-amd64-libvirt-xsm 12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-amd64-i386-libvirt-raw  11 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never

Re: [Xen-devel] [PATCH v5 0/4] x86: modify_ldt improvement, test, and config option

[Borislav Petkov]

On Thu, Jul 30, 2015 at 11:53:34AM -0400, Boris Ostrovsky wrote:
> As far as Xen guests are concerned,
> 
> Tested-by: Boris Ostrovsky 

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still
debugging on the v4 thread? Or does that need more fixing?

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

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

[Xen-devel] [PATCH for-4.6 v2] xl/libxl: disable PV vNUMA

[Wei Liu]

Update xl manual and disable PV vNUMA in libxl.

Signed-off-by: Wei Liu 
---
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Dario Faggioli 

v2: improve doc and code comment.
---
 docs/man/xl.cfg.pod.5  | 3 +++
 tools/libxl/libxl_create.c | 9 +
 2 files changed, 12 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index c78c3ba..80e51bb 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -261,6 +261,9 @@ Specify virtual NUMA configuration with positional 
arguments. The
 nth B in the list specifies the configuration of nth
 virtual node.
 
+Note that virtual NUMA for PV guest is not yet supported, because
+there is an issue with cpuid handling that affects PV virtual NUMA.
+
 Each B is a list, which has a form of
 "[VNODE_CONFIG_OPTION,VNODE_CONFIG_OPTION, ... ]"  (without quotes).
 
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 4f2f50b..4b63559 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -860,6 +860,15 @@ static void initiate_domain_create(libxl__egc *egc,
 goto error_out;
 }
 
+/* PV vNUMA is not yet supported because there is an issue with
+ * cpuid handling.
+ */
+if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&
+d_config->b_info.num_vnuma_nodes) {
+LOG(ERROR, "PV vNUMA is not yet supported");
+goto error_out;
+}
+
 ret = libxl__domain_create_info_setdefault(gc, &d_config->c_info);
 if (ret) {
 LOG(ERROR, "Unable to set domain create info defaults");
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v5 0/4] x86: modify_ldt improvement, test, and config option

[Andrew Cooper]

On 30/07/15 17:05, Borislav Petkov wrote:
> On Thu, Jul 30, 2015 at 11:53:34AM -0400, Boris Ostrovsky wrote:
>> As far as Xen guests are concerned,
>>
>> Tested-by: Boris Ostrovsky 
> Does that mean, this patch 1/4 fixes the 32bit issue you guys are still
> debugging on the v4 thread? Or does that need more fixing?
>

I was going to say... This v5 pre-dates figuring out what was wrong with
32bit Xen.  v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?

~Andrew

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

[Xen-devel] [PATCH] x86, amd_ucode: Skip microcode updates for final levels

[Aravind Gopalakrishnan]

Some of older[Fam10h] systems require that the microcode versions
that it comes up with should not be updated by the microcode driver.
Otherwise, system hangs are known to occur.

In this patch, we check for those microcode versions and abort the
update process if existing microcode level is already applied by
the BIOS.

A linux version of the patch has already made it into tip-
http://marc.info/?l=linux-kernel&m=143703405627170

Signed-off-by: Aravind Gopalakrishnan 
---
 xen/arch/x86/microcode_amd.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index f79b397..c958a47 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -347,6 +347,30 @@ static int container_fast_forward(const void *data, size_t 
size_left, size_t *of
 return 0;
 }
 
+static unsigned int final_levels[] = {
+0x0198,
+0x019f,
+0x01af,
+0
+};
+
+static bool_t check_final_patch_levels(int cpu)
+{
+/*
+ * Check the current patch levels on the cpu. If they are equal to
+ * any of the 'final_levels', then we should not update the microcode
+ * patch on the cpu as system will hang otherwise.
+ */
+struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+int i;
+
+for ( i = 0; final_levels[i]; i++ )
+if ( uci->cpu_sig.rev == final_levels[i] )
+return 1;
+
+return 0;
+}
+
 static int cpu_request_microcode(int cpu, const void *buf, size_t bufsize)
 {
 struct microcode_amd *mc_amd, *mc_old;
@@ -369,6 +393,13 @@ static int cpu_request_microcode(int cpu, const void *buf, 
size_t bufsize)
 goto out;
 }
 
+if ( check_final_patch_levels(cpu) )
+{
+pr_debug("microcode: Cannot update microcode patch on the cpu as we 
hit a final level\n");
+error = -EPERM;
+goto out;
+}
+
 mc_amd = xmalloc(struct microcode_amd);
 if ( !mc_amd )
 {
-- 
1.9.1


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

Re: [Xen-devel] [PATCH v5 0/4] x86: modify_ldt improvement, test, and config option

[Boris Ostrovsky]

On 07/30/2015 12:12 PM, Andrew Cooper wrote:

On 30/07/15 17:05, Borislav Petkov wrote:

On Thu, Jul 30, 2015 at 11:53:34AM -0400, Boris Ostrovsky wrote:

As far as Xen guests are concerned,

Tested-by: Boris Ostrovsky 

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still
debugging on the v4 thread? Or does that need more fixing?


I was going to say... This v5 pre-dates figuring out what was wrong with
32bit Xen.  v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?



Only V5, no extra changes.

And perhaps dropping aliases in xen_alloc_ldt() may be sufficient since 
with that done we will only have one mapping so a subsequent fault will 
have "correct" cr2 provided by the hypervisor (from your earlier email 
it sounded that hypervisor may have been providing incorrect cr2 if 
alias exists)


-boris

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

Re: [Xen-devel] [PATCH for-4.6 4/8] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA

[Andrew Cooper]

On 29/07/15 12:28, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH for-4.6 4/8] docs/libxl: Re-specify 
> XENSTORE_DATA as EMULATOR_XENSTORE_DATA"):
>> The legacy "toolstack" record as implemented in libxl turns out not to
>> be 32/64bit safe.  As migration v2 has not shipped yet, take this
>> opportunity to adjust the specification and fix the incompatibility.
>>
>> Libxl shall loose all knowledge of the old "toolstack" blob and use this
>> EMULATOR_XENSTORE_DATA record instead.  Compatibility shall be handled
>> by the legacy conversion script.
> ...
>> -A record containing xenstore key/value pairs of data.
>> +A set of xenstore key/value pairs for a specific emulator associated with \
> the
>> +domain.
> Wrap damage (and elsewhere in this doc).

The entirely of this document is wrapped at 78 character, like all other
work I do in the Xen tree.

Furthermore this is compatible with both the Xen CODING_STYLE (< 80) and
the libxl CODING_STYLE (75-80).

~Andrew

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

Re: [Xen-devel] [PATCH for-4.6 4/8] docs/libxl: Re-specify XENSTORE_DATA as EMULATOR_XENSTORE_DATA

[Andrew Cooper]

On 29/07/15 10:35, David Vrabel wrote:
>
>> +EMULATOR\_XENSTORE\_DATA
>> +
>>  
>> -A record containing xenstore key/value pairs of data.
>> +A set of xenstore key/value pairs for a specific emulator associated with 
>> the
>> +domain.
>>  
>>   0 1 2 3 4 5 6 7 octet
>> -+-+
>> -| xenstore key/value pairs|
>> ++++
>> +| emulator_id| index  |
>> ++++
>> +| xenstore key/value data |
>>  ...
>>  +-+
>>  
>> +Xenstore key and value data are encoded as a pair of NUL terminated C
>> +strings.  Keys shall be relative to to the device models xenstore tree for 
>> the
>> +new domain
> This isn't quite descriptive enough, suggest:
>
>"Xenstore key/value data is encoded as a packed sequence of (key,
> value) tuples.  Each (key, value) tuple is a packed pair of NUL
> terminated UTF-8 encoded character strings.  The keys are relative
> to..."
>
> In particular, it is essential that character strings have a well
> defined encoding (I always recommend UTF-8) but the Xenstore protocol
> may specify a different encoding (perhaps ASCII?).

The best I can find is from xenstore.txt which says:

While xenstore and most tools and APIs are capable of dealing with
arbitrary binary data as values, this should generally be avoided.
Data should generally be human-readable for ease of management and
debugging; xenstore is not a high-performance facility and should be
used only for small amounts of control plane data.  Therefore xenstore
values should normally be 7-bit ASCII text strings containing bytes
0x20..0x7f only, and should not contain a trailing nul byte.

I don't think it is wise to constrain the record format further than
xenstore permits.

>   The data may also be
> better specified as a NULL-terminated octet sequence (rather than
> characters).

I will opt for octet sequence, and state that it should be encoded
suitably for xenstore, without actually being more specific.

~Andrew

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

Re: [Xen-devel] [PATCH] x86, amd_ucode: Skip microcode updates for final levels

[Boris Ostrovsky]

On 07/30/2015 12:23 PM, Aravind Gopalakrishnan wrote:

Some of older[Fam10h] systems require that the microcode versions
that it comes up with should not be updated by the microcode driver.
Otherwise, system hangs are known to occur.

In this patch, we check for those microcode versions and abort the
update process if existing microcode level is already applied by
the BIOS.

A linux version of the patch has already made it into tip-
http://marc.info/?l=linux-kernel&m=143703405627170

Signed-off-by: Aravind Gopalakrishnan 
---
  xen/arch/x86/microcode_amd.c | 31 +++
  1 file changed, 31 insertions(+)

diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index f79b397..c958a47 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -347,6 +347,30 @@ static int container_fast_forward(const void *data, size_t 
size_left, size_t *of
  return 0;
  }
  
+static unsigned int final_levels[] = {

+0x0198,
+0x019f,
+0x01af,
+0
+};


Are these values documented somewhere?

-boris



+
+static bool_t check_final_patch_levels(int cpu)
+{
+/*
+ * Check the current patch levels on the cpu. If they are equal to
+ * any of the 'final_levels', then we should not update the microcode
+ * patch on the cpu as system will hang otherwise.
+ */
+struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+int i;
+
+for ( i = 0; final_levels[i]; i++ )
+if ( uci->cpu_sig.rev == final_levels[i] )
+return 1;
+
+return 0;
+}
+
  static int cpu_request_microcode(int cpu, const void *buf, size_t bufsize)
  {
  struct microcode_amd *mc_amd, *mc_old;
@@ -369,6 +393,13 @@ static int cpu_request_microcode(int cpu, const void *buf, 
size_t bufsize)
  goto out;
  }
  
+if ( check_final_patch_levels(cpu) )

+{
+pr_debug("microcode: Cannot update microcode patch on the cpu as we hit a 
final level\n");
+error = -EPERM;
+goto out;
+}
+
  mc_amd = xmalloc(struct microcode_amd);
  if ( !mc_amd )
  {



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

Re: [Xen-devel] [RFC PATCH v3.1 1/2] xsplice: rfc.v3.1

[Johannes Erdfelt]

Thanks for the work on this. I had some comments and questions on the
latest draft.

On Mon, Jul 27, 2015, Konrad Rzeszutek Wilk  wrote:
> +#define XSPLICE_HOWTO_FLAG_PC_REL0x1 /* Is PC relative. */  
> +#define XSPLICE_HOWOT_FLAG_SIGN  0x2 /* Should the new value be treated 
> as signed value. */  

s/HOWOT/HOWTO/

> +struct xsplice_reloc_howto {  
> +uint32_thowto; /* XSPLICE_HOWTO_* */  
> +uint32_tflag; /* XSPLICE_HOWTO_FLAG_* */  
> +uint32_tsize; /* Size, in bytes, of the item to be relocated. */  
> +uint32_tr_shift; /* The value the final relocation is shifted right 
> by; used to drop unwanted data from the relocation. */  
> +uint64_tmask; /* Bitmask for which parts of the instruction or data 
> are replaced with the relocated value. */  
> +uint8_t pad[8]; /* Must be zero. */  
> +};  

I'm curious how r_shift and mask are used. I'm familiar with x86 and
x86_64 and I'm not sure how these fit in. Is this to support other
architectures?

> +### Trampoline (e9 opcode)
> +
> +The e9 opcode used for jmpq uses a 32-bit signed displacement. That means
> +we are limited to up to 2GB of virtual address to place the new code
> +from the old code. That should not be a problem since Xen hypervisor has
> +a very small footprint.
> +
> +However if we need - we can always add two trampolines. One at the 2GB
> +limit that calls the next trampoline.

I'm not sure I understand the concern. At least on x86_64, there is a
ton of unused virtual address space where the hypervisor image is
mapped. Why not simply map memory at the end of virtual address space?

There shouldn't be a concern with 2GB jumps then.

> +Please note there is a small limitation for trampolines in
> +function entries: The target function (+ trailing padding) must be able
> +to accomodate the trampoline. On x86 with +-2 GB relative jumps,
> +this means 5 bytes are  required.
> +
> +Depending on compiler settings, there are several functions in Xen that
> +are smaller (without inter-function padding).
> +
> + 
> +readelf -sW xen-syms | grep " FUNC " | \
> +awk '{ if ($3 < 5) print $3, $4, $5, $8 }'
> +
> +...
> +3 FUNC LOCAL wbinvd_ipi
> +3 FUNC LOCAL shadow_l1_index
> +...
> +
> +A compile-time check for, e.g., a minimum alignment of functions or a
> +runtime check that verifies symbol size (+ padding to next symbols) for
> +that in the hypervisor is advised.

Is this really necessary? The way Xen is currently compiled results in
functions being aligned at 16-byte boundaries. The extra space is padded
with NOPs. Even if a function is only 3 bytes, it still has at least 16
bytes of space to use.

This can be controlled with the -falign-functions option to gcc.

Also, there are ways to get a 5-byte NOP added before the function.
This is what the Linux kernel does for ftrace which is what the recent
Linux kernel live patching support is built on.

It seems like it would be easier to be explicit during the build process
than do runtime checks to ensure there is enough space.

> +### When to patch
> +
> +During the discussion on the design two candidates bubbled where
> +the call stack for each CPU would be deterministic. This would
> +minimize the chance of the patch not being applied due to safety
> +checks failing.

It would be nice to have the consistency model be more explicit.

Maybe using the terminology from this LKML post?

https://lkml.org/lkml/2014/11/7/354

> +To randezvous all the CPUs an barrier with an maximum timeout (which
> +could be adjusted), combined with forcing all other CPUs through the
> +hypervisor with IPIs, can be utilized to have all the CPUs be lockstep.

s/randezvous/rendezvous/

> +### Compiling the hypervisor code
> +
> +Hotpatch generation often requires support for compiling the target
> +with -ffunction-sections / -fdata-sections.  Changes would have to
> +be done to the linker scripts to support this.

Is this for correctness reasons?

I understand this would be a good idea to reduce the size of patches,
but I wanted to make sure I'm not missing something.

> +### Symbol names
> +
> +
> +Xen as it is now, has a couple of non-unique symbol names which will
> +make runtime symbol identification hard.  Sometimes, static symbols
> +simply have the same name in C files, sometimes such symbols get
> +included via header files, and some C files are also compiled
> +multiple times and linked under different names (guest_walk.c).

I'm not sure I understand the problem with static symbols. They aren't
visible outside of the .c file, so when performing the linking against
the target xen image, there shouldn't be any conflicts.

JE


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

Re: [Xen-devel] [PATCH] x86, amd_ucode: Skip microcode updates for final levels

[Andrew Cooper]

On 30/07/15 17:23, Aravind Gopalakrishnan wrote:
> Some of older[Fam10h] systems require that the microcode versions
> that it comes up with should not be updated by the microcode driver.
> Otherwise, system hangs are known to occur.
>
> In this patch, we check for those microcode versions and abort the
> update process if existing microcode level is already applied by
> the BIOS.
>
> A linux version of the patch has already made it into tip-
> http://marc.info/?l=linux-kernel&m=143703405627170
>
> Signed-off-by: Aravind Gopalakrishnan 
> ---
>  xen/arch/x86/microcode_amd.c | 31 +++
>  1 file changed, 31 insertions(+)
>
> diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
> index f79b397..c958a47 100644
> --- a/xen/arch/x86/microcode_amd.c
> +++ b/xen/arch/x86/microcode_amd.c
> @@ -347,6 +347,30 @@ static int container_fast_forward(const void *data, 
> size_t size_left, size_t *of
>  return 0;
>  }
>  

Please include the same comment as the Linux patch, explaining that
these microcode versions can't be updated from.

I would also like to see some documentation from AMD concerning this.

> +static unsigned int final_levels[] = {
> +0x0198,
> +0x019f,
> +0x01af,
> +0
> +};
> +
> +static bool_t check_final_patch_levels(int cpu)
> +{
> +/*
> + * Check the current patch levels on the cpu. If they are equal to
> + * any of the 'final_levels', then we should not update the microcode
> + * patch on the cpu as system will hang otherwise.
> + */
> +struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
> +int i;

unsigned

> +
> +for ( i = 0; final_levels[i]; i++ )

ARRAY_SIZE(), and drop the 0 on the end of the list.

> +if ( uci->cpu_sig.rev == final_levels[i] )
> +return 1;
> +
> +return 0;
> +}
> +
>  static int cpu_request_microcode(int cpu, const void *buf, size_t bufsize)
>  {
>  struct microcode_amd *mc_amd, *mc_old;
> @@ -369,6 +393,13 @@ static int cpu_request_microcode(int cpu, const void 
> *buf, size_t bufsize)
>  goto out;
>  }
>  
> +if ( check_final_patch_levels(cpu) )
> +{
> +pr_debug("microcode: Cannot update microcode patch on the cpu as we 
> hit a final level\n");

pr_debug() is compiled out completely.  I would suggest
printk(XENLOG_INFO instead.

~Andrew

> +error = -EPERM;
> +goto out;
> +}
> +
>  mc_amd = xmalloc(struct microcode_amd);
>  if ( !mc_amd )
>  {


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

[Xen-devel] [PATCHv3 05/10] xen/balloon: rationalize memory hotplug stats

[David Vrabel]

The stats used for memory hotplug make no sense and are fiddled with
in odd ways.  Remove them and introduce total_pages to track the total
number of pages (both populated and unpopulated) including those within
hotplugged regions (note that this includes not yet onlined pages).

This will be used in a subsequent commit (xen/balloon: only hotplug
additional memory if required) when deciding whether additional memory
needs to be hotplugged.

Signed-off-by: David Vrabel 
Reviewed-by: Daniel Kiper 
---
 drivers/xen/balloon.c | 75 +--
 include/xen/balloon.h |  5 +---
 2 files changed, 13 insertions(+), 67 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 7ecbbc5..932d232 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -194,21 +194,6 @@ static enum bp_state update_schedule(enum bp_state state)
 }
 
 #ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-static long current_credit(void)
-{
-   return balloon_stats.target_pages - balloon_stats.current_pages -
-   balloon_stats.hotplug_pages;
-}
-
-static bool balloon_is_inflated(void)
-{
-   if (balloon_stats.balloon_low || balloon_stats.balloon_high ||
-   balloon_stats.balloon_hotplug)
-   return true;
-   else
-   return false;
-}
-
 static struct resource *additional_memory_resource(phys_addr_t size)
 {
struct resource *res;
@@ -289,10 +274,7 @@ static enum bp_state reserve_additional_memory(long credit)
goto err;
}
 
-   balloon_hotplug -= credit;
-
-   balloon_stats.hotplug_pages += credit;
-   balloon_stats.balloon_hotplug = balloon_hotplug;
+   balloon_stats.total_pages += balloon_hotplug;
 
return BP_DONE;
   err:
@@ -308,11 +290,6 @@ static void xen_online_page(struct page *page)
 
__balloon_append(page);
 
-   if (balloon_stats.hotplug_pages)
-   --balloon_stats.hotplug_pages;
-   else
-   --balloon_stats.balloon_hotplug;
-
mutex_unlock(&balloon_mutex);
 }
 
@@ -329,32 +306,22 @@ static struct notifier_block xen_memory_nb = {
.priority = 0
 };
 #else
-static long current_credit(void)
+static enum bp_state reserve_additional_memory(long credit)
 {
-   unsigned long target = balloon_stats.target_pages;
-
-   target = min(target,
-balloon_stats.current_pages +
-balloon_stats.balloon_low +
-balloon_stats.balloon_high);
-
-   return target - balloon_stats.current_pages;
+   balloon_stats.target_pages = balloon_stats.current_pages;
+   return BP_DONE;
 }
+#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
 
-static bool balloon_is_inflated(void)
+static long current_credit(void)
 {
-   if (balloon_stats.balloon_low || balloon_stats.balloon_high)
-   return true;
-   else
-   return false;
+   return balloon_stats.target_pages - balloon_stats.current_pages;
 }
 
-static enum bp_state reserve_additional_memory(long credit)
+static bool balloon_is_inflated(void)
 {
-   balloon_stats.target_pages = balloon_stats.current_pages;
-   return BP_DONE;
+   return balloon_stats.balloon_low || balloon_stats.balloon_high;
 }
-#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
 
 static enum bp_state increase_reservation(unsigned long nr_pages)
 {
@@ -367,15 +334,6 @@ static enum bp_state increase_reservation(unsigned long 
nr_pages)
.domid= DOMID_SELF
};
 
-#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-   if (!balloon_stats.balloon_low && !balloon_stats.balloon_high) {
-   nr_pages = min(nr_pages, balloon_stats.balloon_hotplug);
-   balloon_stats.hotplug_pages += nr_pages;
-   balloon_stats.balloon_hotplug -= nr_pages;
-   return BP_DONE;
-   }
-#endif
-
if (nr_pages > ARRAY_SIZE(frame_list))
nr_pages = ARRAY_SIZE(frame_list);
 
@@ -438,15 +396,6 @@ static enum bp_state decrease_reservation(unsigned long 
nr_pages, gfp_t gfp)
.domid= DOMID_SELF
};
 
-#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
-   if (balloon_stats.hotplug_pages) {
-   nr_pages = min(nr_pages, balloon_stats.hotplug_pages);
-   balloon_stats.hotplug_pages -= nr_pages;
-   balloon_stats.balloon_hotplug += nr_pages;
-   return BP_DONE;
-   }
-#endif
-
if (nr_pages > ARRAY_SIZE(frame_list))
nr_pages = ARRAY_SIZE(frame_list);
 
@@ -636,6 +585,8 @@ static void __init balloon_add_region(unsigned long 
start_pfn,
   don't subtract from it. */
__balloon_append(page);
}
+
+   balloon_stats.total_pages += extra_pfn_end - start_pfn;
 }
 
 static int __init balloon_init(void)
@@ -653,6 +604,7 @@ static int __init balloon_init(void)
balloon_stats.target_pages  = balloon_stats.cur

[Xen-devel] [PATCHv3 02/10] xen/balloon: remove scratch page left overs

[David Vrabel]

Commit 0bb599fd30108883b00c7d4a226eeb49111e6932 (xen: remove scratch
frames for ballooned pages and m2p override) removed the use of the
scratch page for ballooned out pages.

Remove some left over function definitions.

Signed-off-by: David Vrabel 
Reviewed-by: Daniel Kiper 
---
 include/xen/balloon.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index a4c1c6a..cc2e1a7 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -29,9 +29,6 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages,
bool highmem);
 void free_xenballooned_pages(int nr_pages, struct page **pages);
 
-struct page *get_balloon_scratch_page(void);
-void put_balloon_scratch_page(void);
-
 struct device;
 #ifdef CONFIG_XEN_SELFBALLOONING
 extern int register_xen_selfballooning(struct device *dev);
-- 
2.1.4


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

[Xen-devel] [PATCHv3 08/10] xen/balloon: use hotplugged pages for foreign mappings etc.

[David Vrabel]

alloc_xenballooned_pages() is used to get ballooned pages to back
foreign mappings etc.  Instead of having to balloon out real pages,
use (if supported) hotplugged memory.

This makes more memory available to the guest and reduces
fragmentation in the p2m.

This is only enabled if the xen.balloon.hotplug_unpopulated sysctl is
set to 1.  This sysctl defaults to 0 in case the udev rules to
automatically online hotplugged memory do not exist.

Signed-off-by: David Vrabel 
---
v3:
- Add xen.balloon.hotplug_unpopulated sysctl to enable use of hotplug
  for unpopulated pages.
---
 drivers/xen/balloon.c | 87 +--
 include/xen/balloon.h |  1 +
 2 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 2a01da7..3094f38f 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -55,6 +55,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -71,6 +72,46 @@
 #include 
 #include 
 
+static int xen_hotplug_unpopulated;
+
+#ifdef CONFIG_XEN_BALLOON_MEMORY_HOTPLUG
+
+static int zero;
+static int one = 1;
+
+static struct ctl_table balloon_table[] = {
+   {
+   .procname   = "hotplug_unpopulated",
+   .data   = &xen_hotplug_unpopulated,
+   .maxlen = sizeof(int),
+   .mode   = 0644,
+   .proc_handler   = proc_dointvec_minmax,
+   .extra1 = &zero,
+   .extra2 = &one,
+   },
+   { }
+};
+
+static struct ctl_table balloon_root[] = {
+   {
+   .procname   = "balloon",
+   .mode   = 0555,
+   .child  = balloon_table,
+   },
+   { }
+};
+
+static struct ctl_table xen_root[] = {
+   {
+   .procname   = "xen",
+   .mode   = 0555,
+   .child  = balloon_root,
+   },
+   { }
+};
+
+#endif
+
 /*
  * balloon_process() state:
  *
@@ -99,6 +140,7 @@ static xen_pfn_t frame_list[PAGE_SIZE / sizeof(unsigned 
long)];
 
 /* List of ballooned pages, threaded through the mem_map array. */
 static LIST_HEAD(ballooned_pages);
+static DECLARE_WAIT_QUEUE_HEAD(balloon_wq);
 
 /* Main work function, always executed in process context. */
 static void balloon_process(struct work_struct *work);
@@ -127,6 +169,7 @@ static void __balloon_append(struct page *page)
list_add(&page->lru, &ballooned_pages);
balloon_stats.balloon_low++;
}
+   wake_up(&balloon_wq);
 }
 
 static void balloon_append(struct page *page)
@@ -242,7 +285,8 @@ static enum bp_state reserve_additional_memory(void)
int nid, rc;
unsigned long balloon_hotplug;
 
-   credit = balloon_stats.target_pages - balloon_stats.total_pages;
+   credit = balloon_stats.target_pages + balloon_stats.target_unpopulated
+   - balloon_stats.total_pages;
 
/*
 * Already hotplugged enough pages?  Wait for them to be
@@ -323,7 +367,7 @@ static struct notifier_block xen_memory_nb = {
 static enum bp_state reserve_additional_memory(void)
 {
balloon_stats.target_pages = balloon_stats.current_pages;
-   return BP_DONE;
+   return BP_ECANCELED;
 }
 #endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
 
@@ -517,6 +561,28 @@ void balloon_set_new_target(unsigned long target)
 }
 EXPORT_SYMBOL_GPL(balloon_set_new_target);
 
+static int add_ballooned_pages(int nr_pages)
+{
+   enum bp_state st;
+
+   if (xen_hotplug_unpopulated) {
+   st = reserve_additional_memory();
+   if (st != BP_ECANCELED) {
+   mutex_unlock(&balloon_mutex);
+   wait_event(balloon_wq,
+  !list_empty(&ballooned_pages));
+   mutex_lock(&balloon_mutex);
+   return 0;
+   }
+   }
+
+   st = decrease_reservation(nr_pages, GFP_USER);
+   if (st != BP_DONE)
+   return -ENOMEM;
+
+   return 0;
+}
+
 /**
  * alloc_xenballooned_pages - get pages that have been ballooned out
  * @nr_pages: Number of pages to get
@@ -527,26 +593,26 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
 {
int pgno = 0;
struct page *page;
+
mutex_lock(&balloon_mutex);
+
+   balloon_stats.target_unpopulated += nr_pages;
+
while (pgno < nr_pages) {
page = balloon_retrieve(true);
if (page) {
pages[pgno++] = page;
} else {
-   enum bp_state st;
-   st = decrease_reservation(nr_pages - pgno, GFP_USER);
-   if (st != BP_DONE)
+   ret = add_ballooned_pages(nr_pages - pgno);
+   if (ret < 0)
goto out_undo;
}
}
mutex_u

[Xen-devel] [PATCHv3 06/10] xen/balloon: only hotplug additional memory if required

[David Vrabel]

Now that we track the total number of pages (included hotplugged
regions), it is easy to determine if more memory needs to be
hotplugged.

Add a new BP_WAIT state to signal that the balloon process needs to
wait until kicked by the memory add notifier (when the new section is
onlined by userspace).

Signed-off-by: David Vrabel 
---
v3:
- Return BP_WAIT if enough sections are already hotplugged.

v2:
- New BP_WAIT status after adding new memory sections.
---
 drivers/xen/balloon.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 932d232..e8b45e8 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -75,12 +75,14 @@
  * balloon_process() state:
  *
  * BP_DONE: done or nothing to do,
+ * BP_WAIT: wait to be rescheduled,
  * BP_EAGAIN: error, go to sleep,
  * BP_ECANCELED: error, balloon operation canceled.
  */
 
 enum bp_state {
BP_DONE,
+   BP_WAIT,
BP_EAGAIN,
BP_ECANCELED
 };
@@ -167,6 +169,9 @@ static struct page *balloon_next_page(struct page *page)
 
 static enum bp_state update_schedule(enum bp_state state)
 {
+   if (state == BP_WAIT)
+   return BP_WAIT;
+
if (state == BP_ECANCELED)
return BP_ECANCELED;
 
@@ -231,12 +236,22 @@ static void release_memory_resource(struct resource 
*resource)
kfree(resource);
 }
 
-static enum bp_state reserve_additional_memory(long credit)
+static enum bp_state reserve_additional_memory(void)
 {
+   long credit;
struct resource *resource;
int nid, rc;
unsigned long balloon_hotplug;
 
+   credit = balloon_stats.target_pages - balloon_stats.total_pages;
+
+   /*
+* Already hotplugged enough pages?  Wait for them to be
+* onlined.
+*/
+   if (credit <= 0)
+   return BP_WAIT;
+
balloon_hotplug = round_up(credit, PAGES_PER_SECTION);
 
resource = additional_memory_resource(balloon_hotplug * PAGE_SIZE);
@@ -276,7 +291,7 @@ static enum bp_state reserve_additional_memory(long credit)
 
balloon_stats.total_pages += balloon_hotplug;
 
-   return BP_DONE;
+   return BP_WAIT;
   err:
release_memory_resource(resource);
return BP_ECANCELED;
@@ -306,7 +321,7 @@ static struct notifier_block xen_memory_nb = {
.priority = 0
 };
 #else
-static enum bp_state reserve_additional_memory(long credit)
+static enum bp_state reserve_additional_memory(void)
 {
balloon_stats.target_pages = balloon_stats.current_pages;
return BP_DONE;
@@ -473,7 +488,7 @@ static void balloon_process(struct work_struct *work)
if (balloon_is_inflated())
state = increase_reservation(credit);
else
-   state = reserve_additional_memory(credit);
+   state = reserve_additional_memory();
}
 
if (credit < 0)
-- 
2.1.4


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

[Xen-devel] [PATCHv3 09/10] x86/xen: export xen_alloc_p2m_entry()

[David Vrabel]

Rename alloc_p2m() to xen_alloc_p2m_entry() and export it.

This is useful for ensuring that a p2m entry is allocated (i.e., not a
shared missing or identity entry) so that subsequent set_phys_to_machine()
calls will require no further allocations.

Signed-off-by: David Vrabel 
---
v3:
- Make xen_alloc_p2m_entry() a nop on auto-xlate guests.
---
 arch/x86/include/asm/xen/page.h |  2 ++
 arch/x86/xen/p2m.c  | 19 +--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index c44a5d5..960b380 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -45,6 +45,8 @@ extern unsigned long *xen_p2m_addr;
 extern unsigned long  xen_p2m_size;
 extern unsigned long  xen_max_p2m_pfn;
 
+extern int xen_alloc_p2m_entry(unsigned long pfn);
+
 extern unsigned long get_phys_to_machine(unsigned long pfn);
 extern bool set_phys_to_machine(unsigned long pfn, unsigned long mfn);
 extern bool __set_phys_to_machine(unsigned long pfn, unsigned long mfn);
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 8b7f18e..0215897 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -503,7 +503,7 @@ static pte_t *alloc_p2m_pmd(unsigned long addr, pte_t 
*pte_pg)
  * the new pages are installed with cmpxchg; if we lose the race then
  * simply free the page we allocated and use the one that's there.
  */
-static bool alloc_p2m(unsigned long pfn)
+int xen_alloc_p2m_entry(unsigned long pfn)
 {
unsigned topidx, mididx;
unsigned long *top_mfn_p, *mid_mfn;
@@ -513,6 +513,9 @@ static bool alloc_p2m(unsigned long pfn)
unsigned long addr = (unsigned long)(xen_p2m_addr + pfn);
unsigned long p2m_pfn;
 
+   if (xen_feature(XENFEAT_auto_translated_physmap))
+   return 0;
+
topidx = p2m_top_index(pfn);
mididx = p2m_mid_index(pfn);
 
@@ -524,7 +527,7 @@ static bool alloc_p2m(unsigned long pfn)
/* PMD level is missing, allocate a new one */
ptep = alloc_p2m_pmd(addr, pte_pg);
if (!ptep)
-   return false;
+   return -ENOMEM;
}
 
if (p2m_top_mfn) {
@@ -541,7 +544,7 @@ static bool alloc_p2m(unsigned long pfn)
 
mid_mfn = alloc_p2m_page();
if (!mid_mfn)
-   return false;
+   return -ENOMEM;
 
p2m_mid_mfn_init(mid_mfn, p2m_missing);
 
@@ -567,7 +570,7 @@ static bool alloc_p2m(unsigned long pfn)
 
p2m = alloc_p2m_page();
if (!p2m)
-   return false;
+   return -ENOMEM;
 
if (p2m_pfn == PFN_DOWN(__pa(p2m_missing)))
p2m_init(p2m);
@@ -590,8 +593,9 @@ static bool alloc_p2m(unsigned long pfn)
free_p2m_page(p2m);
}
 
-   return true;
+   return 0;
 }
+EXPORT_SYMBOL(xen_alloc_p2m_entry);
 
 unsigned long __init set_phys_range_identity(unsigned long pfn_s,
  unsigned long pfn_e)
@@ -648,7 +652,10 @@ bool __set_phys_to_machine(unsigned long pfn, unsigned 
long mfn)
 bool set_phys_to_machine(unsigned long pfn, unsigned long mfn)
 {
if (unlikely(!__set_phys_to_machine(pfn, mfn))) {
-   if (!alloc_p2m(pfn))
+   int ret;
+
+   ret = xen_alloc_p2m_entry(pfn);
+   if (ret < 0)
return false;
 
return __set_phys_to_machine(pfn, mfn);
-- 
2.1.4


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

[Xen-devel] [PATCHv3 01/10] mm: memory hotplug with an existing resource

[David Vrabel]

Add add_memory_resource() to add memory using an existing "System RAM"
resource.  This is useful if the memory region is being located by
finding a free resource slot with allocate_resource().

Xen guests will make use of this in their balloon driver to hotplug
arbitrary amounts of memory in response to toolstack requests.

Signed-off-by: David Vrabel 
Cc: Andrew Morton 
---
 include/linux/memory_hotplug.h |  2 ++
 mm/memory_hotplug.c| 28 +---
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index 6ffa0ac..c76d371 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -11,6 +11,7 @@ struct zone;
 struct pglist_data;
 struct mem_section;
 struct memory_block;
+struct resource;
 
 #ifdef CONFIG_MEMORY_HOTPLUG
 
@@ -266,6 +267,7 @@ static inline void remove_memory(int nid, u64 start, u64 
size) {}
 extern int walk_memory_range(unsigned long start_pfn, unsigned long end_pfn,
void *arg, int (*func)(struct memory_block *, void *));
 extern int add_memory(int nid, u64 start, u64 size);
+extern int add_memory_resource(int nid, struct resource *resource);
 extern int zone_for_memory(int nid, u64 start, u64 size, int zone_default);
 extern int arch_add_memory(int nid, u64 start, u64 size);
 extern int offline_pages(unsigned long start_pfn, unsigned long nr_pages);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 003dbe4..169770a 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1224,23 +1224,21 @@ int zone_for_memory(int nid, u64 start, u64 size, int 
zone_default)
 }
 
 /* we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG */
-int __ref add_memory(int nid, u64 start, u64 size)
+int __ref add_memory_resource(int nid, struct resource *res)
 {
+   u64 start, size;
pg_data_t *pgdat = NULL;
bool new_pgdat;
bool new_node;
-   struct resource *res;
int ret;
 
+   start = res->start;
+   size = resource_size(res);
+
ret = check_hotplug_memory_range(start, size);
if (ret)
return ret;
 
-   res = register_memory_resource(start, size);
-   ret = -EEXIST;
-   if (!res)
-   return ret;
-
{   /* Stupid hack to suppress address-never-null warning */
void *p = NODE_DATA(nid);
new_pgdat = !p;
@@ -1290,6 +1288,22 @@ out:
mem_hotplug_done();
return ret;
 }
+EXPORT_SYMBOL_GPL(add_memory_resource);
+
+int __ref add_memory(int nid, u64 start, u64 size)
+{
+   struct resource *res;
+   int ret;
+
+   res = register_memory_resource(start, size);
+   if (!res)
+   return -EEXIST;
+
+   ret = add_memory_resource(nid, res);
+   if (ret < 0)
+   release_memory_resource(res);
+   return ret;
+}
 EXPORT_SYMBOL_GPL(add_memory);
 
 #ifdef CONFIG_MEMORY_HOTREMOVE
-- 
2.1.4


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

[Xen-devel] [PATCHv3 00/10] mm, xen/balloon: memory hotplug improvements

[David Vrabel]

The series improves the use of hotplug memory in the Xen balloon
driver.

- Reliably find a non-conflicting location for the hotplugged memory
  (this fixes memory hotplug in a number of cases, particularly in
  dom0).

- Use hotplugged memory for alloc_xenballooned_pages() (keeping more
  memory available for the domain and reducing fragmentation of the
  p2m).

Changes in v3:
- xen.balloon.hotplug_unpopulated sysctl to enable using hotplug to
  provide unpopulated (ballooned) pages.
- Fix xen_alloc_p2m_entry() for auto-translated guests.

Changes in v2:
- New BP_WAIT state to signal the balloon process to wait for
  userspace to online the new memory.
- Preallocate P2M entries in alloc_xenballooned_pages() so they do not
  need allocated later (in a context where GFP_KERNEL allocations are
  not possible).

David


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

[Xen-devel] [PATCHv3 07/10] xen/balloon: make alloc_xenballoon_pages() always allocate low pages

[David Vrabel]

All users of alloc_xenballoon_pages() wanted low memory pages, so
remove the option for high memory.

Signed-off-by: David Vrabel 
Reviewed-by: Daniel Kiper 
---
 arch/x86/xen/grant-table.c |  2 +-
 drivers/xen/balloon.c  | 21 -
 drivers/xen/grant-table.c  |  2 +-
 drivers/xen/privcmd.c  |  2 +-
 drivers/xen/xenbus/xenbus_client.c |  3 +--
 include/xen/balloon.h  |  3 +--
 6 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/arch/x86/xen/grant-table.c b/arch/x86/xen/grant-table.c
index 1580e7a..e079500 100644
--- a/arch/x86/xen/grant-table.c
+++ b/arch/x86/xen/grant-table.c
@@ -133,7 +133,7 @@ static int __init xlated_setup_gnttab_pages(void)
kfree(pages);
return -ENOMEM;
}
-   rc = alloc_xenballooned_pages(nr_grant_frames, pages, 0 /* lowmem */);
+   rc = alloc_xenballooned_pages(nr_grant_frames, pages);
if (rc) {
pr_warn("%s Couldn't balloon alloc %ld pfns rc:%d\n", __func__,
nr_grant_frames, rc);
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index e8b45e8..2a01da7 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -136,17 +136,16 @@ static void balloon_append(struct page *page)
 }
 
 /* balloon_retrieve: rescue a page from the balloon, if it is not empty. */
-static struct page *balloon_retrieve(bool prefer_highmem)
+static struct page *balloon_retrieve(bool require_lowmem)
 {
struct page *page;
 
if (list_empty(&ballooned_pages))
return NULL;
 
-   if (prefer_highmem)
-   page = list_entry(ballooned_pages.prev, struct page, lru);
-   else
-   page = list_entry(ballooned_pages.next, struct page, lru);
+   page = list_entry(ballooned_pages.next, struct page, lru);
+   if (require_lowmem && PageHighMem(page))
+   return NULL;
list_del(&page->lru);
 
if (PageHighMem(page))
@@ -522,24 +521,20 @@ EXPORT_SYMBOL_GPL(balloon_set_new_target);
  * alloc_xenballooned_pages - get pages that have been ballooned out
  * @nr_pages: Number of pages to get
  * @pages: pages returned
- * @highmem: allow highmem pages
  * @return 0 on success, error otherwise
  */
-int alloc_xenballooned_pages(int nr_pages, struct page **pages, bool highmem)
+int alloc_xenballooned_pages(int nr_pages, struct page **pages)
 {
int pgno = 0;
struct page *page;
mutex_lock(&balloon_mutex);
while (pgno < nr_pages) {
-   page = balloon_retrieve(highmem);
-   if (page && (highmem || !PageHighMem(page))) {
+   page = balloon_retrieve(true);
+   if (page) {
pages[pgno++] = page;
} else {
enum bp_state st;
-   if (page)
-   balloon_append(page);
-   st = decrease_reservation(nr_pages - pgno,
-   highmem ? GFP_HIGHUSER : GFP_USER);
+   st = decrease_reservation(nr_pages - pgno, GFP_USER);
if (st != BP_DONE)
goto out_undo;
}
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 62f591f..a4b702c 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -687,7 +687,7 @@ int gnttab_alloc_pages(int nr_pages, struct page **pages)
int i;
int ret;
 
-   ret = alloc_xenballooned_pages(nr_pages, pages, false);
+   ret = alloc_xenballooned_pages(nr_pages, pages);
if (ret < 0)
return ret;
 
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5a29616..59cfec9 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -401,7 +401,7 @@ static int alloc_empty_pages(struct vm_area_struct *vma, 
int numpgs)
if (pages == NULL)
return -ENOMEM;
 
-   rc = alloc_xenballooned_pages(numpgs, pages, 0);
+   rc = alloc_xenballooned_pages(numpgs, pages);
if (rc != 0) {
pr_warn("%s Could not alloc %d pfns rc:%d\n", __func__,
numpgs, rc);
diff --git a/drivers/xen/xenbus/xenbus_client.c 
b/drivers/xen/xenbus/xenbus_client.c
index 9ad3272..2a2da04 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -614,8 +614,7 @@ static int xenbus_map_ring_valloc_hvm(struct xenbus_device 
*dev,
if (!node)
return -ENOMEM;
 
-   err = alloc_xenballooned_pages(nr_grefs, node->hvm.pages,
-  false /* lowmem */);
+   err = alloc_xenballooned_pages(nr_grefs, node->hvm.pages);
if (err)
goto out_err;
 
diff --git a/include/xen/balloon.h b/include/xen/balloon.h
index c8aee7a..83efdeb 100644
--- a/include/xen/balloon.h
+++ b/include/xen/balloon.h
@@ -22,8 +22,7 @

[Xen-devel] [PATCHv3 03/10] x86/xen: discard RAM regions above the maximum reservation

[David Vrabel]

During setup, discard RAM regions that are above the maximum
reservation (instead of marking them as E820_UNUSABLE).  This allows
hotplug memory to be placed at these addresses.

Signed-off-by: David Vrabel 
Reviewed-by: Daniel Kiper 
---
 arch/x86/xen/setup.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index 55f388e..32910c5 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -646,6 +646,7 @@ char * __init xen_memory_setup(void)
phys_addr_t addr = map[i].addr;
phys_addr_t size = map[i].size;
u32 type = map[i].type;
+   bool discard = false;
 
if (type == E820_RAM) {
if (addr < mem_end) {
@@ -656,10 +657,11 @@ char * __init xen_memory_setup(void)
xen_add_extra_mem(addr, size);
xen_max_p2m_pfn = PFN_DOWN(addr + size);
} else
-   type = E820_UNUSABLE;
+   discard = true;
}
 
-   xen_align_and_add_e820_region(addr, size, type);
+   if (!discard)
+   xen_align_and_add_e820_region(addr, size, type);
 
map[i].addr += size;
map[i].size -= size;
-- 
2.1.4


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

[Xen-devel] [PATCHv3 04/10] xen/balloon: find non-conflicting regions to place hotplugged memory

[David Vrabel]

Instead of placing hotplugged memory at the end of RAM (which may
conflict with PCI devices or reserved regions) use allocate_resource()
to get a new, suitably aligned resource that does not conflict.

Signed-off-by: David Vrabel 
Reviewed-by: Daniel Kiper 
---
v3:
- Remove stale comment.
---
 drivers/xen/balloon.c | 73 +--
 1 file changed, 53 insertions(+), 20 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index fd93369..7ecbbc5 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -54,6 +54,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -208,26 +209,56 @@ static bool balloon_is_inflated(void)
return false;
 }
 
-/*
- * reserve_additional_memory() adds memory region of size >= credit above
- * max_pfn. New region is section aligned and size is modified to be multiple
- * of section size. Those features allow optimal use of address space and
- * establish proper alignment when this function is called first time after
- * boot (last section not fully populated at boot time contains unused memory
- * pages with PG_reserved bit not set; online_pages_range() does not allow page
- * onlining in whole range if first onlined page does not have PG_reserved
- * bit set). Real size of added memory is established at page onlining stage.
- */
+static struct resource *additional_memory_resource(phys_addr_t size)
+{
+   struct resource *res;
+   int ret;
+
+   res = kzalloc(sizeof(*res), GFP_KERNEL);
+   if (!res)
+   return NULL;
+
+   res->name = "System RAM";
+   res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
+
+   ret = allocate_resource(&iomem_resource, res,
+   size, 0, -1,
+   PAGES_PER_SECTION * PAGE_SIZE, NULL, NULL);
+   if (ret < 0) {
+   pr_err("Cannot allocate new System RAM resource\n");
+   kfree(res);
+   return NULL;
+   }
+
+   return res;
+}
+
+static void release_memory_resource(struct resource *resource)
+{
+   if (!resource)
+   return;
+
+   /*
+* No need to reset region to identity mapped since we now
+* know that no I/O can be in this region
+*/
+   release_resource(resource);
+   kfree(resource);
+}
 
 static enum bp_state reserve_additional_memory(long credit)
 {
+   struct resource *resource;
int nid, rc;
-   u64 hotplug_start_paddr;
-   unsigned long balloon_hotplug = credit;
+   unsigned long balloon_hotplug;
+
+   balloon_hotplug = round_up(credit, PAGES_PER_SECTION);
+
+   resource = additional_memory_resource(balloon_hotplug * PAGE_SIZE);
+   if (!resource)
+   goto err;
 
-   hotplug_start_paddr = PFN_PHYS(SECTION_ALIGN_UP(max_pfn));
-   balloon_hotplug = round_up(balloon_hotplug, PAGES_PER_SECTION);
-   nid = memory_add_physaddr_to_nid(hotplug_start_paddr);
+   nid = memory_add_physaddr_to_nid(resource->start);
 
 #ifdef CONFIG_XEN_HAVE_PVMMU
 /*
@@ -242,21 +273,20 @@ static enum bp_state reserve_additional_memory(long 
credit)
if (!xen_feature(XENFEAT_auto_translated_physmap)) {
unsigned long pfn, i;
 
-   pfn = PFN_DOWN(hotplug_start_paddr);
+   pfn = PFN_DOWN(resource->start);
for (i = 0; i < balloon_hotplug; i++) {
if (!set_phys_to_machine(pfn + i, INVALID_P2M_ENTRY)) {
pr_warn("set_phys_to_machine() failed, no 
memory added\n");
-   return BP_ECANCELED;
+   goto err;
}
 }
}
 #endif
 
-   rc = add_memory(nid, hotplug_start_paddr, balloon_hotplug << 
PAGE_SHIFT);
-
+   rc = add_memory_resource(nid, resource);
if (rc) {
pr_warn("Cannot add additional memory (%i)\n", rc);
-   return BP_ECANCELED;
+   goto err;
}
 
balloon_hotplug -= credit;
@@ -265,6 +295,9 @@ static enum bp_state reserve_additional_memory(long credit)
balloon_stats.balloon_hotplug = balloon_hotplug;
 
return BP_DONE;
+  err:
+   release_memory_resource(resource);
+   return BP_ECANCELED;
 }
 
 static void xen_online_page(struct page *page)
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v5 12/22] xen/arm: ITS: Add GICR register emulation

[Julien Grall]

Hi Vijay,

On 27/07/15 12:11, vijay.kil...@gmail.com wrote:
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c

[..]

> +static int gicv3_dist_supports_lpis(void)
> +{
> +return readl_relaxed(GICD + GICD_TYPER) & GICD_TYPER_LPIS_SUPPORTED;
> +}
> +
>  static int __cpuinit gicv3_cpu_init(void)
>  {
>  int i;
> @@ -1274,6 +1279,11 @@ static int __init gicv3_init(void)
>  
>  spin_lock(&gicv3.lock);
>  
> +if ( gicv3_dist_supports_lpis() )
> +gicv3_info.lpi_supported = 1;
> +else
> +gicv3_info.lpi_supported = 0;
> +

You will avoid 3 lines if you do:

gicv3_info.lpi_supported = !!gicv3_dist_supports_lpis();

>  gicv3_dist_init();
>  res = gicv3_cpu_init();
>  gicv3_hyp_init();
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 1757193..af8a34b 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -67,6 +67,11 @@ unsigned int gic_number_lines(void)
>  return gic_hw_ops->info->nr_lines;
>  }
>  
> +bool_t gic_lpi_supported(void)
> +{
> +return gic_hw_ops->info->lpi_supported;
> +}
> +
>  void gic_save_state(struct vcpu *v)
>  {
>  ASSERT(!local_irq_is_enabled());
> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
> index 1c7d9b6..4afb62b 100644
> --- a/xen/arch/arm/vgic-v3-its.c
> +++ b/xen/arch/arm/vgic-v3-its.c

Can you explain why the emulation of the LPI property table has to be
done in the vITS code?

Overall, there is nothing vITS specific in this code and all the
functions you've introduced within this file are called only by the
vgic-v3 code.

> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -76,6 +77,34 @@ static inline uint32_t vits_get_max_collections(struct 
> domain *d)
>  return (d->max_vcpus + 1);
>  }
>  
> +static void vits_disable_lpi(struct vcpu *v, uint32_t vlpi)
> +{
> +struct pending_irq *p;
> +
> +p = irq_to_pending(v, vlpi);
> +clear_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> +gic_remove_from_queues(v, vlpi);
> +}
> +
> +static void vits_enable_lpi(struct vcpu *v, uint32_t vlpi, uint8_t priority)
> +{
> +struct pending_irq *p;
> +unsigned long flags;
> +
> +p = irq_to_pending(v, vlpi);
> +
> +set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
> +
> +spin_lock_irqsave(&v->arch.vgic.lock, flags);
> +
> +/*XXX: raise on right vcpu */

As said on the previous versions, I think there will be locking issue
given that pending_irq structure is protected by the vCPU where the IRQ
is locked.

If you think it's not the case please explain why but don't leave the
question unanswered.

> +if ( !list_empty(&p->inflight) &&
> + !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
> +gic_raise_guest_irq(v, vlpi, p->priority);
> +
> +spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
> +}
> +
>  static int vits_access_guest_table(struct domain *d, paddr_t entry, void 
> *addr,
> uint32_t size, bool_t set)
>  {
> @@ -551,6 +580,145 @@ static inline uint32_t vits_get_word(uint32_t 
> reg_offset, uint64_t val)
>  return (u32)(val >> 32);
>  }
>  
> +static int vgic_v3_gits_lpi_mmio_read(struct vcpu *v, mmio_info_t *info)
> +{
> +uint32_t offset;
> +struct vgic_its *vits = v->domain->arch.vgic.vits;
> +struct hsr_dabt dabt = info->dabt;
> +struct cpu_user_regs *regs = guest_cpu_user_regs();
> +register_t *r = select_user_reg(regs, dabt.reg);
> +
> +offset = info->gpa - (vits->propbase & MASK_4K);
> +
> +DPRINTK("%pv: vITS: LPI Table read offset 0x%"PRIx32"\n", v, offset);
> +spin_lock(&vits->prop_lock);

There is a potential deadlock with this lock here and the one in
vits_get_priority (see patch #16):

- The spin lock is taken in vgic_v3_gits_lpi_mmio_read
< LPI received on the same CPU
- vits_get_priority try to get the lock
=> Not possible because already locked

This could be avoid by disabling the IRQ when taking the spin lock and
reenable it when you release it.

> +if ( dabt.size == DABT_DOUBLE_WORD )

Please use a switch rather than if/else if/else if... and assuming the
last else the 8bit reading.

> +*r = *((u64*)vits->prop_page + offset);
> +else if (dabt.size == DABT_WORD )
> +*r = *((u32*)vits->prop_page + offset);
> +else if (dabt.size == DABT_HALF_WORD )
> +*r = *((u16*)vits->prop_page + offset);

The returned value won't be correct for 64, 32 and 16 bit read.

The cast is only applied to vits->prop_page, therefore the pointer will
be increment by offset * sizeof(uN).

You need to do the arithmetic on the pointer and then doing the cast.

> +else
> +*r = *((u8*)vits->prop_page + offset);
> +spin_unlock(&vits->prop_lock);
> +
> +return 1;
> +}
> +
> +static int vgic_v3_gits_lpi_mmio_write(struct vcpu *v, mmio_info_t *info)
> +{
> +uint32_t offset, vid;
> +uint8_t cfg, *p, i, 

Re: [Xen-devel] [PATCH v5 0/4] x86: modify_ldt improvement, test, and config option

[Andrew Cooper]

On 30/07/15 17:31, Boris Ostrovsky wrote:
> On 07/30/2015 12:12 PM, Andrew Cooper wrote:
>> On 30/07/15 17:05, Borislav Petkov wrote:
>>> On Thu, Jul 30, 2015 at 11:53:34AM -0400, Boris Ostrovsky wrote:
 As far as Xen guests are concerned,

 Tested-by: Boris Ostrovsky 
>>> Does that mean, this patch 1/4 fixes the 32bit issue you guys are still
>>> debugging on the v4 thread? Or does that need more fixing?
>>>
>> I was going to say... This v5 pre-dates figuring out what was wrong with
>> 32bit Xen.  v5 1/4 is still susceptible.
>>
>> Boris: does your Tested-by cover v5 + proposed fix?
>>
>
> Only V5, no extra changes.

Including running the ldt_gdt test?

>
> And perhaps dropping aliases in xen_alloc_ldt() may be sufficient
> since with that done we will only have one mapping so a subsequent
> fault will have "correct" cr2 provided by the hypervisor (from your
> earlier email it sounded that hypervisor may have been providing
> incorrect cr2 if alias exists)

They are sufficient to fix the first of the two bugs, but the free side
still has no protection against a missing l2, unless I am missing
something in the rest of the series?

~Andrew

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

Re: [Xen-devel] [PATCH v5 13/22] xen/arm: ITS: Implement gic_is_lpi helper function

[Julien Grall]

Hi Vijay,

On 27/07/15 12:11, vijay.kil...@gmail.com wrote:
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index a9a5874..f80f291 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -359,12 +359,14 @@ struct gic_hw_operations {
>  int (*secondary_init)(void);
>  int (*make_hwdom_dt_node)(const struct domain *d,
>const struct dt_device_node *node, void *fdt);
> +bool_t (*is_lpi)(unsigned int irq);

I would much prefer to see a new field nr_lpis in the gic_info structure
and get the check directly in gic_is_lpi.

>  };
>  
>  void register_gic_ops(const struct gic_hw_operations *ops);
>  int gic_make_hwdom_dt_node(const struct domain *d,
> const struct dt_device_node *node,
> void *fdt);
> +bool_t gic_is_lpi(unsigned int irq);
>  
>  #endif /* __ASSEMBLY__ */
>  #endif
> 

Regards,

-- 
Julien Grall

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

[Xen-devel] [PATCHv3 10/10] xen/balloon: pre-allocate p2m entries for ballooned pages

[David Vrabel]

Pages returned by alloc_xenballooned_pages() will be used for grant
mapping which will call set_phys_to_machine() (in PV guests).

Ballooned pages are set as INVALID_P2M_ENTRY in the p2m and thus may
be using the (shared) missing tables and a subsequent
set_phys_to_machine() will need to allocate new tables.

Since the grant mapping may be done from a context that cannot sleep,
the p2m entries must already be allocated.

Signed-off-by: David Vrabel 
---
 drivers/xen/balloon.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 3094f38f..e040cf4 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -593,6 +593,7 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
 {
int pgno = 0;
struct page *page;
+   int ret = -ENOMEM;
 
mutex_lock(&balloon_mutex);
 
@@ -602,6 +603,11 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
page = balloon_retrieve(true);
if (page) {
pages[pgno++] = page;
+#ifdef CONFIG_XEN_HAVE_PVMMU
+   ret = xen_alloc_p2m_entry(page_to_pfn(page));
+   if (ret < 0)
+   goto out_undo;
+#endif
} else {
ret = add_ballooned_pages(nr_pages - pgno);
if (ret < 0)
@@ -613,7 +619,7 @@ int alloc_xenballooned_pages(int nr_pages, struct page 
**pages)
  out_undo:
mutex_unlock(&balloon_mutex);
free_xenballooned_pages(pgno, pages);
-   return -ENOMEM;
+   return ret;
 }
 EXPORT_SYMBOL(alloc_xenballooned_pages);
 
-- 
2.1.4


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

Re: [Xen-devel] [PATCH v5 0/4] x86: modify_ldt improvement, test, and config option

[Boris Ostrovsky]

On 07/30/2015 01:06 PM, Andrew Cooper wrote:

On 30/07/15 17:31, Boris Ostrovsky wrote:

On 07/30/2015 12:12 PM, Andrew Cooper wrote:

On 30/07/15 17:05, Borislav Petkov wrote:

On Thu, Jul 30, 2015 at 11:53:34AM -0400, Boris Ostrovsky wrote:

As far as Xen guests are concerned,

Tested-by: Boris Ostrovsky 

Does that mean, this patch 1/4 fixes the 32bit issue you guys are still
debugging on the v4 thread? Or does that need more fixing?


I was going to say... This v5 pre-dates figuring out what was wrong with
32bit Xen.  v5 1/4 is still susceptible.

Boris: does your Tested-by cover v5 + proposed fix?


Only V5, no extra changes.

Including running the ldt_gdt test?


Yes, except that 32-on-64 doesn't work, but that's not Xen-specific.

Still, user-visible behavior changes.




And perhaps dropping aliases in xen_alloc_ldt() may be sufficient
since with that done we will only have one mapping so a subsequent
fault will have "correct" cr2 provided by the hypervisor (from your
earlier email it sounded that hypervisor may have been providing
incorrect cr2 if alias exists)

They are sufficient to fix the first of the two bugs, but the free side
still has no protection against a missing l2, unless I am missing
something in the rest of the series?


Without aliases a subsequent fault *will* fill correct l2, won't it?

-boris

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

Re: [Xen-devel] [PATCH for-4.6 5/8] tools/libxl: Save and restore EMULATOR_XENSTORE_DATA content

[Andrew Cooper]

On 29/07/15 12:49, Ian Jackson wrote:
> Andrew Cooper writes ("[PATCH for-4.6 5/8] tools/libxl: Save and restore 
> EMULATOR_XENSTORE_DATA content"):
>> The new EMULATOR_XENSTORE_DATA content is a sequence of NUL terminated
>> key/value strings, with the key relative to the device model's xenstore
>> tree.
>>
>> A sample might look like (as decoded by verify-stream-v2):
>>
>> Emulator Xenstore Data (Qemu Upstream, idx 0)
>>   'physmap/1f0/start_addr' = 'f000'
>>   'physmap/1f0/size' = '80'
>>   'physmap/1f0/name' = 'vga.vram'
>>
>> This patch introduces libxl helpers to save and restore this new format.
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Ian Campbell 
>> CC: Ian Jackson 
>> CC: Wei Liu 
>> ---
>>  tools/libxl/libxl_dom.c  |  141 
>> ++
>>  tools/libxl/libxl_internal.h |4 ++
>>  2 files changed, 145 insertions(+)
>>
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index fea..885eb5e 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -1151,6 +1151,65 @@ int libxl__toolstack_restore(uint32_t domid, const 
>> uint8_t *ptr,
>>  return ret;
>>  }
>>  
>> +int libxl__restore_emulator_xenstore_data(libxl__domain_create_state *dcs,
>> +  const char *ptr, uint32_t size)
>> +{
>> +STATE_AO_GC(dcs->ao);
>> +const char *key = ptr, *val;
>> +int rc;
>> +
>> +const uint32_t domid = dcs->guest_domid;
>> +const uint32_t dm_domid = libxl_get_stubdom_id(CTX, domid);
>> +const char *xs_root = libxl__device_model_xs_path(gc, dm_domid, domid\
> , "");
>
> I see wrap damage due to your overly long lines.  (Throughout.)
>
>> +
>> +while ( size ) {
> Coding style.  (Throughout.)
>
>> +/* Sanitise key. */
>> +size_t keylen = strnlen(key, size);
> I think this code suggests to me that the format chosen is not very
> nice.  But, anyway:
>
> Can we please have a black box subfunction which extracts the next
> nul-terminated string from this buffer ?  That would isolate all the
> pointer arithmetic and give it a formal interface.
>
> You might consider whether it is better to work with
>   const char *end = ptr + size;
> since that avoids the need to remember to update both ptr and size
> whenever walking through the array (a common source of security bugs).
>
>> +int libxl__save_emulator_xenstore_data(libxl__domain_suspend_state *dss,
>> +   uint8_t **callee_buf,
>> +   uint32_t *callee_len)
>> +{
> ...
>> +/* &foo[rel_start] is the xenstore path starting at the 'p' of physmap 
>> */
>> +rel_start = strlen(xs_path) - 7;
> This is horrible.

What do you recommend instead?

>
>> +entries = libxl__xs_directory(gc, 0, xs_path, &nr_entries);
>> +if (!entries || nr_entries == 0) { rc = 0; goto out; }
>> +
>> +for (i = 0; i < nr_entries; ++i) {
>> +const char *start_addr, *start_addr_val,
>> +*size, *size_val, *name, *name_val;
>> +
>> +start_addr = libxl__device_model_xs_path(
>> +gc, dm_domid, domid, "/physmap/%s/start_addr", entries[i]);
>> +size = libxl__device_model_xs_path(
>> +gc, dm_domid, domid, "/physmap/%s/size", entries[i]);
>> +name = libxl__device_model_xs_path(
>> +gc, dm_domid, domid, "/physmap/%s/name", entries[i]);
> I don't understand why you don't copy the whole subdirectory.  This
> needs to be explained.
>
> If you do want to copy only some of the keys, this thing where you do
> tiny bits of the work, each time three times, is a very strange
> structure.

I am replicating the behaviour of the old libxl__toolstack_save(), not
redesigning the logic from scratch.  (In an ideal world, all this cruft
would be limited to qemu alone, which is the sole producer and consumer,
rather than split across qemu, xenstore and libxl, but that boat has
long since sailed.)

>
>> +/*
>> + * Appends 's' to 'buf', including a NUL teriminator.  Mutates
>> + * 'buf', 'ptr' and 'len' in scope.
>> + */
>> +#define APPEND_STRING(s) ({ \
> Please, there is no need for this to be a macro.  If you need it, make
> it a function.  Yes, you'll have to pass some of its state to it.  But
> that means you may (usefully) write down what its assumptions are.
>
> Also, if you restructure this I think this macro will vanish.
>
>> +size_t sz = strlen(s) + 1;  \
>> +ptr = realloc(buf, len + sz);   \
>> +if (!ptr) { rc = ERROR_NOMEM; goto out; }   \
> Please use the gc.  I guess you are going to tell me that you can't
> use the gc because of the remus memory leak problem.  Do I need to go
> and retrofit the per-iteration sub-ao myself ?

Again, because that what older toolstack_save() did, but without the
bug