On 06.05.20 18:42, Luca Miccio wrote:


On 5/4/20 8:32 PM, Jan Kiszka wrote:
On 21.04.20 12:03, 'Marco Solieri' via Jailhouse wrote:
From: Luca Miccio <[email protected]>

Add functions for colored page creation and destruction and initialize
coloring on the platform.

The story of the life of a coloring page can be summarized as follows.

1. Bits in the address that are useful for defining colors are computed,
     and used for all mappings. The page size used to obtain the lower
limit
     is assumed to be aligned with the `PAGE_SIZE` constant defaulting at
     4KiB, and also as the unit for the mapping operation, even when
     consecutive pages would be possible.

2. The colored regions can then mapped with a new paging function and
     destructed with the old one, because `paging_destroy*` acts on
virtual
     addresses while coloring happens on the physical ones.
     Paging_create has to handle the remap to root_cell too when e.g.
     destroying cells.

3. The colored unmap function is instead used only when destroying the
     root cell mapping, since we assume that the root cell uses a 1:1
mapping
     for memory regions.

Signed-off-by: Luca Miccio <[email protected]>
Signed-off-by: Marco Solieri <[email protected]>
---
   hypervisor/include/jailhouse/paging.h |  11 ++
   hypervisor/paging.c                   | 155 ++++++++++++++++++++++++++
   2 files changed, 166 insertions(+)

diff --git a/hypervisor/include/jailhouse/paging.h
b/hypervisor/include/jailhouse/paging.h
index 5513c4ec..032a3a04 100644
--- a/hypervisor/include/jailhouse/paging.h
+++ b/hypervisor/include/jailhouse/paging.h
@@ -267,6 +267,17 @@ int paging_destroy(const struct paging_structures
*pg_structs,
              unsigned long virt, unsigned long size,
              unsigned long paging_flags);
   +int paging_create_colored(const struct paging_structures *pg_structs,
+              unsigned long phys, unsigned long size,
+              unsigned long virt, unsigned long access_flags,
+              unsigned long paging_flags,
+              unsigned long *color_bitmask, bool identity_map);
+
+int paging_destroy_colored(const struct paging_structures *pg_structs,
+               unsigned long virt, unsigned long size,
+               unsigned long paging_flags,
+               unsigned long *color_bitmask);
+
   void *paging_map_device(unsigned long phys, unsigned long size);
   void paging_unmap_device(unsigned long phys, void *virt, unsigned
long size);
   diff --git a/hypervisor/paging.c b/hypervisor/paging.c
index 876f1521..e8f741c2 100644
--- a/hypervisor/paging.c
+++ b/hypervisor/paging.c
@@ -5,6 +5,8 @@
    *
    * Authors:
    *  Jan Kiszka <[email protected]>
+ *  Luca Miccio <[email protected]> (cache coloring support)
+ *  Marco Solieri <[email protected]> (cache coloring support)
    *
    * This work is licensed under the terms of the GNU GPL, version 2.
See
    * the COPYING file in the top-level directory.
@@ -14,6 +16,7 @@
   #include <jailhouse/printk.h>
   #include <jailhouse/string.h>
   #include <jailhouse/control.h>
+#include <jailhouse/coloring.h>
     #define BITS_PER_PAGE        (PAGE_SIZE * 8)
   @@ -438,6 +441,153 @@ int paging_destroy(const struct
paging_structures *pg_structs,
       return 0;
   }
   +/**
+ * Create or modify a colored page map.
+ * @param pg_structs    Descriptor of paging structures to be used.
+ * @param phys        Physical address of the region to be mapped.
+ * @param size        Size of the region.
+ * @param virt        Virtual address the region should be mapped to.
+ * @param access_flags    Flags describing the permitted page access,
see
+ * @ref PAGE_ACCESS_FLAGS.
+ * @param color_bitmask    Bitmask specifying value of coloring.
+ * @param identity_map    If true the mapping will be 1:1.
+ *
+ * @return 0 on success, negative error code otherwise.
+ *
+ * @note The function uses only 4 KiB page size for mapping.
+ *
+ * @see paging_destroy_colored
+ * @see paging_get_guest_pages
+ */
+int paging_create_colored(const struct paging_structures *pg_structs,
+              unsigned long phys, unsigned long size,
+              unsigned long virt, unsigned long access_flags,
+              unsigned long paging_flags,
+              unsigned long *color_bitmask, bool identity_map)
+{
+
+    phys &= PAGE_MASK;
+    virt &= PAGE_MASK;
+    size = PAGE_ALIGN(size);
+
+    while (size > 0) {
+        const struct paging *paging = pg_structs->root_paging;
+        page_table_t pt = pg_structs->root_table;
+        pt_entry_t pte;
+        int err;
+
+        phys = next_colored(phys, color_bitmask);
+        if (identity_map)
+            virt = phys;
+
+        while (1) {
+            pte = paging->get_entry(pt, virt);
+            if (paging->page_size == PAGE_SIZE) {
+                paging->set_terminal(pte, phys, access_flags);
+                flush_pt_entry(pte, paging_flags);
+                break;
+            }
+            /* Loop until 4K page size by splitting hugepages */
+            if (paging->entry_valid(pte, PAGE_PRESENT_FLAGS)) {
+                err = split_hugepage(pg_structs->hv_paging,
+                            paging, pte, virt,
+                            paging_flags);
+                if (err)
+                    return err;
+                pt = paging_phys2hvirt(
+                    paging->get_next_pt(pte));
+            } else {
+                pt = page_alloc(&mem_pool, 1);
+                if (!pt)
+                    return -ENOMEM;
+
+                paging->set_next_pt(pte, paging_hvirt2phys(pt));
+                flush_pt_entry(pte, paging_flags);
+            }
+            paging++;
+        }
+        if (pg_structs == &hv_paging_structs)
+            arch_paging_flush_page_tlbs(virt);
+
+        phys += paging->page_size;
+        virt += paging->page_size;
+        size -= paging->page_size;
+    }
+    return 0;
+}
+

Isn't paging_create(...) the same as
paging_create_colored(..., color_bitmask=full, identity_map=dont-care)?
Same fore paging_destroy. This duplication of highly sensitive code must
be avoided.


Actually paging_create_colored forces the usage of PAGE_SIZE
granularity. Considering that next_colored can be "bypassed" if we use 0
for color_bitmask, we can choose between passing 0 or full as
color_bitmask if we want to avoid executing unnecessary code. So I think
that paging_create should be the same as paging_create_colored(...,
color_bitmask=[0|full], identity_map=dont-care) only if PAGING_HUGE is
not set in paging_flags. Am I right?
If so, I think that we can integrate the coloring part into
paging_create if it's ok for you. This, of course, will require
modifying all its occurrences.

You can provide a wrapper if the common (non-colored) case would just mean passing in common identical additional parameters.


On the other hand paging_destroy_colored does not seem to be the same as
the non-colored version. As I said we need to use PAGE_SIZE granularity
but looking at paging_destroy, if I understand correctly, it checks only
if the size to unamp fully covers the hugepage. So I think that if we
try to unmap a colored region bigger than 2 MiB (page entry size after
PAGE_SIZE if the latter is 4KiB), the function will not split the
hugepage. Correct me if I am wrong.


If hugepages can be used or not has nothing to do with whether coloring is used or not. It depends on how large the strides are. If they happen to be wider than the smallest hugepage size, it would in fact be beneficial to have a generic implementation. Can't this already happen with 128 colors and 127 of them assigned to a single inmate?

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

--
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/e7c7a7c0-48df-0ebe-0d2d-7715417ee93e%40siemens.com.

Reply via email to