Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Message-ID: <91b1fbc8-7308-43d1-a27a-90dc21fa7f40@suse.com>
Date:   Tue, 24 Oct 2023 16:31:43 +0300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v14 12/23] x86/virt/tdx: Allocate and set up PAMTs for
 TDMRs
Content-Language: en-US
To:     "Huang, Kai" <kai.huang@intel.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc:     "sathyanarayanan.kuppuswamy@linux.intel.com" 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "david@redhat.com" <david@redhat.com>,
        "bagasdotme@gmail.com" <bagasdotme@gmail.com>,
        "ak@linux.intel.com" <ak@linux.intel.com>,
        "kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>,
        "Christopherson,, Sean" <seanjc@google.com>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "Yamahata, Isaku" <isaku.yamahata@intel.com>,
        "Luck, Tony" <tony.luck@intel.com>,
        "hpa@zytor.com" <hpa@zytor.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "Shahar, Sagi" <sagis@google.com>,
        "imammedo@redhat.com" <imammedo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>, "Gao, Chao" <chao.gao@intel.com>,
        "rafael@kernel.org" <rafael@kernel.org>,
        "Brown, Len" <len.brown@intel.com>,
        "Huang, Ying" <ying.huang@intel.com>,
        "Williams, Dan J" <dan.j.williams@intel.com>,
        "x86@kernel.org" <x86@kernel.org>
References: <cover.1697532085.git.kai.huang@intel.com>
 <ac0e625f0a7a7207e1e7b52c75e9b48091a7f060.1697532085.git.kai.huang@intel.com>
 <33857df5-3639-4db4-acdd-7b692852b601@suse.com>
 <457f0b5366134c2b09d6312c32e5c84b123e7911.camel@intel.com>
From:   Nikolay Borisov <nik.borisov@suse.com>
In-Reply-To: <457f0b5366134c2b09d6312c32e5c84b123e7911.camel@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dXhIS2Q2SHhXNmZqQnJadk1QZ1JLb2tLa0NpMiszVzRIaktJajBwdTd2M2dO?=
 =?utf-8?B?YlRVa3h4Tnd4ck9CVllIMEJRVWFHeDQ1cWtDdlpyOUdpK0tkeWlmR1VzYmkw?=
 =?utf-8?B?d0RQaHpkV2hUWkxESVJwbk5UaGFicHhOOHhDNjhRQU1haS9WQnRSSUlYWjhi?=
 =?utf-8?B?V0piWGx2OVcydi83RWMwbVhPMUp5ZzVlTVJhaGRYNzNrS3Y4dHhvNGZNcXVH?=
 =?utf-8?B?aTJUZE1LdlJVMGdYZ0g2UVo4RUd4b21HWUhDZlJxWTBURk1sMHhiU2NCS0Yr?=
 =?utf-8?B?UU11MGkySTFWSzFtbzQrUmh1MHl5MkJUOVhvL0JrRUpneHVGUEt5V0ljeVpw?=
 =?utf-8?B?NmhlV2JVTldjdXJqK1g3Q1p2RHgvMHZyMy9WUGdsd29FWUo2Ym03R2tyZE1G?=
 =?utf-8?B?eWR1QUZmOHowZG1YRkhXZmhSTDhiZ3MvZ3lVK2gwaFVzaldSenFDQTJhejhY?=
 =?utf-8?B?R3NyZy9lVG1sWSs1cW1lb01hRjYwYzV4NDZOSlhuNit2bE5lMUYrRlhzWE9R?=
 =?utf-8?B?ay92TWI1Nm4zYnY5NlAvNzJ0UWxKdlJOdWtkSjJqdDNrNXpjUUhGTWtLeXdM?=
 =?utf-8?B?UDMrVzZ3Y3Z2eEdIRzB1V3g5bCtVSVg5QmRNMWF1UmNYNHFSakxsc01MNEVJ?=
 =?utf-8?B?T0NCVHNNOC9GaUZNaE1VWEhkZE9JSjIwOHQrSkQ4NmxlV0VYMS90ME5wNHky?=
 =?utf-8?B?SHhGWG12NWQrNFhLVTVQY1FlYy9MSjQ0ODQxK00vdHlvVXRwZWpuVTN2eUJx?=
 =?utf-8?B?NlFZcW8xckp5UkEwMzBCd1hDalpkUFh0M1o1R1FpY25IUlluaVhmRW10ZTh1?=
 =?utf-8?B?d2VCQjRUQ09mZGo2aWU2dVE2RGxiTnpPNXZrYjYzWCtVL0dxd3J5dndHZVRQ?=
 =?utf-8?B?RUo4bXRkbjZBRTRseDhCZW40UzVVMHcvWjBhemtYbHY3WnFhTGh3cDJaV0lj?=
 =?utf-8?B?aEkrVU13SmdrQUZlVFBuNmxja28xMWI2YnNFZXFoZUNXemdGVWZ0L0pveld2?=
 =?utf-8?B?WEhUakFBSVZITlJNcEdHeTlaZC8zSTR0NEtHY240NFBuUkpyOHNnUW05TmV1?=
 =?utf-8?B?NWRPVGFRaXhMNXFKNVJEa1lmWm1RcW52M3gvOU1qL2tMV3krR3A3SzJyeXpY?=
 =?utf-8?B?amU4SkFOb1RjSWgvM2dzL0ZjWUNsdnlaQXNMd0htS1RqTGt4ZnJ5N1pUWG9x?=
 =?utf-8?B?V2doMSs0aHVkTlhyeFFIT21tM1Y3VnZYWnF3bjRVOUNHRlFQN09JSTJUREhS?=
 =?utf-8?B?YkkxQzl6N0tXbmcrU1RGRmgrVjQxWnJDMFFtMjZmVS80S1RYSitWWWF6UlJw?=
 =?utf-8?B?WlFQZWRoQ1lZNVRTSzgwYkgxUGZaWUhHY0VCWCt4OGx0ZFZEK1FRTTZ2ZXhq?=
 =?utf-8?B?ODgyYStJRDlpeTVVaHo2aWxSSjd2TG9NcityZHRqRUd2S0dVTEIxS09mSk9m?=
 =?utf-8?B?dmtqVTR6aGdsLzd6UDI5R292ai9XWDZQV2xTWFVIR01pTnRsS1h6U3V1M2hL?=
 =?utf-8?B?V0NPU0l5eE03MHB2eEFJU3NSWThrM253ZndqaHZZTEtIY3VYY1BYQ3N5NEFv?=
 =?utf-8?B?eUh0SUQ4Qzh4V3hlbVNuOFAzZENMbzZQQmJGVC9xODd3dzhMYnFBL3lXSTgw?=
 =?utf-8?B?MXdkL2hYQWlHZmVzcVYrYUJja3oydldtWnl0ajdLRThibWRhL1BmYXdpdzZW?=
 =?utf-8?B?RXVHQTVCVFhtZ2VMQk04Nkcwc0oydFVGT3dNcWxLc1RMQnh0NDRCWmNRa0E1?=
 =?utf-8?B?aUtRZDBvbEk3RVRmNDk5aGJmTGFieFcxTXJqcEZXdUFRV3R0VzAwdXhWalph?=
 =?utf-8?B?R1cxWnpUM0hqeVlyTEhFeDIxNVFSdDhoY2hoK0xudEJSbytqd1JEMTJYYzZt?=
 =?utf-8?B?VXBpODVEZUIxZzhpL2FTVDR6TmhTSkk1cEh2dEFRMkI2WkhQL1NHeEtCTWZB?=
 =?utf-8?B?MnJOdVF6RmRyaTk2R2EzdUFWL1grQ2p6RkhmZkNtRlcxNnYraWtsZGg4bUJ0?=
 =?utf-8?B?c3JKUDlaZk80Y2FTZFRBUmw0NSsxTmtpVk9KQWg4djhOMm93MHhnTnBray9Y?=
 =?utf-8?B?RFVXdEh4SDdGd0w4SHVNVk9TdXdxd3ZKQ1FnbmFMZ25hVC96TVlmL2c5eUpK?=
 =?utf-8?B?UDJnaTJZNEhWKy9KWUtYZ3FFbWVoVGgwejBHNXl5bXFlMFVGaFpTZW5rMkgx?=
 =?utf-8?Q?YUZ2O2ik+fYHadgyl/0/COF9s4hIhgT/7fAetSkIwf/R?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 440da009-8e08-4bc7-328e-08dbd4959359
X-MS-Exchange-CrossTenant-AuthSource: PA4PR04MB7790.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2023 13:31:49.1844
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: oMJduQ0tqAv6g/7Ma258Te0lOgvgP8KPlG7Rbk7crNaRwwle95bzsGnIyky5gOeZZyqYUjpIAE6Y5WDwjwGuIg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXPR04MB8112
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.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 (fry.vger.email [0.0.0.0]); Tue, 24 Oct 2023 06:32:09 -0700 (PDT)


On 24.10.23 г. 13:49 ч., Huang, Kai wrote:
> On Tue, 2023-10-24 at 08:53 +0300, Nikolay Borisov wrote:
>>
>> On 17.10.23 г. 13:14 ч., Kai Huang wrote:
>>
>> <snip>
>>
>>>    arch/x86/Kconfig                  |   1 +
>>>    arch/x86/include/asm/shared/tdx.h |   1 +
>>>    arch/x86/virt/vmx/tdx/tdx.c       | 215 +++++++++++++++++++++++++++++-
>>>    arch/x86/virt/vmx/tdx/tdx.h       |   1 +
>>>    4 files changed, 213 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>>> index 864e43b008b1..ee4ac117aa3c 100644
>>> --- a/arch/x86/Kconfig
>>> +++ b/arch/x86/Kconfig
>>> @@ -1946,6 +1946,7 @@ config INTEL_TDX_HOST
>>>    	depends on KVM_INTEL
>>>    	depends on X86_X2APIC
>>>    	select ARCH_KEEP_MEMBLOCK
>>> +	depends on CONTIG_ALLOC
>>>    	help
>>>    	  Intel Trust Domain Extensions (TDX) protects guest VMs from malicious
>>>    	  host and certain physical attacks.  This option enables necessary TDX
>>> diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
>>> index abcca86b5af3..cb59fe329b00 100644
>>> --- a/arch/x86/include/asm/shared/tdx.h
>>> +++ b/arch/x86/include/asm/shared/tdx.h
>>> @@ -58,6 +58,7 @@
>>>    #define TDX_PS_4K	0
>>>    #define TDX_PS_2M	1
>>>    #define TDX_PS_1G	2
>>> +#define TDX_PS_NR	(TDX_PS_1G + 1)
>>
>> nit: I'd prefer if you those defines are turned into an enum and
>> subsequently this enum type can be used in the definition of
>> tdmr_get_pamt_sz(). However, at this point I consider this a
>> bikeshedding and you can do that iff you are going to respin the series
>> due to other feedback as well.
>>
>> <snip>
> 
> Thanks for comments.  But to be honest I don't get why enum is better, and I
> would like to leave this to future work if really needed.  Please note these
> TDX_PS_xx are also used by TDX guest code (see patch 2).

The reason being self-documenting code. In particular when looking at 
the signature of tdmr_get_pamt_sz if the 2nd argument was enum 
tdx_ps_size or some such it would be immediately evident what this 
parameter is all about. Right now it's just a plain int which can be 
anything. Indeed, this is mostly cosmetic so hence it's minor.