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().
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/da05bc0c-c47d-f8f7-df61-339eb97a9cc2%40oth-regensburg.de.
