Re: [Xen-devel] [PATCH v3 2/7] docs: Improve documentation and parsing for iommu=

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Jan 16, 2019 at 09:00:45AM +0000, Andrew Cooper wrote:
> Update parse_iommu_param() to uniformly use parse_boolean(), so the sub
> booleans behave like other Xen boolean options.  Reposition the
> custom_param() to avoid a forward declaration of parse_iommu_param().
> 
> Rewrite the command line documentation almost from scratch, including
> far more detail.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks for doing this:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> CC: Juergen Gross <jgross@xxxxxxxx>
> 
> v3:
>  * New
> ---
>  docs/misc/xen-command-line.pandoc | 153 
> ++++++++++++++++++++------------------
>  xen/drivers/passthrough/iommu.c   |  63 +++++-----------
>  2 files changed, 99 insertions(+), 117 deletions(-)
> 
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index 243193d..ab486e0 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1146,104 +1146,113 @@ detection of systems known to misbehave upon 
> accesses to that port.
>  > Default: `new` unless directed-EOI is supported
>  
>  ### iommu
> -> `= List of [ <boolean> | force | required | intremap | intpost | qinval | 
> snoop | sharept | dom0-passthrough | dom0-strict | amd-iommu-perdev-intremap 
> | workaround_bios_bug | igfx | verbose | debug ]`
> +    = List of [ <bool>, verbose, debug, force, required,

Should this be in the form of "..., verbose=<bool>, debug=<bool>,
..."?

So it's in the same format that's used by the dom0 option
documentation in patch 1.

> +                sharept, intremap, intpost,
> +                snoop, qinval, igfx, workaround_bios_bug,
> +                amd-iommu-perdev-intremap,
> +                dom0-{passthrough,strict} ]
>  
> -> Sub-options:
> +    All sub-options are boolean in nature.

Oh, maybe that's enough and you don't need the =<bool> I suggested
above.

>  
> -> `<boolean>`
> +I/O Memory Memory Units perform a function similar to the CPU MMU (hence the
> +name), but typically exist as a discrete device, integrated as part of a PCI
> +Root Complex.  The most common configuration is to have one IOMMU per package
> +(for on-die PCIe devices and directly attached PCIe lanes), and one IOMMU
> +covering the remaining I/O in the system.
>  
> -> Default: `on`
> -
> ->> Control the use of IOMMU(s) in the system.
> -
> -> All other sub-options are of boolean kind and can be prefixed with `no-` to
> -> effect the inverse meaning.
> -
> -> `force` or `required`
> +The functionality in an IOMMU commonly falls into two orthogonal categories:
>  
> -> Default: `false`
> -
> ->> Don't continue booting unless IOMMU support is found and can be 
> initialized
> ->> successfully.
> +1.  DMA remapping which uses a pagetable-like hierarchical structure and maps
> +    I/O Virtual Addresses (DFNs - Device Frame Numbers in Xen's terminology)
> +    to System Physical Addresses (MFNs - Machine Frame Numbers in Xen's
> +    terminology).
>  
> -> `intremap`
> +2.  Interrupt Remapping, which controls incoming Message Signalled Interrupt
> +    requests, including their routing to specific CPUs.
>  
> -> Default: `true`
> +IOMMU functionality can be used either to provide a translation which the
> +hardware device driver isn't aware of (e.g. PCI Passthrough and a native
> +driver inside the guest) or to enforce fine-grained control over the memory
> +and interrupts which a device is attempting to access.
>  
> ->> Control the use of interrupt remapping (DMA remapping will always be 
> enabled
> ->> if IOMMU functionality is enabled).
> +By default, IOMMUs are configured for use if they are available.  An overall
> +boolean (e.g. `iommu=no`) can override this and leave the IOMMUs disabled.
>  
> -> `intpost`
> +*   The `verbose` and `debug` booleans can be used to print additional
> +    diagnostic information.  Neither are active by default.
>  
> -> Default: `false`
> +*   The `force` and `required` booleans are synonymous and, when requested, 
> will
> +    prevent Xen from booting if IOMMUs aren't discovered and enabled
> +    successfully.
>  
> ->> Control the use of interrupt posting, which depends on the availability of
> ->> interrupt remapping.
> -
> -> `qinval` (VT-d)
> -
> -> Default: `true`
> -
> ->> Control the use of Queued Invalidation.
> -
> -> `snoop` (Intel)
> -
> -> Default: `true`
> +*   The `sharept` boolean controls whether the IOMMU pagetables are shared 
> with
> +    the CPU-side HAP pagetables, or allocated separately.  Sharing reduces 
> the
> +    memory overhead, but doesn't work in combination with CPU-side
> +    pagefault-based features, e.g. dirty VRAM tracking when a PCI device is
> +    assigned.
>  
> ->> Control the use of Snoop Control.
> -
> -> `sharept`
> -
> -> Default: `true`
> -
> ->> Control whether CPU and IOMMU page tables should be shared.
> -
> -> `dom0-passthrough`
> -
> -> **WARNING: This command line option is deprecated, and superseded by
> -> _dom0-iommu=passthrough_ - using both options in combination is 
> undefined.**
> -
> -> `dom0-strict`
> +    Due to implementation choices, sharing pagetables doesn't work on AMD
> +    hardware, and this option is ignored.  It is enabled by default on Intel
                                                                         ^ 
compatible

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel