[Xen-devel] [PATCH] xen/apic: Update the comment for apic_id_mask

[Wei Jiangang]

verify_local_APIC() had been removed by
commit 4399c03c6780 ("x86/apic: Remove verify_local_APIC()"),
so apic_id_mask isn't used by it.

Signed-off-by: Wei Jiangang 
---
 arch/x86/xen/apic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c
index db52a7fafcc2..9cbb1f48381b 100644
--- a/arch/x86/xen/apic.c
+++ b/arch/x86/xen/apic.c
@@ -177,7 +177,7 @@ static struct apic xen_pv_apic = {
 
.get_apic_id= xen_get_apic_id,
.set_apic_id= xen_set_apic_id, /* Can be NULL on 
32-bit. */
-   .apic_id_mask   = 0xFF << 24, /* Used by 
verify_local_APIC. Match with what xen_get_apic_id does. */
+   .apic_id_mask   = 0xFF << 24, /* Match with what 
xen_get_apic_id does. */
 
.cpu_mask_to_apicid_and = flat_cpu_mask_to_apicid_and,
 
-- 
1.9.3




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

[Xen-devel] [qemu-mainline test] 96732: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 96703
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 96703
 test-amd64-amd64-xl-rtds  6 xen-boot fail   like 96703

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

version targeted for testing:
 qemuu91d35509903464c7f4b9ed56be223d7370d3597c
baseline version:
 qemuu791b7d2340cfafcac9af7864343cf23504d57804

Last test of basis96703  2016-07-06 06:02:24 Z0 days
Testing same since96732  2016-07-06 18:12:54 Z0 days1 attempts


People who touched revisions under test:
  Andreas Färber 
  Changlong Xie 
  Denis V. Lunev 
  Eric Blake 
  Gerd Hoffmann 
  John Snow 
  Kevin Wolf 
  Marc-André Lureau 
  Markus Armbruster 
  Max Reitz 
  Paolo Bonzini 
  Peter Maydell 
  Richard Henderson 
  Sergey Sorokin 
  Stefan Hajnoczi 

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

Re: [Xen-devel] [PATCH v4] x86/cpuid: AVX-512 Feature Detection

[Kang, Luwei]

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Tuesday, July 5, 2016 3:03 PM
> To: Kang, Luwei 
> Cc: andrew.coop...@citrix.com; chao.p.p...@linux.intel.com; xen-
> de...@lists.xen.org
> Subject: RE: [PATCH v4] x86/cpuid: AVX-512 Feature Detection
> 
> >>> On 05.07.16 at 04:31,  wrote:
> 
> First of all - please don't top post.
> 
> > What about remove the dependency between AVX2 and AVX512F ( AVX2:
> [AVX512F], ) ?
> 
> Yes, that's what I think we want, but we need Andrew's agreement here.
> 

Hi Andrew,  what is your opinion ?

> > From: Jan Beulich [mailto:jbeul...@suse.com]
> > Sent: Friday, July 1, 2016 3:56 PM
>  On 30.06.16 at 07:50,  wrote:
> >> --- a/xen/tools/gen-cpuid.py
> >> +++ b/xen/tools/gen-cpuid.py
> >> @@ -243,6 +243,17 @@ def crunch_numbers(state):
> >>  # AMD K6-2+ and K6-III processors shipped with 3DNow+, beyond
> the
> >>  # standard 3DNow in the earlier K6 processors.
> >>  _3DNOW: [_3DNOWEXT],
> >> +
> >> +# AVX2 is an extension to AVX, providing mainly new integer
> instructions.
> >> +# In principle, AVX512 only takes YMM register state as a
> prerequisite
> >> +# and many AVX2 instructions are extended by AVX512F to 512-bit
> forms.
> >> +AVX2: [AVX512F],
> >
> > I think this was meant to be "but" or "yet" instead of "and". And then
> > I'm not particularly happy about the asymmetry with AVX (which is not
> > dependent on SSE or any of its successors, despite extending many of
> > them to 256-bit forms). Plus e.g. FMA gets extended too, but isn't being
> made a dependency.
> > And quite opposite to that, at least some AVX2 instructions get
> > extended only by e.g. AVX512BW.
> > Nor are any of the extended forms listed to require other than the
> > AVX512* bits checked in the doc.
> >
> > Jan
> 
> 


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

Re: [Xen-devel] [PATCH v2] xen/arm: register clocks used by the hypervisor

[Michael Turquette]

