Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Daniel Kiper]

> Hmm... this code is being redone at the moment... this might conflict.

Is this available somewhere? May I have a look at it?

Daniel

PS I am on holiday until 02/01/2013 and I could not
   have access to my email box. Please be patient.
   At worst case I will send reply when I will be
   back at office.

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Borislav Petkov]

On Thu, Dec 27, 2012 at 03:19:24PM -0800, Daniel Kiper wrote:
> > Hmm... this code is being redone at the moment... this might conflict.
> 
> Is this available somewhere? May I have a look at it?

http://marc.info/?l=linux-kernel&m=135581534620383

The for-x86-boot-v7 and -v8 branches.

HTH.

-- 
Regards/Gruss,
Boris.

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Daniel Kiper]

On Fri, Dec 28, 2012 at 01:59:27PM +0100, Borislav Petkov wrote:
> On Thu, Dec 27, 2012 at 03:19:24PM -0800, Daniel Kiper wrote:
> > > Hmm... this code is being redone at the moment... this might conflict.
> >
> > Is this available somewhere? May I have a look at it?
>
> http://marc.info/?l=linux-kernel&m=135581534620383
>
> The for-x86-boot-v7 and -v8 branches.
>
> HTH.

Thanks.

Daniel

[PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Daniel Kiper]

Some implementations (e.g. Xen PVOPS) could not use part of identity page table
to construct transition page table. It means that they require separate PUDs,
PMDs and PTEs for virtual and physical (identity) mapping. To satisfy that
requirement add extra pointer to PGD, PUD, PMD and PTE and align existing code.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 arch/x86/include/asm/kexec.h       |   10 +++++++---
 arch/x86/kernel/machine_kexec_64.c |   12 ++++++------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index 6080d26..cedd204 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -157,9 +157,13 @@ struct kimage_arch {
 };
 #else
 struct kimage_arch {
-	pud_t *pud;
-	pmd_t *pmd;
-	pte_t *pte;
+	pgd_t *pgd;
+	pud_t *pud0;
+	pud_t *pud1;
+	pmd_t *pmd0;
+	pmd_t *pmd1;
+	pte_t *pte0;
+	pte_t *pte1;
 };
 #endif
 
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index b3ea9db..976e54b 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -137,9 +137,9 @@ out:
 
 static void free_transition_pgtable(struct kimage *image)
 {
-	free_page((unsigned long)image->arch.pud);
-	free_page((unsigned long)image->arch.pmd);
-	free_page((unsigned long)image->arch.pte);
+	free_page((unsigned long)image->arch.pud0);
+	free_page((unsigned long)image->arch.pmd0);
+	free_page((unsigned long)image->arch.pte0);
 }
 
 static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
@@ -157,7 +157,7 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
 		pud = (pud_t *)get_zeroed_page(GFP_KERNEL);
 		if (!pud)
 			goto err;
-		image->arch.pud = pud;
+		image->arch.pud0 = pud;
 		set_pgd(pgd, __pgd(__pa(pud) | _KERNPG_TABLE));
 	}
 	pud = pud_offset(pgd, vaddr);
@@ -165,7 +165,7 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
 		pmd = (pmd_t *)get_zeroed_page(GFP_KERNEL);
 		if (!pmd)
 			goto err;
-		image->arch.pmd = pmd;
+		image->arch.pmd0 = pmd;
 		set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
 	}
 	pmd = pmd_offset(pud, vaddr);
@@ -173,7 +173,7 @@ static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
 		pte = (pte_t *)get_zeroed_page(GFP_KERNEL);
 		if (!pte)
 			goto err;
-		image->arch.pte = pte;
+		image->arch.pte0 = pte;
 		set_pmd(pmd, __pmd(__pa(pte) | _KERNPG_TABLE));
 	}
 	pte = pte_offset_kernel(pmd, vaddr);
-- 
1.5.6.5

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[H. Peter Anvin]

Hmm... this code is being redone at the moment... this might conflict.

Daniel Kiper <daniel.kiper@oracle.com> wrote:

