Re: [PATCH v7 09/14] arm: initial drop

Andrew Jones
2014-07-16T09:39:48Z

On Wed, Jul 16, 2014 at 11:22:18AM +0200, Paolo Bonzini wrote:
>
> >diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> >new file mode 100644
> >index 0000000000000..da9dfd7b1f118
> >--- /dev/null
> >+++ b/arm/unittests.cfg
> >@@ -0,0 +1,18 @@
> >+# Define your new unittest following the convention:
> >+# [unittest_name]
> >+# file = foo.flat # Name of the flat file to be used
> >+# smp = 2 # Number of processors the VM will use during this test
> >+# extra_params = -append <params...> # Additional parameters used
> >+# arch = arm/arm64 # Only if test case is specific to one
> >+# groups = group1 group2 # Used to identify test cases with run_tests -g ...
> >+
> >+#
> >+# Test that the configured number of processors (smp = <num>), and
> >+# that the configured amount of memory (-m <MB>) are correctly setup
> >+# by the framework.
> >+#
> >+[selftest::setup]
> >+file = selftest.flat
> >+smp = 1
> >+extra_params = -m 256 -append 'setup smp=1 mem=256'
> >+groups = selftest
>
> Nice. :)
>
> >diff --git a/lib/arm/asm/page.h b/lib/arm/asm/page.h
> >new file mode 100644
> >index 0000000000000..91a4bc3b7f86e
> >--- /dev/null
> >+++ b/lib/arm/asm/page.h
> >@@ -0,0 +1 @@
> >+#include "asm-generic/page.h"
> >diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
> >new file mode 100644
> >index 0000000000000..21445ef2085fc
> >--- /dev/null
> >+++ b/lib/arm/asm/setup.h
> >@@ -0,0 +1,27 @@
> >+#ifndef _ASMARM_SETUP_H_
> >+#define _ASMARM_SETUP_H_
> >+/*
> >+ * Copyright (C) 2014, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> >+ *
> >+ * This work is licensed under the terms of the GNU LGPL, version 2.
> >+ */
> >+#include "libcflat.h"
> >+#include "alloc.h"
> >+
> >+#define NR_CPUS 8
> >+extern u32 cpus[NR_CPUS];
> >+extern int nr_cpus;
> >+
> >+extern phys_addr_t __phys_offset, __phys_end;
> >+
> >+#define PHYS_OFFSET (__phys_offset)
> >+#define PHYS_END (__phys_end)
> >+#define PHYS_SHIFT 40
> >+#define PHYS_SIZE (1ULL << PHYS_SHIFT)
> >+#define PHYS_MASK (PHYS_SIZE - 1ULL)
>
> Can you explain these? I'm not sure I understand this:

arm with LPAE can address 40-bit addrs. PHYS_MASK is handy
to assert all addresses we expect to be addressable, are.

>
> >+ mem_start = regs[0].addr;
> >+ mem_end = mem_start + regs[0].size;
> >+
> >+ assert(!(mem_start & ~PHYS_MASK) && !((mem_end-1) & ~PHYS_MASK));
> >+ assert(freemem_start >= mem_start && freemem_start < mem_end);
> >+
> >+ __phys_offset = mem_start; /* PHYS_OFFSET */
> >+ __phys_end = mem_end; /* PHYS_END */
>
> and I think the macro indirection (__phys_offset vs. PHYS_OFFSET, __phys_end
> vs. PHYS_END) is unnecessary: just call the variables phys_offset and
> phys_end.
>

PHYS_OFFSET is consistent with the kernel naming, so I'd like to keep
that. I invented PHYS_END, as it can serve a nice utility
(mem_size = PHYS_END - PHYS_OFFSET), and I wouldn't want to leave it
as the odd one out by not granting it the privilege of capital letters.

drew