Quoting Mark Rutland (2016-07-05 04:07:37)
> Hi,
> 
> On Tue, Jul 05, 2016 at 12:45:34PM +0200, Dirk Behme wrote:
> > On 05.07.2016 12:39, Mark Rutland wrote:
> > >On Tue, Jul 05, 2016 at 08:50:23AM +0200, Dirk Behme wrote:
> > >>+- clocks: one or more clocks to be registered.
> > >>+  Xen hypervisor drivers might replace native drivers, resulting in
> > >>+  clocks not registered by these native drivers. To avoid that these
> > >>+  unregistered clocks are disabled by the Linux kernel initialization
> > >>+  register them in the hypervisor node.
> > >>+  An example for this are the clocks of a serial driver already enabled
> > >>+  by the firmware. If the clocks used by the serial hardware interface
> > >>+  are not registered by the serial driver itself the serial output
> > >>+  might stop once the Linux kernel initialization disables the 'unused'
> > >>+  clocks.
> > >
> > >The above describes the set of problems, but doesn't set out the actual
> > >contract. It also covers a number of Linux implementation details in
> > >abstract.
> > 
> > Could you kindly be a little more specific which 'implementation
> > details' you don't like?
> 
> The fact that we disable some clocks at init time is a driver model
> thing that depends on various factors (e.g. cmdline options), and it's
> something that could be moved around. We only mention disabling, and not
> rate change (which could happen, even if it doesn't today).
> 
> I don't think that we need to describe the Linux behaviour at all.
> 
> > E.g. to my understanding, the 'implementation detail' that Linux
> > disables unregistered clocks is needed for the description.
> > 
> > If you have a different wording in mind, could you kindly share that?
> 
> Something like:
> 
> - clocks: a list of phandle + clock-specifier pairs 
>   Clocks described by this property are reserved for use by Xen, and the
>   OS must not alter their state any way, such as disabling or gating a
>   clock, or modifying its rate. Ensuring this may impose constraints on
>   parent clocks or other resources used by the clock tree.
> 
>   Note: this property is used to proxy clocks for devices Xen has taken
>   ownership of, such as UARTs, for which the associated clock
>   controller(s) remain under the control of Dom0.

Fully agree that we should forget the clk_disable_unused stuff entirely.
Mark's copy above is good, and just for the fun of it I have provided an
alternative version. Feel free to pick and choose what you want:

- clocks: one or more clocks to be enabled.
  Xen hypervisor drivers might replace native OS drivers. The result is
  that some important clocks that are enabled by the OS in the non-Xen
  case are not properly enabled in the presence of Xen. The clocks
  property enumerates the clocks that must be enabled by the Xen clock
  consumer.
  An example is a serial driver enabled by the hypervisor. Xen must
  consume and enable these clocks in the OS to ensure behavior continues
  after firmware configures the UART hardware and corresponding clock
  harder.

Regards,
Mike

> 
> > >As I commented previously [1], the binding should describe the set of
> > >guarantees that you rewquire (e.g. that the clocks must be left as-is,
> > >not gated, and their rates left unchanged).
> > >
> > >Please describe the specific set of guarantees that you require.
> > 
> > To my understanding this is done, already: "avoid that these ...
> > clocks are disabled"
> 
> My point of contention here is that while this might tell a dts author
> what to place in this property, it doesn't specify what the OS should
> do.
> 
> Thanks,
> Mark.

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

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

[osstest service owner]

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

Regressions :-(

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

version targeted for testing:
 ovmf 6ff71a134f5b974549bf991aa594986bc5f873ee
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   43 days
Failing since 94750  2016-05-25 03:43:08 Z   42 days   94 attempts
Testing same since96719  2016-07-06 13:45:47 Z0 days1 attempts


People who touched revisions under test:
  Anandakrishnan Loganathan 
  Ard Biesheuvel 
  Bi, Dandan 
  Bret Barkelew 
  Bruce Cran 
  Bruce Cran 
  Chao Zhang 
  Cinnamon Shia 
  Cohen, Eugene 
  Dandan Bi 
  Darbin Reyes 
  david wei 
  Eric Dong 
  Eugene Cohen 
  Evan Lloyd 
  Evgeny Yakovlev 
  Feng Tian 
  Fu Siyuan 
  Fu, Siyuan 
  Gary Li 
  Gary Lin 
  Giri P Mudusuru 
  Graeme Gregory 
  Hao Wu 
  Hegde Nagaraj P 
  hegdenag 
  Heyi Guo 
  Jan D?bro? 
  Jan Dabros 
  Jeff Fan 
  Jiaxin Wu 
  Jiewen Yao 
  Joe Zhou 
  Jordan Justen 
  Katie Dellaquila 
  Laszlo Ersek 
  Liming Gao 
  Lu, ShifeiX A 
  lushifex 
  Marcin Wojtas 
  Mark Rutland 
  Marvin H?user 
  Marvin Haeuser 
  Maurice Ma 
  Michael Zimmermann 
  Qiu Shumin 
  Ruiyu Ni 
  Ryan Harkin 
  Sami Mujawar 
  Satya Yarlagadda 
  Shannon Zhao 
  Sriram Subramanian 
  Star Zeng 
  Sunny Wang 
  Tapan Shah 
  Thomas Palmer 
  Yarlagadda, Satya P 
  Yonghong Zhu 
  Zhang Lubo 
  Zhang, Chao B 

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



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

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

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

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


Not pushing.

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

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

Re: [Xen-devel] [PATCH v8 4/6] arm/vm_event: get/set registers

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 3:12 PM, Julien Grall  wrote:
>
>
> On 06/07/2016 20:23, Tamas K Lengyel wrote:
>>
>> On Wed, Jul 6, 2016 at 1:43 AM, Jan Beulich  wrote:
>>
>> On 05.07.16 at 20:37,  wrote:

 +struct vm_event_regs_arm64 {
 +uint64_t x0;
 +uint64_t x1;
 +uint64_t x2;
 +uint64_t x3;
 +uint64_t x4;
 +uint64_t x5;
 +uint64_t x6;
 +uint64_t x7;
 +uint64_t x8;
 +uint64_t x9;
 +uint64_t x10;
 +uint64_t x11;
 +uint64_t x12;
 +uint64_t x13;
 +uint64_t x14;
 +uint64_t x15;
 +uint64_t x16;
 +uint64_t x17;
 +uint64_t x18;
 +uint64_t x19;
 +uint64_t x20;
 +uint64_t x21;
 +uint64_t x22;
 +uint64_t x23;
 +uint64_t x24;
 +uint64_t x25;
 +uint64_t x26;
 +uint64_t x27;
 +uint64_t x28;
 +uint64_t x29;
 +uint64_t x30;
 +uint64_t pc;
 +};
>>>
>>>
>>> Isn't the stack pointer a fully separate register in aarch64? Not
>>> making available something as essential as that seems wrong to
>>> me.
>>>
>>
>> The register is available for access already, so unless there is an
>> actual use-case that requires it to be transmitted through vm_event I
>> don't see the point for transmitting it. So as I mentioned in my other
>> response, I'm inclined to reduce this patch to the bare essentials my
>> use-case requires at this point and leave the extensions up for the
>> future when - and if - it will be of use. Since this patch is just an
>> optimization, if transmitting such reduced set is not acceptable for
>> some reason, I'll forgo this patch entirely.
>
>
> Here we go with again the same argument: "this is not necessary for my
> use-case". This data structure is part of the ABI between the hypervisor and
> the vm-event app, i.e modifying this structure for adding ARM64/ARM
> registers will result to an incompatibility with a previous version of the
> hypervisor. Better to do it now than in a couple of years when vm-event will
> have more users. I agree that it is time consuming to get an ABI correct,
> but it will save users to recompile/ship another version of their vm-event
> app because of this incompatibility.
>
> As mentioned in a previous thread, the main use-case for trapping an SMC is
> emulating the call, hence a vm-event app would want to have access to the
> general-purpose registers. And yes, I know that your use-case is different
> and does not require those registers, I already expressed my concerns about
> it.
>
> Now, if you drop this patch, how will you retrieve the vCPU register? I
> guess via DOMCTL_getvcpucontext? If so, if the vCPU has not been paused, you
> will get a context which is different compare to the time the vm-event has
> occurred. And yes, I know that in your use-case the vCPU is paused. This
> call will always be more expensive than passing the registers with event.

Ack but as right now this patch is just an optimization with no
use-case where it is required, so I'll just drop it from my series.

>
> Anyway, I really don't think we ask for something really difficult to
> accomplish.
>

That may be so and it can be revisited in the future when and if it
will be a hard requirement.

Thanks,
Tamas

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

[Xen-devel] [xen-unstable test] 96712: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

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

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

version targeted for testing:
 xen  fea586d801f75317cb8cf593e8beba842391da62
baseline version:
 xen  bb4f41b3dff831faaf5a3248e0ecd123024d7f8f

Last test of basis96611  2016-07-04 01:58:03 Z2 days
Failing since 96634  2016-07-04 12:50:54 Z2 days4 attempts
Testing same since96688  2016-07-05 18:49:37 Z1 days2 attempts


People who touched revisions under test:
  Andrew Cooper 
  Corneliu ZUZU 
  Jan Beulich 
  Kevin Tian 
  Razvan Cojocaru 
  Tamas K Lengyel 
  Wei Liu 

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

Re: [Xen-devel] [PATCH v8 4/6] arm/vm_event: get/set registers

[Julien Grall]

On 06/07/2016 20:23, Tamas K Lengyel wrote:

On Wed, Jul 6, 2016 at 1:43 AM, Jan Beulich  wrote:

On 05.07.16 at 20:37,  wrote:

+struct vm_event_regs_arm64 {
+uint64_t x0;
+uint64_t x1;
+uint64_t x2;
+uint64_t x3;
+uint64_t x4;
+uint64_t x5;
+uint64_t x6;
+uint64_t x7;
+uint64_t x8;
+uint64_t x9;
+uint64_t x10;
+uint64_t x11;
+uint64_t x12;
+uint64_t x13;
+uint64_t x14;
+uint64_t x15;
+uint64_t x16;
+uint64_t x17;
+uint64_t x18;
+uint64_t x19;
+uint64_t x20;
+uint64_t x21;
+uint64_t x22;
+uint64_t x23;
+uint64_t x24;
+uint64_t x25;
+uint64_t x26;
+uint64_t x27;
+uint64_t x28;
+uint64_t x29;
+uint64_t x30;
+uint64_t pc;
+};


Isn't the stack pointer a fully separate register in aarch64? Not
making available something as essential as that seems wrong to
me.



The register is available for access already, so unless there is an
actual use-case that requires it to be transmitted through vm_event I
don't see the point for transmitting it. So as I mentioned in my other
response, I'm inclined to reduce this patch to the bare essentials my
use-case requires at this point and leave the extensions up for the
future when - and if - it will be of use. Since this patch is just an
optimization, if transmitting such reduced set is not acceptable for
some reason, I'll forgo this patch entirely.


Here we go with again the same argument: "this is not necessary for my 
use-case". This data structure is part of the ABI between the hypervisor 
and the vm-event app, i.e modifying this structure for adding ARM64/ARM 
registers will result to an incompatibility with a previous version of 
the hypervisor. Better to do it now than in a couple of years when 
vm-event will have more users. I agree that it is time consuming to get 
an ABI correct, but it will save users to recompile/ship another version 
of their vm-event app because of this incompatibility.


As mentioned in a previous thread, the main use-case for trapping an SMC 
is emulating the call, hence a vm-event app would want to have access to 
the general-purpose registers. And yes, I know that your use-case is 
different and does not require those registers, I already expressed my 
concerns about it.


Now, if you drop this patch, how will you retrieve the vCPU register? I 
guess via DOMCTL_getvcpucontext? If so, if the vCPU has not been paused, 
you will get a context which is different compare to the time the 
vm-event has occurred. And yes, I know that in your use-case the vCPU is 
paused. This call will always be more expensive than passing the 
registers with event.


Anyway, I really don't think we ask for something really difficult to 
accomplish.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] xen/arm: register clocks used by the hypervisor

[Michael Turquette]

Hi Julien,

Quoting Julien Grall (2016-07-06 06:10:52)
> On 06/07/16 02:34, Michael Turquette wrote:
> > Hi!
> 
> Hello Michael,
> 
> > Quoting Dirk Behme (2016-06-30 03:32:32)
> >> Some clocks might be used by the Xen hypervisor and not by the Linux
> >> kernel. If these are not registered by the Linux kernel, they might be
> >> disabled by clk_disable_unused() as the kernel doesn't know that they
> >> are used. The clock of the serial console handled by Xen is one
> >> example for this. It might be disabled by clk_disable_unused() which
> >> stops the whole serial output, even from Xen, then.
> >
> > This whole thread had me confused until I realized that it all boiled
> > down to some nomenclature issues (for me).
> >
> > This code does not _register_ any clocks. It simply gets them and
> > enables them, which is what every other clk consumer in the Linux kernel
> > does. More details below.
> >
> >>
> >> Up to now, the workaround for this has been to use the Linux kernel
> >> command line parameter 'clk_ignore_unused'. See Xen bug
> >>
> >> http://bugs.xenproject.org/xen/bug/45
> >
> > clk_ignore_unused is a band-aid, not a proper medical solution. Setting
> > that flag will not turn clocks on for you, nor will it guarantee that
> > those clocks are never turned off in the future. It looks like you
> > figured this out correctly in the patch below but it is worth repeating.
> >
> > Also the new CLK_IS_CRITICAL flag might be of interest to you, but that
> > flag only exists as a way to enable clocks that must be enabled for the
> > system to function (hence, "critical") AND when those same clocks do not
> > have an accompanying Linux driver to consume them and enable them.
> 
> I don't think we want the kernel to enable the clock for the hypervisor. 
> We want to tell the kernel "don't touch at all to this clock, it does 
> not belong to you".

But the patch *does* touch the clock from the kernel. It enables the
clock via a call clk_prepare_enable. I'm utterly confused.

Regards,
Mike

> 
> Regards,
> 
> -- 
> Julien Grall

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  db59dbc46e0312a38978d22c6bd72b554a2f1c91
baseline version:
 xen  fea586d801f75317cb8cf593e8beba842391da62

Last test of basis96669  2016-07-05 10:01:51 Z1 days
Testing same since96731  2016-07-06 18:06:43 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 
  Roger Pau Monné 
  Wei Liu 

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



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

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

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

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


Pushing revision :

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

Re: [Xen-devel] [PATCH v8 4/6] arm/vm_event: get/set registers

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 1:43 AM, Jan Beulich  wrote:
 On 05.07.16 at 20:37,  wrote:
>> +struct vm_event_regs_arm64 {
>> +uint64_t x0;
>> +uint64_t x1;
>> +uint64_t x2;
>> +uint64_t x3;
>> +uint64_t x4;
>> +uint64_t x5;
>> +uint64_t x6;
>> +uint64_t x7;
>> +uint64_t x8;
>> +uint64_t x9;
>> +uint64_t x10;
>> +uint64_t x11;
>> +uint64_t x12;
>> +uint64_t x13;
>> +uint64_t x14;
>> +uint64_t x15;
>> +uint64_t x16;
>> +uint64_t x17;
>> +uint64_t x18;
>> +uint64_t x19;
>> +uint64_t x20;
>> +uint64_t x21;
>> +uint64_t x22;
>> +uint64_t x23;
>> +uint64_t x24;
>> +uint64_t x25;
>> +uint64_t x26;
>> +uint64_t x27;
>> +uint64_t x28;
>> +uint64_t x29;
>> +uint64_t x30;
>> +uint64_t pc;
>> +};
>
> Isn't the stack pointer a fully separate register in aarch64? Not
> making available something as essential as that seems wrong to
> me.
>

The register is available for access already, so unless there is an
actual use-case that requires it to be transmitted through vm_event I
don't see the point for transmitting it. So as I mentioned in my other
response, I'm inclined to reduce this patch to the bare essentials my
use-case requires at this point and leave the extensions up for the
future when - and if - it will be of use. Since this patch is just an
optimization, if transmitting such reduced set is not acceptable for
some reason, I'll forgo this patch entirely.

Thanks,
Tamas

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

Re: [Xen-devel] [PATCH v8 4/6] arm/vm_event: get/set registers

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 12:04 PM, Julien Grall  wrote:
>
>
> On 05/07/16 19:37, Tamas K Lengyel wrote:
>>
>> +void vm_event_fill_regs(vm_event_request_t *req)
>> +{
>> +const struct cpu_user_regs *regs = guest_cpu_user_regs();
>> +
>> +req->data.regs.arm.cpsr = regs->cpsr;
>> +req->data.regs.arm.ttbr0 = READ_SYSREG64(TTBR0_EL1);
>> +req->data.regs.arm.ttbr1 = READ_SYSREG64(TTBR1_EL1);
>> +
>> +if ( psr_mode_is_32bit(regs->cpsr) )
>> +{
>> +req->data.regs.arm.arch.arm32.r0 = regs->r0;
>> +req->data.regs.arm.arch.arm32.r1 = regs->r1;
>> +req->data.regs.arm.arch.arm32.r2 = regs->r2;
>> +req->data.regs.arm.arch.arm32.r3 = regs->r3;
>> +req->data.regs.arm.arch.arm32.r4 = regs->r4;
>> +req->data.regs.arm.arch.arm32.r5 = regs->r5;
>> +req->data.regs.arm.arch.arm32.r6 = regs->r6;
>> +req->data.regs.arm.arch.arm32.r7 = regs->r7;
>> +req->data.regs.arm.arch.arm32.r8 = regs->r8;
>> +req->data.regs.arm.arch.arm32.r9 = regs->r9;
>> +req->data.regs.arm.arch.arm32.r10 = regs->r10;
>> +req->data.regs.arm.arch.arm32.r11 = regs->fp;
>> +req->data.regs.arm.arch.arm32.r12 = regs->r12;
>> +req->data.regs.arm.arch.arm32.r13 = regs->sp;
>
>
> Please look at the description of "sp" in cpu_user_regs. You will notice
> this is only valid for the hypervisor.
>
> There are multiple stack pointers for the guest depending on the running
> mode (see B9.2.1 in ARM DDI 0406C.c), so you may want to pass all of them.
>
>> +req->data.regs.arm.arch.arm32.r14 = regs->lr;
>
>
> Whilst lr is an union with lr_usr on ARM32 port, for the ARM64 port they are
> distinct (see cpu_users). So you would use the wrong register here.
>
> However, as for sp, there are multiple lr pointers for the guest depending
> on the running mode. So you will pass the wrong lr if the guest is running
> in another mode than user.
>

This patch is becoming a lot more work then what I need it for so I'm
inclined to just reduce it to the bare minimum my use-case requires,
which is only cpsr, pc and ttbr0 and ttbr1. I had reservation about
growing the data field in the vm_event struct anyway, especially since
there is no use-case that requires it. In case someone has an actual
use-case in the future that requires other registers to be submitted
through vm_event, the interface is available for extension.

Thanks,
Tamas

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

Re: [Xen-devel] [PATCH v8 2/6] arm: filter SMC exceptions with failed condition checks

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 11:31 AM, Julien Grall  wrote:
> On 05/07/16 19:37, Tamas K Lengyel wrote:
>>
>> In AArch32 state, the ARMv8-A architecture permits, but does not require,
>> this trap to apply to conditional SMC instructions that fail their
>> condition
>> code check, in the same way as with traps on other conditional
>> instructions.
>
>
> Please add a quote to the spec.
>
>> Signed-off-by: Tamas K Lengyel 
>> Suggested-by: Julien Grall 
>> ---
>> Cc: Stefano Stabellini 
>> Cc: Julien Grall 
>> ---
>>   xen/arch/arm/traps.c | 15 +--
>>   1 file changed, 13 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 44926ca..627e8c9 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -2507,6 +2507,17 @@ bad_data_abort:
>>   inject_dabt_exception(regs, info.gva, hsr.len);
>>   }
>>
>> +static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
>> +{
>> +if ( !check_conditional_instr(regs, hsr) )
>
>
> This function is checking the EC, it considers that EC > 0x10 will be
> unconditional. However, the SMC exception class is 0x13 when taken from
> AArch32 and 0x17 when taken from AArch64.
>
> Furthermore, for ARMv7, the register is Reserved UNK/SBZP (see B3-1431 in
> ARM DDI 0406C.c). I.e the software should not rely on the field reading as
> all 0s (see Glossary-2736).
>
> For ARMv8, when the SMC is taken from AArch64 (see D7-1942 in ARM DDI
> 0487A.j), the register is RES0 which means the software should not rely on
> the value to always be 0 (see Glossary-5734). When the SMC is taken from
> AArch32, the field CV is only valid if CCKNOWNPASS is 1 (this field does not
> exist for the other exception class).
>
> So this code need more extra care. It would also be nice to have a
> description in the code explaining what's going on.

Ack, there seems to be enough corner-cases here that I'm unfamiliar
with that I would rather not attempt implementing this as I likely
would miss something. I'll wait for your patch with the rest of my
series as that route will likely have less overhead anyway.

Thanks,
Tamas

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

Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Julien Grall]

On 06/07/16 17:35, Tamas K Lengyel wrote:

On Wed, Jul 6, 2016 at 10:29 AM, Julien Grall  wrote:



On 06/07/16 17:05, Tamas K Lengyel wrote:


On Wed, Jul 6, 2016 at 9:54 AM, Julien Grall  wrote:


Taken aside the VMFUNC, it looks like insecure to expose a HVMOP to the
guest which could modify the memaccess attribute of a region.

I thought the whole purpose of VM introspection is to avoid trusting the
guest (kernel + userspace). The first thing a malware will do is trying
to
do is getting access to the kernel. Once it gets this access, it could
remove all the memory access restriction to avoid trapping.



That's why I'm saying systems that use this will likely do extra steps
to ensure kernel integrity. In use-cases where this is to be used
exclusively for external monitoring the system can be restricted with
XSM to not allow the guest to issue the hvmops. And remember, on x86
this system is not exclusively used for introspection.



I am not aware on how x86 is using alt2pm. And this series didn't give much
explanation how this is supposed to work...




As for ARM - as there is no hardware features like this available -
our goal is to use altp2m in purely external usecases so exposing
these ops to the guest is not required. For the first prototype it
made sense to mirror the x86 side to reduce the possibility of
introducing some bug.




No, this is not the right approach. We should not introduce potential
security issue just because x86 side does it and it "reduces the
possibility
of introducing some bug".

You will have to give me another reason to accept a such patch.



The first revision of a large series is highly unlikely to get
accepted on the first run so we have been working with the assumption
that there will be new revisions. The prototype has been working well
enough for our internal tests to warrant not submitting it as PATCH
RFC. Since this is Sergej's first work with Xen it helped to mirror
the x86 to get him up to speed while working on the prototype and
reducing the complexity he has to keep track of. Now that this phase
is complete the adjustments can be made as required, such as not
exposing these hvmops to ARM guests.



A such large series is already hard to review, it is even harder when the
contributor leaves code unfinished because he assumed there will be a new
revision. Actually this is the whole purpose of tagging the series with
"RFC". It is used that the series is not in a state where it could
potentially be committed.



The code is not in an unfinished state by any means as it passes _our_
tests and works as expected. I think the assumption we made that there
will be required adjustments is very reasonable on any patch series.
So I'm not sure what the problem is.


Well, I am bit surprised that this series is passing your tests (you may 
want to update them?). Before sending a new version, I would recommend 
to go through the locking of each path. I tried to comment on every 
locking issue I have spotted, although I may have miss some.


I would also recommend you to go through the ARM ARM to see how the TLBs 
behaves because switching between page tables with the same VMID could 
be harmful if not doing correctly. I have mentioned that you could use 
different VMID for the page table, however this may have impact on other 
part of the memory such as the cache. (this would need to be investigated).


I know it is not an easy task. The P2M code is complex, so it would 
benefit for all the reviewers to have an explanation in the cover letter 
how this is supposed to work.


Regards,

--
Julien Grall

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

[Xen-devel] [PATCH v1] hvmloader, pci: Don't try to relocate memory if 64-bit BAR is bigger than 4GB

[Konrad Rzeszutek Wilk]

There is no point. We can try to balloon out the memory between
hvm_info->low_mem_pgend to pci_mem_end and we will never be able
have a hole big enough for 4GB MMIO.

Instead just let it go above 4GB in the 64-bit zone.

Note that prior to this patch the hvmloader would relocate as much
memory as it could under 4GB:
Low MMIO hole not large enough for all devices, relocating some BARs to 64-bit
Relocating 0x pages from 0e0001000 to 21000 for lowmem MMIO hole
Relocating 0x pages from 0d0002000 to 21000 for lowmem MMIO hole
Relocating 0x pages from 0c0003000 to 22fffe000 for lowmem MMIO hole
Relocating 0x pages from 0b0004000 to 23fffd000 for lowmem MMIO hole
Relocating 0x pages from 0a0005000 to 24fffc000 for lowmem MMIO hole
Relocating 0x pages from 090006000 to 25fffb000 for lowmem MMIO hole
Relocating 0x pages from 080007000 to 26fffa000 for lowmem MMIO hole
Relocating 0x7 pages from 08000 to 27fff9000 for lowmem MMIO hole

which is completely pointless.

Signed-off-by: Konrad Rzeszutek Wilk 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
---
 tools/firmware/hvmloader/pci.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/tools/firmware/hvmloader/pci.c b/tools/firmware/hvmloader/pci.c
index 4eb1a31..29c0a9a 100644
--- a/tools/firmware/hvmloader/pci.c
+++ b/tools/firmware/hvmloader/pci.c
@@ -80,7 +80,7 @@ void pci_setup(void)
 {
 uint8_t is_64bar, using_64bar, bar64_relocate = 0;
 uint32_t devfn, bar_reg, cmd, bar_data, bar_data_upper;
-uint64_t base, bar_sz, bar_sz_upper, mmio_total = 0;
+uint64_t base, bar_sz, bar_sz_upper, mmio_total = 0, mmio_64bit_total = 0;
 uint32_t vga_devfn = 256;
 uint16_t class, vendor_id, device_id;
 unsigned int bar, pin, link, isa_irq;
@@ -269,8 +269,19 @@ void pci_setup(void)
 if ( ((bar_data & PCI_BASE_ADDRESS_SPACE) ==
   PCI_BASE_ADDRESS_SPACE_MEMORY) ||
  (bar_reg == PCI_ROM_ADDRESS) )
-mmio_total += bar_sz;
-
+{
+/* If bigger than 4GB, don't try to put under 4GB. */
+if ( is_64bar && bar_sz > (1ull<<32) )
+{
+mmio_64bit_total += bar_sz;
+/*
+ * As this may not trigger now that mmio_total could be
+ * less than 2GB, so force it.
+ */
+bar64_relocate = 1;
+} else
+mmio_total += bar_sz;
+}
 nr_bars++;
 
 /*The upper half is already calculated, skip it! */
@@ -349,7 +360,7 @@ void pci_setup(void)
 pci_mem_start = hvm_info->low_mem_pgend << PAGE_SHIFT;
 }
 
-if ( mmio_total > (pci_mem_end - pci_mem_start) )
+if ( mmio_total > (pci_mem_end - pci_mem_start) || bar64_relocate )
 {
 printf("Low MMIO hole not large enough for all devices,"
" relocating some BARs to 64-bit\n");
@@ -431,7 +442,7 @@ void pci_setup(void)
  * Should either of those two conditions change, this code will break.
  */
 using_64bar = bars[i].is_64bar && bar64_relocate
-&& (mmio_total > (mem_resource.max - mem_resource.base));
+&& (mmio_total + mmio_64bit_total > (mem_resource.max - 
mem_resource.base));
 bar_data = pci_readl(devfn, bar_reg);
 
 if ( (bar_data & PCI_BASE_ADDRESS_SPACE) ==
@@ -451,7 +462,10 @@ void pci_setup(void)
 resource = _resource;
 bar_data &= ~PCI_BASE_ADDRESS_MEM_MASK;
 }
-mmio_total -= bar_sz;
+if ( bars[i].is_64bar && bar_sz > (1ull<<32) )
+mmio_64bit_total -= bar_sz;
+else
+mmio_total -= bar_sz;
 }
 else
 {
-- 
2.4.11


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

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

[Julien Grall]

Hi Fu Wei,

On 06/07/16 18:58, Fu Wei wrote:

On 5 July 2016 at 18:48, Julien Grall  wrote:

Hi Fu Wei,

What is the status of this series? IIRC the comments on this version were
minor.


Sorry, I missed the suggestion of docs, I have rebased them to latest
upstream grub and posted v5 just now.


Thank you for the new series :). I will give a look tomorrow.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v8 4/6] arm/vm_event: get/set registers

[Julien Grall]

On 05/07/16 19:37, Tamas K Lengyel wrote:

+void vm_event_fill_regs(vm_event_request_t *req)
+{
+const struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+req->data.regs.arm.cpsr = regs->cpsr;
+req->data.regs.arm.ttbr0 = READ_SYSREG64(TTBR0_EL1);
+req->data.regs.arm.ttbr1 = READ_SYSREG64(TTBR1_EL1);
+
+if ( psr_mode_is_32bit(regs->cpsr) )
+{
+req->data.regs.arm.arch.arm32.r0 = regs->r0;
+req->data.regs.arm.arch.arm32.r1 = regs->r1;
+req->data.regs.arm.arch.arm32.r2 = regs->r2;
+req->data.regs.arm.arch.arm32.r3 = regs->r3;
+req->data.regs.arm.arch.arm32.r4 = regs->r4;
+req->data.regs.arm.arch.arm32.r5 = regs->r5;
+req->data.regs.arm.arch.arm32.r6 = regs->r6;
+req->data.regs.arm.arch.arm32.r7 = regs->r7;
+req->data.regs.arm.arch.arm32.r8 = regs->r8;
+req->data.regs.arm.arch.arm32.r9 = regs->r9;
+req->data.regs.arm.arch.arm32.r10 = regs->r10;
+req->data.regs.arm.arch.arm32.r11 = regs->fp;
+req->data.regs.arm.arch.arm32.r12 = regs->r12;
+req->data.regs.arm.arch.arm32.r13 = regs->sp;


Please look at the description of "sp" in cpu_user_regs. You will notice 
this is only valid for the hypervisor.


There are multiple stack pointers for the guest depending on the running 
mode (see B9.2.1 in ARM DDI 0406C.c), so you may want to pass all of them.



+req->data.regs.arm.arch.arm32.r14 = regs->lr;


Whilst lr is an union with lr_usr on ARM32 port, for the ARM64 port they 
are distinct (see cpu_users). So you would use the wrong register here.


However, as for sp, there are multiple lr pointers for the guest 
depending on the running mode. So you will pass the wrong lr if the 
guest is running in another mode than user.


Regards,

--
Julien Grall

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

[Xen-devel] [qemu-mainline test] 96703: tolerable FAIL - PUSHED

[osstest service owner]

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

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  6 xen-boot  fail REGR. vs. 94856
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 94856
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 94856

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

version targeted for testing:
 qemuu791b7d2340cfafcac9af7864343cf23504d57804
baseline version:
 qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe

Last test of basis94856  2016-05-27 20:14:49 Z   39 days
Failing since 94983  2016-05-31 09:40:12 Z   36 days   52 attempts
Testing same since96703  2016-07-06 06:02:24 Z0 days1 attempts


People who touched revisions under test:
  Aaron Larson 
  Alberto Garcia 
  Aleksandar Markovic 
  Alex Bennée 
  Alex Bligh 
  Alex Williamson 
  Alexander Graf 
  Alexander Shopov 
  Alexey Kardashevskiy 
  Alistair Francis 
  Amit Shah 
  Andrea Arcangeli 
  Andrew Jeffery 
  Andrew Jones 
  Andrey Smirnov 
  Aneesh Kumar K.V 
  Anthony PERARD 
  Anton Blanchard 
  Ard Biesheuvel 
  Artyom Tarasenko 
  Ashijeet Acharya 
  Aurelien Jarno 
  Bastian Koppelmann 
  Benjamin Herrenschmidt 
  Bharata B Rao 
  Bret Ketchum 
  Cao jin 
  Changlong Xie 
  Chao Peng 
  Chen Fan 
  Christian 

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

[fu . wei]

From: Fu Wei 

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

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

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

ChangeLog:
v5: Update the introduction of xen_module commands in docs/grub.texi,
according to the suggestion from Julien Grall

v4: http://lists.gnu.org/archive/html/grub-devel/2016-05/
according to the XSM loading mechanism of Xen(upstreamed),
update the introduction of xen_module commands in docs/grub.texi

v3: http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00314.html
reorder the patches
update the introduction of xen_module commands in docs/grub.texi

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

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

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

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

-- 
2.5.5


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

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

[Fu Wei]

Hi Julien,

On 5 July 2016 at 18:48, Julien Grall  wrote:
> Hi Fu Wei,
>
> What is the status of this series? IIRC the comments on this version were
> minor.

Sorry, I missed the suggestion of docs, I have rebased them to latest
upstream grub and posted v5 just now.

Thanks for reminding me

>
> Regards,
>
>
> On 10/05/16 15:03, fu@linaro.org wrote:
>>
>> From: Fu Wei 
>>
>> This patchset add xen_boot support into grup-mkconfig for
>> generating xen boot entrances automatically
>>
>> Also update the docs/grub.texi for new xen_boot commands.
>>
>> This patchset has been tested on Foudation model with a bug fix:
>> http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00205.html
>>
>> ChangeLog:
>> v4: http://lists.gnu.org/archive/html/grub-devel/2016-05/
>>  according to the XSM loading mechanism of Xen(upstreamed),
>>  update the introduction of xen_module commands in docs/grub.texi
>>
>> v3: http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00314.html
>>  reorder the patches
>>  update the introduction of xen_module commands in docs/grub.texi
>>
>> v2: http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00282.html
>>  add "--nounzip" option support in xen_module
>>  use "feature_xen_boot" instead of "grub_xen_boot"
>>  update the introduction of xen boot commands in docs/grub.texi
>>
>> v1 :first upstream patchset:
>>  http://lists.gnu.org/archive/html/grub-devel/2016-02/msg00264.html
>>
>> Fu Wei (4):
>>i386,xen: Add xen_hypervisor and xen_module aliases for i386
>>arm64: add "--nounzip" option support in xen_module command
>>* util/grub.d/20_linux_xen.in: Add xen_boot command support
>>arm64: update the introduction of xen boot commands in docs/grub.texi
>>
>>   docs/grub.texi| 33 ++---
>>   grub-core/loader/arm64/xen_boot.c | 17 +
>>   grub-core/loader/i386/xen.c   |  7 +++
>>   grub-core/normal/main.c   |  2 +-
>>   util/grub.d/20_linux_xen.in   | 13 ++---
>>   5 files changed, 45 insertions(+), 27 deletions(-)
>>
>
> --
> Julien Grall



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat

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

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

[fu . wei]

From: Fu Wei 

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

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


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

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

[fu . wei]

From: Fu Wei 

delete: xen_linux, xen_initrd, xen_xsm
add: xen_module

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

xen_boot: Remove obsolete module type distinctions.

Also bases on the module loading mechanism of Xen code:
488c2a8 docs/arm64: clarify the documention for loading XSM support
67831c4 docs/arm64: update the documentation for loading XSM support
ca32012 xen/arm64: check XSM Magic from the second unknown module.

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

diff --git a/docs/grub.texi b/docs/grub.texi
index 82f6fa4..85c913e 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3861,9 +3861,7 @@ you forget a command, you can run the command 
@command{help}
 * videoinfo::   List available video modes
 @comment * xen_*::  Xen boot commands
 * xen_hypervisor::  Load xen hypervisor binary
-* xen_linux::   Load dom0 kernel for xen hypervisor
-* xen_initrd::  Load dom0 initrd for dom0 kernel
-* xen_xsm:: Load xen security module for xen hypervisor
+* xen_module::  Load xen modules for xen hypervisor
 @end menu
 
 
@@ -5141,30 +5139,18 @@ verbatim as the @dfn{kernel command-line}. Any other 
binaries must be
 reloaded after using this command.
 @end deffn
 
-@node xen_linux
-@subsection xen_linux
+@node xen_module
+@subsection xen_module
 
-@deffn Command xen_linux file [arguments]
-Load a dom0 kernel image for xen hypervisor at the booting process of xen.
+@deffn Command xen_module [--nounzip] file [arguments]
+Load a module for xen hypervisor at the booting process of xen.
 The rest of the line is passed verbatim as the module command line.
+Modules should be loaded in the following order:
+ - dom0 kernel image
+ - dom0 ramdisk if present
+ - XSM policy if present
 @end deffn
 
-@node xen_initrd
-@subsection xen_initrd
-
-@deffn Command xen_initrd file
-Load a initrd image for dom0 kernel at the booting process of xen.
-@end deffn
-
-@node xen_xsm
-@subsection xen_xsm
-
-@deffn Command xen_xsm file
-Load a xen security module for xen hypervisor at the booting process of xen.
-See @uref{http://wiki.xen.org/wiki/XSM} for more detail.
-@end deffn
-
-
 @node Networking commands
 @section The list of networking commands
 
-- 
2.5.5


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

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

[fu . wei]

From: Fu Wei 

This patch adds the support of xen_boot command:
xen_hypervisor
xen_module

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

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

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


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

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

[fu . wei]

From: Fu Wei 

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

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

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


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

Re: [Xen-devel] [PATCH v8 4/6] arm/vm_event: get/set registers

[Julien Grall]

On 06/07/16 08:43, Jan Beulich wrote:

On 05.07.16 at 20:37,  wrote:

+struct vm_event_regs_arm64 {
+uint64_t x0;
+uint64_t x1;
+uint64_t x2;
+uint64_t x3;
+uint64_t x4;
+uint64_t x5;
+uint64_t x6;
+uint64_t x7;
+uint64_t x8;
+uint64_t x9;
+uint64_t x10;
+uint64_t x11;
+uint64_t x12;
+uint64_t x13;
+uint64_t x14;
+uint64_t x15;
+uint64_t x16;
+uint64_t x17;
+uint64_t x18;
+uint64_t x19;
+uint64_t x20;
+uint64_t x21;
+uint64_t x22;
+uint64_t x23;
+uint64_t x24;
+uint64_t x25;
+uint64_t x26;
+uint64_t x27;
+uint64_t x28;
+uint64_t x29;
+uint64_t x30;
+uint64_t pc;
+};


Isn't the stack pointer a fully separate register in aarch64?


It is. Actually, there are 2 stack pointers (one for the kernel, the 
other for userspace. See sp_el0 and sp_el1.


Regards,

--
Julien Grall

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

[Xen-devel] [PATCH] credi2-ratelimit: Implement rate limit for credit2 scheduler

[Anshul Makkar anshul.makkar]

From: Anshul Makkar 

Rate limit assures that a vcpu will execute for a minimum amount of time before
being put at the back of a queue or being preempted by higher priority thread.

It introduces a minimum amount of latency to enable a VM to batch its work and
it also ensures that system is not spending most of its time in
VMEXIT/VMENTRY because of VM that is waking/sleeping at high rate.

ratelimit can be disabled by setting it to 0.

Signed-off-by: Anshul Makkar 
---
---
 xen/common/sched_credit2.c | 115 ++---
 1 file changed, 98 insertions(+), 17 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 1933ff1..6718574 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -171,6 +171,11 @@ integer_param("sched_credit2_migrate_resist", 
opt_migrate_resist);
 #define c2r(_ops, _cpu) (CSCHED2_PRIV(_ops)->runq_map[(_cpu)])
 /* CPU to runqueue struct macro */
 #define RQD(_ops, _cpu) (_PRIV(_ops)->rqd[c2r(_ops, _cpu)])
+/* Find the max of time slice */
+#define MAX_TSLICE(t1, t2)  \
+   ({ typeof (t1) _t1 = (t1); \
+  typeof (t1) _t2 = (t2); \
+  _t1 > _t2 ? _t1 < 0 ? 0 : _t1 : _t2 < 0 ? 0 : _t2; })
 
 /*
  * Shifts for load average.
@@ -280,6 +285,7 @@ struct csched2_private {
 struct csched2_runqueue_data rqd[NR_CPUS];
 
 unsigned int load_window_shift;
+unsigned ratelimit_us; /* each cpupool can have its onw ratelimit */
 };
 
 /*
@@ -1588,6 +1594,34 @@ csched2_dom_cntl(
 return rc;
 }
 
+static int csched2_sys_cntl(const struct scheduler *ops,
+struct xen_sysctl_scheduler_op *sc)
+{
+int rc = -EINVAL;
+xen_sysctl_credit_schedule_t *params = >u.sched_credit;
+struct csched2_private *prv = CSCHED2_PRIV(ops);
+unsigned long flags;
+
+switch (sc->cmd )
+{
+case XEN_SYSCTL_SCHEDOP_putinfo:
+if ( params->ratelimit_us &&
+( params->ratelimit_us < CSCHED2_MIN_TIMER ||
+  params->ratelimit_us > MICROSECS(CSCHED2_MAX_TIMER) ))
+return rc;
+spin_lock_irqsave(>lock, flags);
+prv->ratelimit_us = params->ratelimit_us;
+spin_unlock_irqrestore(>lock, flags);
+break;
+
+case XEN_SYSCTL_SCHEDOP_getinfo:
+params->ratelimit_us = prv->ratelimit_us;
+rc = 0;
+break;
+}
+return rc;
+}
+
 static void *
 csched2_alloc_domdata(const struct scheduler *ops, struct domain *dom)
 {
@@ -1657,12 +1691,15 @@ csched2_dom_destroy(const struct scheduler *ops, struct 
domain *dom)
 
 /* How long should we let this vcpu run for? */
 static s_time_t
-csched2_runtime(const struct scheduler *ops, int cpu, struct csched2_vcpu 
*snext)
+csched2_runtime(const struct scheduler *ops, int cpu,
+struct csched2_vcpu *snext, s_time_t now)
 {
-s_time_t time; 
+s_time_t time;
 int rt_credit; /* Proposed runtime measured in credits */
 struct csched2_runqueue_data *rqd = RQD(ops, cpu);
 struct list_head *runq = >runq;
+s_time_t runtime = 0;
+struct csched2_private *prv = CSCHED2_PRIV(ops);
 
 /*
  * If we're idle, just stay so. Others (or external events)
@@ -1680,6 +1717,14 @@ csched2_runtime(const struct scheduler *ops, int cpu, 
struct csched2_vcpu *snext
 
 /* 1) Basic time: Run until credit is 0. */
 rt_credit = snext->credit;
+if (snext->vcpu->is_running)
+runtime = now - snext->vcpu->runstate.state_entry_time;
+if ( runtime < 0 )
+{
+runtime = 0;
+d2printk("%s: Time went backwards? now %"PRI_stime" state_entry_time 
%"PRI_stime"\n",
+  _func__, now, snext->runstate.state_entry_time);
+}
 
 /* 2) If there's someone waiting whose credit is positive,
  * run until your credit ~= his */
@@ -1695,11 +1740,24 @@ csched2_runtime(const struct scheduler *ops, int cpu, 
struct csched2_vcpu *snext
 }
 
 /* The next guy may actually have a higher credit, if we've tried to
- * avoid migrating him from a different cpu.  DTRT.  */
+ * avoid migrating him from a different cpu.  DTRT.
+ * Even if the next guy has higher credit and current vcpu has executed
+ * for less amount of time than rate limit, allow it to run for minimum
+ * amount of time.
+ */
 if ( rt_credit <= 0 )
 {
-time = CSCHED2_MIN_TIMER;
-SCHED_STAT_CRANK(runtime_min_timer);
+if ( snext->vcpu->is_running && prv->ratelimit_us)
+   /* implies the current one has executed for time < ratelimit and 
thus
+* it has neen selcted int runq_candidate to run next.
+* No need to check for this condition again.
+*/
+time = MAX_TSLICE(CSCHED2_MIN_TIMER,
+   MICROSECS(prv->ratelimit_us) - runtime);
+

Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests

[Boris Ostrovsky]

On 07/06/2016 01:03 PM, Julien Grall wrote:
>
>
> On 06/07/16 17:30, Boris Ostrovsky wrote:
>> On 07/06/2016 12:04 PM, Julien Grall wrote:
>>> Hi Boris,
>>>
>>> On 06/07/16 16:50, Boris Ostrovsky wrote:
 On 07/06/2016 07:05 AM, Julien Grall wrote:
>> +static int populate_acpi_pages(struct xc_dom_image *dom,
>> +   xen_pfn_t *extents,
>> +   unsigned int num_pages,
>> +   struct acpi_ctxt *ctxt)
>> +{
>> +int rc;
>> +xc_interface *xch = dom->xch;
>> +uint32_t domid = dom->guest_domid;
>> +unsigned long idx, first_high_idx = (1ull << (32 -
>> ctxt->page_shift));
>> +
>> +for (; num_pages; num_pages--, extents++) {
>> +
>> +if (xc_domain_populate_physmap(xch, domid, 1, 0, 0,
>> extents)
>> == 1)
>
> It looks like this is working because libxl is setting the maximum
> size of the domain with some slack (1MB). You might get in trouble if
> the slack is reduced or used by someone or the ACPI blob is
> increasing.


 I saw your conversation about slack with Stefano and I am not sure I
 understood what it was about. If this was about padding guest's memory
 to be able to put there some additional data (such ACPI tables) then
 this is not my intention here: if I can't populate (because it is
 already populated, I guess) then I try to move memory around with code
 below. That's what mem_hole_populate_ram() does as well.
>>>
>>> The maximum amount of memory that could be assigned to a domain is
>>> fixed per-domain. This maximum amount does not take into account the
>>> size of the ACPI blob. So you may end up to fail because all the
>>> memory was assigned somewhere else.
>>
>> Why shouldn't max amount of guest's memory include ACPI tables? I think
>> we should fail and not rely on this slack if no more memory is
>> available.
>>
>> ...
>
> Because, at least for ARM, the ACPI memory region is not part of the
> "real" RAM. So this value is not taken into account into the current
> max mem.


Hmm.. I've always assumed it is part of memory but marked as a special
type in e820 map on x86. Maybe Andrew or Jan can comment on this.


> My point is we need to add the size of the ACPI blob to max mem to
> avoid any error here.

So you (or Shannon) plan on doing this for ARM, right (it's not done
with the current version of patches)?

>
>>
>>>

> However, as mentioned in the ACPI thread [1], all the blobs are
> generally loaded by libxc and not libxl. This is more true on ARM
> because the guest address space is controlled by libxc (the position
> of all the blob are decided by it).

 The difference is that I not only load the tables here but also build
 them. Which may or may not be the right thing to do in libxc.

 I suppose I can defer loading (and then keep pointer to tables in
 acpitable_blob) but the then I need to keep RSDP descriptor somewhere
 else (it is not part of the blob since it lives in lower MB of the
 guest).
>>>
>>> The device tree for ARM are built in libxl and loaded for libxc. IHMO,
>>> it would be strange to have a different pattern for ACPI.
>>
>> Is RSDP part of the ACPI blob for ARM? If not, how do you load it?
>
> RSDP is part of the ACPI blob for ARM.


And it's not for x86. So to separate building and loading the tables
would require two blobs.

I suppose we cal loop over blobs in xc_dom_load_acpi in
https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg00309.html


-boris




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

Re: [Xen-devel] [PATCH 10/19] xen: credit2: rework load tracking logic

[George Dunlap]

On Sat, Jun 18, 2016 at 12:12 AM, Dario Faggioli
 wrote:
> The existing load tracking code was hard to understad and
> maintain, and not entirely consistent. This is due to a
> number of reasons:
>  - code and comments were not in perfect sync, making it
>difficult to figure out what the intent of a particular
>choice was (e.g., the choice of 18 for load_window_shift);
>  - the math, although effective, was not entirely consistent.
>In fact, we were doing (if W is the lenght of the window):
>
> avgload = (delta*load*W + (W - delta)*avgload)/W
> avgload = avgload + delta*load - delta*avgload/W
>
>which does not match any known variant of 'smoothing
>moving average'. In fact, it should have been:
>
> avgload = avgload + delta*load/W - delta*avgload/W
>
>(for details on why, see the doc comments inside this
>patch.). Furthermore, with
>
> avgload ~= avgload + W*load - avgload
> avgload ~= W*load
>
> The reason why the formula above sort of worked was because
> the number of bits used for the fractional parts of the
> values used in fixed point math and the number of bits used
> for the lenght of the window were the same (load_window_shift
> was being used for both).
>
> This may look handy, but it introduced a (not especially well
> documented) dependency between the lenght of the window and
> the precision of the calculations, which really should be
> two independent things. Especially if treating them as such
> (like it is done in this patch) does not lead to more
> complex maths (same number of multiplications and shifts, and
> there is still room for some optimization).
>
> Therefore, in this patch, we:
>  - split length of the window and precision (and, since there
>is already a command line parameter for length of window,
>introduce one for precision too),
>  - align the math with one proper incarnation of exponential
>smoothing (at no added cost),
>  - add comments, about the details of the algorithm and the
>math used.
>
> While there fix a couple of style issues as well (pointless
> initialization, long lines, comments).
>
> Signed-off-by: Dario Faggioli 

Reviewed-by: George Dunlap 

And queued.  Thanks for the work.

 -George

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

Re: [Xen-devel] [PATCH v8 2/6] arm: filter SMC exceptions with failed condition checks

[Julien Grall]

On 05/07/16 19:37, Tamas K Lengyel wrote:

In AArch32 state, the ARMv8-A architecture permits, but does not require,
this trap to apply to conditional SMC instructions that fail their condition
code check, in the same way as with traps on other conditional instructions.


Please add a quote to the spec.


Signed-off-by: Tamas K Lengyel 
Suggested-by: Julien Grall 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
  xen/arch/arm/traps.c | 15 +--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 44926ca..627e8c9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2507,6 +2507,17 @@ bad_data_abort:
  inject_dabt_exception(regs, info.gva, hsr.len);
  }

+static void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
+{
+if ( !check_conditional_instr(regs, hsr) )


This function is checking the EC, it considers that EC > 0x10 will be 
unconditional. However, the SMC exception class is 0x13 when taken from 
AArch32 and 0x17 when taken from AArch64.


Furthermore, for ARMv7, the register is Reserved UNK/SBZP (see B3-1431 
in ARM DDI 0406C.c). I.e the software should not rely on the field 
reading as all 0s (see Glossary-2736).


For ARMv8, when the SMC is taken from AArch64 (see D7-1942 in ARM DDI 
0487A.j), the register is RES0 which means the software should not rely 
on the value to always be 0 (see Glossary-5734). When the SMC is taken 
from AArch32, the field CV is only valid if CCKNOWNPASS is 1 (this field 
does not exist for the other exception class).


So this code need more extra care. It would also be nice to have a 
description in the code explaining what's going on.



+{
+advance_pc(regs, hsr);
+return;
+}
+
+inject_undef_exception(regs, hsr);
+}
+
  static void enter_hypervisor_head(struct cpu_user_regs *regs)
  {
  if ( guest_mode(regs) )
@@ -2582,7 +2593,7 @@ 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);
+do_trap_smc(regs, hsr);
  break;
  case HSR_EC_HVC32:
  GUEST_BUG_ON(!psr_mode_is_32bit(regs->cpsr));
@@ -2615,7 +2626,7 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs 
*regs)
   */
  GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));
  perfc_incr(trap_smc64);
-inject_undef64_exception(regs, hsr.len);
+do_trap_smc(regs, hsr);
  break;
  case HSR_EC_SYSREG:
  GUEST_BUG_ON(psr_mode_is_32bit(regs->cpsr));



Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH] libxl/netbsd: check num_exec in hotplug function

[Wei Liu]

On Mon, Jul 04, 2016 at 01:13:42PM +0100, Wei Liu wrote:
> This basically replicates the same logic in libxl_linux.c but with one
> change -- only test num_exec == 0 in nic hotplug case because NetBSD let
> QEMU call a script itself. Without this patch libxl will loop
> indefinitely trying to execute hotplug script.
> 
> Reported-by: John Nemeth 
> Signed-off-by: Wei Liu 
> Acked-by: Roger Pau Monné 

Queued.

I think Roger's ack is sufficient for BSD related code and John
confirmed in the other thread this fixed his problem.

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

Re: [Xen-devel] [PATCH 4/6] libxl: debug output for args and env when invoking hotplug script

[Wei Liu]

On Mon, Jul 04, 2016 at 06:06:27PM +0100, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH 4/6] libxl: debug output for args and env when 
> invoking hotplug script"):
> > > From 49714976c5fde3d08baa6f34295b3f7db6a81444 Mon Sep 17 00:00:00 2001
> > > From: Wei Liu 
> > > Date: Fri, 15 Apr 2016 12:56:03 +0100
> > > Subject: [PATCH] libxl: debug output for args and env when invoking 
> > > hotplug
> > >  script
> > > 
> > > Signed-off-by: Wei Liu 
> > 
> > Ian, Ping?
> 
> Sorry, yes.
> 
> Acked-by: Ian Jackson 

Queued. Thanks.

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

Re: [Xen-devel] [PATCH 14/18] arm/altp2m: Add HVMOP_altp2m_set_mem_access.

[Julien Grall]

Hi,

On 04/07/16 12:45, Sergej Proskurin wrote:

--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -2085,6 +2085,159 @@ bool_t p2m_mem_access_check(paddr_t gpa, vaddr_t gla, 
const struct npfec npfec)


[...]


+static inline
+int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
+  struct p2m_domain *ap2m, p2m_access_t a,
+  gfn_t gfn)
+{
+p2m_type_t p2mt;
+xenmem_access_t xma_old;
+paddr_t gpa = pfn_to_paddr(gfn_x(gfn));
+paddr_t maddr, mask = 0;
+unsigned int level;
+unsigned long mattr;
+int rc;
+
+static const p2m_access_t memaccess[] = {
+#define ACCESS(ac) [XENMEM_access_##ac] = p2m_access_##ac
+ACCESS(n),
+ACCESS(r),
+ACCESS(w),
+ACCESS(rw),
+ACCESS(x),
+ACCESS(rx),
+ACCESS(wx),
+ACCESS(rwx),
+ACCESS(rx2rw),
+ACCESS(n2rwx),
+#undef ACCESS
+};
+
+/* Check if entry is part of the altp2m view. */
+spin_lock(>lock);
+maddr = __p2m_lookup(ap2m, gpa, );
+spin_unlock(>lock);
+
+/* Check host p2m if no valid entry in ap2m. */
+if ( maddr == INVALID_PADDR )
+{
+/* Check if entry is part of the host p2m view. */
+spin_lock(>lock);
+maddr = __p2m_lookup(hp2m, gpa, );
+if ( maddr == INVALID_PADDR || p2mt != p2m_ram_rw )
+goto out;
+
+rc = __p2m_get_mem_access(hp2m, gfn, _old);
+if ( rc )
+goto out;
+
+rc = p2m_get_gfn_level_and_attr(hp2m, gpa, , );
+if ( rc )
+goto out;
+spin_unlock(>lock);
+
+mask = level_masks[level];
+
+/* If this is a superpage, copy that first. */
+if ( level != 3 )
+{
+rc = apply_p2m_changes(d, ap2m, INSERT,
+   gpa & mask,
+   (gpa + level_sizes[level]) & mask,
+   maddr & mask, mattr, 0, p2mt,
+   memaccess[xma_old]);
+if ( rc < 0 )
+goto out;
+}
+}
+else
+{
+spin_lock(>lock);
+rc = p2m_get_gfn_level_and_attr(ap2m, gpa, , );
+spin_unlock(>lock);
+if ( rc )
+goto out;
+}
+
+/* Set mem access attributes - currently supporting only one (4K) page. */
+mask = level_masks[3];
+return apply_p2m_changes(d, ap2m, INSERT,
+ gpa & mask,
+ (gpa + level_sizes[level]) & mask,
+ maddr & mask, mattr, 0, p2mt, a);
+
+out:
+if ( spin_is_locked(>lock) )
+spin_unlock(>lock);


I have not spotted this before. spin_is_locked does not ensure that the 
lock was taken by this CPU. So you may unlock for another CPU.



+
+return -ESRCH;
+}
+


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 14/18] arm/altp2m: Add HVMOP_altp2m_set_mem_access.

