* [PATCH 0/2] Ease development with a PVH Xen and XTF PVH dom0 @ 2018-12-20 23:40 Andrew Cooper 2018-12-20 23:40 ` [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= Andrew Cooper 2018-12-20 23:40 ` [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option Andrew Cooper 0 siblings, 2 replies; 18+ messages in thread From: Andrew Cooper @ 2018-12-20 23:40 UTC (permalink / raw) To: Xen-devel; +Cc: Andrew Cooper Andrew Cooper (2): xen/dom0: Improve documentation for dom0= and dom0-iommu= xen/dom0: Add a dom0-iommu=none option docs/misc/xen-command-line.markdown | 109 ++++++++++++++++++++++-------------- xen/arch/x86/dom0_build.c | 6 -- xen/common/kernel.c | 21 +++++++ xen/drivers/passthrough/iommu.c | 6 +- xen/include/xen/lib.h | 7 +++ 5 files changed, 101 insertions(+), 48 deletions(-) -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-20 23:40 [PATCH 0/2] Ease development with a PVH Xen and XTF PVH dom0 Andrew Cooper @ 2018-12-20 23:40 ` Andrew Cooper 2018-12-21 12:08 ` Jan Beulich 2018-12-21 12:08 ` Roger Pau Monné 2018-12-20 23:40 ` [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option Andrew Cooper 1 sibling, 2 replies; 18+ messages in thread From: Andrew Cooper @ 2018-12-20 23:40 UTC (permalink / raw) To: Xen-devel Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Jan Beulich, Roger Pau Monné Update to the latest metadata style, and expand each of the clauses with more information, including applicable CONFIG_* options. Drop the redundant comment beside parse_dom0_param(), to avoid it getting out of sync with the main documentation. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien.grall@arm.com> Please double check for correctness. The text matches my understanding/reading of the code, but some of it is rather subtle going. It occurs to me that: * The choice of dom0 boot mode should in part be derived from the available CONFIG_* options, and ELF notes advertised in the dom0 kernel. * AMD probably needs to gain an `ivmd=` to mirror `rmrr=` on the Intel side, because we know there are other errors in the IVRS table. * Neither of map-{inclusive,reserved} should be active by default, even on Intel hardware, and we should (wherever possible) have quirks like we have for all other firmware screwups. Requiring the user to diagnose/work around firmware problems like this is quite rude. But this is all future work to do. --- docs/misc/xen-command-line.markdown | 103 ++++++++++++++++++++++-------------- xen/arch/x86/dom0_build.c | 6 --- 2 files changed, 62 insertions(+), 47 deletions(-) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 44ee51a..94ee703 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -636,55 +636,76 @@ trace feature is only enabled in debugging builds of Xen. Specify the bit width of the DMA heap. -### dom0 (x86) -> `= List of [ pvh | shadow ]` +### dom0 +> `= List of [ pvh=<bool>, shadow=<bool> ]` -> Sub-options: - -> `pvh` +> Applicability: x86 -> Default: `false` +Controls for how dom0 is constructed on x86 systems. -Flag that makes a dom0 boot in PVHv2 mode. +* The `pvh` boolean controls whether dom0 is constructed as a PV or a PVH + guest. For backwards compatibility, the default is PV. In addition, the + following requirements must be met. -> `shadow` + * The dom0 kernel selected by the boot loader must be capable of the + selected mode. + * For a PV dom0, Xen must have been compiled with `CONFIG_PV` enabled. + * For a PVH dom0, Xen must have been compiled with `CONFIG_HVM` enabled, + and the hardware must have VT-x/SVM extensions available. -> Default: `false` +* The `shadow` boolean is only applicable when dom0 is constructed as a PVH + guest, and controls whether dom0 uses hardware assisted paging, or shadow + paging. The default is HAP when available, and shadow otherwise. -Flag that makes a dom0 use shadow paging. Only works when "pvh" is -enabled. + This option is unavailable when `CONFIG_SHADOW_PAGING` is compiled out. A + PVH dom0 cannot be used if `CONFIG_SHADOW_PAGING` is compiled out, and the + hardware is not HAP-capable. ### dom0-iommu -> `= List of [ passthrough | strict | map-inclusive ]` - -This list of booleans controls the iommu usage by Dom0: - -* `passthrough`: disables DMA remapping for Dom0. Default is `false`. Note that - this option is hard coded to `false` for a PVH Dom0 and any attempt to - overwrite it from the command line is ignored. - -* `strict`: sets up DMA remapping only for the RAM Dom0 actually got assigned. - Default is `false` which means Dom0 will get mappings for all the host - RAM except regions in use by Xen. Note that this option is hard coded to - `true` for a PVH Dom0 and any attempt to overwrite it from the command line - is ignored. - -* `map-inclusive`: sets up DMA remapping for all the non-RAM regions below 4GB - except for unusable ranges. Use this to work around firmware issues providing - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU - accesses for Dom0, with this option all pages up to 4GB, not marked as - unusable in the E820 table, will get a mapping established. Note that this - option is only applicable to a PV Dom0 and is enabled by default on Intel - hardware. - -* `map-reserved`: sets up DMA remapping for all the reserved regions in the - memory map for Dom0. Use this to work around firmware issues providing - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU - accesses for Dom0, all memory regions marked as reserved in the memory map - that don't overlap with any MMIO region from emulated devices will be - identity mapped. This option maps a subset of the memory that would be - mapped when using the `map-inclusive` option. This option is available to all - Dom0 modes and is enabled by default on Intel hardware. +> `= List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>, +> map-reserved=<bool> ]` + +Controls for the dom0 IOMMU setup. + +* The `passthrough` boolean is applicable to x86 PV dom0's only and defaults + to false. It controls whether the IOMMU is fully disabled for devices + belonging to dom0 (`passthrough=1`), or whether the IOMMU is set up with + an identity transform for dom0 (`passthrough=0`) to prevent dom0 from + DMA'ing outside of its permitted areas. + + This option is hardwired to false for x86 PVH dom0's (where a non-identity + transform is required for dom0 to function), and is ignored for ARM. + +* The `strict` boolean is applicable to x86 PV dom0's only and defaults to + false. It controls whether dom0 can have IOMMU mappings for all domain + RAM in the system, or only for its allocated RAM (and grant mappings etc.) + + This option is hardwired to true for x86 PVH dom0's (as RAM belonging to + other domains in the system don't live in a compatible address space), and + is ignored for ARM. + +* The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up DMA + remapping for all non-RAM regions below 4GB except for unusable ranges. + + Typically, some devices in a system use bits of RAM for communication, and + these areas should be listed via RMRR or IVMD entries in the APCI tables, + so Xen can ensure that they are identity-mapped in the IOMMU. However, + some firmware makes mistakes writing its APCI tables, and this option is a + coarse-grain workaround for those errors. + + Where possible, finer grain corrections should be made with the `rmrr=`, + `ivrs_hpet=` or `ivrs_ioapic=` command line options. + + This option is enabled by default on x86 Intel systems, disabled by + default on other x86 systems, and invalid on ARM systems. + +* The `map-reserved` functionality is very similar to `map-inclusive`, but is + applicable to both x86 PV and PVH dom0's, and represents a subset of the + correction by only mapping reserved memory regions rather than all non-RAM + regions. + + This option is enabled by default on x86 Intel systems, disabled by + default on other x86 systems, and invalid on ARM systems. ### dom0\_ioports\_disable (x86) > `= List of <hex>-<hex>` diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c index 54737da..85d4ff2 100644 --- a/xen/arch/x86/dom0_build.c +++ b/xen/arch/x86/dom0_build.c @@ -282,12 +282,6 @@ bool __initdata opt_dom0_shadow; #endif bool __initdata dom0_pvh; -/* - * List of parameters that affect Dom0 creation: - * - * - pvh Create a PVHv2 Dom0. - * - shadow Use shadow paging for Dom0. - */ static int __init parse_dom0_param(const char *s) { const char *ss; -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-20 23:40 ` [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= Andrew Cooper @ 2018-12-21 12:08 ` Jan Beulich 2018-12-21 12:44 ` Andrew Cooper 2018-12-21 12:08 ` Roger Pau Monné 1 sibling, 1 reply; 18+ messages in thread From: Jan Beulich @ 2018-12-21 12:08 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Xen-devel, Roger Pau Monne >>> On 21.12.18 at 00:40, <andrew.cooper3@citrix.com> wrote: > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -636,55 +636,76 @@ trace feature is only enabled in debugging builds of Xen. > > Specify the bit width of the DMA heap. > > -### dom0 (x86) > -> `= List of [ pvh | shadow ]` > +### dom0 > +> `= List of [ pvh=<bool>, shadow=<bool> ]` > > -> Sub-options: > - > -> `pvh` > +> Applicability: x86 Why the new tag, when everything else uses (x86) next to the option name? > ### dom0-iommu > -> `= List of [ passthrough | strict | map-inclusive ]` > - > -This list of booleans controls the iommu usage by Dom0: > - > -* `passthrough`: disables DMA remapping for Dom0. Default is `false`. Note that > - this option is hard coded to `false` for a PVH Dom0 and any attempt to > - overwrite it from the command line is ignored. > - > -* `strict`: sets up DMA remapping only for the RAM Dom0 actually got assigned. > - Default is `false` which means Dom0 will get mappings for all the host > - RAM except regions in use by Xen. Note that this option is hard coded to > - `true` for a PVH Dom0 and any attempt to overwrite it from the command line > - is ignored. > - > -* `map-inclusive`: sets up DMA remapping for all the non-RAM regions below 4GB > - except for unusable ranges. Use this to work around firmware issues providing > - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU > - accesses for Dom0, with this option all pages up to 4GB, not marked as > - unusable in the E820 table, will get a mapping established. Note that this > - option is only applicable to a PV Dom0 and is enabled by default on Intel > - hardware. > - > -* `map-reserved`: sets up DMA remapping for all the reserved regions in the > - memory map for Dom0. Use this to work around firmware issues providing > - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU > - accesses for Dom0, all memory regions marked as reserved in the memory map > - that don't overlap with any MMIO region from emulated devices will be > - identity mapped. This option maps a subset of the memory that would be > - mapped when using the `map-inclusive` option. This option is available to all > - Dom0 modes and is enabled by default on Intel hardware. > +> `= List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>, > +> map-reserved=<bool> ]` > + > +Controls for the dom0 IOMMU setup. > + > +* The `passthrough` boolean is applicable to x86 PV dom0's only and defaults > + to false. It controls whether the IOMMU is fully disabled for devices > + belonging to dom0 (`passthrough=1`), or whether the IOMMU is set up with > + an identity transform for dom0 (`passthrough=0`) to prevent dom0 from > + DMA'ing outside of its permitted areas. > + > + This option is hardwired to false for x86 PVH dom0's (where a non-identity > + transform is required for dom0 to function), and is ignored for ARM. > + > +* The `strict` boolean is applicable to x86 PV dom0's only and defaults to > + false. It controls whether dom0 can have IOMMU mappings for all domain > + RAM in the system, or only for its allocated RAM (and grant mappings etc.) > + > + This option is hardwired to true for x86 PVH dom0's (as RAM belonging to > + other domains in the system don't live in a compatible address space), and > + is ignored for ARM. > + > +* The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up DMA > + remapping for all non-RAM regions below 4GB except for unusable ranges. I don't thinks this is PV-specific, just its default is. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-21 12:08 ` Jan Beulich @ 2018-12-21 12:44 ` Andrew Cooper 2019-01-04 9:07 ` Jan Beulich 0 siblings, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2018-12-21 12:44 UTC (permalink / raw) To: Jan Beulich Cc: Julien Grall, Stefano Stabellini, Wei Liu, Xen-devel, Roger Pau Monne [-- Attachment #1.1: Type: text/plain, Size: 4570 bytes --] On 21/12/2018 12:08, Jan Beulich wrote: >>>> On 21.12.18 at 00:40, <andrew.cooper3@citrix.com> wrote: >> --- a/docs/misc/xen-command-line.markdown >> +++ b/docs/misc/xen-command-line.markdown >> @@ -636,55 +636,76 @@ trace feature is only enabled in debugging builds of Xen. >> >> Specify the bit width of the DMA heap. >> >> -### dom0 (x86) >> -> `= List of [ pvh | shadow ]` >> +### dom0 >> +> `= List of [ pvh=<bool>, shadow=<bool> ]` >> >> -> Sub-options: >> - >> -> `pvh` >> +> Applicability: x86 > Why the new tag, when everything else uses (x86) next to the > option name? See the commit message of c/s a3a99df44e5405d2092ec59087681765fa4cdee7 The problem is with the generated HTML anchors when trying to cross reference the options. > >> ### dom0-iommu >> -> `= List of [ passthrough | strict | map-inclusive ]` >> - >> -This list of booleans controls the iommu usage by Dom0: >> - >> -* `passthrough`: disables DMA remapping for Dom0. Default is `false`. Note that >> - this option is hard coded to `false` for a PVH Dom0 and any attempt to >> - overwrite it from the command line is ignored. >> - >> -* `strict`: sets up DMA remapping only for the RAM Dom0 actually got assigned. >> - Default is `false` which means Dom0 will get mappings for all the host >> - RAM except regions in use by Xen. Note that this option is hard coded to >> - `true` for a PVH Dom0 and any attempt to overwrite it from the command line >> - is ignored. >> - >> -* `map-inclusive`: sets up DMA remapping for all the non-RAM regions below 4GB >> - except for unusable ranges. Use this to work around firmware issues providing >> - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU >> - accesses for Dom0, with this option all pages up to 4GB, not marked as >> - unusable in the E820 table, will get a mapping established. Note that this >> - option is only applicable to a PV Dom0 and is enabled by default on Intel >> - hardware. >> - >> -* `map-reserved`: sets up DMA remapping for all the reserved regions in the >> - memory map for Dom0. Use this to work around firmware issues providing >> - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU >> - accesses for Dom0, all memory regions marked as reserved in the memory map >> - that don't overlap with any MMIO region from emulated devices will be >> - identity mapped. This option maps a subset of the memory that would be >> - mapped when using the `map-inclusive` option. This option is available to all >> - Dom0 modes and is enabled by default on Intel hardware. >> +> `= List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>, >> +> map-reserved=<bool> ]` >> + >> +Controls for the dom0 IOMMU setup. >> + >> +* The `passthrough` boolean is applicable to x86 PV dom0's only and defaults >> + to false. It controls whether the IOMMU is fully disabled for devices >> + belonging to dom0 (`passthrough=1`), or whether the IOMMU is set up with >> + an identity transform for dom0 (`passthrough=0`) to prevent dom0 from >> + DMA'ing outside of its permitted areas. >> + >> + This option is hardwired to false for x86 PVH dom0's (where a non-identity >> + transform is required for dom0 to function), and is ignored for ARM. >> + >> +* The `strict` boolean is applicable to x86 PV dom0's only and defaults to >> + false. It controls whether dom0 can have IOMMU mappings for all domain >> + RAM in the system, or only for its allocated RAM (and grant mappings etc.) >> + >> + This option is hardwired to true for x86 PVH dom0's (as RAM belonging to >> + other domains in the system don't live in a compatible address space), and >> + is ignored for ARM. >> + >> +* The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up DMA >> + remapping for all non-RAM regions below 4GB except for unusable ranges. > I don't thinks this is PV-specific, just its default is. >From arch_iommu_hwdom_init(): /* Inclusive mappings are enabled by default for PV. */ if ( iommu_hwdom_inclusive == -1 ) iommu_hwdom_inclusive = is_pv_domain(d); /* Reserved IOMMU mappings are enabled by default. */ if ( iommu_hwdom_reserved == -1 ) iommu_hwdom_reserved = 1; if ( iommu_hwdom_inclusive && !is_pv_domain(d) ) { printk(XENLOG_WARNING "IOMMU inclusive mappings are only supported on PV Dom0\n"); iommu_hwdom_inclusive = 0; } Attempting to use this option with a PVH dom0 will cause Xen to force it off. ~Andrew [-- Attachment #1.2: Type: text/html, Size: 5570 bytes --] [-- Attachment #2: Type: text/plain, Size: 157 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-21 12:44 ` Andrew Cooper @ 2019-01-04 9:07 ` Jan Beulich 0 siblings, 0 replies; 18+ messages in thread From: Jan Beulich @ 2019-01-04 9:07 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Xen-devel, Roger Pau Monne >>> On 21.12.18 at 13:44, <andrew.cooper3@citrix.com> wrote: > On 21/12/2018 12:08, Jan Beulich wrote: >>>>> On 21.12.18 at 00:40, <andrew.cooper3@citrix.com> wrote: >>> --- a/docs/misc/xen-command-line.markdown >>> +++ b/docs/misc/xen-command-line.markdown >>> @@ -636,55 +636,76 @@ trace feature is only enabled in debugging builds of Xen. >>> >>> Specify the bit width of the DMA heap. >>> >>> -### dom0 (x86) >>> -> `= List of [ pvh | shadow ]` >>> +### dom0 >>> +> `= List of [ pvh=<bool>, shadow=<bool> ]` >>> >>> -> Sub-options: >>> - >>> -> `pvh` >>> +> Applicability: x86 >> Why the new tag, when everything else uses (x86) next to the >> option name? > > See the commit message of c/s a3a99df44e5405d2092ec59087681765fa4cdee7 > > The problem is with the generated HTML anchors when trying to cross > reference the options. That commit message doesn't talk about anchors, so I can only guess about the connection. But I think I can see your point. From a purely textual perspective I have to admit I'd prefer the original model (without the new tag), but if cross refs don't work I agree it's better to switch. >>> ### dom0-iommu >>> -> `= List of [ passthrough | strict | map-inclusive ]` >>> - >>> -This list of booleans controls the iommu usage by Dom0: >>> - >>> -* `passthrough`: disables DMA remapping for Dom0. Default is `false`. Note that >>> - this option is hard coded to `false` for a PVH Dom0 and any attempt to >>> - overwrite it from the command line is ignored. >>> - >>> -* `strict`: sets up DMA remapping only for the RAM Dom0 actually got assigned. >>> - Default is `false` which means Dom0 will get mappings for all the host >>> - RAM except regions in use by Xen. Note that this option is hard coded to >>> - `true` for a PVH Dom0 and any attempt to overwrite it from the command line >>> - is ignored. >>> - >>> -* `map-inclusive`: sets up DMA remapping for all the non-RAM regions below 4GB >>> - except for unusable ranges. Use this to work around firmware issues providing >>> - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU >>> - accesses for Dom0, with this option all pages up to 4GB, not marked as >>> - unusable in the E820 table, will get a mapping established. Note that this >>> - option is only applicable to a PV Dom0 and is enabled by default on Intel >>> - hardware. >>> - >>> -* `map-reserved`: sets up DMA remapping for all the reserved regions in the >>> - memory map for Dom0. Use this to work around firmware issues providing >>> - incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU >>> - accesses for Dom0, all memory regions marked as reserved in the memory map >>> - that don't overlap with any MMIO region from emulated devices will be >>> - identity mapped. This option maps a subset of the memory that would be >>> - mapped when using the `map-inclusive` option. This option is available to all >>> - Dom0 modes and is enabled by default on Intel hardware. >>> +> `= List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>, >>> +> map-reserved=<bool> ]` >>> + >>> +Controls for the dom0 IOMMU setup. >>> + >>> +* The `passthrough` boolean is applicable to x86 PV dom0's only and defaults >>> + to false. It controls whether the IOMMU is fully disabled for devices >>> + belonging to dom0 (`passthrough=1`), or whether the IOMMU is set up with >>> + an identity transform for dom0 (`passthrough=0`) to prevent dom0 from >>> + DMA'ing outside of its permitted areas. >>> + >>> + This option is hardwired to false for x86 PVH dom0's (where a non-identity >>> + transform is required for dom0 to function), and is ignored for ARM. >>> + >>> +* The `strict` boolean is applicable to x86 PV dom0's only and defaults to >>> + false. It controls whether dom0 can have IOMMU mappings for all domain >>> + RAM in the system, or only for its allocated RAM (and grant mappings etc.) >>> + >>> + This option is hardwired to true for x86 PVH dom0's (as RAM belonging to >>> + other domains in the system don't live in a compatible address space), and >>> + is ignored for ARM. >>> + >>> +* The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up DMA >>> + remapping for all non-RAM regions below 4GB except for unusable ranges. >> I don't thinks this is PV-specific, just its default is. > > From arch_iommu_hwdom_init(): > > /* Inclusive mappings are enabled by default for PV. */ > if ( iommu_hwdom_inclusive == -1 ) > iommu_hwdom_inclusive = is_pv_domain(d); > /* Reserved IOMMU mappings are enabled by default. */ > if ( iommu_hwdom_reserved == -1 ) > iommu_hwdom_reserved = 1; > > if ( iommu_hwdom_inclusive && !is_pv_domain(d) ) > { > printk(XENLOG_WARNING > "IOMMU inclusive mappings are only supported on PV Dom0\n"); > iommu_hwdom_inclusive = 0; > } > > > Attempting to use this option with a PVH dom0 will cause Xen to force it > off. Oh, I see - one of the bad effects of disconnects in source code (the 3rd if() would better be else-if() for the 1st). Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-20 23:40 ` [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= Andrew Cooper 2018-12-21 12:08 ` Jan Beulich @ 2018-12-21 12:08 ` Roger Pau Monné 2018-12-21 13:13 ` Andrew Cooper 1 sibling, 1 reply; 18+ messages in thread From: Roger Pau Monné @ 2018-12-21 12:08 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On Thu, Dec 20, 2018 at 11:40:51PM +0000, Andrew Cooper wrote: > Update to the latest metadata style, and expand each of the clauses with more > information, including applicable CONFIG_* options. > > Drop the redundant comment beside parse_dom0_param(), to avoid it getting out > of sync with the main documentation. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks! A couple of fixes below, because the original text is actually wrong... > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien.grall@arm.com> > > Please double check for correctness. The text matches my > understanding/reading of the code, but some of it is rather subtle going. > > It occurs to me that: > > * The choice of dom0 boot mode should in part be derived from the available > CONFIG_* options, and ELF notes advertised in the dom0 kernel. This is indeed doable, but would require parsing the dom0 kernel before building the domain. > > * AMD probably needs to gain an `ivmd=` to mirror `rmrr=` on the Intel side, > because we know there are other errors in the IVRS table. Yes, albeit using rmrr is quite cumbersome because it's mostly a trial-and-error process until there are no more iommu faults (unless you can get the correct rmrr command for your hardware somewhere). > > * Neither of map-{inclusive,reserved} should be active by default, even on > Intel hardware, and we should (wherever possible) have quirks like we have > for all other firmware screwups. Requiring the user to diagnose/work > around firmware problems like this is quite rude. That would indeed be nice, but I think there are too many vendor firmware versions to be able to correctly identify such quirks, the more that vendors don't even list missing RMRR as erratum. > +Controls for the dom0 IOMMU setup. > + > +* The `passthrough` boolean is applicable to x86 PV dom0's only and defaults > + to false. It controls whether the IOMMU is fully disabled for devices > + belonging to dom0 (`passthrough=1`), or whether the IOMMU is set up with > + an identity transform for dom0 (`passthrough=0`) to prevent dom0 from > + DMA'ing outside of its permitted areas. > + > + This option is hardwired to false for x86 PVH dom0's (where a non-identity > + transform is required for dom0 to function), and is ignored for ARM. > + > +* The `strict` boolean is applicable to x86 PV dom0's only and defaults to > + false. It controls whether dom0 can have IOMMU mappings for all domain > + RAM in the system, or only for its allocated RAM (and grant mappings etc.) > + > + This option is hardwired to true for x86 PVH dom0's (as RAM belonging to > + other domains in the system don't live in a compatible address space), and > + is ignored for ARM. > + > +* The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up DMA > + remapping for all non-RAM regions below 4GB except for unusable ranges. > + > + Typically, some devices in a system use bits of RAM for communication, and > + these areas should be listed via RMRR or IVMD entries in the APCI tables, > + so Xen can ensure that they are identity-mapped in the IOMMU. However, > + some firmware makes mistakes writing its APCI tables, and this option is a > + coarse-grain workaround for those errors. > + > + Where possible, finer grain corrections should be made with the `rmrr=`, > + `ivrs_hpet=` or `ivrs_ioapic=` command line options. > + > + This option is enabled by default on x86 Intel systems, disabled by > + default on other x86 systems, and invalid on ARM systems. I'm afraid the previous text was wrong. I later discovered that AMD also had such workarounds applied by default, and unified the code, but failed to update the documentation, sorry. map-inclusive is enabled by default on x86 for a PV dom0. See xen/drivers/passthrough/x86/iommu.c:215 (arch_iommu_hwdom_init). > + > +* The `map-reserved` functionality is very similar to `map-inclusive`, but is > + applicable to both x86 PV and PVH dom0's, and represents a subset of the > + correction by only mapping reserved memory regions rather than all non-RAM > + regions. > + > + This option is enabled by default on x86 Intel systems, disabled by > + default on other x86 systems, and invalid on ARM systems. map-reserved is enabled by default on x86, xen/drivers/passthrough/x86/iommu.c:218 (arch_iommu_hwdom_init). The text itself looks OK to me. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-21 12:08 ` Roger Pau Monné @ 2018-12-21 13:13 ` Andrew Cooper 2018-12-21 16:06 ` Roger Pau Monné 0 siblings, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2018-12-21 13:13 UTC (permalink / raw) To: Roger Pau Monné Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On 21/12/2018 12:08, Roger Pau Monné wrote: > On Thu, Dec 20, 2018 at 11:40:51PM +0000, Andrew Cooper wrote: >> Update to the latest metadata style, and expand each of the clauses with more >> information, including applicable CONFIG_* options. >> >> Drop the redundant comment beside parse_dom0_param(), to avoid it getting out >> of sync with the main documentation. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Thanks! A couple of fixes below, because the original text is actually > wrong... TBH, that is my default assumption every time I do work like this :) > >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Wei Liu <wei.liu2@citrix.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Stefano Stabellini <sstabellini@kernel.org> >> CC: Julien Grall <julien.grall@arm.com> >> >> Please double check for correctness. The text matches my >> understanding/reading of the code, but some of it is rather subtle going. >> >> It occurs to me that: >> >> * The choice of dom0 boot mode should in part be derived from the available >> CONFIG_* options, and ELF notes advertised in the dom0 kernel. > This is indeed doable, but would require parsing the dom0 kernel > before building the domain. I don't see anything wrong with parsing the ELF headers ahead of building the domain. From the overall boot time, its just an order-of-operations issue. > >> * AMD probably needs to gain an `ivmd=` to mirror `rmrr=` on the Intel side, >> because we know there are other errors in the IVRS table. > Yes, albeit using rmrr is quite cumbersome because it's mostly a > trial-and-error process until there are no more iommu faults (unless > you can get the correct rmrr command for your hardware somewhere). > >> * Neither of map-{inclusive,reserved} should be active by default, even on >> Intel hardware, and we should (wherever possible) have quirks like we have >> for all other firmware screwups. Requiring the user to diagnose/work >> around firmware problems like this is quite rude. > That would indeed be nice, but I think there are too many vendor > firmware versions to be able to correctly identify such quirks, the > more that vendors don't even list missing RMRR as erratum. I don't agree. We already have quirks based on DMI (at the moment, mainly for reboot overrides), and the vast majority of the offending cases are the BMC shared mailbox, which will be in a fixed per-platform location. I don't expect we'll ever find and fix all quirks, but where we do find suitable ones, we should put them into the boot code. > >> +Controls for the dom0 IOMMU setup. >> + >> +* The `passthrough` boolean is applicable to x86 PV dom0's only and defaults >> + to false. It controls whether the IOMMU is fully disabled for devices >> + belonging to dom0 (`passthrough=1`), or whether the IOMMU is set up with >> + an identity transform for dom0 (`passthrough=0`) to prevent dom0 from >> + DMA'ing outside of its permitted areas. >> + >> + This option is hardwired to false for x86 PVH dom0's (where a non-identity >> + transform is required for dom0 to function), and is ignored for ARM. >> + >> +* The `strict` boolean is applicable to x86 PV dom0's only and defaults to >> + false. It controls whether dom0 can have IOMMU mappings for all domain >> + RAM in the system, or only for its allocated RAM (and grant mappings etc.) >> + >> + This option is hardwired to true for x86 PVH dom0's (as RAM belonging to >> + other domains in the system don't live in a compatible address space), and >> + is ignored for ARM. >> + >> +* The `map-inclusive` boolean is applicable to x86 PV dom0's, and sets up DMA >> + remapping for all non-RAM regions below 4GB except for unusable ranges. >> + >> + Typically, some devices in a system use bits of RAM for communication, and >> + these areas should be listed via RMRR or IVMD entries in the APCI tables, >> + so Xen can ensure that they are identity-mapped in the IOMMU. However, >> + some firmware makes mistakes writing its APCI tables, and this option is a >> + coarse-grain workaround for those errors. >> + >> + Where possible, finer grain corrections should be made with the `rmrr=`, >> + `ivrs_hpet=` or `ivrs_ioapic=` command line options. >> + >> + This option is enabled by default on x86 Intel systems, disabled by >> + default on other x86 systems, and invalid on ARM systems. > I'm afraid the previous text was wrong. I later discovered that AMD > also had such workarounds applied by default, and unified the code, > but failed to update the documentation, sorry. > > map-inclusive is enabled by default on x86 for a PV dom0. See > xen/drivers/passthrough/x86/iommu.c:215 (arch_iommu_hwdom_init). > >> + >> +* The `map-reserved` functionality is very similar to `map-inclusive`, but is >> + applicable to both x86 PV and PVH dom0's, and represents a subset of the >> + correction by only mapping reserved memory regions rather than all non-RAM >> + regions. >> + >> + This option is enabled by default on x86 Intel systems, disabled by >> + default on other x86 systems, and invalid on ARM systems. > map-reserved is enabled by default on x86, > xen/drivers/passthrough/x86/iommu.c:218 (arch_iommu_hwdom_init). Ok for both. Will fix up. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-21 13:13 ` Andrew Cooper @ 2018-12-21 16:06 ` Roger Pau Monné 2018-12-24 12:46 ` Andrew Cooper 0 siblings, 1 reply; 18+ messages in thread From: Roger Pau Monné @ 2018-12-21 16:06 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On Fri, Dec 21, 2018 at 01:13:25PM +0000, Andrew Cooper wrote: > On 21/12/2018 12:08, Roger Pau Monné wrote: > > On Thu, Dec 20, 2018 at 11:40:51PM +0000, Andrew Cooper wrote: > >> Update to the latest metadata style, and expand each of the clauses with more > >> information, including applicable CONFIG_* options. > >> > >> Drop the redundant comment beside parse_dom0_param(), to avoid it getting out > >> of sync with the main documentation. > >> > >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Thanks! A couple of fixes below, because the original text is actually > > wrong... > > TBH, that is my default assumption every time I do work like this :) In this case it's my fault :(, because I changed the code and forgot about the docs. > > > >> --- > >> CC: Jan Beulich <JBeulich@suse.com> > >> CC: Wei Liu <wei.liu2@citrix.com> > >> CC: Roger Pau Monné <roger.pau@citrix.com> > >> CC: Stefano Stabellini <sstabellini@kernel.org> > >> CC: Julien Grall <julien.grall@arm.com> > >> > >> Please double check for correctness. The text matches my > >> understanding/reading of the code, but some of it is rather subtle going. > >> > >> It occurs to me that: > >> > >> * The choice of dom0 boot mode should in part be derived from the available > >> CONFIG_* options, and ELF notes advertised in the dom0 kernel. > > This is indeed doable, but would require parsing the dom0 kernel > > before building the domain. > > I don't see anything wrong with parsing the ELF headers ahead of > building the domain. From the overall boot time, its just an > order-of-operations issue. Oh yes, I didn't mean my comment to sound like criticism. I agree there should be no issues in parsing the ELF earlier, or if there are issues they should be fixed. > > > >> * AMD probably needs to gain an `ivmd=` to mirror `rmrr=` on the Intel side, > >> because we know there are other errors in the IVRS table. > > Yes, albeit using rmrr is quite cumbersome because it's mostly a > > trial-and-error process until there are no more iommu faults (unless > > you can get the correct rmrr command for your hardware somewhere). > > > >> * Neither of map-{inclusive,reserved} should be active by default, even on > >> Intel hardware, and we should (wherever possible) have quirks like we have > >> for all other firmware screwups. Requiring the user to diagnose/work > >> around firmware problems like this is quite rude. > > That would indeed be nice, but I think there are too many vendor > > firmware versions to be able to correctly identify such quirks, the > > more that vendors don't even list missing RMRR as erratum. > > I don't agree. We already have quirks based on DMI (at the moment, > mainly for reboot overrides), and the vast majority of the offending > cases are the BMC shared mailbox, which will be in a fixed per-platform > location. IIRC I've only found a single box that worked without map-reserved, and that's my NUC which has firmware from Intel. And even in that case the USB ports weren't fully working. I guess such quirks could be applied based on the chipset version then if Xen realizes the firmware is either wrong or missing obvious RMRR regions? > I don't expect we'll ever find and fix all quirks, but where we do find > suitable ones, we should put them into the boot code. Sadly I agree. What I'm worried about is turning the default map-{inclusive/reserved} to off, that's likely to make dom0 unable to boot on a huge amount of hardware. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-21 16:06 ` Roger Pau Monné @ 2018-12-24 12:46 ` Andrew Cooper 2019-01-04 12:04 ` Roger Pau Monné 0 siblings, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2018-12-24 12:46 UTC (permalink / raw) To: Roger Pau Monné Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On 21/12/2018 16:06, Roger Pau Monné wrote: > On Fri, Dec 21, 2018 at 01:13:25PM +0000, Andrew Cooper wrote: >> On 21/12/2018 12:08, Roger Pau Monné wrote: >>> On Thu, Dec 20, 2018 at 11:40:51PM +0000, Andrew Cooper wrote: >>>> * AMD probably needs to gain an `ivmd=` to mirror `rmrr=` on the Intel side, >>>> because we know there are other errors in the IVRS table. >>> Yes, albeit using rmrr is quite cumbersome because it's mostly a >>> trial-and-error process until there are no more iommu faults (unless >>> you can get the correct rmrr command for your hardware somewhere). >>> >>>> * Neither of map-{inclusive,reserved} should be active by default, even on >>>> Intel hardware, and we should (wherever possible) have quirks like we have >>>> for all other firmware screwups. Requiring the user to diagnose/work >>>> around firmware problems like this is quite rude. >>> That would indeed be nice, but I think there are too many vendor >>> firmware versions to be able to correctly identify such quirks, the >>> more that vendors don't even list missing RMRR as erratum. >> I don't agree. We already have quirks based on DMI (at the moment, >> mainly for reboot overrides), and the vast majority of the offending >> cases are the BMC shared mailbox, which will be in a fixed per-platform >> location. > IIRC I've only found a single box that worked without map-reserved, > and that's my NUC which has firmware from Intel. And even in that case > the USB ports weren't fully working. > > I guess such quirks could be applied based on the chipset version then > if Xen realizes the firmware is either wrong or missing obvious RMRR > regions? Ah - I'd forgotten the USB legacy keyboard support case. We ought to be able to reverse engineer a suitable RMRR from the USB port configuration. > >> I don't expect we'll ever find and fix all quirks, but where we do find >> suitable ones, we should put them into the boot code. > Sadly I agree. What I'm worried about is turning the default > map-{inclusive/reserved} to off, that's likely to make dom0 unable to > boot on a huge amount of hardware. We've already got a difference between map-{inclusive/reserved} where the former is enabled for PV and strictly unavailable for PVH, whereas the latter is enabled for both. When it comes to the IOMMU setup, there should not be a difference between PV and PVH, because the difference in virtulisation mode is not relevant to how the devices on the system behave. IMO, map-inclusive should be disabled by default, and (due to map-reserved being on by default), I expect we'll see change in behaviour. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2018-12-24 12:46 ` Andrew Cooper @ 2019-01-04 12:04 ` Roger Pau Monné 2019-01-04 12:06 ` Andrew Cooper 0 siblings, 1 reply; 18+ messages in thread From: Roger Pau Monné @ 2019-01-04 12:04 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On Mon, Dec 24, 2018 at 12:46:06PM +0000, Andrew Cooper wrote: > When it comes to the IOMMU setup, there should not be a difference > between PV and PVH, because the difference in virtulisation mode is not > relevant to how the devices on the system behave. > > IMO, map-inclusive should be disabled by default, and (due to > map-reserved being on by default), I expect we'll see change in behaviour. I'm OK with this, would you like to send a patch to that effect? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= 2019-01-04 12:04 ` Roger Pau Monné @ 2019-01-04 12:06 ` Andrew Cooper 0 siblings, 0 replies; 18+ messages in thread From: Andrew Cooper @ 2019-01-04 12:06 UTC (permalink / raw) To: Roger Pau Monné Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On 04/01/2019 12:04, Roger Pau Monné wrote: > On Mon, Dec 24, 2018 at 12:46:06PM +0000, Andrew Cooper wrote: >> When it comes to the IOMMU setup, there should not be a difference >> between PV and PVH, because the difference in virtulisation mode is not >> relevant to how the devices on the system behave. >> >> IMO, map-inclusive should be disabled by default, and (due to >> map-reserved being on by default), I expect we'll see change in behaviour. > I'm OK with this, would you like to send a patch to that effect? There is a patch to this effect in the v2 series which I posted. I'll have to post a v3 to rebase over the strncmp(... ss - s) issue, but the content in the series is broadly fine. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option 2018-12-20 23:40 [PATCH 0/2] Ease development with a PVH Xen and XTF PVH dom0 Andrew Cooper 2018-12-20 23:40 ` [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= Andrew Cooper @ 2018-12-20 23:40 ` Andrew Cooper 2018-12-21 12:13 ` Jan Beulich 2018-12-21 12:44 ` Roger Pau Monné 1 sibling, 2 replies; 18+ messages in thread From: Andrew Cooper @ 2018-12-20 23:40 UTC (permalink / raw) To: Xen-devel Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Julien Grall, Jan Beulich, Roger Pau Monné For development purposes, it is very convenient to boot Xen as a PVH guest, with an XTF PV or PVH "dom0". The edit-compile-go cycle is a matter of seconds, and you can resonably insert printk() debugging in places which which would be completely infeasible when booting fully-fledged guests. However, the PVH dom0 path insists on having a working IOMMU, which doesn't exist when virtualised as a PVH guest, and isn't necessary for XTF anyway. Introduce a developer mode to skip the IOMMU requirement. To fix a corner case with command line parsing, cmdline_strcmp() is introduced. Because we no longer tokenise comma separated list with NUL's, strcmp(line, "opt") doesn't work for a string in the middle of the comma separated list, and strncmp("opt", s, ss - s) eagerly matches all options which begin with "opt". Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien.grall@arm.com> Slightly RFC. I've been carrying this patch locally for ages, but decided that the approach is more likely to be accepted: diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index c68a722..87f0fd9 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -117,8 +117,6 @@ int arch_iommu_populate_page_table(struct domain *d) void __hwdom_init arch_iommu_check_autotranslated_hwdom(struct domain *d) { - if ( !iommu_enabled ) - panic("Presently, iommu must be enabled for PVH hardware domain\n"); } int arch_iommu_domain_init(struct domain *d) --- docs/misc/xen-command-line.markdown | 8 +++++++- xen/common/kernel.c | 21 +++++++++++++++++++++ xen/drivers/passthrough/iommu.c | 6 +++++- xen/include/xen/lib.h | 7 +++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 94ee703..eb0a65e 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -663,7 +663,7 @@ Controls for how dom0 is constructed on x86 systems. ### dom0-iommu > `= List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>, -> map-reserved=<bool> ]` +> map-reserved=<bool>, none ]` Controls for the dom0 IOMMU setup. @@ -707,6 +707,12 @@ Controls for the dom0 IOMMU setup. This option is enabled by default on x86 Intel systems, disabled by default on other x86 systems, and invalid on ARM systems. +* The `none` option is intended for development purposes only, and skips + certain safety checks pertaining to the correct IOMMU configuration for + dom0 to boot. + + Incorrect use of this option may result in a malfunctioning system. + ### dom0\_ioports\_disable (x86) > `= List of <hex>-<hex>` diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 5766a0f..fa2d9f3 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -4,6 +4,7 @@ * Copyright (c) 2002-2005 K A Fraser */ +#include <xen/ctype.h> #include <xen/init.h> #include <xen/lib.h> #include <xen/errno.h> @@ -271,6 +272,26 @@ int parse_boolean(const char *name, const char *s, const char *e) return -1; } +int cmdline_strcmp(const char *frag, const char *name) +{ + while ( 1 ) + { + int res = (*frag - *name); + + if ( res || *name == '\0' ) + { + /* NUL in 'name' matching punctuation in 'frag' implies success. */ + if ( *name == '\0' && ispunct(*frag) ) + res = 0; + + return res; + } + + frag++; + name++; + } +} + unsigned int tainted; /** diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c index ac62d7f..67efb10 100644 --- a/xen/drivers/passthrough/iommu.c +++ b/xen/drivers/passthrough/iommu.c @@ -59,6 +59,7 @@ bool_t __read_mostly iommu_snoop = 1; bool_t __read_mostly iommu_qinval = 1; bool_t __read_mostly iommu_intremap = 1; +bool __hwdom_initdata iommu_hwdom_none; bool __hwdom_initdata iommu_hwdom_strict; bool __read_mostly iommu_hwdom_passthrough; int8_t __hwdom_initdata iommu_hwdom_inclusive = -1; @@ -158,6 +159,8 @@ static int __init parse_dom0_iommu_param(const char *s) iommu_hwdom_inclusive = val; else if ( (val = parse_boolean("map-reserved", s, ss)) >= 0 ) iommu_hwdom_reserved = val; + else if ( !cmdline_strcmp(s, "none") ) + iommu_hwdom_none = true; else rc = -EINVAL; @@ -189,7 +192,8 @@ static void __hwdom_init check_hwdom_reqs(struct domain *d) if ( !paging_mode_translate(d) ) return; - arch_iommu_check_autotranslated_hwdom(d); + if ( !iommu_hwdom_none ) + arch_iommu_check_autotranslated_hwdom(d); iommu_hwdom_passthrough = false; iommu_hwdom_strict = true; diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h index 972fc84..58a7ea9 100644 --- a/xen/include/xen/lib.h +++ b/xen/include/xen/lib.h @@ -79,6 +79,13 @@ int parse_bool(const char *s, const char *e); */ int parse_boolean(const char *name, const char *s, const char *e); +/** + * Very similar to strcmp(), but will declare a match if the NUL in 'name' + * lines up with punctuationin 'frag'. Designed for picking exact string + * matches out of a comma-separated command line fragment. + */ +int cmdline_strcmp(const char *frag, const char *name); + /*#define DEBUG_TRACE_DUMP*/ #ifdef DEBUG_TRACE_DUMP extern void debugtrace_dump(void); -- 2.1.4 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option 2018-12-20 23:40 ` [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option Andrew Cooper @ 2018-12-21 12:13 ` Jan Beulich 2018-12-21 12:55 ` Andrew Cooper 2018-12-21 12:44 ` Roger Pau Monné 1 sibling, 1 reply; 18+ messages in thread From: Jan Beulich @ 2018-12-21 12:13 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Xen-devel, Roger Pau Monne >>> On 21.12.18 at 00:40, <andrew.cooper3@citrix.com> wrote: > @@ -271,6 +272,26 @@ int parse_boolean(const char *name, const char *s, const char *e) > return -1; > } > > +int cmdline_strcmp(const char *frag, const char *name) __init ? > +{ > + while ( 1 ) > + { > + int res = (*frag - *name); > + > + if ( res || *name == '\0' ) > + { > + /* NUL in 'name' matching punctuation in 'frag' implies success. */ > + if ( *name == '\0' && ispunct(*frag) ) > + res = 0; Isn't ispunct() true for dashes and perhaps also underscores? I don't think it can be this generic, the more that ... > --- a/xen/include/xen/lib.h > +++ b/xen/include/xen/lib.h > @@ -79,6 +79,13 @@ int parse_bool(const char *s, const char *e); > */ > int parse_boolean(const char *name, const char *s, const char *e); > > +/** > + * Very similar to strcmp(), but will declare a match if the NUL in 'name' > + * lines up with punctuationin 'frag'. Designed for picking exact string > + * matches out of a comma-separated command line fragment. > + */ > +int cmdline_strcmp(const char *frag, const char *name); ... you talk of commas only here. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option 2018-12-21 12:13 ` Jan Beulich @ 2018-12-21 12:55 ` Andrew Cooper 2019-01-04 9:11 ` Jan Beulich 0 siblings, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2018-12-21 12:55 UTC (permalink / raw) To: Jan Beulich Cc: Julien Grall, Stefano Stabellini, Wei Liu, Xen-devel, Roger Pau Monne On 21/12/2018 12:13, Jan Beulich wrote: >>>> On 21.12.18 at 00:40, <andrew.cooper3@citrix.com> wrote: >> @@ -271,6 +272,26 @@ int parse_boolean(const char *name, const char *s, const char *e) >> return -1; >> } >> >> +int cmdline_strcmp(const char *frag, const char *name) > __init ? I think there are some runtime parameters in need of some fixing as well. > >> +{ >> + while ( 1 ) >> + { >> + int res = (*frag - *name); >> + >> + if ( res || *name == '\0' ) >> + { >> + /* NUL in 'name' matching punctuation in 'frag' implies success. */ >> + if ( *name == '\0' && ispunct(*frag) ) >> + res = 0; > Isn't ispunct() true for dashes and perhaps also underscores? > I don't think it can be this generic, the more that ... > >> --- a/xen/include/xen/lib.h >> +++ b/xen/include/xen/lib.h >> @@ -79,6 +79,13 @@ int parse_bool(const char *s, const char *e); >> */ >> int parse_boolean(const char *name, const char *s, const char *e); >> >> +/** >> + * Very similar to strcmp(), but will declare a match if the NUL in 'name' >> + * lines up with punctuationin 'frag'. Designed for picking exact string >> + * matches out of a comma-separated command line fragment. >> + */ >> +int cmdline_strcmp(const char *frag, const char *name); > ... you talk of commas only here. I actually borrowed this function from my CPUID cmdline patch. In 99% of cases, we only need to match = and , but we have some other parameters such as psr= which use : for delimiters, hence the use of ispunct(). As an alternative, I could revert back to explicitly checking the expected punctuation. It is not as if this is a fastpath. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option 2018-12-21 12:55 ` Andrew Cooper @ 2019-01-04 9:11 ` Jan Beulich 0 siblings, 0 replies; 18+ messages in thread From: Jan Beulich @ 2019-01-04 9:11 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Xen-devel, Roger Pau Monne >>> On 21.12.18 at 13:55, <andrew.cooper3@citrix.com> wrote: > On 21/12/2018 12:13, Jan Beulich wrote: >>>>> On 21.12.18 at 00:40, <andrew.cooper3@citrix.com> wrote: >>> @@ -271,6 +272,26 @@ int parse_boolean(const char *name, const char *s, const char *e) >>> return -1; >>> } >>> >>> +int cmdline_strcmp(const char *frag, const char *name) >> __init ? > > I think there are some runtime parameters in need of some fixing as well. Ideally we'd drop __init at the point such a change indeed goes in. As a compromise you may want to mention the reason for the omission in the description (preferably with a concrete example). >>> +{ >>> + while ( 1 ) >>> + { >>> + int res = (*frag - *name); >>> + >>> + if ( res || *name == '\0' ) >>> + { >>> + /* NUL in 'name' matching punctuation in 'frag' implies success. */ >>> + if ( *name == '\0' && ispunct(*frag) ) >>> + res = 0; >> Isn't ispunct() true for dashes and perhaps also underscores? >> I don't think it can be this generic, the more that ... >> >>> --- a/xen/include/xen/lib.h >>> +++ b/xen/include/xen/lib.h >>> @@ -79,6 +79,13 @@ int parse_bool(const char *s, const char *e); >>> */ >>> int parse_boolean(const char *name, const char *s, const char *e); >>> >>> +/** >>> + * Very similar to strcmp(), but will declare a match if the NUL in 'name' >>> + * lines up with punctuationin 'frag'. Designed for picking exact string >>> + * matches out of a comma-separated command line fragment. >>> + */ >>> +int cmdline_strcmp(const char *frag, const char *name); >> ... you talk of commas only here. > > I actually borrowed this function from my CPUID cmdline patch. In 99% > of cases, we only need to match = and , but we have some other > parameters such as psr= which use : for delimiters, hence the use of > ispunct(). > > As an alternative, I could revert back to explicitly checking the > expected punctuation. It is not as if this is a fastpath. Imo this would be better than treating punctuation we don't want to consider separators the same as intended-to-be-separators. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option 2018-12-20 23:40 ` [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option Andrew Cooper 2018-12-21 12:13 ` Jan Beulich @ 2018-12-21 12:44 ` Roger Pau Monné 2018-12-21 13:01 ` Andrew Cooper 1 sibling, 1 reply; 18+ messages in thread From: Roger Pau Monné @ 2018-12-21 12:44 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On Thu, Dec 20, 2018 at 11:40:52PM +0000, Andrew Cooper wrote: > For development purposes, it is very convenient to boot Xen as a PVH guest, > with an XTF PV or PVH "dom0". The edit-compile-go cycle is a matter of > seconds, and you can resonably insert printk() debugging in places which which > would be completely infeasible when booting fully-fledged guests. > > However, the PVH dom0 path insists on having a working IOMMU, which doesn't > exist when virtualised as a PVH guest, and isn't necessary for XTF anyway. > > Introduce a developer mode to skip the IOMMU requirement. This looks very similar to the current 'passthrough' option, maybe it would be enough to allow PVH dom0 to use the passthrough option provided a warning is added to arch_iommu_check_autotranslated_hwdom? Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option 2018-12-21 12:44 ` Roger Pau Monné @ 2018-12-21 13:01 ` Andrew Cooper 2018-12-21 15:55 ` Roger Pau Monné 0 siblings, 1 reply; 18+ messages in thread From: Andrew Cooper @ 2018-12-21 13:01 UTC (permalink / raw) To: Roger Pau Monné Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On 21/12/2018 12:44, Roger Pau Monné wrote: > On Thu, Dec 20, 2018 at 11:40:52PM +0000, Andrew Cooper wrote: >> For development purposes, it is very convenient to boot Xen as a PVH guest, >> with an XTF PV or PVH "dom0". The edit-compile-go cycle is a matter of >> seconds, and you can resonably insert printk() debugging in places which which >> would be completely infeasible when booting fully-fledged guests. >> >> However, the PVH dom0 path insists on having a working IOMMU, which doesn't >> exist when virtualised as a PVH guest, and isn't necessary for XTF anyway. >> >> Introduce a developer mode to skip the IOMMU requirement. > This looks very similar to the current 'passthrough' option, maybe it > would be enough to allow PVH dom0 to use the passthrough option > provided a warning is added to > arch_iommu_check_autotranslated_hwdom? I considered that, but "dom0-iommu=passthrough" isn't an accurate description of what is going on. Frankly, its not correct for PV either. TBH, dom0-iommu=none is better for both. How about I introduce that as the new option, and leave passthrough as a legacy alias? ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option 2018-12-21 13:01 ` Andrew Cooper @ 2018-12-21 15:55 ` Roger Pau Monné 0 siblings, 0 replies; 18+ messages in thread From: Roger Pau Monné @ 2018-12-21 15:55 UTC (permalink / raw) To: Andrew Cooper Cc: Julien Grall, Stefano Stabellini, Wei Liu, Jan Beulich, Xen-devel On Fri, Dec 21, 2018 at 01:01:22PM +0000, Andrew Cooper wrote: > On 21/12/2018 12:44, Roger Pau Monné wrote: > > On Thu, Dec 20, 2018 at 11:40:52PM +0000, Andrew Cooper wrote: > >> For development purposes, it is very convenient to boot Xen as a PVH guest, > >> with an XTF PV or PVH "dom0". The edit-compile-go cycle is a matter of > >> seconds, and you can resonably insert printk() debugging in places which which > >> would be completely infeasible when booting fully-fledged guests. > >> > >> However, the PVH dom0 path insists on having a working IOMMU, which doesn't > >> exist when virtualised as a PVH guest, and isn't necessary for XTF anyway. > >> > >> Introduce a developer mode to skip the IOMMU requirement. > > This looks very similar to the current 'passthrough' option, maybe it > > would be enough to allow PVH dom0 to use the passthrough option > > provided a warning is added to > > arch_iommu_check_autotranslated_hwdom? > > I considered that, but "dom0-iommu=passthrough" isn't an accurate > description of what is going on. Frankly, its not correct for PV either. And what about turning dom0-iommu into a boolean option itself, so that you can do "dom0-iommu=false"? I think that's more similar to other Xen command line options that have a boolean value and/or a list of sub-options. > TBH, dom0-iommu=none is better for both. How about I introduce that as > the new option, and leave passthrough as a legacy alias? That sounds good, I would like to avoid (if possible) the proliferation of new iommu_hwdom_* variables, because we already have a bunch and the functionality introduced by the 'none' option looks very similar to what 'passthrough' aims to achieve. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-01-04 12:06 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-20 23:40 [PATCH 0/2] Ease development with a PVH Xen and XTF PVH dom0 Andrew Cooper 2018-12-20 23:40 ` [PATCH 1/2] xen/dom0: Improve documentation for dom0= and dom0-iommu= Andrew Cooper 2018-12-21 12:08 ` Jan Beulich 2018-12-21 12:44 ` Andrew Cooper 2019-01-04 9:07 ` Jan Beulich 2018-12-21 12:08 ` Roger Pau Monné 2018-12-21 13:13 ` Andrew Cooper 2018-12-21 16:06 ` Roger Pau Monné 2018-12-24 12:46 ` Andrew Cooper 2019-01-04 12:04 ` Roger Pau Monné 2019-01-04 12:06 ` Andrew Cooper 2018-12-20 23:40 ` [PATCH 2/2] xen/dom0: Add a dom0-iommu=none option Andrew Cooper 2018-12-21 12:13 ` Jan Beulich 2018-12-21 12:55 ` Andrew Cooper 2019-01-04 9:11 ` Jan Beulich 2018-12-21 12:44 ` Roger Pau Monné 2018-12-21 13:01 ` Andrew Cooper 2018-12-21 15:55 ` Roger Pau Monné
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.