Hi Peter,
On 5/10/21 12:30 PM, Peter Gonda wrote:
>> +static int snp_make_page_shared(struct kvm_vcpu *vcpu, gpa_t gpa, kvm_pfn_t pfn, int level)
>> +{
>> + struct rmpupdate val;
>> + int rc, rmp_level;
>> + struct rmpentry *e;
>> +
>> + e = snp_lookup_page_in_rmptable(pfn_to_page(pfn), &rmp_level);
>> + if (!e)
>> + return -EINVAL;
>> +
>> + if (!rmpentry_assigned(e))
>> + return 0;
>> +
>> + /* Log if the entry is validated */
>> + if (rmpentry_validated(e))
>> + pr_debug_ratelimited("Remove RMP entry for a validated gpa 0x%llx\n", gpa);
>> +
>> + /*
>> + * Is the page part of an existing 2M RMP entry ? Split the 2MB into multiple
>> + * of 4K-page before making the memory shared.
>> + */
>> + if ((level == PG_LEVEL_4K) && (rmp_level == PG_LEVEL_2M)) {
>> + rc = snp_rmptable_psmash(vcpu, pfn);
>> + if (rc)
>> + return rc;
>> + }
>> +
>> + memset(&val, 0, sizeof(val));
>> + val.pagesize = X86_TO_RMP_PG_LEVEL(level);
> This is slightly different from Rev 2.00 of the GHCB spec. This
> defaults to 2MB page sizes, when the spec says the only valid settings
> for level are 0 -> 4k pages or 1 -> 2MB pages. Should this enforce the
> same strictness as the spec?
The caller of the snp_make_page_shared() must pass the x86 page level.
We should reach here after all the guest provide value have passed
through checks.
The call sequence in this case should be:
snp_handle_vmgexit_msr_protocol()
__snp_handle_page_state_change(vcpu, gfn_to_gpa(gfn), PG_LEVEL_4K)
snp_make_page_shared(..., level)
Am I missing something ?
>> + return rmpupdate(pfn_to_page(pfn), &val);
>> +}
>> +
>> +static int snp_make_page_private(struct kvm_vcpu *vcpu, gpa_t gpa, kvm_pfn_t pfn, int level)
>> +{
>> + struct kvm_sev_info *sev = &to_kvm_svm(vcpu->kvm)->sev_info;
>> + struct rmpupdate val;
>> + struct rmpentry *e;
>> + int rmp_level;
>> +
>> + e = snp_lookup_page_in_rmptable(pfn_to_page(pfn), &rmp_level);
>> + if (!e)
>> + return -EINVAL;
>> +
>> + /* Log if the entry is validated */
>> + if (rmpentry_validated(e))
>> + pr_err_ratelimited("Asked to make a pre-validated gpa %llx private\n", gpa);
>> +
>> + memset(&val, 0, sizeof(val));
>> + val.gpa = gpa;
>> + val.asid = sev->asid;
>> + val.pagesize = X86_TO_RMP_PG_LEVEL(level);
> Same comment as above.
See my above response.
>
>> + val.assigned = true;
>> +
>> + return rmpupdate(pfn_to_page(pfn), &val);
>> +}
