[PATCH v2] xen: Fix incorrect taint constant

[PATCH v2] xen: Fix incorrect taint constant

[Andrew Cooper]

Insecure is the word being looked for here.  Especially given the nature of
the sole caller, and the (correct) comment next to it.

Also update the taint marker from 'U' to 'I' for consistency; this isn't
expected to impact anyone in practice.

Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>

v2:
 * Fix commmit message
 * Update comment ahead of print_tainted()
 * Change the emitted char from 'U' to 'I'
---
 xen/arch/arm/cpuerrata.c | 2 +-
 xen/common/kernel.c      | 4 ++--
 xen/include/xen/lib.h    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 1abacfe5bb67..d0658aedb6aa 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -695,7 +695,7 @@ void __init enable_errata_workarounds(void)
                     "**** Only trusted guests should be used.                             ****\n");
 
         /* Taint the machine has being insecure */
-        add_taint(TAINT_MACHINE_UNSECURE);
+        add_taint(TAINT_MACHINE_INSECURE);
     }
 #endif
 }
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index f7b1f65f373c..b8b845763d9d 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -343,8 +343,8 @@ unsigned int tainted;
  *  'C' - Console output is synchronous.
  *  'E' - An error (e.g. a machine check exceptions) has been injected.
  *  'H' - HVM forced emulation prefix is permitted.
+ *  'I' - Platform is insecure (usually due to an errata on the platform).
  *  'M' - Machine had a machine check experience.
- *  'U' - Platform is unsecure (usually due to an errata on the platform).
  *  'S' - Out of spec CPU (One core has a feature incompatible with others).
  *
  *      The string is overwritten by the next call to print_taint().
@@ -354,7 +354,7 @@ char *print_tainted(char *str)
     if ( tainted )
     {
         snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c%c",
-                 tainted & TAINT_MACHINE_UNSECURE ? 'U' : ' ',
+                 tainted & TAINT_MACHINE_INSECURE ? 'I' : ' ',
                  tainted & TAINT_MACHINE_CHECK ? 'M' : ' ',
                  tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ',
                  tainted & TAINT_ERROR_INJECT ? 'E' : ' ',
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index e914ccade095..75ae7489b9f0 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -201,7 +201,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_MACHINE_UNSECURE          (1u << 4)
+#define TAINT_MACHINE_INSECURE          (1u << 4)
 #define TAINT_CPU_OUT_OF_SPEC           (1u << 5)
 extern unsigned int tainted;
 #define TAINT_STRING_MAX_LEN            20

base-commit: 67fdffef9246c82cecd8db28838ed09e79e2528a
-- 
2.30.2

Re: [PATCH v2] xen: Fix incorrect taint constant

[Bertrand Marquis]

Hi Andrew,

> On 5 Jun 2023, at 13:23, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> Insecure is the word being looked for here.  Especially given the nature of
> the sole caller, and the (correct) comment next to it.
> 
> Also update the taint marker from 'U' to 'I' for consistency; this isn't
> expected to impact anyone in practice.
> 
> Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

Re: [PATCH v2] xen: Fix incorrect taint constant

[Jan Beulich]

On 05.06.2023 13:23, Andrew Cooper wrote:
> v2:
>  * Fix commmit message
>  * Update comment ahead of print_tainted()
>  * Change the emitted char from 'U' to 'I'

Just to mention it: With this additional change I'm no longer
considering this applicable to stable trees. I realize that one could
argue that by taking it we could accelerate learning that we broke
someone (if anyone), but I'm not inclined to follow this line of
thinking. If Arm maintainers (seeing that only Arm uses this right
now) think they want the change nevertheless, I'd include it,
though.

Jan

Re: [PATCH v2] xen: Fix incorrect taint constant

[Bertrand Marquis]

Hi Jan,

> On 7 Jun 2023, at 10:10, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 05.06.2023 13:23, Andrew Cooper wrote:
>> v2:
>> * Fix commmit message
>> * Update comment ahead of print_tainted()
>> * Change the emitted char from 'U' to 'I'
> 
> Just to mention it: With this additional change I'm no longer
> considering this applicable to stable trees. I realize that one could
> argue that by taking it we could accelerate learning that we broke
> someone (if anyone), but I'm not inclined to follow this line of
> thinking. If Arm maintainers (seeing that only Arm uses this right
> now) think they want the change nevertheless, I'd include it,
> though.
> 

Not sure if it is what you mean here, but this is not a candidate for 
backports definitely.

Or do you mean something else here ?

Bertrand

> Jan

Re: [PATCH v2] xen: Fix incorrect taint constant

[Andrew Cooper]

On 07/06/2023 9:10 am, Jan Beulich wrote:
> On 05.06.2023 13:23, Andrew Cooper wrote:
>> v2:
>>  * Fix commmit message
>>  * Update comment ahead of print_tainted()
>>  * Change the emitted char from 'U' to 'I'
> Just to mention it: With this additional change I'm no longer
> considering this applicable to stable trees. I realize that one could
> argue that by taking it we could accelerate learning that we broke
> someone (if anyone), but I'm not inclined to follow this line of
> thinking. If Arm maintainers (seeing that only Arm uses this right
> now) think they want the change nevertheless, I'd include it,
> though.

I'm not overly fussed.  It's not hard to rebase around.

~Andrew

Re: [PATCH v2] xen: Fix incorrect taint constant

[Jan Beulich]

On 07.06.2023 10:20, Bertrand Marquis wrote:
> Hi Jan,
> 
>> On 7 Jun 2023, at 10:10, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 05.06.2023 13:23, Andrew Cooper wrote:
>>> v2:
>>> * Fix commmit message
>>> * Update comment ahead of print_tainted()
>>> * Change the emitted char from 'U' to 'I'
>>
>> Just to mention it: With this additional change I'm no longer
>> considering this applicable to stable trees. I realize that one could
>> argue that by taking it we could accelerate learning that we broke
>> someone (if anyone), but I'm not inclined to follow this line of
>> thinking. If Arm maintainers (seeing that only Arm uses this right
>> now) think they want the change nevertheless, I'd include it,
>> though.
>>
> 
> Not sure if it is what you mean here, but this is not a candidate for 
> backports definitely.
> 
> Or do you mean something else here ?

No, you got me right - I was asking whether to backport this.

Jan

Re: [PATCH v2] xen: Fix incorrect taint constant

[Bertrand Marquis]

Hi Andrew,

> On 7 Jun 2023, at 10:27, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> On 07/06/2023 9:10 am, Jan Beulich wrote:
>> On 05.06.2023 13:23, Andrew Cooper wrote:
>>> v2:
>>> * Fix commmit message
>>> * Update comment ahead of print_tainted()
>>> * Change the emitted char from 'U' to 'I'
>> Just to mention it: With this additional change I'm no longer
>> considering this applicable to stable trees. I realize that one could
>> argue that by taking it we could accelerate learning that we broke
>> someone (if anyone), but I'm not inclined to follow this line of
>> thinking. If Arm maintainers (seeing that only Arm uses this right
>> now) think they want the change nevertheless, I'd include it,
>> though.
> 
> I'm not overly fussed.  It's not hard to rebase around.

I agree with Jan here and I do not think we should backport.

This is not fixing a bug and could induce some misunderstanding if
we backport this change on a stable version.

Cheers
Bertrand

> 
> ~Andrew