[kvm-unit-tests,7/9] s390x: initialize the physical allocator
diff mbox

Message ID 20180110215348.315-8-david@redhat.com
State New
Headers show

Commit Message

David Hildenbrand Jan. 10, 2018, 9:53 p.m. UTC
We now have the range of free memory, let's initialize the physical
allocator. It is now possible to use alloc_page()/alloc_pages().

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 lib/s390x/sclp.c | 13 +++++++++++++
 s390x/Makefile   |  3 +++
 2 files changed, 16 insertions(+)

Comments

Thomas Huth Jan. 11, 2018, 10:29 a.m. UTC | #1
On 10.01.2018 22:53, David Hildenbrand wrote:
> We now have the range of free memory, let's initialize the physical
> allocator. It is now possible to use alloc_page()/alloc_pages().
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  lib/s390x/sclp.c | 13 +++++++++++++
>  s390x/Makefile   |  3 +++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
> index ee56820..c0492b8 100644
> --- a/lib/s390x/sclp.c
> +++ b/lib/s390x/sclp.c
> @@ -14,14 +14,23 @@
>  #include <asm/page.h>
>  #include <asm/arch_def.h>
>  #include "sclp.h"
> +#include <alloc_phys.h>
> +
> +extern unsigned long stacktop;
>  
>  static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
>  static uint64_t storage_increment_size;
>  static uint64_t max_ram_size;
>  static uint64_t ram_size;
>  
> +static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
> +{
> +	phys_alloc_init(freemem_start, ram_size - freemem_start);
> +}
> +
>  void sclp_memory_setup(void)
>  {
> +	phys_addr_t freemem_start;
>  	ReadInfo *ri = (void *)_sccb;
>  	uint64_t rnmax, rnsize;
>  
> @@ -49,4 +58,8 @@ void sclp_memory_setup(void)
>  	while (ram_size < max_ram_size &&
>  	       tprot(ram_size + storage_increment_size - 1))
>  		ram_size += storage_increment_size;
> +
> +	/* leave another extra page free */
> +	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;

<bikeshedpainting>
I think I'd rather move that above line into mem_init() instead...
</bikeshedpainting>

 Thomas
Paolo Bonzini Jan. 11, 2018, 2 p.m. UTC | #2
On 11/01/2018 11:29, Thomas Huth wrote:
>> +	/* leave another extra page free */
>> +	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;
> <bikeshedpainting>
> I think I'd rather move that above line into mem_init() instead...
> </bikeshedpainting>

Also say why. :)

Paolo
Thomas Huth Jan. 11, 2018, 3:16 p.m. UTC | #3
On 11.01.2018 15:00, Paolo Bonzini wrote:
> On 11/01/2018 11:29, Thomas Huth wrote:
>>> +	/* leave another extra page free */
>>> +	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;
>> <bikeshedpainting>
>> I think I'd rather move that above line into mem_init() instead...
>> </bikeshedpainting>
> 
> Also say why. :)

The calculation does not really belong to the other stuff of
sclp_memory_setup(). And you then also you don't need that freemem_start
variable in sclp_memory_setup() anymore.

 Thomas
Paolo Bonzini Jan. 11, 2018, 3:49 p.m. UTC | #4
On 11/01/2018 16:16, Thomas Huth wrote:
> On 11.01.2018 15:00, Paolo Bonzini wrote:
>> On 11/01/2018 11:29, Thomas Huth wrote:
>>>> +	/* leave another extra page free */
>>>> +	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;
>>> <bikeshedpainting>
>>> I think I'd rather move that above line into mem_init() instead...
>>> </bikeshedpainting>
>>
>> Also say why. :)
> 
> The calculation does not really belong to the other stuff of
> sclp_memory_setup(). And you then also you don't need that freemem_start
> variable in sclp_memory_setup() anymore.

Sorry, say why another page should be free...

Paolo
David Hildenbrand Jan. 11, 2018, 9:08 p.m. UTC | #5
On 11.01.2018 16:49, Paolo Bonzini wrote:
> On 11/01/2018 16:16, Thomas Huth wrote:
>> On 11.01.2018 15:00, Paolo Bonzini wrote:
>>> On 11/01/2018 11:29, Thomas Huth wrote:
>>>>> +	/* leave another extra page free */
>>>>> +	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;
>>>> <bikeshedpainting>
>>>> I think I'd rather move that above line into mem_init() instead...
>>>> </bikeshedpainting>
>>>
>>> Also say why. :)
>>
>> The calculation does not really belong to the other stuff of
>> sclp_memory_setup(). And you then also you don't need that freemem_start
>> variable in sclp_memory_setup() anymore.
> 
> Sorry, say why another page should be free...
> 
> Paolo
> 

Of course because I was trying to hide another bug :)

... without this, I got strange hangs when running tests (like
overwriting the stack).

Turned out we are setting the initital stack to stacktop instead of
stackptr. With that fixed, it works without the extra page.

Patch
diff mbox

diff --git a/lib/s390x/sclp.c b/lib/s390x/sclp.c
index ee56820..c0492b8 100644
--- a/lib/s390x/sclp.c
+++ b/lib/s390x/sclp.c
@@ -14,14 +14,23 @@ 
 #include <asm/page.h>
 #include <asm/arch_def.h>
 #include "sclp.h"
+#include <alloc_phys.h>
+
+extern unsigned long stacktop;
 
 static char _sccb[PAGE_SIZE] __attribute__((__aligned__(4096)));
 static uint64_t storage_increment_size;
 static uint64_t max_ram_size;
 static uint64_t ram_size;
 
+static void mem_init(phys_addr_t freemem_start, phys_addr_t mem_end)
+{
+	phys_alloc_init(freemem_start, ram_size - freemem_start);
+}
+
 void sclp_memory_setup(void)
 {
+	phys_addr_t freemem_start;
 	ReadInfo *ri = (void *)_sccb;
 	uint64_t rnmax, rnsize;
 
@@ -49,4 +58,8 @@  void sclp_memory_setup(void)
 	while (ram_size < max_ram_size &&
 	       tprot(ram_size + storage_increment_size - 1))
 		ram_size += storage_increment_size;
+
+	/* leave another extra page free */
+	freemem_start = ((phys_addr_t)&stacktop + PAGE_SIZE) & PAGE_MASK;
+	mem_init(freemem_start, ram_size);
 }
diff --git a/s390x/Makefile b/s390x/Makefile
index ce63dd1..4198fdc 100644
--- a/s390x/Makefile
+++ b/s390x/Makefile
@@ -21,6 +21,9 @@  asm-offsets = lib/$(ARCH)/asm-offsets.h
 include $(SRCDIR)/scripts/asm-offsets.mak
 
 cflatobjs += lib/util.o
+cflatobjs += lib/alloc.o
+cflatobjs += lib/alloc_phys.o
+cflatobjs += lib/alloc_page.o
 cflatobjs += lib/alloc_phys.o
 cflatobjs += lib/s390x/io.o
 cflatobjs += lib/s390x/stack.o