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: <49227bcd-3e54-4a4d-5416-dcb9315a1802@amd.com>
Date:   Tue, 4 Oct 2022 12:12:10 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] KVM: SVM: Fix reserved fields of struct sev_es_save_area
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>
Cc:     tglx@linutronix.de, bp@alien8.de, mingo@redhat.com,
        dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
        venu.busireddy@oracle.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        "Lendacky, Thomas" <Thomas.Lendacky@amd.com>, bilbao@vt.edu
References: <986a5886-4ddc-aa88-db52-e8781ec95aed@amd.com>
 <dd357565-d428-499f-fa0e-e35aa043449f@amd.com> <YzxfdQ7DrT9X6t7j@google.com>
From:   Carlos Bilbao <carlos.bilbao@amd.com>
In-Reply-To: <YzxfdQ7DrT9X6t7j@google.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?UWpSbWEzdFJyK2xWRm9GWmkzZ1lHT2pOazNyVGFxbTJ2SVFjOVFhTFVtWTBX?=
 =?utf-8?B?Y2MzdGF6bjhnMjRxSkRjakc3eExlYjQvc1BHL0hURmRqWmV3WERUdDdZWGNL?=
 =?utf-8?B?R2owZ0tOODRTWm9sdm1xZDUvMjdIb1pxMFF0ZFJ5K0VTck8zNVZyMER5Yk90?=
 =?utf-8?B?UUFESnZMTDR5dHM3cm13cE5Xb0R3RVU5Y1g2aEJJUFlFT0VyNGZ5OTU5MFpD?=
 =?utf-8?B?eVU2VnFNUkN6YUJ4NVl2ZGlHWmNXTmJwQUNFbVA1K21SY3RTSk5PSVZmVE13?=
 =?utf-8?B?eGJUVG1OZ3gxbUVvLzIzcVk1Zk85OVZtcTdubVVCUjdBcW5PTHNYaEJOdDlu?=
 =?utf-8?B?d3BRRHRGRnR4SXpOUldoQVJjemJTd2NxUzdRQitBdUNudzRCTzA4WHVvMlU0?=
 =?utf-8?B?dDVZZlJRT1gwbXgzRTRIZlA3cWhDcEhGN040NjZxV3IvTElXRmUvcHdIbUtC?=
 =?utf-8?B?VXhIb000ekJQOTZPbm1sM1ZJUUdZS0wrUTk2UFI4TFVYNksrai9SaHdqNGFi?=
 =?utf-8?B?Zy9yYTNNaVMwQlRtZWJaYVRyRnJMVkZlLzBSQzJxR3AxSjlEaXd5UHhucEkr?=
 =?utf-8?B?d3p6RXh4Q2RjbjlBdlU4MXRuWm9JTEtDbnN2dm91VndhcWNxOXpMTWREZHFm?=
 =?utf-8?B?REIyQ2lXSUh6c2FyOFlwbUpoV3dnVEVXTVlGdG9uZ0hicDJXZjE0aUN5RXBk?=
 =?utf-8?B?NmtrRzVSQTVTVDBMbVJraHB2cTBNV1BrbllqUkprZkNmSW9weHVRblZNUVQz?=
 =?utf-8?B?bU1PWlhJakRBOVVPSUJKUmZ2ODVoaklYTzRLaThyZVRKV1RHMElyWXpWQnl2?=
 =?utf-8?B?R293STRVNTJOM0o2NzhFNi8weENINWdTSDhwM0I1b0hOM2g4OHE1b3o1RVJw?=
 =?utf-8?B?UjlwQ0tlY2hjWlI3Q0JkS040OHR1SVM2eXlkbHJqS1RnenRwWG1HRWphZkZh?=
 =?utf-8?B?UXpPMWdYREJQY3pmRXZjaGhXb0FkUlVRTHlZZDZXb1pjamcrRVErNjlxblg3?=
 =?utf-8?B?UFBxL2lRQ0wyNTR0QTJMWUc1VDMrV1ZGUVVtREVHWTk0T2pacFNwVWJvQnhz?=
 =?utf-8?B?RUJtTkJxb2V3WW94QTBrQWhxS0kxN0xFM09zTHBhLzBBejJSY0QvczgrR2ZE?=
 =?utf-8?B?S2pYekNsWnZzTEpnTFpxZ0h3RGY0SG1WZjh1eElFRXFsdnJxeEhBcXc3V2Zv?=
 =?utf-8?B?cCtIWkFMY1B1SUMvYnIrb2xyd1ZPOG5TR1pLUUExWkZiNzM3RlRRb3RveWtr?=
 =?utf-8?B?QUVLT05oY3V1SWQ5VXVKTzVGQjFqSmZtZnBCQUJiQS9XVDIrWVJsV0RKS1hh?=
 =?utf-8?B?NEpXTUxFY1RSb1I5bEhieG5PVmlIcWNKTll0Nm9oT05nSDZEeDY1MVVmOS91?=
 =?utf-8?B?dGowSFJWUU1lQS9EKzJxbFZINUdGMXE4eElRS0Z1OHRlaUgySG5iOHRhZTAx?=
 =?utf-8?B?VURGcnI0Q1FoOFhBakZpL1JQVGhKVWl3MTFWLzFGNyt1ZDl4dFY4UURlTTEx?=
 =?utf-8?B?SHk4Z2xuUFp5VEJHUHhObXIyVTZtbGhmWkhDckVUWmEzKzl1ZEh4a3N3QXVp?=
 =?utf-8?B?TGp4RmpVM0dRQnNXOUFleE42cTNqMEdXOWFGQzRybkJpM053dGtGMmQ3TEwx?=
 =?utf-8?B?TGszc2FpNHVQaUtuN3cxcFBWNTNqZHpuUVpmUlNrd3k5d2xTVTNMVTZCSldI?=
 =?utf-8?B?M25PVHI3ajJLcUcwUjNxaU91c1JoR1ErY1ZyL2hMS0N2RXJBUUhpSU1Ec2pJ?=
 =?utf-8?B?bDdEcmlNOFZ5dlZVN2lnTGJsS1RlU2FpNmZuYzhrVnAzbjNpOU5XUEtOWHlk?=
 =?utf-8?B?c3pndEIxb0lZMFJoMmRIalFveDVzK3hwMzVMeUkrNEVhN1NzL3l1Zm1IZ0Iv?=
 =?utf-8?B?R00vYkt6bFNrbzVJTWh4ZVZIdm1FaUsxQUpMU3pCY1JYNEg0emNsWFAyVm92?=
 =?utf-8?B?bjh0WHV4cVlTRDdEVVJEUS9RU3BBM25ZL0RCRk5EdnZvUFQ2MVBtMXNzSDlq?=
 =?utf-8?B?OGRCM3FiS1Fmb2xGbGozVGVXdDdzTjgzN05LRmYzQithTHdwci9xUHNiQm9h?=
 =?utf-8?B?emxZYi8wN2RrdVJKOUw4UnFXNE4wT0h4TlpMM2lkMFNURG03cm82Z1ZkVzNW?=
 =?utf-8?Q?KD6UlZZoW9OSp6fqfsWIXV0eP?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7853656a-8188-4aa4-0d43-08daa62b94e4
X-MS-Exchange-CrossTenant-AuthSource: BL1PR12MB5874.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Oct 2022 17:12:13.9530
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: AcLX/V5hR7QiOLIE5SOUzpnhNvdSiiHV5QvtU1qDDA/cRALIw4QDUirvdMJ32bT/bVF6nzInhLI9dgOFHbefiA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6212
X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 10/4/22 11:29, Sean Christopherson wrote:
> On Tue, Oct 04, 2022, Carlos Bilbao wrote:
>> On 10/4/22 09:05, Carlos Bilbao wrote:
>>
>>> Reserved fields of struct sev_es_save_area are named by their order of
>>> appearance, but right now they jump from reserved_5 to reserved_7. Rename
>>> them with the correct order.
>>>
>>> Fixes: 6d3b3d34e39eb ("KVM: SVM: Update the SEV-ES save area mapping")
>> Actually, there is no bug, so this Fix tag could go. Thanks!!
> Fixes: is appropriate, if we think it's worth fixing.  Personally, I don't think
> it's worth the churn/effort to keep the reserved numbers accurate, e.g. if the
> two bytes in reserved_1 are used, then every other field will need to be updated
> just to accomodate a tiny change.  We'll find ourselves in a similar situation if
> field is added in the middle of reserved_3,
>
> If we really want to the number to have any kind of meaning without needing a pile
> of churn for every update, the best idea I can think of is to name them reserved_<offset>.
> That way only the affected reserved field needs to be modified when adding new
> legal fields.  But that has it's own flavor of maintenance burden as calculating
> and verifying the offset is a waste of everyone's time.
>
> TL;DR: I vote to sweep this under the rug and live with arbitrary/bad numbers.
Well, the discussion on what is the most appropriate way to name reserved
fields is orthogonal to this patch, IMO.

This change just follows the prior approach (reserved_x), but correctly.
Keep in mind that the existence of reserved_{1,5} and reserved_{7,11}
implies there's a reserved_6 (there isn't). Why knowingly keep something
that's wrong, even if small?

If the maintainers think this is worth changing, I will submit a new patch
without the "Fix" tag (nothing was broken) and with a new subject line
(instead of "Fix reserved fields" I will use "Order correctly reserved
fields").

Thanks,
Carlos