diff mbox series

[Part2,v6,03/49] x86/sev: Add the host SEV-SNP initialization support

Message ID 8f4eef289aba5067582d0d3535299c22a4e5c4c4.1655761627.git.ashish.kalra@amd.com (mailing list archive)
State New
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) | expand

Commit Message

Kalra, Ashish June 20, 2022, 11:02 p.m. UTC
From: Brijesh Singh <brijesh.singh@amd.com>

The memory integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). The RMP is a single data
structure shared across the system that contains one entry for every 4K
page of DRAM that may be used by SEV-SNP VMs. The goal of RMP is to
track the owner of each page of memory. Pages of memory can be owned by
the hypervisor, owned by a specific VM or owned by the AMD-SP. See APM2
section 15.36.3 for more detail on RMP.

The RMP table is used to enforce access control to memory. The table itself
is not directly writable by the software. New CPU instructions (RMPUPDATE,
PVALIDATE, RMPADJUST) are used to manipulate the RMP entries.

Based on the platform configuration, the BIOS reserves the memory used
for the RMP table. The start and end address of the RMP table must be
queried by reading the RMP_BASE and RMP_END MSRs. If the RMP_BASE and
RMP_END are not set then disable the SEV-SNP feature.

The SEV-SNP feature is enabled only after the RMP table is successfully
initialized.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 arch/x86/include/asm/disabled-features.h |   8 +-
 arch/x86/include/asm/msr-index.h         |   6 +
 arch/x86/kernel/sev.c                    | 144 +++++++++++++++++++++++
 3 files changed, 157 insertions(+), 1 deletion(-)

Comments

