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;
Message-ID: <caa2ae100494c4604d08d9459e6d3314be2dcf26.camel@intel.com>
Subject: Re: [PATCH v3 01/21] x86/virt/tdx: Detect SEAM
From:   Kai Huang <kai.huang@intel.com>
To:     Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc:     seanjc@google.com, pbonzini@redhat.com, dave.hansen@intel.com,
        len.brown@intel.com, tony.luck@intel.com,
        rafael.j.wysocki@intel.com, reinette.chatre@intel.com,
        dan.j.williams@intel.com, peterz@infradead.org, ak@linux.intel.com,
        kirill.shutemov@linux.intel.com, isaku.yamahata@intel.com
Date:   Tue, 19 Apr 2022 15:38:42 +1200
In-Reply-To: <8e2269a7-3e71-5030-8d04-1e8e3fc4323f@linux.intel.com>
References: <cover.1649219184.git.kai.huang@intel.com>
         <ab118fb9bd39b200feb843660a9b10421943aa70.1649219184.git.kai.huang@intel.com>
         <8e2269a7-3e71-5030-8d04-1e8e3fc4323f@linux.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


> > +
> > +static void detect_seam(struct cpuinfo_x86 *c)
> > +{
> 
> why not do this check directly in tdx_detect_cpu()?

The second patch will detect TDX KeyID too.  I suppose you are saying below is
better?

void tdx_detect_cpu(struct cpuinfo_x86 *c)
{
	if (c == &boot_cpu_data) {
		detect_seam_bsp(c);
		detect_tdx_keyids_bsp(c);
	} else {
		detect_seam_ap(c);
		detect_tdx_keyids_ap(c);
	}
}

I personally don't see how above is better than the current way.  Instead, I
think having SEAM and TDX KeyID detection code in single function respectively
is more flexible for future extension (if needed).


> 
> > +	if (c == &boot_cpu_data)
> > +		detect_seam_bsp(c);
> > +	else
> > +		detect_seam_ap(c);
> > +}
> > +
> > +void tdx_detect_cpu(struct cpuinfo_x86 *c)
> > +{
> > +	detect_seam(c);
> > +}
>