[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <af3e0e99-3688-5801-27d2-3848dd0ad043@redhat.com>
Date: Thu, 21 Jun 2018 17:45:51 +0800
From: lijiang <lijiang@...hat.com>
To: Baoquan He <bhe@...hat.com>
Cc: Tom Lendacky <thomas.lendacky@....com>,
linux-kernel@...r.kernel.org, dyoung@...hat.com,
iommu@...ts.linux-foundation.org, kexec@...ts.infradead.org
Subject: Re: [PATCH 3/4 V3] Remap the device table of IOMMU in encrypted
manner for kdump
在 2018年06月21日 16:39, Baoquan He 写道:
> On 06/21/18 at 01:42pm, lijiang wrote:
>> 在 2018年06月21日 00:42, Tom Lendacky 写道:
>>> On 6/16/2018 3:27 AM, Lianbo Jiang wrote:
>>>> In kdump mode, it will copy the device table of IOMMU from the old
>>>> device table, which is encrypted when SME is enabled in the first
>>>> kernel. So we must remap it in encrypted manner in order to be
>>>> automatically decrypted when we read.
>>>>
>>>> Signed-off-by: Lianbo Jiang <lijiang@...hat.com>
>>>> ---
>>>> Some changes:
>>>> 1. add some comments
>>>> 2. clean compile warning.
>>>>
>>>> drivers/iommu/amd_iommu_init.c | 15 ++++++++++++++-
>>>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
>>>> index 904c575..a20af4c 100644
>>>> --- a/drivers/iommu/amd_iommu_init.c
>>>> +++ b/drivers/iommu/amd_iommu_init.c
>>>> @@ -889,11 +889,24 @@ static bool copy_device_table(void)
>>>> }
>>>>
>>>> old_devtb_phys = entry & PAGE_MASK;
>>>> +
>>>> + /*
>>>> + * When sme enable in the first kernel, old_devtb_phys includes the
>>>> + * memory encryption mask(sme_me_mask), we must remove the memory
>>>> + * encryption mask to obtain the true physical address in kdump mode.
>>>> + */
>>>> + if (mem_encrypt_active() && is_kdump_kernel())
>>>> + old_devtb_phys = __sme_clr(old_devtb_phys);
>>>> +
>>>
>>> You can probably just use "if (is_kdump_kernel())" here, since memory
>>> encryption is either on in both the first and second kernel or off in
>>> both the first and second kernel. At which point __sme_clr() will do
>>> the proper thing.
>>>
>>> Actually, this needs to be done no matter what. When doing either the
>>> ioremap_encrypted() or the memremap(), the physical address should not
>>> include the encryption bit/mask.
>>>
>>> Thanks,
>>> Tom
>>>
>> Thanks for your comments. If we don't remove the memory encryption mask, it will
>> return false because the 'old_devtb_phys >= 0x100000000ULL' may become true.
>
> Lianbo, you may not get what Tom suggested. Tom means no matter what it
> is, encrypted or not in 1st kernel, we need get pure physicall address,
> and using below code is always right for both cases.
>
> if (is_kdump_kernel())
> old_devtb_phys = __sme_clr(old_devtb_phys);
>
Thank you, Baoquan. I understood what Tom said. I just want to explain why removing
the memory encryption mask is necessary before the 'old_devtb_phys >= 0x100000000ULL'.
Lianbo
> And this is simpler. You even can add one line of code comment to say
> like "Physical address w/o encryption mask is needed here."
>>
>> Lianbo
>>>> if (old_devtb_phys >= 0x100000000ULL) {
>>>> pr_err("The address of old device table is above 4G, not trustworthy!\n");
>>>> return false;
>>>> }
>>>> - old_devtb = memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);
>>>> + old_devtb = (mem_encrypt_active() && is_kdump_kernel())
>>>> + ? (__force void *)ioremap_encrypted(old_devtb_phys,
>>>> + dev_table_size)
>>>> + : memremap(old_devtb_phys, dev_table_size, MEMREMAP_WB);> +
>>>> if (!old_devtb)
>>>> return false;
>>>>
>>>>
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec
Powered by blists - more mailing lists