Re: [PATCH v8 13/40] x86/kernel: Make the bss.decrypted section shared in RMP table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux