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
