* [PATCH 0/2] makedumpfile needs to remove the memory encryption
@ 2019-01-22 8:03 Lianbo Jiang
2019-01-22 8:03 ` [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table Lianbo Jiang
2019-01-22 8:03 ` [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address Lianbo Jiang
0 siblings, 2 replies; 13+ messages in thread
From: Lianbo Jiang @ 2019-01-22 8:03 UTC (permalink / raw)
To: kexec; +Cc: dyoung, k-hagio, anderson, bhe
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table
2019-01-22 8:03 [PATCH 0/2] makedumpfile needs to remove the memory encryption Lianbo Jiang
@ 2019-01-22 8:03 ` Lianbo Jiang
2019-01-23 21:51 ` Kazuhito Hagio
2019-01-22 8:03 ` [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address Lianbo Jiang
1 sibling, 1 reply; 13+ messages in thread
From: Lianbo Jiang @ 2019-01-22 8:03 UTC (permalink / raw)
To: kexec; +Cc: dyoung, k-hagio, anderson, bhe
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-22 8:03 [PATCH 0/2] makedumpfile needs to remove the memory encryption Lianbo Jiang
2019-01-22 8:03 ` [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table Lianbo Jiang
@ 2019-01-22 8:03 ` Lianbo Jiang
2019-01-23 22:16 ` Kazuhito Hagio
1 sibling, 1 reply; 13+ messages in thread
From: Lianbo Jiang @ 2019-01-22 8:03 UTC (permalink / raw)
To: kexec; +Cc: dyoung, k-hagio, anderson, bhe
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* RE: [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table
2019-01-22 8:03 ` [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table Lianbo Jiang
@ 2019-01-23 21:51 ` Kazuhito Hagio
2019-01-24 9:23 ` lijiang
0 siblings, 1 reply; 13+ messages in thread
From: Kazuhito Hagio @ 2019-01-23 21:51 UTC (permalink / raw)
To: Lianbo Jiang; +Cc: anderson, dyoung, kexec, bhe
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-22 8:03 ` [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address Lianbo Jiang
@ 2019-01-23 22:16 ` Kazuhito Hagio
2019-01-24 19:33 ` Kazuhito Hagio
2019-01-25 3:06 ` lijiang
0 siblings, 2 replies; 13+ messages in thread
From: Kazuhito Hagio @ 2019-01-23 22:16 UTC (permalink / raw)
To: Lianbo Jiang; +Cc: anderson, dyoung, kexec, bhe
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table
2019-01-23 21:51 ` Kazuhito Hagio
@ 2019-01-24 9:23 ` lijiang
0 siblings, 0 replies; 13+ messages in thread
From: lijiang @ 2019-01-24 9:23 UTC (permalink / raw)
To: Kazuhito Hagio; +Cc: anderson, dyoung, kexec, bhe
在 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-23 22:16 ` Kazuhito Hagio
@ 2019-01-24 19:33 ` Kazuhito Hagio
2019-01-25 3:17 ` lijiang
2019-01-25 3:06 ` lijiang
1 sibling, 1 reply; 13+ messages in thread
From: Kazuhito Hagio @ 2019-01-24 19:33 UTC (permalink / raw)
To: Lianbo Jiang; +Cc: dyoung, anderson, kexec, bhe
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-23 22:16 ` Kazuhito Hagio
2019-01-24 19:33 ` Kazuhito Hagio
@ 2019-01-25 3:06 ` lijiang
2019-01-25 3:55 ` dyoung
1 sibling, 1 reply; 13+ messages in thread
From: lijiang @ 2019-01-25 3:06 UTC (permalink / raw)
To: Kazuhito Hagio; +Cc: anderson, dyoung, kexec, bhe
在 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-24 19:33 ` Kazuhito Hagio
@ 2019-01-25 3:17 ` lijiang
0 siblings, 0 replies; 13+ messages in thread
From: lijiang @ 2019-01-25 3:17 UTC (permalink / raw)
To: Kazuhito Hagio; +Cc: dyoung, anderson, kexec, bhe
在 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-25 3:06 ` lijiang
@ 2019-01-25 3:55 ` dyoung
2019-01-25 14:32 ` Lendacky, Thomas
0 siblings, 1 reply; 13+ messages in thread
From: dyoung @ 2019-01-25 3:55 UTC (permalink / raw)
To: lijiang; +Cc: anderson, Kazuhito Hagio, kexec, bhe, Tom Lendacky
+ 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-25 3:55 ` dyoung
@ 2019-01-25 14:32 ` Lendacky, Thomas
2019-01-28 1:55 ` lijiang
0 siblings, 1 reply; 13+ messages in thread
From: Lendacky, Thomas @ 2019-01-25 14:32 UTC (permalink / raw)
To: dyoung, lijiang; +Cc: anderson, Kazuhito Hagio, kexec, bhe
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-25 14:32 ` Lendacky, Thomas
@ 2019-01-28 1:55 ` lijiang
2019-01-28 3:15 ` lijiang
0 siblings, 1 reply; 13+ messages in thread
From: lijiang @ 2019-01-28 1:55 UTC (permalink / raw)
To: Lendacky, Thomas, dyoung; +Cc: anderson, Kazuhito Hagio, kexec, bhe
在 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address
2019-01-28 1:55 ` lijiang
@ 2019-01-28 3:15 ` lijiang
0 siblings, 0 replies; 13+ messages in thread
From: lijiang @ 2019-01-28 3:15 UTC (permalink / raw)
To: Lendacky, Thomas, dyoung; +Cc: anderson, Kazuhito Hagio, kexec, bhe
在 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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-01-28 3:15 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 8:03 [PATCH 0/2] makedumpfile needs to remove the memory encryption Lianbo Jiang
2019-01-22 8:03 ` [PATCH 1/2] Makedumpfile: add a new variable 'sme_mask' to number_table Lianbo Jiang
2019-01-23 21:51 ` Kazuhito Hagio
2019-01-24 9:23 ` lijiang
2019-01-22 8:03 ` [PATCH 2/2] Remove the memory encryption mask to obtain the true physical address Lianbo Jiang
2019-01-23 22:16 ` Kazuhito Hagio
2019-01-24 19:33 ` Kazuhito Hagio
2019-01-25 3:17 ` lijiang
2019-01-25 3:06 ` lijiang
2019-01-25 3:55 ` dyoung
2019-01-25 14:32 ` Lendacky, Thomas
2019-01-28 1:55 ` lijiang
2019-01-28 3:15 ` lijiang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.