On 03/01/20 10:22 pm, Jan Kiszka wrote:
Please make sure a "From:" line is at the top so that I don't have to
add that manually. git will add that line when you set a different
--from when calling format-patch.
Sure, This time i used git format-patch ..... -4 to format, annotate and
send directly.
will check why the from: line is missing
On 30.12.19 14:24, 'Nikhil Devshatwar' via Jailhouse wrote:
Add support for Texas Instrument's Peripheral Virtualization Unit
* Define a new IOMMU type and extra fields in the platform_data
* Add new cofig option CONFIG_IOMMU_TI_PVU
* Integrate with the arm iommu support such that multiple types
of IOMMU can be supported.
Signed-off-by: Nikhil Devshatwar <[email protected]>
---
hypervisor/arch/arm-common/include/asm/cell.h | 7 +
.../arch/arm-common/include/asm/iommu.h | 1 +
.../arch/arm-common/include/asm/ti-pvu.h | 32 +
hypervisor/arch/arm-common/iommu.c | 9 +
hypervisor/arch/arm64/Kbuild | 1 +
hypervisor/arch/arm64/ti-pvu.c | 556 ++++++++++++++++++
hypervisor/arch/arm64/ti-pvu_priv.h | 141 +++++
include/jailhouse/cell-config.h | 4 +
8 files changed, 751 insertions(+)
create mode 100644 hypervisor/arch/arm-common/include/asm/ti-pvu.h
create mode 100644 hypervisor/arch/arm64/ti-pvu.c
create mode 100644 hypervisor/arch/arm64/ti-pvu_priv.h
diff --git a/hypervisor/arch/arm-common/include/asm/cell.h
b/hypervisor/arch/arm-common/include/asm/cell.h
index 5b1e4207..9c6e8c6f 100644
--- a/hypervisor/arch/arm-common/include/asm/cell.h
+++ b/hypervisor/arch/arm-common/include/asm/cell.h
@@ -15,10 +15,17 @@
#include <jailhouse/paging.h>
+struct pvu_tlb_entry;
+
struct arch_cell {
struct paging_structures mm;
u32 irq_bitmap[1024/32];
+
+ struct {
+ u8 ent_count;
+ struct pvu_tlb_entry *entries;
+ } iommu_pvu; /**< ARM PVU specific fields. */
};
#endif /* !_JAILHOUSE_ASM_CELL_H */
diff --git a/hypervisor/arch/arm-common/include/asm/iommu.h
b/hypervisor/arch/arm-common/include/asm/iommu.h
index dde762c0..399248dc 100644
--- a/hypervisor/arch/arm-common/include/asm/iommu.h
+++ b/hypervisor/arch/arm-common/include/asm/iommu.h
@@ -16,6 +16,7 @@
#include <jailhouse/cell.h>
#include <jailhouse/utils.h>
#include <jailhouse/cell-config.h>
+#include <asm/ti-pvu.h>
#define for_each_stream_id(sid, config, counter) \
for ((sid) = (jailhouse_cell_stream_ids(config)[0]), (counter)
= 0; \
diff --git a/hypervisor/arch/arm-common/include/asm/ti-pvu.h
b/hypervisor/arch/arm-common/include/asm/ti-pvu.h
new file mode 100644
index 00000000..a3ef72f7
--- /dev/null
+++ b/hypervisor/arch/arm-common/include/asm/ti-pvu.h
@@ -0,0 +1,32 @@
+/*
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) 2018 Texas Instruments Incorporated -
http://www.ti.com/
+ *
+ * TI PVU IOMMU unit API headers
+ *
+ * Authors:
+ * Nikhil Devshatwar <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef _IOMMMU_PVU_H_
+#define _IOMMMU_PVU_H_
Re-inclusion protection should only be added when actually needed.
Got it
+
+#include <jailhouse/config.h>
+
+#ifdef CONFIG_IOMMU_TI_PVU
Can we try to model this without a compile-time switch?
Only reason I put a config is because this applies only to one platform.
Will remove this in v2
+
+int pvu_iommu_map_memory(struct cell *cell,
+ const struct jailhouse_memory *mem);
+
+int pvu_iommu_unmap_memory(struct cell *cell,
+ const struct jailhouse_memory *mem);
+
+int pvu_iommu_config_commit(struct cell *cell);
+
+#endif /* CONFIG_IOMMU_TI_PVU */
+
+#endif /* _IOMMMU_PVU_H_ */
diff --git a/hypervisor/arch/arm-common/iommu.c
b/hypervisor/arch/arm-common/iommu.c
index b3100d03..b6b61f52 100644
--- a/hypervisor/arch/arm-common/iommu.c
+++ b/hypervisor/arch/arm-common/iommu.c
@@ -26,15 +26,24 @@ unsigned int iommu_count_units(void)
int iommu_map_memory_region(struct cell *cell,
const struct jailhouse_memory *mem)
{
+#ifdef CONFIG_IOMMU_TI_PVU
+ return pvu_iommu_map_memory(cell, mem);
+#endif
return 0;
}
int iommu_unmap_memory_region(struct cell *cell,
const struct jailhouse_memory *mem)
{
+#ifdef CONFIG_IOMMU_TI_PVU
+ return pvu_iommu_unmap_memory(cell, mem);
+#endif
return 0;
}
void iommu_config_commit(struct cell *cell)
{
+#ifdef CONFIG_IOMMU_TI_PVU
+ pvu_iommu_config_commit(cell);
+#endif
}
diff --git a/hypervisor/arch/arm64/Kbuild b/hypervisor/arch/arm64/Kbuild
index 323b78b6..8012c46e 100644
--- a/hypervisor/arch/arm64/Kbuild
+++ b/hypervisor/arch/arm64/Kbuild
@@ -21,3 +21,4 @@ always := lib.a
lib-y := $(common-objs-y)
lib-y += entry.o setup.o control.o mmio.o paging.o caches.o traps.o
smmu-v3.o
+lib-$(CONFIG_IOMMU_TI_PVU) += ti-pvu.o
diff --git a/hypervisor/arch/arm64/ti-pvu.c
b/hypervisor/arch/arm64/ti-pvu.c
new file mode 100644
index 00000000..02380baa
--- /dev/null
+++ b/hypervisor/arch/arm64/ti-pvu.c
@@ -0,0 +1,556 @@
+/*
+ * Jailhouse, a Linux-based partitioning hypervisor
+ *
+ * Copyright (c) 2018 Texas Instruments Incorporated -
http://www.ti.com/
+ *
+ * TI PVU IOMMU unit
+ *
+ * Peripheral Virtualization Unit(PVU) is an IOMMU (memory management
+ * unit for DMA) which is designed for 2nd stage address translation
in a
+ * real time manner.
+ *
+ * Unlike ARM-SMMU, all the memory mapping information is stored in the
+ * local registers instead of the in-memory page tables.
+ *
+ * There are limitations on the number of available contexts, page
sizes,
+ * number of pages that can be mapped, etc.
+ *
+ * PVU is desgined to be programmed with all the memory mapping at
once.
+ * Therefore, it defers the actual register programming till
config_commit.
+ * Also, it does not support unmapping of the pages at runtime.
+ *
+ * Authors:
+ * Nikhil Devshatwar <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
See
+ * the COPYING file in the top-level directory.
+ */
+
+#include <jailhouse/unit.h>
+#include <jailhouse/cell.h>
+#include <jailhouse/entry.h>
+#include <jailhouse/paging.h>
+#include <jailhouse/control.h>
+#include <jailhouse/printk.h>
+#include <asm/iommu.h>
+#include <asm/ti-pvu.h>
+#include "ti-pvu_priv.h"
Single-user header - let's fold this in.
alright
+
+#define MAX_PVU_ENTRIES (PAGE_SIZE / sizeof (struct
pvu_tlb_entry))
+#define MAX_VIRTID 7
+
+static struct pvu_dev pvu_units[JAILHOUSE_MAX_IOMMU_UNITS];
+static unsigned int pvu_count;
+
+static const u64 PVU_PAGE_SIZE_BYTES[] = {
This is not a macro or define, so let's decapitalize its name.
will do
+ [LPAE_PAGE_SZ_4K] = 4 * 1024,
+ [LPAE_PAGE_SZ_16K] = 16 * 1024,
+ [LPAE_PAGE_SZ_64K] = 64 * 1024,
+ [LPAE_PAGE_SZ_2M] = 2 * 1024 * 1024,
+ [LPAE_PAGE_SZ_32M] = 32 * 1024 * 1024,
+ [LPAE_PAGE_SZ_512M] = 512 * 1024 * 1024,
+ [LPAE_PAGE_SZ_1G] = 1 * 1024 * 1024 * 1024,
+ [LPAE_PAGE_SZ_16G] = 16ULL * 1024 * 1024 * 1024,
Is there another use case the LPAE_PAGE_SZ constants in sight?
Otherwise, I would simply fill the array in the right order with the
actual values.
will do
+};
+
+static inline u32 is_aligned(u64 addr, u64 size)
+{
+ return (addr % size) == 0;
+}
+
+static void pvu_tlb_enable(struct pvu_dev *dev, u16 tlbnum)
+{
+ struct pvu_hw_tlb *tlb;
+
+ tlb = (struct pvu_hw_tlb *)dev->tlb_base + tlbnum;
+ mmio_write32_field(&tlb->chain, PVU_TLB_LOG_DIS_MASK, 0);
+ mmio_write32_field(&tlb->chain, PVU_TLB_EN_MASK, 1);
+}
+
+static void pvu_tlb_disable(struct pvu_dev *dev, u16 tlbnum)
+{
+ struct pvu_hw_tlb *tlb;
+
+ tlb = (struct pvu_hw_tlb *)dev->tlb_base + tlbnum;
+ mmio_write32_field(&tlb->chain, PVU_TLB_EN_MASK, 0);
+ mmio_write32_field(&tlb->chain, PVU_TLB_LOG_DIS_MASK, 1);
+}
+
+static u32 pvu_tlb_is_enabled(struct pvu_dev *dev, u16 tlbnum)
+{
+ struct pvu_hw_tlb *tlb;
+
+ tlb = (struct pvu_hw_tlb *)dev->tlb_base + tlbnum;
+ if (mmio_read32_field(&tlb->chain, PVU_TLB_EN_MASK))
+ return 1;
+ else
+ return 0;
+}
+
+static int pvu_tlb_chain(struct pvu_dev *dev, u16 tlbnum, u16 tlb_next)
+{
+ struct pvu_hw_tlb *tlb;
+
+ if (tlb_next <= tlbnum || tlb_next <= dev->max_virtid)
+ return -EINVAL;
+
+ tlb = (struct pvu_hw_tlb *)dev->tlb_base + tlbnum;
+ mmio_write32_field(&tlb->chain, PVU_TLB_CHAIN_MASK, tlb_next);
+ return 0;
+}
+
+static u32 pvu_tlb_next(struct pvu_dev *dev, u16 tlbnum)
+{
+ struct pvu_hw_tlb *tlb;
+
+ tlb = (struct pvu_hw_tlb *)dev->tlb_base + tlbnum;
+ return mmio_read32_field(&tlb->chain, PVU_TLB_CHAIN_MASK);
+}
+
+static u32 pvu_tlb_alloc(struct pvu_dev *dev, u16 virtid)
+{
+ int i;
Let's use an appropriate unsigned type here, e.g. unsigned int.
will review signedness of all variables and fix it
+
+ for (i = dev->max_virtid + 1; i < dev->num_tlbs; i++) {
+ if (dev->tlb_data[i] == 0) {
+ dev->tlb_data[i] = virtid << dev->num_entries;
+ return i;
+ }
+
[snip]
+{
+ /*
+ * dummy unmap for now
+ * PVU does not support dynamic unmap
+ * Works well for static partitioning
Huh!? But this breaks cell create/destroy cycles, without any user
notice, no? And will root cell devices keep access to inmate memory that
is carved out during cell creation?
Is that a hardware limitation?
Looks like a blocker...
Yes, this is a hardware limitation. I it designed for static partitioning.
Although, I made sure to not break memory isolatio with the following
workaround:
When booting a root cell for Jailhouse, you would typically carveout
memory for the
inmate cell. I have defined the cell configs such that, in the root cell
config, RAM region for inmate is
NOT marked with MEM_DMA, this way it never gets mapped in PVU.
When creating cell, root cell maps the inmate RAM loadable region, here
that memory is not
mapped in IO space.
---> Limitation of this is that you cannot DMA copy the images in the
loadable sections,
which we are not doing anyways
When destroying the cell, Jailhouse should map the memory back to the
root cell.
Here, again, the inmate RAM region gets ignored in IO mapping because of
lacking flag MEM_DMA
cell_create and cell_destroy work in regression, tested successfully.
+ */
+ return 0;
+}
+
+int pvu_iommu_config_commit(struct cell *cell)
+{
[snip]
+#define LPAE_PAGE_SZ_2M 3
+#define LPAE_PAGE_SZ_32M 4
+#define LPAE_PAGE_SZ_512M 5
+#define LPAE_PAGE_SZ_1G 6
+#define LPAE_PAGE_SZ_16G 7
+
+#define LPAE_PAGE_PERM_UR (1 << 15)
+#define LPAE_PAGE_PERM_UW (1 << 14)
+#define LPAE_PAGE_PERM_UX (1 << 13)
+#define LPAE_PAGE_PERM_SR (1 << 12)
+#define LPAE_PAGE_PERM_SW (1 << 11)
+#define LPAE_PAGE_PERM_SX (1 << 10)
+
+#define LPAE_PAGE_MEM_DEVICE (0 << 8)
+#define LPAE_PAGE_MEM_WRITEBACK (1 << 8)
These two are unused - accidentally?
Unused, will remove
+#define LPAE_PAGE_MEM_WRITETHROUGH (2 << 8) > +
+#define LPAE_PAGE_PREFETCH (1 << 6)
+#define LPAE_PAGE_INNER_SHARABLE (1 << 5)
These two as well. I would recommend adding only used constants.
will do
+#define LPAE_PAGE_OUTER_SHARABLE (1 << 4)
+
+#define LPAE_PAGE_IS_NOALLOC (0 << 2)
+#define LPAE_PAGE_IS_WR_ALLOC (1 << 2)
+#define LPAE_PAGE_IS_RD_ALLOC (2 << 2)
+#define LPAE_PAGE_IS_RDWR_ALLOC (3 << 2)
+
+#define LPAE_PAGE_OS_NOALLOC (0 << 0)
+#define LPAE_PAGE_OS_WR_ALLOC (1 << 0)
+#define LPAE_PAGE_OS_RD_ALLOC (2 << 0)
+#define LPAE_PAGE_OS_RDWR_ALLOC (3 << 0)
Here are some unused consts as well.
will remove
+
+struct pvu_hw_tlb_entry {
+ u32 reg0;
+ u32 reg1;
+ u32 reg2;
+ u32 reg3;
+ u32 reg4;
+ u32 reg5;
+ u32 reg6;
+ u32 reg7;
Do these regs really have no names? Can we use an array then?
Yes, the TRM actually has these names for the TLB entry
+};
+
+#define PVU_TLB_EN_MASK (1 << 31)
+#define PVU_TLB_LOG_DIS_MASK (1 << 30)
+#define PVU_TLB_FAULT_MASK (1 << 29)
+#define PVU_TLB_CHAIN_MASK (0xfff)
+
+struct pvu_hw_tlb {
+ u32 chain;
+ u8 resv_32[28];
+ struct pvu_hw_tlb_entry entry[8];
+ u8 resv_4096[3808];
+};
+
+struct pvu_tlb_entry {
+ u64 virt_addr;
+ u64 phys_addr;
+ u64 size;
+ u64 flags;
+};
+
+struct pvu_dev {
+ u32 *cfg_base;
+ u32 *tlb_base;
+
+ u32 num_tlbs;
+ u32 num_entries;
+ u16 max_virtid;
+
+ u16 tlb_data[PVU_NUM_TLBS];
+};
+
+#endif /* __TI_PVU_PRIV_H__ */
diff --git a/include/jailhouse/cell-config.h
b/include/jailhouse/cell-config.h
index d435b9f7..9bb84492 100644
--- a/include/jailhouse/cell-config.h
+++ b/include/jailhouse/cell-config.h
@@ -203,6 +203,7 @@ struct jailhouse_pci_capability {
#define JAILHOUSE_IOMMU_AMD 1
#define JAILHOUSE_IOMMU_INTEL 2
#define JAILHOUSE_IOMMU_SMMUV3 3
+#define JAILHOUSE_IOMMU_PVU 4
struct jailhouse_iommu {
__u32 type;
@@ -213,6 +214,9 @@ struct jailhouse_iommu {
__u8 amd_base_cap;
__u8 amd_msi_cap;
__u32 amd_features;
+
+ __u64 tipvu_tlb_base;
+ __u32 tipvu_tlb_size;
Time to stick the amd fields in their own sub-struct, as well as the
tipvu ones, and then put both struct into a union. Analogously to
jailhouse_system.platform_info.
Agreed, I will do that.
} __attribute__((packed));
struct jailhouse_pio {
Jan
Thanks for the review
Nikhil Devshatwar
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/22d04be0-c674-16a7-f36f-89f06419372c%40ti.com.