>Some implementations (e.g. Xen PVOPS) could not use part of identity
>page table
>to construct transition page table. It means that they require separate
>PUDs,
>PMDs and PTEs for virtual and physical (identity) mapping. To satisfy
>that
>requirement add extra pointer to PGD, PUD, PMD and PTE and align
>existing code.
>
>Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>---
> arch/x86/include/asm/kexec.h       |   10 +++++++---
> arch/x86/kernel/machine_kexec_64.c |   12 ++++++------
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
>diff --git a/arch/x86/include/asm/kexec.h
>b/arch/x86/include/asm/kexec.h
>index 6080d26..cedd204 100644
>--- a/arch/x86/include/asm/kexec.h
>+++ b/arch/x86/include/asm/kexec.h
>@@ -157,9 +157,13 @@ struct kimage_arch {
> };
> #else
> struct kimage_arch {
>-	pud_t *pud;
>-	pmd_t *pmd;
>-	pte_t *pte;
>+	pgd_t *pgd;
>+	pud_t *pud0;
>+	pud_t *pud1;
>+	pmd_t *pmd0;
>+	pmd_t *pmd1;
>+	pte_t *pte0;
>+	pte_t *pte1;
> };
> #endif
> 
>diff --git a/arch/x86/kernel/machine_kexec_64.c
>b/arch/x86/kernel/machine_kexec_64.c
>index b3ea9db..976e54b 100644
>--- a/arch/x86/kernel/machine_kexec_64.c
>+++ b/arch/x86/kernel/machine_kexec_64.c
>@@ -137,9 +137,9 @@ out:
> 
> static void free_transition_pgtable(struct kimage *image)
> {
>-	free_page((unsigned long)image->arch.pud);
>-	free_page((unsigned long)image->arch.pmd);
>-	free_page((unsigned long)image->arch.pte);
>+	free_page((unsigned long)image->arch.pud0);
>+	free_page((unsigned long)image->arch.pmd0);
>+	free_page((unsigned long)image->arch.pte0);
> }
> 
> static int init_transition_pgtable(struct kimage *image, pgd_t *pgd)
>@@ -157,7 +157,7 @@ static int init_transition_pgtable(struct kimage
>*image, pgd_t *pgd)
> 		pud = (pud_t *)get_zeroed_page(GFP_KERNEL);
> 		if (!pud)
> 			goto err;
>-		image->arch.pud = pud;
>+		image->arch.pud0 = pud;
> 		set_pgd(pgd, __pgd(__pa(pud) | _KERNPG_TABLE));
> 	}
> 	pud = pud_offset(pgd, vaddr);
>@@ -165,7 +165,7 @@ static int init_transition_pgtable(struct kimage
>*image, pgd_t *pgd)
> 		pmd = (pmd_t *)get_zeroed_page(GFP_KERNEL);
> 		if (!pmd)
> 			goto err;
>-		image->arch.pmd = pmd;
>+		image->arch.pmd0 = pmd;
> 		set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
> 	}
> 	pmd = pmd_offset(pud, vaddr);
>@@ -173,7 +173,7 @@ static int init_transition_pgtable(struct kimage
>*image, pgd_t *pgd)
> 		pte = (pte_t *)get_zeroed_page(GFP_KERNEL);
> 		if (!pte)
> 			goto err;
>-		image->arch.pte = pte;
>+		image->arch.pte0 = pte;
> 		set_pmd(pmd, __pmd(__pa(pte) | _KERNPG_TABLE));
> 	}
> 	pte = pte_offset_kernel(pmd, vaddr);

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Jan Beulich]

>>> On 27.12.12 at 03:18, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> Some implementations (e.g. Xen PVOPS) could not use part of identity page 
> table
> to construct transition page table. It means that they require separate 
> PUDs,
> PMDs and PTEs for virtual and physical (identity) mapping. To satisfy that
> requirement add extra pointer to PGD, PUD, PMD and PTE and align existing 
> code.

So you keep posting this despite it having got pointed out on each
earlier submission that this is unnecessary, proven by the fact that
the non-pvops Xen kernels can get away without it. Why?

Jan

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Daniel Kiper]

On Thu, Jan 03, 2013 at 09:34:55AM +0000, Jan Beulich wrote:
> >>> On 27.12.12 at 03:18, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > Some implementations (e.g. Xen PVOPS) could not use part of identity page table
> > to construct transition page table. It means that they require separate PUDs,
> > PMDs and PTEs for virtual and physical (identity) mapping. To satisfy that
> > requirement add extra pointer to PGD, PUD, PMD and PTE and align existing
> > code.
>
> So you keep posting this despite it having got pointed out on each
> earlier submission that this is unnecessary, proven by the fact that
> the non-pvops Xen kernels can get away without it. Why?

