diff mbox series

[2/2] docs/hypervisor-guide: Code Coverage

Message ID 1554303600-27933-3-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series docs: Improve coverage docs | expand

Commit Message

Andrew Cooper April 3, 2019, 3 p.m. UTC
During a discussion in person, it was identified that Coverage doesn't
currently work for ARM yet.  Also, there are a number of errors with the
existing coverage document.

Take the opportunity to rewrite it in RST, making it easier to follow for a
non-expert user.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@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>
CC: Julien Grall <julien.grall@arm.com>
CC: Artem Mygaiev <artem_mygaiev@epam.com>
---
 docs/hypervisor-guide/code-coverage.rst | 102 ++++++++++++++++++++++++++
 docs/hypervisor-guide/index.rst         |   2 +
 docs/misc/coverage.pandoc               | 124 --------------------------------
 3 files changed, 104 insertions(+), 124 deletions(-)
 create mode 100644 docs/hypervisor-guide/code-coverage.rst
 delete mode 100644 docs/misc/coverage.pandoc

Comments

Artem Mygaiev April 4, 2019, 12:07 p.m. UTC | #1
Hello Andrew

Minor note below.

On Wed, 2019-04-03 at 16:00 +0100, Andrew Cooper wrote:

[snip]

> +  For ARM builds, while Xen will compile with ``CONFIG_COVERAGE``
> enabled, the
> +  resulting binary will not successfully boot if it exceeds 2MB in
> size.
> +  Xen's early memory management code needs adjusting to resolve this
> issue.

Julien mentioned that on staging the binary is 1.5MB only so we must be
able to run it without changes to Xen mem management. I will ask
someone to try it today or tomorrow and get back with results

 -- Artem
Wei Liu April 4, 2019, 12:10 p.m. UTC | #2
On Wed, Apr 03, 2019 at 04:00:00PM +0100, Andrew Cooper wrote:
> During a discussion in person, it was identified that Coverage doesn't
> currently work for ARM yet.  Also, there are a number of errors with the
> existing coverage document.
> 
> Take the opportunity to rewrite it in RST, making it easier to follow for a
> non-expert user.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>

(with or without adjustment to the statement regarding ARM)
Julien Grall April 4, 2019, 12:29 p.m. UTC | #3
On 04/04/2019 13:07, Artem Mygaiev wrote:
> Hello Andrew

Hi,

> Minor note below.
> 
> On Wed, 2019-04-03 at 16:00 +0100, Andrew Cooper wrote:
> 
> [snip]
> 
>> +  For ARM builds, while Xen will compile with ``CONFIG_COVERAGE``
>> enabled, the
>> +  resulting binary will not successfully boot if it exceeds 2MB in
>> size.
>> +  Xen's early memory management code needs adjusting to resolve this
>> issue.
> 
> Julien mentioned that on staging the binary is 1.5MB only so we must be
> able to run it without changes to Xen mem management. I will ask
> someone to try it today or tomorrow and get back with results

I tried with a different .config and Xen will use 1.8MB. I have 
computed the size using (_start - _end).

This is dangerously close to 2MB and it might be possible be
possible that we reach the limit with another .config.

I think Andrew's statement is correct. It does not say the binary
will always be above 2MB but it might exceeds it. It only warns
that it may happen and fail to boot.

The boot failure may not be obvious for the user. I wrote the
patch below to fail when linking instead. We can consider it
for backporting as well:

commit 256528c97b4c265911d960073facd4fdffcc650a
Author: Julien Grall <julien.grall@arm.com>
Date:   Thu Apr 4 13:22:46 2019 +0100

    xen/arm: Check Xen size when linking
    
    The linker will happily link Xen if it is bigger than what we can handle
    (e.g 2MB). This will result to unexpected failure after boot.
    
    This unexpected failure can be prevented by forbidding linking if Xen is
    bigger than 2MB.
    
    Signed-off-by: Julien Grall <julien.grall@arm.com>

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 1e72906477..28dc2a6070 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -242,3 +242,4 @@ ASSERT( _end_boot - start <= PAGE_SIZE, "Boot code is larger than 4K")
  */
 ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is misaligned")
 ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
+ASSERT(_end - start < XEN_SLOT_SIZE, "Xen is larger than 2M")
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index bc89e84f4d..98d7765410 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -107,6 +107,7 @@
  *  Unused
  */
 
+#define XEN_SLOT_SIZE         MB(2)
 #define XEN_VIRT_START         _AT(vaddr_t,0x00200000)
 #define FIXMAP_ADDR(n)        (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE)
