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

[osstest service owner]

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

Failures :-/ but no regressions.

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

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

version targeted for testing:
 xen  16b69afbba6b8692c96ec5a6864c35fa2fac1d36
baseline version:
 xen  db59dbc46e0312a38978d22c6bd72b554a2f1c91

Last test of basis96756  2016-07-07 08:02:24 Z0 days
Testing same since96769  2016-07-07 18:52:54 Z0 days1 attempts


People who touched revisions under test:
  Corneliu ZUZU 
  Razvan Cojocaru 
  Tamas K Lengyel 
  Tamas K Lengyel 

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  

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

[Dirk Behme]

On 08.07.2016 04:50, Michael Turquette wrote:

Quoting Dirk Behme (2016-07-07 00:32:34)

Hi Michael,

On 06.07.2016 22:42, Michael Turquette wrote:

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.



Maybe we need some advice here :)


Sure!




I've used clk_prepare_enable() 'just' to get the enable count incremented


clk_prepare_enabled will *enable* the clock signal if it currently
disabled or gated. In other words, if the physical line is not toggling
before the call, it will be after the call returns.



http://lxr.free-electrons.com/source/drivers/clk/clk.c#L751

Because it's my understanding that enable_count is needed to prevent
clk_disable_unused() from disabling the clock.


Having a positive enable_count will prevent the clock from being
disabled by both clk_disable_unused AND from the Sneaky Sibling
Attack(tm).

The Sneaky Sibling Attack(tm) occurs when clock A and clock B are
siblings and share the same parent, clock C. If clock A is enabled in
hardware (by bootloader, firmware or hypervisor), but does NOT have a
positive enable_count (in Linux), then it is possible that a driver
might call clk_enable(clk_B) then clk_disable(clk_B), which will result
in the disable action propagating up the parent chain and disabling
clk_C, the shared parent. This will of course gate clk_A, which is
clocked by clk_C, breaking things for you.

So you need to be worried about more than just clk_disable_unused.

The simple fact is is that if a piece of software knows that it needs
for its clock to be enabled, it should actively enable it with
clk_prepare_enable.

Doing some weird stuff with CLK_IGNORE_UNUSED or anything else is just
hoping that your clock will not be disabled, and that is the wrong
strategy.




If there is an other / better / correct way to achieve that, please let
us know.


Well, you should not "try to prevent a clock from being disabled", you
should "enable the clock that you need to use".




I've had a look to use the CLK_IGNORE_UNUSED flag, too. But couldn't
find a function exported by the clock framework to set that flag (?)


Right, the flags are immutable and must be set by the clock provider
driver before registering the clock. Toggling flags at run-time is a
misuse of the flags, and clock consumer drivers should never care about
the flags. They are internal to the clock framework.

In conclusion, I think that your patch does the right thing. The Xen
node consumes the clocks that it needs to manage and it calls
clk_prepare_enable on them.



Ok, thanks!



The two issues to resolve are:

1) does consuming these hardware resources from the Linux kernel fit
correctly with the Xen model?



I think so. Julien, what do you think?



2) the language of the binding description makes this way more confusing
than it needs to be. Just claim the resources you need and enable them,
which is an OS-agnostic action.



I'll post a v3 patch trying to update the description with your proposals.


Best regards

Dirk

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

Re: [Xen-devel] [PATCH v3 17/17] libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ

[Shannon Zhao]

On 2016/7/8 0:57, Julien Grall wrote:
> On 07/07/16 17:15, Wei Liu wrote:
>> On Tue, Jul 05, 2016 at 11:12:47AM +0800, Shannon Zhao wrote:
>>> From: Shannon Zhao 
>>>
>>> The guest kernel will get the event channel interrupt information via
>>> domain param HVM_PARAM_CALLBACK_IRQ. Initialize it here.
>>>
>>> Signed-off-by: Shannon Zhao 
>>> ---
>>>   tools/libxl/libxl_arm.c | 11 +++
>>>   1 file changed, 11 insertions(+)
>>>
>>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>>> index bc38318..acacba0 100644
>>> --- a/tools/libxl/libxl_arm.c
>>> +++ b/tools/libxl/libxl_arm.c
>>> @@ -900,8 +900,19 @@ int
>>> libxl__arch_domain_init_hw_description(libxl__gc *gc,
>>>  struct xc_dom_image *dom)
>>>   {
>>>   int rc;
>>> +uint64_t val;
>>>
>>>   assert(info->type == LIBXL_DOMAIN_TYPE_PV);
>>> +
>>> +/* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
>>> +val = (uint64_t)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
>>> +val |= (2 << 8); /* Active-low level-sensitive  */
>>
>> Please avoid using magic numbers here -- 56, 2 and 8.
> 
> The magic numbers are described in public/hvm/params.h however there is
> no defines associated to them.
> 
> The public header would need to be updated if we don't want the value
> hardcoded in libxl.
> 
ok, so do we decide to update that?

>>
>> Another question to Julien and Stefano: is it normal for ARM guest to
>> use hvm callback vector?
> 
> Yes. The HVM callback vector is used by ACPI guest to find the PPI
> (per-cpu interrupt) which will be used to notify event.
> 
> This is how DOM0 is using ACPI on ARM (see [1]), I don't think we should
> differ here.
> 
> BTW, I have noticed that the design doc is only available on the ML.
> Shannon, would it be possible to send a patch to add it in docs/misc/arm?
> 
Ok, I'll send a patch later.

> [2] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html

-- 
Shannon


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

Re: [Xen-devel] [PATCH v3 07/17] libxl/arm: Construct ACPI RSDP table

[Shannon Zhao]

On 2016/7/8 6:43, Boris Ostrovsky wrote:
>> +calculate_checksum(rsdp, offsetof(struct acpi_table_rsdp, checksum),
>> > +   acpitables[RSDP].size);
> Should this be extended_checksum? checksum is for ACPI v1 (and thus for
> smaller size structure, with only RSDT pointer).
Ah, right. Will fix this.

Thanks,
-- 
Shannon


___
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 Dirk Behme (2016-07-07 00:32:34)
> Hi Michael,
> 
> On 06.07.2016 22:42, Michael Turquette wrote:
> > 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.
> 
> 
> Maybe we need some advice here :)

Sure!

> 
> 
> I've used clk_prepare_enable() 'just' to get the enable count incremented

clk_prepare_enabled will *enable* the clock signal if it currently
disabled or gated. In other words, if the physical line is not toggling
before the call, it will be after the call returns.

> 
> http://lxr.free-electrons.com/source/drivers/clk/clk.c#L751
> 
> Because it's my understanding that enable_count is needed to prevent 
> clk_disable_unused() from disabling the clock.

Having a positive enable_count will prevent the clock from being
disabled by both clk_disable_unused AND from the Sneaky Sibling
Attack(tm).

The Sneaky Sibling Attack(tm) occurs when clock A and clock B are
siblings and share the same parent, clock C. If clock A is enabled in
hardware (by bootloader, firmware or hypervisor), but does NOT have a
positive enable_count (in Linux), then it is possible that a driver
might call clk_enable(clk_B) then clk_disable(clk_B), which will result
in the disable action propagating up the parent chain and disabling
clk_C, the shared parent. This will of course gate clk_A, which is
clocked by clk_C, breaking things for you.

So you need to be worried about more than just clk_disable_unused.

The simple fact is is that if a piece of software knows that it needs
for its clock to be enabled, it should actively enable it with
clk_prepare_enable.

Doing some weird stuff with CLK_IGNORE_UNUSED or anything else is just
hoping that your clock will not be disabled, and that is the wrong
strategy.

> 
> 
> If there is an other / better / correct way to achieve that, please let 
> us know.

Well, you should not "try to prevent a clock from being disabled", you
should "enable the clock that you need to use".

> 
> 
> I've had a look to use the CLK_IGNORE_UNUSED flag, too. But couldn't 
> find a function exported by the clock framework to set that flag (?)

Right, the flags are immutable and must be set by the clock provider
driver before registering the clock. Toggling flags at run-time is a
misuse of the flags, and clock consumer drivers should never care about
the flags. They are internal to the clock framework.

In conclusion, I think that your patch does the right thing. The Xen
node consumes the clocks that it needs to manage and it calls
clk_prepare_enable on them. The two issues to resolve are:

1) does consuming these hardware resources from the Linux kernel fit
correctly with the Xen model?
2) the language of the binding description makes this way more confusing
than it needs to be. Just claim the resources you need and enable them,
which is an OS-agnostic action.

Regards,
Mike

> 
> 
> Best regards
> 
> Dirk

___
Xen-devel mailing list

[Xen-devel] [PATCH] vmx/monitor: CPUID events

[Tamas K Lengyel]

This patch implements sending notification to a monitor subscriber when an
x86/vmx guest executes the CPUID instruction.

Signed-off-by: Tamas K Lengyel 
---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: Razvan Cojocaru 
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Jun Nakajima 
Cc: Kevin Tian 
---
 tools/libxc/include/xenctrl.h   |  1 +
 tools/libxc/xc_monitor.c| 13 +
 tools/tests/xen-access/xen-access.c | 33 -
 xen/arch/x86/hvm/monitor.c  | 16 
 xen/arch/x86/hvm/vmx/vmx.c  | 23 +++
 xen/arch/x86/monitor.c  | 13 +
 xen/include/asm-x86/domain.h|  1 +
 xen/include/asm-x86/hvm/monitor.h   |  1 +
 xen/include/asm-x86/monitor.h   |  3 ++-
 xen/include/public/domctl.h |  1 +
 xen/include/public/vm_event.h   |  8 
 11 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 4a85b4a..e904bd5 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2167,6 +2167,7 @@ int xc_monitor_guest_request(xc_interface *xch, domid_t 
domain_id,
  bool enable, bool sync);
 int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
 bool enable, bool sync);
+int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
 /**
  * This function enables / disables emulation for each REP for a
  * REP-compatible instruction.
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index 264992c..4298813 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -172,6 +172,19 @@ int xc_monitor_debug_exceptions(xc_interface *xch, domid_t 
domain_id,
 return do_domctl(xch, );
 }
 
+int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable)
+{
+DECLARE_DOMCTL;
+
+domctl.cmd = XEN_DOMCTL_monitor_op;
+domctl.domain = domain_id;
+domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE
+: XEN_DOMCTL_MONITOR_OP_DISABLE;
+domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_CPUID;
+
+return do_domctl(xch, );
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/tools/tests/xen-access/xen-access.c 
b/tools/tests/xen-access/xen-access.c
index 02655d5..d525b82 100644
--- a/tools/tests/xen-access/xen-access.c
+++ b/tools/tests/xen-access/xen-access.c
@@ -337,7 +337,7 @@ void usage(char* progname)
 {
 fprintf(stderr, "Usage: %s [-m]  write|exec", progname);
 #if defined(__i386__) || defined(__x86_64__)
-fprintf(stderr, "|breakpoint|altp2m_write|altp2m_exec|debug");
+fprintf(stderr, 
"|breakpoint|altp2m_write|altp2m_exec|debug|cpuid");
 #endif
 fprintf(stderr,
 "\n"
@@ -364,6 +364,7 @@ int main(int argc, char *argv[])
 int shutting_down = 0;
 int altp2m = 0;
 int debug = 0;
+int cpuid = 1;
 uint16_t altp2m_view_id = 0;
 
 char* progname = argv[0];
@@ -426,6 +427,10 @@ int main(int argc, char *argv[])
 {
 debug = 1;
 }
+else if ( !strcmp(argv[0], "cpuid") )
+{
+cpuid = 1;
+}
 #endif
 else
 {
@@ -548,6 +553,16 @@ int main(int argc, char *argv[])
 }
 }
 
+if ( cpuid )
+{
+rc = xc_monitor_cpuid(xch, domain_id, 1);
+if ( rc < 0 )
+{
+ERROR("Error %d setting cpuid listener with vm_event\n", rc);
+goto exit;
+}
+}
+
 /* Wait for access */
 for (;;)
 {
@@ -560,6 +575,8 @@ int main(int argc, char *argv[])
 rc = xc_monitor_software_breakpoint(xch, domain_id, 0);
 if ( debug )
 rc = xc_monitor_debug_exceptions(xch, domain_id, 0, 0);
+if ( cpuid )
+rc = xc_monitor_cpuid(xch, domain_id, 0);
 
 if ( altp2m )
 {
@@ -716,6 +733,20 @@ int main(int argc, char *argv[])
 }
 
 break;
+case VM_EVENT_REASON_CPUID:
+printf("CPUID executed: rip=%016"PRIx64", vcpu %d. Insn 
length: %"PRIu32" " \
+   "EAX: 0x%"PRIx64" EBX: 0x%"PRIx64" ECX: 0x%"PRIx64" 
EDX: 0x%"PRIx64"\n",
+   req.data.regs.x86.rip,
+   req.vcpu_id,
+   req.u.cpuid.insn_length,
+   req.data.regs.x86.rax,
+   req.data.regs.x86.rbx,
+   req.data.regs.x86.rcx,
+   req.data.regs.x86.rdx);
+rsp.flags |= VM_EVENT_FLAG_SET_REGISTERS;
+rsp.data = req.data;
+rsp.data.regs.x86.rip += req.u.cpuid.insn_length;
+

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl  15 guest-start/debian.repeat fail REGR. vs. 96732

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-rtds  9 debian-install   fail blocked in 96732
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 96732
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 96732

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-amd64-i386-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-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass
 test-armhf-armhf-xl-credit2  13 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-cubietruck 12 migrate-support-checkfail never pass
 test-armhf-armhf-xl-cubietruck 13 saverestore-support-checkfail never pass
 test-armhf-armhf-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:
 qemuu5563168c530e2cde8e000ee7aa4afc0ea4d0b42e
baseline version:
 qemuu91d35509903464c7f4b9ed56be223d7370d3597c

Last test of basis96732  2016-07-06 18:12:54 Z1 days
Testing same since96765  2016-07-07 10:44:12 Z0 days1 attempts


People who touched revisions under test:
  Jason Wang 
  Peter Maydell 

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

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

[Tamas K Lengyel]

On Wed, Jul 6, 2016 at 9:54 AM, Corneliu ZUZU  wrote:
> 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 

Acked-by: Tamas K Lengyel 

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

Re: [Xen-devel] [PATCH v3 07/17] libxl/arm: Construct ACPI RSDP table

[Boris Ostrovsky]

On 07/04/2016 11:12 PM, Shannon Zhao wrote:
> From: Shannon Zhao 
>
> Construct ACPI RSDP table and add a helper to calculate the ACPI table
> checksum.
>
> Signed-off-by: Shannon Zhao 
> ---
>  tools/libxl/libxl_arm_acpi.c | 35 +++
>  1 file changed, 35 insertions(+)
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index 7a59126..9df1573 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -33,6 +33,9 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>  
> +#define ACPI_BUILD_APPNAME6 "XenARM"
> +#define ACPI_BUILD_APPNAME4 "Xen "
> +
>  enum {
>  RSDP,
>  XSDT,
> @@ -109,6 +112,36 @@ static int libxl__estimate_acpi_size(libxl__gc *gc,
>  return 0;
>  }
>  
> +static void calculate_checksum(void *table, uint32_t checksum_offset,
> +   uint32_t length)
> +{
> +uint8_t *p, sum = 0;
> +
> +p = table;
> +p[checksum_offset] = 0;
> +
> +while ( length-- )
> +sum = sum + *p++;
> +
> +p = table;
> +p[checksum_offset] = -sum;
> +}
> +
> +static void make_acpi_rsdp(libxl__gc *gc, struct xc_dom_image *dom,
> +   struct acpitable acpitables[])
> +{
> +uint64_t offset = acpitables[RSDP].addr - GUEST_ACPI_BASE;
> +struct acpi_table_rsdp *rsdp = dom->acpitable_blob + offset;
> +
> +memcpy(rsdp->signature, "RSD PTR ", sizeof(rsdp->signature));
> +memcpy(rsdp->oem_id, ACPI_BUILD_APPNAME6, sizeof(rsdp->oem_id));
> +rsdp->length = acpitables[RSDP].size;
> +rsdp->revision = 0x02;
> +rsdp->xsdt_physical_address = acpitables[XSDT].addr;
> +calculate_checksum(rsdp, offsetof(struct acpi_table_rsdp, checksum),
> +   acpitables[RSDP].size);

Should this be extended_checksum? checksum is for ACPI v1 (and thus for
smaller size structure, with only RSDT pointer).

-boris


> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>  libxl__domain_build_state *state,
>  struct xc_dom_image *dom)
> @@ -134,6 +167,8 @@ int libxl__prepare_acpi(libxl__gc *gc, 
> libxl_domain_build_info *info,
>  if (rc)
>  return rc;
>  
> +make_acpi_rsdp(gc, dom, acpitables);
> +
>  return 0;
>  }
>  




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

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

[osstest service owner]

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

Regressions :-(

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

version targeted for testing:
 ovmf 1c03582b070a8feb0b43539cd918a37e3bd5b8da
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   43 days   96 attempts
Testing same since96762  2016-07-07 10:03:49 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 
  Ni, Ruiyu 
  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 8479 lines long.)

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

[Xen-devel] [seabios test] 96767: tolerable FAIL - PUSHED

[osstest service owner]

flight 96767 seabios real [real]
http://logs.test-lab.xenproject.org/osstest/logs/96767/

Failures :-/ but no regressions.

Regressions which are regarded as allowable (not blocking):
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 96501

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-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-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass

version targeted for testing:
 seabios  13213a252286372efa5f72b4119faafd5dff5db1
baseline version:
 seabios  20f83d5c7c0f9ae5f775b6701c205349abe003fb

Last test of basis96501  2016-07-01 12:13:20 Z6 days
Testing same since96767  2016-07-07 15:43:40 Z0 days1 attempts


People who touched revisions under test:
  Kevin O'Connor 

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-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmpass
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm pass
 test-amd64-amd64-qemuu-nested-amdfail
 test-amd64-i386-qemuu-rhel6hvm-amd   pass
 test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
 test-amd64-i386-xl-qemuu-debianhvm-amd64 pass
 test-amd64-amd64-xl-qemuu-win7-amd64 fail
 test-amd64-i386-xl-qemuu-win7-amd64  pass
 test-amd64-amd64-qemuu-nested-intel  pass
 test-amd64-i386-qemuu-rhel6hvm-intel pass
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 pass
 test-amd64-amd64-xl-qemuu-winxpsp3   pass
 test-amd64-i386-xl-qemuu-winxpsp3pass



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=seabios
+ revision=13213a252286372efa5f72b4119faafd5dff5db1
+ . ./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 seabios 
13213a252286372efa5f72b4119faafd5dff5db1
+ branch=seabios
+ revision=13213a252286372efa5f72b4119faafd5dff5db1
+ . ./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=seabios
+ xenbranch=xen-unstable
+ '[' xseabios = xlinux ']'
+ linuxbranch=
+ '[' x = x ']'
+ qemuubranch=qemu-upstream-unstable
+ select_prevxenbranch
++ ./cri-getprevxenbranch xen-unstable
+ prevxenbranch=xen-4.7-testing
+ '[' x13213a252286372efa5f72b4119faafd5dff5db1 = x ']'
+ : tested/2.6.39.x
+ . ./ap-common

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

[osstest service owner]

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

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  730bdfa418fc8c809695ff5d96bc6f7a3b8827ba
baseline version:
 xen  16b69afbba6b8692c96ec5a6864c35fa2fac1d36

Last test of basis96766  2016-07-07 13:02:33 Z0 days
Testing same since96770  2016-07-07 19:00:59 Z0 days1 attempts


People who touched revisions under test:
  Julien Grall 

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=730bdfa418fc8c809695ff5d96bc6f7a3b8827ba
+ . ./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 
730bdfa418fc8c809695ff5d96bc6f7a3b8827ba
+ branch=xen-unstable-smoke
+ revision=730bdfa418fc8c809695ff5d96bc6f7a3b8827ba
+ . ./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
+ '[' x730bdfa418fc8c809695ff5d96bc6f7a3b8827ba = x ']'
+ : tested/2.6.39.x
+ . ./ap-common
++ : osst...@xenbits.xen.org
+++ getconfig OsstestUpstream
+++ perl -e '
use Osstest;
readglobalconfig();
print $c{"OsstestUpstream"} or die $!;
'
++ :
++ : git://xenbits.xen.org/xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/xen.git
++ : git://xenbits.xen.org/qemu-xen-traditional.git
++ : git://git.kernel.org
++ : git://git.kernel.org/pub/scm/linux/kernel/git
++ : git
++ : git://xenbits.xen.org/libvirt.git
++ : osst...@xenbits.xen.org:/home/xen/git/libvirt.git
++ : git://xenbits.xen.org/libvirt.git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : git
++ : git://xenbits.xen.org/rumpuser-xen.git
++ : osst...@xenbits.xen.org:/home/xen/git/rumpuser-xen.git
+++ besteffort_repo https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ cached_repo https://github.com/rumpkernel/rumpkernel-netbsd-src 
'[fetch=try]'
+++ local repo=https://github.com/rumpkernel/rumpkernel-netbsd-src
+++ local 'options=[fetch=try]'
 getconfig GitCacheProxy
 perl -e '
use Osstest;
readglobalconfig();
print $c{"GitCacheProxy"} or die $!;
'
+++ local cache=git://cache:9419/
+++ '[' xgit://cache:9419/ '!=' x ']'
+++ echo 

Re: [Xen-devel] [PATCH v2] xen/arm64: Use the correct TLBs flush instruction to nuke stage-2 TLBs

[Stefano Stabellini]

On Thu, 7 Jul 2016, Julien Grall wrote:
> The function flush_tlb is called to invalidate the TLBs for the current
> domain when the stage-2 page tables are modified.
> 
> On ARMv8, the instruction "tlbi vmalle1is" (resp. "tlbi vmalle1") will
> invalidate stage 1 entries associated to the current VMID (see D4-1811 in
> ARM DDI 0487A.j).
> 
> Given that an implementation is allowed to cache separately stage 1 and
> stage 2 translation (see D4.7.1), the instructions will not remove stage
> 2 entries when the translation is not combined in a single entry.
> This will result the TLBs to hold invalid entries and possibly multiple
> entries using the same VA.
> 
> Use "tlbi vmalls12e1is" (resp. "tlbi vmalls12e1"), to flush both stage
> 1 and 2 entries when the domain p2m is changed.
> 
> Also modify flush_tlb_local to invalidate stage 1 and 2 for the local
> TLBs. Note that this function is used in the instruction abort path
> before translating a GVA to a IPA. As far as I understand is to avoid a
> guest poisoning the DTLB when memacces is in use. We might be able to
> only invalidate stage 1 entries. However, I choose the safest way for now
> (i.e invalidating stage 1 and 2 entries). We would need to introduce a
> new set of helpers when we will want to restrict it.
> 
> Signed-off-by: Julien Grall 

Thanks.

Reviewed-by: Stefano Stabellini 

I'll commit it shortly.

> --
> This would need to be backported on any version of Xen currently
> supported (IIRC think up to Xen 4.5).
> 
> Without this patch, stage 2 TLBs won't be flushed if the TLBs
> cache intermediate translations entries (IPA -> PA).
> 
> Changes in v2:
> - Fix instructions name in the commit message
> ---
>  xen/include/asm-arm/arm64/flushtlb.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/arm64/flushtlb.h 
> b/xen/include/asm-arm/arm64/flushtlb.h
> index a73df92..942f2d3 100644
> --- a/xen/include/asm-arm/arm64/flushtlb.h
> +++ b/xen/include/asm-arm/arm64/flushtlb.h
> @@ -6,7 +6,7 @@ static inline void flush_tlb_local(void)
>  {
>  asm volatile(
>  "dsb sy;"
> -"tlbi vmalle1;"
> +"tlbi vmalls12e1;"
>  "dsb sy;"
>  "isb;"
>  : : : "memory");
> @@ -17,7 +17,7 @@ static inline void flush_tlb(void)
>  {
>  asm volatile(
>  "dsb sy;"
> -"tlbi vmalle1is;"
> +"tlbi vmalls12e1is;"
>  "dsb sy;"
>  "isb;"
>  : : : "memory");
> -- 
> 1.9.1
> 

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

[Xen-devel] [PATCH v2] xen/arm64: Use the correct TLBs flush instruction to nuke stage-2 TLBs

[Julien Grall]

The function flush_tlb is called to invalidate the TLBs for the current
domain when the stage-2 page tables are modified.

On ARMv8, the instruction "tlbi vmalle1is" (resp. "tlbi vmalle1") will
invalidate stage 1 entries associated to the current VMID (see D4-1811 in
ARM DDI 0487A.j).

Given that an implementation is allowed to cache separately stage 1 and
stage 2 translation (see D4.7.1), the instructions will not remove stage
2 entries when the translation is not combined in a single entry.
This will result the TLBs to hold invalid entries and possibly multiple
entries using the same VA.

Use "tlbi vmalls12e1is" (resp. "tlbi vmalls12e1"), to flush both stage
1 and 2 entries when the domain p2m is changed.

Also modify flush_tlb_local to invalidate stage 1 and 2 for the local
TLBs. Note that this function is used in the instruction abort path
before translating a GVA to a IPA. As far as I understand is to avoid a
guest poisoning the DTLB when memacces is in use. We might be able to
only invalidate stage 1 entries. However, I choose the safest way for now
(i.e invalidating stage 1 and 2 entries). We would need to introduce a
new set of helpers when we will want to restrict it.

Signed-off-by: Julien Grall 

--
This would need to be backported on any version of Xen currently
supported (IIRC think up to Xen 4.5).

Without this patch, stage 2 TLBs won't be flushed if the TLBs
cache intermediate translations entries (IPA -> PA).

Changes in v2:
- Fix instructions name in the commit message
---
 xen/include/asm-arm/arm64/flushtlb.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/include/asm-arm/arm64/flushtlb.h 
b/xen/include/asm-arm/arm64/flushtlb.h
index a73df92..942f2d3 100644
--- a/xen/include/asm-arm/arm64/flushtlb.h
+++ b/xen/include/asm-arm/arm64/flushtlb.h
@@ -6,7 +6,7 @@ static inline void flush_tlb_local(void)
 {
 asm volatile(
 "dsb sy;"
-"tlbi vmalle1;"
+"tlbi vmalls12e1;"
 "dsb sy;"
 "isb;"
 : : : "memory");
@@ -17,7 +17,7 @@ static inline void flush_tlb(void)
 {
 asm volatile(
 "dsb sy;"
-"tlbi vmalle1is;"
+"tlbi vmalls12e1is;"
 "dsb sy;"
 "isb;"
 : : : "memory");
-- 
1.9.1


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

[Xen-devel] [OSSTEST PATCH 1/2] Executive: Support substeps

[Ian Jackson]

ts-* scripts can now create `substeps'.  For the purposes of
archaeology etc., a substep is just like a step.  But it does
correspond to a single specific ts-* invocation.

Instead, it is started and finished explicitly as required.

The whole job implementation code needs to explicitly assign a unique
stable testid to each substep.

The `script' parameter is stored in the `step' field in the database,
which is used only for reporting.  These do not need to be unique.

All substeps started are should also be finished, by the end of the
job.  If this is not done, the job will be regarded as broken (if it
is not already failed or aborted).  (But a substep might be finished
by a different ts-* script to the one that started it.)

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 Osstest/JobDB/Executive.pm  | 51 +
 Osstest/JobDB/Standalone.pm | 10 +
 Osstest/TestSupport.pm  | 11 ++
 sg-run-job  |  6 ++
 tcl/JobDB-Executive.tcl | 32 
 tcl/JobDB-Standalone.tcl|  2 ++
 6 files changed, 112 insertions(+)

diff --git a/Osstest/JobDB/Executive.pm b/Osstest/JobDB/Executive.pm
index c0af21c..fdcb177 100644
--- a/Osstest/JobDB/Executive.pm
+++ b/Osstest/JobDB/Executive.pm
@@ -161,6 +161,57 @@ END
 logm("starting $flight started=$now") if $count>0;
 }
 
+sub step_start ($$) { #method
+my ($jd,$testid,$script) = @_;
+my $snq = $dbh_tests->prepare(<prepare(<execute($flight,$job);
+   ($stepno) = $snq->fetchrow_array();
+   $stepno //= 0;
+   $stepno++;
+   $createq->execute($flight,$job,$stepno, $script, $testid,time);
+});
+logm("-- substep $stepno $testid running --");
+}
+
+sub step_finish ($$) { #method
+my ($jd,$testid,$stepstatus) = @_;
+die "$flight.$job $testid $stepstatus" unless grep { $stepstatus eq $_ }
+   qw(pass fail blocked broken);
+my $checkq = $dbh_tests->prepare(<prepare(<execute($flight,$job,$testid);
+   ($stepno, $oldstatus) = $checkq->fetchrow_array();
+   die "$flight.$job $testid $stepno $oldstatus ?"
+   unless $oldstatus eq 'running';
+   $setq->execute($stepstatus,time, $flight,$job,$testid, $stepno);
+});
+logm("-- substep $stepno $testid $stepstatus --");
+}
+
 sub host_check_allocated ($$) { #method
 my ($jd, $ho) = @_;
 
diff --git a/Osstest/JobDB/Standalone.pm b/Osstest/JobDB/Standalone.pm
index 98d0173..21ea07b 100644
--- a/Osstest/JobDB/Standalone.pm
+++ b/Osstest/JobDB/Standalone.pm
@@ -87,6 +87,16 @@ sub current_flight ($) {
 
 sub job_ensure_started ($) { }
 
+sub step_start ($$) {
+my ($jd,$testid,$script) = @_;
+logm("== $flight.$job step $testid running $script ==");
+}
+
+sub step_finish ($$) { #method
+my ($jd,$testid,$stepstatus) = @_;
+logm("== $flight.$job step $testid $stepstatus ==");
+}
+
 sub host_check_allocated ($$) { #method
 my ($jd, $ho) = @_;
 
diff --git a/Osstest/TestSupport.pm b/Osstest/TestSupport.pm
index 9e6479a..76c3a57 100644
--- a/Osstest/TestSupport.pm
+++ b/Osstest/TestSupport.pm
@@ -42,6 +42,7 @@ BEGIN {
   ts_get_host_guest
 
   fail broken logm $logm_handle $logm_prefix
+  substep_start substep_finish
   get_filecontents
   report_once
 
@@ -228,6 +229,16 @@ END
  : "($flight.$job not marked $newst)");
 }
 
+sub substep_start ($$) {
+my ($testid,$script) = @_;
+$mjobdb->step_start($testid,$script);
+}
+
+sub substep_finish ($$) {
+my ($testid,$stepstatus) = @_;
+$mjobdb->step_finish($testid,$stepstatus);
+}
+
 sub get_filecontents ($;$) {
 my ($path, $ifnoent) = @_;  # $ifnoent=undef => is error
 my $data= get_filecontents_core_quiet($path);
diff --git a/sg-run-job b/sg-run-job
index 0847d2f..60855b4 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -88,6 +88,12 @@ proc run-job {job} {
run-ts  !broken capture-logs  ts-logs-capture + host
 }
 
+if {$ok} {
+if 

[Xen-devel] [OSSTEST PATCH 2/2] DO NOT APPLY make-flight-substep-test

[Ian Jackson]

This is a simple test case for substeps.

Signed-off-by: Ian Jackson 
CC: Wei Liu 
---
 make-flight-substep-test | 14 ++
 sg-run-job   |  7 +++
 2 files changed, 21 insertions(+)
 create mode 100755 make-flight-substep-test

diff --git a/make-flight-substep-test b/make-flight-substep-test
new file mode 100755
index 000..44b0451
--- /dev/null
+++ b/make-flight-substep-test
@@ -0,0 +1,14 @@
+#!/bin/sh
+set -e
+
+flight=`./cs-flight-create play xen-unstable`
+
+runvars=bogus=0
+
+for j in test-1 test-2; do
+./cs-job-create $flight $j test-substeps \
+   $runvars
+runvars=bogus=1
+done
+
+echo $flight
diff --git a/sg-run-job b/sg-run-job
index 60855b4..05e7725 100755
--- a/sg-run-job
+++ b/sg-run-job
@@ -423,6 +423,13 @@ proc run-job/test-rumpuserxen {} {
  ts-guest-destroy-hardhost   $g   +
 }
 
+proc need-hosts/test-substeps {} { }
+proc run-job/test-substeps {} {
+run-ts . =   ts-substep-test suba:foo 1 subb:wombat suba=pass
+run-ts . =.2 ts-substep-test subx:foo 1 subx=fail
+run-ts . =.3 ts-substep-test ,bogus subb=pass
+}
+
 if {[file exists sg-run-job-adhoc]} {
 source sg-run-job-adhoc
 }
-- 
2.1.4


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

Re: [Xen-devel] [PATCH] xen/arm64: Use the correct TLBs flush instruction to nuke stage-2 TLBs

[Stefano Stabellini]

On Thu, 7 Jul 2016, Julien Grall wrote:
> The function flush_tlb is called to invalidate the TLBs for the current
> domain when the stage-2 page tables are modified.
> 
> On ARMv8, the instruction "tlbi vmalle1is" (resp. "tlbi vmalle1") will
> invalidate stage 1 entries associated to the current VMID (see D4-1811 in
> ARM DDI 0487A.j).
> 
> Given that an implementation is allowed to cache separately stage 1 and
> stage 2 translation (see D4.7.1), the instructions will not remove stage
> 2 entries when the translation is not combined in a single entry.
> This will result the TLBs to hold invalid entries and possibly multiple
> entries using the same VA.
> 
> Use "tlbi vmall12e1is" (resp. "tlbi vmall12e1"), to flush both stage 1
^ vmalls12e1is  ^ vmalls12e1

Please fix the commit message using the right commands.


> and stage 2 entries when the domain p2m is changed.
> 
> Also modify flush_tlb_local to invalidate stage 1 and 2 for the local
> TLBs. Note that this function is used in the instruction abort path
> before translating a GVA to a IPA. As far as I understand is to avoid a
> guest poisoning the DTLB when memacces is in use. We might be able to
> only invalidate stage 1 entries. However, I choose the safest way for now
> (i.e invalidating stage 1 and 2 entries). We would need to introduce a
> new set of helpers when we will want to restrict it.
> 
> Signed-off-by: Julien Grall 
> 
> --
> This would need to be backported on any version of Xen currently
> supported (IIRC think up to Xen 4.5).
> 
> Without this patch, stage 2 TLBs won't be flushed if the TLBs
> cache intermediate translations entries (IPA -> PA).
> ---
>  xen/include/asm-arm/arm64/flushtlb.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/include/asm-arm/arm64/flushtlb.h 
> b/xen/include/asm-arm/arm64/flushtlb.h
> index a73df92..942f2d3 100644
> --- a/xen/include/asm-arm/arm64/flushtlb.h
> +++ b/xen/include/asm-arm/arm64/flushtlb.h
> @@ -6,7 +6,7 @@ static inline void flush_tlb_local(void)
>  {
>  asm volatile(
>  "dsb sy;"
> -"tlbi vmalle1;"
> +"tlbi vmalls12e1;"
>  "dsb sy;"
>  "isb;"
>  : : : "memory");
> @@ -17,7 +17,7 @@ static inline void flush_tlb(void)
>  {
>  asm volatile(
>  "dsb sy;"
> -"tlbi vmalle1is;"
> +"tlbi vmalls12e1is;"
>  "dsb sy;"
>  "isb;"
>  : : : "memory");
> -- 
> 1.9.1
> 

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

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

[osstest service owner]

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

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-libvirt-xsm  6 xen-boot   fail in 96739 pass in 96756
 test-amd64-amd64-xl-rtds  6 xen-boot   fail in 96739 pass in 96756
 test-armhf-armhf-xl-arndale  15 guest-start/debian.repeat   fail pass in 96739

Regressions which are regarded as allowable (not blocking):
 test-armhf-armhf-xl-rtds 15 guest-start/debian.repeat fail REGR. vs. 96712
 build-amd64-rumpuserxen   6 xen-buildfail   like 96712
 build-i386-rumpuserxen6 xen-buildfail   like 96712
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 96712
 test-amd64-i386-xl-qemut-win7-amd64 16 guest-stop  fail like 96712
 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-stop fail like 96712
 test-amd64-amd64-xl-rtds  9 debian-install   fail   like 96712
 test-amd64-amd64-xl-qemuu-win7-amd64 16 guest-stop fail like 96712

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-i386-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-amd64-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-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-amd64-amd64-libvirt-vhd 11 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-multivcpu 13 saverestore-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 12 migrate-support-checkfail  never pass
 test-armhf-armhf-xl  12 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  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-raw 13 guest-saverestorefail   never pass
 test-armhf-armhf-libvirt-raw 11 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 11 migrate-support-checkfail never pass
 test-armhf-armhf-libvirt-qcow2 13 guest-saverestorefail never pass
 test-armhf-armhf-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-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  db59dbc46e0312a38978d22c6bd72b554a2f1c91
baseline version:
 xen  fea586d801f75317cb8cf593e8beba842391da62

Last test of basis96712  2016-07-06 10:02:51 Z1 days
Testing same since96739  2016-07-06 22:16:30 Z0 days2 attempts


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

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

Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code

[Boris Ostrovsky]

On 07/07/2016 01:15 PM, Wei Liu wrote:
> On Thu, Jul 07, 2016 at 01:09:28PM -0400, Boris Ostrovsky wrote:
>> On 07/07/2016 12:58 PM, Ian Jackson wrote:
>>> There are two serious problems with this.
>>>
>>> 1. You have dropped the copyright attribution to Intel and Xensource.
>>>
>>> 2. You have changed the licence to BSD-style, even though the original
>>>was GPLv2-only.
>>
>> I meant this to be a GPLv2 license. And I made the same mistake in a
>> couple of other files.
>>
> The other question is, will this GPLv2 file be linked against other
> toolstack components (libxl is LGPL)? If so, how should we deal with
> different licences?

Two new libxl files will be LGPL and but libacpi files will be GPLv2
(because most of the files there are already GPLv2, I just added a
couple of new ones).

Which would mean that a non-GPL users cannot link against libxl anymore,
right?

-boris

>
> Wei.
>
>> -boris
>>
>>
>>>
>>> Please be more careful!
>>>
>>> Thanks,
>>> Ian.
>>



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

On 07/05/2016 05:37 PM, Julien Grall wrote:

When altp2m is active, the hostp2m is not used. All changes are applied
directly to the individual altp2m views. As far as I know, the only
situations, where a mapping is removed from a specific p2m view is in
the functions unmap_regions_rw_cache, unmap_mmio_regions, and
guest_physmap_remove_page. Each one of these functions provide, however
the hostp2m to the function apply_p2m_changes, where the mapping is
eventually removed. So, we definitely remove mappings from the hostp2m.
However, these would not be removed from the associated altp2m views.

Can you state a scenario, when and why pages might need to be removed at
run time from the hostp2m? In that case, maybe it would make sense to
extend the three functions (unmap_regions_rw_cache, unmap_mmio_regions,
and guest_physmap_remove_page) to additionally remove the mappings from
all available altp2m views?


All the functions, you mentioned can be called after the domain has been
created. If you grep guest_physmap_remove_page in the code source, you
will find that it is used to unmap grant entry from the memory (see
replace_grant_host_mapping) or when decreasing the memory reservation
(see do_memory_op).

Note that, from my understanding, x86 has the same problem.



Yes, the x86 implementation works similarly to the one for ARM. I will
need to think about that. One solution could simply extend the
previously mentioned functions by unmapping the particular entry in all
currently available altp2m views (assuming, we allocate the altp2m views
on demand, as discussed in patch #05).

Whatever the solution will be, it will need to be ported to the x86
implementation as well.


Actually, the x86 code is propagating the change (see 
p2m_altp2m_propagate_change). I missed it while going the code the first 
time.


Cheers,

--
Julien Grall

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

[Lars Kurth]

Alright,

it appears we are at an impasse here. Not hosting the code on xenbits as
suggested by David, seems to be the worst solution and will benefit
no-one. 

> If we can't get consensus on something like this, the sensible thing
> to do would be to vote. Our governance docs don't really cope with
> this kind of multi-answer question; they only do yes/no.

I am not convinced that we need a formal process in this case. Our
governance has the mechanism to referee, when there is disagreement. For
code changes the referee would be the maintainer/committer which owns a
piece of code and the mechanism would work by withholding an ACK. For
unowned changes the referee would be the project lead: but we have none
and in fact we want none.

The next level up is the Advisory Board: but I really don't want to go to
the AB with a bike-shed issue like this.


In particular as WE DO ACTUALLY HAVE CONSENSUS for a compromise by the two
main people disagreeing.

> On 20/06/16 18:03, Ian Jackson wrote:

> I could live with "xtf", although I think it's rather too short.



> On 07/07/2016 12:26, "Andrew Cooper"  wrote:
>> On 07/07/16 12:10, Lars Kurth wrote:
>> @Andrew: would something like test/xtf.git work

> It would, although given a straight choice I would prefer
> xen-test-framework.git over its abbreviation.


So let's just go with "./xtf.git" and make use of the "Description" field
in http://xenbits.xen.org/gitweb/ to add a bit more verbosity. Adding
something such as "Xen Test Framework and Suite for creating
microkernel-based tests". This is accurate and searchable.

It is no worse than "raisin.git", "osstest.git", and other top-level repos.

Maybe we can make improve the description for "./osstest.git": something
along the lines of "Xen Test Framework and Suite, used for Open Source Xen
Continuous Integration that also acts as push gate" or something like it.
That would be more accurate than what we have now.

Compromise
A.1) Create "xtf.git" and use "Xen Test Framework and Suite for creating
microkernel-based tests" in Description field

A.2) Update description for osstest.git to "Xen Test Framework and Suite,
used for Open Source Xen Continuous Integration that also acts as push
gate"

> Out of curiosity, I searched for it on google, and found my written
> documentation as the top hit.
>
> 
>https://www.google.co.uk/search?q=xen+test+framework=utf-8=utf-8
>_rd=cr=InxeV52HDYHc-QG0-qN4

That has nothing to do with the repo name. The reason why google finds
this page is because
http://xenbits.xen.org/people/andrewcoop/xen-test-framework/ exists, but
no equivalent page exists for OSSTEST.


Improvements to web searchability for "xen test framework" to ensure that
searches for both frameworks lead somewhere sensible
B.1) http://xenbits.xen.org/people/andrewcoop/xen-test-framework/ should
be move under docs and re-named to "XTF: Xen Test Framework and Suite for
creating microkernel-based tests"

B.2) Add a similar page under docs for OSSTEST with a similarly verbose
title, e.g. "OSSTEST: Xen Test Framework and Suite for Open Source Xen
Continuous Integration"

That should address everyones concern, as far as I can tell from the the
e-mail thread. If anyone disagrees, please shout within the next few days.

Best Regards
Lars
P.S.: I moved fixing some of our governance issues towards the top of my
TODO list


On 07/07/2016 15:11, "Andrew Cooper"  wrote:

>On 07/07/16 14:59, Ian Jackson wrote:
>> Andrew Cooper writes ("Re: [Xen-devel] xenbits "official" repo for XTF
>>(was Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)"):
>>> On 07/07/16 12:10, Lars Kurth wrote:
 @Andrew: would something like test/xtf.git work
>> I would live with that.
>>
>>> It would, although given a straight choice I would prefer
>>> xen-test-framework.git over its abbreviation.
>> This conversation is in danger of going round in circles.
>
>Right - let me draw a line in the sand.
>
>The current name is xen-test-framework, and that has been around and
>used for several years now.  XTF is also used frequently as an
>abbreviation.
>
>A concern has been raised about the correctness of this name, but no
>alternatives have been suggested which come close to being a plausible
>replacement.  (All suggestions are less accurate or descriptive than the
>current name).
>
>As the author, I chose the current name, and chose it as the most
>appropriate name I could think of.  If others feel strongly that it
>isn't, then the onus is on them to choose a specific alternative and
>argue as to why it is more appropriate.  Nothing along this line has
>occurred.
>
>~Andrew

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

Re: [Xen-devel] [PATCH v1 19/20] libxl/pvhv2: Include APIC page in MMIO hole for PVHv2 guests

[Wei Liu]

On Thu, Jul 07, 2016 at 01:02:12PM -0400, Boris Ostrovsky wrote:
> On 07/07/2016 12:47 PM, Wei Liu wrote:
> > On Tue, Jul 05, 2016 at 03:05:18PM -0400, Boris Ostrovsky wrote:
> >>  
> >> @@ -1006,10 +1009,21 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
> >>  dom->target_pages = mem_size >> XC_PAGE_SHIFT;
> >>  if (dom->mmio_size == 0 && device_model)
> >>  dom->mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
> >> -else if (dom->mmio_size == 0 && !device_model)
> >> -dom->mmio_size = GB(4) -
> >> -((X86_HVM_END_SPECIAL_REGION - 
> >> X86_HVM_NR_SPECIAL_PAGES)
> >> -<< XC_PAGE_SHIFT);
> >> +else if (dom->mmio_size == 0 && !device_model) {
> >> +#if defined(__i386__) || defined(__x86_64__)
> >> +if (libxl_defbool_val(info->u.hvm.apic)) {
> >> +/* Make sure LAPIC_BASE_ADDRESS is below special pages */
> >> +assert(X86_HVM_END_SPECIAL_REGION - 
> >> X86_HVM_NR_SPECIAL_PAGES)
> >> +  << XC_PAGE_SHIFT) - LAPIC_BASE_ADDRESS)) >= 
> >> XC_PAGE_SIZE);
> >> +dom->mmio_size = GB(4) - LAPIC_BASE_ADDRESS;
> >> +} else
> >> +dom->mmio_size = GB(4) -
> >> +((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
> >> + << XC_PAGE_SHIFT);
> >> +#else
> >> +assert(1);
> > This looks a bit odd. Do you want to avoid "if {}" (nothing in braces)?
> >
> > If this branch doesn't nothing on ARM, maybe you can just do
> >
> > #if defined(x86)
> >  else if () {
> >  }
> > #endif
> >
> > ?
> 
> Sure, I could do that. I was trying to flag a case on ARM when we show
> up here without device model. Which (the flagging) we haven't done
> before so perhaps it's not needed.
> 

Ah, I'm fine with the original code then -- if ARM change is
anticipated.

Wei.

> -boris
> 
> >   
> >> +#endif
> >> +}
> >>  lowmem_end = mem_size;
> >>  highmem_end = 0;
> >>  mmio_start = (1ull << 32) - dom->mmio_size;
> >> -- 
> >> 1.7.1
> >>
> 

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

Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code

[Wei Liu]

On Thu, Jul 07, 2016 at 01:09:28PM -0400, Boris Ostrovsky wrote:
> On 07/07/2016 12:58 PM, Ian Jackson wrote:
> >
> > There are two serious problems with this.
> >
> > 1. You have dropped the copyright attribution to Intel and Xensource.
> >
> > 2. You have changed the licence to BSD-style, even though the original
> >was GPLv2-only.
> 
> 
> I meant this to be a GPLv2 license. And I made the same mistake in a
> couple of other files.
> 

The other question is, will this GPLv2 file be linked against other
toolstack components (libxl is LGPL)? If so, how should we deal with
different licences?

Wei.

> -boris
> 
> 
> >
> >
> > Please be more careful!
> >
> > Thanks,
> > Ian.
> 
> 

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

[Xen-devel] [xen-unstable baseline-only test] 66529: trouble: blocked/broken/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

flight 66529 xen-unstable running [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/66529/

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-rumpuserxen-amd64  broken
 build-amd64-libvirt   broken
 test-amd64-amd64-xl-pvh-amd   broken
 test-amd64-amd64-xl-pvh-intel  broken
 build-amd64-rumpuserxen   broken
 test-amd64-amd64-xl-credit2   broken
 test-amd64-amd64-migrupgrade  broken
 test-amd64-amd64-xl-multivcpu  broken
 test-amd64-amd64-libvirt-xsm  broken
 test-amd64-amd64-xl-xsm   broken
 test-amd64-amd64-libvirt  broken
 test-amd64-amd64-pygrub   broken
 test-amd64-amd64-xl-rtds  broken
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsmbroken
 test-amd64-amd64-xl   broken
 test-amd64-amd64-qemuu-nested-amd  broken
 test-amd64-amd64-amd64-pvgrub  broken
 test-amd64-amd64-xl-qemut-debianhvm-amd64broken
 test-amd64-amd64-i386-pvgrub  broken
 test-amd64-amd64-libvirt-pair  broken
 test-amd64-amd64-xl-qcow2 broken
 test-amd64-amd64-xl-qemut-win7-amd64  broken
 test-amd64-amd64-xl-qemuu-win7-amd64  broken
 test-amd64-amd64-pair broken
 test-amd64-amd64-xl-qemuu-ovmf-amd64  broken
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   broken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmbroken
 test-amd64-amd64-qemuu-nested-intel  broken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64broken
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsmbroken
 test-amd64-amd64-libvirt-vhd  broken
 test-amd64-amd64-xl-qemuu-winxpsp3  broken
 test-amd64-amd64-xl-qemut-winxpsp3  broken
 build-armhf   2 hosts-allocate   running
 build-armhf-xsm   2 hosts-allocate   running
 build-armhf-pvops 2 hosts-allocate   running
 build-i386-pvops  2 hosts-allocate   running
 build-i3862 hosts-allocate   running

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-rumpuserxen-i386  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-midway1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-raw  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 build-i386-libvirt1 build-check(1)   blocked  n/a
 build-i386-rumpuserxen1 build-check(1)   blocked  n/a
 test-amd64-i386-qemut-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-xsm   1 build-check(1)   blocked  n/a
 test-amd64-i386-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-libvirt   1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-xsm1 build-check(1)   blocked  n/a
 test-amd64-i386-qemut-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-qemuu-rhel6hvm-amd  1 build-check(1)   blocked n/a
 test-amd64-i386-freebsd10-i386  1 build-check(1)   blocked  n/a
 test-amd64-i386-freebsd10-amd64  1 build-check(1)   blocked  n/a
 test-amd64-i386-qemuu-rhel6hvm-intel  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-pair  1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64  1 build-check(1) blocked n/a
 test-amd64-i386-xl-raw1 build-check(1)   blocked  n/a
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm  1 build-check(1) blocked n/a
 test-amd64-i386-xl-qemuu-win7-amd64  

[Xen-devel] [qemu-mainline baseline-only test] 66530: trouble: blocked/broken/pass

[Platform Team regression test user]

This run is configured for baseline tests only.

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

Failures and problems with tests :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-amd64-libvirt   broken
 build-i386-libvirtbroken
 test-amd64-amd64-xl-pvh-intel  broken
 test-amd64-amd64-xl-pvh-amd   broken
 test-amd64-amd64-xl   broken
 test-amd64-i386-qemuu-rhel6hvm-intel  broken
 test-amd64-i386-xlbroken
 test-amd64-amd64-xl-multivcpu  broken
 test-amd64-amd64-xl-credit2   broken
 test-amd64-i386-xl-xsmbroken
 test-amd64-amd64-xl-rtds  broken
 test-amd64-amd64-libvirt  broken
 test-amd64-amd64-libvirt-xsm  broken
 test-amd64-i386-libvirt   broken
 test-amd64-amd64-xl-xsm   broken
 test-amd64-i386-qemuu-rhel6hvm-amd  broken
 test-amd64-i386-libvirt-xsm   broken
 test-amd64-i386-xl-qemuu-ovmf-amd64  broken
 test-amd64-amd64-xl-qemuu-ovmf-amd64  broken
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmbroken
 test-amd64-amd64-qemuu-nested-intel  broken
 test-amd64-i386-freebsd10-i386  broken
 test-amd64-amd64-i386-pvgrub  broken
 test-amd64-i386-freebsd10-amd64  broken
 test-amd64-amd64-qemuu-nested-amd  broken
 test-amd64-amd64-pair broken
 test-amd64-amd64-amd64-pvgrub  broken
 test-amd64-amd64-pygrub   broken
 test-amd64-i386-pair  broken
 test-amd64-amd64-libvirt-pair  broken
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   broken
 test-amd64-amd64-libvirt-vhd  broken
 test-amd64-i386-xl-rawbroken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64broken
 test-amd64-i386-xl-qemuu-winxpsp3  broken
 test-amd64-i386-xl-qemuu-win7-amd64  broken
 test-amd64-amd64-xl-qemuu-win7-amd64  broken
 test-amd64-i386-libvirt-pair  broken
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm broken
 test-amd64-amd64-xl-qcow2 broken
 test-amd64-i386-xl-qemuu-debianhvm-amd64 broken
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsmbroken
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 broken
 test-amd64-amd64-xl-qemuu-winxpsp3  broken
 build-armhf   2 hosts-allocate   running
 build-armhf-xsm   2 hosts-allocate   running
 build-armhf-pvops 2 hosts-allocate   running

Tests which did not succeed, but are not blocking:
 build-armhf-libvirt   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-xsm  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-xl-midway1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-vhd   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-rtds  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   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a

version targeted for testing:
 qemuu91d35509903464c7f4b9ed56be223d7370d3597c
baseline version:
 qemuud6550e9ed2e1a60d889dfb721de00d9a4e3bafbe

Last test of basis66485  2016-07-01 10:47:40 Z6 days
Testing same since0  1970-01-01 00:00:00 Z 16989 days0 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 
  Andreas Färber 
  Andrew Jeffery 
  Andrew Jones 
  Andrey Smirnov 

Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code

[Boris Ostrovsky]

On 07/07/2016 12:58 PM, Ian Jackson wrote:
>
> There are two serious problems with this.
>
> 1. You have dropped the copyright attribution to Intel and Xensource.
>
> 2. You have changed the licence to BSD-style, even though the original
>was GPLv2-only.


I meant this to be a GPLv2 license. And I made the same mistake in a
couple of other files.

-boris


>
>
> Please be more careful!
>
> Thanks,
> Ian.



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

Re: [Xen-devel] [PATCH v1 19/20] libxl/pvhv2: Include APIC page in MMIO hole for PVHv2 guests

[Boris Ostrovsky]

On 07/07/2016 12:47 PM, Wei Liu wrote:
> On Tue, Jul 05, 2016 at 03:05:18PM -0400, Boris Ostrovsky wrote:
>>  
>> @@ -1006,10 +1009,21 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>>  dom->target_pages = mem_size >> XC_PAGE_SHIFT;
>>  if (dom->mmio_size == 0 && device_model)
>>  dom->mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
>> -else if (dom->mmio_size == 0 && !device_model)
>> -dom->mmio_size = GB(4) -
>> -((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
>> -<< XC_PAGE_SHIFT);
>> +else if (dom->mmio_size == 0 && !device_model) {
>> +#if defined(__i386__) || defined(__x86_64__)
>> +if (libxl_defbool_val(info->u.hvm.apic)) {
>> +/* Make sure LAPIC_BASE_ADDRESS is below special pages */
>> +assert(X86_HVM_END_SPECIAL_REGION - 
>> X86_HVM_NR_SPECIAL_PAGES)
>> +  << XC_PAGE_SHIFT) - LAPIC_BASE_ADDRESS)) >= 
>> XC_PAGE_SIZE);
>> +dom->mmio_size = GB(4) - LAPIC_BASE_ADDRESS;
>> +} else
>> +dom->mmio_size = GB(4) -
>> +((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
>> + << XC_PAGE_SHIFT);
>> +#else
>> +assert(1);
> This looks a bit odd. Do you want to avoid "if {}" (nothing in braces)?
>
> If this branch doesn't nothing on ARM, maybe you can just do
>
> #if defined(x86)
>  else if () {
>  }
> #endif
>
> ?

Sure, I could do that. I was trying to flag a case on ARM when we show
up here without device model. Which (the flagging) we haven't done
before so perhaps it's not needed.

-boris

>   
>> +#endif
>> +}
>>  lowmem_end = mem_size;
>>  highmem_end = 0;
>>  mmio_start = (1ull << 32) - dom->mmio_size;
>> -- 
>> 1.7.1
>>


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

Re: [Xen-devel] [PATCH v1 02/20] acpi/hvmloader: Move acpi_info initialization out of ACPI code

[Ian Jackson]

Boris Ostrovsky writes ("[PATCH v1 02/20] acpi/hvmloader: Move acpi_info 
initialization out of ACPI code"):
> acpi_info can be initialized by hvmloader itself. Now ACPI code
> doesn't need to use hvmloader-private variables/routines such as
> uart_exists(), lpt_exists() etc.
...
> Signed-off-by: Boris Ostrovsky 
...
> * Create libacpi.h for libacpi interface definitions

Nacked-by: Ian Jackson 

I'm afraid this patch contains a licence violation, contrary to your
S-O-B.


You have moved this:

> --- a/tools/firmware/hvmloader/acpi/build.c
> +++ b/tools/firmware/hvmloader/acpi/build.c
...
> -struct acpi_info {
> -uint8_t  com1_present:1;/* 0[0] - System has COM1? */
> -uint8_t  com2_present:1;/* 0[1] - System has COM2? */
> -uint8_t  lpt1_present:1;/* 0[2] - System has LPT1? */


Into a new file:

> --- /dev/null
> +++ b/tools/firmware/hvmloader/acpi/libacpi.h
> @@ -0,0 +1,80 @@
...
> +struct acpi_info {
> +uint8_t  com1_present:1;/* 0[0] - System has COM1? */
> +uint8_t  com2_present:1;/* 0[1] - System has COM2? */
> +uint8_t  lpt1_present:1;/* 0[2] - System has LPT1? */


The original file has this copyright header:

  /*
   * Copyright (c) 2004, Intel Corporation.
   * Copyright (c) 2006, Keir Fraser, XenSource Inc.
   *
   * This program is free software; you can redistribute it and/or
   * modify it under the terms and conditions of the GNU General
   * Public License, version 2, as published by the Free Software
   * Foundation.
   *
   * This program is distributed in the hope it will be useful, but
   * WITHOUT ANY WARRANTY; without even the implied warranty of
   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
   * General Public License for more details.
   *
   * You should have received a copy of the GNU General Public License
   * along with this program; If not, see
   * .
   */

(rewrapped for legibility).  The dates haven't been updated for ages,
but this file is clearly GPLv2-only.


However your new file has this copyright header (again, wrapped);

> +/**
> + * libacpi.h
> + * 
> + * libacpi interfaces
> + * 
> + * Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> + */

There are two serious problems with this.

1. You have dropped the copyright attribution to Intel and Xensource.

2. You have changed the licence to BSD-style, even though the original
   was GPLv2-only.


Please be more careful!

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH v3 17/17] libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ

[Julien Grall]

Hi Wei,

On 07/07/16 17:15, Wei Liu wrote:

On Tue, Jul 05, 2016 at 11:12:47AM +0800, Shannon Zhao wrote:

From: Shannon Zhao 

The guest kernel will get the event channel interrupt information via
domain param HVM_PARAM_CALLBACK_IRQ. Initialize it here.

Signed-off-by: Shannon Zhao 
---
  tools/libxl/libxl_arm.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index bc38318..acacba0 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -900,8 +900,19 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
 struct xc_dom_image *dom)
  {
  int rc;
+uint64_t val;

  assert(info->type == LIBXL_DOMAIN_TYPE_PV);
+
+/* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
+val = (uint64_t)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
+val |= (2 << 8); /* Active-low level-sensitive  */


Please avoid using magic numbers here -- 56, 2 and 8.


The magic numbers are described in public/hvm/params.h however there is 
no defines associated to them.


The public header would need to be updated if we don't want the value 
hardcoded in libxl.




Another question to Julien and Stefano: is it normal for ARM guest to
use hvm callback vector?


Yes. The HVM callback vector is used by ACPI guest to find the PPI 
(per-cpu interrupt) which will be used to notify event.


This is how DOM0 is using ACPI on ARM (see [1]), I don't think we should 
differ here.


BTW, I have noticed that the design doc is only available on the ML.
Shannon, would it be possible to send a patch to add it in docs/misc/arm?

[2] https://lists.xen.org/archives/html/xen-devel/2015-11/msg00488.html

Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH 19/19] xen: credit2: use cpumask_first instead of cpumask_any when choosing cpu

[Dario Faggioli]

On Tue, 2016-06-21 at 11:42 +0100, David Vrabel wrote:
> On 18/06/16 00:13, Dario Faggioli wrote:
> > 
> > because it is cheaper, and there is no much point in
> > randomizing which cpu gets selected anyway, as such
> > choice will be overridden shortly after, in runq_tickle().
> > 
> > If we really feel the need (e.g., we prove it worth with
> > benchmarking), we can record the last cpu which was used
> > by csched2_cpu_pick() and migrate() in a per-runq variable,
> > and then use cpumask_cycle()... but this really does not
> > look necessary.
> Isn't this backwards?  Surely you should demonstrate that this change
> is
> beneficial before proposing it?
> 
Right. I think it's my fault having presented things this way.

This patch get rid of something that is pure overhead, and getting rid
of overhead is, in general, a good thing.

There is only one possible situation under which we may actually end up
favouring lower pCPU IDs, and it is unlikely enough that it is IMO, of
no concern.

But in any case, let's just drop this patch. I'm rerunning the
benchmarks anyway, I'll consider doing a set of runs with and without
this patch, and check if it does make any difference.

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



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

Re: [Xen-devel] [PATCH v1 19/20] libxl/pvhv2: Include APIC page in MMIO hole for PVHv2 guests

[Wei Liu]

On Tue, Jul 05, 2016 at 03:05:18PM -0400, Boris Ostrovsky wrote:
> Signed-off-by: Boris Ostrovsky 
> ---
> 
> New patch
> 
>  tools/libxl/Makefile|2 ++
>  tools/libxl/libxl_dom.c |   22 ++
>  2 files changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 9fee752..3a2d64a 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -165,6 +165,8 @@ $(XL_OBJS) $(TEST_PROG_OBJS) _libxl.api-for-check: \
>  $(XL_OBJS): CFLAGS += $(CFLAGS_XL)
>  $(XL_OBJS): CFLAGS += -include $(XEN_ROOT)/tools/config.h # libxl_json.h 
> needs it.
>  
> +libxl_dom.o: CFLAGS += -I$(XEN_ROOT)/tools  # include libacpi/x86.h
> +
>  SAVE_HELPER_OBJS = libxl_save_helper.o _libxl_save_msgs_helper.o
>  $(SAVE_HELPER_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenevtchn)
>  
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index ccc41b4..ba3472e 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -24,6 +24,9 @@
>  #include 
>  #include 
>  #include 
> +#if defined(__i386__) || defined(__x86_64__)
> +#include 
> +#endif
>  
>  #include "_paths.h"
>  
> @@ -1006,10 +1009,21 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
>  dom->target_pages = mem_size >> XC_PAGE_SHIFT;
>  if (dom->mmio_size == 0 && device_model)
>  dom->mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
> -else if (dom->mmio_size == 0 && !device_model)
> -dom->mmio_size = GB(4) -
> -((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
> -<< XC_PAGE_SHIFT);
> +else if (dom->mmio_size == 0 && !device_model) {
> +#if defined(__i386__) || defined(__x86_64__)
> +if (libxl_defbool_val(info->u.hvm.apic)) {
> +/* Make sure LAPIC_BASE_ADDRESS is below special pages */
> +assert(X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
> +  << XC_PAGE_SHIFT) - LAPIC_BASE_ADDRESS)) >= 
> XC_PAGE_SIZE);
> +dom->mmio_size = GB(4) - LAPIC_BASE_ADDRESS;
> +} else
> +dom->mmio_size = GB(4) -
> +((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
> + << XC_PAGE_SHIFT);
> +#else
> +assert(1);

This looks a bit odd. Do you want to avoid "if {}" (nothing in braces)?

If this branch doesn't nothing on ARM, maybe you can just do

#if defined(x86)
 else if () {
 }
#endif

?
  
> +#endif
> +}
>  lowmem_end = mem_size;
>  highmem_end = 0;
>  mmio_start = (1ull << 32) - dom->mmio_size;
> -- 
> 1.7.1
> 

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

Re: [Xen-devel] [PATCH v3 11/17] libxl/arm: Construct ACPI MADT table

[Julien Grall]

Hi Wei,

On 07/07/16 17:11, Wei Liu wrote:

On Tue, Jul 05, 2016 at 11:12:41AM +0800, Shannon Zhao wrote:

From: Shannon Zhao 

According to the GIC version, construct the MADT table.

Signed-off-by: Shannon Zhao 
---
  tools/libxl/libxl_arm_acpi.c | 83 
  1 file changed, 83 insertions(+)

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index c2599b7..96ce605 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -199,6 +199,86 @@ static void make_acpi_gtdt(libxl__gc *gc, struct 
xc_dom_image *dom,
 acpitables[GTDT].size);
  }

+static void make_acpi_madt_gicc(void *table, int nr_cpus, uint64_t gicc_base)
+{
+uint32_t i;
+struct acpi_madt_generic_interrupt *gicc = table;
+
+for (i = 0; i < nr_cpus; i++) {
+gicc->header.type = ACPI_MADT_TYPE_GENERIC_INTERRUPT;
+gicc->header.length = sizeof(*gicc);
+gicc->base_address = gicc_base;
+gicc->cpu_interface_number = i;
+gicc->arm_mpidr = libxl__compute_mpdir(i);
+gicc->uid = i;
+gicc->flags = ACPI_MADT_ENABLED;
+gicc++;
+}
+}
+
+static void make_acpi_madt_gicd(void *table, uint64_t gicd_base,
+uint8_t gic_version)
+{
+struct acpi_madt_generic_distributor *gicd = table;
+
+gicd->header.type = ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR;
+gicd->header.length = sizeof(*gicd);
+gicd->base_address = gicd_base;
+/* This version field has no meaning before ACPI 5.1 errata. */
+gicd->version = gic_version;
+}
+
+static void make_acpi_madt_gicr(void *table, uint64_t gicr_base,
+uint64_t gicr_size)
+{
+struct acpi_madt_generic_redistributor *gicr = table;
+
+gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
+gicr->header.length = sizeof(*gicr);
+gicr->base_address = gicr_base;
+gicr->length = gicr_size;
+}
+
+static int make_acpi_madt(libxl__gc *gc, struct xc_dom_image *dom, int nr_cpus,
+  xc_domain_configuration_t *xc_config,
+  struct acpitable acpitables[])
+{
+uint64_t offset = acpitables[MADT].addr - GUEST_ACPI_BASE;
+struct acpi_table_madt *madt = dom->acpitable_blob + offset;
+void *table = dom->acpitable_blob + offset;
+
+switch (xc_config->gic_version) {
+case XEN_DOMCTL_CONFIG_GIC_V2:
+table += sizeof(struct acpi_table_madt);
+make_acpi_madt_gicc(table, nr_cpus, GUEST_GICC_BASE);
+
+table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
+make_acpi_madt_gicd(table, GUEST_GICD_BASE, ACPI_MADT_GIC_VERSION_V2);
+break;
+case XEN_DOMCTL_CONFIG_GIC_V3:
+table += sizeof(struct acpi_table_madt);
+make_acpi_madt_gicc(table, nr_cpus, 0);
+
+table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
+make_acpi_madt_gicd(table, GUEST_GICV3_GICD_BASE,
+ACPI_MADT_GIC_VERSION_V3);
+
+table += sizeof(struct acpi_madt_generic_distributor);
+make_acpi_madt_gicr(table, GUEST_GICV3_GICR0_BASE,
+GUEST_GICV3_GICR0_SIZE);
+break;
+default:
+LOG(ERROR, "Unknown GIC version");
+return ERROR_FAIL;
+}
+


Why is this code snippet referencing libxc structure? I would think all
relevant information is already available in libxl, right?


The hypervisor may choose the version of the GIC emulated if the user 
did not specified one. Those values are not replicated in libxl because 
this will be encoded in the hvm records during the migration (see the 
thread on the patch which introduced xc_config [1]).


FWIW, this is similar to what is done for the device tree case.




+make_acpi_header(>header, "APIC", acpitables[MADT].size, 3);
+calculate_checksum(madt, offsetof(struct acpi_table_header, checksum),
+   acpitables[MADT].size);
+
+return 0;
+}
+
  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
  libxl__domain_build_state *state,
  struct xc_dom_image *dom)
@@ -227,6 +307,9 @@ int libxl__prepare_acpi(libxl__gc *gc, 
libxl_domain_build_info *info,
  make_acpi_rsdp(gc, dom, acpitables);
  make_acpi_xsdt(gc, dom, acpitables);
  make_acpi_gtdt(gc, dom, acpitables);
+rc = make_acpi_madt(gc, dom, info->max_vcpus, xc_config, acpitables);
+if (rc)
+   return rc;

  return 0;
  }
--
2.0.4






[1] https://lists.xen.org/archives/html/xen-devel/2015-02/msg02538.html

--
Julien Grall

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

Re: [Xen-devel] [PATCH 13/19] xen: credit2: make the code less experimental

[Dario Faggioli]

On Thu, 2016-07-07 at 16:17 +0100, George Dunlap wrote:
> On Sat, Jun 18, 2016 at 12:12 AM, Dario Faggioli
>  wrote:
> > @@ -680,8 +677,8 @@ __update_svc_load(const struct scheduler *ops,
> >  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);
> > +printk("WARNING: %s: Time went backwards? now
> > %"PRI_stime" llu %"PRI_stime"\n",
> > +   __func__, now, svc->load_last_update);
> Hmm, I'm afraid this makes all Jan's comments from patch 7 which I
> argued against since it was just a debugging message now valid.
> 
Yes, and on second thoughts --as I wrote myself earlier today-- I think
we actually want to keep these debug only.

I'll make things that way when resending.

> > @@ -1540,9 +1536,26 @@ static void migrate(const struct scheduler
> > *ops,
> >  struct csched2_runqueue_data *trqd,
> >  s_time_t now)
> >  {
> > -if ( svc->flags & CSFLAG_scheduled )
> > +bool_t running = svc->flags & CSFLAG_scheduled;
> > +bool_t on_runq = __vcpu_on_runq(svc);
> What's the point of having these variables here?  AFAICS 'running' is
> used exactly once; and on_runq is only used inside the original else
> {
> } clause where it was before.
> 
Mmm... not much indeed. AFAICR, it's a remnant from a previous version
of the patch. Sorry.

> > @@ -2069,12 +2076,13 @@ csched2_schedule(
> >  }
> >  }
> >  }
> > -printk("%s: pcpu %d rq %d, but scurr %pv assigned to "
> > +printk("DEBUG: %s: pcpu %d rq %d, but scurr %pv assigned
> > to "
> > "pcpu %d rq %d!\n",
> > __func__,
> > cpu, this_rqi,
> > scurr->vcpu, scurr->vcpu->processor, other_rqi);
> >  }
> > +#endif
> Do we need this path anymore? I think it was just there to help
> debugging; but all this should have been sorted out a long time ago.
> :-)
> 
Right. I'm more than up for killing it.

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



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

Re: [Xen-devel] [PATCH v3 06/17] libxl/arm: Estimate the size of ACPI tables

[Julien Grall]

Hi Wei,

On 07/07/16 17:07, Wei Liu wrote:

On Tue, Jul 05, 2016 at 11:12:36AM +0800, Shannon Zhao wrote:

  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
  libxl__domain_build_state *state,
  struct xc_dom_image *dom)
  {
  const libxl_version_info *vers;
+int rc;
+struct acpitable acpitables[NUMS];
+
+/* convenience aliases */
+xc_domain_configuration_t *xc_config = >config;



Julien seemed to have suggested this structure shouldn't be used. Did I
misremember?


On the previous version, I said we should not extend this structure for 
parameter which is not used by the hypervisor (e.g the hypervisor does 
not need to know whether the guest will use ACPI).


However, this structure have to be used to get the version of the GIC 
that will be emulated for the guest. This is because the hypervisor 
might choose the version if the user did not specify one.


Cheers,

--
Julien Grall

___
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 07/07/16 16:36, Daniel De Graaf wrote:

On 07/06/2016 12:19 PM, anshul makkar wrote:

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.


Restricted how?  The fallback types have the resource_type attribute.

Restricted if they are unlabeled.


Neverallow rules are actually not present in the binary policy; they act as
compile-time assertions in the policy build.


Fine.



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.



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.


This prevents using delegate_devices without allowing access to unlabeled
devices.  If you think this should be a macro, I would suggest making a new
one named something like "delegate_unlabeled_devices".



Agreed. That's a better approach.
I believe this macro can make the default policy more flexible and 
useful for more general audience, so it should be there in the policy. I 
can submit patch for the same. Your thoughts ?


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

Re: [Xen-devel] [PATCH 16/18] arm/altp2m: Extended libxl to activate altp2m on ARM.

[Wei Liu]

On Mon, Jul 04, 2016 at 01:45:45PM +0200, Sergej Proskurin wrote:
> The current implementation allows to set the parameter HVM_PARAM_ALTP2M.
> This parameter allows further usage of altp2m on ARM. For this, we
> define an additional altp2m field for PV domains as part of the
> libxl_domain_type struct. This field can be set only on ARM systems
> through the "altp2m" switch in the domain's configuration file (i.e.
> set altp2m=1).
> 
> Signed-off-by: Sergej Proskurin 
> ---
> Cc: Ian Jackson 
> Cc: Wei Liu 
> ---
>  tools/libxl/libxl_create.c  |  1 +
>  tools/libxl/libxl_dom.c | 14 ++
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/xl_cmdimpl.c|  5 +
>  4 files changed, 21 insertions(+)
> 
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 1b99472..40b5f61 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -400,6 +400,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>  b_info->cmdline = b_info->u.pv.cmdline;
>  b_info->u.pv.cmdline = NULL;
>  }
> +libxl_defbool_setdefault(_info->u.pv.altp2m, false);
>  break;
>  default:
>  LOG(ERROR, "invalid domain type %s in create info",
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index ec29060..ab023a2 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -277,6 +277,16 @@ err:
>  }
>  #endif
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +static void pv_set_conf_params(xc_interface *handle, uint32_t domid,
> +   libxl_domain_build_info *const info)
> +{
> +if ( libxl_defbool_val(info->u.pv.altp2m) )

Coding style.

> +xc_hvm_param_set(handle, domid, HVM_PARAM_ALTP2M,
> + libxl_defbool_val(info->u.pv.altp2m));
> +}
> +#endif
> +
>  static void hvm_set_conf_params(xc_interface *handle, uint32_t domid,
>  libxl_domain_build_info *const info)
>  {
> @@ -433,6 +443,10 @@ int libxl__build_pre(libxl__gc *gc, uint32_t domid,
>  return rc;
>  #endif
>  }
> +#if defined(__arm__) || defined(__aarch64__)
> +else /* info->type == LIBXL_DOMAIN_TYPE_PV */
> +pv_set_conf_params(ctx->xch, domid, info);
> +#endif
>  
>  rc = libxl__arch_domain_create(gc, d_config, domid);
>  
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index ef614be..0a164f9 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -554,6 +554,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>("features", string, {'const': True}),
># Use host's E820 for PCI passthrough.
>("e820_host", libxl_defbool),
> +  ("altp2m", libxl_defbool),

Why is this placed in PV instead of arch_arm?

You would also need a LIBXL_HAVE macro in libxl.h for the new field.

>])),
>   ("invalid", None),
>   ], keyvar_init_val = "LIBXL_DOMAIN_TYPE_INVALID")),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 6459eec..12c6e48 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1718,6 +1718,11 @@ static void parse_config_data(const char 
> *config_source,
>  exit(1);
>  }
>  
> +#if defined(__arm__) || defined(__aarch64__)
> +/* Enable altp2m for PV guests solely on ARM */
> +xlu_cfg_get_defbool(config, "altp2m", _info->u.pv.altp2m, 0);
> +#endif
> +
>  break;
>  }
>  default:
> -- 
> 2.8.3
> 

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

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Wei Liu]

On Thu, Jul 07, 2016 at 12:12:48PM -0400, Boris Ostrovsky wrote:
> On 07/07/2016 12:06 PM, Julien Grall wrote:
> >
> >
> > On 07/07/16 16:48, Boris Ostrovsky wrote:
> >> On 07/07/2016 11:41 AM, Wei Liu wrote:
> >>> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
>  From: Shannon Zhao 
> 
>  Factor out codes for generating DTB to prepare for adding ACPI tables
>  generation codes.
> 
>  Signed-off-by: Shannon Zhao 
>  Acked-by: Wei Liu 
>  ---
>    tools/libxl/libxl_arm.c | 18 --
>    1 file changed, 12 insertions(+), 6 deletions(-)
> 
>  diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>  index ddd80aa..4a57dd7 100644
>  --- a/tools/libxl/libxl_arm.c
>  +++ b/tools/libxl/libxl_arm.c
>  @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc,
>  void *fdt, void *pfdt)
> 
>    #define FDT_MAX_SIZE (1<<20)
> 
>  -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>  -   libxl_domain_build_info
>  *info,
>  -  
>  libxl__domain_build_state *state,
>  -   struct xc_dom_image *dom)
>  +static int libxl__prepare_dtb(libxl__gc *gc,
>  libxl_domain_build_info *info,
>  +   libxl__domain_build_state *state,
>  +   struct xc_dom_image *dom)
> >>> I've queued this up for committing and will fix the indentation as I go
> >>> along.
> >>
> >>
> >> I don't think this can be ready for committing since it sits on top of
> >> my not-yet-reviewed series (which is pretty much guaranteed to require a
> >> new spin).
> >
> > This is only used by the ARM code which you don't modify. So I don't
> > see any issue to commit this patch.
> 
> I thought Wei was referring to the whole series being prepared for
> committing. If he was talking only about this patch (and possibly
> selected other patches) then yes, there is no dependency here. I know
> that further patches in this series want to see tools/libacpi, for
> example, and that is not ready.
> 

Oh yes, sure. This patch series is not yet ready to go in. :-)

I was only about to commit this first patch to reduce Shannon's patch
queue length.

> -boris
> 
> 

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

Re: [Xen-devel] xenstored memory leak

[Wei Liu]

On Wed, Jul 06, 2016 at 09:31:38AM +0200, 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.
> 
> Did anyone notice this memory leak before?
> 

No. But I don't normally look at xenstored memory consumption anyway...

> David, it seems ocaml based xenstore doesn't have such a problem. How
> mature is the xenstore Mirage-OS variant? Could it be easily integrated
> into the Xen build?
> 
> 
> Juergen

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

Re: [Xen-devel] [PATCH v3 17/17] libxl/arm: Initialize domain param HVM_PARAM_CALLBACK_IRQ

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:47AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> The guest kernel will get the event channel interrupt information via
> domain param HVM_PARAM_CALLBACK_IRQ. Initialize it here.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  tools/libxl/libxl_arm.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index bc38318..acacba0 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -900,8 +900,19 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> struct xc_dom_image *dom)
>  {
>  int rc;
> +uint64_t val;
>  
>  assert(info->type == LIBXL_DOMAIN_TYPE_PV);
> +
> +/* Set the value of domain param HVM_PARAM_CALLBACK_IRQ. */
> +val = (uint64_t)HVM_PARAM_CALLBACK_TYPE_PPI << 56;
> +val |= (2 << 8); /* Active-low level-sensitive  */

Please avoid using magic numbers here -- 56, 2 and 8.

Another question to Julien and Stefano: is it normal for ARM guest to
use hvm callback vector?

Wei.

> +val |= GUEST_EVTCHN_PPI & 0xff;
> +rc = xc_hvm_param_set(dom->xch, dom->guest_domid, HVM_PARAM_CALLBACK_IRQ,
> +  val);
> +if (rc)
> +return rc;
> +
>  rc = libxl__prepare_dtb(gc, info, state, dom);
>  if (rc)
>  return rc;
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Julien Grall]

On 07/07/16 17:12, Boris Ostrovsky wrote:

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



On 07/07/16 16:48, Boris Ostrovsky wrote:

On 07/07/2016 11:41 AM, Wei Liu wrote:

On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:

From: Shannon Zhao 

Factor out codes for generating DTB to prepare for adding ACPI tables
generation codes.

Signed-off-by: Shannon Zhao 
Acked-by: Wei Liu 
---
   tools/libxl/libxl_arm.c | 18 --
   1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index ddd80aa..4a57dd7 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc,
void *fdt, void *pfdt)

   #define FDT_MAX_SIZE (1<<20)

-int libxl__arch_domain_init_hw_description(libxl__gc *gc,
-   libxl_domain_build_info
*info,
-
libxl__domain_build_state *state,
-   struct xc_dom_image *dom)
+static int libxl__prepare_dtb(libxl__gc *gc,
libxl_domain_build_info *info,
+   libxl__domain_build_state *state,
+   struct xc_dom_image *dom)

I've queued this up for committing and will fix the indentation as I go
along.



I don't think this can be ready for committing since it sits on top of
my not-yet-reviewed series (which is pretty much guaranteed to require a
new spin).


This is only used by the ARM code which you don't modify. So I don't
see any issue to commit this patch.


I thought Wei was referring to the whole series being prepared for
committing. If he was talking only about this patch (and possibly
selected other patches) then yes, there is no dependency here. I know
that further patches in this series want to see tools/libacpi, for
example, and that is not ready.


I know about this. I don't think this series is ready to be fully 
committed due to some the disagreement such as where to handle the ACPI 
blob.


However, it would be nice to get trivial patch to slim down the size of 
the series.


Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Boris Ostrovsky]

On 07/07/2016 12:06 PM, Julien Grall wrote:
>
>
> On 07/07/16 16:48, Boris Ostrovsky wrote:
>> On 07/07/2016 11:41 AM, Wei Liu wrote:
>>> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
 From: Shannon Zhao 

 Factor out codes for generating DTB to prepare for adding ACPI tables
 generation codes.

 Signed-off-by: Shannon Zhao 
 Acked-by: Wei Liu 
 ---
   tools/libxl/libxl_arm.c | 18 --
   1 file changed, 12 insertions(+), 6 deletions(-)

 diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
 index ddd80aa..4a57dd7 100644
 --- a/tools/libxl/libxl_arm.c
 +++ b/tools/libxl/libxl_arm.c
 @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc,
 void *fdt, void *pfdt)

   #define FDT_MAX_SIZE (1<<20)

 -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
 -   libxl_domain_build_info
 *info,
 -  
 libxl__domain_build_state *state,
 -   struct xc_dom_image *dom)
 +static int libxl__prepare_dtb(libxl__gc *gc,
 libxl_domain_build_info *info,
 +   libxl__domain_build_state *state,
 +   struct xc_dom_image *dom)
>>> I've queued this up for committing and will fix the indentation as I go
>>> along.
>>
>>
>> I don't think this can be ready for committing since it sits on top of
>> my not-yet-reviewed series (which is pretty much guaranteed to require a
>> new spin).
>
> This is only used by the ARM code which you don't modify. So I don't
> see any issue to commit this patch.

I thought Wei was referring to the whole series being prepared for
committing. If he was talking only about this patch (and possibly
selected other patches) then yes, there is no dependency here. I know
that further patches in this series want to see tools/libacpi, for
example, and that is not ready.

-boris



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

Re: [Xen-devel] [PATCH v3 11/17] libxl/arm: Construct ACPI MADT table

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:41AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> According to the GIC version, construct the MADT table.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  tools/libxl/libxl_arm_acpi.c | 83 
> 
>  1 file changed, 83 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index c2599b7..96ce605 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -199,6 +199,86 @@ static void make_acpi_gtdt(libxl__gc *gc, struct 
> xc_dom_image *dom,
> acpitables[GTDT].size);
>  }
>  
> +static void make_acpi_madt_gicc(void *table, int nr_cpus, uint64_t gicc_base)
> +{
> +uint32_t i;
> +struct acpi_madt_generic_interrupt *gicc = table;
> +
> +for (i = 0; i < nr_cpus; i++) {
> +gicc->header.type = ACPI_MADT_TYPE_GENERIC_INTERRUPT;
> +gicc->header.length = sizeof(*gicc);
> +gicc->base_address = gicc_base;
> +gicc->cpu_interface_number = i;
> +gicc->arm_mpidr = libxl__compute_mpdir(i);
> +gicc->uid = i;
> +gicc->flags = ACPI_MADT_ENABLED;
> +gicc++;
> +}
> +}
> +
> +static void make_acpi_madt_gicd(void *table, uint64_t gicd_base,
> +uint8_t gic_version)
> +{
> +struct acpi_madt_generic_distributor *gicd = table;
> +
> +gicd->header.type = ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR;
> +gicd->header.length = sizeof(*gicd);
> +gicd->base_address = gicd_base;
> +/* This version field has no meaning before ACPI 5.1 errata. */
> +gicd->version = gic_version;
> +}
> +
> +static void make_acpi_madt_gicr(void *table, uint64_t gicr_base,
> +uint64_t gicr_size)
> +{
> +struct acpi_madt_generic_redistributor *gicr = table;
> +
> +gicr->header.type = ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR;
> +gicr->header.length = sizeof(*gicr);
> +gicr->base_address = gicr_base;
> +gicr->length = gicr_size;
> +}
> +
> +static int make_acpi_madt(libxl__gc *gc, struct xc_dom_image *dom, int 
> nr_cpus,
> +  xc_domain_configuration_t *xc_config,
> +  struct acpitable acpitables[])
> +{
> +uint64_t offset = acpitables[MADT].addr - GUEST_ACPI_BASE;
> +struct acpi_table_madt *madt = dom->acpitable_blob + offset;
> +void *table = dom->acpitable_blob + offset;
> +
> +switch (xc_config->gic_version) {
> +case XEN_DOMCTL_CONFIG_GIC_V2:
> +table += sizeof(struct acpi_table_madt);
> +make_acpi_madt_gicc(table, nr_cpus, GUEST_GICC_BASE);
> +
> +table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
> +make_acpi_madt_gicd(table, GUEST_GICD_BASE, 
> ACPI_MADT_GIC_VERSION_V2);
> +break;
> +case XEN_DOMCTL_CONFIG_GIC_V3:
> +table += sizeof(struct acpi_table_madt);
> +make_acpi_madt_gicc(table, nr_cpus, 0);
> +
> +table += sizeof(struct acpi_madt_generic_interrupt) * nr_cpus;
> +make_acpi_madt_gicd(table, GUEST_GICV3_GICD_BASE,
> +ACPI_MADT_GIC_VERSION_V3);
> +
> +table += sizeof(struct acpi_madt_generic_distributor);
> +make_acpi_madt_gicr(table, GUEST_GICV3_GICR0_BASE,
> +GUEST_GICV3_GICR0_SIZE);
> +break;
> +default:
> +LOG(ERROR, "Unknown GIC version");
> +return ERROR_FAIL;
> +}
> +

Why is this code snippet referencing libxc structure? I would think all
relevant information is already available in libxl, right?

> +make_acpi_header(>header, "APIC", acpitables[MADT].size, 3);
> +calculate_checksum(madt, offsetof(struct acpi_table_header, checksum),
> +   acpitables[MADT].size);
> +
> +return 0;
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>  libxl__domain_build_state *state,
>  struct xc_dom_image *dom)
> @@ -227,6 +307,9 @@ int libxl__prepare_acpi(libxl__gc *gc, 
> libxl_domain_build_info *info,
>  make_acpi_rsdp(gc, dom, acpitables);
>  make_acpi_xsdt(gc, dom, acpitables);
>  make_acpi_gtdt(gc, dom, acpitables);
> +rc = make_acpi_madt(gc, dom, info->max_vcpus, xc_config, acpitables);
> +if (rc)
> + return rc;
>  
>  return 0;
>  }
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Wei Liu]

On Thu, Jul 07, 2016 at 05:06:11PM +0100, Julien Grall wrote:
> 
> 
> On 07/07/16 16:48, Boris Ostrovsky wrote:
> >On 07/07/2016 11:41 AM, Wei Liu wrote:
> >>On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
> >>>From: Shannon Zhao 
> >>>
> >>>Factor out codes for generating DTB to prepare for adding ACPI tables
> >>>generation codes.
> >>>
> >>>Signed-off-by: Shannon Zhao 
> >>>Acked-by: Wei Liu 
> >>>---
> >>>  tools/libxl/libxl_arm.c | 18 --
> >>>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>>
> >>>diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> >>>index ddd80aa..4a57dd7 100644
> >>>--- a/tools/libxl/libxl_arm.c
> >>>+++ b/tools/libxl/libxl_arm.c
> >>>@@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, 
> >>>void *pfdt)
> >>>
> >>>  #define FDT_MAX_SIZE (1<<20)
> >>>
> >>>-int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> >>>-   libxl_domain_build_info *info,
> >>>-   libxl__domain_build_state 
> >>>*state,
> >>>-   struct xc_dom_image *dom)
> >>>+static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info 
> >>>*info,
> >>>+   libxl__domain_build_state *state,
> >>>+   struct xc_dom_image *dom)
> >>I've queued this up for committing and will fix the indentation as I go
> >>along.
> >
> >
> >I don't think this can be ready for committing since it sits on top of
> >my not-yet-reviewed series (which is pretty much guaranteed to require a
> >new spin).
> 
> This is only used by the ARM code which you don't modify. So I don't see any
> issue to commit this patch.

Ah, so I can go ahead after all.

> 
> Regards,
> 
> 
> -- 
> Julien Grall

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

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Wei Liu]

On Thu, Jul 07, 2016 at 11:48:49AM -0400, Boris Ostrovsky wrote:
> On 07/07/2016 11:41 AM, Wei Liu wrote:
> > On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
> >> From: Shannon Zhao 
> >>
> >> Factor out codes for generating DTB to prepare for adding ACPI tables
> >> generation codes.
> >>
> >> Signed-off-by: Shannon Zhao 
> >> Acked-by: Wei Liu 
> >> ---
> >>  tools/libxl/libxl_arm.c | 18 --
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> >> index ddd80aa..4a57dd7 100644
> >> --- a/tools/libxl/libxl_arm.c
> >> +++ b/tools/libxl/libxl_arm.c
> >> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, 
> >> void *pfdt)
> >>  
> >>  #define FDT_MAX_SIZE (1<<20)
> >>  
> >> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> >> -   libxl_domain_build_info *info,
> >> -   libxl__domain_build_state 
> >> *state,
> >> -   struct xc_dom_image *dom)
> >> +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info 
> >> *info,
> >> +   libxl__domain_build_state *state,
> >> +   struct xc_dom_image *dom)
> > I've queued this up for committing and will fix the indentation as I go
> > along.
> 
> 
> I don't think this can be ready for committing since it sits on top of
> my not-yet-reviewed series (which is pretty much guaranteed to require a
> new spin).
> 

I thought this patch posted long time ago. But things might have
changed. I will wait.

> 
> -boris
> 
> 
> 

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

Re: [Xen-devel] [PATCH v3 06/17] libxl/arm: Estimate the size of ACPI tables

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:36AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Estimate the size of ACPI tables and reserve a memory map space for ACPI
> tables.
> 
> Signed-off-by: Shannon Zhao 

I will leave this patch to Stefano and Julien.

> ---
>  tools/libxl/libxl_arm_acpi.c  | 85 
> +++
>  xen/include/public/arch-arm.h |  4 ++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index d1c066d..7a59126 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -33,11 +33,92 @@ extern const unsigned char dsdt_anycpu_arm[];
>  _hidden
>  extern const int dsdt_anycpu_arm_len;
>  
> +enum {
> +RSDP,
> +XSDT,
> +GTDT,
> +MADT,
> +FADT,
> +DSDT,
> +NUMS,
> +};
> +
> +struct acpitable {
> +uint64_t addr;
> +size_t size;
> +};
> +
> +static int libxl__estimate_acpi_size(libxl__gc *gc,
> + libxl_domain_build_info *info,
> + struct xc_dom_image *dom,
> + xc_domain_configuration_t *xc_config,
> + struct acpitable acpitables[])
> +{
> +uint64_t size;
> +
> +acpitables[RSDP].addr = GUEST_ACPI_BASE;
> +acpitables[RSDP].size = sizeof(struct acpi_table_rsdp);
> +dom->acpitable_size += ROUNDUP(acpitables[RSDP].size, 3);
> +
> +acpitables[XSDT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +/*
> + * Currently only 3 tables(GTDT, FADT, MADT) are pointed by XSDT. Alloc
> + * entries for them.
> + */
> +acpitables[XSDT].size = sizeof(struct acpi_table_xsdt) +
> +sizeof(uint64_t) * 2;
> +dom->acpitable_size += ROUNDUP(acpitables[XSDT].size, 3);
> +
> +acpitables[GTDT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +acpitables[GTDT].size = sizeof(struct acpi_table_gtdt);
> +dom->acpitable_size += ROUNDUP(acpitables[GTDT].size, 3);
> +
> +acpitables[MADT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +
> +switch (xc_config->gic_version) {
> +case XEN_DOMCTL_CONFIG_GIC_V2:
> +size = sizeof(struct acpi_table_madt) +
> +   sizeof(struct acpi_madt_generic_interrupt) * info->max_vcpus +
> +   sizeof(struct acpi_madt_generic_distributor);
> +break;
> +case XEN_DOMCTL_CONFIG_GIC_V3:
> +size = sizeof(struct acpi_table_madt) +
> +   sizeof(struct acpi_madt_generic_interrupt) * info->max_vcpus +
> +   sizeof(struct acpi_madt_generic_distributor) +
> +   sizeof(struct acpi_madt_generic_redistributor);
> +break;
> +default:
> +LOG(ERROR, "Unknown GIC version");
> +return ERROR_FAIL;
> +}
> +
> +acpitables[MADT].size = size;
> +dom->acpitable_size += ROUNDUP(acpitables[MADT].size, 3);
> +
> +acpitables[FADT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +acpitables[FADT].size = sizeof(struct acpi_table_fadt);
> +dom->acpitable_size += ROUNDUP(acpitables[FADT].size, 3);
> +
> +acpitables[DSDT].addr = GUEST_ACPI_BASE + dom->acpitable_size;
> +acpitables[DSDT].size = dsdt_anycpu_arm_len;
> +dom->acpitable_size += ROUNDUP(acpitables[DSDT].size, 3);
> +
> +assert(dom->acpitable_size <= GUEST_ACPI_SIZE);
> +dom->acpitable_blob = libxl__zalloc(gc, dom->acpitable_size);
> +

Goto style error handling.

> +return 0;
> +}
> +
>  int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
>  libxl__domain_build_state *state,
>  struct xc_dom_image *dom)
>  {
>  const libxl_version_info *vers;
> +int rc;
> +struct acpitable acpitables[NUMS];
> +
> +/* convenience aliases */
> +xc_domain_configuration_t *xc_config = >config;
>  

Julien seemed to have suggested this structure shouldn't be used. Did I
misremember?

>  vers = libxl_get_version_info(CTX);
>  if (vers == NULL)
> @@ -49,6 +130,10 @@ int libxl__prepare_acpi(libxl__gc *gc, 
> libxl_domain_build_info *info,
>  dom->acpitable_blob = NULL;
>  dom->acpitable_size = 0;
>  
> +rc = libxl__estimate_acpi_size(gc, info, dom, xc_config, acpitables);
> +if (rc)
> +return rc;
> +
>  return 0;
>  }

Goto style error handling please.

>  
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 4a49254..008a2a0 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -406,6 +406,10 @@ typedef uint64_t xen_callback_t;
>  #define GUEST_GICV3_GICR0_BASE 0x0302ULL/* vCPU0 - vCPU127 */
>  #define GUEST_GICV3_GICR0_SIZE 0x0100ULL
>  
> +/* ACPI tables physical address */
> +#define GUEST_ACPI_BASE 0x2000ULL
> +#define GUEST_ACPI_SIZE 0x0020ULL
> +
>  /*
>   * 16MB == 4096 pages 

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Julien Grall]

On 07/07/16 16:48, Boris Ostrovsky wrote:

On 07/07/2016 11:41 AM, Wei Liu wrote:

On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:

From: Shannon Zhao 

Factor out codes for generating DTB to prepare for adding ACPI tables
generation codes.

Signed-off-by: Shannon Zhao 
Acked-by: Wei Liu 
---
  tools/libxl/libxl_arm.c | 18 --
  1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index ddd80aa..4a57dd7 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, void 
*pfdt)

  #define FDT_MAX_SIZE (1<<20)

-int libxl__arch_domain_init_hw_description(libxl__gc *gc,
-   libxl_domain_build_info *info,
-   libxl__domain_build_state *state,
-   struct xc_dom_image *dom)
+static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
+   libxl__domain_build_state *state,
+   struct xc_dom_image *dom)

I've queued this up for committing and will fix the indentation as I go
along.



I don't think this can be ready for committing since it sits on top of
my not-yet-reviewed series (which is pretty much guaranteed to require a
new spin).


This is only used by the ARM code which you don't modify. So I don't see 
any issue to commit this patch.


Regards,


--
Julien Grall

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

Re: [Xen-devel] [PATCH 17/19] xen: credit2: the private scheduler lock can be an rwlock.

[George Dunlap]

On Sat, Jun 18, 2016 at 12:13 AM, Dario Faggioli
 wrote:
> In fact, the data it protects only change either at init-time,
> during cpupools manipulation, or when changing domains' weights.
> In all other cases (namely, load balancing, reading weights
> and status dumping), information is only read.
>
> Therefore, let the lock be an read/write one. This means there
> is no full serialization point for the whole scheduler and
> for all the pCPUs of the host any longer.
>
> This is particularly good for scalability (especially when doing
> load balancing).
>
> Also, update the high level description of the locking discipline,
> and take the chance for rewording it a little bit (as well as
> for adding a couple of locking related ASSERT()-s).
>
> Signed-off-by: Dario Faggioli 

Looks good:

Reviewed-by: George Dunlap 

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

Re: [Xen-devel] [PATCH] xen-blkback: constify instance of "struct attribute_group"

[Jan Beulich]

>>> On 07.07.16 at 17:37,  wrote:
> On Thu, Jul 07, 2016 at 10:52:18AM +0100, Andrew Cooper wrote:
>> On 07/07/16 10:45, Roger Pau Monne wrote:
>> > On Thu, Jul 07, 2016 at 01:38:58AM -0600, Jan Beulich wrote:
>> >> The functions such get passed to have been taking pointers to const
>> >> since at least 2.6.16.
>> >>
>> >> Signed-off-by: Jan Beulich 
>> > Acked-by: Roger Pau Monné 
>> >
>> > Although the wording in the commit message looks weird to me, but I'm not 
>> > a 
> 
>> > native speaker anyway.
>> 
>> As a native speaker, I can't parse it either.
>> 
>> I think s/such/these/ is needed.
> 
> The functions such as these have been taking pointers to const
> since at least 2.6.16. 

I had taken Andrew's suggestion literally and changed it to "The
functions these get passed to have been taking pointers to const
since at least 2.6.16" for a possible (if needed) resubmission.

Jan

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

Re: [Xen-devel] [PATCH v8 6/6] tools/xen-access: add test-case for ARM SMC

[Tamas K Lengyel]

On Thu, Jul 7, 2016 at 4:05 AM, Julien Grall  wrote:
>
>
> On 05/07/16 19:37, Tamas K Lengyel wrote:
>>
>> +#if defined(__arm__) || defined(__aarch64__)
>> +case VM_EVENT_REASON_PRIVILEGED_CALL:
>> +{
>> +const struct vm_event_regs_arm *in_regs =
>> 
>> +struct vm_event_regs_arm *out_regs =
>> 
>> +bool is32bit = !!(in_regs->cpsr & PSR_MODE_BIT);
>> +uint64_t pc;
>> +
>> +*out_regs = *in_regs;
>> +
>> +if ( is32bit ) {
>
>
> The open-bracket should be on a separate line.
>
>> +pc = in_regs->arch.arm32.pc;
>> +out_regs->arch.arm32.pc += 4;
>
>
> I suspect you will have to update the CSPR if the SMC instruction is part of
> an IT block (see advance_pc code in arch/arm/traps.c).
>
>> +} else {
>
>
> The open-bracket should be on a separate line.
>
>> +pc = in_regs->arch.arm64.pc;
>> +out_regs->arch.arm64.pc += 8;
>
>
> SMC instruction length is 4 bytes not 8 (see encoding in C6.2.165 in DDI
> 0487A.j).
>
>> +}
>> +
>> +printf("Privileged call: pc=%016"PRIx64" (vcpu
>> %d)\n",
>> +   pc, req.vcpu_id);
>> +
>> +rsp.flags |= VM_EVENT_FLAG_SET_REGISTERS;
>> +}
>> +break;
>> +#endif
>>   default:
>>   fprintf(stderr, "UNKNOWN REASON CODE %d\n", req.reason);
>>   }
>>
>
> Regards,
>
> --
> Julien Grall

Good points, thanks!

Tamas

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

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

[Jan Beulich]

>>> On 07.07.16 at 17:27,  wrote:
>> > @@ -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) )
>> 
>> Now that you use this constant a second time, it clearly needs to
>> become a #define.
> 
> Do you have a preference on the ull? Most of the code has ull but there
> are two instances of ULL?

Not really. I kind of dislike both of them.

Jan


___
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 Thu, Jul 7, 2016 at 4:09 AM, Julien Grall  wrote:
>
>
> On 07/07/16 10:57, Jan Beulich wrote:
>
> On 07.07.16 at 11:46,  wrote:
>>>
>>> Anyway, I think this patch was in a good state (though few registers
>>> were needed clarification). Assuming it is ok to break the compatibility
>>> later on, I will not oppose to have a reduce set.
>>
>>
>> Iirc we did bump that interface version already after the tree got
>> opened for 4.8.
>
>
> I had the impression that Tamas does not plan to add the full set of the
> registers for 4.8. If so, we would have to bump another time later.

I have no current plans for sending the full set of registers. As
Razvan pointed pointed out earlier, it would be better to do that as
part of a bigger extension of vm_event that would allow the ring to be
multi-page. That however is well beyond what I'm aiming to achieve
here at the moment. I rewrote the vm_event system specifically to
allow extensions to it that break the ABI in a way that applications
can safely deal with it. Yes, it may require applications to be
recompiled for newer versions of Xen but that is to be expected and
should not be a show-stopper.

Thanks,
Tamas

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

Re: [Xen-devel] [PATCH v3 05/17] libxl/arm: Generate static ACPI DSDT table

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:35AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> It uses static DSDT table like the way x86 uses. Currently the DSDT
> table only contains processor device objects and it generates the
> maximal objects which so far is 128.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  tools/libacpi/Makefile| 15 -
>  tools/libacpi/mk_dsdt.c   | 51 
> ---
>  tools/libxl/Makefile  |  5 -
>  tools/libxl/libxl_arm_acpi.c  |  5 +
>  xen/include/public/arch-arm.h |  3 +++
>  5 files changed, 64 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/libacpi/Makefile b/tools/libacpi/Makefile
> index 4068d9a..0401810 100644
> --- a/tools/libacpi/Makefile
> +++ b/tools/libacpi/Makefile
> @@ -22,6 +22,7 @@ MK_DSDT = $(ACPI_BUILD_DIR)/mk_dsdt
>  # Sources to be generated
>  C_SRC = $(ACPI_BUILD_DIR)/dsdt_anycpu.c $(ACPI_BUILD_DIR)/dsdt_15cpu.c 
>  C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_qemu_xen.c 
> $(ACPI_BUILD_DIR)/dsdt_pvh.c
> +C_SRC += $(ACPI_BUILD_DIR)/dsdt_anycpu_arm.c
>  H_SRC = $(ACPI_BUILD_DIR)/ssdt_s3.h $(ACPI_BUILD_DIR)/ssdt_s4.h 
> $(ACPI_BUILD_DIR)/ssdt_pm.h $(ACPI_BUILD_DIR)/ssdt_tpm.h
>  
>  vpath iasl $(PATH)
> @@ -35,7 +36,7 @@ $(H_SRC): $(ACPI_BUILD_DIR)/%.h: %.asl iasl
>   cd $(CURDIR)
>  
>  $(MK_DSDT): mk_dsdt.c
> - $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -o $@ mk_dsdt.c
> + $(HOSTCC) $(HOSTCFLAGS) $(CFLAGS_xeninclude) -D__XEN_TOOLS__ -o $@ 
> mk_dsdt.c

Why is this needed? Which unstable hypervisor interface you need in
order to build this?

Wei.

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

Re: [Xen-devel] [PATCH v3 04/17] libxl/arm: prepare for constructing ACPI tables

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:34AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> It only constructs the ACPI tables for 64-bit ARM DomU when user enables
> acpi because 32-bit DomU doesn't support ACPI.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  tools/libxl/Makefile |  4 
>  tools/libxl/libxl_arm.c  | 19 ++-
>  tools/libxl/libxl_arm.h  | 33 ++
>  tools/libxl/libxl_arm_acpi.c | 56 
> 
>  4 files changed, 111 insertions(+), 1 deletion(-)
>  create mode 100644 tools/libxl/libxl_arm.h
>  create mode 100644 tools/libxl/libxl_arm_acpi.c
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 0cf9f6a..88ab4d2 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -91,6 +91,10 @@ acpi:
>  
>  LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o libxl_x86.o libxl_psr.o 
> libxl_x86_acpi.o
>  LIBXL_OBJS-$(CONFIG_ARM) += libxl_nocpuid.o libxl_arm.o libxl_libfdt_compat.o
> +LIBXL_OBJS-$(CONFIG_ARM) += libxl_arm_acpi.o
> +
> +libxl_arm_acpi.o: libxl_arm_acpi.c
> + $(CC) -c $(CFLAGS) -I../../xen/include/ -o $@ libxl_arm_acpi.c
>  
>  ifeq ($(CONFIG_NetBSD),y)
>  LIBXL_OBJS-y += libxl_netbsd.o
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 4a57dd7..7c522e1 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -1,6 +1,7 @@
>  #include "libxl_internal.h"
>  #include "libxl_arch.h"
>  #include "libxl_libfdt_compat.h"
> +#include "libxl_arm.h"
>  
>  #include 
>  #include 
> @@ -885,8 +886,24 @@ int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> libxl__domain_build_state *state,
> struct xc_dom_image *dom)
>  {
> +int rc;
> +
>  assert(info->type == LIBXL_DOMAIN_TYPE_PV);
> -return libxl__prepare_dtb(gc, info, state, dom);
> +rc = libxl__prepare_dtb(gc, info, state, dom);
> +if (rc)
> +return rc;
> +
> +if (!libxl_defbool_val(info->arch_arm.acpi)) {
> +LOG(DEBUG, "Generating ACPI tables is disabled by user.");
> +return 0;
> +}
> +
> +if (strcmp(dom->guest_type, "xen-3.0-aarch64")) {
> +LOG(ERROR, "Can not enable xl option 'acpi' for %s", 
> dom->guest_type);
> +return ERROR_FAIL;
> +}
> +
> +return libxl__prepare_acpi(gc, info, state, dom);

Please use goto style error handling for consistency. See
tools/libxl/CODING_STYLE. Please fix all other instances as well.


>  }
>  
>  static void finalise_one_memory_node(libxl__gc *gc, void *fdt,
> diff --git a/tools/libxl/libxl_arm.h b/tools/libxl/libxl_arm.h
> new file mode 100644
> index 000..1c01177
> --- /dev/null
> +++ b/tools/libxl/libxl_arm.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (C) 2016  Linaro Ltd.
> + *
> + * Author: Shannon Zhao 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_internal.h"
> +#include "libxl_arch.h"
> +
> +#include 
> +
> +_hidden
> +int libxl__prepare_acpi(libxl__gc *gc, libxl_domain_build_info *info,
> +libxl__domain_build_state *state,
> +struct xc_dom_image *dom);
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> new file mode 100644
> index 000..8c273f9
> --- /dev/null
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -0,0 +1,56 @@
> +/*
> + * ARM DomU ACPI generation
> + *
> + * Copyright (C) 2016  Linaro Ltd.
> + *
> + * Author: Shannon Zhao 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +#include "libxl_arm.h"
> +
> +#include 
> +
> +typedef uint8_t u8;
> +typedef uint16_t u16;
> +typedef uint32_t u32;
> +typedef uint64_t u64;
> +
> +#include 
> 

Re: [Xen-devel] [PATCH v3 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:33AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Add a configuration option for ARM DomU so that user can deicde to use
> ACPI or not. This option is defaultly false.
> 
> Signed-off-by: Shannon Zhao 
> ---
>  tools/libxl/libxl.h | 5 +
>  tools/libxl/libxl_types.idl | 1 +
>  tools/libxl/xl_cmdimpl.c| 4 
>  3 files changed, 10 insertions(+)
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 2c0f868..ccaf87b 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -234,6 +234,11 @@
>  #define LIBXL_HAVE_BUILDINFO_ARM_GIC_VERSION 1
>  
>  /*
> + * libxl_domain_build_info has the arm.acpi field.
> + */
> +#define LIBXL_HAVE_BUILDINFO_ARM_ACPI 1
> +
> +/*
>   * LIBXL_HAVE_SOFT_RESET indicates that libxl supports performing
>   * 'soft reset' for domains and there is 'soft_reset' shutdown reason
>   * in enum libxl_shutdown_reason.
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index ef614be..426b868 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -560,6 +560,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>  
>  
>  ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
> +   ("acpi", libxl_defbool),
>])),
>  
>  ], dir=DIR_IN
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 6459eec..0634ffa 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2506,6 +2506,10 @@ skip_usbdev:
>  }
>   }
>  
> +if (xlu_cfg_get_defbool(config, "acpi", _info->arch_arm.acpi, 0)) {
> +libxl_defbool_set(_info->arch_arm.acpi, 0);
> +}
> +

Setting .acpi to false should be done in libxl.

See libxl__domain_build_info_setdefault.

>  xlu_cfg_destroy(config);
>  }
>  
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] [PATCH 15/19] xen: credit2: only marshall trace point arguments if tracing enabled

[George Dunlap]

On Sat, Jun 18, 2016 at 12:13 AM, Dario Faggioli
 wrote:
> Signed-off-by: Dario Faggioli 

> @@ -1696,10 +1702,8 @@ static void balance_load(const struct scheduler *ops, 
> int cpu, s_time_t now)
>
>  cpus_max = cpumask_weight(>active);
>  i = cpumask_weight(>active);
> -if ( i > cpus_max )
> -cpus_max = i;

What is this about?

Other than that, looks good, thanks.

 -George

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

Re: [Xen-devel] [PATCH 16/19] tools: tracing: deal with new Credit2 events

[George Dunlap]

On Sat, Jun 18, 2016 at 12:13 AM, Dario Faggioli
 wrote:
> more specifically, with: TICKLE_NEW, RUNQ_MAX_WEIGHT,
> MIGRATE, LOAD_CHECK, LOAD_BALANCE and PICKED_CPU, and
> in both both xenalyze and formats (for xentrace_format).
>
> Signed-off-by: Dario Faggioli 

Acked-by: George Dunlap 

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

Re: [Xen-devel] [PATCH 14/19] xen: credit2: add yet some more tracing

[George Dunlap]

On Mon, Jun 20, 2016 at 9:15 AM, Jan Beulich  wrote:
 On 18.06.16 at 01:12,  wrote:
>> @@ -1484,6 +1489,23 @@ csched2_cpu_pick(const struct scheduler *ops, struct 
>> vcpu *vc)
>>  out_up:
>>  spin_unlock(>lock);
>>
>> +/* TRACE */
>> +{
>> +struct {
>> +uint64_t b_avgload;
>> +unsigned vcpu:16, dom:16;
>> +unsigned rq_id:16, new_cpu:16;
>> +   } d;
>> +d.b_avgload = prv->rqd[min_rqi].b_avgload;
>> +d.dom = vc->domain->domain_id;
>> +d.vcpu = vc->vcpu_id;
>> +d.rq_id = c2r(ops, new_cpu);
>> +d.new_cpu = new_cpu;
>
> I guess this follows pre-existing style, but it would seem more natural
> to me for the variable to have an initializer instead of this series of
> assignments.

Well that doesn't actually save you that much typing, and I think it's
probably (slightly) less easy to read.  But the biggest thing at this
point is that it's inconsistent with what's there. :-)

 -George

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

Re: [Xen-devel] [PATCH 13/19] xen: credit2: make the code less experimental

[George Dunlap]

On Sat, Jun 18, 2016 at 12:12 AM, Dario Faggioli
 wrote:
> Mainly, almost all of the BUG_ON-s can be converted into
> ASSERTS, and the debug printk either removed or turned
> into tracing.
>
> The 'TODO' list, in a comment at the beginning of the file,
> was also stale, so remove items that were still there but
> are actually done.
>
> Signed-off-by: Dario Faggioli 

Overall looks good.  A couple of things...

> @@ -680,8 +677,8 @@ __update_svc_load(const struct scheduler *ops,
>  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);
> +printk("WARNING: %s: Time went backwards? now %"PRI_stime" llu 
> %"PRI_stime"\n",
> +   __func__, now, svc->load_last_update);

Hmm, I'm afraid this makes all Jan's comments from patch 7 which I
argued against since it was just a debugging message now valid.

> @@ -1540,9 +1536,26 @@ static void migrate(const struct scheduler *ops,
>  struct csched2_runqueue_data *trqd,
>  s_time_t now)
>  {
> -if ( svc->flags & CSFLAG_scheduled )
> +bool_t running = svc->flags & CSFLAG_scheduled;
> +bool_t on_runq = __vcpu_on_runq(svc);

What's the point of having these variables here?  AFAICS 'running' is
used exactly once; and on_runq is only used inside the original else {
} clause where it was before.

> @@ -2069,12 +2076,13 @@ csched2_schedule(
>  }
>  }
>  }
> -printk("%s: pcpu %d rq %d, but scurr %pv assigned to "
> +printk("DEBUG: %s: pcpu %d rq %d, but scurr %pv assigned to "
> "pcpu %d rq %d!\n",
> __func__,
> cpu, this_rqi,
> scurr->vcpu, scurr->vcpu->processor, other_rqi);
>  }
> +#endif

Do we need this path anymore? I think it was just there to help
debugging; but all this should have been sorted out a long time ago.
:-)

Thanks,
 -George

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

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Boris Ostrovsky]

On 07/07/2016 11:41 AM, Wei Liu wrote:
> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao 
>>
>> Factor out codes for generating DTB to prepare for adding ACPI tables
>> generation codes.
>>
>> Signed-off-by: Shannon Zhao 
>> Acked-by: Wei Liu 
>> ---
>>  tools/libxl/libxl_arm.c | 18 --
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index ddd80aa..4a57dd7 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, 
>> void *pfdt)
>>  
>>  #define FDT_MAX_SIZE (1<<20)
>>  
>> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>> -   libxl_domain_build_info *info,
>> -   libxl__domain_build_state *state,
>> -   struct xc_dom_image *dom)
>> +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
>> +   libxl__domain_build_state *state,
>> +   struct xc_dom_image *dom)
> I've queued this up for committing and will fix the indentation as I go
> along.


