Re: [PATCH v3 6/8] x86/sev: Treat the contiguous RMP table as a single RMP segment

[Tom Lendacky <thomas.lendacky@amd.com>]

On 10/18/24 08:56, Tom Lendacky wrote:
> On 10/18/24 00:59, Neeraj Upadhyay wrote:
>> On 9/30/2024 8:52 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@amd.com>
>>> ---
>>>  arch/x86/virt/svm/sev.c | 195 ++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 176 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>>> index 81e21d833cf0..ebfb924652f8 100644
>>> --- a/arch/x86/virt/svm/sev.c
>>> +++ b/arch/x86/virt/svm/sev.c
>>> @@ -18,6 +18,7 @@
>>>  #include <linux/cpumask.h>
>>>  #include <linux/iommu.h>
>>>  #include <linux/amd-iommu.h>
>>> +#include <linux/nospec.h>
>>>  
>>>  #include <asm/sev.h>
>>>  #include <asm/processor.h>
>>> @@ -74,12 +75,42 @@ struct rmpentry_raw {
>>>   */
>>>  #define RMPTABLE_CPU_BOOKKEEPING_SZ	0x4000
>>>  
>>> +/*
>>> + * For a non-segmented RMP table, use the maximum physical addressing as the
>>> + * segment size in order to always arrive at index 0 in the table.
>>> + */
>>> +#define RMPTABLE_NON_SEGMENTED_SHIFT	52
>>> +
>>> +struct rmp_segment_desc {
>>> +	struct rmpentry_raw *rmp_entry;
>>> +	u64 max_index;
>>> +	u64 size;
>>> +};
>>> +
>>> +/*
>>> + * Segmented RMP Table support.
>>> + *   - The segment size is used for two purposes:
>>> + *     - Identify the amount of memory covered by an RMP segment
>>> + *     - Quickly locate an RMP segment table entry for a physical address
>>> + *
>>> + *   - The RMP segment table contains pointers to an RMP table that covers
>>> + *     a specific portion of memory. There can be up to 512 8-byte entries,
>>> + *     one pages worth.
>>> + */
>>> +static struct rmp_segment_desc **rmp_segment_table __ro_after_init;
>>> +static unsigned int rst_max_index __ro_after_init = 512;
>>> +
>>> +static u64 rmp_segment_size_max;
>>> +static unsigned int rmp_segment_coverage_shift;
>>> +static unsigned long rmp_segment_coverage_size;
>>> +static unsigned long rmp_segment_coverage_mask;
>>
>> rmp_segment_size_max is of type u64 and rmp_segment_coverage_size is 1 << 52
>> for single RMP segment. So, maybe use u64 for rmp_segment_coverage_size
>> and rmp_segment_coverage_mask also?
> 
> This is 64-bit only code where unsigned long is the same size as u64 and
> is typically preferred when dealing with numbers like this, which is why I
> use that here. It does get a bit confusing because of the use of u64 and
> unsigned long but I tried to keep things in sync between usages of the
> same type as much as possible.

But let me see what everything looks like if I unify all the fields to u64...

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>
>>
>> - Neeraj