Andrew Cooper April 4, 2019, 12:46 p.m. UTC | #4
On 04/04/2019 13:29, Julien Grall wrote:
>
> On 04/04/2019 13:07, Artem Mygaiev wrote:
>> Hello Andrew
> Hi,
>
>> Minor note below.
>>
>> On Wed, 2019-04-03 at 16:00 +0100, Andrew Cooper wrote:
>>
>> [snip]
>>
>>> +  For ARM builds, while Xen will compile with ``CONFIG_COVERAGE``
>>> enabled, the
>>> +  resulting binary will not successfully boot if it exceeds 2MB in
>>> size.
>>> +  Xen's early memory management code needs adjusting to resolve this
>>> issue.
>> Julien mentioned that on staging the binary is 1.5MB only so we must be
>> able to run it without changes to Xen mem management. I will ask
>> someone to try it today or tomorrow and get back with results
> I tried with a different .config and Xen will use 1.8MB. I have 
> computed the size using (_start - _end).
>
> This is dangerously close to 2MB and it might be possible be
> possible that we reach the limit with another .config.
>
> I think Andrew's statement is correct. It does not say the binary
> will always be above 2MB but it might exceeds it. It only warns
> that it may happen and fail to boot.
>
> The boot failure may not be obvious for the user. I wrote the
> patch below to fail when linking instead. We can consider it
> for backporting as well:
>
> commit 256528c97b4c265911d960073facd4fdffcc650a
> Author: Julien Grall <julien.grall@arm.com>
> Date:   Thu Apr 4 13:22:46 2019 +0100
>
>     xen/arm: Check Xen size when linking
>     
>     The linker will happily link Xen if it is bigger than what we can handle
>     (e.g 2MB). This will result to unexpected failure after boot.
>     
>     This unexpected failure can be prevented by forbidding linking if Xen is
>     bigger than 2MB.
>     
>     Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 1e72906477..28dc2a6070 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -242,3 +242,4 @@ ASSERT( _end_boot - start <= PAGE_SIZE, "Boot code is larger than 4K")
>   */
>  ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is misaligned")
>  ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
> +ASSERT(_end - start < XEN_SLOT_SIZE, "Xen is larger than 2M")

I think you probably want an opencoded MB(2) here, or the comment will
become stale as soon as the other issues get fixed.  That way, someone
will be forced to adjust this comment when the limit is lifted.

Given a patch like this, I'll drop the warning from the docs.

As an aside, do you happen to know which versions of GCC are expected to
work.  Alternatively, shall I just say that ARM isn't supported, by
virtue of it never having actually been tested.

~Andrew
Julien Grall April 4, 2019, 1:26 p.m. UTC | #5
Hi,

On 04/04/2019 13:46, Andrew Cooper wrote:
> On 04/04/2019 13:29, Julien Grall wrote:
>>
>> On 04/04/2019 13:07, Artem Mygaiev wrote:
>>> Hello Andrew
>> Hi,
>>
>>> Minor note below.
>>>
>>> On Wed, 2019-04-03 at 16:00 +0100, Andrew Cooper wrote:
>>>
>>> [snip]
>>>
>>>> +  For ARM builds, while Xen will compile with ``CONFIG_COVERAGE``
>>>> enabled, the
>>>> +  resulting binary will not successfully boot if it exceeds 2MB in
>>>> size.
>>>> +  Xen's early memory management code needs adjusting to resolve this
>>>> issue.
>>> Julien mentioned that on staging the binary is 1.5MB only so we must be
>>> able to run it without changes to Xen mem management. I will ask
>>> someone to try it today or tomorrow and get back with results
>> I tried with a different .config and Xen will use 1.8MB. I have
>> computed the size using (_start - _end).
>>
>> This is dangerously close to 2MB and it might be possible be
>> possible that we reach the limit with another .config.
>>
>> I think Andrew's statement is correct. It does not say the binary
>> will always be above 2MB but it might exceeds it. It only warns
>> that it may happen and fail to boot.
>>
>> The boot failure may not be obvious for the user. I wrote the
>> patch below to fail when linking instead. We can consider it
>> for backporting as well:
>>
>> commit 256528c97b4c265911d960073facd4fdffcc650a
>> Author: Julien Grall <julien.grall@arm.com>
>> Date:   Thu Apr 4 13:22:46 2019 +0100
>>
>>      xen/arm: Check Xen size when linking
>>      
>>      The linker will happily link Xen if it is bigger than what we can handle
>>      (e.g 2MB). This will result to unexpected failure after boot.
>>      
>>      This unexpected failure can be prevented by forbidding linking if Xen is
>>      bigger than 2MB.
>>      
>>      Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
>> index 1e72906477..28dc2a6070 100644
>> --- a/xen/arch/arm/xen.lds.S
>> +++ b/xen/arch/arm/xen.lds.S
>> @@ -242,3 +242,4 @@ ASSERT( _end_boot - start <= PAGE_SIZE, "Boot code is larger than 4K")
>>    */
>>   ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is misaligned")
>>   ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
>> +ASSERT(_end - start < XEN_SLOT_SIZE, "Xen is larger than 2M")
> 
> I think you probably want an opencoded MB(2) here, or the comment will
> become stale as soon as the other issues get fixed.  That way, someone
> will be forced to adjust this comment when the limit is lifted.

I would prefer to make the message agnostic. This could potentially allow us to 
make XEN_SLOT_SIZE configurable.

> 
> Given a patch like this, I'll drop the warning from the docs.
> 
> As an aside, do you happen to know which versions of GCC are expected to
> work.  Alternatively, shall I just say that ARM isn't supported, by
> virtue of it never having actually been tested.

I have no idea. I haven't tried more than building GCOV.

It would be best if it is marked as unsupported until someone actually test it.

Cheers,
diff mbox series

Patch

diff --git a/docs/hypervisor-guide/code-coverage.rst b/docs/hypervisor-guide/code-coverage.rst
new file mode 100644
index 0000000..0b7181f
--- /dev/null
+++ b/docs/hypervisor-guide/code-coverage.rst
@@ -0,0 +1,102 @@ 
+Code Coverage
+=============
+
+Xen can be compiled with coverage support.  When configured, Xen will record
+the coverage of its own basic blocks.  Being a piece of system software rather
+than a userspace, it can't automatically write coverage out to the filesystem,
+so some extra steps are required to collect and process the data.
+
+
+Compiling Xen
+-------------
+
+Coverage support is dependent on the compiler and toolchain used.  However, as
+Xen isn't a userspace application, it can't use the compiler supplied library.
+Instead, Xen has to provide some parts of the implementation itself.
+
+For x86, coverage support was introduced with GCC 3.4 or later, and Clang 3.9
+or later, and Xen is compatible with these.  However, the compiler internal
+formats do change occasionally, and this may involve adjustments to Xen.
+While we do our best to keep up with these changes, Xen may not be compatible
+with bleeding edge compilers.
+
+To build with coverage support, enable ``CONFIG_COVERAGE`` in Kconfig.  The
+build system will automatically select the appropriate format based on the
+compiler in use.
+
+The resulting binary will record its own coverage while running.
+
+.. warning::
+
+  For ARM builds, while Xen will compile with ``CONFIG_COVERAGE`` enabled, the
+  resulting binary will not successfully boot if it exceeds 2MB in size.
+  Xen's early memory management code needs adjusting to resolve this issue.
+
+
+Accessing the raw coverage data
+-------------------------------
+
+The ``SYSCTL_coverage_op`` hypercall is used to interact with the coverage
+data.  A dom0 userspace helper, ``xenconv`` is provided as well, which thinly
+wraps this hypercall.
+
+The ``read`` subcommand can be used to obtain the raw coverage data::
+
+  [root@host ~]# xencov read > coverage.dat
+
+This is toolchain-specific data and needs to be fed back to the appropriate
+programs to post-process.
+
+Alternatively, the ``reset`` subcommand can be used reset all counters back to
+0::
+
+  [root@host ~]# xencov reset
+
+
+GCC coverage
+------------
+
+A build using GCC's coverage will result in ``*.gcno`` artefact for every
+object file.  The raw coverage data needs splitting to form the matching
+``*.gcda`` files.
+
+An example of how to view the data is as follows.  It additionally depends on
+presence of ``lcov`` which is a graphical frontend to ``gcov``.
+
+* Obtain the raw coverage data from the test host, and pull it back to the
+  build working tree.
+* Use ``xencov_split`` to extract the ``*.gcda`` files.  Note that full build
+  paths are used by the tools, so splitting needs to output relative to ``/``.
+* Use ``geninfo`` to post-process the raw data.
+* Use ``genhtml`` to render the results as HTML.
+* View the results in a browser.
+
+::
+
+  xen.git/xen$ ssh root@host xencov read > coverage.dat
+  xen.git/xen$ ../tools/xencov_split coverage.dat --output-dir=/
+  xen.git/xen$ geninfo . -o cov.info
+  xen.git/xen$ genhtml cov.info -o cov/
+  xen.git/xen$ $BROWSER cov/index.html
+
+Clang coverage
+--------------
+
+An example of how to view the data is as follows.
+
+* Obtain the raw coverage data from the test host, and pull it back to the
+  build working tree.
+* Use ``llvm-profdata`` to post-process the raw data.
+* Use ``llvm-cov show`` in combination with ``xen-syms`` from the build to
+  render the results as HTML.
+* View the results in a browser.
+
+::
+
+  xen.git/xen$ ssh root@host xencov read > xen.profraw
+  xen.git/xen$ llvm-profdata merge xen.profraw -o xen.profdata
+  xen.git/xen$ llvm-cov show -format=html -output-dir=cov/ xen-syms -instr-profile=xen.profdata
+  xen.git/xen$ $BROWSER cov/index.html
+
+Full documentation on Clang's coverage capabilities can be found at:
+https://clang.llvm.org/docs/SourceBasedCodeCoverage.html
diff --git a/docs/hypervisor-guide/index.rst b/docs/hypervisor-guide/index.rst
index 3e0cada..cbcae39 100644
--- a/docs/hypervisor-guide/index.rst
+++ b/docs/hypervisor-guide/index.rst
@@ -3,3 +3,5 @@  Hypervisor documentation
 
 .. toctree::
   :maxdepth: 2
