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
