Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <191409fcadfed4c08eb818335008cb36167d9477.camel@intel.com>
Subject: Re: [PATCH v4 1/3] x86/tdx: Add TDX Guest attestation interface
 driver
From:   Kai Huang <kai.huang@intel.com>
To:     Isaku Yamahata <isaku.yamahata@gmail.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc:     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,
        "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>, linux-kernel@vger.kernel.org
Date:   Wed, 27 Apr 2022 17:57:57 +1200
In-Reply-To: <20220427054523.GA4167861@ls.amr.corp.intel.com>
References: <20220422233418.1203092-1-sathyanarayanan.kuppuswamy@linux.intel.com>
         <20220422233418.1203092-2-sathyanarayanan.kuppuswamy@linux.intel.com>
         <20220427054523.GA4167861@ls.amr.corp.intel.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 (3.42.4-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2022-04-26 at 22:45 -0700, Isaku Yamahata wrote:
> > + */
> > +long tdx_mcall_tdreport(void *data, void *reportdata)
> > +{
> > +	u64 ret;
> > +
> > +	/*
> > +	 * Check for a valid TDX guest to ensure this API is only
> > +	 * used by TDX guest platform. Also make sure "data" and
> > +	 * "reportdata" pointers are valid.
> > +	 */
> > +	if (!data || !reportdata ||
> > !cpu_feature_enabled(X86_FEATURE_TDX_GUEST))
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Pass the physical address of user generated report data
> > +	 * and the physical address of output buffer to the TDX module
> > +	 * to generate the TD report. Generated data contains
> > +	 * measurements/configuration data of the TD guest. More info
> > +	 * about ABI can be found in TDX 1.0 Module specification, sec
> > +	 * titled "TDG.MR.REPORT".
> > +	 */
> > +	ret = __tdx_module_call(TDX_GET_REPORT, virt_to_phys(data),
> > +				virt_to_phys(reportdata), 0, 0, NULL);
> > +
> > +	if (ret)
> > +		return TDCALL_RETURN_CODE(ret);
> 
> Why is status code masked?
> 
> 
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(tdx_mcall_tdreport);
> 
> Can this function be put into attest.c. Instead, wecan export
> __tdx_module_call.

As we discussed, there's no need to export this anymore.

Btw, I even think we can move this function to attest.c so it can be static. 
Theoretically there should be no other caller of this except attest.c.  I
understand in this way we need to define TDX_GET_REPORT macro in attest.c but
this looks fine to me.  This function can even be removed, and we can call
__tdx_module_call() directly in attest.c.  This function literally doesn't do
anything meaningful more than __tdx_module_call(), instead, calling
__tdx_module_call() directly gives us more context, i.e., we can easily see the
buffer is allocated by kernel and passed to TDX module, etc.

-- 
Thanks,
-Kai