Peter Gonda June 21, 2022, 3:47 p.m. UTC | #1
On Mon, Jun 20, 2022 at 5:02 PM Ashish Kalra <Ashish.Kalra@amd.com> wrote:
>
> From: Brijesh Singh <brijesh.singh@amd.com>
>
> The memory integrity guarantees of SEV-SNP are enforced through a new
> structure called the Reverse Map Table (RMP). The RMP is a single data
> structure shared across the system that contains one entry for every 4K
> page of DRAM that may be used by SEV-SNP VMs. The goal of RMP is to
> track the owner of each page of memory. Pages of memory can be owned by
> the hypervisor, owned by a specific VM or owned by the AMD-SP. See APM2
> section 15.36.3 for more detail on RMP.
>
> The RMP table is used to enforce access control to memory. The table itself
> is not directly writable by the software. New CPU instructions (RMPUPDATE,
> PVALIDATE, RMPADJUST) are used to manipulate the RMP entries.
>
> Based on the platform configuration, the BIOS reserves the memory used
> for the RMP table. The start and end address of the RMP table must be
> queried by reading the RMP_BASE and RMP_END MSRs. If the RMP_BASE and
> RMP_END are not set then disable the SEV-SNP feature.
>
> The SEV-SNP feature is enabled only after the RMP table is successfully
> initialized.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  arch/x86/include/asm/disabled-features.h |   8 +-
>  arch/x86/include/asm/msr-index.h         |   6 +
>  arch/x86/kernel/sev.c                    | 144 +++++++++++++++++++++++
>  3 files changed, 157 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
> index 36369e76cc63..c1be3091a383 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -68,6 +68,12 @@
>  # define DISABLE_TDX_GUEST     (1 << (X86_FEATURE_TDX_GUEST & 31))
>  #endif
>
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> +# define DISABLE_SEV_SNP       0
> +#else
> +# define DISABLE_SEV_SNP       (1 << (X86_FEATURE_SEV_SNP & 31))
> +#endif
> +
>  /*
>   * Make sure to add features to the correct mask
>   */
> @@ -91,7 +97,7 @@
>                          DISABLE_ENQCMD)
>  #define DISABLED_MASK17        0
>  #define DISABLED_MASK18        0
> -#define DISABLED_MASK19        0
> +#define DISABLED_MASK19        (DISABLE_SEV_SNP)
>  #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
>
>  #endif /* _ASM_X86_DISABLED_FEATURES_H */
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 9e2e7185fc1d..57a8280e283a 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -507,6 +507,8 @@
>  #define MSR_AMD64_SEV_ENABLED          BIT_ULL(MSR_AMD64_SEV_ENABLED_BIT)
>  #define MSR_AMD64_SEV_ES_ENABLED       BIT_ULL(MSR_AMD64_SEV_ES_ENABLED_BIT)
>  #define MSR_AMD64_SEV_SNP_ENABLED      BIT_ULL(MSR_AMD64_SEV_SNP_ENABLED_BIT)
> +#define MSR_AMD64_RMP_BASE             0xc0010132
> +#define MSR_AMD64_RMP_END              0xc0010133
>
>  #define MSR_AMD64_VIRT_SPEC_CTRL       0xc001011f
>
> @@ -581,6 +583,10 @@
>  #define MSR_AMD64_SYSCFG               0xc0010010
>  #define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT       23
>  #define MSR_AMD64_SYSCFG_MEM_ENCRYPT   BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_EN_BIT            24
> +#define MSR_AMD64_SYSCFG_SNP_EN                BIT_ULL(MSR_AMD64_SYSCFG_SNP_EN_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT       25
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN   BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)
>  #define MSR_K8_INT_PENDING_MSG         0xc0010055
>  /* C1E active bits in int pending message */
>  #define K8_INTP_C1E_ACTIVE_MASK                0x18000000
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index f01f4550e2c6..3a233b5d47c5 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -22,6 +22,8 @@
>  #include <linux/efi.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
> +#include <linux/cpumask.h>
> +#include <linux/iommu.h>
>
>  #include <asm/cpu_entry_area.h>
>  #include <asm/stacktrace.h>
> @@ -38,6 +40,7 @@
>  #include <asm/apic.h>
>  #include <asm/cpuid.h>
>  #include <asm/cmdline.h>
> +#include <asm/iommu.h>
>
>  #define DR7_RESET_VALUE        0x400
>
> @@ -57,6 +60,12 @@
>  #define AP_INIT_CR0_DEFAULT            0x60000010
>  #define AP_INIT_MXCSR_DEFAULT          0x1f80
>
> +/*
> + * The first 16KB from the RMP_BASE is used by the processor for the
> + * bookkeeping, the range need to be added during the RMP entry lookup.
> + */
> +#define RMPTABLE_CPU_BOOKKEEPING_SZ    0x4000
> +
>  /* For early boot hypervisor communication in SEV-ES enabled guests */
>  static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE);
>
> @@ -69,6 +78,10 @@ static struct ghcb *boot_ghcb __section(".data");
>  /* Bitmap of SEV features supported by the hypervisor */
>  static u64 sev_hv_features __ro_after_init;
>
> +static unsigned long rmptable_start __ro_after_init;
> +static unsigned long rmptable_end __ro_after_init;
> +
> +
>  /* #VC handler runtime per-CPU data */
>  struct sev_es_runtime_data {
>         struct ghcb ghcb_page;
> @@ -2218,3 +2231,134 @@ static int __init snp_init_platform_device(void)
>         return 0;
>  }
>  device_initcall(snp_init_platform_device);
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt)    "SEV-SNP: " fmt
> +
> +static int __snp_enable(unsigned int cpu)
> +{
> +       u64 val;
> +
> +       if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> +               return 0;
> +
> +       rdmsrl(MSR_AMD64_SYSCFG, val);
> +
> +       val |= MSR_AMD64_SYSCFG_SNP_EN;
> +       val |= MSR_AMD64_SYSCFG_SNP_VMPL_EN;
> +
> +       wrmsrl(MSR_AMD64_SYSCFG, val);
> +
> +       return 0;
> +}
> +
> +static __init void snp_enable(void *arg)
> +{
> +       __snp_enable(smp_processor_id());
> +}
> +
> +static bool get_rmptable_info(u64 *start, u64 *len)
> +{
> +       u64 calc_rmp_sz, rmp_sz, rmp_base, rmp_end, nr_pages;
> +
> +       rdmsrl(MSR_AMD64_RMP_BASE, rmp_base);
> +       rdmsrl(MSR_AMD64_RMP_END, rmp_end);
> +
> +       if (!rmp_base || !rmp_end) {
> +               pr_info("Memory for the RMP table has not been reserved by BIOS\n");
> +               return false;
> +       }
> +
> +       rmp_sz = rmp_end - rmp_base + 1;
> +
> +       /*
> +        * Calculate the amount the memory that must be reserved by the BIOS to
> +        * address the full system RAM. The reserved memory should also cover the
> +        * RMP table itself.
> +        *
> +        * See PPR Family 19h Model 01h, Revision B1 section 2.1.4.2 for more
> +        * information on memory requirement.
> +        */
> +       nr_pages = totalram_pages();
> +       calc_rmp_sz = (((rmp_sz >> PAGE_SHIFT) + nr_pages) << 4) + RMPTABLE_CPU_BOOKKEEPING_SZ;
> +
> +       if (calc_rmp_sz > rmp_sz) {
> +               pr_info("Memory reserved for the RMP table does not cover full system RAM (expected 0x%llx got 0x%llx)\n",
> +                       calc_rmp_sz, rmp_sz);
> +               return false;
> +       }
> +
> +       *start = rmp_base;
> +       *len = rmp_sz;
> +
> +       pr_info("RMP table physical address 0x%016llx - 0x%016llx\n", rmp_base, rmp_end);
> +
> +       return true;
> +}
> +
> +static __init int __snp_rmptable_init(void)
> +{
> +       u64 rmp_base, sz;
> +       void *start;
> +       u64 val;
> +
> +       if (!get_rmptable_info(&rmp_base, &sz))
> +               return 1;
> +
> +       start = memremap(rmp_base, sz, MEMREMAP_WB);
> +       if (!start) {
> +               pr_err("Failed to map RMP table 0x%llx+0x%llx\n", rmp_base, sz);
> +               return 1;
> +       }
> +
> +       /*
> +        * Check if SEV-SNP is already enabled, this can happen if we are coming from
> +        * kexec boot.
> +        */
> +       rdmsrl(MSR_AMD64_SYSCFG, val);
> +       if (val & MSR_AMD64_SYSCFG_SNP_EN)
> +               goto skip_enable;
> +
> +       /* Initialize the RMP table to zero */
> +       memset(start, 0, sz);
> +
> +       /* Flush the caches to ensure that data is written before SNP is enabled. */
> +       wbinvd_on_all_cpus();
> +
> +       /* Enable SNP on all CPUs. */
> +       on_each_cpu(snp_enable, NULL, 1);
> +
> +skip_enable:
> +       rmptable_start = (unsigned long)start;
> +       rmptable_end = rmptable_start + sz;

Since in get_rmptable_info() `rmp_sz = rmp_end - rmp_base + 1;` should
this be `rmptable_end = rmptable_start + sz - 1;`?

> +
> +       return 0;
> +}
> +
> +static int __init snp_rmptable_init(void)
> +{
> +       if (!boot_cpu_has(X86_FEATURE_SEV_SNP))
> +               return 0;
> +
> +       if (!iommu_sev_snp_supported())
> +               goto nosnp;
> +
> +       if (__snp_rmptable_init())
> +               goto nosnp;
> +
> +       cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", __snp_enable, NULL);
> +
> +       return 0;
> +
> +nosnp:
> +       setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> +       return 1;
> +}
> +
> +/*
> + * This must be called after the PCI subsystem. This is because before enabling
> + * the SNP feature we need to ensure that IOMMU supports the SEV-SNP feature.
> + * The iommu_sev_snp_support() is used for checking the feature, and it is
> + * available after subsys_initcall().
> + */
> +fs_initcall(snp_rmptable_init);
> --
> 2.25.1
>
Kalra, Ashish June 21, 2022, 5:59 p.m. UTC | #2
[Public]

