On 07/06/17 13:13, Jan Beulich wrote:
On 07.06.17 at 14:04, <punit.agra...@arm.com> wrote:
Commit 726b737574 makes an unrelated change deleting a line setting the
page from mfn. Although the page variable is not used, it is an
unrelated change. The setting of the page variable was introduced to
match the else part of is_domain_direct_mapped() in populate_physmap().

Re-introduce the missing hunk.

Fixes: 726b737574 ("Avoid excess icache flushes in populate_physmap() before
domain has been created")
Signed-off-by: Punit Agrawal <punit.agra...@arm.com>
---
 xen/common/memory.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 34d2dda8b4..a3cb572530 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -221,6 +221,7 @@ static void populate_physmap(struct memop_args *a)
                 }

                 mfn = gpfn;
+                page = mfn_to_page(mfn);
             }
             else
             {

While I certainly don't mind this being re-added, I'm also not sure
it's worthwhile now that the line is gone, and it's not needed for
anything. I'll let other REST maintainers give their opinions ...

I am not a REST maintainers but I think it would be better to keep the page = mfn_to_page(mfn). This is matching the behavior of the else part where page will always point to first base page.

Indeed we don't use it today, but nothing prevent a future patch to do it and it would be difficult to spot the mismatch...

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Reply via email to