>-----Original Message-----
>From: Eric Auger <eric.au...@redhat.com>
>Subject: Re: [PATCH v5 11/21] intel_iommu: Handle PASID entry removal and
>update
snip...
>> +
>> +/*
>> + * This function is a loop function which return value determines if
>whose returned value determines whether current vtd_as iterator matches
>the pasid cache entry info passed in user_data and needs to be removed
>from the pasid cache.
Will do.
>> + * vtd_as including cached pasid entry is removed.
>> + *
>> + * For PCI_NO_PASID, when corresponding cached pasid entry is cleared,
>> + * it returns false so that vtd_as is reserved as it's owned by PCI
>> + * sub-system. For other pasid, it returns true so vtd_as is removed.
>> + */
>> +static gboolean vtd_flush_pasid_locked(gpointer key, gpointer value,
>> + gpointer user_data)
>> +{
>> + VTDPASIDCacheInfo *pc_info = user_data;
>> + VTDAddressSpace *vtd_as = value;
>> + VTDPASIDCacheEntry *pc_entry = &vtd_as->pasid_cache_entry;
>> + VTDPASIDEntry pe;
>> + uint16_t did;
>> + uint32_t pasid;
>> + int ret;
>> +
>> + if (!pc_entry->valid) {
>> + return false;
>> + }
>> + did = VTD_SM_PASID_ENTRY_DID(&pc_entry->pasid_entry);
>> +
>> + if (vtd_as_to_iommu_pasid_locked(vtd_as, &pasid)) {
>> + goto remove;
>> + }
>> +
>> + switch (pc_info->type) {
>> + case VTD_PASID_CACHE_PASIDSI:
>> + if (pc_info->pasid != pasid) {
>> + return false;
>> + }
>> + /* fall through */
>> + case VTD_PASID_CACHE_DOMSI:
>> + if (pc_info->did != did) {
>> + return false;
>> + }
>> + /* fall through */
>> + case VTD_PASID_CACHE_GLOBAL_INV:
>> + break;
>> + default:
>> + error_setg(&error_fatal, "invalid pc_info->type for flush");
>> + }
>> +
>> + /*
>> + * pasid cache invalidation may indicate a present pasid entry to
>present
>> + * pasid entry modification. To cover such case, vIOMMU emulator
>needs to
>> + * fetch latest guest pasid entry and compares with cached pasid
>entry,
>> + * then update pasid cache.
>> + */
>> + ret = vtd_dev_get_pe_from_pasid(vtd_as, pasid, &pe);
>> + if (ret) {
>> + /*
>> + * No valid pasid entry in guest memory. e.g. pasid entry was
>modified
>> + * to be either all-zero or non-present. Either case means
>existing
>> + * pasid cache should be removed.
>> + */
>> + goto remove;
>> + }
>> +
>> + /*
>> + * Update cached pasid entry if it's stale compared to what's in guest
>> + * memory.
>> + */
>> + if (!vtd_pasid_entry_compare(&pe, &pc_entry->pasid_entry)) {
>> + pc_entry->pasid_entry = pe;
>> + }
>> + return false;
>> +
>> +remove:
>> + pc_entry->valid = false;
>> +
>> + /*
>> + * Don't remove address space of PCI_NO_PASID which is created for
>PCI
>> + * sub-system.
>> + */
>> + if (vtd_as->pasid == PCI_NO_PASID) {
>> + return false;
>> + }
>> + return true;
>> +}
>> +
>> +/*
>> + * For a PASID cache invalidation, this function handles below scenarios:
>> + * a) a present cached pasid entry needs to be removed
>> + * b) a present cached pasid entry needs to be updated
>> + */
>> +static void vtd_pasid_cache_sync(IntelIOMMUState *s,
>VTDPASIDCacheInfo *pc_info)
>> +{
>> + if (!s->flts || !s->root_scalable || !s->dmar_enabled) {
>> + return;
>> + }
>> +
>> + vtd_iommu_lock(s);
>> + /*
>> + * a,b): loop all the existing vtd_as instances for pasid cache removal
>> + or update.
>> + */
>> + g_hash_table_foreach_remove(s->vtd_address_spaces,
>vtd_flush_pasid_locked,
>> + pc_info);
>> + vtd_iommu_unlock(s);
>> +}
>> +
>> +static bool vtd_process_pasid_desc(IntelIOMMUState *s,
>> + VTDInvDesc *inv_desc)
>> +{
>> + uint16_t did;
>> + uint32_t pasid;
>> + VTDPASIDCacheInfo pc_info;
>> + uint64_t mask[4] = {VTD_INV_DESC_PASIDC_RSVD_VAL0,
>VTD_INV_DESC_ALL_ONE,
>> + VTD_INV_DESC_ALL_ONE,
>VTD_INV_DESC_ALL_ONE};
>> +
>> + if (!vtd_inv_desc_reserved_check(s, inv_desc, mask, true,
>> + __func__, "pasid cache inv"))
>{
>> + return false;
>> + }
>> +
>> + did = VTD_INV_DESC_PASIDC_DID(inv_desc);
>> + pasid = VTD_INV_DESC_PASIDC_PASID(inv_desc);
>> +
>> + switch (VTD_INV_DESC_PASIDC_G(inv_desc)) {
>> + case VTD_INV_DESC_PASIDC_G_DSI:
>> + trace_vtd_pasid_cache_dsi(did);
>> + pc_info.type = VTD_PASID_CACHE_DOMSI;
>> + pc_info.did = did;
>> + break;
>> +
>> + case VTD_INV_DESC_PASIDC_G_PASID_SI:
>> + /* PASID selective implies a DID selective */
>> + trace_vtd_pasid_cache_psi(did, pasid);
>> + pc_info.type = VTD_PASID_CACHE_PASIDSI;
>> + pc_info.did = did;
>> + pc_info.pasid = pasid;
>> + break;
>> +
>> + case VTD_INV_DESC_PASIDC_G_GLOBAL:
>> + trace_vtd_pasid_cache_gsi();
>> + pc_info.type = VTD_PASID_CACHE_GLOBAL_INV;
>> + break;
>> +
>> + default:
>> + error_report_once("invalid granularity field in PASID-cache
>invalidate "
>> + "descriptor, hi: 0x%"PRIx64" lo: 0x%"
>PRIx64,
>> + inv_desc->val[1], inv_desc->val[0]);
>what's the point of printing the 2nd 64b? Looking at Figure 6-2 in the
>spec (6.5.2.2. PASID-cache invalidate descriptor) it does not seem to
>contain anything?
I think it's a tradition in intel_iommu.c to print hi and low for 128bit or
val[3-0] for 256bit inv_desc, even though hi may be reserved.
>
>Besides I read in the spec:
>Domain-ID (DID): The DID field indicates the target domain-id. Hardware
>ignores bits 31:(16+N), where N is the domain-id width reported in the
>Capability Register.
>
>How do you make sure N is same on both pIOMMU and vIOMMU?
There is no relationship between pIOMMU's and vIOMMU's DID. host and guest
kernel manage their DID separately.
Thanks
Zhenzhong
