Hi zhenzhong,
On 23/12/2024 06:50, 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.
>
>
> Hi Clement,
>
>> -----Original Message-----
>> From: CLEMENT MATHIEU--DRIF <clement.mathieu--drif@eviden.com>
>> Subject: [PATCH v1 04/19] intel_iommu: Fill the PASID field when creating an
>> IOMMUTLBEntry
>>
>> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>>
>> PASID value must be used by devices as a key (or part of a key)
>> when populating their ATC with the IOTLB entries returned by the IOMMU.
>>
>> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com>
>> ---
>> hw/i386/intel_iommu.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index aad132e367..a92ef9fe74 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -2119,6 +2119,9 @@ static bool vtd_do_iommu_translate(VTDAddressSpace
>> *vtd_as, PCIBus *bus,
>>
>> vtd_iommu_lock(s);
>>
>> + /* fill the pasid before getting rid2pasid */
>> + entry->pasid = pasid;
> Seems unnecessary because vtd_iommu_translate() already assigned it.
You are right
>
>> +
>> cc_entry = &vtd_as->context_cache_entry;
>>
>> /* Try to fetch pte from IOTLB, we don't need RID2PASID logic */
>> @@ -2260,6 +2263,7 @@ out:
>> entry->translated_addr = vtd_get_pte_addr(pte, s->aw_bits) & page_mask;
>> entry->addr_mask = ~page_mask;
>> entry->perm = access_flags;
>> + /* pasid already set */
>> return true;
>>
>> error:
>> @@ -2268,6 +2272,7 @@ error:
>> entry->translated_addr = 0;
>> entry->addr_mask = 0;
>> entry->perm = IOMMU_NONE;
>> + entry->pasid = PCI_NO_PASID;
> Shouldn't we keep original pasid value?
will do
>
>> return false;
>> }
>>
>> @@ -2511,6 +2516,7 @@ static void
>> vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s,
>> .translated_addr = 0,
>> .addr_mask = size - 1,
>> .perm = IOMMU_NONE,
>> + .pasid = pasid,
> Shouldn't we assign vtd_dev_as->pasid?
will do
>
>> },
>> };
>> memory_region_notify_iommu(&vtd_as->iommu, 0, event);
>> @@ -3098,6 +3104,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);
>> }
>>
>> @@ -3680,6 +3687,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;
>>
>> @@ -3692,6 +3700,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;
> Shouldn't we keep earlier assigned pasid value as above?
will do
>
> Thanks
> Zhenzhong
I'll wait for more reviews before sending a v2
Thanks
cmd