mbox series

[00/11] Implement generic prot_guest_has() helper function

Message ID cover.1627424773.git.thomas.lendacky@amd.com
Headers show
Series Implement generic prot_guest_has() helper function | expand

Message

Tom Lendacky July 27, 2021, 10:26 p.m. UTC
This patch series provides a generic helper function, prot_guest_has(),
to replace the sme_active(), sev_active(), sev_es_active() and
mem_encrypt_active() functions.

It is expected that as new protected virtualization technologies are
added to the kernel, they can all be covered by a single function call
instead of a collection of specific function calls all called from the
same locations.

The powerpc and s390 patches have been compile tested only. Can the
folks copied on this series verify that nothing breaks for them.

Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Baoquan He <bhe@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Will Deacon <will@kernel.org>

---

Patches based on:
  https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
  commit 79e920060fa7 ("Merge branch 'WIP/fixes'")

Tom Lendacky (11):
  mm: Introduce a function to check for virtualization protection
    features
  x86/sev: Add an x86 version of prot_guest_has()
  powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
  x86/sme: Replace occurrences of sme_active() with prot_guest_has()
  x86/sev: Replace occurrences of sev_active() with prot_guest_has()
  x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()
  treewide: Replace the use of mem_encrypt_active() with
    prot_guest_has()
  mm: Remove the now unused mem_encrypt_active() function
  x86/sev: Remove the now unused mem_encrypt_active() function
  powerpc/pseries/svm: Remove the now unused mem_encrypt_active()
    function
  s390/mm: Remove the now unused mem_encrypt_active() function

 arch/Kconfig                               |  3 ++
 arch/powerpc/include/asm/mem_encrypt.h     |  5 --
 arch/powerpc/include/asm/protected_guest.h | 30 +++++++++++
 arch/powerpc/platforms/pseries/Kconfig     |  1 +
 arch/s390/include/asm/mem_encrypt.h        |  2 -
 arch/x86/Kconfig                           |  1 +
 arch/x86/include/asm/kexec.h               |  2 +-
 arch/x86/include/asm/mem_encrypt.h         | 13 +----
 arch/x86/include/asm/protected_guest.h     | 27 ++++++++++
 arch/x86/kernel/crash_dump_64.c            |  4 +-
 arch/x86/kernel/head64.c                   |  4 +-
 arch/x86/kernel/kvm.c                      |  3 +-
 arch/x86/kernel/kvmclock.c                 |  4 +-
 arch/x86/kernel/machine_kexec_64.c         | 19 +++----
 arch/x86/kernel/pci-swiotlb.c              |  9 ++--
 arch/x86/kernel/relocate_kernel_64.S       |  2 +-
 arch/x86/kernel/sev.c                      |  6 +--
 arch/x86/kvm/svm/svm.c                     |  3 +-
 arch/x86/mm/ioremap.c                      | 16 +++---
 arch/x86/mm/mem_encrypt.c                  | 60 +++++++++++++++-------
 arch/x86/mm/mem_encrypt_identity.c         |  3 +-
 arch/x86/mm/pat/set_memory.c               |  3 +-
 arch/x86/platform/efi/efi_64.c             |  9 ++--
 arch/x86/realmode/init.c                   |  8 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 +-
 drivers/gpu/drm/drm_cache.c                |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c        |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c        |  6 +--
 drivers/iommu/amd/init.c                   |  7 +--
 drivers/iommu/amd/iommu.c                  |  3 +-
 drivers/iommu/amd/iommu_v2.c               |  3 +-
 drivers/iommu/iommu.c                      |  3 +-
 fs/proc/vmcore.c                           |  6 +--
 include/linux/mem_encrypt.h                |  4 --
 include/linux/protected_guest.h            | 37 +++++++++++++
 kernel/dma/swiotlb.c                       |  4 +-
 36 files changed, 218 insertions(+), 104 deletions(-)
 create mode 100644 arch/powerpc/include/asm/protected_guest.h
 create mode 100644 arch/x86/include/asm/protected_guest.h
 create mode 100644 include/linux/protected_guest.h

Comments

Tom Lendacky July 27, 2021, 10:37 p.m. UTC | #1
On 7/27/21 5:26 PM, Tom Lendacky wrote:
> This patch series provides a generic helper function, prot_guest_has(),
> to replace the sme_active(), sev_active(), sev_es_active() and
> mem_encrypt_active() functions.
> 
> It is expected that as new protected virtualization technologies are
> added to the kernel, they can all be covered by a single function call
> instead of a collection of specific function calls all called from the
> same locations.
> 
> The powerpc and s390 patches have been compile tested only. Can the
> folks copied on this series verify that nothing breaks for them.

