The RMPREAD instruction returns an architecture defined format of an
RMP entry. This is the preferred method for examining RMP entries.
In preparation for using the RMPREAD instruction, convert the existing
code that directly accesses the RMP to map the raw RMP information into
the architecture defined format.
RMPREAD output returns a status bit for the 2MB region status. If the
input page address is 2MB aligned and any other pages within the 2MB
region are assigned, then 2MB region status will be set to 1. Otherwise,
the 2MB region status will be set to 0. For systems that do not support
RMPREAD, calculating this value would require looping over all of the RMP
table entries within that range until one is found with the assigned bit
set. Since this bit is not defined in the current format, and so not used
today, do not incur the overhead associated with calculating it.
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
arch/x86/virt/svm/sev.c | 121 +++++++++++++++++++++++++++++-----------
1 file changed, 87 insertions(+), 34 deletions(-)
diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
index 0ce17766c0e5..4d095affdb4d 100644
--- a/arch/x86/virt/svm/sev.c
+++ b/arch/x86/virt/svm/sev.c
@@ -30,6 +30,22 @@
#include <asm/cmdline.h>
#include <asm/iommu.h>
+/*
+ * The RMP entry format as returned by the RMPREAD instruction.
+ */
+struct rmpread {
+ u64 gpa;
+ u8 assigned :1,
+ rsvd1 :7;
+ u8 pagesize :1,
+ hpage_region_status :1,
+ rsvd2 :6;
+ u8 immutable :1,
+ rsvd3 :7;
+ u8 rsvd4;
+ u32 asid;
+} __packed;
+
/*
* The RMP entry format is not architectural. The format is defined in PPR
* Family 19h Model 01h, Rev B1 processor.
@@ -270,48 +286,77 @@ static int __init snp_rmptable_init(void)
*/
device_initcall(snp_rmptable_init);
-static struct rmpentry *get_rmpentry(u64 pfn)
+static struct rmpentry *__get_rmpentry(u64 pfn)
{
- if (WARN_ON_ONCE(pfn > rmptable_max_pfn))
- return ERR_PTR(-EFAULT);
-
- return &rmptable[pfn];
-}
-
-static struct rmpentry *__snp_lookup_rmpentry(u64 pfn, int *level)
-{
- struct rmpentry *large_entry, *entry;
-
- if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+ if (!rmptable)
return ERR_PTR(-ENODEV);
- entry = get_rmpentry(pfn);
- if (IS_ERR(entry))
- return entry;
+ if (unlikely(pfn > rmptable_max_pfn))
+ return ERR_PTR(-EFAULT);
+
+ return rmptable + pfn;
+}
+
+static int get_rmpentry(u64 pfn, struct rmpread *entry)
+{
+ struct rmpentry *e;
+
+ e = __get_rmpentry(pfn);
+ if (IS_ERR(e))
+ return PTR_ERR(e);
+
+ /*
+ * Map the RMP table entry onto the RMPREAD output format.
+ * The 2MB region status indicator (hpage_region_status field) is not
+ * calculated, since the overhead could be significant and the field
+ * is not used.
+ */
+ memset(entry, 0, sizeof(*entry));
+ entry->gpa = e->gpa << PAGE_SHIFT;
+ entry->asid = e->asid;
+ entry->assigned = e->assigned;
+ entry->pagesize = e->pagesize;
+ entry->immutable = e->immutable;
+
+ return 0;
+}
+
+static int __snp_lookup_rmpentry(u64 pfn, struct rmpread *entry, int *level)
+{
+ struct rmpread large_entry;
+ int ret;
+
+ if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+ return -ENODEV;
+
+ ret = get_rmpentry(pfn, entry);
+ if (ret)
+ return ret;
/*
* Find the authoritative RMP entry for a PFN. This can be either a 4K
* RMP entry or a special large RMP entry that is authoritative for a
* whole 2M area.
*/
- large_entry = get_rmpentry(pfn & PFN_PMD_MASK);
- if (IS_ERR(large_entry))
- return large_entry;
+ ret = get_rmpentry(pfn & PFN_PMD_MASK, &large_entry);
+ if (ret)
+ return ret;
- *level = RMP_TO_PG_LEVEL(large_entry->pagesize);
+ *level = RMP_TO_PG_LEVEL(large_entry.pagesize);
- return entry;
+ return 0;
}
int snp_lookup_rmpentry(u64 pfn, bool *assigned, int *level)
{
- struct rmpentry *e;
+ struct rmpread rmpread;
+ int ret;
- e = __snp_lookup_rmpentry(pfn, level);
- if (IS_ERR(e))
- return PTR_ERR(e);
+ ret = __snp_lookup_rmpentry(pfn, &rmpread, level);
+ if (ret)
+ return ret;
- *assigned = !!e->assigned;
+ *assigned = !!rmpread.assigned;
return 0;
}
EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
@@ -324,18 +369,26 @@ EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
*/
static void dump_rmpentry(u64 pfn)
{
- u64 pfn_i, pfn_end;
+ struct rmpread rmpread;
struct rmpentry *e;
- int level;
+ u64 pfn_i, pfn_end;
+ int level, ret;
- e = __snp_lookup_rmpentry(pfn, &level);
- if (IS_ERR(e)) {
- pr_err("Failed to read RMP entry for PFN 0x%llx, error %ld\n",
- pfn, PTR_ERR(e));
+ ret = __snp_lookup_rmpentry(pfn, &rmpread, &level);
+ if (ret) {
+ pr_err("Failed to read RMP entry for PFN 0x%llx, error %d\n",
+ pfn, ret);
return;
}
- if (e->assigned) {
+ if (rmpread.assigned) {
+ e = __get_rmpentry(pfn);
+ if (IS_ERR(e)) {
+ pr_err("Failed to read RMP contents for PFN 0x%llx, error %ld\n",
+ pfn, PTR_ERR(e));
+ return;
+ }
+
pr_info("PFN 0x%llx, RMP entry: [0x%016llx - 0x%016llx]\n",
pfn, e->lo, e->hi);
return;
@@ -356,9 +409,9 @@ static void dump_rmpentry(u64 pfn)
pfn, pfn_i, pfn_end);
while (pfn_i < pfn_end) {
- e = __snp_lookup_rmpentry(pfn_i, &level);
+ e = __get_rmpentry(pfn_i);
if (IS_ERR(e)) {
- pr_err("Error %ld reading RMP entry for PFN 0x%llx\n",
+ pr_err("Error %ld reading RMP contents for PFN 0x%llx\n",
PTR_ERR(e), pfn_i);
pfn_i++;
continue;
--
2.46.2On Wed, Oct 23, 2024 at 01:41:55PM -0500, Tom Lendacky wrote:
> +/*
> + * The RMP entry format as returned by the RMPREAD instruction.
> + */
> +struct rmpread {
Hmm, I'm not sure this is better. "rmread" is an instruction but then you have
a struct called this way too. Strange. :-\
I think you almost had it already with a little more explanations:
https://lore.kernel.org/r/20241018111043.GAZxJCM8DK-wEjDJZR@fat_crate.local
The convention being that the _raw entry is what's in the actual table and
rmpentry is what RMPREAD returns. I think this is waaay more natural.
Hmmm.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 10/25/24 07:09, Borislav Petkov wrote:
> On Wed, Oct 23, 2024 at 01:41:55PM -0500, Tom Lendacky wrote:
>> +/*
>> + * The RMP entry format as returned by the RMPREAD instruction.
>> + */
>> +struct rmpread {
>
> Hmm, I'm not sure this is better. "rmread" is an instruction but then you have
> a struct called this way too. Strange. :-\
>
> I think you almost had it already with a little more explanations:
>
> https://lore.kernel.org/r/20241018111043.GAZxJCM8DK-wEjDJZR@fat_crate.local
>
> The convention being that the _raw entry is what's in the actual table and
> rmpentry is what RMPREAD returns. I think this is waaay more natural.
>
> Hmmm.
Just wanted to show you what it looks like. There still is a lot of change
because of the new argument and using a structure now instead of the
direct entry.
I can change back and maybe add some more detail above the struct names if
that suffices.
Thanks,
Tom
>
On Fri, Oct 25, 2024 at 08:47:06AM -0500, Tom Lendacky wrote:
> Just wanted to show you what it looks like. There still is a lot of change
> because of the new argument and using a structure now instead of the
> direct entry.
Ah ok.
> I can change back and maybe add some more detail above the struct names if
> that suffices.
Yeah, I think "struct rmpread" is simply a strange thing to have when there is
an instruction of that name. I think the naming of the structs should aim at
being descriptive as to what they are. Hypothetical example:
struct rmpentry_as_found_in_the_rmptable ...
struct rmpentry_as_returned_by_rmpread
Now make the above shorter by keeping the information. :-)
IOW, on the one hand, "rmpentry_raw" or "rmpentry_direct" or
"rmpentry_<something>", and, OTOH, just "struct rmpentry" kinda makes sense
and it is short enough.
At least how I see it rn...
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 10/25/24 08:47, Tom Lendacky wrote:
> On 10/25/24 07:09, Borislav Petkov wrote:
>> On Wed, Oct 23, 2024 at 01:41:55PM -0500, Tom Lendacky wrote:
>>> +/*
>>> + * The RMP entry format as returned by the RMPREAD instruction.
>>> + */
>>> +struct rmpread {
>>
>> Hmm, I'm not sure this is better. "rmread" is an instruction but then you have
>> a struct called this way too. Strange. :-\
>>
>> I think you almost had it already with a little more explanations:
>>
>> https://lore.kernel.org/r/20241018111043.GAZxJCM8DK-wEjDJZR@fat_crate.local
And why am I not getting the replies you made to the v2 series but getting
replies to the v3 and v4 series... very frustrating... can't find them in
quarantine or anywhere, dang email system.
Thanks,
Tom
>>
>> The convention being that the _raw entry is what's in the actual table and
>> rmpentry is what RMPREAD returns. I think this is waaay more natural.
>>
>> Hmmm.
>
> Just wanted to show you what it looks like. There still is a lot of change
> because of the new argument and using a structure now instead of the
> direct entry.
>
> I can change back and maybe add some more detail above the struct names if
> that suffices.
>
> Thanks,
> Tom
>
>>
On Fri, Oct 25, 2024 at 08:56:09AM -0500, Tom Lendacky wrote:
> And why am I not getting the replies you made to the v2 series but getting
> replies to the v3 and v4 series... very frustrating... can't find them in
> quarantine or anywhere, dang email system.
You can always switch to an email system which works for kernel development
and use the corporate thing for something else.
Like I do.
:-)
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
,
On 10/23/2024 1:41 PM, Tom Lendacky wrote:
> The RMPREAD instruction returns an architecture defined format of an
> RMP entry. This is the preferred method for examining RMP entries.
>
> In preparation for using the RMPREAD instruction, convert the existing
> code that directly accesses the RMP to map the raw RMP information into
> the architecture defined format.
>
> RMPREAD output returns a status bit for the 2MB region status. If the
> input page address is 2MB aligned and any other pages within the 2MB
> region are assigned, then 2MB region status will be set to 1. Otherwise,
> the 2MB region status will be set to 0. For systems that do not support
> RMPREAD, calculating this value would require looping over all of the RMP
> table entries within that range until one is found with the assigned bit
> set. Since this bit is not defined in the current format, and so not used
> today, do not incur the overhead associated with calculating it.
>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> arch/x86/virt/svm/sev.c | 121 +++++++++++++++++++++++++++++-----------
> 1 file changed, 87 insertions(+), 34 deletions(-)
>
> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
> index 0ce17766c0e5..4d095affdb4d 100644
> --- a/arch/x86/virt/svm/sev.c
> +++ b/arch/x86/virt/svm/sev.c
> @@ -30,6 +30,22 @@
> #include <asm/cmdline.h>
> #include <asm/iommu.h>
>
> +/*
> + * The RMP entry format as returned by the RMPREAD instruction.
> + */
> +struct rmpread {
> + u64 gpa;
> + u8 assigned :1,
> + rsvd1 :7;
> + u8 pagesize :1,
> + hpage_region_status :1,
> + rsvd2 :6;
> + u8 immutable :1,
> + rsvd3 :7;
> + u8 rsvd4;
> + u32 asid;
> +} __packed;
> +
> /*
> * The RMP entry format is not architectural. The format is defined in PPR
> * Family 19h Model 01h, Rev B1 processor.
> @@ -270,48 +286,77 @@ static int __init snp_rmptable_init(void)
> */
> device_initcall(snp_rmptable_init);
>
> -static struct rmpentry *get_rmpentry(u64 pfn)
> +static struct rmpentry *__get_rmpentry(u64 pfn)
> {
> - if (WARN_ON_ONCE(pfn > rmptable_max_pfn))
> - return ERR_PTR(-EFAULT);
> -
> - return &rmptable[pfn];
> -}
> -
> -static struct rmpentry *__snp_lookup_rmpentry(u64 pfn, int *level)
> -{
> - struct rmpentry *large_entry, *entry;
> -
> - if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
> + if (!rmptable)
> return ERR_PTR(-ENODEV);
>
> - entry = get_rmpentry(pfn);
> - if (IS_ERR(entry))
> - return entry;
> + if (unlikely(pfn > rmptable_max_pfn))
rmptable_max_pfn is initialized in patch#5, shouldn't this patch be moved after patch #5 ?
> + return ERR_PTR(-EFAULT);
> +
> + return rmptable + pfn;
Again, rmptable is initialized/setup in patch#5, similarly shouldn't this patch be moved
after patch #5 ?
Thanks,
Ashish
> +}
> +
> +static int get_rmpentry(u64 pfn, struct rmpread *entry)
> +{
> + struct rmpentry *e;
> +
> + e = __get_rmpentry(pfn);
> + if (IS_ERR(e))
> + return PTR_ERR(e);
> +
> + /*
> + * Map the RMP table entry onto the RMPREAD output format.
> + * The 2MB region status indicator (hpage_region_status field) is not
> + * calculated, since the overhead could be significant and the field
> + * is not used.
> + */
> + memset(entry, 0, sizeof(*entry));
> + entry->gpa = e->gpa << PAGE_SHIFT;
> + entry->asid = e->asid;
> + entry->assigned = e->assigned;
> + entry->pagesize = e->pagesize;
> + entry->immutable = e->immutable;
> +
> + return 0;
> +}
> +
> +static int __snp_lookup_rmpentry(u64 pfn, struct rmpread *entry, int *level)
> +{
> + struct rmpread large_entry;
> + int ret;
> +
> + if (!cc_platform_has(CC_ATTR_HOST_SEV_SNP))
> + return -ENODEV;
> +
> + ret = get_rmpentry(pfn, entry);
> + if (ret)
> + return ret;
>
> /*
> * Find the authoritative RMP entry for a PFN. This can be either a 4K
> * RMP entry or a special large RMP entry that is authoritative for a
> * whole 2M area.
> */
> - large_entry = get_rmpentry(pfn & PFN_PMD_MASK);
> - if (IS_ERR(large_entry))
> - return large_entry;
> + ret = get_rmpentry(pfn & PFN_PMD_MASK, &large_entry);
> + if (ret)
> + return ret;
>
> - *level = RMP_TO_PG_LEVEL(large_entry->pagesize);
> + *level = RMP_TO_PG_LEVEL(large_entry.pagesize);
>
> - return entry;
> + return 0;
> }
>
> int snp_lookup_rmpentry(u64 pfn, bool *assigned, int *level)
> {
> - struct rmpentry *e;
> + struct rmpread rmpread;
> + int ret;
>
> - e = __snp_lookup_rmpentry(pfn, level);
> - if (IS_ERR(e))
> - return PTR_ERR(e);
> + ret = __snp_lookup_rmpentry(pfn, &rmpread, level);
> + if (ret)
> + return ret;
>
> - *assigned = !!e->assigned;
> + *assigned = !!rmpread.assigned;
> return 0;
> }
> EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
> @@ -324,18 +369,26 @@ EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
> */
> static void dump_rmpentry(u64 pfn)
> {
> - u64 pfn_i, pfn_end;
> + struct rmpread rmpread;
> struct rmpentry *e;
> - int level;
> + u64 pfn_i, pfn_end;
> + int level, ret;
>
> - e = __snp_lookup_rmpentry(pfn, &level);
> - if (IS_ERR(e)) {
> - pr_err("Failed to read RMP entry for PFN 0x%llx, error %ld\n",
> - pfn, PTR_ERR(e));
> + ret = __snp_lookup_rmpentry(pfn, &rmpread, &level);
> + if (ret) {
> + pr_err("Failed to read RMP entry for PFN 0x%llx, error %d\n",
> + pfn, ret);
> return;
> }
>
> - if (e->assigned) {
> + if (rmpread.assigned) {
> + e = __get_rmpentry(pfn);
> + if (IS_ERR(e)) {
> + pr_err("Failed to read RMP contents for PFN 0x%llx, error %ld\n",
> + pfn, PTR_ERR(e));
> + return;
> + }
> +
> pr_info("PFN 0x%llx, RMP entry: [0x%016llx - 0x%016llx]\n",
> pfn, e->lo, e->hi);
> return;
> @@ -356,9 +409,9 @@ static void dump_rmpentry(u64 pfn)
> pfn, pfn_i, pfn_end);
>
> while (pfn_i < pfn_end) {
> - e = __snp_lookup_rmpentry(pfn_i, &level);
> + e = __get_rmpentry(pfn_i);
> if (IS_ERR(e)) {
> - pr_err("Error %ld reading RMP entry for PFN 0x%llx\n",
> + pr_err("Error %ld reading RMP contents for PFN 0x%llx\n",
> PTR_ERR(e), pfn_i);
> pfn_i++;
> continue;
On 10/25/24 04:00, Kalra, Ashish wrote: > , > On 10/23/2024 1:41 PM, Tom Lendacky wrote: >> The RMPREAD instruction returns an architecture defined format of an >> RMP entry. This is the preferred method for examining RMP entries. >> >> In preparation for using the RMPREAD instruction, convert the existing >> code that directly accesses the RMP to map the raw RMP information into >> the architecture defined format. >> >> RMPREAD output returns a status bit for the 2MB region status. If the >> input page address is 2MB aligned and any other pages within the 2MB >> region are assigned, then 2MB region status will be set to 1. Otherwise, >> the 2MB region status will be set to 0. For systems that do not support >> RMPREAD, calculating this value would require looping over all of the RMP >> table entries within that range until one is found with the assigned bit >> set. Since this bit is not defined in the current format, and so not used >> today, do not incur the overhead associated with calculating it. >> >> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> >> --- >> arch/x86/virt/svm/sev.c | 121 +++++++++++++++++++++++++++++----------- >> 1 file changed, 87 insertions(+), 34 deletions(-) >> >> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c >> index 0ce17766c0e5..4d095affdb4d 100644 >> --- a/arch/x86/virt/svm/sev.c >> +++ b/arch/x86/virt/svm/sev.c >> >> - entry = get_rmpentry(pfn); >> - if (IS_ERR(entry)) >> - return entry; >> + if (unlikely(pfn > rmptable_max_pfn)) > > rmptable_max_pfn is initialized in patch#5, shouldn't this patch be moved after patch #5 ? > >> + return ERR_PTR(-EFAULT); >> + >> + return rmptable + pfn; > > Again, rmptable is initialized/setup in patch#5, similarly shouldn't this patch be moved > after patch #5 ? rmptable_max_pfn is initialized in snp_rmptable_init(), as is rmptable. Thanks, Tom > > Thanks, > Ashish >
© 2016 - 2024 Red Hat, Inc.