> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 15 April 2020 09:45
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian Jackson
> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
> 
> Patch 1 was considered to become part of the XSA, but it was then
> decided against. The other two are a little bit of cleanup, albeit
> there's certainly far more room for tidying. Yet then again Paul,
> in his mail from Mar 13, was asking whether we shouldn't drop
> xenoprof altogether, at which point cleaning up the code would be
> wasted effort.
> 

That's still my opinion. This is a large chunk of (only passively maintained) code which I think is of very limited value (since it relates to an old tool, and it only works for PV domains).

  Paul

On 15/04/2020 09:50, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 15 April 2020 09:45
>> To: xen-devel@lists.xenproject.org
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian Jackson
>> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
>> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
>> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
>>
>> Patch 1 was considered to become part of the XSA, but it was then
>> decided against. The other two are a little bit of cleanup, albeit
>> there's certainly far more room for tidying. Yet then again Paul,
>> in his mail from Mar 13, was asking whether we shouldn't drop
>> xenoprof altogether, at which point cleaning up the code would be
>> wasted effort.
>>
> That's still my opinion. This is a large chunk of (only passively maintained) code which I think is of very limited value (since it relates to an old tool, and it only works for PV domains).

... and yet, noone has bothered getting any other profiler in to a
half-usable state.

You can already Kconfig it out, and yes it is a PITA to use on modern
systems where at the minimum, you need to patch the CPU model whitelist,
and in some cases extend the MSR whitelist in Xen, but at this point
where there are 0 viable alternatives for profiling, removing it would
be a deeply short-sighted move.

~Andrew

Hi Paul,

On 15/04/2020 09:50, Paul Durrant wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 15 April 2020 09:45
>> To: xen-devel@lists.xenproject.org
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian Jackson
>> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
>> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
>> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
>>
>> Patch 1 was considered to become part of the XSA, but it was then
>> decided against. The other two are a little bit of cleanup, albeit
>> there's certainly far more room for tidying. Yet then again Paul,
>> in his mail from Mar 13, was asking whether we shouldn't drop
>> xenoprof altogether, at which point cleaning up the code would be
>> wasted effort.
>>
> 
> That's still my opinion. This is a large chunk of (only passively maintained) code which I think is of very limited value (since it relates to an old tool, and it only works for PV domains).

While there are no active user we are aware of, this is an example on 
how to implement a profiler backend with Xen. So I would agree with 
Andrew here.

IIRC, the reason behind your request is it makes difficult for your 
xenheap work. Am I correct? If so, do you have a thread explaining the 
issues?

Cheers,

-- 
Julien Grall

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: 20 April 2020 15:15
> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
> <wl@xen.org>
> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
> 
> Hi Paul,
> 
> On 15/04/2020 09:50, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 15 April 2020 09:45
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian
> Jackson
> >> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
> >> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
> >> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
> >>
> >> Patch 1 was considered to become part of the XSA, but it was then
> >> decided against. The other two are a little bit of cleanup, albeit
> >> there's certainly far more room for tidying. Yet then again Paul,
> >> in his mail from Mar 13, was asking whether we shouldn't drop
> >> xenoprof altogether, at which point cleaning up the code would be
> >> wasted effort.
> >>
> >
> > That's still my opinion. This is a large chunk of (only passively maintained) code which I think is
> of very limited value (since it relates to an old tool, and it only works for PV domains).
> 
> While there are no active user we are aware of, this is an example on
> how to implement a profiler backend with Xen. So I would agree with
> Andrew here.
> 
> IIRC, the reason behind your request is it makes difficult for your
> xenheap work. Am I correct? If so, do you have a thread explaining the
> issues?

After shared info and grant table, it is the only other occurrence of a xenheap page shared with a non-system domain. Also, it cannot be trivially replaced with an 'extra' domheap page because its assignment changes. Thus a whole bunch of cleanup work that I was hoping to do (largely in domain_relinquish_resources and free_domheap_pages) is either ruled out, or would have to special-case this type of page.
Also, I am unconvinced that PV guests are sufficiently common these days (apart from dom0) that profiling them is of any real use.

  Paul

Hi Paul,

On 20/04/2020 16:01, Paul Durrant wrote:
>> -----Original Message-----
>> From: Julien Grall <julien@xen.org>
>> Sent: 20 April 2020 15:15
>> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
>> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
>> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
>> <wl@xen.org>
>> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
>>
>> Hi Paul,
>>
>> On 15/04/2020 09:50, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Jan Beulich <jbeulich@suse.com>
>>>> Sent: 15 April 2020 09:45
>>>> To: xen-devel@lists.xenproject.org
>>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian
>> Jackson
>>>> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
>>>> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
>>>> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
>>>>
>>>> Patch 1 was considered to become part of the XSA, but it was then
>>>> decided against. The other two are a little bit of cleanup, albeit
>>>> there's certainly far more room for tidying. Yet then again Paul,
>>>> in his mail from Mar 13, was asking whether we shouldn't drop
>>>> xenoprof altogether, at which point cleaning up the code would be
>>>> wasted effort.
>>>>
>>>
>>> That's still my opinion. This is a large chunk of (only passively maintained) code which I think is
>> of very limited value (since it relates to an old tool, and it only works for PV domains).
>>
>> While there are no active user we are aware of, this is an example on
>> how to implement a profiler backend with Xen. So I would agree with
>> Andrew here.
>>
>> IIRC, the reason behind your request is it makes difficult for your
>> xenheap work. Am I correct? If so, do you have a thread explaining the
>> issues?
> 
> After shared info and grant table, it is the only other occurrence of a xenheap page shared with a non-system domain. Also, it cannot be trivially replaced with an 'extra' domheap page because its assignment changes. Thus a whole bunch of cleanup work that I was hoping to do (largely in domain_relinquish_resources and free_domheap_pages) is either ruled out, or would have to special-case this type of page.

My knowledge of xenoprof is very limited, so I might be wrong.

 From my understanding, a buffer can only be shared between two domains:
    - When in passive mode, the buffer will be shared with the primary 
profiler (always the hwdom per the check in xenoprof_op_init()).
    - When in active mode, the buffer will be shared with the domain 
requesting to be profiled.

Would it be possible to consider to have a separate buffer for the 
passive mode and active mode?

> Also, I am unconvinced that PV guests are sufficiently common these days (apart from dom0) that profiling them is of any real use.

Even an HVM guest can't profile itself, I think it would be useful to 
have dom0 to profile an HVM guest. Are you suggesting this doesn't work?

Cheers,

-- 
Julien Grall

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: 24 April 2020 11:35
> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
> <wl@xen.org>
> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
> 
> Hi Paul,
> 
> On 20/04/2020 16:01, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Julien Grall <julien@xen.org>
> >> Sent: 20 April 2020 15:15
> >> To: paul@xen.org; 'Jan Beulich' <jbeulich@suse.com>; xen-devel@lists.xenproject.org
> >> Cc: 'Andrew Cooper' <andrew.cooper3@citrix.com>; 'George Dunlap' <george.dunlap@citrix.com>; 'Ian
> >> Jackson' <ian.jackson@eu.citrix.com>; 'Stefano Stabellini' <sstabellini@kernel.org>; 'Wei Liu'
> >> <wl@xen.org>
> >> Subject: Re: [PATCH 0/3] xenoprof: XSA-313 follow-up
> >>
> >> Hi Paul,
> >>
> >> On 15/04/2020 09:50, Paul Durrant wrote:
> >>>> -----Original Message-----
> >>>> From: Jan Beulich <jbeulich@suse.com>
> >>>> Sent: 15 April 2020 09:45
> >>>> To: xen-devel@lists.xenproject.org
> >>>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap <george.dunlap@citrix.com>; Ian
> >> Jackson
> >>>> <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Stefano Stabellini
> >>>> <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; Paul Durrant <paul@xen.org>
> >>>> Subject: [PATCH 0/3] xenoprof: XSA-313 follow-up
> >>>>
> >>>> Patch 1 was considered to become part of the XSA, but it was then
> >>>> decided against. The other two are a little bit of cleanup, albeit
> >>>> there's certainly far more room for tidying. Yet then again Paul,
> >>>> in his mail from Mar 13, was asking whether we shouldn't drop
> >>>> xenoprof altogether, at which point cleaning up the code would be
> >>>> wasted effort.
> >>>>
> >>>
> >>> That's still my opinion. This is a large chunk of (only passively maintained) code which I think
> is
> >> of very limited value (since it relates to an old tool, and it only works for PV domains).
> >>
> >> While there are no active user we are aware of, this is an example on
> >> how to implement a profiler backend with Xen. So I would agree with
> >> Andrew here.
> >>
> >> IIRC, the reason behind your request is it makes difficult for your
> >> xenheap work. Am I correct? If so, do you have a thread explaining the
> >> issues?
> >
> > After shared info and grant table, it is the only other occurrence of a xenheap page shared with a
> non-system domain. Also, it cannot be trivially replaced with an 'extra' domheap page because its
> assignment changes. Thus a whole bunch of cleanup work that I was hoping to do (largely in
> domain_relinquish_resources and free_domheap_pages) is either ruled out, or would have to special-case
> this type of page.
> 
> My knowledge of xenoprof is very limited, so I might be wrong.
> 
>  From my understanding, a buffer can only be shared between two domains:
>     - When in passive mode, the buffer will be shared with the primary
> profiler (always the hwdom per the check in xenoprof_op_init()).
>     - When in active mode, the buffer will be shared with the domain
> requesting to be profiled.
> 
> Would it be possible to consider to have a separate buffer for the
> passive mode and active mode?

I think that may well work, yes.

> 
> > Also, I am unconvinced that PV guests are sufficiently common these days (apart from dom0) that
> profiling them is of any real use.
> 
> Even an HVM guest can't profile itself, I think it would be useful to
> have dom0 to profile an HVM guest. Are you suggesting this doesn't work?
> 

No. That may work for a PV dom0; I'll have to try it. So, yes, passive profiling may indeed still have value.

  Paul