[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 7/8] xen: arm: use superpages in p2m when pages are suitably aligned

On 12/06/14 08:30, Ian Campbell wrote:
On Wed, 2014-06-11 at 23:19 +0100, Julien Grall wrote:
Hi Ian,

While I was looking closer to this patch I found something strange. Why
all the callers of guest_physmap_add_page in the directory common don't
check that the function success to create the mapping?

"directory common"? I don't get your meaning.

Sorry, I meant xen/common/

+                return P2M_ONE_PROGRESS;
+            }
+            else if ( level == 3 )
+                return -ENOMEM;
+        }
+        BUG_ON(level == 3); /* L3 is always superpage aligned */

Did you mean page-aligned?

What I meant was that an L3 entry is always "superpage aligned" because
it is the smallest possible element. Since I wrote that I renamed my
is_superpage_aligned function to is_mapping_aligned. I should perhaps
update this comment to reflect that, which would make it clearer.

Oh ok. In my mind, L3 is using page alignment not superpage alignment. I think was confuse because in your cover letter you define superpage as 2M or 1G mappings.


+    case INSERT:
+        if ( is_mapping_aligned(*addr, end_gpaddr, *maddr, level_size) &&
+           /* We do not handle replacing an existing table with a superpage */
+             (level == 3 || !p2m_table(orig_pte)) )
+        {
+            /* New mapping is superpage aligned, make it */
+            pte = mfn_to_p2m_entry(*maddr >> PAGE_SHIFT, mattr, t);
+            if ( level < 3 )

It's funny, sometimes you use level < 3 and some level != 3 (see in


I think this pte.p2m.table set can be handled directly in
mfn_to_p2m_entry. This will avoid duplicating code.

It can't because mfn_to_p2m_entry is used to create both table and
mapping style entries.

There is only on call where we don't override the pte.p2m.table bit (the one at the end of p2m_create table).

I would move this extra test in the mfn_to_p2m_entry and override only for this specific case.


+        }
+        else
+        {
+            /* New mapping is not superpage aligned, create a new table entry 
+            BUG_ON(level == 3); /* L3 is always superpage aligned */

Did you mean page-aligned?


-        /* Got the next page */
-        addr += PAGE_SIZE;
+        rc = apply_one_level(d, &third[third_table_offset(addr)],
+                             3, flush_pt, op,
+                             start_gpaddr, end_gpaddr,
+                             &addr, &maddr, &flush,
+                             mattr, t);
+        if ( rc < 0 ) goto out;

Shall we redo the whole range if the mapping has failed here?


Undo sorry.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.