| Message ID | 20231031120215.3307356-1-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-4.18] docs: Fix IOMMU command line docs some more | expand |
Hi Andrew, > On Oct 31, 2023, at 20:02, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > Make the command line docs match the actual implementation, and state that the > default behaviour is selected at compile time. > > Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices optional") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Release-acked-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
On Tue, Oct 31, 2023 at 12:02:15PM +0000, Andrew Cooper wrote: > Make the command line docs match the actual implementation, and state that the > default behaviour is selected at compile time. > > Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices optional") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Wei Liu <wl@xen.org> > CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > CC: Henry Wang <Henry.Wang@arm.com> > --- > docs/misc/xen-command-line.pandoc | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc > index 6b07d0f3a17f..9a19a04157cb 100644 > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses to that port. > > Default: `new` unless directed-EOI is supported > > ### iommu > - = List of [ <bool>, verbose, debug, force, required, quarantine[=scratch-page], > + = List of [ <bool>, verbose, debug, force, required, > + quarantine=<bool>|scratch-page, I think this should be quarantine=[<bool>|scratch-page], as just using iommu=quarantine is a valid syntax and will enable basic quarantine. IOW: the bool or scratch-page parameters are optional. > sharept, superpages, intremap, intpost, crash-disable, > snoop, qinval, igfx, amd-iommu-perdev-intremap, > dom0-{passthrough,strict} ] > @@ -1519,7 +1520,8 @@ boolean (e.g. `iommu=no`) can override this and leave the IOMMUs disabled. > successfully. > > * The `quarantine` option can be used to control Xen's behavior when > - de-assigning devices from guests. > + de-assigning devices from guests. The default behaviour is chosen at > + compile time, and is one of `CONFIG_IOMMU_QUARANTINE_{NONE,BASIC,SCRATCH_PAGE}`. Do we also want to state that the current build time default is BASIC if the user hasn't selected otherwise? It's kind of problematic though, as distros might select a different build time default and then the documentation would be out of sync. Thanks, Roger.
On 31/10/2023 12:24 pm, Roger Pau Monné wrote: > On Tue, Oct 31, 2023 at 12:02:15PM +0000, Andrew Cooper wrote: >> Make the command line docs match the actual implementation, and state that the >> default behaviour is selected at compile time. >> >> Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices optional") >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> >> --- >> CC: Jan Beulich <JBeulich@suse.com> >> CC: Roger Pau Monné <roger.pau@citrix.com> >> CC: Wei Liu <wl@xen.org> >> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >> CC: Henry Wang <Henry.Wang@arm.com> >> --- >> docs/misc/xen-command-line.pandoc | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc >> index 6b07d0f3a17f..9a19a04157cb 100644 >> --- a/docs/misc/xen-command-line.pandoc >> +++ b/docs/misc/xen-command-line.pandoc >> @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses to that port. >> > Default: `new` unless directed-EOI is supported >> >> ### iommu >> - = List of [ <bool>, verbose, debug, force, required, quarantine[=scratch-page], >> + = List of [ <bool>, verbose, debug, force, required, >> + quarantine=<bool>|scratch-page, > I think this should be quarantine=[<bool>|scratch-page], as just using > iommu=quarantine is a valid syntax and will enable basic quarantine. > IOW: the bool or scratch-page parameters are optional. =<bool> already has that meaning, and this is the form we use elsewhere. > >> sharept, superpages, intremap, intpost, crash-disable, >> snoop, qinval, igfx, amd-iommu-perdev-intremap, >> dom0-{passthrough,strict} ] >> @@ -1519,7 +1520,8 @@ boolean (e.g. `iommu=no`) can override this and leave the IOMMUs disabled. >> successfully. >> >> * The `quarantine` option can be used to control Xen's behavior when >> - de-assigning devices from guests. >> + de-assigning devices from guests. The default behaviour is chosen at >> + compile time, and is one of `CONFIG_IOMMU_QUARANTINE_{NONE,BASIC,SCRATCH_PAGE}`. > Do we also want to state that the current build time default is BASIC > if the user hasn't selected otherwise? This is an instruction to look at the .config file and see which it is. The exceptional case of someone doing a build from clean isn't particularly interesting. Not least because they will be prompted for it and given the choices. ~Andrew
On Tue, Oct 31, 2023 at 01:29:04PM +0000, Andrew Cooper wrote: > On 31/10/2023 12:24 pm, Roger Pau Monné wrote: > > On Tue, Oct 31, 2023 at 12:02:15PM +0000, Andrew Cooper wrote: > >> Make the command line docs match the actual implementation, and state that the > >> default behaviour is selected at compile time. > >> > >> Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices optional") > >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > >> --- > >> CC: Jan Beulich <JBeulich@suse.com> > >> CC: Roger Pau Monné <roger.pau@citrix.com> > >> CC: Wei Liu <wl@xen.org> > >> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > >> CC: Henry Wang <Henry.Wang@arm.com> > >> --- > >> docs/misc/xen-command-line.pandoc | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc > >> index 6b07d0f3a17f..9a19a04157cb 100644 > >> --- a/docs/misc/xen-command-line.pandoc > >> +++ b/docs/misc/xen-command-line.pandoc > >> @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses to that port. > >> > Default: `new` unless directed-EOI is supported > >> > >> ### iommu > >> - = List of [ <bool>, verbose, debug, force, required, quarantine[=scratch-page], > >> + = List of [ <bool>, verbose, debug, force, required, > >> + quarantine=<bool>|scratch-page, > > I think this should be quarantine=[<bool>|scratch-page], as just using > > iommu=quarantine is a valid syntax and will enable basic quarantine. > > IOW: the bool or scratch-page parameters are optional. > > =<bool> already has that meaning, and this is the form we use elsewhere. I guess I got confused by some other options using `[ ]` to denote optional parameters, but I see it's not used by all of them. Thanks, Roger.
On 31/10/2023 1:45 pm, Roger Pau Monné wrote: > On Tue, Oct 31, 2023 at 01:29:04PM +0000, Andrew Cooper wrote: >> On 31/10/2023 12:24 pm, Roger Pau Monné wrote: >>> On Tue, Oct 31, 2023 at 12:02:15PM +0000, Andrew Cooper wrote: >>>> Make the command line docs match the actual implementation, and state that the >>>> default behaviour is selected at compile time. >>>> >>>> Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices optional") >>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks. > >>>> --- >>>> CC: Jan Beulich <JBeulich@suse.com> >>>> CC: Roger Pau Monné <roger.pau@citrix.com> >>>> CC: Wei Liu <wl@xen.org> >>>> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> >>>> CC: Henry Wang <Henry.Wang@arm.com> >>>> --- >>>> docs/misc/xen-command-line.pandoc | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc >>>> index 6b07d0f3a17f..9a19a04157cb 100644 >>>> --- a/docs/misc/xen-command-line.pandoc >>>> +++ b/docs/misc/xen-command-line.pandoc >>>> @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses to that port. >>>> > Default: `new` unless directed-EOI is supported >>>> >>>> ### iommu >>>> - = List of [ <bool>, verbose, debug, force, required, quarantine[=scratch-page], >>>> + = List of [ <bool>, verbose, debug, force, required, >>>> + quarantine=<bool>|scratch-page, >>> I think this should be quarantine=[<bool>|scratch-page], as just using >>> iommu=quarantine is a valid syntax and will enable basic quarantine. >>> IOW: the bool or scratch-page parameters are optional. >> =<bool> already has that meaning, and this is the form we use elsewhere. > I guess I got confused by some other options using `[ ]` to denote > optional parameters, but I see it's not used by all of them. Yeah, it's a mess, sadly. One of many things I've not had time to fix, but at least this is closer to the normal syntax than before. ~Andrew
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 6b07d0f3a17f..9a19a04157cb 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -1480,7 +1480,8 @@ detection of systems known to misbehave upon accesses to that port. > Default: `new` unless directed-EOI is supported ### iommu - = List of [ <bool>, verbose, debug, force, required, quarantine[=scratch-page], + = List of [ <bool>, verbose, debug, force, required, + quarantine=<bool>|scratch-page, sharept, superpages, intremap, intpost, crash-disable, snoop, qinval, igfx, amd-iommu-perdev-intremap, dom0-{passthrough,strict} ] @@ -1519,7 +1520,8 @@ boolean (e.g. `iommu=no`) can override this and leave the IOMMUs disabled. successfully. * The `quarantine` option can be used to control Xen's behavior when - de-assigning devices from guests. + de-assigning devices from guests. The default behaviour is chosen at + compile time, and is one of `CONFIG_IOMMU_QUARANTINE_{NONE,BASIC,SCRATCH_PAGE}`. When a PCI device is assigned to an untrusted domain, it is possible for that domain to program the device to DMA to an arbitrary address.
Make the command line docs match the actual implementation, and state that the default behaviour is selected at compile time. Fixes: 980d6acf1517 ("IOMMU: make DMA containment of quarantined devices optional") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> CC: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> CC: Henry Wang <Henry.Wang@arm.com> --- docs/misc/xen-command-line.pandoc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) base-commit: 9659b2a6d73b14620e187f9c626a09323853c459