Skip to contentSkip to conversation

PCI/TSM: coco/sev-guest: Implement SEV-TIO PCIe TDISP (phase2)

RFC PATCH In review Alexey Kardashevskiy 5 participants 25 messages Sep 16 – Nov 9

Subsystems (14)
Memory management x86 KVM PCI +10 more subsystems
lore
Share & tools
lore

Alexey Kardashevskiy

author maintainer
#0 lore raw

cover PCI/TSM: coco/sev-guest: Implement SEV-TIO PCIe TDISP (phase2)

Here are some patches to continue enabling SEV-TIO on AMD.

SEV-TIO allows guests to establish trust in a device that supports TEE Device Interface Security Protocol (TDISP, defined in PCIe r6.0+) and then interact with the device via private memory.

In order to streamline upstreaming process, a common TSM infrastructure is being developed in collaboration with Intel+ARM+RiscV. There is Documentation/driver-api/pci/tsm.rst with proposed phases:
1. IDE: encrypt PCI, host only
2. TDISP in guest: lock + accept flow, interface report <= WE ARE HERE
3. Enable on host: secure MMIO + DMA, KVM changes
4. Device attestation: certificates, measurements

Acronyms:
TEE - Trusted Execution Environments, a concept of managing trust between the host and devices TSM - TEE Security Manager (TSM), an entity which ensures security on the host
PSP - AMD platform secure processor (also "ASP", "AMD-SP"), acts as TSM on AMD.
SEV TIO - the TIO protocol implemented by the PSP and used by the host, extension to SEV-SNP GHCB - guest/host communication block - a protocol for guest-to-host communication via a shared page TDISP - TEE Device Interface Security Protocol (PCIe).

Flow:
- Boot guest OS, load sev-guest.ko which registers itself as a TSM
- PCI TSM creates sysfs nodes under "tsm" subdirectory in for all TDISP-capable devices
- lock the device via:

	echo tsm0 > "/sys/bus/pci/devices/0000:01:00.0/tsm/lock"
- accept the device via:
	echo 1 > "/sys/bus/pci/devices/0000:01:00.0/tsm/accept"

- load the device driver:
- DMA to encrypted memory should work right away
- Reported TEE MMIO regions will be mapped as encrypted

Patches 01/17..05/17 are fixes and can go in sooner.
Patches 06/17..17/17 are the minimum required by the VM to get encrypted MMIO and DMA.

Doing "io_tlb_default_mem.for_alloc = true" in swiotlb_init_remap() enabled T=1 DMA to shared guest memory.

The previous conversation is here:
https://lore.kernel.org/r/[email protected]

This is based on the last Dan's patch series rebased on top of v7.3-rc2 with the DMA SWIOTLB fixes from Aneesh.

The whole tree is here: https://github.com/AMDESE/linux-kvm/commits/tsm-next/
The host support is here: https://github.com/AMDESE/linux-kvm/commits/tsm
Some raw QEMU sketch is here: https://github.com/AMDESE/qemu/commits/tsm-next

Please comment. Thanks.

The SEV TIO spec:
https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/58271.pdf

Alexey Kardashevskiy (16):
pci/dma/tsm: Call disable DMA bus hook on cleanup pci/tsm: Fix stale comment about TDI report range start tsm/core: Store range_id in pci_tsm_mmio_entry crypto/ccp/tsm: Use TSM API for DOE
tsm-core: Register nevertheless
x86/io/tsm: Allow mixed ioremap for shared+private BARs x86/dma: Revert "x86: Remove unnecessary architecture-specific

    <asm/device.h>"

x86/dma: Add ARCH_HAS_PHYS_TO_DMA
dma/swiotlb: Force shared DMA for allocatios from SWIOTLB tsm/core: Add TDI status
coco/sev-guest: Allow multiple source files in the driver x86/sev: Pass HV features to sev-guest device via platform data x86/sev: Add GHCB calls for SEV-TIO
x86/sev: Implement guest TSM driver for SEV-TIO (phase2, DMA) x86/sev: Enable secure MMIO (phase2)
x86/sev: Flush IOMMU TLB for trusted devices

Dan Williams (1):
x86, dma: Allow accepted devices to map private memory

 arch/x86/Kconfig                                    |   1 +
 drivers/virt/coco/sev-guest/Kconfig                 |   1 +
 drivers/virt/coco/sev-guest/Makefile                |   5 +-
 arch/x86/include/asm/device.h                       |  13 +
 arch/x86/include/asm/dma-direct.h                   |  77 +++
 arch/x86/include/asm/sev-common.h                   |   3 +
 arch/x86/include/asm/sev.h                          |  13 +
 arch/x86/include/uapi/asm/svm.h                     |  43 ++
 drivers/virt/coco/sev-guest/sev-guest.h             |  20 +
 include/linux/dma-direct.h                          |   2 +-
 include/linux/io.h                                  |   8 +
 include/linux/ioport.h                              |   3 +
 include/linux/pci-tsm.h                             |  71 ++
 include/uapi/linux/sev-guest.h                      |  12 +
 arch/x86/coco/sev/core.c                            | 148 +++-
 arch/x86/mm/ioremap.c                               |   2 +-
 arch/x86/mm/mem_encrypt.c                           |   5 +-
 drivers/crypto/ccp/sev-dev-tsm.c                    |   5 +-
 drivers/pci/pci-driver.c                            |   3 +
 drivers/pci/tsm/core.c                              |   3 +-
 drivers/virt/coco/sev-guest/{sev-guest.c => core.c} |  25 +-
 drivers/virt/coco/sev-guest/tio.c                   | 725 ++++++++++++++++++++
 drivers/virt/coco/tsm-core.c                        |  16 +-
 drivers/xen/swiotlb-xen.c                           |   2 +-
 kernel/dma/direct.c                                 |   3 +-
 kernel/dma/swiotlb.c                                |   2 +-
 kernel/resource.c                                   |  63 ++
 mm/ioremap.c                                        |   2 +-
 28 files changed, 1249 insertions(+), 27 deletions(-)
 create mode 100644 arch/x86/include/asm/device.h
 create mode 100644 arch/x86/include/asm/dma-direct.h
 create mode 100644 drivers/virt/coco/sev-guest/sev-guest.h
 rename drivers/virt/coco/sev-guest/{sev-guest.c => core.c} (97%)
 create mode 100644 drivers/virt/coco/sev-guest/tio.c
Signature · 1 line
2.55.0
author maintainer
#1 lore raw

RFC PATCH 1/17 pci/dma/tsm: Call disable DMA bus hook on cleanup

Since XXXX ("PCI, device core: Add private memory access for DEVICE_TRUST_TCB") the TSM subsystem relies on the PCI subsystem to ask the TSM to enable DMA.

That misses teardown path to disable DMA, add it now.

Fixes XXXX ("PCI, device core: Add private memory access for DEVICE_TRUST_TCB")

---

Squash into "PCI, device core: Add private memory access for DEVICE_TRUST_TCB"?

---
 drivers/pci/pci-driver.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/pci/pci-driver.c +3 -0
index a036f3d7e6d5..633926b55da6 100644
@@ -1739,6 +1739,9 @@ static void pci_dma_cleanup(struct device *dev)
 
 	if (!driver->driver_managed_dma)
 		iommu_device_unuse_default_domain(dev);
