Date:   Tue, 4 Jan 2022 16:31:46 -0600
From:   Venu Busireddy <venu.busireddy@oracle.com>
To:     Brijesh Singh <brijesh.singh@amd.com>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
        linux-coco@lists.linux.dev, linux-mm@kvack.org,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Joerg Roedel <jroedel@suse.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        "H. Peter Anvin" <hpa@zytor.com>, Ard Biesheuvel <ardb@kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Jim Mattson <jmattson@google.com>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Sergio Lopez <slp@redhat.com>, Peter Gonda <pgonda@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Dov Murik <dovmurik@linux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@ibm.com>,
        Borislav Petkov <bp@alien8.de>,
        Michael Roth <michael.roth@amd.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A . Shutemov" <kirill@shutemov.name>,
        Andi Kleen <ak@linux.intel.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        tony.luck@intel.com, marcorr@google.com,
        sathyanarayanan.kuppuswamy@linux.intel.com
Subject: Re: [PATCH v8 15/40] x86/mm: Add support to validate memory when
 changing C-bit
Message-ID: <YdTK0h2ZKu4UxrhQ@dt>
References: <20211210154332.11526-1-brijesh.singh@amd.com>
 <20211210154332.11526-16-brijesh.singh@amd.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211210154332.11526-16-brijesh.singh@amd.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?4pp+4LQRYVjkRutYkaIE1QypYxFgPTFy+SSdf2nKQX3zznfv6ugJso8onnM8?=
 =?us-ascii?Q?nZn5MH63X7RZ3rM02TgNdAdo2gVdyZqOOovLMkBG26ab/X2UY9aYI85x4rIE?=
 =?us-ascii?Q?cYU7Ncxj4UuGRGyRkrgm2hABWMnDzccNSJwy5tDoe11HPP+zf5T19LGc5+ta?=
 =?us-ascii?Q?V9B9QkEi8mkL5xthRr7Q7wRwZDYPqUIvEmEtM1sOi+L/dUlIkYTJG4k/B+J6?=
 =?us-ascii?Q?CuKu78CywnDpTFxF5YpvxFoonGzeAz3TxYiTtLqoJ9TJoqaALoI3tc02H79k?=
 =?us-ascii?Q?qBZzzH0Ggzg0tbQYDKXOw2GurSDW5zZIc2ybCw9lMn3v4eAwMZZaWTJavVDH?=
 =?us-ascii?Q?GHybFehlKR7Q9NXaQoWBgZ+0RAWkSNCIFlAmqMk/siTTScGaJQ3fEuPG49tp?=
 =?us-ascii?Q?jDYt3xBp6Au527HGGTEdstt9rUB2aG8Qh5l5plGQ335Qr1RkslDsw7JAxHjy?=
 =?us-ascii?Q?sMyZ0IzxtlJQy39RbLcFUl2s6WZ9xOSIC86LMrvgW8ro9d0qunU6EgNmUa6/?=
 =?us-ascii?Q?qeiFVW2R4KBTeDBS9KndXsTg6urCDJ6OqmB3XEvUBJ4qmRd+XSEVkcHyIX25?=
 =?us-ascii?Q?Y6DyTEc6GbI0y9IY0G6vl9RxT/h00Utbt7CAYq/SbzqMgDHge7DwHXTe+qdI?=
 =?us-ascii?Q?6GEMX8c0HzzjOuvJXGUIqz2EeUjBJlQ4HfOM97Tt1bjKLwu7Uia4Mr0k6To3?=
 =?us-ascii?Q?LY0I23nTNp4YoNF3o9vI3UuyhlzN1HY62rnjLCJwHIeDK1sHg2rmBeeS+UpV?=
 =?us-ascii?Q?E4Shacr1rcgIb5up6zgx6GVAKrkjO2GsFKbeT0sVfkmw6sHpoP0adqBsHvIK?=
 =?us-ascii?Q?0XHpjyUKlShqKEs8KFCsz+xwpZDdP9FNHJvROyEns0ug6/J3VeWY2WKXGb+S?=
 =?us-ascii?Q?VDPg3daeuJUO9lBIxCdZ672OxaIpMdmTNe/8PckKFO/Ln+naGEBg2V8QddQK?=
 =?us-ascii?Q?cvq1OfZycXHBrR/Mis8yIk6HPnKmZzY803pKo3fNBRg/3qr1nr5wA9UcP9qV?=
 =?us-ascii?Q?DXs6XOBaNYYlUQf93Jzwl2r81pXRvHHbuwJ3Rob/Bcx7nu5NXxuty4VCkTE3?=
 =?us-ascii?Q?och02UXyInFXEK91eJLWBd3p/tnHbDVzcBqGnmP81gkx19WJSTFebW/eucsP?=
 =?us-ascii?Q?T3z/j9wqWXkJEjZMSXmkG5fbLznoCYTuOPuBerrYnAompmWtI4Ptud6eMIPh?=
 =?us-ascii?Q?hcxBCbi6caFCmFTWHZJTTGFgtg3Nft40KbzLNAOOJWYH4mojT6MWKlPuuC0b?=
 =?us-ascii?Q?kleRmArOd5LZV+lapJk8gPcncfhDViTpLSInbX7yYYmg/6hOdPPY+dIUMtJL?=
 =?us-ascii?Q?JhKkHTuYoFXSwIv2p1JaqMcbU7FPd9BtvSEpnU75Qv30cjyITGVcTALp4j3c?=
 =?us-ascii?Q?9zvs8K4eSjJsWa6+4ezySBNCQ9IzpBorrqVMCRjBFmu4NDg7Gqhs5ZzmlJit?=
 =?us-ascii?Q?JCTv2A6FKXVqKUu9ekKzNuO3Ii6UmnscmrDtcCzb46VZLTMmGwEQnDu5cSIz?=
 =?us-ascii?Q?mENPrjsqREwIrVl7ogEg0wyy872z5dGtzpPbgTOoxFj2CNjdVb1LqDCGv6UB?=
 =?us-ascii?Q?YZL7C4bvqX5axUd9AH0H5/BuAItwVwEVXy0Buq5BQmNfgPozdOcQkok8pMVC?=
 =?us-ascii?Q?XMVj8btEohCxPDAaUlbzQF1eqiqNF6x8aSvukwSKrtEOnHIxlJDzknZBGlN+?=
 =?us-ascii?Q?EwVmUg=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 88f8f182-298d-4f58-2f5f-08d9cfd2017b
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB2576.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jan 2022 22:31:52.7156
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: QD9dOxYEUcGHpPi+FcpE0MI2nRGc8Gji2OBxabDWm4fYR5VtDKVsXZBUPVG64Re8Lnqwdn9I6ugD28W3u5g4ELmpIxdw9BnO+u3DxUyK2TI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR10MB2573
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10217 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxscore=0 bulkscore=0
 spamscore=0 adultscore=0 phishscore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2201040143
X-Proofpoint-GUID: 9KfRaklx_0qArIExAAxuL1ymaxdhtVTP
X-Proofpoint-ORIG-GUID: 9KfRaklx_0qArIExAAxuL1ymaxdhtVTP
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2021-12-10 09:43:07 -0600, Brijesh Singh wrote:
> The set_memory_{encrypt,decrypt}() are used for changing the pages

s/set_memory_{encrypt,decrypt}/snp_set_memory_{shared,private}/

> from decrypted (shared) to encrypted (private) and vice versa.
> When SEV-SNP is active, the page state transition needs to go through
> additional steps.
> 
> If the page is transitioned from shared to private, then perform the
> following after the encryption attribute is set in the page table:
> 
> 1. Issue the page state change VMGEXIT to add the memory region in
>    the RMP table.
> 2. Validate the memory region after the RMP entry is added.
> 
> To maintain the security guarantees, if the page is transitioned from
> private to shared, then perform the following before encryption attribute
> is removed from the page table:
> 
> 1. Invalidate the page.
> 2. Issue the page state change VMGEXIT to remove the page from RMP table.
> 
> To change the page state in the RMP table, use the Page State Change
> VMGEXIT defined in the GHCB specification.
> 
> The GHCB specification provides the flexibility to use either 4K or 2MB
> page size in during the page state change (PSC) request. For now use the
> 4K page size for all the PSC until page size tracking is supported in the
> kernel.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>

[snip]

> diff --git a/arch/x86/kernel/sev.c b/arch/x86/kernel/sev.c
> index 2971aa280ce6..35c772bf9f6c 100644
> --- a/arch/x86/kernel/sev.c
> +++ b/arch/x86/kernel/sev.c
> @@ -574,7 +574,7 @@ static void pvalidate_pages(unsigned long vaddr, unsigned int npages, bool valid
>  	}
>  }
>  
> -static void __init early_set_page_state(unsigned long paddr, unsigned int npages, enum psc_op op)
> +static void __init early_set_pages_state(unsigned long paddr, unsigned int npages, enum psc_op op)

Is there a need to change the name? "npages" can take a value of 1 too.
Hence, early_set_page_state() appears to be a better name!

> +		/*
> +		 * Page State Change VMGEXIT can pass error code through
> +		 * exit_info_2.
> +		 */

Collapse into one line?

> +void snp_set_memory_shared(unsigned long vaddr, unsigned int npages)
> +{
> +	if (!cc_platform_has(CC_ATTR_SEV_SNP))
> +		return;
> +
> +	pvalidate_pages(vaddr, npages, 0);

Replace '0' with "false"?

> +
> +	set_pages_state(vaddr, npages, SNP_PAGE_STATE_SHARED);
> +}
> +
> +void snp_set_memory_private(unsigned long vaddr, unsigned int npages)
> +{
> +	if (!cc_platform_has(CC_ATTR_SEV_SNP))
> +		return;
> +
> +	set_pages_state(vaddr, npages, SNP_PAGE_STATE_PRIVATE);
> +
> +	pvalidate_pages(vaddr, npages, 1);

Replace '1' with "true"?

Venu