linux-kernel - Re: [PATCH v15 2/3] virt: Add TDX guest driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y1OIHjFp2r58fDPI@kroah.com>
Date:   Sat, 22 Oct 2022 08:05:18 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        Shuah Khan <shuah@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Tony Luck <tony.luck@...el.com>,
        Kai Huang <kai.huang@...el.com>,
        Wander Lairson Costa <wander@...hat.com>,
        Isaku Yamahata <isaku.yamahata@...il.com>,
        marcelo.cerri@...onical.com, tim.gardner@...onical.com,
        khalid.elmously@...onical.com, philip.cox@...onical.com,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH v15 2/3] virt: Add TDX guest driver

On Fri, Oct 21, 2022 at 04:51:34PM -0700, Sathyanarayanan Kuppuswamy wrote:
> 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))

Please use this as it's what all other users of the x86cpu module device
table code uses, right?

And what is the "speed" difference here?  Is is measurable and where
does it matter?

thanks,

greg k-h