+
+  code-coverage
diff --git a/docs/misc/coverage.pandoc b/docs/misc/coverage.pandoc
deleted file mode 100644
index 3554659..0000000
--- a/docs/misc/coverage.pandoc
+++ /dev/null
@@ -1,124 +0,0 @@ 
-# Coverage support for Xen
-
-Coverage support allows you to get coverage information from Xen execution.
-You can see how many times a line is executed.
-
-Some compilers have specific options that enable the collection of this
-information. Every basic block in the code will be instrumented by the compiler
-to compute these statistics. It should not be used in production as it slows
-down your hypervisor.
-
-# GCOV (GCC coverage)
-
-## Enable coverage
-
-Test coverage support can be turned on compiling Xen with the `CONFIG_COVERAGE`
-option set to `y`.
-
-Change your `.config` or run `make -C xen menuconfig`.
-
-## Extract coverage data
-
-To extract data you use a simple utility called `xencov`.
-It allows you to do 2 operations:
-
-* `xencov read` extract data
-* `xencov reset` reset all coverage counters
-
-Another utility (`xencov_split`) is used to split extracted data file into
-files needed by userspace tools.
-
-## Split coverage data
-
-Once you extracted data from Xen, it is time to create files which the coverage
-tools can understand. To do it you need to run `xencov_split` utility.
-
-The utility just takes an input file and splits the blob into gcc .gcda files
-in the same directory that you execute the script. As file names are generated
-relative to the current directory, it could be a good idea to run the script
-from `/` on your build machine.
-
-Code for splitting the blob is put in another utility for some reason:
-* It is simpler to maintain a high level script than a C program;
-* You don't need to execute on the Xen host so you just need to copy the file to
-  your development box (you usually need development files anyway).
-
-## Possible use
-
-**This section is just an example on how to use these tools!**
-
-This example assumes you compiled Xen from `~/xen-unstable` and installed into
-the host. **Consider that if you even recompile Xen you are not able to use
-blob extracted from xencov!**
-
-* Ensure the `lcov` package is installed
-* From the Xen host machine extract the coverage blob
-
-        cd /root
-        xencov read coverage.dat
-
-* Copy the extracted blob to your dev machine
-
-        cd ~
-        scp root@myhost:coverage.dat
-
-* Extract the coverage information
-
-        (cd / && xencov_split ~/coverage.dat)
-
-* Produce coverage html output
-
-        cd ~/xen-unstable
-        rm -rf cov.info cov
-        geninfo -o cov.info xen
-        mkdir cov
-        genhtml -o cov cov.info
-
-* See output in a browser
-
-        firefox cov/index.html
-
-# LLVM coverage
-
-## Enable coverage
-
-Coverage can be enabled using a Kconfig option, from the top-level directory
-use the following command to display the Kconfig menu:
-
-    make -C xen menuconfig clang=y
-
-The code coverage option can be found inside of the "Debugging Options"
-section. After enabling it just compile Xen as you would normally do:
-
-    make xen clang=y
-
-## Extract coverage data
-
-LLVM coverage can be extracted from the hypervisor using the `xencov` tool.
-The following actions are available:
-
-* `xencov read` extract data
-* `xencov reset` reset all coverage counters
-* `xencov read-reset` extract data and reset counters at the same time.
-
-## Possible use
-
-**This section is just an example on how to use these tools!**
-
-This example assumes you compiled Xen and copied the xen-syms file from
-xen/xen-syms into your current directory.
-
-* Extract the coverage data from Xen:
-
-    xencov read xen.profraw
-
-* Convert the data into a profile. Note that you can merge more than one
-  profraw file into a single profdata file.
-
-    llvm-profdata merge xen.profraw -o xen.profdata
-
-* Generate a HTML report of the code coverage:
-
-    llvm-cov show -format=html -output-dir=cov/ xen-syms -instr-profile=xen.profdata
-
-* Open cov/index.html with your browser in order to display the profile.