Received-SPF: pass (google.com: domain of linux-kernel+bounces-22196-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 10 Jan 2024 15:54:51 +0300
From: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
To: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: x86@kernel.org, Dave Hansen <dave.hansen@linux.intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Xiaoyao Li <xiaoyao.li@intel.com>, linux-kernel@vger.kernel.org,
	linux-coco@lists.linux.dev
Subject: Re: [PATCH v1] virt: tdx-guest: Handle GetQuote request error code
Message-ID: <20240110125451.whabimhece7dw5jn@box>
References: <20240109054604.2562620-1-sathyanarayanan.kuppuswamy@linux.intel.com>
 <20240109131740.nk54gdmri6gpwkta@box.shutemov.name>
 <6805ad1f-2c91-4a8d-98c9-5da337dd13f6@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6805ad1f-2c91-4a8d-98c9-5da337dd13f6@linux.intel.com>

On Tue, Jan 09, 2024 at 07:56:56PM -0800, Kuppuswamy Sathyanarayanan wrote:
> 
> 
> On 1/9/2024 5:17 AM, Kirill A . Shutemov wrote:
> > On Tue, Jan 09, 2024 at 05:46:04AM +0000, Kuppuswamy Sathyanarayanan wrote:
> >> Currently when a user requests for the Quote generation, the Quote
> >> generation handler (tdx_report_new()) only checks whether the VMM
> >> successfully processes the Quote generation request (status !=
> >> GET_QUOTE_IN_FLIGHT) and returns the output to the user without
> >> validating the status of the output data. Since VMM can return error
> >> even after processing the Quote request, returning success just after
> >> successful processing will create confusion to the user. Although for
> >> the failed request, output buffer length will be zero and can also be
> >> used by the user to identify the failure case, it will be more clear to
> >> return error for all failed cases. So validate the Quote output status
> >> and return error code for all failed cases.
> > 
> > Could you split commit message into several paragraphs? It would be easier
> > to get along.
> > 
> > It can be helpful to follow structure like:
> > 
> > <Background>
> > 
> > <Problem>
> > 
> > <Solution>
> > 
> 
> How about the following version?
> 
> During the TDX guest attestation process, TSM ConfigFS ABI is used by
> the user attestation agent to get the signed VM measurement data (a.k.a
> Quote), which can be used by a remote verifier to validate the
> trustworthiness of the guest. When a user requests for the Quote data
> via the ConfigFS ABI, the TDX Quote generation handler
> (tdx_report_new()) forwards the request to VMM (or QE) via a hypercall,
> and then shares the output with the user.
> 
> Currently, when handling the Quote generation request, tdx_report_new()
> handler only checks whether the VMM successfully processed the request
> and if it is true it returns success and shares the output to the user
> without actually validating the output data. Since the VMM can return
> error even after processing the Quote request, always returning success
> for the processed requests is incorrect and will create confusion to
> the user. Although for the failed request, output buffer length will
> be zero and can also be used by the user to identify the failure case,
> it will be more clear to return error for all failed cases.
> 
> So when handling the Quote generation request, validate the Quote data
> output status and return error code for all failed cases.

I would drop the start of the sentence upto ',' here, but otherwise looks
good to me.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov