> > On 19/04/2021 17:37, Ralf Ramsauer wrote: > > Hi Bram, > > > > On 19/04/2021 14:17, Bram Hooimeijer wrote: > >> When creating a new page table, the table should be filled with > >> zeroes to prevent decoding invalid entries as valid in the future. > >> Given that in the inmate memory space no assumptions can be made on > >> the contents of unallocated heapspace, zeroing needs to be done explicitly. > > > > Don't we already zero pages when reloading a guest? (I'm not sure) > > IOW: Did you really experience dirty pages? > > Just cross-checked: Looks like we don't zero inmate memory on cell reload. > Now I wonder why I never experienced a similar bug… But we haven't many > inmates that make excessive use of the heap.
I was just writing that I got to the same conclusion. Before sending in the patch, I was seriously considering what I overlooked, given that I never ran into this before either, even when doing memory latency experiments. This occasion it was a single MMIO address, and thus I didn't move the heap_pos to an address beyond the communication region, if that might influence something. I guess not, and sheer luck is involved so far. > > > > > Anyway, if we don't, then we should think if we should zero them. The > > question is, if it is acceptable to leave artefacts of previously > > running inmates in memory. > > I think we should rather implement a zalloc() wrapper around alloc(), which > gives us guarantees to return zeroed pages. We have some other spots in > libinmate where we could substitute alloc/memset-sequences in libinmate, e.g. > in lib/arm-common/mem.c and lib/x86/smp.c. I would also prefer zalloc(), as memset clears the area per byte which seems inefficient. Though I expect the store-buffer in the CPU to combine the writes. Actually, the alloc/memset sequence in arm-common/mem.c prevents the issue from arising on Arm platforms, and this patch uses memset just to be consistent with the Arm implementation. I'll send a new proposal. Thanks, Bram > > Ralf > > > > > Thanks > > Ralf > > > >> > >> Signed-off-by: Bram Hooimeijer > >> <[email protected]> > >> --- > >> inmates/lib/x86/mem.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/inmates/lib/x86/mem.c b/inmates/lib/x86/mem.c index > >> 7e1c8b83..45424ea1 100644 > >> --- a/inmates/lib/x86/mem.c > >> +++ b/inmates/lib/x86/mem.c > >> @@ -58,6 +58,7 @@ void map_range(void *start, unsigned long size, enum > map_type map_type) > >> pt = (unsigned long *)(*pt_entry & PAGE_MASK); > >> } else { > >> pt = alloc(PAGE_SIZE, PAGE_SIZE); > >> + memset(pt, 0, PAGE_SIZE); > >> *pt_entry = (unsigned long)pt | PAGE_DEFAULT_FLAGS; > >> } > >> > >> @@ -66,6 +67,7 @@ void map_range(void *start, unsigned long size, enum > map_type map_type) > >> pt = (unsigned long *)(*pt_entry & PAGE_MASK); > >> } else { > >> pt = alloc(PAGE_SIZE, PAGE_SIZE); > >> + memset(pt, 0, PAGE_SIZE); > >> *pt_entry = (unsigned long)pt | PAGE_DEFAULT_FLAGS; > >> } > >> > >> > > -- 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/AS8PR02MB666320D586D371E642EEC562B6499%40AS8PR02MB6663.eurprd02.prod.outlook.com.