Sorry but I forgot to reply for your email last time.

I am still not convinced. I have tested SUSE kernel itself and it does not work.
Maybe I missed something but... Please check arch/x86/kernel/machine_kexec_64.c:init_transition_pgtable()

I can see:

vaddr = (unsigned long)relocate_kernel;

and later:

pgd += pgd_index(vaddr);
...

It is wrong. relocate_kernel() virtual address in Xen is different
than its virtual address in Linux Kernel. That is why transition
page table could not be established in Linux Kernel and so on...
How does this work in SUSE? I do not have an idea.

I am happy to fix that but whatever fix for it is
I would like to be sure that it works.

Daniel

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Jan Beulich]

>>> On 04.01.13 at 16:15, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> On Thu, Jan 03, 2013 at 09:34:55AM +0000, Jan Beulich wrote:
>> >>> On 27.12.12 at 03:18, Daniel Kiper <daniel.kiper@oracle.com> wrote:
>> > Some implementations (e.g. Xen PVOPS) could not use part of identity page 
> table
>> > to construct transition page table. It means that they require separate 
> PUDs,
>> > PMDs and PTEs for virtual and physical (identity) mapping. To satisfy that
>> > requirement add extra pointer to PGD, PUD, PMD and PTE and align existing
>> > code.
>>
>> So you keep posting this despite it having got pointed out on each
>> earlier submission that this is unnecessary, proven by the fact that
>> the non-pvops Xen kernels can get away without it. Why?
> 
> Sorry but I forgot to reply for your email last time.
> 
> I am still not convinced. I have tested SUSE kernel itself and it does not 
> work.
> Maybe I missed something but... Please check 
> arch/x86/kernel/machine_kexec_64.c:init_transition_pgtable()
> 
> I can see:
> 
> vaddr = (unsigned long)relocate_kernel;
> 
> and later:
> 
> pgd += pgd_index(vaddr);
> ...

I think that mapping is simply irrelevant, as the code at
relocate_kernel gets copied to the control page and
invoked there (other than in the native case, where
relocate_kernel() gets invoked directly).

Jan

> It is wrong. relocate_kernel() virtual address in Xen is different
> than its virtual address in Linux Kernel. That is why transition
> page table could not be established in Linux Kernel and so on...
> How does this work in SUSE? I do not have an idea.
> 
> I am happy to fix that but whatever fix for it is
> I would like to be sure that it works.
> 
> Daniel

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Daniel Kiper]

On Fri, Jan 04, 2013 at 04:12:32PM +0000, Jan Beulich wrote:
> >>> On 04.01.13 at 16:15, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > On Thu, Jan 03, 2013 at 09:34:55AM +0000, Jan Beulich wrote:
> >> >>> On 27.12.12 at 03:18, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> >> > Some implementations (e.g. Xen PVOPS) could not use part of identity page table
> >> > to construct transition page table. It means that they require separate PUDs,
> >> > PMDs and PTEs for virtual and physical (identity) mapping. To satisfy that
> >> > requirement add extra pointer to PGD, PUD, PMD and PTE and align existing
> >> > code.
> >>
> >> So you keep posting this despite it having got pointed out on each
> >> earlier submission that this is unnecessary, proven by the fact that
> >> the non-pvops Xen kernels can get away without it. Why?
> >
> > Sorry but I forgot to reply for your email last time.
> >
> > I am still not convinced. I have tested SUSE kernel itself and it does not work.
> > Maybe I missed something but... Please check
> > arch/x86/kernel/machine_kexec_64.c:init_transition_pgtable()
> >
> > I can see:
> >
> > vaddr = (unsigned long)relocate_kernel;
> >
> > and later:
> >
> > pgd += pgd_index(vaddr);
> > ...
>
> I think that mapping is simply irrelevant, as the code at
> relocate_kernel gets copied to the control page and
> invoked there (other than in the native case, where
> relocate_kernel() gets invoked directly).

Right, so where is virtual mapping of control page established?
I could not find relevant code in SLES kernel which does that.

Daniel

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Jan Beulich]

>>> On 04.01.13 at 18:25, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> Right, so where is virtual mapping of control page established?
> I could not find relevant code in SLES kernel which does that.

In the hypervisor (xen/arch/x86/machine_kexec.c:machine_kexec_load()).
xen/arch/x86/machine_kexec.c:machine_kexec() then simply uses
image->page_list[1].

Jan

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Daniel Kiper]

On Mon, Jan 07, 2013 at 09:48:20AM +0000, Jan Beulich wrote:
> >>> On 04.01.13 at 18:25, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > Right, so where is virtual mapping of control page established?
> > I could not find relevant code in SLES kernel which does that.
>
> In the hypervisor (xen/arch/x86/machine_kexec.c:machine_kexec_load()).
> xen/arch/x86/machine_kexec.c:machine_kexec() then simply uses
> image->page_list[1].

This (xen/arch/x86/machine_kexec.c:machine_kexec_load()) maps relevant
page (allocated earlier by dom0) in hypervisor fixmap area. However,
it does not make relevant mapping in transition page table which
leads to crash when %cr3 is switched from Xen page table to
transition page table.

Daniel

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Jan Beulich]

>>> On 07.01.13 at 13:52, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> On Mon, Jan 07, 2013 at 09:48:20AM +0000, Jan Beulich wrote:
>> >>> On 04.01.13 at 18:25, Daniel Kiper <daniel.kiper@oracle.com> wrote:
>> > Right, so where is virtual mapping of control page established?
>> > I could not find relevant code in SLES kernel which does that.
>>
>> In the hypervisor (xen/arch/x86/machine_kexec.c:machine_kexec_load()).
>> xen/arch/x86/machine_kexec.c:machine_kexec() then simply uses
>> image->page_list[1].
> 
> This (xen/arch/x86/machine_kexec.c:machine_kexec_load()) maps relevant
> page (allocated earlier by dom0) in hypervisor fixmap area. However,
> it does not make relevant mapping in transition page table which
> leads to crash when %cr3 is switched from Xen page table to
> transition page table.

That indeed could explain _random_ failures - the fixmap entries
get created with _PAGE_GLOBAL set, i.e. don't get flushed with
the CR3 write unless CR4.PGE is clear.

And I don't see how your allocation of intermediate page tables
would help: You wouldn't know where the mapping of the control
page lives until you're actually in the early relocate_kernel code.
Or was it that what distinguishes your cloned code from the
native original?

Jan

Re: [PATCH v3 02/11] x86/kexec: Add extra pointers to transition page table PGD, PUD, PMD and PTE

[Daniel Kiper]

On Mon, Jan 07, 2013 at 01:05:10PM +0000, Jan Beulich wrote:
> >>> On 07.01.13 at 13:52, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> > On Mon, Jan 07, 2013 at 09:48:20AM +0000, Jan Beulich wrote:
> >> >>> On 04.01.13 at 18:25, Daniel Kiper <daniel.kiper@oracle.com> wrote:
> >> > Right, so where is virtual mapping of control page established?
> >> > I could not find relevant code in SLES kernel which does that.
> >>
> >> In the hypervisor (xen/arch/x86/machine_kexec.c:machine_kexec_load()).
> >> xen/arch/x86/machine_kexec.c:machine_kexec() then simply uses
> >> image->page_list[1].
> >
> > This (xen/arch/x86/machine_kexec.c:machine_kexec_load()) maps relevant
> > page (allocated earlier by dom0) in hypervisor fixmap area. However,
> > it does not make relevant mapping in transition page table which
> > leads to crash when %cr3 is switched from Xen page table to
> > transition page table.
>
> That indeed could explain _random_ failures - the fixmap entries
> get created with _PAGE_GLOBAL set, i.e. don't get flushed with
> the CR3 write unless CR4.PGE is clear.

This does not matter. As I stated earlier virtual mapping is wrong.
relocate_kernel() is mapped at its virtual address in Linux kernel
instead of control page at its virtual address in Xen hypervisor.
I tested SLES kernel once again. It does not work.

> And I don't see how your allocation of intermediate page tables
> would help: You wouldn't know where the mapping of the control
> page lives until you're actually in the early relocate_kernel code.

Right. Allocation itself is not a solution for this problem.
It should be acompanied by code which establishes transition
page table in relocate_kernel() (which is later copied
to control page, i.e. code of relocate_kernel()).

> Or was it that what distinguishes your cloned code from the
> native original?

No, my code is based on native original.
There are some implementation differences.

Daniel