| Message ID | 20230605100512.1748007-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: Fix incorrect taint constant | expand |
On 05.06.2023 12:05, Andrew Cooper wrote: > Insecure the word being looked for here. Especially given the nature of the Nit: Missing "is"? > sole caller, and the (correct) comment next to it. > > I've left the taint constant as 'U' as it's a rather more user-visible. > > Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com> > I'm happy to change 'U' to 'I' if we think that no-one is going to be impacted > by it. I just wasn't sure. I agree with what you have done, i.e. leaving it as is. Jan
On 05/06/2023 11:10 am, Jan Beulich wrote: > On 05.06.2023 12:05, Andrew Cooper wrote: >> Insecure the word being looked for here. Especially given the nature of the > Nit: Missing "is"? Oops yes. > >> sole caller, and the (correct) comment next to it. >> >> I've left the taint constant as 'U' as it's a rather more user-visible. >> >> Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type") >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Acked-by: Jan Beulich <jbeulich@suse.com> Thanks, although I've got one extra hunk to add having just done the other half of the taint work. diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 14ce6b40ce06..ff67f00e41bb 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -344,7 +344,7 @@ unsigned int tainted; * 'E' - An error (e.g. a machine check exceptions) has been injected. * 'H' - HVM forced emulation prefix is permitted. * 'M' - Machine had a machine check experience. - * 'U' - Platform is unsecure (usually due to an errata on the platform). + * 'U' - Platform is insecure (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(). >> I'm happy to change 'U' to 'I' if we think that no-one is going to be impacted >> by it. I just wasn't sure. > I agree with what you have done, i.e. leaving it as is. Yeah, I assumed that was going to be the preferred route. ~Andrew
Hi Andrew, > On 5 Jun 2023, at 12:05, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > Insecure the word being looked for here. Especially given the nature of the > sole caller, and the (correct) comment next to it. Good finding. > > I've left the taint constant as 'U' as it's a rather more user-visible. I would vote to change the U in I here as it will make it more coherent with the doc after your added fix for it. > > Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.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> > > I'm happy to change 'U' to 'I' if we think that no-one is going to be impacted > by it. I just wasn't sure. Here i do not think many will be impacted so I would rather make this coherent. Cheers Bertrand > --- > xen/arch/arm/cpuerrata.c | 2 +- > xen/common/kernel.c | 2 +- > xen/include/xen/lib.h | 2 +- > 3 files changed, 3 insertions(+), 3 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..14ce6b40ce06 100644 > --- a/xen/common/kernel.c > +++ b/xen/common/kernel.c > @@ -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 ? 'U' : ' ', > 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 >
On 05.06.2023 12:14, Andrew Cooper wrote: > On 05/06/2023 11:10 am, Jan Beulich wrote: >> On 05.06.2023 12:05, Andrew Cooper wrote: >>> Insecure the word being looked for here. Especially given the nature of the >> Nit: Missing "is"? > > Oops yes. > >> >>> sole caller, and the (correct) comment next to it. >>> >>> I've left the taint constant as 'U' as it's a rather more user-visible. >>> >>> Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type") >>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Acked-by: Jan Beulich <jbeulich@suse.com> > > Thanks, although I've got one extra hunk to add having just done the > other half of the taint work. > > diff --git a/xen/common/kernel.c b/xen/common/kernel.c > index 14ce6b40ce06..ff67f00e41bb 100644 > --- a/xen/common/kernel.c > +++ b/xen/common/kernel.c > @@ -344,7 +344,7 @@ unsigned int tainted; > * 'E' - An error (e.g. a machine check exceptions) has been injected. > * 'H' - HVM forced emulation prefix is permitted. > * 'M' - Machine had a machine check experience. > - * 'U' - Platform is unsecure (usually due to an errata on the platform). > + * 'U' - Platform is insecure (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(). My ack (quite obviously) stands with this further adjustment. Jan
On 05/06/2023 11:29 am, Bertrand Marquis wrote: > Hi Andrew, > >> On 5 Jun 2023, at 12:05, Andrew Cooper <andrew.cooper3@citrix.com> wrote: >> >> Insecure the word being looked for here. Especially given the nature of the >> sole caller, and the (correct) comment next to it. > Good finding. > >> I've left the taint constant as 'U' as it's a rather more user-visible. > I would vote to change the U in I here as it will make it more coherent > with the doc after your added fix for it. > >> Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type") >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.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> >> >> I'm happy to change 'U' to 'I' if we think that no-one is going to be impacted >> by it. I just wasn't sure. > Here i do not think many will be impacted so I would rather make this coherent. Ok. I'll submit a v2 with everything adjusted. ~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..14ce6b40ce06 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -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 ? 'U' : ' ', 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 the word being looked for here. Especially given the nature of the sole caller, and the (correct) comment next to it. I've left the taint constant as 'U' as it's a rather more user-visible. Fixes: 82c0d3d491cc ("xen: Add an unsecure Taint type") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.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> I'm happy to change 'U' to 'I' if we think that no-one is going to be impacted by it. I just wasn't sure. --- xen/arch/arm/cpuerrata.c | 2 +- xen/common/kernel.c | 2 +- xen/include/xen/lib.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) base-commit: 67fdffef9246c82cecd8db28838ed09e79e2528a