Messages in this thread | | | From | "Tian, Kevin" <> | Subject | RE: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management | Date | Wed, 16 May 2018 08:56:30 +0000 |
| |
> From: Lu Baolu [mailto:baolu.lu@linux.intel.com] > Sent: Wednesday, May 16, 2018 4:01 PM > > Hi Joerg, > > Thank you for looking at my patches. > > On 05/15/2018 10:11 PM, Joerg Roedel wrote: > > On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote: > >> PATCH 4~9 implement per domain PASID table. Current per IOMMU > >> PASID table implementation is insecure in the cases where > >> multiple devices under one single IOMMU unit support PASID > >> feature. With per domain PASID table, we can achieve finer > >> protection and isolation granularity. > > > > Hold on, we hat discussions in the past about doing a system-wide pasid > > space, so that every mm_struct with devices attached gets the same pasid > > across all devices it is talking to. Reason was that some devices (will) > > require this to work correctly. This goes into the opposite direction, > > so I am a bit confused here. Please explain, is this not longer > > necessary? > > You are right. System-wide pasid space is necessary, hence PATCH > 1~3 implement it. But PATCH 4~9 don't go into the opposite direction, > it's designed to address another potential issue.
one thing you may want to highlight is, even with PATCH 4-9 it's still doing system-wide PASID space allocation. Just PASID table itself is kept per-device for isolation purpose as you described below, i.e. each device can access only those PASIDs which are allocated to itself while the allocation happens system-wide...
> > With system-wide pasid space, we can use a system-wide pasid table, > or just keep what we have now(per iommu unit pasid table). Both > system-wide and per iommu unitpasid table mean that two devices > might share a single pasid table. That will result in an issue. > > For an example, device A is assigned to access the memory space of > process A, and device B is assigned to access the memory space of > process B. The dma remapping infrastructure looks like: > > .------------------. > .----------------. | | > | | | | > .----------------. | Paging structure | > | PASID X |--| | for Process A | > .----------------. | | | > | | --->'------------------' > .----------------. .----------------. > | | | PASID Y |--| > .----------------. .----------------. | > | Dev_A context |---| | | | .------------------. > '----------------' | .----------------. | | | > | | | | | | | | > '----------------' | .----------------. | | Paging structure | > | Dev_B context | -->| | | | for Process B | > '----------------'----->'----------------' | | | > | | system-wide v-->'------------------' > .----------------. pasid table > | | > '----------------' > Intel iommu > context table > > > Since dev_A and dev_B share a pasid table, the side effect is that a flawed > dev_A might access the memory space of process B (with pasid y). Vice > versa, > a flawed dev_B might access memory space of process A (with pasid x). > > What PATCH 4~9 do is to remove such possibility by assigning a pasid table > for each pci device. Hence, the remapping infrastructure looks like: > > > .------------------. > | | > .----------------. | | > | | | Paging structure | > .----------------. | for Process A | > | PASID X | | | > .----------------.----->'------------------' > | | > .----------------. > | | > .----------------. > | | > .----------------. > .----------------. | | > | | .----------------. > .----------------. | | > | Dev_A context |------>'----------------' > '----------------' pasid table > | | for Dev_A > '----------------' > | Dev_B context |--> > '----------------' | .----------------. > | | | | | .------------------. > .----------------. | .----------------. | | > | | | | | | | > '----------------' | .----------------. | Paging structure | > Intel iommu | | | | for Process B | > context table | .----------------. | | > | | PASID Y |----->'------------------' > | .----------------. > | | | > | .----------------. > | | | > | .----------------. > v--->| | > '----------------' > pasid table > for Dev_B > > > With this, dev_A has no means to access memory of process B and vice > versa. > > Best regards, > Lu Baolu
| |