* [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 11:47 [PATCH 0/5] gcov: more cleanup Wei Liu
@ 2016-09-02 11:47 ` Wei Liu
2016-09-02 11:56 ` Jan Beulich
2016-09-02 11:47 ` [PATCH 2/5] gcov: collect more sections to constructor list Wei Liu
` (4 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Wei Liu @ 2016-09-02 11:47 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Jan Beulich
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
The location of warning_add() is a bit arbitrary because gcov doesn't
have an initialisation routine.
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/common/gcov/gcov.c | 5 +++++
xen/common/kernel.c | 6 ++++--
xen/drivers/char/console.c | 9 +++++++++
xen/include/xen/lib.h | 1 +
4 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
index b5717b9..6d18729 100644
--- a/xen/common/gcov/gcov.c
+++ b/xen/common/gcov/gcov.c
@@ -23,6 +23,11 @@
#include <public/xen.h>
#include <public/gcov.h>
+const char __initconst warning_gcov[] =
+ "WARNING: GCOV SUPPORT IS ENABLED\n"
+ "This option is *ONLY* intended to aid testing of Xen.\n"
+ "Please *DO NOT* use this in production.\n";
+
static struct gcov_info *info_list;
/*
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 2d3db64..324cc24 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -173,6 +173,7 @@ unsigned int tainted;
*
* 'C' - Console output is synchronous.
* 'E' - An error (e.g. a machine check exceptions) has been injected.
+ * 'G' - GCov support is enabled.
* 'H' - HVM forced emulation prefix is permitted.
* 'M' - Machine had a machine check experience.
*
@@ -182,11 +183,12 @@ char *print_tainted(char *str)
{
if ( tainted )
{
- snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c",
+ snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c",
tainted & TAINT_MACHINE_CHECK ? 'M' : ' ',
tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ',
tainted & TAINT_ERROR_INJECT ? 'E' : ' ',
- tainted & TAINT_HVM_FEP ? 'H' : ' ');
+ tainted & TAINT_HVM_FEP ? 'H' : ' ',
+ tainted & TAINT_GCOV ? 'G' : ' ');
}
else
{
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 650035d..77604f9 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -792,6 +792,10 @@ void __init console_init_postirq(void)
console_init_ring();
}
+#ifdef CONFIG_GCOV
+extern char warning_gcov[];
+#endif
+
void __init console_endboot(void)
{
printk("Std. Loglevel: %s", loglvl_str(xenlog_lower_thresh));
@@ -802,6 +806,11 @@ void __init console_endboot(void)
printk(" (Rate-limited: %s)", loglvl_str(xenlog_guest_upper_thresh));
printk("\n");
+#ifdef CONFIG_GCOV
+ warning_add(warning_gcov);
+ add_taint(TAINT_GCOV);
+#endif
+
warning_print();
video_endboot();
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index e518adc..7bcc910 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -143,6 +143,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c);
#define TAINT_MACHINE_CHECK (1u << 1)
#define TAINT_ERROR_INJECT (1u << 2)
#define TAINT_HVM_FEP (1u << 3)
+#define TAINT_GCOV (1u << 4)
extern unsigned int tainted;
#define TAINT_STRING_MAX_LEN 20
extern char *print_tainted(char *str);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 11:47 ` [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled Wei Liu
@ 2016-09-02 11:56 ` Jan Beulich
2016-09-02 12:01 ` Wei Liu
0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2016-09-02 11:56 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Xen-devel
>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
Since this is a config option - why bother issuing a warning and
tainting the hypervisor?
> --- a/xen/common/gcov/gcov.c
> +++ b/xen/common/gcov/gcov.c
> @@ -23,6 +23,11 @@
> #include <public/xen.h>
> #include <public/gcov.h>
>
> +const char __initconst warning_gcov[] =
> + "WARNING: GCOV SUPPORT IS ENABLED\n"
> + "This option is *ONLY* intended to aid testing of Xen.\n"
> + "Please *DO NOT* use this in production.\n";
Note the type (const) difference between this and ...
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -792,6 +792,10 @@ void __init console_init_postirq(void)
> console_init_ring();
> }
>
> +#ifdef CONFIG_GCOV
> +extern char warning_gcov[];
> +#endif
... this. That's one of the reasons declarations of stuff defined in
C sources should be put in a header, which then gets included by
both producer and consumer(s). But ...
> @@ -802,6 +806,11 @@ void __init console_endboot(void)
> printk(" (Rate-limited: %s)", loglvl_str(xenlog_guest_upper_thresh));
> printk("\n");
>
> +#ifdef CONFIG_GCOV
> + warning_add(warning_gcov);
> + add_taint(TAINT_GCOV);
> +#endif
... (if we want this in the first place) how about
#ifdef CONFIG_GCOV
{
static const char __initconst warning_gcov[] = "...";
warning_add(warning_gcov);
add_taint(TAINT_GCOV);
}
#endif
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 11:56 ` Jan Beulich
@ 2016-09-02 12:01 ` Wei Liu
2016-09-02 12:06 ` Jan Beulich
2016-09-02 13:21 ` Julien Grall
0 siblings, 2 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 12:01 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Xen-devel
On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>
> Since this is a config option - why bother issuing a warning and
> tainting the hypervisor?
>
Because there isn't a clear indicator if gcov is enabled.
I think it would be valuable to just tell from the backtrace or console
log that gcov is enabled, then we can legitimately refuse to provide
(security) support for such builds.
> > --- a/xen/common/gcov/gcov.c
> > +++ b/xen/common/gcov/gcov.c
> > @@ -23,6 +23,11 @@
> > #include <public/xen.h>
> > #include <public/gcov.h>
> >
> > +const char __initconst warning_gcov[] =
> > + "WARNING: GCOV SUPPORT IS ENABLED\n"
> > + "This option is *ONLY* intended to aid testing of Xen.\n"
> > + "Please *DO NOT* use this in production.\n";
>
> Note the type (const) difference between this and ...
>
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -792,6 +792,10 @@ void __init console_init_postirq(void)
> > console_init_ring();
> > }
> >
> > +#ifdef CONFIG_GCOV
> > +extern char warning_gcov[];
> > +#endif
>
> ... this. That's one of the reasons declarations of stuff defined in
> C sources should be put in a header, which then gets included by
> both producer and consumer(s). But ...
>
> > @@ -802,6 +806,11 @@ void __init console_endboot(void)
> > printk(" (Rate-limited: %s)", loglvl_str(xenlog_guest_upper_thresh));
> > printk("\n");
> >
> > +#ifdef CONFIG_GCOV
> > + warning_add(warning_gcov);
> > + add_taint(TAINT_GCOV);
> > +#endif
>
> ... (if we want this in the first place) how about
>
> #ifdef CONFIG_GCOV
> {
> static const char __initconst warning_gcov[] = "...";
>
> warning_add(warning_gcov);
> add_taint(TAINT_GCOV);
> }
> #endif
>
Fine with me.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 12:01 ` Wei Liu
@ 2016-09-02 12:06 ` Jan Beulich
2016-09-02 12:13 ` Andrew Cooper
2016-09-02 13:21 ` Julien Grall
1 sibling, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2016-09-02 12:06 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Xen-devel
>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>
>> Since this is a config option - why bother issuing a warning and
>> tainting the hypervisor?
>>
>
> Because there isn't a clear indicator if gcov is enabled.
>
> I think it would be valuable to just tell from the backtrace or console
> log that gcov is enabled, then we can legitimately refuse to provide
> (security) support for such builds.
Then perhaps making it match the "debug=" would be the better
approach for a feature not controlled on the command line?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 12:06 ` Jan Beulich
@ 2016-09-02 12:13 ` Andrew Cooper
2016-09-02 12:26 ` Jan Beulich
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Cooper @ 2016-09-02 12:13 UTC (permalink / raw)
To: Jan Beulich, Wei Liu
Cc: Stefano Stabellini, George Dunlap, Ian Jackson, Tim Deegan, Xen-devel
On 02/09/16 13:06, Jan Beulich wrote:
>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>> Since this is a config option - why bother issuing a warning and
>>> tainting the hypervisor?
>>>
>> Because there isn't a clear indicator if gcov is enabled.
>>
>> I think it would be valuable to just tell from the backtrace or console
>> log that gcov is enabled, then we can legitimately refuse to provide
>> (security) support for such builds.
> Then perhaps making it match the "debug=" would be the better
> approach for a feature not controlled on the command line?
I would prefer not to make it depend on debug=
Coverage on a release hypervisor is equally important, and will be
different from a debug hypervisor.
I am on the fence as to whether a taint is right to use, but I do think
that a "with GCOV" is needed somewhere obvious on the banner line.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 12:13 ` Andrew Cooper
@ 2016-09-02 12:26 ` Jan Beulich
2016-09-02 12:30 ` Andrew Cooper
0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2016-09-02 12:26 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, Xen-devel
>>> On 02.09.16 at 14:13, <andrew.cooper3@citrix.com> wrote:
> On 02/09/16 13:06, Jan Beulich wrote:
>>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
>>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>>> Since this is a config option - why bother issuing a warning and
>>>> tainting the hypervisor?
>>>>
>>> Because there isn't a clear indicator if gcov is enabled.
>>>
>>> I think it would be valuable to just tell from the backtrace or console
>>> log that gcov is enabled, then we can legitimately refuse to provide
>>> (security) support for such builds.
>> Then perhaps making it match the "debug=" would be the better
>> approach for a feature not controlled on the command line?
>
> I would prefer not to make it depend on debug=
>
> Coverage on a release hypervisor is equally important, and will be
> different from a debug hypervisor.
I didn't say "depend on", but "match" (which I mean just logging wise).
> I am on the fence as to whether a taint is right to use, but I do think
> that a "with GCOV" is needed somewhere obvious on the banner line.
Right, hence the matching goal with "debug=".
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 12:26 ` Jan Beulich
@ 2016-09-02 12:30 ` Andrew Cooper
2016-09-02 13:34 ` Wei Liu
0 siblings, 1 reply; 27+ messages in thread
From: Andrew Cooper @ 2016-09-02 12:30 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, Xen-devel
On 02/09/16 13:26, Jan Beulich wrote:
>>>> On 02.09.16 at 14:13, <andrew.cooper3@citrix.com> wrote:
>> On 02/09/16 13:06, Jan Beulich wrote:
>>>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
>>>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>>>> Since this is a config option - why bother issuing a warning and
>>>>> tainting the hypervisor?
>>>>>
>>>> Because there isn't a clear indicator if gcov is enabled.
>>>>
>>>> I think it would be valuable to just tell from the backtrace or console
>>>> log that gcov is enabled, then we can legitimately refuse to provide
>>>> (security) support for such builds.
>>> Then perhaps making it match the "debug=" would be the better
>>> approach for a feature not controlled on the command line?
>> I would prefer not to make it depend on debug=
>>
>> Coverage on a release hypervisor is equally important, and will be
>> different from a debug hypervisor.
> I didn't say "depend on", but "match" (which I mean just logging wise).
>
>> I am on the fence as to whether a taint is right to use, but I do think
>> that a "with GCOV" is needed somewhere obvious on the banner line.
> Right, hence the matching goal with "debug=".
Ah - I see what you mean. Yes - that would be fine by me.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 12:30 ` Andrew Cooper
@ 2016-09-02 13:34 ` Wei Liu
0 siblings, 0 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 13:34 UTC (permalink / raw)
To: Andrew Cooper
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Tim Deegan,
Ian Jackson, Jan Beulich, Xen-devel
On Fri, Sep 02, 2016 at 01:30:40PM +0100, Andrew Cooper wrote:
> On 02/09/16 13:26, Jan Beulich wrote:
> >>>> On 02.09.16 at 14:13, <andrew.cooper3@citrix.com> wrote:
> >> On 02/09/16 13:06, Jan Beulich wrote:
> >>>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
> >>>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
> >>>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> >>>>> Since this is a config option - why bother issuing a warning and
> >>>>> tainting the hypervisor?
> >>>>>
> >>>> Because there isn't a clear indicator if gcov is enabled.
> >>>>
> >>>> I think it would be valuable to just tell from the backtrace or console
> >>>> log that gcov is enabled, then we can legitimately refuse to provide
> >>>> (security) support for such builds.
> >>> Then perhaps making it match the "debug=" would be the better
> >>> approach for a feature not controlled on the command line?
> >> I would prefer not to make it depend on debug=
> >>
> >> Coverage on a release hypervisor is equally important, and will be
> >> different from a debug hypervisor.
> > I didn't say "depend on", but "match" (which I mean just logging wise).
> >
> >> I am on the fence as to whether a taint is right to use, but I do think
> >> that a "with GCOV" is needed somewhere obvious on the banner line.
> > Right, hence the matching goal with "debug=".
>
> Ah - I see what you mean. Yes - that would be fine by me.
>
Fine by me, too.
I will replace this patch with a new one.
Wei.
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 12:01 ` Wei Liu
2016-09-02 12:06 ` Jan Beulich
@ 2016-09-02 13:21 ` Julien Grall
2016-09-02 13:34 ` Wei Liu
1 sibling, 1 reply; 27+ messages in thread
From: Julien Grall @ 2016-09-02 13:21 UTC (permalink / raw)
To: Wei Liu, Jan Beulich
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Xen-devel
Hi Wei,
On 02/09/16 13:01, Wei Liu wrote:
> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>
>> Since this is a config option - why bother issuing a warning and
>> tainting the hypervisor?
>>
>
> Because there isn't a clear indicator if gcov is enabled.
I think this is an issue to all .config option. If I am not mistaken,
currently you are not able to know whether option FOO has been enabled
for your kernel.
Maybe we should integrate the .config in the binary?
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled
2016-09-02 13:21 ` Julien Grall
@ 2016-09-02 13:34 ` Wei Liu
0 siblings, 0 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 13:34 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Jan Beulich, Xen-devel
On Fri, Sep 02, 2016 at 02:21:28PM +0100, Julien Grall wrote:
> Hi Wei,
>
> On 02/09/16 13:01, Wei Liu wrote:
> >On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
> >>>>>On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> >>
> >>Since this is a config option - why bother issuing a warning and
> >>tainting the hypervisor?
> >>
> >
> >Because there isn't a clear indicator if gcov is enabled.
>
> I think this is an issue to all .config option. If I am not mistaken,
> currently you are not able to know whether option FOO has been enabled for
> your kernel.
>
> Maybe we should integrate the .config in the binary?
>
I think that would be a good idea. It is orthogonal to what I'm trying
to do here though.
Wei.
> Regards,
>
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/5] gcov: collect more sections to constructor list
2016-09-02 11:47 [PATCH 0/5] gcov: more cleanup Wei Liu
2016-09-02 11:47 ` [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled Wei Liu
@ 2016-09-02 11:47 ` Wei Liu
2016-09-02 11:58 ` Jan Beulich
2016-09-05 10:10 ` Julien Grall
2016-09-02 11:47 ` [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV Wei Liu
` (3 subsequent siblings)
5 siblings, 2 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 11:47 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich
The version of gcc (4.9.2) I use put constructors into .init_array*
section(s). Collect those sections into constructor list as well.
Modify both arm and x86 scripts to keep them in sync.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
xen/arch/arm/xen.lds.S | 2 ++
xen/arch/x86/xen.lds.S | 2 ++
2 files changed, 4 insertions(+)
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index b24e93b..ffff13a 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -166,6 +166,8 @@ SECTIONS
. = ALIGN(8);
__ctors_start = .;
+ *(.ctors)
+ *(SORT(.init_array.*))
*(.init_array)
__ctors_end = .;
} :text
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 67cfda1..8957efd 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -205,6 +205,8 @@ SECTIONS
. = ALIGN(8);
__ctors_start = .;
*(.ctors)
+ *(SORT(.init_array.*))
+ *(.init_array)
__ctors_end = .;
} :text
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 2/5] gcov: collect more sections to constructor list
2016-09-02 11:47 ` [PATCH 2/5] gcov: collect more sections to constructor list Wei Liu
@ 2016-09-02 11:58 ` Jan Beulich
2016-09-02 12:12 ` Wei Liu
2016-09-05 10:10 ` Julien Grall
1 sibling, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2016-09-02 11:58 UTC (permalink / raw)
To: Wei Liu; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Xen-devel
>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -205,6 +205,8 @@ SECTIONS
> . = ALIGN(8);
> __ctors_start = .;
> *(.ctors)
> + *(SORT(.init_array.*))
> + *(.init_array)
> __ctors_end = .;
> } :text
Is the ordering meaningful? If so, a comment is warranted. If not,
elsewhere we have <section> precede <section>.* .
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] gcov: collect more sections to constructor list
2016-09-02 11:58 ` Jan Beulich
@ 2016-09-02 12:12 ` Wei Liu
0 siblings, 0 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 12:12 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Wei Liu, Xen-devel
On Fri, Sep 02, 2016 at 05:58:43AM -0600, Jan Beulich wrote:
> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -205,6 +205,8 @@ SECTIONS
> > . = ALIGN(8);
> > __ctors_start = .;
> > *(.ctors)
> > + *(SORT(.init_array.*))
> > + *(.init_array)
> > __ctors_end = .;
> > } :text
>
> Is the ordering meaningful? If so, a comment is warranted. If not,
> elsewhere we have <section> precede <section>.* .
>
I don't think the order matters.
I will swap the position of the two entries.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/5] gcov: collect more sections to constructor list
2016-09-02 11:47 ` [PATCH 2/5] gcov: collect more sections to constructor list Wei Liu
2016-09-02 11:58 ` Jan Beulich
@ 2016-09-05 10:10 ` Julien Grall
1 sibling, 0 replies; 27+ messages in thread
From: Julien Grall @ 2016-09-05 10:10 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Andrew Cooper, Stefano Stabellini, Jan Beulich
Hi Wei,
On 02/09/2016 12:47, Wei Liu wrote:
> The version of gcc (4.9.2) I use put constructors into .init_array*
> section(s). Collect those sections into constructor list as well.
>
> Modify both arm and x86 scripts to keep them in sync.
With Jan's comment handled:
Acked-by: Julien Grall <julien.grall@arm.com>
Regards,
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> xen/arch/arm/xen.lds.S | 2 ++
> xen/arch/x86/xen.lds.S | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index b24e93b..ffff13a 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -166,6 +166,8 @@ SECTIONS
>
> . = ALIGN(8);
> __ctors_start = .;
> + *(.ctors)
> + *(SORT(.init_array.*))
> *(.init_array)
> __ctors_end = .;
> } :text
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 67cfda1..8957efd 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -205,6 +205,8 @@ SECTIONS
> . = ALIGN(8);
> __ctors_start = .;
> *(.ctors)
> + *(SORT(.init_array.*))
> + *(.init_array)
> __ctors_end = .;
> } :text
>
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV
2016-09-02 11:47 [PATCH 0/5] gcov: more cleanup Wei Liu
2016-09-02 11:47 ` [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled Wei Liu
2016-09-02 11:47 ` [PATCH 2/5] gcov: collect more sections to constructor list Wei Liu
@ 2016-09-02 11:47 ` Wei Liu
2016-09-02 11:59 ` Jan Beulich
2016-09-19 14:30 ` Ian Jackson
2016-09-02 11:47 ` [PATCH 4/5] gcov: add option to determine gcov format Wei Liu
` (2 subsequent siblings)
5 siblings, 2 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 11:47 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Jan Beulich
The sole purpose of TEST_COVERAGE macro is to guard the availability of
gcov sysctl. Now we have a proper CONFIG_GCOV, use it.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/Rules.mk | 2 +-
xen/common/sysctl.c | 2 +-
xen/include/xen/gcov.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 696aaa8..a9fda71 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -116,7 +116,7 @@ subdir-all := $(subdir-y) $(subdir-n)
$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
ifeq ($(CONFIG_GCOV),y)
-$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage
endif
ifeq ($(lto),y)
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 55f2077..8aea6ef 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -396,7 +396,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
}
break;
-#ifdef TEST_COVERAGE
+#ifdef CONFIG_GCOV
case XEN_SYSCTL_coverage_op:
ret = sysctl_coverage_op(&op->u.coverage_op);
break;
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
index 27c5c37..a7d4a35 100644
--- a/xen/include/xen/gcov.h
+++ b/xen/include/xen/gcov.h
@@ -86,7 +86,7 @@ struct gcov_info
/**
* Sysctl operations for coverage
*/
-#ifdef TEST_COVERAGE
+#ifdef CONFIG_GCOV
int sysctl_coverage_op(xen_sysctl_coverage_op_t *op);
#endif
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV
2016-09-02 11:47 ` [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV Wei Liu
@ 2016-09-02 11:59 ` Jan Beulich
2016-09-19 14:30 ` Ian Jackson
1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2016-09-02 11:59 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Xen-devel
>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> The sole purpose of TEST_COVERAGE macro is to guard the availability of
> gcov sysctl. Now we have a proper CONFIG_GCOV, use it.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV
2016-09-02 11:47 ` [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV Wei Liu
2016-09-02 11:59 ` Jan Beulich
@ 2016-09-19 14:30 ` Ian Jackson
1 sibling, 0 replies; 27+ messages in thread
From: Ian Jackson @ 2016-09-19 14:30 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Tim Deegan,
Jan Beulich, Xen-devel
Wei Liu writes ("[PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV"):
> The sole purpose of TEST_COVERAGE macro is to guard the availability of
> gcov sysctl. Now we have a proper CONFIG_GCOV, use it.
FAOD my reading of xen/Kconfig.debug is that CONFIG_GCOV depends on
CONFIG_DEBUG which says
This option is intended for development purposes
only, and not for production use.
and is therefore out of security support.
Accordingly I think the only security review needed for the hypervisor
part is to check that everything is indeed disabled without
CONFIG_GCOV.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/5] gcov: add option to determine gcov format
2016-09-02 11:47 [PATCH 0/5] gcov: more cleanup Wei Liu
` (2 preceding siblings ...)
2016-09-02 11:47 ` [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV Wei Liu
@ 2016-09-02 11:47 ` Wei Liu
2016-09-02 12:01 ` Jan Beulich
2016-09-06 8:34 ` Wei Liu
2016-09-02 11:47 ` [PATCH 5/5] gcov: split out gcc version specific stuff Wei Liu
2016-09-02 12:04 ` [PATCH 0/5] gcov: more cleanup Andrew Cooper
5 siblings, 2 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 11:47 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Jan Beulich
Currently only gcc 3.4 format is supported.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/Kconfig.debug | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 06afd80..2366b06 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -33,6 +33,19 @@ config GCOV
---help---
Enable gcov (a test coverage program in GCC) support.
+choice
+ prompt "Specify Gcov format"
+ depends on GCOV
+ ---help---
+ The gcov format is determined by gcc version.
+
+config GCOV_FORMAT_3_4
+ bool "GCC 3.4 format"
+ ---help---
+ Select this option to use the format specified in GCC 3.4.
+
+endchoice
+
config LOCK_PROFILE
bool "Lock Profiling"
---help---
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 4/5] gcov: add option to determine gcov format
2016-09-02 11:47 ` [PATCH 4/5] gcov: add option to determine gcov format Wei Liu
@ 2016-09-02 12:01 ` Jan Beulich
2016-09-02 12:08 ` Wei Liu
2016-09-06 8:34 ` Wei Liu
1 sibling, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2016-09-02 12:01 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Ian Jackson,
Tim Deegan, Xen-devel
>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> Currently only gcc 3.4 format is supported.
Doesn't this patch contradict your coverage letter? Here you provide
means to add support for further formats, but there you said there's
no obvious route to that goal.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] gcov: add option to determine gcov format
2016-09-02 12:01 ` Jan Beulich
@ 2016-09-02 12:08 ` Wei Liu
2016-09-02 15:43 ` Wei Liu
0 siblings, 1 reply; 27+ messages in thread
From: Wei Liu @ 2016-09-02 12:08 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Xen-devel
On Fri, Sep 02, 2016 at 06:01:22AM -0600, Jan Beulich wrote:
> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> > Currently only gcc 3.4 format is supported.
>
> Doesn't this patch contradict your coverage letter? Here you provide
> means to add support for further formats, but there you said there's
> no obvious route to that goal.
>
There is a way, we can ditch the old interface and just hand back the
blob.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] gcov: add option to determine gcov format
2016-09-02 12:08 ` Wei Liu
@ 2016-09-02 15:43 ` Wei Liu
0 siblings, 0 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 15:43 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Xen-devel
On Fri, Sep 02, 2016 at 01:08:27PM +0100, Wei Liu wrote:
> On Fri, Sep 02, 2016 at 06:01:22AM -0600, Jan Beulich wrote:
> > >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> > > Currently only gcc 3.4 format is supported.
> >
> > Doesn't this patch contradict your coverage letter? Here you provide
> > means to add support for further formats, but there you said there's
> > no obvious route to that goal.
> >
>
> There is a way, we can ditch the old interface and just hand back the
> blob.
>
Let me try to make myself clearer because now I reread my reply it
doesn't seem to convey my thought.
There are two issues:
1. The sysctl interface is tied to gcc 3.4 format.
2. The implementation inside Xen is tied to gcc 3.4 format.
My cover letter was referring to #1 because there is no way to fit newer
gcc format into the existing Xen sysctl coverage interface. To solve #1
I'm afraid we need to design new interfaces.
#2 is independent of #1. Regardless of what the sysctl interface looks
like, Xen needs to know which format to use (size of the structure,
offset of fields etc) in order to extract information.
This patch (sorta) deals with #2 and is one step towards dealing with
#1.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/5] gcov: add option to determine gcov format
2016-09-02 11:47 ` [PATCH 4/5] gcov: add option to determine gcov format Wei Liu
2016-09-02 12:01 ` Jan Beulich
@ 2016-09-06 8:34 ` Wei Liu
1 sibling, 0 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-06 8:34 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Jan Beulich
On Fri, Sep 02, 2016 at 12:47:08PM +0100, Wei Liu wrote:
> Currently only gcc 3.4 format is supported.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
What should I do with this patch and the next one?
I'm fine with dropping both and replace them with a patch to
Kconfig.debug to make clear the format is currently 3.4, if people feel
strongly about that.
Wei.
> ---
> xen/Kconfig.debug | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 06afd80..2366b06 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -33,6 +33,19 @@ config GCOV
> ---help---
> Enable gcov (a test coverage program in GCC) support.
>
> +choice
> + prompt "Specify Gcov format"
> + depends on GCOV
> + ---help---
> + The gcov format is determined by gcc version.
> +
> +config GCOV_FORMAT_3_4
> + bool "GCC 3.4 format"
> + ---help---
> + Select this option to use the format specified in GCC 3.4.
> +
> +endchoice
> +
> config LOCK_PROFILE
> bool "Lock Profiling"
> ---help---
> --
> 2.1.4
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/5] gcov: split out gcc version specific stuff
2016-09-02 11:47 [PATCH 0/5] gcov: more cleanup Wei Liu
` (3 preceding siblings ...)
2016-09-02 11:47 ` [PATCH 4/5] gcov: add option to determine gcov format Wei Liu
@ 2016-09-02 11:47 ` Wei Liu
2016-09-02 12:04 ` [PATCH 0/5] gcov: more cleanup Andrew Cooper
5 siblings, 0 replies; 27+ messages in thread
From: Wei Liu @ 2016-09-02 11:47 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, Jan Beulich
Gcov record format is tied to specific gcc versions. The current code in
tree conforms to gcc 3.4 format.
Move structure definitions to a specific file and factor out some
version specific functions.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/common/gcov/gcov.c | 64 ++++++++++++++++++++++-------------
xen/common/gcov/gcov_3_4.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++
xen/include/xen/gcov.h | 80 -------------------------------------------
3 files changed, 125 insertions(+), 103 deletions(-)
create mode 100644 xen/common/gcov/gcov_3_4.h
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
index 6d18729..2c2f05a 100644
--- a/xen/common/gcov/gcov.c
+++ b/xen/common/gcov/gcov.c
@@ -22,6 +22,11 @@
#include <xen/guest_access.h>
#include <public/xen.h>
#include <public/gcov.h>
+#if defined(CONFIG_GCOV_FORMAT_3_4)
+#include "gcov_3_4.h"
+#else
+# error Gcov format not supported
+#endif
const char __initconst warning_gcov[] =
"WARNING: GCOV SUPPORT IS ENABLED\n"
@@ -69,10 +74,13 @@ void __gcov_merge_delta(gcov_type *counters, unsigned int n_counters)
/* Unused. */
}
-static inline int counter_active(const struct gcov_info *info, unsigned int type)
+#if defined(CONFIG_GCOV_FORMAT_3_4)
+static inline int counter_active(const struct gcov_info *info,
+ unsigned int type)
{
return (1 << type) & info->ctr_mask;
}
+#endif
typedef struct write_iter_t
{
@@ -122,6 +130,37 @@ static inline void align_iter(write_iter_t *iter)
(iter->write_offset + sizeof(uint64_t) - 1) & -sizeof(uint64_t);
}
+#if defined(CONFIG_GCOV_FORMAT_3_4)
+static int dump(write_iter_t *iter, const struct gcov_info *info)
+{
+ const struct gcov_ctr_info *ctr;
+ size_t size_fn = sizeof(struct gcov_fn_info);
+ int type;
+ int ret;
+
+ /* dump counters */
+ ctr = info->counts;
+ for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr )
+ {
+ align_iter(iter);
+ chk(write32(iter, XENCOV_TAG_COUNTER(type)));
+ chk(write32(iter, ctr->num));
+ chk(write_raw(iter, ctr->values,
+ ctr->num * sizeof(ctr->values[0])));
+
+ size_fn += sizeof(unsigned);
+ }
+
+ /* dump all functions together */
+ align_iter(iter);
+ chk(write32(iter, XENCOV_TAG_FUNC));
+ chk(write32(iter, info->n_functions));
+ chk(write_raw(iter, info->functions, info->n_functions * size_fn));
+
+ return 0;
+}
+#endif
+
static int write_gcov(write_iter_t *iter)
{
struct gcov_info *info;
@@ -133,34 +172,13 @@ static int write_gcov(write_iter_t *iter)
/* dump all files */
for ( info = info_list ; info; info = info->next )
{
- const struct gcov_ctr_info *ctr;
- int type;
- size_t size_fn = sizeof(struct gcov_fn_info);
-
align_iter(iter);
chk(write32(iter, XENCOV_TAG_FILE));
chk(write32(iter, info->version));
chk(write32(iter, info->stamp));
chk(write_string(iter, info->filename));
- /* dump counters */
- ctr = info->counts;
- for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr )
- {
- align_iter(iter);
- chk(write32(iter, XENCOV_TAG_COUNTER(type)));
- chk(write32(iter, ctr->num));
- chk(write_raw(iter, ctr->values,
- ctr->num * sizeof(ctr->values[0])));
-
- size_fn += sizeof(unsigned);
- }
-
- /* dump all functions together */
- align_iter(iter);
- chk(write32(iter, XENCOV_TAG_FUNC));
- chk(write32(iter, info->n_functions));
- chk(write_raw(iter, info->functions, info->n_functions * size_fn));
+ chk(dump(iter, info));
}
/* stop tag */
diff --git a/xen/common/gcov/gcov_3_4.h b/xen/common/gcov/gcov_3_4.h
new file mode 100644
index 0000000..e10c568
--- /dev/null
+++ b/xen/common/gcov/gcov_3_4.h
@@ -0,0 +1,84 @@
+/*
+ * Profiling infrastructure declarations.
+ *
+ * This file is based on gcc-internal definitions. Data structures are
+ * defined to be compatible with gcc counterparts. For a better
+ * understanding, refer to gcc source: gcc/gcov-io.h.
+ *
+ * Copyright IBM Corp. 2009
+ * Author(s): Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
+ *
+ * Uses gcc-internal data definitions.
+ */
+
+#ifndef __XEN_GCOV_3_4_H__
+#define __XEN_GCOV_3_4_H__
+
+/*
+ * Profiling data types used for gcc 3.4 and above - these are defined by
+ * gcc and need to be kept as close to the original definition as possible to
+ * remain compatible.
+ */
+
+typedef uint64_t gcov_type;
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @ident: object file-unique function identifier
+ * @checksum: function checksum
+ * @n_ctrs: number of values per counter type belonging to this function
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ */
+struct gcov_fn_info
+{
+ unsigned int ident;
+ unsigned int checksum;
+ unsigned int n_ctrs[0];
+};
+
+/**
+ * struct gcov_ctr_info - profiling data per counter type
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ * @merge: merge function for counter values of this type (unused)
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the values array.
+ */
+struct gcov_ctr_info
+{
+ unsigned int num;
+ gcov_type *values;
+ void (*merge)(gcov_type *, unsigned int);
+};
+
+/**
+ * struct gcov_info - profiling data per object file
+ * @version: gcov version magic indicating the gcc version used for compilation
+ * @next: list head for a singly-linked list
+ * @stamp: time stamp
+ * @filename: name of the associated gcov data file
+ * @n_functions: number of instrumented functions
+ * @functions: function data
+ * @ctr_mask: mask specifying which counter types are active
+ * @counts: counter data per counter type
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the next pointer.
+ */
+struct gcov_info
+{
+ unsigned int version;
+ struct gcov_info *next;
+ unsigned int stamp;
+ const char *filename;
+ unsigned int n_functions;
+ const struct gcov_fn_info *functions;
+ unsigned int ctr_mask;
+ struct gcov_ctr_info counts[0];
+};
+
+
+#endif /* __XEN_GCOV_3_4_H__ */
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
index a7d4a35..f6a1ac7 100644
--- a/xen/include/xen/gcov.h
+++ b/xen/include/xen/gcov.h
@@ -1,88 +1,8 @@
-/*
- * Profiling infrastructure declarations.
- *
- * This file is based on gcc-internal definitions. Data structures are
- * defined to be compatible with gcc counterparts. For a better
- * understanding, refer to gcc source: gcc/gcov-io.h.
- *
- * Copyright IBM Corp. 2009
- * Author(s): Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
- *
- * Uses gcc-internal data definitions.
- */
-
#ifndef __XEN_GCOV_H__
#define __XEN_GCOV_H__ __XEN_GCOV_H__
#include <public/sysctl.h>
-/*
- * Profiling data types used for gcc 3.4 and above - these are defined by
- * gcc and need to be kept as close to the original definition as possible to
- * remain compatible.
- */
-
-typedef uint64_t gcov_type;
-
-/**
- * struct gcov_fn_info - profiling meta data per function
- * @ident: object file-unique function identifier
- * @checksum: function checksum
- * @n_ctrs: number of values per counter type belonging to this function
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time.
- */
-struct gcov_fn_info
-{
- unsigned int ident;
- unsigned int checksum;
- unsigned int n_ctrs[0];
-};
-
-/**
- * struct gcov_ctr_info - profiling data per counter type
- * @num: number of counter values for this type
- * @values: array of counter values for this type
- * @merge: merge function for counter values of this type (unused)
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the values array.
- */
-struct gcov_ctr_info
-{
- unsigned int num;
- gcov_type *values;
- void (*merge)(gcov_type *, unsigned int);
-};
-
-/**
- * struct gcov_info - profiling data per object file
- * @version: gcov version magic indicating the gcc version used for compilation
- * @next: list head for a singly-linked list
- * @stamp: time stamp
- * @filename: name of the associated gcov data file
- * @n_functions: number of instrumented functions
- * @functions: function data
- * @ctr_mask: mask specifying which counter types are active
- * @counts: counter data per counter type
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the next pointer.
- */
-struct gcov_info
-{
- unsigned int version;
- struct gcov_info *next;
- unsigned int stamp;
- const char *filename;
- unsigned int n_functions;
- const struct gcov_fn_info *functions;
- unsigned int ctr_mask;
- struct gcov_ctr_info counts[0];
-};
-
-
/**
* Sysctl operations for coverage
*/
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 0/5] gcov: more cleanup
2016-09-02 11:47 [PATCH 0/5] gcov: more cleanup Wei Liu
` (4 preceding siblings ...)
2016-09-02 11:47 ` [PATCH 5/5] gcov: split out gcc version specific stuff Wei Liu
@ 2016-09-02 12:04 ` Andrew Cooper
2016-09-02 12:06 ` Wei Liu
5 siblings, 1 reply; 27+ messages in thread
From: Andrew Cooper @ 2016-09-02 12:04 UTC (permalink / raw)
To: Wei Liu, Xen-devel
On 02/09/16 12:47, Wei Liu wrote:
> This series further cleans up existing gcov code. It should be now clear that
> Xen's gcov is using gcc 3.4 format.
>
> I have tried to integrate gcc 4.7 format. The amount of work is moderate in
> terms of coding effort, but I now believe it is a mistake to have invented xen
> specific format which is in fact tied to gcc 3.4. I can't map gcc 4.7 format to
> Xen's own format, so I stop here.
Given that the interface has clearly bitrotten since it was first
introduced, I think it is reasonable to declare amnesty to fix it properly.
As the format is chosen at configure time, I think it is reasonable for
the Xen interface to just hand back blobs, and require the tools in dom0
to know how to interpret them.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/5] gcov: more cleanup
2016-09-02 12:04 ` [PATCH 0/5] gcov: more cleanup Andrew Cooper
@ 2016-09-02 12:06 ` Wei Liu
2016-09-02 12:15 ` Andrew Cooper
0 siblings, 1 reply; 27+ messages in thread
From: Wei Liu @ 2016-09-02 12:06 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, Wei Liu
On Fri, Sep 02, 2016 at 01:04:45PM +0100, Andrew Cooper wrote:
> On 02/09/16 12:47, Wei Liu wrote:
> > This series further cleans up existing gcov code. It should be now clear that
> > Xen's gcov is using gcc 3.4 format.
> >
> > I have tried to integrate gcc 4.7 format. The amount of work is moderate in
> > terms of coding effort, but I now believe it is a mistake to have invented xen
> > specific format which is in fact tied to gcc 3.4. I can't map gcc 4.7 format to
> > Xen's own format, so I stop here.
>
> Given that the interface has clearly bitrotten since it was first
> introduced, I think it is reasonable to declare amnesty to fix it properly.
>
> As the format is chosen at configure time, I think it is reasonable for
> the Xen interface to just hand back blobs, and require the tools in dom0
> to know how to interpret them.
>
Yes. I think that's how I would do it. But I don't have time for it now.
Wei.
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/5] gcov: more cleanup
2016-09-02 12:06 ` Wei Liu
@ 2016-09-02 12:15 ` Andrew Cooper
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Cooper @ 2016-09-02 12:15 UTC (permalink / raw)
To: Wei Liu; +Cc: Xen-devel
On 02/09/16 13:06, Wei Liu wrote:
> On Fri, Sep 02, 2016 at 01:04:45PM +0100, Andrew Cooper wrote:
>> On 02/09/16 12:47, Wei Liu wrote:
>>> This series further cleans up existing gcov code. It should be now clear that
>>> Xen's gcov is using gcc 3.4 format.
>>>
>>> I have tried to integrate gcc 4.7 format. The amount of work is moderate in
>>> terms of coding effort, but I now believe it is a mistake to have invented xen
>>> specific format which is in fact tied to gcc 3.4. I can't map gcc 4.7 format to
>>> Xen's own format, so I stop here.
>> Given that the interface has clearly bitrotten since it was first
>> introduced, I think it is reasonable to declare amnesty to fix it properly.
>>
>> As the format is chosen at configure time, I think it is reasonable for
>> the Xen interface to just hand back blobs, and require the tools in dom0
>> to know how to interpret them.
>>
> Yes. I think that's how I would do it. But I don't have time for it now.
That's perfectly ok. Thanks for getting this far.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread