[v4] xen: introduce CONFIG_SYSCTL

[PATCH v4 17/20] xen/sysctl: make CONFIG_COVERAGE depend on CONFIG_SYSCTL

Posted by Penny Zheng 5 months ago

All coverage-related op shall be wrapped around with CONFIG_SYSCTL,
so we shall make CONFIG_COVERAGE depend on CONFIG_SYSCTL.

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v1 -> v2:
- commit message refactor
---
v3 -> v4:
- commit message refactor
---
 xen/Kconfig.debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index d14093017e..c4198f0ac8 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -37,7 +37,7 @@ config SELF_TESTS
 
 config COVERAGE
 	bool "Code coverage support"
-	depends on !LIVEPATCH
+	depends on !LIVEPATCH && SYSCTL
 	select SUPPRESS_DUPLICATE_SYMBOL_WARNINGS if !ENFORCE_UNIQUE_SYMBOLS
 	help
 	  Enable code coverage support.
-- 
2.34.1

Re: [PATCH v4 17/20] xen/sysctl: make CONFIG_COVERAGE depend on CONFIG_SYSCTL

Posted by Jan Beulich 4 months, 3 weeks ago

On 28.05.2025 11:17, Penny Zheng wrote:
> All coverage-related op shall be wrapped around with CONFIG_SYSCTL,
> so we shall make CONFIG_COVERAGE depend on CONFIG_SYSCTL.
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> v1 -> v2:
> - commit message refactor
> ---
> v3 -> v4:
> - commit message refactor

Despite these efforts the description still fails to say _why_ the
supposed wrapping is needed. And "supposed" because I can't really
see any wrapping. All you do is add a dependency.

And btw ...

> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -37,7 +37,7 @@ config SELF_TESTS
>  
>  config COVERAGE
>  	bool "Code coverage support"
> -	depends on !LIVEPATCH
> +	depends on !LIVEPATCH && SYSCTL

... "depends on SYSCTL && !LIVEPATCH" would feel more naturally to
me.

Jan

RE: [PATCH v4 17/20] xen/sysctl: make CONFIG_COVERAGE depend on CONFIG_SYSCTL

Posted by Penny, Zheng 4 months, 2 weeks ago

[Public]

Hi

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Tuesday, June 10, 2025 9:48 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Anthony PERARD <anthony.perard@vates.tech>;
> Orzel, Michal <Michal.Orzel@amd.com>; Julien Grall <julien@xen.org>; Roger Pau
> Monné <roger.pau@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen-
> devel@lists.xenproject.org
> Subject: Re: [PATCH v4 17/20] xen/sysctl: make CONFIG_COVERAGE depend
> on CONFIG_SYSCTL
>
> On 28.05.2025 11:17, Penny Zheng wrote:
> > All coverage-related op shall be wrapped around with CONFIG_SYSCTL, so
> > we shall make CONFIG_COVERAGE depend on CONFIG_SYSCTL.
> >
> > Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> > ---
> > v1 -> v2:
> > - commit message refactor
> > ---
> > v3 -> v4:
> > - commit message refactor
>
> Despite these efforts the description still fails to say _why_ the supposed wrapping
> is needed. And "supposed" because I can't really see any wrapping. All you do is
> add a dependency.
>

Will change it to
"
Users rely on SYSCTL_coverage_op hypercall to interact with the coverage data,
that is, according operations shall be wrapped around with CONFIG_SYSCTL.
Right now, it is compiled under CONFIG_COVERAGE, so we shall make
CONFIG_COVERAGE depend on CONFIG_SYSCTL
"

> And btw ...
>
> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -37,7 +37,7 @@ config SELF_TESTS
> >
> >  config COVERAGE
> >     bool "Code coverage support"
> > -   depends on !LIVEPATCH
> > +   depends on !LIVEPATCH && SYSCTL
>
> ... "depends on SYSCTL && !LIVEPATCH" would feel more naturally to me.

Understood

>
> Jan