| Message ID | 20230728194320.3082120-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: Some MISRA Rule 5.3 fixes | expand |
On 28.07.2023 21:43, Andrew Cooper wrote: > This is used in an assertion only, which is somewhat dubious to begin with and > won't surivive the x86-S work (where TR will become be a NUL selector). I'm kind of okay with the removal, but I can't read anything like the above out of the doc. Can you point me at where this is said? Jan > Delete it now. This avoids many cases where as a global symbol, it shadows > local string variables. > > 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: Roberto Bagnara <roberto.bagnara@bugseng.com> > CC: Nicola Vetrini <nicola.vetrini@bugseng.com> > --- > xen/arch/x86/hvm/svm/svm.c | 2 -- > xen/arch/x86/include/asm/desc.h | 9 --------- > 2 files changed, 11 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 56cb2f61bb75..4d29ad3bc36a 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1614,8 +1614,6 @@ static int _svm_cpu_up(bool bsp) > /* Initialize OSVW bits to be used by guests */ > svm_host_osvw_init(); > > - /* Minimal checking that enough CPU setup was done by now. */ > - ASSERT(str() == TSS_SELECTOR); > svm_vmsave_pa(per_cpu(host_vmcb, cpu)); > > return 0; > diff --git a/xen/arch/x86/include/asm/desc.h b/xen/arch/x86/include/asm/desc.h > index 225a864c483e..a1e0807d97ed 100644 > --- a/xen/arch/x86/include/asm/desc.h > +++ b/xen/arch/x86/include/asm/desc.h > @@ -238,15 +238,6 @@ static inline void ltr(unsigned int sel) > __asm__ __volatile__ ( "ltr %w0" :: "rm" (sel) : "memory" ); > } > > -static inline unsigned int str(void) > -{ > - unsigned int sel; > - > - __asm__ ( "str %0" : "=r" (sel) ); > - > - return sel; > -} > - > #endif /* !__ASSEMBLY__ */ > > #endif /* __ARCH_DESC_H */
On 28/07/2023 21:43, Andrew Cooper wrote: > This is used in an assertion only, which is somewhat dubious to begin > with and > won't surivive the x86-S work (where TR will become be a NUL selector). > > Delete it now. This avoids many cases where as a global symbol, it > shadows > local string variables. > > 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: Roberto Bagnara <roberto.bagnara@bugseng.com> > CC: Nicola Vetrini <nicola.vetrini@bugseng.com> > --- > xen/arch/x86/hvm/svm/svm.c | 2 -- > xen/arch/x86/include/asm/desc.h | 9 --------- > 2 files changed, 11 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 56cb2f61bb75..4d29ad3bc36a 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1614,8 +1614,6 @@ static int _svm_cpu_up(bool bsp) > /* Initialize OSVW bits to be used by guests */ > svm_host_osvw_init(); > > - /* Minimal checking that enough CPU setup was done by now. */ > - ASSERT(str() == TSS_SELECTOR); > svm_vmsave_pa(per_cpu(host_vmcb, cpu)); > > return 0; > diff --git a/xen/arch/x86/include/asm/desc.h > b/xen/arch/x86/include/asm/desc.h > index 225a864c483e..a1e0807d97ed 100644 > --- a/xen/arch/x86/include/asm/desc.h > +++ b/xen/arch/x86/include/asm/desc.h > @@ -238,15 +238,6 @@ static inline void ltr(unsigned int sel) > __asm__ __volatile__ ( "ltr %w0" :: "rm" (sel) : "memory" ); > } > > -static inline unsigned int str(void) > -{ > - unsigned int sel; > - > - __asm__ ( "str %0" : "=r" (sel) ); > - > - return sel; > -} > - > #endif /* !__ASSEMBLY__ */ > > #endif /* __ARCH_DESC_H */ With respect to shadowing (Rule 5.3) Tested-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
On 31/07/2023 9:25 am, Jan Beulich wrote: > On 28.07.2023 21:43, Andrew Cooper wrote: >> This is used in an assertion only, which is somewhat dubious to begin with and >> won't surivive the x86-S work (where TR will become be a NUL selector). > I'm kind of okay with the removal, but I can't read anything like the > above out of the doc. Can you point me at where this is said? A future draft of the spec. FRED removes the IDT completely, most of the TSS, and can let you get away with GDT/LDT limits of 0. x86-S removes the final aspects of the TSS (the IO perm bitmap, and PVI). Intel have agreed that being able to (effectively) `ltr $0x0000` to set the TSS invalid (like NULL selectors do for all other segments) would be useful, as it means you don't need to have a transiently non-empty GDT just to load an empty TR. ~Andrew
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 56cb2f61bb75..4d29ad3bc36a 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1614,8 +1614,6 @@ static int _svm_cpu_up(bool bsp) /* Initialize OSVW bits to be used by guests */ svm_host_osvw_init(); - /* Minimal checking that enough CPU setup was done by now. */ - ASSERT(str() == TSS_SELECTOR); svm_vmsave_pa(per_cpu(host_vmcb, cpu)); return 0; diff --git a/xen/arch/x86/include/asm/desc.h b/xen/arch/x86/include/asm/desc.h index 225a864c483e..a1e0807d97ed 100644 --- a/xen/arch/x86/include/asm/desc.h +++ b/xen/arch/x86/include/asm/desc.h @@ -238,15 +238,6 @@ static inline void ltr(unsigned int sel) __asm__ __volatile__ ( "ltr %w0" :: "rm" (sel) : "memory" ); } -static inline unsigned int str(void) -{ - unsigned int sel; - - __asm__ ( "str %0" : "=r" (sel) ); - - return sel; -} - #endif /* !__ASSEMBLY__ */ #endif /* __ARCH_DESC_H */
This is used in an assertion only, which is somewhat dubious to begin with and won't surivive the x86-S work (where TR will become be a NUL selector). Delete it now. This avoids many cases where as a global symbol, it shadows local string variables. 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: Roberto Bagnara <roberto.bagnara@bugseng.com> CC: Nicola Vetrini <nicola.vetrini@bugseng.com> --- xen/arch/x86/hvm/svm/svm.c | 2 -- xen/arch/x86/include/asm/desc.h | 9 --------- 2 files changed, 11 deletions(-)