Hello Peter,

>> +static __init int __snp_rmptable_init(void) {
>> +       u64 rmp_base, sz;
>> +       void *start;
>> +       u64 val;
>> +
>> +       if (!get_rmptable_info(&rmp_base, &sz))
>> +               return 1;
>> +
>> +       start = memremap(rmp_base, sz, MEMREMAP_WB);
>> +       if (!start) {
>> +               pr_err("Failed to map RMP table 0x%llx+0x%llx\n", rmp_base, sz);
>> +               return 1;
>> +       }
>> +
>> +       /*
>> +        * Check if SEV-SNP is already enabled, this can happen if we are coming from
>> +        * kexec boot.
>> +        */
>> +       rdmsrl(MSR_AMD64_SYSCFG, val);
>> +       if (val & MSR_AMD64_SYSCFG_SNP_EN)
>> +               goto skip_enable;
>> +
>> +       /* Initialize the RMP table to zero */
>> +       memset(start, 0, sz);
>> +
>> +       /* Flush the caches to ensure that data is written before SNP is enabled. */
>> +       wbinvd_on_all_cpus();
>> +
>> +       /* Enable SNP on all CPUs. */
>> +       on_each_cpu(snp_enable, NULL, 1);
>> +
>> +skip_enable:
>> +       rmptable_start = (unsigned long)start;
>> +       rmptable_end = rmptable_start + sz;

> Since in get_rmptable_info() `rmp_sz = rmp_end - rmp_base + 1;` should this be `rmptable_end = rmptable_start + sz - 1;`?

Yes, it should be.

Thanks,
Ashish
Marc Orr June 23, 2022, 8:48 p.m. UTC | #3
On Mon, Jun 20, 2022 at 4:02 PM Ashish Kalra <Ashish.Kalra@amd.com> wrote:
>
> From: Brijesh Singh <brijesh.singh@amd.com>
>
> The memory integrity guarantees of SEV-SNP are enforced through a new
> structure called the Reverse Map Table (RMP). The RMP is a single data
> structure shared across the system that contains one entry for every 4K
> page of DRAM that may be used by SEV-SNP VMs. The goal of RMP is to
> track the owner of each page of memory. Pages of memory can be owned by
> the hypervisor, owned by a specific VM or owned by the AMD-SP. See APM2
> section 15.36.3 for more detail on RMP.
>
> The RMP table is used to enforce access control to memory. The table itself
> is not directly writable by the software. New CPU instructions (RMPUPDATE,
> PVALIDATE, RMPADJUST) are used to manipulate the RMP entries.
>
> Based on the platform configuration, the BIOS reserves the memory used
> for the RMP table. The start and end address of the RMP table must be
> queried by reading the RMP_BASE and RMP_END MSRs. If the RMP_BASE and
> RMP_END are not set then disable the SEV-SNP feature.
>
> The SEV-SNP feature is enabled only after the RMP table is successfully
> initialized.
>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  arch/x86/include/asm/disabled-features.h |   8 +-
>  arch/x86/include/asm/msr-index.h         |   6 +
>  arch/x86/kernel/sev.c                    | 144 +++++++++++++++++++++++
>  3 files changed, 157 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
> index 36369e76cc63..c1be3091a383 100644
> --- a/arch/x86/include/asm/disabled-features.h
> +++ b/arch/x86/include/asm/disabled-features.h
> @@ -68,6 +68,12 @@
>  # define DISABLE_TDX_GUEST     (1 << (X86_FEATURE_TDX_GUEST & 31))
>  #endif
>
> +#ifdef CONFIG_AMD_MEM_ENCRYPT
> +# define DISABLE_SEV_SNP       0
> +#else
> +# define DISABLE_SEV_SNP       (1 << (X86_FEATURE_SEV_SNP & 31))
> +#endif
> +
>  /*
>   * Make sure to add features to the correct mask
>   */
> @@ -91,7 +97,7 @@
>                          DISABLE_ENQCMD)
>  #define DISABLED_MASK17        0
>  #define DISABLED_MASK18        0
> -#define DISABLED_MASK19        0
> +#define DISABLED_MASK19        (DISABLE_SEV_SNP)
>  #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
>
>  #endif /* _ASM_X86_DISABLED_FEATURES_H */
> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
> index 9e2e7185fc1d..57a8280e283a 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -507,6 +507,8 @@
>  #define MSR_AMD64_SEV_ENABLED          BIT_ULL(MSR_AMD64_SEV_ENABLED_BIT)
>  #define MSR_AMD64_SEV_ES_ENABLED       BIT_ULL(MSR_AMD64_SEV_ES_ENABLED_BIT)
>  #define MSR_AMD64_SEV_SNP_ENABLED      BIT_ULL(MSR_AMD64_SEV_SNP_ENABLED_BIT)
> +#define MSR_AMD64_RMP_BASE             0xc0010132
> +#define MSR_AMD64_RMP_END              0xc0010133
>
>  #define MSR_AMD64_VIRT_SPEC_CTRL       0xc001011f
>
> @@ -581,6 +583,10 @@
>  #define MSR_AMD64_SYSCFG               0xc0010010
>  #define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT       23
>  #define MSR_AMD64_SYSCFG_MEM_ENCRYPT   BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_EN_BIT            24
> +#define MSR_AMD64_SYSCFG_SNP_EN                BIT_ULL(MSR_AMD64_SYSCFG_SNP_EN_BIT)
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT       25
> +#define MSR_AMD64_SYSCFG_SNP_VMPL_EN   BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)

nit: The alignment here looks off. The rest of the file left-aligns
the macro definition column under a comment header. The bad alignment
can be viewed on the github version of this patch:
https://github.com/AMDESE/linux/commit/5101daef92f448c046207b701c0c420b1fce3eaf

>  #define MSR_K8_INT_PENDING_MSG         0xc0010055
>  /* C1E active bits in int pending message */
>  #define K8_INTP_C1E_ACTIVE_MASK                0x18000000
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index f01f4550e2c6..3a233b5d47c5 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -22,6 +22,8 @@
>  #include <linux/efi.h>
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
> +#include <linux/cpumask.h>
> +#include <linux/iommu.h>
>
>  #include <asm/cpu_entry_area.h>
>  #include <asm/stacktrace.h>
> @@ -38,6 +40,7 @@
>  #include <asm/apic.h>
>  #include <asm/cpuid.h>
>  #include <asm/cmdline.h>
> +#include <asm/iommu.h>
>
>  #define DR7_RESET_VALUE        0x400
>
> @@ -57,6 +60,12 @@
>  #define AP_INIT_CR0_DEFAULT            0x60000010
>  #define AP_INIT_MXCSR_DEFAULT          0x1f80
>
> +/*
> + * The first 16KB from the RMP_BASE is used by the processor for the
> + * bookkeeping, the range need to be added during the RMP entry lookup.
> + */
> +#define RMPTABLE_CPU_BOOKKEEPING_SZ    0x4000
> +
>  /* For early boot hypervisor communication in SEV-ES enabled guests */
>  static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE);
>
> @@ -69,6 +78,10 @@ static struct ghcb *boot_ghcb __section(".data");
>  /* Bitmap of SEV features supported by the hypervisor */
>  static u64 sev_hv_features __ro_after_init;
>
> +static unsigned long rmptable_start __ro_after_init;
> +static unsigned long rmptable_end __ro_after_init;
> +
> +
>  /* #VC handler runtime per-CPU data */
>  struct sev_es_runtime_data {
>         struct ghcb ghcb_page;
> @@ -2218,3 +2231,134 @@ static int __init snp_init_platform_device(void)
>         return 0;
>  }
>  device_initcall(snp_init_platform_device);
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt)    "SEV-SNP: " fmt
> +
> +static int __snp_enable(unsigned int cpu)
> +{
> +       u64 val;
> +
> +       if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> +               return 0;
> +
> +       rdmsrl(MSR_AMD64_SYSCFG, val);
> +
> +       val |= MSR_AMD64_SYSCFG_SNP_EN;
> +       val |= MSR_AMD64_SYSCFG_SNP_VMPL_EN;
> +
> +       wrmsrl(MSR_AMD64_SYSCFG, val);
> +
> +       return 0;
> +}
> +
> +static __init void snp_enable(void *arg)
> +{
> +       __snp_enable(smp_processor_id());
> +}
> +
> +static bool get_rmptable_info(u64 *start, u64 *len)
> +{
> +       u64 calc_rmp_sz, rmp_sz, rmp_base, rmp_end, nr_pages;
> +
> +       rdmsrl(MSR_AMD64_RMP_BASE, rmp_base);
> +       rdmsrl(MSR_AMD64_RMP_END, rmp_end);
> +
> +       if (!rmp_base || !rmp_end) {
> +               pr_info("Memory for the RMP table has not been reserved by BIOS\n");
> +               return false;
> +       }
> +
> +       rmp_sz = rmp_end - rmp_base + 1;
> +
> +       /*
> +        * Calculate the amount the memory that must be reserved by the BIOS to
> +        * address the full system RAM. The reserved memory should also cover the
> +        * RMP table itself.
> +        *
> +        * See PPR Family 19h Model 01h, Revision B1 section 2.1.4.2 for more
> +        * information on memory requirement.
> +        */
> +       nr_pages = totalram_pages();
> +       calc_rmp_sz = (((rmp_sz >> PAGE_SHIFT) + nr_pages) << 4) + RMPTABLE_CPU_BOOKKEEPING_SZ;
> +
> +       if (calc_rmp_sz > rmp_sz) {
> +               pr_info("Memory reserved for the RMP table does not cover full system RAM (expected 0x%llx got 0x%llx)\n",
> +                       calc_rmp_sz, rmp_sz);
> +               return false;
> +       }
> +
> +       *start = rmp_base;
> +       *len = rmp_sz;
> +
> +       pr_info("RMP table physical address 0x%016llx - 0x%016llx\n", rmp_base, rmp_end);
> +
> +       return true;
> +}
> +
> +static __init int __snp_rmptable_init(void)
> +{
> +       u64 rmp_base, sz;
> +       void *start;
> +       u64 val;
> +
> +       if (!get_rmptable_info(&rmp_base, &sz))
> +               return 1;
> +
> +       start = memremap(rmp_base, sz, MEMREMAP_WB);
> +       if (!start) {
> +               pr_err("Failed to map RMP table 0x%llx+0x%llx\n", rmp_base, sz);
> +               return 1;
> +       }
> +
> +       /*
> +        * Check if SEV-SNP is already enabled, this can happen if we are coming from
> +        * kexec boot.
> +        */
> +       rdmsrl(MSR_AMD64_SYSCFG, val);
> +       if (val & MSR_AMD64_SYSCFG_SNP_EN)
> +               goto skip_enable;
> +
> +       /* Initialize the RMP table to zero */
> +       memset(start, 0, sz);
> +
> +       /* Flush the caches to ensure that data is written before SNP is enabled. */
> +       wbinvd_on_all_cpus();
> +
> +       /* Enable SNP on all CPUs. */
> +       on_each_cpu(snp_enable, NULL, 1);
> +
> +skip_enable:
> +       rmptable_start = (unsigned long)start;
> +       rmptable_end = rmptable_start + sz;
> +
> +       return 0;
> +}
> +
> +static int __init snp_rmptable_init(void)
> +{
> +       if (!boot_cpu_has(X86_FEATURE_SEV_SNP))
> +               return 0;
> +
> +       if (!iommu_sev_snp_supported())
> +               goto nosnp;
> +
> +       if (__snp_rmptable_init())
> +               goto nosnp;
> +
> +       cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", __snp_enable, NULL);
> +
> +       return 0;
> +
> +nosnp:
> +       setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> +       return 1;

Seems odd that we're returning 1 here, rather than 0. I tried to
figure out how the initcall return values are used and failed. My
impression was 0 means success and a negative number means failure.
But maybe this is normal.

> +}
> +
> +/*
> + * This must be called after the PCI subsystem. This is because before enabling
> + * the SNP feature we need to ensure that IOMMU supports the SEV-SNP feature.
> + * The iommu_sev_snp_support() is used for checking the feature, and it is
> + * available after subsys_initcall().
> + */
> +fs_initcall(snp_rmptable_init);
> --
> 2.25.1
>
Kalra, Ashish June 23, 2022, 10:22 p.m. UTC | #4
[AMD Official Use Only - General]

