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

[Jason Wang <jasowang@redhat.com>]

On 2017年04月20日 13:28, Peter Xu wrote:
> 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,
>

I don't mind. It can be done on top.

Thanks