diff mbox series

[RFC,v7,16/64] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction

Message ID 20221214194056.161492-17-michael.roth@amd.com
State New
Headers
Series Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support |

Commit Message

Michael Roth Dec. 14, 2022, 7:40 p.m. UTC 
  From: Brijesh Singh <brijesh.singh@amd.com>

The RMPUPDATE instruction writes a new RMP entry in the RMP Table. The
hypervisor will use the instruction to add pages to the RMP table. See
APM3 for details on the instruction operations.

The PSMASH instruction expands a 2MB RMP entry into a corresponding set
of contiguous 4KB-Page RMP entries. The hypervisor will use this
instruction to adjust the RMP entry without invalidating the previous
RMP entry.

Add the following external interface API functions:

int psmash(u64 pfn);
psmash is used to smash a 2MB aligned page into 4K
pages while preserving the Validated bit in the RMP.

int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, bool immutable);
Used to assign a page to guest using the RMPUPDATE instruction.

int rmp_make_shared(u64 pfn, enum pg_level level);
Used to transition a page to hypervisor/shared state using the RMPUPDATE instruction.

Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
[mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 arch/x86/include/asm/sev.h | 24 ++++++++++
 arch/x86/kernel/sev.c      | 95 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

Comments

Alexander Graf Jan. 31, 2023, 9:26 p.m. UTC | #1 
On 14.12.22 20:40, Michael Roth wrote:
> From: Brijesh Singh <brijesh.singh@amd.com>
>
> The RMPUPDATE instruction writes a new RMP entry in the RMP Table. The
> hypervisor will use the instruction to add pages to the RMP table. See
> APM3 for details on the instruction operations.
>
> The PSMASH instruction expands a 2MB RMP entry into a corresponding set
> of contiguous 4KB-Page RMP entries. The hypervisor will use this
> instruction to adjust the RMP entry without invalidating the previous
> RMP entry.
>
> Add the following external interface API functions:
>
> int psmash(u64 pfn);
> psmash is used to smash a 2MB aligned page into 4K
> pages while preserving the Validated bit in the RMP.
>
> int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, bool immutable);
> Used to assign a page to guest using the RMPUPDATE instruction.
>
> int rmp_make_shared(u64 pfn, enum pg_level level);
> Used to transition a page to hypervisor/shared state using the RMPUPDATE instruction.
>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> [mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>   arch/x86/include/asm/sev.h | 24 ++++++++++
>   arch/x86/kernel/sev.c      | 95 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 119 insertions(+)
>
> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index 8d3ce2ad27da..4eeedcaca593 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -80,10 +80,15 @@ extern bool handle_vc_boot_ghcb(struct pt_regs *regs);
>
>   /* Software defined (when rFlags.CF = 1) */
>   #define PVALIDATE_FAIL_NOUPDATE                255
> +/* RMUPDATE detected 4K page and 2MB page overlap. */
> +#define RMPUPDATE_FAIL_OVERLAP         7
>
>   /* RMP page size */
>   #define RMP_PG_SIZE_4K                 0
> +#define RMP_PG_SIZE_2M                 1
>   #define RMP_TO_X86_PG_LEVEL(level)     (((level) == RMP_PG_SIZE_4K) ? PG_LEVEL_4K : PG_LEVEL_2M)
> +#define X86_TO_RMP_PG_LEVEL(level)     (((level) == PG_LEVEL_4K) ? RMP_PG_SIZE_4K : RMP_PG_SIZE_2M)
> +
>   #define RMPADJUST_VMSA_PAGE_BIT                BIT(16)
>
>   /* SNP Guest message request */
> @@ -133,6 +138,15 @@ struct snp_secrets_page_layout {
>          u8 rsvd3[3840];
>   } __packed;
>
> +struct rmp_state {
> +       u64 gpa;
> +       u8 assigned;
> +       u8 pagesize;
> +       u8 immutable;
> +       u8 rsvd;
> +       u32 asid;
> +} __packed;
> +
>   #ifdef CONFIG_AMD_MEM_ENCRYPT
>   extern struct static_key_false sev_es_enable_key;
>   extern void __sev_es_ist_enter(struct pt_regs *regs);
> @@ -198,6 +212,9 @@ bool snp_init(struct boot_params *bp);
>   void __init __noreturn snp_abort(void);
>   int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err);
>   int snp_lookup_rmpentry(u64 pfn, int *level);
> +int psmash(u64 pfn);
> +int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, bool immutable);
> +int rmp_make_shared(u64 pfn, enum pg_level level);
>   #else
>   static inline void sev_es_ist_enter(struct pt_regs *regs) { }
>   static inline void sev_es_ist_exit(void) { }
> @@ -223,6 +240,13 @@ static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
>          return -ENOTTY;
>   }
>   static inline int snp_lookup_rmpentry(u64 pfn, int *level) { return 0; }
> +static inline int psmash(u64 pfn) { return -ENXIO; }
> +static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid,
> +                                  bool immutable)
> +{
> +       return -ENODEV;
> +}
> +static inline int rmp_make_shared(u64 pfn, enum pg_level level) { return -ENODEV; }
>   #endif
>
>   #endif
> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 706675561f49..67035d34adad 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -2523,3 +2523,98 @@ int snp_lookup_rmpentry(u64 pfn, int *level)
>          return !!rmpentry_assigned(e);
>   }
>   EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
> +
> +/*
> + * psmash is used to smash a 2MB aligned page into 4K
> + * pages while preserving the Validated bit in the RMP.
> + */
> +int psmash(u64 pfn)
> +{
> +       unsigned long paddr = pfn << PAGE_SHIFT;
> +       int ret;
> +
> +       if (!pfn_valid(pfn))
> +               return -EINVAL;


We (and many other clouds) use a neat trick to reduce the number of 
struct pages Linux allocates for guest memory: In its simplest form, add 
mem= to the kernel cmdline and mmap() /dev/mem to access the reserved 
memory instead.

This means that the system covers more RAM than Linux contains, which 
means pfn_valid() is no longer a good indication whether a page is 
indeed valid. KVM handles this case fine, but this code does not.

Is there any particular reason why we need this check (and similar ones 
below and in other RMP related patches) in the first place? I would 
expect that PSMASH and friends return failure codes for invalid pfns.


Alex





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Kalra, Ashish Feb. 1, 2023, 5:14 p.m. UTC | #2 
On 1/31/2023 3:26 PM, Alexander Graf wrote:
> 
> On 14.12.22 20:40, Michael Roth wrote:
>> From: Brijesh Singh <brijesh.singh@amd.com>
>>
>> The RMPUPDATE instruction writes a new RMP entry in the RMP Table. The
>> hypervisor will use the instruction to add pages to the RMP table. See
>> APM3 for details on the instruction operations.
>>
>> The PSMASH instruction expands a 2MB RMP entry into a corresponding set
>> of contiguous 4KB-Page RMP entries. The hypervisor will use this
>> instruction to adjust the RMP entry without invalidating the previous
>> RMP entry.
>>
>> Add the following external interface API functions:
>>
>> int psmash(u64 pfn);
>> psmash is used to smash a 2MB aligned page into 4K
>> pages while preserving the Validated bit in the RMP.
>>
>> int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, 
>> bool immutable);
>> Used to assign a page to guest using the RMPUPDATE instruction.
>>
>> int rmp_make_shared(u64 pfn, enum pg_level level);
>> Used to transition a page to hypervisor/shared state using the 
>> RMPUPDATE instruction.
>>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> [mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]
>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>> ---
>>   arch/x86/include/asm/sev.h | 24 ++++++++++
>>   arch/x86/kernel/sev.c      | 95 ++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 119 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>> index 8d3ce2ad27da..4eeedcaca593 100644
>> --- a/arch/x86/include/asm/sev.h
>> +++ b/arch/x86/include/asm/sev.h
>> @@ -80,10 +80,15 @@ extern bool handle_vc_boot_ghcb(struct pt_regs 
>> *regs);
>>
>>   /* Software defined (when rFlags.CF = 1) */
>>   #define PVALIDATE_FAIL_NOUPDATE                255
>> +/* RMUPDATE detected 4K page and 2MB page overlap. */
>> +#define RMPUPDATE_FAIL_OVERLAP         7
>>
>>   /* RMP page size */
>>   #define RMP_PG_SIZE_4K                 0
>> +#define RMP_PG_SIZE_2M                 1
>>   #define RMP_TO_X86_PG_LEVEL(level)     (((level) == RMP_PG_SIZE_4K) 
>> ? PG_LEVEL_4K : PG_LEVEL_2M)
>> +#define X86_TO_RMP_PG_LEVEL(level)     (((level) == PG_LEVEL_4K) ? 
>> RMP_PG_SIZE_4K : RMP_PG_SIZE_2M)
>> +
>>   #define RMPADJUST_VMSA_PAGE_BIT                BIT(16)
>>
>>   /* SNP Guest message request */
>> @@ -133,6 +138,15 @@ struct snp_secrets_page_layout {
>>          u8 rsvd3[3840];
>>   } __packed;
>>
>> +struct rmp_state {
>> +       u64 gpa;
>> +       u8 assigned;
>> +       u8 pagesize;
>> +       u8 immutable;
>> +       u8 rsvd;
>> +       u32 asid;
>> +} __packed;
>> +
>>   #ifdef CONFIG_AMD_MEM_ENCRYPT
>>   extern struct static_key_false sev_es_enable_key;
>>   extern void __sev_es_ist_enter(struct pt_regs *regs);
>> @@ -198,6 +212,9 @@ bool snp_init(struct boot_params *bp);
>>   void __init __noreturn snp_abort(void);
>>   int snp_issue_guest_request(u64 exit_code, struct snp_req_data 
>> *input, unsigned long *fw_err);
>>   int snp_lookup_rmpentry(u64 pfn, int *level);
>> +int psmash(u64 pfn);
>> +int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, 
>> bool immutable);
>> +int rmp_make_shared(u64 pfn, enum pg_level level);
>>   #else
>>   static inline void sev_es_ist_enter(struct pt_regs *regs) { }
>>   static inline void sev_es_ist_exit(void) { }
>> @@ -223,6 +240,13 @@ static inline int snp_issue_guest_request(u64 
>> exit_code, struct snp_req_data *in
>>          return -ENOTTY;
>>   }
>>   static inline int snp_lookup_rmpentry(u64 pfn, int *level) { return 
>> 0; }
>> +static inline int psmash(u64 pfn) { return -ENXIO; }
>> +static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level 
>> level, int asid,
>> +                                  bool immutable)
>> +{
>> +       return -ENODEV;
>> +}
>> +static inline int rmp_make_shared(u64 pfn, enum pg_level level) { 
>> return -ENODEV; }
>>   #endif
>>
>>   #endif
>> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
>> index 706675561f49..67035d34adad 100644
>> --- a/arch/x86/kernel/sev.c
>> +++ b/arch/x86/kernel/sev.c
>> @@ -2523,3 +2523,98 @@ int snp_lookup_rmpentry(u64 pfn, int *level)
>>          return !!rmpentry_assigned(e);
>>   }
>>   EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
>> +
>> +/*
>> + * psmash is used to smash a 2MB aligned page into 4K
>> + * pages while preserving the Validated bit in the RMP.
>> + */
>> +int psmash(u64 pfn)
>> +{
>> +       unsigned long paddr = pfn << PAGE_SHIFT;
>> +       int ret;
>> +
>> +       if (!pfn_valid(pfn))
>> +               return -EINVAL;
> 
> 
> We (and many other clouds) use a neat trick to reduce the number of 
> struct pages Linux allocates for guest memory: In its simplest form, add 
> mem= to the kernel cmdline and mmap() /dev/mem to access the reserved 
> memory instead.
> 
> This means that the system covers more RAM than Linux contains, which 
> means pfn_valid() is no longer a good indication whether a page is 
> indeed valid. KVM handles this case fine, but this code does not.

Hmm...but then is also using max_pfn reliable ?

> 
> Is there any particular reason why we need this check (and similar ones 
> below and in other RMP related patches) in the first place. I would > expect that PSMASH and friends return failure codes for invalid pfns.
> 

Yes, PSMASH does out of bounds check on the input SPA and additionally 
checks if SPA is 2M aligned, so guess we can rely on using PSMASH 
failing on invalid pfns.

Thanks,
Ashish
Alexander Graf Feb. 1, 2023, 5:20 p.m. UTC | #3 
On 01.02.23 18:14, Kalra, Ashish wrote:
>
> On 1/31/2023 3:26 PM, Alexander Graf wrote:
>>
>> On 14.12.22 20:40, Michael Roth wrote:
>>> From: Brijesh Singh <brijesh.singh@amd.com>
>>>
>>> The RMPUPDATE instruction writes a new RMP entry in the RMP Table. The
>>> hypervisor will use the instruction to add pages to the RMP table. See
>>> APM3 for details on the instruction operations.
>>>
>>> The PSMASH instruction expands a 2MB RMP entry into a corresponding set
>>> of contiguous 4KB-Page RMP entries. The hypervisor will use this
>>> instruction to adjust the RMP entry without invalidating the previous
>>> RMP entry.
>>>
>>> Add the following external interface API functions:
>>>
>>> int psmash(u64 pfn);
>>> psmash is used to smash a 2MB aligned page into 4K
>>> pages while preserving the Validated bit in the RMP.
>>>
>>> int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid,
>>> bool immutable);
>>> Used to assign a page to guest using the RMPUPDATE instruction.
>>>
>>> int rmp_make_shared(u64 pfn, enum pg_level level);
>>> Used to transition a page to hypervisor/shared state using the
>>> RMPUPDATE instruction.
>>>
>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>> [mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]
>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>>> ---
>>>   arch/x86/include/asm/sev.h | 24 ++++++++++
>>>   arch/x86/kernel/sev.c      | 95 
>>> ++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 119 insertions(+)
>>>
>>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>>> index 8d3ce2ad27da..4eeedcaca593 100644
>>> --- a/arch/x86/include/asm/sev.h
>>> +++ b/arch/x86/include/asm/sev.h
>>> @@ -80,10 +80,15 @@ extern bool handle_vc_boot_ghcb(struct pt_regs
>>> *regs);
>>>
>>>   /* Software defined (when rFlags.CF = 1) */
>>>   #define PVALIDATE_FAIL_NOUPDATE                255
>>> +/* RMUPDATE detected 4K page and 2MB page overlap. */
>>> +#define RMPUPDATE_FAIL_OVERLAP         7
>>>
>>>   /* RMP page size */
>>>   #define RMP_PG_SIZE_4K                 0
>>> +#define RMP_PG_SIZE_2M                 1
>>>   #define RMP_TO_X86_PG_LEVEL(level)     (((level) == RMP_PG_SIZE_4K)
>>> ? PG_LEVEL_4K : PG_LEVEL_2M)
>>> +#define X86_TO_RMP_PG_LEVEL(level)     (((level) == PG_LEVEL_4K) ?
>>> RMP_PG_SIZE_4K : RMP_PG_SIZE_2M)
>>> +
>>>   #define RMPADJUST_VMSA_PAGE_BIT                BIT(16)
>>>
>>>   /* SNP Guest message request */
>>> @@ -133,6 +138,15 @@ struct snp_secrets_page_layout {
>>>          u8 rsvd3[3840];
>>>   } __packed;
>>>
>>> +struct rmp_state {
>>> +       u64 gpa;
>>> +       u8 assigned;
>>> +       u8 pagesize;
>>> +       u8 immutable;
>>> +       u8 rsvd;
>>> +       u32 asid;
>>> +} __packed;
>>> +
>>>   #ifdef CONFIG_AMD_MEM_ENCRYPT
>>>   extern struct static_key_false sev_es_enable_key;
>>>   extern void __sev_es_ist_enter(struct pt_regs *regs);
>>> @@ -198,6 +212,9 @@ bool snp_init(struct boot_params *bp);
>>>   void __init __noreturn snp_abort(void);
>>>   int snp_issue_guest_request(u64 exit_code, struct snp_req_data
>>> *input, unsigned long *fw_err);
>>>   int snp_lookup_rmpentry(u64 pfn, int *level);
>>> +int psmash(u64 pfn);
>>> +int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid,
>>> bool immutable);
>>> +int rmp_make_shared(u64 pfn, enum pg_level level);
>>>   #else
>>>   static inline void sev_es_ist_enter(struct pt_regs *regs) { }
>>>   static inline void sev_es_ist_exit(void) { }
>>> @@ -223,6 +240,13 @@ static inline int snp_issue_guest_request(u64
>>> exit_code, struct snp_req_data *in
>>>          return -ENOTTY;
>>>   }
>>>   static inline int snp_lookup_rmpentry(u64 pfn, int *level) { return
>>> 0; }
>>> +static inline int psmash(u64 pfn) { return -ENXIO; }
>>> +static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level
>>> level, int asid,
>>> +                                  bool immutable)
>>> +{
>>> +       return -ENODEV;
>>> +}
>>> +static inline int rmp_make_shared(u64 pfn, enum pg_level level) {
>>> return -ENODEV; }
>>>   #endif
>>>
>>>   #endif
>>> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
>>> index 706675561f49..67035d34adad 100644
>>> --- a/arch/x86/kernel/sev.c
>>> +++ b/arch/x86/kernel/sev.c
>>> @@ -2523,3 +2523,98 @@ int snp_lookup_rmpentry(u64 pfn, int *level)
>>>          return !!rmpentry_assigned(e);
>>>   }
>>>   EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
>>> +
>>> +/*
>>> + * psmash is used to smash a 2MB aligned page into 4K
>>> + * pages while preserving the Validated bit in the RMP.
>>> + */
>>> +int psmash(u64 pfn)
>>> +{
>>> +       unsigned long paddr = pfn << PAGE_SHIFT;
>>> +       int ret;
>>> +
>>> +       if (!pfn_valid(pfn))
>>> +               return -EINVAL;
>>
>>
>> We (and many other clouds) use a neat trick to reduce the number of
>> struct pages Linux allocates for guest memory: In its simplest form, add
>> mem= to the kernel cmdline and mmap() /dev/mem to access the reserved
>> memory instead.
>>
>> This means that the system covers more RAM than Linux contains, which
>> means pfn_valid() is no longer a good indication whether a page is
>> indeed valid. KVM handles this case fine, but this code does not.
>
> Hmm...but then is also using max_pfn reliable ?


I would expect it to not be reliable as it only looks at E820_TYPE_RAM, 
yes. Do you rely on max_pfn anywhere?


>
>>
>> Is there any particular reason why we need this check (and similar ones
>> below and in other RMP related patches) in the first place. I would > 
>> expect that PSMASH and friends return failure codes for invalid pfns.
>>
>
> Yes, PSMASH does out of bounds check on the input SPA and additionally
> checks if SPA is 2M aligned, so guess we can rely on using PSMASH
> failing on invalid pfns.


Perfect, please remove all the superfluous checks then. If you want to 
make our life easy, I'd recommend you try to try the patch set with mem= 
passed on the host and tell QEMU to mmap() /dev/mem for guest RAM. That 
way you should be able to find any other pitfalls :)


Alex





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Kalra, Ashish Feb. 2, 2023, 7:04 p.m. UTC | #4 
On 2/1/2023 11:20 AM, Alexander Graf wrote:
> 
> On 01.02.23 18:14, Kalra, Ashish wrote:
>>
>> On 1/31/2023 3:26 PM, Alexander Graf wrote:
>>>
>>> On 14.12.22 20:40, Michael Roth wrote:
>>>> From: Brijesh Singh <brijesh.singh@amd.com>
>>>>
>>>> The RMPUPDATE instruction writes a new RMP entry in the RMP Table. The
>>>> hypervisor will use the instruction to add pages to the RMP table. See
>>>> APM3 for details on the instruction operations.
>>>>
>>>> The PSMASH instruction expands a 2MB RMP entry into a corresponding set
>>>> of contiguous 4KB-Page RMP entries. The hypervisor will use this
>>>> instruction to adjust the RMP entry without invalidating the previous
>>>> RMP entry.
>>>>
>>>> Add the following external interface API functions:
>>>>
>>>> int psmash(u64 pfn);
>>>> psmash is used to smash a 2MB aligned page into 4K
>>>> pages while preserving the Validated bit in the RMP.
>>>>
>>>> int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid,
>>>> bool immutable);
>>>> Used to assign a page to guest using the RMPUPDATE instruction.
>>>>
>>>> int rmp_make_shared(u64 pfn, enum pg_level level);
>>>> Used to transition a page to hypervisor/shared state using the
>>>> RMPUPDATE instruction.
>>>>
>>>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>>> [mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]
>>>> Signed-off-by: Michael Roth <michael.roth@amd.com>
>>>> ---
>>>>   arch/x86/include/asm/sev.h | 24 ++++++++++
>>>>   arch/x86/kernel/sev.c      | 95 
>>>> ++++++++++++++++++++++++++++++++++++++
>>>>   2 files changed, 119 insertions(+)
>>>>
>>>> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
>>>> index 8d3ce2ad27da..4eeedcaca593 100644
>>>> --- a/arch/x86/include/asm/sev.h
>>>> +++ b/arch/x86/include/asm/sev.h
>>>> @@ -80,10 +80,15 @@ extern bool handle_vc_boot_ghcb(struct pt_regs
>>>> *regs);
>>>>
>>>>   /* Software defined (when rFlags.CF = 1) */
>>>>   #define PVALIDATE_FAIL_NOUPDATE                255
>>>> +/* RMUPDATE detected 4K page and 2MB page overlap. */
>>>> +#define RMPUPDATE_FAIL_OVERLAP         7
>>>>
>>>>   /* RMP page size */
>>>>   #define RMP_PG_SIZE_4K                 0
>>>> +#define RMP_PG_SIZE_2M                 1
>>>>   #define RMP_TO_X86_PG_LEVEL(level)     (((level) == RMP_PG_SIZE_4K)
>>>> ? PG_LEVEL_4K : PG_LEVEL_2M)
>>>> +#define X86_TO_RMP_PG_LEVEL(level)     (((level) == PG_LEVEL_4K) ?
>>>> RMP_PG_SIZE_4K : RMP_PG_SIZE_2M)
>>>> +
>>>>   #define RMPADJUST_VMSA_PAGE_BIT                BIT(16)
>>>>
>>>>   /* SNP Guest message request */
>>>> @@ -133,6 +138,15 @@ struct snp_secrets_page_layout {
>>>>          u8 rsvd3[3840];
>>>>   } __packed;
>>>>
>>>> +struct rmp_state {
>>>> +       u64 gpa;
>>>> +       u8 assigned;
>>>> +       u8 pagesize;
>>>> +       u8 immutable;
>>>> +       u8 rsvd;
>>>> +       u32 asid;
>>>> +} __packed;
>>>> +
>>>>   #ifdef CONFIG_AMD_MEM_ENCRYPT
>>>>   extern struct static_key_false sev_es_enable_key;
>>>>   extern void __sev_es_ist_enter(struct pt_regs *regs);
>>>> @@ -198,6 +212,9 @@ bool snp_init(struct boot_params *bp);
>>>>   void __init __noreturn snp_abort(void);
>>>>   int snp_issue_guest_request(u64 exit_code, struct snp_req_data
>>>> *input, unsigned long *fw_err);
>>>>   int snp_lookup_rmpentry(u64 pfn, int *level);
>>>> +int psmash(u64 pfn);
>>>> +int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid,
>>>> bool immutable);
>>>> +int rmp_make_shared(u64 pfn, enum pg_level level);
>>>>   #else
>>>>   static inline void sev_es_ist_enter(struct pt_regs *regs) { }
>>>>   static inline void sev_es_ist_exit(void) { }
>>>> @@ -223,6 +240,13 @@ static inline int snp_issue_guest_request(u64
>>>> exit_code, struct snp_req_data *in
>>>>          return -ENOTTY;
>>>>   }
>>>>   static inline int snp_lookup_rmpentry(u64 pfn, int *level) { return
>>>> 0; }
>>>> +static inline int psmash(u64 pfn) { return -ENXIO; }
>>>> +static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level
>>>> level, int asid,
>>>> +                                  bool immutable)
>>>> +{
>>>> +       return -ENODEV;
>>>> +}
>>>> +static inline int rmp_make_shared(u64 pfn, enum pg_level level) {
>>>> return -ENODEV; }
>>>>   #endif
>>>>
>>>>   #endif
>>>> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
>>>> index 706675561f49..67035d34adad 100644
>>>> --- a/arch/x86/kernel/sev.c
>>>> +++ b/arch/x86/kernel/sev.c
>>>> @@ -2523,3 +2523,98 @@ int snp_lookup_rmpentry(u64 pfn, int *level)
>>>>          return !!rmpentry_assigned(e);
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
>>>> +
>>>> +/*
>>>> + * psmash is used to smash a 2MB aligned page into 4K
>>>> + * pages while preserving the Validated bit in the RMP.
>>>> + */
>>>> +int psmash(u64 pfn)
>>>> +{
>>>> +       unsigned long paddr = pfn << PAGE_SHIFT;
>>>> +       int ret;
>>>> +
>>>> +       if (!pfn_valid(pfn))
>>>> +               return -EINVAL;
>>>
>>>
>>> We (and many other clouds) use a neat trick to reduce the number of
>>> struct pages Linux allocates for guest memory: In its simplest form, add
>>> mem= to the kernel cmdline and mmap() /dev/mem to access the reserved
>>> memory instead.
>>>
>>> This means that the system covers more RAM than Linux contains, which
>>> means pfn_valid() is no longer a good indication whether a page is
>>> indeed valid. KVM handles this case fine, but this code does not.
>>
>> Hmm...but then is also using max_pfn reliable ?
> 
> 
> I would expect it to not be reliable as it only looks at E820_TYPE_RAM, 
> yes. Do you rely on max_pfn anywhere?
>  

We use it to check if the RMP table is covering the whole system RAM, 
to get the max. addressable PFN, which should be fine.

Thanks,
Ashish
Liam Merwick Feb. 8, 2023, 4:30 p.m. UTC | #5 
On 14/12/2022 19:40, Michael Roth wrote:
> From: Brijesh Singh <brijesh.singh@amd.com>
> 
> The RMPUPDATE instruction writes a new RMP entry in the RMP Table. The
> hypervisor will use the instruction to add pages to the RMP table. See
> APM3 for details on the instruction operations.
> 
> The PSMASH instruction expands a 2MB RMP entry into a corresponding set
> of contiguous 4KB-Page RMP entries. The hypervisor will use this
> instruction to adjust the RMP entry without invalidating the previous
> RMP entry.
> 
> Add the following external interface API functions:
> 
> int psmash(u64 pfn);
> psmash is used to smash a 2MB aligned page into 4K
> pages while preserving the Validated bit in the RMP.
> 
> int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, bool immutable);
> Used to assign a page to guest using the RMPUPDATE instruction.
> 
> int rmp_make_shared(u64 pfn, enum pg_level level);
> Used to transition a page to hypervisor/shared state using the RMPUPDATE instruction.
> 
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> [mdr: add RMPUPDATE retry logic for transient FAIL_OVERLAP errors]
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>   arch/x86/include/asm/sev.h | 24 ++++++++++
>   arch/x86/kernel/sev.c      | 95 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 119 insertions(+)
> 

