linux-kernel - Re: [PATCH v4 6/8] x86/sev: Treat the contiguous RMP table as a single RMP segment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9d1792de-ef5e-b1c2-cbbe-55b1ef616341@amd.com>
Date: Fri, 25 Oct 2024 08:43:45 -0500
From: Tom Lendacky <thomas.lendacky@....com>
To: "Kalra, Ashish" <ashish.kalra@....com>, linux-kernel@...r.kernel.org,
 x86@...nel.org
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 Michael Roth <michael.roth@....com>, Nikunj A Dadhania <nikunj@....com>,
 Neeraj Upadhyay <Neeraj.Upadhyay@....com>
Subject: Re: [PATCH v4 6/8] x86/sev: Treat the contiguous RMP table as a
 single RMP segment

On 10/25/24 06:51, Kalra, Ashish wrote:
> On 10/23/2024 1:42 PM, Tom Lendacky wrote:
>> In preparation for support of a segmented RMP table, treat the contiguous
>> RMP table as a segmented RMP table with a single segment covering all
>> of memory. By treating a contiguous RMP table as a single segment, much
>> of the code that initializes and accesses the RMP can be re-used.
>>
>> Segmented RMP tables can have up to 512 segment entries. Each segment
>> will have metadata associated with it to identify the segment location,
>> the segment size, etc. The segment data and the physical address are used
>> to determine the index of the segment within the table and then the RMP
>> entry within the segment. For an actual segmented RMP table environment,
>> much of the segment information will come from a configuration MSR. For
>> the contiguous RMP, though, much of the information will be statically
>> defined.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@....com>
>> ---
>>  arch/x86/virt/svm/sev.c | 193 ++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 174 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index dd256e76e443..043b2582e10e 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c

>> -	return rmptable + pfn;
>> +	segment_index = RMP_ENTRY_INDEX(paddr);
>> +	if (unlikely(segment_index >= desc->max_index))
>> +		return ERR_PTR(-EFAULT);
>> +	segment_index = array_index_nospec(segment_index, desc->max_index);
>> +
>> +	return desc->rmp_entry + segment_index;
>>  }
>>  
> 
> Ah ok, __get_rmpentry() is finalized here in this patch and rmptable & rmptable_max_pfn 
> are removed as part of this patch, so probably my comments on patch #1 are not relevant. 
> 
> This is somewhat confusing, maybe some comments can be added in patch #1 to explain that
> __get_rmpentry() will be finalized in this patch ?
> 
> My confusion results from the following : 
> 
> Patch #1: 
> __get_rmpentry() 
> --- refers to rmptable.
> 
> Patch #5:
> rmptable initialized here.
> 
> Patch #6:
> rmptable not required and removed in this patch.
> do final cleanup for __get_rmpentry(). 

Patch #1 was just a restructuring, rmptable_max_pfn and rmptable are both
initialized in snp_rmptable_init() as they previously were. So no real
explanation needed.

Thanks,
Tom

> 
> Thanks,
> Ashish
> 
>>  static int get_rmpentry(u64 pfn, struct rmpread *entry)