>> +static int __init snp_rmptable_init(void) {
>> +       if (!boot_cpu_has(X86_FEATURE_SEV_SNP))
>> +               return 0;
>> +
>> +       if (!iommu_sev_snp_supported())
>> +               goto nosnp;
>> +
>> +       if (__snp_rmptable_init())
>> +               goto nosnp;
>> +
>> +       cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, 
>> + "x86/rmptable_init:online", __snp_enable, NULL);
>> +
>> +       return 0;
>> +
>> +nosnp:
>> +       setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
>> +       return 1;

>Seems odd that we're returning 1 here, rather than 0. I tried to figure out how the initcall return values are used and failed. My impression was 0 means success and a negative number means failure.
>But maybe this is normal.

I think that initcall values are typically ignored, but it should return 0 on success and negative on error. So probably should fix this to return something like -ENOSYS instead of 1.

Thanks,
Ashish
Borislav Petkov July 17, 2022, 10:01 a.m. UTC | #5
On Mon, Jun 20, 2022 at 11:02:01PM +0000, Ashish Kalra wrote:
> +/*
> + * The first 16KB from the RMP_BASE is used by the processor for the
> + * bookkeeping, the range need to be added during the RMP entry lookup.

needs

> +static int __snp_enable(unsigned int cpu)
> +{
> +	u64 val;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> +		return 0;
> +
> +	rdmsrl(MSR_AMD64_SYSCFG, val);
> +
> +	val |= MSR_AMD64_SYSCFG_SNP_EN;
> +	val |= MSR_AMD64_SYSCFG_SNP_VMPL_EN;
> +
> +	wrmsrl(MSR_AMD64_SYSCFG, val);
> +
> +	return 0;
> +}
> +
> +static __init void snp_enable(void *arg)
> +{
> +	__snp_enable(smp_processor_id());
> +}

Get rid of that silly wrapper - you're not even using that @cpu argument.

> +static bool get_rmptable_info(u64 *start, u64 *len)
> +{
> +	u64 calc_rmp_sz, rmp_sz, rmp_base, rmp_end, nr_pages;
> +
> +	rdmsrl(MSR_AMD64_RMP_BASE, rmp_base);
> +	rdmsrl(MSR_AMD64_RMP_END, rmp_end);
> +
> +	if (!rmp_base || !rmp_end) {
> +		pr_info("Memory for the RMP table has not been reserved by BIOS\n");

pr_err

> +		return false;
> +	}
> +
> +	rmp_sz = rmp_end - rmp_base + 1;
> +
> +	/*
> +	 * Calculate the amount the memory that must be reserved by the BIOS to
> +	 * address the full system RAM. The reserved memory should also cover the

"... address the whole RAM."

> +	 * RMP table itself.
> +	 *
> +	 * See PPR Family 19h Model 01h, Revision B1 section 2.1.4.2 for more
> +	 * information on memory requirement.

That section number will change over time - if you want to refer to some
section just use its title so that people can at least grep for the
relevant text.

> +	 */
> +	nr_pages = totalram_pages();
> +	calc_rmp_sz = (((rmp_sz >> PAGE_SHIFT) + nr_pages) << 4) + RMPTABLE_CPU_BOOKKEEPING_SZ;

use totalram_pages() directly and get rid of nr_pages.

> +
> +	if (calc_rmp_sz > rmp_sz) {
> +		pr_info("Memory reserved for the RMP table does not cover full system RAM (expected 0x%llx got 0x%llx)\n",
> +			calc_rmp_sz, rmp_sz);

pr_err

> +		return false;
> +	}
> +
> +	*start = rmp_base;
> +	*len = rmp_sz;
> +
> +	pr_info("RMP table physical address 0x%016llx - 0x%016llx\n", rmp_base, rmp_end);

"RMP table physical address range: ...[0x.. - 0x..]"

> +
> +	return true;
> +}
> +
> +static __init int __snp_rmptable_init(void)

