On 5/27/21 6:49 AM, Borislav Petkov wrote:
> On Fri, Apr 30, 2021 at 07:16:12AM -0500, Brijesh Singh wrote:
>> + /*
>> + * The ROM memory is not part of the E820 system RAM and is not pre-validated
>> + * by the BIOS. The kernel page table maps the ROM region as encrypted memory,
>> + * the SEV-SNP requires the encrypted memory must be validated before the
>> + * access. Validate the ROM before accessing it.
>> + */
>> + n = ((system_rom_resource.end + 1) - video_rom_resource.start) >> PAGE_SHIFT;
>> + early_snp_set_memory_private((unsigned long)__va(video_rom_resource.start),
>> + video_rom_resource.start, n);
> From last review:
>
> I don't like this sprinkling of SNP-special stuff that needs to be done,
> around the tree. Instead, pls define a function called
>
> snp_prep_memory(unsigned long pa, unsigned int num_pages, enum operation);
In the previous patch we were doing:
if (sev_snp_active()) {
early_set_memory_private(....)
}
Based on your feedback on other patches, I moved the sev_snp_active()
check inside the function. The callsites can now make unconditional call
to change the page state. After implementing that feedback, I don't see
a strong reason for yet another helper unless I am missing something:
snp_prep_memory(pa, n, SNP_PAGE_PRIVATE) == snp_set_memory_private(pa, n)
snp_prep_memory(pa, n, SNP_PAGE_SHARED) == snp_set_memory_shared(pa, n)
Let me know if you still think that snp_prep_memory() helper is required.
-Brijesh
> or so which does all the manipulation needed and the callsites only
> simply unconditionally call that function so that all detail is
> extracted and optimized away when not config-enabled.
>
