[PATCH 0/2] makedumpfile needs to remove the memory encryption

[PATCH 0/2] makedumpfile needs to remove the memory encryption

[Lianbo Jiang]

The patchset did two things:
[1] add a new variable 'sme_mask' to number_table

The variable will be used to store the sme mask for crashed kernel,
the sme_mask denotes whether the old memory is encrypted or not.

[2] remove the memory encryption mask to obtain the true physical
address

For AMD machine with SME feature, if SME is enabled in the first
kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
the memory encryption mask, so makedumpfile needs to remove the
memory encryption mask to obtain the true physical address.

References:

x86/kdump: Export the SME mask to vmcoreinfo
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=65f750e5457aef9a8085a99d613fea0430303e93
https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=f263245a0ce2c4e23b89a58fa5f7dfc048e11929

Lianbo Jiang (2):
  Makedumpfile: add a new variable 'sme_mask' to number_table
  Remove the memory encryption mask to obtain the true physical address

 arch/x86_64.c  | 3 +++
 makedumpfile.c | 4 ++++
 makedumpfile.h | 1 +
 3 files changed, 8 insertions(+)

-- 
2.17.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table

[Lianbo Jiang]

It will be used to store the sme mask for crashed kernel, the
sme_mask denotes whether the old memory is encrypted or not.

Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
---
 makedumpfile.c | 3 +++
 makedumpfile.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/makedumpfile.c b/makedumpfile.c
index 8923538..a03aaa1 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1743,6 +1743,7 @@ get_structure_info(void)
 	ENUM_NUMBER_INIT(NR_FREE_PAGES, "NR_FREE_PAGES");
 	ENUM_NUMBER_INIT(N_ONLINE, "N_ONLINE");
 	ENUM_NUMBER_INIT(pgtable_l5_enabled, "pgtable_l5_enabled");
+	ENUM_NUMBER_INIT(sme_mask, "sme_mask");
 
 	ENUM_NUMBER_INIT(PG_lru, "PG_lru");
 	ENUM_NUMBER_INIT(PG_private, "PG_private");
@@ -2276,6 +2277,7 @@ write_vmcoreinfo_data(void)
 	WRITE_NUMBER("NR_FREE_PAGES", NR_FREE_PAGES);
 	WRITE_NUMBER("N_ONLINE", N_ONLINE);
 	WRITE_NUMBER("pgtable_l5_enabled", pgtable_l5_enabled);
+	WRITE_NUMBER("sme_mask", sme_mask);
 
 	WRITE_NUMBER("PG_lru", PG_lru);
 	WRITE_NUMBER("PG_private", PG_private);
@@ -2672,6 +2674,7 @@ read_vmcoreinfo(void)
 	READ_NUMBER("NR_FREE_PAGES", NR_FREE_PAGES);
 	READ_NUMBER("N_ONLINE", N_ONLINE);
 	READ_NUMBER("pgtable_l5_enabled", pgtable_l5_enabled);
+	READ_NUMBER("sme_mask", sme_mask);
 
 	READ_NUMBER("PG_lru", PG_lru);
 	READ_NUMBER("PG_private", PG_private);
diff --git a/makedumpfile.h b/makedumpfile.h
index 73813ed..e97b2e7 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1912,6 +1912,7 @@ struct number_table {
 	long	NR_FREE_PAGES;
 	long	N_ONLINE;
 	long	pgtable_l5_enabled;
+	long	sme_mask;
 
 	/*
  	* Page flags
-- 
2.17.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

RE: [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table

[Kazuhito Hagio]

Hi Lianbo,

On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
> It will be used to store the sme mask for crashed kernel, the
> sme_mask denotes whether the old memory is encrypted or not.
> 
> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> ---
>  makedumpfile.c | 3 +++
>  makedumpfile.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 8923538..a03aaa1 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -1743,6 +1743,7 @@ get_structure_info(void)
>  	ENUM_NUMBER_INIT(NR_FREE_PAGES, "NR_FREE_PAGES");
>  	ENUM_NUMBER_INIT(N_ONLINE, "N_ONLINE");
>  	ENUM_NUMBER_INIT(pgtable_l5_enabled, "pgtable_l5_enabled");
> +	ENUM_NUMBER_INIT(sme_mask, "sme_mask");

This is useless because the sme_mask is not enum number.
Please remove it.

And, dividing this patchset into the two patches doesn't make sense
to me in this case. Could you merge them into a patch?

Thanks,
Kazu

> 
>  	ENUM_NUMBER_INIT(PG_lru, "PG_lru");
>  	ENUM_NUMBER_INIT(PG_private, "PG_private");
> @@ -2276,6 +2277,7 @@ write_vmcoreinfo_data(void)
>  	WRITE_NUMBER("NR_FREE_PAGES", NR_FREE_PAGES);
>  	WRITE_NUMBER("N_ONLINE", N_ONLINE);
>  	WRITE_NUMBER("pgtable_l5_enabled", pgtable_l5_enabled);
> +	WRITE_NUMBER("sme_mask", sme_mask);
> 
>  	WRITE_NUMBER("PG_lru", PG_lru);
>  	WRITE_NUMBER("PG_private", PG_private);
> @@ -2672,6 +2674,7 @@ read_vmcoreinfo(void)
>  	READ_NUMBER("NR_FREE_PAGES", NR_FREE_PAGES);
>  	READ_NUMBER("N_ONLINE", N_ONLINE);
>  	READ_NUMBER("pgtable_l5_enabled", pgtable_l5_enabled);
> +	READ_NUMBER("sme_mask", sme_mask);
> 
>  	READ_NUMBER("PG_lru", PG_lru);
>  	READ_NUMBER("PG_private", PG_private);
> diff --git a/makedumpfile.h b/makedumpfile.h
> index 73813ed..e97b2e7 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -1912,6 +1912,7 @@ struct number_table {
>  	long	NR_FREE_PAGES;
>  	long	N_ONLINE;
>  	long	pgtable_l5_enabled;
> +	long	sme_mask;
> 
>  	/*
>   	* Page flags
> --
> 2.17.1
> 



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table

[lijiang]

在 2019年01月24日 05:51, Kazuhito Hagio 写道:
> Hi Lianbo,
> 
> On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
>> It will be used to store the sme mask for crashed kernel, the
>> sme_mask denotes whether the old memory is encrypted or not.
>>
>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>> ---
>>  makedumpfile.c | 3 +++
>>  makedumpfile.h | 1 +
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index 8923538..a03aaa1 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -1743,6 +1743,7 @@ get_structure_info(void)
>>  	ENUM_NUMBER_INIT(NR_FREE_PAGES, "NR_FREE_PAGES");
>>  	ENUM_NUMBER_INIT(N_ONLINE, "N_ONLINE");
>>  	ENUM_NUMBER_INIT(pgtable_l5_enabled, "pgtable_l5_enabled");
>> +	ENUM_NUMBER_INIT(sme_mask, "sme_mask");
> 
> This is useless because the sme_mask is not enum number.
> Please remove it.
> Ok. Thanks for your comment.

> And, dividing this patchset into the two patches doesn't make sense
> to me in this case. Could you merge them into a patch?
> 
Sure, they will be merged into a patch in next post.

Regards,
Lianbo

> Thanks,
> Kazu
> 
>>
>>  	ENUM_NUMBER_INIT(PG_lru, "PG_lru");
>>  	ENUM_NUMBER_INIT(PG_private, "PG_private");
>> @@ -2276,6 +2277,7 @@ write_vmcoreinfo_data(void)
>>  	WRITE_NUMBER("NR_FREE_PAGES", NR_FREE_PAGES);
>>  	WRITE_NUMBER("N_ONLINE", N_ONLINE);
>>  	WRITE_NUMBER("pgtable_l5_enabled", pgtable_l5_enabled);
>> +	WRITE_NUMBER("sme_mask", sme_mask);
>>
>>  	WRITE_NUMBER("PG_lru", PG_lru);
>>  	WRITE_NUMBER("PG_private", PG_private);
>> @@ -2672,6 +2674,7 @@ read_vmcoreinfo(void)
>>  	READ_NUMBER("NR_FREE_PAGES", NR_FREE_PAGES);
>>  	READ_NUMBER("N_ONLINE", N_ONLINE);
>>  	READ_NUMBER("pgtable_l5_enabled", pgtable_l5_enabled);
>> +	READ_NUMBER("sme_mask", sme_mask);
>>
>>  	READ_NUMBER("PG_lru", PG_lru);
>>  	READ_NUMBER("PG_private", PG_private);
>> diff --git a/makedumpfile.h b/makedumpfile.h
>> index 73813ed..e97b2e7 100644
>> --- a/makedumpfile.h
>> +++ b/makedumpfile.h
>> @@ -1912,6 +1912,7 @@ struct number_table {
>>  	long	NR_FREE_PAGES;
>>  	long	N_ONLINE;
>>  	long	pgtable_l5_enabled;
>> +	long	sme_mask;
>>
>>  	/*
>>   	* Page flags
>> --
>> 2.17.1
>>
> 
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[Lianbo Jiang]

For AMD machine with SME feature, if SME is enabled in the first
kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
the memory encryption mask, so makedumpfile needs to remove the
memory encryption mask to obtain the true physical address.

Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
---
 arch/x86_64.c  | 3 +++
 makedumpfile.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index 537fb78..7651d36 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
 			return NOT_PADDR;
 		}
 		pud_paddr  = pgd & ENTRY_MASK;
+		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
 	}
 
 	/*
@@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
 	 * Get PMD.
 	 */
 	pmd_paddr  = pud_pte & ENTRY_MASK;
+	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
 	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
 	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
 		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
@@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
 	 * Get PTE.
 	 */
 	pte_paddr  = pmd_pte & ENTRY_MASK;
+	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
 	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
 	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
 		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
diff --git a/makedumpfile.c b/makedumpfile.c
index a03aaa1..81c7bb4 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -977,6 +977,7 @@ next_page:
 	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
 
 	pgaddr = PAGEBASE(paddr);
+	pgaddr = pgaddr & ~(NUMBER(sme_mask));
 	pgbuf = cache_search(pgaddr, read_size);
 	if (!pgbuf) {
 		++cache_miss;
-- 
2.17.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

RE: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[Kazuhito Hagio]

On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
> For AMD machine with SME feature, if SME is enabled in the first
> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
> the memory encryption mask, so makedumpfile needs to remove the
> memory encryption mask to obtain the true physical address.
> 
> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> ---
>  arch/x86_64.c  | 3 +++
>  makedumpfile.c | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/arch/x86_64.c b/arch/x86_64.c
> index 537fb78..7651d36 100644
> --- a/arch/x86_64.c
> +++ b/arch/x86_64.c
> @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>  			return NOT_PADDR;
>  		}
>  		pud_paddr  = pgd & ENTRY_MASK;
> +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
>  	}
> 
>  	/*
> @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>  	 * Get PMD.
>  	 */
>  	pmd_paddr  = pud_pte & ENTRY_MASK;
> +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
>  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
>  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
>  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
> @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>  	 * Get PTE.
>  	 */
>  	pte_paddr  = pmd_pte & ENTRY_MASK;
> +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
>  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
>  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
>  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
> diff --git a/makedumpfile.c b/makedumpfile.c
> index a03aaa1..81c7bb4 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -977,6 +977,7 @@ next_page:
>  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
> 
>  	pgaddr = PAGEBASE(paddr);
> +	pgaddr = pgaddr & ~(NUMBER(sme_mask));

Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
So the four lines added above need

  if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
    ...

and, what I'm wondering is whether it doesn't need to take hugepages
into account such as this

392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
394                         (vaddr & ~PMD_MASK);
"arch/x86_64.c"

Thanks,
Kazu


>  	pgbuf = cache_search(pgaddr, read_size);
>  	if (!pgbuf) {
>  		++cache_miss;
> --
> 2.17.1
> 



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

RE: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[Kazuhito Hagio]

On 1/23/2019 5:16 PM, Kazuhito Hagio wrote:
> On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
> > For AMD machine with SME feature, if SME is enabled in the first
> > kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
> > the memory encryption mask, so makedumpfile needs to remove the
> > memory encryption mask to obtain the true physical address.
> >
> > Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> > ---
> >  arch/x86_64.c  | 3 +++
> >  makedumpfile.c | 1 +
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/arch/x86_64.c b/arch/x86_64.c
> > index 537fb78..7651d36 100644
> > --- a/arch/x86_64.c
> > +++ b/arch/x86_64.c
> > @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
> >  			return NOT_PADDR;
> >  		}
> >  		pud_paddr  = pgd & ENTRY_MASK;
> > +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
> >  	}
> >
> >  	/*
> > @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
> >  	 * Get PMD.
> >  	 */
> >  	pmd_paddr  = pud_pte & ENTRY_MASK;
> > +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
> >  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
> >  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
> >  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
> > @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
> >  	 * Get PTE.
> >  	 */
> >  	pte_paddr  = pmd_pte & ENTRY_MASK;
> > +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
> >  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
> >  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
> >  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
> > diff --git a/makedumpfile.c b/makedumpfile.c
> > index a03aaa1..81c7bb4 100644
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -977,6 +977,7 @@ next_page:
> >  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
> >
> >  	pgaddr = PAGEBASE(paddr);
> > +	pgaddr = pgaddr & ~(NUMBER(sme_mask));
> 
> Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
> if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
> So the four lines added above need
> 
>   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>     ...

Considering hugepage and the code, it might be better to add
a local variable for the mask value to __vtop4_x86_64() function
and mask it without condition, for example

  unsigned long sme_mask = ~0UL;

  if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
      sme_mask = ~(NUMBER(sme_mask));
  ...
  pud_paddr = pgd & ENTRY_MASK & sme_mask;

to avoid adding lots of 'if' statements.

Thanks,
Kazu

> 
> and, what I'm wondering is whether it doesn't need to take hugepages
> into account such as this
> 
> 392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
> 393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
> 394                         (vaddr & ~PMD_MASK);
> "arch/x86_64.c"
> 
> Thanks,
> Kazu
> 
> 
> >  	pgbuf = cache_search(pgaddr, read_size);
> >  	if (!pgbuf) {
> >  		++cache_miss;
> > --
> > 2.17.1
> >
> 
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[lijiang]

在 2019年01月25日 03:33, Kazuhito Hagio 写道:
> On 1/23/2019 5:16 PM, Kazuhito Hagio wrote:
>> On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
>>> For AMD machine with SME feature, if SME is enabled in the first
>>> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
>>> the memory encryption mask, so makedumpfile needs to remove the
>>> memory encryption mask to obtain the true physical address.
>>>
>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>> ---
>>>  arch/x86_64.c  | 3 +++
>>>  makedumpfile.c | 1 +
>>>  2 files changed, 4 insertions(+)
>>>
>>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>>> index 537fb78..7651d36 100644
>>> --- a/arch/x86_64.c
>>> +++ b/arch/x86_64.c
>>> @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>  			return NOT_PADDR;
>>>  		}
>>>  		pud_paddr  = pgd & ENTRY_MASK;
>>> +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
>>>  	}
>>>
>>>  	/*
>>> @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>  	 * Get PMD.
>>>  	 */
>>>  	pmd_paddr  = pud_pte & ENTRY_MASK;
>>> +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
>>>  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
>>>  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
>>>  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
>>> @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>  	 * Get PTE.
>>>  	 */
>>>  	pte_paddr  = pmd_pte & ENTRY_MASK;
>>> +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
>>>  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
>>>  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
>>>  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
>>> diff --git a/makedumpfile.c b/makedumpfile.c
>>> index a03aaa1..81c7bb4 100644
>>> --- a/makedumpfile.c
>>> +++ b/makedumpfile.c
>>> @@ -977,6 +977,7 @@ next_page:
>>>  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
>>>
>>>  	pgaddr = PAGEBASE(paddr);
>>> +	pgaddr = pgaddr & ~(NUMBER(sme_mask));
>>
>> Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
>> if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
>> So the four lines added above need
>>
>>   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>>     ...
> 
> Considering hugepage and the code, it might be better to add
> a local variable for the mask value to __vtop4_x86_64() function
> and mask it without condition, for example
> 
>   unsigned long sme_mask = ~0UL;
> 
>   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>       sme_mask = ~(NUMBER(sme_mask));
>   ...
>   pud_paddr = pgd & ENTRY_MASK & sme_mask;
> 
> to avoid adding lots of 'if' statements.
> 

Good idea. Thank you, Kazu.

> Thanks,
> Kazu
> 
>>
>> and, what I'm wondering is whether it doesn't need to take hugepages
>> into account such as this
>>
>> 392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
>> 393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
>> 394                         (vaddr & ~PMD_MASK);
>> "arch/x86_64.c"
>>
>> Thanks,
>> Kazu
>>
>>
>>>  	pgbuf = cache_search(pgaddr, read_size);
>>>  	if (!pgbuf) {
>>>  		++cache_miss;
>>> --
>>> 2.17.1
>>>
>>
>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
> 
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[lijiang]

在 2019年01月24日 06:16, Kazuhito Hagio 写道:
> On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
>> For AMD machine with SME feature, if SME is enabled in the first
>> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
>> the memory encryption mask, so makedumpfile needs to remove the
>> memory encryption mask to obtain the true physical address.
>>
>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>> ---
>>  arch/x86_64.c  | 3 +++
>>  makedumpfile.c | 1 +
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>> index 537fb78..7651d36 100644
>> --- a/arch/x86_64.c
>> +++ b/arch/x86_64.c
>> @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>  			return NOT_PADDR;
>>  		}
>>  		pud_paddr  = pgd & ENTRY_MASK;
>> +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
>>  	}
>>
>>  	/*
>> @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>  	 * Get PMD.
>>  	 */
>>  	pmd_paddr  = pud_pte & ENTRY_MASK;
>> +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
>>  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
>>  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
>>  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
>> @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>  	 * Get PTE.
>>  	 */
>>  	pte_paddr  = pmd_pte & ENTRY_MASK;
>> +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
>>  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
>>  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
>>  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index a03aaa1..81c7bb4 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -977,6 +977,7 @@ next_page:
>>  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
>>
>>  	pgaddr = PAGEBASE(paddr);
>> +	pgaddr = pgaddr & ~(NUMBER(sme_mask));
> 
> Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
> if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
> So the four lines added above need
> 
>   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>     ...
> 

Thank you very much for pointing out my mistake.

I will improve it and post again.

> and, what I'm wondering is whether it doesn't need to take hugepages
> into account such as this
> 
> 392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
> 393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
> 394                         (vaddr & ~PMD_MASK);
> "arch/x86_64.c"
> 

This is a good question. Theoretically, it should be modified accordingly for
huge pages case.

But makedumpfile still works well without this change. And i'm sure that the
huge pages are enabled in crashed kernel. This is very strange.

Thanks.
Lianbo

> Thanks,
> Kazu
> 
> 
>>  	pgbuf = cache_search(pgaddr, read_size);
>>  	if (!pgbuf) {
>>  		++cache_miss;
>> --
>> 2.17.1
>>
> 
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[dyoung]

+ Tom
On 01/25/19 at 11:06am, lijiang wrote:
> 在 2019年01月24日 06:16, Kazuhito Hagio 写道:
> > On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
> >> For AMD machine with SME feature, if SME is enabled in the first
> >> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
> >> the memory encryption mask, so makedumpfile needs to remove the
> >> memory encryption mask to obtain the true physical address.
> >>
> >> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> >> ---
> >>  arch/x86_64.c  | 3 +++
> >>  makedumpfile.c | 1 +
> >>  2 files changed, 4 insertions(+)
> >>
> >> diff --git a/arch/x86_64.c b/arch/x86_64.c
> >> index 537fb78..7651d36 100644
> >> --- a/arch/x86_64.c
> >> +++ b/arch/x86_64.c
> >> @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
> >>  			return NOT_PADDR;
> >>  		}
> >>  		pud_paddr  = pgd & ENTRY_MASK;
> >> +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
> >>  	}
> >>
> >>  	/*
> >> @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
> >>  	 * Get PMD.
> >>  	 */
> >>  	pmd_paddr  = pud_pte & ENTRY_MASK;
> >> +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
> >>  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
> >>  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
> >>  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
> >> @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
> >>  	 * Get PTE.
> >>  	 */
> >>  	pte_paddr  = pmd_pte & ENTRY_MASK;
> >> +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
> >>  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
> >>  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
> >>  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
> >> diff --git a/makedumpfile.c b/makedumpfile.c
> >> index a03aaa1..81c7bb4 100644
> >> --- a/makedumpfile.c
> >> +++ b/makedumpfile.c
> >> @@ -977,6 +977,7 @@ next_page:
> >>  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
> >>
> >>  	pgaddr = PAGEBASE(paddr);
> >> +	pgaddr = pgaddr & ~(NUMBER(sme_mask));
> > 
> > Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
> > if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
> > So the four lines added above need
> > 
> >   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> >     ...
> > 
> 
> Thank you very much for pointing out my mistake.
> 
> I will improve it and post again.
> 
> > and, what I'm wondering is whether it doesn't need to take hugepages
> > into account such as this
> > 
> > 392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
> > 393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
> > 394                         (vaddr & ~PMD_MASK);
> > "arch/x86_64.c"
> > 
> 
> This is a good question. Theoretically, it should be modified accordingly for
> huge pages case.
> 
> But makedumpfile still works well without this change. And i'm sure that the
> huge pages are enabled in crashed kernel. This is very strange.
> 
> Thanks.
> Lianbo
> 
> > Thanks,
> > Kazu
> > 
> > 
> >>  	pgbuf = cache_search(pgaddr, read_size);
> >>  	if (!pgbuf) {
> >>  		++cache_miss;
> >> --
> >> 2.17.1
> >>
> > 
> > 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[Lendacky, Thomas]

On 1/24/19 9:55 PM, dyoung@redhat.com wrote:
> + Tom
> On 01/25/19 at 11:06am, lijiang wrote:
>> 在 2019年01月24日 06:16, Kazuhito Hagio 写道:
>>> On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
>>>> For AMD machine with SME feature, if SME is enabled in the first
>>>> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
>>>> the memory encryption mask, so makedumpfile needs to remove the
>>>> memory encryption mask to obtain the true physical address.
>>>>
>>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>>> ---
>>>>  arch/x86_64.c  | 3 +++
>>>>  makedumpfile.c | 1 +
>>>>  2 files changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>>>> index 537fb78..7651d36 100644
>>>> --- a/arch/x86_64.c
>>>> +++ b/arch/x86_64.c
>>>> @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>  			return NOT_PADDR;
>>>>  		}
>>>>  		pud_paddr  = pgd & ENTRY_MASK;
>>>> +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
>>>>  	}
>>>>
>>>>  	/*
>>>> @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>  	 * Get PMD.
>>>>  	 */
>>>>  	pmd_paddr  = pud_pte & ENTRY_MASK;
>>>> +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
>>>>  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
>>>>  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
>>>>  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
>>>> @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>  	 * Get PTE.
>>>>  	 */
>>>>  	pte_paddr  = pmd_pte & ENTRY_MASK;
>>>> +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
>>>>  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
>>>>  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
>>>>  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
>>>> diff --git a/makedumpfile.c b/makedumpfile.c
>>>> index a03aaa1..81c7bb4 100644
>>>> --- a/makedumpfile.c
>>>> +++ b/makedumpfile.c
>>>> @@ -977,6 +977,7 @@ next_page:
>>>>  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
>>>>
>>>>  	pgaddr = PAGEBASE(paddr);
>>>> +	pgaddr = pgaddr & ~(NUMBER(sme_mask));
>>>
>>> Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
>>> if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
>>> So the four lines added above need
>>>
>>>   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>>>     ...
>>>
>>
>> Thank you very much for pointing out my mistake.
>>
>> I will improve it and post again.

Might be worth creating a local variable that includes ENTRY_MASK and
NUMBER(sme_mask) so that you make the check just once. Then use that
variable in place of ENTRY_MASK in the remainder of the function so
that the correct value is used throughout.

This would also cover the 5-level path which would make this future
proof should AMD someday support 5-level paging.

>>
>>> and, what I'm wondering is whether it doesn't need to take hugepages
>>> into account such as this
>>>
>>> 392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
>>> 393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
>>> 394                         (vaddr & ~PMD_MASK);
>>> "arch/x86_64.c"
>>>
>>
>> This is a good question. Theoretically, it should be modified accordingly for
>> huge pages case.

Yes, this should also have the ~(NUMBER(sme_mask)) applied to it. You
can probably add some debugging to see if you're hitting this case and
whether the encryption bit (sme_mask) is set just to help understand what
is occurring. This also goes for the 1GB page check above.  However, if
you use my suggestion of a local variable then you should be covered.

Thanks,
Tom

>>
>> But makedumpfile still works well without this change. And i'm sure that the
>> huge pages are enabled in crashed kernel. This is very strange.
>>
>> Thanks.
>> Lianbo
>>
>>> Thanks,
>>> Kazu
>>>
>>>
>>>>  	pgbuf = cache_search(pgaddr, read_size);
>>>>  	if (!pgbuf) {
>>>>  		++cache_miss;
>>>> --
>>>> 2.17.1
>>>>
>>>
>>>
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[lijiang]

在 2019年01月25日 22:32, Lendacky, Thomas 写道:
> On 1/24/19 9:55 PM, dyoung@redhat.com wrote:
>> + Tom
>> On 01/25/19 at 11:06am, lijiang wrote:
>>> 在 2019年01月24日 06:16, Kazuhito Hagio 写道:
>>>> On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
>>>>> For AMD machine with SME feature, if SME is enabled in the first
>>>>> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
>>>>> the memory encryption mask, so makedumpfile needs to remove the
>>>>> memory encryption mask to obtain the true physical address.
>>>>>
>>>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>>>> ---
>>>>>  arch/x86_64.c  | 3 +++
>>>>>  makedumpfile.c | 1 +
>>>>>  2 files changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>>>>> index 537fb78..7651d36 100644
>>>>> --- a/arch/x86_64.c
>>>>> +++ b/arch/x86_64.c
>>>>> @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>>  			return NOT_PADDR;
>>>>>  		}
>>>>>  		pud_paddr  = pgd & ENTRY_MASK;
>>>>> +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
>>>>>  	}
>>>>>
>>>>>  	/*
>>>>> @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>>  	 * Get PMD.
>>>>>  	 */
>>>>>  	pmd_paddr  = pud_pte & ENTRY_MASK;
>>>>> +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
>>>>>  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
>>>>>  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
>>>>>  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
>>>>> @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>>  	 * Get PTE.
>>>>>  	 */
>>>>>  	pte_paddr  = pmd_pte & ENTRY_MASK;
>>>>> +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
>>>>>  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
>>>>>  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
>>>>>  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
>>>>> diff --git a/makedumpfile.c b/makedumpfile.c
>>>>> index a03aaa1..81c7bb4 100644
>>>>> --- a/makedumpfile.c
>>>>> +++ b/makedumpfile.c
>>>>> @@ -977,6 +977,7 @@ next_page:
>>>>>  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
>>>>>
>>>>>  	pgaddr = PAGEBASE(paddr);
>>>>> +	pgaddr = pgaddr & ~(NUMBER(sme_mask));
>>>>
>>>> Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
>>>> if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
>>>> So the four lines added above need
>>>>
>>>>   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>>>>     ...
>>>>
>>>
>>> Thank you very much for pointing out my mistake.
>>>
>>> I will improve it and post again.
> 
> Might be worth creating a local variable that includes ENTRY_MASK and
> NUMBER(sme_mask) so that you make the check just once. Then use that
> variable in place of ENTRY_MASK in the remainder of the function so
> that the correct value is used throughout.
> 
> This would also cover the 5-level path which would make this future
> proof should AMD someday support 5-level paging.
> 

Thank you, Tom. Makedumpfile will cover the 5-level path in next post,
though AMD does not support 5-level paging yet.

Thanks.
Lianbo

>>>
>>>> and, what I'm wondering is whether it doesn't need to take hugepages
>>>> into account such as this
>>>>
>>>> 392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
>>>> 393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
>>>> 394                         (vaddr & ~PMD_MASK);
>>>> "arch/x86_64.c"
>>>>
>>>
>>> This is a good question. Theoretically, it should be modified accordingly for
>>> huge pages case.
> 
> Yes, this should also have the ~(NUMBER(sme_mask)) applied to it. You
> can probably add some debugging to see if you're hitting this case and
> whether the encryption bit (sme_mask) is set just to help understand what
> is occurring. This also goes for the 1GB page check above.  However, if
> you use my suggestion of a local variable then you should be covered.
> 
> Thanks,
> Tom
> 
>>>
>>> But makedumpfile still works well without this change. And i'm sure that the
>>> huge pages are enabled in crashed kernel. This is very strange.
>>>
>>> Thanks.
>>> Lianbo
>>>
>>>> Thanks,
>>>> Kazu
>>>>
>>>>
>>>>>  	pgbuf = cache_search(pgaddr, read_size);
>>>>>  	if (!pgbuf) {
>>>>>  		++cache_miss;
>>>>> --
>>>>> 2.17.1
>>>>>
>>>>
>>>>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address

[lijiang]

在 2019年01月28日 09:55, lijiang 写道:
> 在 2019年01月25日 22:32, Lendacky, Thomas 写道:
>> On 1/24/19 9:55 PM, dyoung@redhat.com wrote:
>>> + Tom
>>> On 01/25/19 at 11:06am, lijiang wrote:
>>>> 在 2019年01月24日 06:16, Kazuhito Hagio 写道:
>>>>> On 1/22/2019 3:03 AM, Lianbo Jiang wrote:
>>>>>> For AMD machine with SME feature, if SME is enabled in the first
>>>>>> kernel, the crashed kernel's page table(pgd/pud/pmd/pte) contains
>>>>>> the memory encryption mask, so makedumpfile needs to remove the
>>>>>> memory encryption mask to obtain the true physical address.
>>>>>>
>>>>>> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
>>>>>> ---
>>>>>>  arch/x86_64.c  | 3 +++
>>>>>>  makedumpfile.c | 1 +
>>>>>>  2 files changed, 4 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>>>>>> index 537fb78..7651d36 100644
>>>>>> --- a/arch/x86_64.c
>>>>>> +++ b/arch/x86_64.c
>>>>>> @@ -346,6 +346,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>>>  			return NOT_PADDR;
>>>>>>  		}
>>>>>>  		pud_paddr  = pgd & ENTRY_MASK;
>>>>>> +		pud_paddr = pud_paddr & ~(NUMBER(sme_mask));
>>>>>>  	}
>>>>>>
>>>>>>  	/*
>>>>>> @@ -371,6 +372,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>>>  	 * Get PMD.
>>>>>>  	 */
>>>>>>  	pmd_paddr  = pud_pte & ENTRY_MASK;
>>>>>> +	pmd_paddr = pmd_paddr & ~(NUMBER(sme_mask));
>>>>>>  	pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
>>>>>>  	if (!readmem(PADDR, pmd_paddr, &pmd_pte, sizeof pmd_pte)) {
>>>>>>  		ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
>>>>>> @@ -391,6 +393,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
>>>>>>  	 * Get PTE.
>>>>>>  	 */
>>>>>>  	pte_paddr  = pmd_pte & ENTRY_MASK;
>>>>>> +	pte_paddr = pte_paddr & ~(NUMBER(sme_mask));
>>>>>>  	pte_paddr += pte_index(vaddr) * sizeof(unsigned long);
>>>>>>  	if (!readmem(PADDR, pte_paddr, &pte, sizeof pte)) {
>>>>>>  		ERRMSG("Can't get pte (pte_paddr:%lx).\n", pte_paddr);
>>>>>> diff --git a/makedumpfile.c b/makedumpfile.c
>>>>>> index a03aaa1..81c7bb4 100644
>>>>>> --- a/makedumpfile.c
>>>>>> +++ b/makedumpfile.c
>>>>>> @@ -977,6 +977,7 @@ next_page:
>>>>>>  	read_size = MIN(info->page_size - PAGEOFFSET(paddr), size);
>>>>>>
>>>>>>  	pgaddr = PAGEBASE(paddr);
>>>>>> +	pgaddr = pgaddr & ~(NUMBER(sme_mask));
>>>>>
>>>>> Since NUMBER(sme_mask) is initialized with -1 (NOT_FOUND_NUMBER),
>>>>> if the sme_mask is not in vmcoreinfo, ~(NUMBER(sme_mask)) will be 0.
>>>>> So the four lines added above need
>>>>>
>>>>>   if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>>>>>     ...
>>>>>
>>>>
>>>> Thank you very much for pointing out my mistake.
>>>>
>>>> I will improve it and post again.
>>
>> Might be worth creating a local variable that includes ENTRY_MASK and
>> NUMBER(sme_mask) so that you make the check just once. Then use that
>> variable in place of ENTRY_MASK in the remainder of the function so
>> that the correct value is used throughout.
>>

Ok.

>> This would also cover the 5-level path which would make this future
>> proof should AMD someday support 5-level paging.
>>
> 
> Thank you, Tom. Makedumpfile will cover the 5-level path in next post,
> though AMD does not support 5-level paging yet.
> 

I mean that i will improve this patch and cover the 5-level path in patch v2.

Thanks.

> Thanks.
> Lianbo
> 
>>>>
>>>>> and, what I'm wondering is whether it doesn't need to take hugepages
>>>>> into account such as this
>>>>>
>>>>> 392         if (pmd_pte & _PAGE_PSE)        /* 2MB pages */
>>>>> 393                 return (pmd_pte & ENTRY_MASK & PMD_MASK) +
>>>>> 394                         (vaddr & ~PMD_MASK);
>>>>> "arch/x86_64.c"
>>>>>
>>>>
>>>> This is a good question. Theoretically, it should be modified accordingly for
>>>> huge pages case.
>>
>> Yes, this should also have the ~(NUMBER(sme_mask)) applied to it. You
>> can probably add some debugging to see if you're hitting this case and
>> whether the encryption bit (sme_mask) is set just to help understand what
>> is occurring. This also goes for the 1GB page check above.  However, if
>> you use my suggestion of a local variable then you should be covered.
>>

Thank you, Tom.

I will modify this patch and cover the huge pages case in patch v2. 

Thanks.
Lianbo

>> Thanks,
>> Tom
>>
>>>>
>>>> But makedumpfile still works well without this change. And i'm sure that the
>>>> huge pages are enabled in crashed kernel. This is very strange.
>>>>
>>>> Thanks.
>>>> Lianbo
>>>>
>>>>> Thanks,
>>>>> Kazu
>>>>>
>>>>>
>>>>>>  	pgbuf = cache_search(pgaddr, read_size);
>>>>>>  	if (!pgbuf) {
>>>>>>  		++cache_miss;
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>
>>>>>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec