[AMD Official Use Only - General]
Hello Dov,
>> +int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid,
>> +bool immutable) {
>> + struct rmpupdate val;
>> +
>> + if (!pfn_valid(pfn))
>> + return -EINVAL;
>> +
>Should we add more checks on the arguments?
>1. asid must be > 0
>2. gpa must be aligned according to 'level'
>3. gpa must be below the maximal address for the guest
Ok, yes it surely makes sense to add more checks on the arguments.
>"Note that the guest physical address space is limited according to CPUID Fn80000008_EAX and thus the GPAs used by the firmware in measurement calculation are equally limited. Hypervisors should not attempt to map pages outside of this limit."
>(-SNP ABI spec page 86, section 8.17 SNP_LAUNCH_UPDATE)
>But note that in patch 28 of this series we have:
>+ /* Transition the VMSA page to a firmware state. */
>+ ret = rmp_make_private(pfn, -1, PG_LEVEL_4K, sev->asid, true);
>That (u64)(-1) value for the gpa argument violates conditions 2 and 3 from my list above.
>And indeed when calculating measurements we see that the GPA value for the VMSA pages is 0x0000FFFF_FFFFF000, and not (u64)(-1). [1] [2]
>Instead of checks, we can mask the gpa argument so that rmpupdate will get the correct value. Not sure which approach is preferable.
Well, the firmware is anyway masking the gpa argument as you observe in the launch digest, so probably we should do the same here too.
Thanks,
Ashish
