Received-SPF: pass (google.com: domain of linux-kernel+bounces-167125-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <2c671b0f-fc45-4073-aff7-f65461d2e785@intel.com>
Date: Fri, 3 May 2024 12:54:40 +1200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/5] x86/virt/tdx: Move TDMR metadata fields map table to
 local variable
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
CC: "Hansen, Dave" <dave.hansen@intel.com>, "seanjc@google.com"
	<seanjc@google.com>, "bp@alien8.de" <bp@alien8.de>, "x86@kernel.org"
	<x86@kernel.org>, "peterz@infradead.org" <peterz@infradead.org>,
	"hpa@zytor.com" <hpa@zytor.com>, "mingo@redhat.com" <mingo@redhat.com>,
	"kirill.shutemov@linux.intel.com" <kirill.shutemov@linux.intel.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>, "pbonzini@redhat.com"
	<pbonzini@redhat.com>, "jgross@suse.com" <jgross@suse.com>, "Yamahata, Isaku"
	<isaku.yamahata@intel.com>
References: <cover.1709288433.git.kai.huang@intel.com>
 <41cd371d8a9caadf183e3ab464c57f9f715184d3.1709288433.git.kai.huang@intel.com>
 <21310a611d652f14b39da4a88a75ded1d155672e.camel@intel.com>
 <6d173173-97de-42a9-85f7-20c5b6a2e6fe@intel.com>
 <6cbb56898011b8b0adbff216f1318dc5529b7d1f.camel@intel.com>
Content-Language: en-US
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <6cbb56898011b8b0adbff216f1318dc5529b7d1f.camel@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bWYzSjBqcXNWODNyblB1b0FvRm15dWVCcWpwbzF4NFhjaFZ4VmVneHY4bk5V?=
 =?utf-8?B?UTJQL05LZloyS1Azb1pIR3pLYTNXTXEwNUhrSUJrZFZ4ckxzWitQUkdheWxr?=
 =?utf-8?B?VVQ5UCt4T294K1N3aEQzWlFROVJNajk0T3RZL3JrMkFXcCtoekt6T2ppa25H?=
 =?utf-8?B?RU5QYmJVUXQrcyt6VFNYb1ZWZVRoS2FUNUJCVzgwQkloR0hSc3gzSU0vY0NY?=
 =?utf-8?B?UUxqOG8rcFJCNlVDcFdOd0tndmg0V21JT2srenZENnVWN1Jzck41SHhnNkM1?=
 =?utf-8?B?NEU0eXRnSDY4NDF3OVp6bjFaaWtkZU9lWnBRd1V0Rk52TUxMSUtsYmNrTXJl?=
 =?utf-8?B?ci9zMTM4ZkFsQkpJdGZIMVZRMjN1MlBHQk5IVTdZV3VmNitLL3NuSFQxeEgr?=
 =?utf-8?B?RFJPa2xkbVF6RjIxaVJRR1FzVHYvK2NuYWxIUmhvV1c0cjZUcmNyUmxwb1ND?=
 =?utf-8?B?SEFLVitNYlMxaEExZjRGZXNtMjJJR3NqSGNPeVhUZHdJamJLT2x4dkJOSmVW?=
 =?utf-8?B?TmVGMFJmcWxZOWZRRmVSSXBqeEtUYXlMYURoRnpoTDBYSTVpM2Q3ekczWlFu?=
 =?utf-8?B?Z3pjbEdjSmRYYmxuL0JJcFBSc3UwYXptakhvemdFWlFPcmZ1cVJMZ3orNzE4?=
 =?utf-8?B?R3hmdnlodHZPVnBwOUlCK0dwWCtJN0wybVpNZHRveko3UVBjT1ZKTThoYTRZ?=
 =?utf-8?B?RVcvQ0NKZzU3aGZHVzJWRE9sdkttNjB1d1RHcXBjc1ZqT1o5cjN2TE9IYmF2?=
 =?utf-8?B?R3NxdlZUNFI2KzBBRnppeDNqY2VFWDBPeFhqNk1JZHNqSXk3VVJBK1VsWUw3?=
 =?utf-8?B?dDJISm82ODg0aWRXTUZwWllhZW5vbnRDbWpPbWRkdzkyYlVKR1c1YzVPVFhE?=
 =?utf-8?B?cHRkaFRpTjBHOUJ4Z1dYVEJCbnVRZ3RwWWt0NHZEYUg1MWM1bUcxSHJzRzRv?=
 =?utf-8?B?TzgzMWQvSlJsL3BUenI5alQwSVh3aGVscHJZeWVuQTBaSWdQcXJ5KzlIVmN5?=
 =?utf-8?B?MUZHaWQwaTJUYXBYVHZOS3hhMVJCMXgzanJuZkIwQTNRRmV0UGxETjRieXVL?=
 =?utf-8?B?TTNSM3ZibUZGSk1vWExubDZJTHd0Sy9hcTRubEJrNE5ZUENqWjM3WlJ2RHM1?=
 =?utf-8?B?eFpFSmZUOFFmbDlvZUNKZlQ3WUdFcUJrVWFHeUdjZDQ0Qmk4L1grZS90VEVW?=
 =?utf-8?B?bkdhSnU2dE1VWFFlaEtyRVFGaFR5NyswNThXU29vWjAxTWpPZ1diaktneDRv?=
 =?utf-8?B?S094L21ZQlVJWHlmRFExQVNxQ3hrV1plT2wyV1JraXQvNWhLT1ZsTHpJR3VV?=
 =?utf-8?B?cDI4bDhlR0RPL2dqaXE1bFdVZnEvU2VTQ3FLcER5WVJCVy9STWN1MXdmaTRp?=
 =?utf-8?B?cm1IVkliYnM5dHFDMFM3VTBOcUVjZHNxYTZyWm1DYW1yZVRPN3NGRlZKQWND?=
 =?utf-8?B?UHoyV1l1UVNGT1RjaGlzUXVLZDJyOXZ0UDF1K2xGR29tVVBaZ3dnL0xpU2Nm?=
 =?utf-8?B?T2RMRHN0N0ZYZExIL05ZTlF6anZrd1F4OEFoNXhrUnJlUHNzVkNqVHBCdWcv?=
 =?utf-8?B?V2x0cys5ZDh0cVRrSUVQV21PZ0FVQWZudDlYZzhTNit0UVhvQzRQb0U1OTN5?=
 =?utf-8?B?ZFB1UDFKeDIrbXM4TFYxdE4xbVgyK0tHUnFsYUcyV1RzQnNNdms4NDFnT3Bj?=
 =?utf-8?B?Z0ZnRHNEUDBUanE1WXg3U0lXSVB5aEl2dmZkUWRYVG8xZmlrQzFEUjE1THpL?=
 =?utf-8?B?N3J6eVVaSS9Rb0hNeXhtbW40ZmZoTDVGaFdJT3hsQy9ZdVg0UVllWDZyZ3pP?=
 =?utf-8?B?SmcyQ3doQk15U0ZacktnMHBWMW9ZSmluK294Y1NTYktNRWUxdjF2Z3duRXh2?=
 =?utf-8?B?dTJZRnFTVXZ6ZGFCZXNsTzE4ZnU3c0RCY2prN3pwQ3NTTnhRbXVRRzQzTkdh?=
 =?utf-8?B?Z25VZzcyTnl5THRKNTVyYisrQ0N1UVNnL3V3d2NWKy90a1VTMmlZWTF0ZUVv?=
 =?utf-8?B?b3NEN1pBVmdzTU5qZjU4V0d6eW4zaVJqZkk0d01CTjQxYkhjUVBaa0pONEt6?=
 =?utf-8?B?cnBqUjkxbFlTQlNFNHB1QnYzeE5XeGVZajh0bHlEREJQVUlVN25xYTRTcCtP?=
 =?utf-8?Q?vZJV2JfhADbLA1qehnwrU8oiF?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 22749874-ed92-4d10-c4a7-08dc6b0ba31b
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 May 2024 00:54:50.6768
 (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: M2KP4AdyzSBJDoI/geOFRQoArhJqIORibqQ+UUPh9FTsHuJ1awq2D2cd82V75CEJ+l4ztXnj4OhbssQG420b4g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4808
X-OriginatorOrg: intel.com


On 3/05/2024 12:29 pm, Edgecombe, Rick P wrote:
> On Fri, 2024-05-03 at 12:18 +1200, Huang, Kai wrote:
>>
>>
>> On 3/05/2024 12:09 pm, Edgecombe, Rick P wrote:
>>> On Sat, 2024-03-02 at 00:20 +1300, Kai Huang wrote:
>>>> The kernel reads all TDMR related global metadata fields based on a
>>>> table which maps the metadata fields to the corresponding members of
>>>> 'struct tdx_tdmr_sysinfo'.
>>>>
>>>> Currently this table is a static variable.  But this table is only used
>>>> by the function which reads these metadata fields and becomes useless
>>>> after reading is done.
>>>>
>>>> Change the table to function local variable.  This also saves the
>>>> storage of the table from the kernel image.
>>>
>>> It seems like a reasonable change, but I don't see how it helps the purpose
>>> of
>>> this series. It seems more like generic cleanup. Can you explain?
>>
>> It doesn't help KVM from exporting API's perspective.
>>
>> I just uses this series for some small improvement (that I believe) of
>> the current code too.
>>
>> I can certainly drop this if you don't want it, but it's just a small
>> change and I don't see the benefit of sending it out separately.
> 
> The change makes sense to me by itself, but it needs to be called out as
> unrelated cleanup. Otherwise it will be confusing to anyone reviewing this from
> the perspective of something KVM TDX needs.
> 
> Don't have a super strong opinion. But given the choice, I would prefer it gets
> separated, because to me it's a lower priority then the rest (which is high).

OK I'll drop this one.