linux-kernel - Re: [PATCH v2 1/1] iommu/vt-d: Fix kdump kernels boot failure with scalable mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 23 Aug 2022 13:33:40 +0800
From:   Baolu Lu <baolu.lu@...ux.intel.com>
To:     "Tian, Kevin" <kevin.tian@...el.com>,
        "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>
Cc:     baolu.lu@...ux.intel.com, Joerg Roedel <joro@...tes.org>,
        Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Jerry Snitselaar <jsnitsel@...hat.com>,
        "Jin, Wen" <wen.jin@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v2 1/1] iommu/vt-d: Fix kdump kernels boot failure with
 scalable mode

On 2022/8/22 12:42, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@...ux.intel.com>
>> Sent: Thursday, August 18, 2022 7:13 PM
>>>>    	rtaddr_reg = dmar_readq(iommu->reg + DMAR_RTADDR_REG);
>>>> -	ext        = !!(rtaddr_reg & DMA_RTADDR_RTT);
>>>> -	new_ext    = !!ecap_ecs(iommu->ecap);
>>>> +	ext        = !!(rtaddr_reg & DMA_RTADDR_SMT);
>>>> +	new_ext    = !!ecap_smts(iommu->ecap);
>>>
>>> should be !!sm_supported()
>>
>> Not really. The IOMMU was setup by the previous kernel. Here we just
>> check whether the scalable mode was enabled there.
> 
> You want to compare whether old kernel and new kernel enable
> the same mode. ecap_smts is only about the capability. only
> sm_supported() can tell the mode which is actually used by the
> new kernel.

Oh, yes! You are right. I will update this.

> 
>>
>>>
>>>>
>>>>    	/*
>>>>    	 * The RTT bit can only be changed when translation is disabled,
>>>> @@ -2747,6 +2705,10 @@ static int copy_translation_tables(struct
>>>> intel_iommu *iommu)
>>>>    	if (new_ext != ext)
>>>>    		return -EINVAL;
>>>>
>>>> +	iommu->copied_tables = bitmap_zalloc(BIT_ULL(16), GFP_KERNEL);
>>>> +	if (!iommu->copied_tables)
>>>> +		return -ENOMEM;
>>>> +
>>>>    	old_rt_phys = rtaddr_reg & VTD_PAGE_MASK;
>>>>    	if (!old_rt_phys)
>>>>    		return -EINVAL;
>>>
>>> Out of curiosity. What is the rationale that we copy root table and
>>> context tables but not pasid tables?
>>
>> We only copy the context table and reconstruct it when the default
>> domain is attached. Before that, there's no need to reconstruct the
>> pasid table, hence it's safe to use the previous pasid tables.
>>
> 
> I still didn't get why context table must be reconstructed but not
> pasid table...

The pasid table is also reconstructed. The context table entry and the
pasid tables are reconstructed together, hence there's no need to copy
the pasid table.

Best regards,
baolu