Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management

[Lu Baolu]

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.

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