[Sergej Proskurin]

On 07/06/2016 06:12 PM, Tamas K Lengyel wrote:
> On Wed, Jul 6, 2016 at 8:32 AM, Julien Grall  wrote:
>>
>> On 05/07/16 22:55, Sergej Proskurin wrote:
>>> Hello Julien,
>>>
>>> On 07/05/2016 02:49 PM, Julien Grall wrote:
 Hello Sergej,

 On 04/07/16 12:45, Sergej Proskurin wrote:
> +static inline
> +int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain
> *hp2m,
> +  struct p2m_domain *ap2m, p2m_access_t a,
> +  gfn_t gfn)
> +{

 [...]

> +/* Set mem access attributes - currently supporting only one (4K)
> page. */
> +mask = level_masks[3];
> +return apply_p2m_changes(d, ap2m, INSERT,
> + gpa & mask,
> + (gpa + level_sizes[level]) & mask,
> + maddr & mask, mattr, 0, p2mt, a);

 The operation INSERT will remove the previous mapping and decrease page
 reference for foreign mapping (see p2m_put_l3_page). So if you set the
 memory access for this kind of page, the reference count will be wrong
 afterward.

>>> I see your point. As far as I understand, the purpose of the function
>>> p2m_put_l3_page to simply decrement the ref count of the particular pte
>>> and free the page if its' ref count reaches zero. Since p2m_put_l3_page
>>> is called only twice in p2m.c, we could insert a check
>>> (p2m_is_hostp2m/altp2m) making sure that the ref count is decremented
>>> only if the p2m in question is the hostp2m. This will make sure that the
>>> ref count is maintained correctly.
>>
>> Why don't you use the operation MEMACCESS?
> I believe it would require duplicating some parts of the INSERT
> routine as MEMACCESS assumes the pte is already present in the p2m,
> which may not be the case for ap2m.
>
Yes, exactly. MEMACCESS expects the particular entry to be present in
the associated p2m view. We could use it but only in addition to INSERT.
Otherwise we will not be able to insert the entry into the targeted
altp2m view.Because of this, IMHO it would make sense to introduce an
additional (unlikely) check making sure that the ref count is not
decremented if altp2m is active.

Cheers,
~Sergej


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

Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests

[Julien Grall]

On 06/07/16 17:30, Boris Ostrovsky wrote:

On 07/06/2016 12:04 PM, Julien Grall wrote:

Hi Boris,

On 06/07/16 16:50, Boris Ostrovsky wrote:

On 07/06/2016 07:05 AM, Julien Grall wrote:

+static int populate_acpi_pages(struct xc_dom_image *dom,
+   xen_pfn_t *extents,
+   unsigned int num_pages,
+   struct acpi_ctxt *ctxt)
+{
+int rc;
+xc_interface *xch = dom->xch;
+uint32_t domid = dom->guest_domid;
+unsigned long idx, first_high_idx = (1ull << (32 -
ctxt->page_shift));
+
+for (; num_pages; num_pages--, extents++) {
+
+if (xc_domain_populate_physmap(xch, domid, 1, 0, 0, extents)
== 1)


It looks like this is working because libxl is setting the maximum
size of the domain with some slack (1MB). You might get in trouble if
the slack is reduced or used by someone or the ACPI blob is increasing.



I saw your conversation about slack with Stefano and I am not sure I
understood what it was about. If this was about padding guest's memory
to be able to put there some additional data (such ACPI tables) then
this is not my intention here: if I can't populate (because it is
already populated, I guess) then I try to move memory around with code
below. That's what mem_hole_populate_ram() does as well.


The maximum amount of memory that could be assigned to a domain is
fixed per-domain. This maximum amount does not take into account the
size of the ACPI blob. So you may end up to fail because all the
memory was assigned somewhere else.


Why shouldn't max amount of guest's memory include ACPI tables? I think
we should fail and not rely on this slack if no more memory is available.

...


Because, at least for ARM, the ACPI memory region is not part of the 
"real" RAM. So this value is not taken into account into the current max 
mem. My point is we need to add the size of the ACPI blob to max mem to 
avoid any error here.









However, as mentioned in the ACPI thread [1], all the blobs are
generally loaded by libxc and not libxl. This is more true on ARM
because the guest address space is controlled by libxc (the position
of all the blob are decided by it).


The difference is that I not only load the tables here but also build
them. Which may or may not be the right thing to do in libxc.

I suppose I can defer loading (and then keep pointer to tables in
acpitable_blob) but the then I need to keep RSDP descriptor somewhere
else (it is not part of the blob since it lives in lower MB of the
guest).


The device tree for ARM are built in libxl and loaded for libxc. IHMO,
it would be strange to have a different pattern for ACPI.


Is RSDP part of the ACPI blob for ARM? If not, how do you load it?


RSDP is part of the ACPI blob for ARM.

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 14/18] arm/altp2m: Add HVMOP_altp2m_set_mem_access.

[Julien Grall]

On 06/07/16 17:12, Tamas K Lengyel wrote:

On Wed, Jul 6, 2016 at 8:32 AM, Julien Grall  wrote:



On 05/07/16 22:55, Sergej Proskurin wrote:


Hello Julien,

On 07/05/2016 02:49 PM, Julien Grall wrote:


Hello Sergej,

On 04/07/16 12:45, Sergej Proskurin wrote:


+static inline
+int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain
*hp2m,
+  struct p2m_domain *ap2m, p2m_access_t a,
+  gfn_t gfn)
+{



[...]


+/* Set mem access attributes - currently supporting only one (4K)
page. */
+mask = level_masks[3];
+return apply_p2m_changes(d, ap2m, INSERT,
+ gpa & mask,
+ (gpa + level_sizes[level]) & mask,
+ maddr & mask, mattr, 0, p2mt, a);



The operation INSERT will remove the previous mapping and decrease page
reference for foreign mapping (see p2m_put_l3_page). So if you set the
memory access for this kind of page, the reference count will be wrong
afterward.



I see your point. As far as I understand, the purpose of the function
p2m_put_l3_page to simply decrement the ref count of the particular pte
and free the page if its' ref count reaches zero. Since p2m_put_l3_page
is called only twice in p2m.c, we could insert a check
(p2m_is_hostp2m/altp2m) making sure that the ref count is decremented
only if the p2m in question is the hostp2m. This will make sure that the
ref count is maintained correctly.



Why don't you use the operation MEMACCESS?


I believe it would require duplicating some parts of the INSERT
routine as MEMACCESS assumes the pte is already present in the p2m,
which may not be the case for ap2m.


Hmmm right. The p2m code is already complex so extending MEMACCESS will 
be worst. I don't much like the idea of a check which I think is very 
fragile, although it would be my preference for now.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 09/19] xen: credit2: avoid calling __update_svc_load() multiple times on the same vcpu

[George Dunlap]

On Sat, Jun 18, 2016 at 12:12 AM, Dario Faggioli
 wrote:
> by not resetting the variable that should guard against
> that at the beginning of each step of the outer loop.
>
> Signed-off-by: Dario Faggioli 

Oops. :-/

Reviewed-by: George Dunlap 

And queued.

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

Re: [Xen-devel] [PATCH 08/19] xen: credit2: when tickling, check idle cpus first

[George Dunlap]

On Sat, Jun 18, 2016 at 12:12 AM, Dario Faggioli
 wrote:
> If there are idle pCPUs, it's always better to try to
> "ship" the new vCPU there, instead than letting it
> preempting on a currently busy one.
>
> This commit also adds a cpumask_test_or_cycle() helper
> function, to make it easier to code the preference for
> the pCPU where the vCPU was running before.
>
> Signed-off-by: Dario Faggioli 

Reviewed-by: George Dunlap 

And queued.

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

Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 10:29 AM, Julien Grall  wrote:
>
>
> On 06/07/16 17:05, Tamas K Lengyel wrote:
>>
>> On Wed, Jul 6, 2016 at 9:54 AM, Julien Grall  wrote:
>>>
>>> Taken aside the VMFUNC, it looks like insecure to expose a HVMOP to the
>>> guest which could modify the memaccess attribute of a region.
>>>
>>> I thought the whole purpose of VM introspection is to avoid trusting the
>>> guest (kernel + userspace). The first thing a malware will do is trying
>>> to
>>> do is getting access to the kernel. Once it gets this access, it could
>>> remove all the memory access restriction to avoid trapping.
>>
>>
>> That's why I'm saying systems that use this will likely do extra steps
>> to ensure kernel integrity. In use-cases where this is to be used
>> exclusively for external monitoring the system can be restricted with
>> XSM to not allow the guest to issue the hvmops. And remember, on x86
>> this system is not exclusively used for introspection.
>
>
> I am not aware on how x86 is using alt2pm. And this series didn't give much
> explanation how this is supposed to work...
>
>>>
 As for ARM - as there is no hardware features like this available -
 our goal is to use altp2m in purely external usecases so exposing
 these ops to the guest is not required. For the first prototype it
 made sense to mirror the x86 side to reduce the possibility of
 introducing some bug.
>>>
>>>
>>>
>>> No, this is not the right approach. We should not introduce potential
>>> security issue just because x86 side does it and it "reduces the
>>> possibility
>>> of introducing some bug".
>>>
>>> You will have to give me another reason to accept a such patch.
>>
>>
>> The first revision of a large series is highly unlikely to get
>> accepted on the first run so we have been working with the assumption
>> that there will be new revisions. The prototype has been working well
>> enough for our internal tests to warrant not submitting it as PATCH
>> RFC. Since this is Sergej's first work with Xen it helped to mirror
>> the x86 to get him up to speed while working on the prototype and
>> reducing the complexity he has to keep track of. Now that this phase
>> is complete the adjustments can be made as required, such as not
>> exposing these hvmops to ARM guests.
>
>
> A such large series is already hard to review, it is even harder when the
> contributor leaves code unfinished because he assumed there will be a new
> revision. Actually this is the whole purpose of tagging the series with
> "RFC". It is used that the series is not in a state where it could
> potentially be committed.
>

The code is not in an unfinished state by any means as it passes _our_
tests and works as expected. I think the assumption we made that there
will be required adjustments is very reasonable on any patch series.
So I'm not sure what the problem is.

Tamas

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

Re: [Xen-devel] [PATCH v1 00/20] Make ACPI builder available to components other than hvmloader

[Boris Ostrovsky]

On 07/06/2016 12:04 PM, Roger Pau Monné wrote:
> Hello,
>
> Thanks for the refresh! Do you have this series in some git tree I can pull 
> from?

Sorry, forgot to include this in the message:

git://oss.oracle.com/git/bostrovs/xen.git:acpi_v1



>
> On Tue, Jul 05, 2016 at 03:04:59PM -0400, Boris Ostrovsky wrote:
>> This is V1 of the series posted earlier as an RFC
>>
>> The goal here is to build ACPI tables for PVHv2/HVMlite guests while reusing 
>> existing
>> hvmloader's ACPI builder code. The builder is provided as a library in 
>> tools/libacpi.
>>
>> Main changes from RFC are:
>> * Move toolstack code that loads the tables into PVHv2 guest from libxc to 
>> libxl
>> * Don't provide the builder to hypervisor as we didn't find a user for that 
>> (at least so far)
>> * Update e820 map for PVHv2 guests when ACPI tables are loaded
>> * Don't set HW_REDUCED_ACPI flags: this flag is only available starting with 
>> ACPI v5
> Hm, I still think HW_REDUCED_ACPI should be set for the time being, 
> considering that we don't provide PM timer or RTC for example. Not setting 
> this would be a violation of the ACPI specification, and would mean 
> introducing Xen specific hacks yet again to guest OSes, in order to disable 
> those devices.
>
> Is the fact that HW_REDUCED_ACPI was introduced in ACPI v5 a problem?

Yes, because we build v2 tables and they are somewhat different.

-boris

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

Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests

[Boris Ostrovsky]

On 07/06/2016 12:04 PM, Julien Grall wrote:
> Hi Boris,
>
> On 06/07/16 16:50, Boris Ostrovsky wrote:
>> On 07/06/2016 07:05 AM, Julien Grall wrote:
 +static int populate_acpi_pages(struct xc_dom_image *dom,
 +   xen_pfn_t *extents,
 +   unsigned int num_pages,
 +   struct acpi_ctxt *ctxt)
 +{
 +int rc;
 +xc_interface *xch = dom->xch;
 +uint32_t domid = dom->guest_domid;
 +unsigned long idx, first_high_idx = (1ull << (32 -
 ctxt->page_shift));
 +
 +for (; num_pages; num_pages--, extents++) {
 +
 +if (xc_domain_populate_physmap(xch, domid, 1, 0, 0, extents)
 == 1)
>>>
>>> It looks like this is working because libxl is setting the maximum
>>> size of the domain with some slack (1MB). You might get in trouble if
>>> the slack is reduced or used by someone or the ACPI blob is increasing.
>>
>>
>> I saw your conversation about slack with Stefano and I am not sure I
>> understood what it was about. If this was about padding guest's memory
>> to be able to put there some additional data (such ACPI tables) then
>> this is not my intention here: if I can't populate (because it is
>> already populated, I guess) then I try to move memory around with code
>> below. That's what mem_hole_populate_ram() does as well.
>
> The maximum amount of memory that could be assigned to a domain is
> fixed per-domain. This maximum amount does not take into account the
> size of the ACPI blob. So you may end up to fail because all the
> memory was assigned somewhere else.

Why shouldn't max amount of guest's memory include ACPI tables? I think
we should fail and not rely on this slack if no more memory is available.

...

>
>>
>>> However, as mentioned in the ACPI thread [1], all the blobs are
>>> generally loaded by libxc and not libxl. This is more true on ARM
>>> because the guest address space is controlled by libxc (the position
>>> of all the blob are decided by it).
>>
>> The difference is that I not only load the tables here but also build
>> them. Which may or may not be the right thing to do in libxc.
>>
>> I suppose I can defer loading (and then keep pointer to tables in
>> acpitable_blob) but the then I need to keep RSDP descriptor somewhere
>> else (it is not part of the blob since it lives in lower MB of the
>> guest).
>
> The device tree for ARM are built in libxl and loaded for libxc. IHMO,
> it would be strange to have a different pattern for ACPI. 

Is RSDP part of the ACPI blob for ARM? If not, how do you load it?

-boris


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

Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Julien Grall]

On 06/07/16 17:05, Tamas K Lengyel wrote:

On Wed, Jul 6, 2016 at 9:54 AM, Julien Grall  wrote:

Taken aside the VMFUNC, it looks like insecure to expose a HVMOP to the
guest which could modify the memaccess attribute of a region.

I thought the whole purpose of VM introspection is to avoid trusting the
guest (kernel + userspace). The first thing a malware will do is trying to
do is getting access to the kernel. Once it gets this access, it could
remove all the memory access restriction to avoid trapping.


That's why I'm saying systems that use this will likely do extra steps
to ensure kernel integrity. In use-cases where this is to be used
exclusively for external monitoring the system can be restricted with
XSM to not allow the guest to issue the hvmops. And remember, on x86
this system is not exclusively used for introspection.


I am not aware on how x86 is using alt2pm. And this series didn't give 
much explanation how this is supposed to work...





As for ARM - as there is no hardware features like this available -
our goal is to use altp2m in purely external usecases so exposing
these ops to the guest is not required. For the first prototype it
made sense to mirror the x86 side to reduce the possibility of
introducing some bug.



No, this is not the right approach. We should not introduce potential
security issue just because x86 side does it and it "reduces the possibility
of introducing some bug".

You will have to give me another reason to accept a such patch.


The first revision of a large series is highly unlikely to get
accepted on the first run so we have been working with the assumption
that there will be new revisions. The prototype has been working well
enough for our internal tests to warrant not submitting it as PATCH
RFC. Since this is Sergej's first work with Xen it helped to mirror
the x86 to get him up to speed while working on the prototype and
reducing the complexity he has to keep track of. Now that this phase
is complete the adjustments can be made as required, such as not
exposing these hvmops to ARM guests.


A such large series is already hard to review, it is even harder when 
the contributor leaves code unfinished because he assumed there will be 
a new revision. Actually this is the whole purpose of tagging the series 
with "RFC". It is used that the series is not in a state where it could 
potentially be committed.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 07/19] xen: credit2: prevent load balancing to go mad if time goes backwards

[George Dunlap]

On Mon, Jun 20, 2016 at 9:02 AM, Jan Beulich  wrote:
 On 18.06.16 at 01:12,  wrote:
>> This really should not happen, but:
>>  1. it does happen! Investigation is ongoing here:
>> http://lists.xen.org/archives/html/xen-devel/2016-06/msg00922.html
>>  2. even when 1 will be fixed it makes sense and is easy enough
>> to have a 'safety catch' for it.
>>
>> The reason why this is particularly bad for Credit2 is that
>> negative values of delta mean out of scale high load (because
>> of the conversion to unsigned). This, for instance in the
>> case of runqueue load, results in a runqueue having its load
>> updated to values of the order of 1% or so, which in turns
>> means that the load balancer will migrate everything off from
>> the pCPUs in the runqueue, and leave them idle until the load
>> gets back to something sane... which may indeed take a while!
>>
>> This is not a fix for the problem of time going backwards. In
>> fact, if that happens a lot, load tracking accuracy is still
>> compromized, but at least the effect is a lot less bad than
>> before.
>>
>> Signed-off-by: Dario Faggioli 
>> ---
>> Cc: George Dunlap 
>> Cc: Anshul Makkar 
>> Cc: David Vrabel 
>> ---
>>  xen/common/sched_credit2.c |   12 
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
>> index 50f8dfd..b73d034 100644
>> --- a/xen/common/sched_credit2.c
>> +++ b/xen/common/sched_credit2.c
>> @@ -404,6 +404,12 @@ __update_runq_load(const struct scheduler *ops,
>>  else
>>  {
>>  delta = now - rqd->load_last_update;
>> +if ( unlikely(delta < 0) )
>> +{
>> +d2printk("%s: Time went backwards? now %"PRI_stime" llu 
>> %"PRI_stime"\n",
>> + __func__, now, rqd->load_last_update);
>> +delta = 0;
>> +}
>>
>>  rqd->avgload =
>>  ( ( delta * ( (unsigned long long)rqd->load << 
>> prv->load_window_shift ) )
>> @@ -455,6 +461,12 @@ __update_svc_load(const struct scheduler *ops,
>>  else
>>  {
>>  delta = now - svc->load_last_update;
>> +if ( unlikely(delta < 0) )
>> +{
>> +d2printk("%s: Time went backwards? now %"PRI_stime" llu 
>> %"PRI_stime"\n",
>> + __func__, now, svc->load_last_update);
>> +delta = 0;
>> +}
>>
>>  svc->avgload =
>>  ( ( delta * ( (unsigned long long)vcpu_load << 
>> prv->load_window_shift ) )
>
> Do the absolute times really matter here? I.e. wouldn't it be more
> useful to simply log the value of delta?
>
> Also, may I ask you to use the L modifier in favor of the ll one, for
> being one byte shorter (and hence, even if just very slightly,
> reducing both image size and cache pressure)?
>
> And finally, instead of logging function names, could the two
> messages be made distinguishable by other means resulting in less
> data issued to the log (and potentially needing transmission over
> a slow serial line)?

The reason this is under a "d2printk" is because it's really only to
help developers in debugging.  In-tree this warning isn't even on with
debug=y; you have to go to the top of the file and change the #define
to make it even exist.

Given that, I don't think the quibbles over the code size or the
length of what's logged really matter.  I think we should just take it
as it is.

Reviewed-by: George Dunlap 

 -George

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

Re: [Xen-devel] [PATCH v3 4/8] x86/vm_event/monitor/cr: check for vm-event subscriber on domctl

[Corneliu ZUZU]

On 7/6/2016 7:15 PM, Corneliu ZUZU wrote:

On 7/6/2016 7:01 PM, Jan Beulich wrote:

On 06.07.16 at 17:52,  wrote:

Also adjust returned error code for similar check from -EINVAL to more
descriptive -ENOSYS (XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP).

I'm not sure that's more descriptive, and we really try to not use
ENOSYS for other than unimplemented hypercalls. EOPNOTSUPP
perhaps?

Jan


Well,  it's not quite an 'unsupported operation' and neither does the 
toolstack user communicate an 'invalid value', he must just be noticed 
that something (the vm-event subsystem) needs to be initialised before 
the operation can be done.
But I agree ENOSYS is not acceptable either (I only now see it 
signifies "Function not implemented", my bad for not peeking at that 
before using it, I expected differently).


What about ENODEV, i.e. "No such device."? We need an error code 
saying "Device not initialised"...


Thanks,
Corneliu.


Or ENOTCONN..?

Corneliu.

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

Re: [Xen-devel] default XSM policy for PCI passthrough for unlabeled resources.

[anshul makkar]

On 06/07/16 16:59, Daniel De Graaf wrote:

On 07/06/2016 11:34 AM, anshul makkar wrote:

Hi,


It allows the resource to be added and removed by the source domain to
target domain, but its use by target domain is blocked.


This rule only mandates the use of resource_type for resource types.  If
you are creating a new resource type, follow the example in nic_dev.te.
Agreed, but inherently it means that "use" of any unlabeled resource be 
it irq, ioport or iomem or nic_dev is restricted.



The resource can be used only if it has been labeled using
flask-label-pci command which needs to be rerun after every boot and
after every policy reload.


Yes; this gives the most control over what resources can be delegated.
Policy reloads are supposed to be rare (on a production system) and you
already need special boot scripts (or parameters) to set up the device
for passthrough, so this can be added there.  However, I agree this can
be more work than a "default" FLASK policy should require.

Try adding a module with the following rules, which should allow domU to
use unlabeled devices:

use_device(domU_t, irq_t)
use_device(domU_t, ioport_t)
use_device(domU_t, iomem_t)
use_device(domU_t, device_t)
Yes, it does work , but I have added these in delegate_device to make it 
restrict to the case where there is delegation.


If this works, that module could be added to the default policy.


Given that what we ship is just a sample default policy for reference
which is expected to be permissive in most of the scenarios so that it
doesn't affect the basic functionalities, is this "neverallow" rule
needed ?

Thanks
Anshul Makkar


The neverallow rules are just there to ensure that the attributes are
being used correctly.


Anshul

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

Re: [Xen-devel] [PATCH v3 4/8] x86/vm_event/monitor/cr: check for vm-event subscriber on domctl

[Corneliu ZUZU]

On 7/6/2016 7:01 PM, Jan Beulich wrote:

On 06.07.16 at 17:52,  wrote:

Also adjust returned error code for similar check from -EINVAL to more
descriptive -ENOSYS (XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP).

I'm not sure that's more descriptive, and we really try to not use
ENOSYS for other than unimplemented hypercalls. EOPNOTSUPP
perhaps?

Jan


Well,  it's not quite an 'unsupported operation' and neither does the 
toolstack user communicate an 'invalid value', he must just be noticed 
that something (the vm-event subsystem) needs to be initialised before 
the operation can be done.
But I agree ENOSYS is not acceptable either (I only now see it signifies 
"Function not implemented", my bad for not peeking at that before using 
it, I expected differently).


What about ENODEV, i.e. "No such device."? We need an error code saying 
"Device not initialised"...


Thanks,
Corneliu.

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

Re: [Xen-devel] [PATCH 06/19] xen: credit2: read NOW() with the proper runq lock held

[George Dunlap]

On Mon, Jun 20, 2016 at 8:56 AM, Jan Beulich  wrote:
 On 18.06.16 at 01:12,  wrote:
>> Yet another situation very similar to 779511f4bf5ae
>> ("sched: avoid races on time values read from NOW()").
>>
>> In fact, when more than one runqueue is involved, we need
>> to make sure that the following does not happen:
>>  1. take the lock of 1st runq
>>  2. now = NOW()
>>  3. take the lock of 2nd runq
>>  4. use now
>>
>> as, if we have to wait at step 3, the value in now may
>> be stale when we get to use it at step 4.
>
> Is this really meaningful here? We're talking of trylocks, which don't
> incur any delay other than the latency of the LOCKed (on x86)
> instruction to determine lock availability.

This makes sense to me -- Dario?

 -George

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

Re: [Xen-devel] [PATCH 14/18] arm/altp2m: Add HVMOP_altp2m_set_mem_access.

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 8:32 AM, Julien Grall  wrote:
>
>
> On 05/07/16 22:55, Sergej Proskurin wrote:
>>
>> Hello Julien,
>>
>> On 07/05/2016 02:49 PM, Julien Grall wrote:
>>>
>>> Hello Sergej,
>>>
>>> On 04/07/16 12:45, Sergej Proskurin wrote:

 +static inline
 +int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain
 *hp2m,
 +  struct p2m_domain *ap2m, p2m_access_t a,
 +  gfn_t gfn)
 +{
>>>
>>>
>>> [...]
>>>
 +/* Set mem access attributes - currently supporting only one (4K)
 page. */
 +mask = level_masks[3];
 +return apply_p2m_changes(d, ap2m, INSERT,
 + gpa & mask,
 + (gpa + level_sizes[level]) & mask,
 + maddr & mask, mattr, 0, p2mt, a);
>>>
>>>
>>> The operation INSERT will remove the previous mapping and decrease page
>>> reference for foreign mapping (see p2m_put_l3_page). So if you set the
>>> memory access for this kind of page, the reference count will be wrong
>>> afterward.
>>>
>>
>> I see your point. As far as I understand, the purpose of the function
>> p2m_put_l3_page to simply decrement the ref count of the particular pte
>> and free the page if its' ref count reaches zero. Since p2m_put_l3_page
>> is called only twice in p2m.c, we could insert a check
>> (p2m_is_hostp2m/altp2m) making sure that the ref count is decremented
>> only if the p2m in question is the hostp2m. This will make sure that the
>> ref count is maintained correctly.
>
>
> Why don't you use the operation MEMACCESS?

I believe it would require duplicating some parts of the INSERT
routine as MEMACCESS assumes the pte is already present in the p2m,
which may not be the case for ap2m.

Tamas

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

Re: [Xen-devel] [PATCH 0/3] libxl: add framework for device types

[Ian Jackson]

Juergen Gross writes ("Re: [PATCH 0/3] libxl: add framework for device types"):
> On 06/07/16 14:47, Ian Jackson wrote:
> > Juergen Gross writes ("Re: [PATCH 0/3] libxl: add framework for device 
> > types"):
> >> On 06/07/16 13:04, Ian Jackson wrote:
>  +for (i = 0; i < d_config->num_pcidevs; i++) {
>  +rc = libxl__device_pci_add(gc, domid, _config->pcidevs[i], 1);
>  +if (rc < 0) {
>  +LOG(ERROR, "libxl_device_pci_add failed: %d", rc);
>  +goto out;
>  +}
>  +}
>  +
> >>>
> >>> And there is similar code in 3/3 for dtdevs.  Could that be lifted
> >>> away somehow ?  (You'd have to take some care about the types, sadly;
> >>> ie, I think libxl__device_pci_add might have to start to take a
> >>> void*; maybe some macros could make things typesafe?)
> >>
> >> I thought about this idea already. I think we would end up with more
> >> code which would be rather unpleasant to read. Main reason is the
> >> need for a dtdev wrapper function and the pci backend creation.
> > 
> > I'm not sure what you mean by dtdev wrapper function.
> 
> The loop for dtdev calls a xc_ function with different parameters than
> the one for pci. We'd need a libxl__device_dtdev_add() wrapper function
> to do the xc_ call.

Oh, I see.

Sorry.  I was just suggesting that the iteration over the devices, and
the error handling, could be lifted out.  Not the contents of
libxl__device_FOO_add.

And yes, we'd need libxl__device_dtdev_add.

> 
> > But if you don't think this is feasible I won't insist on it.  The
> > approach you have is already a big improvement.
> 
> Thanks.
> 
> I'm planning to add more to it (e.g. I'd like to get rid of the
> MERGE() macro in libxl_retrieve_domain_configuration() in favor of a
> device type hook in order to be able to have _all_ stuff for one type
> in one source file.

Right.  That would be brilliant.

Ian.

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

Re: [Xen-devel] [PATCH 05/19] xen: credit2: do not warn if calling burn_credits more than once

[George Dunlap]

On Sat, Jun 18, 2016 at 12:11 AM, Dario Faggioli
 wrote:
> on the same vcpu, without NOW() having changed.
>
> This is, in fact, a legitimate use case. If it happens,
> we should just do nothing, without producing any warning
> or debug message.
>
> While there, fix style and add a couple of branching
> hints.
>
> Signed-off-by: Dario Faggioli 

Reviewed-by: George Dunlap 

And queued.

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

Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 9:54 AM, Julien Grall  wrote:
> (Add x86 maintainers)
>
>
> On 06/07/16 16:23, Tamas K Lengyel wrote:
>>
>> On Wed, Jul 6, 2016 at 7:43 AM, Julien Grall  wrote:
>>>
>>> Hello Sergej,
>>>
>>>
>>> On 06/07/16 10:14, Sergej Proskurin wrote:


 On 07/05/2016 12:19 PM, Julien Grall wrote:
>
>
> Hello Sergej,
>
> On 04/07/16 12:45, Sergej Proskurin wrote:
>>
>>
>> +static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
>> +{
>> +struct xen_hvm_altp2m_op a;
>> +struct domain *d = NULL;
>> +int rc = 0;
>> +
>> +if ( !hvm_altp2m_supported() )
>> +return -EOPNOTSUPP;
>> +
>> +if ( copy_from_guest(, arg, 1) )
>> +return -EFAULT;
>> +
>> +if ( a.pad1 || a.pad2 ||
>> + (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
>> + (a.cmd < HVMOP_altp2m_get_domain_state) ||
>> + (a.cmd > HVMOP_altp2m_change_gfn) )
>> +return -EINVAL;
>> +
>> +d = (a.cmd != HVMOP_altp2m_vcpu_enable_notify) ?
>> +rcu_lock_domain_by_any_id(a.domain) :
>> rcu_lock_current_domain();
>> +
>> +if ( d == NULL )
>> +return -ESRCH;
>> +
>> +if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
>> + (a.cmd != HVMOP_altp2m_set_domain_state) &&
>> + !d->arch.altp2m_active )
>> +{
>> +rc = -EOPNOTSUPP;
>> +goto out;
>> +}
>> +
>> +if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
>> +goto out;
>
>
>
> I think this is the best place to ask a couple of questions related to
> who can access altp2m. Based on this call, a guest is allowed to
> manage its own altp2m. Can you explain why we would want a guest to do
> that?
>

 On x86, altp2m might be used by the guest in the #VE (Virtualization
 Exception). On ARM, there is indeed not necessary for a guest to access
 altp2m. Could you provide me with information, how to best restrict
 non-privileged guests (not only dom0) from accessing these HVMOPs? Can
 thisbedone by means of xsm? Thank you.
>>>
>>>
>>>
>>> This does not looks safe for both x86 and ARM. From my understanding a
>>> malware would be able to modify an altp2m, switching between 2 view...
>>> which
>>> would lead to remove the entire purpose of altp2m.
>>
>>
>> Well, the whole purpose of the VMFUNC instruction right now is to be
>> able to switch the hypervisor's tables (like altp2m) from within the
>> guest. So yes, if you have a malicious guest then it could control
>> it's own altp2m views. I would assume there are other safe-guards
>> in-place for systems that use this to ensure kernel-integrity and thus
>> prevent arbitrary use of these functions. AFAIK there are only
>> experimental systems based on this so whether it's less or more secure
>> is debatable and likely depend on your implementation.
>
>
> Taken aside the VMFUNC, it looks like insecure to expose a HVMOP to the
> guest which could modify the memaccess attribute of a region.
>
> I thought the whole purpose of VM introspection is to avoid trusting the
> guest (kernel + userspace). The first thing a malware will do is trying to
> do is getting access to the kernel. Once it gets this access, it could
> remove all the memory access restriction to avoid trapping.

That's why I'm saying systems that use this will likely do extra steps
to ensure kernel integrity. In use-cases where this is to be used
exclusively for external monitoring the system can be restricted with
XSM to not allow the guest to issue the hvmops. And remember, on x86
this system is not exclusively used for introspection.

>
>> As for ARM - as there is no hardware features like this available -
>> our goal is to use altp2m in purely external usecases so exposing
>> these ops to the guest is not required. For the first prototype it
>> made sense to mirror the x86 side to reduce the possibility of
>> introducing some bug.
>
>
> No, this is not the right approach. We should not introduce potential
> security issue just because x86 side does it and it "reduces the possibility
> of introducing some bug".
>
> You will have to give me another reason to accept a such patch.

The first revision of a large series is highly unlikely to get
accepted on the first run so we have been working with the assumption
that there will be new revisions. The prototype has been working well
enough for our internal tests to warrant not submitting it as PATCH
RFC. Since this is Sergej's first work with Xen it helped to mirror
the x86 to get him up to speed while working on the prototype and
reducing the complexity he has to keep track of. Now that this phase
is complete the adjustments can be made as required, such as not
exposing these hvmops to ARM guests.

Cheers,
Tamas

Re: [Xen-devel] [PATCH 04/19] xen: credit2: kill useless helper function choose_cpu

[George Dunlap]

On Sat, Jun 18, 2016 at 12:11 AM, Dario Faggioli
 wrote:
> In fact, it has the same signature of csched2_cpu_pick,
> which also is its uniqe caller.
>
> Signed-off-by: Dario Faggioli 

Reviewed-by: George Dunlap 

And queued, fixing up the spelling of "unique".

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

Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests

[Julien Grall]

Hi Boris,

On 06/07/16 16:50, Boris Ostrovsky wrote:

On 07/06/2016 07:05 AM, Julien Grall wrote:

+static int populate_acpi_pages(struct xc_dom_image *dom,
+   xen_pfn_t *extents,
+   unsigned int num_pages,
+   struct acpi_ctxt *ctxt)
+{
+int rc;
+xc_interface *xch = dom->xch;
+uint32_t domid = dom->guest_domid;
+unsigned long idx, first_high_idx = (1ull << (32 -
ctxt->page_shift));
+
+for (; num_pages; num_pages--, extents++) {
+
+if (xc_domain_populate_physmap(xch, domid, 1, 0, 0, extents)
== 1)


It looks like this is working because libxl is setting the maximum
size of the domain with some slack (1MB). You might get in trouble if
the slack is reduced or used by someone or the ACPI blob is increasing.



I saw your conversation about slack with Stefano and I am not sure I
understood what it was about. If this was about padding guest's memory
to be able to put there some additional data (such ACPI tables) then
this is not my intention here: if I can't populate (because it is
already populated, I guess) then I try to move memory around with code
below. That's what mem_hole_populate_ram() does as well.


The maximum amount of memory that could be assigned to a domain is fixed 
per-domain. This maximum amount does not take into account the size of 
the ACPI blob. So you may end up to fail because all the memory was 
assigned somewhere else.







+continue;
+
+if (dom->highmem_end) {
+idx = --dom->highmem_end;
+if ( idx == first_high_idx )
+dom->highmem_end = 0;
+} else
+idx = --dom->lowmem_end;
+
+rc = xc_domain_add_to_physmap(xch, domid,
+  XENMAPSPACE_gmfn,
+  idx, *extents);
+if (rc)
+return rc;
+}
+
+return 0;
+}
+
+int libxl__dom_load_acpi(libxl__gc *gc,
+ libxl_domain_build_info *info,
+ struct xc_dom_image *dom)
+{
+struct acpi_config config = {0};
+struct acpi_ctxt ctxt;
+uint32_t domid = dom->guest_domid;
+xc_interface *xch = dom->xch;
+int rc, i, acpi_pages_num;
+xen_pfn_t extent, *extents;
+void *acpi_pages, *guest_acpi_pages = NULL;
+unsigned long page_mask;
+
+if ((info->type != LIBXL_DOMAIN_TYPE_HVM) ||
+(info->device_model_version !=
LIBXL_DEVICE_MODEL_VERSION_NONE))
+return 0;
+
+ctxt.page_size = XC_DOM_PAGE_SIZE(dom);
+ctxt.page_shift = XC_DOM_PAGE_SHIFT(dom);
+page_mask = (1UL << ctxt.page_shift) - 1;
+
+ctxt.mem_ops.alloc = mem_alloc;
+ctxt.mem_ops.v2p = virt_to_phys;
+
+rc = init_acpi_config(gc, dom, info, );
+if (rc) {
+LOG(ERROR, "%s: init_acpi_config failed (rc=%d)",
__FUNCTION__, rc);
+return rc;
+}
+
+/* Map page that will hold RSDP */
+extent = RSDP_ADDRESS >> ctxt.page_shift;
+rc = populate_acpi_pages(dom, , 1, );
+if (rc) {
+LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
+__FUNCTION__, rc);
+goto out;
+}
+config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
+  ctxt.page_size,
+  PROT_READ |
PROT_WRITE,
+  RSDP_ADDRESS

ctxt.page_shift);

+if (!config.rsdp) {
+LOG(ERROR, "%s: Can't map acpi_physical", __FUNCTION__);
+rc = -1;
+goto out;
+}
+
+/* Map acpi_info page */
+extent = ACPI_INFO_PHYSICAL_ADDRESS >> ctxt.page_shift;
+rc = populate_acpi_pages(dom, , 1, );
+if (rc) {
+LOG(ERROR, "%s: populate_acpi_pages for acpi_info failed
with %d",
+__FUNCTION__, rc);
+goto out;
+}
+
+config.ainfop = (unsigned long)xc_map_foreign_range(xch, domid,
+ctxt.page_size,
+PROT_READ |
PROT_WRITE,
+
ACPI_INFO_PHYSICAL_ADDRESS >> ctxt.page_shift);


Loading the ACPI blob on ARM will be very similar except for the base
address. So it would be nice to share some code with it.

However, as mentioned in the ACPI thread [1], all the blobs are
generally loaded by libxc and not libxl. This is more true on ARM
because the guest address space is controlled by libxc (the position
of all the blob are decided by it).


The difference is that I not only load the tables here but also build
them. Which may or may not be the right thing to do in libxc.

I suppose I can defer loading (and then keep pointer to tables in
acpitable_blob) but the then I need to keep RSDP descriptor somewhere
else (it is not part of the blob since it lives in lower MB of the guest).


The device tree for ARM are built in libxl and loaded for libxc. IHMO, 
it would be strange to have a different 

Re: [Xen-devel] [PATCH v1 00/20] Make ACPI builder available to components other than hvmloader

[Roger Pau Monné]

Hello,

Thanks for the refresh! Do you have this series in some git tree I can pull 
from?

On Tue, Jul 05, 2016 at 03:04:59PM -0400, Boris Ostrovsky wrote:
> This is V1 of the series posted earlier as an RFC
> 
> The goal here is to build ACPI tables for PVHv2/HVMlite guests while reusing 
> existing
> hvmloader's ACPI builder code. The builder is provided as a library in 
> tools/libacpi.
> 
> Main changes from RFC are:
> * Move toolstack code that loads the tables into PVHv2 guest from libxc to 
> libxl
> * Don't provide the builder to hypervisor as we didn't find a user for that 
> (at least so far)
> * Update e820 map for PVHv2 guests when ACPI tables are loaded
> * Don't set HW_REDUCED_ACPI flags: this flag is only available starting with 
> ACPI v5

Hm, I still think HW_REDUCED_ACPI should be set for the time being, 
considering that we don't provide PM timer or RTC for example. Not setting 
this would be a violation of the ACPI specification, and would mean 
introducing Xen specific hacks yet again to guest OSes, in order to disable 
those devices.

Is the fact that HW_REDUCED_ACPI was introduced in ACPI v5 a problem?

Roger.

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

Re: [Xen-devel] [PATCH v3 4/8] x86/vm_event/monitor/cr: check for vm-event subscriber on domctl

[Jan Beulich]

>>> On 06.07.16 at 17:52,  wrote:
> Also adjust returned error code for similar check from -EINVAL to more
> descriptive -ENOSYS (XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP).

I'm not sure that's more descriptive, and we really try to not use
ENOSYS for other than unimplemented hypercalls. EOPNOTSUPP
perhaps?

Jan


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

Re: [Xen-devel] [PATCH 03/19] xen: credit2: insert and tickle don't need a cpu parameter

[George Dunlap]

On Sat, Jun 18, 2016 at 12:11 AM, Dario Faggioli
 wrote:
> In fact, they always operate on the svc->processor of
> the csched2_vcpu passed to them.
>
> No functional change intended.
>
> Signed-off-by: Dario Faggioli 

Acked-by: George Dunlap 

And queued this one and patch 2.

Good to see those finally go. :-)

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

Re: [Xen-devel] default XSM policy for PCI passthrough for unlabeled resources.

[Daniel De Graaf]

On 07/06/2016 11:34 AM, anshul makkar wrote:

Hi,

Default XSM policy doesn't allow the use of unlabeled PCI resources that have 
been passed through to target domain.

xen.te
# Resources must be declared using . resource_type
neverallow * ~resource_type:resource use;

It allows the resource to be added and removed by the source domain to target 
domain, but its use by target domain is blocked.


This rule only mandates the use of resource_type for resource types.  If you 
are creating a new resource type, follow the example in nic_dev.te.


The resource can be used only if it has been labeled using flask-label-pci 
command which needs to be rerun after every boot and after every policy reload.


Yes; this gives the most control over what resources can be delegated.  Policy reloads 
are supposed to be rare (on a production system) and you already need special boot 
scripts (or parameters) to set up the device for passthrough, so this can be added there. 
 However, I agree this can be more work than a "default" FLASK policy should 
require.


The above approach reduces the flexibility and necessitates admin intervention to give 
passthrough rights after host has booted. Also, in general if I want to allow all domUs 
to have PCI passthough access to all PCI device, I have no other way apart from disabling 
the "neverallow" rule.


Try adding a module with the following rules, which should allow domU to use 
unlabeled devices:

use_device(domU_t, irq_t)
use_device(domU_t, ioport_t)
use_device(domU_t, iomem_t)
use_device(domU_t, device_t)

If this works, that module could be added to the default policy.


Given that what we ship is just a sample default policy for reference which is expected 
to be permissive in most of the scenarios so that it doesn't affect the basic 
functionalities, is this "neverallow" rule needed ?

Thanks
Anshul Makkar


The neverallow rules are just there to ensure that the attributes are being 
used correctly.

--
Daniel De Graaf
National Security Agency

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

[Xen-devel] [PATCH v3 8/8] minor #include change

[Corneliu ZUZU]

Move xen/paging.h #include from hvm/monitor.h to hvm/monitor.c (include strictly
where needed) and also change to asm/paging.h (include strictly what's needed).

Signed-off-by: Corneliu ZUZU 
Acked-by: Tamas K Lengyel 
Acked-by: Razvan Cojocaru 
---
Changed since v2: 
---
 xen/arch/x86/hvm/monitor.c| 1 +
 xen/include/asm-x86/hvm/monitor.h | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index bbe5952..db6db6f 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
diff --git a/xen/include/asm-x86/hvm/monitor.h 
b/xen/include/asm-x86/hvm/monitor.h
index 8d4ef19..1c8ec6c 100644
--- a/xen/include/asm-x86/hvm/monitor.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -19,7 +19,6 @@
 #ifndef __ASM_X86_HVM_MONITOR_H__
 #define __ASM_X86_HVM_MONITOR_H__
 
-#include 
 #include 
 
 enum hvm_monitor_debug_type
-- 
2.5.0


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

[Xen-devel] [PATCH v3 7/8] minor fixes (formatting, comments, unused includes etc.)

[Corneliu ZUZU]

Minor fixes:
 - remove some empty lines
 - remove some unused includes
 - multi-line comment fixes
 - 80-columns formatting fixes

Signed-off-by: Corneliu ZUZU 
Acked-by: Razvan Cojocaru 
Acked-by: Julien Grall 
---
Changed since v2: 
---
 xen/arch/arm/domain.c |  1 -
 xen/arch/arm/traps.c  |  1 -
 xen/arch/x86/hvm/hvm.c|  3 ---
 xen/arch/x86/hvm/vmx/vmx.c|  1 -
 xen/arch/x86/monitor.c|  1 -
 xen/arch/x86/vm_event.c   |  3 ---
 xen/common/monitor.c  |  1 -
 xen/common/vm_event.c |  6 --
 xen/include/asm-arm/vm_event.h|  9 +++--
 xen/include/asm-x86/hvm/monitor.h |  1 -
 xen/include/asm-x86/monitor.h |  3 ---
 xen/include/asm-x86/vm_event.h|  1 -
 xen/include/public/vm_event.h | 38 +++---
 xen/include/xen/vm_event.h|  1 -
 14 files changed, 26 insertions(+), 44 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 6ce4645..61fc08e 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -294,7 +294,6 @@ static void continue_new_vcpu(struct vcpu *prev)
 else
 /* check_wakeup_from_wait(); */
 reset_stack_and_jump(return_to_new_vcpu64);
-
 }
 
 void context_switch(struct vcpu *prev, struct vcpu *next)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 44926ca..fb01703 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -439,7 +439,6 @@ static void inject_abt32_exception(struct cpu_user_regs 
*regs,
 far |= addr << 32;
 WRITE_SYSREG(far, FAR_EL1);
 WRITE_SYSREG(fsr, IFSR32_EL2);
-
 #endif
 }
 else
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 091cabe..32d2d5e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -31,7 +31,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -63,11 +62,9 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 0fa3fea..ee108b5 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -34,7 +34,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 4d4db33..8423931 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -21,7 +21,6 @@
 
 #include 
 #include 
-#include 
 #include 
 
 static inline void write_ctrlreg_adjust_traps(struct domain *d, uint8_t index)
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index e37d6d3..2b1abe2 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -18,9 +18,6 @@
  * License along with this program; If not, see .
  */
 
-#include 
-#include 
-#include 
 #include 
 
 /* Implicitly serialized by the domctl lock. */
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index 436214a..08d1d98 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 40a7e56..93747eb 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -782,8 +782,10 @@ void vm_event_vcpu_unpause(struct vcpu *v)
 {
 int old, new, prev = v->vm_event_pause_count.counter;
 
-/* All unpause requests as a result of toolstack responses.  Prevent
- * underflow of the vcpu pause count. */
+/*
+ * All unpause requests as a result of toolstack responses.
+ * Prevent underflow of the vcpu pause count.
+ */
 do
 {
 old = prev;
diff --git a/xen/include/asm-arm/vm_event.h b/xen/include/asm-arm/vm_event.h
index a3fc4ce..ccc4b60 100644
--- a/xen/include/asm-arm/vm_event.h
+++ b/xen/include/asm-arm/vm_event.h
@@ -23,21 +23,18 @@
 #include 
 #include 
 
-static inline
-int vm_event_init_domain(struct domain *d)
+static inline int vm_event_init_domain(struct domain *d)
 {
 /* Nothing to do. */
 return 0;
 }
 
-static inline
-void vm_event_cleanup_domain(struct domain *d)
+static inline void vm_event_cleanup_domain(struct domain *d)
 {
 memset(>monitor, 0, sizeof(d->monitor));
 }
 
-static inline
-void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
+static inline void vm_event_toggle_singlestep(struct domain *d, struct vcpu *v)
 {
 /* Not supported on ARM. */
 }
diff --git a/xen/include/asm-x86/hvm/monitor.h 
b/xen/include/asm-x86/hvm/monitor.h
index 8b0f119..8d4ef19 100644
--- a/xen/include/asm-x86/hvm/monitor.h
+++ b/xen/include/asm-x86/hvm/monitor.h
@@ -19,7 +19,6 @@
 #ifndef __ASM_X86_HVM_MONITOR_H__
 #define __ASM_X86_HVM_MONITOR_H__
 
-#include 
 #include 
 #include 
 
diff --git 

[Xen-devel] [PATCH v3 6/8] x86/vm-event: minor ASSERT fix, add 'unlikely'

[Corneliu ZUZU]

Minor fixes:

* vm_event_register_write_resume: ASSERT on non-NULL v->arch.vm_event instead of
  >arch.vm_event->write_data.
* add 'unlikely' in if

Signed-off-by: Corneliu ZUZU 
---
Changed since v2: 
---
 xen/arch/x86/hvm/hvm.c  | 2 +-
 xen/arch/x86/vm_event.c | 6 --
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index ac6d9eb..091cabe 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -475,7 +475,7 @@ void hvm_do_resume(struct vcpu *v)
 
 if ( unlikely(v->arch.vm_event) )
 {
-if ( v->arch.vm_event->emulate_flags )
+if ( unlikely(v->arch.vm_event->emulate_flags) )
 {
 enum emul_kind kind = EMUL_KIND_NORMAL;
 
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index ff2ba92..e37d6d3 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -96,14 +96,16 @@ void vm_event_register_write_resume(struct vcpu *v, 
vm_event_response_t *rsp)
 {
 if ( rsp->flags & VM_EVENT_FLAG_DENY )
 {
-struct monitor_write_data *w = >arch.vm_event->write_data;
+struct monitor_write_data *w;
 
-ASSERT(w);
+ASSERT(v->arch.vm_event);
 
 /* deny flag requires the vCPU to be paused */
 if ( !atomic_read(>vm_event_pause_count) )
 return;
 
+w = >arch.vm_event->write_data;
+
 switch ( rsp->reason )
 {
 case VM_EVENT_REASON_MOV_TO_MSR:
-- 
2.5.0


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

Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Julien Grall]

(Add x86 maintainers)

On 06/07/16 16:23, Tamas K Lengyel wrote:

On Wed, Jul 6, 2016 at 7:43 AM, Julien Grall  wrote:

Hello Sergej,


On 06/07/16 10:14, Sergej Proskurin wrote:


On 07/05/2016 12:19 PM, Julien Grall wrote:


Hello Sergej,

On 04/07/16 12:45, Sergej Proskurin wrote:


+static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+struct xen_hvm_altp2m_op a;
+struct domain *d = NULL;
+int rc = 0;
+
+if ( !hvm_altp2m_supported() )
+return -EOPNOTSUPP;
+
+if ( copy_from_guest(, arg, 1) )
+return -EFAULT;
+
+if ( a.pad1 || a.pad2 ||
+ (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
+ (a.cmd < HVMOP_altp2m_get_domain_state) ||
+ (a.cmd > HVMOP_altp2m_change_gfn) )
+return -EINVAL;
+
+d = (a.cmd != HVMOP_altp2m_vcpu_enable_notify) ?
+rcu_lock_domain_by_any_id(a.domain) :
rcu_lock_current_domain();
+
+if ( d == NULL )
+return -ESRCH;
+
+if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
+ (a.cmd != HVMOP_altp2m_set_domain_state) &&
+ !d->arch.altp2m_active )
+{
+rc = -EOPNOTSUPP;
+goto out;
+}
+
+if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
+goto out;



I think this is the best place to ask a couple of questions related to
who can access altp2m. Based on this call, a guest is allowed to
manage its own altp2m. Can you explain why we would want a guest to do
that?



On x86, altp2m might be used by the guest in the #VE (Virtualization
Exception). On ARM, there is indeed not necessary for a guest to access
altp2m. Could you provide me with information, how to best restrict
non-privileged guests (not only dom0) from accessing these HVMOPs? Can
thisbedone by means of xsm? Thank you.



This does not looks safe for both x86 and ARM. From my understanding a
malware would be able to modify an altp2m, switching between 2 view... which
would lead to remove the entire purpose of altp2m.


Well, the whole purpose of the VMFUNC instruction right now is to be
able to switch the hypervisor's tables (like altp2m) from within the
guest. So yes, if you have a malicious guest then it could control
it's own altp2m views. I would assume there are other safe-guards
in-place for systems that use this to ensure kernel-integrity and thus
prevent arbitrary use of these functions. AFAIK there are only
experimental systems based on this so whether it's less or more secure
is debatable and likely depend on your implementation.


Taken aside the VMFUNC, it looks like insecure to expose a HVMOP to the 
guest which could modify the memaccess attribute of a region.


I thought the whole purpose of VM introspection is to avoid trusting the 
guest (kernel + userspace). The first thing a malware will do is trying 
to do is getting access to the kernel. Once it gets this access, it 
could remove all the memory access restriction to avoid trapping.



As for ARM - as there is no hardware features like this available -
our goal is to use altp2m in purely external usecases so exposing
these ops to the guest is not required. For the first prototype it
made sense to mirror the x86 side to reduce the possibility of
introducing some bug.


No, this is not the right approach. We should not introduce potential 
security issue just because x86 side does it and it "reduces the 
possibility of introducing some bug".


You will have to give me another reason to accept a such patch.

Regards,

--
Julien Grall

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

[Xen-devel] [PATCH v3 5/8] x86/vm_event_resume: surround VM_EVENT_REASON_MOV_TO_MSR w/ CONFIG_X86

[Corneliu ZUZU]

VM_EVENT_REASON_MOV_TO_MSR is X86-specific, surround w/ #ifdef accordingly.

Signed-off-by: Corneliu ZUZU 
Acked-by: Razvan Cojocaru 
---
Changed since v2: 
---
 xen/common/vm_event.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 47ae96c..40a7e56 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -394,7 +394,9 @@ void vm_event_resume(struct domain *d, struct 
vm_event_domain *ved)
  */
 switch ( rsp.reason )
 {
+#ifdef CONFIG_X86
 case VM_EVENT_REASON_MOV_TO_MSR:
+#endif
 case VM_EVENT_REASON_WRITE_CTRLREG:
 vm_event_register_write_resume(v, );
 break;
-- 
2.5.0


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

[Xen-devel] [PATCH v3 4/8] x86/vm_event/monitor/cr: check for vm-event subscriber on domctl

[Corneliu ZUZU]

Enforce presence of a monitor vm-event subscriber when the toolstack user calls
xc_monitor_write_ctrlreg (XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG domctl).
Without this change, ASSERT(v->arch.vm_event) @ hvm_set_cr0 and such would fail
if the toolstack user calls xc_monitor_write_ctrlreg(...) w/ enable = true,
without first calling xc_monitor_enable().

Also adjust returned error code for similar check from -EINVAL to more
descriptive -ENOSYS (XEN_DOMCTL_MONITOR_OP_EMULATE_EACH_REP).

Signed-off-by: Corneliu ZUZU 
---
 xen/arch/x86/monitor.c| 7 +++
 xen/include/asm-x86/monitor.h | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/monitor.c b/xen/arch/x86/monitor.c
index 42f31bf..4d4db33 100644
--- a/xen/arch/x86/monitor.c
+++ b/xen/arch/x86/monitor.c
@@ -188,6 +188,13 @@ int arch_monitor_domctl_event(struct domain *d,
 unsigned int ctrlreg_bitmask;
 bool_t old_status;
 
+/*
+ * Enabling cr-write vm-events without a vm_event subscriber is
+ * meaningless.
+ */
+if ( !vm_event_domain_initialised(d) )
+return -ENOSYS;
+
 /* sanity check: avoid left-shift undefined behavior */
 if ( unlikely(mop->u.mov_to_cr.index > 31) )
 return -EINVAL;
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index 9238ec8..2213124 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -52,7 +52,7 @@ int arch_monitor_domctl_op(struct domain *d, struct 
xen_domctl_monitor_op *mop)
 if ( vm_event_domain_initialised(d) )
 d->arch.mem_access_emulate_each_rep = !!mop->event;
 else
-rc = -EINVAL;
+rc = -ENOSYS;
 
 domain_unpause(d);
 break;
-- 
2.5.0


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

[Xen-devel] [PATCH v3 3/8] x86/vm-event/monitor: don't compromise monitor_write_data on domain cleanup

[Corneliu ZUZU]

The arch_vm_event structure is dynamically allocated and freed @
vm_event_cleanup_domain. This cleanup is triggered e.g. when the toolstack user
disables domain monitoring (xc_monitor_disable), which in turn effectively
discards any information that was in arch_vm_event.write_data.

But this can yield unexpected behavior since if a CR-write was awaiting to be
committed on the scheduling tail (hvm_do_resume->arch_monitor_write_data)
before xc_monitor_disable is called, then the domain CR write is wrongfully
ignored, which of course, in these cases, can easily render a domain crash.

To fix the issue, this patch:
- makes arch_vm_event.emul_read_data dynamically allocated
- in vm_event_cleanup_domain, if there are still uncommitted writes in
  arch_vm_event.write_data:
- only frees emul_read_data
- defers xfree of the entire arch_vm_event until vcpu/domain destroyal
- otherwise arch_vm_event is freed in vm_event_cleanup_domain, as before

For clarity, also introduce inline functions that check initialisation of the
vm_event subsystem for a vcpu/domain (vm_event_{vcpu,domain}_initialised), since
that is now true only when both arch_vm_event and arch_vm_event.emul_read_data
are non-NULL.

Signed-off-by: Corneliu ZUZU 
---
Changed since v2:
  * introduce vm_event_{vcpu,domain}_initialised inline functions for clarity
  * xfree arch_vm_event in vm_event_cleanup_domain as before if there are no
uncommitted writes in arch_vm_event.write_data
---
 xen/arch/x86/domain.c  |  9 +++--
 xen/arch/x86/hvm/emulate.c |  6 +++---
 xen/arch/x86/hvm/hvm.c |  2 ++
 xen/arch/x86/mm/p2m.c  |  2 +-
 xen/arch/x86/vm_event.c| 35 +--
 xen/common/vm_event.c  | 12 
 xen/include/asm-x86/domain.h   | 17 +++--
 xen/include/asm-x86/monitor.h  |  3 ++-
 xen/include/asm-x86/vm_event.h | 13 -
 9 files changed, 79 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index bb59247..0313208 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -56,6 +56,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -492,8 +493,12 @@ int vcpu_initialise(struct vcpu *v)
 
 void vcpu_destroy(struct vcpu *v)
 {
-xfree(v->arch.vm_event);
-v->arch.vm_event = NULL;
+if ( unlikely(v->arch.vm_event) )
+{
+xfree(v->arch.vm_event->emul_read_data);
+xfree(v->arch.vm_event);
+v->arch.vm_event = NULL;
+}
 
 if ( is_pv_32bit_vcpu(v) )
 {
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 855af4d..59e2344 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -73,12 +73,12 @@ static int set_context_data(void *buffer, unsigned int size)
 {
 struct vcpu *curr = current;
 
-if ( curr->arch.vm_event )
+if ( vm_event_vcpu_initialised(curr) )
 {
 unsigned int safe_size =
-min(size, curr->arch.vm_event->emul_read_data.size);
+min(size, curr->arch.vm_event->emul_read_data->size);
 
-memcpy(buffer, curr->arch.vm_event->emul_read_data.data, safe_size);
+memcpy(buffer, curr->arch.vm_event->emul_read_data->data, safe_size);
 memset(buffer + safe_size, 0, size - safe_size);
 return X86EMUL_OKAY;
 }
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index e3829d2..ac6d9eb 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -479,6 +479,8 @@ void hvm_do_resume(struct vcpu *v)
 {
 enum emul_kind kind = EMUL_KIND_NORMAL;
 
+ASSERT(v->arch.vm_event->emul_read_data);
+
 if ( v->arch.vm_event->emulate_flags &
  VM_EVENT_FLAG_SET_EMUL_READ_DATA )
 kind = EMUL_KIND_SET_CONTEXT;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 16733a4..6616626 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1642,7 +1642,7 @@ void p2m_mem_access_emulate_check(struct vcpu *v,
 v->arch.vm_event->emulate_flags = violation ? rsp->flags : 0;
 
 if ( (rsp->flags & VM_EVENT_FLAG_SET_EMUL_READ_DATA) )
-v->arch.vm_event->emul_read_data = rsp->data.emul_read_data;
+*v->arch.vm_event->emul_read_data = rsp->data.emul_read_data;
 }
 }
 
diff --git a/xen/arch/x86/vm_event.c b/xen/arch/x86/vm_event.c
index 80f84d6..ff2ba92 100644
--- a/xen/arch/x86/vm_event.c
+++ b/xen/arch/x86/vm_event.c
@@ -30,12 +30,18 @@ int vm_event_init_domain(struct domain *d)
 
 for_each_vcpu ( d, v )
 {
-if ( v->arch.vm_event )
+if ( likely(!v->arch.vm_event) )
+{
+v->arch.vm_event = xzalloc(struct arch_vm_event);
+if ( !v->arch.vm_event )
+return -ENOMEM;
+}
+else if ( unlikely(v->arch.vm_event->emul_read_data) )
 continue;
 
-

Re: [Xen-devel] [PATCH 02/19] xen: sched: make the 'tickled' perf counter clearer

[George Dunlap]

On Sat, Jun 18, 2016 at 12:11 AM, Dario Faggioli
 wrote:
> In fact, what we have right now, i.e., tickle_idlers_none
> and tickle_idlers_some, is not good enough for describing
> what really happens in the various tickling functions of
> the various scheduler.
>
> Switch to a more descriptive set of counters, such as:
>  - tickled_no_cpu: for when we don't tickle anyone
>  - tickled_idle_cpu: for when we tickle one or more
>  idler
>  - tickled_busy_cpu: for when we tickle one or more
>  non-idler
>
> While there, fix style of an "out:" label in sched_rt.c.
>
> Signed-off-by: Dario Faggioli 

Looks good:

Acked-by: George Dunlap 

> ---
> Cc: George Dunlap 
> Cc: Meng Xu 
> Cc: Anshul Makkar 
> Cc: David Vrabel 
> ---
>  xen/common/sched_credit.c|   10 +++---
>  xen/common/sched_credit2.c   |   12 +---
>  xen/common/sched_rt.c|8 +---
>  xen/include/xen/perfc_defn.h |5 +++--
>  4 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index a6645a2..a54bb2d 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -385,7 +385,9 @@ static inline void __runq_tickle(struct csched_vcpu *new)
>   || (idlers_empty && new->pri > cur->pri) )
>  {
>  if ( cur->pri != CSCHED_PRI_IDLE )
> -SCHED_STAT_CRANK(tickle_idlers_none);
> +SCHED_STAT_CRANK(tickled_busy_cpu);
> +else
> +SCHED_STAT_CRANK(tickled_idle_cpu);
>  __cpumask_set_cpu(cpu, );
>  }
>  else if ( !idlers_empty )
> @@ -444,13 +446,13 @@ static inline void __runq_tickle(struct csched_vcpu 
> *new)
>  set_bit(_VPF_migrating, >vcpu->pause_flags);
>  }
>  /* Tickle cpu anyway, to let new preempt cur. */
> -SCHED_STAT_CRANK(tickle_idlers_none);
> +SCHED_STAT_CRANK(tickled_busy_cpu);
>  __cpumask_set_cpu(cpu, );
>  }
>  else if ( !new_idlers_empty )
>  {
>  /* Which of the idlers suitable for new shall we wake up? */
> -SCHED_STAT_CRANK(tickle_idlers_some);
> +SCHED_STAT_CRANK(tickled_idle_cpu);
>  if ( opt_tickle_one_idle )
>  {
>  this_cpu(last_tickle_cpu) =
> @@ -479,6 +481,8 @@ static inline void __runq_tickle(struct csched_vcpu *new)
>  /* Send scheduler interrupts to designated CPUs */
>  cpumask_raise_softirq(, SCHEDULE_SOFTIRQ);
>  }
> +else
> +SCHED_STAT_CRANK(tickled_no_cpu);
>  }
>
>  static void
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index cf8455c..0246453 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -589,6 +589,7 @@ runq_tickle(const struct scheduler *ops, unsigned int 
> cpu, struct csched2_vcpu *
>  i = cpumask_cycle(cpu, );
>  if ( i < nr_cpu_ids )
>  {
> +SCHED_STAT_CRANK(tickled_idle_cpu);
>  ipid = i;
>  goto tickle;
>  }
> @@ -637,11 +638,12 @@ runq_tickle(const struct scheduler *ops, unsigned int 
> cpu, struct csched2_vcpu *
>   * than the migrate resistance */
>  if ( ipid == -1 || lowest + CSCHED2_MIGRATE_RESIST > new->credit )
>  {
> -SCHED_STAT_CRANK(tickle_idlers_none);
> -goto no_tickle;
> +SCHED_STAT_CRANK(tickled_no_cpu);
> +return;
>  }
>
> -tickle:
> +SCHED_STAT_CRANK(tickled_busy_cpu);
> + tickle:
>  BUG_ON(ipid == -1);
>
>  /* TRACE */ {
> @@ -654,11 +656,7 @@ tickle:
>(unsigned char *));
>  }
>  cpumask_set_cpu(ipid, >tickled);
> -SCHED_STAT_CRANK(tickle_idlers_some);
>  cpu_raise_softirq(ipid, SCHEDULE_SOFTIRQ);
> -
> -no_tickle:
> -return;
>  }
>
>  /*
> diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c
> index 5b077d7..dd1c4d3 100644
> --- a/xen/common/sched_rt.c
> +++ b/xen/common/sched_rt.c
> @@ -1140,6 +1140,7 @@ runq_tickle(const struct scheduler *ops, struct rt_vcpu 
> *new)
>  /* 1) if new's previous cpu is idle, kick it for cache benefit */
>  if ( is_idle_vcpu(curr_on_cpu(new->vcpu->processor)) )
>  {
> +SCHED_STAT_CRANK(tickled_idle_cpu);
>  cpu_to_tickle = new->vcpu->processor;
>  goto out;
>  }
> @@ -1151,6 +1152,7 @@ runq_tickle(const struct scheduler *ops, struct rt_vcpu 
> *new)
>  iter_vc = curr_on_cpu(cpu);
>  if ( is_idle_vcpu(iter_vc) )
>  {
> +SCHED_STAT_CRANK(tickled_idle_cpu);
>  cpu_to_tickle = cpu;
>  goto out;
>  }
> @@ -1164,14 +1166,15 @@ runq_tickle(const struct scheduler *ops, struct 
> rt_vcpu 

[Xen-devel] [PATCH v3 2/8] x86/vm-event/monitor: relocate code-motion more appropriately

[Corneliu ZUZU]

For readability:

* Add function arch_monitor_write_data (in x86/monitor.c) and separate handling
of monitor_write_data there (previously done directly in hvm_do_resume).

* Separate enabling/disabling of CPU_BASED_CR3_LOAD_EXITING for CR3 monitor
vm-events from CR0 node @ vmx_update_guest_cr(v, 0) into newly added function
vmx_vm_event_update_cr3_traps(d); also, for a better interface, create generic
functions write_ctrlreg_adjust_traps and write_ctrlreg_disable_traps (in
x86/monitor.c) which deal with setting/unsetting any traps for cr-write monitor
vm-events.

Signed-off-by: Corneliu ZUZU 
---
Changed since v2:
  * add write_ctrlreg_disable_traps, call from arch_monitor_cleanup_domain
---
 xen/arch/x86/hvm/hvm.c| 46 ++-
 xen/arch/x86/hvm/vmx/vmx.c| 58 +++---
 xen/arch/x86/monitor.c| 66 ++-
 xen/include/asm-x86/hvm/vmx/vmx.h |  1 +
 xen/include/asm-x86/monitor.h |  2 ++
 5 files changed, 131 insertions(+), 42 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c89ab6e..e3829d2 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -475,8 +475,6 @@ void hvm_do_resume(struct vcpu *v)
 
 if ( unlikely(v->arch.vm_event) )
 {
-struct monitor_write_data *w = >arch.vm_event->write_data;
-
 if ( v->arch.vm_event->emulate_flags )
 {
 enum emul_kind kind = EMUL_KIND_NORMAL;
@@ -494,29 +492,7 @@ void hvm_do_resume(struct vcpu *v)
 v->arch.vm_event->emulate_flags = 0;
 }
 
-if ( w->do_write.msr )
-{
-hvm_msr_write_intercept(w->msr, w->value, 0);
-w->do_write.msr = 0;
-}
-
-if ( w->do_write.cr0 )
-{
-hvm_set_cr0(w->cr0, 0);
-w->do_write.cr0 = 0;
-}
-
-if ( w->do_write.cr4 )
-{
-hvm_set_cr4(w->cr4, 0);
-w->do_write.cr4 = 0;
-}
-
-if ( w->do_write.cr3 )
-{
-hvm_set_cr3(w->cr3, 0);
-w->do_write.cr3 = 0;
-}
+arch_monitor_write_data(v);
 }
 
 /* Inject pending hw/sw trap */
@@ -2206,7 +2182,10 @@ int hvm_set_cr0(unsigned long value, bool_t may_defer)
 
 if ( hvm_monitor_crX(CR0, value, old_value) )
 {
-/* The actual write will occur in hvm_do_resume(), if permitted. */
+/*
+ * The actual write will occur in arch_monitor_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.cr0 = 1;
 v->arch.vm_event->write_data.cr0 = value;
 
@@ -2308,7 +2287,10 @@ int hvm_set_cr3(unsigned long value, bool_t may_defer)
 
 if ( hvm_monitor_crX(CR3, value, old) )
 {
-/* The actual write will occur in hvm_do_resume(), if permitted. */
+/*
+ * The actual write will occur in arch_monitor_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.cr3 = 1;
 v->arch.vm_event->write_data.cr3 = value;
 
@@ -2388,7 +2370,10 @@ int hvm_set_cr4(unsigned long value, bool_t may_defer)
 
 if ( hvm_monitor_crX(CR4, value, old_cr) )
 {
-/* The actual write will occur in hvm_do_resume(), if permitted. */
+/*
+ * The actual write will occur in arch_monitor_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.cr4 = 1;
 v->arch.vm_event->write_data.cr4 = value;
 
@@ -3767,7 +3752,10 @@ int hvm_msr_write_intercept(unsigned int msr, uint64_t 
msr_content,
 {
 ASSERT(v->arch.vm_event);
 
-/* The actual write will occur in hvm_do_resume() (if permitted). */
+/*
+ * The actual write will occur in arch_monitor_write_data(), if
+ * permitted.
+ */
 v->arch.vm_event->write_data.do_write.msr = 1;
 v->arch.vm_event->write_data.msr = msr;
 v->arch.vm_event->write_data.value = msr_content;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 8ab074f..0fa3fea 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1442,11 +1442,6 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned 
int cr)
 if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
 v->arch.hvm_vmx.exec_control |= cr3_ctls;
 
-/* Trap CR3 updates if CR3 memory events are enabled. */
-if ( v->domain->arch.monitor.write_ctrlreg_enabled &
- monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
-v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;
-
 if ( old_ctls != v->arch.hvm_vmx.exec_control )
 vmx_update_cpu_exec_control(v);
 }
@@ -3899,6 +3894,59 @@ void 

Re: [Xen-devel] [PATCH v1 20/20] libxl/acpi: Build ACPI tables for HVMlite guests

[Boris Ostrovsky]

On 07/06/2016 07:05 AM, Julien Grall wrote:
> (CC Stefano)
>
> Hi Boris,
>
> On 05/07/16 20:05, Boris Ostrovsky wrote:
>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h
>> index 34a853c..7c6536b 100644
>> --- a/tools/libxl/libxl_arch.h
>> +++ b/tools/libxl/libxl_arch.h
>> @@ -62,4 +62,7 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
>>   uint32_t domid,
>>   struct xc_dom_image *dom);
>>
>> +int libxl__dom_load_acpi(libxl__gc *gc,
>> + libxl_domain_build_info *info,
>> + struct xc_dom_image *dom);
>
> This file contains arch specific function with are called by the
> generic libxl code.
>
> I don't think this is the right file for an x86 specific function
> which has a generic name. IHMO, this should go in libxl_x86_acpi.h
> with "x86" in the name and with '_hidden' attribute.

Right. I used to call this routine from libxl_dom.c. I moved the call
site to libxl_x86.c (as you suggested earlier) but forgot to move the
declaration.

>
>>   #endif
>
> [...]
>
>> +static int populate_acpi_pages(struct xc_dom_image *dom,
>> +   xen_pfn_t *extents,
>> +   unsigned int num_pages,
>> +   struct acpi_ctxt *ctxt)
>> +{
>> +int rc;
>> +xc_interface *xch = dom->xch;
>> +uint32_t domid = dom->guest_domid;
>> +unsigned long idx, first_high_idx = (1ull << (32 -
>> ctxt->page_shift));
>> +
>> +for (; num_pages; num_pages--, extents++) {
>> +
>> +if (xc_domain_populate_physmap(xch, domid, 1, 0, 0, extents)
>> == 1)
>
> It looks like this is working because libxl is setting the maximum
> size of the domain with some slack (1MB). You might get in trouble if
> the slack is reduced or used by someone or the ACPI blob is increasing.


I saw your conversation about slack with Stefano and I am not sure I
understood what it was about. If this was about padding guest's memory
to be able to put there some additional data (such ACPI tables) then
this is not my intention here: if I can't populate (because it is
already populated, I guess) then I try to move memory around with code
below. That's what mem_hole_populate_ram() does as well.


>
>> +continue;
>> +
>> +if (dom->highmem_end) {
>> +idx = --dom->highmem_end;
>> +if ( idx == first_high_idx )
>> +dom->highmem_end = 0;
>> +} else
>> +idx = --dom->lowmem_end;
>> +
>> +rc = xc_domain_add_to_physmap(xch, domid,
>> +  XENMAPSPACE_gmfn,
>> +  idx, *extents);
>> +if (rc)
>> +return rc;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +int libxl__dom_load_acpi(libxl__gc *gc,
>> + libxl_domain_build_info *info,
>> + struct xc_dom_image *dom)
>> +{
>> +struct acpi_config config = {0};
>> +struct acpi_ctxt ctxt;
>> +uint32_t domid = dom->guest_domid;
>> +xc_interface *xch = dom->xch;
>> +int rc, i, acpi_pages_num;
>> +xen_pfn_t extent, *extents;
>> +void *acpi_pages, *guest_acpi_pages = NULL;
>> +unsigned long page_mask;
>> +
>> +if ((info->type != LIBXL_DOMAIN_TYPE_HVM) ||
>> +(info->device_model_version !=
>> LIBXL_DEVICE_MODEL_VERSION_NONE))
>> +return 0;
>> +
>> +ctxt.page_size = XC_DOM_PAGE_SIZE(dom);
>> +ctxt.page_shift = XC_DOM_PAGE_SHIFT(dom);
>> +page_mask = (1UL << ctxt.page_shift) - 1;
>> +
>> +ctxt.mem_ops.alloc = mem_alloc;
>> +ctxt.mem_ops.v2p = virt_to_phys;
>> +
>> +rc = init_acpi_config(gc, dom, info, );
>> +if (rc) {
>> +LOG(ERROR, "%s: init_acpi_config failed (rc=%d)",
>> __FUNCTION__, rc);
>> +return rc;
>> +}
>> +
>> +/* Map page that will hold RSDP */
>> +extent = RSDP_ADDRESS >> ctxt.page_shift;
>> +rc = populate_acpi_pages(dom, , 1, );
>> +if (rc) {
>> +LOG(ERROR, "%s: populate_acpi_pages for rsdp failed with %d",
>> +__FUNCTION__, rc);
>> +goto out;
>> +}
>> +config.rsdp = (unsigned long)xc_map_foreign_range(xch, domid,
>> +  ctxt.page_size,
>> +  PROT_READ |
>> PROT_WRITE,
>> +  RSDP_ADDRESS
>> >> ctxt.page_shift);
>> +if (!config.rsdp) {
>> +LOG(ERROR, "%s: Can't map acpi_physical", __FUNCTION__);
>> +rc = -1;
>> +goto out;
>> +}
>> +
>> +/* Map acpi_info page */
>> +extent = ACPI_INFO_PHYSICAL_ADDRESS >> ctxt.page_shift;
>> +rc = populate_acpi_pages(dom, , 1, );
>> +if (rc) {
>> +LOG(ERROR, "%s: populate_acpi_pages for acpi_info failed
>> with %d",
>> +__FUNCTION__, rc);
>> +goto out;
>> +}
>> +
>> +config.ainfop = (unsigned 

[Xen-devel] [PATCH v3 1/8] x86/vmx_update_guest_cr: minor optimization

[Corneliu ZUZU]

Minor optimization @ vmx_update_guest_cr: checks if v->arch.hvm_vmx.exec_control
was modified before actually calling vmx_update_cpu_exec_control(v).

Signed-off-by: Corneliu ZUZU 
---
Changed since v2: 
---
 xen/arch/x86/hvm/vmx/vmx.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index df19579..8ab074f 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1434,8 +1434,10 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned 
int cr)
 if ( paging_mode_hap(v->domain) )
 {
 /* Manage GUEST_CR3 when CR0.PE=0. */
+uint32_t old_ctls = v->arch.hvm_vmx.exec_control;
 uint32_t cr3_ctls = (CPU_BASED_CR3_LOAD_EXITING |
  CPU_BASED_CR3_STORE_EXITING);
+
 v->arch.hvm_vmx.exec_control &= ~cr3_ctls;
 if ( !hvm_paging_enabled(v) && !vmx_unrestricted_guest(v) )
 v->arch.hvm_vmx.exec_control |= cr3_ctls;
@@ -1445,7 +1447,8 @@ static void vmx_update_guest_cr(struct vcpu *v, unsigned 
int cr)
  monitor_ctrlreg_bitmask(VM_EVENT_X86_CR3) )
 v->arch.hvm_vmx.exec_control |= CPU_BASED_CR3_LOAD_EXITING;
 
-vmx_update_cpu_exec_control(v);
+if ( old_ctls != v->arch.hvm_vmx.exec_control )
+vmx_update_cpu_exec_control(v);
 }
 
 if ( !nestedhvm_vcpu_in_guestmode(v) )
-- 
2.5.0


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

Re: [Xen-devel] [PATCH v3 1/2] Interface for grant copy operation in libs.

[Roger Pau Monné]

On Wed, Jun 22, 2016 at 05:49:59PM +0100, Wei Liu wrote:
> On Wed, Jun 22, 2016 at 03:52:43PM +0100, Wei Liu wrote:
> > On Wed, Jun 22, 2016 at 02:52:47PM +0100, David Vrabel wrote:
> > > On 22/06/16 14:29, Wei Liu wrote:
> > > > On Wed, Jun 22, 2016 at 01:37:50PM +0100, David Vrabel wrote:
> > > >> On 22/06/16 12:21, Wei Liu wrote:
> > > >>> On Wed, Jun 22, 2016 at 10:37:24AM +0100, David Vrabel wrote:
> > >  On 22/06/16 09:38, Paulina Szubarczyk wrote:
> > > > In a linux part an ioctl(gntdev, IOCTL_GNTDEV_GRANT_COPY, ..)
> > > > system call is invoked. In mini-os the operation is yet not
> > > > implemented. For other OSs there is a dummy implementation.
> > >  [...]
> > > > --- a/tools/libs/gnttab/linux.c
> > > > +++ b/tools/libs/gnttab/linux.c
> > > > @@ -235,6 +235,51 @@ int osdep_gnttab_unmap(xengnttab_handle *xgt,
> > > >  return 0;
> > > >  }
> > > >  
> > > > +int osdep_gnttab_grant_copy(xengnttab_handle *xgt,
> > > > +uint32_t count,
> > > > +xengnttab_grant_copy_segment_t *segs)
> > > > +{
> > > > +int i, rc;
> > > > +int fd = xgt->fd;
> > > > +struct ioctl_gntdev_grant_copy copy;
> > > > +
> > > > +copy.segments = calloc(count, sizeof(struct 
> > > > ioctl_gntdev_grant_copy_segment));
> > > > +copy.count = count;
> > > > +for (i = 0; i < count; i++)
> > > > +{
> > > > +copy.segments[i].flags = segs[i].flags;
> > > > +copy.segments[i].len = segs[i].len;
> > > > +if (segs[i].flags == GNTCOPY_dest_gref) 
> > > > +{
> > > > +copy.segments[i].dest.foreign.ref = 
> > > > segs[i].dest.foreign.ref;
> > > > +copy.segments[i].dest.foreign.domid = 
> > > > segs[i].dest.foreign.domid;
> > > > +copy.segments[i].dest.foreign.offset = 
> > > > segs[i].dest.foreign.offset;
> > > > +copy.segments[i].source.virt = segs[i].source.virt;
> > > > +} 
> > > > +else 
> > > > +{
> > > > +copy.segments[i].source.foreign.ref = 
> > > > segs[i].source.foreign.ref;
> > > > +copy.segments[i].source.foreign.domid = 
> > > > segs[i].source.foreign.domid;
> > > > +copy.segments[i].source.foreign.offset = 
> > > > segs[i].source.foreign.offset;
> > > > +copy.segments[i].dest.virt = segs[i].dest.virt;
> > > > +}
> > > > +}
> > > > +
> > > > +rc = ioctl(fd, IOCTL_GNTDEV_GRANT_COPY, );
> > > > +if (rc) 
> > > > +{
> > > > +GTERROR(xgt->logger, "ioctl GRANT COPY failed %d ", errno);
> > > > +}
> > > > +else 
> > > > +{
> > > > +for (i = 0; i < count; i++)
> > > > +segs[i].status = copy.segments[i].status;
> > > > +}
> > > > +
> > > > +free(copy.segments);
> > > > +return rc;
> > > > +}
> > > 
> > >  I know Wei asked for this but you've replaced what should be a single
> > >  pointer assignment with a memory allocation and two loops over all 
> > >  the
> > >  segments.
> > > 
> > >  This is a hot path and the two structures (the libxengnttab one and 
> > >  the
> > >  Linux kernel one) are both part of their respective ABIs and won't
> > >  change so Wei's concern that they might change in the future is 
> > >  unfounded.
> > > 
> > > >>>
> > > >>> The fundamental question is: will the ABI between the library and the
> > > >>> kernel ever go mismatch?
> > > >>>
> > > >>> My answer is "maybe".  My rationale is that everything goes across
> > > >>> boundary of components need to be considered with caution. And I tend 
> > > >>> to
> > > >>> assume the worst things will happen.
> > > >>>
> > > >>> To guarantee that they will never go mismatch is to have
> > > >>>
> > > >>>typedef ioctl_gntdev_grant_copy_segment 
> > > >>> xengnttab_grant_copy_segment_t;
> > > >>>
> > > >>> But that's not how the code is written.
> > > >>>
> > > >>> I would like to hear a third opinion. Is my concern unfounded? Am I 
> > > >>> too
> > > >>> cautious? Is there any compelling argument that I missed?
> > > >>>
> > > >>> Somewhat related, can we have some numbers please? It could well be 
> > > >>> the
> > > >>> cost of the two loops is much cheaper than whatever is going on inside
> > > >>> the kernel / hypervisor. And it could turn out that the numbers render
> > > >>> this issue moot.
> > > >>
> > > >> I did some (very) adhoc measurements and with the worst case of single
> > > >> short segments for each ioctl, the optimized version of
> > > >> osdep_gnttab_grant_copy() looks to be ~5% faster.
> > > >>
> > > >> This is enough of a difference that we should use the optimized 
> > > >> version.
> > > >>
> > > >> The unoptimized version also adds an 

[Xen-devel] [PATCH v3 0/8] x86/vm-event: Adjustments & fixes

[Corneliu ZUZU]

This patch-series makes some adjustments and fixes to the X86 vm-events code.

Summary:
Corneliu ZUZU (8):
  1. x86/vmx_update_guest_cr: minor optimization
  2. x86/vm-event/monitor: relocate code-motion more appropriately
  3. x86/vm-event/monitor: don't compromise monitor_write_data on domain
 cleanup
  4. x86/vm_event/monitor/cr: check for vm-event subscriber on domctl
  5. x86/vm_event_resume: surround VM_EVENT_REASON_MOV_TO_MSR w/ CONFIG_X86
Acked-by: Razvan Cojocaru 
  6. x86/vm-event: minor ASSERT fix, add 'unlikely'
  7. minor fixes (formatting, comments, unused includes etc.)
Acked-by: Razvan Cojocaru 
Acked-by: Julien Grall 
  8. minor #include change
Acked-by: Tamas K Lengyel 
Acked-by: Razvan Cojocaru 

Significant changes since v2:
  0/8: patch 4 added
  2/8: call vmx_vm_event_update_cr3_traps from arch_monitor_cleanup_domain also
  3/8: * introduce vm_event_{vcpu,domain}_initialised inline functions for 
clarity
   * xfree arch_vm_event in vm_event_cleanup_domain as before if there are 
no
 uncommitted writes in arch_vm_event.write_data

 xen/arch/arm/domain.c |  1 -
 xen/arch/arm/traps.c  |  1 -
 xen/arch/x86/domain.c |  9 +++--
 xen/arch/x86/hvm/emulate.c|  6 ++--
 xen/arch/x86/hvm/hvm.c| 53 +++-
 xen/arch/x86/hvm/monitor.c|  1 +
 xen/arch/x86/hvm/vmx/vmx.c| 64 ++
 xen/arch/x86/mm/p2m.c |  2 +-
 xen/arch/x86/monitor.c| 72 ++-
 xen/arch/x86/vm_event.c   | 44 ++--
 xen/common/monitor.c  |  1 -
 xen/common/vm_event.c | 20 +--
 xen/include/asm-arm/vm_event.h|  9 ++---
 xen/include/asm-x86/domain.h  | 17 +
 xen/include/asm-x86/hvm/monitor.h |  2 --
 xen/include/asm-x86/hvm/vmx/vmx.h |  1 +
 xen/include/asm-x86/monitor.h | 10 +++---
 xen/include/asm-x86/vm_event.h| 14 ++--
 xen/include/public/vm_event.h | 38 ++---
 xen/include/xen/vm_event.h|  1 -
 20 files changed, 255 insertions(+), 111 deletions(-)

-- 
2.5.0


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

Re: [Xen-devel] [PATCH v1 18/20] libxl/acpi: Add ACPI e820 entry

[Boris Ostrovsky]

On 07/06/2016 06:00 AM, Julien Grall wrote:
> (CC Stefano)
>
> Hi Boris,
>
> On 05/07/16 20:05, Boris Ostrovsky wrote:
>> Add entry for ACPI tables created for PVHv2 guests to e820 map.
>>
>> Signed-off-by: Boris Ostrovsky 
>> ---
>>
>> New patch
>>
>>   tools/libxc/include/xc_dom.h |4 
>>   tools/libxl/libxl_dom.c  |8 
>>   tools/libxl/libxl_x86.c  |   11 +++
>>   3 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
>> index 6cb10c4..ec2da14 100644
>> --- a/tools/libxc/include/xc_dom.h
>> +++ b/tools/libxc/include/xc_dom.h
>> @@ -102,6 +102,10 @@ struct xc_dom_image {
>>   xen_vaddr_t virt_alloc_end;
>>   xen_vaddr_t bsd_symtab_start;
>>
>> +/* ACPI tables (PVHv2 only) */
>> +xen_pfn_t acpi_pfn;
>> +xen_pfn_t acpi_pages;
>> +
>
> It would be good if we can share the fields with ARM (see [1]).


This is slightly different: acpi_pfn is address in guest physical space
(and is used for building guest's e820 map) and acpitable_blob is a
pointer to virtual address in the toolstack (or is it not?).

acpi_pages and acpitable_size could be used interchangeably though so
that could be shared.

-boris

>
> Regards,
>
> [1]
> https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg00301.html
>



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

Re: [Xen-devel] [PATCH 01/19] xen: sched: leave CPUs doing tasklet work alone.

[George Dunlap]

On Sat, Jun 18, 2016 at 12:11 AM, Dario Faggioli
 wrote:
> In both Credit1 and Credit2, stop considering a pCPU idle,
> if the reason why the idle vCPU is being selected, is to
> do tasklet work.
>
> Not doing so means that the tickling and load balancing
> logic, seeing the pCPU as idle, considers it a candidate
> for picking up vCPUs. But the pCPU won't actually pick
> up or schedule any vCPU, which would then remain in the
> runqueue, which is bas, especially if there were other,

*bad

> truly idle pCPUs, that could execute it.
>
> The only drawback is that we can't assume that a pCPU is
> in always marked as idle when being removed from an
> instance of the Credit2 scheduler (csched2_deinit_pdata).
> In fact, if we are in stop-machine (i.e., during suspend
> or shutdown), the pCPUs are running the stopmachine_tasklet
> and hence are actually marked as busy. On the other hand,
> when removing a pCPU from a Credit2 pool, it will indeed
> be idle. The only thing we can do, therefore, is to
> remove the BUG_ON() check.
>
> Signed-off-by: Dario Faggioli 
> ---
> Cc: George Dunlap 
> Cc: Anshul Makkar 
> Cc: David Vrabel 
> ---
>  xen/common/sched_credit.c  |   12 ++--
>  xen/common/sched_credit2.c |   14 ++
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index a38a63d..a6645a2 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -1819,24 +1819,24 @@ csched_schedule(
>  else
>  snext = csched_load_balance(prv, cpu, snext, );
>
> + out:

Sorry if I'm being a bit dense, but why is this moving up here?  As
far as I can tell the only time the 'out' label will be used, the
'idler' status of the cpu cannot change.

At very least moving it up here introduces a bug, since now...

>  /*
>   * Update idlers mask if necessary. When we're idling, other CPUs
>   * will tickle us when they get extra work.
>   */
> -if ( snext->pri == CSCHED_PRI_IDLE )
> +if ( tasklet_work_scheduled || snext->pri != CSCHED_PRI_IDLE )
>  {
> -if ( !cpumask_test_cpu(cpu, prv->idlers) )
> -cpumask_set_cpu(cpu, prv->idlers);
> +if ( cpumask_test_cpu(cpu, prv->idlers) )
> +cpumask_clear_cpu(cpu, prv->idlers);
>  }
> -else if ( cpumask_test_cpu(cpu, prv->idlers) )
> +else if ( !cpumask_test_cpu(cpu, prv->idlers) )
>  {
> -cpumask_clear_cpu(cpu, prv->idlers);
> +cpumask_set_cpu(cpu, prv->idlers);
>  }
>
>  if ( !is_idle_vcpu(snext->vcpu) )
>  snext->start_time += now;

...this will happen twice in the case (once in the if() clause, once
here).  (Although arguably the one in the if() clause should go away
and the out: label should be moved above this line anyway).

Other than that, looks good.

 -George

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

Re: [Xen-devel] [PATCH v5 10/17] xen/arm: map_regions_rw_cache: Map the region with p2m->default_access

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 5:10 AM, Julien Grall  wrote:
> (CC Tamas)
>
> On 06/07/16 11:43, Stefano Stabellini wrote:
>>
>> On Tue, 28 Jun 2016, Julien Grall wrote:
>>>
>>> The parameter 'access' is used by memaccess to restrict temporarily the
>>> permission. This parameter should not be used for other purpose (such
>>> as restricting permanently the permission).
>>>
>>> The type p2m_mmio_direct will map the region Read-Write and
>>> non-executable. Note that this is already the current behavior with the
>>> combination of the type and the access. So there is no functional
>>> change.
>>>
>>> Signed-off-by: Julien Grall 
>>
>>
>> I could be mistaken, but isn't default_access actually p2m_access_rwx?
>
>
> By default, the access is p2m_access_rwx. However this can be changed by
> memaccess and the new default value is stored in arch->p2m.default_access. I
> have CCed Tamas to confirm that.

Yes, that's correct.

Cheers,
Tamas

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

[Xen-devel] default XSM policy for PCI passthrough for unlabeled resources.

[anshul makkar]

Hi,

Default XSM policy doesn't allow the use of unlabeled PCI resources that 
have been passed through to target domain.


xen.te
# Resources must be declared using . resource_type 


neverallow * ~resource_type:resource use;

It allows the resource to be added and removed by the source domain to 
target domain, but its use by target domain is blocked.


The resource can be used only if it has been labeled using 
flask-label-pci command which needs to be rerun after every boot and 
after every policy reload.


The above approach reduces the flexibility and necessitates admin 
intervention to give passthrough rights after host has booted. Also, in 
general if I want to allow all domUs to have PCI passthough access to 
all PCI device, I have no other way apart from disabling the 
"neverallow" rule.


Given that what we ship is just a sample default policy for reference 
which is expected to be permissive in most of the scenarios so that it 
doesn't affect the basic functionalities, is this "neverallow" rule needed ?


Thanks
Anshul Makkar









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

[Xen-devel] [OSSTEST PATCH 5/8] ts-hosts-allocate-Executive: Replace some odd commas with semicolons

[Ian Jackson]

There does not seem to be any reason for this.  I think it must be a
leftover from a previous use of { A => X, B => Y, ... } syntax.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 ts-hosts-allocate-Executive | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 13a0652..69f1cf6 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -709,8 +709,8 @@ sub compute_booking_list () {
$book->{Job}= "$flight.$job";
$book->{Reso}= "$sel->{restype} $sel->{resname}";
$book->{Xinfo}= $hid->{Ident};
-   $book->{Start}= $best->{Start},
-   $book->{End}= $best->{Start} + $best->{Duration},
+   $book->{Start}= $best->{Start};
+   $book->{End}= $best->{Start} + $best->{Duration};
push @bookings, $book;
 }
 return { Bookings => \@bookings };
-- 
2.1.4


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

[Xen-devel] [OSSTEST PATCH 2/8] ms-flights-summary: Remove spurious \ in keys \%{ something }

[Ian Jackson]

With some versions of Perl this generates a warning which causes
ms-flights-summary to fail.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 ms-flights-summary | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ms-flights-summary b/ms-flights-summary
index ea72088..c703d81 100755
--- a/ms-flights-summary
+++ b/ms-flights-summary
@@ -359,7 +359,7 @@ printf("%d flight(s) consisting of %s job(s)%s%s anonymous/rogue
 
 my %summarycounts;
 foreach my $fi (values %flights) {
-$summarycounts{$_} += $fi->{Stats}{$_} foreach (keys \%{ $fi->{Stats} });
+$summarycounts{$_} += $fi->{Stats}{$_} foreach (keys %{ $fi->{Stats} });
 }
 my @summarycounts = sort_stats \%summarycounts;
 
-- 
2.1.4


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

[Xen-devel] [OSSTEST PATCH 1/8] Tcl: Use tclsh8.5

[Ian Jackson]

I have checked that tclsh8.5 and TclX work on osstest.test-lab (and
also osstest.xs.citrite.net).  TclX seems to be provided by tcl8.4 but
work with tcl8.5 (at least on wheezy and jessie).

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 README| 2 +-
 ms-ownerdaemon| 2 +-
 ms-queuedaemon| 2 +-
 ms-reportuptime   | 2 +-
 sg-execute-flight | 2 +-
 sg-run-job| 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/README b/README
index 0a346dc..3aa35d2 100644
--- a/README
+++ b/README
@@ -246,7 +246,7 @@ To run osstest in standalone mode:
 
  - You need to install
  sqlite3
- tcl8.4 tclx8.4 libsqlite3-tcl
+ tcl8.5 tclx8.4 libsqlite3-tcl
  libdbi-perl libdbd-sqlite3-perl
  pax rsync
  curl
diff --git a/ms-ownerdaemon b/ms-ownerdaemon
index 33ee238..5b52339 100755
--- a/ms-ownerdaemon
+++ b/ms-ownerdaemon
@@ -1,4 +1,4 @@
-#!/usr/bin/tclsh8.4
+#!/usr/bin/tclsh8.5
 # -*- Tcl -*- 
 # usage: ./ms-ownerdaemon  ... | logger
 
diff --git a/ms-queuedaemon b/ms-queuedaemon
index 2b8d621..8affacc 100755
--- a/ms-queuedaemon
+++ b/ms-queuedaemon
@@ -1,4 +1,4 @@
-#!/usr/bin/tclsh8.4
+#!/usr/bin/tclsh8.5
 # -*- Tcl -*- 
 # usage: ./ms-queuedaemon  ... | logger
 
diff --git a/ms-reportuptime b/ms-reportuptime
index aeac483..fe72b78 100755
--- a/ms-reportuptime
+++ b/ms-reportuptime
@@ -1,4 +1,4 @@
-#!/usr/bin/tclsh8.4
+#!/usr/bin/tclsh8.5
 # -*- Tcl -*- 
 # usage: ./ms-reportuptime
 
diff --git a/sg-execute-flight b/sg-execute-flight
index 4e3fcf2..27fc04c 100755
--- a/sg-execute-flight
+++ b/sg-execute-flight
@@ -1,4 +1,4 @@
-#!/usr/bin/tclsh8.4
+#!/usr/bin/tclsh8.5
 # -*- Tcl -*- 
 # usage: ./sg-execute-flight FLIGHT BLESSING
 
diff --git a/sg-run-job b/sg-run-job
index 8b2d5e1..920834a 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -1,4 +1,4 @@
-#!/usr/bin/tclsh8.4
+#!/usr/bin/tclsh8.5
 
 # This is part of "osstest", an automated testing framework for Xen.
 # Copyright (C) 2009-2013 Citrix Inc.
-- 
2.1.4


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

[Xen-devel] [OSSTEST PATCH 3/8] mg-schema-test-database: Direct logs to local directory

[Ian Jackson]

Do not pollute a shared log area with logs of flights whose numbers
are valid only in the context of our test database.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 mg-schema-test-database | 8 
 1 file changed, 8 insertions(+)

diff --git a/mg-schema-test-database b/mg-schema-test-database
index 4e0ee68..88a75cf 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -338,6 +338,14 @@ QueueDaemonPort ${ctrlports#*,}
 ExecutiveDbOwningRoleRegexp .*
 QueueDaemonHoldoff 3
 QueueDaemonRetry 5
+Logs $PWD/logs
+Stash $PWD/logs
+Results $PWD/results
+Publish=undef
+LogsPublish=undef
+ResultsPublish=undef
+HarnessPublishGitUserHost=undef
+HarnessPublishGitRepoDir=undef
 END
mv -f $tcfg.tmp $tcfg
 
-- 
2.1.4


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

[Xen-devel] [OSSTEST PATCH 4/8] ms-planner: Support ClientNotes

[Ian Jackson]

No users yet.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 README.planner | 22 ++
 ms-planner | 23 +++
 2 files changed, 45 insertions(+)

diff --git a/README.planner b/README.planner
index daccef5..80c2506 100644
--- a/README.planner
+++ b/README.planner
@@ -286,6 +286,13 @@ Plan is:
Live
Job flight.job which made the allocation
 
+   ClientNotes
+   map from note key
+   to one of
+   scalar
+   array of values supplied by clients
+   hash of keys/values supplied by clients
+
* = internal to plan
** = computed by launder_check_plan
+ = as shown to clients
@@ -302,6 +309,12 @@ Booking list is:
Share   optional struct containing
Type
Shares
+   [ClientNotes]
+   as for Plan; merged by
+   for scalar, updating from Booking if specified
+   for array, appending Booking to Plan
+   for hash, updating individual values from Booking
+   type must not have changed!
 
 Sharing resources:
 
@@ -313,3 +326,12 @@ Bookings which do not create the share do not mention the 
master.
 
 Note that whether a resource is free, or simply nonexistent, is not
 represented.
+
+
+Note keys
+-
+
+Note key name (key in ClientNotes):
+   Note value format:
+
+(none yet defined)
diff --git a/ms-planner b/ms-planner
index abf5fac..bce6e13 100755
--- a/ms-planner
+++ b/ms-planner
@@ -245,6 +245,7 @@ sub cmd_reset () {
 $plan->{Start}= time;
 $plan->{Events}= { };
 $plan->{Unprocessed}= [ ];
+$plan->{ClientNotes}= { };
 
 my %magictask;
 foreach my $taskrefkey (qw(preparing shared)) {
@@ -415,6 +416,9 @@ sub cmd_get_plan () {
}
$jplan->{Events}{$reso}= \@jevts;
 }
+
+$jplan->{ClientNotes} = $plan->{ClientNotes};
+
 print to_json($jplan),"\n" or die $!;
 }
 
@@ -529,6 +533,25 @@ sub cmd_book_resources () {
  );
 }
 
+my $jnotes = $jbookings->{ClientNotes} // { };
+foreach my $k (sort keys %$jnotes) {
+   my $v = $jnotes->{$k};
+   my $newt = ref $v // '(none)';
+   if (exists $plan->{ClientNotes}{$k}) {
+   my $oldt = ref $plan->{ClientNotes}{$k} // '(none)';
+   die "$k $oldt -> $newt" unless $oldt eq $newt;
+   }
+   if (!ref $v) {
+   $plan->{ClientNotes}{$k} = $v;
+   } elsif (ref $v eq 'HASH') {
+   $plan->{ClientNotes}{$k}{$_} = $v->{$_} foreach keys %$v;
+   } elsif (ref $v eq 'ARRAY') {
+   push @{ $plan->{ClientNotes}{$_} }, @{ $jnotes->{$_} };
+   } else {
+   die "$k $newt";
+   }
+}
+
 check_write_new_plan();
 }
 
-- 
2.1.4


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

[Xen-devel] [OSSTEST PATCH 8/8] DO NOT APPLY make-flight-diverse-test: test case for diverse-CLASS hostflag

[Ian Jackson]

This generates a flight with two jobs, test-1 and test-2.  They can be
run, and are supposed to allocate different hosts, and will bomb out
after host allocation.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 make-flight-diverse-test | 11 +++
 1 file changed, 11 insertions(+)
 create mode 100755 make-flight-diverse-test

diff --git a/make-flight-diverse-test b/make-flight-diverse-test
new file mode 100755
index 000..dffd23b
--- /dev/null
+++ b/make-flight-diverse-test
@@ -0,0 +1,11 @@
+#!/bin/sh
+set -e
+
+flight=`./cs-flight-create play xen-unstable`
+
+for j in test-1 test-2; do
+./cs-job-create $flight $j test-debian \
+all_hostflags="blessed-adhoc,diverse-d"
+done
+
+echo $flight
-- 
2.1.4


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

[Xen-devel] [OSSTEST PATCH 0/8] Support diverse-CLASS hostflag

[Ian Jackson]

Here is a quick series for osstest which provides support for a new
kind of hostflag diverse-CLASS.  The host allocator will not consider
any host for such an allocation, which has been allocated for any
other such allocation (with the same CLASS) in the same flight.

This is going to be useful because we want to run several identical
XTF test jobs but run each one on a different host (since there is
probably little value on running two jobs on the same host in the same
flight).

Some of the state has to be threaded through the planner daemon.
Without the planner daemon patches, the projection will be wrong,
slightly messing up planning estimates etc., but the actual
allocations will, I think, be right.

It is fine to run the new host allocator against an old planner daemon
(even a production one), at least for testing.


These patches can be found in my personal xenbits repo
  xenbits.xen.org:/home/iwj/ext/osstest.git
  base.diverse-class.v1 .. wip.diverse-class.v1


These three

 [OSSTEST PATCH 1/8] Tcl: Use tclsh8.5
 [OSSTEST PATCH 2/8] ms-flights-summary: Remove spurious \ in keys \%{
 [OSSTEST PATCH 3/8] mg-schema-test-database: Direct logs to local

are preparatory patches which are probably going to be helpful if
someone wants to test the new planner (as I did, using the test
database facility).

Then there is the meat:

 [OSSTEST PATCH 4/8] ms-planner: Support ClientNotes
 [OSSTEST PATCH 5/8] ts-hosts-allocate-Executive: Replace some odd
 [OSSTEST PATCH 6/8] ts-hosts-allocate-Executive: pass $plan to
 [OSSTEST PATCH 7/8] ts-hosts-allocate-Executive: Support

And one helpful test script:

 [OSSTEST PATCH 8/8] DO NOT APPLY make-flight-diverse-test: test case


Thanks,
Ian.

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

[Xen-devel] [OSSTEST PATCH 7/8] ts-hosts-allocate-Executive: Support diverse-CLASS hostflag

[Ian Jackson]

Specifically:
 * Parse it out of the hostflags when constructing the hid
 * Look for the `hostalloc-diverse-FLIGHT-CLASS' ClientNote in
   the resource plan, to avoid inappropriately planning to reuse hosts.
 * Look for the `diversehosts_CLASS' runvar in other jobs in this flight,
   to find out who might have allocated with the same CLASS.  (This
   sort of duplicates information in *hostflags and *host, but digging
   the information out of the latter two would be very tiresome.)
 * Check each of the above for each candidate host.
 * Set the ClientNote when we are preparing a booking.
 * Set the runvar when we do the allocation.
 * Document the ClientNote.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 README.planner  |  3 +++
 ts-hosts-allocate-Executive | 59 +++--
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/README.planner b/README.planner
index 80c2506..bb18432 100644
--- a/README.planner
+++ b/README.planner
@@ -335,3 +335,6 @@ Note key name (key in ClientNotes):
Note value format:
 
 (none yet defined)
+
+hostalloc-diverse--
+   {  => 'info for debug', ... }
diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index d4a96d5..72283de 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -213,7 +213,7 @@ sub compute_hids () {
 our %equivs;
 
 foreach my $ident (@ARGV) {
-my $hid= { Conds => { host => [] } };
+my $hid= { Conds => { host => [] }, Diverse => [] };
 my $override_use;
 if ($ident =~ m/\=/) {
 $hid->{OverrideUse}= $'; #'
@@ -251,6 +251,11 @@ sub compute_hids () {
 my $equiv= $hid->{Equiv}= $equivs{$formalclass};
 print DEBUG "HID $ident FLAG $flag EQUIV $equiv->{Wanted}\n";
 next;
+} elsif ($flag =~ m/^diverse-/) {
+my $dclass= $'; #'
+   push @{ $hid->{Diverse} }, $dclass;
+   print DEBUG "HID $ident FLAG $flag DIVERSE $dclass\n";
+next;
 } elsif ($flag =~ m/^\w+:/) {
my (@c) = split /:/, $flag;
my $o;
@@ -322,9 +327,40 @@ END
 LIMIT 1;
 END
 
+# we are looking for other jobs where the runvar diversehosts_CLASS
+# (a space-separated list of hostnames) has been set
+my $diverseq = $dbh_tests->prepare(<{Diverse} }) {
+   print DEBUG "HID $hid->{Ident} DIVERSE $dclass:";
+   # look for allocated
+   $diverseq->execute($flight, "diversehosts_$dclass");
+   my ($djob, $dhosts) = $diverseq->fetchrow_array();
+   my $excl = sub {
+   my ($dh,$why) = @_;
+   print DEBUG " $dh ($why)";
+   $diverse_excluded{$dclass}{$dh} = $why;
+   };
+   if (defined $dhosts) {
+   $excl->($_, "alloc'd job $job") foreach split / /, $dhosts;
+   }
+   # look for booked
+   my $notekey = "hostalloc-diverse-$flight-$dclass";
+   my $booked = $plan->{ClientNotes}{$notekey};
+   $excl->($_, "boooked ".$booked->{$_}) foreach sort keys %$booked;
+   print DEBUG ".\n";
+}
+
   CANDIDATE:
 while (my $candrow= $findhostsq->fetchrow_hashref()) {
 $candrow->{Warnings}= [ ];
@@ -375,6 +411,13 @@ END
 "specified host lacks flags @missingflags";
 }
 
+   foreach my $dclass (@{ $hid->{Diverse} }) {
+   my $excl = $diverse_excluded{$dclass}{ $candrow->{resname} };
+   next unless defined $excl;
+   print DEBUG "$dbg excluded by diverse-$dclass ($excl)\n";
+   next CANDIDATE unless defined $use;
+   }
+
foreach my $c (@{ $hid->{Conds}{$candrow->{restype}} }) {
if (!$c->check($candrow->{restype}, $candrow->{resname})) {
print DEBUG "$dbg failed $c condition\n";
@@ -629,6 +672,13 @@ sub alloc_hosts () {
 my $use= $r{ $hid->{Ident} };
 next if defined $use;
 store_runvar($hid->{Ident}, $sel->{resname});
+   foreach my $dclass (@{ $hid->{Diverse} }) {
+   my $drk = "diversehosts_$dclass";
+   my %dused;
+   $dused{$_} = 1 foreach split / /, ($r{$drk} // '');
+   $dused{$sel->{resname}} = 1;
+   store_runvar($drk, join ' ', sort keys %dused);
+   }
 }
 
 logm("host allocation: all successful and recorded.");
@@ -697,6 +747,7 @@ sub attempt_allocation {
 
 sub compute_booking_list () {
 my @bookings;
+my %cnotes;
 foreach my $hid (@hids) {
my $sel= $hid->{Selected};
my $alloc= $hid->{Allocated};
@@ -712,8 +763,12 @@ sub compute_booking_list () {

[Xen-devel] [OSSTEST PATCH 6/8] ts-hosts-allocate-Executive: pass $plan to hid_find_possibilities

[Ian Jackson]

We are going to want this to implement the new diverse-CLASS hostflag.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 ts-hosts-allocate-Executive | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ts-hosts-allocate-Executive b/ts-hosts-allocate-Executive
index 69f1cf6..d4a96d5 100755
--- a/ts-hosts-allocate-Executive
+++ b/ts-hosts-allocate-Executive
@@ -272,8 +272,8 @@ sub compute_hids () {
 }
 }
 
-sub hid_find_possibilities ($) {
-my ($hid) = @_;
+sub hid_find_possibilities ($$) {
+my ($hid, $plan) = @_;
 
 delete $hid->{Candidates};
 
@@ -647,7 +647,7 @@ sub attempt_allocation {
 
 foreach my $hid (@hids) {
delete $hid->{Allocated};
-   hid_find_possibilities($hid);
+   hid_find_possibilities($hid, $plan);
 }
 optimally_order_hids();
 
-- 
2.1.4


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

Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 7:43 AM, Julien Grall  wrote:
> Hello Sergej,
>
>
> On 06/07/16 10:14, Sergej Proskurin wrote:
>>
>> On 07/05/2016 12:19 PM, Julien Grall wrote:
>>>
>>> Hello Sergej,
>>>
>>> On 04/07/16 12:45, Sergej Proskurin wrote:

 +static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
 +{
 +struct xen_hvm_altp2m_op a;
 +struct domain *d = NULL;
 +int rc = 0;
 +
 +if ( !hvm_altp2m_supported() )
 +return -EOPNOTSUPP;
 +
 +if ( copy_from_guest(, arg, 1) )
 +return -EFAULT;
 +
 +if ( a.pad1 || a.pad2 ||
 + (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
 + (a.cmd < HVMOP_altp2m_get_domain_state) ||
 + (a.cmd > HVMOP_altp2m_change_gfn) )
 +return -EINVAL;
 +
 +d = (a.cmd != HVMOP_altp2m_vcpu_enable_notify) ?
 +rcu_lock_domain_by_any_id(a.domain) :
 rcu_lock_current_domain();
 +
 +if ( d == NULL )
 +return -ESRCH;
 +
 +if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
 + (a.cmd != HVMOP_altp2m_set_domain_state) &&
 + !d->arch.altp2m_active )
 +{
 +rc = -EOPNOTSUPP;
 +goto out;
 +}
 +
 +if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
 +goto out;
>>>
>>>
>>> I think this is the best place to ask a couple of questions related to
>>> who can access altp2m. Based on this call, a guest is allowed to
>>> manage its own altp2m. Can you explain why we would want a guest to do
>>> that?
>>>
>>
>> On x86, altp2m might be used by the guest in the #VE (Virtualization
>> Exception). On ARM, there is indeed not necessary for a guest to access
>> altp2m. Could you provide me with information, how to best restrict
>> non-privileged guests (not only dom0) from accessing these HVMOPs? Can
>> thisbedone by means of xsm? Thank you.
>
>
> This does not looks safe for both x86 and ARM. From my understanding a
> malware would be able to modify an altp2m, switching between 2 view... which
> would lead to remove the entire purpose of altp2m.

Well, the whole purpose of the VMFUNC instruction right now is to be
able to switch the hypervisor's tables (like altp2m) from within the
guest. So yes, if you have a malicious guest then it could control
it's own altp2m views. I would assume there are other safe-guards
in-place for systems that use this to ensure kernel-integrity and thus
prevent arbitrary use of these functions. AFAIK there are only
experimental systems based on this so whether it's less or more secure
is debatable and likely depend on your implementation.

As for ARM - as there is no hardware features like this available -
our goal is to use altp2m in purely external usecases so exposing
these ops to the guest is not required. For the first prototype it
made sense to mirror the x86 side to reduce the possibility of
introducing some bug.

Cheers,
Tamas

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

[Xen-devel] [linux-3.18 baseline-only test] 66492: trouble: blocked/broken/fail/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 66492 linux-3.18 real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/66492/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-rumpuserxen   4 capture-logs !broken [st=!broken!]
 build-armhf-pvops 4 capture-logs !broken [st=!broken!]
 build-armhf-xsm   4 capture-logs !broken [st=!broken!]
 build-armhf   4 capture-logs !broken [st=!broken!]
 build-amd64-rumpuserxen   3 host-install(3) broken REGR. vs. 44416

Regressions which are regarded as allowable (not blocking):
 build-armhf-pvops 3 host-install(3)  broken like 44416
 build-armhf-xsm   3 host-install(3)  broken like 44416
 build-armhf   3 host-install(3)  broken like 44416
 build-i386-rumpuserxen6 xen-buildfail   like 44416

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

version targeted for testing:
 linuxd420f00c7bfb405884dd71fb7f87974f0d1be455
baseline version:
 linux6b12ebc0ecce75d7bd3660cd85f8b47a615c2071

Last test of basis44416  2016-05-14 19:20:44 Z   52 days
Testing same since66492  2016-07-01 10:54:36 Z5 days1 attempts


People who touched revisions under test:
  "Eric W. Biederman" 
  "Kirill A. Shutemov" 
  AceLan Kao 
  Adrian Hunter 
  Al Viro 
  Alan Stern 
  Alex Deucher 
  Andre Przywara 
  Andreas Noever 
  Andreas Werner 
  Andrew Donnellan 
  Andrew Jeffery 
  Andrew Morton 
  Andy Lutomirski  # On a Dell XPS 13 9350
  Anton Blanchard 
  Aristeu Rozanski 
  Arnaldo Carvalho de Melo 
  Arnd Bergmann 
  Artem Bityutskiy 
  Axel Lin 
  Ben Dooks 
  Ben Skeggs 
  Bjorn Helgaas 
  Bob Copeland 
  Bob Moore 
  Bobi Mihalca 
  Borislav Petkov 
  Brian Norris 
  Catalin Marinas 
  Catalin Vasile 
  Chen Yu 
  

[Xen-devel] [PATCH v2 0/4] libxl: add framework for device types

[Juergen Gross]

Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
especially on domain creation introduce a framework for that purpose.

I especially found it annoying that e.g. the vtpm callback issued the
error message for a failed attach of nic devices.

Changes in V2:
- added new patch 4 to move more pvusb specific stuff into libxl_pvusb.c
- patch 1: add macro to fill struct libxl__device_type as suggested by
  Ian Jackson
- patch 1: make struct libxl__device_type variables const as requested by
  Ian Jackson

Juergen Gross (4):
  libxl: add framework for device types
  libxl: refactor domcreate_attach_pci() to use device type framework
  libxl: refactor domcreate_attach_dtdev() to use device type framework
  libxl: move DEFINE_DEVICE* macros to libxl_internal.h

 tools/libxl/libxl.c  | 146 +--
 tools/libxl/libxl_create.c   | 271 +--
 tools/libxl/libxl_device.c   |  36 --
 tools/libxl/libxl_internal.h | 127 ++--
 tools/libxl/libxl_pci.c  |  32 +
 tools/libxl/libxl_pvusb.c|  29 +++--
 6 files changed, 255 insertions(+), 386 deletions(-)

-- 
2.6.6


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

[Xen-devel] [PATCH v2 4/4] libxl: move DEFINE_DEVICE* macros to libxl_internal.h

[Juergen Gross]

In order to be able to have all functions related to a device type in
a single source file move the macros used to generate device type
specific functions to libxl_internal.h. Rename the macros as they are
no longer local to a source file. While at it hide device remove and
device destroy in one macro as those are always used in pairs. Move
usage of the macros to the appropriate source files.

Signed-off-by: Juergen Gross 
---
 tools/libxl/libxl.c  | 143 +--
 tools/libxl/libxl_device.c   |  36 ---
 tools/libxl/libxl_internal.h | 106 +---
 tools/libxl/libxl_pvusb.c|  25 +---
 4 files changed, 115 insertions(+), 195 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index b3deef0..68b77b9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1980,7 +1980,7 @@ out:
 
/**/
 
 /* generic callback for devices that only need to set ao_complete */
-static void device_addrm_aocomplete(libxl__egc *egc, libxl__ao_device *aodev)
+void device_addrm_aocomplete(libxl__egc *egc, libxl__ao_device *aodev)
 {
 STATE_AO_GC(aodev->ao);
 
@@ -2055,9 +2055,9 @@ static int libxl__device_from_vtpm(libxl__gc *gc, 
uint32_t domid,
return 0;
 }
 
-void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
-   libxl_device_vtpm *vtpm,
-   libxl__ao_device *aodev)
+static void libxl__device_vtpm_add(libxl__egc *egc, uint32_t domid,
+   libxl_device_vtpm *vtpm,
+   libxl__ao_device *aodev)
 {
 STATE_AO_GC(aodev->ao);
 flexarray_t *front;
@@ -2629,8 +2629,9 @@ out:
 return;
 }
 
-void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
-   libxl_device_disk *disk, libxl__ao_device *aodev)
+static void libxl__device_disk_add(libxl__egc *egc, uint32_t domid,
+   libxl_device_disk *disk,
+   libxl__ao_device *aodev)
 {
 device_disk_add(egc, domid, disk, aodev, NULL, NULL);
 }
@@ -3432,8 +3433,9 @@ static int libxl__device_from_nic(libxl__gc *gc, uint32_t 
domid,
 return 0;
 }
 
-void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
-   libxl_device_nic *nic, libxl__ao_device *aodev)
+static void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
+  libxl_device_nic *nic,
+  libxl__ao_device *aodev)
 {
 STATE_AO_GC(aodev->ao);
 flexarray_t *front;
@@ -4298,136 +4300,49 @@ out:
 
 
/**/
 
-/* Macro for defining device remove/destroy functions in a compact way */
 /* The following functions are defined:
+ * libxl_device_disk_add
+ * libxl__add_disks
  * libxl_device_disk_remove
  * libxl_device_disk_destroy
+ * libxl_device_nic_add
+ * libxl__add_nics
  * libxl_device_nic_remove
  * libxl_device_nic_destroy
+ * libxl_device_vtpm_add
+ * libxl__add_vtpms
  * libxl_device_vtpm_remove
  * libxl_device_vtpm_destroy
  * libxl_device_vkb_remove
  * libxl_device_vkb_destroy
  * libxl_device_vfb_remove
  * libxl_device_vfb_destroy
- * libxl_device_usbctrl_remove
- * libxl_device_usbctrl_destroy
  */
-#define DEFINE_DEVICE_REMOVE_EXT(type, remtype, removedestroy, f)\
-int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,   \
-uint32_t domid, libxl_device_##type *type,  \
-const libxl_asyncop_how *ao_how)\
-{   \
-AO_CREATE(ctx, domid, ao_how);  \
-libxl__device *device;  \
-libxl__ao_device *aodev;\
-int rc; \
-\
-GCNEW(device);  \
-rc = libxl__device_from_##type(gc, domid, type, device);\
-if (rc != 0) goto out;  \
-\
-GCNEW(aodev);   \
-libxl__prepare_ao_device(ao, aodev);\
-aodev->action = LIBXL__DEVICE_ACTION_REMOVE;\
-aodev->dev = device;\
-aodev->callback = device_addrm_aocomplete;  \
-aodev->force = f;   \
-libxl__initiate_device_##remtype##_remove(egc, aodev);  

[Xen-devel] [PATCH v2 2/4] libxl: refactor domcreate_attach_pci() to use device type framework

[Juergen Gross]

Signed-off-by: Juergen Gross 
---
 tools/libxl/libxl_create.c   | 54 ++--
 tools/libxl/libxl_internal.h |  1 +
 tools/libxl/libxl_pci.c  | 32 ++
 3 files changed, 40 insertions(+), 47 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 5e05f6f..354c73f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,10 +742,8 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
 int ret);
 
-static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
- int ret);
-static void domcreate_attach_dtdev(libxl__egc *egc,
-   libxl__domain_create_state *dcs);
+static void domcreate_attach_dtdev(libxl__egc *egc, libxl__multidev *multidev,
+   int ret);
 
 static void domcreate_console_available(libxl__egc *egc,
 libxl__domain_create_state *dcs);
@@ -1406,6 +1404,7 @@ static const struct libxl_device_type *device_type_tbl[] 
= {
 __vtpm_devtype,
 __usbctrl_devtype,
 __usbdev_devtype,
+__pcidev_devtype,
 };
 
 static void domcreate_attach_devices(libxl__egc *egc,
@@ -1440,7 +1439,7 @@ static void domcreate_attach_devices(libxl__egc *egc,
 return;
 }
 
-domcreate_attach_pci(egc, multidev, 0);
+domcreate_attach_dtdev(egc, multidev, 0);
 return;
 
 error_out:
@@ -1480,52 +1479,13 @@ error_out:
 domcreate_complete(egc, dcs, ret);
 }
 
-static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *multidev,
- int ret)
-{
-libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-STATE_AO_GC(dcs->ao);
-int i;
-int domid = dcs->guest_domid;
-
-/* convenience aliases */
-libxl_domain_config *const d_config = dcs->guest_config;
-
-if (ret) {
-goto error_out;
-}
-
-for (i = 0; i < d_config->num_pcidevs; i++) {
-ret = libxl__device_pci_add(gc, domid, _config->pcidevs[i], 1);
-if (ret < 0) {
-LOG(ERROR, "libxl_device_pci_add failed: %d", ret);
-goto error_out;
-}
-}
-
-if (d_config->num_pcidevs > 0) {
-ret = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
-d_config->num_pcidevs);
-if (ret < 0) {
-LOG(ERROR, "libxl_create_pci_backend failed: %d", ret);
-goto error_out;
-}
-}
-
-domcreate_attach_dtdev(egc, dcs);
-return;
-
-error_out:
-assert(ret);
-domcreate_complete(egc, dcs, ret);
-}
-
 static void domcreate_attach_dtdev(libxl__egc *egc,
-   libxl__domain_create_state *dcs)
+   libxl__multidev *multidev,
+   int ret)
 {
+libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
 STATE_AO_GC(dcs->ao);
 int i;
-int ret;
 int domid = dcs->guest_domid;
 
 /* convenience aliases */
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d16161a..79ce392 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3408,6 +3408,7 @@ extern const struct libxl_device_type libxl__nic_devtype;
 extern const struct libxl_device_type libxl__vtpm_devtype;
 extern const struct libxl_device_type libxl__usbctrl_devtype;
 extern const struct libxl_device_type libxl__usbdev_devtype;
+extern const struct libxl_device_type libxl__pcidev_devtype;
 /*- Domain destruction -*/
 
 /* Domain destruction has been split into two functions:
diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 236bdd0..9676687 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1291,6 +1291,36 @@ out:
 return rc;
 }
 
+static void libxl__add_pcidevs(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+   libxl_domain_config *d_config,
+   libxl__multidev *multidev)
+{
+AO_GC;
+libxl__ao_device *aodev = libxl__multidev_prepare(multidev);
+int i, rc = 0;
+
+for (i = 0; i < d_config->num_pcidevs; i++) {
+rc = libxl__device_pci_add(gc, domid, _config->pcidevs[i], 1);
+if (rc < 0) {
+LOG(ERROR, "libxl_device_pci_add failed: %d", rc);
+goto out;
+}
+}
+
+if (d_config->num_pcidevs > 0) {
+rc = libxl__create_pci_backend(gc, domid, d_config->pcidevs,
+d_config->num_pcidevs);
+if (rc < 0) {
+LOG(ERROR, "libxl_create_pci_backend failed: %d", rc);
+goto out;
+}
+}
+
+out:
+aodev->rc = rc;
+aodev->callback(egc, aodev);
+}
+
 static int qemu_pci_remove_xenstore(libxl__gc *gc, uint32_t domid,
 

[Xen-devel] [PATCH v2 1/4] libxl: add framework for device types

[Juergen Gross]

Instead of duplicate coding for each device type (vtpms, usbctrls, ...)
especially on domain creation introduce a framework for that purpose.

Signed-off-by: Juergen Gross 
---
V2: - add macro to fill struct libxl__device_type as suggested by
  Ian Jackson
- make struct libxl__device_type variables const as requested by
  Ian Jackson
---
 tools/libxl/libxl.c  |   3 +
 tools/libxl/libxl_create.c   | 163 +--
 tools/libxl/libxl_internal.h |  20 ++
 tools/libxl/libxl_pvusb.c|   4 ++
 4 files changed, 76 insertions(+), 114 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 1c81239..b3deef0 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -7434,6 +7434,9 @@ out:
 return rc;
 }
 
+DEFINE_DEVICE_TYPE_STRUCT(nic);
+DEFINE_DEVICE_TYPE_STRUCT(vtpm);
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 1b99472..5e05f6f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,12 +742,6 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
 int ret);
 
-static void domcreate_attach_vtpms(libxl__egc *egc, libxl__multidev *multidev,
-   int ret);
-static void domcreate_attach_usbctrls(libxl__egc *egc,
-  libxl__multidev *multidev, int ret);
-static void domcreate_attach_usbdevs(libxl__egc *egc, libxl__multidev 
*multidev,
- int ret);
 static void domcreate_attach_pci(libxl__egc *egc, libxl__multidev *aodevs,
  int ret);
 static void domcreate_attach_dtdev(libxl__egc *egc,
@@ -1407,6 +1401,53 @@ static void domcreate_launch_dm(libxl__egc *egc, 
libxl__multidev *multidev,
 domcreate_complete(egc, dcs, ret);
 }
 
+static const struct libxl_device_type *device_type_tbl[] = {
+__nic_devtype,
+__vtpm_devtype,
+__usbctrl_devtype,
+__usbdev_devtype,
+};
+
+static void domcreate_attach_devices(libxl__egc *egc,
+ libxl__multidev *multidev,
+ int ret)
+{
+libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
+STATE_AO_GC(dcs->ao);
+int domid = dcs->guest_domid;
+libxl_domain_config *const d_config = dcs->guest_config;
+const struct libxl_device_type *dt;
+
+if (ret) {
+LOG(ERROR, "unable to add %s devices",
+device_type_tbl[dcs->device_type_idx]->type);
+goto error_out;
+}
+
+dcs->device_type_idx++;
+if (dcs->device_type_idx < ARRAY_SIZE(device_type_tbl)) {
+dt = device_type_tbl[dcs->device_type_idx];
+if (*(int *)((void *)d_config + dt->num_offset) > 0) {
+/* Attach devices */
+libxl__multidev_begin(ao, >multidev);
+dcs->multidev.callback = domcreate_attach_devices;
+dt->add(egc, ao, domid, d_config, >multidev);
+libxl__multidev_prepared(egc, >multidev, 0);
+return;
+}
+
+domcreate_attach_devices(egc, >multidev, 0);
+return;
+}
+
+domcreate_attach_pci(egc, multidev, 0);
+return;
+
+error_out:
+assert(ret);
+domcreate_complete(egc, dcs, ret);
+}
+
 static void domcreate_devmodel_started(libxl__egc *egc,
libxl__dm_spawn_state *dmss,
int ret)
@@ -1430,113 +1471,8 @@ static void domcreate_devmodel_started(libxl__egc *egc,
 }
 }
 
-/* Plug nic interfaces */
-if (d_config->num_nics > 0) {
-/* Attach nics */
-libxl__multidev_begin(ao, >multidev);
-dcs->multidev.callback = domcreate_attach_vtpms;
-libxl__add_nics(egc, ao, domid, d_config, >multidev);
-libxl__multidev_prepared(egc, >multidev, 0);
-return;
-}
-
-domcreate_attach_vtpms(egc, >multidev, 0);
-return;
-
-error_out:
-assert(ret);
-domcreate_complete(egc, dcs, ret);
-}
-
-static void domcreate_attach_vtpms(libxl__egc *egc,
-   libxl__multidev *multidev,
-   int ret)
-{
-   libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-   STATE_AO_GC(dcs->ao);
-   int domid = dcs->guest_domid;
-
-   libxl_domain_config* const d_config = dcs->guest_config;
-
-   if(ret) {
-   LOG(ERROR, "unable to add nic devices");
-   goto error_out;
-   }
-
-/* Plug vtpm devices */
-   if (d_config->num_vtpms > 0) {
-   /* Attach vtpms */
-   libxl__multidev_begin(ao, >multidev);
-   dcs->multidev.callback = domcreate_attach_usbctrls;
-   libxl__add_vtpms(egc, ao, domid, d_config, >multidev);
-   libxl__multidev_prepared(egc, >multidev, 0);
-   return;
-   }
-
-   

[Xen-devel] [PATCH v2 3/4] libxl: refactor domcreate_attach_dtdev() to use device type framework

[Juergen Gross]

Signed-off-by: Juergen Gross 
---
 tools/libxl/libxl_create.c | 68 +-
 1 file changed, 31 insertions(+), 37 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 354c73f..828f254 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -742,9 +742,6 @@ static void domcreate_bootloader_done(libxl__egc *egc,
 static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *aodevs,
 int ret);
 
-static void domcreate_attach_dtdev(libxl__egc *egc, libxl__multidev *multidev,
-   int ret);
-
 static void domcreate_console_available(libxl__egc *egc,
 libxl__domain_create_state *dcs);
 
@@ -1399,12 +1396,39 @@ static void domcreate_launch_dm(libxl__egc *egc, 
libxl__multidev *multidev,
 domcreate_complete(egc, dcs, ret);
 }
 
+static void libxl__add_dtdevs(libxl__egc *egc, libxl__ao *ao, uint32_t domid,
+  libxl_domain_config *d_config,
+  libxl__multidev *multidev)
+{
+AO_GC;
+libxl__ao_device *aodev = libxl__multidev_prepare(multidev);
+int i, rc = 0;
+
+for (i = 0; i < d_config->num_dtdevs; i++) {
+const libxl_device_dtdev *dtdev = _config->dtdevs[i];
+
+LOG(DEBUG, "Assign device \"%s\" to dom%u", dtdev->path, domid);
+rc = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
+if (rc < 0) {
+LOG(ERROR, "xc_assign_dtdevice failed: %d", rc);
+goto out;
+}
+}
+
+out:
+aodev->rc = rc;
+aodev->callback(egc, aodev);
+}
+
+static DEFINE_DEVICE_TYPE_STRUCT(dtdev);
+
 static const struct libxl_device_type *device_type_tbl[] = {
 __nic_devtype,
 __vtpm_devtype,
 __usbctrl_devtype,
 __usbdev_devtype,
 __pcidev_devtype,
+__dtdev_devtype,
 };
 
 static void domcreate_attach_devices(libxl__egc *egc,
@@ -1439,7 +1463,10 @@ static void domcreate_attach_devices(libxl__egc *egc,
 return;
 }
 
-domcreate_attach_dtdev(egc, multidev, 0);
+domcreate_console_available(egc, dcs);
+
+domcreate_complete(egc, dcs, 0);
+
 return;
 
 error_out:
@@ -1479,39 +1506,6 @@ error_out:
 domcreate_complete(egc, dcs, ret);
 }
 
-static void domcreate_attach_dtdev(libxl__egc *egc,
-   libxl__multidev *multidev,
-   int ret)
-{
-libxl__domain_create_state *dcs = CONTAINER_OF(multidev, *dcs, multidev);
-STATE_AO_GC(dcs->ao);
-int i;
-int domid = dcs->guest_domid;
-
-/* convenience aliases */
-libxl_domain_config *const d_config = dcs->guest_config;
-
-for (i = 0; i < d_config->num_dtdevs; i++) {
-const libxl_device_dtdev *dtdev = _config->dtdevs[i];
-
-LOG(DEBUG, "Assign device \"%s\" to dom%u", dtdev->path, domid);
-ret = xc_assign_dt_device(CTX->xch, domid, dtdev->path);
-if (ret < 0) {
-LOG(ERROR, "xc_assign_dtdevice failed: %d", ret);
-goto error_out;
-}
-}
-
-domcreate_console_available(egc, dcs);
-
-domcreate_complete(egc, dcs, 0);
-return;
-
-error_out:
-assert(ret);
-domcreate_complete(egc, dcs, ret);
-}
-
 static void domcreate_complete(libxl__egc *egc,
libxl__domain_create_state *dcs,
int rc)
-- 
2.6.6


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

Re: [Xen-devel] [PATCH v1 01/20] hvmloader: Provide hvmloader_acpi_build_tables()

[Konrad Rzeszutek Wilk]

On Tue, Jul 05, 2016 at 03:05:00PM -0400, Boris Ostrovsky wrote:
> In preparation for moving out ACPI builder make all
> BIOSes call hvmloader_acpi_build_tables() instead of
> calling ACPI code directly.
> 
> No functional changes.
> 
> Signed-off-by: Boris Ostrovsky 

Reviewed-by: Konrad Rzeszutek Wilk 
> ---
> 
> Changes in v1:
> * Added last sentence to commit message
> 
>  tools/firmware/hvmloader/ovmf.c|2 +-
>  tools/firmware/hvmloader/rombios.c |2 +-
>  tools/firmware/hvmloader/seabios.c |2 +-
>  tools/firmware/hvmloader/util.c|7 +++
>  tools/firmware/hvmloader/util.h|4 
>  5 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/firmware/hvmloader/ovmf.c b/tools/firmware/hvmloader/ovmf.c
> index db9fa7a..74fec56 100644
> --- a/tools/firmware/hvmloader/ovmf.c
> +++ b/tools/firmware/hvmloader/ovmf.c
> @@ -125,7 +125,7 @@ static void ovmf_acpi_build_tables(void)
>  .dsdt_15cpu_len = 0
>  };
>  
> -acpi_build_tables(, ACPI_PHYSICAL_ADDRESS);
> +hvmloader_acpi_build_tables(, ACPI_PHYSICAL_ADDRESS);
>  }
>  
>  static void ovmf_create_smbios_tables(void)
> diff --git a/tools/firmware/hvmloader/rombios.c 
> b/tools/firmware/hvmloader/rombios.c
> index 1f15b94..1f8fc2d 100644
> --- a/tools/firmware/hvmloader/rombios.c
> +++ b/tools/firmware/hvmloader/rombios.c
> @@ -180,7 +180,7 @@ static void rombios_acpi_build_tables(void)
>  .dsdt_15cpu_len = dsdt_15cpu_len,
>  };
>  
> -acpi_build_tables(, ACPI_PHYSICAL_ADDRESS);
> +hvmloader_acpi_build_tables(, ACPI_PHYSICAL_ADDRESS);
>  }
>  
>  static void rombios_create_mp_tables(void)
> diff --git a/tools/firmware/hvmloader/seabios.c 
> b/tools/firmware/hvmloader/seabios.c
> index c6b3d9f..9f66a79 100644
> --- a/tools/firmware/hvmloader/seabios.c
> +++ b/tools/firmware/hvmloader/seabios.c
> @@ -100,7 +100,7 @@ static void seabios_acpi_build_tables(void)
>  .dsdt_15cpu_len = 0,
>  };
>  
> -acpi_build_tables(, rsdp);
> +hvmloader_acpi_build_tables(, rsdp);
>  add_table(rsdp);
>  }
>  
> diff --git a/tools/firmware/hvmloader/util.c b/tools/firmware/hvmloader/util.c
> index 9382709..9af29b1 100644
> --- a/tools/firmware/hvmloader/util.c
> +++ b/tools/firmware/hvmloader/util.c
> @@ -21,6 +21,7 @@
>  #include "config.h"
>  #include "hypercall.h"
>  #include "ctype.h"
> +#include "acpi/acpi2_0.h"
>  #include 
>  #include 
>  #include 
> @@ -856,6 +857,12 @@ int hpet_exists(unsigned long hpet_base)
>  return ((hpet_id >> 16) == 0x8086);
>  }
>  
> +void hvmloader_acpi_build_tables(struct acpi_config *config,
> + unsigned int physical)
> +{
> +acpi_build_tables(config, physical);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/firmware/hvmloader/util.h b/tools/firmware/hvmloader/util.h
> index 3126817..b226df4 100644
> --- a/tools/firmware/hvmloader/util.h
> +++ b/tools/firmware/hvmloader/util.h
> @@ -273,6 +273,10 @@ extern struct e820map memory_map;
>  bool check_overlap(uint64_t start, uint64_t size,
> uint64_t reserved_start, uint64_t reserved_size);
>  
> +struct acpi_config;
> +void hvmloader_acpi_build_tables(struct acpi_config *config,
> + unsigned int physical);
> +
>  #endif /* __HVMLOADER_UTIL_H__ */
>  
>  /*
> -- 
> 1.7.1
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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

Re: [Xen-devel] [PATCH 11/18] arm/altp2m: Make flush_tlb_domain ready for altp2m.

[Sergej Proskurin]

Hi Julien,


On 07/06/2016 04:28 PM, Julien Grall wrote:
>
>
> On 05/07/16 21:21, Sergej Proskurin wrote:
>> I agree on this point as well. However, we should maybe think of another
>> name for flush_tlb_domain. Especially, if we do not flush the entire
>> domain (including all currently active altp2m views). What do you think?
>
> This function is only used within p2m.c, although it is exported. What
> about renaming this function to flush_tlb_p2m and instead pass the p2m
> in parameter?
>
> Regards,
>

That sounds good to me :) I will do that. Thank you.

Cheers,
~Sergej

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

Re: [Xen-devel] [PATCH 14/18] arm/altp2m: Add HVMOP_altp2m_set_mem_access.

[Julien Grall]

On 05/07/16 22:55, Sergej Proskurin wrote:

Hello Julien,

On 07/05/2016 02:49 PM, Julien Grall wrote:

Hello Sergej,

On 04/07/16 12:45, Sergej Proskurin wrote:

+static inline
+int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain *hp2m,
+  struct p2m_domain *ap2m, p2m_access_t a,
+  gfn_t gfn)
+{


[...]


+/* Set mem access attributes - currently supporting only one (4K)
page. */
+mask = level_masks[3];
+return apply_p2m_changes(d, ap2m, INSERT,
+ gpa & mask,
+ (gpa + level_sizes[level]) & mask,
+ maddr & mask, mattr, 0, p2mt, a);


The operation INSERT will remove the previous mapping and decrease page
reference for foreign mapping (see p2m_put_l3_page). So if you set the
memory access for this kind of page, the reference count will be wrong
afterward.



I see your point. As far as I understand, the purpose of the function
p2m_put_l3_page to simply decrement the ref count of the particular pte
and free the page if its' ref count reaches zero. Since p2m_put_l3_page
is called only twice in p2m.c, we could insert a check
(p2m_is_hostp2m/altp2m) making sure that the ref count is decremented
only if the p2m in question is the hostp2m. This will make sure that the
ref count is maintained correctly.


Why don't you use the operation MEMACCESS?

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 11/18] arm/altp2m: Make flush_tlb_domain ready for altp2m.

[Julien Grall]

On 05/07/16 21:21, Sergej Proskurin wrote:

I agree on this point as well. However, we should maybe think of another
name for flush_tlb_domain. Especially, if we do not flush the entire
domain (including all currently active altp2m views). What do you think?


This function is only used within p2m.c, although it is exported. What 
about renaming this function to flush_tlb_p2m and instead pass the p2m 
in parameter?


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 15/18] arm/altp2m: Add altp2m paging mechanism.

[Julien Grall]

On 06/07/16 09:33, Sergej Proskurin wrote:

On 07/04/2016 10:53 PM, Julien Grall wrote:

Can we introduce a function which return the xma, mfn, order,
attribute at once? It will avoid to browse the p2m 3 times which is
really expensive on ARMv7 because the p2m is not mapped in the virtual
address space of Xen.



I was already thinking of at least merging p2m_get_gfn_level_and_attr
with __p2m_lookup. But it would also make sense to introduce an entirely
new function, which does just that.I believe increasing the overhead of
__p2m_lookup would not be a good solution. Thank you.


How the overhead of __p2m_lookup will be increased? level, mattr and 
type is already available in p2m_lookup. For the mem access, as you pass 
a pointer you can retrieve the access only when the pointer is not NULL.





+spin_unlock(>lock);
+
+mask = level_masks[level];
+
+rc = apply_p2m_changes(d, *ap2m, INSERT,
+   pfn_to_paddr(gfn_x(gfn)) & mask,
+   (pfn_to_paddr(gfn_x(gfn)) +
level_sizes[level]) & mask,
+   maddr & mask, mattr, 0, p2mt,
+   memaccess[xma]);


The page associated to the MFN is not locked, so another thread could
decide to remove the page from the domain and then the altp2m would
contain an entry to something that does not belong to the domain
anymore. Note that x86 is doing the same. So I am not sure why it is
considered safe there...



If I understand you correctly, unlocking the hp2m->lock after calling
apply_p2m_changeswould already solve this issue, right? Thanks.


That would solve it. However, you will add a lot of contention on hp2m 
when each vCPU is using a different empty view.





+if ( rc )
+{
+gdprintk(XENLOG_ERR, "failed to set entry for %lx -> %lx p2m
%lx\n",
+(unsigned long)pfn_to_paddr(gfn_x(gfn)), (unsigned
long)(maddr), (unsigned long)*ap2m);
+domain_crash(hp2m->domain);
+}
+
+return 1;
+
+out:
+spin_unlock(>lock);
+return 0;
+}
+
  static void p2m_init_altp2m_helper(struct domain *d, unsigned int i)
  {
  struct p2m_domain *p2m = d->arch.altp2m_p2m[i];


[...]


@@ -2429,6 +2460,8 @@ static void do_trap_data_abort_guest(struct
cpu_user_regs *regs,
   const union hsr hsr)
  {
  const struct hsr_dabt dabt = hsr.dabt;
+struct vcpu *v = current;
+struct p2m_domain *p2m = NULL;
  int rc;
  mmio_info_t info;

@@ -2449,6 +2482,12 @@ static void do_trap_data_abort_guest(struct
cpu_user_regs *regs,
  info.gpa = get_faulting_ipa();
  else
  {
+/*
+ * When using altp2m, this flush is required to get rid of
old TLB
+ * entries and use the new, lazily copied, ap2m entries.
+ */
+flush_tlb_local();


Can you give more details why this flush is required?



Without the flush, the guest crashed to an unresolved data abort. To be
more precise, after the first lazy-copy of a mapping from the hostp2m to
the currently active altp2m view, the system crashed because it was not
able to find the new mapping in its altp2m table. The explicit flush
solved this issue quite nicely.


This explicit flush likely means there is another issue in the code. 
This flush should not be necessary.


Can you give more details about the exact data abort problem? Is it 
because the translation VA -> IPA is failing?




As I answer your question, I am starting to think that the crash was a
result of of a lack of a memory barrier because even with the old
(hostp2m's) TLBs present, the translation would not present an issue
(the mapping would be the same as the p2m entry is simply copied from
the hostp2m to the active altp2m view). Also, the guest would reuse the
old TLBs, the system would have used the access rights of the hostp2m,
and hence again be trapped by Xen.


I am sorry but I don't understand what you are trying to say.


I will try to solve this issue by means of a memory barrier, thank you.


Can you details where you think there is a lack of memory barrier?

Regards,

--
Julien Grall

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

Re: [Xen-devel] Kernel panic on Xen virtualisation in Debian

[George Dunlap]

On Mon, Jul 4, 2016 at 7:06 PM, Jan Prunk  wrote:
> Hello !
>
> I am posting Xen virtualisation bug links to this e-mail address,
> because I wasn't able to find the Xen specific bugtracker list.
> This bug has been discovered in 2015 and so far it hasn't been
> resolved through the Debian/Kernel bug lists. I submit the
> links to bug reports for you.
>
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=804079

The serial log at the bottom looks like there was a crash in the ipv6
handling as the result of a packet delivery, perhaps?  David / Wei, do
you have any ideas?  Not sure who else has worked on the netback side
of things.

 -George

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

Re: [Xen-devel] xenbits "official" repo for XTF (was Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)

[Konrad Rzeszutek Wilk]

> Looking at the above, it occurs to me that, this whole area seems to be a
> little inconsistent anyway and could do with a little house-keeping. We
> have
> - osstest.git
> - there also is osstest/*.git which seems to be odd and seems to have been
> inactive for a while (not very clear to me what these do)
> - and we have old and inactive xentesttools/*.git

They are not inactive. I have some patches for it and we are using
it (Boris and me).

> - and we are adding a new repo for XTF
> 
> 
> Maybe, moving everything test related under testing/* or test/* would be
> sensible, but that would cause some disruption (not sure how bad that
> would be). It would address A, B and D. It wouldn't make C much worse than
> now.

I don't mind the xentesttools going under a 'test' directory. 

> 
> We already follow a similar pattern for out-of-tree via pvdrivers/*
> 
> It does in fact occur to me that some of the older inactive repos should
> be archived somehow: candidates seem to be kemari/*, xentesttools/*,
> xenclient/*, xcp/*, ... pollute the namespace. If we are concerned about
> the namespace, we should address this at some point. There are also some
> inactive top level git repos such as linux-2.5-xen.git and quite a few
> inactive people repos.

The Linux ones, the linux-pvops.git could go.

> 
> >Lars, can you please advise what process we need to use to come to
> >closure on this decision ?
> 

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

Re: [Xen-devel] [PATCH v2] xen: arm64: Add support for Renesas RCar Gen3 H3 Salvator-X platform

[Julien Grall]

On 06/07/16 15:03, Dirk Behme wrote:

On 06.07.2016 15:17, Julien Grall wrote:

Hi Dirk,

On 06/07/16 07:33, Dirk Behme wrote:

Could you share the U-Boot commands how you load and esp. start Xen? For
loading you use TFTP? How do you start Xen with U-Boot, then? I think we
have to pass the device tree address in x0 and the Linux kernel image
address in x2. How do you do this with an U-Boot command?


U-boot can load Xen from TFTP or from the SD-card. This is the same as
booting a baremetal kernel with U-boot.

There is a section on the wiki page to explain how to create the
device-tree node for the boot modules [1] and the allwinner page [2]
gives a full example how to boot Xen with U-boot via tftp (Note that it
could easily be adapted to load from the SD-Card).

Regards,

[1]
http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Boot_Modules


[2]
http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/Allwinner#Boot_script




Hmm, sorry, I still seem to miss anything :(

I've loaded xen/xen with U-Boot to 0x4A00 and then try to start it
as described in [2] above:

=> bootz 0x4A00 - 0x4800
Bad Linux ARM zImage magic!

I'm not sure why xen/xen is assumed to be a zImage?


Oh, sorry I meant to say that bootz should be replaced by booti but 
forgot to write it down.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v2] xen: arm64: Add support for Renesas RCar Gen3 H3 Salvator-X platform

[Dirk Behme]

On 06.07.2016 15:17, Julien Grall wrote:

Hi Dirk,

On 06/07/16 07:33, Dirk Behme wrote:

Could you share the U-Boot commands how you load and esp. start Xen? For
loading you use TFTP? How do you start Xen with U-Boot, then? I think we
have to pass the device tree address in x0 and the Linux kernel image
address in x2. How do you do this with an U-Boot command?


U-boot can load Xen from TFTP or from the SD-card. This is the same as
booting a baremetal kernel with U-boot.

There is a section on the wiki page to explain how to create the
device-tree node for the boot modules [1] and the allwinner page [2]
gives a full example how to boot Xen with U-boot via tftp (Note that it
could easily be adapted to load from the SD-Card).

Regards,

[1]
http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions#Boot_Modules

[2]
http://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions/Allwinner#Boot_script



Hmm, sorry, I still seem to miss anything :(

I've loaded xen/xen with U-Boot to 0x4A00 and then try to start it 
as described in [2] above:


=> bootz 0x4A00 - 0x4800
Bad Linux ARM zImage magic!

I'm not sure why xen/xen is assumed to be a zImage?

To verify that the address is correct, I can use U-Boot's go command 
(which then will fail, later, as I can't pass the device tree address):


=> go 0x4A00
## Starting application at 0x4A00 ...
- UART enabled -
- CPU  booting -
- Current EL 0008 -
- Xen starting at EL2 -
- Zero BSS -
- Setting up control registers -
- Turning on paging -
- Ready -
(XEN)
(XEN) 
(XEN) Panic on CPU 0:
(XEN) No valid device tree
(XEN)
(XEN) 
(XEN)
(XEN) Reboot in five seconds...


Best regards

Dirk









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

Re: [Xen-devel] xenstored memory leak

[Andrew Cooper]

On 06/07/16 14:55, Juergen Gross wrote:
> On 06/07/16 15:48, Andrew Cooper wrote:
>> On 06/07/16 08:31, Juergen Gross wrote:
>>> While testing some patches for support of ballooning in Mini-OS by using
>>> the xenstore domain I realized that each xl create/destroy pair would
>>> increase memory consumption in Mini-OS by about 5kB. Wondering whether
>>> this is a xenstore domain only effect I did the same test with xenstored
>>> and oxenstored daemons.
>>>
>>> xenstored showed the same behavior, the "referenced" size showed by the
>>> pmap command grew by about 5kB for each create/destroy pair.
>>>
>>> oxenstored seemed to be even worse in the beginning (about 6kB for each
>>> pair), but after about 100 create/destroys the value seemed to be
>>> rather stable.
>> Do you mean that after a while, you see oxenstored not leaking any
>> further memory, even with new domains being created?
> In my test: yes. I did:
>
> while true
> do
>   xl create minios.xl
>   sleep 3
>   xl shutdown minios
>   sleep 2
> done
>
> After about 200 iterations memory usage with oxenstored was stable. I
> stopped the loop after more than 1000 iterations.
>
>> Ocaml is a garbage collected languague, so you would expect the process
>> to get larger until the GC decides to kick in.
> Okay. This explains the pattern.
>
>>> Did anyone notice this memory leak before?
>> We have not encountered this in XenServer stress scenarios.
> You are using oxenstored, right? The real leak is in xenstored only.

Correct.

>
>> (It is entirely possible that this specific to something xl does which
>> Xapi doesn't.)
> I doubt that. I'm seeing the leak with the C-variant of xenstore, both
> as daemon and as stubdom.

Right, and we haven't used C xenstored in the last decade.

~Andrew

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

Re: [Xen-devel] xenstored memory leak

[Juergen Gross]

On 06/07/16 15:48, Andrew Cooper wrote:
> On 06/07/16 08:31, Juergen Gross wrote:
>> While testing some patches for support of ballooning in Mini-OS by using
>> the xenstore domain I realized that each xl create/destroy pair would
>> increase memory consumption in Mini-OS by about 5kB. Wondering whether
>> this is a xenstore domain only effect I did the same test with xenstored
>> and oxenstored daemons.
>>
>> xenstored showed the same behavior, the "referenced" size showed by the
>> pmap command grew by about 5kB for each create/destroy pair.
>>
>> oxenstored seemed to be even worse in the beginning (about 6kB for each
>> pair), but after about 100 create/destroys the value seemed to be
>> rather stable.
> 
> Do you mean that after a while, you see oxenstored not leaking any
> further memory, even with new domains being created?

In my test: yes. I did:

while true
do
  xl create minios.xl
  sleep 3
  xl shutdown minios
  sleep 2
done

After about 200 iterations memory usage with oxenstored was stable. I
stopped the loop after more than 1000 iterations.

> Ocaml is a garbage collected languague, so you would expect the process
> to get larger until the GC decides to kick in.

Okay. This explains the pattern.

>> Did anyone notice this memory leak before?
> 
> We have not encountered this in XenServer stress scenarios.

You are using oxenstored, right? The real leak is in xenstored only.

> (It is entirely possible that this specific to something xl does which
> Xapi doesn't.)

I doubt that. I'm seeing the leak with the C-variant of xenstore, both
as daemon and as stubdom.


Juergen


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