diff mbox series

[ats_vtd,v2,20/25] intel_iommu: fill the PASID field when creating an instance of IOMMUTLBEntry

Message ID 20240515071057.33990-21-clement.mathieu--drif@eviden.com (mailing list archive)
State New, archived
Headers show
Series ATS support for VT-d | expand

Commit Message

CLEMENT MATHIEU--DRIF May 15, 2024, 7:14 a.m. UTC 
Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
---
 hw/i386/intel_iommu.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Duan, Zhenzhong May 17, 2024, 10:40 a.m. UTC | #1 
>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>Subject: [PATCH ats_vtd v2 20/25] intel_iommu: fill the PASID field when
>creating an instance of IOMMUTLBEntry
>
>Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>---
> hw/i386/intel_iommu.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>index 53f17d66c0..c4ebd4569e 100644
>--- a/hw/i386/intel_iommu.c
>+++ b/hw/i386/intel_iommu.c
>@@ -2299,6 +2299,7 @@ out:
>     entry->translated_addr = vtd_get_slpte_addr(pte, s->aw_bits) &
>page_mask;
>     entry->addr_mask = ~page_mask;
>     entry->perm = access_flags;
>+    entry->pasid = pasid;

For PCI_NO_PASID, do we want to assign PCI_NO_PASID or rid2pasid?

Thanks
Zhenzhong

>     return true;
>
> error:
>@@ -2307,6 +2308,7 @@ error:
>     entry->translated_addr = 0;
>     entry->addr_mask = 0;
>     entry->perm = IOMMU_NONE;
>+    entry->pasid = PCI_NO_PASID;
>     return false;
> }
>
>@@ -3497,6 +3499,7 @@ static void
>vtd_piotlb_pasid_invalidate_notify(IntelIOMMUState *s,
>                 event.entry.target_as = &address_space_memory;
>                 event.entry.iova = notifier->start;
>                 event.entry.perm = IOMMU_NONE;
>+                event.entry.pasid = pasid;
>                 event.entry.addr_mask = notifier->end - notifier->start;
>                 event.entry.translated_addr = 0;
>
>@@ -3678,6 +3681,7 @@ static void
>vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
>             event.entry.target_as = &address_space_memory;
>             event.entry.iova = addr;
>             event.entry.perm = IOMMU_NONE;
>+            event.entry.pasid = pasid;
>             event.entry.addr_mask = size - 1;
>             event.entry.translated_addr = 0;
>
>@@ -4335,6 +4339,7 @@ static void
>do_invalidate_device_tlb(VTDAddressSpace *vtd_dev_as,
>     event.entry.iova = addr;
>     event.entry.perm = IOMMU_NONE;
>     event.entry.translated_addr = 0;
>+    event.entry.pasid = vtd_dev_as->pasid;
>     memory_region_notify_iommu(&vtd_dev_as->iommu, 0, event);
> }
>
>@@ -4911,6 +4916,7 @@ static IOMMUTLBEntry
>vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>     IOMMUTLBEntry iotlb = {
>         /* We'll fill in the rest later. */
>         .target_as = &address_space_memory,
>+        .pasid = vtd_as->pasid,
>     };
>     bool success;
>
>@@ -4923,6 +4929,7 @@ static IOMMUTLBEntry
>vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>         iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
>         iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
>         iotlb.perm = IOMMU_RW;
>+        iotlb.pasid = PCI_NO_PASID;
>         success = true;
>     }
>
>--
>2.44.0
CLEMENT MATHIEU--DRIF May 17, 2024, 11:11 a.m. UTC | #2 
On 17/05/2024 12:40, Duan, Zhenzhong wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
>> -----Original Message-----
>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>> Subject: [PATCH ats_vtd v2 20/25] intel_iommu: fill the PASID field when
>> creating an instance of IOMMUTLBEntry
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> ---
>> hw/i386/intel_iommu.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 53f17d66c0..c4ebd4569e 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -2299,6 +2299,7 @@ out:
>>      entry->translated_addr = vtd_get_slpte_addr(pte, s->aw_bits) &
>> page_mask;
>>      entry->addr_mask = ~page_mask;
>>      entry->perm = access_flags;
>> +    entry->pasid = pasid;
> For PCI_NO_PASID, do we want to assign PCI_NO_PASID or rid2pasid?
we have the following statement a few lines above :
if (rid2pasid) {
         pasid = VTD_CE_GET_RID2PASID(&ce);
}

so we store rid2pasid if the feature is enabled.

But maybe we should store PCI_NO_PASID because the rest of the world is 
not supposed to be aware of what we are doing with rid2pasid.

Does it look good to you?
>
> Thanks
> Zhenzhong
>
>>      return true;
>>
>> error:
>> @@ -2307,6 +2308,7 @@ error:
>>      entry->translated_addr = 0;
>>      entry->addr_mask = 0;
>>      entry->perm = IOMMU_NONE;
>> +    entry->pasid = PCI_NO_PASID;
>>      return false;
>> }
>>
>> @@ -3497,6 +3499,7 @@ static void
>> vtd_piotlb_pasid_invalidate_notify(IntelIOMMUState *s,
>>                  event.entry.target_as = &address_space_memory;
>>                  event.entry.iova = notifier->start;
>>                  event.entry.perm = IOMMU_NONE;
>> +                event.entry.pasid = pasid;
>>                  event.entry.addr_mask = notifier->end - notifier->start;
>>                  event.entry.translated_addr = 0;
>>
>> @@ -3678,6 +3681,7 @@ static void
>> vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
>>              event.entry.target_as = &address_space_memory;
>>              event.entry.iova = addr;
>>              event.entry.perm = IOMMU_NONE;
>> +            event.entry.pasid = pasid;
>>              event.entry.addr_mask = size - 1;
>>              event.entry.translated_addr = 0;
>>
>> @@ -4335,6 +4339,7 @@ static void
>> do_invalidate_device_tlb(VTDAddressSpace *vtd_dev_as,
>>      event.entry.iova = addr;
>>      event.entry.perm = IOMMU_NONE;
>>      event.entry.translated_addr = 0;
>> +    event.entry.pasid = vtd_dev_as->pasid;
>>      memory_region_notify_iommu(&vtd_dev_as->iommu, 0, event);
>> }
>>
>> @@ -4911,6 +4916,7 @@ static IOMMUTLBEntry
>> vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>>      IOMMUTLBEntry iotlb = {
>>          /* We'll fill in the rest later. */
>>          .target_as = &address_space_memory,
>> +        .pasid = vtd_as->pasid,
>>      };
>>      bool success;
>>
>> @@ -4923,6 +4929,7 @@ static IOMMUTLBEntry
>> vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>>          iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
>>          iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
>>          iotlb.perm = IOMMU_RW;
>> +        iotlb.pasid = PCI_NO_PASID;
>>          success = true;
>>      }
>>
>> --
>> 2.44.0
Duan, Zhenzhong May 21, 2024, 3:11 a.m. UTC | #3 
>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>Subject: Re: [PATCH ats_vtd v2 20/25] intel_iommu: fill the PASID field when
>creating an instance of IOMMUTLBEntry
>
>
>On 17/05/2024 12:40, Duan, Zhenzhong wrote:
>> Caution: External email. Do not open attachments or click links, unless this
>email comes from a known sender and you know the content is safe.
>>
>>
>>> -----Original Message-----
>>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>>> Subject: [PATCH ats_vtd v2 20/25] intel_iommu: fill the PASID field when
>>> creating an instance of IOMMUTLBEntry
>>>
>>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--
>drif@eviden.com>
>>> ---
>>> hw/i386/intel_iommu.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index 53f17d66c0..c4ebd4569e 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -2299,6 +2299,7 @@ out:
>>>      entry->translated_addr = vtd_get_slpte_addr(pte, s->aw_bits) &
>>> page_mask;
>>>      entry->addr_mask = ~page_mask;
>>>      entry->perm = access_flags;
>>> +    entry->pasid = pasid;
>> For PCI_NO_PASID, do we want to assign PCI_NO_PASID or rid2pasid?
>we have the following statement a few lines above :
>if (rid2pasid) {
>         pasid = VTD_CE_GET_RID2PASID(&ce);
>}
>
>so we store rid2pasid if the feature is enabled.
>
>But maybe we should store PCI_NO_PASID because the rest of the world is
>not supposed to be aware of what we are doing with rid2pasid.
>
>Does it look good to you?

Yes, that make sense.

>>
>> Thanks
>> Zhenzhong
>>
>>>      return true;
>>>
>>> error:
>>> @@ -2307,6 +2308,7 @@ error:
>>>      entry->translated_addr = 0;
>>>      entry->addr_mask = 0;
>>>      entry->perm = IOMMU_NONE;
>>> +    entry->pasid = PCI_NO_PASID;
>>>      return false;
>>> }
>>>
>>> @@ -3497,6 +3499,7 @@ static void
>>> vtd_piotlb_pasid_invalidate_notify(IntelIOMMUState *s,
>>>                  event.entry.target_as = &address_space_memory;
>>>                  event.entry.iova = notifier->start;
>>>                  event.entry.perm = IOMMU_NONE;
>>> +                event.entry.pasid = pasid;
>>>                  event.entry.addr_mask = notifier->end - notifier->start;
>>>                  event.entry.translated_addr = 0;
>>>
>>> @@ -3678,6 +3681,7 @@ static void
>>> vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
>>>              event.entry.target_as = &address_space_memory;
>>>              event.entry.iova = addr;
>>>              event.entry.perm = IOMMU_NONE;
>>> +            event.entry.pasid = pasid;
>>>              event.entry.addr_mask = size - 1;
>>>              event.entry.translated_addr = 0;
>>>
>>> @@ -4335,6 +4339,7 @@ static void
>>> do_invalidate_device_tlb(VTDAddressSpace *vtd_dev_as,
>>>      event.entry.iova = addr;
>>>      event.entry.perm = IOMMU_NONE;
>>>      event.entry.translated_addr = 0;
>>> +    event.entry.pasid = vtd_dev_as->pasid;
>>>      memory_region_notify_iommu(&vtd_dev_as->iommu, 0, event);
>>> }
>>>
>>> @@ -4911,6 +4916,7 @@ static IOMMUTLBEntry
>>> vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>>>      IOMMUTLBEntry iotlb = {
>>>          /* We'll fill in the rest later. */
>>>          .target_as = &address_space_memory,
>>> +        .pasid = vtd_as->pasid,
>>>      };
>>>      bool success;
>>>
>>> @@ -4923,6 +4929,7 @@ static IOMMUTLBEntry
>>> vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>>>          iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
>>>          iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
>>>          iotlb.perm = IOMMU_RW;
>>> +        iotlb.pasid = PCI_NO_PASID;
>>>          success = true;
>>>      }
>>>
>>> --
>>> 2.44.0
CLEMENT MATHIEU--DRIF May 21, 2024, 5:09 a.m. UTC | #4 
On 21/05/2024 05:11, Duan, Zhenzhong wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
>> -----Original Message-----
>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>> Subject: Re: [PATCH ats_vtd v2 20/25] intel_iommu: fill the PASID field when
>> creating an instance of IOMMUTLBEntry
>>
>>
>> On 17/05/2024 12:40, Duan, Zhenzhong wrote:
>>> Caution: External email. Do not open attachments or click links, unless this
>> email comes from a known sender and you know the content is safe.
>>>
>>>> -----Original Message-----
>>>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>>>> Subject: [PATCH ats_vtd v2 20/25] intel_iommu: fill the PASID field when
>>>> creating an instance of IOMMUTLBEntry
>>>>
>>>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--
>> drif@eviden.com>
>>>> ---
>>>> hw/i386/intel_iommu.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>>> index 53f17d66c0..c4ebd4569e 100644
>>>> --- a/hw/i386/intel_iommu.c
>>>> +++ b/hw/i386/intel_iommu.c
>>>> @@ -2299,6 +2299,7 @@ out:
>>>>       entry->translated_addr = vtd_get_slpte_addr(pte, s->aw_bits) &
>>>> page_mask;
>>>>       entry->addr_mask = ~page_mask;
>>>>       entry->perm = access_flags;
>>>> +    entry->pasid = pasid;
>>> For PCI_NO_PASID, do we want to assign PCI_NO_PASID or rid2pasid?
>> we have the following statement a few lines above :
>> if (rid2pasid) {
>>          pasid = VTD_CE_GET_RID2PASID(&ce);
>> }
>>
>> so we store rid2pasid if the feature is enabled.
>>
>> But maybe we should store PCI_NO_PASID because the rest of the world is
>> not supposed to be aware of what we are doing with rid2pasid.
>>
>> Does it look good to you?
> Yes, that make sense.
ok, will do
>
>>> Thanks
>>> Zhenzhong
>>>
>>>>       return true;
>>>>
>>>> error:
>>>> @@ -2307,6 +2308,7 @@ error:
>>>>       entry->translated_addr = 0;
>>>>       entry->addr_mask = 0;
>>>>       entry->perm = IOMMU_NONE;
>>>> +    entry->pasid = PCI_NO_PASID;
>>>>       return false;
>>>> }
>>>>
>>>> @@ -3497,6 +3499,7 @@ static void
>>>> vtd_piotlb_pasid_invalidate_notify(IntelIOMMUState *s,
>>>>                   event.entry.target_as = &address_space_memory;
>>>>                   event.entry.iova = notifier->start;
>>>>                   event.entry.perm = IOMMU_NONE;
>>>> +                event.entry.pasid = pasid;
>>>>                   event.entry.addr_mask = notifier->end - notifier->start;
>>>>                   event.entry.translated_addr = 0;
>>>>
>>>> @@ -3678,6 +3681,7 @@ static void
>>>> vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
>>>>               event.entry.target_as = &address_space_memory;
>>>>               event.entry.iova = addr;
>>>>               event.entry.perm = IOMMU_NONE;
>>>> +            event.entry.pasid = pasid;
>>>>               event.entry.addr_mask = size - 1;
>>>>               event.entry.translated_addr = 0;
>>>>
>>>> @@ -4335,6 +4339,7 @@ static void
>>>> do_invalidate_device_tlb(VTDAddressSpace *vtd_dev_as,
>>>>       event.entry.iova = addr;
>>>>       event.entry.perm = IOMMU_NONE;
>>>>       event.entry.translated_addr = 0;
>>>> +    event.entry.pasid = vtd_dev_as->pasid;
>>>>       memory_region_notify_iommu(&vtd_dev_as->iommu, 0, event);
>>>> }
>>>>
>>>> @@ -4911,6 +4916,7 @@ static IOMMUTLBEntry
>>>> vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>>>>       IOMMUTLBEntry iotlb = {
>>>>           /* We'll fill in the rest later. */
>>>>           .target_as = &address_space_memory,
>>>> +        .pasid = vtd_as->pasid,
>>>>       };
>>>>       bool success;
>>>>
>>>> @@ -4923,6 +4929,7 @@ static IOMMUTLBEntry
>>>> vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
>>>>           iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
>>>>           iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
>>>>           iotlb.perm = IOMMU_RW;
>>>> +        iotlb.pasid = PCI_NO_PASID;
>>>>           success = true;
>>>>       }
>>>>
>>>> --
>>>> 2.44.0
diff mbox series

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 53f17d66c0..c4ebd4569e 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2299,6 +2299,7 @@  out:
     entry->translated_addr = vtd_get_slpte_addr(pte, s->aw_bits) & page_mask;
     entry->addr_mask = ~page_mask;
     entry->perm = access_flags;
+    entry->pasid = pasid;
     return true;
 
 error:
@@ -2307,6 +2308,7 @@  error:
     entry->translated_addr = 0;
     entry->addr_mask = 0;
     entry->perm = IOMMU_NONE;
+    entry->pasid = PCI_NO_PASID;
     return false;
 }
 
@@ -3497,6 +3499,7 @@  static void vtd_piotlb_pasid_invalidate_notify(IntelIOMMUState *s,
                 event.entry.target_as = &address_space_memory;
                 event.entry.iova = notifier->start;
                 event.entry.perm = IOMMU_NONE;
+                event.entry.pasid = pasid;
                 event.entry.addr_mask = notifier->end - notifier->start;
                 event.entry.translated_addr = 0;
 
@@ -3678,6 +3681,7 @@  static void vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t domain_id,
             event.entry.target_as = &address_space_memory;
             event.entry.iova = addr;
             event.entry.perm = IOMMU_NONE;
+            event.entry.pasid = pasid;
             event.entry.addr_mask = size - 1;
             event.entry.translated_addr = 0;
 
@@ -4335,6 +4339,7 @@  static void do_invalidate_device_tlb(VTDAddressSpace *vtd_dev_as,
     event.entry.iova = addr;
     event.entry.perm = IOMMU_NONE;
     event.entry.translated_addr = 0;
+    event.entry.pasid = vtd_dev_as->pasid;
     memory_region_notify_iommu(&vtd_dev_as->iommu, 0, event);
 }
 
@@ -4911,6 +4916,7 @@  static IOMMUTLBEntry vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
     IOMMUTLBEntry iotlb = {
         /* We'll fill in the rest later. */
         .target_as = &address_space_memory,
+        .pasid = vtd_as->pasid,
     };
     bool success;
 
@@ -4923,6 +4929,7 @@  static IOMMUTLBEntry vtd_iommu_translate(IOMMUMemoryRegion *iommu, hwaddr addr,
         iotlb.translated_addr = addr & VTD_PAGE_MASK_4K;
         iotlb.addr_mask = ~VTD_PAGE_MASK_4K;
         iotlb.perm = IOMMU_RW;
+        iotlb.pasid = PCI_NO_PASID;
         success = true;
     }