Re: [PATCH v4 10/17] intel_iommu: Process PASID-based iotlb invalidation

[Yi Liu <yi.l.liu@intel.com>]

On 2024/11/4 13:40, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Monday, November 4, 2024 10:51 AM
>> Subject: Re: [PATCH v4 10/17] intel_iommu: Process PASID-based iotlb
>> invalidation
>>
>> On 2024/9/30 17:26, Zhenzhong Duan wrote:
>>> PASID-based iotlb (piotlb) is used during walking Intel
>>> VT-d stage-1 page table.
>>>
>>> This emulates the stage-1 page table iotlb invalidation requested
>>> by a PASID-based IOTLB Invalidate Descriptor (P_IOTLB).
>>>
>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>
>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>> ---
>>>    hw/i386/intel_iommu_internal.h |  3 +++
>>>    hw/i386/intel_iommu.c          | 45 ++++++++++++++++++++++++++++++++++
>>>    2 files changed, 48 insertions(+)
>>>
>>> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
>>> index 4c3e75e593..20d922d600 100644
>>> --- a/hw/i386/intel_iommu_internal.h
>>> +++ b/hw/i386/intel_iommu_internal.h
>>> @@ -453,6 +453,9 @@ typedef union VTDInvDesc VTDInvDesc;
>>>    #define VTD_INV_DESC_PIOTLB_PSI_IN_PASID  (3ULL << 4)
>>>    #define VTD_INV_DESC_PIOTLB_DID(val)      (((val) >> 16) &
>> VTD_DOMAIN_ID_MASK)
>>>    #define VTD_INV_DESC_PIOTLB_PASID(val)    (((val) >> 32) & 0xfffffULL)
>>> +#define VTD_INV_DESC_PIOTLB_AM(val)       ((val) & 0x3fULL)
>>> +#define VTD_INV_DESC_PIOTLB_IH(val)       (((val) >> 6) & 0x1)
>>> +#define VTD_INV_DESC_PIOTLB_ADDR(val)     ((val) & ~0xfffULL)
>>>    #define VTD_INV_DESC_PIOTLB_RSVD_VAL0     0xfff000000000f1c0ULL
>>>    #define VTD_INV_DESC_PIOTLB_RSVD_VAL1     0xf80ULL
>>>
>>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>>> index 46bde1ad40..289278ce30 100644
>>> --- a/hw/i386/intel_iommu.c
>>> +++ b/hw/i386/intel_iommu.c
>>> @@ -322,6 +322,28 @@ static gboolean vtd_hash_remove_by_page(gpointer
>> key, gpointer value,
>>>        return (entry->gfn & info->mask) == gfn || entry->gfn == gfn_tlb;
>>>    }
>>>
>>> +static gboolean vtd_hash_remove_by_page_piotlb(gpointer key, gpointer
>> value,
>>> +                                               gpointer user_data)
>>> +{
>>> +    VTDIOTLBEntry *entry = (VTDIOTLBEntry *)value;
>>> +    VTDIOTLBPageInvInfo *info = (VTDIOTLBPageInvInfo *)user_data;
>>> +    uint64_t gfn = (info->addr >> VTD_PAGE_SHIFT_4K) & info->mask;
>>> +    uint64_t gfn_tlb = (info->addr & entry->mask) >> VTD_PAGE_SHIFT_4K;
>>> +
>>> +    /*
>>> +     * According to spec, PASID-based-IOTLB Invalidation in page granularity
>>> +     * doesn't invalidate IOTLB entries caching second-stage (PGTT=010b)
>>> +     * or pass-through (PGTT=100b) mappings. Nested isn't supported yet,
>>> +     * so only need to check first-stage (PGTT=001b) mappings.
>>> +     */
>>> +    if (entry->pgtt != VTD_SM_PASID_ENTRY_FLT) {
>>> +        return false;
>>> +    }
>>> +
>>> +    return entry->domain_id == info->domain_id && entry->pasid == info->pasid
>> &&
>>> +           ((entry->gfn & info->mask) == gfn || entry->gfn == gfn_tlb);
>>> +}
>>> +
>>>    /* Reset all the gen of VTDAddressSpace to zero and set the gen of
>>>     * IntelIOMMUState to 1.  Must be called with IOMMU lock held.
>>>     */
>>> @@ -2884,11 +2906,30 @@ static void
>> vtd_piotlb_pasid_invalidate(IntelIOMMUState *s,
>>>        }
>>>    }
>>>
>>> +static void vtd_piotlb_page_invalidate(IntelIOMMUState *s, uint16_t
>> domain_id,
>>> +                                       uint32_t pasid, hwaddr addr, uint8_t am,
>>> +                                       bool ih)
>>
>> @ih is not used, perhaps you can drop it. Seems like we don't cache paging
>> structure, hence ih can be ignored so far. Besides this, the patch looks
>> good to me.
> 
> OK, will drop it. But nesting series needs it, see below.
> I'll drop it in this series and add back in nesting series.

yep, you can add it back when it's going to be used. :)

-- 
Regards,
Yi Liu