Re: [PATCH V3 00/29]

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Sep 21, 2017 at 11:01:41PM -0400, Lan Tianyu wrote:
> Change since v2:
>        1) Remove vIOMMU hypercall of query capabilities and introduce when necessary.
>        2) Remove length field of vIOMMU create parameter of vIOMMU hypercall
>        3) Introduce irq remapping mode callback to vIOMMU framework and vIOMMU device models
> can check irq remapping mode by vendor specific ways.
>        4) Update vIOMMU docs.
>        5) Other changes please see patches' change logs.
> 
> Change since v1:
>        1) Fix coding style issues
>        2) Add definitions for vIOMMU type and capabilities
>        3) Change vIOMMU kconfig and select vIOMMU default on x86
>        4) Put vIOMMU creation in libxl__arch_domain_create()
>        5) Make vIOMMU structure of tool stack more general for both PV and HVM.
> 
> Change since RFC v2:
>        1) Move vvtd.c to drivers/passthrough/vtd directroy. 
>        2) Make vIOMMU always built in on x86
>        3) Add new boot cmd "viommu" to enable viommu function
>        4) Fix some code stype issues.
> 
> Change since RFC v1:
>        1) Add Xen virtual IOMMU doc docs/misc/viommu.txt
>        2) Move vIOMMU hypercall of create/destroy vIOMMU and query  
> capabilities from dmop to domctl suggested by Paul Durrant. Because
> these hypercalls can be done in tool stack and more VM mode(E,G PVH
> or other modes don't use Qemu) can be benefit.
>        3) Add check of input MMIO address and length.
>        4) Add iommu_type in vIOMMU hypercall parameter to specify
> vendor vIOMMU device model(E,G Intel VTD, AMD or ARM IOMMU. So far
> only support Intel VTD).
>        5) Add save and restore support for vvtd
> 
> 
> This patchset is to introduce vIOMMU framework and add virtual VTD's
> interrupt remapping support according "Xen virtual IOMMU high level
> design doc V3"(https://lists.xenproject.org/archives/html/xen-devel/
> 2016-11/msg01391.html).
> 
> - vIOMMU framework
> New framework provides viommu_ops and help functions to abstract
> vIOMMU operations(E,G create, destroy, handle irq remapping request
> and so on). Vendors(Intel, ARM, AMD and son) can implement their
> vIOMMU callbacks.
> 
> - Virtual VTD
> We enable irq remapping function and covers both
> MSI and IOAPIC interrupts. Don't support post interrupt mode emulation
> and post interrupt mode enabled on host with virtual VTD. will add
> later.

Hello,

Just a couple of generic comments on the whole series:

 - Please make sure that the result after each patch is buildable. It
   is of extreme importance the that Xen tree is bisectable at all
   points.

 - Regarding the organization of the series, I would rather prefer
   that you place the design document at the beginning (like it's done
   now), then the hypervisor changes (possibly the generic framework
   first, then the vvtd functionality and finally all the hooks into
   common code) and the toolstack side at the end. This might be just
   my personal taste, but I think it's clearer to review/understand
   rather than mixed as it is now.

 - Finally, please try to make sure that each patch introduces the
   helpers or structures that it needs. For example don't place all
   the "static inline" helpers together with a bunch of structures in
   an isolated patch, and then a bunch of patches that start making
   use of them. Instead introduce the structures or helpers in the
   context when they are used. An exception of this might be for very
   big or generic structures.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel