Re: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
在 2019年01月28日 22:24, Lendacky, Thomas 写道:
> On 1/27/19 11:46 PM, 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
>> ---
>> Changes since v1:
>> 1. Merge them into a patch.
>> 2. The sme_mask is not an enum number, remove it.
>> 3. Sanity check whether the sme_mask is in vmcoreinfo.
>> 4. Deal with the huge pages case.
>> 5. Cover the 5-level path.
>>
>> arch/x86_64.c | 30 +-
>> makedumpfile.c | 4
>> makedumpfile.h | 1 +
>> 3 files changed, 22 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>> index 537fb78..7b3ed10 100644
>> --- a/arch/x86_64.c
>> +++ b/arch/x86_64.c
>> @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>> pagetable)
>> unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
>> unsigned long pte_paddr, pte;
>> unsigned long p4d_paddr, p4d_pte;
>> +unsigned long sme_me_mask = ~0UL;
>>
>> /*
>> * Get PGD.
>> @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>> pagetable)
>> return NOT_PADDR;
>> }
>>
>> +if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>> +sme_me_mask = ~(NUMBER(sme_mask));
>
> This is a bit confusing since this isn't the sme_me_mask any more, but the
> complement. Might want to somehow rename this so that it doesn't cause any
> confusion.
>
Thanks for your comment.
I will change the sme_me_mask to entry_mask in patch v3.
>> +
>> if (check_5level_paging()) {
>> page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
>> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
>> @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>> pagetable)
>> return NOT_PADDR;
>> }
>> if (info->vaddr_for_vtop == vaddr)
>> -MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
>> +MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
>> sme_me_mask));
>
> No need to remove the mask here. You're just printing out the value of
> the entry. It might be nice to know whether the encryption bit is set or
> not - after all, ENTRY_MASK is still part of this value.
>
Ok, i will remove it in patch v3.
>>
>> if (!(pgd & _PAGE_PRESENT)) {
>> ERRMSG("Can't get a valid pgd.\n");
>> @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>> pagetable)
>> /*
>> * Get P4D.
>> */
>> -p4d_paddr = pgd & ENTRY_MASK;
>> +p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
>
> This goes back to my original comment that you should just make a local
> variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
> performing this ANDing everywhere ENTRY_MASK is used - except then you
> miss the one at the very end of this routine on the return statement.
>
Ok, i will improve them in patch v3.
>> p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
>> if (!readmem(PADDR, p4d_paddr, _pte, sizeof p4d_pte)) {
>> ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
>> p4d_paddr);
>> return NOT_PADDR;
>> }
>> if (info->vaddr_for_vtop == vaddr)
>> -MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
>> +MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
>> sme_me_mask));
>>
>> if (!(p4d_pte & _PAGE_PRESENT)) {
>> ERRMSG("Can't get a valid p4d_pte.\n");
>> return NOT_PADDR;
>> }
>> -pud_paddr = p4d_pte & ENTRY_MASK;
>> +pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
>> }else {
>> page_dir += pgd_index(vaddr) * sizeof(unsigned long);
>> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
>> @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>> pagetable)
>> return NOT_PADDR;
>> }
>> if (info->vaddr_for_vtop == vaddr)
>> -MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
>> +MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
>> sme_me_mask));
>>
>> if (!(pgd & _PAGE_PRESENT)) {
>> ERRMSG("Can't get a valid pgd.\n");
>> return NOT_PADDR;
>> }
>> -pud_paddr = pgd & ENTRY_MASK;
>> +pud_paddr = pgd & ENTRY_MASK & sme_me_mask;
>> }
>>
>> /*
>> @@ -357,47 +361,47 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>> pagetable)
>> return NOT_PADDR;
>>
Re: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
在 2019年01月29日 03:45, Kazuhito Hagio 写道:
> On 1/28/2019 9:24 AM, Lendacky, Thomas wrote:
>> On 1/27/19 11:46 PM, 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
>>> ---
>>> Changes since v1:
>>> 1. Merge them into a patch.
>>> 2. The sme_mask is not an enum number, remove it.
>>> 3. Sanity check whether the sme_mask is in vmcoreinfo.
>>> 4. Deal with the huge pages case.
>>> 5. Cover the 5-level path.
>>>
>>> arch/x86_64.c | 30 +-
>>> makedumpfile.c | 4
>>> makedumpfile.h | 1 +
>>> 3 files changed, 22 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>>> index 537fb78..7b3ed10 100644
>>> --- a/arch/x86_64.c
>>> +++ b/arch/x86_64.c
>>> @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
>>> unsigned long pte_paddr, pte;
>>> unsigned long p4d_paddr, p4d_pte;
>>> + unsigned long sme_me_mask = ~0UL;
>>>
>>> /*
>>> * Get PGD.
>>> @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> return NOT_PADDR;
>>> }
>>>
>>> + if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>>> + sme_me_mask = ~(NUMBER(sme_mask));
>>
>> This is a bit confusing since this isn't the sme_me_mask any more, but the
>> complement. Might want to somehow rename this so that it doesn't cause any
>> confusion.
>>
>>> +
>>> if (check_5level_paging()) {
>>> page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
>>> @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> return NOT_PADDR;
>>> }
>>> if (info->vaddr_for_vtop == vaddr)
>>> - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
>>> + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
>>> sme_me_mask));
>>
>> No need to remove the mask here. You're just printing out the value of
>> the entry. It might be nice to know whether the encryption bit is set or
>> not - after all, ENTRY_MASK is still part of this value.
>
> Agreed.
>
>>
>>>
>>> if (!(pgd & _PAGE_PRESENT)) {
>>> ERRMSG("Can't get a valid pgd.\n");
>>> @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> /*
>>> * Get P4D.
>>> */
>>> - p4d_paddr = pgd & ENTRY_MASK;
>>> + p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
>>
>> This goes back to my original comment that you should just make a local
>> variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
>> performing this ANDing everywhere ENTRY_MASK is used - except then you
>> miss the one at the very end of this routine on the return statement.
>
> This was my idea I said to Lianbo before seeing your comment, but
> yes, including ENTRY_MASK in a local variable is better than that.
> Thanks for your good suggestion.
>
> As for the variable's name, I think that "entry_mask" is good enough,
> but any better name?
>
> unsigned long entry_mask = ENTRY_MASK;
>
> if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> entry_mask &= ~(NUMBER(sme_mask));
> ...
> p4d_paddr = pgd & entry_mask;
>
Ok. Thanks.
> And, I found that the find_vmemmap_x86_64() function also uses the
> page table for the -e option and looks to be affected by SME.
> Lianbo, would you fix the function, too?
>
Yes, it's my pleasure. Thank you, Kazu.
I will fix this function in patch v3.
Regards,
Lianbo
> Thanks,
> Kazu
>
>>
>>> p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, p4d_paddr, _pte, sizeof p4d_pte)) {
>>> ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
>>> p4d_paddr);
>>> return NOT_PADDR;
>>> }
>>> if (info->vaddr_for_vtop == vaddr)
>>> - MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
>>> + MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
>>> sme_me_mask));
>>>
>>> if (!(p4d_pte & _PAGE_PRESENT)) {
>>> ERRMSG("Can't get a valid p4d_pte.\n");
>>> return NOT_PADDR;
>>> }
>>> - pud_paddr = p4d_pte & ENTRY_MASK;
>>> + pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
>>> }else {
>>> page_dir += pgd_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
>>> @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>>
Re: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
On 1/28/19 1:45 PM, Kazuhito Hagio wrote:
> On 1/28/2019 9:24 AM, Lendacky, Thomas wrote:
>> On 1/27/19 11:46 PM, 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
>>> ---
>>> Changes since v1:
>>> 1. Merge them into a patch.
>>> 2. The sme_mask is not an enum number, remove it.
>>> 3. Sanity check whether the sme_mask is in vmcoreinfo.
>>> 4. Deal with the huge pages case.
>>> 5. Cover the 5-level path.
>>>
>>> arch/x86_64.c | 30 +-
>>> makedumpfile.c | 4
>>> makedumpfile.h | 1 +
>>> 3 files changed, 22 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/x86_64.c b/arch/x86_64.c
>>> index 537fb78..7b3ed10 100644
>>> --- a/arch/x86_64.c
>>> +++ b/arch/x86_64.c
>>> @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
>>> unsigned long pte_paddr, pte;
>>> unsigned long p4d_paddr, p4d_pte;
>>> + unsigned long sme_me_mask = ~0UL;
>>>
>>> /*
>>> * Get PGD.
>>> @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> return NOT_PADDR;
>>> }
>>>
>>> + if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
>>> + sme_me_mask = ~(NUMBER(sme_mask));
>>
>> This is a bit confusing since this isn't the sme_me_mask any more, but the
>> complement. Might want to somehow rename this so that it doesn't cause any
>> confusion.
>>
>>> +
>>> if (check_5level_paging()) {
>>> page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
>>> @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> return NOT_PADDR;
>>> }
>>> if (info->vaddr_for_vtop == vaddr)
>>> - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
>>> + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
>>> sme_me_mask));
>>
>> No need to remove the mask here. You're just printing out the value of
>> the entry. It might be nice to know whether the encryption bit is set or
>> not - after all, ENTRY_MASK is still part of this value.
>
> Agreed.
>
>>
>>>
>>> if (!(pgd & _PAGE_PRESENT)) {
>>> ERRMSG("Can't get a valid pgd.\n");
>>> @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>> /*
>>> * Get P4D.
>>> */
>>> - p4d_paddr = pgd & ENTRY_MASK;
>>> + p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
>>
>> This goes back to my original comment that you should just make a local
>> variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
>> performing this ANDing everywhere ENTRY_MASK is used - except then you
>> miss the one at the very end of this routine on the return statement.
>
> This was my idea I said to Lianbo before seeing your comment, but
> yes, including ENTRY_MASK in a local variable is better than that.
> Thanks for your good suggestion.
>
> As for the variable's name, I think that "entry_mask" is good enough,
> but any better name?
No, I think "entry_mask" is just fine.
>
> unsigned long entry_mask = ENTRY_MASK;
>
> if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> entry_mask &= ~(NUMBER(sme_mask));
> ...
> p4d_paddr = pgd & entry_mask;
Yup, exactly what I was thinking.
Thanks,
Tom
>
> And, I found that the find_vmemmap_x86_64() function also uses the
> page table for the -e option and looks to be affected by SME.
> Lianbo, would you fix the function, too?
>
> Thanks,
> Kazu
>
>>
>>> p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, p4d_paddr, _pte, sizeof p4d_pte)) {
>>> ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
>>> p4d_paddr);
>>> return NOT_PADDR;
>>> }
>>> if (info->vaddr_for_vtop == vaddr)
>>> - MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
>>> + MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
>>> sme_me_mask));
>>>
>>> if (!(p4d_pte & _PAGE_PRESENT)) {
>>> ERRMSG("Can't get a valid p4d_pte.\n");
>>> return NOT_PADDR;
>>> }
>>> - pud_paddr = p4d_pte & ENTRY_MASK;
>>> + pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
>>> }else {
>>> page_dir += pgd_index(vaddr) * sizeof(unsigned long);
>>> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
>>> @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
>>> pagetable)
>>>
RE: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
On 1/28/2019 9:24 AM, Lendacky, Thomas wrote:
> On 1/27/19 11:46 PM, 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
> > ---
> > Changes since v1:
> > 1. Merge them into a patch.
> > 2. The sme_mask is not an enum number, remove it.
> > 3. Sanity check whether the sme_mask is in vmcoreinfo.
> > 4. Deal with the huge pages case.
> > 5. Cover the 5-level path.
> >
> > arch/x86_64.c | 30 +-
> > makedumpfile.c | 4
> > makedumpfile.h | 1 +
> > 3 files changed, 22 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/x86_64.c b/arch/x86_64.c
> > index 537fb78..7b3ed10 100644
> > --- a/arch/x86_64.c
> > +++ b/arch/x86_64.c
> > @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
> > unsigned long pte_paddr, pte;
> > unsigned long p4d_paddr, p4d_pte;
> > + unsigned long sme_me_mask = ~0UL;
> >
> > /*
> > * Get PGD.
> > @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > return NOT_PADDR;
> > }
> >
> > + if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> > + sme_me_mask = ~(NUMBER(sme_mask));
>
> This is a bit confusing since this isn't the sme_me_mask any more, but the
> complement. Might want to somehow rename this so that it doesn't cause any
> confusion.
>
> > +
> > if (check_5level_paging()) {
> > page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
> > if (!readmem(PADDR, page_dir, , sizeof pgd)) {
> > @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > return NOT_PADDR;
> > }
> > if (info->vaddr_for_vtop == vaddr)
> > - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> > + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
> > sme_me_mask));
>
> No need to remove the mask here. You're just printing out the value of
> the entry. It might be nice to know whether the encryption bit is set or
> not - after all, ENTRY_MASK is still part of this value.
Agreed.
>
> >
> > if (!(pgd & _PAGE_PRESENT)) {
> > ERRMSG("Can't get a valid pgd.\n");
> > @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > /*
> > * Get P4D.
> > */
> > - p4d_paddr = pgd & ENTRY_MASK;
> > + p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
>
> This goes back to my original comment that you should just make a local
> variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
> performing this ANDing everywhere ENTRY_MASK is used - except then you
> miss the one at the very end of this routine on the return statement.
This was my idea I said to Lianbo before seeing your comment, but
yes, including ENTRY_MASK in a local variable is better than that.
Thanks for your good suggestion.
As for the variable's name, I think that "entry_mask" is good enough,
but any better name?
unsigned long entry_mask = ENTRY_MASK;
if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
entry_mask &= ~(NUMBER(sme_mask));
...
p4d_paddr = pgd & entry_mask;
And, I found that the find_vmemmap_x86_64() function also uses the
page table for the -e option and looks to be affected by SME.
Lianbo, would you fix the function, too?
Thanks,
Kazu
>
> > p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
> > if (!readmem(PADDR, p4d_paddr, _pte, sizeof p4d_pte)) {
> > ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
> > p4d_paddr);
> > return NOT_PADDR;
> > }
> > if (info->vaddr_for_vtop == vaddr)
> > - MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
> > + MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
> > sme_me_mask));
> >
> > if (!(p4d_pte & _PAGE_PRESENT)) {
> > ERRMSG("Can't get a valid p4d_pte.\n");
> > return NOT_PADDR;
> > }
> > - pud_paddr = p4d_pte & ENTRY_MASK;
> > + pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
> > }else {
> > page_dir += pgd_index(vaddr) * sizeof(unsigned long);
> > if (!readmem(PADDR, page_dir, , sizeof pgd)) {
> > @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> > pagetable)
> > return NOT_PADDR;
> > }
> > if (info->vaddr_for_vtop == vaddr)
> > - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> > +
Re: [PATCH v2] Remove the memory encryption mask to obtain the true physical address
On 1/27/19 11:46 PM, 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
> ---
> Changes since v1:
> 1. Merge them into a patch.
> 2. The sme_mask is not an enum number, remove it.
> 3. Sanity check whether the sme_mask is in vmcoreinfo.
> 4. Deal with the huge pages case.
> 5. Cover the 5-level path.
>
> arch/x86_64.c | 30 +-
> makedumpfile.c | 4
> makedumpfile.h | 1 +
> 3 files changed, 22 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86_64.c b/arch/x86_64.c
> index 537fb78..7b3ed10 100644
> --- a/arch/x86_64.c
> +++ b/arch/x86_64.c
> @@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
> unsigned long pte_paddr, pte;
> unsigned long p4d_paddr, p4d_pte;
> + unsigned long sme_me_mask = ~0UL;
>
> /*
>* Get PGD.
> @@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
>
> + if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
> + sme_me_mask = ~(NUMBER(sme_mask));
This is a bit confusing since this isn't the sme_me_mask any more, but the
complement. Might want to somehow rename this so that it doesn't cause any
confusion.
> +
> if (check_5level_paging()) {
> page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
> @@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
> sme_me_mask));
No need to remove the mask here. You're just printing out the value of
the entry. It might be nice to know whether the encryption bit is set or
not - after all, ENTRY_MASK is still part of this value.
>
> if (!(pgd & _PAGE_PRESENT)) {
> ERRMSG("Can't get a valid pgd.\n");
> @@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> /*
>* Get P4D.
>*/
> - p4d_paddr = pgd & ENTRY_MASK;
> + p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
This goes back to my original comment that you should just make a local
variable that is "ENTRY_MASK & ~(NUMBER(sme_mask))" since you are
performing this ANDing everywhere ENTRY_MASK is used - except then you
miss the one at the very end of this routine on the return statement.
> p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
> if (!readmem(PADDR, p4d_paddr, _pte, sizeof p4d_pte)) {
> ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
> p4d_paddr);
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
> + MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
> sme_me_mask));
>
> if (!(p4d_pte & _PAGE_PRESENT)) {
> ERRMSG("Can't get a valid p4d_pte.\n");
> return NOT_PADDR;
> }
> - pud_paddr = p4d_pte & ENTRY_MASK;
> + pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
> }else {
> page_dir += pgd_index(vaddr) * sizeof(unsigned long);
> if (!readmem(PADDR, page_dir, , sizeof pgd)) {
> @@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
> + MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
> sme_me_mask));
>
> if (!(pgd & _PAGE_PRESENT)) {
> ERRMSG("Can't get a valid pgd.\n");
> return NOT_PADDR;
> }
> - pud_paddr = pgd & ENTRY_MASK;
> + pud_paddr = pgd & ENTRY_MASK & sme_me_mask;
> }
>
> /*
> @@ -357,47 +361,47 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
> pagetable)
> return NOT_PADDR;
> }
> if (info->vaddr_for_vtop == vaddr)
> - MSG(" PUD : %16lx => %16lx\n", pud_paddr, pud_pte);
> + MSG(" PUD : %16lx => %16lx\n", pud_paddr, (pud_pte &
> sme_me_mask));
>
> if (!(pud_pte & _PAGE_PRESENT)) {
>
[PATCH v2] 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.
Signed-off-by: Lianbo Jiang
---
Changes since v1:
1. Merge them into a patch.
2. The sme_mask is not an enum number, remove it.
3. Sanity check whether the sme_mask is in vmcoreinfo.
4. Deal with the huge pages case.
5. Cover the 5-level path.
arch/x86_64.c | 30 +-
makedumpfile.c | 4
makedumpfile.h | 1 +
3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/arch/x86_64.c b/arch/x86_64.c
index 537fb78..7b3ed10 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -291,6 +291,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
unsigned long page_dir, pgd, pud_paddr, pud_pte, pmd_paddr, pmd_pte;
unsigned long pte_paddr, pte;
unsigned long p4d_paddr, p4d_pte;
+ unsigned long sme_me_mask = ~0UL;
/*
* Get PGD.
@@ -302,6 +303,9 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
return NOT_PADDR;
}
+ if (NUMBER(sme_mask) != NOT_FOUND_NUMBER)
+ sme_me_mask = ~(NUMBER(sme_mask));
+
if (check_5level_paging()) {
page_dir += pgd5_index(vaddr) * sizeof(unsigned long);
if (!readmem(PADDR, page_dir, , sizeof pgd)) {
@@ -309,7 +313,7 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long pagetable)
return NOT_PADDR;
}
if (info->vaddr_for_vtop == vaddr)
- MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
+ MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
sme_me_mask));
if (!(pgd & _PAGE_PRESENT)) {
ERRMSG("Can't get a valid pgd.\n");
@@ -318,20 +322,20 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
pagetable)
/*
* Get P4D.
*/
- p4d_paddr = pgd & ENTRY_MASK;
+ p4d_paddr = pgd & ENTRY_MASK & sme_me_mask;
p4d_paddr += p4d_index(vaddr) * sizeof(unsigned long);
if (!readmem(PADDR, p4d_paddr, _pte, sizeof p4d_pte)) {
ERRMSG("Can't get p4d_pte (p4d_paddr:%lx).\n",
p4d_paddr);
return NOT_PADDR;
}
if (info->vaddr_for_vtop == vaddr)
- MSG(" P4D : %16lx => %16lx\n", p4d_paddr, p4d_pte);
+ MSG(" P4D : %16lx => %16lx\n", p4d_paddr, (p4d_pte &
sme_me_mask));
if (!(p4d_pte & _PAGE_PRESENT)) {
ERRMSG("Can't get a valid p4d_pte.\n");
return NOT_PADDR;
}
- pud_paddr = p4d_pte & ENTRY_MASK;
+ pud_paddr = p4d_pte & ENTRY_MASK & sme_me_mask;
}else {
page_dir += pgd_index(vaddr) * sizeof(unsigned long);
if (!readmem(PADDR, page_dir, , sizeof pgd)) {
@@ -339,13 +343,13 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
pagetable)
return NOT_PADDR;
}
if (info->vaddr_for_vtop == vaddr)
- MSG(" PGD : %16lx => %16lx\n", page_dir, pgd);
+ MSG(" PGD : %16lx => %16lx\n", page_dir, (pgd &
sme_me_mask));
if (!(pgd & _PAGE_PRESENT)) {
ERRMSG("Can't get a valid pgd.\n");
return NOT_PADDR;
}
- pud_paddr = pgd & ENTRY_MASK;
+ pud_paddr = pgd & ENTRY_MASK & sme_me_mask;
}
/*
@@ -357,47 +361,47 @@ __vtop4_x86_64(unsigned long vaddr, unsigned long
pagetable)
return NOT_PADDR;
}
if (info->vaddr_for_vtop == vaddr)
- MSG(" PUD : %16lx => %16lx\n", pud_paddr, pud_pte);
+ MSG(" PUD : %16lx => %16lx\n", pud_paddr, (pud_pte &
sme_me_mask));
if (!(pud_pte & _PAGE_PRESENT)) {
ERRMSG("Can't get a valid pud_pte.\n");
return NOT_PADDR;
}
if (pud_pte & _PAGE_PSE)/* 1GB pages */
- return (pud_pte & ENTRY_MASK & PUD_MASK) +
+ return (pud_pte & ENTRY_MASK & PUD_MASK & sme_me_mask) +
(vaddr & ~PUD_MASK);
/*
* Get PMD.
*/
- pmd_paddr = pud_pte & ENTRY_MASK;
+ pmd_paddr = pud_pte & ENTRY_MASK & sme_me_mask;
pmd_paddr += pmd_index(vaddr) * sizeof(unsigned long);
if (!readmem(PADDR, pmd_paddr, _pte, sizeof pmd_pte)) {
ERRMSG("Can't get pmd_pte (pmd_paddr:%lx).\n", pmd_paddr);
return NOT_PADDR;
}
if (info->vaddr_for_vtop == vaddr)
- MSG(" PMD : %16lx =>