s/int/bool/

> +{
> +	u64 rmp_base, sz;
> +	void *start;
> +	u64 val;
> +
> +	if (!get_rmptable_info(&rmp_base, &sz))
> +		return 1;
> +
> +	start = memremap(rmp_base, sz, MEMREMAP_WB);
> +	if (!start) {
> +		pr_err("Failed to map RMP table 0x%llx+0x%llx\n", rmp_base, sz);
							^^^^^^

either write the size in decimal or do a normal interval.

> +		return 1;
> +	}
> +
> +	/*
> +	 * Check if SEV-SNP is already enabled, this can happen if we are coming from

Who is "we"?

Pls get rid of all "we" in the comments and use passive formulations.

> +	 * kexec boot.
> +	 */
> +	rdmsrl(MSR_AMD64_SYSCFG, val);
> +	if (val & MSR_AMD64_SYSCFG_SNP_EN)
> +		goto skip_enable;
> +
> +	/* Initialize the RMP table to zero */
> +	memset(start, 0, sz);

Do I understand it correctly that in the kexec case the second, kexec-ed
kernel is reusing the previous kernel's RMP table so it should not be
cleared?

> +
> +	/* Flush the caches to ensure that data is written before SNP is enabled. */
> +	wbinvd_on_all_cpus();
> +
> +	/* Enable SNP on all CPUs. */
> +	on_each_cpu(snp_enable, NULL, 1);
> +
> +skip_enable:
> +	rmptable_start = (unsigned long)start;
> +	rmptable_end = rmptable_start + sz;
> +
> +	return 0;
> +}
> +
> +static int __init snp_rmptable_init(void)
> +{
> +	if (!boot_cpu_has(X86_FEATURE_SEV_SNP))

cpu_feature_enabled

> +		return 0;
> +
> +	if (!iommu_sev_snp_supported())
> +		goto nosnp;
> +
> +	if (__snp_rmptable_init())
> +		goto nosnp;
> +
> +	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", __snp_enable, NULL);
> +
> +	return 0;
> +
> +nosnp:
> +	setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
> +	return 1;
> +}
> +
> +/*
> + * This must be called after the PCI subsystem. This is because before enabling
> + * the SNP feature we need to ensure that IOMMU supports the SEV-SNP feature.
> + * The iommu_sev_snp_support() is used for checking the feature, and it is
> + * available after subsys_initcall().

I'd much more appreciate here a short formulation explaining why is
IOMMU needed for SNP rather than the obvious.

Thx.
Kalra, Ashish July 19, 2022, 3:56 a.m. UTC | #6
[AMD Official Use Only - General]

Hello Boris,

>> +	 * See PPR Family 19h Model 01h, Revision B1 section 2.1.4.2 for more
>> +	 * information on memory requirement.

>That section number will change over time - if you want to refer to some section just use its title so that people can at least grep for the relevant text.

This will all go into sev.c, instead of the header file, as this is non-architectural and per-processor and the structure won't be exposed to the rest
of the kernel.  The above PPR reference and potentially in future an architectural method of reading the RMP table entries will be moved into it.

>> +	 */
>> +	nr_pages = totalram_pages();
>> +	calc_rmp_sz = (((rmp_sz >> PAGE_SHIFT) + nr_pages) << 4) + 
>> +RMPTABLE_CPU_BOOKKEEPING_SZ;

>use totalram_pages() directly and get rid of nr_pages.
Ok.

>> +	 * kexec boot.
>> +	 */
>> +	rdmsrl(MSR_AMD64_SYSCFG, val);
>> +	if (val & MSR_AMD64_SYSCFG_SNP_EN)
>> +		goto skip_enable;
>> +
>> +	/* Initialize the RMP table to zero */
>> +	memset(start, 0, sz);

>Do I understand it correctly that in the kexec case the second, kexec-ed kernel is reusing the previous kernel's RMP table so it should not be cleared?
I believe that with kexec and after issuing the shutdown command, the RMP table needs to be fully initialized, so we should be re-initializing the RMP
table to zero here.

>>
>> +
>> +static int __init snp_rmptable_init(void) {
>> +	if (!boot_cpu_has(X86_FEATURE_SEV_SNP))

>cpu_feature_enabled
Ok.

>> +		return 0;
>> +
>> +	if (!iommu_sev_snp_supported())
>> +		goto nosnp;
>> +
>> +	if (__snp_rmptable_init())
>> +		goto nosnp;
>> +
>> +	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", 
>> +__snp_enable, NULL);
>> +
>> +	return 0;
>> +
>> +nosnp:
>> +	setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
>> +	return 1;
>> +}
>> +
>> +/*
>> + * This must be called after the PCI subsystem. This is because 
>> +before enabling
>> + * the SNP feature we need to ensure that IOMMU supports the SEV-SNP feature.
>> + * The iommu_sev_snp_support() is used for checking the feature, and 
>> +it is
>> + * available after subsys_initcall().

>I'd much more appreciate here a short formulation explaining why is IOMMU needed for SNP rather than the obvious.

Yes, IOMMU is enforced for SNP to ensure that HV cannot program DMA directly into guest private memory. In case of SNP,
the IOMMU makes sure that the page(s) used for DMA are HV owned.

Thanks,
Ashish
Borislav Petkov July 19, 2022, 8:38 a.m. UTC | #7
On Tue, Jul 19, 2022 at 03:56:25AM +0000, Kalra, Ashish wrote:
> > That section number will change over time - if you want to refer to
> > some section just use its title so that people can at least grep for
> > the relevant text.
>
> This will all go into sev.c, instead of the header file, as this is
> non-architectural and per-processor and the structure won't be exposed
> to the rest of the kernel. The above PPR reference and potentially in
> future an architectural method of reading the RMP table entries will
> be moved into it.

I fail to see how this addresses my comment... All I'm saying is, the
"section 2.1.4.2" number will change so don't quote it in the text but
quote the section *name* instead.

> I believe that with kexec and after issuing the shutdown command,
> the RMP table needs to be fully initialized, so we should be
> re-initializing the RMP table to zero here.

And yet you're doing:

        /*
         * Check if SEV-SNP is already enabled, this can happen if we are coming from
         * kexec boot.
         */
        rdmsrl(MSR_AMD64_SYSCFG, val);
        if (val & MSR_AMD64_SYSCFG_SNP_EN)
                goto skip_enable;		<-------- skip zeroing


