On 1/6/22 2:16 PM, Venu Busireddy wrote:
On 2022-01-06 13:06:13 -0600, Tom Lendacky wrote:
On 1/6/22 11:40 AM, Venu Busireddy wrote:
On 2022-01-05 15:39:22 -0600, Brijesh Singh wrote:
On 1/5/22 2:27 PM, Dave Hansen wrote:
On 1/5/22 11:52, Brijesh Singh wrote:
for (; vaddr < vaddr_end; vaddr += PMD_SIZE) {
+ /*
+ * When SEV-SNP is active then transition the
page to shared in the RMP
+ * table so that it is consistent with the page
table attribute change.
+ */
+ early_snp_set_memory_shared(__pa(vaddr),
__pa(vaddr), PTRS_PER_PMD);
Shouldn't the first argument be vaddr as below?
Nope, sme_postprocess_startup() is called while we are fixing the
initial page table and running with identity mapping (so va == pa).
I'm not sure I've ever seen a line of code that wanted a comment so badly.
The early_snp_set_memory_shared() call the PVALIDATE instruction to clear
the validated bit from the BSS region. The PVALIDATE instruction needs a
virtual address, so we need to use the identity mapped virtual address so
that PVALIDATE can clear the validated bit. I will add more comments to
clarify it.
Looking forward to see your final comments explaining this. I can't
still follow why, when PVALIDATE needs the virtual address, we are doing
a __pa() on the vaddr.
It's because of the phase of booting that the kernel is in. At this point,
the kernel is running in identity mapped mode (VA == PA). The
__start_bss_decrypted address is a regular kernel address, e.g. for the
kernel I'm on it is 0xffffffffa7600000. Since the PVALIDATE instruction
requires a valid virtual address, the code needs to perform a __pa() against
__start_bss_decrypted to get the identity mapped virtual address that is
currently in place.
Perhaps my confusion stems from the fact that __pa(x) is defined either
as "((unsigned long ) (x))" (for the cases where paddr and vaddr are
same), or as "__phys_addr((unsigned long )(x))", where a vaddr needs to
be converted to a paddr. If the paddr and vaddr are same in our case,
what exactly is the _pa(vaddr) doing to the vaddr?
But they are not the same and the head64.c file is compiled without
defining a value for __pa(), so __pa() is __phys_addr((unsigned long)(x)).
The virtual address value of __start_bss_decrypted, for me, is
0xffffffffa7600000, and that does not equal the physical address (take a
look at your /proc/kallsyms). However, since the code is running identity
mapped and with a page table without kernel virtual addresses, it cannot
use that value. It needs to convert that value to the identity mapped
virtual address and that is done using __pa(). Only after using __pa() on
__start_bss_decrypted, do you get a virtual address that maps to and is
equal to the physical address.
You may want to step through the boot code using KVM to see what the
environment is and why things are done the way they are.
Thanks,
Tom
Venu
