Hi Olof, thanks for the review. 

quoted

+	if (have_vrma)
+		pSeries_find_hpte_vrma();
+
    

This will break kexec builds on non-pseries. It's referring to platform
code that might not be built.

  

Ok. Will call this function from lpar.c instead of
default_machine_kexec().

Does this function find the vrma, or save it away? Seems like the name
is misleading.

  

Well it finds a vrma entry and saves it. I thought of
pSeries_find_save_hpte_vrma(), but decided against it. I could
change it to pSeries_save_hpte_vrma().

Is ppc64_vrma_page_size really the size, or the shift? Above would
indicate that it's really a shift value.

  

It is a shift. I will change it to ppc64_vrma_page_shift.

Why is 16M hardcoded here, when you're taking such great care to read
out the pagesize earlier?

  

Hrmm. Ok will use the vrma_page_shift value.

quoted

+			((dword0 & HPTE_V_MASK) == MAGIC_SKIP_HPTE)) {
    

Indentation
  

Done.

quoted

+				/* store the hpte */
+				hpte_vrma_slots[num_hpte_vrma_slots++] = slot;
    

Here you rely on global exported state (num_hpte_vrma_slots), increasing it without
checking for limits. What happens if this function is ever called twice? Should you
set it to 0 in the beginning of the function and check it against the size of the
hpte_vrma_slots array instead?
  

Will add proper checks for num_hpte_vrma_slots variable value.

quoted

+extern void pSeries_find_hpte_vrma(void);

    

Same comment as above: This isn't a kexec function as much as a pseries function, so
it should be defined in some other header instead.
  

quoted

 
+#define HPTE_V_RMA_VPN         ASM_CONST(0x001FFFFFF0000000)
+#define HPTE_V_MASK            ASM_CONST(0xc000000000000000)
+#define MAGIC_SKIP_HPTE        ASM_CONST(0x4000000000000000)
+#define HPTE_V_RMA_NUM         16
    

"MAGIC_SKIP_HPTE"? I'm sure there's a proper name for this field in the
PAPR, isn't there? Also, HPTE_V_RMA_NUM isn't a HPTE_V field, it shouldn't
have that prefix. It's not a property of the mmu in the first place.

These should maybe be local defines in the pseries lpar code instead, since it's
more of a lpar<->phyp interface than mmu programming interface.
  

Will move them to pseries lpar code.
Updated patch on its way.

Thanks
-Sachin