|
[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
As agreed offline, the commit subject will be changed to "Introduce API
to set platform default allocator".
Costin
On 06/27/2018 05:34 PM, Yuri Volchkov wrote:
> 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
>>
>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |