Hi Greg,
On 10/20/22 9:39 PM, Greg Kroah-Hartman wrote:
+#ifdef MODULE +static const struct x86_cpu_id tdx_guest_ids[] = {
- X86_MATCH_FEATURE(X86_FEATURE_TDX_GUEST, NULL),
- {}
+}; +MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids); +#endif
Why the #ifdef? Should not be needed, right?
I have added it to fix the following warning reported by 0-day.
https://lore.kernel.org/lkml/202209211607.tCtTWKbV-lkp@intel.com/
It is related to nullifying the MODULE_DEVICE_TABLE in #ifndef MODULE case in linux/module.h.
Then fix it properly, by correctly using that structure no matter what. You don't do that here...
I think we can use __maybe_unused attribute to fix this warning like mentioned below. Are you fine with it?
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c @@ -118,13 +118,11 @@ static void __exit tdx_guest_exit(void) } module_exit(tdx_guest_exit);
-#ifdef MODULE -static const struct x86_cpu_id tdx_guest_ids[] = { +static const struct x86_cpu_id __maybe_unused tdx_guest_ids[] = { X86_MATCH_FEATURE(X86_FEATURE_TDX_GUEST, NULL), {} }; MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids); -#endif
Solution 2: -----------
We can also modify the code to use this structure in all cases like below. But it requires me to use slower x86_match_cpu() in place of cpu_feature_enabled() which I think is unnecessary.
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c @@ -103,9 +103,15 @@ static struct miscdevice tdx_misc_dev = { .fops = &tdx_guest_fops, };
+static const struct x86_cpu_id tdx_guest_ids[] = { + X86_MATCH_FEATURE(X86_FEATURE_TDX_GUEST, NULL), + {} +}; +MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids); + static int __init tdx_guest_init(void) { - if (!cpu_feature_enabled(X86_FEATURE_TDX_GUEST)) + if (!x86_match_cpu(tdx_guest_ids)) return -ENODEV;
return misc_register(&tdx_misc_dev); @@ -118,14 +124,6 @@ static void __exit tdx_guest_exit(void) } module_exit(tdx_guest_exit);
-#ifdef MODULE -static const struct x86_cpu_id tdx_guest_ids[] = { - X86_MATCH_FEATURE(X86_FEATURE_TDX_GUEST, NULL), - {} -}; -MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids); -#endif