Re: [Qemu-devel] [PATCH 7/7] intel_iommu: support passthrough (PT)

[Peter Xu <peterx@redhat.com>]

On Thu, Apr 20, 2017 at 01:18:28PM +0800, Jason Wang wrote:
> 
> 
> On 2017年04月18日 12:21, Peter Xu wrote:
> >On Tue, Apr 18, 2017 at 12:00:13PM +0800, Jason Wang wrote:
> >>
> >>On 2017年04月18日 11:50, Peter Xu wrote:
> >>>On Tue, Apr 18, 2017 at 11:23:35AM +0800, Jason Wang wrote:
> >>>>On 2017年04月17日 18:58, Peter Xu wrote:
> >>>[...]
> >>>
> >>>>>+static void vtd_switch_address_space(VTDAddressSpace *as)
> >>>>>+{
> >>>>>+    bool use_iommu;
> >>>>>+
> >>>>>+    assert(as);
> >>>>>+
> >>>>>+    use_iommu = as->iommu_state->dmar_enabled;
> >>>>>+    if (use_iommu) {
> >>>>>+        /* Further checks per-device configuration */
> >>>>>+        use_iommu &= !vtd_dev_pt_enabled(as);
> >>>>>+    }
> >>>>Looks like you can use as->iommu_state->dmar_enabled &&
> >>>>!vtd_dev_pt_enabled(as)
> >>>vtd_dev_pt_enalbed() needs to read the guest memory (starting from
> >>>reading root entry), which is slightly slow. I was trying to avoid
> >>>unecessary reads.
> >>>
> >>>[...]
> >>I think compiler won't go to vtd_dev_pt_enabled() if dmar_enabled is false.
> >You are right. I'll switch.
> >
> >>>>>@@ -991,6 +1058,18 @@ static void vtd_do_iommu_translate(VTDAddressSpace *vtd_as, PCIBus *bus,
> >>>>>          cc_entry->context_cache_gen = s->context_cache_gen;
> >>>>>      }
> >>>>>+    /*
> >>>>>+     * We don't need to translate for pass-through context entries.
> >>>>>+     * Also, let's ignore IOTLB caching as well for PT devices.
> >>>>>+     */
> >>>>>+    if (vtd_ce_get_type(&ce) == VTD_CONTEXT_TT_PASS_THROUGH) {
> >>>>>+        entry->translated_addr = entry->iova;
> >>>>>+        entry->addr_mask = VTD_PAGE_SIZE - 1;
> >>>>>+        entry->perm = IOMMU_RW;
> >>>>>+        trace_vtd_translate_pt(source_id, entry->iova);
> >>>>>+        return;
> >>>>>+    }
> >>>>Several questions here:
> >>>>
> >>>>1) Is this just for vhost?
> >>>No. When caching mode is not enabled, all passthroughed devices should
> >>>be using this path.
> >>Ok, then it looks better to switch the address space if we've found it was
> >>PT?
> >Do you mean to switch in that if() above? Then when invalidate context
> >entry, we switch back if needed?
> 
> Yes.

Sure. Do you mind if I put this into another standalone patch? I see
it an enhancement that can be separated from current one. Thanks,

-- 
Peter Xu