Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 15 Sep 2022 13:07:05 +0200
From:   Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To:     Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc:     Dave Hansen <dave.hansen@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        Shuah Khan <shuah@kernel.org>,
        "H . Peter Anvin" <hpa@zytor.com>,
        "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
        Tony Luck <tony.luck@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Kai Huang <kai.huang@intel.com>,
        Wander Lairson Costa <wander@redhat.com>,
        Isaku Yamahata <isaku.yamahata@gmail.com>,
        marcelo.cerri@canonical.com, tim.gardner@canonical.com,
        khalid.elmously@canonical.com, philip.cox@canonical.com,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-doc@vger.kernel.org
Subject: Re: [PATCH v13 1/3] x86/tdx: Add TDX Guest attestation interface
 driver
Message-ID: <YyMHWQYUJHyIJMfA@kroah.com>
References: <20220909192708.1113126-1-sathyanarayanan.kuppuswamy@linux.intel.com>
 <20220909192708.1113126-2-sathyanarayanan.kuppuswamy@linux.intel.com>
 <7c3cc265-869b-b2fc-43f2-d2cbd0bc142c@intel.com>
 <1182ef92-cae3-b7c1-8339-8e8bfa48f2e3@linux.intel.com>
 <d238f235-5623-d152-c0d3-2ef851f88473@intel.com>
 <d5b54f8c-2e03-4ac8-8b8a-605c7f0f601b@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d5b54f8c-2e03-4ac8-8b8a-605c7f0f601b@linux.intel.com>
Precedence: bulk

On Wed, Sep 14, 2022 at 05:30:45PM -0700, Sathyanarayanan Kuppuswamy wrote:
> I have also included info about why we don't use a separate config
> option for it.
> 
> The code for the SEV equivalent of this TDX attestation functionality
> can be found in drivers/virt/coco/sev-guest/. It is implemented as a
> platform module driver, and it can be enabled using the CONFIG_SEV_GUEST
> config option. However, in the case of TDX, it is implemented as a
> built-in driver in the arch/x86/coco/tdx/tdx.c because of the following
> reasons:
> 
> 1. Attestation is expected to be needed by all distributions that support
>    TDX. Therefore, using a separate configuration option is not necessary.
>    With TDX support, it can be enabled by default, and a built-in driver
>    model will work better in this use case.

No, that's not valid.  Distros want to enable everything, but only load
stuff that is only present.  You don't allow this for this code, which
isn't ok.

> 2. Since it is not a conventional device driver and the code is very simple,
>    creating an individual driver for it may be an overkill.

"simple" is not the issue, again, this should be a "normal" driver that
autoloads when the hardware is present and not load when the hardware is
not present.  This is not "special" to avoid the normal functionality of
all other drivers.

So again, no, this is not ok, fix this properly, don't be lazy.

thanks,

greg k-h