So which is it?

> Yes, IOMMU is enforced for SNP to ensure that HV cannot program DMA
> directly into guest private memory. In case of SNP, the IOMMU makes
> sure that the page(s) used for DMA are HV owned.

Yes, now put that in the comment above the

	fs_initcall(snp_rmptable_init);

line.

Thx.
Kalra, Ashish July 19, 2022, 11:34 a.m. UTC | #8
[AMD Official Use Only - General]

Hello Boris,

>> > That section number will change over time - if you want to refer to 
>> > some section just use its title so that people can at least grep for 
>> > the relevant text.
>>
>> This will all go into sev.c, instead of the header file, as this is 
>> non-architectural and per-processor and the structure won't be exposed 
>> to the rest of the kernel. The above PPR reference and potentially in 
>> future an architectural method of reading the RMP table entries will 
>> be moved into it.

>I fail to see how this addresses my comment... All I'm saying is, the "section 2.1.4.2" number will change so don't quote it in the text but quote the section *name* instead.

Yes I agree with your comments, all I am saying is that these comments will move into sev.c instead of the header file.

>> I believe that with kexec and after issuing the shutdown command, the 
>> RMP table needs to be fully initialized, so we should be 
>> re-initializing the RMP table to zero here.

>And yet you're doing:

 >       /*
 >        * Check if SEV-SNP is already enabled, this can happen if we are coming from
 >        * kexec boot.
 >        */
 >       rdmsrl(MSR_AMD64_SYSCFG, val);
 >       if (val & MSR_AMD64_SYSCFG_SNP_EN)
 >               goto skip_enable;		<-------- skip zeroing

>So which is it?

Again what I meant is that this will be fixed to reset the RMP table for kexec boot too.

>> Yes, IOMMU is enforced for SNP to ensure that HV cannot program DMA 
>> directly into guest private memory. In case of SNP, the IOMMU makes 
>> sure that the page(s) used for DMA are HV owned.

>>Yes, now put that in the comment above the

>	fs_initcall(snp_rmptable_init);

>line.

Yes.

Thanks,
Ashish
diff mbox series

Patch

diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index 36369e76cc63..c1be3091a383 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -68,6 +68,12 @@ 
 # define DISABLE_TDX_GUEST	(1 << (X86_FEATURE_TDX_GUEST & 31))
 #endif
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+# define DISABLE_SEV_SNP	0
+#else
+# define DISABLE_SEV_SNP	(1 << (X86_FEATURE_SEV_SNP & 31))
+#endif
+
 /*
  * Make sure to add features to the correct mask
  */
@@ -91,7 +97,7 @@ 
 			 DISABLE_ENQCMD)
 #define DISABLED_MASK17	0
 #define DISABLED_MASK18	0
-#define DISABLED_MASK19	0
+#define DISABLED_MASK19	(DISABLE_SEV_SNP)
 #define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 20)
 
 #endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index 9e2e7185fc1d..57a8280e283a 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -507,6 +507,8 @@ 
 #define MSR_AMD64_SEV_ENABLED		BIT_ULL(MSR_AMD64_SEV_ENABLED_BIT)
 #define MSR_AMD64_SEV_ES_ENABLED	BIT_ULL(MSR_AMD64_SEV_ES_ENABLED_BIT)
 #define MSR_AMD64_SEV_SNP_ENABLED	BIT_ULL(MSR_AMD64_SEV_SNP_ENABLED_BIT)
+#define MSR_AMD64_RMP_BASE		0xc0010132
+#define MSR_AMD64_RMP_END		0xc0010133
 
 #define MSR_AMD64_VIRT_SPEC_CTRL	0xc001011f
 
@@ -581,6 +583,10 @@ 
 #define MSR_AMD64_SYSCFG		0xc0010010
 #define MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT	23
 #define MSR_AMD64_SYSCFG_MEM_ENCRYPT	BIT_ULL(MSR_AMD64_SYSCFG_MEM_ENCRYPT_BIT)
+#define MSR_AMD64_SYSCFG_SNP_EN_BIT		24
+#define MSR_AMD64_SYSCFG_SNP_EN		BIT_ULL(MSR_AMD64_SYSCFG_SNP_EN_BIT)
+#define MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT	25
+#define MSR_AMD64_SYSCFG_SNP_VMPL_EN	BIT_ULL(MSR_AMD64_SYSCFG_SNP_VMPL_EN_BIT)
 #define MSR_K8_INT_PENDING_MSG		0xc0010055
 /* C1E active bits in int pending message */
 #define K8_INTP_C1E_ACTIVE_MASK		0x18000000
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index f01f4550e2c6..3a233b5d47c5 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -22,6 +22,8 @@ 
 #include <linux/efi.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
+#include <linux/cpumask.h>
+#include <linux/iommu.h>
 
 #include <asm/cpu_entry_area.h>
 #include <asm/stacktrace.h>
@@ -38,6 +40,7 @@ 
 #include <asm/apic.h>
 #include <asm/cpuid.h>
 #include <asm/cmdline.h>
+#include <asm/iommu.h>
 
 #define DR7_RESET_VALUE        0x400
 
@@ -57,6 +60,12 @@ 
 #define AP_INIT_CR0_DEFAULT		0x60000010
 #define AP_INIT_MXCSR_DEFAULT		0x1f80
 
+/*
+ * The first 16KB from the RMP_BASE is used by the processor for the
+ * bookkeeping, the range need to be added during the RMP entry lookup.
+ */
+#define RMPTABLE_CPU_BOOKKEEPING_SZ	0x4000
+
 /* For early boot hypervisor communication in SEV-ES enabled guests */
 static struct ghcb boot_ghcb_page __bss_decrypted __aligned(PAGE_SIZE);
 
@@ -69,6 +78,10 @@  static struct ghcb *boot_ghcb __section(".data");
 /* Bitmap of SEV features supported by the hypervisor */
 static u64 sev_hv_features __ro_after_init;
 
+static unsigned long rmptable_start __ro_after_init;
+static unsigned long rmptable_end __ro_after_init;
+
+
 /* #VC handler runtime per-CPU data */
 struct sev_es_runtime_data {
 	struct ghcb ghcb_page;
@@ -2218,3 +2231,134 @@  static int __init snp_init_platform_device(void)
 	return 0;
 }
 device_initcall(snp_init_platform_device);
+
+#undef pr_fmt
+#define pr_fmt(fmt)	"SEV-SNP: " fmt
+
+static int __snp_enable(unsigned int cpu)
+{
+	u64 val;
+
+	if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+		return 0;
+
+	rdmsrl(MSR_AMD64_SYSCFG, val);
+
+	val |= MSR_AMD64_SYSCFG_SNP_EN;
+	val |= MSR_AMD64_SYSCFG_SNP_VMPL_EN;
+
+	wrmsrl(MSR_AMD64_SYSCFG, val);
+
+	return 0;
+}
+
+static __init void snp_enable(void *arg)
+{
+	__snp_enable(smp_processor_id());
+}
+
+static bool get_rmptable_info(u64 *start, u64 *len)
+{
+	u64 calc_rmp_sz, rmp_sz, rmp_base, rmp_end, nr_pages;
+
+	rdmsrl(MSR_AMD64_RMP_BASE, rmp_base);
+	rdmsrl(MSR_AMD64_RMP_END, rmp_end);
+
+	if (!rmp_base || !rmp_end) {
+		pr_info("Memory for the RMP table has not been reserved by BIOS\n");
+		return false;
+	}
+
+	rmp_sz = rmp_end - rmp_base + 1;
+
+	/*
+	 * Calculate the amount the memory that must be reserved by the BIOS to
+	 * address the full system RAM. The reserved memory should also cover the
+	 * RMP table itself.
+	 *
+	 * See PPR Family 19h Model 01h, Revision B1 section 2.1.4.2 for more
+	 * information on memory requirement.
+	 */
+	nr_pages = totalram_pages();
+	calc_rmp_sz = (((rmp_sz >> PAGE_SHIFT) + nr_pages) << 4) + RMPTABLE_CPU_BOOKKEEPING_SZ;
+
+	if (calc_rmp_sz > rmp_sz) {
+		pr_info("Memory reserved for the RMP table does not cover full system RAM (expected 0x%llx got 0x%llx)\n",
+			calc_rmp_sz, rmp_sz);
+		return false;
+	}
+
+	*start = rmp_base;
+	*len = rmp_sz;
+
+	pr_info("RMP table physical address 0x%016llx - 0x%016llx\n", rmp_base, rmp_end);
+
+	return true;
+}
+
+static __init int __snp_rmptable_init(void)
+{
+	u64 rmp_base, sz;
+	void *start;
+	u64 val;
+
+	if (!get_rmptable_info(&rmp_base, &sz))
+		return 1;
+
+	start = memremap(rmp_base, sz, MEMREMAP_WB);
+	if (!start) {
+		pr_err("Failed to map RMP table 0x%llx+0x%llx\n", rmp_base, sz);
+		return 1;
+	}
+
+	/*
+	 * Check if SEV-SNP is already enabled, this can happen if we are coming from
+	 * kexec boot.
+	 */
+	rdmsrl(MSR_AMD64_SYSCFG, val);
+	if (val & MSR_AMD64_SYSCFG_SNP_EN)
+		goto skip_enable;
+
+	/* Initialize the RMP table to zero */
+	memset(start, 0, sz);
+
+	/* Flush the caches to ensure that data is written before SNP is enabled. */
+	wbinvd_on_all_cpus();
+
+	/* Enable SNP on all CPUs. */
+	on_each_cpu(snp_enable, NULL, 1);
+
+skip_enable:
+	rmptable_start = (unsigned long)start;
+	rmptable_end = rmptable_start + sz;
+
+	return 0;
+}
+
+static int __init snp_rmptable_init(void)
+{
+	if (!boot_cpu_has(X86_FEATURE_SEV_SNP))
+		return 0;
+
+	if (!iommu_sev_snp_supported())
+		goto nosnp;
+
+	if (__snp_rmptable_init())
+		goto nosnp;
+
+	cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/rmptable_init:online", __snp_enable, NULL);
+
+	return 0;
+
+nosnp:
+	setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
+	return 1;
+}
+
+/*
+ * This must be called after the PCI subsystem. This is because before enabling
+ * the SNP feature we need to ensure that IOMMU supports the SEV-SNP feature.
+ * The iommu_sev_snp_support() is used for checking the feature, and it is
+ * available after subsys_initcall().
+ */
+fs_initcall(snp_rmptable_init);