Hi Ralf,
> Hi Bram,
>
> On 19/04/2021 19:09, Bram Hooimeijer wrote:
> > Adds zalloc(size, align) which allocates memory and fills it with zeros.
> > Replaces usage of alloc() in inmates/lib with zalloc().
> >
> > Motivation is that use of alloc without zeroing in mem.c:map_range()
> > results in illegal page tables if memory contains artefacts.
> >
> > Signed-off-by: Bram Hooimeijer
> > <[email protected]>
> > ---
> > inmates/lib/alloc.c | 13 +++++++++++++
> > inmates/lib/arm-common/mem.c | 3 +--
> > inmates/lib/include/inmate_common.h | 1 +
> > inmates/lib/x86/mem.c | 4 ++--
> > inmates/lib/x86/smp.c | 2 +-
> > 5 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/inmates/lib/alloc.c b/inmates/lib/alloc.c index
> > e68e6cc1..f4c06571 100644
> > --- a/inmates/lib/alloc.c
> > +++ b/inmates/lib/alloc.c
> > @@ -5,6 +5,7 @@
> > *
> > * Authors:
> > * Jan Kiszka <[email protected]>
> > + * Bram Hooimeijer <[email protected]>
> > *
> > * This work is licensed under the terms of the GNU GPL, version 2. See
> > * the COPYING file in the top-level directory.
> > @@ -47,3 +48,15 @@ void *alloc(unsigned long size, unsigned long align)
> > heap_pos = base + size;
> > return (void *)base;
> > }
> > +
> > +void *zalloc(unsigned long size, unsigned long align) {
> > + void *base = alloc(size, align);
> > + unsigned long *p = base;
> > + unsigned long s = size / sizeof(unsigned long);
>
> What if size % sizeof(ulong) != 0?
>
> We don't have that case anywhere at the moment, but you introduce the
> chance to return some dirty bytes for the sake of optimisation.
>
> > +
> > + while (s--)
> > + *p++ = 0;
>
> I understand that this is more performant than calling memset, but I
> recommend to avoid microoptimisation here. If you really run into performance
> issues, then improve memset().
Fair enough! Maybe I'll go around optimizing memset at some point, but let's
keep
it simple for now. I will send a new patch, with your proposal basically.
Thanks for looking into it.
Bram
>
> So I'd recommend sth. like:
>
> void *base;
>
> base = alloc(size, align);
> memset(base, 0, size);
>
> return base;
>
> The rest looks good! I don't expect anything to explode on untested
> architectures as well.
>
> Thanks
> Ralf
>
> > +
> > + return base;
> > +}
> > diff --git a/inmates/lib/arm-common/mem.c
> > b/inmates/lib/arm-common/mem.c index 5951fafc..5064b002 100644
> > --- a/inmates/lib/arm-common/mem.c
> > +++ b/inmates/lib/arm-common/mem.c
> > @@ -56,8 +56,7 @@ void map_range(void *start, unsigned long size, enum
> map_type map_type)
> > while (size) {
> > pgd_index = PGD_INDEX(vaddr);
> > if (!(page_directory[pgd_index] & LONG_DESC_TABLE)) {
> > - pmd = alloc(PAGE_SIZE, PAGE_SIZE);
> > - memset(pmd, 0, PAGE_SIZE);
> > + pmd = zalloc(PAGE_SIZE, PAGE_SIZE);
> > /* ensure the page table walker will see the zeroes */
> > synchronization_barrier();
> >
> > diff --git a/inmates/lib/include/inmate_common.h
> > b/inmates/lib/include/inmate_common.h
> > index 5af1213a..1c20a0af 100644
> > --- a/inmates/lib/include/inmate_common.h
> > +++ b/inmates/lib/include/inmate_common.h
> > @@ -103,6 +103,7 @@ void __attribute__((format(printf, 1, 2)))
> > printk(const char *fmt, ...); extern unsigned long heap_pos;
> >
> > void *alloc(unsigned long size, unsigned long align);
> > +void *zalloc(unsigned long size, unsigned long align);
> >
> > void *memset(void *s, int c, unsigned long n); void *memcpy(void *d,
> > const void *s, unsigned long n); diff --git a/inmates/lib/x86/mem.c
> > b/inmates/lib/x86/mem.c index 7e1c8b83..764bdb4b 100644
> > --- a/inmates/lib/x86/mem.c
> > +++ b/inmates/lib/x86/mem.c
> > @@ -57,7 +57,7 @@ void map_range(void *start, unsigned long size, enum
> map_type map_type)
> > if (*pt_entry & PAGE_FLAG_PRESENT) {
> > pt = (unsigned long *)(*pt_entry & PAGE_MASK);
> > } else {
> > - pt = alloc(PAGE_SIZE, PAGE_SIZE);
> > + pt = zalloc(PAGE_SIZE, PAGE_SIZE);
> > *pt_entry = (unsigned long)pt | PAGE_DEFAULT_FLAGS;
> > }
> >
> > @@ -65,7 +65,7 @@ void map_range(void *start, unsigned long size, enum
> map_type map_type)
> > if (*pt_entry & PAGE_FLAG_PRESENT) {
> > pt = (unsigned long *)(*pt_entry & PAGE_MASK);
> > } else {
> > - pt = alloc(PAGE_SIZE, PAGE_SIZE);
> > + pt = zalloc(PAGE_SIZE, PAGE_SIZE);
> > *pt_entry = (unsigned long)pt | PAGE_DEFAULT_FLAGS;
> > }
> >
> > diff --git a/inmates/lib/x86/smp.c b/inmates/lib/x86/smp.c index
> > 94ce2913..625ddaf0 100644
> > --- a/inmates/lib/x86/smp.c
> > +++ b/inmates/lib/x86/smp.c
> > @@ -54,7 +54,7 @@ void smp_start_cpu(unsigned int cpu_id, void
> (*entry)(void))
> > u64 base_val = ((u64)cpu_id << 32) | APIC_LVL_ASSERT;
> >
> > ap_entry = entry;
> > - stack = alloc(PAGE_SIZE, PAGE_SIZE) + PAGE_SIZE;
> > + stack = zalloc(PAGE_SIZE, PAGE_SIZE) + PAGE_SIZE;
> >
> > write_msr(X2APIC_ICR, base_val | APIC_DM_INIT);
> > delay_us(10000);
> >
--
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/AS8PR02MB66635D8DEC85910F04F0ED27B6489%40AS8PR02MB6663.eurprd02.prod.outlook.com.
