LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Tom Lendacky <thomas.lendacky@amd.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: "Kuppuswamy,
	Sathyanarayanan"  <sathyanarayanan.kuppuswamy@linux.intel.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org,
	iommu@lists.linux-foundation.org, kvm@vger.kernel.org,
	linux-efi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
	linux-graphics-maintainer@vmware.com,
	amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	kexec@lists.infradead.org, linux-fsdevel@vger.kernel.org,
	Borislav Petkov <bp@alien8.de>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Joerg Roedel <joro@8bytes.org>, Andi Kleen <ak@linux.intel.com>,
	Tianyu Lan <Tianyu.Lan@microsoft.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Will Deacon <will@kernel.org>, Dave Young <dyoung@redhat.com>,
	Baoquan He <bhe@redhat.com>
Subject: Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()
Date: Wed, 11 Aug 2021 10:52:55 -0500	[thread overview]
Message-ID: <0a819549-e481-c004-7da8-82ba427b13ce@amd.com> (raw)
In-Reply-To: <20210811121917.ghxi7g4mctuybhbk@box.shutemov.name>

On 8/11/21 7:19 AM, Kirill A. Shutemov wrote:
> On Tue, Aug 10, 2021 at 02:48:54PM -0500, Tom Lendacky wrote:
>> On 8/10/21 1:45 PM, Kuppuswamy, Sathyanarayanan wrote:
>>>
>>>
>>> On 7/27/21 3:26 PM, Tom Lendacky wrote:
>>>> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
>>>> index de01903c3735..cafed6456d45 100644
>>>> --- a/arch/x86/kernel/head64.c
>>>> +++ b/arch/x86/kernel/head64.c
>>>> @@ -19,7 +19,7 @@
>>>>   #include <linux/start_kernel.h>
>>>>   #include <linux/io.h>
>>>>   #include <linux/memblock.h>
>>>> -#include <linux/mem_encrypt.h>
>>>> +#include <linux/protected_guest.h>
>>>>   #include <linux/pgtable.h>
>>>>     #include <asm/processor.h>
>>>> @@ -285,7 +285,7 @@ unsigned long __head __startup_64(unsigned long
>>>> physaddr,
>>>>        * there is no need to zero it after changing the memory encryption
>>>>        * attribute.
>>>>        */
>>>> -    if (mem_encrypt_active()) {
>>>> +    if (prot_guest_has(PATTR_MEM_ENCRYPT)) {
>>>>           vaddr = (unsigned long)__start_bss_decrypted;
>>>>           vaddr_end = (unsigned long)__end_bss_decrypted;
>>>
>>>
>>> Since this change is specific to AMD, can you replace PATTR_MEM_ENCRYPT with
>>> prot_guest_has(PATTR_SME) || prot_guest_has(PATTR_SEV). It is not used in
>>> TDX.
>>
>> This is a direct replacement for now.
> 
> With current implementation of prot_guest_has() for TDX it breaks boot for
> me.
> 
> Looking at code agains, now I *think* the reason is accessing a global
> variable from __startup_64() inside TDX version of prot_guest_has().
> 
> __startup_64() is special. If you access any global variable you need to
> use fixup_pointer(). See comment before __startup_64().
> 
> I'm not sure how you get away with accessing sme_me_mask directly from
> there. Any clues? Maybe just a luck and complier generates code just right
> for your case, I donno.

Hmm... yeah, could be that the compiler is using rip-relative addressing
for it because it lives in the .data section?

For the static variables in mem_encrypt_identity.c I did an assembler rip
relative LEA, but probably could have passed physaddr to sme_enable() and
used a fixup_pointer() style function, instead.

> 
> A separate point is that TDX version of prot_guest_has() relies on
> cpu_feature_enabled() which is not ready at this point.

Does TDX have to do anything special to make memory able to be shared with
the hypervisor?  You might have to use something that is available earlier
than cpu_feature_enabled() in that case (should you eventually support
kvmclock).

> 
> I think __bss_decrypted fixup has to be done if sme_me_mask is non-zero.
> Or just do it uncoditionally because it's NOP for sme_me_mask == 0.

For SNP, we'll have to additionally call the HV to update the RMP to make
the memory shared. But that could also be done unconditionally since the
early_snp_set_memory_shared() routine will check for SNP before doing
anything.

Thanks,
Tom

> 
>> I think the change you're requesting
>> should be done as part of the TDX support patches so it's clear why it is
>> being changed.
>>
>> But, wouldn't TDX still need to do something with this shared/unencrypted
>> area, though? Or since it is shared, there's actually nothing you need to
>> do (the bss decrpyted section exists even if CONFIG_AMD_MEM_ENCRYPT is not
>> configured)?
> 
> AFAICS, only kvmclock uses __bss_decrypted. We don't enable kvmclock in
> TDX at the moment. It may change in the future.
> 

  reply	other threads:[~2021-08-11 15:53 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 22:26 [PATCH 00/11] Implement generic prot_guest_has() helper function Tom Lendacky
2021-07-27 22:26 ` [PATCH 01/11] mm: Introduce a function to check for virtualization protection features Tom Lendacky
2021-07-28 13:17   ` Christoph Hellwig
2021-07-28 16:28     ` Borislav Petkov
2021-08-02 10:34   ` Joerg Roedel
2021-08-11 14:53   ` Kuppuswamy, Sathyanarayanan
2021-08-11 15:39     ` Tom Lendacky
2021-07-27 22:26 ` [PATCH 02/11] x86/sev: Add an x86 version of prot_guest_has() Tom Lendacky
2021-07-28 13:22   ` Christoph Hellwig
2021-07-29 14:24     ` Tom Lendacky
2021-08-02 10:35   ` Joerg Roedel
2021-07-27 22:26 ` [PATCH 03/11] powerpc/pseries/svm: Add a powerpc " Tom Lendacky
2021-07-27 22:26 ` [PATCH 04/11] x86/sme: Replace occurrences of sme_active() with prot_guest_has() Tom Lendacky
2021-08-02 10:37   ` Joerg Roedel
2021-07-27 22:26 ` [PATCH 05/11] x86/sev: Replace occurrences of sev_active() " Tom Lendacky
2021-08-02 10:42   ` Joerg Roedel
2021-07-27 22:26 ` [PATCH 06/11] x86/sev: Replace occurrences of sev_es_active() " Tom Lendacky
2021-08-02 10:45   ` Joerg Roedel
2021-08-09 21:59     ` Tom Lendacky
2021-08-09 22:08       ` Kuppuswamy, Sathyanarayanan
2021-07-27 22:26 ` [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() " Tom Lendacky
2021-07-30 22:34   ` Sean Christopherson
2021-08-09 21:55     ` Tom Lendacky
2021-08-02 12:42   ` Christophe Leroy
2021-08-09 22:04     ` Tom Lendacky
2021-08-10 18:45   ` Kuppuswamy, Sathyanarayanan
2021-08-10 19:48     ` Tom Lendacky
2021-08-10 20:09       ` Kuppuswamy, Sathyanarayanan
2021-08-11 12:19       ` Kirill A. Shutemov
2021-08-11 15:52         ` Tom Lendacky [this message]
2021-08-12 10:07           ` Kirill A. Shutemov
2021-08-13 17:08             ` Tom Lendacky
2021-08-13 20:17               ` Tom Lendacky
2021-07-27 22:26 ` [PATCH 08/11] mm: Remove the now unused mem_encrypt_active() function Tom Lendacky
2021-08-02 10:47   ` Joerg Roedel
2021-07-27 22:26 ` [PATCH 09/11] x86/sev: " Tom Lendacky
2021-08-02 10:46   ` Joerg Roedel
2021-07-27 22:26 ` [PATCH 10/11] powerpc/pseries/svm: " Tom Lendacky
2021-07-27 22:26 ` [PATCH 11/11] s390/mm: " Tom Lendacky
2021-07-27 22:37 ` [PATCH 00/11] Implement generic prot_guest_has() helper function Tom Lendacky
2021-07-28 11:50 ` Christian König
2021-08-09  1:41 ` Kuppuswamy, Sathyanarayanan
2021-08-09 22:16   ` Tom Lendacky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a819549-e481-c004-7da8-82ba427b13ce@amd.com \
    --to=thomas.lendacky@amd.com \
    --cc=Tianyu.Lan@microsoft.com \
    --cc=airlied@linux.ie \
    --cc=ak@linux.intel.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=bhe@redhat.com \
    --cc=bp@alien8.de \
    --cc=brijesh.singh@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dave.hansen@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=dyoung@redhat.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=joro@8bytes.org \
    --cc=kexec@lists.infradead.org \
    --cc=kirill@shutemov.name \
    --cc=kvm@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-graphics-maintainer@vmware.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mingo@redhat.com \
    --cc=mripard@kernel.org \
    --cc=peterz@infradead.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    --cc=tglx@linutronix.de \
    --cc=tzimmermann@suse.de \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --subject='Re: [PATCH 07/11] treewide: Replace the use of mem_encrypt_active() with prot_guest_has()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).