[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9ce26117-27eb-4f3e-8ac5-a37ea5d15802@amd.com>
Date: Fri, 25 Oct 2024 04:00:47 -0500
From: "Kalra, Ashish" <ashish.kalra@....com>
To: Tom Lendacky <thomas.lendacky@....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 1/8] x86/sev: Prepare for using the RMPREAD instruction
to access the RMP
,
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@....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;
Powered by blists - more mailing lists