| Message ID | a50918ba3415be4186a91161fe3bbd839153d8b2.1708933498.git.isaku.yamahata@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v19,001/130] x86/virt/tdx: Rename _offset to _member for TD_SYSINFO_MAP() macro | expand |
On 26/02/2024 9:25 pm, isaku.yamahata@intel.com wrote: > From: Isaku Yamahata <isaku.yamahata@intel.com> > > This patch fixes the following warnings. > > In file included from arch/x86/kernel/asm-offsets.c:22: > arch/x86/include/asm/tdx.h:92:87: warning: shift count >= width of type [-Wshift-count-overflow] > arch/x86/include/asm/tdx.h:20:21: note: expanded from macro 'TDX_ERROR' > #define TDX_ERROR _BITUL(63) > > ^~~~~~~~~~ > > Also consistently use ULL for TDX_SEAMCALL_VMFAILINVALID. > > Fixes: 527a534c7326 ("x86/tdx: Provide common base for SEAMCALL and TDCALL C wrappers") +Kirill. This kinda fix should be sent out as a separate patch. > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com> > --- > arch/x86/include/asm/tdx.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h > index 16be3a1e4916..1e9dcdf9912b 100644 > --- a/arch/x86/include/asm/tdx.h > +++ b/arch/x86/include/asm/tdx.h > @@ -17,9 +17,9 @@ > * Bits 47:40 == 0xFF indicate Reserved status code class that never used by > * TDX module. > */ > -#define TDX_ERROR _BITUL(63) > +#define TDX_ERROR _BITULL(63) > #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40)) > -#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000)) > +#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _ULL(0xFFFF0000)) > > #define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) > #define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD) Both TDX guest and TDX host code depends on X86_64 in the Kconfig. This issue seems due to asm-offsets.c includes <asm/tdx.h> unconditionally. It doesn't make sense to generate any TDX related code in asm-offsets.h so I am wondering whether it is better to just make the inclusion of <asm/tdx.h> conditionally or move it the asm-offsets_64.c? Kirill what's your opinion? Btw after quick try seems I cannot reproduce this (w/o this KVM TDX patchset). Isaku, could you share your .config?
On Thu, Feb 29, 2024 at 11:49:13AM +1300, Huang, Kai wrote: > > > On 26/02/2024 9:25 pm, isaku.yamahata@intel.com wrote: > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > This patch fixes the following warnings. > > > > In file included from arch/x86/kernel/asm-offsets.c:22: > > arch/x86/include/asm/tdx.h:92:87: warning: shift count >= width of type [-Wshift-count-overflow] > > arch/x86/include/asm/tdx.h:20:21: note: expanded from macro 'TDX_ERROR' > > #define TDX_ERROR _BITUL(63) > > > > ^~~~~~~~~~ > > I think you trim the warning message. I don't see the actual user of the define. Define itself will not generate the warning. You need to actually use it outside of preprocessor. I don't understand who would use it in 32-bit code. Maybe fixing it this way masking other issue. That said, I don't object the change itself. We just need to understand the context more.
On Fri, Mar 01, 2024 at 01:36:43PM +0200, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote: > On Thu, Feb 29, 2024 at 11:49:13AM +1300, Huang, Kai wrote: > > > > > > On 26/02/2024 9:25 pm, isaku.yamahata@intel.com wrote: > > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > > > This patch fixes the following warnings. > > > > > > In file included from arch/x86/kernel/asm-offsets.c:22: > > > arch/x86/include/asm/tdx.h:92:87: warning: shift count >= width of type [-Wshift-count-overflow] > > > arch/x86/include/asm/tdx.h:20:21: note: expanded from macro 'TDX_ERROR' > > > #define TDX_ERROR _BITUL(63) > > > > > > ^~~~~~~~~~ > > > > > I think you trim the warning message. I don't see the actual user of the > define. Define itself will not generate the warning. You need to actually > use it outside of preprocessor. I don't understand who would use it in > 32-bit code. Maybe fixing it this way masking other issue. > > That said, I don't object the change itself. We just need to understand > the context more. v18 used it as stub function. v19 dropped it as the stub was not needed.
On 5/03/2024 9:12 pm, Isaku Yamahata wrote: > On Fri, Mar 01, 2024 at 01:36:43PM +0200, > "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote: > >> On Thu, Feb 29, 2024 at 11:49:13AM +1300, Huang, Kai wrote: >>> >>> >>> On 26/02/2024 9:25 pm, isaku.yamahata@intel.com wrote: >>>> From: Isaku Yamahata <isaku.yamahata@intel.com> >>>> >>>> This patch fixes the following warnings. >>>> >>>> In file included from arch/x86/kernel/asm-offsets.c:22: >>>> arch/x86/include/asm/tdx.h:92:87: warning: shift count >= width of type [-Wshift-count-overflow] >>>> arch/x86/include/asm/tdx.h:20:21: note: expanded from macro 'TDX_ERROR' >>>> #define TDX_ERROR _BITUL(63) >>>> >>>> ^~~~~~~~~~ >>>> >> >> I think you trim the warning message. I don't see the actual user of the >> define. Define itself will not generate the warning. You need to actually >> use it outside of preprocessor. I don't understand who would use it in >> 32-bit code. Maybe fixing it this way masking other issue. >> >> That said, I don't object the change itself. We just need to understand >> the context more. > > v18 used it as stub function. v19 dropped it as the stub was not needed. Sorry I literally don't understand what you are talking about here. Please just clarify (at least): - Does this problem exist in upstream code? - If it does, what is the root cause, and how to reproduce?
On Wed, Mar 06, 2024 at 10:35:43AM +1300, "Huang, Kai" <kai.huang@intel.com> wrote: > > > On 5/03/2024 9:12 pm, Isaku Yamahata wrote: > > On Fri, Mar 01, 2024 at 01:36:43PM +0200, > > "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> wrote: > > > > > On Thu, Feb 29, 2024 at 11:49:13AM +1300, Huang, Kai wrote: > > > > > > > > > > > > On 26/02/2024 9:25 pm, isaku.yamahata@intel.com wrote: > > > > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > > > > > > > This patch fixes the following warnings. > > > > > > > > > > In file included from arch/x86/kernel/asm-offsets.c:22: > > > > > arch/x86/include/asm/tdx.h:92:87: warning: shift count >= width of type [-Wshift-count-overflow] > > > > > arch/x86/include/asm/tdx.h:20:21: note: expanded from macro 'TDX_ERROR' > > > > > #define TDX_ERROR _BITUL(63) > > > > > > > > > > ^~~~~~~~~~ > > > > > > > > > > > I think you trim the warning message. I don't see the actual user of the > > > define. Define itself will not generate the warning. You need to actually > > > use it outside of preprocessor. I don't understand who would use it in > > > 32-bit code. Maybe fixing it this way masking other issue. > > > > > > That said, I don't object the change itself. We just need to understand > > > the context more. > > > > v18 used it as stub function. v19 dropped it as the stub was not needed. > > Sorry I literally don't understand what you are talking about here. > > Please just clarify (at least): > > - Does this problem exist in upstream code? No. > - If it does, what is the root cause, and how to reproduce? v18 had a problem because it has stub function. v19 doesn't have problem because it deleted the stub function.
>> Please just clarify (at least): >> >> - Does this problem exist in upstream code? > > No. > >> - If it does, what is the root cause, and how to reproduce? > > v18 had a problem because it has stub function. v19 doesn't have problem because > it deleted the stub function. What is the "stub function"?? If "v19 doesn't have problem", why do you even _need_ this patch?? I am tired of guessing, but I don't care anymore given it's not a problem in upstream code.
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h index 16be3a1e4916..1e9dcdf9912b 100644 --- a/arch/x86/include/asm/tdx.h +++ b/arch/x86/include/asm/tdx.h @@ -17,9 +17,9 @@ * Bits 47:40 == 0xFF indicate Reserved status code class that never used by * TDX module. */ -#define TDX_ERROR _BITUL(63) +#define TDX_ERROR _BITULL(63) #define TDX_SW_ERROR (TDX_ERROR | GENMASK_ULL(47, 40)) -#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _UL(0xFFFF0000)) +#define TDX_SEAMCALL_VMFAILINVALID (TDX_SW_ERROR | _ULL(0xFFFF0000)) #define TDX_SEAMCALL_GP (TDX_SW_ERROR | X86_TRAP_GP) #define TDX_SEAMCALL_UD (TDX_SW_ERROR | X86_TRAP_UD)