|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH 3/6] plat: Introducing the platform memory allocator
The patch itself looks good to me. But I would change the commit
message. And squash this patch with the next one (4/6 Setting the
platform memory allocator).
The least important note: the most common way across many projects is to
give a subject in imperative. For example "introduce this" "fix that"
rather than "added this" or "fixing that". Probably we should update
CONTRIBUTING.md when Simon is back.
The subject itself sounds like you are introducing the whole new
system for allocating memory for a platform. But in fact that is just
an alias to an existing allocator, which platform code should use.
And the first sentence confirms this wrong impression even further. When
I first read it, I thought that platform needs some sort of early
allocator, because the regular one is not available at the boot time.
Let me propose a draft for commit message:
"set platform default allocator
Platform code needs to know which allocator it should use for allocating
its internal data structures. With this patch, this information is
provided at the boot stage.
Also, as soon as platform gets an allocator, it is a good time to
trigger the creation of internal memory mappings (which would be
essential for platform setup before running the application)"
Costin Lupu <costin.lupu@xxxxxxxxx> writes:
> Kernel subsystems may require memory allocation for internal
> initializations and therefore such operations cannot occur before a
> memory allocator is initialized. We introduce the platform memory
> allocator which should be set right after initializing the default
> memory allocator. Setting the platform allocator also triggers the
> creation of internal memory mappings which would be essential for
> platform setup before running the application. An example of subsystem
> which would need such mappings is the grants subsystem for Xen. The
> current patch contains changes for all platforms.
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
> include/uk/plat/memory.h | 15 ++++++++++++
> plat/common/include/memory.h | 47 ++++++++++++++++++++++++++++++++++++
> plat/common/memory.c | 57
> ++++++++++++++++++++++++++++++++++++++++++++
> plat/kvm/Makefile.uk | 1 +
> plat/kvm/memory.c | 5 ++++
> plat/linuxu/memory.c | 5 ++++
> plat/xen/Makefile.uk | 1 +
> plat/xen/memory.c | 5 ++++
> 8 files changed, 136 insertions(+)
> create mode 100644 plat/common/include/memory.h
> create mode 100644 plat/common/memory.c
>
> diff --git a/include/uk/plat/memory.h b/include/uk/plat/memory.h
> index 4678eb6..60e52e6 100644
> --- a/include/uk/plat/memory.h
> +++ b/include/uk/plat/memory.h
> @@ -37,6 +37,7 @@
> #define __UKPLAT_MEMORY_H__
>
> #include <uk/arch/types.h>
> +#include <uk/alloc.h>
> #include <uk/config.h>
>
> #ifdef __cplusplus
> @@ -77,6 +78,20 @@ int ukplat_memregion_count(void);
> */
> int ukplat_memregion_get(int i, struct ukplat_memregion_desc *mrd);
>
> +/**
> + * Sets the platform memory allocator and triggers the platform memory
> mappings
> + * for which an allocator is needed.
> + * @param a Memory allocator
> + * @return 0 on success, < 0 otherwise
> + */
> +int ukplat_memallocator_set(struct uk_alloc *a);
> +
> +/**
> + * Returns the platform memory allocator
> + * @return Platform memory allocator address
> + */
> +struct uk_alloc *ukplat_memallocator_get(void);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/plat/common/include/memory.h b/plat/common/include/memory.h
> new file mode 100644
> index 0000000..f627348
> --- /dev/null
> +++ b/plat/common/include/memory.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +/*
> + * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
> + *
> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of the copyright holder nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + *
> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> + */
> +
> +#ifndef __PLAT_CMN_MEMORY_H__
> +#define __PLAT_CMN_MEMORY_H__
> +
> +/**
> + * Initializes the platform memory mappings which require an allocator. This
> + * function must always be called after initializing a memory allocator and
> + * before initializing the subsystems that require memory allocation. It is
> an
> + * internal function common to all platforms.
> + * @return 0 on success, < 0 otherwise
> + */
> +int _ukplat_mem_mappings_init(void);
> +
> +#endif /* __PLAT_CMN_MEMORY_H__ */
> diff --git a/plat/common/memory.c b/plat/common/memory.c
> new file mode 100644
> index 0000000..30983a7
> --- /dev/null
> +++ b/plat/common/memory.c
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: BSD-3-Clause */
> +/*
> + * Authors: Costin Lupu <costin.lupu@xxxxxxxxx>
> + *
> + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of the copyright holder nor the names of its
> + * contributors may be used to endorse or promote products derived from
> + * this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + *
> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> + */
> +
> +#include <uk/plat/memory.h>
> +#include <memory.h>
> +
> +static struct uk_alloc *plat_allocator;
> +
> +int ukplat_memallocator_set(struct uk_alloc *a)
> +{
> + UK_ASSERT(a != NULL);
> +
> + if (plat_allocator != NULL)
> + return -1;
> +
> + plat_allocator = a;
> +
> + _ukplat_mem_mappings_init();
> +
> + return 0;
> +}
> +
> +struct uk_alloc *ukplat_memallocator_get(void)
> +{
> + return plat_allocator;
> +}
> diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
> index 2705fd1..9bedb37 100644
> --- a/plat/kvm/Makefile.uk
> +++ b/plat/kvm/Makefile.uk
> @@ -40,5 +40,6 @@ LIBKVMPLAT_SRCS-y += $(LIBKVMPLAT_BASE)/irq.c
> LIBKVMPLAT_SRCS-y += $(LIBKVMPLAT_BASE)/time.c
> LIBKVMPLAT_SRCS-y += $(LIBKVMPLAT_BASE)/tscclock.c
> LIBKVMPLAT_SRCS-y += $(UK_PLAT_COMMON_BASE)/lcpu.c|common
> +LIBKVMPLAT_SRCS-y += $(UK_PLAT_COMMON_BASE)/memory.c|common
>
> LIBKVMPCI_SRCS-y +=
> $(UK_PLAT_COMMON_BASE)/pci_bus.c|common
> diff --git a/plat/kvm/memory.c b/plat/kvm/memory.c
> index 3fe1558..11c993d 100644
> --- a/plat/kvm/memory.c
> +++ b/plat/kvm/memory.c
> @@ -131,3 +131,8 @@ int ukplat_memregion_get(int i, struct
> ukplat_memregion_desc *m)
>
> return ret;
> }
> +
> +int _ukplat_mem_mappings_init(void)
> +{
> + return 0;
> +}
> diff --git a/plat/linuxu/memory.c b/plat/linuxu/memory.c
> index b7dbc43..35d0d95 100644
> --- a/plat/linuxu/memory.c
> +++ b/plat/linuxu/memory.c
> @@ -69,3 +69,8 @@ int ukplat_memregion_get(int i, struct
> ukplat_memregion_desc *m)
>
> return ret;
> }
> +
> +int _ukplat_mem_mappings_init(void)
> +{
> + return 0;
> +}
> diff --git a/plat/xen/Makefile.uk b/plat/xen/Makefile.uk
> index 1341da8..3305a84 100644
> --- a/plat/xen/Makefile.uk
> +++ b/plat/xen/Makefile.uk
> @@ -28,6 +28,7 @@ LIBXENPLAT_CINCLUDES-y +=
> -I$(UK_PLAT_COMMON_BASE)/include
> LIBXENPLAT_SRCS-y += $(LIBXENPLAT_BASE)/hypervisor.c
> LIBXENPLAT_SRCS-y += $(LIBXENPLAT_BASE)/memory.c
> LIBXENPLAT_SRCS-y += $(UK_PLAT_COMMON_BASE)/lcpu.c|common
> +LIBXENPLAT_SRCS-y += $(UK_PLAT_COMMON_BASE)/memory.c|common
>
> ifneq (,$(filter x86_32 x86_64,$(CONFIG_UK_ARCH)))
> LIBXENPLAT_SRCS-$(CONFIG_ARCH_X86_64) +=
> $(UK_PLAT_COMMON_BASE)/x86/trace.c|common
> diff --git a/plat/xen/memory.c b/plat/xen/memory.c
> index 18df5da..6ae92f4 100644
> --- a/plat/xen/memory.c
> +++ b/plat/xen/memory.c
> @@ -119,3 +119,8 @@ int ukplat_memregion_get(int i, struct
> ukplat_memregion_desc *m)
>
> return 0;
> }
> +
> +int _ukplat_mem_mappings_init(void)
> +{
> + return 0;
> +}
> --
> 2.11.0
>
--
Yuri Volchkov
Software Specialist
NEC Europe Ltd
Kurfürsten-Anlage 36
D-69115 Heidelberg
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |