Re: [edk2-devel] [PATCH 05/13] MdePkg/Register/Amd: define GHCB macro for the Page State Change

["Laszlo Ersek" <lersek@redhat.com>]

On 05/07/21 22:38, Brijesh Singh wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> The Page State Change NAE exit will be used by the SEV-SNP guest to
> request a page state change using the GHCB protocol. See the GHCB
> spec section 4.1.6 and 2.3.1 for more detail on the structure
> definitions.
> 
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  MdePkg/Include/Register/Amd/Fam17Msr.h | 15 ++++++++++++
>  MdePkg/Include/Register/Amd/Ghcb.h     | 33 ++++++++++++++++++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/MdePkg/Include/Register/Amd/Fam17Msr.h b/MdePkg/Include/Register/Amd/Fam17Msr.h
> index 542e4cdf4782..62014854d9b7 100644
> --- a/MdePkg/Include/Register/Amd/Fam17Msr.h
> +++ b/MdePkg/Include/Register/Amd/Fam17Msr.h
> @@ -58,6 +58,19 @@ typedef union {
>      UINT64  GuestFrameNumber:52;
>    } GhcbGpaRegister;
>  
> +  struct {
> +    UINT64 Function:12;
> +    UINT64 GuestFrameNumber:40;
> +    UINT64 Operation:4;
> +    UINT64 Reserved:8;
> +  } SnpPageStateChangeRequest;
> +
> +  struct {
> +    UINT32 Function:12;
> +    UINT32 Reserved:20;
> +    UINT32 ErrorCode;
> +  } SnpPageStateChangeResponse;
> +
>    VOID    *Ghcb;
>  
>    UINT64  GhcbPhysicalAddress;
> @@ -69,6 +82,8 @@ typedef union {
>  #define GHCB_INFO_CPUID_RESPONSE                    5
>  #define GHCB_INFO_GHCB_GPA_REGISTER_REQUEST         18
>  #define GHCB_INFO_GHCB_GPA_REGISTER_RESPONSE        19
> +#define GHCB_INFO_SNP_PAGE_STATE_CHANGE_REQUEST     20
> +#define GHCB_INFO_SNP_PAGE_STATE_CHANGE_RESPONSE    21
>  #define GHCB_HYPERVISOR_FEATURES_REQUEST            128
>  #define GHCB_HYPERVISOR_FEATURES_RESPONSE           129
>  #define GHCB_INFO_TERMINATE_REQUEST                 256
> diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h
> index 326b11479779..a15b4b7e2760 100644
> --- a/MdePkg/Include/Register/Amd/Ghcb.h
> +++ b/MdePkg/Include/Register/Amd/Ghcb.h
> @@ -54,6 +54,7 @@
>  #define SVM_EXIT_NMI_COMPLETE                   0x80000003ULL
>  #define SVM_EXIT_AP_RESET_HOLD                  0x80000004ULL
>  #define SVM_EXIT_AP_JUMP_TABLE                  0x80000005ULL
> +#define SVM_EXIT_SNP_PAGE_STATE_CHANGE          0x80000010ULL
>  #define SVM_EXIT_HYPERVISOR_FEATURES            0x8000FFFDULL
>  #define SVM_EXIT_UNSUPPORTED                    0x8000FFFFULL
>  
> @@ -162,4 +163,36 @@ typedef union {
>  #define GHCB_HV_FEATURES_SNP_AP_CREATE                    (GHCB_HV_FEATURES_SNP | BIT1)
>  #define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION         (GHCB_HV_FEATURES_SNP_AP_CREATE | BIT2)
>  #define GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION_TIMER   (GHCB_HV_FEATURES_SNP_RESTRICTED_INJECTION | BIT3)
> +
> +//
> +// SNP Page State Change.
> +//
> +// Note that the PSMASH and UNSMASH operations are not supported when using the MSR protocol.
> +//
> +#define SNP_PAGE_STATE_PRIVATE              1
> +#define SNP_PAGE_STATE_SHARED               2
> +#define SNP_PAGE_STATE_PSMASH               3
> +#define SNP_PAGE_STATE_UNSMASH              4
> +
> +typedef struct {
> +  UINT64  CurrentPage:12;
> +  UINT64  GuestFrameNumber:40;
> +  UINT64  Operation:4;
> +  UINT64  PageSize:1;
> +  UINT64  Reserved: 7;

(1) You didn't remove the stray space: my point (6) in
<https://listman.redhat.com/archives/edk2-devel-archive/2021-May/msg00044.html>.

I'll fix it up.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks,
Laszlo

> +} SNP_PAGE_STATE_ENTRY;
> +
> +typedef struct {
> +  UINT16 CurrentEntry;
> +  UINT16 EndEntry;
> +  UINT32 Reserved;
> +} SNP_PAGE_STATE_HEADER;
> +
> +#define SNP_PAGE_STATE_MAX_ENTRY            253
> +
> +typedef struct {
> +  SNP_PAGE_STATE_HEADER  Header;
> +  SNP_PAGE_STATE_ENTRY   Entry[SNP_PAGE_STATE_MAX_ENTRY];
> +} SNP_PAGE_STATE_CHANGE_INFO;
> +
>  #endif
>