On Thu, Dec 22, 2016 at 07:34:10PM +0800, Jason Wang wrote:
>
>
> On 2016年12月22日 19:04, Peter Xu wrote:
> >On Thu, Dec 22, 2016 at 05:52:58PM +0800, Jason Wang wrote:
> >>
> >>On 2016年12月22日 17:48, Peter Xu wrote:
> >>> /* Handle Translation Enable/Disable */
> >>> static void vtd_handle_gcmd_te(IntelIOMMUState *s, bool en)
> >>> {
> >>>+ if (s->dmar_enabled == en) {
> >>>+ return;
> >>>+ }
> >>>+
> >>> VTD_DPRINTF(CSR, "Translation Enable %s", (en ? "on" : "off"));
> >>> if (en) {
> >>>@@ -1196,6 +1237,8 @@ static void vtd_handle_gcmd_te(IntelIOMMUState *s,
> >>>bool en)
> >>> /* Ok - report back to driver */
> >>> vtd_set_clear_mask_long(s, DMAR_GSTS_REG, VTD_GSTS_TES, 0);
> >>> }
> >>>+
> >>>+ vtd_switch_address_space_all(s, en);
> >>> }
> >>We may need something like notifier here to tell e.g vhost to stop device
> >>IOTLB. (Since it's likely this series were applied after device IOTLB
> >>patches)
> >Yes, I missed vhost case.
> >
> >To notify vhost, IMO we should be able to use memory listeners just
> >like how vfio devices do (please refer to vfio_memory_listener).
>
> Just for switching? This seems an overkill since we don't depends on it for
> all other things. Guest should setup correct mappings by explicitly notify
> device IOTLB. A quick glance at ATS spec, for enabling and disabling, looks
> like it was done through enable bit of ASTctl instead of here.
>
> So we are probably ok here :)
So we need a more general way to notify ATS about this, right? (e.g.,
for future devices that support ATS as well, not only vhost)
>
> >However, I think the bigger issue is we still don't have a dynamic way
> >to turn vhost DMAR on/off, right?
>
> The API was ready for this, just issue another set_feature ioctl without
> IOMMU_PLATFORM. (But unfortunately, vhost need a small patch to make this
> work).
That'll be nice. :)
>
> >
> >If so, we may need to re-touch all the parts to enable the dynamic
> >switching of DMA remapping - QEMU vhost, kernel vhost, and virtio on
> >the guest side... I start to doubt whether that effort will worth it
> >due to this single change, especially if this feature (dynamic on/off
> >DMA remapping) won't be used by most VMs (i.e., Linux should only turn
> >VT-d on when kernel detects it, and should never turn it off in
> >anyway).
>
> For vhost part, the changes should be very minor, probably just:
>
> - a patch to make sure vhost can disable device IOTLB during feature set
> - properly handling enabling bit of ATSctl in qemu (probably through an
> notifier)
Do you think I should provide another patch for the domain switch
notification (along with this one)? Or I can just postpone this patch
until DMAR series merged (after all this patch is not an urgent one).
>
> >
> >(However I do think this is an improvement to current VT-d though)
> >
> >Thanks,
> >
> >-- peterx
> >
>
> +1. We should try to emulate exactly what hardware does to avoid breaking
> all kinds of guest or userspace drivers.
Yes. Thanks,
-- peterx
