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

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

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.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

> +{
> +    VTDIOTLBPageInvInfo info;
> +
> +    info.domain_id = domain_id;
> +    info.pasid = pasid;
> +    info.addr = addr;
> +    info.mask = ~((1 << am) - 1);
> +
> +    vtd_iommu_lock(s);
> +    g_hash_table_foreach_remove(s->iotlb,
> +                                vtd_hash_remove_by_page_piotlb, &info);
> +    vtd_iommu_unlock(s);
> +}
> +
>   static bool vtd_process_piotlb_desc(IntelIOMMUState *s,
>                                       VTDInvDesc *inv_desc)
>   {
>       uint16_t domain_id;
>       uint32_t pasid;
> +    uint8_t am;
> +    hwaddr addr;
>   
>       if ((inv_desc->val[0] & VTD_INV_DESC_PIOTLB_RSVD_VAL0) ||
>           (inv_desc->val[1] & VTD_INV_DESC_PIOTLB_RSVD_VAL1) ||
> @@ -2909,6 +2950,10 @@ static bool vtd_process_piotlb_desc(IntelIOMMUState *s,
>           break;
>   
>       case VTD_INV_DESC_PIOTLB_PSI_IN_PASID:
> +        am = VTD_INV_DESC_PIOTLB_AM(inv_desc->val[1]);
> +        addr = (hwaddr) VTD_INV_DESC_PIOTLB_ADDR(inv_desc->val[1]);
> +        vtd_piotlb_page_invalidate(s, domain_id, pasid, addr, am,
> +                                   VTD_INV_DESC_PIOTLB_IH(inv_desc->val[1]));
>           break;
>   
>       default:

-- 
Regards,
Yi Liu