I wanted to get this out before I head out on vacation at the end of the
week. I'll only be out for a week, but I won't be able to respond to any
feedback until I get back.

I'm still not a fan of the name prot_guest_has() because it is used for
some baremetal checks, but really haven't been able to come up with
anything better. So take it with a grain of salt where the sme_active()
calls are replaced by prot_guest_has().

Also, let me know if the treewide changes in patch #7 need to be further
split out by tree.

Thanks,
Tom

> 
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Will Deacon <will@kernel.org>
> 
> ---
> 
> Patches based on:
>   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
>   commit 79e920060fa7 ("Merge branch 'WIP/fixes'")
> 
> Tom Lendacky (11):
>   mm: Introduce a function to check for virtualization protection
>     features
>   x86/sev: Add an x86 version of prot_guest_has()
>   powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
>   x86/sme: Replace occurrences of sme_active() with prot_guest_has()
>   x86/sev: Replace occurrences of sev_active() with prot_guest_has()
>   x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()
>   treewide: Replace the use of mem_encrypt_active() with
>     prot_guest_has()
>   mm: Remove the now unused mem_encrypt_active() function
>   x86/sev: Remove the now unused mem_encrypt_active() function
>   powerpc/pseries/svm: Remove the now unused mem_encrypt_active()
>     function
>   s390/mm: Remove the now unused mem_encrypt_active() function
> 
>  arch/Kconfig                               |  3 ++
>  arch/powerpc/include/asm/mem_encrypt.h     |  5 --
>  arch/powerpc/include/asm/protected_guest.h | 30 +++++++++++
>  arch/powerpc/platforms/pseries/Kconfig     |  1 +
>  arch/s390/include/asm/mem_encrypt.h        |  2 -
>  arch/x86/Kconfig                           |  1 +
>  arch/x86/include/asm/kexec.h               |  2 +-
>  arch/x86/include/asm/mem_encrypt.h         | 13 +----
>  arch/x86/include/asm/protected_guest.h     | 27 ++++++++++
>  arch/x86/kernel/crash_dump_64.c            |  4 +-
>  arch/x86/kernel/head64.c                   |  4 +-
>  arch/x86/kernel/kvm.c                      |  3 +-
>  arch/x86/kernel/kvmclock.c                 |  4 +-
>  arch/x86/kernel/machine_kexec_64.c         | 19 +++----
>  arch/x86/kernel/pci-swiotlb.c              |  9 ++--
>  arch/x86/kernel/relocate_kernel_64.S       |  2 +-
>  arch/x86/kernel/sev.c                      |  6 +--
>  arch/x86/kvm/svm/svm.c                     |  3 +-
>  arch/x86/mm/ioremap.c                      | 16 +++---
>  arch/x86/mm/mem_encrypt.c                  | 60 +++++++++++++++-------
>  arch/x86/mm/mem_encrypt_identity.c         |  3 +-
>  arch/x86/mm/pat/set_memory.c               |  3 +-
>  arch/x86/platform/efi/efi_64.c             |  9 ++--
>  arch/x86/realmode/init.c                   |  8 +--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 +-
>  drivers/gpu/drm/drm_cache.c                |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c        |  4 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_msg.c        |  6 +--
>  drivers/iommu/amd/init.c                   |  7 +--
>  drivers/iommu/amd/iommu.c                  |  3 +-
>  drivers/iommu/amd/iommu_v2.c               |  3 +-
>  drivers/iommu/iommu.c                      |  3 +-
>  fs/proc/vmcore.c                           |  6 +--
>  include/linux/mem_encrypt.h                |  4 --
>  include/linux/protected_guest.h            | 37 +++++++++++++
>  kernel/dma/swiotlb.c                       |  4 +-
>  36 files changed, 218 insertions(+), 104 deletions(-)
>  create mode 100644 arch/powerpc/include/asm/protected_guest.h
>  create mode 100644 arch/x86/include/asm/protected_guest.h
>  create mode 100644 include/linux/protected_guest.h
>
Christian König July 28, 2021, 11:50 a.m. UTC | #2
Am 28.07.21 um 00:26 schrieb Tom Lendacky:
> This patch series provides a generic helper function, prot_guest_has(),
> to replace the sme_active(), sev_active(), sev_es_active() and
> mem_encrypt_active() functions.
>
> It is expected that as new protected virtualization technologies are
> added to the kernel, they can all be covered by a single function call
> instead of a collection of specific function calls all called from the
> same locations.
>
> The powerpc and s390 patches have been compile tested only. Can the
> folks copied on this series verify that nothing breaks for them.

As GPU driver dev I'm only one end user of this, but at least from the 
high level point of view that makes totally sense to me.

Feel free to add an Acked-by: Christian König <christian.koenig@amd.com>.

We could run that through the AMD GPU unit tests, but I fear we actually 
don't test on a system with SEV/SME active.

Going to raise that on our team call today.

Regards,
Christian.

>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Will Deacon <will@kernel.org>
>
> ---
>
> Patches based on:
>    https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master
>    commit 79e920060fa7 ("Merge branch 'WIP/fixes'")
>
> Tom Lendacky (11):
>    mm: Introduce a function to check for virtualization protection
>      features
>    x86/sev: Add an x86 version of prot_guest_has()
>    powerpc/pseries/svm: Add a powerpc version of prot_guest_has()
>    x86/sme: Replace occurrences of sme_active() with prot_guest_has()
>    x86/sev: Replace occurrences of sev_active() with prot_guest_has()
>    x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()
>    treewide: Replace the use of mem_encrypt_active() with
>      prot_guest_has()
>    mm: Remove the now unused mem_encrypt_active() function
>    x86/sev: Remove the now unused mem_encrypt_active() function
>    powerpc/pseries/svm: Remove the now unused mem_encrypt_active()
>      function
>    s390/mm: Remove the now unused mem_encrypt_active() function
>
>   arch/Kconfig                               |  3 ++
>   arch/powerpc/include/asm/mem_encrypt.h     |  5 --
>   arch/powerpc/include/asm/protected_guest.h | 30 +++++++++++
>   arch/powerpc/platforms/pseries/Kconfig     |  1 +
>   arch/s390/include/asm/mem_encrypt.h        |  2 -
>   arch/x86/Kconfig                           |  1 +
>   arch/x86/include/asm/kexec.h               |  2 +-
>   arch/x86/include/asm/mem_encrypt.h         | 13 +----
>   arch/x86/include/asm/protected_guest.h     | 27 ++++++++++
>   arch/x86/kernel/crash_dump_64.c            |  4 +-
>   arch/x86/kernel/head64.c                   |  4 +-
>   arch/x86/kernel/kvm.c                      |  3 +-
>   arch/x86/kernel/kvmclock.c                 |  4 +-
>   arch/x86/kernel/machine_kexec_64.c         | 19 +++----
>   arch/x86/kernel/pci-swiotlb.c              |  9 ++--
>   arch/x86/kernel/relocate_kernel_64.S       |  2 +-
>   arch/x86/kernel/sev.c                      |  6 +--
>   arch/x86/kvm/svm/svm.c                     |  3 +-
>   arch/x86/mm/ioremap.c                      | 16 +++---
>   arch/x86/mm/mem_encrypt.c                  | 60 +++++++++++++++-------
>   arch/x86/mm/mem_encrypt_identity.c         |  3 +-
>   arch/x86/mm/pat/set_memory.c               |  3 +-
>   arch/x86/platform/efi/efi_64.c             |  9 ++--
>   arch/x86/realmode/init.c                   |  8 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 +-
>   drivers/gpu/drm/drm_cache.c                |  4 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c        |  4 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_msg.c        |  6 +--
>   drivers/iommu/amd/init.c                   |  7 +--
>   drivers/iommu/amd/iommu.c                  |  3 +-
>   drivers/iommu/amd/iommu_v2.c               |  3 +-
>   drivers/iommu/iommu.c                      |  3 +-
>   fs/proc/vmcore.c                           |  6 +--
>   include/linux/mem_encrypt.h                |  4 --
>   include/linux/protected_guest.h            | 37 +++++++++++++
>   kernel/dma/swiotlb.c                       |  4 +-
>   36 files changed, 218 insertions(+), 104 deletions(-)
>   create mode 100644 arch/powerpc/include/asm/protected_guest.h
>   create mode 100644 arch/x86/include/asm/protected_guest.h
>   create mode 100644 include/linux/protected_guest.h
>
Christoph Hellwig July 28, 2021, 1:17 p.m. UTC | #3
On Tue, Jul 27, 2021 at 05:26:04PM -0500, Tom Lendacky via iommu wrote:
> In prep for other protected virtualization technologies, introduce a

> generic helper function, prot_guest_has(), that can be used to check

> for specific protection attributes, like memory encryption. This is

> intended to eliminate having to add multiple technology-specific checks

> to the code (e.g. if (sev_active() || tdx_active())).


So common checks obviously make sense, but I really hate the stupid
multiplexer.  Having one well-documented helper per feature is much
easier to follow.

> +#define PATTR_MEM_ENCRYPT		0	/* Encrypted memory */

> +#define PATTR_HOST_MEM_ENCRYPT		1	/* Host encrypted memory */

> +#define PATTR_GUEST_MEM_ENCRYPT		2	/* Guest encrypted memory */

> +#define PATTR_GUEST_PROT_STATE		3	/* Guest encrypted state */


The kerneldoc comments on these individual helpers will give you plenty
of space to properly document what they indicate and what a (potential)
caller should do based on them.  Something the above comments completely
fail to.
Borislav Petkov July 28, 2021, 4:28 p.m. UTC | #4
On Wed, Jul 28, 2021 at 02:17:27PM +0100, Christoph Hellwig wrote:
> So common checks obviously make sense, but I really hate the stupid

> multiplexer.  Having one well-documented helper per feature is much

> easier to follow.


We had that in x86 - it was called cpu_has_<xxx> where xxx is the
feature bit. It didn't scale with the sheer amount of feature bits that
kept getting added so we do cpu_feature_enabled(X86_FEATURE_XXX) now.

The idea behind this is very similar - those protected guest flags
will only grow in the couple of tens range - at least - so having a
multiplexer is a lot simpler, I'd say, than having a couple of tens of
helpers. And those PATTR flags should have good, readable names, btw.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Joerg Roedel Aug. 2, 2021, 10:34 a.m. UTC | #5
On Tue, Jul 27, 2021 at 05:26:04PM -0500, Tom Lendacky wrote:
> In prep for other protected virtualization technologies, introduce a
> generic helper function, prot_guest_has(), that can be used to check
> for specific protection attributes, like memory encryption. This is
> intended to eliminate having to add multiple technology-specific checks
> to the code (e.g. if (sev_active() || tdx_active())).
> 
> Co-developed-by: Andi Kleen <ak@linux.intel.com>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> Co-developed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Reviewed-by: Joerg Roedel <jroedel@suse.de>
Joerg Roedel Aug. 2, 2021, 10:42 a.m. UTC | #6
On Tue, Jul 27, 2021 at 05:26:08PM -0500, Tom Lendacky wrote:
> Replace occurrences of sev_active() with the more generic prot_guest_has()
> using PATTR_GUEST_MEM_ENCRYPT, except for in arch/x86/mm/mem_encrypt*.c
> where PATTR_SEV will be used. If future support is added for other memory
> encryption technologies, the use of PATTR_GUEST_MEM_ENCRYPT can be
> updated, as required, to use PATTR_SEV.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>

Reviewed-by: Joerg Roedel <jroedel@suse.de>
Joerg Roedel Aug. 2, 2021, 10:45 a.m. UTC | #7
On Tue, Jul 27, 2021 at 05:26:09PM -0500, Tom Lendacky wrote:
> @@ -48,7 +47,7 @@ static void sme_sev_setup_real_mode(struct trampoline_header *th)

>  	if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT))

>  		th->flags |= TH_FLAGS_SME_ACTIVE;

>  

> -	if (sev_es_active()) {

> +	if (prot_guest_has(PATTR_GUEST_PROT_STATE)) {

>  		/*

>  		 * Skip the call to verify_cpu() in secondary_startup_64 as it

>  		 * will cause #VC exceptions when the AP can't handle them yet.


Not sure how TDX will handle AP booting, are you sure it needs this
special setup as well? Otherwise a check for SEV-ES would be better
instead of the generic PATTR_GUEST_PROT_STATE.

Regards,

Joerg
Joerg Roedel Aug. 2, 2021, 10:46 a.m. UTC | #8
On Tue, Jul 27, 2021 at 05:26:12PM -0500, Tom Lendacky wrote:
> The mem_encrypt_active() function has been replaced by prot_guest_has(),

> so remove the implementation.

> 

> Cc: Thomas Gleixner <tglx@linutronix.de>

> Cc: Ingo Molnar <mingo@redhat.com>

> Cc: Borislav Petkov <bp@alien8.de>

> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>


Reviewed-by: Joerg Roedel <jroedel@suse.de>
Kuppuswamy Sathyanarayanan Aug. 9, 2021, 1:41 a.m. UTC | #9
Hi Tom,

On 7/27/21 3:26 PM, Tom Lendacky wrote:
> This patch series provides a generic helper function, prot_guest_has(),

> to replace the sme_active(), sev_active(), sev_es_active() and

> mem_encrypt_active() functions.

> 

> It is expected that as new protected virtualization technologies are

> added to the kernel, they can all be covered by a single function call

> instead of a collection of specific function calls all called from the

> same locations.

> 

> The powerpc and s390 patches have been compile tested only. Can the

> folks copied on this series verify that nothing breaks for them.


With this patch set, select ARCH_HAS_PROTECTED_GUEST and set
CONFIG_AMD_MEM_ENCRYPT=n, creates following error.

ld: arch/x86/mm/ioremap.o: in function `early_memremap_is_setup_data':
arch/x86/mm/ioremap.c:672: undefined reference to `early_memremap_decrypted'

It looks like early_memremap_is_setup_data() is not protected with
appropriate config.


> 

> Cc: Andi Kleen <ak@linux.intel.com>

> Cc: Andy Lutomirski <luto@kernel.org>

> Cc: Ard Biesheuvel <ardb@kernel.org>

> Cc: Baoquan He <bhe@redhat.com>

> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> Cc: Borislav Petkov <bp@alien8.de>

> Cc: Christian Borntraeger <borntraeger@de.ibm.com>

> Cc: Daniel Vetter <daniel@ffwll.ch>

> Cc: Dave Hansen <dave.hansen@linux.intel.com>

> Cc: Dave Young <dyoung@redhat.com>

> Cc: David Airlie <airlied@linux.ie>

> Cc: Heiko Carstens <hca@linux.ibm.com>

> Cc: Ingo Molnar <mingo@redhat.com>

> Cc: Joerg Roedel <joro@8bytes.org>

> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

> Cc: Maxime Ripard <mripard@kernel.org>

> Cc: Michael Ellerman <mpe@ellerman.id.au>

> Cc: Paul Mackerras <paulus@samba.org>

> Cc: Peter Zijlstra <peterz@infradead.org>

> Cc: Thomas Gleixner <tglx@linutronix.de>

> Cc: Thomas Zimmermann <tzimmermann@suse.de>

> Cc: Vasily Gorbik <gor@linux.ibm.com>

> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>

> Cc: Will Deacon <will@kernel.org>

> 

> ---

> 

> Patches based on:

>    https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master

>    commit 79e920060fa7 ("Merge branch 'WIP/fixes'")

> 

> Tom Lendacky (11):

>    mm: Introduce a function to check for virtualization protection

>      features

>    x86/sev: Add an x86 version of prot_guest_has()

>    powerpc/pseries/svm: Add a powerpc version of prot_guest_has()

>    x86/sme: Replace occurrences of sme_active() with prot_guest_has()

>    x86/sev: Replace occurrences of sev_active() with prot_guest_has()

>    x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()

>    treewide: Replace the use of mem_encrypt_active() with

>      prot_guest_has()

>    mm: Remove the now unused mem_encrypt_active() function

>    x86/sev: Remove the now unused mem_encrypt_active() function

>    powerpc/pseries/svm: Remove the now unused mem_encrypt_active()

>      function

>    s390/mm: Remove the now unused mem_encrypt_active() function

> 

>   arch/Kconfig                               |  3 ++

>   arch/powerpc/include/asm/mem_encrypt.h     |  5 --

>   arch/powerpc/include/asm/protected_guest.h | 30 +++++++++++

>   arch/powerpc/platforms/pseries/Kconfig     |  1 +

>   arch/s390/include/asm/mem_encrypt.h        |  2 -

>   arch/x86/Kconfig                           |  1 +

>   arch/x86/include/asm/kexec.h               |  2 +-

>   arch/x86/include/asm/mem_encrypt.h         | 13 +----

>   arch/x86/include/asm/protected_guest.h     | 27 ++++++++++

>   arch/x86/kernel/crash_dump_64.c            |  4 +-

>   arch/x86/kernel/head64.c                   |  4 +-

>   arch/x86/kernel/kvm.c                      |  3 +-

>   arch/x86/kernel/kvmclock.c                 |  4 +-

>   arch/x86/kernel/machine_kexec_64.c         | 19 +++----

>   arch/x86/kernel/pci-swiotlb.c              |  9 ++--

>   arch/x86/kernel/relocate_kernel_64.S       |  2 +-

>   arch/x86/kernel/sev.c                      |  6 +--

>   arch/x86/kvm/svm/svm.c                     |  3 +-

>   arch/x86/mm/ioremap.c                      | 16 +++---

>   arch/x86/mm/mem_encrypt.c                  | 60 +++++++++++++++-------

>   arch/x86/mm/mem_encrypt_identity.c         |  3 +-

>   arch/x86/mm/pat/set_memory.c               |  3 +-

>   arch/x86/platform/efi/efi_64.c             |  9 ++--

>   arch/x86/realmode/init.c                   |  8 +--

>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 +-

>   drivers/gpu/drm/drm_cache.c                |  4 +-

>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c        |  4 +-

>   drivers/gpu/drm/vmwgfx/vmwgfx_msg.c        |  6 +--

>   drivers/iommu/amd/init.c                   |  7 +--

>   drivers/iommu/amd/iommu.c                  |  3 +-

>   drivers/iommu/amd/iommu_v2.c               |  3 +-

>   drivers/iommu/iommu.c                      |  3 +-

>   fs/proc/vmcore.c                           |  6 +--

>   include/linux/mem_encrypt.h                |  4 --

>   include/linux/protected_guest.h            | 37 +++++++++++++

>   kernel/dma/swiotlb.c                       |  4 +-

>   36 files changed, 218 insertions(+), 104 deletions(-)

>   create mode 100644 arch/powerpc/include/asm/protected_guest.h

>   create mode 100644 arch/x86/include/asm/protected_guest.h

>   create mode 100644 include/linux/protected_guest.h

> 


-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Tom Lendacky Aug. 9, 2021, 9:59 p.m. UTC | #10
On 8/2/21 5:45 AM, Joerg Roedel wrote:
> On Tue, Jul 27, 2021 at 05:26:09PM -0500, Tom Lendacky wrote:

>> @@ -48,7 +47,7 @@ static void sme_sev_setup_real_mode(struct trampoline_header *th)

>>  	if (prot_guest_has(PATTR_HOST_MEM_ENCRYPT))

>>  		th->flags |= TH_FLAGS_SME_ACTIVE;

>>  

>> -	if (sev_es_active()) {

>> +	if (prot_guest_has(PATTR_GUEST_PROT_STATE)) {

>>  		/*

>>  		 * Skip the call to verify_cpu() in secondary_startup_64 as it

>>  		 * will cause #VC exceptions when the AP can't handle them yet.

> 

> Not sure how TDX will handle AP booting, are you sure it needs this

> special setup as well? Otherwise a check for SEV-ES would be better

> instead of the generic PATTR_GUEST_PROT_STATE.


Yes, I'm not sure either. I figure that change can be made, if needed, as
part of the TDX support.

Thanks,
Tom

> 

> Regards,

> 

> Joerg

>
Kuppuswamy Sathyanarayanan Aug. 9, 2021, 10:08 p.m. UTC | #11
On 8/9/21 2:59 PM, Tom Lendacky wrote:
>> Not sure how TDX will handle AP booting, are you sure it needs this

>> special setup as well? Otherwise a check for SEV-ES would be better

>> instead of the generic PATTR_GUEST_PROT_STATE.

> Yes, I'm not sure either. I figure that change can be made, if needed, as

> part of the TDX support.


We don't plan to set PROT_STATE. So it does not affect TDX.
For SMP, we use MADT ACPI table for AP booting.

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Tom Lendacky Aug. 9, 2021, 10:16 p.m. UTC | #12
On 8/8/21 8:41 PM, Kuppuswamy, Sathyanarayanan wrote:
> Hi Tom,

> 

> On 7/27/21 3:26 PM, Tom Lendacky wrote:

>> This patch series provides a generic helper function, prot_guest_has(),

>> to replace the sme_active(), sev_active(), sev_es_active() and

>> mem_encrypt_active() functions.

>>

>> It is expected that as new protected virtualization technologies are

>> added to the kernel, they can all be covered by a single function call

>> instead of a collection of specific function calls all called from the

>> same locations.

>>

>> The powerpc and s390 patches have been compile tested only. Can the

>> folks copied on this series verify that nothing breaks for them.

> 

> With this patch set, select ARCH_HAS_PROTECTED_GUEST and set

> CONFIG_AMD_MEM_ENCRYPT=n, creates following error.

> 

> ld: arch/x86/mm/ioremap.o: in function `early_memremap_is_setup_data':

> arch/x86/mm/ioremap.c:672: undefined reference to `early_memremap_decrypted'

> 

> It looks like early_memremap_is_setup_data() is not protected with

> appropriate config.


Ok, thanks for finding that. I'll fix that.

Thanks,
Tom

> 

> 

>>

>> Cc: Andi Kleen <ak@linux.intel.com>

>> Cc: Andy Lutomirski <luto@kernel.org>

>> Cc: Ard Biesheuvel <ardb@kernel.org>

>> Cc: Baoquan He <bhe@redhat.com>

>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>

>> Cc: Borislav Petkov <bp@alien8.de>

>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>

>> Cc: Daniel Vetter <daniel@ffwll.ch>

>> Cc: Dave Hansen <dave.hansen@linux.intel.com>

>> Cc: Dave Young <dyoung@redhat.com>

>> Cc: David Airlie <airlied@linux.ie>

>> Cc: Heiko Carstens <hca@linux.ibm.com>

>> Cc: Ingo Molnar <mingo@redhat.com>

>> Cc: Joerg Roedel <joro@8bytes.org>

>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

>> Cc: Maxime Ripard <mripard@kernel.org>

>> Cc: Michael Ellerman <mpe@ellerman.id.au>

>> Cc: Paul Mackerras <paulus@samba.org>

>> Cc: Peter Zijlstra <peterz@infradead.org>

>> Cc: Thomas Gleixner <tglx@linutronix.de>

>> Cc: Thomas Zimmermann <tzimmermann@suse.de>

>> Cc: Vasily Gorbik <gor@linux.ibm.com>

>> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>

>> Cc: Will Deacon <will@kernel.org>

>>

>> ---

>>

>> Patches based on:

>>   

>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftip%2Ftip.git&amp;data=04%7C01%7Cthomas.lendacky%40amd.com%7C563b5e30a3254f6739aa08d95ad6e242%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637640701228434514%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=vx9v4EmYqVTsJ7KSr97gQaBWJ%2Fq%2BE9NOzXMhe3Fp7T8%3D&amp;reserved=0

>> master

>>    commit 79e920060fa7 ("Merge branch 'WIP/fixes'")

>>

>> Tom Lendacky (11):

>>    mm: Introduce a function to check for virtualization protection

>>      features

>>    x86/sev: Add an x86 version of prot_guest_has()

>>    powerpc/pseries/svm: Add a powerpc version of prot_guest_has()

>>    x86/sme: Replace occurrences of sme_active() with prot_guest_has()

>>    x86/sev: Replace occurrences of sev_active() with prot_guest_has()

>>    x86/sev: Replace occurrences of sev_es_active() with prot_guest_has()

>>    treewide: Replace the use of mem_encrypt_active() with

>>      prot_guest_has()

>>    mm: Remove the now unused mem_encrypt_active() function

>>    x86/sev: Remove the now unused mem_encrypt_active() function

>>    powerpc/pseries/svm: Remove the now unused mem_encrypt_active()

>>      function

>>    s390/mm: Remove the now unused mem_encrypt_active() function

>>

>>   arch/Kconfig                               |  3 ++

>>   arch/powerpc/include/asm/mem_encrypt.h     |  5 --

>>   arch/powerpc/include/asm/protected_guest.h | 30 +++++++++++

>>   arch/powerpc/platforms/pseries/Kconfig     |  1 +

>>   arch/s390/include/asm/mem_encrypt.h        |  2 -

>>   arch/x86/Kconfig                           |  1 +

>>   arch/x86/include/asm/kexec.h               |  2 +-

>>   arch/x86/include/asm/mem_encrypt.h         | 13 +----

>>   arch/x86/include/asm/protected_guest.h     | 27 ++++++++++

>>   arch/x86/kernel/crash_dump_64.c            |  4 +-

>>   arch/x86/kernel/head64.c                   |  4 +-

>>   arch/x86/kernel/kvm.c                      |  3 +-

>>   arch/x86/kernel/kvmclock.c                 |  4 +-

>>   arch/x86/kernel/machine_kexec_64.c         | 19 +++----

>>   arch/x86/kernel/pci-swiotlb.c              |  9 ++--

>>   arch/x86/kernel/relocate_kernel_64.S       |  2 +-

>>   arch/x86/kernel/sev.c                      |  6 +--

>>   arch/x86/kvm/svm/svm.c                     |  3 +-

>>   arch/x86/mm/ioremap.c                      | 16 +++---

>>   arch/x86/mm/mem_encrypt.c                  | 60 +++++++++++++++-------

>>   arch/x86/mm/mem_encrypt_identity.c         |  3 +-

>>   arch/x86/mm/pat/set_memory.c               |  3 +-

>>   arch/x86/platform/efi/efi_64.c             |  9 ++--

>>   arch/x86/realmode/init.c                   |  8 +--

>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  4 +-

>>   drivers/gpu/drm/drm_cache.c                |  4 +-

>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c        |  4 +-

>>   drivers/gpu/drm/vmwgfx/vmwgfx_msg.c        |  6 +--

>>   drivers/iommu/amd/init.c                   |  7 +--

>>   drivers/iommu/amd/iommu.c                  |  3 +-

>>   drivers/iommu/amd/iommu_v2.c               |  3 +-

>>   drivers/iommu/iommu.c                      |  3 +-

>>   fs/proc/vmcore.c                           |  6 +--

>>   include/linux/mem_encrypt.h                |  4 --

>>   include/linux/protected_guest.h            | 37 +++++++++++++

>>   kernel/dma/swiotlb.c                       |  4 +-

>>   36 files changed, 218 insertions(+), 104 deletions(-)

>>   create mode 100644 arch/powerpc/include/asm/protected_guest.h

>>   create mode 100644 arch/x86/include/asm/protected_guest.h

>>   create mode 100644 include/linux/protected_guest.h

>>

>
Kuppuswamy Sathyanarayanan Aug. 11, 2021, 2:53 p.m. UTC | #13
On 7/27/21 3:26 PM, Tom Lendacky wrote:
> diff --git a/include/linux/protected_guest.h b/include/linux/protected_guest.h

> new file mode 100644

> index 000000000000..f8ed7b72967b

> --- /dev/null

> +++ b/include/linux/protected_guest.h

> @@ -0,0 +1,32 @@

> +/* SPDX-License-Identifier: GPL-2.0-only */

> +/*

> + * Protected Guest (and Host) Capability checks

> + *

> + * Copyright (C) 2021 Advanced Micro Devices, Inc.

> + *

> + * Author: Tom Lendacky<thomas.lendacky@amd.com>

> + */

> +

> +#ifndef _PROTECTED_GUEST_H

> +#define _PROTECTED_GUEST_H

> +

> +#ifndef __ASSEMBLY__


Can you include headers for bool type and false definition?

--- a/include/linux/protected_guest.h
+++ b/include/linux/protected_guest.h
@@ -12,6 +12,9 @@

  #ifndef __ASSEMBLY__

+#include <linux/types.h>
+#include <linux/stddef.h>

Otherwise, I see following errors in multi-config auto testing.

include/linux/protected_guest.h:40:15: error: unknown type name 'bool'
include/linux/protected_guest.h:40:63: error: 'false' undeclared (first use in this functi


> +

> +#define PATTR_MEM_ENCRYPT		0	/* Encrypted memory */

> +#define PATTR_HOST_MEM_ENCRYPT		1	/* Host encrypted memory */

> +#define PATTR_GUEST_MEM_ENCRYPT		2	/* Guest encrypted memory */

> +#define PATTR_GUEST_PROT_STATE		3	/* Guest encrypted state */

> +

> +#ifdef CONFIG_ARCH_HAS_PROTECTED_GUEST

> +

> +#include <asm/protected_guest.h>

> +

> +#else	/* !CONFIG_ARCH_HAS_PROTECTED_GUEST */

> +

> +static inline bool prot_guest_has(unsigned int attr) { return false; }

> +

> +#endif	/* CONFIG_ARCH_HAS_PROTECTED_GUEST */

> +

> +#endif	/* __ASSEMBLY__ */

> +

> +#endif	/* _PROTECTED_GUEST_H */


-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Tom Lendacky Aug. 11, 2021, 3:39 p.m. UTC | #14
On 8/11/21 9:53 AM, Kuppuswamy, Sathyanarayanan wrote:
> On 7/27/21 3:26 PM, Tom Lendacky wrote:

>> diff --git a/include/linux/protected_guest.h

>> b/include/linux/protected_guest.h

>> new file mode 100644

>> index 000000000000..f8ed7b72967b

>> --- /dev/null

>> +++ b/include/linux/protected_guest.h

>> @@ -0,0 +1,32 @@

>> +/* SPDX-License-Identifier: GPL-2.0-only */

>> +/*

>> + * Protected Guest (and Host) Capability checks

>> + *

>> + * Copyright (C) 2021 Advanced Micro Devices, Inc.

>> + *

>> + * Author: Tom Lendacky<thomas.lendacky@amd.com>

>> + */

>> +

>> +#ifndef _PROTECTED_GUEST_H

>> +#define _PROTECTED_GUEST_H

>> +

>> +#ifndef __ASSEMBLY__

> 

> Can you include headers for bool type and false definition?


Can do.

Thanks,
Tom

> 

> --- a/include/linux/protected_guest.h

> +++ b/include/linux/protected_guest.h

> @@ -12,6 +12,9 @@

> 

>  #ifndef __ASSEMBLY__

> 

> +#include <linux/types.h>

> +#include <linux/stddef.h>

> 

> Otherwise, I see following errors in multi-config auto testing.

> 

> include/linux/protected_guest.h:40:15: error: unknown type name 'bool'

> include/linux/protected_guest.h:40:63: error: 'false' undeclared (first

> use in this functi

>