[Xen-devel] [linux-4.1 test] 97644: regressions - FAIL

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
96211
 test-amd64-i386-freebsd10-amd64  9 freebsd-installfail REGR. vs. 96211
 test-amd64-i386-xl9 debian-installfail REGR. vs. 96211
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 96211
 test-amd64-i386-xl-qemut-winxpsp3  9 windows-install  fail REGR. vs. 96211
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 96211
 test-amd64-i386-xl-raw9 debian-di-install fail REGR. vs. 96211
 test-amd64-i386-libvirt   9 debian-installfail REGR. vs. 96211
 test-amd64-i386-freebsd10-i386  9 freebsd-install fail REGR. vs. 96211
 test-armhf-armhf-xl   9 debian-installfail REGR. vs. 96211
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 96211
 test-amd64-amd64-i386-pvgrub  6 xen-boot  fail REGR. vs. 96211
 test-amd64-i386-qemut-rhel6hvm-amd  9 redhat-install  fail REGR. vs. 96211
 test-armhf-armhf-xl-multivcpu  9 debian-install   fail REGR. vs. 96211
 test-amd64-i386-qemuu-rhel6hvm-amd  9 redhat-install  fail REGR. vs. 96211
 test-armhf-armhf-libvirt  9 debian-installfail REGR. vs. 96211
 test-amd64-i386-libvirt-xsm   9 debian-installfail REGR. vs. 96211
 test-armhf-armhf-libvirt-xsm  9 debian-installfail REGR. vs. 96211
 test-armhf-armhf-xl-cubietruck  9 debian-install  fail REGR. vs. 96211
 test-amd64-amd64-xl-qemut-debianhvm-amd64  6 xen-boot fail REGR. vs. 96211
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-bootfail REGR. vs. 96211
 test-amd64-amd64-xl-credit2   6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-qemuu-nested-intel  6 xen-boot   fail REGR. vs. 96211
 test-amd64-i386-xl-xsm9 debian-installfail REGR. vs. 96211
 test-armhf-armhf-xl-xsm   9 debian-installfail REGR. vs. 96211
 test-amd64-amd64-pygrub   6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-xl   6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  6 xen-boot fail REGR. vs. 96211
 test-amd64-amd64-xl-qcow2 6 xen-boot  fail REGR. vs. 96211
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 96211
 test-amd64-amd64-xl-pvh-amd   6 xen-boot  fail REGR. vs. 96211
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
96211
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 96211
 test-amd64-i386-qemuu-rhel6hvm-intel  9 redhat-installfail REGR. vs. 96211
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-bootfail REGR. vs. 96211
 test-amd64-amd64-xl-xsm   6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail REGR. 
vs. 96211
 test-amd64-amd64-libvirt-xsm  6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 96211
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-boot  fail REGR. vs. 96211
 test-armhf-armhf-xl-credit2   9 debian-installfail REGR. vs. 96211
 test-amd64-amd64-xl-multivcpu  6 xen-boot fail REGR. vs. 96211
 test-amd64-amd64-libvirt  6 xen-boot  fail REGR. vs. 96211
 test-amd64-i386-qemut-rhel6hvm-intel  9 redhat-installfail REGR. vs. 96211
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
96211
 test-amd64-amd64-xl-qemuu-win7-amd64  6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 96211
 test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-libvirt-vhd  6 xen-boot  fail REGR. vs. 96211
 test-amd64-amd64-amd64-pvgrub  6 xen-boot fail REGR. vs. 96211
 test-amd64-amd64-qemuu-nested-amd  6 xen-boot fail REGR. vs. 96211
 test-amd64-i386-xl-qemuu-winxpsp3  9 windows-install  fail REGR. vs. 96211
 test-armhf-armhf-xl-arndale   9 debian-installfail REGR. vs. 96211
 test-amd64-amd64-xl-pvh-intel  6 xen-boot fail REGR. vs. 96211
 test-amd64-i386-pair 15 debian-install/dst_host   fail REGR. vs. 96211
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail REGR. vs. 96211
 test-amd64-i386-libvirt-pair 15 debian-install/dst_host   fail REGR. vs. 96211
 test-armhf-armhf-libvirt-qcow2  9 debian-di-install   fail REGR. vs. 96211
 test-amd64-i386-xl-qemut-win7-amd64  9 windows-installfail REGR. vs. 96211
 test-amd64-amd64-pair  

Re: [Xen-devel] [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[Juergen Gross]

On 20/07/16 07:10, SF Markus Elfring wrote:
> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct 
> vscsibk_pend *pending_req,
>   tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>   if (!tmr) {
>   target_put_sess_cmd(se_cmd);
> - goto err;
> + goto do_resp;
>   }

 Hmm, I'm not convinced this is an improvement.

 I'd rather rename the new error label to "put_cmd" and get rid of the
 braces in above if statement:

 -  if (!tmr) {
 -  target_put_sess_cmd(se_cmd);
 -  goto err;
 -  }
 +  if (!tmr)
 +  goto put_cmd;

 and then in the error path:

 -err:
 +put_cmd:
 +  target_put_sess_cmd(se_cmd);
>>>
>>> I am unsure on the relevance of this function on such a source position.
>>> Would it make sense to move it further down at the end?
>>
>> You only want to call it in the first error case (allocation failure).
> 
> Thanks for your clarification.
> 
> I find that my update suggestion (from Saturday) is still appropriate
> in this case.
> https://lkml.org/lkml/2016/7/16/172

And I still think it isn't an improvement: Nack

 +free_tmr:
kfree(tmr);
>>>
>>> How do you think about to skip this function call after a memory
>>> allocation failure?
>>
>> I think this just doesn't matter. If it were a hot path, yes. But trying
>> to do micro-optimizations in an error path is just not worth the effort.
> 
> Would you like to reduce also the amount of function calls in such special
> run-time situations?

I just don't care for the extra 2 or 3 nsecs. Readability is more
important here.

>> I like a linear error path containing all the needed cleanups best.
> 
> I would prefer to keep the discussed single function call within
> the basic block of the if statement.
> 
> Have we got different opinions about the shown implementation details?

Yes.


Juergen


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

Re: [Xen-devel] [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[SF Markus Elfring]

 @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend 
 *pending_req,
tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
if (!tmr) {
target_put_sess_cmd(se_cmd);
 -  goto err;
 +  goto do_resp;
}
>>>
>>> Hmm, I'm not convinced this is an improvement.
>>>
>>> I'd rather rename the new error label to "put_cmd" and get rid of the
>>> braces in above if statement:
>>>
>>> -   if (!tmr) {
>>> -   target_put_sess_cmd(se_cmd);
>>> -   goto err;
>>> -   }
>>> +   if (!tmr)
>>> +   goto put_cmd;
>>>
>>> and then in the error path:
>>>
>>> -err:
>>> +put_cmd:
>>> +   target_put_sess_cmd(se_cmd);
>>
>> I am unsure on the relevance of this function on such a source position.
>> Would it make sense to move it further down at the end?
> 
> You only want to call it in the first error case (allocation failure).

Thanks for your clarification.

I find that my update suggestion (from Saturday) is still appropriate
in this case.
https://lkml.org/lkml/2016/7/16/172


>>> +free_tmr:
>>> kfree(tmr);
>>
>> How do you think about to skip this function call after a memory
>> allocation failure?
> 
> I think this just doesn't matter. If it were a hot path, yes. But trying
> to do micro-optimizations in an error path is just not worth the effort.

Would you like to reduce also the amount of function calls in such special
run-time situations?


> I like a linear error path containing all the needed cleanups best.

I would prefer to keep the discussed single function call within
the basic block of the if statement.

Have we got different opinions about the shown implementation details?

Regards,
Markus

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

Re: [Xen-devel] [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[Juergen Gross]

On 19/07/16 16:56, SF Markus Elfring wrote:
>>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend 
>>> *pending_req,
>>> tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>> if (!tmr) {
>>> target_put_sess_cmd(se_cmd);
>>> -   goto err;
>>> +   goto do_resp;
>>> }
>>
>> Hmm, I'm not convinced this is an improvement.
>>
>> I'd rather rename the new error label to "put_cmd" and get rid of the
>> braces in above if statement:
>>
>> -if (!tmr) {
>> -target_put_sess_cmd(se_cmd);
>> -goto err;
>> -}
>> +if (!tmr)
>> +goto put_cmd;
>>
>> and then in the error path:
>>
>> -err:
>> +put_cmd:
>> +target_put_sess_cmd(se_cmd);
> 
> I am unsure on the relevance of this function on such a source position.
> Would it make sense to move it further down at the end?

You only want to call it in the first error case (allocation failure).

>> +free_tmr:
>>  kfree(tmr);
> 
> How do you think about to skip this function call after a memory
> allocation failure?

I think this just doesn't matter. If it were a hot path, yes. But trying
to do micro-optimizations in an error path is just not worth the effort.

I like a linear error path containing all the needed cleanups best.


Juergen

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

Re: [Xen-devel] [DRAFT v2] XenSock protocol design document

[Juergen Gross]

On 20/07/16 00:38, Stefano Stabellini wrote:
> On Fri, 15 Jul 2016, Paul Durrant wrote:
>>> -Original Message-
>>> From: Juergen Gross [mailto:jgr...@suse.com]
>>> Sent: 15 July 2016 12:37
>>> To: Stefano Stabellini; xen-de...@lists.xenproject.org
>>> Cc: joao.m.mart...@oracle.com; Wei Liu; Roger Pau Monne; Lars Kurth;
>>> boris.ostrov...@oracle.com; Paul Durrant
>>> Subject: Re: [DRAFT v2] XenSock protocol design document
>>>
>>> On 13/07/16 17:47, Stefano Stabellini wrote:
 Hi all,

 This is the design document of the XenSock protocol. You can find
 prototypes of the Linux frontend and backend drivers here:
>>> ...
 ### Commands Ring

 The shared ring is used by the frontend to forward socket API calls to the
 backend. I'll refer to this ring as **commands ring** to distinguish it 
 from
 other rings which will be created later in the lifecycle of the protocol 
 (data
 rings). The ring format is defined using the familiar `DEFINE_RING_TYPES`
>>> macro
 (`xen/include/public/io/ring.h`). Frontend requests are allocated on the
>>> ring
 using the `RING_GET_REQUEST` macro.

 The format is defined as follows:

 #define XENSOCK_SOCKET 0
 #define XENSOCK_CONNECT1
 #define XENSOCK_RELEASE2
 #define XENSOCK_BIND   3
 #define XENSOCK_LISTEN 4
 #define XENSOCK_ACCEPT 5
 #define XENSOCK_POLL   6

 struct xen_xensock_request {
uint32_t id; /* private to guest, echoed in response */
uint32_t cmd; /* command to execute */
uint64_t sockid;
union {
struct xen_xensock_socket {
uint32_t domain;
uint32_t type;
uint32_t protocol;
} socket;
struct xen_xensock_connect {
uint8_t addr[28];
uint32_t len;
uint32_t flags;
grant_ref_t ref;
uint32_t evtchn;
} connect;
struct xen_xensock_bind {
uint8_t addr[28];
uint32_t len;
} bind;
struct xen_xensock_listen {
uint32_t backlog;
} listen;
struct xen_xensock_accept {
uint64_t sockid;
grant_ref_t ref;
uint32_t evtchn;
} accept;
} u;
 };
>>>
>>> Please add padding at the end (or a dummy union member) to make sure
>>> 32- and 64-bit variants have the same size (I believe now the size will
>>> be 60 bytes on 32-bit system and 64 bytes on 64-bit).
> 
> Well spotted! You have a point, I think you are right, even though it
> makes the struct a bit awkward.

Why awkward? just add a "uint8_t dummy[48];" to u.


Juergen

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

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

[osstest service owner]

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

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 9ba25c7db7e918c3c911dd20641ba54ce721e872
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   56 days
Failing since 94750  2016-05-25 03:43:08 Z   55 days  119 attempts
Testing same since97653  2016-07-19 09:40:21 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 
  Hegde, Nagaraj P 
  hegdenag 
  Heyi Guo 
  Jan D?bro? 
  Jan Dabros 
  Jeff Fan 
  Jeremy Linton 
  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 
  Mudusuru, Giri P 
  Ni, Ruiyu 
  Qiu Shumin 
  Ruiyu Ni 
  Ruiyu Ni　
  Ryan Harkin 
  Sami Mujawar 
  Satya Yarlagadda 
  Shannon Zhao 
  Sriram Subramanian 
  Star Zeng 
  Subramanian, Sriram (EG Servers Platform SW) 
  Sunny Wang 
  Tapan Shah 
  Thomas Palmer 
  Yarlagadda, Satya P 
  Yonghong Zhu 
  Zhang Lubo 
  Zhang, Chao B 
  Zhang, Lubo 

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 10838 lines long.)

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

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

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-i386-xl-qemut-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
96188
 test-amd64-amd64-pygrub   6 xen-boot  fail REGR. vs. 96188
 test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 96188
 test-amd64-i386-xl-qemut-win7-amd64  9 windows-installfail REGR. vs. 96188
 test-armhf-armhf-libvirt  9 debian-installfail REGR. vs. 96188
 test-amd64-i386-xl-qemut-winxpsp3  9 windows-install  fail REGR. vs. 96188
 test-amd64-amd64-amd64-pvgrub  6 xen-boot fail REGR. vs. 96188
 test-amd64-i386-freebsd10-amd64  9 freebsd-installfail REGR. vs. 96188
 test-amd64-i386-qemut-rhel6hvm-amd  9 redhat-install  fail REGR. vs. 96188
 test-amd64-i386-xl-qemut-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 96188
 test-amd64-i386-xl-xsm9 debian-installfail REGR. vs. 96188
 test-armhf-armhf-libvirt-qcow2  9 debian-di-install   fail REGR. vs. 96188
 test-amd64-i386-qemuu-rhel6hvm-amd  9 redhat-install  fail REGR. vs. 96188
 test-amd64-i386-qemut-rhel6hvm-intel  9 redhat-installfail REGR. vs. 96188
 test-amd64-amd64-xl-qemuu-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 96188
 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 96188
 test-amd64-amd64-xl-qemut-winxpsp3  6 xen-bootfail REGR. vs. 96188
 test-amd64-amd64-i386-pvgrub  6 xen-boot  fail REGR. vs. 96188
 test-amd64-amd64-xl-multivcpu  6 xen-boot fail REGR. vs. 96188
 test-amd64-amd64-qemuu-nested-intel  6 xen-boot   fail REGR. vs. 96188
 test-amd64-amd64-xl   6 xen-boot  fail REGR. vs. 96188
 test-amd64-i386-xl-qemuu-winxpsp3-vcpus1 9 windows-install fail REGR. vs. 96188
 test-amd64-amd64-libvirt-xsm  6 xen-boot  fail REGR. vs. 96188
 test-amd64-amd64-libvirt  6 xen-boot  fail REGR. vs. 96188
 test-armhf-armhf-xl-multivcpu  9 debian-install   fail REGR. vs. 96188
 test-amd64-i386-xl-qemuu-win7-amd64  9 windows-installfail REGR. vs. 96188
 test-amd64-i386-libvirt   9 debian-installfail REGR. vs. 96188
 test-amd64-amd64-xl-qemut-debianhvm-amd64-xsm  6 xen-boot fail REGR. vs. 96188
 test-amd64-amd64-libvirt-vhd  6 xen-boot  fail REGR. vs. 96188
 test-amd64-i386-xl-raw9 debian-di-install fail REGR. vs. 96188
 test-amd64-i386-qemuu-rhel6hvm-intel  9 redhat-installfail REGR. vs. 96188
 test-amd64-i386-libvirt-xsm   9 debian-installfail REGR. vs. 96188
 test-amd64-amd64-xl-qemut-stubdom-debianhvm-amd64-xsm 6 xen-boot fail REGR. 
vs. 96188
 test-amd64-amd64-xl-qemuu-debianhvm-amd64  6 xen-boot fail REGR. vs. 96188
 test-amd64-amd64-xl-pvh-amd   6 xen-boot  fail REGR. vs. 96188
 test-amd64-amd64-xl-qemuu-ovmf-amd64  6 xen-boot  fail REGR. vs. 96188
 test-amd64-amd64-xl-qemut-win7-amd64  6 xen-boot  fail REGR. vs. 96188
 test-amd64-amd64-xl-xsm   6 xen-boot  fail REGR. vs. 96188
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 6 xen-boot fail REGR. vs. 
96188
 test-amd64-amd64-xl-credit2   6 xen-boot  fail REGR. vs. 96188
 test-armhf-armhf-xl-cubietruck  9 debian-install  fail REGR. vs. 96188
 test-amd64-amd64-xl-qemuu-win7-amd64  6 xen-boot  fail REGR. vs. 96188
 test-amd64-i386-freebsd10-i386  9 freebsd-install fail REGR. vs. 96188
 test-amd64-i386-xl9 debian-installfail REGR. vs. 96188
 test-amd64-amd64-xl-qemut-debianhvm-amd64  6 xen-boot fail REGR. vs. 96188
 test-amd64-amd64-xl-qemuu-winxpsp3  6 xen-bootfail REGR. vs. 96188
 test-amd64-amd64-xl-qcow2 6 xen-boot  fail REGR. vs. 96188
 test-amd64-i386-xl-qemuu-ovmf-amd64  9 debian-hvm-install fail REGR. vs. 96188
 test-armhf-armhf-libvirt-xsm  9 debian-installfail REGR. vs. 96188
 test-armhf-armhf-xl-vhd   9 debian-di-install fail REGR. vs. 96188
 test-amd64-amd64-qemuu-nested-amd  6 xen-boot fail REGR. vs. 96188
 test-amd64-i386-xl-qemuu-debianhvm-amd64 9 debian-hvm-install fail REGR. vs. 
96188
 test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 9 debian-hvm-install fail REGR. 
vs. 96188
 test-armhf-armhf-xl-credit2   9 debian-installfail REGR. vs. 96188
 test-amd64-i386-xl-qemuu-winxpsp3  9 windows-install  fail REGR. vs. 96188
 test-armhf-armhf-xl   9 debian-installfail REGR. vs. 96188
 test-armhf-armhf-xl-arndale   9 debian-installfail REGR. vs. 96188
 test-amd64-i386-libvirt-pair 15 debian-install/dst_host   fail REGR. vs. 96188
 test-amd64-amd64-pair 9 xen-boot/src_host fail REGR. vs. 96188
 

Re: [Xen-devel] [PATCH] acpi: Re-license ACPI builder files from GPLv2 to LGPLv2.1

[Kouya Shimura]

Stefan Berger  writes:

> Daniel Kiper  wrote on 07/19/2016 11:00:04 AM:
>
>> Subject: Re: [PATCH] acpi: Re-license ACPI builder files from GPLv2 
>> to LGPLv2.1
>> 
>> On Mon, Jul 18, 2016 at 10:01:27AM -0400, Boris Ostrovsky wrote:
>> > ACPI builder is currently distributed under GPLv2 license.
>> >
>> > We plan to make the builder available to components other
>> > than the hvmloader (which is also GPLv2). Some of these
>> > components (such as libxl) may be distributed under LGPL-2.1
>> > so that they can be used by non-GPLv2 callers.  But this
>> > will not be possible if we incorporate the ACPI builder in
>> > those other components.
>> >
>> > To avoid this problem we are relicensing sources in ACPI
>> > bulder directory to the Lesser GNU Public License (LGPL)
>> > version 2.1
>> >
>> > Signed-off-by: Boris Ostrovsky 
>> > CC: Kouya Shimura 
>> > CC: Daniel Kiper 
>> > CC: Stefan Berger 
>> > CC: Simon Horman 
>> > CC: Keir Fraser 
>> > CC: Ian Jackson 
>> > CC: Lars Kurth 
>> 
>> Acked-by: Daniel Kiper 
> Acked-by: Stefan Berger 
Acked-by: Kouya Shimura 

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

[Xen-devel] [xen-unstable baseline-only test] 66616: regressions - FAIL

[Platform Team regression test user]

This run is configured for baseline tests only.

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

Regressions :-(

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

Regressions which are regarded as allowable (not blocking):
 build-amd64-rumpuserxen   6 xen-buildfail   like 66611
 build-i386-rumpuserxen6 xen-buildfail   like 66611
 test-amd64-i386-xl-qemuu-win7-amd64 16 guest-stop  fail like 66611
 test-amd64-amd64-i386-pvgrub 10 guest-start  fail   like 66611
 test-amd64-amd64-qemuu-nested-intel 16 debian-hvm-install/l1/l2 fail like 66611

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

version targeted for testing:
 xen  e763268781d341fef05d461f3057e6ced5e033f2
baseline version:
 xen  b48be35ac86cd6369124cf06ca3006d086095297

Last test of basis66611  2016-07-16 17:19:41 Z3 days
Testing same since66616  2016-07-19 15:16:34 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Dario Faggioli 
  George Dunlap 
  Wei Liu 

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

Re: [Xen-devel] [PATCH v2 8/9] xen/arm: acpi: route all unused IRQs to DOM0

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> It is not possible to know which IRQs will be used by DOM0 when ACPI is
> inuse. The approach implemented by this patch, will route all unused
> IRQs to DOM0 before it has booted.
> 
> The number of IRQs routed is based on the maximum SPIs supported by the
> hardware (up to ~1000). However, some of them might not be wired. So we
> would allocate resource for nothing.
> 
> For each IRQ routed, Xen is allocating memory for irqaction (40 bytes)
> and irq_guest (16 bytes). So in the worst case scenario ~54KB of memory
> will be allocated. Given that ACPI will mostly be used by server, I
> think it is a small drawback.
> 
> map_irq_to_domain is slightly reworked to remove the dependency on
> device-tree. So the function can be also be used for ACPI and will
> avoid code duplication.
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Stefano Stabellini 


> ---
> Changes in v2:
> - Rename acpi_permit_spi_access to acpi_route_spis
> - Update the comment in the function acpi_route_spis
> ---
>  xen/arch/arm/domain_build.c | 28 
>  1 file changed, 12 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 60db9e4..5b2f8ad 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -903,11 +903,10 @@ static int make_timer_node(const struct domain *d, void 
> *fdt,
>  return res;
>  }
>  
> -static int map_irq_to_domain(const struct dt_device_node *dev,
> - struct domain *d, unsigned int irq)
> +static int map_irq_to_domain(struct domain *d, unsigned int irq,
> + bool_t need_mapping, const char *devname)
>  
>  {
> -bool_t need_mapping = !dt_device_for_passthrough(dev);
>  int res;
>  
>  res = irq_permit_access(d, irq);
> @@ -927,7 +926,7 @@ static int map_irq_to_domain(const struct dt_device_node 
> *dev,
>   */
>  vgic_reserve_virq(d, irq);
>  
> -res = route_irq_to_guest(d, irq, irq, dt_node_name(dev));
> +res = route_irq_to_guest(d, irq, irq, devname);
>  if ( res < 0 )
>  {
>  printk(XENLOG_ERR "Unable to map IRQ%"PRId32" to dom%d\n",
> @@ -947,6 +946,7 @@ static int map_dt_irq_to_domain(const struct 
> dt_device_node *dev,
>  struct domain *d = data;
>  unsigned int irq = dt_irq->irq;
>  int res;
> +bool_t need_mapping = !dt_device_for_passthrough(dev);
>  
>  if ( irq < NR_LOCAL_IRQS )
>  {
> @@ -965,7 +965,7 @@ static int map_dt_irq_to_domain(const struct 
> dt_device_node *dev,
>  return res;
>  }
>  
> -res = map_irq_to_domain(dev, d, irq);
> +res = map_irq_to_domain(d, irq, need_mapping, dt_node_name(dev));
>  
>  return 0;
>  }
> @@ -1103,7 +1103,7 @@ static int handle_device(struct domain *d, struct 
> dt_device_node *dev)
>  return res;
>  }
>  
> -res = map_irq_to_domain(dev, d, res);
> +res = map_irq_to_domain(d, res, need_mapping, dt_node_name(dev));
>  if ( res )
>  return res;
>  }
> @@ -1343,15 +1343,14 @@ static int acpi_iomem_deny_access(struct domain *d)
>  return gic_iomem_deny_access(d);
>  }
>  
> -static int acpi_permit_spi_access(struct domain *d)
> +static int acpi_route_spis(struct domain *d)
>  {
>  int i, res;
>  struct irq_desc *desc;
>  
>  /*
> - * Here just permit Dom0 to access the SPIs which Xen doesn't use. Then 
> when
> - * Dom0 configures the interrupt, set the interrupt type and route it to
> - * Dom0.
> + * Route the IRQ to hardware domain and permit the access.
> + * The interrupt type will be set by set by the hardware domain.
>   */
>  for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
>  {
> @@ -1362,13 +1361,10 @@ static int acpi_permit_spi_access(struct domain *d)
>  if ( desc->action != NULL)
>  continue;
>  
> -res = irq_permit_access(d, i);
> +/* XXX: Shall we use a proper devname? */
> +res = map_irq_to_domain(d, i, true, "ACPI");
>  if ( res )
> -{
> -printk(XENLOG_ERR "Unable to permit to dom%u access to IRQ %u\n",
> -   d->domain_id, i);
>  return res;
> -}
>  }
>  
>  return 0;
> @@ -1902,7 +1898,7 @@ static int prepare_acpi(struct domain *d, struct 
> kernel_info *kinfo)
>  if ( rc != 0 )
>  return rc;
>  
> -rc = acpi_permit_spi_access(d);
> +rc = acpi_route_spis(d);
>  if ( rc != 0 )
>  return rc;
>  
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [PATCH v2 9/9] xen/arm: Fix coding style and update comment in acpi_route_spis

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> The comment was not correctly indented. Also the preferred name for the
> initial domain is "hardware domain" and not "dom0, so replace it.
> 
> Signed-off-by: Julien Grall 

Acked-by: Stefano Stabellini 


> ---
> Changes in v2:
> - Patch added
> ---
>  xen/arch/arm/domain_build.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5b2f8ad..35ab08d 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1355,8 +1355,9 @@ static int acpi_route_spis(struct domain *d)
>  for( i = NR_LOCAL_IRQS; i < vgic_num_irqs(d); i++ )
>  {
>  /*
> -  * TODO: Exclude the SPIs SMMU uses which should not be routed to Dom0.
> -  */
> + * TODO: Exclude the SPIs SMMU uses which should not be routed to
> + * the hardware domain.
> + */
>  desc = irq_to_desc(i);
>  if ( desc->action != NULL)
>  continue;
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [PATCH v2 7/9] xen/arm: Allow DOM0 to set the IRQ type

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> The function route_irq_to_guest mandates the IRQ type, stored in
> desc->arch.type, to be valid. However, in case of ACPI, these
> information is not part of the static tables. Therefore Xen needs to
> rely on DOM0 to provide a valid type based on the firmware tables.
> 
> A new helper, irq_type_set_by_domain is provided to check whether a
> domain is allowed to set the IRQ type. For now, only DOM0 is allowed to
> configure.
> 
> When the helper returns 1, the routing function will not check whether
> the IRQ type is correctly set and configure the GIC. Instead, this will
> be done when the domain will enable the interrupt.
> 
> Note that irq_set_spi_type is not called because it validates the type
> and does not allow it the domain to change the type after the first
> write. It means that desc->arch.type may never be set, which is fine
> because the field is only used to configure the type during the routing.
> 
> Based on 4.3.13 in ARM IHI 0048B.b, changing the value of Int_config is
> UNPREDICTABLE when the corresponding interrupt is not disabled.
> 
> Therefore, setting the IRQ type when the guest is writing into ICFGR
> would require more work to make sure the IRQ has been disabled before
> writing into the host ICFGR. As the behavior is UNPREDICTABLE, the type
> will be set before enabling the physical IRQ associated to the virtual IRQ.
> 
> Signed-off-by: Julien Grall 
> 
> ---
> 
> It might be possible to let any domain configure the IRQ
> type (could be useful when passthrough an IRQ with ACPI). However, we would
> need to consider any potential security impact beforehand.
> 
> Changes in v2:
> - Rename the patch
> - Allow any DOM0 to set the IRQ type
> - Re-use in part of vgic_get_virq_type from
> "Configure SPI interrupt type and route to Dom0 dynamically".
> - Add rationale why the IRQ type is set in enable
> ---
>  xen/arch/arm/gic.c|  5 +++--
>  xen/arch/arm/irq.c| 13 -
>  xen/arch/arm/vgic.c   | 19 +++
>  xen/include/asm-arm/gic.h |  3 +++
>  xen/include/asm-arm/irq.h |  6 ++
>  5 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 72bb885..63c744a 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -97,7 +97,7 @@ void gic_restore_state(struct vcpu *v)
>  }
>  
>  /* desc->irq needs to be disabled before calling this function */
> -static void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
> +void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>  /*
>   * IRQ must be disabled before configuring it (see 4.3.13 in ARM IHI
> @@ -160,7 +160,8 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>  desc->handler = gic_hw_ops->gic_guest_irq_type;
>  set_bit(_IRQ_GUEST, >status);
>  
> -gic_set_irq_type(desc, desc->arch.type);
> +if ( !irq_type_set_by_domain(d) )
> +gic_set_irq_type(desc, desc->arch.type);
>  gic_set_irq_priority(desc, priority);
>  
>  p->desc = desc;
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 3fc22f2..06d4843 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -395,6 +395,17 @@ bool_t is_assignable_irq(unsigned int irq)
>  }
>  
>  /*
> + * Only the hardware domain is allowed to set the configure the
> + * interrupt type for now.
> + *
> + * XXX: See whether it is possible to let any domain configure the type.
> + */
> +bool_t irq_type_set_by_domain(const struct domain *d)
> +{
> +return (d == hardware_domain);
> +}
> +
> +/*
>   * Route an IRQ to a specific guest.
>   * For now only SPIs are assignable to the guest.
>   */
> @@ -449,7 +460,7 @@ int route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>  
>  spin_lock_irqsave(>lock, flags);
>  
> -if ( desc->arch.type == IRQ_TYPE_INVALID )
> +if ( !irq_type_set_by_domain(d) && desc->arch.type == IRQ_TYPE_INVALID )
>  {
>  printk(XENLOG_G_ERR "IRQ %u has not been configured\n", irq);
>  retval = -EIO;
> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> index 5070452..a7ccfe7 100644
> --- a/xen/arch/arm/vgic.c
> +++ b/xen/arch/arm/vgic.c
> @@ -344,6 +344,22 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n)
>  }
>  }
>  
> +#define VGIC_ICFG_MASK(intr) (1 << ((2 * ((intr) % 16)) + 1))
> +
> +/* The function should be called with the rank lock taken */
> +static inline unsigned int vgic_get_virq_type(struct vcpu *v, int n, int 
> index)
> +{
> +struct vgic_irq_rank *r = vgic_get_rank(v, n);
> +uint32_t tr = r->icfg[index >> 4];
> +
> +ASSERT(spin_is_locked(>lock));
> +
> +if ( tr & VGIC_ICFG_MASK(index) )
> +return IRQ_TYPE_EDGE_RISING;
> +else
> +return IRQ_TYPE_LEVEL_HIGH;
> +}
> +
>  void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
>  {
>  const unsigned long mask = r;

Re: [Xen-devel] [PATCH v2 4/9] xen/arm: gic: set_type: Pass the type in parameter rather than in desc->arch.type

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> A follow-up patch will not store the type in desc->arch.type. Also, the
> callback prototype is more logical.
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Stefano Stabellini 

> ---
> Changes in v2:
> - gic_set_irq_type has been dropped by mistake in
> gic_route_irq_to_xen. Re-add it!
> ---
>  xen/arch/arm/gic-v2.c |  3 +--
>  xen/arch/arm/gic-v3.c |  3 +--
>  xen/arch/arm/gic.c| 10 +-
>  xen/include/asm-arm/gic.h |  4 ++--
>  4 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 69ed72d..9bd9d0b 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -236,11 +236,10 @@ static unsigned int gicv2_read_irq(void)
>  return (readl_gicc(GICC_IAR) & GICC_IA_IRQ);
>  }
>  
> -static void gicv2_set_irq_type(struct irq_desc *desc)
> +static void gicv2_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>  uint32_t cfg, actual, edgebit;
>  unsigned int irq = desc->irq;
> -unsigned int type = desc->arch.type;
>  
>  spin_lock();
>  /* Set edge / level */
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 781f25c..b8be395 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -471,12 +471,11 @@ static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
>   MPIDR_AFFINITY_LEVEL(mpidr, 0));
>  }
>  
> -static void gicv3_set_irq_type(struct irq_desc *desc)
> +static void gicv3_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>  uint32_t cfg, actual, edgebit;
>  void __iomem *base;
>  unsigned int irq = desc->irq;
> -unsigned int type = desc->arch.type;
>  
>  /* SGI's are always edge-triggered not need to call GICD_ICFGR0 */
>  ASSERT(irq >= NR_GIC_SGI);
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index c63c862..b9371a7 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -96,12 +96,12 @@ void gic_restore_state(struct vcpu *v)
>  gic_restore_pending_irqs(v);
>  }
>  
> -static void gic_set_irq_type(struct irq_desc *desc)
> +static void gic_set_irq_type(struct irq_desc *desc, unsigned int type)
>  {
>  ASSERT(spin_is_locked(>lock));
> -ASSERT(desc->arch.type != IRQ_TYPE_INVALID);
> +ASSERT(type != IRQ_TYPE_INVALID);
>  
> -gic_hw_ops->set_irq_type(desc);
> +gic_hw_ops->set_irq_type(desc, type);
>  }
>  
>  static void gic_set_irq_priority(struct irq_desc *desc, unsigned int 
> priority)
> @@ -121,7 +121,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, unsigned 
> int priority)
>  
>  desc->handler = gic_hw_ops->gic_host_irq_type;
>  
> -gic_set_irq_type(desc);
> +gic_set_irq_type(desc, desc->arch.type);
>  gic_set_irq_priority(desc, priority);
>  }
>  
> @@ -154,7 +154,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>  desc->handler = gic_hw_ops->gic_guest_irq_type;
>  set_bit(_IRQ_GUEST, >status);
>  
> -gic_set_irq_type(desc);
> +gic_set_irq_type(desc, desc->arch.type);
>  gic_set_irq_priority(desc, priority);
>  
>  p->desc = desc;
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 3f39f79..2214e87 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -328,8 +328,8 @@ struct gic_hw_operations {
>  void (*deactivate_irq)(struct irq_desc *irqd);
>  /* Read IRQ id and Ack */
>  unsigned int (*read_irq)(void);
> -/* Set IRQ type - type is taken from desc->arch.type */
> -void (*set_irq_type)(struct irq_desc *desc);
> +/* Set IRQ type */
> +void (*set_irq_type)(struct irq_desc *desc, unsigned int type);
>  /* Set IRQ priority */
>  void (*set_irq_priority)(struct irq_desc *desc, unsigned int priority);
>  /* Send SGI */
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [PATCH v2 2/9] xen/arm: gic: Do not configure affinity during routing

[Stefano Stabellini]

On Thu, 14 Jul 2016, Julien Grall wrote:
> The affinity of a guest IRQ is set every time the guest enable it (see
> vgic_enable_irqs).
> 
> It is not necessary to set the affinity when the IRQ is routed to the
> guest because Xen will never receive the IRQ until it hass been enabled
> by the guest.
> 
> To keep gic_route_irq_to_{xen,guest} behaving the same way (i.e just
> setting up the routing), the affinity of IRQ routed to Xen is moved into
> __setup_irq.
> 
> Signed-off-by: Julien grall 

Reviewed-by: Stefano Stabellini 


> ---
> Changes in v2:
> - Patch renamed
> - Set the affinity for IRQ routed to Xen in __setup_irq
> ---
>  xen/arch/arm/gic.c| 11 +++
>  xen/arch/arm/irq.c|  4 ++--
>  xen/include/asm-arm/gic.h |  3 +--
>  3 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 5726a05..bc814a0 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -97,24 +97,19 @@ void gic_restore_state(struct vcpu *v)
>  }
>  
>  /*
> - * needs to be called with a valid cpu_mask, ie each cpu in the mask has
> - * already called gic_cpu_init
>   * - desc.lock must be held
>   * - arch.type must be valid (i.e != IRQ_TYPE_INVALID)
>   */
>  static void gic_set_irq_properties(struct irq_desc *desc,
> -   const cpumask_t *cpu_mask,
> unsigned int priority)
>  {
>  gic_hw_ops->set_irq_properties(desc, priority);
> -desc->handler->set_affinity(desc, cpu_mask);
>  }
>  
>  /* Program the GIC to route an interrupt to the host (i.e. Xen)
>   * - needs to be called with desc.lock held
>   */
> -void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
> -  unsigned int priority)
> +void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int priority)
>  {
>  ASSERT(priority <= 0xff); /* Only 8 bits of priority */
>  ASSERT(desc->irq < gic_number_lines());/* Can't route interrupts that 
> don't exist */
> @@ -123,7 +118,7 @@ void gic_route_irq_to_xen(struct irq_desc *desc, const 
> cpumask_t *cpu_mask,
>  
>  desc->handler = gic_hw_ops->gic_host_irq_type;
>  
> -gic_set_irq_properties(desc, cpu_mask, priority);
> +gic_set_irq_properties(desc, priority);
>  }
>  
>  /* Program the GIC to route an interrupt to a guest
> @@ -155,7 +150,7 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
> virq,
>  desc->handler = gic_hw_ops->gic_guest_irq_type;
>  set_bit(_IRQ_GUEST, >status);
>  
> -gic_set_irq_properties(desc, cpumask_of(v_target->processor), priority);
> +gic_set_irq_properties(desc, priority);
>  
>  p->desc = desc;
>  res = 0;
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 2f8af72..3fc22f2 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -370,6 +370,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, 
> struct irqaction *new)
>  /* First time the IRQ is setup */
>  if ( disabled )
>  {
> +gic_route_irq_to_xen(desc, GIC_PRI_IRQ);
>  /* It's fine to use smp_processor_id() because:
>   * For PPI: irq_desc is banked
>   * For SPI: we don't care for now which CPU will receive the
> @@ -377,8 +378,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, 
> struct irqaction *new)
>   * TODO: Handle case where SPI is setup on different CPU than
>   * the targeted CPU and the priority.
>   */
> -gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
> - GIC_PRI_IRQ);
> +irq_set_affinity(desc, cpumask_of(smp_processor_id()));
>  desc->handler->startup(desc);
>  }
>  
> diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
> index 2fc6126..7ba3846 100644
> --- a/xen/include/asm-arm/gic.h
> +++ b/xen/include/asm-arm/gic.h
> @@ -223,8 +223,7 @@ enum gic_version {
>  extern enum gic_version gic_hw_version(void);
>  
>  /* Program the GIC to route an interrupt */
> -extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t 
> *cpu_mask,
> - unsigned int priority);
> +extern void gic_route_irq_to_xen(struct irq_desc *desc, unsigned int 
> priority);
>  extern int gic_route_irq_to_guest(struct domain *, unsigned int virq,
>struct irq_desc *desc,
>unsigned int priority);
> -- 
> 1.9.1
> 

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

Re: [Xen-devel] [DRAFT v2] XenSock protocol design document

[Stefano Stabellini]

On Fri, 15 Jul 2016, Paul Durrant wrote:
> > -Original Message-
> > From: Juergen Gross [mailto:jgr...@suse.com]
> > Sent: 15 July 2016 12:37
> > To: Stefano Stabellini; xen-de...@lists.xenproject.org
> > Cc: joao.m.mart...@oracle.com; Wei Liu; Roger Pau Monne; Lars Kurth;
> > boris.ostrov...@oracle.com; Paul Durrant
> > Subject: Re: [DRAFT v2] XenSock protocol design document
> > 
> > On 13/07/16 17:47, Stefano Stabellini wrote:
> > > Hi all,
> > >
> > > This is the design document of the XenSock protocol. You can find
> > > prototypes of the Linux frontend and backend drivers here:
> > ...
> > > ### Commands Ring
> > >
> > > The shared ring is used by the frontend to forward socket API calls to the
> > > backend. I'll refer to this ring as **commands ring** to distinguish it 
> > > from
> > > other rings which will be created later in the lifecycle of the protocol 
> > > (data
> > > rings). The ring format is defined using the familiar `DEFINE_RING_TYPES`
> > macro
> > > (`xen/include/public/io/ring.h`). Frontend requests are allocated on the
> > ring
> > > using the `RING_GET_REQUEST` macro.
> > >
> > > The format is defined as follows:
> > >
> > > #define XENSOCK_SOCKET 0
> > > #define XENSOCK_CONNECT1
> > > #define XENSOCK_RELEASE2
> > > #define XENSOCK_BIND   3
> > > #define XENSOCK_LISTEN 4
> > > #define XENSOCK_ACCEPT 5
> > > #define XENSOCK_POLL   6
> > >
> > > struct xen_xensock_request {
> > >   uint32_t id; /* private to guest, echoed in response */
> > >   uint32_t cmd; /* command to execute */
> > >   uint64_t sockid;
> > >   union {
> > >   struct xen_xensock_socket {
> > >   uint32_t domain;
> > >   uint32_t type;
> > >   uint32_t protocol;
> > >   } socket;
> > >   struct xen_xensock_connect {
> > >   uint8_t addr[28];
> > >   uint32_t len;
> > >   uint32_t flags;
> > >   grant_ref_t ref;
> > >   uint32_t evtchn;
> > >   } connect;
> > >   struct xen_xensock_bind {
> > >   uint8_t addr[28];
> > >   uint32_t len;
> > >   } bind;
> > >   struct xen_xensock_listen {
> > >   uint32_t backlog;
> > >   } listen;
> > >   struct xen_xensock_accept {
> > >   uint64_t sockid;
> > >   grant_ref_t ref;
> > >   uint32_t evtchn;
> > >   } accept;
> > >   } u;
> > > };
> > 
> > Please add padding at the end (or a dummy union member) to make sure
> > 32- and 64-bit variants have the same size (I believe now the size will
> > be 60 bytes on 32-bit system and 64 bytes on 64-bit).

Well spotted! You have a point, I think you are right, even though it
makes the struct a bit awkward.


> Actually, rather than this bunch of structs that assume a System V ABI, maybe 
> we need a spec. more along the lines of the (ancient) TPI doc. 
> http://pubs.opengroup.org/onlinepubs/009618999/toc.htm. After all, like TPI, 
> this is a message passing protocol.

The C struct was supposed to be only descriptive: I wrote the binary
layouts too. In fact the C struct doesn't even have to be part of the
spec, I included it because I find it more intuitive. I'll make the
wording clearer on this point. However it is true that the layouts don't
cover stuff generated by DEFINE_RING_TYPES.

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

Re: [Xen-devel] [PATCH v2] xl: add option to leave domain paused after migration

[Boris Ostrovsky]

On 07/19/2016 04:58 AM, Roger Pau Monne wrote:
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index d8530f0..fd80442 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -4742,7 +4742,7 @@ static void migrate_domain(uint32_t domid, const char 
> *rune, int debug,
>  exit(EXIT_FAILURE);
>  }
>  
> -static void migrate_receive(int debug, int daemonize, int monitor,
> +static void migrate_receive(int debug, int daemonize, int monitor, int pause,

This causes a name shadowing error on an old compiler:

cc1: warnings being treated as errors
xl_cmdimpl.c: In function ‘migrate_receive’:
xl_cmdimpl.c:4781: error: declaration of ‘pause’ shadows a global
declaration
/usr/include/unistd.h:466: error: shadowed declaration is here
xl_cmdimpl.c: In function ‘main_migrate_receive’:
xl_cmdimpl.c:5008: error: declaration of ‘pause’ shadows a global
declaration
/usr/include/unistd.h:466: error: shadowed declaration is here
xl_cmdimpl.c: In function ‘main_migrate’:
xl_cmdimpl.c:5094: error: declaration of ‘pause’ shadows a global
declaration
/usr/include/unistd.h:466: error: shadowed declaration is here
make: *** [xl_cmdimpl.o] Error 1

FC-64  gcc
--version | head -1
gcc (GCC) 4.4.4 20100503 (Red Hat 4.4.4-2)
FC-64  grep
pause /usr/include/unistd.h
extern int pause (void);
FC-64 



-boris


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

Re: [Xen-devel] [PATCH v2 2/3] x86/emulate: add support of emulating SSE2 instruction {, v}movd mm, m32

[Mihai Donțu]

On Monday 18 July 2016 15:57:09 Andrew Cooper wrote:
> On 18/07/16 15:30, Mihai Donțu wrote:
> > @@ -4409,6 +4409,10 @@ x86_emulate(
> >  case 0x6f: /* movq mm/m64,mm */
> > /* {,v}movdq{a,u} xmm/m128,xmm */
> > /* vmovdq{a,u} ymm/m256,ymm */
> > +case 0x7e: /* movd mm,r/m32 */
> > +   /* movq mm,r/m64 */
> > +   /* {,v}movd xmm,r/m32 */
> > +   /* {,v}movq xmm,r/m64 */  
> 
> This exposes a vulnerability where a guest can clobber local state in
> x86_emulate, by specifying registers such as %ebx as the destination.
> 
> You must either
> 1) Move this case up above the fail_if(ea.type != OP_MEM); check, or
> 2) modify the stub logic to convert a GPR destination to a memory
> address pointing into _regs.

I'm taking a look at (2) as it feels like the best approach. If I'm not
making any good progress in the coming days, I'll fallback to (1).

Thank you,

-- 
Mihai DONȚU


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

[Xen-devel] [linux-3.18 bisection] complete test-amd64-i386-xl-qemut-debianhvm-amd64

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-xl-qemut-debianhvm-amd64
testid debian-hvm-install

Tree: linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
  Bug introduced:  a2d8c514753276394d68414f563591f174ef86cb
  Bug not present: 8f620446135b64ca6f96cf32066a76d64e79a388
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/97669/


  commit a2d8c514753276394d68414f563591f174ef86cb
  Author: Lukasz Odzioba 
  Date:   Fri Jun 24 14:50:01 2016 -0700
  
  mm/swap.c: flush lru pvecs on compound page arrival
  
  [ Upstream commit 8f182270dfec432e93fae14f9208a6b9af01009f ]
  
  Currently we can have compound pages held on per cpu pagevecs, which
  leads to a lot of memory unavailable for reclaim when needed.  In the
  systems with hundreads of processors it can be GBs of memory.
  
  On of the way of reproducing the problem is to not call munmap
  explicitly on all mapped regions (i.e.  after receiving SIGTERM).  After
  that some pages (with THP enabled also huge pages) may end up on
  lru_add_pvec, example below.
  
void main() {
#pragma omp parallel
{
size_t size = 55 * 1000 * 1000; // smaller than  MEM/CPUS
void *p = mmap(NULL, size, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS , -1, 0);
if (p != MAP_FAILED)
memset(p, 0, size);
//munmap(p, size); // uncomment to make the problem go away
}
}
  
  When we run it with THP enabled it will leave significant amount of
  memory on lru_add_pvec.  This memory will be not reclaimed if we hit
  OOM, so when we run above program in a loop:
  
for i in `seq 100`; do ./a.out; done
  
  many processes (95% in my case) will be killed by OOM.
  
  The primary point of the LRU add cache is to save the zone lru_lock
  contention with a hope that more pages will belong to the same zone and
  so their addition can be batched.  The huge page is already a form of
  batched addition (it will add 512 worth of memory in one go) so skipping
  the batching seems like a safer option when compared to a potential
  excess in the caching which can be quite large and much harder to fix
  because lru_add_drain_all is way to expensive and it is not really clear
  what would be a good moment to call it.
  
  Similarly we can reproduce the problem on lru_deactivate_pvec by adding:
  madvise(p, size, MADV_FREE); after memset.
  
  This patch flushes lru pvecs on compound page arrival making the problem
  less severe - after applying it kill rate of above example drops to 0%,
  due to reducing maximum amount of memory held on pvec from 28MB (with
  THP) to 56kB per CPU.
  
  Suggested-by: Michal Hocko 
  Link: 
http://lkml.kernel.org/r/1466180198-18854-1-git-send-email-lukasz.odzi...@intel.com
  Signed-off-by: Lukasz Odzioba 
  Acked-by: Michal Hocko 
  Cc: Kirill Shutemov 
  Cc: Andrea Arcangeli 
  Cc: Vladimir Davydov 
  Cc: Ming Li 
  Cc: Minchan Kim 
  Cc: 
  Signed-off-by: Andrew Morton 
  Signed-off-by: Linus Torvalds 
  Signed-off-by: Sasha Levin 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-3.18/test-amd64-i386-xl-qemut-debianhvm-amd64.debian-hvm-install.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/linux-3.18/test-amd64-i386-xl-qemut-debianhvm-amd64.debian-hvm-install
 --summary-out=tmp/97669.bisection-summary --basis-template=96188 
--blessings=real,real-bisect linux-3.18 
test-amd64-i386-xl-qemut-debianhvm-amd64 debian-hvm-install
Searching for failure / basis pass:
 97592 fail [host=huxelrebe0] / 96188 [host=huxelrebe1] 96161 
[host=chardonnay1] 95844 [host=baroque1] 95809 [host=pinot1] 95597 ok.
Failure / basis pass flights: 97592 / 95597
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
Tree: linuxfirmware 

[Xen-devel] [linux-4.1 bisection] complete test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm
testid debian-hvm-install

Tree: linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://xenbits.xen.org/qemu-xen.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  linux 
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git
  Bug introduced:  c5ad33184354260be6d05de57e46a5498692f6d6
  Bug not present: c5bcec6cbcbf520f088dc7939934bbf10c20c5a5
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/97670/


  commit c5ad33184354260be6d05de57e46a5498692f6d6
  Author: Lukasz Odzioba 
  Date:   Fri Jun 24 14:50:01 2016 -0700
  
  mm/swap.c: flush lru pvecs on compound page arrival
  
  [ Upstream commit 8f182270dfec432e93fae14f9208a6b9af01009f ]
  
  Currently we can have compound pages held on per cpu pagevecs, which
  leads to a lot of memory unavailable for reclaim when needed.  In the
  systems with hundreads of processors it can be GBs of memory.
  
  On of the way of reproducing the problem is to not call munmap
  explicitly on all mapped regions (i.e.  after receiving SIGTERM).  After
  that some pages (with THP enabled also huge pages) may end up on
  lru_add_pvec, example below.
  
void main() {
#pragma omp parallel
{
size_t size = 55 * 1000 * 1000; // smaller than  MEM/CPUS
void *p = mmap(NULL, size, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS , -1, 0);
if (p != MAP_FAILED)
memset(p, 0, size);
//munmap(p, size); // uncomment to make the problem go away
}
}
  
  When we run it with THP enabled it will leave significant amount of
  memory on lru_add_pvec.  This memory will be not reclaimed if we hit
  OOM, so when we run above program in a loop:
  
for i in `seq 100`; do ./a.out; done
  
  many processes (95% in my case) will be killed by OOM.
  
  The primary point of the LRU add cache is to save the zone lru_lock
  contention with a hope that more pages will belong to the same zone and
  so their addition can be batched.  The huge page is already a form of
  batched addition (it will add 512 worth of memory in one go) so skipping
  the batching seems like a safer option when compared to a potential
  excess in the caching which can be quite large and much harder to fix
  because lru_add_drain_all is way to expensive and it is not really clear
  what would be a good moment to call it.
  
  Similarly we can reproduce the problem on lru_deactivate_pvec by adding:
  madvise(p, size, MADV_FREE); after memset.
  
  This patch flushes lru pvecs on compound page arrival making the problem
  less severe - after applying it kill rate of above example drops to 0%,
  due to reducing maximum amount of memory held on pvec from 28MB (with
  THP) to 56kB per CPU.
  
  Suggested-by: Michal Hocko 
  Link: 
http://lkml.kernel.org/r/1466180198-18854-1-git-send-email-lukasz.odzi...@intel.com
  Signed-off-by: Lukasz Odzioba 
  Acked-by: Michal Hocko 
  Cc: Kirill Shutemov 
  Cc: Andrea Arcangeli 
  Cc: Vladimir Davydov 
  Cc: Ming Li 
  Cc: Minchan Kim 
  Cc: 
  Signed-off-by: Andrew Morton 
  Signed-off-by: Linus Torvalds 
  Signed-off-by: Sasha Levin 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/linux-4.1/test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm.debian-hvm-install.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/linux-4.1/test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm.debian-hvm-install
 --summary-out=tmp/97670.bisection-summary --basis-template=96211 
--blessings=real,real-bisect linux-4.1 
test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm debian-hvm-install
Searching for failure / basis pass:
 97613 fail [host=elbling0] / 96211 [host=fiano1] 96183 [host=chardonnay1] 
96160 [host=italia0] 95848 [host=nocera1] 95818 [host=pinot1] 95591 
[host=fiano0] 95517 [host=chardonnay0] 95455 [host=pinot0] 95408 
[host=huxelrebe0] 94729 [host=chardonnay0] 94034 [host=huxelrebe1] 93220 
[host=chardonnay1] 93111 [host=rimava1] 92143 [host=merlot1] 

Re: [Xen-devel] [PATCH] acpi: Re-license ACPI builder files from GPLv2 to LGPLv2.1

[Stefan Berger]

Daniel Kiper  wrote on 07/19/2016 11:00:04 AM:

> Subject: Re: [PATCH] acpi: Re-license ACPI builder files from GPLv2 
> to LGPLv2.1
> 
> On Mon, Jul 18, 2016 at 10:01:27AM -0400, Boris Ostrovsky wrote:
> > ACPI builder is currently distributed under GPLv2 license.
> >
> > We plan to make the builder available to components other
> > than the hvmloader (which is also GPLv2). Some of these
> > components (such as libxl) may be distributed under LGPL-2.1
> > so that they can be used by non-GPLv2 callers.  But this
> > will not be possible if we incorporate the ACPI builder in
> > those other components.
> >
> > To avoid this problem we are relicensing sources in ACPI
> > bulder directory to the Lesser GNU Public License (LGPL)
> > version 2.1
> >
> > Signed-off-by: Boris Ostrovsky 
> > CC: Kouya Shimura 
> > CC: Daniel Kiper 
> > CC: Stefan Berger 
> > CC: Simon Horman 
> > CC: Keir Fraser 
> > CC: Ian Jackson 
> > CC: Lars Kurth 
> 
> Acked-by: Daniel Kiper 
Acked-by: Stefan Berger 


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

Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[Tamas K Lengyel]

On Tue, Jul 19, 2016 at 11:11 AM, George Dunlap
 wrote:
> On 18/07/16 18:06, Tamas K Lengyel wrote:
>>> Incremental improvements are welcome; but they must not cause
>>> regressions in existing functionality.
>>
>> Existing functionality does not get impaired by this as what happens
>> right now is a hypervisor crash. I don't see how things can get any
>> worst than that.
>
> Also from another thread:
>> If anyone else would have been interested in getting the two systems
>> working together othen then me they probably would have complained
>> already that hey this crashes the hypervisor. My point being that at
>> this point the impact of this patch is likely really low.
>
> From a user perspective, "failing intermittently in some strange and
> unpredictable way" is definitely worse than a hypervisor crash. :-)
>
> My concern is that someone will start using guests which use the altp2m
> interface internally, and that will all work; and then maybe separately
> they will start doing some sort of memory sharing between guests, and
> that will all work; and then at some point they'll do memory sharing on
> a guest using the altp2m functionality internally, and suddenly they'll
> get strange intermittent errors where things don't behave the way they
> expect and they don't know why.  A hypervisor crash that tells them
> exactly what code has the problem is definitely preferable.

Well, IMHO that's where documenting the expected use-case and the
known corner-cases comes into play.

>
>>> The code as it is in the tree right now was intended to allow both
>>> sharing and altp2m to be enabled on the same domain, just not over the
>>> same gfn range.  And it was intended to be robust -- that is, the
>>> sharing code and the altp2m code don't need to be aware of each other
>>> and try not to step on each other's toes; each can just do its own thing
>>> and Xen will make sure that nothing bad happens (by preventing pages
>>> with an altp2m entry from being shared, and unsharing pages for which an
>>> altp2m entry is created).
>>>
>>> It sounds like that's broken right now; it should therefore be fixed.
>>> When it is fixed, you'll be able to use both altp2m and sharing on the
>>> same domain; Xen will simply prevent sharing from happening on gfn
>>> ranges with altp2m entries.
>>
>> No, that's incorrect, it's the other way around. If you were to try to
>> share pages for which you have altp2m entries it will happily oblige.
>> It will just fail to do the altp2m actions for entries of shared
>> entries (copying the mapping to the altp2m view, mem_access, etc).
>
> It's quite possible I missed something, but that's not how I read the
> code.  Before sharing a page you have to have to call
> mem_sharing_nominate_page(), which calls page_make_sharable().
> page_make_sharable() will make sure that it has exactly the expected
> number of references; which for gfns is 2 and for grant references is 4.
>
> When you map an mfn into an altp2m of a different gfn, you'll increase
> the reference count.  So it appears to me that if you attempt to share a
> page which is mapped in an altp2m, then the nominate operation will fail
> (with -E2BIG, of all things).
>
> Am I mistaken about that?

Hm, no you may be right. I was thinking of the type checking only. If
the reference count prevents pages with alt2pm entries from being
shared  - going from p2m_ram_rw -> p2m_ram_shared - then from my
perspective that is fine and I'm not planning on changing that. What
I'm trying to get working is if the type is already p2m_ram_shared and
is going from p2m_ram_shared -> p2m_ram_rw. I would also like to be
able to do mem_access settings for the p2m_ram_shared type pte in an
altp2m view.

>
> (BTB this would probably still be the case even after your patch.)
>
> Also, as far as I can tell, "It will just fail to do the altp2m actions
> for entries of shared entries" is not true; instead, the page will be
> un-shared and the altp2m action will then take place.  Is this not the case?

So right now when the entry is p2m_ram_shared it will crash the
hypervisor because of the lock ordering issue during unsharing. If the
lock ordering issue is fixed, the unsharing event will result in the
altp2m propagate change taking the p2m setting from the hostp2m and
copying it to all affected altp2m views, overwriting any setting that
may have been stored there. This is the situation that can be
monitored with mem_access so that the user can perform the unsharing
and recreating the necessary altp2m settings manually. What I mean in
the quoted sentence is that the altp2m ops do a type-check right now,
so if you shared a page before, the type check will make all altp2m
ops fail on that entry. So for example if you have a shared pte, and
then try to do altp2m mem_access setting on it, it will fail.

>
>>> An even bigger improvement would be to allow the same gfns to be subject
>>> both to altp2m and sharing at the same time.  But this 

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

version targeted for testing:
 libvirt  c62e9d4199afb0e6cff1b6818330b115417addc1
baseline version:
 libvirt  fe8bad38f58f8b60518947441fb3be8d89d51c58

Last test of basis97416  2016-07-16 04:21:36 Z3 days
Testing same since97638  2016-07-19 04:22:58 Z0 days1 attempts


People who touched revisions under test:
  Andrea Bolognani 
  Cole Robinson 
  Jiri Denemark 
  Ján Tomko 
  Maxim Nestratov 
  Nikolay Shirokovskiy 
  Olga Krishtal 

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



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

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

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

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


Pushing revision :

+ branch=libvirt
+ revision=c62e9d4199afb0e6cff1b6818330b115417addc1
+ . ./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
++ 

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

[osstest service owner]

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

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  22b430e0e3c5f3d071cb8e2713d7ea33ee8624ec
baseline version:
 xen  e763268781d341fef05d461f3057e6ced5e033f2

Last test of basis97614  2016-07-18 18:15:32 Z0 days
Testing same since97661  2016-07-19 15:03:13 Z0 days1 attempts


People who touched revisions under test:
  Ian Jackson 
  Juergen Gross 
  Marek Marczykowski-Górecki 
  Roger Pau Monne 
  Roger Pau Monné 
  Wei Liu 

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



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

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

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

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


Pushing revision :

+ branch=xen-unstable-smoke
+ revision=22b430e0e3c5f3d071cb8e2713d7ea33ee8624ec
+ . ./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 
22b430e0e3c5f3d071cb8e2713d7ea33ee8624ec
+ branch=xen-unstable-smoke
+ revision=22b430e0e3c5f3d071cb8e2713d7ea33ee8624ec
+ . ./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
+ '[' x22b430e0e3c5f3d071cb8e2713d7ea33ee8624ec = 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 '
   

[Xen-devel] [PATCH 1/2] arm/traps: fix bug in dump_guest_s1_walk L1 page table offset computation

[Jonathan Daugherty]

The dump_guest_s1_walk function was incorrectly using the top 10 bits of
the virtual address to select the L1 page table index.  The correct
amount is 12 bits, resulting in a shift of 20 bits rather than 22.

For more details, see the ARMv7-A ARM, section B3.5, "Short-descriptor
translation table format."

Signed-off-by: Jonathan Daugherty 
---
 xen/arch/arm/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index a2eb1da..0c10c4d 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2346,7 +2346,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
 }
 first = map_domain_page(mfn);
 
-offset = addr >> (12+10);
+offset = addr >> (12+8);
 printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
offset, pfn_to_paddr(mfn_x(mfn)), first[offset]);
 if ( !(first[offset] & 0x1) ||
-- 
2.9.2


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

[Xen-devel] [PATCH 2/2] arm/traps: fix bug in dump_guest_s1_walk handling of level 2 page tables

[Jonathan Daugherty]

dump_guest_s1_walk intends to walk to level 2 page table entries but
was failing to do so because of a check that caused level 2 page table
descriptors to be ignored. This change fixes the check so that level 2
page table walks occur as intended by ignoring descriptors unless their
low two bits match the expected sequence [0,1].

For more information, see the ARMv7-A ARM, section B3.5.

Signed-off-by: Jonathan Daugherty 
---
 xen/arch/arm/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 0c10c4d..dfb1949 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2350,7 +2350,7 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t addr)
 printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
offset, pfn_to_paddr(mfn_x(mfn)), first[offset]);
 if ( !(first[offset] & 0x1) ||
- !(first[offset] & 0x2) )
+  (first[offset] & 0x2) )
 goto done;
 
 mfn = p2m_lookup(d, _gfn(paddr_to_pfn(first[offset])), NULL);
-- 
2.9.2


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

Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[George Dunlap]

On 18/07/16 18:06, Tamas K Lengyel wrote:
>> Incremental improvements are welcome; but they must not cause
>> regressions in existing functionality.
> 
> Existing functionality does not get impaired by this as what happens
> right now is a hypervisor crash. I don't see how things can get any
> worst than that.

Also from another thread:
> If anyone else would have been interested in getting the two systems
> working together othen then me they probably would have complained
> already that hey this crashes the hypervisor. My point being that at
> this point the impact of this patch is likely really low.

From a user perspective, "failing intermittently in some strange and
unpredictable way" is definitely worse than a hypervisor crash. :-)

My concern is that someone will start using guests which use the altp2m
interface internally, and that will all work; and then maybe separately
they will start doing some sort of memory sharing between guests, and
that will all work; and then at some point they'll do memory sharing on
a guest using the altp2m functionality internally, and suddenly they'll
get strange intermittent errors where things don't behave the way they
expect and they don't know why.  A hypervisor crash that tells them
exactly what code has the problem is definitely preferable.

>> The code as it is in the tree right now was intended to allow both
>> sharing and altp2m to be enabled on the same domain, just not over the
>> same gfn range.  And it was intended to be robust -- that is, the
>> sharing code and the altp2m code don't need to be aware of each other
>> and try not to step on each other's toes; each can just do its own thing
>> and Xen will make sure that nothing bad happens (by preventing pages
>> with an altp2m entry from being shared, and unsharing pages for which an
>> altp2m entry is created).
>>
>> It sounds like that's broken right now; it should therefore be fixed.
>> When it is fixed, you'll be able to use both altp2m and sharing on the
>> same domain; Xen will simply prevent sharing from happening on gfn
>> ranges with altp2m entries.
> 
> No, that's incorrect, it's the other way around. If you were to try to
> share pages for which you have altp2m entries it will happily oblige.
> It will just fail to do the altp2m actions for entries of shared
> entries (copying the mapping to the altp2m view, mem_access, etc).

It's quite possible I missed something, but that's not how I read the
code.  Before sharing a page you have to have to call
mem_sharing_nominate_page(), which calls page_make_sharable().
page_make_sharable() will make sure that it has exactly the expected
number of references; which for gfns is 2 and for grant references is 4.

When you map an mfn into an altp2m of a different gfn, you'll increase
the reference count.  So it appears to me that if you attempt to share a
page which is mapped in an altp2m, then the nominate operation will fail
(with -E2BIG, of all things).

Am I mistaken about that?

(BTB this would probably still be the case even after your patch.)

Also, as far as I can tell, "It will just fail to do the altp2m actions
for entries of shared entries" is not true; instead, the page will be
un-shared and the altp2m action will then take place.  Is this not the case?

>> An even bigger improvement would be to allow the same gfns to be subject
>> both to altp2m and sharing at the same time.  But this requires thinking
>> carefully about all the corner cases and making sure that they all work
>> correctly.
> 
> And this is exactly what this patch allows you to do. An entry can now
> be both shared, get properly copied to altp2m views, allow setting
> mem_access in altp2m views, etc. The only situation you have to take
> core of is when the type of the entry changes from shared to unshared
> as that resets the altp2m views.

I described another situation you have to be careful of in an earlier
e-mail:
- host gfn A is marked "shared"
- altp2m gfn O is mapped to gfn A (thus also marked as 'shared')
- Guest writes to gfn O, Xen attempts to unshare the page.

In this circumstance, the fault will ends up in
__mem_sharing_unshare_page(), which will calls rmap_retrieve(d, O, mA).
This returns NULL because gfn O was never put in the reverse map, and
you BUG().

Again, am I misreading what would happen?

I'm pretty sure if I went looking I could find some more situations you
need to avoid to prevent problems.

So the next big missing piece of information in this discussion is
exactly what you do need from this system.  You're using altp2m and
mem_sharing (and mem_access) on the same domain, that's obvious.  Which
features of altp2m are you using -- are you mainly using the mem_access
changes, or are you also using the gfn mapping functionality?

Also, how important is it that pages using altp2m functionality not be
un-shared -- i.e., what proportion of a guest's pages do you expect to
be shared, and what proportion do you need to perform altp2m operations on?

So there 

[Xen-devel] [qemu-mainline test] 97627: regressions - trouble: blocked/broken/fail/pass

[osstest service owner]

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

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-amd64-amd64-libvirt-xsm 11 guest-start   fail REGR. vs. 96791
 test-amd64-amd64-libvirt-pair 20 guest-start/debian   fail REGR. vs. 96791
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 9 debian-hvm-install fail 
REGR. vs. 96791
 test-amd64-amd64-libvirt 11 guest-start   fail REGR. vs. 96791
 test-amd64-amd64-xl-qcow2 9 debian-di-install fail REGR. vs. 96791
 test-amd64-amd64-xl-qemuu-ovmf-amd64 9 debian-hvm-install fail REGR. vs. 96791
 test-amd64-amd64-libvirt-vhd  9 debian-di-install fail REGR. vs. 96791
 build-armhf-pvops 4 host-build-prep   fail REGR. vs. 96791

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

Tests which did not succeed, but are not blocking:
 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-arndale   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-xsm   1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-cubietruck  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-credit2   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl-multivcpu  1 build-check(1)   blocked  n/a
 test-armhf-armhf-xl   1 build-check(1)   blocked  n/a
 test-armhf-armhf-libvirt-qcow2  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-amd64-amd64-xl-pvh-amd  11 guest-start  fail   never pass
 test-amd64-amd64-xl-pvh-intel 11 guest-start  fail  never pass
 test-amd64-i386-libvirt-xsm  12 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  12 migrate-support-checkfail   never pass
 test-amd64-amd64-qemuu-nested-amd 16 debian-hvm-install/l1/l2  fail never pass
 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 migrate-support-check 
fail never pass

version targeted for testing:
 qemuu3913d3707e3debfbf0d2d014a1a793394993b088
baseline version:
 qemuu4f4a9ca4a4386c137301b3662faba076455ff15a

Last test of basis96791  2016-07-08 12:20:07 Z   11 days
Failing since 97271  2016-07-13 13:44:26 Z6 days   10 attempts
Testing same since97627  2016-07-18 22:46:32 Z0 days1 attempts


People who touched revisions under test:
  Alberto Garcia 
  Alex Bennée 
  Alexander Yarygin 
  Andrew Jones 
  Anthony PERARD 
  Ashok Raj 
  Benjamin Herrenschmidt 
  Bharata B Rao 
  Cao jin 
  Cornelia Huck 
  Cédric Le Goater 
  Daniel P. Berrange 
  David Gibson 
  David Hildenbrand 
  Denis V. Lunev 
  Dmitry Osipenko 
  Eduardo Habkost 
  Eric Blake 
  Eugene (jno) Dvurechenski 
  Evgeny Yakovlev 
  Fam Zheng 
  Gerd Hoffmann 
  Gonglei 
  Greg Kurz 
  Haibin Wang 
  Haozhong Zhang 
  Igor Mammedov 
  James Hogan 
  Jarkko Lavinen 
  Jeff Cody 
  Jing Liu 
  Kevin Wolf 
  Laszlo Ersek 
  Leon Alrae 
  Lin Ma 
  Marc Marí 
  Marc-André Lureau 
  Marcin Krzeminski 
  Mark Cave-Ayland 
  Markus Armbruster 
  Max Filippov 
  Max Reitz 
  Paolo Bonzini 
  Paul Burton 
  Peter Lieven 
  Peter Maydell 
  Pierre Morel 
  Reda Sallahi 
  Richard 

Re: [Xen-devel] [PATCH v7] x86/mem-sharing: mem-sharing a range of memory

[Tamas K Lengyel]

On Tue, Jul 19, 2016 at 10:55 AM, Andrew Cooper
 wrote:
> On 19/07/16 17:54, Tamas K Lengyel wrote:
>> On Tue, Jul 19, 2016 at 10:49 AM, Andrew Cooper
>>  wrote:
>>> On 19/07/16 17:27, Tamas K Lengyel wrote:
>> +{
>> +int rc = 0;
>> +shr_handle_t sh, ch;
>> +unsigned long start =
>> +range->_scratchspace ? range->_scratchspace : range->start;
> This can be shortened to "unsigned long start = range->_scratchspace ?:
> range->start;" and fit on a single line.
 I'm not that familiar with this style of the syntax, does that have
 the effect of setting start = _scratchspace when _scratchspace is not
 0?
>>> It is a GCC extension
>>> https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/Conditionals.html which
>>> allows you to omit the middle parameter if it is identical to the first.
>> Are we OK with using syntax that is based on a compiler extension? I
>> recall some cases where that was frowned upon (like using the 0b
>> prefix).
>
> We already use these all over the place.
>
> The problem with 0b is that it isn't supported in all versions of GCC we
> support.
>

Alright, sound good!

Thanks,
Tamas

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

Re: [Xen-devel] [PATCH v7] x86/mem-sharing: mem-sharing a range of memory

[Andrew Cooper]

On 19/07/16 17:54, Tamas K Lengyel wrote:
> On Tue, Jul 19, 2016 at 10:49 AM, Andrew Cooper
>  wrote:
>> On 19/07/16 17:27, Tamas K Lengyel wrote:
> +{
> +int rc = 0;
> +shr_handle_t sh, ch;
> +unsigned long start =
> +range->_scratchspace ? range->_scratchspace : range->start;
 This can be shortened to "unsigned long start = range->_scratchspace ?:
 range->start;" and fit on a single line.
>>> I'm not that familiar with this style of the syntax, does that have
>>> the effect of setting start = _scratchspace when _scratchspace is not
>>> 0?
>> It is a GCC extension
>> https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/Conditionals.html which
>> allows you to omit the middle parameter if it is identical to the first.
> Are we OK with using syntax that is based on a compiler extension? I
> recall some cases where that was frowned upon (like using the 0b
> prefix).

We already use these all over the place.

The problem with 0b is that it isn't supported in all versions of GCC we
support.

~Andrew

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

Re: [Xen-devel] [PATCH v7] x86/mem-sharing: mem-sharing a range of memory

[Tamas K Lengyel]

On Tue, Jul 19, 2016 at 10:49 AM, Andrew Cooper
 wrote:
> On 19/07/16 17:27, Tamas K Lengyel wrote:
>>
 +{
 +int rc = 0;
 +shr_handle_t sh, ch;
 +unsigned long start =
 +range->_scratchspace ? range->_scratchspace : range->start;
>>> This can be shortened to "unsigned long start = range->_scratchspace ?:
>>> range->start;" and fit on a single line.
>> I'm not that familiar with this style of the syntax, does that have
>> the effect of setting start = _scratchspace when _scratchspace is not
>> 0?
>
> It is a GCC extension
> https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/Conditionals.html which
> allows you to omit the middle parameter if it is identical to the first.

Are we OK with using syntax that is based on a compiler extension? I
recall some cases where that was frowned upon (like using the 0b
prefix).

Tamas

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

Re: [Xen-devel] [PATCH v7] x86/mem-sharing: mem-sharing a range of memory

[Andrew Cooper]

On 19/07/16 17:27, Tamas K Lengyel wrote:
>
>>> +{
>>> +int rc = 0;
>>> +shr_handle_t sh, ch;
>>> +unsigned long start =
>>> +range->_scratchspace ? range->_scratchspace : range->start;
>> This can be shortened to "unsigned long start = range->_scratchspace ?:
>> range->start;" and fit on a single line.
> I'm not that familiar with this style of the syntax, does that have
> the effect of setting start = _scratchspace when _scratchspace is not
> 0?

It is a GCC extension
https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/Conditionals.html which
allows you to omit the middle parameter if it is identical to the first.

It is very useful for chaining together a load of items where you want
to stop at the first non-zero one.

x = a ?: b ?: c ?: d;

but can also be used with functions calls which 0 success, nonzero error
semantics:

rc = a() ?: b() ?: c() ?: d();

If you don't need to do any special cleanup in-between them.

>>> +/*
>>> + * We only propagate -ENOMEM as individual pages may fail with -EINVAL,
>>> + * and for range sharing we only care if -ENOMEM was encountered so we 
>>> reset
>>> + * rc here.
>>> + */
>>> +if ( rc < 0 && rc != -ENOMEM )
>> Would you mind putting in an ASSERT(rc == -EINVAL) here, if we believe
>> that to be an ok case to ignore?  In the future if more errors get
>> raised, we don't want to silently lose a more serious error which should
>> be propagated up.
> Well, in that case I can just change the if statement to rc == -EINVAL.

That is a much better suggestion.

>>> @@ -1468,6 +1520,94 @@ int 
>>> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
>>>  }
>>>  break;
>>>
>>> +case XENMEM_sharing_op_range_share:
>>> +{
>>> +unsigned long max_sgfn, max_cgfn;
>>> +struct domain *cd;
>>> +
>>> +rc = -EINVAL;
>>> +if( mso.u.range._pad[0] || mso.u.range._pad[1] ||
>>> +mso.u.range._pad[2] )
>>> +goto out;
>>> +
>>> +/*
>>> + * We use _scratchscape for the hypercall continuation value.
>>> + * Ideally the user sets this to 0 in the beginning but
>>> + * there is no good way of enforcing that here, so we just 
>>> check
>>> + * that it's at least in range.
>>> + */
>>> +if ( mso.u.range._scratchspace &&
>>> +(mso.u.range._scratchspace < mso.u.range.start ||
>>> + mso.u.range._scratchspace > mso.u.range.end) )
>> Alignment (extra space) for these two lines.
> You mean add an extra space or that there is an extra space?

Please add an extra space in.  It should look like:

if ( mso.u.range._scratchspace &&
 (mso.u.range._scratchspace ...
  mso.u.range._scratchspace ...

~Andrew

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

Re: [Xen-devel] [PATCH v7] x86/mem-sharing: mem-sharing a range of memory

[Tamas K Lengyel]

On Tue, Jul 19, 2016 at 1:54 AM, Julien Grall  wrote:
> Hello Tamas,
>
> On 18/07/2016 22:14, Tamas K Lengyel wrote:
>>
>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>> index e904bd5..0ca94cd 100644
>> --- a/tools/libxc/include/xenctrl.h
>> +++ b/tools/libxc/include/xenctrl.h
>> @@ -2334,6 +2334,21 @@ int xc_memshr_add_to_physmap(xc_interface *xch,
>>  domid_t client_domain,
>>  unsigned long client_gfn);
>>
>> +/* Allows to deduplicate a range of memory of a client domain. Using
>> + * this function is equivalent of calling xc_memshr_nominate_gfn for each
>> gfn
>> + * in the two domains followed by xc_memshr_share_gfns.
>> + *
>> + * May fail with -EINVAL if the source and client domain have different
>> + * memory size or if memory sharing is not enabled on either of the
>> domains.
>> + * May also fail with -ENOMEM if there isn't enough memory available to
>> store
>> + * the sharing metadata before deduplication can happen.
>> + */
>> +int xc_memshr_range_share(xc_interface *xch,
>> +  domid_t source_domain,
>> +  domid_t client_domain,
>> +  unsigned long start,
>> +  unsigned long end);
>
>
> I know the rest of memshr interface in libxc is using "unsigned long".
> However, this should really be "uint64_t" to match the interface and avoid
> issue with 32-bit toolstack on 64-bit hypervisor.

Sounds good to me.

>
>> +
>>  /* Debug calls: return the number of pages referencing the shared frame
>> backing
>>   * the input argument. Should be one or greater.
>>   *
>
>
> [...]
>
>> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
>> index a522423..6d00228 100644
>> --- a/xen/arch/x86/mm/mem_sharing.c
>> +++ b/xen/arch/x86/mm/mem_sharing.c
>
>
> [...]
>
>> @@ -1468,6 +1520,94 @@ int
>> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
>>  }
>>  break;
>>
>> +case XENMEM_sharing_op_range_share:
>> +{
>> +unsigned long max_sgfn, max_cgfn;
>> +struct domain *cd;
>> +
>> +rc = -EINVAL;
>> +if( mso.u.range._pad[0] || mso.u.range._pad[1] ||
>
>
> NIT: missing space after the "if".
>
>> +mso.u.range._pad[2] )
>> +goto out;
>> +
>
>
> Regards,
>
> --
> Julien Grall

Thanks!
Tamas

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

Re: [Xen-devel] [PATCH v7] x86/mem-sharing: mem-sharing a range of memory

[Tamas K Lengyel]

On Mon, Jul 18, 2016 at 3:47 PM, Andrew Cooper
 wrote:
> On 18/07/2016 22:14, Tamas K Lengyel wrote:
>> Currently mem-sharing can be performed on a page-by-page basis from the 
>> control
>> domain. However, this process is quite wasteful when a range of pages have to
>> be deduplicated.
>>
>> This patch introduces a new mem_sharing memop for range sharing where
>> the user doesn't have to separately nominate each page in both the source and
>> destination domain, and the looping over all pages happen in the hypervisor.
>> This significantly reduces the overhead of sharing a range of memory.
>>
>> Signed-off-by: Tamas K Lengyel 
>> Acked-by: Wei Liu 
>
> Some style nits, and one functional suggestion.
>
> If you are happy with the suggestion, then Reviewed-by: Andrew Cooper
> 

Thanks!

>
>> diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
>> index a522423..6d00228 100644
>> --- a/xen/arch/x86/mm/mem_sharing.c
>> +++ b/xen/arch/x86/mm/mem_sharing.c
>> @@ -1294,6 +1294,58 @@ int relinquish_shared_pages(struct domain *d)
>>  return rc;
>>  }
>>
>> +static int range_share(struct domain *d, struct domain *cd,
>> +  struct mem_sharing_op_range *range)
>
> Alignment.
>
>> +{
>> +int rc = 0;
>> +shr_handle_t sh, ch;
>> +unsigned long start =
>> +range->_scratchspace ? range->_scratchspace : range->start;
>
> This can be shortened to "unsigned long start = range->_scratchspace ?:
> range->start;" and fit on a single line.

I'm not that familiar with this style of the syntax, does that have
the effect of setting start = _scratchspace when _scratchspace is not
0?

>
>> +
>> +while( range->end >= start )
>> +{
>> +/*
>> + * We only break out if we run out of memory as individual pages may
>> + * legitimately be unsharable and we just want to skip over those.
>> + */
>> +rc = mem_sharing_nominate_page(d, start, 0, );
>> +if ( rc == -ENOMEM )
>> +break;
>
> Newline here please
>
>> +if ( !rc )
>> +{
>> +rc = mem_sharing_nominate_page(cd, start, 0, );
>> +if ( rc == -ENOMEM )
>> +break;
>
> And here.
>
>> +if ( !rc )
>> +{
>> +/* If we get here this should be guaranteed to succeed. */
>> +rc = mem_sharing_share_pages(d, start, sh,
>> + cd, start, ch);
>> +ASSERT(!rc);
>> +}
>> +}
>> +
>> +/* Check for continuation if it's not the last iteration. */
>> +if ( range->end >= ++start && hypercall_preempt_check() )
>> +{
>> +rc = 1;
>> +break;
>> +}
>> +}
>> +
>> +range->_scratchspace = start;
>> +
>> +/*
>> + * We only propagate -ENOMEM as individual pages may fail with -EINVAL,
>> + * and for range sharing we only care if -ENOMEM was encountered so we 
>> reset
>> + * rc here.
>> + */
>> +if ( rc < 0 && rc != -ENOMEM )
>
> Would you mind putting in an ASSERT(rc == -EINVAL) here, if we believe
> that to be an ok case to ignore?  In the future if more errors get
> raised, we don't want to silently lose a more serious error which should
> be propagated up.

Well, in that case I can just change the if statement to rc == -EINVAL.

>
>> +rc = 0;
>> +
>> +return rc;
>> +}
>> +
>>  int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
>>  {
>>  int rc;
>> @@ -1468,6 +1520,94 @@ int 
>> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
>>  }
>>  break;
>>
>> +case XENMEM_sharing_op_range_share:
>> +{
>> +unsigned long max_sgfn, max_cgfn;
>> +struct domain *cd;
>> +
>> +rc = -EINVAL;
>> +if( mso.u.range._pad[0] || mso.u.range._pad[1] ||
>> +mso.u.range._pad[2] )
>> +goto out;
>> +
>> +/*
>> + * We use _scratchscape for the hypercall continuation value.
>> + * Ideally the user sets this to 0 in the beginning but
>> + * there is no good way of enforcing that here, so we just check
>> + * that it's at least in range.
>> + */
>> +if ( mso.u.range._scratchspace &&
>> +(mso.u.range._scratchspace < mso.u.range.start ||
>> + mso.u.range._scratchspace > mso.u.range.end) )
>
> Alignment (extra space) for these two lines.

You mean add an extra space or that there is an extra space?

>
>> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
>> index 29ec571..e0bc018 100644
>> --- a/xen/include/public/memory.h
>> +++ b/xen/include/public/memory.h
>> @@ -500,7 +501,14 @@ struct xen_mem_sharing_op {
>>  uint64_aligned_t client_gfn; 

Re: [Xen-devel] Xen 4.8 Development Update

[Wei Liu]

On Tue, Jul 19, 2016 at 10:06:57AM -0600, Tamas K Lengyel wrote:
> On Tue, Jul 19, 2016 at 7:48 AM, Wei Liu  wrote:
> > This email only tracks big items for xen.git tree. Please reply for items 
> > you
> > woulk like to see in 4.8 so that people have an idea what is going on and
> > prioritise accordingly.
> >
> > You're welcome to provide description and use cases of the feature you're
> > working on.
> >
> > = Timeline =
> >
> > We now adopt a fixed cut-off date scheme. We will release twice a
> > year. The upcoming 4.8 timeline are as followed:
> >
> > * Last posting date: September 16, 2016
> > * Hard code freeze: September 30, 2016
> > * RC1: TBD
> > * Release: December 2, 2016
> >
> > Note that we don't have freeze exception scheme anymore. All patches
> > that wish to go into 4.8 must be posted no later than the last posting
> > date. All patches posted after that date will be automatically queued
> > into next release.
> >
> > RCs will be arranged immediately after freeze.
> >
> > = Projects =
> 
> [...]
> 
> > === ARM ===
> 
> I'm mentoring Sergej Proskurin who is developing altp2m for ARM as
> part of a Google Summer of Code project. Current status is that first
> version of the series has been posted, working on reviews, second
> version should be sent shortly. We are hopeful to be able to meet the
> merge window with this for 4.8.
> 

Right. I missed that.

Looking forward to the patches!

Wei.

> Tamas

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

Re: [Xen-devel] Xen 4.8 Development Update

[Tamas K Lengyel]

On Tue, Jul 19, 2016 at 7:48 AM, Wei Liu  wrote:
> This email only tracks big items for xen.git tree. Please reply for items you
> woulk like to see in 4.8 so that people have an idea what is going on and
> prioritise accordingly.
>
> You're welcome to provide description and use cases of the feature you're
> working on.
>
> = Timeline =
>
> We now adopt a fixed cut-off date scheme. We will release twice a
> year. The upcoming 4.8 timeline are as followed:
>
> * Last posting date: September 16, 2016
> * Hard code freeze: September 30, 2016
> * RC1: TBD
> * Release: December 2, 2016
>
> Note that we don't have freeze exception scheme anymore. All patches
> that wish to go into 4.8 must be posted no later than the last posting
> date. All patches posted after that date will be automatically queued
> into next release.
>
> RCs will be arranged immediately after freeze.
>
> = Projects =

[...]

> === ARM ===

I'm mentoring Sergej Proskurin who is developing altp2m for ARM as
part of a Google Summer of Code project. Current status is that first
version of the series has been posted, working on reviews, second
version should be sent shortly. We are hopeful to be able to meet the
merge window with this for 4.8.

Tamas

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

Re: [Xen-devel] [PATCH v2] xl: add option to leave domain paused after migration

[Wei Liu]

On Tue, Jul 19, 2016 at 05:50:32PM +0200, Roger Pau Monne wrote:
> On Tue, Jul 19, 2016 at 02:15:28PM +0100, Wei Liu wrote:
> > On Tue, Jul 19, 2016 at 10:58:15AM +0200, Roger Pau Monne wrote:
> > > This is useful for debugging domains that crash on resume from migration.
> > > 
> > > Signed-off-by: Roger Pau Monné 
> > > ---
> > > Cc: ian.jack...@eu.citrix.com
> > > Cc: wei.l...@citrix.com
> > > ---
> > > Changes since v1:
> > >  - Document the newly added option in the xl man page.
> > > ---
> > >  docs/man/xl.pod.1 |  4 
> > >  tools/libxl/xl_cmdimpl.c  | 29 +++--
> > >  tools/libxl/xl_cmdtable.c |  3 ++-
> > >  3 files changed, 25 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> > > index f4dc32c..f3a2bcb 100644
> > > --- a/docs/man/xl.pod.1
> > > +++ b/docs/man/xl.pod.1
> > 
> > Actually you should patch xl.pod.1.in.
> > 
> > No need to resend. I've fixed it up for you.
> 
> Oh, I don't remember there being a ".in" version of the man page in the 
> past, sorry. Thanks for fixing it up.
> 

It was introduced recently by me. I wouldn't be surprised if people
aren't aware of its existence.

Wei.

> Roger.

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

Re: [Xen-devel] [PATCH v2] xl: add option to leave domain paused after migration

[Roger Pau Monne]

On Tue, Jul 19, 2016 at 02:15:28PM +0100, Wei Liu wrote:
> On Tue, Jul 19, 2016 at 10:58:15AM +0200, Roger Pau Monne wrote:
> > This is useful for debugging domains that crash on resume from migration.
> > 
> > Signed-off-by: Roger Pau Monné 
> > ---
> > Cc: ian.jack...@eu.citrix.com
> > Cc: wei.l...@citrix.com
> > ---
> > Changes since v1:
> >  - Document the newly added option in the xl man page.
> > ---
> >  docs/man/xl.pod.1 |  4 
> >  tools/libxl/xl_cmdimpl.c  | 29 +++--
> >  tools/libxl/xl_cmdtable.c |  3 ++-
> >  3 files changed, 25 insertions(+), 11 deletions(-)
> > 
> > diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> > index f4dc32c..f3a2bcb 100644
> > --- a/docs/man/xl.pod.1
> > +++ b/docs/man/xl.pod.1
> 
> Actually you should patch xl.pod.1.in.
> 
> No need to resend. I've fixed it up for you.

Oh, I don't remember there being a ".in" version of the man page in the 
past, sorry. Thanks for fixing it up.

Roger.

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

[Xen-devel] [PATCH v2 2/2] xen: credit2: fix potential issues in csched2_cpu_pick with tracing enabled

[Dario Faggioli]

In fact, when not finding a suitable runqueue where to
place a vCPU, and hence using a fallback, we either:
 - don't issue any trace record (while we should),
 - risk underruning when accessing the runqueues
   array, while preparing the trace record.

Fix both issues and, while there, also a couple of style
problems found nearby.

Spotted by Coverity.

Signed-off-by: Dario Faggioli 
Reported-by: Andrew Cooper 
---
Cc: George Dunlap 
Cc: Anshul Makkar 
---
Changes from v1:
 * cite Coverity in the changelog.
---
 xen/common/sched_credit2.c |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index a55240f..3009ff9 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1443,7 +1443,8 @@ csched2_cpu_pick(const struct scheduler *ops, struct vcpu 
*vc)
 {
 /* We may be here because someone requested us to migrate. */
 __clear_bit(__CSFLAG_runq_migrate_request, >flags);
-return get_fallback_cpu(svc);
+new_cpu = get_fallback_cpu(svc);
+goto out;
 }
 
 /* First check to see if we're here because someone else suggested a place
@@ -1505,7 +1506,7 @@ csched2_cpu_pick(const struct scheduler *ops, struct vcpu 
*vc)
 if ( rqd_avgload < min_avgload )
 {
 min_avgload = rqd_avgload;
-min_rqi=i;
+min_rqi = i;
 }
 }
 
@@ -1520,20 +1521,20 @@ csched2_cpu_pick(const struct scheduler *ops, struct 
vcpu *vc)
 BUG_ON(new_cpu >= nr_cpu_ids);
 }
 
-out_up:
+ out_up:
 read_unlock(>lock);
-
+ out:
 if ( unlikely(tb_init_done) )
 {
 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;
 d.dom = vc->domain->domain_id;
 d.vcpu = vc->vcpu_id;
 d.rq_id = c2r(ops, new_cpu);
+d.b_avgload = prv->rqd[d.rq_id].b_avgload;
 d.new_cpu = new_cpu;
 __trace_var(TRC_CSCHED2_PICKED_CPU, 1,
 sizeof(d),


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

[Xen-devel] [PATCH v2 1/2] xen: credit2: fix two s_time_t handling issues in load balancing

[Dario Faggioli]

both introduced in d205f8a7f48e2ec ("xen: credit2: rework
load tracking logic").

First, in __update_runq_load(), the ASSERT() was actually
useless. Let's instead check that the computed value of
the load has not overflowed (and hence gone negative).

While there, do that in __update_svc_load() as well.

Second, in balance_load(), cpus_max needs being extended
in order to be correctly shifted, and the result compared
with an s_time_t value, without risking loosing info.

Spotted by Coverity.

Signed-off-by: Dario Faggioli 
Reported-by: Andrew Cooper 
---
Cc: George Dunlap 
Cc: Anshul Makkar 
---
Changed from v1:
 * fixed a '> 0' which wanted to be '>= 0' in the ASSERT()-s;
 * cite Coverity in the changelog.
---
 xen/common/sched_credit2.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index b33ba7a..a55240f 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -656,7 +656,8 @@ __update_runq_load(const struct scheduler *ops,
 rqd->load += change;
 rqd->load_last_update = now;
 
-ASSERT(rqd->avgload <= STIME_MAX && rqd->b_avgload <= STIME_MAX);
+/* Overflow, capable of making the load look negative, must not occur. */
+ASSERT(rqd->avgload >= 0 && rqd->b_avgload >= 0);
 
 if ( unlikely(tb_init_done) )
 {
@@ -714,6 +715,9 @@ __update_svc_load(const struct scheduler *ops,
 }
 svc->load_last_update = now;
 
+/* Overflow, capable of making the load look negative, must not occur. */
+ASSERT(svc->avgload >= 0);
+
 if ( unlikely(tb_init_done) )
 {
 struct {
@@ -1742,7 +1746,7 @@ retry:
  * If we're under 100% capacaty, only shift if load difference
  * is > 1.  otherwise, shift if under 12.5%
  */
-if ( load_max < (cpus_max << prv->load_precision_shift) )
+if ( load_max < ((s_time_t)cpus_max << prv->load_precision_shift) )
 {
 if ( st.load_delta < (1ULL << (prv->load_precision_shift +
opt_underload_balance_tolerance)) )


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

[Xen-devel] [PATCH v2 0/2] xen: Credit2: fix two issues from recently committed series.

[Dario Faggioli]

v2 of <146892985892.30642.2392453881110942183.st...@solace.fritz.box>, as v1
was making things worse!

In fact, there was a bug in patch 1 which turned the ASSERT() from being
useless to being wrong, and it was actually triggering.

Sorry for the noise.

Regards,
Dario
---
Dario Faggioli (2):
  xen: credit2: fix two s_time_t handling issues in load balancing
  xen: credit2: fix potential issues in csched2_cpu_pick with tracing 
enabled

 xen/common/sched_credit2.c |   21 +
 1 file changed, 13 insertions(+), 8 deletions(-)
--
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)

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

Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[Tamas K Lengyel]

On Tue, Jul 19, 2016 at 7:36 AM, Ian Jackson  wrote:
> Tamas: George brought this thread to my attention.  I'm sorry that you
> feel blocked and/or overruled.  The hypervisor MM code is not my area
> of expertise, but I have a keen interest in seeing a good, productive
> and friendly Xen community.  I definitely don't want to see you pushed
> away, and driven to maintain an out-of-tree patchset.
>
> Reading through your mails there seem to still have unresolved
> detailed technical disagreements between you and George about the
> existing behaviours in Xen, and the effects of your proposed changes.
>
> Right now I would like to ask both you and George to sort out those
> factual disagreements.  I expect that you can do so.  I hope that then
> the way forward will be clear: ie that you and George willbe in
> agreement about the direction in which the code should be going.
>
> I think that would be better than getting into a more abstract
> conversation about which use cases exist or are important, or an
> argument about areas of responsibility or authority.
>
> If either of you feel that you aren't able to agree on the facts, or
> that that conversation is not proceeding constructively, I'm be happy
> to try to help.  You can contact me by email in public or private, or
> find me as Diziet on irc.
>
> (In a complex codebase like Xen there will always be overlap or
> interference between different maintainers' bailiwicks, so we
> definitely do need to be able to come to some kind of agreement,
> rather than everyone insisting on their own authority in what they
> regard as their own area.)
>
> Regards,
> Ian.

Thanks Ian, I agree and hope we can get back to technical issues as
well. I certainly didn't mean to escalate this. I do hope we can get
to the bottom of what concerns are applicable to this change and
discuss what we can do to address those in a reasonable fashion.

Best,
Tamas

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

Re: [Xen-devel] [PATCH] XSM-docs: Flask operates on domain types and not on individual domain. Updated the documentation to reflect this.

[Daniel De Graaf]

On 07/19/2016 11:21 AM, anshul makkar wrote:

On 19/07/16 14:30, Doug Goldstein wrote:

On 7/19/16 4:05 AM, Anshul Makkar wrote:

Signed-off-by: Anshul Makkar 
---
  * Resending the patch due to incomplete subject in the previous patch.

  docs/misc/xsm-flask.txt | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
---
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 62f15dd..bf8bb6e 100644
  Some examples of what FLASK can do:
- - Prevent two domains from communicating via event channels or grants
- - Control which domains can use device passthrough (and which devices)
+ - Prevent two domains types from communicating via event channels or grants
+ - Control which type of domains can use device passthrough (and which devices)


I disagree with this snippet. This is an example of what you can do with
FLASK. You can use flask to do those two actions. Adding the word
"types" in there takes it from being a concrete example to being more
ambiguous.

"Prevent domains belonging to different types to communicate via event channels or 
grants". Does this sounds better.

I think that its important to use the word "type" so that user doesn't get a 
wrong impression that the policy is per domain, while in actual its per type.


I think it would be clearer to leave the examples as is, but add a sentence to 
the
following paragraph about how the policy is written based on types.

For the other changes, I agree Doug's rewording is a bit clearer than the 
original.

--
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] XSM-docs: Flask operates on domain types and not on individual domain. Updated the documentation to reflect this.

[anshul makkar]

On 19/07/16 14:30, Doug Goldstein wrote:

On 7/19/16 4:05 AM, Anshul Makkar wrote:

Signed-off-by: Anshul Makkar 
---
  * Resending the patch due to incomplete subject in the previous patch.

  docs/misc/xsm-flask.txt | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
---
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 62f15dd..bf8bb6e 100644
  Some examples of what FLASK can do:
- - Prevent two domains from communicating via event channels or grants
- - Control which domains can use device passthrough (and which devices)
+ - Prevent two domains types from communicating via event channels or grants
+ - Control which type of domains can use device passthrough (and which devices)


I disagree with this snippet. This is an example of what you can do with
FLASK. You can use flask to do those two actions. Adding the word
"types" in there takes it from being a concrete example to being more
ambiguous.
"Prevent domains belonging to different types to communicate via event 
channels or grants". Does this sounds better.


I think that its important to use the word "type" so that user doesn't 
get a wrong impression that the policy is per domain, while in actual 
its per type.



   - Restrict or audit operations performed by privileged domains
   - Prevent a privileged domain from arbitrarily mapping pages from other 
domains

@@ -160,10 +160,10 @@ the policy can be reloaded using "xl loadpolicy".
  The example policy included with Xen demonstrates most of the features of 
FLASK
  that can be used without dom0 disaggregation. The main types for domUs are:

- - domU_t is a domain that can communicate with any other domU_t
+ - domU_t is a domain type that can communicate with any other domU_t types.


"A domain labeled with domU_t can communicate with any other domain
labeled with type domU_t."

Rephrasing is fine.



   - isolated_domU_t can only communicate with dom0
   - prot_domU_t is a domain type whose creation can be disabled with a boolean
- - nomigrate_t is a domain that must be created via the nomigrate_t_building
+ - nomigrate_t is a domain type that must be created via the 
nomigrate_t_building
 type, and whose memory cannot be read by dom0 once created


"A domain labeled with nomigeate_t is a domain that"

Rephrasing is fine.




  HVM domains with stubdomain device models also need a type for the stub 
domain.







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

Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[Tamas K Lengyel]

On Tue, Jul 19, 2016 at 5:39 AM, George Dunlap  wrote:
> On 18/07/16 18:06, Tamas K Lengyel wrote:
 Anyhow, at this point I'm
 going to start carrying out-of-tree patches for Xen in my project and
 just resign from my mem_sharing maintainership as I feel like it's
 pretty pointless.
>>>
>>> I'm sorry that you're discouraged; all I can say is that I hope you
>>> reconsider.  I'm not trying to block you, and I'm not ignoring your use
>>> case; it's the job of a maintainer to look at *everyone's* use cases and
>>> try to make sure that they are all accommodated in so far as it is
>>> possible.
>>>
>>> I'm also trying to make sure that the code you end up using in your
>>> project is robust and reliable.  It seems to me like if the current
>>> implementation was fixed, your life would be a lot easier than if we
>>> accept your patch as it is -- your sharing code could just worry about
>>> sharing, your altp2m code could just worry about whatever it's trying to
>>> do, without having to carefully avoid corner cases or manually fix
>>> things up when corner cases happen.  A bit less sharing would happen,
>>> because fewer pages would be eligible to be shared, but overall you'd
>>> have a lot less bugs and headache.
>>>
>>> I invested a lot of my very limited time carefully going through both
>>> sets of code before I answered your e-mail, and I spent a lot of time
>>> trying to explain the kinds of interactions I think will be a problem.
>>> I could have just acked the patch without doing that; but I think that
>>> would have been both less good for you, and less good for the project as
>>> a whole.
>>
>> I certainly appreciate your time spent on this. However, I don't see
>> the point of being maintainer if my opinion on what constitutes an
>> improvement of the system just gets overruled.
>
> You're not being overruled; you're just being asked to make a case for a
> change you want to make to an area of code that I maintain (the p2m
> code).  And the discussion is by no means over; I started the most
> recent discussion by saying "Correct me if I'm wrong", and it looks like
> there are still a number of places where we have different views of the
> facts of the matter.  Once we've established those we may end up with
> closer opinions.
>
> Working together means that sometimes you have to spend the time and
> effort to understand where other people are coming from -- what they
> think is important, what they think is true; and then working with that
> -- correcting them on places where they have misconceptions (or
> double-checking your own beliefs to make sure that you're not mistaken);
> communicating what it is that you think is important, and then trying to
> come up with a way forward that takes everyone's values into account, or
> convincing someone that a particular way really is the best way forward
> (which may mean convincing them to change their priorities somewhat).
>
> I am sorry that the tone of this conversation has heated up.  But the
> reason I've been "raising my voice" as it were is because I've been
> trying to ask questions and raise potential issues, and I feel like
> you've been just hand-waving them away.  You may be 100% right, but it
> is my duty as the maintainer of the p2m code to not accept code until
> I'm reasonably convinced that it's a good way forward.

By no means I meant to heat-up the conversation or hand-wave your
concerns away. I do understand what it takes to work with the
community and that it takes cooperation for that to happen. I was not
hand-waving your concerns away but describing how the two systems
could interact safely together while agreeing with you that yes, there
are still scenarios where it would not be wise to turn two
experimental systems on together.

>
>> I would like to hear the
>> other maintainers opinion on this matter as well but I'm not
>> interested in arguing endlessly or initiating or vote, so if the patch
>> is not allowed in I will accept that decision but I will see no point
>> in continuing as maintainer of the system.
>
> At a basic level, the other maintainers will agree that I shouldn't
> accept code unless I am convinced it's for the good of the project.  But
> since this is a technical issue, before anyone would express an opinion
> to ask me to change my mind, they would want a more complete view of the
> facts of the matter -- facts which you and I are still in the process of
> sorting out.

Both of these systems are fairly complicated and not many people have
been looking at them in-depth, so I most certainly appreciate the time
you spent on reviewing thus far. But your conclusion that there is a
"long way to go here" tells me that an arbitrary criteria is getting
pushed on me that I don't even know how to address. The altp2m system
got merged while it was known that it crashes the hypervisor when
mem_sharing is used, but now when an incremental fix introduces at
least one setup where they 

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

[osstest service owner]

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

Failures :-/ but no regressions.

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

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

version targeted for testing:
 xen  e763268781d341fef05d461f3057e6ced5e033f2
baseline version:
 xen  b48be35ac86cd6369124cf06ca3006d086095297

Last test of basis97562  2016-07-18 02:08:02 Z1 days
Testing same since97623  2016-07-18 21:15:06 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Dario Faggioli 
  George Dunlap 
  Wei Liu 

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

Re: [Xen-devel] [PATCH] acpi: Re-license ACPI builder files from GPLv2 to LGPLv2.1

[Daniel Kiper]

On Mon, Jul 18, 2016 at 10:01:27AM -0400, Boris Ostrovsky wrote:
> ACPI builder is currently distributed under GPLv2 license.
>
> We plan to make the builder available to components other
> than the hvmloader (which is also GPLv2). Some of these
> components (such as libxl) may be distributed under LGPL-2.1
> so that they can be used by non-GPLv2 callers.  But this
> will not be possible if we incorporate the ACPI builder in
> those other components.
>
> To avoid this problem we are relicensing sources in ACPI
> bulder directory to the Lesser GNU Public License (LGPL)
> version 2.1
>
> Signed-off-by: Boris Ostrovsky 
> CC: Kouya Shimura 
> CC: Daniel Kiper 
> CC: Stefan Berger 
> CC: Simon Horman 
> CC: Keir Fraser 
> CC: Ian Jackson 
> CC: Lars Kurth 

Acked-by: Daniel Kiper 

Daniel

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

Re: [Xen-devel] [PATCH 2/3] xen-scsiback: One function call less in scsiback_device_action() after error detection

[SF Markus Elfring]

>> @@ -606,7 +606,7 @@ static void scsiback_device_action(struct vscsibk_pend 
>> *pending_req,
>>  tmr = kzalloc(sizeof(struct scsiback_tmr), GFP_KERNEL);
>>  if (!tmr) {
>>  target_put_sess_cmd(se_cmd);
>> -goto err;
>> +goto do_resp;
>>  }
> 
> Hmm, I'm not convinced this is an improvement.
> 
> I'd rather rename the new error label to "put_cmd" and get rid of the
> braces in above if statement:
> 
> - if (!tmr) {
> - target_put_sess_cmd(se_cmd);
> - goto err;
> - }
> + if (!tmr)
> + goto put_cmd;
> 
> and then in the error path:
> 
> -err:
> +put_cmd:
> + target_put_sess_cmd(se_cmd);

I am unsure on the relevance of this function on such a source position.
Would it make sense to move it further down at the end?


> +free_tmr:
>   kfree(tmr);

How do you think about to skip this function call after a memory
allocation failure?

Regards,
Markus

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

Re: [Xen-devel] [PATCH v1 10/20] acpi/hvmloader: Replace mem_alloc() and virt_to_phys() with memory ops

[Boris Ostrovsky]

On 07/19/2016 05:11 AM, Jan Beulich wrote:
 Boris Ostrovsky  07/08/16 6:20 PM >>>
>> On 07/08/2016 11:35 AM, Jan Beulich wrote:
>> On 08.07.16 at 17:23,  wrote:
 Is it up to the builder to decide which tables are important and which
 are not?
>>> I'm afraid that's not so easy to tell. If for example we can't fit the
>>> HPET table, the guest could be run without HPET unless a HPET
>>> was specifically requested (rather than just defaulted to).
>> But again --- how will the caller know that it was only HPET table that
>> was not built?
> Why would the caller care? I guess examples could be found where it is
> necessary for the caller to know, but for the specific example (and at least
> some others) failure is of no interest to the caller - it's only the guest 
> which
> is affected.

HPET was just an example, the same question could be asked for (almost)
any other table.

But I can see that we can defer to the guest to deal with ACPI
brokenness, although some not built tables will almost certainly lead to
guest's failure.

(We probably will not get to use this new free() op anyway since failure
to allocate memory is currently the only possible error and there is one
allocation per table)


-boris






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

[Xen-devel] Xen 4.8 Development Update

[Wei Liu]

This email only tracks big items for xen.git tree. Please reply for items you
woulk like to see in 4.8 so that people have an idea what is going on and
prioritise accordingly.

You're welcome to provide description and use cases of the feature you're
working on.

= Timeline =

We now adopt a fixed cut-off date scheme. We will release twice a
year. The upcoming 4.8 timeline are as followed:

* Last posting date: September 16, 2016
* Hard code freeze: September 30, 2016
* RC1: TBD
* Release: December 2, 2016

Note that we don't have freeze exception scheme anymore. All patches
that wish to go into 4.8 must be posted no later than the last posting
date. All patches posted after that date will be automatically queued
into next release.

RCs will be arranged immediately after freeze.

= Projects =

== Hypervisor == 

*  Make credit2 default scheduler for Xen
  -  Dario Faggioli

=== x86 === 

*  Allow ioreq server interface to support XenGT
  -  Yu Zhang
  -  Paul Durrant

*  PVHv2 support
  -  Roger Pau Monne

*  vNVDIMM support
  -  Haozhong Zhang

=== ARM === 

*  Xen ARM DomU ACPI support
  -  Shannon Zhao

*  Alternative patching support
  -  Julien Grall

== Toolstack == 

*  Make ACPI builder available to components other than hvmloader
  -  Boris Ostrovsky

*  Libxl PVSCSI support
  -  Olaf Hering

*  HVM USB passthrough
  -  George Dunlap

*  Load BIOS via toolstack
  -  Anthony Perard

*  Libxl depriv QEMU
  -  Ian Jackson

*  Clean up all hard-coded paths in toolstack code
  -  Wei Liu

*  Logging solution for Xen system
  -  Wei Liu

== Mini-OS == 

*  Mini-os ballooning support
  -  Juergen Gross

== Documentation == 

*  Feature maturity lifecycle
  -  Lars Kurth

== Completed == 

*  Refactor libxl device handling framework
  -  Juergen Gross

*  IOMMU flush issue
  -  Quan Xu

*  Refactor XSM policy
  -  Daniel De Graaf


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

Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[Ian Jackson]

Tamas: George brought this thread to my attention.  I'm sorry that you
feel blocked and/or overruled.  The hypervisor MM code is not my area
of expertise, but I have a keen interest in seeing a good, productive
and friendly Xen community.  I definitely don't want to see you pushed
away, and driven to maintain an out-of-tree patchset.

Reading through your mails there seem to still have unresolved
detailed technical disagreements between you and George about the
existing behaviours in Xen, and the effects of your proposed changes.

Right now I would like to ask both you and George to sort out those
factual disagreements.  I expect that you can do so.  I hope that then
the way forward will be clear: ie that you and George willbe in
agreement about the direction in which the code should be going.

I think that would be better than getting into a more abstract
conversation about which use cases exist or are important, or an
argument about areas of responsibility or authority.

If either of you feel that you aren't able to agree on the facts, or
that that conversation is not proceeding constructively, I'm be happy
to try to help.  You can contact me by email in public or private, or
find me as Diziet on irc.

(In a complex codebase like Xen there will always be overlap or
interference between different maintainers' bailiwicks, so we
definitely do need to be able to come to some kind of agreement,
rather than everyone insisting on their own authority in what they
regard as their own area.)

Regards,
Ian.

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

Re: [Xen-devel] [PATCH 1/2] xen: credit2: fix two s_time_t handling issues in load balancing

[Dario Faggioli]

On Tue, 2016-07-19 at 14:07 +0200, Dario Faggioli wrote:
> --- a/xen/common/sched_credit2.c
> +++ b/xen/common/sched_credit2.c
> @@ -656,7 +656,8 @@ __update_runq_load(const struct scheduler *ops,
>  rqd->load += change;
>  rqd->load_last_update = now;
>  
> -ASSERT(rqd->avgload <= STIME_MAX && rqd->b_avgload <=
> STIME_MAX);
> +/* Overflow, capable of making the load look negative, must not
> occur. */
> +ASSERT(rqd->avgload > 0 && rqd->b_avgload > 0);
>  
Wait! This quite obviously wants to be '>= 0' !!

Sorry for the glaring mistake. v2 coming...

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] XSM-docs: Flask operates on domain types and not on individual domain. Updated the documentation to reflect this.

[Doug Goldstein]

On 7/19/16 4:05 AM, Anshul Makkar wrote:
> Signed-off-by: Anshul Makkar 
> ---
>  * Resending the patch due to incomplete subject in the previous patch.
> 
>  docs/misc/xsm-flask.txt | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> ---
> diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
> index 62f15dd..bf8bb6e 100644
> --- a/docs/misc/xsm-flask.txt
> +++ b/docs/misc/xsm-flask.txt
> @@ -9,8 +9,8 @@ controls over Xen domains, allowing the policy writer to 
> define what
>  interactions between domains, devices, and the hypervisor are permitted.
>  
>  Some examples of what FLASK can do:
> - - Prevent two domains from communicating via event channels or grants
> - - Control which domains can use device passthrough (and which devices)
> + - Prevent two domains types from communicating via event channels or grants
> + - Control which type of domains can use device passthrough (and which 
> devices)

I disagree with this snippet. This is an example of what you can do with
FLASK. You can use flask to do those two actions. Adding the word
"types" in there takes it from being a concrete example to being more
ambiguous.

>   - Restrict or audit operations performed by privileged domains
>   - Prevent a privileged domain from arbitrarily mapping pages from other 
> domains
>  
> @@ -160,10 +160,10 @@ the policy can be reloaded using "xl loadpolicy".
>  The example policy included with Xen demonstrates most of the features of 
> FLASK
>  that can be used without dom0 disaggregation. The main types for domUs are:
>  
> - - domU_t is a domain that can communicate with any other domU_t
> + - domU_t is a domain type that can communicate with any other domU_t types.

"A domain labeled with domU_t can communicate with any other domain
labeled with type domU_t."

>   - isolated_domU_t can only communicate with dom0
>   - prot_domU_t is a domain type whose creation can be disabled with a boolean
> - - nomigrate_t is a domain that must be created via the nomigrate_t_building
> + - nomigrate_t is a domain type that must be created via the 
> nomigrate_t_building
> type, and whose memory cannot be read by dom0 once created

"A domain labeled with nomigeate_t is a domain that"

>  
>  HVM domains with stubdomain device models also need a type for the stub 
> domain.
> 


-- 
Doug Goldstein



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 v3] libxl: trigger attach events for devices attached before xl devd startup

[Wei Liu]

On Mon, Jul 18, 2016 at 04:31:30PM +0100, Wei Liu wrote:
> On Sat, Jul 16, 2016 at 01:47:56AM +0200, Marek Marczykowski-Górecki wrote:
> > When this daemon is started after creating backend device, that device
> > will not be configured.
> > 
> > Racy situation:
> > 1. driver domain is started
> > 2. frontend domain is started (just after kicking driver domain off)
> > 3. device in frontend domain is connected to the backend (as specified
> >in frontend domain configuration)
> > 4. xl devd is started in driver domain
> > 
> > End result is that backend device in driver domain is not configured
> > (like network interface is not enabled), so the device doesn't work.
> > 
> > Fix this by artifically triggering events for devices already present in
> > xenstore before xl devd is started. Do this only after xenstore watch is
> > already registered, and only for devices not already initialized (in
> > XenbusStateInitWait state).
> > 
> > Cc: Ian Jackson 
> > Cc: Wei Liu 
> > Signed-off-by: Marek Marczykowski-Górecki 
> 
> Acked-by: Wei Liu 

Queued. Thanks.

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

Re: [Xen-devel] [PATCH v3 0/5] xenstore: fix memory leak of xenstored

[Wei Liu]

Queued. Thanks.

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

Re: [Xen-devel] [PATCH v2] xenstore: add memory allocation debugging capability

[Wei Liu]

On Tue, Jul 19, 2016 at 02:08:18PM +0200, Juergen Gross wrote:
> Add support for debugging memory allocation statistics to xenstored.
> Specifying "-M " on the command line will enable the feature.
> Whenever xenstored receives SIGUSR1 it will dump out a full talloc
> report to . This helps finding e.g. memory leaks in xenstored.
> 
> Signed-off-by: Juergen Gross 

Queued with Ian's ack.

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

Re: [Xen-devel] [PATCH v2] xl: add option to leave domain paused after migration

[Wei Liu]

On Tue, Jul 19, 2016 at 10:58:15AM +0200, Roger Pau Monne wrote:
> This is useful for debugging domains that crash on resume from migration.
> 
> Signed-off-by: Roger Pau Monné 
> ---
> Cc: ian.jack...@eu.citrix.com
> Cc: wei.l...@citrix.com
> ---
> Changes since v1:
>  - Document the newly added option in the xl man page.
> ---
>  docs/man/xl.pod.1 |  4 
>  tools/libxl/xl_cmdimpl.c  | 29 +++--
>  tools/libxl/xl_cmdtable.c |  3 ++-
>  3 files changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index f4dc32c..f3a2bcb 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1

Actually you should patch xl.pod.1.in.

No need to resend. I've fixed it up for you.

Wei.

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

Re: [Xen-devel] [PATCH 0/2] xen: Credit2: fix two issues from recently committed series.

[Andrew Cooper]

On 19/07/16 13:26, Dario Faggioli wrote:
> On Tue, 2016-07-19 at 13:20 +0100, Andrew Cooper wrote:
>> On 19/07/16 13:06, Dario Faggioli wrote:
>>> Series committed yesterday, to be precise, and two (not too big, at
>>> least :-))
>>> issues, have been found already (thanks Andrew!).
>> I tend to put "Spotted by Coverity." in the commit message of these,
>> even when we can't provide CID numbers.
>>
>> Its not like I spotted these from code inspection.
>>
> Ok, I see. As you say, since I could not provide a meaningful CID, that
> would not seem too useful to say either, to me.
>
> But in any case, if you feel strong enough about it, I'm ok resending
> with that, or about anyone committing the patches adding it.

I am sure this can be tweaked on commit if there are no other issues.

I presume George will take care of committing in due course.

~Andrew

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

Re: [Xen-devel] [PATCH 0/2] xen: Credit2: fix two issues from recently committed series.

[Dario Faggioli]

On Tue, 2016-07-19 at 13:20 +0100, Andrew Cooper wrote:
> On 19/07/16 13:06, Dario Faggioli wrote:
> > 
> > Series committed yesterday, to be precise, and two (not too big, at
> > least :-))
> > issues, have been found already (thanks Andrew!).
> I tend to put "Spotted by Coverity." in the commit message of these,
> even when we can't provide CID numbers.
> 
> Its not like I spotted these from code inspection.
> 
Ok, I see. As you say, since I could not provide a meaningful CID, that
would not seem too useful to say either, to me.

But in any case, if you feel strong enough about it, I'm ok resending
with that, or about anyone committing the patches adding it.

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



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

Re: [Xen-devel] [PATCH 0/2] xen: Credit2: fix two issues from recently committed series.

[Andrew Cooper]

On 19/07/16 13:06, Dario Faggioli wrote:
> Series committed yesterday, to be precise, and two (not too big, at least :-))
> issues, have been found already (thanks Andrew!).

I tend to put "Spotted by Coverity." in the commit message of these,
even when we can't provide CID numbers.

Its not like I spotted these from code inspection.

~Andrew

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

[Xen-devel] [PATCH v2] xenstore: add memory allocation debugging capability

[Juergen Gross]

Add support for debugging memory allocation statistics to xenstored.
Specifying "-M " on the command line will enable the feature.
Whenever xenstored receives SIGUSR1 it will dump out a full talloc
report to . This helps finding e.g. memory leaks in xenstored.

Signed-off-by: Juergen Gross 
---
To be applied on top of my "xenstore: fix memory leak of xenstored"
series. In fact this patch was used to find the problem the series
fixed and I used it to verify the patches are working.

Signed-off-by: Juergen Gross 
---
 tools/xenstore/xenstored_core.c | 34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 51fb0b3..8bb1eff 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -81,6 +81,7 @@ static int reopen_log_pipe[2];
 static int reopen_log_pipe0_pollfd_idx = -1;
 static char *tracefile = NULL;
 static TDB_CONTEXT *tdb_ctx = NULL;
+static bool trigger_talloc_report = false;
 
 static void corrupt(struct connection *conn, const char *fmt, ...);
 static void check_store(void);
@@ -1743,6 +1744,10 @@ static void init_sockets(int **psock, int **pro_sock)
static int minus_one = -1;
*psock = *pro_sock = _one;
 }
+
+static void do_talloc_report(int sig)
+{
+}
 #else
 static int destroy_fd(void *_fd)
 {
@@ -1876,6 +1881,11 @@ static void init_sockets(int **psock, int **pro_sock)
 
 
 }
+
+static void do_talloc_report(int sig)
+{
+   trigger_talloc_report = true;
+}
 #endif
 
 static void usage(void)
@@ -1901,6 +1911,7 @@ static void usage(void)
 "  the store is corrupted (debug only),\n"
 "  -I, --internal-db   store database in memory, not on disk\n"
 "  -L, --preserve-localto request that /local is preserved on start-up,\n"
+"  -M, --memory-debug   support memory debugging to file,\n"
 "  -V, --verbose   to request verbose execution.\n");
 }
 
@@ -1923,6 +1934,7 @@ static struct option options[] = {
{ "internal-db", 0, NULL, 'I' },
{ "verbose", 0, NULL, 'V' },
{ "watch-nb", 1, NULL, 'W' },
+   { "memory-debug", 1, NULL, 'M' },
{ NULL, 0, NULL, 0 } };
 
 extern void dump_conn(struct connection *conn); 
@@ -1938,12 +1950,13 @@ int main(int argc, char *argv[])
bool outputpid = false;
bool no_domain_init = false;
const char *pidfile = NULL;
+   const char *memfile = NULL;
int timeout;
 #if defined(XEN_SYSTEMD_ENABLED)
bool systemd;
 #endif
 
-   while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
+   while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:M:", options,
  NULL)) != -1) {
switch (opt) {
case 'D':
@@ -1997,6 +2010,9 @@ int main(int argc, char *argv[])
case 'p':
priv_domid = strtol(optarg, NULL, 10);
break;
+   case 'M':
+   memfile = optarg;
+   break;
}
}
if (optind != argc)
@@ -2033,6 +2049,11 @@ int main(int argc, char *argv[])
/* Don't kill us with SIGPIPE. */
signal(SIGPIPE, SIG_IGN);
 
