Received-SPF: pass (google.com: domain of linux-kernel+bounces-108250-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 19 Mar 2024 17:09:20 -0700
From: Isaku Yamahata <isaku.yamahata@intel.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Yamahata, Isaku" <isaku.yamahata@intel.com>,
	"Zhang, Tina" <tina.zhang@intel.com>,
	"seanjc@google.com" <seanjc@google.com>,
	"Yuan, Hang" <hang.yuan@intel.com>,
	"Huang, Kai" <kai.huang@intel.com>, "Chen, Bo2" <chen.bo@intel.com>,
	"sagis@google.com" <sagis@google.com>,
	"isaku.yamahata@gmail.com" <isaku.yamahata@gmail.com>,
	"Aktas, Erdem" <erdemaktas@google.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"sean.j.christopherson@intel.com" <sean.j.christopherson@intel.com>,
	"Yao, Yuan" <yuan.yao@intel.com>,
	"binbin.wu@linux.intel.com" <binbin.wu@linux.intel.com>,
	isaku.yamahata@linux.intel.com
Subject: Re: [PATCH v19 029/130] KVM: TDX: Add C wrapper functions for
 SEAMCALLs to the TDX module
Message-ID: <20240320000920.GD1994522@ls.amr.corp.intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <7cfd33d896fce7b49bcf4b7179d0ded22c06b8c2.1708933498.git.isaku.yamahata@intel.com>
 <3370738d1f6d0335e82adf81ebd2d1b2868e517d.camel@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <3370738d1f6d0335e82adf81ebd2d1b2868e517d.camel@intel.com>

On Tue, Mar 19, 2024 at 11:24:37PM +0000,
"Edgecombe, Rick P" <rick.p.edgecombe@intel.com> wrote:

> On Mon, 2024-02-26 at 00:25 -0800, isaku.yamahata@intel.com wrote:
> > +
> > +static inline u64 tdh_mem_sept_add(hpa_t tdr, gpa_t gpa, int level,
> > hpa_t page,
> > +                                  struct tdx_module_args *out)
> > +{
> > +       struct tdx_module_args in = {
> > +               .rcx = gpa | level,
> > +               .rdx = tdr,
> > +               .r8 = page,
> > +       };
> > +
> > +       clflush_cache_range(__va(page), PAGE_SIZE);
> > +       return tdx_seamcall(TDH_MEM_SEPT_ADD, &in, out);
> > +}
> 
> The caller of this later in the series looks like this:
> 
> 	err = tdh_mem_sept_add(kvm_tdx, gpa, tdx_level, hpa, &out);
> 	if (unlikely(err == TDX_ERROR_SEPT_BUSY))
> 		return -EAGAIN;
> 	if (unlikely(err == (TDX_EPT_ENTRY_STATE_INCORRECT |
> TDX_OPERAND_ID_RCX))) {
> 		union tdx_sept_entry entry = {
> 			.raw = out.rcx,
> 		};
> 		union tdx_sept_level_state level_state = {
> 			.raw = out.rdx,
> 		};
> 
> 		/* someone updated the entry with same value. */
> 		if (level_state.level == tdx_level &&
> 		    level_state.state == TDX_SEPT_PRESENT &&
> 		    !entry.leaf && entry.pfn == (hpa >> PAGE_SHIFT))
> 			return -EAGAIN;
> 	}
> 
> The helper abstracts setting the arguments into the proper registers
> fields passed in, but doesn't abstract pulling the result out from the
> register fields. Then the caller has to manually extract them in this
> verbose way. Why not have the helper do both?

Yes. Let me update those arguments.
-- 
Isaku Yamahata <isaku.yamahata@intel.com>