On 5/6/20 6:51 PM, Jan Kiszka wrote: > 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. >
Yes. You're right. >> >> 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? > Actually, I didn't get the point here. My concern was that paging_destroy does not act *always* on PAGE_SIZE granularity. If we need to unmap colored memory with size > 2 MiB from the root cell, how can we do it with the current implementation of page_destroy? Luca. > Jan > -- 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/216f41f3-0254-a746-8be2-d2cf45550f4a%40gmail.com.
signature.asc
Description: OpenPGP digital signature