I don't think this can be ready for committing since it sits on top of
my not-yet-reviewed series (which is pretty much guaranteed to require a
new spin).


-boris




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

Re: [Xen-devel] [PATCH v3 02/17] libxc: Add placeholders for ACPI tables blob and size

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:32AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Add placeholders for ACPI tables blob and size here so that ACPI blob
> can be accessed from libxl and libxc.
> 
> Signed-off-by: Shannon Zhao 

Acked-by: Wei Liu 

Though this patch is simple, it seems that these fields would only be
used later patches, so I will wait until all of them are sorted.

> ---
>  tools/libxc/include/xc_dom.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index 6cb10c4..cbeb4fb 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -64,6 +64,8 @@ struct xc_dom_image {
>  size_t ramdisk_size;
>  void *devicetree_blob;
>  size_t devicetree_size;
> +void *acpitable_blob;
> +size_t acpitable_size;
>  
>  size_t max_kernel_size;
>  size_t max_ramdisk_size;
> -- 
> 2.0.4
> 
> 

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

Re: [Xen-devel] (no subject)

[Dario Faggioli]

On Thu, 2016-07-07 at 13:48 +0100, George Dunlap wrote:
> On 07/07/16 12:03, Dario Faggioli wrote:
> > On Thu, 2016-07-07 at 10:36 +0100, George Dunlap wrote:
> > > Could you re-send this with the trace change moved from the
> > > previous
> > > patch to this patch?
> > > 
> > So, you're saying I should change both Xen, xentrace_format and
> > xenalyze.c all at once, in the same patch, right?
> > 
> But now that xenalyze is in-tree, I think we want to avoid situations
> where the in-tree xenalyze is broken, even just for one changeset, if
> we
> can avoid it.
> 
Ok, this makes sense. Will do.

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



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

Re: [Xen-devel] [PATCH v3 01/17] libxl/arm: Factor out codes for generating DTB

[Wei Liu]

On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Factor out codes for generating DTB to prepare for adding ACPI tables
> generation codes.
> 
> Signed-off-by: Shannon Zhao 
> Acked-by: Wei Liu 
> ---
>  tools/libxl/libxl_arm.c | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index ddd80aa..4a57dd7 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, 
> void *pfdt)
>  
>  #define FDT_MAX_SIZE (1<<20)
>  
> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> -   libxl_domain_build_info *info,
> -   libxl__domain_build_state *state,
> -   struct xc_dom_image *dom)
> +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
> +   libxl__domain_build_state *state,
> +   struct xc_dom_image *dom)

I've queued this up for committing and will fix the indentation as I go
along.

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

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

[Boris Ostrovsky]

On 07/07/2016 11:25 AM, Konrad Rzeszutek Wilk wrote:
> On Thu, Jul 07, 2016 at 11:28:18AM +0800, Wei Jiangang wrote:
>> verify_local_APIC() had been removed by
>> commit 4399c03c6780 ("x86/apic: Remove verify_local_APIC()"),
>> so apic_id_mask isn't used by it.

Is anyone actually using this field? It looks like 4399c03c6780 removed
the only user.

-boris


> CC-ing the proper maintainers.
>> 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 mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] xen-blkback: constify instance of "struct attribute_group"

[Konrad Rzeszutek Wilk]

On Thu, Jul 07, 2016 at 10:52:18AM +0100, Andrew Cooper wrote:
> On 07/07/16 10:45, Roger Pau Monne wrote:
> > On Thu, Jul 07, 2016 at 01:38:58AM -0600, Jan Beulich wrote:
> >> The functions such get passed to have been taking pointers to const
> >> since at least 2.6.16.
> >>
> >> Signed-off-by: Jan Beulich 
> > Acked-by: Roger Pau Monné 
> >
> > Although the wording in the commit message looks weird to me, but I'm not a 
> > native speaker anyway.
> 
> As a native speaker, I can't parse it either.
> 
> I think s/such/these/ is needed.

The functions such as these have been taking pointers to const
since at least 2.6.16. 

?
> 
> ~Andrew
> 
> ___
> 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] default XSM policy for PCI passthrough for unlabeled resources.

[Daniel De Graaf]

On 07/06/2016 12:19 PM, anshul makkar wrote:

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.


Restricted how?  The fallback types have the resource_type attribute.

Neverallow rules are actually not present in the binary policy; they act as
compile-time assertions in the policy build.




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.


This prevents using delegate_devices without allowing access to unlabeled
devices.  If you think this should be a macro, I would suggest making a new
one named something like "delegate_unlabeled_devices".

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH v2 16/17] libxc/xc_dom_arm: Copy ACPI tables to guest space

[Wei Liu]

On Thu, Jun 23, 2016 at 07:46:42PM +0100, Julien Grall wrote:
> Hi Shannon,
> 
> On 23/06/2016 04:17, Shannon Zhao wrote:
> >From: Shannon Zhao 
> >
> >Copy all the ACPI tables to guest space so that UEFI or guest could
> >access them.
> >
> >Signed-off-by: Shannon Zhao 
> >---
> > tools/libxc/xc_dom_arm.c | 51 
> > 
> > 1 file changed, 51 insertions(+)
> >
> >diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> >index 64a8b67..6a0a5b7 100644
> >--- a/tools/libxc/xc_dom_arm.c
> >+++ b/tools/libxc/xc_dom_arm.c
> >@@ -63,6 +63,47 @@ static int setup_pgtables_arm(struct xc_dom_image *dom)
> >
> > /*  
> > */
> >
> >+static int xc_dom_copy_acpi(struct xc_dom_image *dom)
> >+{
> >+int rc, i;
> >+uint32_t pages_num = ROUNDUP(dom->acpitable_size, XC_PAGE_SHIFT) >>
> >+ XC_PAGE_SHIFT;
> >+const xen_pfn_t base = GUEST_ACPI_BASE >> XC_PAGE_SHIFT;
> >+xen_pfn_t *p2m;
> >+void *acpi_pages;
> >+
> >+p2m = malloc(pages_num * sizeof(*p2m));
> >+for (i = 0; i < pages_num; i++)
> >+p2m[i] = base + i;
> >+
> >+rc = xc_domain_populate_physmap_exact(dom->xch, dom->guest_domid,
> >+  pages_num, 0, 0, p2m);
> 
> H... it looks like this is working because libxl is setting the maximum
> size of the domain with some slack (1MB). However, I guess the slack was for
> something else. Wei, Stefano, Ian, can you confirm?

Please don't rely on this slack. Account memory properly please.

Wei.

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

[Xen-devel] [PATCH v5 1/2] xsm: rework policy_buffer globals

[Daniel De Graaf]

This makes the buffers function parameters instead of globals, in
preparation for adding alternate locations for the policy.

Signed-off-by: Daniel De Graaf 
---

This patch is new in v5.

 xen/include/xsm/xsm.h| 13 ++---
 xen/xsm/flask/hooks.c|  2 +-
 xen/xsm/flask/include/security.h |  2 +-
 xen/xsm/flask/ss/policydb.h  |  2 +-
 xen/xsm/flask/ss/services.c  |  2 +-
 xen/xsm/xsm_core.c   | 17 +++--
 xen/xsm/xsm_policy.c | 21 ++---
 7 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 4b8843d..e83dca2 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -43,9 +43,6 @@ enum xsm_default {
 };
 typedef enum xsm_default xsm_default_t;
 
-extern char *policy_buffer;
-extern u32 policy_size;
-
 struct xsm_operations {
 void (*security_domaininfo) (struct domain *d,
 struct xen_domctl_getdomaininfo *info);
@@ -740,12 +737,14 @@ extern int xsm_multiboot_init(unsigned long *module_map,
   void *(*bootstrap_map)(const module_t *));
 extern int xsm_multiboot_policy_init(unsigned long *module_map,
  const multiboot_info_t *mbi,
- void *(*bootstrap_map)(const module_t *));
+ void *(*bootstrap_map)(const module_t *),
+ void **policy_buffer,
+ size_t *policy_size);
 #endif
 
 #ifdef CONFIG_HAS_DEVICE_TREE
 extern int xsm_dt_init(void);
-extern int xsm_dt_policy_init(void);
+extern int xsm_dt_policy_init(void **policy_buffer, size_t *policy_size);
 extern bool has_xsm_magic(paddr_t);
 #endif
 
@@ -755,9 +754,9 @@ extern struct xsm_operations dummy_xsm_ops;
 extern void xsm_fixup_ops(struct xsm_operations *ops);
 
 #ifdef CONFIG_FLASK
-extern void flask_init(void);
+extern void flask_init(const void *policy_buffer, size_t policy_size);
 #else
-static inline void flask_init(void)
+static inline void flask_init(const void *policy_buffer, size_t policy_size)
 {
 }
 #endif
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 2692a6f..3555907 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -1815,7 +1815,7 @@ static struct xsm_operations flask_ops = {
 .xen_version = flask_xen_version,
 };
 
-__init void flask_init(void)
+__init void flask_init(const void *policy_buffer, size_t policy_size)
 {
 int ret = -ENOENT;
 
diff --git a/xen/xsm/flask/include/security.h b/xen/xsm/flask/include/security.h
index 1da020d..ec8b442 100644
--- a/xen/xsm/flask/include/security.h
+++ b/xen/xsm/flask/include/security.h
@@ -52,7 +52,7 @@ enum flask_bootparam_t {
 extern enum flask_bootparam_t flask_bootparam;
 extern int flask_mls_enabled;
 
-int security_load_policy(void * data, size_t len);
+int security_load_policy(const void *data, size_t len);
 
 struct av_decision {
 u32 allowed;
diff --git a/xen/xsm/flask/ss/policydb.h b/xen/xsm/flask/ss/policydb.h
index 238a042..d3b409a 100644
--- a/xen/xsm/flask/ss/policydb.h
+++ b/xen/xsm/flask/ss/policydb.h
@@ -277,7 +277,7 @@ extern int policydb_read(struct policydb *p, void *fp);
 #define TARGET_XEN_OLD 0
 
 struct policy_file {
-char *data;
+const char *data;
 size_t len;
 };
 
diff --git a/xen/xsm/flask/ss/services.c b/xen/xsm/flask/ss/services.c
index 86f94c9..b2c5c44 100644
--- a/xen/xsm/flask/ss/services.c
+++ b/xen/xsm/flask/ss/services.c
@@ -1353,7 +1353,7 @@ static int security_preserve_bools(struct policydb *p);
  * This function will flush the access vector cache after
  * loading the new policy.
  */
-int security_load_policy(void *data, size_t len)
+int security_load_policy(const void *data, size_t len)
 {
 struct policydb oldpolicydb, newpolicydb;
 struct sidtab oldsidtab, newsidtab;
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 8df1a3c..3d132be 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -36,7 +36,7 @@ static inline int verify(struct xsm_operations *ops)
 return 0;
 }
 
-static int __init xsm_core_init(void)
+static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 {
 if ( verify(_xsm_ops) )
 {
@@ -46,7 +46,7 @@ static int __init xsm_core_init(void)
 }
 
 xsm_ops = _xsm_ops;
-flask_init();
+flask_init(policy_buffer, policy_size);
 
 return 0;
 }
@@ -57,12 +57,15 @@ int __init xsm_multiboot_init(unsigned long *module_map,
   void *(*bootstrap_map)(const module_t *))
 {
 int ret = 0;
+void *policy_buffer = NULL;
+size_t policy_size = 0;
 
 printk("XSM Framework v" XSM_FRAMEWORK_VERSION " initialized\n");
 
 if ( XSM_MAGIC )
 {
-ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
+ret = xsm_multiboot_policy_init(module_map, mbi, 

[Xen-devel] [PATCH v5 2/2] xsm: add a default policy to .init.data

[Daniel De Graaf]

This adds a Kconfig option and support for including the XSM policy from
tools/flask/policy in the hypervisor so that the bootloader does not
need to provide a policy to get sane behavior from an XSM-enabled
hypervisor.  The policy provided by the bootloader, if present, will
override the built-in policy.

The XSM policy is not moved out of tools because that remains the
primary location for installing and configuring the policy.

Signed-off-by: Daniel De Graaf 
Reviewed-by: Konrad Rzeszutek Wilk 
---

Changes since v4:
 - Fixed clean target in xsm/flask/Makefile
 - Dropped now-unneeded const-dropping cast of policy_buffer

 Config.mk   |  6 ++
 INSTALL | 10 --
 docs/misc/xen-command-line.markdown | 16 +---
 docs/misc/xsm-flask.txt | 30 +++---
 xen/common/Kconfig  | 20 
 xen/include/xsm/xsm.h   |  5 +
 xen/xsm/flask/.gitignore|  1 +
 xen/xsm/flask/Makefile  | 13 -
 xen/xsm/flask/gen-policy.py | 23 +++
 xen/xsm/xsm_core.c  |  8 
 10 files changed, 107 insertions(+), 25 deletions(-)
 create mode 100644 xen/xsm/flask/.gitignore
 create mode 100644 xen/xsm/flask/gen-policy.py

diff --git a/Config.mk b/Config.mk
index 723e129..01316ae 100644
--- a/Config.mk
+++ b/Config.mk
@@ -147,6 +147,12 @@ export XEN_HAS_BUILD_ID=y
 build_id_linker := --build-id=sha1
 endif
 
+ifndef XEN_HAS_CHECKPOLICY
+CHECKPOLICY ?= checkpolicy
+XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && 
echo y || echo n)
+export XEN_HAS_CHECKPOLICY
+endif
+
 # as-insn: Check whether assembler supports an instruction.
 # Usage: cflags-y += $(call as-insn "insn",option-yes,option-no)
 as-insn = $(if $(shell echo 'void _(void) { asm volatile ( $(2) ); }' \
diff --git a/INSTALL b/INSTALL
index 616a67a..9759354 100644
--- a/INSTALL
+++ b/INSTALL
@@ -269,10 +269,16 @@ Building the python tools may fail unless certain options 
are passed to
 setup.py. Config.mk contains additional info how to use this variable.
 PYTHON_PREFIX_ARG=
 
-The hypervisor may be build with XSM/Flask support, which can be changed
+The hypervisor may be built with XSM/Flask support, which can be changed
 by running:
 make -C xen menuconfig
-and enabling XSM/Flask in the 'Common Features' menu.
+and enabling XSM/Flask in the 'Common Features' menu.  A security policy
+is required to use XSM/Flask; if the SELinux policy compiler is
+available, the policy from tools can be included in the hypervisor.
+This option is enabled by default if XSM is enabled and the compiler
+(checkpolicy) is found.  The location of this executable can be set
+using the environment variable.
+CHECKPOLICY=
 
 Do a build for coverage.
 coverage=y
diff --git a/docs/misc/xen-command-line.markdown 
b/docs/misc/xen-command-line.markdown
index 2a088ca..5500242 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -712,13 +712,15 @@ enabled by running either:
   with untrusted guests.  If a policy is provided by the bootloader, it will be
   loaded; errors will be reported to the ring buffer but will not prevent
   booting.  The policy can be changed to enforcing mode using "xl setenforce".
-* `enforcing`: This requires a security policy to be provided by the bootloader
-  and will enter enforcing mode prior to the creation of domain 0.  If a valid
-  policy is not provided, the hypervisor will not continue booting.
-* `late`: This disables loading of the security policy from the bootloader.
-  FLASK will be enabled but will not enforce access controls until a policy is
-  loaded by a domain using "xl loadpolicy".  Once a policy is loaded, FLASK 
will
-  run in enforcing mode unless "xl setenforce" has changed that setting.
+* `enforcing`: This will cause the security server to enter enforcing mode 
prior
+  to the creation of domain 0.  If an valid policy is not provided by the
+  bootloader and no built-in policy is present, the hypervisor will not 
continue
+  booting.
+* `late`: This disables loading of the built-in security policy or the policy
+  provided by the bootloader.  FLASK will be enabled but will not enforce 
access
+  controls until a policy is loaded by a domain using "xl loadpolicy".  Once a
+  policy is loaded, FLASK will run in enforcing mode unless "xl setenforce" has
+  changed that setting.
 * `disabled`: This causes the XSM framework to revert to the dummy module.  The
   dummy module provides the same security policy as is used when compiling the
   hypervisor without support for XSM.  The xsm\_op hypercall can also be used 
to
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 2f42585..62f15dd 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -141,21 +141,21 @@ only type enforcement 

Re: [Xen-devel] [PATCH 14/19] xen: credit2: add yet some more tracing

[George Dunlap]

On Sat, Jun 18, 2016 at 12:12 AM, Dario Faggioli
 wrote:
> (and fix the style of two labels as well.)
>
> Signed-off-by: Dario Faggioli 

Acked-by: George Dunlap 

> ---
> Cc: George Dunlap 
> Cc: Anshul Makkar 
> Cc: David Vrabel 
> ---
>  xen/common/sched_credit2.c |   58 
> +---
>  1 file changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
> index ba3a78a..e9f3f13 100644
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -46,6 +46,9 @@
>  #define TRC_CSCHED2_TICKLE_NEW   TRC_SCHED_CLASS_EVT(CSCHED2, 13)
>  #define TRC_CSCHED2_RUNQ_MAX_WEIGHT  TRC_SCHED_CLASS_EVT(CSCHED2, 14)
>  #define TRC_CSCHED2_MIGRATE  TRC_SCHED_CLASS_EVT(CSCHED2, 15)
> +#define TRC_CSCHED2_LOAD_CHECK   TRC_SCHED_CLASS_EVT(CSCHED2, 16)
> +#define TRC_CSCHED2_LOAD_BALANCE TRC_SCHED_CLASS_EVT(CSCHED2, 17)
> +#define TRC_CSCHED2_PICKED_CPU   TRC_SCHED_CLASS_EVT(CSCHED2, 19)
>
>  /*
>   * WARNING: This is still in an experimental phase.  Status and work can be 
> found at the
> @@ -709,6 +712,8 @@ update_load(const struct scheduler *ops,
>  struct csched2_runqueue_data *rqd,
>  struct csched2_vcpu *svc, int change, s_time_t now)
>  {
> +trace_var(TRC_CSCHED2_UPDATE_LOAD, 1, 0,  NULL);
> +
>  __update_runq_load(ops, rqd, change, now);
>  if ( svc )
>  __update_svc_load(ops, svc, change, now);
> @@ -1484,6 +1489,23 @@ csched2_cpu_pick(const struct scheduler *ops, struct 
> vcpu *vc)
>  out_up:
>  spin_unlock(>lock);
>
> +/* TRACE */
> +{
> +struct {
> +uint64_t b_avgload;
> +unsigned vcpu:16, dom:16;
> +unsigned rq_id:16, new_cpu:16;
> +   } d;
> +d.b_avgload = prv->rqd[min_rqi].b_avgload;
> +d.dom = vc->domain->domain_id;
> +d.vcpu = vc->vcpu_id;
> +d.rq_id = c2r(ops, new_cpu);
> +d.new_cpu = new_cpu;
> +trace_var(TRC_CSCHED2_PICKED_CPU, 1,
> +  sizeof(d),
> +  (unsigned char *));
> +}
> +
>  return new_cpu;
>  }
>
> @@ -1611,7 +1633,7 @@ static void balance_load(const struct scheduler *ops, 
> int cpu, s_time_t now)
>  bool_t inner_load_updated = 0;
>
>  balance_state_t st = { .best_push_svc = NULL, .best_pull_svc = NULL };
> -
> +
>  /*
>   * Basic algorithm: Push, pull, or swap.
>   * - Find the runqueue with the furthest load distance
> @@ -1677,6 +1699,20 @@ static void balance_load(const struct scheduler *ops, 
> int cpu, s_time_t now)
>  if ( i > cpus_max )
>  cpus_max = i;
>
> +/* TRACE */
> +{
> +struct {
> +unsigned lrq_id:16, orq_id:16;
> +unsigned load_delta;
> +} d;
> +d.lrq_id = st.lrqd->id;
> +d.orq_id = st.orqd->id;
> +d.load_delta = st.load_delta;
> +trace_var(TRC_CSCHED2_LOAD_CHECK, 1,
> +  sizeof(d),
> +  (unsigned char *));
> +}
> +
>  /*
>   * If we're under 100% capacaty, only shift if load difference
>   * is > 1.  otherwise, shift if under 12.5%
> @@ -1705,6 +1741,21 @@ static void balance_load(const struct scheduler *ops, 
> int cpu, s_time_t now)
>  if ( unlikely(st.orqd->id < 0) )
>  goto out_up;
>
> +/* TRACE */
> +{
> +struct {
> +uint64_t lb_avgload, ob_avgload;
> +unsigned lrq_id:16, orq_id:16;
> +} d;
> +d.lrq_id = st.lrqd->id;
> +d.lb_avgload = st.lrqd->b_avgload;
> +d.orq_id = st.orqd->id;
> +d.ob_avgload = st.orqd->b_avgload;
> +trace_var(TRC_CSCHED2_LOAD_BALANCE, 1,
> +  sizeof(d),
> +  (unsigned char *));
> +}
> +
>  now = NOW();
>
>  /* Look for "swap" which gives the best load average
> @@ -1756,10 +1807,9 @@ static void balance_load(const struct scheduler *ops, 
> int cpu, s_time_t now)
>  if ( st.best_pull_svc )
>  migrate(ops, st.best_pull_svc, st.lrqd, now);
>
> -out_up:
> + out_up:
>  spin_unlock(>lock);
> -
> -out:
> + out:
>  return;
>  }
>
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

Re: [Xen-devel] [PATCH] xen-blkback: really don't leak mode property

[Konrad Rzeszutek Wilk]

On Thu, Jul 07, 2016 at 11:17:14AM +0200, Roger Pau Monne wrote:
> On Thu, Jul 07, 2016 at 01:38:13AM -0600, Jan Beulich wrote:
> > Commit 9d092603cc ("xen-blkback: do not leak mode property") left one
> > path unfixed; correct this.
> > 
> > Signed-off-by: Jan Beulich 
> 
> Acked-by: Roger Pau Monné 

Queued.

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

Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a configuration option for ARM DomU ACPI

[Wei Liu]

On Mon, Jun 27, 2016 at 11:40:32AM +0100, Julien Grall wrote:
> Hi Shannon,
> 
> On 23/06/16 15:34, Shannon Zhao wrote:
> >On 2016年06月23日 21:39, Stefano Stabellini wrote:
> >>On Thu, 23 Jun 2016, Shannon Zhao wrote:
> From: Shannon Zhao 
> 
> Add a configuration option for ARM DomU so that user can deicde to use
> ACPI or not. This option is defaultly false.
> 
> Signed-off-by: Shannon Zhao 
> ---
>   tools/libxl/libxl_arm.c   | 3 +++
>   tools/libxl/libxl_types.idl   | 1 +
>   tools/libxl/xl_cmdimpl.c  | 4 
>   xen/include/public/arch-arm.h | 1 +
>   4 files changed, 9 insertions(+)
> 
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 8f15d9b..cc5a717 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -77,6 +77,9 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>   return ERROR_FAIL;
>   }
> 
> +xc_config->acpi = libxl_defbool_val(d_config->b_info.arch_arm.acpi)
> +  ? true : false;
> +
>   return 0;
>   }
> 
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index ef614be..426b868 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -560,6 +560,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
> 
> 
>   ("arch_arm", Struct(None, [("gic_version", libxl_gic_version),
> +   ("acpi", libxl_defbool),
> ])),
> 
>   ], dir=DIR_IN
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 6459eec..0634ffa 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2506,6 +2506,10 @@ skip_usbdev:
>   }
>    }
> 
> +if (xlu_cfg_get_defbool(config, "acpi", _info->arch_arm.acpi, 0)) {
> +libxl_defbool_set(_info->arch_arm.acpi, 0);
> +}
> >>We cannot share the existing code to parse the acpi paramter because
> >>that is saved in b_info->u.hvm.acpi, right?
> >Yes.
> >>It's a pity. I wonder if we
> >>could refactor the existing code so that we can actually share the acpi
> >>parameter between x86 and arm.
> >>
> >I have no idea about this since I'm not familiar with this. But is there
> >any downsides of current way? Because for x86, it will use
> >b_info->u.hvm.acpi and for ARM it will use b_info->arch_arm.acpi. I
> >think they don't conflict even though we store it at two places.
> 
> Yes, there is a downside.  Toolstack, such as libvirt, would need to have
> separate code for x86 and ARM in order to enable/disable ACPI.
> 
> I would introduce a new generic acpi parameters, deprecate
> b_info->u.hvm.acpi. Ian, Stefano, Wei, any opinions?
> 

Yeah, we can deprecate that field. But we need to take care to not break
users of the old field.

Wei.

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

Re: [Xen-devel] [PATCH v5 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Wei Liu]

On Thu, Jul 07, 2016 at 09:07:59AM -0600, Jan Beulich wrote:
> >>> On 07.07.16 at 16:55,  wrote:
> > Cc HV maintainers
> > 
> > I'm of course fine with moving this structure somewhere else.
> > 
> > On Wed, Jun 22, 2016 at 06:15:37PM +0100, Anthony PERARD wrote:
> > 
> >> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> > 
> > 
> > I suspect it would make more sense to move it to public/arch-x86/xen.h.
> 
> The question is whether we really mean this to remain x86 specific:
> The way it's now there's nothing inherently x86-ish there. But if
> that's the plan (which the conditional around it supports) then the
> suggested alternative resting place seems appropriate to me.
> 

For one, ARM doesn't distinguish PV vs HVM vs PVH (yet). Calling it HVM
for ARM would be wrong IMHO.

I've CC'ed Stefano and Julien to let them express their opinions.

Wei.

> Jan
> 

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

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

[Konrad Rzeszutek Wilk]

> > +{
> > +/* If bigger than 4GB, don't try to put under 4GB. */
> > +if ( is_64bar && bar_sz > (1ull<<32) )
> 
> Clearly at the very least this should be >=. However, even when
> it's a 2Gb BAR, we won't be able to fit it (as it can't go at address
> zero, nor at address 0x8000, both for different reasons).


.. snip..
> > @@ -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) )
> 
> Now that you use this constant a second time, it clearly needs to
> become a #define.

Do you have a preference on the ull? Most of the code has ull but there
are two instances of ULL?

> 
> Jan
> 

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

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

[Konrad Rzeszutek Wilk]

On Thu, Jul 07, 2016 at 11:28:18AM +0800, Wei Jiangang wrote:
> verify_local_APIC() had been removed by
> commit 4399c03c6780 ("x86/apic: Remove verify_local_APIC()"),
> so apic_id_mask isn't used by it.

CC-ing the proper maintainers.
> 
> 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 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

[Jan Beulich]

>>> On 07.07.16 at 17:12,  wrote:

> 
> On 07/07/16 16:08, Boris Ostrovsky wrote:
>> On 07/07/2016 04:38 AM, Jan Beulich wrote:
>> On 06.07.16 at 19:33,  wrote:
 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.
>>> I guess you both mean the same - note how Julien says _"real" RAM_.
>>> So from a hypervisor resource consumption view this ought to be
>>> considered RAM; from a guest view it wouldn't be.
>>
>> In which case we shouldn't pad maxmem specified by guest's config file.
>> Space to put ACPI tables must come from requested resources. If the
>> tables don't fit then more memory should have been asked for.
> 
> Why? In the case of ARM, the ACPI tables lives outside the guest RAM in 
> a special region. I would have expect to be the same in x86.

This is still RAM from a host resource accounting POV.

> If so, I don't think this should be part of the maxmem requested by the 
> user. IIRC, it is the same of the PCI ROM. The maxmem is incremented by 
> the toolstack.

For some parts iirc, and not for others. See also (for example) the
recent discussion George had with PGNet Dev
(https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg00353.html).

Jan


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

On Thu, Jul 07, 2016 at 12:26:09PM +0100, Andrew Cooper wrote:
> On 07/07/16 12:10, Lars Kurth wrote:
> >> On 6 Jul 2016, at 15:22, Konrad Rzeszutek Wilk  
> >> wrote:
> >>
> >>> 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).
> > That is good to know.
> >
> >>> - 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. 
> > @Ian, @George: Any other views regarding say osstest and the other 
> > osstest/* directories? 
> >
> > @Andrew: would something like test/xtf.git work
> 
> It would, although given a straight choice I would prefer
> xen-test-framework.git over its abbreviation.

+1

> 
> ~Andrew

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

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

[osstest service owner]

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

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  16b69afbba6b8692c96ec5a6864c35fa2fac1d36
baseline version:
 xen  db59dbc46e0312a38978d22c6bd72b554a2f1c91

Last test of basis96731  2016-07-06 18:06:43 Z0 days
Testing same since96766  2016-07-07 13:02:33 Z0 days1 attempts


People who touched revisions under test:
  Corneliu ZUZU 
  Razvan Cojocaru 
  Tamas K Lengyel 
  Tamas K Lengyel 

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=16b69afbba6b8692c96ec5a6864c35fa2fac1d36
+ . ./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 
16b69afbba6b8692c96ec5a6864c35fa2fac1d36
+ branch=xen-unstable-smoke
+ revision=16b69afbba6b8692c96ec5a6864c35fa2fac1d36
+ . ./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
+ '[' x16b69afbba6b8692c96ec5a6864c35fa2fac1d36 = 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();

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

[Julien Grall]

On 07/07/16 16:08, Boris Ostrovsky wrote:

On 07/07/2016 04:38 AM, Jan Beulich wrote:

On 06.07.16 at 19:33,  wrote:

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.

I guess you both mean the same - note how Julien says _"real" RAM_.
So from a hypervisor resource consumption view this ought to be
considered RAM; from a guest view it wouldn't be.


In which case we shouldn't pad maxmem specified by guest's config file.
Space to put ACPI tables must come from requested resources. If the
tables don't fit then more memory should have been asked for.


Why? In the case of ARM, the ACPI tables lives outside the guest RAM in 
a special region. I would have expect to be the same in x86.


If so, I don't think this should be part of the maxmem requested by the 
user. IIRC, it is the same of the PCI ROM. The maxmem is incremented by 
the toolstack.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v5 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Andrew Cooper]

On 07/07/16 16:09, Jan Beulich wrote:
 On 07.07.16 at 17:02,  wrote:
>> On 22/06/16 18:15, Anthony PERARD wrote:
>>> Instead of having several representation of hvm_start_info in C, define
>>> it in public/xen.h so both libxc and hvmloader can use it.
>>>
>>> Signed-off-by: Anthony PERARD 
>>> ---
>>> Change in V5:
>>> - remove packed attribute.
>>>
>>> New in V4.
>>> ---
>>>  tools/libxc/include/xc_dom.h | 31 ---
>>>  xen/include/public/xen.h | 27 +++
>> +1 to the public interface, but I would recommend introducing a new file
>> public/hvm/start_info.h rather than extending the general dumping ground
>> which is xen.h
> But then - see my other reply - perhaps public/arch-x86/hvm/start_info.h?

Fine by me.

~Andrew

___
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

[Jan Beulich]

>>> On 07.07.16 at 17:04,  wrote:
> On 07/07/2016 04:35 AM, Jan Beulich wrote:
> On 06.07.16 at 18:32,  wrote:
>>> On 07/06/2016 12:04 PM, Roger Pau Monné wrote:
 On Tue, Jul 05, 2016 at 03:04:59PM -0400, Boris Ostrovsky wrote:
> * 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.
>> So couldn't we switch to building v5 tables (or even v6) for PVH
>> (and perhaps re-using the "acpi=" config setting to allow specifying a
>> version - with any value above 1 indicating the requested version)? I
>> certainly agree that setting a v5 flag in a v2 table is bad (best we can
>> hope for is that any consumer would ignore such a flag).
> 
> Hmm... I thought earlier that v2 and v5(+) tables are different but now
> that I went back this doesn't seem to be the case. It's v1 vs v2 that
> changed formats.

Yeah, they didn't repeat that mistake again.

Jan

> If that's the case (I'll check more carefully) then yes, we can provide
> PVH guests with higher versions.
> 
> -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/07/2016 04:38 AM, Jan Beulich wrote:
 On 06.07.16 at 19:33,  wrote:
>> 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.
> I guess you both mean the same - note how Julien says _"real" RAM_.
> So from a hypervisor resource consumption view this ought to be
> considered RAM; from a guest view it wouldn't be.

In which case we shouldn't pad maxmem specified by guest's config file.
Space to put ACPI tables must come from requested resources. If the
tables don't fit then more memory should have been asked for.

-boris



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

Re: [Xen-devel] [PATCH v5 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Andrew Cooper]

On 22/06/16 18:15, Anthony PERARD wrote:
> Instead of having several representation of hvm_start_info in C, define
> it in public/xen.h so both libxc and hvmloader can use it.
>
> Signed-off-by: Anthony PERARD 
> ---
> Change in V5:
> - remove packed attribute.
>
> New in V4.
> ---
>  tools/libxc/include/xc_dom.h | 31 ---
>  xen/include/public/xen.h | 27 +++

+1 to the public interface, but I would recommend introducing a new file
public/hvm/start_info.h rather than extending the general dumping ground
which is xen.h

~Andrew

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

Re: [Xen-devel] [PATCH v5 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Jan Beulich]

>>> On 07.07.16 at 17:02,  wrote:
> On 22/06/16 18:15, Anthony PERARD wrote:
>> Instead of having several representation of hvm_start_info in C, define
>> it in public/xen.h so both libxc and hvmloader can use it.
>>
>> Signed-off-by: Anthony PERARD 
>> ---
>> Change in V5:
>> - remove packed attribute.
>>
>> New in V4.
>> ---
>>  tools/libxc/include/xc_dom.h | 31 ---
>>  xen/include/public/xen.h | 27 +++
> 
> +1 to the public interface, but I would recommend introducing a new file
> public/hvm/start_info.h rather than extending the general dumping ground
> which is xen.h

But then - see my other reply - perhaps public/arch-x86/hvm/start_info.h?

Jan


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

Re: [Xen-devel] [PATCH v5 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Jan Beulich]

>>> On 07.07.16 at 16:55,  wrote:
> Cc HV maintainers
> 
> I'm of course fine with moving this structure somewhere else.
> 
> On Wed, Jun 22, 2016 at 06:15:37PM +0100, Anthony PERARD wrote:
> 
>> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> 
> 
> I suspect it would make more sense to move it to public/arch-x86/xen.h.

The question is whether we really mean this to remain x86 specific:
The way it's now there's nothing inherently x86-ish there. But if
that's the plan (which the conditional around it supports) then the
suggested alternative resting place seems appropriate to me.

Jan


___
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/07/2016 04:35 AM, Jan Beulich wrote:
 On 06.07.16 at 18:32,  wrote:
>> On 07/06/2016 12:04 PM, Roger Pau Monné wrote:
>>> On Tue, Jul 05, 2016 at 03:04:59PM -0400, Boris Ostrovsky wrote:
 * 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.
> So couldn't we switch to building v5 tables (or even v6) for PVH
> (and perhaps re-using the "acpi=" config setting to allow specifying a
> version - with any value above 1 indicating the requested version)? I
> certainly agree that setting a v5 flag in a v2 table is bad (best we can
> hope for is that any consumer would ignore such a flag).

Hmm... I thought earlier that v2 and v5(+) tables are different but now
that I went back this doesn't seem to be the case. It's v1 vs v2 that
changed formats.

If that's the case (I'll check more carefully) then yes, we can provide
PVH guests with higher versions.

-boris



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

[Ian Jackson]

Ian Jackson writes ("Re: [Xen-devel] xenbits "official" repo for XTF (was Re: 
[PATCH 0/2] xtf: add launcher (+1 bugfix)"):
> David Vrabel writes ("Re: [Xen-devel] xenbits "official" repo for XTF (was 
> Re: [PATCH 0/2] xtf: add launcher (+1 bugfix)"):
> > Really?  Is it that difficult to accept that the original project author
> > gets to choose the name?
> 
> Yes.
> 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=614907#113

Sorry, I should have given the authoritative link:

https://lists.debian.org/debian-devel-announce/2012/07/msg2.html

> As I say, I think osstest has a far better claim to the name `Xen Test
> Framework'.  But I haven't called it that because I have an
> understanding that namespace landgrabs are antisocial.

Ian.

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

Re: [Xen-devel] [PATCH v4] xsm: add a default policy to .init.data

[Jan Beulich]

>>> On 07.07.16 at 16:44,  wrote:
> On 07/07/2016 06:30 AM, Jan Beulich wrote:
> On 05.07.16 at 19:44,  wrote:
>>> +static inline void xsm_policy_init(void)
>>> +{
>>> +#ifdef CONFIG_XSM_POLICY
>>> +if ( policy_size == 0 )
>>> +{
>>> +policy_buffer = (char*)xsm_init_policy;
>>
>> Can't xsm_init_policy by of type const char[] then, avoiding the need
>> for a cast (you certainly shouldn't be casting away constness)? If not,
>> besides adding the const please also add a blank before the *.
> 
> The policy_buffer global cannot be a const char* because it is passed to
> xfree() below (only in ARM); the cast would only be moved.  The buffer is
> never modified, if that's what you are asking.
> 
> The reason that xsm_init_policy is unsigned is to avoid compiler warnings
> resulting from assigning values such as 0xF3 to a signed character.

This is all ugly, but you're the maintainer, so you know best.

Jan


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

[Ian Jackson]

David Vrabel writes ("Re: [Xen-devel] xenbits "official" repo for XTF (was Re: 
[PATCH 0/2] xtf: add launcher (+1 bugfix)"):
> Really?  Is it that difficult to accept that the original project author
> gets to choose the name?

Yes.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=614907#113

As I say, I think osstest has a far better claim to the name `Xen Test
Framework'.  But I haven't called it that because I have an
understanding that namespace landgrabs are antisocial.

Ian.

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

Re: [Xen-devel] [PATCH v5 05/14] libxl: Load guest BIOS from file

[Wei Liu]

On Wed, Jun 22, 2016 at 06:15:36PM +0100, Anthony PERARD wrote:
> The path to the BIOS blob can be overriden by the xl's
> bios_path_override option, or provided by u.hvm.bios_firmware in the
> domain_build_info struct by other libxl user.
> 
> Signed-off-by: Anthony PERARD 
> 
> ---
> Changes in V5:
> - man page, use B<> to highlight config option in description.
> - rename config option from `bios_override` to `bios_path_override`
> - store libxl_read_file_contents() return value into r instead of e
>   (just renamed the variable)
> - rename domain_build_info.u.hvm.bios_firmware to system_firmware
> 
> Changes in V4:
> - updating man page to have bios_override described.
> - return ERROR_INVAL in libxl__load_hvm_firmware_module when the file is
>   empty.
> 
> Changes in V3:
> - move seabios_path and ovmf_path to libxl_path.c (with renaming)
> - fix some coding style
> - warn for empty file
> - remove rombios stuff (will still be built-in hvmloader)
> - rename field bios_filename in domain_build_info to bios_firmware to
>   follow naming of acpi and smbios.
> - log an error after libxl_read_file_contents() only when it return ENOENT
> - return an error on empty file.
> - added #define LIBXL_HAVE_BUILDINFO_HVM_BIOS_FIRMWARE
> ---
>  docs/man/xl.cfg.pod.5.in |  9 +++
>  tools/libxl/libxl.h  |  8 +++
>  tools/libxl/libxl_dom.c  | 57 
> 
>  tools/libxl/libxl_internal.h |  2 ++
>  tools/libxl/libxl_paths.c| 10 
>  tools/libxl/libxl_types.idl  |  1 +
>  tools/libxl/xl_cmdimpl.c | 11 ++---
>  7 files changed, 95 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in
> index 3bb27d0..a685b83 100644
> --- a/docs/man/xl.cfg.pod.5.in
> +++ b/docs/man/xl.cfg.pod.5.in
> @@ -1212,6 +1212,15 @@ Requires device_model_version=qemu-xen.
>  
>  =back
>  
> +=item 

Re: [Xen-devel] [PATCH v5 06/14] xen: Move the hvm_start_info C representation from libxc to public/xen.h

[Wei Liu]

Cc HV maintainers

I'm of course fine with moving this structure somewhere else.

On Wed, Jun 22, 2016 at 06:15:37PM +0100, Anthony PERARD wrote:

> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h


I suspect it would make more sense to move it to public/arch-x86/xen.h.

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

Re: [Xen-devel] [PATCH v5 04/14] firmware/makefile: install BIOS blob ...

[Wei Liu]

On Wed, Jun 22, 2016 at 06:15:35PM +0100, Anthony PERARD wrote:
> ... into the firmware directory, along with hvmloader.
> 
> Signed-off-by: Anthony PERARD 

Acked-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v5 02/14] libxc: Prepare a start info structure for hvmloader

[Wei Liu]

On Wed, Jun 22, 2016 at 06:15:33PM +0100, Anthony PERARD wrote:
[...]
>   * Allocate and clear additional ioreq server pages. The default
>   * server will use the IOREQ and BUFIOREQ special pages above.
> @@ -672,6 +675,14 @@ static int alloc_magic_pages_hvm(struct xc_dom_image 
> *dom)
>   NR_IOREQ_SERVER_PAGES);
>  }
>  
> +rc = xc_dom_alloc_segment(dom, >start_info_seg,
> +  "HVMlite start info", 0, start_info_size);

Should we just call this "HVM start info"? It is not restricted to
hvmlite anymore.

Wei.

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

Re: [Xen-devel] [PATCH v5 01/14] libxc: Rework extra module initialisation

[Wei Liu]

On Wed, Jun 22, 2016 at 06:15:32PM +0100, Anthony PERARD wrote:
> This patch use xc_dom_alloc_segment() to allocate the memory space for the
> ACPI modules and the SMBIOS modules. This is to replace the arbitrary
> placement of 1MB (+ extra for MB alignement) after the hvmloader image.
> 
> This patch can help if one add extra ACPI table and hvmloader contain
> OVMF (OVMF is a 2MB binary), as in that case the extra ACPI table could
> easily be loaded past the address 4MB, but hvmloader use a range of
> memory from 4MB to 10MB to perform tests and in the process, clears the
> memory, before loading the modules.
> 
> Signed-off-by: Anthony PERARD 
> ---
> Changes in V5:
> - rewrite patch description
> 
> Changes in V4:
> - check that guest_addr_out have a reasonable value.
> 
> New patch in V3.
> ---
>  tools/libxc/xc_dom_hvmloader.c | 131 
> -
>  1 file changed, 38 insertions(+), 93 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_hvmloader.c b/tools/libxc/xc_dom_hvmloader.c
> index 330d5e8..da8b995 100644
> --- a/tools/libxc/xc_dom_hvmloader.c
> +++ b/tools/libxc/xc_dom_hvmloader.c
> @@ -129,98 +129,52 @@ static elf_errorstatus xc_dom_parse_hvm_kernel(struct 
> xc_dom_image *dom)
>  return rc;
>  }
>  
> -static int modules_init(struct xc_dom_image *dom,
> -uint64_t vend, struct elf_binary *elf,
> -uint64_t *mstart_out, uint64_t *mend_out)
> +static int module_init_one(struct xc_dom_image *dom,
> +   struct xc_hvm_firmware_module *module,
> +   char *name)
>  {
> -#define MODULE_ALIGN 1UL << 7
> -#define MB_ALIGN 1UL << 20
> -#define MKALIGN(x, a) (((uint64_t)(x) + (a) - 1) & ~(uint64_t)((a) - 1))
> -uint64_t total_len = 0, offset1 = 0;
> +struct xc_dom_seg seg;
> +void *dest;
>  
> -if ( dom->acpi_module.length == 0 && dom->smbios_module.length == 0 )
> -return 0;
> -
> -/* Find the total length for the firmware modules with a reasonable large
> - * alignment size to align each the modules.
> - */
> -total_len = MKALIGN(dom->acpi_module.length, MODULE_ALIGN);
> -offset1 = total_len;
> -total_len += MKALIGN(dom->smbios_module.length, MODULE_ALIGN);
> -
> -/* Want to place the modules 1Mb+change behind the loader image. */
> -*mstart_out = MKALIGN(elf->pend, MB_ALIGN) + (MB_ALIGN);
> -*mend_out = *mstart_out + total_len;
> -
> -if ( *mend_out > vend )
> -return -1;
> -
> -if ( dom->acpi_module.length != 0 )
> -dom->acpi_module.guest_addr_out = *mstart_out;
> -if ( dom->smbios_module.length != 0 )
> -dom->smbios_module.guest_addr_out = *mstart_out + offset1;
> +if ( module->length )
> +{
> +if ( xc_dom_alloc_segment(dom, , name, 0, module->length) )
> +goto err;
> +dest = xc_dom_seg_to_ptr(dom, );
> +if ( dest == NULL )
> +{
> +DOMPRINTF("%s: xc_dom_seg_to_ptr(dom, ) => NULL",
> +  __FUNCTION__);
> +goto err;
> +}
> +memcpy(dest, module->data, module->length);
> +module->guest_addr_out = seg.vstart;
> +if ( module->guest_addr_out > UINT32_MAX ||
> + module->guest_addr_out + module->length > UINT32_MAX )
> +{
> +DOMPRINTF("%s: Module %s would be loaded abrove 4GB",
> +  __FUNCTION__, name);
> +goto err;
> +}

One question:

Can this check also account for MMIO hole below 4G? Maybe use
dom->mmio_size?

Other than this, this patch looks good.

Wei.

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

Re: [Xen-devel] [PATCH] XSM/policy: Allow the source domain access to settime and setdomainhandle domctls while creating domain.

[Wei Liu]

On Thu, Jul 07, 2016 at 10:49:01AM -0400, Daniel De Graaf wrote:
> On 07/07/2016 09:45 AM, anshul.makkar.anshul.mak...@citrix.com wrote:
> >From: Anshul Makkar 
> >
> >This patch resolves the following permission denied scenarios while creating
> >new domU :
> >avc:  denied  { setdomainhandle } for domid=0 target=1
> >scontext=system_u:system_r:dom0_t tcontext=system_u:system_r:domU_t 
> >tclass=domain
> >
> >avc:  denied  { settime } for domid=0 target=1 
> >scontext=system_u:system_r:dom0_t
> >tcontext=system_u:system_r:domU_t tclass=domain
> >
> >Signed-off-by: Anshul Makkar 
> 
> Acked-by: Daniel De Graaf 

Queued. Thank you both.

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

Re: [Xen-devel] [PATCH] XSM/policy: Allow the source domain access to settime and setdomainhandle domctls while creating domain.

[Daniel De Graaf]

On 07/07/2016 09:45 AM, anshul.makkar.anshul.mak...@citrix.com wrote:

From: Anshul Makkar 

This patch resolves the following permission denied scenarios while creating
new domU :
avc:  denied  { setdomainhandle } for domid=0 target=1
scontext=system_u:system_r:dom0_t tcontext=system_u:system_r:domU_t 
tclass=domain

avc:  denied  { settime } for domid=0 target=1 scontext=system_u:system_r:dom0_t
tcontext=system_u:system_r:domU_t tclass=domain

Signed-off-by: Anshul Makkar 


Acked-by: Daniel De Graaf 

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

Re: [Xen-devel] [PATCH v2] tools/hotplug: Add native systemd xendriverdomain.service

[Rusty Bird]

Wei Liu:
>> diff --git a/tools/configure.ac b/tools/configure.ac
>> index 8704927..e08fa8e 100644
>> --- a/tools/configure.ac
>> +++ b/tools/configure.ac
>> @@ -437,6 +437,7 @@ AS_IF([test "x$systemd" = "xy"], [
>>  hotplug/Linux/systemd/xenconsoled.service
>>  hotplug/Linux/systemd/xendomains.service
>>  hotplug/Linux/systemd/xenstored.service
>> +hotplug/Linux/systemd/xendriverdomain.service
> 
> I failed to mention that I would like to sort this list alphabetically,
> i.e. the new addition should be moved before xenstored.service.  I can
> make the adjustment while committing if you don't object.

Of course, thank you!

Rusty



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v4] xsm: add a default policy to .init.data

[Daniel De Graaf]

On 07/07/2016 06:30 AM, Jan Beulich wrote:

On 05.07.16 at 19:44,  wrote:

--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -762,6 +762,13 @@ static inline void flask_init(void)
 }
 #endif

+#ifdef CONFIG_XSM_POLICY
+extern const unsigned char xsm_init_policy[];
+extern const int xsm_init_policy_size;


unsigned int or size_t please.


--- a/xen/xsm/flask/Makefile
+++ b/xen/xsm/flask/Makefile
@@ -27,6 +27,17 @@ $(FLASK_H_FILES): $(FLASK_H_DEPEND)
 $(AV_H_FILES): $(AV_H_DEPEND)
$(CONFIG_SHELL) policy/mkaccess_vector.sh $(AWK) $(AV_H_DEPEND)

+obj-$(CONFIG_XSM_POLICY) += policy.o
+
+POLICY_SRC := $(XEN_ROOT)/tools/flask/policy/xenpolicy-$(XEN_FULLVERSION)
+
+policy.bin: FORCE
+   $(MAKE) -C $(XEN_ROOT)/tools/flask/policy
+   cmp -s $(POLICY_SRC) $@ || cp $(POLICY_SRC) $@
+
+policy.c: policy.bin gen-policy.py
+   $(PYTHON) gen-policy.py < $< > $@
+
 .PHONY: clean
 clean::
rm -f $(ALL_H_FILES) *.o $(DEPS)


I suppose the clean target then also needs adjustment?


Yes, it does.


+static inline void xsm_policy_init(void)
+{
+#ifdef CONFIG_XSM_POLICY
+if ( policy_size == 0 )
+{
+policy_buffer = (char*)xsm_init_policy;


Can't xsm_init_policy by of type const char[] then, avoiding the need
for a cast (you certainly shouldn't be casting away constness)? If not,
besides adding the const please also add a blank before the *.


The policy_buffer global cannot be a const char* because it is passed to
xfree() below (only in ARM); the cast would only be moved.  The buffer is
never modified, if that's what you are asking.

The reason that xsm_init_policy is unsigned is to avoid compiler warnings
resulting from assigning values such as 0xF3 to a signed character.

--
Daniel De Graaf
National Security Agency

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

Re: [Xen-devel] [PATCH v2] tools/hotplug: Add native systemd xendriverdomain.service

[Wei Liu]

On Thu, Jul 07, 2016 at 02:09:32PM +, Rusty Bird wrote:
> A dedicated Xen driver domain init service starts "xl devd" in domU. But
> currently, it is only supplied in the form of a SysV init script, which
> systemd users run through a backward compatiblity wrapper automatically
> generated by systemd-sysv-generator. This patch adds a (naturally more
> lightweight) native systemd unit to be used instead.
> 
> The xendriverdomain service is only relevant to domU, but should not run
> in dom0. Therefore, the systemd unit uses "ConditionVirtualization=xen",
> which evaluates to true in domU and (since systemd version 214, released
> on 2014-06-11) to false in dom0. Users or distributors who need to be
> compatible with even older systemd versions, but still want to prevent
> "xl devd" startup in dom0, could add the following line in [Service]:
> ExecStartPre=/bin/sh -c "! grep -q control_d /proc/xen/capabilities"
> 
> (Please rerun autogen.sh after applying this patch)
> 
> Signed-off-by: Rusty Bird 
> Cc: Ian Jackson 
> Cc: Wei Liu 

Thanks, the commit message is clearer now.

Acked-by: Wei Liu 

> ---
> Changed since v1:
>   * more detailed commit message
> 
>  tools/configure.ac |  1 +
>  tools/hotplug/Linux/systemd/Makefile   |  1 +
>  tools/hotplug/Linux/systemd/xendriverdomain.service.in | 14 ++
>  3 files changed, 16 insertions(+)
>  create mode 100644 tools/hotplug/Linux/systemd/xendriverdomain.service.in
> 
> diff --git a/tools/configure.ac b/tools/configure.ac
> index 8704927..e08fa8e 100644
> --- a/tools/configure.ac
> +++ b/tools/configure.ac
> @@ -437,6 +437,7 @@ AS_IF([test "x$systemd" = "xy"], [
>  hotplug/Linux/systemd/xenconsoled.service
>  hotplug/Linux/systemd/xendomains.service
>  hotplug/Linux/systemd/xenstored.service
> +hotplug/Linux/systemd/xendriverdomain.service

I failed to mention that I would like to sort this list alphabetically,
i.e. the new addition should be moved before xenstored.service.  I can
make the adjustment while committing if you don't object.

Wei.

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

Re: [Xen-devel] [PATCH] tools/hotplug: Add native systemd xendriverdomain.service

[Rusty Bird]

Wei Liu:
> While I understand it might be necessary to have a dedicated service
> file for xendriverdomain, I can't seem to be able to figure out the
> rationale from the commit message.
> 
> After thinking a bit harder, may I suggest commit message like this (and
> please correct me if I'm wrong):
> 
> We need to have a dedicated service for xendriverdoamin in driver
> domain. This patch creates a service file for it. This service is only
> relevant to DomU.
> 
> The service file uses ConditionVirtualization=xen becuase that evaluates
> to false in Dom0 while true in DomU, so that we only starts this service
> in DomU.

Thanks for the feedback! I've sent a v2 patch with a less cryptic commit
message that incorporates this information.

Rusty



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2] tools/hotplug: Add native systemd xendriverdomain.service

[Rusty Bird]

A dedicated Xen driver domain init service starts "xl devd" in domU. But
currently, it is only supplied in the form of a SysV init script, which
systemd users run through a backward compatiblity wrapper automatically
generated by systemd-sysv-generator. This patch adds a (naturally more
lightweight) native systemd unit to be used instead.

The xendriverdomain service is only relevant to domU, but should not run
in dom0. Therefore, the systemd unit uses "ConditionVirtualization=xen",
which evaluates to true in domU and (since systemd version 214, released
on 2014-06-11) to false in dom0. Users or distributors who need to be
compatible with even older systemd versions, but still want to prevent
"xl devd" startup in dom0, could add the following line in [Service]:
ExecStartPre=/bin/sh -c "! grep -q control_d /proc/xen/capabilities"

(Please rerun autogen.sh after applying this patch)

Signed-off-by: Rusty Bird 
Cc: Ian Jackson 
Cc: Wei Liu 
---
Changed since v1:
  * more detailed commit message

 tools/configure.ac |  1 +
 tools/hotplug/Linux/systemd/Makefile   |  1 +
 tools/hotplug/Linux/systemd/xendriverdomain.service.in | 14 ++
 3 files changed, 16 insertions(+)
 create mode 100644 tools/hotplug/Linux/systemd/xendriverdomain.service.in

diff --git a/tools/configure.ac b/tools/configure.ac
index 8704927..e08fa8e 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -437,6 +437,7 @@ AS_IF([test "x$systemd" = "xy"], [
 hotplug/Linux/systemd/xenconsoled.service
 hotplug/Linux/systemd/xendomains.service
 hotplug/Linux/systemd/xenstored.service
+hotplug/Linux/systemd/xendriverdomain.service
 hotplug/Linux/systemd/xenstored.socket
 hotplug/Linux/systemd/xenstored_ro.socket
 ])
diff --git a/tools/hotplug/Linux/systemd/Makefile 
b/tools/hotplug/Linux/systemd/Makefile
index 83e3b32..558e459 100644
--- a/tools/hotplug/Linux/systemd/Makefile
+++ b/tools/hotplug/Linux/systemd/Makefile
@@ -15,6 +15,7 @@ XEN_SYSTEMD_SERVICE += xen-qemu-dom0-disk-backend.service
 XEN_SYSTEMD_SERVICE += xendomains.service
 XEN_SYSTEMD_SERVICE += xen-watchdog.service
 XEN_SYSTEMD_SERVICE += xen-init-dom0.service
+XEN_SYSTEMD_SERVICE += xendriverdomain.service
 
 ALL_XEN_SYSTEMD =  $(XEN_SYSTEMD_MODULES)  \
$(XEN_SYSTEMD_MOUNT)\
diff --git a/tools/hotplug/Linux/systemd/xendriverdomain.service.in 
b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
new file mode 100644
index 000..c0cd454
--- /dev/null
+++ b/tools/hotplug/Linux/systemd/xendriverdomain.service.in
@@ -0,0 +1,14 @@
+[Unit]
+Description=Xen driver domain device daemon
+DefaultDependencies=no
+Requires=proc-xen.mount
+After=proc-xen.mount
+ConditionVirtualization=xen
+
+[Service]
+Type=forking
+ExecStart=@sbindir@/xl devd --pidfile=/var/run/xldevd.pid
+PIDFile=/var/run/xldevd.pid
+
+[Install]
+WantedBy=multi-user.target
-- 
2.5.5


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