Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Fri, 6 Oct 2023 19:09:49 +0800
From:   "Sun, Yi" <Yi.Sun@intel.com>
To:     "Huang, Kai" <kai.huang@intel.com>
CC:     "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "x86@kernel.org" <x86@kernel.org>, "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>,
        "sathyanarayanan.kuppuswamy@linux.intel.com" 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        "Su, Heng" <heng.su@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "Yan, Dongcheng" <dongcheng.yan@intel.com>,
        "yi.sun@linux.intel.com" <yi.sun@linux.intel.com>
Subject: Re: [PATCH v4] x86/tdx: Dump TDX version During the TD Bootup
Message-ID: <ZR/q/VaR8gF1CdOg@ysun46-mobl.ccr.corp.intel.com>
References: <20231006062855.875631-1-yi.sun@intel.com>
 <9db9a874e5b7ff62e9cb419f51279047438c30ec.camel@intel.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Disposition: inline
In-Reply-To: <9db9a874e5b7ff62e9cb419f51279047438c30ec.camel@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?T1Y1Z040bUFiN3ZiMHVRYVJldEE4L0Z5bHBkRmhtRWkyNFdPQmNMU0U5N1M0?=
 =?utf-8?B?UVBoMkY3NS9QTHlOV3pGRHY4M3p0RHpJaGFiSmZBd2JiWDRIWDlLS1BvY0tP?=
 =?utf-8?B?VjJ4c01ZTVZvN0IwNVY2SFB4ZDl0ODdqQVR4N1hjcmRLM252WXV4d2ZOZW01?=
 =?utf-8?B?Sm5BRlUvR2ZVQS9Pck81d2FKM1dRVzh5cGdZaE96L21vWit3R0JqZDV2M3pJ?=
 =?utf-8?B?TUQ4OGZvd3FneThhaGdiR2dsdy9seGNWNlo2eTJWd0FnRmlSeUgrVFJhMG1H?=
 =?utf-8?B?WHU4VE9OY2lMazFXQitKQ3ZPUGlkL242WElvMFJTSmQ3bXp3R2pEWHJNR0pK?=
 =?utf-8?B?VDNPTFkzb3BEYUJ4cnJqQmtQZUM1THl4cW8vYTVhMElxWE5UMzVrVm1JWmdQ?=
 =?utf-8?B?M1JjN1VjeVArYWl3Z1VOVUZsU3htbFF1WHUwUXBPZ3Nua3Z3djFveFFvcTcz?=
 =?utf-8?B?S0V4OVh5cUdYY0tqYkVFL3F2TGgzaHBKVGQ3WmZ4NGZDbzA4b3F4QkwzdzVX?=
 =?utf-8?B?UnNlMzBKdHc0cHhEalRlQmxCNW04dHEzU2xSaHhuekpSeFppak1STWlKNVZW?=
 =?utf-8?B?QUk3K2xFR2pSU0xwVTlvVURxT2Z6YWFEU05RUFNpbWxzaUhmRnJ0SUlGODJa?=
 =?utf-8?B?YURQckg0OExqZXdGMEg3R2k3UWR4RGpyU1o1QUY5bk9nd1NrenNVTmVMUkNh?=
 =?utf-8?B?bWFGb1BMZGJlODY0c0Y5eGJpMWk3ajFxNWRIczBRcmhrNkNoaDU4cVNraXFE?=
 =?utf-8?B?cnNnbEthWWtaWEJ4T25SYk5mdWk4Rm9SY1hJVVo0dHdIdkdBMi8zb2toMUxX?=
 =?utf-8?B?K043ZFVSSFFCQU9IYmhCZDdsL2V0bXh4eGVreDZlM3ZOV2xvUXVDUXJkdVl1?=
 =?utf-8?B?R3Y4K2xSWnlxYXhIUGEvMGI1RXFHUWlyZE8reDU0MHIyOEZIZGY3R1FlMTRp?=
 =?utf-8?B?SWhXKzdGRkJtZEMrOXhucWJmbmlrWC9UV3ZINUdIaDRUMndPYnkyL0tuclpS?=
 =?utf-8?B?ZTNwQkNaRzcrNDU0RTRLSFVRM2dGeTFYTEtZZjVmdXdoN1hIUmExWlV2dlI5?=
 =?utf-8?B?N1piTTJURnVYRldPUnY3aWpwU1c5aVdVeTdXMjZZZTFvbHQwVnBFWDlRR0FQ?=
 =?utf-8?B?NWVuZ0dSQ0w2cDU2eG9iWmRtc3d3bStrQnNBTDVsR2prZzlmKzlLQjRabGJp?=
 =?utf-8?B?V0xzSzBQKzgyVDJWZldVVDZzV1FWam9wZ2lYS2k5ZkZBRHdpdHJMMHdyUnVC?=
 =?utf-8?B?YlU4ZGZ4ZENDQWR6K3o1U2wvT1VQb3FqVVBtZi9SYTZtYUZsZEVzVzZUSTRK?=
 =?utf-8?B?TS9JQkdaazFhMGtXeVlJUVRRVHJRbkNuRVdkTUxSWHBvVXJpdlhNalROUTlP?=
 =?utf-8?B?MldldXlTNW4yU0pVdzlXb0NqaHY5dEE4V2hDRWFBUWk3OWEzLzVqa3VreFdN?=
 =?utf-8?B?YVhlZjRhK05mYUpxbHFqUGhlaW9rTnk1ZllhMnVhektYdXdEWWZXRFBFdEpq?=
 =?utf-8?B?bEo0VFhhZWF4T2d1ZXNtZGNDSEptY0F1VlZuSXJvRHZKb0tsUDc0bDVrNjlL?=
 =?utf-8?B?WEpUNkZCcmU3ZElHRkxoMXZjdjMzdkh6Zy9ndXJ0VDRqOFA1Z2hKTmpXRE4r?=
 =?utf-8?B?UFYrR3NlcEFHQmFkQWxIMXBjVncvUjdaOEVvU0ZCeWE4R202Umh2eFFoMzBD?=
 =?utf-8?B?WU1GZnFkY2N5MXhxRHV5SHF2MFJJVWhMYnBmQmN5Z0l0M0RmUjJTeG9raDcz?=
 =?utf-8?B?MmFPNEhCQmdaTHJXeERuNkJadkNuZlZ6UDNJV0Z5TGphcHJWMVBzUk00RERl?=
 =?utf-8?B?bG5tdWlOR1g1cVRtNloxbzFvSnk1NnBCL044cmJEZzVmZlc3S0lHNmFheUY0?=
 =?utf-8?B?YXhlZUNQLy93VS9GSER3OGZkTkIvdC9va2p4MnNmeXg1SmFWamI3cE8vek1j?=
 =?utf-8?B?d3c5U2FONlhaQ0FTTUkxSkdKT2lBNGNlMkM2SXlRUTB3K0VyRFBGNHJBN3pu?=
 =?utf-8?B?UEp0UmdGc3RoQ08wSVZobU0ySURkRy9iTXRGak0wRHhVdDlDUWFDY2dPcnZl?=
 =?utf-8?B?U25CWnRPYk9OcVBGblJpemtPNTdCa042OHY4eCtrbDNDdy9CbjhQMjNDNGxZ?=
 =?utf-8?Q?MhMkYBg1IrSItSYbe9Pk6kSD3?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ac0773b1-e0ab-408c-54d6-08dbc6799160
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3094.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2023 14:36:03.8406
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 4jNo9/dlLQtMK/x6OnFRhLJBT76SQqHaemeUlV5392ob32eDQmV1WM7wBrVOOObrebWAYji45sNVy0txfSH/1A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7854
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=3.8 required=5.0 tests=DATE_IN_PAST_03_06,
	DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_SBL_CSS,
	SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Fri, 06 Oct 2023 07:36:25 -0700 (PDT)
X-Spam-Level: ***

On 06.10.2023 17:48, Huang, Kai wrote:
>On Fri, 2023-10-06 at 14:28 +0800, Yi Sun wrote:
>
>
>It's better if you can maintain the code change history among different versions
>so that people can see what changes have been done:
>
>	<your SoB>
>	---
>	code change history
>	---
>	...
>
>The "code change history" will be thrown away when the patch is applied.
>
I'll refine commit log and the change history.

...

>> +	td_sys->vendor_id = (u32)args.r8;
>> +
>> +	args.rdx = TDX_SYS_MAJOR_FID;
>> +	__tdcall_ret(TDX_SYS_RD, &args);
>> +
>> +	td_sys->major_version = (u16)args.r8;
>> +
>> +	args.rdx = TDX_SYS_MINOR_FID;
>> +	__tdcall_ret(TDX_SYS_RD, &args);
>> +
>> +	td_sys->minor_version = (u16)args.r8;
>> +
>> +	return;
>> +
>> +	/* TDX 1.0 does not have the TDCALL TDG.SYS.RD */
>> +version_1_0:
>> +	td_sys->vendor_id = TDX_VENDOR_INTEL;
>> +	td_sys->major_version = 1;
>> +	td_sys->minor_version = 0;
>
>So the main reason that I think you can just print module version in this
>function instead of tdx_early_init() is because IIUC you are setting up some
>random value for TDX module 1.0.  Probably it's better to just print it's module
>1.0 w/o mentioning major/minor.

Actually, it is not a random value here; it is the fixed value for TDX 1.0
becasue there is not such version info in TDX 1.0.
I assume we can state major=1 and minor=0 when TDX 1.0 is detected.
This will prevent confusion for the users.

>As replied above, I think you can print module info in tdg_get_sysinfo().
>
>If you do so, perhaps to rename the function to something like
>detect_tdx_version().
>
>You can either still keep
>
>	pr_info("Guest detected\n");
>
>here, or remove it but embed it to the message when you print module version.
Get it. I'll do that.
>
>
>> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
>> index f74695dea217..d326509832e6 100644
>> --- a/arch/x86/include/asm/shared/tdx.h
>> +++ b/arch/x86/include/asm/shared/tdx.h
>> @@ -17,6 +17,7 @@
>>  #define TDG_MR_REPORT			4
>>  #define TDG_MEM_PAGE_ACCEPT		6
>>  #define TDG_VM_WR			8
>> +#define TDX_SYS_RD			11
>
>TDG_SYS_RD, please.

Appreciate Kai's detailed comments.

Thanks
    --Sun, Yi