qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3] intel_iommu: allow dynamic switch of IOMMU r


From: Jason Wang
Subject: Re: [Qemu-devel] [PATCH v3] intel_iommu: allow dynamic switch of IOMMU region
Date: Mon, 26 Dec 2016 10:45:55 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1



On 2016年12月23日 11:26, Peter Xu wrote:
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)

Yes, if we want to make ATS visible to guest. But looks like it needs more e.g VFIO support for device IOTLB invalidation.


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).

My understanding is, at least device IOTLB does not care about this. So we can leave it as is.

Thanks



reply via email to

[Prev in Thread] Current Thread [Next in Thread]