| Message ID | 20230605112303.1764183-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] xen: Fix incorrect taint constant | expand |
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
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
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
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
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
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
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
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(-) base-commit: 67fdffef9246c82cecd8db28838ed09e79e2528a