+
+	if (device_tcb_trusted(dev))
+		pci_tsm_disable_dma(to_pci_dev(dev));
 }
 
 /*
Signature · 1 line
2.55.0
maintainer
#2 lore raw

Re: [RFC PATCH kernel 01/17] pci/dma/tsm: Call disable DMA bus hook on cleanup

drivers/pci/pci-driver.c static void pci_dma_cleanup(struct device *dev) · line 1743

Quoted 1 line · Alexey Kardashevskiy
drivers/pci/pci-driver.c +1 -0
+	if (device_tcb_trusted(dev))

Where is that function and the rest of the gunk needed for me to apply before I review those?

The patches on that branch:

https://github.com/AMDESE/linux-kvm/commits/tsm-next

which come before those here are enough?

Thx.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#3 lore raw

Re: [RFC PATCH kernel 01/17] pci/dma/tsm: Call disable DMA bus hook on cleanup

Quoted 11 lines · Borislav Petkov

drivers/pci/pci-driver.c static void pci_dma_cleanup(struct device *dev) · line 1743

Quoted 1 line · Alexey Kardashevskiy
drivers/pci/pci-driver.c +1 -0
+	if (device_tcb_trusted(dev))

Where is that function and the rest of the gunk needed for me to apply before I review those?

The patches on that branch:

https://github.com/AMDESE/linux-kvm/commits/tsm-next

which come before those here are enough?

Correct, the patches between v7.3-rc2 and 01/17 (can drop samples and tools, obviously) are enough. It is basically recent tagged upstream + rebased Dan's https://lore.kernel.org/r/[email protected]

What device_tcb_trusted() means is still debatable, the earlier proposal was including a module flag saying that the driver is trusted too, I think we are ditching it now.

Thanks,

Signature · 1 line
Alexey
maintainer
#4 lore raw

Re: [RFC PATCH kernel 01/17] pci/dma/tsm: Call disable DMA bus hook on cleanup

Quoted 3 lines · Alexey Kardashevskiy

Correct, the patches between v7.3-rc2 and 01/17 (can drop samples and tools, obviously) are enough. It is basically recent tagged upstream + rebased Dan's https://lore.kernel.org/r/[email protected]

You probably should rebase that branch on 7.4 because it has caught a bunch of conflicts since the last time...

Thx.

Signature · 4 lines
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
author maintainer
#5 lore raw

RFC PATCH 2/17 pci/tsm: Fix stale comment about TDI report range start

In reply to Alexey Kardashevskiy

The PCIe spec r6 and later defines the MMIO range start in 4K units which was not the intention. The upcoming change makes it a byte address. The structure is already fixed to match the new definition but the comment is stale, fix it.

---
 drivers/pci/tsm/core.c | 1 -
 1 file changed, 1 deletion(-)
drivers/pci/tsm/core.c +0 -1
index 9ac216ad896d..2a18be5be56e 100644
@@ -623,7 +623,6 @@ EXPORT_SYMBOL_GPL(pci_tsm_mmio_teardown);
 #define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_UPDATABLE BIT(3)
 #define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID GENMASK(31, 16)
 
-/* An interface report 'pfn' is 4K in size */
 struct pci_tsm_devif_mmio {
 	__le64 phys;
 	__le32 nr_pfns;
Signature · 1 line
2.55.0
#6 lore raw

Re: [RFC PATCH kernel 02/17] pci/tsm: Fix stale comment about TDI report range start

Quoted 4 lines · Alexey Kardashevskiy

The PCIe spec r6 and later defines the MMIO range start in 4K units which was not the intention. The upcoming change makes it a byte address. The structure is already fixed to match the new definition but the comment is stale, fix it.

Please include specific reference to current spec, e.g., "PCIe r7.0, sec x".

I'm not sure who's intention is referred to here.

Please make the subject line match others in drivers/pci in style, including capitalization. Use "git log --oneline drivers/pci" to see.

Quoted 20 lines
---
 drivers/pci/tsm/core.c | 1 -
 1 file changed, 1 deletion(-)
drivers/pci/tsm/core.c +0 -1
index 9ac216ad896d..2a18be5be56e 100644
@@ -623,7 +623,6 @@ EXPORT_SYMBOL_GPL(pci_tsm_mmio_teardown);
 #define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_IS_UPDATABLE BIT(3)
 #define PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID GENMASK(31, 16)
 
-/* An interface report 'pfn' is 4K in size */
 struct pci_tsm_devif_mmio {
 	__le64 phys;
 	__le32 nr_pfns;
Signature · 1 line
2.55.0
author maintainer
#7 lore raw

RFC PATCH 3/17 tsm/core: Store range_id in pci_tsm_mmio_entry

In reply to Alexey Kardashevskiy

The pci_tsm_mmio struct represent a parsed TDI report which platform TSM driver will apply at the TDI accept step.

On top of what the existing structure stores already, the AMD TSM driver is going to need a range ID for the PSP communication.

Instead of calculating it in the platform TSM driver, just pass it along.

While at this, set the flag as resource_contains() checks for the type when pci_tsm_mmio_setup() inserts the range.

---

Not quite sure if IORESOURCE_MEM deserves a separate fix; also if it is just me who hit this.

---
 include/linux/pci-tsm.h | 2 ++
 drivers/pci/tsm/core.c  | 2 ++
 2 files changed, 4 insertions(+)
include/linux/pci-tsm.h +2 -0
index 6d5fadd79360..9b34304d9a1f 100644
@@ -161,10 +161,12 @@ struct pci_tsm_pf0 {
  * @res: MMIO address range (typically Guest Physical Address, GPA)
  * @tsm_offset: Host Physical Address, HPA obfuscation offset added by the TSM.
  *		Translates report addresses to GPA.
+ * @range_id: PCI BAR index for this range
  */
 struct pci_tsm_mmio_entry {
 	struct resource res;
 	u64 tsm_offset;
+	unsigned char range_id;
 };
 
 struct pci_tsm_mmio {
drivers/pci/tsm/core.c +2 -0
index 2a18be5be56e..a197d56edbfb 100644
@@ -746,6 +746,8 @@ struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
 
 		entry->res.start = range.start;
 		entry->res.end = range.end;
+		entry->res.flags = IORESOURCE_MEM;
+		entry->range_id = bar;
 		entry->tsm_offset = tsm_offset;
 		mmio->nr++;
 	}
Signature · 1 line
2.55.0
author maintainer
#8 lore raw

RFC PATCH 4/17 crypto/ccp/tsm: Use TSM API for DOE

In reply to Alexey Kardashevskiy

Compared to the currently used pci_doe(), the pci_tsm_doe_transfer() helper performs a few more sanity checks, use that helper now.

---
 drivers/crypto/ccp/sev-dev-tsm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/crypto/ccp/sev-dev-tsm.c +3 -2
index 46f2539d2d5a..6e704ad239af 100644
@@ -40,8 +40,9 @@ static int sev_tio_spdm_cmd(struct tio_dsm *dsm, int ret)
 
 	/* ret > 0 means "SPDM requested" */
 	while (ret == PCI_DOE_FEATURE_CMA || ret == PCI_DOE_FEATURE_SSESSION) {
-		ret = pci_doe(dsm->tsm.doe_mb, PCI_VENDOR_ID_PCI_SIG, ret,
-			      spdm->req, spdm->req_len, spdm->rsp, spdm->rsp_len);
+		ret = pci_tsm_doe_transfer(dsm->tsm.base_tsm.pdev, ret,
+					   spdm->req, spdm->req_len,
+					   spdm->rsp, spdm->rsp_len);
 		if (ret < 0)
 			break;
 
Signature · 1 line
2.55.0
author maintainer
#9 lore raw

RFC PATCH 5/17 tsm-core: Register nevertheless

In reply to Alexey Kardashevskiy

When this and platform TSM are built-in, there is no way to control the init order other than alphabetical.

Force the class init when a TSM platform driver registers itself.

---

The alternative is making it a subsys_init() and drop tsm_exit().

---
 drivers/virt/coco/tsm-core.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/virt/coco/tsm-core.c +15 -1
index 0843b77c6549..accee6a5cba8 100644
@@ -51,6 +51,7 @@ static const struct attribute_group *tsm_pci_groups[] = {
 };
 
 static void tsm_release(struct device *);
+static bool tsm_class_registered;
 static const struct class tsm_class = {
 	.name		= "tsm",
 	.dev_release	= tsm_release,
@@ -58,6 +59,14 @@ static const struct class tsm_class = {
 };
 static DEFINE_IDA(tsm_ida);
 
+static int tsm_class_init(void)
+{
+	int ret = class_register(&tsm_class);
+
+	tsm_class_registered = ret == 0;
+	return ret;
+}
+
 static int match_id(struct device *dev, const void *data)
 {
 	struct tsm_dev *tsm_dev = container_of(dev, struct tsm_dev, dev);
@@ -82,6 +91,9 @@ static struct tsm_dev *alloc_tsm_dev(struct device *parent)
 
 	struct tsm_dev *tsm_dev __free(kfree) =
 		kzalloc_obj(*tsm_dev);
+
+	tsm_class_init();
+
 	if (!tsm_dev)
 		return ERR_PTR(-ENOMEM);
 
@@ -254,12 +266,14 @@ static void tsm_release(struct device *dev)
 
 static int __init tsm_init(void)
 {
-	return class_register(&tsm_class);
+	return tsm_class_init();
 }
 module_init(tsm_init)
 
 static void __exit tsm_exit(void)
 {
+	if (!tsm_class_registered)
+		return;
 	class_unregister(&tsm_class);
 	xa_destroy(&tsm_ide_streams);
 }
Signature · 1 line
2.55.0
author maintainer
#10 lore raw

RFC PATCH 6/17 x86/io/tsm: Allow mixed ioremap for shared+private BARs

In reply to Alexey Kardashevskiy

A TDISP device advertises private MMIO ranges via a TDI report. Upon transitioning to RUN, accesses to those must be made with the encrypted bit set in a PTE.

A device can allow private access to a part of a BAR, for example, MSIX BAR (when MSIX config is not locked or MSIX is emulated by the HV).

The existing __ioremap_caller() fails if not every single page has the same protection in PTE and this breaks device drivers (usually TDISP-unaware) calling pci_iomap() and such.

Adjust __ioremap_caller() to map such MMIO BAR in chunks.

---
 include/linux/io.h     |  8 +++
 include/linux/ioport.h |  3 +
 arch/x86/mm/ioremap.c  |  2 +-
 kernel/resource.c      | 63 ++++++++++++++++++++
 mm/ioremap.c           |  2 +-
 5 files changed, 76 insertions(+), 2 deletions(-)
include/linux/io.h +8 -0
index 0642c7ee41db..9f5d5928f30c 100644
@@ -27,6 +27,8 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count);
 #ifdef CONFIG_MMU
 int ioremap_page_range(unsigned long addr, unsigned long end,
 		       phys_addr_t phys_addr, pgprot_t prot);
+int ioremap_map_page_range(unsigned long vaddr, phys_addr_t phys_addr,
+			   unsigned long size, pgprot_t prot);
 int vmap_page_range(unsigned long addr, unsigned long end,
 		    phys_addr_t phys_addr, pgprot_t prot);
 #else
@@ -35,6 +37,12 @@ static inline int ioremap_page_range(unsigned long addr, unsigned long end,
 {
 	return 0;
 }
+static inline int ioremap_map_page_range(unsigned long vaddr,
+					 phys_addr_t phys_addr,
+					 unsigned long size, pgprot_t prot)
+{
+	return 0;
+}
 static inline int vmap_page_range(unsigned long addr, unsigned long end,
 				  phys_addr_t phys_addr, pgprot_t prot)
 {
include/linux/ioport.h +3 -0
index 122f1eefb4b9..49d35b31f22d 100644
@@ -444,8 +444,11 @@ walk_iomem_res_desc(unsigned long desc, unsigned long flags, u64 start, u64 end,
 		    void *arg, int (*func)(struct resource *, void *));
 extern int walk_soft_reserve_res(u64 start, u64 end, void *arg,
 				 int (*func)(struct resource *, void *));
+extern int walk_encrypted_mem_res(u64 start, u64 end, void *arg,
+				  int (*func)(struct resource *, void *));
 extern int
 region_intersects_soft_reserve(resource_size_t start, size_t size);
+extern int region_intersects_encrypted(resource_size_t start, size_t size);
 
 struct resource *devm_request_free_mem_region(struct device *dev,
 		struct resource *base, unsigned long size);
arch/x86/mm/ioremap.c +1 -1
index 12c8180ca1ba..976ce6d5cf70 100644
@@ -298,7 +298,7 @@ __ioremap_caller(resource_size_t phys_addr, unsigned long size,
 	if (memtype_kernel_map_sync(phys_addr, size, pcm))
 		goto err_free_area;
 
-	if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot))
+	if (ioremap_map_page_range(vaddr, phys_addr, size, prot))
 		goto err_free_area;
 
 	ret_addr = (void __iomem *) (vaddr + offset);
kernel/resource.c +63 -0

75 lines not on this page · raw message

mm/ioremap.c +1 -1
index c36dd9f62fd5..ccf938628e55 100644
@@ -40,7 +40,7 @@ void __iomem *generic_ioremap_prot(phys_addr_t phys_addr, size_t size,
 	vaddr = (unsigned long)area->addr;
 	area->phys_addr = phys_addr;
 
-	if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot)) {
+	if (ioremap_map_page_range(vaddr, phys_addr, size, prot)) {
 		free_vm_area(area);
 		return NULL;
 	}
Signature · 1 line
2.55.0
author maintainer
#11 lore raw

RFC PATCH 7/17 x86/dma: Revert "x86: Remove unnecessary architecture-specific <asm/device.h>"

In reply to Alexey Kardashevskiy

The trusted guest device DMA setup is going to distinguish encrypted and decrypted memory and dev_archdata is a logical place for it.

This reverts commit c256d2a8adf2f5670ca262979c451ec4c1108e38.

---
 arch/x86/include/asm/device.h | 11 +++++++++++
 1 file changed, 11 insertions(+)
arch/x86/include/asm/device.h new file +11 -0
new file mode 100644
index 000000000000..7c0a52ca2f4d
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_X86_DEVICE_H
+#define _ASM_X86_DEVICE_H
+
+struct dev_archdata {
+};
+
+struct pdev_archdata {
+};
+
+#endif /* _ASM_X86_DEVICE_H */
Signature · 1 line
2.55.0
author maintainer
#12 lore raw

RFC PATCH 8/17 x86/dma: Add ARCH_HAS_PHYS_TO_DMA

In reply to Alexey Kardashevskiy

Confidential VMs work with both trusted and legacy (untrusted devices). The trusted devices allow DMA to/from encrypted guest memory while legacy devices have to use SWIOTLB or share pages for DMA.

In case of P2P between trusted and legacy devices, the trusted device has to perform DMA with T=1 (a PCIe IDE TLP bit saying "encrypt") but the target may be shared.

One way of allowing the above on AMD SEV is using IOMMU sDTE vTOM feature which is a bus address:
- below that address all accesses target private guest memory;
- above - shared memory.

Effectively this adds some high bit in a DMA handle to mark "shared" (on AMD).

Copy the generic phys_to_dma / dma_to_phys helpers from include/linux/dma-direct.h into arch/x86/include/asm/dma-direct.h and select ARCH_HAS_PHYS_TO_DMA for x86.

The x86 implementation is the same as the generic code for __phys_to_dma, dma_range_map handling, and SME encryption, with two additions for confidential devices:
- cc_shared_dma_offset and cc_private_dma_offset in dev_archdata
- when TDI is accepted is true, phys_to_dma() adds the shared or private offset (selected by DMA_ATTR_CC_SHARED), and dma_to_phys() subtracts it on the way back

phys_to_dma() also gains an attrs argument so dma_capable() and swiotlb can pass mapping attributes through. Update the call sites accordingly.

---

DMA_ATTR_CC_SHARED or __DMA_ATTR_ALLOC_CC_SHARED?

---
 arch/x86/Kconfig                  |  1 +
 arch/x86/include/asm/device.h     |  2 +
 arch/x86/include/asm/dma-direct.h | 77 ++++++++++++++++++++
 include/linux/dma-direct.h        |  2 +-
 drivers/xen/swiotlb-xen.c         |  2 +-
 kernel/dma/swiotlb.c              |  2 +-
 6 files changed, 83 insertions(+), 3 deletions(-)
arch/x86/Kconfig +1 -0
index 15fd9ec5ecac..c4df431e74b2 100644
@@ -112,6 +112,7 @@ config X86
 	select ARCH_HAS_UBSAN
 	select ARCH_HAS_DEBUG_WX
 	select ARCH_HAS_ZONE_DMA_SET if EXPERT
+	select ARCH_HAS_PHYS_TO_DMA
 	select ARCH_HAVE_NMI_SAFE_CMPXCHG
 	select ARCH_HAVE_EXTRA_ELF_NOTES
 	select ARCH_MEMORY_ORDER_TSO
arch/x86/include/asm/device.h +2 -0
index 7c0a52ca2f4d..0dbc2c125522 100644
@@ -3,6 +3,8 @@
 #define _ASM_X86_DEVICE_H
 
 struct dev_archdata {
+	dma_addr_t cc_shared_dma_offset;
+	dma_addr_t cc_private_dma_offset;
 };
 
 struct pdev_archdata {
arch/x86/include/asm/dma-direct.h new file +77 -0

77 lines not on this page · raw message

include/linux/dma-direct.h +1 -1
index daa31a1adf7b..616b66ccf322 100644
@@ -150,7 +150,7 @@ static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size,
 		return false;
 
 	if (is_ram && !IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) &&
-	    min(addr, end) < phys_to_dma(dev, PFN_PHYS(min_low_pfn)))
+	    min(addr, end) < phys_to_dma(dev, PFN_PHYS(min_low_pfn), attrs))
 		return false;
 
 	return end <= min_not_zero(*dev->dma_mask, dev->bus_dma_limit);
drivers/xen/swiotlb-xen.c +1 -1
index e2538824ef52..915c174f8b56 100644
@@ -55,7 +55,7 @@ static inline phys_addr_t xen_phys_to_bus(struct device *dev, phys_addr_t paddr)
 
 static inline dma_addr_t xen_phys_to_dma(struct device *dev, phys_addr_t paddr)
 {
-	return phys_to_dma(dev, xen_phys_to_bus(dev, paddr));
+	return phys_to_dma(dev, xen_phys_to_bus(dev, paddr), 0);
 }
 
 static inline phys_addr_t xen_bus_to_phys(struct device *dev,
kernel/dma/swiotlb.c +1 -1
index ded7016a46a7..d7c7d15740ae 100644
@@ -1764,7 +1764,7 @@ dma_addr_t swiotlb_map(struct device *dev, phys_addr_t paddr, size_t size,
 	phys_addr_t swiotlb_addr;
 	dma_addr_t dma_addr;
 
-	trace_swiotlb_bounced(dev, phys_to_dma(dev, paddr), size);
+	trace_swiotlb_bounced(dev, phys_to_dma(dev, paddr, attrs), size);
 
 	swiotlb_addr = swiotlb_tbl_map_single(dev, paddr, size, 0, dir, &attrs);
 	if (swiotlb_addr == (phys_addr_t)DMA_MAPPING_ERROR)
Signature · 1 line
2.55.0
author maintainer
#13 lore raw

RFC PATCH 9/17 dma/swiotlb: Force shared DMA for allocatios from SWIOTLB

In reply to Alexey Kardashevskiy

SWIOTLB allocations are always decrypted but the DMA layer is unaware of those and will miss to adjust the DMA handle.

Force __DMA_ATTR_ALLOC_CC_SHARED if SWIOTLB is used for memory allocations.

This will be exploited by AMD SEV guest vTOM feature where DMA handles of shared memory have some high bit set (not to be confused with the Cbit).

---
 kernel/dma/direct.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
kernel/dma/direct.c +2 -1
index da665ca22d5c..dacf513d45fd 100644
@@ -273,13 +273,14 @@ void *dma_direct_alloc(struct device *dev, size_t size,
 	}
 
 	if (is_swiotlb_for_alloc(dev)) {
-		page = dma_direct_alloc_swiotlb(dev, size, attrs);
+		page = dma_direct_alloc_swiotlb(dev, size, attrs | __DMA_ATTR_ALLOC_CC_SHARED);
 		if (page) {
 			/*
 			 * swiotlb allocations comes from pool already marked
 			 * decrypted
 			 */
 			mark_mem_decrypt = false;
+			attrs |= __DMA_ATTR_ALLOC_CC_SHARED;
 			goto setup_page;
 		}
 		return NULL;
Signature · 1 line
2.55.0
author maintainer
#14 lore raw

RFC PATCH 10/17 x86, dma: Allow accepted devices to map private memory

In reply to Alexey Kardashevskiy

From: Dan Williams <[email protected]>

With the arrival of "accepted" devices, devices that have been enabled to DMA to private encrypted memory, coherent DMA allocation no longer requires page conversion. Update force_dma_unencrypted() to skip accepted devices.

Cc Dave Hansen <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
[aik: re-added device_cc_accepted()]
---
 arch/x86/mm/mem_encrypt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/mm/mem_encrypt.c +3 -2
index 95bae74fdab2..805800ddbd79 100644
@@ -20,10 +20,11 @@
 bool force_dma_unencrypted(struct device *dev)
 {
 	/*
-	 * For SEV, all DMA must be to unencrypted addresses.
+	 * Require unencrypted DMA unless the device has been "accepted",
+	 * enabled by a TSM driver to DMA to private encrypted memory.
 	 */
 	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
-		return true;
+		return !__device_cc_accepted(dev);
 
 	/*
 	 * For SME, all DMA must be to unencrypted addresses if the
Signature · 1 line
2.55.0
#15 lore raw

Re: [RFC PATCH kernel 10/17] x86, dma: Allow accepted devices to map private memory

arch/x86/mm/mem_encrypt.c line 27

Quoted 10 lines · Alexey Kardashevskiy

bool force_dma_unencrypted(struct device *dev)
{

arch/x86/mm/mem_encrypt.c +3 -2
 	/*
-	 * For SEV, all DMA must be to unencrypted addresses.
+	 * Require unencrypted DMA unless the device has been "accepted",
+	 * enabled by a TSM driver to DMA to private encrypted memory.
 	 */
 	if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
-		return true;
+		return !__device_cc_accepted(dev);

I want this out of arch code, after Aneesh's recent rework Nicolin is trying this approach:

https://lore.kernel.org/all/abddd433493324a6bfdf8ba9710e96d90f0d1ccc.1789010941.git.nicolinc@nvidia.com/

Jason

author maintainer
#16 lore raw

RFC PATCH 11/17 tsm/core: Add TDI status

In reply to Alexey Kardashevskiy

Define a structure with all info about a TDI such as TDISP status, bind state, used START_INTERFACE options and the report digest.

---
 include/linux/pci-tsm.h | 69 ++++++++++++++++++++
 1 file changed, 69 insertions(+)
include/linux/pci-tsm.h +69 -0
index 9b34304d9a1f..2ca577e2a37e 100644
@@ -11,6 +11,7 @@
 struct pci_tsm;
 struct tsm_dev;
 struct kvm;
+struct tsm_tdi_status;
 enum pci_tsm_req_scope;
 
 /*
@@ -88,6 +89,7 @@ struct pci_tsm_ops {
 
 	int (*refresh_evidence)(struct pci_tsm *tsm, const void *nonce,
 				size_t nonce_len);
+	int (*tdi_status)(struct pci_dev *pdev, struct tsm_tdi_status *ts);
 };
 
 /**
@@ -331,4 +333,71 @@ static inline ssize_t pci_tsm_guest_req(struct pci_dev *pdev,
 	return -ENXIO;
 }
 #endif
+
+/* private: */
+
+enum tsm_tdisp_state {
+	TDISP_STATE_CONFIG_UNLOCKED = 0,
+	TDISP_STATE_CONFIG_LOCKED = 1,
+	TDISP_STATE_RUN = 2,
+	TDISP_STATE_ERROR = 3,
+};
+
+enum tsm_tdisp_status {
+	TDISP_STATE_BOUND = 0,
+	TDISP_STATE_INVALID = 1,
+	TDISP_STATE_UNBOUND = 2,
+};
+
+/*
+ * struct tdisp_interface_id - TDISP INTERFACE_ID Definition
+ *
+ * @function_id: Identifies the function of the device hosting the TDI
+ *   15:0: @rid: Requester ID
+ *   23:16: @rseg: Requester Segment (Reserved if Requester Segment Valid is Clear)
+ *   24: @rseg_valid: Requester Segment Valid
+ *   31:25 – Reserved
+ * 8B - Reserved
+ */
+#define TSM_TDISP_IID_REQUESTER_ID      GENMASK(15, 0)
+#define TSM_TDISP_IID_RSEG              GENMASK(23, 16)
+#define TSM_TDISP_IID_RSEG_VALID        BIT(24)
+
+struct tdisp_interface_id {
+	__u32 function_id; /* TSM_TDISP_IID_xxxx */
+	__u8 reserved[8];
+} __packed;
+
+struct tsm_tdi_status {
+	__u8 status; /* enum tsm_tdisp_status */
+	__u8 state; /* enum tsm_tdisp_state */
+	__u8 meas_digest_fresh;
+	__u8 meas_digest_valid;
+	__u8 all_request_redirect;
+	__u8 bind_p2p;
+	__u8 lock_msix;
+	__u8 no_fw_update;
+	__u16 cache_line_size;
+	__u64 spdm_algos; /* Bitmask of TSM_SPDM_ALGOS */
+	__u8 certs_digest[48];
+	__u8 meas_digest[48];
+	__u8 interface_report_digest[48];
+	__u64 intf_report_counter;
+	struct tdisp_interface_id id;
+	__u64 tdi_id;
+} __packed;
+
+enum tsm_spdm_algos {
+	TSM_SPDM_ALGOS_DHE_SECP256R1,
+	TSM_SPDM_ALGOS_DHE_SECP384R1,
+	TSM_SPDM_ALGOS_AEAD_AES_128_GCM,
+	TSM_SPDM_ALGOS_AEAD_AES_256_GCM,
+	TSM_SPDM_ALGOS_ASYM_TPM_ALG_RSASSA_3072,
+	TSM_SPDM_ALGOS_ASYM_TPM_ALG_ECDSA_ECC_NIST_P256,
+	TSM_SPDM_ALGOS_ASYM_TPM_ALG_ECDSA_ECC_NIST_P384,
+	TSM_SPDM_ALGOS_HASH_TPM_ALG_SHA_256,
+	TSM_SPDM_ALGOS_HASH_TPM_ALG_SHA_384,
+	TSM_SPDM_ALGOS_KEY_SCHED_SPDM_KEY_SCHEDULE,
+};
+
 #endif /*__PCI_TSM_H */
Signature · 1 line
2.55.0
author maintainer
#17 lore raw

RFC PATCH 12/17 coco/sev-guest: Allow multiple source files in the driver

In reply to Alexey Kardashevskiy

Prepare for SEV-TIO support as it is going to equal or bigger than the existing sev_guest.c which is already 700 lines and keeps growing.

No behavioural change expected.

---
 drivers/virt/coco/sev-guest/Makefile                |  2 +-
 drivers/virt/coco/sev-guest/sev-guest.h             | 16 ++++++++++++++++
 drivers/virt/coco/sev-guest/{sev-guest.c => core.c} | 10 ++--------
 3 files changed, 19 insertions(+), 9 deletions(-)
drivers/virt/coco/sev-guest/Makefile +1 -1
index 63d67c27723a..a8eb28b6736e 100644
@@ -1,2 +1,2 @@
 # SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_SEV_GUEST) += sev-guest.o
+obj-$(CONFIG_SEV_GUEST) += core.o
drivers/virt/coco/sev-guest/sev-guest.h new file +16 -0
new file mode 100644
index 000000000000..b2a97778e635
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SEV_GUEST_H__
+#define __SEV_GUEST_H__
+
+#include <linux/miscdevice.h>
+#include <asm/sev.h>
+
+struct snp_guest_dev {
+	struct device *dev;
+	struct miscdevice misc;
+
+	struct snp_msg_desc *msg_desc;
+};
+
+#endif /* __SEV_GUEST_H__ */
drivers/virt/coco/sev-guest/core.c renamed from drivers/virt/coco/sev-guest/sev-guest.c +2 -8
similarity index 99%
rename from drivers/virt/coco/sev-guest/sev-guest.c
rename to drivers/virt/coco/sev-guest/core.c
index 935537a41469..98f698809720 100644
@@ -27,19 +27,13 @@
 #include <uapi/linux/psp-sev.h>
 
 #include <asm/svm.h>
-#include <asm/sev.h>
+
+#include "sev-guest.h"
 
 #define DEVICE_NAME	"sev-guest"
 
 #define SVSM_MAX_RETRIES		3
 
-struct snp_guest_dev {
-	struct device *dev;
-	struct miscdevice misc;
-
-	struct snp_msg_desc *msg_desc;
-};
-
 /*
  * The VMPCK ID represents the key used by the SNP guest to communicate with the
  * SEV firmware in the AMD Secure Processor (ASP, aka PSP). By default, the key
Signature · 1 line
2.55.0
author maintainer
#18 lore raw

RFC PATCH 13/17 x86/sev: Pass HV features to sev-guest device via platform data

In reply to Alexey Kardashevskiy

The SEV HV advertises supported options via GHCB, the guest stores bitmask in the SEV core.

The sev-guest device is a user visible platform device (/dev/sev-guest) which provides the interface to communicate with the HV via GHCB. It may choose to limit the functionality depending on what is available. The first user is going to be SEV-TIO which is going to trigger creation of "tsm" folders in the PCI sysfs but only if GHCB advertised such support.

This should cause no behavioural change.

---
 arch/x86/coco/sev/core.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
arch/x86/coco/sev/core.c +8 -6
index cc292d7c6fd1..5b912f42f493 100644
@@ -1381,11 +1381,6 @@ static int snp_issue_guest_request(struct snp_guest_req *req)
 	return ret;
 }
 
-static struct platform_device sev_guest_device = {
-	.name		= "sev-guest",
-	.id		= -1,
-};
-
 static struct platform_device tpm_svsm_device = {
 	.name		= "tpm-svsm",
 	.id		= -1,
@@ -1393,10 +1388,17 @@ static struct platform_device tpm_svsm_device = {
 
 static int __init snp_init_platform_device(void)
 {
+	struct platform_device *dev;
+
 	if (!cc_platform_has(CC_ATTR_GUEST_SEV_SNP))
 		return -ENODEV;
 
-	if (platform_device_register(&sev_guest_device))
+	dev = platform_device_register_data(NULL, "sev-guest", -1,
+					    &sev_hv_features,
+					    sizeof(sev_hv_features));
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+	if (!dev)
 		return -ENODEV;
 
 	if (snp_svsm_vtpm_probe() &&
Signature · 1 line
2.55.0
author maintainer
#19 lore raw

RFC PATCH 14/17 x86/sev: Add GHCB calls for SEV-TIO

In reply to Alexey Kardashevskiy

SEV-TIO is a PSP protocol allowing PCI pass through of trusted devices (TDI == TEE Device Interface) to an SNP VM. The VMM advertises the support via HV_FEATURES and implements new calls:

1) TDI operation: the host executes on the guest behalf:
- TDI bind/unbind to/from the SNP VM in the PSP;
- TDI start/stop which translates to TDISP interface start/stop.

2) TIO guest request for an encrypted communication channel between the SNP VM and the PSP, it follows the existing Extended Guest Request pattern to read device evidence:
- get TDI status;
- validate MMIO ranges (a variant of the PVALIDATE instruction for MMIO RMPs);
- configure sDTE (a secure IOMMU descriptor).

Extend snp_req_data to pass more parameters.

---
 arch/x86/include/asm/sev-common.h |  1 +
 arch/x86/include/asm/sev.h        |  5 +++
 arch/x86/include/uapi/asm/svm.h   | 40 +++++++++++++++++++
 arch/x86/coco/sev/core.c          | 42 ++++++++++++++++++++
 4 files changed, 88 insertions(+)
arch/x86/include/asm/sev-common.h +1 -0
index 01a6e4dbe423..ff763c3c5d63 100644
@@ -137,6 +137,7 @@ enum psc_op {
 #define GHCB_HV_FT_SNP			BIT_ULL(0)
 #define GHCB_HV_FT_SNP_AP_CREATION	BIT_ULL(1)
 #define GHCB_HV_FT_SNP_MULTI_VMPL	BIT_ULL(5)
+#define GHCB_HV_FT_SNP_SEV_TIO		BIT_ULL(7)
 
 /*
  * SNP Page State Change NAE event
arch/x86/include/asm/sev.h +5 -0
index 9e7a077c445d..89aaccb053ba 100644
@@ -149,6 +149,9 @@ struct snp_req_data {
 	unsigned long resp_gpa;
 	unsigned long data_gpa;
 	unsigned int data_npages;
+	unsigned int guest_rid;
+	unsigned long npages;
+	unsigned long param;
 };
 
 #define MAX_AUTHTAG_LEN		32
@@ -597,6 +600,8 @@ static inline void sev_evict_cache(void *va, int npages)
 	}
 }
 
+int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id);
+
 #else	/* !CONFIG_AMD_MEM_ENCRYPT */
 
 #define snp_vmpl 0
arch/x86/include/uapi/asm/svm.h +40 -0
index 010a45c9f614..93597ad492bf 100644
@@ -122,6 +122,44 @@
 #define SVM_VMGEXIT_SAVIC_REGISTER_GPA		0
 #define SVM_VMGEXIT_SAVIC_UNREGISTER_GPA	1
 #define SVM_VMGEXIT_SAVIC_SELF_GPA		~0ULL
+#define SVM_VMGEXIT_SEV_TIO_GR			0x80000020ull
+#define SVM_VMGEXIT_SEV_TIO_GR_INFO_STATE	BIT(0)
+#define SVM_VMGEXIT_SEV_TIO_GR_INFO_CERTS	BIT(1)
+#define SVM_VMGEXIT_SEV_TIO_GR_INFO_MEAS	BIT(2)
+#define SVM_VMGEXIT_SEV_TIO_GR_INFO_REPORT	BIT(3)
+
+#define SVM_VMGEXIT_SEV_TIO_GR_SDTE_VALIDATE	BIT(0)
+#define SVM_VMGEXIT_SEV_TIO_GR_SDTE_VTOM	GENMASK_ULL(51, 21)
+
+/*
+ * TIO_GUEST_REQUEST's MMIO_VALIDATE_REQ/MMIO_CONFIG_REQ encoding for MMIO in RDX:
+ *
+ * T....... ....GGGG GGGGGGGG GGGGGGGG GGGGGGGG GGGGGGGG GGGG.... ........
+ * Where:
+ *	G - guest physical address
+ *	T - TEE
+ */
+#define SVM_VMGEXIT_SEV_TIO_GR_MMIO_GFN(r)      (((r) & 0x000FFFFFFFFFF000ULL) >> PAGE_SHIFT)
+#define SVM_VMGEXIT_SEV_TIO_GR_MMIO_RESERVED(r) ((r) & 0x7FF0000000000FFFULL)
+#define SVM_VMGEXIT_SEV_TIO_GR_MMIO_PRIVATE(r)  (!!((r) & BIT(63)))
+
+#define SVM_VMGEXIT_SEV_TIO_GR_MMIO_NUM(r)      ((uint32_t)((r) >> 32))
+#define SVM_VMGEXIT_SEV_TIO_GR_MMIO_ADDR(r)     ((uint32_t)((r) & 0xFFFFFFFF))
+
+#define SVM_VMGEXIT_SEV_TIO_GR_MMIO_MK_VALIDATE(start, private) \
+	((SVM_VMGEXIT_SEV_TIO_GR_MMIO_GFN(start) << PAGE_SHIFT) | \
+	((private) ? BIT(63) : 0))
+
+#define SVM_VMGEXIT_SEV_TIO_OP			0x80000021ull
+#define SVM_VMGEXIT_SEV_TIO_GR_MMIO_MK_NUM_BDFN(n, bdfn) (((uint64_t)(n) << 32) | (bdfn))
+
+#define SVM_VMGEXIT_SEV_TIO_OP_PARAM(guest_id, action)	((u64)(action)<<32|(guest_id))
+#define SVM_VMGEXIT_SEV_TIO_OP_ACTION(exitinfo1)	((exitinfo1)>>32)
+#define SVM_VMGEXIT_SEV_TIO_OP_GUEST_ID(exitinfo1)	((exitinfo1) & 0xFFFFFFFF)
+#define SVM_VMGEXIT_SEV_TIO_OP_BIND	0
+#define SVM_VMGEXIT_SEV_TIO_OP_UNBIND	1
+#define SVM_VMGEXIT_SEV_TIO_OP_RUN	2
+#define SVM_VMGEXIT_SEV_TIO_OP_STOP	3
 #define SVM_VMGEXIT_HV_FEATURES			0x8000fffdull
 #define SVM_VMGEXIT_TERM_REQUEST		0x8000fffeull
 #define SVM_VMGEXIT_TERM_REASON(reason_set, reason_code)	\
@@ -245,6 +283,8 @@
 	{ SVM_VMGEXIT_GUEST_REQUEST,	"vmgexit_guest_request" }, \
 	{ SVM_VMGEXIT_EXT_GUEST_REQUEST, "vmgexit_ext_guest_request" }, \
 	{ SVM_VMGEXIT_AP_CREATION,	"vmgexit_ap_creation" }, \
+	{ SVM_VMGEXIT_SEV_TIO_GR,	"vmgexit_sev_tio_guest_request" }, \
+	{ SVM_VMGEXIT_SEV_TIO_OP,	"vmgexit_sev_tio_op" }, \
 	{ SVM_VMGEXIT_HV_FEATURES,	"vmgexit_hypervisor_feature" }, \
 	{ SVM_EXIT_ERR,         "invalid_guest_state" }
 
arch/x86/coco/sev/core.c +42 -0

66 lines not on this page · raw message

Signature · 1 line
2.55.0
author maintainer
#20 lore raw

RFC PATCH 15/17 x86/sev: Implement guest TSM driver for SEV-TIO (phase2, DMA)

In reply to Alexey Kardashevskiy

Add the guest-side PCI TSM backend for AMD SEV-TIO, wiring the generic pci_tsm framework to GHCB TIO operations and PSP guest requests.

Register pci_tsm_ops from sev-guest on probe when TIO GHCB support is present (gated by the tsm_enable module parameter).
lock() binds a TDI, constructs a devsec TSM object and transitions to CONFIG_LOCKED; accept() transitions the TDI to RUN and programs the SDTE with vTOM and guest access permissions; unlock() invalidates the SDTE, unbinds the TDI, quiesces DMA and transitions to CONFIG_UNLOCKED.

Introduce tio.c with TIO guest-request handling (TIO_MSG_SDTE_WRITE_REQ and related message layouts) and set cc_shared_dma_offset from the configurable tsm_vtom so shared DMA addresses are translated for the hypervisor. Extend asm/sev.h with TIO message types and TDISP SPDM algorithm constants.

Build tio.o when PCI_TSM is enabled and select PCI_TSM from SEV_GUEST Kconfig.

---
 drivers/virt/coco/sev-guest/Kconfig     |   1 +
 drivers/virt/coco/sev-guest/Makefile    |   3 +
 arch/x86/include/asm/sev.h              |   8 +
 drivers/virt/coco/sev-guest/sev-guest.h |   4 +
 drivers/virt/coco/sev-guest/core.c      |  14 +
 drivers/virt/coco/sev-guest/tio.c       | 347 ++++++++++++++++++++
 6 files changed, 377 insertions(+)
drivers/virt/coco/sev-guest/Kconfig +1 -0
index a6405ab6c2c3..4255072dfa1a 100644
@@ -3,6 +3,7 @@ config SEV_GUEST
 	default m
 	depends on AMD_MEM_ENCRYPT
 	select TSM_REPORTS
+	select PCI_TSM if PCI
 	help
 	  SEV-SNP firmware provides the guest a mechanism to communicate with
 	  the PSP without risk from a malicious hypervisor who wishes to read,
drivers/virt/coco/sev-guest/Makefile +3 -0
index a8eb28b6736e..84a8b0dd62cd 100644
@@ -1,2 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_SEV_GUEST) += core.o
+ifeq ($(CONFIG_PCI_TSM),y)
+obj-$(CONFIG_SEV_GUEST) += tio.o
+endif
arch/x86/include/asm/sev.h +8 -0
index 89aaccb053ba..375cb5346ec3 100644
@@ -182,6 +182,14 @@ enum msg_type {
 
 	SNP_MSG_TSC_INFO_REQ = 17,
 	SNP_MSG_TSC_INFO_RSP,
+	TIO_MSG_TDI_INFO_REQ = 19,
+	TIO_MSG_TDI_INFO_RSP = 20,
+	TIO_MSG_MMIO_VALIDATE_REQ = 21,
+	TIO_MSG_MMIO_VALIDATE_RSP = 22,
+	TIO_MSG_MMIO_CONFIG_REQ = 23,
+	TIO_MSG_MMIO_CONFIG_RSP = 24,
+	TIO_MSG_SDTE_WRITE_REQ = 25,
+	TIO_MSG_SDTE_WRITE_RSP = 26,
 
 	SNP_MSG_TYPE_MAX
 };
drivers/virt/coco/sev-guest/sev-guest.h +4 -0
index b2a97778e635..c823a782739f 100644
@@ -11,6 +11,10 @@ struct snp_guest_dev {
 	struct miscdevice misc;
 
 	struct snp_msg_desc *msg_desc;
+
+	struct tsm_dev *tsmdev;
 };
 
+void sev_guest_tsm_set_ops(bool set, struct snp_guest_dev *snp_dev);
+
 #endif /* __SEV_GUEST_H__ */
drivers/virt/coco/sev-guest/core.c +14 -0
index 98f698809720..8448c123ab1e 100644
@@ -45,6 +45,10 @@ static int vmpck_id = -1;
 module_param(vmpck_id, int, 0444);
 MODULE_PARM_DESC(vmpck_id, "The VMPCK ID to use when communicating with the PSP.");
 
+static bool tsm_enable = true;
+module_param(tsm_enable, bool, 0644);
+MODULE_PARM_DESC(tsm_enable, "Enable SEV TIO");
+
 static inline struct snp_guest_dev *to_snp_dev(struct file *file)
 {
 	struct miscdevice *dev = file->private_data;
@@ -668,6 +672,14 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 	snp_dev->msg_desc = mdesc;
 	dev_info(dev, "Initialized SEV guest driver (using VMPCK%d communication key)\n",
 		 mdesc->vmpck_id);
+
+	u64 *sev_hv_features_ptr = dev_get_platdata(&pdev->dev);
+	if (!sev_hv_features_ptr || !(*sev_hv_features_ptr & GHCB_HV_FT_SNP_SEV_TIO))
+		tsm_enable = false;
+
+	if (tsm_enable)
+		sev_guest_tsm_set_ops(true, snp_dev);
+
 	return 0;
 
 e_msg_init:
@@ -681,6 +693,8 @@ static void __exit sev_guest_remove(struct platform_device *pdev)
 	struct snp_guest_dev *snp_dev = platform_get_drvdata(pdev);
 
 	snp_msg_free(snp_dev->msg_desc);
+	if (tsm_enable)
+		sev_guest_tsm_set_ops(false, snp_dev);
 	misc_deregister(&snp_dev->misc);
 }
 
drivers/virt/coco/sev-guest/tio.c new file +347 -0

347 lines not on this page · raw message

Signature · 1 line
2.55.0
author maintainer
#21 lore raw

RFC PATCH 16/17 x86/sev: Enable secure MMIO (phase2)

In reply to Alexey Kardashevskiy

In order to enable secure MMIO, RMP needs to be updated. Unlike RAM (where PVALIDATE validate the RMP entry), MMIO RMP entries require assistance from the PSP.

Add TDI_INFO TIO guest request command. Request the TDI interface report from the PSP. Use the report to validate MMIO ranges reported as non-NonTEE (i.e. encrypted). The TSM subsystem notifies the PCI subsystem to automatically map validated MMIO ranges as encrypted.

Since for MMIO the TSM driver needs TDISP report, add shared memory buffers for receiving the device evidence. Use DMA SWIOTLB for these buffers, explicitly, by forcing 32bit DMA mask.

---
 include/uapi/linux/sev-guest.h     |  12 +
 drivers/virt/coco/sev-guest/core.c |   3 +-
 drivers/virt/coco/sev-guest/tio.c  | 394 +++++++++++++++++++-
 3 files changed, 400 insertions(+), 9 deletions(-)
include/uapi/linux/sev-guest.h +12 -0
index fcdfea767fca..28db79886ca5 100644
@@ -13,6 +13,7 @@
 #define __UAPI_LINUX_SEV_GUEST_H_
 
 #include <linux/types.h>
+#include <linux/uuid.h>
 
 #define SNP_REPORT_USER_DATA_SIZE 64
 
@@ -96,4 +97,15 @@ struct snp_ext_report_req {
 #define SNP_GUEST_VMM_ERR_INVALID_LEN	1
 #define SNP_GUEST_VMM_ERR_BUSY		2
 
+/* Optional Certificates/measurements/report data from TIO_GUEST_REQUEST */
+struct tio_blob_table_entry {
+	guid_t guid;
+	__u32 offset;
+	__u32 length;
+} __packed;
+
+/* Attestation report: 70dc5b0e-0cc0-4cd5-97bb-ff0ba25bf320 */
+#define TIO_GUID_REPORT \
+	GUID_INIT(0x70dc5b0e, 0x0cc0, 0x4cd5, 0x97, 0xbb, 0xff, 0x0b, 0xa2, 0x5b, 0xf3, 0x20)
+
 #endif /* __UAPI_LINUX_SEV_GUEST_H_ */
drivers/virt/coco/sev-guest/core.c +2 -1
index 8448c123ab1e..8710c41daed6 100644
@@ -23,6 +23,7 @@
 #include <linux/uuid.h>
 #include <linux/configfs.h>
 #include <linux/mm.h>
+#include <linux/dma-mapping.h>
 #include <uapi/linux/sev-guest.h>
 #include <uapi/linux/psp-sev.h>
 
@@ -677,7 +678,7 @@ static int __init sev_guest_probe(struct platform_device *pdev)
 	if (!sev_hv_features_ptr || !(*sev_hv_features_ptr & GHCB_HV_FT_SNP_SEV_TIO))
 		tsm_enable = false;
 
-	if (tsm_enable)
+	if (tsm_enable && !dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)))
 		sev_guest_tsm_set_ops(true, snp_dev);
 
 	return 0;
drivers/virt/coco/sev-guest/tio.c +386 -8

469 lines not on this page · raw message

Signature · 1 line
2.55.0
author maintainer
#22 lore raw

RFC PATCH 17/17 x86/sev: Flush IOMMU TLB for trusted devices

In reply to Alexey Kardashevskiy

IOMMU performs RMP checks when SNP is enabled, the results are cached along with the IOMMU translations. When a VM lowers permission of a mapped page (moves to a lower VMPL level or from read+write to read-only or private to shared), the cached RMP check results require invalidation.

At the moment the only way to invalidate IOMMU cache is the RMPUPDATE instruction which flushes all IOMMU TLBs. It is a host privileged instruction so a VM needs a way to ensure the host has done it. Note that the guest's RMPADJUST/PVALIDATE do not flush IOMMU TLBs.

The host implements a new "IOMMU TLB Flush" VMGEXIT code which is advertised via bit#11 in the GHCB Hypervisor capabilities.

Use RMPUPDATE in the following way:
- allocate a page per VCPU (to allow lockless flushing);
- When invalidation is needed, copy two patterns (A and B) to the page;
- invalidate the page so the host can make it shared;
- use new GHCB call to request RMPUPDATE on the host;
- the host makes the page shared;
- the host clears pattern A;
- the host makes the page private again;
- the host returns to the guest;
- check if pattern A has changed and pattern B has not;
- if the above failed, panic().

The patterns are located far enough to not hit the same cache line to work with the cipher text hiding feature.

The host can choose to not execute the request, WARN_ON if this is the case. Further patches will attempt to handle this in other way.

---
 arch/x86/include/asm/sev-common.h |  2 +
 arch/x86/include/uapi/asm/svm.h   |  3 +
 arch/x86/coco/sev/core.c          | 92 ++++++++++++++++++++
 3 files changed, 97 insertions(+)
arch/x86/include/asm/sev-common.h +2 -0
index ff763c3c5d63..51abf8d061fa 100644
@@ -138,6 +138,7 @@ enum psc_op {
 #define GHCB_HV_FT_SNP_AP_CREATION	BIT_ULL(1)
 #define GHCB_HV_FT_SNP_MULTI_VMPL	BIT_ULL(5)
 #define GHCB_HV_FT_SNP_SEV_TIO		BIT_ULL(7)
+#define GHCB_HV_FT_SNP_IOMMU_TLB_FLUSH	BIT_ULL(11)
 
 /*
  * SNP Page State Change NAE event
@@ -210,6 +211,7 @@ struct snp_psc_desc {
 #define GHCB_TERM_SECURE_TSC		10	/* Secure TSC initialization failed */
 #define GHCB_TERM_SVSM_CA_REMAP_FAIL	11	/* SVSM is present but CA could not be remapped */
 #define GHCB_TERM_SAVIC_FAIL		12	/* Secure AVIC-specific failure */
+#define GHCB_TERM_IOMMUTLB_FLUSH	13	/* IOMMUTLB flush failed for SEV-TIO device */
 
 #define GHCB_RESP_CODE(v)		((v) & GHCB_MSR_INFO_MASK)
 
arch/x86/include/uapi/asm/svm.h +3 -0
index 93597ad492bf..269050942c8e 100644
@@ -160,6 +160,8 @@
 #define SVM_VMGEXIT_SEV_TIO_OP_UNBIND	1
 #define SVM_VMGEXIT_SEV_TIO_OP_RUN	2
 #define SVM_VMGEXIT_SEV_TIO_OP_STOP	3
+#define SVM_VMGEXIT_IOMMU_TLB_FLUSH		0x80000022ull
+#define SVM_VMGEXIT_IOMMU_TLB_FLUSH_NO_ACTION	1
 #define SVM_VMGEXIT_HV_FEATURES			0x8000fffdull
 #define SVM_VMGEXIT_TERM_REQUEST		0x8000fffeull
 #define SVM_VMGEXIT_TERM_REASON(reason_set, reason_code)	\
@@ -285,6 +287,7 @@
 	{ SVM_VMGEXIT_AP_CREATION,	"vmgexit_ap_creation" }, \
 	{ SVM_VMGEXIT_SEV_TIO_GR,	"vmgexit_sev_tio_guest_request" }, \
 	{ SVM_VMGEXIT_SEV_TIO_OP,	"vmgexit_sev_tio_op" }, \
+	{ SVM_VMGEXIT_IOMMU_TLB_FLUSH, "vmgexit_sev_tio_iommu_tlb_flush" }, \
 	{ SVM_VMGEXIT_HV_FEATURES,	"vmgexit_hypervisor_feature" }, \
 	{ SVM_EXIT_ERR,         "invalid_guest_state" }
 
arch/x86/coco/sev/core.c +92 -0

122 lines not on this page · raw message

Signature · 1 line
2.55.0
#23 lore raw

Re: [RFC PATCH kernel 17/17] x86/sev: Flush IOMMU TLB for trusted devices

Quoted 41 lines · Alexey Kardashevskiy
(quoted diff) +34 -0
+static int alloc_iommu_tlb_flush_ghcb_pages(void)
+{
+       unsigned int cpu;
+       struct page *pg;
+       void *p;
+
+       /*
+        * Allocate per CPU pages while encrypted DMA is not happening yet
+        * and smashing is cheap.
+        */
+       for_each_possible_cpu(cpu) {
+               if (per_cpu(iommu_tlb_flush_ghcb_page, cpu))
+                       continue;
+
+               pg = alloc_pages_node(cpu_to_node(cpu), GFP_KERNEL, 0);
+               if (!pg)
+                       return -ENOMEM;
+
+               p = page_to_virt(pg);
+               /* Trigger psmash in the host os now to avoid psmash race later */
+               snp_set_memory_shared((unsigned long)p, 1);
+               snp_set_memory_private((unsigned long)p, 1);
+               per_cpu(iommu_tlb_flush_ghcb_page, cpu) = p;
+       }
+
+       return 0;
+}
 
 int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
 {
@@ -111,6 +142,24 @@ int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
        struct ghcb *ghcb;
        int ret;
 
+       if (!(sev_hv_features & GHCB_HV_FT_SNP_SEV_TIO))
+               return -EPERM;
+
+       if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN || op == SVM_VMGEXIT_SEV_TIO_OP_STOP) {
+               if (!(sev_hv_features & GHCB_HV_FT_SNP_IOMMU_TLB_FLUSH))
+                       return -EPERM;
+

Review int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id) · line 161

Quoted 10 lines
(quoted diff) +10 -0
+               if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN) {
+                       if (atomic_inc_return(&sev_tio_devices_num) == 1) {
+                               ret = alloc_iommu_tlb_flush_ghcb_pages();
+                               if (ret)
+                                       return ret;
+                       }
+               } else if (atomic_dec_return(&sev_tio_devices_num) == 0) {
+                       /* Do cleanup or leave it like this? */
+               }
+       }

Hi Alexey,

When testing this series and accepting a locked TDI in the guest (echo 1 > /sys/bus/pci/devices/.../tsm/accept), the guest immediately terminates with 0x1:0xd (SEV_TERM_SET_LINUX, GHCB_TERM_IOMMUTLB_FLUSH).

In sev_tio_op(), sev_tio_devices_num is incremented from 0 to 1 before alloc_iommu_tlb_flush_ghcb_pages() allocates and initializes the per-CPU iommu_tlb_flush_ghcb_page buffers:

1. atomic_inc_return(&sev_tio_devices_num) sets sev_tio_devices_num = 1 while iommu_tlb_flush_ghcb_page is still NULL on all CPUs.
2. alloc_iommu_tlb_flush_ghcb_pages() allocates p for cpu = 0 and calls snp_set_memory_shared((unsigned long)p, 1) to pre-smash the 2M page before per_cpu(iommu_tlb_flush_ghcb_page, cpu) is assigned (and before other CPUs' pages are allocated, in case this task is running on cpu > 0).
3. snp_set_memory_shared() -> __set_pages_state() sees atomic_read(&sev_tio_devices_num) != 0 and calls ghcb_flush_iommu_tlb().
4. ghcb_flush_iommu_tlb() reads this_cpu_read(iommu_tlb_flush_ghcb_page), gets NULL, and returns -ENOMEM.
5. __set_pages_state() treats the non-zero return as fatal and calls sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_IOMMUTLB_FLUSH).

Calling alloc_iommu_tlb_flush_ghcb_pages() before incrementing sev_tio_devices_num avoids triggering ghcb_flush_iommu_tlb() while the per-CPU pages are still being pre-smashed and initialized:

arch/x86/coco/sev/core.c +6 -7
@@ -150,13 +150,12 @@ int sev_tio_op(...)
             return -EPERM;
 
         if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN) {
-            if (atomic_inc_return(&sev_tio_devices_num) == 1) {
-                ret = alloc_iommu_tlb_flush_ghcb_pages();
-                if (ret)
-                    return ret;
-            }
-        } else if (atomic_dec_return(&sev_tio_devices_num) == 0) {
-            /* Do cleanup or leave it like this? */
+            ret = alloc_iommu_tlb_flush_ghcb_pages();
+            if (ret)
+                return ret;
+            atomic_inc(&sev_tio_devices_num);
+        } else {
+            atomic_dec_if_positive(&sev_tio_devices_num);
         }
     }
 
Quoted 216 lines · Alexey Kardashevskiy

IOMMU performs RMP checks when SNP is enabled, the results are cached along with the IOMMU translations. When a VM lowers permission of a mapped page (moves to a lower VMPL level or from read+write to read-only or private to shared), the cached RMP check results require invalidation.

At the moment the only way to invalidate IOMMU cache is the RMPUPDATE instruction which flushes all IOMMU TLBs. It is a host privileged instruction so a VM needs a way to ensure the host has done it. Note that the guest's RMPADJUST/PVALIDATE do not flush IOMMU TLBs.

The host implements a new "IOMMU TLB Flush" VMGEXIT code which is advertised via bit#11 in the GHCB Hypervisor capabilities.

Use RMPUPDATE in the following way:
- allocate a page per VCPU (to allow lockless flushing);
- When invalidation is needed, copy two patterns (A and B) to the page;
- invalidate the page so the host can make it shared;
- use new GHCB call to request RMPUPDATE on the host;
- the host makes the page shared;
- the host clears pattern A;
- the host makes the page private again;
- the host returns to the guest;
- check if pattern A has changed and pattern B has not;
- if the above failed, panic().

The patterns are located far enough to not hit the same cache line to work with the cipher text hiding feature.

The host can choose to not execute the request, WARN_ON if this is the case. Further patches will attempt to handle this in other way.

---
 arch/x86/include/asm/sev-common.h |  2 +
 arch/x86/include/uapi/asm/svm.h   |  3 +
 arch/x86/coco/sev/core.c          | 92 ++++++++++++++++++++
 3 files changed, 97 insertions(+)
arch/x86/include/asm/sev-common.h +2 -0
index ff763c3c5d63..51abf8d061fa 100644
@@ -138,6 +138,7 @@ enum psc_op {
 #define GHCB_HV_FT_SNP_AP_CREATION     BIT_ULL(1)
 #define GHCB_HV_FT_SNP_MULTI_VMPL      BIT_ULL(5)
 #define GHCB_HV_FT_SNP_SEV_TIO         BIT_ULL(7)
+#define GHCB_HV_FT_SNP_IOMMU_TLB_FLUSH BIT_ULL(11)
 
 /*
  * SNP Page State Change NAE event
@@ -210,6 +211,7 @@ struct snp_psc_desc {
 #define GHCB_TERM_SECURE_TSC           10      /* Secure TSC initialization failed */
 #define GHCB_TERM_SVSM_CA_REMAP_FAIL   11      /* SVSM is present but CA could not be remapped */
 #define GHCB_TERM_SAVIC_FAIL           12      /* Secure AVIC-specific failure */
+#define GHCB_TERM_IOMMUTLB_FLUSH       13      /* IOMMUTLB flush failed for SEV-TIO device */
 
 #define GHCB_RESP_CODE(v)              ((v) & GHCB_MSR_INFO_MASK)
 
arch/x86/include/uapi/asm/svm.h +3 -0
index 93597ad492bf..269050942c8e 100644
@@ -160,6 +160,8 @@
 #define SVM_VMGEXIT_SEV_TIO_OP_UNBIND  1
 #define SVM_VMGEXIT_SEV_TIO_OP_RUN     2
 #define SVM_VMGEXIT_SEV_TIO_OP_STOP    3
+#define SVM_VMGEXIT_IOMMU_TLB_FLUSH            0x80000022ull
+#define SVM_VMGEXIT_IOMMU_TLB_FLUSH_NO_ACTION  1
 #define SVM_VMGEXIT_HV_FEATURES                        0x8000fffdull
 #define SVM_VMGEXIT_TERM_REQUEST               0x8000fffeull
 #define SVM_VMGEXIT_TERM_REASON(reason_set, reason_code)       \
@@ -285,6 +287,7 @@
        { SVM_VMGEXIT_AP_CREATION,      "vmgexit_ap_creation" }, \
        { SVM_VMGEXIT_SEV_TIO_GR,       "vmgexit_sev_tio_guest_request" }, \
        { SVM_VMGEXIT_SEV_TIO_OP,       "vmgexit_sev_tio_op" }, \
+       { SVM_VMGEXIT_IOMMU_TLB_FLUSH, "vmgexit_sev_tio_iommu_tlb_flush" }, \
        { SVM_VMGEXIT_HV_FEATURES,      "vmgexit_hypervisor_feature" }, \
        { SVM_EXIT_ERR,         "invalid_guest_state" }
 
arch/x86/coco/sev/core.c +92 -1
index ed0e4546d5e5..aa5a3abb4796 100644
@@ -44,6 +44,7 @@
 #include <asm/cpuid/api.h>
 #include <asm/cmdline.h>
 #include <asm/msr.h>
+#include <asm/archrandom.h>
 
 #include "internal.h"
 
@@ -103,6 +104,36 @@ static unsigned long snp_tsc_freq_khz __ro_after_init;
 
 DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
 DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
+DEFINE_PER_CPU(u8 *, iommu_tlb_flush_ghcb_page);
+static atomic_t sev_tio_devices_num;
+
+static int alloc_iommu_tlb_flush_ghcb_pages(void)
+{
+       unsigned int cpu;
+       struct page *pg;
+       void *p;
+
+       /*
+        * Allocate per CPU pages while encrypted DMA is not happening yet
+        * and smashing is cheap.
+        */
+       for_each_possible_cpu(cpu) {
+               if (per_cpu(iommu_tlb_flush_ghcb_page, cpu))
+                       continue;
+
+               pg = alloc_pages_node(cpu_to_node(cpu), GFP_KERNEL, 0);
+               if (!pg)
+                       return -ENOMEM;
+
+               p = page_to_virt(pg);
+               /* Trigger psmash in the host os now to avoid psmash race later */
+               snp_set_memory_shared((unsigned long)p, 1);
+               snp_set_memory_private((unsigned long)p, 1);
+               per_cpu(iommu_tlb_flush_ghcb_page, cpu) = p;
+       }
+
+       return 0;
+}
 
 int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
 {
@@ -111,6 +142,24 @@ int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
        struct ghcb *ghcb;
        int ret;
 
+       if (!(sev_hv_features & GHCB_HV_FT_SNP_SEV_TIO))
+               return -EPERM;
+
+       if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN || op == SVM_VMGEXIT_SEV_TIO_OP_STOP) {
+               if (!(sev_hv_features & GHCB_HV_FT_SNP_IOMMU_TLB_FLUSH))
+                       return -EPERM;
+
+               if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN) {
+                       if (atomic_inc_return(&sev_tio_devices_num) == 1) {
+                               ret = alloc_iommu_tlb_flush_ghcb_pages();
+                               if (ret)
+                                       return ret;
+                       }
+               } else if (atomic_dec_return(&sev_tio_devices_num) == 0) {
+                       /* Do cleanup or leave it like this? */
+               }
+       }
+
        /* __sev_get_ghcb() needs IRQs disabled because it uses per-CPU GHCB. */
        guard(irqsave)();
 
@@ -347,6 +396,42 @@ static int vmgexit_psc(struct ghcb *ghcb, struct snp_psc_desc *desc)
        return ret;
 }
 
+static int ghcb_flush_iommu_tlb(struct ghcb *ghcb)
+{
+       /* AES encrypts with 16 byte blocks */
+       unsigned long s1[BITS_TO_LONGS(128)], s2[BITS_TO_LONGS(128)];
+       void *p = this_cpu_read(iommu_tlb_flush_ghcb_page), *p2;
+       struct es_em_ctxt ctxt;
+       int ret;
+
+       if (!p)
+               return -ENOMEM;
+
+       /* Keep patterns apart far enough to not share the same cache line */
+       p2 = (u8 *) p + 2048;
+
+       vc_ghcb_invalidate(ghcb);
+
+       BUILD_BUG_ON(ARRAY_SIZE(s1) != 2);
+       if (!rdrand_long(s1) || !rdrand_long(s1 + 1) ||
+           !rdrand_long(s2) || !rdrand_long(s2 + 1))
+               return -EFAULT;
+
+       memcpy(p, s1, sizeof(s1));
+       memcpy(p2, s2, sizeof(s2));
+
+       pvalidate((unsigned long) p, RMP_PG_SIZE_4K, false);
+       ret = sev_es_ghcb_hv_call(ghcb, &ctxt, SVM_VMGEXIT_IOMMU_TLB_FLUSH, __pa(p), 0);
+       pvalidate((unsigned long) p, RMP_PG_SIZE_4K, true);
+
+       /* Ensure that the host change is visible */
+       smp_mb();
+
+       if (!memcmp(p, s1, sizeof(s1)) || memcmp(p2, s2, sizeof(s2)))
+               return -EFAULT;
+
+       return 0;
+}
 static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long vaddr,
                                       unsigned long vaddr_end, int op)
 {
@@ -404,6 +489,13 @@ static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long
        if (!ghcb || vmgexit_psc(ghcb, data))
                sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PSC);
 
+       if (atomic_read(&sev_tio_devices_num)) {
+               int ret = ghcb_flush_iommu_tlb(ghcb);
+
+               if (ret)
+                       sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_IOMMUTLB_FLUSH);
+       }
+
        __sev_put_ghcb(&state);
 
        local_irq_restore(flags);
--

2.55.0

Signature · 1 line
Jianxiong Gao
author maintainer
#24 lore raw

Re: [RFC PATCH kernel 17/17] x86/sev: Flush IOMMU TLB for trusted devices

Quoted 55 lines · Jianxiong Gao
Quoted 41 lines · Alexey Kardashevskiy
(quoted diff) +34 -0
+static int alloc_iommu_tlb_flush_ghcb_pages(void)
+{
+       unsigned int cpu;
+       struct page *pg;
+       void *p;
+
+       /*
+        * Allocate per CPU pages while encrypted DMA is not happening yet
+        * and smashing is cheap.
+        */
+       for_each_possible_cpu(cpu) {
+               if (per_cpu(iommu_tlb_flush_ghcb_page, cpu))
+                       continue;
+
+               pg = alloc_pages_node(cpu_to_node(cpu), GFP_KERNEL, 0);
+               if (!pg)
+                       return -ENOMEM;
+
+               p = page_to_virt(pg);
+               /* Trigger psmash in the host os now to avoid psmash race later */
+               snp_set_memory_shared((unsigned long)p, 1);
+               snp_set_memory_private((unsigned long)p, 1);
+               per_cpu(iommu_tlb_flush_ghcb_page, cpu) = p;
+       }
+
+       return 0;
+}
 
  int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
  {
@@ -111,6 +142,24 @@ int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
         struct ghcb *ghcb;
         int ret;
 
+       if (!(sev_hv_features & GHCB_HV_FT_SNP_SEV_TIO))
+               return -EPERM;
+
+       if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN || op == SVM_VMGEXIT_SEV_TIO_OP_STOP) {
+               if (!(sev_hv_features & GHCB_HV_FT_SNP_IOMMU_TLB_FLUSH))
+                       return -EPERM;
+

Review int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id) · line 161

Quoted 10 lines
(quoted diff) +10 -0
+               if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN) {
+                       if (atomic_inc_return(&sev_tio_devices_num) == 1) {
+                               ret = alloc_iommu_tlb_flush_ghcb_pages();
+                               if (ret)
+                                       return ret;
+                       }
+               } else if (atomic_dec_return(&sev_tio_devices_num) == 0) {
+                       /* Do cleanup or leave it like this? */
+               }
+       }

Hi Alexey,

Quoted 3 lines

When testing this series and accepting a locked TDI in the guest (echo 1 > /sys/bus/pci/devices/.../tsm/accept), the guest immediately terminates with 0x1:0xd (SEV_TERM_SET_LINUX, GHCB_TERM_IOMMUTLB_FLUSH).

Yup, you are right, I do have it fixed exactly like this in my current working tree, just screwed up my rebase (as a newer CPU will do this invalidate differently) and posted a broken version :-/ Sorry about that. Thanks,

Quoted 263 lines

In sev_tio_op(), sev_tio_devices_num is incremented from 0 to 1 before alloc_iommu_tlb_flush_ghcb_pages() allocates and initializes the per-CPU iommu_tlb_flush_ghcb_page buffers:

1. atomic_inc_return(&sev_tio_devices_num) sets sev_tio_devices_num = 1 while iommu_tlb_flush_ghcb_page is still NULL on all CPUs.
2. alloc_iommu_tlb_flush_ghcb_pages() allocates p for cpu = 0 and calls snp_set_memory_shared((unsigned long)p, 1) to pre-smash the 2M page before per_cpu(iommu_tlb_flush_ghcb_page, cpu) is assigned (and before other CPUs' pages are allocated, in case this task is running on cpu > 0).
3. snp_set_memory_shared() -> __set_pages_state() sees

    atomic_read(&sev_tio_devices_num) != 0 and calls ghcb_flush_iommu_tlb().

4. ghcb_flush_iommu_tlb() reads this_cpu_read(iommu_tlb_flush_ghcb_page), gets NULL, and returns -ENOMEM.
5. __set_pages_state() treats the non-zero return as fatal and calls sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_IOMMUTLB_FLUSH).

