Re: [PATCH v8 12/40] x86/sev: Add helper for validating pages in early enc attribute changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Venu,


On 1/3/22 5:28 PM, Venu Busireddy wrote:
...

+
+	 /*
+	  * Ask the hypervisor to mark the memory pages as private in the RMP
+	  * table.
+	  */

Indentation is off. While at it, you may want to collapse it into a one
line comment.


Based on previous review feedback I tried to keep the comment to 80 character limit.

+	early_set_page_state(paddr, npages, SNP_PAGE_STATE_PRIVATE);
+
+	/* Validate the memory pages after they've been added in the RMP table. */
+	pvalidate_pages(vaddr, npages, 1);
+}
+
+void __init early_snp_set_memory_shared(unsigned long vaddr, unsigned long paddr,
+					unsigned int npages)
+{
+	if (!cc_platform_has(CC_ATTR_SEV_SNP))
+		return;
+
+	/*
+	 * Invalidate the memory pages before they are marked shared in the
+	 * RMP table.
+	 */

Collapse into one line?


same as above.

...

+		/*
+		 * ON SNP, the page state in the RMP table must happen
+		 * before the page table updates.
+		 */
+		early_snp_set_memory_shared((unsigned long)__va(pa), pa, 1);

I know "1" implies "true", but to emphasize that the argument is
actually a boolean, could you please change the "1" to "true?"


I assume you mean the last argument to the early_snp_set_memory_{private,shared}. Please note that its a number of pages (unsigned int). The 'true' does not make sense to me.

+	}
+
  	/* Change the page encryption mask. */
  	new_pte = pfn_pte(pfn, new_prot);
  	set_pte_atomic(kpte, new_pte);
+
+	/*
+	 * If page is set encrypted in the page table, then update the RMP table to
+	 * add this page as private.
+	 */
+	if (enc)
+		early_snp_set_memory_private((unsigned long)__va(pa), pa, 1);

Here too, could you please change the "1" to "true?"


same as above.

thanks




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux