This mostly boils down to initialising the Cache Coherent Interconnect (CCI). We do this by looking in the device-tree for a CCI node, that way the same semihosting bootwrapper binary can be used on both the big.LITTLE models and on the A15 models which don't have a CCI.
[PATCH 1/3] bootwrapper: Allow for multiple clusters in boot CPU [PATCH 2/3] bootwrapper: Factor out parsing of fdt #address-cells [PATCH 3/3] bootwrapper: Initialise CCI device if found in the fdt
boot.S | 2 +- semi_loader.c | 104 +++++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 84 insertions(+), 22 deletions(-)
From: Jon Medhurst tixy@linaro.org
Check all the CPU affinity fields of MPIDR, so we select only the first CPU of the first cluster as the one to boot on.
Signed-off-by: Jon Medhurst tixy@linaro.org --- boot.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot.S b/boot.S index 727119a..40ebd33 100644 --- a/boot.S +++ b/boot.S @@ -121,7 +121,7 @@ start:
@ Check CPU nr again mrc p15, 0, r0, c0, c0, 5 @ MPIDR (ARMv7 only) - and r0, r0, #15 @ CPU number + bfc r0, #24, #8 @ CPU number, taking multicluster into account cmp r0, #0 @ primary CPU? beq 2f
On Thu, Sep 27, 2012 at 07:06:18PM +0100, Jon Medhurst (Tixy) wrote:
From: Jon Medhurst tixy@linaro.org
Check all the CPU affinity fields of MPIDR, so we select only the first CPU of the first cluster as the one to boot on.
This seems reasonable.
The bfc could be a bic, but we only care about ARMv7 here anyway.
Cheers ---Dave
Signed-off-by: Jon Medhurst tixy@linaro.org
boot.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot.S b/boot.S index 727119a..40ebd33 100644 --- a/boot.S +++ b/boot.S @@ -121,7 +121,7 @@ start: @ Check CPU nr again mrc p15, 0, r0, c0, c0, 5 @ MPIDR (ARMv7 only)
- and r0, r0, #15 @ CPU number
- bfc r0, #24, #8 @ CPU number, taking multicluster into account cmp r0, #0 @ primary CPU? beq 2f
1.7.10.4
From: Jon Medhurst tixy@linaro.org
We will be reusing this functionality later.
Signed-off-by: Jon Medhurst tixy@linaro.org --- semi_loader.c | 56 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 22 deletions(-)
diff --git a/semi_loader.c b/semi_loader.c index cbe911c..c9750be 100644 --- a/semi_loader.c +++ b/semi_loader.c @@ -97,6 +97,39 @@ static int _fdt_make_node(void *fdt, int parentoffset, const char *name) return fdt_add_subnode(fdt, parentoffset, name); }
+static void _fdt_address_and_size_cells(void *fdt, int* addrcells, int* sizecells) +{ + int e; + uint32_t const *p; + + if(!(p = fdt_getprop(fdt, 0, "#address-cells", &e))) + goto libfdt_error; + else if(e != 4) + goto size_error; + *addrcells = fdt32_to_cpu(*p); + if(!(p = fdt_getprop(fdt, 0, "#size-cells", &e))) + goto libfdt_error; + else if(e != 4) + goto size_error; + *sizecells = fdt32_to_cpu(*p); + + /* + * Sanity-check address sizes, since addresses and sizes which do + * not take up exactly 4 or 8 bytes are not supported. + */ + if ((*addrcells != 1 && *addrcells != 2) || + (*sizecells != 1 && *sizecells != 2)) + goto size_error; + + return; + +libfdt_error: + fatal("libfdt: ", fdt_strerror(e), ", while looking for #address-cells/#size-cells\n"); + +size_error: + fatal("Unexpected/invalid #address-cells/#size-cells in device tree\n"); +} + static void update_fdt(void **dest, struct loader_info *info) { int e; @@ -112,25 +145,7 @@ static void update_fdt(void **dest, struct loader_info *info) if((e = fdt_open_into((void *)info->fdt_start, fdt, FDT_SIZE_MAX)) < 0) goto libfdt_error;
- /* - * Sanity-check address sizes, since addresses and sizes which do - * not take up exactly 4 or 8 bytes are not supported. - */ - { - if(!(p = fdt_getprop(fdt, 0, "#address-cells", &e))) - goto libfdt_error; - else if(e != 4) - goto size_error; - addrcells = fdt32_to_cpu(*p); - if(!(p = fdt_getprop(fdt, 0, "#size-cells", &e))) - goto libfdt_error; - else if(e != 4) - goto size_error; - sizecells = fdt32_to_cpu(*p); - if ((addrcells != 1 && addrcells != 2) || - (sizecells != 1 && sizecells != 2)) - goto size_error; - } + _fdt_address_and_size_cells(fdt, &addrcells, &sizecells);
/* * Add a memory node, but only if there isn't one already. If @@ -225,9 +240,6 @@ no_add_memory: libfdt_error: fatal("libfdt: ", fdt_strerror(e), ", while updating device tree\n"); - -size_error: - fatal("Unexpected/invalid #address-cells/#size-cells in device tree\n"); }
static int is_space(char c)
On Thu, Sep 27, 2012 at 07:06:19PM +0100, Jon Medhurst (Tixy) wrote:
From: Jon Medhurst tixy@linaro.org
We will be reusing this functionality later.
Commit message? ;)
Makes sense to factor this out, though.
Cheers ---Dave
Signed-off-by: Jon Medhurst tixy@linaro.org
semi_loader.c | 56 ++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 22 deletions(-)
diff --git a/semi_loader.c b/semi_loader.c index cbe911c..c9750be 100644 --- a/semi_loader.c +++ b/semi_loader.c @@ -97,6 +97,39 @@ static int _fdt_make_node(void *fdt, int parentoffset, const char *name) return fdt_add_subnode(fdt, parentoffset, name); } +static void _fdt_address_and_size_cells(void *fdt, int* addrcells, int* sizecells) +{
- int e;
- uint32_t const *p;
- if(!(p = fdt_getprop(fdt, 0, "#address-cells", &e)))
goto libfdt_error;
- else if(e != 4)
goto size_error;
- *addrcells = fdt32_to_cpu(*p);
- if(!(p = fdt_getprop(fdt, 0, "#size-cells", &e)))
goto libfdt_error;
- else if(e != 4)
goto size_error;
- *sizecells = fdt32_to_cpu(*p);
- /*
* Sanity-check address sizes, since addresses and sizes which do
* not take up exactly 4 or 8 bytes are not supported.
*/
- if ((*addrcells != 1 && *addrcells != 2) ||
(*sizecells != 1 && *sizecells != 2))
goto size_error;
- return;
+libfdt_error:
- fatal("libfdt: ", fdt_strerror(e), ", while looking for #address-cells/#size-cells\n");
+size_error:
- fatal("Unexpected/invalid #address-cells/#size-cells in device tree\n");
+}
static void update_fdt(void **dest, struct loader_info *info) { int e; @@ -112,25 +145,7 @@ static void update_fdt(void **dest, struct loader_info *info) if((e = fdt_open_into((void *)info->fdt_start, fdt, FDT_SIZE_MAX)) < 0) goto libfdt_error;
- /*
* Sanity-check address sizes, since addresses and sizes which do
* not take up exactly 4 or 8 bytes are not supported.
*/
- {
if(!(p = fdt_getprop(fdt, 0, "#address-cells", &e)))
goto libfdt_error;
else if(e != 4)
goto size_error;
addrcells = fdt32_to_cpu(*p);
if(!(p = fdt_getprop(fdt, 0, "#size-cells", &e)))
goto libfdt_error;
else if(e != 4)
goto size_error;
sizecells = fdt32_to_cpu(*p);
if ((addrcells != 1 && addrcells != 2) ||
(sizecells != 1 && sizecells != 2))
goto size_error;
- }
- _fdt_address_and_size_cells(fdt, &addrcells, &sizecells);
/* * Add a memory node, but only if there isn't one already. If @@ -225,9 +240,6 @@ no_add_memory: libfdt_error: fatal("libfdt: ", fdt_strerror(e), ", while updating device tree\n");
-size_error:
- fatal("Unexpected/invalid #address-cells/#size-cells in device tree\n");
} static int is_space(char c) -- 1.7.10.4
On Mon, 2012-10-01 at 17:35 +0100, Dave Martin wrote:
On Thu, Sep 27, 2012 at 07:06:19PM +0100, Jon Medhurst (Tixy) wrote:
From: Jon Medhurst tixy@linaro.org
We will be reusing this functionality later.
Commit message? ;)
Yep, that's what it is ;-) Couldn't think what more needed to be said. Subject says what we're doing, body says why. Could reword it like:
A subsequent patch will also need to obtain #address-cells and #size-cells from the fdt.
but I guess you were expecting something else?
On Mon, Oct 01, 2012 at 06:36:57PM +0100, Jon Medhurst (Tixy) wrote:
On Mon, 2012-10-01 at 17:35 +0100, Dave Martin wrote:
On Thu, Sep 27, 2012 at 07:06:19PM +0100, Jon Medhurst (Tixy) wrote:
From: Jon Medhurst tixy@linaro.org
We will be reusing this functionality later.
Commit message? ;)
Yep, that's what it is ;-) Couldn't think what more needed to be said. Subject says what we're doing, body says why. Could reword it like:
A subsequent patch will also need to obtain #address-cells and #size-cells from the fdt.
but I guess you were expecting something else?
For some reason I had missed the context in the subject line -- apologies for that. I guess it's enough.
Cheers ---Dave
From: Jon Medhurst tixy@linaro.org
The A15xA7 models simulate a Cache Coherent Interconnect (CCI) and this needs to be initialised correctly for Linux to boot.
To perform this initiation we add the new function configure_from_fdt() which will look in the fdt for devices to initialise. In this first case we look for the CCI node and if found then setup this device.
Signed-off-by: Jon Medhurst tixy@linaro.org --- semi_loader.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/semi_loader.c b/semi_loader.c index c9750be..ca70633 100644 --- a/semi_loader.c +++ b/semi_loader.c @@ -242,6 +242,54 @@ libfdt_error: fatal("libfdt: ", fdt_strerror(e), ", while updating device tree\n"); }
+/* For accessing 32-bit device ports */ +#define io32(p) (*(volatile uint32_t *)(p)) + +static void init_cci(unsigned cci) +{ + info("Initialising CCI\n"); + + /* + * Ideally, the CCI device tree binding would include suitable + * information so we can correctly configure the CCI, but for + * now we'll just hard-code settings for the present A15xA7 + * models. + */ + + /* Turn on CCI snoops and DVM messages */ + io32(cci+0x4000) = 0x3; /* A15 cluster */ + io32(cci+0x5000) = 0x3; /* A7 cluster */ + + /* Wait while change pending bit of status register is set */ + while(io32(cci+0xc) & 0x1) + {} +} + +static void configure_from_fdt(struct loader_info *info) +{ + void *fdt = (void *)info->fdt_start; + uint32_t const *p; + int addrcells, sizecells; + int offset, len; + + if(!fdt) + return; + + _fdt_address_and_size_cells(fdt, &addrcells, &sizecells); + + /* See if there is a CCI device to initialise */ + offset = fdt_node_offset_by_compatible(fdt, 0, "arm,cci"); + if (offset >= 0) { + p = fdt_getprop(fdt, offset, "reg", &len); + if(len != (addrcells + sizecells) * 4) + info("Failed parsing device-tree node for CCI\n"); + else + init_cci(fdt32_to_cpu(p[addrcells - 1])); + } + + return; +} + static int is_space(char c) { return c == ' '; @@ -598,4 +646,6 @@ args_done: atag_append(&atagp, ATAG_NONE, 0, 0);
update_fdt(&phys, info); + + configure_from_fdt(info); }
On Thu, Sep 27, 2012 at 07:06:20PM +0100, Jon Medhurst (Tixy) wrote:
From: Jon Medhurst tixy@linaro.org
The A15xA7 models simulate a Cache Coherent Interconnect (CCI) and this needs to be initialised correctly for Linux to boot.
To perform this initiation we add the new function configure_from_fdt() which will look in the fdt for devices to initialise. In this first case we look for the CCI node and if found then setup this device.
Signed-off-by: Jon Medhurst tixy@linaro.org
semi_loader.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/semi_loader.c b/semi_loader.c index c9750be..ca70633 100644 --- a/semi_loader.c +++ b/semi_loader.c @@ -242,6 +242,54 @@ libfdt_error: fatal("libfdt: ", fdt_strerror(e), ", while updating device tree\n"); } +/* For accessing 32-bit device ports */ +#define io32(p) (*(volatile uint32_t *)(p))
+static void init_cci(unsigned cci) +{
- info("Initialising CCI\n");
- /*
* Ideally, the CCI device tree binding would include suitable
* information so we can correctly configure the CCI, but for
* now we'll just hard-code settings for the present A15xA7
* models.
*/
- /* Turn on CCI snoops and DVM messages */
- io32(cci+0x4000) = 0x3; /* A15 cluster */
- io32(cci+0x5000) = 0x3; /* A7 cluster */
Ultimately, CCI slave port assignments will be deducible from the DT topology, but we don't have this yet, so I guess this is OK for now.
For the intergrated switcher work we can have a simple local patch for this until/unless this (or the switcher) becomes more generic.
- /* Wait while change pending bit of status register is set */
- while(io32(cci+0xc) & 0x1)
{}
+}
+static void configure_from_fdt(struct loader_info *info) +{
- void *fdt = (void *)info->fdt_start;
- uint32_t const *p;
- int addrcells, sizecells;
- int offset, len;
- if(!fdt)
return;
- _fdt_address_and_size_cells(fdt, &addrcells, &sizecells);
- /* See if there is a CCI device to initialise */
- offset = fdt_node_offset_by_compatible(fdt, 0, "arm,cci");
I'm guessing this binding isn't official yet, but it'll do for now...
- if (offset >= 0) {
p = fdt_getprop(fdt, offset, "reg", &len);
if(len != (addrcells + sizecells) * 4)
info("Failed parsing device-tree node for CCI\n");
else
init_cci(fdt32_to_cpu(p[addrcells - 1]));
I think this is worth a comment. I presume you're trying to get the low 32 bits of the address here (and assuming that the high 32 bits are zero).
Cheers ---Dave
- }
- return;
+}
static int is_space(char c) { return c == ' '; @@ -598,4 +646,6 @@ args_done: atag_append(&atagp, ATAG_NONE, 0, 0); update_fdt(&phys, info);
- configure_from_fdt(info);
}
1.7.10.4
On Mon, 2012-10-01 at 17:33 +0100, Dave Martin wrote:
On Thu, Sep 27, 2012 at 07:06:20PM +0100, Jon Medhurst (Tixy) wrote:
- if (offset >= 0) {
p = fdt_getprop(fdt, offset, "reg", &len);
if(len != (addrcells + sizecells) * 4)
info("Failed parsing device-tree node for CCI\n");
else
init_cci(fdt32_to_cpu(p[addrcells - 1]));
I think this is worth a comment. I presume you're trying to get the low 32 bits of the address here (and assuming that the high 32 bits are zero).
That's correct. I've been assuming this bootwrapper is only good for 32-bit systems anyway, or am I mistake?
I'll add an appropriate comment either way.
Thanks for reviewing these patches.
On Mon, Oct 01, 2012 at 06:03:54PM +0100, Jon Medhurst (Tixy) wrote:
On Mon, 2012-10-01 at 17:33 +0100, Dave Martin wrote:
On Thu, Sep 27, 2012 at 07:06:20PM +0100, Jon Medhurst (Tixy) wrote:
- if (offset >= 0) {
p = fdt_getprop(fdt, offset, "reg", &len);
if(len != (addrcells + sizecells) * 4)
info("Failed parsing device-tree node for CCI\n");
else
init_cci(fdt32_to_cpu(p[addrcells - 1]));
I think this is worth a comment. I presume you're trying to get the low 32 bits of the address here (and assuming that the high 32 bits are zero).
That's correct. I've been assuming this bootwrapper is only good for 32-bit systems anyway, or am I mistake?
Yes sure. Unless we turn the MMU on, we can't access any peripherals beyond 4GB, so this limit is currently inherent all over the place.
My comment was just due to the fact that I ended up scratching my head for a bit figuring out what the [addrcells - 1] was actually doing.
I'll add an appropriate comment either way.
Thanks -- I don't think is needs more than a 1-liner.
Cheers ---Dave
On 1 October 2012 19:33, Dave Martin dave.martin@linaro.org wrote:
On Thu, Sep 27, 2012 at 07:06:20PM +0100, Jon Medhurst (Tixy) wrote:
/* Turn on CCI snoops and DVM messages */
io32(cci+0x4000) = 0x3; /* A15 cluster */
io32(cci+0x5000) = 0x3; /* A7 cluster */
Ultimately, CCI slave port assignments will be deducible from the DT topology, but we don't have this yet, so I guess this is OK for now.
For the intergrated switcher work we can have a simple local patch for this until/unless this (or the switcher) becomes more generic.
Would the local patch be for the bootwrapper or the integrated switcher kernel? The latter would be nice, as it would allow "one bootwrapper for everything". With the former we could still have an ifdef and build both bootwrappers from same source.
Riku
/* Wait while change pending bit of status register is set */
while(io32(cci+0xc) & 0x1)
{}
+}
+static void configure_from_fdt(struct loader_info *info) +{
void *fdt = (void *)info->fdt_start;
uint32_t const *p;
int addrcells, sizecells;
int offset, len;
if(!fdt)
return;
_fdt_address_and_size_cells(fdt, &addrcells, &sizecells);
/* See if there is a CCI device to initialise */
offset = fdt_node_offset_by_compatible(fdt, 0, "arm,cci");
I'm guessing this binding isn't official yet, but it'll do for now...
if (offset >= 0) {
p = fdt_getprop(fdt, offset, "reg", &len);
if(len != (addrcells + sizecells) * 4)
info("Failed parsing device-tree node for CCI\n");
else
init_cci(fdt32_to_cpu(p[addrcells - 1]));
I think this is worth a comment. I presume you're trying to get the low 32 bits of the address here (and assuming that the high 32 bits are zero).
Cheers ---Dave
}
return;
+}
static int is_space(char c) { return c == ' '; @@ -598,4 +646,6 @@ args_done: atag_append(&atagp, ATAG_NONE, 0, 0);
update_fdt(&phys, info);
configure_from_fdt(info);
}
1.7.10.4
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
On Tue, Oct 02, 2012 at 12:30:54PM +0300, Riku Voipio wrote:
On 1 October 2012 19:33, Dave Martin dave.martin@linaro.org wrote:
On Thu, Sep 27, 2012 at 07:06:20PM +0100, Jon Medhurst (Tixy) wrote:
/* Turn on CCI snoops and DVM messages */
io32(cci+0x4000) = 0x3; /* A15 cluster */
io32(cci+0x5000) = 0x3; /* A7 cluster */
Ultimately, CCI slave port assignments will be deducible from the DT topology, but we don't have this yet, so I guess this is OK for now.
For the intergrated switcher work we can have a simple local patch for this until/unless this (or the switcher) becomes more generic.
Would the local patch be for the bootwrapper or the integrated switcher kernel? The latter would be nice, as it would allow "one bootwrapper for everything". With the former we could still have an ifdef and build both bootwrappers from same source.
I think we are actually now not far away from the main bootwrapper being usable for IKS.
We need to be able to boot Linux in the Secure world (that can be a generic command-line option), and we need the initial CCI configuration to match the way the model was started (i.e., which CPUs were held in reset initially). The latter may be a bit of a hack, for now.
If the "local patch" can be put in a sufficiently non-invasive form, I don't see why we couldn't merge it and drop the IKS bootwrapper. I haven't had a lot of time to play with this though...
Cheers ---Dave
/* Wait while change pending bit of status register is set */
while(io32(cci+0xc) & 0x1)
{}
+}
+static void configure_from_fdt(struct loader_info *info) +{
void *fdt = (void *)info->fdt_start;
uint32_t const *p;
int addrcells, sizecells;
int offset, len;
if(!fdt)
return;
_fdt_address_and_size_cells(fdt, &addrcells, &sizecells);
/* See if there is a CCI device to initialise */
offset = fdt_node_offset_by_compatible(fdt, 0, "arm,cci");
I'm guessing this binding isn't official yet, but it'll do for now...
if (offset >= 0) {
p = fdt_getprop(fdt, offset, "reg", &len);
if(len != (addrcells + sizecells) * 4)
info("Failed parsing device-tree node for CCI\n");
else
init_cci(fdt32_to_cpu(p[addrcells - 1]));
I think this is worth a comment. I presume you're trying to get the low 32 bits of the address here (and assuming that the high 32 bits are zero).
Cheers ---Dave
}
return;
+}
static int is_space(char c) { return c == ' '; @@ -598,4 +646,6 @@ args_done: atag_append(&atagp, ATAG_NONE, 0, 0);
update_fdt(&phys, info);
configure_from_fdt(info);
}
1.7.10.4
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev