| Message ID | 1412088716-32557-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, 2014-09-30 at 15:51 +0100, Julien Grall wrote: > {~0,} only initializes the first cell of the array to ~0. The other cells > are initialized to 0. > > Change the initialization to a loop and, at the same time, do the same > for the mappings. I'm not sure a loop is the best option here vs either { ~0, ~0, ~0, ~0 } or a memset, but OK. I don't think mappings is wrong as it is though, any reason to change? > This is fixing boot after 82985d7 "xen: arm: handle variable p2m levels > in apply_p2m_changes" on platform where the root-level doesn't have > concatenate table (such as the Foundation Model). You forgot your S-o-b.
On 10/01/2014 11:33 AM, Ian Campbell wrote: > On Tue, 2014-09-30 at 15:51 +0100, Julien Grall wrote: >> {~0,} only initializes the first cell of the array to ~0. The other cells >> are initialized to 0. >> >> Change the initialization to a loop and, at the same time, do the same >> for the mappings. > > I'm not sure a loop is the best option here vs either { ~0, ~0, ~0, ~0 } > or a memset, but OK. Thinking a bit more, we effectively need to initialize only the first 2 cells. It's because the P2M can start either at level 0 or level 1. Anyway I will use { ~0, ~0, ~0, ~0 }. > I don't think mappings is wrong as it is though, any reason to change? { NULL, } initialize the first cell to NULL and the other to 0. As NULL is equal to (void *)0 it's fine, but I would prefer if we can get a rid of this construction. I would prefer if we do { NULL, NULL, NULL, NULL }. >> This is fixing boot after 82985d7 "xen: arm: handle variable p2m levels >> in apply_p2m_changes" on platform where the root-level doesn't have >> concatenate table (such as the Foundation Model). > > You forgot your S-o-b. Oh right. I will resend a patch
On Wed, 2014-10-01 at 16:00 +0100, Julien Grall wrote: > On 10/01/2014 11:33 AM, Ian Campbell wrote: > > On Tue, 2014-09-30 at 15:51 +0100, Julien Grall wrote: > >> {~0,} only initializes the first cell of the array to ~0. The other cells > >> are initialized to 0. > >> > >> Change the initialization to a loop and, at the same time, do the same > >> for the mappings. > > > > I'm not sure a loop is the best option here vs either { ~0, ~0, ~0, ~0 } > > or a memset, but OK. > > Thinking a bit more, we effectively need to initialize only the first 2 > cells. It's because the P2M can start either at level 0 or level 1. > > Anyway I will use { ~0, ~0, ~0, ~0 }. > > > I don't think mappings is wrong as it is though, any reason to change? > > { NULL, } initialize the first cell to NULL and the other to 0. > > As NULL is equal to (void *)0 it's fine, but I would prefer if we can > get a rid of this construction. > > I would prefer if we do { NULL, NULL, NULL, NULL }. OK. > > >> This is fixing boot after 82985d7 "xen: arm: handle variable p2m levels > >> in apply_p2m_changes" on platform where the root-level doesn't have > >> concatenate table (such as the Foundation Model). > > > > You forgot your S-o-b. > > Oh right. I will resend a patch > > >
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index 70929fc..3fa7e63 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -716,15 +716,21 @@ static int apply_p2m_changes(struct domain *d, { int rc, ret; struct p2m_domain *p2m = &d->arch.p2m; - lpae_t *mappings[4] = { NULL, }; + lpae_t *mappings[4]; paddr_t addr, orig_maddr = maddr; unsigned int level = 0; unsigned int cur_root_table = ~0; - unsigned int cur_offset[4] = { ~0, }; + unsigned int cur_offset[4]; unsigned int count = 0; bool_t flush = false; bool_t flush_pt; + for (level = 0; level < 4; level++) + { + mappings[level] = NULL; + cur_offset[level] = ~0; + } + /* Some IOMMU don't support coherent PT walk. When the p2m is * shared with the CPU, Xen has to make sure that the PT changes have * reached the memory
{~0,} only initializes the first cell of the array to ~0. The other cells are initialized to 0. Change the initialization to a loop and, at the same time, do the same for the mappings. This is fixing boot after 82985d7 "xen: arm: handle variable p2m levels in apply_p2m_changes" on platform where the root-level doesn't have concatenate table (such as the Foundation Model). --- This is a bug fix for Xen 4.5. This make Xen booting again on the Foundation Model and any platform that doesn't have concatenate table for root-level. --- xen/arch/arm/p2m.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)