linux-kernel - Re: [PATCH v2 1/4] iommu/vt-d: Check before setting PGSNP bit in pasid table entry

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]

Message-ID: <4b2cd2a7-d715-882c-9cce-533dcff8bc79@linux.intel.com>
Date:   Sun, 24 Apr 2022 12:37:44 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     "Tian, Kevin" <kevin.tian@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>
Cc:     "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/4] iommu/vt-d: Check before setting PGSNP bit in
 pasid table entry

On 2022/4/24 11:37, Tian, Kevin wrote:
>>> This should be rebased on top of Jason's enforce coherency series
>>> instead of blindly setting it. No matter whether it's legacy mode
>>> where we set SNP in PTE or scalable mode where we set PGSNP
>>> in PASID entry for entire page table, the trigger point should be
>>> same i.e. when someone calls enforce_cache_coherency().
>> With Jason's enforce coherency series merged, we even don't need to set
>> PGSNP bit of a pasid entry for second level translation. 2nd level
>> always supports SNP in PTEs, so set PGSNP in pasid table entry is
>> unnecessary.
>>
> Yes, this sounds correct for 2nd-level.
> 
> but setting PGSNP of 1st level translation is also relevant to that
> change when talking about enforcing coherency in the guest. In
> this case PASID_FLAG_PAGE_SNOOP should be set also after
> enforce_cache_coherency() is called.

Yes. Agreed.

> Currently it's always set for unmanaged domain in
> domain_setup_first_level():
> 
> 	if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
> 		flags |= PASID_FLAG_PAGE_SNOOP;
> 
> Suppose we need a separate interface to update PGSNP after pasid
> entry is set up.

Currently enforcing coherency is only used in VFIO. In the VFIO use
case, it's safe to always set PGSNP when an UNMANAGED domain is attached
on the first level pasid translation. We could add support of updating
PGSNP after pasid entry setup once there's a real need.

Best regards,
baolu