| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Jul 2022 10:10:20 -0700
From: Sathyanarayanan Kuppuswamy
<sathyanarayanan.kuppuswamy@...ux.intel.com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc: "Kirill A. Shutemov" <kirill@...temov.name>,
Kai Huang <kai.huang@...el.com>,
Dave Hansen <dave.hansen@...el.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H . Peter Anvin" <hpa@...or.com>, Tony Luck <tony.luck@...el.com>,
Andi Kleen <ak@...ux.intel.com>,
Wander Lairson Costa <wander@...hat.com>,
Isaku Yamahata <isaku.yamahata@...il.com>,
marcelo.cerri@...onical.com, tim.gardner@...onical.com,
khalid.elmously@...onical.com, philip.cox@...onical.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 4/5] x86/mm: Add noalias variants of
set_memory_*crypted() functions
On 7/19/22 9:13 AM, Kirill A. Shutemov wrote:
> On Mon, Jul 18, 2022 at 07:22:52AM -0700, Sathyanarayanan Kuppuswamy wrote:
>> Hi Dave,
>>
>> On 7/5/22 8:29 AM, Kirill A. Shutemov wrote:
>>>>> I still don't like the idea of using the DMA API itself. But, maybe we
>>>>> need some common infrastructure that the DMA API and this code use which
>>>>> says, "get me some pages that I can safely make shared".
>>>> Right. For instance any KVM PV feature would require shared memory, and DMA API
>>>> normally doesn't fit (taking 'struct kvm_steal_time' as example).
>>>>
>>>> Maybe we can reserve a CMA for this purpose.
>>> CMA for couple low traffic users sounds like an overkill. It will create
>>> an separate pool just for them.
>>>
>>> I think the best way is to add an dummy device and couple of helpers
>>> around DMA API that would allow to tap into swiotlb.
>>>
>>> Maybe hide it inside CC infrastructure. Like cc_decrypted_alloc() and
>>> cc_decrypted_free().
>>
>> I also think creating a generic device in the CC layer, and using it to allocate
>> memory via DMA APIs is a better approach for this issue. Following is the sample
>> implementation to give you an idea on how it looks. Please let me know
>> your comments.
>>
>> cc_shmem_alloc/free() APIs can be used by CC users to allocate and
>> free shared memory.
>
> We usually use 'decrypted' term in kernel. cc_decrypted_alloc()/_free().
Fine with me. I will use it.
>
>> Other vendors can define their own shared memory allocation and free
>> logic via struct shmem_priv alloc/free/init hooks.
>>
>> --- a/arch/x86/coco/Makefile
>> +++ b/arch/x86/coco/Makefile
>> @@ -3,6 +3,6 @@ CFLAGS_REMOVE_core.o = -pg
>> KASAN_SANITIZE_core.o := n
>> CFLAGS_core.o += -fno-stack-protector
>>
>> -obj-y += core.o
>> +obj-y += core.o shmem.o
>
> Rename shmem.o -> mem.o ?
Ok.
>
>> diff --git a/arch/x86/coco/core.c b/arch/x86/coco/core.c
>> index 49b44f881484..62fe68d1f60a 100644
>> --- a/arch/x86/coco/core.c
>> +++ b/arch/x86/coco/core.c
>> @@ -13,7 +13,7 @@
>> #include <asm/coco.h>
>> #include <asm/processor.h>
>>
>> -static enum cc_vendor vendor __ro_after_init;
>> +enum cc_vendor vendor __ro_after_init;
>> static u64 cc_mask __ro_after_init;
>>
>> static bool intel_cc_platform_has(enum cc_attr attr)
>> @@ -23,6 +23,7 @@ static bool intel_cc_platform_has(enum cc_attr attr)
>> case CC_ATTR_HOTPLUG_DISABLED:
>> case CC_ATTR_GUEST_MEM_ENCRYPT:
>> case CC_ATTR_MEM_ENCRYPT:
>> + case CC_ATTR_SHMEM:
>> return true;
>> default:
>> return false;
>
> I don't think we need a new attribute. CC_ATTR_MEM_ENCRYPT has to be
> enough.
Ok. Make sense.
>
>> @@ -134,6 +135,11 @@ __init void cc_set_vendor(enum cc_vendor v)
>> vendor = v;
>> }
>>
>> +enum cc_vendor cc_get_vendor(void)
>> +{
>> + return vendor;
>> +}
>> +
>> __init void cc_set_mask(u64 mask)
>> {
>> cc_mask = mask;
>>
>> +++ b/arch/x86/coco/shmem.c
>> @@ -0,0 +1,156 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Confidential Computing Shared Memory Allocator
>> + *
>> + * Copyright (C) 2022 Intel Corporation, Inc.
>> + *
>> + */
>> +
>> +#undef pr_fmt
>> +#define pr_fmt(fmt) "CC: " fmt
>> +
>> +#include <linux/export.h>
>> +#include <linux/cma.h>
>> +#include <linux/mm.h>
>> +#include <linux/cc_platform.h>
>> +#include <linux/set_memory.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#include <asm/coco.h>
>> +#include <asm/processor.h>
>> +
>> +#define CC_SHMEM_DRIVER "cc-shmem"
>> +
>> +struct platform_device *shmem_pdev;
>> +
>> +struct shmem_priv
>> +{
>> + int (*init)(struct platform_device *dev);
>> + void* (*alloc)(size_t size, gfp_t gfp, struct shmem_priv *priv);
>> + void (*free)(void *addr, size_t size, struct shmem_priv *priv);
>> + struct platform_device *pdev;
>> + void *data;
>> +};
>> +
>> +void *cc_shmem_alloc(size_t size, gfp_t gfp)
>> +{
>> + struct shmem_priv *priv;
>> +
>> + if (!shmem_pdev)
>> + return NULL;
>> +
>> + priv = platform_get_drvdata(shmem_pdev);
>> +
>> + return priv->alloc(size, gfp, priv);
>> +}
>> +
>> +void cc_shmem_free(void *addr, size_t size)
>> +{
>> + struct shmem_priv *priv;
>> +
>> + if (!shmem_pdev)
>> + return;
>> +
>> + priv = platform_get_drvdata(shmem_pdev);
>> +
>> + priv->free(addr, size, priv);
>> +}
>> +
>> +static int intel_shmem_init(struct platform_device *pdev)
>> +{
>> + struct shmem_priv *priv;
>> + dma_addr_t *handle;
>> +
>> + handle = devm_kmalloc(&pdev->dev, sizeof(*handle), GFP_KERNEL);
>> + if (!handle)
>> + return -ENOMEM;
>> +
>> + priv = platform_get_drvdata(pdev);
>> +
>> + priv->data = handle;
>> +
>> + return dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
>> +}
>> +
>> +static void *intel_shmem_alloc(size_t size, gfp_t gfp, struct shmem_priv *priv)
>> +{
>> + dma_addr_t *handle = (dma_addr_t *) priv->data;
>> +
>> + return dma_alloc_coherent(&priv->pdev->dev, size, handle, gfp);
>> +}
>> +
>> +static void intel_shmem_free(void *addr, size_t size, struct shmem_priv *priv)
>> +{
>> + dma_addr_t *handle = (dma_addr_t *) priv->data;
>> +
>> + return dma_free_coherent(&priv->pdev->dev, size, addr, *handle);
>> +}
>> +
>> +static struct shmem_priv intel_shmem = {
>> + .init = intel_shmem_init,
>> + .alloc = intel_shmem_alloc,
>> + .free = intel_shmem_free,
>> +};
>
> Hm. What is Intel-specific here. Looks like a generic thing, no?
>
> Maybe just drop all vendor stuff. CC_ATTR_MEM_ENCRYPT should be enough.
I thought that not all CC vendors would want to use DMA APIs for shared
buffer allocation. So adding a vendor layer would give them a way to implement
their own model.
>
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Powered by blists - more mailing lists