Calling alloc_iommu_tlb_flush_ghcb_pages() before incrementing sev_tio_devices_num avoids triggering ghcb_flush_iommu_tlb() while the per-CPU pages are still being pre-smashed and initialized:

arch/x86/coco/sev/core.c +6 -7
@@ -150,13 +150,12 @@ int sev_tio_op(...)
              return -EPERM;
 
          if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN) {
-            if (atomic_inc_return(&sev_tio_devices_num) == 1) {
-                ret = alloc_iommu_tlb_flush_ghcb_pages();
-                if (ret)
-                    return ret;
-            }
-        } else if (atomic_dec_return(&sev_tio_devices_num) == 0) {
-            /* Do cleanup or leave it like this? */
+            ret = alloc_iommu_tlb_flush_ghcb_pages();
+            if (ret)
+                return ret;
+            atomic_inc(&sev_tio_devices_num);
+        } else {
+            atomic_dec_if_positive(&sev_tio_devices_num);
          }
      }
 
Quoted 216 lines · Alexey Kardashevskiy

IOMMU performs RMP checks when SNP is enabled, the results are cached along with the IOMMU translations. When a VM lowers permission of a mapped page (moves to a lower VMPL level or from read+write to read-only or private to shared), the cached RMP check results require invalidation.

At the moment the only way to invalidate IOMMU cache is the RMPUPDATE instruction which flushes all IOMMU TLBs. It is a host privileged instruction so a VM needs a way to ensure the host has done it. Note that the guest's RMPADJUST/PVALIDATE do not flush IOMMU TLBs.

The host implements a new "IOMMU TLB Flush" VMGEXIT code which is advertised via bit#11 in the GHCB Hypervisor capabilities.

Use RMPUPDATE in the following way:
- allocate a page per VCPU (to allow lockless flushing);
- When invalidation is needed, copy two patterns (A and B) to the page;
- invalidate the page so the host can make it shared;
- use new GHCB call to request RMPUPDATE on the host;
- the host makes the page shared;
- the host clears pattern A;
- the host makes the page private again;
- the host returns to the guest;
- check if pattern A has changed and pattern B has not;
- if the above failed, panic().

The patterns are located far enough to not hit the same cache line to work with the cipher text hiding feature.

The host can choose to not execute the request, WARN_ON if this is the case. Further patches will attempt to handle this in other way.

---
  arch/x86/include/asm/sev-common.h |  2 +
  arch/x86/include/uapi/asm/svm.h   |  3 +
  arch/x86/coco/sev/core.c          | 92 ++++++++++++++++++++
  3 files changed, 97 insertions(+)
arch/x86/include/asm/sev-common.h +2 -0
index ff763c3c5d63..51abf8d061fa 100644
@@ -138,6 +138,7 @@ enum psc_op {
  #define GHCB_HV_FT_SNP_AP_CREATION     BIT_ULL(1)
  #define GHCB_HV_FT_SNP_MULTI_VMPL      BIT_ULL(5)
  #define GHCB_HV_FT_SNP_SEV_TIO         BIT_ULL(7)
+#define GHCB_HV_FT_SNP_IOMMU_TLB_FLUSH BIT_ULL(11)
 
  /*
   * SNP Page State Change NAE event
@@ -210,6 +211,7 @@ struct snp_psc_desc {
  #define GHCB_TERM_SECURE_TSC           10      /* Secure TSC initialization failed */
  #define GHCB_TERM_SVSM_CA_REMAP_FAIL   11      /* SVSM is present but CA could not be remapped */
  #define GHCB_TERM_SAVIC_FAIL           12      /* Secure AVIC-specific failure */
+#define GHCB_TERM_IOMMUTLB_FLUSH       13      /* IOMMUTLB flush failed for SEV-TIO device */
 
  #define GHCB_RESP_CODE(v)              ((v) & GHCB_MSR_INFO_MASK)
 
arch/x86/include/uapi/asm/svm.h +3 -0
index 93597ad492bf..269050942c8e 100644
@@ -160,6 +160,8 @@
  #define SVM_VMGEXIT_SEV_TIO_OP_UNBIND  1
  #define SVM_VMGEXIT_SEV_TIO_OP_RUN     2
  #define SVM_VMGEXIT_SEV_TIO_OP_STOP    3
+#define SVM_VMGEXIT_IOMMU_TLB_FLUSH            0x80000022ull
+#define SVM_VMGEXIT_IOMMU_TLB_FLUSH_NO_ACTION  1
  #define SVM_VMGEXIT_HV_FEATURES                        0x8000fffdull
  #define SVM_VMGEXIT_TERM_REQUEST               0x8000fffeull
  #define SVM_VMGEXIT_TERM_REASON(reason_set, reason_code)       \
@@ -285,6 +287,7 @@
         { SVM_VMGEXIT_AP_CREATION,      "vmgexit_ap_creation" }, \
         { SVM_VMGEXIT_SEV_TIO_GR,       "vmgexit_sev_tio_guest_request" }, \
         { SVM_VMGEXIT_SEV_TIO_OP,       "vmgexit_sev_tio_op" }, \
+       { SVM_VMGEXIT_IOMMU_TLB_FLUSH, "vmgexit_sev_tio_iommu_tlb_flush" }, \
         { SVM_VMGEXIT_HV_FEATURES,      "vmgexit_hypervisor_feature" }, \
         { SVM_EXIT_ERR,         "invalid_guest_state" }
 
arch/x86/coco/sev/core.c +92 -1
index ed0e4546d5e5..aa5a3abb4796 100644
@@ -44,6 +44,7 @@
  #include <asm/cpuid/api.h>
  #include <asm/cmdline.h>
  #include <asm/msr.h>
+#include <asm/archrandom.h>
 
  #include "internal.h"
 
@@ -103,6 +104,36 @@ static unsigned long snp_tsc_freq_khz __ro_after_init;
 
  DEFINE_PER_CPU(struct sev_es_runtime_data*, runtime_data);
  DEFINE_PER_CPU(struct sev_es_save_area *, sev_vmsa);
+DEFINE_PER_CPU(u8 *, iommu_tlb_flush_ghcb_page);
+static atomic_t sev_tio_devices_num;
+
+static int alloc_iommu_tlb_flush_ghcb_pages(void)
+{
+       unsigned int cpu;
+       struct page *pg;
+       void *p;
+
+       /*
+        * Allocate per CPU pages while encrypted DMA is not happening yet
+        * and smashing is cheap.
+        */
+       for_each_possible_cpu(cpu) {
+               if (per_cpu(iommu_tlb_flush_ghcb_page, cpu))
+                       continue;
+
+               pg = alloc_pages_node(cpu_to_node(cpu), GFP_KERNEL, 0);
+               if (!pg)
+                       return -ENOMEM;
+
+               p = page_to_virt(pg);
+               /* Trigger psmash in the host os now to avoid psmash race later */
+               snp_set_memory_shared((unsigned long)p, 1);
+               snp_set_memory_private((unsigned long)p, 1);
+               per_cpu(iommu_tlb_flush_ghcb_page, cpu) = p;
+       }
+
+       return 0;
+}
 
  int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
  {
@@ -111,6 +142,24 @@ int sev_tio_op(u32 guest_rid, unsigned int op, u64 *fw_err, u64 *tdi_id)
         struct ghcb *ghcb;
         int ret;
 
+       if (!(sev_hv_features & GHCB_HV_FT_SNP_SEV_TIO))
+               return -EPERM;
+
+       if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN || op == SVM_VMGEXIT_SEV_TIO_OP_STOP) {
+               if (!(sev_hv_features & GHCB_HV_FT_SNP_IOMMU_TLB_FLUSH))
+                       return -EPERM;
+
+               if (op == SVM_VMGEXIT_SEV_TIO_OP_RUN) {
+                       if (atomic_inc_return(&sev_tio_devices_num) == 1) {
+                               ret = alloc_iommu_tlb_flush_ghcb_pages();
+                               if (ret)
+                                       return ret;
+                       }
+               } else if (atomic_dec_return(&sev_tio_devices_num) == 0) {
+                       /* Do cleanup or leave it like this? */
+               }
+       }
+
         /* __sev_get_ghcb() needs IRQs disabled because it uses per-CPU GHCB. */
         guard(irqsave)();
 
@@ -347,6 +396,42 @@ static int vmgexit_psc(struct ghcb *ghcb, struct snp_psc_desc *desc)
         return ret;
  }
 
+static int ghcb_flush_iommu_tlb(struct ghcb *ghcb)
+{
+       /* AES encrypts with 16 byte blocks */
+       unsigned long s1[BITS_TO_LONGS(128)], s2[BITS_TO_LONGS(128)];
+       void *p = this_cpu_read(iommu_tlb_flush_ghcb_page), *p2;
+       struct es_em_ctxt ctxt;
+       int ret;
+
+       if (!p)
+               return -ENOMEM;
+
+       /* Keep patterns apart far enough to not share the same cache line */
+       p2 = (u8 *) p + 2048;
+
+       vc_ghcb_invalidate(ghcb);
+
+       BUILD_BUG_ON(ARRAY_SIZE(s1) != 2);
+       if (!rdrand_long(s1) || !rdrand_long(s1 + 1) ||
+           !rdrand_long(s2) || !rdrand_long(s2 + 1))
+               return -EFAULT;
+
+       memcpy(p, s1, sizeof(s1));
+       memcpy(p2, s2, sizeof(s2));
+
+       pvalidate((unsigned long) p, RMP_PG_SIZE_4K, false);
+       ret = sev_es_ghcb_hv_call(ghcb, &ctxt, SVM_VMGEXIT_IOMMU_TLB_FLUSH, __pa(p), 0);
+       pvalidate((unsigned long) p, RMP_PG_SIZE_4K, true);
+
+       /* Ensure that the host change is visible */
+       smp_mb();
+
+       if (!memcmp(p, s1, sizeof(s1)) || memcmp(p2, s2, sizeof(s2)))
+               return -EFAULT;
+
+       return 0;
+}
  static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long vaddr,
                                        unsigned long vaddr_end, int op)
  {
@@ -404,6 +489,13 @@ static unsigned long __set_pages_state(struct snp_psc_desc *data, unsigned long
         if (!ghcb || vmgexit_psc(ghcb, data))
                 sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PSC);
 
+       if (atomic_read(&sev_tio_devices_num)) {
+               int ret = ghcb_flush_iommu_tlb(ghcb);
+
+               if (ret)
+                       sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_IOMMUTLB_FLUSH);
+       }
+
         __sev_put_ghcb(&state);
 
         local_irq_restore(flags);
--

2.55.0

Signature · 1 line
Alexey