Received-SPF: pass (google.com: domain of linux-kernel+bounces-99657-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <cfcca607-c070-4bf8-82a9-d4f335d56a51@intel.com>
Date: Tue, 12 Mar 2024 12:26:35 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 04/21] KVM: x86/mmu: Allow non-zero value for non-present
 SPTE and removed SPTE
To: Paolo Bonzini <pbonzini@redhat.com>, <linux-kernel@vger.kernel.org>,
	<kvm@vger.kernel.org>
CC: <seanjc@google.com>, <michael.roth@amd.com>, <isaku.yamahata@intel.com>,
	<thomas.lendacky@amd.com>, Binbin Wu <binbin.wu@linux.intel.com>
References: <20240227232100.478238-1-pbonzini@redhat.com>
 <20240227232100.478238-5-pbonzini@redhat.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240227232100.478238-5-pbonzini@redhat.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NFgwV3JRMGhZV3c3cnhFeG9WMHA5WXBFWXRLK2pBOWU4MFZVT2h4TXN3ZmxN?=
 =?utf-8?B?UUZxbFpjTnRZc0tGU3VadnlXeWJoZHZHb3ZtVEJQdjg5amJIQUsrVmRKQnJy?=
 =?utf-8?B?bDh4RFpXUExJaVphUHVQclB6YzdhTFJjSzhCMjlaczNDZi9OWTkvS3hnbXdY?=
 =?utf-8?B?SWhXejRncWlDQzA1bFFEcUpacUZUcEtQMWF1TEh2UkYva09DWjVISnJhWVpB?=
 =?utf-8?B?SVFvTkJKRVdJK1Y2OHhvbEV6WmZaRFpwbENRV2pDSDZyRnR3eG1CRWF3RFFk?=
 =?utf-8?B?SVlrM2gxMmpIdjBrSFF2dDAzb0M0Sy9DWno4M0gybWh4aDR2QnNHVDYrTWI4?=
 =?utf-8?B?MVlpci9PdlBWcU01ZkNmUktIelFOdEUvYi9EbUk1WXdlR0g0bGw0bTBJY3do?=
 =?utf-8?B?RmtaUTY0UXE1bnJpblgzTmJVMXIySmczZUZuWFZqZlZZWjVRL2Vic0oxbjJz?=
 =?utf-8?B?ZzhpaHduSzBPVWtoZGpYdEszNFVoTXU3UE9QaE15WUFHVHl6c3RWVkhPRGpm?=
 =?utf-8?B?NTlQMDc4YUU2bExlaSs1cHZIMzd3dXZnYkk3anR4WHFOZTlFbnJROG5BM0Nr?=
 =?utf-8?B?ZjRBVEpVdnBqODJ4Tk0zQ2dCblVEYjkwUFFHUjEvOVhNUDhFdnNZYVdCaFp6?=
 =?utf-8?B?bnZrODJLWktUTXJFL1JudkZ6VDFEUzlWc2VYWkxWRXpGRk91dGRtYnh1TFN1?=
 =?utf-8?B?YXNpak0rRlhuM2I5QkpBQUYyRGs4bTQ2cU01VWR3Y0x0MytNeHZEczNNOHo4?=
 =?utf-8?B?aE1QT0VXdnF2RXYrc3pGb0V4SVl1SkxDbzMvanFUVHpwazN2TFRZY2owR2x5?=
 =?utf-8?B?RlJGZk56bVpKODVkbmZKaVFkdFFQZFZjZUptVDd1OHBZWGxOU3pxNU9vR1ZM?=
 =?utf-8?B?MWZYazM3YnV4aTJZdDNwTkpUQkxGeExhV0FmRldMOUxtb2d4OUIvV0Y1eW9j?=
 =?utf-8?B?Qm5uUHQ0OVZkRTAwUVRQWWNlNUhpYWZzcHJIdGpqamdaanRiNUovajBNTmpv?=
 =?utf-8?B?azg4UHhpK05TZExpQ2tybkUxM1NXVnk3d1BaYkJSbERWbFdBdmxETDVOdUN2?=
 =?utf-8?B?RkJFVjE3dWpXRVlUWjdaT3N6TWdoYUhoMXdQZUh5S2xwSC9BMXRWSmhvYUhz?=
 =?utf-8?B?Vi93V1J0c2Q3Z0tJTnpVQ0R2N2xsRFR5bTJzV01EQnMraGp5UmR0NDZBZzBx?=
 =?utf-8?B?MkNibHh6MVlFbGZMN2Zodkt1WHFmd0NwNHByTmtWR3BydWxqTHZvSk4yamRo?=
 =?utf-8?B?cDMzSXRYLzlia3ovdTcyQk0wMW5uaHY5Y0I2cWZ0VS9uT1FKTDMwL3VicmFO?=
 =?utf-8?B?ODJqYmdxSXdjdUZOc0l5dFB6cDhTTHpTaFNYMzd2R3M0Zy9FTlZHSUdPZksv?=
 =?utf-8?B?ZE1sK2xGTU5BNU9oTytJSFFPdEVSNDRwSDhwSmlpcnVVVk5NZ21JL2plQ2ZS?=
 =?utf-8?B?bXBMNTdkNzlSem1WTi9rSEZZSmF1TDZMRXNvY0JIRmQwOThPUFB0WmlZTUUr?=
 =?utf-8?B?ak9ETlBvMFg0N2tyT0trL1pDRGV3V1VyU1czK0hLSGdLMUt0a2s0N2didFg3?=
 =?utf-8?B?NXN0bFExR09Ca2M2c2d3bytNLzV5eDQ0RU9zUEtJWHJmOVE5blVrMkxDM21S?=
 =?utf-8?B?SWx2RUdxMnJHSXZ3a0wxMndrbXhVa01haUtHTmRqQUFsZGNxTXJhRmZhUzFw?=
 =?utf-8?B?Q2FiN2p0NDg0Tzc1SWZha216Nk1mcFZnODZnOWZSNlgxWk9FOUIwaE81L2Fh?=
 =?utf-8?B?QjduMlEvTFBMMFZLYVRhU3JLSjM0YkdTaUhMWkhDcGhPQ3YwRkRqMmxkeEZ3?=
 =?utf-8?B?cUdmQ1RhTEtpQTY4MDRLY25FSlNIbnhvOUZRc08wcVU1ektOOFNiWXNNOUh5?=
 =?utf-8?B?azlHUnRtdXhVQjJWVmV6YUowWHExVndxU2MyT3VORDZvbWcxUFo5SnhjVm5D?=
 =?utf-8?B?eVZpV0ZaMDBtUXZsaVhOVFdLZkwyb1liSHJhci9XWExHRytFNW1iUVc3SFlS?=
 =?utf-8?B?MitNVXNGZ2VRaVV2dEEvYTFJNUpLZVNncGlPU3lwOXcxUjZQTE8xMmRpREkz?=
 =?utf-8?B?eWliRXRueVlyTG1RVWZma1FhZ01qSlpVNFhwcGZ5ckRJZ09uVkk5WlZSMkQ1?=
 =?utf-8?Q?6+R+rf4dPcw6dTPACRAzS43Y8?=
X-MS-Exchange-CrossTenant-Network-Message-Id: def5ea16-e5ff-4f0d-5854-08dc4222b694
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Mar 2024 23:26:44.1267
 (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: XeP9PhZMu62RymtC4B3j4I25vxdOHGrsLbmtxOw3JiDjwYiVq2UvM6g9+d42ZKz6AK/PDBA3q2anhWj9I11O0A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7529
X-OriginatorOrg: intel.com


>   
> +/*
> + * Non-present SPTE value for both VMX and SVM for TDP MMU.

In the previous patch, SHADOW_NONPRESENT_VALUE is also used in the 
shadow MMU code.  So here when you change SHADOW_NONPRESENT_VALUE to a 
non-zero value, the "for TDP MMU" part doesn't stand.

I am wondering whether we can just avoid using SHADOW_NONPRESENT_VALUE 
in shadow MMU code in the previous patch, and state explicitly that we 
are only going to support TDP MMU for non-zero value for non-present SPTE?

> + * For SVM NPT, for non-present spte (bit 0 = 0), other bits are ignored.
> + * For VMX EPT, bit 63 is ignored if #VE is disabled. (EPT_VIOLATION_VE=0)
> + *              bit 63 is #VE suppress if #VE is enabled. (EPT_VIOLATION_VE=1)
> + * For TDX:
> + *   TDX module sets EPT_VIOLATION_VE for Secure-EPT and conventional EPT
> + */
> +#ifdef CONFIG_X86_64
> +#define SHADOW_NONPRESENT_VALUE	BIT_ULL(63)
> +static_assert(!(SHADOW_NONPRESENT_VALUE & SPTE_MMU_PRESENT_MASK));
> +#else
>   #define SHADOW_NONPRESENT_VALUE	0ULL
> +#endif
>   
>   extern u64 __read_mostly shadow_host_writable_mask;
>   extern u64 __read_mostly shadow_mmu_writable_mask;
> @@ -196,7 +209,7 @@ extern u64 __read_mostly shadow_nonpresent_or_rsvd_mask;
>    *
>    * Only used by the TDP MMU.
>    */
> -#define REMOVED_SPTE	0x5a0ULL
> +#define REMOVED_SPTE	(SHADOW_NONPRESENT_VALUE | 0x5a0ULL)

I kinda prefer moving this chunk to the previous patch, because the 
reason to have SHADOW_NONPRESENT_VALUE is to have a non-zero value for 
non-present SPTEs, which include the REMOVED_SPTE.

But just my 2cents.