On 12/17/21 2:47 PM, Venu Busireddy wrote:
>> * the caches.
>> */
>> - if ((set | clr) & _PAGE_ENC)
>> + if ((set | clr) & _PAGE_ENC) {
>> clflush_page(address);
>>
>> + /*
>> + * If the encryption attribute is being cleared, then change
>> + * the page state to shared in the RMP table.
>> + */
>> + if (clr)
> This function is also called by set_page_non_present() with clr set to
> _PAGE_PRESENT. Do we want to change the page state to shared even when
> the page is not present? If not, shouldn't the check be (clr & _PAGE_ENC)?
I am not able to follow your comment. Here we only pay attention to the
encryption attribute, if encryption attribute is getting cleared then
make PSC. In the case ov set_page_non_present(), the outer if() block
will return false. Am I missing something ?
>> + /*
>> + * If private -> shared then invalidate the page before requesting the
> This comment is confusing. We don't know what the present state is,
> right? If we don't, shouldn't we just say:
>
> If the operation is SNP_PAGE_STATE_SHARED, invalidate the page before
> requesting the state change in the RMP table.
>
By default all the pages are private, so I don't see any issue with
saying "private -> shared".
>> + * state change in the RMP table.
>> + */
>> + if (op == SNP_PAGE_STATE_SHARED && pvalidate(paddr, RMP_PG_SIZE_4K, 0))
>> + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
>> +
>> + /* Issue VMGEXIT to change the page state in RMP table. */
>> + sev_es_wr_ghcb_msr(GHCB_MSR_PSC_REQ_GFN(paddr >> PAGE_SHIFT, op));
>> + VMGEXIT();
>> +
>> + /* Read the response of the VMGEXIT. */
>> + val = sev_es_rd_ghcb_msr();
>> + if ((GHCB_RESP_CODE(val) != GHCB_MSR_PSC_RESP) || GHCB_MSR_PSC_RESP_VAL(val))
>> + sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PSC);
>> +
>> + /*
>> + * Now that page is added in the RMP table, validate it so that it is
>> + * consistent with the RMP entry.
> The page is not "added", right? Shouldn't we just say:
Technically, PSC modifies the RMP entry, so I should use that instead
of calling "added".
> Validate the page so that it is consistent with the RMP entry.
Yes, I am okay with it.
> Venu