...

> +
> +static int rmpupdate(u64 pfn, struct rmp_state *val)
> +{
> +	unsigned long paddr = pfn << PAGE_SHIFT;
> +	int retries = 0;
> +	int ret;
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
> +		return -ENXIO;
> +
> +retry:
> +	/* Binutils version 2.36 supports the RMPUPDATE mnemonic. */
> +	asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
> +		     : "=a"(ret)
> +		     : "a"(paddr), "c"((unsigned long)val)
> +		     : "memory", "cc");
> +
> +	if (ret) {
> +		if (!retries) {
> +			pr_err("RMPUPDATE failed, ret: %d, pfn: %llx, npages: %d, level: %d, retrying (max: %d)...\n",
> +			       ret, pfn, npages, level, 2 * num_present_cpus());

This patch isn't bisectable - 'npages' isn't defined in this patch - 
it's defined later in Patch18

otherwise LGTM

Regards,
Liam


> +			dump_stack();
> +		}
> +		retries++;
> +		if (retries < 2 * num_present_cpus())
> +			goto retry;
> +	} else if (retries > 0) {
> +		pr_err("RMPUPDATE for pfn %llx succeeded after %d retries\n", pfn, retries);
> +	}
> +
> +	return ret;
> +}
diff mbox series

Patch

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 8d3ce2ad27da..4eeedcaca593 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -80,10 +80,15 @@  extern bool handle_vc_boot_ghcb(struct pt_regs *regs);
 
 /* Software defined (when rFlags.CF = 1) */
 #define PVALIDATE_FAIL_NOUPDATE		255
+/* RMUPDATE detected 4K page and 2MB page overlap. */
+#define RMPUPDATE_FAIL_OVERLAP		7
 
 /* RMP page size */
 #define RMP_PG_SIZE_4K			0
+#define RMP_PG_SIZE_2M			1
 #define RMP_TO_X86_PG_LEVEL(level)	(((level) == RMP_PG_SIZE_4K) ? PG_LEVEL_4K : PG_LEVEL_2M)
+#define X86_TO_RMP_PG_LEVEL(level)	(((level) == PG_LEVEL_4K) ? RMP_PG_SIZE_4K : RMP_PG_SIZE_2M)
+
 #define RMPADJUST_VMSA_PAGE_BIT		BIT(16)
 
 /* SNP Guest message request */
@@ -133,6 +138,15 @@  struct snp_secrets_page_layout {
 	u8 rsvd3[3840];
 } __packed;
 
+struct rmp_state {
+	u64 gpa;
+	u8 assigned;
+	u8 pagesize;
+	u8 immutable;
+	u8 rsvd;
+	u32 asid;
+} __packed;
+
 #ifdef CONFIG_AMD_MEM_ENCRYPT
 extern struct static_key_false sev_es_enable_key;
 extern void __sev_es_ist_enter(struct pt_regs *regs);
@@ -198,6 +212,9 @@  bool snp_init(struct boot_params *bp);
 void __init __noreturn snp_abort(void);
 int snp_issue_guest_request(u64 exit_code, struct snp_req_data *input, unsigned long *fw_err);
 int snp_lookup_rmpentry(u64 pfn, int *level);
+int psmash(u64 pfn);
+int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, bool immutable);
+int rmp_make_shared(u64 pfn, enum pg_level level);
 #else
 static inline void sev_es_ist_enter(struct pt_regs *regs) { }
 static inline void sev_es_ist_exit(void) { }
@@ -223,6 +240,13 @@  static inline int snp_issue_guest_request(u64 exit_code, struct snp_req_data *in
 	return -ENOTTY;
 }
 static inline int snp_lookup_rmpentry(u64 pfn, int *level) { return 0; }
+static inline int psmash(u64 pfn) { return -ENXIO; }
+static inline int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid,
+				   bool immutable)
+{
+	return -ENODEV;
+}
+static inline int rmp_make_shared(u64 pfn, enum pg_level level) { return -ENODEV; }
 #endif
 
 #endif
diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
index 706675561f49..67035d34adad 100644
--- a/arch/x86/kernel/sev.c
+++ b/arch/x86/kernel/sev.c
@@ -2523,3 +2523,98 @@  int snp_lookup_rmpentry(u64 pfn, int *level)
 	return !!rmpentry_assigned(e);
 }
 EXPORT_SYMBOL_GPL(snp_lookup_rmpentry);
+
+/*
+ * psmash is used to smash a 2MB aligned page into 4K
+ * pages while preserving the Validated bit in the RMP.
+ */
+int psmash(u64 pfn)
+{
+	unsigned long paddr = pfn << PAGE_SHIFT;
+	int ret;
+
+	if (!pfn_valid(pfn))
+		return -EINVAL;
+
+	if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+		return -ENXIO;
+
+	/* Binutils version 2.36 supports the PSMASH mnemonic. */
+	asm volatile(".byte 0xF3, 0x0F, 0x01, 0xFF"
+		      : "=a"(ret)
+		      : "a"(paddr)
+		      : "memory", "cc");
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(psmash);
+
+static int rmpupdate(u64 pfn, struct rmp_state *val)
+{
+	unsigned long paddr = pfn << PAGE_SHIFT;
+	int retries = 0;
+	int ret;
+
+	if (!cpu_feature_enabled(X86_FEATURE_SEV_SNP))
+		return -ENXIO;
+
+retry:
+	/* Binutils version 2.36 supports the RMPUPDATE mnemonic. */
+	asm volatile(".byte 0xF2, 0x0F, 0x01, 0xFE"
+		     : "=a"(ret)
+		     : "a"(paddr), "c"((unsigned long)val)
+		     : "memory", "cc");
+
+	if (ret) {
+		if (!retries) {
+			pr_err("RMPUPDATE failed, ret: %d, pfn: %llx, npages: %d, level: %d, retrying (max: %d)...\n",
+			       ret, pfn, npages, level, 2 * num_present_cpus());
+			dump_stack();
+		}
+		retries++;
+		if (retries < 2 * num_present_cpus())
+			goto retry;
+	} else if (retries > 0) {
+		pr_err("RMPUPDATE for pfn %llx succeeded after %d retries\n", pfn, retries);
+	}
+
+	return ret;
+}
+
+/*
+ * Assign a page to guest using the RMPUPDATE instruction.
+ */
+int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, bool immutable)
+{
+	struct rmp_state val;
+
+	if (!pfn_valid(pfn))
+		return -EINVAL;
+
+	memset(&val, 0, sizeof(val));
+	val.assigned = 1;
+	val.asid = asid;
+	val.immutable = immutable;
+	val.gpa = gpa;
+	val.pagesize = X86_TO_RMP_PG_LEVEL(level);
+
+	return rmpupdate(pfn, &val);
+}
+EXPORT_SYMBOL_GPL(rmp_make_private);
+
+/*
+ * Transition a page to hypervisor/shared state using the RMPUPDATE instruction.
+ */
+int rmp_make_shared(u64 pfn, enum pg_level level)
+{
+	struct rmp_state val;
+
+	if (!pfn_valid(pfn))
+		return -EINVAL;
+
+	memset(&val, 0, sizeof(val));
+	val.pagesize = X86_TO_RMP_PG_LEVEL(level);
+
+	return rmpupdate(pfn, &val);
+}
+EXPORT_SYMBOL_GPL(rmp_make_shared);