From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas Lengyel Subject: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 23 Jun 2014 18:31:40 +0200 Message-ID: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3412067784055527810==" Return-path: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "xen-devel@lists.xen.org" , Aravindh Puthiyaparambil , Ian Jackson List-Id: xen-devel@lists.xenproject.org --===============3412067784055527810== Content-Type: multipart/alternative; boundary=001a11370a34c05eba04fc8360f7 --001a11370a34c05eba04fc8360f7 Content-Type: text/plain; charset=UTF-8 Hi everyone, commit 6ae2df93c277b4093b3e54c9606387 d1ba6d10fe into xen-staging includes a new function in xenctrl.h, xc_mem_event_enable. This function name however has been used previously in xenctrl.h up till at least Xen 4.1.2 for a different purpose. We have been using autoconf to check which version of the mem_access API is available in Xen by checking if xc_mem_event_enable is available, signaling that the mem_access API is Xen 4.1 style, and for xc_mem_access_enable signaling 4.2+ style API. See https://github.com/bdpayne/libvmi/blob/master/configure.ac#L140 for more details. Now with this function being reintroduced, it becomes more complicated to determine which version of the mem_access API does Xen actually provide. A #define indicating mem_access API version would nicely overcome this issue, or naming xc_mem_event_enable something else. Furthermore, the new xc_mem_event_enable function unconditionally unpauses the VM. This may not be a desired behavior in all cases, especially if the VM was in a paused state when the function was called. Best, Tamas --001a11370a34c05eba04fc8360f7 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi everyone,
commit 6ae2df93c277b4093b3e54c96= 06387
d1ba6d10fe into xen-staging includes a new function in xenctrl.h,=20 xc_mem_event_enable. This function name however has been used previously in xenctrl.h up till at least Xen 4.1.2 for a different purpose. We=20 have been using autoconf to check which version of the mem_access API is available in Xen by checking if xc_mem_event_enable is available,=20 signaling that the mem_access API is Xen 4.1 style, and for=20 xc_mem_access_enable signaling 4.2+ style API. See https= ://github.com/bdpayne/libvmi/blob/master/configure.ac#L140 for more det= ails.

Now with this function being reintroduced, it becomes more=20 complicated to determine which version of the mem_access API does Xen=20 actually provide. A #define indicating mem_access API version would=20 nicely overcome this issue, or=C2=A0 naming xc_mem_event_enable=20 something else.

Furthermore, the new xc_mem_event_enable funct= ion unconditionally unpauses the VM. This may not be a desired behavior in = all cases, especially if the VM was in a paused state when the function was= called.

Best,
Tamas
--001a11370a34c05eba04fc8360f7-- --===============3412067784055527810== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3412067784055527810==-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Fri, 27 Jun 2014 16:20:55 +0100 Message-ID: <1403882455.3169.72.camel@kazak.uk.xensource.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas Lengyel Cc: Aravindh Puthiyaparambil , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On Mon, 2014-06-23 at 18:31 +0200, Tamas Lengyel wrote: > Hi everyone, > commit 6ae2df93c277b4093b3e54c9606387 > d1ba6d10fe into xen-staging includes a new function in xenctrl.h, > xc_mem_event_enable. This function name however has been used > previously in xenctrl.h up till at least Xen 4.1.2 for a different > purpose. We have been using autoconf to check which version of the > mem_access API is available in Xen by checking if xc_mem_event_enable > is available, signaling that the mem_access API is Xen 4.1 style, and > for xc_mem_access_enable signaling 4.2+ style API. See > https://github.com/bdpayne/libvmi/blob/master/configure.ac#L140 for > more details. > > > Now with this function being reintroduced, it becomes more complicated > to determine which version of the mem_access API does Xen actually > provide. A #define indicating mem_access API version would nicely > overcome this issue, or naming xc_mem_event_enable something else. Doesn't configure support checking for functions with a given prototype? > Furthermore, the new xc_mem_event_enable function unconditionally > unpauses the VM. This may not be a desired behavior in all cases, > especially if the VM was in a paused state when the function was > called. domain pauses are referenced counted on the hypervisor side. Ian. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas Lengyel Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 14:06:24 +0200 Message-ID: References: <1403882455.3169.72.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3009633509711569096==" Return-path: In-Reply-To: <1403882455.3169.72.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Aravindh Puthiyaparambil , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --===============3009633509711569096== Content-Type: multipart/alternative; boundary=001a11c31a3af8f83204fd0c7c1d --001a11c31a3af8f83204fd0c7c1d Content-Type: text/plain; charset=UTF-8 > > Now with this function being reintroduced, it becomes more complicated > > to determine which version of the mem_access API does Xen actually > > provide. A #define indicating mem_access API version would nicely > > overcome this issue, or naming xc_mem_event_enable something else. > > Doesn't configure support checking for functions with a given prototype? > It does but in a very hacky way, essentially trying to compile code where the function is being called with different prototypes. We can work around it but a clean solution would be preferred at some point. > > Furthermore, the new xc_mem_event_enable function unconditionally > > unpauses the VM. This may not be a desired behavior in all cases, > > especially if the VM was in a paused state when the function was > > called. > > domain pauses are referenced counted on the hypervisor side. > > Ian. > > I did a quick test with xc_domain_pause being called twice, then xc_domain_unpause once and the domain was in unpaused state at the end. Same with the xen-access example, a paused domain gets automatically unpaused by libxc, either though the xen-access code doesn't specifically say it should be. This is now a rather opaque behavior of libxc. As it is not merged into master I guess I will just submit a patch for it.. Tamas --001a11c31a3af8f83204fd0c7c1d Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

=
> Now with this function being reintroduced, it becomes more complicated=
> to determine which version of the mem_access API does Xen actually
> provide. A #define indicating mem_access API version would nicely
> overcome this issue, or =C2=A0naming xc_mem_event_enable something els= e.

Doesn't configure support checking for functions with a given pro= totype?

It does but in a very hacky way= , essentially trying to compile code where the function is being called wit= h different prototypes. We can work around it but a clean solution would be= preferred at some point.

=C2=A0

Ian.


I did a quick test with xc_domain_p= ause being called twice, then xc_domain_unpause once and the domain was in = unpaused state at the end. Same with the xen-access example, a paused domai= n gets automatically unpaused by libxc, either though the xen-access code d= oesn't specifically say it should be. This is now a rather opaque behav= ior of libxc. As it is not merged into master I guess I will just submit a = patch for it..

Tamas

--001a11c31a3af8f83204fd0c7c1d-- --===============3009633509711569096== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============3009633509711569096==-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Jackson Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 13:42:59 +0100 Message-ID: <21425.23379.795171.801716@mariner.uk.xensource.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas Lengyel Cc: Aravindh Puthiyaparambil , Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add helper API to setup ring and enable mem_access""): > > > Now with this function being reintroduced, it becomes more complicated > > to determine which version of the mem_access API does Xen actually > > provide. A #define indicating mem_access API version would nicely > > overcome this issue, or naming xc_mem_event_enable something else. > > Doesn't configure support checking for functions with a given prototype? > > It does but in a very hacky way, essentially trying to compile code where the > function is being called with different prototypes. We can work around it but a > clean solution would be preferred at some point. I agree with your criticism, TBH. Aravindh/Ian, can we rename this function ? > This is now a rather opaque behavior of libxc. As it is not merged > into master I guess I will just submit a patch for it.. Yes. I think this would be the right approach to addressing both of these problems. (Please send two patches, one for each.) Thanks, Ian. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 13:44:36 +0100 Message-ID: <1404132276.14488.41.camel@kazak.uk.xensource.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <21425.23379.795171.801716@mariner.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Jackson Cc: Tamas Lengyel , Aravindh Puthiyaparambil , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: > Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add helper API to setup ring and enable mem_access""): > > > > > Now with this function being reintroduced, it becomes more complicated > > > to determine which version of the mem_access API does Xen actually > > > provide. A #define indicating mem_access API version would nicely > > > overcome this issue, or naming xc_mem_event_enable something else. > > > > Doesn't configure support checking for functions with a given prototype? > > > > It does but in a very hacky way, essentially trying to compile code where the > > function is being called with different prototypes. We can work around it but a > > clean solution would be preferred at some point. > > I agree with your criticism, TBH. Aravindh/Ian, can we rename this > function ? I have no objection to some other name. > > This is now a rather opaque behavior of libxc. As it is not merged > > into master I guess I will just submit a patch for it.. > > Yes. I think this would be the right approach to addressing both of > these problems. (Please send two patches, one for each.) > > Thanks, > Ian. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 14:07:41 +0100 Message-ID: <53B1611D.2080502@citrix.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1404132276.14488.41.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Ian Jackson Cc: Tamas Lengyel , "xen-devel@lists.xen.org" , Aravindh Puthiyaparambil List-Id: xen-devel@lists.xenproject.org On 30/06/14 13:44, Ian Campbell wrote: > On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: >> Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add helper API to setup ring and enable mem_access""): >>> > Now with this function being reintroduced, it becomes more complicated >>> > to determine which version of the mem_access API does Xen actually >>> > provide. A #define indicating mem_access API version would nicely >>> > overcome this issue, or naming xc_mem_event_enable something else. >>> >>> Doesn't configure support checking for functions with a given prototype? >>> >>> It does but in a very hacky way, essentially trying to compile code where the >>> function is being called with different prototypes. We can work around it but a >>> clean solution would be preferred at some point. >> I agree with your criticism, TBH. Aravindh/Ian, can we rename this >> function ? > I have no objection to some other name. > >>> This is now a rather opaque behavior of libxc. As it is not merged >>> into master I guess I will just submit a patch for it.. >> Yes. I think this would be the right approach to addressing both of >> these problems. (Please send two patches, one for each.) >> >> Thanks, >> Ian. There is a bug in Xen. domctl pause and unpause hypercalls are not properly refcounted. Patch on its way. ~Andrew From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 14:09:11 +0100 Message-ID: <1404133751.14488.56.camel@kazak.uk.xensource.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> <53B1611D.2080502@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53B1611D.2080502@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: Tamas Lengyel , "xen-devel@lists.xen.org" , Ian Jackson , Aravindh Puthiyaparambil List-Id: xen-devel@lists.xenproject.org On Mon, 2014-06-30 at 14:07 +0100, Andrew Cooper wrote: > On 30/06/14 13:44, Ian Campbell wrote: > > On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: > >> Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add helper API to setup ring and enable mem_access""): > >>> > Now with this function being reintroduced, it becomes more complicated > >>> > to determine which version of the mem_access API does Xen actually > >>> > provide. A #define indicating mem_access API version would nicely > >>> > overcome this issue, or naming xc_mem_event_enable something else. > >>> > >>> Doesn't configure support checking for functions with a given prototype? > >>> > >>> It does but in a very hacky way, essentially trying to compile code where the > >>> function is being called with different prototypes. We can work around it but a > >>> clean solution would be preferred at some point. > >> I agree with your criticism, TBH. Aravindh/Ian, can we rename this > >> function ? > > I have no objection to some other name. > > > >>> This is now a rather opaque behavior of libxc. As it is not merged > >>> into master I guess I will just submit a patch for it.. > >> Yes. I think this would be the right approach to addressing both of > >> these problems. (Please send two patches, one for each.) > >> > >> Thanks, > >> Ian. > > There is a bug in Xen. domctl pause and unpause hypercalls are not > properly refcounted. Patch on its way. I had convinced myself by reading that they were, but Tamas' experimental evidence obviously counters that ;-) Ian. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 14:10:53 +0100 Message-ID: <53B161DD.1010809@citrix.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> <53B1611D.2080502@citrix.com> <1404133751.14488.56.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1404133751.14488.56.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Tamas Lengyel , "xen-devel@lists.xen.org" , Ian Jackson , Aravindh Puthiyaparambil List-Id: xen-devel@lists.xenproject.org On 30/06/14 14:09, Ian Campbell wrote: > On Mon, 2014-06-30 at 14:07 +0100, Andrew Cooper wrote: >> On 30/06/14 13:44, Ian Campbell wrote: >>> On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: >>>> Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add helper API to setup ring and enable mem_access""): >>>>> > Now with this function being reintroduced, it becomes more complicated >>>>> > to determine which version of the mem_access API does Xen actually >>>>> > provide. A #define indicating mem_access API version would nicely >>>>> > overcome this issue, or naming xc_mem_event_enable something else. >>>>> >>>>> Doesn't configure support checking for functions with a given prototype? >>>>> >>>>> It does but in a very hacky way, essentially trying to compile code where the >>>>> function is being called with different prototypes. We can work around it but a >>>>> clean solution would be preferred at some point. >>>> I agree with your criticism, TBH. Aravindh/Ian, can we rename this >>>> function ? >>> I have no objection to some other name. >>> >>>>> This is now a rather opaque behavior of libxc. As it is not merged >>>>> into master I guess I will just submit a patch for it.. >>>> Yes. I think this would be the right approach to addressing both of >>>> these problems. (Please send two patches, one for each.) >>>> >>>> Thanks, >>>> Ian. >> There is a bug in Xen. domctl pause and unpause hypercalls are not >> properly refcounted. Patch on its way. > I had convinced myself by reading that they were, but Tamas' > experimental evidence obviously counters that ;-) > > Ian. > The "d->is_paused_by_controller" is a boolean, not an atomic count. The second pause doesn't add an extra pause ref to the domain, and the first unpause resumes the domain. ~Andrew From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 15:12:15 +0100 Message-ID: <53B18C5F020000780001E96B@mail.emea.novell.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> <53B1611D.2080502@citrix.com> <1404133751.14488.56.camel@kazak.uk.xensource.com> <53B161DD.1010809@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53B161DD.1010809@citrix.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , Ian Campbell Cc: Tamas Lengyel , "xen-devel@lists.xen.org" , Ian Jackson , Aravindh Puthiyaparambil List-Id: xen-devel@lists.xenproject.org >>> On 30.06.14 at 15:10, wrote: > On 30/06/14 14:09, Ian Campbell wrote: >> On Mon, 2014-06-30 at 14:07 +0100, Andrew Cooper wrote: >>> On 30/06/14 13:44, Ian Campbell wrote: >>>> On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: >>>>> Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add > helper API to setup ring and enable mem_access""): >>>>>> > Now with this function being reintroduced, it becomes more complicated >>>>>> > to determine which version of the mem_access API does Xen actually >>>>>> > provide. A #define indicating mem_access API version would nicely >>>>>> > overcome this issue, or naming xc_mem_event_enable something else. >>>>>> >>>>>> Doesn't configure support checking for functions with a given prototype? >>>>>> >>>>>> It does but in a very hacky way, essentially trying to compile code where > the >>>>>> function is being called with different prototypes. We can work around it > but a >>>>>> clean solution would be preferred at some point. >>>>> I agree with your criticism, TBH. Aravindh/Ian, can we rename this >>>>> function ? >>>> I have no objection to some other name. >>>> >>>>>> This is now a rather opaque behavior of libxc. As it is not merged >>>>>> into master I guess I will just submit a patch for it.. >>>>> Yes. I think this would be the right approach to addressing both of >>>>> these problems. (Please send two patches, one for each.) >>>>> >>>>> Thanks, >>>>> Ian. >>> There is a bug in Xen. domctl pause and unpause hypercalls are not >>> properly refcounted. Patch on its way. >> I had convinced myself by reading that they were, but Tamas' >> experimental evidence obviously counters that ;-) > > The "d->is_paused_by_controller" is a boolean, not an atomic count. > > The second pause doesn't add an extra pause ref to the domain, and the > first unpause resumes the domain. Which, as you see, is (in the hypervisor) intended behavior. If the tool stack wants this to be ref-counted, it needs to do so by itself. Jan From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 15:19:34 +0100 Message-ID: <53B171F6.10406@citrix.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> <53B1611D.2080502@citrix.com> <1404133751.14488.56.camel@kazak.uk.xensource.com> <53B161DD.1010809@citrix.com> <53B18C5F020000780001E96B@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <53B18C5F020000780001E96B@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Ian Campbell Cc: Tamas Lengyel , Aravindh Puthiyaparambil , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 30/06/14 15:12, Jan Beulich wrote: >>>> On 30.06.14 at 15:10, wrote: >> On 30/06/14 14:09, Ian Campbell wrote: >>> On Mon, 2014-06-30 at 14:07 +0100, Andrew Cooper wrote: >>>> On 30/06/14 13:44, Ian Campbell wrote: >>>>> On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: >>>>>> Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add >> helper API to setup ring and enable mem_access""): >>>>>>> > Now with this function being reintroduced, it becomes more complicated >>>>>>> > to determine which version of the mem_access API does Xen actually >>>>>>> > provide. A #define indicating mem_access API version would nicely >>>>>>> > overcome this issue, or naming xc_mem_event_enable something else. >>>>>>> >>>>>>> Doesn't configure support checking for functions with a given prototype? >>>>>>> >>>>>>> It does but in a very hacky way, essentially trying to compile code where >> the >>>>>>> function is being called with different prototypes. We can work around it >> but a >>>>>>> clean solution would be preferred at some point. >>>>>> I agree with your criticism, TBH. Aravindh/Ian, can we rename this >>>>>> function ? >>>>> I have no objection to some other name. >>>>> >>>>>>> This is now a rather opaque behavior of libxc. As it is not merged >>>>>>> into master I guess I will just submit a patch for it.. >>>>>> Yes. I think this would be the right approach to addressing both of >>>>>> these problems. (Please send two patches, one for each.) >>>>>> >>>>>> Thanks, >>>>>> Ian. >>>> There is a bug in Xen. domctl pause and unpause hypercalls are not >>>> properly refcounted. Patch on its way. >>> I had convinced myself by reading that they were, but Tamas' >>> experimental evidence obviously counters that ;-) >> The "d->is_paused_by_controller" is a boolean, not an atomic count. >> >> The second pause doesn't add an extra pause ref to the domain, and the >> first unpause resumes the domain. > Which, as you see, is (in the hypervisor) intended behavior. If the > tool stack wants this to be ref-counted, it needs to do so by itself. > > Jan I can see that it is not the current behaviour, but no indication as to why it is intended not to be properly recounted. With memaccess setup like this it is perfectly possible to have multiple toolstack userspace processes needing to pause/unpause the domain over small periods to ensure safety. Refcounting in Xen is the only plausible way of maintaining consistency; there is no safe way to refcount this in the toolstack. The patch from Tanas introduces a TOCTOU race which won't solve the problem in the general case. ~Andrew From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas Lengyel Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 16:49:54 +0200 Message-ID: References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1461594083009982013==" Return-path: In-Reply-To: <1404132276.14488.41.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Aravindh Puthiyaparambil , Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --===============1461594083009982013== Content-Type: multipart/alternative; boundary=047d7bf0cad4a52c8804fd0ec529 --047d7bf0cad4a52c8804fd0ec529 Content-Type: text/plain; charset=UTF-8 On Mon, Jun 30, 2014 at 2:44 PM, Ian Campbell wrote: > On Mon, 2014-06-30 at 13:42 +0100, Ian Jackson wrote: > > Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem_access: Add > helper API to setup ring and enable mem_access""): > > > > > > > Now with this function being reintroduced, it becomes more > complicated > > > > to determine which version of the mem_access API does Xen > actually > > > > provide. A #define indicating mem_access API version would nicely > > > > overcome this issue, or naming xc_mem_event_enable something > else. > > > > > > Doesn't configure support checking for functions with a given > prototype? > > > > > > It does but in a very hacky way, essentially trying to compile code > where the > > > function is being called with different prototypes. We can work around > it but a > > > clean solution would be preferred at some point. > > > > I agree with your criticism, TBH. Aravindh/Ian, can we rename this > > function ? > > I have no objection to some other name. > A question regarding renaming the xc_mem_event_enable function. The public xenctrl.h clearly says /** * mem_event operations. Internal use only. */ There are only three of these, xc_mem_event_control, xc_mem_event_memop and xc_mem_event_enable. Wouldn't it make more sense to just exclude these functions from the public header and move them to xc_private.h? Why have internal functions in the public header? Tamas --047d7bf0cad4a52c8804fd0ec529 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On M= on, Jun 30, 2014 at 2:44 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
On Mon, 2= 014-06-30 at 13:42 +0100, Ian Jackson wrote:
> Tamas Lengyel writes ("Re: [Xen-devel] Issues regarding "mem= _access: Add helper API to setup ring and enable mem_access""): > >
> > =C2=A0 =C2=A0 > Now with this function being reintroduced, it = becomes more complicated
> > =C2=A0 =C2=A0 > to determine which version of the mem_access A= PI does Xen actually
> > =C2=A0 =C2=A0 > provide. A #define indicating mem_access API v= ersion would nicely
> > =C2=A0 =C2=A0 > overcome this issue, or =C2=A0naming xc_mem_ev= ent_enable something else.
> >
> > =C2=A0 =C2=A0 Doesn't configure support checking for function= s with a given prototype?
> >
> > It does but in a very hacky way, essentially trying to compile co= de where the
> > function is being called with different prototypes. We can work a= round it but a
> > clean solution would be preferred at some point.
>
> I agree with your criticism, TBH. =C2=A0Aravindh/Ian, can we rename th= is
> function ?

I have no objection to some other name.

A question regarding renaming the xc_mem_event_= enable function. The public xenctrl.h clearly says

/**
=C2=A0* me= m_event operations. Internal use only.
=C2=A0*/

There are only three of these, xc_mem_event_control, xc_mem_event_memop and=20 xc_mem_event_enable. Wouldn't it make more sense to just exclude these= =20 functions from the public header and move them to xc_private.h? Why have in= ternal functions in the public header?

Tamas
--047d7bf0cad4a52c8804fd0ec529-- --===============1461594083009982013== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============1461594083009982013==-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Aravindh Puthiyaparambil (aravindp)" Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 22:14:28 +0000 Message-ID: <97A500D504438F4ABC02EBA81613CC633185EDA6@xmb-aln-x02.cisco.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas Lengyel , Ian Campbell Cc: Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org > > I agree with your criticism, TBH. Aravindh/Ian, can we rename this > > function ? > > I have no objection to some other name. How about xc_enable_mem_event()? If that is fine, I will submit a patch. >A question regarding renaming the xc_mem_event_enable function. The >public xenctrl.h clearly says > >/** > * mem_event operations. Internal use only. > */ > >There are only three of these, xc_mem_event_control, >xc_mem_event_memop and xc_mem_event_enable. Wouldn't it make more >sense to just exclude these functions from the public header and move them >to xc_private.h? Why have internal functions in the public header? I too think these can be moved to the xc_private.h. IanC / IanJ, what are your thoughts on doing this? Thanks, Aravindh From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Aravindh Puthiyaparambil (aravindp)" Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Mon, 30 Jun 2014 22:31:32 +0000 Message-ID: <97A500D504438F4ABC02EBA81613CC633185EDF5@xmb-aln-x02.cisco.com> References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas Lengyel , Ian Campbell Cc: Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org >> > I agree with your criticism, TBH. Aravindh/Ian, can we rename this >> > function ? >> >> I have no objection to some other name. > >How about xc_enable_mem_event()? If that is fine, I will submit a patch. > >>A question regarding renaming the xc_mem_event_enable function. The >>public xenctrl.h clearly says >> >>/** >> * mem_event operations. Internal use only. >> */ >> >>There are only three of these, xc_mem_event_control, >>xc_mem_event_memop and xc_mem_event_enable. Wouldn't it make >more >>sense to just exclude these functions from the public header and move >them >>to xc_private.h? Why have internal functions in the public header? > >I too think these can be moved to the xc_private.h. IanC / IanJ, what are your >thoughts on doing this? Forgot to add that if this move is done then I am assuming the rename is not required. Correct? Thanks, Aravindh From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas Lengyel Subject: Re: Issues regarding "mem_access: Add helper API to setup ring and enable mem_access" Date: Wed, 2 Jul 2014 13:54:41 +0200 Message-ID: References: <1403882455.3169.72.camel@kazak.uk.xensource.com> <21425.23379.795171.801716@mariner.uk.xensource.com> <1404132276.14488.41.camel@kazak.uk.xensource.com> <97A500D504438F4ABC02EBA81613CC633185EDF5@xmb-aln-x02.cisco.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5383753705448170694==" Return-path: In-Reply-To: <97A500D504438F4ABC02EBA81613CC633185EDF5@xmb-aln-x02.cisco.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Aravindh Puthiyaparambil (aravindp)" , Andres Lagar-Cavilla Cc: Ian Jackson , Ian Campbell , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --===============5383753705448170694== Content-Type: multipart/alternative; boundary=001a1132f770b41b6b04fd348e63 --001a1132f770b41b6b04fd348e63 Content-Type: text/plain; charset=ISO-8859-1 On Tue, Jul 1, 2014 at 12:31 AM, Aravindh Puthiyaparambil (aravindp) < aravindp@cisco.com> wrote: > >> > I agree with your criticism, TBH. Aravindh/Ian, can we rename > this > >> > function ? > >> > >> I have no objection to some other name. > > > >How about xc_enable_mem_event()? If that is fine, I will submit a patch. > > > >>A question regarding renaming the xc_mem_event_enable function. The > >>public xenctrl.h clearly says > >> > >>/** > >> * mem_event operations. Internal use only. > >> */ > >> > >>There are only three of these, xc_mem_event_control, > >>xc_mem_event_memop and xc_mem_event_enable. Wouldn't it make > >more > >>sense to just exclude these functions from the public header and move > >them > >>to xc_private.h? Why have internal functions in the public header? > > > >I too think these can be moved to the xc_private.h. IanC / IanJ, what are > your > >thoughts on doing this? > > Forgot to add that if this move is done then I am assuming the rename is > not required. Correct? > > Thanks, > Aravindh > With relocating these functions to xc_private.h the issue I had would be solved so no renaming would be required. My patch for doing that is on its way momentarily. Tamas --001a1132f770b41b6b04fd348e63 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On T= ue, Jul 1, 2014 at 12:31 AM, Aravindh Puthiyaparambil (aravindp) <aravin= dp@cisco.com> wrote:
>> = =A0 =A0 =A0> I agree with your criticism, TBH. =A0Aravindh/Ian, can we r= ename this
>> =A0 =A0 =A0> function ?
>>
>> =A0 =A0 =A0I have no objection to some other name.
>
>How about xc_enable_mem_event()? If that is fine,= I will submit a patch.
>
>>A question regarding renaming the xc_mem_even= t_enable function. The
>>public xenctrl.h clearly says
>>
>>/**
>> * mem_event operations. Internal use only.
>> */
>>
>>There are only three of these, xc_mem_event_control,
>>xc_mem_event_memop and xc_mem_event_enable. Wouldn't it make >more
>>sense to just exclude these functions from the public header and mo= ve
>them
>>to xc_private.h? Why have internal functions in the public header?<= br> >
>I too think these can be moved to the xc_private.= h. IanC / IanJ, what are your
>thoughts on doing this?

Forgot to add that if this move is done then I am assuming the rename= is not required. Correct?

Thanks,
Aravindh

With relocating these functions to x= c_private.h the issue I had would be solved so no renaming would be require= d. My patch for doing that is on its way momentarily.

Tamas

--001a1132f770b41b6b04fd348e63-- --===============5383753705448170694== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============5383753705448170694==--