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

Re: [Xen-devel] [PATCH] expand x86 arch_shared_info to support >3 level p2m tree

On 09/09/2014 11:36 AM, Andrew Cooper wrote:
On 09/09/14 10:28, Jan Beulich wrote:
On 08.09.14 at 18:11, <JGross@xxxxxxxx> wrote:
On 09/08/2014 03:59 PM, Andrew Cooper wrote:
On 08/09/14 14:48, Juergen Gross wrote:
The x86 struct arch_shared_info field pfn_to_mfn_frame_list_list
currently contains the mfn of the top level page frame of the 3 level
p2m tree, which is used by the Xen tools during saving and restoring
(and live migration) of pv domains. With three levels of the p2m tree
it is possible to support up to 512 GB of RAM for a pv domain.
Specifically only 64bit PV domains have the 512GB limit.

32bit PV domains have a far larger supported RAM as they can fit twice
as many mfns in each p2m page.

   To be
able to support more RAM an additional level is to be added.

This patch expands struct arch_shared_info with a new p2m tree root
and the number of levels of the p2m tree. The new information is
indicated by the domain to be valid by storing ~0UL into
pfn_to_mfn_frame_list_list (this should be done only if more than
three levels are needed, of course).

Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
   xen/include/public/arch-x86/xen.h | 8 +++++++-
   1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/xen/include/public/arch-x86/xen.h
index f35804b..b7fa2b6 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -224,7 +224,13 @@ struct arch_shared_info {
       /* Frame containing list of mfns containing list of mfns containing
p2m. */
       xen_pfn_t     pfn_to_mfn_frame_list_list;
       unsigned long nmi_reason;
-    uint64_t pad[32];
+    /*
+     * Following two fields are valid if pfn_to_mfn_frame_list_list
+     * ~0UL.
+     */
+    unsigned long p2m_levels;   /* number of levels of p2m tree */
+    xen_pfn_t     p2m_root;     /* p2m tree top level mfn */
+    uint64_t pad[30];
This padding is now wrong, as unsigned long is only 4 bytes in 32bit,
and it is adjacent to another unsigned long.
The padding in this case is a nightmare.

For 32 bits arch_shared_info_t is 64 bit aligned due to uint64_t pad[].
No - uint64_t is 4-byte aligned on 32-bit.

This enforces a 4 byte hole before the structure (there are 3 4-byte
fields before it in shared_info_t). And before pad[] there is another
4-byte hole.

And don't forget: on 32 bits xen_pfn_t is 4 bytes, too. I could either
align each new variable explicitly to 8 bytes or I could use a union.

As arch_shared_info_t is at the end of shared_info_t I could just ignore
the alignment/padding ...

What is your preference?
As I view it, the exact padding size here doesn't really matter: The
shared info lives in a separate page anyway.


Which begs the question why the padding exists in the first place?

arch_shared_info is the final field in shared_info, and shared_info is
explicitly documented not to necessarily have a consistent size across
different versions of Xen.

Frankly, I think the padding can just be removed.  It serves no purpose,
and attempting to maintain it correctly is proving very difficult.

Okay. I'll remove the padding.


Xen-devel mailing list



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