On Wed, Jan 05, 2022 at 12:19:45PM +0800, Jason Wang wrote:
> @@ -1725,11 +1780,16 @@ static bool vtd_do_iommu_translate(VTDAddressSpace
> *vtd_as, PCIBus *bus,
> cc_entry->context_cache_gen = s->context_cache_gen;
> }
>
> + /* Try to fetch slpte form IOTLB */
> + if ((pasid == PCI_NO_PASID) && s->root_scalable) {
> + pasid = VTD_CE_GET_RID2PASID(&ce);
> + }
> +
> /*
> * We don't need to translate for pass-through context entries.
> * Also, let's ignore IOTLB caching as well for PT devices.
> */
> - if (vtd_dev_pt_enabled(s, &ce)) {
> + if (vtd_dev_pt_enabled(s, &ce, pasid)) {
> entry->iova = addr & VTD_PAGE_MASK_4K;
> entry->translated_addr = entry->iova;
> entry->addr_mask = ~VTD_PAGE_MASK_4K;
> @@ -1750,14 +1810,24 @@ static bool vtd_do_iommu_translate(VTDAddressSpace
> *vtd_as, PCIBus *bus,
> return true;
> }
>
> + iotlb_entry = vtd_lookup_iotlb(s, source_id, addr, pasid);
> + if (iotlb_entry) {
> + trace_vtd_iotlb_page_hit(source_id, addr, iotlb_entry->slpte,
> + iotlb_entry->domain_id);
> + slpte = iotlb_entry->slpte;
> + access_flags = iotlb_entry->access_flags;
> + page_mask = iotlb_entry->mask;
> + goto out;
> + }
IIUC the iotlb lookup moved down just because the pasid==NO_PASID case then
we'll need to fetch the default pasid from the context entry. That looks
reasonable.
It's just a bit of pity because logically it'll slow down iotlb hits due to
context entry operations. When NO_PASID we could have looked up iotlb without
checking pasid at all, assuming that "default pasid" will always match. But
that is a little bit hacky.
vIOMMU seems to be mostly used for assigned devices and dpdk in production in
the future due to its slowness otherwise.. so maybe not a big deal at all.
[...]
> @@ -2011,7 +2083,52 @@ static void vtd_iotlb_page_invalidate(IntelIOMMUState
> *s, uint16_t domain_id,
> vtd_iommu_lock(s);
> g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_page, &info);
> vtd_iommu_unlock(s);
> - vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am);
> + vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am, PCI_NO_PASID);
> +}
> +
> +static void vtd_iotlb_page_pasid_invalidate(IntelIOMMUState *s,
> + uint16_t domain_id,
> + hwaddr addr, uint8_t am,
> + uint32_t pasid)
> +{
> + VTDIOTLBPageInvInfo info;
> +
> + trace_vtd_inv_desc_iotlb_pasid_pages(domain_id, addr, am, pasid);
> +
> + assert(am <= VTD_MAMV);
> + info.domain_id = domain_id;
> + info.addr = addr;
> + info.mask = ~((1 << am) - 1);
> + info.pasid = pasid;
> + vtd_iommu_lock(s);
> + g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_page_pasid,
> &info);
> + vtd_iommu_unlock(s);
> + vtd_iotlb_page_invalidate_notify(s, domain_id, addr, am, pasid);
Hmm, I think indeed we need a notification, but it'll be unnecessary for
e.g. vfio map notifiers, because this is 1st level invalidation and at least so
far vfio map notifiers are rewalking only the 2nd level page table, so it'll be
destined to be a no-op and pure overhead.
> +}
> +
> +static void vtd_iotlb_pasid_invalidate(IntelIOMMUState *s, uint16_t
> domain_id,
> + uint32_t pasid)
> +{
> + VTDIOTLBPageInvInfo info;
> + VTDAddressSpace *vtd_as;
> + VTDContextEntry ce;
> +
> + trace_vtd_inv_desc_iotlb_pasid(domain_id, pasid);
> +
> + info.domain_id = domain_id;
> + info.pasid = pasid;
> + vtd_iommu_lock(s);
> + g_hash_table_foreach_remove(s->iotlb, vtd_hash_remove_by_pasid, &info);
> + vtd_iommu_unlock(s);
> +
> + QLIST_FOREACH(vtd_as, &s->vtd_as_with_notifiers, next) {
> + if (!vtd_dev_to_context_entry(s, pci_bus_num(vtd_as->bus),
> + vtd_as->devfn, &ce) &&
> + domain_id == vtd_get_domain_id(s, &ce, vtd_as->pasid) &&
> + pasid == vtd_as->pasid) {
> + vtd_sync_shadow_page_table(vtd_as);
Do we need to rewalk the shadow pgtable (which is the 2nd level, afaict) even
if we got the 1st level pgtable invalidated?
> + }
> + }
> }
The rest looks mostly good to me; thanks.
--
Peter Xu