+   if (memfile) {
+   talloc_enable_null_tracking();
+   signal(SIGUSR1, do_talloc_report);
+   }
+
 #if defined(XEN_SYSTEMD_ENABLED)
if (!systemd)
 #endif
@@ -2079,6 +2100,17 @@ int main(int argc, char *argv[])
for (;;) {
struct connection *conn, *next;
 
+   if (trigger_talloc_report) {
+   FILE *out;
+
+   trigger_talloc_report = false;
+   out = fopen(memfile, "a");
+   if (out) {
+   talloc_report_full(NULL, out);
+   fclose(out);
+   }
+   }
+
if (poll(fds, nr_fds, timeout) < 0) {
if (errno == EINTR)
continue;
-- 
2.6.6


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

[Xen-devel] [PATCH 1/2] xen: credit2: fix two s_time_t handling issues in load balancing

[Dario Faggioli]

both introduced in d205f8a7f48e2ec ("xen: credit2: rework
load tracking logic").

First, in __update_runq_load(), the ASSERT() was actually
useless. Let's instead check that the computed value of
the load has not overflowed (and hence gone negative).

While there, do that in __update_svc_load() as well.

Second, in balance_load(), cpus_max needs being extended
in order to be correctly shifted, and the result compared
with an s_time_t value, without risking loosing info.

Signed-off-by: Dario Faggioli 
Reported-by: Andrew Cooper 
---
Cc: George Dunlap 
Cc: Anshul Makkar 
---
 xen/common/sched_credit2.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index b33ba7a..3d3e4ae 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -656,7 +656,8 @@ __update_runq_load(const struct scheduler *ops,
 rqd->load += change;
 rqd->load_last_update = now;
 
-ASSERT(rqd->avgload <= STIME_MAX && rqd->b_avgload <= STIME_MAX);
+/* Overflow, capable of making the load look negative, must not occur. */
+ASSERT(rqd->avgload > 0 && rqd->b_avgload > 0);
 
 if ( unlikely(tb_init_done) )
 {
@@ -714,6 +715,9 @@ __update_svc_load(const struct scheduler *ops,
 }
 svc->load_last_update = now;
 
+/* Overflow, capable of making the load look negative, must not occur. */
+ASSERT(svc->avgload > 0);
+
 if ( unlikely(tb_init_done) )
 {
 struct {
@@ -1742,7 +1746,7 @@ retry:
  * If we're under 100% capacaty, only shift if load difference
  * is > 1.  otherwise, shift if under 12.5%
  */
-if ( load_max < (cpus_max << prv->load_precision_shift) )
+if ( load_max < ((s_time_t)cpus_max << prv->load_precision_shift) )
 {
 if ( st.load_delta < (1ULL << (prv->load_precision_shift +
opt_underload_balance_tolerance)) )


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

[Xen-devel] [PATCH 2/2] xen: credit2: fix potential issues in csched2_cpu_pick with tracing enabled

[Dario Faggioli]

In fact, when not finding a suitable runqueue where to
place a vCPU, and hence using a fallback, we either:
 - don't issue any trace record (while we should),
 - risk underruning when accessing the runqueues
   array, while preparing the trace record.

Fix both issues and, while there, also a couple of style
problems found nearby.

Signed-off-by: Dario Faggioli 
Reported-by: Andrew Cooper 
---
Cc: George Dunlap 
Cc: Anshul Makkar 
---
 xen/common/sched_credit2.c |   13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index 3d3e4ae..7bfb24a 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -1443,7 +1443,8 @@ csched2_cpu_pick(const struct scheduler *ops, struct vcpu 
*vc)
 {
 /* We may be here because someone requested us to migrate. */
 __clear_bit(__CSFLAG_runq_migrate_request, >flags);
-return get_fallback_cpu(svc);
+new_cpu = get_fallback_cpu(svc);
+goto out;
 }
 
 /* First check to see if we're here because someone else suggested a place
@@ -1505,7 +1506,7 @@ csched2_cpu_pick(const struct scheduler *ops, struct vcpu 
*vc)
 if ( rqd_avgload < min_avgload )
 {
 min_avgload = rqd_avgload;
-min_rqi=i;
+min_rqi = i;
 }
 }
 
@@ -1520,20 +1521,20 @@ csched2_cpu_pick(const struct scheduler *ops, struct 
vcpu *vc)
 BUG_ON(new_cpu >= nr_cpu_ids);
 }
 
-out_up:
+ out_up:
 read_unlock(>lock);
-
+ out:
 if ( unlikely(tb_init_done) )
 {
 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;
 d.dom = vc->domain->domain_id;
 d.vcpu = vc->vcpu_id;
 d.rq_id = c2r(ops, new_cpu);
+d.b_avgload = prv->rqd[d.rq_id].b_avgload;
 d.new_cpu = new_cpu;
 __trace_var(TRC_CSCHED2_PICKED_CPU, 1,
 sizeof(d),


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

Re: [Xen-devel] [PATCH] xenstore: add memory allocation debugging capability

[Wei Liu]

On Fri, Jul 15, 2016 at 07:28:26AM +0200, Juergen Gross wrote:
> Add support for debugging memory allocation statistics to xenstored.
> Specifying "-M " on the command line will enable the feature.
> Whenever xenstored receives SIGUSR1 it will dump out a full talloc
> report to . This helps finding e.g. memory leaks in xenstored.
> 
> Signed-off-by: Juergen Gross 
> ---
> To be applied on top of my "xenstore: fix memory leak of xenstored"
> series. In fact this patch was used to find the problem the series
> fixed and I used it to verify the patches are working.

This patch doesn't seem to apply cleanly anymore.

The hunk rejected is:

diff a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c 
(rejected hunks)
@@ -1860,9 +1872,10 @@ int main(int argc, char *argv[])
bool outputpid = false;
bool no_domain_init = false;
const char *pidfile = NULL;
+   const char *memfile = NULL;
int timeout;
 
-   while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:",
options,
+   while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:M:",
options,
  NULL)) != -1) {
switch (opt) {
case 'D':
@@ -1942,6 +1958,11 @@ int main(int argc, char *argv[])
/* Don't kill us with SIGPIPE. */
signal(SIGPIPE, SIG_IGN);
 
+   if (memfile) {
+   talloc_enable_null_tracking();
+   signal(SIGUSR1, do_talloc_report);
+   }
+
init_sockets(, _sock);
 
init_pipe(reopen_log_pipe);


Doesn't seem to be immediately obvious to me why this is rejected.

Can you please rebase this patch and resend?

Wei.


> ---
>  tools/xenstore/xenstored_core.c | 34 +-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 8cb12c7..ab737eb 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -71,6 +71,7 @@ static int reopen_log_pipe[2];
>  static int reopen_log_pipe0_pollfd_idx = -1;
>  static char *tracefile = NULL;
>  static TDB_CONTEXT *tdb_ctx = NULL;
> +static bool trigger_talloc_report = false;
>  
>  static void corrupt(struct connection *conn, const char *fmt, ...);
>  static void check_store(void);
> @@ -1743,6 +1744,10 @@ static void init_sockets(int **psock, int **pro_sock)
>   static int minus_one = -1;
>   *psock = *pro_sock = _one;
>  }
> +
> +static void do_talloc_report(int sig)
> +{
> +}
>  #else
>  static int destroy_fd(void *_fd)
>  {
> @@ -1798,6 +1803,11 @@ static void init_sockets(int **psock, int **pro_sock)
>  
>  
>  }
> +
> +static void do_talloc_report(int sig)
> +{
> + trigger_talloc_report = true;
> +}
>  #endif
>  
>  static void usage(void)
> @@ -1823,6 +1833,7 @@ static void usage(void)
>  "  the store is corrupted (debug only),\n"
>  "  -I, --internal-db   store database in memory, not on disk\n"
>  "  -L, --preserve-localto request that /local is preserved on 
> start-up,\n"
> +"  -M, --memory-debug   support memory debugging to file,\n"
>  "  -V, --verbose   to request verbose execution.\n");
>  }
>  
> @@ -1845,6 +1856,7 @@ static struct option options[] = {
>   { "internal-db", 0, NULL, 'I' },
>   { "verbose", 0, NULL, 'V' },
>   { "watch-nb", 1, NULL, 'W' },
> + { "memory-debug", 1, NULL, 'M' },
>   { NULL, 0, NULL, 0 } };
>  
>  extern void dump_conn(struct connection *conn); 
> @@ -1860,9 +1872,10 @@ int main(int argc, char *argv[])
>   bool outputpid = false;
>   bool no_domain_init = false;
>   const char *pidfile = NULL;
> + const char *memfile = NULL;
>   int timeout;
>  
> - while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:", options,
> + while ((opt = getopt_long(argc, argv, "DE:F:HNPS:t:T:RLVW:M:", options,
> NULL)) != -1) {
>   switch (opt) {
>   case 'D':
> @@ -1916,6 +1929,9 @@ int main(int argc, char *argv[])
>   case 'p':
>   priv_domid = strtol(optarg, NULL, 10);
>   break;
> + case 'M':
> + memfile = optarg;
> + break;
>   }
>   }
>   if (optind != argc)
> @@ -1942,6 +1958,11 @@ int main(int argc, char *argv[])
>   /* Don't kill us with SIGPIPE. */
>   signal(SIGPIPE, SIG_IGN);
>  
> + if (memfile) {
> + talloc_enable_null_tracking();
> + signal(SIGUSR1, do_talloc_report);
> + }
> +
>   init_sockets(, _sock);
>  
>   init_pipe(reopen_log_pipe);
> @@ -1978,6 +1999,17 @@ int main(int argc, char *argv[])
>   for (;;) {
>   struct connection *conn, *next;
>  
> + if (trigger_talloc_report) {
> + FILE *out;
> +
> + trigger_talloc_report = false;
> +  

Re: [Xen-devel] [PATCH v4] altp2m: Allow the hostp2m entries to be of type p2m_ram_shared

[George Dunlap]

On 18/07/16 18:06, Tamas K Lengyel wrote:
>>> Anyhow, at this point I'm
>>> going to start carrying out-of-tree patches for Xen in my project and
>>> just resign from my mem_sharing maintainership as I feel like it's
>>> pretty pointless.
>>
>> I'm sorry that you're discouraged; all I can say is that I hope you
>> reconsider.  I'm not trying to block you, and I'm not ignoring your use
>> case; it's the job of a maintainer to look at *everyone's* use cases and
>> try to make sure that they are all accommodated in so far as it is
>> possible.
>>
>> I'm also trying to make sure that the code you end up using in your
>> project is robust and reliable.  It seems to me like if the current
>> implementation was fixed, your life would be a lot easier than if we
>> accept your patch as it is -- your sharing code could just worry about
>> sharing, your altp2m code could just worry about whatever it's trying to
>> do, without having to carefully avoid corner cases or manually fix
>> things up when corner cases happen.  A bit less sharing would happen,
>> because fewer pages would be eligible to be shared, but overall you'd
>> have a lot less bugs and headache.
>>
>> I invested a lot of my very limited time carefully going through both
>> sets of code before I answered your e-mail, and I spent a lot of time
>> trying to explain the kinds of interactions I think will be a problem.
>> I could have just acked the patch without doing that; but I think that
>> would have been both less good for you, and less good for the project as
>> a whole.
> 
> I certainly appreciate your time spent on this. However, I don't see
> the point of being maintainer if my opinion on what constitutes an
> improvement of the system just gets overruled.

You're not being overruled; you're just being asked to make a case for a
change you want to make to an area of code that I maintain (the p2m
code).  And the discussion is by no means over; I started the most
recent discussion by saying "Correct me if I'm wrong", and it looks like
there are still a number of places where we have different views of the
facts of the matter.  Once we've established those we may end up with
closer opinions.

Working together means that sometimes you have to spend the time and
effort to understand where other people are coming from -- what they
think is important, what they think is true; and then working with that
-- correcting them on places where they have misconceptions (or
double-checking your own beliefs to make sure that you're not mistaken);
communicating what it is that you think is important, and then trying to
come up with a way forward that takes everyone's values into account, or
convincing someone that a particular way really is the best way forward
(which may mean convincing them to change their priorities somewhat).

I am sorry that the tone of this conversation has heated up.  But the
reason I've been "raising my voice" as it were is because I've been
trying to ask questions and raise potential issues, and I feel like
you've been just hand-waving them away.  You may be 100% right, but it
is my duty as the maintainer of the p2m code to not accept code until
I'm reasonably convinced that it's a good way forward.

> I would like to hear the
> other maintainers opinion on this matter as well but I'm not
> interested in arguing endlessly or initiating or vote, so if the patch
> is not allowed in I will accept that decision but I will see no point
> in continuing as maintainer of the system.

At a basic level, the other maintainers will agree that I shouldn't
accept code unless I am convinced it's for the good of the project.  But
since this is a technical issue, before anyone would express an opinion
to ask me to change my mind, they would want a more complete view of the
facts of the matter -- facts which you and I are still in the process of
sorting out.

> As pretty much my
> project is the only use-case where these two systems would be used
> together at this point, and since I already require my users to
> compile Xen from source it is just easier to go this route then what
> you suggest and exploring and remedying all possible ways the two
> systems could be misused when setup in ways they were not intended. If
> these were considered stable features and not experimental I would
> agree, but that's just not the case. So I think both of our time is
> better spent doing other things then arguing. 

So a lot of points here.

You have no idea what other projects are doing.  Lots of people take the
Xen code, do something with it internally, and and we never hear from
them.  Or maybe they're in a start-up in stealth mode and will announce
themselves in due course.  If you step down from being a maintainer and
stop engaging with the community you'll be in the same position.

There's a very obvious other use case which I've been talking about from
the beginning: A host administrator / cloud provider / user wants to
both 1) use page sharing to improve 

[Xen-devel] [PATCH v3 0/5] xenstore: fix memory leak of xenstored

[Juergen Gross]

xenstored has a memory leak when setting watches: a no longer active
watch which fired in the past will still use some memory. This is
critical for long running connections to xenstored like the qemu
process serving as a qdisk backend for dom0. It will use some few
kB in xenstored for each domain create/destroy pair.

Fix this leak by using a temporary memory context for all allocations
in xenstored when firing a watch event.

Changes in V3:
- renamed temporary context parameter name and added comments in 
  patches 2-5 as requested by Wei Liu

Changes in V2:
- modified patch description as requested by Ian Jackson
- split up patch 2 as requested by Ian Jackson

Juergen Gross (5):
  xenstore: call each xenstored command function with temporary context
  xenstore: add explicit memory context parameter to get_parent()
  xenstore: add explicit memory context parameter to read_node()
  xenstore: add explicit memory context parameter to get_node()
  xenstore: use temporary memory context for firing watches

 tools/xenstore/xenstored_core.c| 134 -
 tools/xenstore/xenstored_core.h|   4 +
 tools/xenstore/xenstored_domain.c  |  20 +++--
 tools/xenstore/xenstored_domain.h  |  10 +--
 tools/xenstore/xenstored_transaction.c |   5 +-
 tools/xenstore/xenstored_transaction.h |   2 +-
 tools/xenstore/xenstored_watch.c   |  22 --
 tools/xenstore/xenstored_watch.h   |   3 +-
 8 files changed, 123 insertions(+), 77 deletions(-)

-- 
2.6.6


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

[Xen-devel] [PATCH v3 5/5] xenstore: use temporary memory context for firing watches

[Juergen Gross]

Use a temporary memory context for memory allocations when firing
watches. This will avoid leaking memory in case of long living
connections and/or xenstore entries.

This requires adding a new parameter to fire_watches() and add_event()
to specify the memory context to use for allocations.

Signed-off-by: Juergen Gross 
Reviewed-by: Wei Liu 
Acked-by: Ian Jackson 
---
 tools/xenstore/xenstored_core.c|  8 
 tools/xenstore/xenstored_domain.c  |  6 +++---
 tools/xenstore/xenstored_transaction.c |  2 +-
 tools/xenstore/xenstored_watch.c   | 22 --
 tools/xenstore/xenstored_watch.h   |  3 ++-
 5 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 4239410..1232169 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -961,7 +961,7 @@ static void do_write(struct connection *conn, struct 
buffered_data *in)
}
 
add_change_node(conn->transaction, name, false);
-   fire_watches(conn, name, false);
+   fire_watches(conn, in, name, false);
send_ack(conn, XS_WRITE);
 }
 
@@ -986,7 +986,7 @@ static void do_mkdir(struct connection *conn, struct 
buffered_data *in)
return;
}
add_change_node(conn->transaction, name, false);
-   fire_watches(conn, name, false);
+   fire_watches(conn, in, name, false);
}
send_ack(conn, XS_MKDIR);
 }
@@ -1112,7 +1112,7 @@ static void do_rm(struct connection *conn, struct 
buffered_data *in)
 
if (_rm(conn, node, name)) {
add_change_node(conn->transaction, name, true);
-   fire_watches(conn, name, true);
+   fire_watches(conn, in, name, true);
send_ack(conn, XS_RM);
}
 }
@@ -1188,7 +1188,7 @@ static void do_set_perms(struct connection *conn, struct 
buffered_data *in)
}
 
add_change_node(conn->transaction, name, false);
-   fire_watches(conn, name, false);
+   fire_watches(conn, in, name, false);
send_ack(conn, XS_SET_PERMS);
 }
 
diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
index c66539a..5de93d4 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -204,7 +204,7 @@ static int destroy_domain(void *_domain)
unmap_interface(domain->interface);
}
 
-   fire_watches(NULL, "@releaseDomain", false);
+   fire_watches(NULL, domain, "@releaseDomain", false);
 
return 0;
 }
@@ -232,7 +232,7 @@ static void domain_cleanup(void)
}
 
if (notify)
-   fire_watches(NULL, "@releaseDomain", false);
+   fire_watches(NULL, NULL, "@releaseDomain", false);
 }
 
 /* We scan all domains rather than use the information given here. */
@@ -389,7 +389,7 @@ void do_introduce(struct connection *conn, struct 
buffered_data *in)
/* Now domain belongs to its connection. */
talloc_steal(domain->conn, domain);
 
-   fire_watches(NULL, "@introduceDomain", false);
+   fire_watches(NULL, in, "@introduceDomain", false);
} else if ((domain->mfn == mfn) && (domain->conn != conn)) {
/* Use XS_INTRODUCE for recreating the xenbus event-channel. */
if (domain->port)
diff --git a/tools/xenstore/xenstored_transaction.c 
b/tools/xenstore/xenstored_transaction.c
index 3cde26e..34720fa 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -227,7 +227,7 @@ void do_transaction_end(struct connection *conn, struct 
buffered_data *in)
 
/* Fire off the watches for everything that changed. */
list_for_each_entry(i, >changes, list)
-   fire_watches(conn, i->node, i->recurse);
+   fire_watches(conn, in, i->node, i->recurse);
generation++;
}
send_ack(conn, XS_TRANSACTION_END);
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index beefd6c..856750e 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -47,7 +47,12 @@ struct watch
char *node;
 };
 
+/*
+ * Send a watch event.
+ * Temporary memory allocations are done with ctx.
+ */
 static void add_event(struct connection *conn,
+ void *ctx,
  struct watch *watch,
  const char *name)
 {
@@ -57,7 +62,7 @@ static void add_event(struct connection *conn,
 
if (!check_event_node(name)) {
/* Can this conn load node, or see that it doesn't exist? */
-   struct node *node = get_node(conn, name, name, XS_PERM_READ);
+   struct node *node = get_node(conn, ctx, name, XS_PERM_READ);
 

[Xen-devel] [PATCH v3 3/5] xenstore: add explicit memory context parameter to read_node()

[Juergen Gross]

Add a parameter to xenstored read_node() function to explicitly
specify the memory context to be used for allocations. This will make
it easier to avoid memory leaks by using a context which is freed
soon.

When calling read_node() select a sensible memory context for the new
parameter by preferring a temporary one.

Signed-off-by: Juergen Gross 
Reviewed-by: Wei Liu 
Acked-by: Ian Jackson 
---
 tools/xenstore/xenstored_core.c | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index f2c12ab..c462115 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -397,8 +397,12 @@ bool is_child(const char *child, const char *parent)
return child[len] == '/' || child[len] == '\0';
 }
 
-/* If it fails, returns NULL and sets errno. */
-static struct node *read_node(struct connection *conn, const char *name)
+/*
+ * If it fails, returns NULL and sets errno.
+ * Temporary memory allocations will be done with ctx.
+ */
+static struct node *read_node(struct connection *conn, const void *ctx,
+ const char *name)
 {
TDB_DATA key, data;
uint32_t *p;
@@ -419,7 +423,7 @@ static struct node *read_node(struct connection *conn, 
const char *name)
return NULL;
}
 
-   node = talloc(name, struct node);
+   node = talloc(ctx, struct node);
node->name = talloc_strdup(node, name);
node->parent = NULL;
node->tdb = tdb_context(conn);
@@ -526,7 +530,7 @@ static enum xs_perm_type ask_parents(struct connection 
*conn, const char *name)
 
do {
name = get_parent(name, name);
-   node = read_node(conn, name);
+   node = read_node(conn, name, name);
if (node)
break;
} while (!streq(name, "/"));
@@ -567,7 +571,7 @@ struct node *get_node(struct connection *conn,
errno = EINVAL;
return NULL;
}
-   node = read_node(conn, name);
+   node = read_node(conn, name, name);
/* If we don't have permission, we don't have node. */
if (node) {
if ((perm_for_conn(conn, node->perms, node->num_perms) & perm)
@@ -823,7 +827,7 @@ static struct node *construct_node(struct connection *conn, 
const char *name)
char *children, *parentname = get_parent(name, name);
 
/* If parent doesn't exist, create it. */
-   parent = read_node(conn, parentname);
+   parent = read_node(conn, parentname, parentname);
if (!parent)
parent = construct_node(conn, parentname);
if (!parent)
@@ -988,7 +992,7 @@ static void delete_node(struct connection *conn, struct 
node *node)
for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) {
struct node *child;
 
-   child = read_node(conn, 
+   child = read_node(conn, node,
  talloc_asprintf(node, "%s/%s", node->name,
  node->children + i));
if (child) {
@@ -1040,7 +1044,7 @@ static int _rm(struct connection *conn, struct node 
*node, const char *name)
/* Delete from parent first, then if we crash, the worst that can
   happen is the child will continue to take up space, but will
   otherwise be unreachable. */
-   struct node *parent = read_node(conn, get_parent(name, name));
+   struct node *parent = read_node(conn, name, get_parent(name, name));
if (!parent) {
send_error(conn, EINVAL);
return 0;
@@ -1059,7 +1063,7 @@ static int _rm(struct connection *conn, struct node 
*node, const char *name)
 static void internal_rm(const char *name)
 {
char *tname = talloc_strdup(NULL, name);
-   struct node *node = read_node(NULL, tname);
+   struct node *node = read_node(NULL, tname, tname);
if (node)
_rm(NULL, node, tname);
talloc_free(node);
@@ -1077,7 +1081,7 @@ static void do_rm(struct connection *conn, struct 
buffered_data *in)
if (!node) {
/* Didn't exist already?  Fine, if parent exists. */
if (errno == ENOENT) {
-   node = read_node(conn, get_parent(in, name));
+   node = read_node(conn, in, get_parent(in, name));
if (node) {
send_ack(conn, XS_RM);
return;
@@ -1608,7 +1612,7 @@ static void remember_string(struct hashtable *hash, const 
char *str)
  */
 static void check_store_(const char *name, struct hashtable *reachable)
 {
-   struct node *node = read_node(NULL, name);
+   struct node *node = read_node(NULL, name, name);
 
if (node) {

[Xen-devel] [PATCH v3 2/5] xenstore: add explicit memory context parameter to get_parent()

[Juergen Gross]

Add a parameter to xenstored get_parent() function to explicitly
specify the memory context to be used for allocations. This will make
it easier to avoid memory leaks by using a context which is freed
soon.

When available use a temporary context when calling get_parent(),
otherwise mimic the old behavior by calling get_parent() with the same
argument for both parameters.

Signed-off-by: Juergen Gross 
Reviewed-by: Wei Liu 
Acked-by: Ian Jackson 
---
 tools/xenstore/xenstored_core.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 94c809c..f2c12ab 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -507,12 +507,16 @@ static enum xs_perm_type perm_for_conn(struct connection 
*conn,
return perms[0].perms & mask;
 }
 
-static char *get_parent(const char *node)
+/*
+ * Get name of node parent.
+ * Temporary memory allocations are done with ctx.
+ */
+static char *get_parent(const void *ctx, const char *node)
 {
char *slash = strrchr(node + 1, '/');
if (!slash)
-   return talloc_strdup(node, "/");
-   return talloc_asprintf(node, "%.*s", (int)(slash - node), node);
+   return talloc_strdup(ctx, "/");
+   return talloc_asprintf(ctx, "%.*s", (int)(slash - node), node);
 }
 
 /* What do parents say? */
@@ -521,7 +525,7 @@ static enum xs_perm_type ask_parents(struct connection 
*conn, const char *name)
struct node *node;
 
do {
-   name = get_parent(name);
+   name = get_parent(name, name);
node = read_node(conn, name);
if (node)
break;
@@ -816,7 +820,7 @@ static struct node *construct_node(struct connection *conn, 
const char *name)
const char *base;
unsigned int baselen;
struct node *parent, *node;
-   char *children, *parentname = get_parent(name);
+   char *children, *parentname = get_parent(name, name);
 
/* If parent doesn't exist, create it. */
parent = read_node(conn, parentname);
@@ -1036,7 +1040,7 @@ static int _rm(struct connection *conn, struct node 
*node, const char *name)
/* Delete from parent first, then if we crash, the worst that can
   happen is the child will continue to take up space, but will
   otherwise be unreachable. */
-   struct node *parent = read_node(conn, get_parent(name));
+   struct node *parent = read_node(conn, get_parent(name, name));
if (!parent) {
send_error(conn, EINVAL);
return 0;
@@ -1073,7 +1077,7 @@ static void do_rm(struct connection *conn, struct 
buffered_data *in)
if (!node) {
/* Didn't exist already?  Fine, if parent exists. */
if (errno == ENOENT) {
-   node = read_node(conn, get_parent(name));
+   node = read_node(conn, get_parent(in, name));
if (node) {
send_ack(conn, XS_RM);
return;
-- 
2.6.6


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

[Xen-devel] [PATCH v3 4/5] xenstore: add explicit memory context parameter to get_node()

[Juergen Gross]

Add a parameter to xenstored get_node() function to explicitly
specify the memory context to be used for allocations. This will make
it easier to avoid memory leaks by using a context which is freed
soon.

This requires adding the temporary context to errno_from_parents() and
ask_parents(), too.

When calling get_node() select a sensible memory context for the new
parameter by preferring a temporary one.

Signed-off-by: Juergen Gross 
Reviewed-by: Wei Liu 
Acked-by: Ian Jackson 
---
 tools/xenstore/xenstored_core.c  | 50 +---
 tools/xenstore/xenstored_core.h  |  1 +
 tools/xenstore/xenstored_watch.c |  2 +-
 3 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c462115..4239410 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -523,14 +523,18 @@ static char *get_parent(const void *ctx, const char *node)
return talloc_asprintf(ctx, "%.*s", (int)(slash - node), node);
 }
 
-/* What do parents say? */
-static enum xs_perm_type ask_parents(struct connection *conn, const char *name)
+/*
+ * What do parents say?
+ * Temporary memory allocations are done with ctx.
+ */
+static enum xs_perm_type ask_parents(struct connection *conn, const void *ctx,
+const char *name)
 {
struct node *node;
 
do {
-   name = get_parent(name, name);
-   node = read_node(conn, name, name);
+   name = get_parent(ctx, name);
+   node = read_node(conn, ctx, name);
if (node)
break;
} while (!streq(name, "/"));
@@ -544,24 +548,32 @@ static enum xs_perm_type ask_parents(struct connection 
*conn, const char *name)
return perm_for_conn(conn, node->perms, node->num_perms);
 }
 
-/* We have a weird permissions system.  You can allow someone into a
+/*
+ * We have a weird permissions system.  You can allow someone into a
  * specific node without allowing it in the parents.  If it's going to
  * fail, however, we don't want the errno to indicate any information
- * about the node. */
-static int errno_from_parents(struct connection *conn, const char *node,
- int errnum, enum xs_perm_type perm)
+ * about the node.
+ * Temporary memory allocations are done with ctx.
+ */
+static int errno_from_parents(struct connection *conn, const void *ctx,
+ const char *node, int errnum,
+ enum xs_perm_type perm)
 {
/* We always tell them about memory failures. */
if (errnum == ENOMEM)
return errnum;
 
-   if (ask_parents(conn, node) & perm)
+   if (ask_parents(conn, ctx, node) & perm)
return errnum;
return EACCES;
 }
 
-/* If it fails, returns NULL and sets errno. */
+/*
+ * If it fails, returns NULL and sets errno.
+ * Temporary memory allocations are done with ctx.
+ */
 struct node *get_node(struct connection *conn,
+ const void *ctx,
  const char *name,
  enum xs_perm_type perm)
 {
@@ -571,7 +583,7 @@ struct node *get_node(struct connection *conn,
errno = EINVAL;
return NULL;
}
-   node = read_node(conn, name, name);
+   node = read_node(conn, ctx, name);
/* If we don't have permission, we don't have node. */
if (node) {
if ((perm_for_conn(conn, node->perms, node->num_perms) & perm)
@@ -582,7 +594,7 @@ struct node *get_node(struct connection *conn,
}
/* Clean up errno if they weren't supposed to know. */
if (!node) 
-   errno = errno_from_parents(conn, name, errno, perm);
+   errno = errno_from_parents(conn, ctx, name, errno, perm);
return node;
 }
 
@@ -775,7 +787,7 @@ static void send_directory(struct connection *conn, struct 
buffered_data *in)
const char *name = onearg(in);
 
name = canonicalize(conn, name);
-   node = get_node(conn, name, XS_PERM_READ);
+   node = get_node(conn, in, name, XS_PERM_READ);
if (!node) {
send_error(conn, errno);
return;
@@ -790,7 +802,7 @@ static void do_read(struct connection *conn, struct 
buffered_data *in)
const char *name = onearg(in);
 
name = canonicalize(conn, name);
-   node = get_node(conn, name, XS_PERM_READ);
+   node = get_node(conn, in, name, XS_PERM_READ);
if (!node) {
send_error(conn, errno);
return;
@@ -927,7 +939,7 @@ static void do_write(struct connection *conn, struct 
buffered_data *in)
datalen = in->used - offset;
 
name = canonicalize(conn, vec[0]);
-   node = get_node(conn, name, XS_PERM_WRITE);
+   node = get_node(conn, in, name, 

Re: [Xen-devel] [PATCH v3 2/2] qdisk - hw/block/xen_disk: grant copy implementation

[Paulina Szubarczyk]

On 07/15/2016 06:55 PM, Anthony PERARD wrote:

On Wed, Jun 22, 2016 at 10:38:53AM +0200, Paulina Szubarczyk wrote:

Copy data operated on during request from/to local buffers to/from
the grant references.

Before grant copy operation local buffers must be allocated what is
done by calling ioreq_init_copy_buffers. For the 'read' operation,
first, the qemu device invokes the read operation on local buffers
and on the completion grant copy is called and buffers are freed.
For the 'write' operation grant copy is performed before invoking
write by qemu device.

A new value 'feature_grant_copy' is added to recognize when the
grant copy operation is supported by a guest.
The body of the function 'ioreq_runio_qemu_aio' is moved to
'ioreq_runio_qemu_aio_blk' and in the 'ioreq_runio_qemu_aio' depending
on the support for grant copy according checks, initialization, grant
operation are made, then the 'ioreq_runio_qemu_aio_blk' function is
called.

Signed-off-by: Paulina Szubarczyk 



diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 37e14d1..4eca06a 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -500,6 +503,99 @@ static int ioreq_map(struct ioreq *ioreq)
  return 0;
  }

+static void* get_buffer(int count)
+{
+return xc_memalign(xen_xc, XC_PAGE_SIZE, count*XC_PAGE_SIZE);


Instead of xc_memalign, I think you need to call qemu_memalign() here.
Have a look at the file HACKING, the section '3. Low level memory
management'. Also, you probably do not need an the extra function
get_buffer() and can call qemu_memalign() directly in
ioreq_init_copy_buffers().



Ok, I will changed that.


+}
+
+static void free_buffers(struct ioreq *ioreq)
+{
+int i;
+
+for (i = 0; i < ioreq->v.niov; i++) {
+ioreq->page[i] = NULL;
+}
+
+free(ioreq->pages);


With the use of qemu_memalign, this would need to be qemu_vfree().


+}
+
+static int ioreq_init_copy_buffers(struct ioreq *ioreq) {
+int i;
+
+if (ioreq->v.niov == 0) {
+return 0;
+}
+
+ioreq->pages = get_buffer(ioreq->v.niov);
+if (!ioreq->pages) {
+return -1;
+}
+
+for (i = 0; i < ioreq->v.niov; i++) {
+ioreq->page[i] = ioreq->pages + i*XC_PAGE_SIZE;
+ioreq->v.iov[i].iov_base += (uintptr_t)ioreq->page[i];


Is the += intended here?



I was suggested by ioreq_map assignment to the ioreq->v.iov[i].iov_base 
which is made that way. But I do not think that makes sense to sum up 
the pointers. I will change it to =.



+}
+
+return 0;
+}
+
+static int ioreq_copy(struct ioreq *ioreq)
+{
+XenGnttab gnt = ioreq->blkdev->xendev.gnttabdev;
+xengnttab_grant_copy_segment_t segs[BLKIF_MAX_SEGMENTS_PER_REQUEST];
+int i, count = 0, r, rc;
+int64_t file_blk = ioreq->blkdev->file_blk;
+
+if (ioreq->v.niov == 0) {
+return 0;
+}
+
+count = ioreq->v.niov;
+
+for (i = 0; i < count; i++) {
+
+if (ioreq->req.operation == BLKIF_OP_READ) {
+segs[i].flags = GNTCOPY_dest_gref;
+segs[i].dest.foreign.ref = ioreq->refs[i];
+segs[i].dest.foreign.domid = ioreq->domids[i];
+segs[i].dest.foreign.offset = ioreq->req.seg[i].first_sect * 
file_blk;
+segs[i].source.virt = ioreq->v.iov[i].iov_base;
+} else {
+segs[i].flags = GNTCOPY_source_gref;
+segs[i].source.foreign.ref = ioreq->refs[i];
+segs[i].source.foreign.domid = ioreq->domids[i];
+segs[i].source.foreign.offset = ioreq->req.seg[i].first_sect * 
file_blk;
+segs[i].dest.virt = ioreq->v.iov[i].iov_base;
+}
+segs[i].len = (ioreq->req.seg[i].last_sect
+   - ioreq->req.seg[i].first_sect + 1) * file_blk;
+
+}
+
+rc = xengnttab_grant_copy(gnt, count, segs);
+
+if (rc) {
+xen_be_printf(>blkdev->xendev, 0,
+  "failed to copy data %d \n", rc);
+ioreq->aio_errors++;
+return -1;
+} else {
+r = 0;
+}
+
+for (i = 0; i < count; i++) {
+if (segs[i].status != GNTST_okay) {
+xen_be_printf(>blkdev->xendev, 3,
+  "failed to copy data %d for gref %d, domid %d\n", rc,
+  ioreq->refs[i], ioreq->domids[i]);
+ioreq->aio_errors++;
+r = -1;
+}
+}
+
+return r;
+}
+
  static int ioreq_runio_qemu_aio(struct ioreq *ioreq);

  static void qemu_aio_complete(void *opaque, int ret)
@@ -528,8 +624,31 @@ static void qemu_aio_complete(void *opaque, int ret)
  return;
  }

+if (ioreq->blkdev->feature_grant_copy) {
+switch (ioreq->req.operation) {
+case BLKIF_OP_READ:
+/* in case of failure ioreq->aio_errors is increased */
+ioreq_copy(ioreq);
+free_buffers(ioreq);
+break;
+case BLKIF_OP_WRITE:
+case BLKIF_OP_FLUSH_DISKCACHE:
+if 

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

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH v2 03/17] libxl/arm: Add a 
configuration option for ARM DomU ACPI"):
...
> > >>> I know but here we want to unify the acpi option for x86 and ARM while
> > >>> on x86 it's true by default. What I want to ask is that how to
> > >>> distinguish x86 and ARM in libxl__domain_build_info_setdefault(), so we
> > >>> can assign acpi with different default value for x86 and ARM.
> > >>
> > >> By using #ifdef in the code?

We normally try to deal with this kind of thing by separating the
arch-specific code into separate files, which are compiled as needed.

Maybe libxl__arch_domain_prepare_config is the right place ?

> > > Maybe this could not work since CONFIG_ARM can not be accessed in libxl
> > > in current codes. I'm not sure why it can't work. Wei, do you have any
> > > suggestion?
> > > 
> > And is it ok to use
> > #if defined(__arm__) || defined(__aarch64__)
> > ?
> 
> I am not a Libxl maintainer anymore, but I think that should be OK or at
> least it would be a step in the right direction.

I definitely don't want open-coded alternations like this.  If an
#ifdef is needed, a single feature macro should be (if necessary
invented) and tested.

But as I say I think this can probably be done with libxl_arch.h,
libxl_arm.c, libxl_x86.c, etc.

Thanks,
Ian.

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

Re: [Xen-devel] [PATCH v2 5/5] xenstore: use temporary memory context for firing watches

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2 5/5] xenstore: use temporary memory context for 
firing watches"):
> On Mon, Jul 18, 2016 at 09:31:29AM +0200, Juergen Gross wrote:
> >  static void add_event(struct connection *conn,
> > + void *tmp,
> 
> tmp -> ctx or context.

Once again,

Acked-by: Ian Jackson 

Thanks for your work.  I guess you will make the changes Wei asked
for, and you should then retain my acks.

Regards,
Ian.

___
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, Jul 18, 2016 at 12:40:43PM -0700, Stefano Stabellini wrote:
[...]
> > #if defined(__arm__) || defined(__aarch64__)
> > ?
> 
> I am not a Libxl maintainer anymore, but I think that should be OK or at
> least it would be a step in the right direction.

Yes, I think that's the correct ifdefs to use.

Wei.

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

Re: [Xen-devel] [PATCH v2 4/5] xenstore: add explicit memory context parameter to get_node()

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2 4/5] xenstore: add explicit memory context 
parameter to get_node()"):
> On Mon, Jul 18, 2016 at 09:31:28AM +0200, Juergen Gross wrote:
> > Add a parameter to xenstored get_node() function to explicitly
> > specify the memory context to be used for allocations. This will make
> > it easier to avoid memory leaks by using a context which is freed
> > soon.
...
> mem -> ctx or context here and other places.

Indeed, but, as before, regardless:

Acked-by: Ian Jackson 

___
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 Fri, Jul 15, 2016 at 05:39:32PM +0800, Shannon Zhao wrote:
[...]
> > 
> > It would be trivial to have another option in xl.cfg to allow MB
> > granularity. But I don't think that's a good idea. Asking for more
> > memory when you don't really know how much is enough is not very useful.
> > If an admin can know how much is needed, surely the library can be
> > taught to obtain that knowledge, too.
> > 
> > We need to decide which model we should go with. And, if we decide to
> > diverge, document the difference between x86 and ARM model.
> > 
> Hi Wei,
> 
> Do you decide how to add the size of ACPI blob to max_memkb?
> 

AFAICT ARM and x86 maintainers hold different opinions on how memory
should be accounted.

I would like to have a unified memory accounting model. But if we can't
have that at the moment, I'm fine with divergence, but please document
it somewhere (comment near code snippet, in header, or a file under docs
etc). And the amount added to max_memkb needs to be properly calculated,
not some magic number, so that we have a chance in the future to
confidently change how we do thing.


Wei.

> Thanks,
> -- 
> Shannon
> 

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

Re: [Xen-devel] [PATCH v2 3/5] xenstore: add explicit memory context parameter to read_node()

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2 3/5] xenstore: add explicit memory context 
parameter to read_node()"):
> On Mon, Jul 18, 2016 at 09:31:27AM +0200, Juergen Gross wrote:
> >  /* If it fails, returns NULL and sets errno. */
> > -static struct node *read_node(struct connection *conn, const char *name)
> > +static struct node *read_node(struct connection *conn, const void *mem,
> > + const char *name)
> 
> Same here: mem -> ctx or context.

Again, I agree, but, regardless:

Acked-by: Ian Jackson 

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

[Xen-devel] [PATCH v2 2/5] xenstore: add explicit memory context parameter to get_parent()

[Ian Jackson]

Juergen Gross writes ("[PATCH v2 2/5] xenstore: add explicit memory context 
parameter to get_parent()"):
> Add a parameter to xenstored get_parent() function to explicitly
> specify the memory context to be used for allocations. This will make
> it easier to avoid memory leaks by using a context which is freed
> soon.
> 
> When available use a temporary context when calling get_parent(),
> otherwise mimic the old behavior by calling get_parent() with the same
> argument for both parameters.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH v2 2/5] xenstore: add explicit memory context parameter to get_parent()

[Ian Jackson]

Wei Liu writes ("Re: [PATCH v2 2/5] xenstore: add explicit memory context 
parameter to get_parent()"):
> I would name mem ctx or context instead. And maybe document this
> function a bit saying that memory allocation is done with the first
> parameter.
> 
> With those cosmetic issues fixed:
> 
> Reviewed-by: Wei Liu 

These would be good improvements.

Ian.

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

[Xen-devel] [PATCH v2 1/5] xenstore: call each xenstored command function with temporary context

[Ian Jackson]

Juergen Gross writes ("[PATCH v2 1/5] xenstore: call each xenstored command 
function with temporary context"):
> In order to be able to avoid leaving temporary memory allocated after
> processing of a command in xenstored call all command functions with
> the temporary "in" context. Each function can then make use of that
> temporary context for allocating temporary memory instead of either
> leaving that memory allocated until the connection is dropped (or
> even until end of xenstored) or freeing the memory itself.
> 
> This requires to modify the interfaces of the functions taking only
> one argument from the connection by moving the call of onearg() into
> the single functions. Other than that no functional change.

Thanks for splitting this out.

Acked-by: Ian Jackson 

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

Re: [Xen-devel] [PATCH v2 5/5] xenstore: use temporary memory context for firing watches

[Wei Liu]

On Mon, Jul 18, 2016 at 09:31:29AM +0200, Juergen Gross wrote:
>  static void add_event(struct connection *conn,
> +   void *tmp,

tmp -> ctx or context.

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 11/11] xen: credit2: implement true SMT support

[Dario Faggioli]

On Tue, 2016-07-19 at 11:05 +0100, George Dunlap wrote:
> On 19/07/16 10:57, Dario Faggioli wrote:
> > 
> > > What about folding in something like the attached patch?
> > > 
> > I'd be totally fine with this.
> Do you mean you ack me folding in that particular patch (so that the
> resulting commit looks like the attached)?
> 
Yes, I do.

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 2/2] qdisk - hw/block/xen_disk: grant copy implementation

[Paulina Szubarczyk]

On 07/15/2016 07:11 PM, Anthony PERARD wrote:

On Fri, Jul 15, 2016 at 12:15:45PM +0100, Wei Liu wrote:

On Fri, Jul 15, 2016 at 12:28:48PM +0200, Paulina Szubarczyk wrote:



On 07/14/2016 12:37 PM, Wei Liu wrote:

On Wed, Jun 22, 2016 at 10:38:53AM +0200, Paulina Szubarczyk wrote:

diff --git a/configure b/configure
index e41876a..355d3fa 100755
--- a/configure
+++ b/configure
@@ -1843,7 +1843,7 @@ fi
  # xen probe

  if test "$xen" != "no" ; then
-  xen_libs="-lxenstore -lxenctrl -lxenguest"
+  xen_libs="-lxenstore -lxenctrl -lxenguest -lxengnttab"



First thing, -lxengnttab should be in xen_stable_libs.


Do I understand correctly that I should add a new variable
"xen_stable_libs"? I could not find it in the qemu tree used anywhere else.



Hmm... there is already one in upstream QEMU -- which means you're
perhaps using qemu-xen tree.

I think all new development should happen on upstream qemu, not in our
qemu-xen tree.


The probing needs to be more sophisticated.

You need to probe the new function your added as well. Just a few lines
below xen_stable_libs there is a section for hand-coded probing source
code, which you would need to modify.

Assuming your gnttab change will be merged into 4.8 (the release under
development at the moment), you need to have a separate program for it.


I will add that.


After you've done proper probing, you will know which version of Xen
this qemu is compiling against.  And then, there should be some fallback
mechanism to compile and run this qemu with older version of xen. This
is not too hard because you can guard your code with feature flag or
ifdef (please consult Stefan and Anthony which method to use).

Feel free to ask questions. I will try my best to explain.



+blkdev->feature_grant_copy =
+(xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0);


This is a bit problematic. As this patch stands, it won't compile on
older version of Xen because there is no such function there.


There is a variable CONFIG_XEN_CTRL_INTERFACE_VERSION holding current
version of the Xen control library this qemu is configured with. It is set
from the configure file. The feature could be guarded with ifdef by a new
variable CONFIG_XEN_LIBS_INTERFACE_VERSION or they could be unified to
CONFIG_XEN_TOOLS_INTERFACE_VERSION to not fill the same value twice.



Another way is to provide a stub for this function to always return 0.

Please wait for Stefano and Anthony to see which method they prefer.


I think using CONFIG_XEN_CTRL_INTERFACE_VERSION is fine. With maybe a
stub of xengnttab_grant_copy() in xen_common.h.

I will add the stub but the structure xengnttab_grant_copy_segment need 
to be repeated in the xen_common.h again, it is also not defined in the 
earlier versions.


Paulina

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

Re: [Xen-devel] [PATCH v3 2/2] qdisk - hw/block/xen_disk: grant copy implementation

[Paulina Szubarczyk]

On 07/19/2016 11:12 AM, Roger Pau Monné wrote:

On Wed, Jun 22, 2016 at 10:38:53AM +0200, Paulina Szubarczyk wrote:

Copy data operated on during request from/to local buffers to/from
the grant references.

Before grant copy operation local buffers must be allocated what is
done by calling ioreq_init_copy_buffers. For the 'read' operation,
first, the qemu device invokes the read operation on local buffers
and on the completion grant copy is called and buffers are freed.
For the 'write' operation grant copy is performed before invoking
write by qemu device.

A new value 'feature_grant_copy' is added to recognize when the
grant copy operation is supported by a guest.
The body of the function 'ioreq_runio_qemu_aio' is moved to
'ioreq_runio_qemu_aio_blk' and in the 'ioreq_runio_qemu_aio' depending
on the support for grant copy according checks, initialization, grant
operation are made, then the 'ioreq_runio_qemu_aio_blk' function is
called.

Signed-off-by: Paulina Szubarczyk 
---
Changes since v2:
- to use the xengnttab_* function directly added -lxengnttab to configure
   and include  in include/hw/xen/xen_common.h
- in ioreq_copy removed an out path, changed a log level, made explicit
   assignement to 'xengnttab_copy_grant_segment'
* I did not change the way of testing if grant_copy operation is implemented.
   As far as I understand if the code from gnttab_unimp.c is used then the 
gnttab
   device is unavailable and the handler to gntdev would be invalid. But
   if the handler is valid then the ioctl should return operation unimplemented
   if the gntdev does not implement the operation.

  configure   |   2 +-
  hw/block/xen_disk.c | 171 
  include/hw/xen/xen_common.h |   2 +
  3 files changed, 162 insertions(+), 13 deletions(-)


[...]


@@ -1020,10 +1160,17 @@ static int blk_connect(struct XenDevice *xendev)

  xen_be_bind_evtchn(>xendev);

+blkdev->feature_grant_copy =
+(xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0);


Isn't this going to trigger an abort on OSes that don't implement
xengnttab_grant_copy? AFAICT the 'unimplemented' handler in libgnttab for
this is just an abort.


So is the xengnttab_map_grant_refs and the pointer to 
blkdev->xendev.gnttabdev would be invalid so the sring would not be 
initialized a few lines earlier in that function leading to the fail of 
the initialization. In case the gntdev does not implement the ioctl then 
only an error code will be returned.


Paulina


Roger.



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

Re: [Xen-devel] [PATCH v2 11/11] xen: credit2: implement true SMT support

[George Dunlap]

On 19/07/16 10:57, Dario Faggioli wrote:
> On Tue, 2016-07-19 at 10:39 +0100, George Dunlap wrote:
>> On Mon, Jul 18, 2016 at 6:24 PM, Dario Faggioli
>>  wrote:
>>>  
>>> If you're saying that this discrepancy between rqd->idle's and
>>> rqd->smt_idle's semantic is, at minimum, unideal, I do agree... but
>>> I
>>> think, for now at least, it's worth living with it.
>> I hadn't actually said anything, but you know me well enough to guess
>> what I'm thinking. :-)  
>>
> Hehe. :-)
> 
>> I am somewhat torn between feeling like the
>> inconsistency and as you say, the fact that this is a distinct
>> improvement and it would seem a bit petty to insist that you either
>> wait or produce a patch to change idle at the same time.
>>
> If we go ahead, I sign up for double checking and, if possible, fixing
> the inconsistency.
> 
>> But I do think that the difference needs to be called out a bit
>> better.  
>>
> Yes, I was about to re-replying saying "perhaps we should add a comment
> about this".
> 
>> What about folding in something like the attached patch?
>>
> I'd be totally fine with this.

Do you mean you ack me folding in that particular patch (so that the
resulting commit looks like the attached)?

 -George

commit 9dfa4b90867dedf4b1db0523a76c7007cbb9bd40
Author: George Dunlap 
Commit: George Dunlap 

xen: credit2: implement true SMT support

In fact, right now, we recommend keepeing runqueues
arranged per-core, so that it is the inter-runqueue load
balancing code that automatically spreads the work in an
SMT friendly way. This means that any other runq
arrangement one may want to use falls short of SMT
scheduling optimizations.

This commit implements SMT awareness --similar to the
one we have in Credit1-- for any possible runq
arrangement. This turned out to be pretty easy to do,
as the logic can live entirely in runq_tickle()
(although, in order to avoid for_each_cpu loops in
that function, we use a new cpumask which indeed needs
to be updated in other places).

In addition to disentangling SMT awareness from load
balancing, this also allows us to support the
sched_smt_power_savings parametar in Credit2 as well.

Signed-off-by: Dario Faggioli 
Signed-off-by: George Dunlap 
Reviewed-by: Anshul Makkar 

diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index b33ba7a..3e1720c 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -353,8 +353,9 @@ struct csched2_runqueue_data {
 struct list_head svc;  /* List of all vcpus assigned to this runqueue */
 unsigned int max_weight;
 
-cpumask_t idle,/* Currently idle */
-tickled;   /* Another cpu in the queue is already targeted for this one */
+cpumask_t idle,/* Currently idle pcpus */
+smt_idle,  /* Fully idle-and-untickled cores (see below) */
+tickled;   /* Have been asked to go through schedule */
 int load;  /* Instantaneous load: Length of queue  + num non-idle threads */
 s_time_t load_last_update;  /* Last time average was updated */
 s_time_t avgload;   /* Decaying queue load */
@@ -415,6 +416,79 @@ struct csched2_dom {
 };
 
 /*
+ * Hyperthreading (SMT) support.
+ *
+ * We use a special per-runq mask (smt_idle) and update it according to the
+ * following logic:
+ *  - when _all_ the SMT sibling in a core are idle, all their corresponding
+ *bits are set in the smt_idle mask;
+ *  - when even _just_one_ of the SMT siblings in a core is not idle, all the
+ *bits correspondings to it and to all its siblings are clear in the
+ *smt_idle mask.
+ *
+ * Once we have such a mask, it is easy to implement a policy that, either:
+ *  - uses fully idle cores first: it is enough to try to schedule the vcpus
+ *on pcpus from smt_idle mask first. This is what happens if
+ *sched_smt_power_savings was not set at boot (default), and it maximizes
+ *true parallelism, and hence performance;
+ *  - uses already busy cores first: it is enough to try to schedule the vcpus
+ *on pcpus that are idle, but are not in smt_idle. This is what happens if
+ *sched_smt_power_savings is set at boot, and it allows as more cores as
+ *possible to stay in low power states, minimizing power consumption.
+ *
+ * This logic is entirely implemented in runq_tickle(), and that is enough.
+ * In fact, in this scheduler, placement of a vcpu on one of the pcpus of a
+ * runq, _always_ happens by means of tickling:
+ *  - when a vcpu wakes up, it calls csched2_vcpu_wake(), which calls
+ *runq_tickle();
+ *  - when a migration is initiated in schedule.c, we call csched2_cpu_pick(),
+ *csched2_vcpu_migrate() (which calls migrate()) and csched2_vcpu_wake().
+ 

Re: [Xen-devel] [PATCH v2 4/5] xenstore: add explicit memory context parameter to get_node()

[Wei Liu]

On Mon, Jul 18, 2016 at 09:31:28AM +0200, Juergen Gross wrote:
> Add a parameter to xenstored get_node() function to explicitly
> specify the memory context to be used for allocations. This will make
> it easier to avoid memory leaks by using a context which is freed
> soon.
> 
> This requires adding the temporary context to errno_from_parents() and
> ask_parents(), too.
> 
> When calling get_node() select a sensible memory context for the new
> parameter by preferring a temporary one.
> 
> Signed-off-by: Juergen Gross 
> ---
>  tools/xenstore/xenstored_core.c  | 33 ++---
>  tools/xenstore/xenstored_core.h  |  1 +
>  tools/xenstore/xenstored_watch.c |  2 +-
>  3 files changed, 20 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index e5c74f4..095ba00 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -517,13 +517,14 @@ static char *get_parent(const void *mem, const char 
> *node)
>  }
>  
>  /* What do parents say? */
> -static enum xs_perm_type ask_parents(struct connection *conn, const char 
> *name)
> +static enum xs_perm_type ask_parents(struct connection *conn, const void 
> *mem,
> +  const char *name)

mem -> ctx or context here and other places.

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 2/5] xenstore: add explicit memory context parameter to get_parent()

[Wei Liu]

On Mon, Jul 18, 2016 at 09:31:26AM +0200, Juergen Gross wrote:
> Add a parameter to xenstored get_parent() function to explicitly
> specify the memory context to be used for allocations. This will make
> it easier to avoid memory leaks by using a context which is freed
> soon.
> 
> When available use a temporary context when calling get_parent(),
> otherwise mimic the old behavior by calling get_parent() with the same
> argument for both parameters.
> 
> Signed-off-by: Juergen Gross 
> ---
>  tools/xenstore/xenstored_core.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 94c809c..9448ee8 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -507,12 +507,12 @@ static enum xs_perm_type perm_for_conn(struct 
> connection *conn,
>   return perms[0].perms & mask;
>  }
>  
> -static char *get_parent(const char *node)
> +static char *get_parent(const void *mem, const char *node)

I would name mem ctx or context instead. And maybe document this
function a bit saying that memory allocation is done with the first
parameter.

With those cosmetic issues fixed:

Reviewed-by: Wei Liu 

>  {
>   char *slash = strrchr(node + 1, '/');
>   if (!slash)
> - return talloc_strdup(node, "/");
> - return talloc_asprintf(node, "%.*s", (int)(slash - node), node);
> + return talloc_strdup(mem, "/");
> + return talloc_asprintf(mem, "%.*s", (int)(slash - node), node);
>  }
>  
>  /* What do parents say? */
> @@ -521,7 +521,7 @@ static enum xs_perm_type ask_parents(struct connection 
> *conn, const char *name)
>   struct node *node;
>  
>   do {
> - name = get_parent(name);
> + name = get_parent(name, name);
>   node = read_node(conn, name);
>   if (node)
>   break;
> @@ -816,7 +816,7 @@ static struct node *construct_node(struct connection 
> *conn, const char *name)
>   const char *base;
>   unsigned int baselen;
>   struct node *parent, *node;
> - char *children, *parentname = get_parent(name);
> + char *children, *parentname = get_parent(name, name);
>  
>   /* If parent doesn't exist, create it. */
>   parent = read_node(conn, parentname);
> @@ -1036,7 +1036,7 @@ static int _rm(struct connection *conn, struct node 
> *node, const char *name)
>   /* Delete from parent first, then if we crash, the worst that can
>  happen is the child will continue to take up space, but will
>  otherwise be unreachable. */
> - struct node *parent = read_node(conn, get_parent(name));
> + struct node *parent = read_node(conn, get_parent(name, name));
>   if (!parent) {
>   send_error(conn, EINVAL);
>   return 0;
> @@ -1073,7 +1073,7 @@ static void do_rm(struct connection *conn, struct 
> buffered_data *in)
>   if (!node) {
>   /* Didn't exist already?  Fine, if parent exists. */
>   if (errno == ENOENT) {
> - node = read_node(conn, get_parent(name));
> + node = read_node(conn, get_parent(in, name));
>   if (node) {
>   send_ack(conn, XS_RM);
>   return;
> -- 
> 2.6.6
> 

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

Re: [Xen-devel] [PATCH v2 3/5] xenstore: add explicit memory context parameter to read_node()

[Wei Liu]

On Mon, Jul 18, 2016 at 09:31:27AM +0200, Juergen Gross wrote:
> Add a parameter to xenstored read_node() function to explicitly
> specify the memory context to be used for allocations. This will make
> it easier to avoid memory leaks by using a context which is freed
> soon.
> 
> When calling read_node() select a sensible memory context for the new
> parameter by preferring a temporary one.
> 
> Signed-off-by: Juergen Gross 
> ---
>  tools/xenstore/xenstored_core.c | 24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
> index 9448ee8..e5c74f4 100644
> --- a/tools/xenstore/xenstored_core.c
> +++ b/tools/xenstore/xenstored_core.c
> @@ -398,7 +398,8 @@ bool is_child(const char *child, const char *parent)
>  }
>  
>  /* If it fails, returns NULL and sets errno. */
> -static struct node *read_node(struct connection *conn, const char *name)
> +static struct node *read_node(struct connection *conn, const void *mem,
> +   const char *name)

Same here: mem -> ctx or context.

Reviewed-by: Wei Liu 

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

Re: [Xen-devel] [PATCH v2 1/5] xenstore: call each xenstored command function with temporary context

[Wei Liu]

On Mon, Jul 18, 2016 at 09:31:25AM +0200, Juergen Gross wrote:
> In order to be able to avoid leaving temporary memory allocated after
> processing of a command in xenstored call all command functions with
> the temporary "in" context. Each function can then make use of that
> temporary context for allocating temporary memory instead of either
> leaving that memory allocated until the connection is dropped (or
> even until end of xenstored) or freeing the memory itself.
> 
> This requires to modify the interfaces of the functions taking only
> one argument from the connection by moving the call of onearg() into
> the single functions. Other than that no functional change.
> 
> Signed-off-by: Juergen Gross 

Reviewed-by: Wei Liu 

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

[Xen-devel] [qemu-mainline bisection] complete test-amd64-amd64-libvirt-xsm

[osstest service owner]

branch xen-unstable
xenbranch xen-unstable
job test-amd64-amd64-libvirt-xsm
testid guest-start

Tree: libvirt git://xenbits.xen.org/libvirt.git
Tree: libvirt_gnulib git://git.sv.gnu.org/gnulib.git
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://git.qemu.org/qemu.git
Tree: xen git://xenbits.xen.org/xen.git

*** Found and reproduced problem changeset ***

  Bug is in tree:  qemuu git://git.qemu.org/qemu.git
  Bug introduced:  095497ffc66b7f031ff2a17f1e50f5cb105ce588
  Bug not present: 5a693efda84d7df5136cc2bd31c959bb1530b0c9
  Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/97649/


  commit 095497ffc66b7f031ff2a17f1e50f5cb105ce588
  Author: Peter Lieven 
  Date:   Thu Jun 30 12:00:46 2016 +0200
  
  vnc-enc-tight: use thread local storage for palette
  
  currently the color counting palette is allocated from heap, used and 
destroyed
  for each single subrect. Use a static palette per thread for this purpose 
and
  avoid the malloc and free for each update.
  
  Signed-off-by: Peter Lieven 
  Reviewed-by: Paolo Bonzini 
  Message-id: 1467280846-9674-1-git-send-email...@kamp.de
  Signed-off-by: Gerd Hoffmann 


For bisection revision-tuple graph see:
   
http://logs.test-lab.xenproject.org/osstest/results/bisect/qemu-mainline/test-amd64-amd64-libvirt-xsm.guest-start.html
Revision IDs in each graph node refer, respectively, to the Trees above.


Running cs-bisection-step 
--graph-out=/home/logs/results/bisect/qemu-mainline/test-amd64-amd64-libvirt-xsm.guest-start
 --summary-out=tmp/97649.bisection-summary --basis-template=96791 
--blessings=real,real-bisect qemu-mainline test-amd64-amd64-libvirt-xsm 
guest-start
Searching for failure / basis pass:
 97567 fail [host=fiano0] / 96791 [host=italia1] 96776 [host=nocera0] 96765 
[host=godello1] 96732 [host=huxelrebe0] 96703 [host=godello0] 96683 
[host=fiano1] 96652 [host=baroque1] 96618 [host=pinot1] 96580 
[host=chardonnay0] 96557 [host=huxelrebe1] 96527 [host=elbling0] 96513 
[host=italia0] 96502 [host=baroque0] 96480 [host=chardonnay1] 96447 
[host=elbling1] 96367 [host=merlot1] 96347 ok.
Failure / basis pass flights: 97567 / 96347
(tree with no url: minios)
(tree with no url: ovmf)
(tree with no url: seabios)
Tree: libvirt git://xenbits.xen.org/libvirt.git
Tree: libvirt_gnulib git://git.sv.gnu.org/gnulib.git
Tree: linux git://xenbits.xen.org/linux-pvops.git
Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git
Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git
Tree: qemuu git://git.qemu.org/qemu.git
Tree: xen git://xenbits.xen.org/xen.git
Latest fe8bad38f58f8b60518947441fb3be8d89d51c58 
68b6adebef05670a312fb92b05e7bd089d2ed43a 
44dd5e6b1cf505485d839bd62d47e29a36232d67 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
6e20809727261599e8527c456eb078c0e89139a1 
6b92bbfe812746fe7841a24c24e6460f5359ce72 
b48be35ac86cd6369124cf06ca3006d086095297
Basis pass 0b4645a7e061abc8a4be71fe89865cf248ce6e56 
246b3b28808ee5f4664be674dce573af9497fc7a 
44dd5e6b1cf505485d839bd62d47e29a36232d67 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
6e20809727261599e8527c456eb078c0e89139a1 
7dd929dfdc5c52ce79b21bf557ff506e89acbf63 
8384dc2d95538c5910d98db3df3ff5448bf0af48
Generating revisions with ./adhoc-revtuple-generator  
git://xenbits.xen.org/libvirt.git#0b4645a7e061abc8a4be71fe89865cf248ce6e56-fe8bad38f58f8b60518947441fb3be8d89d51c58
 
git://git.sv.gnu.org/gnulib.git#246b3b28808ee5f4664be674dce573af9497fc7a-68b6adebef05670a312fb92b05e7bd089d2ed43a
 
git://xenbits.xen.org/linux-pvops.git#44dd5e6b1cf505485d839bd62d47e29a36232d67-44dd5e6b1cf505485d839bd62d47e29a36232d67
 
git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860
 
git://xenbits.xen.org/qemu-xen-traditional.git#6e20809727261599e8527c456eb078c0e89139a1-6e20809727261599e8527c456eb078c0e89139a1
 
git://git.qemu.org/qemu.git#7dd929dfdc5c52ce79b21bf557ff506e89acbf63-6b92bbfe812746fe7841a24c24e6460f5359ce72
 
git://xenbits.xen.org/xen.git#8384dc2d95538c5910d98db3df3ff5448bf0af48-b48be35ac86cd6369124cf06ca3006d086095297
From git://cache:9419/git://git.qemu.org/qemu
   a098fbc..08b558f  master -> origin/master
Loaded 21464 nodes in revision graph
Searching for test results:
 96347 pass 0b4645a7e061abc8a4be71fe89865cf248ce6e56 
246b3b28808ee5f4664be674dce573af9497fc7a 
44dd5e6b1cf505485d839bd62d47e29a36232d67 
c530a75c1e6a472b0eb9558310b518f0dfcd8860 
6e20809727261599e8527c456eb078c0e89139a1 
7dd929dfdc5c52ce79b21bf557ff506e89acbf63 
8384dc2d95538c5910d98db3df3ff5448bf0af48
 96367 [host=merlot1]
 96447 [host=elbling1]
 96480 [host=chardonnay1]
 96502 [host=baroque0]
 96557 [host=huxelrebe1]
 96513 [host=italia0]
 96527 [host=elbling0]
 

Re: [Xen-devel] [PATCH v2 11/11] xen: credit2: implement true SMT support

[Dario Faggioli]

On Tue, 2016-07-19 at 10:39 +0100, George Dunlap wrote:
> On Mon, Jul 18, 2016 at 6:24 PM, Dario Faggioli
>  wrote:
> > 
> > If you're saying that this discrepancy between rqd->idle's and
> > rqd->smt_idle's semantic is, at minimum, unideal, I do agree... but
> > I
> > think, for now at least, it's worth living with it.
> I hadn't actually said anything, but you know me well enough to guess
> what I'm thinking. :-)  
>
Hehe. :-)

> I am somewhat torn between feeling like the
> inconsistency and as you say, the fact that this is a distinct
> improvement and it would seem a bit petty to insist that you either
> wait or produce a patch to change idle at the same time.
> 
If we go ahead, I sign up for double checking and, if possible, fixing
the inconsistency.

> But I do think that the difference needs to be called out a bit
> better.  
>
Yes, I was about to re-replying saying "perhaps we should add a comment
about this".

> What about folding in something like the attached patch?
> 
I'd be totally fine with this.

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 03/16] x86/monitor: mechanical renames

[Corneliu ZUZU]

On 7/18/2016 9:07 PM, Andrew Cooper wrote:

On 15/07/16 08:18, Corneliu ZUZU wrote:

On 7/12/2016 9:10 AM, Corneliu ZUZU wrote:

On 7/11/2016 7:43 PM, Tamas K Lengyel wrote:

On Sat, Jul 9, 2016 at 12:46 PM, Corneliu ZUZU
 wrote:

On 7/9/2016 9:10 PM, Tamas K Lengyel wrote:

On Fri, Jul 8, 2016 at 10:13 PM, Corneliu ZUZU

wrote:

Arch-specific vm-event functions in x86/vm_event.h -e.g.
vm_event_init_domain()-
don't have an 'arch_' prefix. Apply the same rule for monitor
functions -
originally the only two monitor functions that had an 'arch_'
prefix were
arch_monitor_domctl_event() and arch_monitor_domctl_op(), but I
gave them
that
prefix because -they had a counterpart function in common code-,
that
being
monitor_domctl().

This should actually be the other way around - ie adding the arch_
prefix to vm_event functions that lack it.

Given that the majority of the arch-specific functions called from
common-code don't have an 'arch_' prefix unless they have a common
counterpart, I was guessing that was the rule. It made sense in my
head
since I saw in that the intention of avoiding naming conflicts (i.e
you
can't have monitor_domctl() both on the common-side and on the
arch-side, so
prepend 'arch_' to the latter). I noticed you guys also 'skip' the
prefix
when sending patches, so that reinforced my assumption.


Having the arch_ prefix is
helpful to know that the function is dealing with the arch specific
structs and not common.

Personally I don't see much use in 'knowing that the function is
dealing
with the arch structs' from the call-site and you can tell that
from the
implementation-site just by looking at the path of its source file.
Also,
the code is pretty much localized in the arch directory anyway so
usually
one wouldn't have to go back and forth between common and arch that
often.
What really bothers me though is always having to read 'arch_' when
spelling
a function-name and also that it makes the function name longer
without much
benefit. Your suggestion of adding it to pretty much all functions
that make
up the interface to common just adds to that headache. :-D


Similarly that's why we have the hvm_ prefix
for functions in hvm/monitor.

'hvm_'  doesn't seem to me more special than 'monitor_', for
instance, but
maybe that's just me.


Let this also be the rule for future 'arch_' functions additions,
and
with this
patch remove the 'arch_' prefix from the monitor functions that
don't
have a
counterpart in common-code (all but those 2 aforementioned).

Even if there are no common counter-parts to the function, the arch_
prefix should remain, so I won't be able to ack this patch.

Tamas

Having said the above, are you still of the same opinion?

Yes, I am. While it's not a hard rule to always apply these prefix, it
does make sense to have them so I don't see benefit in removing the
existing prefixes.

Well, for one the benefit would be not confusing developers by
creating inconsistencies: what's the rule here, i.e. why isn't a
function such as alloc_domain_struct prefixed w/ 'arch_' but
arch_domain_create is? The reason seems to be the latter having a
common counterpart while the former not, at least that's what I see
being done all over the code-base. Also, I've done this before and
you seemed to agree:
https://www.mail-archive.com/xen-devel%40lists.xen.org/msg57617.html
(Q1). You also suggested creating arch-specific functions without the
prefix:
https://www.mail-archive.com/xen-devel%40lists.xen.org/msg57336.html
. Why the sudden change of heart?

2ndly and obviously, removing the prefixes would make function names
shorter and clearer (e.g. -read- "arch_vm_event_vcpu_unpause" and
then read "vm_event_vcpu_unpause").

3rd reason is that adding the prefix to -all- arch-specific functions
called from common would mean having a lot new functions with the
prefix. I'd read the prefix over and over again and at some point I'd
get annoyed and say "ok, ok, it's arch_, I get it; why use this
prefix so much again?".

4th reason is that the advantage of telling that the function
accesses arch structures is much too little considering that idk,

50% of the codebase is arch-specific, so it doesn't provide much

information, this categorization is too broad to deserve a special
prefix. Whereas using the prefix only for functions that do have a
common counterpart gives you the extra information that the
'operation' is only -partly- arch-specific, i.e. to see the whole
picture, look @ the common-side implementation. Keep in mind that
we'd also be -losing that information- if we were to apply the 'go
with arch_ for all' rule.. (this could be a 5th reason)


Adding arch_ prefix to the ones that don't already
have one is optional, I was just pointing out that if you really feel
like standardizing the naming convention, that's where I would like
things to move towards to.

Tamas

I don't think I'd say this patch "standardizes the naming convention"
but rather "fixes 

Re: [Xen-devel] [PATCH] docs/misc/hvmlite: Sync up hvm_start_info data structure

[Anthony PERARD]

On Mon, Jul 18, 2016 at 06:49:33PM +0100, Andrew Cooper wrote:
> On 18/07/16 17:15, Anthony PERARD wrote:
> > It as been modified by:
> > 3c8d890 x86/PVHv2: update the start info structure layout
> > 247d38c xen: change the sizes of memory fields in the HVM start info to be 
> > 64bits
> >
> > Signed-off-by: Anthony PERARD 
> 
> Now that we have (or are just about to get) the start info in the public
> API/ABI, it would be better to refer to its canonical location, than to
> try to keep multiple copies up to date.

I guess I can add a patch to my hvmloader patch series.

-- 
Anthony PERARD

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

Re: [Xen-devel] [PATCH v3 2/2] qdisk - hw/block/xen_disk: grant copy implementation

[Roger Pau Monné]

On Wed, Jun 22, 2016 at 10:38:53AM +0200, Paulina Szubarczyk wrote:
> Copy data operated on during request from/to local buffers to/from
> the grant references.
> 
> Before grant copy operation local buffers must be allocated what is
> done by calling ioreq_init_copy_buffers. For the 'read' operation,
> first, the qemu device invokes the read operation on local buffers
> and on the completion grant copy is called and buffers are freed.
> For the 'write' operation grant copy is performed before invoking
> write by qemu device.
> 
> A new value 'feature_grant_copy' is added to recognize when the
> grant copy operation is supported by a guest.
> The body of the function 'ioreq_runio_qemu_aio' is moved to
> 'ioreq_runio_qemu_aio_blk' and in the 'ioreq_runio_qemu_aio' depending
> on the support for grant copy according checks, initialization, grant
> operation are made, then the 'ioreq_runio_qemu_aio_blk' function is
> called.
> 
> Signed-off-by: Paulina Szubarczyk 
> ---
> Changes since v2:
> - to use the xengnttab_* function directly added -lxengnttab to configure
>   and include  in include/hw/xen/xen_common.h
> - in ioreq_copy removed an out path, changed a log level, made explicit 
>   assignement to 'xengnttab_copy_grant_segment'
> * I did not change the way of testing if grant_copy operation is implemented.
>   As far as I understand if the code from gnttab_unimp.c is used then the 
> gnttab 
>   device is unavailable and the handler to gntdev would be invalid. But 
>   if the handler is valid then the ioctl should return operation 
> unimplemented 
>   if the gntdev does not implement the operation.
> 
>  configure   |   2 +-
>  hw/block/xen_disk.c | 171 
> 
>  include/hw/xen/xen_common.h |   2 +
>  3 files changed, 162 insertions(+), 13 deletions(-)

[...]
 
> @@ -1020,10 +1160,17 @@ static int blk_connect(struct XenDevice *xendev)
>  
>  xen_be_bind_evtchn(>xendev);
>  
> +blkdev->feature_grant_copy =
> +(xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 
> 0);

Isn't this going to trigger an abort on OSes that don't implement 
xengnttab_grant_copy? AFAICT the 'unimplemented' handler in libgnttab for 
this is just an abort.

Roger.

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

Re: [Xen-devel] [PATCH v1 10/20] acpi/hvmloader: Replace mem_alloc() and virt_to_phys() with memory ops

[Jan Beulich]

>>> Boris Ostrovsky  07/08/16 6:20 PM >>>
>On 07/08/2016 11:35 AM, Jan Beulich wrote:
> On 08.07.16 at 17:23,  wrote:
>>> Is it up to the builder to decide which tables are important and which
>>> are not?
>> I'm afraid that's not so easy to tell. If for example we can't fit the
>> HPET table, the guest could be run without HPET unless a HPET
>> was specifically requested (rather than just defaulted to).
>
>But again --- how will the caller know that it was only HPET table that
>was not built?

Why would the caller care? I guess examples could be found where it is
necessary for the caller to know, but for the specific example (and at least
some others) failure is of no interest to the caller - it's only the guest which
is affected.

Jan




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

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

[osstest service owner]

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

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 fc3f005aee72aa5c3e1cf825381f8c3755b02101
baseline version:
 ovmf dc99315b8732b6e3032d01319d3f534d440b43d0

Last test of basis94748  2016-05-24 22:43:25 Z   55 days
Failing since 94750  2016-05-25 03:43:08 Z   55 days  118 attempts
Testing same since97622  2016-07-18 21:01:53 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 
  Hegde, Nagaraj P 
  hegdenag 
  Heyi Guo 
  Jan D?bro? 
  Jan Dabros 
  Jeff Fan 
  Jeremy Linton 
  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 
  Mudusuru, Giri P 
  Ni, Ruiyu 
  Qiu Shumin 
  Ruiyu Ni 
  Ruiyu Ni　
  Ryan Harkin 
  Sami Mujawar 
  Satya Yarlagadda 
  Shannon Zhao 
  Sriram Subramanian 
  Star Zeng 
  Subramanian, Sriram (EG Servers Platform SW) 
  Sunny Wang 
  Tapan Shah 
  Thomas Palmer 
  Yarlagadda, Satya P 
  Yonghong Zhu 
  Zhang Lubo 
  Zhang, Chao B 
  Zhang, Lubo 

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 10828 lines long.)

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

[Xen-devel] [PATCH] XSM-docs: Flask operates on domain types and not on individual domain. Updated the documentation to reflect this.

[Anshul Makkar]

Signed-off-by: Anshul Makkar 
---
 * Resending the patch due to incomplete subject in the previous patch.

 docs/misc/xsm-flask.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)
---
diff --git a/docs/misc/xsm-flask.txt b/docs/misc/xsm-flask.txt
index 62f15dd..bf8bb6e 100644
--- a/docs/misc/xsm-flask.txt
+++ b/docs/misc/xsm-flask.txt
@@ -9,8 +9,8 @@ controls over Xen domains, allowing the policy writer to define 
what
 interactions between domains, devices, and the hypervisor are permitted.
 
 Some examples of what FLASK can do:
- - Prevent two domains from communicating via event channels or grants
- - Control which domains can use device passthrough (and which devices)
+ - Prevent two domains types from communicating via event channels or grants
+ - Control which type of domains can use device passthrough (and which devices)
  - Restrict or audit operations performed by privileged domains
  - Prevent a privileged domain from arbitrarily mapping pages from other 
domains
 
@@ -160,10 +160,10 @@ the policy can be reloaded using "xl loadpolicy".
 The example policy included with Xen demonstrates most of the features of FLASK
 that can be used without dom0 disaggregation. The main types for domUs are:
 
- - domU_t is a domain that can communicate with any other domU_t
+ - domU_t is a domain type that can communicate with any other domU_t types.
  - isolated_domU_t can only communicate with dom0
  - prot_domU_t is a domain type whose creation can be disabled with a boolean
- - nomigrate_t is a domain that must be created via the nomigrate_t_building
+ - nomigrate_t is a domain type that must be created via the 
nomigrate_t_building
type, and whose memory cannot be read by dom0 once created
 
 HVM domains with stubdomain device models also need a type for the stub domain.
-- 
1.9.1


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

Re: [Xen-devel] [PATCH v2] xl: add option to leave domain paused after migration

[Wei Liu]

On Tue, Jul 19, 2016 at 10:58:15AM +0200, Roger Pau Monne wrote:
> This is useful for debugging domains that crash on resume from migration.
> 
> Signed-off-by: Roger Pau Monné 

Acked-by: Wei Liu 

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

[Xen-devel] [PATCH v2] xl: add option to leave domain paused after migration

[Roger Pau Monne]

This is useful for debugging domains that crash on resume from migration.

Signed-off-by: Roger Pau Monné 
---
Cc: ian.jack...@eu.citrix.com
Cc: wei.l...@citrix.com
---
Changes since v1:
 - Document the newly added option in the xl man page.
---
 docs/man/xl.pod.1 |  4 
 tools/libxl/xl_cmdimpl.c  | 29 +++--
 tools/libxl/xl_cmdtable.c |  3 ++-
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index f4dc32c..f3a2bcb 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -443,6 +443,10 @@ Send  instead of config file from creation.
 
 Print huge (!) amount of debug during the migration process.
 
+=item B<-p>
+
+Leave the domain on the receive side paused after migration.
+
 =back
 
 =item B [I] I I
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d8530f0..fd80442 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4742,7 +4742,7 @@ static void migrate_domain(uint32_t domid, const char 
*rune, int debug,
 exit(EXIT_FAILURE);
 }
 
-static void migrate_receive(int debug, int daemonize, int monitor,
+static void migrate_receive(int debug, int daemonize, int monitor, int pause,
 int send_fd, int recv_fd,
 libxl_checkpointed_stream checkpointed,
 char *colo_proxy_script)
@@ -4850,8 +4850,10 @@ static void migrate_receive(int debug, int daemonize, 
int monitor,
 if (rc) goto perhaps_destroy_notify_rc;
 }
 
-rc = libxl_domain_unpause(ctx, domid);
-if (rc) goto perhaps_destroy_notify_rc;
+if (!pause) {
+rc = libxl_domain_unpause(ctx, domid);
+if (rc) goto perhaps_destroy_notify_rc;
+}
 
 fprintf(stderr, "migration target: Domain started successsfully.\n");
 rc = 0;
@@ -4965,7 +4967,7 @@ int main_restore(int argc, char **argv)
 
 int main_migrate_receive(int argc, char **argv)
 {
-int debug = 0, daemonize = 1, monitor = 1;
+int debug = 0, daemonize = 1, monitor = 1, pause = 0;
 libxl_checkpointed_stream checkpointed = LIBXL_CHECKPOINTED_STREAM_NONE;
 int opt;
 char *script = NULL;
@@ -4976,7 +4978,7 @@ int main_migrate_receive(int argc, char **argv)
 COMMON_LONG_OPTS
 };
 
-SWITCH_FOREACH_OPT(opt, "Fedr", opts, "migrate-receive", 0) {
+SWITCH_FOREACH_OPT(opt, "Fedrp", opts, "migrate-receive", 0) {
 case 'F':
 daemonize = 0;
 break;
@@ -4996,13 +4998,16 @@ int main_migrate_receive(int argc, char **argv)
 case 0x200:
 script = optarg;
 break;
+case 'p':
+pause = 1;
+break;
 }
 
 if (argc-optind != 0) {
 help("migrate-receive");
 return EXIT_FAILURE;
 }
-migrate_receive(debug, daemonize, monitor,
+migrate_receive(debug, daemonize, monitor, pause,
 STDOUT_FILENO, STDIN_FILENO,
 checkpointed, script);
 
@@ -5048,14 +5053,14 @@ int main_migrate(int argc, char **argv)
 const char *ssh_command = "ssh";
 char *rune = NULL;
 char *host;
-int opt, daemonize = 1, monitor = 1, debug = 0;
+int opt, daemonize = 1, monitor = 1, debug = 0, pause = 0;
 static struct option opts[] = {
 {"debug", 0, 0, 0x100},
 {"live", 0, 0, 0x200},
 COMMON_LONG_OPTS
 };
 
-SWITCH_FOREACH_OPT(opt, "FC:s:e", opts, "migrate", 2) {
+SWITCH_FOREACH_OPT(opt, "FC:s:ep", opts, "migrate", 2) {
 case 'C':
 config_filename = optarg;
 break;
@@ -5069,6 +5074,9 @@ int main_migrate(int argc, char **argv)
 daemonize = 0;
 monitor = 0;
 break;
+case 'p':
+pause = 1;
+break;
 case 0x100: /* --debug */
 debug = 1;
 break;
@@ -5096,12 +5104,13 @@ int main_migrate(int argc, char **argv)
 } else {
 verbose_len = (minmsglevel_default - minmsglevel) + 2;
 }
-xasprintf(, "exec %s %s xl%s%.*s migrate-receive%s%s",
+xasprintf(, "exec %s %s xl%s%.*s migrate-receive%s%s%s",
   ssh_command, host,
   pass_tty_arg ? " -t" : "",
   verbose_len, verbose_buf,
   daemonize ? "" : " -e",
-  debug ? " -d" : "");
+  debug ? " -d" : "",
+  pause ? " -p" : "");
 }
 
 migrate_domain(domid, rune, debug, config_filename);
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
index bf69ffb..85c1e0f 100644
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -164,7 +164,8 @@ struct cmd_spec cmd_table[] = {
   "migrate-receive [-d -e]\n"
   "-e  Do not wait in the background (on ) for the 
death\n"
   "of the domain.\n"
-  "--debug Print huge (!) amount of debug during the migration 
process."
+  "--debug Print huge (!) 

Re: [Xen-devel] Is: Revert c5ad33184354260be6d05de57e46a5498692f6d6 "mm/swap.c: flush lru pvecs on compound page arrival" from stable tree? Was:[osstest-ad...@xenproject.org: [linux-4.1 bisection] com

[Sebastian Gottschall]

No such Message-ID known.



Am 19.07.2016 um 10:32 schrieb Michal Hocko:

[CCing Sasha]

On Mon 18-07-16 11:30:46, Konrad Rzeszutek Wilk wrote:

Hey Lukasz,

We found that your patch in the automated Xen test-case ends up
OOMing the box when trying to install guests. This worked prior
to your patch.

See serial log:
http://logs.test-lab.xenproject.org/osstest/logs/97597/test-amd64-i386-qemut-rhel6hvm-amd/serial-pinot0.log

Would it be OK to revert this patch from the stable trees?

The fix up is trivial so I believe it would be better to apply the
follow up fix
http://lkml.kernel.org/r/20160714175521.3675e...@gandalf.local.home



--
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Berliner Ring 101, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

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

[Xen-devel] [PATCH 5/9] mini-os: add nr_free_pages counter

[Juergen Gross]

Add a variable holding the number of available memory pages. This will
aid auto-ballooning later.

Signed-off-by: Juergen Gross 
---
 include/mm.h | 1 +
 mm.c | 6 ++
 2 files changed, 7 insertions(+)

diff --git a/include/mm.h b/include/mm.h
index a48f485..b97b43e 100644
--- a/include/mm.h
+++ b/include/mm.h
@@ -42,6 +42,7 @@
 #define STACK_SIZE_PAGE_ORDER __STACK_SIZE_PAGE_ORDER
 #define STACK_SIZE __STACK_SIZE
 
+extern unsigned long nr_free_pages;
 
 void init_mm(void);
 unsigned long alloc_pages(int order);
diff --git a/mm.c b/mm.c
index 0dd4862..263a356 100644
--- a/mm.c
+++ b/mm.c
@@ -53,6 +53,8 @@ static unsigned long *alloc_bitmap;
 #define allocated_in_map(_pn) \
 (alloc_bitmap[(_pn)/PAGES_PER_MAPWORD] & (1UL<<((_pn)&(PAGES_PER_MAPWORD-1
 
+unsigned long nr_free_pages;
+
 /*
  * Hint regarding bitwise arithmetic in map_{alloc,free}:
  *  -(1<= n. 
@@ -81,6 +83,8 @@ static void map_alloc(unsigned long first_page, unsigned long 
nr_pages)
 while ( ++curr_idx < end_idx ) alloc_bitmap[curr_idx] = ~0UL;
 alloc_bitmap[curr_idx] |= (1UL<