[LTP] [PATCH v5 1/4] lib/tst_kconfig.c: add any kconfig with or without expected value function

[LTP] [PATCH v5 1/4] lib/tst_kconfig.c: add any kconfig with or without expected value function

[Pengfei Xu]

Example: CONFIG_X86_INTEL_UMIP=y for umip kconfig before and v5.4
           mainline kernel.
         CONFIG_X86_UMIP=y for umip kconfig after v5.5 mainline kernel.
Format: CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y
        CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP|NA

Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
---
 lib/tst_kconfig.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
index 4b51413e5..ac553b91e 100644
--- a/lib/tst_kconfig.c
+++ b/lib/tst_kconfig.c
@@ -167,7 +167,8 @@ void tst_kconfig_read(const char *const *kconfigs,
 	struct match matches[cnt];
 	FILE *fp;
 	unsigned int i, j;
-	char buf[1024];
+	char buf[1024], kconfig_multi[100];
+	char *kconfig_token = NULL, *p_left = NULL;
 
 	for (i = 0; i < cnt; i++) {
 		const char *val = strchr(kconfigs[i], '=');
@@ -176,12 +177,9 @@ void tst_kconfig_read(const char *const *kconfigs,
 			tst_brk(TBROK, "Invalid config string '%s'", kconfigs[i]);
 
 		matches[i].match = 0;
-		matches[i].len = strlen(kconfigs[i]);
 
-		if (val) {
+		if (val)
 			matches[i].val = val + 1;
-			matches[i].len -= strlen(val);
-		}
 
 		results[i].match = 0;
 		results[i].value = NULL;
@@ -193,17 +191,29 @@ void tst_kconfig_read(const char *const *kconfigs,
 
 	while (fgets(buf, sizeof(buf), fp)) {
 		for (i = 0; i < cnt; i++) {
-			if (match(&matches[i], kconfigs[i], &results[i], buf)) {
-				for (j = 0; j < cnt; j++) {
-					if (matches[j].match)
-						break;
+			memset(kconfig_multi, 0, sizeof(kconfig_multi));
+			/* strtok_r will split kconfigs[i] to multi string, so copy it */
+			memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
+			kconfig_token = strtok_r(kconfig_multi, "|=", &p_left);
+
+			while (kconfig_token != NULL) {
+				if (strncmp("CONFIG_", kconfig_token, 7))
+					tst_brk(TBROK, "Invalid config string '%s'", kconfig_token);
+				matches[i].len = strlen(kconfig_token);
+				if (match(&matches[i], kconfig_token, &results[i], buf)) {
+					for (j = 0; j < cnt; j++) {
+						if (matches[j].match)
+							break;
+					}
+					if (j == cnt)
+						goto exit;
 				}
-
-				if (j == cnt)
-					goto exit;
+				kconfig_token = strtok_r(NULL, "|=", &p_left);
+				/* avoid to use after "=" string */
+				if (strlen(p_left) == 0)
+					break;
 			}
 		}
-
 	}
 
 exit:
-- 
2.14.1

[LTP] [PATCH v5 2/4] lib: add any kconfig with or without expected value into kconfig test

[Pengfei Xu]

config01/02/03/04 should be passed for UMIP kconfig
All cases in config05 should be failed.

Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
---
 lib/newlib_tests/config01       | 1 +
 lib/newlib_tests/config02       | 1 +
 lib/newlib_tests/config03       | 1 +
 lib/newlib_tests/config04       | 1 +
 lib/newlib_tests/config05       | 4 ++++
 lib/newlib_tests/test_kconfig.c | 5 +++++
 6 files changed, 13 insertions(+)

diff --git a/lib/newlib_tests/config01 b/lib/newlib_tests/config01
index 96d68d836..085c9368c 100644
--- a/lib/newlib_tests/config01
+++ b/lib/newlib_tests/config01
@@ -2,3 +2,4 @@
 CONFIG_MMU=y
 CONFIG_EXT4_FS=m
 CONFIG_PGTABLE_LEVELS=4
+CONFIG_X86_UMIP=y
diff --git a/lib/newlib_tests/config02 b/lib/newlib_tests/config02
index 2de45cff8..ca71d26c1 100644
--- a/lib/newlib_tests/config02
+++ b/lib/newlib_tests/config02
@@ -2,3 +2,4 @@
 # CONFIG_MMU is not set
 CONFIG_EXT4_FS=m
 CONFIG_PGTABLE_LEVELS=4
+CONFIG_X86_INTEL_UMIP=y
diff --git a/lib/newlib_tests/config03 b/lib/newlib_tests/config03
index 1a3b9e648..8a92def74 100644
--- a/lib/newlib_tests/config03
+++ b/lib/newlib_tests/config03
@@ -2,3 +2,4 @@
 CONFIG_MMU=y
 CONFIG_EXT4_FS=m
 CONFIG_PGTABLE_LEVELS=44
+CONFIG_X86_UMIP=y
diff --git a/lib/newlib_tests/config04 b/lib/newlib_tests/config04
index cce7051ae..424157fec 100644
--- a/lib/newlib_tests/config04
+++ b/lib/newlib_tests/config04
@@ -1,4 +1,5 @@
 # Unexpected CONFIG_EXT4_FS compiled in
 CONFIG_MMU=y
 CONFIG_EXT4_FS=y
+CONFIG_X86_INTEL_UMIP=y
 CONFIG_PGTABLE_LEVELS=4
diff --git a/lib/newlib_tests/config05 b/lib/newlib_tests/config05
index a9d7bab4d..85c8ad22f 100644
--- a/lib/newlib_tests/config05
+++ b/lib/newlib_tests/config05
@@ -1,3 +1,7 @@
 # Everything is wrong
 CONFIG_EXT4_FS=y
 CONFIG_PGTABLE_LEVELS=44
+CONFIG_X86_UMI=y
+CONFIG_X86_UMIPP=y
+CONFIG_X86_INTEL_UMI=y
+CONFIG_X86_INTEL_UMIPP=y
diff --git a/lib/newlib_tests/test_kconfig.c b/lib/newlib_tests/test_kconfig.c
index d9c662fc5..d3a1c2b0b 100644
--- a/lib/newlib_tests/test_kconfig.c
+++ b/lib/newlib_tests/test_kconfig.c
@@ -12,8 +12,13 @@ static void do_test(void)
 
 static const char *kconfigs[] = {
 	"CONFIG_MMU",
+	/* one CONFIG_A without expected value, |NA as the end is optional */
+	"CONFIG_MMU|NA",
 	"CONFIG_EXT4_FS=m",
 	"CONFIG_PGTABLE_LEVELS=4",
+	"CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",
+	/* CONFIG_A|CONFIG_B without expected value, need to add |NA as the end */
+	"CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP|NA",
 	NULL
 };
 
-- 
2.14.1

[LTP] [PATCH v5 3/4] lib: add usage that any kconfig with or without expected value in document

[Pengfei Xu]

Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
---
 doc/test-writing-guidelines.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 79d857fea..2299b6982 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -1590,7 +1590,13 @@ aborted with 'TCONF' if any of the required options were not set.
 #include "tst_test.h"
 
 static const char *kconfigs[] = {
-	"CONFIG_X86_INTEL_UMIP",
+	"CONFIG_EXT4_FS=y",
+	"CONFIG_MMU",
+	/* one CONFIG_A without expected value, add |NA as the end is optional */
+	"CONFIG_MMU|NA",
+	"CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",
+	/* CONFIG_A|CONFIG_B without expected value, need to add |NA as the end */
+	"CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP|NA",
 	NULL
 };
 
-- 
2.14.1

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Pengfei Xu]

From v5.5 main line, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
to "CONFIG_X86_UMIP=y".
We could use "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" to check kconfig
CONFIG_X86_INTEL_UMIP=y(old kernel) or CONFIG_X86_UMIP=y(new kernel) for umip.

Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
---
 testcases/kernel/security/umip/umip_basic_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/security/umip/umip_basic_test.c b/testcases/kernel/security/umip/umip_basic_test.c
index 1baa26c52..24eca8890 100644
--- a/testcases/kernel/security/umip/umip_basic_test.c
+++ b/testcases/kernel/security/umip/umip_basic_test.c
@@ -171,7 +171,7 @@ static struct tst_test test = {
 	.forks_child = 1,
 	.test = verify_umip_instruction,
 	.needs_kconfigs = (const char *[]){
-		"CONFIG_X86_INTEL_UMIP=y",
+		"CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",
 		NULL
 	},
 	.needs_root = 1,
-- 
2.14.1

[LTP] [PATCH v5 1/4] lib/tst_kconfig.c: add any kconfig with or without expected value function

[Cyril Hrubis]

Hi!
>  	for (i = 0; i < cnt; i++) {
>  		const char *val = strchr(kconfigs[i], '=');
> @@ -176,12 +177,9 @@ void tst_kconfig_read(const char *const *kconfigs,
>  			tst_brk(TBROK, "Invalid config string '%s'", kconfigs[i]);
>  
>  		matches[i].match = 0;
> -		matches[i].len = strlen(kconfigs[i]);
>  
> -		if (val) {
> +		if (val)
>  			matches[i].val = val + 1;
> -			matches[i].len -= strlen(val);
> -		}
>  
>  		results[i].match = 0;
>  		results[i].value = NULL;
> @@ -193,17 +191,29 @@ void tst_kconfig_read(const char *const *kconfigs,
>  
>  	while (fgets(buf, sizeof(buf), fp)) {
>  		for (i = 0; i < cnt; i++) {
> -			if (match(&matches[i], kconfigs[i], &results[i], buf)) {
> -				for (j = 0; j < cnt; j++) {
> -					if (matches[j].match)
> -						break;
> +			memset(kconfig_multi, 0, sizeof(kconfig_multi));
> +			/* strtok_r will split kconfigs[i] to multi string, so copy it */
> +			memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
> +			kconfig_token = strtok_r(kconfig_multi, "|=", &p_left);
> +
> +			while (kconfig_token != NULL) {
> +				if (strncmp("CONFIG_", kconfig_token, 7))
> +					tst_brk(TBROK, "Invalid config string '%s'", kconfig_token);
> +				matches[i].len = strlen(kconfig_token);
> +				if (match(&matches[i], kconfig_token, &results[i], buf)) {
> +					for (j = 0; j < cnt; j++) {
> +						if (matches[j].match)
> +							break;
> +					}
> +					if (j == cnt)
> +						goto exit;

I do not think that this actually works correctly. One of the problems I
see is that we do match only the CONFIG_FOO part in the
tst_kconfig_read() and the result value is evaluated later on. This
means that if we had something as "CONFIG_FOO=5|CONFIG_FOO=4" the code
will pick up only the first occurence of the = and we would end up doing
strcmp("4", "5|CONFIG_FOO=4") which would fail as well.

If we wanted to have proper solution for logic statements inside of the
kconfig parser we would have to isolate the CONFIG_FOO names first, pass
them to the tst_kconfig_read() function, that would get us values for
all config variables we need, then we could split the configs strings
greadily on | and evaluate them one after another.

So the first function would have to be able to get arrays of strings and
return another array of strings isolating the CONFIG_FOO variables. That
would be passed to tst_kconfig_read() that would yield results[] array,
for all interesting variables. From that point we can split the kconfig
strings by | and evaluate one after another until we get match or end of
the string.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5 1/4] lib/tst_kconfig.c: add any kconfig with or without expected value function

[Pengfei Xu]

Hi Cyril,
  Thanks for advice!
  I only support the CONFIG_A|CONFIG_B=X function, and there is only 1 '='
  contained in 1 line kconfig check.
  Could we add the function first, only support CONFIG_A|CONFIG_B=X function?

  Next to consider the function for CONFIG_A=X|CONFIG_B=Y.
  And I tried to add the function you mentioned in tst_kconfig.c,
  found CONFIG_A=X|CONFIG_B=Y, if kconfig set as CONFIG_A=Y, it will meet
  judge correctly issue, there is no kconfig item string in tst_kconfig_res.
struct tst_kconfig_res {
	char match;  // match char like y|m|v
	char *value; // *value like 4|44
};

  Or shall we need change the kconfig verify way?

  Thanks!
  BR.

On 2020-01-29 at 17:19:57 +0100, Cyril Hrubis wrote:
> Hi!
> >  	for (i = 0; i < cnt; i++) {
> >  		const char *val = strchr(kconfigs[i], '=');
> > @@ -176,12 +177,9 @@ void tst_kconfig_read(const char *const *kconfigs,
> >  			tst_brk(TBROK, "Invalid config string '%s'", kconfigs[i]);
> >  
> >  		matches[i].match = 0;
> > -		matches[i].len = strlen(kconfigs[i]);
> >  
> > -		if (val) {
> > +		if (val)
> >  			matches[i].val = val + 1;
> > -			matches[i].len -= strlen(val);
> > -		}
> >  
> >  		results[i].match = 0;
> >  		results[i].value = NULL;
> > @@ -193,17 +191,29 @@ void tst_kconfig_read(const char *const *kconfigs,
> >  
> >  	while (fgets(buf, sizeof(buf), fp)) {
> >  		for (i = 0; i < cnt; i++) {
> > -			if (match(&matches[i], kconfigs[i], &results[i], buf)) {
> > -				for (j = 0; j < cnt; j++) {
> > -					if (matches[j].match)
> > -						break;
> > +			memset(kconfig_multi, 0, sizeof(kconfig_multi));
> > +			/* strtok_r will split kconfigs[i] to multi string, so copy it */
> > +			memcpy(kconfig_multi, kconfigs[i], strlen(kconfigs[i]));
> > +			kconfig_token = strtok_r(kconfig_multi, "|=", &p_left);
> > +
> > +			while (kconfig_token != NULL) {
> > +				if (strncmp("CONFIG_", kconfig_token, 7))
> > +					tst_brk(TBROK, "Invalid config string '%s'", kconfig_token);
> > +				matches[i].len = strlen(kconfig_token);
> > +				if (match(&matches[i], kconfig_token, &results[i], buf)) {
> > +					for (j = 0; j < cnt; j++) {
> > +						if (matches[j].match)
> > +							break;
> > +					}
> > +					if (j == cnt)
> > +						goto exit;
> 
> I do not think that this actually works correctly. One of the problems I
> see is that we do match only the CONFIG_FOO part in the
> tst_kconfig_read() and the result value is evaluated later on. This
> means that if we had something as "CONFIG_FOO=5|CONFIG_FOO=4" the code
> will pick up only the first occurence of the = and we would end up doing
> strcmp("4", "5|CONFIG_FOO=4") which would fail as well.
> 
> If we wanted to have proper solution for logic statements inside of the
> kconfig parser we would have to isolate the CONFIG_FOO names first, pass
> them to the tst_kconfig_read() function, that would get us values for
> all config variables we need, then we could split the configs strings
> greadily on | and evaluate them one after another.
> 
> So the first function would have to be able to get arrays of strings and
> return another array of strings isolating the CONFIG_FOO variables. That
> would be passed to tst_kconfig_read() that would yield results[] array,
> for all interesting variables. From that point we can split the kconfig
> strings by | and evaluate one after another until we get match or end of
> the string.
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Petr Vorel]

Hi Xu,

> From v5.5 main line, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
> to "CONFIG_X86_UMIP=y".
> We could use "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" to check kconfig
> CONFIG_X86_INTEL_UMIP=y(old kernel) or CONFIG_X86_UMIP=y(new kernel) for umip.

> Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
> ---
>  testcases/kernel/security/umip/umip_basic_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/testcases/kernel/security/umip/umip_basic_test.c b/testcases/kernel/security/umip/umip_basic_test.c
> index 1baa26c52..24eca8890 100644
> --- a/testcases/kernel/security/umip/umip_basic_test.c
> +++ b/testcases/kernel/security/umip/umip_basic_test.c
> @@ -171,7 +171,7 @@ static struct tst_test test = {
>  	.forks_child = 1,
>  	.test = verify_umip_instruction,
>  	.needs_kconfigs = (const char *[]){
> -		"CONFIG_X86_INTEL_UMIP=y",
> +		"CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",

We're sorry to get to your patch now, after 5 months.

Thanks for a report and your effort to fix the problem. But this does not work,
because current implementation does not support '|' as bitwise or, with this
patch will result on tests being skipped for both cases.

While it'd be easy to implement support for bitwise or in tst_kconfig_read(),
it might be enough just to check for kernel version:

.needs_kconfigs = (const char *[]){
#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 5, 0)
	"CONFIG_X86_INTEL_UMIP=y",
#else
	"CONFIG_X86_UMIP=y",
#endif

But that will work unless this feature is not backported (IMHO commit
b971880fe79f ("x86/Kconfig: Rename UMIP config parameter") is kind of cleanup,
therefore unlikely to be backported, but it can happen).

Kind regards,
Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Pengfei Xu]

Hi Petr,
  Thanks and my feedback is as below.

  Thanks.
  BR.

On 2020-05-25 at 23:24:01 +0200, Petr Vorel wrote:
> Hi Xu,
> 
> > From v5.5 main line, umip kconfig changed from "CONFIG_X86_INTEL_UMIP=y"
> > to "CONFIG_X86_UMIP=y".
> > We could use "CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y" to check kconfig
> > CONFIG_X86_INTEL_UMIP=y(old kernel) or CONFIG_X86_UMIP=y(new kernel) for umip.
> 
> > Signed-off-by: Pengfei Xu <pengfei.xu@intel.com>
> > ---
> >  testcases/kernel/security/umip/umip_basic_test.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> > diff --git a/testcases/kernel/security/umip/umip_basic_test.c b/testcases/kernel/security/umip/umip_basic_test.c
> > index 1baa26c52..24eca8890 100644
> > --- a/testcases/kernel/security/umip/umip_basic_test.c
> > +++ b/testcases/kernel/security/umip/umip_basic_test.c
> > @@ -171,7 +171,7 @@ static struct tst_test test = {
> >  	.forks_child = 1,
> >  	.test = verify_umip_instruction,
> >  	.needs_kconfigs = (const char *[]){
> > -		"CONFIG_X86_INTEL_UMIP=y",
> > +		"CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y",
> 
> We're sorry to get to your patch now, after 5 months.
> 
> Thanks for a report and your effort to fix the problem. But this does not work,
> because current implementation does not support '|' as bitwise or, with this
> patch will result on tests being skipped for both cases.
  CONFIG_A|CONFIG_B=y means CONGIG_A or CONGIG_B equal 'y', it will meet the
  test condition. So it's as expected; only could not find CONFIG_A and
  CONFIG_B equal to 'y', then it will not meet the test condition and exit.
  It should be as expected.
  Thank you for considering this patch again.

> 
> While it'd be easy to implement support for bitwise or in tst_kconfig_read(),
> it might be enough just to check for kernel version:
> 
> .needs_kconfigs = (const char *[]){
> #if LINUX_VERSION_CODE < KERNEL_VERSION(5, 5, 0)
> 	"CONFIG_X86_INTEL_UMIP=y",
> #else
> 	"CONFIG_X86_UMIP=y",
> #endif
> 
> But that will work unless this feature is not backported (IMHO commit
> b971880fe79f ("x86/Kconfig: Rename UMIP config parameter") is kind of cleanup,
> therefore unlikely to be backported, but it can happen).
> 
> Kind regards,
> Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Petr Vorel]

Hi Xu,

...
> > Thanks for a report and your effort to fix the problem. But this does not work,
> > because current implementation does not support '|' as bitwise or, with this
> > patch will result on tests being skipped for both cases.
>   CONFIG_A|CONFIG_B=y means CONGIG_A or CONGIG_B equal 'y', it will meet the
>   test condition. So it's as expected; only could not find CONFIG_A and
>   CONFIG_B equal to 'y', then it will not meet the test condition and exit.
>   It should be as expected.
>   Thank you for considering this patch again.

Well, I understand your syntax, that you mean | as bitwise or :).
But where did you find that this syntax is supported? Have a look in
tst_kconfig_read() implementation (lib/tst_kconfig.c), there is nothing like
this. And, indeed, if you test your patch on both CONFIG_X86_INTEL_UMIP=y and
CONFIG_X86_UMIP=y, it end up with:

tst_kconfig.c:252: INFO: Missing kernel CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y
tst_kconfig.c:284: CONF: Aborting due to unsuitable kernel config, see above!

which confirm my statement there is no bitwise or support implemented :).
Or am I missing something?

And it might be questionable if CONFIG_A|CONFIG_B=y would mean (CONFIG_A|CONFIG_B)=y
or (CONFIG_A|CONFIG_B=y), thus it should be CONFIG_A=y|CONFIG_B=y

But given the fact this functionality is needed just for a single test and can
be workarounded I suggested to use LINUX_VERSION_CODE instead.
The only problem can happen when this is backported to the old code. But we
could also try to detect that with custom call twice tst_kconfig_read() in the
test setup.

Kind regards,
Petr

> > While it'd be easy to implement support for bitwise or in tst_kconfig_read(),
> > it might be enough just to check for kernel version:

> > .needs_kconfigs = (const char *[]) {
> > #if LINUX_VERSION_CODE < KERNEL_VERSION(5, 5, 0)
> > 	"CONFIG_X86_INTEL_UMIP=y",
> > #else
> > 	"CONFIG_X86_UMIP=y",
> > #endif

> > But that will work unless this feature is not backported (IMHO commit
> > b971880fe79f ("x86/Kconfig: Rename UMIP config parameter") is kind of cleanup,
> > therefore unlikely to be backported, but it can happen).

> > Kind regards,
> > Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Petr Vorel]

Hi Xu,

> ...
> > > Thanks for a report and your effort to fix the problem. But this does not work,
> > > because current implementation does not support '|' as bitwise or, with this
> > > patch will result on tests being skipped for both cases.
> >   CONFIG_A|CONFIG_B=y means CONGIG_A or CONGIG_B equal 'y', it will meet the
> >   test condition. So it's as expected; only could not find CONFIG_A and
> >   CONFIG_B equal to 'y', then it will not meet the test condition and exit.
> >   It should be as expected.
> >   Thank you for considering this patch again.

> Well, I understand your syntax, that you mean | as bitwise or :).
> But where did you find that this syntax is supported? Have a look in
> tst_kconfig_read() implementation (lib/tst_kconfig.c), there is nothing like
> this. And, indeed, if you test your patch on both CONFIG_X86_INTEL_UMIP=y and
> CONFIG_X86_UMIP=y, it end up with:

> tst_kconfig.c:252: INFO: Missing kernel CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y
> tst_kconfig.c:284: CONF: Aborting due to unsuitable kernel config, see above!

> which confirm my statement there is no bitwise or support implemented :).
> Or am I missing something?

OK I now realized, that it's a 4th patch of patchset. I thought it's just single
patch, because the rest was rejected by Cyril:
https://patchwork.ozlabs.org/project/ltp/list/?series=149804&state=*

But it looks like Cyril is not against the implementation, it just needs to be
fixed:
https://patchwork.ozlabs.org/comment/2352151/

Kind regards,
Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Pengfei Xu]

Hi Petr,

On 2020-05-26 at 11:27:03 +0200, Petr Vorel wrote:
> Hi Xu,
> 
> > ...
> > > > Thanks for a report and your effort to fix the problem. But this does not work,
> > > > because current implementation does not support '|' as bitwise or, with this
> > > > patch will result on tests being skipped for both cases.
> > >   CONFIG_A|CONFIG_B=y means CONGIG_A or CONGIG_B equal 'y', it will meet the
> > >   test condition. So it's as expected; only could not find CONFIG_A and
> > >   CONFIG_B equal to 'y', then it will not meet the test condition and exit.
> > >   It should be as expected.
> > >   Thank you for considering this patch again.
> 
> > Well, I understand your syntax, that you mean | as bitwise or :).
> > But where did you find that this syntax is supported? Have a look in
> > tst_kconfig_read() implementation (lib/tst_kconfig.c), there is nothing like
> > this. And, indeed, if you test your patch on both CONFIG_X86_INTEL_UMIP=y and
> > CONFIG_X86_UMIP=y, it end up with:
> 
> > tst_kconfig.c:252: INFO: Missing kernel CONFIG_X86_INTEL_UMIP|CONFIG_X86_UMIP=y
> > tst_kconfig.c:284: CONF: Aborting due to unsuitable kernel config, see above!
> 
> > which confirm my statement there is no bitwise or support implemented :).
> > Or am I missing something?
> 
> OK I now realized, that it's a 4th patch of patchset. I thought it's just single
> patch, because the rest was rejected by Cyril:
> https://patchwork.ozlabs.org/project/ltp/list/?series=149804&state=*
> 
> But it looks like Cyril is not against the implementation, it just needs to be
> fixed:
> https://patchwork.ozlabs.org/comment/2352151/

You are right, actually it could be worked as my suggest way:
"CONFIG_A|CONFIG_B=Y".
I tried to use Cyril's advice "CONFIG_A=X|CONFIG_B=Y" way, which will
add more code complexity, so I just want to solve the problem I am currently
facing.
If we really need the "CONFIG_A=X|CONFIG_B=Y" function, which cannot be
satisfied by "CONFIG_A|CONFIG_B=Y" function in the future, then we could add
this function I think.
Thanks for your considering.

BR
Thanks!

Pengfei

> 
> Kind regards,
> Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Petr Vorel]

Hi Pengfei,

> > But it looks like Cyril is not against the implementation, it just needs to be
> > fixed:
> > https://patchwork.ozlabs.org/comment/2352151/

> You are right, actually it could be worked as my suggest way:
> "CONFIG_A|CONFIG_B=Y".
> I tried to use Cyril's advice "CONFIG_A=X|CONFIG_B=Y" way, which will
> add more code complexity, so I just want to solve the problem I am currently
> facing.
> If we really need the "CONFIG_A=X|CONFIG_B=Y" function, which cannot be
> satisfied by "CONFIG_A|CONFIG_B=Y" function in the future, then we could add
> this function I think.
> Thanks for your considering.

I'd also think that we need "CONFIG_A=X|CONFIG_B=Y", because
"CONFIG_A|CONFIG_B=Y" is ambiguous (we support both CONFIG_FOO and
CONFIG_FOO=bar and this must stay even with |).

Will you send a patch for that or shell I fix it with LINUX_VERSION_CODE <
KERNEL_VERSION for now?

Kind regards,
Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Pengfei Xu]

Hi Petr,

On 2020-05-26 at 12:11:33 +0200, Petr Vorel wrote:
> Hi Pengfei,
> 
> > > But it looks like Cyril is not against the implementation, it just needs to be
> > > fixed:
> > > https://patchwork.ozlabs.org/comment/2352151/
> 
> > You are right, actually it could be worked as my suggest way:
> > "CONFIG_A|CONFIG_B=Y".
> > I tried to use Cyril's advice "CONFIG_A=X|CONFIG_B=Y" way, which will
> > add more code complexity, so I just want to solve the problem I am currently
> > facing.
> > If we really need the "CONFIG_A=X|CONFIG_B=Y" function, which cannot be
> > satisfied by "CONFIG_A|CONFIG_B=Y" function in the future, then we could add
> > this function I think.
> > Thanks for your considering.
> 
> I'd also think that we need "CONFIG_A=X|CONFIG_B=Y", because
> "CONFIG_A|CONFIG_B=Y" is ambiguous (we support both CONFIG_FOO and
> CONFIG_FOO=bar and this must stay even with |).
> 
> Will you send a patch for that or shell I fix it with LINUX_VERSION_CODE <
> KERNEL_VERSION for now?

  Ok, thanks for your LINUX_VERSION way to fix this issue.
  Thanks!
  BR.

> 
> Kind regards,
> Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Xu, Pengfei]

Hi Petr,
  Seems LINUX_VERSION_CODE way it not suitable when test platform is not compiled platform.
  Need to use " if ((tst_kvercmp(5, 5, 0)) >= 0)" way.

Thanks!
BR
Pengfei
+86 021 61164364?? 



-----Original Message-----
From: Pengfei Xu <pengfei.xu@intel.com> 
Sent: Tuesday, May 26, 2020 18:37
To: Petr Vorel <pvorel@suse.cz>
Cc: ltp <ltp@lists.linux.it>; Neri, Ricardo <ricardo.neri@intel.com>; Su, Heng <heng.su@intel.com>; Kasten, Robert A <robert.a.kasten@intel.com>; Cyril Hrubis <chrubis@suse.cz>; Jan Stancek <jstancek@redhat.com>; Li Wang <liwang@redhat.com>
Subject: Re: [LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

Hi Petr,

On 2020-05-26 at 12:11:33 +0200, Petr Vorel wrote:
> Hi Pengfei,
> 
> > > But it looks like Cyril is not against the implementation, it just 
> > > needs to be
> > > fixed:
> > > https://patchwork.ozlabs.org/comment/2352151/
> 
> > You are right, actually it could be worked as my suggest way:
> > "CONFIG_A|CONFIG_B=Y".
> > I tried to use Cyril's advice "CONFIG_A=X|CONFIG_B=Y" way, which 
> > will add more code complexity, so I just want to solve the problem I 
> > am currently facing.
> > If we really need the "CONFIG_A=X|CONFIG_B=Y" function, which cannot 
> > be satisfied by "CONFIG_A|CONFIG_B=Y" function in the future, then 
> > we could add this function I think.
> > Thanks for your considering.
> 
> I'd also think that we need "CONFIG_A=X|CONFIG_B=Y", because 
> "CONFIG_A|CONFIG_B=Y" is ambiguous (we support both CONFIG_FOO and 
> CONFIG_FOO=bar and this must stay even with |).
> 
> Will you send a patch for that or shell I fix it with 
> LINUX_VERSION_CODE < KERNEL_VERSION for now?

  Ok, thanks for your LINUX_VERSION way to fix this issue.
  Thanks!
  BR.

> 
> Kind regards,
> Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Petr Vorel]

Hi Pengfei,

>   Seems LINUX_VERSION_CODE way it not suitable when test platform is not compiled platform.
Well, you're expected to have installed kernel headers for target platform, when
you cross compile. That's what embedded distros like buildroot or yocto do.
We already use constructs like this in the code.

>   Need to use " if ((tst_kvercmp(5, 5, 0)) >= 0)" way.
As you noticed in previous mail, this will not work, as it's code outside of the
function, so you cannot call any function.

Kind regards,
Petr

[LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

[Xu, Pengfei]

Hi Petr,
   Ok, thanks!

Thanks!
BR
Pengfei
+86 021 61164364?? 



-----Original Message-----
From: Petr Vorel <pvorel@suse.cz> 
Sent: Wednesday, May 27, 2020 14:27
To: Xu, Pengfei <pengfei.xu@intel.com>
Cc: ltp <ltp@lists.linux.it>; Neri, Ricardo <ricardo.neri@intel.com>; Su, Heng <heng.su@intel.com>; Kasten, Robert A <robert.a.kasten@intel.com>; Cyril Hrubis <chrubis@suse.cz>; Jan Stancek <jstancek@redhat.com>; Li Wang <liwang@redhat.com>
Subject: Re: [LTP] [PATCH v5 4/4] umip_basic_test.c: improve kconfig verification to avoid umip wrong abort case

Hi Pengfei,

>   Seems LINUX_VERSION_CODE way it not suitable when test platform is not compiled platform.
Well, you're expected to have installed kernel headers for target platform, when you cross compile. That's what embedded distros like buildroot or yocto do.
We already use constructs like this in the code.

>   Need to use " if ((tst_kvercmp(5, 5, 0)) >= 0)" way.
As you noticed in previous mail, this will not work, as it's code outside of the function, so you cannot call any function.

Kind regards,
Petr